Add plugin backend reload capability #3112
Conversation
vault/plugin_reload.go
Outdated
| c.mountsLock.Lock() | ||
| defer c.mountsLock.Unlock() | ||
|
|
||
| for _, mount := range mounts { |
There was a problem hiding this comment.
This can partially reload some backends before failing. I wonder if this should be an all-or-nothing operation, where reload executed only after a check on all mounts successful.
There was a problem hiding this comment.
Maybe it should use multierror to return which mounts were unable to be reloaded. That way it gets through as many as it can
calvn
changed the title
| defer c.mountsLock.Unlock() | ||
|
|
||
| // Filter mount entries that only matches the plugin name | ||
| for _, entry := range c.mounts.Entries { |
There was a problem hiding this comment.
This function likely needs to iterate through c.auth.Entries too, in case the plugin name they give is an auth plugin.
| } | ||
| if resp != nil { | ||
| t.Fatalf("bad: %v", resp) | ||
| } |
There was a problem hiding this comment.
I realize this is making sure there's no error, but is there a way to verify that a reload actually happened other than no error?
There was a problem hiding this comment.
I am not sure how this can be done, since the backend has no access/knowledge of the underlying plugin process.
vault/plugin_reload.go
Outdated
| } | ||
|
|
||
| // Call initialize; this takes care of init tasks that must be run after | ||
| // the ignore paths are collected. |
There was a problem hiding this comment.
Yes, but keep in mind that on the ent side there needs to be logic here to handle the ignore paths in case they've changed (so likely, remove the ones that are there at the beginning, then add them now).
vault/plugin_reload.go
Outdated
| } | ||
|
|
||
| func (c *Core) reloadPluginCommon(entry *MountEntry) error { | ||
| if entry.Type == "plugin" { |
There was a problem hiding this comment.
This is a small nit, but it probably would be cleaner to verify the entry type is a plugin outside of this function and only call reloadPluginCommon if it is indeed a plugin.
vault/logical_system.go
Outdated
| @@ -821,6 +821,27 @@ func NewSystemBackend(core *Core) *SystemBackend { | |||
| HelpSynopsis: strings.TrimSpace(sysHelp["plugin-catalog"][0]), | |||
| HelpDescription: strings.TrimSpace(sysHelp["plugin-catalog"][1]), | |||
| }, | |||
| &framework.Path{ | |||
| Pattern: "plugin/backend/reload$", | |||
There was a problem hiding this comment.
Should this be plugins/ to match the existing endpoint? Also since it operates on multiple plugins
|
We should also add the ability to automatically reload a plugin process in the event of a plugin crash, the plugin backend could just watch for |
No description provided.