From 83a2b622853fc68e96be7a5d8630f3c5594648ea Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 10 Apr 2024 12:12:11 -0400 Subject: [PATCH 01/16] packer: remove temp zipfile after installation The zipfile containing the binaries we attempt to install from a remote source is placed in the temporary directory of the host. In general it is wiped automatically by the OS, but in some cases (windows typically), it isn't. To avoid cluttering the temporary directory, we clean-up after ourselves, and remove the temporary zip file that we create when attempting to install a plugin, regardless of it succeeding or not. --- packer/plugin-getter/plugins.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 69c49387030..80bae56397b 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -800,7 +800,11 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) errs = multierror.Append(errs, err) return nil, errs } - defer tmpFile.Close() + defer func() { + tmpFilePath := tmpFile.Name() + tmpFile.Close() + os.Remove(tmpFilePath) + }() // start fetching binary remoteZipFile, err := getter.Get("zip", GetOptions{ @@ -837,10 +841,6 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) if err := checksum.Checksummer.Checksum(checksum.Expected, tmpFile); err != nil { err := fmt.Errorf("%w. Is the checksum file correct ? Is the binary file correct ?", err) errs = multierror.Append(errs, err) - log.Printf("%s, truncating the zipfile", err) - if err := tmpFile.Truncate(0); err != nil { - log.Printf("[TRACE] %v", err) - } continue } From 96f24f905506a471a91ff12957d5796c5f4b426f Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 10 Apr 2024 17:26:05 -0400 Subject: [PATCH 02/16] plugin: factorise calling describe on binaries Given that calling the describe command on plugins and deserialising the output as a plugin description is something done in multiple places in the code, we factorise this operation so we don't need to copy/paste the code around. --- packer/plugin-getter/plugins.go | 22 ++++++++++++++-------- packer/plugin.go | 9 ++------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 80bae56397b..752f845fe87 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -211,18 +211,12 @@ func (pr Requirement) ListInstallations(opts ListInstallationsOptions) (InstallL } } - descOut, err := exec.Command(path, "describe").Output() + describeInfo, err := GetPluginDescription(path) if err != nil { - log.Printf("couldn't call describe on %q, ignoring", path) + log.Printf("failed to call describe on %q: %s", path, err) continue } - var describeInfo pluginsdk.SetDescription - err = json.Unmarshal(descOut, &describeInfo) - if err != nil { - log.Printf("%q: describe output deserialization error %q, ignoring", path, err) - } - // versionsStr now looks like v1.2.3_x5.1 or amazon_v1.2.3_x5.1 parts := strings.SplitN(versionsStr, "_", 2) pluginVersionStr, protocolVersionStr := parts[0], parts[1] @@ -933,6 +927,18 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) return nil, errs } +func GetPluginDescription(pluginPath string) (pluginsdk.SetDescription, error) { + out, err := exec.Command(pluginPath, "describe").Output() + if err != nil { + return pluginsdk.SetDescription{}, err + } + + desc := pluginsdk.SetDescription{} + err = json.Unmarshal(out, &desc) + + return desc, err +} + func init() { var err error // Should never error if both components are set diff --git a/packer/plugin.go b/packer/plugin.go index de85071d7ad..cda4118f5e3 100644 --- a/packer/plugin.go +++ b/packer/plugin.go @@ -5,7 +5,6 @@ package packer import ( "crypto/sha256" - "encoding/json" "fmt" "log" "os" @@ -127,13 +126,9 @@ func (c *PluginConfig) Discover() error { // if the "packer-plugin-amazon" binary had an "ebs" builder one could use // the "amazon-ebs" builder. func (c *PluginConfig) DiscoverMultiPlugin(pluginName, pluginPath string) error { - out, err := exec.Command(pluginPath, "describe").Output() + desc, err := plugingetter.GetPluginDescription(pluginPath) if err != nil { - return err - } - var desc pluginsdk.SetDescription - if err := json.Unmarshal(out, &desc); err != nil { - return err + return fmt.Errorf("failed to get plugin description from executable %q: %s", pluginPath, err) } pluginPrefix := pluginName + "-" From 7a4ddf3188c512f1c0cbbbec925e4f2663b42001 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 10 Apr 2024 17:27:20 -0400 Subject: [PATCH 03/16] packer: check if errs is nil before getting length When installing a remote plugin, and after we've either successfully installed a binary, or exhausted all the possible sources, we print a final error message if nothing was reported. However, given that errs is a pointer to a structure, and if no errors were produced, the the error list could be nil, leading to the call to `Len()' to crash Packer. This is exceedingly rare as in general the code attempts to read multiple sources from Github, and therefore we almost always get some error reported, but while changing the function's code, I managed to make it crash while removing/changing some error statements. Therefore to avoid future surprises, we first check that `errs' is not nil before invoking `Len()' on it, as no errors and no plugins installed mean that something went wrong all the same. --- packer/plugin-getter/plugins.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 752f845fe87..c4a837fdd84 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -917,7 +917,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) } } - if errs.Len() == 0 { + if errs == nil || errs.Len() == 0 { err := fmt.Errorf("could not find a local nor a remote checksum for plugin %q %q", pr.Identifier, pr.VersionConstraints) errs = multierror.Append(errs, err) } From 80d9e8b321155c327e6b49eba668e6465f8fbed6 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 10 Apr 2024 17:32:12 -0400 Subject: [PATCH 04/16] packer: alias version as goversion for getter Since we named the version from the getter `version', this means we have a naming conflict inside the loop that attempts to install a versioned candidate for a plugin, making it impossible to invoke something from the go-version package. Since we'll introduce a change that needs the latter capability, we must either rename the local variable to something else than `version', or we need to alias the package locally. This commit implements the latter, opting to call the package goversion. --- packer/plugin-getter/plugins.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index c4a837fdd84..4f892a5cd66 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -22,7 +22,7 @@ import ( "time" "github.com/hashicorp/go-multierror" - "github.com/hashicorp/go-version" + goversion "github.com/hashicorp/go-version" pluginsdk "github.com/hashicorp/packer-plugin-sdk/plugin" "github.com/hashicorp/packer-plugin-sdk/tmp" "github.com/hashicorp/packer/hcl2template/addrs" @@ -47,7 +47,7 @@ type Requirement struct { // VersionConstraints as defined by user. Empty ( to be avoided ) means // highest found version. - VersionConstraints version.Constraints + VersionConstraints goversion.Constraints } type BinaryInstallationOptions struct { @@ -220,7 +220,7 @@ func (pr Requirement) ListInstallations(opts ListInstallationsOptions) (InstallL // versionsStr now looks like v1.2.3_x5.1 or amazon_v1.2.3_x5.1 parts := strings.SplitN(versionsStr, "_", 2) pluginVersionStr, protocolVersionStr := parts[0], parts[1] - ver, err := version.NewVersion(pluginVersionStr) + ver, err := goversion.NewVersion(pluginVersionStr) if err != nil { // could not be parsed, ignoring the file log.Printf("found %q with an incorrect %q version, ignoring it. %v", path, pluginVersionStr, err) @@ -237,13 +237,13 @@ func (pr Requirement) ListInstallations(opts ListInstallationsOptions) (InstallL continue } - rawVersion, err := version.NewVersion(pluginVersionStr) + rawVersion, err := goversion.NewVersion(pluginVersionStr) if err != nil { log.Printf("malformed version string in filename %q: %s, ignoring", pluginVersionStr, err) continue } - descVersion, err := version.NewVersion(describeInfo.Version) + descVersion, err := goversion.NewVersion(describeInfo.Version) if err != nil { log.Printf("malformed reported version string %q: %s, ignoring", describeInfo.Version, err) continue @@ -411,7 +411,7 @@ type GetOptions struct { BinaryInstallationOptions - version *version.Version + version *goversion.Version expectedZipFilename string } @@ -614,7 +614,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) getters := opts.Getters log.Printf("[TRACE] getting available versions for the %s plugin", pr.Identifier) - versions := version.Collection{} + versions := goversion.Collection{} var errs *multierror.Error for _, getter := range getters { @@ -642,7 +642,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) continue } for _, release := range releases { - v, err := version.NewVersion(release.Version) + v, err := goversion.NewVersion(release.Version) if err != nil { err := fmt.Errorf("could not parse release version %s. %w", release.Version, err) errs = multierror.Append(errs, err) From 1d1935a116f005471da1e4aec12cdfb038b758e8 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 10 Apr 2024 17:34:05 -0400 Subject: [PATCH 05/16] packer/getter: don't report other os/arch as errs Whenever a Github release exposes an entry for another OS/arch combination, this gets registered as an error, which in the event no binary is compatible with the host's OS/arch, gets reported at the end of the getter process. While this is sound in theory, in practice we get the list of all the combinations that don't match the host's, which is not something a Packer user can act on, and might therefore be more confusing than helping to solve the issue. Therefore we opt in this commit to stop registering those cases as real errors, and only log them as an INFO statement. --- packer/plugin-getter/plugins.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 4f892a5cd66..96fce771418 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -726,9 +726,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) continue } if err := entry.validate("v"+version.String(), opts.BinaryInstallationOptions); err != nil { - err := fmt.Errorf("ignoring invalid remote binary %s: %s", entry.Filename, err) - errs = multierror.Append(errs, err) - log.Printf("[TRACE] %s", err) + log.Printf("[INFO] ignoring invalid remote binary %s: %s", entry.Filename, err) continue } From e8a5814ec5508cb6a08f6d34fbd22ce9dbd9c274 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 10 Apr 2024 17:43:48 -0400 Subject: [PATCH 06/16] packer: fix filename in verbose logs --- packer/plugin-getter/plugins.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 96fce771418..ceac6188d59 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -864,7 +864,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) break } if copyFrom == nil { - err := fmt.Errorf("could not find a %s file in zipfile", checksum.Filename) + err := fmt.Errorf("could not find a %q file in zipfile", expectedBinaryFilename) errs = multierror.Append(errs, err) return nil, errs } From 13493f39095f80e10d058d77b4562ed109a96550 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Tue, 23 Apr 2024 15:17:04 -0400 Subject: [PATCH 07/16] packer: remove duplicate version variable When listing installed plugins, we check that the plugin's reported version through describe matches what's in the name of the file. Doing do, we were parsing the same version string twice without modifying it, which was not necessary, so this commit changes that. --- packer/plugin-getter/plugins.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index ceac6188d59..4d4d57eec33 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -237,19 +237,13 @@ func (pr Requirement) ListInstallations(opts ListInstallationsOptions) (InstallL continue } - rawVersion, err := goversion.NewVersion(pluginVersionStr) - if err != nil { - log.Printf("malformed version string in filename %q: %s, ignoring", pluginVersionStr, err) - continue - } - descVersion, err := goversion.NewVersion(describeInfo.Version) if err != nil { log.Printf("malformed reported version string %q: %s, ignoring", describeInfo.Version, err) continue } - if rawVersion.Compare(descVersion) != 0 { + if ver.Compare(descVersion) != 0 { log.Printf("plugin %q reported version %q while its name implies version %q, ignoring", path, describeInfo.Version, pluginVersionStr) continue } @@ -271,7 +265,7 @@ func (pr Requirement) ListInstallations(opts ListInstallationsOptions) (InstallL // Note: we use the raw version name here, without the pre-release // suffix, as otherwise constraints reject them, which is not // what we want by default. - if !pr.VersionConstraints.Check(rawVersion.Core()) { + if !pr.VersionConstraints.Check(ver.Core()) { log.Printf("[TRACE] version %q of file %q does not match constraint %q", pluginVersionStr, path, pr.VersionConstraints.String()) continue } From 728e1156302fd1d902c60b32ca20a77f0690bf30 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 10 Apr 2024 17:37:36 -0400 Subject: [PATCH 08/16] packer: ensure versions match for remote installs Since we're hardening what Packer is able to load locally when it comes to plugins, we need also to harden the installation process a bit. While testing we noticed some remotes had published their plugins with version mismatches between the tag and the binary. This was not a problem in the past, as Packer did not care for this, only the binary name was important, and the plugin could be installed without problem. Nowadays however, since Packer enforces the plugin version reported in the name to be the same as the plugin self-reported version, this makes it impossible for the installed plugin to load anymore in such an instance. Therefore in order to limit confusion, and so users are able to understand the problem and report it to the plugins with that mismatch, we instead install the plugin as a dev version, and report it to the user so they can still use it, but are able to report it to the plugin developers. --- packer/plugin-getter/plugins.go | 76 ++++++++++++++++++++++++++-- packer/plugin-getter/plugins_test.go | 38 +++++++++++--- 2 files changed, 104 insertions(+), 10 deletions(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 4d4d57eec33..51e6d7b452e 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -24,6 +24,7 @@ import ( "github.com/hashicorp/go-multierror" goversion "github.com/hashicorp/go-version" pluginsdk "github.com/hashicorp/packer-plugin-sdk/plugin" + "github.com/hashicorp/packer-plugin-sdk/random" "github.com/hashicorp/packer-plugin-sdk/tmp" "github.com/hashicorp/packer/hcl2template/addrs" "golang.org/x/mod/semver" @@ -863,9 +864,76 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) return nil, errs } - outputFile, err := os.OpenFile(outputFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755) + tempPluginPath := filepath.Join(os.TempDir(), fmt.Sprintf( + "packer-plugin-temp-%s%s", + random.Numbers(8), + opts.BinaryInstallationOptions.Ext)) + + // Save binary to temp so we can ensure it is really the version advertised + tempOutput, err := os.OpenFile(tempPluginPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755) + if err != nil { + log.Printf("[ERROR] failed to create temp plugin executable: %s", err) + return nil, multierror.Append(errs, err) + } + defer os.Remove(tempPluginPath) + + _, err = io.Copy(tempOutput, copyFrom) + if err != nil { + log.Printf("[ERROR] failed to copy uncompressed binary to %q: %s", tempPluginPath, err) + return nil, multierror.Append(errs, err) + } + + // Not a problem on most platforms, but unsure Windows will let us execute an already + // open file, so we close it temporarily to avoid problems + _ = tempOutput.Close() + + desc, err := GetPluginDescription(tempPluginPath) + if err != nil { + err := fmt.Errorf("failed to describe plugin binary %q: %s", tempPluginPath, err) + errs = multierror.Append(errs, err) + continue + } + + descVersion, err := goversion.NewSemver(desc.Version) + if err != nil { + err := fmt.Errorf("invalid self-reported version %q: %s", desc.Version, err) + errs = multierror.Append(errs, err) + continue + } + + if descVersion.Core().Compare(version.Core()) != 0 { + log.Printf("[ERROR] binary reported version (%q) is different from the expected %q, skipping", desc.Version, version.String()) + continue + } + + localOutputFileName := outputFileName + + // Since releases only can be installed remotely, a non-empty prerelease version + // means something's not right on the release, as it should report a final version instead. + // + // Therefore to avoid surprises (and avoid being able to install a version that + // cannot be loaded), we error here, and advise users to manually install the plugin if they + // need it. + if descVersion.Prerelease() != "" { + fmt.Printf("Plugin %q release v%s binary reports version %q. This is likely an upstream issue.\n"+ + "Try opening an issue on the plugin repository asking them to update the plugin's version information.\n", + pr.Identifier.String(), version, desc.Version) + fmt.Printf("If you need this exact version of the plugin, you can download the binary from the source and install "+ + "it locally with `packer plugins install --path '' %q`.\n"+ + "This will install the plugin in version %q (required_plugins constraints may need to be updated).\n", + pr.Identifier.String(), desc.Version) + continue + } + + copyFrom, err = os.OpenFile(tempPluginPath, os.O_RDONLY, 0755) + if err != nil { + log.Printf("[ERROR] failed to re-open temporary plugin file %q: %s", tempPluginPath, err) + return nil, multierror.Append(errs, err) + } + + outputFile, err := os.OpenFile(localOutputFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755) if err != nil { - err := fmt.Errorf("failed to create %s: %w", outputFileName, err) + err := fmt.Errorf("failed to create %s: %w", localOutputFileName, err) errs = multierror.Append(errs, err) return nil, errs } @@ -890,7 +958,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) log.Printf("[WARNING] %v, ignoring", err) } - if err := os.WriteFile(outputFileName+checksum.Checksummer.FileExt(), []byte(hex.EncodeToString(cs)), 0644); err != nil { + if err := os.WriteFile(localOutputFileName+checksum.Checksummer.FileExt(), []byte(hex.EncodeToString(cs)), 0644); err != nil { err := fmt.Errorf("failed to write local binary checksum file: %s", err) errs = multierror.Append(errs, err) log.Printf("[WARNING] %v, ignoring", err) @@ -898,7 +966,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) // Success !! return &Installation{ - BinaryPath: strings.ReplaceAll(outputFileName, "\\", "/"), + BinaryPath: strings.ReplaceAll(localOutputFileName, "\\", "/"), Version: "v" + version.String(), }, nil } diff --git a/packer/plugin-getter/plugins_test.go b/packer/plugin-getter/plugins_test.go index 85bcab42e80..f0518f50190 100644 --- a/packer/plugin-getter/plugins_test.go +++ b/packer/plugin-getter/plugins_test.go @@ -13,6 +13,7 @@ import ( "log" "os" "path/filepath" + "runtime" "strings" "testing" @@ -199,12 +200,17 @@ func TestRequirement_InstallLatest(t *testing.T) { ChecksumFileEntries: map[string][]ChecksumFileEntry{ "2.10.0": {{ Filename: "packer-plugin-amazon_v2.10.0_x6.0_darwin_amd64.zip", - Checksum: "43156b1900dc09b026b54610c4a152edd277366a7f71ff3812583e4a35dd0d4a", + Checksum: "5763f8b5b5ed248894e8511a089cf399b96c7ef92d784fb30ee6242a7cb35bce", }}, }, Zips: map[string]io.ReadCloser{ "github.com/hashicorp/packer-plugin-amazon/packer-plugin-amazon_v2.10.0_x6.0_darwin_amd64.zip": zipFile(map[string]string{ - "packer-plugin-amazon_v2.10.0_x6.0_darwin_amd64": "v2.10.0_x6.0_darwin_amd64", + // Make the false plugin echo an output that matches a subset of `describe` for install to work + // + // Note: this won't work on Windows as they don't have bin/sh, but this will + // eventually be replaced by acceptance tests. + "packer-plugin-amazon_v2.10.0_x6.0_darwin_amd64": `#!/bin/sh +echo '{"version":"v2.10.0","api_version":"x6.0"}'`, }), }, }, @@ -246,12 +252,17 @@ func TestRequirement_InstallLatest(t *testing.T) { ChecksumFileEntries: map[string][]ChecksumFileEntry{ "2.10.1": {{ Filename: "packer-plugin-amazon_v2.10.1_x6.1_darwin_amd64.zip", - Checksum: "90ca5b0f13a90238b62581bbf30bacd7e2c9af6592c7f4849627bddbcb039dec", + Checksum: "51451da5cd7f1ecd8699668d806bafe58a9222430842afbefdc62a6698dab260", }}, }, Zips: map[string]io.ReadCloser{ "github.com/hashicorp/packer-plugin-amazon/packer-plugin-amazon_v2.10.1_x6.1_darwin_amd64.zip": zipFile(map[string]string{ - "packer-plugin-amazon_v2.10.1_x6.1_darwin_amd64": "v2.10.1_x6.1_darwin_amd64", + // Make the false plugin echo an output that matches a subset of `describe` for install to work + // + // Note: this won't work on Windows as they don't have bin/sh, but this will + // eventually be replaced by acceptance tests. + "packer-plugin-amazon_v2.10.1_x6.1_darwin_amd64": `#!/bin/sh +echo '{"version":"v2.10.1","api_version":"x6.1"}'`, }), }, }, @@ -293,12 +304,17 @@ func TestRequirement_InstallLatest(t *testing.T) { ChecksumFileEntries: map[string][]ChecksumFileEntry{ "2.10.0": {{ Filename: "packer-plugin-amazon_v2.10.0_x6.1_linux_amd64.zip", - Checksum: "825fc931ae0cb151df0c56be41a17a9136c4d1f1ee73ddb8ed6baa17cef31afa", + Checksum: "5196f57f37e18bfeac10168db6915caae0341bfc4168ebc3d2b959d746cebd0a", }}, }, Zips: map[string]io.ReadCloser{ "github.com/hashicorp/packer-plugin-amazon/packer-plugin-amazon_v2.10.0_x6.1_linux_amd64.zip": zipFile(map[string]string{ - "packer-plugin-amazon_v2.10.0_x6.1_linux_amd64": "v2.10.0_x6.1_linux_amd64", + // Make the false plugin echo an output that matches a subset of `describe` for install to work + // + // Note: this won't work on Windows as they don't have bin/sh, but this will + // eventually be replaced by acceptance tests. + "packer-plugin-amazon_v2.10.0_x6.1_linux_amd64": `#!/bin/sh +echo '{"version":"v2.10.0","api_version":"x6.1"}'`, }), }, }, @@ -402,6 +418,16 @@ func TestRequirement_InstallLatest(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + switch tt.name { + case "upgrade-with-diff-protocol-version", + "upgrade-with-same-protocol-version", + "upgrade-with-one-missing-checksum-file": + if runtime.GOOS != "windows" { + break + } + t.Skipf("Test %q cannot run on Windows because of a shell script being invoked, skipping.", tt.name) + } + log.Printf("starting %s test", tt.name) identifier, diags := addrs.ParsePluginSourceString("github.com/hashicorp/" + tt.fields.Identifier) From 958c4cff50fd77e267a37654ce574ffcf549939d Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Mon, 29 Apr 2024 16:09:57 -0400 Subject: [PATCH 09/16] init: fix --force and pre-releases installed case When packer init is invoked with a --force argument, but no --update, we clamp the version to install based on the last one locally installed. Doing this may however cause the constraint to always be false if the latest available version of a plugin is a pre-release, as none of the upstream constraints will match that. Therefore this commit changes how the constraint is derived from the local list of installations, so that only the last installation that matches the original constraint will be used, and not a pre-release. --- command/init.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/command/init.go b/command/init.go index 2d8a1937759..10054917489 100644 --- a/command/init.go +++ b/command/init.go @@ -127,7 +127,20 @@ for more info.`) } if cla.Force && !cla.Upgrade { - pluginRequirement.VersionConstraints, _ = gversion.NewConstraint(fmt.Sprintf("=%s", installs[len(installs)-1].Version)) + // Only place another constaint to the latest release + // binary, if any, otherwise this is essentially the same + // as an upgrade + var installVersion string + for _, install := range installs { + ver, _ := gversion.NewVersion(install.Version) + if ver.Prerelease() == "" { + installVersion = install.Version + } + } + + if installVersion != "" { + pluginRequirement.VersionConstraints, _ = gversion.NewConstraint(fmt.Sprintf("=%s", installVersion)) + } } } From 380d897959ba68ce9b32593d8163375f362296f1 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Thu, 2 May 2024 09:52:51 -0400 Subject: [PATCH 10/16] command: list releases only for remote installs When installing a plugin from a remote source, we list the installed plugins that match the constraints specified, and if the constraint is already satisfied, we don't do anything. However, since remote installation is only relevant for releases of a plugin, we should only look at the installed releases of a plugin, and not consider pre-releases for that step. This wasn't the case before this commit, as if a prerelease version of a commit (ex: 10.8.1-dev), and we try to invoke `packer init` with a constraint on this version specifically, Packer would locate that pre-release and assume it was already installed, so would silently succeed the command and do nothing. This isn't the expected behaviour as we should install the final release of that plugin, regardless of any prerelease installation of the plugin. So this commit fixes that by only listing releases, so we don't report the plugin being already installed if a prerelease is what's installed. --- command/init.go | 1 + command/plugins_install.go | 1 + 2 files changed, 2 insertions(+) diff --git a/command/init.go b/command/init.go index 10054917489..a9a99342639 100644 --- a/command/init.go +++ b/command/init.go @@ -85,6 +85,7 @@ for more info.`) Checksummers: []plugingetter.Checksummer{ {Type: "sha256", Hash: sha256.New()}, }, + ReleasesOnly: true, }, } diff --git a/command/plugins_install.go b/command/plugins_install.go index 59e197a3f6a..57419f827ae 100644 --- a/command/plugins_install.go +++ b/command/plugins_install.go @@ -130,6 +130,7 @@ func (c *PluginsInstallCommand) RunContext(buildCtx context.Context, args *Plugi Checksummers: []plugingetter.Checksummer{ {Type: "sha256", Hash: sha256.New()}, }, + ReleasesOnly: true, }, } if runtime.GOOS == "windows" { From 709fdcd8241487f5117dbe684248f90516893bf8 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Thu, 2 May 2024 09:57:32 -0400 Subject: [PATCH 11/16] plugin-getter: rm vocal log on checksum entries When a checksum file for a release is downloaded and iterated upon to find the compatible binary for a release, we used to log each non-compatible entry in the logs. This is noisy as we know there's going to be multiple entries that don't match the host's os/arch, and there's no good reason to show those, so we silence them. --- packer/plugin-getter/plugins.go | 1 - 1 file changed, 1 deletion(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 51e6d7b452e..4ad02f60c81 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -721,7 +721,6 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) continue } if err := entry.validate("v"+version.String(), opts.BinaryInstallationOptions); err != nil { - log.Printf("[INFO] ignoring invalid remote binary %s: %s", entry.Filename, err) continue } From 103fffafa34a4732d14165c74d18a0167b121356 Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Fri, 3 May 2024 14:20:30 -0400 Subject: [PATCH 12/16] Update plugin-getter version matching check This change is an attempt to remove the need for additional temporary files, along with calls to stat the temp files, to reduce the number of files being created, opened, and closed. In addition to this change, the logic for falling back to a previous version if the highest matched version is a pre-release has been removed. Instead we will assume that any prior versions will exhibit the same issue and return immediately. A user can install the version manually if they will or they can modify their version constraint to a properly released version. --- packer/plugin-getter/plugins.go | 171 +++++++++++++------------------- 1 file changed, 67 insertions(+), 104 deletions(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 4ad02f60c81..32a31075b00 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -24,7 +24,6 @@ import ( "github.com/hashicorp/go-multierror" goversion "github.com/hashicorp/go-version" pluginsdk "github.com/hashicorp/packer-plugin-sdk/plugin" - "github.com/hashicorp/packer-plugin-sdk/random" "github.com/hashicorp/packer-plugin-sdk/tmp" "github.com/hashicorp/packer/hcl2template/addrs" "golang.org/x/mod/semver" @@ -741,11 +740,8 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) } expectedZipFilename := checksum.Filename expectedBinaryFilename := strings.TrimSuffix(expectedZipFilename, filepath.Ext(expectedZipFilename)) + opts.BinaryInstallationOptions.Ext + outputFileName := filepath.Join(outputFolder, expectedBinaryFilename) - outputFileName := filepath.Join( - outputFolder, - expectedBinaryFilename, - ) for _, potentialChecksumer := range opts.Checksummers { // First check if a local checksum file is already here in the expected // download folder. Here we want to download a binary so we only check @@ -758,7 +754,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) Checksummer: potentialChecksumer, } - log.Printf("[TRACE] found a pre-exising %q checksum file", potentialChecksumer.Type) + log.Printf("[TRACE] found a pre-existing %q checksum file", potentialChecksumer.Type) // if outputFile is there and matches the checksum: do nothing more. if err := localChecksum.ChecksumFile(localChecksum.Expected, outputFileName); err == nil && !opts.Force { log.Printf("[INFO] %s v%s plugin is already correctly installed in %q", pr.Identifier, version, outputFileName) @@ -767,9 +763,6 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) } } - // The last folder from the installation list is where we will install. - outputFileName = filepath.Join(outputFolder, expectedBinaryFilename) - // create directories if need be if err := os.MkdirAll(outputFolder, 0755); err != nil { err := fmt.Errorf("could not create plugin folder %q: %w", outputFolder, err) @@ -777,12 +770,24 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) log.Printf("[TRACE] %s", err.Error()) return nil, errs } - for _, getter := range getters { + // start fetching binary + remoteZipFile, err := getter.Get("zip", GetOptions{ + PluginRequirement: pr, + BinaryInstallationOptions: opts.BinaryInstallationOptions, + version: version, + expectedZipFilename: expectedZipFilename, + }) + if err != nil { + errs = multierror.Append(errs, + 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)) + continue + } // create temporary file that will receive a temporary binary.zip tmpFile, err := tmp.File("packer-plugin-*.zip") if err != nil { - err = fmt.Errorf("could not create temporary file to dowload plugin: %w", err) + err = fmt.Errorf("could not create temporary file to download plugin: %w", err) errs = multierror.Append(errs, err) return nil, errs } @@ -791,38 +796,20 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) tmpFile.Close() os.Remove(tmpFilePath) }() - - // start fetching binary - remoteZipFile, err := getter.Get("zip", GetOptions{ - PluginRequirement: pr, - BinaryInstallationOptions: opts.BinaryInstallationOptions, - version: version, - 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, version, err) - errs = multierror.Append(errs, err) - log.Printf("[TRACE] %v", err) - continue - } - // write binary to tmp file _, err = io.Copy(tmpFile, remoteZipFile) _ = remoteZipFile.Close() if err != nil { err := fmt.Errorf("Error getting plugin, trying another getter: %w", err) errs = multierror.Append(errs, err) - log.Printf("[TRACE] %s", err) continue } if _, err := tmpFile.Seek(0, 0); err != nil { - err := fmt.Errorf("Error seeking begining of temporary file for checksumming, continuing: %w", err) + err := fmt.Errorf("Error seeking beginning of temporary file for checksumming, continuing: %w", err) errs = multierror.Append(errs, err) - log.Printf("[TRACE] %s", err) continue } - // verify that the checksum for the zip is what we expect. if err := checksum.Checksummer.Checksum(checksum.Expected, tmpFile); err != nil { err := fmt.Errorf("%w. Is the checksum file correct ? Is the binary file correct ?", err) @@ -830,17 +817,9 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) continue } - tmpFileStat, err := tmpFile.Stat() + zr, err := zip.OpenReader(tmpFile.Name()) if err != nil { - err := fmt.Errorf("failed to stat: %w", err) - errs = multierror.Append(errs, err) - return nil, errs - } - - zr, err := zip.NewReader(tmpFile, tmpFileStat.Size()) - if err != nil { - err := fmt.Errorf("zip : %v", err) - errs = multierror.Append(errs, err) + errs = multierror.Append(errs, fmt.Errorf("zip : %v", err)) return nil, errs } @@ -851,8 +830,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) } copyFrom, err = f.Open() if err != nil { - err := fmt.Errorf("failed to open temp file: %w", err) - errs = multierror.Append(errs, err) + multierror.Append(errs, fmt.Errorf("failed to open temp file: %w", err)) return nil, errs } break @@ -863,50 +841,61 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) return nil, errs } - tempPluginPath := filepath.Join(os.TempDir(), fmt.Sprintf( - "packer-plugin-temp-%s%s", - random.Numbers(8), - opts.BinaryInstallationOptions.Ext)) - - // Save binary to temp so we can ensure it is really the version advertised - tempOutput, err := os.OpenFile(tempPluginPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755) + tmpBinFileName := filepath.Join(os.TempDir(), expectedBinaryFilename) if err != nil { - log.Printf("[ERROR] failed to create temp plugin executable: %s", err) - return nil, multierror.Append(errs, err) + err = fmt.Errorf("could not create temporary file to download plugin: %w", err) + errs = multierror.Append(errs, err) + return nil, errs } - defer os.Remove(tempPluginPath) + defer func() { + tmpBinPath := tmpBinFileName + os.Remove(tmpBinPath) + }() - _, err = io.Copy(tempOutput, copyFrom) + tmpOutputFile, err := os.OpenFile(tmpBinFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755) if err != nil { - log.Printf("[ERROR] failed to copy uncompressed binary to %q: %s", tempPluginPath, err) - return nil, multierror.Append(errs, err) + err := fmt.Errorf("failed to create %s: %w", tmpBinFileName, err) + errs = multierror.Append(errs, err) + return nil, errs + } + if _, err := io.Copy(tmpOutputFile, copyFrom); err != nil { + err := fmt.Errorf("extract file: %w", err) + errs = multierror.Append(errs, err) + return nil, errs } - // Not a problem on most platforms, but unsure Windows will let us execute an already - // open file, so we close it temporarily to avoid problems - _ = tempOutput.Close() + if _, err := tmpOutputFile.Seek(0, 0); err != nil { + err := fmt.Errorf("Error seeking beginning of binary file for checksumming: %w", err) + errs = multierror.Append(errs, err) + log.Printf("[WARNING] %v, ignoring", err) + } - desc, err := GetPluginDescription(tempPluginPath) + outputFileCS, err := checksum.Checksummer.Sum(tmpOutputFile) if err != nil { - err := fmt.Errorf("failed to describe plugin binary %q: %s", tempPluginPath, err) + err := fmt.Errorf("failed to checksum binary file: %s", err) errs = multierror.Append(errs, err) - continue + log.Printf("[WARNING] %v, ignoring", err) } + tmpOutputFile.Close() + desc, err := GetPluginDescription(tmpBinFileName) + if err != nil { + err := fmt.Errorf("failed to describe plugin binary %q: %s", tmpBinFileName, err) + errs = multierror.Append(errs, err) + continue + } descVersion, err := goversion.NewSemver(desc.Version) if err != nil { err := fmt.Errorf("invalid self-reported version %q: %s", desc.Version, err) errs = multierror.Append(errs, err) continue } - if descVersion.Core().Compare(version.Core()) != 0 { - log.Printf("[ERROR] binary reported version (%q) is different from the expected %q, skipping", desc.Version, version.String()) + err := fmt.Errorf("binary reported version (%q) is different from the expected %q, skipping", desc.Version, version.String()) + errs = multierror.Append(errs, err) continue } - localOutputFileName := outputFileName - // Since releases only can be installed remotely, a non-empty prerelease version // means something's not right on the release, as it should report a final version instead. // @@ -914,50 +903,26 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) // cannot be loaded), we error here, and advise users to manually install the plugin if they // need it. if descVersion.Prerelease() != "" { - fmt.Printf("Plugin %q release v%s binary reports version %q. This is likely an upstream issue.\n"+ - "Try opening an issue on the plugin repository asking them to update the plugin's version information.\n", - pr.Identifier.String(), version, desc.Version) - fmt.Printf("If you need this exact version of the plugin, you can download the binary from the source and install "+ - "it locally with `packer plugins install --path '' %q`.\n"+ - "This will install the plugin in version %q (required_plugins constraints may need to be updated).\n", - pr.Identifier.String(), desc.Version) - continue - } + err := fmt.Errorf(`binary for %[1]q release v%[2]s reported version %[3]q. - copyFrom, err = os.OpenFile(tempPluginPath, os.O_RDONLY, 0755) - if err != nil { - log.Printf("[ERROR] failed to re-open temporary plugin file %q: %s", tempPluginPath, err) - return nil, multierror.Append(errs, err) - } +Remote installation of pre-release binaries is unsupported an likely an upstream plugin issue. +We encourage you to open an issue on the plugin repository asking to update the version information. +If you require this specific version of the plugin, you can manually download the binary from the source and install +the versioned binary as %[3]q using the following command: - outputFile, err := os.OpenFile(localOutputFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755) - if err != nil { - err := fmt.Errorf("failed to create %s: %w", localOutputFileName, err) - errs = multierror.Append(errs, err) - return nil, errs - } - defer outputFile.Close() +packer plugins install --path '' %[1]q`, + pr.Identifier.String(), version, desc.Version) - if _, err := io.Copy(outputFile, copyFrom); err != nil { - err := fmt.Errorf("extract file: %w", err) errs = multierror.Append(errs, err) return nil, errs } - if _, err := outputFile.Seek(0, 0); err != nil { - err := fmt.Errorf("Error seeking begining of binary file for checksumming: %w", err) + if err := os.Rename(tmpBinFileName, outputFileName); err != nil { + err := fmt.Errorf("failed to write local binary file to plugin path: %s", err) errs = multierror.Append(errs, err) - log.Printf("[WARNING] %v, ignoring", err) - } - - cs, err := checksum.Checksummer.Sum(outputFile) - if err != nil { - err := fmt.Errorf("failed to checksum binary file: %s", err) - errs = multierror.Append(errs, err) - log.Printf("[WARNING] %v, ignoring", err) + return nil, errs } - - if err := os.WriteFile(localOutputFileName+checksum.Checksummer.FileExt(), []byte(hex.EncodeToString(cs)), 0644); err != nil { + if err := os.WriteFile(outputFileName+checksum.Checksummer.FileExt(), []byte(hex.EncodeToString(outputFileCS)), 0644); err != nil { err := fmt.Errorf("failed to write local binary checksum file: %s", err) errs = multierror.Append(errs, err) log.Printf("[WARNING] %v, ignoring", err) @@ -965,7 +930,7 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) // Success !! return &Installation{ - BinaryPath: strings.ReplaceAll(localOutputFileName, "\\", "/"), + BinaryPath: strings.ReplaceAll(outputFileName, "\\", "/"), Version: "v" + version.String(), }, nil } @@ -976,13 +941,11 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) } } - if errs == nil || errs.Len() == 0 { + if errs.ErrorOrNil() == nil { err := fmt.Errorf("could not find a local nor a remote checksum for plugin %q %q", pr.Identifier, pr.VersionConstraints) errs = multierror.Append(errs, err) } - errs = multierror.Append(errs, fmt.Errorf("could not install any compatible version of plugin %q", pr.Identifier)) - return nil, errs } From ff4a114406bca36852d76bbcd5271975aaed0a4c Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Fri, 3 May 2024 21:37:39 -0400 Subject: [PATCH 13/16] Add PrereleaseError for handling failed pre-release version installs ``` ~> packer init mondoo_req.pkr.hcl Failed getting the "github.com/mondoohq/cnspec" plugin: error: Remote installation of the plugin version 10.8.1-dev is unsupported. This is likely an upstream issue with the 10.8.1 release, which should be reported. If you require this specific version of the plugin, download the binary and install it manually. packer plugins install --path '' github.com/mondoohq/cnspec ``` --- packer/plugin-getter/plugins.go | 46 +++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 32a31075b00..5e897ddf6bd 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -93,6 +93,25 @@ func (rlerr *RateLimitError) Error() string { return s } +// PrereleaseInstallError is returned when a getter encounters the install of a pre-release version. +type PrereleaseInstallError struct { + RequestedVersion, ReportedVersion string + Source string +} + +func (pe *PrereleaseInstallError) Error() string { + s := strings.Builder{} + s.WriteString("error:\n") + fmt.Fprintf(&s, "Remote installation of the plugin version %s is unsupported.\n", pe.ReportedVersion) + + if pe.RequestedVersion != pe.ReportedVersion { + fmt.Fprintf(&s, "This is likely an upstream issue with the %s release, which should be reported.\n", pe.RequestedVersion) + } + s.WriteString("If you require this specific version of the plugin, download the binary and install it manually.\n") + fmt.Fprintf(&s, "\npacker plugins install --path '' %s\n", pe.Source) + return s.String() +} + func (pr Requirement) FilenamePrefix() string { if pr.Identifier == nil { return "packer-plugin-" @@ -869,7 +888,6 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) errs = multierror.Append(errs, err) log.Printf("[WARNING] %v, ignoring", err) } - outputFileCS, err := checksum.Checksummer.Sum(tmpOutputFile) if err != nil { err := fmt.Errorf("failed to checksum binary file: %s", err) @@ -896,25 +914,19 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) continue } - // Since releases only can be installed remotely, a non-empty prerelease version - // means something's not right on the release, as it should report a final version instead. + // Since only final releases can be installed remotely, a non-empty prerelease version + // means something's not right on the release, as it should report a final version. // // Therefore to avoid surprises (and avoid being able to install a version that // cannot be loaded), we error here, and advise users to manually install the plugin if they // need it. if descVersion.Prerelease() != "" { - err := fmt.Errorf(`binary for %[1]q release v%[2]s reported version %[3]q. - -Remote installation of pre-release binaries is unsupported an likely an upstream plugin issue. -We encourage you to open an issue on the plugin repository asking to update the version information. -If you require this specific version of the plugin, you can manually download the binary from the source and install -the versioned binary as %[3]q using the following command: - -packer plugins install --path '' %[1]q`, - pr.Identifier.String(), version, desc.Version) - - errs = multierror.Append(errs, err) - return nil, errs + err := PrereleaseInstallError{ + Source: pr.Identifier.String(), + RequestedVersion: version.String(), + ReportedVersion: desc.Version, + } + return nil, &err } if err := os.Rename(tmpBinFileName, outputFileName); err != nil { @@ -926,6 +938,8 @@ packer plugins install --path '' %[1]q`, err := fmt.Errorf("failed to write local binary checksum file: %s", err) errs = multierror.Append(errs, err) log.Printf("[WARNING] %v, ignoring", err) + os.Remove(outputFileName) + continue } // Success !! @@ -934,10 +948,8 @@ packer plugins install --path '' %[1]q`, Version: "v" + version.String(), }, nil } - } } - } } From e6555d21b858bc138c3c87a567449625f5c91f93 Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Mon, 6 May 2024 22:10:45 -0400 Subject: [PATCH 14/16] Use single buffer for storing binary file before writes * Only create plugin directories if there is potential plugin install --- packer/plugin-getter/plugins.go | 69 +++++++++++++++++---------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 5e897ddf6bd..c36b9ede4e6 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -5,6 +5,7 @@ package plugingetter import ( "archive/zip" + "bytes" "encoding/hex" "encoding/json" "fmt" @@ -782,13 +783,6 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) } } - // create directories if need be - if err := os.MkdirAll(outputFolder, 0755); err != nil { - err := fmt.Errorf("could not create plugin folder %q: %w", outputFolder, err) - errs = multierror.Append(errs, err) - log.Printf("[TRACE] %s", err.Error()) - return nil, errs - } for _, getter := range getters { // start fetching binary remoteZipFile, err := getter.Get("zip", GetOptions{ @@ -823,7 +817,6 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) errs = multierror.Append(errs, err) continue } - if _, err := tmpFile.Seek(0, 0); err != nil { err := fmt.Errorf("Error seeking beginning of temporary file for checksumming, continuing: %w", err) errs = multierror.Append(errs, err) @@ -835,7 +828,6 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) errs = multierror.Append(errs, err) continue } - zr, err := zip.OpenReader(tmpFile.Name()) if err != nil { errs = multierror.Append(errs, fmt.Errorf("zip : %v", err)) @@ -860,40 +852,28 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) return nil, errs } + var outputFileData bytes.Buffer + if _, err := io.Copy(&outputFileData, copyFrom); err != nil { + err := fmt.Errorf("extract file: %w", err) + errs = multierror.Append(errs, err) + return nil, errs + } tmpBinFileName := filepath.Join(os.TempDir(), expectedBinaryFilename) + tmpOutputFile, err := os.OpenFile(tmpBinFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755) if err != nil { err = fmt.Errorf("could not create temporary file to download plugin: %w", err) errs = multierror.Append(errs, err) return nil, errs } defer func() { - tmpBinPath := tmpBinFileName - os.Remove(tmpBinPath) + os.Remove(tmpBinFileName) }() - tmpOutputFile, err := os.OpenFile(tmpBinFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755) - if err != nil { - err := fmt.Errorf("failed to create %s: %w", tmpBinFileName, err) - errs = multierror.Append(errs, err) - return nil, errs - } - if _, err := io.Copy(tmpOutputFile, copyFrom); err != nil { + if _, err := tmpOutputFile.Write(outputFileData.Bytes()); err != nil { err := fmt.Errorf("extract file: %w", err) errs = multierror.Append(errs, err) return nil, errs } - - if _, err := tmpOutputFile.Seek(0, 0); err != nil { - err := fmt.Errorf("Error seeking beginning of binary file for checksumming: %w", err) - errs = multierror.Append(errs, err) - log.Printf("[WARNING] %v, ignoring", err) - } - outputFileCS, err := checksum.Checksummer.Sum(tmpOutputFile) - if err != nil { - err := fmt.Errorf("failed to checksum binary file: %s", err) - errs = multierror.Append(errs, err) - log.Printf("[WARNING] %v, ignoring", err) - } tmpOutputFile.Close() desc, err := GetPluginDescription(tmpBinFileName) @@ -913,7 +893,6 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) errs = multierror.Append(errs, err) continue } - // Since only final releases can be installed remotely, a non-empty prerelease version // means something's not right on the release, as it should report a final version. // @@ -929,12 +908,34 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) return nil, &err } - if err := os.Rename(tmpBinFileName, outputFileName); err != nil { - err := fmt.Errorf("failed to write local binary file to plugin path: %s", err) + // create directories if need be + if err := os.MkdirAll(outputFolder, 0755); err != nil { + err := fmt.Errorf("could not create plugin folder %q: %w", outputFolder, err) + errs = multierror.Append(errs, err) + log.Printf("[TRACE] %s", err.Error()) + return nil, errs + } + + outputFile, err := os.OpenFile(outputFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755) + if err != nil { + err = fmt.Errorf("could not create final plugin binary file: %w", err) errs = multierror.Append(errs, err) return nil, errs } - if err := os.WriteFile(outputFileName+checksum.Checksummer.FileExt(), []byte(hex.EncodeToString(outputFileCS)), 0644); err != nil { + if _, err := outputFile.Write(outputFileData.Bytes()); err != nil { + err = fmt.Errorf("could not write final plugin binary file: %w", err) + errs = multierror.Append(errs, err) + return nil, errs + } + outputFile.Close() + + cs, err := checksum.Checksummer.Sum(&outputFileData) + if err != nil { + err := fmt.Errorf("failed to checksum binary file: %s", err) + errs = multierror.Append(errs, err) + log.Printf("[WARNING] %v, ignoring", err) + } + if err := os.WriteFile(outputFileName+checksum.Checksummer.FileExt(), []byte(hex.EncodeToString(cs)), 0644); err != nil { err := fmt.Errorf("failed to write local binary checksum file: %s", err) errs = multierror.Append(errs, err) log.Printf("[WARNING] %v, ignoring", err) From 15c40c2972d5da046104cb652d8ba557c9a9f6df Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Mon, 6 May 2024 22:24:51 -0400 Subject: [PATCH 15/16] Add ConintuableInstallError for continuing installation on version mismatches --- packer/plugin-getter/plugins.go | 90 ++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 35 deletions(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index c36b9ede4e6..138bb084d6d 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -8,6 +8,7 @@ import ( "bytes" "encoding/hex" "encoding/json" + "errors" "fmt" "io" "io/fs" @@ -100,19 +101,28 @@ type PrereleaseInstallError struct { Source string } -func (pe *PrereleaseInstallError) Error() string { +func (e *PrereleaseInstallError) Error() string { s := strings.Builder{} - s.WriteString("error:\n") - fmt.Fprintf(&s, "Remote installation of the plugin version %s is unsupported.\n", pe.ReportedVersion) + fmt.Fprintf(&s, "Error: Remote installation of the plugin version %s is unsupported.\n", e.ReportedVersion) - if pe.RequestedVersion != pe.ReportedVersion { - fmt.Fprintf(&s, "This is likely an upstream issue with the %s release, which should be reported.\n", pe.RequestedVersion) + if e.RequestedVersion != e.ReportedVersion { + fmt.Fprintf(&s, "This is likely an upstream issue with the %s release, which should be reported.\n", e.RequestedVersion) } s.WriteString("If you require this specific version of the plugin, download the binary and install it manually.\n") - fmt.Fprintf(&s, "\npacker plugins install --path '' %s\n", pe.Source) + fmt.Fprintf(&s, "\npacker plugins install --path '' %s\n", e.Source) return s.String() } +// ContinuableInstallError describe a failed getter install that is +// capable of falling back to next available version. +type ContinuableInstallError struct { + Err error +} + +func (e *ContinuableInstallError) Error() string { + return fmt.Sprintf("Continuing to next available version: %s", e.Err) +} + func (pr Requirement) FilenamePrefix() string { if pr.Identifier == nil { return "packer-plugin-" @@ -876,36 +886,13 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) } tmpOutputFile.Close() - desc, err := GetPluginDescription(tmpBinFileName) - if err != nil { - err := fmt.Errorf("failed to describe plugin binary %q: %s", tmpBinFileName, err) - errs = multierror.Append(errs, err) - continue - } - descVersion, err := goversion.NewSemver(desc.Version) - if err != nil { - err := fmt.Errorf("invalid self-reported version %q: %s", desc.Version, err) + if err := checkVersion(tmpBinFileName, pr.Identifier.String(), version); err != nil { errs = multierror.Append(errs, err) - continue - } - if descVersion.Core().Compare(version.Core()) != 0 { - err := fmt.Errorf("binary reported version (%q) is different from the expected %q, skipping", desc.Version, version.String()) - errs = multierror.Append(errs, err) - continue - } - // Since only final releases can be installed remotely, a non-empty prerelease version - // means something's not right on the release, as it should report a final version. - // - // Therefore to avoid surprises (and avoid being able to install a version that - // cannot be loaded), we error here, and advise users to manually install the plugin if they - // need it. - if descVersion.Prerelease() != "" { - err := PrereleaseInstallError{ - Source: pr.Identifier.String(), - RequestedVersion: version.String(), - ReportedVersion: desc.Version, + var continuableError *ContinuableInstallError + if errors.As(err, &continuableError) { + continue } - return nil, &err + return nil, errs } // create directories if need be @@ -915,7 +902,6 @@ func (pr *Requirement) InstallLatest(opts InstallOptions) (*Installation, error) log.Printf("[TRACE] %s", err.Error()) return nil, errs } - outputFile, err := os.OpenFile(outputFileName, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755) if err != nil { err = fmt.Errorf("could not create final plugin binary file: %w", err) @@ -974,6 +960,40 @@ func GetPluginDescription(pluginPath string) (pluginsdk.SetDescription, error) { return desc, err } +// checkVersion checks the described version of a plugin binary against the requested version constriant. +// A ContinuableInstallError is returned upon a version mismatch to indicate that the caller should try the next +// available version. A PrereleaseInstallError is returned to indicate an unsupported version install. +func checkVersion(binPath string, identifier string, version *goversion.Version) error { + desc, err := GetPluginDescription(binPath) + if err != nil { + err := fmt.Errorf("failed to describe plugin binary %q: %s", binPath, err) + return &ContinuableInstallError{Err: err} + } + descVersion, err := goversion.NewSemver(desc.Version) + if err != nil { + err := fmt.Errorf("invalid self-reported version %q: %s", desc.Version, err) + return &ContinuableInstallError{Err: err} + } + if descVersion.Core().Compare(version.Core()) != 0 { + err := fmt.Errorf("binary reported version (%q) is different from the expected %q, skipping", desc.Version, version.String()) + return &ContinuableInstallError{Err: err} + } + // Since only final releases can be installed remotely, a non-empty prerelease version + // means something's not right on the release, as it should report a final version. + // + // Therefore to avoid surprises (and avoid being able to install a version that + // cannot be loaded), we error here, and advise users to manually install the plugin if they + // need it. + if descVersion.Prerelease() != "" { + return &PrereleaseInstallError{ + Source: identifier, + RequestedVersion: version.String(), + ReportedVersion: desc.Version, + } + } + return nil +} + func init() { var err error // Should never error if both components are set From e5434bf2a7a7742060eead4ae5e419eeabdefa4c Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Tue, 7 May 2024 10:51:50 -0400 Subject: [PATCH 16/16] Add check to prevent the installation of version constraints matching a prerelease * Refactor InstallError string messages --- packer/plugin-getter/plugins.go | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/packer/plugin-getter/plugins.go b/packer/plugin-getter/plugins.go index 138bb084d6d..9813e35afde 100644 --- a/packer/plugin-getter/plugins.go +++ b/packer/plugin-getter/plugins.go @@ -97,19 +97,17 @@ func (rlerr *RateLimitError) Error() string { // PrereleaseInstallError is returned when a getter encounters the install of a pre-release version. type PrereleaseInstallError struct { - RequestedVersion, ReportedVersion string - Source string + PluginSrc string + Err error } func (e *PrereleaseInstallError) Error() string { - s := strings.Builder{} - fmt.Fprintf(&s, "Error: Remote installation of the plugin version %s is unsupported.\n", e.ReportedVersion) - - if e.RequestedVersion != e.ReportedVersion { - fmt.Fprintf(&s, "This is likely an upstream issue with the %s release, which should be reported.\n", e.RequestedVersion) - } + var s strings.Builder + s.WriteString(e.Err.Error() + "\n") + s.WriteString("Remote installation of pre-release plugin versions is unsupported.\n") + s.WriteString("This is likely an upstream issue, which should be reported.\n") s.WriteString("If you require this specific version of the plugin, download the binary and install it manually.\n") - fmt.Fprintf(&s, "\npacker plugins install --path '' %s\n", e.Source) + s.WriteString("\npacker plugins install --path '' " + e.PluginSrc) return s.String() } @@ -978,6 +976,12 @@ func checkVersion(binPath string, identifier string, version *goversion.Version) err := fmt.Errorf("binary reported version (%q) is different from the expected %q, skipping", desc.Version, version.String()) return &ContinuableInstallError{Err: err} } + if version.Prerelease() != "" { + return &PrereleaseInstallError{ + PluginSrc: identifier, + Err: errors.New("binary reported a pre-release version of " + version.String()), + } + } // Since only final releases can be installed remotely, a non-empty prerelease version // means something's not right on the release, as it should report a final version. // @@ -986,9 +990,8 @@ func checkVersion(binPath string, identifier string, version *goversion.Version) // need it. if descVersion.Prerelease() != "" { return &PrereleaseInstallError{ - Source: identifier, - RequestedVersion: version.String(), - ReportedVersion: desc.Version, + PluginSrc: identifier, + Err: errors.New("binary reported a pre-release version of " + descVersion.String()), } } return nil