feat: support hooks per provisioning layer#7382
Conversation
0fa3ae0 to
df286b5
Compare
There was a problem hiding this comment.
Pull request overview
Adds first-class hook support scoped to individual provisioning layers, and wires that behavior through azd’s provisioning flow, azd hooks run, schemas, and tests to address per-layer provisioning hook needs.
Changes:
- Extend
infra.layers[].hooks(pre/post-provision) and execute those hooks duringazd provisionper layer. - Add provisioning-layer discovery and filtering to
azd hooks runvia--layer. - Update v1.0/alpha schemas and add unit/functional/snapshot coverage (including hook YAML compatibility moved into
ext.HooksConfig).
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| schemas/v1.0/azure.yaml.json | Adds infra.layers (with per-layer hooks) and conditional validation to prevent mixing root infra fields with layers. |
| schemas/alpha/azure.yaml.json | Adds per-layer hooks support in the alpha schema. |
| cli/azd/test/functional/testdata/samples/hooks/infra/main.parameters.json | Adds functional sample parameters for the hooks test infra. |
| cli/azd/test/functional/testdata/samples/hooks/infra/main.bicep | Adds minimal functional sample infra for hooks testing. |
| cli/azd/test/functional/testdata/samples/hooks/azure.yaml | Adds a functional sample exercising command/service/layer hooks. |
| cli/azd/test/functional/testdata/samples/funcapp copy/up.log | Removes a previously checked-in functional log artifact. |
| cli/azd/test/functional/hooks_test.go | Adds functional coverage for hook registration/execution across deploy/provision and hooks run. |
| cli/azd/pkg/project/project_config.go | Replaces local HooksConfig implementation with an alias to shared ext.HooksConfig. |
| cli/azd/pkg/infra/provisioning/provider.go | Adds per-layer hooks config, validates allowed hook names, and adds layer path resolution helper. |
| cli/azd/pkg/infra/provisioning/provider_test.go | Adds coverage for layer absolute path resolution and hook validation rules. |
| cli/azd/pkg/ext/hooks_config.go | Introduces shared YAML marshal/unmarshal compatibility for legacy single-hook vs multi-hook formats. |
| cli/azd/pkg/ext/hooks_config_test.go | Adds unit tests for the new ext.HooksConfig marshal/unmarshal behavior. |
| cli/azd/internal/cmd/provision.go | Executes per-layer hooks during provisioning and passes layer metadata into lifecycle args. |
| cli/azd/cmd/testdata/TestUsage-azd-hooks.snap | Updates usage snapshot text for hooks command. |
| cli/azd/cmd/testdata/TestUsage-azd-hooks-run.snap | Updates usage snapshot text and includes --layer flag. |
| cli/azd/cmd/testdata/TestFigSpec.ts | Updates Fig spec snapshots for updated descriptions and --layer option. |
| cli/azd/cmd/hooks.go | Adds --layer flag, layer hook execution, and layer hook validation aggregation for azd hooks run. |
| cli/azd/cmd/hooks_test.go | Adds unit tests verifying layer hooks run and are filterable via --layer. |
Switch the hooks sample to a lightweight App Service app, verify deploy via command and prerestore hooks, and keep `azd hooks run` coverage in local-only subtests so the smoke stays fast. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
52a5153 to
cf7c36a
Compare
cf7c36a to
be00383
Compare
jongio
left a comment
There was a problem hiding this comment.
PR Review - #7382
feat: support hooks per provisioning layer by @weikanglim
Summary
What: Adds first-class hook support (preprovision/postprovision) scoped to individual provisioning layers in azure.yaml. Extends azd hooks run with a --layer flag, updates both alpha and v1.0 schemas, and moves the shared HooksConfig YAML compat logic from project to ext for reuse.
Why: Fixes #7186 - users with multi-layer provisioning setups need per-layer hooks to run setup/teardown scripts specific to each infrastructure layer.
Assessment: Solid implementation that follows existing patterns well. The layer hooks integration in provision.go correctly nests layer hooks around project hooks. The type alias migration is clean and backward-compatible. Test coverage is thorough with unit, snapshot, and functional tests.
Findings
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic | 0 | 0 | 1 | 0 |
| Error Handling | 0 | 0 | 1 | 0 |
| Refactoring | 0 | 0 | 0 | 1 |
| Total | 0 | 0 | 2 | 1 |
Test Coverage Estimate
- Well covered: Layer hook execution, filtering, telemetry, flag mutual exclusion, YAML marshal/unmarshal, path resolution, validation rules
- Likely missing: Error paths for
--layerwith no layers defined, successful post-hook ordering
What's Done Well
- Clean separation of HooksConfig into
extpackage with backward-compatible type aliases - Thorough validation: duplicate layer names, only preprovision/postprovision allowed, root hooks rejected
- Good functional tests with trace-file verification of execution order
3 inline comments below.
be00383 to
89878e6
Compare
jongio
left a comment
There was a problem hiding this comment.
Previous comments addressed. One new item from the refactoring:
- provider.go:191 - layer hook validation error isn't wrapped with
errWrap, inconsistent with all other layer errors - provider.go:142 - dead
len(o.Hooks) > 0check inanyIncompatibleFieldsSet(early return at line 136 catches it first)
Per-scope validation fix looks solid - good separation of cwd per hook scope.
Validate hooks using scope-specific working directories so relative file hooks are resolved correctly for services and layers, remove the redundant layer hook conversion during provision, and clean up provisioning validation error wrapping so root and layer errors are wrapped consistently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
89878e6 to
e8a1d72
Compare
wbreza
left a comment
There was a problem hiding this comment.
PR Review — #7382
feat: support hooks per provisioning layer by @weikanglim
Summary
What: Adds first-class hook support (preprovision/postprovision) scoped to individual provisioning layers. Extends azd hooks run with --layer flag. Moves shared HooksConfig to ext package. Comprehensive test coverage.
Why: Fixes #7186 — multi-layer provisioning setups need per-layer hooks for setup/teardown scripts.
Assessment: Clean implementation with thorough coverage. One significant architectural concern: the hook nesting order differs from the established deploy-hooks model and will surprise users.
Prior Review Verification
@jongio's resolved findings (3 of 5) verified as fixed. 2 remain open and agreed (missing errWrap, dead check).
Findings
| Priority | Count |
|---|---|
| High | 1 |
| Total | 1 new + 2 open from jongio |
What's Done Well
- Clean separation of
HooksConfigintoextpackage with backward-compatible type alias - Thorough validation: duplicate layers, only preprovision/postprovision allowed, root hooks rejected
- Per-scope hook validation (jongio fix) with deduplication via
warningKeysis architecturally correct - Comprehensive tests: unit, snapshot, and functional with trace-file verification
--layer/--servicemutual exclusion with properErrorWithSuggestion
Review performed with GitHub Copilot CLI
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.
Previous comments addressed. Looks good now.
- Add 1.24.0 (Unreleased) CHANGELOG section documenting: - --non-interactive flag alias and AZD_NON_INTERACTIVE env var (Azure#7392) - Hooks per provisioning layer in azure.yaml (Azure#7382) - Docker build network option in azure.yaml (Azure#7361) - Improved azd update error message for admin-managed installs (Azure#7417) - Deprecation of -e short flag in AI extensions (Azure#7313) - Add AZD_NON_INTERACTIVE to environment-variables.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add 'update' command as Beta to feature-stages.md (PR Azure#7489) - Update azd-update.md design doc status to Beta, remove alpha toggle section - Add new layer-hooks.md documenting per-provisioning-layer hooks (PR Azure#7382) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Fixes #7186 by adding first-class hook support for individual provisioning layers and wiring that support through the CLI, schema, and tests.
High-level changes:
infra.layers[].hookssupport and execute layer hooks duringazd provisionazd hooks runwith layer discovery and filtering via--layerazure.yamlschemasext.HooksConfigand reuse shared layer path resolutionprovisionandhooks run, including a faster hooks sample for the deploy smokeValidation
go test ./test/functional -run 'Test_CLI_Hooks(_Run)?_RegistrationAndRun' -count=1Fixes #7186