diff --git a/internal/commands/ix/ix_inputs.go b/internal/commands/ix/ix_inputs.go index c44cecf3..1ee43326 100644 --- a/internal/commands/ix/ix_inputs.go +++ b/internal/commands/ix/ix_inputs.go @@ -83,6 +83,9 @@ func buildUpdateIXRequestFromFlags(cmd *cobra.Command) (*megaport.UpdateIXReques if cmd.Flags().Changed("name") { name, _ := cmd.Flags().GetString("name") + if name == "" { + return nil, validation.NewValidationError("name", name, "cannot be empty") + } req.Name = &name } @@ -154,14 +157,17 @@ func buildUpdateIXRequestFromFlags(cmd *cobra.Command) (*megaport.UpdateIXReques func buildUpdateIXRequestFromJSON(jsonStr, jsonFile string) (*megaport.UpdateIXRequest, error) { jsonData, err := utils.ReadJSONInput(jsonStr, jsonFile) if err != nil { - return nil, err + return nil, exitcodes.NewUsageError(err) } req := &megaport.UpdateIXRequest{} if err := json.Unmarshal(jsonData, req); err != nil { - return nil, fmt.Errorf("failed to parse JSON: %w", err) + return nil, exitcodes.NewUsageError(fmt.Errorf("failed to parse JSON: %w", err)) } + if req.Name != nil && *req.Name == "" { + return nil, validation.NewValidationError("name", "", "cannot be empty") + } if req.ASN != nil { if err := validation.ValidateASN(*req.ASN); err != nil { return nil, err @@ -183,5 +189,11 @@ func buildUpdateIXRequestFromJSON(jsonStr, jsonFile string) (*megaport.UpdateIXR } } + if req.Name == nil && req.RateLimit == nil && req.CostCentre == nil && req.VLAN == nil && + req.MACAddress == nil && req.ASN == nil && req.Password == nil && req.PublicGraph == nil && + req.ReverseDns == nil && req.AEndProductUid == nil && req.Shutdown == nil { + return nil, exitcodes.NewUsageError(fmt.Errorf("at least one field must be updated")) + } + return req, nil } diff --git a/internal/commands/ix/ix_inputs_test.go b/internal/commands/ix/ix_inputs_test.go index bb4ae181..72b51f7a 100644 --- a/internal/commands/ix/ix_inputs_test.go +++ b/internal/commands/ix/ix_inputs_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/megaport/megaport-cli/internal/base/exitcodes" + "github.com/megaport/megaport-cli/internal/validation" megaport "github.com/megaport/megaportgo" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -29,6 +30,21 @@ func TestBuildUpdateIXRequestFromJSON_BothEmpty(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "failed to parse JSON") assert.Nil(t, req) + + var cliErr *exitcodes.CLIError + require.True(t, errors.As(err, &cliErr)) + assert.Equal(t, exitcodes.Usage, cliErr.Code) +} + +func TestBuildUpdateIXRequestFromJSON_FileNotFound(t *testing.T) { + req, err := buildUpdateIXRequestFromJSON("", filepath.Join(t.TempDir(), "missing.json")) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to read JSON file") + assert.Nil(t, req) + + var cliErr *exitcodes.CLIError + require.True(t, errors.As(err, &cliErr)) + assert.Equal(t, exitcodes.Usage, cliErr.Code) } func TestBuildIXRequestFromJSON_AllFields(t *testing.T) { @@ -119,6 +135,39 @@ func TestBuildUpdateIXRequestFromJSON_PointerFields(t *testing.T) { } } +func TestBuildUpdateIXRequestFromJSON_AtLeastOneFieldExitCode(t *testing.T) { + req, err := buildUpdateIXRequestFromJSON(`{}`, "") + assert.Nil(t, req) + assert.Contains(t, err.Error(), "at least one field must be updated") + + var cliErr *exitcodes.CLIError + require.True(t, errors.As(err, &cliErr)) + assert.Equal(t, exitcodes.Usage, cliErr.Code) +} + +func TestBuildUpdateIXRequestFromJSON_EmptyNameRejected(t *testing.T) { + req, err := buildUpdateIXRequestFromJSON(`{"name":""}`, "") + assert.Nil(t, req) + + var valErr *validation.ValidationError + require.True(t, errors.As(err, &valErr)) + assert.Equal(t, "name", valErr.Field) +} + +func TestBuildUpdateIXRequestFromFlags_EmptyNameRejected(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().String("name", "", "") + require.NoError(t, cmd.Flags().Set("name", "")) + cmd.Flags().Lookup("name").Changed = true + + req, err := buildUpdateIXRequestFromFlags(cmd) + assert.Nil(t, req) + + var valErr *validation.ValidationError + require.True(t, errors.As(err, &valErr)) + assert.Equal(t, "name", valErr.Field) +} + func TestBuildUpdateIXRequestFromFlags_NoopWhenUnchanged(t *testing.T) { cmd := &cobra.Command{Use: "test"} cmd.Flags().String("name", "", "") diff --git a/internal/commands/ix/ix_test.go b/internal/commands/ix/ix_test.go index c4e853f3..88e1ae62 100644 --- a/internal/commands/ix/ix_test.go +++ b/internal/commands/ix/ix_test.go @@ -1134,12 +1134,14 @@ func TestBuildUpdateIXRequestFromJSON(t *testing.T) { expectedError: "failed to parse JSON", }, { - name: "empty JSON object", - jsonStr: `{}`, - validate: func(t *testing.T, req *megaport.UpdateIXRequest) { - assert.Nil(t, req.Name) - assert.Nil(t, req.RateLimit) - }, + name: "empty JSON object", + jsonStr: `{}`, + expectedError: "at least one field must be updated", + }, + { + name: "misspelled key matches no known field", + jsonStr: `{"nam":"Updated IX"}`, + expectedError: "at least one field must be updated", }, { name: "valid JSON file", diff --git a/internal/commands/managed_account/managed_account_inputs.go b/internal/commands/managed_account/managed_account_inputs.go index 601f4731..a9e4e8a9 100644 --- a/internal/commands/managed_account/managed_account_inputs.go +++ b/internal/commands/managed_account/managed_account_inputs.go @@ -29,6 +29,10 @@ func buildManagedAccountRequestFromFlags(cmd *cobra.Command) (*megaport.ManagedA accountName, _ := cmd.Flags().GetString("account-name") accountRef, _ := cmd.Flags().GetString("account-ref") + if accountName == "" || accountRef == "" { + return nil, exitcodes.NewUsageError(fmt.Errorf("accountName and accountRef are required")) + } + req := &megaport.ManagedAccountRequest{ AccountName: accountName, AccountRef: accountRef, @@ -38,7 +42,16 @@ func buildManagedAccountRequestFromFlags(cmd *cobra.Command) (*megaport.ManagedA } func buildManagedAccountRequestFromJSON(jsonStr, jsonFile string) (*megaport.ManagedAccountRequest, error) { - return parseManagedAccountRequestJSON(jsonStr, jsonFile) + req, err := parseManagedAccountRequestJSON(jsonStr, jsonFile) + if err != nil { + return nil, err + } + + if req.AccountName == "" || req.AccountRef == "" { + return nil, exitcodes.NewUsageError(fmt.Errorf("accountName and accountRef are required")) + } + + return req, nil } // buildUpdateManagedAccountRequestFromFlags seeds the request from the current diff --git a/internal/commands/managed_account/managed_account_inputs_test.go b/internal/commands/managed_account/managed_account_inputs_test.go index cccbb4d9..1c88c16b 100644 --- a/internal/commands/managed_account/managed_account_inputs_test.go +++ b/internal/commands/managed_account/managed_account_inputs_test.go @@ -90,9 +90,10 @@ func TestParseManagedAccountRequestJSON(t *testing.T) { func TestBuildManagedAccountRequestFromFlags(t *testing.T) { tests := []struct { - name string - flags map[string]string - validate func(t *testing.T, req *megaport.ManagedAccountRequest) + name string + flags map[string]string + expectedError string + validate func(t *testing.T, req *megaport.ManagedAccountRequest) }{ { name: "both flags provided", @@ -103,29 +104,33 @@ func TestBuildManagedAccountRequestFromFlags(t *testing.T) { }, }, { - name: "name only", - flags: map[string]string{"account-name": "Test Account"}, - validate: func(t *testing.T, req *megaport.ManagedAccountRequest) { - assert.Equal(t, "Test Account", req.AccountName) - assert.Equal(t, "", req.AccountRef) - }, + name: "name only", + flags: map[string]string{"account-name": "Test Account"}, + expectedError: "accountName and accountRef are required", }, { - name: "no flags (defaults)", - flags: map[string]string{}, - validate: func(t *testing.T, req *megaport.ManagedAccountRequest) { - assert.Equal(t, "", req.AccountName) - assert.Equal(t, "", req.AccountRef) - }, + name: "no flags (defaults)", + flags: map[string]string{}, + expectedError: "accountName and accountRef are required", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { req, err := buildManagedAccountRequestFromFlags(newManagedAccountCmd(tt.flags)) - assert.NoError(t, err) - assert.NotNil(t, req) - tt.validate(t, req) + + if tt.expectedError != "" { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedError) + + var cliErr *exitcodes.CLIError + require.True(t, errors.As(err, &cliErr)) + assert.Equal(t, exitcodes.Usage, cliErr.Code) + } else { + assert.NoError(t, err) + assert.NotNil(t, req) + tt.validate(t, req) + } }) } } @@ -148,12 +153,9 @@ func TestBuildManagedAccountRequestFromJSON(t *testing.T) { }, }, { - name: "valid JSON string with partial fields", - jsonStr: `{"accountName":"Partial Account"}`, - validate: func(t *testing.T, req *megaport.ManagedAccountRequest) { - assert.Equal(t, "Partial Account", req.AccountName) - assert.Equal(t, "", req.AccountRef) - }, + name: "JSON string missing account-ref", + jsonStr: `{"accountName":"Partial Account"}`, + expectedError: "accountName and accountRef are required", }, { name: "invalid JSON syntax", @@ -161,12 +163,9 @@ func TestBuildManagedAccountRequestFromJSON(t *testing.T) { expectedError: "failed to parse JSON", }, { - name: "empty JSON object", - jsonStr: `{}`, - validate: func(t *testing.T, req *megaport.ManagedAccountRequest) { - assert.Equal(t, "", req.AccountName) - assert.Equal(t, "", req.AccountRef) - }, + name: "empty JSON object", + jsonStr: `{}`, + expectedError: "accountName and accountRef are required", }, { name: "valid JSON file", diff --git a/internal/commands/mve/mve_inputs.go b/internal/commands/mve/mve_inputs.go index 2a9dced7..aa8e6e6f 100644 --- a/internal/commands/mve/mve_inputs.go +++ b/internal/commands/mve/mve_inputs.go @@ -685,9 +685,15 @@ func processJSONUpdateMVEInput(jsonStr, jsonFilePath, mveUID string) (*megaport. MVEID: mveUID, } + // Gate on presence, not a non-empty value, so an explicit "name": "" is + // rejected deterministically instead of being silently treated the same + // as the key being absent, matching the flag path. if name, present, err := utils.JSONString(jsonData, "name"); err != nil { return nil, false, err - } else if present && name != "" { + } else if present { + if name == "" { + return nil, false, validation.NewValidationError("name", name, "cannot be empty") + } req.Name = name } @@ -748,7 +754,13 @@ func processFlagUpdateMVEInput(cmd *cobra.Command, mveUID string) (*megaport.Mod MVEID: mveUID, } - if name != "" { + // Gate on Changed, not a non-empty value, so an explicit --name "" is + // rejected deterministically instead of being silently treated the same + // as the flag not being passed at all. + if cmd.Flags().Changed("name") { + if name == "" { + return nil, false, validation.NewValidationError("name", name, "cannot be empty") + } req.Name = name } diff --git a/internal/commands/mve/mve_inputs_test.go b/internal/commands/mve/mve_inputs_test.go index 31891352..9088fffe 100644 --- a/internal/commands/mve/mve_inputs_test.go +++ b/internal/commands/mve/mve_inputs_test.go @@ -630,6 +630,20 @@ func TestProcessFlagUpdateMVEInput(t *testing.T) { } } +func TestProcessFlagUpdateMVEInput_ExplicitEmptyNameRejected(t *testing.T) { + cmd := createTestCmd() + require.NoError(t, cmd.Flags().Set("name", "")) + _, _, err := processFlagUpdateMVEInput(cmd, "mve-123") + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot be empty") +} + +func TestProcessJSONUpdateMVEInput_ExplicitEmptyNameRejected(t *testing.T) { + _, _, err := processJSONUpdateMVEInput(`{"name":""}`, "", "mve-123") + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot be empty") +} + func TestProcessFlagUpdateMVEInput_CostCentreProvided(t *testing.T) { t.Run("flag not set reports not provided", func(t *testing.T) { cmd := createTestCmd() diff --git a/internal/commands/nat_gateway/nat_gateway_actions.go b/internal/commands/nat_gateway/nat_gateway_actions.go index f3e9c235..5e40c7ec 100644 --- a/internal/commands/nat_gateway/nat_gateway_actions.go +++ b/internal/commands/nat_gateway/nat_gateway_actions.go @@ -298,6 +298,8 @@ func UpdateNATGateway(cmd *cobra.Command, args []string, noColor bool) error { explicit.AutoRenewTerm = cmd.Flags().Changed("auto-renew") explicit.SessionCount = cmd.Flags().Changed("session-count") explicit.DiversityZone = cmd.Flags().Changed("diversity-zone") + explicit.PromoCode = cmd.Flags().Changed("promo-code") + explicit.ServiceLevelReference = cmd.Flags().Changed("service-level-reference") // ASN and BGPShutdownDefault have no flag path; never explicit in flag mode. } else if interactive { req, explicit, err = promptForUpdateNATGatewayDetails(uid, noColor) @@ -363,6 +365,16 @@ func mergeUpdateDefaults(req *megaport.UpdateNATGatewayRequest, original *megapo if req.Term == 0 { req.Term = original.Term } + // PromoCode and ServiceLevelReference carry omitempty, so an explicit "" is + // dropped from the wire and the server clears the field. Only inherit the + // original when the caller didn't provide the field, so a deliberate clear + // isn't overwritten with the stale value. + if !explicit.PromoCode && req.PromoCode == "" { + req.PromoCode = original.PromoCode + } + if !explicit.ServiceLevelReference && req.ServiceLevelReference == "" { + req.ServiceLevelReference = original.ServiceLevelReference + } if !explicit.SessionCount && req.Config.SessionCount == 0 { req.Config.SessionCount = original.Config.SessionCount } diff --git a/internal/commands/nat_gateway/nat_gateway_additional_test.go b/internal/commands/nat_gateway/nat_gateway_additional_test.go index 4ff4bb44..77f03674 100644 --- a/internal/commands/nat_gateway/nat_gateway_additional_test.go +++ b/internal/commands/nat_gateway/nat_gateway_additional_test.go @@ -555,6 +555,28 @@ func TestProcessJSONUpdateNATGatewayInputBoolPresence(t *testing.T) { assert.True(t, explicit.DiversityZone, "explicit empty string must be tracked so mergeUpdateDefaults does not override it") assert.Equal(t, "", req.Config.DiversityZone) }) + t.Run("promoCode absent returns explicit=false", func(t *testing.T) { + _, explicit, err := processJSONUpdateNATGatewayInput(`{"name":"GW"}`, "", "uid") + assert.NoError(t, err) + assert.False(t, explicit.PromoCode) + }) + t.Run("promoCode present empty returns explicit=true", func(t *testing.T) { + req, explicit, err := processJSONUpdateNATGatewayInput(`{"promoCode":""}`, "", "uid") + assert.NoError(t, err) + assert.True(t, explicit.PromoCode, "explicit empty string must be tracked so mergeUpdateDefaults does not override it") + assert.Equal(t, "", req.PromoCode) + }) + t.Run("serviceLevelReference absent returns explicit=false", func(t *testing.T) { + _, explicit, err := processJSONUpdateNATGatewayInput(`{"name":"GW"}`, "", "uid") + assert.NoError(t, err) + assert.False(t, explicit.ServiceLevelReference) + }) + t.Run("serviceLevelReference present empty returns explicit=true", func(t *testing.T) { + req, explicit, err := processJSONUpdateNATGatewayInput(`{"serviceLevelReference":""}`, "", "uid") + assert.NoError(t, err) + assert.True(t, explicit.ServiceLevelReference, "explicit empty string must be tracked so mergeUpdateDefaults does not override it") + assert.Equal(t, "", req.ServiceLevelReference) + }) } func TestMergeUpdateDefaultsExplicitBools(t *testing.T) { @@ -615,6 +637,38 @@ func TestMergeUpdateDefaultsExplicitBools(t *testing.T) { }) } +func TestMergeUpdateDefaultsPromoCodeAndServiceLevelReference(t *testing.T) { + original := &megaport.NATGateway{ + ProductName: "Original", + LocationID: 1, + Speed: 1000, + Term: 12, + PromoCode: "ORIGINAL-PROMO", + ServiceLevelReference: "ORIGINAL-SLR", + } + + t.Run("blank promo code and service level reference keep current value", func(t *testing.T) { + req := &megaport.UpdateNATGatewayRequest{ProductUID: "uid"} + mergeUpdateDefaults(req, original, updateExplicitFields{}) + assert.Equal(t, "ORIGINAL-PROMO", req.PromoCode, "blank promo code must inherit from original") + assert.Equal(t, "ORIGINAL-SLR", req.ServiceLevelReference, "blank service level reference must inherit from original") + }) + + t.Run("provided promo code and service level reference are not overridden", func(t *testing.T) { + req := &megaport.UpdateNATGatewayRequest{ProductUID: "uid", PromoCode: "NEW-PROMO", ServiceLevelReference: "NEW-SLR"} + mergeUpdateDefaults(req, original, updateExplicitFields{}) + assert.Equal(t, "NEW-PROMO", req.PromoCode) + assert.Equal(t, "NEW-SLR", req.ServiceLevelReference) + }) + + t.Run("explicit blank promo code and service level reference are not overridden", func(t *testing.T) { + req := &megaport.UpdateNATGatewayRequest{ProductUID: "uid"} + mergeUpdateDefaults(req, original, updateExplicitFields{PromoCode: true, ServiceLevelReference: true}) + assert.Empty(t, req.PromoCode, "explicitly cleared promo code must not inherit from original") + assert.Empty(t, req.ServiceLevelReference, "explicitly cleared service level reference must not inherit from original") + }) +} + func TestProcessFlagCreateNATGatewayInput(t *testing.T) { cmd := newTestCmd("create") require.NoError(t, cmd.Flags().Set("name", "Flag GW")) diff --git a/internal/commands/nat_gateway/nat_gateway_inputs.go b/internal/commands/nat_gateway/nat_gateway_inputs.go index 23aa69cb..15e3d376 100644 --- a/internal/commands/nat_gateway/nat_gateway_inputs.go +++ b/internal/commands/nat_gateway/nat_gateway_inputs.go @@ -89,11 +89,14 @@ func processFlagCreateNATGatewayInput(cmd *cobra.Command) (*megaport.CreateNATGa if resourceTagsStr != "" || resourceTagsFile != "" { tagData, err := utils.ReadJSONInput(resourceTagsStr, resourceTagsFile) if err != nil { - return nil, fmt.Errorf("failed to read resource tags: %w", err) + return nil, exitcodes.NewUsageError(fmt.Errorf("failed to read resource tags: %w", err)) } var tagsMap map[string]string if err := json.Unmarshal(tagData, &tagsMap); err != nil { - return nil, fmt.Errorf("failed to parse resource tags JSON: %w", err) + return nil, exitcodes.NewUsageError(fmt.Errorf("failed to parse resource tags JSON: %w", err)) + } + if err := utils.RejectEmptyTagKeys(tagsMap); err != nil { + return nil, exitcodes.NewUsageError(err) } for k, v := range tagsMap { resourceTags = append(resourceTags, megaport.ResourceTag{Key: k, Value: v}) @@ -132,15 +135,18 @@ func processFlagCreateNATGatewayInput(cmd *cobra.Command) (*megaport.CreateNATGa // BGPShutdownDefault *bool) get a corresponding entry here because their zero // value is ambiguous. Fields with genuinely invalid zero values // (LocationID, Speed, Term — all must be positive) do not need tracking -// because 0 can only mean "omitted". Fields with omitempty on the SDK request -// (PromoCode, ServiceLevelReference, ResourceTags) are safe to leave as plain -// strings/slices because the API ignores empty/nil values for those. +// because 0 can only mean "omitted". PromoCode and ServiceLevelReference are +// likewise plain strings; mergeUpdateDefaults inherits them from the original +// resource when left blank rather than relying on server-side omitempty +// behavior. type updateExplicitFields struct { - AutoRenewTerm bool // was autoRenewTerm present in input? - BGPShutdownDefault bool // was bgpShutdownDefault present in input? - ASN bool // was asn present in JSON input? (no flag path for ASN) - SessionCount bool // was sessionCount present in JSON input? - DiversityZone bool // was diversityZone present in JSON input? + AutoRenewTerm bool // was autoRenewTerm present in input? + BGPShutdownDefault bool // was bgpShutdownDefault present in input? + ASN bool // was asn present in JSON input? (no flag path for ASN) + SessionCount bool // was sessionCount present in JSON input? + DiversityZone bool // was diversityZone present in JSON input? + PromoCode bool // was promoCode present in input? + ServiceLevelReference bool // was serviceLevelReference present in input? } // processJSONUpdateNATGatewayInput parses a JSON update request and returns the @@ -148,8 +154,10 @@ type updateExplicitFields struct { // zero value could be a valid intentional value (bool, int for ASN/SessionCount, // string for DiversityZone) use pointer types in the raw struct so nil means // "not provided" and mergeUpdateDefaults will inherit from the original resource. -// Fields with genuinely invalid zero values (LocationID, Speed, Term) or omitempty -// in the SDK request (PromoCode, ServiceLevelReference) use plain types. +// Fields with genuinely invalid zero values (LocationID, Speed, Term) use plain +// types. PromoCode and ServiceLevelReference use pointer types too: they carry +// omitempty in the SDK request, so an explicit "" is dropped from the wire and +// the server clears the field, but only if we don't inherit the original first. func processJSONUpdateNATGatewayInput(jsonStr, jsonFile, uid string) (*megaport.UpdateNATGatewayRequest, updateExplicitFields, error) { jsonData, err := utils.ReadJSONInput(jsonStr, jsonFile) if err != nil { @@ -166,8 +174,8 @@ func processJSONUpdateNATGatewayInput(jsonStr, jsonFile, uid string) (*megaport. ASN *int `json:"asn"` BGPShutdownDefault *bool `json:"bgpShutdownDefault"` AutoRenewTerm *bool `json:"autoRenewTerm"` - PromoCode string `json:"promoCode"` - ServiceLevelReference string `json:"serviceLevelReference"` + PromoCode *string `json:"promoCode"` + ServiceLevelReference *string `json:"serviceLevelReference"` ResourceTags map[string]string `json:"resourceTags"` } if err := json.Unmarshal(jsonData, &raw); err != nil { @@ -175,15 +183,17 @@ func processJSONUpdateNATGatewayInput(jsonStr, jsonFile, uid string) (*megaport. } if err := utils.RejectEmptyTagKeys(raw.ResourceTags); err != nil { - return nil, updateExplicitFields{}, err + return nil, updateExplicitFields{}, exitcodes.NewUsageError(err) } explicit := updateExplicitFields{ - AutoRenewTerm: raw.AutoRenewTerm != nil, - BGPShutdownDefault: raw.BGPShutdownDefault != nil, - ASN: raw.ASN != nil, - SessionCount: raw.SessionCount != nil, - DiversityZone: raw.DiversityZone != nil, + AutoRenewTerm: raw.AutoRenewTerm != nil, + BGPShutdownDefault: raw.BGPShutdownDefault != nil, + ASN: raw.ASN != nil, + SessionCount: raw.SessionCount != nil, + DiversityZone: raw.DiversityZone != nil, + PromoCode: raw.PromoCode != nil, + ServiceLevelReference: raw.ServiceLevelReference != nil, } var autoRenew, bgpShutdown bool @@ -204,6 +214,13 @@ func processJSONUpdateNATGatewayInput(jsonStr, jsonFile, uid string) (*megaport. if raw.DiversityZone != nil { diversityZone = *raw.DiversityZone } + var promoCode, serviceLevelRef string + if raw.PromoCode != nil { + promoCode = *raw.PromoCode + } + if raw.ServiceLevelReference != nil { + serviceLevelRef = *raw.ServiceLevelReference + } req := &megaport.UpdateNATGatewayRequest{ ProductUID: uid, @@ -212,8 +229,8 @@ func processJSONUpdateNATGatewayInput(jsonStr, jsonFile, uid string) (*megaport. Speed: raw.Speed, Term: raw.Term, AutoRenewTerm: autoRenew, - PromoCode: raw.PromoCode, - ServiceLevelReference: raw.ServiceLevelReference, + PromoCode: promoCode, + ServiceLevelReference: serviceLevelRef, Config: megaport.NATGatewayNetworkConfig{ ASN: asn, BGPShutdownDefault: bgpShutdown, @@ -253,11 +270,14 @@ func processFlagUpdateNATGatewayInput(cmd *cobra.Command, uid string) (*megaport if resourceTagsStr != "" || resourceTagsFile != "" { tagData, err := utils.ReadJSONInput(resourceTagsStr, resourceTagsFile) if err != nil { - return nil, fmt.Errorf("failed to read resource tags: %w", err) + return nil, exitcodes.NewUsageError(fmt.Errorf("failed to read resource tags: %w", err)) } var tagsMap map[string]string if err := json.Unmarshal(tagData, &tagsMap); err != nil { - return nil, fmt.Errorf("failed to parse resource tags JSON: %w", err) + return nil, exitcodes.NewUsageError(fmt.Errorf("failed to parse resource tags JSON: %w", err)) + } + if err := utils.RejectEmptyTagKeys(tagsMap); err != nil { + return nil, exitcodes.NewUsageError(err) } for k, v := range tagsMap { resourceTags = append(resourceTags, megaport.ResourceTag{Key: k, Value: v}) diff --git a/internal/commands/nat_gateway/nat_gateway_inputs_test.go b/internal/commands/nat_gateway/nat_gateway_inputs_test.go index 91b5c527..2dedbe56 100644 --- a/internal/commands/nat_gateway/nat_gateway_inputs_test.go +++ b/internal/commands/nat_gateway/nat_gateway_inputs_test.go @@ -183,7 +183,11 @@ func TestProcessFlagCreateNATGatewayInput_InvalidResourceTagsJSON(t *testing.T) require.NoError(t, cmd.Flags().Set("resource-tags", `{invalid}`)) _, err := processFlagCreateNATGatewayInput(cmd) - assert.Error(t, err) + require.Error(t, err) + + var cliErr *exitcodes.CLIError + require.True(t, errors.As(err, &cliErr)) + assert.Equal(t, exitcodes.Usage, cliErr.Code) } func TestProcessFlagCreateNATGatewayInput_ValidResourceTags(t *testing.T) { @@ -199,6 +203,22 @@ func TestProcessFlagCreateNATGatewayInput_ValidResourceTags(t *testing.T) { assert.Len(t, req.ResourceTags, 2) } +func TestProcessFlagCreateNATGatewayInput_RejectsEmptyTagKey(t *testing.T) { + cmd := newCreateFlagsCmd() + require.NoError(t, cmd.Flags().Set("name", "Tag GW")) + require.NoError(t, cmd.Flags().Set("term", "12")) + require.NoError(t, cmd.Flags().Set("speed", "1000")) + require.NoError(t, cmd.Flags().Set("location-id", "1")) + require.NoError(t, cmd.Flags().Set("resource-tags", `{"":"x"}`)) + + _, err := processFlagCreateNATGatewayInput(cmd) + require.Error(t, err) + + var cliErr *exitcodes.CLIError + require.True(t, errors.As(err, &cliErr)) + assert.Equal(t, exitcodes.Usage, cliErr.Code) +} + // ---- processJSONUpdateNATGatewayInput ---- func TestProcessJSONUpdateNATGatewayInput_RejectsEmptyTagKey(t *testing.T) { @@ -206,6 +226,10 @@ func TestProcessJSONUpdateNATGatewayInput_RejectsEmptyTagKey(t *testing.T) { `{"name":"GW","resourceTags":{"":"x"}}`, "", "uid-empty-tag") require.Error(t, err) assert.Contains(t, err.Error(), "tag key must not be empty") + + var cliErr *exitcodes.CLIError + require.True(t, errors.As(err, &cliErr)) + assert.Equal(t, exitcodes.Usage, cliErr.Code) } func TestProcessJSONUpdateNATGatewayInput_ValidResourceTags(t *testing.T) { @@ -362,5 +386,21 @@ func TestProcessFlagUpdateNATGatewayInput_InvalidResourceTags(t *testing.T) { require.NoError(t, cmd.Flags().Set("resource-tags", `{invalid}`)) _, err := processFlagUpdateNATGatewayInput(cmd, "uid-bad") - assert.Error(t, err) + require.Error(t, err) + + var cliErr *exitcodes.CLIError + require.True(t, errors.As(err, &cliErr)) + assert.Equal(t, exitcodes.Usage, cliErr.Code) +} + +func TestProcessFlagUpdateNATGatewayInput_RejectsEmptyTagKey(t *testing.T) { + cmd := newUpdateFlagsCmd() + require.NoError(t, cmd.Flags().Set("resource-tags", `{"":"x"}`)) + + _, err := processFlagUpdateNATGatewayInput(cmd, "uid-empty-tag") + require.Error(t, err) + + var cliErr *exitcodes.CLIError + require.True(t, errors.As(err, &cliErr)) + assert.Equal(t, exitcodes.Usage, cliErr.Code) } diff --git a/internal/commands/nat_gateway/nat_gateway_prompts.go b/internal/commands/nat_gateway/nat_gateway_prompts.go index c612cb3d..7637805d 100644 --- a/internal/commands/nat_gateway/nat_gateway_prompts.go +++ b/internal/commands/nat_gateway/nat_gateway_prompts.go @@ -216,12 +216,14 @@ func promptForUpdateNATGatewayDetails(uid string, noColor bool) (*megaport.Updat return nil, explicit, err } req.PromoCode = strings.TrimSpace(promoCode) + explicit.PromoCode = req.PromoCode != "" serviceLevelRef, err := utils.ResourcePrompt("nat-gateway", "Service level reference (leave empty to keep current): ", noColor) if err != nil { return nil, explicit, err } req.ServiceLevelReference = strings.TrimSpace(serviceLevelRef) + explicit.ServiceLevelReference = req.ServiceLevelReference != "" tagsMap, err := utils.ResourceTagsPrompt(noColor) if err != nil { diff --git a/internal/commands/partners/partners_actions.go b/internal/commands/partners/partners_actions.go index 47e21f06..81816c01 100644 --- a/internal/commands/partners/partners_actions.go +++ b/internal/commands/partners/partners_actions.go @@ -2,6 +2,7 @@ package partners import ( "fmt" + "strings" "github.com/megaport/megaport-cli/internal/base/output" "github.com/megaport/megaport-cli/internal/commands/config" @@ -106,15 +107,17 @@ func FindPartners(cmd *cobra.Command, args []string, noColor bool) error { return err } - format, err := utils.Prompt("Output format [table/json] (default: table): ", noColor) + format, err := utils.Prompt("Output format [table/json/csv/xml] (default: table): ", noColor) if err != nil { output.PrintError("Failed to get output format: %v", noColor, err) return err } + normalizedFormat := strings.ToLower(strings.TrimSpace(format)) selectedFormat := "table" - if format == "json" { - selectedFormat = "json" + switch normalizedFormat { + case "json", "csv", "xml": + selectedFormat = normalizedFormat } filteredPartners := filterPartners(partners, productName, connectType, companyName, locationID, diversityZone) diff --git a/internal/commands/partners/partners_actions_test.go b/internal/commands/partners/partners_actions_test.go index fa03a206..7ff5bc40 100644 --- a/internal/commands/partners/partners_actions_test.go +++ b/internal/commands/partners/partners_actions_test.go @@ -29,11 +29,12 @@ func TestFindPartners(t *testing.T) { }() tests := []struct { - name string - prompts []string - expectedError string - setupMock func(*testing.T, *MockPartnerService) - expectedCount int + name string + prompts []string + expectedError string + setupMock func(*testing.T, *MockPartnerService) + expectedCount int + expectedFormat string }{ { name: "successful search with all filters", @@ -129,6 +130,70 @@ func TestFindPartners(t *testing.T) { }, expectedCount: 0, }, + { + name: "uppercase format is normalized", + prompts: []string{ + "", "", "", "", "", + "JSON", + }, + expectedError: "", + setupMock: func(t *testing.T, m *MockPartnerService) { + m.listPartnersResponse = []*megaport.PartnerMegaport{ + {ProductName: "Test Product", ConnectType: "AWS", CompanyName: "Amazon", LocationId: 123, DiversityZone: "blue"}, + } + m.listPartnersErr = nil + }, + expectedCount: 1, + expectedFormat: "json", + }, + { + name: "format with surrounding whitespace is normalized", + prompts: []string{ + "", "", "", "", "", + " table ", + }, + expectedError: "", + setupMock: func(t *testing.T, m *MockPartnerService) { + m.listPartnersResponse = []*megaport.PartnerMegaport{ + {ProductName: "Test Product", ConnectType: "AWS", CompanyName: "Amazon", LocationId: 123, DiversityZone: "blue"}, + } + m.listPartnersErr = nil + }, + expectedCount: 1, + expectedFormat: "table", + }, + { + name: "csv format is honored", + prompts: []string{ + "", "", "", "", "", + "csv", + }, + expectedError: "", + setupMock: func(t *testing.T, m *MockPartnerService) { + m.listPartnersResponse = []*megaport.PartnerMegaport{ + {ProductName: "Test Product", ConnectType: "AWS", CompanyName: "Amazon", LocationId: 123, DiversityZone: "blue"}, + } + m.listPartnersErr = nil + }, + expectedCount: 1, + expectedFormat: "csv", + }, + { + name: "unrecognized format falls back to table", + prompts: []string{ + "", "", "", "", "", + "yaml", + }, + expectedError: "", + setupMock: func(t *testing.T, m *MockPartnerService) { + m.listPartnersResponse = []*megaport.PartnerMegaport{ + {ProductName: "Test Product", ConnectType: "AWS", CompanyName: "Amazon", LocationId: 123, DiversityZone: "blue"}, + } + m.listPartnersErr = nil + }, + expectedCount: 1, + expectedFormat: "table", + }, } for _, tt := range tests { @@ -158,8 +223,10 @@ func TestFindPartners(t *testing.T) { }) var capturedPartners []*megaport.PartnerMegaport + var capturedFormat string printPartnersFunc = func(partners []*megaport.PartnerMegaport, format string, noColor bool) error { capturedPartners = partners + capturedFormat = format return nil } @@ -173,6 +240,9 @@ func TestFindPartners(t *testing.T) { assert.NoError(t, err) assert.Equal(t, len(tt.prompts), promptIndex, "not all prompts were used") assert.Equal(t, tt.expectedCount, len(capturedPartners), "incorrect number of filtered partners") + if tt.expectedFormat != "" { + assert.Equal(t, tt.expectedFormat, capturedFormat, "output format not normalized as expected") + } } }) } diff --git a/internal/commands/vxc/vxc_actions.go b/internal/commands/vxc/vxc_actions.go index 981a8be1..02dae62e 100644 --- a/internal/commands/vxc/vxc_actions.go +++ b/internal/commands/vxc/vxc_actions.go @@ -253,7 +253,9 @@ func buildVXCRequest(cmd *cobra.Command, ctx context.Context, client *megaport.C cmd.Flags().Changed("a-end-vlan") || cmd.Flags().Changed("b-end-uid") || cmd.Flags().Changed("b-end-vlan") }, - FromJSON: buildVXCRequestFromJSON, + FromJSON: func(jsonStr, jsonFile string) (*megaport.BuyVXCRequest, error) { + return buildVXCRequestFromJSON(jsonStr, jsonFile, ctx, client.VXCService) + }, FromFlags: func() (*megaport.BuyVXCRequest, error) { return buildVXCRequestFromFlags(cmd, ctx, client.VXCService) }, diff --git a/internal/commands/vxc/vxc_inputs.go b/internal/commands/vxc/vxc_inputs.go index 63d2f59a..8ffea98a 100644 --- a/internal/commands/vxc/vxc_inputs.go +++ b/internal/commands/vxc/vxc_inputs.go @@ -19,7 +19,7 @@ var buildVXCRequestFromFlags = func(cmd *cobra.Command, ctx context.Context, svc name, _ := cmd.Flags().GetString("name") if name == "" { - return nil, fmt.Errorf("name is required") + return nil, exitcodes.NewUsageError(validation.NewValidationError("VXC name", name, "cannot be empty")) } rateLimit, _ := cmd.Flags().GetInt("rate-limit") @@ -212,7 +212,7 @@ func parseVXCEndpointConfig(endConfigRaw map[string]interface{}, endLabel string return config, nil } -func buildVXCRequestFromJSON(jsonStr string, jsonFilePath string) (*megaport.BuyVXCRequest, error) { +func buildVXCRequestFromJSON(jsonStr string, jsonFilePath string, ctx context.Context, svc megaport.VXCService) (*megaport.BuyVXCRequest, error) { if jsonStr == "" && jsonFilePath == "" { return nil, exitcodes.NewUsageError(fmt.Errorf("either json or json-file must be provided")) } @@ -228,42 +228,85 @@ func buildVXCRequestFromJSON(jsonStr string, jsonFilePath string) (*megaport.Buy return nil, exitcodes.NewUsageError(fmt.Errorf("failed to parse JSON: %w", err)) } - portUID, present, err := utils.JSONString(rawData, "portUid") - if err != nil { + // Parse A-End configuration early: its partner config, if present, can + // resolve a missing top-level portUid the same way flags mode does. + var aEndConfig megaport.VXCOrderEndpointConfiguration + if aEndConfigRaw, present, err := utils.JSONObject(rawData, "aEndConfiguration"); err != nil { return nil, exitcodes.NewUsageError(err) - } - if !present { - return nil, exitcodes.NewUsageError(validation.NewValidationError("portUid", "", "Port UID is required")) + } else if present { + aEndConfig, err = parseVXCEndpointConfig(aEndConfigRaw, "A-End") + if err != nil { + return nil, err + } } - // Create the base request - req := &megaport.BuyVXCRequest{ - PortUID: portUID, + // Parse and format-check vxcName/rateLimit/term before any partner-port + // lookup, so malformed JSON fails fast without an API round-trip. + vxcName, _, err := utils.JSONString(rawData, "vxcName") + if err != nil { + return nil, exitcodes.NewUsageError(err) } - // Set simple fields - if vxcName, present, err := utils.JSONString(rawData, "vxcName"); err != nil { + rateLimit, _, err := utils.JSONNumber(rawData, "rateLimit") + if err != nil { return nil, exitcodes.NewUsageError(err) - } else if present { - req.VXCName = vxcName + } + if rateLimit != math.Trunc(rateLimit) { + return nil, exitcodes.NewUsageError(fmt.Errorf("rateLimit must be a whole number, got %v", rateLimit)) } - if rateLimit, present, err := utils.JSONNumber(rawData, "rateLimit"); err != nil { + term, _, err := utils.JSONNumber(rawData, "term") + if err != nil { return nil, exitcodes.NewUsageError(err) - } else if present { - if rateLimit != math.Trunc(rateLimit) { - return nil, exitcodes.NewUsageError(fmt.Errorf("rateLimit must be a whole number, got %v", rateLimit)) + } + if term != math.Trunc(term) { + return nil, exitcodes.NewUsageError(fmt.Errorf("term must be a whole number, got %v", term)) + } + + // Required fields the buy request can't do without, checked before any + // partner-port lookup so a partial JSON body fails fast rather than + // spending an API round-trip on a request that will fail validation anyway. + requireBuyFields := func() error { + if vxcName == "" { + return exitcodes.NewUsageError(validation.NewValidationError("VXC name", vxcName, "cannot be empty")) + } + if err := validation.ValidateRateLimit(int(rateLimit)); err != nil { + return exitcodes.NewUsageError(err) } - req.RateLimit = int(rateLimit) + if err := validation.ValidateContractTerm(int(term)); err != nil { + return exitcodes.NewUsageError(err) + } + return nil } - if term, present, err := utils.JSONNumber(rawData, "term"); err != nil { + portUID, present, err := utils.JSONString(rawData, "portUid") + if err != nil { return nil, exitcodes.NewUsageError(err) - } else if present { - if term != math.Trunc(term) { - return nil, exitcodes.NewUsageError(fmt.Errorf("term must be a whole number, got %v", term)) + } + if !present { + if aEndConfig.PartnerConfig == nil { + return nil, exitcodes.NewUsageError(validation.NewValidationError("portUid", "", "Port UID is required")) + } + if err := requireBuyFields(); err != nil { + return nil, err + } + uid, err := resolvePartnerPortUID(ctx, svc, aEndConfig.PartnerConfig) + if err != nil { + return nil, fmt.Errorf("failed to look up A-End Partner Port: %w", err) } - req.Term = int(term) + if uid == "" { + return nil, exitcodes.NewUsageError(validation.NewValidationError("portUid", "", "Port UID is required")) + } + portUID = uid + } + + // Create the base request + req := &megaport.BuyVXCRequest{ + PortUID: portUID, + AEndConfiguration: aEndConfig, + VXCName: vxcName, + RateLimit: int(rateLimit), + Term: int(term), } if shutdown, present, err := utils.JSONBool(rawData, "shutdown"); err != nil { @@ -301,18 +344,9 @@ func buildVXCRequestFromJSON(jsonStr string, jsonFilePath string) (*megaport.Buy req.ResourceTags = tags } - // Handle A-End configuration - if aEndConfigRaw, present, err := utils.JSONObject(rawData, "aEndConfiguration"); err != nil { - return nil, exitcodes.NewUsageError(err) - } else if present { - aEndConfig, err := parseVXCEndpointConfig(aEndConfigRaw, "A-End") - if err != nil { - return nil, err - } - req.AEndConfiguration = aEndConfig - } - - // Handle B-End configuration + // Handle B-End configuration. Resolve a missing productUID from the + // partner config the same way flags mode does, so the same logical + // purchase succeeds through either input mode. if bEndConfigRaw, present, err := utils.JSONObject(rawData, "bEndConfiguration"); err != nil { return nil, exitcodes.NewUsageError(err) } else if present { @@ -320,6 +354,19 @@ func buildVXCRequestFromJSON(jsonStr string, jsonFilePath string) (*megaport.Buy if err != nil { return nil, err } + if bEndConfig.ProductUID == "" && bEndConfig.PartnerConfig != nil { + if err := requireBuyFields(); err != nil { + return nil, err + } + uid, err := resolvePartnerPortUID(ctx, svc, bEndConfig.PartnerConfig) + if err != nil { + return nil, fmt.Errorf("failed to look up B-End Partner Port: %w", err) + } + if uid == "" { + return nil, exitcodes.NewUsageError(fmt.Errorf("bEndConfiguration.productUID was neither provided nor could be looked up")) + } + bEndConfig.ProductUID = uid + } req.BEndConfiguration = bEndConfig } diff --git a/internal/commands/vxc/vxc_inputs_test.go b/internal/commands/vxc/vxc_inputs_test.go index b9aa16a7..83653404 100644 --- a/internal/commands/vxc/vxc_inputs_test.go +++ b/internal/commands/vxc/vxc_inputs_test.go @@ -1289,7 +1289,7 @@ func TestBuildVXCRequestFromJSON(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := buildVXCRequestFromJSON(tt.jsonStr, tt.jsonFilePath) + result, err := buildVXCRequestFromJSON(tt.jsonStr, tt.jsonFilePath, context.Background(), &MockVXCService{}) if tt.expectedError != "" { assert.Error(t, err) assert.Contains(t, err.Error(), tt.expectedError) @@ -1311,7 +1311,7 @@ func TestBuildVXCRequestFromJSON_RejectsEmptyTagKey(t *testing.T) { const payload = `{"portUid":"port-1","vxcName":"Test VXC","rateLimit":1000,"term":12,"resourceTags":{"":"x"},"bEndConfiguration":{"productUID":"port-2"}}` t.Run("via json", func(t *testing.T) { - _, err := buildVXCRequestFromJSON(payload, "") + _, err := buildVXCRequestFromJSON(payload, "", context.Background(), &MockVXCService{}) require.Error(t, err) assert.Contains(t, err.Error(), "tag key must not be empty") @@ -1328,7 +1328,7 @@ func TestBuildVXCRequestFromJSON_RejectsEmptyTagKey(t *testing.T) { require.NoError(t, err) require.NoError(t, tmp.Close()) - _, err = buildVXCRequestFromJSON("", tmp.Name()) + _, err = buildVXCRequestFromJSON("", tmp.Name(), context.Background(), &MockVXCService{}) require.Error(t, err) assert.Contains(t, err.Error(), "tag key must not be empty") @@ -1343,7 +1343,7 @@ func TestBuildVXCRequestFromJSON_EndpointConfigMalformedField(t *testing.T) { // top-level field checks in buildVXCRequestFromJSON. payload := `{"portUid":"port-1","rateLimit":1000,"term":12,"aEndConfiguration":{"vlan":"not-a-number"}}` - _, err := buildVXCRequestFromJSON(payload, "") + _, err := buildVXCRequestFromJSON(payload, "", context.Background(), &MockVXCService{}) require.Error(t, err) assert.Contains(t, err.Error(), "A-End") @@ -1352,6 +1352,105 @@ func TestBuildVXCRequestFromJSON_EndpointConfigMalformedField(t *testing.T) { assert.Equal(t, exitcodes.Usage, cliErr.Code) } +func TestBuildVXCRequestFromJSON_PartnerPortResolution(t *testing.T) { + t.Run("A-End portUid resolved from partner config when omitted", func(t *testing.T) { + payload := `{"vxcName":"Test VXC","rateLimit":1000,"term":12,"aEndConfiguration":{"partnerConfig":{"connectType":"AZURE","serviceKey":"azure-key"}},"bEndConfiguration":{"productUID":"port-2"}}` + svc := &MockVXCService{ + LookupPartnerPortsResponse: &megaport.LookupPartnerPortsResponse{ProductUID: "resolved-a-end"}, + } + + req, err := buildVXCRequestFromJSON(payload, "", context.Background(), svc) + require.NoError(t, err) + assert.Equal(t, "resolved-a-end", req.PortUID) + }) + + t.Run("A-End portUid lookup failure surfaces as error", func(t *testing.T) { + payload := `{"vxcName":"Test VXC","rateLimit":1000,"term":12,"aEndConfiguration":{"partnerConfig":{"connectType":"AZURE","serviceKey":"azure-key"}},"bEndConfiguration":{"productUID":"port-2"}}` + svc := &MockVXCService{ + LookupPartnerPortsError: fmt.Errorf("lookup failed"), + } + + _, err := buildVXCRequestFromJSON(payload, "", context.Background(), svc) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to look up A-End Partner Port") + }) + + t.Run("malformed rateLimit fails before A-End partner-port lookup", func(t *testing.T) { + payload := `{"vxcName":"Test VXC","rateLimit":1000.5,"term":12,"aEndConfiguration":{"partnerConfig":{"connectType":"AZURE","serviceKey":"azure-key"}},"bEndConfiguration":{"productUID":"port-2"}}` + svc := &MockVXCService{ + LookupPartnerPortsError: fmt.Errorf("lookup should not have been called"), + } + + _, err := buildVXCRequestFromJSON(payload, "", context.Background(), svc) + require.Error(t, err) + assert.Contains(t, err.Error(), "rateLimit must be a whole number") + }) + + t.Run("missing vxcName fails before A-End partner-port lookup", func(t *testing.T) { + payload := `{"rateLimit":1000,"term":12,"aEndConfiguration":{"partnerConfig":{"connectType":"AZURE","serviceKey":"azure-key"}},"bEndConfiguration":{"productUID":"port-2"}}` + svc := &MockVXCService{ + LookupPartnerPortsError: fmt.Errorf("lookup should not have been called"), + } + + _, err := buildVXCRequestFromJSON(payload, "", context.Background(), svc) + require.Error(t, err) + assert.Contains(t, err.Error(), "VXC name") + }) + + t.Run("missing vxcName fails before B-End partner-port lookup", func(t *testing.T) { + payload := `{"portUid":"port-1","rateLimit":1000,"term":12,"bEndConfiguration":{"partnerConfig":{"connectType":"GOOGLE","pairingKey":"google-key"}}}` + svc := &MockVXCService{ + LookupPartnerPortsError: fmt.Errorf("lookup should not have been called"), + } + + _, err := buildVXCRequestFromJSON(payload, "", context.Background(), svc) + require.Error(t, err) + assert.Contains(t, err.Error(), "VXC name") + }) + + t.Run("B-End productUID resolved from partner config when omitted", func(t *testing.T) { + payload := `{"portUid":"port-1","vxcName":"Test VXC","rateLimit":1000,"term":12,"bEndConfiguration":{"partnerConfig":{"connectType":"GOOGLE","pairingKey":"google-key"}}}` + svc := &MockVXCService{ + LookupPartnerPortsResponse: &megaport.LookupPartnerPortsResponse{ProductUID: "resolved-b-end"}, + } + + req, err := buildVXCRequestFromJSON(payload, "", context.Background(), svc) + require.NoError(t, err) + assert.Equal(t, "resolved-b-end", req.BEndConfiguration.ProductUID) + }) + + t.Run("B-End productUID lookup failure surfaces as error", func(t *testing.T) { + payload := `{"portUid":"port-1","vxcName":"Test VXC","rateLimit":1000,"term":12,"bEndConfiguration":{"partnerConfig":{"connectType":"GOOGLE","pairingKey":"google-key"}}}` + svc := &MockVXCService{ + LookupPartnerPortsError: fmt.Errorf("lookup failed"), + } + + _, err := buildVXCRequestFromJSON(payload, "", context.Background(), svc) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to look up B-End Partner Port") + }) + + t.Run("A-End partner config that resolves to no UID still requires portUid", func(t *testing.T) { + payload := `{"vxcName":"Test VXC","rateLimit":1000,"term":12,"aEndConfiguration":{"partnerConfig":{"connectType":"AWS","ownerAccount":"123"}},"bEndConfiguration":{"productUID":"port-2"}}` + + _, err := buildVXCRequestFromJSON(payload, "", context.Background(), &MockVXCService{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "Port UID is required") + }) + + t.Run("B-End partner config that resolves to no UID still requires productUID", func(t *testing.T) { + payload := `{"portUid":"port-1","vxcName":"Test VXC","rateLimit":1000,"term":12,"bEndConfiguration":{"partnerConfig":{"connectType":"AWS","ownerAccount":"123"}}}` + + _, err := buildVXCRequestFromJSON(payload, "", context.Background(), &MockVXCService{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "productUID was neither provided nor could be looked up") + + var cliErr *exitcodes.CLIError + require.True(t, errors.As(err, &cliErr), "expected a *exitcodes.CLIError, got %T: %v", err, err) + assert.Equal(t, exitcodes.Usage, cliErr.Code) + }) +} + func TestBuildUpdateVXCRequestFromJSON_PartnerConfigs(t *testing.T) { tests := []struct { name string diff --git a/internal/commands/vxc/vxc_prompts.go b/internal/commands/vxc/vxc_prompts.go index 25602106..b0d89ff0 100644 --- a/internal/commands/vxc/vxc_prompts.go +++ b/internal/commands/vxc/vxc_prompts.go @@ -245,6 +245,8 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport return nil, fmt.Errorf("failed to fetch VXC details: %w", err) } + fieldsUpdated := false + fmt.Fprintf(os.Stderr, "Current name: %s\n", vxc.Name) updateName, err := utils.ResourcePrompt("vxc", "Update name? (yes/no): ", noColor) if err != nil { @@ -255,7 +257,11 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport if err != nil { return nil, err } + if name == "" { + return nil, validation.NewValidationError("VXC name", name, "cannot be empty") + } req.Name = &name + fieldsUpdated = true } fmt.Fprintf(os.Stderr, "Current rate limit: %d Mbps\n", vxc.RateLimit) @@ -276,6 +282,7 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport return nil, err } req.RateLimit = &rateLimit + fieldsUpdated = true } fmt.Fprintf(os.Stderr, "Current term: %d months\n", vxc.ContractTermMonths) @@ -296,6 +303,7 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport return nil, err } req.Term = &term + fieldsUpdated = true } fmt.Fprintf(os.Stderr, "Current cost centre: %s\n", vxc.CostCentre) @@ -309,6 +317,7 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport return nil, err } req.CostCentre = &costCentre + fieldsUpdated = true } shutdownStatus := "No" @@ -327,6 +336,7 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport } shutdown := strings.ToLower(shutdownStr) == "yes" req.Shutdown = &shutdown + fieldsUpdated = true } fmt.Fprintf(os.Stderr, "Current A-End VLAN: %d\n", vxc.AEndConfiguration.VLAN) @@ -347,6 +357,7 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport return nil, err } req.AEndVLAN = &aEndVLAN + fieldsUpdated = true } fmt.Fprintf(os.Stderr, "Current B-End VLAN: %d\n", vxc.BEndConfiguration.VLAN) @@ -367,6 +378,7 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport return nil, err } req.BEndVLAN = &bEndVLAN + fieldsUpdated = true } innerVLANAEnd := 0 @@ -393,6 +405,7 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport } req.AEndInnerVLAN = &aEndInnerVLAN + fieldsUpdated = true } innerVLANBEnd := 0 @@ -419,6 +432,7 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport } req.BEndInnerVLAN = &bEndInnerVLAN + fieldsUpdated = true } fmt.Fprintf(os.Stderr, "Current A-End UID: %s\n", vxc.AEndConfiguration.UID) @@ -432,6 +446,7 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport return nil, err } req.AEndProductUID = &aEndUID + fieldsUpdated = true } fmt.Fprintf(os.Stderr, "Current B-End UID: %s\n", vxc.BEndConfiguration.UID) @@ -445,6 +460,7 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport return nil, err } req.BEndProductUID = &bEndUID + fieldsUpdated = true } wantsAEndPartnerConfig, err := utils.ResourcePrompt("vxc", "Do you want to configure an A-End VRouter partner configuration? (yes/no): ", noColor) @@ -458,6 +474,7 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport return nil, err } req.AEndPartnerConfig = aEndPartnerConfig + fieldsUpdated = true } wantsBEndPartnerConfig, err := utils.ResourcePrompt("vxc", "Do you want to configure a B-End VRouter partner configuration? (yes/no): ", noColor) @@ -471,6 +488,11 @@ var buildUpdateVXCRequestFromPrompt = func(ctx context.Context, client *megaport return nil, err } req.BEndPartnerConfig = bEndPartnerConfig + fieldsUpdated = true + } + + if !fieldsUpdated { + return nil, fmt.Errorf("at least one field must be updated") } return req, nil diff --git a/internal/commands/vxc/vxc_prompts_test.go b/internal/commands/vxc/vxc_prompts_test.go index dfa00a5f..c733f172 100644 --- a/internal/commands/vxc/vxc_prompts_test.go +++ b/internal/commands/vxc/vxc_prompts_test.go @@ -989,9 +989,10 @@ func TestBuildUpdateVXCRequestFromPrompt(t *testing.T) { } tests := []struct { - name string - responses []string - verify func(t *testing.T, req *megaport.UpdateVXCRequest) + name string + responses []string + expectedError string + verify func(t *testing.T, req *megaport.UpdateVXCRequest) }{ { name: "skip all", @@ -1010,21 +1011,7 @@ func TestBuildUpdateVXCRequestFromPrompt(t *testing.T) { "no", // A-End VRouter config "no", // B-End VRouter config }, - verify: func(t *testing.T, req *megaport.UpdateVXCRequest) { - assert.Nil(t, req.Name) - assert.Nil(t, req.RateLimit) - assert.Nil(t, req.Term) - assert.Nil(t, req.CostCentre) - assert.Nil(t, req.Shutdown) - assert.Nil(t, req.AEndVLAN) - assert.Nil(t, req.BEndVLAN) - assert.Nil(t, req.AEndInnerVLAN) - assert.Nil(t, req.BEndInnerVLAN) - assert.Nil(t, req.AEndProductUID) - assert.Nil(t, req.BEndProductUID) - assert.True(t, req.WaitForUpdate) - // WaitForTime is set by the caller (UpdateVXC), not the prompt builder - }, + expectedError: "at least one field must be updated", }, { name: "update name and rate limit", @@ -1092,6 +1079,71 @@ func TestBuildUpdateVXCRequestFromPrompt(t *testing.T) { assert.Equal(t, -1, vrouterCfg.Interfaces[0].VLAN) }, }, + { + name: "empty name rejected", + responses: []string{ + "yes", // update name + "", // new name (empty) + }, + expectedError: "cannot be empty", + }, + { + name: "update remaining fields", + responses: []string{ + "no", // update name + "no", // update rate limit + "yes", // update term + "24", // new term + "yes", // update cost centre + "New CC", // new cost centre + "yes", // update shutdown + "yes", // shut down the VXC + "yes", // update A-End VLAN + "101", // new A-End VLAN + "yes", // update B-End VLAN + "201", // new B-End VLAN + "yes", // update A-End inner VLAN + "301", // new A-End inner VLAN + "yes", // update B-End inner VLAN + "401", // new B-End inner VLAN + "yes", // update A-End product UID + "new-a-uid", // new A-End product UID + "yes", // update B-End product UID + "new-b-uid", // new B-End product UID + "no", // A-End VRouter config + "yes", // B-End VRouter config + "1", // number of interfaces + "", // VLAN (untagged) + "", // interface type (default subInterface) + "no", // add IP addresses + "no", // add IP routes + "no", // add NAT IPs + "no", // configure BFD + "no", // configure BGP + }, + verify: func(t *testing.T, req *megaport.UpdateVXCRequest) { + assert.NotNil(t, req.Term) + assert.Equal(t, 24, *req.Term) + assert.NotNil(t, req.CostCentre) + assert.Equal(t, "New CC", *req.CostCentre) + assert.NotNil(t, req.Shutdown) + assert.True(t, *req.Shutdown) + assert.NotNil(t, req.AEndVLAN) + assert.Equal(t, 101, *req.AEndVLAN) + assert.NotNil(t, req.BEndVLAN) + assert.Equal(t, 201, *req.BEndVLAN) + assert.NotNil(t, req.AEndInnerVLAN) + assert.Equal(t, 301, *req.AEndInnerVLAN) + assert.NotNil(t, req.BEndInnerVLAN) + assert.Equal(t, 401, *req.BEndInnerVLAN) + assert.NotNil(t, req.AEndProductUID) + assert.Equal(t, "new-a-uid", *req.AEndProductUID) + assert.NotNil(t, req.BEndProductUID) + assert.Equal(t, "new-b-uid", *req.BEndProductUID) + assert.Nil(t, req.AEndPartnerConfig) + assert.NotNil(t, req.BEndPartnerConfig) + }, + }, } for _, tc := range tests { @@ -1105,6 +1157,11 @@ func TestBuildUpdateVXCRequestFromPrompt(t *testing.T) { mockClient := &megaport.Client{VXCService: mockSvc} req, err := buildUpdateVXCRequestFromPrompt(context.Background(), mockClient, "vxc-uid-123", true) + if tc.expectedError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedError) + return + } assert.NoError(t, err) assert.NotNil(t, req) tc.verify(t, req) @@ -1114,19 +1171,20 @@ func TestBuildUpdateVXCRequestFromPrompt(t *testing.T) { func TestBuildUpdateVXCRequestFromPromptShutdownStatusDisplay(t *testing.T) { responses := []string{ - "no", // update name - "no", // update rate limit - "no", // update term - "no", // update cost centre - "no", // update shutdown - "no", // update A-End VLAN - "no", // update B-End VLAN - "no", // update A-End inner VLAN - "no", // update B-End inner VLAN - "no", // update A-End UID - "no", // update B-End UID - "no", // A-End VRouter config - "no", // B-End VRouter config + "yes", // update name + "New VXC Name", // new name + "no", // update rate limit + "no", // update term + "no", // update cost centre + "no", // update shutdown + "no", // update A-End VLAN + "no", // update B-End VLAN + "no", // update A-End inner VLAN + "no", // update B-End inner VLAN + "no", // update A-End UID + "no", // update B-End UID + "no", // A-End VRouter config + "no", // B-End VRouter config } tests := []struct {