feat: avoid prompting for layer outputs in pipeline config#7296
Open
weikanglim wants to merge 1 commit intoAzure:mainfrom
Open
feat: avoid prompting for layer outputs in pipeline config#7296weikanglim wants to merge 1 commit intoAzure:mainfrom
pipeline config#7296weikanglim wants to merge 1 commit intoAzure:mainfrom
Conversation
weikanglim
changed the title
pipeline config
weikanglim
force-pushed
the
weikanglim/virtual-layer-planned-outputs
branch
3 times, most recently
from
505b3bc to
0c9a632
Compare
weikanglim
force-pushed
the
weikanglim/virtual-layer-planned-outputs
branch
from
0c9a632 to
91a7eaf
Compare
weikanglim
requested review from
JeffreyCA,
hemarina,
rajeshkamal5050,
tg-msft,
vhvb1989 and
wbreza
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for azd pipeline config to understand dependencies between layered infra plans by treating outputs from earlier layers as “virtually resolved” environment variables, so later layers don’t prompt for those parameters during pipeline setup.
Changes:
- Extend the provisioning provider interface with
PlannedOutputs()and plumb it throughprovisioning.Manager. - Add
Options.VirtualEnvand update the Bicep provider’s parameter loading/prompting to treat virtual-mapped params as resolved (skip prompting/config). - Update
pipeline configto accumulate plan-time outputs across layers and add tests covering virtual env substitution + prompt skipping.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/pkg/infra/provisioning/provider.go | Adds VirtualEnv to options and introduces PlannedOutput + Provider.PlannedOutputs() API. |
| cli/azd/pkg/infra/provisioning/manager.go | Exposes Manager.PlannedOutputs() to forward to the initialized provider. |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go | Implements virtual env substitution, tracks virtual-mapped parameters, skips prompting for them, and implements PlannedOutputs(). |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go | Adds/adjusts tests for virtual env substitution and required-parameter prompt skipping. |
| cli/azd/cmd/pipeline.go | Accumulates planned outputs across layers into a virtual env map for pipeline config. |
| cli/azd/pkg/infra/provisioning/terraform/terraform_provider.go | Adds no-op PlannedOutputs() implementation. |
| cli/azd/pkg/infra/provisioning/test/test_provider.go | Adds no-op PlannedOutputs() implementation for the test provider. |
| cli/azd/pkg/devcenter/provision_provider.go | Adds no-op PlannedOutputs() implementation. |
Comment on lines
+2840
to
+2844
| for key := range compileResult.Template.Outputs { | ||
| outputs = append(outputs, provisioning.PlannedOutput{ | ||
| Name: key, | ||
| }) | ||
| } |
There was a problem hiding this comment.
PlannedOutputs currently includes every template output, including secure outputs (secureString/secureObject). Secure outputs can’t be retrieved from ARM deployments and are skipped when populating .env, so counting them as plan-time outputs can cause pipeline config to skip prompting for downstream parameters that will never be populated. Filter out secured outputs here (e.g., via azure.IsSecuredARMType(param.Type)) so only persistable outputs are returned.
Comment on lines
+201
to
+204
| outputs, err := p.provisioningManager.PlannedOutputs(ctx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get outputs for provider %s: %w", pipelineProviderName, err) | ||
| } |
There was a problem hiding this comment.
PlannedOutputs is called for every layer, even when len(layers) == 1 (where VirtualEnv is never applied). Consider calling PlannedOutputs (and maintaining virtualEnv) only when len(layers) > 1 to avoid extra provider work in the common single-layer case.
Comment on lines
1766
to
1769
| } | ||
| // principalId and locations are intentionally excluded from the mapped env vars as | ||
| // they are global env vars | ||
| result.mappedEnvVars = append(result.mappedEnvVars, name) |
There was a problem hiding this comment.
The comment says principalId and locations are excluded from mappedEnvVars, but AZURE_LOCATION will still be appended to result.mappedEnvVars (only principal id/type early-return). Either update the comment or adjust the logic so the documented behavior matches what’s tracked.
See below for a potential fix:
} else {
// principalId and locations are intentionally excluded from the mapped env vars as
// they are global env vars
result.mappedEnvVars = append(result.mappedEnvVars, name)
}
Comment on lines
196
to
+203
| return nil, fmt.Errorf("failed to get parameters for provider %s: %w", pipelineProviderName, err) | ||
| } | ||
|
|
||
| allParameters = append(allParameters, providerParameters...) | ||
|
|
||
| outputs, err := p.provisioningManager.PlannedOutputs(ctx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get outputs for provider %s: %w", pipelineProviderName, err) |
There was a problem hiding this comment.
This error message uses pipelineProviderName (CI provider like GitHub/AzDO) in the "failed to get outputs for provider %s" text, but the failing component here is the infra provider for the current layer. Consider reporting the infra provider kind/name (e.g., layer.Provider or the provisioning provider’s Name()) to make troubleshooting clearer.
Suggested change
| return nil, fmt.Errorf("failed to get parameters for provider %s: %w", pipelineProviderName, err) | |
| } | |
| allParameters = append(allParameters, providerParameters...) | |
| outputs, err := p.provisioningManager.PlannedOutputs(ctx) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to get outputs for provider %s: %w", pipelineProviderName, err) | |
| return nil, fmt.Errorf("failed to get parameters for provider %s: %w", layer.Provider, err) | |
| } | |
| allParameters = append(allParameters, providerParameters...) | |
| outputs, err := p.provisioningManager.PlannedOutputs(ctx) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to get outputs for provider %s: %w", layer.Provider, err) |
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
vhvb1989
approved these changes
Mar 26, 2026
Member
vhvb1989
left a comment
vhvb1989
left a comment
There was a problem hiding this comment.
Part of me feels like pipeline config is reaching its limit and will stay way behind from any agent -
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add support for
pipeline configto detect outputs between layers and avoid prompting for them.Why
In a normal provision run, these outputs are not user-provided, and would be automatically set by the provisioning engine.
What's changing
cmd/pipeline.go, we collect outputs from each previous layer, store them as a virtual env-mapped values.A new
PlannedOutputs()function is added to the infra provider interface that represents plan-time outputs (no resolved values).Testing
Fixes #7182