fix(objectstorage): retry enabling the project on 409 conflict

[FabianHardt]

Problem

stackit_objectstorage_bucket, stackit_objectstorage_credential and
stackit_objectstorage_credentials_group each enable object storage for the
project before creating their own object. When two of them are created in the
same apply and nothing forces an order, 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 actually wrong — the competing call enables
the project a moment later.

Minimal reproducer (both resources in one config, no reference between them):

resource "stackit_objectstorage_bucket" "b" {
  project_id = var.project_id
  name       = "some-bucket"
}

resource "stackit_objectstorage_credentials_group" "g" {
  project_id = var.project_id
  name       = "some-group"
}

The comment in enableProject already assumes idempotency — "Creation will
also be successful if the project is already enabled" — which holds for
sequential calls but not for concurrent ones.

Change

enableProject retries on 409 and leaves every other error untouched. An
apply no longer depends on the order in which Terraform happens to start the
resources.

depends_on works around it today, but that requires knowing about an implicit
API call the resource documentation does not mention — the error message points
at object storage projects, not at a missing dependency.

The retry is deliberately narrow rather than utils.RetryRequest: that helper
also retries errors which are not GenericOpenAPIError, which would slow the
existing unit tests down.

Tests

Added to credentialsgroup/resource_test.go:

• TestEnableProjectRetriesOnConflict — succeeds immediately, one conflict then
  success, and conflicts until the attempts are used up; asserts the number of
  API calls in each case
• TestEnableProjectDoesNotRetryOtherErrors — a 403 must fail on the first
  attempt

The retry delay is a package variable so tests can shorten it.

go test ./stackit/internal/services/objectstorage/...   -> ok (all packages)
gofmt -l / go vet                                       -> clean

Note

The three copies of enableProject are identical; I kept the duplication to
keep the diff reviewable. Happy to extract it into
objectstorage/utils instead if you prefer.

[FabianHardt]

Good call, thanks — swapped in utils.RetryRequest. It drops the hand-rolled loop from all three resources (net -11 lines) and keeps the behaviour I was after: 4 attempts, 2s apart, retrying only on 409.

One difference I want to flag rather than bury, because it's a change and not a refactor: RetryRequest only applies RetryStatusCodes when the error type-asserts to *oapierror.GenericOpenAPIError. Anything else — a transport or network failure — now gets retried too, where my loop bailed out immediately. For an idempotent enable call that seems fine to me, but it's your call whether that's the semantics you want.

It surfaced in the existing TestEnableProject: its mock returns a plain error, so the failing case now uses every attempt and the test slept for 6s in each of the three packages. I shrank the retry delay in those tests — they're back under 2s.

go build, go vet and go test ./stackit/internal/services/objectstorage/... all pass.

[github-actions]

This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it.

[FabianHardt]

Not stale from my side — this is still waiting on a review.

@SerseusWasTaken your suggestion is in: the hand-rolled loop is gone from all
three resources in favour of utils.RetryRequest, same behaviour (4 attempts,
2s apart, retry only on 409), net -11 lines.

The one open question from my comment on 3 August, since it is a behaviour
change rather than a refactor: RetryRequest only applies RetryStatusCodes
when the error type-asserts to *oapierror.GenericOpenAPIError, so transport
and network failures are now retried as well, where my original loop bailed out
immediately. For an idempotent enable call I think that is fine, but it is your
call whether those are the semantics you want — happy to narrow it if not.

Build, vet and go test ./stackit/internal/services/objectstorage/... pass.

[SerseusWasTaken]

Hey @FabianHardt, thank you for the contribution.
I've created an internal issue to review and test this PR. We will get to a thorough review soon.

Best Regards,
Jonas from the STACKIT Developer Tools team

[SerseusWasTaken]

Please also update the descriptions of bucket/resource.go and credentialsgroup/resource.go as they state:

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.

This is outdated with this change

[SerseusWasTaken]

Thanks again for your contribution 😄 I've tested it and it works as expected. Once i ran into the case that the enablement took longer than a minute so one of the resources ran out of retries before the other call was finished. But i'll take that as a one-of hiccup and don't think we need to increase the retry number.

If you have any objections with my review above feel free to voice any concerns.

[FabianHardt]

Thanks for testing and for the review @SerseusWasTaken! All three points are in:

• enableProject is deduplicated into objectstorage/utils as EnableProject, and bucket, credential and credentialsgroup call it from there. It now takes the project ID directly instead of the package-local model, so the utils package doesn't depend on any resource types.
• TestEnableProjectRetriesOnConflict and TestEnableProjectDoesNotRetryOtherErrors (plus the plain TestEnableProject) moved to utils/util_test.go, so the retry behaviour is covered once for every caller. The per-package duplicates and their now-unused mocks are gone.
• The outdated depends_on advice is removed from the bucket and credentialsgroup schema descriptions, docs regenerated with tfplugindocs.

On the one-off you hit where the enablement took over a minute: agreed, I'd leave the retry count as is — happy to bump it if it shows up again.

Build, vet and go test ./stackit/internal/services/objectstorage/... pass.

[FabianHardt]

Thanks @cgoetz-inovex, all four suggestions are in with the latest commit:

• The retry tests now run inside synctest.Test bubbles, so they use fake time instead of overriding the delay — no more global mutation, and the utils test package is down to well under a second. TestEnableProjectDoesNotRetryOtherErrors stays unwrapped since it never sleeps.
• With no test touching it anymore, enableProjectRetryDelay is a const now.
• The wrapped error reads enable object storage project: %w — you're right, the function enables rather than creates, and dropping the failed to prefix avoids stacking it through the wrap chain.

Build, vet and go test ./stackit/internal/services/objectstorage/... pass.

[cgoetz-inovex]

@FabianHardt , I've run CI -> linting errors

[FabianHardt]

Fixed — the local config variable in EnableProject shadowed the imported core/config package (gocritic importShadow); it's renamed to retryConfig. I've reproduced the CI lint locally with the repo's custom golangci-lint binary and it's clean now.