feat: avoid prompting for layer outputs in pipeline config#7296
Open
weikanglim wants to merge 2 commits intoAzure:mainfrom
Open
feat: avoid prompting for layer outputs in pipeline config#7296weikanglim wants to merge 2 commits intoAzure:mainfrom
pipeline config#7296weikanglim wants to merge 2 commits intoAzure:mainfrom
Conversation
weikanglim
changed the title
pipeline config
weikanglim
force-pushed
the
weikanglim/virtual-layer-planned-outputs
branch
4 times, most recently
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. |
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 -
weikanglim
force-pushed
the
weikanglim/virtual-layer-planned-outputs
branch
from
91a7eaf to
5d5b215
Compare
jongio
reviewed
Mar 27, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
PR Review - #7296
feat: avoid prompting for layer outputs in pipeline config by @weikanglim
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic | 0 | 0 | 2 | 1 |
| Architecture | 0 | 0 | 1 | 0 |
| Quality | 0 | 0 | 0 | 1 |
| Total | 0 | 0 | 3 | 2 |
Key Findings
- [MEDIUM] Logic: Multi-layer error path double-wraps
layer.Name-inputParametersalready includes it, and the caller wraps again, producinglayer 'foo': layer "foo": failed to... - [MEDIUM] Logic: If a parameter value mixes virtual and real env vars (e.g.,
${PREV_LAYER_OUTPUT}/${REAL_SECRET}),hasVirtualEnvVarcauses the whole parameter to be skipped. The real env var mapping is recorded inenvMappingbefore the skip, butensureParameterswon't process the parameter at all - doesParameters()still surface the real env var's CI/CD requirement? - [MEDIUM] Architecture:
.bicepparamcompilation goes throughcompileBicep()/BuildBicepParamusingp.env.Environ()directly.VirtualEnvisn't injected there, so multi-layer pipelines with.bicepparamfiles won't get virtual env substitution. - [LOW] Logic: Duplicate output names across layers are silently overwritten in
virtualEnv(last-writer-wins) - worth documenting if intentional - [LOW] Quality: Comment "save a dummy value that is easily looked at" doesn't explain the sentinel format
What's Done Well
- Clean split between single-layer and multi-layer paths keeps the common case simple
PlannedOutputtype is intentionally minimal - good API design restraint- Thorough test coverage: virtual env substitution, parameter skipping, and secure output filtering each have dedicated tests
- All Provider implementations updated consistently with no-op stubs
- Previous review feedback fully addressed
3 inline comments below.
|
|
||
| parameters, err := inputParameters(layer) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("layer '%s': %w", layer.Name, err) |
Member
There was a problem hiding this comment.
[MEDIUM] Logic: Double-wrapped layer name in error message
inputParameters() already wraps its errors with the layer name (e.g., layer "foo": failed to initialize...). This outer fmt.Errorf wraps again, producing:
layer 'foo': layer "foo": failed to initialize infra provider bicep: <err>
The single-layer path at line 208 returns the error directly. This should do the same.
Suggested change
| return nil, fmt.Errorf("layer '%s': %w", layer.Name, err) | |
| return nil, err |
|
|
||
| // Save current outputs | ||
| for _, output := range outputs { | ||
| // save a dummy value that is easily looked at |
Member
There was a problem hiding this comment.
[LOW] Quality: This comment doesn't explain why the value uses the layerName--outputName format.
Suggested change
| // save a dummy value that is easily looked at | |
| // Use a sentinel value encoding the source layer, so downstream parameter | |
| // resolution can trace which layer provides each virtual env var. |
| parametersMappedToAzureLocation = append( | ||
| parametersMappedToAzureLocation, substResult.parametersMappedToAzureLocation...) | ||
|
|
||
| if substResult.hasVirtualEnvVar { |
Member
There was a problem hiding this comment.
[MEDIUM] Logic: Mixed virtual + real env var references in one parameter
If a parameter value references both a virtual env var and a real one (e.g., "${PREV_LAYER_OUTPUT}/${REAL_SECRET}"), hasVirtualEnvVar is set and this continue skips the parameter entirely. The real env var IS recorded in envMapping above, but ensureParameters also skips virtual parameters - so the real portion won't be prompted or configured.
Is it safe to assume that a parameter referencing a virtual env var will never also reference a real one? If so, worth a comment here. If not, this needs a different strategy for mixed references (e.g., only skip prompting but still track the real env vars for CI/CD config).
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