Refactor plugin invoke

private/buf/bufctl/controller.go

@@ -22,7 +22,6 @@ import (
 	"io/fs"
 	"log/slog"
 	"net/http"
-	"os"
 	"sort"
 
 	"buf.build/go/protoyaml"
@@ -46,15 +45,13 @@ import (
 	"github.com/bufbuild/buf/private/pkg/httpauth"
 	"github.com/bufbuild/buf/private/pkg/ioext"
 	"github.com/bufbuild/buf/private/pkg/normalpath"
-	"github.com/bufbuild/buf/private/pkg/pluginrpcutil"
 	"github.com/bufbuild/buf/private/pkg/protoencoding"
 	"github.com/bufbuild/buf/private/pkg/slicesext"
 	"github.com/bufbuild/buf/private/pkg/storage/storageos"
 	"github.com/bufbuild/buf/private/pkg/syserror"
 	"github.com/bufbuild/buf/private/pkg/wasm"
 	"github.com/bufbuild/protovalidate-go"
 	"google.golang.org/protobuf/proto"
-	"pluginrpc.com/pluginrpc"
 )
 
 // ImageWithConfig pairs an Image with lint and breaking configuration.
@@ -133,6 +130,11 @@ type Controller interface {
 		defaultMessageEncoding buffetch.MessageEncoding,
 		options ...FunctionOption,
 	) error
+	// GetCheckRunnerProvider gets a CheckRunnerProvider for the given input.
+	//
+	// The returned RunnerProvider will be able to run lint and breaking checks
+	// using the PluginConfigs from the input. The input provided will resolve
+	// the PluginKeys from the related buf.lock file.
 	GetCheckRunnerProvider(
 		ctx context.Context,
 		input string,
@@ -723,7 +725,8 @@ func (c *controller) GetCheckRunnerProvider(
 	input string,
 	wasmRuntime wasm.Runtime,
 	options ...FunctionOption,
-) (bufcheck.RunnerProvider, error) {
+) (_ bufcheck.RunnerProvider, retErr error) {
+	defer c.handleFileAnnotationSetRetError(&retErr)
 	functionOptions := newFunctionOptions(c)
 	for _, option := range options {
 		option(functionOptions)
@@ -736,29 +739,11 @@ func (c *controller) GetCheckRunnerProvider(
 	if err != nil {
 		return nil, err
 	}
-	return bufcheck.RunnerProviderFunc(func(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) {
-		plugin, err := c.getPluginForPluginConfig(ctx, pluginKeyProvider, pluginConfig)
-		if err != nil {
-			return nil, err
-		}
-		switch {
-		case plugin.IsWasm():
-			getData := plugin.Data
-			return pluginrpcutil.NewWasmRunner(
-				wasmRuntime,
-				getData,
-				pluginConfig.Name(),
-				pluginConfig.Args()...,
-			), nil
-		case plugin.IsLocal():
-			return pluginrpcutil.NewLocalRunner(
-				pluginConfig.Name(),
-				pluginConfig.Args()...,
-			), nil
-		default:
-			return nil, syserror.Newf("unknown plugin type: %s", plugin.OpaqueID())
-		}
-	}), nil
+	return bufcheck.NewLocalRunnerProvider(
+		wasmRuntime,
+		pluginKeyProvider,
+		c.pluginDataProvider,
+	), nil
 }
 
 func (c *controller) getImage(
@@ -1216,7 +1201,7 @@ Declare %q in the deps key in your buf.yaml.`,
 
 // getPluginKeyProviderForRef create a new PluginKeyProvider for the Ref.
 //
-// Remote plugins refs are resolved to keys from the workspace buf.lock file.
+// Remote plugins Refs are resolved to PluginKeys from the workspace buf.lock file.
 // If the Ref is a MessageRef, we use the current directory buf.lock file.
 func (c *controller) getPluginKeyProviderForRef(
 	ctx context.Context,
@@ -1289,71 +1274,6 @@ func (c *controller) getPluginKeyProviderForRef(
 	}
 }
 
-// getPluginForPluginConfig resolves the plugin for the given PluginConfig.
-//
-// Remote plugins are resolved by fetching the plugin key for the given Ref, and then fetching
-// the plugin data for the plugin key.
-func (c *controller) getPluginForPluginConfig(
-	ctx context.Context,
-	pluginKeyProvider bufplugin.PluginKeyProvider,
-	pluginConfig bufconfig.PluginConfig,
-) (bufplugin.Plugin, error) {
-	switch pluginConfigType := pluginConfig.Type(); pluginConfigType {
-	case bufconfig.PluginConfigTypeLocal:
-		return bufplugin.NewLocalPlugin(
-			pluginConfig.Name(),
-			pluginConfig.Args(),
-		)
-	case bufconfig.PluginConfigTypeLocalWasm:
-		pluginName := pluginConfig.Name()
-		return bufplugin.NewLocalWasmPlugin(
-			nil, // We don't have a FullName for a local Wasm plugin.
-			pluginName,
-			pluginConfig.Args(),
-			func() ([]byte, error) {
-				moduleWasm, err := os.ReadFile(pluginName)
-				if err != nil {
-					return nil, fmt.Errorf("could not read plugin %q: %v", pluginName, err)
-				}
-				return moduleWasm, nil
-			},
-		)
-	case bufconfig.PluginConfigTypeRemoteWasm:
-		pluginRef := pluginConfig.Ref()
-		if pluginRef == nil {
-			return nil, syserror.Newf("Ref is required for remote plugins")
-		}
-		pluginKeys, err := pluginKeyProvider.GetPluginKeysForPluginRefs(ctx, []bufparse.Ref{pluginRef}, bufplugin.DigestTypeP1)
-		if err != nil {
-			if errors.Is(err, fs.ErrNotExist) {
-				return nil, fmt.Errorf("plugin %q not found in workspace. ", pluginRef.FullName().String())
-			}
-			return nil, err
-		}
-		if len(pluginKeys) != 1 {
-			return nil, syserror.Newf("expected 1 plugin key, got %d", len(pluginKeys))
-		}
-		pluginKey := pluginKeys[0]
-		return bufplugin.NewRemoteWasmPlugin(
-			pluginKey.FullName(),
-			pluginConfig.Args(),
-			pluginKey.CommitID(),
-			func() ([]byte, error) {
-				pluginDatas, err := c.pluginDataProvider.GetPluginDatasForPluginKeys(ctx, []bufplugin.PluginKey{pluginKey})
-				if err != nil {
-					return nil, err
-				}
-				if len(pluginDatas) != 1 {
-					return nil, syserror.Newf("expected 1 plugin data, got %d", len(pluginDatas))
-				}
-				return pluginDatas[0].Data()
-			},
-		)
-	default:
-		return nil, syserror.Newf("unknown plugin config type: %v", pluginConfigType)
-	}
-}
-
 // handleFileAnnotationSetError will attempt to handle the error as a FileAnnotationSet, and if so, print
 // the FileAnnotationSet to the writer with the given error format while returning ErrFileAnnotation.
 //

private/buf/bufmigrate/migrator.go

@@ -29,6 +29,7 @@ import (
 	"github.com/bufbuild/buf/private/bufpkg/bufconfig"
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule"
 	"github.com/bufbuild/buf/private/bufpkg/bufparse"
+	"github.com/bufbuild/buf/private/bufpkg/bufplugin"
 	"github.com/bufbuild/buf/private/pkg/normalpath"
 	"github.com/bufbuild/buf/private/pkg/slicesext"
 	"github.com/bufbuild/buf/private/pkg/storage"
@@ -695,7 +696,11 @@ func equivalentCheckConfigInV2(
 ) (bufconfig.CheckConfig, error) {
 	// No need for custom lint/breaking plugins since there's no plugins to migrate from <=v1.
 	// TODO: If we ever need v3, then we will have to deal with this.
-	client, err := bufcheck.NewClient(logger, bufcheck.NewLocalRunnerProvider(wasm.UnimplementedRuntime))
+	client, err := bufcheck.NewClient(logger, bufcheck.NewLocalRunnerProvider(
+		wasm.UnimplementedRuntime,
+		bufplugin.NopPluginKeyProvider,
+		bufplugin.NopPluginDataProvider,
+	))
 	if err != nil {
 		return nil, err
 	}

private/buf/bufworkspace/workspace_provider.go

@@ -418,6 +418,7 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2(
 	v2Targeting *v2Targeting,
 ) (*workspace, error) {
 	moduleSetBuilder := bufmodule.NewModuleSetBuilder(ctx, w.logger, w.moduleDataProvider, w.commitProvider)
+	var remotePluginKeys []bufplugin.PluginKey
 	bufLockFile, err := bufconfig.GetBufLockFileForPrefix(
 		ctx,
 		bucket,
@@ -446,6 +447,7 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2(
 				false,
 			)
 		}
+		remotePluginKeys = bufLockFile.RemotePluginKeys()
 	}
 	// Only check for duplicate module description in v2, which would be an user error, i.e.
 	// This is not a system error:
@@ -503,7 +505,7 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2(
 		moduleSet,
 		v2Targeting.bucketIDToModuleConfig,
 		v2Targeting.bufYAMLFile.PluginConfigs(),
-		bufLockFile.RemotePluginKeys(),
+		remotePluginKeys,
 		v2Targeting.bufYAMLFile.ConfiguredDepModuleRefs(),
 		true,
 	)

private/buf/cmd/buf/buf_test.go

@@ -35,6 +35,7 @@ import (
 	"github.com/bufbuild/buf/private/bufpkg/bufcheck"
 	"github.com/bufbuild/buf/private/bufpkg/bufconfig"
 	"github.com/bufbuild/buf/private/bufpkg/bufimage"
+	"github.com/bufbuild/buf/private/bufpkg/bufplugin"
 	imagev1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/image/v1"
 	"github.com/bufbuild/buf/private/pkg/app/appcmd"
 	"github.com/bufbuild/buf/private/pkg/app/appcmd/appcmdtesting"
@@ -1350,7 +1351,11 @@ func TestCheckLsBreakingRulesFromConfigExceptDeprecated(t *testing.T) {
 			// Do not need any custom lint/breaking plugins here.
 			client, err := bufcheck.NewClient(
 				slogtestext.NewLogger(t),
-				bufcheck.NewLocalRunnerProvider(wasm.UnimplementedRuntime),
+				bufcheck.NewLocalRunnerProvider(
+					wasm.UnimplementedRuntime,
+					bufplugin.NopPluginKeyProvider,
+					bufplugin.NopPluginDataProvider,
+				),
 			)
 			require.NoError(t, err)
 			allRules, err := client.AllRules(context.Background(), check.RuleTypeBreaking, version)

private/buf/cmd/buf/command/breaking/breaking.go

@@ -163,11 +163,14 @@ func run(
 	}
 	// Do not exclude imports here. bufcheck's Client requires all imports.
 	// Use bufcheck's BreakingWithExcludeImports.
+	inputControllerOptions := []bufctl.FunctionOption{
+		bufctl.WithTargetPaths(flags.Paths, flags.ExcludePaths),
+		bufctl.WithConfigOverride(flags.Config),
+	}
 	imageWithConfigs, err := controller.GetTargetImageWithConfigs(
 		ctx,
 		input,
-		bufctl.WithTargetPaths(flags.Paths, flags.ExcludePaths),
-		bufctl.WithConfigOverride(flags.Config),
+		inputControllerOptions...,
 	)
 	if err != nil {
 		return err
@@ -183,14 +186,11 @@ func run(
 	}
 	// Do not exclude imports here. bufcheck's Client requires all imports.
 	// Use bufcheck's BreakingWithExcludeImports.
-	controllerOptions := []bufctl.FunctionOption{
-		bufctl.WithTargetPaths(externalPaths, flags.ExcludePaths),
-		bufctl.WithConfigOverride(flags.AgainstConfig),
-	}
 	againstImageWithConfigs, err := controller.GetTargetImageWithConfigs(
 		ctx,
 		flags.Against,
-		controllerOptions...,
+		bufctl.WithTargetPaths(externalPaths, flags.ExcludePaths),
+		bufctl.WithConfigOverride(flags.AgainstConfig),
 	)
 	if err != nil {
 		return err
@@ -223,7 +223,7 @@ func run(
 		ctx,
 		input,
 		wasmRuntime,
-		controllerOptions...,
+		inputControllerOptions...,
 	)
 	if err != nil {
 		return err

private/buf/cmd/buf/command/mod/internal/internal.go

@@ -24,6 +24,7 @@ import (
 	"github.com/bufbuild/buf/private/buf/bufcli"
 	"github.com/bufbuild/buf/private/bufpkg/bufcheck"
 	"github.com/bufbuild/buf/private/bufpkg/bufconfig"
+	"github.com/bufbuild/buf/private/bufpkg/bufplugin"
 	"github.com/bufbuild/buf/private/pkg/app/appcmd"
 	"github.com/bufbuild/buf/private/pkg/app/appext"
 	"github.com/bufbuild/buf/private/pkg/slicesext"
@@ -175,7 +176,11 @@ func lsRun(
 	// BufYAMLFiles <=v1 never had plugins.
 	client, err := bufcheck.NewClient(
 		container.Logger(),
-		bufcheck.NewLocalRunnerProvider(wasm.UnimplementedRuntime),
+		bufcheck.NewLocalRunnerProvider(
+			wasm.UnimplementedRuntime,
+			bufplugin.NopPluginKeyProvider,
+			bufplugin.NopPluginDataProvider,
+		),
 		bufcheck.ClientWithStderr(container.Stderr()),
 	)
 	if err != nil {

private/buf/cmd/protoc-gen-buf-breaking/breaking.go

@@ -28,6 +28,7 @@ import (
 	"github.com/bufbuild/buf/private/bufpkg/bufanalysis"
 	"github.com/bufbuild/buf/private/bufpkg/bufcheck"
 	"github.com/bufbuild/buf/private/bufpkg/bufimage"
+	"github.com/bufbuild/buf/private/bufpkg/bufplugin"
 	"github.com/bufbuild/buf/private/pkg/encoding"
 	"github.com/bufbuild/buf/private/pkg/protodescriptor"
 	"github.com/bufbuild/buf/private/pkg/protoencoding"
@@ -125,7 +126,11 @@ func handle(
 	// The protoc plugins do not support custom lint/breaking change plugins for now.
 	client, err := bufcheck.NewClient(
 		container.Logger(),
-		bufcheck.NewLocalRunnerProvider(wasm.UnimplementedRuntime),
+		bufcheck.NewLocalRunnerProvider(
+			wasm.UnimplementedRuntime,
+			bufplugin.NopPluginKeyProvider,
+			bufplugin.NopPluginDataProvider,
+		),
 		bufcheck.ClientWithStderr(pluginEnv.Stderr),
 	)
 	if err != nil {

private/buf/cmd/protoc-gen-buf-lint/lint.go

@@ -27,6 +27,7 @@ import (
 	"github.com/bufbuild/buf/private/bufpkg/bufanalysis"
 	"github.com/bufbuild/buf/private/bufpkg/bufcheck"
 	"github.com/bufbuild/buf/private/bufpkg/bufimage"
+	"github.com/bufbuild/buf/private/bufpkg/bufplugin"
 	"github.com/bufbuild/buf/private/pkg/encoding"
 	"github.com/bufbuild/buf/private/pkg/protodescriptor"
 	"github.com/bufbuild/buf/private/pkg/protoencoding"
@@ -100,7 +101,11 @@ func handle(
 	// The protoc plugins do not support custom lint/breaking change plugins for now.
 	client, err := bufcheck.NewClient(
 		container.Logger(),
-		bufcheck.NewLocalRunnerProvider(wasm.UnimplementedRuntime),
+		bufcheck.NewLocalRunnerProvider(
+			wasm.UnimplementedRuntime,
+			bufplugin.NopPluginKeyProvider,
+			bufplugin.NopPluginDataProvider,
+		),
 		bufcheck.ClientWithStderr(pluginEnv.Stderr),
 	)
 	if err != nil {

private/bufpkg/bufcheck/breaking_test.go

@@ -1346,7 +1346,11 @@ func testBreaking(
 	require.NoError(t, err)
 	breakingConfig := workspace.GetBreakingConfigForOpaqueID(opaqueID)
 	require.NotNil(t, breakingConfig)
-	client, err := bufcheck.NewClient(logger, bufcheck.NewLocalRunnerProvider(wasm.UnimplementedRuntime))
+	client, err := bufcheck.NewClient(logger, bufcheck.NewLocalRunnerProvider(
+		wasm.UnimplementedRuntime,
+		bufplugin.NopPluginKeyProvider,
+		bufplugin.NopPluginDataProvider,
+	))
 	require.NoError(t, err)
 	err = client.Breaking(
 		ctx,

private/bufpkg/bufcheck/bufcheck.go

@@ -22,6 +22,7 @@ import (
 	"buf.build/go/bufplugin/check"
 	"github.com/bufbuild/buf/private/bufpkg/bufconfig"
 	"github.com/bufbuild/buf/private/bufpkg/bufimage"
+	"github.com/bufbuild/buf/private/bufpkg/bufplugin"
 	"github.com/bufbuild/buf/private/pkg/slicesext"
 	"github.com/bufbuild/buf/private/pkg/syserror"
 	"github.com/bufbuild/buf/private/pkg/wasm"
@@ -169,7 +170,8 @@ func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (plug
 	return r(pluginConfig)
 }
 
-// NewLocalRunnerProvider returns a new RunnerProvider for the wasm.Runtime.
+// NewLocalRunnerProvider returns a new RunnerProvider for the wasm.Runtime and
+// the given plugin providers.
 //
 // This implementation should only be used for local applications. It is safe to
 // use concurrently.
@@ -178,11 +180,22 @@ func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (plug
 // The supported types are:
 //   - bufconfig.PluginConfigTypeLocal
 //   - bufconfig.PluginConfigTypeLocalWasm
+//   - bufconfig.PluginConfigTypeRemoteWasm
 //
 // If the PluginConfigType is not supported, an error is returned.
-func NewLocalRunnerProvider(wasmRuntime wasm.Runtime) RunnerProvider {
+// To disable support for Wasm plugins, set wasmRuntime to wasm.UnimplementedRuntime.
+// To disable support for bufconfig.PluginConfigTypeRemoteWasm Plugins, set
+// pluginKeyProvider and pluginDataProvider to bufplugin.NopPluginKeyProvider
+// and bufplugin.NopPluginDataProvider.
+func NewLocalRunnerProvider(
+	wasmRuntime wasm.Runtime,
+	pluginKeyProvider bufplugin.PluginKeyProvider,
+	pluginDataProvider bufplugin.PluginDataProvider,
+) RunnerProvider {
 	return newRunnerProvider(
 		wasmRuntime,
+		pluginKeyProvider,
+		pluginDataProvider,
 	)
 }