Refactor bufcheck client options for plugin configuration (#3762)

Author: emcfarlane

private/buf/bufctl/controller.go

@@ -487,15 +487,14 @@ func (c *controller) GetTargetImageWithConfigsAndCheckClient(
 				pluginConfigs,
 			),
 		}
-		pluginRunnerProvider := bufcheck.NewLocalRunnerProvider(
-			wasmRuntime,
-			pluginKeyProvider,
-			c.pluginDataProvider,
-		)
 		checkClient, err := bufcheck.NewClient(
 			c.logger,
-			pluginRunnerProvider,
 			bufcheck.ClientWithStderr(c.container.Stderr()),
+			bufcheck.ClientWithRunnerProvider(
+				bufcheck.NewLocalRunnerProvider(wasmRuntime),
+			),
+			bufcheck.ClientWithLocalWasmPluginsFromOS(),
+			bufcheck.ClientWithRemoteWasmPlugins(pluginKeyProvider, c.pluginDataProvider),
 		)
 		if err != nil {
 			return nil, nil, err
@@ -802,15 +801,17 @@ func (c *controller) GetCheckClientForWorkspace(
 	if err != nil {
 		return nil, err
 	}
-	pluginRunnerProvider := bufcheck.NewLocalRunnerProvider(
-		wasmRuntime,
-		pluginKeyProvider,
-		c.pluginDataProvider,
-	)
 	return bufcheck.NewClient(
 		c.logger,
-		pluginRunnerProvider,
 		bufcheck.ClientWithStderr(c.container.Stderr()),
+		bufcheck.ClientWithRunnerProvider(
+			bufcheck.NewLocalRunnerProvider(wasmRuntime),
+		),
+		bufcheck.ClientWithLocalWasmPluginsFromOS(),
+		bufcheck.ClientWithRemoteWasmPlugins(
+			pluginKeyProvider,
+			c.pluginDataProvider,
+		),
 	)
 }
 

private/buf/bufmigrate/migrator.go

@@ -30,12 +30,10 @@ 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"
 	"github.com/bufbuild/buf/private/pkg/storage/storagemem"
-	"github.com/bufbuild/buf/private/pkg/wasm"
 	"github.com/google/uuid"
 )
 
@@ -697,11 +695,7 @@ 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,
-		bufplugin.NopPluginKeyProvider,
-		bufplugin.NopPluginDataProvider,
-	))
+	client, err := bufcheck.NewClient(logger)
 	if err != nil {
 		return nil, err
 	}

private/buf/cmd/buf/buf_test.go

@@ -36,7 +36,6 @@ 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"
@@ -46,7 +45,6 @@ import (
 	"github.com/bufbuild/buf/private/pkg/slogtestext"
 	"github.com/bufbuild/buf/private/pkg/storage/storageos"
 	"github.com/bufbuild/buf/private/pkg/storage/storagetesting"
-	"github.com/bufbuild/buf/private/pkg/wasm"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -1345,14 +1343,7 @@ func TestCheckLsBreakingRulesFromConfigExceptDeprecated(t *testing.T) {
 		t.Run(version.String(), func(t *testing.T) {
 			t.Parallel()
 			// Do not need any custom lint/breaking plugins here.
-			client, err := bufcheck.NewClient(
-				slogtestext.NewLogger(t),
-				bufcheck.NewLocalRunnerProvider(
-					wasm.UnimplementedRuntime,
-					bufplugin.NopPluginKeyProvider,
-					bufplugin.NopPluginDataProvider,
-				),
-			)
+			client, err := bufcheck.NewClient(slogtestext.NewLogger(t))
 			require.NoError(t, err)
 			allRules, err := client.AllRules(context.Background(), check.RuleTypeBreaking, version)
 			require.NoError(t, err)

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

@@ -24,13 +24,11 @@ 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"
 	"github.com/bufbuild/buf/private/pkg/stringutil"
 	"github.com/bufbuild/buf/private/pkg/syserror"
-	"github.com/bufbuild/buf/private/pkg/wasm"
 	"github.com/spf13/pflag"
 )
 
@@ -176,11 +174,6 @@ func lsRun(
 	// BufYAMLFiles <=v1 never had plugins.
 	client, err := bufcheck.NewClient(
 		container.Logger(),
-		bufcheck.NewLocalRunnerProvider(
-			wasm.UnimplementedRuntime,
-			bufplugin.NopPluginKeyProvider,
-			bufplugin.NopPluginDataProvider,
-		),
 		bufcheck.ClientWithStderr(container.Stderr()),
 	)
 	if err != nil {

private/buf/cmd/buf/command/plugin/pluginpush/pluginpush.go

@@ -153,6 +153,7 @@ func upload(
 		var err error
 		plugin, err = bufplugin.NewLocalWasmPlugin(
 			pluginFullName,
+			flags.Binary,
 			nil, // args
 			func() ([]byte, error) {
 				wasmBinary, err := os.ReadFile(flags.Binary)

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

@@ -28,11 +28,9 @@ 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"
-	"github.com/bufbuild/buf/private/pkg/wasm"
 	"github.com/bufbuild/protoplugin"
 )
 
@@ -126,11 +124,6 @@ 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,
-			bufplugin.NopPluginKeyProvider,
-			bufplugin.NopPluginDataProvider,
-		),
 		bufcheck.ClientWithStderr(pluginEnv.Stderr),
 	)
 	if err != nil {

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

@@ -27,11 +27,9 @@ 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"
-	"github.com/bufbuild/buf/private/pkg/wasm"
 	"github.com/bufbuild/protoplugin"
 )
 
@@ -101,11 +99,6 @@ 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,
-			bufplugin.NopPluginKeyProvider,
-			bufplugin.NopPluginDataProvider,
-		),
 		bufcheck.ClientWithStderr(pluginEnv.Stderr),
 	)
 	if err != nil {

private/bufpkg/bufcheck/breaking_test.go

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

private/bufpkg/bufcheck/bufcheck.go

@@ -23,6 +23,7 @@ import (
 	"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/pluginrpcutil"
 	"github.com/bufbuild/buf/private/pkg/slicesext"
 	"github.com/bufbuild/buf/private/pkg/syserror"
 	"github.com/bufbuild/buf/private/pkg/wasm"
@@ -176,55 +177,41 @@ func WithPluginConfigs(pluginConfigs ...bufconfig.PluginConfig) ClientFunctionOp
 
 // RunnerProvider provides pluginrpc.Runners for a given plugin config.
 type RunnerProvider interface {
-	NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error)
+	NewRunner(plugin bufplugin.Plugin) (pluginrpc.Runner, error)
 }
 
 // RunnerProviderFunc is a function that implements RunnerProvider.
-type RunnerProviderFunc func(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error)
+type RunnerProviderFunc func(plugin bufplugin.Plugin) (pluginrpc.Runner, error)
 
 // NewRunner implements RunnerProvider.
 //
 // RunnerProvider selects the correct Runner based on the type of pluginConfig.
-func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) {
-	return r(pluginConfig)
+func (r RunnerProviderFunc) NewRunner(plugin bufplugin.Plugin) (pluginrpc.Runner, error) {
+	return r(plugin)
 }
 
-// NewLocalRunnerProvider returns a new RunnerProvider for the wasm.Runtime and
-// the given plugin providers.
+// NewLocalRunnerProvider returns a new RunnerProvider to invoke plugins locally.
 //
 // This implementation should only be used for local applications. It is safe to
 // use concurrently.
 //
-// The RunnerProvider selects the correct Runner based on the PluginConfigType.
-// The supported types are:
-//   - bufconfig.PluginConfigTypeLocal
-//   - bufconfig.PluginConfigTypeLocalWasm
-//   - bufconfig.PluginConfigTypeRemoteWasm
+// The RunnerProvider selects the correct Runner based on the Plugin:
+//   - Local plugins will be run with pluginrpcutil.NewLocalRunner.
+//   - Local Wasm plugins will be run with pluginrpcutil.NewWasmRunner.
+//   - Remote Wasm plugins will be run with pluginrpcutil.NewWasmRunner.
 //
-// If the PluginConfigType is not supported, an error is returned.
+// If the plugin type is not supported, an error is returned.
 // 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,
-	)
+func NewLocalRunnerProvider(wasmRuntime wasm.Runtime) RunnerProvider {
+	return newLocalRunnerProvider(wasmRuntime)
 }
 
 // NewClient returns a new Client.
 func NewClient(
 	logger *slog.Logger,
-	runnerProvider RunnerProvider,
 	options ...ClientOption,
 ) (Client, error) {
-	return newClient(logger, runnerProvider, options...)
+	return newClient(logger, options...)
 }
 
 // ClientOption is an option for a new Client.
@@ -239,6 +226,47 @@ func ClientWithStderr(stderr io.Writer) ClientOption {
 	}
 }
 
+// ClientWithRunnerProvider returns a new ClientOption that specifies a RunnerProvider.
+//
+// The runnerProvider is used to create pluginrpc.Runners for the plugins.
+// By default, only builtin plugins are used.
+func ClientWithRunnerProvider(runnerProvider RunnerProvider) ClientOption {
+	return func(clientOptions *clientOptions) {
+		clientOptions.runnerProvider = runnerProvider
+	}
+}
+
+// ClientWithLocalWasmPlugins returns a new ClientOption that specifies reading Wasm plugins.
+//
+// The readBucket is used to read the Wasm plugin data from the filesystem.
+func ClientWithLocalWasmPlugins(readFile func(string) ([]byte, error)) ClientOption {
+	return func(clientOptions *clientOptions) {
+		clientOptions.pluginReadFile = readFile
+	}
+}
+
+// ClientWithLocalWasmPluginsFromOS returns a new ClientOption that specifies reading Wasm plugins
+// from the OS.
+//
+// This is only used for local applications.
+func ClientWithLocalWasmPluginsFromOS() ClientOption {
+	return func(clientOptions *clientOptions) {
+		clientOptions.pluginReadFile = pluginrpcutil.ReadWasmFileFromOS
+	}
+}
+
+// ClientWithRemoteWasmPlugins returns a new ClientOption that specifies the remote plugin key
+// and data providers.
+func ClientWithRemoteWasmPlugins(
+	pluginKeyProvider bufplugin.PluginKeyProvider,
+	pluginDataProvider bufplugin.PluginDataProvider,
+) ClientOption {
+	return func(clientOptions *clientOptions) {
+		clientOptions.pluginKeyProvider = pluginKeyProvider
+		clientOptions.pluginDataProvider = pluginDataProvider
+	}
+}
+
 // PrintRules prints the rules to the Writer.
 func PrintRules(writer io.Writer, rules []Rule, options ...PrintRulesOption) (retErr error) {
 	return printRules(writer, rules, options...)