Fix C# + SQL Azure template #550
Conversation
karolz-ms
requested review from
jongio,
wbreza,
ellismg,
vhvb1989,
hemarina and
weikanglim
as code owners
|
Manual test completed, no issues found |
karolz-ms
force-pushed
the
csharp-azure-sql3
branch
from
cd482bc
to
a1a5cf2
Compare
There was a problem hiding this comment.
Some small questions about what happens if you end up running provision multiple times. I will admit that I am both excited and scared by the command running technology we are adding, but I can't really think of a better way to do this (all my solutions look like use uniqueString so we end up with something "unguessable" but it would be easy to compute if you knew all the inputs, which doesn't seem great).
Thinking about what this would look like in TF or Pulumi, as you mention we'd have the random resource there and it would be used to generate a password, but critically, the output would be stored in the state file and not change run to run.
Part of me wonders if we should be using [randomPassword()] instead of $(randomPassword) since that's closer to ARM syntax, but that's pretty bike-sheddy.
|
(need to do some testing, do not merge) |
|
Tested the new template and command, all looks good. @ellismg please review again! |
Adds password generation capability to AZD CLI
karolz-ms
force-pushed
the
csharp-azure-sql3
branch
from
53be81a
to
d805a7f
Compare
There was a problem hiding this comment.
A few suggestions - I feel most strongly about us not enumerating the existing KeyVault resources from the deployment if we can, I think that's possible and reasonable to do now that we pass the KeyVault name as a parameter to secretOrRandomPassword in but if I'm missing something let me know.
Repoman Generation ResultsRepoman pushed changes to remotes for the following projects: Project: todo-csharp-sqlRemote: azure-samples-stagingBranch: pr/550You can initialize this project with: azd init -t Azure-Samples/todo-csharp-sql -b pr/550View Changes | Compare Changes |
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsStandalone Binary
Container |
|
Thank you, Matt, your suggestions and collaboration made it way better than what I could have come up with on my own! |
A bigger change than just the template because we need to add password-generation capability to AZD (for Bicep). Note that if we decide to have an equivalent Terraform template, no extra work will be necessary because Terraform already has password generation built-in.
The changes to deployment provider (renaming
Preview()toPrepare()method and havingDeploy()take aPreparedDeploymentis something @ellismg @HadwaAbdelhalem and @wbreza discussed offline.I will need some help with making this template subject to automated tests and flipping the corresponding repo to public--TIA!
Fixes #450