From ea3b70abf08fc2ed3fc93f7ed75f695d79d44c58 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 25 Jul 2023 17:16:13 -0700 Subject: [PATCH] stackeval: Component calls are now referenceable This means it's valid to write references like "component.foo" in expressions in a stack configuration, where the result is built from the planned output values of the root module of each component instance. For embedded stacks we intentionally designed the language to require explicit types on all output values so that we'd still be able to do type checking even when we can't calculate the output values yet. Unfortunately we can't do the same trick for components because the main Terraform module language treats output values as dynamically-typed and so an output value we can't evaluate yet could have literally any type. Therefore we concede and just return cty.DynamicVal in most failure cases here; perhaps a future edition of the main Terraform language can improve this by requiring explicitly-typed output values there too. --- internal/rpcapi/stacks.go | 4 +- internal/rpcapi/stacks_test.go | 7 --- internal/stacks/stackruntime/helper_test.go | 10 ++++ .../internal/stackeval/component.go | 49 ++++++++++++++- .../internal/stackeval/component_config.go | 11 ++++ .../internal/stackeval/component_instance.go | 33 ++++++++++ .../stackruntime/internal/stackeval/stack.go | 15 ++++- .../internal/stackeval/stack_config.go | 15 ++++- internal/stacks/stackruntime/plan_test.go | 60 ++++++++++++++----- .../with-single-resource.tf | 9 +++ .../with-single-resource.tfstack.hcl | 8 +++ internal/stacks/stackruntime/validate_test.go | 2 +- 12 files changed, 192 insertions(+), 31 deletions(-) diff --git a/internal/rpcapi/stacks.go b/internal/rpcapi/stacks.go index 3e3ca00faa..2594f41795 100644 --- a/internal/rpcapi/stacks.go +++ b/internal/rpcapi/stacks.go @@ -163,10 +163,10 @@ func (s *stacksServer) PlanStackChanges(req *terraform1.PlanStackChanges_Request default: return status.Errorf(codes.InvalidArgument, "unsupported planning mode %d", req.PlanMode) } - log.Printf("[TRACE] plan mode is %s", planMode) // TEMP: Just so planMode is used for now + log.Printf("[TRACE] plan mode is %s", planMode) // TODO: Just so planMode is used for now if len(req.PreviousState) != 0 { - // TEMP: We don't yet support planning from a prior state. + // TODO: We don't yet support planning from a prior state. return status.Errorf(codes.InvalidArgument, "don't yet support planning with a previous state") } diff --git a/internal/rpcapi/stacks_test.go b/internal/rpcapi/stacks_test.go index df00c3a547..bce7d90d0f 100644 --- a/internal/rpcapi/stacks_test.go +++ b/internal/rpcapi/stacks_test.go @@ -224,13 +224,6 @@ func TestStacksPlanStackChanges(t *testing.T) { handles := newHandleTable() stacksServer := newStacksServer(handles) - // TEMP: For now this RPC doesn't actually really do any work and instead - // just immediately emits a warning diagnostic admitting that it's a liar. - // We'll replace this with a real implementation in later commits. - // This test implementation is equally fake, using invalid stubs of - // everything just to get the right types for this to compile and - // generate the expected warning. - fakeSourceBundle := &sourcebundle.Bundle{} bundleHnd := handles.NewSourceBundle(fakeSourceBundle) emptyConfig := &stackconfig.Config{ diff --git a/internal/stacks/stackruntime/helper_test.go b/internal/stacks/stackruntime/helper_test.go index 07c03383f2..91cffc4b95 100644 --- a/internal/stacks/stackruntime/helper_test.go +++ b/internal/stacks/stackruntime/helper_test.go @@ -6,8 +6,10 @@ import ( "github.com/hashicorp/go-slug/sourceaddrs" "github.com/hashicorp/go-slug/sourcebundle" + "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/stacks/stackconfig" "github.com/hashicorp/terraform/internal/tfdiags" + "github.com/zclconf/go-cty/cty" ) // This file has helper functions used by other tests. It doesn't contain any @@ -97,3 +99,11 @@ func reportDiagnosticsForTest(t *testing.T, diags tfdiags.Diagnostics) { t.FailNow() } } + +func mustPlanDynamicValue(v cty.Value) plans.DynamicValue { + ret, err := plans.NewDynamicValue(v, v.Type()) + if err != nil { + panic(err) + } + return ret +} diff --git a/internal/stacks/stackruntime/internal/stackeval/component.go b/internal/stacks/stackruntime/internal/stackeval/component.go index 71cdff4502..5bcda8f11e 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component.go +++ b/internal/stacks/stackruntime/internal/stackeval/component.go @@ -221,9 +221,56 @@ func (c *Component) Instances(ctx context.Context, phase EvalPhase) map[addrs.In } } +func (c *Component) ResultValue(ctx context.Context, phase EvalPhase) cty.Value { + decl := c.Declaration(ctx) + insts := c.Instances(ctx, phase) + + switch { + case decl.ForEach != nil: + // NOTE: Unlike with StackCall, we must return object types rather than + // map types here since the main Terraform language does not require + // exact type constraints for its output values and so each instance of + // a component can potentially produce a different object type. + + if insts == nil { + // If we don't even know what instances we have then we can't + // predict anything about our result. + return cty.DynamicVal + } + + // We expect that the instances all have string keys, which will + // become the keys of a map that we're returning. + elems := make(map[string]cty.Value, len(insts)) + for instKey, inst := range insts { + k, ok := instKey.(addrs.StringKey) + if !ok { + panic(fmt.Sprintf("stack call with for_each has invalid instance key of type %T", instKey)) + } + elems[string(k)] = inst.ResultValue(ctx, phase) + } + return cty.ObjectVal(elems) + + default: + if insts == nil { + // If we don't even know what instances we have then we can't + // predict anything about our result. + return cty.DynamicVal + } + if len(insts) != 1 { + // Should not happen: we should have exactly one instance with addrs.NoKey + panic("single-instance stack call does not have exactly one instance") + } + inst, ok := insts[addrs.NoKey] + if !ok { + panic("single-instance stack call does not have an addrs.NoKey instance") + } + return inst.ResultValue(ctx, phase) + } +} + // ExprReferenceValue implements Referenceable. func (c *Component) ExprReferenceValue(ctx context.Context, phase EvalPhase) cty.Value { - panic("unimplemented") + return c.ResultValue(ctx, phase) } // PlanChanges implements Plannable by performing plan-time validation of diff --git a/internal/stacks/stackruntime/internal/stackeval/component_config.go b/internal/stacks/stackruntime/internal/stackeval/component_config.go index d3c4d774de..014c26a250 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_config.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_config.go @@ -165,6 +165,17 @@ func (c *ComponentConfig) InputsType(ctx context.Context) (cty.Type, *typeexpr.D return retTy, defs } +// ExprReferenceValue implements Referenceable. +func (c *ComponentConfig) ExprReferenceValue(ctx context.Context, phase EvalPhase) cty.Value { + // Currently we don't say anything at all about component results during + // validation, since the main Terraform language's validate call doesn't + // return any information about hypothetical root module output values. + // We don't expose ComponentConfig in any scope outside of the validation + // phase, so this is sufficient for all phases. (See [Component] for how + // component results get calculated during the plan and apply phases.) + return cty.DynamicVal +} + // Validate implements Validatable. func (c *ComponentConfig) Validate(ctx context.Context) tfdiags.Diagnostics { var diags tfdiags.Diagnostics diff --git a/internal/stacks/stackruntime/internal/stackeval/component_instance.go b/internal/stacks/stackruntime/internal/stackeval/component_instance.go index ecdc8bccc6..64ab813497 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_instance.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_instance.go @@ -223,6 +223,39 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla ) } +func (c *ComponentInstance) ResultValue(ctx context.Context, phase EvalPhase) cty.Value { + switch phase { + case PlanPhase: + plan := c.ModuleTreePlan(ctx) + if plan == nil { + // Planning seems to have failed so we cannot decide a result value yet. + // FIXME: Should use an unknown value of a specific object type + // constraint derived from the root module's output values. + return cty.DynamicVal + } + + // During the plan phase we use the planned output changes to construct + // our value. + outputChanges := plan.Changes.Outputs + attrs := make(map[string]cty.Value, len(outputChanges)) + for _, changeSrc := range outputChanges { + name := changeSrc.Addr.OutputValue.Name + change, err := changeSrc.Decode() + if err != nil { + attrs[name] = cty.DynamicVal + } + attrs[name] = change.After + } + return cty.ObjectVal(attrs) + + default: + // We can't produce a concrete value for any other phase. + // FIXME: Should use an unknown value of a specific object type + // constraint derived from the root module's output values. + return cty.DynamicVal + } +} + // ResolveExpressionReference implements ExpressionScope. func (c *ComponentInstance) ResolveExpressionReference(ctx context.Context, ref stackaddrs.Reference) (Referenceable, tfdiags.Diagnostics) { stack := c.call.Stack(ctx) diff --git a/internal/stacks/stackruntime/internal/stackeval/stack.go b/internal/stacks/stackruntime/internal/stackeval/stack.go index 4ba73ab452..2a48007edb 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stack.go +++ b/internal/stacks/stackruntime/internal/stackeval/stack.go @@ -323,18 +323,29 @@ func (s *Stack) resolveExpressionReference(ctx context.Context, ref stackaddrs.R if ret == nil { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Reference to undefined input variable", + Summary: "Reference to undeclared input variable", Detail: fmt.Sprintf("There is no variable %q block declared in this stack.", addr.Name), Subject: ref.SourceRange.ToHCL().Ptr(), }) } return ret, diags + case stackaddrs.Component: + ret := s.Component(ctx, addr) + if ret == nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Reference to undeclared component", + Detail: fmt.Sprintf("There is no component %q block declared in this stack.", addr.Name), + Subject: ref.SourceRange.ToHCL().Ptr(), + }) + } + return ret, diags case stackaddrs.StackCall: ret := s.EmbeddedStackCall(ctx, addr) if ret == nil { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Reference to undefined embedded stack", + Summary: "Reference to undeclared embedded stack", Detail: fmt.Sprintf("There is no stack %q block declared this stack.", addr.Name), Subject: ref.SourceRange.ToHCL().Ptr(), }) diff --git a/internal/stacks/stackruntime/internal/stackeval/stack_config.go b/internal/stacks/stackruntime/internal/stackeval/stack_config.go index cc84d5e883..33771e26b8 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stack_config.go +++ b/internal/stacks/stackruntime/internal/stackeval/stack_config.go @@ -286,18 +286,29 @@ func (s *StackConfig) resolveExpressionReference(ctx context.Context, ref stacka if ret == nil { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Reference to undefined input variable", + Summary: "Reference to undeclared input variable", Detail: fmt.Sprintf("There is no variable %q block declared in this stack.", addr.Name), Subject: ref.SourceRange.ToHCL().Ptr(), }) } return ret, diags + case stackaddrs.Component: + ret := s.Component(ctx, addr) + if ret == nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Reference to undeclared component", + Detail: fmt.Sprintf("There is no component %q block declared in this stack.", addr.Name), + Subject: ref.SourceRange.ToHCL().Ptr(), + }) + } + return ret, diags case stackaddrs.StackCall: ret := s.StackCall(ctx, addr) if ret == nil { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Reference to undefined embedded stack", + Summary: "Reference to undeclared embedded stack", Detail: fmt.Sprintf("There is no stack %q block declared this stack.", addr.Name), Subject: ref.SourceRange.ToHCL().Ptr(), }) diff --git a/internal/stacks/stackruntime/plan_test.go b/internal/stacks/stackruntime/plan_test.go index e10c4a7e7b..dbd99404df 100644 --- a/internal/stacks/stackruntime/plan_test.go +++ b/internal/stacks/stackruntime/plan_test.go @@ -2,6 +2,8 @@ package stackruntime import ( "context" + "fmt" + "sort" "testing" "github.com/google/go-cmp/cmp" @@ -11,6 +13,7 @@ import ( "github.com/hashicorp/terraform/internal/stacks/stackplan" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/hashicorp/terraform/version" + "github.com/zclconf/go-cty/cty" ) func TestPlanWithSingleResource(t *testing.T) { @@ -34,10 +37,32 @@ func TestPlanWithSingleResource(t *testing.T) { t.Errorf("unexpected diagnostics\n%s", diags.ErrWithWarnings().Error()) } + // The order of emission for our planned changes is unspecified since it + // depends on how the various goroutines get scheduled, and so we'll + // arbitrarily sort gotChanges lexically by the name of the change type + // so that we have some dependable order to diff against below. + sort.Slice(gotChanges, func(i, j int) bool { + ic := gotChanges[i] + jc := gotChanges[j] + return fmt.Sprintf("%T", ic) < fmt.Sprintf("%T", jc) + }) + wantChanges := []stackplan.PlannedChange{ + &stackplan.PlannedChangeApplyable{ + Applyable: true, + }, &stackplan.PlannedChangeHeader{ TerraformVersion: version.SemVer, }, + &stackplan.PlannedChangeOutputValue{ + Addr: stackaddrs.OutputValue{Name: "obj"}, + Action: plans.Create, + OldValue: mustPlanDynamicValue(cty.NullVal(cty.DynamicPseudoType)), + NewValue: mustPlanDynamicValue(cty.ObjectVal(map[string]cty.Value{ + "input": cty.StringVal("hello"), + "output": cty.UnknownVal(cty.String), + })), + }, &stackplan.PlannedChangeResourceInstancePlanned{ ComponentInstanceAddr: stackaddrs.Absolute( stackaddrs.RootStackInstance, @@ -62,25 +87,30 @@ func TestPlanWithSingleResource(t *testing.T) { }, ChangeSrc: plans.ChangeSrc{ Action: plans.Create, - Before: plans.DynamicValue{0xc0}, // MessagePack-encoded null + Before: mustPlanDynamicValue(cty.NullVal(cty.DynamicPseudoType)), After: plans.DynamicValue{ - // This is a MessagePack-encoded object type conforming - // to the terraform_data resource type's schema. - // FIXME: Should write this a different way that - // is more scrutable and won't break each time something - // gets added to the terraform_data schema. - 0x84, 0xa2, 0x69, 0x64, 0xc7, 0x03, 0x0c, 0x81, 0x01, - 0xc2, 0xa5, 0x69, 0x6e, 0x70, 0x75, 0x74, 0xc0, 0xa6, - 0x6f, 0x75, 0x74, 0x70, 0x75, 0x74, 0xc0, 0xb0, 0x74, - 0x72, 0x69, 0x67, 0x67, 0x65, 0x72, 0x73, 0x5f, 0x72, - 0x65, 0x70, 0x6c, 0x61, 0x63, 0x65, 0xc0, + // This is an object conforming to the terraform_data + // resource type's schema. + // + // FIXME: Should write this a different way that is + // scrutable and won't break each time something gets + // added to the terraform_data schema. (We can't use + // mustPlanDynamicValue here because the resource type + // uses DynamicPseudoType attributes, which require + // explicitly-typed encoding.) + 0x84, 0xa2, 0x69, 0x64, 0xc7, 0x03, 0x0c, 0x81, + 0x01, 0xc2, 0xa5, 0x69, 0x6e, 0x70, 0x75, 0x74, + 0x92, 0xc4, 0x08, 0x22, 0x73, 0x74, 0x72, 0x69, + 0x6e, 0x67, 0x22, 0xa5, 0x68, 0x65, 0x6c, 0x6c, + 0x6f, 0xa6, 0x6f, 0x75, 0x74, 0x70, 0x75, 0x74, + 0x92, 0xc4, 0x08, 0x22, 0x73, 0x74, 0x72, 0x69, + 0x6e, 0x67, 0x22, 0xd4, 0x00, 0x00, 0xb0, 0x74, + 0x72, 0x69, 0x67, 0x67, 0x65, 0x72, 0x73, 0x5f, + 0x72, 0x65, 0x70, 0x6c, 0x61, 0x63, 0x65, 0xc0, }, }, }, }, - &stackplan.PlannedChangeApplyable{ - Applyable: true, - }, } if diff := cmp.Diff(wantChanges, gotChanges); diff != "" { @@ -160,6 +190,4 @@ func collectPlanOutput(changesCh <-chan stackplan.PlannedChange, diagsCh <-chan diags = append(diags, diag) } } - - return changes, diags } diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-resource/with-single-resource.tf b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-resource/with-single-resource.tf index bbf2c6861c..b89447e101 100644 --- a/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-resource/with-single-resource.tf +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-resource/with-single-resource.tf @@ -7,4 +7,13 @@ terraform { } resource "terraform_data" "main" { + input = "hello" +} + +output "input" { + value = terraform_data.main.input +} + +output "output" { + value = terraform_data.main.output } diff --git a/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-resource/with-single-resource.tfstack.hcl b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-resource/with-single-resource.tfstack.hcl index 3aaf69bccd..e040177441 100644 --- a/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-resource/with-single-resource.tfstack.hcl +++ b/internal/stacks/stackruntime/testdata/mainbundle/test/with-single-resource/with-single-resource.tfstack.hcl @@ -4,3 +4,11 @@ component "self" { # is the canonical form. A bug in go-slug's sourceaddrs package? source = "./." } + +output "obj" { + type = object({ + input = string + output = string + }) + value = component.self +} diff --git a/internal/stacks/stackruntime/validate_test.go b/internal/stacks/stackruntime/validate_test.go index 1d81d9d20e..e26694918f 100644 --- a/internal/stacks/stackruntime/validate_test.go +++ b/internal/stacks/stackruntime/validate_test.go @@ -60,7 +60,7 @@ func TestValidate_undeclaredVariable(t *testing.T) { var wantDiags tfdiags.Diagnostics wantDiags = wantDiags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: "Reference to undefined input variable", + Summary: "Reference to undeclared input variable", Detail: `There is no variable "a" block declared in this stack.`, Subject: &hcl.Range{ Filename: mainBundleSourceAddrStr("validate-undeclared-variable/validate-undeclared-variable.tfstack.hcl"),