Improve ux around ACL token to help users avoid overwriting node/service identities (#16506)

* Deprecate merge-node-identities and merge-service-identities flags

* added tests for node identities changes

* added changelog file and docs
This commit is contained in:
Ronald 2023-03-06 16:00:39 +01:00 committed by GitHub
parent dac0cc90ed
commit 7f6f12089f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 125 additions and 29 deletions

8
.changelog/16506.txt Normal file
View File

@ -0,0 +1,8 @@
```release-note:deprecation
cli: Deprecate the `-merge-node-identites` and `-merge-service-identities` flags from the `consul token update` command in favor of: `-append-node-identity` and `-append-service-identity`.
```
```release-note:improvement
cli: added `-append-service-identity` and `-append-node-identity` flags to the `consul token update` command.
These flags allow updates to a token's node identities/service identities without having to override them.
```

View File

@ -25,37 +25,35 @@ type cmd struct {
http *flags.HTTPFlags http *flags.HTTPFlags
help string help string
tokenAccessorID string tokenAccessorID string
policyIDs []string policyIDs []string
appendPolicyIDs []string appendPolicyIDs []string
policyNames []string policyNames []string
appendPolicyNames []string appendPolicyNames []string
roleIDs []string roleIDs []string
appendRoleIDs []string appendRoleIDs []string
roleNames []string roleNames []string
appendRoleNames []string appendRoleNames []string
serviceIdents []string serviceIdents []string
nodeIdents []string nodeIdents []string
description string appendNodeIdents []string
mergeServiceIdents bool appendServiceIdents []string
mergeNodeIdents bool description string
showMeta bool showMeta bool
format string format string
// DEPRECATED // DEPRECATED
mergeRoles bool mergeServiceIdents bool
mergePolicies bool mergeNodeIdents bool
tokenID string mergeRoles bool
mergePolicies bool
tokenID string
} }
func (c *cmd) init() { func (c *cmd) init() {
c.flags = flag.NewFlagSet("", flag.ContinueOnError) c.flags = flag.NewFlagSet("", flag.ContinueOnError)
c.flags.BoolVar(&c.showMeta, "meta", false, "Indicates that token metadata such "+ c.flags.BoolVar(&c.showMeta, "meta", false, "Indicates that token metadata such "+
"as the content hash and raft indices should be shown for each entry") "as the content hash and raft indices should be shown for each entry")
c.flags.BoolVar(&c.mergeServiceIdents, "merge-service-identities", false, "Merge the new service identities "+
"with the existing service identities")
c.flags.BoolVar(&c.mergeNodeIdents, "merge-node-identities", false, "Merge the new node identities "+
"with the existing node identities")
c.flags.StringVar(&c.tokenAccessorID, "accessor-id", "", "The Accessor ID of the token to update. "+ c.flags.StringVar(&c.tokenAccessorID, "accessor-id", "", "The Accessor ID of the token to update. "+
"It may be specified as a unique ID prefix but will error if the prefix "+ "It may be specified as a unique ID prefix but will error if the prefix "+
"matches multiple token Accessor IDs") "matches multiple token Accessor IDs")
@ -79,9 +77,15 @@ func (c *cmd) init() {
c.flags.Var((*flags.AppendSliceValue)(&c.serviceIdents), "service-identity", "Name of a "+ c.flags.Var((*flags.AppendSliceValue)(&c.serviceIdents), "service-identity", "Name of a "+
"service identity to use for this token. May be specified multiple times. Format is "+ "service identity to use for this token. May be specified multiple times. Format is "+
"the SERVICENAME or SERVICENAME:DATACENTER1,DATACENTER2,...") "the SERVICENAME or SERVICENAME:DATACENTER1,DATACENTER2,...")
c.flags.Var((*flags.AppendSliceValue)(&c.appendServiceIdents), "append-service-identity", "Name of a "+
"service identity to use for this token. This token retains existing service identities. May be specified"+
"multiple times. Format is the SERVICENAME or SERVICENAME:DATACENTER1,DATACENTER2,...")
c.flags.Var((*flags.AppendSliceValue)(&c.nodeIdents), "node-identity", "Name of a "+ c.flags.Var((*flags.AppendSliceValue)(&c.nodeIdents), "node-identity", "Name of a "+
"node identity to use for this token. May be specified multiple times. Format is "+ "node identity to use for this token. May be specified multiple times. Format is "+
"NODENAME:DATACENTER") "NODENAME:DATACENTER")
c.flags.Var((*flags.AppendSliceValue)(&c.appendNodeIdents), "append-node-identity", "Name of a "+
"node identity to use for this token. This token retains existing node identities. May be "+
"specified multiple times. Format is NODENAME:DATACENTER")
c.flags.StringVar( c.flags.StringVar(
&c.format, &c.format,
"format", "format",
@ -101,6 +105,10 @@ func (c *cmd) init() {
"Use -append-policy-id or -append-policy-name instead.") "Use -append-policy-id or -append-policy-name instead.")
c.flags.BoolVar(&c.mergeRoles, "merge-roles", false, "DEPRECATED. "+ c.flags.BoolVar(&c.mergeRoles, "merge-roles", false, "DEPRECATED. "+
"Use -append-role-id or -append-role-name instead.") "Use -append-role-id or -append-role-name instead.")
c.flags.BoolVar(&c.mergeServiceIdents, "merge-service-identities", false, "DEPRECATED. "+
"Use -append-service-identity instead.")
c.flags.BoolVar(&c.mergeNodeIdents, "merge-node-identities", false, "DEPRECATED. "+
"Use -append-node-identity instead.")
} }
func (c *cmd) Run(args []string) int { func (c *cmd) Run(args []string) int {
@ -147,13 +155,38 @@ func (c *cmd) Run(args []string) int {
t.Description = c.description t.Description = c.description
} }
hasAppendServiceFields := len(c.appendServiceIdents) > 0
hasServiceFields := len(c.serviceIdents) > 0
if hasAppendServiceFields && hasServiceFields {
c.UI.Error("Cannot combine the use of service-identity flag with append-service-identity. " +
"To set or overwrite existing service identities, use -service-identity. " +
"To append to existing service identities, use -append-service-identity.")
return 1
}
parsedServiceIdents, err := acl.ExtractServiceIdentities(c.serviceIdents) parsedServiceIdents, err := acl.ExtractServiceIdentities(c.serviceIdents)
if hasAppendServiceFields {
parsedServiceIdents, err = acl.ExtractServiceIdentities(c.appendServiceIdents)
}
if err != nil { if err != nil {
c.UI.Error(err.Error()) c.UI.Error(err.Error())
return 1 return 1
} }
hasAppendNodeFields := len(c.appendNodeIdents) > 0
hasNodeFields := len(c.nodeIdents) > 0
if hasAppendNodeFields && hasNodeFields {
c.UI.Error("Cannot combine the use of node-identity flag with append-node-identity. " +
"To set or overwrite existing node identities, use -node-identity. " +
"To append to existing node identities, use -append-node-identity.")
return 1
}
parsedNodeIdents, err := acl.ExtractNodeIdentities(c.nodeIdents) parsedNodeIdents, err := acl.ExtractNodeIdentities(c.nodeIdents)
if hasAppendNodeFields {
parsedNodeIdents, err = acl.ExtractNodeIdentities(c.appendNodeIdents)
}
if err != nil { if err != nil {
c.UI.Error(err.Error()) c.UI.Error(err.Error())
return 1 return 1
@ -310,7 +343,7 @@ func (c *cmd) Run(args []string) int {
} }
} }
if c.mergeServiceIdents { if c.mergeServiceIdents || hasAppendServiceFields {
for _, svcid := range parsedServiceIdents { for _, svcid := range parsedServiceIdents {
found := -1 found := -1
for i, link := range t.ServiceIdentities { for i, link := range t.ServiceIdentities {
@ -330,7 +363,7 @@ func (c *cmd) Run(args []string) int {
t.ServiceIdentities = parsedServiceIdents t.ServiceIdentities = parsedServiceIdents
} }
if c.mergeNodeIdents { if c.mergeNodeIdents || hasAppendNodeFields {
for _, nodeid := range parsedNodeIdents { for _, nodeid := range parsedNodeIdents {
found := false found := false
for _, link := range t.NodeIdentities { for _, link := range t.NodeIdentities {

View File

@ -109,6 +109,22 @@ func TestTokenUpdateCommand(t *testing.T) {
require.ElementsMatch(t, expected, token.NodeIdentities) require.ElementsMatch(t, expected, token.NodeIdentities)
}) })
// update with append-node-identity
t.Run("append-node-identity", func(t *testing.T) {
token := run(t, []string{
"-http-addr=" + a.HTTPAddr(),
"-accessor-id=" + token.AccessorID,
"-token=root",
"-append-node-identity=third:node",
"-description=test token",
})
require.Len(t, token.NodeIdentities, 3)
require.Equal(t, "third", token.NodeIdentities[2].NodeName)
require.Equal(t, "node", token.NodeIdentities[2].Datacenter)
})
// update with policy by name // update with policy by name
t.Run("policy-name", func(t *testing.T) { t.Run("policy-name", func(t *testing.T) {
token := run(t, []string{ token := run(t, []string{
@ -135,6 +151,33 @@ func TestTokenUpdateCommand(t *testing.T) {
require.Len(t, token.Policies, 1) require.Len(t, token.Policies, 1)
}) })
// update with service-identity
t.Run("service-identity", func(t *testing.T) {
token := run(t, []string{
"-http-addr=" + a.HTTPAddr(),
"-accessor-id=" + token.AccessorID,
"-token=root",
"-service-identity=service:datapalace",
"-description=test token",
})
require.Len(t, token.ServiceIdentities, 1)
require.Equal(t, "service", token.ServiceIdentities[0].ServiceName)
})
// update with append-service-identity
t.Run("append-service-identity", func(t *testing.T) {
token := run(t, []string{
"-http-addr=" + a.HTTPAddr(),
"-accessor-id=" + token.AccessorID,
"-token=root",
"-append-service-identity=web",
"-description=test token",
})
require.Len(t, token.ServiceIdentities, 2)
require.Equal(t, "web", token.ServiceIdentities[1].ServiceName)
})
// update with no description shouldn't delete the current description // update with no description shouldn't delete the current description
t.Run("merge-description", func(t *testing.T) { t.Run("merge-description", func(t *testing.T) {
token := run(t, []string{ token := run(t, []string{

View File

@ -49,6 +49,8 @@ Usage: `consul acl policy update [options] [args]`
the value is a file path to load the rules from. `-` may also be given to the value is a file path to load the rules from. `-` may also be given to
indicate that the rules are available on stdin. indicate that the rules are available on stdin.
~> Specifying `-rules` will overwrite existing rules.
- `-valid-datacenter=<value>` - Datacenter that the policy should be valid within. - `-valid-datacenter=<value>` - Datacenter that the policy should be valid within.
This flag may be specified multiple times. This flag may be specified multiple times.

View File

@ -33,8 +33,9 @@ Usage: `consul acl token update [options]`
- `-id=<string>` - The Accessor ID of the token to read. It may be specified as a - `-id=<string>` - The Accessor ID of the token to read. It may be specified as a
unique ID prefix but will error if the prefix matches multiple token Accessor IDs unique ID prefix but will error if the prefix matches multiple token Accessor IDs
- `merge-node-identities` - Merge the new node identities with the existing node - `-merge-node-identities` - Deprecated. Merge the new node identities with the existing node
identities. identities.
~> This is deprecated and will be removed in a future Consul version. Use `append-node-identity` instead.
- `-merge-policies` - Deprecated. Merge the new policies with the existing policies. - `-merge-policies` - Deprecated. Merge the new policies with the existing policies.
@ -46,15 +47,20 @@ instead.
~> This is deprecated and will be removed in a future Consul version. Use `append-role-id` or `append-role-name` ~> This is deprecated and will be removed in a future Consul version. Use `append-role-id` or `append-role-name`
instead. instead.
- `-merge-service-identities` - Merge the new service identities with the existing service identities. - `-merge-service-identities` - Deprecated. Merge the new service identities with the existing service identities.
~> This is deprecated and will be removed in a future Consul version. Use `append-service-identity` instead.
- `-meta` - Indicates that token metadata such as the content hash and Raft indices should be - `-meta` - Indicates that token metadata such as the content hash and Raft indices should be
shown for each entry. shown for each entry.
- `-node-identity=<value>` - Name of a node identity to use for this role. May - `-node-identity=<value>` - Name of a node identity to use for this role. Overwrites existing node identity. May
be specified multiple times. Format is `NODENAME:DATACENTER`. Added in Consul be specified multiple times. Format is `NODENAME:DATACENTER`. Added in Consul
1.8.1. 1.8.1.
- `-append-node-identity=<value>` - Name of a node identity to add to this role. May
be specified multiple times. The token retains existing node identities. Format is `NODENAME:DATACENTER`.
- `-policy-id=<value>` - ID of a policy to use for this token. Overwrites existing policies. May be specified multiple times. - `-policy-id=<value>` - ID of a policy to use for this token. Overwrites existing policies. May be specified multiple times.
- `-policy-name=<value>` - Name of a policy to use for this token. Overwrites existing policies. May be specified multiple times. - `-policy-name=<value>` - Name of a policy to use for this token. Overwrites existing policies. May be specified multiple times.
@ -76,9 +82,13 @@ instead.
- `-append-role-name=<value>` - Name of a role to add to this token. The token retains existing roles. May be specified multiple times. - `-append-role-name=<value>` - Name of a role to add to this token. The token retains existing roles. May be specified multiple times.
- `-service-identity=<value>` - Name of a service identity to use for this - `-service-identity=<value>` - Name of a service identity to use for this
token. May be specified multiple times. Format is the `SERVICENAME` or token. Overwrites existing service identities. May be specified multiple times. Format is the `SERVICENAME` or
`SERVICENAME:DATACENTER1,DATACENTER2,...` `SERVICENAME:DATACENTER1,DATACENTER2,...`
- `-append-service-identity=<value>` - Name of a service identity to add to this
token. May be specified multiple times. The token retains existing service identities.
Format is the `SERVICENAME` or `SERVICENAME:DATACENTER1,DATACENTER2,...`
- `-format={pretty|json}` - Command output format. The default value is `pretty`. - `-format={pretty|json}` - Command output format. The default value is `pretty`.
#### Enterprise Options #### Enterprise Options