-
Notifications
You must be signed in to change notification settings - Fork 9.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
internal/backend/remote-state/azure: respect NextMarker #33396
base: main
Are you sure you want to change the base?
internal/backend/remote-state/azure: respect NextMarker #33396
Conversation
I'll be doing ad-hoc testing on this tomorrow at work to ensure it works as expected but I could use some advice or guidance on how to write up some automated tests covering this edge case. This is only my second attempt at anything in go and I'm having a bit of trouble finding my way around the test suite. |
Thanks, I'll notify the Azure provider team. |
@crw thanks for your continued help. :) For what it is worth I've deployed this locally and it does seem to have resolved the issue I'm having at work. |
We're nearing the 1 year birthday of issue #31719 , is there anything I can do to help get this a little more traction? Every time there's a new Terraform version released I need to re-integrate and re-test a personal build including my fix to ensure we don't run into problems. If the issue is the quality of my code I'm more than happy to incorporate any suggestions or requests from the team. I'd also be happy for my branch to be scrapped in favor of code from an engineer more familiar with Go. The only change that needs to be made is to ensure the state backend paginates through the results of the ListBlobs() fetch API when the API says pagination is necessary (via the NextMarker parameter). The current upstream code doesn't paginate even when it is required. |
Hi @coderjoe, I can see this PR is on the triage backlog for the Azure Provider team. Although the backends are maintained by the provider teams, they are not as high a priority as provider work. In my experience, the provider team will irregularly set aside time to review a batch of backend PRs, as their schedule allows. That said, I will raise it again. |
This updates the backend state to check for and respect the NextMarker continuation if present rather than just omitting the results behind the continuation. Relates to: hashicorp#31719
ab85882
to
104a9be
Compare
We're on to 2 years with no review. I've gone ahead and rebased onto the latest master so someone can take a look. If I don't hear back by the next time I do my next open contribution review I'll close them myself. Heres hoping we can get it fixed before then though - I grow tired of manually patching terraform with every release just to keep using it. |
Hi @coderjoe, I understand the frustration. The Azure team here hasn't looked at any of the Azure backend issues or PRs in some time, but I know that there is a desire to do so. Unfortunately the desire to look at these hasn't yet translated into action. I am hopeful this will get looked at before your next window closes. |
@crw We are a significant Microsoft customer. Is there anything we can do to help prioritize this with our assigned Microsoft Cloud Solution Architect or Customer Success Manager? This issue has caused unintended resource deletions on our end, so this PR would be highly beneficial. |
I am going to defer to the local expert, however my understanding is that these issues can get prioritized with Microsoft, as they have a team that works on Azure Provider issues (ancillary to the HashiCorp team that owns the AzureRM Provider). |
@crw Do you have an update for us? |
Apologies for my previous comment, I mistakenly thought I was in an issue thread not a PR thread. In this case, we are waiting for the Azure Provider team at HashiCorp to prioritize this PR review. Thanks for your continued interest in this PR. |
When enumerating blob resources in Azure the service may return a result containing all of the results, or it might return a response containing only a subset of the results and a marker intended to be used to fetch the next subset in a subesquent call.
The azure backend state does not currently check for or enumerate across returned NextMarker values when using ListBlobs while fetching the list of workspaces. This results in partial results being returned when the API returns results requiring iteration via
marker
andNextMarker
This fixes the issue by updating the backend state to check for and respect the NextMarker continuation ensuring that the full results set is always processed regardless of whether or not the result set is limited via
maxresults
or is limited by the 5000 item per result set limit.For more information see:
https://learn.microsoft.com/en-us/rest/api/storageservices/enumerating-blob-resources#Subheading2
https://learn.microsoft.com/en-us/rest/api/storageservices/list-blobs?tabs=azure-ad#uri-parameters
Fixes #31719
Target Release
1.5.x
Draft CHANGELOG entry
BUG FIXES