diff --git a/private/buf/bufmigrate/migrator.go b/private/buf/bufmigrate/migrator.go index 0a17a2b33f..cf69bb2960 100644 --- a/private/buf/bufmigrate/migrator.go +++ b/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.NewRunnerProvider(wasm.UnimplementedRuntime)) + client, err := bufcheck.NewClient(logger, bufcheck.NewLocalRunnerProvider( + wasm.UnimplementedRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + )) if err != nil { return nil, err } diff --git a/private/buf/cmd/buf/buf_test.go b/private/buf/cmd/buf/buf_test.go index d5b0d13fcf..f5ec4472ce 100644 --- a/private/buf/cmd/buf/buf_test.go +++ b/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.NewRunnerProvider(wasm.UnimplementedRuntime), + bufcheck.NewLocalRunnerProvider( + wasm.UnimplementedRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), ) require.NoError(t, err) allRules, err := client.AllRules(context.Background(), check.RuleTypeBreaking, version) diff --git a/private/buf/cmd/buf/command/beta/lsp/lsp.go b/private/buf/cmd/buf/command/beta/lsp/lsp.go index 24f4bec061..bdf7a21229 100644 --- a/private/buf/cmd/buf/command/beta/lsp/lsp.go +++ b/private/buf/cmd/buf/command/beta/lsp/lsp.go @@ -27,6 +27,7 @@ import ( "github.com/bufbuild/buf/private/buf/bufcli" "github.com/bufbuild/buf/private/buf/buflsp" "github.com/bufbuild/buf/private/bufpkg/bufcheck" + "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/ioext" @@ -115,7 +116,11 @@ func run( }() checkClient, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(wasmRuntime), + bufcheck.NewLocalRunnerProvider( + wasmRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index 71b699c9a2..0a5b6990e5 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -25,6 +25,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/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/slicesext" @@ -220,7 +221,11 @@ func run( for i, imageWithConfig := range imageWithConfigs { client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(wasmRuntime), + bufcheck.NewLocalRunnerProvider( + wasmRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index f1dfeb5524..83566a4a95 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/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/normalpath" @@ -196,7 +197,11 @@ func lsRun( }() client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(wasmRuntime), + bufcheck.NewLocalRunnerProvider( + wasmRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index 496a0d6638..9a40b720c0 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -23,6 +23,7 @@ import ( "github.com/bufbuild/buf/private/buf/bufctl" "github.com/bufbuild/buf/private/bufpkg/bufanalysis" "github.com/bufbuild/buf/private/bufpkg/bufcheck" + "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/stringutil" @@ -145,7 +146,11 @@ func run( for _, imageWithConfig := range imageWithConfigs { client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(wasmRuntime), + bufcheck.NewLocalRunnerProvider( + wasmRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/mod/internal/internal.go b/private/buf/cmd/buf/command/mod/internal/internal.go index c802e34e50..5aa14892c4 100644 --- a/private/buf/cmd/buf/command/mod/internal/internal.go +++ b/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.NewRunnerProvider(wasm.UnimplementedRuntime), + bufcheck.NewLocalRunnerProvider( + wasm.UnimplementedRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { diff --git a/private/buf/cmd/buf/command/plugin/pluginpush/pluginpush.go b/private/buf/cmd/buf/command/plugin/pluginpush/pluginpush.go index 4f509d1c01..1df99b4e0f 100644 --- a/private/buf/cmd/buf/command/plugin/pluginpush/pluginpush.go +++ b/private/buf/cmd/buf/command/plugin/pluginpush/pluginpush.go @@ -151,6 +151,7 @@ func upload( var err error plugin, err = bufplugin.NewLocalWasmPlugin( pluginFullName, + nil, // args func() ([]byte, error) { wasmBinary, err := os.ReadFile(flags.Binary) if err != nil { diff --git a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go b/private/buf/cmd/protoc-gen-buf-breaking/breaking.go index 1de59b0149..82d8b856c2 100644 --- a/private/buf/cmd/protoc-gen-buf-breaking/breaking.go +++ b/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.NewRunnerProvider(wasm.UnimplementedRuntime), + bufcheck.NewLocalRunnerProvider( + wasm.UnimplementedRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), bufcheck.ClientWithStderr(pluginEnv.Stderr), ) if err != nil { diff --git a/private/buf/cmd/protoc-gen-buf-lint/lint.go b/private/buf/cmd/protoc-gen-buf-lint/lint.go index d7ed7b0a1a..f29fef7659 100644 --- a/private/buf/cmd/protoc-gen-buf-lint/lint.go +++ b/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.NewRunnerProvider(wasm.UnimplementedRuntime), + bufcheck.NewLocalRunnerProvider( + wasm.UnimplementedRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), bufcheck.ClientWithStderr(pluginEnv.Stderr), ) if err != nil { diff --git a/private/bufpkg/bufcheck/breaking_test.go b/private/bufpkg/bufcheck/breaking_test.go index 2bab7cbf4d..a5c2d1d6b5 100644 --- a/private/bufpkg/bufcheck/breaking_test.go +++ b/private/bufpkg/bufcheck/breaking_test.go @@ -30,6 +30,7 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" + "github.com/bufbuild/buf/private/bufpkg/bufplugin" "github.com/bufbuild/buf/private/pkg/slogtestext" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/wasm" @@ -1344,7 +1345,11 @@ func testBreaking( require.NoError(t, err) breakingConfig := workspace.GetBreakingConfigForOpaqueID(opaqueID) require.NotNil(t, breakingConfig) - client, err := bufcheck.NewClient(logger, bufcheck.NewRunnerProvider(wasm.UnimplementedRuntime)) + client, err := bufcheck.NewClient(logger, bufcheck.NewLocalRunnerProvider( + wasm.UnimplementedRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + )) require.NoError(t, err) err = client.Breaking( ctx, diff --git a/private/bufpkg/bufcheck/bufcheck.go b/private/bufpkg/bufcheck/bufcheck.go index 6036e7b401..4ced80116d 100644 --- a/private/bufpkg/bufcheck/bufcheck.go +++ b/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,7 @@ func (r RunnerProviderFunc) NewRunner(pluginConfig bufconfig.PluginConfig) (plug return r(pluginConfig) } -// NewRunnerProvider returns a new RunnerProvider for the wasm.Runtime. +// NewLocalRunnerProvider returns a new RunnerProvider for the wasm.Runtime. // // This implementation should only be used for local applications. It is safe to // use concurrently. @@ -178,13 +179,18 @@ 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 NewRunnerProvider( +func NewLocalRunnerProvider( wasmRuntime wasm.Runtime, + pluginKeyProvider bufplugin.PluginKeyProvider, + pluginDataProvider bufplugin.PluginDataProvider, ) RunnerProvider { return newRunnerProvider( wasmRuntime, + pluginKeyProvider, + pluginDataProvider, ) } diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index 626f3738b2..29c99f6b78 100644 --- a/private/bufpkg/bufcheck/lint_test.go +++ b/private/bufpkg/bufcheck/lint_test.go @@ -27,6 +27,7 @@ import ( "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/bufpkg/bufimage" "github.com/bufbuild/buf/private/bufpkg/bufmodule" + "github.com/bufbuild/buf/private/bufpkg/bufplugin" "github.com/bufbuild/buf/private/pkg/slogtestext" "github.com/bufbuild/buf/private/pkg/storage/storageos" "github.com/bufbuild/buf/private/pkg/wasm" @@ -1355,7 +1356,11 @@ func testLintWithOptions( }) client, err := bufcheck.NewClient( logger, - bufcheck.NewRunnerProvider(wasmRuntime), + bufcheck.NewLocalRunnerProvider( + wasmRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), ) require.NoError(t, err) err = client.Lint( diff --git a/private/bufpkg/bufcheck/multi_client_test.go b/private/bufpkg/bufcheck/multi_client_test.go index 8fde74ddae..895ffe85dd 100644 --- a/private/bufpkg/bufcheck/multi_client_test.go +++ b/private/bufpkg/bufcheck/multi_client_test.go @@ -24,6 +24,7 @@ import ( "buf.build/go/bufplugin/check/checkutil" "buf.build/go/bufplugin/option" "github.com/bufbuild/buf/private/bufpkg/bufconfig" + "github.com/bufbuild/buf/private/bufpkg/bufplugin" "github.com/bufbuild/buf/private/pkg/slicesext" "github.com/bufbuild/buf/private/pkg/slogtestext" "github.com/bufbuild/buf/private/pkg/stringutil" @@ -182,13 +183,17 @@ func TestMultiClientCannotHaveOverlappingRulesWithBuiltIn(t *testing.T) { client, err := newClient( slogtestext.NewLogger(t), - NewRunnerProvider(wasm.UnimplementedRuntime), + NewLocalRunnerProvider( + wasm.UnimplementedRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( "buf-plugin-duplicate-rule", nil, - []string{"buf-plugin-duplicate-rule"}, + nil, ) require.NoError(t, err) emptyOptions, err := option.NewOptions(nil) @@ -275,13 +280,17 @@ func TestMultiClientCannotHaveOverlappingCategoriesWithBuiltIn(t *testing.T) { client, err := newClient( slogtestext.NewLogger(t), - NewRunnerProvider(wasm.UnimplementedRuntime), + NewLocalRunnerProvider( + wasm.UnimplementedRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), ) require.NoError(t, err) duplicateBuiltInRulePluginConfig, err := bufconfig.NewLocalPluginConfig( "buf-plugin-duplicate-category", nil, - []string{"buf-plugin-duplicate-category"}, + nil, ) require.NoError(t, err) emptyOptions, err := option.NewOptions(nil) diff --git a/private/bufpkg/bufcheck/runner_provider.go b/private/bufpkg/bufcheck/runner_provider.go index 013c1a892f..e3541f355d 100644 --- a/private/bufpkg/bufcheck/runner_provider.go +++ b/private/bufpkg/bufcheck/runner_provider.go @@ -15,9 +15,12 @@ package bufcheck import ( - "fmt" + "context" + "sync" "github.com/bufbuild/buf/private/bufpkg/bufconfig" + "github.com/bufbuild/buf/private/bufpkg/bufparse" + "github.com/bufbuild/buf/private/bufpkg/bufplugin" "github.com/bufbuild/buf/private/pkg/pluginrpcutil" "github.com/bufbuild/buf/private/pkg/syserror" "github.com/bufbuild/buf/private/pkg/wasm" @@ -25,37 +28,133 @@ import ( ) type runnerProvider struct { - wasmRuntime wasm.Runtime + wasmRuntime wasm.Runtime + pluginKeyProvider bufplugin.PluginKeyProvider + pluginDataProvider bufplugin.PluginDataProvider } func newRunnerProvider( wasmRuntime wasm.Runtime, + pluginKeyProvider bufplugin.PluginKeyProvider, + pluginDataProvider bufplugin.PluginDataProvider, ) *runnerProvider { return &runnerProvider{ - wasmRuntime: wasmRuntime, + wasmRuntime: wasmRuntime, + pluginKeyProvider: pluginKeyProvider, + pluginDataProvider: pluginDataProvider, } } func (r *runnerProvider) NewRunner(pluginConfig bufconfig.PluginConfig) (pluginrpc.Runner, error) { switch pluginConfig.Type() { case bufconfig.PluginConfigTypeLocal: - path := pluginConfig.Path() return pluginrpcutil.NewRunner( - // We know that Path is of at least length 1. - path[0], - path[1:]..., + pluginConfig.Name(), + pluginConfig.Args()..., ), nil case bufconfig.PluginConfigTypeLocalWasm: - path := pluginConfig.Path() - return pluginrpcutil.NewWasmRunner( + return pluginrpcutil.NewLocalWasmRunner( r.wasmRuntime, - // We know that Path is of at least length 1. - path[0], - path[1:]..., + pluginConfig.Name(), + pluginConfig.Args()..., ), nil - case bufconfig.PluginConfigTypeRemote: - return nil, fmt.Errorf("remote plugins are not supported") + case bufconfig.PluginConfigTypeRemoteWasm: + return newRemoteWasmPluginRunner( + r.wasmRuntime, + r.pluginKeyProvider, + r.pluginDataProvider, + pluginConfig.Ref(), + pluginConfig.Args(), + ) default: return nil, syserror.Newf("unknown PluginConfigType: %v", pluginConfig.Type()) } } + +// *** PRIVATE *** + +// remoteWasmPluginRunner is a Runner that runs a remote Wasm plugin. +// +// This is a wrapper around a pluginrpc.Runner that first resolves the Ref to +// a PluginKey using the PluginKeyProvider. It then loads the PluginData for +// the PluginKey using the PluginDataProvider. The PluginData is then used to +// create the pluginrpc.Runner. The Runner is only loaded once and is cached +// for future calls. However, if the Runner fails to load it will try to +// reload on the next call. +type remoteWasmPluginRunner struct { + wasmRuntime wasm.Runtime + pluginKeyProvider bufplugin.PluginKeyProvider + pluginDataProvider bufplugin.PluginDataProvider + pluginRef bufparse.Ref + pluginArgs []string + // lock protects runner. + lock sync.RWMutex + runner pluginrpc.Runner +} + +func newRemoteWasmPluginRunner( + wasmRuntime wasm.Runtime, + pluginKeyProvider bufplugin.PluginKeyProvider, + pluginDataProvider bufplugin.PluginDataProvider, + pluginRef bufparse.Ref, + pluginArgs []string, +) (*remoteWasmPluginRunner, error) { + return &remoteWasmPluginRunner{ + wasmRuntime: wasmRuntime, + pluginKeyProvider: pluginKeyProvider, + pluginDataProvider: pluginDataProvider, + pluginRef: pluginRef, + pluginArgs: pluginArgs, + }, nil +} + +func (r *remoteWasmPluginRunner) Run(ctx context.Context, env pluginrpc.Env) (retErr error) { + delegate, err := r.loadRunnerOnce(ctx) + if err != nil { + return err + } + return delegate.Run(ctx, env) +} + +func (r *remoteWasmPluginRunner) loadRunnerOnce(ctx context.Context) (pluginrpc.Runner, error) { + r.lock.RLock() + if r.runner != nil { + r.lock.RUnlock() + return r.runner, nil + } + r.lock.RUnlock() + r.lock.Lock() + defer r.lock.Unlock() + if r.runner == nil { + runner, err := r.loadRunner(ctx) + if err != nil { + // The error isn't stored to avoid ctx cancellation issues. On the next call, + // the runner will be reloaded instead of returning the erorr. + return nil, err + } + r.runner = runner + } + return r.runner, nil +} + +func (r *remoteWasmPluginRunner) loadRunner(ctx context.Context) (pluginrpc.Runner, error) { + pluginKeys, err := r.pluginKeyProvider.GetPluginKeysForPluginRefs(ctx, []bufparse.Ref{r.pluginRef}, bufplugin.DigestTypeP1) + if err != nil { + return nil, err + } + if len(pluginKeys) != 1 { + return nil, syserror.Newf("expected 1 PluginKey, got %d", len(pluginKeys)) + } + // Load the data for the plugin now to ensure the context is valid for the entire operation. + pluginDatas, err := r.pluginDataProvider.GetPluginDatasForPluginKeys(ctx, pluginKeys) + if err != nil { + return nil, err + } + if len(pluginDatas) != 1 { + return nil, syserror.Newf("expected 1 PluginData, got %d", len(pluginDatas)) + } + data := pluginDatas[0] + // The program name is the FullName of the plugin. + programName := r.pluginRef.FullName().String() + return pluginrpcutil.NewWasmRunner(r.wasmRuntime, data.Data, programName, r.pluginArgs...), nil +} diff --git a/private/bufpkg/bufconfig/plugin_config.go b/private/bufpkg/bufconfig/plugin_config.go index d35f9f55da..ea53cbaa47 100644 --- a/private/bufpkg/bufconfig/plugin_config.go +++ b/private/bufpkg/bufconfig/plugin_config.go @@ -19,7 +19,6 @@ import ( "fmt" "os" "path/filepath" - "strings" "github.com/bufbuild/buf/private/bufpkg/bufparse" "github.com/bufbuild/buf/private/pkg/encoding" @@ -31,8 +30,8 @@ const ( PluginConfigTypeLocal PluginConfigType = iota + 1 // PluginConfigTypeLocalWasm is the local Wasm plugin config type. PluginConfigTypeLocalWasm - // PluginConfigTypeRemote is the remote plugin config type. - PluginConfigTypeRemote + // PluginConfigTypeRemoteWasm is the remote Wasm plugin config type. + PluginConfigTypeRemoteWasm ) // PluginConfigType is a generate plugin configuration type. @@ -49,10 +48,10 @@ type PluginConfig interface { // TODO: Will want good validation and good error messages for what this decodes. // Otherwise we will confuse users. Do QA. Options() map[string]any - // Path returns the path, including arguments, to invoke the binary plugin. + // Args returns the arguments, excluding the plugin name, to invoke the plugin. // - // This is not empty only when the plugin is local. - Path() []string + // This may be empty. + Args() []string // Ref returns the plugin reference. // // This is only non-nil when the plugin is remote. @@ -65,12 +64,12 @@ type PluginConfig interface { func NewLocalPluginConfig( name string, options map[string]any, - path []string, + args []string, ) (PluginConfig, error) { return newLocalPluginConfig( name, options, - path, + args, ) } @@ -97,7 +96,7 @@ type pluginConfig struct { pluginConfigType PluginConfigType name string options map[string]any - path []string + args []string ref bufparse.Ref } @@ -123,6 +122,7 @@ func newPluginConfigForExternalV2( if len(path) == 0 { return nil, errors.New("must specify a path to the plugin") } + name, args := path[0], path[1:] // Remote plugins are specified as plugin references. if pluginRef, err := bufparse.ParseRef(path[0]); err == nil { // Check if the local filepath exists, if it does presume its @@ -132,67 +132,70 @@ func newPluginConfigForExternalV2( return newRemotePluginConfig( pluginRef, options, + args, ) } } // Wasm plugins are suffixed with .wasm. Otherwise, it's a binary. if filepath.Ext(path[0]) == ".wasm" { return newLocalWasmPluginConfig( - strings.Join(path, " "), + name, options, - path, + args, ) } return newLocalPluginConfig( - strings.Join(path, " "), + name, options, - path, + args, ) } func newLocalPluginConfig( name string, options map[string]any, - path []string, + args []string, ) (*pluginConfig, error) { - if len(path) == 0 { - return nil, errors.New("must specify a path to the plugin") + if name == "" { + return nil, errors.New("must specify a name to the plugin") } return &pluginConfig{ pluginConfigType: PluginConfigTypeLocal, name: name, options: options, - path: path, + args: args, }, nil } func newLocalWasmPluginConfig( name string, options map[string]any, - path []string, + args []string, ) (*pluginConfig, error) { - if len(path) == 0 { - return nil, errors.New("must specify a path to the plugin") + if name == "" { + return nil, errors.New("must specify a name to the plugin") } - if filepath.Ext(path[0]) != ".wasm" { - return nil, fmt.Errorf("must specify a path to the plugin, and the first path argument must end with .wasm") + if filepath.Ext(name) != ".wasm" { + return nil, fmt.Errorf("must specify a name to the plugin, and the name must end with .wasm") } return &pluginConfig{ pluginConfigType: PluginConfigTypeLocalWasm, name: name, options: options, - path: path, + args: args, }, nil } func newRemotePluginConfig( pluginRef bufparse.Ref, options map[string]any, + args []string, ) (*pluginConfig, error) { return &pluginConfig{ - pluginConfigType: PluginConfigTypeRemote, - name: pluginRef.FullName().Name(), + pluginConfigType: PluginConfigTypeRemoteWasm, + name: pluginRef.String(), options: options, + args: args, ref: pluginRef, }, nil } @@ -209,8 +212,8 @@ func (p *pluginConfig) Options() map[string]any { return p.options } -func (p *pluginConfig) Path() []string { - return p.path +func (p *pluginConfig) Args() []string { + return p.args } func (p *pluginConfig) Ref() bufparse.Ref { @@ -229,15 +232,11 @@ func newExternalV2ForPluginConfig( externalBufYAMLFilePluginV2 := externalBufYAMLFilePluginV2{ Options: pluginConfig.Options(), } - switch pluginConfig.Type() { - case PluginConfigTypeLocal: - path := pluginConfig.Path() - switch { - case len(path) == 1: - externalBufYAMLFilePluginV2.Plugin = path[0] - case len(path) > 1: - externalBufYAMLFilePluginV2.Plugin = path - } + args := pluginConfig.Args() + if len(args) == 0 { + externalBufYAMLFilePluginV2.Plugin = pluginConfig.Name() + } else { + externalBufYAMLFilePluginV2.Plugin = append([]string{pluginConfig.Name()}, args...) } return externalBufYAMLFilePluginV2, nil } diff --git a/private/bufpkg/bufplugin/plugin.go b/private/bufpkg/bufplugin/plugin.go index df03de8060..3096a0ac78 100644 --- a/private/bufpkg/bufplugin/plugin.go +++ b/private/bufpkg/bufplugin/plugin.go @@ -32,22 +32,29 @@ type Plugin interface { // // An OpaqueID's structure should not be relied upon, and is not a // globally-unique identifier. It's uniqueness property only applies to - // the lifetime of the Plugin, and only within Plugin commonly built - // from the Workspace root. + // the lifetime of the Plugin, and only within the Workspace the Plugin + // is defined in. // - // If two Plugins have the same FullName, they will have the same OpaqueID. + // If two Plugins have the same Name and Args, they will have the same OpaqueID. OpaqueID() string - // Path returns the path, including arguments, to invoke the binary plugin. + // Name returns the name of the Plugin. + // - For local Plugins, this is the path to the executable binary. + // - For local Wasm Plugins, this is the path to the Wasm binary. + // - For remote Plugins, this is the FullName of the Plugin in the form + // remote/owner/name. // - // This is not empty only when the Plugin is local. - Path() []string + // This is never empty. + Name() string + // Args returns the arguments to invoke the Plugin. + // + // May be nil. + Args() []string // FullName returns the full name of the Plugin. // // May be nil. Callers should not rely on this value being present. // However, this is always present for remote Plugins. // - // At least one of FullName or Path will always be present. Use OpaqueID - // as an always-present identifier. + // Use OpaqueID as an always-present identifier. FullName() bufparse.FullName // CommitID returns the BSR ID of the Commit. // @@ -107,12 +114,14 @@ type Plugin interface { // NewLocalWasmPlugin returns a new Plugin for a local Wasm plugin. func NewLocalWasmPlugin( pluginFullName bufparse.FullName, + args []string, getData func() ([]byte, error), ) (Plugin, error) { return newPlugin( "", // description pluginFullName, - nil, // path + pluginFullName.String(), + args, uuid.Nil, // commitID true, // isWasm true, // isLocal @@ -125,7 +134,8 @@ func NewLocalWasmPlugin( type plugin struct { description string pluginFullName bufparse.FullName - path []string + name string + args []string commitID uuid.UUID isWasm bool isLocal bool @@ -137,18 +147,19 @@ type plugin struct { func newPlugin( description string, pluginFullName bufparse.FullName, - path []string, + name string, + args []string, commitID uuid.UUID, isWasm bool, isLocal bool, getData func() ([]byte, error), ) (*plugin, error) { + if name == "" { + return nil, syserror.New("name not present when constructing a Plugin") + } if isWasm && getData == nil { return nil, syserror.Newf("getData not present when constructing a Wasm Plugin") } - if !isWasm && len(path) == 0 { - return nil, syserror.New("path not present when constructing a non-Wasm Plugin") - } if !isLocal && pluginFullName == nil { return nil, syserror.New("pluginFullName not present when constructing a remote Plugin") } @@ -164,7 +175,8 @@ func newPlugin( plugin := &plugin{ description: description, pluginFullName: pluginFullName, - path: path, + name: name, + args: args, commitID: commitID, isWasm: isWasm, isLocal: isLocal, @@ -175,14 +187,15 @@ func newPlugin( } func (p *plugin) OpaqueID() string { - if p.pluginFullName != nil { - return p.pluginFullName.String() - } - return strings.Join(p.path, " ") + return strings.Join(append([]string{p.name}, p.args...), " ") +} + +func (p *plugin) Name() string { + return p.name } -func (p *plugin) Path() []string { - return p.path +func (p *plugin) Args() []string { + return p.args } func (p *plugin) FullName() bufparse.FullName { diff --git a/private/pkg/pluginrpcutil/pluginrpcutil.go b/private/pkg/pluginrpcutil/pluginrpcutil.go index 3164e316fa..a585a47946 100644 --- a/private/pkg/pluginrpcutil/pluginrpcutil.go +++ b/private/pkg/pluginrpcutil/pluginrpcutil.go @@ -15,6 +15,9 @@ package pluginrpcutil import ( + "fmt" + "os" + "github.com/bufbuild/buf/private/pkg/wasm" "pluginrpc.com/pluginrpc" ) @@ -24,9 +27,36 @@ func NewRunner(programName string, programArgs ...string) pluginrpc.Runner { return newRunner(programName, programArgs...) } -// NewWasmRunner returns a new pluginrpc.Runner for the wasm.Runtime and program name. +// NewWasmRunner returns a new pluginrpc.Runner for the wasm.Runtime and Wasm data. +// +// This is used for Wasm plugins. getData returns the Wasm data. +// The program name is the name of the source file. Must be non-empty. +// The program args are the arguments to the program. May be empty. +func NewWasmRunner(delegate wasm.Runtime, getData func() ([]byte, error), programName string, programArgs ...string) pluginrpc.Runner { + return newWasmRunner(delegate, getData, programName, programArgs...) +} + +// NewLocalWasmRunner returns a new pluginrpc.Runner for the local wasm file. // -// This runner is used for local Wasm plugins. The program name is the path to the Wasm file. -func NewWasmRunner(delegate wasm.Runtime, programName string, programArgs ...string) pluginrpc.Runner { - return newWasmRunner(delegate, programName, programArgs...) +// This is used for local Wasm plugins. The program name is the path to the Wasm file. +// The program args are the arguments to the program. May be empty. +// The Wasm file is read from the filesystem. +func NewLocalWasmRunner(delegate wasm.Runtime, programName string, programArgs ...string) pluginrpc.Runner { + getData := func() ([]byte, error) { + // Find the plugin path. We use the same logic as exec.LookPath, but we do + // not require the file to be executable. So check the local directory + // first before checking the PATH. + var path string + if fileInfo, err := os.Stat(programName); err == nil && !fileInfo.IsDir() { + path = programName + } else { + var err error + path, err = unsafeLookPath(programName) + if err != nil { + return nil, fmt.Errorf("could not find file %q in PATH: %v", programName, err) + } + } + return os.ReadFile(path) + } + return NewWasmRunner(delegate, getData, programName, programArgs...) } diff --git a/private/pkg/pluginrpcutil/wasm_runner.go b/private/pkg/pluginrpcutil/wasm_runner.go index 0c7640ab41..ec8f9099bb 100644 --- a/private/pkg/pluginrpcutil/wasm_runner.go +++ b/private/pkg/pluginrpcutil/wasm_runner.go @@ -18,7 +18,6 @@ import ( "context" "errors" "fmt" - "os" "os/exec" "slices" "sync" @@ -29,6 +28,7 @@ import ( type wasmRunner struct { delegate wasm.Runtime + getData func() ([]byte, error) programName string programArgs []string // lock protects compiledModule and compiledModuleErr. Store called as @@ -41,11 +41,13 @@ type wasmRunner struct { func newWasmRunner( delegate wasm.Runtime, + getData func() ([]byte, error), programName string, programArgs ...string, ) *wasmRunner { return &wasmRunner{ delegate: delegate, + getData: getData, programName: programName, programArgs: programArgs, } @@ -79,22 +81,9 @@ func (r *wasmRunner) loadCompiledModuleOnce(ctx context.Context) (wasm.CompiledM } func (r *wasmRunner) loadCompiledModule(ctx context.Context) (wasm.CompiledModule, error) { - // Find the plugin path. We use the same logic as exec.LookPath, but we do - // not require the file to be executable. So check the local directory - // first before checking the PATH. - var path string - if fileInfo, err := os.Stat(r.programName); err == nil && !fileInfo.IsDir() { - path = r.programName - } else { - var err error - path, err = unsafeLookPath(r.programName) - if err != nil { - return nil, fmt.Errorf("could not find plugin %q in PATH: %v", r.programName, err) - } - } - moduleWasm, err := os.ReadFile(path) + moduleWasm, err := r.getData() if err != nil { - return nil, fmt.Errorf("could not read plugin %q: %v", r.programName, err) + return nil, fmt.Errorf("could not read plugin %q: %err", r.programName, err) } // Compile the module. This CompiledModule is never released, so // subsequent calls to this function will benefit from the cached