From 3c926839b1d24d5cd20217f2ea539e2bf9bb23fb Mon Sep 17 00:00:00 2001 From: Stefan VanBuren Date: Tue, 31 Mar 2026 16:23:36 -0400 Subject: [PATCH 1/3] Test registry-enabled plugins depend on registry-enabled plugins --- tests/plugins_test.go | 61 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tests/plugins_test.go b/tests/plugins_test.go index 4528571c2..3c32f29f7 100644 --- a/tests/plugins_test.go +++ b/tests/plugins_test.go @@ -455,6 +455,67 @@ func TestPyPIDependencies(t *testing.T) { } } +func TestRegistryDepsHaveRegistryConfig(t *testing.T) { + t.Parallel() + plugins := loadAllPlugins(t) + // Build a lookup map by "name:version". + pluginByRef := make(map[string]*plugin.Plugin, len(plugins)) + for _, p := range plugins { + pluginByRef[p.String()] = p + } + // registryType returns a non-empty string identifying which registry type is configured, or "". + registryType := func(p *plugin.Plugin) string { + switch { + case p.Registry.Go != nil: + return "go" + case p.Registry.NPM != nil: + return "npm" + case p.Registry.Maven != nil: + return "maven" + case p.Registry.Swift != nil: + return "swift" + case p.Registry.Python != nil: + return "python" + case p.Registry.Cargo != nil: + return "cargo" + case p.Registry.Nuget != nil: + return "nuget" + case p.Registry.Cmake != nil: + return "cmake" + default: + return "" + } + } + // checkDeps recursively verifies that all transitive dependencies of p have the expected registry type. + var checkDeps func(t *testing.T, p *plugin.Plugin, want string, visited map[string]bool) + checkDeps = func(t *testing.T, p *plugin.Plugin, want string, visited map[string]bool) { + t.Helper() + for _, dep := range p.Deps { + if visited[dep.Plugin] { + continue + } + visited[dep.Plugin] = true + depPlugin, ok := pluginByRef[dep.Plugin] + require.Truef(t, ok, "dependency %q not found", dep.Plugin) + assert.Equalf(t, want, registryType(depPlugin), + "dependency %q of plugin %q has registry type %q, want %q", + dep.Plugin, p.String(), registryType(depPlugin), want, + ) + checkDeps(t, depPlugin, want, visited) + } + } + for _, p := range plugins { + rt := registryType(p) + if rt == "" { + continue + } + t.Run(fmt.Sprintf("%s/%s@%s", p.Identity.Owner(), p.Identity.Plugin(), p.PluginVersion), func(t *testing.T) { + t.Parallel() + checkDeps(t, p, rt, make(map[string]bool)) + }) + } +} + func runPluginWithImage(ctx context.Context, t *testing.T, basedir string, pluginMeta *plugin.Plugin, image string, goPkgPrefix string) string { t.Helper() gendir := filepath.Join(basedir, "gen") From b9abdd2ddf7b81c68e704a9a5633b8d61991e21c Mon Sep 17 00:00:00 2001 From: Stefan VanBuren Date: Tue, 31 Mar 2026 16:26:00 -0400 Subject: [PATCH 2/3] Add allowlist of plugins missing registry deps We shouldn't add to this list, but we also aren't planning to go back to patch these particular plugin versions. --- tests/plugins_test.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/plugins_test.go b/tests/plugins_test.go index 3c32f29f7..f945d147e 100644 --- a/tests/plugins_test.go +++ b/tests/plugins_test.go @@ -84,6 +84,13 @@ exec docker run --log-driver=none --rm -i {{.ImageName}}:{{.Version}} "$@" "buf.build/community/mercari-grpc-federation": {"eliza": true, "petapis": true}, "buf.build/googlecloudplatform/bq-schema": {"eliza": true, "petapis": true}, } + // allowedMissingRegistryDeps lists dependencies that are known to lack a registry config for + // a given registry type. The key is "pluginRef -> depRef" (e.g. "buf.build/grpc/python:v1.59.1 -> buf.build/protocolbuffers/python:v24.4"). + // Only add entries here for old plugin versions where the dep predates registry config support. + allowedMissingRegistryDeps = map[string]bool{ + "buf.build/grpc/python:v1.59.1 -> buf.build/protocolbuffers/python:v24.4": true, + "buf.build/grpc/python:v1.59.2 -> buf.build/protocolbuffers/python:v24.4": true, + } ) func TestGeneration(t *testing.T) { @@ -497,10 +504,13 @@ func TestRegistryDepsHaveRegistryConfig(t *testing.T) { visited[dep.Plugin] = true depPlugin, ok := pluginByRef[dep.Plugin] require.Truef(t, ok, "dependency %q not found", dep.Plugin) - assert.Equalf(t, want, registryType(depPlugin), - "dependency %q of plugin %q has registry type %q, want %q", - dep.Plugin, p.String(), registryType(depPlugin), want, - ) + key := fmt.Sprintf("%s -> %s", p.String(), dep.Plugin) + if registryType(depPlugin) != want && !allowedMissingRegistryDeps[key] { + assert.Failf(t, "missing registry config", + "dependency %q of plugin %q has registry type %q, want %q", + dep.Plugin, p.String(), registryType(depPlugin), want, + ) + } checkDeps(t, depPlugin, want, visited) } } From b0be09ac7bc7fda8c768b29ee84878cf3b460c1b Mon Sep 17 00:00:00 2001 From: Stefan VanBuren Date: Tue, 31 Mar 2026 16:53:55 -0400 Subject: [PATCH 3/3] Add handling for missing an updated registry type --- tests/plugins_test.go | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/tests/plugins_test.go b/tests/plugins_test.go index f945d147e..b826f5abc 100644 --- a/tests/plugins_test.go +++ b/tests/plugins_test.go @@ -14,6 +14,7 @@ import ( "os" "os/exec" "path/filepath" + "reflect" "strconv" "strings" "testing" @@ -470,8 +471,12 @@ func TestRegistryDepsHaveRegistryConfig(t *testing.T) { for _, p := range plugins { pluginByRef[p.String()] = p } - // registryType returns a non-empty string identifying which registry type is configured, or "". - registryType := func(p *plugin.Plugin) string { + // registryType returns a string identifying which registry type is configured, + // or "" if no registry is configured. Fails the test if any pointer field on + // the registry struct is non-nil but not handled by the switch, so this test + // stays up to date when new registry types are added to ExternalRegistryConfig. + registryType := func(t *testing.T, p *plugin.Plugin) string { + t.Helper() switch { case p.Registry.Go != nil: return "go" @@ -490,6 +495,14 @@ func TestRegistryDepsHaveRegistryConfig(t *testing.T) { case p.Registry.Cmake != nil: return "cmake" default: + rv := reflect.ValueOf(p.Registry) + rt := reflect.TypeFor[bufremotepluginconfig.ExternalRegistryConfig]() + for i := range rt.NumField() { + field := rt.Field(i) + if field.Type.Kind() == reflect.Pointer && !rv.Field(i).IsNil() { + assert.Failf(t, "unrecognized registry type", "plugin %q has unrecognized registry type field %q - update registryType() in this test", p.String(), field.Name) + } + } return "" } } @@ -505,17 +518,17 @@ func TestRegistryDepsHaveRegistryConfig(t *testing.T) { depPlugin, ok := pluginByRef[dep.Plugin] require.Truef(t, ok, "dependency %q not found", dep.Plugin) key := fmt.Sprintf("%s -> %s", p.String(), dep.Plugin) - if registryType(depPlugin) != want && !allowedMissingRegistryDeps[key] { + if registryType(t, depPlugin) != want && !allowedMissingRegistryDeps[key] { assert.Failf(t, "missing registry config", "dependency %q of plugin %q has registry type %q, want %q", - dep.Plugin, p.String(), registryType(depPlugin), want, + dep.Plugin, p.String(), registryType(t, depPlugin), want, ) } checkDeps(t, depPlugin, want, visited) } } for _, p := range plugins { - rt := registryType(p) + rt := registryType(t, p) if rt == "" { continue }