Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions cli/azd/internal/tracing/events/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,10 @@ const (
// CopilotSessionEvent tracks session creation or resumption.
CopilotSessionEvent = "copilot.session"
)

// Preflight validation events.
const (
// PreflightValidationEvent tracks the local preflight validation operation
// and its outcome (passed, warnings accepted, aborted).
PreflightValidationEvent = "validation.preflight"
)
46 changes: 46 additions & 0 deletions cli/azd/internal/tracing/fields/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,52 @@ var (
}
)

// Preflight validation related fields
var (
// PreflightOutcomeKey records the outcome of preflight validation.
//
// Example: "passed", "warnings_accepted", "aborted_by_errors", "aborted_by_user"
PreflightOutcomeKey = AttributeKey{
Key: attribute.Key("validation.preflight.outcome"),
Classification: SystemMetadata,
Purpose: FeatureInsight,
}
Comment thread
vhvb1989 marked this conversation as resolved.

// PreflightDiagnosticsKey records the list of diagnostic IDs emitted by preflight checks.
//
// Example: ["role_assignment_missing", "role_assignment_conditional"]
PreflightDiagnosticsKey = AttributeKey{
Key: attribute.Key("validation.preflight.diagnostics"),
Classification: SystemMetadata,
Purpose: FeatureInsight,
}

// PreflightRulesKey records the list of rule IDs that were executed.
//
// Example: ["role_assignment_permissions"]
PreflightRulesKey = AttributeKey{
Key: attribute.Key("validation.preflight.rules"),
Classification: SystemMetadata,
Purpose: FeatureInsight,
}

// PreflightWarningCountKey records the number of warnings produced by preflight validation.
PreflightWarningCountKey = AttributeKey{
Key: attribute.Key("validation.preflight.warning.count"),
Classification: SystemMetadata,
Purpose: FeatureInsight,
IsMeasurement: true,
}

// PreflightErrorCountKey records the number of errors produced by preflight validation.
PreflightErrorCountKey = AttributeKey{
Key: attribute.Key("validation.preflight.error.count"),
Classification: SystemMetadata,
Purpose: FeatureInsight,
IsMeasurement: true,
}
)

// The value used for ServiceNameKey
const ServiceNameAzd = "azd"

Expand Down
79 changes: 74 additions & 5 deletions cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ import (
"github.com/azure/azure-dev/cli/azd/pkg/prompt"
"github.com/azure/azure-dev/cli/azd/pkg/tools/bicep"
"github.com/drone/envsubst"

"github.com/azure/azure-dev/cli/azd/internal/tracing"
"github.com/azure/azure-dev/cli/azd/internal/tracing/events"
"github.com/azure/azure-dev/cli/azd/internal/tracing/fields"
Comment thread
vhvb1989 marked this conversation as resolved.
Outdated
)

type bicepFileMode int
Expand Down Expand Up @@ -2133,6 +2137,14 @@ func (p *BicepProvider) convertToDeployment(bicepTemplate azure.ArmTemplate) pro
return result
}

// Preflight validation outcome constants for telemetry.
const (
preflightOutcomePassed = "passed"
preflightOutcomeWarningsAccepted = "warnings_accepted"
preflightOutcomeAbortedByErrors = "aborted_by_errors"
preflightOutcomeAbortedByUser = "aborted_by_user"
)
Comment thread
vhvb1989 marked this conversation as resolved.

// validatePreflight runs client-side preflight validation on the ARM template.
// It returns (abort, err) where:
// - abort=true, err=nil: checks detected issues and the deployment should be skipped (exit code 0).
Expand All @@ -2147,6 +2159,9 @@ func (p *BicepProvider) validatePreflight(
tags map[string]*string,
options map[string]any,
) (bool, error) {
ctx, span := tracing.Start(ctx, events.PreflightValidationEvent)
defer span.End()
Comment thread
vhvb1989 marked this conversation as resolved.
Outdated

// Run local preflight validation before sending to Azure.
// Local validation catches common issues without requiring a network round-trip.
localPreflight := newLocalArmPreflight(modulePath, p.bicepCli, target)
Expand All @@ -2155,20 +2170,48 @@ func (p *BicepProvider) validatePreflight(
// local preflight pipeline. The check inspects whether the template contains
// Microsoft.Authorization/roleAssignments and, if so, verifies the current
// principal has the required write permission.
localPreflight.AddCheck(p.checkRoleAssignmentPermissions)
localPreflight.AddCheck(PreflightCheck{
RuleID: "role_assignment_permissions",
Fn: p.checkRoleAssignmentPermissions,
})

// Record the rule IDs that will be executed for telemetry.
ruleIDs := make([]string, len(localPreflight.checks))
for i, check := range localPreflight.checks {
ruleIDs[i] = check.RuleID
}
span.SetAttributes(fields.PreflightRulesKey.StringSlice(ruleIDs))
Comment thread
vhvb1989 marked this conversation as resolved.
Outdated

results, err := localPreflight.validate(ctx, p.console, armTemplate, armParameters)
if err != nil {
return false, fmt.Errorf("local preflight validation failed: %w", err)
Comment thread
vhvb1989 marked this conversation as resolved.
}

// Compute telemetry metrics from the results.
var diagnosticIDs []string
var warningCount, errorCount int
for _, result := range results {
if result.DiagnosticID != "" {
diagnosticIDs = append(diagnosticIDs, result.DiagnosticID)
}
if result.Severity == PreflightCheckError {
errorCount++
} else {
warningCount++
}
}
span.SetAttributes(fields.PreflightDiagnosticsKey.StringSlice(diagnosticIDs))
span.SetAttributes(fields.PreflightWarningCountKey.Int(warningCount))
span.SetAttributes(fields.PreflightErrorCountKey.Int(errorCount))

// Build a UX report from the preflight results and display it.
if len(results) > 0 {
report := &ux.PreflightReport{}
for _, result := range results {
report.Items = append(report.Items, ux.PreflightReportItem{
IsError: result.Severity == PreflightCheckError,
Message: result.Message,
IsError: result.Severity == PreflightCheckError,
DiagnosticID: result.DiagnosticID,
Message: result.Message,
})
}
p.console.MessageUxItem(ctx, report)
Expand All @@ -2178,6 +2221,7 @@ func (p *BicepProvider) validatePreflight(
// successfully detected problems and the deployment is intentionally aborted.
// This is not an internal failure, so no error is returned (exit code 0).
p.console.Message(ctx, "Preflight validation detected errors, deployment aborted.")
p.setPreflightOutcome(span, preflightOutcomeAbortedByErrors, diagnosticIDs)
return true, nil
}

Expand All @@ -2192,14 +2236,37 @@ func (p *BicepProvider) validatePreflight(
}
if !continueDeployment {
// User chose not to continue — this is an intentional abort, not a failure.
p.setPreflightOutcome(span, preflightOutcomeAbortedByUser, diagnosticIDs)
return true, nil
}
p.setPreflightOutcome(span, preflightOutcomeWarningsAccepted, diagnosticIDs)
}
} else {
p.setPreflightOutcome(span, preflightOutcomePassed, nil)
}

return false, target.ValidatePreflight(ctx, armTemplate, armParameters, tags, options)
}

// setPreflightOutcome records the preflight outcome on both the span and as a usage-level
// attribute so it can be correlated with the overall deployment result.
func (p *BicepProvider) setPreflightOutcome(
span tracing.Span,
outcome string,
diagnosticIDs []string,
) {
span.SetAttributes(fields.PreflightOutcomeKey.String(outcome))

// Set usage-level attributes so the parent command span (cmd.provision / cmd.up) can
// correlate preflight outcome with the final deployment result. This enables tracking
// false positives (warnings_accepted + deploy succeeds) and true positives
// (warnings_accepted + deploy fails).
tracing.SetUsageAttributes(
fields.PreflightOutcomeKey.String(outcome),
fields.PreflightDiagnosticsKey.StringSlice(diagnosticIDs),
)
}

// checkRoleAssignmentPermissions is a PreflightCheckFn that verifies the current principal
// has Microsoft.Authorization/roleAssignments/write permission when the template contains
// role assignments. The PermissionsService is resolved lazily via the service locator so it
Expand Down Expand Up @@ -2251,7 +2318,8 @@ func (p *BicepProvider) checkRoleAssignmentPermissions(

if !hasPermission.HasPermission {
return &PreflightCheckResult{
Severity: PreflightCheckWarning,
Severity: PreflightCheckWarning,
DiagnosticID: "role_assignment_missing",
Message: fmt.Sprintf(
"the current principal (%s) does not have permission to create role assignments "+
"(Microsoft.Authorization/roleAssignments/write) on subscription %s. "+
Expand All @@ -2267,7 +2335,8 @@ func (p *BicepProvider) checkRoleAssignmentPermissions(

if hasPermission.Conditional {
return &PreflightCheckResult{
Severity: PreflightCheckWarning,
Severity: PreflightCheckWarning,
DiagnosticID: "role_assignment_conditional",
Message: fmt.Sprintf(
"the current principal (%s) has conditional permission to create role "+
"assignments (Microsoft.Authorization/roleAssignments/write) on "+
Expand Down
77 changes: 77 additions & 0 deletions cli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ import (
"github.com/azure/azure-dev/cli/azd/test/mocks/mockaccount"
"github.com/azure/azure-dev/cli/azd/test/mocks/mockazapi"
"github.com/azure/azure-dev/cli/azd/test/mocks/mockenv"
"github.com/azure/azure-dev/cli/azd/test/mocks/mocktracing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/attribute"
)

func TestBicepPlan(t *testing.T) {
Expand Down Expand Up @@ -1910,3 +1912,78 @@ func TestHelperEvalParamEnvSubst(t *testing.T) {
require.Contains(t, substResult.mappedEnvVars, "VAR2")
require.False(t, substResult.hasUnsetEnvVar)
}

func TestSetPreflightOutcome_SetsSpanAndUsageAttributes(t *testing.T) {
span := &mocktracing.Span{}
provider := &BicepProvider{}

diagnosticIDs := []string{"role_assignment_missing", "role_assignment_conditional"}
provider.setPreflightOutcome(span, preflightOutcomeWarningsAccepted, diagnosticIDs)

// Verify outcome is set on the span directly.
outcomeAttr := findSpanAttribute(span.Attributes, "validation.preflight.outcome")
require.NotNil(t, outcomeAttr, "expected outcome attribute on span")
require.Equal(t, preflightOutcomeWarningsAccepted, outcomeAttr.Value.AsString())

// Diagnostics and outcome are also set as usage-level attributes for
// correlation on the parent command span (cmd.provision / cmd.up).
// Usage attributes are stored globally via tracing.SetUsageAttributes,
// not on this span, so we only verify the span-level outcome above.
}
Comment thread
vhvb1989 marked this conversation as resolved.
Outdated

func TestSetPreflightOutcome_AllOutcomeValues(t *testing.T) {
tests := []struct {
name string
outcome string
diagnosticIDs []string
}{
{
name: "passed",
outcome: preflightOutcomePassed,
diagnosticIDs: nil,
},
{
name: "warnings accepted",
outcome: preflightOutcomeWarningsAccepted,
diagnosticIDs: []string{"role_assignment_missing"},
},
{
name: "aborted by errors",
outcome: preflightOutcomeAbortedByErrors,
diagnosticIDs: []string{"role_assignment_missing"},
},
{
name: "aborted by user",
outcome: preflightOutcomeAbortedByUser,
diagnosticIDs: []string{"role_assignment_conditional"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
span := &mocktracing.Span{}
provider := &BicepProvider{}

provider.setPreflightOutcome(span, tt.outcome, tt.diagnosticIDs)

outcomeAttr := findSpanAttribute(
span.Attributes, "validation.preflight.outcome",
)
require.NotNil(t, outcomeAttr)
require.Equal(t, tt.outcome, outcomeAttr.Value.AsString())
})
}
}

// findSpanAttribute searches for an attribute by key and returns a pointer to it, or nil.
func findSpanAttribute(
attrs []attribute.KeyValue,
key attribute.Key,
) *attribute.KeyValue {
for i := range attrs {
if attrs[i].Key == key {
return &attrs[i]
}
}
return nil
}
29 changes: 22 additions & 7 deletions cli/azd/pkg/infra/provisioning/bicep/local_preflight.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,10 @@ const (
type PreflightCheckResult struct {
// Severity indicates whether this result is a warning or a blocking error.
Severity PreflightCheckSeverity
// DiagnosticID is a unique, stable identifier for this specific finding type
// (e.g. "role_assignment_missing"). Used in telemetry to correlate actioned
// warnings with deployment outcomes and to track error frequency over time.
DiagnosticID string
// Message is a human-readable description of the finding.
Message string
}
Expand Down Expand Up @@ -300,6 +304,17 @@ type PreflightCheckFn func(
valCtx *validationContext,
) (*PreflightCheckResult, error)

// PreflightCheck pairs a unique rule identifier with its check function.
// The RuleID is a stable, unique string used in telemetry to identify which rule
// produced a result (e.g. for crash tracking). Each rule may emit results with
// different DiagnosticIDs to distinguish specific finding types.
type PreflightCheck struct {
// RuleID is a unique, stable identifier for the rule (e.g. "role_assignment_permissions").
RuleID string
// Fn is the check function that performs the validation.
Fn PreflightCheckFn
}

// localArmPreflight provides local (client-side) validation of an ARM template before deployment.
// It parses the template and parameters to build a comprehensive view of all resources that would
// be deployed, enabling early detection of issues without making Azure API calls.
Expand All @@ -315,7 +330,7 @@ type localArmPreflight struct {
// target is the deployment scope (subscription or resource group) used to derive snapshot options.
// It may be nil, in which case snapshot options are left empty.
target infra.Deployment
checks []PreflightCheckFn
checks []PreflightCheck
}

// newLocalArmPreflight creates a new instance of localArmPreflight.
Expand All @@ -326,10 +341,10 @@ func newLocalArmPreflight(modulePath string, bicepCli *bicep.Cli, target infra.D
return &localArmPreflight{modulePath: modulePath, bicepCli: bicepCli, target: target}
}

// AddCheck registers a preflight check function to be executed during validate.
// Check functions are invoked in the order they are added.
func (l *localArmPreflight) AddCheck(fn PreflightCheckFn) {
l.checks = append(l.checks, fn)
// AddCheck registers a preflight check to be executed during validate.
// Checks are invoked in the order they are added.
func (l *localArmPreflight) AddCheck(check PreflightCheck) {
l.checks = append(l.checks, check)
}

// validate performs local preflight validation on the given ARM template and parameters.
Expand Down Expand Up @@ -417,9 +432,9 @@ func (l *localArmPreflight) validate(

var results []PreflightCheckResult
for _, check := range l.checks {
result, err := check(ctx, valCtx)
result, err := check.Fn(ctx, valCtx)
if err != nil {
return results, fmt.Errorf("preflight check failed: %w", err)
return results, fmt.Errorf("preflight check %q failed: %w", check.RuleID, err)
}
if result != nil {
results = append(results, *result)
Expand Down
Loading
Loading