From 9dfb7a5ccd07f45065d713febedd0db52685033f Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Thu, 24 Aug 2023 15:29:11 -0400 Subject: [PATCH 01/28] hcl2template: remove unused shouldContinue bool Not sure why this was defined and returned, but the value was set, but never used, as such this is not useful to keep in the code, so let's simplify this now. --- hcl2template/types.packer_config.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 819107e86b3..9ef8de242e4 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -380,7 +380,7 @@ func (cfg *PackerConfig) evaluateDatasources(skipExecution bool) hcl.Diagnostics // 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 +395,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. @@ -410,7 +409,7 @@ func (cfg *PackerConfig) recursivelyEvaluateDatasources(ref DatasourceRef, depen "other data sources, or your data sources have a cyclic " + "dependency. Please simplify your config to continue. ", }) - return dependencies, diags, false + return dependencies, diags } ds := cfg.Datasources[ref] @@ -421,11 +420,11 @@ 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 } } } @@ -435,14 +434,14 @@ func (cfg *PackerConfig) recursivelyEvaluateDatasources(ref DatasourceRef, depen datasource, startDiags := cfg.startDatasource(cfg.parser.PluginConfig.DataSources, ref, true) 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 +454,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 From 8f22241e3d84f8beabbe8681330e9938d767afd2 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Thu, 24 Aug 2023 16:09:25 -0400 Subject: [PATCH 02/28] hcl2template: simplify datasource evaluation Since datasources are recursively evaluated depending on their dependencies, we don't need to pre-execute those that depend on nothing, as the recursive traversal of the datasources will take care of that for us. --- hcl2template/types.packer_config.go | 44 ++--------------------------- 1 file changed, 3 insertions(+), 41 deletions(-) diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 9ef8de242e4..386d2598677 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -313,6 +313,8 @@ func (cfg *PackerConfig) evaluateDatasources(skipExecution bool) hcl.Diagnostics // with the datasources in its context. // This is essentially creating a very primitive DAG just for data // source interdependencies. + dependencies[ref] = []DatasourceRef{} + block := ds.block body := block.Body attrs, _ := body.JustAttributes() @@ -330,51 +332,11 @@ func (cfg *PackerConfig) evaluateDatasources(skipExecution bool) hcl.Diagnostics 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} - } + dependencies[ref] = append(dependencies[ref], dependsOn) skipFirstEval = true } } } - - // 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 From 47d2aff246a0e1147316a640484eb48fcb42ebda Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Thu, 24 Aug 2023 16:10:51 -0400 Subject: [PATCH 03/28] hcl2template: report localtion for cycle detection When a datasource fails to be evaluated because a cycle has been detected, we point out one of the links of the chain now so that users have a better idea of what to look at. --- hcl2template/types.packer_config.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 386d2598677..1a80619878e 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -361,6 +361,8 @@ func (cfg *PackerConfig) recursivelyEvaluateDatasources(ref DatasourceRef, depen var diags hcl.Diagnostics var moreDiags hcl.Diagnostics + ds := cfg.Datasources[ref] + if depth > 10 { // Add a comment about recursion. diags = append(diags, &hcl.Diagnostic{ @@ -370,11 +372,11 @@ 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: &ds.block.DefRange, }) return dependencies, diags } - ds := cfg.Datasources[ref] // Make sure everything ref depends on has already been evaluated. for _, dep := range dependencies[ref] { if _, ok := dependencies[dep]; ok { From 041a5ee8eef0804338100c74dab140df3a2414e2 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Thu, 24 Aug 2023 16:11:55 -0400 Subject: [PATCH 04/28] hcl2template: extract attr filter code from ds Datasources use their attribute's expressions to determine whether or not they depend on another datasource, in order to get the list of dependencies and execute them before executing a datasource. This code may be useful later on for figuring out the dependencies for any block, so we move this code to the utils.go file, and use this for datasources. --- hcl2template/types.packer_config.go | 31 ++++++++++------------------- hcl2template/utils.go | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 1a80619878e..b8d37d7ac35 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -5,7 +5,6 @@ package hcl2template import ( "fmt" - "log" "sort" "strings" @@ -315,27 +314,17 @@ func (cfg *PackerConfig) evaluateDatasources(skipExecution bool) hcl.Diagnostics // source interdependencies. dependencies[ref] = []DatasourceRef{} - 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) - dependencies[ref] = append(dependencies[ref], dependsOn) - skipFirstEval = true - } + // 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) } } diff --git a/hcl2template/utils.go b/hcl2template/utils.go index dc2399a1034..4b983e91f6f 100644 --- a/hcl2template/utils.go +++ b/hcl2template/utils.go @@ -187,3 +187,23 @@ func ConvertPluginConfigValueToHCLValue(v interface{}) (cty.Value, error) { } return buildValue, nil } + +func GetVarsByType(block *hcl.Block, topLevelLabels ...string) []hcl.Traversal { + attributes, _ := block.Body.JustAttributes() + + var vars []hcl.Traversal + + for _, attr := range attributes { + for _, variable := range attr.Expr.Variables() { + rootLabel := variable.RootName() + for _, label := range topLevelLabels { + if label == rootLabel { + vars = append(vars, variable) + break + } + } + } + } + + return vars +} From 87dbdb18c4304f4b78f5162986fd0937e4a40da5 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Fri, 25 Aug 2023 14:55:26 -0400 Subject: [PATCH 05/28] hcl2template: simplify startDatasource function --- hcl2template/types.datasource.go | 17 +++++++++-------- hcl2template/types.packer_config.go | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/hcl2template/types.datasource.go b/hcl2template/types.datasource.go index 8ac2c160827..ce0ef9cf12c 100644 --- 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,13 +64,15 @@ 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: fmt.Sprintf("packer does not currently know any data source."), Severity: hcl.DiagError, @@ -79,9 +80,9 @@ func (cfg *PackerConfig) startDatasource(dataSourceStore packer.DatasourceStore, 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, @@ -89,7 +90,7 @@ func (cfg *PackerConfig) startDatasource(dataSourceStore packer.DatasourceStore, 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 index b8d37d7ac35..1ae57a79724 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -384,7 +384,7 @@ func (cfg *PackerConfig) recursivelyEvaluateDatasources(ref DatasourceRef, depen // 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 From 161713450a55296436f8f27be2ea44c84f53bc23 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Fri, 25 Aug 2023 17:00:03 -0400 Subject: [PATCH 06/28] hcl2template: fix func to get vars from a config The previous implementation of the GetVarsByType function worked only on top-level attributes, ignoring the nested blocks in the structure. This implies that if a datasource depends on another through an expression within a nested block, we may not execute it first, and then executing this datasource before its dependent is possible, resulting in an error in the end. This commit is an attempt at making this more reliable for HCL configs, but only works on configs lifted from HCL files for now. We need to make this more reliable for later iterations. --- hcl2template/utils.go | 54 ++++++++++++++++++++++++++++++++----------- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/hcl2template/utils.go b/hcl2template/utils.go index 4b983e91f6f..9c8e5622b6d 100644 --- a/hcl2template/utils.go +++ b/hcl2template/utils.go @@ -12,6 +12,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" @@ -188,22 +189,49 @@ 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. +// +// TODO: make it work on JSON serialised HCL2 templates. func GetVarsByType(block *hcl.Block, topLevelLabels ...string) []hcl.Traversal { - attributes, _ := block.Body.JustAttributes() - - var vars []hcl.Traversal - - for _, attr := range attributes { - for _, variable := range attr.Expr.Variables() { - rootLabel := variable.RootName() - for _, label := range topLevelLabels { - if label == rootLabel { - vars = append(vars, variable) - break - } + 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 vars + 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 } From 69792308bfaff9688c3c254ae40075e64c29cdf7 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Thu, 24 Aug 2023 14:44:05 -0400 Subject: [PATCH 07/28] hcl2template: rework parsing logic In previous versions of Packer, the parser would load a HCL file (in either HCL or JSON format), and process it in phases. This process meant that if the file was not valid in the first place, we'd have some duplicate errors first. Another thing is that sources and build blocks were not being parsed at the beginning of the parsing phase at all, but were discovered later in the process, when the data sources have been executed and variables have been evaluated. This commit changes the parsing to happen all at once, so we parse the files with the parser, which will fail should one file be malformatted, then we visit the body once using the top-level schema for a Packer build template, handling misplaced blocks once in the process. Then, since the builds and sources may contain some dynamic blocks that need to be expanded once their context is ready (i.e. the variables/datasources they depend on), we can expand the build block for them, and populate their respective objects. This last step should also be adapted for datasources later, as they may need to support dynamic blocks, and be expanded lazily. --- hcl2template/common_test.go | 3 + hcl2template/parser.go | 394 +++++++++++++----- hcl2template/plugin.go | 15 + hcl2template/types.build.go | 84 ++-- .../types.build.hcp_packer_registry.go | 2 +- .../types.build.hcp_packer_registry_test.go | 10 + hcl2template/types.build.post-processor.go | 2 +- hcl2template/types.build.provisioners.go | 2 +- hcl2template/types.build.provisioners_test.go | 2 +- hcl2template/types.build_test.go | 28 +- hcl2template/types.datasource.go | 29 -- hcl2template/types.packer_config.go | 49 +-- hcl2template/types.packer_config_test.go | 15 +- hcl2template/types.required_plugins.go | 31 -- hcl2template/types.required_plugins_test.go | 2 +- hcl2template/types.source.go | 39 +- hcl2template/types.source_test.go | 6 +- hcl2template/types.variables.go | 29 -- hcl2template/types.variables_test.go | 5 +- 19 files changed, 435 insertions(+), 312 deletions(-) diff --git a/hcl2template/common_test.go b/hcl2template/common_test.go index 831ce5d3596..08c6ae647eb 100644 --- a/hcl2template/common_test.go +++ b/hcl2template/common_test.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/go-version" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclparse" + "github.com/hashicorp/hcl/v2/hclsyntax" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" "github.com/hashicorp/packer-plugin-sdk/template/config" "github.com/hashicorp/packer/builder/null" @@ -348,6 +349,7 @@ var cmpOpts = []cmp.Option{ cmpopts.IgnoreUnexported( PackerConfig{}, Variable{}, + BuildBlock{}, SourceBlock{}, DatasourceBlock{}, ProvisionerBlock{}, @@ -376,6 +378,7 @@ var cmpOpts = []cmp.Option{ cmpopts.IgnoreFields(packer.CoreBuildPostProcessor{}, "HCLConfig", ), + cmpopts.IgnoreTypes(hclsyntax.Body{}), cmpopts.IgnoreTypes(hcl2template.MockBuilder{}), cmpopts.IgnoreTypes(HCL2Ref{}), cmpopts.IgnoreTypes([]*LocalBlock{}), diff --git a/hcl2template/parser.go b/hcl2template/parser.go index fc9182bf04f..95422bfad6e 100644 --- a/hcl2template/parser.go +++ b/hcl2template/parser.go @@ -11,10 +11,14 @@ import ( "github.com/hashicorp/go-version" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/ext/dynblock" + "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/hcl/v2/hclparse" + "github.com/hashicorp/hcl/v2/hclsyntax" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + pkrfunction "github.com/hashicorp/packer/hcl2template/function" "github.com/hashicorp/packer/packer" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/function" ) const ( @@ -163,33 +167,8 @@ func (p *Parser) Parse(filename string, varFiles []string, argVars map[string]st return cfg, diags } - // Decode required_plugins blocks. - // - // Note: using `latest` ( or actually an empty string ) in a config file - // does not work and packer will ask you to pick a version - { - for _, file := range files { - diags = append(diags, cfg.decodeRequiredPluginsBlock(file)...) - } - } - - // Decode variable blocks so that they are available later on. Here locals - // can use input variables so we decode input variables first. - { - for _, file := range files { - diags = append(diags, cfg.decodeInputVariables(file)...) - } - - for _, file := range files { - morediags := p.decodeDatasources(file, cfg) - diags = append(diags, morediags...) - } - - for _, file := range files { - moreLocals, morediags := parseLocalVariableBlocks(file) - diags = append(diags, morediags...) - cfg.LocalBlocks = append(cfg.LocalBlocks, moreLocals...) - } + for _, file := range files { + diags = append(diags, cfg.decodeFile(file)...) } // parse var files @@ -293,112 +272,311 @@ func filterVarsFromLogs(inputOrLocal Variables) { } } -func (cfg *PackerConfig) Initialize(opts packer.InitializeOptions) hcl.Diagnostics { - diags := cfg.InputVariables.ValidateValues() - diags = append(diags, cfg.LocalVariables.ValidateValues()...) - diags = append(diags, cfg.evaluateDatasources(opts.SkipDatasourcesExecution)...) - diags = append(diags, checkForDuplicateLocalDefinition(cfg.LocalBlocks)...) - diags = append(diags, cfg.evaluateLocalVariables(cfg.LocalBlocks)...) +// decodeFile attempts to decode the configuration from a HCL file, and starts +// populating the config from it. +func (cfg *PackerConfig) decodeFile(file *hcl.File) hcl.Diagnostics { + var diags hcl.Diagnostics - filterVarsFromLogs(cfg.InputVariables) - filterVarsFromLogs(cfg.LocalVariables) + content, moreDiags := file.Body.Content(configSchema) + diags = append(diags, moreDiags...) + // If basic parsing failed, we should not continue + if diags.HasErrors() { + return diags + } - // parse the actual content // rest - for _, file := range cfg.files { - diags = append(diags, cfg.parser.parseConfig(file, cfg)...) + for _, block := range content.Blocks { + diags = append(diags, cfg.decodeBlock(block)...) } - diags = append(diags, cfg.initializeBlocks()...) + return diags +} + +func (cfg *PackerConfig) decodeBlock(block *hcl.Block) hcl.Diagnostics { + switch block.Type { + case packerLabel: + return cfg.decodePackerBlock(block) + case dataSourceLabel: + return cfg.decodeDatasource(block) + case variableLabel: + return cfg.decodeVariableBlock(block) + case variablesLabel: + return cfg.decodeVariablesBlock(block) + case localLabel: + return cfg.decodeLocalBlock(block) + case localsLabel: + return cfg.decodeLocalsBlock(block) + // NOTE: Both build and source blocks can be dynamically expanded. + // + // This means that while at this time we can already get some information + // about them, we may not be able to decode their final form at this time + // and we can only do so when their dependencies are evaluated. + case buildLabel: + return cfg.decodeBuildBlock(block) + case sourceLabel: + return cfg.decodeSourceBlock(block) + } + + return hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid block type", + Detail: fmt.Sprintf("The block %q is not a valid top-level block", block.Type), + Subject: &block.DefRange, + }, + } +} + +func (cfg *PackerConfig) decodePackerBlock(block *hcl.Block) hcl.Diagnostics { + var diags hcl.Diagnostics + content, contentDiags := block.Body.Content(packerBlockSchema) + diags = append(diags, contentDiags...) + + // We ignore "packer_version"" here because + // sniffCoreVersionRequirements already dealt with that + for _, innerBlock := range content.Blocks { + switch innerBlock.Type { + case "required_plugins": + reqs, reqsDiags := decodeRequiredPluginsBlock(innerBlock) + diags = append(diags, reqsDiags...) + cfg.Packer.RequiredPlugins = append(cfg.Packer.RequiredPlugins, reqs) + default: + continue + } + + } + + return diags +} + +func (cfg *PackerConfig) decodeDatasource(block *hcl.Block) hcl.Diagnostics { + datasource, diags := cfg.decodeDataBlock(block) + if diags.HasErrors() { + return diags + } + ref := datasource.Ref() + if existing, found := cfg.Datasources[ref]; found { + return append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Duplicate " + dataSourceLabel + " block", + Detail: fmt.Sprintf("This "+dataSourceLabel+" block has the "+ + "same data type and name as a previous block declared "+ + "at %s. Each "+dataSourceLabel+" must have a unique name per builder type.", + existing.block.DefRange.Ptr()), + Subject: datasource.block.DefRange.Ptr(), + }) + } + if cfg.Datasources == nil { + cfg.Datasources = Datasources{} + } + cfg.Datasources[ref] = *datasource return diags } -// parseConfig looks in the found blocks for everything that is not a variable -// block. -func (p *Parser) parseConfig(f *hcl.File, cfg *PackerConfig) hcl.Diagnostics { +func (cfg *PackerConfig) decodeDataBlock(block *hcl.Block) (*DatasourceBlock, hcl.Diagnostics) { var diags hcl.Diagnostics + r := &DatasourceBlock{ + Type: block.Labels[0], + Name: block.Labels[1], + block: block, + } - body := f.Body - body = dynblock.Expand(body, cfg.EvalContext(DatasourceContext, nil)) - content, moreDiags := body.Content(configSchema) - diags = append(diags, moreDiags...) + if !hclsyntax.ValidIdentifier(r.Type) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid data source name", + Detail: badIdentifierDetail, + Subject: &block.LabelRanges[0], + }) + } + if !hclsyntax.ValidIdentifier(r.Name) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid data resource name", + Detail: badIdentifierDetail, + Subject: &block.LabelRanges[1], + }) + } - for _, block := range content.Blocks { - switch block.Type { - case sourceLabel: - source, moreDiags := p.decodeSource(block) - diags = append(diags, moreDiags...) - if moreDiags.HasErrors() { - continue - } + return r, diags +} - ref := source.Ref() - if existing, found := cfg.Sources[ref]; found { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Duplicate " + sourceLabel + " block", - Detail: fmt.Sprintf("This "+sourceLabel+" block has the "+ - "same builder type and name as a previous block declared "+ - "at %s. Each "+sourceLabel+" must have a unique name per builder type.", - existing.block.DefRange.Ptr()), - Subject: source.block.DefRange.Ptr(), - }) - continue - } +func (cfg *PackerConfig) decodeLocalBlock(block *hcl.Block) hcl.Diagnostics { + name := block.Labels[0] - if cfg.Sources == nil { - cfg.Sources = map[SourceRef]SourceBlock{} - } - cfg.Sources[ref] = source + content, diags := block.Body.Content(localBlockSchema) + if !hclsyntax.ValidIdentifier(name) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid local name", + Detail: badIdentifierDetail, + Subject: &block.LabelRanges[0], + }) + } - case buildLabel: - build, moreDiags := p.decodeBuildConfig(block, cfg) - diags = append(diags, moreDiags...) - if moreDiags.HasErrors() { - continue - } + l := &LocalBlock{ + Name: name, + } - cfg.Builds = append(cfg.Builds, build) - } + if attr, exists := content.Attributes["sensitive"]; exists { + valDiags := gohcl.DecodeExpression(attr.Expr, nil, &l.Sensitive) + diags = append(diags, valDiags...) + } + + if def, ok := content.Attributes["expression"]; ok { + l.Expr = def.Expr + } + + cfg.LocalBlocks = append(cfg.LocalBlocks, l) + + return diags +} + +func (cfg *PackerConfig) decodeLocalsBlock(block *hcl.Block) hcl.Diagnostics { + attrs, diags := block.Body.JustAttributes() + + for name, attr := range attrs { + cfg.LocalBlocks = append(cfg.LocalBlocks, &LocalBlock{ + Name: name, + Expr: attr.Expr, + }) } return diags } -func (p *Parser) decodeDatasources(file *hcl.File, cfg *PackerConfig) hcl.Diagnostics { +func (cfg *PackerConfig) decodeVariableBlock(block *hcl.Block) hcl.Diagnostics { + // for input variables we allow to use env in the default value section. + ectx := &hcl.EvalContext{ + Functions: map[string]function.Function{ + "env": pkrfunction.EnvFunc, + }, + } + + return cfg.InputVariables.decodeVariableBlock(block, ectx) +} + +func (cfg *PackerConfig) decodeVariablesBlock(block *hcl.Block) hcl.Diagnostics { + // for input variables we allow to use env in the default value section. + ectx := &hcl.EvalContext{ + Functions: map[string]function.Function{ + "env": pkrfunction.EnvFunc, + }, + } + var diags hcl.Diagnostics - body := file.Body - content, moreDiags := body.Content(configSchema) + attrs, moreDiags := block.Body.JustAttributes() diags = append(diags, moreDiags...) + for key, attr := range attrs { + moreDiags = cfg.InputVariables.decodeVariable(key, attr, ectx) + diags = append(diags, moreDiags...) + } - for _, block := range content.Blocks { - switch block.Type { - case dataSourceLabel: - datasource, moreDiags := p.decodeDataBlock(block) - diags = append(diags, moreDiags...) - if moreDiags.HasErrors() { - continue - } - ref := datasource.Ref() - if existing, found := cfg.Datasources[ref]; found { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Duplicate " + dataSourceLabel + " block", - Detail: fmt.Sprintf("This "+dataSourceLabel+" block has the "+ - "same data type and name as a previous block declared "+ - "at %s. Each "+dataSourceLabel+" must have a unique name per builder type.", - existing.block.DefRange.Ptr()), - Subject: datasource.block.DefRange.Ptr(), - }) - continue - } - if cfg.Datasources == nil { - cfg.Datasources = Datasources{} - } - cfg.Datasources[ref] = *datasource - } + return diags +} + +// decodeBuildBlock shallowly decodes a build block from the config. +// +// The final decoding step (which requires an up-to-date context) will be done +// when we need it. +func (cfg *PackerConfig) decodeBuildBlock(block *hcl.Block) hcl.Diagnostics { + build := &BuildBlock{ + block: block, + } + + cfg.Builds = append(cfg.Builds, build) + + return nil +} + +func (cfg *PackerConfig) decodeSourceBlock(block *hcl.Block) hcl.Diagnostics { + source, diags := cfg.decodeSource(block) + if diags.HasErrors() { + return diags + } + + if cfg.Sources == nil { + cfg.Sources = map[SourceRef]SourceBlock{} + } + + ref := source.Ref() + if existing, found := cfg.Sources[ref]; found { + return append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Duplicate " + sourceLabel + " block", + Detail: fmt.Sprintf("This "+sourceLabel+" block has the "+ + "same builder type and name as a previous block declared "+ + "at %s. Each "+sourceLabel+" must have a unique name per builder type.", + existing.block.DefRange.Ptr()), + Subject: source.block.DefRange.Ptr(), + }) + } + + cfg.Sources[ref] = source + + return diags +} + +// decodeBuildSource reads a used source block from a build: +// +// build { +// source "type.example" { +// name = "local_name" +// } +// } +func (cfg *PackerConfig) decodeBuildSource(block *hcl.Block) (SourceUseBlock, hcl.Diagnostics) { + ref := sourceRefFromString(block.Labels[0]) + out := SourceUseBlock{SourceRef: ref} + var b struct { + Name string `hcl:"name,optional"` + Rest hcl.Body `hcl:",remain"` + } + diags := gohcl.DecodeBody(block.Body, nil, &b) + if diags.HasErrors() { + return out, diags + } + out.LocalName = b.Name + out.Body = b.Rest + return out, nil +} + +func (source *SourceBlock) finalizeDecodeSource(cfg *PackerConfig) hcl.Diagnostics { + if source.Ready { + return nil + } + + source.Ready = true + dyn := dynblock.Expand(source.block.Body, cfg.EvalContext(DatasourceContext, nil)) + // Expand without a base schema since nothing is known in advance for a + // source, but we still want to expand dynamic blocks if any + _, rem, diags := dyn.PartialContent(&hcl.BodySchema{}) + + // Only try to expand once, regardless of whether the source succeeded + // to expand dynamic data or not. + source.Ready = true + if diags.HasErrors() { + return diags + } + + source.block = &hcl.Block{ + Labels: []string{ + source.Type, + source.Name, + }, + Body: rem, } return diags } + +func (cfg *PackerConfig) decodeSource(block *hcl.Block) (SourceBlock, hcl.Diagnostics) { + source := SourceBlock{ + Type: block.Labels[0], + Name: block.Labels[1], + block: block, + } + var diags hcl.Diagnostics + + return source, diags +} diff --git a/hcl2template/plugin.go b/hcl2template/plugin.go index 323263596f8..287ce8fa0a3 100644 --- a/hcl2template/plugin.go +++ b/hcl2template/plugin.go @@ -128,6 +128,14 @@ func (cfg *PackerConfig) initializeBlocks() hcl.Diagnostics { var diags hcl.Diagnostics for _, build := range cfg.Builds { + // Since the build's contents may not have been dynamically + // expanded when we first loaded the config from the template + // file, we decode it now. + diags = append(diags, build.finalizeDecode(cfg)...) + if diags.HasErrors() { + continue + } + for i := range build.Sources { // here we grab a pointer to the source usage because we will set // its body. @@ -158,6 +166,13 @@ func (cfg *PackerConfig) initializeBlocks() hcl.Diagnostics { continue } + // Before attempting to use the body for merging, we + // finalise its decoding if necessary. + diags = append(diags, sourceDefinition.finalizeDecodeSource(cfg)...) + if diags.HasErrors() { + continue + } + body := sourceDefinition.block.Body if srcUsage.Body != nil { // merge additions into source definition to get a new body. diff --git a/hcl2template/types.build.go b/hcl2template/types.build.go index 648305ee202..bd8778fc0cd 100644 --- a/hcl2template/types.build.go +++ b/hcl2template/types.build.go @@ -7,6 +7,7 @@ import ( "fmt" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/ext/dynblock" "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/zclconf/go-cty/cty" @@ -59,6 +60,12 @@ type BuildBlock struct { // Name is a string representing the named build to show in the logs Name string + // Block is the raw hcl block lifted from the HCL file + block *hcl.Block + // ready marks whether or not there's any decoding left to do before + // using the data from the build block. + ready bool + // A description of what this build does, it could be used in a inspect // call for example. Description string @@ -87,9 +94,18 @@ type BuildBlock struct { type Builds []*BuildBlock -// decodeBuildConfig is called when a 'build' block has been detected. It will -// load the references to the contents of the build block. -func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildBlock, hcl.Diagnostics) { +// finalizeDecode finalises decoding the build block. +// +// This is only called after we've finished evaluating the dependencies for the +// build, and will expand the dynamic block for it, if any were present at first. +func (build *BuildBlock) finalizeDecode(cfg *PackerConfig) hcl.Diagnostics { + // If the build is already populated, we don't attempt to do anything here. + if build.ready { + return nil + } + + build.ready = true + var b struct { Name string `hcl:"name,optional"` Description string `hcl:"description,optional"` @@ -97,25 +113,30 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildB Config hcl.Body `hcl:",remain"` } - body := block.Body - diags := gohcl.DecodeBody(body, cfg.EvalContext(LocalContext, nil), &b) - if diags.HasErrors() { - return nil, diags - } + var diags hcl.Diagnostics - build := &BuildBlock{ - HCL2Ref: newHCL2Ref(block, b.Config), - } + body := build.block.Body + // At this point we can discard this decode's diags since it has already + // been sucessfully done once during the initial pre-decoding phase (at + // parsing-time) + _ = gohcl.DecodeBody(body, cfg.EvalContext(LocalContext, nil), &b) + // Here we'll replace the base contents from what we re-extracted at the + // time, as some things may be derived from other components through expressions + // or interpolation. build.Name = b.Name build.Description = b.Description - build.HCL2Ref.DefRange = block.DefRange + build.HCL2Ref = newHCL2Ref(build.block, b.Config) - // Expose build.name during parsing of pps and provisioners ectx := cfg.EvalContext(BuildContext, nil) - ectx.Variables[buildAccessor] = cty.ObjectVal(map[string]cty.Value{ - "name": cty.StringVal(b.Name), - }) + // Expand dynamics: we wrap the config in a dynblock and request the final + // content. If something cannot be expanded for some reason here (invalid + // reference, unknown values, etc.), this will fail, as it should. + dyn := dynblock.Expand(b.Config, ectx) + content, expandDiags := dyn.Content(buildSchema) + if expandDiags.HasErrors() { + return append(diags, expandDiags...) + } // We rely on `hadSource` to determine which error to proc. // @@ -124,6 +145,11 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildB // source is processed. hadSource := false + // Expose build.name during parsing of pps and provisioners + ectx.Variables[buildAccessor] = cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal(b.Name), + }) + for _, buildFrom := range b.FromSources { hadSource = true @@ -135,11 +161,11 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildB diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid " + sourceLabel + " reference", - Detail: "A " + sourceLabel + " type is made of three parts that are" + + Detail: "A " + sourceLabel + " type is made of two or three parts that are" + "split by a dot `.`; each part must start with a letter and " + "may contain only letters, digits, underscores, and dashes." + "A valid source reference looks like: `source.type.name`", - Subject: block.DefRange.Ptr(), + Subject: build.block.DefRange.Ptr(), }) continue } @@ -148,12 +174,6 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildB build.Sources = append(build.Sources, SourceUseBlock{SourceRef: ref}) } - body = b.Config - content, moreDiags := body.Content(buildSchema) - diags = append(diags, moreDiags...) - if diags.HasErrors() { - return nil, diags - } for _, block := range content.Blocks { switch block.Type { case buildHCPPackerRegistryLabel: @@ -165,7 +185,7 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildB }) continue } - hcpPackerRegistry, moreDiags := p.decodeHCPRegistry(block, cfg) + hcpPackerRegistry, moreDiags := cfg.decodeHCPRegistry(block) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { continue @@ -173,14 +193,14 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildB build.HCPPackerRegistry = hcpPackerRegistry case sourceLabel: hadSource = true - ref, moreDiags := p.decodeBuildSource(block) + ref, moreDiags := cfg.decodeBuildSource(block) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { continue } build.Sources = append(build.Sources, ref) case buildProvisionerLabel: - p, moreDiags := p.decodeProvisioner(block, ectx) + p, moreDiags := cfg.decodeProvisioner(block, ectx) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { continue @@ -195,14 +215,14 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildB }) continue } - p, moreDiags := p.decodeProvisioner(block, ectx) + p, moreDiags := cfg.decodeProvisioner(block, ectx) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { continue } build.ErrorCleanupProvisionerBlock = p case buildPostProcessorLabel: - pp, moreDiags := p.decodePostProcessor(block, ectx) + pp, moreDiags := cfg.decodePostProcessor(block, ectx) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { continue @@ -219,7 +239,7 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildB errored := false postProcessors := []*PostProcessorBlock{} for _, block := range content.Blocks { - pp, moreDiags := p.decodePostProcessor(block, ectx) + pp, moreDiags := cfg.decodePostProcessor(block, ectx) diags = append(diags, moreDiags...) if moreDiags.HasErrors() { errored = true @@ -238,9 +258,9 @@ func (p *Parser) decodeBuildConfig(block *hcl.Block, cfg *PackerConfig) (*BuildB Summary: "missing source reference", Detail: "a build block must reference at least one source to be built", Severity: hcl.DiagError, - Subject: block.DefRange.Ptr(), + Subject: build.block.DefRange.Ptr(), }) } - return build, diags + return diags } diff --git a/hcl2template/types.build.hcp_packer_registry.go b/hcl2template/types.build.hcp_packer_registry.go index b64b3ff4e55..b28621a034a 100644 --- a/hcl2template/types.build.hcp_packer_registry.go +++ b/hcl2template/types.build.hcp_packer_registry.go @@ -23,7 +23,7 @@ type HCPPackerRegistryBlock struct { HCL2Ref } -func (p *Parser) decodeHCPRegistry(block *hcl.Block, cfg *PackerConfig) (*HCPPackerRegistryBlock, hcl.Diagnostics) { +func (cfg *PackerConfig) decodeHCPRegistry(block *hcl.Block) (*HCPPackerRegistryBlock, hcl.Diagnostics) { par := &HCPPackerRegistryBlock{} body := block.Body diff --git a/hcl2template/types.build.hcp_packer_registry_test.go b/hcl2template/types.build.hcp_packer_registry_test.go index 3ada389c29d..cfef9527751 100644 --- a/hcl2template/types.build.hcp_packer_registry_test.go +++ b/hcl2template/types.build.hcp_packer_registry_test.go @@ -118,6 +118,11 @@ func Test_ParseHCPPackerRegistryBlock(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Basedir: filepath.Join("testdata", "hcp_par"), + Builds: Builds{ + &BuildBlock{ + Name: "bucket-slug", + }, + }, }, true, true, nil, @@ -129,6 +134,11 @@ func Test_ParseHCPPackerRegistryBlock(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Basedir: filepath.Join("testdata", "hcp_par"), + Builds: Builds{ + &BuildBlock{ + Name: "bucket-slug", + }, + }, }, true, true, nil, diff --git a/hcl2template/types.build.post-processor.go b/hcl2template/types.build.post-processor.go index 8844eadff11..1cf943b0bd6 100644 --- a/hcl2template/types.build.post-processor.go +++ b/hcl2template/types.build.post-processor.go @@ -26,7 +26,7 @@ func (p *PostProcessorBlock) String() string { return fmt.Sprintf(buildPostProcessorLabel+"-block %q %q", p.PType, p.PName) } -func (p *Parser) decodePostProcessor(block *hcl.Block, ectx *hcl.EvalContext) (*PostProcessorBlock, hcl.Diagnostics) { +func (cfg *PackerConfig) decodePostProcessor(block *hcl.Block, ectx *hcl.EvalContext) (*PostProcessorBlock, hcl.Diagnostics) { var b struct { Name string `hcl:"name,optional"` Only []string `hcl:"only,optional"` diff --git a/hcl2template/types.build.provisioners.go b/hcl2template/types.build.provisioners.go index b08eca59f63..dd6fb71ae7f 100644 --- a/hcl2template/types.build.provisioners.go +++ b/hcl2template/types.build.provisioners.go @@ -77,7 +77,7 @@ func (p *ProvisionerBlock) String() string { return fmt.Sprintf(buildProvisionerLabel+"-block %q %q", p.PType, p.PName) } -func (p *Parser) decodeProvisioner(block *hcl.Block, ectx *hcl.EvalContext) (*ProvisionerBlock, hcl.Diagnostics) { +func (cfg *PackerConfig) decodeProvisioner(block *hcl.Block, ectx *hcl.EvalContext) (*ProvisionerBlock, hcl.Diagnostics) { var b struct { Name string `hcl:"name,optional"` PauseBefore string `hcl:"pause_before,optional"` diff --git a/hcl2template/types.build.provisioners_test.go b/hcl2template/types.build.provisioners_test.go index 284651f2cbd..f4a1558a5d7 100644 --- a/hcl2template/types.build.provisioners_test.go +++ b/hcl2template/types.build.provisioners_test.go @@ -52,7 +52,7 @@ func TestPackerConfig_ParseProvisionerBlock(t *testing.T) { Column: 1, Byte: 0, }) - _, diags = cfg.parser.decodeProvisioner(provBlock, nil) + _, diags = cfg.decodeProvisioner(provBlock, nil) if !diags.HasErrors() { if !test.expectError { diff --git a/hcl2template/types.build_test.go b/hcl2template/types.build_test.go index 8647821dd18..808929ed83a 100644 --- a/hcl2template/types.build_test.go +++ b/hcl2template/types.build_test.go @@ -64,7 +64,9 @@ func TestParse_build(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Basedir: filepath.Join("testdata", "build"), - Builds: nil, + Builds: Builds{ + &BuildBlock{}, + }, }, true, true, nil, @@ -118,6 +120,18 @@ func TestParse_build(t *testing.T) { Sources: map[SourceRef]SourceBlock{ refVBIsoUbuntu1204: {Type: "virtualbox-iso", Name: "ubuntu-1204"}, }, + Builds: Builds{ + &BuildBlock{ + Sources: []SourceUseBlock{ + { + SourceRef: refVBIsoUbuntu1204, + }, + }, + ErrorCleanupProvisionerBlock: &ProvisionerBlock{ + PType: "shell-local", + }, + }, + }, }, true, true, []packersdk.Build{&packer.CoreBuild{ @@ -142,7 +156,9 @@ func TestParse_build(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Basedir: filepath.Join("testdata", "build"), - Builds: nil, + Builds: Builds{ + &BuildBlock{}, + }, }, true, true, []packersdk.Build{&packer.CoreBuild{}}, @@ -195,7 +211,9 @@ func TestParse_build(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Basedir: filepath.Join("testdata", "build"), - Builds: nil, + Builds: Builds{ + &BuildBlock{}, + }, }, true, true, []packersdk.Build{}, @@ -565,7 +583,9 @@ func TestParse_build(t *testing.T) { CorePackerVersionString: lockedVersion, Basedir: filepath.Join("testdata", "build"), InputVariables: Variables{}, - Builds: nil, + Builds: Builds{ + &BuildBlock{}, + }, }, true, true, []packersdk.Build{}, diff --git a/hcl2template/types.datasource.go b/hcl2template/types.datasource.go index ce0ef9cf12c..aee2b9f2986 100644 --- a/hcl2template/types.datasource.go +++ b/hcl2template/types.datasource.go @@ -7,7 +7,6 @@ import ( "fmt" "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/hclsyntax" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" hcl2shim "github.com/hashicorp/packer/hcl2template/shim" "github.com/zclconf/go-cty/cty" @@ -131,31 +130,3 @@ func (cfg *PackerConfig) startDatasource(ds DatasourceBlock) (packersdk.Datasour } return datasource, diags } - -func (p *Parser) decodeDataBlock(block *hcl.Block) (*DatasourceBlock, hcl.Diagnostics) { - var diags hcl.Diagnostics - r := &DatasourceBlock{ - Type: block.Labels[0], - Name: block.Labels[1], - block: block, - } - - if !hclsyntax.ValidIdentifier(r.Type) { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid data source name", - Detail: badIdentifierDetail, - Subject: &block.LabelRanges[0], - }) - } - if !hclsyntax.ValidIdentifier(r.Name) { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid data resource name", - Detail: badIdentifierDetail, - Subject: &block.LabelRanges[1], - }) - } - - return r, diags -} diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 1ae57a79724..2a74b12fed4 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -178,40 +178,6 @@ func (c *PackerConfig) decodeInputVariables(f *hcl.File) hcl.Diagnostics { return diags } -// parseLocalVariableBlocks looks in the AST for 'local' and 'locals' blocks and -// returns them all. -func parseLocalVariableBlocks(f *hcl.File) ([]*LocalBlock, hcl.Diagnostics) { - var diags hcl.Diagnostics - - content, moreDiags := f.Body.Content(configSchema) - diags = append(diags, moreDiags...) - - var locals []*LocalBlock - - for _, block := range content.Blocks { - switch block.Type { - case localLabel: - block, moreDiags := decodeLocalBlock(block) - diags = append(diags, moreDiags...) - if moreDiags.HasErrors() { - return locals, diags - } - locals = append(locals, block) - case localsLabel: - attrs, moreDiags := block.Body.JustAttributes() - diags = append(diags, moreDiags...) - for name, attr := range attrs { - locals = append(locals, &LocalBlock{ - Name: name, - Expr: attr.Expr, - }) - } - } - } - - return locals, diags -} - func (c *PackerConfig) evaluateAllLocalVariables(locals []*LocalBlock) hcl.Diagnostics { var diags hcl.Diagnostics @@ -832,3 +798,18 @@ func (p *PackerConfig) InspectConfig(opts packer.InspectConfigOptions) int { ui.Say(p.printBuilds()) return 0 } + +func (cfg *PackerConfig) Initialize(opts packer.InitializeOptions) hcl.Diagnostics { + diags := cfg.InputVariables.ValidateValues() + diags = append(diags, cfg.LocalVariables.ValidateValues()...) + diags = append(diags, cfg.evaluateDatasources(opts.SkipDatasourcesExecution)...) + diags = append(diags, checkForDuplicateLocalDefinition(cfg.LocalBlocks)...) + diags = append(diags, cfg.evaluateLocalVariables(cfg.LocalBlocks)...) + + filterVarsFromLogs(cfg.InputVariables) + filterVarsFromLogs(cfg.LocalVariables) + + diags = append(diags, cfg.initializeBlocks()...) + + return diags +} diff --git a/hcl2template/types.packer_config_test.go b/hcl2template/types.packer_config_test.go index 5eae6a2f21f..3af072164b7 100644 --- a/hcl2template/types.packer_config_test.go +++ b/hcl2template/types.packer_config_test.go @@ -575,8 +575,19 @@ func TestParser_no_init(t *testing.T) { Type: cty.List(cty.String), }, }, - Sources: nil, - Builds: nil, + Sources: map[SourceRef]SourceBlock{ + refAWSV3MyImage: { + Type: "amazon-v3-ebs", + Name: "my-image", + }, + refVBIsoUbuntu1204: { + Type: "virtualbox-iso", + Name: "ubuntu-1204", + }, + }, + Builds: Builds{ + &BuildBlock{}, + }, }, false, false, []packersdk.Build{}, diff --git a/hcl2template/types.required_plugins.go b/hcl2template/types.required_plugins.go index d5e010345ea..c7e585f7893 100644 --- a/hcl2template/types.required_plugins.go +++ b/hcl2template/types.required_plugins.go @@ -12,37 +12,6 @@ import ( "github.com/zclconf/go-cty/cty" ) -func (cfg *PackerConfig) decodeRequiredPluginsBlock(f *hcl.File) hcl.Diagnostics { - var diags hcl.Diagnostics - - content, moreDiags := f.Body.Content(configSchema) - diags = append(diags, moreDiags...) - - for _, block := range content.Blocks { - switch block.Type { - case packerLabel: - content, contentDiags := block.Body.Content(packerBlockSchema) - diags = append(diags, contentDiags...) - - // We ignore "packer_version"" here because - // sniffCoreVersionRequirements already dealt with that - - for _, innerBlock := range content.Blocks { - switch innerBlock.Type { - case "required_plugins": - reqs, reqsDiags := decodeRequiredPluginsBlock(innerBlock) - diags = append(diags, reqsDiags...) - cfg.Packer.RequiredPlugins = append(cfg.Packer.RequiredPlugins, reqs) - default: - continue - } - - } - } - } - return diags -} - // RequiredPlugin represents a declaration of a dependency on a particular // Plugin version or source. type RequiredPlugin struct { diff --git a/hcl2template/types.required_plugins_test.go b/hcl2template/types.required_plugins_test.go index 0329d1505ff..e9bd9c48833 100644 --- a/hcl2template/types.required_plugins_test.go +++ b/hcl2template/types.required_plugins_test.go @@ -138,7 +138,7 @@ func TestPackerConfig_required_plugin_parse(t *testing.T) { if len(diags) > 0 { t.Fatal(diags) } - if diags := cfg.decodeRequiredPluginsBlock(file); len(diags) > 0 { + if diags := cfg.decodeFile(file); len(diags) > 0 { t.Fatal(diags) } diff --git a/hcl2template/types.source.go b/hcl2template/types.source.go index 46b9caac98e..450c8fd12c8 100644 --- a/hcl2template/types.source.go +++ b/hcl2template/types.source.go @@ -9,7 +9,6 @@ import ( "strconv" "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/gohcl" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" hcl2shim "github.com/hashicorp/packer/hcl2template/shim" "github.com/zclconf/go-cty/cty" @@ -23,6 +22,10 @@ type SourceBlock struct { // Given name; if any Name string + // ready signals that the source block is ready for use, i.e. it does not + // need some dynamic expansion before being used. + Ready bool + block *hcl.Block // LocalName can be set in a singular source block from a build block, it @@ -66,40 +69,6 @@ func (b *SourceUseBlock) ctyValues() map[string]cty.Value { } } -// decodeBuildSource reads a used source block from a build: -// -// build { -// source "type.example" { -// name = "local_name" -// } -// } -func (p *Parser) decodeBuildSource(block *hcl.Block) (SourceUseBlock, hcl.Diagnostics) { - ref := sourceRefFromString(block.Labels[0]) - out := SourceUseBlock{SourceRef: ref} - var b struct { - Name string `hcl:"name,optional"` - Rest hcl.Body `hcl:",remain"` - } - diags := gohcl.DecodeBody(block.Body, nil, &b) - if diags.HasErrors() { - return out, diags - } - out.LocalName = b.Name - out.Body = b.Rest - return out, nil -} - -func (p *Parser) decodeSource(block *hcl.Block) (SourceBlock, hcl.Diagnostics) { - source := SourceBlock{ - Type: block.Labels[0], - Name: block.Labels[1], - block: block, - } - var diags hcl.Diagnostics - - return source, diags -} - func (cfg *PackerConfig) startBuilder(source SourceUseBlock, ectx *hcl.EvalContext) (packersdk.Builder, hcl.Diagnostics, []string) { var diags hcl.Diagnostics diff --git a/hcl2template/types.source_test.go b/hcl2template/types.source_test.go index 47caeec5a75..3aa5cbd62fb 100644 --- a/hcl2template/types.source_test.go +++ b/hcl2template/types.source_test.go @@ -90,8 +90,10 @@ func TestParse_source(t *testing.T) { parseTestArgs{"testdata/sources/nonexistent.pkr.hcl", nil, nil}, &PackerConfig{ CorePackerVersionString: lockedVersion, - Builds: nil, - Basedir: filepath.Join("testdata", "sources"), + Builds: Builds{ + &BuildBlock{}, + }, + Basedir: filepath.Join("testdata", "sources"), Sources: map[SourceRef]SourceBlock{ {Type: "nonexistent", Name: "ubuntu-1204"}: {Type: "nonexistent", Name: "ubuntu-1204"}, }, diff --git a/hcl2template/types.variables.go b/hcl2template/types.variables.go index 33df28a84ee..f45aa72913c 100644 --- a/hcl2template/types.variables.go +++ b/hcl2template/types.variables.go @@ -278,35 +278,6 @@ var localBlockSchema = &hcl.BodySchema{ }, } -func decodeLocalBlock(block *hcl.Block) (*LocalBlock, hcl.Diagnostics) { - name := block.Labels[0] - - content, diags := block.Body.Content(localBlockSchema) - if !hclsyntax.ValidIdentifier(name) { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid local name", - Detail: badIdentifierDetail, - Subject: &block.LabelRanges[0], - }) - } - - l := &LocalBlock{ - Name: name, - } - - if attr, exists := content.Attributes["sensitive"]; exists { - valDiags := gohcl.DecodeExpression(attr.Expr, nil, &l.Sensitive) - diags = append(diags, valDiags...) - } - - if def, ok := content.Attributes["expression"]; ok { - l.Expr = def.Expr - } - - return l, diags -} - // decodeVariableBlock decodes a "variable" block // ectx is passed only in the evaluation of the default value. func (variables *Variables) decodeVariableBlock(block *hcl.Block, ectx *hcl.EvalContext) hcl.Diagnostics { diff --git a/hcl2template/types.variables_test.go b/hcl2template/types.variables_test.go index a19b80bd990..37bfc779f0d 100644 --- a/hcl2template/types.variables_test.go +++ b/hcl2template/types.variables_test.go @@ -248,7 +248,10 @@ func TestParse_variables(t *testing.T) { parseTestArgs{"testdata/variables/unset_used_string_variable.pkr.hcl", nil, nil}, &PackerConfig{ CorePackerVersionString: lockedVersion, - Basedir: filepath.Join("testdata", "variables"), + Builds: Builds{ + &BuildBlock{}, + }, + Basedir: filepath.Join("testdata", "variables"), InputVariables: Variables{ "foo": &Variable{ Name: "foo", From d5ddb2809daa07015356038c3441310163d823b1 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Fri, 1 Sep 2023 12:00:31 -0400 Subject: [PATCH 08/28] hcl2template: make datasources hold pointer refs The `Datasources' type holds a map of datasource references to their respective block. In this map, the blocks were structures rather than pointer to structures, which makes updating their contents a harder process than it should be. To allow for in-place changes for the datasource blocks, we change the type of the blocks to be a pointer instead. --- hcl2template/parser.go | 2 +- hcl2template/types.datasource.go | 2 +- hcl2template/types.packer_config_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hcl2template/parser.go b/hcl2template/parser.go index 95422bfad6e..8f0883a2220 100644 --- a/hcl2template/parser.go +++ b/hcl2template/parser.go @@ -368,7 +368,7 @@ func (cfg *PackerConfig) decodeDatasource(block *hcl.Block) hcl.Diagnostics { if cfg.Datasources == nil { cfg.Datasources = Datasources{} } - cfg.Datasources[ref] = *datasource + cfg.Datasources[ref] = datasource return diags } diff --git a/hcl2template/types.datasource.go b/hcl2template/types.datasource.go index aee2b9f2986..63558f28ad0 100644 --- a/hcl2template/types.datasource.go +++ b/hcl2template/types.datasource.go @@ -26,7 +26,7 @@ type DatasourceRef struct { Name string } -type Datasources map[DatasourceRef]DatasourceBlock +type Datasources map[DatasourceRef]*DatasourceBlock func (data *DatasourceBlock) Ref() DatasourceRef { return DatasourceRef{ diff --git a/hcl2template/types.packer_config_test.go b/hcl2template/types.packer_config_test.go index 3af072164b7..ee65f7a1d43 100644 --- a/hcl2template/types.packer_config_test.go +++ b/hcl2template/types.packer_config_test.go @@ -136,7 +136,7 @@ func TestParser_complete(t *testing.T) { }, }, Datasources: Datasources{ - DatasourceRef{Type: "amazon-ami", Name: "test"}: DatasourceBlock{ + DatasourceRef{Type: "amazon-ami", Name: "test"}: &DatasourceBlock{ Type: "amazon-ami", Name: "test", value: cty.StringVal("foo"), From 891bae751828c774a5cdbce4bb76da159710e96d Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Fri, 1 Sep 2023 12:02:18 -0400 Subject: [PATCH 09/28] hcl2template: break down datasource execution Since we want to allow for independently executing datasources when needed, we break down the logic to do so. Each datasource will now hold a list of dependencies. For now, these dependencies are all datasources themselves, but this may change in the future. Then, actual execution of the datasource is self-contained within a method of the structure, so that we can individually execute them. Finally, we replace the current sequential execution by a method of the PackerConfig with an approach akin to local variable evaluation, where we execute all the datasources one-by-one, and if they cannot be executed, move along. We retry this loop for as many times as we don't reach either a state where all the datasources have been executed; or when we reach a terminal state where datasources cannot be evaluated because of dependencies remaining, and no changes have occurred after a try. --- hcl2template/parser.go | 2 + hcl2template/types.datasource.go | 161 ++++++++++++++++++++++++++++ hcl2template/types.packer_config.go | 122 +-------------------- 3 files changed, 164 insertions(+), 121 deletions(-) diff --git a/hcl2template/parser.go b/hcl2template/parser.go index 8f0883a2220..d7c57ef1907 100644 --- a/hcl2template/parser.go +++ b/hcl2template/parser.go @@ -370,6 +370,8 @@ func (cfg *PackerConfig) decodeDatasource(block *hcl.Block) hcl.Diagnostics { } cfg.Datasources[ref] = datasource + datasource.getDependencies() + return diags } diff --git a/hcl2template/types.datasource.go b/hcl2template/types.datasource.go index 63558f28ad0..126582be38d 100644 --- a/hcl2template/types.datasource.go +++ b/hcl2template/types.datasource.go @@ -7,8 +7,10 @@ import ( "fmt" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hcldec" 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" ) @@ -19,6 +21,9 @@ type DatasourceBlock struct { value cty.Value block *hcl.Block + + // dependencies is the list of datasources to execute before this one + dependencies []DatasourceRef } type DatasourceRef struct { @@ -35,6 +40,101 @@ func (data *DatasourceBlock) Ref() DatasourceRef { } } +func (ds *DatasourceBlock) getDependencies() { + var dependencies []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. + dependencies = append(dependencies, DatasourceRef{ + Type: v[1].(hcl.TraverseAttr).Name, + Name: v[2].(hcl.TraverseAttr).Name, + }) + } + + ds.dependencies = dependencies +} + +const notReadyDataSourceError = "Dependencies not ready" + +// executed returns whether or not the datasource was executed +// +// Having a non-empty cty.Value object means this was filled-up after the +// datasource has been executed, so this is what we use for this test. +func (ds DatasourceBlock) executed() bool { + return ds.value != cty.Value{} +} + +// Execute starts the datasource and executes it immediately. +// +// If its dependencies are not ready for execution, this will return an error +// and will only execute when all its dependencies have executed. +func (ds *DatasourceBlock) Execute(cfg *PackerConfig, skipExecution bool) hcl.Diagnostics { + var diags hcl.Diagnostics + + ok := true + for _, depRef := range ds.dependencies { + dep := cfg.Datasources[depRef] + if dep == nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Nonexistent dependency referenced", + Detail: fmt.Sprintf("The referenced datasource %s.%s is not defined in the configuration, so this datasource won't be able to execute.", dep.Type, dep.Name), + Subject: &ds.block.DefRange, + }) + ok = false + continue + } + + if !dep.executed() { + ok = false + } + } + + if !ok { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: notReadyDataSourceError, + Detail: "At least one dependency for the datasource is not executed already", + Subject: &ds.block.DefRange, + }) + return 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, diags := cfg.startDatasource(*ds) + if diags.HasErrors() { + return diags + } + + if skipExecution { + placeholderValue := cty.UnknownVal(hcldec.ImpliedType(datasource.OutputSpec())) + ds.value = placeholderValue + return diags + } + + opts, _ := decodeHCL2Spec(ds.block.Body, cfg.EvalContext(DatasourceContext, nil), datasource) + sp := packer.CheckpointReporter.AddSpan(ds.Type, "datasource", opts) + realValue, err := datasource.Execute() + sp.End(err) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Summary: err.Error(), + Subject: &ds.block.DefRange, + Severity: hcl.DiagError, + }) + return diags + } + + ds.value = realValue + return diags +} + func (ds *Datasources) Values() (map[string]cty.Value, hcl.Diagnostics) { var diags hcl.Diagnostics res := map[string]cty.Value{} @@ -130,3 +230,64 @@ func (cfg *PackerConfig) startDatasource(ds DatasourceBlock) (packersdk.Datasour } return datasource, diags } + +// datasourcesDone checks whether all the datasources have been executed or not +func (cfg *PackerConfig) datasourcesDone() bool { + for _, ds := range cfg.Datasources { + if !ds.executed() { + return false + } + } + + return true +} + +func (cfg *PackerConfig) executeDatasources(skipExecution bool) hcl.Diagnostics { + // If we are done with datasources execution, we leave immediately + if cfg.datasourcesDone() { + return nil + } + + var outDiags hcl.Diagnostics + + foundSomething := false +outerDSEval: + for _, ds := range cfg.Datasources { + if ds.executed() { + continue + } + + diags := ds.Execute(cfg, skipExecution) + for _, diag := range diags { + if diag.Summary == notReadyDataSourceError { + // If we have a not ready error in the + // datasource list, we should attempt to run the + // rest, and eventually settle if we cannot move + // any further + continue outerDSEval + } + } + + foundSomething = true + + outDiags = append(outDiags, diags...) + } + + if !foundSomething { + return append(outDiags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "No datasource could be executed", + Detail: `While trying to recurisvely evaluating datasources, we could not find a next datasource to execute. +This is likely due to a cyclic dependency in your datasources`, + }) + } + + // If we couldn't execute a datasource for whatever reason, we leave + if outDiags.HasErrors() { + return outDiags + } + + // If we still found something to execute, we recursively execute the + // remainder of the datasources + return outDiags.Extend(cfg.executeDatasources(skipExecution)) +} diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 2a74b12fed4..6bb965785fd 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -10,7 +10,6 @@ import ( "github.com/gobwas/glob" hcl "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/hcldec" "github.com/hashicorp/hcl/v2/hclsyntax" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" pkrfunction "github.com/hashicorp/packer/hcl2template/function" @@ -264,125 +263,6 @@ func (c *PackerConfig) evaluateLocalVariable(local *LocalBlock) hcl.Diagnostics return diags } -func (cfg *PackerConfig) evaluateDatasources(skipExecution bool) hcl.Diagnostics { - var diags hcl.Diagnostics - - dependencies := map[DatasourceRef][]DatasourceRef{} - for ref, ds := range cfg.Datasources { - if ds.value != (cty.Value{}) { - continue - } - // Pre-examine body of this data source to see if it uses another data - // 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. - 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 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) - // Deduplicate diagnostics to prevent recursion messes. - cleanedDiags := map[string]*hcl.Diagnostic{} - for _, diag := range moreDiags { - cleanedDiags[diag.Summary] = diag - } - - for _, diag := range cleanedDiags { - diags = append(diags, diag) - } - } - - return diags -} - -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 - - ds := cfg.Datasources[ref] - - if depth > 10 { - // Add a comment about recursion. - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Max datasource recursion depth exceeded.", - Detail: "An error occured while recursively evaluating data " + - "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: &ds.block.DefRange, - }) - return dependencies, diags - } - - // Make sure everything ref depends on has already been evaluated. - for _, dep := range dependencies[ref] { - if _, ok := dependencies[dep]; ok { - depth += 1 - // 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 = cfg.recursivelyEvaluateDatasources(dep, dependencies, skipExecution, depth) - diags = append(diags, moreDiags...) - if moreDiags.HasErrors() { - diags = append(diags, moreDiags...) - 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(ds) - if startDiags.HasErrors() { - diags = append(diags, startDiags...) - return dependencies, diags - } - - if skipExecution { - placeholderValue := cty.UnknownVal(hcldec.ImpliedType(datasource.OutputSpec())) - ds.value = placeholderValue - cfg.Datasources[ref] = ds - return dependencies, diags - } - - opts, _ := decodeHCL2Spec(ds.block.Body, cfg.EvalContext(DatasourceContext, nil), datasource) - sp := packer.CheckpointReporter.AddSpan(ref.Type, "datasource", opts) - 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, - }) - return dependencies, diags - } - - ds.value = realValue - cfg.Datasources[ref] = ds - // remove ref from the dependencies map. - delete(dependencies, ref) - return dependencies, diags -} - // getCoreBuildProvisioners takes a list of provisioner block, starts according // provisioners and sends parsed HCL2 over to it. func (cfg *PackerConfig) getCoreBuildProvisioners(source SourceUseBlock, blocks []*ProvisionerBlock, ectx *hcl.EvalContext) ([]packer.CoreBuildProvisioner, hcl.Diagnostics) { @@ -802,7 +682,7 @@ func (p *PackerConfig) InspectConfig(opts packer.InspectConfigOptions) int { func (cfg *PackerConfig) Initialize(opts packer.InitializeOptions) hcl.Diagnostics { diags := cfg.InputVariables.ValidateValues() diags = append(diags, cfg.LocalVariables.ValidateValues()...) - diags = append(diags, cfg.evaluateDatasources(opts.SkipDatasourcesExecution)...) + diags = append(diags, cfg.executeDatasources(opts.SkipDatasourcesExecution)...) diags = append(diags, checkForDuplicateLocalDefinition(cfg.LocalBlocks)...) diags = append(diags, cfg.evaluateLocalVariables(cfg.LocalBlocks)...) From 747e26bef2aa92b7704004125a8127b503ffb7cc Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 6 Sep 2023 10:16:33 -0400 Subject: [PATCH 10/28] hcl2template: split traversal filtering In some cases, rather than walking through a full block of HCL in order to lift the references to other components of a build, we only have the expression to walk on. From this expression, we can lift the variables as a collection of hcl.Traversal, from which we then filter only those that interest us. So, since this is a use-case, we split the function that lifts the expressions from a hcl block and filter them based on the root type of the traversal into two separate functions. --- hcl2template/utils.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hcl2template/utils.go b/hcl2template/utils.go index 9c8e5622b6d..6c0c1af93b4 100644 --- a/hcl2template/utils.go +++ b/hcl2template/utils.go @@ -208,6 +208,10 @@ func GetVarsByType(block *hcl.Block, topLevelLabels ...string) []hcl.Traversal { } } + return filterTraversalsByRootType(travs, topLevelLabels...) +} + +func filterTraversalsByRootType(travs []hcl.Traversal, topLevelLabels ...string) []hcl.Traversal { var rets []hcl.Traversal for _, t := range travs { varRootname := t.RootName() From 8fc1a2618f9bb38cf765b63fd778b1e160561485 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 6 Sep 2023 10:19:28 -0400 Subject: [PATCH 11/28] hcl2template: breakdown local variable evaluation As with datasources, we break down the local variable evaluation in two functions, one doing the evaluation for a local variable, and the sequential/recursive evaluation of all the local variables lifted from the packer template. This, plus dependency management will let us later reuse it for another evaluation strategy, while reusing those components as much as possible. --- hcl2template/parser.go | 7 +- hcl2template/types.packer_config.go | 67 +----------- hcl2template/types.variables.go | 148 +++++++++++++++++++++++++++ hcl2template/types.variables_test.go | 1 - 4 files changed, 154 insertions(+), 69 deletions(-) diff --git a/hcl2template/parser.go b/hcl2template/parser.go index d7c57ef1907..261af644bd0 100644 --- a/hcl2template/parser.go +++ b/hcl2template/parser.go @@ -428,6 +428,7 @@ func (cfg *PackerConfig) decodeLocalBlock(block *hcl.Block) hcl.Diagnostics { if def, ok := content.Attributes["expression"]; ok { l.Expr = def.Expr } + l.getDependencies() cfg.LocalBlocks = append(cfg.LocalBlocks, l) @@ -438,10 +439,12 @@ func (cfg *PackerConfig) decodeLocalsBlock(block *hcl.Block) hcl.Diagnostics { attrs, diags := block.Body.JustAttributes() for name, attr := range attrs { - cfg.LocalBlocks = append(cfg.LocalBlocks, &LocalBlock{ + l := &LocalBlock{ Name: name, Expr: attr.Expr, - }) + } + l.getDependencies() + cfg.LocalBlocks = append(cfg.LocalBlocks, l) } return diags diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 6bb965785fd..65934203f86 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -177,49 +177,6 @@ func (c *PackerConfig) decodeInputVariables(f *hcl.File) hcl.Diagnostics { return diags } -func (c *PackerConfig) evaluateAllLocalVariables(locals []*LocalBlock) hcl.Diagnostics { - var diags hcl.Diagnostics - - for _, local := range locals { - diags = append(diags, c.evaluateLocalVariable(local)...) - } - - return diags -} - -func (c *PackerConfig) evaluateLocalVariables(locals []*LocalBlock) hcl.Diagnostics { - var diags hcl.Diagnostics - - if len(locals) == 0 { - return diags - } - - if c.LocalVariables == nil { - c.LocalVariables = Variables{} - } - - for foundSomething := true; foundSomething; { - foundSomething = false - for i := 0; i < len(locals); { - local := locals[i] - moreDiags := c.evaluateLocalVariable(local) - if moreDiags.HasErrors() { - i++ - continue - } - foundSomething = true - locals = append(locals[:i], locals[i+1:]...) - } - } - - if len(locals) != 0 { - // get errors from remaining variables - return c.evaluateAllLocalVariables(locals) - } - - return diags -} - func checkForDuplicateLocalDefinition(locals []*LocalBlock) hcl.Diagnostics { var diags hcl.Diagnostics @@ -241,28 +198,6 @@ func checkForDuplicateLocalDefinition(locals []*LocalBlock) hcl.Diagnostics { return diags } -func (c *PackerConfig) evaluateLocalVariable(local *LocalBlock) hcl.Diagnostics { - var diags hcl.Diagnostics - - value, moreDiags := local.Expr.Value(c.EvalContext(LocalContext, nil)) - diags = append(diags, moreDiags...) - if moreDiags.HasErrors() { - return diags - } - c.LocalVariables[local.Name] = &Variable{ - Name: local.Name, - Sensitive: local.Sensitive, - Values: []VariableAssignment{{ - Value: value, - Expr: local.Expr, - From: "default", - }}, - Type: value.Type(), - } - - return diags -} - // getCoreBuildProvisioners takes a list of provisioner block, starts according // provisioners and sends parsed HCL2 over to it. func (cfg *PackerConfig) getCoreBuildProvisioners(source SourceUseBlock, blocks []*ProvisionerBlock, ectx *hcl.EvalContext) ([]packer.CoreBuildProvisioner, hcl.Diagnostics) { @@ -684,7 +619,7 @@ func (cfg *PackerConfig) Initialize(opts packer.InitializeOptions) hcl.Diagnosti diags = append(diags, cfg.LocalVariables.ValidateValues()...) diags = append(diags, cfg.executeDatasources(opts.SkipDatasourcesExecution)...) diags = append(diags, checkForDuplicateLocalDefinition(cfg.LocalBlocks)...) - diags = append(diags, cfg.evaluateLocalVariables(cfg.LocalBlocks)...) + diags = append(diags, cfg.evaluateLocalVariables()...) filterVarsFromLogs(cfg.InputVariables) filterVarsFromLogs(cfg.LocalVariables) diff --git a/hcl2template/types.variables.go b/hcl2template/types.variables.go index f45aa72913c..44dc51dc773 100644 --- a/hcl2template/types.variables.go +++ b/hcl2template/types.variables.go @@ -30,6 +30,154 @@ type LocalBlock struct { // When Sensitive is set to true Packer will try its best to hide/obfuscate // the variable from the output stream. By replacing the text. Sensitive bool + // dependencies is the list of dependencies for the variable. + // + // We only focus on other local variables, since with the current + // scheduling model, datasources are necessarily all executed at the + // time we attempt to evaluate the value of the expression + dependencies []string + // evaluated keeps track of whether the variable has been evaluated or not + // + // This is tracked as an argument here since otherwise we don't + // necessarily know with certainty if the variable was evaluated or not, as + // its value gets added to config.LocalVariables only if it succeeded. + evaluated bool +} + +func (local *LocalBlock) getDependencies() { + var dependencies []string + + // In borderline cases (read invalid template), the expression may not + // be part of the parsed HCL template. While this is invalid, and caught + // during validation, the validation happens later in the process, so + // calling this on a local block without an expression crashes if we + // continue here. + // To avoid that, we simply return immediately if there's no expression + // in the block. + expr := local.Expr + if expr == nil { + return + } + + // Note: when looking at the expressions, we only need to care about + // attributes, as HCL2 expressions are not allowed in a block's labels. + locals := filterTraversalsByRootType(expr.Variables(), "local") + for _, local := range locals { + // If for some reason one reference is incomplete, trying to + // access the next step in the traversal will crash after. + // + // To avoid this problem, we ignore local variable references + // without at least 2 parts in the traversal. + if len(local) < 2 { + continue + } + dependencies = append(dependencies, local[1].(hcl.TraverseAttr).Name) + } + + local.dependencies = dependencies +} + +const varNotReadyForEval = "Local variable not ready for evaluation" + +func (local *LocalBlock) Evaluate(config *PackerConfig) hcl.Diagnostics { + // No need to re-evaluate if already done + if local.evaluated { + return nil + } + + var diags hcl.Diagnostics + + // Is dependencies have not been evaluated yet, we don't need to try yet + // as the expression will error when we attmpt to. + for _, dep := range local.dependencies { + // LocalVariables contains the list + if config.LocalVariables[dep] == nil { + return append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: varNotReadyForEval, + Detail: "Local variable local.%s is not ready yet to be evaluated as its dependencies are not ready yet.", + }) + } + } + + value, moreDiags := local.Expr.Value(config.EvalContext(LocalContext, nil)) + diags = append(diags, moreDiags...) + // We always set this to true after attempting to evaluate the value + // as otherwise we don't have a good way to guess if the block needs + // ulterior evaluation. + local.evaluated = true + + if moreDiags.HasErrors() { + return diags + } + + if config.LocalVariables == nil { + config.LocalVariables = Variables{} + } + + config.LocalVariables[local.Name] = &Variable{ + Name: local.Name, + Sensitive: local.Sensitive, + Values: []VariableAssignment{{ + Value: value, + Expr: local.Expr, + From: "default", + }}, + Type: value.Type(), + } + + return diags +} + +func (c PackerConfig) localVariablesEvaluationDone() bool { + for _, loc := range c.LocalBlocks { + if !loc.evaluated { + return false + } + } + + return true +} + +func (c *PackerConfig) evaluateLocalVariables() hcl.Diagnostics { + if len(c.LocalBlocks) == 0 { + return nil + } + + // If we're done evaluating variables, we can leave immediately + if c.localVariablesEvaluationDone() { + return nil + } + + var diags hcl.Diagnostics + + found := false + for _, loc := range c.LocalBlocks { + if loc.evaluated { + continue + } + + evalDiags := loc.Evaluate(c) + // If there's a not ready for eval error, we continue iterating + // on the other variable blocks, until we reach a point where + // we can evaluate it. + if evalDiags.HasErrors() && + evalDiags[0].Summary == varNotReadyForEval { + continue + } + + found = true + } + + if !found { + return append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Failed to evaluate local variables", + Detail: "Packer couldn't evaluate any more variables, and some are pending. This likely means your configuration has a dependency cycle in the local variables, that needs to be corrected before building the template.", + }) + } + + return diags.Extend(c.evaluateLocalVariables()) } // VariableAssignment represents a way a variable was set: the expression diff --git a/hcl2template/types.variables_test.go b/hcl2template/types.variables_test.go index 37bfc779f0d..2c18c870189 100644 --- a/hcl2template/types.variables_test.go +++ b/hcl2template/types.variables_test.go @@ -396,7 +396,6 @@ func TestParse_variables(t *testing.T) { &PackerConfig{ CorePackerVersionString: lockedVersion, Basedir: filepath.Join("testdata", "variables"), - LocalVariables: Variables{}, }, true, true, []packersdk.Build{}, From a07bd93f5f62aaf226ea47876732594e4eb87964 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 6 Sep 2023 11:07:35 -0400 Subject: [PATCH 12/28] hcl2template: remove value validation for locals Local variables can't have a validation block in their definition, so this step in not useful and should be removed. Besides, since the validation was done on the local variables before evaluation, it did nothing at all, as the PackerConfig.LocalVariables collection gets populated during evaluation, so this is essentially a no-op, and can be safely removed. --- hcl2template/types.packer_config.go | 1 - 1 file changed, 1 deletion(-) diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 65934203f86..412fff85aff 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -616,7 +616,6 @@ func (p *PackerConfig) InspectConfig(opts packer.InspectConfigOptions) int { func (cfg *PackerConfig) Initialize(opts packer.InitializeOptions) hcl.Diagnostics { diags := cfg.InputVariables.ValidateValues() - diags = append(diags, cfg.LocalVariables.ValidateValues()...) diags = append(diags, cfg.executeDatasources(opts.SkipDatasourcesExecution)...) diags = append(diags, checkForDuplicateLocalDefinition(cfg.LocalBlocks)...) diags = append(diags, cfg.evaluateLocalVariables()...) From 58b078f3d3c76cd3a38cf085d40953166425eac4 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 6 Sep 2023 11:59:15 -0400 Subject: [PATCH 13/28] hcl2template: use diags.Extend instead of append Since hcl.Diagnostics supports extending it through the `Extend' method, we might as well use it instead of manually appending a series of diagnostics to it. --- hcl2template/types.packer_config.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 412fff85aff..4892d4cfab2 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -616,14 +616,14 @@ func (p *PackerConfig) InspectConfig(opts packer.InspectConfigOptions) int { func (cfg *PackerConfig) Initialize(opts packer.InitializeOptions) hcl.Diagnostics { diags := cfg.InputVariables.ValidateValues() - diags = append(diags, cfg.executeDatasources(opts.SkipDatasourcesExecution)...) - diags = append(diags, checkForDuplicateLocalDefinition(cfg.LocalBlocks)...) - diags = append(diags, cfg.evaluateLocalVariables()...) + diags = diags.Extend(checkForDuplicateLocalDefinition(cfg.LocalBlocks)) + diags = diags.Extend(cfg.executeDatasources(opts.SkipDatasourcesExecution)) + diags = diags.Extend(cfg.evaluateLocalVariables()) filterVarsFromLogs(cfg.InputVariables) filterVarsFromLogs(cfg.LocalVariables) - diags = append(diags, cfg.initializeBlocks()...) + diags = diags.Extend(cfg.initializeBlocks()) return diags } From 996c4a851f94316ce696c63826892ee5ae674237 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Fri, 8 Sep 2023 14:17:43 -0400 Subject: [PATCH 14/28] hcl2template: make consts and functions public Since these constants and functions will be later useful for the extraction of the sequential logic in a scheduler, we change the visibility of those now. --- hcl2template/types.datasource.go | 14 +++++++------- hcl2template/types.variables.go | 10 +++++++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/hcl2template/types.datasource.go b/hcl2template/types.datasource.go index 126582be38d..60706d28d18 100644 --- a/hcl2template/types.datasource.go +++ b/hcl2template/types.datasource.go @@ -58,13 +58,13 @@ func (ds *DatasourceBlock) getDependencies() { ds.dependencies = dependencies } -const notReadyDataSourceError = "Dependencies not ready" +const NotReadyDataSourceError = "Dependencies not ready" // executed returns whether or not the datasource was executed // // Having a non-empty cty.Value object means this was filled-up after the // datasource has been executed, so this is what we use for this test. -func (ds DatasourceBlock) executed() bool { +func (ds DatasourceBlock) Executed() bool { return ds.value != cty.Value{} } @@ -89,7 +89,7 @@ func (ds *DatasourceBlock) Execute(cfg *PackerConfig, skipExecution bool) hcl.Di continue } - if !dep.executed() { + if !dep.Executed() { ok = false } } @@ -97,7 +97,7 @@ func (ds *DatasourceBlock) Execute(cfg *PackerConfig, skipExecution bool) hcl.Di if !ok { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: notReadyDataSourceError, + Summary: NotReadyDataSourceError, Detail: "At least one dependency for the datasource is not executed already", Subject: &ds.block.DefRange, }) @@ -234,7 +234,7 @@ func (cfg *PackerConfig) startDatasource(ds DatasourceBlock) (packersdk.Datasour // datasourcesDone checks whether all the datasources have been executed or not func (cfg *PackerConfig) datasourcesDone() bool { for _, ds := range cfg.Datasources { - if !ds.executed() { + if !ds.Executed() { return false } } @@ -253,13 +253,13 @@ func (cfg *PackerConfig) executeDatasources(skipExecution bool) hcl.Diagnostics foundSomething := false outerDSEval: for _, ds := range cfg.Datasources { - if ds.executed() { + if ds.Executed() { continue } diags := ds.Execute(cfg, skipExecution) for _, diag := range diags { - if diag.Summary == notReadyDataSourceError { + if diag.Summary == NotReadyDataSourceError { // If we have a not ready error in the // datasource list, we should attempt to run the // rest, and eventually settle if we cannot move diff --git a/hcl2template/types.variables.go b/hcl2template/types.variables.go index 44dc51dc773..26c874c7ff1 100644 --- a/hcl2template/types.variables.go +++ b/hcl2template/types.variables.go @@ -77,7 +77,11 @@ func (local *LocalBlock) getDependencies() { local.dependencies = dependencies } -const varNotReadyForEval = "Local variable not ready for evaluation" +const VarNotReadyForEval = "Local variable not ready for evaluation" + +func (local LocalBlock) Evaluated() bool { + return local.evaluated +} func (local *LocalBlock) Evaluate(config *PackerConfig) hcl.Diagnostics { // No need to re-evaluate if already done @@ -94,7 +98,7 @@ func (local *LocalBlock) Evaluate(config *PackerConfig) hcl.Diagnostics { if config.LocalVariables[dep] == nil { return append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: varNotReadyForEval, + Summary: VarNotReadyForEval, Detail: "Local variable local.%s is not ready yet to be evaluated as its dependencies are not ready yet.", }) } @@ -162,7 +166,7 @@ func (c *PackerConfig) evaluateLocalVariables() hcl.Diagnostics { // on the other variable blocks, until we reach a point where // we can evaluate it. if evalDiags.HasErrors() && - evalDiags[0].Summary == varNotReadyForEval { + evalDiags[0].Summary == VarNotReadyForEval { continue } From dfbea7f0fffe40d58ab61676d0a124ec105f1823 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Mon, 11 Sep 2023 11:06:11 -0400 Subject: [PATCH 15/28] hcl2template: breakdown CoreBuild creation When processing builds, we convert build blocks/sources to a series of CoreBuild, which contains the sequence of provisioners and post-processors to run, and the builder to use to boot the instance and create the image from. This process of converting builds used to be completely sequential, and orchestrated by the `GetBuilds' function. This gets broken down into a `ToCoreBuild' function on the build block, whose function is to lift and filter-out (based on --only/--except) the core builds from a valid build block, and this gets invoked from `GetBuilds'. This breakdown is useful as a preparation to the DAG's introduction to the code later on. --- hcl2template/types.build.go | 106 +++++++++++++++++ hcl2template/types.packer_config.go | 170 ++++------------------------ 2 files changed, 130 insertions(+), 146 deletions(-) diff --git a/hcl2template/types.build.go b/hcl2template/types.build.go index bd8778fc0cd..de3304b0fcb 100644 --- a/hcl2template/types.build.go +++ b/hcl2template/types.build.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/hcl/v2/ext/dynblock" "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/hashicorp/packer/packer" "github.com/zclconf/go-cty/cty" ) @@ -264,3 +265,108 @@ func (build *BuildBlock) finalizeDecode(cfg *PackerConfig) hcl.Diagnostics { return diags } + +// ToCoreBuilds extracts the core builds from a build block. +// +// Since build blocks can have multiple sources, it can lead to multiple builds +// for each build block. +func (build BuildBlock) ToCoreBuilds(cfg *PackerConfig) ([]*packer.CoreBuild, hcl.Diagnostics) { + var res []*packer.CoreBuild + var diags hcl.Diagnostics + + for _, srcUsage := range build.Sources { + _, found := cfg.Sources[srcUsage.SourceRef] + if !found { + diags = append(diags, &hcl.Diagnostic{ + Summary: fmt.Sprintf("Unknown %s %s", sourceLabel, srcUsage.String()), + Subject: build.HCL2Ref.DefRange.Ptr(), + Severity: hcl.DiagError, + Detail: fmt.Sprintf("Known: %v", cfg.Sources), + }) + continue + } + + pcb := &packer.CoreBuild{ + BuildName: build.Name, + Type: srcUsage.String(), + } + if !cfg.keepBuild(pcb) { + continue + } + + builder, moreDiags, generatedVars := cfg.startBuilder(srcUsage, cfg.EvalContext(BuildContext, nil)) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + continue + } + + decoded, _ := decodeHCL2Spec(srcUsage.Body, cfg.EvalContext(BuildContext, nil), builder) + pcb.HCLConfig = decoded + + // If the builder has provided a list of to-be-generated variables that + // should be made accessible to provisioners, pass that list into + // the provisioner prepare() so that the provisioner can appropriately + // validate user input against what will become available. Otherwise, + // only pass the default variables, using the basic placeholder data. + unknownBuildValues := map[string]cty.Value{} + for _, k := range append(packer.BuilderDataCommonKeys, generatedVars...) { + unknownBuildValues[k] = cty.StringVal("") + } + unknownBuildValues["name"] = cty.StringVal(build.Name) + + variables := map[string]cty.Value{ + sourcesAccessor: cty.ObjectVal(srcUsage.ctyValues()), + buildAccessor: cty.ObjectVal(unknownBuildValues), + } + + provisioners, moreDiags := cfg.getCoreBuildProvisioners(srcUsage, build.ProvisionerBlocks, cfg.EvalContext(BuildContext, variables)) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + continue + } + pps, moreDiags := cfg.getCoreBuildPostProcessors(srcUsage, build.PostProcessorsLists, cfg.EvalContext(BuildContext, variables)) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + continue + } + + if build.ErrorCleanupProvisionerBlock != nil && + !build.ErrorCleanupProvisionerBlock.OnlyExcept.Skip(srcUsage.String()) { + errorCleanupProv, moreDiags := cfg.getCoreBuildProvisioner(srcUsage, build.ErrorCleanupProvisionerBlock, cfg.EvalContext(BuildContext, variables)) + diags = append(diags, moreDiags...) + if moreDiags.HasErrors() { + continue + } + pcb.CleanupProvisioner = errorCleanupProv + } + + pcb.Builder = builder + pcb.Provisioners = provisioners + pcb.PostProcessors = pps + + res = append(res, pcb) + } + + return res, diags +} + +func (cfg *PackerConfig) keepBuild(cb *packer.CoreBuild) bool { + keep := false + for _, onlyGlob := range cfg.only { + if onlyGlob.Match(cb.Name()) { + keep = true + break + } + } + if !keep && len(cfg.only) > 0 { + return false + } + + for _, exceptGlob := range cfg.except { + if exceptGlob.Match(cb.Name()) { + return false + } + } + + return true +} diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 4892d4cfab2..caf34d1086a 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -257,7 +257,7 @@ func (cfg *PackerConfig) getCoreBuildProvisioner(source SourceUseBlock, pb *Prov // getCoreBuildProvisioners takes a list of post processor block, starts // according provisioners and sends parsed HCL2 over to it. -func (cfg *PackerConfig) getCoreBuildPostProcessors(source SourceUseBlock, blocksList [][]*PostProcessorBlock, ectx *hcl.EvalContext, exceptMatches *int) ([][]packer.CoreBuildPostProcessor, hcl.Diagnostics) { +func (cfg *PackerConfig) getCoreBuildPostProcessors(source SourceUseBlock, blocksList [][]*PostProcessorBlock, ectx *hcl.EvalContext) ([][]packer.CoreBuildPostProcessor, hcl.Diagnostics) { var diags hcl.Diagnostics res := [][]packer.CoreBuildPostProcessor{} for _, blocks := range blocksList { @@ -276,7 +276,6 @@ func (cfg *PackerConfig) getCoreBuildPostProcessors(source SourceUseBlock, block for _, exceptGlob := range cfg.except { if exceptGlob.Match(name) { exclude = true - *exceptMatches = *exceptMatches + 1 break } } @@ -308,175 +307,54 @@ func (cfg *PackerConfig) getCoreBuildPostProcessors(source SourceUseBlock, block return res, diags } -// GetBuilds returns a list of packer Build based on the HCL2 parsed build -// blocks. All Builders, Provisioners and Post Processors will be started and -// configured. func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Build, hcl.Diagnostics) { - res := []packersdk.Build{} + var allBuilds []packersdk.Build var diags hcl.Diagnostics - possibleBuildNames := []string{} - - cfg.debug = opts.Debug - cfg.force = opts.Force - cfg.onError = opts.OnError if len(cfg.Builds) == 0 { - return res, append(diags, &hcl.Diagnostic{ + return nil, append(diags, &hcl.Diagnostic{ Summary: "Missing build block", Detail: "A build block with one or more sources is required for executing a build.", Severity: hcl.DiagError, }) } - for _, build := range cfg.Builds { - for _, srcUsage := range build.Sources { - src, found := cfg.Sources[srcUsage.SourceRef] - if !found { - diags = append(diags, &hcl.Diagnostic{ - Summary: "Unknown " + sourceLabel + " " + srcUsage.String(), - Subject: build.HCL2Ref.DefRange.Ptr(), - Severity: hcl.DiagError, - Detail: fmt.Sprintf("Known: %v", cfg.Sources), - }) - continue - } - - pcb := &packer.CoreBuild{ - BuildName: build.Name, - Type: srcUsage.String(), - } - - pcb.SetDebug(cfg.debug) - pcb.SetForce(cfg.force) - pcb.SetOnError(cfg.onError) - - // Apply the -only and -except command-line options to exclude matching builds. - buildName := pcb.Name() - possibleBuildNames = append(possibleBuildNames, buildName) - // -only - if len(opts.Only) > 0 { - onlyGlobs, diags := convertFilterOption(opts.Only, "only") - if diags.HasErrors() { - return nil, diags - } - cfg.only = onlyGlobs - include := false - for _, onlyGlob := range onlyGlobs { - if onlyGlob.Match(buildName) { - include = true - break - } - } - if !include { - continue - } - opts.OnlyMatches++ - } - - // -except - if len(opts.Except) > 0 { - exceptGlobs, diags := convertFilterOption(opts.Except, "except") - if diags.HasErrors() { - return nil, diags - } - cfg.except = exceptGlobs - exclude := false - for _, exceptGlob := range exceptGlobs { - if exceptGlob.Match(buildName) { - exclude = true - break - } - } - if exclude { - opts.ExceptMatches++ - continue - } - } - - builder, moreDiags, generatedVars := cfg.startBuilder(srcUsage, cfg.EvalContext(BuildContext, nil)) - diags = append(diags, moreDiags...) - if moreDiags.HasErrors() { - continue - } - - decoded, _ := decodeHCL2Spec(srcUsage.Body, cfg.EvalContext(BuildContext, nil), builder) - pcb.HCLConfig = decoded - - // If the builder has provided a list of to-be-generated variables that - // should be made accessible to provisioners, pass that list into - // the provisioner prepare() so that the provisioner can appropriately - // validate user input against what will become available. Otherwise, - // only pass the default variables, using the basic placeholder data. - unknownBuildValues := map[string]cty.Value{} - for _, k := range append(packer.BuilderDataCommonKeys, generatedVars...) { - unknownBuildValues[k] = cty.StringVal("") - } - unknownBuildValues["name"] = cty.StringVal(build.Name) - - variables := map[string]cty.Value{ - sourcesAccessor: cty.ObjectVal(srcUsage.ctyValues()), - buildAccessor: cty.ObjectVal(unknownBuildValues), - } + var convertDiags hcl.Diagnostics + cfg.debug = opts.Debug + cfg.except, convertDiags = convertFilterOption(opts.Except, "except") + diags = diags.Extend(convertDiags) + cfg.only, convertDiags = convertFilterOption(opts.Only, "only") + diags = diags.Extend(convertDiags) + cfg.force = opts.Force + cfg.onError = opts.OnError - provisioners, moreDiags := cfg.getCoreBuildProvisioners(srcUsage, build.ProvisionerBlocks, cfg.EvalContext(BuildContext, variables)) - diags = append(diags, moreDiags...) - if moreDiags.HasErrors() { - continue - } - pps, moreDiags := cfg.getCoreBuildPostProcessors(srcUsage, build.PostProcessorsLists, cfg.EvalContext(BuildContext, variables), &opts.ExceptMatches) - diags = append(diags, moreDiags...) - if moreDiags.HasErrors() { - continue - } + for _, build := range cfg.Builds { + cbs, cbDiags := build.ToCoreBuilds(cfg) + diags = diags.Extend(cbDiags) - if build.ErrorCleanupProvisionerBlock != nil && - !build.ErrorCleanupProvisionerBlock.OnlyExcept.Skip(srcUsage.String()) { - errorCleanupProv, moreDiags := cfg.getCoreBuildProvisioner(srcUsage, build.ErrorCleanupProvisionerBlock, cfg.EvalContext(BuildContext, variables)) - diags = append(diags, moreDiags...) - if moreDiags.HasErrors() { - continue - } - pcb.CleanupProvisioner = errorCleanupProv - } + for _, cb := range cbs { + cb.SetDebug(opts.Debug) + cb.SetForce(opts.Force) + cb.SetOnError(opts.OnError) - pcb.Builder = builder - pcb.Provisioners = provisioners - pcb.PostProcessors = pps - pcb.Prepared = true + cb.Prepared = true // Prepare just sets the "prepareCalled" flag on CoreBuild, since // we did all the prep here. - _, err := pcb.Prepare() + _, err := cb.Prepare() if err != nil { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: fmt.Sprintf("Preparing packer core build %s failed", src.Ref().String()), + Summary: fmt.Sprintf("Preparing packer core build %s failed", cb.Name()), Detail: err.Error(), - Subject: build.HCL2Ref.DefRange.Ptr(), }) - continue } - res = append(res, pcb) + allBuilds = append(allBuilds, cb) } } - if len(opts.Only) > opts.OnlyMatches { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagWarning, - Summary: "an 'only' option was passed, but not all matches were found for the given build.", - Detail: fmt.Sprintf("Possible build names: %v.\n"+ - "These could also be matched with a glob pattern like: 'happycloud.*'", possibleBuildNames), - }) - } - if len(opts.Except) > opts.ExceptMatches { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagWarning, - Summary: "an 'except' option was passed, but did not match any build.", - Detail: fmt.Sprintf("Possible build names: %v.\n"+ - "These could also be matched with a glob pattern like: 'happycloud.*'", possibleBuildNames), - }) - } - return res, diags + + return allBuilds, diags } var PackerConsoleHelp = strings.TrimSpace(` From c9c1898bf7ef7807da33f33f340c14b256e16914 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Mon, 11 Sep 2023 15:47:31 -0400 Subject: [PATCH 16/28] hcl2template: track unused only/except options When a user specifies a --only/--except option in their command-line, and it turns-out to be unused, Packer signals it with a Warning diagnostic. This diagnostic, in previous versions of Packer, would only state that one option did not match anything, without stating which option did not match a build or post-processor. This commit tracks the usage of each option so we can report later which of those options was not used in this warning diagnostic. --- hcl2template/types.build.go | 6 ++- hcl2template/types.packer_config.go | 71 +++++++++++++++++++++++++++-- hcl2template/utils.go | 18 ++++++-- 3 files changed, 87 insertions(+), 8 deletions(-) diff --git a/hcl2template/types.build.go b/hcl2template/types.build.go index de3304b0fcb..b0a3a731b19 100644 --- a/hcl2template/types.build.go +++ b/hcl2template/types.build.go @@ -352,9 +352,10 @@ func (build BuildBlock) ToCoreBuilds(cfg *PackerConfig) ([]*packer.CoreBuild, hc func (cfg *PackerConfig) keepBuild(cb *packer.CoreBuild) bool { keep := false - for _, onlyGlob := range cfg.only { + for p, onlyGlob := range cfg.only { if onlyGlob.Match(cb.Name()) { keep = true + cfg.onlyUses[p] = true break } } @@ -362,8 +363,9 @@ func (cfg *PackerConfig) keepBuild(cb *packer.CoreBuild) bool { return false } - for _, exceptGlob := range cfg.except { + for p, exceptGlob := range cfg.except { if exceptGlob.Match(cb.Name()) { + cfg.exceptUses[p] = true return false } } diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index caf34d1086a..76161928508 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -61,11 +61,18 @@ type PackerConfig struct { files []*hcl.File // Fields passed as command line flags - except []glob.Glob - only []glob.Glob force bool debug bool onError string + + // except/only are options to filter builds and post-processors + // + // if an option is specified but unused, we print a warning, stating + // which option is specified, but unused + except map[string]glob.Glob + only map[string]glob.Glob + exceptUses map[string]bool + onlyUses map[string]bool } type ValidationOptions struct { @@ -273,9 +280,10 @@ func (cfg *PackerConfig) getCoreBuildPostProcessors(source SourceUseBlock, block } // -except exclude := false - for _, exceptGlob := range cfg.except { + for p, exceptGlob := range cfg.except { if exceptGlob.Match(name) { exclude = true + cfg.exceptUses[p] = true break } } @@ -307,6 +315,50 @@ func (cfg *PackerConfig) getCoreBuildPostProcessors(source SourceUseBlock, block return res, diags } +func (cfg *PackerConfig) prepareGlobUsage() { + if cfg.onlyUses == nil { + cfg.onlyUses = map[string]bool{} + } + for only := range cfg.only { + cfg.onlyUses[only] = false + } + + if cfg.exceptUses == nil { + cfg.exceptUses = map[string]bool{} + } + for except := range cfg.except { + cfg.exceptUses[except] = false + } +} + +func (cfg *PackerConfig) reportUnusedFilters(buildNames []string) hcl.Diagnostics { + var diags hcl.Diagnostics + + onlyUnused := getUnusedFilters(cfg.onlyUses) + if onlyUnused != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Unused --only filters specified", + Detail: fmt.Sprintf( + "Some --only options were specified in the command-line, but weren't used to get builds from the template: %v\n"+ + "List of available builds: %v", onlyUnused, buildNames), + }) + } + + exceptUnused := getUnusedFilters(cfg.exceptUses) + if exceptUnused != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Unused --except filters specified", + Detail: fmt.Sprintf( + "Some --except options were specified in the command-line, but weren't used to filter out builds or post-processors from the template: %v\n"+ + "List of available builds: %v", exceptUnused, buildNames), + }) + } + + return diags +} + func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Build, hcl.Diagnostics) { var allBuilds []packersdk.Build var diags hcl.Diagnostics @@ -328,6 +380,8 @@ func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Bu cfg.force = opts.Force cfg.onError = opts.OnError + cfg.prepareGlobUsage() + for _, build := range cfg.Builds { cbs, cbDiags := build.ToCoreBuilds(cfg) diags = diags.Extend(cbDiags) @@ -354,6 +408,17 @@ func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Bu } } + buildNames := []string{} + for _, cb := range allBuilds { + buildNames = append(buildNames, cb.Name()) + } + + diags = diags.Extend( + cfg.reportUnusedFilters( + buildNames, + ), + ) + return allBuilds, diags } diff --git a/hcl2template/utils.go b/hcl2template/utils.go index 6c0c1af93b4..f4cdeb8e59e 100644 --- a/hcl2template/utils.go +++ b/hcl2template/utils.go @@ -101,8 +101,8 @@ func GetHCL2Files(filename, hclSuffix, jsonSuffix string) (hclFiles, jsonFiles [ } // Convert -only and -except globs to glob.Glob instances. -func convertFilterOption(patterns []string, optionName string) ([]glob.Glob, hcl.Diagnostics) { - var globs []glob.Glob +func convertFilterOption(patterns []string, optionName string) (map[string]glob.Glob, hcl.Diagnostics) { + globs := map[string]glob.Glob{} var diags hcl.Diagnostics for _, pattern := range patterns { @@ -113,12 +113,24 @@ func convertFilterOption(patterns []string, optionName string) ([]glob.Glob, hcl Severity: hcl.DiagError, }) } - globs = append(globs, g) + globs[pattern] = g } return globs, diags } +func getUnusedFilters(filterUsed map[string]bool) []string { + var unused []string + + for p, ok := range filterUsed { + if !ok { + unused = append(unused, p) + } + } + + return unused +} + func PrintableCtyValue(v cty.Value) string { if !v.IsWhollyKnown() { return "" From d4ce57d9a781a4af0cd5c8733335499085559bae Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Mon, 11 Sep 2023 16:24:04 -0400 Subject: [PATCH 17/28] hcl2template: make config's cmd arguments public Since those arguments will need to be updated from outside the hcl2template package, we change their visibility from private to public. --- hcl2template/types.build.go | 10 ++-- hcl2template/types.build.post-processor.go | 6 +-- hcl2template/types.build.provisioners.go | 6 +-- hcl2template/types.packer_config.go | 56 +++++++++++----------- hcl2template/types.packer_config_test.go | 2 +- hcl2template/types.source.go | 6 +-- hcl2template/utils.go | 2 +- 7 files changed, 44 insertions(+), 44 deletions(-) diff --git a/hcl2template/types.build.go b/hcl2template/types.build.go index b0a3a731b19..f4f93cd753d 100644 --- a/hcl2template/types.build.go +++ b/hcl2template/types.build.go @@ -352,20 +352,20 @@ func (build BuildBlock) ToCoreBuilds(cfg *PackerConfig) ([]*packer.CoreBuild, hc func (cfg *PackerConfig) keepBuild(cb *packer.CoreBuild) bool { keep := false - for p, onlyGlob := range cfg.only { + for p, onlyGlob := range cfg.Only { if onlyGlob.Match(cb.Name()) { keep = true - cfg.onlyUses[p] = true + cfg.OnlyUses[p] = true break } } - if !keep && len(cfg.only) > 0 { + if !keep && len(cfg.Only) > 0 { return false } - for p, exceptGlob := range cfg.except { + for p, exceptGlob := range cfg.Except { if exceptGlob.Match(cb.Name()) { - cfg.exceptUses[p] = true + cfg.ExceptUses[p] = true return false } } diff --git a/hcl2template/types.build.post-processor.go b/hcl2template/types.build.post-processor.go index 1cf943b0bd6..eeb08c1ec0a 100644 --- a/hcl2template/types.build.post-processor.go +++ b/hcl2template/types.build.post-processor.go @@ -73,9 +73,9 @@ func (cfg *PackerConfig) startPostProcessor(source SourceUseBlock, pp *PostProce builderVars := source.builderVariables() builderVars["packer_core_version"] = cfg.CorePackerVersionString - builderVars["packer_debug"] = strconv.FormatBool(cfg.debug) - builderVars["packer_force"] = strconv.FormatBool(cfg.force) - builderVars["packer_on_error"] = cfg.onError + builderVars["packer_debug"] = strconv.FormatBool(cfg.Debug) + builderVars["packer_force"] = strconv.FormatBool(cfg.Force) + builderVars["packer_on_error"] = cfg.OnError hclPostProcessor := &HCL2PostProcessor{ PostProcessor: postProcessor, diff --git a/hcl2template/types.build.provisioners.go b/hcl2template/types.build.provisioners.go index dd6fb71ae7f..de99ba7cafa 100644 --- a/hcl2template/types.build.provisioners.go +++ b/hcl2template/types.build.provisioners.go @@ -182,9 +182,9 @@ func (cfg *PackerConfig) startProvisioner(source SourceUseBlock, pb *Provisioner builderVars := source.builderVariables() builderVars["packer_core_version"] = cfg.CorePackerVersionString - builderVars["packer_debug"] = strconv.FormatBool(cfg.debug) - builderVars["packer_force"] = strconv.FormatBool(cfg.force) - builderVars["packer_on_error"] = cfg.onError + builderVars["packer_debug"] = strconv.FormatBool(cfg.Debug) + builderVars["packer_force"] = strconv.FormatBool(cfg.Force) + builderVars["packer_on_error"] = cfg.OnError hclProvisioner := &HCL2Provisioner{ Provisioner: provisioner, diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 76161928508..6d2df157f49 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -61,18 +61,18 @@ type PackerConfig struct { files []*hcl.File // Fields passed as command line flags - force bool - debug bool - onError string + Force bool + Debug bool + OnError string // except/only are options to filter builds and post-processors // // if an option is specified but unused, we print a warning, stating // which option is specified, but unused - except map[string]glob.Glob - only map[string]glob.Glob - exceptUses map[string]bool - onlyUses map[string]bool + Except map[string]glob.Glob + Only map[string]glob.Glob + ExceptUses map[string]bool + OnlyUses map[string]bool } type ValidationOptions struct { @@ -280,10 +280,10 @@ func (cfg *PackerConfig) getCoreBuildPostProcessors(source SourceUseBlock, block } // -except exclude := false - for p, exceptGlob := range cfg.except { + for p, exceptGlob := range cfg.Except { if exceptGlob.Match(name) { exclude = true - cfg.exceptUses[p] = true + cfg.ExceptUses[p] = true break } } @@ -315,26 +315,26 @@ func (cfg *PackerConfig) getCoreBuildPostProcessors(source SourceUseBlock, block return res, diags } -func (cfg *PackerConfig) prepareGlobUsage() { - if cfg.onlyUses == nil { - cfg.onlyUses = map[string]bool{} +func (cfg *PackerConfig) PrepareGlobUsage() { + if cfg.OnlyUses == nil { + cfg.OnlyUses = map[string]bool{} } - for only := range cfg.only { - cfg.onlyUses[only] = false + for only := range cfg.Only { + cfg.OnlyUses[only] = false } - if cfg.exceptUses == nil { - cfg.exceptUses = map[string]bool{} + if cfg.ExceptUses == nil { + cfg.ExceptUses = map[string]bool{} } - for except := range cfg.except { - cfg.exceptUses[except] = false + for except := range cfg.Except { + cfg.ExceptUses[except] = false } } -func (cfg *PackerConfig) reportUnusedFilters(buildNames []string) hcl.Diagnostics { +func (cfg *PackerConfig) ReportUnusedFilters(buildNames []string) hcl.Diagnostics { var diags hcl.Diagnostics - onlyUnused := getUnusedFilters(cfg.onlyUses) + onlyUnused := getUnusedFilters(cfg.OnlyUses) if onlyUnused != nil { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagWarning, @@ -345,7 +345,7 @@ func (cfg *PackerConfig) reportUnusedFilters(buildNames []string) hcl.Diagnostic }) } - exceptUnused := getUnusedFilters(cfg.exceptUses) + exceptUnused := getUnusedFilters(cfg.ExceptUses) if exceptUnused != nil { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagWarning, @@ -372,15 +372,15 @@ func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Bu } var convertDiags hcl.Diagnostics - cfg.debug = opts.Debug - cfg.except, convertDiags = convertFilterOption(opts.Except, "except") + cfg.Debug = opts.Debug + cfg.Except, convertDiags = ConvertFilterOption(opts.Except, "except") diags = diags.Extend(convertDiags) - cfg.only, convertDiags = convertFilterOption(opts.Only, "only") + cfg.Only, convertDiags = ConvertFilterOption(opts.Only, "only") diags = diags.Extend(convertDiags) - cfg.force = opts.Force - cfg.onError = opts.OnError + cfg.Force = opts.Force + cfg.OnError = opts.OnError - cfg.prepareGlobUsage() + cfg.PrepareGlobUsage() for _, build := range cfg.Builds { cbs, cbDiags := build.ToCoreBuilds(cfg) @@ -414,7 +414,7 @@ func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Bu } diags = diags.Extend( - cfg.reportUnusedFilters( + cfg.ReportUnusedFilters( buildNames, ), ) diff --git a/hcl2template/types.packer_config_test.go b/hcl2template/types.packer_config_test.go index ee65f7a1d43..158060cde67 100644 --- a/hcl2template/types.packer_config_test.go +++ b/hcl2template/types.packer_config_test.go @@ -436,7 +436,7 @@ func TestParser_ValidateFilterOption(t *testing.T) { for _, test := range tests { t.Run(test.pattern, func(t *testing.T) { - _, diags := convertFilterOption([]string{test.pattern}, "") + _, diags := ConvertFilterOption([]string{test.pattern}, "") if diags.HasErrors() && !test.expectError { t.Fatalf("Expected %s to parse as glob", test.pattern) } diff --git a/hcl2template/types.source.go b/hcl2template/types.source.go index 450c8fd12c8..79c4fbf3863 100644 --- a/hcl2template/types.source.go +++ b/hcl2template/types.source.go @@ -105,9 +105,9 @@ func (cfg *PackerConfig) startBuilder(source SourceUseBlock, ectx *hcl.EvalConte // easier to reason about. builderVars := source.builderVariables() builderVars["packer_core_version"] = cfg.CorePackerVersionString - builderVars["packer_debug"] = strconv.FormatBool(cfg.debug) - builderVars["packer_force"] = strconv.FormatBool(cfg.force) - builderVars["packer_on_error"] = cfg.onError + builderVars["packer_debug"] = strconv.FormatBool(cfg.Debug) + builderVars["packer_force"] = strconv.FormatBool(cfg.Force) + builderVars["packer_on_error"] = cfg.OnError generatedVars, warning, err := builder.Prepare(builderVars, decoded) moreDiags = warningErrorsToDiags(cfg.Sources[source.SourceRef].block, warning, err) diff --git a/hcl2template/utils.go b/hcl2template/utils.go index f4cdeb8e59e..70f419b4d3e 100644 --- a/hcl2template/utils.go +++ b/hcl2template/utils.go @@ -101,7 +101,7 @@ func GetHCL2Files(filename, hclSuffix, jsonSuffix string) (hclFiles, jsonFiles [ } // Convert -only and -except globs to glob.Glob instances. -func convertFilterOption(patterns []string, optionName string) (map[string]glob.Glob, hcl.Diagnostics) { +func ConvertFilterOption(patterns []string, optionName string) (map[string]glob.Glob, hcl.Diagnostics) { globs := map[string]glob.Glob{} var diags hcl.Diagnostics From 01651b19f70c786cdd040c2b7489d8f08b033bb9 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Mon, 11 Sep 2023 16:47:50 -0400 Subject: [PATCH 18/28] hcl2template: move initialization logic to builds Since the initialization code was embedded in the sequential logic that we'll be moving away from soon, we move that to the build block itself, so we're able to invoke it in any order later on. --- hcl2template/plugin.go | 91 +---------------------------------- hcl2template/types.build.go | 96 +++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 90 deletions(-) diff --git a/hcl2template/plugin.go b/hcl2template/plugin.go index 287ce8fa0a3..27911ca873e 100644 --- a/hcl2template/plugin.go +++ b/hcl2template/plugin.go @@ -11,7 +11,6 @@ import ( "strings" "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/packer-plugin-sdk/didyoumean" pluginsdk "github.com/hashicorp/packer-plugin-sdk/plugin" plugingetter "github.com/hashicorp/packer/packer/plugin-getter" ) @@ -128,95 +127,7 @@ func (cfg *PackerConfig) initializeBlocks() hcl.Diagnostics { var diags hcl.Diagnostics for _, build := range cfg.Builds { - // Since the build's contents may not have been dynamically - // expanded when we first loaded the config from the template - // file, we decode it now. - diags = append(diags, build.finalizeDecode(cfg)...) - if diags.HasErrors() { - continue - } - - for i := range build.Sources { - // here we grab a pointer to the source usage because we will set - // its body. - srcUsage := &(build.Sources[i]) - if !cfg.parser.PluginConfig.Builders.Has(srcUsage.Type) { - diags = append(diags, &hcl.Diagnostic{ - Summary: "Unknown " + buildSourceLabel + " type " + srcUsage.Type, - Subject: &build.HCL2Ref.DefRange, - Detail: fmt.Sprintf("known builders: %v", cfg.parser.PluginConfig.Builders.List()), - Severity: hcl.DiagError, - }) - continue - } - - sourceDefinition, found := cfg.Sources[srcUsage.SourceRef] - if !found { - availableSrcs := listAvailableSourceNames(cfg.Sources) - detail := fmt.Sprintf("Known: %v", availableSrcs) - if sugg := didyoumean.NameSuggestion(srcUsage.SourceRef.String(), availableSrcs); sugg != "" { - detail = fmt.Sprintf("Did you mean to use %q?", sugg) - } - diags = append(diags, &hcl.Diagnostic{ - Summary: "Unknown " + sourceLabel + " " + srcUsage.SourceRef.String(), - Subject: build.HCL2Ref.DefRange.Ptr(), - Severity: hcl.DiagError, - Detail: detail, - }) - continue - } - - // Before attempting to use the body for merging, we - // finalise its decoding if necessary. - diags = append(diags, sourceDefinition.finalizeDecodeSource(cfg)...) - if diags.HasErrors() { - continue - } - - body := sourceDefinition.block.Body - if srcUsage.Body != nil { - // merge additions into source definition to get a new body. - body = hcl.MergeBodies([]hcl.Body{body, srcUsage.Body}) - } - - srcUsage.Body = body - } - - for _, provBlock := range build.ProvisionerBlocks { - if !cfg.parser.PluginConfig.Provisioners.Has(provBlock.PType) { - diags = append(diags, &hcl.Diagnostic{ - Summary: fmt.Sprintf("Unknown "+buildProvisionerLabel+" type %q", provBlock.PType), - Subject: provBlock.HCL2Ref.TypeRange.Ptr(), - Detail: fmt.Sprintf("known "+buildProvisionerLabel+"s: %v", cfg.parser.PluginConfig.Provisioners.List()), - Severity: hcl.DiagError, - }) - } - } - - if build.ErrorCleanupProvisionerBlock != nil { - if !cfg.parser.PluginConfig.Provisioners.Has(build.ErrorCleanupProvisionerBlock.PType) { - diags = append(diags, &hcl.Diagnostic{ - Summary: fmt.Sprintf("Unknown "+buildErrorCleanupProvisionerLabel+" type %q", build.ErrorCleanupProvisionerBlock.PType), - Subject: build.ErrorCleanupProvisionerBlock.HCL2Ref.TypeRange.Ptr(), - Detail: fmt.Sprintf("known "+buildErrorCleanupProvisionerLabel+"s: %v", cfg.parser.PluginConfig.Provisioners.List()), - Severity: hcl.DiagError, - }) - } - } - - for _, ppList := range build.PostProcessorsLists { - for _, ppBlock := range ppList { - if !cfg.parser.PluginConfig.PostProcessors.Has(ppBlock.PType) { - diags = append(diags, &hcl.Diagnostic{ - Summary: fmt.Sprintf("Unknown "+buildPostProcessorLabel+" type %q", ppBlock.PType), - Subject: ppBlock.HCL2Ref.TypeRange.Ptr(), - Detail: fmt.Sprintf("known "+buildPostProcessorLabel+"s: %v", cfg.parser.PluginConfig.PostProcessors.List()), - Severity: hcl.DiagError, - }) - } - } - } - + diags = diags.Extend(build.Initialize(cfg)) } return diags diff --git a/hcl2template/types.build.go b/hcl2template/types.build.go index f4f93cd753d..bda255b1cb5 100644 --- a/hcl2template/types.build.go +++ b/hcl2template/types.build.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/hcl/v2/ext/dynblock" "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/hashicorp/packer-plugin-sdk/didyoumean" "github.com/hashicorp/packer/packer" "github.com/zclconf/go-cty/cty" ) @@ -266,6 +267,101 @@ func (build *BuildBlock) finalizeDecode(cfg *PackerConfig) hcl.Diagnostics { return diags } +func (build *BuildBlock) Initialize(cfg *PackerConfig) hcl.Diagnostics { + var diags hcl.Diagnostics + + // Since the build's contents may not have been dynamically + // expanded when we first loaded the config from the template + // file, we decode it now. + diags = append(diags, build.finalizeDecode(cfg)...) + if diags.HasErrors() { + return diags + } + + for i := range build.Sources { + // here we grab a pointer to the source usage because we will set + // its body. + srcUsage := &(build.Sources[i]) + if !cfg.parser.PluginConfig.Builders.Has(srcUsage.Type) { + diags = append(diags, &hcl.Diagnostic{ + Summary: "Unknown build type " + srcUsage.Type, + Subject: &build.HCL2Ref.DefRange, + Detail: fmt.Sprintf("known builders: %v", cfg.parser.PluginConfig.Builders.List()), + Severity: hcl.DiagError, + }) + continue + } + + sourceDefinition, found := cfg.Sources[srcUsage.SourceRef] + if !found { + availableSrcs := listAvailableSourceNames(cfg.Sources) + detail := fmt.Sprintf("Known: %v", availableSrcs) + if sugg := didyoumean.NameSuggestion(srcUsage.SourceRef.String(), availableSrcs); sugg != "" { + detail = fmt.Sprintf("Did you mean to use %q?", sugg) + } + diags = append(diags, &hcl.Diagnostic{ + Summary: "Unknown " + sourceLabel + " " + srcUsage.SourceRef.String(), + Subject: build.HCL2Ref.DefRange.Ptr(), + Severity: hcl.DiagError, + Detail: detail, + }) + continue + } + + // Before attempting to use the body for merging, we + // finalise its decoding if necessary. + diags = append(diags, sourceDefinition.finalizeDecodeSource(cfg)...) + if diags.HasErrors() { + continue + } + + body := sourceDefinition.block.Body + if srcUsage.Body != nil { + // merge additions into source definition to get a new body. + body = hcl.MergeBodies([]hcl.Body{body, srcUsage.Body}) + } + + srcUsage.Body = body + } + + for _, provBlock := range build.ProvisionerBlocks { + if !cfg.parser.PluginConfig.Provisioners.Has(provBlock.PType) { + diags = append(diags, &hcl.Diagnostic{ + Summary: fmt.Sprintf("Unknown "+buildProvisionerLabel+" type %q", provBlock.PType), + Subject: provBlock.HCL2Ref.TypeRange.Ptr(), + Detail: fmt.Sprintf("known "+buildProvisionerLabel+"s: %v", cfg.parser.PluginConfig.Provisioners.List()), + Severity: hcl.DiagError, + }) + } + } + + if build.ErrorCleanupProvisionerBlock != nil { + if !cfg.parser.PluginConfig.Provisioners.Has(build.ErrorCleanupProvisionerBlock.PType) { + diags = append(diags, &hcl.Diagnostic{ + Summary: fmt.Sprintf("Unknown "+buildErrorCleanupProvisionerLabel+" type %q", build.ErrorCleanupProvisionerBlock.PType), + Subject: build.ErrorCleanupProvisionerBlock.HCL2Ref.TypeRange.Ptr(), + Detail: fmt.Sprintf("known "+buildErrorCleanupProvisionerLabel+"s: %v", cfg.parser.PluginConfig.Provisioners.List()), + Severity: hcl.DiagError, + }) + } + } + + for _, ppList := range build.PostProcessorsLists { + for _, ppBlock := range ppList { + if !cfg.parser.PluginConfig.PostProcessors.Has(ppBlock.PType) { + diags = append(diags, &hcl.Diagnostic{ + Summary: fmt.Sprintf("Unknown "+buildPostProcessorLabel+" type %q", ppBlock.PType), + Subject: ppBlock.HCL2Ref.TypeRange.Ptr(), + Detail: fmt.Sprintf("known "+buildPostProcessorLabel+"s: %v", cfg.parser.PluginConfig.PostProcessors.List()), + Severity: hcl.DiagError, + }) + } + } + } + + return diags +} + // ToCoreBuilds extracts the core builds from a build block. // // Since build blocks can have multiple sources, it can lead to multiple builds From 05199ec5c8ebf19091609c2f5c029883ad26a382 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 13 Sep 2023 09:39:30 -0400 Subject: [PATCH 19/28] packer: change visibility of builds and variables Since these will be used by the schedulers' code, we need to make it available from outside the `packer' package, so we make them public. --- packer/core.go | 32 ++++++++++++++++---------------- packer/core_test.go | 16 ++++++++-------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/packer/core.go b/packer/core.go index 732f026360f..86f8d006883 100644 --- a/packer/core.go +++ b/packer/core.go @@ -31,8 +31,8 @@ type Core struct { Template *template.Template components ComponentFinder - variables map[string]string - builds map[string]*template.Builder + Variables map[string]string + Builds map[string]*template.Builder version string secrets []string @@ -124,7 +124,7 @@ func NewCore(c *CoreConfig) *Core { core := &Core{ Template: c.Template, components: c.Components, - variables: c.Variables, + Variables: c.Variables, version: c.Version, only: c.Only, except: c.Except, @@ -164,7 +164,7 @@ func (core *Core) initialize() error { // Go through and interpolate all the build names. We should be able // to do this at this point with the variables. - core.builds = make(map[string]*template.Builder) + core.Builds = make(map[string]*template.Builder) for _, b := range core.Template.Builders { v, err := interpolate.Render(b.Name, core.Context()) if err != nil { @@ -173,7 +173,7 @@ func (core *Core) initialize() error { b.Name, err) } - core.builds[v] = b + core.Builds[v] = b } return nil @@ -197,8 +197,8 @@ func (c *Core) BuildNames(only, except []string) []string { c.except = except c.only = only - r := make([]string, 0, len(c.builds)) - for n := range c.builds { + r := make([]string, 0, len(c.Builds)) + for n := range c.Builds { onlyPos := sort.SearchStrings(only, n) foundInOnly := onlyPos < len(only) && only[onlyPos] == n if len(only) > 0 && !foundInOnly { @@ -327,7 +327,7 @@ func (c *Core) GetBuilds(opts GetBuildsOptions) ([]packersdk.Build, hcl.Diagnost // Build returns the Build object for the given name. func (c *Core) Build(n string) (packersdk.Build, error) { // Setup the builder - configBuilder, ok := c.builds[n] + configBuilder, ok := c.Builds[n] if !ok { return nil, fmt.Errorf("no such build found: %s", n) } @@ -438,7 +438,7 @@ func (c *Core) Build(n string) (packersdk.Build, error) { Provisioners: provisioners, CleanupProvisioner: cleanupProvisioner, TemplatePath: c.Template.Path, - Variables: c.variables, + Variables: c.Variables, } //configBuilder.Name is left uninterpolated so we must check against @@ -454,7 +454,7 @@ func (c *Core) Build(n string) (packersdk.Build, error) { func (c *Core) Context() *interpolate.Context { return &interpolate.Context{ TemplatePath: c.Template.Path, - UserVariables: c.variables, + UserVariables: c.Variables, CorePackerVersionString: packerversion.FormattedVersion(), } } @@ -725,7 +725,7 @@ func (c *Core) validate() error { var err error for n, v := range c.Template.Variables { if v.Required { - if _, ok := c.variables[n]; !ok { + if _, ok := c.Variables[n]; !ok { err = multierror.Append(err, fmt.Errorf( "required variable not set: %s", n)) } @@ -800,7 +800,7 @@ func (c *Core) renderVarsRecursively() (*interpolate.Context, error) { } // overwrite template variables with command-line-read variables - for k, v := range c.variables { + for k, v := range c.Variables { repeatMap[k] = v allKeys = append(allKeys, k) } @@ -834,8 +834,8 @@ func (c *Core) renderVarsRecursively() (*interpolate.Context, error) { // We only get here if interpolation has succeeded, so something is // different in this loop than in the last one. changed = true - c.variables[kv.Key] = renderedV - ctx.UserVariables = c.variables + c.Variables[kv.Key] = renderedV + ctx.UserVariables = c.Variables // Remove fully-interpolated variables from the map, and flag // variables that still need interpolating for a repeat. done, err := isDoneInterpolating(kv.Value) @@ -891,8 +891,8 @@ func (c *Core) renderVarsRecursively() (*interpolate.Context, error) { } func (c *Core) init() error { - if c.variables == nil { - c.variables = make(map[string]string) + if c.Variables == nil { + c.Variables = make(map[string]string) } // Go through the variables and interpolate the environment and // user variables diff --git a/packer/core_test.go b/packer/core_test.go index b1694cbf86c..222f0e958d9 100644 --- a/packer/core_test.go +++ b/packer/core_test.go @@ -164,16 +164,16 @@ func TestCoreBuild_IgnoreTemplateVariables(t *testing.T) { testCoreTemplate(t, config, fixtureDir("build-ignore-template-variable.json")) core := TestCore(t, config) - if core.variables["http_ip"] != "{{ .HTTPIP }}" { + if core.Variables["http_ip"] != "{{ .HTTPIP }}" { t.Fatalf("bad: User variable http_ip={{ .HTTPIP }} should not be interpolated") } - if core.variables["var"] != "test_{{ .PACKER_TEST_TEMP }}" { - t.Fatalf("bad: User variable var should be half interpolated to var=test_{{ .PACKER_TEST_TEMP }} but was var=%s", core.variables["var"]) + if core.Variables["var"] != "test_{{ .PACKER_TEST_TEMP }}" { + t.Fatalf("bad: User variable var should be half interpolated to var=test_{{ .PACKER_TEST_TEMP }} but was var=%s", core.Variables["var"]) } - if core.variables["array_var"] != "us-west-1,us-west-2" { - t.Fatalf("bad: User variable array_var should be \"us-west-1,us-west-2\" but was %s", core.variables["var"]) + if core.Variables["array_var"] != "us-west-1,us-west-2" { + t.Fatalf("bad: User variable array_var should be \"us-west-1,us-west-2\" but was %s", core.Variables["var"]) } build, err := core.Build("test") @@ -563,7 +563,7 @@ func TestCore_InterpolateUserVars(t *testing.T) { } if !tc.Err { - for k, v := range ccf.variables { + for k, v := range ccf.Variables { if tc.Expected[k] != v { t.Fatalf("Expected %s but got %s", tc.Expected[k], v) } @@ -631,7 +631,7 @@ func TestCore_InterpolateUserVars_VarFile(t *testing.T) { t.Fatalf("err: %s\n\n%s", tc.File, diags) } if !tc.Err { - for k, v := range ccf.variables { + for k, v := range ccf.Variables { if tc.Expected[k] != v { t.Fatalf("Expected value %s for key %s but got %s", tc.Expected[k], k, v) @@ -781,7 +781,7 @@ func TestEnvAndFileVars(t *testing.T) { if diags.HasErrors() { t.Fatalf("err: %s\n\n%s", "complex-recursed-env-user-var-file.json", diags) } - for k, v := range ccf.variables { + for k, v := range ccf.Variables { if expected[k] != v { t.Fatalf("Expected value %s for key %s but got %s", expected[k], k, v) From 0de1e1816c14376cfbb681266b94a0d165855f68 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 13 Sep 2023 15:39:05 -0400 Subject: [PATCH 20/28] hcl2template: move duplicate check to PackerConfig Since this will be required to be called from elsewhere later, we make this function public, and since it relies on data from config to be passed in at callsite, we may as well move this function to be a PackerConfig method instead. --- hcl2template/types.packer_config.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index 6d2df157f49..bb202ddbb7e 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -184,13 +184,13 @@ func (c *PackerConfig) decodeInputVariables(f *hcl.File) hcl.Diagnostics { return diags } -func checkForDuplicateLocalDefinition(locals []*LocalBlock) hcl.Diagnostics { +func (cfg *PackerConfig) CheckForDuplicateLocalDefinition() hcl.Diagnostics { var diags hcl.Diagnostics // we could sort by name and then check contiguous names to use less memory, // but using a map sounds good enough. names := map[string]struct{}{} - for _, local := range locals { + for _, local := range cfg.LocalBlocks { if _, found := names[local.Name]; found { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, @@ -559,7 +559,7 @@ func (p *PackerConfig) InspectConfig(opts packer.InspectConfigOptions) int { func (cfg *PackerConfig) Initialize(opts packer.InitializeOptions) hcl.Diagnostics { diags := cfg.InputVariables.ValidateValues() - diags = diags.Extend(checkForDuplicateLocalDefinition(cfg.LocalBlocks)) + diags = diags.Extend(cfg.CheckForDuplicateLocalDefinition()) diags = diags.Extend(cfg.executeDatasources(opts.SkipDatasourcesExecution)) diags = diags.Extend(cfg.evaluateLocalVariables()) From 4be4c6ed84c21fa9bbe86912f00c55b0d98b02a5 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Mon, 18 Sep 2023 15:05:20 -0400 Subject: [PATCH 21/28] hcl2template: keep hcl2 files in a map Since printing the diagnostics to users requires using a map of filename to HCL files, we change this in the config definition for HCL templates. --- hcl2template/parser.go | 6 +++--- hcl2template/types.packer_config.go | 6 +++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/hcl2template/parser.go b/hcl2template/parser.go index 261af644bd0..375f1eb5ce0 100644 --- a/hcl2template/parser.go +++ b/hcl2template/parser.go @@ -95,7 +95,7 @@ const ( // init should be called next to expand dynamic blocks and verify that used // things do exist. func (p *Parser) Parse(filename string, varFiles []string, argVars map[string]string) (*PackerConfig, hcl.Diagnostics) { - var files []*hcl.File + files := map[string]*hcl.File{} var diags hcl.Diagnostics // parse config files @@ -118,12 +118,12 @@ func (p *Parser) Parse(filename string, varFiles []string, argVars map[string]st for _, filename := range hclFiles { f, moreDiags := p.ParseHCLFile(filename) diags = append(diags, moreDiags...) - files = append(files, f) + files[filename] = f } for _, filename := range jsonFiles { f, moreDiags := p.ParseJSONFile(filename) diags = append(diags, moreDiags...) - files = append(files, f) + files[filename] = f } if diags.HasErrors() { return nil, diags diff --git a/hcl2template/types.packer_config.go b/hcl2template/types.packer_config.go index bb202ddbb7e..72aa05c321e 100644 --- a/hcl2template/types.packer_config.go +++ b/hcl2template/types.packer_config.go @@ -58,7 +58,7 @@ type PackerConfig struct { HCPVars map[string]cty.Value parser *Parser - files []*hcl.File + files map[string]*hcl.File // Fields passed as command line flags Force bool @@ -359,6 +359,10 @@ func (cfg *PackerConfig) ReportUnusedFilters(buildNames []string) hcl.Diagnostic return diags } +func (cfg *PackerConfig) Files() map[string]*hcl.File { + return cfg.files +} + func (cfg *PackerConfig) GetBuilds(opts packer.GetBuildsOptions) ([]packersdk.Build, hcl.Diagnostics) { var allBuilds []packersdk.Build var diags hcl.Diagnostics From 0065f731ae8e0ab0f85ea3b55587c0e3c30c78d9 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Tue, 19 Sep 2023 10:34:39 -0400 Subject: [PATCH 22/28] packer: make Core.Validate public Since we'll move the initialization code to the schedulers once they're in the codebase, we make the validation function public. --- packer/core.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packer/core.go b/packer/core.go index 86f8d006883..ba3006b3ce9 100644 --- a/packer/core.go +++ b/packer/core.go @@ -152,7 +152,7 @@ func (c *Core) Initialize(_ InitializeOptions) hcl.Diagnostics { } func (core *Core) initialize() error { - if err := core.validate(); err != nil { + if err := core.Validate(); err != nil { return err } if err := core.init(); err != nil { @@ -692,7 +692,7 @@ func (c *Core) FixConfig(opts FixConfigOptions) hcl.Diagnostics { // // This will automatically call template.validate() in addition to doing // richer semantic checks around variables and so on. -func (c *Core) validate() error { +func (c *Core) Validate() error { // First validate the template in general, we can't do anything else // unless the template itself is valid. if err := c.Template.Validate(); err != nil { From 979601c88b1d1cb4dc3d34a4425bcd818d4ac838 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 13 Sep 2023 11:20:25 -0400 Subject: [PATCH 23/28] command: move command logic to using schedulers In order to be able to change how templates are processed later on, we move the logic that processes templates to a new abstraction: Scheduler. Right now, one implementation of Scheduler exists: sequential. This is essentially the same approach as what we have now embedded in the configs, but is now moved to this structure, so we can change it later. --- command/build.go | 311 +--------------------- command/packer-manifest.json | 50 ++++ command/scheduler.go | 21 ++ command/sequential/hcl/builds.go | 87 ++++++ command/sequential/hcl/datasources.go | 64 +++++ command/sequential/hcl/scheduler.go | 20 ++ command/sequential/hcl/variables.go | 86 ++++++ command/sequential/json/builds.go | 87 ++++++ command/sequential/json/scheduler.go | 26 ++ command/sequential/json/variables.go | 204 ++++++++++++++ command/sequential_scheduler.go | 368 ++++++++++++++++++++++++++ 11 files changed, 1015 insertions(+), 309 deletions(-) create mode 100644 command/packer-manifest.json create mode 100644 command/scheduler.go create mode 100644 command/sequential/hcl/builds.go create mode 100644 command/sequential/hcl/datasources.go create mode 100644 command/sequential/hcl/scheduler.go create mode 100644 command/sequential/hcl/variables.go create mode 100644 command/sequential/json/builds.go create mode 100644 command/sequential/json/scheduler.go create mode 100644 command/sequential/json/variables.go create mode 100644 command/sequential_scheduler.go diff --git a/command/build.go b/command/build.go index c77687c3586..bf1eca45efa 100644 --- a/command/build.go +++ b/command/build.go @@ -6,22 +6,12 @@ package command import ( "bytes" "context" - "errors" - "fmt" - "log" "math" - "strconv" "strings" - "sync" - "time" "github.com/hashicorp/hcl/v2" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" - "github.com/hashicorp/packer/internal/hcp/registry" - "github.com/hashicorp/packer/packer" - "golang.org/x/sync/semaphore" - "github.com/hako/durafmt" "github.com/posener/complete" ) @@ -93,305 +83,8 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, cla *BuildArgs) int return ret } - diags = packerStarter.Initialize(packer.InitializeOptions{}) - bundledDiags := c.DetectBundledPlugins(packerStarter) - diags = append(bundledDiags, diags...) - ret = writeDiags(c.Ui, nil, diags) - if ret != 0 { - return ret - } - - hcpRegistry, diags := registry.New(packerStarter, c.Ui) - ret = writeDiags(c.Ui, nil, diags) - if ret != 0 { - return ret - } - - defer hcpRegistry.IterationStatusSummary() - - err := hcpRegistry.PopulateIteration(buildCtx) - if err != nil { - return writeDiags(c.Ui, nil, hcl.Diagnostics{ - &hcl.Diagnostic{ - Summary: "HCP: populating iteration failed", - Severity: hcl.DiagError, - Detail: err.Error(), - }, - }) - } - - builds, diags := packerStarter.GetBuilds(packer.GetBuildsOptions{ - Only: cla.Only, - Except: cla.Except, - Debug: cla.Debug, - Force: cla.Force, - OnError: cla.OnError, - }) - - // here, something could have gone wrong but we still want to run valid - // builds. - ret = writeDiags(c.Ui, nil, diags) - if len(builds) == 0 && ret != 0 { - return ret - } - - if cla.Debug { - c.Ui.Say("Debug mode enabled. Builds will not be parallelized.") - } - - // Compile all the UIs for the builds - colors := [5]packer.UiColor{ - packer.UiColorGreen, - packer.UiColorCyan, - packer.UiColorMagenta, - packer.UiColorYellow, - packer.UiColorBlue, - } - buildUis := make(map[packersdk.Build]packersdk.Ui) - for i := range builds { - ui := c.Ui - if cla.Color { - // Only set up UI colors if -machine-readable isn't set. - if _, ok := c.Ui.(*packer.MachineReadableUi); !ok { - ui = &packer.ColoredUi{ - Color: colors[i%len(colors)], - Ui: ui, - } - ui.Say(fmt.Sprintf("%s: output will be in this color.", builds[i].Name())) - if i+1 == len(builds) { - // Add a newline between the color output and the actual output - c.Ui.Say("") - } - } - } - // Now add timestamps if requested - if cla.TimestampUi { - ui = &packer.TimestampedUi{ - Ui: ui, - } - } - - buildUis[builds[i]] = ui - } - log.Printf("Build debug mode: %v", cla.Debug) - log.Printf("Force build: %v", cla.Force) - log.Printf("On error: %v", cla.OnError) - - if len(builds) == 0 { - return writeDiags(c.Ui, nil, hcl.Diagnostics{ - &hcl.Diagnostic{ - Summary: "No builds to run", - Detail: "A build command cannot run without at least one build to process. " + - "If the only or except flags have been specified at run time check that" + - " at least one build is selected for execution.", - Severity: hcl.DiagError, - }, - }) - } - - // Get the start of the build command - buildCommandStart := time.Now() - - // Run all the builds in parallel and wait for them to complete - var wg sync.WaitGroup - var artifacts = struct { - sync.RWMutex - m map[string][]packersdk.Artifact - }{m: make(map[string][]packersdk.Artifact)} - // Get the builds we care about - var errs = struct { - sync.RWMutex - m map[string]error - }{m: make(map[string]error)} - limitParallel := semaphore.NewWeighted(cla.ParallelBuilds) - for i := range builds { - if err := buildCtx.Err(); err != nil { - log.Println("Interrupted, not going to start any more builds.") - break - } - - b := builds[i] - name := b.Name() - ui := buildUis[b] - if err := limitParallel.Acquire(buildCtx, 1); err != nil { - ui.Error(fmt.Sprintf("Build '%s' failed to acquire semaphore: %s", name, err)) - errs.Lock() - errs.m[name] = err - errs.Unlock() - break - } - // Increment the waitgroup so we wait for this item to finish properly - wg.Add(1) - - // Run the build in a goroutine - go func() { - // Get the start of the build - buildStart := time.Now() - - defer wg.Done() - - defer limitParallel.Release(1) - - err := hcpRegistry.StartBuild(buildCtx, b) - // Seems odd to require this error check here. Now that it is an error we can just exit with diag - if err != nil { - // If the build is already done, we skip without a warning - if errors.As(err, ®istry.ErrBuildAlreadyDone{}) { - ui.Say(fmt.Sprintf("skipping already done build %q", name)) - return - } - writeDiags(c.Ui, nil, hcl.Diagnostics{ - &hcl.Diagnostic{ - Summary: fmt.Sprintf( - "hcp: failed to start build %q", - name), - Severity: hcl.DiagError, - Detail: err.Error(), - }, - }) - return - } - - log.Printf("Starting build run: %s", name) - runArtifacts, err := b.Run(buildCtx, ui) - - // Get the duration of the build and parse it - buildEnd := time.Now() - buildDuration := buildEnd.Sub(buildStart) - fmtBuildDuration := durafmt.Parse(buildDuration).LimitFirstN(2) - - runArtifacts, hcperr := hcpRegistry.CompleteBuild( - buildCtx, - b, - runArtifacts, - err) - if hcperr != nil { - writeDiags(c.Ui, nil, hcl.Diagnostics{ - &hcl.Diagnostic{ - Summary: fmt.Sprintf( - "failed to complete HCP-enabled build %q", - name), - Severity: hcl.DiagError, - Detail: hcperr.Error(), - }, - }) - } - - if err != nil { - ui.Error(fmt.Sprintf("Build '%s' errored after %s: %s", name, fmtBuildDuration, err)) - errs.Lock() - errs.m[name] = err - errs.Unlock() - } else { - ui.Say(fmt.Sprintf("Build '%s' finished after %s.", name, fmtBuildDuration)) - if runArtifacts != nil { - artifacts.Lock() - artifacts.m[name] = runArtifacts - artifacts.Unlock() - } - } - }() - - if cla.Debug { - log.Printf("Debug enabled, so waiting for build to finish: %s", b.Name()) - wg.Wait() - } - - if cla.ParallelBuilds == 1 { - log.Printf("Parallelization disabled, waiting for build to finish: %s", b.Name()) - wg.Wait() - } - } - - // Wait for both the builds to complete and the interrupt handler, - // if it is interrupted. - log.Printf("Waiting on builds to complete...") - wg.Wait() - - // Get the duration of the buildCommand command and parse it - buildCommandEnd := time.Now() - buildCommandDuration := buildCommandEnd.Sub(buildCommandStart) - fmtBuildCommandDuration := durafmt.Parse(buildCommandDuration).LimitFirstN(2) - c.Ui.Say(fmt.Sprintf("\n==> Wait completed after %s", fmtBuildCommandDuration)) - - if err := buildCtx.Err(); err != nil { - c.Ui.Say("Cleanly cancelled builds after being interrupted.") - return 1 - } - - if len(errs.m) > 0 { - c.Ui.Machine("error-count", strconv.FormatInt(int64(len(errs.m)), 10)) - - c.Ui.Error("\n==> Some builds didn't complete successfully and had errors:") - for name, err := range errs.m { - // Create a UI for the machine readable stuff to be targeted - ui := &packer.TargetedUI{ - Target: name, - Ui: c.Ui, - } - - ui.Machine("error", err.Error()) - - c.Ui.Error(fmt.Sprintf("--> %s: %s", name, err)) - } - } - - if len(artifacts.m) > 0 { - c.Ui.Say("\n==> Builds finished. The artifacts of successful builds are:") - for name, buildArtifacts := range artifacts.m { - // Create a UI for the machine readable stuff to be targeted - ui := &packer.TargetedUI{ - Target: name, - Ui: c.Ui, - } - - // Machine-readable helpful - ui.Machine("artifact-count", strconv.FormatInt(int64(len(buildArtifacts)), 10)) - - for i, artifact := range buildArtifacts { - var message bytes.Buffer - fmt.Fprintf(&message, "--> %s: ", name) - - if artifact != nil { - fmt.Fprint(&message, artifact.String()) - } else { - fmt.Fprint(&message, "") - } - - iStr := strconv.FormatInt(int64(i), 10) - if artifact != nil { - ui.Machine("artifact", iStr, "builder-id", artifact.BuilderId()) - ui.Machine("artifact", iStr, "id", artifact.Id()) - ui.Machine("artifact", iStr, "string", artifact.String()) - - files := artifact.Files() - ui.Machine("artifact", - iStr, - "files-count", strconv.FormatInt(int64(len(files)), 10)) - for fi, file := range files { - fiStr := strconv.FormatInt(int64(fi), 10) - ui.Machine("artifact", iStr, "file", fiStr, file) - } - } else { - ui.Machine("artifact", iStr, "nil") - } - - ui.Machine("artifact", iStr, "end") - c.Ui.Say(message.String()) - - } - - } - } else { - c.Ui.Say("\n==> Builds finished but no artifacts were created.") - } - - if len(errs.m) > 0 { - // If any errors occurred, exit with a non-zero exit status - ret = 1 - } - - return ret + scheduler := NewScheduler(packerStarter, c.Ui, buildCtx) + return scheduler.Build(cla) } func (*BuildCommand) Help() string { diff --git a/command/packer-manifest.json b/command/packer-manifest.json new file mode 100644 index 00000000000..5e1b19d9446 --- /dev/null +++ b/command/packer-manifest.json @@ -0,0 +1,50 @@ +{ + "builds": [ + { + "name": "test", + "builder_type": "null", + "build_time": 1694200646, + "files": null, + "artifact_id": "Null", + "packer_run_uuid": "", + "custom_data": null + }, + { + "name": "test", + "builder_type": "null", + "build_time": 1694200658, + "files": null, + "artifact_id": "Null", + "packer_run_uuid": "", + "custom_data": null + }, + { + "name": "test", + "builder_type": "null", + "build_time": 1694201568, + "files": null, + "artifact_id": "Null", + "packer_run_uuid": "", + "custom_data": null + }, + { + "name": "test", + "builder_type": "null", + "build_time": 1694201578, + "files": null, + "artifact_id": "Null", + "packer_run_uuid": "", + "custom_data": null + }, + { + "name": "test", + "builder_type": "null", + "build_time": 1694201592, + "files": null, + "artifact_id": "Null", + "packer_run_uuid": "", + "custom_data": null + } + ], + "last_run_uuid": "" +} \ No newline at end of file diff --git a/command/scheduler.go b/command/scheduler.go new file mode 100644 index 00000000000..0acc0c4d973 --- /dev/null +++ b/command/scheduler.go @@ -0,0 +1,21 @@ +package command + +import ( + "context" + + packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + "github.com/hashicorp/packer/packer" +) + +type Scheduler interface { + Build(*BuildArgs) int +} + +// NewScheduler returns a new scheduler for running commands with. +func NewScheduler( + cfg packer.Handler, + ui packersdk.Ui, + context context.Context, +) Scheduler { + return NewSequentialScheduler(cfg, ui, context) +} diff --git a/command/sequential/hcl/builds.go b/command/sequential/hcl/builds.go new file mode 100644 index 00000000000..addc99d532f --- /dev/null +++ b/command/sequential/hcl/builds.go @@ -0,0 +1,87 @@ +package schedulers + +import ( + "fmt" + + "github.com/hashicorp/hcl/v2" + packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + "github.com/hashicorp/packer/hcl2template" +) + +func (s *HCLSequentialScheduler) PrepareBuilds() hcl.Diagnostics { + // verify that all used plugins do exist + var diags hcl.Diagnostics + + if len(s.config.Builds) == 0 { + return diags.Append(&hcl.Diagnostic{ + Summary: "Missing build block", + Detail: "A build block with one or more sources is required for executing a build.", + Severity: hcl.DiagError, + }) + } + + for _, build := range s.config.Builds { + diags = diags.Extend(build.Initialize(s.config)) + } + + return diags +} + +func (s *HCLSequentialScheduler) FilterBuilds( + debug, force bool, + onError string, + except, only []string, +) ([]packersdk.Build, hcl.Diagnostics) { + var allBuilds []packersdk.Build + var diags hcl.Diagnostics + + var convertDiags hcl.Diagnostics + s.config.Debug = debug + s.config.Except, convertDiags = hcl2template.ConvertFilterOption(except, "except") + diags = diags.Extend(convertDiags) + s.config.Only, convertDiags = hcl2template.ConvertFilterOption(only, "only") + diags = diags.Extend(convertDiags) + s.config.Force = force + s.config.OnError = onError + + s.config.PrepareGlobUsage() + + for _, build := range s.config.Builds { + cbs, cbDiags := build.ToCoreBuilds(s.config) + diags = diags.Extend(cbDiags) + + for _, cb := range cbs { + cb.SetDebug(debug) + cb.SetForce(force) + cb.SetOnError(onError) + + cb.Prepared = true + + // Prepare just sets the "prepareCalled" flag on CoreBuild, since + // we did all the prep in `ToCoreBuilds`. + _, err := cb.Prepare() + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Preparing packer core build %s failed", cb.Name()), + Detail: err.Error(), + }) + } + + allBuilds = append(allBuilds, cb) + } + } + + buildNames := []string{} + for _, cb := range allBuilds { + buildNames = append(buildNames, cb.Name()) + } + + diags = diags.Extend( + s.config.ReportUnusedFilters( + buildNames, + ), + ) + + return allBuilds, diags +} diff --git a/command/sequential/hcl/datasources.go b/command/sequential/hcl/datasources.go new file mode 100644 index 00000000000..1e550c60540 --- /dev/null +++ b/command/sequential/hcl/datasources.go @@ -0,0 +1,64 @@ +package schedulers + +import ( + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/packer/hcl2template" +) + +// datasourcesDone checks whether all the datasources have been executed or not +func (s *HCLSequentialScheduler) datasourcesDone() bool { + for _, ds := range s.config.Datasources { + if !ds.Executed() { + return false + } + } + + return true +} + +func (s *HCLSequentialScheduler) ExecuteDataSources(skipDatasourcesExecution bool) hcl.Diagnostics { + // If we are done with datasources execution, we leave immediately + if s.datasourcesDone() { + return nil + } + + var outDiags hcl.Diagnostics + + foundSomething := false + for _, ds := range s.config.Datasources { + if ds.Executed() { + continue + } + + diags := ds.Execute(s.config, skipDatasourcesExecution) + if diags.HasErrors() && diags[0].Summary == hcl2template.NotReadyDataSourceError { + // If we have a not ready error in the + // datasource list, we should attempt to run the + // rest, and eventually settle if we cannot move + // any further + continue + } + + foundSomething = true + + outDiags = append(outDiags, diags...) + } + + if !foundSomething { + return append(outDiags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "No datasource could be executed", + Detail: `While trying to recurisvely evaluating datasources, we could not find a next datasource to execute. +This is likely due to a cyclic dependency in your datasources`, + }) + } + + // If we couldn't execute a datasource for whatever reason, we leave + if outDiags.HasErrors() { + return outDiags + } + + // If we still found something to execute, we recursively execute the + // remainder of the datasources + return outDiags.Extend(s.ExecuteDataSources(skipDatasourcesExecution)) +} diff --git a/command/sequential/hcl/scheduler.go b/command/sequential/hcl/scheduler.go new file mode 100644 index 00000000000..acc5897eec5 --- /dev/null +++ b/command/sequential/hcl/scheduler.go @@ -0,0 +1,20 @@ +package schedulers + +import ( + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/packer/hcl2template" +) + +type HCLSequentialScheduler struct { + config *hcl2template.PackerConfig +} + +func NewScheduler(config *hcl2template.PackerConfig) *HCLSequentialScheduler { + return &HCLSequentialScheduler{ + config: config, + } +} + +func (s *HCLSequentialScheduler) FileMap() map[string]*hcl.File { + return s.config.Files() +} diff --git a/command/sequential/hcl/variables.go b/command/sequential/hcl/variables.go new file mode 100644 index 00000000000..ff5deb955e6 --- /dev/null +++ b/command/sequential/hcl/variables.go @@ -0,0 +1,86 @@ +package schedulers + +import ( + "github.com/hashicorp/hcl/v2" + packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + "github.com/hashicorp/packer/hcl2template" + "github.com/zclconf/go-cty/cty" +) + +func (s HCLSequentialScheduler) localVariablesEvaluationDone() bool { + for _, loc := range s.config.LocalBlocks { + if !loc.Evaluated() { + return false + } + } + + return true +} + +func (s *HCLSequentialScheduler) EvaluateVariables() hcl.Diagnostics { + diags := s.config.InputVariables.ValidateValues() + diags = diags.Extend(s.config.CheckForDuplicateLocalDefinition()) + + diags = diags.Extend(s.evaluateVariables()) + + filterVarsFromLogs(s.config.InputVariables) + filterVarsFromLogs(s.config.LocalVariables) + + return diags +} + +func (s *HCLSequentialScheduler) evaluateVariables() hcl.Diagnostics { + if len(s.config.LocalBlocks) == 0 { + return nil + } + + // If we're done evaluating variables, we can leave immediately + if s.localVariablesEvaluationDone() { + return nil + } + + var diags hcl.Diagnostics + + found := false + for _, loc := range s.config.LocalBlocks { + if loc.Evaluated() { + continue + } + + evalDiags := loc.Evaluate(s.config) + // If there's a not ready for eval error, we continue iterating + // on the other variable blocks, until we reach a point where + // we can evaluate it. + if evalDiags.HasErrors() && + evalDiags[0].Summary == hcl2template.VarNotReadyForEval { + continue + } + + found = true + } + + if !found { + return append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Failed to evaluate local variables", + Detail: "Packer couldn't evaluate any more variables, and some are pending. This likely means your configuration has a dependency cycle in the local variables, that needs to be corrected before building the template.", + }) + } + + return diags.Extend(s.evaluateVariables()) +} + +func filterVarsFromLogs(inputOrLocal hcl2template.Variables) { + for _, variable := range inputOrLocal { + if !variable.Sensitive { + continue + } + value := variable.Value() + _ = cty.Walk(value, func(_ cty.Path, nested cty.Value) (bool, error) { + if nested.IsWhollyKnown() && !nested.IsNull() && nested.Type().Equals(cty.String) { + packersdk.LogSecretFilter.Set(nested.AsString()) + } + return true, nil + }) + } +} diff --git a/command/sequential/json/builds.go b/command/sequential/json/builds.go new file mode 100644 index 00000000000..ca64f3a2a26 --- /dev/null +++ b/command/sequential/json/builds.go @@ -0,0 +1,87 @@ +package json + +import ( + "fmt" + "log" + + "github.com/hashicorp/hcl/v2" + packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + "github.com/hashicorp/packer-plugin-sdk/template" + "github.com/hashicorp/packer-plugin-sdk/template/interpolate" +) + +func (s *JSONSequentialScheduler) PrepareBuilds() hcl.Diagnostics { + var diags hcl.Diagnostics + + // Go through and interpolate all the build names. We should be able + // to do this at this point with the variables. + s.config.Builds = make(map[string]*template.Builder) + for _, b := range s.config.Template.Builders { + v, err := interpolate.Render(b.Name, s.config.Context()) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Build interpolation failure", + Detail: fmt.Sprintf("Error interpolating builder '%s': %s", + b.Name, err), + }) + } + + s.config.Builds[v] = b + } + + return diags +} + +// This is used for json templates to launch the build plugins. +// They will be prepared via b.Prepare() later. +func (s *JSONSequentialScheduler) FilterBuilds( + debug, force bool, + onError string, + except, only []string, +) ([]packersdk.Build, hcl.Diagnostics) { + buildNames := s.config.BuildNames(only, except) + builds := []packersdk.Build{} + diags := hcl.Diagnostics{} + for _, n := range buildNames { + b, err := s.config.Build(n) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Failed to initialize build %q", n), + Detail: err.Error(), + }) + continue + } + + // Now that build plugin has been launched, call Prepare() + log.Printf("Preparing build: %s", b.Name()) + b.SetDebug(debug) + b.SetForce(force) + b.SetOnError(onError) + + warnings, err := b.Prepare() + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Failed to prepare build: %q", n), + Detail: err.Error(), + }) + continue + } + + // Only append builds to list if the Prepare() is successful. + builds = append(builds, b) + + if len(warnings) > 0 { + for _, warning := range warnings { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: fmt.Sprintf("Warning when preparing build: %q", n), + Detail: warning, + }) + } + } + } + return builds, diags +} diff --git a/command/sequential/json/scheduler.go b/command/sequential/json/scheduler.go new file mode 100644 index 00000000000..8a86a6941ad --- /dev/null +++ b/command/sequential/json/scheduler.go @@ -0,0 +1,26 @@ +package json + +import ( + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/packer/packer" +) + +type JSONSequentialScheduler struct { + config *packer.Core +} + +func NewScheduler(config *packer.Core) *JSONSequentialScheduler { + return &JSONSequentialScheduler{ + config: config, + } +} + +// EvaluateDataSources is a noop in JSON as data sources are not supported in this mode. +func (s *JSONSequentialScheduler) ExecuteDataSources(bool) hcl.Diagnostics { + return nil +} + +// In a JSON template, there are no HCL files, so this is always nil. +func (s *JSONSequentialScheduler) FileMap() map[string]*hcl.File { + return nil +} diff --git a/command/sequential/json/variables.go b/command/sequential/json/variables.go new file mode 100644 index 00000000000..025da27e365 --- /dev/null +++ b/command/sequential/json/variables.go @@ -0,0 +1,204 @@ +package json + +import ( + "fmt" + "regexp" + "sort" + "strings" + "text/template" + + "github.com/hashicorp/hcl/v2" + packersdk "github.com/hashicorp/packer-plugin-sdk/packer" + "github.com/hashicorp/packer-plugin-sdk/template/interpolate" +) + +func isDoneInterpolating(v string) (bool, error) { + // Check for whether the var contains any more references to `user`, wrapped + // in interpolation syntax. + filter := `{{\s*user\s*\x60.*\x60\s*}}` + matched, err := regexp.MatchString(filter, v) + if err != nil { + return false, fmt.Errorf("Can't tell if interpolation is done: %s", err) + } + if matched { + // not done interpolating; there's still a call to "user" in a template + // engine + return false, nil + } + // No more calls to "user" as a template engine, so we're done. + return true, nil +} + +func (s *JSONSequentialScheduler) renderVarsRecursively() (*interpolate.Context, error) { + ctx := s.config.Context() + ctx.EnableEnv = true + ctx.UserVariables = make(map[string]string) + shouldRetry := true + changed := false + failedInterpolation := "" + + // Why this giant loop? User variables can be recursively defined. For + // example: + // "variables": { + // "foo": "bar", + // "baz": "{{user `foo`}}baz", + // "bang": "bang{{user `baz`}}" + // }, + // In this situation, we cannot guarantee that we've added "foo" to + // UserVariables before we try to interpolate "baz" the first time. We need + // to have the option to loop back over in order to add the properly + // interpolated "baz" to the UserVariables map. + // Likewise, we'd need to loop up to two times to properly add "bang", + // since that depends on "baz" being set, which depends on "foo" being set. + + // We break out of the while loop either if all our variables have been + // interpolated or if after 100 loops we still haven't succeeded in + // interpolating them. Please don't actually nest your variables in 100 + // layers of other variables. Please. + + // c.Template.Variables is populated by variables defined within the Template + // itself + // c.variables is populated by variables read in from the command line and + // var-files. + // We need to read the keys from both, then loop over all of them to figure + // out the appropriate interpolations. + + repeatMap := make(map[string]string) + allKeys := make([]string, 0) + + // load in template variables + for k, v := range s.config.Template.Variables { + repeatMap[k] = v.Default + allKeys = append(allKeys, k) + } + + // overwrite template variables with command-line-read variables + for k, v := range s.config.Variables { + repeatMap[k] = v + allKeys = append(allKeys, k) + } + + // sort map to force the following loop to be deterministic. + sort.Strings(allKeys) + type keyValue struct { + Key string + Value string + } + sortedMap := make([]keyValue, len(repeatMap)) + for _, k := range allKeys { + sortedMap = append(sortedMap, keyValue{k, repeatMap[k]}) + } + + // Regex to exclude any build function variable or template variable + // from interpolating earlier + // E.g.: {{ .HTTPIP }} won't interpolate now + renderFilter := "{{(\\s|)\\.(.*?)(\\s|)}}" + + for i := 0; i < 100; i++ { + shouldRetry = false + changed = false + deleteKeys := []string{} + // First, loop over the variables in the template + for _, kv := range sortedMap { + // Interpolate the default + renderedV, err := interpolate.RenderRegex(kv.Value, ctx, renderFilter) + switch err.(type) { + case nil: + // We only get here if interpolation has succeeded, so something is + // different in this loop than in the last one. + changed = true + s.config.Variables[kv.Key] = renderedV + ctx.UserVariables = s.config.Variables + // Remove fully-interpolated variables from the map, and flag + // variables that still need interpolating for a repeat. + done, err := isDoneInterpolating(kv.Value) + if err != nil { + return ctx, err + } + if done { + deleteKeys = append(deleteKeys, kv.Key) + } else { + shouldRetry = true + } + case template.ExecError: + castError := err.(template.ExecError) + if strings.Contains(castError.Error(), interpolate.ErrVariableNotSetString) { + shouldRetry = true + failedInterpolation = fmt.Sprintf(`"%s": "%s"; error: %s`, kv.Key, kv.Value, err) + } else { + return ctx, err + } + default: + return ctx, fmt.Errorf( + // unexpected interpolation error: abort the run + "error interpolating default value for '%s': %s", + kv.Key, err) + } + } + if !shouldRetry { + break + } + + // Clear completed vars from sortedMap before next loop. Do this one + // key at a time because the indices are gonna change ever time you + // delete from the map. + for _, k := range deleteKeys { + for ind, kv := range sortedMap { + if kv.Key == k { + sortedMap = append(sortedMap[:ind], sortedMap[ind+1:]...) + break + } + } + } + deleteKeys = []string{} + } + + if !changed && shouldRetry { + return ctx, fmt.Errorf("Failed to interpolate %s: Please make sure that "+ + "the variable you're referencing has been defined; Packer treats "+ + "all variables used to interpolate other user variables as "+ + "required.", failedInterpolation) + } + + return ctx, nil +} + +func (s *JSONSequentialScheduler) getEvaluatedVariables() ([]string, error) { + if s.config.Variables == nil { + s.config.Variables = make(map[string]string) + } + // Go through the variables and interpolate the environment and + // user variables + ctx, err := s.renderVarsRecursively() + if err != nil { + return nil, err + } + + secrets := []string{} + + for _, v := range s.config.Template.SensitiveVariables { + secret := ctx.UserVariables[v.Key] + secrets = append(secrets, secret) + } + + return secrets, nil +} + +func (s *JSONSequentialScheduler) EvaluateVariables() hcl.Diagnostics { + secrets, err := s.getEvaluatedVariables() + + if err != nil { + return hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Failed to evaluate variables", + Detail: err.Error(), + }, + } + } + for _, secret := range secrets { + packersdk.LogSecretFilter.Set(secret) + } + + return nil +} diff --git a/command/sequential_scheduler.go b/command/sequential_scheduler.go new file mode 100644 index 00000000000..9a2a770d5b0 --- /dev/null +++ b/command/sequential_scheduler.go @@ -0,0 +1,368 @@ +package command + +import ( + "bytes" + "context" + "errors" + "fmt" + "log" + "strconv" + "sync" + "time" + + "github.com/hako/durafmt" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/packer/hcl2template" + "github.com/hashicorp/packer/internal/hcp/registry" + "github.com/hashicorp/packer/packer" + "golang.org/x/sync/semaphore" + + hclscheduler "github.com/hashicorp/packer/command/sequential/hcl" + jsonscheduler "github.com/hashicorp/packer/command/sequential/json" + + packersdk "github.com/hashicorp/packer-plugin-sdk/packer" +) + +type SpecialisedSequentialScheduler interface { + ExecuteDataSources(skip bool) hcl.Diagnostics + EvaluateVariables() hcl.Diagnostics + PrepareBuilds() hcl.Diagnostics + FilterBuilds( + debug, force bool, + onError string, + except, only []string, + ) ([]packersdk.Build, hcl.Diagnostics) + FileMap() map[string]*hcl.File +} + +type SequentialScheduler struct { + scheduler SpecialisedSequentialScheduler + handler packer.Handler + ui packersdk.Ui + context context.Context + hcpRegistry registry.Registry +} + +func NewSequentialScheduler( + h packer.Handler, + ui packersdk.Ui, + context context.Context, +) *SequentialScheduler { + sched := &SequentialScheduler{ + handler: h, + ui: ui, + context: context, + } + + switch handler := h.(type) { + case *packer.Core: + sched.scheduler = jsonscheduler.NewScheduler(handler) + case *hcl2template.PackerConfig: + sched.scheduler = hclscheduler.NewScheduler(handler) + } + + return sched +} + +// WriteDiags writes all the diagnostics to the Ui, and returns non-zero if some errors were contained. +func (s *SequentialScheduler) WriteDiags(diags hcl.Diagnostics) int { + return writeDiags(s.ui, s.scheduler.FileMap(), diags) +} + +func (s *SequentialScheduler) prepare(skipDatasourcesExecution bool) hcl.Diagnostics { + diags := s.scheduler.ExecuteDataSources(skipDatasourcesExecution) + + diags = diags.Extend(s.scheduler.EvaluateVariables()) + + diags = diags.Extend(s.scheduler.PrepareBuilds()) + + return diags + +} + +func (s *SequentialScheduler) Build(args *BuildArgs) int { + // For builds, we always execute all the datasources + diags := s.prepare(false) + ret := s.WriteDiags(diags) + if ret != 0 { + return ret + } + + s.hcpRegistry, diags = registry.New(s.handler, s.ui) + + defer s.hcpRegistry.IterationStatusSummary() + + err := s.hcpRegistry.PopulateIteration(s.context) + if err != nil { + s.WriteDiags(hcl.Diagnostics{ + &hcl.Diagnostic{ + Summary: "HCP: populating iteration failed", + Severity: hcl.DiagError, + Detail: err.Error(), + }}) + } + + builds, diags := s.scheduler.FilterBuilds( + args.Debug, args.Force, + args.OnError, + args.Except, args.Only, + ) + + // Here we print the errors, but we don't leave immediately if some + // valid builds remain to be executed. + // + // We only leave after this if no build remains. + s.WriteDiags(diags) + + log.Printf("Build debug mode: %v", args.Debug) + log.Printf("Force build: %v", args.Force) + log.Printf("On error: %v", args.OnError) + + if len(builds) == 0 { + return s.WriteDiags(hcl.Diagnostics{ + &hcl.Diagnostic{ + Summary: "No builds to run", + Detail: "A build command cannot run without at least one build to process. " + + "If the only or except flags have been specified at run time check that" + + " at least one build is selected for execution.", + Severity: hcl.DiagError, + }}) + } + + if args.Debug { + s.ui.Say("Debug mode enabled. Builds will not be parallelized.") + } + + // Compile all the UIs for the builds + colors := [5]packer.UiColor{ + packer.UiColorGreen, + packer.UiColorCyan, + packer.UiColorMagenta, + packer.UiColorYellow, + packer.UiColorBlue, + } + buildUis := make(map[packersdk.Build]packersdk.Ui) + for i := range builds { + ui := s.ui + if args.Color { + // Only set up UI colors if -machine-readable isn't set. + if _, ok := s.ui.(*packer.MachineReadableUi); !ok { + ui = &packer.ColoredUi{ + Color: colors[i%len(colors)], + Ui: ui, + } + ui.Say(fmt.Sprintf("%s: output will be in this color.", builds[i].Name())) + if i+1 == len(builds) { + // Add a newline between the color output and the actual output + s.ui.Say("") + } + } + } + // Now add timestamps if requested + if args.TimestampUi { + ui = &packer.TimestampedUi{ + Ui: ui, + } + } + + buildUis[builds[i]] = ui + } + + // Get the start of the build command + buildCommandStart := time.Now() + + // Run all the builds in parallel and wait for them to complete + var wg sync.WaitGroup + var artifacts = struct { + sync.RWMutex + m map[string][]packersdk.Artifact + }{m: make(map[string][]packersdk.Artifact)} + // Get the builds we care about + var errs = struct { + sync.RWMutex + m map[string]error + }{m: make(map[string]error)} + limitParallel := semaphore.NewWeighted(args.ParallelBuilds) + for i := range builds { + if err := s.context.Err(); err != nil { + log.Println("Interrupted, not going to start any more builds.") + break + } + + b := builds[i] + name := b.Name() + ui := buildUis[b] + if err := limitParallel.Acquire(s.context, 1); err != nil { + ui.Error(fmt.Sprintf("Build '%s' failed to acquire semaphore: %s", name, err)) + errs.Lock() + errs.m[name] = err + errs.Unlock() + break + } + // Increment the waitgroup so we wait for this item to finish properly + wg.Add(1) + + // Run the build in a goroutine + go func() { + // Get the start of the build + buildStart := time.Now() + + defer wg.Done() + + defer limitParallel.Release(1) + + err := s.hcpRegistry.StartBuild(s.context, b) + // Seems odd to require this error check here. Now that it is an error we can just exit with diag + if err != nil { + // If the build is already done, we skip without a warning + if errors.As(err, ®istry.ErrBuildAlreadyDone{}) { + ui.Say(fmt.Sprintf("skipping already done build %q", name)) + return + } + diags = diags.Append(&hcl.Diagnostic{ + Summary: fmt.Sprintf( + "hcp: failed to start build %q", + name), + Severity: hcl.DiagError, + Detail: err.Error(), + }) + return + } + + log.Printf("Starting build run: %s", name) + runArtifacts, err := b.Run(s.context, ui) + + // Get the duration of the build and parse it + buildEnd := time.Now() + buildDuration := buildEnd.Sub(buildStart) + fmtBuildDuration := durafmt.Parse(buildDuration).LimitFirstN(2) + + runArtifacts, hcperr := s.hcpRegistry.CompleteBuild( + s.context, + b, + runArtifacts, + err) + if hcperr != nil { + diags = diags.Append(&hcl.Diagnostic{ + Summary: fmt.Sprintf( + "failed to complete HCP-enabled build %q", + name), + Severity: hcl.DiagError, + Detail: hcperr.Error(), + }) + } + + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Build failed"), + Detail: fmt.Sprintf("Build '%s' errored after %s: %s", name, fmtBuildDuration, err), + }) + errs.Lock() + errs.m[name] = err + errs.Unlock() + } else { + ui.Say(fmt.Sprintf("Build '%s' finished after %s.", name, fmtBuildDuration)) + if runArtifacts != nil { + artifacts.Lock() + artifacts.m[name] = runArtifacts + artifacts.Unlock() + } + } + }() + + if args.Debug || args.ParallelBuilds == 1 { + log.Printf("Parallelization disabled, waiting for build to finish: %s", b.Name()) + wg.Wait() + } + } + + // Wait for both the builds to complete and the interrupt handler, + // if it is interrupted. + log.Printf("Waiting on builds to complete...") + wg.Wait() + + // Get the duration of the buildCommand command and parse it + buildCommandEnd := time.Now() + buildCommandDuration := buildCommandEnd.Sub(buildCommandStart) + fmtBuildCommandDuration := durafmt.Parse(buildCommandDuration).LimitFirstN(2) + s.ui.Say(fmt.Sprintf("\n==> Wait completed after %s", fmtBuildCommandDuration)) + + if err := s.context.Err(); err != nil { + return s.WriteDiags(hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Build cancelled", + Detail: "Cleanly cancelled builds after being interrupted.", + }}) + } + + if len(errs.m) > 0 { + s.ui.Machine("error-count", strconv.FormatInt(int64(len(errs.m)), 10)) + + s.ui.Error("\n==> Some builds didn't complete successfully and had errors:") + for name, err := range errs.m { + // Create a UI for the machine readable stuff to be targeted + ui := &packer.TargetedUI{ + Target: name, + Ui: s.ui, + } + + ui.Machine("error", err.Error()) + + s.ui.Error(fmt.Sprintf("--> %s: %s", name, err)) + } + } + + if len(artifacts.m) > 0 { + s.ui.Say("\n==> Builds finished. The artifacts of successful builds are:") + for name, buildArtifacts := range artifacts.m { + // Create a UI for the machine readable stuff to be targeted + ui := &packer.TargetedUI{ + Target: name, + Ui: s.ui, + } + + // Machine-readable helpful + ui.Machine("artifact-count", strconv.FormatInt(int64(len(buildArtifacts)), 10)) + + for i, artifact := range buildArtifacts { + var message bytes.Buffer + fmt.Fprintf(&message, "--> %s: ", name) + + if artifact != nil { + fmt.Fprint(&message, artifact.String()) + } else { + fmt.Fprint(&message, "") + } + + iStr := strconv.FormatInt(int64(i), 10) + if artifact != nil { + ui.Machine("artifact", iStr, "builder-id", artifact.BuilderId()) + ui.Machine("artifact", iStr, "id", artifact.Id()) + ui.Machine("artifact", iStr, "string", artifact.String()) + + files := artifact.Files() + ui.Machine("artifact", + iStr, + "files-count", strconv.FormatInt(int64(len(files)), 10)) + for fi, file := range files { + fiStr := strconv.FormatInt(int64(fi), 10) + ui.Machine("artifact", iStr, "file", fiStr, file) + } + } else { + ui.Machine("artifact", iStr, "nil") + } + + ui.Machine("artifact", iStr, "end") + s.ui.Say(message.String()) + + } + + } + } else { + s.ui.Say("\n==> Builds finished but no artifacts were created.") + } + + return s.WriteDiags(diags) +} From 61a3d6af989b27cdf9e6401ac1cc4d0d014a9165 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Tue, 19 Sep 2023 10:35:23 -0400 Subject: [PATCH 24/28] command: migrate validate to schedulers --- command/scheduler.go | 1 + command/sequential/json/variables.go | 11 +++++++++ command/sequential_scheduler.go | 23 +++++++++++++++++++ command/validate.go | 34 ++++------------------------ command/validate_test.go | 6 ++--- 5 files changed, 42 insertions(+), 33 deletions(-) diff --git a/command/scheduler.go b/command/scheduler.go index 0acc0c4d973..4c7c59cf917 100644 --- a/command/scheduler.go +++ b/command/scheduler.go @@ -9,6 +9,7 @@ import ( type Scheduler interface { Build(*BuildArgs) int + Validate(*ValidateArgs) int } // NewScheduler returns a new scheduler for running commands with. diff --git a/command/sequential/json/variables.go b/command/sequential/json/variables.go index 025da27e365..01a4fc078a7 100644 --- a/command/sequential/json/variables.go +++ b/command/sequential/json/variables.go @@ -185,6 +185,17 @@ func (s *JSONSequentialScheduler) getEvaluatedVariables() ([]string, error) { } func (s *JSONSequentialScheduler) EvaluateVariables() hcl.Diagnostics { + err := s.config.Validate() + if err != nil { + return hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid template", + Detail: err.Error(), + }, + } + } + secrets, err := s.getEvaluatedVariables() if err != nil { diff --git a/command/sequential_scheduler.go b/command/sequential_scheduler.go index 9a2a770d5b0..dd091c250b8 100644 --- a/command/sequential_scheduler.go +++ b/command/sequential_scheduler.go @@ -80,6 +80,29 @@ func (s *SequentialScheduler) prepare(skipDatasourcesExecution bool) hcl.Diagnos } +func (s *SequentialScheduler) Validate(args *ValidateArgs) int { + // If we're only checking syntax, then we're done already + if args.SyntaxOnly { + s.ui.Say("Syntax-only check passed. Everything looks okay.") + return 0 + } + + diags := s.prepare(!args.EvaluateDatasources) + ret := s.WriteDiags(diags) + if ret != 0 { + return ret + } + + _, diags = s.scheduler.FilterBuilds(false, false, "", args.Except, args.Only) + + fixerDiags := s.handler.FixConfig(packer.FixConfigOptions{ + Mode: packer.Diff, + }) + diags = append(diags, fixerDiags...) + + return s.WriteDiags(diags) +} + func (s *SequentialScheduler) Build(args *BuildArgs) int { // For builds, we always execute all the datasources diags := s.prepare(false) diff --git a/command/validate.go b/command/validate.go index 60199bdbf71..dfc59f0a536 100644 --- a/command/validate.go +++ b/command/validate.go @@ -7,8 +7,6 @@ import ( "context" "strings" - "github.com/hashicorp/packer/packer" - "github.com/posener/complete" ) @@ -54,44 +52,20 @@ func (c *ValidateCommand) RunContext(ctx context.Context, cla *ValidateArgs) int cla.MetaArgs.WarnOnUndeclaredVar = false } - packerStarter, ret := c.GetConfig(&cla.MetaArgs) + cfg, ret := c.GetConfig(&cla.MetaArgs) if ret != 0 { return 1 } - // If we're only checking syntax, then we're done already - if cla.SyntaxOnly { - c.Ui.Say("Syntax-only check passed. Everything looks okay.") - return 0 - } - - diags := packerStarter.DetectPluginBinaries() - ret = writeDiags(c.Ui, nil, diags) - if ret != 0 { - return ret - } - - diags = packerStarter.Initialize(packer.InitializeOptions{ - SkipDatasourcesExecution: !cla.EvaluateDatasources, - }) - bundledDiags := c.DetectBundledPlugins(packerStarter) - diags = append(bundledDiags, diags...) + diags := cfg.DetectPluginBinaries() ret = writeDiags(c.Ui, nil, diags) if ret != 0 { return ret } - _, diags = packerStarter.GetBuilds(packer.GetBuildsOptions{ - Only: cla.Only, - Except: cla.Except, - }) + sched := NewScheduler(cfg, c.Ui, ctx) + ret = sched.Validate(cla) - fixerDiags := packerStarter.FixConfig(packer.FixConfigOptions{ - Mode: packer.Diff, - }) - diags = append(diags, fixerDiags...) - - ret = writeDiags(c.Ui, nil, diags) if ret == 0 { c.Ui.Say("The configuration is valid.") } diff --git a/command/validate_test.go b/command/validate_test.go index 5b9da04d78b..f9b5110e02e 100644 --- a/command/validate_test.go +++ b/command/validate_test.go @@ -149,7 +149,7 @@ func TestValidateCommandBadVersion(t *testing.T) { } stdout, stderr := GetStdoutAndErrFromTestMeta(t, c.Meta) - expected := `Error: + expected := `Error: Invalid template This template requires Packer version 101.0.0 or higher; using 100.0.0 @@ -370,8 +370,8 @@ func TestValidateCommand_ShowLineNumForMissing(t *testing.T) { stdout, stderr := GetStdoutAndErrFromTestMeta(t, c.Meta) expected := fmt.Sprintf(`Error: Unknown source file.cho - on %s line 6: - (source code not available) + on %s line 6, in build: + 6: build { Known: [file.chocolate] From 3556c55c0986dafc7aa39909577832b725083eca Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Tue, 19 Sep 2023 14:33:35 -0400 Subject: [PATCH 25/28] command: move inspect to schedulers --- command/inspect.go | 8 +------- command/scheduler.go | 1 + command/sequential_scheduler.go | 6 ++++++ 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/command/inspect.go b/command/inspect.go index 7a77ab76bb8..682892fad7d 100644 --- a/command/inspect.go +++ b/command/inspect.go @@ -7,7 +7,6 @@ import ( "context" "strings" - "github.com/hashicorp/packer/packer" "github.com/posener/complete" ) @@ -48,12 +47,7 @@ func (c *InspectCommand) RunContext(ctx context.Context, cla *InspectArgs) int { return ret } - // here we ignore init diags to allow unknown variables to be used - _ = packerStarter.Initialize(packer.InitializeOptions{}) - - return packerStarter.InspectConfig(packer.InspectConfigOptions{ - Ui: c.Ui, - }) + return NewScheduler(packerStarter, c.Ui, ctx).Inspect(cla) } func (*InspectCommand) Help() string { diff --git a/command/scheduler.go b/command/scheduler.go index 4c7c59cf917..d3400a1140f 100644 --- a/command/scheduler.go +++ b/command/scheduler.go @@ -10,6 +10,7 @@ import ( type Scheduler interface { Build(*BuildArgs) int Validate(*ValidateArgs) int + Inspect(*InspectArgs) int } // NewScheduler returns a new scheduler for running commands with. diff --git a/command/sequential_scheduler.go b/command/sequential_scheduler.go index dd091c250b8..c45ffaf3f55 100644 --- a/command/sequential_scheduler.go +++ b/command/sequential_scheduler.go @@ -77,7 +77,13 @@ func (s *SequentialScheduler) prepare(skipDatasourcesExecution bool) hcl.Diagnos diags = diags.Extend(s.scheduler.PrepareBuilds()) return diags +} +func (s *SequentialScheduler) Inspect(args *InspectArgs) int { + s.prepare(false) + return s.handler.InspectConfig(packer.InspectConfigOptions{ + Ui: s.ui, + }) } func (s *SequentialScheduler) Validate(args *ValidateArgs) int { From 3be2dea4144806bb56aca4b0e9c464d8a77b8a84 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Tue, 19 Sep 2023 14:45:36 -0400 Subject: [PATCH 26/28] command: move console to schedulers --- command/console.go | 2 +- command/scheduler.go | 1 + command/sequential_scheduler.go | 5 +++++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/command/console.go b/command/console.go index f942ec4bd4f..6b9f4b0b20f 100644 --- a/command/console.go +++ b/command/console.go @@ -63,7 +63,7 @@ func (c *ConsoleCommand) RunContext(ctx context.Context, cla *ConsoleArgs) int { return ret } - _ = packerStarter.Initialize(packer.InitializeOptions{}) + NewScheduler(packerStarter, c.Ui, ctx).Console(cla) // Determine if stdin is a pipe. If so, we evaluate directly. if c.StdinPiped() { diff --git a/command/scheduler.go b/command/scheduler.go index d3400a1140f..0ab5963ade1 100644 --- a/command/scheduler.go +++ b/command/scheduler.go @@ -11,6 +11,7 @@ type Scheduler interface { Build(*BuildArgs) int Validate(*ValidateArgs) int Inspect(*InspectArgs) int + Console(*ConsoleArgs) int } // NewScheduler returns a new scheduler for running commands with. diff --git a/command/sequential_scheduler.go b/command/sequential_scheduler.go index c45ffaf3f55..badbc8a3d6b 100644 --- a/command/sequential_scheduler.go +++ b/command/sequential_scheduler.go @@ -79,6 +79,11 @@ func (s *SequentialScheduler) prepare(skipDatasourcesExecution bool) hcl.Diagnos return diags } +func (s *SequentialScheduler) Console(args *ConsoleArgs) int { + s.prepare(false) + return 0 +} + func (s *SequentialScheduler) Inspect(args *InspectArgs) int { s.prepare(false) return s.handler.InspectConfig(packer.InspectConfigOptions{ From 9e0c3546be1434e73419e4ca922589ebb5486fd3 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Tue, 19 Sep 2023 15:58:33 -0400 Subject: [PATCH 27/28] command: migrate hcl2_upgrade to schedulers As the last command that relies on the deprecated interfaces for interacting with a template, we move hcl2_upgrade to the schedulers, at least for the part that prepares the template for translation. --- command/hcl2_upgrade.go | 6 ++---- command/scheduler.go | 1 + command/sequential_scheduler.go | 10 ++++++++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/command/hcl2_upgrade.go b/command/hcl2_upgrade.go index 7802d409875..3353f4cb1d1 100644 --- a/command/hcl2_upgrade.go +++ b/command/hcl2_upgrade.go @@ -135,7 +135,7 @@ type BlockParser interface { Write(*bytes.Buffer) } -func (c *HCL2UpgradeCommand) RunContext(_ context.Context, cla *HCL2UpgradeArgs) int { +func (c *HCL2UpgradeCommand) RunContext(ctx context.Context, cla *HCL2UpgradeArgs) int { var output io.Writer if err := os.MkdirAll(filepath.Dir(cla.OutputFile), 0755); err != nil { c.Ui.Error(fmt.Sprintf("Failed to create output directory: %v", err)) @@ -163,9 +163,7 @@ func (c *HCL2UpgradeCommand) RunContext(_ context.Context, cla *HCL2UpgradeArgs) } core := hdl.(*packer.Core) - if err := core.Initialize(packer.InitializeOptions{}); err != nil { - c.Ui.Error(fmt.Sprintf("Ignoring following initialization error: %v", err)) - } + NewScheduler(core, c.Ui, ctx).HCL2Upgrade(cla) tpl := core.Template // Parse blocks diff --git a/command/scheduler.go b/command/scheduler.go index 0ab5963ade1..24b86db1db8 100644 --- a/command/scheduler.go +++ b/command/scheduler.go @@ -12,6 +12,7 @@ type Scheduler interface { Validate(*ValidateArgs) int Inspect(*InspectArgs) int Console(*ConsoleArgs) int + HCL2Upgrade(*HCL2UpgradeArgs) int } // NewScheduler returns a new scheduler for running commands with. diff --git a/command/sequential_scheduler.go b/command/sequential_scheduler.go index badbc8a3d6b..a4c432cd650 100644 --- a/command/sequential_scheduler.go +++ b/command/sequential_scheduler.go @@ -79,6 +79,16 @@ func (s *SequentialScheduler) prepare(skipDatasourcesExecution bool) hcl.Diagnos return diags } +func (s *SequentialScheduler) HCL2Upgrade(args *HCL2UpgradeArgs) int { + // No need to execute the datasources in this case, as we rewrite them + // in HCL, and there are no datasources in JSON templates. + s.prepare(true) + // Preparing builds is safe here as well, since we don't risk encountering + // any dynamic blocks in JSON templates. + s.scheduler.PrepareBuilds() + return 0 +} + func (s *SequentialScheduler) Console(args *ConsoleArgs) int { s.prepare(false) return 0 From 31c4925fba432c6cabec742c6862aa9db10e5903 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Tue, 19 Sep 2023 14:49:56 -0400 Subject: [PATCH 28/28] packer: remove Initialise/GetBuilds from handler Since the Initialise/GetBuilds functions are not used anymore by Packer's commands directly, as they are superseded by the scheduler's implementations, we can remove them from the Handler interface. --- packer/run_interfaces.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packer/run_interfaces.go b/packer/run_interfaces.go index 07829e629e1..fa215322fd1 100644 --- a/packer/run_interfaces.go +++ b/packer/run_interfaces.go @@ -20,13 +20,6 @@ type GetBuildsOptions struct { ExceptMatches, OnlyMatches int } -type BuildGetter interface { - // GetBuilds return all possible builds for a config. It also starts all - // builders. - // TODO(azr): rename to builder starter ? - GetBuilds(GetBuildsOptions) ([]packersdk.Build, hcl.Diagnostics) -} - type Evaluator interface { // EvaluateExpression is meant to be used in the `packer console` command. // It parses the input string and returns what needs to be displayed. In @@ -51,12 +44,10 @@ type PluginBinaryDetector interface { // run a build we will start the builds and then the core of Packer handles // execution. type Handler interface { - Initialize(InitializeOptions) hcl.Diagnostics // PluginRequirements returns the list of plugin Requirements from the // config file. PluginRequirements() (plugingetter.Requirements, hcl.Diagnostics) Evaluator - BuildGetter ConfigFixer ConfigInspector PluginBinaryDetector