Repository: bufbuild/buf (go)

Issue:
buf 1.32.2 breaking doesn't respect v2 config file when using binpb input
### What's up?

Sorry for lack of test case, but this should be pretty easy to reproduce:

I was trying to set up a check using `buf breaking` like this:

```
buf breaking image1.bin --against image2.bin --config buf.yaml
```

The image files were not generated by buf, they were generated by `protoc -o`

I tried using buf 1.32.2 with a config like:

```
version: v2
breaking:
  use:
    - WIRE
```

This did the breaking check using the default `FILE` level, the `WIRE` level was not respected. I also tried adding `ignore` settings that were not respected.

I downgraded to buf 1.6.0 and used the same command line with the same config except `version: v1`, and it respected the breaking settings as expected.

Pull request:
Set top-level `LintConfig` and `BreakingConfig` and use for image inputs
This PR exposes the top-level lint and breaking configs for `buf.yaml` files.

For `v1` `buf.yaml` files, there is only ever one `ModuleConfig`, so the
corresponding lint and breaking configs are used.

For `v2` `buf.yaml` files, if a top-level lint and/or breaking config exists, then
we return those.
Otherwise, `TopLevelLintConfig()`/`TopLevelBreakingConfig` will return `nil`.

We use the top-level lint/breaking configs, if possible, with image inputs, since we
cannot determine which specific `v2` module config to use with images. In
the case where there is no top-level config, we fall back to defaults.

Fixes #3080 

Patch:
diff --git a/private/buf/bufctl/controller.go b/private/buf/bufctl/controller.go
index 88a9c268f..503ca76aa 100644
--- a/private/buf/bufctl/controller.go
+++ b/private/buf/bufctl/controller.go
@@ -393,20 +393,24 @@ 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 {
-			switch fileVersion := bufYAMLFile.FileVersion(); fileVersion {
-			case bufconfig.FileVersionV1Beta1, bufconfig.FileVersionV1:
-				moduleConfigs := bufYAMLFile.ModuleConfigs()
-				if len(moduleConfigs) != 1 {
-					return nil, fmt.Errorf("expected 1 ModuleConfig for FileVersion %v, got %d", len(moduleConfigs), fileVersion)
+			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)
 				}
-				lintConfig = moduleConfigs[0].LintConfig()
-				breakingConfig = moduleConfigs[0].BreakingConfig()
-			case bufconfig.FileVersionV2:
-				// Do nothing. Use the default LintConfig and BreakingConfig. With
-				// the new buf.yamls with multiple modules, we don't know what lint or
-				// breaking config to apply.
-			default:
-				return nil, syserror.Newf("unknown FileVersion: %v", fileVersion)
+				// v2 config without a top-level lint config, use v2 default
+				lintConfig = bufconfig.DefaultLintConfigV2
+			} else {
+				lintConfig = topLevelLintConfig
+			}
+			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)
+				}
+				// v2 config without a top-level breaking config, use v2 default
+				breakingConfig = bufconfig.DefaultBreakingConfigV2
+			} else {
+				breakingConfig = topLevelBreakingConfig
 			}
 		}
 		return []ImageWithConfig{
diff --git a/private/buf/buffetch/buffetch.go b/private/buf/buffetch/buffetch.go
index 5080b9db3..acab16713 100644
--- a/private/buf/buffetch/buffetch.go
+++ b/private/buf/buffetch/buffetch.go
@@ -94,7 +94,7 @@ type Ref interface {
 	internalRef() internal.Ref
 }
 
-// MessageRef is an message file reference.
+// MessageRef is a message file reference.
 type MessageRef interface {
 	Ref
 	MessageEncoding() MessageEncoding
diff --git a/private/bufpkg/bufconfig/buf_yaml_file.go b/private/bufpkg/bufconfig/buf_yaml_file.go
index b02fd4652..a78878481 100644
--- a/private/bufpkg/bufconfig/buf_yaml_file.go
+++ b/private/bufpkg/bufconfig/buf_yaml_file.go
@@ -74,6 +74,20 @@ type BufYAMLFile interface {
 	// All ModuleConfigs will have unique ModuleFullNames.
 	// Sorted by DirPath.
 	ModuleConfigs() []ModuleConfig
+	// TopLevelLintConfig returns the top-level LintConfig for the File.
+	//
+	// For v1 buf.yaml files, there is only ever a single LintConfig, so this is returned.
+	// For v2 buf.yaml files, if a top-level lint config exists, then it will be the top-level
+	// lint config. Otherwise, this will return nil, so callers should be aware this may be
+	// empty.
+	TopLevelLintConfig() LintConfig
+	// TopLevelBreakingConfig returns the top-level BreakingConfig for the File.
+	//
+	// For v1 buf.yaml files, there is only ever a single BreakingConfig, so this is returned.
+	// For v2 buf.yaml files, if a top-level breaking config exists, then it will be the top-level
+	// breaking config. Otherwise, this will return nil, so callers should be aware this may be
+	// empty.
+	TopLevelBreakingConfig() BreakingConfig
 	// ConfiguredDepModuleRefs returns the configured dependencies of the Workspace as ModuleRefs.
 	//
 	// These come from buf.yaml files.
@@ -93,7 +107,14 @@ func NewBufYAMLFile(
 	moduleConfigs []ModuleConfig,
 	configuredDepModuleRefs []bufmodule.ModuleRef,
 ) (BufYAMLFile, error) {
-	return newBufYAMLFile(fileVersion, nil, moduleConfigs, configuredDepModuleRefs)
+	return newBufYAMLFile(
+		fileVersion,
+		nil,
+		moduleConfigs,
+		nil, // Do not set top-level lint config, use only module configs
+		nil, // Do not set top-level breaking config, use only module configs
+		configuredDepModuleRefs,
+	)
 }
 
 // GetBufYAMLFileForPrefix gets the buf.yaml file at the given bucket prefix.
@@ -190,6 +211,8 @@ type bufYAMLFile struct {
 	fileVersion             FileVersion
 	objectData              ObjectData
 	moduleConfigs           []ModuleConfig
+	topLevelLintConfig      LintConfig
+	topLevelBreakingConfig  BreakingConfig
 	configuredDepModuleRefs []bufmodule.ModuleRef
 }
 
@@ -197,6 +220,8 @@ func newBufYAMLFile(
 	fileVersion FileVersion,
 	objectData ObjectData,
 	moduleConfigs []ModuleConfig,
+	topLevelLintConfig LintConfig,
+	topLevelBreakingConfig BreakingConfig,
 	configuredDepModuleRefs []bufmodule.ModuleRef,
 ) (*bufYAMLFile, error) {
 	if (fileVersion == FileVersionV1Beta1 || fileVersion == FileVersionV1) && len(moduleConfigs) > 1 {
@@ -255,6 +280,8 @@ func newBufYAMLFile(
 		fileVersion:             fileVersion,
 		objectData:              objectData,
 		moduleConfigs:           moduleConfigs,
+		topLevelLintConfig:      topLevelLintConfig,
+		topLevelBreakingConfig:  topLevelBreakingConfig,
 		configuredDepModuleRefs: configuredDepModuleRefs,
 	}, nil
 }
@@ -275,6 +302,14 @@ func (c *bufYAMLFile) ModuleConfigs() []ModuleConfig {
 	return slicesext.Copy(c.moduleConfigs)
 }
 
+func (c *bufYAMLFile) TopLevelLintConfig() LintConfig {
+	return c.topLevelLintConfig
+}
+
+func (c *bufYAMLFile) TopLevelBreakingConfig() BreakingConfig {
+	return c.topLevelBreakingConfig
+}
+
 func (c *bufYAMLFile) ConfiguredDepModuleRefs() []bufmodule.ModuleRef {
 	return slicesext.Copy(c.configuredDepModuleRefs)
 }
@@ -351,6 +386,8 @@ func readBufYAMLFile(
 			[]ModuleConfig{
 				moduleConfig,
 			},
+			lintConfig,
+			breakingConfig,
 			configuredDepModuleRefs,
 		)
 	case FileVersionV2:
@@ -464,6 +501,30 @@ func readBufYAMLFile(
 			}
 			moduleConfigs = append(moduleConfigs, moduleConfig)
 		}
+		var topLevelLintConfig LintConfig
+		if !defaultExternalLintConfig.isEmpty() {
+			topLevelLintConfig, err = getLintConfigForExternalLintV2(
+				fileVersion,
+				defaultExternalLintConfig,
+				".",   // The top-level module config always has the root "."
+				false, // Not module-specific configuration
+			)
+			if err != nil {
+				return nil, err
+			}
+		}
+		var topLevelBreakingConfig BreakingConfig
+		if !defaultExternalBreakingConfig.isEmpty() {
+			topLevelBreakingConfig, err = getBreakingConfigForExternalBreaking(
+				fileVersion,
+				defaultExternalBreakingConfig,
+				".",   // The top-level module config always has the root "."
+				false, // Not module-specific configuration
+			)
+			if err != nil {
+				return nil, err
+			}
+		}
 		configuredDepModuleRefs, err := getConfiguredDepModuleRefsForExternalDeps(externalBufYAMLFile.Deps)
 		if err != nil {
 			return nil, err
@@ -472,6 +533,8 @@ func readBufYAMLFile(
 			fileVersion,
 			objectData,
 			moduleConfigs,
+			topLevelLintConfig,
+			topLevelBreakingConfig,
 			configuredDepModuleRefs,
 		)
 	default:
diff --git a/private/bufpkg/bufconfig/lint_config.go b/private/bufpkg/bufconfig/lint_config.go
index 3849da204..721698d09 100644
--- a/private/bufpkg/bufconfig/lint_config.go
+++ b/private/bufpkg/bufconfig/lint_config.go
@@ -34,7 +34,7 @@ var (
 		false,
 		false,
 		"",
-		false,
+		true, // We default to allowing comment ignores in v2
 	)
 )
 


Test patch:
diff --git a/private/buf/cmd/buf/buf_test.go b/private/buf/cmd/buf/buf_test.go
index a4626cc06..7f17c8c72 100644
--- a/private/buf/cmd/buf/buf_test.go
+++ b/private/buf/cmd/buf/buf_test.go
@@ -2464,6 +2464,23 @@ a/v3/a.proto:7:10:Field "2" on message "Foo" changed name from "value" to "Value
 		"--exclude-path",
 		filepath.Join("a", "v3", "foo"),
 	)
+	testRunStdoutStderrNoWarn(
+		t,
+		nil,
+		bufctl.ExitCodeFileAnnotation,
+		`a/v3/a.proto:6:3:Field "1" with name "key" on message "Foo" changed type from "string" to "int32". See https://developers.google.com/protocol-buffers/docs/proto3#updating for wire compatibility rules.`,
+		"",
+		"breaking",
+		filepath.Join(tempDir, "current.binpb"),
+		"--against",
+		filepath.Join(tempDir, "previous.binpb"),
+		"--path",
+		filepath.Join("a", "v3"),
+		"--exclude-path",
+		filepath.Join("a", "v3", "foo"),
+		"--config",
+		`{"version":"v2","breaking":{"use":["WIRE"]}}`,
+	)
 }
 
 func TestVersion(t *testing.T) {