From 6086a96344f38dbe1c27a4ddd62322ef5bf0a090 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Tue, 21 Jul 2026 08:12:11 -0700 Subject: [PATCH 1/4] ESD-1676: Fix NAT gateway diagnostics poll treating in-progress 400 as fatal The async route-diagnostics operation endpoint reports "still processing" with an HTTP 400 whose body is {"message":"The polling result for async mode is not ready yet"} and reports completion with a 200 carrying a data array that may legitimately be empty. The poll treated the in-progress 400 as fatal and returned on the first iteration, and treated an empty 200 as "still processing", the reverse of the real contract. Add IsDiagnosticsInProgressError (mirroring IsServiceNotFoundError) and have pollDiagnosticsRoutes keep polling on that signal, return any other error, and treat a 200 as the completed result including an empty slice. Add poll-cadence override fields on the service (mirroring the MCR looking-glass poll) so the transition is unit-tested deterministically. --- CHANGELOG.md | 1 + errors.go | 12 ++++ nat_gateway.go | 7 +++ nat_gateway_diagnostics.go | 54 +++++++++++++---- nat_gateway_test.go | 118 ++++++++++++++++++++++++++++--------- 5 files changed, 155 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c7d515..6d6902f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Add `WithCallContext` client option that sets the `X-Call-Context` header so API calls act on behalf of a managed account (identified by company UID). ## Changes +- Fix NAT gateway diagnostics polling treating the API's in-progress signal (HTTP 400 `The polling result for async mode is not ready yet`) as a fatal error. The poll now keeps polling while the operation is processing, returns a completed 200 result including an empty route array, and leaves every other error fatal. - Bump Go toolchain to 1.26.5 to pick up a `crypto/tls` fix for an Encrypted Client Hello privacy leak ([GO-2026-5856](https://pkg.go.dev/vuln/GO-2026-5856)). # 1.0.0 Release diff --git a/errors.go b/errors.go index e2c2597..74a9912 100644 --- a/errors.go +++ b/errors.go @@ -125,6 +125,18 @@ func IsServiceNotFoundError(err error) bool { strings.Contains(apiErr.Message, "Could not find a service with UID") } +// IsDiagnosticsInProgressError reports whether err is the async diagnostics +// "still processing" signal: the non-standard HTTP 400 the routes operation +// endpoint returns while the result is not yet ready. +func IsDiagnosticsInProgressError(err error) bool { + apiErr, ok := err.(*ErrorResponse) + if !ok || apiErr.Response == nil { + return false + } + return apiErr.Response.StatusCode == http.StatusBadRequest && + strings.Contains(apiErr.Message, "The polling result for async mode is not ready yet") +} + // ErrTransitVXCCancelLaterNotAllowed is returned when attempting to schedule Transit VXC deletion for later (only CANCEL_NOW is allowed) var ErrTransitVXCCancelLaterNotAllowed = errors.New("transit vxc (megaport internet) does not support scheduled deletion (cancel later), only immediate deletion (CANCEL_NOW) is allowed") diff --git a/nat_gateway.go b/nat_gateway.go index 53a5934..fe012b6 100644 --- a/nat_gateway.go +++ b/nat_gateway.go @@ -112,6 +112,13 @@ func NewNATGatewayService(c *Client) *NATGatewayServiceOp { // NATGatewayServiceOp handles communication with NAT Gateway methods of the Megaport API. type NATGatewayServiceOp struct { Client *Client + // pollInitialDelay overrides diagnosticsPollInitialDelay when non-zero. + // pollInterval overrides diagnosticsPollInterval when non-zero. + // pollTimeout overrides diagnosticsPollTimeout when non-zero. + // Intended for tests that want to avoid real-time waits. + pollInitialDelay time.Duration + pollInterval time.Duration + pollTimeout time.Duration } // GetNATGatewayTelemetryRequest represents a request to get telemetry data for a NAT Gateway. diff --git a/nat_gateway_diagnostics.go b/nat_gateway_diagnostics.go index 0b829c2..9136924 100644 --- a/nat_gateway_diagnostics.go +++ b/nat_gateway_diagnostics.go @@ -26,6 +26,30 @@ const ( diagnosticsPollTimeout = 60 * time.Second ) +// effectivePollInitialDelay returns the initial delay before the first poll. +func (svc *NATGatewayServiceOp) effectivePollInitialDelay() time.Duration { + if svc.pollInitialDelay != 0 { + return svc.pollInitialDelay + } + return diagnosticsPollInitialDelay +} + +// effectivePollInterval returns the interval between polls. +func (svc *NATGatewayServiceOp) effectivePollInterval() time.Duration { + if svc.pollInterval != 0 { + return svc.pollInterval + } + return diagnosticsPollInterval +} + +// effectivePollTimeout returns the SDK-managed overall poll timeout. +func (svc *NATGatewayServiceOp) effectivePollTimeout() time.Duration { + if svc.pollTimeout != 0 { + return svc.pollTimeout + } + return diagnosticsPollTimeout +} + // ListNATGatewayIPRoutesAsync submits an IP routes diagnostics request. func (svc *NATGatewayServiceOp) ListNATGatewayIPRoutesAsync(ctx context.Context, productUID, ipAddress string) (string, error) { if productUID == "" { @@ -106,11 +130,14 @@ func (svc *NATGatewayServiceOp) GetNATGatewayDiagnosticsRoutes(ctx context.Conte } // pollDiagnosticsRoutes polls GetNATGatewayDiagnosticsRoutes until the -// operation returns a non-empty result, the SDK-managed -// diagnosticsPollTimeout elapses, or the caller's context is cancelled. -// Empty responses are treated as "still processing". +// operation completes, the SDK-managed diagnosticsPollTimeout elapses, or the +// caller's context is cancelled. While the operation is still processing the +// endpoint returns an HTTP 400 that IsDiagnosticsInProgressError detects, and +// the loop keeps polling; any other error is returned to the caller. A 200 +// response is the completed result and is returned as-is, including an empty +// route slice. func (svc *NATGatewayServiceOp) pollDiagnosticsRoutes(ctx context.Context, productUID, operationID string) ([]*NATGatewayRoute, error) { - pollCtx, cancel := context.WithTimeout(ctx, diagnosticsPollTimeout) + pollCtx, cancel := context.WithTimeout(ctx, svc.effectivePollTimeout()) defer cancel() // pollDoneErr returns ctx.Err() when the caller's context is the one that // fired (cancellation or caller-imposed deadline) and @@ -126,18 +153,25 @@ func (svc *NATGatewayServiceOp) pollDiagnosticsRoutes(ctx context.Context, produ select { case <-pollCtx.Done(): return nil, pollDoneErr() - case <-time.After(diagnosticsPollInitialDelay): + case <-time.After(svc.effectivePollInitialDelay()): } - ticker := time.NewTicker(diagnosticsPollInterval) + ticker := time.NewTicker(svc.effectivePollInterval()) defer ticker.Stop() for { routes, err := svc.GetNATGatewayDiagnosticsRoutes(pollCtx, productUID, operationID) - if err != nil { - return nil, err - } - if len(routes) > 0 { + if err == nil { return routes, nil } + if !IsDiagnosticsInProgressError(err) { + // A poll-context expiry mid-request surfaces as a wrapped context + // error; attribute it to the deadline/cancellation via pollDoneErr + // so callers get a consistent error, leaving genuine API failures + // untouched. + if pollCtx.Err() != nil && (errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled)) { + return nil, pollDoneErr() + } + return nil, err + } select { case <-pollCtx.Done(): return nil, pollDoneErr() diff --git a/nat_gateway_test.go b/nat_gateway_test.go index 2f50fa2..bdd3584 100644 --- a/nat_gateway_test.go +++ b/nat_gateway_test.go @@ -1228,9 +1228,24 @@ func (suite *NATGatewayClientTestSuite) TestGetNATGatewayDiagnosticsRoutesValida suite.ErrorIs(err, ErrNATGatewayDiagnosticsOperationEmpty) } -func (suite *NATGatewayClientTestSuite) TestListNATGatewayIPRoutesPolling() { +// fastPollNATService returns the suite's NAT Gateway service with the poll +// cadence collapsed to millisecond delays so poll-driven tests run without +// real-time waits. +func (suite *NATGatewayClientTestSuite) fastPollNATService() *NATGatewayServiceOp { + op, ok := suite.client.NATGatewayService.(*NATGatewayServiceOp) + suite.Require().True(ok) + op.pollInitialDelay = time.Millisecond + op.pollInterval = time.Millisecond + return op +} + +const diagnosticsInProgressBody = `{"message":"The polling result for async mode is not ready yet"}` + +// TestListNATGatewayIPRoutesPollInProgressThenComplete drives the poll through +// the real API contract: an in-progress HTTP 400 followed by a completed 200. +func (suite *NATGatewayClientTestSuite) TestListNATGatewayIPRoutesPollInProgressThenComplete() { ctx := context.Background() - natSvc := suite.client.NATGatewayService + natSvc := suite.fastPollNATService() productUID := "uid-poll" var opCalls atomic.Int32 @@ -1240,46 +1255,95 @@ func (suite *NATGatewayClientTestSuite) TestListNATGatewayIPRoutesPolling() { fmt.Fprint(w, `{"message":"ok","terms":"","data":"op-poll"}`) }) suite.mux.HandleFunc("/v3/products/nat_gateways/"+productUID+"/diagnostics/routes/operation", func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - // First call returns empty (still processing); subsequent calls return data. + // First poll: still processing (HTTP 400). Subsequent: completed 200. if opCalls.Add(1) == 1 { - fmt.Fprint(w, `{"message":"ok","terms":"","data":[]}`) + w.WriteHeader(http.StatusBadRequest) + fmt.Fprint(w, diagnosticsInProgressBody) return } + w.Header().Set("Content-Type", "application/json") fmt.Fprint(w, `{"message":"ok","terms":"","data":[ {"prefix":"10.0.0.0/24","protocol":"STATIC","nextHop":{"ip":"10.0.0.1","vxc":{"id":"vxc-1","name":"v1"}}}, {"prefix":"192.168.0.0/16","asPath":"65000","origin":"IGP","best":true,"nextHop":{"ip":"10.0.0.2","vxc":{"id":"vxc-2","name":"v2"}}} ]}`) }) - // Bypass the long polling defaults by polling directly via the async + Get methods, - // so this test stays fast. The poll timeout/interval are package-level constants - // and not worth plumbing through a setter just for testing. - opID, err := natSvc.ListNATGatewayIPRoutesAsync(ctx, productUID, "") + routes, err := natSvc.ListNATGatewayIPRoutes(ctx, productUID, "") suite.Require().NoError(err) - suite.Equal("op-poll", opID) + suite.Len(routes, 1) // only the IP route is extracted; the BGP route is dropped + suite.Equal("10.0.0.0/24", routes[0].Prefix) + suite.GreaterOrEqual(opCalls.Load(), int32(2)) +} + +// TestListNATGatewayIPRoutesPollEmptyComplete verifies a 200 with an empty data +// array is a completed result: the poll returns empty without further polling. +func (suite *NATGatewayClientTestSuite) TestListNATGatewayIPRoutesPollEmptyComplete() { + ctx := context.Background() + natSvc := suite.fastPollNATService() + productUID := "uid-empty" + + var opCalls atomic.Int32 - // Drain the empty response then the populated one. - routes, err := natSvc.GetNATGatewayDiagnosticsRoutes(ctx, productUID, opID) + suite.mux.HandleFunc("/v3/products/nat_gateways/"+productUID+"/diagnostics/routes/ip", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"message":"ok","terms":"","data":"op-empty"}`) + }) + suite.mux.HandleFunc("/v3/products/nat_gateways/"+productUID+"/diagnostics/routes/operation", func(w http.ResponseWriter, r *http.Request) { + opCalls.Add(1) + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"message":"ok","terms":"","data":[]}`) + }) + + routes, err := natSvc.ListNATGatewayIPRoutes(ctx, productUID, "") suite.Require().NoError(err) suite.Empty(routes) + suite.Equal(int32(1), opCalls.Load()) +} - routes, err = natSvc.GetNATGatewayDiagnosticsRoutes(ctx, productUID, opID) - suite.Require().NoError(err) - suite.Len(routes, 2) +// TestListNATGatewayIPRoutesPollFatalError verifies a non-in-progress error is +// returned unchanged rather than swallowed as "still processing". +func (suite *NATGatewayClientTestSuite) TestListNATGatewayIPRoutesPollFatalError() { + ctx := context.Background() + natSvc := suite.fastPollNATService() + productUID := "uid-fatal" - // Discriminator: one IP, one BGP. - var ipCount, bgpCount int - for _, r := range routes { - if r.IP != nil { - ipCount++ - } - if r.BGP != nil { - bgpCount++ - } - } - suite.Equal(1, ipCount) - suite.Equal(1, bgpCount) + var opCalls atomic.Int32 + + suite.mux.HandleFunc("/v3/products/nat_gateways/"+productUID+"/diagnostics/routes/ip", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"message":"ok","terms":"","data":"op-fatal"}`) + }) + suite.mux.HandleFunc("/v3/products/nat_gateways/"+productUID+"/diagnostics/routes/operation", func(w http.ResponseWriter, r *http.Request) { + opCalls.Add(1) + w.WriteHeader(http.StatusBadRequest) + fmt.Fprint(w, `{"message":"unknown operationId"}`) + }) + + _, err := natSvc.ListNATGatewayIPRoutes(ctx, productUID, "") + suite.Require().Error(err) + suite.False(IsDiagnosticsInProgressError(err)) + suite.Equal(int32(1), opCalls.Load()) +} + +// TestListNATGatewayIPRoutesPollTimeout verifies an operation that never +// completes surfaces ErrNATGatewayDiagnosticsTimeout. +func (suite *NATGatewayClientTestSuite) TestListNATGatewayIPRoutesPollTimeout() { + ctx := context.Background() + natSvc := suite.fastPollNATService() + natSvc.pollTimeout = 20 * time.Millisecond + productUID := "uid-timeout" + + suite.mux.HandleFunc("/v3/products/nat_gateways/"+productUID+"/diagnostics/routes/ip", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"message":"ok","terms":"","data":"op-timeout"}`) + }) + suite.mux.HandleFunc("/v3/products/nat_gateways/"+productUID+"/diagnostics/routes/operation", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + fmt.Fprint(w, diagnosticsInProgressBody) + }) + + _, err := natSvc.ListNATGatewayIPRoutes(ctx, productUID, "") + suite.ErrorIs(err, ErrNATGatewayDiagnosticsTimeout) } // --- Prefix list round-trip ---------------------------------------------- From 2b5cf9bb6dd76cd978e7ccba3daf3e655a109aea Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Tue, 21 Jul 2026 08:16:24 -0700 Subject: [PATCH 2/4] ESD-1676: Interleave poll-override field doc comments Match the mcr_looking_glass sibling's per-field comment style. --- nat_gateway.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/nat_gateway.go b/nat_gateway.go index fe012b6..7714cdc 100644 --- a/nat_gateway.go +++ b/nat_gateway.go @@ -113,12 +113,14 @@ func NewNATGatewayService(c *Client) *NATGatewayServiceOp { type NATGatewayServiceOp struct { Client *Client // pollInitialDelay overrides diagnosticsPollInitialDelay when non-zero. + // Intended for tests that want to avoid real-time waits. + pollInitialDelay time.Duration // pollInterval overrides diagnosticsPollInterval when non-zero. + // Intended for tests that want to avoid real-time waits. + pollInterval time.Duration // pollTimeout overrides diagnosticsPollTimeout when non-zero. // Intended for tests that want to avoid real-time waits. - pollInitialDelay time.Duration - pollInterval time.Duration - pollTimeout time.Duration + pollTimeout time.Duration } // GetNATGatewayTelemetryRequest represents a request to get telemetry data for a NAT Gateway. From 26b5521250de83166b9a567f307a76be9bd928b1 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Tue, 21 Jul 2026 08:21:14 -0700 Subject: [PATCH 3/4] ESD-1676: Add caller-cancellation poll test, strengthen timeout assertion Cover the pollDoneErr caller-context path (returns context.Canceled, not the timeout sentinel) and assert the timeout test tolerated at least one in-progress poll. --- nat_gateway_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/nat_gateway_test.go b/nat_gateway_test.go index bdd3584..23e1924 100644 --- a/nat_gateway_test.go +++ b/nat_gateway_test.go @@ -1333,17 +1333,46 @@ func (suite *NATGatewayClientTestSuite) TestListNATGatewayIPRoutesPollTimeout() natSvc.pollTimeout = 20 * time.Millisecond productUID := "uid-timeout" + var opCalls atomic.Int32 + suite.mux.HandleFunc("/v3/products/nat_gateways/"+productUID+"/diagnostics/routes/ip", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") fmt.Fprint(w, `{"message":"ok","terms":"","data":"op-timeout"}`) }) suite.mux.HandleFunc("/v3/products/nat_gateways/"+productUID+"/diagnostics/routes/operation", func(w http.ResponseWriter, r *http.Request) { + opCalls.Add(1) w.WriteHeader(http.StatusBadRequest) fmt.Fprint(w, diagnosticsInProgressBody) }) _, err := natSvc.ListNATGatewayIPRoutes(ctx, productUID, "") suite.ErrorIs(err, ErrNATGatewayDiagnosticsTimeout) + suite.GreaterOrEqual(opCalls.Load(), int32(1)) // at least one in-progress poll was tolerated before the timeout +} + +// TestListNATGatewayIPRoutesPollCallerCancelled verifies that when the caller's +// own context is cancelled mid-poll, the poll returns that cancellation error +// rather than the SDK-managed timeout sentinel. +func (suite *NATGatewayClientTestSuite) TestListNATGatewayIPRoutesPollCallerCancelled() { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + natSvc := suite.fastPollNATService() + productUID := "uid-cancel" + + suite.mux.HandleFunc("/v3/products/nat_gateways/"+productUID+"/diagnostics/routes/ip", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"message":"ok","terms":"","data":"op-cancel"}`) + }) + suite.mux.HandleFunc("/v3/products/nat_gateways/"+productUID+"/diagnostics/routes/operation", func(w http.ResponseWriter, r *http.Request) { + // Deliver one in-progress 400, then cancel the caller's context so the + // next poll iteration surfaces the caller's cancellation. + w.WriteHeader(http.StatusBadRequest) + fmt.Fprint(w, diagnosticsInProgressBody) + cancel() + }) + + _, err := natSvc.ListNATGatewayIPRoutes(ctx, productUID, "") + suite.ErrorIs(err, context.Canceled) } // --- Prefix list round-trip ---------------------------------------------- From f8c411bda744c5aa70cac1900a16409d5caad021 Mon Sep 17 00:00:00 2001 From: MegaportPhilipBrowne Date: Tue, 28 Jul 2026 10:01:47 -0700 Subject: [PATCH 4/4] ESD-1676: Unexport the diagnostics in-progress error helper The check matches a message substring on the non-standard 400 the NAT gateway routes operation endpoint returns while a result is not ready. There is no error code behind that signal, and other diagnostics endpoints report the same state differently, so it should not be public SDK surface. Renames IsDiagnosticsInProgressError to isNATGatewayDiagnosticsInProgress. --- errors.go | 10 ++++++---- nat_gateway_diagnostics.go | 10 +++++----- nat_gateway_test.go | 2 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/errors.go b/errors.go index 74a9912..691cebe 100644 --- a/errors.go +++ b/errors.go @@ -125,10 +125,12 @@ func IsServiceNotFoundError(err error) bool { strings.Contains(apiErr.Message, "Could not find a service with UID") } -// IsDiagnosticsInProgressError reports whether err is the async diagnostics -// "still processing" signal: the non-standard HTTP 400 the routes operation -// endpoint returns while the result is not yet ready. -func IsDiagnosticsInProgressError(err error) bool { +// isNATGatewayDiagnosticsInProgress reports whether err is the async diagnostics +// "still processing" signal: the non-standard HTTP 400 the NAT gateway routes +// operation endpoint returns while the result is not yet ready. The signal is a +// message string with no error code behind it, and other diagnostics APIs +// report the same state differently, so this stays specific to NAT gateway. +func isNATGatewayDiagnosticsInProgress(err error) bool { apiErr, ok := err.(*ErrorResponse) if !ok || apiErr.Response == nil { return false diff --git a/nat_gateway_diagnostics.go b/nat_gateway_diagnostics.go index 9136924..66f4b85 100644 --- a/nat_gateway_diagnostics.go +++ b/nat_gateway_diagnostics.go @@ -132,10 +132,10 @@ func (svc *NATGatewayServiceOp) GetNATGatewayDiagnosticsRoutes(ctx context.Conte // pollDiagnosticsRoutes polls GetNATGatewayDiagnosticsRoutes until the // operation completes, the SDK-managed diagnosticsPollTimeout elapses, or the // caller's context is cancelled. While the operation is still processing the -// endpoint returns an HTTP 400 that IsDiagnosticsInProgressError detects, and -// the loop keeps polling; any other error is returned to the caller. A 200 -// response is the completed result and is returned as-is, including an empty -// route slice. +// endpoint returns an HTTP 400 that isNATGatewayDiagnosticsInProgress +// detects, and the loop keeps polling; any other error is returned to the +// caller. A 200 response is the completed result and is returned as-is, +// including an empty route slice. func (svc *NATGatewayServiceOp) pollDiagnosticsRoutes(ctx context.Context, productUID, operationID string) ([]*NATGatewayRoute, error) { pollCtx, cancel := context.WithTimeout(ctx, svc.effectivePollTimeout()) defer cancel() @@ -162,7 +162,7 @@ func (svc *NATGatewayServiceOp) pollDiagnosticsRoutes(ctx context.Context, produ if err == nil { return routes, nil } - if !IsDiagnosticsInProgressError(err) { + if !isNATGatewayDiagnosticsInProgress(err) { // A poll-context expiry mid-request surfaces as a wrapped context // error; attribute it to the deadline/cancellation via pollDoneErr // so callers get a consistent error, leaving genuine API failures diff --git a/nat_gateway_test.go b/nat_gateway_test.go index 23e1924..4c8d555 100644 --- a/nat_gateway_test.go +++ b/nat_gateway_test.go @@ -1321,7 +1321,7 @@ func (suite *NATGatewayClientTestSuite) TestListNATGatewayIPRoutesPollFatalError _, err := natSvc.ListNATGatewayIPRoutes(ctx, productUID, "") suite.Require().Error(err) - suite.False(IsDiagnosticsInProgressError(err)) + suite.False(isNATGatewayDiagnosticsInProgress(err)) suite.Equal(int32(1), opCalls.Load()) }