search

Azure / bicep-registry-modules

Bicep registry modules
MIT License
472 stars 327 forks source link

[AVM Module Issue]: Sql Server Module should add the creation of `Microsoft.KeyVault/vaults/secrets` #2608

Open NanaXiong00 opened 3 months ago

NanaXiong00 commented 3 months ago

Check for previous/existing GitHub issues

Issue Type?

Feature Request

Module Name

avm/res/sql/server

(Optional) Module Version

No response

Description

Please add the creation of Microsoft.KeyVault/vaults/secrets. In azd, we allow the user to pass a keyvault reference when creating a sql server. If they do provide one, then we set a keyvault secret that contains the sqlAdminPassword, appUserPassword, connectionString. Refer to this issue: https://github.com/Azure/bicep-registry-modules/issues/632

For example (azd):

CC: @jongio

(Optional) Correlation Id

No response

avm-team-linter[bot] commented 3 months ago

@NanaXiong00, thanks for submitting this issue for the avm/res/sql/server module!

[!IMPORTANT] Please note, that this module is currently orphaned. The @Azure/avm-core-team-technical-bicep, will attempt to find an owner for it. In the meantime, the core team may assist with this issue. Thank you for your patience!

AlexanderSehr commented 3 months ago

This is connected to

jtracey93 commented 2 months ago

Hey @NanaXiong00 & @jongio, this module avm/res/sql/server is currently orphaned 😢

Do yourselves or anyone you know want to take ownership of this module and make the the required changes to the module as per: https://github.com/Azure/bicep-registry-modules/issues/1934#issuecomment-2223349183 ?

RR

v-xuto commented 2 months ago

@jtracey93 We will try to fix this issue. @jongio for notification.

v-xuto commented 2 months ago

@jtracey93 PR is ready, please review.

AlexanderSehr commented 2 months ago

as commented in the PR, this depends on https://github.com/Azure/bicep-registry-modules/issues/1934

zedy-wj commented 2 months ago

According to the comments here: https://github.com/Azure/bicep-registry-modules/pull/2859#discussion_r1693915323, it seems more appropriate to set KeyVault Secrets outside the module. Should this issue continue?

AlexanderSehr commented 2 months ago

According to the comments here: #2859 (comment), it seems more appropriate to set KeyVault Secrets outside the module. Should this issue continue?

Hey @zedy-wj, please note: The comment only relates to the addition of a secret value to the interface. The idea of the issue should be that the module can store all the service's credentials (e.g., access keys, connection strings, etc.) in a given Key Vault, if the user opts in. This is something that we already agreed on implementing, and I just need to maintainers to greenlight the suggested spec so that we can go ahead and implement the interface.

The commented line goes a step beyond that and not only does the above, but also would have the user be able to add custom secrets to that mix which I'd say should be done outside the module. In this case specifically, a password is added which could/should just as well be set outside the module's scope. If we don't, I'm of the (subjective) opinion, that we're bloating the key vault secrets feature beyond the module's scope as one could essentially start setting any secrets via the module, even if they're completely unrelated.

I hope that makes sense. Happy to discuss :)