Add configurable base plugin support in buf.plugin.yaml #3513

mfridman · 2024-12-05T18:35:06Z

This PR adds an optional base_plugin field to the Go registry config, allowing plugins like gRPC and Connect to use a different base plugin instead of the default protocolbuffers/go. This enables Generated SDK scenarios:

Multi-plugins to generate correct imports for module dependencies, rather than using the BSR's default protocolbuffers/go
Custom plugins (like gRPC) to properly integrate with custom multi-plugins for import path generation (both plugin and module dependencies)

Note: A multi-plugin bundles generators like protoc-gen-go, protoc-gen-go-vtproto, and protoc-gen-go-json that output to the same package and are packaged using: https://github.com/bufbuild/tools/tree/main/cmd/protoc-gen-multi

Example:

version: v1
name: remote/org/go-grpc
plugin_version: v1.5.1
output_languages:
  - go
deps:
  - plugin: remote/org/go-multi:v0.1.0
registry:
  go:
    base_plugin: remote/org/go-multi # --- NEW FIELD
    min_version: "1.21"
    deps:
      - module: google.golang.org/grpc
        version: v1.68.1
  opts:
    - paths=source_relative
    - require_unimplemented_servers=false
    - separate_package=true

github-actions · 2024-12-05T18:35:25Z

The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
`✅ passed`	`✅ passed`	`✅ passed`	`✅ passed`	Dec 5, 2024, 7:40 PM

mfridman · 2024-12-05T18:37:57Z

private/bufpkg/bufremoteplugin/bufremotepluginconfig/bufremotepluginconfig.go

@@ -440,6 +441,7 @@ type ExternalGoRegistryConfig struct {
 		Module  string `json:"module,omitempty" yaml:"module,omitempty"`
 		Version string `json:"version,omitempty" yaml:"version,omitempty"`
 	} `json:"deps,omitempty" yaml:"deps,omitempty"`
+	BasePluginName string `json:"base_plugin_name,omitempty" yaml:"base_plugin_name,omitempty"`


Suggested change

BasePluginName string `json:"base_plugin_name,omitempty" yaml:"base_plugin_name,omitempty"`

BasePlugin string `json:"base_plugin,omitempty" yaml:"base_plugin,omitempty"`

This might be clearer and more concise. The intended format is remote/owner/plugin

Changed in a2edfb7 to "base plugin" terminology.

mfridman · 2024-12-05T18:51:33Z

proto/buf/alpha/registry/v1alpha1/plugin_curation.proto

+  // This affects how imports are resolved - the specified plugin's import path will be used as the
+  // base path for all generated code, replacing the default protocolbuffers/go import paths. Used
+  // when depending on non-default BSR plugins.
+  string base_plugin_name = 3;


@srikrsna-buf pointed out this could be a boolean, like use_current_plugin_as_base. But that'll only solve item 1. in the description (specifically, module dependencies).

You could also argue if a plugin has no plugin deps, then the default (on the BSR) should be to use the plugin itself as the base (for module deps) instead of protocolbuffers/go. Example of petapis -> paymentaips module dep:

- v1alpha1 "bufbuild.internal/gen/go/acme/paymentapis/protocolbuffers/go/payment/v1alpha1" + v1alpha1 "bufbuild.internal/gen/go/acme/paymentapis/custom-plugins/go/payment/v1alpha1"

Either way, I went a step further and made this a configurable string so it satisfies both 1. and 2. Where 2. is module and plugin dependencies.

mfridman · 2024-12-05T19:10:28Z

private/bufpkg/bufremoteplugin/bufremotepluginconfig/bufremotepluginconfig.go

@@ -440,6 +441,7 @@ type ExternalGoRegistryConfig struct {
 		Module  string `json:"module,omitempty" yaml:"module,omitempty"`
 		Version string `json:"version,omitempty" yaml:"version,omitempty"`
 	} `json:"deps,omitempty" yaml:"deps,omitempty"`
+	BasePlugin string `json:"base_plugin,omitempty" yaml:"base_plugin,omitempty"`


An alternative way to configure this might be to expose an is_base: true within deps, e.g.,

deps: - plugin: remote/org/go-multi:v0.1.0 is_base: true

And allowing exactly one plugin within deps to have this be set to true.

However, I don't like this because it leaks a Generated SDK configuration beyond the registry config. (which at the moment is Go-specific).

stefanvanburen · 2024-12-05T19:42:16Z

private/bufpkg/bufremoteplugin/bufremotepluginconfig/config.go

+		// Validate the base plugin is included as one of the plugin dependencies when both are
+		// specified. This ensures there's exactly one base type and it has a known dependency to
+		// generate imports correctly and build a correct Go mod file.
+		if len(pluginDependencies) > 0 {
+			ok := slices.ContainsFunc(pluginDependencies, func(ref bufremotepluginref.PluginReference) bool {
+				return ref.IdentityString() == basePlugin.IdentityString()
+			})
+			if !ok {
+				return nil, fmt.Errorf("base plugin %q not found in plugin dependencies", externalGoRegistryConfig.BasePlugin)
+			}
+		}


Is the intention that if there's no plugin dependency, assigning base_plugin has no effect? Is there a use-case for assigning it without a matching dependency?

Not quite, we still want to support option 1 in the description.

Suppose you have a multi-plugin that slams ALL the generators into a single multi-plugin. In this scenario, there are no plugin deps (all plugins output to the same package), but we still need to generate the correct module imports.

E.g., think of acme/petapis, which depends on acme/paymentapis. Without specifying the base type, we default to protocolbuffers/go, but what we want is something like this:

package petv1 import ( - v1alpha1 "remote/gen/go/acme/paymentapis/protocolbuffers/go/payment/v1alpha1" + v1alpha1 "remote/gen/go/acme/paymentapis/custom-plugins/go/payment/v1alpha1"

Likewise, we need to build a go.mod file that has the correct Go module:

module remote/gen/go/acme/petapis/custom-plugins/go go 1.21 require ( - remote/gen/go/acme/paymentapis/protocolbuffers/go v0.2.0-20241204162507-076bca1034ed.1 + remote/gen/go/acme/paymentapis/custom-plugins/go v0.2.0-20241204162507-076bca1034ed.1

ah, I see, was not thinking about the multi-plugin. That makes sense to me.

I wonder if the BSR should support option 1 with no further configuration. If there are no plugin dependencies, use the plugin name itself (instead of protocolbuffers/go) for module dependencies.

Effectively, the same as manually setting just so protocolbuffers/go isn't used.

version: v1 name: remote/org/go-multi # --- SAME plugin_version: v0.1.0 output_languages: - go registry: go: base_plugin: remote/org/go-multi # --- SAME min_version: "1.21" deps: - module: google.golang.org/protobuf version: v1.35.2 - module: github.com/planetscale/vtprotobuf version: v0.6.0 # Add the options to invoke each plugin for the generated SDK. opts: - --go_out=. - --go_opt=paths=source_relative - --go-json_out=. - --go-json_opt=paths=source_relative,emit_defaults=true - --go-vtproto_out=. - --go-vtproto_opt=paths=source_relative,features=marshal+unmarshal+size

mfridman added 2 commits December 5, 2024 12:52

update proto

57be7b9

update implementation

86ec011

mfridman requested a review from bufdev as a code owner December 5, 2024 18:35

mfridman commented Dec 5, 2024

View reviewed changes

code comment

f2ad326

mfridman commented Dec 5, 2024

View reviewed changes

mfridman added 2 commits December 5, 2024 13:55

change to "base plugin"

a2edfb7

cleanup comments and tests

a8d1340

mfridman commented Dec 5, 2024

View reviewed changes

mfridman added 2 commits December 5, 2024 14:36

cleanup and update logic

e27a190

clarify comment

2f9dbb7

stefanvanburen reviewed Dec 5, 2024

View reviewed changes

stefanvanburen approved these changes Dec 5, 2024

View reviewed changes

mfridman requested review from emcfarlane and srikrsna-buf December 5, 2024 20:02

emcfarlane approved these changes Dec 5, 2024

View reviewed changes

srikrsna-buf approved these changes Dec 6, 2024

View reviewed changes

mfridman merged commit 6c6e323 into main Dec 6, 2024
10 checks passed

mfridman deleted the mf/plugin-go-config branch December 6, 2024 18:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add configurable base plugin support in buf.plugin.yaml #3513

Add configurable base plugin support in buf.plugin.yaml #3513

mfridman commented Dec 5, 2024 •

edited

Loading

github-actions bot commented Dec 5, 2024 •

edited

Loading

mfridman Dec 5, 2024

mfridman Dec 5, 2024

mfridman Dec 5, 2024

mfridman Dec 5, 2024 •

edited

Loading

stefanvanburen Dec 5, 2024

mfridman Dec 5, 2024 •

edited

Loading

stefanvanburen Dec 5, 2024

mfridman Dec 5, 2024 •

edited

Loading

	BasePluginName string `json:"base_plugin_name,omitempty" yaml:"base_plugin_name,omitempty"`
	BasePlugin string `json:"base_plugin,omitempty" yaml:"base_plugin,omitempty"`

Add configurable base plugin support in buf.plugin.yaml #3513

Add configurable base plugin support in buf.plugin.yaml #3513

Conversation

mfridman commented Dec 5, 2024 • edited Loading

Example:

github-actions bot commented Dec 5, 2024 • edited Loading

mfridman Dec 5, 2024

Choose a reason for hiding this comment

mfridman Dec 5, 2024

Choose a reason for hiding this comment

mfridman Dec 5, 2024

Choose a reason for hiding this comment

mfridman Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

stefanvanburen Dec 5, 2024

Choose a reason for hiding this comment

mfridman Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

stefanvanburen Dec 5, 2024

Choose a reason for hiding this comment

mfridman Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

mfridman commented Dec 5, 2024 •

edited

Loading

github-actions bot commented Dec 5, 2024 •

edited

Loading

mfridman Dec 5, 2024 •

edited

Loading

mfridman Dec 5, 2024 •

edited

Loading

mfridman Dec 5, 2024 •

edited

Loading