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]: Duplicate Private Link service connection names in all modules #1042

Closed
1 task done
tyconsulting opened this issue Feb 19, 2024 · 5 comments · Fixed by #1089, #1090, #1091, #1092 or #1087
Closed
1 task done
Assignees
Labels
Needs: Triage 🔍 Maintainers need to triage still Status: Response Overdue 🚩 When an issue/PR has not been responded to for X amount of days Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: Bug 🐛 Something isn't working

Comments

@tyconsulting
Copy link
Contributor

tyconsulting commented Feb 19, 2024

Check for previous/existing GitHub issues

  • I have checked for previous/existing GitHub issues

Issue Type?

Bug

Module Name

Other, as defined below...

(Optional) Module Name if not listed above

All modules that create Private Endpoints

(Optional) Module Version

No response

Description

When multiple private endpoints are defined, the PL service connection name are the same:
image

To fix this bug, this is what I have done:

  1. in the module calling the PE module, extend the privateEndpointType by adding an additional property:
  @description('Optional. The name of the Private Link Service Connection.')
  privateLinkServiceConnectionName: string?
  1. When calling the PE module, update the name property inside privateLinkServiceConnections to name: privateEndpoint.?privateLinkServiceConnectionName ?? '${name}-${index}' i.e.

image

AND

image

This will allow me to give it a more meaningful name if required, and if I don't specify a name, a numeric character will append to the end of the name ('${name}-${index}')

This is the end result:

image

(Optional) Correlation Id

No response

@tyconsulting tyconsulting added Needs: Triage 🔍 Maintainers need to triage still Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue labels Feb 19, 2024
@github-project-automation github-project-automation bot moved this to Todo in Bicep Feb 19, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Type: Bug 🐛 Something isn't working label Feb 19, 2024
@tyconsulting
Copy link
Contributor Author

Hi @segraef @AlexanderSehr this is also related to the multiple PE discussion we had few days ago (#962).

@segraef
Copy link
Contributor

segraef commented Feb 19, 2024

Thanks @tyconsulting, having a look at it.

@segraef segraef self-assigned this Feb 19, 2024
@AlexanderSehr
Copy link
Contributor

AlexanderSehr commented Feb 19, 2024

We're starting to pile them up. Thanks for all the great feedback @tyconsulting. @segraef, I guess right now everything is just waiting on the schema approval, right? I'd suggest to add this one in there as well, if you'd agree.

As an alternative we could however also re-use the private endpoint name logic, right? There's only ever one private link connection per private endpoint, so if we'd also be using the following it should also work (key vault example):

name: privateEndpoint.?name ?? '${last(split(<singularMainResourceType>.id, '/'))}-${privateEndpoint.?service ?? '<defaultServiceName>'}-${index}'

Though instead of saying privateEndpoint.?name in the front, we could also use privateEndpoint.?privateLinkServiceConnectionName as per your suggestion.
That would be:

name: privateEndpoint.?privateLinkServiceConnectionName ?? '${last(split(<singularMainResourceType>.id, '/'))}-${privateEndpoint.?service ?? '<defaultServiceName>'}-${index}'

and the same for the manual connection.

AlexanderSehr pushed a commit that referenced this issue Feb 25, 2024
## Description
Currently testing all multi-PE services as per
https://learn.microsoft.com/en-us/azure/private-link/private-endpoint-overview#private-link-resource

Private-link resource name | Resource type | Sub-resources
-- | -- | --
Azure Automation | Microsoft.Automation/automationAccounts | Webhook,
DSCAndHybridWorker
Azure Backup | Microsoft.RecoveryServices/vaults | AzureBackup,
AzureSiteRecovery
Azure Batch | Microsoft.Batch/batchAccounts | batchAccount,
nodeManagement
Azure Cosmos DB | Microsoft.AzureCosmosDB/databaseAccounts | SQL,
MongoDB, Cassandra, Gremlin, Table
Azure Databricks | Microsoft.Databricks/workspaces | databricks_ui_api,
browser_authentication
Azure Media Services | Microsoft.Media/mediaservices | keydelivery,
liveevent, streamingendpoint
Azure Storage | Microsoft.Storage/storageAccounts | Blob (blob,
blob_secondary)Table (table, table_secondary)Queue (queue,
queue_secondary)File (file, file_secondary)Web (web, web_secondary)Dfs
(dfs, dfs_secondary)
Azure Synapse Analytics | Microsoft.Synapse/workspaces | Sql,
SqlOnDemand, Dev

Closes
- Azure/Azure-Verified-Modules#620
- #962
- #946
- #1042

AVM Issues
- Azure/Azure-Verified-Modules#621

## Pipeline Reference

| Pipeline |
| -------- |
|
[![avm.res.automation.automation-account](https://github.com/segraef/bicep-registry-modules/actions/workflows/avm.res.automation.automation-account.yml/badge.svg?branch=fix%2Fpe-schema)](https://github.com/segraef/bicep-registry-modules/actions/workflows/avm.res.automation.automation-account.yml)
|
|
[![avm.res.batch.batch-account](https://github.com/segraef/bicep-registry-modules/actions/workflows/avm.res.batch.batch-account.yml/badge.svg?branch=fix%2Fpe-schema)](https://github.com/segraef/bicep-registry-modules/actions/workflows/avm.res.batch.batch-account.yml)|
|
[![avm.res.databricks.workspace](https://github.com/segraef/bicep-registry-modules/actions/workflows/avm.res.databricks.workspace.yml/badge.svg?branch=fix%2Fpe-schema)](https://github.com/segraef/bicep-registry-modules/actions/workflows/avm.res.databricks.workspace.yml)|
|
[![avm.res.document-db.database-account](https://github.com/segraef/bicep-registry-modules/actions/workflows/avm.res.document-db.database-account.yml/badge.svg?branch=fix%2Fpe-schema)](https://github.com/segraef/bicep-registry-modules/actions/workflows/avm.res.document-db.database-account.yml)|
|
[![avm.res.storage.storage-account](https://github.com/segraef/bicep-registry-modules/actions/workflows/avm.res.storage.storage-account.yml/badge.svg?branch=fix%2Fpe-schema)](https://github.com/segraef/bicep-registry-modules/actions/workflows/avm.res.storage.storage-account.yml)|
|
[![avm.res.synapse.workspace](https://github.com/segraef/bicep-registry-modules/actions/workflows/avm.res.synapse.workspace.yml/badge.svg?branch=fix%2Fpe-schema)](https://github.com/segraef/bicep-registry-modules/actions/workflows/avm.res.synapse.workspace.yml)
|



## Type of Change

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

- [ ] Update to CI Environment or utlities (Non-module effecting
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.
- [x] 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`.
  - [x] 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

<!-- Please keep up to day with the contribution guide at
https://aka.ms/avm/contribute/bicep -->

---------

Co-authored-by: Kris Baranek <[email protected]>
@AlexanderSehr AlexanderSehr linked a pull request Feb 25, 2024 that will close this issue
8 tasks

Tagging the AVM Core Team (@Azure/avm-core-team) due to a module owner or contributor having not responded to this issue within 3 business days. The AVM Core Team will attempt to contact the module owners/contributors directly.

@segraef
Copy link
Contributor

segraef commented Mar 3, 2024

Implementation in progress, see linked PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment