Skip to content

Commit 139f92e

Browse files
doriabledoriable
authored
Add support for plugin path overrides in protoc-gen-buf-{lint,breaking} (#3871)
1 parent 0e6bbb7 commit 139f92e

File tree

3 files changed

+122
-42
lines changed

3 files changed

+122
-42
lines changed

‎private/buf/cmd/internal/internal.go‎

Lines changed: 77 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"errors"
2020
"fmt"
2121
"io/fs"
22+
"path/filepath"
2223

2324
"buf.build/go/app"
2425
"buf.build/go/app/appext"
@@ -28,52 +29,68 @@ import (
2829
"github.com/bufbuild/protoplugin"
2930
)
3031

31-
// GetModuleConfigForProtocPlugin gets ModuleConfigs for the protoc plugin implementations.
32+
// GetModuleConfigAndPluginConfigsForProtocPlugin gets the [bufmodule.ModuleConfig] and
33+
// [bufmodule.PluginConfig]s for the specified module for the protoc plugin implementations.
34+
//
35+
// The caller can provide overrides for plugin paths in the plugin configurations. The protoc
36+
// plugin implementations do not support remote plugins. Also, for use-cases such as Bazel,
37+
// access to local binaries might require an explicit path override. So, this allows callers
38+
// to pass a map of plugin name to local path to override the plugin configuration.
39+
//
40+
// We also return all check configs for the option [bufcheck.WithRelatedCheckConfigs] to
41+
// validate the plugin configs when running lint/breaking.
3242
//
3343
// This is the same in both plugins so we just pulled it out to a common spot.
34-
func GetModuleConfigForProtocPlugin(
44+
func GetModuleConfigAndPluginConfigsForProtocPlugin(
3545
ctx context.Context,
3646
configOverride string,
3747
module string,
38-
) (bufconfig.ModuleConfig, error) {
48+
pluginPathOverrides map[string]string,
49+
) (bufconfig.ModuleConfig, []bufconfig.PluginConfig, []bufconfig.CheckConfig, error) {
3950
bufYAMLFile, err := bufcli.GetBufYAMLFileForDirPathOrOverride(
4051
ctx,
4152
".",
4253
configOverride,
4354
)
4455
if err != nil {
4556
if errors.Is(err, fs.ErrNotExist) {
46-
return bufconfig.DefaultModuleConfigV1, nil
57+
// There are no plugin configs by default.
58+
return bufconfig.DefaultModuleConfigV2, nil, []bufconfig.CheckConfig{bufconfig.DefaultLintConfigV2, bufconfig.DefaultBreakingConfigV2}, nil
4759
}
48-
return nil, err
60+
return nil, nil, nil, err
4961
}
5062
if module == "" {
5163
module = "."
5264
}
65+
pluginConfigs, err := getPluginConfigsForPluginPathOverrides(bufYAMLFile.PluginConfigs(), pluginPathOverrides)
66+
if err != nil {
67+
return nil, nil, nil, err
68+
}
69+
var allCheckConfigs []bufconfig.CheckConfig
5370
// Multiple modules in a v2 workspace may have the same moduleDirPath.
5471
moduleConfigsFound := []bufconfig.ModuleConfig{}
5572
for _, moduleConfig := range bufYAMLFile.ModuleConfigs() {
73+
allCheckConfigs = append(allCheckConfigs, moduleConfig.LintConfig(), moduleConfig.BreakingConfig())
5674
// If we have a v1beta1 or v1 buf.yaml, dirPath will be ".". Using the ModuleConfig from
5775
// a v1beta1 or v1 buf.yaml file matches the pre-refactor behavior.
5876
//
5977
// If we have a v2 buf.yaml, users have to provide a module path or full name, otherwise
6078
// we can't deduce what ModuleConfig to use.
6179
if fullName := moduleConfig.FullName(); fullName != nil && fullName.String() == module {
62-
// Can return here because BufYAMLFile guarantees that module full names are unique across
63-
// its module configs.
64-
return moduleConfig, nil
80+
moduleConfigsFound = append(moduleConfigsFound, moduleConfig)
81+
continue
6582
}
6683
if dirPath := moduleConfig.DirPath(); dirPath == module {
6784
moduleConfigsFound = append(moduleConfigsFound, moduleConfig)
6885
}
6986
}
7087
switch len(moduleConfigsFound) {
7188
case 0:
72-
return nil, fmt.Errorf("no module found for %q", module)
89+
return nil, nil, nil, fmt.Errorf("no module found for %q", module)
7390
case 1:
74-
return moduleConfigsFound[0], nil
91+
return moduleConfigsFound[0], pluginConfigs, allCheckConfigs, nil
7592
default:
76-
return nil, fmt.Errorf("multiple modules found at %q, specify its full name as <remote/owner/module> instead", module)
93+
return nil, nil, nil, fmt.Errorf("multiple modules found at %q, specify its full name as <remote/owner/module> instead", module)
7794
}
7895
}
7996

@@ -136,3 +153,52 @@ func newAppContainerForPluginEnv(pluginEnv protoplugin.PluginEnv) (*appContainer
136153
ArgContainer: app.NewArgContainer(),
137154
}, nil
138155
}
156+
157+
// This processes the plugin path overrides for the protoc plugin implementations. It does
158+
// the following:
159+
// - For each plugin config, it checks if a path override was configured
160+
// - If an override was found, then a new config override is created based on whether the
161+
// override is a WASM path.
162+
// - If no override was found, if the plugin config is a remote plugin, we return an error,
163+
// since remote plugins are not supported for protoc plugin implementations. Otherwise,
164+
// we use the plugin config as-is.
165+
func getPluginConfigsForPluginPathOverrides(
166+
pluginConfigs []bufconfig.PluginConfig,
167+
pluginPathOverrides map[string]string,
168+
) ([]bufconfig.PluginConfig, error) {
169+
overridePluginConfigs := make([]bufconfig.PluginConfig, len(pluginConfigs))
170+
for i, pluginConfig := range pluginConfigs {
171+
if overridePath, ok := pluginPathOverrides[pluginConfig.Name()]; ok {
172+
var overridePluginConfig bufconfig.PluginConfig
173+
var err error
174+
// Check if the override path is a WASM path, if so, treat as a local WASM plugin
175+
if filepath.Ext(overridePath) == ".wasm" {
176+
overridePluginConfig, err = bufconfig.NewLocalWasmPluginConfig(
177+
overridePath,
178+
pluginConfig.Options(),
179+
pluginConfig.Args(),
180+
)
181+
if err != nil {
182+
return nil, err
183+
}
184+
} else {
185+
// Otherwise, treat it as a non-WASM local plugin.
186+
overridePluginConfig, err = bufconfig.NewLocalPluginConfig(
187+
overridePath,
188+
pluginConfig.Options(),
189+
pluginConfig.Args(),
190+
)
191+
if err != nil {
192+
return nil, err
193+
}
194+
}
195+
overridePluginConfigs[i] = overridePluginConfig
196+
continue
197+
}
198+
if pluginConfig.Type() == bufconfig.PluginConfigTypeRemoteWasm {
199+
return nil, fmt.Errorf("remote plugin %s cannot be run with protoc plugin", pluginConfig.Name())
200+
}
201+
overridePluginConfigs[i] = pluginConfig
202+
}
203+
return overridePluginConfigs, nil
204+
}

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

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,6 @@ func handle(
109109
if externalConfig.ExcludeImports {
110110
breakingOptions = append(breakingOptions, bufcheck.BreakingWithExcludeImports())
111111
}
112-
moduleConfig, err := internal.GetModuleConfigForProtocPlugin(
113-
ctx,
114-
encoding.GetJSONStringOrStringValue(externalConfig.InputConfig),
115-
externalConfig.Module,
116-
)
117-
if err != nil {
118-
return err
119-
}
120112
image, err := bufimage.NewImageForCodeGeneratorRequest(request.CodeGeneratorRequest())
121113
if err != nil {
122114
return err
@@ -140,6 +132,21 @@ func handle(
140132
if err != nil {
141133
return err
142134
}
135+
moduleConfig, pluginConfigs, allCheckConfigs, err := internal.GetModuleConfigAndPluginConfigsForProtocPlugin(
136+
ctx,
137+
encoding.GetJSONStringOrStringValue(externalConfig.InputConfig),
138+
externalConfig.Module,
139+
externalConfig.PluginOverrides,
140+
)
141+
if err != nil {
142+
return err
143+
}
144+
if len(pluginConfigs) > 0 {
145+
breakingOptions = append(breakingOptions, bufcheck.WithPluginConfigs(pluginConfigs...))
146+
// We add all check configs (both lint and breaking) across all configured modules in buf.yaml
147+
// as related configs to check if plugins have rules configured.
148+
breakingOptions = append(breakingOptions, bufcheck.WithRelatedCheckConfigs(allCheckConfigs...))
149+
}
143150
if err := client.Breaking(
144151
ctx,
145152
moduleConfig.BreakingConfig(),
@@ -168,13 +175,14 @@ func handle(
168175
type externalConfig struct {
169176
AgainstInput string `json:"against_input,omitempty" yaml:"against_input,omitempty"`
170177
// This was never actually used, but we keep it around for we can do unmarshal strict without breaking anyone.
171-
AgainstInputConfig json.RawMessage `json:"against_input_config,omitempty" yaml:"against_input_config,omitempty"`
172-
InputConfig json.RawMessage `json:"input_config,omitempty" yaml:"input_config,omitempty"`
173-
Module string `json:"module,omitempty" yaml:"module,omitempty"`
174-
LimitToInputFiles bool `json:"limit_to_input_files,omitempty" yaml:"limit_to_input_files,omitempty"`
175-
ExcludeImports bool `json:"exclude_imports,omitempty" yaml:"exclude_imports,omitempty"`
176-
LogLevel string `json:"log_level,omitempty" yaml:"log_level,omitempty"`
177-
LogFormat string `json:"log_format,omitempty" yaml:"log_format,omitempty"`
178-
ErrorFormat string `json:"error_format,omitempty" yaml:"error_format,omitempty"`
179-
Timeout time.Duration `json:"timeout,omitempty" yaml:"timeout,omitempty"`
178+
AgainstInputConfig json.RawMessage `json:"against_input_config,omitempty" yaml:"against_input_config,omitempty"`
179+
InputConfig json.RawMessage `json:"input_config,omitempty" yaml:"input_config,omitempty"`
180+
Module string `json:"module,omitempty" yaml:"module,omitempty"`
181+
LimitToInputFiles bool `json:"limit_to_input_files,omitempty" yaml:"limit_to_input_files,omitempty"`
182+
ExcludeImports bool `json:"exclude_imports,omitempty" yaml:"exclude_imports,omitempty"`
183+
LogLevel string `json:"log_level,omitempty" yaml:"log_level,omitempty"`
184+
LogFormat string `json:"log_format,omitempty" yaml:"log_format,omitempty"`
185+
ErrorFormat string `json:"error_format,omitempty" yaml:"error_format,omitempty"`
186+
Timeout time.Duration `json:"timeout,omitempty" yaml:"timeout,omitempty"`
187+
PluginOverrides map[string]string `json:"plugin_overrides,omitempty" yaml:"plugin_overrides,omitempty"`
180188
}

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

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,6 @@ func handle(
8080
}
8181
ctx, cancel := context.WithTimeout(ctx, timeout)
8282
defer cancel()
83-
moduleConfig, err := internal.GetModuleConfigForProtocPlugin(
84-
ctx,
85-
encoding.GetJSONStringOrStringValue(externalConfig.InputConfig),
86-
externalConfig.Module,
87-
)
88-
if err != nil {
89-
return err
90-
}
9183
// With the "buf lint" command, we build the image and then the linter can report
9284
// unused imports that the compiler reports. But with a plugin, we get descriptors
9385
// that are already built and no access to any possible associated compiler warnings.
@@ -115,10 +107,23 @@ func handle(
115107
if err != nil {
116108
return err
117109
}
110+
moduleConfig, pluginConfigs, allCheckConfigs, err := internal.GetModuleConfigAndPluginConfigsForProtocPlugin(
111+
ctx,
112+
encoding.GetJSONStringOrStringValue(externalConfig.InputConfig),
113+
externalConfig.Module,
114+
externalConfig.PluginOverrides,
115+
)
116+
if err != nil {
117+
return err
118+
}
118119
if err := client.Lint(
119120
ctx,
120121
moduleConfig.LintConfig(),
121122
image,
123+
bufcheck.WithPluginConfigs(pluginConfigs...),
124+
// We add all check configs (both lint and breaking) across all configured modules in buf.yaml
125+
// as related configs to check if plugins have rules configured.
126+
bufcheck.WithRelatedCheckConfigs(allCheckConfigs...),
122127
); err != nil {
123128
var fileAnnotationSet bufanalysis.FileAnnotationSet
124129
if errors.As(err, &fileAnnotationSet) {
@@ -148,10 +153,11 @@ func handle(
148153
}
149154

150155
type externalConfig struct {
151-
InputConfig json.RawMessage `json:"input_config,omitempty" yaml:"input_config,omitempty"`
152-
Module string `json:"module,omitempty" yaml:"module,omitempty"`
153-
LogLevel string `json:"log_level,omitempty" yaml:"log_level,omitempty"`
154-
LogFormat string `json:"log_format,omitempty" yaml:"log_format,omitempty"`
155-
ErrorFormat string `json:"error_format,omitempty" yaml:"error_format,omitempty"`
156-
Timeout time.Duration `json:"timeout,omitempty" yaml:"timeout,omitempty"`
156+
InputConfig json.RawMessage `json:"input_config,omitempty" yaml:"input_config,omitempty"`
157+
Module string `json:"module,omitempty" yaml:"module,omitempty"`
158+
LogLevel string `json:"log_level,omitempty" yaml:"log_level,omitempty"`
159+
LogFormat string `json:"log_format,omitempty" yaml:"log_format,omitempty"`
160+
ErrorFormat string `json:"error_format,omitempty" yaml:"error_format,omitempty"`
161+
Timeout time.Duration `json:"timeout,omitempty" yaml:"timeout,omitempty"`
162+
PluginOverrides map[string]string `json:"plugin_overrides,omitempty" yaml:"plugin_overrides,omitempty"`
157163
}

0 commit comments

Comments
 (0)