From 6a361b67bba72e3c5aca0aabab2abafd1615bff2 Mon Sep 17 00:00:00 2001 From: Ruben Hoenle Date: Thu, 17 Sep 2026 16:19:11 +0200 Subject: [PATCH 1/4] feat(objectstorage): retry on HTTP 429 rate limits relates to #1764 --- .../services/objectstorage/utils/util.go | 116 +++++++++++++++++- .../services/objectstorage/utils/util_test.go | 22 +++- 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/stackit/internal/services/objectstorage/utils/util.go b/stackit/internal/services/objectstorage/utils/util.go index 8e3ab1a8a..25631abf9 100644 --- a/stackit/internal/services/objectstorage/utils/util.go +++ b/stackit/internal/services/objectstorage/utils/util.go @@ -1,9 +1,13 @@ package utils import ( + "bytes" "context" "fmt" + "io" + "math/rand/v2" "net/http" + "strconv" "time" objectstorage "github.com/stackitcloud/stackit-sdk-go/services/objectstorage/v2api" @@ -35,14 +39,124 @@ func EnableProject(ctx context.Context, projectId, region string, client objects return nil } +// RetryTransport wraps an underlying RoundTripper to retry on HTTP 429 rate limit errors with jitter. +type RetryTransport struct { + Base http.RoundTripper + MaxRetries int + BaseBackoff time.Duration + MaxJitter time.Duration +} + +func (t *RetryTransport) RoundTrip(req *http.Request) (*http.Response, error) { + base := t.Base + if base == nil { + base = http.DefaultTransport + } + + // Preserve request body for retries if present + var bodyBytes []byte + if req.Body != nil && req.Body != http.NoBody { + var err error + bodyBytes, err = io.ReadAll(req.Body) + if err != nil { + return nil, err + } + + err = req.Body.Close() + if err != nil { + return nil, err + } + } + + var resp *http.Response + var err error + + for attempt := 0; attempt <= t.MaxRetries; attempt++ { + // Re-hydrate the request body on each attempt + if bodyBytes != nil { + req.Body = io.NopCloser(bytes.NewReader(bodyBytes)) + } + + resp, err = base.RoundTrip(req) + + // If success or non-429 error, return immediately + if err != nil || resp.StatusCode != http.StatusTooManyRequests { + return resp, err + } + + // Stop if max retries reached + if attempt == t.MaxRetries { + break + } + + // Calculate base sleep duration (value of Retry-After Header, if header isn't present falls back to exponential backoff) + wait := t.getWaitDuration(resp, attempt) + + // Always add random jitter regardless of Retry-After header presence. Else all resource / datasource + // goroutines would try again in parallel after exactly the same interval. + jitter := time.Duration(rand.Int64N(int64(t.MaxJitter))) //nolint:gosec // only used for jitter + totalWait := wait + jitter + + // Drain and close response body before retrying to reuse TCP connections + _, err = io.Copy(io.Discard, resp.Body) + if err != nil { + return nil, err + } + + err = resp.Body.Close() + if err != nil { + return nil, err + } + + select { + case <-req.Context().Done(): + return nil, req.Context().Err() + case <-time.After(totalWait): + } + } + + return resp, err +} + +func (t *RetryTransport) getWaitDuration(resp *http.Response, attempt int) time.Duration { + if retryAfter := resp.Header.Get("Retry-After"); retryAfter != "" { + // Try parsing as integer seconds + if seconds, err := strconv.Atoi(retryAfter); err == nil { + return time.Duration(seconds) * time.Second + } + // Try parsing as HTTP-Date string + if date, err := http.ParseTime(retryAfter); err == nil { + if d := time.Until(date); d > 0 { + return d + } + } + } + + // Fallback to exponential backoff + return t.BaseBackoff * (1 << attempt) +} + func ConfigureClient(ctx context.Context, providerData *core.ProviderData, diags *diag.Diagnostics) *objectstorage.APIClient { + // Add middleware to retry on HTTP 429 rate limits. + // This solution is **not** intended to be copied to each and every service (!!). + // This should be rolled out centrally instead, should be easily doable after + // this refactoring: https://github.com/stackitcloud/terraform-provider-stackit/pull/1663 + retryRoundTripper := &RetryTransport{ + Base: providerData.RoundTripper, + MaxRetries: 3, + BaseBackoff: 1 * time.Second, + MaxJitter: 500 * time.Millisecond, // Always added to wait time + } + apiClientConfigOptions := []config.ConfigurationOption{ - config.WithCustomAuth(providerData.RoundTripper), + config.WithCustomAuth(retryRoundTripper), utils.UserAgentConfigOption(providerData.Version), } + if providerData.ObjectStorageCustomEndpoint != "" { apiClientConfigOptions = append(apiClientConfigOptions, config.WithEndpoint(providerData.ObjectStorageCustomEndpoint)) } + apiClient, err := objectstorage.NewAPIClient(apiClientConfigOptions...) if err != nil { core.LogAndAddError(ctx, diags, "Error configuring API client", fmt.Sprintf("Configuring client: %v. This is an error related to the provider configuration, not to the resource configuration", err)) diff --git a/stackit/internal/services/objectstorage/utils/util_test.go b/stackit/internal/services/objectstorage/utils/util_test.go index 1a174b0dd..b3632a0a9 100644 --- a/stackit/internal/services/objectstorage/utils/util_test.go +++ b/stackit/internal/services/objectstorage/utils/util_test.go @@ -2,6 +2,7 @@ package utils import ( "context" + "crypto/tls" "fmt" "net/http" "os" @@ -33,6 +34,10 @@ func TestConfigureClient(t *testing.T) { t.Errorf("error setting env variable: %v", err) } + var roundTripper http.RoundTripper = &http.Transport{ + TLSClientConfig: &tls.Config{MinVersion: tls.VersionTLS13}, + } + type args struct { providerData *core.ProviderData } @@ -46,13 +51,21 @@ func TestConfigureClient(t *testing.T) { name: "default endpoint", args: args{ providerData: &core.ProviderData{ - Version: testVersion, + Version: testVersion, + RoundTripper: roundTripper, }, }, expected: func() *objectstorage.APIClient { apiClient, err := objectstorage.NewAPIClient( utils.UserAgentConfigOption(testVersion), + config.WithCustomAuth(&RetryTransport{ + Base: roundTripper, + MaxRetries: 3, + BaseBackoff: 1 * time.Second, + MaxJitter: 500 * time.Millisecond, + }), ) + if err != nil { t.Errorf("error configuring client: %v", err) } @@ -65,6 +78,7 @@ func TestConfigureClient(t *testing.T) { args: args{ providerData: &core.ProviderData{ Version: testVersion, + RoundTripper: roundTripper, ObjectStorageCustomEndpoint: testCustomEndpoint, }, }, @@ -72,6 +86,12 @@ func TestConfigureClient(t *testing.T) { apiClient, err := objectstorage.NewAPIClient( utils.UserAgentConfigOption(testVersion), config.WithEndpoint(testCustomEndpoint), + config.WithCustomAuth(&RetryTransport{ + Base: roundTripper, + MaxRetries: 3, + BaseBackoff: 1 * time.Second, + MaxJitter: 500 * time.Millisecond, + }), ) if err != nil { t.Errorf("error configuring client: %v", err) From 3e028cfdf7d0837cbc8b391e12af43a0eeb9ee83 Mon Sep 17 00:00:00 2001 From: Ruben Hoenle Date: Thu, 17 Sep 2026 17:30:28 +0200 Subject: [PATCH 2/4] fix acc tests --- .../services/objectstorage/objectstorage_acc_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/stackit/internal/services/objectstorage/objectstorage_acc_test.go b/stackit/internal/services/objectstorage/objectstorage_acc_test.go index a6dcd0ba6..bd4981aa9 100644 --- a/stackit/internal/services/objectstorage/objectstorage_acc_test.go +++ b/stackit/internal/services/objectstorage/objectstorage_acc_test.go @@ -143,10 +143,8 @@ func TestAccObjectStorageResourceMin(t *testing.T) { } data "stackit_objectstorage_default_retention" "retention" { - bucket_name = stackit_objectstorage_bucket.bucket_object_lock.name - project_id = var.project_id - days = var.retention_days - mode = var.retention_mode + bucket_name = stackit_objectstorage_bucket.bucket_object_lock.name + project_id = var.project_id } `, testutil.NewConfigBuilder().BuildProviderConfig()+resourceMinConfig, From ab17d04bd79a6c92af8203b87e07d1f417d0a366 Mon Sep 17 00:00:00 2001 From: Marcel Jacek Date: Thu, 17 Sep 2026 17:23:57 +0200 Subject: [PATCH 3/4] fix: use built-in methods to clone request --- .../services/objectstorage/utils/util.go | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/stackit/internal/services/objectstorage/utils/util.go b/stackit/internal/services/objectstorage/utils/util.go index 25631abf9..080224ae0 100644 --- a/stackit/internal/services/objectstorage/utils/util.go +++ b/stackit/internal/services/objectstorage/utils/util.go @@ -53,11 +53,10 @@ func (t *RetryTransport) RoundTrip(req *http.Request) (*http.Response, error) { base = http.DefaultTransport } - // Preserve request body for retries if present - var bodyBytes []byte - if req.Body != nil && req.Body != http.NoBody { - var err error - bodyBytes, err = io.ReadAll(req.Body) + // Preserve request body for retries if present without mutating the original request + getBody := req.GetBody + if getBody == nil && req.Body != nil && req.Body != http.NoBody { + bodyBytes, err := io.ReadAll(req.Body) if err != nil { return nil, err } @@ -66,18 +65,26 @@ func (t *RetryTransport) RoundTrip(req *http.Request) (*http.Response, error) { if err != nil { return nil, err } + + getBody = func() (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader(bodyBytes)), nil + } } var resp *http.Response var err error for attempt := 0; attempt <= t.MaxRetries; attempt++ { - // Re-hydrate the request body on each attempt - if bodyBytes != nil { - req.Body = io.NopCloser(bytes.NewReader(bodyBytes)) + reqClone := req.Clone(req.Context()) + if getBody != nil { + var bodyErr error + reqClone.Body, bodyErr = getBody() + if bodyErr != nil { + return nil, bodyErr + } } - resp, err = base.RoundTrip(req) + resp, err = base.RoundTrip(reqClone) // If success or non-429 error, return immediately if err != nil || resp.StatusCode != http.StatusTooManyRequests { From 840091abee5bad4d6086c0379d7dfa73cb248445 Mon Sep 17 00:00:00 2001 From: Ruben Hoenle Date: Fri, 18 Sep 2026 18:01:03 +0200 Subject: [PATCH 4/4] add unit test --- .../services/objectstorage/utils/util.go | 2 +- .../services/objectstorage/utils/util_test.go | 81 ++++++++++++++++++- 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/stackit/internal/services/objectstorage/utils/util.go b/stackit/internal/services/objectstorage/utils/util.go index 080224ae0..74de0c5bb 100644 --- a/stackit/internal/services/objectstorage/utils/util.go +++ b/stackit/internal/services/objectstorage/utils/util.go @@ -151,7 +151,7 @@ func ConfigureClient(ctx context.Context, providerData *core.ProviderData, diags retryRoundTripper := &RetryTransport{ Base: providerData.RoundTripper, MaxRetries: 3, - BaseBackoff: 1 * time.Second, + BaseBackoff: 10 * time.Second, MaxJitter: 500 * time.Millisecond, // Always added to wait time } diff --git a/stackit/internal/services/objectstorage/utils/util_test.go b/stackit/internal/services/objectstorage/utils/util_test.go index b3632a0a9..4de27765d 100644 --- a/stackit/internal/services/objectstorage/utils/util_test.go +++ b/stackit/internal/services/objectstorage/utils/util_test.go @@ -5,12 +5,14 @@ import ( "crypto/tls" "fmt" "net/http" + "net/http/httptest" "os" "reflect" "testing" "testing/synctest" "time" + "github.com/google/uuid" "github.com/hashicorp/terraform-plugin-framework/diag" sdkClients "github.com/stackitcloud/stackit-sdk-go/core/clients" "github.com/stackitcloud/stackit-sdk-go/core/config" @@ -61,7 +63,7 @@ func TestConfigureClient(t *testing.T) { config.WithCustomAuth(&RetryTransport{ Base: roundTripper, MaxRetries: 3, - BaseBackoff: 1 * time.Second, + BaseBackoff: 10 * time.Second, MaxJitter: 500 * time.Millisecond, }), ) @@ -89,7 +91,7 @@ func TestConfigureClient(t *testing.T) { config.WithCustomAuth(&RetryTransport{ Base: roundTripper, MaxRetries: 3, - BaseBackoff: 1 * time.Second, + BaseBackoff: 10 * time.Second, MaxJitter: 500 * time.Millisecond, }), ) @@ -118,6 +120,81 @@ func TestConfigureClient(t *testing.T) { } } +func TestClientRetry(t *testing.T) { + ctx := context.Background() + diags := diag.Diagnostics{} + + testProjectId := uuid.New().String() + const testRegion = "eu01" + const testBucketName = "karl-otto" + + attempts := 0 + + // Create mock server returning HTTP 429 on first & second call, HTTP 200 on final retry + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts++ + + if r.URL.Path != fmt.Sprintf("/v2/project/%s/regions/%s/bucket/%s", testProjectId, testRegion, testBucketName) { + t.Fatalf("invalid endpoint called") + } + + // first request: HTTP 429 *with* Retry-After header + if attempts == 1 { + w.Header().Set("Retry-After", "1") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusTooManyRequests) + _, err := w.Write([]byte(`{"error": "rate_limit_exceeded"}`)) + if err != nil { + t.Fatalf("error writing response: %v", err) + } + return + } + + // second request: HTTP 429 *without* Retry-After header (we expect base backoff to be used now) + if attempts == 2 { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusTooManyRequests) + _, err := w.Write([]byte(`{"error": "rate_limit_exceeded"}`)) + if err != nil { + t.Fatalf("error writing response: %v", err) + } + return + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, err := w.Write([]byte(`{ + "bucket": { + "name": "bucket-1", + "objectLockEnabled": false, + "region": "eu01", + "urlPathStyle": "https://object.storage.eu01.onstackit.cloud/bucket-1", + "urlVirtualHostedStyle": "https://bucket-1.object.storage.eu01.onstackit.cloud" + }, + "project": "` + testProjectId + `"}`)) + if err != nil { + t.Fatalf("error writing response: %v", err) + } + })) + defer server.Close() + + client := ConfigureClient(ctx, &core.ProviderData{ + ObjectStorageCustomEndpoint: server.URL, + }, &diags) + if diags.HasError() { + t.Fatalf("error configuring client: %v", diags) + } + + _, err := client.DefaultAPI.GetBucket(ctx, testProjectId, testRegion, testBucketName).Execute() + if err != nil { + t.Fatalf("unexpected request error: %v", err) + } + + if attempts != 3 { + t.Fatalf("expected 3 attempts, got %d", attempts) + } +} + func TestEnableProject(t *testing.T) { tests := []struct { description string