From 4cd7ad4721cd3aba078482cabe9b01de8202dfa2 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet <105649352+lbajolet-hashicorp@users.noreply.github.com> Date: Mon, 16 Jun 2025 23:52:09 -0400 Subject: [PATCH] hcl2template: err on malformed local/data dep (#13340) * hcl2template: err on malformed local/data dep When introducing the DAG for locals and datasources, we forgot to handle one limit case: if a dependency for a local or data is malformed, we didn't check that a vertex was associated to it, leading to the final DAG being malformed, and the DAG library would crash in this case. This commit fixes this problem by checking that the dependency does exist before attempting to add it to the graph as an edge for a vertex, so that it is reported accurately, and do that Packer doesn't crash. * error message change --------- Co-authored-by: anshul sharma --- hcl2template/parser.go | 42 ++++++++++++---- packer_test/dag_tests/malformed_dep_test.go | 49 +++++++++++++++++++ .../templates/malformed_data_dep.pkr.hcl | 8 +++ .../templates/malformed_local_dep.pkr.hcl | 4 ++ 4 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 packer_test/dag_tests/malformed_dep_test.go create mode 100644 packer_test/dag_tests/templates/malformed_data_dep.pkr.hcl create mode 100644 packer_test/dag_tests/templates/malformed_local_dep.pkr.hcl diff --git a/hcl2template/parser.go b/hcl2template/parser.go index 5e2670bb5..1bc5d6467 100644 --- a/hcl2template/parser.go +++ b/hcl2template/parser.go @@ -9,6 +9,7 @@ import ( "path/filepath" "reflect" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-version" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/ext/dynblock" @@ -403,6 +404,8 @@ func (cfg *PackerConfig) buildPrereqsDAG() (*dag.AcyclicGraph, error) { verticesMap := map[string]dag.Vertex{} + var err error + // Do a first pass to create all the vertices for ref := range cfg.Datasources { // We keep a reference to the datasource separately from where it @@ -434,27 +437,48 @@ func (cfg *PackerConfig) buildPrereqsDAG() (*dag.AcyclicGraph, error) { for _, ds := range cfg.Datasources { dsName := fmt.Sprintf("data.%s", ds.Name()) + source := verticesMap[dsName] + if source == nil { + err = multierror.Append(err, fmt.Errorf("unable to find source vertex %q for dependency analysis, this is likely a Packer bug", dsName)) + continue + } + for _, dep := range ds.Dependencies { - retGraph.Connect( - dag.BasicEdge(verticesMap[dsName], - verticesMap[dep.String()])) + target := verticesMap[dep.String()] + if target == nil { + err = multierror.Append(err, fmt.Errorf("could not get dependency %q for %q, %q missing in template", dep.String(), dsName, dep.String())) + continue + } + + retGraph.Connect(dag.BasicEdge(source, target)) } } for _, loc := range cfg.LocalBlocks { locName := fmt.Sprintf("local.%s", loc.LocalName) + source := verticesMap[locName] + if source == nil { + err = multierror.Append(err, fmt.Errorf("unable to find source vertex %q for dependency analysis, this is likely a Packer bug", locName)) + continue + } + for _, dep := range loc.dependencies { - retGraph.Connect( - dag.BasicEdge(verticesMap[locName], - verticesMap[dep.String()])) + target := verticesMap[dep.String()] + + if target == nil { + err = multierror.Append(err, fmt.Errorf("could not get dependency %q for %q, %q missing in template", dep.String(), locName, dep.String())) + continue + } + + retGraph.Connect(dag.BasicEdge(source, target)) } } - if err := retGraph.Validate(); err != nil { - return nil, err + if validateErr := retGraph.Validate(); validateErr != nil { + err = multierror.Append(err, validateErr) } - return &retGraph, nil + return &retGraph, err } func (cfg *PackerConfig) evaluateBuildPrereqs(skipDatasources bool) hcl.Diagnostics { diff --git a/packer_test/dag_tests/malformed_dep_test.go b/packer_test/dag_tests/malformed_dep_test.go new file mode 100644 index 000000000..ba3a1a63b --- /dev/null +++ b/packer_test/dag_tests/malformed_dep_test.go @@ -0,0 +1,49 @@ +package main + +import ( + "fmt" + + "github.com/hashicorp/packer/packer_test/common/check" +) + +type malformedDepTestCase struct { + name string + command string + templatePath string + useSequential bool +} + +func genMalformedDepTestCases() []malformedDepTestCase { + retVals := []malformedDepTestCase{} + + for _, cmd := range []string{"build", "validate"} { + for _, template := range []string{"./templates/malformed_data_dep.pkr.hcl", "./templates/malformed_local_dep.pkr.hcl"} { + for _, seq := range []bool{true, false} { + retVals = append(retVals, malformedDepTestCase{ + name: fmt.Sprintf("Malformed dep packer %s --use-sequential-evaluation=%t %s", + cmd, seq, template), + command: cmd, + templatePath: template, + useSequential: seq, + }) + } + } + } + + return retVals +} + +func (ts *PackerDAGTestSuite) TestMalformedDependency() { + pluginDir := ts.MakePluginDir() + defer pluginDir.Cleanup() + + for _, tc := range genMalformedDepTestCases() { + ts.Run(tc.name, func() { + ts.PackerCommand().UsePluginDir(pluginDir). + SetArgs(tc.command, + fmt.Sprintf("--use-sequential-evaluation=%t", tc.useSequential), + tc.templatePath). + Assert(check.MustFail()) + }) + } +} diff --git a/packer_test/dag_tests/templates/malformed_data_dep.pkr.hcl b/packer_test/dag_tests/templates/malformed_data_dep.pkr.hcl new file mode 100644 index 000000000..d0fc29245 --- /dev/null +++ b/packer_test/dag_tests/templates/malformed_data_dep.pkr.hcl @@ -0,0 +1,8 @@ +data "http" "trusted_ca_certificates" { + method = "GET" + url = "http://example.com/ca-bundle.crt" +} + +locals { + test = data.trusted_ca_certificates.url +} diff --git a/packer_test/dag_tests/templates/malformed_local_dep.pkr.hcl b/packer_test/dag_tests/templates/malformed_local_dep.pkr.hcl new file mode 100644 index 000000000..4b9b91ab2 --- /dev/null +++ b/packer_test/dag_tests/templates/malformed_local_dep.pkr.hcl @@ -0,0 +1,4 @@ +data "http" "trusted_ca_certificates" { + method = "GET" + url = local.no_dep +}