From e6e99e6994d67d79937aec3822f410408002ab4e Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Thu, 29 Feb 2024 15:01:18 +0000 Subject: [PATCH] Stub for namespace cache warming fix (#25688) --- vault/auth.go | 6 ++++++ vault/mount.go | 18 ++++++++++++++++++ vault/namespaces.go | 6 ++++++ 3 files changed, 30 insertions(+) diff --git a/vault/auth.go b/vault/auth.go index ab74c56301..f6d4e984d3 100644 --- a/vault/auth.go +++ b/vault/auth.go @@ -659,6 +659,12 @@ func (c *Core) loadCredentials(ctx context.Context) error { } entry.namespace = ns + // Subtle: in `loadMounts` there is a call to namespaceManager.Register in + // the equivalent place to here. It's non-obvious why that is needed (see + // the large comment there) but it's equally subtle why it better NOT to do + // the same here. For more rationale see + // https://github.com/hashicorp/vault-enterprise/pull/5437#issuecomment-1954434213. + // Sync values to the cache entry.SyncCache() } diff --git a/vault/mount.go b/vault/mount.go index 42bfbde68d..c3ba77d670 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -1309,6 +1309,24 @@ func (c *Core) loadMounts(ctx context.Context) error { } entry.namespace = ns + // Subtle/hacky: re-register ns with namespaceManager to ensure that in the + // case where duplicate namespaces existed due to historical race bugs, we + // still ensure the correct namespaces are the ones that "win" the path + // registrations. This became an issue when we added Core.warmNamespaceCache + // and subsequently discovered customers had latent corruption where there + // were multiple namespace IDs with the same path in the namespaces config + // entry and loading them into cache made it non-deterministic which one won + // the slot in the path index due to map iteration randomness. By explicitly + // re-registering them here we enforce the same consistent order of + // registration into the path index that we had before warming and so + // prevent any apparent regressions. Note that mount entries are a slice not + // a map so consistent even if we ever did allow duplicates to be stored. We + // want to fix all the root causes of those duplicates but also want to + // restore the previous behavior that masked them as a problem. They will + // likely be there for a long time to come as they are persistent now even + // if we find all the bugs that allowed it to get in that state. + NamespaceRegister(ctx, ns, c) + // Sync values to the cache entry.SyncCache() } diff --git a/vault/namespaces.go b/vault/namespaces.go index af4f5c5dc3..c8cc8c0631 100644 --- a/vault/namespaces.go +++ b/vault/namespaces.go @@ -17,3 +17,9 @@ func namespaceByID(ctx context.Context, nsID string, c *Core) (*namespace.Namesp } return nil, namespace.ErrNoNamespace } + +var NamespaceRegister func(context.Context, *namespace.Namespace, *Core) error = namespaceRegister + +func namespaceRegister(ctx context.Context, ns *namespace.Namespace, c *Core) error { + return nil +}