plugins: Filter builtins by RunningVersion (#17816)

This commit adds some logic to handle the case where a mount entry has a
non-builtin RunningVersion. This ensures that we only report deprecation
status for builtins.

changelog/17816.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+plugins: Only report deprecation status for builtin plugins.
+```

vault/external_plugin_test.go

@@ -277,6 +277,85 @@ func TestCore_EnableExternalPlugin_MultipleVersions(t *testing.T) {
 	}
 }
 
+func TestCore_EnableExternalPlugin_ShadowBuiltin(t *testing.T) {
+	pluginDir, cleanup := MakeTestPluginDir(t)
+	t.Cleanup(func() { cleanup(t) })
+
+	// create an external plugin to shadow the builtin "approle"
+	plugin := compilePlugin(t, consts.PluginTypeCredential, "v1.2.3", pluginDir)
+	err := os.Link(path.Join(pluginDir, plugin.fileName), path.Join(pluginDir, "approle"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	pluginName := "approle"
+	conf := &CoreConfig{
+		BuiltinRegistry: NewMockBuiltinRegistry(),
+		PluginDirectory: pluginDir,
+	}
+	c := TestCoreWithSealAndUI(t, conf)
+	c, _, _ = testCoreUnsealed(t, c)
+
+	verifyAuthListDeprecationStatus := func(authName string, checkExists bool) error {
+		req := logical.TestRequest(t, logical.ReadOperation, mountTable(consts.PluginTypeCredential))
+		req.Data = map[string]interface{}{
+			"type": authName,
+		}
+		resp, err := c.systemBackend.HandleRequest(namespace.RootContext(nil), req)
+		if err != nil {
+			return err
+		}
+		status := resp.Data["deprecation_status"]
+		if checkExists && status == nil {
+			return fmt.Errorf("expected deprecation status but found none")
+		} else if !checkExists && status != nil {
+			return fmt.Errorf("expected nil deprecation status but found %q", status)
+		}
+		return nil
+	}
+
+	// Create a new auth method with builtin approle
+	mountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "")
+
+	// Read the auth table to verify deprecation status
+	if err := verifyAuthListDeprecationStatus(pluginName, true); err != nil {
+		t.Fatal(err)
+	}
+
+	// Register a shadow plugin
+	registerPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential.String(), "v1.2.3", plugin.sha256, plugin.fileName)
+
+	// Verify auth table hasn't changed
+	if err := verifyAuthListDeprecationStatus(pluginName, true); err != nil {
+		t.Fatal(err)
+	}
+
+	// Remount auth method using registered shadow plugin
+	unmountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "")
+	mountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "")
+
+	// Verify auth table has changed
+	if err := verifyAuthListDeprecationStatus(pluginName, false); err != nil {
+		t.Fatal(err)
+	}
+
+	// Deregister shadow plugin
+	deregisterPlugin(t, c.systemBackend, pluginName, consts.PluginTypeSecrets.String(), "v1.2.3", plugin.sha256, plugin.fileName)
+
+	// Verify auth table hasn't changed
+	if err := verifyAuthListDeprecationStatus(pluginName, false); err != nil {
+		t.Fatal(err)
+	}
+
+	// Remount auth method
+	unmountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "")
+	mountPlugin(t, c.systemBackend, pluginName, consts.PluginTypeCredential, "", "")
+
+	// Verify auth table has changed
+	if err := verifyAuthListDeprecationStatus(pluginName, false); err != nil {
+		t.Fatal(err)
+	}
+}
+
 func TestCore_EnableExternalKv_MultipleVersions(t *testing.T) {
 	pluginDir, cleanup := MakeTestPluginDir(t)
 	t.Cleanup(func() { cleanup(t) })
@@ -759,6 +838,43 @@ func mountPlugin(t *testing.T, sys *SystemBackend, pluginName string, pluginType
 	}
 }
 
+func unmountPlugin(t *testing.T, sys *SystemBackend, pluginName string, pluginType consts.PluginType, version, path string) {
+	t.Helper()
+	var mountPath string
+	if path == "" {
+		mountPath = mountTable(pluginType)
+	} else {
+		mountPath = mountTableWithPath(consts.PluginTypeSecrets, path)
+	}
+	req := logical.TestRequest(t, logical.DeleteOperation, mountPath)
+	req.Data = map[string]interface{}{
+		"type": pluginName,
+	}
+	if version != "" {
+		req.Data["config"] = map[string]interface{}{
+			"plugin_version": version,
+		}
+	}
+	resp, err := sys.HandleRequest(namespace.RootContext(nil), req)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%v resp:%#v", err, resp)
+	}
+}
+
+func deregisterPlugin(t *testing.T, sys *SystemBackend, pluginName, pluginType, version, sha, command string) {
+	t.Helper()
+	req := logical.TestRequest(t, logical.DeleteOperation, fmt.Sprintf("plugins/catalog/%s/%s", pluginType, pluginName))
+	req.Data = map[string]interface{}{
+		"command": command,
+		"sha256":  sha,
+		"version": version,
+	}
+	resp, err := sys.HandleRequest(namespace.RootContext(nil), req)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%v resp:%#v", err, resp)
+	}
+}
+
 func mountTable(pluginType consts.PluginType) string {
 	return mountTableWithPath(pluginType, "foo")
 }

vault/mount.go

@@ -717,8 +717,13 @@ func (c *Core) builtinTypeFromMountEntry(ctx context.Context, entry *MountEntry)
 		return consts.PluginTypeUnknown
 	}
 
+	// Builtin plugins should contain the "builtin" string in their RunningVersion
+	if !strings.Contains(entry.RunningVersion, "builtin") {
+		return consts.PluginTypeUnknown
+	}
+
 	builtinPluginType := func(name string, pluginType consts.PluginType) (consts.PluginType, bool) {
-		plugin, err := c.pluginCatalog.Get(ctx, name, pluginType, "")
+		plugin, err := c.pluginCatalog.Get(ctx, name, pluginType, entry.RunningVersion)
 		if err == nil && plugin != nil && plugin.Builtin {
 			return plugin.Type, true
 		}