From dab283ad2721d659abf4de73f85f1e4bd90ed5d1 Mon Sep 17 00:00:00 2001 From: Fabian Hardt Date: Sun, 2 Aug 2026 11:42:28 +0200 Subject: [PATCH 1/2] 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/2] 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