Fix plugin reload mounts #15579
Merged
Fix plugin reload mounts #15579
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
robmonte
reviewed
May 24, 2022
Comment on lines
53
to
54
| if ns.ID != entry.Namespace().ID { | ||
| continue |
There was a problem hiding this comment.
This looks like it would also be returning a fake success here when in fact the reload fails. Would changing this too also be an improvement?
There was a problem hiding this comment.
Good thinking! I wasn't sure so I tested it and we get the following error when using namespaces
Error reloading plugin/mounts: Error making API request.
Namespace: ns2/
URL: PUT http://127.0.0.1:8200/v1/sys/plugins/reload/backend
Code: 500. Errors:
* 3 errors occurred:
* cannot fetch mount entry on "auth/userpass-single-sdk-1_9-1/"
* cannot fetch mount entry on "auth/userpass-single-sdk-1_9-2/"
* cannot fetch mount entry on "auth/userpass-single-sdk-1_9-3/"
The call to c.router.MatchingMountEntry above will ensure the namespace is applied before searching for the mount. So I think this check may be redundant?
Gabrielopesantos
pushed a commit
to Gabrielopesantos/vault
that referenced
this pull request
Jun 6, 2022
* fix plugin reload mounts * do not require sys/ prefix * update plugin reload docs with examples * fix unit test credential read path * update docs to reflect correct cli usage * allow sys/auth/foo or auth/foo * append trailing slash if it doesn't exist in request * add changelog * use correct changelog number
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix a bug where the command
vault plugin reload -mounts <plugin-mount-path>silently fails for auth plugins. This bug results in the plugin process is not actually getting killed/cleaned up but reports that the reload was successful.This change allows the
-mountsflag to except any ofsys/auth/foo/sys/auth/fooauth/foo/auth/foofor an auth plugin mounted with
vault auth enable -path=foo my-foo-auth-plugin; orbar/barfor a secrets plugin mounted with
vault secrets enable -path=bar my-bar-secrets-pluginAdditionally, we add some docs examples for the unintuitive
vault plugin reload ...commands.