Merge pull request #27 from shihanng/optional

Support Optional Object Type Attributes

.github/workflows/main.yml

@@ -12,7 +12,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        version: ["1.19", "1.18", "1.17"]
+        version: ["1.20", "1.19", "1.18"]
 
     steps:
       - name: Set up Go
@@ -30,6 +30,7 @@ jobs:
         run: make test
 
       - name: Send coverage
+        if: ${{ matrix.version == '1.20' }}
         env:
           COVERALLS_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
@@ -46,4 +47,5 @@ jobs:
       - name: Set up golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.49.0
+          version: v1.52.2
+          args: --timeout=3m

.github/workflows/release.yml

@@ -16,7 +16,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v3
         with:
-          go-version: "1.19"
+          go-version: "1.20"
       - name: Run GoReleaser
         uses: goreleaser/goreleaser-action@v3
         with:

go.mod

@@ -1,30 +1,40 @@
 module github.com/shihanng/tfvar
 
-go 1.13
+go 1.18
 
 require (
-	github.com/agext/levenshtein v1.2.3 // indirect
 	github.com/cockroachdb/errors v1.7.3
+	github.com/hashicorp/hcl/v2 v2.16.2
+	github.com/sebdah/goldie/v2 v2.5.3
+	github.com/spf13/afero v1.5.1
+	github.com/spf13/cobra v1.0.0
+	github.com/stretchr/testify v1.6.1
+	github.com/zclconf/go-cty v1.13.1
+	go.uber.org/zap v1.16.0
+)
+
+require (
+	github.com/agext/levenshtein v1.2.3 // indirect
+	github.com/apparentlymart/go-textseg/v13 v13.0.0 // indirect
+	github.com/cockroachdb/logtags v0.0.0-20190617123548-eb05cc24525f // indirect
 	github.com/cockroachdb/redact v1.0.4 // indirect
+	github.com/cockroachdb/sentry-go v0.6.1-cockroachdb.2 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/google/go-cmp v0.5.4 // indirect
-	github.com/hashicorp/hcl/v2 v2.8.2
+	github.com/inconshreveable/mousetrap v1.0.0 // indirect
 	github.com/kr/pretty v0.2.1 // indirect
 	github.com/kr/text v0.2.0 // indirect
 	github.com/kylelemons/godebug v1.1.0 // indirect
 	github.com/mitchellh/go-wordwrap v1.0.1 // indirect
-	github.com/sebdah/goldie/v2 v2.5.3
+	github.com/pkg/errors v0.9.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/sergi/go-diff v1.2.0 // indirect
-	github.com/spf13/afero v1.5.1
-	github.com/spf13/cobra v1.0.0
 	github.com/spf13/pflag v1.0.5 // indirect
-	github.com/stretchr/testify v1.6.1
-	github.com/zclconf/go-cty v1.10.0
+	go.uber.org/atomic v1.7.0 // indirect
 	go.uber.org/multierr v1.6.0 // indirect
-	go.uber.org/zap v1.16.0
 	golang.org/x/lint v0.0.0-20201208152925-83fdc39ff7b5 // indirect
-	golang.org/x/mod v0.4.1 // indirect
-	golang.org/x/tools v0.0.0-20210115202250-e0d201561e39 // indirect
+	golang.org/x/text v0.9.0 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 // indirect
 	honnef.co/go/tools v0.0.1-2020.1.5 // indirect

pkg/configs/module.go

@@ -1,3 +1,6 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
 package configs
 
 import (

pkg/configs/named_values.go

@@ -24,10 +24,11 @@ type Variable struct {
 	Type cty.Type
 	// ConstraintType is used for decoding and type conversions, and may
 	// contain nested ObjectWithOptionalAttr types.
-
 	ConstraintType cty.Type
-	ParsingMode    VariableParsingMode
-	Sensitive      bool
+	TypeDefaults   *typeexpr.Defaults
+
+	ParsingMode VariableParsingMode
+	Sensitive   bool
 
 	DescriptionSet bool
 	SensitiveSet   bool
@@ -99,9 +100,10 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
 	}
 
 	if attr, exists := content.Attributes["type"]; exists {
-		ty, parseMode, tyDiags := decodeVariableType(attr.Expr)
+		ty, tyDefaults, parseMode, tyDiags := decodeVariableType(attr.Expr)
 		diags = append(diags, tyDiags...)
 		v.ConstraintType = ty
+		v.TypeDefaults = tyDefaults
 		v.Type = ty.WithoutOptionalAttributesDeep()
 		v.ParsingMode = parseMode
 	}
@@ -134,6 +136,14 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
 		// the type might not be set; we'll catch that during merge.
 		if v.ConstraintType != cty.NilType {
 			var err error
+			// If the type constraint has defaults, we must apply those
+			// defaults to the variable default value before type conversion,
+			// unless the default value is null. Null is excluded from the
+			// type default application process as a special case, to allow
+			// nullable variables to have a null default value.
+			if v.TypeDefaults != nil && !val.IsNull() {
+				val = v.TypeDefaults.Apply(val)
+			}
 			val, err = convert.Convert(val, v.ConstraintType)
 			if err != nil {
 				diags = append(diags, &hcl.Diagnostic{
@@ -172,7 +182,7 @@ func decodeVariableBlock(block *hcl.Block, override bool) (*Variable, hcl.Diagno
 	return v, diags
 }
 
-func decodeVariableType(expr hcl.Expression) (cty.Type, VariableParsingMode, hcl.Diagnostics) {
+func decodeVariableType(expr hcl.Expression) (cty.Type, *typeexpr.Defaults, VariableParsingMode, hcl.Diagnostics) {
 	if exprIsNativeQuotedString(expr) {
 		// If a user provides the pre-0.12 form of variable type argument where
 		// the string values "string", "list" and "map" are accepted, we
@@ -183,7 +193,7 @@ func decodeVariableType(expr hcl.Expression) (cty.Type, VariableParsingMode, hcl
 		// in the normal codepath below.
 		val, diags := expr.Value(nil)
 		if diags.HasErrors() {
-			return cty.DynamicPseudoType, VariableParseHCL, diags
+			return cty.DynamicPseudoType, nil, VariableParseHCL, diags
 		}
 		str := val.AsString()
 		switch str {
@@ -194,25 +204,25 @@ func decodeVariableType(expr hcl.Expression) (cty.Type, VariableParsingMode, hcl
 				Detail:   "Terraform 0.11 and earlier required type constraints to be given in quotes, but that form is now deprecated and will be removed in a future version of Terraform. Remove the quotes around \"string\".",
 				Subject:  expr.Range().Ptr(),
 			})
-			return cty.DynamicPseudoType, VariableParseLiteral, diags
+			return cty.DynamicPseudoType, nil, VariableParseLiteral, diags
 		case "list":
 			diags = append(diags, &hcl.Diagnostic{
 				Severity: hcl.DiagError,
 				Summary:  "Invalid quoted type constraints",
 				Detail:   "Terraform 0.11 and earlier required type constraints to be given in quotes, but that form is now deprecated and will be removed in a future version of Terraform. Remove the quotes around \"list\" and write list(string) instead to explicitly indicate that the list elements are strings.",
 				Subject:  expr.Range().Ptr(),
 			})
-			return cty.DynamicPseudoType, VariableParseHCL, diags
+			return cty.DynamicPseudoType, nil, VariableParseHCL, diags
 		case "map":
 			diags = append(diags, &hcl.Diagnostic{
 				Severity: hcl.DiagError,
 				Summary:  "Invalid quoted type constraints",
 				Detail:   "Terraform 0.11 and earlier required type constraints to be given in quotes, but that form is now deprecated and will be removed in a future version of Terraform. Remove the quotes around \"map\" and write map(string) instead to explicitly indicate that the map elements are strings.",
 				Subject:  expr.Range().Ptr(),
 			})
-			return cty.DynamicPseudoType, VariableParseHCL, diags
+			return cty.DynamicPseudoType, nil, VariableParseHCL, diags
 		default:
-			return cty.DynamicPseudoType, VariableParseHCL, hcl.Diagnostics{{
+			return cty.DynamicPseudoType, nil, VariableParseHCL, hcl.Diagnostics{{
 				Severity: hcl.DiagError,
 				Summary:  "Invalid legacy variable type hint",
 				Detail:   `To provide a full type expression, remove the surrounding quotes and give the type expression directly.`,
@@ -227,23 +237,23 @@ func decodeVariableType(expr hcl.Expression) (cty.Type, VariableParsingMode, hcl
 	// elements are consistent. This is the same as list(any) or map(any).
 	switch hcl.ExprAsKeyword(expr) {
 	case "list":
-		return cty.List(cty.DynamicPseudoType), VariableParseHCL, nil
+		return cty.List(cty.DynamicPseudoType), nil, VariableParseHCL, nil
 	case "map":
-		return cty.Map(cty.DynamicPseudoType), VariableParseHCL, nil
+		return cty.Map(cty.DynamicPseudoType), nil, VariableParseHCL, nil
 	}
 
-	ty, diags := typeexpr.TypeConstraint(expr)
+	ty, typeDefaults, diags := typeexpr.TypeConstraintWithDefaults(expr)
 	if diags.HasErrors() {
-		return cty.DynamicPseudoType, VariableParseHCL, diags
+		return cty.DynamicPseudoType, nil, VariableParseHCL, diags
 	}
 
 	switch {
 	case ty.IsPrimitiveType():
 		// Primitive types use literal parsing.
-		return ty, VariableParseLiteral, diags
+		return ty, typeDefaults, VariableParseLiteral, diags
 	default:
 		// Everything else uses HCL parsing
-		return ty, VariableParseHCL, diags
+		return ty, typeDefaults, VariableParseHCL, diags
 	}
 }
 

pkg/tfvar/testdata/defaults/main.tf

@@ -50,3 +50,14 @@ variable "password" {
   description = "the root password to use with the database"
   sensitive   = true
 }
+
+variable "with_optional_attribute" {
+  type = object({
+    a = string
+    b = optional(string)
+    c = optional(number, 127)
+  })
+  default = {
+    a = "val-a"
+  }
+}

pkg/tfvar/testdata/normal/main.tf

@@ -2,6 +2,11 @@ variable "resource_name" {}
 variable "instance_name" {
   default = "my-instance"
 }
+variable "object" {
+  type = object({
+    name = optional(string)
+  })
+}
 
 moved {
   from = aws_instance.a

pkg/tfvar/testdata/tfe_resource.golden.tf

@@ -61,3 +61,16 @@ resource "tfe_variable" "region" {
   workspace_id = null
   category     = "terraform"
 }
+
+resource "tfe_variable" "with_optional_attribute" {
+  key = "with_optional_attribute"
+  value = {
+    a = "val-a"
+    b = null
+    c = 127
+  }
+  sensitive    = false
+  description  = ""
+  workspace_id = null
+  category     = "terraform"
+}

pkg/tfvar/testdata/workspace_payload.golden.json

@@ -76,3 +76,16 @@
     }
   }
 }
+{
+  "data": {
+    "type": "vars",
+    "attributes": {
+      "key": "with_optional_attribute",
+      "value": "{ a = 'val-a', b = null, c = 127 }",
+      "description": "",
+      "category": "terraform",
+      "hcl": false,
+      "sensitive": false
+    }
+  }
+}

pkg/tfvar/tfvar_test.go

@@ -31,6 +31,7 @@ func TestLoad(t *testing.T) {
 			want: []Variable{
 				{Name: "resource_name", parsingMode: configs.VariableParseLiteral},
 				{Name: "instance_name", Value: cty.StringVal("my-instance"), parsingMode: configs.VariableParseLiteral},
+				{Name: "object", parsingMode: configs.VariableParseHCL},
 			},
 			assertion: assert.NoError,
 		},
@@ -68,6 +69,7 @@ export TF_VAR_docker_ports='[{ external = 8300, internal = 8301, protocol = "tcp
 export TF_VAR_instance_name='my-instance'
 export TF_VAR_password=''
 export TF_VAR_region=''
+export TF_VAR_with_optional_attribute='{ a = "val-a", b = null, c = 127 }'
 `
 	assert.Equal(t, expected, buf.String())
 }
@@ -96,6 +98,11 @@ docker_ports = [{
 instance_name = "my-instance"
 password      = null
 region        = null
+with_optional_attribute = {
+  a = "val-a"
+  b = null
+  c = 127
+}
 `
 	assert.Equal(t, expected, buf.String())
 }