storage_account_blob_container_sas_data_source - add support for more permissions#28452
Closed
Kiliandeca wants to merge 2 commits intohashicorp:mainfrom
Closed
Conversation
mbfrahry
requested changes
May 1, 2025
Member
mbfrahry
left a comment
There was a problem hiding this comment.
Hey @Kiliandeca. This PR looks good but there are some formatting issues to take care of if you don't mind
Contributor
Author
|
Thanks for the review and sorry for the delay, I couldn't work on it before. I made all the changes requested to harmonise the doc formatting and the weird tab in the test file. Let me know if you need anything else done. |
Contributor
Author
|
Hello @mbfrahry, sorry for tagging you but there hasn't been any news on this PR for months, do you need anything more from me? |
14 tasks
Member
|
Hey @Kiliandeca, apologies for the delay in getting this in. I had to update it and open a new PR here #32149 but those changes are now and should go out in the next release. Thanks! |
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.
Community Note
Description
This PR add support for more permissions on the storage container sas token data source as they are defined in: https://learn.microsoft.com/en-us/rest/api/storageservices/create-service-sas#permissions-for-a-directory-container-or-blob
To avoid a breaking change I switched the permissions properties to optional, existing terraform code should not be affected by this change and won't have more permissions than before.
As the
storage_account_blob_container_sas_data_sourceandazurerm_storage_account_sasare very similar I also switched the permissions properties to optional to keep the same behavior between the two.Another point to discuss,
tagsandfindalready exist inazurerm_storage_account_sasbut are named respectivelytagandfilter. The documentation for them don't translate the permission letter to the same word . I followed the names of the container sas token documentation but you might want to reuse the same name as the storage account sas token to keep consistency in the provider, let me know and I'll edit the PR accordingly.vs
PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
storage_account_blob_container_sas_data_source- support for more permissions and make them optionalThis is a (please select all that apply):
Related Issue(s)
Fixes #28421
Note
If this PR changes meaningfully during the course of review please update the title and description as required.