From decccf9e3f27c0c10e797b678b03ad0e594df1f3 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Thu, 5 Mar 2026 16:23:06 +0100 Subject: [PATCH 1/4] improve error message when using not const variables in module sources --- internal/terraform/context_init_test.go | 4 ++-- internal/terraform/node_module_install.go | 14 ++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/internal/terraform/context_init_test.go b/internal/terraform/context_init_test.go index 9bf78943be..8cb4ff8ffe 100644 --- a/internal/terraform/context_init_test.go +++ b/internal/terraform/context_init_test.go @@ -138,7 +138,7 @@ module "example" { return tfdiags.Diagnostics{}.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Invalid module source`, - Detail: `The value of a reference in the module source is unknown.`, + Detail: "The value of a reference in the module source is unknown." + constVariableDetail, Subject: &hcl.Range{ Filename: filepath.Join(m.SourceDir, "main.tf"), Start: hcl.Pos{Line: 6, Column: 27, Byte: 82}, @@ -625,7 +625,7 @@ module "nested" { return tfdiags.Diagnostics{}.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Invalid module source`, - Detail: `The value of a reference in the module source is unknown.`, + Detail: "The value of a reference in the module source is unknown." + constVariableDetail, Subject: &hcl.Range{ Filename: filepath.Join(mc["./modules/example"].SourceDir, "main.tf"), Start: hcl.Pos{Line: 7, Column: 27, Byte: 82}, diff --git a/internal/terraform/node_module_install.go b/internal/terraform/node_module_install.go index be5c5cc9a1..4a4e62ce4f 100644 --- a/internal/terraform/node_module_install.go +++ b/internal/terraform/node_module_install.go @@ -154,6 +154,8 @@ func (n *nodeInstallModule) DynamicExpand(ctx EvalContext) (*Graph, tfdiags.Diag return &g, nil } +const constVariableDetail = "\nOnly literal values and constant variables (with const = true) are allowed for this attribute, as well as values derived from these." + func evalSource(sourceExpr hcl.Expression, hasVersion bool, ctx EvalContext) (addrs.ModuleSource, string, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics var addr addrs.ModuleSource @@ -192,7 +194,7 @@ func evalSource(sourceExpr hcl.Expression, hasVersion bool, ctx EvalContext) (ad diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid module source", - Detail: "The module source contains a reference that is unknown during init.", + Detail: "The module source contains a reference that is unknown during init." + constVariableDetail, Subject: sourceExpr.Range().Ptr(), }) return nil, "", diags @@ -214,7 +216,7 @@ func evalSource(sourceExpr hcl.Expression, hasVersion bool, ctx EvalContext) (ad diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid module source", - Detail: "The value of a reference in the module source is unknown.", + Detail: "The value of a reference in the module source is unknown." + constVariableDetail, Subject: part.Range().Ptr(), Expression: part, EvalContext: hclCtx, @@ -226,7 +228,7 @@ func evalSource(sourceExpr hcl.Expression, hasVersion bool, ctx EvalContext) (ad diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid module source", - Detail: "The module source contains a reference that is unknown.", + Detail: "The module source contains a reference that is unknown." + constVariableDetail, Subject: sourceExpr.Range().Ptr(), }) return nil, "", diags @@ -335,7 +337,7 @@ func evalVersionConstraint(versionExpr hcl.Expression, ctx EvalContext) (configs diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid module version", - Detail: "The module version contains a reference that is unknown during init.", + Detail: "The module version contains a reference that is unknown during init." + constVariableDetail, Subject: versionExpr.Range().Ptr(), }) return ret, diags @@ -357,7 +359,7 @@ func evalVersionConstraint(versionExpr hcl.Expression, ctx EvalContext) (configs diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid module version", - Detail: "The value of a reference in the module version is unknown.", + Detail: "The value of a reference in the module version is unknown." + constVariableDetail, Subject: part.Range().Ptr(), Expression: part, EvalContext: hclCtx, @@ -369,7 +371,7 @@ func evalVersionConstraint(versionExpr hcl.Expression, ctx EvalContext) (configs diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid module version", - Detail: "The module version contains a reference that is unknown.", + Detail: "The module version contains a reference that is unknown." + constVariableDetail, Subject: versionExpr.Range().Ptr(), }) return ret, diags From 95c9d6f42c6e16b5f756373ff38a8125b538b22a Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Fri, 6 Mar 2026 17:43:21 +0100 Subject: [PATCH 2/4] improve formatting Co-authored-by: Daniel Banck --- internal/terraform/node_module_install.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/terraform/node_module_install.go b/internal/terraform/node_module_install.go index 4a4e62ce4f..daf3368bfa 100644 --- a/internal/terraform/node_module_install.go +++ b/internal/terraform/node_module_install.go @@ -154,7 +154,7 @@ func (n *nodeInstallModule) DynamicExpand(ctx EvalContext) (*Graph, tfdiags.Diag return &g, nil } -const constVariableDetail = "\nOnly literal values and constant variables (with const = true) are allowed for this attribute, as well as values derived from these." +const constVariableDetail = "\n\nOnly literal values and constant variables (with const = true) are allowed for this attribute, as well as values derived from these." func evalSource(sourceExpr hcl.Expression, hasVersion bool, ctx EvalContext) (addrs.ModuleSource, string, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics From ac7206c9194ef8be09878541ad09f7b67d3d4625 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Tue, 10 Mar 2026 13:34:18 +0000 Subject: [PATCH 3/4] test: Fix E2E tests to use correct errors for assertions, remove use of ioutil in package (#38254) --- internal/command/e2etest/provider_dev_test.go | 9 ++++----- internal/e2e/e2e.go | 5 ++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/internal/command/e2etest/provider_dev_test.go b/internal/command/e2etest/provider_dev_test.go index 739561b8d2..fe3b89ccf1 100644 --- a/internal/command/e2etest/provider_dev_test.go +++ b/internal/command/e2etest/provider_dev_test.go @@ -5,7 +5,6 @@ package e2etest import ( "fmt" - "io/ioutil" "os" "path/filepath" "strings" @@ -47,7 +46,7 @@ func TestProviderDevOverrides(t *testing.T) { providerExe := e2e.GoBuild("github.com/hashicorp/terraform/internal/provider-simple/main", providerExePrefix) t.Logf("temporary provider executable is %s", providerExe) - err := ioutil.WriteFile(filepath.Join(tf.WorkDir(), "dev.tfrc"), []byte(fmt.Sprintf(` + err := os.WriteFile(filepath.Join(tf.WorkDir(), "dev.tfrc"), []byte(fmt.Sprintf(` provider_installation { dev_overrides { "example.com/test/test" = %q @@ -86,7 +85,7 @@ func TestProviderDevOverrides(t *testing.T) { t.Errorf("stdout doesn't include the warning about development overrides\nwant: %s\n%s", want, got) } - stdout, _, _ = tf.Run("init") + stdout, _, err = tf.Run("init") if err != nil { t.Fatalf("unexpected error: %e", err) } @@ -129,7 +128,7 @@ func TestProviderDevOverridesWithProviderToDownload(t *testing.T) { providerExe := e2e.GoBuild("github.com/hashicorp/terraform/internal/provider-simple/main", providerExePrefix) t.Logf("temporary provider executable is %s", providerExe) - err := ioutil.WriteFile(filepath.Join(tf.WorkDir(), "dev.tfrc"), []byte(fmt.Sprintf(` + err := os.WriteFile(filepath.Join(tf.WorkDir(), "dev.tfrc"), []byte(fmt.Sprintf(` provider_installation { dev_overrides { "example.com/test/test" = %q @@ -143,7 +142,7 @@ func TestProviderDevOverridesWithProviderToDownload(t *testing.T) { tf.AddEnv("TF_CLI_CONFIG_FILE=dev.tfrc") - stdout, stderr, _ := tf.Run("providers") + stdout, stderr, err := tf.Run("providers") if err != nil { t.Fatalf("unexpected error: %s\n%s", err, stderr) } diff --git a/internal/e2e/e2e.go b/internal/e2e/e2e.go index 5cdc7b1b2a..2695d77d77 100644 --- a/internal/e2e/e2e.go +++ b/internal/e2e/e2e.go @@ -7,7 +7,6 @@ import ( "bytes" "fmt" "io" - "io/ioutil" "os" "os/exec" "path/filepath" @@ -177,7 +176,7 @@ func (b *binary) OpenFile(path ...string) (*os.File, error) { // directory. func (b *binary) ReadFile(path ...string) ([]byte, error) { flatPath := b.Path(path...) - return ioutil.ReadFile(flatPath) + return os.ReadFile(flatPath) } // FileExists is a helper for easily testing whether a particular file @@ -247,7 +246,7 @@ func (b *binary) SetLocalState(state *states.State) error { func GoBuild(pkgPath, tmpPrefix string) string { dir, prefix := filepath.Split(tmpPrefix) - tmpFile, err := ioutil.TempFile(dir, prefix) + tmpFile, err := os.CreateTemp(dir, prefix) if err != nil { panic(err) } From f9cfdf1ebe8131dac81cd92d6c4f4185c8ea031c Mon Sep 17 00:00:00 2001 From: Daniel Banck Date: Wed, 11 Mar 2026 11:53:21 +0100 Subject: [PATCH 4/4] Refactoring: Modernize graph command to use arguments --- internal/command/arguments/graph.go | 63 ++++++++++ internal/command/arguments/graph_test.go | 147 +++++++++++++++++++++++ internal/command/graph.go | 45 +++---- 3 files changed, 225 insertions(+), 30 deletions(-) create mode 100644 internal/command/arguments/graph.go create mode 100644 internal/command/arguments/graph_test.go diff --git a/internal/command/arguments/graph.go b/internal/command/arguments/graph.go new file mode 100644 index 0000000000..291ad2dfe0 --- /dev/null +++ b/internal/command/arguments/graph.go @@ -0,0 +1,63 @@ +// Copyright IBM Corp. 2014, 2026 +// SPDX-License-Identifier: BUSL-1.1 + +package arguments + +import "github.com/hashicorp/terraform/internal/tfdiags" + +// Graph represents the command-line arguments for the graph command. +type Graph struct { + // DrawCycles highlights any cycles in the graph with colored edges. + DrawCycles bool + + // GraphType is the type of operation graph to output (plan, + // plan-refresh-only, plan-destroy, or apply). Empty string means the + // default resource-dependency summary. + GraphType string + + // ModuleDepth is a deprecated option that was used in prior versions to + // control the depth of modules shown. + ModuleDepth int + + // Verbose enables verbose graph output. + Verbose bool + + // Plan is the path to a saved plan file to render as a graph. + Plan string +} + +// ParseGraph processes CLI arguments, returning a Graph value and errors. +// If errors are encountered, a Graph value is still returned representing +// the best effort interpretation of the arguments. +func ParseGraph(args []string) (*Graph, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + graph := &Graph{ + ModuleDepth: -1, + } + + cmdFlags := defaultFlagSet("graph") + cmdFlags.BoolVar(&graph.DrawCycles, "draw-cycles", false, "draw-cycles") + cmdFlags.StringVar(&graph.GraphType, "type", "", "type") + cmdFlags.IntVar(&graph.ModuleDepth, "module-depth", -1, "module-depth") + cmdFlags.BoolVar(&graph.Verbose, "verbose", false, "verbose") + cmdFlags.StringVar(&graph.Plan, "plan", "", "plan") + + if err := cmdFlags.Parse(args); err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + err.Error(), + )) + } + + args = cmdFlags.Args() + if len(args) > 0 { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Too many command line arguments", + "Expected no positional arguments. Did you mean to use -chdir?", + )) + } + + return graph, diags +} diff --git a/internal/command/arguments/graph_test.go b/internal/command/arguments/graph_test.go new file mode 100644 index 0000000000..227707e039 --- /dev/null +++ b/internal/command/arguments/graph_test.go @@ -0,0 +1,147 @@ +// Copyright IBM Corp. 2014, 2026 +// SPDX-License-Identifier: BUSL-1.1 + +package arguments + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform/internal/tfdiags" +) + +func TestParseGraph_valid(t *testing.T) { + testCases := map[string]struct { + args []string + want *Graph + }{ + "defaults": { + nil, + &Graph{ + ModuleDepth: -1, + }, + }, + "plan type": { + []string{"-type=plan"}, + &Graph{ + GraphType: "plan", + ModuleDepth: -1, + }, + }, + "apply type": { + []string{"-type=apply"}, + &Graph{ + GraphType: "apply", + ModuleDepth: -1, + }, + }, + "draw-cycles": { + []string{"-draw-cycles", "-type=plan"}, + &Graph{ + DrawCycles: true, + GraphType: "plan", + ModuleDepth: -1, + }, + }, + "plan file": { + []string{"-plan=tfplan"}, + &Graph{ + Plan: "tfplan", + ModuleDepth: -1, + }, + }, + "verbose": { + []string{"-verbose"}, + &Graph{ + Verbose: true, + ModuleDepth: -1, + }, + }, + "module-depth": { + []string{"-module-depth=2"}, + &Graph{ + ModuleDepth: 2, + }, + }, + "all flags": { + []string{"-draw-cycles", "-type=plan-destroy", "-plan=tfplan", "-verbose", "-module-depth=3"}, + &Graph{ + DrawCycles: true, + GraphType: "plan-destroy", + Plan: "tfplan", + Verbose: true, + ModuleDepth: 3, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, diags := ParseGraph(tc.args) + if len(diags) > 0 { + t.Fatalf("unexpected diags: %v", diags) + } + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Fatalf("unexpected result\n%s", diff) + } + }) + } +} + +func TestParseGraph_invalid(t *testing.T) { + testCases := map[string]struct { + args []string + want *Graph + wantDiags tfdiags.Diagnostics + }{ + "unknown flag": { + []string{"-wat"}, + &Graph{ + ModuleDepth: -1, + }, + tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + "flag provided but not defined: -wat", + ), + }, + }, + "positional argument": { + []string{"extra"}, + &Graph{ + ModuleDepth: -1, + }, + tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Too many command line arguments", + "Expected no positional arguments. Did you mean to use -chdir?", + ), + }, + }, + "too many positional arguments": { + []string{"bad", "bad"}, + &Graph{ + ModuleDepth: -1, + }, + tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Too many command line arguments", + "Expected no positional arguments. Did you mean to use -chdir?", + ), + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + got, gotDiags := ParseGraph(tc.args) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Fatalf("unexpected result\n%s", diff) + } + tfdiags.AssertDiagnosticsMatch(t, gotDiags, tc.wantDiags) + }) + } +} diff --git a/internal/command/graph.go b/internal/command/graph.go index ded3c1f70b..8e4b8efa57 100644 --- a/internal/command/graph.go +++ b/internal/command/graph.go @@ -24,27 +24,14 @@ type GraphCommand struct { Meta } -func (c *GraphCommand) Run(args []string) int { - var drawCycles bool - var graphTypeStr string - var moduleDepth int - var verbose bool - var planPath string - - args = c.Meta.process(args) - cmdFlags := c.Meta.defaultFlagSet("graph") - cmdFlags.BoolVar(&drawCycles, "draw-cycles", false, "draw-cycles") - cmdFlags.StringVar(&graphTypeStr, "type", "", "type") - cmdFlags.IntVar(&moduleDepth, "module-depth", -1, "module-depth") - cmdFlags.BoolVar(&verbose, "verbose", false, "verbose") - cmdFlags.StringVar(&planPath, "plan", "", "plan") - cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } - if err := cmdFlags.Parse(args); err != nil { - c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error())) +func (c *GraphCommand) Run(rawArgs []string) int { + args, diags := arguments.ParseGraph(c.Meta.process(rawArgs)) + if diags.HasErrors() { + c.showDiagnostics(diags) return 1 } - configPath, err := ModulePath(cmdFlags.Args()) + configPath, err := ModulePath(nil) if err != nil { c.Ui.Error(err.Error()) return 1 @@ -58,16 +45,14 @@ func (c *GraphCommand) Run(args []string) int { // Try to load plan if path is specified var planFile *planfile.WrappedPlanFile - if planPath != "" { - planFile, err = c.PlanFile(planPath) + if args.Plan != "" { + planFile, err = c.PlanFile(args.Plan) if err != nil { c.Ui.Error(err.Error()) return 1 } } - var diags tfdiags.Diagnostics - // Load the backend b, backendDiags := c.backend(".", arguments.ViewHuman) diags = diags.Append(backendDiags) @@ -106,9 +91,9 @@ func (c *GraphCommand) Run(args []string) int { c.showDiagnostics(diags) return 1 } - lr.Core.SetGraphOpts(&terraform.ContextGraphOpts{SkipGraphValidation: drawCycles}) + lr.Core.SetGraphOpts(&terraform.ContextGraphOpts{SkipGraphValidation: args.DrawCycles}) - if graphTypeStr == "" { + if args.GraphType == "" { if planFile == nil { // Simple resource dependency mode: // This is based on the plan graph but we then further reduce it down @@ -125,13 +110,13 @@ func (c *GraphCommand) Run(args []string) int { g := fullG.ResourceGraph() return c.resourceOnlyGraph(g) } else { - graphTypeStr = "apply" + args.GraphType = "apply" } } var g *terraform.Graph var graphDiags tfdiags.Diagnostics - switch graphTypeStr { + switch args.GraphType { case "plan": g, graphDiags = lr.Core.PlanGraphForUI(lr.Config, lr.InputState, plans.NormalMode) case "plan-refresh-only": @@ -162,7 +147,7 @@ func (c *GraphCommand) Run(args []string) int { graphDiags = graphDiags.Append(tfdiags.Sourceless( tfdiags.Error, "Graph type no longer available", - fmt.Sprintf("The graph type %q is no longer available. Use -type=plan instead to get a similar result.", graphTypeStr), + fmt.Sprintf("The graph type %q is no longer available. Use -type=plan instead to get a similar result.", args.GraphType), )) default: graphDiags = graphDiags.Append(tfdiags.Sourceless( @@ -178,9 +163,9 @@ func (c *GraphCommand) Run(args []string) int { } graphStr, err := terraform.GraphDot(g, &dag.DotOpts{ - DrawCycles: drawCycles, - MaxDepth: moduleDepth, - Verbose: verbose, + DrawCycles: args.DrawCycles, + MaxDepth: args.ModuleDepth, + Verbose: args.Verbose, }) if err != nil { c.Ui.Error(fmt.Sprintf("Error converting graph: %s", err))