feat: Script Provisioning Provider extension (microsoft.azd.scripts)#7737
feat: Script Provisioning Provider extension (microsoft.azd.scripts)#7737wbreza wants to merge 5 commits into
Conversation
0665896 to
30e76ca
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… conflict azd now reserves --debug as a global flag. Extensions receive debug state via AZD_DEBUG and AZD_EXT_DEBUG environment variables instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
left a comment
There was a problem hiding this comment.
6 findings from deep review. Key items:
- 2 HIGH: nondeterministic env var resolution order, no provider.go tests
- 2 MEDIUM: ContinueOnError zero-value limitation, raw stderr warning
- 2 LOW: destroy outputs discarded, missing ctx check between iterations
The extension architecture is clean - good separation of config/resolver/executor/collector. The 4-layer env resolution model is well-designed. Tests cover the individual components thoroughly.
Implements the Script Provisioning Provider extension that enables shell script-based provisioning and teardown workflows in azd. This extension registers a 'scripts' provisioning provider via gRPC, allowing users to configure bash/PowerShell scripts as their infrastructure provider in azure.yaml. Key components: - Extension scaffold following microsoft.azd.demo pattern - Config parsing with validation (path safety, kind inference, platform overrides) - EnvResolver with 4-layer environment variable merging - ScriptExecutor for bash/pwsh script execution - OutputCollector for outputs.json discovery and parsing - Full ProvisioningProvider interface implementation Resolves Azure#7734, Azure#7735, Azure#7736 Part of Azure#7733 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix ContinueOnError bug: platform override no longer unconditionally resets the field when the override doesn't set it - Fix getAzdEnv: propagate context cancellation, warn on other errors instead of silently swallowing all failures - Add 10MB size limit on outputs.json to prevent OOM - Remove dead ScriptResult fields (Stdout/Stderr never populated) - Remove unused exported functions (OutputsToEnvMap, OutputsToProvisioning) - Extract toProtoOutputs helper to reduce duplication - Improve shBinary() portability: use LookPath with /bin/sh fallback - Add platform override tests (ContinueOnError preservation, env merge) - Add executor unit tests (buildShellCommand, mapToEnvSlice) - Fix hardcoded /tmp path in test to use t.TempDir() - Update README: clarify secrets are plain values in alpha Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Sort env map keys before iteration for deterministic resolution - Change ContinueOnError to *bool to allow platform overrides to explicitly disable continue-on-error from base config - Replace fmt.Fprintf(os.Stderr) with log.Printf for consistent extension framework logging - Add ctx.Err() check between script iterations for responsive cancellation - Add comment clarifying destroy script outputs are intentionally discarded Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
30e76ca to
15a7e82
Compare
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
jongio
left a comment
There was a problem hiding this comment.
Three additional items beyond the existing review. The architecture is clean - good separation of config/resolver/executor/collector, and the security validation (path traversal, absolute paths) is solid.
| return nil, fmt.Errorf("checking outputs file %q: %w", outputsPath, err) | ||
| } | ||
|
|
||
| if fi.Size() > maxOutputFileSize { |
There was a problem hiding this comment.
The collector reads outputs.json from the script directory without verifying it was produced by the current execution. If a previous run left an outputs.json and the script doesn't overwrite it this time, stale outputs are silently returned.
Two options:
- Delete (or rename)
outputs.jsonbefore execution so absence means no outputs - Check mtime vs script start time
For alpha this is probably fine, but worth a comment noting the assumption that scripts always overwrite if they produce outputs.
| // Sort keys for deterministic resolution order — important when | ||
| // variables in the same env map reference each other. | ||
| for _, k := range slices.Sorted(maps.Keys(sc.Env)) { | ||
| tmpl := sc.Env[k] |
There was a problem hiding this comment.
nit: The sorted iteration makes resolution deterministic, which is good. Worth noting the behavior explicitly though - if A refs B and A < B alphabetically, A resolves first and gets empty string because B hasn't been processed yet.
A one-line comment like // NB: forward references within the same env map resolve to empty would save someone a debugging session.
| Write-Host "Running: go build ``" | ||
| PrintFlags -flags $buildFlags | ||
| go build @buildFlags | ||
| if ($LASTEXITCODE) { |
There was a problem hiding this comment.
GOEXPERIMENT=loopvar became the default in Go 1.22. Since the module targets Go 1.26, this block is dead code and can be removed.
|
Hi @@wbreza. Thank you for your interest in helping to improve the Azure Developer CLI experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days. |
|
Hi @@wbreza. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing "/reopen" if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the "no-recent-activity" label; otherwise, this is likely to be closed again with the next cleanup pass. |
Description
Implements the Script Provisioning Provider extension (
microsoft.azd.scripts) that enables shell script-based provisioning and teardown workflows in azd. This is a first-party extension that registers ascriptsprovisioning provider via gRPC.Epic: #7733
Depends on: #7482 (provisioning provider framework)
Resolves: #7734, #7735, #7736
Architecture
The extension is a pure provisioning provider — no custom CLI commands, no MCP server. It registers via
WithProvisioningProvider("scripts", factory)and implements the fullazdext.ProvisioningProviderinterface.Key Components
internal/provisioning/config.goinfra.configwith validationinternal/provisioning/env_resolver.gointernal/provisioning/executor.gointernal/provisioning/output_collector.gooutputs.jsonfilesinternal/provisioning/provider.goProvisioningProviderimplementationUser Configuration
Testing
Checklist
go buildscriptsprovider via gRPC.sh->sh,.ps1->pwsh)${EXPRESSION}substitution