stacks: applyable status should be dependent on applyable components (#…

…35350)

internal/rpcapi/stacks_test.go

@@ -293,13 +293,13 @@ func TestStacksPlanStackChanges(t *testing.T) {
 				PlannedChange: &terraform1.PlannedChange{
 					Raw: []*anypb.Any{
 						mustMarshalAnyPb(&tfstackdata1.PlanApplyable{
-							Applyable: true,
+							Applyable: false,
 						}),
 					},
 					Descriptions: []*terraform1.PlannedChange_ChangeDescription{
 						{
 							Description: &terraform1.PlannedChange_ChangeDescription_PlanApplyable{
-								PlanApplyable: true,
+								PlanApplyable: false,
 							},
 						},
 					},

internal/stacks/stackruntime/internal/stackeval/stack.go

@@ -577,7 +577,9 @@ func (s *Stack) PlanChanges(ctx context.Context) ([]stackplan.PlannedChange, tfd
 
 		// TODO: For now we just assume that all values are being created.
 		// Once we have a prior state we should compare with that to
-		// produce accurate change actions.
+		// produce accurate change actions. Also, once outputs are stored in
+		// state, we should update the definition of Applyable for a stack to
+		// reflect updates to outputs making a stack "applyable".
 
 		v, markses := v.UnmarkDeepWithPaths()
 		sensitivePaths, otherMarkses := marks.PathsWithMark(markses, marks.Sensitive)

internal/stacks/stackruntime/plan.go

@@ -5,7 +5,6 @@ package stackruntime
 
 import (
 	"context"
-	"sync"
 	"time"
 
 	"github.com/hashicorp/terraform/internal/addrs"
@@ -32,16 +31,15 @@ import (
 // through resp after passing it to this function, aside from the implicit
 // modifications to the internal state of channels caused by reading them.
 func Plan(ctx context.Context, req *PlanRequest, resp *PlanResponse) {
-	var respMu sync.Mutex // must hold this when accessing fields of resp, aside from channel sends
-	resp.Applyable = true // we'll reset this to false later if appropriate
-
 	// Whatever return path we take, we must close our channels to allow
 	// a caller to see that the operation is complete.
 	defer func() {
 		close(resp.Diagnostics)
 		close(resp.PlannedChanges) // MUST be the last channel to close
 	}()
 
+	var errored, applyable bool
+
 	main := stackeval.NewForPlanning(req.Config, req.PrevState, stackeval.PlanOpts{
 		PlanningMode:        req.PlanMode,
 		InputVariableValues: req.InputValues,
@@ -53,14 +51,16 @@ func Plan(ctx context.Context, req *PlanRequest, resp *PlanResponse) {
 	main.PlanAll(ctx, stackeval.PlanOutput{
 		AnnouncePlannedChange: func(ctx context.Context, change stackplan.PlannedChange) {
 			resp.PlannedChanges <- change
+			if componentChange, ok := change.(*stackplan.PlannedChangeComponentInstance); ok {
+				if componentChange.PlanApplyable {
+					applyable = true
+				}
+			}
 		},
 		AnnounceDiagnostics: func(ctx context.Context, diags tfdiags.Diagnostics) {
 			for _, diag := range diags {
 				if diag.Severity() == tfdiags.Error {
-					respMu.Lock()
-					// NOTE: Applyable can never become true again after this point.
-					resp.Applyable = false
-					respMu.Unlock()
+					errored = true
 				}
 				resp.Diagnostics <- diag
 			}
@@ -75,6 +75,10 @@ func Plan(ctx context.Context, req *PlanRequest, resp *PlanResponse) {
 		resp.Diagnostics <- diag
 	}
 
+	// An overall stack plan is applyable if at least one of its component
+	// instances is applyable and we had no error diagnostics.
+	resp.Applyable = !errored && applyable
+
 	// Before we return we'll emit one more special planned change just to
 	// remember in the raw plan sequence whether we considered this plan to be
 	// applyable, so we don't need to rely on the caller to remember

internal/stacks/stackruntime/plan_test.go

@@ -334,7 +334,7 @@ func TestPlanWithVariableDefaults(t *testing.T) {
 
 			wantChanges := []stackplan.PlannedChange{
 				&stackplan.PlannedChangeApplyable{
-					Applyable: true,
+					Applyable: false,
 				},
 				&stackplan.PlannedChangeHeader{
 					TerraformVersion: version.SemVer,
@@ -666,7 +666,7 @@ func TestPlanVariableOutputRoundtripNested(t *testing.T) {
 
 	wantChanges := []stackplan.PlannedChange{
 		&stackplan.PlannedChangeApplyable{
-			Applyable: true,
+			Applyable: false,
 		},
 		&stackplan.PlannedChangeHeader{
 			TerraformVersion: version.SemVer,
@@ -1729,12 +1729,7 @@ func TestPlanWithDeferredResource(t *testing.T) {
 
 	wantChanges := []stackplan.PlannedChange{
 		&stackplan.PlannedChangeApplyable{
-			// It's slightly confusing that this is true, but the only component
-			// is not applyable. This is because this is based on the presence
-			// of any diagnostics, while the component is not applyable because
-			// it has no pending changes. This difference seems to be
-			// deliberate. (TODO(TF-15445): Consider revisiting this.)
-			Applyable: true,
+			Applyable: false,
 		},
 		&stackplan.PlannedChangeComponentInstance{
 			Addr: stackaddrs.Absolute(
@@ -2343,7 +2338,7 @@ func TestPlanWithDeferredEmbeddedStackForEach(t *testing.T) {
 
 	wantChanges := []stackplan.PlannedChange{
 		&stackplan.PlannedChangeApplyable{
-			Applyable: true,
+			Applyable: false,
 		},
 		&stackplan.PlannedChangeHeader{
 			TerraformVersion: version.SemVer,
@@ -2478,7 +2473,7 @@ func TestPlanWithDeferredEmbeddedStackAndComponentForEach(t *testing.T) {
 
 	wantChanges := []stackplan.PlannedChange{
 		&stackplan.PlannedChangeApplyable{
-			Applyable: true,
+			Applyable: false,
 		},
 		&stackplan.PlannedChangeHeader{
 			TerraformVersion: version.SemVer,
@@ -2666,7 +2661,7 @@ func TestPlanWithDeferredProviderForEach(t *testing.T) {
 
 	wantChanges := []stackplan.PlannedChange{
 		&stackplan.PlannedChangeApplyable{
-			Applyable: true,
+			Applyable: false,
 		},
 		&stackplan.PlannedChangeComponentInstance{
 			Addr: stackaddrs.Absolute(