plugins: Mount missing plugin entries and skip loading (#18189)

* Skip plugin startup for missing plugins
* Skip secrets startup for missing plugins
* Add changelog for bugfix
* Make plugin handling on unseal version-aware
* Update plugin lazy-load logic/comments for readability
* Add register/mount/deregister/seal/unseal go test
* Consolidate lazy mount logic to prevent inconsistencies

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>

changelog/18189.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+plugins: Skip loading but still mount data associated with missing plugins on unseal.
+```

vault/auth.go

@@ -800,11 +800,8 @@ func (c *Core) setupCredentials(ctx context.Context) error {
 		backend, entry.RunningSha256, err = c.newCredentialBackend(ctx, entry, sysView, view)
 		if err != nil {
 			c.logger.Error("failed to create credential entry", "path", entry.Path, "error", err)
-			plug, plugerr := c.pluginCatalog.Get(ctx, entry.Type, consts.PluginTypeCredential, "")
+
-			if plugerr == nil && plug != nil && !plug.Builtin {
+			if c.isMountable(ctx, entry, consts.PluginTypeCredential) {
-				// If we encounter an error instantiating the backend due to an error,
-				// skip backend initialization but register the entry to the mount table
-				// to preserve storage and path.
 				c.logger.Warn("skipping plugin-based credential entry", "path", entry.Path)
 				goto ROUTER_MOUNT
 			}

vault/core.go

@@ -3042,6 +3042,32 @@ func (c *Core) readFeatureFlags(ctx context.Context) (*FeatureFlags, error) {
 	return &flags, nil
 }
 
+// isMountable tells us whether or not we can continue mounting a plugin-based
+// mount entry after failing to instantiate a backend. We do this to preserve
+// the storage and path when a plugin is missing or has otherwise been
+// misconfigured. This allows users to recover from errors when starting Vault
+// with misconfigured plugins. It should not be possible for existing builtins
+// to be misconfigured, so that is a fatal error.
+func (c *Core) isMountable(ctx context.Context, entry *MountEntry, pluginType consts.PluginType) bool {
+	// Prevent a panic early on
+	if entry == nil || c.pluginCatalog == nil {
+		return false
+	}
+
+	// Handle aliases
+	t := entry.Type
+	if alias, ok := mountAliases[t]; ok {
+		t = alias
+	}
+
+	plug, err := c.pluginCatalog.Get(ctx, t, pluginType, entry.Version)
+	if err != nil {
+		return false
+	}
+
+	return plug == nil || !plug.Builtin
+}
+
 // MatchingMount returns the path of the mount that will be responsible for
 // handling the given request path.
 func (c *Core) MatchingMount(ctx context.Context, reqPath string) string {

vault/external_plugin_test.go

@@ -277,6 +277,71 @@ func TestCore_EnableExternalPlugin_MultipleVersions(t *testing.T) {
 	}
 }
 
+func TestCore_EnableExternalPlugin_Deregister_SealUnseal(t *testing.T) {
+	pluginDir, cleanup := MakeTestPluginDir(t)
+	t.Cleanup(func() { cleanup(t) })
+
+	// create an external plugin to shadow the builtin "pending-removal-test-plugin"
+	pluginName := "therug"
+	plugin := compilePlugin(t, consts.PluginTypeCredential, "", pluginDir)
+	err := os.Link(path.Join(pluginDir, plugin.fileName), path.Join(pluginDir, pluginName))
+	if err != nil {
+		t.Fatal(err)
+	}
+	conf := &CoreConfig{
+		BuiltinRegistry: NewMockBuiltinRegistry(),
+		PluginDirectory: pluginDir,
+	}
+
+	c := TestCoreWithSealAndUI(t, conf)
+	c, keys, root := testCoreUnsealed(t, c)
+
+	// Register a plugin
+	registerPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential.String(), "", plugin.sha256, plugin.fileName)
+	mountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "")
+	plugct := len(c.pluginCatalog.externalPlugins)
+	if plugct != 1 {
+		t.Fatalf("expected a single external plugin entry after registering, got: %d", plugct)
+	}
+
+	// Now pull the rug out from underneath us
+	deregisterPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential.String(), "", "", "")
+
+	if err := c.Seal(root); err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	for i, key := range keys {
+		unseal, err := TestCoreUnseal(c, key)
+		if err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		if i+1 == len(keys) && !unseal {
+			t.Fatalf("err: should be unsealed")
+		}
+	}
+
+	plugct = len(c.pluginCatalog.externalPlugins)
+	if plugct != 0 {
+		t.Fatalf("expected no plugin entries after unseal, got: %d", plugct)
+	}
+
+	found := false
+	mounts, err := c.ListAuths()
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, mount := range mounts {
+		if mount.Type == pluginName {
+			found = true
+			break
+		}
+	}
+
+	if !found {
+		t.Fatalf("expected to find %s mount, but got none", pluginName)
+	}
+}
+
 func TestCore_EnableExternalPlugin_ShadowBuiltin(t *testing.T) {
 	pluginDir, cleanup := MakeTestPluginDir(t)
 	t.Cleanup(func() { cleanup(t) })

vault/mount.go

@@ -1467,10 +1467,8 @@ func (c *Core) setupMounts(ctx context.Context) error {
 		backend, entry.RunningSha256, err = c.newLogicalBackend(ctx, entry, sysView, view)
 		if err != nil {
 			c.logger.Error("failed to create mount entry", "path", entry.Path, "error", err)
-			if !c.builtinRegistry.Contains(entry.Type, consts.PluginTypeSecrets) {
+
-				// If we encounter an error instantiating the backend due to an error,
+			if c.isMountable(ctx, entry, consts.PluginTypeSecrets) {
-				// skip backend initialization but register the entry to the mount table
-				// to preserve storage and path.
 				c.logger.Warn("skipping plugin-based mount entry", "path", entry.Path)
 				goto ROUTER_MOUNT
 			}