From f5bce1bd392c32eb3d6a131aef2ef12bd97905ed Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Mon, 14 Sep 2020 16:53:37 -0400 Subject: [PATCH] terraform: refactor graphNodeImportState and graphNodeImportState (#26243) EvalTrees The import EvalTree()s have been replaced with Execute + straight-through code, which let me remove eval_import_state.go. graphNodeImportStateSub's Execute() function is still using the old-style (not-refactored) calls to EvalRefresh and EvalWriteState; those are being saved for a later refactor once all (or at least most) of the EvalTree()s are gone. I've added incredibly basic tests for both Execute() functions; they are only enough to verify basics - the real testing happens in context_import_test.go. --- terraform/eval_import_state.go | 95 --------------- terraform/transform_import_state.go | 144 ++++++++++++++--------- terraform/transform_import_state_test.go | 109 +++++++++++++++++ 3 files changed, 197 insertions(+), 151 deletions(-) delete mode 100644 terraform/eval_import_state.go create mode 100644 terraform/transform_import_state_test.go diff --git a/terraform/eval_import_state.go b/terraform/eval_import_state.go deleted file mode 100644 index a60f4a0a2..000000000 --- a/terraform/eval_import_state.go +++ /dev/null @@ -1,95 +0,0 @@ -package terraform - -import ( - "fmt" - "log" - - "github.com/hashicorp/terraform/addrs" - "github.com/hashicorp/terraform/providers" - "github.com/hashicorp/terraform/states" - "github.com/hashicorp/terraform/tfdiags" -) - -// EvalImportState is an EvalNode implementation that performs an -// ImportState operation on a provider. This will return the imported -// states but won't modify any actual state. -type EvalImportState struct { - Addr addrs.ResourceInstance - Provider *providers.Interface - ID string - Output *[]providers.ImportedResource -} - -// TODO: test -func (n *EvalImportState) Eval(ctx EvalContext) (interface{}, error) { - absAddr := n.Addr.Absolute(ctx.Path()) - provider := *n.Provider - var diags tfdiags.Diagnostics - - { - // Call pre-import hook - err := ctx.Hook(func(h Hook) (HookAction, error) { - return h.PreImportState(absAddr, n.ID) - }) - if err != nil { - return nil, err - } - } - - resp := provider.ImportResourceState(providers.ImportResourceStateRequest{ - TypeName: n.Addr.Resource.Type, - ID: n.ID, - }) - diags = diags.Append(resp.Diagnostics) - if diags.HasErrors() { - return nil, diags.Err() - } - - imported := resp.ImportedResources - - for _, obj := range imported { - log.Printf("[TRACE] EvalImportState: import %s %q produced instance object of type %s", absAddr.String(), n.ID, obj.TypeName) - } - - if n.Output != nil { - *n.Output = imported - } - - { - // Call post-import hook - err := ctx.Hook(func(h Hook) (HookAction, error) { - return h.PostImportState(absAddr, imported) - }) - if err != nil { - return nil, err - } - } - - return nil, nil -} - -// EvalImportStateVerify verifies the state after ImportState and -// after the refresh to make sure it is non-nil and valid. -type EvalImportStateVerify struct { - Addr addrs.ResourceInstance - State **states.ResourceInstanceObject -} - -// TODO: test -func (n *EvalImportStateVerify) Eval(ctx EvalContext) (interface{}, error) { - var diags tfdiags.Diagnostics - - state := *n.State - if state.Value.IsNull() { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Cannot import non-existent remote object", - fmt.Sprintf( - "While attempting to import an existing object to %s, the provider detected that no object exists with the given id. Only pre-existing objects can be imported; check that the id is correct and that it is associated with the provider's configured region or endpoint, or use \"terraform apply\" to create a new remote object for this resource.", - n.Addr.String(), - ), - )) - } - - return nil, diags.ErrWithWarnings() -} diff --git a/terraform/transform_import_state.go b/terraform/transform_import_state.go index 158bbfe15..bbeba7aa7 100644 --- a/terraform/transform_import_state.go +++ b/terraform/transform_import_state.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "log" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" @@ -70,7 +71,7 @@ type graphNodeImportState struct { var ( _ GraphNodeModulePath = (*graphNodeImportState)(nil) - _ GraphNodeEvalable = (*graphNodeImportState)(nil) + _ GraphNodeExecutable = (*graphNodeImportState)(nil) _ GraphNodeProviderConsumer = (*graphNodeImportState)(nil) _ GraphNodeDynamicExpandable = (*graphNodeImportState)(nil) ) @@ -114,28 +115,48 @@ func (n *graphNodeImportState) ModulePath() addrs.Module { return n.Addr.Module.Module() } -// GraphNodeEvalable impl. -func (n *graphNodeImportState) EvalTree() EvalNode { - var provider providers.Interface - +// GraphNodeExecutable impl. +func (n *graphNodeImportState) Execute(ctx EvalContext, op walkOperation) error { // Reset our states n.states = nil - // Return our sequence - return &EvalSequence{ - Nodes: []EvalNode{ - &EvalGetProvider{ - Addr: n.ResolvedProvider, - Output: &provider, - }, - &EvalImportState{ - Addr: n.Addr.Resource, - Provider: &provider, - ID: n.ID, - Output: &n.states, - }, - }, + provider, _, err := GetProvider(ctx, n.ResolvedProvider) + if err != nil { + return err } + + // import state + absAddr := n.Addr.Resource.Absolute(ctx.Path()) + var diags tfdiags.Diagnostics + + // Call pre-import hook + err = ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreImportState(absAddr, n.ID) + }) + if err != nil { + return err + } + + resp := provider.ImportResourceState(providers.ImportResourceStateRequest{ + TypeName: n.Addr.Resource.Resource.Type, + ID: n.ID, + }) + diags = diags.Append(resp.Diagnostics) + if diags.HasErrors() { + return diags.Err() + } + + imported := resp.ImportedResources + for _, obj := range imported { + log.Printf("[TRACE] graphNodeImportState: import %s %q produced instance object of type %s", absAddr.String(), n.ID, obj.TypeName) + } + n.states = imported + + // Call post-import hook + err = ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostImportState(absAddr, imported) + }) + return err } // GraphNodeDynamicExpandable impl. @@ -194,9 +215,9 @@ func (n *graphNodeImportState) DynamicExpand(ctx EvalContext) (*Graph, error) { } // For each of the states, we add a node to handle the refresh/add to state. - // "n.states" is populated by our own EvalTree with the result of - // ImportState. Since DynamicExpand is always called after EvalTree, this - // is safe. + // "n.states" is populated by our own Execute with the result of + // ImportState. Since DynamicExpand is always called after Execute, this is + // safe. for i, state := range n.states { g.Add(&graphNodeImportStateSub{ TargetAddr: addrs[i], @@ -226,7 +247,7 @@ type graphNodeImportStateSub struct { var ( _ GraphNodeModuleInstance = (*graphNodeImportStateSub)(nil) - _ GraphNodeEvalable = (*graphNodeImportStateSub)(nil) + _ GraphNodeExecutable = (*graphNodeImportStateSub)(nil) ) func (n *graphNodeImportStateSub) Name() string { @@ -237,43 +258,54 @@ func (n *graphNodeImportStateSub) Path() addrs.ModuleInstance { return n.TargetAddr.Module } -// GraphNodeEvalable impl. -func (n *graphNodeImportStateSub) EvalTree() EvalNode { +// GraphNodeExecutable impl. +func (n *graphNodeImportStateSub) Execute(ctx EvalContext, op walkOperation) error { // If the Ephemeral type isn't set, then it is an error if n.State.TypeName == "" { - err := fmt.Errorf("import of %s didn't set type", n.TargetAddr.String()) - return &EvalReturnError{Error: &err} + return fmt.Errorf("import of %s didn't set type", n.TargetAddr.String()) } state := n.State.AsInstanceObject() - - var provider providers.Interface - var providerSchema *ProviderSchema - return &EvalSequence{ - Nodes: []EvalNode{ - &EvalGetProvider{ - Addr: n.ResolvedProvider, - Output: &provider, - Schema: &providerSchema, - }, - &EvalRefresh{ - Addr: n.TargetAddr.Resource, - ProviderAddr: n.ResolvedProvider, - Provider: &provider, - ProviderSchema: &providerSchema, - State: &state, - Output: &state, - }, - &EvalImportStateVerify{ - Addr: n.TargetAddr.Resource, - State: &state, - }, - &EvalWriteState{ - Addr: n.TargetAddr.Resource, - ProviderAddr: n.ResolvedProvider, - ProviderSchema: &providerSchema, - State: &state, - }, - }, + provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider) + if err != nil { + return err } + + // EvalRefresh + evalRefresh := &EvalRefresh{ + Addr: n.TargetAddr.Resource, + ProviderAddr: n.ResolvedProvider, + Provider: &provider, + ProviderSchema: &providerSchema, + State: &state, + Output: &state, + } + _, err = evalRefresh.Eval(ctx) + if err != nil { + return err + } + + // Verify the existance of the imported resource + if state.Value.IsNull() { + var diags tfdiags.Diagnostics + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Cannot import non-existent remote object", + fmt.Sprintf( + "While attempting to import an existing object to %s, the provider detected that no object exists with the given id. Only pre-existing objects can be imported; check that the id is correct and that it is associated with the provider's configured region or endpoint, or use \"terraform apply\" to create a new remote object for this resource.", + n.TargetAddr.Resource.String(), + ), + )) + return diags.Err() + } + + //EvalWriteState + evalWriteState := &EvalWriteState{ + Addr: n.TargetAddr.Resource, + ProviderAddr: n.ResolvedProvider, + ProviderSchema: &providerSchema, + State: &state, + } + _, err = evalWriteState.Eval(ctx) + return err } diff --git a/terraform/transform_import_state_test.go b/terraform/transform_import_state_test.go new file mode 100644 index 000000000..9068a3654 --- /dev/null +++ b/terraform/transform_import_state_test.go @@ -0,0 +1,109 @@ +package terraform + +import ( + "strings" + "testing" + + "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/configs/configschema" + "github.com/hashicorp/terraform/providers" + "github.com/hashicorp/terraform/states" + "github.com/zclconf/go-cty/cty" +) + +func TestGraphNodeImportStateExecute(t *testing.T) { + state := states.NewState() + provider := testProvider("aws") + provider.ImportStateReturn = []*InstanceState{ + &InstanceState{ + ID: "bar", + Ephemeral: EphemeralState{Type: "aws_instance"}, + }, + } + ctx := &MockEvalContext{ + StateState: state.SyncWrapper(), + ProviderProvider: provider, + } + + // Import a new aws_instance.foo, this time with ID=bar. The original + // aws_instance.foo object should be removed from state and replaced with + // the new. + node := graphNodeImportState{ + Addr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + ID: "bar", + ResolvedProvider: addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("aws"), + Module: addrs.RootModule, + }, + } + + err := node.Execute(ctx, walkImport) + if err != nil { + t.Fatalf("Unexpected error: %s", err.Error()) + } + + if len(node.states) != 1 { + t.Fatalf("Wrong result! Expected one imported resource, got %d", len(node.states)) + } + // Verify the ID for good measure + id := node.states[0].State.GetAttr("id") + if !id.RawEquals(cty.StringVal("bar")) { + t.Fatalf("Wrong result! Expected id \"bar\", got %q", id.AsString()) + } +} + +func TestGraphNodeImportStateSubExecute(t *testing.T) { + state := states.NewState() + provider := testProvider("aws") + ctx := &MockEvalContext{ + StateState: state.SyncWrapper(), + ProviderProvider: provider, + ProviderSchemaSchema: &ProviderSchema{ + ResourceTypes: map[string]*configschema.Block{ + "aws_instance": { + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + }, + }, + }, + }, + } + + importedResource := providers.ImportedResource{ + TypeName: "aws_instance", + State: cty.ObjectVal(map[string]cty.Value{"id": cty.StringVal("bar")}), + } + + node := graphNodeImportStateSub{ + TargetAddr: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "aws_instance", + Name: "foo", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + State: importedResource, + ResolvedProvider: addrs.AbsProviderConfig{ + Provider: addrs.NewDefaultProvider("aws"), + Module: addrs.RootModule, + }, + } + err := node.Execute(ctx, walkImport) + if err != nil { + t.Fatalf("Unexpected error: %s", err.Error()) + } + + // check for resource in state + actual := strings.TrimSpace(state.String()) + expected := `aws_instance.foo: + ID = bar + provider = provider["registry.terraform.io/hashicorp/aws"]` + if actual != expected { + t.Fatalf("bad state after import: \n%s", actual) + } +}