Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
3 changes: 3 additions & 0 deletions cli/azd/.vscode/cspell.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,9 @@ overrides:
- filename: extensions/azure.ai.models/internal/cmd/custom_create.go
words:
- Qwen
- filename: pkg/infra/provisioning/bicep/local_preflight.go
words:
- actioned
ignorePaths:
- "**/*_test.go"
- "**/mock*.go"
Expand Down
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
101 changes: 92 additions & 9 deletions cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/cognitiveservices/armcognitiveservices"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
"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"
"github.com/azure/azure-dev/cli/azd/pkg/account"
"github.com/azure/azure-dev/cli/azd/pkg/ai"
"github.com/azure/azure-dev/cli/azd/pkg/async"
Comment thread
vhvb1989 marked this conversation as resolved.
Expand Down Expand Up @@ -2133,6 +2136,16 @@ 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"
preflightOutcomeSkipped = "skipped"
preflightOutcomeError = "error"
)
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 @@ -2146,7 +2159,12 @@ func (p *BicepProvider) validatePreflight(
armParameters azure.ArmParameters,
tags map[string]*string,
options map[string]any,
) (bool, error) {
) (abort bool, err error) {
ctx, span := tracing.Start(ctx, events.PreflightValidationEvent)
defer func() {
span.EndWithStatus(err)
}()

// 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 +2173,54 @@ 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,
})

results, err := localPreflight.validate(ctx, p.console, armTemplate, armParameters)
if err != nil {
p.setPreflightOutcome(span, preflightOutcomeError, nil)
return false, fmt.Errorf("local preflight validation failed: %w", err)
}

// Record the rule IDs that actually executed. This is done after validate()
// because validate() may skip checks entirely (e.g. when the bicep snapshot
// is unavailable). A nil results with nil error means checks were skipped.
Comment thread
vhvb1989 marked this conversation as resolved.
Outdated
if results == nil {
p.setPreflightOutcome(span, preflightOutcomeSkipped, nil)
}
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

// 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,28 +2230,57 @@ 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
}

if report.HasWarnings() {
continueDeployment, promptErr := p.console.Confirm(ctx, input.ConsoleOptions{
Message: "Preflight validation found warnings that may cause the deployment to fail. " +
"Do you want to continue?",
Message: "Preflight validation found warnings that may cause the " +
"deployment to fail. Do you want to continue?",
DefaultValue: true,
})
if promptErr != nil {
return false, fmt.Errorf("prompting for preflight confirmation: %w", promptErr)
p.setPreflightOutcome(
span, preflightOutcomeError, diagnosticIDs,
)
return false, fmt.Errorf(
"prompting for preflight confirmation: %w", promptErr,
)
}
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 if results != nil {
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 +2332,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 +2349,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
95 changes: 95 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 @@ -22,6 +22,7 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/appconfiguration/armappconfiguration"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/keyvault/armkeyvault"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources"
"github.com/azure/azure-dev/cli/azd/internal/tracing"
"github.com/azure/azure-dev/cli/azd/pkg/account"
"github.com/azure/azure-dev/cli/azd/pkg/azapi"
"github.com/azure/azure-dev/cli/azd/pkg/azure"
Expand All @@ -38,9 +39,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 +1913,95 @@ 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())

// Verify usage-level attributes are set for parent command span correlation.
usageAttrs := tracing.GetUsageAttributes()
usageOutcome := findSpanAttribute(usageAttrs, "validation.preflight.outcome")
require.NotNil(t, usageOutcome, "expected outcome in usage attributes")
require.Equal(t, preflightOutcomeWarningsAccepted, usageOutcome.Value.AsString())

usageDiag := findSpanAttribute(
usageAttrs, "validation.preflight.diagnostics",
)
require.NotNil(t, usageDiag, "expected diagnostics in usage attributes")
require.Equal(t, diagnosticIDs, usageDiag.Value.AsStringSlice())
}

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"},
},
{
name: "skipped",
outcome: preflightOutcomeSkipped,
diagnosticIDs: nil,
},
{
name: "error",
outcome: preflightOutcomeError,
diagnosticIDs: nil,
},
}

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
}
Loading
Loading