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.

Sign up for GitHub

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

Jump to bottom

generate precise resource types during validate #29862

Merged
merged 2 commits into from
Nov 3, 2021
Merged

generate precise resource types during validate #29862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b8b061
generate precise resource types during validate
jbardin Nov 2, 2021
647d360
we don't need an abs provider address
jbardin Nov 2, 2021
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
1 change: 1 addition & 0 deletions internal/terraform/context_plan_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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},
},
},
Expand Down
81 changes: 35 additions & 46 deletions internal/terraform/evaluate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,24 @@ 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.
schema := d.getResourceSchema(addr, config.Provider)
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, config.Provider),
Subject: rng.ToHCL().Ptr(),
})
return cty.DynamicVal, diags
}
ty := schema.ImpliedType()

rs := d.Evaluator.State.Resource(addr.Absolute(d.ModulePath))

if rs == nil {
Expand All @@ -675,33 +693,26 @@ 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
}
}

providerAddr := rs.ProviderConfig
if d.Operation != walkValidate {
log.Printf("[ERROR] missing state for %q while in %s\n", addr.Absolute(d.ModulePath), d.Operation)
}

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
// 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
}
}
}
ty := schema.ImpliedType()

// Decode all instances in the current state
instances := map[addrs.InstanceKey]cty.Value{}
Expand Down Expand Up @@ -862,33 +873,11 @@ 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 {
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 section was redundant with the default case before we checked the instance state, since there is no instance state during validation. Verified that this condition was no longer reached during tests.

// 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
}

func (d *evaluationStateData) getResourceSchema(addr addrs.Resource, providerAddr addrs.AbsProviderConfig) *configschema.Block {
schema, _, err := d.Evaluator.Plugins.ResourceTypeSchema(providerAddr.Provider, addr.Mode, addr.Type)
func (d *evaluationStateData) getResourceSchema(addr addrs.Resource, providerAddr addrs.Provider) *configschema.Block {
schema, _, err := d.Evaluator.Plugins.ResourceTypeSchema(providerAddr, addr.Mode, addr.Type)
if err != nil {
// We have plently other codepaths that will detect and report
// schema lookup errors before we'd reach this point, so we'll just
Expand Down
2 changes: 1 addition & 1 deletion internal/terraform/testdata/plan-self-ref-multi-all/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
resource "aws_instance" "web" {
foo = "${aws_instance.web.*.foo}"
Copy link
Member Author

Choose a reason for hiding this comment

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

improved evaluation found a very old invalid test which was assigning a list(list(string)) to a list(string) (though that could have been flattened in the legacy type system, but was never caught post-0.12)

foo = aws_instance.web[*].id
count = 4
}