-
Notifications
You must be signed in to change notification settings - Fork 198
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
pipeline additional vars secrs #3155
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like that we're thinking about how to extend pipeline config
beyond its current state. Overall, the feature by itself makes a ton of sense.
If I could wave a magic wand, the part I would be really curious about is how we can really streamline this entire process at a product level to make it a more cohesive experience.
For me, it seems like as a user:
- I have some metadata that defines required and optional inputs/parameters
- The environment is expected to supply these values
azd could be smart enough to say "Here's all the things you need to configure". This would work both for local deployments, and for CI deployments. In the current model, all these concepts are rather loose, which the user has to understand "local configuration" then "map to CI configuration".
There are several and significant differences between
|
A user wants deployments to be repeatable. It should be repeatable on a local machine and on CI.
A user cares about the sensitivity of values. Environment secrets vs. variables should exist as a general concept, even on a local environment. We currently don't do this but we should. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for kicking this off, @vhvb1989. A few high level comments:
-
I'm really excited about the overall experience here. I think having our project guide folks through the configuration process of the pipeline is really useful. I'm glad we're spending some time here.
-
I'm wondering if we should merge the
variables
andsecrets
sections together in favor of a single group, and then have somesecret: bool
property on every value? -
It would be nice if we could have some properties on each input? We might start with
description
to explain how the value is used? -
If we go down that line, the values in
azure.yaml
here become more like inputs that can be configured with values that might default to something in the.env
instead of just copying values from.env
toazure.yaml
. Is that better for us long term? -
I'm nervous about
additionalVariablesAsSecrets
. I feel like this is a footgun that's going to lead to bad UX and people to do the wrong thing. Do you think we need it?
Thank you @ellismg , let me add some thoughts here:
I considered this at the beginning. But, while it looks good for someone using only variables (and maybe just one secret only): pipeline:
variables:
- foo
- bar
- other:
- secret: true I don't like how it looks for someone using only secrets, as it becomes very repetitive. pipeline:
variables:
- foo:
- secret: true
- bar:
- secret: true
- other:
- secret: true That's why I decided to break it into a section for each, as a way to avoid the repetition (and extra lines).
What would azd do with such description? pipeline:
variables:
# Need this to do foo on github...
- foo
- bar
I don't really want to go that route. As it can open the risk of saving secrets on azure.yaml. The list of secrets/variables in azure.yaml should be an indirection to the values (similar to the Key Vault approach).
I think we need. It gives a flexible option to users with templates with a huge list of variables to just send it all to the pipeline. And, since we don't know if there might be sensitive info, I made it secrets only. The next thing I want to do (eventually), is to use a go-template for the pipeline definition. Then, mapping the variables/secrets to the pipeline definition would also be done by azd :). And I want to let customers to update/use their own go-template to override the pipeline definition :) |
Yeah - seeing this listed out in code does make it seem like separating them makes sense.
But I think this leads to bad outcomes. For example, if I look at the sample, I see that we'll end up setting resource group names as secrets, which means the CI output will be worse. I think we want to force authors to make a decision on if these values are secrets since we can't answer that question and defaulting to "secret" leads to a much worse experience.
I was thinking about this feature slightly differently than you. In my view, I was hoping it would be: "here's a set of values that need to be configured on the pipeline, and |
@ellismg , let me do one last try in the name of "additional as secrets" idea
The way it currently works is, only the env vars which are not included in the list of variables (that's where the The fundamental issue I want to have a good story here is having a way to define the minimum (and really required) set of fields to define the pipeline vars/secrets. If we take a look to the base customer's issue here: #1597 . The wish-list is a way to bulk the entire azd-env into gh pipeline secrets. We can even see they are currently trying things like
What do you think? Can we try it :) Another orthogonal fact: |
I do think this is the crux of the issue. My concern here is that we are making it too easy for people to say the set of vars/secrets is "everything" without that actually being the right answer. One thing I'm still struggling with is the fact that we use these vars and secrets for a lot of infrastructure configuration that would be controlled via I think that the answer of "write some sed" for the "dump everything in the .env file into GH" is actually a decent answer. I could imagine adding more output format to I think I would like it if we had a way to clearly state: "Here's the exact set of values that need to be configured on your pipeline" to make things work. I like that with what you have here I can look at a single place in
I agree there will be a lot here - but I feel like this being self describing for us will be the right long term thing. We'll be able top build better tooling on top of stuff. |
@ellismg , I have remove the all-vars-as-secrets functionality. Please take a look |
@ellismg ping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small comments around the caching, but LGTM otherwise. Thanks as always for putting up with my feedback, @vhvb1989.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have any issues with the code changes but would urge us to consider whether this is the approach we should be taking with regard to setting secrets in your pipeline.
Having this new secrets
node under pipeline sets the precedent that your secrets should be available in the azd environment which is actually an anti-pattern.
Instead we should be thinking about how to leverage more secure secrets storage like keyvault.
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSIContainer
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Remove in-memory `main.parameters.json` cache. By removing this in-memory cache, we incur an extra load from disk, but without any noticeable behavioral changes. Background: In-memory caching for `main.parameters.json` was added initially to avoid a re-prompt when the user declines the prompt "Save the value in the environment for future use?" (#3155). This was added as a way to communicate within `azd` that "this value is configured for the current azd process lifetime". This was superseded by the changes in #3479 where we always saved the values to `config.json` and removed the prompt. Fixes #3973, #4310
This change adds support for projects to define additional variables or secrets to be set as part of the gh actions workflow or azdo pipelines.
fix: #1597
Strategy
Adding 2 new fields to
azure.yaml
for pipeline:With this configuration, azd will find
variables
within the azd-environment and set them as variables for the pipeline duringazd pipeline config
.The same will happen with
secrets
, but the values from azd-environment are set as secrets on the pipeline.other required changes
Call
ensureParameters()
during bicep_provider initialization. This change enablesazd pipeline config
to prompt for required parameters and gives the option to save the answer. When the answer is saved, it is used during CI/CD.This is a change that covers a similar scenario as the one we have for .NET Aspire, which prompts user to select what services to expose. But, instead of services to expose, azd ask the user for required bicep parameters w/o deploying.
Create memory cache for compile bicep and ensure parameters. Since we are compiling bicep during init and also checking parameters, we want to skip doing this again when a plan is created (within the same azd execution).