diff --git a/internal/config/validation_schema.go b/internal/config/validation_schema.go index 24282cee0..a1cf906c5 100644 --- a/internal/config/validation_schema.go +++ b/internal/config/validation_schema.go @@ -16,6 +16,28 @@ import ( "github.com/santhosh-tekuri/jsonschema/v5" ) +const ( + // maxSchemaFetchRetries is the number of fetch attempts before giving up. + maxSchemaFetchRetries = 3 +) + +// schemaFetchRetryDelay is the base delay between retry attempts using exponential +// backoff (1×, 2×, 4×, …). It is a variable so tests can override it to zero for +// fast execution. +var schemaFetchRetryDelay = time.Second + +// schemaHTTPClientTimeout is the per-attempt HTTP request timeout. It is a variable +// so tests can shorten it to avoid long waits when testing timeout behaviour. +var schemaHTTPClientTimeout = 10 * time.Second + +// isTransientHTTPError returns true for status codes that indicate a temporary +// server-side condition (rate-limiting or transient failure) worth retrying. +func isTransientHTTPError(statusCode int) bool { + return statusCode == http.StatusTooManyRequests || + statusCode == http.StatusServiceUnavailable || + (statusCode >= 500 && statusCode < 600) +} + var ( // Compile regex patterns from schema for additional validation containerPattern = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9./_-]*(:([a-zA-Z0-9._-]+|latest))?$`) @@ -83,22 +105,56 @@ func fetchAndFixSchema(url string) ([]byte, error) { logSchema.Printf("Fetching schema from URL: %s", url) client := &http.Client{ - Timeout: 10 * time.Second, + Timeout: schemaHTTPClientTimeout, } - fetchStart := time.Now() - resp, err := client.Get(url) - if err != nil { - logSchema.Printf("Schema fetch failed after %v: %v", time.Since(fetchStart), err) - return nil, fmt.Errorf("failed to fetch schema from %s: %w", url, err) + var resp *http.Response + var lastErr error + + for attempt := 1; attempt <= maxSchemaFetchRetries; attempt++ { + if attempt > 1 { + delay := schemaFetchRetryDelay << uint(attempt-2) // 1×, 2×, 4× base delay + logSchema.Printf("Retrying schema fetch (attempt %d/%d) after %v: %v", attempt, maxSchemaFetchRetries, delay, lastErr) + time.Sleep(delay) + } + + fetchStart := time.Now() + var err error + resp, err = client.Get(url) + if err != nil { + logSchema.Printf("Schema fetch attempt %d failed after %v: %v", attempt, time.Since(fetchStart), err) + lastErr = fmt.Errorf("failed to fetch schema from %s: %w", url, err) + resp = nil + continue + } + logSchema.Printf("HTTP request attempt %d completed in %v with status %d", attempt, time.Since(fetchStart), resp.StatusCode) + + if resp.StatusCode == http.StatusOK { + lastErr = nil + break + } + + if isTransientHTTPError(resp.StatusCode) { + lastErr = fmt.Errorf("failed to fetch schema: HTTP %d", resp.StatusCode) + logSchema.Printf("Schema fetch attempt %d returned transient error: HTTP %d, will retry", attempt, resp.StatusCode) + resp.Body.Close() + resp = nil + continue + } + + // Permanent HTTP error (404, 403, 401, etc.) — do not retry. + lastErr = fmt.Errorf("failed to fetch schema: HTTP %d", resp.StatusCode) + logSchema.Printf("Schema fetch returned permanent error: HTTP %d", resp.StatusCode) + resp.Body.Close() + resp = nil + break } - defer resp.Body.Close() - logSchema.Printf("HTTP request completed in %v", time.Since(fetchStart)) - if resp.StatusCode != http.StatusOK { - logSchema.Printf("Schema fetch returned non-OK status: %d", resp.StatusCode) - return nil, fmt.Errorf("failed to fetch schema: HTTP %d", resp.StatusCode) + if resp == nil { + return nil, lastErr } + defer resp.Body.Close() + logSchema.Printf("HTTP request completed in %v", time.Since(startTime)) readStart := time.Now() schemaBytes, err := io.ReadAll(resp.Body) diff --git a/internal/config/validation_schema_fetch_test.go b/internal/config/validation_schema_fetch_test.go index db110dfea..bb9580be3 100644 --- a/internal/config/validation_schema_fetch_test.go +++ b/internal/config/validation_schema_fetch_test.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "strings" + "sync/atomic" "testing" "time" @@ -13,6 +14,13 @@ import ( "github.com/stretchr/testify/require" ) +func init() { + // Speed up all retry-related tests by disabling inter-attempt delays and + // shortening the per-attempt HTTP timeout so timeout tests don't take 30 s. + schemaFetchRetryDelay = 0 + schemaHTTPClientTimeout = 200 * time.Millisecond +} + // TestFetchAndFixSchema_SuccessfulFetch tests the happy path where schema is fetched successfully func TestFetchAndFixSchema_SuccessfulFetch(t *testing.T) { // Create a minimal valid schema for testing @@ -52,35 +60,48 @@ func TestFetchAndFixSchema_SuccessfulFetch(t *testing.T) { // TestFetchAndFixSchema_HTTPError tests handling of HTTP error responses func TestFetchAndFixSchema_HTTPError(t *testing.T) { tests := []struct { - name string - statusCode int - wantErr string + name string + statusCode int + wantErr string + wantRequests int // expected number of HTTP requests (1 = no retry, 3 = full retry) }{ { - name: "404 Not Found", - statusCode: http.StatusNotFound, - wantErr: "failed to fetch schema: HTTP 404", + name: "404 Not Found", + statusCode: http.StatusNotFound, + wantErr: "failed to fetch schema: HTTP 404", + wantRequests: 1, // permanent error, no retry }, { - name: "500 Internal Server Error", - statusCode: http.StatusInternalServerError, - wantErr: "failed to fetch schema: HTTP 500", + name: "500 Internal Server Error", + statusCode: http.StatusInternalServerError, + wantErr: "failed to fetch schema: HTTP 500", + wantRequests: maxSchemaFetchRetries, // transient, retried }, { - name: "403 Forbidden", - statusCode: http.StatusForbidden, - wantErr: "failed to fetch schema: HTTP 403", + name: "403 Forbidden", + statusCode: http.StatusForbidden, + wantErr: "failed to fetch schema: HTTP 403", + wantRequests: 1, // permanent error, no retry }, { - name: "503 Service Unavailable", - statusCode: http.StatusServiceUnavailable, - wantErr: "failed to fetch schema: HTTP 503", + name: "503 Service Unavailable", + statusCode: http.StatusServiceUnavailable, + wantErr: "failed to fetch schema: HTTP 503", + wantRequests: maxSchemaFetchRetries, // transient, retried + }, + { + name: "429 Too Many Requests", + statusCode: http.StatusTooManyRequests, + wantErr: "failed to fetch schema: HTTP 429", + wantRequests: maxSchemaFetchRetries, // transient, retried }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + var requestCount atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount.Add(1) w.WriteHeader(tt.statusCode) })) defer server.Close() @@ -90,6 +111,8 @@ func TestFetchAndFixSchema_HTTPError(t *testing.T) { assert.Error(t, err) assert.Nil(t, result) assert.Contains(t, err.Error(), tt.wantErr) + assert.Equal(t, int32(tt.wantRequests), requestCount.Load(), + "expected %d HTTP request(s) for status %d", tt.wantRequests, tt.statusCode) }) } } @@ -108,9 +131,11 @@ func TestFetchAndFixSchema_NetworkError(t *testing.T) { // TestFetchAndFixSchema_Timeout tests handling of request timeouts func TestFetchAndFixSchema_Timeout(t *testing.T) { - // Create a server that delays longer than the client timeout + // Create a server that delays longer than the configured client timeout. + // The init() in this test file sets schemaHTTPClientTimeout = 200ms, so any + // delay > 200ms will trigger a timeout on each attempt. server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - time.Sleep(15 * time.Second) // fetchAndFixSchema has 10 second timeout + time.Sleep(500 * time.Millisecond) w.WriteHeader(http.StatusOK) })) defer server.Close() @@ -589,3 +614,53 @@ func TestFetchAndFixSchema_LargeSchema(t *testing.T) { require.True(t, ok) assert.Equal(t, 100, len(properties), "Should preserve all 100 properties") } + +// TestFetchAndFixSchema_RetrySucceedsAfterTransientError verifies that a transient +// error on the first attempt is retried and the eventual success is returned. +func TestFetchAndFixSchema_RetrySucceedsAfterTransientError(t *testing.T) { + validSchema := map[string]interface{}{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + } + schemaJSON, err := json.Marshal(validSchema) + require.NoError(t, err) + + var requestCount atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + n := requestCount.Add(1) + if n < 3 { + // First two attempts return 429 + w.WriteHeader(http.StatusTooManyRequests) + return + } + w.WriteHeader(http.StatusOK) + w.Write(schemaJSON) + })) + defer server.Close() + + result, err := fetchAndFixSchema(server.URL) + + require.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, int32(3), requestCount.Load(), "should have made 3 requests (2 failures + 1 success)") +} + +// TestFetchAndFixSchema_ExponentialBackoffDelays verifies that all retries are attempted +// when transient errors occur, and that the function eventually gives up after +// maxSchemaFetchRetries attempts. +func TestFetchAndFixSchema_ExponentialBackoffDelays(t *testing.T) { + var requestCount atomic.Int32 + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount.Add(1) + w.WriteHeader(http.StatusTooManyRequests) + })) + defer server.Close() + + _, err := fetchAndFixSchema(server.URL) + + require.Error(t, err) + assert.Equal(t, int32(maxSchemaFetchRetries), requestCount.Load(), + "should make exactly maxSchemaFetchRetries requests before giving up") + assert.Contains(t, err.Error(), "failed to fetch schema: HTTP 429") +}