From 6b8b0617bbdb36689180628d9f7d8d400de9c625 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 2 Nov 2021 15:06:49 -0400 Subject: [PATCH] generate precise resource types during validate Allow `GetResource` to return correct types values during validation, rather than relying on `cty.DynamicVal` as a placeholder. This allows other dependent expressions to be more correctly evaluated. --- internal/terraform/context_plan_test.go | 1 + internal/terraform/evaluate.go | 78 ++++++++----------- .../testdata/plan-self-ref-multi-all/main.tf | 2 +- 3 files changed, 36 insertions(+), 45 deletions(-) diff --git a/internal/terraform/context_plan_test.go b/internal/terraform/context_plan_test.go index cfd51da8c..1159f0b42 100644 --- a/internal/terraform/context_plan_test.go +++ b/internal/terraform/context_plan_test.go @@ -5308,6 +5308,7 @@ func TestContext2Plan_selfRefMultiAll(t *testing.T) { ResourceTypes: map[string]*configschema.Block{ "aws_instance": { Attributes: map[string]*configschema.Attribute{ + "id": {Type: cty.String, Computed: true}, "foo": {Type: cty.List(cty.String), Optional: true}, }, }, diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index 71e02d969..c4d45efbf 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -654,6 +654,25 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc return cty.DynamicVal, diags } + // Build the provider address from configuration, since we may not have + // state available in all cases. + // We need to build an abs provider address, but we can use a default + // instance since we're only interested in the schema. + providerAddr := moduleAddr.ProviderConfigDefault(config.Provider) + schema := d.getResourceSchema(addr, providerAddr) + if schema == nil { + // This shouldn't happen, since validation before we get here should've + // taken care of it, but we'll show a reasonable error message anyway. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Missing resource type schema`, + Detail: fmt.Sprintf("No schema is available for %s in %s. This is a bug in Terraform and should be reported.", addr, providerAddr), + Subject: rng.ToHCL().Ptr(), + }) + return cty.DynamicVal, diags + } + ty := schema.ImpliedType() + rs := d.Evaluator.State.Resource(addr.Absolute(d.ModulePath)) if rs == nil { @@ -675,34 +694,27 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc // (since a planned destroy cannot yet remove root outputs), we // need to return a dynamic value here to allow evaluation to // continue. - log.Printf("[ERROR] unknown instance %q referenced during plan", addr.Absolute(d.ModulePath)) + log.Printf("[ERROR] unknown instance %q referenced during %s", addr.Absolute(d.ModulePath), d.Operation) return cty.DynamicVal, diags } - default: - // We should only end up here during the validate walk, - // since later walks should have at least partial states populated - // for all resources in the configuration. - return cty.DynamicVal, diags + if d.Operation != walkValidate { + log.Printf("[ERROR] missing state for %q while in %s\n", addr.Absolute(d.ModulePath), d.Operation) + } + + // Validation is done with only the configuration, so generate + // unknown values of the correct shape for evaluation. + switch { + case config.Count != nil: + return cty.UnknownVal(cty.List(ty)), diags + case config.ForEach != nil: + return cty.UnknownVal(cty.Map(ty)), diags + default: + return cty.UnknownVal(ty), diags + } } } - providerAddr := rs.ProviderConfig - - schema := d.getResourceSchema(addr, providerAddr) - if schema == nil { - // This shouldn't happen, since validation before we get here should've - // taken care of it, but we'll show a reasonable error message anyway. - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: `Missing resource type schema`, - Detail: fmt.Sprintf("No schema is available for %s in %s. This is a bug in Terraform and should be reported.", addr, providerAddr), - Subject: rng.ToHCL().Ptr(), - }) - return cty.DynamicVal, diags - } - ty := schema.ImpliedType() - // Decode all instances in the current state instances := map[addrs.InstanceKey]cty.Value{} pendingDestroy := d.Evaluator.Changes.IsFullDestroy() @@ -862,28 +874,6 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc ret = val } - // since the plan was not yet created during validate, the values we - // collected here may not correspond with configuration, so they must be - // unknown. - if d.Operation == walkValidate { - // While we know the type here and it would be nice to validate whether - // indexes are valid or not, because tuples and objects have fixed - // numbers of elements we can't simply return an unknown value of the - // same type since we have not expanded any instances during - // validation. - // - // In order to validate the expression a little precisely, we'll create - // an unknown map or list here to get more type information. - switch { - case config.Count != nil: - ret = cty.UnknownVal(cty.List(ty)) - case config.ForEach != nil: - ret = cty.UnknownVal(cty.Map(ty)) - default: - ret = cty.UnknownVal(ty) - } - } - return ret, diags } diff --git a/internal/terraform/testdata/plan-self-ref-multi-all/main.tf b/internal/terraform/testdata/plan-self-ref-multi-all/main.tf index d3a9857f7..a4d28f8a0 100644 --- a/internal/terraform/testdata/plan-self-ref-multi-all/main.tf +++ b/internal/terraform/testdata/plan-self-ref-multi-all/main.tf @@ -1,4 +1,4 @@ resource "aws_instance" "web" { - foo = "${aws_instance.web.*.foo}" + foo = aws_instance.web[*].id count = 4 }