From dab283ad2721d659abf4de73f85f1e4bd90ed5d1 Mon Sep 17 00:00:00 2001 From: Fabian Hardt Date: Sun, 2 Aug 2026 11:42:28 +0200 Subject: [PATCH 1/7] fix(objectstorage): retry enabling the project on 409 conflict bucket, credential and credentials group each enable object storage for the project before creating their own object. When two of them are created in the same apply, Terraform runs them in parallel and the API rejects the losing call: Error: Enabling object storage project before creation: failed to create object storage project: 409 Conflict ([{project.create_conflict Two concurrent calls try to create the same project}]), status code 409 The apply fails, although nothing is wrong - the competing call enables the project a moment later. The comment in enableProject already assumed the call to be idempotent ("Creation will also be successful if the project is already enabled"), which holds for sequential calls but not for concurrent ones. enableProject now retries on 409 and leaves every other error untouched, so an apply no longer depends on the order in which Terraform happens to start the resources. Users can work around it today with depends_on, but that requires knowing about an implicit call that the resource documentation does not mention. The retry is deliberately narrow rather than utils.RetryRequest: that helper also retries errors that are not API errors, which would slow down the existing unit tests. Signed-off-by: Fabian Hardt --- .../services/objectstorage/bucket/resource.go | 39 ++++++++-- .../objectstorage/credential/resource.go | 38 ++++++++-- .../credentialsgroup/resource.go | 39 ++++++++-- .../credentialsgroup/resource_test.go | 71 +++++++++++++++++++ 4 files changed, 172 insertions(+), 15 deletions(-) diff --git a/stackit/internal/services/objectstorage/bucket/resource.go b/stackit/internal/services/objectstorage/bucket/resource.go index ccf9efa08..1fce45725 100644 --- a/stackit/internal/services/objectstorage/bucket/resource.go +++ b/stackit/internal/services/objectstorage/bucket/resource.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "strings" + "time" "github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault" "github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier" @@ -394,14 +395,42 @@ func mapFields(bucketResp *objectstorage.GetBucketResponse, model *Model, region return nil } +const ( + // Two object storage resources created in the same apply enable the project concurrently; + // the API answers the losing call with 409. See enableProject. + enableProjectAttempts = 4 +) + +// Overridden in tests to keep them fast. +var enableProjectRetryDelay = 2 * time.Second + // enableProject enables object storage for the specified project. If the project is already enabled, nothing happens func enableProject(ctx context.Context, model *Model, region string, client objectstorage.DefaultAPI) error { projectId := model.ProjectId.ValueString() - // From the object storage OAS: Creation will also be successful if the project is already enabled, but will not create a duplicate - _, err := client.EnableService(ctx, projectId, region).Execute() - if err != nil { - return fmt.Errorf("failed to create object storage project: %w", err) + // From the object storage OAS: Creation will also be successful if the project is already enabled, but will not create a duplicate. + // That holds for sequential calls. Two object storage resources created in the same apply call this concurrently, + // and the API rejects the second one with 409 project.create_conflict ("Two concurrent calls try to create the + // same project"). Retrying is safe: once the competing call has finished, enabling an already enabled project succeeds. + var err error + for attempt := 0; attempt < enableProjectAttempts; attempt++ { + _, err = client.EnableService(ctx, projectId, region).Execute() + if err == nil { + return nil + } + + var oapiErr *oapierror.GenericOpenAPIError + if !errors.As(err, &oapiErr) || oapiErr.StatusCode != http.StatusConflict { + break + } + + timer := time.NewTimer(enableProjectRetryDelay) + select { + case <-ctx.Done(): + timer.Stop() + return ctx.Err() + case <-timer.C: + } } - return nil + return fmt.Errorf("failed to create object storage project: %w", err) } diff --git a/stackit/internal/services/objectstorage/credential/resource.go b/stackit/internal/services/objectstorage/credential/resource.go index cd57d4c9c..f7bdcecc7 100644 --- a/stackit/internal/services/objectstorage/credential/resource.go +++ b/stackit/internal/services/objectstorage/credential/resource.go @@ -490,16 +490,44 @@ func (r *credentialResource) ImportState(ctx context.Context, req resource.Impor tflog.Info(ctx, "ObjectStorage credential state imported") } +const ( + // Two object storage resources created in the same apply enable the project concurrently; + // the API answers the losing call with 409. See enableProject. + enableProjectAttempts = 4 +) + +// Overridden in tests to keep them fast. +var enableProjectRetryDelay = 2 * time.Second + // enableProject enables object storage for the specified project. If the project is already enabled, nothing happens func enableProject(ctx context.Context, model *Model, region string, client objectstorage.DefaultAPI) error { projectId := model.ProjectId.ValueString() - // From the object storage OAS: Creation will also be successful if the project is already enabled, but will not create a duplicate - _, err := client.EnableService(ctx, projectId, region).Execute() - if err != nil { - return fmt.Errorf("failed to create object storage project: %w", err) + // From the object storage OAS: Creation will also be successful if the project is already enabled, but will not create a duplicate. + // That holds for sequential calls. Two object storage resources created in the same apply call this concurrently, + // and the API rejects the second one with 409 project.create_conflict ("Two concurrent calls try to create the + // same project"). Retrying is safe: once the competing call has finished, enabling an already enabled project succeeds. + var err error + for attempt := 0; attempt < enableProjectAttempts; attempt++ { + _, err = client.EnableService(ctx, projectId, region).Execute() + if err == nil { + return nil + } + + var oapiErr *oapierror.GenericOpenAPIError + if !errors.As(err, &oapiErr) || oapiErr.StatusCode != http.StatusConflict { + break + } + + timer := time.NewTimer(enableProjectRetryDelay) + select { + case <-ctx.Done(): + timer.Stop() + return ctx.Err() + case <-timer.C: + } } - return nil + return fmt.Errorf("failed to create object storage project: %w", err) } func toCreatePayload(model *Model) (*objectstorage.CreateAccessKeyPayload, error) { diff --git a/stackit/internal/services/objectstorage/credentialsgroup/resource.go b/stackit/internal/services/objectstorage/credentialsgroup/resource.go index e0c34f284..b11e5adc6 100644 --- a/stackit/internal/services/objectstorage/credentialsgroup/resource.go +++ b/stackit/internal/services/objectstorage/credentialsgroup/resource.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "strings" + "time" "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/conversion" objectstorageUtils "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/objectstorage/utils" @@ -380,16 +381,44 @@ func mapCredentialsGroup(credentialsGroup objectstorage.CredentialsGroup, model return nil } +const ( + // Two object storage resources created in the same apply enable the project concurrently; + // the API answers the losing call with 409. See enableProject. + enableProjectAttempts = 4 +) + +// Overridden in tests to keep them fast. +var enableProjectRetryDelay = 2 * time.Second + // enableProject enables object storage for the specified project. If the project is already enabled, nothing happens func enableProject(ctx context.Context, model *Model, region string, client objectstorage.DefaultAPI) error { projectId := model.ProjectId.ValueString() - // From the object storage OAS: Creation will also be successful if the project is already enabled, but will not create a duplicate - _, err := client.EnableService(ctx, projectId, region).Execute() - if err != nil { - return fmt.Errorf("failed to create object storage project: %w", err) + // From the object storage OAS: Creation will also be successful if the project is already enabled, but will not create a duplicate. + // That holds for sequential calls. Two object storage resources created in the same apply call this concurrently, + // and the API rejects the second one with 409 project.create_conflict ("Two concurrent calls try to create the + // same project"). Retrying is safe: once the competing call has finished, enabling an already enabled project succeeds. + var err error + for attempt := 0; attempt < enableProjectAttempts; attempt++ { + _, err = client.EnableService(ctx, projectId, region).Execute() + if err == nil { + return nil + } + + var oapiErr *oapierror.GenericOpenAPIError + if !errors.As(err, &oapiErr) || oapiErr.StatusCode != http.StatusConflict { + break + } + + timer := time.NewTimer(enableProjectRetryDelay) + select { + case <-ctx.Done(): + timer.Stop() + return ctx.Err() + case <-timer.C: + } } - return nil + return fmt.Errorf("failed to create object storage project: %w", err) } // readCredentialsGroups gets all the existing credentials groups for the specified project, diff --git a/stackit/internal/services/objectstorage/credentialsgroup/resource_test.go b/stackit/internal/services/objectstorage/credentialsgroup/resource_test.go index c044dc54e..1993805e5 100644 --- a/stackit/internal/services/objectstorage/credentialsgroup/resource_test.go +++ b/stackit/internal/services/objectstorage/credentialsgroup/resource_test.go @@ -3,7 +3,11 @@ package objectstorage import ( "context" "fmt" + "net/http" "testing" + "time" + + "github.com/stackitcloud/stackit-sdk-go/core/oapierror" "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework/types" @@ -317,3 +321,70 @@ func TestReadCredentialsGroups(t *testing.T) { }) } } + +// Two object storage resources created in the same apply enable the project concurrently. +// The API answers the losing call with 409 project.create_conflict; enableProject must retry +// instead of failing the apply. +func TestEnableProjectRetriesOnConflict(t *testing.T) { + tests := []struct { + description string + conflicts int + isValid bool + wantAttempts int + }{ + {"succeeds immediately", 0, true, 1}, + {"one conflict, then success", 1, true, 2}, + {"conflicts until the attempts are used up", enableProjectAttempts, false, enableProjectAttempts}, + } + + old := enableProjectRetryDelay + enableProjectRetryDelay = time.Millisecond + defer func() { enableProjectRetryDelay = old }() + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + attempts := 0 + client := &objectstorage.DefaultAPIServiceMock{ + EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { + attempts++ + if attempts <= tt.conflicts { + return nil, &oapierror.GenericOpenAPIError{StatusCode: http.StatusConflict} + } + return &objectstorage.ProjectStatus{}, nil + }), + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + err := enableProject(ctx, &Model{}, "eu01", client) + if tt.isValid && err != nil { + t.Fatalf("Should not have failed: %v", err) + } + if !tt.isValid && err == nil { + t.Fatal("Should have failed") + } + if attempts != tt.wantAttempts { + t.Fatalf("Expected %d attempts, got %d", tt.wantAttempts, attempts) + } + }) + } +} + +// A non-conflict error must not be retried. +func TestEnableProjectDoesNotRetryOtherErrors(t *testing.T) { + attempts := 0 + client := &objectstorage.DefaultAPIServiceMock{ + EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { + attempts++ + return nil, &oapierror.GenericOpenAPIError{StatusCode: http.StatusForbidden} + }), + } + + if err := enableProject(context.Background(), &Model{}, "eu01", client); err == nil { + t.Fatal("Should have failed") + } + if attempts != 1 { + t.Fatalf("Expected a single attempt, got %d", attempts) + } +} From e65cbdb20436df05b1588291ce7be214906c3e0d Mon Sep 17 00:00:00 2001 From: Fabian Hardt Date: Mon, 3 Aug 2026 10:02:56 +0200 Subject: [PATCH 2/7] Use the shared retry helper instead of a hand-rolled loop Per review: utils.RetryRequest already covers this, and the loop was duplicated across all three resources. One behavioural difference worth naming: RetryRequest only filters by status code when the error can be type-asserted to *oapierror.GenericOpenAPIError. Anything else - a network failure, a transport error - is now retried as well, where the previous loop bailed out immediately. For an idempotent enable call that seems reasonable, but it is a change, not a refactor. It also shows up in the existing TestEnableProject: its mock returns a plain error, so the failing case now uses every attempt. Those tests shrink the retry delay so they stay fast. --- .../services/objectstorage/bucket/resource.go | 28 ++++++------------- .../objectstorage/bucket/resource_test.go | 9 ++++++ .../objectstorage/credential/resource.go | 28 ++++++------------- .../objectstorage/credential/resource_test.go | 8 ++++++ .../credentialsgroup/resource.go | 28 ++++++------------- .../credentialsgroup/resource_test.go | 8 ++++++ 6 files changed, 49 insertions(+), 60 deletions(-) diff --git a/stackit/internal/services/objectstorage/bucket/resource.go b/stackit/internal/services/objectstorage/bucket/resource.go index 1fce45725..f6e4d68fa 100644 --- a/stackit/internal/services/objectstorage/bucket/resource.go +++ b/stackit/internal/services/objectstorage/bucket/resource.go @@ -412,25 +412,13 @@ func enableProject(ctx context.Context, model *Model, region string, client obje // That holds for sequential calls. Two object storage resources created in the same apply call this concurrently, // and the API rejects the second one with 409 project.create_conflict ("Two concurrent calls try to create the // same project"). Retrying is safe: once the competing call has finished, enabling an already enabled project succeeds. - var err error - for attempt := 0; attempt < enableProjectAttempts; attempt++ { - _, err = client.EnableService(ctx, projectId, region).Execute() - if err == nil { - return nil - } - - var oapiErr *oapierror.GenericOpenAPIError - if !errors.As(err, &oapiErr) || oapiErr.StatusCode != http.StatusConflict { - break - } - - timer := time.NewTimer(enableProjectRetryDelay) - select { - case <-ctx.Done(): - timer.Stop() - return ctx.Err() - case <-timer.C: - } + config := utils.RetryConfig{ + Attempts: enableProjectAttempts, + Delay: enableProjectRetryDelay, + RetryStatusCodes: []int{http.StatusConflict}, } - return fmt.Errorf("failed to create object storage project: %w", err) + if _, err := utils.RetryRequest(ctx, client.EnableService(ctx, projectId, region).Execute, config); err != nil { + return fmt.Errorf("failed to create object storage project: %w", err) + } + return nil } diff --git a/stackit/internal/services/objectstorage/bucket/resource_test.go b/stackit/internal/services/objectstorage/bucket/resource_test.go index 97625d2ff..530f7f486 100644 --- a/stackit/internal/services/objectstorage/bucket/resource_test.go +++ b/stackit/internal/services/objectstorage/bucket/resource_test.go @@ -5,6 +5,7 @@ import ( _ "embed" "fmt" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework/types" @@ -122,6 +123,14 @@ func TestMapFields(t *testing.T) { } func TestEnableProject(t *testing.T) { + // enableProject retries, and the mock returns a plain error rather than an + // *oapierror.GenericOpenAPIError - RetryRequest only filters by status code + // when it can type-assert the error, so the failing case uses up every + // attempt. Without shrinking the delay this test would sleep for seconds. + oldDelay := enableProjectRetryDelay + enableProjectRetryDelay = time.Millisecond + defer func() { enableProjectRetryDelay = oldDelay }() + tests := []struct { description string enableFails bool diff --git a/stackit/internal/services/objectstorage/credential/resource.go b/stackit/internal/services/objectstorage/credential/resource.go index f7bdcecc7..7f261fe76 100644 --- a/stackit/internal/services/objectstorage/credential/resource.go +++ b/stackit/internal/services/objectstorage/credential/resource.go @@ -507,27 +507,15 @@ func enableProject(ctx context.Context, model *Model, region string, client obje // That holds for sequential calls. Two object storage resources created in the same apply call this concurrently, // and the API rejects the second one with 409 project.create_conflict ("Two concurrent calls try to create the // same project"). Retrying is safe: once the competing call has finished, enabling an already enabled project succeeds. - var err error - for attempt := 0; attempt < enableProjectAttempts; attempt++ { - _, err = client.EnableService(ctx, projectId, region).Execute() - if err == nil { - return nil - } - - var oapiErr *oapierror.GenericOpenAPIError - if !errors.As(err, &oapiErr) || oapiErr.StatusCode != http.StatusConflict { - break - } - - timer := time.NewTimer(enableProjectRetryDelay) - select { - case <-ctx.Done(): - timer.Stop() - return ctx.Err() - case <-timer.C: - } + config := utils.RetryConfig{ + Attempts: enableProjectAttempts, + Delay: enableProjectRetryDelay, + RetryStatusCodes: []int{http.StatusConflict}, } - return fmt.Errorf("failed to create object storage project: %w", err) + if _, err := utils.RetryRequest(ctx, client.EnableService(ctx, projectId, region).Execute, config); err != nil { + return fmt.Errorf("failed to create object storage project: %w", err) + } + return nil } func toCreatePayload(model *Model) (*objectstorage.CreateAccessKeyPayload, error) { diff --git a/stackit/internal/services/objectstorage/credential/resource_test.go b/stackit/internal/services/objectstorage/credential/resource_test.go index 6d55d8f1f..35207feb3 100644 --- a/stackit/internal/services/objectstorage/credential/resource_test.go +++ b/stackit/internal/services/objectstorage/credential/resource_test.go @@ -161,6 +161,14 @@ func TestMapFields(t *testing.T) { } func TestEnableProject(t *testing.T) { + // enableProject retries, and the mock returns a plain error rather than an + // *oapierror.GenericOpenAPIError - RetryRequest only filters by status code + // when it can type-assert the error, so the failing case uses up every + // attempt. Without shrinking the delay this test would sleep for seconds. + oldDelay := enableProjectRetryDelay + enableProjectRetryDelay = time.Millisecond + defer func() { enableProjectRetryDelay = oldDelay }() + const testRegion = "eu01" id := fmt.Sprintf("%s,%s,%s", "pid", testRegion, "cgid,cid") tests := []struct { diff --git a/stackit/internal/services/objectstorage/credentialsgroup/resource.go b/stackit/internal/services/objectstorage/credentialsgroup/resource.go index b11e5adc6..6035d6f8c 100644 --- a/stackit/internal/services/objectstorage/credentialsgroup/resource.go +++ b/stackit/internal/services/objectstorage/credentialsgroup/resource.go @@ -398,27 +398,15 @@ func enableProject(ctx context.Context, model *Model, region string, client obje // That holds for sequential calls. Two object storage resources created in the same apply call this concurrently, // and the API rejects the second one with 409 project.create_conflict ("Two concurrent calls try to create the // same project"). Retrying is safe: once the competing call has finished, enabling an already enabled project succeeds. - var err error - for attempt := 0; attempt < enableProjectAttempts; attempt++ { - _, err = client.EnableService(ctx, projectId, region).Execute() - if err == nil { - return nil - } - - var oapiErr *oapierror.GenericOpenAPIError - if !errors.As(err, &oapiErr) || oapiErr.StatusCode != http.StatusConflict { - break - } - - timer := time.NewTimer(enableProjectRetryDelay) - select { - case <-ctx.Done(): - timer.Stop() - return ctx.Err() - case <-timer.C: - } + config := utils.RetryConfig{ + Attempts: enableProjectAttempts, + Delay: enableProjectRetryDelay, + RetryStatusCodes: []int{http.StatusConflict}, } - return fmt.Errorf("failed to create object storage project: %w", err) + if _, err := utils.RetryRequest(ctx, client.EnableService(ctx, projectId, region).Execute, config); err != nil { + return fmt.Errorf("failed to create object storage project: %w", err) + } + return nil } // readCredentialsGroups gets all the existing credentials groups for the specified project, diff --git a/stackit/internal/services/objectstorage/credentialsgroup/resource_test.go b/stackit/internal/services/objectstorage/credentialsgroup/resource_test.go index 1993805e5..2154b8ffb 100644 --- a/stackit/internal/services/objectstorage/credentialsgroup/resource_test.go +++ b/stackit/internal/services/objectstorage/credentialsgroup/resource_test.go @@ -135,6 +135,14 @@ func TestMapFields(t *testing.T) { } func TestEnableProject(t *testing.T) { + // enableProject retries, and the mock returns a plain error rather than an + // *oapierror.GenericOpenAPIError - RetryRequest only filters by status code + // when it can type-assert the error, so the failing case uses up every + // attempt. Without shrinking the delay this test would sleep for seconds. + oldDelay := enableProjectRetryDelay + enableProjectRetryDelay = time.Millisecond + defer func() { enableProjectRetryDelay = oldDelay }() + tests := []struct { description string enableFails bool From e1b6b4e9fc124076cc9aa46383992c0c82e88fca Mon Sep 17 00:00:00 2001 From: Fabian Hardt Date: Tue, 1 Sep 2026 11:47:14 +0200 Subject: [PATCH 3/7] refactor(objectstorage): deduplicate enableProject into the utils package Move the retrying enableProject helper from the bucket, credential and credentialsgroup resources to objectstorage/utils as EnableProject, and move its tests to utils/util_test.go so the retry behaviour is covered once for every caller. --- .../services/objectstorage/bucket/resource.go | 31 +---- .../objectstorage/bucket/resource_test.go | 60 --------- .../objectstorage/credential/resource.go | 30 +---- .../objectstorage/credential/resource_test.go | 89 ------------- .../credentialsgroup/resource.go | 31 +---- .../credentialsgroup/resource_test.go | 120 ------------------ .../services/objectstorage/utils/util.go | 28 ++++ .../services/objectstorage/utils/util_test.go | 119 +++++++++++++++++ 8 files changed, 150 insertions(+), 358 deletions(-) diff --git a/stackit/internal/services/objectstorage/bucket/resource.go b/stackit/internal/services/objectstorage/bucket/resource.go index f6e4d68fa..0ed7a7348 100644 --- a/stackit/internal/services/objectstorage/bucket/resource.go +++ b/stackit/internal/services/objectstorage/bucket/resource.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" "strings" - "time" "github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault" "github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier" @@ -212,7 +211,7 @@ func (r *bucketResource) Create(ctx context.Context, req resource.CreateRequest, ctx = tflog.SetField(ctx, "region", region) // Handle project init - err := enableProject(ctx, &model, region, r.client.DefaultAPI) + err := objectstorageUtils.EnableProject(ctx, projectId, region, r.client.DefaultAPI) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating bucket", fmt.Sprintf("Enabling object storage project before creation: %v", err)) return @@ -394,31 +393,3 @@ func mapFields(bucketResp *objectstorage.GetBucketResponse, model *Model, region model.ObjectLock = types.BoolValue(bucket.ObjectLockEnabled) return nil } - -const ( - // Two object storage resources created in the same apply enable the project concurrently; - // the API answers the losing call with 409. See enableProject. - enableProjectAttempts = 4 -) - -// Overridden in tests to keep them fast. -var enableProjectRetryDelay = 2 * time.Second - -// enableProject enables object storage for the specified project. If the project is already enabled, nothing happens -func enableProject(ctx context.Context, model *Model, region string, client objectstorage.DefaultAPI) error { - projectId := model.ProjectId.ValueString() - - // From the object storage OAS: Creation will also be successful if the project is already enabled, but will not create a duplicate. - // That holds for sequential calls. Two object storage resources created in the same apply call this concurrently, - // and the API rejects the second one with 409 project.create_conflict ("Two concurrent calls try to create the - // same project"). Retrying is safe: once the competing call has finished, enabling an already enabled project succeeds. - config := utils.RetryConfig{ - Attempts: enableProjectAttempts, - Delay: enableProjectRetryDelay, - RetryStatusCodes: []int{http.StatusConflict}, - } - if _, err := utils.RetryRequest(ctx, client.EnableService(ctx, projectId, region).Execute, config); err != nil { - return fmt.Errorf("failed to create object storage project: %w", err) - } - return nil -} diff --git a/stackit/internal/services/objectstorage/bucket/resource_test.go b/stackit/internal/services/objectstorage/bucket/resource_test.go index 530f7f486..2e0d739e1 100644 --- a/stackit/internal/services/objectstorage/bucket/resource_test.go +++ b/stackit/internal/services/objectstorage/bucket/resource_test.go @@ -1,33 +1,15 @@ package objectstorage import ( - "context" _ "embed" "fmt" "testing" - "time" "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework/types" objectstorage "github.com/stackitcloud/stackit-sdk-go/services/objectstorage/v2api" ) -type mockSettings struct { - returnError bool -} - -func newAPIMock(settings *mockSettings) objectstorage.DefaultAPI { - return &objectstorage.DefaultAPIServiceMock{ - EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { - if settings.returnError { - return nil, fmt.Errorf("create project failed") - } - - return &objectstorage.ProjectStatus{}, nil - }), - } -} - func TestMapFields(t *testing.T) { const testRegion = "eu01" id := fmt.Sprintf("%s,%s,%s", "pid", testRegion, "bname") @@ -121,45 +103,3 @@ func TestMapFields(t *testing.T) { }) } } - -func TestEnableProject(t *testing.T) { - // enableProject retries, and the mock returns a plain error rather than an - // *oapierror.GenericOpenAPIError - RetryRequest only filters by status code - // when it can type-assert the error, so the failing case uses up every - // attempt. Without shrinking the delay this test would sleep for seconds. - oldDelay := enableProjectRetryDelay - enableProjectRetryDelay = time.Millisecond - defer func() { enableProjectRetryDelay = oldDelay }() - - tests := []struct { - description string - enableFails bool - isValid bool - }{ - { - "default_values", - false, - true, - }, - { - "error_response", - true, - false, - }, - } - for _, tt := range tests { - t.Run(tt.description, func(t *testing.T) { - client := newAPIMock(&mockSettings{ - returnError: tt.enableFails, - }) - - err := enableProject(context.Background(), &Model{}, "eu01", client) - if !tt.isValid && err == nil { - t.Fatalf("Should have failed") - } - if tt.isValid && err != nil { - t.Fatalf("Should not have failed: %v", err) - } - }) - } -} diff --git a/stackit/internal/services/objectstorage/credential/resource.go b/stackit/internal/services/objectstorage/credential/resource.go index df363505b..08393489d 100644 --- a/stackit/internal/services/objectstorage/credential/resource.go +++ b/stackit/internal/services/objectstorage/credential/resource.go @@ -281,7 +281,7 @@ func (r *credentialResource) Create(ctx context.Context, req resource.CreateRequ ctx = tflog.SetField(ctx, "region", region) // Handle project init - err := enableProject(ctx, &model, region, r.client.DefaultAPI) + err := objectstorageUtils.EnableProject(ctx, projectId, region, r.client.DefaultAPI) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating credential", fmt.Sprintf("Enabling object storage project before creation: %v", err)) return @@ -490,34 +490,6 @@ func (r *credentialResource) ImportState(ctx context.Context, req resource.Impor tflog.Info(ctx, "ObjectStorage credential state imported") } -const ( - // Two object storage resources created in the same apply enable the project concurrently; - // the API answers the losing call with 409. See enableProject. - enableProjectAttempts = 4 -) - -// Overridden in tests to keep them fast. -var enableProjectRetryDelay = 2 * time.Second - -// enableProject enables object storage for the specified project. If the project is already enabled, nothing happens -func enableProject(ctx context.Context, model *Model, region string, client objectstorage.DefaultAPI) error { - projectId := model.ProjectId.ValueString() - - // From the object storage OAS: Creation will also be successful if the project is already enabled, but will not create a duplicate. - // That holds for sequential calls. Two object storage resources created in the same apply call this concurrently, - // and the API rejects the second one with 409 project.create_conflict ("Two concurrent calls try to create the - // same project"). Retrying is safe: once the competing call has finished, enabling an already enabled project succeeds. - config := utils.RetryConfig{ - Attempts: enableProjectAttempts, - Delay: enableProjectRetryDelay, - RetryStatusCodes: []int{http.StatusConflict}, - } - if _, err := utils.RetryRequest(ctx, client.EnableService(ctx, projectId, region).Execute, config); err != nil { - return fmt.Errorf("failed to create object storage project: %w", err) - } - return nil -} - func toCreatePayload(model *Model) (*objectstorage.CreateAccessKeyPayload, error) { if model == nil { return nil, fmt.Errorf("nil model") diff --git a/stackit/internal/services/objectstorage/credential/resource_test.go b/stackit/internal/services/objectstorage/credential/resource_test.go index 35207feb3..7a81e9b32 100644 --- a/stackit/internal/services/objectstorage/credential/resource_test.go +++ b/stackit/internal/services/objectstorage/credential/resource_test.go @@ -15,22 +15,6 @@ import ( objectstorage "github.com/stackitcloud/stackit-sdk-go/services/objectstorage/v2api" ) -type mockSettings struct { - returnError bool -} - -func newAPIMock(settings *mockSettings) objectstorage.DefaultAPI { - return &objectstorage.DefaultAPIServiceMock{ - EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { - if settings.returnError { - return nil, fmt.Errorf("create project failed") - } - - return &objectstorage.ProjectStatus{}, nil - }), - } -} - func TestMapFields(t *testing.T) { now := time.Now() const testRegion = "eu01" @@ -160,79 +144,6 @@ func TestMapFields(t *testing.T) { } } -func TestEnableProject(t *testing.T) { - // enableProject retries, and the mock returns a plain error rather than an - // *oapierror.GenericOpenAPIError - RetryRequest only filters by status code - // when it can type-assert the error, so the failing case uses up every - // attempt. Without shrinking the delay this test would sleep for seconds. - oldDelay := enableProjectRetryDelay - enableProjectRetryDelay = time.Millisecond - defer func() { enableProjectRetryDelay = oldDelay }() - - const testRegion = "eu01" - id := fmt.Sprintf("%s,%s,%s", "pid", testRegion, "cgid,cid") - tests := []struct { - description string - expected Model - enableFails bool - isValid bool - }{ - { - "default_values", - Model{ - Id: types.StringValue(id), - ProjectId: types.StringValue("pid"), - CredentialsGroupId: types.StringValue("cgid"), - CredentialId: types.StringValue("cid"), - Name: types.StringNull(), - AccessKey: types.StringNull(), - SecretAccessKey: types.StringNull(), - ExpirationTimestamp: types.StringNull(), - RotateWhenChanged: types.MapNull(types.StringType), - }, - false, - true, - }, - { - "error_response", - Model{ - Id: types.StringValue(id), - ProjectId: types.StringValue("pid"), - CredentialsGroupId: types.StringValue("cgid"), - CredentialId: types.StringValue("cid"), - Name: types.StringNull(), - AccessKey: types.StringNull(), - SecretAccessKey: types.StringNull(), - ExpirationTimestamp: types.StringNull(), - RotateWhenChanged: types.MapNull(types.StringType), - }, - true, - false, - }, - } - for _, tt := range tests { - t.Run(tt.description, func(t *testing.T) { - client := newAPIMock(&mockSettings{ - returnError: tt.enableFails, - }) - - model := &Model{ - ProjectId: tt.expected.ProjectId, - CredentialsGroupId: tt.expected.CredentialsGroupId, - CredentialId: tt.expected.CredentialId, - RotateWhenChanged: types.MapNull(types.StringType), - } - err := enableProject(context.Background(), model, "eu01", client) - if !tt.isValid && err == nil { - t.Fatalf("Should have failed") - } - if tt.isValid && err != nil { - t.Fatalf("Should not have failed: %v", err) - } - }) - } -} - func TestReadCredentials(t *testing.T) { now := time.Now() const testRegion = "eu01" diff --git a/stackit/internal/services/objectstorage/credentialsgroup/resource.go b/stackit/internal/services/objectstorage/credentialsgroup/resource.go index 1870a32bd..5196a9f4b 100644 --- a/stackit/internal/services/objectstorage/credentialsgroup/resource.go +++ b/stackit/internal/services/objectstorage/credentialsgroup/resource.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" "strings" - "time" "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/conversion" objectstorageUtils "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/objectstorage/utils" @@ -193,7 +192,7 @@ func (r *credentialsGroupResource) Create(ctx context.Context, req resource.Crea } // Handle project init - err := enableProject(ctx, &model, region, r.client.DefaultAPI) + err := objectstorageUtils.EnableProject(ctx, projectId, region, r.client.DefaultAPI) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating credentials group", fmt.Sprintf("Enabling object storage project before creation: %v", err)) return @@ -381,34 +380,6 @@ func mapCredentialsGroup(credentialsGroup objectstorage.CredentialsGroup, model return nil } -const ( - // Two object storage resources created in the same apply enable the project concurrently; - // the API answers the losing call with 409. See enableProject. - enableProjectAttempts = 4 -) - -// Overridden in tests to keep them fast. -var enableProjectRetryDelay = 2 * time.Second - -// enableProject enables object storage for the specified project. If the project is already enabled, nothing happens -func enableProject(ctx context.Context, model *Model, region string, client objectstorage.DefaultAPI) error { - projectId := model.ProjectId.ValueString() - - // From the object storage OAS: Creation will also be successful if the project is already enabled, but will not create a duplicate. - // That holds for sequential calls. Two object storage resources created in the same apply call this concurrently, - // and the API rejects the second one with 409 project.create_conflict ("Two concurrent calls try to create the - // same project"). Retrying is safe: once the competing call has finished, enabling an already enabled project succeeds. - config := utils.RetryConfig{ - Attempts: enableProjectAttempts, - Delay: enableProjectRetryDelay, - RetryStatusCodes: []int{http.StatusConflict}, - } - if _, err := utils.RetryRequest(ctx, client.EnableService(ctx, projectId, region).Execute, config); err != nil { - return fmt.Errorf("failed to create object storage project: %w", err) - } - return nil -} - // readCredentialsGroups gets all the existing credentials groups for the specified project, // finds the credentials group that is being read and updates the state. // Returns True if the credential was found, False otherwise. diff --git a/stackit/internal/services/objectstorage/credentialsgroup/resource_test.go b/stackit/internal/services/objectstorage/credentialsgroup/resource_test.go index 2154b8ffb..11dff8e22 100644 --- a/stackit/internal/services/objectstorage/credentialsgroup/resource_test.go +++ b/stackit/internal/services/objectstorage/credentialsgroup/resource_test.go @@ -3,11 +3,7 @@ package objectstorage import ( "context" "fmt" - "net/http" "testing" - "time" - - "github.com/stackitcloud/stackit-sdk-go/core/oapierror" "github.com/google/go-cmp/cmp" "github.com/hashicorp/terraform-plugin-framework/types" @@ -21,13 +17,6 @@ type mockSettings struct { func newAPIMock(settings *mockSettings) objectstorage.DefaultAPI { return &objectstorage.DefaultAPIServiceMock{ - EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { - if settings.returnError { - return nil, fmt.Errorf("create project failed") - } - - return &objectstorage.ProjectStatus{}, nil - }), ListCredentialsGroupsExecuteMock: new(func(_ objectstorage.ApiListCredentialsGroupsRequest) (*objectstorage.ListCredentialsGroupsResponse, error) { if settings.returnError { return nil, fmt.Errorf("get credentials groups failed") @@ -134,48 +123,6 @@ func TestMapFields(t *testing.T) { } } -func TestEnableProject(t *testing.T) { - // enableProject retries, and the mock returns a plain error rather than an - // *oapierror.GenericOpenAPIError - RetryRequest only filters by status code - // when it can type-assert the error, so the failing case uses up every - // attempt. Without shrinking the delay this test would sleep for seconds. - oldDelay := enableProjectRetryDelay - enableProjectRetryDelay = time.Millisecond - defer func() { enableProjectRetryDelay = oldDelay }() - - tests := []struct { - description string - enableFails bool - isValid bool - }{ - { - "default_values", - false, - true, - }, - { - "error_response", - true, - false, - }, - } - for _, tt := range tests { - t.Run(tt.description, func(t *testing.T) { - client := newAPIMock(&mockSettings{ - returnError: tt.enableFails, - }) - - err := enableProject(context.Background(), &Model{}, "eu01", client) - if !tt.isValid && err == nil { - t.Fatalf("Should have failed") - } - if tt.isValid && err != nil { - t.Fatalf("Should not have failed: %v", err) - } - }) - } -} - func TestReadCredentialsGroups(t *testing.T) { const testRegion = "eu01" id := fmt.Sprintf("%s,%s,%s", "pid", testRegion, "cid") @@ -329,70 +276,3 @@ func TestReadCredentialsGroups(t *testing.T) { }) } } - -// Two object storage resources created in the same apply enable the project concurrently. -// The API answers the losing call with 409 project.create_conflict; enableProject must retry -// instead of failing the apply. -func TestEnableProjectRetriesOnConflict(t *testing.T) { - tests := []struct { - description string - conflicts int - isValid bool - wantAttempts int - }{ - {"succeeds immediately", 0, true, 1}, - {"one conflict, then success", 1, true, 2}, - {"conflicts until the attempts are used up", enableProjectAttempts, false, enableProjectAttempts}, - } - - old := enableProjectRetryDelay - enableProjectRetryDelay = time.Millisecond - defer func() { enableProjectRetryDelay = old }() - - for _, tt := range tests { - t.Run(tt.description, func(t *testing.T) { - attempts := 0 - client := &objectstorage.DefaultAPIServiceMock{ - EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { - attempts++ - if attempts <= tt.conflicts { - return nil, &oapierror.GenericOpenAPIError{StatusCode: http.StatusConflict} - } - return &objectstorage.ProjectStatus{}, nil - }), - } - - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - err := enableProject(ctx, &Model{}, "eu01", client) - if tt.isValid && err != nil { - t.Fatalf("Should not have failed: %v", err) - } - if !tt.isValid && err == nil { - t.Fatal("Should have failed") - } - if attempts != tt.wantAttempts { - t.Fatalf("Expected %d attempts, got %d", tt.wantAttempts, attempts) - } - }) - } -} - -// A non-conflict error must not be retried. -func TestEnableProjectDoesNotRetryOtherErrors(t *testing.T) { - attempts := 0 - client := &objectstorage.DefaultAPIServiceMock{ - EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { - attempts++ - return nil, &oapierror.GenericOpenAPIError{StatusCode: http.StatusForbidden} - }), - } - - if err := enableProject(context.Background(), &Model{}, "eu01", client); err == nil { - t.Fatal("Should have failed") - } - if attempts != 1 { - t.Fatalf("Expected a single attempt, got %d", attempts) - } -} diff --git a/stackit/internal/services/objectstorage/utils/util.go b/stackit/internal/services/objectstorage/utils/util.go index b107ced32..efd372efb 100644 --- a/stackit/internal/services/objectstorage/utils/util.go +++ b/stackit/internal/services/objectstorage/utils/util.go @@ -3,6 +3,8 @@ package utils import ( "context" "fmt" + "net/http" + "time" objectstorage "github.com/stackitcloud/stackit-sdk-go/services/objectstorage/v2api" @@ -13,6 +15,32 @@ import ( "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils" ) +const ( + // Two object storage resources created in the same apply enable the project concurrently; + // the API answers the losing call with 409. See EnableProject. + enableProjectAttempts = 4 +) + +// Overridden in tests to keep them fast. +var enableProjectRetryDelay = 2 * time.Second + +// EnableProject enables object storage for the specified project. If the project is already enabled, nothing happens +func EnableProject(ctx context.Context, projectId, region string, client objectstorage.DefaultAPI) error { + // From the object storage OAS: Creation will also be successful if the project is already enabled, but will not create a duplicate. + // That holds for sequential calls. Two object storage resources created in the same apply call this concurrently, + // and the API rejects the second one with 409 project.create_conflict ("Two concurrent calls try to create the + // same project"). Retrying is safe: once the competing call has finished, enabling an already enabled project succeeds. + config := utils.RetryConfig{ + Attempts: enableProjectAttempts, + Delay: enableProjectRetryDelay, + RetryStatusCodes: []int{http.StatusConflict}, + } + if _, err := utils.RetryRequest(ctx, client.EnableService(ctx, projectId, region).Execute, config); err != nil { + return fmt.Errorf("failed to create object storage project: %w", err) + } + return nil +} + func ConfigureClient(ctx context.Context, providerData *core.ProviderData, diags *diag.Diagnostics) *objectstorage.APIClient { apiClientConfigOptions := []config.ConfigurationOption{ config.WithCustomAuth(providerData.RoundTripper), diff --git a/stackit/internal/services/objectstorage/utils/util_test.go b/stackit/internal/services/objectstorage/utils/util_test.go index 669241adb..952d40e8a 100644 --- a/stackit/internal/services/objectstorage/utils/util_test.go +++ b/stackit/internal/services/objectstorage/utils/util_test.go @@ -2,13 +2,17 @@ package utils import ( "context" + "fmt" + "net/http" "os" "reflect" "testing" + "time" "github.com/hashicorp/terraform-plugin-framework/diag" sdkClients "github.com/stackitcloud/stackit-sdk-go/core/clients" "github.com/stackitcloud/stackit-sdk-go/core/config" + "github.com/stackitcloud/stackit-sdk-go/core/oapierror" objectstorage "github.com/stackitcloud/stackit-sdk-go/services/objectstorage/v2api" "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core" @@ -92,3 +96,118 @@ func TestConfigureClient(t *testing.T) { }) } } + +func TestEnableProject(t *testing.T) { + // EnableProject retries, and the mock returns a plain error rather than an + // *oapierror.GenericOpenAPIError - RetryRequest only filters by status code + // when it can type-assert the error, so the failing case uses up every + // attempt. Without shrinking the delay this test would sleep for seconds. + oldDelay := enableProjectRetryDelay + enableProjectRetryDelay = time.Millisecond + defer func() { enableProjectRetryDelay = oldDelay }() + + tests := []struct { + description string + enableFails bool + isValid bool + }{ + { + "default_values", + false, + true, + }, + { + "error_response", + true, + false, + }, + } + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + client := &objectstorage.DefaultAPIServiceMock{ + EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { + if tt.enableFails { + return nil, fmt.Errorf("create project failed") + } + + return &objectstorage.ProjectStatus{}, nil + }), + } + + err := EnableProject(context.Background(), "pid", "eu01", client) + if !tt.isValid && err == nil { + t.Fatalf("Should have failed") + } + if tt.isValid && err != nil { + t.Fatalf("Should not have failed: %v", err) + } + }) + } +} + +// Two object storage resources created in the same apply enable the project concurrently. +// The API answers the losing call with 409 project.create_conflict; EnableProject must retry +// instead of failing the apply. +func TestEnableProjectRetriesOnConflict(t *testing.T) { + tests := []struct { + description string + conflicts int + isValid bool + wantAttempts int + }{ + {"succeeds immediately", 0, true, 1}, + {"one conflict, then success", 1, true, 2}, + {"conflicts until the attempts are used up", enableProjectAttempts, false, enableProjectAttempts}, + } + + old := enableProjectRetryDelay + enableProjectRetryDelay = time.Millisecond + defer func() { enableProjectRetryDelay = old }() + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + attempts := 0 + client := &objectstorage.DefaultAPIServiceMock{ + EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { + attempts++ + if attempts <= tt.conflicts { + return nil, &oapierror.GenericOpenAPIError{StatusCode: http.StatusConflict} + } + return &objectstorage.ProjectStatus{}, nil + }), + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + err := EnableProject(ctx, "pid", "eu01", client) + if tt.isValid && err != nil { + t.Fatalf("Should not have failed: %v", err) + } + if !tt.isValid && err == nil { + t.Fatal("Should have failed") + } + if attempts != tt.wantAttempts { + t.Fatalf("Expected %d attempts, got %d", tt.wantAttempts, attempts) + } + }) + } +} + +// A non-conflict error must not be retried. +func TestEnableProjectDoesNotRetryOtherErrors(t *testing.T) { + attempts := 0 + client := &objectstorage.DefaultAPIServiceMock{ + EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { + attempts++ + return nil, &oapierror.GenericOpenAPIError{StatusCode: http.StatusForbidden} + }), + } + + if err := EnableProject(context.Background(), "pid", "eu01", client); err == nil { + t.Fatal("Should have failed") + } + if attempts != 1 { + t.Fatalf("Expected a single attempt, got %d", attempts) + } +} From b244ec7a956aca1150f07498e1540bfadd13e0bd Mon Sep 17 00:00:00 2001 From: Fabian Hardt Date: Tue, 1 Sep 2026 11:47:27 +0200 Subject: [PATCH 4/7] docs(objectstorage): drop outdated depends_on advice from bucket and credentials group With the 409 retry in place, bucket and credentialsgroup no longer need to be created sequentially via depends_on. Docs regenerated with tfplugindocs. --- docs/resources/objectstorage_bucket.md | 4 ++-- docs/resources/objectstorage_credentials_group.md | 4 ++-- stackit/internal/services/objectstorage/bucket/resource.go | 2 +- .../services/objectstorage/credentialsgroup/resource.go | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/resources/objectstorage_bucket.md b/docs/resources/objectstorage_bucket.md index d33b52065..d567106d8 100644 --- a/docs/resources/objectstorage_bucket.md +++ b/docs/resources/objectstorage_bucket.md @@ -3,13 +3,13 @@ page_title: "stackit_objectstorage_bucket Resource - stackit" subcategory: "" description: |- - ObjectStorage bucket resource schema. Must have a region specified in the provider configuration. If you are creating credentialsgroup and bucket resources simultaneously, please include the depends_on field so that they are created sequentially. This prevents errors from concurrent calls to the service enablement that is done in the background. + ObjectStorage bucket resource schema. Must have a region specified in the provider configuration. ~> This resource cannot be destroyed if the bucket contains objects. Please ensure the bucket is empty before attempting to destroy it. --- # stackit_objectstorage_bucket (Resource) -ObjectStorage bucket resource schema. Must have a `region` specified in the provider configuration. If you are creating `credentialsgroup` and `bucket` resources simultaneously, please include the `depends_on` field so that they are created sequentially. This prevents errors from concurrent calls to the service enablement that is done in the background. +ObjectStorage bucket resource schema. Must have a `region` specified in the provider configuration. ~> This resource cannot be destroyed if the bucket contains objects. Please ensure the bucket is empty before attempting to destroy it. diff --git a/docs/resources/objectstorage_credentials_group.md b/docs/resources/objectstorage_credentials_group.md index a4c90d8b4..909566e4f 100644 --- a/docs/resources/objectstorage_credentials_group.md +++ b/docs/resources/objectstorage_credentials_group.md @@ -3,12 +3,12 @@ page_title: "stackit_objectstorage_credentials_group Resource - stackit" subcategory: "" description: |- - ObjectStorage credentials group resource schema. Must have a region specified in the provider configuration. If you are creating credentialsgroup and bucket resources simultaneously, please include the depends_on field so that they are created sequentially. This prevents errors from concurrent calls to the service enablement that is done in the background. + ObjectStorage credentials group resource schema. Must have a region specified in the provider configuration. --- # stackit_objectstorage_credentials_group (Resource) -ObjectStorage credentials group resource schema. Must have a `region` specified in the provider configuration. If you are creating `credentialsgroup` and `bucket` resources simultaneously, please include the `depends_on` field so that they are created sequentially. This prevents errors from concurrent calls to the service enablement that is done in the background. +ObjectStorage credentials group resource schema. Must have a `region` specified in the provider configuration. ## Example Usage diff --git a/stackit/internal/services/objectstorage/bucket/resource.go b/stackit/internal/services/objectstorage/bucket/resource.go index 0ed7a7348..7bcb1cab5 100644 --- a/stackit/internal/services/objectstorage/bucket/resource.go +++ b/stackit/internal/services/objectstorage/bucket/resource.go @@ -119,7 +119,7 @@ func (r *bucketResource) Configure(ctx context.Context, req resource.ConfigureRe // Schema defines the schema for the resource. func (r *bucketResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { descriptions := map[string]string{ - "main": "ObjectStorage bucket resource schema. Must have a `region` specified in the provider configuration. If you are creating `credentialsgroup` and `bucket` resources simultaneously, please include the `depends_on` field so that they are created sequentially. This prevents errors from concurrent calls to the service enablement that is done in the background.\n\n" + + "main": "ObjectStorage bucket resource schema. Must have a `region` specified in the provider configuration.\n\n" + "~> This resource cannot be destroyed if the bucket contains objects. Please ensure the bucket is empty before attempting to destroy it.", "id": "Terraform's internal resource identifier. It is structured as \"`project_id`,`region`,`name`\".", "name": "The bucket name. It must be DNS conform.", diff --git a/stackit/internal/services/objectstorage/credentialsgroup/resource.go b/stackit/internal/services/objectstorage/credentialsgroup/resource.go index 5196a9f4b..6d69d0413 100644 --- a/stackit/internal/services/objectstorage/credentialsgroup/resource.go +++ b/stackit/internal/services/objectstorage/credentialsgroup/resource.go @@ -108,7 +108,7 @@ func (r *credentialsGroupResource) Configure(ctx context.Context, req resource.C // Schema defines the schema for the resource. func (r *credentialsGroupResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { descriptions := map[string]string{ - "main": "ObjectStorage credentials group resource schema. Must have a `region` specified in the provider configuration. If you are creating `credentialsgroup` and `bucket` resources simultaneously, please include the `depends_on` field so that they are created sequentially. This prevents errors from concurrent calls to the service enablement that is done in the background.", + "main": "ObjectStorage credentials group resource schema. Must have a `region` specified in the provider configuration.", "id": "Terraform's internal data source identifier. It is structured as \"`project_id`,`region`,`credentials_group_id`\".", "credentials_group_id": "The credentials group ID", "name": "The credentials group's display name.", From 3955779635b35d66367ee68107fb7f59a15186b4 Mon Sep 17 00:00:00 2001 From: Fabian Hardt Date: Tue, 1 Sep 2026 11:56:50 +0200 Subject: [PATCH 5/7] chore(objectstorage): tighten EnableProject comments --- .../internal/services/objectstorage/utils/util.go | 14 ++++---------- .../services/objectstorage/utils/util_test.go | 9 ++------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/stackit/internal/services/objectstorage/utils/util.go b/stackit/internal/services/objectstorage/utils/util.go index efd372efb..1fc088e82 100644 --- a/stackit/internal/services/objectstorage/utils/util.go +++ b/stackit/internal/services/objectstorage/utils/util.go @@ -15,21 +15,15 @@ import ( "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils" ) -const ( - // Two object storage resources created in the same apply enable the project concurrently; - // the API answers the losing call with 409. See EnableProject. - enableProjectAttempts = 4 -) +const enableProjectAttempts = 4 // Overridden in tests to keep them fast. var enableProjectRetryDelay = 2 * time.Second -// EnableProject enables object storage for the specified project. If the project is already enabled, nothing happens +// EnableProject enables object storage for the specified project. If the project is already enabled, nothing happens. +// Two resources created in the same apply call this concurrently and the API rejects the losing call with +// 409 project.create_conflict; retrying is safe, since enabling an already enabled project succeeds. func EnableProject(ctx context.Context, projectId, region string, client objectstorage.DefaultAPI) error { - // From the object storage OAS: Creation will also be successful if the project is already enabled, but will not create a duplicate. - // That holds for sequential calls. Two object storage resources created in the same apply call this concurrently, - // and the API rejects the second one with 409 project.create_conflict ("Two concurrent calls try to create the - // same project"). Retrying is safe: once the competing call has finished, enabling an already enabled project succeeds. config := utils.RetryConfig{ Attempts: enableProjectAttempts, Delay: enableProjectRetryDelay, diff --git a/stackit/internal/services/objectstorage/utils/util_test.go b/stackit/internal/services/objectstorage/utils/util_test.go index 952d40e8a..7b95ed5ca 100644 --- a/stackit/internal/services/objectstorage/utils/util_test.go +++ b/stackit/internal/services/objectstorage/utils/util_test.go @@ -98,10 +98,7 @@ func TestConfigureClient(t *testing.T) { } func TestEnableProject(t *testing.T) { - // EnableProject retries, and the mock returns a plain error rather than an - // *oapierror.GenericOpenAPIError - RetryRequest only filters by status code - // when it can type-assert the error, so the failing case uses up every - // attempt. Without shrinking the delay this test would sleep for seconds. + // The plain mock error is retried on every attempt; shrink the delay to keep the test fast. oldDelay := enableProjectRetryDelay enableProjectRetryDelay = time.Millisecond defer func() { enableProjectRetryDelay = oldDelay }() @@ -145,9 +142,7 @@ func TestEnableProject(t *testing.T) { } } -// Two object storage resources created in the same apply enable the project concurrently. -// The API answers the losing call with 409 project.create_conflict; EnableProject must retry -// instead of failing the apply. +// A 409 from a concurrent enable call must be retried instead of failing the apply. func TestEnableProjectRetriesOnConflict(t *testing.T) { tests := []struct { description string From d7cfdb052df20d4eca9820c48b773bc66f59952c Mon Sep 17 00:00:00 2001 From: Fabian Hardt Date: Tue, 1 Sep 2026 16:19:07 +0200 Subject: [PATCH 6/7] refactor(objectstorage): use synctest in EnableProject tests Fake time via testing/synctest replaces overriding the retry delay, so enableProjectRetryDelay becomes a const. Also rewords the wrapped error to "enable object storage project", which is what the call does. --- .../services/objectstorage/utils/util.go | 10 +-- .../services/objectstorage/utils/util_test.go | 90 +++++++++---------- 2 files changed, 48 insertions(+), 52 deletions(-) diff --git a/stackit/internal/services/objectstorage/utils/util.go b/stackit/internal/services/objectstorage/utils/util.go index 1fc088e82..533d0ef00 100644 --- a/stackit/internal/services/objectstorage/utils/util.go +++ b/stackit/internal/services/objectstorage/utils/util.go @@ -15,10 +15,10 @@ import ( "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils" ) -const enableProjectAttempts = 4 - -// Overridden in tests to keep them fast. -var enableProjectRetryDelay = 2 * time.Second +const ( + enableProjectAttempts = 4 + enableProjectRetryDelay = 2 * time.Second +) // EnableProject enables object storage for the specified project. If the project is already enabled, nothing happens. // Two resources created in the same apply call this concurrently and the API rejects the losing call with @@ -30,7 +30,7 @@ func EnableProject(ctx context.Context, projectId, region string, client objects RetryStatusCodes: []int{http.StatusConflict}, } if _, err := utils.RetryRequest(ctx, client.EnableService(ctx, projectId, region).Execute, config); err != nil { - return fmt.Errorf("failed to create object storage project: %w", err) + return fmt.Errorf("enable object storage project: %w", err) } return nil } diff --git a/stackit/internal/services/objectstorage/utils/util_test.go b/stackit/internal/services/objectstorage/utils/util_test.go index 7b95ed5ca..1a174b0dd 100644 --- a/stackit/internal/services/objectstorage/utils/util_test.go +++ b/stackit/internal/services/objectstorage/utils/util_test.go @@ -7,6 +7,7 @@ import ( "os" "reflect" "testing" + "testing/synctest" "time" "github.com/hashicorp/terraform-plugin-framework/diag" @@ -98,11 +99,6 @@ func TestConfigureClient(t *testing.T) { } func TestEnableProject(t *testing.T) { - // The plain mock error is retried on every attempt; shrink the delay to keep the test fast. - oldDelay := enableProjectRetryDelay - enableProjectRetryDelay = time.Millisecond - defer func() { enableProjectRetryDelay = oldDelay }() - tests := []struct { description string enableFails bool @@ -121,23 +117,25 @@ func TestEnableProject(t *testing.T) { } for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { - client := &objectstorage.DefaultAPIServiceMock{ - EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { - if tt.enableFails { - return nil, fmt.Errorf("create project failed") - } - - return &objectstorage.ProjectStatus{}, nil - }), - } + synctest.Test(t, func(t *testing.T) { + client := &objectstorage.DefaultAPIServiceMock{ + EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { + if tt.enableFails { + return nil, fmt.Errorf("create project failed") + } + + return &objectstorage.ProjectStatus{}, nil + }), + } - err := EnableProject(context.Background(), "pid", "eu01", client) - if !tt.isValid && err == nil { - t.Fatalf("Should have failed") - } - if tt.isValid && err != nil { - t.Fatalf("Should not have failed: %v", err) - } + err := EnableProject(context.Background(), "pid", "eu01", client) + if !tt.isValid && err == nil { + t.Fatalf("Should have failed") + } + if tt.isValid && err != nil { + t.Fatalf("Should not have failed: %v", err) + } + }) }) } } @@ -155,36 +153,34 @@ func TestEnableProjectRetriesOnConflict(t *testing.T) { {"conflicts until the attempts are used up", enableProjectAttempts, false, enableProjectAttempts}, } - old := enableProjectRetryDelay - enableProjectRetryDelay = time.Millisecond - defer func() { enableProjectRetryDelay = old }() - for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { - attempts := 0 - client := &objectstorage.DefaultAPIServiceMock{ - EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { - attempts++ - if attempts <= tt.conflicts { - return nil, &oapierror.GenericOpenAPIError{StatusCode: http.StatusConflict} - } - return &objectstorage.ProjectStatus{}, nil - }), - } + synctest.Test(t, func(t *testing.T) { + attempts := 0 + client := &objectstorage.DefaultAPIServiceMock{ + EnableServiceExecuteMock: new(func(_ objectstorage.ApiEnableServiceRequest) (*objectstorage.ProjectStatus, error) { + attempts++ + if attempts <= tt.conflicts { + return nil, &oapierror.GenericOpenAPIError{StatusCode: http.StatusConflict} + } + return &objectstorage.ProjectStatus{}, nil + }), + } - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() - err := EnableProject(ctx, "pid", "eu01", client) - if tt.isValid && err != nil { - t.Fatalf("Should not have failed: %v", err) - } - if !tt.isValid && err == nil { - t.Fatal("Should have failed") - } - if attempts != tt.wantAttempts { - t.Fatalf("Expected %d attempts, got %d", tt.wantAttempts, attempts) - } + err := EnableProject(ctx, "pid", "eu01", client) + if tt.isValid && err != nil { + t.Fatalf("Should not have failed: %v", err) + } + if !tt.isValid && err == nil { + t.Fatal("Should have failed") + } + if attempts != tt.wantAttempts { + t.Fatalf("Expected %d attempts, got %d", tt.wantAttempts, attempts) + } + }) }) } } From ffcdd3eca0eff5aab962a18f059414b96f507c4d Mon Sep 17 00:00:00 2001 From: Fabian Hardt Date: Tue, 1 Sep 2026 17:23:37 +0200 Subject: [PATCH 7/7] fix(objectstorage): rename retry config variable shadowing config package --- stackit/internal/services/objectstorage/utils/util.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stackit/internal/services/objectstorage/utils/util.go b/stackit/internal/services/objectstorage/utils/util.go index 533d0ef00..8e3ab1a8a 100644 --- a/stackit/internal/services/objectstorage/utils/util.go +++ b/stackit/internal/services/objectstorage/utils/util.go @@ -24,12 +24,12 @@ const ( // Two resources created in the same apply call this concurrently and the API rejects the losing call with // 409 project.create_conflict; retrying is safe, since enabling an already enabled project succeeds. func EnableProject(ctx context.Context, projectId, region string, client objectstorage.DefaultAPI) error { - config := utils.RetryConfig{ + retryConfig := utils.RetryConfig{ Attempts: enableProjectAttempts, Delay: enableProjectRetryDelay, RetryStatusCodes: []int{http.StatusConflict}, } - if _, err := utils.RetryRequest(ctx, client.EnableService(ctx, projectId, region).Execute, config); err != nil { + if _, err := utils.RetryRequest(ctx, client.EnableService(ctx, projectId, region).Execute, retryConfig); err != nil { return fmt.Errorf("enable object storage project: %w", err) } return nil