From d722d3c6340c91525a70444879fbd4ef4a2e134e Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 26 Mar 2025 16:28:39 -0400 Subject: [PATCH] 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. --- 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 1f44e0ce7..d18eef5d6 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" @@ -398,6 +399,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 @@ -429,27 +432,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("unknown dependency %q for %q", dep.String(), dsName)) + 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("unknown dependency %q for %q", dep.String(), locName)) + 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 +}