diff --git a/internal/cmd/secrets-manager/user/delete/delete.go b/internal/cmd/secrets-manager/user/delete/delete.go index 702dbf2df..618aaed2b 100644 --- a/internal/cmd/secrets-manager/user/delete/delete.go +++ b/internal/cmd/secrets-manager/user/delete/delete.go @@ -63,13 +63,13 @@ func NewCmd() *cobra.Command { instanceLabel = model.InstanceId } - userLabel, userDescription, err := secretsManagerUtils.GetUserDetails(ctx, apiClient, model.ProjectId, model.InstanceId, model.UserId) + userLabel, err := secretsManagerUtils.GetUserLabel(ctx, apiClient, model.ProjectId, model.InstanceId, model.UserId) if err != nil { - userLabel = model.UserId + userLabel = fmt.Sprintf("%q", model.UserId) } if !model.AssumeYes { - prompt := fmt.Sprintf("Are you sure you want to delete user %q (%q) of instance %q? (This cannot be undone)", userLabel, userDescription, instanceLabel) + prompt := fmt.Sprintf("Are you sure you want to delete user %s of instance %q? (This cannot be undone)", userLabel, instanceLabel) err = confirm.PromptForConfirmation(cmd, prompt) if err != nil { return err @@ -83,7 +83,7 @@ func NewCmd() *cobra.Command { return fmt.Errorf("delete Secrets Manager user: %w", err) } - cmd.Printf("Deleted user %q of instance %q\n", userLabel, instanceLabel) + cmd.Printf("Deleted user %s of instance %q\n", userLabel, instanceLabel) return nil }, } diff --git a/internal/cmd/secrets-manager/user/update/update.go b/internal/cmd/secrets-manager/user/update/update.go index 3027ad079..9fe27207d 100644 --- a/internal/cmd/secrets-manager/user/update/update.go +++ b/internal/cmd/secrets-manager/user/update/update.go @@ -67,13 +67,9 @@ func NewCmd() *cobra.Command { instanceLabel = model.InstanceId } - var userLabel string - - userName, userDescription, err := secretsManagerUtils.GetUserDetails(ctx, apiClient, model.ProjectId, model.InstanceId, model.UserId) + userLabel, err := secretsManagerUtils.GetUserLabel(ctx, apiClient, model.ProjectId, model.InstanceId, model.UserId) if err != nil { userLabel = fmt.Sprintf("%q", model.UserId) - } else { - userLabel = fmt.Sprintf("%q (%s)", userName, userDescription) } if !model.AssumeYes { diff --git a/internal/pkg/services/secrets-manager/utils/utils.go b/internal/pkg/services/secrets-manager/utils/utils.go index 7927de71c..0b53f64f5 100644 --- a/internal/pkg/services/secrets-manager/utils/utils.go +++ b/internal/pkg/services/secrets-manager/utils/utils.go @@ -20,10 +20,22 @@ func GetInstanceName(ctx context.Context, apiClient SecretsManagerClient, projec return *resp.Name, nil } -func GetUserDetails(ctx context.Context, apiClient SecretsManagerClient, projectId, instanceId, userId string) (username, description string, err error) { +func GetUserLabel(ctx context.Context, apiClient SecretsManagerClient, projectId, instanceId, userId string) (string, error) { resp, err := apiClient.GetUserExecute(ctx, projectId, instanceId, userId) if err != nil { - return "", "", fmt.Errorf("get Secrets Manager user: %w", err) + return "", fmt.Errorf("get Secrets Manager user: %w", err) } - return *resp.Username, *resp.Description, nil + + if resp.Username == nil || *resp.Username == "" { + // Should never happen, username is auto-generated + return "", fmt.Errorf("username is empty") + } + + var userLabel string + if resp.Description == nil || *resp.Description == "" { + userLabel = fmt.Sprintf("%q", *resp.Username) + } else { + userLabel = fmt.Sprintf("%q (%s)", *resp.Username, *resp.Description) + } + return userLabel, nil } diff --git a/internal/pkg/services/secrets-manager/utils/utils_test.go b/internal/pkg/services/secrets-manager/utils/utils_test.go index add1773aa..d79ca49f7 100644 --- a/internal/pkg/services/secrets-manager/utils/utils_test.go +++ b/internal/pkg/services/secrets-manager/utils/utils_test.go @@ -93,12 +93,11 @@ func TestGetInstanceName(t *testing.T) { func TestGetUserDetails(t *testing.T) { tests := []struct { - description string - getUserFails bool - GetUserResp *secretsmanager.User - isValid bool - expectedUserName string - expectedDescription string + description string + getUserFails bool + GetUserResp *secretsmanager.User + isValid bool + expectedOutput string }{ { description: "base", @@ -106,9 +105,39 @@ func TestGetUserDetails(t *testing.T) { Username: utils.Ptr(testUserName), Description: utils.Ptr(testDescription), }, - isValid: true, - expectedUserName: testUserName, - expectedDescription: testDescription, + isValid: true, + expectedOutput: fmt.Sprintf("%q (%s)", testUserName, testDescription), + }, + { + description: "user has no description", + GetUserResp: &secretsmanager.User{ + Username: utils.Ptr(testUserName), + }, + isValid: true, + expectedOutput: fmt.Sprintf("%q", testUserName), + }, + { + description: "user has empty description", + GetUserResp: &secretsmanager.User{ + Username: utils.Ptr(testUserName), + Description: utils.Ptr(""), + }, + isValid: true, + expectedOutput: fmt.Sprintf("%q", testUserName), + }, + { + description: "user has empty username", + GetUserResp: &secretsmanager.User{ + Username: utils.Ptr(""), + }, + isValid: false, + }, + { + description: "user has no username", + GetUserResp: &secretsmanager.User{ + Username: nil, + }, + isValid: false, }, { description: "get user fails", @@ -124,7 +153,7 @@ func TestGetUserDetails(t *testing.T) { getUserResp: tt.GetUserResp, } - username, description, err := GetUserDetails(context.Background(), client, testProjectId, testInstanceId, testUserId) + userLabel, err := GetUserLabel(context.Background(), client, testProjectId, testInstanceId, testUserId) if tt.isValid && err != nil { t.Errorf("failed on valid input") @@ -135,11 +164,8 @@ func TestGetUserDetails(t *testing.T) { if !tt.isValid { return } - if username != tt.expectedUserName { - t.Errorf("expected username to be %s, got %s", tt.expectedUserName, username) - } - if description != tt.expectedDescription { - t.Errorf("expected description to be %s, got %s", tt.expectedDescription, description) + if userLabel != tt.expectedOutput { + t.Errorf("expected user label to be %s, got %s", tt.expectedOutput, userLabel) } }) }