diff --git a/agent/consul/acl_endpoint_test.go b/agent/consul/acl_endpoint_test.go index 7af96994c..d1dedecd1 100644 --- a/agent/consul/acl_endpoint_test.go +++ b/agent/consul/acl_endpoint_test.go @@ -3336,6 +3336,7 @@ func TestACLEndpoint_AuthMethodSet(t *testing.T) { t.Run("Update - allow type to default", func(t *testing.T) { reqMethod := newAuthMethod("test") + reqMethod.DisplayName = "updated display name 1" reqMethod.Description = "test modified 1" reqMethod.Type = "" // unset @@ -3355,12 +3356,14 @@ func TestACLEndpoint_AuthMethodSet(t *testing.T) { method := methodResp.AuthMethod require.Equal(t, method.Name, "test") + require.Equal(t, method.DisplayName, "updated display name 1") require.Equal(t, method.Description, "test modified 1") require.Equal(t, method.Type, "testing") }) t.Run("Update - specify type", func(t *testing.T) { reqMethod := newAuthMethod("test") + reqMethod.DisplayName = "updated display name 2" reqMethod.Description = "test modified 2" req := structs.ACLAuthMethodSetRequest{ @@ -3379,6 +3382,7 @@ func TestACLEndpoint_AuthMethodSet(t *testing.T) { method := methodResp.AuthMethod require.Equal(t, method.Name, "test") + require.Equal(t, method.DisplayName, "updated display name 2") require.Equal(t, method.Description, "test modified 2") require.Equal(t, method.Type, "testing") }) diff --git a/agent/structs/acl.go b/agent/structs/acl.go index be0631079..4676fb28f 100644 --- a/agent/structs/acl.go +++ b/agent/structs/acl.go @@ -994,8 +994,9 @@ func (rules ACLBindingRules) Sort() { type ACLAuthMethodListStub struct { Name string - Description string Type string + DisplayName string `json:",omitempty"` + Description string `json:",omitempty"` CreateIndex uint64 ModifyIndex uint64 EnterpriseMeta @@ -1004,8 +1005,9 @@ type ACLAuthMethodListStub struct { func (p *ACLAuthMethod) Stub() *ACLAuthMethodListStub { return &ACLAuthMethodListStub{ Name: p.Name, - Description: p.Description, Type: p.Type, + DisplayName: p.DisplayName, + Description: p.Description, CreateIndex: p.CreateIndex, ModifyIndex: p.ModifyIndex, EnterpriseMeta: p.EnterpriseMeta, @@ -1038,8 +1040,13 @@ type ACLAuthMethod struct { // Immutable once set and only settable during create. Type string + // DisplayName is an optional name to use instead of the Name field when + // displaying information about this auth method in any kind of user + // interface. + DisplayName string `json:",omitempty"` + // Description is just an optional bunch of explanatory text. - Description string + Description string `json:",omitempty"` // Configuration is arbitrary configuration for the auth method. This // should only contain primitive values and containers (such as lists and diff --git a/api/acl.go b/api/acl.go index f1b6fcd46..197430278 100644 --- a/api/acl.go +++ b/api/acl.go @@ -37,6 +37,7 @@ type ACLToken struct { Roles []*ACLTokenRoleLink `json:",omitempty"` ServiceIdentities []*ACLServiceIdentity `json:",omitempty"` Local bool + AuthMethod string `json:",omitempty"` ExpirationTTL time.Duration `json:",omitempty"` ExpirationTime *time.Time `json:",omitempty"` CreateTime time.Time `json:",omitempty"` @@ -60,6 +61,7 @@ type ACLTokenListEntry struct { Roles []*ACLTokenRoleLink `json:",omitempty"` ServiceIdentities []*ACLServiceIdentity `json:",omitempty"` Local bool + AuthMethod string `json:",omitempty"` ExpirationTime *time.Time `json:",omitempty"` CreateTime time.Time Hash []byte @@ -180,7 +182,8 @@ type ACLBindingRule struct { type ACLAuthMethod struct { Name string Type string - Description string + DisplayName string `json:",omitempty"` + Description string `json:",omitempty"` // Configuration is arbitrary configuration for the auth method. This // should only contain primitive values and containers (such as lists and @@ -198,7 +201,8 @@ type ACLAuthMethod struct { type ACLAuthMethodListEntry struct { Name string Type string - Description string + DisplayName string `json:",omitempty"` + Description string `json:",omitempty"` CreateIndex uint64 ModifyIndex uint64 diff --git a/command/acl/authmethod/create/authmethod_create.go b/command/acl/authmethod/create/authmethod_create.go index 7d45fdd95..b4151f166 100644 --- a/command/acl/authmethod/create/authmethod_create.go +++ b/command/acl/authmethod/create/authmethod_create.go @@ -27,6 +27,7 @@ type cmd struct { authMethodType string name string + displayName string description string k8sHost string @@ -62,6 +63,12 @@ func (c *cmd) init() { "", "The new auth method's name. This flag is required.", ) + c.flags.StringVar( + &c.displayName, + "display-name", + "", + "An optional name to use instead of the name when displaying this auth method in a UI.", + ) c.flags.StringVar( &c.description, "description", @@ -130,6 +137,7 @@ func (c *cmd) Run(args []string) int { newAuthMethod := &api.ACLAuthMethod{ Type: c.authMethodType, Name: c.name, + DisplayName: c.displayName, Description: c.description, } diff --git a/command/acl/authmethod/create/authmethod_create_test.go b/command/acl/authmethod/create/authmethod_create_test.go index 054234857..6ffd6dc15 100644 --- a/command/acl/authmethod/create/authmethod_create_test.go +++ b/command/acl/authmethod/create/authmethod_create_test.go @@ -10,11 +10,12 @@ import ( "github.com/hashicorp/consul/agent" "github.com/hashicorp/consul/agent/connect" + "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/acl" "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/testrpc" + "github.com/hashicorp/go-uuid" "github.com/mitchellh/cli" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" // activate testing auth method @@ -46,6 +47,7 @@ func TestAuthMethodCreateCommand(t *testing.T) { defer a.Shutdown() testrpc.WaitForLeader(t, a.RPC, "dc1") + client := a.Client() t.Run("type required", func(t *testing.T) { args := []string{ @@ -98,6 +100,8 @@ func TestAuthMethodCreateCommand(t *testing.T) { "-token=root", "-type=testing", "-name=test", + "-description=desc", + "-display-name=display", } ui := cli.NewMockUi() @@ -106,6 +110,15 @@ func TestAuthMethodCreateCommand(t *testing.T) { code := cmd.Run(args) require.Equal(t, code, 0) require.Empty(t, ui.ErrorWriter.String()) + + got := getTestMethod(t, client, "test") + expect := &api.ACLAuthMethod{ + Name: "test", + Type: "testing", + DisplayName: "display", + Description: "desc", + } + require.Equal(t, expect, got) }) } @@ -126,6 +139,7 @@ func TestAuthMethodCreateCommand_JSON(t *testing.T) { defer a.Shutdown() testrpc.WaitForLeader(t, a.RPC, "dc1") + client := a.Client() t.Run("type required", func(t *testing.T) { args := []string{ @@ -148,6 +162,8 @@ func TestAuthMethodCreateCommand_JSON(t *testing.T) { "-token=root", "-type=testing", "-name=test", + "-description=desc", + "-display-name=display", "-format=json", } @@ -162,8 +178,16 @@ func TestAuthMethodCreateCommand_JSON(t *testing.T) { require.Contains(t, out, "test") var jsonOutput json.RawMessage - err := json.Unmarshal([]byte(out), &jsonOutput) - assert.NoError(t, err) + require.NoError(t, json.Unmarshal([]byte(out), &jsonOutput)) + + got := getTestMethod(t, client, "test") + expect := &api.ACLAuthMethod{ + Name: "test", + Type: "testing", + DisplayName: "display", + Description: "desc", + } + require.Equal(t, expect, got) }) } @@ -184,13 +208,15 @@ func TestAuthMethodCreateCommand_k8s(t *testing.T) { defer a.Shutdown() testrpc.WaitForLeader(t, a.RPC, "dc1") + client := a.Client() t.Run("k8s host required", func(t *testing.T) { + name := getTestName(t) args := []string{ "-http-addr=" + a.HTTPAddr(), "-token=root", "-type=kubernetes", - "-name=k8s", + "-name", name, } ui := cli.NewMockUi() @@ -202,11 +228,12 @@ func TestAuthMethodCreateCommand_k8s(t *testing.T) { }) t.Run("k8s ca cert required", func(t *testing.T) { + name := getTestName(t) args := []string{ "-http-addr=" + a.HTTPAddr(), "-token=root", "-type=kubernetes", - "-name=k8s", + "-name", name, "-kubernetes-host=https://foo.internal:8443", } @@ -221,11 +248,12 @@ func TestAuthMethodCreateCommand_k8s(t *testing.T) { ca := connect.TestCA(t, nil) t.Run("k8s jwt required", func(t *testing.T) { + name := getTestName(t) args := []string{ "-http-addr=" + a.HTTPAddr(), "-token=root", "-type=kubernetes", - "-name=k8s", + "-name", name, "-kubernetes-host=https://foo.internal:8443", "-kubernetes-ca-cert", ca.RootCert, } @@ -239,11 +267,12 @@ func TestAuthMethodCreateCommand_k8s(t *testing.T) { }) t.Run("create k8s", func(t *testing.T) { + name := getTestName(t) args := []string{ "-http-addr=" + a.HTTPAddr(), "-token=root", "-type=kubernetes", - "-name=k8s", + "-name", name, "-kubernetes-host", "https://foo.internal:8443", "-kubernetes-ca-cert", ca.RootCert, "-kubernetes-service-account-jwt", acl.TestKubernetesJWT_A, @@ -255,17 +284,30 @@ func TestAuthMethodCreateCommand_k8s(t *testing.T) { code := cmd.Run(args) require.Equal(t, code, 0) require.Empty(t, ui.ErrorWriter.String()) + + got := getTestMethod(t, client, name) + expect := &api.ACLAuthMethod{ + Name: name, + Type: "kubernetes", + Config: map[string]interface{}{ + "Host": "https://foo.internal:8443", + "CACert": ca.RootCert, + "ServiceAccountJWT": acl.TestKubernetesJWT_A, + }, + } + require.Equal(t, expect, got) }) caFile := filepath.Join(testDir, "ca.crt") require.NoError(t, ioutil.WriteFile(caFile, []byte(ca.RootCert), 0600)) t.Run("create k8s with cert file", func(t *testing.T) { + name := getTestName(t) args := []string{ "-http-addr=" + a.HTTPAddr(), "-token=root", "-type=kubernetes", - "-name=k8s", + "-name", name, "-kubernetes-host", "https://foo.internal:8443", "-kubernetes-ca-cert", "@" + caFile, "-kubernetes-service-account-jwt", acl.TestKubernetesJWT_A, @@ -277,5 +319,42 @@ func TestAuthMethodCreateCommand_k8s(t *testing.T) { code := cmd.Run(args) require.Equal(t, code, 0) require.Empty(t, ui.ErrorWriter.String()) + + got := getTestMethod(t, client, name) + expect := &api.ACLAuthMethod{ + Name: name, + Type: "kubernetes", + Config: map[string]interface{}{ + "Host": "https://foo.internal:8443", + "CACert": ca.RootCert, + "ServiceAccountJWT": acl.TestKubernetesJWT_A, + }, + } + require.Equal(t, expect, got) }) } + +func getTestMethod(t *testing.T, client *api.Client, methodName string) *api.ACLAuthMethod { + t.Helper() + + method, _, err := client.ACL().AuthMethodRead( + methodName, + &api.QueryOptions{Token: "root"}, + ) + require.NoError(t, err) + require.NotNil(t, method) + + // zero these out since we don't really care + method.CreateIndex = 0 + method.ModifyIndex = 0 + + return method +} + +func getTestName(t *testing.T) string { + t.Helper() + + id, err := uuid.GenerateUUID() + require.NoError(t, err) + return "test-" + id +} diff --git a/command/acl/authmethod/formatter.go b/command/acl/authmethod/formatter.go index ef6f1c271..4f1aaeee0 100644 --- a/command/acl/authmethod/formatter.go +++ b/command/acl/authmethod/formatter.go @@ -54,6 +54,9 @@ func (f *prettyFormatter) FormatAuthMethod(method *api.ACLAuthMethod) (string, e if method.Namespace != "" { buffer.WriteString(fmt.Sprintf("Namespace: %s\n", method.Namespace)) } + if method.DisplayName != "" { + buffer.WriteString(fmt.Sprintf("DisplayName: %s\n", method.DisplayName)) + } buffer.WriteString(fmt.Sprintf("Description: %s\n", method.Description)) if f.showMeta { buffer.WriteString(fmt.Sprintf("Create Index: %d\n", method.CreateIndex)) @@ -87,6 +90,9 @@ func (f *prettyFormatter) formatAuthMethodListEntry(method *api.ACLAuthMethodLis if method.Namespace != "" { buffer.WriteString(fmt.Sprintf(" Namespace: %s\n", method.Namespace)) } + if method.DisplayName != "" { + buffer.WriteString(fmt.Sprintf(" DisplayName: %s\n", method.DisplayName)) + } buffer.WriteString(fmt.Sprintf(" Description: %s\n", method.Description)) if f.showMeta { buffer.WriteString(fmt.Sprintf(" Create Index: %d\n", method.CreateIndex)) diff --git a/command/acl/authmethod/update/authmethod_update.go b/command/acl/authmethod/update/authmethod_update.go index a28a2adc9..483d1df7e 100644 --- a/command/acl/authmethod/update/authmethod_update.go +++ b/command/acl/authmethod/update/authmethod_update.go @@ -27,6 +27,7 @@ type cmd struct { name string + displayName string description string k8sHost string @@ -58,6 +59,13 @@ func (c *cmd) init() { "The auth method name.", ) + c.flags.StringVar( + &c.displayName, + "display-name", + "", + "An optional name to use instead of the name when displaying this auth method in a UI.", + ) + c.flags.StringVar( &c.description, "description", @@ -147,6 +155,7 @@ func (c *cmd) Run(args []string) int { method = &api.ACLAuthMethod{ Name: currentAuthMethod.Name, Type: currentAuthMethod.Type, + DisplayName: c.displayName, Description: c.description, } @@ -172,6 +181,9 @@ func (c *cmd) Run(args []string) int { methodCopy := *currentAuthMethod method = &methodCopy + if c.displayName != "" { + method.DisplayName = c.displayName + } if c.description != "" { method.Description = c.description } diff --git a/command/acl/authmethod/update/authmethod_update_test.go b/command/acl/authmethod/update/authmethod_update_test.go index 46903dc34..a8435d3c4 100644 --- a/command/acl/authmethod/update/authmethod_update_test.go +++ b/command/acl/authmethod/update/authmethod_update_test.go @@ -16,7 +16,6 @@ import ( "github.com/hashicorp/consul/testrpc" "github.com/hashicorp/go-uuid" "github.com/mitchellh/cli" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" // activate testing auth method @@ -66,10 +65,11 @@ func TestAuthMethodUpdateCommand(t *testing.T) { }) t.Run("update nonexistent method", func(t *testing.T) { + name := getTestName(t) args := []string{ "-http-addr=" + a.HTTPAddr(), "-token=root", - "-name=test", + "-name", name, } ui := cli.NewMockUi() @@ -106,6 +106,7 @@ func TestAuthMethodUpdateCommand(t *testing.T) { "-http-addr=" + a.HTTPAddr(), "-token=root", "-name=" + name, + "-display-name", "updated display", "-description", "updated description", } @@ -116,13 +117,15 @@ func TestAuthMethodUpdateCommand(t *testing.T) { require.Equal(t, code, 0) require.Empty(t, ui.ErrorWriter.String()) - method, _, err := client.ACL().AuthMethodRead( - name, - &api.QueryOptions{Token: "root"}, - ) - require.NoError(t, err) - require.NotNil(t, method) - require.Equal(t, "updated description", method.Description) + got := getTestMethod(t, client, name) + expect := &api.ACLAuthMethod{ + Name: name, + Type: "testing", + DisplayName: "updated display", + Description: "updated description", + Config: map[string]interface{}{}, + } + require.Equal(t, expect, got) }) } @@ -187,6 +190,7 @@ func TestAuthMethodUpdateCommand_JSON(t *testing.T) { "-http-addr=" + a.HTTPAddr(), "-token=root", "-name=" + name, + "-display-name", "updated display", "-description", "updated description", "-format=json", } @@ -195,22 +199,23 @@ func TestAuthMethodUpdateCommand_JSON(t *testing.T) { cmd := New(ui) code := cmd.Run(args) + output := ui.OutputWriter.String() + require.Equal(t, code, 0) require.Empty(t, ui.ErrorWriter.String()) - method, _, err := client.ACL().AuthMethodRead( - name, - &api.QueryOptions{Token: "root"}, - ) - require.NoError(t, err) - require.NotNil(t, method) - require.Equal(t, "updated description", method.Description) - - output := ui.OutputWriter.String() - var jsonOutput json.RawMessage - err = json.Unmarshal([]byte(output), &jsonOutput) - assert.NoError(t, err) + require.NoError(t, json.Unmarshal([]byte(output), &jsonOutput)) + + got := getTestMethod(t, client, name) + expect := &api.ACLAuthMethod{ + Name: name, + Type: "testing", + DisplayName: "updated display", + Description: "updated description", + Config: map[string]interface{}{}, + } + require.Equal(t, expect, got) }) } @@ -250,11 +255,12 @@ func TestAuthMethodUpdateCommand_noMerge(t *testing.T) { }) t.Run("update nonexistent method", func(t *testing.T) { + name := getTestName(t) args := []string{ "-http-addr=" + a.HTTPAddr(), "-token=root", "-no-merge", - "-name=test", + "-name", name, } ui := cli.NewMockUi() @@ -292,6 +298,7 @@ func TestAuthMethodUpdateCommand_noMerge(t *testing.T) { "-token=root", "-no-merge", "-name=" + name, + "-display-name", "updated display", "-description", "updated description", } @@ -302,13 +309,14 @@ func TestAuthMethodUpdateCommand_noMerge(t *testing.T) { require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String()) require.Empty(t, ui.ErrorWriter.String()) - method, _, err := client.ACL().AuthMethodRead( - name, - &api.QueryOptions{Token: "root"}, - ) - require.NoError(t, err) - require.NotNil(t, method) - require.Equal(t, "updated description", method.Description) + got := getTestMethod(t, client, name) + expect := &api.ACLAuthMethod{ + Name: name, + Type: "testing", + DisplayName: "updated display", + Description: "updated description", + } + require.Equal(t, expect, got) }) } @@ -366,6 +374,7 @@ func TestAuthMethodUpdateCommand_k8s(t *testing.T) { "-http-addr=" + a.HTTPAddr(), "-token=root", "-name=" + name, + "-display-name", "updated display", "-description", "updated description", "-kubernetes-host", "https://foo-new.internal:8443", "-kubernetes-ca-cert", ca2.RootCert, @@ -379,15 +388,22 @@ func TestAuthMethodUpdateCommand_k8s(t *testing.T) { require.Equal(t, code, 0) require.Empty(t, ui.ErrorWriter.String()) - method, _, err := client.ACL().AuthMethodRead( - name, - &api.QueryOptions{Token: "root"}, - ) - require.NoError(t, err) - require.NotNil(t, method) - require.Equal(t, "updated description", method.Description) + got := getTestMethod(t, client, name) + expect := &api.ACLAuthMethod{ + Name: name, + Type: "kubernetes", + DisplayName: "updated display", + Description: "updated description", + Config: map[string]interface{}{ + "Host": "https://foo-new.internal:8443", + "CACert": ca2.RootCert, + "ServiceAccountJWT": acl.TestKubernetesJWT_B, + }, + } + require.Equal(t, expect, got) - config, err := api.ParseKubernetesAuthMethodConfig(method.Config) + // also just double check our convenience parsing + config, err := api.ParseKubernetesAuthMethodConfig(got.Config) require.NoError(t, err) require.Equal(t, "https://foo-new.internal:8443", config.Host) require.Equal(t, ca2.RootCert, config.CACert) @@ -726,3 +742,28 @@ func TestAuthMethodUpdateCommand_k8s_noMerge(t *testing.T) { require.Equal(t, acl.TestKubernetesJWT_B, config.ServiceAccountJWT) }) } + +func getTestMethod(t *testing.T, client *api.Client, methodName string) *api.ACLAuthMethod { + t.Helper() + + method, _, err := client.ACL().AuthMethodRead( + methodName, + &api.QueryOptions{Token: "root"}, + ) + require.NoError(t, err) + require.NotNil(t, method) + + // zero these out since we don't really care + method.CreateIndex = 0 + method.ModifyIndex = 0 + + return method +} + +func getTestName(t *testing.T) string { + t.Helper() + + id, err := uuid.GenerateUUID() + require.NoError(t, err) + return "test-" + id +} diff --git a/command/acl/token/formatter.go b/command/acl/token/formatter.go index 7180a513b..d5bb781c5 100644 --- a/command/acl/token/formatter.go +++ b/command/acl/token/formatter.go @@ -57,6 +57,9 @@ func (f *prettyFormatter) FormatToken(token *api.ACLToken) (string, error) { } buffer.WriteString(fmt.Sprintf("Description: %s\n", token.Description)) buffer.WriteString(fmt.Sprintf("Local: %t\n", token.Local)) + if token.AuthMethod != "" { + buffer.WriteString(fmt.Sprintf("Auth Method: %s\n", token.AuthMethod)) + } buffer.WriteString(fmt.Sprintf("Create Time: %v\n", token.CreateTime)) if token.ExpirationTime != nil && !token.ExpirationTime.IsZero() { buffer.WriteString(fmt.Sprintf("Expiration Time: %v\n", *token.ExpirationTime)) @@ -121,6 +124,9 @@ func (f *prettyFormatter) formatTokenListEntry(token *api.ACLTokenListEntry) str } buffer.WriteString(fmt.Sprintf("Description: %s\n", token.Description)) buffer.WriteString(fmt.Sprintf("Local: %t\n", token.Local)) + if token.AuthMethod != "" { + buffer.WriteString(fmt.Sprintf("Auth Method: %s\n", token.AuthMethod)) + } buffer.WriteString(fmt.Sprintf("Create Time: %v\n", token.CreateTime)) if token.ExpirationTime != nil && !token.ExpirationTime.IsZero() { buffer.WriteString(fmt.Sprintf("Expiration Time: %v\n", *token.ExpirationTime)) diff --git a/website/pages/api-docs/acl/auth-methods.mdx b/website/pages/api-docs/acl/auth-methods.mdx index 681fe66f2..3c104e91b 100644 --- a/website/pages/api-docs/acl/auth-methods.mdx +++ b/website/pages/api-docs/acl/auth-methods.mdx @@ -40,6 +40,7 @@ The table below shows this endpoint's support for - `Name` `(string: )` - Specifies a name for the ACL auth method. The name can contain alphanumeric characters, dashes `-`, and underscores `_`. This field is immutable and must be unique. + - `Type` `(string: )` - The type of auth method being configured. The only allowed value in Consul 1.5.0 is `"kubernetes"`. This field is immutable. @@ -47,6 +48,9 @@ The table below shows this endpoint's support for - `Description` `(string: "")` - Free form human readable description of the auth method. +- `DisplayName` `(string: "")` - An optional name to use instead of the `Name` + field when displaying information about this auth method. Added in Consul 1.8.0. + - `Config` `(map[string]string: )` - The raw configuration to use for the chosen auth method. Contents will vary depending upon the type chosen. For more information on configuring specific auth method types, see the [auth @@ -184,6 +188,9 @@ The table below shows this endpoint's support for - `Description` `(string: "")` - Free form human readable description of the auth method. +- `DisplayName` `(string: "")` - An optional name to use instead of the `Name` + field when displaying information about this auth method. Added in Consul 1.8.0. + - `Config` `(map[string]string: )` - The raw configuration to use for the chosen auth method. Contents will vary depending upon the type chosen. For more information on configuring specific auth method types, see the [auth