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

[AVM Module Issue]: Unable to create private endpoints for Private Link Scope when VNet is in different subscription #3835

Closed
1 task done
JamesDawson opened this issue Nov 22, 2024 · 11 comments · Fixed by #4132
Assignees
Labels
Class: Resource Module 📦 This is a resource module Needs: Core Team 🧞 This item needs the AVM Core Team to review it Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: Bug 🐛 Something isn't working

Comments

@JamesDawson
Copy link

Check for previous/existing GitHub issues

  • I have checked for previous/existing GitHub issues

Issue Type?

Bug

Module Name

avm/res/insights/private-link-scope

(Optional) Module Version

0.5.2

Description

A deployment failure happens when the Private Link Scope resource is in a different subscription to the Virtual Network.

Generally the error is ResourceGroupNotFound on the private endpoint resource group (since it exists in a different subscription). If a resource group happens to exist with the same name in the PLS subscription, then the deployment produces an InvalidResourceReference.

Private Endpoints must be provisioned in the same subscription as the Virtual Network they are connected to. Whilst this module allows you to customise the resource group for the private endpoint, it currently assumes the same subscription as the Private Link Scope resource.

To avoid needing another parameter, perhaps the scope on this line should infer the subscription from the subnet resource ID?

scope: resourceGroup(privateEndpoint.?resourceGroupName ?? '')

(Optional) Correlation Id

No response

@JamesDawson JamesDawson added Needs: Triage 🔍 Maintainers need to triage still Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue labels Nov 22, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Type: Bug 🐛 Something isn't working label Nov 22, 2024
@avm-team-linter avm-team-linter bot added the Class: Resource Module 📦 This is a resource module label Nov 22, 2024
Copy link

@JamesDawson, thanks for submitting this issue for the avm/res/insights/private-link-scope module!

Important

A member of the @Azure/avm-res-insights-privatelinkscope-module-owners-bicep or @Azure/avm-res-insights-privatelinkscope-module-contributors-bicep team will review it soon!

@github-project-automation github-project-automation bot moved this to Needs: Triage in AVM - Module Issues Nov 22, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Status: Response Overdue 🚩 When an issue/PR has not been responded to for X amount of days label Nov 28, 2024
@JamesDawson
Copy link
Author

JamesDawson commented Dec 2, 2024

I was looking at using the avm/res/key-vault/vault module today and noticed that this module has the same issue:

scope: resourceGroup(privateEndpoint.?resourceGroupName ?? '')

Based on a skim read of the following search results, this seems to be an issue across the board:
https://github.com/search?q=repo%3AAzure%2Fbicep-registry-modules+br%2Fpublic%3Aavm%2Fres%2Fnetwork%2Fprivate-endpoint+language%3ABicep&type=code&l=Bicep

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Immediate Attention ‼️ Immediate attention of module owner / AVM team is needed label Dec 6, 2024
@AlexanderSehr AlexanderSehr added the Needs: Core Team 🧞 This item needs the AVM Core Team to review it label Dec 20, 2024
@AlexanderSehr
Copy link
Contributor

AlexanderSehr commented Dec 20, 2024

Hey folks,
thanks for raising this. I just linked the CoreTeam to it as this is a fundamental issue of the PE Spec and not one module in particular (as was correctly raised).
Without looking to deep into it, if we consider that the PE must be created in the same subscription as the subnet, then the interface should probaly not ask for the ResourceGroupName at all and by default use the one that is defined in the subnetResourceId property.

The implementation would then change to

module keyVault_privateEndpoints 'br/public:avm/res/network/private-endpoint:0.9.0' = [
  for (privateEndpoint, index) in (privateEndpoints ?? []): {
    name: '${uniqueString(deployment().name, location)}-keyVault-PrivateEndpoint-${index}'
    scope: scope: resourceGroup(
    split((privateEndpoint.?subnetResourceId?? '//'), '/')[2],
    split((privateEndpoint.?subnetResourceId?? '////'), '/')[4]
    )
    params: {
      name: privateEndpoint.?name ?? 'pep-${last(split(keyVault.id, '/'))}-${privateEndpoint.?service ?? 'vault'}-${index}'
      (...)
      subnetResourceId: privateEndpoint.subnetResourceId
      location: privateEndpoint.?location ?? reference(
        split(privateEndpoint.subnetResourceId, '/subnets/')[0],
        '2020-06-01',
        'Full'
      ).location
    }
  }
]

Thoughts?

cc: @eriqua, @ChrisSidebotham, @jtracey93 & @ReneHezser for awareness

@ReneHezser
Copy link
Contributor

I think that makes sense (using the resource group of the subnet). Would there be a use-case where the location is a different one?

@JamesDawson
Copy link
Author

Thanks @AlexanderSehr. That seems fine, although we would still need optional control over the resource group used to host the private endpoints.

In a scenario where the network resources are managed by a central team, an application team may not have permissions to provision resources alongside the VNet. Therefore being able to specify an alternative resource group name, within the same subscription, would be necessary.

@AlexanderSehr
Copy link
Contributor

Hey @JamesDawson,
thanks for your input. So I take it you'd prefer a solution like

module keyVault_privateEndpoints 'br/public:avm/res/network/private-endpoint:0.9.0' = [
  for (privateEndpoint, index) in (privateEndpoints ?? []): {
    name: '${uniqueString(deployment().name, location)}-keyVault-PrivateEndpoint-${index}'
    scope: !empty(privateEndpoint.?resourceGroupResourceId)
      ? resourceGroup(
          split((privateEndpoint.?resourceGroupResourceId ?? '//'), '/')[2],
          split((privateEndpoint.?resourceGroupResourceId ?? '////'), '/')[4]
        )
      : resourceGroup(
          split((privateEndpoint.?subnetResourceId ?? '//'), '/')[2],
          split((privateEndpoint.?subnetResourceId ?? '////'), '/')[4]
        )
    params: {
      name: privateEndpoint.?name ?? 'pep-${last(split(keyVault.id, '/'))}-${privateEndpoint.?service ?? 'vault'}-${index}'
      (...)
      subnetResourceId: privateEndpoint.subnetResourceId
      location: privateEndpoint.?location ?? reference(
        split(privateEndpoint.subnetResourceId, '/subnets/')[0],
        '2020-06-01',
        'Full'
      ).location
    }
  }
]

In other words: if a resource group resource Id is specified, it will deploy the private endpoint into this resource group (in whatever subscription that resource group is in). If not specified, it will default to deploying the private endpoint into the resource group & subscription of the used virtual network subnet.

@JamesDawson
Copy link
Author

Yep that looks good @AlexanderSehr.

@AlexanderSehr AlexanderSehr removed Needs: Triage 🔍 Maintainers need to triage still Needs: Immediate Attention ‼️ Immediate attention of module owner / AVM team is needed Status: Response Overdue 🚩 When an issue/PR has not been responded to for X amount of days labels Jan 3, 2025
@AlexanderSehr
Copy link
Contributor

Yep that looks good @AlexanderSehr.

Great, thanks @JamesDawson.
We just discussed it in the core team and @ReneHezser is working on the interface update.

@AlexanderSehr
Copy link
Contributor

AlexanderSehr commented Feb 14, 2025

Hey @JamesDawson,
while we're rolling out the above interface, and ask by @ahmadabdalla came up whether we should not rather use the main resources (e.g., Key Vaults) RG as the default location for the PE, and via the already suggested resourceGroupResourceId allow the user to specify a different RG (in a different subscription - as per your requirement).

The subtile change would be from

    scope: resourceGroup(
      split(privateEndpoint.?resourceGroupResourceId ?? privateEndpoint.?subnetResourceId, '/')[2],
      split(privateEndpoint.?resourceGroupResourceId ?? privateEndpoint.?subnetResourceId, '/')[4]
    )

to

    scope: resourceGroup(
      split(privateEndpoint.?resourceGroupResourceId ?? resourceGroup().id, '/')[2],
      split(privateEndpoint.?resourceGroupResourceId ?? resourceGroup().id, '/')[4]
    )

While we'll be discussing this ASAP internally, and while both implementations will statify the requirements and are merely about the 'most common use case' - I'd be curious to hear your thoughts as well.

I'll quote from @ahmadabdalla on this matter

In most scenarios, PEs are deployed alongside their main resource in their own RG vs. the VNET RG. Customer app teams may have subnet join permissions on a centralised VNET in a Landing zone, but may not have permissions to deploy into it. Also considering billing and resource lifecycle perspective.

Ref: #4449

@github-project-automation github-project-automation bot moved this from Needs: Triage to Done in AVM - Module Issues Feb 14, 2025
@ahmadabdalla
Copy link
Contributor

ahmadabdalla commented Feb 14, 2025

Cc @tyconsulting for visibility as he's originally raised this 💪

@JamesDawson
Copy link
Author

Thanks @AlexanderSehr, using the resource's RG seems like a sensible default.

AlexanderSehr added a commit that referenced this issue Feb 18, 2025
…by default (#4449)

## Description

Background:
#3835 (comment)
Linked to: Azure/Azure-Verified-Modules#1857

Changes the deployment so that the main resource's (e.g., Key Vaults) RG
is used as the default location for the PE. The already implemented
`resourceGroupResourceId` will continue to allow the user to specify a
different RG (in a different subscription, if needed).

The primary change is from
```bicep
    scope: resourceGroup(
      split(privateEndpoint.?resourceGroupResourceId ?? privateEndpoint.?subnetResourceId, '/')[2],
      split(privateEndpoint.?resourceGroupResourceId ?? privateEndpoint.?subnetResourceId, '/')[4]
    )
```
to
```bicep
    scope: resourceGroup(
      split(privateEndpoint.?resourceGroupResourceId ?? resourceGroup().id, '/')[2],
      split(privateEndpoint.?resourceGroupResourceId ?? resourceGroup().id, '/')[4]
    )
```

I'll quote from @ahmadabdalla on this matter
> In most scenarios, PEs are deployed alongside their main resource in
their own RG vs. the VNET RG. Customer app teams may have subnet join
permissions on a centralised VNET in a Landing zone, but may not have
permissions to deploy into it. Also considering billing and resource
lifecycle perspective.

cc: @JamesDawson

Ref: #4449

## Pipeline Reference

<!-- Insert your Pipeline Status Badge below -->

| Pipeline |
| -------- |
|          |

## Type of Change

<!-- Use the checkboxes [x] on the options that are relevant. -->

- [ ] Update to CI Environment or utilities (Non-module affecting
changes)
- [ ] Azure Verified Module updates:
- [ ] Bugfix containing backwards-compatible bug fixes, and I have NOT
bumped the MAJOR or MINOR version in `version.json`:
- [ ] Someone has opened a bug report issue, and I have included "Closes
#{bug_report_issue_number}" in the PR description.
- [ ] The bug was found by the module author, and no one has opened an
issue to report it yet.
- [ ] Feature update backwards compatible feature updates, and I have
bumped the MINOR version in `version.json`.
- [x] Breaking changes and I have bumped the MAJOR version in
`version.json`.
  - [ ] Update to documentation
anderseide pushed a commit to anderseide/avm-bicep-registry-modules that referenced this issue Feb 19, 2025
…he subnet resource group (Azure#4132)

## Description

- Adds UDTs
- deploys private endpoint in the subnet resource group, if not
explicitly specified

Closes Azure#3835 
Azure#4404 

## Pipeline Reference

<!-- Insert your Pipeline Status Badge below -->

| Pipeline |
| -------- |
|
[![avm.res.insights.private-link-scope](https://github.com/ReneHezser/bicep-registry-modules/actions/workflows/avm.res.insights.private-link-scope.yml/badge.svg?branch=issue-3835)](https://github.com/ReneHezser/bicep-registry-modules/actions/workflows/avm.res.insights.private-link-scope.yml)
|

## Type of Change

- [ ] Update to CI Environment or utilities (Non-module affecting
changes)
- [x] Azure Verified Module updates:
- [ ] Bugfix containing backwards-compatible bug fixes, and I have NOT
bumped the MAJOR or MINOR version in `version.json`:
- [x] Someone has opened a bug report issue, and I have included "Closes
#{bug_report_issue_number}" in the PR description.
- [ ] The bug was found by the module author, and no one has opened an
issue to report it yet.
- [ ] Feature update backwards compatible feature updates, and I have
bumped the MINOR version in `version.json`.
- [ ] Breaking changes and I have bumped the MAJOR version in
`version.json`.
  - [ ] Update to documentation

## Checklist

- [x] I'm sure there are no other open Pull Requests for the same
update/change
- [x] I have run `Set-AVMModule` locally to generate the supporting
module files.
- [x] My corresponding pipelines / checks run clean and green without
any errors or warnings
anderseide pushed a commit to anderseide/avm-bicep-registry-modules that referenced this issue Feb 19, 2025
…by default (Azure#4449)

## Description

Background:
Azure#3835 (comment)
Linked to: Azure/Azure-Verified-Modules#1857

Changes the deployment so that the main resource's (e.g., Key Vaults) RG
is used as the default location for the PE. The already implemented
`resourceGroupResourceId` will continue to allow the user to specify a
different RG (in a different subscription, if needed).

The primary change is from
```bicep
    scope: resourceGroup(
      split(privateEndpoint.?resourceGroupResourceId ?? privateEndpoint.?subnetResourceId, '/')[2],
      split(privateEndpoint.?resourceGroupResourceId ?? privateEndpoint.?subnetResourceId, '/')[4]
    )
```
to
```bicep
    scope: resourceGroup(
      split(privateEndpoint.?resourceGroupResourceId ?? resourceGroup().id, '/')[2],
      split(privateEndpoint.?resourceGroupResourceId ?? resourceGroup().id, '/')[4]
    )
```

I'll quote from @ahmadabdalla on this matter
> In most scenarios, PEs are deployed alongside their main resource in
their own RG vs. the VNET RG. Customer app teams may have subnet join
permissions on a centralised VNET in a Landing zone, but may not have
permissions to deploy into it. Also considering billing and resource
lifecycle perspective.

cc: @JamesDawson

Ref: Azure#4449

## Pipeline Reference

<!-- Insert your Pipeline Status Badge below -->

| Pipeline |
| -------- |
|          |

## Type of Change

<!-- Use the checkboxes [x] on the options that are relevant. -->

- [ ] Update to CI Environment or utilities (Non-module affecting
changes)
- [ ] Azure Verified Module updates:
- [ ] Bugfix containing backwards-compatible bug fixes, and I have NOT
bumped the MAJOR or MINOR version in `version.json`:
- [ ] Someone has opened a bug report issue, and I have included "Closes
#{bug_report_issue_number}" in the PR description.
- [ ] The bug was found by the module author, and no one has opened an
issue to report it yet.
- [ ] Feature update backwards compatible feature updates, and I have
bumped the MINOR version in `version.json`.
- [x] Breaking changes and I have bumped the MAJOR version in
`version.json`.
  - [ ] Update to documentation
anderseide pushed a commit to anderseide/avm-bicep-registry-modules that referenced this issue Feb 23, 2025
…he subnet resource group (Azure#4132)

## Description

- Adds UDTs
- deploys private endpoint in the subnet resource group, if not
explicitly specified

Closes Azure#3835 
Azure#4404 

## Pipeline Reference

<!-- Insert your Pipeline Status Badge below -->

| Pipeline |
| -------- |
|
[![avm.res.insights.private-link-scope](https://github.com/ReneHezser/bicep-registry-modules/actions/workflows/avm.res.insights.private-link-scope.yml/badge.svg?branch=issue-3835)](https://github.com/ReneHezser/bicep-registry-modules/actions/workflows/avm.res.insights.private-link-scope.yml)
|

## Type of Change

- [ ] Update to CI Environment or utilities (Non-module affecting
changes)
- [x] Azure Verified Module updates:
- [ ] Bugfix containing backwards-compatible bug fixes, and I have NOT
bumped the MAJOR or MINOR version in `version.json`:
- [x] Someone has opened a bug report issue, and I have included "Closes
#{bug_report_issue_number}" in the PR description.
- [ ] The bug was found by the module author, and no one has opened an
issue to report it yet.
- [ ] Feature update backwards compatible feature updates, and I have
bumped the MINOR version in `version.json`.
- [ ] Breaking changes and I have bumped the MAJOR version in
`version.json`.
  - [ ] Update to documentation

## Checklist

- [x] I'm sure there are no other open Pull Requests for the same
update/change
- [x] I have run `Set-AVMModule` locally to generate the supporting
module files.
- [x] My corresponding pipelines / checks run clean and green without
any errors or warnings
anderseide pushed a commit to anderseide/avm-bicep-registry-modules that referenced this issue Feb 23, 2025
…by default (Azure#4449)

## Description

Background:
Azure#3835 (comment)
Linked to: Azure/Azure-Verified-Modules#1857

Changes the deployment so that the main resource's (e.g., Key Vaults) RG
is used as the default location for the PE. The already implemented
`resourceGroupResourceId` will continue to allow the user to specify a
different RG (in a different subscription, if needed).

The primary change is from
```bicep
    scope: resourceGroup(
      split(privateEndpoint.?resourceGroupResourceId ?? privateEndpoint.?subnetResourceId, '/')[2],
      split(privateEndpoint.?resourceGroupResourceId ?? privateEndpoint.?subnetResourceId, '/')[4]
    )
```
to
```bicep
    scope: resourceGroup(
      split(privateEndpoint.?resourceGroupResourceId ?? resourceGroup().id, '/')[2],
      split(privateEndpoint.?resourceGroupResourceId ?? resourceGroup().id, '/')[4]
    )
```

I'll quote from @ahmadabdalla on this matter
> In most scenarios, PEs are deployed alongside their main resource in
their own RG vs. the VNET RG. Customer app teams may have subnet join
permissions on a centralised VNET in a Landing zone, but may not have
permissions to deploy into it. Also considering billing and resource
lifecycle perspective.

cc: @JamesDawson

Ref: Azure#4449

## Pipeline Reference

<!-- Insert your Pipeline Status Badge below -->

| Pipeline |
| -------- |
|          |

## Type of Change

<!-- Use the checkboxes [x] on the options that are relevant. -->

- [ ] Update to CI Environment or utilities (Non-module affecting
changes)
- [ ] Azure Verified Module updates:
- [ ] Bugfix containing backwards-compatible bug fixes, and I have NOT
bumped the MAJOR or MINOR version in `version.json`:
- [ ] Someone has opened a bug report issue, and I have included "Closes
#{bug_report_issue_number}" in the PR description.
- [ ] The bug was found by the module author, and no one has opened an
issue to report it yet.
- [ ] Feature update backwards compatible feature updates, and I have
bumped the MINOR version in `version.json`.
- [x] Breaking changes and I have bumped the MAJOR version in
`version.json`.
  - [ ] Update to documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Class: Resource Module 📦 This is a resource module Needs: Core Team 🧞 This item needs the AVM Core Team to review it Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: Bug 🐛 Something isn't working
Projects
4 participants