Add secure() for secrets
This PR makes it so that you can securely pass secrets into the container-app modules, by wrapping it in a secure() decorator. Unfortunately, we can't mark arrays as @secure, so you have to instead pass in an object of the secret key/values, and we turn that back into the expected array format.
Note that I haven't tested these exact modules since I'm working off a fork of the modules (my use case isn't compatible with the azd modules.. will file an issue about that). You can see my change here, which works: https://github.com/Azure-Samples/langfuse-on-azure/pull/9
I don't think any of the TODO templates utilize secrets for container apps, so not sure there's a test to do on the azd template side.
@v-xuto - Can you please search the templates published at awesome-azd for container apps that use secrets to see how big of a breaking change this would be?
@jongio @wbreza , templates are not automatically updated to the latest bits from /core templates. We don't even have a quick way of doing the update. When folks want to pull and sync to latest /core templates, they need to clone azure-dev repo, pull latest main and copy/paste the core modules folder to their template.
For what I've seen, folks would only try to update individual modules when there's an error of there is a required version update for the smart-defaults.
Is that enough data to lower the relevance of the breaking changes effects on /core modules?
It might be good to check for secrets usage just to make sure there aren't any there that are using current secrets, since they result in security alerts. But agree that I don't particularly think it should block if it doesnt affect TODO (which I didnt see evidence of, but worth checking again!)
Yes, just a scan to see if they are using and warn them as a courtesy.
@v-xuto please...
- Check all awesome azd templates for container/secrets usage and report back.
- Regression test related todo templates that use this with the changes from this PR.
@jongio
- For all awesome templates,running:
azd template list, there are a total of 97 templates, of which 35 use container/secrets. - For related todo templates(
todo-nodejs-mongo-aca,todo-python-mongo-aca,todo-java-mongo-aca), We did not continue to use the current PR change for testing because we encountered the following issues before any changes were made.
@jongio
- For all awesome templates,running:
azd template list, there are a total of 97 templates, of which 35 use container/secrets.- For related todo templates(
todo-nodejs-mongo-aca,todo-python-mongo-aca,todo-java-mongo-aca), We did not continue to use the current PR change for testing because we encountered the following issues before any changes were made.
@wbreza thoughts on that error?
@jongio , the error was due to: https://teams.microsoft.com/l/message/19:[email protected]/1711500134341?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=3e17dcb0-4257-4a30-b843-77f47f1d4121&parentMessageId=1711500134341&teamName=Azure%20SDK&channelName=Engineering%20System%20%F0%9F%9B%A0%EF%B8%8F&createdTime=1711500134341
It should work now, as the roles are restored
btw, @jongio @v-jiaodi , there are big changes on all todo-templates seating on the staging branch.
You might want to look into that branch instead of main until we release the new version (probably next week)
It should work now, as the roles are restored
@vhvb1989 Tested again and still encountered the same issue.
Environment:
- Template:
todo-nodejs-mongo-aca. - branch:
staging - OS: Windows
- Azd version : 1.8.0-beta.1 (commit https://github.com/Azure/azure-dev/commit/733bfdc26c871be4e9b3e077069a6e0d209c837d)
Here is my sub permission list:
@vhvb1989 Tested again and still encountered the same issue.
Should really be fixed now
@v-jiaodi , can you try again? (You might need to azd auth logout and azd auth login again.
The roles should be restored by now
@vhvb1989 Now it can work normally.
- Regression test related todo templates that use this with the changes from this PR.
@jongio For related todo templates(todo-nodejs-mongo-aca,todo-python-mongo-aca,todo-java-mongo-aca), it works with the changes from this PR.
@rajeshkamal5050 - It would be good to include this in breaking changes list with instructions on how to implement with the new method. Pamela is that something you can write up?
Suggested for releasse notes-
Breaking change, made to improve security: The container-app.bicep and container-app-upsert.bicep modules now mark the secrets parameter as secure(), and expect secrets to be an object (versus an array). You will need to reformat your array of [{name: 'secret-name', value: secretValue}, ...] as an object like {'secret-name': secretValue, ...} instead.
@pamelafox FYI {SECRET_NAME: SECRET_VALUE, ...} doesn't work. You need {secret-name: SECRET_VALUE, ...} since you're creating a secret name and not the environment variable name of the secret.
@weikanglim Ah yeah, my placeholders were misleading. Updated suggested text to look more like a real diff (such as https://github.com/Azure-Samples/langfuse-on-azure/commit/105148fffde072dd03e56c8d686963e665b8013e)
