From fe12d53e777680a4a8c443cdef142bba23366ff8 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 11:32:20 +0100 Subject: [PATCH 01/17] addr: remove support for defaulting plugin namespace and host --- hcl2template/addrs/plugin.go | 71 +++++++++++++------------------ hcl2template/addrs/plugin_test.go | 32 ++++++++++++++ 2 files changed, 61 insertions(+), 42 deletions(-) create mode 100644 hcl2template/addrs/plugin_test.go diff --git a/hcl2template/addrs/plugin.go b/hcl2template/addrs/plugin.go index 88aa53fc2..f26ee796e 100644 --- a/hcl2template/addrs/plugin.go +++ b/hcl2template/addrs/plugin.go @@ -10,17 +10,13 @@ import ( // Plugin encapsulates a single plugin type. type Plugin struct { - Type string - Namespace string Hostname string + Namespace string + Type string } func (p Plugin) RealRelativePath() string { - ns := DefaultPluginNamespace - if p.Namespace != "" { - ns = p.Namespace - } - return ns + "/packer-plugin-" + p.Type + return p.Namespace + "/packer-plugin-" + p.Type } func (p Plugin) Parts() []string { @@ -34,20 +30,23 @@ func (p Plugin) String() string { // ForDisplay returns a user-friendly FQN string, simplified for readability. If // the plugin is using the default hostname, the hostname is omitted. func (p *Plugin) ForDisplay() string { + const ( + // These will be hidden if they are a prefix + DefaultHashicorpPluginHost = "github.com" + DefaultHashicorpPluginNamespace = "hashicorp" + ) + parts := []string{} - if p.Hostname != DefaultPluginHost { + if p.Hostname != DefaultHashicorpPluginHost { parts = append(parts, p.Hostname) } - if p.Namespace != DefaultPluginNamespace { + if p.Namespace != DefaultHashicorpPluginNamespace { parts = append(parts, p.Namespace) } parts = append(parts, p.Type) return strings.Join(parts, "/") } -const DefaultPluginHost = "github.com" -const DefaultPluginNamespace = "hashicorp" - // ParsePluginPart processes an addrs.Plugin namespace or type string // provided by an end-user, producing a normalized version if possible or // an error if the string contains invalid characters. @@ -120,18 +119,18 @@ func IsPluginPartNormalized(str string) (bool, error) { // hostname/namespace/name func ParsePluginSourceString(str string) (*Plugin, hcl.Diagnostics) { ret := &Plugin{ - Hostname: DefaultPluginHost, - Namespace: DefaultPluginNamespace, + Hostname: "", + Namespace: "", } var diags hcl.Diagnostics // split the source string into individual components parts := strings.Split(str, "/") - if len(parts) == 0 || len(parts) > 3 { + if len(parts) != 3 { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid plugin source string", - Detail: `The "source" attribute must be in the format "[hostname/][namespace/]name"`, + Detail: `The "source" attribute must be in the format "hostname/namespace/name"`, }) return nil, diags } @@ -142,7 +141,7 @@ func ParsePluginSourceString(str string) (*Plugin, hcl.Diagnostics) { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid plugin source string", - Detail: `The "source" attribute must be in the format "[hostname/][namespace/]name"`, + Detail: `The "source" attribute must be in the format "hostname/namespace/name"`, }) return nil, diags } @@ -161,33 +160,21 @@ func ParsePluginSourceString(str string) (*Plugin, hcl.Diagnostics) { } ret.Type = name - if len(parts) == 1 { - return ret, diags - } - - if len(parts) >= 2 { - // the namespace is always the second-to-last part - givenNamespace := parts[len(parts)-2] - namespace, err := ParsePluginPart(givenNamespace) - if err != nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid plugin namespace", - Detail: fmt.Sprintf(`Invalid plugin namespace %q in source %q: %s"`, namespace, str, err), - }) - return nil, diags - } - ret.Namespace = namespace + // the namespace is always the second-to-last part + givenNamespace := parts[len(parts)-2] + namespace, err := ParsePluginPart(givenNamespace) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid plugin namespace", + Detail: fmt.Sprintf(`Invalid plugin namespace %q in source %q: %s"`, namespace, str, err), + }) + return nil, diags } + ret.Namespace = namespace - // Final Case: 3 parts - if len(parts) == 3 { - // the hostname is always the first part in a three-part source string - hostname := parts[0] - // TODO(azr): validate host ? Can this be something else than a - // github.com host for now? - ret.Hostname = hostname - } + // the hostname is always the first part in a three-part source string + ret.Hostname = parts[0] // Due to how plugin executables are named and plugin git repositories // are conventionally named, it's a reasonable and diff --git a/hcl2template/addrs/plugin_test.go b/hcl2template/addrs/plugin_test.go new file mode 100644 index 000000000..191f103fa --- /dev/null +++ b/hcl2template/addrs/plugin_test.go @@ -0,0 +1,32 @@ +package addrs + +import ( + "reflect" + "testing" +) + +func TestParsePluginSourceString(t *testing.T) { + type args struct { + str string + } + tests := []struct { + args args + want *Plugin + wantDiags bool + }{ + {args{"potato"}, nil, true}, + {args{"hashicorp/azr"}, nil, true}, + {args{"github.com/hashicorp/azr"}, &Plugin{"github.com", "hashicorp", "azr"}, false}, + } + for _, tt := range tests { + t.Run(tt.args.str, func(t *testing.T) { + got, gotDiags := ParsePluginSourceString(tt.args.str) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ParsePluginSourceString() got = %v, want %v", got, tt.want) + } + if tt.wantDiags == (len(gotDiags) == 0) { + t.Errorf("Unexpected diags %s", gotDiags) + } + }) + } +} From 9f545c28feacb6760542b8aaaa636dadbee53b32 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 11:47:44 +0100 Subject: [PATCH 02/17] required_plugins: prevent using `plugin = "version"`, and show an example --- hcl2template/types.required_plugins.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/hcl2template/types.required_plugins.go b/hcl2template/types.required_plugins.go index 072966021..b39558e1d 100644 --- a/hcl2template/types.required_plugins.go +++ b/hcl2template/types.required_plugins.go @@ -96,18 +96,23 @@ func decodeRequiredPluginsBlock(block *hcl.Block) (*RequiredPlugins, hcl.Diagnos switch { case expr.Type().IsPrimitiveType(): - vc, reqDiags := decodeVersionConstraint(attr) - diags = append(diags, reqDiags...) - rp.Requirement = vc - rp.Type, err = addrs.ParsePluginSourceString(name) - if err != nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid plugin type", - Detail: fmt.Sprintf(`Invalid plugin type %q: %s"`, name, err), - }) + c := "version" + if cs, _ := decodeVersionConstraint(attr); len(cs.Required) > 0 { + c = cs.Required.String() } + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid plugin requirement", + Detail: fmt.Sprintf(`'%s = "%s"' plugin requirement calls are not possible.`+ + ` You must define a whole block. For example:`+"\n"+ + `%[1]s = {`+"\n"+ + ` source = github.com/hashicorp/%[1]s`+"\n"+ + ` version = %[2]s`+"\n"+`}`, + name, c), + Subject: attr.Range.Ptr(), + }) + case expr.Type().IsObjectType(): if !expr.Type().HasAttribute("version") { diags = append(diags, &hcl.Diagnostic{ From aeecfcd422f225f6f45a384de9b3b69efafc8db7 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 11:49:02 +0100 Subject: [PATCH 03/17] show version constrain error in case it's handy --- hcl2template/types.required_plugins.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hcl2template/types.required_plugins.go b/hcl2template/types.required_plugins.go index b39558e1d..8fc66064b 100644 --- a/hcl2template/types.required_plugins.go +++ b/hcl2template/types.required_plugins.go @@ -145,8 +145,10 @@ func decodeRequiredPluginsBlock(block *hcl.Block) (*RequiredPlugins, hcl.Diagnos diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid version constraint", - Detail: "This string does not use correct version constraint syntax. See https://www.packer.io/docs/templates/hcl_templates/blocks/packer#version-constraint-syntax for docs.", - Subject: attr.Expr.Range().Ptr(), + Detail: "This string does not use correct version constraint syntax. " + + "See https://www.packer.io/docs/templates/hcl_templates/blocks/packer#version-constraint-syntax for docs.\n" + + err.Error(), + Subject: attr.Expr.Range().Ptr(), }) continue } From 029729225dad9c296c8dc369aa88f2ea778e8bdf Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 12:21:10 +0100 Subject: [PATCH 04/17] tests and fixes --- .../duplicate_required_plugins/packer.pkr.hcl | 10 ++- .../testdata/init/imports/build.pkr.hcl | 19 +++-- .../init/invalid_inexplicit_source.pkr.hcl | 8 +++ .../init/invalid_inexplicit_source_2.pkr.hcl | 8 +++ .../init/invalid_short_source.pkr.hcl | 5 ++ hcl2template/types.packer_config_test.go | 70 +++++++++++++++++-- hcl2template/types.required_plugins.go | 13 +++- hcl2template/types.variables.go | 3 +- 8 files changed, 114 insertions(+), 22 deletions(-) create mode 100644 hcl2template/testdata/init/invalid_inexplicit_source.pkr.hcl create mode 100644 hcl2template/testdata/init/invalid_inexplicit_source_2.pkr.hcl create mode 100644 hcl2template/testdata/init/invalid_short_source.pkr.hcl diff --git a/hcl2template/testdata/init/duplicate_required_plugins/packer.pkr.hcl b/hcl2template/testdata/init/duplicate_required_plugins/packer.pkr.hcl index b60e10ac0..1cf3f291e 100644 --- a/hcl2template/testdata/init/duplicate_required_plugins/packer.pkr.hcl +++ b/hcl2template/testdata/init/duplicate_required_plugins/packer.pkr.hcl @@ -1,7 +1,13 @@ packer { required_plugins { - amazon = ">= v0" - amazon = ">= v4" + amazon = { + source = "github.com/hashicorp/amazon" + version = ">= v0" + } + amazon = { + source = "github.com/hashicorp/amazon" + version = ">= v4" + } } } \ No newline at end of file diff --git a/hcl2template/testdata/init/imports/build.pkr.hcl b/hcl2template/testdata/init/imports/build.pkr.hcl index 453fa045e..7220f3b93 100644 --- a/hcl2template/testdata/init/imports/build.pkr.hcl +++ b/hcl2template/testdata/init/imports/build.pkr.hcl @@ -3,29 +3,26 @@ packer { required_version = ">= v1" required_plugins { - amazon = ">= v0" - + amazon = { + source = "github.com/hashicorp/amazon" + version = ">= v0" + } amazon-v1 = { - source = "amazon" + source = "github.com/hashicorp/amazon" version = ">= v1" } - amazon-v2 = { - source = "amazon" + source = "github.com/hashicorp/amazon" version = ">= v2" } - - amazon-v3 = { - source = "hashicorp/amazon" + source = "github.com/hashicorp/amazon" version = ">= v3" } - amazon-v3-azr = { - source = "azr/amazon" + source = "github.com/azr/amazon" version = ">= v3" } - amazon-v4 = { source = "github.com/hashicorp/amazon" version = ">= v4" diff --git a/hcl2template/testdata/init/invalid_inexplicit_source.pkr.hcl b/hcl2template/testdata/init/invalid_inexplicit_source.pkr.hcl new file mode 100644 index 000000000..3c84ffc87 --- /dev/null +++ b/hcl2template/testdata/init/invalid_inexplicit_source.pkr.hcl @@ -0,0 +1,8 @@ +packer { + required_plugins { + amazon = { + source = "amazon" + version = ">= v0" + } + } +} \ No newline at end of file diff --git a/hcl2template/testdata/init/invalid_inexplicit_source_2.pkr.hcl b/hcl2template/testdata/init/invalid_inexplicit_source_2.pkr.hcl new file mode 100644 index 000000000..3e71b828e --- /dev/null +++ b/hcl2template/testdata/init/invalid_inexplicit_source_2.pkr.hcl @@ -0,0 +1,8 @@ +packer { + required_plugins { + amazon = { + source = "hashicorp/amazon" + version = ">= v0" + } + } +} \ No newline at end of file diff --git a/hcl2template/testdata/init/invalid_short_source.pkr.hcl b/hcl2template/testdata/init/invalid_short_source.pkr.hcl new file mode 100644 index 000000000..13ea61a42 --- /dev/null +++ b/hcl2template/testdata/init/invalid_short_source.pkr.hcl @@ -0,0 +1,5 @@ +packer { + required_plugins { + amazon = ">= v0" + } +} \ No newline at end of file diff --git a/hcl2template/types.packer_config_test.go b/hcl2template/types.packer_config_test.go index 87c7825bf..a9a0488e6 100644 --- a/hcl2template/types.packer_config_test.go +++ b/hcl2template/types.packer_config_test.go @@ -431,7 +431,7 @@ func TestParser_no_init(t *testing.T) { RequiredPlugins: map[string]*RequiredPlugin{ "amazon": { Name: "amazon", - Source: "", + Source: "github.com/hashicorp/amazon", Type: &addrs.Plugin{ Type: "amazon", Namespace: "hashicorp", @@ -443,7 +443,7 @@ func TestParser_no_init(t *testing.T) { }, "amazon-v1": { Name: "amazon-v1", - Source: "amazon", + Source: "github.com/hashicorp/amazon", Type: &addrs.Plugin{ Type: "amazon", Namespace: "hashicorp", @@ -455,7 +455,7 @@ func TestParser_no_init(t *testing.T) { }, "amazon-v2": { Name: "amazon-v2", - Source: "amazon", + Source: "github.com/hashicorp/amazon", Type: &addrs.Plugin{ Type: "amazon", Namespace: "hashicorp", @@ -467,7 +467,7 @@ func TestParser_no_init(t *testing.T) { }, "amazon-v3": { Name: "amazon-v3", - Source: "hashicorp/amazon", + Source: "github.com/hashicorp/amazon", Type: &addrs.Plugin{ Type: "amazon", Namespace: "hashicorp", @@ -479,7 +479,7 @@ func TestParser_no_init(t *testing.T) { }, "amazon-v3-azr": { Name: "amazon-v3-azr", - Source: "azr/amazon", + Source: "github.com/azr/amazon", Type: &addrs.Plugin{ Type: "amazon", Namespace: "azr", @@ -610,6 +610,66 @@ func TestParser_no_init(t *testing.T) { []packersdk.Build{}, false, }, + {"invalid_inexplicit_source.pkr.hcl", + defaultParser, + parseTestArgs{"testdata/init/invalid_inexplicit_source.pkr.hcl", nil, nil}, + &PackerConfig{ + Packer: struct { + VersionConstraints []VersionConstraint + RequiredPlugins []*RequiredPlugins + }{ + VersionConstraints: nil, + RequiredPlugins: []*RequiredPlugins{ + {}, + }, + }, + CorePackerVersionString: lockedVersion, + Basedir: "testdata/init", + }, + true, true, + []packersdk.Build{}, + false, + }, + {"invalid_short_source.pkr.hcl", + defaultParser, + parseTestArgs{"testdata/init/invalid_short_source.pkr.hcl", nil, nil}, + &PackerConfig{ + Packer: struct { + VersionConstraints []VersionConstraint + RequiredPlugins []*RequiredPlugins + }{ + VersionConstraints: nil, + RequiredPlugins: []*RequiredPlugins{ + {}, + }, + }, + CorePackerVersionString: lockedVersion, + Basedir: "testdata/init", + }, + true, true, + []packersdk.Build{}, + false, + }, + {"invalid_short_source_2.pkr.hcl", + defaultParser, + parseTestArgs{"testdata/init/invalid_inexplicit_source_2.pkr.hcl", nil, nil}, + &PackerConfig{ + Packer: struct { + VersionConstraints []VersionConstraint + RequiredPlugins []*RequiredPlugins + }{ + VersionConstraints: nil, + RequiredPlugins: []*RequiredPlugins{ + {}, + }, + }, + CorePackerVersionString: lockedVersion, + Basedir: "testdata/init", + }, + true, true, + []packersdk.Build{}, + false, + }, } testParse_only_Parse(t, tests) } diff --git a/hcl2template/types.required_plugins.go b/hcl2template/types.required_plugins.go index 8fc66064b..3e2ae4b3f 100644 --- a/hcl2template/types.required_plugins.go +++ b/hcl2template/types.required_plugins.go @@ -62,7 +62,11 @@ func (cfg *PackerConfig) decodeImplicitRequiredPluginsBlocks(f *hcl.File) hcl.Di // RequiredPlugin represents a declaration of a dependency on a particular // Plugin version or source. type RequiredPlugin struct { - Name string + Name string + // Source used to be able to tell how the template referenced this source, + // for example, "awesomecloud" instead of github.com/awesome/awesomecloud. + // This one is left here in case we want to go back to allowing inexplicit + // source url definitions. Source string Type *addrs.Plugin Requirement VersionConstraint @@ -77,7 +81,7 @@ type RequiredPlugins struct { func decodeRequiredPluginsBlock(block *hcl.Block) (*RequiredPlugins, hcl.Diagnostics) { attrs, diags := block.Body.JustAttributes() ret := &RequiredPlugins{ - RequiredPlugins: make(map[string]*RequiredPlugin), + RequiredPlugins: nil, DeclRange: block.DefRange, } for name, attr := range attrs { @@ -112,6 +116,7 @@ func decodeRequiredPluginsBlock(block *hcl.Block) (*RequiredPlugins, hcl.Diagnos name, c), Subject: attr.Range.Ptr(), }) + continue case expr.Type().IsObjectType(): if !expr.Type().HasAttribute("version") { @@ -186,6 +191,7 @@ func decodeRequiredPluginsBlock(block *hcl.Block) (*RequiredPlugins, hcl.Diagnos } } diags = append(diags, sourceDiags...) + continue } else { rp.Type = p } @@ -214,6 +220,9 @@ func decodeRequiredPluginsBlock(block *hcl.Block) (*RequiredPlugins, hcl.Diagnos }) } + if ret.RequiredPlugins == nil { + ret.RequiredPlugins = make(map[string]*RequiredPlugin) + } ret.RequiredPlugins[rp.Name] = rp } diff --git a/hcl2template/types.variables.go b/hcl2template/types.variables.go index 217ead998..750869f9b 100644 --- a/hcl2template/types.variables.go +++ b/hcl2template/types.variables.go @@ -82,10 +82,9 @@ func (v *Variable) GoString() string { // validateValue ensures that all of the configured custom validations for a // variable value are passing. -// func (v *Variable) validateValue(val VariableAssignment) (diags hcl.Diagnostics) { if len(v.Validations) == 0 { - log.Printf("[TRACE] validateValue: not active for %s, so skipping", v.Name) + // log.Printf("[TRACE] validateValue: not active for %s, so skipping", v.Name) return nil } From 4cb94a67b06d93b57df40626f68133d781e592c6 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 12:25:45 +0100 Subject: [PATCH 05/17] Update types.variables.go --- hcl2template/types.variables.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hcl2template/types.variables.go b/hcl2template/types.variables.go index 750869f9b..b56b5f5a7 100644 --- a/hcl2template/types.variables.go +++ b/hcl2template/types.variables.go @@ -84,7 +84,7 @@ func (v *Variable) GoString() string { // variable value are passing. func (v *Variable) validateValue(val VariableAssignment) (diags hcl.Diagnostics) { if len(v.Validations) == 0 { - // log.Printf("[TRACE] validateValue: not active for %s, so skipping", v.Name) + log.Printf("[TRACE] validateValue: not active for %s, so skipping", v.Name) return nil } From 632e918c52ef5695708d302770b0b7b122412699 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 13:38:22 +0100 Subject: [PATCH 06/17] Update plugins_test.go --- packer/plugin-getter/plugins_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packer/plugin-getter/plugins_test.go b/packer/plugin-getter/plugins_test.go index 0fc30fb0a..bbf9745d6 100644 --- a/packer/plugin-getter/plugins_test.go +++ b/packer/plugin-getter/plugins_test.go @@ -43,7 +43,7 @@ func TestPlugin_ListInstallations(t *testing.T) { { "darwin_amazon_prot_5.0", fields{ - Identifier: "amazon", + Identifier: "github.com/hashicorp/amazon", }, ListInstallationsOptions{ []string{ @@ -80,7 +80,7 @@ func TestPlugin_ListInstallations(t *testing.T) { { "darwin_amazon_prot_5.1", fields{ - Identifier: "amazon", + Identifier: "github.com/hashicorp/amazon", }, ListInstallationsOptions{ []string{ @@ -121,7 +121,7 @@ func TestPlugin_ListInstallations(t *testing.T) { { "windows_amazon", fields{ - Identifier: "amazon", + Identifier: "github.com/hashicorp/amazon", }, ListInstallationsOptions{ []string{ @@ -159,7 +159,7 @@ func TestPlugin_ListInstallations(t *testing.T) { { "windows_google_multifolder", fields{ - Identifier: "hashicorp/google", + Identifier: "github.com/hashicorp/google", }, ListInstallationsOptions{ []string{ @@ -542,7 +542,7 @@ func TestRequirement_InstallLatest(t *testing.T) { t.Run(tt.name, func(t *testing.T) { log.Printf("starting %s test", tt.name) - identifier, diags := addrs.ParsePluginSourceString(tt.fields.Identifier) + identifier, diags := addrs.ParsePluginSourceString("github.com/hashicorp/" + tt.fields.Identifier) if len(diags) != 0 { t.Fatalf("ParsePluginSourceString(%q): %v", tt.fields.Identifier, diags) } From 7809242f417c2ac4287e75f30892e2c89020bd28 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 13:41:23 +0100 Subject: [PATCH 07/17] quote template example --- hcl2template/types.required_plugins.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hcl2template/types.required_plugins.go b/hcl2template/types.required_plugins.go index 3e2ae4b3f..736aeb2c1 100644 --- a/hcl2template/types.required_plugins.go +++ b/hcl2template/types.required_plugins.go @@ -111,8 +111,8 @@ func decodeRequiredPluginsBlock(block *hcl.Block) (*RequiredPlugins, hcl.Diagnos Detail: fmt.Sprintf(`'%s = "%s"' plugin requirement calls are not possible.`+ ` You must define a whole block. For example:`+"\n"+ `%[1]s = {`+"\n"+ - ` source = github.com/hashicorp/%[1]s`+"\n"+ - ` version = %[2]s`+"\n"+`}`, + ` source = "github.com/hashicorp/%[1]s"`+"\n"+ + ` version = "%[2]s"`+"\n"+`}`, name, c), Subject: attr.Range.Ptr(), }) From 72e4dc4cb583517f24c1ec32fe2488b24fb2bf9c Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 13:58:58 +0100 Subject: [PATCH 08/17] update docs to remove 'magic' required_plugin block usages --- hcl2template/parser.go | 4 +- packer/plugin-getter/plugins.go | 2 +- website/content/docs/commands/init.mdx | 9 ++--- website/content/docs/plugins/index.mdx | 39 ++++++++----------- .../templates/hcl_templates/blocks/packer.mdx | 5 +-- .../partials/plugins/plugin-location.mdx | 16 ++++---- 6 files changed, 33 insertions(+), 42 deletions(-) diff --git a/hcl2template/parser.go b/hcl2template/parser.go index 15c6cc37c..6a14fba13 100644 --- a/hcl2template/parser.go +++ b/hcl2template/parser.go @@ -156,7 +156,9 @@ func (p *Parser) Parse(filename string, varFiles []string, argVars map[string]st // equivalent of having : // packer { // required_plugins { - // amazon = "latest" + // amazon = { + // version = "latest" + // source = "github.com/hashicorp/amazon" // } // } // Note: using `latest` ( or actually an empty string ) in a config file diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index c7909174c..7fdcdac96 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -27,7 +27,7 @@ type Requirements []*Requirement type Requirement struct { // Plugin accessor as defined in the config file. // For Packer, using : - // required_plugins { amazon = ">= v0" } + // required_plugins { amazon = {...} } // Will set Accessor to `amazon`. Accessor string diff --git a/website/content/docs/commands/init.mdx b/website/content/docs/commands/init.mdx index 6d1e7c948..e0875474b 100644 --- a/website/content/docs/commands/init.mdx +++ b/website/content/docs/commands/init.mdx @@ -44,11 +44,10 @@ block : ```hcl packer { required_plugins { - myawesomecloud = { + happycloud = { version = ">= 2.7.0" - source = "azr/myawesomecloud" + source = "azr/happycloud" } - happycloud = ">= 2.7.0" } } ``` @@ -57,8 +56,8 @@ HashiCorp does not officially verify third party Packer plugins, plugins not und ## Plugin Selection Plugin selection depends on the source and version constraints defined within the `required_plugins` block. -For each of the required plugins Packer will query the source repository `azr/myawesomecloud` whose fully qualified address -is `https://github.com/azr/packer-plugin-myawesomecloud` for a plugin matching the version constraints for the host operating system. +For each of the required plugins Packer will query the source repository `github.com/azr/happycloud` whose fully qualified address +is `https://github.com/azr/packer-plugin-happycloud` for a plugin matching the version constraints for the host operating system. Packer init will install the latest found version matching the version selection in the `required_plugins` section. Make sure to set a correct [version diff --git a/website/content/docs/plugins/index.mdx b/website/content/docs/plugins/index.mdx index 0e333a0e0..0852f5ea9 100644 --- a/website/content/docs/plugins/index.mdx +++ b/website/content/docs/plugins/index.mdx @@ -76,11 +76,11 @@ Here is an example `required_plugins` block: required_plugins { myawesomecloud = { version = ">= 2.7.0" - source = "azr/myawesomecloud" + source = "github.com/azr/myawesomecloud" } happycloud = { version = ">= 1.1.3" - source = "azr/happycloud" + source = "github.com/azr/happycloud" } } } @@ -130,7 +130,7 @@ If we change the required_plugins block to use a different local name "foo": required_plugins { foo = { version = ">= 2.7.0" - source = "azr/myawesomecloud" + source = "github.com/azr/myawesomecloud" } } ``` @@ -151,27 +151,23 @@ to download it. Source addresses consist of three parts delimited by slashes (`/`), as follows: -`[/]/` +`//` -* **Hostname** (optional): The hostname of the location/service that - distributes the plugin. If omitted, this defaults to - `github.com`, we recommend explicitly setting the hostname. Currently, the - only valid "hostname" is github.com, but we plan to eventually support plugins - downloaded from other domains. +* **Hostname:** The hostname of the location/service that + distributes the plugin. Currently, the only valid "hostname" is github.com, + but we plan to eventually support plugins downloaded from other domains. * **Namespace:** An organizational namespace within the specified host. - This often is the organization that publishes the plugin. If omitted, this - defaults to `hashicorp`. We recommend explicitly setting the namespace. + This often is the organization that publishes the plugin. * **Type:** A short name for the platform or system the plugin manages. The type is usually the plugin's preferred local name. For example, the fictional `myawesomecloud` plugin could belong to the `hashicorp` namespace on `github.com`, so its `source` could be -`github.com/hashicorp/myawesomecloud`, `hashicorp/myawesomecloud` or -`myawesomecloud`. Note: the actual _repository_ that myawesomecloud comes from -must always have the name format -`www.github.com/hashicorp/packer-plugin-myawesomecloud`, but the +`github.com/hashicorp/myawesomecloud`, +Note: the actual _repository_ that myawesomecloud comes from must always have +the name format `github.com/hashicorp/packer-plugin-myawesomecloud`, but the `required_plugins` block omits the redundant `packer-plugin-` repository prefix for brevity. @@ -179,9 +175,8 @@ The source address with all three components given explicitly is called the plugin's _fully-qualified address_. You will see fully-qualified address in various outputs, like error messages, but in most cases a simplified display version is used. Therefore you may see the shortened version `"myawesomecloud"` -instead of `"github.com/hashicorp/myawesomecloud"`. - --> **Note:** We recommend using explicit source addresses for all plugins. +instead of `"github.com/hashicorp/myawesomecloud"`. This will happen only when +the prefix is `github.com/hashicorp`. ## Plugin location @@ -192,17 +187,15 @@ instead of `"github.com/hashicorp/myawesomecloud"`. Using the following example : ```hcl required_plugins { - myawesomecloud = { + happycloud = { version = ">= 2.7.0" - source = "azr/myawesomecloud" + source = "github.com/azr/happycloud" } - happycloud = ">= 2.7.0" } ``` The plugin getter will look for plugins located at: -* github.com/azr/packer-plugin-myawesomecloud -* github.com/hashicorp/packer-plugin-happycloud +* github.com/azr/packer-plugin-happycloud Packer will error if you set the `packer-plugin-` prefix in a `source`. This will avoid conflicting with other plugins for other tools, like Terraform. diff --git a/website/content/docs/templates/hcl_templates/blocks/packer.mdx b/website/content/docs/templates/hcl_templates/blocks/packer.mdx index b68bd1226..b88d761b8 100644 --- a/website/content/docs/templates/hcl_templates/blocks/packer.mdx +++ b/website/content/docs/templates/hcl_templates/blocks/packer.mdx @@ -59,11 +59,10 @@ version constraint. ```hcl packer { required_plugins { - myawesomecloud = { + happycloud = { version = ">= 2.7.0" - source = "hashicorp/myawesomecloud" + source = "github.com/hashicorp/happycloud" } - happycloud = ">= 2.7.0" } } ``` diff --git a/website/content/partials/plugins/plugin-location.mdx b/website/content/partials/plugins/plugin-location.mdx index 76ab8f6e6..3935c2bf9 100644 --- a/website/content/partials/plugins/plugin-location.mdx +++ b/website/content/partials/plugins/plugin-location.mdx @@ -24,28 +24,26 @@ colon (`:`) on other systems. The order priority will be kept. Using the following example : ```hcl required_plugins { - myawesomecloud = { + happycloud = { version = ">= 2.7.0" - source = "azr/myawesomecloud" + source = "github.com/azr/happycloud" } - happycloud = ">= 2.7.0" } ``` The plugin getter will then install the binaries in the following location for a system with no `PACKER_PLUGIN_PATH` env var set. -* `PACKER_HOME_DIR/plugins/github.com/azr/myawesomecloud/` * `PACKER_HOME_DIR/plugins/github.com/hashicorp/happycloud/` During initialization, on a `darwin_amd64` system, Packer will look-up for the following files: -* `PACKER_EXEC_DIR/github.com/azr/myawesomecloud/packer-plugin-myawesomecloud_*_darwin_amd64_x5` -* `./github.com/azr/myawesomecloud/packer-plugin-myawesomecloud_*_darwin_amd64_x5` -* `PACKER_HOME_DIR/plugins/github.com/azr/myawesomecloud/packer-plugin-myawesomecloud_*_darwin_amd64_x5` -* `PACKER_PLUGIN_PATH/github.com/azr/myawesomecloud/packer-plugin-myawesomecloud_*_darwin_amd64_x5` -* `./packer-plugin-myawesomecloud` +* `PACKER_EXEC_DIR/github.com/azr/happycloud/packer-plugin-happycloud_*_darwin_amd64_x5.0` +* `./github.com/azr/happycloud/packer-plugin-happycloud_*_darwin_amd64_x5.0` +* `PACKER_HOME_DIR/plugins/github.com/azr/happycloud/packer-plugin-happycloud_*_darwin_amd64_x5.0` +* `PACKER_PLUGIN_PATH/github.com/azr/happycloud/packer-plugin-happycloud_*_darwin_amd64_x5.0` +* `./packer-plugin-happycloud` The first plugin-name/version files found will take precedence. From 4487152d1e83c51ad8316f426a4635d35e6b634f Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 14:05:17 +0100 Subject: [PATCH 09/17] cosmetic commit --- hcl2template/addrs/plugin.go | 2 +- hcl2template/types.packer_config_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hcl2template/addrs/plugin.go b/hcl2template/addrs/plugin.go index f26ee796e..9b6c451bd 100644 --- a/hcl2template/addrs/plugin.go +++ b/hcl2template/addrs/plugin.go @@ -40,7 +40,7 @@ func (p *Plugin) ForDisplay() string { if p.Hostname != DefaultHashicorpPluginHost { parts = append(parts, p.Hostname) } - if p.Namespace != DefaultHashicorpPluginNamespace { + if p.Namespace != DefaultHashicorpPluginNamespace && len(parts) == 0 { parts = append(parts, p.Namespace) } parts = append(parts, p.Type) diff --git a/hcl2template/types.packer_config_test.go b/hcl2template/types.packer_config_test.go index a9a0488e6..e4b526068 100644 --- a/hcl2template/types.packer_config_test.go +++ b/hcl2template/types.packer_config_test.go @@ -650,7 +650,7 @@ func TestParser_no_init(t *testing.T) { []packersdk.Build{}, false, }, - {"invalid_short_source_2.pkr.hcl", + {"invalid_inexplicit_source_2.pkr.hcl", defaultParser, parseTestArgs{"testdata/init/invalid_inexplicit_source_2.pkr.hcl", nil, nil}, &PackerConfig{ From 429262030fa5c70a050ab3d7fe6f3b0667a50cc5 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 14:11:00 +0100 Subject: [PATCH 10/17] fix paths for windows ! --- hcl2template/types.packer_config_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hcl2template/types.packer_config_test.go b/hcl2template/types.packer_config_test.go index e4b526068..5a522b4d0 100644 --- a/hcl2template/types.packer_config_test.go +++ b/hcl2template/types.packer_config_test.go @@ -1,6 +1,7 @@ package hcl2template import ( + "path/filepath" "testing" "github.com/hashicorp/go-version" @@ -624,7 +625,7 @@ func TestParser_no_init(t *testing.T) { }, }, CorePackerVersionString: lockedVersion, - Basedir: "testdata/init", + Basedir: filepath.Clean("testdata/init"), }, true, true, []packersdk.Build{}, @@ -644,7 +645,7 @@ func TestParser_no_init(t *testing.T) { }, }, CorePackerVersionString: lockedVersion, - Basedir: "testdata/init", + Basedir: filepath.Clean("testdata/init"), }, true, true, []packersdk.Build{}, @@ -664,7 +665,7 @@ func TestParser_no_init(t *testing.T) { }, }, CorePackerVersionString: lockedVersion, - Basedir: "testdata/init", + Basedir: filepath.Clean("testdata/init"), }, true, true, []packersdk.Build{}, From ba87656273dcc8955211f84bc067a1560f7db457 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 14:20:50 +0100 Subject: [PATCH 11/17] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e606c254..ae74420d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### FEATURES ** New Command** (HCL only) `packer init` command will download plugins defined - in a new `required_plugins` block [GH-10304] + in a new `required_plugins` block [GH-10304] [GH-10633]. ** New Plugin Type** Data sources can be implemented (blog post forthcoming). [GH-10440] ** New Plugin** Aws Secrets Manager data source [GH-10505] [GH-10467] From 13b34e2c73f4d3e572140c78632ec1af82f3269e Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 15:13:36 +0100 Subject: [PATCH 12/17] Update 1.7-template-upgrade.mdx --- website/content/guides/1.7-template-upgrade.mdx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/website/content/guides/1.7-template-upgrade.mdx b/website/content/guides/1.7-template-upgrade.mdx index 1d7f8c108..08eec9145 100644 --- a/website/content/guides/1.7-template-upgrade.mdx +++ b/website/content/guides/1.7-template-upgrade.mdx @@ -135,10 +135,12 @@ in the rest of the template. Here it is a brief explanation of each field: - `version` - Should follow the [version constraints](/docs/templates/hcl_templates/blocks/packer#version-constraints). -- `source` - Should have the plugin's organizational namespace on GitHub and its short name. Packer will be responsible -for determining the full GitHub address. - For example, if the source is `sylviamoss/comment`, Packer will download the binaries from `github.com/sylviamoss/packer-plugin-comment`. - To learn more about the source field, check out the [Source Address](/docs/plugins#source-addresses) documentation. +- `source` - Should have the plugin's organizational namespace on GitHub and its + short name. Packer will be responsible for determining the full GitHub + address. For example, if the source is `github.com/sylviamoss/comment`, Packer + will download the binaries from `github.com/sylviamoss/packer-plugin-comment`. + To learn more about the source field, check out the [Source + Address](/docs/plugins#source-addresses) documentation. - `local_name`- Can be replaced with whatever you want, and the new value will become the name of the plugin. For example: ```hcl From 516e919c5e3a5a2bf591f065f2b9760494c05b99 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 15:15:00 +0100 Subject: [PATCH 13/17] Update init.mdx --- website/content/docs/commands/init.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/content/docs/commands/init.mdx b/website/content/docs/commands/init.mdx index e0875474b..e4c3dbad1 100644 --- a/website/content/docs/commands/init.mdx +++ b/website/content/docs/commands/init.mdx @@ -46,7 +46,7 @@ packer { required_plugins { happycloud = { version = ">= 2.7.0" - source = "azr/happycloud" + source = "github.com/azr/happycloud" } } } From 8208f425c87965fea2e35b1d7824df62a464b2d4 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 15:32:42 +0100 Subject: [PATCH 14/17] addrs: remove Plugin.ForDisplay func, the String one does the job to make things less confusing --- command/init.go | 4 ++-- hcl2template/addrs/plugin.go | 25 ++++--------------------- hcl2template/plugin.go | 8 ++++---- packer/plugin-getter/plugins.go | 14 +++++++------- website/content/docs/plugins/index.mdx | 5 +---- 5 files changed, 18 insertions(+), 38 deletions(-) diff --git a/command/init.go b/command/init.go index ed5903030..eab38b648 100644 --- a/command/init.go +++ b/command/init.go @@ -109,7 +109,7 @@ func (c *InitCommand) RunContext(buildCtx context.Context, cla *InitArgs) int { return 1 } - log.Printf("[TRACE] for plugin %s found %d matching installation(s)", pluginRequirement.Identifier.ForDisplay(), len(installs)) + log.Printf("[TRACE] for plugin %s found %d matching installation(s)", pluginRequirement.Identifier, len(installs)) if len(installs) > 0 && cla.Upgrade == false { continue @@ -125,7 +125,7 @@ func (c *InitCommand) RunContext(buildCtx context.Context, cla *InitArgs) int { ret = 1 } if newInstall != nil { - msg := fmt.Sprintf("Installed plugin %s %s in %q", pluginRequirement.Identifier.ForDisplay(), newInstall.Version, newInstall.BinaryPath) + msg := fmt.Sprintf("Installed plugin %s %s in %q", pluginRequirement.Identifier, newInstall.Version, newInstall.BinaryPath) ui.Say(msg) } } diff --git a/hcl2template/addrs/plugin.go b/hcl2template/addrs/plugin.go index 9b6c451bd..8fae62620 100644 --- a/hcl2template/addrs/plugin.go +++ b/hcl2template/addrs/plugin.go @@ -27,26 +27,6 @@ func (p Plugin) String() string { return strings.Join(p.Parts(), "/") } -// ForDisplay returns a user-friendly FQN string, simplified for readability. If -// the plugin is using the default hostname, the hostname is omitted. -func (p *Plugin) ForDisplay() string { - const ( - // These will be hidden if they are a prefix - DefaultHashicorpPluginHost = "github.com" - DefaultHashicorpPluginNamespace = "hashicorp" - ) - - parts := []string{} - if p.Hostname != DefaultHashicorpPluginHost { - parts = append(parts, p.Hostname) - } - if p.Namespace != DefaultHashicorpPluginNamespace && len(parts) == 0 { - parts = append(parts, p.Namespace) - } - parts = append(parts, p.Type) - return strings.Join(parts, "/") -} - // ParsePluginPart processes an addrs.Plugin namespace or type string // provided by an end-user, producing a normalized version if possible or // an error if the string contains invalid characters. @@ -204,7 +184,10 @@ func ParsePluginSourceString(str string) (*Plugin, hcl.Diagnostics) { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid plugin type", - Detail: fmt.Sprintf("Plugin source %q has a type with the prefix %q, which isn't valid. Although that prefix is often used in the names of version control repositories for Packer plugins, plugin source strings should not include it.\n\nDid you mean %q?", ret.ForDisplay(), userErrorPrefix, suggestedAddr.ForDisplay()), + Detail: fmt.Sprintf("Plugin source %q has a type with the prefix %q, which isn't valid. "+ + "Although that prefix is often used in the names of version control repositories for Packer plugins, "+ + "plugin source strings should not include it.\n"+ + "\nDid you mean %q?", ret, userErrorPrefix, suggestedAddr), }) return nil, diags } diff --git a/hcl2template/plugin.go b/hcl2template/plugin.go index 4b1e81859..b43abdeb5 100644 --- a/hcl2template/plugin.go +++ b/hcl2template/plugin.go @@ -78,7 +78,7 @@ func (cfg *PackerConfig) detectPluginBinaries() hcl.Diagnostics { if err != nil { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: fmt.Sprintf("Failed to list installation for %s", pluginRequirement.Identifier.ForDisplay()), + Summary: fmt.Sprintf("Failed to list installation for %s", pluginRequirement.Identifier), Detail: err.Error(), }) continue @@ -86,18 +86,18 @@ func (cfg *PackerConfig) detectPluginBinaries() hcl.Diagnostics { if len(sortedInstalls) == 0 { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: fmt.Sprintf("no plugin installed for %s %v", pluginRequirement.Identifier.ForDisplay(), pluginRequirement.VersionConstraints.String()), + Summary: fmt.Sprintf("no plugin installed for %s %v", pluginRequirement.Identifier, pluginRequirement.VersionConstraints.String()), Detail: "Did you run packer init for this project ?", }) continue } - log.Printf("[TRACE] Found the following %q installations: %v", pluginRequirement.Identifier.ForDisplay(), sortedInstalls) + log.Printf("[TRACE] Found the following %q installations: %v", pluginRequirement.Identifier, sortedInstalls) install := sortedInstalls[len(sortedInstalls)-1] err = cfg.parser.PluginConfig.DiscoverMultiPlugin(pluginRequirement.Accessor, install.BinaryPath) if err != nil { diags = append(diags, &hcl.Diagnostic{ Severity: hcl.DiagError, - Summary: fmt.Sprintf("Error discovering plugin %s", pluginRequirement.Identifier.ForDisplay()), + Summary: fmt.Sprintf("Error discovering plugin %s", pluginRequirement.Identifier), Detail: err.Error(), }) continue diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 7fdcdac96..9dd670741 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -81,7 +81,7 @@ func (pr Requirement) ListInstallations(opts ListInstallationsOptions) (InstallL res := InstallList{} FilenamePrefix := pr.FilenamePrefix() filenameSuffix := opts.filenameSuffix() - log.Printf("[TRACE] listing potential installations for %q that match %q. %#v", pr.Identifier.ForDisplay(), pr.VersionConstraints, opts) + log.Printf("[TRACE] listing potential installations for %q that match %q. %#v", pr.Identifier, pr.VersionConstraints, opts) for _, knownFolder := range opts.FromFolders { glob := filepath.Join(knownFolder, pr.Identifier.Hostname, pr.Identifier.Namespace, pr.Identifier.Type, FilenamePrefix+"*"+filenameSuffix) @@ -345,7 +345,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) getters := opts.Getters fail := fmt.Errorf("could not find a local nor a remote checksum for plugin %q %q", pr.Identifier, pr.VersionConstraints) - log.Printf("[TRACE] getting available versions for the the %s plugin", pr.Identifier.ForDisplay()) + log.Printf("[TRACE] getting available versions for the the %s plugin", pr.Identifier) versions := version.Collection{} for _, getter := range getters { @@ -397,7 +397,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) log.Printf("[DEBUG] will try to install: %s", versions) if len(versions) == 0 { - err := fmt.Errorf("no release version found for the %s plugin matching the constraint(s): %q", pr.Identifier.ForDisplay(), pr.VersionConstraints.String()) + err := fmt.Errorf("no release version found for the %s plugin matching the constraint(s): %q", pr.Identifier, pr.VersionConstraints.String()) return nil, err } @@ -411,7 +411,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) filepath.Join(pr.Identifier.Parts()...), ) - log.Printf("[TRACE] fetching checksums file for the %q version of the %s plugin in %q...", version, pr.Identifier.ForDisplay(), outputFolder) + log.Printf("[TRACE] fetching checksums file for the %q version of the %s plugin in %q...", version, pr.Identifier, outputFolder) var checksum *FileChecksum for _, getter := range getters { @@ -428,7 +428,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) version: version, }) if err != nil { - err := fmt.Errorf("could not get %s checksum file for %s version %s. Is the file present on the release and correctly named ? %s", checksummer.Type, pr.Identifier.ForDisplay(), version, err) + err := fmt.Errorf("could not get %s checksum file for %s version %s. Is the file present on the release and correctly named ? %s", checksummer.Type, pr.Identifier, version, err) log.Printf("[TRACE] %s", err.Error()) return nil, err } @@ -486,7 +486,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) log.Printf("[TRACE] found a pre-exising %q checksum file", potentialChecksumer.Type) // if outputFile is there and matches the checksum: do nothing more. if err := localChecksum.ChecksumFile(localChecksum.Expected, potentialOutputFilename); err == nil { - log.Printf("[INFO] %s v%s plugin is already correctly installed in %q", pr.Identifier.ForDisplay(), version, potentialOutputFilename) + log.Printf("[INFO] %s v%s plugin is already correctly installed in %q", pr.Identifier, version, potentialOutputFilename) return nil, nil } } @@ -519,7 +519,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) expectedZipFilename: expectedZipFilename, }) if err != nil { - err := fmt.Errorf("could not get binary for %s version %s. Is the file present on the release and correctly named ? %s", pr.Identifier.ForDisplay(), version, err) + err := fmt.Errorf("could not get binary for %s version %s. Is the file present on the release and correctly named ? %s", pr.Identifier, version, err) log.Printf("[TRACE] %v", err) continue } diff --git a/website/content/docs/plugins/index.mdx b/website/content/docs/plugins/index.mdx index 0852f5ea9..84daeeac5 100644 --- a/website/content/docs/plugins/index.mdx +++ b/website/content/docs/plugins/index.mdx @@ -173,10 +173,7 @@ for brevity. The source address with all three components given explicitly is called the plugin's _fully-qualified address_. You will see fully-qualified address in -various outputs, like error messages, but in most cases a simplified display -version is used. Therefore you may see the shortened version `"myawesomecloud"` -instead of `"github.com/hashicorp/myawesomecloud"`. This will happen only when -the prefix is `github.com/hashicorp`. +various outputs, like error messages. ## Plugin location From f7ee3f8ead5dd4f7008a67871144554021ed1172 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 15 Feb 2021 17:24:56 +0100 Subject: [PATCH 15/17] Update website/content/guides/1.7-template-upgrade.mdx Co-authored-by: Sylvia Moss --- website/content/guides/1.7-template-upgrade.mdx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/website/content/guides/1.7-template-upgrade.mdx b/website/content/guides/1.7-template-upgrade.mdx index 08eec9145..5a90394ee 100644 --- a/website/content/guides/1.7-template-upgrade.mdx +++ b/website/content/guides/1.7-template-upgrade.mdx @@ -135,8 +135,8 @@ in the rest of the template. Here it is a brief explanation of each field: - `version` - Should follow the [version constraints](/docs/templates/hcl_templates/blocks/packer#version-constraints). -- `source` - Should have the plugin's organizational namespace on GitHub and its - short name. Packer will be responsible for determining the full GitHub +- `source` - Should have the GitHub hostname, the plugin's organizational namespace, and its short name. +Packer will be responsible for determining the full GitHub address. For example, if the source is `github.com/sylviamoss/comment`, Packer will download the binaries from `github.com/sylviamoss/packer-plugin-comment`. To learn more about the source field, check out the [Source @@ -182,4 +182,3 @@ and won't need to run `init` again unless you want to upgrade the plugin version A template with a `required_plugins` block should **always** be initialised at least once with `packer init` before `packer build`. If the template is built before init, Packer will fail and ask for initialisation. - From f48583c57edf673ad1f141caaf1c5375e022da99 Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Tue, 16 Feb 2021 13:31:18 -0500 Subject: [PATCH 16/17] github/getter: Adds a hostname check to Get function This change adds a simple hostname validation check to validate that a plugins source address is github.com before continuing with the Get call. An issue was encountered when using a hostname different from github.com, where the getter would continue to pull a plugin from GitHub even if the hostname was something like "example.com". See log details below. Before change ``` 2021/02/16 12:49:41 [TRACE] fetching checksums file for the "0.0.2" version of the example.com/hashicorp/docker plugin in "/home/wilken/.packer.d/plugins/example.com/hashicorp/docker"... 2021/02/16 12:49:41 [DEBUG] github-getter: getting "https://github.com/hashicorp/packer-plugin-docker/releases/download/v0.0.2/packer-plugin-docker_v0.0.2_SHA256SUMS" 2021/02/16 12:49:42 [TRACE] Ignoring remote binary packer-plugin-docker_v0.0.2_x5.0_linux_arm64.zip, wrong system, expected 2021/02/16 12:49:42 [TRACE] About to get: packer-plugin-docker_v0.0.2_x5.0_linux_amd64.zip 2021/02/16 12:49:42 [DEBUG] github-getter: getting "https://github.com/hashicorp/packer-plugin-docker/releases/download/v0.0.2/packer-plugin-docker_v0.0.2_x5.0_linux_amd64.zip" ``` After change ``` 2021/02/16 13:36:32 [TRACE] for plugin example.com/hashicorp/docker found 0 matching installation(s) 2021/02/16 13:36:32 [TRACE] getting available versions for the the example.com/hashicorp/docker plugin 2021/02/16 13:36:32 [TRACE] &{%!q(*github.Client=) "packer-getter-github-1.7.0-dev"} getter could not get release: example.com/hashicorp/docker doesn't appear to be a valid github.com source address; check source and try again. 2021/02/16 13:36:32 [DEBUG] will try to install: [] 2021/02/16 13:36:32 [INFO] (telemetry) Finalizing. no release version found for the example.com/hashicorp/docker plugin matching the constraint(s): ">=v0.0.2" 2021/02/16 13:36:32 waiting for all plugin processes to complete... ``` --- packer/plugin-getter/github/getter.go | 6 ++++++ packer/plugin-getter/plugins.go | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packer/plugin-getter/github/getter.go b/packer/plugin-getter/github/getter.go index c34e092a8..c33c55d77 100644 --- a/packer/plugin-getter/github/getter.go +++ b/packer/plugin-getter/github/getter.go @@ -23,6 +23,7 @@ import ( const ( ghTokenAccessor = "PACKER_GITHUB_API_TOKEN" defaultUserAgent = "packer-plugin-getter" + defaultHostname = "github.com" ) type Getter struct { @@ -154,6 +155,11 @@ func (t *HostSpecificTokenAuthTransport) base() http.RoundTripper { } func (g *Getter) Get(what string, opts plugingetter.GetOptions) (io.ReadCloser, error) { + if opts.PluginRequirement.Identifier.Hostname != defaultHostname { + s := opts.PluginRequirement.Identifier.String() + " doesn't appear to be a valid " + defaultHostname + " source address; check source and try again." + return nil, errors.New(s) + } + ctx := context.TODO() if g.Client == nil { var tc *http.Client diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 9dd670741..9595b67a0 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -345,7 +345,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) getters := opts.Getters fail := fmt.Errorf("could not find a local nor a remote checksum for plugin %q %q", pr.Identifier, pr.VersionConstraints) - log.Printf("[TRACE] getting available versions for the the %s plugin", pr.Identifier) + log.Printf("[TRACE] getting available versions for the %s plugin", pr.Identifier) versions := version.Collection{} for _, getter := range getters { From 728c5a217dae1fa75a9d09a8328dfa1eedbd6f3b Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Tue, 16 Feb 2021 14:03:29 -0500 Subject: [PATCH 17/17] Add test case for non-github hostname Tests results on current branch; install succeeded which was not expected ``` 2021/02/16 14:02:24 ui: Installed plugin example.com/sylviamoss/comment v0.2.19 in "/tmp/pkr-test-cfg-dir-6_pkr_config458005728/example.com/sylviamoss/comment/packer-plugin-comment_v0.2.19_x5.0_linux_amd64" init_test.go:361: InitCommand.Run() = 0, want 1 init_test.go:381: unexpected dir hash after init: string( - "h1:47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=", + "h1:iVtzkl/nVm2KiLvlz8rH56ME8QEqRxq8+XT2Lo6bzGU=", ) --- FAIL: TestInitCommand_Run (6.39s) --- PASS: TestInitCommand_Run/already-installed-no-op (0.01s) --- PASS: TestInitCommand_Run/already-installed-no-op/-subtest-0 (0.00s) --- PASS: TestInitCommand_Run/already-installed-upgrade (2.30s) --- PASS: TestInitCommand_Run/already-installed-upgrade/-subtest-0 (0.06s) --- PASS: TestInitCommand_Run/release-with-no-binary (0.17s) --- PASS: TestInitCommand_Run/manually-installed-single-component-plugin-works (1.32s) --- PASS: TestInitCommand_Run/manually-installed-single-component-plugin-works/-subtest-0 (0.01s) --- PASS: TestInitCommand_Run/manually-installed-single-component-plugin-old-api-fails (1.42s) --- PASS: TestInitCommand_Run/manually-installed-single-component-plugin-old-api-fails/-subtest-0 (0.01s) --- FAIL: TestInitCommand_Run/unsupported-non-github-source-address (1.18s) ``` Tests results after change with change in this branch ``` 2021/02/16 14:03:14 [TRACE] getting available versions for the example.com/sylviamoss/comment plugin 2021/02/16 14:03:14 [TRACE] &{%!q(*github.Client=) "packer-getter-github-1.7.0-dev"} getter could not get release: example.com/sylviamoss/comment doesn't appear to be a valid github.com source address; check source and try again. 2021/02/16 14:03:14 [DEBUG] will try to install: [] 2021/02/16 14:03:14 ui error: no release version found for the example.com/sylviamoss/comment plugin matching the constraint(s): "v0.2.19" --- PASS: TestInitCommand_Run (5.38s) --- PASS: TestInitCommand_Run/already-installed-no-op (0.01s) --- PASS: TestInitCommand_Run/already-installed-no-op/-subtest-0 (0.00s) --- PASS: TestInitCommand_Run/already-installed-upgrade (2.08s) --- PASS: TestInitCommand_Run/already-installed-upgrade/-subtest-0 (0.07s) --- PASS: TestInitCommand_Run/release-with-no-binary (0.21s) --- PASS: TestInitCommand_Run/manually-installed-single-component-plugin-works (1.20s) --- PASS: TestInitCommand_Run/manually-installed-single-component-plugin-works/-subtest-0 (0.01s) --- PASS: TestInitCommand_Run/manually-installed-single-component-plugin-old-api-fails (1.88s) --- PASS: TestInitCommand_Run/manually-installed-single-component-plugin-old-api-fails/-subtest-0 (0.01s) --- PASS: TestInitCommand_Run/unsupported-non-github-source-address (0.00s) ``` --- command/init_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/command/init_test.go b/command/init_test.go index 0b86379c7..6a44bc620 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -291,6 +291,32 @@ func TestInitCommand_Run(t *testing.T) { testBuild{want: 1}.fn, }, }, + { + "unsupported-non-github-source-address", + []func(t *testing.T, tc testCaseInit){ + skipInitTestUnlessEnVar(acctest.TestEnvVar).fn, + }, + testMetaFile(t), + nil, + "h1:47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=", + map[string]string{ + `cfg.pkr.hcl`: ` + packer { + required_plugins { + comment = { + source = "example.com/sylviamoss/comment" + version = "v0.2.19" + } + } + }`, + }, + cfg.dir("6_pkr_config"), + cfg.dir("6_pkr_user_folder"), + 1, + nil, + "h1:47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=", + nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {