From db7596c0452fcbc7c90e2c12b389b2fb61d8ea66 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 13 Oct 2017 10:11:10 -0400 Subject: [PATCH] use the inherited provider configs in the graph Use the configured providers directly, rather than looking for inherited provider configuration during graph evaluation. First remove the provider config cache, and the associated SetProviderConfig and ParentProviderConfig methods on the eval context. Every provider must be configured, so there's no need to look for configuration from other provider instances. The config.ProviderConfig struct now has a Scope field which stores the proper path for the interpolation scope. To get this metadata to the interpolator, we add an EvalInterpolatProvider node which can carry the ProviderConfig, and an InterpolateProvider context method to carry the ProviderConfig.Scope into the InterplationScope. Some of the tests could be adjusted to account for the new inheritance behavior, and some were simply no longer valid and will be removed. The remaining tests have questions on how they should work in practice. This mostly concerns orphaned modules where there is no longer a way to obtain a provider. In some cases we may require that a minimal provider config be present to handle the destroy process, but we need further testing. All disabled code was commented out in this commit to record any additional comments. The following commit will be a cleanup pass. --- terraform/context_apply_test.go | 174 +++++++++--------- terraform/context_import_test.go | 2 + terraform/context_plan_test.go | 115 ++++++------ terraform/context_validate_test.go | 135 +++++++------- terraform/eval_context.go | 11 +- terraform/eval_context_builtin.go | 109 +++++++---- terraform/eval_context_mock.go | 50 +++-- terraform/eval_interpolate.go | 23 +++ terraform/eval_provider.go | 13 +- terraform/eval_provider_test.go | 79 ++++---- terraform/evaltree_provider.go | 8 +- terraform/graph_walk_context.go | 24 +-- terraform/module_dependencies_test.go | 12 +- terraform/node_provider_abstract.go | 4 +- terraform/node_provider_disabled.go | 2 +- .../import-provider-inherit/child/main.tf | 1 + .../import-provider-inherit/main.tf | 7 +- 17 files changed, 435 insertions(+), 334 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 36f1140e7..22d7d6db3 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -2639,105 +2639,107 @@ module.child: `) } -func TestContext2Apply_moduleOrphanProvider(t *testing.T) { - m := testModule(t, "apply-module-orphan-provider-inherit") - p := testProvider("aws") - p.ApplyFn = testApplyFn - p.DiffFn = testDiffFn +//// FIXME: how do we handle this one? +//func TestContext2Apply_moduleOrphanProvider(t *testing.T) { +// m := testModule(t, "apply-module-orphan-provider-inherit") +// p := testProvider("aws") +// p.ApplyFn = testApplyFn +// p.DiffFn = testDiffFn - p.ConfigureFn = func(c *ResourceConfig) error { - if _, ok := c.Get("value"); !ok { - return fmt.Errorf("value is not found") - } +// p.ConfigureFn = func(c *ResourceConfig) error { +// if _, ok := c.Get("value"); !ok { +// return fmt.Errorf("value is not found") +// } - return nil - } +// return nil +// } - // Create a state with an orphan module - state := &State{ - Modules: []*ModuleState{ - &ModuleState{ - Path: []string{"root", "child"}, - Resources: map[string]*ResourceState{ - "aws_instance.bar": &ResourceState{ - Type: "aws_instance", - Primary: &InstanceState{ - ID: "bar", - }, - }, - }, - }, - }, - } +// // Create a state with an orphan module +// state := &State{ +// Modules: []*ModuleState{ +// &ModuleState{ +// Path: []string{"root", "child"}, +// Resources: map[string]*ResourceState{ +// "aws_instance.bar": &ResourceState{ +// Type: "aws_instance", +// Primary: &InstanceState{ +// ID: "bar", +// }, +// }, +// }, +// }, +// }, +// } - ctx := testContext2(t, &ContextOpts{ - Module: m, - State: state, - ProviderResolver: ResourceProviderResolverFixed( - map[string]ResourceProviderFactory{ - "aws": testProviderFuncFixed(p), - }, - ), - }) +// ctx := testContext2(t, &ContextOpts{ +// Module: m, +// State: state, +// ProviderResolver: ResourceProviderResolverFixed( +// map[string]ResourceProviderFactory{ +// "aws": testProviderFuncFixed(p), +// }, +// ), +// }) - if _, err := ctx.Plan(); err != nil { - t.Fatalf("err: %s", err) - } +// if _, err := ctx.Plan(); err != nil { +// t.Fatalf("err: %s", err) +// } - if _, err := ctx.Apply(); err != nil { - t.Fatalf("err: %s", err) - } -} +// if _, err := ctx.Apply(); err != nil { +// t.Fatalf("err: %s", err) +// } +//} -func TestContext2Apply_moduleOrphanGrandchildProvider(t *testing.T) { - m := testModule(t, "apply-module-orphan-provider-inherit") - p := testProvider("aws") - p.ApplyFn = testApplyFn - p.DiffFn = testDiffFn +//// FIXME: how do we handle this one? +//func TestContext2Apply_moduleOrphanGrandchildProvider(t *testing.T) { +// m := testModule(t, "apply-module-orphan-provider-inherit") +// p := testProvider("aws") +// p.ApplyFn = testApplyFn +// p.DiffFn = testDiffFn - p.ConfigureFn = func(c *ResourceConfig) error { - if _, ok := c.Get("value"); !ok { - return fmt.Errorf("value is not found") - } +// p.ConfigureFn = func(c *ResourceConfig) error { +// if _, ok := c.Get("value"); !ok { +// return fmt.Errorf("value is not found") +// } - return nil - } +// return nil +// } - // Create a state with an orphan module that is nested (grandchild) - state := &State{ - Modules: []*ModuleState{ - &ModuleState{ - Path: []string{"root", "parent", "child"}, - Resources: map[string]*ResourceState{ - "aws_instance.bar": &ResourceState{ - Type: "aws_instance", - Primary: &InstanceState{ - ID: "bar", - }, - }, - }, - }, - }, - } +// // Create a state with an orphan module that is nested (grandchild) +// state := &State{ +// Modules: []*ModuleState{ +// &ModuleState{ +// Path: []string{"root", "parent", "child"}, +// Resources: map[string]*ResourceState{ +// "aws_instance.bar": &ResourceState{ +// Type: "aws_instance", +// Primary: &InstanceState{ +// ID: "bar", +// }, +// }, +// }, +// }, +// }, +// } - ctx := testContext2(t, &ContextOpts{ - Module: m, - State: state, - ProviderResolver: ResourceProviderResolverFixed( - map[string]ResourceProviderFactory{ - "aws": testProviderFuncFixed(p), - }, - ), - }) +// ctx := testContext2(t, &ContextOpts{ +// Module: m, +// State: state, +// ProviderResolver: ResourceProviderResolverFixed( +// map[string]ResourceProviderFactory{ +// "aws": testProviderFuncFixed(p), +// }, +// ), +// }) - if _, err := ctx.Plan(); err != nil { - t.Fatalf("err: %s", err) - } +// if _, err := ctx.Plan(); err != nil { +// t.Fatalf("err: %s", err) +// } - if _, err := ctx.Apply(); err != nil { - t.Fatalf("err: %s", err) - } -} +// if _, err := ctx.Apply(); err != nil { +// t.Fatalf("err: %s", err) +// } +//} func TestContext2Apply_moduleGrandchildProvider(t *testing.T) { m := testModule(t, "apply-module-grandchild-provider-inherit") diff --git a/terraform/context_import_test.go b/terraform/context_import_test.go index 1316b5c0b..df5d87994 100644 --- a/terraform/context_import_test.go +++ b/terraform/context_import_test.go @@ -227,6 +227,8 @@ func TestContextImport_moduleProvider(t *testing.T) { } // Test that import sets up the graph properly for provider inheritance +// FIXME: import must declare a provider in an empty config. Should that go +// back to being automatically inherited? func TestContextImport_providerInherit(t *testing.T) { p := testProvider("aws") m := testModule(t, "import-provider-inherit") diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index bf9f7a65e..f1edab840 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -643,66 +643,67 @@ func TestContext2Plan_moduleProviderInheritDeep(t *testing.T) { } } -func TestContext2Plan_moduleProviderDefaults(t *testing.T) { - var l sync.Mutex - var calls []string - toCount := 0 +//// REMOVING: we no longer override child provider config +//func TestContext2Plan_moduleProviderDefaults(t *testing.T) { +// var l sync.Mutex +// var calls []string +// toCount := 0 - m := testModule(t, "plan-module-provider-defaults") - ctx := testContext2(t, &ContextOpts{ - Module: m, - ProviderResolver: ResourceProviderResolverFixed( - map[string]ResourceProviderFactory{ - "aws": func() (ResourceProvider, error) { - l.Lock() - defer l.Unlock() +// m := testModule(t, "plan-module-provider-defaults") +// ctx := testContext2(t, &ContextOpts{ +// Module: m, +// ProviderResolver: ResourceProviderResolverFixed( +// map[string]ResourceProviderFactory{ +// "aws": func() (ResourceProvider, error) { +// l.Lock() +// defer l.Unlock() - p := testProvider("aws") - p.ConfigureFn = func(c *ResourceConfig) error { - if v, ok := c.Get("from"); !ok || v.(string) != "root" { - return fmt.Errorf("bad") - } - if v, ok := c.Get("to"); ok && v.(string) == "child" { - toCount++ - } +// p := testProvider("aws") +// p.ConfigureFn = func(c *ResourceConfig) error { +// if v, ok := c.Get("from"); !ok || v.(string) != "root" { +// return fmt.Errorf("bad") +// } +// if v, ok := c.Get("to"); ok && v.(string) == "child" { +// toCount++ +// } - return nil - } - p.DiffFn = func( - info *InstanceInfo, - state *InstanceState, - c *ResourceConfig) (*InstanceDiff, error) { - v, _ := c.Get("from") +// return nil +// } +// p.DiffFn = func( +// info *InstanceInfo, +// state *InstanceState, +// c *ResourceConfig) (*InstanceDiff, error) { +// v, _ := c.Get("from") - l.Lock() - defer l.Unlock() - calls = append(calls, v.(string)) - return testDiffFn(info, state, c) - } - return p, nil - }, - }, - ), - }) +// l.Lock() +// defer l.Unlock() +// calls = append(calls, v.(string)) +// return testDiffFn(info, state, c) +// } +// return p, nil +// }, +// }, +// ), +// }) - _, err := ctx.Plan() - if err != nil { - t.Fatalf("err: %s", err) - } +// _, err := ctx.Plan() +// if err != nil { +// t.Fatalf("err: %s", err) +// } - if toCount != 1 { - t.Fatalf( - "provider in child didn't set proper config\n\n"+ - "toCount: %d", toCount) - } +// if toCount != 1 { +// t.Fatalf( +// "provider in child didn't set proper config\n\n"+ +// "toCount: %d", toCount) +// } - actual := calls - sort.Strings(actual) - expected := []string{"child", "root"} - if !reflect.DeepEqual(actual, expected) { - t.Fatalf("bad: %#v", actual) - } -} +// actual := calls +// sort.Strings(actual) +// expected := []string{"child", "root"} +// if !reflect.DeepEqual(actual, expected) { +// t.Fatalf("bad: %#v", actual) +// } +//} func TestContext2Plan_moduleProviderDefaultsVar(t *testing.T) { var l sync.Mutex @@ -749,10 +750,14 @@ func TestContext2Plan_moduleProviderDefaultsVar(t *testing.T) { expected := []string{ "root\n", - "root\nchild\n", + // this test originally verified that a parent provider config can + // partially override a child. That's no longer the case, so the child + // config is used in its entirety here. + //"root\nchild\n", + "child\nchild\n", } if !reflect.DeepEqual(calls, expected) { - t.Fatalf("BAD: %#v", calls) + t.Fatalf("expected:\n%#v\ngot:\n%#v\n", expected, calls) } } diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go index 34a0c8615..d02df58b9 100644 --- a/terraform/context_validate_test.go +++ b/terraform/context_validate_test.go @@ -321,78 +321,81 @@ func TestContext2Validate_moduleDepsShouldNotCycle(t *testing.T) { } } -func TestContext2Validate_moduleProviderInherit(t *testing.T) { - m := testModule(t, "validate-module-pc-inherit") - p := testProvider("aws") - c := testContext2(t, &ContextOpts{ - Module: m, - ProviderResolver: ResourceProviderResolverFixed( - map[string]ResourceProviderFactory{ - "aws": testProviderFuncFixed(p), - }, - ), - }) +//// REMOVING: change in behavior, this should not be inherited +//func TestContext2Validate_moduleProviderInherit(t *testing.T) { +// m := testModule(t, "validate-module-pc-inherit") +// p := testProvider("aws") +// c := testContext2(t, &ContextOpts{ +// Module: m, +// ProviderResolver: ResourceProviderResolverFixed( +// map[string]ResourceProviderFactory{ +// "aws": testProviderFuncFixed(p), +// }, +// ), +// }) - p.ValidateFn = func(c *ResourceConfig) ([]string, []error) { - return nil, c.CheckSet([]string{"set"}) - } +// p.ValidateFn = func(c *ResourceConfig) ([]string, []error) { +// return nil, c.CheckSet([]string{"set"}) +// } - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %s", e) - } -} +// w, e := c.Validate() +// if len(w) > 0 { +// t.Fatalf("bad: %#v", w) +// } +// if len(e) > 0 { +// t.Fatalf("bad: %s", e) +// } +//} -func TestContext2Validate_moduleProviderInheritOrphan(t *testing.T) { - m := testModule(t, "validate-module-pc-inherit-orphan") - p := testProvider("aws") - c := testContext2(t, &ContextOpts{ - Module: m, - ProviderResolver: ResourceProviderResolverFixed( - map[string]ResourceProviderFactory{ - "aws": testProviderFuncFixed(p), - }, - ), - State: &State{ - Modules: []*ModuleState{ - &ModuleState{ - Path: []string{"root", "child"}, - Resources: map[string]*ResourceState{ - "aws_instance.bar": &ResourceState{ - Type: "aws_instance", - Primary: &InstanceState{ - ID: "bar", - }, - }, - }, - }, - }, - }, - }) +//// FIXME: provider must still exist in config, but we should be able to locate +//// it elsewhere +//func TestContext2Validate_moduleProviderInheritOrphan(t *testing.T) { +// m := testModule(t, "validate-module-pc-inherit-orphan") +// p := testProvider("aws") +// c := testContext2(t, &ContextOpts{ +// Module: m, +// ProviderResolver: ResourceProviderResolverFixed( +// map[string]ResourceProviderFactory{ +// "aws": testProviderFuncFixed(p), +// }, +// ), +// State: &State{ +// Modules: []*ModuleState{ +// &ModuleState{ +// Path: []string{"root", "child"}, +// Resources: map[string]*ResourceState{ +// "aws_instance.bar": &ResourceState{ +// Type: "aws_instance", +// Primary: &InstanceState{ +// ID: "bar", +// }, +// }, +// }, +// }, +// }, +// }, +// }) - p.ValidateFn = func(c *ResourceConfig) ([]string, []error) { - v, ok := c.Get("set") - if !ok { - return nil, []error{fmt.Errorf("not set")} - } - if v != "bar" { - return nil, []error{fmt.Errorf("bad: %#v", v)} - } +// p.ValidateFn = func(c *ResourceConfig) ([]string, []error) { +// v, ok := c.Get("set") +// if !ok { +// return nil, []error{fmt.Errorf("not set")} +// } +// if v != "bar" { +// return nil, []error{fmt.Errorf("bad: %#v", v)} +// } - return nil, nil - } +// return nil, nil +// } - w, e := c.Validate() - if len(w) > 0 { - t.Fatalf("bad: %#v", w) - } - if len(e) > 0 { - t.Fatalf("bad: %s", e) - } -} +// w, e := c.Validate() +// if len(w) > 0 { +// t.Fatalf("bad: %#v", w) +// } +// if len(e) > 0 { +// t.Fatalf("bad: %s", e) +// } +//} func TestContext2Validate_moduleProviderVar(t *testing.T) { m := testModule(t, "validate-module-pc-vars") diff --git a/terraform/eval_context.go b/terraform/eval_context.go index a1f815b7d..83b764d74 100644 --- a/terraform/eval_context.go +++ b/terraform/eval_context.go @@ -40,8 +40,8 @@ type EvalContext interface { // is used to store the provider configuration for inheritance lookups // with ParentProviderConfig(). ConfigureProvider(string, *ResourceConfig) error - SetProviderConfig(string, *ResourceConfig) error - ParentProviderConfig(string) *ResourceConfig + //SetProviderConfig(string, *ResourceConfig) error + //ParentProviderConfig(string) *ResourceConfig // ProviderInput and SetProviderInput are used to configure providers // from user input. @@ -69,6 +69,13 @@ type EvalContext interface { // that is currently being acted upon. Interpolate(*config.RawConfig, *Resource) (*ResourceConfig, error) + // InterpolateProvider takes a ProviderConfig and interpolates it with the + // stored interpolation scope. Since provider configurations can be + // inherited, the interpolation scope may be different from the current + // context path. Interplation is otherwise executed the same as in the + // Interpolation method. + InterpolateProvider(*config.ProviderConfig, *Resource) (*ResourceConfig, error) + // SetVariables sets the variables for the module within // this context with the name n. This function call is additive: // the second parameter is merged with any previous call. diff --git a/terraform/eval_context_builtin.go b/terraform/eval_context_builtin.go index 3dcfb2275..5a0bb69e1 100644 --- a/terraform/eval_context_builtin.go +++ b/terraform/eval_context_builtin.go @@ -30,11 +30,11 @@ type BuiltinEvalContext struct { InterpolaterVars map[string]map[string]interface{} InterpolaterVarLock *sync.Mutex - Components contextComponentFactory - Hooks []Hook - InputValue UIInput - ProviderCache map[string]ResourceProvider - ProviderConfigCache map[string]*ResourceConfig + Components contextComponentFactory + Hooks []Hook + InputValue UIInput + ProviderCache map[string]ResourceProvider + //ProviderConfigCache map[string]*ResourceConfig ProviderInputConfig map[string]map[string]interface{} ProviderLock *sync.Mutex ProvisionerCache map[string]ResourceProvisioner @@ -150,26 +150,27 @@ func (ctx *BuiltinEvalContext) ConfigureProvider( return fmt.Errorf("Provider '%s' not initialized", n) } - if err := ctx.SetProviderConfig(n, cfg); err != nil { - return nil - } + //if err := ctx.SetProviderConfig(n, cfg); err != nil { + // return nil + //} return p.Configure(cfg) } -func (ctx *BuiltinEvalContext) SetProviderConfig( - n string, cfg *ResourceConfig) error { - providerPath := make([]string, len(ctx.Path())+1) - copy(providerPath, ctx.Path()) - providerPath[len(providerPath)-1] = n +// REMOVING: each provider will contain its own configuration +//func (ctx *BuiltinEvalContext) SetProviderConfig( +// n string, cfg *ResourceConfig) error { +// providerPath := make([]string, len(ctx.Path())+1) +// copy(providerPath, ctx.Path()) +// providerPath[len(providerPath)-1] = n - // Save the configuration - ctx.ProviderLock.Lock() - ctx.ProviderConfigCache[PathCacheKey(providerPath)] = cfg - ctx.ProviderLock.Unlock() +// // Save the configuration +// ctx.ProviderLock.Lock() +// ctx.ProviderConfigCache[PathCacheKey(providerPath)] = cfg +// ctx.ProviderLock.Unlock() - return nil -} +// return nil +//} func (ctx *BuiltinEvalContext) ProviderInput(n string) map[string]interface{} { ctx.ProviderLock.Lock() @@ -203,26 +204,27 @@ func (ctx *BuiltinEvalContext) SetProviderInput(n string, c map[string]interface ctx.ProviderLock.Unlock() } -func (ctx *BuiltinEvalContext) ParentProviderConfig(n string) *ResourceConfig { - ctx.ProviderLock.Lock() - defer ctx.ProviderLock.Unlock() +// REMOVING: Each provider will contain its own configuration +//func (ctx *BuiltinEvalContext) ParentProviderConfig(n string) *ResourceConfig { +// ctx.ProviderLock.Lock() +// defer ctx.ProviderLock.Unlock() - // Make a copy of the path so we can safely edit it - path := ctx.Path() - pathCopy := make([]string, len(path)+1) - copy(pathCopy, path) +// // Make a copy of the path so we can safely edit it +// path := ctx.Path() +// pathCopy := make([]string, len(path)+1) +// copy(pathCopy, path) - // Go up the tree. - for i := len(path) - 1; i >= 0; i-- { - pathCopy[i+1] = n - k := PathCacheKey(pathCopy[:i+2]) - if v, ok := ctx.ProviderConfigCache[k]; ok { - return v - } - } +// // Go up the tree. +// for i := len(path) - 1; i >= 0; i-- { +// pathCopy[i+1] = n +// k := PathCacheKey(pathCopy[:i+2]) +// if v, ok := ctx.ProviderConfigCache[k]; ok { +// return v +// } +// } - return nil -} +// return nil +//} func (ctx *BuiltinEvalContext) InitProvisioner( n string) (ResourceProvisioner, error) { @@ -289,6 +291,7 @@ func (ctx *BuiltinEvalContext) CloseProvisioner(n string) error { func (ctx *BuiltinEvalContext) Interpolate( cfg *config.RawConfig, r *Resource) (*ResourceConfig, error) { + if cfg != nil { scope := &InterpolationScope{ Path: ctx.Path(), @@ -311,6 +314,40 @@ func (ctx *BuiltinEvalContext) Interpolate( return result, nil } +func (ctx *BuiltinEvalContext) InterpolateProvider( + pc *config.ProviderConfig, r *Resource) (*ResourceConfig, error) { + + var cfg *config.RawConfig + + if pc != nil && pc.RawConfig != nil { + path := pc.Scope + if len(path) == 0 { + path = ctx.Path() + } + + scope := &InterpolationScope{ + Path: path, + Resource: r, + } + + cfg = pc.RawConfig + + vs, err := ctx.Interpolater.Values(scope, cfg.Variables) + if err != nil { + return nil, err + } + + // Do the interpolation + if err := cfg.Interpolate(vs); err != nil { + return nil, err + } + } + + result := NewResourceConfig(cfg) + result.interpolateForce() + return result, nil +} + func (ctx *BuiltinEvalContext) Path() []string { return ctx.PathValue } diff --git a/terraform/eval_context_mock.go b/terraform/eval_context_mock.go index 4f90d5b12..bd903e4fc 100644 --- a/terraform/eval_context_mock.go +++ b/terraform/eval_context_mock.go @@ -45,13 +45,13 @@ type MockEvalContext struct { ConfigureProviderConfig *ResourceConfig ConfigureProviderError error - SetProviderConfigCalled bool - SetProviderConfigName string - SetProviderConfigConfig *ResourceConfig + //SetProviderConfigCalled bool + //SetProviderConfigName string + //SetProviderConfigConfig *ResourceConfig - ParentProviderConfigCalled bool - ParentProviderConfigName string - ParentProviderConfigConfig *ResourceConfig + //ParentProviderConfigCalled bool + //ParentProviderConfigName string + //ParentProviderConfigConfig *ResourceConfig InitProvisionerCalled bool InitProvisionerName string @@ -72,6 +72,12 @@ type MockEvalContext struct { InterpolateConfigResult *ResourceConfig InterpolateError error + InterpolateProviderCalled bool + InterpolateProviderConfig *config.ProviderConfig + InterpolateProviderResource *Resource + InterpolateProviderConfigResult *ResourceConfig + InterpolateProviderError error + PathCalled bool PathPath []string @@ -134,19 +140,19 @@ func (c *MockEvalContext) ConfigureProvider(n string, cfg *ResourceConfig) error return c.ConfigureProviderError } -func (c *MockEvalContext) SetProviderConfig( - n string, cfg *ResourceConfig) error { - c.SetProviderConfigCalled = true - c.SetProviderConfigName = n - c.SetProviderConfigConfig = cfg - return nil -} +//func (c *MockEvalContext) SetProviderConfig( +// n string, cfg *ResourceConfig) error { +// c.SetProviderConfigCalled = true +// c.SetProviderConfigName = n +// c.SetProviderConfigConfig = cfg +// return nil +//} -func (c *MockEvalContext) ParentProviderConfig(n string) *ResourceConfig { - c.ParentProviderConfigCalled = true - c.ParentProviderConfigName = n - return c.ParentProviderConfigConfig -} +//func (c *MockEvalContext) ParentProviderConfig(n string) *ResourceConfig { +// c.ParentProviderConfigCalled = true +// c.ParentProviderConfigName = n +// return c.ParentProviderConfigConfig +//} func (c *MockEvalContext) ProviderInput(n string) map[string]interface{} { c.ProviderInputCalled = true @@ -186,6 +192,14 @@ func (c *MockEvalContext) Interpolate( return c.InterpolateConfigResult, c.InterpolateError } +func (c *MockEvalContext) InterpolateProvider( + config *config.ProviderConfig, resource *Resource) (*ResourceConfig, error) { + c.InterpolateProviderCalled = true + c.InterpolateProviderConfig = config + c.InterpolateProviderResource = resource + return c.InterpolateProviderConfigResult, c.InterpolateError +} + func (c *MockEvalContext) Path() []string { c.PathCalled = true return c.PathPath diff --git a/terraform/eval_interpolate.go b/terraform/eval_interpolate.go index 78c881360..6a78a6bbb 100644 --- a/terraform/eval_interpolate.go +++ b/terraform/eval_interpolate.go @@ -31,3 +31,26 @@ func (n *EvalInterpolate) Eval(ctx EvalContext) (interface{}, error) { return nil, nil } + +// EvalInterpolateProvider is an EvalNode implementation that takes a +// ProviderConfig and interpolates it. Provider configurations are the only +// "inherited" type of configuration we have, and the original raw config may +// have a different interpolation scope. +type EvalInterpolateProvider struct { + Config *config.ProviderConfig + Resource *Resource + Output **ResourceConfig +} + +func (n *EvalInterpolateProvider) Eval(ctx EvalContext) (interface{}, error) { + rc, err := ctx.InterpolateProvider(n.Config, n.Resource) + if err != nil { + return nil, err + } + + if n.Output != nil { + *n.Output = rc + } + + return nil, nil +} diff --git a/terraform/eval_provider.go b/terraform/eval_provider.go index 092fd18d8..8c0870a83 100644 --- a/terraform/eval_provider.go +++ b/terraform/eval_provider.go @@ -14,7 +14,8 @@ type EvalSetProviderConfig struct { } func (n *EvalSetProviderConfig) Eval(ctx EvalContext) (interface{}, error) { - return nil, ctx.SetProviderConfig(n.Provider, *n.Config) + return nil, nil + //return nil, ctx.SetProviderConfig(n.Provider, *n.Config) } // EvalBuildProviderConfig outputs a *ResourceConfig that is properly @@ -44,11 +45,11 @@ func (n *EvalBuildProviderConfig) Eval(ctx EvalContext) (interface{}, error) { cfg = NewResourceConfig(merged) } - // Get the parent configuration if there is one - if parent := ctx.ParentProviderConfig(n.Provider); parent != nil { - merged := cfg.raw.Merge(parent.raw) - cfg = NewResourceConfig(merged) - } + //// Get the parent configuration if there is one + //if parent := ctx.ParentProviderConfig(n.Provider); parent != nil { + // merged := cfg.raw.Merge(parent.raw) + // cfg = NewResourceConfig(merged) + //} *n.Output = cfg return nil, nil diff --git a/terraform/eval_provider_test.go b/terraform/eval_provider_test.go index 40af542e2..01610c957 100644 --- a/terraform/eval_provider_test.go +++ b/terraform/eval_provider_test.go @@ -26,10 +26,10 @@ func TestEvalBuildProviderConfig(t *testing.T) { } ctx := &MockEvalContext{ - ParentProviderConfigConfig: testResourceConfig(t, map[string]interface{}{ - "inherited_from_parent": "parent", - "set_in_config_and_parent": "parent", - }), + //ParentProviderConfigConfig: testResourceConfig(t, map[string]interface{}{ + // "inherited_from_parent": "parent", + // "set_in_config_and_parent": "parent", + //}), ProviderInputConfig: map[string]interface{}{ "set_in_config": "input", "set_by_input": "input", @@ -39,53 +39,50 @@ func TestEvalBuildProviderConfig(t *testing.T) { t.Fatalf("err: %s", err) } - // This is a merger of the following, with later items taking precedence: - // - "config" (the config as written in the current module, with all - // interpolation expressions resolved) - // - ProviderInputConfig (mock of config produced by the input walk, after - // prompting the user interactively for values unspecified in config) - // - ParentProviderConfigConfig (mock of config inherited from a parent module) + // We expect the provider config with the added input value expected := map[string]interface{}{ "set_in_config": "input", // in practice, input map contains identical literals from config - "set_in_config_and_parent": "parent", - "inherited_from_parent": "parent", - "computed_in_config": "config", - "set_by_input": "input", + "set_in_config_and_parent": "config", + // no longer merging + //"inherited_from_parent": "parent", + "computed_in_config": "config", + "set_by_input": "input", } if !reflect.DeepEqual(config.Raw, expected) { - t.Fatalf("incorrect merged config %#v; want %#v", config.Raw, expected) + t.Fatalf("incorrect merged config:\n%#v\nwanted:\n%#v", config.Raw, expected) } } -func TestEvalBuildProviderConfig_parentPriority(t *testing.T) { - config := testResourceConfig(t, map[string]interface{}{}) - provider := "foo" +//// REMOVING: this is no longer expected behavior +//func TestEvalBuildProviderConfig_parentPriority(t *testing.T) { +// config := testResourceConfig(t, map[string]interface{}{}) +// provider := "foo" - n := &EvalBuildProviderConfig{ - Provider: provider, - Config: &config, - Output: &config, - } +// n := &EvalBuildProviderConfig{ +// Provider: provider, +// Config: &config, +// Output: &config, +// } - ctx := &MockEvalContext{ - ParentProviderConfigConfig: testResourceConfig(t, map[string]interface{}{ - "foo": "bar", - }), - ProviderInputConfig: map[string]interface{}{ - "foo": "baz", - }, - } - if _, err := n.Eval(ctx); err != nil { - t.Fatalf("err: %s", err) - } +// ctx := &MockEvalContext{ +// //ParentProviderConfigConfig: testResourceConfig(t, map[string]interface{}{ +// // "foo": "bar", +// //}), +// ProviderInputConfig: map[string]interface{}{ +// "foo": "baz", +// }, +// } +// if _, err := n.Eval(ctx); err != nil { +// t.Fatalf("err: %s", err) +// } - expected := map[string]interface{}{ - "foo": "bar", - } - if !reflect.DeepEqual(config.Raw, expected) { - t.Fatalf("bad: %#v", config.Raw) - } -} +// expected := map[string]interface{}{ +// "foo": "bar", +// } +// if !reflect.DeepEqual(config.Raw, expected) { +// t.Fatalf("expected: %#v, got: %#v", expected, config.Raw) +// } +//} func TestEvalConfigProvider_impl(t *testing.T) { var _ EvalNode = new(EvalConfigProvider) diff --git a/terraform/evaltree_provider.go b/terraform/evaltree_provider.go index 00392efed..378341ed1 100644 --- a/terraform/evaltree_provider.go +++ b/terraform/evaltree_provider.go @@ -6,7 +6,7 @@ import ( // ProviderEvalTree returns the evaluation tree for initializing and // configuring providers. -func ProviderEvalTree(n string, config *config.RawConfig) EvalNode { +func ProviderEvalTree(n string, config *config.ProviderConfig) EvalNode { var provider ResourceProvider var resourceConfig *ResourceConfig @@ -22,7 +22,7 @@ func ProviderEvalTree(n string, config *config.RawConfig) EvalNode { Name: n, Output: &provider, }, - &EvalInterpolate{ + &EvalInterpolateProvider{ Config: config, Output: &resourceConfig, }, @@ -48,7 +48,7 @@ func ProviderEvalTree(n string, config *config.RawConfig) EvalNode { Name: n, Output: &provider, }, - &EvalInterpolate{ + &EvalInterpolateProvider{ Config: config, Output: &resourceConfig, }, @@ -78,7 +78,7 @@ func ProviderEvalTree(n string, config *config.RawConfig) EvalNode { Name: n, Output: &provider, }, - &EvalInterpolate{ + &EvalInterpolateProvider{ Config: config, Output: &resourceConfig, }, diff --git a/terraform/graph_walk_context.go b/terraform/graph_walk_context.go index e63b46035..6d49a6cf1 100644 --- a/terraform/graph_walk_context.go +++ b/terraform/graph_walk_context.go @@ -32,10 +32,10 @@ type ContextGraphWalker struct { interpolaterVars map[string]map[string]interface{} interpolaterVarLock sync.Mutex providerCache map[string]ResourceProvider - providerConfigCache map[string]*ResourceConfig - providerLock sync.Mutex - provisionerCache map[string]ResourceProvisioner - provisionerLock sync.Mutex + //providerConfigCache map[string]*ResourceConfig + providerLock sync.Mutex + provisionerCache map[string]ResourceProvisioner + provisionerLock sync.Mutex } func (w *ContextGraphWalker) EnterPath(path []string) EvalContext { @@ -67,13 +67,13 @@ func (w *ContextGraphWalker) EnterPath(path []string) EvalContext { w.interpolaterVarLock.Unlock() ctx := &BuiltinEvalContext{ - StopContext: w.StopContext, - PathValue: path, - Hooks: w.Context.hooks, - InputValue: w.Context.uiInput, - Components: w.Context.components, - ProviderCache: w.providerCache, - ProviderConfigCache: w.providerConfigCache, + StopContext: w.StopContext, + PathValue: path, + Hooks: w.Context.hooks, + InputValue: w.Context.uiInput, + Components: w.Context.components, + ProviderCache: w.providerCache, + //ProviderConfigCache: w.providerConfigCache, ProviderInputConfig: w.Context.providerInputConfig, ProviderLock: &w.providerLock, ProvisionerCache: w.provisionerCache, @@ -151,7 +151,7 @@ func (w *ContextGraphWalker) ExitEvalTree( func (w *ContextGraphWalker) init() { w.contexts = make(map[string]*BuiltinEvalContext, 5) w.providerCache = make(map[string]ResourceProvider, 5) - w.providerConfigCache = make(map[string]*ResourceConfig, 5) + //w.providerConfigCache = make(map[string]*ResourceConfig, 5) w.provisionerCache = make(map[string]ResourceProvisioner, 5) w.interpolaterVars = make(map[string]map[string]interface{}, 5) } diff --git a/terraform/module_dependencies_test.go b/terraform/module_dependencies_test.go index e63739f81..39e4e7de5 100644 --- a/terraform/module_dependencies_test.go +++ b/terraform/module_dependencies_test.go @@ -73,7 +73,8 @@ func TestModuleTreeDependencies(t *testing.T) { Providers: moduledeps.Providers{ "foo": moduledeps.ProviderDependency{ Constraints: discovery.AllVersions, - Reason: moduledeps.ProviderDependencyImplicit, + //Reason: moduledeps.ProviderDependencyImplicit, + Reason: moduledeps.ProviderDependencyExplicit, }, "foo.baz": moduledeps.ProviderDependency{ Constraints: discovery.AllVersions, @@ -118,11 +119,13 @@ func TestModuleTreeDependencies(t *testing.T) { Providers: moduledeps.Providers{ "foo": moduledeps.ProviderDependency{ Constraints: discovery.AllVersions, - Reason: moduledeps.ProviderDependencyInherited, + //Reason: moduledeps.ProviderDependencyInherited, + Reason: moduledeps.ProviderDependencyExplicit, }, "baz": moduledeps.ProviderDependency{ Constraints: discovery.AllVersions, - Reason: moduledeps.ProviderDependencyImplicit, + //Reason: moduledeps.ProviderDependencyImplicit, + Reason: moduledeps.ProviderDependencyExplicit, }, }, Children: []*moduledeps.Module{ @@ -135,7 +138,8 @@ func TestModuleTreeDependencies(t *testing.T) { }, "bar": moduledeps.ProviderDependency{ Constraints: discovery.AllVersions, - Reason: moduledeps.ProviderDependencyInherited, + //Reason: moduledeps.ProviderDependencyInherited, + Reason: moduledeps.ProviderDependencyExplicit, }, }, }, diff --git a/terraform/node_provider_abstract.go b/terraform/node_provider_abstract.go index 6cc836560..4a2c00087 100644 --- a/terraform/node_provider_abstract.go +++ b/terraform/node_provider_abstract.go @@ -60,12 +60,12 @@ func (n *NodeAbstractProvider) ProviderName() string { } // GraphNodeProvider -func (n *NodeAbstractProvider) ProviderConfig() *config.RawConfig { +func (n *NodeAbstractProvider) ProviderConfig() *config.ProviderConfig { if n.Config == nil { return nil } - return n.Config.RawConfig + return n.Config } // GraphNodeAttachProvider diff --git a/terraform/node_provider_disabled.go b/terraform/node_provider_disabled.go index 25e7e620e..e1d0b2483 100644 --- a/terraform/node_provider_disabled.go +++ b/terraform/node_provider_disabled.go @@ -20,7 +20,7 @@ func (n *NodeDisabledProvider) EvalTree() EvalNode { var resourceConfig *ResourceConfig return &EvalSequence{ Nodes: []EvalNode{ - &EvalInterpolate{ + &EvalInterpolateProvider{ Config: n.ProviderConfig(), Output: &resourceConfig, }, diff --git a/terraform/test-fixtures/import-provider-inherit/child/main.tf b/terraform/test-fixtures/import-provider-inherit/child/main.tf index b7db25411..773b4c0f0 100644 --- a/terraform/test-fixtures/import-provider-inherit/child/main.tf +++ b/terraform/test-fixtures/import-provider-inherit/child/main.tf @@ -1 +1,2 @@ # Empty +provider "aws" {} diff --git a/terraform/test-fixtures/import-provider-inherit/main.tf b/terraform/test-fixtures/import-provider-inherit/main.tf index ae152d73a..9c4104608 100644 --- a/terraform/test-fixtures/import-provider-inherit/main.tf +++ b/terraform/test-fixtures/import-provider-inherit/main.tf @@ -2,4 +2,9 @@ provider "aws" { foo = "bar" } -module "child" { source = "./child" } +module "child" { + source = "./child" + providers { + "aws" = "aws" + } +}