Rename client-side "preflight" to "provision validation" (#7113)#8844
Rename client-side "preflight" to "provision validation" (#7113)#8844vhvb1989 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Renames azd’s local client-side “preflight” provisioning checks to “provision validation”, splits the existing config gate so local validation and ARM ValidatePreflight can be disabled independently, and updates related telemetry/docs/tests to reduce terminology confusion.
Changes:
- Split the previous
provision.preflightgate intovalidation.provision(local checks) vsprovision.preflight(server-side ARM ValidatePreflight), and update the Bicep provider flow accordingly. - Rename telemetry event/fields from
validation.preflight*tovalidation.provision*and outcomes fromaborted_by_*tocanceled_by_*, updating metrics-audit docs and public telemetry reference. - Rename/refresh UX report + snapshots and functional/unit tests to match the new “provision validation” terminology.
Reviewed changes
Copilot reviewed 34 out of 46 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/specs/metrics-audit/telemetry-schema.md | Renames the validation event/fields and updates outcomes in the schema doc. |
| docs/specs/metrics-audit/feature-telemetry-matrix.md | Updates provision command telemetry inventory to the new event/field names. |
| docs/specs/exegraph/spec.md | Updates terminology in exegraph spec (“validation-cancel” wording). |
| docs/reference/telemetry-data.md | Updates public telemetry reference for the renamed validation event/fields/outcomes. |
| docs/concepts/glossary.md | Renames glossary entry to “Provision Validation” and updates the disable config key. |
| docs/architecture/system-overview.md | Updates high-level provisioning pipeline terminology. |
| docs/architecture/provisioning-pipeline.md | Updates pipeline stage naming and documents the new config split. |
| cli/azd/test/functional/testdata/samples/ai-quota/sub-deployment/infra/main.bicep | Updates sample comment to “validation” terminology. |
| cli/azd/test/functional/testdata/samples/ai-quota/rg-deployment/infra/main.bicep | Updates sample comment to “validation” terminology. |
| cli/azd/test/functional/testdata/samples/ai-quota/README.md | Updates sample README references and test filename reference. |
| cli/azd/test/functional/provision_validation_quota_test.go | Renames and updates functional tests to “provision validation” naming and stdin helpers. |
| cli/azd/resources/config_options.yaml | Documents the new validation.provision config option and clarifies provision.preflight. |
| cli/azd/pkg/output/ux/testdata/TestProvisionValidationReport_Snapshot_RoleAssignmentMissing.snap | New/updated snapshot for renamed UX report output. |
| cli/azd/pkg/output/ux/testdata/TestProvisionValidationReport_Snapshot_RoleAssignmentConditional.snap | New/updated snapshot for renamed UX report output. |
| cli/azd/pkg/output/ux/testdata/TestProvisionValidationReport_Snapshot_ReservedResourceName.snap | New/updated snapshot for renamed UX report output. |
| cli/azd/pkg/output/ux/testdata/TestProvisionValidationReport_Snapshot_AllWarningsCombined.snap | New/updated snapshot for renamed UX report output. |
| cli/azd/pkg/output/ux/testdata/TestProvisionValidationReport_Snapshot_AiModelQuotaExceeded.snap | New/updated snapshot for renamed UX report output. |
| cli/azd/pkg/output/ux/testdata/TestProvisionValidationReport_Snapshot_AiModelNotFound.snap | New/updated snapshot for renamed UX report output. |
| cli/azd/pkg/output/ux/provision_validation_report.go | Renames preflight report UX types to provision validation report equivalents. |
| cli/azd/pkg/output/ux/provision_validation_report_test.go | Updates tests and snapshots for the renamed UX report. |
| cli/azd/pkg/infra/provisioning/provider.go | Lowercases DeploymentStateSkipped value and renames the “aborted” skip reason. |
| cli/azd/pkg/infra/provisioning/manager.go | Updates skipped-reason handling to the new “validation canceled” constant. |
| cli/azd/pkg/infra/provisioning/bicep/role_assignment_check_test.go | Renames test helpers/types to provision validation equivalents. |
| cli/azd/pkg/infra/provisioning/bicep/provision_validation.go | Renames and refactors local validation pipeline types and temp file naming. |
| cli/azd/pkg/infra/provisioning/bicep/provision_validation_test.go | Updates unit tests for the renamed validation pipeline. |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go | Splits config gates, renames telemetry + outcomes, and separates local vs ARM preflight steps. |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go | Updates tests to assert renamed telemetry keys/outcomes and method names. |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider_reserved_names_test.go | Updates severity enum name in reserved-names validation tests. |
| cli/azd/pkg/infra/provisioning/bicep/bicep_provider_coverage_test.go | Updates coverage tests for renamed constructor/types. |
| cli/azd/magefile.go | Updates excluded playback test names to new provision validation test names. |
| cli/azd/internal/tracing/fields/fields.go | Renames tracing attribute keys for provision validation. |
| cli/azd/internal/tracing/events/events.go | Renames validation event constant to validation.provision. |
| cli/azd/internal/errors.go | Updates comment to reflect provision validation decline behavior. |
| cli/azd/internal/cmd/provision_test.go | Renames regression test and updates skipped reason constant. |
| cli/azd/internal/cmd/provision_graph.go | Updates the graph path sentinel/error translation for validation-canceled behavior. |
| cli/azd/docs/environment-variables.md | Updates AZD_DEPLOYMENT_ID_FILE docs to reference “canceled by provision validation”. |
| cli/azd/docs/design/provision-validation.md | Renames and updates design doc for the new feature naming and config split. |
| cli/azd/cmd/middleware/error_test.go | Updates test text for wrapped ErrAbortedByUser case. |
| cli/azd/CHANGELOG.md | Adds breaking change entry describing rename + config split + telemetry rename. |
| cli/azd/.vscode/cspell.yaml | Updates cspell override path to the renamed Go file. |
Comments suppressed due to low confidence (2)
cli/azd/docs/design/provision-validation.md:104
- azd-code-reviewer: This section documents check return values as
*ProvisionValidationCheckResult, but the implementation returns a slice ([]ProvisionValidationCheckResult). The docs should match the actual API so new checks are written correctly.
cli/azd/docs/design/provision-validation.md:114 - azd-code-reviewer: The "Adding a New Check" snippet uses
validator.AddCheck(func(...) (*ProvisionValidationCheckResult, error) { ... }), but the code registers checks viaProvisionValidationCheck{RuleID, Fn}whereFnreturns([]ProvisionValidationCheckResult, error). The snippet should be updated to prevent copy/paste errors.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 48 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
cli/azd/pkg/output/ux/provision_validation_report.go:112
- azd-code-reviewer: The JSON envelope message here still says "validation: ..." even though the feature is now consistently called "provision validation" elsewhere. This string can show up in JSON output/logging, so keeping the label consistent makes it easier to interpret and search.
cli/azd/docs/design/provision-validation.md:200 - azd-code-reviewer: This design doc describes check functions returning
*ProvisionValidationCheckResult, but the implementation now returns[]ProvisionValidationCheckResult(a slice) from each check function. Updating the type in the doc keeps the design reference accurate for anyone adding new validation rules.
jongio
left a comment
There was a problem hiding this comment.
The rename is well-structured and the config gate split into two independent keys (validation.provision vs provision.preflight) is a solid design choice. The function decomposition into traceLocalProvisionValidation + runLocalProvisionValidation cleanly separates tracing concerns from validation logic.
One issue worth addressing: after this PR, both ErrOperationCancelled and ErrAbortedByUser have the identical message "operation canceled by user". These sentinels serve different semantic roles (the first is for Ctrl+C / explicit cancellation, the second is for the user declining to proceed with exit code 0). Identical messages make log analysis and debugging harder. See inline comment for details.
|
|
||
| // ErrAbortedByUser indicates the user intentionally declined to proceed (e.g. preflight warnings). | ||
| // ErrAbortedByUser indicates the user intentionally declined to proceed (e.g. provision validation warnings). | ||
| // This is not a failure — the CLI should exit with code 0. |
There was a problem hiding this comment.
After this change, both ErrOperationCancelled (line 81) and ErrAbortedByUser (line 85) produce the same .Error() string: "operation canceled by user". They serve different semantic roles:
ErrOperationCancelled: explicit cancellation (e.g., Ctrl+C, user hit cancel in a prompt)ErrAbortedByUser: user intentionally declined to proceed (e.g., answered No to validation warnings, exit code 0)
While errors.Is() works on pointer identity (so no functional breakage), identical messages make debugging and log correlation harder. The test in runner_test.go:77 also asserts require.Equal(t, internal.ErrAbortedByUser.Error(), err.Error()) to verify no wrapping, which would now also match a (hypothetically unwrapped) ErrOperationCancelled.
Consider keeping distinct messages, e.g.:
| // This is not a failure — the CLI should exit with code 0. | |
| // ErrAbortedByUser indicates the user intentionally declined to proceed (e.g. provision validation warnings). | |
| // This is not a failure - the CLI should exit with code 0. | |
| ErrAbortedByUser = errors.New("operation declined by user") |
hemarina
left a comment
There was a problem hiding this comment.
Nice cleanup — disambiguating "preflight" into three distinct meanings has been a long-standing source of confusion, and splitting the local vs. server-side gates is the right model. The telemetry rename + downstream notifications (#1790/#1792) and the docs sync are thorough.
One thing I'd like to confirm before this ships — the config split has no migration path, and the silent re-enablement is a behavior change with real blast radius:
Today on main, a single provision.preflight off skips the entire validatePreflight block — i.e., both the local client-side checks (newLocalArmPreflight: role-assignment, AI-model quota, reserved-names) and the server-side target.ValidatePreflight call. After this PR, provision.preflight off gates only the ARM call, and the local checks key off the new validation.provision. So any user who set provision.preflight off will have local validation silently re-enabled on upgrade.
That matters because the local checks auto-abort on errors without prompting:
if report.HasErrors() {
p.console.Message(ctx, "... deployment aborted.")
return true, nil // skips deploy, no prompt — fires even under --no-prompt/CI
}So someone who set provision.preflight off to get past a false-positive local error (or in a CI pipeline) can see a previously-green azd provision/azd up start failing after an azd upgrade, until they discover and set validation.provision off. The CHANGELOG flags this as breaking, but the failure mode is silent — the old key just quietly stops affecting local validation.
Could we soften this with a back-compat shim — e.g., if validation.provision is unset and provision.preflight=off, also skip local validation, emitting a one-time deprecation notice pointing at the new key? That preserves existing users' intent through the rename. If the silent re-enablement is intentional (e.g., we want those users back on local validation), that's a defensible call — just want to confirm it's deliberate rather than an artifact of the gate split.
Rename azd's client-side pre-deployment check feature from "preflight" to "provision validation" to avoid confusion with the Azure ARM Preflight API. - Split the single gate into two config keys: provision.preflight now controls ONLY the server-side ARM preflight call; new validation.provision controls azd's local (client-side) validation. - Rename internal symbols (PreflightCheck* -> ValidationCheck*, localArmPreflight -> provisionValidator, validatePreflight -> validateProvision, Preflight*Key -> ProvisionValidation*Key, PreflightReport -> ProvisionValidationReport). - Rename telemetry event validation.preflight -> validation.provision and fields validation.preflight.* -> validation.provision.*; outcome values aborted_by_* -> canceled_by_*. - Rename extension validation check_type "local-preflight" -> "provision". - Replace "abort" UI terminology with "canceled"; lowercase DeploymentStateSkipped. - Rename files, tests, recordings, design doc, and update telemetry/audit docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
7877350 to
1bf6a0a
Compare
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
jongio
left a comment
There was a problem hiding this comment.
Re-reviewed after rebase. The rename is consistent, the config gate split into �alidation.provision (local) and provision.preflight (ARM) is well-designed, and the function decomposition into raceLocalProvisionValidation +
unLocalProvisionValidation cleanly separates concerns.
My previous comment about ErrOperationCancelled and ErrAbortedByUser having identical messages still applies after this rebase. See inline.
| // ErrAbortedByUser indicates the user intentionally declined to proceed (e.g. provision validation warnings). | ||
| // This is not a failure — the CLI should exit with code 0. | ||
| ErrAbortedByUser = errors.New("operation aborted by user") | ||
| ErrAbortedByUser = errors.New("operation canceled by user") |
There was a problem hiding this comment.
Still unresolved from my earlier review: after this PR, both ErrOperationCancelled (line 81) and ErrAbortedByUser (line 85) resolve to the identical string "operation canceled by user".
These sentinels have different semantics:
ErrOperationCancelled: Ctrl+C or explicit cancellation (non-zero exit)ErrAbortedByUser: user declined to proceed at a prompt (exit 0)
errors.Is() still works (different pointers), but identical messages make log grep, debugging, and user-facing output ambiguous. Previously they were distinguishable ("cancelled" vs "aborted").
Consider giving ErrAbortedByUser a distinct message, e.g. "operation declined by user" or "user chose not to proceed", so the two remain distinguishable in logs and output without needing to inspect the call stack.
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Fixes #7113
Summary
Renames azd's client-side "local preflight" provisioning check to "provision validation" to avoid confusion with the Azure ARM Preflight API. The term "preflight" was overloaded across three unrelated meanings; this PR renames only the local client-side feature and leaves the server-side ARM preflight call and the
mage preflightdev checks untouched.Behavioral change — config gate split
The single
provision.preflightgate is split into two independent keys:provision.preflight offtarget.ValidatePreflight)validation.provision off(new)Issue sub-tasks addressed
DeploymentStateSkippedvalue to"deployment state"aborted_by_*→canceled_by_*)Telemetry rename
validation.preflightvalidation.provisionvalidation.preflight.*fieldsvalidation.provision.*aborted_by_user/aborted_by_errorscanceled_by_user/canceled_by_errorsTelemetry docs (
telemetry-data.md, metrics-audit schema + matrix), the design doc, architecture docs, config option descriptions, and CHANGELOG are all updated in sync. Downstream analytics issues (Azure/azure-dev-pr#1790, Azure/azure-dev-pr#1792) have been notified with the field mapping and forward-compatible KQL.Renamed files
bicep/local_preflight.go→bicep/provision_validation.go(+ test)output/ux/preflight_report.go→output/ux/provision_validation_report.go(+ test, + 6 snapshots)test/functional/preflight_quota_test.go→provision_validation_quota_test.go(+ 6 recordings)docs/design/local-preflight-validation.md→docs/design/provision-validation.mdValidation
go build ./..., targeted unit tests, andcmdfig/usage snapshot tests passgolangci-lint run— 0 issuescspell,gofmt -s, copyright-check — cleango.mod/go.sumchanges