diff --git a/packages/agentproxy/ca.go b/packages/agentproxy/ca.go index a05af03d..f38e3783 100644 --- a/packages/agentproxy/ca.go +++ b/packages/agentproxy/ca.go @@ -16,7 +16,7 @@ import ( "time" "github.com/Infisical/infisical-merge/packages/api" - "github.com/go-resty/resty/v2" + "github.com/Infisical/infisical-merge/packages/util" "github.com/rs/zerolog/log" ) @@ -166,7 +166,11 @@ func (c *caManager) resignIntermediateLocked() error { } pubPem := pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: pubDer}) - client := resty.New().SetAuthToken(c.token()) + client, err := util.GetRestyClientWithCustomHeaders() + if err != nil { + return err + } + client.SetAuthToken(c.token()) resp, err := api.CallSignAgentProxyIntermediateCa(client, api.SignAgentProxyIntermediateCaRequest{ PublicKey: string(pubPem), }) diff --git a/packages/agentproxy/cache.go b/packages/agentproxy/cache.go index abc254b7..710e8e21 100644 --- a/packages/agentproxy/cache.go +++ b/packages/agentproxy/cache.go @@ -11,7 +11,6 @@ import ( "github.com/Infisical/infisical-merge/packages/api" "github.com/Infisical/infisical-merge/packages/util" - "github.com/go-resty/resty/v2" "github.com/rs/zerolog/log" ) @@ -231,7 +230,11 @@ type resolveParams struct { // resolveServices lists the proxied services for a scope and attaches credential values. Shared by // both resolvers; the differences live in resolveParams. func resolveServices(scope agentScope, p resolveParams) ([]*resolvedService, error) { - client := resty.New().SetAuthToken(p.discoveryToken) + client, err := util.GetRestyClientWithCustomHeaders() + if err != nil { + return nil, err + } + client.SetAuthToken(p.discoveryToken) listResp, err := api.CallListProxiedServices(client, api.ListProxiedServicesRequest{ ProjectID: scope.projectID, Environment: scope.environment, diff --git a/packages/agentproxy/leases.go b/packages/agentproxy/leases.go index d8d91341..59eb9000 100644 --- a/packages/agentproxy/leases.go +++ b/packages/agentproxy/leases.go @@ -11,7 +11,7 @@ import ( "time" "github.com/Infisical/infisical-merge/packages/api" - "github.com/go-resty/resty/v2" + "github.com/Infisical/infisical-merge/packages/util" "github.com/rs/zerolog/log" "golang.org/x/sync/singleflight" ) @@ -99,7 +99,11 @@ func newLeaseStore(proxyToken func() string) *leaseStore { func defaultLeaseMinter(proxyToken func() string) leaseMinter { return func(args leaseMintArgs) (leaseMintResult, error) { - client := resty.New().SetAuthToken(proxyToken()) + client, err := util.GetRestyClientWithCustomHeaders() + if err != nil { + return leaseMintResult{}, err + } + client.SetAuthToken(proxyToken()) resp, err := api.CallCreateDynamicSecretLeaseV1(client, api.CreateDynamicSecretLeaseV1Request{ ProjectSlug: args.projectSlug, Environment: args.environment, @@ -116,8 +120,12 @@ func defaultLeaseMinter(proxyToken func() string) leaseMinter { func defaultLeaseRevoker(proxyToken func() string) leaseRevoker { return func(leaseID, projectSlug, environment, path string) error { - client := resty.New().SetAuthToken(proxyToken()) - _, err := api.CallRevokeDynamicSecretLeaseV1(client, api.RevokeDynamicSecretLeaseV1Request{ + client, err := util.GetRestyClientWithCustomHeaders() + if err != nil { + return err + } + client.SetAuthToken(proxyToken()) + _, err = api.CallRevokeDynamicSecretLeaseV1(client, api.RevokeDynamicSecretLeaseV1Request{ LeaseID: leaseID, ProjectSlug: projectSlug, Environment: environment, diff --git a/packages/agentproxy/proxy.go b/packages/agentproxy/proxy.go index aa7df7c9..4edf0388 100644 --- a/packages/agentproxy/proxy.go +++ b/packages/agentproxy/proxy.go @@ -19,7 +19,7 @@ import ( "time" "github.com/Infisical/infisical-merge/packages/api" - "github.com/go-resty/resty/v2" + "github.com/Infisical/infisical-merge/packages/util" "github.com/rs/zerolog" "github.com/rs/zerolog/log" ) @@ -175,7 +175,12 @@ func (ps *proxyServer) flushUsage() { ps.usage = make(map[string]struct{}) ps.usageMu.Unlock() - client := resty.New().SetAuthToken(ps.opts.ProxyToken()).SetTimeout(usageReportTimeout) + client, err := util.GetRestyClientWithPolicy(util.BestEffortRetryPolicy()) + if err != nil { + log.Debug().Err(err).Msg("failed to build usage-reporting client; dropping batch") + return + } + client.SetAuthToken(ps.opts.ProxyToken()).SetTimeout(usageReportTimeout) for serviceID := range snapshot { if err := api.CallReportProxiedServiceUsage(client, serviceID); err != nil { // Warn once: the usual cause is a missing Report Usage permission, which fails every attempt. diff --git a/packages/cmd/agent.go b/packages/cmd/agent.go index 93ce80e5..9b82db17 100644 --- a/packages/cmd/agent.go +++ b/packages/cmd/agent.go @@ -1684,15 +1684,14 @@ func (tm *AgentManager) RevokeCredentials() error { // Refreshes the existing access token func (tm *AgentManager) RefreshAccessToken(accessToken string) error { - httpClient, err := util.GetRestyClientWithCustomHeaders() + policy := util.AgentRetryPolicy() + policy.ReplaySafe = true // renewal extends the presented token, so a replay cannot double-apply + + httpClient, err := util.GetRestyClientWithPolicy(policy) if err != nil { return err } - httpClient.SetRetryCount(10000). - SetRetryMaxWaitTime(20 * time.Second). - SetRetryWaitTime(5 * time.Second) - response, err := api.CallMachineIdentityRefreshAccessToken(httpClient, api.UniversalAuthRefreshRequest{AccessToken: accessToken}) if err != nil { return err diff --git a/packages/cmd/agent_proxy.go b/packages/cmd/agent_proxy.go index ad908bce..72075a84 100644 --- a/packages/cmd/agent_proxy.go +++ b/packages/cmd/agent_proxy.go @@ -172,7 +172,11 @@ func runAgentProxyConnect(cmd *cobra.Command, args []string) { Set("credentialSource", tokenSource). Set("allowReadableBrokeredSecrets", allowReadableBrokered)) - httpClient := resty.New().SetAuthToken(token.Token) + httpClient, err := util.GetRestyClientWithCustomHeaders() + if err != nil { + util.HandleError(err, "Failed to build the API client") + } + httpClient.SetAuthToken(token.Token) caResp, err := api.CallGetAgentProxyCa(httpClient) if err != nil { diff --git a/packages/cmd/agent_proxy_run.go b/packages/cmd/agent_proxy_run.go index dfc134bf..20f74a50 100644 --- a/packages/cmd/agent_proxy_run.go +++ b/packages/cmd/agent_proxy_run.go @@ -85,7 +85,11 @@ func runAgentProxyRun(cmd *cobra.Command, args []string) { // The single identity for the run: fetches config and secret values in the parent. The child gets none of it. src := resolveDeveloperTokenSource(cmd) - httpClient := resty.New().SetAuthToken(src.token()) + httpClient, err := util.GetRestyClientWithCustomHeaders() + if err != nil { + util.HandleError(err, "Failed to build the API client") + } + httpClient.SetAuthToken(src.token()) placeholders := fetchLocalProxiedServiceConfig(httpClient, projectID, environment, secretPath) local := &agentproxy.LocalOptions{ diff --git a/packages/cmd/login.go b/packages/cmd/login.go index 873efce5..b695a40f 100644 --- a/packages/cmd/login.go +++ b/packages/cmd/login.go @@ -638,7 +638,6 @@ func getFreshUserCredentials(email string, password string) (*api.GetLoginV3Resp if err != nil { return nil, err } - httpClient.SetRetryCount(5) loginV3Response, err := api.CallLoginV3(httpClient, api.GetLoginV3Request{ Email: email, @@ -658,7 +657,6 @@ func getFreshUserCredentialsWithSrp(email string, password string) (*api.GetLogi if err != nil { return nil, nil, err } - httpClient.SetRetryCount(5) params := srp.GetParams(4096) secret1 := srp.GenKey() diff --git a/packages/pam/agent/run.go b/packages/pam/agent/run.go index c17d812b..c65c380b 100644 --- a/packages/pam/agent/run.go +++ b/packages/pam/agent/run.go @@ -54,7 +54,10 @@ type Options struct { // Run binds a proxy per account and launches the agent. It returns the child's exit code. func Run(opts Options) (int, error) { - httpClient := resty.New() + httpClient, err := util.GetRestyClientWithCustomHeaders() + if err != nil { + return 1, fmt.Errorf("failed to build the API client: %w", err) + } httpClient.SetHeader("User-Agent", api.USER_AGENT) // Read the token per request rather than fixing it once. Sessions are created lazily and ended at diff --git a/packages/pam/local/access.go b/packages/pam/local/access.go index d0debefa..6e50ab90 100644 --- a/packages/pam/local/access.go +++ b/packages/pam/local/access.go @@ -68,7 +68,11 @@ func StartPAMAccess(accessToken, path, reason, durationStr, targetHost string, p log.Info().Msgf("Starting PAM access for: %s", strings.TrimPrefix(displayPath, "/")) log.Info().Msgf("Session duration: %s", durationStr) - httpClient := resty.New() + httpClient, err := util.GetRestyClientWithCustomHeaders() + if err != nil { + util.HandleError(err, "Failed to build the API client") + return + } httpClient.SetAuthToken(accessToken) httpClient.SetHeader("User-Agent", api.USER_AGENT) diff --git a/packages/util/common.go b/packages/util/common.go index 125db9e6..3849e5c8 100644 --- a/packages/util/common.go +++ b/packages/util/common.go @@ -45,7 +45,14 @@ func ValidateInfisicalAPIConnection() (ok bool) { return err == nil } +// GetRestyClientWithCustomHeaders is the single place API clients are built, which is what applies +// the retry policy everywhere. Do not construct resty clients directly; TestNoDirectRestyConstruction +// enforces this. func GetRestyClientWithCustomHeaders() (*resty.Client, error) { + return GetRestyClientWithPolicy(DefaultRetryPolicy()) +} + +func GetRestyClientWithPolicy(policy RetryPolicy) (*resty.Client, error) { httpClient := resty.New() customHeaders := os.Getenv("INFISICAL_CUSTOM_HEADERS") if customHeaders != "" { @@ -56,7 +63,7 @@ func GetRestyClientWithCustomHeaders() (*resty.Client, error) { httpClient.SetHeaders(headers) } - return httpClient, nil + return applyRetryPolicy(httpClient, policy), nil } func GetInfisicalCustomHeadersMap() (map[string]string, error) { diff --git a/packages/util/helper.go b/packages/util/helper.go index 2c8625bb..867d64c9 100644 --- a/packages/util/helper.go +++ b/packages/util/helper.go @@ -334,10 +334,6 @@ func UniversalAuthLogin(clientId string, clientSecret string) (api.UniversalAuth return api.UniversalAuthLoginResponse{}, err } - httpClient.SetRetryCount(10000). - SetRetryMaxWaitTime(20 * time.Second). - SetRetryWaitTime(5 * time.Second) - tokenResponse, err := api.CallUniversalAuthLogin(httpClient, api.UniversalAuthLoginRequest{ClientId: clientId, ClientSecret: clientSecret}) if err != nil { return api.UniversalAuthLoginResponse{}, err @@ -347,16 +343,14 @@ func UniversalAuthLogin(clientId string, clientSecret string) (api.UniversalAuth } func RenewMachineIdentityAccessToken(accessToken string) (string, error) { + policy := DefaultRetryPolicy() + policy.ReplaySafe = true // renewal extends the presented token, so a replay cannot double-apply - httpClient, err := GetRestyClientWithCustomHeaders() + httpClient, err := GetRestyClientWithPolicy(policy) if err != nil { return "", err } - httpClient.SetRetryCount(10000). - SetRetryMaxWaitTime(20 * time.Second). - SetRetryWaitTime(5 * time.Second) - request := api.UniversalAuthRefreshRequest{ AccessToken: accessToken, } diff --git a/packages/util/retry.go b/packages/util/retry.go new file mode 100644 index 00000000..fe5d6b30 --- /dev/null +++ b/packages/util/retry.go @@ -0,0 +1,304 @@ +package util + +import ( + "context" + "crypto/tls" + "errors" + "io" + "net" + "net/http" + "os" + "strconv" + "strings" + "time" + + "github.com/go-resty/resty/v2" + "github.com/rs/zerolog/log" +) + +const ( + defaultRetryMaxRetries = 3 + defaultRetryBaseDelay = 500 * time.Millisecond + defaultRetryMaxDelay = 10 * time.Second + + agentRetryMaxRetries = 30 + agentRetryMaxDelay = 30 * time.Second +) + +var retryableStatusCodes = map[int]bool{ + http.StatusTooManyRequests: true, + http.StatusBadGateway: true, + http.StatusServiceUnavailable: true, + http.StatusGatewayTimeout: true, +} + +// RetryPolicy bounds how a client retries transient failures. +type RetryPolicy struct { + // MaxRetries counts attempts after the first. Zero disables retries. + MaxRetries int + BaseDelay time.Duration + MaxDelay time.Duration + + // ReplaySafe asserts that every request sent through this client is safe to send more than + // once, letting POST and PATCH retry like idempotent methods. Set it only where a replay + // cannot double-apply a write. + ReplaySafe bool +} + +// DefaultRetryPolicy suits one-shot commands where a user or script is waiting on the result. +func DefaultRetryPolicy() RetryPolicy { + return RetryPolicy{ + MaxRetries: defaultRetryMaxRetries, + BaseDelay: defaultRetryBaseDelay, + MaxDelay: defaultRetryMaxDelay, + }.withEnvOverrides() +} + +// AgentRetryPolicy suits long-running processes that should ride out an outage rather than exit. +func AgentRetryPolicy() RetryPolicy { + return RetryPolicy{ + MaxRetries: agentRetryMaxRetries, + BaseDelay: defaultRetryBaseDelay, + MaxDelay: agentRetryMaxDelay, + }.withEnvOverrides() +} + +// BestEffortRetryPolicy suits fire-and-forget calls. Not env-tunable so paths that run during +// shutdown stay time-bounded. +func BestEffortRetryPolicy() RetryPolicy { + return RetryPolicy{ + MaxRetries: 1, + BaseDelay: 200 * time.Millisecond, + MaxDelay: time.Second, + } +} + +// withEnvOverrides applies INFISICAL_RETRY_*, which affect every command run in the environment. +func (p RetryPolicy) withEnvOverrides() RetryPolicy { + if raw := os.Getenv(INFISICAL_RETRY_BASE_DELAY_NAME); raw != "" { + if delay, err := ParseTimeDurationString(raw, true); err == nil { + p.BaseDelay = delay + } else { + log.Warn().Msgf("ignoring %s: %v", INFISICAL_RETRY_BASE_DELAY_NAME, err) + } + } + + if raw := os.Getenv(INFISICAL_RETRY_MAX_DELAY_NAME); raw != "" { + if delay, err := ParseTimeDurationString(raw, true); err == nil { + p.MaxDelay = delay + } else { + log.Warn().Msgf("ignoring %s: %v", INFISICAL_RETRY_MAX_DELAY_NAME, err) + } + } + + if raw := os.Getenv(INFISICAL_RETRY_MAX_RETRIES_NAME); raw != "" { + if maxRetries, err := strconv.Atoi(raw); err == nil && maxRetries >= 0 { + p.MaxRetries = maxRetries + } else { + log.Warn().Msgf("ignoring %s: must be a non-negative integer, got %q", INFISICAL_RETRY_MAX_RETRIES_NAME, raw) + } + } + + if p.MaxDelay > 0 && p.BaseDelay > p.MaxDelay { + p.BaseDelay = p.MaxDelay + } + + return p +} + +func applyRetryPolicy(httpClient *resty.Client, policy RetryPolicy) *resty.Client { + httpClient.SetLogger(restyLogAdapter{}) + + if policy.MaxRetries <= 0 { + return httpClient + } + + httpClient. + SetRetryCount(policy.MaxRetries). + SetRetryWaitTime(policy.BaseDelay). + SetRetryMaxWaitTime(policy.MaxDelay). + SetRetryAfter(func(_ *resty.Client, res *resty.Response) (time.Duration, error) { + return retryDelay(res), nil + }) + + // The first AddRetryCondition replaces resty's built-in retry-on-error default, so this one + // condition must cover transport errors and status codes both. + httpClient.AddRetryCondition(func(res *resty.Response, err error) bool { + return shouldRetryRequest(policy, res, err) + }) + + httpClient.AddRetryHook(retryLogger(policy)) + + return httpClient +} + +func shouldRetryRequest(policy RetryPolicy, res *resty.Response, err error) bool { + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return false + } + + if err != nil { + if !isRetryableTransportError(err) { + return false + } + + // Pre-delivery failures are safe to replay for any method. res is nil when resty failed + // before sending; a non-nil response always carries its Request. + if res == nil || requestNeverReachedServer(err) { + return true + } + + // Mid-flight failure: the server may have committed the request with only the response + // lost, so replaying a non-idempotent method could double-apply it. + return policy.ReplaySafe || isIdempotentMethod(res.Request.Method) + } + + if res == nil || !retryableStatusCodes[res.StatusCode()] { + return false + } + + // Retrying sooner than the server asked for would only get rejected again. + if wait, ok := parseRetryAfter(res.Header().Get("Retry-After")); ok && wait > policy.MaxDelay { + return false + } + + // A 429 was rejected before the server acted on it, so any method may repeat it. + if res.StatusCode() == http.StatusTooManyRequests { + return true + } + + return policy.ReplaySafe || isIdempotentMethod(res.Request.Method) +} + +// Per RFC 9110 9.2.2. +func isIdempotentMethod(method string) bool { + switch method { + case http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodTrace, + http.MethodPut, http.MethodDelete: + return true + default: + return false + } +} + +func requestNeverReachedServer(err error) bool { + var dnsErr *net.DNSError + if errors.As(err, &dnsErr) { + return true + } + + var opErr *net.OpError + return errors.As(err, &opErr) && opErr.Op == "dial" +} + +func isRetryableTransportError(err error) bool { + if err == nil { + return false + } + + // Deterministic, but wrapped in *url.Error like everything else, so this must come before the + // net.Error check. crypto/tls wraps every x509 verification error in this type. + var certErr *tls.CertificateVerificationError + if errors.As(err, &certErr) { + return false + } + + var netErr net.Error + if errors.As(err, &netErr) { + return true + } + + // A truncated response body surfaces as a bare io error, not a net.Error. + return errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) +} + +// retryDelay returns the server-requested wait; zero tells resty to use its jittered backoff. +// No cap needed: shouldRetryRequest declines waits beyond MaxDelay. +func retryDelay(res *resty.Response) time.Duration { + if res == nil { + return 0 + } + + if wait, ok := parseRetryAfter(res.Header().Get("Retry-After")); ok { + return wait + } + + return 0 +} + +const maxRetryAfter = 24 * time.Hour + +func parseRetryAfter(value string) (time.Duration, bool) { + value = strings.TrimSpace(value) + if value == "" { + return 0, false + } + + if seconds, err := strconv.Atoi(value); err == nil { + switch { + case seconds <= 0: + return 0, false + case seconds > int(maxRetryAfter/time.Second): + return maxRetryAfter, true + } + return time.Duration(seconds) * time.Second, true + } + + if deadline, err := http.ParseTime(value); err == nil { + if wait := time.Until(deadline); wait > 0 { + return min(wait, maxRetryAfter), true + } + } + + return 0, false +} + +// restyLogAdapter routes resty's internal logging through zerolog. Everything maps to debug: +// resty's request-path warnings and errors duplicate retryLogger and the returned error. +type restyLogAdapter struct{} + +func (restyLogAdapter) Errorf(format string, v ...any) { + log.Debug().Str("component", "resty").Msgf(format, v...) +} + +func (restyLogAdapter) Warnf(format string, v ...any) { + log.Debug().Str("component", "resty").Msgf(format, v...) +} + +func (restyLogAdapter) Debugf(format string, v ...any) { + log.Debug().Str("component", "resty").Msgf(format, v...) +} + +// Debug level: warning on every retry would be noise for scripted use. +func retryLogger(policy RetryPolicy) resty.OnRetryFunc { + return func(res *resty.Response, err error) { + event := log.Debug() + exhausted := false + + if res != nil { + // Resty runs retry hooks on the final attempt too. + exhausted = res.Request.Attempt > policy.MaxRetries + + event = event. + Str("method", res.Request.Method). + Str("url", res.Request.URL). + Int("attempt", res.Request.Attempt). + Int("maxRetries", policy.MaxRetries) + + if res.StatusCode() != 0 { + event = event.Int("status", res.StatusCode()) + } + } + + if err != nil { + event = event.Err(err) + } + + if exhausted { + event.Msg("request failed and retries are exhausted") + return + } + + event.Msg("request failed, retrying") + } +} diff --git a/packages/util/retry_test.go b/packages/util/retry_test.go new file mode 100644 index 00000000..fdb59aea --- /dev/null +++ b/packages/util/retry_test.go @@ -0,0 +1,626 @@ +package util + +import ( + "bytes" + "context" + "crypto/x509" + "errors" + "fmt" + "io" + "io/fs" + "net" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "sync/atomic" + "syscall" + "testing" + "time" + + "github.com/go-resty/resty/v2" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func testPolicy(maxRetries int) RetryPolicy { + return RetryPolicy{ + MaxRetries: maxRetries, + BaseDelay: time.Millisecond, + MaxDelay: 5 * time.Millisecond, + } +} + +// Attempts are counted client-side so failures that never reach a server still count. +func newTestClient(t *testing.T, policy RetryPolicy) (*resty.Client, *atomic.Int32) { + t.Helper() + + var attempts atomic.Int32 + client := applyRetryPolicy(resty.New(), policy) + client.OnBeforeRequest(func(_ *resty.Client, _ *resty.Request) error { + attempts.Add(1) + return nil + }) + + return client, &attempts +} + +func TestRetryStatusCodes(t *testing.T) { + const maxRetries = 2 + + tests := []struct { + name string + status int + wantAttempts int32 + }{ + {"429 too many requests is retried", http.StatusTooManyRequests, maxRetries + 1}, + {"502 bad gateway is retried", http.StatusBadGateway, maxRetries + 1}, + {"503 service unavailable is retried", http.StatusServiceUnavailable, maxRetries + 1}, + {"504 gateway timeout is retried", http.StatusGatewayTimeout, maxRetries + 1}, + + {"400 bad request is not retried", http.StatusBadRequest, 1}, + {"401 unauthorized is not retried", http.StatusUnauthorized, 1}, + {"403 forbidden is not retried", http.StatusForbidden, 1}, + {"404 not found is not retried", http.StatusNotFound, 1}, + {"422 unprocessable is not retried", http.StatusUnprocessableEntity, 1}, + {"500 internal server error is not retried", http.StatusInternalServerError, 1}, + {"200 ok is not retried", http.StatusOK, 1}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(test.status) + })) + defer server.Close() + + client, attempts := newTestClient(t, testPolicy(maxRetries)) + res, err := client.R().Get(server.URL) + + require.NoError(t, err, "a status-code failure should surface as a response, not an error") + assert.Equal(t, test.status, res.StatusCode()) + assert.Equal(t, test.wantAttempts, attempts.Load()) + }) + } +} + +// A 5xx on POST may have been committed server-side, so only 429 is safe to repeat there. +func TestRetryMethodSafety(t *testing.T) { + const maxRetries = 2 + + tests := []struct { + method string + status int + wantAttempts int32 + }{ + {http.MethodGet, http.StatusGatewayTimeout, maxRetries + 1}, + {http.MethodPut, http.StatusGatewayTimeout, maxRetries + 1}, + {http.MethodDelete, http.StatusGatewayTimeout, maxRetries + 1}, + {http.MethodPost, http.StatusGatewayTimeout, 1}, + {http.MethodPost, http.StatusBadGateway, 1}, + {http.MethodPost, http.StatusServiceUnavailable, 1}, + {http.MethodPost, http.StatusTooManyRequests, maxRetries + 1}, + {http.MethodPatch, http.StatusGatewayTimeout, 1}, + {http.MethodPatch, http.StatusTooManyRequests, maxRetries + 1}, + } + + for _, test := range tests { + t.Run(fmt.Sprintf("%s %d", test.method, test.status), func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(test.status) + })) + defer server.Close() + + client, attempts := newTestClient(t, testPolicy(maxRetries)) + _, err := client.R().Execute(test.method, server.URL) + + require.NoError(t, err) + assert.Equal(t, test.wantAttempts, attempts.Load()) + }) + } +} + +func TestRetrySucceedsAfterTransientFailure(t *testing.T) { + var hits atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if hits.Add(1) < 3 { + w.WriteHeader(http.StatusServiceUnavailable) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + defer server.Close() + + client, attempts := newTestClient(t, testPolicy(3)) + res, err := client.R().Get(server.URL) + + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode()) + assert.Equal(t, `{"ok":true}`, res.String()) + assert.Equal(t, int32(3), attempts.Load(), "should stop retrying as soon as a request succeeds") +} + +func TestRetryDisabledWhenMaxRetriesIsZero(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusTooManyRequests) + })) + defer server.Close() + + client, attempts := newTestClient(t, testPolicy(0)) + _, err := client.R().Get(server.URL) + + require.NoError(t, err) + assert.Equal(t, int32(1), attempts.Load()) +} + +func TestRetryOnTransportError(t *testing.T) { + client, attempts := newTestClient(t, testPolicy(2)) + _, err := client.R().Get(deadAddress(t)) + + require.Error(t, err) + assert.Equal(t, int32(3), attempts.Load()) +} + +func TestNoRetryOnTLSTrustFailure(t *testing.T) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + client, attempts := newTestClient(t, testPolicy(3)) + _, err := client.R().Get(server.URL) + + require.Error(t, err) + assert.Equal(t, int32(1), attempts.Load()) +} + +func TestNoRetryOnContextCancellation(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusServiceUnavailable) + })) + defer server.Close() + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + client, attempts := newTestClient(t, testPolicy(3)) + _, err := client.R().SetContext(ctx).Get(server.URL) + + require.Error(t, err) + assert.LessOrEqual(t, attempts.Load(), int32(1)) +} + +func TestRetryHonorsRetryAfterHeader(t *testing.T) { + const retryAfterSeconds = 1 + + var hits atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if hits.Add(1) == 1 { + w.Header().Set("Retry-After", fmt.Sprint(retryAfterSeconds)) + w.WriteHeader(http.StatusTooManyRequests) + return + } + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + // MaxDelay must exceed the header value or the request would not be retried at all. + policy := RetryPolicy{MaxRetries: 2, BaseDelay: time.Millisecond, MaxDelay: 5 * time.Second} + client, _ := newTestClient(t, policy) + + start := time.Now() + res, err := client.R().Get(server.URL) + elapsed := time.Since(start) + + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode()) + assert.GreaterOrEqual(t, elapsed, retryAfterSeconds*time.Second, + "should wait as long as the server asked rather than using its own backoff") +} + +func TestParseRetryAfter(t *testing.T) { + tests := []struct { + name string + value string + want time.Duration + ok bool + }{ + {"empty header", "", 0, false}, + {"seconds", "30", 30 * time.Second, true}, + {"seconds with surrounding space", " 5 ", 5 * time.Second, true}, + {"zero seconds falls back to default backoff", "0", 0, false}, + {"negative seconds falls back to default backoff", "-5", 0, false}, + {"unparseable value falls back to default backoff", "soon", 0, false}, + // Uncapped, this multiplies into a negative Duration that would bypass the fail-fast. + {"overflowing seconds are capped", "9999999999", maxRetryAfter, true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, ok := parseRetryAfter(test.value) + assert.Equal(t, test.ok, ok) + assert.Equal(t, test.want, got) + }) + } + + t.Run("future http date", func(t *testing.T) { + got, ok := parseRetryAfter(time.Now().Add(30 * time.Second).UTC().Format(http.TimeFormat)) + require.True(t, ok) + // The header has second granularity and time passes during the call, so allow slack. + assert.InDelta(t, (30 * time.Second).Seconds(), got.Seconds(), 2) + }) + + t.Run("far future http date is capped", func(t *testing.T) { + got, ok := parseRetryAfter(time.Now().Add(100000 * time.Hour).UTC().Format(http.TimeFormat)) + require.True(t, ok) + assert.Equal(t, maxRetryAfter, got) + }) + + t.Run("past http date falls back to default backoff", func(t *testing.T) { + _, ok := parseRetryAfter(time.Now().Add(-time.Minute).UTC().Format(http.TimeFormat)) + assert.False(t, ok) + }) +} + +func TestRetryDelayFallsBackWithoutHeader(t *testing.T) { + assert.Zero(t, retryDelay(nil), "a nil response should defer to resty's own backoff") + + res := &resty.Response{RawResponse: &http.Response{Header: http.Header{}}} + assert.Zero(t, retryDelay(res), "an absent header should defer to resty's own backoff") +} + +func TestRetryAfterBeyondMaxDelayFailsFast(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Retry-After", "60") + w.WriteHeader(http.StatusTooManyRequests) + })) + defer server.Close() + + client, attempts := newTestClient(t, testPolicy(3)) + + start := time.Now() + res, err := client.R().Get(server.URL) + + require.NoError(t, err) + assert.Equal(t, http.StatusTooManyRequests, res.StatusCode()) + assert.Equal(t, int32(1), attempts.Load()) + assert.Less(t, time.Since(start), time.Second) +} + +func TestIsRetryableTransportError(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + {"nil", nil, false}, + // Bare errnos satisfy net.Error, which is what makes an explicit errno list unnecessary. + {"connection reset", syscall.ECONNRESET, true}, + {"broken pipe", syscall.EPIPE, true}, + {"dns failure", &net.DNSError{Err: "no such host", Name: "app.infisical.com"}, true}, + {"wrapped connection reset", fmt.Errorf("posting secret: %w", syscall.ECONNRESET), true}, + {"url-wrapped transport failure", &url.Error{Op: "Post", URL: "http://x", Err: &net.OpError{Op: "read", Err: syscall.ECONNRESET}}, true}, + + {"untrusted certificate authority", x509.UnknownAuthorityError{}, false}, + {"certificate hostname mismatch", x509.HostnameError{Host: "app.infisical.com"}, false}, + {"json marshal failure", errors.New("json: unsupported type"), false}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.want, isRetryableTransportError(test.err)) + }) + } +} + +func TestRetryPolicyEnvOverrides(t *testing.T) { + t.Run("defaults apply when unset", func(t *testing.T) { + t.Setenv(INFISICAL_RETRY_BASE_DELAY_NAME, "") + t.Setenv(INFISICAL_RETRY_MAX_DELAY_NAME, "") + t.Setenv(INFISICAL_RETRY_MAX_RETRIES_NAME, "") + + policy := DefaultRetryPolicy() + assert.Equal(t, defaultRetryMaxRetries, policy.MaxRetries) + assert.Equal(t, defaultRetryBaseDelay, policy.BaseDelay) + assert.Equal(t, defaultRetryMaxDelay, policy.MaxDelay) + }) + + t.Run("env overrides are applied", func(t *testing.T) { + t.Setenv(INFISICAL_RETRY_BASE_DELAY_NAME, "250ms") + t.Setenv(INFISICAL_RETRY_MAX_DELAY_NAME, "45s") + t.Setenv(INFISICAL_RETRY_MAX_RETRIES_NAME, "7") + + policy := DefaultRetryPolicy() + assert.Equal(t, 7, policy.MaxRetries) + assert.Equal(t, 250*time.Millisecond, policy.BaseDelay) + assert.Equal(t, 45*time.Second, policy.MaxDelay) + }) + + t.Run("env overrides also apply to the agent policy", func(t *testing.T) { + t.Setenv(INFISICAL_RETRY_BASE_DELAY_NAME, "") + t.Setenv(INFISICAL_RETRY_MAX_DELAY_NAME, "") + t.Setenv(INFISICAL_RETRY_MAX_RETRIES_NAME, "2") + + policy := AgentRetryPolicy() + assert.Equal(t, 2, policy.MaxRetries, "env should win over the agent's higher default") + assert.Equal(t, agentRetryMaxDelay, policy.MaxDelay) + }) + + t.Run("malformed values are ignored", func(t *testing.T) { + t.Setenv(INFISICAL_RETRY_BASE_DELAY_NAME, "soon") + t.Setenv(INFISICAL_RETRY_MAX_DELAY_NAME, "") + t.Setenv(INFISICAL_RETRY_MAX_RETRIES_NAME, "lots") + + policy := DefaultRetryPolicy() + assert.Equal(t, defaultRetryMaxRetries, policy.MaxRetries) + assert.Equal(t, defaultRetryBaseDelay, policy.BaseDelay) + }) + + t.Run("best-effort policy ignores env overrides", func(t *testing.T) { + t.Setenv(INFISICAL_RETRY_MAX_RETRIES_NAME, "50") + + assert.Equal(t, 1, BestEffortRetryPolicy().MaxRetries) + }) + + t.Run("base delay is clamped to max delay", func(t *testing.T) { + t.Setenv(INFISICAL_RETRY_BASE_DELAY_NAME, "30s") + t.Setenv(INFISICAL_RETRY_MAX_DELAY_NAME, "5s") + t.Setenv(INFISICAL_RETRY_MAX_RETRIES_NAME, "") + + policy := DefaultRetryPolicy() + assert.Equal(t, 5*time.Second, policy.BaseDelay) + assert.Equal(t, 5*time.Second, policy.MaxDelay) + }) +} + +func TestClientsAreBuiltThroughTheSharedConstructor(t *testing.T) { + t.Setenv(INFISICAL_RETRY_MAX_RETRIES_NAME, "") + + client, err := GetRestyClientWithCustomHeaders() + require.NoError(t, err) + assert.Equal(t, defaultRetryMaxRetries, client.RetryCount) + + agentClient, err := GetRestyClientWithPolicy(AgentRetryPolicy()) + require.NoError(t, err) + assert.Equal(t, agentRetryMaxRetries, agentClient.RetryCount) +} + +// A direct resty.New() compiles and works but silently has no retry policy; add new clients via +// GetRestyClientWithCustomHeaders or GetRestyClientWithPolicy instead. +func TestNoDirectRestyConstruction(t *testing.T) { + allowed := map[string]bool{ + filepath.Join("packages", "util", "common.go"): true, + } + + packagesDir := filepath.Join("..", "..", "packages") + repoRoot := filepath.Join("..", "..") + + var offenders []string + + err := filepath.WalkDir(packagesDir, func(path string, entry fs.DirEntry, err error) error { + if err != nil { + return err + } + if entry.IsDir() || !strings.HasSuffix(path, ".go") { + return nil + } + if strings.HasSuffix(path, "_test.go") { + return nil + } + + relative, err := filepath.Rel(repoRoot, path) + if err != nil { + return err + } + if allowed[relative] { + return nil + } + + contents, err := os.ReadFile(path) + if err != nil { + return err + } + + for i, line := range strings.Split(string(contents), "\n") { + if strings.Contains(line, "resty.New(") { + offenders = append(offenders, fmt.Sprintf("%s:%d", relative, i+1)) + } + } + + return nil + }) + require.NoError(t, err) + + assert.Empty(t, offenders, + "these sites construct a resty client directly and so have no retry policy; "+ + "use util.GetRestyClientWithCustomHeaders or util.GetRestyClientWithPolicy instead") +} + +// Resty's default logger writes unstructured lines straight to stderr on each failed attempt and +// on final failure; applyRetryPolicy must route those through zerolog instead. +func TestRestyInternalLoggingIsRoutedThroughZerolog(t *testing.T) { + stderrReader, stderrWriter, err := os.Pipe() + require.NoError(t, err) + + // Resty binds os.Stderr into its default logger at construction, so the swap must happen + // before the client is built to catch anything bypassing the adapter. + originalStderr := os.Stderr + os.Stderr = stderrWriter + t.Cleanup(func() { os.Stderr = originalStderr }) + + var structured bytes.Buffer + originalLogger := log.Logger + log.Logger = zerolog.New(&structured) + t.Cleanup(func() { log.Logger = originalLogger }) + + client, _ := newTestClient(t, testPolicy(1)) + _, err = client.R().Get(deadAddress(t)) + require.Error(t, err) + + os.Stderr = originalStderr + require.NoError(t, stderrWriter.Close()) + captured, err := io.ReadAll(stderrReader) + require.NoError(t, err) + + assert.NotContains(t, string(captured), "RESTY", + "resty wrote its own unstructured log lines instead of going through zerolog") + assert.Contains(t, structured.String(), `"component":"resty"`, + "resty's internal messages should surface as structured debug events tagged with their source") +} + +// newAbruptCloseServer simulates the ambiguous mid-flight failure: request delivered, connection +// dropped before any response. +func newAbruptCloseServer(t *testing.T) (serverURL string, connections func() int32) { + t.Helper() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = listener.Close() }) + + var count atomic.Int32 + go func() { + for { + conn, err := listener.Accept() + if err != nil { + return + } + count.Add(1) + _ = conn.SetReadDeadline(time.Now().Add(2 * time.Second)) + _, _ = conn.Read(make([]byte, 4096)) + _ = conn.Close() + } + }() + + return "http://" + listener.Addr().String(), count.Load +} + +// deadAddress binds then releases a port, so dialing it is refused before anything is sent. +func deadAddress(t *testing.T) string { + t.Helper() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + addr := listener.Addr().String() + require.NoError(t, listener.Close()) + + return "http://" + addr +} + +func TestTransportErrorMethodSafety(t *testing.T) { + const maxRetries = 2 + + t.Run("mid-flight failure does not replay POST", func(t *testing.T) { + serverURL, _ := newAbruptCloseServer(t) + + client, attempts := newTestClient(t, testPolicy(maxRetries)) + _, err := client.R().SetBody(`{"lease":"request"}`).Post(serverURL) + + require.Error(t, err) + assert.Equal(t, int32(1), attempts.Load()) + }) + + t.Run("mid-flight failure does not replay PATCH", func(t *testing.T) { + serverURL, _ := newAbruptCloseServer(t) + + client, attempts := newTestClient(t, testPolicy(maxRetries)) + _, err := client.R().SetBody(`{}`).Patch(serverURL) + + require.Error(t, err) + assert.Equal(t, int32(1), attempts.Load()) + }) + + t.Run("mid-flight failure replays GET", func(t *testing.T) { + serverURL, _ := newAbruptCloseServer(t) + + client, attempts := newTestClient(t, testPolicy(maxRetries)) + _, err := client.R().Get(serverURL) + + require.Error(t, err) + assert.Equal(t, int32(maxRetries+1), attempts.Load()) + }) + + t.Run("connection refused replays POST", func(t *testing.T) { + client, attempts := newTestClient(t, testPolicy(maxRetries)) + _, err := client.R().SetBody(`{}`).Post(deadAddress(t)) + + require.Error(t, err) + assert.Equal(t, int32(maxRetries+1), attempts.Load()) + }) + + t.Run("unresolvable host replays POST", func(t *testing.T) { + client, attempts := newTestClient(t, testPolicy(maxRetries)) + _, err := client.R().SetBody(`{}`).Post("http://this-host-does-not-exist.invalid") + + require.Error(t, err) + assert.Equal(t, int32(maxRetries+1), attempts.Load()) + }) +} + +func TestReplaySafePolicy(t *testing.T) { + const maxRetries = 2 + + replaySafe := testPolicy(maxRetries) + replaySafe.ReplaySafe = true + + t.Run("mid-flight failure replays POST", func(t *testing.T) { + serverURL, _ := newAbruptCloseServer(t) + + client, attempts := newTestClient(t, replaySafe) + _, err := client.R().SetBody(`{"accessToken":"x"}`).Post(serverURL) + + require.Error(t, err) + assert.Equal(t, int32(maxRetries+1), attempts.Load()) + }) + + t.Run("503 on POST is retried", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusServiceUnavailable) + })) + defer server.Close() + + client, attempts := newTestClient(t, replaySafe) + _, err := client.R().SetBody(`{}`).Post(server.URL) + + require.NoError(t, err) + assert.Equal(t, int32(maxRetries+1), attempts.Load()) + }) +} + +func TestRequestNeverReachedServer(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + // Refused connections and dial timeouts arrive as OpError{Op: "dial"}, never bare errnos. + {"refused connection", &net.OpError{Op: "dial", Err: &os.SyscallError{Syscall: "connect", Err: syscall.ECONNREFUSED}}, true}, + {"wrapped dial failure", fmt.Errorf("post: %w", &net.OpError{Op: "dial", Err: syscall.ETIMEDOUT}), true}, + {"dns failure", &net.DNSError{Err: "no such host", Name: "app.infisical.com"}, true}, + + {"read failure mid-flight", &net.OpError{Op: "read", Err: syscall.ECONNRESET}, false}, + {"write failure mid-flight", &net.OpError{Op: "write", Err: syscall.EPIPE}, false}, + {"bare errno lacks dial context", syscall.ECONNREFUSED, false}, + {"unexpected eof", io.ErrUnexpectedEOF, false}, + {"eof", io.EOF, false}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.want, requestNeverReachedServer(test.err)) + }) + } +} + +func TestIsIdempotentMethod(t *testing.T) { + for _, method := range []string{http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodTrace, http.MethodPut, http.MethodDelete} { + assert.True(t, isIdempotentMethod(method), method) + } + for _, method := range []string{http.MethodPost, http.MethodPatch, "PROPFIND", ""} { + assert.False(t, isIdempotentMethod(method), method) + } +}