Skip to content

Commit

Permalink
unknown evaluation of missing instances in import
Browse files Browse the repository at this point in the history
Because import does not yet plan new instances as part of the import
process, we can end up evaluating references to resources which have no
state at all. The fallback for this situation could result in slightly
better values during import. The count and for_each values were
technically incorrect, since the length is not known to be zero, and the
single instance does have a concrete type which we can return.
  • Loading branch information
jbardin committed Sep 27, 2022
1 parent cc964e6 commit 4ff0fd8
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 1 deletion.
70 changes: 70 additions & 0 deletions internal/terraform/context_import_test.go
Expand Up @@ -915,6 +915,76 @@ resource "test_resource" "unused" {
}
}

// New resources in the config during import won't exist for evaluation
// purposes (until import is upgraded to using a complete plan). This means
// that references to them are unknown, but in the case of single instances, we
// can at least know the type of unknown value.
func TestContextImport_newResourceUnknown(t *testing.T) {
p := testProvider("aws")
m := testModuleInline(t, map[string]string{
"main.tf": `
resource "test_resource" "one" {
}
resource "test_resource" "two" {
count = length(flatten([test_resource.one.id]))
}
resource "test_resource" "test" {
}
`})

p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&ProviderSchema{
ResourceTypes: map[string]*configschema.Block{
"test_resource": {
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Computed: true},
},
},
},
})

p.ImportResourceStateResponse = &providers.ImportResourceStateResponse{
ImportedResources: []providers.ImportedResource{
{
TypeName: "test_resource",
State: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("test"),
}),
},
},
}

ctx := testContext2(t, &ContextOpts{
Providers: map[addrs.Provider]providers.Factory{
addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
},
})

state, diags := ctx.Import(m, states.NewState(), &ImportOpts{
Targets: []*ImportTarget{
{
Addr: addrs.RootModuleInstance.ResourceInstance(
addrs.ManagedResourceMode, "test_resource", "test", addrs.NoKey,
),
ID: "test",
},
},
})
if diags.HasErrors() {
t.Fatal(diags.ErrWithWarnings())
}

ri := state.ResourceInstance(mustResourceInstanceAddr("test_resource.test"))
expected := `{"id":"test"}`
if ri == nil || ri.Current == nil {
t.Fatal("no state is recorded for resource instance test_resource.test")
}
if string(ri.Current.AttrsJSON) != expected {
t.Fatalf("expected %q, got %q\n", expected, ri.Current.AttrsJSON)
}
}

const testImportStr = `
aws_instance.foo:
ID = foo
Expand Down
23 changes: 23 additions & 0 deletions internal/terraform/evaluate.go
Expand Up @@ -695,6 +695,29 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc
return cty.DynamicVal, diags
}

case walkImport:
// Import does not yet plan resource changes, so new resources from
// config are not going to be found here. Once walkImport fully
// plans resources, this case should not longer be needed.
// In the single instance case, we can return a typed unknown value
// for the instance to better satisfy other expressions using the
// value. This of course will not help if statically known
// attributes are expected to be known elsewhere, but reduces the
// number of problematic configs for now.
// Unlike in plan and apply above we can't be sure the count or
// for_each instances are empty, so we return a DynamicVal. We
// don't really have a good value to return otherwise -- empty
// values will fail for direct index expressions, and unknown
// Lists and Maps could fail in some type unifications.
switch {
case config.Count != nil:
return cty.DynamicVal, diags
case config.ForEach != nil:
return cty.DynamicVal, diags
default:
return cty.UnknownVal(ty), diags
}

default:
// We should only end up here during the validate walk,
// since later walks should have at least partial states populated
Expand Down
@@ -1,5 +1,5 @@
provider "aws" {
value = "${test_instance.bar.value}"
value = "${test_instance.bar.id}"
}

resource "aws_instance" "foo" {
Expand Down

0 comments on commit 4ff0fd8

Please sign in to comment.