From 3cbfd397219d01e8051d1740b616f6e12558de03 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Fri, 19 Apr 2024 12:50:08 +0200 Subject: [PATCH] stacks: validate unknown component for_each type --- .../internal/stackeval/for_each.go | 14 +- internal/stacks/stackruntime/plan_test.go | 135 ++---------------- ...rom-component-of-invalid-type.tfstack.hcl" | 37 +++++ .../parent/parent.tf | 27 ++++ .../self/self.tf | 23 +++ 5 files changed, 106 insertions(+), 130 deletions(-) create mode 100644 "internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/\020deferred-component-for-each-from-component-of-invalid-type.tfstack.hcl" create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/parent/parent.tf create mode 100644 internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/self/self.tf diff --git a/internal/stacks/stackruntime/internal/stackeval/for_each.go b/internal/stacks/stackruntime/internal/stackeval/for_each.go index 8be7d61ab5..90f9f9dced 100644 --- a/internal/stacks/stackruntime/internal/stackeval/for_each.go +++ b/internal/stacks/stackruntime/internal/stackeval/for_each.go @@ -69,22 +69,18 @@ func evaluateForEachExpr(ctx context.Context, expr hcl.Expression, phase EvalPha case ty.IsObjectType() || ty.IsMapType(): // okay - case !result.Value.IsKnown(): - // we can't validate further without knowing the value - return result, diags - case ty.IsSetType(): - if markSafeLengthInt(result.Value) == 0 { - // we are okay with an empty set - return result, diags - } - // since we can't use a set values that are unknown, we treat the // entire set as unknown if !result.Value.IsWhollyKnown() { return result, diags } + if markSafeLengthInt(result.Value) == 0 { + // we are okay with an empty set + return result, diags + } + if !ty.ElementType().Equals(cty.String) { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, diff --git a/internal/stacks/stackruntime/plan_test.go b/internal/stacks/stackruntime/plan_test.go index 9e68b5c6ea..ad5bf7c87b 100644 --- a/internal/stacks/stackruntime/plan_test.go +++ b/internal/stacks/stackruntime/plan_test.go @@ -1705,9 +1705,9 @@ func TestPlanWithDeferredComponentForEach(t *testing.T) { } } -func TestPlanWithDeferredComponentForEachDueToParentComponentOutput(t *testing.T) { +func TestPlanWithDeferredComponentForEachOfInvalidType(t *testing.T) { ctx := context.Background() - cfg := loadMainBundleConfigForTest(t, "deferred-component-for-each-from-component") + cfg := loadMainBundleConfigForTest(t, "deferred-component-for-each-from-component-of-invalid-type") fakePlanTimestamp, err := time.Parse(time.RFC3339, "1991-08-25T20:57:08Z") if err != nil { @@ -1738,134 +1738,27 @@ func TestPlanWithDeferredComponentForEachDueToParentComponentOutput(t *testing.T Diagnostics: diagsCh, } go Plan(ctx, &req, &resp) - gotChanges, diags := collectPlanOutput(changesCh, diagsCh) + _, diags := collectPlanOutput(changesCh, diagsCh) - reportDiagnosticsForTest(t, diags) - if len(diags) != 0 { - t.FailNow() // We reported the diags above/ + if len(diags) != 1 { + t.Fatalf("expected 1 diagnostic, got %d: %s", len(diags), diags) } - sort.SliceStable(gotChanges, func(i, j int) bool { - return plannedChangeSortKey(gotChanges[i]) < plannedChangeSortKey(gotChanges[j]) - }) - - wantChanges := []stackplan.PlannedChange{ - &stackplan.PlannedChangeApplyable{ - Applyable: true, - }, - &stackplan.PlannedChangeComponentInstance{ - Addr: stackaddrs.Absolute( - stackaddrs.RootStackInstance, - stackaddrs.ComponentInstance{ - Component: stackaddrs.Component{Name: "parent"}, - }, - ), - PlanApplyable: true, - PlanComplete: true, - Action: plans.Create, - PlannedInputValues: map[string]plans.DynamicValue{ - "id": mustPlanDynamicValueDynamicType(cty.NullVal(cty.String)), - "input": mustPlanDynamicValueDynamicType(cty.StringVal("parent")), - }, - PlannedOutputValues: map[string]cty.Value{ - "letters_in_id": cty.UnknownVal(cty.Set(cty.DynamicPseudoType)), - }, - PlannedCheckResults: &states.CheckResults{}, - PlanTimestamp: fakePlanTimestamp, - PlannedInputValueMarks: map[string][]cty.PathValueMarks{ - "id": nil, - "input": nil, - }, - }, - &stackplan.PlannedChangeResourceInstancePlanned{ - ResourceInstanceObjectAddr: stackaddrs.AbsResourceInstanceObject{ - Component: stackaddrs.Absolute( - stackaddrs.RootStackInstance, - stackaddrs.ComponentInstance{ - Component: stackaddrs.Component{Name: "parent"}, - }, - ), - Item: addrs.AbsResourceInstanceObject{ - ResourceInstance: addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "testing_resource", - Name: "data", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - }, - }, - ProviderConfigAddr: addrs.AbsProviderConfig{ - Module: addrs.RootModule, - Provider: addrs.MustParseProviderSourceString("hashicorp/testing"), - }, - ChangeSrc: &plans.ResourceInstanceChangeSrc{ - Addr: addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "testing_resource", - Name: "data", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - PrevRunAddr: addrs.Resource{ - Mode: addrs.ManagedResourceMode, - Type: "testing_resource", - Name: "data", - }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), - ProviderAddr: addrs.AbsProviderConfig{ - Module: addrs.RootModule, - Provider: addrs.MustParseProviderSourceString("hashicorp/testing"), - }, - ChangeSrc: plans.ChangeSrc{ - Action: plans.Create, - Before: mustPlanDynamicValue(cty.NullVal(cty.DynamicPseudoType)), - After: plans.DynamicValue{ - // This is ignored for this test - }, - }, - }, - Schema: stacks_testing_provider.TestingResourceSchema, - }, - &stackplan.PlannedChangeComponentInstance{ - Addr: stackaddrs.Absolute( - stackaddrs.RootStackInstance, - stackaddrs.ComponentInstance{ - Component: stackaddrs.Component{Name: "self"}, - Key: addrs.WildcardKey, - }, - ), - PlanApplyable: false, - PlanComplete: true, - Action: plans.Create, - RequiredComponents: collections.NewSet[stackaddrs.AbsComponent]( - stackaddrs.AbsComponent{ - Stack: stackaddrs.RootStackInstance, - Item: stackaddrs.Component{Name: "parent"}, - }, - ), - PlannedInputValues: map[string]plans.DynamicValue{ - "id": mustPlanDynamicValueDynamicType(cty.NullVal(cty.String)), - "input": mustPlanDynamicValueDynamicType(cty.UnknownVal(cty.String)), - }, - PlannedOutputValues: map[string]cty.Value{}, - PlannedCheckResults: &states.CheckResults{}, - PlanTimestamp: fakePlanTimestamp, - PlannedInputValueMarks: map[string][]cty.PathValueMarks{ - "id": nil, - "input": nil, - }, - }, - &stackplan.PlannedChangeHeader{ - TerraformVersion: version.SemVer, - }, + if diags[0].Severity() != tfdiags.Error { + t.Errorf("expected error diagnostic, got %q", diags[0].Severity()) } - // Ignore dynamic value - gotChanges[2].(*stackplan.PlannedChangeResourceInstancePlanned).ChangeSrc.After = wantChanges[2].(*stackplan.PlannedChangeResourceInstancePlanned).ChangeSrc.After + expectedSummary := "Invalid for_each value" + if diags[0].Description().Summary != expectedSummary { + t.Errorf("expected diagnostic with summary %q, got %q", expectedSummary, diags[0].Description().Summary) + } - if diff := cmp.Diff(wantChanges, gotChanges, ctydebug.CmpOptions, cmpCollectionsSet); diff != "" { - t.Errorf("wrong changes\n%s", diff) + expectedDetail := "The for_each expression must produce either a map of any type or a set of strings. The keys of the map or the set elements will serve as unique identifiers for multiple instances of this component." + if diags[0].Description().Detail != expectedDetail { + t.Errorf("expected diagnostic with detail %q, got %q", expectedDetail, diags[0].Description().Detail) } } -// TODO: Test that we throw diagnostics if the output used in component for each has a non-set type - // collectPlanOutput consumes the two output channels emitting results from // a call to [Plan], and collects all of the data written to them before // returning once changesCh has been closed by the sender to indicate that diff --git "a/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/\020deferred-component-for-each-from-component-of-invalid-type.tfstack.hcl" "b/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/\020deferred-component-for-each-from-component-of-invalid-type.tfstack.hcl" new file mode 100644 index 0000000000..4ace97c011 --- /dev/null +++ "b/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/\020deferred-component-for-each-from-component-of-invalid-type.tfstack.hcl" @@ -0,0 +1,37 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + + +provider "testing" "default" {} + + +component "parent" { + source = "./parent" + + providers = { + testing = provider.testing.default + } + + inputs = { + input = "parent" + } +} + + +component "self" { + source = "./self" + + providers = { + testing = provider.testing.default + } + + inputs = { + input = each.value + } + + for_each = component.parent.letters_in_id // This is a list and no set or map +} diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/parent/parent.tf b/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/parent/parent.tf new file mode 100644 index 0000000000..006a3818fe --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/parent/parent.tf @@ -0,0 +1,27 @@ +terraform { + required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } + } +} + +variable "id" { + type = string + default = null + nullable = true # We'll generate an ID if none provided. +} + +variable "input" { + type = string +} + +resource "testing_resource" "data" { + id = var.id + value = var.input +} + +output "letters_in_id" { + value = split("", testing_resource.data.id) +} diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/self/self.tf b/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/self/self.tf new file mode 100644 index 0000000000..4da49727a5 --- /dev/null +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/deferred-component-for-each-from-component-of-invalid-type/self/self.tf @@ -0,0 +1,23 @@ +terraform { + required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } + } +} + +variable "id" { + type = string + default = null + nullable = true # We'll generate an ID if none provided. +} + +variable "input" { + type = string +} + +resource "testing_resource" "data" { + id = var.id + value = var.input +}