fix(managed object storage): page through the bucket listing - #1049
Open
alisharpasand wants to merge 1 commit into
Open
fix(managed object storage): page through the bucket listing#1049alisharpasand wants to merge 1 commit into
alisharpasand wants to merge 1 commit into
Conversation
getBucket passed nil params to ListObjectStorageBucketMetrics, so it only ever
saw the first page. The API returns 10 buckets by default, which made every
bucket past the tenth invisible to the provider.
The failure is quiet and then unrecoverable. Read finds no match and calls
RemoveResource, so the bucket silently leaves state; the next apply tries to
create it and the API returns 400 because the name is still in use; and
terraform import shares this lookup, so the documented recovery path reports
"Cannot import non-existent remote object". Meanwhile the bucket is healthy
and serving traffic.
Observed on a service with 11 buckets, where the eleventh in lexical order
became unmanageable the moment it was pushed off page one:
GET /object-storage-2/{uuid}/buckets -> 10 buckets
GET /object-storage-2/{uuid}/buckets?limit=100 -> 11 buckets
GET /object-storage-2/{uuid}/buckets?limit=10&offset=10 -> the 11th
Now pages with an explicit limit and stops on a short page, rather than
assuming one request returns everything.
Contributor
|
Hello and thank you for the contribution and finding the issue. There might be some additions coming up the to API that could change how we can fetch the bucket details. Due to that, we need to verify the situation before further review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
getBucketpassesnilparams toListObjectStorageBucketMetrics:The API returns 10 buckets when no
limitis given, so on a service with more than 10 buckets everything past the first page is invisible to the provider.The failure is quiet, and then unrecoverable:
Readfinds no match,getBucketreturnsnil, and the resource callsresp.State.RemoveResource(ctx)— the bucket silently leaves state, with no error or warning.applytries to create it and the API returns 400, because the name is still in use.terraform importshares this same lookup, so the documented recovery reportsCannot import non-existent remote object.The bucket is healthy and serving traffic throughout. It simply cannot be managed by Terraform any more, and which bucket is affected depends only on where the page boundary happens to fall.
Reproduction
On a service with 11 buckets, the eleventh in lexical order became unmanageable the moment it was pushed off page one:
The API is behaving correctly and paginating as documented; only the provider's read is unpaginated.
Fix
Page with an explicit
limit, stopping on a short page, instead of assuming a single request returns everything.Paging rather than passing one large
limit: the page size is the API's to decide, and a service may eventually hold more buckets than any single response returns.Notes
go build,go vetandgo test ./internal/service/managedobjectstorage/all pass.StatusNotFoundbranch is deliberately left in place to keep the semantic diff small.[Unreleased].