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: don't load plugins with metadata in name #12980

Merged
merged 1 commit into from
May 16, 2024

Conversation

lbajolet-hashicorp
Copy link
Contributor

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.

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.
@lbajolet-hashicorp lbajolet-hashicorp requested a review from a team as a code owner May 15, 2024 20:45
@@ -280,6 +280,11 @@ func (pr Requirement) ListInstallations(opts ListInstallationsOptions) (InstallL
continue
}

if ver.Metadata() != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

To confirm this case. On install a binary version that contains metadata will be converted to a final version, correct?

If so then a user can not install using packer init or packer plugins install but if they drop the binary into a directory manually this change will prevent it from loading. Is that correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

The test in acc_test_logic indicate :point_up to be true but I want to confirm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That should be the case indeed, packer plugins install and packer init scrub the metadata part from a release/local's name, the describe command will still report it but ignore it for sorting purposes (as per semver), but then yes, if a plugin is installed with a +metadata in its version, we reject it explicitely.

Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

LGTM

@nywilken nywilken merged commit 682968d into main May 16, 2024
12 checks passed
@lbajolet-hashicorp lbajolet-hashicorp deleted the fix_metadata_loading_in_name branch May 16, 2024 17:11
Copy link

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 Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants