From 1f01ba4dbcfb8850a3f3c9a9c3c2a426208e1794 Mon Sep 17 00:00:00 2001 From: Brandon Croft Date: Tue, 9 Nov 2021 17:02:59 -0700 Subject: [PATCH] fix(run variables): support all variable types (map, list, bool, number, null, string) All run variables remain encoded as strings in the API but will now be expressed as an HCL value to be evaluated correctly by the remote terraform. Previously, only strings were supported. Examples: string: `"quoted literal"` (strings must be quoted) map: `{ foo = "bar" }` list: `["foo", "bar"]` bool: `true` null: `null` number: `0.0001` This requires the API to anticipate that all run variables will be HCL values --- internal/cloud/backend_plan_test.go | 4 +- internal/cloud/cloud_variables.go | 20 ++---- internal/cloud/cloud_variables_test.go | 92 ++++++++++++++++++++++---- 3 files changed, 85 insertions(+), 31 deletions(-) diff --git a/internal/cloud/backend_plan_test.go b/internal/cloud/backend_plan_test.go index ba2508091..162a9841a 100644 --- a/internal/cloud/backend_plan_test.go +++ b/internal/cloud/backend_plan_test.go @@ -477,7 +477,7 @@ func TestCloud_planWithRequiredVariables(t *testing.T) { defer configCleanup() defer done(t) - op.Variables = testVariables(terraform.ValueFromCLIArg, "foo") // "bar" variable value missing + op.Variables = testVariables(terraform.ValueFromCLIArg, "foo") // "bar" variable defined in config is missing op.Workspace = testBackendSingleWorkspaceName run, err := b.Operation(context.Background(), op) @@ -487,7 +487,7 @@ func TestCloud_planWithRequiredVariables(t *testing.T) { <-run.Done() // The usual error of a required variable being missing is deferred and the operation - // is successful + // is successful. if run.Result != backend.OperationSuccess { t.Fatal("expected plan operation to succeed") } diff --git a/internal/cloud/cloud_variables.go b/internal/cloud/cloud_variables.go index 9a9002828..af6d6afcf 100644 --- a/internal/cloud/cloud_variables.go +++ b/internal/cloud/cloud_variables.go @@ -1,14 +1,11 @@ package cloud import ( - "encoding/json" - "fmt" - + "github.com/hashicorp/hcl/v2/hclwrite" "github.com/hashicorp/terraform/internal/backend" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" - ctyjson "github.com/zclconf/go-cty/cty/json" ) func allowedSourceType(source terraform.ValueSourceType) bool { @@ -17,7 +14,7 @@ func allowedSourceType(source terraform.ValueSourceType) bool { // ParseCloudRunVariables accepts a mapping of unparsed values and a mapping of variable // declarations and returns a name/value variable map appropriate for an API run context, -// that is, containing declared string variables only sourced from non-file inputs like CLI args +// that is, containing variables only sourced from non-file inputs like CLI args // and environment variables. However, all variable parsing diagnostics are returned // in order to allow callers to short circuit cloud runs that contain variable // declaration or parsing errors. The only exception is that missing required values are not @@ -36,16 +33,9 @@ func ParseCloudRunVariables(vv map[string]backend.UnparsedVariableValue, decls m continue } - valueData, err := ctyjson.Marshal(v.Value, v.Value.Type()) - if err != nil { - return nil, diags.Append(fmt.Errorf("error marshaling input variable value as json: %w", err)) - } - var variableValue string - if err = json.Unmarshal(valueData, &variableValue); err != nil { - // This should never happen since cty marshaled the value to begin with without error - return nil, diags.Append(fmt.Errorf("error unmarshaling run variable: %w", err)) - } - ret[name] = variableValue + // RunVariables are always expressed as HCL strings + tokens := hclwrite.TokensForValue(v.Value) + ret[name] = string(tokens.Bytes()) } return ret, diags diff --git a/internal/cloud/cloud_variables_test.go b/internal/cloud/cloud_variables_test.go index f8a8a5f1c..9780f788c 100644 --- a/internal/cloud/cloud_variables_test.go +++ b/internal/cloud/cloud_variables_test.go @@ -15,11 +15,16 @@ import ( func TestParseCloudRunVariables(t *testing.T) { t.Run("populates variables from allowed sources", func(t *testing.T) { vv := map[string]backend.UnparsedVariableValue{ - "undeclared": testUnparsedVariableValue{source: terraform.ValueFromCLIArg, value: "0"}, - "declaredFromConfig": testUnparsedVariableValue{source: terraform.ValueFromConfig, value: "1"}, - "declaredFromNamedFile": testUnparsedVariableValue{source: terraform.ValueFromNamedFile, value: "2"}, - "declaredFromCLIArg": testUnparsedVariableValue{source: terraform.ValueFromCLIArg, value: "3"}, - "declaredFromEnvVar": testUnparsedVariableValue{source: terraform.ValueFromEnvVar, value: "4"}, + "undeclared": testUnparsedVariableValue{source: terraform.ValueFromCLIArg, value: cty.StringVal("0")}, + "declaredFromConfig": testUnparsedVariableValue{source: terraform.ValueFromConfig, value: cty.StringVal("1")}, + "declaredFromNamedFileMapString": testUnparsedVariableValue{source: terraform.ValueFromNamedFile, value: cty.MapVal(map[string]cty.Value{"foo": cty.StringVal("bar")})}, + "declaredFromNamedFileBool": testUnparsedVariableValue{source: terraform.ValueFromNamedFile, value: cty.BoolVal(true)}, + "declaredFromNamedFileNumber": testUnparsedVariableValue{source: terraform.ValueFromNamedFile, value: cty.NumberIntVal(2)}, + "declaredFromNamedFileListString": testUnparsedVariableValue{source: terraform.ValueFromNamedFile, value: cty.ListVal([]cty.Value{cty.StringVal("2a"), cty.StringVal("2b")})}, + "declaredFromNamedFileNull": testUnparsedVariableValue{source: terraform.ValueFromNamedFile, value: cty.NullVal(cty.String)}, + "declaredFromNamedMapComplex": testUnparsedVariableValue{source: terraform.ValueFromNamedFile, value: cty.MapVal(map[string]cty.Value{"foo": cty.ObjectVal(map[string]cty.Value{"qux": cty.ListVal([]cty.Value{cty.BoolVal(true), cty.BoolVal(false)})})})}, + "declaredFromCLIArg": testUnparsedVariableValue{source: terraform.ValueFromCLIArg, value: cty.StringVal("3")}, + "declaredFromEnvVar": testUnparsedVariableValue{source: terraform.ValueFromEnvVar, value: cty.StringVal("4")}, } decls := map[string]*configs.Variable{ @@ -34,11 +39,66 @@ func TestParseCloudRunVariables(t *testing.T) { End: hcl.Pos{Line: 2, Column: 1, Byte: 0}, }, }, - "declaredFromNamedFile": { - Name: "declaredFromNamedFile", + "declaredFromNamedFileMapString": { + Name: "declaredFromNamedFileMapString", + Type: cty.Map(cty.String), + ConstraintType: cty.Map(cty.String), + ParsingMode: configs.VariableParseHCL, + DeclRange: hcl.Range{ + Filename: "fake.tf", + Start: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + }, + }, + "declaredFromNamedFileBool": { + Name: "declaredFromNamedFileBool", + Type: cty.Bool, + ConstraintType: cty.Bool, + ParsingMode: configs.VariableParseLiteral, + DeclRange: hcl.Range{ + Filename: "fake.tf", + Start: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + }, + }, + "declaredFromNamedFileNumber": { + Name: "declaredFromNamedFileNumber", + Type: cty.Number, + ConstraintType: cty.Number, + ParsingMode: configs.VariableParseLiteral, + DeclRange: hcl.Range{ + Filename: "fake.tf", + Start: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + }, + }, + "declaredFromNamedFileListString": { + Name: "declaredFromNamedFileListString", + Type: cty.List(cty.String), + ConstraintType: cty.List(cty.String), + ParsingMode: configs.VariableParseHCL, + DeclRange: hcl.Range{ + Filename: "fake.tf", + Start: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + }, + }, + "declaredFromNamedFileNull": { + Name: "declaredFromNamedFileNull", Type: cty.String, ConstraintType: cty.String, - ParsingMode: configs.VariableParseLiteral, + ParsingMode: configs.VariableParseHCL, + DeclRange: hcl.Range{ + Filename: "fake.tf", + Start: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + End: hcl.Pos{Line: 2, Column: 1, Byte: 0}, + }, + }, + "declaredFromNamedMapComplex": { + Name: "declaredFromNamedMapComplex", + Type: cty.DynamicPseudoType, + ConstraintType: cty.DynamicPseudoType, + ParsingMode: configs.VariableParseHCL, DeclRange: hcl.Range{ Filename: "fake.tf", Start: hcl.Pos{Line: 2, Column: 1, Byte: 0}, @@ -81,10 +141,14 @@ func TestParseCloudRunVariables(t *testing.T) { }, } wantVals := make(map[string]string) - - wantVals["declaredFromNamedFile"] = "2" - wantVals["declaredFromCLIArg"] = "3" - wantVals["declaredFromEnvVar"] = "4" + wantVals["declaredFromNamedFileBool"] = "true" + wantVals["declaredFromNamedFileNumber"] = "2" + wantVals["declaredFromNamedFileListString"] = `["2a", "2b"]` + wantVals["declaredFromNamedFileNull"] = "null" + wantVals["declaredFromNamedFileMapString"] = "{\n foo = \"bar\"\n}" + wantVals["declaredFromNamedMapComplex"] = "{\n foo = {\n qux = [true, false]\n }\n}" + wantVals["declaredFromCLIArg"] = `"3"` + wantVals["declaredFromEnvVar"] = `"4"` gotVals, diags := ParseCloudRunVariables(vv, decls) if diff := cmp.Diff(wantVals, gotVals, cmp.Comparer(cty.Value.RawEquals)); diff != "" { @@ -103,12 +167,12 @@ func TestParseCloudRunVariables(t *testing.T) { type testUnparsedVariableValue struct { source terraform.ValueSourceType - value string + value cty.Value } func (v testUnparsedVariableValue) ParseVariableValue(mode configs.VariableParsingMode) (*terraform.InputValue, tfdiags.Diagnostics) { return &terraform.InputValue{ - Value: cty.StringVal(v.value), + Value: v.value, SourceType: v.source, SourceRange: tfdiags.SourceRange{ Filename: "fake.tfvars",