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..691cebe 100644 --- a/errors.go +++ b/errors.go @@ -125,6 +125,20 @@ func IsServiceNotFoundError(err error) bool { strings.Contains(apiErr.Message, "Could not find a service with UID") } +// 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 + } + 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..7714cdc 100644 --- a/nat_gateway.go +++ b/nat_gateway.go @@ -112,6 +112,15 @@ 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. + // 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. + 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..66f4b85 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 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, 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 !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 + // 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..4c8d555 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,124 @@ 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)) +} - // Drain the empty response then the populated one. - routes, err := natSvc.GetNATGatewayDiagnosticsRoutes(ctx, productUID, opID) +// 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 + + 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(isNATGatewayDiagnosticsInProgress(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" + + 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 ----------------------------------------------