Ignore errors from rollback manager invocations

changelog/22235.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+core: Log rollback manager failures during unmount, remount to prevent replication failures on secondary clusters.
+```

vault/external_tests/router/router_ext_test.go

@@ -8,6 +8,8 @@ import (
 
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/helper/testhelpers/minimal"
+	"github.com/hashicorp/vault/sdk/logical"
+	"github.com/hashicorp/vault/vault"
 )
 
 func TestRouter_MountSubpath_Checks(t *testing.T) {
@@ -50,3 +52,34 @@ func testRouter_MountSubpath(t *testing.T, mountPoints []string) {
 	cluster.UnsealCores(t)
 	t.Logf("Done: %#v", mountPoints)
 }
+
+func TestRouter_UnmountRollbackIsntFatal(t *testing.T) {
+	cluster := minimal.NewTestSoloCluster(t, &vault.CoreConfig{
+		LogicalBackends: map[string]logical.Factory{
+			"noop": vault.NoopBackendRollbackErrFactory,
+		},
+	})
+	client := cluster.Cores[0].Client
+
+	if err := client.Sys().Mount("noop", &api.MountInput{
+		Type: "noop",
+	}); err != nil {
+		t.Fatalf("failed to mount PKI: %v", err)
+	}
+
+	if _, err := client.Logical().Write("sys/plugins/reload/backend", map[string]interface{}{
+		"mounts": "noop",
+	}); err != nil {
+		t.Fatalf("expected remount of noop with broken periodic func to succeed; got err=%v", err)
+	}
+
+	if _, err := client.Logical().Write("sys/remount", map[string]interface{}{
+		"from": "noop",
+		"to":   "noop-to",
+	}); err != nil {
+		t.Fatalf("expected remount of noop with broken periodic func to succeed; got err=%v", err)
+	}
+
+	cluster.EnsureCoresSealed(t)
+	cluster.UnsealCores(t)
+}

vault/mount.go

@@ -874,9 +874,13 @@ func (c *Core) unmountInternal(ctx context.Context, path string, updateStorage b
 
 	rCtx := namespace.ContextWithNamespace(c.activeContext, ns)
 	if backend != nil && c.rollback != nil {
-		// Invoke the rollback manager a final time
+		// Invoke the rollback manager a final time. This is not fatal as
+		// various periodic funcs (e.g., PKI) can legitimately error; the
+		// periodic rollback manager logs these errors rather than failing
+		// replication like returning this error would do.
 		if err := c.rollback.Rollback(rCtx, path); err != nil {
-			return err
+			c.logger.Error("ignoring rollback error during unmount", "error", err, "path", path)
+			err = nil
 		}
 	}
 	if backend != nil && c.expiration != nil && updateStorage {
@@ -1142,11 +1146,15 @@ func (c *Core) remountSecretsEngine(ctx context.Context, src, dst namespace.Moun
 	}
 
 	if !c.IsDRSecondary() {
-		// Invoke the rollback manager a final time
+		// Invoke the rollback manager a final time. This is not fatal as
+		// various periodic funcs (e.g., PKI) can legitimately error; the
+		// periodic rollback manager logs these errors rather than failing
+		// replication like returning this error would do.
 		rCtx := namespace.ContextWithNamespace(c.activeContext, ns)
 		if c.rollback != nil && c.router.MatchingBackend(ctx, srcRelativePath) != nil {
 			if err := c.rollback.Rollback(rCtx, srcRelativePath); err != nil {
-				return err
+				c.logger.Error("ignoring rollback error during remount", "error", err, "path", src.Namespace.Path+src.MountPath)
+				err = nil
 			}
 		}
 

vault/router_testing.go

@@ -29,17 +29,27 @@ type NoopBackend struct {
 	DefaultLeaseTTL time.Duration
 	MaxLeaseTTL     time.Duration
 	BackendType     logical.BackendType
+
+	RollbackErrs bool
 }
 
 func NoopBackendFactory(_ context.Context, _ *logical.BackendConfig) (logical.Backend, error) {
 	return &NoopBackend{}, nil
 }
 
+func NoopBackendRollbackErrFactory(_ context.Context, _ *logical.BackendConfig) (logical.Backend, error) {
+	return &NoopBackend{RollbackErrs: true}, nil
+}
+
 func (n *NoopBackend) HandleRequest(ctx context.Context, req *logical.Request) (*logical.Response, error) {
 	if req.TokenEntry() != nil {
 		panic("got a non-nil TokenEntry")
 	}
 
+	if n.RollbackErrs && req.Operation == "rollback" {
+		return nil, fmt.Errorf("no-op backend rollback has erred out")
+	}
+
 	var err error
 	resp := n.Response
 	if n.RequestHandler != nil {