refactor(ufw):

- added tests for update implementation
- fixed -o flag that was already used by the system

Author: alexTiugan

internal/cmd/ufw/rules/update/update.go

@@ -110,7 +110,7 @@ func configureFlags(cmd *cobra.Command) {
 	cmd.Flags().StringP(descriptionFlag, "D", "", "Description")
 	cmd.Flags().StringP(etherTypeFlag, "e", "", "Specifies the bound of the rule (for security rules type)")
 	cmd.Flags().StringP(portRangeFlag, "r", "", "Port range (the Port range to which the rule applies, for security rules type)")
-	cmd.Flags().StringP(protocolFlag, "o", "", "The network protocol (e.g. TCP, UDP, ICMP, for security rules type)")
+	cmd.Flags().String(protocolFlag, "", "The network protocol (e.g. TCP, UDP, ICMP, for security rules type)")
 
 	err := flags.MarkFlagsRequired(cmd, sourceIpFlag)
 	cobra.CheckErr(err)

internal/cmd/ufw/rules/update/update_test.go

@@ -1 +1,272 @@
 package update
+
+import (
+	"context"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/google/uuid"
+	"github.com/spf13/cobra"
+	"github.com/stackitcloud/stackit-cli/internal/pkg/globalflags"
+	"github.com/stackitcloud/stackit-cli/internal/pkg/print"
+	"github.com/stackitcloud/stackit-cli/internal/pkg/testparams"
+	"github.com/stackitcloud/stackit-cli/internal/pkg/testutils"
+	ufw "github.com/stackitcloud/stackit-sdk-go/services/ufw/v1api"
+)
+
+type testCtxKey struct{}
+
+var (
+	testCtx       = context.WithValue(context.Background(), testCtxKey{}, "foo")
+	testClient    = &ufw.APIClient{DefaultAPI: &ufw.DefaultAPIService{}}
+	testProjectId = uuid.NewString()
+	testRuleRefId = uuid.NewString()
+)
+
+const (
+	testRegion   = "eu01"
+	testSourceIp = "1.1.1.1/32"
+)
+
+func fixtureArgValues(mods ...func(argValues []string)) []string {
+	argValues := []string{
+		testRuleRefId,
+	}
+	for _, mod := range mods {
+		mod(argValues)
+	}
+	return argValues
+}
+
+func fixtureFlagValues(mods ...func(flagValues map[string]string)) map[string]string {
+	flagValues := map[string]string{
+		globalflags.ProjectIdFlag: testProjectId,
+		globalflags.RegionFlag:    testRegion,
+		sourceIpFlag:              testSourceIp,
+		directionFlag:             "ingress",
+		descriptionFlag:           "example-description",
+		etherTypeFlag:             "IPv4",
+		portRangeFlag:             "80-443",
+		protocolFlag:              "TCP",
+	}
+	for _, mod := range mods {
+		mod(flagValues)
+	}
+	return flagValues
+}
+
+func fixtureInputModel(mods ...func(model *inputModel)) *inputModel {
+	model := &inputModel{
+		GlobalFlagModel: &globalflags.GlobalFlagModel{
+			ProjectId: testProjectId,
+			Region:    testRegion,
+			Verbosity: globalflags.VerbosityDefault,
+		},
+		RuleRefId:   "", // Left blank because parseInput in the source file currently does not populate it
+		SourceIp:    new(testSourceIp),
+		Direction:   new("ingress"),
+		Description: new("example-description"),
+		EtherType:   new("IPv4"),
+		PortRange:   new("80-443"),
+		Protocol:    new("TCP"),
+	}
+	for _, mod := range mods {
+		mod(model)
+	}
+	return model
+}
+
+func fixtureRequest(mods ...func(request *ufw.ApiUpdateRuleRequest)) ufw.ApiUpdateRuleRequest {
+	request := testClient.DefaultAPI.UpdateRule(testCtx, testProjectId, testRegion, testRuleRefId)
+	request = request.UpdateRulePayload(ufw.UpdateRulePayload{
+		SourceIP:  testSourceIp,
+		Direction: new("ingress"),
+		EtherType: new("IPv4"),
+		PortRange: new("80-443"),
+		Protocol:  new("TCP"),
+	})
+	for _, mod := range mods {
+		mod(&request)
+	}
+	return request
+}
+
+func TestParseInput(t *testing.T) {
+	tests := []struct {
+		description   string
+		argValues     []string
+		flagValues    map[string]string
+		isValid       bool
+		expectedModel *inputModel
+	}{
+		{
+			description:   "base",
+			argValues:     fixtureArgValues(),
+			flagValues:    fixtureFlagValues(),
+			isValid:       true,
+			expectedModel: fixtureInputModel(),
+		},
+		{
+			description: "no values",
+			argValues:   []string{},
+			flagValues:  map[string]string{},
+			isValid:     false,
+		},
+		{
+			description: "no flag values",
+			argValues:   fixtureArgValues(),
+			flagValues:  map[string]string{},
+			isValid:     false,
+		},
+		{
+			description: "required flags only",
+			argValues:   fixtureArgValues(),
+			flagValues: map[string]string{
+				globalflags.ProjectIdFlag: testProjectId,
+				globalflags.RegionFlag:    testRegion,
+				sourceIpFlag:              testSourceIp,
+			},
+			isValid: true,
+			expectedModel: &inputModel{
+				GlobalFlagModel: &globalflags.GlobalFlagModel{
+					ProjectId: testProjectId,
+					Region:    testRegion,
+					Verbosity: globalflags.VerbosityDefault,
+				},
+				SourceIp: new(testSourceIp),
+			},
+		},
+		{
+			description: "project id missing",
+			argValues:   fixtureArgValues(),
+			flagValues: fixtureFlagValues(func(flagValues map[string]string) {
+				delete(flagValues, globalflags.ProjectIdFlag)
+			}),
+			isValid: false,
+		},
+		{
+			description: "project id invalid 1",
+			argValues:   fixtureArgValues(),
+			flagValues: fixtureFlagValues(func(flagValues map[string]string) {
+				flagValues[globalflags.ProjectIdFlag] = ""
+			}),
+			isValid: false,
+		},
+		{
+			description: "project id invalid 2",
+			argValues:   fixtureArgValues(),
+			flagValues: fixtureFlagValues(func(flagValues map[string]string) {
+				flagValues[globalflags.ProjectIdFlag] = "invalid-uuid"
+			}),
+			isValid: false,
+		},
+		{
+			description: "region missing",
+			argValues:   fixtureArgValues(),
+			flagValues: fixtureFlagValues(func(flagValues map[string]string) {
+				delete(flagValues, globalflags.RegionFlag)
+			}),
+			isValid: false,
+		},
+		{
+			description: "source IP missing",
+			argValues:   fixtureArgValues(),
+			flagValues: fixtureFlagValues(func(flagValues map[string]string) {
+				delete(flagValues, sourceIpFlag)
+			}),
+			isValid: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.description, func(t *testing.T) {
+			parseInputWrapper := func(p *print.Printer, cmd *cobra.Command, _ []string) (*inputModel, error) {
+				return parseInput(p, cmd)
+			}
+			testutils.TestParseInput(t, NewCmd, parseInputWrapper, tt.expectedModel, tt.argValues, tt.flagValues, tt.isValid)
+		})
+	}
+}
+
+func TestBuildRequest(t *testing.T) {
+	tests := []struct {
+		description     string
+		model           *inputModel
+		expectedRequest ufw.ApiUpdateRuleRequest
+	}{
+		{
+			description: "base",
+			model: fixtureInputModel(func(model *inputModel) {
+				model.RuleRefId = testRuleRefId // Inject the ID that parseInput currently skips
+			}),
+			expectedRequest: fixtureRequest(),
+		},
+		{
+			description: "required fields only",
+			model: &inputModel{
+				GlobalFlagModel: &globalflags.GlobalFlagModel{
+					ProjectId: testProjectId,
+					Region:    testRegion,
+					Verbosity: globalflags.VerbosityDefault,
+				},
+				RuleRefId: testRuleRefId,
+				SourceIp:  new(testSourceIp),
+			},
+			expectedRequest: testClient.DefaultAPI.UpdateRule(testCtx, testProjectId, testRegion, testRuleRefId).
+				UpdateRulePayload(ufw.UpdateRulePayload{
+					SourceIP: testSourceIp,
+				}),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.description, func(t *testing.T) {
+			request := buildRequest(testCtx, tt.model, testClient)
+
+			diff := cmp.Diff(request, tt.expectedRequest,
+				cmp.AllowUnexported(tt.expectedRequest),
+				cmpopts.EquateComparable(testCtx, ufw.DefaultAPIService{}),
+			)
+			if diff != "" {
+				t.Fatalf("Data does not match: %s", diff)
+			}
+		})
+	}
+}
+
+func TestOutputResult(t *testing.T) {
+	type args struct {
+		outputFormat string
+		async        bool
+		projectLabel string
+		rule         *ufw.UpdateRuleResponse
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{
+			name:    "empty",
+			args:    args{},
+			wantErr: true,
+		},
+		{
+			name: "set empty response",
+			args: args{
+				rule: &ufw.UpdateRuleResponse{},
+			},
+			wantErr: false,
+		},
+	}
+
+	params := testparams.NewTestParams()
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := outputResult(params.Printer, tt.args.outputFormat, tt.args.async, tt.args.projectLabel, tt.args.rule); (err != nil) != tt.wantErr {
+				t.Errorf("outputResult() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}