From fc04699f572d4227140b0df2237a416516f7e837 Mon Sep 17 00:00:00 2001 From: John-Michael Faircloth Date: Wed, 25 May 2022 13:37:42 -0500 Subject: [PATCH] Fix plugin reload mounts (#15579) * 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 --- changelog/15579.txt | 3 + vault/logical_system_integ_test.go | 56 +++++++++++++++---- vault/plugin_reload.go | 24 +++++--- .../content/docs/commands/plugin/reload.mdx | 20 ++++++- 4 files changed, 84 insertions(+), 19 deletions(-) create mode 100644 changelog/15579.txt diff --git a/changelog/15579.txt b/changelog/15579.txt new file mode 100644 index 000000000..f682dd498 --- /dev/null +++ b/changelog/15579.txt @@ -0,0 +1,3 @@ +```release-note:bug +plugin: Fix a bug where plugin reload would falsely report success in certain scenarios. +``` diff --git a/vault/logical_system_integ_test.go b/vault/logical_system_integ_test.go index a32351a3a..174231642 100644 --- a/vault/logical_system_integ_test.go +++ b/vault/logical_system_integ_test.go @@ -455,28 +455,62 @@ func TestSystemBackend_Plugin_SealUnseal(t *testing.T) { } func TestSystemBackend_Plugin_reload(t *testing.T) { - data := map[string]interface{}{ - "plugin": "mock-plugin", + testCases := []struct { + name string + backendType logical.BackendType + data map[string]interface{} + }{ + { + name: "test plugin reload for type credential", + backendType: logical.TypeCredential, + data: map[string]interface{}{ + "plugin": "mock-plugin", + }, + }, + { + name: "test mount reload for type credential", + backendType: logical.TypeCredential, + data: map[string]interface{}{ + "mounts": "sys/auth/mock-0/,auth/mock-1/", + }, + }, + { + name: "test plugin reload for type secret", + backendType: logical.TypeLogical, + data: map[string]interface{}{ + "plugin": "mock-plugin", + }, + }, + { + name: "test mount reload for type secret", + backendType: logical.TypeLogical, + data: map[string]interface{}{ + "mounts": "mock-0/,mock-1", + }, + }, } - t.Run("plugin", func(t *testing.T) { testSystemBackend_PluginReload(t, data) }) - - data = map[string]interface{}{ - "mounts": "mock-0/,mock-1/", + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + testSystemBackend_PluginReload(t, tc.data, tc.backendType) + }) } - t.Run("mounts", func(t *testing.T) { testSystemBackend_PluginReload(t, data) }) } // Helper func to test different reload methods on plugin reload endpoint -func testSystemBackend_PluginReload(t *testing.T, reqData map[string]interface{}) { - cluster := testSystemBackendMock(t, 1, 2, logical.TypeLogical) +func testSystemBackend_PluginReload(t *testing.T, reqData map[string]interface{}, backendType logical.BackendType) { + cluster := testSystemBackendMock(t, 1, 2, backendType) defer cluster.Cleanup() core := cluster.Cores[0] client := core.Client + pathPrefix := "mock-" + if backendType == logical.TypeCredential { + pathPrefix = "auth/" + pathPrefix + } for i := 0; i < 2; i++ { // Update internal value in the backend - resp, err := client.Logical().Write(fmt.Sprintf("mock-%d/internal", i), map[string]interface{}{ + resp, err := client.Logical().Write(fmt.Sprintf("%s%d/internal", pathPrefix, i), map[string]interface{}{ "value": "baz", }) if err != nil { @@ -501,7 +535,7 @@ func testSystemBackend_PluginReload(t *testing.T, reqData map[string]interface{} for i := 0; i < 2; i++ { // Ensure internal backed value is reset - resp, err := client.Logical().Read(fmt.Sprintf("mock-%d/internal", i)) + resp, err := client.Logical().Read(fmt.Sprintf("%s%d/internal", pathPrefix, i)) if err != nil { t.Fatalf("err: %v", err) } diff --git a/vault/plugin_reload.go b/vault/plugin_reload.go index e38b5341a..b0c513e25 100644 --- a/vault/plugin_reload.go +++ b/vault/plugin_reload.go @@ -12,7 +12,7 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) -// reloadPluginMounts reloads provided mounts, regardless of +// reloadMatchingPluginMounts reloads provided mounts, regardless of // plugin name, as long as the backend type is plugin. func (c *Core) reloadMatchingPluginMounts(ctx context.Context, mounts []string) error { c.mountsLock.RLock() @@ -27,18 +27,28 @@ func (c *Core) reloadMatchingPluginMounts(ctx context.Context, mounts []string) var errors error for _, mount := range mounts { + var isAuth bool + // allow any of + // - sys/auth/foo/ + // - sys/auth/foo + // - auth/foo/ + // - auth/foo + if strings.HasPrefix(mount, credentialRoutePrefix) { + isAuth = true + } else if strings.HasPrefix(mount, systemMountPath+credentialRoutePrefix) { + isAuth = true + mount = strings.TrimPrefix(mount, systemMountPath) + } + if !strings.HasSuffix(mount, "/") { + mount += "/" + } + entry := c.router.MatchingMountEntry(ctx, mount) if entry == nil { errors = multierror.Append(errors, fmt.Errorf("cannot fetch mount entry on %q", mount)) continue } - var isAuth bool - fullPath := c.router.MatchingMount(ctx, mount) - if strings.HasPrefix(fullPath, credentialRoutePrefix) { - isAuth = true - } - // We dont reload mounts that are not in the same namespace if ns.ID != entry.Namespace().ID { continue diff --git a/website/content/docs/commands/plugin/reload.mdx b/website/content/docs/commands/plugin/reload.mdx index 076fe1b5c..97d7df147 100644 --- a/website/content/docs/commands/plugin/reload.mdx +++ b/website/content/docs/commands/plugin/reload.mdx @@ -13,13 +13,31 @@ must be provided, but not both. ## Examples -Reload a plugin: +Reload a plugin by name: ```shell-session $ vault plugin reload -plugin my-custom-plugin Success! Reloaded plugin: my-custom-plugin ``` +Reload an auth plugin by mount: + +```shell-session +$ vault plugin reload \ + -mounts auth/my-custom-plugin-1 \ + -mounts auth/my-custom-plugin-2 +Success! Reloaded mounts: [auth/my-custom-plugin-1/ auth/my-custom-plugin-2/] +``` + +Reload a secrets plugin by mount: + +```shell-session +$ vault plugin reload \ + -mounts my-custom-plugin-1 \ + -mounts my-custom-plugin-2 +Success! Reloaded mounts: [my-custom-plugin-1/ my-custom-plugin-2/] +``` + ## Usage The following flags are available in addition to the [standard set of