Add exponential backoff retry logic to OIDC provider initialization (#928)

mgreau · web-flow · commit 3ae8b79b512c · 2025-06-18T10:49:52.000-07:00
Implements retry logic using [github.com/cenkalti/backoff/v5](https://github.com/cenkalti/backoff/v5) to handle connectivity issues with slow or unreliable OIDC providers. The retry mechanism uses exponential backoff (1s → 2s → 4s → 8s → 16s → 30s max) with randomization to avoid thundering herd issues. - Add retry wrapper around `oidc.NewProvider()` calls - Add test coverage for retry scenarios - Improve logging with attempt counts and issuer information --------- Signed-off-by: Maxime Gréau <maxime.greau@chainguard.dev>
diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go
@@ -7,8 +7,11 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"strings"
 	"testing"
+	"time"
 
+	"github.com/cenkalti/backoff/v5"
 	"github.com/chainguard-dev/clog"
 	"github.com/chainguard-dev/terraform-infra-common/pkg/httpmetrics"
 	"github.com/coreos/go-oidc/v3/oidc"
@@ -45,7 +48,7 @@ func Get(ctx context.Context, issuer string) (provider VerifierProvider, err err
 	})
 
 	// Verify the token before we trust anything about it.
-	provider, err = oidc.NewProvider(ctx, issuer)
+	provider, err = newProviderWithRetry(ctx, issuer)
 	if err != nil {
 		return nil, fmt.Errorf("constructing %q provider: %w", issuer, err)
 	}
@@ -57,6 +60,59 @@ func Get(ctx context.Context, issuer string) (provider VerifierProvider, err err
 	return provider, nil
 }
 
+// newProviderWithRetry creates a new OIDC provider with exponential backoff retry logic
+func newProviderWithRetry(ctx context.Context, issuer string) (VerifierProvider, error) {
+	attempt := 0
+
+	operation := func() (VerifierProvider, error) {
+		attempt++
+		p, err := oidc.NewProvider(ctx, issuer)
+		if err != nil {
+			clog.WarnContext(ctx, "provider creation failed", "attempt", attempt, "issuer", issuer, "error", err)
+			// Check for permanent errors that shouldn't be retried
+			if isPermanentError(err) {
+				return nil, backoff.Permanent(err)
+			}
+			return nil, err
+		}
+		if attempt > 1 {
+			clog.InfoContext(ctx, "provider creation succeeded after retry", "attempts", attempt, "issuer", issuer)
+		}
+		return p, nil
+	}
+
+	// Configure exponential backoff: 1s → 2s → 4s → 8s → 16s → 30s (max)
+	// with ±10% jitter to prevent thundering herd issues
+	expBackoff := backoff.NewExponentialBackOff()
+	expBackoff.InitialInterval = 1 * time.Second
+	expBackoff.MaxInterval = 30 * time.Second
+	expBackoff.Multiplier = 2.0
+	expBackoff.RandomizationFactor = 0.1
+
+	return backoff.Retry(ctx, operation, backoff.WithBackOff(expBackoff))
+}
+
+// isPermanentError checks if an error should not be retried based on HTTP status codes
+func isPermanentError(err error) bool {
+	// String matching for HTTP status codes embedded in error messages
+	// This matches go-oidc's pattern: fmt.Errorf("%s: %s", resp.Status, body)
+	errMsg := err.Error()
+	if strings.Contains(errMsg, "400 Bad Request") ||
+		strings.Contains(errMsg, "401 Unauthorized") ||
+		strings.Contains(errMsg, "403 Forbidden") ||
+		strings.Contains(errMsg, "404 Not Found") ||
+		strings.Contains(errMsg, "405 Method Not Allowed") ||
+		strings.Contains(errMsg, "406 Not Acceptable") ||
+		strings.Contains(errMsg, "410 Gone") ||
+		strings.Contains(errMsg, "415 Unsupported Media Type") ||
+		strings.Contains(errMsg, "422 Unprocessable Entity") ||
+		strings.Contains(errMsg, "501 Not Implemented") {
+		return true // Don't retry these permanent client/server errors
+	}
+
+	return false // Retry all other errors
+}
+
 type keysetProvider struct {
 	issuer string
 	keySet oidc.KeySet
@@ -68,7 +124,7 @@ func (s *keysetProvider) Verifier(config *oidc.Config) *oidc.IDTokenVerifier {
 
 // AddTestKeySetVerifier adds a test key set verifier to the provider cachef or the issuer.
 // This is primarily intended for testing - the static key set is not verified against the upstream issuer.
-func AddTestKeySetVerifier(t *testing.T, issuer string, keySet oidc.KeySet) {
+func AddTestKeySetVerifier(_ *testing.T, issuer string, keySet oidc.KeySet) {
 	providers.Add(issuer, &keysetProvider{
 		issuer: issuer,
 		keySet: keySet,
diff --git a/pkg/provider/provider_test.go b/pkg/provider/provider_test.go
@@ -0,0 +1,192 @@
+// Copyright 2025 Chainguard, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package provider
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/coreos/go-oidc/v3/oidc"
+)
+
+func TestNewProviderWithRetry_Success(t *testing.T) {
+	// Create a test server that responds successfully
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		issuerURL := "http://" + r.Host
+		w.Write([]byte(`{"issuer":"` + issuerURL + `","authorization_endpoint":"` + issuerURL + `/auth","token_endpoint":"` + issuerURL + `/token","jwks_uri":"` + issuerURL + `/jwks"}`))
+	}))
+	defer server.Close()
+
+	ctx := context.Background()
+	provider, err := newProviderWithRetry(ctx, server.URL)
+	if err != nil {
+		t.Fatalf("Expected success, got error: %v", err)
+	}
+	if provider == nil {
+		t.Fatal("Expected provider, got nil")
+	}
+}
+
+func TestNewProviderWithRetry_EventualSuccess(t *testing.T) {
+	var attempts int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		attempt := atomic.AddInt32(&attempts, 1)
+		if attempt < 3 {
+			// Fail the first 2 attempts
+			w.WriteHeader(http.StatusInternalServerError)
+			return
+		}
+		// Succeed on the 3rd attempt
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		issuerURL := "http://" + r.Host
+		w.Write([]byte(`{"issuer":"` + issuerURL + `","authorization_endpoint":"` + issuerURL + `/auth","token_endpoint":"` + issuerURL + `/token","jwks_uri":"` + issuerURL + `/jwks"}`))
+	}))
+	defer server.Close()
+
+	ctx := context.Background()
+	start := time.Now()
+	provider, err := newProviderWithRetry(ctx, server.URL)
+	duration := time.Since(start)
+
+	if err != nil {
+		t.Fatalf("Expected eventual success, got error: %v", err)
+	}
+	if provider == nil {
+		t.Fatal("Expected provider, got nil")
+	}
+	// The test server fails twice then succeeds on the third attempt
+	if atomic.LoadInt32(&attempts) != 3 {
+		t.Fatalf("Expected 3 attempts, got %d", attempts)
+	}
+	// Should have taken at least 1 second due to backoff after first failure
+	if duration < 1*time.Second {
+		t.Fatalf("Expected retry backoff, but completed too quickly: %v", duration)
+	}
+}
+
+func TestNewProviderWithRetry_AllAttemptsFail(t *testing.T) {
+	var attempts int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		atomic.AddInt32(&attempts, 1)
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer server.Close()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	provider, err := newProviderWithRetry(ctx, server.URL)
+
+	if err == nil {
+		t.Fatal("Expected error after all retries failed")
+	}
+	if provider != nil {
+		t.Fatal("Expected nil provider after all retries failed")
+	}
+	// With backoff library, we expect multiple attempts but don't need to check exact count
+	if atomic.LoadInt32(&attempts) < 3 {
+		t.Fatalf("Expected at least 3 attempts, got %d", attempts)
+	}
+}
+
+func TestNewProviderWithRetry_ContextCancellation(t *testing.T) {
+	var attempts int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		atomic.AddInt32(&attempts, 1)
+		// Always fail to trigger retries
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer server.Close()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+
+	start := time.Now()
+	provider, err := newProviderWithRetry(ctx, server.URL)
+	duration := time.Since(start)
+
+	if !errors.Is(err, context.DeadlineExceeded) {
+		t.Fatalf("Expected context deadline exceeded, got: %v", err)
+	}
+	if provider != nil {
+		t.Fatal("Expected nil provider after context cancellation")
+	}
+	// Should have attempted at least once but been canceled before completing all retries
+	totalAttempts := atomic.LoadInt32(&attempts)
+	if totalAttempts == 0 {
+		t.Fatal("Expected at least one attempt before context cancellation")
+	}
+	// With backoff library and timeout, we expect some attempts but not too many
+	if totalAttempts > 10 {
+		t.Fatalf("Expected reasonable number of attempts due to context cancellation, got %d", totalAttempts)
+	}
+	// Should have been canceled around the timeout duration
+	if duration > 3*time.Second {
+		t.Fatalf("Expected cancellation around 2s, but took %v", duration)
+	}
+}
+
+func TestIsPermanentError_GoOIDCErrorPatterns(t *testing.T) {
+	tests := []struct {
+		statusCode int
+		body       string
+		permanent  bool
+		name       string
+	}{
+		// Permanent errors - using actual HTTP status codes that will generate real go-oidc errors
+		{400, `{"error":"invalid_request"}`, true, "400 Bad Request should be permanent"},
+		{401, `{"error":"access_denied"}`, true, "401 Unauthorized should be permanent"},
+		{403, `{"error":"insufficient_scope"}`, true, "403 Forbidden should be permanent"},
+		{404, `{"error":"not_found"}`, true, "404 Not Found should be permanent"},
+		{405, `{"error":"method_not_allowed"}`, true, "405 Method Not Allowed should be permanent"},
+		{406, `{"error":"not_acceptable"}`, true, "406 Not Acceptable should be permanent"},
+		{410, `{"error":"gone"}`, true, "410 Gone should be permanent"},
+		{415, `{"error":"unsupported_media_type"}`, true, "415 Unsupported Media Type should be permanent"},
+		{422, `{"error":"unprocessable_entity"}`, true, "422 Unprocessable Entity should be permanent"},
+		{501, `{"error":"not_implemented"}`, true, "501 Not Implemented should be permanent"},
+
+		// Temporary errors - should be retryable
+		{429, `{"error":"rate_limited"}`, false, "429 Too Many Requests should be retryable"},
+		{500, `{"error":"internal_server_error"}`, false, "500 Internal Server Error should be retryable"},
+		{502, `{"error":"bad_gateway"}`, false, "502 Bad Gateway should be retryable"},
+		{503, `{"error":"service_unavailable"}`, false, "503 Service Unavailable should be retryable"},
+		{504, `{"error":"gateway_timeout"}`, false, "504 Gateway Timeout should be retryable"},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// Create a test server that returns the specific HTTP status code
+			// This will generate actual go-oidc errors that we can test against
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				w.WriteHeader(tc.statusCode)
+				w.Write([]byte(tc.body))
+			}))
+			defer server.Close()
+
+			// Use go-oidc to generate the actual error
+			ctx := context.Background()
+			_, err := oidc.NewProvider(ctx, server.URL)
+
+			// go-oidc should return an error for non-200 responses
+			if err == nil {
+				t.Fatalf("Expected go-oidc to return an error for status %d, but got nil", tc.statusCode)
+			}
+
+			// Test our error classification function on the real go-oidc error
+			result := isPermanentError(err)
+			if result != tc.permanent {
+				t.Errorf("isPermanentError() for real go-oidc error %q = %v, want %v", err.Error(), result, tc.permanent)
+			}
+		})
+	}
+}
