rename to append and update tests

hashicorp · Mar 1, 2023 · e7c0a45 · e7c0a45
1 parent f404886
commit e7c0a45
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 52 deletions.
diff --git a/.changelog/16288.txt b/.changelog/16288.txt
@@ -1,8 +1,8 @@
 ```release-note:deprecation
-cli: Deprecate the `-merge-policies` and `-merge-roles` flags from the `consul token update` command in favour of: `-add-policy-id`, `-add-policy-name`, `-add-role-name`, and `-add-role-id`.
+cli: Deprecate the `-merge-policies` and `-merge-roles` flags from the `consul token update` command in favor of: `-append-policy-id`, `-append-policy-name`, `-append-role-name`, and `-append-role-id`.
 ```
 
 ```release-note:improvement
-cli: added `-add-policy-id`, `-add-policy-name`, `-add-role-name`, and `-add-role-id` flags to the `consul token update` command.
+cli: added `-append-policy-id`, `-append-policy-name`, `-append-role-name`, and `-append-role-id` flags to the `consul token update` command.
 These flags will allow updates to token's policies/roles without having to override them completely.  
 ```
diff --git a/command/acl/token/update/token_update.go b/command/acl/token/update/token_update.go
@@ -27,13 +27,13 @@ type cmd struct {
 
 	tokenAccessorID    string
 	policyIDs          []string
-	addPolicyIDs       []string
+	appendPolicyIDs    []string
 	policyNames        []string
-	addPolicyNames     []string
+	appendPolicyNames  []string
 	roleIDs            []string
-	addRoleIDs         []string
+	appendRoleIDs      []string
 	roleNames          []string
-	addRoleNames       []string
+	appendRoleNames    []string
 	serviceIdents      []string
 	nodeIdents         []string
 	description        string
@@ -61,20 +61,20 @@ func (c *cmd) init() {
 		"matches multiple token Accessor IDs")
 	c.flags.StringVar(&c.description, "description", "", "A description of the token")
 	c.flags.Var((*flags.AppendSliceValue)(&c.policyIDs), "policy-id", "ID of a "+
-		"policy to use for this token. May be specified multiple times")
-	c.flags.Var((*flags.AppendSliceValue)(&c.addPolicyIDs), "add-policy-id", "ID of a "+
+		"policy to use for this token. Overwrites existing policies. May be specified multiple times")
+	c.flags.Var((*flags.AppendSliceValue)(&c.appendPolicyIDs), "append-policy-id", "ID of a "+
 		"policy to use for this token. The token retains existing policies. May be specified multiple times")
 	c.flags.Var((*flags.AppendSliceValue)(&c.policyNames), "policy-name", "Name of a "+
-		"policy to use for this token. May be specified multiple times")
-	c.flags.Var((*flags.AppendSliceValue)(&c.addPolicyNames), "add-policy-name", "Name of a "+
+		"policy to use for this token. Overwrites existing policies. May be specified multiple times")
+	c.flags.Var((*flags.AppendSliceValue)(&c.appendPolicyNames), "append-policy-name", "Name of a "+
 		"policy to add to this token. The token retains existing policies. May be specified multiple times")
 	c.flags.Var((*flags.AppendSliceValue)(&c.roleIDs), "role-id", "ID of a "+
-		"role to use for this token. May be specified multiple times")
+		"role to use for this token. Overwrites existing roles. May be specified multiple times")
 	c.flags.Var((*flags.AppendSliceValue)(&c.roleNames), "role-name", "Name of a "+
-		"role to use for this token. May be specified multiple times")
-	c.flags.Var((*flags.AppendSliceValue)(&c.addRoleIDs), "add-role-id", "ID of a "+
+		"role to use for this token. Overwrites existing rolees. May be specified multiple times")
+	c.flags.Var((*flags.AppendSliceValue)(&c.appendRoleIDs), "append-role-id", "ID of a "+
 		"role to add to this token. The token retains existing roles. May be specified multiple times")
-	c.flags.Var((*flags.AppendSliceValue)(&c.addRoleNames), "add-role-name", "Name of a "+
+	c.flags.Var((*flags.AppendSliceValue)(&c.appendRoleNames), "append-role-name", "Name of a "+
 		"role to add to this token. The token retains existing roles. May be specified multiple times")
 	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 "+
@@ -98,9 +98,9 @@ func (c *cmd) init() {
 	// Deprecations
 	c.flags.StringVar(&c.tokenID, "id", "", "DEPRECATED. Use -accessor-id instead.")
 	c.flags.BoolVar(&c.mergePolicies, "merge-policies", false, "DEPRECATED. "+
-		"Use -add-policy-id or -add-policy-name instead.")
+		"Use -append-policy-id or -append-policy-name instead.")
 	c.flags.BoolVar(&c.mergeRoles, "merge-roles", false, "DEPRECATED. "+
-		"Use -add-role-id or -add-role-name instead.")
+		"Use -append-role-id or -append-role-name instead.")
 }
 
 func (c *cmd) Run(args []string) int {
@@ -160,8 +160,8 @@ func (c *cmd) Run(args []string) int {
 	}
 
 	if c.mergePolicies {
-		c.UI.Warn("merge-policies is deprecated and will be removed in future Consul version. " +
-			"Use `add-policy-name` and `add-policy-id` instead.")
+		c.UI.Warn("merge-policies is deprecated and will be removed in a future Consul version. " +
+			"Use `append-policy-name` or `append-policy-id` instead.")
 
 		for _, policyName := range c.policyNames {
 			found := false
@@ -200,16 +200,18 @@ func (c *cmd) Run(args []string) int {
 		}
 	} else {
 
-		hasAddPolicyFields := len(c.addPolicyNames) > 0 || len(c.addPolicyIDs) > 0
+		hasAddPolicyFields := len(c.appendPolicyNames) > 0 || len(c.appendPolicyIDs) > 0
 		hasPolicyFields := len(c.policyIDs) > 0 || len(c.policyNames) > 0
 
 		if hasPolicyFields && hasAddPolicyFields {
-			c.UI.Error("Cannot specify both add-policy-id/add-policy-name and policy-id/policy-name")
+			c.UI.Error("Cannot combine the use of policy-id/policy-name flags with append- variants. " +
+				"To set or overwrite existing policies, use -policy-id or -policy-name. " +
+				"To append to existing policies, use -append-policy-id or -append-policy-name.")
 			return 1
 		}
 
-		policyIDs := c.addPolicyIDs
-		policyNames := c.addPolicyNames
+		policyIDs := c.appendPolicyIDs
+		policyNames := c.appendPolicyNames
 
 		if hasPolicyFields {
 			policyIDs = c.policyIDs
@@ -234,8 +236,8 @@ func (c *cmd) Run(args []string) int {
 	}
 
 	if c.mergeRoles {
-		c.UI.Warn("merge-roles is deprecated and will be removed in future Consul version. " +
-			"Use `add-role-name` and `add-role-id` instead.")
+		c.UI.Warn("merge-roles is deprecated and will be removed in a future Consul version. " +
+			"Use `append-role-name` or `append-role-id` instead.")
 
 		for _, roleName := range c.roleNames {
 			found := false
@@ -273,16 +275,18 @@ func (c *cmd) Run(args []string) int {
 			}
 		}
 	} else {
-		hasAddRoleFields := len(c.addRoleNames) > 0 || len(c.addRoleIDs) > 0
+		hasAddRoleFields := len(c.appendRoleNames) > 0 || len(c.appendRoleIDs) > 0
 		hasRoleFields := len(c.roleIDs) > 0 || len(c.roleNames) > 0
 
 		if hasRoleFields && hasAddRoleFields {
-			c.UI.Error("Cannot specify both add-role-id/add-role-name and role-id/role-name")
+			c.UI.Error("Cannot combine the use of role-id/role-name flags with append- variants. " +
+				"To set or overwrite existing roles, use -role-id or -role-name. " +
+				"To append to existing roles, use -append-role-id or -append-role-name.")
 			return 1
 		}
 
-		roleNames := c.addRoleNames
-		roleIDs := c.addRoleIDs
+		roleNames := c.appendRoleNames
+		roleIDs := c.appendRoleIDs
 
 		if hasRoleFields {
 			roleNames = c.roleNames

diff --git a/command/acl/token/update/token_update_test.go b/command/acl/token/update/token_update_test.go
@@ -122,55 +122,111 @@ func TestTokenUpdateCommand(t *testing.T) {
 		require.Len(t, token.Policies, 1)
 	})
 
-	// update with add-policy-name
-	t.Run("add-policy-name", func(t *testing.T) {
+	// update with policy by id
+	t.Run("policy-id", func(t *testing.T) {
 		token := run(t, []string{
 			"-http-addr=" + a.HTTPAddr(),
 			"-accessor-id=" + token.AccessorID,
 			"-token=root",
-			"-add-policy-name=" + policy.Name,
+			"-policy-id=" + policy.ID,
 			"-description=test token",
 		})
 
 		require.Len(t, token.Policies, 1)
 	})
 
-	// update with policy by id
-	t.Run("policy-id", func(t *testing.T) {
+	// update with no description shouldn't delete the current description
+	t.Run("merge-description", func(t *testing.T) {
 		token := run(t, []string{
 			"-http-addr=" + a.HTTPAddr(),
 			"-accessor-id=" + token.AccessorID,
 			"-token=root",
-			"-policy-id=" + policy.ID,
-			"-description=test token",
+			"-policy-name=" + policy.Name,
 		})
 
-		require.Len(t, token.Policies, 1)
+		require.Equal(t, "test token", token.Description)
 	})
+}
+
+func TestTokenUpdateCommandWithAppend(t *testing.T) {
+	if testing.Short() {
+		t.Skip("too slow for testing.Short")
+	}
+
+	t.Parallel()
+
+	a := agent.NewTestAgent(t, `
+	primary_datacenter = "dc1"
+	acl {
+		enabled = true
+		tokens {
+			initial_management = "root"
+		}
+	}`)
+
+	defer a.Shutdown()
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+
+	// Create a policy
+	client := a.Client()
 
-	// update with add-policy-id
-	t.Run("add-policy-id", func(t *testing.T) {
+	policy, _, err := client.ACL().PolicyCreate(
+		&api.ACLPolicy{Name: "test-policy"},
+		&api.WriteOptions{Token: "root"},
+	)
+	require.NoError(t, err)
+
+	// create a token
+	token, _, err := client.ACL().TokenCreate(
+		&api.ACLToken{Description: "test", Policies: []*api.ACLTokenPolicyLink{{Name: policy.Name}}},
+		&api.WriteOptions{Token: "root"},
+	)
+	require.NoError(t, err)
+
+	//secondary policy
+	secondPolicy, _, policyErr := client.ACL().PolicyCreate(
+		&api.ACLPolicy{Name: "secondary-policy"},
+		&api.WriteOptions{Token: "root"},
+	)
+	require.NoError(t, policyErr)
+
+	run := func(t *testing.T, args []string) *api.ACLToken {
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+
+		code := cmd.Run(append(args, "-format=json"))
+		require.Equal(t, 0, code)
+		require.Empty(t, ui.ErrorWriter.String())
+
+		var token api.ACLToken
+		require.NoError(t, json.Unmarshal(ui.OutputWriter.Bytes(), &token))
+		return &token
+	}
+
+	// update with append-policy-name
+	t.Run("append-policy-name", func(t *testing.T) {
 		token := run(t, []string{
 			"-http-addr=" + a.HTTPAddr(),
 			"-accessor-id=" + token.AccessorID,
 			"-token=root",
-			"-add-policy-id=" + policy.ID,
+			"-append-policy-name=" + secondPolicy.Name,
 			"-description=test token",
 		})
 
-		require.Len(t, token.Policies, 1)
+		require.Len(t, token.Policies, 2)
 	})
 
-	// update with no description shouldn't delete the current description
-	t.Run("merge-description", func(t *testing.T) {
+	// update with append-policy-id
+	t.Run("append-policy-id", func(t *testing.T) {
 		token := run(t, []string{
 			"-http-addr=" + a.HTTPAddr(),
 			"-accessor-id=" + token.AccessorID,
 			"-token=root",
-			"-policy-name=" + policy.Name,
+			"-append-policy-id=" + secondPolicy.ID,
+			"-description=test token",
 		})
 
-		require.Equal(t, "test token", token.Description)
+		require.Len(t, token.Policies, 2)
 	})
 }
 

diff --git a/website/content/commands/acl/token/update.mdx b/website/content/commands/acl/token/update.mdx
@@ -38,12 +38,12 @@ Usage: `consul acl token update [options]`
 
 - `-merge-policies` - Deprecated. Merge the new policies with the existing policies.
 
-~> This is deprecated and will be removed in future Consul version. Use `add-policy-id` or `add-policy-name`
+~> This is deprecated and will be removed in a future Consul version. Use `append-policy-id` or `append-policy-name`
 instead.
 
 - `-merge-roles` - Deprecated. Merge the new roles with the existing roles.
 
-~> This is deprecated and will be removed in future Consul version. Use `add-role-id` or `add-role-name`
+~> This is deprecated and will be removed in a future Consul version. Use `append-role-id` or `append-role-name`
 instead.
 
 - `-merge-service-identities` - Merge the new service identities with the existing service identities.
@@ -59,21 +59,21 @@ instead.
 
 - `-policy-name=<value>` - Name of a policy to use for this token. Overwrites existing policies. May be specified multiple times.
 
-~> `-policy-id` and `-policy-name` will completely overwrite existing token policies. Use `-add-policy-id` or `-add-policy-name` to retain existing policies.
+~> `-policy-id` and `-policy-name` will completely overwrite existing token policies. Use `-append-policy-id` or `-append-policy-name` to retain existing policies.
 
-- `-add-policy-id=<value>` - ID of policy to be added for this token. The token retains existing policies. May be specified multiple times.
+- `-append-policy-id=<value>` - ID of policy to be added for this token. The token retains existing policies. May be specified multiple times.
 
-- `-add-policy-name=<value>` - Name of a policy to be added for this token. The token retains existing policies. May be specified multiple times.
+- `-append-policy-name=<value>` - Name of a policy to be added for this token. The token retains existing policies. May be specified multiple times.
 
 - `-role-id=<value>` - ID of a role to use for this token. Overwrites existing roles. May be specified multiple times.
 
 - `-role-name=<value>` - Name of a role to use for this token. Overwrites existing roles. May be specified multiple times.
 
-~> `-role-id` and `-role-name` will completely overwrite existing policies. Use `-add-role-id` or `-add-role-name` to retain the existing roles.
+~> `-role-id` and `-role-name` will completely overwrite existing roles. Use `-append-role-id` or `-append-role-name` to retain the existing roles.
 
-- `-add-role-id=<value>` - ID of a role to add to this token. The token retains existing roles. May be specified multiple times.
+- `-append-role-id=<value>` - ID of a role to add to this token. The token retains existing roles. May be specified multiple times.
 
-- `-add-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
   token. May be specified multiple times. Format is the `SERVICENAME` or