Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stacks: allow unknown variables during apply operations #36311

Merged
merged 1 commit into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
133 changes: 133 additions & 0 deletions internal/stacks/stackruntime/apply_destroy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,139 @@ func TestApplyDestroy(t *testing.T) {
},
},
},
"destroy after manual removal": {
path: "removed-offline",
state: stackstate.NewStateBuilder().
AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.parent")).
AddDependent(mustAbsComponent("component.child")).
AddOutputValue("value", cty.StringVal("hello"))).
AddResourceInstance(stackstate.NewResourceInstanceBuilder().
SetAddr(mustAbsResourceInstanceObject("component.parent.testing_resource.resource")).
SetProviderAddr(mustDefaultRootProvider("testing")).
SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{
AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{
"id": "parent",
"value": "hello",
}),
Status: states.ObjectReady,
})).
AddComponentInstance(stackstate.NewComponentInstanceBuilder(mustAbsComponentInstance("component.child")).
AddDependency(mustAbsComponent("component.parent")).
AddInputVariable("value", cty.StringVal("hello"))).
AddResourceInstance(stackstate.NewResourceInstanceBuilder().
SetAddr(mustAbsResourceInstanceObject("component.child.testing_resource.resource")).
SetProviderAddr(mustDefaultRootProvider("testing")).
SetResourceInstanceObjectSrc(states.ResourceInstanceObjectSrc{
AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{
"id": "child",
"value": "hello",
}),
Status: states.ObjectReady,
})).
Build(),
store: stacks_testing_provider.NewResourceStoreBuilder().
AddResource("child", cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("child"),
"value": cty.StringVal("hello"),
})).Build(),
cycles: []TestCycle{
{
planMode: plans.DestroyMode,
wantPlannedChanges: []stackplan.PlannedChange{
&stackplan.PlannedChangeApplyable{
Applyable: true,
},
&stackplan.PlannedChangeComponentInstance{
Addr: mustAbsComponentInstance("component.child"),
Action: plans.Delete,
Mode: plans.DestroyMode,
PlanComplete: true,
PlanApplyable: true,
RequiredComponents: collections.NewSet(mustAbsComponent("component.parent")),
PlannedInputValues: map[string]plans.DynamicValue{
"value": mustPlanDynamicValueDynamicType(cty.UnknownVal(cty.String)),
},
PlannedInputValueMarks: map[string][]cty.PathValueMarks{
"value": nil,
},
PlannedOutputValues: make(map[string]cty.Value),
PlannedCheckResults: &states.CheckResults{},
PlanTimestamp: fakePlanTimestamp,
},
&stackplan.PlannedChangeResourceInstancePlanned{
ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.child.testing_resource.resource"),
ChangeSrc: &plans.ResourceInstanceChangeSrc{
Addr: mustAbsResourceInstance("testing_resource.resource"),
PrevRunAddr: mustAbsResourceInstance("testing_resource.resource"),
ProviderAddr: mustDefaultRootProvider("testing"),
ChangeSrc: plans.ChangeSrc{
Action: plans.Delete,
Before: mustPlanDynamicValue(cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("child"),
"value": cty.StringVal("hello"),
})),
After: mustPlanDynamicValue(cty.NullVal(cty.Object(map[string]cty.Type{
"id": cty.String,
"value": cty.String,
}))),
},
},
PriorStateSrc: &states.ResourceInstanceObjectSrc{
AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{
"id": "child",
"value": "hello",
}),
Status: states.ObjectReady,
Dependencies: make([]addrs.ConfigResource, 0),
},
ProviderConfigAddr: mustDefaultRootProvider("testing"),
Schema: stacks_testing_provider.TestingResourceSchema,
},
&stackplan.PlannedChangeComponentInstance{
Addr: mustAbsComponentInstance("component.parent"),
Action: plans.Delete,
Mode: plans.DestroyMode,
PlanComplete: true,
PlanApplyable: false,
PlannedInputValues: make(map[string]plans.DynamicValue),
PlannedOutputValues: map[string]cty.Value{
"value": cty.DynamicVal,
},
PlannedCheckResults: &states.CheckResults{},
PlanTimestamp: fakePlanTimestamp,
},
&stackplan.PlannedChangeResourceInstancePlanned{
ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.parent.testing_resource.resource"),
ProviderConfigAddr: mustDefaultRootProvider("testing"),
},
&stackplan.PlannedChangeHeader{
TerraformVersion: version.SemVer,
},
&stackplan.PlannedChangePlannedTimestamp{
PlannedTimestamp: fakePlanTimestamp,
},
},
wantAppliedChanges: []stackstate.AppliedChange{
&stackstate.AppliedChangeComponentInstanceRemoved{
ComponentAddr: mustAbsComponent("component.child"),
ComponentInstanceAddr: mustAbsComponentInstance("component.child"),
},
&stackstate.AppliedChangeResourceInstanceObject{
ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.child.testing_resource.resource"),
ProviderConfigAddr: mustDefaultRootProvider("testing"),
},
&stackstate.AppliedChangeComponentInstanceRemoved{
ComponentAddr: mustAbsComponent("component.parent"),
ComponentInstanceAddr: mustAbsComponentInstance("component.parent"),
},
&stackstate.AppliedChangeResourceInstanceObject{
ResourceInstanceObjectAddr: mustAbsResourceInstanceObject("component.parent.testing_resource.resource"),
ProviderConfigAddr: mustDefaultRootProvider("testing"),
},
},
},
},
},
"partial destroy recovery": {
path: "component-chain",
description: "this test simulates a partial destroy recovery",
Expand Down
15 changes: 10 additions & 5 deletions internal/stacks/stackruntime/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1463,7 +1463,11 @@ After applying this plan, Terraform will no longer manage these objects. You wil
ComponentInstanceAddr: mustAbsComponentInstance("component.main"),
Dependencies: collections.NewSet(mustAbsComponent("component.self")),
OutputValues: make(map[addrs.OutputValue]cty.Value),
InputVariables: make(map[addrs.InputVariable]cty.Value),
InputVariables: map[addrs.InputVariable]cty.Value{
mustInputVariable("input"): cty.UnknownVal(cty.Map(cty.Object(map[string]cty.Type{
"output": cty.String,
}))),
},
},
&stackstate.AppliedChangeInputVariable{
Addr: mustStackInputVariable("inputs"),
Expand Down Expand Up @@ -2358,7 +2362,10 @@ func TestApplyWithFailedComponent(t *testing.T) {
ComponentInstanceAddr: mustAbsComponentInstance("component.self"),
Dependencies: collections.NewSet(mustAbsComponent("component.parent")),
OutputValues: make(map[addrs.OutputValue]cty.Value),
InputVariables: make(map[addrs.InputVariable]cty.Value),
InputVariables: map[addrs.InputVariable]cty.Value{
mustInputVariable("id"): cty.NullVal(cty.String),
mustInputVariable("input"): cty.UnknownVal(cty.String),
},
},
}

Expand Down Expand Up @@ -3110,16 +3117,14 @@ func TestApplyWithChangedInputValues(t *testing.T) {

go Apply(ctx, &applyReq, &applyResp)
applyChanges, applyDiags := collectApplyOutput(applyChangesCh, diagsCh)
if len(applyDiags) != 1 {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was redundant, we also check the diagnostics on line 3121, so doing it twice was pointless.

t.Fatalf("expected exactly two diagnostics, got %s", applyDiags.ErrWithWarnings())
}

sort.SliceStable(applyDiags, diagnosticSortFunc(applyDiags))
expectDiagnosticsForTest(t, applyDiags,
expectDiagnostic(
tfdiags.Error,
"Inconsistent value for input variable during apply",
"The value for non-ephemeral input variable \"input\" was set to a different value during apply than was set during plan. Only ephemeral input variables can change between the plan and apply phases."),
expectDiagnostic(tfdiags.Error, "Invalid inputs for component", "Invalid input variable definition object: attribute \"input\": string required."),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the extra diagnostic discussed in the description. This is the diagnostic that now prevents the bad component from actually applying.

)

wantChanges := []stackstate.AppliedChange{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,9 @@ func (c *ComponentInstance) CheckInputVariableValues(ctx context.Context, phase
// We actually checked the errors statically already, so we only care about
// the value here.
val, diags := EvalComponentInputVariables(ctx, varDecls, wantTy, defs, decl, phase, c)
if phase == ApplyPhase {
if !val.IsWhollyKnown() {
// We can't apply a configuration that has unknown values in it.
// This means an error has occured somewhere else, while gathering
// the input variables. We return a nil value here, whatever caused
// the error should have raised an error diagnostic separately.
return cty.NilVal, diags
}

// Note, that unknown values during the planning phase are totally fine.
if diags.HasErrors() {
return cty.NilVal, diags
}

return val, diags
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (v *InputVariable) Value(ctx context.Context, phase EvalPhase) cty.Value {
}

func (v *InputVariable) CheckValue(ctx context.Context, phase EvalPhase) (cty.Value, tfdiags.Diagnostics) {
return withCtyDynamicValPlaceholder(doOnceWithDiags(
return doOnceWithDiags(
ctx, v.value.For(phase), v.main,
func(ctx context.Context) (cty.Value, tfdiags.Diagnostics) {
var diags tfdiags.Diagnostics
Expand Down Expand Up @@ -211,7 +211,7 @@ func (v *InputVariable) CheckValue(ctx context.Context, phase EvalPhase) (cty.Va
return cfg.markValue(val), diags
}
},
))
)
}

// ExprReferenceValue implements Referenceable.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
terraform {
required_providers {
testing = {
source = "hashicorp/testing"
version = "0.1.0"
}
}
}

variable "value" {
type = string
}

resource "testing_resource" "resource" {
value = var.value
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
terraform {
required_providers {
testing = {
source = "hashicorp/testing"
version = "0.1.0"
}
}
}

resource "testing_resource" "resource" {}

output "value" {
value = testing_resource.resource.id
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
required_providers {
testing = {
source = "hashicorp/testing"
version = "0.1.0"
}
}

provider "testing" "default" {}

component "parent" {
source = "./parent"

providers = {
testing = provider.testing.default
}
}

component "child" {
source = "./child"

providers = {
testing = provider.testing.default
}

inputs = {
value = component.parent.value
}
}
Loading