Skip to content

Commit 1ce3aac

Browse files
authored
Add --against-registry flag for buf breaking (#3696)
1 parent 8d1bdff commit 1ce3aac

File tree

5 files changed

+134
-17
lines changed

5 files changed

+134
-17
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
- Fix `buf convert` to allow for zero length for `binpb`, `txtpb`, and `yaml` formats.
66
- Fix use of deprecated flag `--include-types` for `buf generate`.
7+
- Add `--against-registry` flag to `buf breaking` that runs breaking checks against the latest
8+
commit on the default branch of the corresponding module in the registry.
79

810
## [v1.50.1] - 2025-03-10
911

private/buf/bufctl/controller.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,13 @@ import (
5454
"google.golang.org/protobuf/proto"
5555
)
5656

57-
// ImageWithConfig pairs an Image with lint and breaking configuration.
57+
// ImageWithConfig pairs an Image with its corresponding [bufmodule.Module] full name
58+
// (which may be nil), [bufmodule.Module] opaque ID, and lint and breaking configurations.
5859
type ImageWithConfig interface {
5960
bufimage.Image
6061

62+
ModuleFullName() bufparse.FullName
63+
ModuleOpaqueID() string
6164
LintConfig() bufconfig.LintConfig
6265
BreakingConfig() bufconfig.BreakingConfig
6366
PluginConfigs() []bufconfig.PluginConfig
@@ -476,6 +479,8 @@ func (c *controller) GetTargetImageWithConfigsAndCheckClient(
476479
imageWithConfigs := []ImageWithConfig{
477480
newImageWithConfig(
478481
image,
482+
nil, // No module name for a single message ref
483+
"", // No module opaque ID for a single message ref
479484
lintConfig,
480485
breakingConfig,
481486
pluginConfigs,
@@ -1170,6 +1175,8 @@ func (c *controller) buildTargetImageWithConfigs(
11701175
imageWithConfigs,
11711176
newImageWithConfig(
11721177
image,
1178+
module.FullName(),
1179+
module.OpaqueID(),
11731180
workspace.GetLintConfigForOpaqueID(module.OpaqueID()),
11741181
workspace.GetBreakingConfigForOpaqueID(module.OpaqueID()),
11751182
workspace.PluginConfigs(),

private/buf/bufctl/image_with_config.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,30 +17,45 @@ package bufctl
1717
import (
1818
"github.com/bufbuild/buf/private/bufpkg/bufconfig"
1919
"github.com/bufbuild/buf/private/bufpkg/bufimage"
20+
"github.com/bufbuild/buf/private/bufpkg/bufparse"
2021
)
2122

2223
type imageWithConfig struct {
2324
bufimage.Image
2425

26+
moduleFullName bufparse.FullName
27+
moduleOpaqueID string
2528
lintConfig bufconfig.LintConfig
2629
breakingConfig bufconfig.BreakingConfig
2730
pluginConfigs []bufconfig.PluginConfig
2831
}
2932

3033
func newImageWithConfig(
3134
image bufimage.Image,
35+
moduleFullName bufparse.FullName,
36+
moduleOpaqueID string,
3237
lintConfig bufconfig.LintConfig,
3338
breakingConfig bufconfig.BreakingConfig,
3439
pluginConfigs []bufconfig.PluginConfig,
3540
) *imageWithConfig {
3641
return &imageWithConfig{
3742
Image: image,
43+
moduleFullName: moduleFullName,
44+
moduleOpaqueID: moduleOpaqueID,
3845
lintConfig: lintConfig,
3946
breakingConfig: breakingConfig,
4047
pluginConfigs: pluginConfigs,
4148
}
4249
}
4350

51+
func (i *imageWithConfig) ModuleFullName() bufparse.FullName {
52+
return i.moduleFullName
53+
}
54+
55+
func (i *imageWithConfig) ModuleOpaqueID() string {
56+
return i.moduleOpaqueID
57+
}
58+
4459
func (i *imageWithConfig) LintConfig() bufconfig.LintConfig {
4560
return i.lintConfig
4661
}

private/buf/cmd/buf/buf_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3558,6 +3558,30 @@ testdata/check_plugins/current/proto/common/v1/breaking.proto:18:1:Enum "common.
35583558
)
35593559
}
35603560

3561+
func TestBreakingAgainstRegistryFlag(t *testing.T) {
3562+
t.Parallel()
3563+
testRunStderr(
3564+
t,
3565+
nil,
3566+
1,
3567+
"Failure: Cannot set both --against and --against-registry",
3568+
"breaking",
3569+
filepath.Join("testdata", "check_plugins", "current", "proto"),
3570+
"--against",
3571+
filepath.Join("testdata", "check_plugins", "previous", "proto"),
3572+
"--against-registry",
3573+
)
3574+
testRunStderr(
3575+
t,
3576+
nil,
3577+
1,
3578+
"Failure: cannot use --against-registry with unnamed module, testdata/success",
3579+
"breaking",
3580+
filepath.Join("testdata", "success"),
3581+
"--against-registry",
3582+
)
3583+
}
3584+
35613585
func TestVersion(t *testing.T) {
35623586
t.Parallel()
35633587
testRunStdout(t, nil, 0, bufcli.Version, "--version")

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

Lines changed: 85 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/bufbuild/buf/private/pkg/app/appext"
3131
"github.com/bufbuild/buf/private/pkg/slicesext"
3232
"github.com/bufbuild/buf/private/pkg/stringutil"
33+
"github.com/bufbuild/buf/private/pkg/syserror"
3334
"github.com/bufbuild/buf/private/pkg/wasm"
3435
"github.com/spf13/pflag"
3536
)
@@ -42,6 +43,7 @@ const (
4243
configFlagName = "config"
4344
againstFlagName = "against"
4445
againstConfigFlagName = "against-config"
46+
againstRegistryFlagName = "against-registry"
4547
excludePathsFlagName = "exclude-path"
4648
disableSymlinksFlagName = "disable-symlinks"
4749
)
@@ -77,6 +79,7 @@ type flags struct {
7779
Config string
7880
Against string
7981
AgainstConfig string
82+
AgainstRegistry bool
8083
ExcludePaths []string
8184
DisableSymlinks bool
8285
// special
@@ -129,7 +132,8 @@ Overrides --%s`,
129132
againstFlagName,
130133
"",
131134
fmt.Sprintf(
132-
`Required. The source, module, or image to check against. Must be one of format %s`,
135+
`Required, except if --%s is set. The source, module, or image to check against. Must be one of format %s`,
136+
againstRegistryFlagName,
133137
buffetch.AllFormatsString,
134138
),
135139
)
@@ -139,14 +143,24 @@ Overrides --%s`,
139143
"",
140144
`The buf.yaml file or data to use to configure the against source, module, or image`,
141145
)
146+
flagSet.BoolVar(
147+
&f.AgainstRegistry,
148+
againstRegistryFlagName,
149+
false,
150+
fmt.Sprintf(
151+
`Run breaking checks against the latest commit on the default branch in the registry. All modules in the input must have a name configured, otherwise this will fail.
152+
If a remote module is not found with the configured name, then this will fail. This cannot be set with --%s.`,
153+
againstFlagName,
154+
),
155+
)
142156
}
143157

144158
func run(
145159
ctx context.Context,
146160
container appext.Container,
147161
flags *flags,
148162
) (retErr error) {
149-
if err := bufcli.ValidateRequiredFlag(againstFlagName, flags.Against); err != nil {
163+
if err := validateFlags(flags); err != nil {
150164
return err
151165
}
152166
input, err := bufcli.GetInputValue(container, flags.InputHashtag, ".")
@@ -194,19 +208,64 @@ func run(
194208
return err
195209
}
196210
}
197-
// Do not exclude imports here. bufcheck's Client requires all imports.
198-
// Use bufcheck's BreakingWithExcludeImports.
199-
againstImageWithConfigs, _, err := controller.GetTargetImageWithConfigsAndCheckClient(
200-
ctx,
201-
flags.Against,
202-
wasm.UnimplementedRuntime,
203-
bufctl.WithTargetPaths(externalPaths, flags.ExcludePaths),
204-
bufctl.WithConfigOverride(flags.AgainstConfig),
205-
)
206-
if err != nil {
207-
return err
211+
var againstImages []bufimage.Image
212+
if flags.Against != "" {
213+
// Do not exclude imports here. bufcheck's Client requires all imports.
214+
// Use bufcheck's BreakingWithExcludeImports.
215+
againstImagesWithConfigs, _, err := controller.GetTargetImageWithConfigsAndCheckClient(
216+
ctx,
217+
flags.Against,
218+
wasm.UnimplementedRuntime,
219+
bufctl.WithTargetPaths(externalPaths, flags.ExcludePaths),
220+
bufctl.WithConfigOverride(flags.AgainstConfig),
221+
)
222+
if err != nil {
223+
return err
224+
}
225+
// We do not require the check configs from the against target once built, so they can
226+
// be dropped here.
227+
againstImages, err = slicesext.MapError(
228+
againstImagesWithConfigs,
229+
func(imageWithConfig bufctl.ImageWithConfig) (bufimage.Image, error) {
230+
againstImage, ok := imageWithConfig.(bufimage.Image)
231+
if !ok {
232+
return nil, syserror.New("imageWithConfig could not be converted to Image")
233+
}
234+
return againstImage, nil
235+
},
236+
)
237+
if err != nil {
238+
return err
239+
}
208240
}
209-
if len(imageWithConfigs) != len(againstImageWithConfigs) {
241+
if flags.AgainstRegistry {
242+
for _, imageWithConfig := range imageWithConfigs {
243+
if imageWithConfig.ModuleFullName() == nil {
244+
if imageWithConfig.ModuleOpaqueID() == "" {
245+
// This can occur in the case of a [buffetch.MessageRef], where we resolve the message
246+
// ref directly from the bucket without building the [bufmodule.Module]. In that case,
247+
// we are unnable to use --against-registry.
248+
return fmt.Errorf("cannot use --%s with unnamed module", againstRegistryFlagName)
249+
}
250+
return fmt.Errorf(
251+
"cannot use --%s with unnamed module, %s",
252+
againstRegistryFlagName,
253+
imageWithConfig.ModuleOpaqueID(),
254+
)
255+
}
256+
againstImage, err := controller.GetImage(
257+
ctx,
258+
imageWithConfig.ModuleFullName().String(),
259+
bufctl.WithTargetPaths(externalPaths, flags.ExcludePaths),
260+
bufctl.WithConfigOverride(flags.AgainstConfig),
261+
)
262+
if err != nil {
263+
return err
264+
}
265+
againstImages = append(againstImages, againstImage)
266+
}
267+
}
268+
if len(imageWithConfigs) != len(againstImages) {
210269
// If workspaces are being used as input, the number
211270
// of images MUST match. Otherwise the results will
212271
// be meaningless and yield false positives.
@@ -216,7 +275,7 @@ func run(
216275
return fmt.Errorf(
217276
"input contained %d images, whereas against contained %d images",
218277
len(imageWithConfigs),
219-
len(againstImageWithConfigs),
278+
len(againstImages),
220279
)
221280
}
222281
// We add all check configs (both lint and breaking) as related configs to check if plugins
@@ -240,7 +299,7 @@ func run(
240299
ctx,
241300
imageWithConfig.BreakingConfig(),
242301
imageWithConfig,
243-
againstImageWithConfigs[i],
302+
againstImages[i],
244303
breakingOptions...,
245304
); err != nil {
246305
var fileAnnotationSet bufanalysis.FileAnnotationSet
@@ -274,3 +333,13 @@ func getExternalPathsForImages[I bufimage.Image, S ~[]I](images S) ([]string, er
274333
}
275334
return slicesext.MapKeysToSlice(externalPaths), nil
276335
}
336+
337+
func validateFlags(flags *flags) error {
338+
if flags.Against == "" && !flags.AgainstRegistry {
339+
return fmt.Errorf("Must set --%s or --%s", againstFlagName, againstRegistryFlagName)
340+
}
341+
if flags.Against != "" && flags.AgainstRegistry {
342+
return fmt.Errorf("Cannot set both --%s and --%s", againstFlagName, againstRegistryFlagName)
343+
}
344+
return nil
345+
}

0 commit comments

Comments
 (0)