Vault 36295 Improve plugin mgmt ux in api and cli #30811

helenfufu · 2025-06-02T18:32:18Z

Description

This PR improves the API and CLI UX for sha256 and command following the below guidelines:

API: When sha256 is provided, command is required (following the existing behavior). When sha256 is not provided, command is ignored and give a warning fmt.Sprintf("When sha256 is unspecified, a plugin artifact is expected for registration and the command parameter %q will be ignored.", command).
CLI: When sha256 is provided and both command and oci_image are not provided, default command to plugin name. When sha256 is not provided, make the API call and propagate up the API warning.

We additionally update the plugin registration docs (overview, API, and CLI) around sha256, command, and version to capture the two supported plugin registration mechanisms at this time: binary with -sha256 and (extracted) artifact with -version only.

Local testing: link

ENT PR: https://github.com/hashicorp/vault-enterprise/pull/8365

TODO only if you're a HashiCorp employee

Backport Labels: If this fix needs to be backported, use the appropriate backport/ label that matches the desired release branch. Note that in the CE repo, the latest release branch will look like backport/x.x.x, but older release branches will be backport/ent/x.x.x+ent.
- LTS: If this fixes a critical security vulnerability or severity 1 bug, it will also need to be backported to the current LTS versions of Vault. To ensure this, use all available enterprise labels.
ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
Jira: If this change has an associated Jira, it's referenced either
in the PR description, commit message, or branch name.
RFC: If this change has an associated RFC, please link it in the description.
ENT PR: If this change has an associated ENT PR, please link it in the
description. Also, make sure the changelog is in this PR, not in your ENT PR.

…rovided

…ning before returning in RegisterPluginWithContext

…cket

github-actions · 2025-06-02T18:45:28Z

CI Results:
All Go tests succeeded! ✅

helenfufu · 2025-06-04T15:26:13Z

website/content/docs/plugins/register.mdx

This PR includes the doc updates for the sha256, command, and version parameters, as well as the current state of the manual registration flow with an extracted artifact.

It does not currently include any doc updates for the download functionality. IMO it would be better to get this into main first so the current state is up-to-date, and then do a subsequent PR for download docs once our vault-enterprise feature branch is shipped.

benashz

This PR is looking great! I added some suggestions/nits for your consideration.

api/sys_plugins.go

benashz · 2025-06-04T15:34:37Z

api/sys_plugins_test.go

 		Version: "v1.0.0",
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
+	if len(resp.Warnings) > 0 {


nit: any chance we could check the Warnings here using reflect.DeepEqual()?

Having a bit of trouble with this, namely needing to differentiate when resp.Warnings is []string(nil) or []string{}, which the length check handles. Are we ok ignoring this nit in favor of prioritizing the download work?

In the case where we don't expect a warning, we should not assert the value. Maybe I am misunderstanding. Is the suggestion to test the non-happy path?

benashz · 2025-06-04T15:36:06Z

command/plugin_deregister_test.go

 			t.Fatal(err)
 		}
+		if len(registerResp.Warnings) > 0 {


nit: similar comment as above about testing the equality rather than the length of Warnings. Testing both is fine however.

Same as above #30811 (comment)

command/plugin_register.go

vault/logical_system.go

benashz · 2025-06-04T15:48:41Z

vault/logical_system.go

-		return logical.ErrorResponse("must provide at least one of command or oci_image"), nil
+	var resp logical.Response
+
+	if sha256 == "" {


Ideally these parameter constraints would be handled by the Vault server API. The vault client could just pass in everything that it got from the command line to Vault and the Vault server would do the parameter checks, returning any errors to the vault client. This is not a blocker, but something to consider for the future, especially since the Vault client may be connecting to some version of Vault that does support plugin artifacts, etc.

Ack for the future, thanks for the input.

There might be a misunderstanding on my side: I thought handlePluginCatalogUpdatein logical_system.go is the API handler? Where would you suggest making the change in the future?

…d-cli

…gisterPluginWithContext

api/sys_plugins.go

… binary plugins, which rely on plugin.Command input; extracted artifact plugins don't rely on plugin.Command input

website/content/api-docs/system/plugins-catalog.mdx

website/content/docs/commands/plugin/register.mdx

Co-authored-by: Sarah Chavis <[email protected]>

fairclothjm

Great work!

* suggestions * move common reqs to a partial * fix typo * tweak reqs * Update website/content/partials/plugins/prepare-plugin.mdx Co-authored-by: helenfufu <[email protected]> * Update website/content/partials/plugins/prepare-plugin.mdx Co-authored-by: helenfufu <[email protected]> * Update website/content/partials/plugins/prepare-plugin.mdx Co-authored-by: helenfufu <[email protected]> * tweak feedback * remove deprecation * Update website/content/partials/plugins/common-requirements.mdx Co-authored-by: helenfufu <[email protected]> * save * Update website/content/docs/plugins/rollback.mdx Co-authored-by: helenfufu <[email protected]> * Update website/content/docs/plugins/upgrade.mdx Co-authored-by: helenfufu <[email protected]> * fix formatting --------- Co-authored-by: helenfufu <[email protected]>

helenfufu added 5 commits June 2, 2025 10:33

cli: only set default command parameter to plugin name if sha256 is p…

16780f4

…rovided

api: write warnings to RegisterPluginResponse, propagate up to cli

0bf96d2

api: filter out 'Endpoint replaced the value of these parameters' war…

3cbd9d1

…ning before returning in RegisterPluginWithContext

docs

3ab4609

add TODO on filtering that links to api type parameter deprecation ti…

7a08810

…cket

helenfufu requested review from thyton and benashz June 2, 2025 18:32

github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jun 2, 2025

benashz changed the title ~~Vault 36295 improve plugin mgmt ux in api and cli~~ Improve plugin mgmt ux in api and cli Jun 4, 2025

benashz changed the title ~~Improve plugin mgmt ux in api and cli~~ Vault 36295 Improve plugin mgmt ux in api and cli Jun 4, 2025

fix tests

76b9dec

helenfufu force-pushed the vault-36295-improve-plugin-mgmt-ux-in-api-and-cli branch from ddb46b0 to 76b9dec Compare June 4, 2025 15:18

helenfufu commented Jun 4, 2025

View reviewed changes

helenfufu added this to the 1.20.0-rc milestone Jun 4, 2025

benashz reviewed Jun 4, 2025

View reviewed changes

Merge branch 'main' into vault-36295-improve-plugin-mgmt-ux-in-api-an…

a9ec8c2

…d-cli

vercel bot deployed to Preview June 4, 2025 16:02 View deployment

helenfufu added 6 commits June 4, 2025 12:21

allocate filteredWarning slice only if there are warnings

814a0d1

improve deferred resp close and early error return conditionals in Re…

04c9bb8

…gisterPluginWithContext

refer to sha256 as cli option -sha256 in command cli usage

512b038

break up ui error lines for sha256 and version flag check

318bc9a

consolidate if statements for sha256 and command, oci_image check in cli

20b2618

consolidate if statements for sha256 and command, oci_image check in api

d021fd1

helenfufu commented Jun 4, 2025

View reviewed changes

api/sys_plugins.go Show resolved Hide resolved

helenfufu marked this pull request as ready for review June 4, 2025 19:06

helenfufu requested review from a team as code owners June 4, 2025 19:06

helenfufu requested a review from brewgator June 4, 2025 19:06

vercel bot deployed to Preview June 24, 2025 17:31 View deployment

only validate plugin.Command in plugin catalog set for downloaded and…

2982aa5

… binary plugins, which rely on plugin.Command input; extracted artifact plugins don't rely on plugin.Command input

helenfufu modified the milestones: 1.21.0-rc1, 1.20.1 Jun 24, 2025

helenfufu removed backport/ent/1.16.x+ent Changes are backported to 1.16.x+ent backport/ent/1.18.x+ent backport/ent/1.19.x+ent labels Jun 24, 2025

thyton previously approved these changes Jun 25, 2025

View reviewed changes

schavis requested changes Jun 25, 2025

View reviewed changes

Update website/content/api-docs/system/plugins-catalog.mdx

ab20a4c

Co-authored-by: Sarah Chavis <[email protected]>

helenfufu dismissed thyton’s stale review via ab20a4c June 25, 2025 23:57

helenfufu and others added 7 commits June 25, 2025 16:58

Update website/content/api-docs/system/plugins-catalog.mdx

4b8446d

Co-authored-by: Sarah Chavis <[email protected]>

Update website/content/api-docs/system/plugins-catalog.mdx

29b5d6e

Co-authored-by: Sarah Chavis <[email protected]>

Update website/content/docs/commands/plugin/register.mdx

4d66328

Co-authored-by: Sarah Chavis <[email protected]>

Update website/content/docs/commands/plugin/register.mdx

e0526ed

Co-authored-by: Sarah Chavis <[email protected]>

Update website/content/docs/commands/plugin/register.mdx

5796723

Co-authored-by: Sarah Chavis <[email protected]>

Update website/content/docs/commands/plugin/register.mdx

d4b2de6

Co-authored-by: Sarah Chavis <[email protected]>

move up enterprise note on plugin register command doc

ca63320

helenfufu requested review from thyton, schavis, benashz and fairclothjm June 26, 2025 00:10

vercel bot deployed to Preview June 26, 2025 00:11 View deployment

schavis mentioned this pull request Jun 26, 2025

[DOCS] Editorial suggestions for PR #30811 #31111

Merged

fairclothjm previously approved these changes Jun 26, 2025

View reviewed changes

schavis dismissed fairclothjm’s stale review via f0d6d25 June 26, 2025 18:19

vercel bot deployed to Preview June 26, 2025 18:26 View deployment

schavis approved these changes Jun 26, 2025

View reviewed changes

thyton approved these changes Jun 27, 2025

View reviewed changes

Vault 36295 Improve plugin mgmt ux in api and cli #30811

Are you sure you want to change the base?

Vault 36295 Improve plugin mgmt ux in api and cli #30811

Uh oh!

Conversation

helenfufu commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

TODO only if you're a HashiCorp employee

Uh oh!

github-actions bot commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

helenfufu Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benashz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

benashz Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

helenfufu Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fairclothjm Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

benashz Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

helenfufu Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benashz Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

helenfufu Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fairclothjm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

helenfufu commented Jun 2, 2025 •

edited

Loading

github-actions bot commented Jun 2, 2025 •

edited

Loading

helenfufu Jun 4, 2025 •

edited

Loading

helenfufu Jun 4, 2025 •

edited

Loading