Merge pull request #12645 from hashicorp/backport/datasource_logic_cl…

…eanup/visually-master-asp This pull request was automerged via backport-assistant
hashicorp · Oct 5, 2023 · 120cdee · 120cdee
2 parents 87dad88 + 08af008
commit 120cdee
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 82 deletions.
diff --git a/hcl2template/types.datasource.go b/hcl2template/types.datasource.go
@@ -10,7 +10,6 @@ import (
 	"github.com/hashicorp/hcl/v2/hclsyntax"
 	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
 	hcl2shim "github.com/hashicorp/packer/hcl2template/shim"
-	"github.com/hashicorp/packer/packer"
 	"github.com/zclconf/go-cty/cty"
 )
 
@@ -65,31 +64,33 @@ func (ds *Datasources) Values() (map[string]cty.Value, hcl.Diagnostics) {
 	return res, diags
 }
 
-func (cfg *PackerConfig) startDatasource(dataSourceStore packer.DatasourceStore, ref DatasourceRef, secondaryEvaluation bool) (packersdk.Datasource, hcl.Diagnostics) {
+func (cfg *PackerConfig) startDatasource(ds DatasourceBlock) (packersdk.Datasource, hcl.Diagnostics) {
 	var diags hcl.Diagnostics
-	block := cfg.Datasources[ref].block
+	block := ds.block
+
+	dataSourceStore := cfg.parser.PluginConfig.DataSources
 
 	if dataSourceStore == nil {
 		diags = append(diags, &hcl.Diagnostic{
-			Summary:  "Unknown " + dataSourceLabel + " type " + ref.Type,
+			Summary:  "Unknown " + dataSourceLabel + " type " + ds.Type,
 			Subject:  block.LabelRanges[0].Ptr(),
 			Detail:   "packer does not currently know any data source.",
 			Severity: hcl.DiagError,
 		})
 		return nil, diags
 	}
 
-	if !dataSourceStore.Has(ref.Type) {
+	if !dataSourceStore.Has(ds.Type) {
 		diags = append(diags, &hcl.Diagnostic{
-			Summary:  "Unknown " + dataSourceLabel + " type " + ref.Type,
+			Summary:  "Unknown " + dataSourceLabel + " type " + ds.Type,
 			Subject:  block.LabelRanges[0].Ptr(),
 			Detail:   fmt.Sprintf("known data sources: %v", dataSourceStore.List()),
 			Severity: hcl.DiagError,
 		})
 		return nil, diags
 	}
 
-	datasource, err := dataSourceStore.Start(ref.Type)
+	datasource, err := dataSourceStore.Start(ds.Type)
 	if err != nil {
 		diags = append(diags, &hcl.Diagnostic{
 			Summary:  err.Error(),
@@ -99,7 +100,7 @@ func (cfg *PackerConfig) startDatasource(dataSourceStore packer.DatasourceStore,
 	}
 	if datasource == nil {
 		diags = append(diags, &hcl.Diagnostic{
-			Summary:  fmt.Sprintf("failed to start datasource plugin %q.%q", ref.Type, ref.Name),
+			Summary:  fmt.Sprintf("failed to start datasource plugin %q.%q", ds.Type, ds.Name),
 			Subject:  &block.DefRange,
 			Severity: hcl.DiagError,
 		})

diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go
@@ -5,7 +5,6 @@ package hcl2template
 
 import (
 	"fmt"
-	"log"
 	"sort"
 	"strings"
 
@@ -311,76 +310,26 @@ func (cfg *PackerConfig) evaluateDatasources(skipExecution bool) hcl.Diagnostics
 		// source in any of its input expressions. If so, skip evaluating it for
 		// now, and add it to a list of datasources to evaluate again, later,
 		// with the datasources in its context.
-		// This is essentially creating a very primitive DAG just for data
-		// source interdependencies.
-		block := ds.block
-		body := block.Body
-		attrs, _ := body.JustAttributes()
-
-		skipFirstEval := false
-		for _, attr := range attrs {
-			vars := attr.Expr.Variables()
-			for _, v := range vars {
-				// check whether the variable is a data source
-				if v.RootName() == "data" {
-					// construct, backwards, the data source type and name we
-					// need to evaluate before this one can be evaluated.
-					dependsOn := DatasourceRef{
-						Type: v[1].(hcl.TraverseAttr).Name,
-						Name: v[2].(hcl.TraverseAttr).Name,
-					}
-					log.Printf("The data source %#v depends on datasource %#v", ref, dependsOn)
-					if dependencies[ref] != nil {
-						dependencies[ref] = append(dependencies[ref], dependsOn)
-					} else {
-						dependencies[ref] = []DatasourceRef{dependsOn}
-					}
-					skipFirstEval = true
-				}
+		dependencies[ref] = []DatasourceRef{}
+
+		// Note: when looking at the expressions, we only need to care about
+		// attributes, as HCL2 expressions are not allowed in a block's labels.
+		vars := GetVarsByType(ds.block, "data")
+		for _, v := range vars {
+			// construct, backwards, the data source type and name we
+			// need to evaluate before this one can be evaluated.
+			dependsOn := DatasourceRef{
+				Type: v[1].(hcl.TraverseAttr).Name,
+				Name: v[2].(hcl.TraverseAttr).Name,
 			}
+			dependencies[ref] = append(dependencies[ref], dependsOn)
 		}
-
-		// Now we have a list of data sources that depend on other data sources.
-		// Don't evaluate these; only evaluate data sources that we didn't
-		// mark  as having dependencies.
-		if skipFirstEval {
-			continue
-		}
-
-		datasource, startDiags := cfg.startDatasource(cfg.parser.PluginConfig.DataSources, ref, false)
-		diags = append(diags, startDiags...)
-		if diags.HasErrors() {
-			continue
-		}
-
-		if skipExecution {
-			placeholderValue := cty.UnknownVal(hcldec.ImpliedType(datasource.OutputSpec()))
-			ds.value = placeholderValue
-			cfg.Datasources[ref] = ds
-			continue
-		}
-
-		dsOpts, _ := decodeHCL2Spec(body, cfg.EvalContext(DatasourceContext, nil), datasource)
-		sp := packer.CheckpointReporter.AddSpan(ref.Type, "datasource", dsOpts)
-		realValue, err := datasource.Execute()
-		sp.End(err)
-		if err != nil {
-			diags = append(diags, &hcl.Diagnostic{
-				Summary:  err.Error(),
-				Subject:  &cfg.Datasources[ref].block.DefRange,
-				Severity: hcl.DiagError,
-			})
-			continue
-		}
-
-		ds.value = realValue
-		cfg.Datasources[ref] = ds
 	}
 
 	// Now that most of our data sources have been started and executed, we can
 	// try to execute the ones that depend on other data sources.
 	for ref := range dependencies {
-		_, moreDiags, _ := cfg.recursivelyEvaluateDatasources(ref, dependencies, skipExecution, 0)
+		_, moreDiags := cfg.recursivelyEvaluateDatasources(ref, dependencies, skipExecution, 0)
 		// Deduplicate diagnostics to prevent recursion messes.
 		cleanedDiags := map[string]*hcl.Diagnostic{}
 		for _, diag := range moreDiags {
@@ -395,10 +344,9 @@ func (cfg *PackerConfig) evaluateDatasources(skipExecution bool) hcl.Diagnostics
 	return diags
 }
 
-func (cfg *PackerConfig) recursivelyEvaluateDatasources(ref DatasourceRef, dependencies map[DatasourceRef][]DatasourceRef, skipExecution bool, depth int) (map[DatasourceRef][]DatasourceRef, hcl.Diagnostics, bool) {
+func (cfg *PackerConfig) recursivelyEvaluateDatasources(ref DatasourceRef, dependencies map[DatasourceRef][]DatasourceRef, skipExecution bool, depth int) (map[DatasourceRef][]DatasourceRef, hcl.Diagnostics) {
 	var diags hcl.Diagnostics
 	var moreDiags hcl.Diagnostics
-	shouldContinue := true
 
 	if depth > 10 {
 		// Add a comment about recursion.
@@ -409,8 +357,9 @@ func (cfg *PackerConfig) recursivelyEvaluateDatasources(ref DatasourceRef, depen
 				"sources. Either your data source depends on more than ten " +
 				"other data sources, or your data sources have a cyclic " +
 				"dependency. Please simplify your config to continue. ",
+			Subject: &(cfg.Datasources[ref]).block.DefRange,
 		})
-		return dependencies, diags, false
+		return dependencies, diags
 	}
 
 	ds := cfg.Datasources[ref]
@@ -421,28 +370,28 @@ func (cfg *PackerConfig) recursivelyEvaluateDatasources(ref DatasourceRef, depen
 			// If this dependency is not in the map, it means we've already
 			// launched and executed this datasource. Otherwise, it means
 			// we still need to run it. RECURSION TIME!!
-			dependencies, moreDiags, shouldContinue = cfg.recursivelyEvaluateDatasources(dep, dependencies, skipExecution, depth)
+			dependencies, moreDiags = cfg.recursivelyEvaluateDatasources(dep, dependencies, skipExecution, depth)
 			diags = append(diags, moreDiags...)
 			if moreDiags.HasErrors() {
 				diags = append(diags, moreDiags...)
-				return dependencies, diags, shouldContinue
+				return dependencies, diags
 			}
 		}
 	}
 	// If we've gotten here, then it means ref doesn't seem to have any further
 	// dependencies we need to evaluate first. Evaluate it, with the cfg's full
 	// data source context.
-	datasource, startDiags := cfg.startDatasource(cfg.parser.PluginConfig.DataSources, ref, true)
+	datasource, startDiags := cfg.startDatasource(ds)
 	if startDiags.HasErrors() {
 		diags = append(diags, startDiags...)
-		return dependencies, diags, shouldContinue
+		return dependencies, diags
 	}
 
 	if skipExecution {
 		placeholderValue := cty.UnknownVal(hcldec.ImpliedType(datasource.OutputSpec()))
 		ds.value = placeholderValue
 		cfg.Datasources[ref] = ds
-		return dependencies, diags, shouldContinue
+		return dependencies, diags
 	}
 
 	opts, _ := decodeHCL2Spec(ds.block.Body, cfg.EvalContext(DatasourceContext, nil), datasource)
@@ -455,14 +404,14 @@ func (cfg *PackerConfig) recursivelyEvaluateDatasources(ref DatasourceRef, depen
 			Subject:  &cfg.Datasources[ref].block.DefRange,
 			Severity: hcl.DiagError,
 		})
-		return dependencies, diags, shouldContinue
+		return dependencies, diags
 	}
 
 	ds.value = realValue
 	cfg.Datasources[ref] = ds
 	// remove ref from the dependencies map.
 	delete(dependencies, ref)
-	return dependencies, diags, shouldContinue
+	return dependencies, diags
 }
 
 // getCoreBuildProvisioners takes a list of provisioner block, starts according

diff --git a/hcl2template/utils.go b/hcl2template/utils.go
@@ -11,6 +11,7 @@ import (
 
 	"github.com/gobwas/glob"
 	"github.com/hashicorp/hcl/v2"
+	"github.com/hashicorp/hcl/v2/hclsyntax"
 	"github.com/hashicorp/packer/hcl2template/repl"
 	hcl2shim "github.com/hashicorp/packer/hcl2template/shim"
 	"github.com/zclconf/go-cty/cty"
@@ -186,3 +187,48 @@ func ConvertPluginConfigValueToHCLValue(v interface{}) (cty.Value, error) {
 	}
 	return buildValue, nil
 }
+
+// GetVarsByType walks through a hcl body, and gathers all the Traversals that
+// have a root type matching one of the specified top-level labels.
+//
+// This will only work on finite, expanded, HCL bodies.
+func GetVarsByType(block *hcl.Block, topLevelLabels ...string) []hcl.Traversal {
+	var travs []hcl.Traversal
+
+	switch body := block.Body.(type) {
+	case *hclsyntax.Body:
+		travs = getVarsByTypeForHCLSyntaxBody(body)
+	default:
+		attrs, _ := body.JustAttributes()
+		for _, attr := range attrs {
+			travs = append(travs, attr.Expr.Variables()...)
+		}
+	}
+
+	var rets []hcl.Traversal
+	for _, t := range travs {
+		varRootname := t.RootName()
+		for _, lbl := range topLevelLabels {
+			if varRootname == lbl {
+				rets = append(rets, t)
+				break
+			}
+		}
+	}
+
+	return rets
+}
+
+func getVarsByTypeForHCLSyntaxBody(body *hclsyntax.Body) []hcl.Traversal {
+	var rets []hcl.Traversal
+
+	for _, attr := range body.Attributes {
+		rets = append(rets, attr.Expr.Variables()...)
+	}
+
+	for _, block := range body.Blocks {
+		rets = append(rets, getVarsByTypeForHCLSyntaxBody(block.Body)...)
+	}
+
+	return rets
+}