command: add --path flag to packer plugins install #12643

lbajolet-hashicorp · 2023-10-03T15:56:17Z

This new flag allows the packer plugins install command to install a plugin from a local binary rather than from Github.

This command will only call describe on the plugin, and won't do any further checks for functionality. The SHA256SUM will be directly computed from the binary, so as with anything manual and potentially sourced by the community, extra care should be applied when invoking this.

NOTE: marking as draft as it is not yet meant to be merged into main, or released soon. This is experimental and we'll decide what to do with it later.

vercel · 2023-10-03T15:56:25Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Updated (UTC)
packer	⬜️ Ignored (Inspect)		Oct 3, 2023 3:56pm

nywilken

I like the approach and understand why the logic for FlagSet is specified within the install command.

command/init.go

nywilken · 2023-11-30T19:40:53Z

command/plugins_install.go

+		}})
+	}
+
+	binaryPath := fmt.Sprintf(


For windows binaries we will need the .exe extension as well.

nywilken · 2023-11-30T20:07:00Z

command/plugins_install.go

@@ -128,3 +198,167 @@ func (c *PluginsInstallCommand) RunContext(buildCtx context.Context, args []stri

 	return 0
 }
+
+func (c *PluginsInstallCommand) InstallFromBinary(args *PluginsInstallArgs) int {


I see what you are doing here. Is there a way to reuse the Checksummer for obtaining and write the checksum file, the InFolders, and BinaryInstallOptions for generating file names.

I have to think through this a little but would there be any benefit to adding this as a method on plugingetter.Requirement?

You can write the code to be something like the following and reuse a bit of the logic from InstallLatest along with the call to describe you have in InstallFromBinary.

// a plugin requirement that matches them all pluginRequirement := plugingetter.Requirement{ Identifier: plugin, } if args.Version != "" { constraints, err := version.NewConstraint(args.Version) if err != nil { c.Ui.Error(err.Error()) return 1 } pluginRequirement.VersionConstraints = constraints } if runtime.GOOS == "windows" && opts.Ext == "" { opts.BinaryInstallationOptions.Ext = ".exe" } var newInstall *plugingetter.Installation var err error // If we did specify a binary to install the plugin from, we ignore // the Github-based getter in favour of installing it directly. if args.PluginPath != "" { newInstall, err := pluignRequirement.InstallLocalBinary(args.PluginPath, plugingetter.InstallOptions{ InFolders: opts.FromFolders, BinaryInstallationOptions: opts.BinaryInstallationOptions, Force: args.Force, //This may not be needed. }) } else { getters := []plugingetter.Getter{ &github.Getter{ UserAgent: "packer-getter-github-" + pkrversion.String(), }, } newInstall, err := pluginRequirement.InstallLatest(plugingetter.InstallOptions{ InFolders: opts.FromFolders, BinaryInstallationOptions: opts.BinaryInstallationOptions, Getters: getters, Force: args.Force, }) } ....

command/plugins_install.go

This new flag allows the `packer plugins install' command to install a plugin from a local binary rather than from Github. This command will only call `describe' on the plugin, and won't do any further checks for functionality. The SHA256SUM will be directly computed from the binary, so as with anything manual and potentially sourced by the community, extra care should be applied when invoking this.

The --force option for packer init and packer plugins install enforces installation of a plugin, even if it is already locally installed. This will become useful if for some reason a pre-existing plugin binary/version is already installed, and we want to overwrite it.

To avoid plugins being installed with a specific version when a path is used for installing a plugin from a locally sourced plugin binary, we explicitly reject the combination of both a path and a version for plugins install.

When installing a plugin with packer plugins install --path, we only accept release versions of a plugin, as otherwise the loading can be inconsistent if for example a user specifies a required_plugins block in their template, in which case the plugins will be ignored. Until we have a simpler loading scheme then, we will reject non-release versions of plugins to avoid confusion.

nywilken

This looks great. Glad we were able to refactor for consistency I left a few suggestions to consolidate code a bit more and tighten up the scope of a few variables.

nywilken · 2023-12-01T19:34:13Z

command/plugins_install.go

 }

-func (c *PluginsInstallCommand) RunContext(buildCtx context.Context, args []string) int {
+func (pa *PluginsInstallArgs) AddFlagSets(flags *flag.FlagSet) {
+	flags.StringVar(&pa.PluginPath, "path", "", "install the plugin from a specific path")


Suggested change

flags.StringVar(&pa.PluginPath, "path", "", "install the plugin from a specific path")

flags.StringVar(&pa.PluginPath, "path", "", "install the binary specified by path as a Packer plugin.")

nywilken · 2023-12-01T19:34:39Z

command/plugins_install.go

-func (c *PluginsInstallCommand) RunContext(buildCtx context.Context, args []string) int {
+func (pa *PluginsInstallArgs) AddFlagSets(flags *flag.FlagSet) {
+	flags.StringVar(&pa.PluginPath, "path", "", "install the plugin from a specific path")
+	flags.BoolVar(&pa.Force, "force", false, "force installation of a plugin, even if already installed")


Suggested change

flags.BoolVar(&pa.Force, "force", false, "force installation of a plugin, even if already installed")

flags.BoolVar(&pa.Force, "force", false, "force installation of the specified plugin, even if already installed.")

nywilken · 2023-12-01T19:36:12Z

command/plugins_install.go

+	}
+
+	if pa.Path != "" && pa.Version != "" {
+		c.Ui.Error("Invalid arguments: a version cannot be specified with --path")


Suggested change

c.Ui.Error("Invalid arguments: a version cannot be specified with --path")

c.Ui.Error("Invalid arguments: a version cannot be specified when using --path to install a local plugin binary")

nywilken · 2023-12-01T19:36:26Z

command/plugins_install.go

+	}
+
+	pa.PluginName = args[0]
+


Suggested change

nywilken · 2023-12-01T19:36:46Z

command/plugins_install.go

@@ -68,30 +133,36 @@ func (c *PluginsInstallCommand) RunContext(buildCtx context.Context, args []stri
 		},
 	}



Suggested change

nywilken · 2023-12-01T19:45:31Z

command/plugins_install.go

+		return writeDiags(c.Ui, nil, hcl.Diagnostics{&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Unable to find plugin to promote",
+			Detail:   fmt.Sprintf("The plugin %q failed to be opened because of an error: %s", args.PluginName, err),


Suggested change

Detail: fmt.Sprintf("The plugin %q failed to be opened because of an error: %s", args.PluginName, err),

Detail: fmt.Sprintf("The plugin %q failed to be opened because of an error: %s", pluginIdentifier, err),

nywilken · 2023-12-01T20:01:40Z

command/plugins_install.go

+			Detail:   fmt.Sprintf("Failed to read plugin binary from %q: %s", args.PluginPath, err),
+		}})
+	}
+	_ = pluginBinary.Close()


Maybe we should handle this error in case close fails.

Eh, the chances of Close failing are excessively thin, and will be closed anyway when the process terminates, so we probably can overlook this one (we would too if we deferred)

nywilken · 2023-12-01T20:02:28Z

command/plugins_install.go

+		outputPrefix+opts.BinaryInstallationOptions.FilenameSuffix(),
+	)
+
+	outputPlugin, err := os.OpenFile(binaryPath, os.O_CREATE|os.O_TRUNC|os.O_RDWR, 0755)


Is this the same permissions we use inside of InstallLatest?

I believe so? If not we should probably settle for 755 for both, since read/execute makes sense for eveyone else, and all perms for the owner

command/plugins_install.go

command/cli.go

nywilken · 2023-12-01T20:20:35Z

command/plugins_install.go

+	pluginContents := &bytes.Buffer{}
+	_, err = io.Copy(pluginContents, pluginBinary)


nit: We only need the pointer for copying.

Suggested change

pluginContents := &bytes.Buffer{}

_, err = io.Copy(pluginContents, pluginBinary)

pluginContents := bytes.Buffer{}

_, err = io.Copy(&pluginContents, pluginBinary)

command/plugins_install.go

nywilken · 2023-12-01T20:27:23Z

command/plugins_install.go

+		return writeDiags(c.Ui, nil, hcl.Diagnostics{&hcl.Diagnostic{
+			Severity: hcl.DiagError,
+			Summary:  "Invalid version",
+			Detail:   fmt.Sprintf("Packer can only install plugin releases with this command (ex: 1.0.0), the binary's reported version is %q", desc.Version),


This doesn't solve the problem for folks using dev builds right.

Suggested change

Detail: fmt.Sprintf("Packer can only install plugin releases with this command (ex: 1.0.0), the binary's reported version is %q", desc.Version),

Detail: fmt.Sprintf("Installation of prerelease binaries with this command is not allowed, the binary's reported version is %q", desc.Version),

~> packer plugins install --path ~/.packer.d/plugins/packer-plugin-amazon github.com/hashicorp/happycloud Error: Invalid version Packer can only install plugin releases with this command (ex: 1.0.0), the binary's reported version is "1.2.9-dev"

This does not indeed for now, it's one of the things we need to address/fix for 1.11.0

nywilken

Something seems off

~>  tree ~/.packer.d/plugins/github.com/hashicorp/
/Users/dev/.packer.d/plugins/github.com/hashicorp/
├── amazon
│   ├── packer-plugin-amazon_v1.2.8_x5.0_darwin_arm64
│   └── packer-plugin-amazon_v1.2.8_x5.0_darwin_arm64_SHA256SUM
└── happycloud
    ├── packer-plugin-happycloud_v1.2.8_x5.0_darwin_arm64
    └── packer-plugin-happycloud_v1.2.8_x5.0_darwin_arm64_SHA256SUM

2 directories, 4 files
~>  packer plugins installed
/Users/dev/.packer.d/plugins/github.com/hashicorp/amazon/packer-plugin-amazon_v1.2.8_x5.0_darwin_arm64

Read original binary into memory to fix case when installation destination and source were the same, resulting in an empty binary.

nywilken

Nice updates ✅

github-actions · 2024-01-04T02:03:19Z

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.

lbajolet-hashicorp force-pushed the plugins_promote_proto branch 2 times, most recently from 054aa21 to 4cc8747 Compare October 16, 2023 19:19

lbajolet-hashicorp changed the title ~~command: add packer plugins promote command~~ command: add --path flag to packer plugins install Oct 16, 2023

lbajolet-hashicorp force-pushed the plugins_promote_proto branch from 4cc8747 to 666039c Compare October 16, 2023 20:51

nywilken added enhancement stage/needs-discussion stage/thinking Flagged for internal discussions about possible enhancements and removed stage/needs-discussion labels Oct 23, 2023

lbajolet-hashicorp force-pushed the plugins_promote_proto branch 3 times, most recently from d1d4d45 to acf3ef9 Compare October 26, 2023 14:24

lbajolet-hashicorp mentioned this pull request Oct 26, 2023

Plugin Discovery in Packer #12668

Closed

lbajolet-hashicorp mentioned this pull request Nov 7, 2023

Add instance_id to state so ID is usable in provisioners hashicorp/packer-plugin-lxd#84

Closed

lbajolet-hashicorp force-pushed the plugins_promote_proto branch 2 times, most recently from 0abc8c0 to 7f43b65 Compare November 24, 2023 16:27

nywilken reviewed Nov 30, 2023

View reviewed changes

lbajolet-hashicorp added 4 commits November 30, 2023 15:23

command: reject version with --path for install

9b029c1

To avoid plugins being installed with a specific version when a path is used for installing a plugin from a locally sourced plugin binary, we explicitly reject the combination of both a path and a version for plugins install.

lbajolet-hashicorp marked this pull request as ready for review December 1, 2023 18:19

lbajolet-hashicorp force-pushed the plugins_promote_proto branch from 7f43b65 to 688c689 Compare December 1, 2023 18:19

lbajolet-hashicorp requested a review from a team as a code owner December 1, 2023 18:19

lbajolet-hashicorp force-pushed the plugins_promote_proto branch from 688c689 to 91f2461 Compare December 1, 2023 18:27

nywilken reviewed Dec 1, 2023

View reviewed changes

command/plugins_install.go Show resolved Hide resolved

nywilken reviewed Dec 1, 2023

View reviewed changes

lbajolet-hashicorp force-pushed the plugins_promote_proto branch from 91f2461 to 6f36012 Compare December 1, 2023 21:50

command: simplify local binary installation

47e35bd

Read original binary into memory to fix case when installation destination and source were the same, resulting in an empty binary.

lbajolet-hashicorp force-pushed the plugins_promote_proto branch from 6f36012 to 62d7afd Compare December 4, 2023 14:50

vercel bot deployed to Preview December 4, 2023 14:54 View deployment

lbajolet-hashicorp added backport/1.10.x Backport PR changes to `release/1.10.x` and removed stage/thinking Flagged for internal discussions about possible enhancements labels Dec 4, 2023

docs: add exerpt on packer plugins install --path

af2a439

lbajolet-hashicorp force-pushed the plugins_promote_proto branch from 62d7afd to af2a439 Compare December 4, 2023 19:17

vercel bot deployed to Preview December 4, 2023 19:21 View deployment

nywilken approved these changes Dec 4, 2023

View reviewed changes

lbajolet-hashicorp merged commit 97e2e9e into main Dec 4, 2023
13 checks passed

hc-github-team-packer mentioned this pull request Dec 4, 2023

Backport of command: add --path flag to packer plugins install into release/1.10.x #12734

Merged

This was referenced Dec 5, 2023

command: add plugin-override opt to build/validate #12650

Closed

Add path attribute to required plugins #12641

Closed

github-actions bot locked as resolved and limited conversation to collaborators Jan 4, 2024

lbajolet-hashicorp deleted the plugins_promote_proto branch July 17, 2024 18:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

command: add --path flag to packer plugins install #12643

command: add --path flag to packer plugins install #12643

lbajolet-hashicorp commented Oct 3, 2023 •

edited

Loading

vercel bot commented Oct 3, 2023

nywilken left a comment

nywilken Nov 30, 2023

nywilken Nov 30, 2023

nywilken left a comment

nywilken Dec 1, 2023

nywilken Dec 1, 2023

nywilken Dec 1, 2023

nywilken Dec 1, 2023

nywilken Dec 1, 2023

nywilken Dec 1, 2023

nywilken Dec 1, 2023

lbajolet-hashicorp Dec 1, 2023

nywilken Dec 1, 2023

lbajolet-hashicorp Dec 1, 2023

nywilken Dec 1, 2023

nywilken Dec 1, 2023

lbajolet-hashicorp Dec 1, 2023

nywilken left a comment

nywilken left a comment

github-actions bot commented Jan 4, 2024

	flags.StringVar(&pa.PluginPath, "path", "", "install the plugin from a specific path")
	flags.StringVar(&pa.PluginPath, "path", "", "install the binary specified by path as a Packer plugin.")

	flags.BoolVar(&pa.Force, "force", false, "force installation of a plugin, even if already installed")
	flags.BoolVar(&pa.Force, "force", false, "force installation of the specified plugin, even if already installed.")

	c.Ui.Error("Invalid arguments: a version cannot be specified with --path")
	c.Ui.Error("Invalid arguments: a version cannot be specified when using --path to install a local plugin binary")

		@@ -68,30 +133,36 @@ func (c *PluginsInstallCommand) RunContext(buildCtx context.Context, args []stri
		},
		}

	Detail: fmt.Sprintf("The plugin %q failed to be opened because of an error: %s", args.PluginName, err),
	Detail: fmt.Sprintf("The plugin %q failed to be opened because of an error: %s", pluginIdentifier, err),

		pluginContents := &bytes.Buffer{}
		_, err = io.Copy(pluginContents, pluginBinary)

	Detail: fmt.Sprintf("Packer can only install plugin releases with this command (ex: 1.0.0), the binary's reported version is %q", desc.Version),
	Detail: fmt.Sprintf("Installation of prerelease binaries with this command is not allowed, the binary's reported version is %q", desc.Version),

command: add --path flag to packer plugins install #12643

command: add --path flag to packer plugins install #12643

Conversation

lbajolet-hashicorp commented Oct 3, 2023 • edited Loading

vercel bot commented Oct 3, 2023

nywilken left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nywilken left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nywilken left a comment

Choose a reason for hiding this comment

nywilken left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 4, 2024

lbajolet-hashicorp commented Oct 3, 2023 •

edited

Loading