From 384392757a45fee225c4995ee356e1f9d5a01e74 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Fri, 6 Dec 2024 15:23:00 -0500 Subject: [PATCH 01/18] Add support for running remote plugins This PR adds support for invoking of remote Wasm plugins locally. Remote plugins Refs are resolved using the buf.lock file in the input's v2 workspace. --- private/buf/bufctl/controller.go | 115 +++++++++++++++ private/buf/buflsp/buflsp.go | 8 +- private/buf/buflsp/file.go | 36 ++++- private/buf/bufmigrate/migrator.go | 7 +- private/buf/bufworkspace/workspace.go | 16 ++- .../buf/bufworkspace/workspace_provider.go | 69 +++++++-- private/buf/bufworkspace/workspace_test.go | 2 + private/buf/cmd/buf/buf_test.go | 7 +- private/buf/cmd/buf/command/beta/lsp/lsp.go | 11 +- .../buf/cmd/buf/command/breaking/breaking.go | 18 ++- .../buf/command/config/internal/internal.go | 10 +- private/buf/cmd/buf/command/lint/lint.go | 18 ++- .../cmd/buf/command/mod/internal/internal.go | 7 +- .../command/plugin/pluginpush/pluginpush.go | 4 + .../cmd/protoc-gen-buf-breaking/breaking.go | 7 +- private/buf/cmd/protoc-gen-buf-lint/lint.go | 7 +- private/bufpkg/bufcheck/breaking_test.go | 8 +- private/bufpkg/bufcheck/bufcheck.go | 15 +- private/bufpkg/bufcheck/lint_test.go | 8 +- private/bufpkg/bufcheck/multi_client_test.go | 17 ++- private/bufpkg/bufcheck/runner_provider.go | 131 ++++++++++++++++-- private/bufpkg/bufconfig/buf_lock_file.go | 50 ++++++- private/bufpkg/bufconfig/plugin_config.go | 106 ++++++++------ private/bufpkg/bufplugin/plugin.go | 80 +++++++++-- .../bufpkg/bufplugin/plugin_key_provider.go | 55 ++++++++ private/pkg/pluginrpcutil/pluginrpcutil.go | 48 ++++++- private/pkg/pluginrpcutil/wasm_runner.go | 22 +-- 27 files changed, 737 insertions(+), 145 deletions(-) diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index 1cb04b8e30..d4543c502e 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -29,6 +29,7 @@ import ( "github.com/bufbuild/buf/private/buf/bufwkt/bufwktstore" "github.com/bufbuild/buf/private/buf/bufworkspace" "github.com/bufbuild/buf/private/bufpkg/bufanalysis" + "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/bufimage/bufimageutil" @@ -48,6 +49,7 @@ import ( "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" ) @@ -128,6 +130,17 @@ 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, + wasmRuntime wasm.Runtime, + options ...FunctionOption, + ) (bufcheck.RunnerProvider, error) } func NewController( @@ -243,6 +256,7 @@ func newController( graphProvider, moduleDataProvider, commitProvider, + pluginKeyProvider, ) controller.workspaceDepManagerProvider = bufworkspace.NewWorkspaceDepManagerProvider( logger, @@ -706,6 +720,32 @@ func (c *controller) PutMessage( return errors.Join(err, writeCloser.Close()) } +func (c *controller) GetCheckRunnerProvider( + ctx context.Context, + input string, + wasmRuntime wasm.Runtime, + options ...FunctionOption, +) (_ bufcheck.RunnerProvider, retErr error) { + defer c.handleFileAnnotationSetRetError(&retErr) + functionOptions := newFunctionOptions(c) + for _, option := range options { + option(functionOptions) + } + ref, err := c.buffetchRefParser.GetRef(ctx, input) + if err != nil { + return nil, err + } + pluginKeyProvider, err := c.getPluginKeyProviderForRef(ctx, ref, functionOptions) + if err != nil { + return nil, err + } + return bufcheck.NewLocalRunnerProvider( + wasmRuntime, + pluginKeyProvider, + c.pluginDataProvider, + ), nil +} + func (c *controller) getImage( ctx context.Context, input string, @@ -1159,6 +1199,81 @@ Declare %q in the deps key in your buf.yaml.`, return nil } +// getPluginKeyProviderForRef create a new PluginKeyProvider for the Ref. +// +// 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, + ref buffetch.Ref, + functionOptions *functionOptions, +) (_ bufplugin.PluginKeyProvider, retErr error) { + switch t := ref.(type) { + case buffetch.ProtoFileRef: + workspace, err := c.getWorkspaceForProtoFileRef(ctx, t, functionOptions) + if err != nil { + return nil, err + } + return bufplugin.NewStaticPluginKeyProvider(workspace.RemotePluginKeys()) + case buffetch.SourceRef: + workspace, err := c.getWorkspaceForSourceRef(ctx, t, functionOptions) + if err != nil { + return nil, err + } + return bufplugin.NewStaticPluginKeyProvider(workspace.RemotePluginKeys()) + case buffetch.ModuleRef: + workspace, err := c.getWorkspaceForModuleRef(ctx, t, functionOptions) + if err != nil { + return nil, err + } + return bufplugin.NewStaticPluginKeyProvider(workspace.RemotePluginKeys()) + case buffetch.MessageRef: + bucket, err := c.storageosProvider.NewReadWriteBucket( + ".", + storageos.ReadWriteBucketWithSymlinksIfSupported(), + ) + if err != nil { + return nil, err + } + bufYAMLFile, err := bufconfig.GetBufYAMLFileForPrefixOrOverride( + ctx, + bucket, + ".", + functionOptions.configOverride, + ) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return nil, err + } + // We did not find a buf.yaml in our current directory, + // and there was no config override. + return bufplugin.NopPluginKeyProvider, nil + } + var pluginKeys []bufplugin.PluginKey + if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { + bufLockFile, err := bufconfig.GetBufLockFileForPrefix( + ctx, + bucket, + // buf.lock files live next to the buf.yaml + ".", + ) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return nil, err + } + // We did not find a buf.lock in our current directory. + // Remote plugins are not available. + return bufplugin.NopPluginKeyProvider, nil + } + pluginKeys = bufLockFile.RemotePluginKeys() + } + return bufplugin.NewStaticPluginKeyProvider(pluginKeys) + default: + // This is a system error. + return nil, syserror.Newf("invalid Ref: %T", ref) + } +} + // 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. // diff --git a/private/buf/buflsp/buflsp.go b/private/buf/buflsp/buflsp.go index 536cf71d43..2d440f64f7 100644 --- a/private/buf/buflsp/buflsp.go +++ b/private/buf/buflsp/buflsp.go @@ -25,11 +25,11 @@ import ( "sync/atomic" "github.com/bufbuild/buf/private/buf/bufctl" - "github.com/bufbuild/buf/private/bufpkg/bufcheck" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/slogext" "github.com/bufbuild/buf/private/pkg/storage" "github.com/bufbuild/buf/private/pkg/storage/storageos" + "github.com/bufbuild/buf/private/pkg/wasm" "go.lsp.dev/jsonrpc2" "go.lsp.dev/protocol" "go.uber.org/zap" @@ -43,7 +43,7 @@ func Serve( wktBucket storage.ReadBucket, container appext.Container, controller bufctl.Controller, - checkClient bufcheck.Client, + wasmRuntime wasm.Runtime, stream jsonrpc2.Stream, ) (jsonrpc2.Conn, error) { // The LSP protocol deals with absolute filesystem paths. This requires us to @@ -68,7 +68,7 @@ func Serve( container: container, logger: container.Logger(), controller: controller, - checkClient: checkClient, + wasmRuntime: wasmRuntime, rootBucket: bucket, wktBucket: wktBucket, } @@ -96,7 +96,7 @@ type lsp struct { logger *slog.Logger controller bufctl.Controller - checkClient bufcheck.Client + wasmRuntime wasm.Runtime rootBucket storage.ReadBucket fileManager *fileManager diff --git a/private/buf/buflsp/file.go b/private/buf/buflsp/file.go index 8bad05ec37..6f2572ce0a 100644 --- a/private/buf/buflsp/file.go +++ b/private/buf/buflsp/file.go @@ -59,8 +59,9 @@ type file struct { version int32 hasText bool // Whether this file has ever had text read into it. - workspace bufworkspace.Workspace - module bufmodule.Module + workspace bufworkspace.Workspace + module bufmodule.Module + checkClient bufcheck.Client againstStrategy againstStrategy againstGitRef string @@ -371,6 +372,24 @@ func (f *file) FindModule(ctx context.Context) { return } + // Get the check runner provider for this file. The client is scoped to + // the input Buf lock file, so we need to get the check runner provider + // for the workspace that contains this file. + checkRunnerProvider, err := f.lsp.controller.GetCheckRunnerProvider(ctx, f.uri.Filename(), f.lsp.wasmRuntime) + if err != nil { + f.lsp.logger.Warn("could not get check runner provider", slogext.ErrorAttr(err)) + return + } + checkClient, err := bufcheck.NewClient( + f.lsp.logger, + checkRunnerProvider, + bufcheck.ClientWithStderr(f.lsp.container.Stderr()), + ) + if err != nil { + f.lsp.logger.Warn("could not create check client", slogext.ErrorAttr(err)) + return + } + // Figure out which module this file belongs to. var module bufmodule.Module for _, mod := range workspace.Modules() { @@ -398,6 +417,7 @@ func (f *file) FindModule(ctx context.Context) { f.workspace = workspace f.module = module + f.checkClient = checkClient } // IndexImports finds URIs for all of the files imported by this file. @@ -641,9 +661,13 @@ func (f *file) RunLints(ctx context.Context) bool { f.lsp.logger.Warn(fmt.Sprintf("could not find image for %q", f.uri)) return false } + if f.checkClient == nil { + f.lsp.logger.Warn(fmt.Sprintf("could not find check client for %q", f.uri)) + return false + } f.lsp.logger.Debug(fmt.Sprintf("running lint for %q in %v", f.uri, f.module.FullName())) - return f.appendLintErrors("buf lint", f.lsp.checkClient.Lint( + return f.appendLintErrors("buf lint", f.checkClient.Lint( ctx, f.workspace.GetLintConfigForOpaqueID(f.module.OpaqueID()), f.image, @@ -664,9 +688,13 @@ func (f *file) RunBreaking(ctx context.Context) bool { f.lsp.logger.Warn(fmt.Sprintf("could not find --against image for %q", f.uri)) return false } + if f.checkClient == nil { + f.lsp.logger.Warn(fmt.Sprintf("could not find check client for %q", f.uri)) + return false + } f.lsp.logger.Debug(fmt.Sprintf("running breaking for %q in %v", f.uri, f.module.FullName())) - return f.appendLintErrors("buf breaking", f.lsp.checkClient.Breaking( + return f.appendLintErrors("buf breaking", f.checkClient.Breaking( ctx, f.workspace.GetBreakingConfigForOpaqueID(f.module.OpaqueID()), f.image, 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/bufworkspace/workspace.go b/private/buf/bufworkspace/workspace.go index 68ea77ac06..b27eb8441f 100644 --- a/private/buf/bufworkspace/workspace.go +++ b/private/buf/bufworkspace/workspace.go @@ -18,6 +18,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/slicesext" ) @@ -72,8 +73,14 @@ type Workspace interface { // detector ignoring these configs anyways. GetBreakingConfigForOpaqueID(opaqueID string) bufconfig.BreakingConfig // PluginConfigs gets the configured PluginConfigs of the Workspace. + // + // These come from buf.yaml files. PluginConfigs() []bufconfig.PluginConfig - // ConfiguredDepModuleRefs returns the configured dependencies of the Workspace as ModuleRefs. + // RemotePluginKeys gets the remote PluginKeys of the Workspace. + // + // These come from buf.lock files. + RemotePluginKeys() []bufplugin.PluginKey + // ConfiguredDepModuleRefs returns the configured dependencies of the Workspace as Refs. // // These come from buf.yaml files. // @@ -105,6 +112,7 @@ type workspace struct { opaqueIDToLintConfig map[string]bufconfig.LintConfig opaqueIDToBreakingConfig map[string]bufconfig.BreakingConfig pluginConfigs []bufconfig.PluginConfig + remotePluginKeys []bufplugin.PluginKey configuredDepModuleRefs []bufparse.Ref // If true, the workspace was created from v2 buf.yamls. @@ -117,6 +125,7 @@ func newWorkspace( opaqueIDToLintConfig map[string]bufconfig.LintConfig, opaqueIDToBreakingConfig map[string]bufconfig.BreakingConfig, pluginConfigs []bufconfig.PluginConfig, + remotePluginKeys []bufplugin.PluginKey, configuredDepModuleRefs []bufparse.Ref, isV2 bool, ) *workspace { @@ -125,6 +134,7 @@ func newWorkspace( opaqueIDToLintConfig: opaqueIDToLintConfig, opaqueIDToBreakingConfig: opaqueIDToBreakingConfig, pluginConfigs: pluginConfigs, + remotePluginKeys: remotePluginKeys, configuredDepModuleRefs: configuredDepModuleRefs, isV2: isV2, } @@ -142,6 +152,10 @@ func (w *workspace) PluginConfigs() []bufconfig.PluginConfig { return slicesext.Copy(w.pluginConfigs) } +func (w *workspace) RemotePluginKeys() []bufplugin.PluginKey { + return slicesext.Copy(w.remotePluginKeys) +} + func (w *workspace) ConfiguredDepModuleRefs() []bufparse.Ref { return slicesext.Copy(w.configuredDepModuleRefs) } diff --git a/private/buf/bufworkspace/workspace_provider.go b/private/buf/bufworkspace/workspace_provider.go index a9811072cf..58152f94b3 100644 --- a/private/buf/bufworkspace/workspace_provider.go +++ b/private/buf/bufworkspace/workspace_provider.go @@ -25,6 +25,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/slogext" @@ -77,12 +78,14 @@ func NewWorkspaceProvider( graphProvider bufmodule.GraphProvider, moduleDataProvider bufmodule.ModuleDataProvider, commitProvider bufmodule.CommitProvider, + pluginKeyProvider bufplugin.PluginKeyProvider, ) WorkspaceProvider { return newWorkspaceProvider( logger, graphProvider, moduleDataProvider, commitProvider, + pluginKeyProvider, ) } @@ -93,6 +96,10 @@ type workspaceProvider struct { graphProvider bufmodule.GraphProvider moduleDataProvider bufmodule.ModuleDataProvider commitProvider bufmodule.CommitProvider + + // pluginKeyProvider is only used for getting remote plugin keys for a single module + // when an override is specified. + pluginKeyProvider bufplugin.PluginKeyProvider } func newWorkspaceProvider( @@ -100,12 +107,14 @@ func newWorkspaceProvider( graphProvider bufmodule.GraphProvider, moduleDataProvider bufmodule.ModuleDataProvider, commitProvider bufmodule.CommitProvider, + pluginKeyProvider bufplugin.PluginKeyProvider, ) *workspaceProvider { return &workspaceProvider{ logger: logger, graphProvider: graphProvider, moduleDataProvider: moduleDataProvider, commitProvider: commitProvider, + pluginKeyProvider: pluginKeyProvider, } } @@ -136,8 +145,11 @@ func (w *workspaceProvider) GetWorkspaceForModuleKey( targetModuleConfig := bufconfig.DefaultModuleConfigV1 // By default, there will be no plugin configs, however, similar to the lint and breaking // configs, there may be an override, in which case, we need to populate the plugin configs - // from the override. - var pluginConfigs []bufconfig.PluginConfig + // from the override. Any remote plugin refs will be resolved by the pluginKeyProvider. + var ( + pluginConfigs []bufconfig.PluginConfig + remotePluginKeys []bufplugin.PluginKey + ) if config.configOverride != "" { bufYAMLFile, err := bufconfig.GetBufYAMLFileForOverride(config.configOverride) if err != nil { @@ -150,7 +162,7 @@ func (w *workspaceProvider) GetWorkspaceForModuleKey( case 1: // If we have a single ModuleConfig, we assume that regardless of whether or not // This ModuleConfig has a name, that this is what the user intends to associate - // with the tqrget module. This also handles the v1 case - v1 buf.yamls will always + // with the target module. This also handles the v1 case - v1 buf.yamls will always // only have a single ModuleConfig, and it was expected pre-refactor that regardless // of if the ModuleConfig had a name associated with it or not, the lint and breaking // config that came from it would be associated. @@ -172,6 +184,26 @@ func (w *workspaceProvider) GetWorkspaceForModuleKey( } if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { pluginConfigs = bufYAMLFile.PluginConfigs() + remotePluginRefs := slicesext.Filter( + slicesext.Map(pluginConfigs, func(pluginConfig bufconfig.PluginConfig) bufparse.Ref { + return pluginConfig.Ref() + }), + func(ref bufparse.Ref) bool { + return ref != nil + }, + ) + // Resolve the remote plugin keys for any remote plugins. + if len(remotePluginRefs) > 0 { + var err error + remotePluginKeys, err = w.pluginKeyProvider.GetPluginKeysForPluginRefs( + ctx, + remotePluginRefs, + bufplugin.DigestTypeP1, + ) + if err != nil { + return nil, err + } + } } } @@ -209,18 +241,18 @@ func (w *workspaceProvider) GetWorkspaceForModuleKey( opaqueIDToLintConfig, opaqueIDToBreakingConfig, pluginConfigs, + remotePluginKeys, nil, false, ), nil } -func (w *workspaceProvider) GetWorkspaceForBucket( +func (w *workspaceProvider) getWorkspaceTargetingForBucket( ctx context.Context, bucket storage.ReadBucket, bucketTargeting buftarget.BucketTargeting, options ...WorkspaceBucketOption, -) (Workspace, error) { - defer slogext.DebugProfile(w.logger)() +) (*workspaceTargeting, error) { config, err := newWorkspaceBucketConfig(options) if err != nil { return nil, err @@ -232,7 +264,7 @@ func (w *workspaceProvider) GetWorkspaceForBucket( return nil, err } } - workspaceTargeting, err := newWorkspaceTargeting( + return newWorkspaceTargeting( ctx, w.logger, config, @@ -241,6 +273,21 @@ func (w *workspaceProvider) GetWorkspaceForBucket( overrideBufYAMLFile, config.ignoreAndDisallowV1BufWorkYAMLs, ) +} + +func (w *workspaceProvider) GetWorkspaceForBucket( + ctx context.Context, + bucket storage.ReadBucket, + bucketTargeting buftarget.BucketTargeting, + options ...WorkspaceBucketOption, +) (Workspace, error) { + defer slogext.DebugProfile(w.logger)() + workspaceTargeting, err := w.getWorkspaceTargetingForBucket( + ctx, + bucket, + bucketTargeting, + options..., + ) if err != nil { return nil, err } @@ -358,7 +405,8 @@ func (w *workspaceProvider) getWorkspaceForBucketAndModuleDirPathsV1Beta1OrV1( return w.getWorkspaceForBucketModuleSet( moduleSet, v1WorkspaceTargeting.bucketIDToModuleConfig, - nil, + nil, // No plugin configs for v1 + nil, // No remote plugin keys for v1 v1WorkspaceTargeting.allConfiguredDepModuleRefs, false, ) @@ -370,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, @@ -398,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: @@ -455,6 +505,7 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2( moduleSet, v2Targeting.bucketIDToModuleConfig, v2Targeting.bufYAMLFile.PluginConfigs(), + remotePluginKeys, v2Targeting.bufYAMLFile.ConfiguredDepModuleRefs(), true, ) @@ -465,6 +516,7 @@ func (w *workspaceProvider) getWorkspaceForBucketModuleSet( moduleSet bufmodule.ModuleSet, bucketIDToModuleConfig map[string]bufconfig.ModuleConfig, pluginConfigs []bufconfig.PluginConfig, + remotePluginKeys []bufplugin.PluginKey, // Expected to already be unique by FullName. configuredDepModuleRefs []bufparse.Ref, isV2 bool, @@ -490,6 +542,7 @@ func (w *workspaceProvider) getWorkspaceForBucketModuleSet( opaqueIDToLintConfig, opaqueIDToBreakingConfig, pluginConfigs, + remotePluginKeys, configuredDepModuleRefs, isV2, ), nil diff --git a/private/buf/bufworkspace/workspace_test.go b/private/buf/bufworkspace/workspace_test.go index 0006c48217..01e59e5049 100644 --- a/private/buf/bufworkspace/workspace_test.go +++ b/private/buf/bufworkspace/workspace_test.go @@ -24,6 +24,7 @@ import ( "github.com/bufbuild/buf/private/buf/buftarget" "github.com/bufbuild/buf/private/bufpkg/bufmodule" "github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduletesting" + "github.com/bufbuild/buf/private/bufpkg/bufplugin" "github.com/bufbuild/buf/private/pkg/dag/dagtest" "github.com/bufbuild/buf/private/pkg/ioext" "github.com/bufbuild/buf/private/pkg/normalpath" @@ -321,6 +322,7 @@ func testNewWorkspaceProvider(t *testing.T, testModuleDatas ...bufmoduletesting. bsrProvider, bsrProvider, bsrProvider, + bufplugin.NopPluginKeyProvider, ) } 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..e7b7b2fc4e 100644 --- a/private/buf/cmd/buf/command/beta/lsp/lsp.go +++ b/private/buf/cmd/buf/command/beta/lsp/lsp.go @@ -26,7 +26,6 @@ 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/pkg/app/appcmd" "github.com/bufbuild/buf/private/pkg/app/appext" "github.com/bufbuild/buf/private/pkg/ioext" @@ -113,16 +112,8 @@ func run( defer func() { retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) }() - checkClient, err := bufcheck.NewClient( - container.Logger(), - bufcheck.NewRunnerProvider(wasmRuntime), - bufcheck.ClientWithStderr(container.Stderr()), - ) - if err != nil { - return err - } - conn, err := buflsp.Serve(ctx, wktBucket, container, controller, checkClient, jsonrpc2.NewStream(transport)) + conn, err := buflsp.Serve(ctx, wktBucket, container, controller, wasmRuntime, jsonrpc2.NewStream(transport)) if err != nil { return err } diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index 71b699c9a2..869cb3a84e 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/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 @@ -216,11 +219,20 @@ func run( defer func() { retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) }() + checkRunnerProvider, err := controller.GetCheckRunnerProvider( + ctx, + input, + wasmRuntime, + inputControllerOptions..., + ) + if err != nil { + return err + } var allFileAnnotations []bufanalysis.FileAnnotation for i, imageWithConfig := range imageWithConfigs { client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(wasmRuntime), + checkRunnerProvider, 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..40af99066c 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -183,6 +183,10 @@ func lsRun( return err } } + controller, err := bufcli.NewController(container) + if err != nil { + return err + } wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err @@ -194,9 +198,13 @@ func lsRun( defer func() { retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) }() + checkRunnerProvider, err := controller.GetCheckRunnerProvider(ctx, ".", wasmRuntime) + if err != nil { + return err + } client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(wasmRuntime), + checkRunnerProvider, 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..839558f346 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -121,11 +121,14 @@ func run( if err != nil { return err } + controllerOptions := []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), + controllerOptions..., ) if err != nil { return err @@ -141,11 +144,20 @@ func run( defer func() { retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) }() + checkRunnerProvider, err := controller.GetCheckRunnerProvider( + ctx, + input, + wasmRuntime, + controllerOptions..., + ) + if err != nil { + return err + } var allFileAnnotations []bufanalysis.FileAnnotation for _, imageWithConfig := range imageWithConfigs { client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewRunnerProvider(wasmRuntime), + checkRunnerProvider, 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..94efe36374 100644 --- a/private/buf/cmd/buf/command/plugin/pluginpush/pluginpush.go +++ b/private/buf/cmd/buf/command/plugin/pluginpush/pluginpush.go @@ -148,9 +148,13 @@ func upload( var plugin bufplugin.Plugin switch { case flags.Binary != "": + // We create a local plugin reference to the Wasm binary. + pluginName := flags.Binary var err error plugin, err = bufplugin.NewLocalWasmPlugin( pluginFullName, + pluginName, + 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..32b4716baa 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" @@ -1312,6 +1313,7 @@ func testBreaking( bufmodule.NopGraphProvider, bufmodule.NopModuleDataProvider, bufmodule.NopCommitProvider, + bufplugin.NopPluginKeyProvider, ) previousWorkspace, err := workspaceProvider.GetWorkspaceForBucket( ctx, @@ -1344,7 +1346,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..d5cee8cfc9 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,8 @@ 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 and +// the given plugin providers. // // This implementation should only be used for local applications. It is safe to // use concurrently. @@ -178,13 +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 NewRunnerProvider( +// 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, ) } diff --git a/private/bufpkg/bufcheck/lint_test.go b/private/bufpkg/bufcheck/lint_test.go index 626f3738b2..cbc8ce64cd 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" @@ -1318,6 +1319,7 @@ func testLintWithOptions( bufmodule.NopGraphProvider, bufmodule.NopModuleDataProvider, bufmodule.NopCommitProvider, + bufplugin.NopPluginKeyProvider, ).GetWorkspaceForBucket( ctx, readWriteBucket, @@ -1355,7 +1357,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..3e7d73c3dd 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,135 @@ 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:]..., + return pluginrpcutil.NewLocalRunner( + 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, + ) 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 + pluginName string + pluginArgs []string + // lock protects runner. + lock sync.RWMutex + runner pluginrpc.Runner +} + +func newRemoteWasmPluginRunner( + wasmRuntime wasm.Runtime, + pluginKeyProvider bufplugin.PluginKeyProvider, + pluginDataProvider bufplugin.PluginDataProvider, + pluginConfig bufconfig.PluginConfig, +) (*remoteWasmPluginRunner, error) { + pluginRef := pluginConfig.Ref() + if pluginRef == nil { + return nil, syserror.Newf("Ref nil on PluginConfig of type %v", bufconfig.PluginConfigTypeRemoteWasm) + } + return &remoteWasmPluginRunner{ + wasmRuntime: wasmRuntime, + pluginKeyProvider: pluginKeyProvider, + pluginDataProvider: pluginDataProvider, + pluginRef: pluginRef, + pluginName: pluginConfig.Name(), + pluginArgs: pluginConfig.Args(), + }, 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] + return pluginrpcutil.NewWasmRunner(r.wasmRuntime, data.Data, r.pluginName, r.pluginArgs...), nil +} diff --git a/private/bufpkg/bufconfig/buf_lock_file.go b/private/bufpkg/bufconfig/buf_lock_file.go index bb0490fd1e..c37abd22a9 100644 --- a/private/bufpkg/bufconfig/buf_lock_file.go +++ b/private/bufpkg/bufconfig/buf_lock_file.go @@ -205,13 +205,22 @@ func newBufLockFile( if err := validateNoDuplicateModuleKeysByFullName(depModuleKeys); err != nil { return nil, err } + if err := validateNoDuplicatePluginKeysByFullName(remotePluginKeys); err != nil { + return nil, err + } switch fileVersion { case FileVersionV1Beta1, FileVersionV1: - if err := validateExpectedDigestType(depModuleKeys, fileVersion, bufmodule.DigestTypeB4); err != nil { + if err := validateModuleExpectedDigestType(depModuleKeys, fileVersion, bufmodule.DigestTypeB4); err != nil { return nil, err } + if len(remotePluginKeys) > 0 { + return nil, errors.New("remote plugins are not supported in v1 or v1beta1 buf.lock files") + } case FileVersionV2: - if err := validateExpectedDigestType(depModuleKeys, fileVersion, bufmodule.DigestTypeB5); err != nil { + if err := validateModuleExpectedDigestType(depModuleKeys, fileVersion, bufmodule.DigestTypeB5); err != nil { + return nil, err + } + if err := validatePluginExpectedDigestType(remotePluginKeys, fileVersion, bufplugin.DigestTypeP1); err != nil { return nil, err } default: @@ -522,6 +531,18 @@ func validateNoDuplicateModuleKeysByFullName(moduleKeys []bufmodule.ModuleKey) e return nil } +func validateNoDuplicatePluginKeysByFullName(pluginKeys []bufplugin.PluginKey) error { + pluginFullNameStringMap := make(map[string]struct{}) + for _, pluginKey := range pluginKeys { + pluginFullNameString := pluginKey.FullName().String() + if _, ok := pluginFullNameStringMap[pluginFullNameString]; ok { + return fmt.Errorf("duplicate plugin %q attempted to be added to lock file", pluginFullNameString) + } + pluginFullNameStringMap[pluginFullNameString] = struct{}{} + } + return nil +} + func validateV1AndV1Beta1DepsHaveCommits(bufLockFile BufLockFile) error { switch fileVersion := bufLockFile.FileVersion(); fileVersion { case FileVersionV1Beta1, FileVersionV1: @@ -545,7 +566,7 @@ func validateV1AndV1Beta1DepsHaveCommits(bufLockFile BufLockFile) error { } } -func validateExpectedDigestType( +func validateModuleExpectedDigestType( moduleKeys []bufmodule.ModuleKey, fileVersion FileVersion, expectedDigestType bufmodule.DigestType, @@ -568,6 +589,29 @@ func validateExpectedDigestType( return nil } +func validatePluginExpectedDigestType( + pluginKeys []bufplugin.PluginKey, + fileVersion FileVersion, + expectedDigestType bufplugin.DigestType, +) error { + for _, pluginKey := range pluginKeys { + digest, err := pluginKey.Digest() + if err != nil { + return err + } + if digest.Type() != expectedDigestType { + return fmt.Errorf( + "%s lock files must use digest type %v, but remote plugin %s had a digest type of %v", + fileVersion, + expectedDigestType, + pluginKey.String(), + digest.Type(), + ) + } + } + return nil +} + // externalBufLockFileV1Beta1V1 represents the v1 or v1beta1 buf.lock file, // which have the same shape. type externalBufLockFileV1Beta1V1 struct { diff --git a/private/bufpkg/bufconfig/plugin_config.go b/private/bufpkg/bufconfig/plugin_config.go index d35f9f55da..967e4f611b 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. @@ -42,17 +41,22 @@ type PluginConfigType int type PluginConfig interface { // Type returns the plugin type. This is never the zero value. Type() PluginConfigType - // Name returns the plugin name. This is never empty. + // Name returns the plugin name. + // + // This may be a path, a remote reference, or a Wasm file. Invoking code + // should check the Type to determine how to interpret this. + // + // This is never empty. Name() string // Options returns the plugin options. // // 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, 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. @@ -64,30 +68,47 @@ type PluginConfig interface { // NewLocalPluginConfig returns a new PluginConfig for a local plugin. func NewLocalPluginConfig( name string, + args []string, options map[string]any, - path []string, ) (PluginConfig, error) { return newLocalPluginConfig( name, + args, options, - path, ) } // NewLocalWasmPluginConfig returns a new PluginConfig for a local Wasm plugin. // -// The first path argument is the path to the Wasm plugin and must end with .wasm. -// The remaining path arguments are the arguments to the Wasm plugin. These are passed -// to the Wasm plugin as command line arguments. +// The name is the path to the Wasm plugin and must end with .wasm. +// The args are the arguments to the Wasm plugin. These are passed to the Wasm plugin +// as command line arguments. func NewLocalWasmPluginConfig( name string, + args []string, options map[string]any, - path []string, ) (PluginConfig, error) { return newLocalWasmPluginConfig( name, + args, + options, + ) +} + +// NewRemoteWasmPluginConfig returns a new PluginConfig for a remote Wasm plugin. +// +// The pluginRef is the remote reference to the plugin. +// The args are the arguments to the remote plugin. These are passed to the remote plugin +// as command line arguments. +func NewRemoteWasmPluginConfig( + pluginRef bufparse.Ref, + args []string, + options map[string]any, +) (PluginConfig, error) { + return newRemotePluginConfig( + pluginRef, + args, options, - path, ) } @@ -95,9 +116,9 @@ func NewLocalWasmPluginConfig( type pluginConfig struct { pluginConfigType PluginConfigType - name string options map[string]any - path []string + name string + args []string ref bufparse.Ref } @@ -123,6 +144,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 @@ -131,6 +153,7 @@ func newPluginConfigForExternalV2( if _, err := os.Stat(path[0]); os.IsNotExist(err) { return newRemotePluginConfig( pluginRef, + args, options, ) } @@ -138,61 +161,63 @@ func newPluginConfigForExternalV2( // Wasm plugins are suffixed with .wasm. Otherwise, it's a binary. if filepath.Ext(path[0]) == ".wasm" { return newLocalWasmPluginConfig( - strings.Join(path, " "), + name, + args, options, - path, ) } return newLocalPluginConfig( - strings.Join(path, " "), + name, + args, options, - path, ) } func newLocalPluginConfig( name string, + args []string, options map[string]any, - path []string, ) (*pluginConfig, error) { - if len(path) == 0 { + if len(name) == 0 { return nil, errors.New("must specify a path to the plugin") } return &pluginConfig{ pluginConfigType: PluginConfigTypeLocal, - name: name, options: options, - path: path, + name: name, + args: args, }, nil } func newLocalWasmPluginConfig( name string, + args []string, options map[string]any, - path []string, ) (*pluginConfig, error) { - if len(path) == 0 { + if len(name) == 0 { return nil, errors.New("must specify a path to the plugin") } - if filepath.Ext(path[0]) != ".wasm" { + if filepath.Ext(name) != ".wasm" { return nil, fmt.Errorf("must specify a path to the plugin, and the first path argument must end with .wasm") } return &pluginConfig{ pluginConfigType: PluginConfigTypeLocalWasm, - name: name, options: options, - path: path, + name: name, + args: args, }, nil } func newRemotePluginConfig( pluginRef bufparse.Ref, + args []string, options map[string]any, ) (*pluginConfig, error) { return &pluginConfig{ - pluginConfigType: PluginConfigTypeRemote, - name: pluginRef.FullName().Name(), + pluginConfigType: PluginConfigTypeRemoteWasm, + name: pluginRef.String(), options: options, + args: args, ref: pluginRef, }, nil } @@ -209,8 +234,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 +254,12 @@ 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() + switch { + case len(args) == 0: + externalBufYAMLFilePluginV2.Plugin = pluginConfig.Name() + case len(args) > 0: + 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..570e8fc2b8 100644 --- a/private/bufpkg/bufplugin/plugin.go +++ b/private/bufpkg/bufplugin/plugin.go @@ -37,10 +37,14 @@ type Plugin interface { // // If two Plugins have the same FullName, 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. // - // 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. + // + // This may be empty. + Args() []string // FullName returns the full name of the Plugin. // // May be nil. Callers should not rely on this value being present. @@ -104,15 +108,35 @@ type Plugin interface { isPlugin() } +// NewLocalPlugin returns a new Plugin for a local plugin. +func NewLocalPlugin( + name string, + args []string, +) (Plugin, error) { + return newPlugin( + "", // description + nil, // pluginFullName + name, + args, + uuid.Nil, // commitID + false, // isWasm + true, // isLocal + nil, // getData + ) +} + // NewLocalWasmPlugin returns a new Plugin for a local Wasm plugin. func NewLocalWasmPlugin( - pluginFullName bufparse.FullName, + fullName bufparse.FullName, + name string, + args []string, getData func() ([]byte, error), ) (Plugin, error) { return newPlugin( "", // description - pluginFullName, - nil, // path + fullName, + name, + args, uuid.Nil, // commitID true, // isWasm true, // isLocal @@ -120,12 +144,32 @@ func NewLocalWasmPlugin( ) } +// NewRemoteWasmPlugin returns a new Plugin for a remote Wasm plugin. +func NewRemoteWasmPlugin( + pluginFullName bufparse.FullName, + args []string, + commitID uuid.UUID, + getData func() ([]byte, error), +) (Plugin, error) { + return newPlugin( + "", // description + pluginFullName, + pluginFullName.String(), + args, + commitID, + true, // isWasm + false, // isLocal + getData, + ) +} + // *** PRIVATE *** type plugin struct { description string pluginFullName bufparse.FullName - path []string + name string + args []string commitID uuid.UUID isWasm bool isLocal bool @@ -137,18 +181,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 +209,8 @@ func newPlugin( plugin := &plugin{ description: description, pluginFullName: pluginFullName, - path: path, + name: name, + args: args, commitID: commitID, isWasm: isWasm, isLocal: isLocal, @@ -178,11 +224,15 @@ func (p *plugin) OpaqueID() string { if p.pluginFullName != nil { return p.pluginFullName.String() } - return strings.Join(p.path, " ") + return p.name + " " + strings.Join(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/bufpkg/bufplugin/plugin_key_provider.go b/private/bufpkg/bufplugin/plugin_key_provider.go index fcde880280..cef430f5db 100644 --- a/private/bufpkg/bufplugin/plugin_key_provider.go +++ b/private/bufpkg/bufplugin/plugin_key_provider.go @@ -16,9 +16,11 @@ package bufplugin import ( "context" + "fmt" "io/fs" "github.com/bufbuild/buf/private/bufpkg/bufparse" + "github.com/bufbuild/buf/private/pkg/slicesext" ) var ( @@ -41,6 +43,32 @@ type PluginKeyProvider interface { GetPluginKeysForPluginRefs(context.Context, []bufparse.Ref, DigestType) ([]PluginKey, error) } +// NewStaticPluginKeyProvider returns a new PluginKeyProvider for a set of PluginKeys. +// +// The set of PluginKeys must be unique by FullName. If there are duplicates, +// an error will be returned. The Ref will be matched to the PluginKey by FullName. +// If the Ref is not found in the set of provided keys, an error with +// fs.ErrNotExist will be returned. +func NewStaticPluginKeyProvider(pluginKeys []PluginKey) (PluginKeyProvider, error) { + if len(pluginKeys) == 0 { + return NopPluginKeyProvider, nil + } + pluginKeysByFullName, err := slicesext.ToUniqueValuesMap(pluginKeys, func(pluginKey PluginKey) string { + return pluginKey.FullName().String() + }) + if err != nil { + return nil, err + } + digetType, err := UniqueDigestTypeForPluginKeys(pluginKeys) + if err != nil { + return nil, err + } + return staticPluginKeyProvider{ + pluginKeysByFullName: pluginKeysByFullName, + digestType: digetType, + }, nil +} + // *** PRIVATE *** type nopPluginKeyProvider struct{} @@ -52,3 +80,30 @@ func (nopPluginKeyProvider) GetPluginKeysForPluginRefs( ) ([]PluginKey, error) { return nil, fs.ErrNotExist } + +type staticPluginKeyProvider struct { + pluginKeysByFullName map[string]PluginKey + digestType DigestType +} + +func (s staticPluginKeyProvider) GetPluginKeysForPluginRefs( + _ context.Context, + refs []bufparse.Ref, + digestType DigestType, +) ([]PluginKey, error) { + if digestType != s.digestType { + return nil, fmt.Errorf("expected DigestType %v, got %v", s.digestType, digestType) + } + pluginKeys := make([]PluginKey, len(refs)) + for i, ref := range refs { + // Only the FullName is used to match the PluginKey. The Ref is not + // validated to match the PluginKey as there is not enough information + // to do so. + pluginKey, ok := s.pluginKeysByFullName[ref.FullName().String()] + if !ok { + return nil, fs.ErrNotExist + } + pluginKeys[i] = pluginKey + } + return pluginKeys, nil +} diff --git a/private/pkg/pluginrpcutil/pluginrpcutil.go b/private/pkg/pluginrpcutil/pluginrpcutil.go index 3164e316fa..326bc87ce8 100644 --- a/private/pkg/pluginrpcutil/pluginrpcutil.go +++ b/private/pkg/pluginrpcutil/pluginrpcutil.go @@ -15,18 +15,54 @@ package pluginrpcutil import ( + "fmt" + "os" + "github.com/bufbuild/buf/private/pkg/wasm" "pluginrpc.com/pluginrpc" ) -// NewRunner returns a new pluginrpc.Runner for the program name. -func NewRunner(programName string, programArgs ...string) pluginrpc.Runner { +// NewLocalRunner returns a new pluginrpc.Runner for the local program. +// +// The programName is the path or name of the program. Any program args are passed to +// the program when it is run. The programArgs may be nil. +func NewLocalRunner(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. +// +// The getData function should return the Wasm module bytes for the program. +// The program name is the name of the program. Any program args are passed to +// the program when it is run. The programArgs may be nil. +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 wasm.Runtime. // -// 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...) +// The program name is the path to the Wasm file. Any program args are passed to +// the program when it is run. The programArgs may be nil. +func NewLocalWasmRunner(delegate wasm.Runtime, programName string, programArgs ...string) pluginrpc.Runner { + getData := func() ([]byte, error) { + // Find the plugin filePath. 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 filePath string + if fileInfo, err := os.Stat(programName); err == nil && !fileInfo.IsDir() { + filePath = programName + } else { + var err error + filePath, err = unsafeLookPath(programName) + if err != nil { + return nil, fmt.Errorf("could not find plugin %q in PATH: %v", programName, err) + } + } + moduleWasm, err := os.ReadFile(filePath) + if err != nil { + return nil, fmt.Errorf("could not read plugin %q: %v", programName, err) + } + return moduleWasm, nil + } + return newWasmRunner(delegate, getData, programName, programArgs...) } diff --git a/private/pkg/pluginrpcutil/wasm_runner.go b/private/pkg/pluginrpcutil/wasm_runner.go index 0c7640ab41..cba797f8ff 100644 --- a/private/pkg/pluginrpcutil/wasm_runner.go +++ b/private/pkg/pluginrpcutil/wasm_runner.go @@ -17,8 +17,6 @@ package pluginrpcutil import ( "context" "errors" - "fmt" - "os" "os/exec" "slices" "sync" @@ -29,6 +27,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 +40,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 +80,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, err } // Compile the module. This CompiledModule is never released, so // subsequent calls to this function will benefit from the cached From 8495fb8425ab66882d16e71cfeb9e2c32a8ee302 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 10 Dec 2024 19:47:15 +0000 Subject: [PATCH 02/18] Validate remote PluginConfigs are present in buf.lock --- CHANGELOG.md | 1 + private/buf/bufctl/controller.go | 53 +++++++++++++++---- private/buf/buflsp/file.go | 2 +- .../buf/bufworkspace/workspace_provider.go | 4 +- private/bufpkg/bufcheck/runner_provider.go | 2 +- .../bufpkg/bufplugin/plugin_key_provider.go | 16 +++--- 6 files changed, 56 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 545e830e20..8640fa7654 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ file. Only WebAssembly check plugins are supported at this time. - Add `buf registry plugin commit {add-label,info,list,resolve}` to manage BSR plugin commits. - Add `buf registry plugin label {archive,info,list,unarchive}` to manage BSR plugin commits. +- Support remote check plugins in `buf lint` and `buf breaking` commands. ## [v1.47.2] - 2024-11-14 diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index d4543c502e..c562cc886a 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -133,8 +133,9 @@ type Controller interface { // 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. + // using the PluginConfigs declared in the input buf.yaml. The input + // provided will resolve to a Workspace, and the remote PluginConfigs + // Refs will be resolved to the PluginKeys from the buf.lock file. GetCheckRunnerProvider( ctx context.Context, input string, @@ -1201,32 +1202,44 @@ Declare %q in the deps key in your buf.yaml.`, // getPluginKeyProviderForRef create a new PluginKeyProvider for the Ref. // -// 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. +// The Ref is resolved to a Workspace. The Workspaces PluginConfigs from the +// buf.yaml file and the PluginKeys from the buf.lock are resolved to create the +// PluginKeyProvider. +// If the Ref is a MessageRef, the current directory buf.lock file is used. +// +// PluginConfigs are validated to ensure that all remote PluginConfigs are +// pinned in the buf.lock file. func (c *controller) getPluginKeyProviderForRef( ctx context.Context, ref buffetch.Ref, functionOptions *functionOptions, ) (_ bufplugin.PluginKeyProvider, retErr error) { + var ( + pluginConfigs []bufconfig.PluginConfig + pluginKeys []bufplugin.PluginKey + ) switch t := ref.(type) { case buffetch.ProtoFileRef: workspace, err := c.getWorkspaceForProtoFileRef(ctx, t, functionOptions) if err != nil { return nil, err } - return bufplugin.NewStaticPluginKeyProvider(workspace.RemotePluginKeys()) + pluginConfigs = workspace.PluginConfigs() + pluginKeys = workspace.RemotePluginKeys() case buffetch.SourceRef: workspace, err := c.getWorkspaceForSourceRef(ctx, t, functionOptions) if err != nil { return nil, err } - return bufplugin.NewStaticPluginKeyProvider(workspace.RemotePluginKeys()) + pluginConfigs = workspace.PluginConfigs() + pluginKeys = workspace.RemotePluginKeys() case buffetch.ModuleRef: workspace, err := c.getWorkspaceForModuleRef(ctx, t, functionOptions) if err != nil { return nil, err } - return bufplugin.NewStaticPluginKeyProvider(workspace.RemotePluginKeys()) + pluginConfigs = workspace.PluginConfigs() + pluginKeys = workspace.RemotePluginKeys() case buffetch.MessageRef: bucket, err := c.storageosProvider.NewReadWriteBucket( ".", @@ -1247,10 +1260,11 @@ func (c *controller) getPluginKeyProviderForRef( } // We did not find a buf.yaml in our current directory, // and there was no config override. + // Remote plugins are not available. return bufplugin.NopPluginKeyProvider, nil } - var pluginKeys []bufplugin.PluginKey if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { + pluginConfigs = bufYAMLFile.PluginConfigs() bufLockFile, err := bufconfig.GetBufLockFileForPrefix( ctx, bucket, @@ -1267,11 +1281,32 @@ func (c *controller) getPluginKeyProviderForRef( } pluginKeys = bufLockFile.RemotePluginKeys() } - return bufplugin.NewStaticPluginKeyProvider(pluginKeys) default: // This is a system error. return nil, syserror.Newf("invalid Ref: %T", ref) } + // Validate that all remote PluginConfigs are present in the buf.lock file. + pluginKeysByFullName, err := slicesext.ToUniqueValuesMap(pluginKeys, func(pluginKey bufplugin.PluginKey) string { + return pluginKey.FullName().String() + }) + if err != nil { + return nil, fmt.Errorf("failed to validate remote PluginKeys: %w", err) + } + // Remote PluginConfig Refs are any PluginConfigs that have a Ref. + remotePluginRefs := slicesext.Filter( + slicesext.Map(pluginConfigs, func(pluginConfig bufconfig.PluginConfig) bufparse.Ref { + return pluginConfig.Ref() + }), + func(pluginRef bufparse.Ref) bool { + return pluginRef != nil + }, + ) + for _, remotePluginRef := range remotePluginRefs { + if _, ok := pluginKeysByFullName[remotePluginRef.FullName().String()]; !ok { + return nil, fmt.Errorf(`remote plugin %q is not in the buf.lock file, use "buf plugin update" to pin remote refs`, remotePluginRef) + } + } + return bufplugin.NewStaticPluginKeyProvider(pluginKeys) } // handleFileAnnotationSetError will attempt to handle the error as a FileAnnotationSet, and if so, print diff --git a/private/buf/buflsp/file.go b/private/buf/buflsp/file.go index 6f2572ce0a..b1a7651dac 100644 --- a/private/buf/buflsp/file.go +++ b/private/buf/buflsp/file.go @@ -373,7 +373,7 @@ func (f *file) FindModule(ctx context.Context) { } // Get the check runner provider for this file. The client is scoped to - // the input Buf lock file, so we need to get the check runner provider + // the inputs buf.lock file, so we need to get the check runner provider // for the workspace that contains this file. checkRunnerProvider, err := f.lsp.controller.GetCheckRunnerProvider(ctx, f.uri.Filename(), f.lsp.wasmRuntime) if err != nil { diff --git a/private/buf/bufworkspace/workspace_provider.go b/private/buf/bufworkspace/workspace_provider.go index 58152f94b3..0852af753a 100644 --- a/private/buf/bufworkspace/workspace_provider.go +++ b/private/buf/bufworkspace/workspace_provider.go @@ -405,8 +405,8 @@ func (w *workspaceProvider) getWorkspaceForBucketAndModuleDirPathsV1Beta1OrV1( return w.getWorkspaceForBucketModuleSet( moduleSet, v1WorkspaceTargeting.bucketIDToModuleConfig, - nil, // No plugin configs for v1 - nil, // No remote plugin keys for v1 + nil, // No PluginConfigs for v1 + nil, // No remote PluginKeys for v1 v1WorkspaceTargeting.allConfiguredDepModuleRefs, false, ) diff --git a/private/bufpkg/bufcheck/runner_provider.go b/private/bufpkg/bufcheck/runner_provider.go index 3e7d73c3dd..7bd3746969 100644 --- a/private/bufpkg/bufcheck/runner_provider.go +++ b/private/bufpkg/bufcheck/runner_provider.go @@ -100,7 +100,7 @@ func newRemoteWasmPluginRunner( ) (*remoteWasmPluginRunner, error) { pluginRef := pluginConfig.Ref() if pluginRef == nil { - return nil, syserror.Newf("Ref nil on PluginConfig of type %v", bufconfig.PluginConfigTypeRemoteWasm) + return nil, syserror.Newf("Ref nil on PluginConfig of type %v", pluginConfig.Type()) } return &remoteWasmPluginRunner{ wasmRuntime: wasmRuntime, diff --git a/private/bufpkg/bufplugin/plugin_key_provider.go b/private/bufpkg/bufplugin/plugin_key_provider.go index cef430f5db..0fba69f8b6 100644 --- a/private/bufpkg/bufplugin/plugin_key_provider.go +++ b/private/bufpkg/bufplugin/plugin_key_provider.go @@ -59,13 +59,8 @@ func NewStaticPluginKeyProvider(pluginKeys []PluginKey) (PluginKeyProvider, erro if err != nil { return nil, err } - digetType, err := UniqueDigestTypeForPluginKeys(pluginKeys) - if err != nil { - return nil, err - } return staticPluginKeyProvider{ pluginKeysByFullName: pluginKeysByFullName, - digestType: digetType, }, nil } @@ -83,7 +78,6 @@ func (nopPluginKeyProvider) GetPluginKeysForPluginRefs( type staticPluginKeyProvider struct { pluginKeysByFullName map[string]PluginKey - digestType DigestType } func (s staticPluginKeyProvider) GetPluginKeysForPluginRefs( @@ -91,9 +85,6 @@ func (s staticPluginKeyProvider) GetPluginKeysForPluginRefs( refs []bufparse.Ref, digestType DigestType, ) ([]PluginKey, error) { - if digestType != s.digestType { - return nil, fmt.Errorf("expected DigestType %v, got %v", s.digestType, digestType) - } pluginKeys := make([]PluginKey, len(refs)) for i, ref := range refs { // Only the FullName is used to match the PluginKey. The Ref is not @@ -103,6 +94,13 @@ func (s staticPluginKeyProvider) GetPluginKeysForPluginRefs( if !ok { return nil, fs.ErrNotExist } + digest, err := pluginKey.Digest() + if err != nil { + return nil, err + } + if digest.Type() != digestType { + return nil, fmt.Errorf("expected DigestType %v, got %v", digestType, digest.Type()) + } pluginKeys[i] = pluginKey } return pluginKeys, nil From 33cccb82255528abf212c2bec153e459936d170c Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 10 Dec 2024 21:37:16 +0000 Subject: [PATCH 03/18] Revert buf.lock validations --- private/bufpkg/bufconfig/buf_lock_file.go | 50 ++--------------------- 1 file changed, 3 insertions(+), 47 deletions(-) diff --git a/private/bufpkg/bufconfig/buf_lock_file.go b/private/bufpkg/bufconfig/buf_lock_file.go index c37abd22a9..bb0490fd1e 100644 --- a/private/bufpkg/bufconfig/buf_lock_file.go +++ b/private/bufpkg/bufconfig/buf_lock_file.go @@ -205,22 +205,13 @@ func newBufLockFile( if err := validateNoDuplicateModuleKeysByFullName(depModuleKeys); err != nil { return nil, err } - if err := validateNoDuplicatePluginKeysByFullName(remotePluginKeys); err != nil { - return nil, err - } switch fileVersion { case FileVersionV1Beta1, FileVersionV1: - if err := validateModuleExpectedDigestType(depModuleKeys, fileVersion, bufmodule.DigestTypeB4); err != nil { + if err := validateExpectedDigestType(depModuleKeys, fileVersion, bufmodule.DigestTypeB4); err != nil { return nil, err } - if len(remotePluginKeys) > 0 { - return nil, errors.New("remote plugins are not supported in v1 or v1beta1 buf.lock files") - } case FileVersionV2: - if err := validateModuleExpectedDigestType(depModuleKeys, fileVersion, bufmodule.DigestTypeB5); err != nil { - return nil, err - } - if err := validatePluginExpectedDigestType(remotePluginKeys, fileVersion, bufplugin.DigestTypeP1); err != nil { + if err := validateExpectedDigestType(depModuleKeys, fileVersion, bufmodule.DigestTypeB5); err != nil { return nil, err } default: @@ -531,18 +522,6 @@ func validateNoDuplicateModuleKeysByFullName(moduleKeys []bufmodule.ModuleKey) e return nil } -func validateNoDuplicatePluginKeysByFullName(pluginKeys []bufplugin.PluginKey) error { - pluginFullNameStringMap := make(map[string]struct{}) - for _, pluginKey := range pluginKeys { - pluginFullNameString := pluginKey.FullName().String() - if _, ok := pluginFullNameStringMap[pluginFullNameString]; ok { - return fmt.Errorf("duplicate plugin %q attempted to be added to lock file", pluginFullNameString) - } - pluginFullNameStringMap[pluginFullNameString] = struct{}{} - } - return nil -} - func validateV1AndV1Beta1DepsHaveCommits(bufLockFile BufLockFile) error { switch fileVersion := bufLockFile.FileVersion(); fileVersion { case FileVersionV1Beta1, FileVersionV1: @@ -566,7 +545,7 @@ func validateV1AndV1Beta1DepsHaveCommits(bufLockFile BufLockFile) error { } } -func validateModuleExpectedDigestType( +func validateExpectedDigestType( moduleKeys []bufmodule.ModuleKey, fileVersion FileVersion, expectedDigestType bufmodule.DigestType, @@ -589,29 +568,6 @@ func validateModuleExpectedDigestType( return nil } -func validatePluginExpectedDigestType( - pluginKeys []bufplugin.PluginKey, - fileVersion FileVersion, - expectedDigestType bufplugin.DigestType, -) error { - for _, pluginKey := range pluginKeys { - digest, err := pluginKey.Digest() - if err != nil { - return err - } - if digest.Type() != expectedDigestType { - return fmt.Errorf( - "%s lock files must use digest type %v, but remote plugin %s had a digest type of %v", - fileVersion, - expectedDigestType, - pluginKey.String(), - digest.Type(), - ) - } - } - return nil -} - // externalBufLockFileV1Beta1V1 represents the v1 or v1beta1 buf.lock file, // which have the same shape. type externalBufLockFileV1Beta1V1 struct { From fdc52f0b330e33a10c043c93a475c984d991c97b Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 10 Dec 2024 21:42:15 +0000 Subject: [PATCH 04/18] Remove unused constructors --- private/bufpkg/bufplugin/plugin.go | 36 ------------------------------ 1 file changed, 36 deletions(-) diff --git a/private/bufpkg/bufplugin/plugin.go b/private/bufpkg/bufplugin/plugin.go index 570e8fc2b8..6f34279731 100644 --- a/private/bufpkg/bufplugin/plugin.go +++ b/private/bufpkg/bufplugin/plugin.go @@ -108,23 +108,6 @@ type Plugin interface { isPlugin() } -// NewLocalPlugin returns a new Plugin for a local plugin. -func NewLocalPlugin( - name string, - args []string, -) (Plugin, error) { - return newPlugin( - "", // description - nil, // pluginFullName - name, - args, - uuid.Nil, // commitID - false, // isWasm - true, // isLocal - nil, // getData - ) -} - // NewLocalWasmPlugin returns a new Plugin for a local Wasm plugin. func NewLocalWasmPlugin( fullName bufparse.FullName, @@ -144,25 +127,6 @@ func NewLocalWasmPlugin( ) } -// NewRemoteWasmPlugin returns a new Plugin for a remote Wasm plugin. -func NewRemoteWasmPlugin( - pluginFullName bufparse.FullName, - args []string, - commitID uuid.UUID, - getData func() ([]byte, error), -) (Plugin, error) { - return newPlugin( - "", // description - pluginFullName, - pluginFullName.String(), - args, - commitID, - true, // isWasm - false, // isLocal - getData, - ) -} - // *** PRIVATE *** type plugin struct { From 27c2537e66e7cd094e05e99e20a821e942bc4608 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 11 Dec 2024 00:23:18 +0000 Subject: [PATCH 05/18] Update docs --- private/bufpkg/bufconfig/plugin_config.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/private/bufpkg/bufconfig/plugin_config.go b/private/bufpkg/bufconfig/plugin_config.go index 967e4f611b..c45e44f1e6 100644 --- a/private/bufpkg/bufconfig/plugin_config.go +++ b/private/bufpkg/bufconfig/plugin_config.go @@ -43,8 +43,12 @@ type PluginConfig interface { Type() PluginConfigType // Name returns the plugin name. // - // This may be a path, a remote reference, or a Wasm file. Invoking code - // should check the Type to determine how to interpret this. + // If the plugin is of Type PluginConfigTypeLocal, the Name is the command + // name or path to the command. + // If the plugin is of Type PluginConfigTypeLocalWasm, the Name is the + // path to the Wasm file. It must end with .wasm. + // If the plugin is of Type PluginConfigTypeRemoteWasm, the Name is a + // vaild bufparse.Ref and Ref returns the plugin reference. // // This is never empty. Name() string @@ -53,7 +57,7 @@ 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 - // Args returns the arguments, to invoke the plugin. + // Args returns the arguments, excluding the name, for the plugin. // // This may be empty. Args() []string From eaaf098c15fd40132f184d72315fddb214129ed0 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 11 Dec 2024 00:31:36 +0000 Subject: [PATCH 06/18] Update docs --- private/buf/bufworkspace/workspace_provider.go | 5 ++++- private/buf/bufworkspace/workspace_test.go | 1 + private/bufpkg/bufplugin/plugin_key_provider.go | 9 +++++---- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/private/buf/bufworkspace/workspace_provider.go b/private/buf/bufworkspace/workspace_provider.go index 0852af753a..f0bf001369 100644 --- a/private/buf/bufworkspace/workspace_provider.go +++ b/private/buf/bufworkspace/workspace_provider.go @@ -184,6 +184,10 @@ func (w *workspaceProvider) GetWorkspaceForModuleKey( } if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { pluginConfigs = bufYAMLFile.PluginConfigs() + // To support remote plugins in the override, we need to + // resolve the remote Refs to PluginKeys. A buf.lock file + // is not expected to be present so we require the + // pluginKeyProvider to resolve these PluginKeys. remotePluginRefs := slicesext.Filter( slicesext.Map(pluginConfigs, func(pluginConfig bufconfig.PluginConfig) bufparse.Ref { return pluginConfig.Ref() @@ -192,7 +196,6 @@ func (w *workspaceProvider) GetWorkspaceForModuleKey( return ref != nil }, ) - // Resolve the remote plugin keys for any remote plugins. if len(remotePluginRefs) > 0 { var err error remotePluginKeys, err = w.pluginKeyProvider.GetPluginKeysForPluginRefs( diff --git a/private/buf/bufworkspace/workspace_test.go b/private/buf/bufworkspace/workspace_test.go index 01e59e5049..bc2f6b96e7 100644 --- a/private/buf/bufworkspace/workspace_test.go +++ b/private/buf/bufworkspace/workspace_test.go @@ -322,6 +322,7 @@ func testNewWorkspaceProvider(t *testing.T, testModuleDatas ...bufmoduletesting. bsrProvider, bsrProvider, bsrProvider, + // TODO: add support for plugins to bufmoduletesting.NewOmniProvider. bufplugin.NopPluginKeyProvider, ) } diff --git a/private/bufpkg/bufplugin/plugin_key_provider.go b/private/bufpkg/bufplugin/plugin_key_provider.go index 0fba69f8b6..967561d77e 100644 --- a/private/bufpkg/bufplugin/plugin_key_provider.go +++ b/private/bufpkg/bufplugin/plugin_key_provider.go @@ -43,12 +43,13 @@ type PluginKeyProvider interface { GetPluginKeysForPluginRefs(context.Context, []bufparse.Ref, DigestType) ([]PluginKey, error) } -// NewStaticPluginKeyProvider returns a new PluginKeyProvider for a set of PluginKeys. +// NewStaticPluginKeyProvider returns a new PluginKeyProvider for a static set of PluginKeys. // // The set of PluginKeys must be unique by FullName. If there are duplicates, -// an error will be returned. The Ref will be matched to the PluginKey by FullName. -// If the Ref is not found in the set of provided keys, an error with -// fs.ErrNotExist will be returned. +// an error will be returned. +// +// When resolving Refs, the Ref will be matched to the PluginKey by FullName. +// If the Ref is not found in the set of provided keys, an fs.ErrNotExist will be returned. func NewStaticPluginKeyProvider(pluginKeys []PluginKey) (PluginKeyProvider, error) { if len(pluginKeys) == 0 { return NopPluginKeyProvider, nil From 45ee6f12f6251e4eec6388403660dc407985163d Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 11 Dec 2024 17:06:20 +0000 Subject: [PATCH 07/18] Refactor RunnerProvider to return with TargetImageConfigs --- private/buf/bufctl/controller.go | 253 ++++++++---------- private/buf/buflsp/file.go | 11 +- .../buf/cmd/buf/command/breaking/breaking.go | 44 ++- .../buf/command/config/internal/internal.go | 11 +- private/buf/cmd/buf/command/lint/lint.go | 17 +- 5 files changed, 138 insertions(+), 198 deletions(-) diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index c562cc886a..7a6519492c 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -94,8 +94,9 @@ type Controller interface { GetTargetImageWithConfigs( ctx context.Context, input string, + wasmRuntime wasm.Runtime, options ...FunctionOption, - ) ([]ImageWithConfig, error) + ) ([]ImageWithConfig, bufcheck.RunnerProvider, error) // GetImportableImageFileInfos gets the importable .proto FileInfos for the given input. // // This includes all files that can be possible imported. For example, if a Module @@ -136,11 +137,10 @@ type Controller interface { // using the PluginConfigs declared in the input buf.yaml. The input // provided will resolve to a Workspace, and the remote PluginConfigs // Refs will be resolved to the PluginKeys from the buf.lock file. - GetCheckRunnerProvider( + GetCheckRunnerProviderForWorkspace( ctx context.Context, - input string, + workspace bufworkspace.Workspace, wasmRuntime wasm.Runtime, - options ...FunctionOption, ) (bufcheck.RunnerProvider, error) } @@ -351,8 +351,9 @@ func (c *controller) GetImageForWorkspace( func (c *controller) GetTargetImageWithConfigs( ctx context.Context, input string, + wasmRuntime wasm.Runtime, options ...FunctionOption, -) (_ []ImageWithConfig, retErr error) { +) (_ []ImageWithConfig, _ bufcheck.RunnerProvider, retErr error) { defer c.handleFileAnnotationSetRetError(&retErr) functionOptions := newFunctionOptions(c) for _, option := range options { @@ -360,38 +361,55 @@ func (c *controller) GetTargetImageWithConfigs( } ref, err := c.buffetchRefParser.GetRef(ctx, input) if err != nil { - return nil, err + return nil, nil, err } + var ( + imageWithConfigs []ImageWithConfig + pluginConfigs []bufconfig.PluginConfig + pluginKeys []bufplugin.PluginKey + ) switch t := ref.(type) { case buffetch.ProtoFileRef: workspace, err := c.getWorkspaceForProtoFileRef(ctx, t, functionOptions) if err != nil { - return nil, err + return nil, nil, err + } + imageWithConfigs, err = c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + if err != nil { + return nil, nil, err } - return c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + pluginConfigs = workspace.PluginConfigs() + pluginKeys = workspace.RemotePluginKeys() case buffetch.SourceRef: workspace, err := c.getWorkspaceForSourceRef(ctx, t, functionOptions) if err != nil { - return nil, err + return nil, nil, err + } + imageWithConfigs, err = c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + if err != nil { + return nil, nil, err } - return c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + pluginConfigs = workspace.PluginConfigs() + pluginKeys = workspace.RemotePluginKeys() case buffetch.ModuleRef: workspace, err := c.getWorkspaceForModuleRef(ctx, t, functionOptions) if err != nil { - return nil, err + return nil, nil, err } - return c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + imageWithConfigs, err = c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + pluginConfigs = workspace.PluginConfigs() + pluginKeys = workspace.RemotePluginKeys() case buffetch.MessageRef: image, err := c.getImageForMessageRef(ctx, t, functionOptions) if err != nil { - return nil, err + return nil, nil, err } bucket, err := c.storageosProvider.NewReadWriteBucket( ".", storageos.ReadWriteBucketWithSymlinksIfSupported(), ) if err != nil { - return nil, err + return nil, nil, err } lintConfig := bufconfig.DefaultLintConfigV1 breakingConfig := bufconfig.DefaultBreakingConfigV1 @@ -404,16 +422,15 @@ func (c *controller) GetTargetImageWithConfigs( ) if err != nil { if !errors.Is(err, fs.ErrNotExist) { - return nil, err + return nil, nil, err } // We did not find a buf.yaml in our current directory, and there was no config override. // Use the defaults. } else { - pluginConfigs = bufYAMLFile.PluginConfigs() if topLevelLintConfig := bufYAMLFile.TopLevelLintConfig(); topLevelLintConfig == nil { // Ensure that this is a v2 config if fileVersion := bufYAMLFile.FileVersion(); fileVersion != bufconfig.FileVersionV2 { - return nil, syserror.Newf("non-v2 version with no top-level lint config: %s", fileVersion) + return nil, nil, syserror.Newf("non-v2 version with no top-level lint config: %s", fileVersion) } // v2 config without a top-level lint config, use v2 default lintConfig = bufconfig.DefaultLintConfigV2 @@ -422,26 +439,59 @@ func (c *controller) GetTargetImageWithConfigs( } if topLevelBreakingConfig := bufYAMLFile.TopLevelBreakingConfig(); topLevelBreakingConfig == nil { if fileVersion := bufYAMLFile.FileVersion(); fileVersion != bufconfig.FileVersionV2 { - return nil, syserror.Newf("non-v2 version with no top-level breaking config: %s", fileVersion) + return nil, nil, syserror.Newf("non-v2 version with no top-level breaking config: %s", fileVersion) } // v2 config without a top-level breaking config, use v2 default breakingConfig = bufconfig.DefaultBreakingConfigV2 } else { breakingConfig = topLevelBreakingConfig } + // Plugin configs are only available in v2 configs. + if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { + pluginConfigs = bufYAMLFile.PluginConfigs() + bufLockFile, err := bufconfig.GetBufLockFileForPrefix( + ctx, + bucket, + // buf.lock files live next to the buf.yaml + ".", + ) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return nil, nil, err + } + // We did not find a buf.lock in our current directory. + // Remote plugins are not available. + pluginKeys = nil + } else { + pluginKeys = bufLockFile.RemotePluginKeys() + } + } } - return []ImageWithConfig{ + imageWithConfigs = []ImageWithConfig{ newImageWithConfig( image, lintConfig, breakingConfig, pluginConfigs, ), - }, nil + } default: // This is a system error. - return nil, syserror.Newf("invalid Ref: %T", ref) + return nil, nil, syserror.Newf("invalid Ref: %T", ref) } + pluginKeyProvider, err := newStaticPluginKeyProviderForPluginConfigs( + pluginConfigs, + pluginKeys, + ) + if err != nil { + return nil, nil, err + } + checkRunnerProvider := bufcheck.NewLocalRunnerProvider( + wasmRuntime, + pluginKeyProvider, + c.pluginDataProvider, + ) + return imageWithConfigs, checkRunnerProvider, nil } func (c *controller) GetImportableImageFileInfos( @@ -721,22 +771,15 @@ func (c *controller) PutMessage( return errors.Join(err, writeCloser.Close()) } -func (c *controller) GetCheckRunnerProvider( +func (c *controller) GetCheckRunnerProviderForWorkspace( ctx context.Context, - input string, + workspace bufworkspace.Workspace, wasmRuntime wasm.Runtime, - options ...FunctionOption, ) (_ bufcheck.RunnerProvider, retErr error) { - defer c.handleFileAnnotationSetRetError(&retErr) - functionOptions := newFunctionOptions(c) - for _, option := range options { - option(functionOptions) - } - ref, err := c.buffetchRefParser.GetRef(ctx, input) - if err != nil { - return nil, err - } - pluginKeyProvider, err := c.getPluginKeyProviderForRef(ctx, ref, functionOptions) + pluginKeyProvider, err := newStaticPluginKeyProviderForPluginConfigs( + workspace.PluginConfigs(), + workspace.RemotePluginKeys(), + ) if err != nil { return nil, err } @@ -1200,115 +1243,6 @@ Declare %q in the deps key in your buf.yaml.`, return nil } -// getPluginKeyProviderForRef create a new PluginKeyProvider for the Ref. -// -// The Ref is resolved to a Workspace. The Workspaces PluginConfigs from the -// buf.yaml file and the PluginKeys from the buf.lock are resolved to create the -// PluginKeyProvider. -// If the Ref is a MessageRef, the current directory buf.lock file is used. -// -// PluginConfigs are validated to ensure that all remote PluginConfigs are -// pinned in the buf.lock file. -func (c *controller) getPluginKeyProviderForRef( - ctx context.Context, - ref buffetch.Ref, - functionOptions *functionOptions, -) (_ bufplugin.PluginKeyProvider, retErr error) { - var ( - pluginConfigs []bufconfig.PluginConfig - pluginKeys []bufplugin.PluginKey - ) - switch t := ref.(type) { - case buffetch.ProtoFileRef: - workspace, err := c.getWorkspaceForProtoFileRef(ctx, t, functionOptions) - if err != nil { - return nil, err - } - pluginConfigs = workspace.PluginConfigs() - pluginKeys = workspace.RemotePluginKeys() - case buffetch.SourceRef: - workspace, err := c.getWorkspaceForSourceRef(ctx, t, functionOptions) - if err != nil { - return nil, err - } - pluginConfigs = workspace.PluginConfigs() - pluginKeys = workspace.RemotePluginKeys() - case buffetch.ModuleRef: - workspace, err := c.getWorkspaceForModuleRef(ctx, t, functionOptions) - if err != nil { - return nil, err - } - pluginConfigs = workspace.PluginConfigs() - pluginKeys = workspace.RemotePluginKeys() - case buffetch.MessageRef: - bucket, err := c.storageosProvider.NewReadWriteBucket( - ".", - storageos.ReadWriteBucketWithSymlinksIfSupported(), - ) - if err != nil { - return nil, err - } - bufYAMLFile, err := bufconfig.GetBufYAMLFileForPrefixOrOverride( - ctx, - bucket, - ".", - functionOptions.configOverride, - ) - if err != nil { - if !errors.Is(err, fs.ErrNotExist) { - return nil, err - } - // We did not find a buf.yaml in our current directory, - // and there was no config override. - // Remote plugins are not available. - return bufplugin.NopPluginKeyProvider, nil - } - if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { - pluginConfigs = bufYAMLFile.PluginConfigs() - bufLockFile, err := bufconfig.GetBufLockFileForPrefix( - ctx, - bucket, - // buf.lock files live next to the buf.yaml - ".", - ) - if err != nil { - if !errors.Is(err, fs.ErrNotExist) { - return nil, err - } - // We did not find a buf.lock in our current directory. - // Remote plugins are not available. - return bufplugin.NopPluginKeyProvider, nil - } - pluginKeys = bufLockFile.RemotePluginKeys() - } - default: - // This is a system error. - return nil, syserror.Newf("invalid Ref: %T", ref) - } - // Validate that all remote PluginConfigs are present in the buf.lock file. - pluginKeysByFullName, err := slicesext.ToUniqueValuesMap(pluginKeys, func(pluginKey bufplugin.PluginKey) string { - return pluginKey.FullName().String() - }) - if err != nil { - return nil, fmt.Errorf("failed to validate remote PluginKeys: %w", err) - } - // Remote PluginConfig Refs are any PluginConfigs that have a Ref. - remotePluginRefs := slicesext.Filter( - slicesext.Map(pluginConfigs, func(pluginConfig bufconfig.PluginConfig) bufparse.Ref { - return pluginConfig.Ref() - }), - func(pluginRef bufparse.Ref) bool { - return pluginRef != nil - }, - ) - for _, remotePluginRef := range remotePluginRefs { - if _, ok := pluginKeysByFullName[remotePluginRef.FullName().String()]; !ok { - return nil, fmt.Errorf(`remote plugin %q is not in the buf.lock file, use "buf plugin update" to pin remote refs`, remotePluginRef) - } - } - return bufplugin.NewStaticPluginKeyProvider(pluginKeys) -} - // 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. // @@ -1492,3 +1426,36 @@ func validateFileAnnotationErrorFormat(fileAnnotationErrorFormat string) error { fileAnnotationErrorFormatFlagName := "error-format" return appcmd.NewInvalidArgumentErrorf("--%s: invalid format: %q", fileAnnotationErrorFormatFlagName, fileAnnotationErrorFormat) } + +// newStaticPluginKeyProvider creates a new PluginKeyProvider for the set of PluginKeys. +// +// The PluginKeys come from the buf.lock file. The PluginKeyProvider is static +// and does not change. PluginConfigs are validated to ensure that all remote +// PluginConfigs are pinned in the buf.lock file. +func newStaticPluginKeyProviderForPluginConfigs( + pluginConfigs []bufconfig.PluginConfig, + pluginKeys []bufplugin.PluginKey, +) (_ bufplugin.PluginKeyProvider, retErr error) { + // Validate that all remote PluginConfigs are present in the buf.lock file. + pluginKeysByFullName, err := slicesext.ToUniqueValuesMap(pluginKeys, func(pluginKey bufplugin.PluginKey) string { + return pluginKey.FullName().String() + }) + if err != nil { + return nil, fmt.Errorf("failed to validate remote PluginKeys: %w", err) + } + // Remote PluginConfig Refs are any PluginConfigs that have a Ref. + remotePluginRefs := slicesext.Filter( + slicesext.Map(pluginConfigs, func(pluginConfig bufconfig.PluginConfig) bufparse.Ref { + return pluginConfig.Ref() + }), + func(pluginRef bufparse.Ref) bool { + return pluginRef != nil + }, + ) + for _, remotePluginRef := range remotePluginRefs { + if _, ok := pluginKeysByFullName[remotePluginRef.FullName().String()]; !ok { + return nil, fmt.Errorf(`remote plugin %q is not in the buf.lock file, use "buf plugin update" to pin remote refs`, remotePluginRef) + } + } + return bufplugin.NewStaticPluginKeyProvider(pluginKeys) +} diff --git a/private/buf/buflsp/file.go b/private/buf/buflsp/file.go index b1a7651dac..816d05ff99 100644 --- a/private/buf/buflsp/file.go +++ b/private/buf/buflsp/file.go @@ -371,11 +371,12 @@ func (f *file) FindModule(ctx context.Context) { f.lsp.logger.Warn("could not load workspace", slog.String("uri", string(f.uri)), slogext.ErrorAttr(err)) return } - - // Get the check runner provider for this file. The client is scoped to - // the inputs buf.lock file, so we need to get the check runner provider - // for the workspace that contains this file. - checkRunnerProvider, err := f.lsp.controller.GetCheckRunnerProvider(ctx, f.uri.Filename(), f.lsp.wasmRuntime) + // Get the bufcheck.RunnerProvider for this workspace. + checkRunnerProvider, err := f.lsp.controller.GetCheckRunnerProviderForWorkspace( + ctx, + workspace, + f.lsp.wasmRuntime, + ) if err != nil { f.lsp.logger.Warn("could not get check runner provider", slogext.ErrorAttr(err)) return diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index 869cb3a84e..1fb67d465f 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -161,16 +161,25 @@ func run( if err != nil { return err } + wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) + if err != nil { + return err + } + wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(wasmRuntimeCacheDir)) + if err != nil { + return err + } + defer func() { + retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) + }() // 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( + imageWithConfigs, checkRunnerProvider, err := controller.GetTargetImageWithConfigs( ctx, input, - inputControllerOptions..., + wasmRuntime, + bufctl.WithTargetPaths(flags.Paths, flags.ExcludePaths), + bufctl.WithConfigOverride(flags.Config), ) if err != nil { return err @@ -186,9 +195,10 @@ func run( } // Do not exclude imports here. bufcheck's Client requires all imports. // Use bufcheck's BreakingWithExcludeImports. - againstImageWithConfigs, err := controller.GetTargetImageWithConfigs( + againstImageWithConfigs, _, err := controller.GetTargetImageWithConfigs( ctx, flags.Against, + wasmRuntime, bufctl.WithTargetPaths(externalPaths, flags.ExcludePaths), bufctl.WithConfigOverride(flags.AgainstConfig), ) @@ -208,26 +218,6 @@ func run( len(againstImageWithConfigs), ) } - wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) - if err != nil { - return err - } - wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(wasmRuntimeCacheDir)) - if err != nil { - return err - } - defer func() { - retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) - }() - checkRunnerProvider, err := controller.GetCheckRunnerProvider( - ctx, - input, - wasmRuntime, - inputControllerOptions..., - ) - if err != nil { - return err - } var allFileAnnotations []bufanalysis.FileAnnotation for i, imageWithConfig := range imageWithConfigs { client, err := bufcheck.NewClient( diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index 40af99066c..97099c809f 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" @@ -183,10 +184,6 @@ func lsRun( return err } } - controller, err := bufcli.NewController(container) - if err != nil { - return err - } wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err @@ -198,13 +195,9 @@ func lsRun( defer func() { retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) }() - checkRunnerProvider, err := controller.GetCheckRunnerProvider(ctx, ".", wasmRuntime) - if err != nil { - return err - } client, err := bufcheck.NewClient( container.Logger(), - checkRunnerProvider, + 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 839558f346..3d2a6274df 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -121,18 +121,6 @@ func run( if err != nil { return err } - controllerOptions := []bufctl.FunctionOption{ - bufctl.WithTargetPaths(flags.Paths, flags.ExcludePaths), - bufctl.WithConfigOverride(flags.Config), - } - imageWithConfigs, err := controller.GetTargetImageWithConfigs( - ctx, - input, - controllerOptions..., - ) - if err != nil { - return err - } wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err @@ -144,11 +132,12 @@ func run( defer func() { retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) }() - checkRunnerProvider, err := controller.GetCheckRunnerProvider( + imageWithConfigs, checkRunnerProvider, err := controller.GetTargetImageWithConfigs( ctx, input, wasmRuntime, - controllerOptions..., + bufctl.WithTargetPaths(flags.Paths, flags.ExcludePaths), + bufctl.WithConfigOverride(flags.Config), ) if err != nil { return err From 45f312e6ee27939f5dc87a93467554db2f2313ce Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 11 Dec 2024 17:09:17 +0000 Subject: [PATCH 08/18] Update docs --- private/buf/bufctl/controller.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index 7a6519492c..a4453b6661 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -131,12 +131,6 @@ 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 declared in the input buf.yaml. The input - // provided will resolve to a Workspace, and the remote PluginConfigs - // Refs will be resolved to the PluginKeys from the buf.lock file. GetCheckRunnerProviderForWorkspace( ctx context.Context, workspace bufworkspace.Workspace, @@ -397,6 +391,9 @@ func (c *controller) GetTargetImageWithConfigs( return nil, nil, err } imageWithConfigs, err = c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + if err != nil { + return nil, nil, err + } pluginConfigs = workspace.PluginConfigs() pluginKeys = workspace.RemotePluginKeys() case buffetch.MessageRef: From 39f6d8146a2cb16155175948ffb6e00230bd6d11 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 11 Dec 2024 20:10:03 +0000 Subject: [PATCH 09/18] Resolve plugins dynamically for ls-rules --- private/buf/bufworkspace/workspace_provider.go | 7 +++---- .../buf/cmd/buf/command/config/internal/internal.go | 13 ++++++++++++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/private/buf/bufworkspace/workspace_provider.go b/private/buf/bufworkspace/workspace_provider.go index f0bf001369..4ff6bd7480 100644 --- a/private/buf/bufworkspace/workspace_provider.go +++ b/private/buf/bufworkspace/workspace_provider.go @@ -184,10 +184,9 @@ func (w *workspaceProvider) GetWorkspaceForModuleKey( } if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { pluginConfigs = bufYAMLFile.PluginConfigs() - // To support remote plugins in the override, we need to - // resolve the remote Refs to PluginKeys. A buf.lock file - // is not expected to be present so we require the - // pluginKeyProvider to resolve these PluginKeys. + // To support remote plugins in the override, we need to resolve the remote + // Refs to PluginKeys. A buf.lock file is not required for this operation. + // We use the BSR to resolve any remote plugin Refs. remotePluginRefs := slicesext.Filter( slicesext.Map(pluginConfigs, func(pluginConfig bufconfig.PluginConfig) bufparse.Ref { return pluginConfig.Ref() diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index 97099c809f..06e1224e4f 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -195,9 +195,20 @@ func lsRun( defer func() { retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) }() + // To support remote plugins in the override, we need to resolve the remote + // Refs to PluginKeys. A buf.lock file is not required for this operation. + // We use the BSR to resolve any remote plugin Refs. + pluginKeyProvider, err := bufcli.NewPluginKeyProvider(container) + if err != nil { + return err + } + pluginDataProvider, err := bufcli.NewPluginDataProvider(container) + if err != nil { + return err + } client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewLocalRunnerProvider(wasmRuntime, bufplugin.NopPluginKeyProvider, bufplugin.NopPluginDataProvider), + bufcheck.NewLocalRunnerProvider(wasmRuntime, pluginKeyProvider, pluginDataProvider), bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { From ca4209d4181036ae677aad469bb766dbeb7c8038 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 11 Dec 2024 22:46:23 +0000 Subject: [PATCH 10/18] wip --- private/buf/bufcli/buf_lock_file.go | 33 ++++++++++++++++ .../buf/command/config/internal/internal.go | 39 +++++++++++++------ 2 files changed, 61 insertions(+), 11 deletions(-) create mode 100644 private/buf/bufcli/buf_lock_file.go diff --git a/private/buf/bufcli/buf_lock_file.go b/private/buf/bufcli/buf_lock_file.go new file mode 100644 index 0000000000..c4962dcd6b --- /dev/null +++ b/private/buf/bufcli/buf_lock_file.go @@ -0,0 +1,33 @@ +// Copyright 2020-2024 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package bufcli + +import ( + "context" + + "github.com/bufbuild/buf/private/bufpkg/bufconfig" +) + +// GetBufLockFileForDirPath gets the buf.lock file for the directory path. +func GetBufLockFileForDirPath( + ctx context.Context, + dirPath string, +) (bufconfig.BufLockFile, error) { + bucket, err := newOSReadWriteBucketWithSymlinks(dirPath) + if err != nil { + return nil, err + } + return bufconfig.GetBufLockFileForPrefix(ctx, bucket, ".") +} diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index 06e1224e4f..6771477e36 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -167,6 +167,8 @@ func lsRun( if flags.Version != "" { configOverride = fmt.Sprintf(`{"version":"%s"}`, flags.Version) } + pluginKeyProvider := bufplugin.NopPluginKeyProvider + pluginDataProvider := bufplugin.NopPluginDataProvider bufYAMLFile, err := bufcli.GetBufYAMLFileForDirPathOrOverride(ctx, ".", configOverride) if err != nil { if !errors.Is(err, fs.ErrNotExist) { @@ -183,6 +185,25 @@ func lsRun( if err != nil { return err } + } else if configOverride != "" { + // To support remote plugins in the override, we need to resolve the remote + // Refs to PluginKeys. A buf.lock file is not required for this operation. + // We use the BSR to resolve any remote plugin Refs. + pluginKeyProvider, err = bufcli.NewPluginKeyProvider(container) + if err != nil { + return err + } + pluginDataProvider, err = bufcli.NewPluginDataProvider(container) + if err != nil { + return err + } + } else if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { + if bufLockFile, err := bufcli.GetBufLockFileForDirPath(ctx, "."); err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return err + } + } else { + } } wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { @@ -195,17 +216,6 @@ func lsRun( defer func() { retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) }() - // To support remote plugins in the override, we need to resolve the remote - // Refs to PluginKeys. A buf.lock file is not required for this operation. - // We use the BSR to resolve any remote plugin Refs. - pluginKeyProvider, err := bufcli.NewPluginKeyProvider(container) - if err != nil { - return err - } - pluginDataProvider, err := bufcli.NewPluginDataProvider(container) - if err != nil { - return err - } client, err := bufcheck.NewClient( container.Logger(), bufcheck.NewLocalRunnerProvider(wasmRuntime, pluginKeyProvider, pluginDataProvider), @@ -305,3 +315,10 @@ func getModuleConfigForModulePath(moduleConfigs []bufconfig.ModuleConfig, module return nil, fmt.Errorf("multiple modules found for %q", modulePath) } } + +func getBufYAMLFileAndRunnerProviderForDirPathOrOverride( + ctx context.Context, + +) (bufconfig.BufYAMLFile, bufcheck.RunnerProvider, error) { + +} From ffb610f9d2f593bb01dc6068f61e7f2a638e4731 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Thu, 12 Dec 2024 14:01:16 +0000 Subject: [PATCH 11/18] Resolve workspace CheckRunnerProvider for ls-rules --- .../buf/command/config/internal/internal.go | 127 ++++++++++++------ 1 file changed, 85 insertions(+), 42 deletions(-) diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index 6771477e36..e324ae888f 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -162,49 +162,10 @@ func lsRun( return appcmd.NewInvalidArgumentErrorf("--%s must be set if --%s is specified", configuredOnlyFlagName, modulePathFlagName) } } - configOverride := flags.Config if flags.Version != "" { configOverride = fmt.Sprintf(`{"version":"%s"}`, flags.Version) } - pluginKeyProvider := bufplugin.NopPluginKeyProvider - pluginDataProvider := bufplugin.NopPluginDataProvider - bufYAMLFile, err := bufcli.GetBufYAMLFileForDirPathOrOverride(ctx, ".", configOverride) - if err != nil { - if !errors.Is(err, fs.ErrNotExist) { - return err - } - bufYAMLFile, err = bufconfig.NewBufYAMLFile( - bufconfig.FileVersionV2, - []bufconfig.ModuleConfig{ - bufconfig.DefaultModuleConfigV2, - }, - nil, - nil, - ) - if err != nil { - return err - } - } else if configOverride != "" { - // To support remote plugins in the override, we need to resolve the remote - // Refs to PluginKeys. A buf.lock file is not required for this operation. - // We use the BSR to resolve any remote plugin Refs. - pluginKeyProvider, err = bufcli.NewPluginKeyProvider(container) - if err != nil { - return err - } - pluginDataProvider, err = bufcli.NewPluginDataProvider(container) - if err != nil { - return err - } - } else if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { - if bufLockFile, err := bufcli.GetBufLockFileForDirPath(ctx, "."); err != nil { - if !errors.Is(err, fs.ErrNotExist) { - return err - } - } else { - } - } wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err @@ -216,9 +177,19 @@ func lsRun( defer func() { retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) }() + bufYAMLFile, checkRunnerProvider, err := getBufYAMLFileAndRunnerProviderForDirPathOrOverride( + ctx, + container, + ".", // Dir path. + configOverride, + wasmRuntime, + ) + if err != nil { + return err + } client, err := bufcheck.NewClient( container.Logger(), - bufcheck.NewLocalRunnerProvider(wasmRuntime, pluginKeyProvider, pluginDataProvider), + checkRunnerProvider, bufcheck.ClientWithStderr(container.Stderr()), ) if err != nil { @@ -318,7 +289,79 @@ func getModuleConfigForModulePath(moduleConfigs []bufconfig.ModuleConfig, module func getBufYAMLFileAndRunnerProviderForDirPathOrOverride( ctx context.Context, - + container appext.Container, + dirPath string, + configOverride string, + wasmRuntime wasm.Runtime, ) (bufconfig.BufYAMLFile, bufcheck.RunnerProvider, error) { - + bufYAMLFile, err := bufcli.GetBufYAMLFileForDirPathOrOverride(ctx, dirPath, configOverride) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return nil, nil, err + } + bufYAMLFile, err = bufconfig.NewBufYAMLFile( + bufconfig.FileVersionV2, + []bufconfig.ModuleConfig{ + bufconfig.DefaultModuleConfigV2, + }, + nil, + nil, + ) + if err != nil { + return nil, nil, err + } + // The buf.lock file was not found, no plugins are configured. + return bufYAMLFile, bufcheck.NewLocalRunnerProvider( + wasmRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), nil + } + pluginConfigs := bufYAMLFile.PluginConfigs() + if bufYAMLFile.FileVersion() != bufconfig.FileVersionV2 || len(pluginConfigs) == 0 { + // The buf.yaml file was found, but no plugins were configured. + return bufYAMLFile, bufcheck.NewLocalRunnerProvider( + wasmRuntime, + bufplugin.NopPluginKeyProvider, + bufplugin.NopPluginDataProvider, + ), nil + } + if configOverride != "" { + // To support remote plugins in the override, we need to resolve the remote + // Refs to PluginKeys. A buf.lock file is not required for this operation. + // We use the BSR to resolve any remote plugin Refs. + pluginKeyProvider, err := bufcli.NewPluginKeyProvider(container) + if err != nil { + return nil, nil, err + } + pluginDataProvider, err := bufcli.NewPluginDataProvider(container) + if err != nil { + return nil, nil, err + } + return bufYAMLFile, bufcheck.NewLocalRunnerProvider( + wasmRuntime, + pluginKeyProvider, + pluginDataProvider, + ), nil + } + // If we have a v2 buf.yaml file, we need to use the Controller to get the + // CheckRunnerProvider for the Workspace. We use the Workspace to avoid + // re-validating the buf.lock plugins. + controller, err := bufcli.NewController(container) + if err != nil { + return nil, nil, err + } + workspace, err := controller.GetWorkspace(ctx, dirPath) + if err != nil { + return nil, nil, err + } + runnerProvider, err := controller.GetCheckRunnerProviderForWorkspace( + ctx, + workspace, + wasmRuntime, + ) + if err != nil { + return nil, nil, err + } + return bufYAMLFile, runnerProvider, nil } From 396127451ad2186dbb312fda5c58512fcf1fcf36 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Thu, 12 Dec 2024 14:09:54 +0000 Subject: [PATCH 12/18] Remove unused --- private/buf/bufcli/buf_lock_file.go | 33 ----------------------------- 1 file changed, 33 deletions(-) delete mode 100644 private/buf/bufcli/buf_lock_file.go diff --git a/private/buf/bufcli/buf_lock_file.go b/private/buf/bufcli/buf_lock_file.go deleted file mode 100644 index c4962dcd6b..0000000000 --- a/private/buf/bufcli/buf_lock_file.go +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2020-2024 Buf Technologies, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package bufcli - -import ( - "context" - - "github.com/bufbuild/buf/private/bufpkg/bufconfig" -) - -// GetBufLockFileForDirPath gets the buf.lock file for the directory path. -func GetBufLockFileForDirPath( - ctx context.Context, - dirPath string, -) (bufconfig.BufLockFile, error) { - bucket, err := newOSReadWriteBucketWithSymlinks(dirPath) - if err != nil { - return nil, err - } - return bufconfig.GetBufLockFileForPrefix(ctx, bucket, ".") -} From aa62c695a31f5046d2aad729570572fae3782c98 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Thu, 12 Dec 2024 14:27:41 +0000 Subject: [PATCH 13/18] Fix message input with config override --- private/buf/bufctl/controller.go | 86 ++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 37 deletions(-) diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index a4453b6661..eb679c4d3f 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -357,45 +357,49 @@ func (c *controller) GetTargetImageWithConfigs( if err != nil { return nil, nil, err } - var ( - imageWithConfigs []ImageWithConfig - pluginConfigs []bufconfig.PluginConfig - pluginKeys []bufplugin.PluginKey - ) switch t := ref.(type) { case buffetch.ProtoFileRef: workspace, err := c.getWorkspaceForProtoFileRef(ctx, t, functionOptions) if err != nil { return nil, nil, err } - imageWithConfigs, err = c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + imageWithConfigs, err := c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + if err != nil { + return nil, nil, err + } + checkRunnerProvider, err := c.GetCheckRunnerProviderForWorkspace(ctx, workspace, wasmRuntime) if err != nil { return nil, nil, err } - pluginConfigs = workspace.PluginConfigs() - pluginKeys = workspace.RemotePluginKeys() + return imageWithConfigs, checkRunnerProvider, nil case buffetch.SourceRef: workspace, err := c.getWorkspaceForSourceRef(ctx, t, functionOptions) if err != nil { return nil, nil, err } - imageWithConfigs, err = c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + imageWithConfigs, err := c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + if err != nil { + return nil, nil, err + } + checkRunnerProvider, err := c.GetCheckRunnerProviderForWorkspace(ctx, workspace, wasmRuntime) if err != nil { return nil, nil, err } - pluginConfigs = workspace.PluginConfigs() - pluginKeys = workspace.RemotePluginKeys() + return imageWithConfigs, checkRunnerProvider, nil case buffetch.ModuleRef: workspace, err := c.getWorkspaceForModuleRef(ctx, t, functionOptions) if err != nil { return nil, nil, err } - imageWithConfigs, err = c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + imageWithConfigs, err := c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) if err != nil { return nil, nil, err } - pluginConfigs = workspace.PluginConfigs() - pluginKeys = workspace.RemotePluginKeys() + checkRunnerProvider, err := c.GetCheckRunnerProviderForWorkspace(ctx, workspace, wasmRuntime) + if err != nil { + return nil, nil, err + } + return imageWithConfigs, checkRunnerProvider, nil case buffetch.MessageRef: image, err := c.getImageForMessageRef(ctx, t, functionOptions) if err != nil { @@ -408,9 +412,12 @@ func (c *controller) GetTargetImageWithConfigs( if err != nil { return nil, nil, err } - lintConfig := bufconfig.DefaultLintConfigV1 - breakingConfig := bufconfig.DefaultBreakingConfigV1 - var pluginConfigs []bufconfig.PluginConfig + var ( + lintConfig = bufconfig.DefaultLintConfigV1 + breakingConfig = bufconfig.DefaultBreakingConfigV1 + pluginConfigs []bufconfig.PluginConfig + pluginKeyProvider = bufplugin.NopPluginKeyProvider + ) bufYAMLFile, err := bufconfig.GetBufYAMLFileForPrefixOrOverride( ctx, bucket, @@ -424,6 +431,7 @@ func (c *controller) GetTargetImageWithConfigs( // We did not find a buf.yaml in our current directory, and there was no config override. // Use the defaults. } else { + pluginConfigs = bufYAMLFile.PluginConfigs() if topLevelLintConfig := bufYAMLFile.TopLevelLintConfig(); topLevelLintConfig == nil { // Ensure that this is a v2 config if fileVersion := bufYAMLFile.FileVersion(); fileVersion != bufconfig.FileVersionV2 { @@ -443,16 +451,20 @@ func (c *controller) GetTargetImageWithConfigs( } else { breakingConfig = topLevelBreakingConfig } - // Plugin configs are only available in v2 configs. - if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { - pluginConfigs = bufYAMLFile.PluginConfigs() - bufLockFile, err := bufconfig.GetBufLockFileForPrefix( + + if functionOptions.configOverride != "" { + // To support remote plugins in the override, we need to resolve the remote + // Refs to PluginKeys. A buf.lock file is not required for this operation. + // We use the BSR to resolve any remote plugin Refs. + pluginKeyProvider = c.pluginKeyProvider + } else if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { + var pluginKeys []bufplugin.PluginKey + if bufLockFile, err := bufconfig.GetBufLockFileForPrefix( ctx, bucket, // buf.lock files live next to the buf.yaml ".", - ) - if err != nil { + ); err != nil { if !errors.Is(err, fs.ErrNotExist) { return nil, nil, err } @@ -462,9 +474,16 @@ func (c *controller) GetTargetImageWithConfigs( } else { pluginKeys = bufLockFile.RemotePluginKeys() } + pluginKeyProvider, err = newStaticPluginKeyProviderForPluginConfigs( + pluginConfigs, + pluginKeys, + ) + if err != nil { + return nil, nil, err + } } } - imageWithConfigs = []ImageWithConfig{ + imageWithConfigs := []ImageWithConfig{ newImageWithConfig( image, lintConfig, @@ -472,23 +491,16 @@ func (c *controller) GetTargetImageWithConfigs( pluginConfigs, ), } + checkRunnerProvider := bufcheck.NewLocalRunnerProvider( + wasmRuntime, + pluginKeyProvider, + c.pluginDataProvider, + ) + return imageWithConfigs, checkRunnerProvider, nil default: // This is a system error. return nil, nil, syserror.Newf("invalid Ref: %T", ref) } - pluginKeyProvider, err := newStaticPluginKeyProviderForPluginConfigs( - pluginConfigs, - pluginKeys, - ) - if err != nil { - return nil, nil, err - } - checkRunnerProvider := bufcheck.NewLocalRunnerProvider( - wasmRuntime, - pluginKeyProvider, - c.pluginDataProvider, - ) - return imageWithConfigs, checkRunnerProvider, nil } func (c *controller) GetImportableImageFileInfos( From 5e6e4ae30a5355be4234dd662c9d1d89b3cda0ea Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Thu, 12 Dec 2024 16:29:14 +0000 Subject: [PATCH 14/18] Update docs --- private/buf/bufworkspace/workspace.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/private/buf/bufworkspace/workspace.go b/private/buf/bufworkspace/workspace.go index b27eb8441f..a1b76b7367 100644 --- a/private/buf/bufworkspace/workspace.go +++ b/private/buf/bufworkspace/workspace.go @@ -74,11 +74,11 @@ type Workspace interface { GetBreakingConfigForOpaqueID(opaqueID string) bufconfig.BreakingConfig // PluginConfigs gets the configured PluginConfigs of the Workspace. // - // These come from buf.yaml files. + // These come from the buf.lock file. Only v2 supports plugins. PluginConfigs() []bufconfig.PluginConfig // RemotePluginKeys gets the remote PluginKeys of the Workspace. // - // These come from buf.lock files. + // These come from the buf.lock file. Only v2 supports plugins. RemotePluginKeys() []bufplugin.PluginKey // ConfiguredDepModuleRefs returns the configured dependencies of the Workspace as Refs. // From 457ca75fd8a294163746ed92c129cf4ca17d6865 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Fri, 13 Dec 2024 17:59:01 +0000 Subject: [PATCH 15/18] Refactor to NewCheckClient API --- private/buf/bufctl/controller.go | 261 ++++++++++++------ private/buf/buflsp/file.go | 20 +- .../buf/bufworkspace/workspace_provider.go | 5 +- .../buf/cmd/buf/command/breaking/breaking.go | 49 ++-- .../buf/command/config/internal/internal.go | 120 ++------ private/buf/cmd/buf/command/lint/lint.go | 21 +- 6 files changed, 237 insertions(+), 239 deletions(-) diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index eb679c4d3f..9560172042 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -94,9 +94,8 @@ type Controller interface { GetTargetImageWithConfigs( ctx context.Context, input string, - wasmRuntime wasm.Runtime, options ...FunctionOption, - ) ([]ImageWithConfig, bufcheck.RunnerProvider, error) + ) ([]ImageWithConfig, error) // GetImportableImageFileInfos gets the importable .proto FileInfos for the given input. // // This includes all files that can be possible imported. For example, if a Module @@ -131,11 +130,27 @@ type Controller interface { defaultMessageEncoding buffetch.MessageEncoding, options ...FunctionOption, ) error - GetCheckRunnerProviderForWorkspace( + // NewCheckClient returns a new CheckClient for the given input. + // + // CheckClients are bound to a specific input to ensure that the correct + // plugin dependencies are used. Where the input is resolvable to a + // Workspace, this function wraps NewCheckClientForWorkspace. For + // message inputs, the local directory or config override is used. + NewCheckClient( + ctx context.Context, + input string, + wasmRuntime wasm.Runtime, + options ...FunctionOption, + ) (bufcheck.Client, error) + // NewCheckClientForWorkspace returns a new CheckClient for the given Workspace. + // + // CheckClients are bound to a specific Workspace to ensure that the correct + // plugin dependencies are used. + NewCheckClientForWorkspace( ctx context.Context, workspace bufworkspace.Workspace, wasmRuntime wasm.Runtime, - ) (bufcheck.RunnerProvider, error) + ) (bufcheck.Client, error) } func NewController( @@ -345,9 +360,8 @@ func (c *controller) GetImageForWorkspace( func (c *controller) GetTargetImageWithConfigs( ctx context.Context, input string, - wasmRuntime wasm.Runtime, options ...FunctionOption, -) (_ []ImageWithConfig, _ bufcheck.RunnerProvider, retErr error) { +) (_ []ImageWithConfig, retErr error) { defer c.handleFileAnnotationSetRetError(&retErr) functionOptions := newFunctionOptions(c) for _, option := range options { @@ -355,69 +369,42 @@ func (c *controller) GetTargetImageWithConfigs( } ref, err := c.buffetchRefParser.GetRef(ctx, input) if err != nil { - return nil, nil, err + return nil, err } switch t := ref.(type) { case buffetch.ProtoFileRef: workspace, err := c.getWorkspaceForProtoFileRef(ctx, t, functionOptions) if err != nil { - return nil, nil, err - } - imageWithConfigs, err := c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) - if err != nil { - return nil, nil, err - } - checkRunnerProvider, err := c.GetCheckRunnerProviderForWorkspace(ctx, workspace, wasmRuntime) - if err != nil { - return nil, nil, err + return nil, err } - return imageWithConfigs, checkRunnerProvider, nil + return c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) case buffetch.SourceRef: workspace, err := c.getWorkspaceForSourceRef(ctx, t, functionOptions) if err != nil { - return nil, nil, err - } - imageWithConfigs, err := c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) - if err != nil { - return nil, nil, err - } - checkRunnerProvider, err := c.GetCheckRunnerProviderForWorkspace(ctx, workspace, wasmRuntime) - if err != nil { - return nil, nil, err + return nil, err } - return imageWithConfigs, checkRunnerProvider, nil + return c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) case buffetch.ModuleRef: workspace, err := c.getWorkspaceForModuleRef(ctx, t, functionOptions) if err != nil { - return nil, nil, err - } - imageWithConfigs, err := c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) - if err != nil { - return nil, nil, err - } - checkRunnerProvider, err := c.GetCheckRunnerProviderForWorkspace(ctx, workspace, wasmRuntime) - if err != nil { - return nil, nil, err + return nil, err } - return imageWithConfigs, checkRunnerProvider, nil + return c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) case buffetch.MessageRef: image, err := c.getImageForMessageRef(ctx, t, functionOptions) if err != nil { - return nil, nil, err + return nil, err } bucket, err := c.storageosProvider.NewReadWriteBucket( ".", storageos.ReadWriteBucketWithSymlinksIfSupported(), ) if err != nil { - return nil, nil, err + return nil, err } - var ( - lintConfig = bufconfig.DefaultLintConfigV1 - breakingConfig = bufconfig.DefaultBreakingConfigV1 - pluginConfigs []bufconfig.PluginConfig - pluginKeyProvider = bufplugin.NopPluginKeyProvider - ) + lintConfig := bufconfig.DefaultLintConfigV1 + breakingConfig := bufconfig.DefaultBreakingConfigV1 + var pluginConfigs []bufconfig.PluginConfig bufYAMLFile, err := bufconfig.GetBufYAMLFileForPrefixOrOverride( ctx, bucket, @@ -426,7 +413,7 @@ func (c *controller) GetTargetImageWithConfigs( ) if err != nil { if !errors.Is(err, fs.ErrNotExist) { - return nil, nil, err + return nil, err } // We did not find a buf.yaml in our current directory, and there was no config override. // Use the defaults. @@ -435,7 +422,7 @@ func (c *controller) GetTargetImageWithConfigs( if topLevelLintConfig := bufYAMLFile.TopLevelLintConfig(); topLevelLintConfig == nil { // Ensure that this is a v2 config if fileVersion := bufYAMLFile.FileVersion(); fileVersion != bufconfig.FileVersionV2 { - return nil, nil, syserror.Newf("non-v2 version with no top-level lint config: %s", fileVersion) + return nil, syserror.Newf("non-v2 version with no top-level lint config: %s", fileVersion) } // v2 config without a top-level lint config, use v2 default lintConfig = bufconfig.DefaultLintConfigV2 @@ -444,62 +431,25 @@ func (c *controller) GetTargetImageWithConfigs( } if topLevelBreakingConfig := bufYAMLFile.TopLevelBreakingConfig(); topLevelBreakingConfig == nil { if fileVersion := bufYAMLFile.FileVersion(); fileVersion != bufconfig.FileVersionV2 { - return nil, nil, syserror.Newf("non-v2 version with no top-level breaking config: %s", fileVersion) + return nil, syserror.Newf("non-v2 version with no top-level breaking config: %s", fileVersion) } // v2 config without a top-level breaking config, use v2 default breakingConfig = bufconfig.DefaultBreakingConfigV2 } else { breakingConfig = topLevelBreakingConfig } - - if functionOptions.configOverride != "" { - // To support remote plugins in the override, we need to resolve the remote - // Refs to PluginKeys. A buf.lock file is not required for this operation. - // We use the BSR to resolve any remote plugin Refs. - pluginKeyProvider = c.pluginKeyProvider - } else if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { - var pluginKeys []bufplugin.PluginKey - if bufLockFile, err := bufconfig.GetBufLockFileForPrefix( - ctx, - bucket, - // buf.lock files live next to the buf.yaml - ".", - ); err != nil { - if !errors.Is(err, fs.ErrNotExist) { - return nil, nil, err - } - // We did not find a buf.lock in our current directory. - // Remote plugins are not available. - pluginKeys = nil - } else { - pluginKeys = bufLockFile.RemotePluginKeys() - } - pluginKeyProvider, err = newStaticPluginKeyProviderForPluginConfigs( - pluginConfigs, - pluginKeys, - ) - if err != nil { - return nil, nil, err - } - } } - imageWithConfigs := []ImageWithConfig{ + return []ImageWithConfig{ newImageWithConfig( image, lintConfig, breakingConfig, pluginConfigs, ), - } - checkRunnerProvider := bufcheck.NewLocalRunnerProvider( - wasmRuntime, - pluginKeyProvider, - c.pluginDataProvider, - ) - return imageWithConfigs, checkRunnerProvider, nil + }, nil default: // This is a system error. - return nil, nil, syserror.Newf("invalid Ref: %T", ref) + return nil, syserror.Newf("invalid Ref: %T", ref) } } @@ -780,11 +730,69 @@ func (c *controller) PutMessage( return errors.Join(err, writeCloser.Close()) } -func (c *controller) GetCheckRunnerProviderForWorkspace( +func (c *controller) NewCheckClient( + ctx context.Context, + input string, + wasmRuntime wasm.Runtime, + options ...FunctionOption, +) (_ bufcheck.Client, retErr error) { + defer c.handleFileAnnotationSetRetError(&retErr) + functionOptions := newFunctionOptions(c) + for _, option := range options { + option(functionOptions) + } + ref, err := c.buffetchRefParser.GetRef(ctx, input) + if err != nil { + return nil, err + } + switch t := ref.(type) { + case buffetch.ProtoFileRef: + workspace, err := c.getWorkspaceForProtoFileRef(ctx, t, functionOptions) + if err != nil { + return nil, err + } + return c.NewCheckClientForWorkspace(ctx, workspace, wasmRuntime) + case buffetch.SourceRef: + workspace, err := c.getWorkspaceForSourceRef(ctx, t, functionOptions) + if err != nil { + return nil, err + } + return c.NewCheckClientForWorkspace(ctx, workspace, wasmRuntime) + case buffetch.ModuleRef: + workspace, err := c.getWorkspaceForModuleRef(ctx, t, functionOptions) + if err != nil { + return nil, err + } + return c.NewCheckClientForWorkspace(ctx, workspace, wasmRuntime) + case buffetch.MessageRef: + // MessageRefs do not resolve to a Workspace, so we resolve the + // PluginKeyProvider by the buf.yaml provided in the current directory. + // This follows the logic in GetTargetImageWithConfigs. + pluginKeyProvider, err := c.getPluginKeyProviderForDirPath(ctx, ".", functionOptions) + if err != nil { + return nil, err + } + pluginRunnerProvider := bufcheck.NewLocalRunnerProvider( + wasmRuntime, + pluginKeyProvider, + c.pluginDataProvider, + ) + return bufcheck.NewClient( + c.logger, + pluginRunnerProvider, + bufcheck.ClientWithStderr(c.container.Stderr()), + ) + default: + // This is a system error. + return nil, syserror.Newf("invalid Ref: %T", ref) + } +} + +func (c *controller) NewCheckClientForWorkspace( ctx context.Context, workspace bufworkspace.Workspace, wasmRuntime wasm.Runtime, -) (_ bufcheck.RunnerProvider, retErr error) { +) (_ bufcheck.Client, retErr error) { pluginKeyProvider, err := newStaticPluginKeyProviderForPluginConfigs( workspace.PluginConfigs(), workspace.RemotePluginKeys(), @@ -792,11 +800,16 @@ func (c *controller) GetCheckRunnerProviderForWorkspace( if err != nil { return nil, err } - return bufcheck.NewLocalRunnerProvider( + pluginRunnerProvider := bufcheck.NewLocalRunnerProvider( wasmRuntime, pluginKeyProvider, c.pluginDataProvider, - ), nil + ) + return bufcheck.NewClient( + c.logger, + pluginRunnerProvider, + bufcheck.ClientWithStderr(c.container.Stderr()), + ) } func (c *controller) getImage( @@ -1278,6 +1291,70 @@ func (c *controller) handleFileAnnotationSetRetError(retErrAddr *error) { } } +// getPluginKeyProviderForDirPath creates a new PluginKeyProvider for the directory path. +// +// The directory path is resolved to a buf.yaml file and a buf.lock file. If the +// buf.yaml file is found, the PluginConfigs from the buf.yaml file and the PluginKeys +// from the buf.lock file are resolved to create the PluginKeyProvider. +// +// If a config override is provided, the PluginConfig remote Refs use the BSR +// to resolve the PluginKeys. No buf.lock is required. +// If the buf.yaml file is not found, the bufplugin.NopPluginKeyProvider is returned. +// If the buf.lock file is not found, the bufplugin.NopPluginKeyProvider is returned. +func (c *controller) getPluginKeyProviderForDirPath( + ctx context.Context, + dirPath string, + functionOptions *functionOptions, +) (_ bufplugin.PluginKeyProvider, retErr error) { + // If there is a config override, we do not use the buf.lock file. + // Remote plugins are available, use the BSR to resolve any plugin Refs. + if functionOptions.configOverride != "" { + return c.pluginKeyProvider, nil + } + var ( + pluginConfigs []bufconfig.PluginConfig + pluginKeys []bufplugin.PluginKey + ) + bucket, err := c.storageosProvider.NewReadWriteBucket( + dirPath, + storageos.ReadWriteBucketWithSymlinksIfSupported(), + ) + if err != nil { + return nil, err + } + bufYAMLFile, err := bufconfig.GetBufYAMLFileForPrefix(ctx, bucket, ".") + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return nil, err + } + // We did not find a buf.yaml in our current directory, + // and there was no config override. + // Remote plugins are not available. + return bufplugin.NopPluginKeyProvider, nil + } + pluginConfigs = bufYAMLFile.PluginConfigs() + if len(pluginConfigs) == 0 || bufYAMLFile.FileVersion() != bufconfig.FileVersionV2 { + // No plugins were found or are supported by the current version. + return bufplugin.NopPluginKeyProvider, nil + } + bufLockFile, err := bufconfig.GetBufLockFileForPrefix( + ctx, + bucket, + // buf.lock files live next to the buf.yaml + ".", + ) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return nil, err + } + // We did not find a buf.lock in our current directory. + // Remote plugins are not available. + return bufplugin.NopPluginKeyProvider, nil + } + pluginKeys = bufLockFile.RemotePluginKeys() + return newStaticPluginKeyProviderForPluginConfigs(pluginConfigs, pluginKeys) +} + func getImageFileInfosForModuleSet(ctx context.Context, moduleSet bufmodule.ModuleSet) ([]bufimage.ImageFileInfo, error) { // Sorted. fileInfos, err := bufmodule.GetFileInfos( diff --git a/private/buf/buflsp/file.go b/private/buf/buflsp/file.go index 816d05ff99..dacb2ebab1 100644 --- a/private/buf/buflsp/file.go +++ b/private/buf/buflsp/file.go @@ -371,23 +371,11 @@ func (f *file) FindModule(ctx context.Context) { f.lsp.logger.Warn("could not load workspace", slog.String("uri", string(f.uri)), slogext.ErrorAttr(err)) return } - // Get the bufcheck.RunnerProvider for this workspace. - checkRunnerProvider, err := f.lsp.controller.GetCheckRunnerProviderForWorkspace( - ctx, - workspace, - f.lsp.wasmRuntime, - ) - if err != nil { - f.lsp.logger.Warn("could not get check runner provider", slogext.ErrorAttr(err)) - return - } - checkClient, err := bufcheck.NewClient( - f.lsp.logger, - checkRunnerProvider, - bufcheck.ClientWithStderr(f.lsp.container.Stderr()), - ) + + // Get the check client for this workspace. + checkClient, err := f.lsp.controller.NewCheckClientForWorkspace(ctx, workspace, f.lsp.wasmRuntime) if err != nil { - f.lsp.logger.Warn("could not create check client", slogext.ErrorAttr(err)) + f.lsp.logger.Warn("could not get check client", slogext.ErrorAttr(err)) return } diff --git a/private/buf/bufworkspace/workspace_provider.go b/private/buf/bufworkspace/workspace_provider.go index 4ff6bd7480..e2a191eb62 100644 --- a/private/buf/bufworkspace/workspace_provider.go +++ b/private/buf/bufworkspace/workspace_provider.go @@ -184,9 +184,8 @@ func (w *workspaceProvider) GetWorkspaceForModuleKey( } if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { pluginConfigs = bufYAMLFile.PluginConfigs() - // To support remote plugins in the override, we need to resolve the remote - // Refs to PluginKeys. A buf.lock file is not required for this operation. - // We use the BSR to resolve any remote plugin Refs. + // To support remote plugins when using a config override, we need to resolve the remote + // Refs to PluginKeys. We use the pluginKeyProvider to resolve any remote plugin Refs. remotePluginRefs := slicesext.Filter( slicesext.Map(pluginConfigs, func(pluginConfig bufconfig.PluginConfig) bufparse.Ref { return pluginConfig.Ref() diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index 1fb67d465f..f1fbc79eb7 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -161,23 +161,11 @@ func run( if err != nil { return err } - wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) - if err != nil { - return err - } - wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(wasmRuntimeCacheDir)) - if err != nil { - return err - } - defer func() { - retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) - }() // Do not exclude imports here. bufcheck's Client requires all imports. // Use bufcheck's BreakingWithExcludeImports. - imageWithConfigs, checkRunnerProvider, err := controller.GetTargetImageWithConfigs( + imageWithConfigs, err := controller.GetTargetImageWithConfigs( ctx, input, - wasmRuntime, bufctl.WithTargetPaths(flags.Paths, flags.ExcludePaths), bufctl.WithConfigOverride(flags.Config), ) @@ -195,10 +183,9 @@ func run( } // Do not exclude imports here. bufcheck's Client requires all imports. // Use bufcheck's BreakingWithExcludeImports. - againstImageWithConfigs, _, err := controller.GetTargetImageWithConfigs( + againstImageWithConfigs, err := controller.GetTargetImageWithConfigs( ctx, flags.Against, - wasmRuntime, bufctl.WithTargetPaths(externalPaths, flags.ExcludePaths), bufctl.WithConfigOverride(flags.AgainstConfig), ) @@ -218,23 +205,37 @@ func run( len(againstImageWithConfigs), ) } + wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) + if err != nil { + return err + } + wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(wasmRuntimeCacheDir)) + if err != nil { + return err + } + defer func() { + retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) + }() + // Get the check client for the input images. + checkClient, err := controller.NewCheckClient( + ctx, + input, + wasmRuntime, + bufctl.WithTargetPaths(flags.Paths, flags.ExcludePaths), + bufctl.WithConfigOverride(flags.Config), + ) + if err != nil { + return err + } var allFileAnnotations []bufanalysis.FileAnnotation for i, imageWithConfig := range imageWithConfigs { - client, err := bufcheck.NewClient( - container.Logger(), - checkRunnerProvider, - bufcheck.ClientWithStderr(container.Stderr()), - ) - if err != nil { - return err - } breakingOptions := []bufcheck.BreakingOption{ bufcheck.WithPluginConfigs(imageWithConfig.PluginConfigs()...), } if flags.ExcludeImports { breakingOptions = append(breakingOptions, bufcheck.BreakingWithExcludeImports()) } - if err := client.Breaking( + if err := checkClient.Breaking( ctx, imageWithConfig.BreakingConfig(), imageWithConfig, diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index e324ae888f..c84dfba6c2 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -22,9 +22,9 @@ import ( "buf.build/go/bufplugin/check" "github.com/bufbuild/buf/private/buf/bufcli" + "github.com/bufbuild/buf/private/buf/bufctl" "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" @@ -166,6 +166,23 @@ func lsRun( if flags.Version != "" { configOverride = fmt.Sprintf(`{"version":"%s"}`, flags.Version) } + bufYAMLFile, err := bufcli.GetBufYAMLFileForDirPathOrOverride(ctx, ".", configOverride) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return err + } + bufYAMLFile, err = bufconfig.NewBufYAMLFile( + bufconfig.FileVersionV2, + []bufconfig.ModuleConfig{ + bufconfig.DefaultModuleConfigV2, + }, + nil, + nil, + ) + if err != nil { + return err + } + } wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err @@ -177,25 +194,19 @@ func lsRun( defer func() { retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) }() - bufYAMLFile, checkRunnerProvider, err := getBufYAMLFileAndRunnerProviderForDirPathOrOverride( - ctx, - container, - ".", // Dir path. - configOverride, - wasmRuntime, - ) + controller, err := bufcli.NewController(container) if err != nil { return err } - client, err := bufcheck.NewClient( - container.Logger(), - checkRunnerProvider, - bufcheck.ClientWithStderr(container.Stderr()), + checkClient, err := controller.NewCheckClient( + ctx, + ".", + wasmRuntime, + bufctl.WithConfigOverride(flags.Config), ) if err != nil { return err } - var rules []bufcheck.Rule if flags.ConfiguredOnly { moduleConfigs := bufYAMLFile.ModuleConfigs() @@ -236,7 +247,7 @@ func lsRun( configuredRuleOptions := []bufcheck.ConfiguredRulesOption{ bufcheck.WithPluginConfigs(bufYAMLFile.PluginConfigs()...), } - rules, err = client.ConfiguredRules( + rules, err = checkClient.ConfiguredRules( ctx, ruleType, checkConfig, @@ -249,7 +260,7 @@ func lsRun( allRulesOptions := []bufcheck.AllRulesOption{ bufcheck.WithPluginConfigs(bufYAMLFile.PluginConfigs()...), } - rules, err = client.AllRules( + rules, err = checkClient.AllRules( ctx, ruleType, bufYAMLFile.FileVersion(), @@ -286,82 +297,3 @@ func getModuleConfigForModulePath(moduleConfigs []bufconfig.ModuleConfig, module return nil, fmt.Errorf("multiple modules found for %q", modulePath) } } - -func getBufYAMLFileAndRunnerProviderForDirPathOrOverride( - ctx context.Context, - container appext.Container, - dirPath string, - configOverride string, - wasmRuntime wasm.Runtime, -) (bufconfig.BufYAMLFile, bufcheck.RunnerProvider, error) { - bufYAMLFile, err := bufcli.GetBufYAMLFileForDirPathOrOverride(ctx, dirPath, configOverride) - if err != nil { - if !errors.Is(err, fs.ErrNotExist) { - return nil, nil, err - } - bufYAMLFile, err = bufconfig.NewBufYAMLFile( - bufconfig.FileVersionV2, - []bufconfig.ModuleConfig{ - bufconfig.DefaultModuleConfigV2, - }, - nil, - nil, - ) - if err != nil { - return nil, nil, err - } - // The buf.lock file was not found, no plugins are configured. - return bufYAMLFile, bufcheck.NewLocalRunnerProvider( - wasmRuntime, - bufplugin.NopPluginKeyProvider, - bufplugin.NopPluginDataProvider, - ), nil - } - pluginConfigs := bufYAMLFile.PluginConfigs() - if bufYAMLFile.FileVersion() != bufconfig.FileVersionV2 || len(pluginConfigs) == 0 { - // The buf.yaml file was found, but no plugins were configured. - return bufYAMLFile, bufcheck.NewLocalRunnerProvider( - wasmRuntime, - bufplugin.NopPluginKeyProvider, - bufplugin.NopPluginDataProvider, - ), nil - } - if configOverride != "" { - // To support remote plugins in the override, we need to resolve the remote - // Refs to PluginKeys. A buf.lock file is not required for this operation. - // We use the BSR to resolve any remote plugin Refs. - pluginKeyProvider, err := bufcli.NewPluginKeyProvider(container) - if err != nil { - return nil, nil, err - } - pluginDataProvider, err := bufcli.NewPluginDataProvider(container) - if err != nil { - return nil, nil, err - } - return bufYAMLFile, bufcheck.NewLocalRunnerProvider( - wasmRuntime, - pluginKeyProvider, - pluginDataProvider, - ), nil - } - // If we have a v2 buf.yaml file, we need to use the Controller to get the - // CheckRunnerProvider for the Workspace. We use the Workspace to avoid - // re-validating the buf.lock plugins. - controller, err := bufcli.NewController(container) - if err != nil { - return nil, nil, err - } - workspace, err := controller.GetWorkspace(ctx, dirPath) - if err != nil { - return nil, nil, err - } - runnerProvider, err := controller.GetCheckRunnerProviderForWorkspace( - ctx, - workspace, - wasmRuntime, - ) - if err != nil { - return nil, nil, err - } - return bufYAMLFile, runnerProvider, nil -} diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index 3d2a6274df..9ff5162acb 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -121,6 +121,15 @@ func run( if err != nil { return err } + imageWithConfigs, err := controller.GetTargetImageWithConfigs( + ctx, + input, + bufctl.WithTargetPaths(flags.Paths, flags.ExcludePaths), + bufctl.WithConfigOverride(flags.Config), + ) + if err != nil { + return err + } wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err @@ -132,7 +141,7 @@ func run( defer func() { retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) }() - imageWithConfigs, checkRunnerProvider, err := controller.GetTargetImageWithConfigs( + checkClient, err := controller.NewCheckClient( ctx, input, wasmRuntime, @@ -144,18 +153,10 @@ func run( } var allFileAnnotations []bufanalysis.FileAnnotation for _, imageWithConfig := range imageWithConfigs { - client, err := bufcheck.NewClient( - container.Logger(), - checkRunnerProvider, - bufcheck.ClientWithStderr(container.Stderr()), - ) - if err != nil { - return err - } lintOptions := []bufcheck.LintOption{ bufcheck.WithPluginConfigs(imageWithConfig.PluginConfigs()...), } - if err := client.Lint( + if err := checkClient.Lint( ctx, imageWithConfig.LintConfig(), imageWithConfig, From 662ae5ffd480ed66e7eff0d7205b639b62497cdf Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 18 Dec 2024 16:49:42 +0100 Subject: [PATCH 16/18] Fix docs --- private/buf/bufctl/controller.go | 10 +++---- private/bufpkg/bufconfig/plugin_config.go | 11 +------ .../bufpkg/bufplugin/plugin_key_provider.go | 29 +++++++++++-------- 3 files changed, 23 insertions(+), 27 deletions(-) diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index 9560172042..f6e30183cc 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -130,9 +130,9 @@ type Controller interface { defaultMessageEncoding buffetch.MessageEncoding, options ...FunctionOption, ) error - // NewCheckClient returns a new CheckClient for the given input. + // NewCheckClient returns a new bufcheck.Client for the given input. // - // CheckClients are bound to a specific input to ensure that the correct + // Clients are bound to a specific input to ensure that the correct // plugin dependencies are used. Where the input is resolvable to a // Workspace, this function wraps NewCheckClientForWorkspace. For // message inputs, the local directory or config override is used. @@ -142,9 +142,9 @@ type Controller interface { wasmRuntime wasm.Runtime, options ...FunctionOption, ) (bufcheck.Client, error) - // NewCheckClientForWorkspace returns a new CheckClient for the given Workspace. + // NewCheckClientForWorkspace returns a new bufcheck.Client for the given Workspace. // - // CheckClients are bound to a specific Workspace to ensure that the correct + // Clients are bound to a specific Workspace to ensure that the correct // plugin dependencies are used. NewCheckClientForWorkspace( ctx context.Context, @@ -1334,7 +1334,7 @@ func (c *controller) getPluginKeyProviderForDirPath( } pluginConfigs = bufYAMLFile.PluginConfigs() if len(pluginConfigs) == 0 || bufYAMLFile.FileVersion() != bufconfig.FileVersionV2 { - // No plugins were found or are supported by the current version. + // No plugins were found or they are not supported by the current version. return bufplugin.NopPluginKeyProvider, nil } bufLockFile, err := bufconfig.GetBufLockFileForPrefix( diff --git a/private/bufpkg/bufconfig/plugin_config.go b/private/bufpkg/bufconfig/plugin_config.go index 57472c9c43..0b402b28fc 100644 --- a/private/bufpkg/bufconfig/plugin_config.go +++ b/private/bufpkg/bufconfig/plugin_config.go @@ -41,16 +41,7 @@ type PluginConfigType int type PluginConfig interface { // Type returns the plugin type. This is never the zero value. Type() PluginConfigType - // Name returns the plugin name. - // - // If the plugin is of Type PluginConfigTypeLocal, the Name is the command - // name or path to the command. - // If the plugin is of Type PluginConfigTypeLocalWasm, the Name is the - // path to the Wasm file. It must end with .wasm. - // If the plugin is of Type PluginConfigTypeRemoteWasm, the Name is a - // vaild bufparse.Ref and Ref returns the plugin reference. - // - // This is never empty. + // Name returns the plugin name. This is never empty. Name() string // Options returns the plugin options. // diff --git a/private/bufpkg/bufplugin/plugin_key_provider.go b/private/bufpkg/bufplugin/plugin_key_provider.go index 967561d77e..71ff358e7a 100644 --- a/private/bufpkg/bufplugin/plugin_key_provider.go +++ b/private/bufpkg/bufplugin/plugin_key_provider.go @@ -51,18 +51,7 @@ type PluginKeyProvider interface { // When resolving Refs, the Ref will be matched to the PluginKey by FullName. // If the Ref is not found in the set of provided keys, an fs.ErrNotExist will be returned. func NewStaticPluginKeyProvider(pluginKeys []PluginKey) (PluginKeyProvider, error) { - if len(pluginKeys) == 0 { - return NopPluginKeyProvider, nil - } - pluginKeysByFullName, err := slicesext.ToUniqueValuesMap(pluginKeys, func(pluginKey PluginKey) string { - return pluginKey.FullName().String() - }) - if err != nil { - return nil, err - } - return staticPluginKeyProvider{ - pluginKeysByFullName: pluginKeysByFullName, - }, nil + return newStaticPluginKeyProvider(pluginKeys) } // *** PRIVATE *** @@ -81,6 +70,22 @@ type staticPluginKeyProvider struct { pluginKeysByFullName map[string]PluginKey } +func newStaticPluginKeyProvider(pluginKeys []PluginKey) (*staticPluginKeyProvider, error) { + var pluginKeysByFullName map[string]PluginKey + if len(pluginKeys) > 0 { + var err error + pluginKeysByFullName, err = slicesext.ToUniqueValuesMap(pluginKeys, func(pluginKey PluginKey) string { + return pluginKey.FullName().String() + }) + if err != nil { + return nil, err + } + } + return &staticPluginKeyProvider{ + pluginKeysByFullName: pluginKeysByFullName, + }, nil +} + func (s staticPluginKeyProvider) GetPluginKeysForPluginRefs( _ context.Context, refs []bufparse.Ref, From 528ba489c504d3511a16d6cf9c33deca60c732e7 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 18 Dec 2024 20:49:49 +0100 Subject: [PATCH 17/18] Refactor GetTargetImageWithConfigs --- private/buf/bufctl/controller.go | 253 +++++++----------- private/buf/buflsp/file.go | 2 +- .../buf/cmd/buf/command/breaking/breaking.go | 39 ++- .../buf/command/config/internal/internal.go | 11 +- private/buf/cmd/buf/command/lint/lint.go | 11 +- 5 files changed, 119 insertions(+), 197 deletions(-) diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index f6e30183cc..52dd43f2bf 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -91,11 +91,18 @@ type Controller interface { workspace bufworkspace.Workspace, options ...FunctionOption, ) (bufimage.Image, error) - GetTargetImageWithConfigs( + // GetTargetImageWithConfigsAndCheckClient gets the target ImageWithConfigs + // with a configured bufcheck Client. + // + // ImageWithConfig scopes the configuration per image for use with breaking + // and lint checks. The Client is bound to the input to ensure that the + // correct plugin dependencies are used. + GetTargetImageWithConfigsAndCheckClient( ctx context.Context, input string, + wasmRuntime wasm.Runtime, options ...FunctionOption, - ) ([]ImageWithConfig, error) + ) ([]ImageWithConfig, bufcheck.Client, error) // GetImportableImageFileInfos gets the importable .proto FileInfos for the given input. // // This includes all files that can be possible imported. For example, if a Module @@ -130,23 +137,11 @@ type Controller interface { defaultMessageEncoding buffetch.MessageEncoding, options ...FunctionOption, ) error - // NewCheckClient returns a new bufcheck.Client for the given input. - // - // Clients are bound to a specific input to ensure that the correct - // plugin dependencies are used. Where the input is resolvable to a - // Workspace, this function wraps NewCheckClientForWorkspace. For - // message inputs, the local directory or config override is used. - NewCheckClient( - ctx context.Context, - input string, - wasmRuntime wasm.Runtime, - options ...FunctionOption, - ) (bufcheck.Client, error) - // NewCheckClientForWorkspace returns a new bufcheck.Client for the given Workspace. + // GetCheckClientForWorkspace returns a new bufcheck Client for the given Workspace. // // Clients are bound to a specific Workspace to ensure that the correct // plugin dependencies are used. - NewCheckClientForWorkspace( + GetCheckClientForWorkspace( ctx context.Context, workspace bufworkspace.Workspace, wasmRuntime wasm.Runtime, @@ -357,11 +352,12 @@ func (c *controller) GetImageForWorkspace( return c.getImageForWorkspace(ctx, workspace, functionOptions) } -func (c *controller) GetTargetImageWithConfigs( +func (c *controller) GetTargetImageWithConfigsAndCheckClient( ctx context.Context, input string, + wasmRuntime wasm.Runtime, options ...FunctionOption, -) (_ []ImageWithConfig, retErr error) { +) (_ []ImageWithConfig, _ bufcheck.Client, retErr error) { defer c.handleFileAnnotationSetRetError(&retErr) functionOptions := newFunctionOptions(c) for _, option := range options { @@ -369,42 +365,41 @@ func (c *controller) GetTargetImageWithConfigs( } ref, err := c.buffetchRefParser.GetRef(ctx, input) if err != nil { - return nil, err + return nil, nil, err } + var workspace bufworkspace.Workspace switch t := ref.(type) { case buffetch.ProtoFileRef: - workspace, err := c.getWorkspaceForProtoFileRef(ctx, t, functionOptions) + workspace, err = c.getWorkspaceForProtoFileRef(ctx, t, functionOptions) if err != nil { - return nil, err + return nil, nil, err } - return c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) case buffetch.SourceRef: - workspace, err := c.getWorkspaceForSourceRef(ctx, t, functionOptions) + workspace, err = c.getWorkspaceForSourceRef(ctx, t, functionOptions) if err != nil { - return nil, err + return nil, nil, err } - return c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) case buffetch.ModuleRef: - workspace, err := c.getWorkspaceForModuleRef(ctx, t, functionOptions) + workspace, err = c.getWorkspaceForModuleRef(ctx, t, functionOptions) if err != nil { - return nil, err + return nil, nil, err } - return c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) case buffetch.MessageRef: image, err := c.getImageForMessageRef(ctx, t, functionOptions) if err != nil { - return nil, err + return nil, nil, err } bucket, err := c.storageosProvider.NewReadWriteBucket( ".", storageos.ReadWriteBucketWithSymlinksIfSupported(), ) if err != nil { - return nil, err + return nil, nil, err } lintConfig := bufconfig.DefaultLintConfigV1 breakingConfig := bufconfig.DefaultBreakingConfigV1 var pluginConfigs []bufconfig.PluginConfig + pluginKeyProvider := bufplugin.NopPluginKeyProvider bufYAMLFile, err := bufconfig.GetBufYAMLFileForPrefixOrOverride( ctx, bucket, @@ -413,16 +408,15 @@ func (c *controller) GetTargetImageWithConfigs( ) if err != nil { if !errors.Is(err, fs.ErrNotExist) { - return nil, err + return nil, nil, err } // We did not find a buf.yaml in our current directory, and there was no config override. // Use the defaults. } else { - pluginConfigs = bufYAMLFile.PluginConfigs() if topLevelLintConfig := bufYAMLFile.TopLevelLintConfig(); topLevelLintConfig == nil { // Ensure that this is a v2 config if fileVersion := bufYAMLFile.FileVersion(); fileVersion != bufconfig.FileVersionV2 { - return nil, syserror.Newf("non-v2 version with no top-level lint config: %s", fileVersion) + return nil, nil, syserror.Newf("non-v2 version with no top-level lint config: %s", fileVersion) } // v2 config without a top-level lint config, use v2 default lintConfig = bufconfig.DefaultLintConfigV2 @@ -431,26 +425,87 @@ func (c *controller) GetTargetImageWithConfigs( } if topLevelBreakingConfig := bufYAMLFile.TopLevelBreakingConfig(); topLevelBreakingConfig == nil { if fileVersion := bufYAMLFile.FileVersion(); fileVersion != bufconfig.FileVersionV2 { - return nil, syserror.Newf("non-v2 version with no top-level breaking config: %s", fileVersion) + return nil, nil, syserror.Newf("non-v2 version with no top-level breaking config: %s", fileVersion) } // v2 config without a top-level breaking config, use v2 default breakingConfig = bufconfig.DefaultBreakingConfigV2 } else { breakingConfig = topLevelBreakingConfig } + // The directory path is resolved to a buf.yaml file and a buf.lock file. If the + // buf.yaml file is found, the PluginConfigs from the buf.yaml file and the PluginKeys + // from the buf.lock file are resolved to create the PluginKeyProvider. + pluginConfigs = bufYAMLFile.PluginConfigs() + // If a config override is provided, the PluginConfig remote Refs use the BSR + // to resolve the PluginKeys. No buf.lock is required. + // If the buf.yaml file is not found, the bufplugin.NopPluginKeyProvider is returned. + // If the buf.lock file is not found, the bufplugin.NopPluginKeyProvider is returned. + if functionOptions.configOverride != "" { + // To support remote plugins in the override, we need to resolve the remote + // Refs to PluginKeys. A buf.lock file is not required for this operation. + // We use the BSR to resolve any remote plugin Refs. + pluginKeyProvider = c.pluginKeyProvider + } else if bufYAMLFile.FileVersion() == bufconfig.FileVersionV2 { + var pluginKeys []bufplugin.PluginKey + if bufLockFile, err := bufconfig.GetBufLockFileForPrefix( + ctx, + bucket, + // buf.lock files live next to the buf.yaml + ".", + ); err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return nil, nil, err + } + // We did not find a buf.lock in our current directory. + // Remote plugins are not available. + pluginKeys = nil + } else { + pluginKeys = bufLockFile.RemotePluginKeys() + } + pluginKeyProvider, err = newStaticPluginKeyProviderForPluginConfigs( + pluginConfigs, + pluginKeys, + ) + if err != nil { + return nil, nil, err + } + } } - return []ImageWithConfig{ + imageWithConfigs := []ImageWithConfig{ newImageWithConfig( image, lintConfig, breakingConfig, pluginConfigs, ), - }, nil + } + pluginRunnerProvider := bufcheck.NewLocalRunnerProvider( + wasmRuntime, + pluginKeyProvider, + c.pluginDataProvider, + ) + checkClient, err := bufcheck.NewClient( + c.logger, + pluginRunnerProvider, + bufcheck.ClientWithStderr(c.container.Stderr()), + ) + if err != nil { + return nil, nil, err + } + return imageWithConfigs, checkClient, nil default: // This is a system error. - return nil, syserror.Newf("invalid Ref: %T", ref) + return nil, nil, syserror.Newf("invalid Ref: %T", ref) + } + targetImageWithConfigs, err := c.buildTargetImageWithConfigs(ctx, workspace, functionOptions) + if err != nil { + return nil, nil, err } + checkClient, err := c.GetCheckClientForWorkspace(ctx, workspace, wasmRuntime) + if err != nil { + return nil, nil, err + } + return targetImageWithConfigs, checkClient, err } func (c *controller) GetImportableImageFileInfos( @@ -730,65 +785,7 @@ func (c *controller) PutMessage( return errors.Join(err, writeCloser.Close()) } -func (c *controller) NewCheckClient( - ctx context.Context, - input string, - wasmRuntime wasm.Runtime, - options ...FunctionOption, -) (_ bufcheck.Client, retErr error) { - defer c.handleFileAnnotationSetRetError(&retErr) - functionOptions := newFunctionOptions(c) - for _, option := range options { - option(functionOptions) - } - ref, err := c.buffetchRefParser.GetRef(ctx, input) - if err != nil { - return nil, err - } - switch t := ref.(type) { - case buffetch.ProtoFileRef: - workspace, err := c.getWorkspaceForProtoFileRef(ctx, t, functionOptions) - if err != nil { - return nil, err - } - return c.NewCheckClientForWorkspace(ctx, workspace, wasmRuntime) - case buffetch.SourceRef: - workspace, err := c.getWorkspaceForSourceRef(ctx, t, functionOptions) - if err != nil { - return nil, err - } - return c.NewCheckClientForWorkspace(ctx, workspace, wasmRuntime) - case buffetch.ModuleRef: - workspace, err := c.getWorkspaceForModuleRef(ctx, t, functionOptions) - if err != nil { - return nil, err - } - return c.NewCheckClientForWorkspace(ctx, workspace, wasmRuntime) - case buffetch.MessageRef: - // MessageRefs do not resolve to a Workspace, so we resolve the - // PluginKeyProvider by the buf.yaml provided in the current directory. - // This follows the logic in GetTargetImageWithConfigs. - pluginKeyProvider, err := c.getPluginKeyProviderForDirPath(ctx, ".", functionOptions) - if err != nil { - return nil, err - } - pluginRunnerProvider := bufcheck.NewLocalRunnerProvider( - wasmRuntime, - pluginKeyProvider, - c.pluginDataProvider, - ) - return bufcheck.NewClient( - c.logger, - pluginRunnerProvider, - bufcheck.ClientWithStderr(c.container.Stderr()), - ) - default: - // This is a system error. - return nil, syserror.Newf("invalid Ref: %T", ref) - } -} - -func (c *controller) NewCheckClientForWorkspace( +func (c *controller) GetCheckClientForWorkspace( ctx context.Context, workspace bufworkspace.Workspace, wasmRuntime wasm.Runtime, @@ -1291,70 +1288,6 @@ func (c *controller) handleFileAnnotationSetRetError(retErrAddr *error) { } } -// getPluginKeyProviderForDirPath creates a new PluginKeyProvider for the directory path. -// -// The directory path is resolved to a buf.yaml file and a buf.lock file. If the -// buf.yaml file is found, the PluginConfigs from the buf.yaml file and the PluginKeys -// from the buf.lock file are resolved to create the PluginKeyProvider. -// -// If a config override is provided, the PluginConfig remote Refs use the BSR -// to resolve the PluginKeys. No buf.lock is required. -// If the buf.yaml file is not found, the bufplugin.NopPluginKeyProvider is returned. -// If the buf.lock file is not found, the bufplugin.NopPluginKeyProvider is returned. -func (c *controller) getPluginKeyProviderForDirPath( - ctx context.Context, - dirPath string, - functionOptions *functionOptions, -) (_ bufplugin.PluginKeyProvider, retErr error) { - // If there is a config override, we do not use the buf.lock file. - // Remote plugins are available, use the BSR to resolve any plugin Refs. - if functionOptions.configOverride != "" { - return c.pluginKeyProvider, nil - } - var ( - pluginConfigs []bufconfig.PluginConfig - pluginKeys []bufplugin.PluginKey - ) - bucket, err := c.storageosProvider.NewReadWriteBucket( - dirPath, - storageos.ReadWriteBucketWithSymlinksIfSupported(), - ) - if err != nil { - return nil, err - } - bufYAMLFile, err := bufconfig.GetBufYAMLFileForPrefix(ctx, bucket, ".") - if err != nil { - if !errors.Is(err, fs.ErrNotExist) { - return nil, err - } - // We did not find a buf.yaml in our current directory, - // and there was no config override. - // Remote plugins are not available. - return bufplugin.NopPluginKeyProvider, nil - } - pluginConfigs = bufYAMLFile.PluginConfigs() - if len(pluginConfigs) == 0 || bufYAMLFile.FileVersion() != bufconfig.FileVersionV2 { - // No plugins were found or they are not supported by the current version. - return bufplugin.NopPluginKeyProvider, nil - } - bufLockFile, err := bufconfig.GetBufLockFileForPrefix( - ctx, - bucket, - // buf.lock files live next to the buf.yaml - ".", - ) - if err != nil { - if !errors.Is(err, fs.ErrNotExist) { - return nil, err - } - // We did not find a buf.lock in our current directory. - // Remote plugins are not available. - return bufplugin.NopPluginKeyProvider, nil - } - pluginKeys = bufLockFile.RemotePluginKeys() - return newStaticPluginKeyProviderForPluginConfigs(pluginConfigs, pluginKeys) -} - func getImageFileInfosForModuleSet(ctx context.Context, moduleSet bufmodule.ModuleSet) ([]bufimage.ImageFileInfo, error) { // Sorted. fileInfos, err := bufmodule.GetFileInfos( diff --git a/private/buf/buflsp/file.go b/private/buf/buflsp/file.go index 685370361c..d5e9288adb 100644 --- a/private/buf/buflsp/file.go +++ b/private/buf/buflsp/file.go @@ -396,7 +396,7 @@ func (f *file) FindModule(ctx context.Context) { } // Get the check client for this workspace. - checkClient, err := f.lsp.controller.NewCheckClientForWorkspace(ctx, workspace, f.lsp.wasmRuntime) + checkClient, err := f.lsp.controller.GetCheckClientForWorkspace(ctx, workspace, f.lsp.wasmRuntime) if err != nil { f.lsp.logger.Warn("could not get check client", slogext.ErrorAttr(err)) return diff --git a/private/buf/cmd/buf/command/breaking/breaking.go b/private/buf/cmd/buf/command/breaking/breaking.go index f1fbc79eb7..1ddbf6359e 100644 --- a/private/buf/cmd/buf/command/breaking/breaking.go +++ b/private/buf/cmd/buf/command/breaking/breaking.go @@ -161,11 +161,23 @@ func run( if err != nil { return err } + wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) + if err != nil { + return err + } + wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(wasmRuntimeCacheDir)) + if err != nil { + return err + } + defer func() { + retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) + }() // Do not exclude imports here. bufcheck's Client requires all imports. // Use bufcheck's BreakingWithExcludeImports. - imageWithConfigs, err := controller.GetTargetImageWithConfigs( + imageWithConfigs, checkClient, err := controller.GetTargetImageWithConfigsAndCheckClient( ctx, input, + wasmRuntime, bufctl.WithTargetPaths(flags.Paths, flags.ExcludePaths), bufctl.WithConfigOverride(flags.Config), ) @@ -183,9 +195,10 @@ func run( } // Do not exclude imports here. bufcheck's Client requires all imports. // Use bufcheck's BreakingWithExcludeImports. - againstImageWithConfigs, err := controller.GetTargetImageWithConfigs( + againstImageWithConfigs, _, err := controller.GetTargetImageWithConfigsAndCheckClient( ctx, flags.Against, + wasm.UnimplementedRuntime, bufctl.WithTargetPaths(externalPaths, flags.ExcludePaths), bufctl.WithConfigOverride(flags.AgainstConfig), ) @@ -205,28 +218,6 @@ func run( len(againstImageWithConfigs), ) } - wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) - if err != nil { - return err - } - wasmRuntime, err := wasm.NewRuntime(ctx, wasm.WithLocalCacheDir(wasmRuntimeCacheDir)) - if err != nil { - return err - } - defer func() { - retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) - }() - // Get the check client for the input images. - checkClient, err := controller.NewCheckClient( - ctx, - input, - wasmRuntime, - bufctl.WithTargetPaths(flags.Paths, flags.ExcludePaths), - bufctl.WithConfigOverride(flags.Config), - ) - if err != nil { - return err - } var allFileAnnotations []bufanalysis.FileAnnotation for i, imageWithConfig := range imageWithConfigs { breakingOptions := []bufcheck.BreakingOption{ diff --git a/private/buf/cmd/buf/command/config/internal/internal.go b/private/buf/cmd/buf/command/config/internal/internal.go index c84dfba6c2..66fb68c483 100644 --- a/private/buf/cmd/buf/command/config/internal/internal.go +++ b/private/buf/cmd/buf/command/config/internal/internal.go @@ -198,11 +198,18 @@ func lsRun( if err != nil { return err } - checkClient, err := controller.NewCheckClient( + workspace, err := controller.GetWorkspace( ctx, ".", + bufctl.WithConfigOverride(configOverride), + ) + if err != nil { + return err + } + checkClient, err := controller.GetCheckClientForWorkspace( + ctx, + workspace, wasmRuntime, - bufctl.WithConfigOverride(flags.Config), ) if err != nil { return err diff --git a/private/buf/cmd/buf/command/lint/lint.go b/private/buf/cmd/buf/command/lint/lint.go index 9ff5162acb..98b8c6debf 100644 --- a/private/buf/cmd/buf/command/lint/lint.go +++ b/private/buf/cmd/buf/command/lint/lint.go @@ -121,15 +121,6 @@ func run( if err != nil { return err } - imageWithConfigs, err := controller.GetTargetImageWithConfigs( - ctx, - input, - bufctl.WithTargetPaths(flags.Paths, flags.ExcludePaths), - bufctl.WithConfigOverride(flags.Config), - ) - if err != nil { - return err - } wasmRuntimeCacheDir, err := bufcli.CreateWasmRuntimeCacheDir(container) if err != nil { return err @@ -141,7 +132,7 @@ func run( defer func() { retErr = errors.Join(retErr, wasmRuntime.Close(ctx)) }() - checkClient, err := controller.NewCheckClient( + imageWithConfigs, checkClient, err := controller.GetTargetImageWithConfigsAndCheckClient( ctx, input, wasmRuntime, From a6faf86ca4d4d4ce63c591698f4b5e5a5e5766a7 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 18 Dec 2024 20:53:43 +0100 Subject: [PATCH 18/18] Docs --- private/buf/bufctl/controller.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go index 52dd43f2bf..a287a7c88d 100644 --- a/private/buf/bufctl/controller.go +++ b/private/buf/bufctl/controller.go @@ -95,8 +95,9 @@ type Controller interface { // with a configured bufcheck Client. // // ImageWithConfig scopes the configuration per image for use with breaking - // and lint checks. The Client is bound to the input to ensure that the - // correct plugin dependencies are used. + // and lint checks. The check Client is bound to the input to ensure that the + // correct remote plugin dependencies are used. A wasmRuntime is provided + // to evaluate Wasm plugins. GetTargetImageWithConfigsAndCheckClient( ctx context.Context, input string, @@ -140,7 +141,8 @@ type Controller interface { // GetCheckClientForWorkspace returns a new bufcheck Client for the given Workspace. // // Clients are bound to a specific Workspace to ensure that the correct - // plugin dependencies are used. + // plugin dependencies are used. A wasmRuntime is provided to evaluate + // Wasm plugins. GetCheckClientForWorkspace( ctx context.Context, workspace bufworkspace.Workspace,