In a couple places in the codebase we didn't check the errors from
functions we execute.
In some cases this is harmless (or at least ignorable), but others may
need to log what went wrong, so for all the reported occurrences we
either ignore explicitly or handle the error with a log.
Some of the variables we create are flagged by our linters as
ineffective assignments, which makes sense as those are generally fed by
code below, so we don't need to use the declaration/assignation syntax
(:=) but instead can fall back to using var with a type to get the zero
value of the declared entity.
Since the enumer implementation we used hadn't been updated for 5+
years, this didn't work with recent linux/go versions, and enumer
crashed while attempting to parse/analyse the source files.
There's another alternative on Github, forked from the one we used,
which seems more maintained now, and does produce the expected files in
Packer.
When Discovering plugins installed through the `Discover` function, we
use the base name of the plugin binary we discovered preliminarly, then
we match its name against a regex to extract the prefix for the plugin's
components.
Extracting the base path used to be done with `path.Base`, which while
working perfectly on UNIX systems, does not on Windows as it uses `\\`
as their path separator.
To circumvent this problem, we use the `filepath` package to extract the
base name of the plugin instead, making the discovery logic work again
on Windows.
Listing installed plugins on Windows requires the extension to be set in
the ListOptions, otherwise they are not discovered.
While working on the discovery code, and consolidating it in a single
location, we've forgotten to pass the argument to ListInstallations, so
that makes it impossible to automatically discover installed components
on Windows.
This commit fixes this issue for the plugins required, and the general
discovery process during build/validate.
If a plugin is installed in the PACKER_PLUGIN_PATH, and its version
contains metadata, we reject it. This is because metadata is free-form
data, which could then make it possible to have multiple conflicting
versions of a plugin installed, so we don't support it and explicitely
reject plugins like those.
A valid plugin with metadata in its version information should be
installed without its metadata part, so there can only be one variant of
the plugin installed at a specific version.
The ParsePluginSource function can be invoked from either a HCL2 context
(when parsing a required_plugins block), or from the command-line
itself.
While in the first context a hcl.Diagnostics is coherent, in case the
source to parse is a command-line argument, for example when installing
or removing a plugin, the error message cannot have an HCL context,
leading to errors that are incorrectly prefixed by a <nil> string dure
to the lack of a reference to attach the diagnostic to.
Therefore, in order to fix this behaviour, the logic that parses plugin
sources now returns an error, and attaching the error to an HCL subject
is done independently, if needed.
When specifying/installing plugins, a source URI is required for Packer
to be able to locate or install a plugin to the local plugin hierarchy.
The plugin hierarchy is based on the plugin source, where each component
in this hierarchy will become a directory.
In order to avoid sources with too many levels of nesting, causing a lot
of mkdirs, we limit the number of sources to 16 in this commit, this
should be long enough for most of our users.
When running a packer command on an HCL2 template, depending on whether
or not there are required_plugin blocks defined, Packer may need to
discover and register a plugin's components multiple times.
This is the behaviour ever since those blocks were introduced to Packer,
but given we are doing the operation multiple times, this is suboptimal.
This commit changes the way things works by first doing the restricted
discovery of plugins (as dictated by required_plugins), then proceeding
to the global discovery, with the change that subsequent component
discoveries will not have precedence over those pre-discovered anymore.
This allows us to invert the call order of both discovery phases safely,
and maintains the constraints described in the templates.
When a user defines PACKER_PLUGIN_PATH in their environment, we need to
error if their path defines multiple directories separated by `:`.
This used to be supported, but this is removed with 1.11 as we're
simplifying the loading process for plugins, so we opted to fall-back to
only one plugin directory supported.
```
~> 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 '<plugin_binary>' github.com/mondoohq/cnspec
```
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.
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.
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.
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.
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.
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.
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.
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.
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.
The source parsing logic was heavily directed towards Github compatible
source URIs, however if we want to support more cases, we need to make
sure we are able to specify those URIs, and to load plugins installed
from those sources.
Right now, since the getters available are only github.com, we will not
support remotely instlling plugins from sources other than github.com,
with the same set of constraints as before. However, we do support now
installing from a local plugin binary to any kind of source, and we
support loading them, including if a template wants this plugin
installed locally with version constraints.
The SDK's Ui interface has had some additions recently for supporting
format-strings and their arguments as paramereters to Say, Ask and
Error.
These changes to the interface means that all the implementations of
that interface need to match the definition, so in this case the
implementations we had in packer/ui.go need to be updated to support
those functions.
When introduced back in January, the version variable extracted from the
binary name had a typo in its name, and was named `protocolVerionStr'
instead of `protocolVersionStr'.
This commit was already merged into main, so it's too late to fix at
introduction site, but we can fix it today as a separate commit ontop
the stack.
If a plugin is installed manually, its version number could be valid but
non-canonical (ex: 1.2.3 vs 01.002.0003).
Since these two versions refer to the same version, but the looks are
different, this may become ambiguous which version should be loaded.
To avoid such a situation, we reject explicitely non-canonical version
numbers in plugins, but only in path, we're aware that because of
metadata, the version from `describe' may already differ from the file
name.
The former way mock plugin sets were created meant that no API version
was set, and since it's private in the SDK, it cannot be set outside of
the package itself.
Fortunately, there is a NewSet function we can call, which initialises a
set properly so we can fill-in the information later.
However, because all the set maps were created in a `var` section, we
cannot create the set with `NewSet`, and then fill the information in
(outside of if there was a fluent interface, but this isn't the case
here).
We therefore opted to keep the variables defined and accessible
globally, but gone through a `init` function to initialise their values
for tests.
As with versions, API versions are useful possibly for ordering plugin
installations in order for Packer to choose which plugin to load.
This could be unnecessary as API versions are stable, and only in dev
plugins this could be a problem normally (there shouldn't be two same
releases of a plugin), but this cements API version in ordering plugins
so we avoid surprises later down the line.
As with the version of the plugin, the API version should also match
between the path and the self-reported API version from the describe
command.
This was not checked before, so users could masquerade a plugin's use of
an API version that may be incompatible with Packer.
To avoid this problem, we make sure both versions are the same, so that
they work as expected.
When Packer discovers binary a bunch of checks are performed, which
ultimately end with a checksum match check.
This however should be the very first thing we do, even before
attempting to run `describe' on the plugin binary we're discovering.
So this commit moves this checksum match to the top of the discovery
process for binaries.
When discovering the installed plugins locally, we perform a couple of
checks on the version, namely that it is valid, if it is a prerelease,
it needs to be a dev, and that the self-reported version matches the one
hinted at through the name of the binary.
This was done through regexes, but those were a wee bit simple when
dealing with versions that have metadata. Those binaries would be
completely ignored by Packer, and never loaded, although they are a
valid use case.
The version library we already used supports those however, and
comparisons are more reliable with them.
So, in order to simplify our code, and make it more reliable, we're
exclusively using this library to perform parsing and comparisons of
versions during the discovery phase.
While migrating to a unified approach for loading plugins, the API
major and minor versions were not added to the constraints for
discovering plugins from the environment, leading to Packer potentially
considering plugins that are not compatible with itself.
This should not be possible, but was due to this omission, which we fix
with this commit.
Since we now support loading pre-releases, we also want Packer to be
able to ignore them by user demand, so we put in place the
infrastructure to modulate this.
When Packer is loaded, we used to perform plugin discovery.
This was done for every call to Packer, including when it is executed as
a plugin, arguably against what the comments document.
Doing this as early in the loading process makes it harder to change
this behaviour, as we'd need to introduce flags aside from the rest, and
handle them manually, which is not optimal.
Therefore, we change this: now when Packer starts executing, it will not
attempt to discover installed plugins anymore, and instead will only try
to load them when a configuration has been parsed, and is being used to
perform actions (typically build/validate).
When a pre-release version of a plugin is locally installed, it may or
may not be loaded depending on the constraints expressed in the template
being executed.
If the template contains constraints for loading the plugin, it would be
ignored, while if that wasn't present, it would be loaded.
This is inconsistent, and deserves to be addressed, which is what this
commit does.
With this change, plugin pre-releases are now loaded, provided the
version reported matches the constraints, independently from its
pre-release suffix `-dev'.
Also, if a release with the same version is also installed alongside the
pre-release version of a plugin, it will have precedence over the
pre-release.
When Packer orders the plugins by their version, we'd assumed it was
ordered from highest to lowest.
However, this was the case because our implementation of `Less' was
actually returning `>=', which is not what it was supposed to do.
This commit therefore fixes the implementation of `Less' to do what it
is documented to do, and ensures the behaviour is correct through
additional testing.
Changing this requires some changes to the loading process as well
because of the aforementioned assumption with regards to ordering.
When a plugin is loaded from Packer, we now check that the version it
reports matches what the name implies. In case there's a mismatch, we
log, and reject the binary.
Right now we had two paths for discovering installed plugins, i.e.
through plugin-getter's `ListInstallations' function, or through the
`Discover' call, which relied on a glob to list the installations.
This was required since we allowed plugins to be installed in multiple
locations, and with different constraints.
Now that we force a certain convention, we can consolidate the logic
into ListInstallations, and rely on that logic in `Discover' to load a
plugin into the PluginConfig for the current Packer run.
The plugin installation list should be sorted according to a name first,
then version second basis.
Right now, we only rely on the glob to add plugin installs to this list,
making the order unreliable since the lexicographical order is not the
order in which we want to see the same plugin ordered (e.g. v1.0.9 >
1.0.10).
To fix this, we implement a logic for sorting a list of installations
that does what's described above with more accuracy.
Since the plugin directory should now be unique instead of a list, we
can use it directly from the receiver's structure instead of as a
parameter to the function.
Lucas Bajolet
Lucas Bajolet
IAMDAVID0920
Wilken Rivera
Devashish