Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions internal/commands/ix/ix_inputs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand All @@ -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"))
}
Comment thread
Phil-Browne marked this conversation as resolved.

return req, nil
}
49 changes: 49 additions & 0 deletions internal/commands/ix/ix_inputs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
Expand Down Expand Up @@ -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", "", "")
Expand Down
14 changes: 8 additions & 6 deletions internal/commands/ix/ix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
15 changes: 14 additions & 1 deletion internal/commands/managed_account/managed_account_inputs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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"))
}
Comment thread
Phil-Browne marked this conversation as resolved.

return req, nil
}

// buildUpdateManagedAccountRequestFromFlags seeds the request from the current
Expand Down
59 changes: 29 additions & 30 deletions internal/commands/managed_account/managed_account_inputs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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)
}
})
}
}
Expand All @@ -148,25 +153,19 @@ 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",
jsonStr: `{invalid json}`,
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",
Expand Down
16 changes: 14 additions & 2 deletions internal/commands/mve/mve_inputs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
14 changes: 14 additions & 0 deletions internal/commands/mve/mve_inputs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
12 changes: 12 additions & 0 deletions internal/commands/nat_gateway/nat_gateway_actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
Loading
Loading