Skip to content

Commit d625ce9

Browse files
committed
[refactor] Add retries
1 parent b0a1949 commit d625ce9

File tree

2 files changed

+669
-52
lines changed

2 files changed

+669
-52
lines changed

internal/secretsource/url/url.go

Lines changed: 166 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"errors"
99
"fmt"
1010
"io"
11+
"math"
12+
"math/rand" // nosemgrep: math-random-used // This is being used for retry jitter
1113
"net/http"
1214
"strings"
1315
"time"
@@ -23,8 +25,27 @@ var (
2325
errFailedToGetSecret = errors.New("failed to get secret")
2426
errInvalidRequestsPerMinuteLimit = errors.New("requestsPerMinuteLimit must be greater than 0")
2527
errInvalidRequestsBurst = errors.New("requestsBurst must be greater than 0")
28+
errInvalidMaxRetries = errors.New("maxRetries must be greater than or equal to 0")
29+
errInvalidRetryBackoff = errors.New("retryBackoffSeconds must be greater than 0")
2630
)
2731

32+
// retryableError wraps an error with HTTP status code information to determine if it should be retried.
33+
type retryableError struct {
34+
err error
35+
statusCode int
36+
}
37+
38+
func (re *retryableError) Error() string {
39+
if re.statusCode > 0 {
40+
return fmt.Sprintf("HTTP %d: %v", re.statusCode, re.err)
41+
}
42+
return re.err.Error()
43+
}
44+
45+
func (re *retryableError) Unwrap() error {
46+
return re.err
47+
}
48+
2849
// extConfig holds the configuration for URL-based secrets.
2950
type extConfig struct {
3051
// URLTemplate is a URL template with {key} placeholder
@@ -50,6 +71,15 @@ type extConfig struct {
5071

5172
// Timeout for HTTP requests in seconds (defaults to 30)
5273
TimeoutSeconds *int `json:"timeoutSeconds"`
74+
75+
// MaxRetries sets the maximum number of retry attempts for failed requests
76+
// Only retries on transient errors (5xx, timeouts, network errors, 429)
77+
// Does not retry on 4xx errors (except 429)
78+
MaxRetries *int `json:"maxRetries"`
79+
80+
// RetryBackoffSeconds sets the base backoff duration in seconds for retries
81+
// Uses exponential backoff: wait = (base ^ attempt) + jitter
82+
RetryBackoffSeconds *int `json:"retryBackoffSeconds"`
5383
}
5484

5585
const (
@@ -62,6 +92,8 @@ const (
6292
defaultRequestsPerMinuteLimit = 300 // 300 requests per minute is one request every 200 ms
6393
defaultRequestsBurst = 10 // Allow a burst of 10 requests
6494
defaultTimeoutSeconds = 30 // 30 seconds timeout
95+
defaultMaxRetries = 3 // 3 retry attempts for transient failures
96+
defaultRetryBackoffSeconds = 1 // 1 second base for exponential backoff
6597
)
6698

6799
//nolint:gochecknoinits // This is how k6 secret source registration works.
@@ -101,43 +133,65 @@ func (us *urlSecrets) Get(key string) (string, error) {
101133
return "", fmt.Errorf("rate limiter error: %w", err)
102134
}
103135

104-
// Replace {key} placeholder in URL template
105-
url := strings.ReplaceAll(us.config.URLTemplate, "{key}", key)
136+
var secret string
137+
maxAttempts := *us.config.MaxRetries + 1 // MaxRetries is the number of retries, so total attempts = retries + 1
138+
backoff := time.Duration(*us.config.RetryBackoffSeconds) * time.Second
106139

107-
method := us.config.Method
108-
if method == "" {
109-
method = http.MethodGet
110-
}
140+
err := retry(ctx, maxAttempts, backoff, func() error {
141+
// Replace {key} placeholder in URL template
142+
url := strings.ReplaceAll(us.config.URLTemplate, "{key}", key)
111143

112-
req, err := http.NewRequestWithContext(ctx, method, url, nil)
113-
if err != nil {
114-
return "", fmt.Errorf("failed to create request: %w", err)
115-
}
144+
method := us.config.Method
145+
if method == "" {
146+
method = http.MethodGet
147+
}
116148

117-
// Add headers
118-
for k, v := range us.config.Headers {
119-
req.Header.Set(k, v)
120-
}
149+
req, err := http.NewRequestWithContext(ctx, method, url, nil)
150+
if err != nil {
151+
return &retryableError{err: fmt.Errorf("failed to create request: %w", err), statusCode: 0}
152+
}
121153

122-
response, err := us.httpClient.Do(req)
123-
if err != nil {
124-
return "", fmt.Errorf("failed to get secret: %w", err)
125-
}
126-
defer func() { _ = response.Body.Close() }()
154+
// Add headers
155+
for k, v := range us.config.Headers {
156+
req.Header.Set(k, v)
157+
}
127158

128-
if response.StatusCode != http.StatusOK {
129-
return "", fmt.Errorf("status code %d: %w", response.StatusCode, errFailedToGetSecret)
130-
}
159+
response, err := us.httpClient.Do(req)
160+
if err != nil {
161+
// Network errors are retryable
162+
return &retryableError{err: fmt.Errorf("failed to get secret: %w", err), statusCode: 0}
163+
}
164+
defer func() { _ = response.Body.Close() }()
131165

132-
body, err := io.ReadAll(response.Body)
133-
if err != nil {
134-
return "", fmt.Errorf("failed to read response: %w", err)
135-
}
166+
if response.StatusCode != http.StatusOK {
167+
statusErr := fmt.Errorf("status code %d: %w", response.StatusCode, errFailedToGetSecret)
168+
169+
// Check if this is a retryable status code
170+
if isRetryableError(response.StatusCode, nil) {
171+
return &retryableError{err: statusErr, statusCode: response.StatusCode}
172+
}
173+
174+
// Non-retryable error (4xx except 429)
175+
return statusErr
176+
}
177+
178+
body, err := io.ReadAll(response.Body)
179+
if err != nil {
180+
return &retryableError{err: fmt.Errorf("failed to read response: %w", err), statusCode: 0}
181+
}
136182

137-
// Extract secret value from response
138-
secret, err := extractSecretFromResponse(body, us.config.ResponsePath)
183+
// Extract secret value from response
184+
extractedSecret, err := extractSecretFromResponse(body, us.config.ResponsePath)
185+
if err != nil {
186+
// Extraction errors are not retryable (indicates config issue)
187+
return fmt.Errorf("failed to extract secret: %w", err)
188+
}
189+
190+
secret = extractedSecret
191+
return nil
192+
})
139193
if err != nil {
140-
return "", fmt.Errorf("failed to extract secret: %w", err)
194+
return "", err
141195
}
142196

143197
return secret, nil
@@ -192,6 +246,71 @@ func extractSecretFromResponse(body []byte, responsePath string) (string, error)
192246
return "", fmt.Errorf("secret value at path %q is not a string", responsePath)
193247
}
194248

249+
// isRetryableError determines if an HTTP status code or error should trigger a retry.
250+
// Retries on:
251+
// - Network errors (connection failures, timeouts)
252+
// - 5xx server errors (server-side issues)
253+
// - 429 Too Many Requests (rate limiting)
254+
// Does NOT retry on:
255+
// - 4xx client errors (except 429) - these indicate issues with the request itself
256+
func isRetryableError(statusCode int, err error) bool {
257+
// Network errors should be retried
258+
if err != nil {
259+
return true
260+
}
261+
262+
// Retry on server errors and rate limiting
263+
if statusCode >= 500 || statusCode == http.StatusTooManyRequests {
264+
return true
265+
}
266+
267+
return false
268+
}
269+
270+
// retry retries to execute a provided function until it succeeds or the maximum
271+
// number of attempts is hit. It waits with exponential backoff between attempts.
272+
// The backoff calculation is: wait = (base ^ attempt) + random jitter up to 1 second.
273+
// Only retries errors that are of type *retryableError.
274+
func retry(ctx context.Context, attempts int, baseBackoff time.Duration, do func() error) error {
275+
r := rand.New(rand.NewSource(time.Now().UnixNano())) //nolint:gosec
276+
277+
var lastErr error
278+
for i := 0; i < attempts; i++ {
279+
if i > 0 {
280+
// Calculate exponential backoff: base^attempt + jitter
281+
wait := time.Duration(math.Pow(baseBackoff.Seconds(), float64(i))) * time.Second
282+
jitter := time.Duration(r.Int63n(1000)) * time.Millisecond
283+
wait += jitter
284+
285+
// Wait with context cancellation support
286+
select {
287+
case <-ctx.Done():
288+
return ctx.Err()
289+
case <-time.After(wait):
290+
}
291+
}
292+
293+
lastErr = do()
294+
if lastErr == nil {
295+
return nil
296+
}
297+
298+
// Check if this error should be retried
299+
var retryErr *retryableError
300+
if !errors.As(lastErr, &retryErr) {
301+
// Not a retryable error, fail immediately
302+
return lastErr
303+
}
304+
305+
// If this is the last attempt, return the error
306+
if i == attempts-1 {
307+
return lastErr
308+
}
309+
}
310+
311+
return lastErr
312+
}
313+
195314
func parseConfigArgument(configArg string) (string, error) {
196315
configKey, configPath, ok := strings.Cut(configArg, "=")
197316
if !ok || configKey != "config" {
@@ -252,5 +371,23 @@ func getConfig(arg string, fs fsext.Fs) (extConfig, error) {
252371
return config, errInvalidRequestsBurst
253372
}
254373

374+
if config.MaxRetries == nil {
375+
maxRetries := defaultMaxRetries
376+
config.MaxRetries = &maxRetries
377+
}
378+
379+
if config.RetryBackoffSeconds == nil {
380+
retryBackoffSeconds := defaultRetryBackoffSeconds
381+
config.RetryBackoffSeconds = &retryBackoffSeconds
382+
}
383+
384+
if *config.MaxRetries < 0 {
385+
return config, errInvalidMaxRetries
386+
}
387+
388+
if *config.RetryBackoffSeconds <= 0 {
389+
return config, errInvalidRetryBackoff
390+
}
391+
255392
return config, nil
256393
}

0 commit comments

Comments
 (0)