From d7cb69e96d6e9a8d82fa473d17c6a25dc1d697f3 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Mon, 17 Oct 2022 10:30:57 +0200 Subject: [PATCH 1/5] Convert variable types before applying defaults --- internal/terraform/context_apply_test.go | 2 +- internal/terraform/eval_variable.go | 16 +++--- internal/terraform/eval_variable_test.go | 62 ++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 9 deletions(-) diff --git a/internal/terraform/context_apply_test.go b/internal/terraform/context_apply_test.go index 4f16453310ee..150141df5fe4 100644 --- a/internal/terraform/context_apply_test.go +++ b/internal/terraform/context_apply_test.go @@ -12027,7 +12027,7 @@ output "out" { Mode: plans.NormalMode, SetVariables: InputValues{ "in": &InputValue{ - Value: cty.MapVal(map[string]cty.Value{ + Value: cty.ObjectVal(map[string]cty.Value{ "required": cty.StringVal("boop"), }), SourceType: ValueFromCaller, diff --git a/internal/terraform/eval_variable.go b/internal/terraform/eval_variable.go index 167840e7c110..c355204b1046 100644 --- a/internal/terraform/eval_variable.go +++ b/internal/terraform/eval_variable.go @@ -90,14 +90,6 @@ func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, raw *In given = defaultVal // must be set, because we checked above that the variable isn't required } - // Apply defaults from the variable's type constraint to the given value, - // unless the given value is null. We do not apply defaults to top-level - // null values, as doing so could prevent assigning null to a nullable - // variable. - if cfg.TypeDefaults != nil && !given.IsNull() { - given = cfg.TypeDefaults.Apply(given) - } - val, err := convert.Convert(given, convertTy) if err != nil { log.Printf("[ERROR] prepareFinalInputVariableValue: %s has unsuitable type\n got: %s\n want: %s", addr, given.Type(), convertTy) @@ -140,6 +132,14 @@ func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, raw *In return cty.UnknownVal(cfg.Type), diags } + // Apply defaults from the variable's type constraint to the given value, + // unless the given value is null. We do not apply defaults to top-level + // null values, as doing so could prevent assigning null to a nullable + // variable. + if cfg.TypeDefaults != nil && !val.IsNull() { + val = cfg.TypeDefaults.Apply(val) + } + // By the time we get here, we know: // - val matches the variable's type constraint // - val is definitely not cty.NilVal, but might be a null value if the given was already null. diff --git a/internal/terraform/eval_variable_test.go b/internal/terraform/eval_variable_test.go index cb6c1bb2b816..7974203f2d07 100644 --- a/internal/terraform/eval_variable_test.go +++ b/internal/terraform/eval_variable_test.go @@ -68,6 +68,15 @@ func TestPrepareFinalInputVariableValue(t *testing.T) { nullable = false type = string } + variable "complex_type_with_nested_default_optional" { + type = set(object({ + name = string + schedules = set(object({ + name = string + cold_storage_after = optional(number, 10) + })) + })) + } ` cfg := testModuleInline(t, map[string]string{ "main.tf": cfgSrc, @@ -399,6 +408,59 @@ func TestPrepareFinalInputVariableValue(t *testing.T) { ``, }, + // complex types + + { + "complex_type_with_nested_default_optional", + cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("test1"), + "schedules": cty.SetVal([]cty.Value{ + cty.MapVal(map[string]cty.Value{ + "name": cty.StringVal("daily"), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("test2"), + "schedules": cty.SetVal([]cty.Value{ + cty.MapVal(map[string]cty.Value{ + "name": cty.StringVal("daily"), + }), + cty.MapVal(map[string]cty.Value{ + "name": cty.StringVal("weekly"), + "cold_storage_after": cty.StringVal("0"), + }), + }), + }), + }), + cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("test1"), + "schedules": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("daily"), + "cold_storage_after": cty.NumberIntVal(10), + }), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("test2"), + "schedules": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("daily"), + "cold_storage_after": cty.NumberIntVal(10), + }), + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("weekly"), + "cold_storage_after": cty.NumberIntVal(0), + }), + }), + }), + }), + ``, + }, + // sensitive { "constrained_string_sensitive_required", From 8859e4df4bac4873fb816964e07047a9bed09e57 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Wed, 19 Oct 2022 10:44:48 +0200 Subject: [PATCH 2/5] revert change to unrelated test --- internal/terraform/context_apply_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/terraform/context_apply_test.go b/internal/terraform/context_apply_test.go index 150141df5fe4..4f16453310ee 100644 --- a/internal/terraform/context_apply_test.go +++ b/internal/terraform/context_apply_test.go @@ -12027,7 +12027,7 @@ output "out" { Mode: plans.NormalMode, SetVariables: InputValues{ "in": &InputValue{ - Value: cty.ObjectVal(map[string]cty.Value{ + Value: cty.MapVal(map[string]cty.Value{ "required": cty.StringVal("boop"), }), SourceType: ValueFromCaller, From 26e594f3663fd340859388c51be9aaaa910107fd Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Wed, 19 Oct 2022 12:50:08 +0200 Subject: [PATCH 3/5] Add another test case to verify behaviour --- go.mod | 2 ++ go.sum | 8 ----- internal/terraform/eval_variable_test.go | 40 ++++++++++++++++++++++-- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/go.mod b/go.mod index cbadb7fe21c0..b1f0d995a569 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,7 @@ module github.com/hashicorp/terraform +replace github.com/zclconf/go-cty => ../../liamcervante/go-cty + require ( cloud.google.com/go v0.81.0 cloud.google.com/go/storage v1.10.0 diff --git a/go.sum b/go.sum index 3fb6279d5c3a..255f1fd90690 100644 --- a/go.sum +++ b/go.sum @@ -246,7 +246,6 @@ github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71 github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8= github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= -github.com/golang/protobuf v1.1.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= @@ -601,7 +600,6 @@ github.com/tombuildsstuff/giovanni v0.15.1 h1:CVRaLOJ7C/eercCrKIsarfJ4SZoGMdBL9Q github.com/tombuildsstuff/giovanni v0.15.1/go.mod h1:0TZugJPEtqzPlMpuJHYfXY6Dq2uLPrXf98D2XQSxNbA= github.com/ulikunitz/xz v0.5.8 h1:ERv8V6GKqVi23rgu5cj9pVfVzJbOqAY2Ntl88O6c2nQ= github.com/ulikunitz/xz v0.5.8/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= -github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk= github.com/vmihailenco/msgpack/v4 v4.3.12 h1:07s4sz9IReOgdikxLTKNbBdqDMLsjPKXwvCazn8G65U= github.com/vmihailenco/msgpack/v4 v4.3.12/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+NXzzngzBKDPIqw4= github.com/vmihailenco/tagparser v0.1.1 h1:quXMXlA39OCbd2wAdTsGDlK9RkOk6Wuw+x37wVyIuWY= @@ -616,11 +614,6 @@ github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= github.com/yuin/goldmark v1.4.0/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= -github.com/zclconf/go-cty v1.0.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= -github.com/zclconf/go-cty v1.1.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= -github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= -github.com/zclconf/go-cty v1.11.1 h1:UMMYDL4riBFaPdzjEWcDdWG7x/Adz8E8f9OX/MGR7V4= -github.com/zclconf/go-cty v1.11.1/go.mod h1:s9IfD1LK5ccNMSWCVFCE2rJfHiZgi7JijgeWIMfhLvA= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b h1:FosyBZYxY34Wul7O/MSKey3txpPYyCqVO5ZyceuQJEI= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8= github.com/zclconf/go-cty-yaml v1.0.2 h1:dNyg4QLTrv2IfJpm7Wtxi55ed5gLGOlPrZ6kMd51hY0= @@ -695,7 +688,6 @@ golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 h1:6zppjxzCulZykYSLyVD golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/net v0.0.0-20180530234432-1e491301e022/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= -golang.org/x/net v0.0.0-20180811021610-c39426892332/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181023162649-9b4f9f5ad519/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= diff --git a/internal/terraform/eval_variable_test.go b/internal/terraform/eval_variable_test.go index 7974203f2d07..15d250ceb193 100644 --- a/internal/terraform/eval_variable_test.go +++ b/internal/terraform/eval_variable_test.go @@ -70,13 +70,28 @@ func TestPrepareFinalInputVariableValue(t *testing.T) { } variable "complex_type_with_nested_default_optional" { type = set(object({ - name = string + name = string schedules = set(object({ - name = string + name = string cold_storage_after = optional(number, 10) })) })) } + variable "complex_type_with_nested_complex_types" { + type = object({ + name = string + nested_object = object({ + name = string + value = optional(string, "foo") + }) + nested_object_with_default = optional(object({ + name = string + value = optional(string, "bar") + }), { + name = "nested_object_with_default" + }) + }) + } ` cfg := testModuleInline(t, map[string]string{ "main.tf": cfgSrc, @@ -460,6 +475,27 @@ func TestPrepareFinalInputVariableValue(t *testing.T) { }), ``, }, + { + "complex_type_with_nested_complex_types", + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("object"), + "nested_object": cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("nested_object"), + }), + }), + cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("object"), + "nested_object": cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("nested_object"), + "value": cty.StringVal("foo"), + }), + "nested_object_with_default": cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("nested_object_with_default"), + "value": cty.StringVal("bar"), + }), + }), + ``, + }, // sensitive { From 8ea1ef5f237a81aff7a39eb71f2dc9e2af7b32ec Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Fri, 28 Oct 2022 15:21:32 +0200 Subject: [PATCH 4/5] update go-cty --- go.mod | 4 +--- go.sum | 8 ++++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index b1f0d995a569..78ebbb218807 100644 --- a/go.mod +++ b/go.mod @@ -1,7 +1,5 @@ module github.com/hashicorp/terraform -replace github.com/zclconf/go-cty => ../../liamcervante/go-cty - require ( cloud.google.com/go v0.81.0 cloud.google.com/go/storage v1.10.0 @@ -77,7 +75,7 @@ require ( github.com/tombuildsstuff/giovanni v0.15.1 github.com/xanzy/ssh-agent v0.3.1 github.com/xlab/treeprint v0.0.0-20161029104018-1d6e34225557 - github.com/zclconf/go-cty v1.11.1 + github.com/zclconf/go-cty v1.12.0 github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b github.com/zclconf/go-cty-yaml v1.0.2 golang.org/x/crypto v0.0.0-20220518034528-6f7dac969898 diff --git a/go.sum b/go.sum index 255f1fd90690..cb72eb778fd8 100644 --- a/go.sum +++ b/go.sum @@ -246,6 +246,7 @@ github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71 github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8= github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= +github.com/golang/protobuf v1.1.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= @@ -600,6 +601,7 @@ github.com/tombuildsstuff/giovanni v0.15.1 h1:CVRaLOJ7C/eercCrKIsarfJ4SZoGMdBL9Q github.com/tombuildsstuff/giovanni v0.15.1/go.mod h1:0TZugJPEtqzPlMpuJHYfXY6Dq2uLPrXf98D2XQSxNbA= github.com/ulikunitz/xz v0.5.8 h1:ERv8V6GKqVi23rgu5cj9pVfVzJbOqAY2Ntl88O6c2nQ= github.com/ulikunitz/xz v0.5.8/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= +github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk= github.com/vmihailenco/msgpack/v4 v4.3.12 h1:07s4sz9IReOgdikxLTKNbBdqDMLsjPKXwvCazn8G65U= github.com/vmihailenco/msgpack/v4 v4.3.12/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+NXzzngzBKDPIqw4= github.com/vmihailenco/tagparser v0.1.1 h1:quXMXlA39OCbd2wAdTsGDlK9RkOk6Wuw+x37wVyIuWY= @@ -614,6 +616,11 @@ github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= github.com/yuin/goldmark v1.4.0/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= +github.com/zclconf/go-cty v1.0.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= +github.com/zclconf/go-cty v1.1.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= +github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= +github.com/zclconf/go-cty v1.12.0 h1:F5E/vbilcrCtat9sYcEjlwwg1mDqbRTjyXR57nnx5sc= +github.com/zclconf/go-cty v1.12.0/go.mod h1:s9IfD1LK5ccNMSWCVFCE2rJfHiZgi7JijgeWIMfhLvA= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b h1:FosyBZYxY34Wul7O/MSKey3txpPYyCqVO5ZyceuQJEI= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b/go.mod h1:ZRKQfBXbGkpdV6QMzT3rU1kSTAnfu1dO8dPKjYprgj8= github.com/zclconf/go-cty-yaml v1.0.2 h1:dNyg4QLTrv2IfJpm7Wtxi55ed5gLGOlPrZ6kMd51hY0= @@ -688,6 +695,7 @@ golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 h1:6zppjxzCulZykYSLyVD golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/net v0.0.0-20180530234432-1e491301e022/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= +golang.org/x/net v0.0.0-20180811021610-c39426892332/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181023162649-9b4f9f5ad519/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= From 58cdcf9763fc08947280889a4ecee5903096f507 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Mon, 31 Oct 2022 13:57:06 +0100 Subject: [PATCH 5/5] Update internal/terraform/eval_variable.go Co-authored-by: alisdair --- internal/terraform/eval_variable.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/terraform/eval_variable.go b/internal/terraform/eval_variable.go index c355204b1046..c489e4f3bea6 100644 --- a/internal/terraform/eval_variable.go +++ b/internal/terraform/eval_variable.go @@ -132,8 +132,8 @@ func prepareFinalInputVariableValue(addr addrs.AbsInputVariableInstance, raw *In return cty.UnknownVal(cfg.Type), diags } - // Apply defaults from the variable's type constraint to the given value, - // unless the given value is null. We do not apply defaults to top-level + // Apply defaults from the variable's type constraint to the converted value, + // unless the converted value is null. We do not apply defaults to top-level // null values, as doing so could prevent assigning null to a nullable // variable. if cfg.TypeDefaults != nil && !val.IsNull() {