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]: Sql Server Module should add the creation of Microsoft.KeyVault/vaults/secrets #2608

Closed
1 task done
NanaXiong00 opened this issue Jul 3, 2024 · 8 comments · Fixed by #2859
Closed
1 task done
Labels
AZD 🧑‍💻 These modules are requested/used by the AZD team. Class: Resource Module 📦 This is a resource module Needs: Attention 👋 Reply has been added to issue, maintainer to review Needs: Core Team 🧞 This item needs the AVM Core Team to review it Status: Module Orphaned 👀 The module has no owner and is therefore orphaned at this time Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: Feature Request ➕ New feature or request

Comments

@NanaXiong00
Copy link
Contributor

Check for previous/existing GitHub issues

  • I have checked 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: #632

For example (azd):

CC: @jongio

(Optional) Correlation Id

No response

@NanaXiong00 NanaXiong00 added Needs: Triage 🔍 Maintainers need to triage still Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue labels Jul 3, 2024
@github-project-automation github-project-automation bot moved this to Needs: Triage in AVM - Module Issues Jul 3, 2024
@avm-team-linter avm-team-linter bot added the Class: Resource Module 📦 This is a resource module label Jul 3, 2024
Copy link

@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!

@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 Jul 9, 2024
@eriqua eriqua added Needs: Core Team 🧞 This item needs the AVM Core Team to review it AZD 🧑‍💻 These modules are requested/used by the AZD team. and removed Needs: Triage 🔍 Maintainers need to triage still Status: Response Overdue 🚩 When an issue/PR has not been responded to for X amount of days labels Jul 10, 2024
@eriqua eriqua moved this from Needs: Triage to In Active Discussion in AVM - Module Issues Jul 10, 2024
@jtracey93 jtracey93 added the Status: Module Orphaned 👀 The module has no owner and is therefore orphaned at this time label Jul 12, 2024
@jtracey93
Copy link
Contributor

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: #1934 (comment) ?

#RR

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Author Feedback 👂 Awaiting feedback from the issue/PR author label Jul 12, 2024
@v-xuto
Copy link
Member

v-xuto commented Jul 15, 2024

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

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Attention 👋 Reply has been added to issue, maintainer to review and removed Needs: Author Feedback 👂 Awaiting feedback from the issue/PR author labels Jul 15, 2024
@v-xuto
Copy link
Member

v-xuto commented Jul 26, 2024

@AlexanderSehr
Copy link
Contributor

as commented in the PR, this depends on #1934

@zedy-wj
Copy link
Member

zedy-wj commented Jul 29, 2024

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

@AlexanderSehr
Copy link
Contributor

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 :)

@github-project-automation github-project-automation bot moved this from In Active Discussion to Done in AVM - Module Issues Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AZD 🧑‍💻 These modules are requested/used by the AZD team. Class: Resource Module 📦 This is a resource module Needs: Attention 👋 Reply has been added to issue, maintainer to review Needs: Core Team 🧞 This item needs the AVM Core Team to review it Status: Module Orphaned 👀 The module has no owner and is therefore orphaned at this time Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: Feature Request ➕ New feature or request
Projects
6 participants