Skip to content
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

AccTest: storage account (network rules) modify the test for private_link property #23383

Merged
merged 4 commits into from
Oct 31, 2023

Conversation

magodo
Copy link
Collaborator

@magodo magodo commented Sep 26, 2023

Since Sep.11 (working well before), the API request for creating (PUT) or updating (PATCH) a storage account's networkAcls.resourceAccessRules will fail with the following error message:

{"error":{"code":"InvalidValuesForRequestParameters","message":"Values for request parameters are invalid: networkAcls.resourceAccessRules[*].resourceId. For more information, see - https://aka.ms/storagenetworkruleset"}}

The reason is Storage has enabled a validation to block invalid resource access rules to be added to storage account.

The allowed resource access rules must be for resource types in this list, while "Microsoft.Network/privateEndpoints" is not in the list.

It's also worth mentioning that there is a comment in the previous test:

	// Not all regions support setting the private endpoint resource as the endpoint resource in network_rules.private_link_access in the storage account
	data.Locations.Primary = "westeurope"

This seems to indicate the behavior is not supportive at all by the service.

This PR changes the resource being set to private_link from a private endpoint resource, to a resource that is "trusted".

Test

terraform-provider-azurerm on  storage_acctest_private_link via 🐹 v1.21.1 took 12m22s
💤  TF_ACC=1 go test -v -timeout=20h ./internal/services/storage -run='TestAccStorageAccount_privateLinkAccess|TestAccStorageAccountNetworkRules_privateLinkAccess|TestAccStorageAccountNetworkRules_SynapseAccess'
=== RUN   TestAccStorageAccountNetworkRules_privateLinkAccess
=== PAUSE TestAccStorageAccountNetworkRules_privateLinkAccess
=== RUN   TestAccStorageAccountNetworkRules_SynapseAccess
=== PAUSE TestAccStorageAccountNetworkRules_SynapseAccess
=== RUN   TestAccStorageAccount_privateLinkAccess
=== PAUSE TestAccStorageAccount_privateLinkAccess
=== CONT  TestAccStorageAccountNetworkRules_privateLinkAccess
=== CONT  TestAccStorageAccount_privateLinkAccess
=== CONT  TestAccStorageAccountNetworkRules_SynapseAccess
--- PASS: TestAccStorageAccount_privateLinkAccess (271.18s)
--- PASS: TestAccStorageAccountNetworkRules_privateLinkAccess (278.01s)
--- PASS: TestAccStorageAccountNetworkRules_SynapseAccess (579.96s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/storage       580.008s

@magodo magodo marked this pull request as ready for review September 26, 2023 03:40
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

Left a question inline but this otherwise LGTM 👍

name = "acctestsearchservice%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
sku = "basic"
Copy link
Contributor

Choose a reason for hiding this comment

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

sanity checking: does the standard sku of search support private link? else this'd break in the future

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tombuildsstuff Yes, I've tried the standard sku, which works fine. I believe the sku doesn't matter as otherwise it should be mentioned in https://learn.microsoft.com/en-us/azure/storage/common/storage-network-security?tabs=azure-portal#trusted-access-based-on-a-managed-identity?

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks for this @magodo. LGTM 👍

@stephybun stephybun merged commit 9f832ba into hashicorp:main Oct 31, 2023
23 checks passed
@github-actions github-actions bot added this to the v3.79.0 milestone Oct 31, 2023
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants