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

[Modules] Update private endpoint module reference to support location input and subnet location reference #3929

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

ahmadabdalla
Copy link
Contributor

@ahmadabdalla ahmadabdalla commented Sep 7, 2023

Description

Currently the nested private endpoint module in each resource has the hardcoded location that uses the reference function to capture the location from the subnet

location: reference(split(privateEndpoint.subnetResourceId, '/subnets/')[0], '2020-06-01', 'Full').location

While this is true where the private endpoint needs to be in the same location as the subnet. This requires any service principal / user deploying the module to have at least 'VNET Read' permissions.

In an environment where RBAC is assigned at the subnet level (i.e. shared VNET infra between different apps), this means that at least 2 RBAC roles are required for the user/service principal to be able to deploy a resource + private endpoint all together.

A workaround is to NOT use the PE feature in each module, but rather deploy the PE separately using the PE module where there is more control on that location

This PR addresses this issue by providing an option on the PE nested module to check if the user has passed the location property and use that, and then fall back to the existing implementation. This is backwards compatible with the existing implementation and supports the scenario where RBAC is done at the subnet level and not the VNET level.

location: contains(privateEndpoint, 'location') ? privateEndpoint.location : reference(split(privateEndpoint.subnetResourceId, '/subnets/')[0], '2020-06-01', 'Full').location

Pipeline references

Pipeline
Automation - AutomationAccounts
Batch - BatchAccounts
Cache - Redis
ContainerRegistry - Registries
DocumentDB - DatabaseAccounts
EventHub - Namespaces
KeyVault - Vaults
Search - SearchServices
ServiceBus - Namespaces

Type of Change

  • Bugfix (non-breaking change which fixes an issue)

Checklist

  • I'm sure there are no other open Pull Requests for the same update/change
  • My corresponding pipelines / checks run clean and green without any errors or warnings
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (readme)
  • I did format my code

@ahmadabdalla ahmadabdalla marked this pull request as ready for review September 7, 2023 06:39
@ahmadabdalla ahmadabdalla requested a review from a team as a code owner September 7, 2023 06:39
@ahmadabdalla ahmadabdalla merged commit 3b791db into main Sep 7, 2023
103 of 107 checks passed
@ahmadabdalla ahmadabdalla deleted the users/ahmad/PElocationBug branch September 7, 2023 06:44
@AlexanderSehr AlexanderSehr added enhancement New feature or request [cat] modules category: modules labels Sep 7, 2023
@AlexanderSehr AlexanderSehr added this to the Release v0.11.0 milestone Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[cat] modules category: modules enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants