Azure / azure-dev

A developer CLI that reduces the time it takes for you to get started on Azure. The Azure Developer CLI (azd) provides a set of developer-friendly commands that map to key stages in your workflow - code, build, deploy, monitor, repeat.
https://aka.ms/azd
MIT License
410 stars 198 forks source link

[Issue] Support for secret reference in parameters.json files #1474

Open pamelafox opened 1 year ago

pamelafox commented 1 year ago

Output from azd version azd version 0.5.0-beta.4-pr.2128509 (commit 8b3f34d1a4706a777bbcbe2960de4a75656d94f3)

Output from az version { "azure-cli": "2.40.0", "azure-cli-core": "2.40.0", "azure-cli-telemetry": "1.0.8", "extensions": { "containerapp": "0.3.11", "containerapp-compose": "0.2.2" } }

Describe the bug

I am trying to come up with a secure way to pass secrets into the container apps module. Unfortunately, container app environment variables don't support keyvault references the way that app service does, so the secrets need to be stored in the app itself.

My first approach was in this PR where I passed it from main.parameters.json to web.parameters.json to web.bicep to the container apps module. That approach didn't work since the parameters became outputs, and we can't have secrets in outputs.

Then I tried to use the keyVault.getSecret() function in my web.bicep file. That did not work, because that function can only be used directly for passing a parameter into a module - I can't use it inside an array or object. I could conceivably add an entirely new bicep file between web.bicep and container-app.bicep to satisfy that constraint, but that adds a lot of unnecessary bicep.

Then I tried to reference keyvault secrets inside web.parameters.json. When I tried it with azd up, it did not work on the deployment step, giving this error:

Deploying services (azd deploy)   
(✓) Done: Deploying service web

ERROR: deploying service: deploying service web package: provisioning infrastructure for app deployment: error deploying infrastructure: failed deploying: starting deployment to resource group: PUT https://management.azure.com/subscriptions/32ea8a26-5b40-4838-b6cb-be5c89a57c16/resourcegroups/flasksurveys-rg/providers/Microsoft.Resources/deployments/flasksurveys-web

--------------------------------------------------------------------------------
RESPONSE 400: 400 Bad Request
ERROR CODE: InvalidDeploymentParameterValue
--------------------------------------------------------------------------------
{  "error": {    "code": "InvalidDeploymentParameterValue",    "message": "The value of deployment parameter 'postgresPassword' is null. Please specify the value or use the parameter reference. See https://aka.ms/arm-create-parameter-file for details."  }}

To figure out whether the problem was azd or Bicep, I then tried using it with the Azure CLI:

az deployment group create --resource-group flasksurveys-rg --template-file infra/web.bicep --parameters infra/web.parameters.json

That didn't work at first, but I realized I had to enable template deployment for the key vault. Once I did that, my deployment worked and I confirmed the container app secret matched the key vault secret.

I double checked with azd up again, and it still did not work. That version of the code is in this branch: https://github.com/pamelafox/flask-surveys-container-app/compare/secretstuff?expand=1

As a workaround, I am using the Python SDK to retrieve the Key Vault secrets in the app code itself: https://github.com/pamelafox/flask-surveys-container-app/blob/main/backend/settings/production.py

That's fine, but some developers may need a declarative way to pass secrets in the infra files, so azd could consider supporting referencing key vault secrets in parameters.json files.

To Reproduce

  1. git clone this branch:

https://github.com/pamelafox/flask-surveys-container-app/compare/secretstuff?expand=1

  1. Change the values here back to the environment variable references: https://github.com/pamelafox/flask-surveys-container-app/compare/secretstuff?expand=1#diff-3828544b78949b9b207e34357d5f185cdbd6c9e7e6eeacbf261239b73452bf41

  2. Run azd up

rajeshkamal5050 commented 1 year ago

@jongio can you take a look and see if we need to create any issue upstream with Bicep?

jongio commented 1 year ago

It looks like azd needs to be updated to support key value in params file.

pamelafox commented 1 year ago

Update: Thanks to the new flow for container apps that doesn't require the additional *.parameters.json file, I can use the approach of specifying the secrets on the container app itself (and then secretRefs in the env variables). Here's a PR where I did that: https://github.com/pamelafox/flask-surveys-container-app/pull/27/files#diff-2f06b0134fd9f3d722ed460c2b3425720dda2da330db115f2bb81f96382468ab

pamelafox commented 1 year ago

Update: And now container apps supports fetching secrets from key vault, so that approach would work as well. I think it'd be fine to close this issue if noone else needs it. I no longer need it.