feature: apply review findings

Author: bahkauv70

internal/cmd/beta/security-group/create/create.go

@@ -18,9 +18,16 @@ import (
 	"github.com/stackitcloud/stackit-sdk-go/services/iaas"
 )
 
+const (
+	nameFlag        = "name"
+	descriptionFlag = "description"
+	statefulFlag    = "stateful"
+	labelsFlag      = "labels"
+)
+
 type inputModel struct {
 	*globalflags.GlobalFlagModel
-	Labels      map[string]any
+	Labels      *map[string]any
 	Description string
 	Name        string
 	Stateful    bool
@@ -29,114 +36,95 @@ type inputModel struct {
 func NewCmd(p *print.Printer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "create",
-		Short: "create security groups",
-		Long:  "create security groups",
+		Short: "Create security groups",
+		Long:  "Create security groups.",
 		Args:  args.NoArgs,
 		Example: examples.Build(
 			examples.NewExample(`create a named group`, `$ stackit beta security-group create --name my-new-group`),
 			examples.NewExample(`create a named group with labels`, `$ stackit beta security-group create --name my-new-group --labels label1=value1,label2=value2`),
 		),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return executeCreate(cmd, p, args)
-		},
-	}
+			ctx := context.Background()
+			model, err := parseInput(p, cmd)
+			if err != nil {
+				return err
+			}
 
-	configureFlags(cmd)
-	return cmd
-}
+			// Configure API client
+			apiClient, err := client.ConfigureClient(p)
+			if err != nil {
+				return err
+			}
 
-func configureFlags(cmd *cobra.Command) {
-	cmd.Flags().String("name", "", "the name of the security group. Must be <= 63 chars")
-	cmd.Flags().String("description", "", "an optional description of the security group. Must be <= 127 chars")
-	cmd.Flags().Bool("stateful", false, "create a stateful or a stateless security group")
-	cmd.Flags().StringSlice("labels", nil, "a list of labels in the form <key>=<value>")
+			if !model.AssumeYes {
+				prompt := fmt.Sprintf("Are you sure you want to create the security group %q?", model.Name)
+				err = p.PromptForConfirmation(prompt)
+				if err != nil {
+					return err
+				}
+			}
 
-	if err := flags.MarkFlagsRequired(cmd, "name"); err != nil {
-		cobra.CheckErr(err)
-	}
-}
+			// Call API
+			request := buildRequest(ctx, model, apiClient)
 
-func executeCreate(cmd *cobra.Command, p *print.Printer, _ []string) error {
-	p.Info("executing create command")
-	ctx := context.Background()
-	model, err := parseInput(p, cmd)
-	if err != nil {
-		return err
-	}
+			operationState := "Created"
+			p.Info("%s security group %q for %q\n", operationState, model.Name, model.ProjectId)
 
-	// Configure API client
-	apiClient, err := client.ConfigureClient(p)
-	if err != nil {
-		return err
-	}
+			group, err := request.Execute()
+			if err != nil {
+				return fmt.Errorf("create security group: %w", err)
+			}
+			if err := outputResult(p, model, group); err != nil {
+				return err
+			}
 
-	if !model.AssumeYes {
-		prompt := fmt.Sprintf("Are you sure you want to create the security group %q?", model.Name)
-		err = p.PromptForConfirmation(prompt)
-		if err != nil {
-			return err
-		}
+			return nil
+		},
 	}
 
-	// Call API
-	request := buildRequest(ctx, model, apiClient)
+	configureFlags(cmd)
+	return cmd
+}
 
-	operationState := "Enabled"
-	if model.Async {
-		operationState = "Triggered security group creation"
-	}
-	p.Info("%s security group %q for %q\n", operationState, model.Name, model.ProjectId)
+func configureFlags(cmd *cobra.Command) {
+	cmd.Flags().String(nameFlag, "", "the name of the security group. Must be <= 63 chars")
+	cmd.Flags().String(descriptionFlag, "", "an optional description of the security group. Must be <= 127 chars")
+	cmd.Flags().Bool(statefulFlag, false, "create a stateful or a stateless security group")
+	cmd.Flags().StringSlice(labelsFlag, nil, "Labels are key-value string pairs which can be attached to a network-interface. E.g. '--labels key1=value1,key2=value2,...'")
 
-	group, err := request.Execute()
-	if err != nil {
-		return fmt.Errorf("create security group: %w", err)
-	}
-	if err:=outputResult(p, model, group);err != nil {
-		return err
+	if err := flags.MarkFlagsRequired(cmd, nameFlag); err != nil {
+		cobra.CheckErr(err)
 	}
-
-	return nil
 }
 
 func parseInput(p *print.Printer, cmd *cobra.Command) (*inputModel, error) {
 	globalFlags := globalflags.Parse(p, cmd)
 	if globalFlags.ProjectId == "" {
 		return nil, &errors.ProjectIdError{}
 	}
-	name := flags.FlagToStringValue(p, cmd, "name")
-	if len(name) >= 64 {
-		return nil, &errors.ArgValidationError{
-			Arg:     "invalid name",
-			Details: "name exceeds 63 characters in length",
-		}
-	}
+	name := flags.FlagToStringValue(p, cmd, nameFlag)
 
 	labels := make(map[string]any)
-	for _, label := range flags.FlagToStringSliceValue(p, cmd, "labels") {
+	for _, label := range flags.FlagToStringSliceValue(p, cmd, labelsFlag) {
 		parts := strings.Split(label, "=")
 		if len(parts) != 2 {
 			return nil, &errors.ArgValidationError{
-				Arg:     "labels",
+				Arg:     labelsFlag,
 				Details: "invalid label declaration. Must be in the form <key>=<value>",
 			}
 		}
 		labels[parts[0]] = parts[1]
 
 	}
-	description := flags.FlagToStringValue(p, cmd, "description")
-	if len(description) >= 128 {
-		return nil, &errors.ArgValidationError{
-			Arg:     "invalid description",
-			Details: "description exceeds 127 characters in length",
-		}
-	}
+	description := flags.FlagToStringValue(p, cmd, descriptionFlag)
+
 	model := inputModel{
 		GlobalFlagModel: globalFlags,
 		Name:            name,
 
-		Labels:      labels,
+		Labels:      &labels,
 		Description: description,
-		Stateful:    flags.FlagToBoolValue(p, cmd, "stateful"),
+		Stateful:    flags.FlagToBoolValue(p, cmd, statefulFlag),
 	}
 
 	if p.IsVerbosityDebug() {
@@ -158,7 +146,7 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient *iaas.APICli
 	if model.Labels != nil {
 		// this check assure that we don't end up with a pointer to nil
 		// which is a thing in go!
-		payload.Labels = &model.Labels
+		payload.Labels = model.Labels
 	}
 	payload.Name = &model.Name
 	payload.Stateful = &model.Stateful
@@ -188,9 +176,6 @@ func outputResult(p *print.Printer, model *inputModel, resp *iaas.SecurityGroup)
 		return nil
 	default:
 		operationState := "Created"
-		if model.Async {
-			operationState = "Triggered creation of"
-		}
 		p.Outputf("%s security group %q\n", operationState, model.Name)
 		return nil
 	}

internal/cmd/beta/security-group/create/create_test.go

@@ -2,7 +2,6 @@ package create
 
 import (
 	"context"
-	"strings"
 	"testing"
 
 	"github.com/stackitcloud/stackit-cli/internal/pkg/globalflags"
@@ -35,11 +34,11 @@ var (
 
 func fixtureFlagValues(mods ...func(flagValues map[string]string)) map[string]string {
 	flagValues := map[string]string{
-		projectIdFlag: testProjectId,
-		"description": testDescription,
-		"labels":      "fooKey=fooValue,barKey=barValue,bazKey=bazValue",
-		"stateful":    "true",
-		"name":        testName,
+		projectIdFlag:   testProjectId,
+		descriptionFlag: testDescription,
+		labelsFlag:      "fooKey=fooValue,barKey=barValue,bazKey=bazValue",
+		"stateful":      "true",
+		nameFlag:        testName,
 	}
 	for _, mod := range mods {
 		mod(flagValues)
@@ -50,7 +49,7 @@ func fixtureFlagValues(mods ...func(flagValues map[string]string)) map[string]st
 func fixtureInputModel(mods ...func(model *inputModel)) *inputModel {
 	model := &inputModel{
 		GlobalFlagModel: &globalflags.GlobalFlagModel{ProjectId: testProjectId, Verbosity: globalflags.VerbosityDefault},
-		Labels:          testLabels,
+		Labels:          &testLabels,
 		Description:     testDescription,
 		Name:            testName,
 		Stateful:        testStateful,
@@ -118,57 +117,43 @@ func TestParseInput(t *testing.T) {
 		{
 			description: "name missing",
 			flagValues: fixtureFlagValues(func(flagValues map[string]string) {
-				delete(flagValues, "name")
-			}),
-			isValid: false,
-		},
-		{
-			description: "name too long",
-			flagValues: fixtureFlagValues(func(flagValues map[string]string) {
-				flagValues["name"] = strings.Repeat("toolong", 1000)
-			}),
-			isValid: false,
-		},
-		{
-			description: "description too long",
-			flagValues: fixtureFlagValues(func(flagValues map[string]string) {
-				flagValues["description"] = strings.Repeat("toolong", 1000)
+				delete(flagValues, nameFlag)
 			}),
 			isValid: false,
 		},
 		{
 			description: "no labels",
 			flagValues: fixtureFlagValues(func(flagValues map[string]string) {
-				delete(flagValues, "labels")
+				delete(flagValues, labelsFlag)
 			}),
 			isValid: true,
 			expectedModel: fixtureInputModel(func(model *inputModel) {
-				model.Labels = map[string]any{}
+				model.Labels = &map[string]any{}
 			}),
 		},
 		{
 			description: "single label",
 			flagValues: fixtureFlagValues(func(flagValues map[string]string) {
-				flagValues["labels"] = "foo=bar"
+				flagValues[labelsFlag] = "foo=bar"
 			}),
 			isValid: true,
 			expectedModel: fixtureInputModel(func(model *inputModel) {
-				model.Labels = map[string]any{
+				model.Labels = &map[string]any{
 					"foo": "bar",
 				}
 			}),
 		},
 		{
 			description: "malformed labels 1",
 			flagValues: fixtureFlagValues(func(flagValues map[string]string) {
-				flagValues["labels"] = "foo=bar=baz"
+				flagValues[labelsFlag] = "foo=bar=baz"
 			}),
 			isValid: false,
 		},
 		{
 			description: "malformed labels 2",
 			flagValues: fixtureFlagValues(func(flagValues map[string]string) {
-				flagValues["labels"] = "foobarbaz"
+				flagValues[labelsFlag] = "foobarbaz"
 			}),
 			isValid: false,
 		},

internal/cmd/beta/security-group/delete/delete.go

@@ -22,61 +22,53 @@ type inputModel struct {
 	Id string
 }
 
-const argNameGroupId = "groupId"
+const argNameGroupId = "GROUP_ID"
 
 func NewCmd(p *print.Printer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "delete",
-		Short: "delete a security group",
-		Long:  "delete a security group by its internal id",
+		Short: "Delete a security group",
+		Long:  "Delete a security group by its internal id.",
 		Args:  args.SingleArg(argNameGroupId, utils.ValidateUUID),
 		Example: examples.Build(
 			examples.NewExample(`delete a named group`, `$ stackit beta security-group delete 43ad419a-c68b-4911-87cd-e05752ac1e31`),
 		),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return executeDelete(cmd, p, args)
-		},
-	}
-
-	return cmd
-}
+			ctx := context.Background()
+			model, err := parseInput(p, cmd, args)
+			if err != nil {
+				return err
+			}
 
-func executeDelete(cmd *cobra.Command, p *print.Printer, args []string) error {
-	p.Info("executing delete command")
-	ctx := context.Background()
-	model, err := parseInput(p, cmd, args)
-	if err != nil {
-		return err
-	}
+			// Configure API client
+			apiClient, err := client.ConfigureClient(p)
+			if err != nil {
+				return err
+			}
 
-	// Configure API client
-	apiClient, err := client.ConfigureClient(p)
-	if err != nil {
-		return err
-	}
+			if !model.AssumeYes {
+				prompt := fmt.Sprintf("Are you sure you want to delete the security group %q?", model.Id)
+				err = p.PromptForConfirmation(prompt)
+				if err != nil {
+					return err
+				}
+			}
 
-	if !model.AssumeYes {
-		prompt := fmt.Sprintf("Are you sure you want to delete the security group %q?", model.Id)
-		err = p.PromptForConfirmation(prompt)
-		if err != nil {
-			return err
-		}
-	}
+			// Call API
+			request := buildRequest(ctx, model, apiClient)
 
-	// Call API
-	request := buildRequest(ctx, model, apiClient)
+			operationState := "Deleted"
+			p.Info("%s security group %q for %q\n", operationState, model.Id, model.ProjectId)
 
-	operationState := "Enabled"
-	if model.Async {
-		operationState = "Triggered security group deletion"
-	}
-	p.Info("%s security group %q for %q\n", operationState, model.Id, model.ProjectId)
+			if err := request.Execute(); err != nil {
+				return fmt.Errorf("delete security group: %w", err)
+			}
 
-	if err := request.Execute(); err != nil {
-		return fmt.Errorf("delete security group: %w", err)
+			return nil
+		},
 	}
 
-	return nil
+	return cmd
 }
 
 func parseInput(p *print.Printer, cmd *cobra.Command, args []string) (*inputModel, error) {