more comments, cosmetic changes

Author: dsa0x

internal/backend/local/backend_apply.go

@@ -97,6 +97,7 @@ func (b *Local) opApply(
 	// If we weren't given a plan, then we refresh/plan
 	if op.PlanFile == nil {
 		// set the policy client to nil for the plan preceding apply
+		// so that policy evaluation is skipped during the plan.
 		lr.PlanOpts.PolicyClient = nil
 		combinedPlanApply = true
 		// Perform the plan
@@ -114,8 +115,6 @@ func (b *Local) opApply(
 			if plan != nil && (len(plan.Changes.Resources) != 0 || len(plan.Changes.Outputs) != 0) {
 				op.View.Plan(plan, schemas)
 			}
-			// Report all policy results that may have accumulated during the plan
-			op.View.PolicyResults(plan.PolicyResults)
 			op.ReportResult(runningOp, diags)
 			return
 		}
@@ -427,9 +426,6 @@ func (b *Local) opApply(
 	var applyState *states.State
 	var applyDiags tfdiags.Diagnostics
 
-	// We use a new store for the apply policy results, as objects that failed during the plan policy
-	// evaluation may have updated data which yields a different policy evaluation result.
-	policyResults := plans.NewPolicyResults()
 	doneCh := make(chan struct{})
 	go func() {
 		defer logging.PanicHandler()
@@ -438,9 +434,9 @@ func (b *Local) opApply(
 		log.Printf("[INFO] backend/local: apply calling Apply")
 		applyState, applyDiags = lr.Core.Apply(plan, lr.Config, &terraform.ApplyOpts{
 			SetVariables:  applyTimeValues,
-			Locks:         providerLocksSnapshot(op.DependencyLocks),
+			ProviderLocks: providerLocksSnapshot(op.DependencyLocks),
 			PolicyClient:  lr.PolicyClient,
-			PolicyResults: policyResults,
+			PolicyResults: plan.PolicyResults,
 		})
 	}()
 
@@ -450,7 +446,7 @@ func (b *Local) opApply(
 	diags = diags.Append(applyDiags)
 
 	// Print the policy results we found during apply
-	op.View.PolicyResults(policyResults)
+	op.View.PolicyResults(plan.PolicyResults)
 
 	// Even on error with an empty state, the state value should not be nil.
 	// Return early here to prevent corrupting any existing state.

internal/backend/local/backend_local.go

@@ -188,7 +188,7 @@ func (b *Local) localRunDirect(op *backendrun.Operation, run *backendrun.LocalRu
 		GenerateConfigPath: op.GenerateConfigOut,
 		DeferralAllowed:    op.DeferralAllowed,
 		Query:              op.Query,
-		Locks:              providerLocksSnapshot(op.DependencyLocks),
+		ProviderLocks:      providerLocksSnapshot(op.DependencyLocks),
 		PolicyClient:       run.PolicyClient,
 	}
 	run.PlanOpts = planOpts
@@ -213,14 +213,14 @@ func (b *Local) localRunDirect(op *backendrun.Operation, run *backendrun.LocalRu
 	)
 	diags = diags.Append(buildDiags)
 	if buildDiags.HasErrors() {
-		return nil, nil, diags
+		return run, nil, diags
 	}
 	run.Config = config
 
 	snapDiags := op.ConfigLoader.AddRootModuleToSnapshot(configSnap, op.ConfigDir)
 	diags = diags.Append(snapDiags)
 	if snapDiags.HasErrors() {
-		return nil, nil, diags
+		return run, nil, diags
 	}
 
 	if errs := config.VerifyDependencySelections(op.DependencyLocks); len(errs) > 0 {

internal/command/apply.go

@@ -109,7 +109,7 @@ func (c *ApplyCommand) Run(rawArgs []string) int {
 	if len(c.policyPaths) > 0 {
 		var policyDiags policy.Diagnostics
 		opReq.PolicyClient, policyDiags = c.PolicyClient(context.Background(), c.policyPaths)
-		// if there has been any errors when setting up the policy client, we'll want to log them
+		// if there has been any errors when setting up the policy client, we'll log them
 		if opReq.View != nil && policyDiags != nil {
 			opReq.View.PolicyResults(&plans.PolicyResults{Diagnostics: policyDiags})
 		}

internal/command/arguments/query.go

@@ -21,10 +21,6 @@ type Query struct {
 	// the found resources in the query and which path the generated file should
 	// be written to.
 	GenerateConfigPath string
-
-	// PolicyPath contains an optional path to any defined policies that should
-	// be applied for this plan operation.
-	PolicyPaths []string
 }
 
 func ParseQuery(args []string) (*Query, tfdiags.Diagnostics) {
@@ -44,7 +40,6 @@ func ParseQuery(args []string) (*Query, tfdiags.Diagnostics) {
 	query.Vars.varFiles = &varFilesFlags
 	cmdFlags.Var(query.Vars.vars, "var", "var")
 	cmdFlags.Var(query.Vars.varFiles, "var-file", "var-file")
-	cmdFlags.Var((*FlagStringSlice)(&query.PolicyPaths), "policies", "policies")
 
 	var json bool
 	cmdFlags.BoolVar(&json, "json", false, "json")

internal/command/format/diagnostic.go

@@ -230,6 +230,8 @@ func (f *snippetFormatter) write() {
 	diag := f.diag
 	buf := f.buf
 
+	// if the diagnostic has a policy range, then it contains policy-specific information
+	// and we will write that first.
 	snippetPrefix := "  on"
 	if diag.PolicyRange != nil {
 

internal/command/meta_policy.go

@@ -35,7 +35,7 @@ func (c *Meta) PolicyClient(ctx context.Context, policyPaths []string) (policy.C
 
 	var diags policy.Diagnostics
 	client, err := policy.Connect(ctx)
-	if client == nil {
+	if err != nil {
 		diags = append(diags, policy.NewErrorDiagnostic(
 			"Failed to connect to policy engine",
 			fmt.Sprintf("Failed to connect to policy engine: %s.", err),
@@ -50,6 +50,7 @@ func (c *Meta) PolicyClient(ctx context.Context, policyPaths []string) (policy.C
 	if srv, ok := client.(policy.CallbackService); ok {
 		callbackServer, cbDiags := srv.RegisterCallbackService(ctx)
 		if cbDiags != nil {
+			client.Stop()
 			return nil, cbDiags
 		}
 		callbackServiceID = callbackServer.ID
@@ -65,6 +66,7 @@ func (c *Meta) PolicyClient(ctx context.Context, policyPaths []string) (policy.C
 	for _, config := range resp.ServerConfigurations() {
 		version, err := constraints.ParseRubyStyleMulti(config.RequiredVersion)
 		if err != nil {
+			client.Stop()
 			diags = append(diags, policy.NewErrorDiagnostic(
 				"Failed to validate required Terraform version",
 				fmt.Sprintf("The policy file %s had a Terraform version constraint that could not be parsed: %s.", config.File, err),

internal/command/query.go

@@ -118,7 +118,7 @@ func (c *QueryCommand) Run(rawArgs []string) int {
 	}
 
 	// Build the operation request
-	opReq, opDiags := c.OperationRequest(be, view, args.ViewType, args.GenerateConfigPath, args.PolicyPaths)
+	opReq, opDiags := c.OperationRequest(be, view, args.ViewType, args.GenerateConfigPath)
 	diags = diags.Append(opDiags)
 	if diags.HasErrors() {
 		view.Diagnostics(diags)
@@ -164,7 +164,6 @@ func (c *QueryCommand) OperationRequest(
 	view views.Query,
 	viewType arguments.ViewType,
 	generateConfigOut string,
-	policyPaths []string,
 ) (*backendrun.Operation, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
@@ -176,16 +175,6 @@ func (c *QueryCommand) OperationRequest(
 	opReq.GenerateConfigOut = generateConfigOut
 	opReq.View = view.Operation()
 	opReq.Query = true
-	opReq.PolicyPaths = policyPaths
-
-	if !c.AllowExperimentalFeatures && len(policyPaths) > 0 {
-		diags = diags.Append(tfdiags.Sourceless(
-			tfdiags.Error,
-			"Failed to parse command-line flags",
-			"The -policies flag is only valid in experimental builds of Terraform.",
-		))
-		return nil, diags
-	}
 
 	var err error
 	opReq.ConfigLoader, err = c.initConfigLoader()

internal/command/views/json/policy.go

@@ -20,6 +20,7 @@ type PolicyInfo struct {
 	Snippet *DiagnosticSnippet `json:"snippet,omitempty"`
 }
 
+// PolicyMetadata contains policy-specific metadata about a diagnostic.
 type PolicyMetadata struct {
 	PolicySetName    string `json:"policy_set_name,omitempty"`
 	PolicySetPath    string `json:"policy_set_path,omitempty"`
@@ -29,6 +30,7 @@ type PolicyMetadata struct {
 	EnforceIndex     *int32 `json:"enforce_index,omitempty"`
 }
 
+// EnforceMetadata contains metadata about the enforcement block which the diagnostic is associated with.
 type EnforceMetadata struct {
 	BlockIndex *int32 `json:"block_index,omitempty"`
 }

internal/command/views/view.go

@@ -158,31 +158,34 @@ func (v *View) PolicyResults(results *plans.PolicyResults) {
 				foundInfo = true
 				buf.WriteString("Policy Info:\n")
 				if info.PolicyRange != nil && info.PolicySnippet != nil {
-					buf.WriteString(fmt.Sprintf(
+					fmt.Fprintf(
+						&buf,
 						"on %s line %d, in %s\n",
 						info.PolicyRange.Filename,
 						info.PolicyRange.Start.Line,
 						info.PolicySnippet.Code,
-					))
+					)
 				} else if enforcement.Policy != nil {
-					buf.WriteString(fmt.Sprintf(
+					fmt.Fprintf(
+						&buf,
 						"in policy %s\n",
 						enforcement.Policy.Address,
-					))
+					)
 				}
-				buf.WriteString(fmt.Sprintf("%q\n", info.Message))
+				fmt.Fprintf(&buf, "%q\n", info.Message)
 
 				if !result.ConfigDeclRange.Empty() {
 					cfgRange := result.ConfigDeclRange
 					resourceContext := string(cfgRange.SliceBytes(configSources[cfgRange.Filename]))
 
 					// Here we want the resource source context
-					buf.WriteString(fmt.Sprintf(
+					fmt.Fprintf(
+						&buf,
 						"\non %s line %d, in %s\n",
 						cfgRange.Filename,
 						cfgRange.Start.Line,
 						resourceContext,
-					))
+					)
 				}
 				buf.WriteString("\n")
 			}

internal/terraform/context_apply.go

@@ -49,9 +49,9 @@ type ApplyOpts struct {
 	// the actual root modules.
 	AllowRootEphemeralOutputs bool
 
-	// Locks is a read-only snapshot of provider locks (from the dependency lock
+	// ProviderLocks is a read-only snapshot of provider locks (from the dependency lock
 	// file). This is required by policy evaluations against providers to access version information.
-	Locks map[addrs.Provider]*depsfile.ProviderLock
+	ProviderLocks map[addrs.Provider]*depsfile.ProviderLock
 
 	// Optional policy client.
 	// When set, policy evaluation logic will be executed in the graph.
@@ -73,7 +73,7 @@ func (po *PlanOpts) ApplyOpts() *ApplyOpts {
 	return &ApplyOpts{
 		ExternalProviders:         po.ExternalProviders,
 		AllowRootEphemeralOutputs: po.AllowRootEphemeralOutputs,
-		Locks:                     po.Locks,
+		ProviderLocks:             po.ProviderLocks,
 	}
 }
 
@@ -221,7 +221,7 @@ func (c *Context) ApplyAndEval(plan *plans.Plan, config *configs.Config, opts *A
 
 		FunctionResults: lang.NewFunctionResultsTable(plan.FunctionResults),
 
-		Locks:         opts.Locks,
+		ProviderLocks: opts.ProviderLocks,
 		PolicyClient:  opts.PolicyClient,
 		PolicyResults: opts.PolicyResults,
 	})