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

Chore: improve plugin source parsing #12863

Closed

Conversation

phuhung273
Copy link

@phuhung273 phuhung273 commented Mar 3, 2024

Related Issues

Motivation

Hi team, when trying to implement a custom domain plugin getter to fix this API rate limitting issue, I realized that it would require a huge PR.

Knowing it would not possible for new contributor to make such a big change. I create this one as a first step to point packer plugin install to another domain instead of github.

Please be aware that this PR does not resolve above issues.

Changes

  • Allow plugin source string to have more than 3 components for more flexible hostname
    Eg: source = "artifactory.mycompany.com/my-remote-repo/hashicorp/amazon"
    Will result in hostname = "artifactory.mycompany.com/my-remote-repo"

Test Evidences

  • Log showing that hostname is parsed correctly
  • Packer still reject hostname which is not github
Screenshot 2024-03-03 141454

@phuhung273 phuhung273 requested a review from a team as a code owner March 3, 2024 07:36
@hashicorp-cla
Copy link

hashicorp-cla commented Mar 3, 2024

CLA assistant check
All committers have signed the CLA.

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.

Hi @phuhung273,

Thanks for taking a jab at those issues, this is something we have on our radar, and will start working on soon.

In that optic, I'm not sure what to do with your PR, as the sources format will likely change in some way when we begin planning the changes to the getters and more.
I'll keep it open for now to allow for discussions to happen, but I don't think we can merge this anytime soon given the amount of unknowns for what's to come on this front.

Hopefully we'll have something to show in the coming months regarding those use-cases!

@phuhung273
Copy link
Author

This issue has been around since 1.7. Until now, most people still happy with the workaround to install locally.

But as this change has been merged 664700d and will be available in 1.11 (please correct me if am wrong), it is becoming more urgent.

@lbajolet-hashicorp
Copy link
Contributor

Hi @phuhung273,

Regarding the commit you linked, I can see why the worries indeed, but rest assured, we are not forbidding local plugin installation, we're changing how they're loaded in order to remove some friction here.
You'll still be able to install plugins manually through packer plugins install --path, so a plugin gets installed locally with the convention Packer will require. Please note that the command already exists in 1.10.0, with a minor difference in that it only lets you install releases of a plugin (i.e. the VersionPrerelease must be empty).

Hopefully this is something you can use to install your plugin? If you try it out with your plugins right now, does it work as expected for you?

@phuhung273
Copy link
Author

Thanks @lbajolet-hashicorp for your detail explanation. LGTM. I'll wait for the plan on source format convention then.

@lbajolet-hashicorp
Copy link
Contributor

Hi @phuhung273,

FYI we just released v1.11.0-alpha of Packer, this contains the changes to the loading process I detailed in my previous responses.

If you can please give it a spin with your process, and let us know if this alleviates your concerns.

@nywilken nywilken added the invalid Out of scope/alignment with the project, or issue is expected, intended behavior label May 15, 2024
@nywilken
Copy link
Contributor

nywilken commented May 15, 2024

@phuhung273 thank you again for opening up this change request. We opted to take a different approach that may suit your team's needs. Here is a preview link to the documentation explaining how to work with non-GitHub sources .

This change will ship with Packer 1.11, which we will release as a beta tomorrow. Please feel free to give the documentation a read and provide any thoughts or feedback.

I'll keep this PR open for now, and we can continue our conversation. I will label it to denote we will not be merging it.

@nywilken
Copy link
Contributor

nywilken commented Jun 3, 2024

👋🏽 closing this now that Packer 1.11 has been released. The updated packer plugins install command will be helpful in installing plugins with a custom source address.

You can download the plugin from your Artifactory server and install it manually using the --path flag. The provided path should be the location for the extracted plugin binary and the source address would be artifactory.mycompany.com/my-remote-repo/hashicorp/amazon

packer plugins install --path my-plugin artifactory.mycompany.com/my-remote-repo/hashicorp/amazon

Using this workflow you would also be able to add a required_plugins block to your template if using HCL2 with your custom source address. Please note that packer init only works with GitHub sources but Packer will honor any version pinning for the custom source address if the plugin is installed manually.

packer {
  required_plugins {
     amazon = {
        source = "artifactory.mycompany.com/my-remote-repo/hashicorp/amazon"
        version = ">1.2.0"
      }
   }
}

@nywilken nywilken closed this Jun 3, 2024
Copy link

github-actions bot commented Jul 4, 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 Jul 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
invalid Out of scope/alignment with the project, or issue is expected, intended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants