From 6d9a8c8716e2e9d99ca033097390b1f0a67b988d Mon Sep 17 00:00:00 2001 From: Ellie Sterner Date: Mon, 12 Aug 2024 19:47:53 -0500 Subject: [PATCH 1/6] check if storage should be updated during invalidation --- vault/mount.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/vault/mount.go b/vault/mount.go index e8bea5ba4c65..9561a680e12a 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -1189,15 +1189,17 @@ func (c *Core) remountSecretsEngine(ctx context.Context, src, dst namespace.Moun srcMatch.Path = dst.MountPath // Update the mount table - if err := c.persistMounts(ctx, c.mounts, &srcMatch.Local); err != nil { - srcMatch.Path = srcPath - srcMatch.Tainted = true - c.mountsLock.Unlock() - if err == logical.ErrReadOnly && c.perfStandby { - return err - } + if updateStorage { + if err := c.persistMounts(ctx, c.mounts, &srcMatch.Local); err != nil { + srcMatch.Path = srcPath + srcMatch.Tainted = true + c.mountsLock.Unlock() + if err == logical.ErrReadOnly && c.perfStandby { + return err + } - return fmt.Errorf("failed to update mount table with error %+v", err) + return fmt.Errorf("failed to update mount table with error %+v", err) + } } // Remount the backend From 19b3b2dde9f34aee8f4f61e5afd07073f4f6a04d Mon Sep 17 00:00:00 2001 From: Ellie Sterner Date: Mon, 12 Aug 2024 19:50:49 -0500 Subject: [PATCH 2/6] add changelog --- changelog/28059.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/28059.txt diff --git a/changelog/28059.txt b/changelog/28059.txt new file mode 100644 index 000000000000..75952303861c --- /dev/null +++ b/changelog/28059.txt @@ -0,0 +1,3 @@ +```release-note:bug +command: The `vault secrets move` command will no longer attempt to write to storage on performance standby nodes. +``` From 159eb7c332c4e23dd20a72a448d50dcc2a3f5b40 Mon Sep 17 00:00:00 2001 From: Ellie Sterner Date: Thu, 15 Aug 2024 16:12:03 -0500 Subject: [PATCH 3/6] add other tests and fix for auth move --- vault/auth.go | 18 ++++--- vault/mount_test.go | 115 ++++++++++++++++++++++++++++++++++++++++++++ vault/testing.go | 17 +++++++ 3 files changed, 142 insertions(+), 8 deletions(-) diff --git a/vault/auth.go b/vault/auth.go index 379d9fb1e667..a176c5c88c19 100644 --- a/vault/auth.go +++ b/vault/auth.go @@ -485,15 +485,17 @@ func (c *Core) remountCredential(ctx context.Context, src, dst namespace.MountPa srcMatch.Path = strings.TrimPrefix(dst.MountPath, credentialRoutePrefix) // Update the mount table - if err := c.persistAuth(ctx, c.auth, &srcMatch.Local); err != nil { - srcMatch.Path = srcPath - srcMatch.Tainted = true - c.authLock.Unlock() - if err == logical.ErrReadOnly && c.perfStandby { - return err - } + if updateStorage { + if err := c.persistAuth(ctx, c.auth, &srcMatch.Local); err != nil { + srcMatch.Path = srcPath + srcMatch.Tainted = true + c.authLock.Unlock() + if err == logical.ErrReadOnly && c.perfStandby { + return err + } - return fmt.Errorf("failed to update auth table with error %+v", err) + return fmt.Errorf("failed to update auth table with error %+v", err) + } } // Remount the backend, setting the existing route entry diff --git a/vault/mount_test.go b/vault/mount_test.go index 77863f77365c..34871fd9de41 100644 --- a/vault/mount_test.go +++ b/vault/mount_test.go @@ -13,7 +13,9 @@ import ( "github.com/armon/go-metrics" "github.com/go-test/deep" + logicalKv "github.com/hashicorp/vault-plugin-secrets-kv" "github.com/hashicorp/vault/audit" + "github.com/hashicorp/vault/builtin/credential/userpass" "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/helper/testhelpers/corehelpers" @@ -21,6 +23,9 @@ import ( "github.com/hashicorp/vault/sdk/helper/compressutil" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/logical" + "github.com/hashicorp/vault/sdk/physical" + "github.com/hashicorp/vault/sdk/physical/inmem" + "github.com/stretchr/testify/require" ) func TestMount_ReadOnlyViewDuringMount(t *testing.T) { @@ -1022,3 +1027,113 @@ func TestCore_MountInitialize(t *testing.T) { } } } + +// This test ensures that a performance standby core does not +// try to persist mount changes to storage during remountSecretsEngine. +func TestCore_RemountSecretsEngine(t *testing.T) { + inm, err := inmem.NewTransactionalInmem(nil, logger) + if err != nil { + t.Fatal(err) + } + inmha, err := inmem.NewInmemHA(nil, logger) + if err != nil { + t.Fatal(err) + } + + coreConfig := &CoreConfig{ + Physical: inm, + HAPhysical: inmha.(physical.HABackend), + DisablePerformanceStandby: false, + // add kv engine + LogicalBackends: map[string]logical.Factory{ + "kv": logicalKv.Factory, + }, + } + + cluster := NewTestCluster(t, coreConfig, &TestClusterOptions{ + NumCores: 2, + }) + + cluster.Start() + + // Wait for cluster to be ready + activeCore := cluster.Cores[0].Core + perfStandbyCore := cluster.Cores[1].Core + TestWaitActive(t, activeCore) + TestWaitPerfStandby(t, perfStandbyCore) + + // Mount kv engine + nsCtx := namespace.RootContext(nil) + _, err = activeCore.systemBackend.HandleRequest(nsCtx, &logical.Request{ + Operation: logical.UpdateOperation, + Path: "mounts/kv1/", + Data: map[string]interface{}{ + "type": "kv", + }, + }) + require.NoError(t, err) + + src := namespace.MountPathDetails{Namespace: namespace.RootNamespace, MountPath: "kv1/"} + dst := namespace.MountPathDetails{Namespace: namespace.RootNamespace, MountPath: "kv2/"} + + // Give it a few seconds to replication userpass1 auth mount to perf standby + corehelpers.RetryUntil(t, 5*time.Second, func() error { + err = perfStandbyCore.remountSecretsEngine(nsCtx, src, dst, false) + return err + }) +} + +// This test ensures that a performance standby core does not +// try to persist auth changes to storage during remountCredential. +func TestCore_RemountAuthEngine(t *testing.T) { + inm, err := inmem.NewTransactionalInmem(nil, logger) + if err != nil { + t.Fatal(err) + } + inmha, err := inmem.NewInmemHA(nil, logger) + if err != nil { + t.Fatal(err) + } + + coreConfig := &CoreConfig{ + Physical: inm, + HAPhysical: inmha.(physical.HABackend), + DisablePerformanceStandby: false, + // add auth method + CredentialBackends: map[string]logical.Factory{ + "userpass": userpass.Factory, + }, + } + + cluster := NewTestCluster(t, coreConfig, &TestClusterOptions{ + NumCores: 2, + }) + + cluster.Start() + + // Wait for cluster to be ready + activeCore := cluster.Cores[0].Core + perfStandbyCore := cluster.Cores[1].Core + TestWaitActive(t, activeCore) + TestWaitPerfStandby(t, perfStandbyCore) + + // Mount auth engine + nsCtx := namespace.RootContext(nil) + _, err = activeCore.systemBackend.HandleRequest(nsCtx, &logical.Request{ + Operation: logical.UpdateOperation, + Path: "auth/userpass1/", + Data: map[string]interface{}{ + "type": "userpass", + }, + }) + require.NoError(t, err) + + src := namespace.MountPathDetails{Namespace: namespace.RootNamespace, MountPath: "auth/userpass1/"} + dst := namespace.MountPathDetails{Namespace: namespace.RootNamespace, MountPath: "auth/userpass2/"} + + // Give it a few seconds to replication userpass1 auth mount to perf standby + corehelpers.RetryUntil(t, 5*time.Second, func() error { + err := perfStandbyCore.remountCredential(nsCtx, src, dst, false) + return err + }) +} diff --git a/vault/testing.go b/vault/testing.go index e007ec008c3e..036087032383 100644 --- a/vault/testing.go +++ b/vault/testing.go @@ -652,6 +652,23 @@ func GenerateRandBytes(length int) ([]byte, error) { return buf, nil } +func TestWaitPerfStandby(t testing.TB, core *Core) { + t.Helper() + start := time.Now() + var perfStandby bool + for time.Now().Sub(start) < 30*time.Second { + perfStandby = core.PerfStandby() + + if perfStandby { + break + } + } + if !perfStandby { + err := errors.New("core not in perf standby mode") + t.Fatal(err) + } +} + func TestWaitActive(t testing.TB, core *Core) { t.Helper() if err := TestWaitActiveWithError(core); err != nil { From 991fe18a2a9915a910de1a2c2d31f38e0228f853 Mon Sep 17 00:00:00 2001 From: Ellie Sterner Date: Thu, 15 Aug 2024 16:15:18 -0500 Subject: [PATCH 4/6] fix changelog --- changelog/28059.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/28059.txt b/changelog/28059.txt index 75952303861c..550fd75af5c6 100644 --- a/changelog/28059.txt +++ b/changelog/28059.txt @@ -1,3 +1,3 @@ ```release-note:bug -command: The `vault secrets move` command will no longer attempt to write to storage on performance standby nodes. +command: The `vault secrets move` and `vault auth move` command will no longer attempt to write to storage on performance standby nodes. ``` From 20db27df870fcd4f5093f2313f560717284d6a24 Mon Sep 17 00:00:00 2001 From: Ellie Sterner Date: Thu, 15 Aug 2024 16:18:52 -0500 Subject: [PATCH 5/6] fix comment --- vault/mount_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vault/mount_test.go b/vault/mount_test.go index 34871fd9de41..d0a6706bb644 100644 --- a/vault/mount_test.go +++ b/vault/mount_test.go @@ -1076,7 +1076,7 @@ func TestCore_RemountSecretsEngine(t *testing.T) { src := namespace.MountPathDetails{Namespace: namespace.RootNamespace, MountPath: "kv1/"} dst := namespace.MountPathDetails{Namespace: namespace.RootNamespace, MountPath: "kv2/"} - // Give it a few seconds to replication userpass1 auth mount to perf standby + // Give it a few seconds to replicate kv1 secrets engine to perf standby corehelpers.RetryUntil(t, 5*time.Second, func() error { err = perfStandbyCore.remountSecretsEngine(nsCtx, src, dst, false) return err @@ -1131,7 +1131,7 @@ func TestCore_RemountAuthEngine(t *testing.T) { src := namespace.MountPathDetails{Namespace: namespace.RootNamespace, MountPath: "auth/userpass1/"} dst := namespace.MountPathDetails{Namespace: namespace.RootNamespace, MountPath: "auth/userpass2/"} - // Give it a few seconds to replication userpass1 auth mount to perf standby + // Give it a few seconds to replicate userpass1 auth mount to perf standby corehelpers.RetryUntil(t, 5*time.Second, func() error { err := perfStandbyCore.remountCredential(nsCtx, src, dst, false) return err From 79132a75b3eb054f2a01650372879cd896c3ad6d Mon Sep 17 00:00:00 2001 From: Ellie Sterner Date: Fri, 16 Aug 2024 08:21:39 -0500 Subject: [PATCH 6/6] remove ent tests --- vault/mount_test.go | 115 -------------------------------------------- 1 file changed, 115 deletions(-) diff --git a/vault/mount_test.go b/vault/mount_test.go index d0a6706bb644..77863f77365c 100644 --- a/vault/mount_test.go +++ b/vault/mount_test.go @@ -13,9 +13,7 @@ import ( "github.com/armon/go-metrics" "github.com/go-test/deep" - logicalKv "github.com/hashicorp/vault-plugin-secrets-kv" "github.com/hashicorp/vault/audit" - "github.com/hashicorp/vault/builtin/credential/userpass" "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/helper/testhelpers/corehelpers" @@ -23,9 +21,6 @@ import ( "github.com/hashicorp/vault/sdk/helper/compressutil" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/logical" - "github.com/hashicorp/vault/sdk/physical" - "github.com/hashicorp/vault/sdk/physical/inmem" - "github.com/stretchr/testify/require" ) func TestMount_ReadOnlyViewDuringMount(t *testing.T) { @@ -1027,113 +1022,3 @@ func TestCore_MountInitialize(t *testing.T) { } } } - -// This test ensures that a performance standby core does not -// try to persist mount changes to storage during remountSecretsEngine. -func TestCore_RemountSecretsEngine(t *testing.T) { - inm, err := inmem.NewTransactionalInmem(nil, logger) - if err != nil { - t.Fatal(err) - } - inmha, err := inmem.NewInmemHA(nil, logger) - if err != nil { - t.Fatal(err) - } - - coreConfig := &CoreConfig{ - Physical: inm, - HAPhysical: inmha.(physical.HABackend), - DisablePerformanceStandby: false, - // add kv engine - LogicalBackends: map[string]logical.Factory{ - "kv": logicalKv.Factory, - }, - } - - cluster := NewTestCluster(t, coreConfig, &TestClusterOptions{ - NumCores: 2, - }) - - cluster.Start() - - // Wait for cluster to be ready - activeCore := cluster.Cores[0].Core - perfStandbyCore := cluster.Cores[1].Core - TestWaitActive(t, activeCore) - TestWaitPerfStandby(t, perfStandbyCore) - - // Mount kv engine - nsCtx := namespace.RootContext(nil) - _, err = activeCore.systemBackend.HandleRequest(nsCtx, &logical.Request{ - Operation: logical.UpdateOperation, - Path: "mounts/kv1/", - Data: map[string]interface{}{ - "type": "kv", - }, - }) - require.NoError(t, err) - - src := namespace.MountPathDetails{Namespace: namespace.RootNamespace, MountPath: "kv1/"} - dst := namespace.MountPathDetails{Namespace: namespace.RootNamespace, MountPath: "kv2/"} - - // Give it a few seconds to replicate kv1 secrets engine to perf standby - corehelpers.RetryUntil(t, 5*time.Second, func() error { - err = perfStandbyCore.remountSecretsEngine(nsCtx, src, dst, false) - return err - }) -} - -// This test ensures that a performance standby core does not -// try to persist auth changes to storage during remountCredential. -func TestCore_RemountAuthEngine(t *testing.T) { - inm, err := inmem.NewTransactionalInmem(nil, logger) - if err != nil { - t.Fatal(err) - } - inmha, err := inmem.NewInmemHA(nil, logger) - if err != nil { - t.Fatal(err) - } - - coreConfig := &CoreConfig{ - Physical: inm, - HAPhysical: inmha.(physical.HABackend), - DisablePerformanceStandby: false, - // add auth method - CredentialBackends: map[string]logical.Factory{ - "userpass": userpass.Factory, - }, - } - - cluster := NewTestCluster(t, coreConfig, &TestClusterOptions{ - NumCores: 2, - }) - - cluster.Start() - - // Wait for cluster to be ready - activeCore := cluster.Cores[0].Core - perfStandbyCore := cluster.Cores[1].Core - TestWaitActive(t, activeCore) - TestWaitPerfStandby(t, perfStandbyCore) - - // Mount auth engine - nsCtx := namespace.RootContext(nil) - _, err = activeCore.systemBackend.HandleRequest(nsCtx, &logical.Request{ - Operation: logical.UpdateOperation, - Path: "auth/userpass1/", - Data: map[string]interface{}{ - "type": "userpass", - }, - }) - require.NoError(t, err) - - src := namespace.MountPathDetails{Namespace: namespace.RootNamespace, MountPath: "auth/userpass1/"} - dst := namespace.MountPathDetails{Namespace: namespace.RootNamespace, MountPath: "auth/userpass2/"} - - // Give it a few seconds to replicate userpass1 auth mount to perf standby - corehelpers.RetryUntil(t, 5*time.Second, func() error { - err := perfStandbyCore.remountCredential(nsCtx, src, dst, false) - return err - }) -}