feat: split cli roles edit command into create and update commands (#17121)

Closes #14239

cli/organizationroles.go

@@ -26,7 +26,8 @@ func (r *RootCmd) organizationRoles(orgContext *OrganizationContext) *serpent.Co
 		},
 		Children: []*serpent.Command{
 			r.showOrganizationRoles(orgContext),
-			r.editOrganizationRole(orgContext),
+			r.updateOrganizationRole(orgContext),
+			r.createOrganizationRole(orgContext),
 		},
 	}
 	return cmd
@@ -99,7 +100,7 @@ func (r *RootCmd) showOrganizationRoles(orgContext *OrganizationContext) *serpen
 	return cmd
 }
 
-func (r *RootCmd) editOrganizationRole(orgContext *OrganizationContext) *serpent.Command {
+func (r *RootCmd) createOrganizationRole(orgContext *OrganizationContext) *serpent.Command {
 	formatter := cliui.NewOutputFormatter(
 		cliui.ChangeFormatterData(
 			cliui.TableFormat([]roleTableRow{}, []string{"name", "display name", "site permissions", "organization permissions", "user permissions"}),
@@ -118,12 +119,12 @@ func (r *RootCmd) editOrganizationRole(orgContext *OrganizationContext) *serpent
 
 	client := new(codersdk.Client)
 	cmd := &serpent.Command{
-		Use:   "edit <role_name>",
-		Short: "Edit an organization custom role",
+		Use:   "create <role_name>",
+		Short: "Create a new organization custom role",
 		Long: FormatExamples(
 			Example{
 				Description: "Run with an input.json file",
-				Command:     "coder roles edit --stdin < role.json",
+				Command:     "coder organization -O <organization_name> roles create --stidin < role.json",
 			},
 		),
 		Options: []serpent.Option{
@@ -152,10 +153,13 @@ func (r *RootCmd) editOrganizationRole(orgContext *OrganizationContext) *serpent
 				return err
 			}
 
-			createNewRole := true
+			existingRoles, err := client.ListOrganizationRoles(ctx, org.ID)
+			if err != nil {
+				return xerrors.Errorf("listing existing roles: %w", err)
+			}
+
 			var customRole codersdk.Role
 			if jsonInput {
-				// JSON Upload mode
 				bytes, err := io.ReadAll(inv.Stdin)
 				if err != nil {
 					return xerrors.Errorf("reading stdin: %w", err)
@@ -175,29 +179,148 @@ func (r *RootCmd) editOrganizationRole(orgContext *OrganizationContext) *serpent
 					return xerrors.Errorf("json input does not appear to be a valid role")
 				}
 
-				existingRoles, err := client.ListOrganizationRoles(ctx, org.ID)
-				if err != nil {
-					return xerrors.Errorf("listing existing roles: %w", err)
+				if role := existingRole(customRole.Name, existingRoles); role != nil {
+					return xerrors.Errorf("The role %s already exists. If you'd like to edit this role use the update command instead", customRole.Name)
 				}
-				for _, existingRole := range existingRoles {
-					if strings.EqualFold(customRole.Name, existingRole.Name) {
-						// Editing an existing role
-						createNewRole = false
-						break
+			} else {
+				if len(inv.Args) == 0 {
+					return xerrors.Errorf("missing role name argument, usage: \"coder organizations roles create <role_name>\"")
+				}
+
+				if role := existingRole(inv.Args[0], existingRoles); role != nil {
+					return xerrors.Errorf("The role %s already exists. If you'd like to edit this role use the update command instead", inv.Args[0])
+				}
+
+				interactiveRole, err := interactiveOrgRoleEdit(inv, org.ID, nil)
+				if err != nil {
+					return xerrors.Errorf("editing role: %w", err)
+				}
+
+				customRole = *interactiveRole
+			}
+
+			var updated codersdk.Role
+			if dryRun {
+				// Do not actually post
+				updated = customRole
+			} else {
+				updated, err = client.CreateOrganizationRole(ctx, customRole)
+				if err != nil {
+					return xerrors.Errorf("patch role: %w", err)
+				}
+			}
+
+			output, err := formatter.Format(ctx, updated)
+			if err != nil {
+				return xerrors.Errorf("formatting: %w", err)
+			}
+
+			_, err = fmt.Fprintln(inv.Stdout, output)
+			return err
+		},
+	}
+
+	return cmd
+}
+
+func (r *RootCmd) updateOrganizationRole(orgContext *OrganizationContext) *serpent.Command {
+	formatter := cliui.NewOutputFormatter(
+		cliui.ChangeFormatterData(
+			cliui.TableFormat([]roleTableRow{}, []string{"name", "display name", "site permissions", "organization permissions", "user permissions"}),
+			func(data any) (any, error) {
+				typed, _ := data.(codersdk.Role)
+				return []roleTableRow{roleToTableView(typed)}, nil
+			},
+		),
+		cliui.JSONFormat(),
+	)
+
+	var (
+		dryRun    bool
+		jsonInput bool
+	)
+
+	client := new(codersdk.Client)
+	cmd := &serpent.Command{
+		Use:   "update <role_name>",
+		Short: "Update an organization custom role",
+		Long: FormatExamples(
+			Example{
+				Description: "Run with an input.json file",
+				Command:     "coder roles update --stdin < role.json",
+			},
+		),
+		Options: []serpent.Option{
+			cliui.SkipPromptOption(),
+			{
+				Name:        "dry-run",
+				Description: "Does all the work, but does not submit the final updated role.",
+				Flag:        "dry-run",
+				Value:       serpent.BoolOf(&dryRun),
+			},
+			{
+				Name:        "stdin",
+				Description: "Reads stdin for the json role definition to upload.",
+				Flag:        "stdin",
+				Value:       serpent.BoolOf(&jsonInput),
+			},
+		},
+		Middleware: serpent.Chain(
+			serpent.RequireRangeArgs(0, 1),
+			r.InitClient(client),
+		),
+		Handler: func(inv *serpent.Invocation) error {
+			ctx := inv.Context()
+			org, err := orgContext.Selected(inv, client)
+			if err != nil {
+				return err
+			}
+
+			existingRoles, err := client.ListOrganizationRoles(ctx, org.ID)
+			if err != nil {
+				return xerrors.Errorf("listing existing roles: %w", err)
+			}
+
+			var customRole codersdk.Role
+			if jsonInput {
+				bytes, err := io.ReadAll(inv.Stdin)
+				if err != nil {
+					return xerrors.Errorf("reading stdin: %w", err)
+				}
+
+				err = json.Unmarshal(bytes, &customRole)
+				if err != nil {
+					return xerrors.Errorf("parsing stdin json: %w", err)
+				}
+
+				if customRole.Name == "" {
+					arr := make([]json.RawMessage, 0)
+					err = json.Unmarshal(bytes, &arr)
+					if err == nil && len(arr) > 0 {
+						return xerrors.Errorf("only 1 role can be sent at a time")
 					}
+					return xerrors.Errorf("json input does not appear to be a valid role")
+				}
+
+				if role := existingRole(customRole.Name, existingRoles); role == nil {
+					return xerrors.Errorf("The role %s does not exist. If you'd like to create this role use the create command instead", customRole.Name)
 				}
 			} else {
 				if len(inv.Args) == 0 {
 					return xerrors.Errorf("missing role name argument, usage: \"coder organizations roles edit <role_name>\"")
 				}
 
-				interactiveRole, newRole, err := interactiveOrgRoleEdit(inv, org.ID, client)
+				role := existingRole(inv.Args[0], existingRoles)
+				if role == nil {
+					return xerrors.Errorf("The role %s does not exist. If you'd like to create this role use the create command instead", inv.Args[0])
+				}
+
+				interactiveRole, err := interactiveOrgRoleEdit(inv, org.ID, &role.Role)
 				if err != nil {
 					return xerrors.Errorf("editing role: %w", err)
 				}
 
 				customRole = *interactiveRole
-				createNewRole = newRole
 
 				preview := fmt.Sprintf("permissions: %d site, %d org, %d user",
 					len(customRole.SitePermissions), len(customRole.OrganizationPermissions), len(customRole.UserPermissions))
@@ -216,12 +339,7 @@ func (r *RootCmd) editOrganizationRole(orgContext *OrganizationContext) *serpent
 				// Do not actually post
 				updated = customRole
 			} else {
-				switch createNewRole {
-				case true:
-					updated, err = client.CreateOrganizationRole(ctx, customRole)
-				default:
-					updated, err = client.UpdateOrganizationRole(ctx, customRole)
-				}
+				updated, err = client.UpdateOrganizationRole(ctx, customRole)
 				if err != nil {
 					return xerrors.Errorf("patch role: %w", err)
 				}
@@ -241,50 +359,27 @@ func (r *RootCmd) editOrganizationRole(orgContext *OrganizationContext) *serpent
 	return cmd
 }
 
-func interactiveOrgRoleEdit(inv *serpent.Invocation, orgID uuid.UUID, client *codersdk.Client) (*codersdk.Role, bool, error) {
-	newRole := false
-	ctx := inv.Context()
-	roles, err := client.ListOrganizationRoles(ctx, orgID)
-	if err != nil {
-		return nil, newRole, xerrors.Errorf("listing roles: %w", err)
-	}
-
-	// Make sure the role actually exists first
-	var originalRole codersdk.AssignableRoles
-	for _, r := range roles {
-		if strings.EqualFold(inv.Args[0], r.Name) {
-			originalRole = r
-			break
-		}
-	}
-
-	if originalRole.Name == "" {
-		_, err = cliui.Prompt(inv, cliui.PromptOptions{
-			Text:      "No organization role exists with that name, do you want to create one?",
-			Default:   "yes",
-			IsConfirm: true,
-		})
-		if err != nil {
-			return nil, newRole, xerrors.Errorf("abort: %w", err)
-		}
-
-		originalRole.Role = codersdk.Role{
+func interactiveOrgRoleEdit(inv *serpent.Invocation, orgID uuid.UUID, updateRole *codersdk.Role) (*codersdk.Role, error) {
+	var originalRole codersdk.Role
+	if updateRole == nil {
+		originalRole = codersdk.Role{
 			Name:           inv.Args[0],
 			OrganizationID: orgID.String(),
 		}
-		newRole = true
+	} else {
+		originalRole = *updateRole
 	}
 
 	// Some checks since interactive mode is limited in what it currently sees
 	if len(originalRole.SitePermissions) > 0 {
-		return nil, newRole, xerrors.Errorf("unable to edit role in interactive mode, it contains site wide permissions")
+		return nil, xerrors.Errorf("unable to edit role in interactive mode, it contains site wide permissions")
 	}
 
 	if len(originalRole.UserPermissions) > 0 {
-		return nil, newRole, xerrors.Errorf("unable to edit role in interactive mode, it contains user permissions")
+		return nil, xerrors.Errorf("unable to edit role in interactive mode, it contains user permissions")
 	}
 
-	role := &originalRole.Role
+	role := &originalRole
 	allowedResources := []codersdk.RBACResource{
 		codersdk.ResourceTemplate,
 		codersdk.ResourceWorkspace,
@@ -303,13 +398,13 @@ customRoleLoop:
 			Options: append(permissionPreviews(role, allowedResources), done, abort),
 		})
 		if err != nil {
-			return role, newRole, xerrors.Errorf("selecting resource: %w", err)
+			return role, xerrors.Errorf("selecting resource: %w", err)
 		}
 		switch selected {
 		case done:
 			break customRoleLoop
 		case abort:
-			return role, newRole, xerrors.Errorf("edit role %q aborted", role.Name)
+			return role, xerrors.Errorf("edit role %q aborted", role.Name)
 		default:
 			strs := strings.Split(selected, "::")
 			resource := strings.TrimSpace(strs[0])
@@ -320,7 +415,7 @@ customRoleLoop:
 				Defaults: defaultActions(role, resource),
 			})
 			if err != nil {
-				return role, newRole, xerrors.Errorf("selecting actions for resource %q: %w", resource, err)
+				return role, xerrors.Errorf("selecting actions for resource %q: %w", resource, err)
 			}
 			applyOrgResourceActions(role, resource, actions)
 			// back to resources!
@@ -329,7 +424,7 @@ customRoleLoop:
 	// This println is required because the prompt ends us on the same line as some text.
 	_, _ = fmt.Println()
 
-	return role, newRole, nil
+	return role, nil
 }
 
 func applyOrgResourceActions(role *codersdk.Role, resource string, actions []string) {
@@ -405,6 +500,16 @@ func roleToTableView(role codersdk.Role) roleTableRow {
 	}
 }
 
+func existingRole(newRoleName string, existingRoles []codersdk.AssignableRoles) *codersdk.AssignableRoles {
+	for _, existingRole := range existingRoles {
+		if strings.EqualFold(newRoleName, existingRole.Name) {
+			return &existingRole
+		}
+	}
+
+	return nil
+}
+
 type roleTableRow struct {
 	Name            string `table:"name,default_sort"`
 	DisplayName     string `table:"display name"`

cli/testdata/coder_organizations_roles_--help.golden

@@ -8,8 +8,9 @@ USAGE:
   Aliases: role
 
 SUBCOMMANDS:
-    edit    Edit an organization custom role
-    show    Show role(s)
+    create    Create a new organization custom role
+    show      Show role(s)
+    update    Update an organization custom role
 
 ———
 Run `coder --help` for a list of global options.

cli/testdata/coder_organizations_roles_create_--help.golden

@@ -0,0 +1,24 @@
+coder v0.0.0-devel
+
+USAGE:
+  coder organizations roles create [flags] <role_name>
+
+  Create a new organization custom role
+
+    - Run with an input.json file:
+  
+       $ coder organization -O <organization_name> roles create --stidin <
+  role.json
+
+OPTIONS:
+      --dry-run bool
+          Does all the work, but does not submit the final updated role.
+
+      --stdin bool
+          Reads stdin for the json role definition to upload.
+
+  -y, --yes bool
+          Bypass prompts.
+
+———
+Run `coder --help` for a list of global options.

cli/testdata/coder_organizations_roles_update_--help.golden

@@ -1,13 +1,13 @@
 coder v0.0.0-devel
 
 USAGE:
-  coder organizations roles edit [flags] <role_name>
+  coder organizations roles update [flags] <role_name>
 
-  Edit an organization custom role
+  Update an organization custom role
 
     - Run with an input.json file:
   
-       $ coder roles edit --stdin < role.json
+       $ coder roles update --stdin < role.json
 
 OPTIONS:
   -c, --column [name|display name|organization id|site permissions|organization permissions|user permissions] (default: name,display name,site permissions,organization permissions,user permissions)

docs/manifest.json

@@ -1200,15 +1200,20 @@
 							"path": "reference/cli/organizations_roles.md"
 						},
 						{
-							"title": "organizations roles edit",
-							"description": "Edit an organization custom role",
-							"path": "reference/cli/organizations_roles_edit.md"
+							"title": "organizations roles create",
+							"description": "Create a new organization custom role",
+							"path": "reference/cli/organizations_roles_create.md"
 						},
 						{
 							"title": "organizations roles show",
 							"description": "Show role(s)",
 							"path": "reference/cli/organizations_roles_show.md"
 						},
+						{
+							"title": "organizations roles update",
+							"description": "Update an organization custom role",
+							"path": "reference/cli/organizations_roles_update.md"
+						},
 						{
 							"title": "organizations settings",
 							"description": "Manage organization settings.",

docs/reference/cli/organizations_roles.md

@@ -15,7 +15,8 @@ coder organizations roles
 
 ## Subcommands
 
-| Name                                               | Purpose                          |
-|----------------------------------------------------|----------------------------------|
-| [<code>show</code>](./organizations_roles_show.md) | Show role(s)                     |
-| [<code>edit</code>](./organizations_roles_edit.md) | Edit an organization custom role |
+| Name                                                   | Purpose                               |
+|--------------------------------------------------------|---------------------------------------|
+| [<code>show</code>](./organizations_roles_show.md)     | Show role(s)                          |
+| [<code>update</code>](./organizations_roles_update.md) | Update an organization custom role    |
+| [<code>create</code>](./organizations_roles_create.md) | Create a new organization custom role |

docs/reference/cli/organizations_roles_create.md

@@ -0,0 +1,44 @@
+<!-- DO NOT EDIT | GENERATED CONTENT -->
+# organizations roles create
+
+Create a new organization custom role
+
+## Usage
+
+```console
+coder organizations roles create [flags] <role_name>
+```
+
+## Description
+
+```console
+  - Run with an input.json file:
+
+     $ coder organization -O <organization_name> roles create --stidin < role.json
+```
+
+## Options
+
+### -y, --yes
+
+|      |                   |
+|------|-------------------|
+| Type | <code>bool</code> |
+
+Bypass prompts.
+
+### --dry-run
+
+|      |                   |
+|------|-------------------|
+| Type | <code>bool</code> |
+
+Does all the work, but does not submit the final updated role.
+
+### --stdin
+
+|      |                   |
+|------|-------------------|
+| Type | <code>bool</code> |
+
+Reads stdin for the json role definition to upload.

docs/reference/cli/organizations_roles_update.md

@@ -1,12 +1,12 @@
 <!-- DO NOT EDIT | GENERATED CONTENT -->
-# organizations roles edit
+# organizations roles update
 
-Edit an organization custom role
+Update an organization custom role
 
 ## Usage
 
 ```console
-coder organizations roles edit [flags] <role_name>
+coder organizations roles update [flags] <role_name>
 ```
 
 ## Description
@@ -14,7 +14,7 @@ coder organizations roles edit [flags] <role_name>
 ```console
   - Run with an input.json file:
 
-     $ coder roles edit --stdin < role.json
+     $ coder roles update --stdin < role.json
 ```
 
 ## Options

enterprise/cli/organization_test.go

@@ -5,10 +5,13 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
 
 	"github.com/coder/coder/v2/cli/clitest"
 	"github.com/coder/coder/v2/coderd/coderdtest"
+	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/database/dbgen"
 	"github.com/coder/coder/v2/coderd/rbac"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/enterprise/coderd/coderdenttest"
@@ -17,7 +20,7 @@ import (
 	"github.com/coder/coder/v2/testutil"
 )
 
-func TestEditOrganizationRoles(t *testing.T) {
+func TestCreateOrganizationRoles(t *testing.T) {
 	t.Parallel()
 
 	// Unit test uses --stdin and json as the role input. The interactive cli would
@@ -34,7 +37,7 @@ func TestEditOrganizationRoles(t *testing.T) {
 		})
 
 		ctx := testutil.Context(t, testutil.WaitMedium)
-		inv, root := clitest.New(t, "organization", "roles", "edit", "--stdin")
+		inv, root := clitest.New(t, "organization", "roles", "create", "--stdin")
 		inv.Stdin = bytes.NewBufferString(fmt.Sprintf(`{
     "name": "new-role",
     "organization_id": "%s",
@@ -72,7 +75,7 @@ func TestEditOrganizationRoles(t *testing.T) {
 		})
 
 		ctx := testutil.Context(t, testutil.WaitMedium)
-		inv, root := clitest.New(t, "organization", "roles", "edit", "--stdin")
+		inv, root := clitest.New(t, "organization", "roles", "create", "--stdin")
 		inv.Stdin = bytes.NewBufferString(fmt.Sprintf(`{
     "name": "new-role",
     "organization_id": "%s",
@@ -185,3 +188,104 @@ func TestShowOrganizations(t *testing.T) {
 		pty.ExpectMatch(orgs["bar"].ID.String())
 	})
 }
+
+func TestUpdateOrganizationRoles(t *testing.T) {
+	t.Parallel()
+
+	t.Run("JSON", func(t *testing.T) {
+		t.Parallel()
+
+		ownerClient, db, owner := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{
+			LicenseOptions: &coderdenttest.LicenseOptions{
+				Features: license.Features{
+					codersdk.FeatureCustomRoles: 1,
+				},
+			},
+		})
+		client, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleOwner())
+
+		// Create a role in the DB with no permissions
+		const expectedRole = "test-role"
+		dbgen.CustomRole(t, db, database.CustomRole{
+			Name:            expectedRole,
+			DisplayName:     "Expected",
+			SitePermissions: nil,
+			OrgPermissions:  nil,
+			UserPermissions: nil,
+			OrganizationID: uuid.NullUUID{
+				UUID:  owner.OrganizationID,
+				Valid: true,
+			},
+		})
+
+		// Update the new role via JSON
+		ctx := testutil.Context(t, testutil.WaitMedium)
+		inv, root := clitest.New(t, "organization", "roles", "update", "--stdin")
+		inv.Stdin = bytes.NewBufferString(fmt.Sprintf(`{
+    "name": "test-role",
+    "organization_id": "%s",
+    "display_name": "",
+    "site_permissions": [],
+    "organization_permissions": [
+		{
+		  "resource_type": "workspace",
+		  "action": "read"
+		}
+    ],
+    "user_permissions": [],
+    "assignable": false,
+    "built_in": false
+  }`, owner.OrganizationID.String()))
+
+		//nolint:gocritic // only owners can edit roles
+		clitest.SetupConfig(t, client, root)
+
+		buf := new(bytes.Buffer)
+		inv.Stdout = buf
+		err := inv.WithContext(ctx).Run()
+		require.NoError(t, err)
+		require.Contains(t, buf.String(), "test-role")
+		require.Contains(t, buf.String(), "1 permissions")
+	})
+
+	t.Run("InvalidRole", func(t *testing.T) {
+		t.Parallel()
+
+		ownerClient, _, owner := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{
+			LicenseOptions: &coderdenttest.LicenseOptions{
+				Features: license.Features{
+					codersdk.FeatureCustomRoles: 1,
+				},
+			},
+		})
+		client, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleOwner())
+
+		// Update the new role via JSON
+		ctx := testutil.Context(t, testutil.WaitMedium)
+		inv, root := clitest.New(t, "organization", "roles", "update", "--stdin")
+		inv.Stdin = bytes.NewBufferString(fmt.Sprintf(`{
+    "name": "test-role",
+    "organization_id": "%s",
+    "display_name": "",
+    "site_permissions": [],
+    "organization_permissions": [
+		{
+		  "resource_type": "workspace",
+		  "action": "read"
+		}
+    ],
+    "user_permissions": [],
+    "assignable": false,
+    "built_in": false
+  }`, owner.OrganizationID.String()))
+
+		//nolint:gocritic // only owners can edit roles
+		clitest.SetupConfig(t, client, root)
+
+		buf := new(bytes.Buffer)
+		inv.Stdout = buf
+		err := inv.WithContext(ctx).Run()
+		require.Error(t, err)
+		require.ErrorContains(t, err, "The role test-role does not exist.")
+	})
+}