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, diff --git a/stackit/internal/services/objectstorage/utils/util.go b/stackit/internal/services/objectstorage/utils/util.go index 8e3ab1a8a..74de0c5bb 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,131 @@ 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 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 + } + + err = req.Body.Close() + 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++ { + 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(reqClone) + + // 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: 10 * 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..4de27765d 100644 --- a/stackit/internal/services/objectstorage/utils/util_test.go +++ b/stackit/internal/services/objectstorage/utils/util_test.go @@ -2,14 +2,17 @@ package utils import ( "context" + "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" @@ -33,6 +36,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 +53,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: 10 * time.Second, + MaxJitter: 500 * time.Millisecond, + }), ) + if err != nil { t.Errorf("error configuring client: %v", err) } @@ -65,6 +80,7 @@ func TestConfigureClient(t *testing.T) { args: args{ providerData: &core.ProviderData{ Version: testVersion, + RoundTripper: roundTripper, ObjectStorageCustomEndpoint: testCustomEndpoint, }, }, @@ -72,6 +88,12 @@ func TestConfigureClient(t *testing.T) { apiClient, err := objectstorage.NewAPIClient( utils.UserAgentConfigOption(testVersion), config.WithEndpoint(testCustomEndpoint), + config.WithCustomAuth(&RetryTransport{ + Base: roundTripper, + MaxRetries: 3, + BaseBackoff: 10 * time.Second, + MaxJitter: 500 * time.Millisecond, + }), ) if err != nil { t.Errorf("error configuring client: %v", err) @@ -98,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