From 212864df9f0760bfb52e690adb00456393f4136e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 09:31:11 +0000 Subject: [PATCH 01/17] Onboard Argus Instance: update command --- internal/cmd/argus/instance/update/update.go | 75 ++- .../cmd/argus/instance/update/update_test.go | 444 +----------------- internal/pkg/services/argus/utils/utils.go | 2 +- 3 files changed, 39 insertions(+), 482 deletions(-) diff --git a/internal/cmd/argus/instance/update/update.go b/internal/cmd/argus/instance/update/update.go index 3bd4cd774..6a7e09b8d 100644 --- a/internal/cmd/argus/instance/update/update.go +++ b/internal/cmd/argus/instance/update/update.go @@ -31,11 +31,11 @@ const ( type inputModel struct { *globalflags.GlobalFlagModel - InstanceId string - PlanName string + InstanceId string + PlanName string + InstanceName string - InstanceName *string - PlanId *string + PlanId *string } func NewCmd() *cobra.Command { @@ -46,11 +46,8 @@ func NewCmd() *cobra.Command { Args: args.SingleArg(instanceIdArg, utils.ValidateUUID), Example: examples.Build( examples.NewExample( - `Update the plan of an Argus instance with ID "xxx" by specifying the plan ID`, + `Update the plan of an Argus instance with ID "xxx"`, "$ stackit argus instance update xxx --plan-id yyy"), - examples.NewExample( - `Update the plan of an Argus instance with ID "xxx" by specifying the plan name`, - "$ stackit argus instance update xxx --plan-name yyy"), examples.NewExample( `Update the name of an Argus instance with ID "xxx"`, "$ stackit argus instance update xxx --name new-instance-name"), @@ -68,8 +65,8 @@ func NewCmd() *cobra.Command { return err } - instanceLabel, err := argusUtils.GetInstanceName(ctx, apiClient, model.InstanceId, model.ProjectId) - if err != nil || instanceLabel == "" { + instanceLabel, err := argusUtils.GetInstanceName(ctx, apiClient, model.ProjectId, model.InstanceId) + if err != nil { instanceLabel = model.InstanceId } @@ -136,18 +133,32 @@ func parseInput(cmd *cobra.Command, inputArgs []string) (*inputModel, error) { planId := flags.FlagToStringPointer(cmd, planIdFlag) planName := flags.FlagToStringValue(cmd, planNameFlag) - instanceName := flags.FlagToStringPointer(cmd, instanceNameFlag) + instanceName := flags.FlagToStringValue(cmd, instanceNameFlag) if planId != nil && (planName != "") { return nil, &cliErr.ArgusInputPlanError{ - Cmd: cmd, + Cmd: cmd, + Args: inputArgs, } } - if planId == nil && planName == "" && instanceName == nil { + //TODO: planID or planName is required + + if planId == nil && planName == "" && instanceName == "" { return nil, &cliErr.EmptyUpdateError{} } + if planId == nil && (planName == "") { + return nil, &cliErr.ArgusInputPlanError{ + Cmd: cmd, + } + } + if planId != nil && (planName != "") { + return nil, &cliErr.ArgusInputPlanError{ + Cmd: cmd, + } + } + return &inputModel{ GlobalFlagModel: globalFlags, InstanceId: instanceId, @@ -160,12 +171,12 @@ func parseInput(cmd *cobra.Command, inputArgs []string) (*inputModel, error) { type argusClient interface { UpdateInstance(ctx context.Context, instanceId, projectId string) argus.ApiUpdateInstanceRequest ListPlansExecute(ctx context.Context, projectId string) (*argus.PlansResponse, error) - GetInstanceExecute(ctx context.Context, instanceId, projectId string) (*argus.GetInstanceResponse, error) } func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) (argus.ApiUpdateInstanceRequest, error) { req := apiClient.UpdateInstance(ctx, model.InstanceId, model.ProjectId) + var planId *string var err error plans, err := apiClient.ListPlansExecute(ctx, model.ProjectId) @@ -173,18 +184,8 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) return req, fmt.Errorf("get Argus plans: %w", err) } - currentInstance, err := apiClient.GetInstanceExecute(ctx, model.InstanceId, model.ProjectId) - if err != nil { - return req, fmt.Errorf("get Argus instance: %w", err) - } - - payload := argus.UpdateInstancePayload{ - PlanId: currentInstance.PlanId, - Name: currentInstance.Name, - } - if model.PlanId == nil && model.PlanName != "" { - payload.PlanId, err = argusUtils.LoadPlanId(model.PlanName, plans) + planId, err = argusUtils.LoadPlanId(model.PlanName, plans) if err != nil { var argusInvalidPlanError *cliErr.ArgusInvalidPlanError if !errors.As(err, &argusInvalidPlanError) { @@ -192,22 +193,20 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) } return req, err } - } else if model.PlanId != nil && model.PlanName == "" { - err := argusUtils.ValidatePlanId(*model.PlanId, plans) - if err != nil { - var argusInvalidPlanError *cliErr.ArgusInvalidPlanError - if !errors.As(err, &argusInvalidPlanError) { - return req, fmt.Errorf("validate plan ID: %w", err) + } else { + // planId is not required for update operation + if model.PlanId != nil { + err := argusUtils.ValidatePlanId(*model.PlanId, plans) + if err != nil { + return req, err } - return req, err } - payload.PlanId = model.PlanId - } - - if model.InstanceName != nil { - payload.Name = model.InstanceName + planId = model.PlanId } - req = req.UpdateInstancePayload(payload) + req = req.UpdateInstancePayload(argus.UpdateInstancePayload{ + PlanId: planId, + Name: &model.InstanceName, + }) return req, nil } diff --git a/internal/cmd/argus/instance/update/update_test.go b/internal/cmd/argus/instance/update/update_test.go index 5a2ab1d33..ef7adbfe0 100644 --- a/internal/cmd/argus/instance/update/update_test.go +++ b/internal/cmd/argus/instance/update/update_test.go @@ -1,443 +1 @@ -package update - -import ( - "context" - "fmt" - "testing" - - "github.com/stackitcloud/stackit-cli/internal/pkg/globalflags" - "github.com/stackitcloud/stackit-cli/internal/pkg/utils" - - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/google/uuid" - "github.com/stackitcloud/stackit-sdk-go/services/argus" -) - -var projectIdFlag = globalflags.ProjectIdFlag - -type testCtxKey struct{} - -var testCtx = context.WithValue(context.Background(), testCtxKey{}, "foo") -var testClient = &argus.APIClient{} - -type argusClientMocked struct { - listPlansError bool - listPlansResponse *argus.PlansResponse - getInstanceError bool - getInstanceResponse *argus.GetInstanceResponse -} - -func (c *argusClientMocked) UpdateInstance(ctx context.Context, instanceId, projectId string) argus.ApiUpdateInstanceRequest { - return testClient.UpdateInstance(ctx, instanceId, projectId) -} - -func (c *argusClientMocked) ListPlansExecute(_ context.Context, _ string) (*argus.PlansResponse, error) { - if c.listPlansError { - return nil, fmt.Errorf("list flavors failed") - } - return c.listPlansResponse, nil -} - -func (c *argusClientMocked) GetInstanceExecute(_ context.Context, _, _ string) (*argus.GetInstanceResponse, error) { - if c.getInstanceError { - return nil, fmt.Errorf("get instance failed") - } - return c.getInstanceResponse, nil -} - -const ( - testInstanceName = "example-instance-name" -) - -var ( - testProjectId = uuid.NewString() - testInstanceId = uuid.NewString() - testPlanId = uuid.NewString() - testNewPlanId = uuid.NewString() -) - -func fixtureArgValues(mods ...func(argValues []string)) []string { - argValues := []string{ - testInstanceId, - } - for _, mod := range mods { - mod(argValues) - } - return argValues -} - -func fixtureFlagValues(mods ...func(flagValues map[string]string)) map[string]string { - flagValues := map[string]string{ - projectIdFlag: testProjectId, - planIdFlag: testNewPlanId, - } - for _, mod := range mods { - mod(flagValues) - } - return flagValues -} - -func fixtureInputModel(mods ...func(model *inputModel)) *inputModel { - model := &inputModel{ - GlobalFlagModel: &globalflags.GlobalFlagModel{ - ProjectId: testProjectId, - }, - InstanceId: testInstanceId, - PlanId: utils.Ptr(testNewPlanId), - } - for _, mod := range mods { - mod(model) - } - return model -} - -func fixtureRequest(mods ...func(request *argus.ApiUpdateInstanceRequest)) argus.ApiUpdateInstanceRequest { - request := testClient.UpdateInstance(testCtx, testInstanceId, testProjectId) - request = request.UpdateInstancePayload(argus.UpdateInstancePayload{ - PlanId: utils.Ptr(testNewPlanId), - Name: utils.Ptr(testInstanceName), - }) - for _, mod := range mods { - mod(&request) - } - return request -} - -func fixturePlansResponse(mods ...func(response *argus.PlansResponse)) *argus.PlansResponse { - response := &argus.PlansResponse{ - Plans: &[]argus.Plan{ - { - Name: utils.Ptr("example-plan-name"), - Id: utils.Ptr(testNewPlanId), - }, - }, - } - for _, mod := range mods { - mod(response) - } - return response -} - -func fixtureGetInstanceResponse(mods ...func(response *argus.GetInstanceResponse)) *argus.GetInstanceResponse { - response := &argus.GetInstanceResponse{ - PlanId: utils.Ptr(testPlanId), - Name: utils.Ptr(testInstanceName), - } - for _, mod := range mods { - mod(response) - } - return response -} - -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: "with plan name", - argValues: fixtureArgValues(), - flagValues: fixtureFlagValues(func(flagValues map[string]string) { - flagValues[planNameFlag] = "plan-name" - delete(flagValues, planIdFlag) - }), - isValid: true, - expectedModel: fixtureInputModel(func(model *inputModel) { - model.PlanId = nil - model.PlanName = "plan-name" - }), - }, - { - description: "with new instance name", - argValues: fixtureArgValues(), - flagValues: fixtureFlagValues(func(flagValues map[string]string) { - flagValues[instanceNameFlag] = "new-instance-name" - delete(flagValues, planIdFlag) - }), - isValid: true, - expectedModel: fixtureInputModel(func(model *inputModel) { - model.PlanId = nil - model.PlanName = "" - model.InstanceName = utils.Ptr("new-instance-name") - }), - }, - { - description: "no values", - argValues: []string{}, - flagValues: map[string]string{}, - isValid: false, - }, - { - description: "no arg values", - argValues: []string{}, - flagValues: fixtureFlagValues(), - isValid: false, - }, - { - description: "no flag values", - argValues: fixtureArgValues(), - flagValues: map[string]string{}, - isValid: false, - }, - { - description: "project id missing", - argValues: fixtureArgValues(), - flagValues: fixtureFlagValues(func(flagValues map[string]string) { - delete(flagValues, projectIdFlag) - }), - isValid: false, - }, - { - description: "project id invalid 1", - argValues: fixtureArgValues(), - flagValues: fixtureFlagValues(func(flagValues map[string]string) { - flagValues[projectIdFlag] = "" - }), - isValid: false, - }, - { - description: "project id invalid 2", - argValues: fixtureArgValues(), - flagValues: fixtureFlagValues(func(flagValues map[string]string) { - flagValues[projectIdFlag] = "invalid-uuid" - }), - isValid: false, - }, - { - description: "instance id invalid 1", - argValues: []string{""}, - flagValues: fixtureFlagValues(), - isValid: false, - }, - { - description: "instance id invalid 2", - argValues: []string{"invalid-uuid"}, - flagValues: fixtureFlagValues(), - isValid: false, - }, - { - description: "invalid with plan ID and plan name", - flagValues: fixtureFlagValues(func(flagValues map[string]string) { - flagValues[planNameFlag] = "plan-name" - }), - isValid: false, - }, - } - - for _, tt := range tests { - t.Run(tt.description, func(t *testing.T) { - cmd := NewCmd() - err := globalflags.Configure(cmd.Flags()) - if err != nil { - t.Fatalf("configure global flags: %v", err) - } - - for flag, value := range tt.flagValues { - err := cmd.Flags().Set(flag, value) - if err != nil { - if !tt.isValid { - return - } - t.Fatalf("setting flag --%s=%s: %v", flag, value, err) - } - } - - err = cmd.ValidateArgs(tt.argValues) - if err != nil { - if !tt.isValid { - return - } - t.Fatalf("error validating args: %v", err) - } - - err = cmd.ValidateRequiredFlags() - if err != nil { - if !tt.isValid { - return - } - t.Fatalf("error validating flags: %v", err) - } - - model, err := parseInput(cmd, tt.argValues) - if err != nil { - if !tt.isValid { - return - } - t.Fatalf("error parsing flags: %v", err) - } - - if !tt.isValid { - t.Fatalf("did not fail on invalid input") - } - diff := cmp.Diff(model, tt.expectedModel) - if diff != "" { - t.Fatalf("Data does not match: %s", diff) - } - }) - } -} - -func TestBuildRequest(t *testing.T) { - tests := []struct { - description string - model *inputModel - expectedRequest argus.ApiUpdateInstanceRequest - getPlansFails bool - getPlansResponse *argus.PlansResponse - getInstanceFails bool - getInstanceResponse *argus.GetInstanceResponse - isValid bool - }{ - { - description: "base", - model: fixtureInputModel(), - expectedRequest: fixtureRequest(), - getPlansResponse: fixturePlansResponse(), - getInstanceResponse: fixtureGetInstanceResponse(), - isValid: true, - }, - { - description: "use plan name", - model: fixtureInputModel( - func(model *inputModel) { - model.PlanId = nil - model.PlanName = "example-plan-name" - }, - ), - expectedRequest: fixtureRequest(), - getPlansResponse: fixturePlansResponse(), - getInstanceResponse: fixtureGetInstanceResponse(), - isValid: true, - }, - { - description: "get plans fails", - model: fixtureInputModel( - func(model *inputModel) { - model.PlanId = nil - model.PlanName = "example-plan-name" - }, - ), - getPlansFails: true, - isValid: false, - }, - { - description: "plan name not found", - model: fixtureInputModel( - func(model *inputModel) { - model.PlanId = nil - model.PlanName = "non-existent-plan" - }, - ), - getPlansResponse: fixturePlansResponse(), - getInstanceResponse: fixtureGetInstanceResponse(), - isValid: false, - }, - { - description: "plan id not found", - model: fixtureInputModel( - func(model *inputModel) { - model.PlanId = utils.Ptr(uuid.NewString()) - }, - ), - getPlansResponse: fixturePlansResponse(), - getInstanceResponse: fixtureGetInstanceResponse(), - isValid: false, - }, - { - description: "plan id, no instance name", - model: fixtureInputModel( - func(model *inputModel) { - model.InstanceName = nil - }, - ), - getPlansResponse: fixturePlansResponse(), - getInstanceResponse: fixtureGetInstanceResponse(), - expectedRequest: fixtureRequest(), - isValid: true, - }, - { - description: "plan name, no instance name", - model: fixtureInputModel( - func(model *inputModel) { - model.PlanId = nil - model.PlanName = "example-plan-name" - model.InstanceName = nil - }, - ), - getPlansResponse: fixturePlansResponse(), - getInstanceResponse: fixtureGetInstanceResponse(), - expectedRequest: fixtureRequest(), - isValid: true, - }, - { - description: "instance name, no plan info", - model: fixtureInputModel( - func(model *inputModel) { - model.PlanId = nil - model.PlanName = "" - model.InstanceName = utils.Ptr("new-instance-name") - }, - ), - getInstanceResponse: fixtureGetInstanceResponse(), - expectedRequest: fixtureRequest(). - UpdateInstancePayload(argus.UpdateInstancePayload{ - PlanId: utils.Ptr(testPlanId), - Name: utils.Ptr("new-instance-name"), - }), - isValid: true, - }, - { - description: "instance name, no plan info, get instance fails", - model: fixtureInputModel( - func(model *inputModel) { - model.PlanId = nil - model.PlanName = "" - model.InstanceName = utils.Ptr("new-instance-name") - }, - ), - getInstanceFails: true, - isValid: false, - }, - } - - for _, tt := range tests { - t.Run(tt.description, func(t *testing.T) { - client := &argusClientMocked{ - listPlansError: tt.getPlansFails, - listPlansResponse: tt.getPlansResponse, - getInstanceError: tt.getInstanceFails, - getInstanceResponse: tt.getInstanceResponse, - } - request, err := buildRequest(testCtx, tt.model, client) - if err != nil { - if !tt.isValid { - return - } - t.Fatalf("error building request: %v", err) - } - - if !tt.isValid { - t.Fatal("expected error but none thrown") - } - - diff := cmp.Diff(request, tt.expectedRequest, - cmp.AllowUnexported(tt.expectedRequest), - cmpopts.EquateComparable(testCtx), - ) - if diff != "" { - t.Fatalf("Data does not match: %s", diff) - } - }) - } -} +package update \ No newline at end of file diff --git a/internal/pkg/services/argus/utils/utils.go b/internal/pkg/services/argus/utils/utils.go index e530c187b..5853a6b64 100644 --- a/internal/pkg/services/argus/utils/utils.go +++ b/internal/pkg/services/argus/utils/utils.go @@ -57,7 +57,7 @@ func LoadPlanId(planName string, resp *argus.PlansResponse) (*string, error) { } type ArgusClient interface { - GetInstanceExecute(ctx context.Context, instanceId, projectId string) (*argus.GetInstanceResponse, error) + GetInstanceExecute(ctx context.Context, projectId, instanceId string) (*argus.GetInstanceResponse, error) } func GetInstanceName(ctx context.Context, apiClient ArgusClient, instanceId, projectId string) (string, error) { From 1f647c32bc158ca94dc4aff1374929fea480c31e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 10:44:35 +0000 Subject: [PATCH 02/17] update command: support no plan info provided --- internal/cmd/argus/instance/update/update.go | 32 +++++-------- internal/pkg/services/argus/utils/utils.go | 10 +++- .../pkg/services/argus/utils/utils_test.go | 48 +++++++++++++++++++ 3 files changed, 70 insertions(+), 20 deletions(-) diff --git a/internal/cmd/argus/instance/update/update.go b/internal/cmd/argus/instance/update/update.go index 6a7e09b8d..e2c4e686b 100644 --- a/internal/cmd/argus/instance/update/update.go +++ b/internal/cmd/argus/instance/update/update.go @@ -46,8 +46,11 @@ func NewCmd() *cobra.Command { Args: args.SingleArg(instanceIdArg, utils.ValidateUUID), Example: examples.Build( examples.NewExample( - `Update the plan of an Argus instance with ID "xxx"`, + `Update the plan of an Argus instance with ID "xxx" by specifying the plan ID`, "$ stackit argus instance update xxx --plan-id yyy"), + examples.NewExample( + `Update the plan of an Argus instance with ID "xxx" by specifying the plan name`, + "$ stackit argus instance update xxx --plan-name yyy"), examples.NewExample( `Update the name of an Argus instance with ID "xxx"`, "$ stackit argus instance update xxx --name new-instance-name"), @@ -65,7 +68,7 @@ func NewCmd() *cobra.Command { return err } - instanceLabel, err := argusUtils.GetInstanceName(ctx, apiClient, model.ProjectId, model.InstanceId) + instanceLabel, err := argusUtils.GetInstanceName(ctx, apiClient, model.InstanceId, model.ProjectId) if err != nil { instanceLabel = model.InstanceId } @@ -137,28 +140,14 @@ func parseInput(cmd *cobra.Command, inputArgs []string) (*inputModel, error) { if planId != nil && (planName != "") { return nil, &cliErr.ArgusInputPlanError{ - Cmd: cmd, - Args: inputArgs, + Cmd: cmd, } } - //TODO: planID or planName is required - if planId == nil && planName == "" && instanceName == "" { return nil, &cliErr.EmptyUpdateError{} } - if planId == nil && (planName == "") { - return nil, &cliErr.ArgusInputPlanError{ - Cmd: cmd, - } - } - if planId != nil && (planName != "") { - return nil, &cliErr.ArgusInputPlanError{ - Cmd: cmd, - } - } - return &inputModel{ GlobalFlagModel: globalFlags, InstanceId: instanceId, @@ -171,6 +160,7 @@ func parseInput(cmd *cobra.Command, inputArgs []string) (*inputModel, error) { type argusClient interface { UpdateInstance(ctx context.Context, instanceId, projectId string) argus.ApiUpdateInstanceRequest ListPlansExecute(ctx context.Context, projectId string) (*argus.PlansResponse, error) + GetInstanceExecute(ctx context.Context, instanceId, projectId string) (*argus.GetInstanceResponse, error) } func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) (argus.ApiUpdateInstanceRequest, error) { @@ -193,8 +183,12 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) } return req, err } + } else if model.PlanId == nil && model.PlanName == "" { + planId, err = argusUtils.GetInstancePlanId(ctx, apiClient, model.InstanceId, model.ProjectId) + if err != nil { + return req, fmt.Errorf("get Argus instance plan ID: %w", err) + } } else { - // planId is not required for update operation if model.PlanId != nil { err := argusUtils.ValidatePlanId(*model.PlanId, plans) if err != nil { @@ -206,7 +200,7 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) req = req.UpdateInstancePayload(argus.UpdateInstancePayload{ PlanId: planId, - Name: &model.InstanceName, + Name: &model.InstanceName, }) return req, nil } diff --git a/internal/pkg/services/argus/utils/utils.go b/internal/pkg/services/argus/utils/utils.go index 5853a6b64..fa028b532 100644 --- a/internal/pkg/services/argus/utils/utils.go +++ b/internal/pkg/services/argus/utils/utils.go @@ -57,7 +57,7 @@ func LoadPlanId(planName string, resp *argus.PlansResponse) (*string, error) { } type ArgusClient interface { - GetInstanceExecute(ctx context.Context, projectId, instanceId string) (*argus.GetInstanceResponse, error) + GetInstanceExecute(ctx context.Context, instanceId, projectId string) (*argus.GetInstanceResponse, error) } func GetInstanceName(ctx context.Context, apiClient ArgusClient, instanceId, projectId string) (string, error) { @@ -67,3 +67,11 @@ func GetInstanceName(ctx context.Context, apiClient ArgusClient, instanceId, pro } return *resp.Name, nil } + +func GetInstancePlanId(ctx context.Context, apiClient ArgusClient, instanceId, projectId string) (*string, error) { + resp, err := apiClient.GetInstanceExecute(ctx, instanceId, projectId) + if err != nil { + return nil, fmt.Errorf("get Argus instance: %w", err) + } + return resp.PlanId, nil +} diff --git a/internal/pkg/services/argus/utils/utils_test.go b/internal/pkg/services/argus/utils/utils_test.go index 8adf5de1b..6ca572aa9 100644 --- a/internal/pkg/services/argus/utils/utils_test.go +++ b/internal/pkg/services/argus/utils/utils_test.go @@ -91,6 +91,54 @@ func TestGetInstanceName(t *testing.T) { } } +func TestGetInstancePlanId(t *testing.T) { + tests := []struct { + description string + getInstanceFails bool + getInstanceResp *argus.GetInstanceResponse + isValid bool + expectedOutput string + }{ + { + description: "base", + getInstanceResp: &argus.GetInstanceResponse{ + PlanId: utils.Ptr(testPlanId), + }, + isValid: true, + expectedOutput: testPlanId, + }, + { + description: "get instance fails", + getInstanceFails: true, + isValid: false, + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + client := &argusClientMocked{ + getInstanceFails: tt.getInstanceFails, + getInstanceResp: tt.getInstanceResp, + } + + output, err := GetInstancePlanId(context.Background(), client, testInstanceId, testProjectId) + + if tt.isValid && err != nil { + t.Errorf("failed on valid input") + } + if !tt.isValid && err == nil { + t.Errorf("did not fail on invalid input") + } + if !tt.isValid { + return + } + if *output != tt.expectedOutput { + t.Errorf("expected output to be %s, got %s", tt.expectedOutput, *output) + } + }) + } +} + func TestLoadPlanId(t *testing.T) { tests := []struct { description string From 5221ce8463348ef9c338a07c9daaed3a99eb5587 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 11:19:06 +0000 Subject: [PATCH 03/17] update command: testing --- .../cmd/argus/instance/create/create_test.go | 30 +- internal/cmd/argus/instance/update/update.go | 8 +- .../cmd/argus/instance/update/update_test.go | 434 +++++++++++++++++- 3 files changed, 452 insertions(+), 20 deletions(-) diff --git a/internal/cmd/argus/instance/create/create_test.go b/internal/cmd/argus/instance/create/create_test.go index e75827f86..bab8ad792 100644 --- a/internal/cmd/argus/instance/create/create_test.go +++ b/internal/cmd/argus/instance/create/create_test.go @@ -22,7 +22,7 @@ var testCtx = context.WithValue(context.Background(), testCtxKey{}, "foo") var testClient = &argus.APIClient{} type argusClientMocked struct { - returnError bool + returnError bool listPlansResponse *argus.PlansResponse } @@ -216,19 +216,19 @@ func TestParseInput(t *testing.T) { func TestBuildRequest(t *testing.T) { tests := []struct { - description string - model *inputModel - expectedRequest argus.ApiCreateInstanceRequest - getPlansFails bool + description string + model *inputModel + expectedRequest argus.ApiCreateInstanceRequest + getPlansFails bool getPlansResponse *argus.PlansResponse - isValid bool + isValid bool }{ { - description: "base", - model: fixtureInputModel(), - expectedRequest: fixtureRequest(), + description: "base", + model: fixtureInputModel(), + expectedRequest: fixtureRequest(), getPlansResponse: fixturePlansResponse(), - isValid: true, + isValid: true, }, { description: "use plan name", @@ -238,9 +238,9 @@ func TestBuildRequest(t *testing.T) { model.PlanName = "example-plan-name" }, ), - expectedRequest: fixtureRequest(), + expectedRequest: fixtureRequest(), getPlansResponse: fixturePlansResponse(), - isValid: true, + isValid: true, }, { description: "get plans fails", @@ -262,7 +262,7 @@ func TestBuildRequest(t *testing.T) { }, ), getPlansResponse: fixturePlansResponse(), - isValid: false, + isValid: false, }, { description: "plan id not found", @@ -272,7 +272,7 @@ func TestBuildRequest(t *testing.T) { }, ), getPlansResponse: fixturePlansResponse(), - isValid: false, + isValid: false, }, { description: "plan id, no instance name", @@ -305,7 +305,7 @@ func TestBuildRequest(t *testing.T) { for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { client := &argusClientMocked{ - returnError: tt.getPlansFails, + returnError: tt.getPlansFails, listPlansResponse: tt.getPlansResponse, } request, err := buildRequest(testCtx, tt.model, client) diff --git a/internal/cmd/argus/instance/update/update.go b/internal/cmd/argus/instance/update/update.go index e2c4e686b..3f66aeac6 100644 --- a/internal/cmd/argus/instance/update/update.go +++ b/internal/cmd/argus/instance/update/update.go @@ -33,8 +33,8 @@ type inputModel struct { *globalflags.GlobalFlagModel InstanceId string PlanName string - InstanceName string + InstanceName *string PlanId *string } @@ -136,7 +136,7 @@ func parseInput(cmd *cobra.Command, inputArgs []string) (*inputModel, error) { planId := flags.FlagToStringPointer(cmd, planIdFlag) planName := flags.FlagToStringValue(cmd, planNameFlag) - instanceName := flags.FlagToStringValue(cmd, instanceNameFlag) + instanceName := flags.FlagToStringPointer(cmd, instanceNameFlag) if planId != nil && (planName != "") { return nil, &cliErr.ArgusInputPlanError{ @@ -144,7 +144,7 @@ func parseInput(cmd *cobra.Command, inputArgs []string) (*inputModel, error) { } } - if planId == nil && planName == "" && instanceName == "" { + if planId == nil && planName == "" && instanceName == nil { return nil, &cliErr.EmptyUpdateError{} } @@ -200,7 +200,7 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) req = req.UpdateInstancePayload(argus.UpdateInstancePayload{ PlanId: planId, - Name: &model.InstanceName, + Name: model.InstanceName, }) return req, nil } diff --git a/internal/cmd/argus/instance/update/update_test.go b/internal/cmd/argus/instance/update/update_test.go index ef7adbfe0..7746d7cc6 100644 --- a/internal/cmd/argus/instance/update/update_test.go +++ b/internal/cmd/argus/instance/update/update_test.go @@ -1 +1,433 @@ -package update \ No newline at end of file +package update + +import ( + "context" + "fmt" + "testing" + + "github.com/stackitcloud/stackit-cli/internal/pkg/globalflags" + "github.com/stackitcloud/stackit-cli/internal/pkg/utils" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/google/uuid" + "github.com/stackitcloud/stackit-sdk-go/services/argus" +) + +var projectIdFlag = globalflags.ProjectIdFlag + +type testCtxKey struct{} + +var testCtx = context.WithValue(context.Background(), testCtxKey{}, "foo") +var testClient = &argus.APIClient{} + +type argusClientMocked struct { + listPlansError bool + listPlansResponse *argus.PlansResponse + getInstanceError bool + getInstanceResponse *argus.GetInstanceResponse +} + +func (c *argusClientMocked) UpdateInstance(ctx context.Context, instanceId, projectId string) argus.ApiUpdateInstanceRequest { + return testClient.UpdateInstance(ctx, instanceId, projectId) +} + +func (c *argusClientMocked) ListPlansExecute(_ context.Context, _ string) (*argus.PlansResponse, error) { + if c.listPlansError { + return nil, fmt.Errorf("list flavors failed") + } + return c.listPlansResponse, nil +} + +func (c *argusClientMocked) GetInstanceExecute(ctx context.Context, instanceId, projectId string) (*argus.GetInstanceResponse, error) { + if c.getInstanceError { + return nil, fmt.Errorf("get instance failed") + } + return c.getInstanceResponse, nil +} + +var ( + testProjectId = uuid.NewString() + testInstanceId = uuid.NewString() + testPlanId = uuid.NewString() +) + +func fixtureArgValues(mods ...func(argValues []string)) []string { + argValues := []string{ + testInstanceId, + } + for _, mod := range mods { + mod(argValues) + } + return argValues +} + +func fixtureFlagValues(mods ...func(flagValues map[string]string)) map[string]string { + flagValues := map[string]string{ + projectIdFlag: testProjectId, + planIdFlag: testPlanId, + } + for _, mod := range mods { + mod(flagValues) + } + return flagValues +} + +func fixtureInputModel(mods ...func(model *inputModel)) *inputModel { + model := &inputModel{ + GlobalFlagModel: &globalflags.GlobalFlagModel{ + ProjectId: testProjectId, + }, + InstanceId: testInstanceId, + PlanId: utils.Ptr(testPlanId), + } + for _, mod := range mods { + mod(model) + } + return model +} + +func fixtureRequest(mods ...func(request *argus.ApiUpdateInstanceRequest)) argus.ApiUpdateInstanceRequest { + request := testClient.UpdateInstance(testCtx, testInstanceId, testProjectId) + request = request.UpdateInstancePayload(argus.UpdateInstancePayload{ + PlanId: utils.Ptr(testPlanId), + }) + for _, mod := range mods { + mod(&request) + } + return request +} + +func fixturePlansResponse(mods ...func(response *argus.PlansResponse)) *argus.PlansResponse { + response := &argus.PlansResponse{ + Plans: &[]argus.Plan{ + { + Name: utils.Ptr("example-plan-name"), + Id: utils.Ptr(testPlanId), + }, + }, + } + for _, mod := range mods { + mod(response) + } + return response +} + +func fixtureGetInstanceResponse(mods ...func(response *argus.GetInstanceResponse)) *argus.GetInstanceResponse { + response := &argus.GetInstanceResponse{ + PlanId: utils.Ptr(testPlanId), + Name: utils.Ptr("example-instance-name"), + } + for _, mod := range mods { + mod(response) + } + return response +} + +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: "with plan name", + argValues: fixtureArgValues(), + flagValues: fixtureFlagValues(func(flagValues map[string]string) { + flagValues[planNameFlag] = "plan-name" + delete(flagValues, planIdFlag) + }), + isValid: true, + expectedModel: fixtureInputModel(func(model *inputModel) { + model.PlanId = nil + model.PlanName = "plan-name" + }), + }, + { + description: "with new instance name", + argValues: fixtureArgValues(), + flagValues: fixtureFlagValues(func(flagValues map[string]string) { + flagValues[instanceNameFlag] = "new-instance-name" + delete(flagValues, planIdFlag) + }), + isValid: true, + expectedModel: fixtureInputModel(func(model *inputModel) { + model.PlanId = nil + model.PlanName = "" + model.InstanceName = utils.Ptr("new-instance-name") + }), + }, + { + description: "no values", + argValues: []string{}, + flagValues: map[string]string{}, + isValid: false, + }, + { + description: "no arg values", + argValues: []string{}, + flagValues: fixtureFlagValues(), + isValid: false, + }, + { + description: "no flag values", + argValues: fixtureArgValues(), + flagValues: map[string]string{}, + isValid: false, + }, + { + description: "project id missing", + argValues: fixtureArgValues(), + flagValues: fixtureFlagValues(func(flagValues map[string]string) { + delete(flagValues, projectIdFlag) + }), + isValid: false, + }, + { + description: "project id invalid 1", + argValues: fixtureArgValues(), + flagValues: fixtureFlagValues(func(flagValues map[string]string) { + flagValues[projectIdFlag] = "" + }), + isValid: false, + }, + { + description: "project id invalid 2", + argValues: fixtureArgValues(), + flagValues: fixtureFlagValues(func(flagValues map[string]string) { + flagValues[projectIdFlag] = "invalid-uuid" + }), + isValid: false, + }, + { + description: "instance id invalid 1", + argValues: []string{""}, + flagValues: fixtureFlagValues(), + isValid: false, + }, + { + description: "instance id invalid 2", + argValues: []string{"invalid-uuid"}, + flagValues: fixtureFlagValues(), + isValid: false, + }, + { + description: "invalid with plan ID and plan name", + flagValues: fixtureFlagValues(func(flagValues map[string]string) { + flagValues[planNameFlag] = "plan-name" + }), + isValid: false, + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + cmd := NewCmd() + err := globalflags.Configure(cmd.Flags()) + if err != nil { + t.Fatalf("configure global flags: %v", err) + } + + for flag, value := range tt.flagValues { + err := cmd.Flags().Set(flag, value) + if err != nil { + if !tt.isValid { + return + } + t.Fatalf("setting flag --%s=%s: %v", flag, value, err) + } + } + + err = cmd.ValidateArgs(tt.argValues) + if err != nil { + if !tt.isValid { + return + } + t.Fatalf("error validating args: %v", err) + } + + err = cmd.ValidateRequiredFlags() + if err != nil { + if !tt.isValid { + return + } + t.Fatalf("error validating flags: %v", err) + } + + model, err := parseInput(cmd, tt.argValues) + if err != nil { + if !tt.isValid { + return + } + t.Fatalf("error parsing flags: %v", err) + } + + if !tt.isValid { + t.Fatalf("did not fail on invalid input") + } + diff := cmp.Diff(model, tt.expectedModel) + if diff != "" { + t.Fatalf("Data does not match: %s", diff) + } + }) + } +} + +func TestBuildRequest(t *testing.T) { + tests := []struct { + description string + model *inputModel + expectedRequest argus.ApiUpdateInstanceRequest + getPlansFails bool + getPlansResponse *argus.PlansResponse + getInstanceFails bool + getInstanceResponse *argus.GetInstanceResponse + isValid bool + }{ + { + description: "base", + model: fixtureInputModel(), + expectedRequest: fixtureRequest(), + getPlansResponse: fixturePlansResponse(), + isValid: true, + }, + { + description: "use plan name", + model: fixtureInputModel( + func(model *inputModel) { + model.PlanId = nil + model.PlanName = "example-plan-name" + }, + ), + expectedRequest: fixtureRequest(), + getPlansResponse: fixturePlansResponse(), + isValid: true, + }, + { + description: "get plans fails", + model: fixtureInputModel( + func(model *inputModel) { + model.PlanId = nil + model.PlanName = "example-plan-name" + }, + ), + getPlansFails: true, + isValid: false, + }, + { + description: "plan name not found", + model: fixtureInputModel( + func(model *inputModel) { + model.PlanId = nil + model.PlanName = "non-existent-plan" + }, + ), + getPlansResponse: fixturePlansResponse(), + isValid: false, + }, + { + description: "plan id not found", + model: fixtureInputModel( + func(model *inputModel) { + model.PlanId = utils.Ptr(uuid.NewString()) + }, + ), + getPlansResponse: fixturePlansResponse(), + isValid: false, + }, + { + description: "plan id, no instance name", + model: fixtureInputModel( + func(model *inputModel) { + model.InstanceName = nil + }, + ), + getPlansResponse: fixturePlansResponse(), + expectedRequest: testClient.UpdateInstance(testCtx, testInstanceId, testProjectId). + UpdateInstancePayload(argus.UpdateInstancePayload{PlanId: utils.Ptr(testPlanId)}), + isValid: true, + }, + { + description: "plan name, no instance name", + model: fixtureInputModel( + func(model *inputModel) { + model.PlanId = nil + model.PlanName = "example-plan-name" + model.InstanceName = nil + }, + ), + getPlansResponse: fixturePlansResponse(), + expectedRequest: testClient.UpdateInstance(testCtx, testInstanceId, testProjectId). + UpdateInstancePayload(argus.UpdateInstancePayload{PlanId: utils.Ptr(testPlanId)}), + isValid: true, + }, + { + description: "instance name, no plan info", + model: fixtureInputModel( + func(model *inputModel) { + model.PlanId = nil + model.PlanName = "" + model.InstanceName = utils.Ptr("new-instance-name") + }, + ), + getInstanceResponse: fixtureGetInstanceResponse(), + expectedRequest: testClient.UpdateInstance(testCtx, testInstanceId, testProjectId). + UpdateInstancePayload(argus.UpdateInstancePayload{ + PlanId: utils.Ptr(testPlanId), + Name: utils.Ptr("new-instance-name"), + }), + isValid: true, + }, + { + description: "instance name, no plan info, get instance fails", + model: fixtureInputModel( + func(model *inputModel) { + model.PlanId = nil + model.PlanName = "" + model.InstanceName = utils.Ptr("new-instance-name") + }, + ), + getInstanceFails: true, + isValid: false, + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + client := &argusClientMocked{ + listPlansError: tt.getPlansFails, + listPlansResponse: tt.getPlansResponse, + getInstanceError: tt.getInstanceFails, + getInstanceResponse: tt.getInstanceResponse, + } + request, err := buildRequest(testCtx, tt.model, client) + if err != nil { + if !tt.isValid { + return + } + t.Fatalf("error building request: %v", err) + } + + if !tt.isValid { + t.Fatal("expected error but none thrown") + } + + diff := cmp.Diff(request, tt.expectedRequest, + cmp.AllowUnexported(tt.expectedRequest), + cmpopts.EquateComparable(testCtx), + ) + if diff != "" { + t.Fatalf("Data does not match: %s", diff) + } + }) + } +} From 14e97da00e3c695dec714b20ec28f242a595e6bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 11:21:00 +0000 Subject: [PATCH 04/17] fix linting --- .../cmd/argus/instance/create/create_test.go | 30 +++++++++---------- internal/cmd/argus/instance/update/update.go | 6 ++-- .../cmd/argus/instance/update/update_test.go | 12 ++++---- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/internal/cmd/argus/instance/create/create_test.go b/internal/cmd/argus/instance/create/create_test.go index bab8ad792..e75827f86 100644 --- a/internal/cmd/argus/instance/create/create_test.go +++ b/internal/cmd/argus/instance/create/create_test.go @@ -22,7 +22,7 @@ var testCtx = context.WithValue(context.Background(), testCtxKey{}, "foo") var testClient = &argus.APIClient{} type argusClientMocked struct { - returnError bool + returnError bool listPlansResponse *argus.PlansResponse } @@ -216,19 +216,19 @@ func TestParseInput(t *testing.T) { func TestBuildRequest(t *testing.T) { tests := []struct { - description string - model *inputModel - expectedRequest argus.ApiCreateInstanceRequest - getPlansFails bool + description string + model *inputModel + expectedRequest argus.ApiCreateInstanceRequest + getPlansFails bool getPlansResponse *argus.PlansResponse - isValid bool + isValid bool }{ { - description: "base", - model: fixtureInputModel(), - expectedRequest: fixtureRequest(), + description: "base", + model: fixtureInputModel(), + expectedRequest: fixtureRequest(), getPlansResponse: fixturePlansResponse(), - isValid: true, + isValid: true, }, { description: "use plan name", @@ -238,9 +238,9 @@ func TestBuildRequest(t *testing.T) { model.PlanName = "example-plan-name" }, ), - expectedRequest: fixtureRequest(), + expectedRequest: fixtureRequest(), getPlansResponse: fixturePlansResponse(), - isValid: true, + isValid: true, }, { description: "get plans fails", @@ -262,7 +262,7 @@ func TestBuildRequest(t *testing.T) { }, ), getPlansResponse: fixturePlansResponse(), - isValid: false, + isValid: false, }, { description: "plan id not found", @@ -272,7 +272,7 @@ func TestBuildRequest(t *testing.T) { }, ), getPlansResponse: fixturePlansResponse(), - isValid: false, + isValid: false, }, { description: "plan id, no instance name", @@ -305,7 +305,7 @@ func TestBuildRequest(t *testing.T) { for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { client := &argusClientMocked{ - returnError: tt.getPlansFails, + returnError: tt.getPlansFails, listPlansResponse: tt.getPlansResponse, } request, err := buildRequest(testCtx, tt.model, client) diff --git a/internal/cmd/argus/instance/update/update.go b/internal/cmd/argus/instance/update/update.go index 3f66aeac6..6c2c831c3 100644 --- a/internal/cmd/argus/instance/update/update.go +++ b/internal/cmd/argus/instance/update/update.go @@ -31,11 +31,11 @@ const ( type inputModel struct { *globalflags.GlobalFlagModel - InstanceId string - PlanName string + InstanceId string + PlanName string InstanceName *string - PlanId *string + PlanId *string } func NewCmd() *cobra.Command { diff --git a/internal/cmd/argus/instance/update/update_test.go b/internal/cmd/argus/instance/update/update_test.go index 7746d7cc6..b6afb2d55 100644 --- a/internal/cmd/argus/instance/update/update_test.go +++ b/internal/cmd/argus/instance/update/update_test.go @@ -39,7 +39,7 @@ func (c *argusClientMocked) ListPlansExecute(_ context.Context, _ string) (*argu return c.listPlansResponse, nil } -func (c *argusClientMocked) GetInstanceExecute(ctx context.Context, instanceId, projectId string) (*argus.GetInstanceResponse, error) { +func (c *argusClientMocked) GetInstanceExecute(_ context.Context, _, _ string) (*argus.GetInstanceResponse, error) { if c.getInstanceError { return nil, fmt.Errorf("get instance failed") } @@ -116,7 +116,7 @@ func fixturePlansResponse(mods ...func(response *argus.PlansResponse)) *argus.Pl func fixtureGetInstanceResponse(mods ...func(response *argus.GetInstanceResponse)) *argus.GetInstanceResponse { response := &argus.GetInstanceResponse{ PlanId: utils.Ptr(testPlanId), - Name: utils.Ptr("example-instance-name"), + Name: utils.Ptr("example-instance-name"), } for _, mod := range mods { mod(response) @@ -397,16 +397,16 @@ func TestBuildRequest(t *testing.T) { }, ), getInstanceFails: true, - isValid: false, + isValid: false, }, } for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { client := &argusClientMocked{ - listPlansError: tt.getPlansFails, - listPlansResponse: tt.getPlansResponse, - getInstanceError: tt.getInstanceFails, + listPlansError: tt.getPlansFails, + listPlansResponse: tt.getPlansResponse, + getInstanceError: tt.getInstanceFails, getInstanceResponse: tt.getInstanceResponse, } request, err := buildRequest(testCtx, tt.model, client) From 8045da73a39af4b4c721d03e90e2accd28e6a5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 11:35:50 +0000 Subject: [PATCH 05/17] update command: handle empty instance name --- internal/cmd/argus/instance/update/update.go | 8 ++++++-- internal/cmd/argus/instance/update/update_test.go | 9 +++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/internal/cmd/argus/instance/update/update.go b/internal/cmd/argus/instance/update/update.go index 6c2c831c3..3c3e5ad48 100644 --- a/internal/cmd/argus/instance/update/update.go +++ b/internal/cmd/argus/instance/update/update.go @@ -144,8 +144,12 @@ func parseInput(cmd *cobra.Command, inputArgs []string) (*inputModel, error) { } } - if planId == nil && planName == "" && instanceName == nil { - return nil, &cliErr.EmptyUpdateError{} + if planId == nil && planName == "" { + if instanceName == nil { + return nil, &cliErr.EmptyUpdateError{} + } else if *instanceName == "" { + return nil, fmt.Errorf("new instance name cannot be empty.") + } } return &inputModel{ diff --git a/internal/cmd/argus/instance/update/update_test.go b/internal/cmd/argus/instance/update/update_test.go index b6afb2d55..0dc38d81a 100644 --- a/internal/cmd/argus/instance/update/update_test.go +++ b/internal/cmd/argus/instance/update/update_test.go @@ -166,6 +166,15 @@ func TestParseInput(t *testing.T) { model.InstanceName = utils.Ptr("new-instance-name") }), }, + { + description: "with empty new instance name", + argValues: fixtureArgValues(), + flagValues: fixtureFlagValues(func(flagValues map[string]string) { + flagValues[instanceNameFlag] = "" + delete(flagValues, planIdFlag) + }), + isValid: false, + }, { description: "no values", argValues: []string{}, From fca58b5d159c1a32fde171f58f439acc04f003a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 14:42:00 +0000 Subject: [PATCH 06/17] update command: handle empty name update and corner cases --- internal/cmd/argus/instance/update/update.go | 33 +++++++++++-------- .../cmd/argus/instance/update/update_test.go | 21 ++++++++---- internal/pkg/services/argus/utils/utils.go | 9 +++++ 3 files changed, 43 insertions(+), 20 deletions(-) diff --git a/internal/cmd/argus/instance/update/update.go b/internal/cmd/argus/instance/update/update.go index 3c3e5ad48..ffd94b8f8 100644 --- a/internal/cmd/argus/instance/update/update.go +++ b/internal/cmd/argus/instance/update/update.go @@ -69,7 +69,7 @@ func NewCmd() *cobra.Command { } instanceLabel, err := argusUtils.GetInstanceName(ctx, apiClient, model.InstanceId, model.ProjectId) - if err != nil { + if err != nil || instanceLabel == "" { instanceLabel = model.InstanceId } @@ -170,7 +170,6 @@ type argusClient interface { func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) (argus.ApiUpdateInstanceRequest, error) { req := apiClient.UpdateInstance(ctx, model.InstanceId, model.ProjectId) - var planId *string var err error plans, err := apiClient.ListPlansExecute(ctx, model.ProjectId) @@ -178,8 +177,18 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) return req, fmt.Errorf("get Argus plans: %w", err) } + currInstanceName, currPlanId, err := argusUtils.GetInstanceDetails(ctx, apiClient, model.InstanceId, model.ProjectId) + if err != nil { + return req, fmt.Errorf("get Argus instance: %w", err) + } + + defaultPayload := argus.UpdateInstancePayload{ + PlanId: currPlanId, + Name: currInstanceName, + } + if model.PlanId == nil && model.PlanName != "" { - planId, err = argusUtils.LoadPlanId(model.PlanName, plans) + defaultPayload.PlanId, err = argusUtils.LoadPlanId(model.PlanName, plans) if err != nil { var argusInvalidPlanError *cliErr.ArgusInvalidPlanError if !errors.As(err, &argusInvalidPlanError) { @@ -187,24 +196,20 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) } return req, err } - } else if model.PlanId == nil && model.PlanName == "" { - planId, err = argusUtils.GetInstancePlanId(ctx, apiClient, model.InstanceId, model.ProjectId) - if err != nil { - return req, fmt.Errorf("get Argus instance plan ID: %w", err) - } - } else { + } else if model.PlanId != nil && model.PlanName == "" { if model.PlanId != nil { err := argusUtils.ValidatePlanId(*model.PlanId, plans) if err != nil { return req, err } } - planId = model.PlanId + defaultPayload.PlanId = model.PlanId + } + + if model.InstanceName != nil { + defaultPayload.Name = model.InstanceName } - req = req.UpdateInstancePayload(argus.UpdateInstancePayload{ - PlanId: planId, - Name: model.InstanceName, - }) + req = req.UpdateInstancePayload(defaultPayload) return req, nil } diff --git a/internal/cmd/argus/instance/update/update_test.go b/internal/cmd/argus/instance/update/update_test.go index 0dc38d81a..8d3ba78ea 100644 --- a/internal/cmd/argus/instance/update/update_test.go +++ b/internal/cmd/argus/instance/update/update_test.go @@ -46,6 +46,10 @@ func (c *argusClientMocked) GetInstanceExecute(_ context.Context, _, _ string) ( return c.getInstanceResponse, nil } +const ( + testInstanceName = "example-instance-name" +) + var ( testProjectId = uuid.NewString() testInstanceId = uuid.NewString() @@ -91,6 +95,7 @@ func fixtureRequest(mods ...func(request *argus.ApiUpdateInstanceRequest)) argus request := testClient.UpdateInstance(testCtx, testInstanceId, testProjectId) request = request.UpdateInstancePayload(argus.UpdateInstancePayload{ PlanId: utils.Ptr(testPlanId), + Name: utils.Ptr(testInstanceName), }) for _, mod := range mods { mod(&request) @@ -116,7 +121,7 @@ func fixturePlansResponse(mods ...func(response *argus.PlansResponse)) *argus.Pl func fixtureGetInstanceResponse(mods ...func(response *argus.GetInstanceResponse)) *argus.GetInstanceResponse { response := &argus.GetInstanceResponse{ PlanId: utils.Ptr(testPlanId), - Name: utils.Ptr("example-instance-name"), + Name: utils.Ptr(testInstanceName), } for _, mod := range mods { mod(response) @@ -307,6 +312,7 @@ func TestBuildRequest(t *testing.T) { model: fixtureInputModel(), expectedRequest: fixtureRequest(), getPlansResponse: fixturePlansResponse(), + getInstanceResponse: fixtureGetInstanceResponse(), isValid: true, }, { @@ -319,6 +325,7 @@ func TestBuildRequest(t *testing.T) { ), expectedRequest: fixtureRequest(), getPlansResponse: fixturePlansResponse(), + getInstanceResponse: fixtureGetInstanceResponse(), isValid: true, }, { @@ -341,6 +348,7 @@ func TestBuildRequest(t *testing.T) { }, ), getPlansResponse: fixturePlansResponse(), + getInstanceResponse: fixtureGetInstanceResponse(), isValid: false, }, { @@ -351,6 +359,7 @@ func TestBuildRequest(t *testing.T) { }, ), getPlansResponse: fixturePlansResponse(), + getInstanceResponse: fixtureGetInstanceResponse(), isValid: false, }, { @@ -361,8 +370,8 @@ func TestBuildRequest(t *testing.T) { }, ), getPlansResponse: fixturePlansResponse(), - expectedRequest: testClient.UpdateInstance(testCtx, testInstanceId, testProjectId). - UpdateInstancePayload(argus.UpdateInstancePayload{PlanId: utils.Ptr(testPlanId)}), + getInstanceResponse: fixtureGetInstanceResponse(), + expectedRequest: fixtureRequest(), isValid: true, }, { @@ -375,8 +384,8 @@ func TestBuildRequest(t *testing.T) { }, ), getPlansResponse: fixturePlansResponse(), - expectedRequest: testClient.UpdateInstance(testCtx, testInstanceId, testProjectId). - UpdateInstancePayload(argus.UpdateInstancePayload{PlanId: utils.Ptr(testPlanId)}), + getInstanceResponse: fixtureGetInstanceResponse(), + expectedRequest: fixtureRequest(), isValid: true, }, { @@ -389,7 +398,7 @@ func TestBuildRequest(t *testing.T) { }, ), getInstanceResponse: fixtureGetInstanceResponse(), - expectedRequest: testClient.UpdateInstance(testCtx, testInstanceId, testProjectId). + expectedRequest: fixtureRequest(). UpdateInstancePayload(argus.UpdateInstancePayload{ PlanId: utils.Ptr(testPlanId), Name: utils.Ptr("new-instance-name"), diff --git a/internal/pkg/services/argus/utils/utils.go b/internal/pkg/services/argus/utils/utils.go index fa028b532..0b4d1333d 100644 --- a/internal/pkg/services/argus/utils/utils.go +++ b/internal/pkg/services/argus/utils/utils.go @@ -75,3 +75,12 @@ func GetInstancePlanId(ctx context.Context, apiClient ArgusClient, instanceId, p } return resp.PlanId, nil } + + +func GetInstanceDetails(ctx context.Context, apiClient ArgusClient, instanceId, projectId string) (name, planId *string, err error) { + resp, err := apiClient.GetInstanceExecute(ctx, instanceId, projectId) + if err != nil { + return nil, nil, fmt.Errorf("get Argus instance: %w", err) + } + return resp.Name, resp.PlanId, nil +} From c8f7f6261a2f1c6c951839b1be68edac3981ce03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 14:44:52 +0000 Subject: [PATCH 07/17] fix linting --- internal/cmd/argus/instance/update/update.go | 20 +++++----- .../cmd/argus/instance/update/update_test.go | 38 +++++++++---------- internal/pkg/services/argus/utils/utils.go | 1 - 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/internal/cmd/argus/instance/update/update.go b/internal/cmd/argus/instance/update/update.go index ffd94b8f8..1ad8558fb 100644 --- a/internal/cmd/argus/instance/update/update.go +++ b/internal/cmd/argus/instance/update/update.go @@ -182,13 +182,13 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) return req, fmt.Errorf("get Argus instance: %w", err) } - defaultPayload := argus.UpdateInstancePayload{ + payload := argus.UpdateInstancePayload{ PlanId: currPlanId, Name: currInstanceName, } if model.PlanId == nil && model.PlanName != "" { - defaultPayload.PlanId, err = argusUtils.LoadPlanId(model.PlanName, plans) + payload.PlanId, err = argusUtils.LoadPlanId(model.PlanName, plans) if err != nil { var argusInvalidPlanError *cliErr.ArgusInvalidPlanError if !errors.As(err, &argusInvalidPlanError) { @@ -197,19 +197,21 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) return req, err } } else if model.PlanId != nil && model.PlanName == "" { - if model.PlanId != nil { - err := argusUtils.ValidatePlanId(*model.PlanId, plans) - if err != nil { - return req, err + err := argusUtils.ValidatePlanId(*model.PlanId, plans) + if err != nil { + var argusInvalidPlanError *cliErr.ArgusInvalidPlanError + if !errors.As(err, &argusInvalidPlanError) { + return req, fmt.Errorf("load plan ID: %w", err) } + return req, err } - defaultPayload.PlanId = model.PlanId + payload.PlanId = model.PlanId } if model.InstanceName != nil { - defaultPayload.Name = model.InstanceName + payload.Name = model.InstanceName } - req = req.UpdateInstancePayload(defaultPayload) + req = req.UpdateInstancePayload(payload) return req, nil } diff --git a/internal/cmd/argus/instance/update/update_test.go b/internal/cmd/argus/instance/update/update_test.go index 8d3ba78ea..f93bb31f5 100644 --- a/internal/cmd/argus/instance/update/update_test.go +++ b/internal/cmd/argus/instance/update/update_test.go @@ -95,7 +95,7 @@ func fixtureRequest(mods ...func(request *argus.ApiUpdateInstanceRequest)) argus request := testClient.UpdateInstance(testCtx, testInstanceId, testProjectId) request = request.UpdateInstancePayload(argus.UpdateInstancePayload{ PlanId: utils.Ptr(testPlanId), - Name: utils.Ptr(testInstanceName), + Name: utils.Ptr(testInstanceName), }) for _, mod := range mods { mod(&request) @@ -308,12 +308,12 @@ func TestBuildRequest(t *testing.T) { isValid bool }{ { - description: "base", - model: fixtureInputModel(), - expectedRequest: fixtureRequest(), - getPlansResponse: fixturePlansResponse(), + description: "base", + model: fixtureInputModel(), + expectedRequest: fixtureRequest(), + getPlansResponse: fixturePlansResponse(), getInstanceResponse: fixtureGetInstanceResponse(), - isValid: true, + isValid: true, }, { description: "use plan name", @@ -323,10 +323,10 @@ func TestBuildRequest(t *testing.T) { model.PlanName = "example-plan-name" }, ), - expectedRequest: fixtureRequest(), - getPlansResponse: fixturePlansResponse(), + expectedRequest: fixtureRequest(), + getPlansResponse: fixturePlansResponse(), getInstanceResponse: fixtureGetInstanceResponse(), - isValid: true, + isValid: true, }, { description: "get plans fails", @@ -347,9 +347,9 @@ func TestBuildRequest(t *testing.T) { model.PlanName = "non-existent-plan" }, ), - getPlansResponse: fixturePlansResponse(), + getPlansResponse: fixturePlansResponse(), getInstanceResponse: fixtureGetInstanceResponse(), - isValid: false, + isValid: false, }, { description: "plan id not found", @@ -358,9 +358,9 @@ func TestBuildRequest(t *testing.T) { model.PlanId = utils.Ptr(uuid.NewString()) }, ), - getPlansResponse: fixturePlansResponse(), + getPlansResponse: fixturePlansResponse(), getInstanceResponse: fixtureGetInstanceResponse(), - isValid: false, + isValid: false, }, { description: "plan id, no instance name", @@ -369,10 +369,10 @@ func TestBuildRequest(t *testing.T) { model.InstanceName = nil }, ), - getPlansResponse: fixturePlansResponse(), + getPlansResponse: fixturePlansResponse(), getInstanceResponse: fixtureGetInstanceResponse(), - expectedRequest: fixtureRequest(), - isValid: true, + expectedRequest: fixtureRequest(), + isValid: true, }, { description: "plan name, no instance name", @@ -383,10 +383,10 @@ func TestBuildRequest(t *testing.T) { model.InstanceName = nil }, ), - getPlansResponse: fixturePlansResponse(), + getPlansResponse: fixturePlansResponse(), getInstanceResponse: fixtureGetInstanceResponse(), - expectedRequest: fixtureRequest(), - isValid: true, + expectedRequest: fixtureRequest(), + isValid: true, }, { description: "instance name, no plan info", diff --git a/internal/pkg/services/argus/utils/utils.go b/internal/pkg/services/argus/utils/utils.go index 0b4d1333d..502e93e35 100644 --- a/internal/pkg/services/argus/utils/utils.go +++ b/internal/pkg/services/argus/utils/utils.go @@ -76,7 +76,6 @@ func GetInstancePlanId(ctx context.Context, apiClient ArgusClient, instanceId, p return resp.PlanId, nil } - func GetInstanceDetails(ctx context.Context, apiClient ArgusClient, instanceId, projectId string) (name, planId *string, err error) { resp, err := apiClient.GetInstanceExecute(ctx, instanceId, projectId) if err != nil { From 60a727a3e1f080f555a9a753c64b08d1b5b17f64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 14:46:52 +0000 Subject: [PATCH 08/17] update command: improve error message --- internal/cmd/argus/instance/update/update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cmd/argus/instance/update/update.go b/internal/cmd/argus/instance/update/update.go index 1ad8558fb..acbf33019 100644 --- a/internal/cmd/argus/instance/update/update.go +++ b/internal/cmd/argus/instance/update/update.go @@ -201,7 +201,7 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) if err != nil { var argusInvalidPlanError *cliErr.ArgusInvalidPlanError if !errors.As(err, &argusInvalidPlanError) { - return req, fmt.Errorf("load plan ID: %w", err) + return req, fmt.Errorf("validate plan ID: %w", err) } return req, err } From a48acfb76bc7c4bd792dedc7d93e59d5c62a8357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 15:48:53 +0000 Subject: [PATCH 09/17] address comments in PR --- internal/cmd/argus/instance/update/update.go | 8 ++-- .../cmd/argus/instance/update/update_test.go | 18 ++----- internal/pkg/services/argus/utils/utils.go | 16 ------- .../pkg/services/argus/utils/utils_test.go | 48 ------------------- 4 files changed, 8 insertions(+), 82 deletions(-) diff --git a/internal/cmd/argus/instance/update/update.go b/internal/cmd/argus/instance/update/update.go index acbf33019..83802e416 100644 --- a/internal/cmd/argus/instance/update/update.go +++ b/internal/cmd/argus/instance/update/update.go @@ -147,8 +147,6 @@ func parseInput(cmd *cobra.Command, inputArgs []string) (*inputModel, error) { if planId == nil && planName == "" { if instanceName == nil { return nil, &cliErr.EmptyUpdateError{} - } else if *instanceName == "" { - return nil, fmt.Errorf("new instance name cannot be empty.") } } @@ -177,14 +175,14 @@ func buildRequest(ctx context.Context, model *inputModel, apiClient argusClient) return req, fmt.Errorf("get Argus plans: %w", err) } - currInstanceName, currPlanId, err := argusUtils.GetInstanceDetails(ctx, apiClient, model.InstanceId, model.ProjectId) + currentInstance, err := apiClient.GetInstanceExecute(ctx, model.InstanceId, model.ProjectId) if err != nil { return req, fmt.Errorf("get Argus instance: %w", err) } payload := argus.UpdateInstancePayload{ - PlanId: currPlanId, - Name: currInstanceName, + PlanId: currentInstance.PlanId, + Name: currentInstance.Name, } if model.PlanId == nil && model.PlanName != "" { diff --git a/internal/cmd/argus/instance/update/update_test.go b/internal/cmd/argus/instance/update/update_test.go index f93bb31f5..5a2ab1d33 100644 --- a/internal/cmd/argus/instance/update/update_test.go +++ b/internal/cmd/argus/instance/update/update_test.go @@ -54,6 +54,7 @@ var ( testProjectId = uuid.NewString() testInstanceId = uuid.NewString() testPlanId = uuid.NewString() + testNewPlanId = uuid.NewString() ) func fixtureArgValues(mods ...func(argValues []string)) []string { @@ -69,7 +70,7 @@ func fixtureArgValues(mods ...func(argValues []string)) []string { func fixtureFlagValues(mods ...func(flagValues map[string]string)) map[string]string { flagValues := map[string]string{ projectIdFlag: testProjectId, - planIdFlag: testPlanId, + planIdFlag: testNewPlanId, } for _, mod := range mods { mod(flagValues) @@ -83,7 +84,7 @@ func fixtureInputModel(mods ...func(model *inputModel)) *inputModel { ProjectId: testProjectId, }, InstanceId: testInstanceId, - PlanId: utils.Ptr(testPlanId), + PlanId: utils.Ptr(testNewPlanId), } for _, mod := range mods { mod(model) @@ -94,7 +95,7 @@ func fixtureInputModel(mods ...func(model *inputModel)) *inputModel { func fixtureRequest(mods ...func(request *argus.ApiUpdateInstanceRequest)) argus.ApiUpdateInstanceRequest { request := testClient.UpdateInstance(testCtx, testInstanceId, testProjectId) request = request.UpdateInstancePayload(argus.UpdateInstancePayload{ - PlanId: utils.Ptr(testPlanId), + PlanId: utils.Ptr(testNewPlanId), Name: utils.Ptr(testInstanceName), }) for _, mod := range mods { @@ -108,7 +109,7 @@ func fixturePlansResponse(mods ...func(response *argus.PlansResponse)) *argus.Pl Plans: &[]argus.Plan{ { Name: utils.Ptr("example-plan-name"), - Id: utils.Ptr(testPlanId), + Id: utils.Ptr(testNewPlanId), }, }, } @@ -171,15 +172,6 @@ func TestParseInput(t *testing.T) { model.InstanceName = utils.Ptr("new-instance-name") }), }, - { - description: "with empty new instance name", - argValues: fixtureArgValues(), - flagValues: fixtureFlagValues(func(flagValues map[string]string) { - flagValues[instanceNameFlag] = "" - delete(flagValues, planIdFlag) - }), - isValid: false, - }, { description: "no values", argValues: []string{}, diff --git a/internal/pkg/services/argus/utils/utils.go b/internal/pkg/services/argus/utils/utils.go index 502e93e35..e530c187b 100644 --- a/internal/pkg/services/argus/utils/utils.go +++ b/internal/pkg/services/argus/utils/utils.go @@ -67,19 +67,3 @@ func GetInstanceName(ctx context.Context, apiClient ArgusClient, instanceId, pro } return *resp.Name, nil } - -func GetInstancePlanId(ctx context.Context, apiClient ArgusClient, instanceId, projectId string) (*string, error) { - resp, err := apiClient.GetInstanceExecute(ctx, instanceId, projectId) - if err != nil { - return nil, fmt.Errorf("get Argus instance: %w", err) - } - return resp.PlanId, nil -} - -func GetInstanceDetails(ctx context.Context, apiClient ArgusClient, instanceId, projectId string) (name, planId *string, err error) { - resp, err := apiClient.GetInstanceExecute(ctx, instanceId, projectId) - if err != nil { - return nil, nil, fmt.Errorf("get Argus instance: %w", err) - } - return resp.Name, resp.PlanId, nil -} diff --git a/internal/pkg/services/argus/utils/utils_test.go b/internal/pkg/services/argus/utils/utils_test.go index 6ca572aa9..8adf5de1b 100644 --- a/internal/pkg/services/argus/utils/utils_test.go +++ b/internal/pkg/services/argus/utils/utils_test.go @@ -91,54 +91,6 @@ func TestGetInstanceName(t *testing.T) { } } -func TestGetInstancePlanId(t *testing.T) { - tests := []struct { - description string - getInstanceFails bool - getInstanceResp *argus.GetInstanceResponse - isValid bool - expectedOutput string - }{ - { - description: "base", - getInstanceResp: &argus.GetInstanceResponse{ - PlanId: utils.Ptr(testPlanId), - }, - isValid: true, - expectedOutput: testPlanId, - }, - { - description: "get instance fails", - getInstanceFails: true, - isValid: false, - }, - } - - for _, tt := range tests { - t.Run(tt.description, func(t *testing.T) { - client := &argusClientMocked{ - getInstanceFails: tt.getInstanceFails, - getInstanceResp: tt.getInstanceResp, - } - - output, err := GetInstancePlanId(context.Background(), client, testInstanceId, testProjectId) - - if tt.isValid && err != nil { - t.Errorf("failed on valid input") - } - if !tt.isValid && err == nil { - t.Errorf("did not fail on invalid input") - } - if !tt.isValid { - return - } - if *output != tt.expectedOutput { - t.Errorf("expected output to be %s, got %s", tt.expectedOutput, *output) - } - }) - } -} - func TestLoadPlanId(t *testing.T) { tests := []struct { description string From 3ac44138622db39b81cb67091dece31cf090c84a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 16:52:03 +0000 Subject: [PATCH 10/17] Update internal/cmd/argus/instance/update/update.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: João Palet --- internal/cmd/argus/instance/update/update.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/cmd/argus/instance/update/update.go b/internal/cmd/argus/instance/update/update.go index 83802e416..36c27b6f5 100644 --- a/internal/cmd/argus/instance/update/update.go +++ b/internal/cmd/argus/instance/update/update.go @@ -144,10 +144,8 @@ func parseInput(cmd *cobra.Command, inputArgs []string) (*inputModel, error) { } } - if planId == nil && planName == "" { - if instanceName == nil { - return nil, &cliErr.EmptyUpdateError{} - } + if planId == nil && planName == "" && instanceName == nil { + return nil, &cliErr.EmptyUpdateError{} } return &inputModel{ From db52518b767bda0a0817204721706611e4be1a15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 16:55:53 +0000 Subject: [PATCH 11/17] fix linting --- internal/cmd/argus/instance/update/update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cmd/argus/instance/update/update.go b/internal/cmd/argus/instance/update/update.go index 36c27b6f5..3bd4cd774 100644 --- a/internal/cmd/argus/instance/update/update.go +++ b/internal/cmd/argus/instance/update/update.go @@ -145,7 +145,7 @@ func parseInput(cmd *cobra.Command, inputArgs []string) (*inputModel, error) { } if planId == nil && planName == "" && instanceName == nil { - return nil, &cliErr.EmptyUpdateError{} + return nil, &cliErr.EmptyUpdateError{} } return &inputModel{ From 86aee51af747388a99df11a36592e1767cfd7bfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 10:44:35 +0000 Subject: [PATCH 12/17] update command: support no plan info provided --- internal/pkg/services/argus/utils/utils.go | 8 ++++ .../pkg/services/argus/utils/utils_test.go | 48 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/internal/pkg/services/argus/utils/utils.go b/internal/pkg/services/argus/utils/utils.go index e530c187b..fa028b532 100644 --- a/internal/pkg/services/argus/utils/utils.go +++ b/internal/pkg/services/argus/utils/utils.go @@ -67,3 +67,11 @@ func GetInstanceName(ctx context.Context, apiClient ArgusClient, instanceId, pro } return *resp.Name, nil } + +func GetInstancePlanId(ctx context.Context, apiClient ArgusClient, instanceId, projectId string) (*string, error) { + resp, err := apiClient.GetInstanceExecute(ctx, instanceId, projectId) + if err != nil { + return nil, fmt.Errorf("get Argus instance: %w", err) + } + return resp.PlanId, nil +} diff --git a/internal/pkg/services/argus/utils/utils_test.go b/internal/pkg/services/argus/utils/utils_test.go index 8adf5de1b..6ca572aa9 100644 --- a/internal/pkg/services/argus/utils/utils_test.go +++ b/internal/pkg/services/argus/utils/utils_test.go @@ -91,6 +91,54 @@ func TestGetInstanceName(t *testing.T) { } } +func TestGetInstancePlanId(t *testing.T) { + tests := []struct { + description string + getInstanceFails bool + getInstanceResp *argus.GetInstanceResponse + isValid bool + expectedOutput string + }{ + { + description: "base", + getInstanceResp: &argus.GetInstanceResponse{ + PlanId: utils.Ptr(testPlanId), + }, + isValid: true, + expectedOutput: testPlanId, + }, + { + description: "get instance fails", + getInstanceFails: true, + isValid: false, + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + client := &argusClientMocked{ + getInstanceFails: tt.getInstanceFails, + getInstanceResp: tt.getInstanceResp, + } + + output, err := GetInstancePlanId(context.Background(), client, testInstanceId, testProjectId) + + if tt.isValid && err != nil { + t.Errorf("failed on valid input") + } + if !tt.isValid && err == nil { + t.Errorf("did not fail on invalid input") + } + if !tt.isValid { + return + } + if *output != tt.expectedOutput { + t.Errorf("expected output to be %s, got %s", tt.expectedOutput, *output) + } + }) + } +} + func TestLoadPlanId(t *testing.T) { tests := []struct { description string From e68033a9d2ffd4249cef7bf1a97baa81b22fa231 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 14:42:00 +0000 Subject: [PATCH 13/17] update command: handle empty name update and corner cases --- internal/pkg/services/argus/utils/utils.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/pkg/services/argus/utils/utils.go b/internal/pkg/services/argus/utils/utils.go index fa028b532..0b4d1333d 100644 --- a/internal/pkg/services/argus/utils/utils.go +++ b/internal/pkg/services/argus/utils/utils.go @@ -75,3 +75,12 @@ func GetInstancePlanId(ctx context.Context, apiClient ArgusClient, instanceId, p } return resp.PlanId, nil } + + +func GetInstanceDetails(ctx context.Context, apiClient ArgusClient, instanceId, projectId string) (name, planId *string, err error) { + resp, err := apiClient.GetInstanceExecute(ctx, instanceId, projectId) + if err != nil { + return nil, nil, fmt.Errorf("get Argus instance: %w", err) + } + return resp.Name, resp.PlanId, nil +} From c6cd64a69aeedfbea221a955f5e8025b85fb65f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 14:44:52 +0000 Subject: [PATCH 14/17] fix linting --- internal/pkg/services/argus/utils/utils.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/pkg/services/argus/utils/utils.go b/internal/pkg/services/argus/utils/utils.go index 0b4d1333d..502e93e35 100644 --- a/internal/pkg/services/argus/utils/utils.go +++ b/internal/pkg/services/argus/utils/utils.go @@ -76,7 +76,6 @@ func GetInstancePlanId(ctx context.Context, apiClient ArgusClient, instanceId, p return resp.PlanId, nil } - func GetInstanceDetails(ctx context.Context, apiClient ArgusClient, instanceId, projectId string) (name, planId *string, err error) { resp, err := apiClient.GetInstanceExecute(ctx, instanceId, projectId) if err != nil { From 66afeff4dc59ffe2c3e01e9a2a6b12f5315f3822 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 15:48:53 +0000 Subject: [PATCH 15/17] address comments in PR --- internal/pkg/services/argus/utils/utils.go | 16 ------- .../pkg/services/argus/utils/utils_test.go | 48 ------------------- 2 files changed, 64 deletions(-) diff --git a/internal/pkg/services/argus/utils/utils.go b/internal/pkg/services/argus/utils/utils.go index 502e93e35..e530c187b 100644 --- a/internal/pkg/services/argus/utils/utils.go +++ b/internal/pkg/services/argus/utils/utils.go @@ -67,19 +67,3 @@ func GetInstanceName(ctx context.Context, apiClient ArgusClient, instanceId, pro } return *resp.Name, nil } - -func GetInstancePlanId(ctx context.Context, apiClient ArgusClient, instanceId, projectId string) (*string, error) { - resp, err := apiClient.GetInstanceExecute(ctx, instanceId, projectId) - if err != nil { - return nil, fmt.Errorf("get Argus instance: %w", err) - } - return resp.PlanId, nil -} - -func GetInstanceDetails(ctx context.Context, apiClient ArgusClient, instanceId, projectId string) (name, planId *string, err error) { - resp, err := apiClient.GetInstanceExecute(ctx, instanceId, projectId) - if err != nil { - return nil, nil, fmt.Errorf("get Argus instance: %w", err) - } - return resp.Name, resp.PlanId, nil -} diff --git a/internal/pkg/services/argus/utils/utils_test.go b/internal/pkg/services/argus/utils/utils_test.go index 6ca572aa9..8adf5de1b 100644 --- a/internal/pkg/services/argus/utils/utils_test.go +++ b/internal/pkg/services/argus/utils/utils_test.go @@ -91,54 +91,6 @@ func TestGetInstanceName(t *testing.T) { } } -func TestGetInstancePlanId(t *testing.T) { - tests := []struct { - description string - getInstanceFails bool - getInstanceResp *argus.GetInstanceResponse - isValid bool - expectedOutput string - }{ - { - description: "base", - getInstanceResp: &argus.GetInstanceResponse{ - PlanId: utils.Ptr(testPlanId), - }, - isValid: true, - expectedOutput: testPlanId, - }, - { - description: "get instance fails", - getInstanceFails: true, - isValid: false, - }, - } - - for _, tt := range tests { - t.Run(tt.description, func(t *testing.T) { - client := &argusClientMocked{ - getInstanceFails: tt.getInstanceFails, - getInstanceResp: tt.getInstanceResp, - } - - output, err := GetInstancePlanId(context.Background(), client, testInstanceId, testProjectId) - - if tt.isValid && err != nil { - t.Errorf("failed on valid input") - } - if !tt.isValid && err == nil { - t.Errorf("did not fail on invalid input") - } - if !tt.isValid { - return - } - if *output != tt.expectedOutput { - t.Errorf("expected output to be %s, got %s", tt.expectedOutput, *output) - } - }) - } -} - func TestLoadPlanId(t *testing.T) { tests := []struct { description string From c280fa2ccb23cb6ed1b18917736ad38ad5467065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 16:23:42 +0000 Subject: [PATCH 16/17] Onboard Argus Instance: delete command --- internal/cmd/argus/instance/delete/delete.go | 114 ++++++++++ .../cmd/argus/instance/delete/delete_test.go | 215 ++++++++++++++++++ internal/cmd/argus/instance/instance.go | 2 + 3 files changed, 331 insertions(+) create mode 100644 internal/cmd/argus/instance/delete/delete.go create mode 100644 internal/cmd/argus/instance/delete/delete_test.go diff --git a/internal/cmd/argus/instance/delete/delete.go b/internal/cmd/argus/instance/delete/delete.go new file mode 100644 index 000000000..345fc7758 --- /dev/null +++ b/internal/cmd/argus/instance/delete/delete.go @@ -0,0 +1,114 @@ +package delete + +import ( + "context" + "fmt" + + "github.com/stackitcloud/stackit-cli/internal/pkg/args" + "github.com/stackitcloud/stackit-cli/internal/pkg/confirm" + "github.com/stackitcloud/stackit-cli/internal/pkg/errors" + "github.com/stackitcloud/stackit-cli/internal/pkg/examples" + "github.com/stackitcloud/stackit-cli/internal/pkg/globalflags" + "github.com/stackitcloud/stackit-cli/internal/pkg/services/argus/client" + argusUtils "github.com/stackitcloud/stackit-cli/internal/pkg/services/argus/utils" + "github.com/stackitcloud/stackit-cli/internal/pkg/spinner" + "github.com/stackitcloud/stackit-cli/internal/pkg/utils" + + "github.com/spf13/cobra" + "github.com/stackitcloud/stackit-sdk-go/services/argus" + "github.com/stackitcloud/stackit-sdk-go/services/argus/wait" +) + +const ( + instanceIdArg = "INSTANCE_ID" +) + +type inputModel struct { + *globalflags.GlobalFlagModel + InstanceId string +} + +func NewCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: fmt.Sprintf("delete %s", instanceIdArg), + Short: "Deletes an Argus instance", + Long: "Deletes an Argus instance.", + Args: args.SingleArg(instanceIdArg, utils.ValidateUUID), + Example: examples.Build( + examples.NewExample( + `Delete an Argus instance with ID "xxx"`, + "$ stackit argus instance delete xxx"), + ), + RunE: func(cmd *cobra.Command, args []string) error { + ctx := context.Background() + model, err := parseInput(cmd, args) + if err != nil { + return err + } + + // Configure API client + apiClient, err := client.ConfigureClient(cmd) + if err != nil { + return err + } + + instanceLabel, err := argusUtils.GetInstanceName(ctx, apiClient, model.InstanceId, model.ProjectId) + if err != nil { + instanceLabel = model.InstanceId + } + + if !model.AssumeYes { + prompt := fmt.Sprintf("Are you sure you want to delete instance %q? (This cannot be undone)", instanceLabel) + err = confirm.PromptForConfirmation(cmd, prompt) + if err != nil { + return err + } + } + + // Call API + req := buildRequest(ctx, model, apiClient) + _, err = req.Execute() + if err != nil { + return fmt.Errorf("delete Argus instance: %w", err) + } + + // Wait for async operation, if async mode not enabled + if !model.Async { + s := spinner.New(cmd) + s.Start("Deleting instance") + _, err = wait.DeleteInstanceWaitHandler(ctx, apiClient, model.InstanceId, model.ProjectId).WaitWithContext(ctx) + if err != nil { + return fmt.Errorf("wait for Argus instance deletion: %w", err) + } + s.Stop() + } + + operationState := "Deleted" + if model.Async { + operationState = "Triggered deletion of" + } + cmd.Printf("%s instance %q\n", operationState, instanceLabel) + return nil + }, + } + return cmd +} + +func parseInput(cmd *cobra.Command, inputArgs []string) (*inputModel, error) { + instanceId := inputArgs[0] + + globalFlags := globalflags.Parse(cmd) + if globalFlags.ProjectId == "" { + return nil, &errors.ProjectIdError{} + } + + return &inputModel{ + GlobalFlagModel: globalFlags, + InstanceId: instanceId, + }, nil +} + +func buildRequest(ctx context.Context, model *inputModel, apiClient *argus.APIClient) argus.ApiDeleteInstanceRequest { + req := apiClient.DeleteInstance(ctx, model.InstanceId, model.ProjectId) + return req +} diff --git a/internal/cmd/argus/instance/delete/delete_test.go b/internal/cmd/argus/instance/delete/delete_test.go new file mode 100644 index 000000000..5508651cb --- /dev/null +++ b/internal/cmd/argus/instance/delete/delete_test.go @@ -0,0 +1,215 @@ +package delete + +import ( + "context" + "testing" + + "github.com/stackitcloud/stackit-cli/internal/pkg/globalflags" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/google/uuid" + "github.com/stackitcloud/stackit-sdk-go/services/argus" +) + +var projectIdFlag = globalflags.ProjectIdFlag + +type testCtxKey struct{} + +var testCtx = context.WithValue(context.Background(), testCtxKey{}, "foo") +var testClient = &argus.APIClient{} +var testProjectId = uuid.NewString() +var testInstanceId = uuid.NewString() + +func fixtureArgValues(mods ...func(argValues []string)) []string { + argValues := []string{ + testInstanceId, + } + for _, mod := range mods { + mod(argValues) + } + return argValues +} + +func fixtureFlagValues(mods ...func(flagValues map[string]string)) map[string]string { + flagValues := map[string]string{ + projectIdFlag: testProjectId, + } + for _, mod := range mods { + mod(flagValues) + } + return flagValues +} + +func fixtureInputModel(mods ...func(model *inputModel)) *inputModel { + model := &inputModel{ + GlobalFlagModel: &globalflags.GlobalFlagModel{ + ProjectId: testProjectId, + }, + InstanceId: testInstanceId, + } + for _, mod := range mods { + mod(model) + } + return model +} + +func fixtureRequest(mods ...func(request *argus.ApiDeleteInstanceRequest)) argus.ApiDeleteInstanceRequest { + request := testClient.DeleteInstance(testCtx, testInstanceId, testProjectId) + 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 arg values", + argValues: []string{}, + flagValues: fixtureFlagValues(), + isValid: false, + }, + { + description: "no flag values", + argValues: fixtureArgValues(), + flagValues: map[string]string{}, + isValid: false, + }, + { + description: "project id missing", + argValues: fixtureArgValues(), + flagValues: fixtureFlagValues(func(flagValues map[string]string) { + delete(flagValues, projectIdFlag) + }), + isValid: false, + }, + { + description: "project id invalid 1", + argValues: fixtureArgValues(), + flagValues: fixtureFlagValues(func(flagValues map[string]string) { + flagValues[projectIdFlag] = "" + }), + isValid: false, + }, + { + description: "project id invalid 2", + argValues: fixtureArgValues(), + flagValues: fixtureFlagValues(func(flagValues map[string]string) { + flagValues[projectIdFlag] = "invalid-uuid" + }), + isValid: false, + }, + { + description: "instance id invalid 1", + argValues: []string{""}, + flagValues: fixtureFlagValues(), + isValid: false, + }, + { + description: "instance id invalid 2", + argValues: []string{"invalid-uuid"}, + flagValues: fixtureFlagValues(), + isValid: false, + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + cmd := NewCmd() + err := globalflags.Configure(cmd.Flags()) + if err != nil { + t.Fatalf("configure global flags: %v", err) + } + + for flag, value := range tt.flagValues { + err := cmd.Flags().Set(flag, value) + if err != nil { + if !tt.isValid { + return + } + t.Fatalf("setting flag --%s=%s: %v", flag, value, err) + } + } + + err = cmd.ValidateArgs(tt.argValues) + if err != nil { + if !tt.isValid { + return + } + t.Fatalf("error validating args: %v", err) + } + + err = cmd.ValidateRequiredFlags() + if err != nil { + if !tt.isValid { + return + } + t.Fatalf("error validating flags: %v", err) + } + + model, err := parseInput(cmd, tt.argValues) + if err != nil { + if !tt.isValid { + return + } + t.Fatalf("error parsing input: %v", err) + } + + if !tt.isValid { + t.Fatalf("did not fail on invalid input") + } + diff := cmp.Diff(model, tt.expectedModel) + if diff != "" { + t.Fatalf("Data does not match: %s", diff) + } + }) + } +} + +func TestBuildRequest(t *testing.T) { + tests := []struct { + description string + model *inputModel + expectedRequest argus.ApiDeleteInstanceRequest + }{ + { + description: "base", + model: fixtureInputModel(), + expectedRequest: fixtureRequest(), + }, + } + + 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), + ) + if diff != "" { + t.Fatalf("Data does not match: %s", diff) + } + }) + } +} diff --git a/internal/cmd/argus/instance/instance.go b/internal/cmd/argus/instance/instance.go index 82b8054a8..e6b3717fc 100644 --- a/internal/cmd/argus/instance/instance.go +++ b/internal/cmd/argus/instance/instance.go @@ -2,6 +2,7 @@ package instance import ( "github.com/stackitcloud/stackit-cli/internal/cmd/argus/instance/create" + "github.com/stackitcloud/stackit-cli/internal/cmd/argus/instance/delete" "github.com/stackitcloud/stackit-cli/internal/cmd/argus/instance/update" "github.com/stackitcloud/stackit-cli/internal/pkg/args" "github.com/stackitcloud/stackit-cli/internal/pkg/utils" @@ -24,4 +25,5 @@ func NewCmd() *cobra.Command { func addSubcommands(cmd *cobra.Command) { cmd.AddCommand(create.NewCmd()) cmd.AddCommand(update.NewCmd()) + cmd.AddCommand(delete.NewCmd()) } From a7a7cf937731c112e01dd5f9a8e614aa82e15a82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diogo=20Ferr=C3=A3o?= Date: Tue, 12 Mar 2024 17:28:28 +0000 Subject: [PATCH 17/17] delete command: add docs --- docs/stackit_argus_instance.md | 1 + docs/stackit_argus_instance_delete.md | 38 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 docs/stackit_argus_instance_delete.md diff --git a/docs/stackit_argus_instance.md b/docs/stackit_argus_instance.md index 8c579a4d8..660143c76 100644 --- a/docs/stackit_argus_instance.md +++ b/docs/stackit_argus_instance.md @@ -29,5 +29,6 @@ stackit argus instance [flags] * [stackit argus](./stackit_argus.md) - Provides functionality for Argus * [stackit argus instance create](./stackit_argus_instance_create.md) - Creates an Argus instance +* [stackit argus instance delete](./stackit_argus_instance_delete.md) - Deletes an Argus instance * [stackit argus instance update](./stackit_argus_instance_update.md) - Updates an Argus instance diff --git a/docs/stackit_argus_instance_delete.md b/docs/stackit_argus_instance_delete.md new file mode 100644 index 000000000..c49d5ec41 --- /dev/null +++ b/docs/stackit_argus_instance_delete.md @@ -0,0 +1,38 @@ +## stackit argus instance delete + +Deletes an Argus instance + +### Synopsis + +Deletes an Argus instance. + +``` +stackit argus instance delete INSTANCE_ID [flags] +``` + +### Examples + +``` + Delete an Argus instance with ID "xxx" + $ stackit argus instance delete xxx +``` + +### Options + +``` + -h, --help Help for "stackit argus instance delete" +``` + +### Options inherited from parent commands + +``` + -y, --assume-yes If set, skips all confirmation prompts + --async If set, runs the command asynchronously + -o, --output-format string Output format, one of ["json" "pretty"] + -p, --project-id string Project ID +``` + +### SEE ALSO + +* [stackit argus instance](./stackit_argus_instance.md) - Provides functionality for Argus instances +