Add policy support to buf config commands (#3983)

Author: emcfarlane

private/buf/cmd/buf/buf_test.go

@@ -715,6 +715,23 @@ PACKAGE_NO_IMPORT_CYCLE                                              Checks that
 	)
 }
 
+func TestCheckPolicyConfigLsLintRulesConfigured(t *testing.T) {
+	t.Parallel()
+	expectedConfiguredOnlyStdout, err := os.ReadFile(filepath.Join("testdata", "policy_list_rules", "expected_ls_lint_rules_configured_only.txt"))
+	require.NoError(t, err)
+	testRunStdout(
+		t,
+		nil,
+		0,
+		string(expectedConfiguredOnlyStdout),
+		"config",
+		"ls-lint-rules",
+		"--config",
+		filepath.Join("testdata", "policy_list_rules", "buf.yaml"),
+		"--configured-only",
+	)
+}
+
 func TestCheckLsLintRulesFromConfig(t *testing.T) {
 	t.Parallel()
 	testRunStdout(

private/buf/cmd/buf/command/config/internal/internal.go

@@ -257,6 +257,7 @@ func lsRun(
 		}
 		configuredRuleOptions := []bufcheck.ConfiguredRulesOption{
 			bufcheck.WithPluginConfigs(bufYAMLFile.PluginConfigs()...),
+			bufcheck.WithPolicyConfigs(bufYAMLFile.PolicyConfigs()...),
 			bufcheck.WithRelatedCheckConfigs(allCheckConfigs...),
 		}
 		rules, err = checkClient.ConfiguredRules(
@@ -271,6 +272,7 @@ func lsRun(
 	} else {
 		allRulesOptions := []bufcheck.AllRulesOption{
 			bufcheck.WithPluginConfigs(bufYAMLFile.PluginConfigs()...),
+			bufcheck.WithPolicyConfigs(bufYAMLFile.PolicyConfigs()...),
 		}
 		rules, err = checkClient.AllRules(
 			ctx,

private/buf/cmd/buf/testdata/policy_list_rules/buf.yaml

@@ -0,0 +1,11 @@
+version: v2
+lint:
+  use:
+    - ENUM_NO_ALLOW_ALIAS
+    - PACKAGE_DIRECTORY_MATCH
+breaking:
+  use:
+    - ENUM_VALUE_NO_DELETE
+    - FIELD_SAME_JSTYPE
+policies:
+  - policy: testdata/policy_list_rules/policy.yaml

private/buf/cmd/buf/testdata/policy_list_rules/expected_ls_lint_rules_configured_only.txt

@@ -0,0 +1,13 @@
+ID                       CATEGORIES                DEFAULT  PURPOSE
+PACKAGE_DIRECTORY_MATCH  MINIMAL, BASIC, STANDARD  *        Checks that all files are in a directory that matches their package name.
+ENUM_NO_ALLOW_ALIAS      BASIC, STANDARD           *        Checks that enums do not have the allow_alias option set.
+
+testdata/policy_list_rules/policy.yaml
+
+PACKAGE_DIRECTORY_MATCH  MINIMAL, BASIC, STANDARD  *        Checks that all files are in a directory that matches their package name.
+ENUM_NO_ALLOW_ALIAS      BASIC, STANDARD           *        Checks that enums do not have the allow_alias option set.
+
+testdata/policy_list_rules/policy.yaml buf-plugin-rpc-ext
+
+PAGE_REQUEST_HAS_TOKEN                             *        Checks that all pagination RPC requests has a page token set.
+PAGE_RESPONSE_HAS_TOKEN                            *        Checks that all pagination RPC responses has a page token set.

private/buf/cmd/buf/testdata/policy_list_rules/policy.yaml

@@ -0,0 +1,13 @@
+version: v2
+lint:
+  use:
+    - ENUM_NO_ALLOW_ALIAS
+    - PACKAGE_DIRECTORY_MATCH
+    - PAGE_REQUEST_HAS_TOKEN
+    - PAGE_RESPONSE_HAS_TOKEN
+breaking:
+  use:
+    - ENUM_VALUE_NO_DELETE
+    - FIELD_SAME_JSTYPE
+plugins:
+  - plugin: buf-plugin-rpc-ext

private/bufpkg/bufcheck/bufcheck.go

@@ -74,6 +74,12 @@ type Rule interface {
 	//
 	// Will be empty for Rules based on builtin plugins.
 	PluginName() string
+	// PolicyName returns the name of the policy that contains this Rule, if any.
+	//
+	// Names are freeform.
+	//
+	// Will be empty for Rules not based on policies.
+	PolicyName() string
 
 	isRule()
 	isRuleOrCategory()
@@ -91,6 +97,12 @@ type Category interface {
 	//
 	// Will be empty for Categorys based on builtin plugins.
 	PluginName() string
+	// PolicyName returns the name of the policy that contains this Category, if any.
+	//
+	// Names are freeform.
+	//
+	// Will be empty for Categorys not based on policies.
+	PolicyName() string
 
 	isCategory()
 	isRuleOrCategory()

private/bufpkg/bufcheck/category.go

@@ -24,18 +24,24 @@ type category struct {
 	check.Category
 
 	pluginName string
+	policyName string
 }
 
-func newCategory(checkCategory check.Category, pluginName string) *category {
+func newCategory(checkCategory check.Category, pluginName string, policyName string) *category {
 	return &category{
 		Category:   checkCategory,
 		pluginName: pluginName,
+		policyName: policyName,
 	}
 }
 
 func (r *category) PluginName() string {
 	return r.pluginName
 }
 
+func (r *category) PolicyName() string {
+	return r.policyName
+}
+
 func (*category) isCategory()       {}
 func (*category) isRuleOrCategory() {}

private/bufpkg/bufcheck/client.go

@@ -17,8 +17,10 @@ package bufcheck
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"log/slog"
 	"strings"
 
@@ -386,7 +388,7 @@ func (c *client) ConfiguredRules(
 	for _, option := range options {
 		option.applyToConfiguredRules(configuredRulesOptions)
 	}
-	allRules, allCategories, err := c.allRulesAndCategories(
+	rules, categories, err := c.allRulesAndCategories(
 		ctx,
 		checkConfig.FileVersion(),
 		configuredRulesOptions.pluginConfigs,
@@ -396,12 +398,46 @@ func (c *client) ConfiguredRules(
 	if err != nil {
 		return nil, err
 	}
-	rulesConfig, err := rulesConfigForCheckConfig(checkConfig, allRules, allCategories, ruleType, configuredRulesOptions.relatedCheckConfigs)
+	rulesConfig, err := rulesConfigForCheckConfig(checkConfig, rules, categories, ruleType, configuredRulesOptions.relatedCheckConfigs)
 	if err != nil {
 		return nil, err
 	}
+	allRules := rulesForRuleIDs(rules, rulesConfig.RuleIDs)
+	policies, err := c.getPolicies(ctx, configuredRulesOptions.policyConfigs)
+	if err != nil {
+		return nil, err
+	}
+	for index, policy := range policies {
+		policyConfig := configuredRulesOptions.policyConfigs[index]
+		pluginConfigs, err := policyToBufConfigPluginConfigs(policy)
+		if err != nil {
+			return nil, err
+		}
+		// Load the check config for the rule type.
+		var policyCheckConfig bufconfig.CheckConfig
+		switch ruleType {
+		case check.RuleTypeLint:
+			policyCheckConfig, err = policyToBufConfigLintConfig(policy, policyConfig)
+		case check.RuleTypeBreaking:
+			policyCheckConfig, err = policyToBufConfigBreakingConfig(policy, policyConfig)
+		default:
+			return nil, fmt.Errorf("unknown check.RuleType: %v", ruleType)
+		}
+		if err != nil {
+			return nil, err
+		}
+		policyRules, policyCategories, err := c.allRulesAndCategories(ctx, policyCheckConfig.FileVersion(), pluginConfigs, policyConfig, false)
+		if err != nil {
+			return nil, err
+		}
+		policyRulesConfig, err := rulesConfigForCheckConfig(policyCheckConfig, policyRules, policyCategories, ruleType, nil)
+		if err != nil {
+			return nil, err
+		}
+		allRules = append(allRules, rulesForRuleIDs(policyRules, policyRulesConfig.RuleIDs)...)
+	}
 	logRulesConfig(c.logger, rulesConfig)
-	return rulesForRuleIDs(allRules, rulesConfig.RuleIDs), nil
+	return allRules, nil
 }
 
 func (c *client) AllRules(
@@ -420,6 +456,22 @@ func (c *client) AllRules(
 	if err != nil {
 		return nil, err
 	}
+	policies, err := c.getPolicies(ctx, allRulesOptions.policyConfigs)
+	if err != nil {
+		return nil, err
+	}
+	for index, policy := range policies {
+		policyConfig := allRulesOptions.policyConfigs[index]
+		pluginConfigs, err := policyToBufConfigPluginConfigs(policy)
+		if err != nil {
+			return nil, err
+		}
+		policyRules, _, err := c.allRulesAndCategories(ctx, fileVersion, pluginConfigs, policyConfig, false)
+		if err != nil {
+			return nil, err
+		}
+		rules = append(rules, policyRules...)
+	}
 	return rulesForType(rules, ruleType), nil
 }
 
@@ -580,6 +632,12 @@ func (c *client) getPlugins(ctx context.Context, pluginConfigs []bufconfig.Plugi
 		pluginRefs := xslices.IndexedToValues(indexedPluginRefs)
 		pluginKeys, err := pluginKeyProvider.GetPluginKeysForPluginRefs(ctx, pluginRefs, bufplugin.DigestTypeP1)
 		if err != nil {
+			if errors.Is(err, fs.ErrNotExist) {
+				if policyConfig != nil {
+					return nil, fmt.Errorf("unable to resolve plugins for policy %q: %w", policyConfig.Name(), err)
+				}
+				return nil, fmt.Errorf("unable to resolve plugins: %w", err)
+			}
 			return nil, err
 		}
 		pluginDatas, err := pluginDataProvider.GetPluginDatasForPluginKeys(ctx, pluginKeys)

private/bufpkg/bufcheck/multi_client.go

@@ -97,10 +97,7 @@ func (c *multiClient) Check(ctx context.Context, request check.Request) ([]*anno
 				defer xslog.DebugProfile(c.logger, slog.String("plugin", delegate.PluginName))()
 				delegateResponse, err := delegate.Client.Check(ctx, delegateRequest)
 				if err != nil {
-					if delegate.PluginName == "" {
-						return err
-					}
-					return fmt.Errorf("plugin %q failed: %w", delegate.PluginName, err)
+					return formatDelegateError(delegate, err)
 				}
 				annotations := xslices.Map(
 					delegateResponse.Annotations(),
@@ -155,14 +152,13 @@ func (c *multiClient) getRulesCategoriesAndChunkedIDs(ctx context.Context) (
 	for i, delegate := range c.checkClientSpecs {
 		delegateCheckRules, err := delegate.Client.ListRules(ctx)
 		if err != nil {
-			if delegate.PluginName == "" {
-				return nil, nil, nil, nil, err
-			}
-			return nil, nil, nil, nil, fmt.Errorf("plugin %q failed: %w", delegate.PluginName, err)
+			return nil, nil, nil, nil, formatDelegateError(delegate, err)
 		}
 		delegateRules := xslices.Map(
 			delegateCheckRules,
-			func(checkRule check.Rule) Rule { return newRule(checkRule, delegate.PluginName) },
+			func(checkRule check.Rule) Rule {
+				return newRule(checkRule, delegate.PluginName, delegate.PolicyName)
+			},
 		)
 		rules = append(rules, delegateRules...)
 		// Already sorted.
@@ -174,14 +170,13 @@ func (c *multiClient) getRulesCategoriesAndChunkedIDs(ctx context.Context) (
 	for i, delegate := range c.checkClientSpecs {
 		delegateCheckCategories, err := delegate.Client.ListCategories(ctx)
 		if err != nil {
-			if delegate.PluginName == "" {
-				return nil, nil, nil, nil, err
-			}
-			return nil, nil, nil, nil, fmt.Errorf("plugin %q failed: %w", delegate.PluginName, err)
+			return nil, nil, nil, nil, formatDelegateError(delegate, err)
 		}
 		delegateCategories := xslices.Map(
 			delegateCheckCategories,
-			func(checkCategory check.Category) Category { return newCategory(checkCategory, delegate.PluginName) },
+			func(checkCategory check.Category) Category {
+				return newCategory(checkCategory, delegate.PluginName, delegate.PolicyName)
+			},
 		)
 		categories = append(categories, delegateCategories...)
 		// Already sorted.
@@ -300,3 +295,19 @@ func (d *duplicateRuleOrCategoryError) duplicateIDs() []string {
 	}
 	return xslices.MapKeysToSortedSlice(d.duplicateIDToRuleOrCategories)
 }
+
+// formatDelegateError formats the error messages including plugin and policy information.
+func formatDelegateError(delegate *checkClientSpec, err error) error {
+	if delegate.PluginName == "" {
+		return err // Built-in rule, return error as-is
+	}
+	var errorMsg strings.Builder
+	errorMsg.WriteString("plugin ")
+	errorMsg.WriteString(fmt.Sprintf("%q", delegate.PluginName))
+	if delegate.PolicyName != "" {
+		errorMsg.WriteString(" from policy ")
+		errorMsg.WriteString(fmt.Sprintf("%q", delegate.PolicyName))
+	}
+	errorMsg.WriteString(" failed: ")
+	return fmt.Errorf("%s%w", errorMsg.String(), err)
+}