From 6055cb632e65a9e5af21d100580da85a37377c29 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 24 May 2019 16:13:29 -0400 Subject: [PATCH] filter unknowns from simple lists and maps in sdk While there was already a check in the sdk to filter unknowns from validation, it missed the case where those were in simple lists and maps. --- builtin/providers/test/resource_map.go | 10 +++++++ builtin/providers/test/resource_map_test.go | 29 +++++++++++++++++++ helper/schema/schema.go | 32 ++++++++++++++++++--- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/builtin/providers/test/resource_map.go b/builtin/providers/test/resource_map.go index 38f684106..12b9e2535 100644 --- a/builtin/providers/test/resource_map.go +++ b/builtin/providers/test/resource_map.go @@ -3,6 +3,7 @@ package test import ( "fmt" + "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/helper/schema" ) @@ -22,6 +23,15 @@ func testResourceMap() *schema.Resource { Type: schema.TypeMap, Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, + ValidateFunc: func(v interface{}, _ string) ([]string, []error) { + errs := []error{} + for k, v := range v.(map[string]interface{}) { + if v == config.UnknownVariableValue { + errs = append(errs, fmt.Errorf("unknown value in ValidateFunc: %q=%q", k, v)) + } + } + return nil, errs + }, }, "map_values": { Type: schema.TypeMap, diff --git a/builtin/providers/test/resource_map_test.go b/builtin/providers/test/resource_map_test.go index 6e279b7f5..0d82d5f4f 100644 --- a/builtin/providers/test/resource_map_test.go +++ b/builtin/providers/test/resource_map_test.go @@ -31,6 +31,35 @@ resource "test_resource_map" "foobar" { }) } +func TestResourceMap_basicWithVars(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + Providers: testAccProviders, + CheckDestroy: testAccCheckResourceDestroy, + Steps: []resource.TestStep{ + { + Config: ` +variable "a" { + default = "a" +} + +variable "b" { + default = "b" +} + +resource "test_resource_map" "foobar" { + name = "test" + map_of_three = { + one = var.a + two = var.b + empty = "" + } +}`, + Check: resource.ComposeTestCheckFunc(), + }, + }, + }) +} + func TestResourceMap_computedMap(t *testing.T) { resource.UnitTest(t, resource.TestCase{ Providers: testAccProviders, diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 6a3c15a64..26b180e03 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -1365,10 +1365,12 @@ func (m schemaMap) validate( "%q: this field cannot be set", k)} } - if raw == config.UnknownVariableValue { - // If the value is unknown then we can't validate it yet. - // In particular, this avoids spurious type errors where downstream - // validation code sees UnknownVariableValue as being just a string. + // If the value is unknown then we can't validate it yet. + // In particular, this avoids spurious type errors where downstream + // validation code sees UnknownVariableValue as being just a string. + // The SDK has to allow the unknown value through initially, so that + // Required fields set via an interpolated value are accepted. + if !isWhollyKnown(raw) { return nil, nil } @@ -1380,6 +1382,28 @@ func (m schemaMap) validate( return m.validateType(k, raw, schema, c) } +// isWhollyKnown returns false if the argument contains an UnknownVariableValue +func isWhollyKnown(raw interface{}) bool { + switch raw := raw.(type) { + case string: + if raw == config.UnknownVariableValue { + return false + } + case []interface{}: + for _, v := range raw { + if !isWhollyKnown(v) { + return false + } + } + case map[string]interface{}: + for _, v := range raw { + if !isWhollyKnown(v) { + return false + } + } + } + return true +} func (m schemaMap) validateConflictingAttributes( k string, schema *Schema,