From c658fe173a3cc178464fdce4bbba013c0abb28e4 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 31 May 2019 20:07:41 -0500 Subject: [PATCH 1/3] test a dynamic block with MinItems in the schema A dynamic block is always going to be a single value during validation, but should not fail when min-items > 1 is specified. --- builtin/providers/test/resource_list.go | 14 +++++- builtin/providers/test/resource_list_test.go | 52 ++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/builtin/providers/test/resource_list.go b/builtin/providers/test/resource_list.go index 36db61993..a8b871369 100644 --- a/builtin/providers/test/resource_list.go +++ b/builtin/providers/test/resource_list.go @@ -95,7 +95,19 @@ func testResourceList() *schema.Resource { Computed: true, Elem: &schema.Schema{Type: schema.TypeString}, }, - + "min_items": { + Type: schema.TypeList, + Optional: true, + MinItems: 2, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "val": { + Type: schema.TypeString, + Required: true, + }, + }, + }, + }, "never_set": { Type: schema.TypeList, MaxItems: 1, diff --git a/builtin/providers/test/resource_list_test.go b/builtin/providers/test/resource_list_test.go index 94c697cee..00ec5491c 100644 --- a/builtin/providers/test/resource_list_test.go +++ b/builtin/providers/test/resource_list_test.go @@ -1,6 +1,7 @@ package test import ( + "regexp" "strings" "testing" @@ -481,3 +482,54 @@ resource "test_resource_list" "b" { }, }) } + +func TestResourceList_dynamicMinItems(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: strings.TrimSpace(` +variable "a" { + type = list(number) + default = [1] +} + +resource "test_resource_list" "b" { + dynamic "min_items" { + for_each = var.a + content { + val = "foo" + } + } +} + `), + ExpectError: regexp.MustCompile(`attribute supports 2`), + }, + resource.TestStep{ + Config: strings.TrimSpace(` +resource "test_resource_list" "a" { + dependent_list { + val = "a" + } + + dependent_list { + val = "b" + } +} +resource "test_resource_list" "b" { + list_block { + string = "constant" + } + dynamic "min_items" { + for_each = test_resource_list.a.computed_list + content { + val = min_items.value + } + } +} + `), + }, + }, + }) +} From c41eb0e6e4acb590c7b79d119c322b50b1a7d032 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 31 May 2019 20:09:20 -0500 Subject: [PATCH 2/3] re-validate config during Plan The config is statically validated early on for structural issues, but the provider can't validate any inputs that were unknown at the time. Run ValidateResourceTypeConfig during Plan, so that the provider can validate the final config values, including those interpolated from other resources. --- terraform/context_plan_test.go | 44 ++++++++++++++++++++++++++++++++++ terraform/eval_diff.go | 14 +++++++++++ 2 files changed, 58 insertions(+) diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 237d376e3..d1a45d62e 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -2,6 +2,7 @@ package terraform import ( "bytes" + "errors" "fmt" "os" "reflect" @@ -5724,6 +5725,49 @@ resource "aws_instance" "foo" { } } +func TestContext2Plan_variableValidation(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +variable "x" { + default = "bar" +} + +resource "aws_instance" "foo" { + foo = var.x +}`, + }) + + p := testProvider("aws") + p.ValidateResourceTypeConfigFn = func(req providers.ValidateResourceTypeConfigRequest) (resp providers.ValidateResourceTypeConfigResponse) { + foo := req.Config.GetAttr("foo").AsString() + if foo == "bar" { + resp.Diagnostics = resp.Diagnostics.Append(errors.New("foo cannot be bar")) + } + return + } + + p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) { + resp.PlannedState = req.ProposedNewState + return + } + + ctx := testContext2(t, &ContextOpts{ + Config: m, + ProviderResolver: providers.ResolverFixed( + map[string]providers.Factory{ + "aws": testProviderFuncFixed(p), + }, + ), + }) + + _, diags := ctx.Plan() + if !diags.HasErrors() { + // Should get this error: + // Unsupported attribute: This object does not have an attribute named "missing" + t.Fatal("succeeded; want errors") + } +} + func checkVals(t *testing.T, expected, got cty.Value) { t.Helper() if !cmp.Equal(expected, got, valueComparer, typeComparer, equateEmpty) { diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index b7acfb06d..ecf489258 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -174,6 +174,20 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { } } + log.Printf("[TRACE] Re-validating config for %q", n.Addr.Absolute(ctx.Path())) + // Allow the provider to validate the final set of values. + // The config was statically validated early on, but there may have been + // unknown values which the provider could not validate at the time. + validateResp := provider.ValidateResourceTypeConfig( + providers.ValidateResourceTypeConfigRequest{ + TypeName: n.Addr.Resource.Type, + Config: configVal, + }, + ) + if validateResp.Diagnostics.HasErrors() { + return nil, validateResp.Diagnostics.InConfigBody(config.Config).Err() + } + // The provider gets an opportunity to customize the proposed new value, // which in turn produces the _planned_ new value. resp := provider.PlanResourceChange(providers.PlanResourceChangeRequest{ From 2b200dd9bb4d5de379e22749cf5f2e2a559b3004 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 1 Jun 2019 11:40:54 -0400 Subject: [PATCH 3/3] re-validate data source config during read Just like resources, early data soruce validation will contain many unknown values. Re-validate once we have the config config. --- terraform/context_refresh_test.go | 40 +++++++++++++++++++++++++++++++ terraform/eval_read_data.go | 11 +++++++++ 2 files changed, 51 insertions(+) diff --git a/terraform/context_refresh_test.go b/terraform/context_refresh_test.go index 0f99abb95..7f6670296 100644 --- a/terraform/context_refresh_test.go +++ b/terraform/context_refresh_test.go @@ -1866,3 +1866,43 @@ test_thing.bar: } } } + +func TestContext2Refresh_dataValidation(t *testing.T) { + m := testModuleInline(t, map[string]string{ + "main.tf": ` +data "aws_data_source" "foo" { + foo = "bar" +} +`, + }) + + p := testProvider("aws") + p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) (resp providers.PlanResourceChangeResponse) { + resp.PlannedState = req.ProposedNewState + return + } + p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) (resp providers.ReadDataSourceResponse) { + resp.State = req.Config + return + } + + ctx := testContext2(t, &ContextOpts{ + Config: m, + ProviderResolver: providers.ResolverFixed( + map[string]providers.Factory{ + "aws": testProviderFuncFixed(p), + }, + ), + }) + + _, diags := ctx.Refresh() + if diags.HasErrors() { + // Should get this error: + // Unsupported attribute: This object does not have an attribute named "missing" + t.Fatal(diags.Err()) + } + + if !p.ValidateDataSourceConfigCalled { + t.Fatal("ValidateDataSourceConfig not called during plan") + } +} diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go index 34f2d60ad..728719a7a 100644 --- a/terraform/eval_read_data.go +++ b/terraform/eval_read_data.go @@ -179,6 +179,17 @@ func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { ) } + log.Printf("[TRACE] Re-validating config for %s", absAddr) + validateResp := provider.ValidateDataSourceConfig( + providers.ValidateDataSourceConfigRequest{ + TypeName: n.Addr.Resource.Type, + Config: configVal, + }, + ) + if validateResp.Diagnostics.HasErrors() { + return nil, validateResp.Diagnostics.InConfigBody(n.Config.Config).Err() + } + // If we get down here then our configuration is complete and we're read // to actually call the provider to read the data. log.Printf("[TRACE] EvalReadData: %s configuration is complete, so reading from provider", absAddr)