-
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
WIP: provisioning: Consider resourceGroup
from azure.yaml
#4314
base: main
Are you sure you want to change the base?
Conversation
We allow specifying the resource group which is used for discovery of resources (instead of probing for a resource group taged with `azd-env-name` inside `azure.yaml`. When doing a resource group scoped deployment, we should consider this value if set and use it instead of prompting the user to select a resource group or create a new one. Fixes Azure#4287
@@ -52,10 +54,9 @@ type StateResult struct { | |||
|
|||
type Provider interface { | |||
Name() string | |||
Initialize(ctx context.Context, projectPath string, options Options) error | |||
Initialize(ctx context.Context, projectPath string, options Options, rg osutil.ExpandableString) error |
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 initially considered trying to have Initialize
take the entire project.ProjectConfig
object so it could grab this property, but that doesn't work because it creates an import cycle between the project
and provisioning
packages.
Sort of a bummer that the resourceGroup
property doesn't hang off of the provisioning.Options
type. It probably would have made more sense to nest that under infra
vs it being a top level property.
ctx context.Context, | ||
projectPath string, | ||
options provisioning.Options, | ||
_ osutil.ExpandableString, |
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.
Wondering what if anything we should do here for Terraform, perhaps we should be adding this to envVars
with something like AZURE_RESOURCE_GROUP
so tf
could observe it?
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
// | ||
// We also need to understand the policy of what happens if both are set - which one wins? I think that perhaps | ||
// it should be `AZURE_RESOURCE_GROUP` since that also allows overriding via `AZURE_RESOURCE_GROUP=foo azd up`. | ||
p.env.DotenvSet(environment.ResourceGroupEnvVarName, rgEval) |
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.
This is pretty gross, IMHO, but the rest of the system right now reads environment.ResourceGroupEnvVarName
from the .env
to find the resource group to use. I suspect that I need to do a deeper refactoring such that we can consider both what is in the azure.yaml
file and what is in the .env
but right now doing things this way lets the rest of the system work.
But there are open questions, hence the big TODO and the draft nature of this PR...
We allow specifying the resource group which is used for discovery of resources (instead of probing for a resource group taged with
azd-env-name
insideazure.yaml
. When doing a resource group scoped deployment, we should consider this value if set and use it instead of prompting the user to select a resource group or create a new one.Fixes #4287