From b1e7015f2724420cc107236ff4d5854650983e3e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 9 Jul 2025 09:28:29 -0600 Subject: [PATCH 1/5] feat: template variables added to input Input can override template vars, implementing the equivalent of '-var' in terraform. --- preview.go | 30 +++++++++-- preview_test.go | 32 ++++++++++++ testdata/tfvars/.auto.tfvars.json | 1 + testdata/tfvars/main.tf | 61 ++++++++++++++++++++++ testdata/tfvars/values.tfvars | 2 + tfvars/load.go | 85 +++++++++++++++++++++++++++++++ 6 files changed, 206 insertions(+), 5 deletions(-) create mode 100644 testdata/tfvars/.auto.tfvars.json create mode 100644 testdata/tfvars/main.tf create mode 100644 testdata/tfvars/values.tfvars create mode 100644 tfvars/load.go diff --git a/preview.go b/preview.go index 62a94e6..e19ac91 100644 --- a/preview.go +++ b/preview.go @@ -7,6 +7,7 @@ import ( "io/fs" "log/slog" "path/filepath" + "strings" "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" "github.com/hashicorp/hcl/v2" @@ -14,6 +15,7 @@ import ( ctyjson "github.com/zclconf/go-cty/cty/json" "github.com/coder/preview/hclext" + "github.com/coder/preview/tfvars" "github.com/coder/preview/types" ) @@ -26,6 +28,7 @@ type Input struct { ParameterValues map[string]string Owner types.WorkspaceOwner Logger *slog.Logger + TFVars map[string]cty.Value } type Output struct { @@ -96,7 +99,18 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn } } - planHook, err := planJSONHook(dir, input) + variableValues, err := tfvars.LoadTFVars(dir, varFiles) + if err != nil { + return nil, hcl.Diagnostics{ + { + Severity: hcl.DiagError, + Summary: "Failed to load tfvars from files", + Detail: err.Error(), + }, + } + } + + planHook, err := PlanJSONHook(dir, input) if err != nil { return nil, hcl.Diagnostics{ { @@ -121,6 +135,11 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn logger := input.Logger if logger == nil { // Default to discarding logs logger = slog.New(slog.DiscardHandler) + } + + // Override with user supplied variables + for k, v := range input.TFVars { + variableValues[k] = v } // moduleSource is "" for a local module @@ -129,11 +148,12 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn parser.OptionStopOnHCLError(false), parser.OptionWithDownloads(false), parser.OptionWithSkipCachedModules(true), - parser.OptionWithTFVarsPaths(varFiles...), parser.OptionWithEvalHook(planHook), parser.OptionWithEvalHook(ownerHook), - parser.OptionWithWorkingDirectoryPath("/"), - parser.OptionWithEvalHook(parameterContextsEvalHook(input)), + parser.OptionWithEvalHook(ParameterContextsEvalHook(input)), + // 'OptionsWithTfVars' cannot be set with 'OptionWithTFVarsPaths'. So load the + // tfvars from the files ourselves and merge with the user-supplied tf vars. + parser.OptionsWithTfVars(variableValues), ) err = p.ParseFS(ctx, ".") @@ -203,7 +223,7 @@ func tfVarFiles(path string, dir fs.FS) ([]string, error) { files = append(files, newFiles...) } - if filepath.Ext(entry.Name()) == ".tfvars" { + if filepath.Ext(entry.Name()) == ".tfvars" || strings.HasSuffix(entry.Name(), ".tfvars.json") { files = append(files, filepath.Join(path, entry.Name())) } } diff --git a/preview_test.go b/preview_test.go index 59a8761..e92d9df 100644 --- a/preview_test.go +++ b/preview_test.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/zclconf/go-cty/cty" "github.com/coder/preview" "github.com/coder/preview/types" @@ -471,6 +472,37 @@ func Test_Extract(t *testing.T) { optNames("GoLand 2024.3", "IntelliJ IDEA Ultimate 2024.3", "PyCharm Professional 2024.3"), }, }, + { + name: "tfvars_from_file", + dir: "tfvars", + expTags: map[string]string{}, + input: preview.Input{ + ParameterValues: map[string]string{}, + }, + unknownTags: []string{}, + params: map[string]assertParam{ + "variable_values": ap(). + def("alex").optVals("alex", "bob", "claire", "jason"), + }, + }, + { + name: "tfvars_from_input", + dir: "tfvars", + expTags: map[string]string{}, + input: preview.Input{ + ParameterValues: map[string]string{}, + TFVars: map[string]cty.Value{ + "one": cty.StringVal("andrew"), + "two": cty.StringVal("bill"), + "three": cty.StringVal("carter"), + }, + }, + unknownTags: []string{}, + params: map[string]assertParam{ + "variable_values": ap(). + def("andrew").optVals("andrew", "bill", "carter", "jason"), + }, + }, { name: "unknownoption", dir: "unknownoption", diff --git a/testdata/tfvars/.auto.tfvars.json b/testdata/tfvars/.auto.tfvars.json new file mode 100644 index 0000000..41e7b79 --- /dev/null +++ b/testdata/tfvars/.auto.tfvars.json @@ -0,0 +1 @@ +{"four":"jason"} \ No newline at end of file diff --git a/testdata/tfvars/main.tf b/testdata/tfvars/main.tf new file mode 100644 index 0000000..129e4ea --- /dev/null +++ b/testdata/tfvars/main.tf @@ -0,0 +1,61 @@ +// Base case for workspace tags + parameters. +terraform { + required_providers { + coder = { + source = "coder/coder" + } + docker = { + source = "kreuzwerker/docker" + version = "3.0.2" + } + } +} + +variable "one" { + default = "alice" + type = string +} + +variable "two" { + default = "bob" + type = string +} + +variable "three" { + default = "charlie" + type = string +} + +variable "four" { + default = "jack" + type = string +} + + +data "coder_parameter" "variable_values" { + name = "variable_values" + description = "Just to show the variable values" + type = "string" + default = var.one + + + option { + name = "one" + value = var.one + } + + option { + name = "two" + value = var.two + } + + option { + name = "three" + value = var.three + } + + option { + name = "four" + value = var.four + } +} \ No newline at end of file diff --git a/testdata/tfvars/values.tfvars b/testdata/tfvars/values.tfvars new file mode 100644 index 0000000..83cabd4 --- /dev/null +++ b/testdata/tfvars/values.tfvars @@ -0,0 +1,2 @@ +one="alex" +three="claire" diff --git a/tfvars/load.go b/tfvars/load.go new file mode 100644 index 0000000..c693551 --- /dev/null +++ b/tfvars/load.go @@ -0,0 +1,85 @@ +// Code taken from https://github.com/aquasecurity/trivy/blob/main/pkg/iac/scanners/terraform/parser/load_vars.go +package tfvars + +import ( + "fmt" + "io/fs" + "path/filepath" + "strings" + + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + hcljson "github.com/hashicorp/hcl/v2/json" + "github.com/zclconf/go-cty/cty" +) + +func LoadTFVars(srcFS fs.FS, filenames []string) (map[string]cty.Value, error) { + combinedVars := make(map[string]cty.Value) + + // Intentionally commented out to avoid loading from host environment + // + //for _, env := range os.Environ() { + // split := strings.Split(env, "=") + // key := split[0] + // if !strings.HasPrefix(key, "TF_VAR_") { + // continue + // } + // key = strings.TrimPrefix(key, "TF_VAR_") + // var val string + // if len(split) > 1 { + // val = split[1] + // } + // combinedVars[key] = cty.StringVal(val) + //} + + for _, filename := range filenames { + vars, err := LoadTFVarsFile(srcFS, filename) + if err != nil { + return nil, fmt.Errorf("failed to load tfvars from %s: %w", filename, err) + } + for k, v := range vars { + combinedVars[k] = v + } + } + + return combinedVars, nil +} + +func LoadTFVarsFile(srcFS fs.FS, filename string) (map[string]cty.Value, error) { + inputVars := make(map[string]cty.Value) + if filename == "" { + return inputVars, nil + } + + src, err := fs.ReadFile(srcFS, filepath.ToSlash(filename)) + if err != nil { + return nil, err + } + + var attrs hcl.Attributes + if strings.HasSuffix(filename, ".json") { + variableFile, err := hcljson.Parse(src, filename) + if err != nil { + return nil, err + } + attrs, err = variableFile.Body.JustAttributes() + if err != nil { + return nil, err + } + } else { + variableFile, err := hclsyntax.ParseConfig(src, filename, hcl.Pos{Line: 1, Column: 1}) + if err != nil { + return nil, err + } + attrs, err = variableFile.Body.JustAttributes() + if err != nil { + return nil, err + } + } + + for _, attr := range attrs { + inputVars[attr.Name], _ = attr.Expr.Value(&hcl.EvalContext{}) + } + + return inputVars, nil +} From 80d4bddb5f22a6d2ceab6e056a4488fe1d5f802e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 9 Jul 2025 09:37:42 -0600 Subject: [PATCH 2/5] fixup --- preview.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/preview.go b/preview.go index e19ac91..28f8d9b 100644 --- a/preview.go +++ b/preview.go @@ -28,7 +28,9 @@ type Input struct { ParameterValues map[string]string Owner types.WorkspaceOwner Logger *slog.Logger - TFVars map[string]cty.Value + // TFVars will override any variables set in '.tfvars' files. + // The value set must be a cty.Value, as the type can be anything. + TFVars map[string]cty.Value } type Output struct { @@ -83,11 +85,6 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn } }() - // TODO: Fix logging. There is no way to pass in an instanced logger to - // the parser. - // slog.SetLogLoggerLevel(slog.LevelDebug) - // slog.SetDefault(slog.New(log.NewHandler(os.Stderr, nil))) - varFiles, err := tfVarFiles("", dir) if err != nil { return nil, hcl.Diagnostics{ @@ -110,7 +107,7 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn } } - planHook, err := PlanJSONHook(dir, input) + planHook, err := planJSONHook(dir, input) if err != nil { return nil, hcl.Diagnostics{ { @@ -135,7 +132,7 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn logger := input.Logger if logger == nil { // Default to discarding logs logger = slog.New(slog.DiscardHandler) - } + } // Override with user supplied variables for k, v := range input.TFVars { @@ -148,9 +145,11 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn parser.OptionStopOnHCLError(false), parser.OptionWithDownloads(false), parser.OptionWithSkipCachedModules(true), + parser.OptionWithTFVarsPaths(varFiles...), parser.OptionWithEvalHook(planHook), parser.OptionWithEvalHook(ownerHook), - parser.OptionWithEvalHook(ParameterContextsEvalHook(input)), + parser.OptionWithWorkingDirectoryPath("/"), + parser.OptionWithEvalHook(parameterContextsEvalHook(input)), // 'OptionsWithTfVars' cannot be set with 'OptionWithTFVarsPaths'. So load the // tfvars from the files ourselves and merge with the user-supplied tf vars. parser.OptionsWithTfVars(variableValues), From 71c12ca579eaf26bb06cbfbfbfe96701366fafd2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 9 Jul 2025 09:40:49 -0600 Subject: [PATCH 3/5] fixup --- preview.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/preview.go b/preview.go index 28f8d9b..083c6a1 100644 --- a/preview.go +++ b/preview.go @@ -134,7 +134,7 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn logger = slog.New(slog.DiscardHandler) } - // Override with user supplied variables + // Override with user-supplied variables for k, v := range input.TFVars { variableValues[k] = v } @@ -145,7 +145,6 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn parser.OptionStopOnHCLError(false), parser.OptionWithDownloads(false), parser.OptionWithSkipCachedModules(true), - parser.OptionWithTFVarsPaths(varFiles...), parser.OptionWithEvalHook(planHook), parser.OptionWithEvalHook(ownerHook), parser.OptionWithWorkingDirectoryPath("/"), From 8ddde200cd6913884421f26ebe62742607abdfbf Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 9 Jul 2025 10:02:36 -0600 Subject: [PATCH 4/5] load tf vars for e2e --- internal/verify/exec.go | 9 +++++---- preview.go | 34 +--------------------------------- previewe2e_test.go | 26 +++++++++++++++++++------- tfvars/load.go | 30 ++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 44 deletions(-) diff --git a/internal/verify/exec.go b/internal/verify/exec.go index 1294025..2c64bfd 100644 --- a/internal/verify/exec.go +++ b/internal/verify/exec.go @@ -56,14 +56,15 @@ func (e WorkingExecutable) Init(ctx context.Context) error { return e.TF.Init(ctx, tfexec.Upgrade(true)) } -func (e WorkingExecutable) Plan(ctx context.Context, outPath string) (bool, error) { - changes, err := e.TF.Plan(ctx, tfexec.Out(outPath)) +func (e WorkingExecutable) Plan(ctx context.Context, outPath string, opts ...tfexec.PlanOption) (bool, error) { + opts = append(opts, tfexec.Out(outPath)) + changes, err := e.TF.Plan(ctx, opts...) return changes, err } -func (e WorkingExecutable) Apply(ctx context.Context) ([]byte, error) { +func (e WorkingExecutable) Apply(ctx context.Context, opts ...tfexec.ApplyOption) ([]byte, error) { var out bytes.Buffer - err := e.TF.ApplyJSON(ctx, &out) + err := e.TF.ApplyJSON(ctx, &out, opts...) return out.Bytes(), err } diff --git a/preview.go b/preview.go index 083c6a1..7cf360d 100644 --- a/preview.go +++ b/preview.go @@ -6,8 +6,6 @@ import ( "fmt" "io/fs" "log/slog" - "path/filepath" - "strings" "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" "github.com/hashicorp/hcl/v2" @@ -85,7 +83,7 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn } }() - varFiles, err := tfVarFiles("", dir) + varFiles, err := tfvars.TFVarFiles("", dir) if err != nil { return nil, hcl.Diagnostics{ { @@ -197,33 +195,3 @@ func (i Input) RichParameterValue(key string) (string, bool) { p, ok := i.ParameterValues[key] return p, ok } - -// tfVarFiles extracts any .tfvars files from the given directory. -// TODO: Test nested directories and how that should behave. -func tfVarFiles(path string, dir fs.FS) ([]string, error) { - dp := "." - entries, err := fs.ReadDir(dir, dp) - if err != nil { - return nil, fmt.Errorf("read dir %q: %w", dp, err) - } - - files := make([]string, 0) - for _, entry := range entries { - if entry.IsDir() { - subD, err := fs.Sub(dir, entry.Name()) - if err != nil { - return nil, fmt.Errorf("sub dir %q: %w", entry.Name(), err) - } - newFiles, err := tfVarFiles(filepath.Join(path, entry.Name()), subD) - if err != nil { - return nil, err - } - files = append(files, newFiles...) - } - - if filepath.Ext(entry.Name()) == ".tfvars" || strings.HasSuffix(entry.Name(), ".tfvars.json") { - files = append(files, filepath.Join(path, entry.Name())) - } - } - return files, nil -} diff --git a/previewe2e_test.go b/previewe2e_test.go index 3a9adb2..bcfeb0b 100644 --- a/previewe2e_test.go +++ b/previewe2e_test.go @@ -10,10 +10,12 @@ import ( "testing" "time" + "github.com/hashicorp/terraform-exec/tfexec" "github.com/stretchr/testify/require" "github.com/coder/preview" "github.com/coder/preview/internal/verify" + "github.com/coder/preview/tfvars" "github.com/coder/preview/types" ) @@ -102,11 +104,11 @@ func Test_VerifyE2E(t *testing.T) { entryWrkPath := t.TempDir() - for _, tfexec := range tfexecs { - tfexec := tfexec + for _, tfexecutable := range tfexecs { + tfexecutable := tfexecutable - t.Run(tfexec.Version, func(t *testing.T) { - wp := filepath.Join(entryWrkPath, tfexec.Version) + t.Run(tfexecutable.Version, func(t *testing.T) { + wp := filepath.Join(entryWrkPath, tfexecutable.Version) err := os.MkdirAll(wp, 0755) require.NoError(t, err, "creating working dir") @@ -118,7 +120,7 @@ func Test_VerifyE2E(t *testing.T) { err = verify.CopyTFFS(wp, subFS) require.NoError(t, err, "copying test data to working dir") - exe, err := tfexec.WorkingDir(wp) + exe, err := tfexecutable.WorkingDir(wp) require.NoError(t, err, "creating working executable") ctx, cancel := context.WithTimeout(context.Background(), time.Minute*2) @@ -126,9 +128,19 @@ func Test_VerifyE2E(t *testing.T) { err = exe.Init(ctx) require.NoError(t, err, "terraform init") + tfVarFiles, err := tfvars.TFVarFiles("", subFS) + require.NoError(t, err, "loading tfvars files") + + planOpts := make([]tfexec.PlanOption, 0) + applyOpts := make([]tfexec.ApplyOption, 0) + for _, varFile := range tfVarFiles { + planOpts = append(planOpts, tfexec.VarFile(varFile)) + applyOpts = append(applyOpts, tfexec.VarFile(varFile)) + } + planOutFile := "tfplan" planOutPath := filepath.Join(wp, planOutFile) - _, err = exe.Plan(ctx, planOutPath) + _, err = exe.Plan(ctx, planOutPath, planOpts...) require.NoError(t, err, "terraform plan") plan, err := exe.ShowPlan(ctx, planOutPath) @@ -141,7 +153,7 @@ func Test_VerifyE2E(t *testing.T) { err = os.WriteFile(filepath.Join(wp, "plan.json"), pd, 0644) require.NoError(t, err, "writing plan.json") - _, err = exe.Apply(ctx) + _, err = exe.Apply(ctx, applyOpts...) require.NoError(t, err, "terraform apply") state, err := exe.Show(ctx) diff --git a/tfvars/load.go b/tfvars/load.go index c693551..bc0831c 100644 --- a/tfvars/load.go +++ b/tfvars/load.go @@ -13,6 +13,36 @@ import ( "github.com/zclconf/go-cty/cty" ) +// TFVarFiles extracts any .tfvars files from the given directory. +// TODO: Test nested directories and how that should behave. +func TFVarFiles(path string, dir fs.FS) ([]string, error) { + dp := "." + entries, err := fs.ReadDir(dir, dp) + if err != nil { + return nil, fmt.Errorf("read dir %q: %w", dp, err) + } + + files := make([]string, 0) + for _, entry := range entries { + if entry.IsDir() { + subD, err := fs.Sub(dir, entry.Name()) + if err != nil { + return nil, fmt.Errorf("sub dir %q: %w", entry.Name(), err) + } + newFiles, err := TFVarFiles(filepath.Join(path, entry.Name()), subD) + if err != nil { + return nil, err + } + files = append(files, newFiles...) + } + + if filepath.Ext(entry.Name()) == ".tfvars" || strings.HasSuffix(entry.Name(), ".tfvars.json") { + files = append(files, filepath.Join(path, entry.Name())) + } + } + return files, nil +} + func LoadTFVars(srcFS fs.FS, filenames []string) (map[string]cty.Value, error) { combinedVars := make(map[string]cty.Value) From 40da3a668ae6f978ab748f8b8a48f6e28240451d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 14 Jul 2025 08:31:25 -0600 Subject: [PATCH 5/5] PR comments --- testdata/tfvars/.auto.tfvars.json | 2 +- testdata/tfvars/main.tf | 2 +- tfvars/load.go | 20 +++++--------------- 3 files changed, 7 insertions(+), 17 deletions(-) diff --git a/testdata/tfvars/.auto.tfvars.json b/testdata/tfvars/.auto.tfvars.json index 41e7b79..879a3b1 100644 --- a/testdata/tfvars/.auto.tfvars.json +++ b/testdata/tfvars/.auto.tfvars.json @@ -1 +1 @@ -{"four":"jason"} \ No newline at end of file +{"four":"jason"} diff --git a/testdata/tfvars/main.tf b/testdata/tfvars/main.tf index 129e4ea..1c950f4 100644 --- a/testdata/tfvars/main.tf +++ b/testdata/tfvars/main.tf @@ -58,4 +58,4 @@ data "coder_parameter" "variable_values" { name = "four" value = var.four } -} \ No newline at end of file +} diff --git a/tfvars/load.go b/tfvars/load.go index bc0831c..ad98456 100644 --- a/tfvars/load.go +++ b/tfvars/load.go @@ -1,4 +1,4 @@ -// Code taken from https://github.com/aquasecurity/trivy/blob/main/pkg/iac/scanners/terraform/parser/load_vars.go +// Code taken from https://github.com/aquasecurity/trivy/blob/0449787eb52854cbdd7f4c5794adbf58965e60f8/pkg/iac/scanners/terraform/parser/load_vars.go package tfvars import ( @@ -46,21 +46,11 @@ func TFVarFiles(path string, dir fs.FS) ([]string, error) { func LoadTFVars(srcFS fs.FS, filenames []string) (map[string]cty.Value, error) { combinedVars := make(map[string]cty.Value) - // Intentionally commented out to avoid loading from host environment + // Intentionally avoid loading terraform variables from the host environment. + // Trivy (and terraform) use os.Environ() to search for "TF_VAR_" prefixed + // environment variables. // - //for _, env := range os.Environ() { - // split := strings.Split(env, "=") - // key := split[0] - // if !strings.HasPrefix(key, "TF_VAR_") { - // continue - // } - // key = strings.TrimPrefix(key, "TF_VAR_") - // var val string - // if len(split) > 1 { - // val = split[1] - // } - // combinedVars[key] = cty.StringVal(val) - //} + // Preview should be sandboxed, so this code should not be included. for _, filename := range filenames { vars, err := LoadTFVarsFile(srcFS, filename)