feat: split the org role edit command into two commands

coder · brettkolodny · Apr 4, 2025 · Mar 26, 2025 · Mar 26, 2025 · Mar 27, 2025
commit 1ba61576d84ab5ca0853fdf7519be6ea0175596a
diff --git a/cli/organizationroles.go b/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 creat
10000
e --stidin < role.json",
 			},
 		),
 		Options: []serpent.Option{
@@ -152,7 +153,11 @@ 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
@@ -175,29 +180,149 @@ 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 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)
+				}
+			} 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 {
+				// JSON Upload mode
-				// JSON Upload mode
-				// JSON Upload mode
+				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("listing existing roles: %w", err)
+					return xerrors.Errorf("parsing stdin json: %w", err)
 				}
-				for _, existingRole := range existingRoles {
-					if strings.EqualFold(customRole.Name, existingRole.Name) {
-						// Editing an existing role
-						createNewRole = false
-						break
+
+				if customRole.Name == "" {
+					arr := make([]json.RawMessage, 0)
+					err = json.Unmarshal(bytes, &arr)
+					if err == nil && len(arr) > 0 {
+						return xerrors.Errorf("the input appears to be an array, only 1 role can be sent at a time")
-						return xerrors.Errorf("the input appears to be an array, only 1 role can be sent at a time")
+						return xerrors.Errorf("only 1 role can be sent at a time")
-						return xerrors.Errorf("the input appears to be an array, only 1 role can be sent at a time")
+						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 exists. If you'd like to create this role use the create command instead", customRole.Name)
-					return xerrors.Errorf("The role %s does not exist exists. If you'd like to create this role use the create command instead", customRole.Name)
+					return xerrors.Errorf("The role %s does not exist. If you'd like to create this role use the create command instead", customRole.Name)
-					return xerrors.Errorf("The role %s does not exist exists. If you'd like to create this role use the create command instead", customRole.Name)
+					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 exists. If you'd like to create this role use the create command instead", inv.Args[0])
-					return xerrors.Errorf("The role %s does not exist exists. If you'd like to create this role use the create command instead", inv.Args[0])
+					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])
-					return xerrors.Errorf("The role %s does not exist exists. If you'd like to create this role use the create command instead", inv.Args[0])
+					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 +341,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 +361,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(origin
F0B1
alRole.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 +400,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 +417,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 +426,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 +502,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"`