Skip to content

Commit 50dedf6

Browse files
EmyrkEmyrk
committed
treat unknown variables as nil
1 parent 8154589 commit 50dedf6

File tree

6 files changed

+123
-56
lines changed

6 files changed

+123
-56
lines changed

‎extract/variable.go‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func VariableFromBlock(block *terraform.Block) types.Variable {
2929
}
3030
return types.Variable{
3131
Name: block.Label(),
32-
Diagnostics: hcl.Diagnostics{&hcl.Diagnostic{
32+
Diagnostics: types.Diagnostics{&hcl.Diagnostic{
3333
Severity: hcl.DiagError,
3434
Summary: "Failed to decode variable type for " + block.Label(),
3535
Detail: err.Error(),
@@ -55,23 +55,22 @@ func VariableFromBlock(block *terraform.Block) types.Variable {
5555
val = defaults.Apply(val)
5656
}
5757

58-
valOK := !val.IsNull() && val.IsWhollyKnown()
59-
typedVal, err := convert.Convert(val, valType)
60-
if err != nil && valOK {
61-
return types.Variable{
62-
Name: block.Label(),
63-
Diagnostics: hcl.Diagnostics{&hcl.Diagnostic{
64-
Severity: hcl.DiagError,
65-
Summary: fmt.Sprintf("Failed to convert variable default value to type %q for %q",
66-
valType.FriendlyNameForConstraint(), block.Label()),
67-
Detail: err.Error(),
68-
Subject: &defSubject,
69-
}},
70-
}
71-
}
58+
canConvert := !val.IsNull() && val.IsWhollyKnown() && valType != cty.NilType
7259

73-
// If the new converted value is ok, use it.
74-
if err == nil {
60+
if canConvert {
61+
typedVal, err := convert.Convert(val, valType)
62+
if err != nil {
63+
return types.Variable{
64+
Name: block.Label(),
65+
Diagnostics: types.Diagnostics{&hcl.Diagnostic{
66+
Severity: hcl.DiagError,
67+
Summary: fmt.Sprintf("Failed to convert variable default value to type %q for %q",
68+
valType.FriendlyNameForConstraint(), block.Label()),
69+
Detail: err.Error(),
70+
Subject: &defSubject,
71+
}},
72+
}
73+
}
7574
val = typedVal
7675
}
7776
} else {
@@ -84,6 +83,5 @@ func VariableFromBlock(block *terraform.Block) types.Variable {
8483
Description: optionalString(block, "description"),
8584
Nullable: optionalBoolean(block, "nullable"),
8685
Sensitive: optionalBoolean(block, "sensitive"),
87-
Ephemeral: optionalBoolean(block, "ephemeral"),
8886
}
8987
}

‎preview_test.go‎

Lines changed: 81 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ func Test_Extract(t *testing.T) {
8080
"numerical": ap().value("5"),
8181
},
8282
variables: map[string]assertVariable{
83-
"string": av().def(cty.StringVal("Hello, world!")).typeEq(cty.String),
83+
"string": av().def(cty.StringVal("Hello, world!")).typeEq(cty.String).
84+
description("test").nullable(true).sensitive(true),
8485
"number": av().def(cty.NumberIntVal(7)).typeEq(cty.Number),
8586
"boolean": av().def(cty.BoolVal(true)).typeEq(cty.Bool),
8687
"coerce_string": av().def(cty.StringVal("5")).typeEq(cty.String),
@@ -162,6 +163,11 @@ func Test_Extract(t *testing.T) {
162163
"indexed_0": ap(),
163164
"indexed_1": ap(),
164165
},
166+
variables: map[string]assertVariable{
167+
"regions": av().def(cty.SetVal([]cty.Value{
168+
cty.StringVal("us"), cty.StringVal("eu"), cty.StringVal("au"),
169+
})).typeEq(cty.Set(cty.String)),
170+
},
165171
},
166172
{
167173
name: "external docker resource without plan data",
@@ -235,6 +241,9 @@ func Test_Extract(t *testing.T) {
235241
def("m7gd.8xlarge").
236242
value("m7gd.8xlarge"),
237243
},
244+
variables: map[string]assertVariable{
245+
"regions": av().typeEq(cty.List(cty.String)),
246+
},
238247
},
239248
{
240249
name: "empty file",
@@ -447,6 +456,9 @@ func Test_Extract(t *testing.T) {
447456
"team": ap().optVals("frontend", "backend", "fullstack"),
448457
"jetbrains_ide": ap(),
449458
},
459+
variables: map[string]assertVariable{
460+
"security": av().def(cty.StringVal("high")).typeEq(cty.String),
461+
},
450462
},
451463
{
452464
name: "count",
@@ -541,6 +553,17 @@ func Test_Extract(t *testing.T) {
541553
optVals("GO", "IU", "PY").
542554
optNames("GoLand 2024.3", "IntelliJ IDEA Ultimate 2024.3", "PyCharm Professional 2024.3"),
543555
},
556+
variables: map[string]assertVariable{
557+
"jetbrains_ides": av().typeEq(cty.List(cty.String)).description("The list of IDE product codes."),
558+
"releases_base_link": av(),
559+
"channel": av(),
560+
"download_base_link": av(),
561+
"arch": av(),
562+
"jetbrains_ide_versions": av().typeEq(cty.Map(cty.Object(map[string]cty.Type{
563+
"build_number": cty.String,
564+
"version": cty.String,
565+
}))),
566+
},
544567
},
545568
{
546569
name: "tfvars_from_file",
@@ -554,6 +577,12 @@ func Test_Extract(t *testing.T) {
554577
"variable_values": ap().
555578
def("alex").optVals("alex", "bob", "claire", "jason"),
556579
},
580+
variables: map[string]assertVariable{
581+
"one": av(),
582+
"two": av(),
583+
"three": av(),
584+
"four": av(),
585+
},
557586
},
558587
{
559588
name: "tfvars_from_input",
@@ -572,6 +601,12 @@ func Test_Extract(t *testing.T) {
572601
"variable_values": ap().
573602
def("andrew").optVals("andrew", "bill", "carter", "jason"),
574603
},
604+
variables: map[string]assertVariable{
605+
"one": av(),
606+
"two": av(),
607+
"three": av(),
608+
"four": av(),
609+
},
575610
},
576611
{
577612
name: "unknownoption",
@@ -583,6 +618,10 @@ func Test_Extract(t *testing.T) {
583618
"unknown": apWithDiags().
584619
errorDiagnostics("The set of options cannot be resolved"),
585620
},
621+
variables: map[string]assertVariable{
622+
// For now, unknown values are treated as nil :shrug:
623+
"unknown": av().def(cty.NilVal),
624+
},
586625
},
587626
} {
588627
t.Run(tc.name, func(t *testing.T) {
@@ -671,6 +710,25 @@ func av() assertVariable {
671710
}
672711
}
673712

713+
func avWithDiags() assertVariable {
714+
return func(t *testing.T, parameter types.Variable) {}
715+
}
716+
717+
func (a assertVariable) errorDiagnostics(patterns ...string) assertVariable {
718+
return a.diagnostics(hcl.DiagError, patterns...)
719+
}
720+
721+
func (a assertVariable) warnDiagnostics(patterns ...string) assertVariable {
722+
return a.diagnostics(hcl.DiagWarning, patterns...)
723+
}
724+
725+
func (a assertVariable) diagnostics(sev hcl.DiagnosticSeverity, patterns ...string) assertVariable {
726+
shadow := patterns
727+
return a.extend(func(t *testing.T, v types.Variable) {
728+
assertDiags(t, sev, v.Diagnostics, shadow...)
729+
})
730+
}
731+
674732
func (a assertVariable) nullable(n bool) assertVariable {
675733
return a.extend(func(t *testing.T, v types.Variable) {
676734
assert.Equal(t, v.Nullable, n, "variable nullable check")
@@ -690,7 +748,6 @@ func (a assertVariable) def(def cty.Value) assertVariable {
690748
got, _ := hclext.AsString(v.Default)
691749
t.Logf("Expected: %s, Value: %s", exp, got)
692750
}
693-
694751
})
695752
}
696753

@@ -700,12 +757,6 @@ func (a assertVariable) sensitive(s bool) assertVariable {
700757
})
701758
}
702759

703-
func (a assertVariable) ephemeral(e bool) assertVariable {
704-
return a.extend(func(t *testing.T, v types.Variable) {
705-
assert.Equal(t, v.Ephemeral, e, "variable ephemeral check")
706-
})
707-
}
708-
709760
func (a assertVariable) description(d string) assertVariable {
710761
return a.extend(func(t *testing.T, v types.Variable) {
711762
assert.Equal(t, v.Description, d, "variable description check")
@@ -736,23 +787,7 @@ func (a assertParam) warnDiagnostics(patterns ...string) assertParam {
736787
func (a assertParam) diagnostics(sev hcl.DiagnosticSeverity, patterns ...string) assertParam {
737788
shadow := patterns
738789
return a.extend(func(t *testing.T, parameter types.Parameter) {
739-
checks := make([]string, len(shadow))
740-
copy(checks, shadow)
741-
742-
DiagLoop:
743-
for _, diag := range parameter.Diagnostics {
744-
if diag.Severity != sev {
745-
continue
746-
}
747-
for i, pat := range checks {
748-
if strings.Contains(diag.Summary, pat) || strings.Contains(diag.Detail, pat) {
749-
checks = append(checks[:i], checks[i+1:]...)
750-
break DiagLoop
751-
}
752-
}
753-
}
754-
755-
assert.Equal(t, []string{}, checks, "missing expected diagnostic errors")
790+
assertDiags(t, sev, parameter.Diagnostics, shadow...)
756791
})
757792
}
758793

@@ -855,3 +890,24 @@ func (a assertVariable) extend(f assertVariable) assertVariable {
855890
f(t, v)
856891
}
857892
}
893+
894+
func assertDiags(t *testing.T, sev hcl.DiagnosticSeverity, diags types.Diagnostics, patterns ...string) {
895+
t.Helper()
896+
checks := make([]string, len(patterns))
897+
copy(checks, patterns)
898+
899+
DiagLoop:
900+
for _, diag := range diags {
901+
if diag.Severity != sev {
902+
continue
903+
}
904+
for i, pat := range checks {
905+
if strings.Contains(diag.Summary, pat) || strings.Contains(diag.Detail, pat) {
906+
checks = append(checks[:i], checks[i+1:]...)
907+
break DiagLoop
908+
}
909+
}
910+
}
911+
912+
assert.Equal(t, []string{}, checks, "missing expected diagnostic errors")
913+
}

‎testdata/static/main.tf‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ terraform {
1414

1515
variable "string" {
1616
default = "Hello, world!"
17+
nullable = true
18+
sensitive = true
19+
description = "test"
1720
}
1821

1922
variable "number" {

‎testdata/unknownoption/main.tf‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ terraform {
1313
}
1414
}
1515

16+
variable "unknown" {
17+
default = data.docker_registry_image.ubuntu.sha256_digest
18+
}
19+
1620
data "coder_parameter" "unknown" {
1721
name = "unknown"
1822
display_name = "Unknown Option Example"

‎types/terraform.go‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,19 @@
11
package types
22

33
import (
4-
"github.com/hashicorp/hcl/v2"
54
"github.com/zclconf/go-cty/cty"
65
)
76

87
type Variable struct {
9-
Name string
10-
Default cty.Value
11-
Type cty.Type
12-
Description string
13-
Nullable bool
14-
Sensitive bool
15-
Ephemeral bool
8+
Name string `json:"name"`
9+
// Unsure how cty values json marshal
10+
Default cty.Value `json:"default"`
11+
Type cty.Type `json:"type"`
12+
Description string `json:"description"`
13+
Nullable bool `json:"nullable"`
14+
Sensitive bool `json:"sensitive"`
1615

1716
// Variables also have 'Validation', which is currently not implemented.
1817

19-
Diagnostics hcl.Diagnostics
18+
Diagnostics Diagnostics `json:"diagnostics"`
2019
}

‎variables.go‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,17 @@ import (
1111
)
1212

1313
func variables(modules terraform.Modules) []types.Variable {
14-
variableBlocks := modules.GetBlocks().OfType("variable")
15-
vars := make([]types.Variable, 0, len(variableBlocks))
16-
for _, block := range variableBlocks {
17-
vars = append(vars, extract.VariableFromBlock(block))
14+
vars := make([]types.Variable, 0)
15+
16+
for _, mod := range modules {
17+
// Only extract variables from root modules. Child modules have their
18+
// vars set in the parent module.
19+
if mod.Parent() == nil {
20+
variableBlocks := mod.GetBlocks().OfType("variable")
21+
for _, block := range variableBlocks {
22+
vars = append(vars, extract.VariableFromBlock(block))
23+
}
24+
}
1825
}
1926

2027
// Sort the variables by name for consistency

0 commit comments

Comments
 (0)