From a29aec3302d56965d44ae7585fe50431f3fe4ec9 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Thu, 9 Jul 2026 17:31:54 -0700 Subject: [PATCH 1/2] ESD-1615-fix(nat-gateway): validate speed/session matrix on update path The speed/session-count availability matrix check ran only on create. Update accepts --speed/--session-count too, so an unsupported pair on update sailed past client-side checks straight to the API. Refactor the validator to take speed/session-count directly and call it from UpdateNATGateway after the partial-update merge, so it checks the effective values. --- .../nat_gateway/nat_gateway_actions.go | 19 +++-- .../commands/nat_gateway/nat_gateway_test.go | 85 +++++++++++++++++++ 2 files changed, 97 insertions(+), 7 deletions(-) diff --git a/internal/commands/nat_gateway/nat_gateway_actions.go b/internal/commands/nat_gateway/nat_gateway_actions.go index f3e9c235..3e84624e 100644 --- a/internal/commands/nat_gateway/nat_gateway_actions.go +++ b/internal/commands/nat_gateway/nat_gateway_actions.go @@ -61,7 +61,7 @@ func CreateNATGateway(cmd *cobra.Command, args []string, noColor bool) error { return err } - if err := validateNATGatewaySpeedSessionMatrix(ctx, client, req, noColor); err != nil { + if err := validateNATGatewaySpeedSessionMatrix(ctx, client, req.Speed, req.Config.SessionCount, noColor); err != nil { output.PrintError("Validation failed: %v", noColor, err) return err } @@ -111,7 +111,7 @@ func CreateNATGateway(cmd *cobra.Command, args []string, noColor bool) error { // fetch failure or an empty matrix is treated as non-fatal (fail open): the // API still enforces the same rule at order time, matching the Terraform // provider's handling. -func validateNATGatewaySpeedSessionMatrix(ctx context.Context, client *megaport.Client, req *megaport.CreateNATGatewayRequest, noColor bool) error { +func validateNATGatewaySpeedSessionMatrix(ctx context.Context, client *megaport.Client, speed, sessionCount int, noColor bool) error { matrix, err := listNATGatewaySessionsFunc(ctx, client) if err != nil { output.PrintWarning("Could not validate speed/session count against the availability matrix: %v", noColor, err) @@ -121,18 +121,18 @@ func validateNATGatewaySpeedSessionMatrix(ctx context.Context, client *megaport. return nil } - result := megaport.NATGatewaySpeedSessionSupported(matrix, req.Speed, req.Config.SessionCount) + result := megaport.NATGatewaySpeedSessionSupported(matrix, speed, sessionCount) if result.Supported { return nil } if !result.SpeedSupported { - return validation.NewValidationError("speed", req.Speed, fmt.Sprintf("not supported; supported speeds (Mbps): %v", result.SupportedSpeeds)) + return validation.NewValidationError("speed", speed, fmt.Sprintf("not supported; supported speeds (Mbps): %v", result.SupportedSpeeds)) } - if req.Config.SessionCount == 0 { + if sessionCount == 0 { return nil } - return validation.NewValidationError("session count", req.Config.SessionCount, - fmt.Sprintf("not supported for %d Mbps; valid session counts: %v", req.Speed, result.SessionsAtSpeed)) + return validation.NewValidationError("session count", sessionCount, + fmt.Sprintf("not supported for %d Mbps; valid session counts: %v", speed, result.SessionsAtSpeed)) } // GetNATGateway handles the nat-gateway get command. @@ -316,6 +316,11 @@ func UpdateNATGateway(cmd *cobra.Command, args []string, noColor bool) error { return err } + if err := validateNATGatewaySpeedSessionMatrix(ctx, client, req.Speed, req.Config.SessionCount, noColor); err != nil { + output.PrintError("Validation failed: %v", noColor, err) + return err + } + spinner := output.PrintResourceUpdating("NAT Gateway", uid, noColor) var updatedGW *megaport.NATGateway err = utils.WithRetry(ctx, func(ctx context.Context) error { diff --git a/internal/commands/nat_gateway/nat_gateway_test.go b/internal/commands/nat_gateway/nat_gateway_test.go index 2673dc5a..36e5770a 100644 --- a/internal/commands/nat_gateway/nat_gateway_test.go +++ b/internal/commands/nat_gateway/nat_gateway_test.go @@ -437,6 +437,91 @@ func TestUpdateNATGateway_EmptyUIDResponse(t *testing.T) { assert.Contains(t, err.Error(), "no NAT Gateway UID") } +func TestUpdateNATGateway_MatrixRejectsUnsupportedSpeed(t *testing.T) { + mock := &MockNATGatewayService{ + GetResult: &megaport.NATGateway{ + ProductUID: "uid-upd", ProductName: "GW", + LocationID: 100, Speed: 1000, Term: 12, + }, + SessionsResult: []*megaport.NATGatewaySession{ + {SpeedMbps: 1000, SessionCount: []int{1, 2, 4}}, + }, + } + defer setupMockNATGateway(mock)() + + cmd := newTestCmd("update") + require.NoError(t, cmd.Flags().Set("speed", "5000")) + + err := UpdateNATGateway(cmd, []string{"uid-upd"}, true) + assert.Error(t, err) + assert.Contains(t, err.Error(), "speed") + assert.Contains(t, err.Error(), "1000") + assert.Nil(t, mock.CapturedUpdateReq) +} + +func TestUpdateNATGateway_MatrixRejectsUnsupportedSessionCount(t *testing.T) { + mock := &MockNATGatewayService{ + GetResult: &megaport.NATGateway{ + ProductUID: "uid-upd", ProductName: "GW", + LocationID: 100, Speed: 1000, Term: 12, + }, + SessionsResult: []*megaport.NATGatewaySession{ + {SpeedMbps: 1000, SessionCount: []int{1, 2, 4}}, + }, + } + defer setupMockNATGateway(mock)() + + cmd := newTestCmd("update") + require.NoError(t, cmd.Flags().Set("session-count", "3")) + + err := UpdateNATGateway(cmd, []string{"uid-upd"}, true) + assert.Error(t, err) + assert.Contains(t, err.Error(), "session count") + assert.Contains(t, err.Error(), "[1 2 4]") + assert.Nil(t, mock.CapturedUpdateReq) +} + +func TestUpdateNATGateway_MatrixAllowsSupportedPair(t *testing.T) { + mock := &MockNATGatewayService{ + GetResult: &megaport.NATGateway{ + ProductUID: "uid-upd", ProductName: "GW", + LocationID: 100, Speed: 1000, Term: 12, + }, + UpdateResult: &megaport.NATGateway{ProductUID: "uid-upd"}, + SessionsResult: []*megaport.NATGatewaySession{ + {SpeedMbps: 1000, SessionCount: []int{1, 2, 4}}, + }, + } + defer setupMockNATGateway(mock)() + + cmd := newTestCmd("update") + require.NoError(t, cmd.Flags().Set("session-count", "2")) + + err := UpdateNATGateway(cmd, []string{"uid-upd"}, true) + assert.NoError(t, err) + require.NotNil(t, mock.CapturedUpdateReq) + assert.Equal(t, 2, mock.CapturedUpdateReq.Config.SessionCount) +} + +func TestUpdateNATGateway_MatrixFetchFailureFallsThrough(t *testing.T) { + mock := &MockNATGatewayService{ + GetResult: &megaport.NATGateway{ + ProductUID: "uid-upd", ProductName: "GW", + LocationID: 100, Speed: 1000, Term: 12, + }, + UpdateResult: &megaport.NATGateway{ProductUID: "uid-upd"}, + SessionsErr: fmt.Errorf("matrix unavailable"), + } + defer setupMockNATGateway(mock)() + + cmd := newTestCmd("update") + require.NoError(t, cmd.Flags().Set("speed", "5000")) + + err := UpdateNATGateway(cmd, []string{"uid-upd"}, true) + assert.NoError(t, err) + require.NotNil(t, mock.CapturedUpdateReq) +} + // ---- Delete ---- func TestDeleteNATGateway_Force(t *testing.T) { From 9df9901121d2d73f15ad4e058a661c8ca4146f6b Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Mon, 13 Jul 2026 07:17:11 -0700 Subject: [PATCH 2/2] ESD-1615: skip matrix validation on update when speed/session unchanged UpdateNATGateway ran the speed/session matrix check unconditionally after merging defaults, so a grandfathered gateway whose speed/session pair was later dropped from the matrix would fail a name/term/auto-renew-only update even though speed and session count weren't touched. Only validate when the post-merge speed or session count actually differs from the original. --- .../nat_gateway/nat_gateway_actions.go | 13 ++++++--- .../commands/nat_gateway/nat_gateway_test.go | 27 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/internal/commands/nat_gateway/nat_gateway_actions.go b/internal/commands/nat_gateway/nat_gateway_actions.go index 3e84624e..28dc46f1 100644 --- a/internal/commands/nat_gateway/nat_gateway_actions.go +++ b/internal/commands/nat_gateway/nat_gateway_actions.go @@ -316,9 +316,16 @@ func UpdateNATGateway(cmd *cobra.Command, args []string, noColor bool) error { return err } - if err := validateNATGatewaySpeedSessionMatrix(ctx, client, req.Speed, req.Config.SessionCount, noColor); err != nil { - output.PrintError("Validation failed: %v", noColor, err) - return err + // Only re-validate against the matrix if speed or session count is + // actually changing. A grandfathered gateway may hold a speed/session + // pair the matrix no longer lists; leaving it untouched must not fail + // an update that only touches unrelated fields. + speedOrSessionChanged := req.Speed != originalGW.Speed || req.Config.SessionCount != originalGW.Config.SessionCount + if speedOrSessionChanged { + if err := validateNATGatewaySpeedSessionMatrix(ctx, client, req.Speed, req.Config.SessionCount, noColor); err != nil { + output.PrintError("Validation failed: %v", noColor, err) + return err + } } spinner := output.PrintResourceUpdating("NAT Gateway", uid, noColor) diff --git a/internal/commands/nat_gateway/nat_gateway_test.go b/internal/commands/nat_gateway/nat_gateway_test.go index 36e5770a..306d122d 100644 --- a/internal/commands/nat_gateway/nat_gateway_test.go +++ b/internal/commands/nat_gateway/nat_gateway_test.go @@ -522,6 +522,33 @@ func TestUpdateNATGateway_MatrixFetchFailureFallsThrough(t *testing.T) { require.NotNil(t, mock.CapturedUpdateReq) } +func TestUpdateNATGateway_MatrixSkippedWhenSpeedAndSessionUnchanged(t *testing.T) { + mock := &MockNATGatewayService{ + GetResult: &megaport.NATGateway{ + ProductUID: "uid-upd", ProductName: "Old Name", + LocationID: 100, Speed: 1000, Term: 12, + Config: megaport.NATGatewayNetworkConfig{SessionCount: 4}, + }, + UpdateResult: &megaport.NATGateway{ProductUID: "uid-upd", ProductName: "New Name"}, + // Matrix no longer lists the grandfathered 1000 Mbps / 4-session pair, + // so validation would reject it if it ran. + SessionsResult: []*megaport.NATGatewaySession{ + {SpeedMbps: 2000, SessionCount: []int{1, 2, 4}}, + }, + } + defer setupMockNATGateway(mock)() + + cmd := newTestCmd("update") + require.NoError(t, cmd.Flags().Set("name", "New Name")) + + err := UpdateNATGateway(cmd, []string{"uid-upd"}, true) + assert.NoError(t, err) + require.NotNil(t, mock.CapturedUpdateReq) + assert.Equal(t, "New Name", mock.CapturedUpdateReq.ProductName) + assert.Equal(t, 1000, mock.CapturedUpdateReq.Speed) + assert.Equal(t, 4, mock.CapturedUpdateReq.Config.SessionCount) +} + // ---- Delete ---- func TestDeleteNATGateway_Force(t *testing.T) {