Skip to content

ESD-1615-fix(nat-gateway): validate speed/session matrix on update path #515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Phil-Browne wants to merge 5 commits into
base: main
Choose a base branch
from
Open

ESD-1615-fix(nat-gateway): validate speed/session matrix on update path #515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a29aec3
ESD-1615-fix(nat-gateway): validate speed/session matrix on update path
Phil-Browne Jul 10, 2026
9df9901
ESD-1615: skip matrix validation on update when speed/session unchanged
Phil-Browne Jul 13, 2026
b5fad8d
Merge remote-tracking branch 'origin/main' into worktree-ESD-1615-nat…
Phil-Browne Jul 15, 2026
60ca995
Merge remote-tracking branch 'origin/main' into worktree-ESD-1615-nat…
Phil-Browne Jul 15, 2026
e124ee9
Merge branch 'main' into worktree-ESD-1615-nat-gateway-update-matrix
Phil-Browne Jul 27, 2026
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
26 changes: 19 additions & 7 deletions internal/commands/nat_gateway/nat_gateway_actions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand Down Expand Up @@ -316,6 +316,18 @@ func UpdateNATGateway(cmd *cobra.Command, args []string, noColor bool) error {
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)
var updatedGW *megaport.NATGateway
err = utils.WithRetry(ctx, func(ctx context.Context) error {
Expand Down
112 changes: 112 additions & 0 deletions internal/commands/nat_gateway/nat_gateway_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,118 @@ 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)
}

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) {
Expand Down
Loading