From 44bc7519a66636388b8068861f0c312d7d2e9642 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Fri, 24 Aug 2018 16:13:50 -0700 Subject: [PATCH] terraform: More wiring in of new provider types This doesn't actually work yet, but it builds and then panics in a pretty satisfying way. --- backend/local/testing.go | 54 +-- command/hook_ui.go | 8 +- command/init.go | 3 +- command/meta.go | 6 +- command/plugins.go | 53 +-- configs/configupgrade/analysis.go | 32 +- configs/configupgrade/upgrader.go | 5 +- helper/resource/testing.go | 298 ++++++------- plans/changes.go | 19 +- plans/changes_src.go | 19 +- plugin/client.go | 11 +- plugin/plugin.go | 9 +- providers/provider.go | 24 +- states/import.go | 40 -- terraform/context_apply_test.go | 48 +-- terraform/context_input_test.go | 9 - terraform/context_validate_test.go | 2 +- terraform/eval_apply.go | 179 +++++--- terraform/eval_context_builtin_test.go | 15 +- terraform/eval_diff.go | 507 +++++++++++++++-------- terraform/eval_import_state.go | 82 ++-- terraform/eval_refresh.go | 44 +- terraform/hook.go | 5 +- terraform/hook_mock.go | 5 +- terraform/hook_stop.go | 3 +- terraform/node_resource_apply.go | 11 +- terraform/node_resource_destroy.go | 1 + terraform/node_resource_plan_instance.go | 1 + terraform/node_resource_refresh.go | 11 +- terraform/provider_mock.go | 59 ++- terraform/provisioner_mock.go | 13 + terraform/resource_provider_mock_test.go | 60 ++- terraform/transform_deposed.go | 11 +- terraform/transform_import_state.go | 24 +- 34 files changed, 984 insertions(+), 687 deletions(-) delete mode 100644 states/import.go diff --git a/backend/local/testing.go b/backend/local/testing.go index c72a77183..346bce1c4 100644 --- a/backend/local/testing.go +++ b/backend/local/testing.go @@ -51,34 +51,38 @@ func TestLocal(t *testing.T) (*Local, func()) { // TestLocalProvider modifies the ContextOpts of the *Local parameter to // have a provider with the given name. func TestLocalProvider(t *testing.T, b *Local, name string, schema *terraform.ProviderSchema) *terraform.MockResourceProvider { - // Build a mock resource provider for in-memory operations - p := new(terraform.MockResourceProvider) - p.GetSchemaReturn = schema - p.DiffReturn = &terraform.InstanceDiff{} - p.RefreshFn = func( - info *terraform.InstanceInfo, - s *terraform.InstanceState) (*terraform.InstanceState, error) { - return s, nil - } - p.ResourcesReturn = []terraform.ResourceType{ - terraform.ResourceType{ - Name: "test_instance", - }, - } + t.Fatalf("TestLocalProvider is not yet updated to use the new provider types") + return nil + /* + // Build a mock resource provider for in-memory operations + p := new(terraform.MockResourceProvider) + p.GetSchemaReturn = schema + p.DiffReturn = &terraform.InstanceDiff{} + p.RefreshFn = func( + info *terraform.InstanceInfo, + s *terraform.InstanceState) (*terraform.InstanceState, error) { + return s, nil + } + p.ResourcesReturn = []terraform.ResourceType{ + terraform.ResourceType{ + Name: "test_instance", + }, + } - // Initialize the opts - if b.ContextOpts == nil { - b.ContextOpts = &terraform.ContextOpts{} - } + // Initialize the opts + if b.ContextOpts == nil { + b.ContextOpts = &terraform.ContextOpts{} + } - // Setup our provider - b.ContextOpts.ProviderResolver = terraform.ResourceProviderResolverFixed( - map[string]terraform.ResourceProviderFactory{ - name: terraform.ResourceProviderFactoryFixed(p), - }, - ) + // Setup our provider + b.ContextOpts.ProviderResolver = providers.ResolverFixed( + map[string]providers.Factory{ + name: providers.FactoryFixed(p), + }, + ) - return p + return p + */ } // TestNewLocalSingle is a factory for creating a TestLocalSingleState. diff --git a/command/hook_ui.go b/command/hook_ui.go index 3600af606..9c94a5077 100644 --- a/command/hook_ui.go +++ b/command/hook_ui.go @@ -10,13 +10,13 @@ import ( "time" "unicode" - "github.com/hashicorp/terraform/plans" - "github.com/mitchellh/cli" "github.com/mitchellh/colorstring" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/plans" + "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/terraform" ) @@ -290,7 +290,7 @@ func (h *UiHook) PreImportState(addr addrs.AbsResourceInstance, importID string) return terraform.HookActionContinue, nil } -func (h *UiHook) PostImportState(addr addrs.AbsResourceInstance, imported []*states.ImportedObject) (terraform.HookAction, error) { +func (h *UiHook) PostImportState(addr addrs.AbsResourceInstance, imported []providers.ImportedResource) (terraform.HookAction, error) { h.once.Do(h.init) h.ui.Output(h.Colorize.Color(fmt.Sprintf( @@ -298,7 +298,7 @@ func (h *UiHook) PostImportState(addr addrs.AbsResourceInstance, imported []*sta for _, s := range imported { h.ui.Output(h.Colorize.Color(fmt.Sprintf( "[reset][green] Imported %s", - s.ResourceType, + s.TypeName, ))) } diff --git a/command/init.go b/command/init.go index 32a814631..76299167c 100644 --- a/command/init.go +++ b/command/init.go @@ -422,8 +422,7 @@ func (c *InitCommand) getProviders(path string, state *states.State, upgrade boo if c.getPlugins { if len(missing) > 0 { - c.Ui.Output(fmt.Sprintf("- Checking for available provider plugins on %s...", - discovery.GetReleaseHost())) + c.Ui.Output("- Checking for available provider plugins...") } for provider, reqd := range missing { diff --git a/command/meta.go b/command/meta.go index a437d09ab..5e6c4f08a 100644 --- a/command/meta.go +++ b/command/meta.go @@ -24,6 +24,8 @@ import ( "github.com/hashicorp/terraform/configs/configload" "github.com/hashicorp/terraform/helper/experiment" "github.com/hashicorp/terraform/helper/wrappedstreams" + "github.com/hashicorp/terraform/providers" + "github.com/hashicorp/terraform/svchost/auth" "github.com/hashicorp/terraform/svchost/disco" "github.com/hashicorp/terraform/terraform" "github.com/hashicorp/terraform/tfdiags" @@ -168,8 +170,8 @@ type PluginOverrides struct { } type testingOverrides struct { - ProviderResolver terraform.ResourceProviderResolver - Provisioners map[string]terraform.ResourceProvisionerFactory + ProviderResolver providers.Resolver + Provisioners map[string]terraform.ProvisionerFactory } // initStatePaths is used to initialize the default values for diff --git a/command/plugins.go b/command/plugins.go index 9ad4b669b..fad4bae98 100644 --- a/command/plugins.go +++ b/command/plugins.go @@ -13,10 +13,14 @@ import ( "strings" plugin "github.com/hashicorp/go-plugin" + "github.com/kardianos/osext" + + terraformProvider "github.com/hashicorp/terraform/builtin/providers/terraform" tfplugin "github.com/hashicorp/terraform/plugin" "github.com/hashicorp/terraform/plugin/discovery" + "github.com/hashicorp/terraform/providers" + "github.com/hashicorp/terraform/provisioners" "github.com/hashicorp/terraform/terraform" - "github.com/kardianos/osext" ) // multiVersionProviderResolver is an implementation of @@ -31,10 +35,10 @@ type multiVersionProviderResolver struct { // (will produce an error if one is set). This should be used only in // exceptional circumstances since it forces the provider's release // schedule to be tied to that of Terraform Core. - Internal map[string]terraform.ResourceProviderFactory + Internal map[string]providers.Factory } -func choosePlugins(avail discovery.PluginMetaSet, internal map[string]terraform.ResourceProviderFactory, reqd discovery.PluginRequirements) map[string]discovery.PluginMeta { +func choosePlugins(avail discovery.PluginMetaSet, internal map[string]providers.Factory, reqd discovery.PluginRequirements) map[string]discovery.PluginMeta { candidates := avail.ConstrainVersions(reqd) ret := map[string]discovery.PluginMeta{} for name, metas := range candidates { @@ -54,8 +58,8 @@ func choosePlugins(avail discovery.PluginMetaSet, internal map[string]terraform. func (r *multiVersionProviderResolver) ResolveProviders( reqd discovery.PluginRequirements, -) (map[string]terraform.ResourceProviderFactory, []error) { - factories := make(map[string]terraform.ResourceProviderFactory, len(reqd)) +) (map[string]providers.Factory, []error) { + factories := make(map[string]providers.Factory, len(reqd)) var errs []error chosen := choosePlugins(r.Available, r.Internal, reqd) @@ -270,20 +274,18 @@ func (m *Meta) providerPluginManuallyInstalledSet() discovery.PluginMetaSet { return plugins } -func (m *Meta) providerResolver() terraform.ResourceProviderResolver { +func (m *Meta) providerResolver() providers.Resolver { return &multiVersionProviderResolver{ Available: m.providerPluginSet(), Internal: m.internalProviders(), } } -func (m *Meta) internalProviders() map[string]terraform.ResourceProviderFactory { - return map[string]terraform.ResourceProviderFactory{ - // FIXME: Re-enable this once the internal provider system is updated - // for the new provider interface. - //"terraform": func() (terraform.ResourceProvider, error) { - // return terraformProvider.Provider(), nil - //}, +func (m *Meta) internalProviders() map[string]providers.Factory { + return map[string]providers.Factory{ + "terraform": func() (providers.Interface, error) { + return terraformProvider.NewProvider(), nil + }, } } @@ -309,7 +311,7 @@ func (m *Meta) missingPlugins(avail discovery.PluginMetaSet, reqd discovery.Plug return missing } -func (m *Meta) provisionerFactories() map[string]terraform.ResourceProvisionerFactory { +func (m *Meta) provisionerFactories() map[string]terraform.ProvisionerFactory { dirs := m.pluginDirs(true) plugins := discovery.FindPlugins("provisioner", dirs) plugins, _ = plugins.ValidateVersions() @@ -320,7 +322,7 @@ func (m *Meta) provisionerFactories() map[string]terraform.ResourceProvisionerFa // name here, even though the discovery interface forces us to pretend // that might not be true. - factories := make(map[string]terraform.ResourceProvisionerFactory) + factories := make(map[string]terraform.ProvisionerFactory) // Wire up the internal provisioners first. These might be overridden // by discovered provisioners below. @@ -357,17 +359,18 @@ func internalPluginClient(kind, name string) (*plugin.Client, error) { cmdArgv := strings.Split(cmdLine, TFSPACE) cfg := &plugin.ClientConfig{ - Cmd: exec.Command(cmdArgv[0], cmdArgv[1:]...), - HandshakeConfig: tfplugin.Handshake, - Managed: true, - Plugins: tfplugin.PluginMap, + Cmd: exec.Command(cmdArgv[0], cmdArgv[1:]...), + HandshakeConfig: tfplugin.Handshake, + Managed: true, + VersionedPlugins: tfplugin.VersionedPlugins, + AllowedProtocols: []plugin.Protocol{plugin.ProtocolGRPC}, } return plugin.NewClient(cfg), nil } -func providerFactory(client *plugin.Client) terraform.ResourceProviderFactory { - return func() (terraform.ResourceProvider, error) { +func providerFactory(client *plugin.Client) providers.Factory { + return func() (providers.Interface, error) { // Request the RPC client so we can get the provider // so we can build the actual RPC-implemented provider. rpcClient, err := client.Client() @@ -380,12 +383,12 @@ func providerFactory(client *plugin.Client) terraform.ResourceProviderFactory { return nil, err } - return raw.(terraform.ResourceProvider), nil + return raw.(providers.Interface), nil } } -func provisionerFactory(client *plugin.Client) terraform.ResourceProvisionerFactory { - return func() (terraform.ResourceProvisioner, error) { +func provisionerFactory(client *plugin.Client) terraform.ProvisionerFactory { + return func() (provisioners.Interface, error) { // Request the RPC client so we can get the provisioner // so we can build the actual RPC-implemented provisioner. rpcClient, err := client.Client() @@ -398,6 +401,6 @@ func provisionerFactory(client *plugin.Client) terraform.ResourceProvisionerFact return nil, err } - return raw.(terraform.ResourceProvisioner), nil + return raw.(provisioners.Interface), nil } } diff --git a/configs/configupgrade/analysis.go b/configs/configupgrade/analysis.go index bca92d434..9b628d036 100644 --- a/configs/configupgrade/analysis.go +++ b/configs/configupgrade/analysis.go @@ -178,26 +178,22 @@ func (u *Upgrader) analyze(ms ModuleSources) (*analysis, error) { return nil, fmt.Errorf("failed to load provider %q: %s", name, err) } - // The current GetSchema interface is non-ideal. We're going to make - // this much simpler in the new interface, but we'll need to shim - // it for now. - resourceTypes := provider.Resources() - dataSources := provider.DataSources() - var resourceTypeNames, dataSourceNames []string - for _, t := range resourceTypes { - resourceTypeNames = append(resourceTypeNames, t.Name) - } - for _, t := range dataSources { - dataSourceNames = append(dataSourceNames, t.Name) - } - schema, err := provider.GetSchema(&terraform.ProviderSchemaRequest{ - DataSources: dataSourceNames, - ResourceTypes: resourceTypeNames, - }) - if err != nil { - return nil, fmt.Errorf("failed to get schema from provider %q: %s", name, err) + resp := provider.GetSchema() + if resp.Diagnostics.HasErrors() { + return nil, resp.Diagnostics.Err() } + schema := &terraform.ProviderSchema{ + Provider: resp.Provider.Block, + ResourceTypes: map[string]*configschema.Block{}, + DataSources: map[string]*configschema.Block{}, + } + for t, s := range resp.ResourceTypes { + schema.ResourceTypes[t] = s.Block + } + for t, s := range resp.DataSources { + schema.DataSources[t] = s.Block + } ret.ProviderSchemas[name] = schema } diff --git a/configs/configupgrade/upgrader.go b/configs/configupgrade/upgrader.go index dc41cd9f3..67ca6af15 100644 --- a/configs/configupgrade/upgrader.go +++ b/configs/configupgrade/upgrader.go @@ -1,12 +1,13 @@ package configupgrade import ( + "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/terraform" ) // Upgrader is the main type in this package, containing all of the // dependencies that are needed to perform upgrades. type Upgrader struct { - Providers terraform.ResourceProviderResolver - Provisioners map[string]terraform.ResourceProvisionerFactory + Providers providers.Resolver + Provisioners map[string]terraform.ProvisionerFactory } diff --git a/helper/resource/testing.go b/helper/resource/testing.go index 517b45bbb..c57588cd2 100644 --- a/helper/resource/testing.go +++ b/helper/resource/testing.go @@ -444,172 +444,176 @@ func ParallelTest(t TestT, c TestCase) { // long, we require the verbose flag so users are able to see progress // output. func Test(t TestT, c TestCase) { - // We only run acceptance tests if an env var is set because they're - // slow and generally require some outside configuration. You can opt out - // of this with OverrideEnvVar on individual TestCases. - if os.Getenv(TestEnvVar) == "" && !c.IsUnitTest { - t.Skip(fmt.Sprintf( - "Acceptance tests skipped unless env '%s' set", - TestEnvVar)) - return - } - - logWriter, err := LogOutput(t) - if err != nil { - t.Error(fmt.Errorf("error setting up logging: %s", err)) - } - log.SetOutput(logWriter) - - // We require verbose mode so that the user knows what is going on. - if !testTesting && !testing.Verbose() && !c.IsUnitTest { - t.Fatal("Acceptance tests must be run with the -v flag on tests") - return - } - - // Run the PreCheck if we have it - if c.PreCheck != nil { - c.PreCheck() - } - - providerResolver, err := testProviderResolver(c) - if err != nil { - t.Fatal(err) - } - opts := terraform.ContextOpts{ProviderResolver: providerResolver} - - // A single state variable to track the lifecycle, starting with no state - var state *terraform.State - - // Go through each step and run it - var idRefreshCheck *terraform.ResourceState - idRefresh := c.IDRefreshName != "" - errored := false - for i, step := range c.Steps { - var err error - log.Printf("[DEBUG] Test: Executing step %d", i) - - if step.SkipFunc != nil { - skip, err := step.SkipFunc() - if err != nil { - t.Fatal(err) - } - if skip { - log.Printf("[WARN] Skipping step %d", i) - continue - } + t.Fatal("resource.Test is not yet updated for the new provider API") + return + /* + // We only run acceptance tests if an env var is set because they're + // slow and generally require some outside configuration. You can opt out + // of this with OverrideEnvVar on individual TestCases. + if os.Getenv(TestEnvVar) == "" && !c.IsUnitTest { + t.Skip(fmt.Sprintf( + "Acceptance tests skipped unless env '%s' set", + TestEnvVar)) + return } - if step.Config == "" && !step.ImportState { - err = fmt.Errorf( - "unknown test mode for step. Please see TestStep docs\n\n%#v", - step) - } else { - if step.ImportState { - if step.Config == "" { - step.Config = testProviderConfig(c) - } - - // Can optionally set step.Config in addition to - // step.ImportState, to provide config for the import. - state, err = testStepImportState(opts, state, step) - } else { - state, err = testStepConfig(opts, state, step) - } - } - - // If we expected an error, but did not get one, fail - if err == nil && step.ExpectError != nil { - errored = true - t.Error(fmt.Sprintf( - "Step %d, no error received, but expected a match to:\n\n%s\n\n", - i, step.ExpectError)) - break - } - - // If there was an error, exit + logWriter, err := LogOutput(t) if err != nil { - // Perhaps we expected an error? Check if it matches - if step.ExpectError != nil { - if !step.ExpectError.MatchString(err.Error()) { - errored = true - t.Error(fmt.Sprintf( - "Step %d, expected error:\n\n%s\n\nTo match:\n\n%s\n\n", - i, err, step.ExpectError)) - break - } - } else { - errored = true - t.Error(fmt.Sprintf( - "Step %d error: %s", i, err)) - break - } + t.Error(fmt.Errorf("error setting up logging: %s", err)) + } + log.SetOutput(logWriter) + + // We require verbose mode so that the user knows what is going on. + if !testTesting && !testing.Verbose() && !c.IsUnitTest { + t.Fatal("Acceptance tests must be run with the -v flag on tests") + return } - // If we've never checked an id-only refresh and our state isn't - // empty, find the first resource and test it. - if idRefresh && idRefreshCheck == nil && !state.Empty() { - // Find the first non-nil resource in the state - for _, m := range state.Modules { - if len(m.Resources) > 0 { - if v, ok := m.Resources[c.IDRefreshName]; ok { - idRefreshCheck = v + // Run the PreCheck if we have it + if c.PreCheck != nil { + c.PreCheck() + } + + providerResolver, err := testProviderResolver(c) + if err != nil { + t.Fatal(err) + } + opts := terraform.ContextOpts{ProviderResolver: providerResolver} + + // A single state variable to track the lifecycle, starting with no state + var state *terraform.State + + // Go through each step and run it + var idRefreshCheck *terraform.ResourceState + idRefresh := c.IDRefreshName != "" + errored := false + for i, step := range c.Steps { + var err error + log.Printf("[DEBUG] Test: Executing step %d", i) + + if step.SkipFunc != nil { + skip, err := step.SkipFunc() + if err != nil { + t.Fatal(err) + } + if skip { + log.Printf("[WARN] Skipping step %d", i) + continue + } + } + + if step.Config == "" && !step.ImportState { + err = fmt.Errorf( + "unknown test mode for step. Please see TestStep docs\n\n%#v", + step) + } else { + if step.ImportState { + if step.Config == "" { + step.Config = testProviderConfig(c) } - break + // Can optionally set step.Config in addition to + // step.ImportState, to provide config for the import. + state, err = testStepImportState(opts, state, step) + } else { + state, err = testStepConfig(opts, state, step) } } - // If we have an instance to check for refreshes, do it - // immediately. We do it in the middle of another test - // because it shouldn't affect the overall state (refresh - // is read-only semantically) and we want to fail early if - // this fails. If refresh isn't read-only, then this will have - // caught a different bug. - if idRefreshCheck != nil { - log.Printf( - "[WARN] Test: Running ID-only refresh check on %s", - idRefreshCheck.Primary.ID) - if err := testIDOnlyRefresh(c, opts, step, idRefreshCheck); err != nil { - log.Printf("[ERROR] Test: ID-only test failed: %s", err) + // If we expected an error, but did not get one, fail + if err == nil && step.ExpectError != nil { + errored = true + t.Error(fmt.Sprintf( + "Step %d, no error received, but expected a match to:\n\n%s\n\n", + i, step.ExpectError)) + break + } + + // If there was an error, exit + if err != nil { + // Perhaps we expected an error? Check if it matches + if step.ExpectError != nil { + if !step.ExpectError.MatchString(err.Error()) { + errored = true + t.Error(fmt.Sprintf( + "Step %d, expected error:\n\n%s\n\nTo match:\n\n%s\n\n", + i, err, step.ExpectError)) + break + } + } else { + errored = true t.Error(fmt.Sprintf( - "[ERROR] Test: ID-only test failed: %s", err)) + "Step %d error: %s", i, err)) break } } - } - } - // If we never checked an id-only refresh, it is a failure. - if idRefresh { - if !errored && len(c.Steps) > 0 && idRefreshCheck == nil { - t.Error("ID-only refresh check never ran.") - } - } + // If we've never checked an id-only refresh and our state isn't + // empty, find the first resource and test it. + if idRefresh && idRefreshCheck == nil && !state.Empty() { + // Find the first non-nil resource in the state + for _, m := range state.Modules { + if len(m.Resources) > 0 { + if v, ok := m.Resources[c.IDRefreshName]; ok { + idRefreshCheck = v + } - // If we have a state, then run the destroy - if state != nil { - lastStep := c.Steps[len(c.Steps)-1] - destroyStep := TestStep{ - Config: lastStep.Config, - Check: c.CheckDestroy, - Destroy: true, - PreventDiskCleanup: lastStep.PreventDiskCleanup, - PreventPostDestroyRefresh: c.PreventPostDestroyRefresh, + break + } + } + + // If we have an instance to check for refreshes, do it + // immediately. We do it in the middle of another test + // because it shouldn't affect the overall state (refresh + // is read-only semantically) and we want to fail early if + // this fails. If refresh isn't read-only, then this will have + // caught a different bug. + if idRefreshCheck != nil { + log.Printf( + "[WARN] Test: Running ID-only refresh check on %s", + idRefreshCheck.Primary.ID) + if err := testIDOnlyRefresh(c, opts, step, idRefreshCheck); err != nil { + log.Printf("[ERROR] Test: ID-only test failed: %s", err) + t.Error(fmt.Sprintf( + "[ERROR] Test: ID-only test failed: %s", err)) + break + } + } + } } - log.Printf("[WARN] Test: Executing destroy step") - state, err := testStep(opts, state, destroyStep) - if err != nil { - t.Error(fmt.Sprintf( - "Error destroying resource! WARNING: Dangling resources\n"+ - "may exist. The full state and error is shown below.\n\n"+ - "Error: %s\n\nState: %s", - err, - state)) + // If we never checked an id-only refresh, it is a failure. + if idRefresh { + if !errored && len(c.Steps) > 0 && idRefreshCheck == nil { + t.Error("ID-only refresh check never ran.") + } } - } else { - log.Printf("[WARN] Skipping destroy test since there is no state.") - } + + // If we have a state, then run the destroy + if state != nil { + lastStep := c.Steps[len(c.Steps)-1] + destroyStep := TestStep{ + Config: lastStep.Config, + Check: c.CheckDestroy, + Destroy: true, + PreventDiskCleanup: lastStep.PreventDiskCleanup, + PreventPostDestroyRefresh: c.PreventPostDestroyRefresh, + } + + log.Printf("[WARN] Test: Executing destroy step") + state, err := testStep(opts, state, destroyStep) + if err != nil { + t.Error(fmt.Sprintf( + "Error destroying resource! WARNING: Dangling resources\n"+ + "may exist. The full state and error is shown below.\n\n"+ + "Error: %s\n\nState: %s", + err, + state)) + } + } else { + log.Printf("[WARN] Skipping destroy test since there is no state.") + } + */ } // testProviderConfig takes the list of Providers in a TestCase and returns a diff --git a/plans/changes.go b/plans/changes.go index a6baea27b..4f7c6f676 100644 --- a/plans/changes.go +++ b/plans/changes.go @@ -80,6 +80,14 @@ type ResourceInstanceChange struct { // Change is an embedded description of the change. Change + // RequiredReplace is a set of paths that caused the change action to be + // Replace rather than Update. Always nil if the change action is not + // Replace. + // + // This is retained only for UI-plan-rendering purposes and so it does not + // currently survive a round-trip through a saved plan file. + RequiredReplace []cty.Path + // Private allows a provider to stash any extra data that is opaque to // Terraform that relates to this change. Terraform will save this // byte-for-byte and return it to the provider in the apply call. @@ -95,11 +103,12 @@ func (rc *ResourceInstanceChange) Encode(ty cty.Type) (*ResourceInstanceChangeSr return nil, err } return &ResourceInstanceChangeSrc{ - Addr: rc.Addr, - DeposedKey: rc.DeposedKey, - ProviderAddr: rc.ProviderAddr, - ChangeSrc: *cs, - Private: rc.Private, + Addr: rc.Addr, + DeposedKey: rc.DeposedKey, + ProviderAddr: rc.ProviderAddr, + ChangeSrc: *cs, + RequiredReplace: rc.RequiredReplace, + Private: rc.Private, }, err } diff --git a/plans/changes_src.go b/plans/changes_src.go index e20eba60e..f97e5fbd4 100644 --- a/plans/changes_src.go +++ b/plans/changes_src.go @@ -34,6 +34,14 @@ type ResourceInstanceChangeSrc struct { // ChangeSrc is an embedded description of the not-yet-decoded change. ChangeSrc + // RequiredReplace is a set of paths that caused the change action to be + // Replace rather than Update. Always nil if the change action is not + // Replace. + // + // This is retained only for UI-plan-rendering purposes and so it does not + // currently survive a round-trip through a saved plan file. + RequiredReplace []cty.Path + // Private allows a provider to stash any extra data that is opaque to // Terraform that relates to this change. Terraform will save this // byte-for-byte and return it to the provider in the apply call. @@ -49,11 +57,12 @@ func (rcs *ResourceInstanceChangeSrc) Decode(ty cty.Type) (*ResourceInstanceChan return nil, err } return &ResourceInstanceChange{ - Addr: rcs.Addr, - DeposedKey: rcs.DeposedKey, - ProviderAddr: rcs.ProviderAddr, - Change: *change, - Private: rcs.Private, + Addr: rcs.Addr, + DeposedKey: rcs.DeposedKey, + ProviderAddr: rcs.ProviderAddr, + Change: *change, + RequiredReplace: rcs.RequiredReplace, + Private: rcs.Private, }, nil } diff --git a/plugin/client.go b/plugin/client.go index 7e2f4fecb..03afd5e16 100644 --- a/plugin/client.go +++ b/plugin/client.go @@ -19,11 +19,12 @@ func ClientConfig(m discovery.PluginMeta) *plugin.ClientConfig { }) return &plugin.ClientConfig{ - Cmd: exec.Command(m.Path), - HandshakeConfig: Handshake, - Managed: true, - Plugins: PluginMap, - Logger: logger, + Cmd: exec.Command(m.Path), + HandshakeConfig: Handshake, + VersionedPlugins: VersionedPlugins, + Managed: true, + Logger: logger, + AllowedProtocols: []plugin.Protocol{plugin.ProtocolGRPC}, } } diff --git a/plugin/plugin.go b/plugin/plugin.go index 00fa7b296..e4fb57761 100644 --- a/plugin/plugin.go +++ b/plugin/plugin.go @@ -6,8 +6,9 @@ import ( // See serve.go for serving plugins -// PluginMap should be used by clients for the map of plugins. -var PluginMap = map[string]plugin.Plugin{ - "provider": &ResourceProviderPlugin{}, - "provisioner": &ResourceProvisionerPlugin{}, +var VersionedPlugins = map[int]plugin.PluginSet{ + 5: { + "provider": &GRPCProviderPlugin{}, + "provisioner": &GRPCProvisionerPlugin{}, + }, } diff --git a/providers/provider.go b/providers/provider.go index 7deb608a2..7617998b2 100644 --- a/providers/provider.go +++ b/providers/provider.go @@ -1,9 +1,11 @@ package providers import ( - "github.com/hashicorp/terraform/configs/configschema" - "github.com/hashicorp/terraform/tfdiags" "github.com/zclconf/go-cty/cty" + + "github.com/hashicorp/terraform/configs/configschema" + "github.com/hashicorp/terraform/states" + "github.com/hashicorp/terraform/tfdiags" ) // Interface represents the set of methods required for a complete resource @@ -298,6 +300,24 @@ type ImportedResource struct { Private []byte } +// AsInstanceObject converts the receiving ImportedObject into a +// ResourceInstanceObject that has status ObjectReady. +// +// The returned object does not know its own resource type, so the caller must +// retain the ResourceType value from the source object if this information is +// needed. +// +// The returned object also has no dependency addresses, but the caller may +// freely modify the direct fields of the returned object without affecting +// the receiver. +func (ir ImportedResource) AsInstanceObject() *states.ResourceInstanceObject { + return &states.ResourceInstanceObject{ + Status: states.ObjectReady, + Value: ir.State, + Private: ir.Private, + } +} + type ReadDataSourceRequest struct { // TypeName is the name of the data source type to Read. TypeName string diff --git a/states/import.go b/states/import.go deleted file mode 100644 index 515f32fba..000000000 --- a/states/import.go +++ /dev/null @@ -1,40 +0,0 @@ -package states - -import ( - "github.com/zclconf/go-cty/cty" -) - -// ImportedObject represents an object being imported into Terraform with the -// help of a provider. An ImportedObject is a RemoteObject that has been read -// by the provider's import handler but hasn't yet been committed to state. -type ImportedObject struct { - ResourceType string - - // Value is the value (of the object type implied by the associated resource - // type schema) that represents this remote object in Terraform Language - // expressions and is compared with configuration when producing a diff. - Value cty.Value - - // Private corresponds to the field of the same name on - // ResourceInstanceObject, where the provider can record private data that - // will be available for future operations. - Private []byte -} - -// AsInstanceObject converts the receiving ImportedObject into a -// ResourceInstanceObject that has status ObjectReady. -// -// The returned object does not know its own resource type, so the caller must -// retain the ResourceType value from the source object if this information is -// needed. -// -// The returned object also has no dependency addresses, but the caller may -// freely modify the direct fields of the returned object without affecting -// the receiver. -func (io *ImportedObject) AsInstanceObject() *ResourceInstanceObject { - return &ResourceInstanceObject{ - Status: ObjectReady, - Value: io.Value, - Private: io.Private, - } -} diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 68ada91fc..63918483d 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -2438,8 +2438,8 @@ func TestContext2Apply_provisionerInterpCount(t *testing.T) { } // Verify apply was invoked - if !pr.ApplyCalled { - t.Fatalf("provisioner was not applied") + if !pr.ProvisionResourceCalled { + t.Fatalf("provisioner was not called") } } @@ -4308,7 +4308,7 @@ func TestContext2Apply_provisionerModule(t *testing.T) { } // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } } @@ -4361,7 +4361,7 @@ func TestContext2Apply_Provisioner_compute(t *testing.T) { } // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } } @@ -4839,7 +4839,7 @@ aws_instance.foo: `) // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } } @@ -4943,7 +4943,7 @@ func TestContext2Apply_provisionerDestroy(t *testing.T) { checkStateString(t, state, ``) // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } } @@ -5004,7 +5004,7 @@ aws_instance.foo: `) // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } } @@ -5074,7 +5074,7 @@ func TestContext2Apply_provisionerDestroyFailContinue(t *testing.T) { checkStateString(t, state, ``) // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } @@ -5153,7 +5153,7 @@ aws_instance.foo: `) // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } @@ -5235,7 +5235,7 @@ aws_instance.foo: `) // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } @@ -5303,7 +5303,7 @@ module.child: `) // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } } @@ -5377,7 +5377,7 @@ func TestContext2Apply_provisionerDestroyRef(t *testing.T) { checkStateString(t, state, ``) // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } } @@ -5483,7 +5483,7 @@ func TestContext2Apply_provisionerResourceRef(t *testing.T) { } // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } } @@ -5531,7 +5531,7 @@ func TestContext2Apply_provisionerSelfRef(t *testing.T) { } // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } } @@ -5586,7 +5586,7 @@ func TestContext2Apply_provisionerMultiSelfRef(t *testing.T) { } // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } @@ -5648,7 +5648,7 @@ func TestContext2Apply_provisionerMultiSelfRefSingle(t *testing.T) { } // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } @@ -5700,7 +5700,7 @@ func TestContext2Apply_provisionerExplicitSelfRef(t *testing.T) { } // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } } @@ -5772,10 +5772,10 @@ func TestContext2Apply_Provisioner_Diff(t *testing.T) { } // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } - pr.ApplyCalled = false + pr.ProvisionResourceCalled = false // Change the state to force a diff mod := state.RootModule() @@ -5820,7 +5820,7 @@ func TestContext2Apply_Provisioner_Diff(t *testing.T) { } // Verify apply was NOT invoked - if pr.ApplyCalled { + if pr.ProvisionResourceCalled { t.Fatalf("provisioner invoked") } } @@ -5972,7 +5972,7 @@ func TestContext2Apply_Provisioner_ConnInfo(t *testing.T) { } // Verify apply was invoked - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatalf("provisioner not invoked") } } @@ -7687,7 +7687,7 @@ func TestContext2Apply_destroyProvisionerWithLocals(t *testing.T) { t.Fatal(diags.Err()) } - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatal("provisioner not called") } } @@ -7767,7 +7767,7 @@ func TestContext2Apply_destroyProvisionerWithMultipleLocals(t *testing.T) { t.Fatal(diags.Err()) } - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatal("provisioner not called") } } @@ -7845,7 +7845,7 @@ func TestContext2Apply_destroyProvisionerWithOutput(t *testing.T) { if diags.HasErrors() { t.Fatal(diags.Err()) } - if !pr.ApplyCalled { + if !pr.ProvisionResourceCalled { t.Fatal("provisioner not called") } diff --git a/terraform/context_input_test.go b/terraform/context_input_test.go index 2256c415e..75de24036 100644 --- a/terraform/context_input_test.go +++ b/terraform/context_input_test.go @@ -79,10 +79,6 @@ func TestContext2Input_moduleComputedOutputElement(t *testing.T) { ), }) - p.InputFn = func(i UIInput, c *ResourceConfig) (*ResourceConfig, error) { - return c, nil - } - if diags := ctx.Input(InputModeStd); diags.HasErrors() { t.Fatalf("input errors: %s", diags.Err()) } @@ -102,11 +98,6 @@ func TestContext2Input_badVarDefault(t *testing.T) { ), }) - p.InputFn = func(i UIInput, c *ResourceConfig) (*ResourceConfig, error) { - c.Config["foo"] = "bar" - return c, nil - } - if diags := ctx.Input(InputModeStd); diags.HasErrors() { t.Fatalf("input errors: %s", diags.Err()) } diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go index 00aeb211d..2423c0d46 100644 --- a/terraform/context_validate_test.go +++ b/terraform/context_validate_test.go @@ -162,7 +162,7 @@ func TestContext2Validate_computedVar(t *testing.T) { return nil, c.CheckSet([]string{"value"}) } - p.ConfigureFn = func(providers.ConfigRequest) error { + p.ConfigureFn = func(c *ResourceConfig) error { return fmt.Errorf("Configure should not be called for provider") } diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go index 600f6fd0f..c96f98787 100644 --- a/terraform/eval_apply.go +++ b/terraform/eval_apply.go @@ -5,95 +5,152 @@ import ( "log" "github.com/hashicorp/go-multierror" + "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" + "github.com/hashicorp/terraform/tfdiags" ) // EvalApply is an EvalNode implementation that writes the diff to // the full diff. type EvalApply struct { - Addr addrs.ResourceInstance - State **states.ResourceInstanceObject - Change **plans.ResourceInstanceChange - Provider *providers.Interface - Output **states.ResourceInstanceObject - CreateNew *bool - Error *error + Addr addrs.ResourceInstance + Config *configs.Resource + State **states.ResourceInstanceObject + Change **plans.ResourceInstanceChange + ProviderAddr addrs.AbsProviderConfig + Provider *providers.Interface + ProviderSchema **ProviderSchema + Output **states.ResourceInstanceObject + CreateNew *bool + Error *error } // TODO: test func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) { - return nil, fmt.Errorf("EvalApply is not yet updated for the new state and plan types") - /* - diff := *n.Diff - provider := *n.Provider - state := *n.State + var diags tfdiags.Diagnostics - // The provider API still expects our legacy InstanceInfo type, so we must shim it. - legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path())) + change := *n.Change + provider := *n.Provider + state := *n.State + absAddr := n.Addr.Absolute(ctx.Path()) - if state == nil { - state = &states.ResourceInstanceObject{} - } + if state == nil { + state = &states.ResourceInstanceObject{} + } - // Flag if we're creating a new instance - if n.CreateNew != nil { - *n.CreateNew = state.ID == "" && !diff.GetDestroy() || diff.RequiresNew() - } + schema := (*n.ProviderSchema).ResourceTypes[n.Addr.Resource.Type] + if schema == nil { + // Should be caught during validation, so we don't bother with a pretty error here + return nil, fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Type) + } - // With the completed diff, apply! - log.Printf("[DEBUG] apply %s: executing Apply", n.Addr) - state, err := provider.Apply(legacyInfo, state, diff) - if state == nil { - state = new(InstanceState) - } - state.init() + if n.CreateNew != nil { + *n.CreateNew = (change.Action == plans.Create || change.Action == plans.Replace) + } - // Force the "id" attribute to be our ID - if state.ID != "" { - state.Attributes["id"] = state.ID - } + configVal := cty.NullVal(cty.DynamicPseudoType) // TODO: Populate this when n.Config is non-nil; will need config and provider schema in here - // If the value is the unknown variable value, then it is an error. - // In this case we record the error and remove it from the state - for ak, av := range state.Attributes { - if av == config.UnknownVariableValue { - err = multierror.Append(err, fmt.Errorf( - "Attribute with unknown value: %s", ak)) - delete(state.Attributes, ak) + log.Printf("[DEBUG] %s: applying the planned %s change", n.Addr.Absolute(ctx.Path()), change.Action) + resp := provider.ApplyResourceChange(providers.ApplyResourceChangeRequest{ + TypeName: n.Addr.Resource.Type, + PriorState: change.Before, + Config: configVal, // TODO + PlannedState: change.After, + PlannedPrivate: change.Private, // TODO + }) + applyDiags := resp.Diagnostics + if n.Config != nil { + applyDiags = applyDiags.InConfigBody(n.Config.Config) + } + diags = diags.Append(applyDiags) + + // Even if there are errors in the returned diagnostics, the provider may + // have returned a _partial_ state for an object that already exists but + // failed to fully configure, and so the remaining code must always run + // to completion but must be defensive against the new value being + // incomplete. + newVal := resp.NewState + + for _, err := range schema.ImpliedType().TestConformance(newVal.Type()) { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider produced invalid object", + fmt.Sprintf( + "Provider %q planned an invalid value for %s%s after apply, and so the result could not be saved.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.", + n.ProviderAddr.ProviderConfig.Type, absAddr, tfdiags.FormatError(err), + ), + )) + } + if diags.HasErrors() { + return nil, diags.Err() + } + + // By this point there must not be any unknown values remaining in our + // object, because we've applied the change and we can't save unknowns + // in our persistent state. If any are present then we will indicate an + // error (which is always a bug in the provider) but we will also replace + // them with nulls so that we can successfully save the portions of the + // returned value that are known. + if !newVal.IsWhollyKnown() { + // To generate better error messages, we'll go for a walk through the + // value and make a separate diagnostic for each unknown value we + // find. + cty.Walk(newVal, func(path cty.Path, val cty.Value) (bool, error) { + if !val.IsKnown() { + pathStr := tfdiags.FormatCtyPath(path) + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider returned invalid result object after apply", + fmt.Sprintf( + "After the apply operation, the provider still indicated an unknown value for %s%s. All values must be known after apply, so this is always a bug in the provider and should be reported in the provider's own repository. Terraform will still save the other known object values in the state.", + n.Addr.Absolute(ctx.Path()), pathStr, + ), + )) } - } + return true, nil + }) - // If the provider produced an InstanceState with an empty id then - // that really means that there's no state at all. - // FIXME: Change the provider protocol so that the provider itself returns - // a null in this case, and stop treating the ID as special. - if state.ID == "" { - state = nil - } + // NOTE: This operation can potentially be lossy if there are multiple + // elements in a set that differ only by unknown values: after + // replacing with null these will be merged together into a single set + // element. Since we can only get here in the presence of a provider + // bug, we accept this because storing a result here is always a + // best-effort sort of thing. + newVal = cty.UnknownAsNull(newVal) + } - // Write the final state - if n.Output != nil { - *n.Output = state + var newState *states.ResourceInstanceObject + if !newVal.IsNull() { // null value indicates that the object is deleted, so we won't set a new state in that case + newState = &states.ResourceInstanceObject{ + Status: states.ObjectReady, // TODO: Consider marking as tainted if the provider returned errors? + Value: newVal, + Private: resp.Private, + Dependencies: nil, // not populated here; this will be mutated by a later eval step } + } - // If there are no errors, then we append it to our output error - // if we have one, otherwise we just output it. - if err != nil { - if n.Error != nil { - helpfulErr := fmt.Errorf("%s: %s", n.Addr, err.Error()) - *n.Error = multierror.Append(*n.Error, helpfulErr) - } else { - return nil, err - } + // Write the final state + if n.Output != nil { + *n.Output = newState + } + + if diags.HasErrors() { + // If the caller provided an error pointer then they are expected to + // handle the error some other way and we treat our own result as + // success. + if n.Error != nil { + err := diags.Err() + n.Error = &err + return nil, nil } + } - return nil, nil - */ + return nil, diags.ErrWithWarnings() } // EvalApplyPre is an EvalNode implementation that does the pre-Apply work diff --git a/terraform/eval_context_builtin_test.go b/terraform/eval_context_builtin_test.go index 340af1ee8..a0d7bed94 100644 --- a/terraform/eval_context_builtin_test.go +++ b/terraform/eval_context_builtin_test.go @@ -46,20 +46,7 @@ func TestBuiltinEvalContextProviderInput(t *testing.T) { func TestBuildingEvalContextInitProvider(t *testing.T) { var lock sync.Mutex - testP := &MockProvider{ - ResourcesReturn: []ResourceType{ - { - Name: "test_thing", - SchemaAvailable: true, - }, - }, - DataSourcesReturn: []DataSource{ - { - Name: "test_thing", - SchemaAvailable: true, - }, - }, - } + testP := &MockProvider{} ctx := testBuiltinEvalContext(t) ctx.ProviderLock = &lock diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index 7f78f1573..b5a77b2bc 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "log" + "reflect" "strings" "github.com/hashicorp/hcl2/hcl" @@ -11,78 +12,67 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" + "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/plans" + "github.com/hashicorp/terraform/plans/objchange" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" + "github.com/hashicorp/terraform/tfdiags" ) -// EvalCompareDiff is an EvalNode implementation that compares two diffs -// and errors if the diffs are not equal. -type EvalCompareDiff struct { - Addr addrs.ResourceInstance - One, Two **plans.ResourceInstanceChange +// EvalCheckPlannedChange is an EvalNode implementation that produces errors +// if the _actual_ expected value is not compatible with what was recorded +// in the plan. +// +// Errors here are most often indicative of a bug in the provider, so our +// error messages will report with that in mind. It's also possible that +// there's a bug in Terraform's Core's own "proposed new value" code in +// EvalDiff. +type EvalCheckPlannedChange struct { + Addr addrs.ResourceInstance + ProviderAddr addrs.AbsProviderConfig + ProviderSchema **ProviderSchema + + // We take ResourceInstanceChange objects here just because that's what's + // convenient to pass in from the evaltree implementation, but we really + // only look at the "After" value of each change. + Planned, Actual **plans.ResourceInstanceChange } -// TODO: test -func (n *EvalCompareDiff) Eval(ctx EvalContext) (interface{}, error) { - return nil, fmt.Errorf("TODO: Replace EvalCompareDiff with EvalCheckPlannedState") - /* - one, two := *n.One, *n.Two +func (n *EvalCheckPlannedChange) Eval(ctx EvalContext) (interface{}, error) { + providerSchema := *n.ProviderSchema + plannedChange := *n.Planned + actualChange := *n.Actual - // If either are nil, let them be empty - if one == nil { - one = new(InstanceDiff) - one.init() - } - if two == nil { - two = new(InstanceDiff) - two.init() - } - oneId, _ := one.GetAttribute("id") - twoId, _ := two.GetAttribute("id") - one.DelAttribute("id") - two.DelAttribute("id") - defer func() { - if oneId != nil { - one.SetAttribute("id", oneId) - } - if twoId != nil { - two.SetAttribute("id", twoId) - } - }() + schema := providerSchema.ResourceTypes[n.Addr.Resource.Type] + if schema == nil { + // Should be caught during validation, so we don't bother with a pretty error here + return nil, fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Type) + } - if same, reason := one.Same(two); !same { - log.Printf("[ERROR] %s: diffs didn't match", n.Addr) - log.Printf("[ERROR] %s: reason: %s", n.Addr, reason) - log.Printf("[ERROR] %s: diff one: %#v", n.Addr, one) - log.Printf("[ERROR] %s: diff two: %#v", n.Addr, two) - return nil, fmt.Errorf( - "%s: diffs didn't match during apply. This is a bug with "+ - "Terraform and should be reported as a GitHub Issue.\n"+ - "\n"+ - "Please include the following information in your report:\n"+ - "\n"+ - " Terraform Version: %s\n"+ - " Resource ID: %s\n"+ - " Mismatch reason: %s\n"+ - " Diff One (usually from plan): %#v\n"+ - " Diff Two (usually from apply): %#v\n"+ - "\n"+ - "Also include as much context as you can about your config, state, "+ - "and the steps you performed to trigger this error.\n", - n.Addr, version.Version, n.Addr, reason, one, two) - } - - return nil, nil - */ + absAddr := n.Addr.Absolute(ctx.Path()) + var diags tfdiags.Diagnostics + errs := objchange.AssertObjectCompatible(schema, plannedChange.After, actualChange.After) + for _, err := range errs { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider produced inconsistent final plan", + fmt.Sprintf( + "When expanding the plan for %s to include new values learned so far during apply, provider %q produced an invalid new value for %s.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.", + absAddr, n.ProviderAddr.ProviderConfig.Type, tfdiags.FormatError(err), + ), + )) + } + return nil, diags.Err() } -// EvalDiff is an EvalNode implementation that does a refresh for -// a resource. +// EvalDiff is an EvalNode implementation that detects changes for a given +// resource instance. type EvalDiff struct { Addr addrs.ResourceInstance Config *configs.Resource Provider *providers.Interface + ProviderAddr addrs.AbsProviderConfig ProviderSchema **ProviderSchema State **states.ResourceInstanceObject PreviousDiff **plans.ResourceInstanceChange @@ -96,146 +86,319 @@ type EvalDiff struct { // TODO: test func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { - return nil, fmt.Errorf("EvalDiff not yet updated for new state and plan types") - /* - state := *n.State - config := *n.Config - provider := *n.Provider - providerSchema := *n.ProviderSchema + state := *n.State + config := *n.Config + provider := *n.Provider + providerSchema := *n.ProviderSchema - if providerSchema == nil { - return nil, fmt.Errorf("provider schema is unavailable for %s", n.Addr) - } + if providerSchema == nil { + return nil, fmt.Errorf("provider schema is unavailable for %s", n.Addr) + } - var diags tfdiags.Diagnostics + var diags tfdiags.Diagnostics - // The provider and hook APIs still expect our legacy InstanceInfo type. - legacyInfo := NewInstanceInfo(n.Addr.Absolute(ctx.Path())) + absAddr := n.Addr.Absolute(ctx.Path()) + priorVal := state.Value - // State still uses legacy-style internal ids, so we need to shim to get - // a suitable key to use. - stateId := NewLegacyResourceInstanceAddress(n.Addr.Absolute(ctx.Path())).stateId() + // Evaluate the configuration + schema := providerSchema.ResourceTypes[n.Addr.Resource.Type] + if schema == nil { + // Should be caught during validation, so we don't bother with a pretty error here + return nil, fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Type) + } + keyData := EvalDataForInstanceKey(n.Addr.Key) + configVal, _, configDiags := ctx.EvaluateBlock(config.Config, schema, nil, keyData) + diags = diags.Append(configDiags) + if configDiags.HasErrors() { + return nil, diags.Err() + } - // Call pre-diff hook - if !n.Stub { - err := ctx.Hook(func(h Hook) (HookAction, error) { - return h.PreDiff(legacyInfo, state) - }) - if err != nil { - return nil, err - } - } + proposedNewVal := objchange.ProposedNewObject(schema, priorVal, configVal) - // The state for the diff must never be nil - diffState := state - if diffState == nil { - diffState = new(InstanceState) - } - diffState.init() - - // Evaluate the configuration - schema := providerSchema.ResourceTypes[n.Addr.Resource.Type] - if schema == nil { - // Should be caught during validation, so we don't bother with a pretty error here - return nil, fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Type) - } - keyData := EvalDataForInstanceKey(n.Addr.Key) - configVal, _, configDiags := ctx.EvaluateBlock(config.Config, schema, nil, keyData) - diags = diags.Append(configDiags) - if configDiags.HasErrors() { - return nil, diags.Err() - } - - // The provider API still expects our legacy ResourceConfig type. - legacyRC := NewResourceConfigShimmed(configVal, schema) - - // Diff! - diff, err := provider.Diff(legacyInfo, diffState, legacyRC) - if err != nil { - return nil, err - } - if diff == nil { - diff = new(InstanceDiff) - } - - // Set DestroyDeposed if we have deposed instances - _, err = readInstanceFromState(ctx, stateId, nil, func(rs *ResourceState) (*InstanceState, error) { - if len(rs.Deposed) > 0 { - diff.DestroyDeposed = true - } - - return nil, nil + // Call pre-diff hook + if !n.Stub { + err := ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreDiff(absAddr, states.CurrentGen, priorVal, proposedNewVal) }) if err != nil { return nil, err } + } - // Preserve the DestroyTainted flag - if n.PreviousDiff != nil { - diff.SetTainted((*n.PreviousDiff).GetDestroyTainted()) + // The provider gets an opportunity to customize the proposed new value, + // which in turn produces the _planned_ new value. + resp := provider.PlanResourceChange(providers.PlanResourceChangeRequest{ + TypeName: n.Addr.Resource.Type, + Config: configVal, + PriorState: priorVal, + ProposedNewState: proposedNewVal, + PriorPrivate: state.Private, + }) + diags = diags.Append(resp.Diagnostics.InConfigBody(config.Config)) + if diags.HasErrors() { + return nil, diags.Err() + } + + plannedNewVal := resp.PlannedState + plannedPrivate := resp.PlannedPrivate + + // We allow the planned new value to disagree with configuration _values_ + // here, since that allows the provider to do special logic like a + // DiffSuppressFunc, but we still require that the provider produces + // a value whose type conforms to the schema. + for _, err := range schema.ImpliedType().TestConformance(plannedNewVal.Type()) { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider produced invalid plan", + fmt.Sprintf( + "Provider %q planned an invalid value for %s%s.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.", + n.ProviderAddr.ProviderConfig.Type, absAddr, tfdiags.FormatError(err), + ), + )) + } + if diags.HasErrors() { + return nil, diags.Err() + } + + { + var moreDiags tfdiags.Diagnostics + plannedNewVal, moreDiags = n.processIgnoreChanges(schema, priorVal, plannedNewVal) + diags = diags.Append(moreDiags) + if moreDiags.HasErrors() { + return nil, diags.Err() } + } - // Require a destroy if there is an ID and it requires new. - if diff.RequiresNew() && state != nil && state.ID != "" { - diff.SetDestroy(true) - } - - // If we're creating a new resource, compute its ID - if diff.RequiresNew() || state == nil || state.ID == "" { - var oldID string - if state != nil { - oldID = state.Attributes["id"] + // The provider produces a list of paths to attributes whose changes mean + // that we must replace rather than update an existing remote object. + // However, we only need to do that if the identified attributes _have_ + // actually changed -- particularly after we may have undone some of the + // changes in processIgnoreChanges -- so now we'll filter that list to + // include only where changes are detected. + var reqRep []cty.Path + if len(resp.RequiresReplace) > 0 { + reqRep = make([]cty.Path, 0, len(resp.RequiresReplace)) + for _, path := range resp.RequiresReplace { + if priorVal.IsNull() { + // If prior is null then we don't expect any RequiresReplace at all, + // because this is a Create action. (This is just to avoid errors + // when we use this value below, if the provider misbehaves.) + continue + } + plannedChangedVal, err := path.Apply(plannedNewVal) + if err != nil { + // This always indicates a provider bug, since RequiresReplace + // should always refer only to whole attributes (and not into + // attribute values themselves) and these should always be + // present, even though they might be null or unknown. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider produced invalid plan", + fmt.Sprintf( + "Provider %q has indicated \"requires replacement\" on %s for a non-existent attribute path %#v.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.", + n.ProviderAddr.ProviderConfig.Type, absAddr, path, + ), + )) + continue + } + priorChangedVal, err := path.Apply(priorVal) + if err != nil { + // Should never happen since prior and changed should be of + // the same type, but we'll allow it for robustness. + reqRep = append(reqRep, path) + } + eqV := plannedChangedVal.Equals(priorChangedVal) + if !eqV.IsKnown() || eqV.False() { + reqRep = append(reqRep, path) } - - // Add diff to compute new ID - diff.init() - diff.SetAttribute("id", &ResourceAttrDiff{ - Old: oldID, - NewComputed: true, - RequiresNew: true, - Type: DiffAttrOutput, - }) } + if diags.HasErrors() { + return nil, diags.Err() + } + } - // filter out ignored attributes - if err := n.processIgnoreChanges(diff); err != nil { + eqV := plannedNewVal.Equals(priorVal) + eq := eqV.IsKnown() && eqV.True() + + var action plans.Action + switch { + case priorVal.IsNull(): + action = plans.Create + case eq: + action = plans.NoOp + case len(reqRep) > 0: + // If there are any "requires replace" paths left _after our filtering + // above_ then this is a replace action. + action = plans.Replace + default: + action = plans.Update + // "Delete" is never chosen here, because deletion plans are always + // created more directly elsewhere, such as in "orphan" handling. + } + + if action == plans.Replace { + // In this strange situation we want to produce a change object that + // shows our real prior object but has a _new_ object that is built + // from a null prior object, since we're going to delete the one + // that has all the computed values on it. + // + // Therefore we'll ask the provider to plan again here, giving it + // a null object for the prior, and then we'll meld that with the + // _actual_ prior state to produce a correctly-shaped replace change. + // The resulting change should show any computed attributes changing + // from known prior values to unknown values, unless the provider is + // able to predict new values for any of these computed attributes. + nullPriorVal := cty.NullVal(schema.ImpliedType()) + resp = provider.PlanResourceChange(providers.PlanResourceChangeRequest{ + TypeName: n.Addr.Resource.Type, + Config: configVal, + PriorState: nullPriorVal, + ProposedNewState: configVal, + PriorPrivate: plannedPrivate, + }) + // We need to tread carefully here, since if there are any warnings + // in here they probably also came out of our previous call to + // PlanResourceChange above, and so we don't want to repeat them. + // Consequently, we break from the usual pattern here and only + // append these new diagnostics if there's at least one error inside. + if resp.Diagnostics.HasErrors() { + diags = diags.Append(resp.Diagnostics.InConfigBody(config.Config)) + return nil, diags.Err() + } + plannedNewVal = resp.PlannedState + plannedPrivate = resp.PlannedPrivate + for _, err := range schema.ImpliedType().TestConformance(plannedNewVal.Type()) { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider produced invalid plan", + fmt.Sprintf( + "Provider %q planned an invalid value for %s%s.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.", + n.ProviderAddr.ProviderConfig.Type, absAddr, tfdiags.FormatError(err), + ), + )) + } + if diags.HasErrors() { + return nil, diags.Err() + } + } + + // Call post-refresh hook + if !n.Stub { + err := ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostDiff(absAddr, states.CurrentGen, action, priorVal, plannedNewVal) + }) + if err != nil { return nil, err } + } - // Call post-refresh hook - if !n.Stub { - err = ctx.Hook(func(h Hook) (HookAction, error) { - return h.PostDiff(legacyInfo, diff) - }) - if err != nil { - return nil, err - } + // Update our output if we care + if n.OutputChange != nil { + *n.OutputChange = &plans.ResourceInstanceChange{ + Addr: absAddr, + Private: plannedPrivate, + ProviderAddr: n.ProviderAddr, + Change: plans.Change{ + Action: action, + Before: priorVal, + After: plannedNewVal, + }, + RequiredReplace: reqRep, } + } - // Update our output if we care - if n.OutputDiff != nil { - *n.OutputDiff = diff + if n.OutputValue != nil { + *n.OutputValue = configVal + } + + // Update the state if we care + if n.OutputState != nil { + *n.OutputState = &states.ResourceInstanceObject{ + Status: states.ObjectReady, + Value: plannedNewVal, } + } - if n.OutputValue != nil { - *n.OutputValue = configVal - } - - // Update the state if we care - if n.OutputState != nil { - *n.OutputState = state - - // Merge our state so that the state is updated with our plan - if !diff.Empty() && n.OutputState != nil { - *n.OutputState = state.MergeDiff(diff) - } - } - - return nil, nil - */ + return nil, nil } -func (n *EvalDiff) processIgnoreChanges(diff *InstanceDiff) error { +func (n *EvalDiff) processIgnoreChanges(schema *configschema.Block, prior, proposed cty.Value) (cty.Value, tfdiags.Diagnostics) { + ignoreChanges := n.Config.Managed.IgnoreChanges + ignoreAll := n.Config.Managed.IgnoreAllChanges + + if len(ignoreChanges) == 0 && !ignoreAll { + return proposed, nil + } + if ignoreAll { + return prior, nil + } + if prior.IsNull() || proposed.IsNull() { + // Ignore changes doesn't apply when we're creating for the first time. + // Proposed should never be null here, but if it is then we'll just let it be. + return proposed, nil + } + + // When we walk below we will be using cty.Path values for comparison, so + // we'll convert our traversals here so we can compare more easily. + ignoreChangesPath := make([]cty.Path, len(ignoreChanges)) + for i, traversal := range ignoreChanges { + path := make(cty.Path, len(traversal)) + for si, step := range traversal { + switch ts := step.(type) { + case hcl.TraverseRoot: + path[si] = cty.GetAttrStep{ + Name: ts.Name, + } + case hcl.TraverseAttr: + path[si] = cty.GetAttrStep{ + Name: ts.Name, + } + case hcl.TraverseIndex: + path[si] = cty.IndexStep{ + Key: ts.Key, + } + default: + panic(fmt.Sprintf("unsupported traversal step %#v", step)) + } + } + ignoreChangesPath[i] = path + } + + var diags tfdiags.Diagnostics + ret, _ := cty.Transform(proposed, func(path cty.Path, v cty.Value) (cty.Value, error) { + // First we must see if this is a path that's being ignored at all. + // We're looking for an exact match here because this walk will visit + // leaf values first and then their containers, and we want to do + // the "ignore" transform once we reach the point indicated, throwing + // away any deeper values we already produced at that point. + var ignoreTraversal hcl.Traversal + for i, candidate := range ignoreChangesPath { + if reflect.DeepEqual(path, candidate) { + ignoreTraversal = ignoreChanges[i] + } + } + if ignoreTraversal == nil { + return v, nil + } + + // If we're able to follow the same path through the prior value, + // we'll take the value there instead, effectively undoing the + // change that was planned. + priorV, err := path.Apply(prior) + if err != nil { + // We just ignore the error and move on here, since we assume it's + // just because the prior value was a slightly-different shape. + // It could potentially also be that the traversal doesn't match + // the schema, but we should've caught that during the validate + // walk if so. + return v, nil + } + return priorV, nil + }) + return ret, diags +} + +func (n *EvalDiff) processIgnoreChangesOld(diff *InstanceDiff) error { if diff == nil || n.Config == nil || n.Config.Managed == nil { return nil } diff --git a/terraform/eval_import_state.go b/terraform/eval_import_state.go index e9c85cf9f..a60f4a0a2 100644 --- a/terraform/eval_import_state.go +++ b/terraform/eval_import_state.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "log" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/providers" @@ -15,57 +16,56 @@ import ( type EvalImportState struct { Addr addrs.ResourceInstance Provider *providers.Interface - Id string - Output *[]*states.ImportedObject + ID string + Output *[]providers.ImportedResource } // TODO: test func (n *EvalImportState) Eval(ctx EvalContext) (interface{}, error) { - return nil, fmt.Errorf("EvalImportState not yet updated for new state/provider types") - /* - absAddr := n.Addr.Absolute(ctx.Path()) - provider := *n.Provider + 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 - } - } - - // Import! - state, err := provider.ImportState(n.Info, n.Id) + { + // Call pre-import hook + err := ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreImportState(absAddr, n.ID) + }) if err != nil { - return nil, fmt.Errorf("import %s (id: %s): %s", absAddr.String(), n.Id, err) + return nil, err } + } - for _, s := range state { - if s == nil { - log.Printf("[TRACE] EvalImportState: import %s %q produced a nil state", absAddr.String(), n.Id) - continue - } - log.Printf("[TRACE] EvalImportState: import %s %q produced state for %s with id %q", absAddr.String(), n.Id, s.Ephemeral.Type, s.ID) + 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 } + } - if n.Output != nil { - *n.Output = state - } - - { - // Call post-import hook - err := ctx.Hook(func(h Hook) (HookAction, error) { - return h.PostImportState(n.Info, state) - }) - if err != nil { - return nil, err - } - } - - return nil, nil - */ + return nil, nil } // EvalImportStateVerify verifies the state after ImportState and diff --git a/terraform/eval_refresh.go b/terraform/eval_refresh.go index 0a3b0e067..1d4e87af0 100644 --- a/terraform/eval_refresh.go +++ b/terraform/eval_refresh.go @@ -7,15 +7,18 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" + "github.com/hashicorp/terraform/tfdiags" ) // EvalRefresh is an EvalNode implementation that does a refresh for // a resource. type EvalRefresh struct { - Addr addrs.ResourceInstance - Provider *providers.Interface - State **states.ResourceInstanceObject - Output **states.ResourceInstanceObject + Addr addrs.ResourceInstance + ProviderAddr addrs.AbsProviderConfig + Provider *providers.Interface + ProviderSchema **ProviderSchema + State **states.ResourceInstanceObject + Output **states.ResourceInstanceObject } // TODO: test @@ -23,10 +26,18 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { state := *n.State absAddr := n.Addr.Absolute(ctx.Path()) + var diags tfdiags.Diagnostics + // If we have no state, we don't do any refreshing if state == nil { log.Printf("[DEBUG] refresh: %s: no state, so not refreshing", n.Addr.Absolute(ctx.Path())) - return nil, nil + return nil, diags.ErrWithWarnings() + } + + schema := (*n.ProviderSchema).ResourceTypes[n.Addr.Resource.Type] + if schema == nil { + // Should be caught during validation, so we don't bother with a pretty error here + return nil, fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Type) } // Call pre-refresh hook @@ -34,7 +45,7 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { return h.PreRefresh(absAddr, states.CurrentGen, state.Value) }) if err != nil { - return nil, err + return nil, diags.ErrWithWarnings() } // Refresh! @@ -46,8 +57,23 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { provider := *n.Provider resp := provider.ReadResource(req) - if resp.Diagnostics.HasErrors() { - return nil, fmt.Errorf("%s: %s", n.Addr.Absolute(ctx.Path()), resp.Diagnostics.Err()) + diags = diags.Append(resp.Diagnostics) + if diags.HasErrors() { + return nil, diags.Err() + } + + for _, err := range schema.ImpliedType().TestConformance(resp.NewState.Type()) { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider produced invalid object", + fmt.Sprintf( + "Provider %q planned an invalid value for %s%s during refresh.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.", + n.ProviderAddr.ProviderConfig.Type, absAddr, tfdiags.FormatError(err), + ), + )) + } + if diags.HasErrors() { + return nil, diags.Err() } state.Value = resp.NewState @@ -64,5 +90,5 @@ func (n *EvalRefresh) Eval(ctx EvalContext) (interface{}, error) { *n.Output = state } - return nil, nil + return nil, diags.ErrWithWarnings() } diff --git a/terraform/hook.go b/terraform/hook.go index fd8965cde..c0bb23ab2 100644 --- a/terraform/hook.go +++ b/terraform/hook.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/plans" + "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" ) @@ -73,7 +74,7 @@ type Hook interface { // PreImportState and PostImportState are called before and after // (respectively) each state import operation for a given resource address. PreImportState(addr addrs.AbsResourceInstance, importID string) (HookAction, error) - PostImportState(addr addrs.AbsResourceInstance, imported []*states.ImportedObject) (HookAction, error) + PostImportState(addr addrs.AbsResourceInstance, imported []providers.ImportedResource) (HookAction, error) // PostStateUpdate is called each time the state is updated. It receives // a deep copy of the state, which it may therefore access freely without @@ -135,7 +136,7 @@ func (*NilHook) PreImportState(addr addrs.AbsResourceInstance, importID string) return HookActionContinue, nil } -func (*NilHook) PostImportState(addr addrs.AbsResourceInstance, imported []*states.ImportedObject) (HookAction, error) { +func (*NilHook) PostImportState(addr addrs.AbsResourceInstance, imported []providers.ImportedResource) (HookAction, error) { return HookActionContinue, nil } diff --git a/terraform/hook_mock.go b/terraform/hook_mock.go index 9585bf80e..6efa31963 100644 --- a/terraform/hook_mock.go +++ b/terraform/hook_mock.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/plans" + "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" ) @@ -103,7 +104,7 @@ type MockHook struct { PostImportStateCalled bool PostImportStateAddr addrs.AbsResourceInstance - PostImportStateNewStates []*states.ImportedObject + PostImportStateNewStates []providers.ImportedResource PostImportStateReturn HookAction PostImportStateError error @@ -253,7 +254,7 @@ func (h *MockHook) PreImportState(addr addrs.AbsResourceInstance, importID strin return h.PreImportStateReturn, h.PreImportStateError } -func (h *MockHook) PostImportState(addr addrs.AbsResourceInstance, imported []*states.ImportedObject) (HookAction, error) { +func (h *MockHook) PostImportState(addr addrs.AbsResourceInstance, imported []providers.ImportedResource) (HookAction, error) { h.Lock() defer h.Unlock() diff --git a/terraform/hook_stop.go b/terraform/hook_stop.go index 917a12681..72c004d33 100644 --- a/terraform/hook_stop.go +++ b/terraform/hook_stop.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/plans" + "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" ) @@ -65,7 +66,7 @@ func (h *stopHook) PreImportState(addr addrs.AbsResourceInstance, importID strin return h.hook() } -func (h *stopHook) PostImportState(addr addrs.AbsResourceInstance, imported []*states.ImportedObject) (HookAction, error) { +func (h *stopHook) PostImportState(addr addrs.AbsResourceInstance, imported []providers.ImportedResource) (HookAction, error) { return h.hook() } diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index ecf8a3450..80a0bce33 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -262,6 +262,7 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe Addr: addr.Resource, Config: n.Config, Provider: &provider, + ProviderAddr: n.ResolvedProvider, ProviderSchema: &providerSchema, State: &state, OutputChange: &diffApply, @@ -276,10 +277,11 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe }, // Compare the diffs - &EvalCompareDiff{ - Addr: addr.Resource, - One: &diff, - Two: &diffApply, + &EvalCheckPlannedChange{ + Addr: addr.Resource, + ProviderAddr: n.ResolvedProvider, + Planned: &diff, + Actual: &diffApply, }, &EvalGetProvider{ @@ -303,6 +305,7 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe }, &EvalApply{ Addr: addr.Resource, + Config: n.Config, State: &state, Change: &diffApply, Provider: &provider, diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index cb820a0af..05733889f 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -265,6 +265,7 @@ func (n *NodeDestroyResourceInstance) EvalTree() EvalNode { }, Else: &EvalApply{ Addr: addr.Resource, + Config: nil, // No configuration because we are destroying State: &state, Change: &changeApply, Provider: &provider, diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index d4c0d4859..217a4eaa7 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -139,6 +139,7 @@ func (n *NodePlannableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe Addr: addr.Resource, Config: n.Config, Provider: &provider, + ProviderAddr: n.ResolvedProvider, ProviderSchema: &providerSchema, State: &state, OutputChange: &change, diff --git a/terraform/node_resource_refresh.go b/terraform/node_resource_refresh.go index 857d392db..81f798b26 100644 --- a/terraform/node_resource_refresh.go +++ b/terraform/node_resource_refresh.go @@ -194,10 +194,12 @@ func (n *NodeRefreshableManagedResourceInstance) evalTreeManagedResource() EvalN }, &EvalRefresh{ - Addr: addr.Resource, - Provider: &provider, - State: &state, - Output: &state, + Addr: addr.Resource, + ProviderAddr: n.ResolvedProvider, + Provider: &provider, + ProviderSchema: &providerSchema, + State: &state, + Output: &state, }, &EvalWriteState{ @@ -255,6 +257,7 @@ func (n *NodeRefreshableManagedResourceInstance) evalTreeManagedResourceNoState( Addr: addr.Resource, Config: n.Config, Provider: &provider, + ProviderAddr: n.ResolvedProvider, ProviderSchema: &providerSchema, State: &state, OutputChange: &change, diff --git a/terraform/provider_mock.go b/terraform/provider_mock.go index 5f2cf3a5c..83cfc134c 100644 --- a/terraform/provider_mock.go +++ b/terraform/provider_mock.go @@ -1,8 +1,11 @@ package terraform import ( + "fmt" "sync" + "github.com/hashicorp/terraform/tfdiags" + "github.com/hashicorp/terraform/providers" ) @@ -16,8 +19,8 @@ type MockProvider struct { // Anything you want, in case you need to store extra data with the mock. Meta interface{} - GetSchemaCalled bool - GetSchemaResponse providers.GetSchemaResponse + GetSchemaCalled bool + GetSchemaReturn *ProviderSchema // This is using ProviderSchema directly rather than providers.GetSchemaResponse for compatibility with old tests ValidateProviderConfigCalled bool ValidateProviderConfigResponse providers.ValidateProviderConfigResponse @@ -45,7 +48,7 @@ type MockProvider struct { ConfigureCalled bool ConfigureResponse providers.ConfigureResponse ConfigureRequest providers.ConfigureRequest - ConfigureFn func(providers.ConfigureRequest) providers.ConfigureResponse + ConfigureNewFn func(providers.ConfigureRequest) providers.ConfigureResponse // Named ConfigureNewFn so we can still have the legacy ConfigureFn declared below StopCalled bool StopFn func() error @@ -78,6 +81,15 @@ type MockProvider struct { CloseCalled bool CloseError error + + // Legacy callbacks: if these are set, we will shim incoming calls for + // new-style methods to these old-fashioned terraform.ResourceProvider + // mock callbacks, for the benefit of older tests that were written against + // the old mock API. + ValidateFn func(c *ResourceConfig) (ws []string, es []error) + ConfigureFn func(c *ResourceConfig) error + DiffFn func(info *InstanceInfo, s *InstanceState, c *ResourceConfig) (*InstanceDiff, error) + ApplyFn func(info *InstanceInfo, s *InstanceState, d *InstanceDiff) (*InstanceState, error) } func (p *MockProvider) GetSchema() providers.GetSchemaResponse { @@ -85,7 +97,24 @@ func (p *MockProvider) GetSchema() providers.GetSchemaResponse { defer p.Unlock() p.GetSchemaCalled = true - return p.GetSchemaResponse + ret := providers.GetSchemaResponse{ + Provider: providers.Schema{ + Block: p.GetSchemaReturn.Provider, + }, + DataSources: map[string]providers.Schema{}, + ResourceTypes: map[string]providers.Schema{}, + } + for n, s := range p.GetSchemaReturn.DataSources { + ret.DataSources[n] = providers.Schema{ + Block: s, + } + } + for n, s := range p.GetSchemaReturn.ResourceTypes { + ret.ResourceTypes[n] = providers.Schema{ + Block: s, + } + } + return ret } func (p *MockProvider) ValidateProviderConfig(r providers.ValidateProviderConfigRequest) providers.ValidateProviderConfigResponse { @@ -107,6 +136,11 @@ func (p *MockProvider) ValidateResourceTypeConfig(r providers.ValidateResourceTy p.ValidateResourceTypeConfigCalled = true p.ValidateResourceTypeConfigRequest = r + if p.ValidateFn != nil { + return providers.ValidateResourceTypeConfigResponse{ + Diagnostics: tfdiags.Diagnostics(nil).Append(fmt.Errorf("legacy ValidateFn handling in MockProvider not actually implemented yet")), + } + } if p.ValidateResourceTypeConfigFn != nil { return p.ValidateResourceTypeConfigFn(r) } @@ -150,7 +184,12 @@ func (p *MockProvider) Configure(r providers.ConfigureRequest) providers.Configu p.ConfigureRequest = r if p.ConfigureFn != nil { - return p.ConfigureFn(r) + return providers.ConfigureResponse{ + Diagnostics: tfdiags.Diagnostics(nil).Append(fmt.Errorf("legacy ConfigureFn handling in MockProvider not actually implemented yet")), + } + } + if p.ConfigureNewFn != nil { + return p.ConfigureNewFn(r) } return p.ConfigureResponse @@ -186,6 +225,11 @@ func (p *MockProvider) PlanResourceChange(r providers.PlanResourceChangeRequest) p.PlanResourceChangeCalled = true p.PlanResourceChangeRequest = r + if p.DiffFn != nil { + return providers.PlanResourceChangeResponse{ + Diagnostics: tfdiags.Diagnostics(nil).Append(fmt.Errorf("legacy DiffFn handling in MockProvider not actually implemented yet")), + } + } if p.PlanResourceChangeFn != nil { return p.PlanResourceChangeFn(r) } @@ -199,6 +243,11 @@ func (p *MockProvider) ApplyResourceChange(r providers.ApplyResourceChangeReques p.ApplyResourceChangeRequest = r p.Unlock() + if p.DiffFn != nil { + return providers.ApplyResourceChangeResponse{ + Diagnostics: tfdiags.Diagnostics(nil).Append(fmt.Errorf("legacy ApplyFn handling in MockProvider not actually implemented yet")), + } + } if p.ApplyResourceChangeFn != nil { return p.ApplyResourceChangeFn(r) } diff --git a/terraform/provisioner_mock.go b/terraform/provisioner_mock.go index 83f80c61e..5dd89714a 100644 --- a/terraform/provisioner_mock.go +++ b/terraform/provisioner_mock.go @@ -1,9 +1,11 @@ package terraform import ( + "fmt" "sync" "github.com/hashicorp/terraform/provisioners" + "github.com/hashicorp/terraform/tfdiags" ) var _ provisioners.Interface = (*MockProvisioner)(nil) @@ -35,6 +37,12 @@ type MockProvisioner struct { CloseCalled bool CloseResponse error CloseFn func() error + + // Legacy callbacks: if these are set, we will shim incoming calls for + // new-style methods to these old-fashioned terraform.ResourceProvider + // mock callbacks, for the benefit of older tests that were written against + // the old mock API. + ApplyFn func(rs *InstanceState, c *ResourceConfig) error } func (p *MockProvisioner) GetSchema() provisioners.GetSchemaResponse { @@ -61,6 +69,11 @@ func (p *MockProvisioner) ProvisionResource(r provisioners.ProvisionResourceRequ p.Lock() p.ProvisionResourceCalled = true p.ProvisionResourceRequest = r + if p.ApplyFn != nil { + return provisioners.ProvisionResourceResponse{ + Diagnostics: tfdiags.Diagnostics(nil).Append(fmt.Errorf("legacy ApplyFn handling in MockProvisioner not actually implemented yet")), + } + } if p.ProvisionResourceFn != nil { fn := p.ProvisionResourceFn p.Unlock() diff --git a/terraform/resource_provider_mock_test.go b/terraform/resource_provider_mock_test.go index 3f681b4e4..ed6d2ba0e 100644 --- a/terraform/resource_provider_mock_test.go +++ b/terraform/resource_provider_mock_test.go @@ -27,10 +27,8 @@ func testProviderComponentFactory(name string, provider providers.Interface) *ba // provider with the given schema for its own configuration. func mockProviderWithConfigSchema(schema *configschema.Block) *MockProvider { return &MockProvider{ - GetSchemaResponse: providers.GetSchemaResponse{ - Provider: providers.Schema{ - Block: schema, - }, + GetSchemaReturn: &ProviderSchema{ + Provider: schema, }, } } @@ -39,29 +37,25 @@ func mockProviderWithConfigSchema(schema *configschema.Block) *MockProvider { // provider with a schema containing a single resource type. func mockProviderWithResourceTypeSchema(name string, schema *configschema.Block) *MockProvider { return &MockProvider{ - GetSchemaResponse: providers.GetSchemaResponse{ - Provider: providers.Schema{ - Block: &configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "string": { - Type: cty.String, - Optional: true, - }, - "list": { - Type: cty.List(cty.String), - Optional: true, - }, - "root": { - Type: cty.Map(cty.String), - Optional: true, - }, + GetSchemaReturn: &ProviderSchema{ + Provider: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "string": { + Type: cty.String, + Optional: true, + }, + "list": { + Type: cty.List(cty.String), + Optional: true, + }, + "root": { + Type: cty.Map(cty.String), + Optional: true, }, }, }, - ResourceTypes: map[string]providers.Schema{ - name: { - Block: schema, - }, + ResourceTypes: map[string]*configschema.Block{ + name: schema, }, }, } @@ -98,19 +92,13 @@ func mockProviderWithDataSourceSchema(name string, schema *configschema.Block) * // objects so that callers can mutate without affecting mock objects. func simpleMockProvider() *MockProvider { return &MockProvider{ - GetSchemaResponse: providers.GetSchemaResponse{ - Provider: providers.Schema{ - Block: simpleTestSchema(), + GetSchemaReturn: &ProviderSchema{ + Provider: simpleTestSchema(), + ResourceTypes: map[string]*configschema.Block{ + "test_object": simpleTestSchema(), }, - ResourceTypes: map[string]providers.Schema{ - "test_object": { - Block: simpleTestSchema(), - }, - }, - DataSources: map[string]providers.Schema{ - "test_object": { - Block: simpleTestSchema(), - }, + DataSources: map[string]*configschema.Block{ + "test_object": simpleTestSchema(), }, }, } diff --git a/terraform/transform_deposed.go b/terraform/transform_deposed.go index 5df9c0bda..599384949 100644 --- a/terraform/transform_deposed.go +++ b/terraform/transform_deposed.go @@ -103,10 +103,12 @@ func (n *graphNodeDeposedResource) EvalTree() EvalNode { Output: &state, }, &EvalRefresh{ - Addr: addr.Resource, - Provider: &provider, - State: &state, - Output: &state, + Addr: addr.Resource, + ProviderAddr: n.ResolvedProvider, + Provider: &provider, + ProviderSchema: &providerSchema, + State: &state, + Output: &state, }, &EvalWriteStateDeposed{ Addr: addr.Resource, @@ -150,6 +152,7 @@ func (n *graphNodeDeposedResource) EvalTree() EvalNode { }, &EvalApply{ Addr: addr.Resource, + Config: nil, // No configuration because we are destroying State: &state, Change: &change, Provider: &provider, diff --git a/terraform/transform_import_state.go b/terraform/transform_import_state.go index 731174e0e..ca038c82d 100644 --- a/terraform/transform_import_state.go +++ b/terraform/transform_import_state.go @@ -3,10 +3,8 @@ package terraform import ( "fmt" - "github.com/hashicorp/terraform/providers" - "github.com/hashicorp/terraform/states" - "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/tfdiags" ) @@ -34,7 +32,7 @@ type graphNodeImportState struct { ProviderAddr addrs.AbsProviderConfig // Provider address given by the user, or implied by the resource type ResolvedProvider addrs.AbsProviderConfig // provider node address after resolution - states []*states.ImportedObject + states []providers.ImportedResource } var ( @@ -85,7 +83,7 @@ func (n *graphNodeImportState) EvalTree() EvalNode { &EvalImportState{ Addr: n.Addr.Resource, Provider: &provider, - Id: n.ID, + ID: n.ID, Output: &n.states, }, }, @@ -112,7 +110,7 @@ func (n *graphNodeImportState) DynamicExpand(ctx EvalContext) (*Graph, error) { addrs := make([]addrs.AbsResourceInstance, len(n.states)) for i, state := range n.states { addr := n.Addr - if t := state.ResourceType; t != "" { + if t := state.TypeName; t != "" { addr.Resource.Resource.Type = t } @@ -174,7 +172,7 @@ func (n *graphNodeImportState) DynamicExpand(ctx EvalContext) (*Graph, error) { // and adding a resource to the state once it is imported. type graphNodeImportStateSub struct { TargetAddr addrs.AbsResourceInstance - State *states.ImportedObject + State providers.ImportedResource ResolvedProvider addrs.AbsProviderConfig } @@ -194,7 +192,7 @@ func (n *graphNodeImportStateSub) Path() addrs.ModuleInstance { // GraphNodeEvalable impl. func (n *graphNodeImportStateSub) EvalTree() EvalNode { // If the Ephemeral type isn't set, then it is an error - if n.State.ResourceType == "" { + if n.State.TypeName == "" { err := fmt.Errorf("import of %s didn't set type", n.TargetAddr.String()) return &EvalReturnError{Error: &err} } @@ -211,10 +209,12 @@ func (n *graphNodeImportStateSub) EvalTree() EvalNode { Schema: &providerSchema, }, &EvalRefresh{ - Addr: n.TargetAddr.Resource, - Provider: &provider, - State: &state, - Output: &state, + Addr: n.TargetAddr.Resource, + ProviderAddr: n.ResolvedProvider, + Provider: &provider, + ProviderSchema: &providerSchema, + State: &state, + Output: &state, }, &EvalImportStateVerify{ Addr: n.TargetAddr.Resource,