Skip to content

Data Source: azurerm_storage_account_sas - fix regression silently dropping permissions#32234

Merged
sreallymatt merged 1 commit intohashicorp:mainfrom
ReinierRothuis:fix/storage-account-sas-permissions-builder
Apr 23, 2026
Merged

Data Source: azurerm_storage_account_sas - fix regression silently dropping permissions#32234
sreallymatt merged 1 commit intohashicorp:mainfrom
ReinierRothuis:fix/storage-account-sas-permissions-builder

Conversation

@ReinierRothuis
Copy link
Copy Markdown
Contributor

Community Note

  • Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
  • Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

This PR fixes a regression introduced in #32149 for the data.azurerm_storage_account_sas data source.

Problem

In #32149 the call site at internal/services/storage/storage_account_sas_data_source.go:212 was changed from BuildPermissionsString to BuildContainerPermissionsString. These two helpers target different SAS surfaces and use different key/letter mappings:

schema key (account SAS) expected letter BuildPermissionsString BuildContainerPermissionsString
read r r r
write w w w
delete d d d
list l l l
add a a a
create c c c
update u u ❌ dropped (no such key)
process p p ❌ returns p only if map key is permissions (wrong meaning)
tag t t ❌ dropped (container helper reads tags)
filter f f ❌ dropped (container helper reads find)

The result: account SAS tokens produced by this data source on v4.69.0 silently lose update, process, tag, and filter, which breaks previously-working configurations (e.g. read + tag + filter returned only r).

Fix

Revert the call at storage_account_sas_data_source.go:212 back to BuildPermissionsString while keeping the Optional permissions nil-guard introduced in #32149. BuildPermissionsString already matches the account SAS schema's keys and letters and its unit test (TestAccDataSourceStorageAccountSas_permissionsString) exercises all 10 keys.

PR Checklist

  • I have followed the guidelines in our Contributing Documentation.
  • I have checked to ensure there aren't other open Pull Requests for the same update/change.
  • I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
  • I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
  • I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.

Changes to existing Resource / Data Source

  • I have added an explanation of what my changes do and why I'd like you to include them.
  • I have written new tests for my resource or datasource changes & updated any relevant documentation.
  • I have successfully run tests with my changes locally.

Testing

Ran the existing unit tests that cover the affected helpers:

$ go test -count=1 -run 'TestAccDataSourceStorageAccountSas_permissionsString|TestAccDataSourceStorageAccountSas_resourceTypesString|TestAccDataSourceStorageAccountSas_servicesString' -v ./internal/services/storage/
=== RUN   TestAccDataSourceStorageAccountSas_resourceTypesString
--- PASS: TestAccDataSourceStorageAccountSas_resourceTypesString (0.00s)
=== RUN   TestAccDataSourceStorageAccountSas_servicesString
--- PASS: TestAccDataSourceStorageAccountSas_servicesString (0.00s)
=== RUN   TestAccDataSourceStorageAccountSas_permissionsString
--- PASS: TestAccDataSourceStorageAccountSas_permissionsString (0.00s)
PASS
ok  	github.com/hashicorp/terraform-provider-azurerm/internal/services/storage	1.747s

TestAccDataSourceStorageAccountSas_permissionsString exercises all 10 schema keys (read/write/delete/list/add/create/update/process/tag/filter) and their expected letters, so it already provides regression coverage for this fix — no new test was required.

Change Log

This is a (please select all that apply):

  • Bug Fix
  • New Feature (ie adding a service, resource, or data source)
  • Enhancement
  • Breaking Change

Related Issue(s)

Fixes #32229

AI Assistance Disclosure

  • AI Assisted - This contribution was made by, or with the assistance of, AI/LLMs

Claude Code was used to help investigate the regression, draft the fix, run the local build/unit tests, and draft this PR description. The root cause, fix, and text were reviewed and confirmed by the contributor before submission.

Rollback Plan

If a change needs to be reverted, we will publish an updated version of the provider.

Changes to Security Controls

No changes to security controls. Note that this fix restores the previously-correct behaviour of SAS token permissions — pre-#32149 configurations relying on update/process/tag/filter will once again include those permissions in the generated token, as originally intended.

…as data source

PR hashicorp#32149 replaced BuildPermissionsString with BuildContainerPermissionsString,
which uses different map keys (tags/find/delete_version/etc.) and different
letter mappings (p=permissions not process) than the account SAS schema,
producing incorrect SAS tokens for account-level signatures.
Copy link
Copy Markdown
Collaborator

@sreallymatt sreallymatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ReinierRothuis - LGTM ✅

@sreallymatt sreallymatt merged commit 4769c0e into hashicorp:main Apr 23, 2026
30 checks passed
@github-actions github-actions Bot added this to the v4.70.0 milestone Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

data.azurerm_storage_account_sas silently drops tag and filter permissions since v4.69.0 (regression from #32149)

2 participants