Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Packer tracks Version and Plugins Metadata #12860

Merged
merged 33 commits into from
Mar 5, 2024

Conversation

devashish-patel
Copy link
Contributor

@devashish-patel devashish-patel commented Feb 28, 2024

  • Track Packer Version metadata
  • Track Plugins metadata

lbajolet-hashicorp and others added 23 commits February 23, 2024 16:28
Since we're removing the alternative plugin installation directories in
favour of only supporting installing them in the PACKER_PLUGIN_PATH
directory, we only return one directory when getting the known plugin
directories.
Since we'll be only accepting plugins installed locally along with a
shasum, and those that adopt the convention we introduced with
required_plugins, we remove support for plugins that only have a
packer-plugin-.* type name.
Since we'll only look in the plugin directory, and not from multiple
sources now, for installing/listing plugins, we can simplify the
structures which used to accept multiple directories so they only accept
one.
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.
The comments weren't capitalised correctly, and the directory example
included the `packer-plugin-' prefix, which should not be the case in
reality.
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.
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.
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.
Since the plugins remove subcommand now only loads valid plugins, it
cannot run on mock data anymore, and the logic for creating a valid
plugin hierarchy is not present in this repository, but does in another,
so we move those tests from here to that repository in order to continue
testing them.
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 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 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).
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.
Since we added the capability for the plugin discovery to ignore
installed pre-releases of plugins, we give that capacity to the validate
and build commands, through a new command-line flag: release-only.
Since we now support loading pre-releases with Packer subcommands, we
relax the contraints we had placed on the `--path' option, so it will
accept any `-dev' binary, in addition to final releases.
In order to test the Packer subcommands, some tests rely on a plugin:
github.com/sylviamoss/comment.

This plugin is outdated compared to our SDK, and some of the binaries
releases don't match what is expected by Packer, i.e. v1.0.0 vs. v0.2.8.

To fix that, we migrate to use the hashicups plugin, which is our demo
plugin for Packer, and is actually maintained, and up-to-date, which
will make those tests more reliable moving forward.
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.
@devashish-patel devashish-patel self-assigned this Feb 28, 2024
@devashish-patel devashish-patel added the core Core components of Packer label Feb 28, 2024
Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, left a few comments, I think the structure/code that tracks the components/plugin relationship can be simplified, but the approach is sound and minimally intrusive, we're on the right track here!

packer/all_plugins_storage.go Outdated Show resolved Hide resolved
packer/all_plugins_storage.go Outdated Show resolved Hide resolved
packer/core.go Outdated Show resolved Hide resolved
packer/all_plugins_storage.go Outdated Show resolved Hide resolved
internal/hcp/registry/json.go Outdated Show resolved Hide resolved
packer/all_plugins_storage.go Outdated Show resolved Hide resolved
packer/all_plugins_storage.go Outdated Show resolved Hide resolved
hcl2template/types.packer_config.go Show resolved Hide resolved
internal/hcp/registry/json.go Outdated Show resolved Hide resolved
packer/build.go Outdated Show resolved Hide resolved
packer/all_plugins_storage.go Outdated Show resolved Hide resolved
packer/core.go Outdated Show resolved Hide resolved
@devashish-patel devashish-patel changed the title Track Plugin Metadata POC: Packer Version and Plugins Metadata Feb 29, 2024
Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last nitpick, but the code LGTM in the current state!

packer/plugin.go Outdated Show resolved Hide resolved
Base automatically changed from support_prerelease_loading to main March 1, 2024 14:04
@nywilken
Copy link
Contributor

nywilken commented Mar 1, 2024

Please rebase onto latest main

packer/build.go Outdated Show resolved Hide resolved
@devashish-patel devashish-patel marked this pull request as ready for review March 5, 2024 17:39
@devashish-patel devashish-patel requested a review from a team as a code owner March 5, 2024 17:39
@devashish-patel devashish-patel changed the title POC: Packer Version and Plugins Metadata Packer tracks Version and Plugins Metadata Mar 5, 2024
@devashish-patel devashish-patel changed the base branch from main to build-metadata-phase-1 March 5, 2024 18:08
@devashish-patel devashish-patel merged commit 9239e22 into build-metadata-phase-1 Mar 5, 2024
11 checks passed
@devashish-patel devashish-patel deleted the plugin-metadata branch March 5, 2024 19:43
Copy link

github-actions bot commented Apr 5, 2024

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core Core components of Packer hcp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants