Replace transport of CLI from gRPC to Connect (bufbuild#1129)

.dockerignore

@@ -13,7 +13,6 @@ private/buf/cmd/buf/workspacetests/other/proto/workspacetest/cache/
 private/bufpkg/bufprotoplugin/cmd/protoc-gen-go-api/protoc-gen-go-api
 private/bufpkg/bufprotoplugin/cmd/protoc-gen-go-apiclient/protoc-gen-go-apiclient
 private/bufpkg/bufprotoplugin/cmd/protoc-gen-go-apiclientconnect/protoc-gen-go-apiclientconnect
-private/bufpkg/bufprotoplugin/cmd/protoc-gen-go-apiclientgrpc/protoc-gen-go-apiclientgrpc
 private/bufpkg/bufstyle/cmd/bufstyle/bufstyle
 private/bufpkg/buftesting/cache/
 private/pkg/bandeps/cmd/bandeps/bandeps

.gitignore

@@ -13,7 +13,6 @@
 /private/bufpkg/bufprotoplugin/cmd/protoc-gen-go-api/protoc-gen-go-api
 /private/bufpkg/bufprotoplugin/cmd/protoc-gen-go-apiclient/protoc-gen-go-apiclient
 /private/bufpkg/bufprotoplugin/cmd/protoc-gen-go-apiclientconnect/protoc-gen-go-apiclientconnect
-/private/bufpkg/bufprotoplugin/cmd/protoc-gen-go-apiclientgrpc/protoc-gen-go-apiclientgrpc
 /private/bufpkg/bufstyle/cmd/bufstyle/bufstyle
 /private/bufpkg/buftesting/cache/
 /private/pkg/bandeps/cmd/bandeps/bandeps

data/template/buf.go-client.gen.yaml

@@ -1,16 +1,11 @@
-# This template generates grpc-go and helper client-side packages.
+# This template generates connect-go and helper client-side packages.
 # This is for situations where the client-side is OSS.
 version: v1
 managed:
   enabled: true
   go_package_prefix:
     default: github.com/bufbuild/buf/private/gen/proto/go
 plugins:
-  - name: go-grpc
-    out: private/gen/proto/go
-    opt:
-      - paths=source_relative
-      - require_unimplemented_servers=false
   - name: connect-go
     out: private/gen/proto/connect
     opt:
@@ -27,14 +22,6 @@ plugins:
       - named_go_package=api=github.com/bufbuild/buf/private/gen/proto/api
       - named_go_package=apiclient=github.com/bufbuild/buf/private/gen/proto/apiclient
     strategy: all
-  - name: go-apiclientgrpc
-    out: private/gen/proto/apiclientgrpc
-    opt:
-      - paths=source_relative
-      - named_go_package=api=github.com/bufbuild/buf/private/gen/proto/api
-      - named_go_package=apiclient=github.com/bufbuild/buf/private/gen/proto/apiclient
-      - named_go_package=apiclientgrpc=github.com/bufbuild/buf/private/gen/proto/apiclientgrpc
-    strategy: all
   - name: go-apiclientconnect
     out: private/gen/proto/apiclientconnect
     opt:

go.mod

@@ -3,7 +3,7 @@ module github.com/bufbuild/buf
 go 1.18
 
 require (
-	github.com/bufbuild/connect-go v0.0.0-20220519164640-df55eca48735
+	github.com/bufbuild/connect-go v0.0.0-20220520175512-2b3d3442ffb8
 	github.com/gofrs/flock v0.8.1
 	github.com/gofrs/uuid v4.2.0+incompatible
 	github.com/google/go-cmp v0.5.8

go.sum

@@ -5,8 +5,8 @@ github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kd
 github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
 github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A=
 github.com/benbjohnson/clock v1.3.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
-github.com/bufbuild/connect-go v0.0.0-20220519164640-df55eca48735 h1:4dg/O0ApWz+XqmV008gCEmkqwu2RxA5oZfYAYa8k8mQ=
-github.com/bufbuild/connect-go v0.0.0-20220519164640-df55eca48735/go.mod h1:BajZGyRXK+Oq6Ddkm7atQ1Tu4W92OMpam7vyhFIf0ww=
+github.com/bufbuild/connect-go v0.0.0-20220520175512-2b3d3442ffb8 h1:ZaD9cVYESl0sBAvcpmSO4wRbOSwS32F5wNPrnDw1XdE=
+github.com/bufbuild/connect-go v0.0.0-20220520175512-2b3d3442ffb8/go.mod h1:BajZGyRXK+Oq6Ddkm7atQ1Tu4W92OMpam7vyhFIf0ww=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
 github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=

make/buf/all.mk

@@ -8,7 +8,6 @@ GO_BINS := $(GO_BINS) \
 	cmd/protoc-gen-buf-lint \
 	private/bufpkg/bufprotoplugin/cmd/protoc-gen-go-api \
 	private/bufpkg/bufprotoplugin/cmd/protoc-gen-go-apiclient \
-	private/bufpkg/bufprotoplugin/cmd/protoc-gen-go-apiclientgrpc \
 	private/bufpkg/bufprotoplugin/cmd/protoc-gen-go-apiclientconnect \
 	private/bufpkg/bufstyle/cmd/bufstyle \
 	private/pkg/bandeps/cmd/bandeps \
@@ -45,7 +44,6 @@ include make/go/dep_buf.mk
 include make/go/dep_minisign.mk
 include make/go/dep_protoc.mk
 include make/go/dep_protoc_gen_go.mk
-include make/go/dep_protoc_gen_go_grpc.mk
 include make/go/dep_protoc_gen_connect_go.mk
 include make/go/dep_go_fuzz.mk
 include make/go/go.mk
@@ -101,9 +99,8 @@ postprepostgenerate:: privateusage
 bufgeneratedeps:: \
 	installprotoc-gen-go-api \
 	installprotoc-gen-go-apiclient \
-	installprotoc-gen-go-apiclientgrpc \
 	installprotoc-gen-go-apiclientconnect \
-	$(PROTOC_GEN_GO) $(PROTOC_GEN_GO_GRPC) $(PROTOC_GEN_CONNECT_GO)
+	$(PROTOC_GEN_GO) $(PROTOC_GEN_CONNECT_GO)
 
 .PHONY: bufgeneratecleango
 bufgeneratecleango:

make/go/dep_protoc_gen_connect_go.mk

@@ -7,8 +7,8 @@ $(call _assert_var,CACHE_VERSIONS)
 $(call _assert_var,CACHE_BIN)
 
 # Settable
-# https://github.com/bufbuild/connect-go 20220518 checked 20220518
-CONNECT_VERSION ?= eaacdf55b404991aff8eca2e933c87168137b85a
+# https://github.com/bufbuild/connect-go 20220520 checked 20220524
+CONNECT_VERSION ?= 2b3d3442ffb851103f14dcfe64025586118b8f77
 
 PROTOC_GEN_CONNECT_GO := $(CACHE_VERSIONS)/connect-go/$(CONNECT_VERSION)
 $(PROTOC_GEN_CONNECT_GO):

private/buf/bufcli/bufcli.go

@@ -56,6 +56,7 @@ import (
 	"github.com/bufbuild/buf/private/pkg/rpc/rpcauth"
 	"github.com/bufbuild/buf/private/pkg/storage/storageos"
 	"github.com/bufbuild/buf/private/pkg/stringutil"
+	"github.com/bufbuild/buf/private/pkg/transport/http2client"
 	"github.com/spf13/pflag"
 	"go.uber.org/zap"
 	"golang.org/x/term"
@@ -551,7 +552,19 @@ func NewConfig(container appflag.Container) (*bufapp.Config, error) {
 
 // NewRegistryProvider creates a new registryv1alpha1apiclient.Provider.
 func NewRegistryProvider(ctx context.Context, container appflag.Container) (registryv1alpha1apiclient.Provider, error) {
-	return newGRPCRegistryProvider(ctx, container)
+	client := http2client.NewClient(
+		http2client.WithObservability(),
+	)
+	options := []bufapiclient.RegistryProviderOption{
+		bufapiclient.RegistryProviderWithContextModifierProvider(NewContextModifierProvider(container)),
+		bufapiclient.RegistryProviderWithAddressMapper(func(address string) string {
+			if buftransport.IsAPISubdomainEnabled(container) {
+				address = buftransport.PrependAPISubdomain(address)
+			}
+			return buftransport.PrependHTTPS(address)
+		}),
+	}
+	return bufapiclient.NewConnectClientProvider(container.Logger(), client, options...)
 }
 
 // NewContextModifierProvider returns a new context modifier provider for API providers.
@@ -817,25 +830,6 @@ func validateErrorFormatFlag(validFormatStrings []string, errorFormatString stri
 	return appcmd.NewInvalidArgumentErrorf("--%s: invalid format: %q", errorFormatFlagName, errorFormatString)
 }
 
-func newGRPCRegistryProvider(ctx context.Context, container appflag.Container) (registryv1alpha1apiclient.Provider, error) {
-	config, err := NewConfig(container)
-	if err != nil {
-		return nil, err
-	}
-	options := []bufapiclient.RegistryProviderOption{
-		bufapiclient.RegistryProviderWithContextModifierProvider(NewContextModifierProvider(container)),
-	}
-	if buftransport.IsAPISubdomainEnabled(container) {
-		options = append(options, bufapiclient.RegistryProviderWithAddressMapper(buftransport.PrependAPISubdomain))
-	}
-	return bufapiclient.NewGRPCClientProvider(
-		ctx,
-		container.Logger(),
-		config.TLS,
-		options...,
-	)
-}
-
 // promptUser reads a line from Stdin, prompting the user with the prompt first.
 // The prompt is repeatedly shown until the user provides a non-empty response.
 // ErrNotATTY is returned if the input containers Stdin is not a terminal.

private/buf/bufcli/errors.go

@@ -18,11 +18,14 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net"
+	"strings"
 
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleref"
+	"github.com/bufbuild/buf/private/bufpkg/buftransport"
 	"github.com/bufbuild/buf/private/pkg/app"
 	"github.com/bufbuild/buf/private/pkg/app/appflag"
-	"github.com/bufbuild/buf/private/pkg/rpc"
+	"github.com/bufbuild/connect-go"
 )
 
 const (
@@ -164,21 +167,46 @@ func NewTemplateNotFoundError(owner string, name string) error {
 // Note that this function will wrap the error so that the underlying error
 // can be recovered via 'errors.Is'.
 func wrapError(err error) error {
-	if err == nil || (err.Error() == "" && !rpc.IsError(err)) {
-		// If the error is nil or empty and not an rpc error, we return it as-is.
-		// This is especially relevant for commands like lint and breaking.
+	if err == nil {
+		return nil
+	}
+	connectErr, ok := asConnectError(err)
+
+	// If error is empty and not a Connect error, we return it as-is.
+	if !ok && err.Error() == "" {
 		return err
 	}
-	rpcCode := rpc.GetErrorCode(err)
-	switch {
-	case rpcCode == rpc.ErrorCodeUnauthenticated, isEmptyUnknownError(err):
-		return errors.New(`Failure: you are not authenticated. Create a new entry in your netrc, using a Buf API Key as the password. For details, visit https://docs.buf.build/bsr/authentication`)
-	case rpcCode == rpc.ErrorCodeUnavailable:
-		return fmt.Errorf(`Failure: the server hosted at that remote is unavailable: %w`, err)
+	// If the error is a Connect error, then interpret it and return an intuitive message
+	if ok {
+		connectCode := connectErr.Code()
+		switch {
+		case connectCode == connect.CodeUnauthenticated, isEmptyUnknownError(err):
+			return errors.New(`Failure: you are not authenticated. Create a new entry in your netrc, using a Buf API Key as the password. For details, visit https://docs.buf.build/bsr/authentication`)
+		case connectCode == connect.CodeUnavailable:
+			msg := `Failure: the server hosted at that remote is unavailable.`
+			// If the returned error is Unavailable, then determine if this is a DNS error.  If so, get the address used
+			// so that we can display a more helpful error message.
+			if dnsError := (&net.DNSError{}); errors.As(err, &dnsError) && dnsError.IsNotFound {
+				// The subdomain is added internally during transport so trim it off when showing the user the invalid address that they entered
+				return fmt.Errorf(`%s Are you sure "%s" is a valid remote address?`, msg, strings.TrimPrefix(dnsError.Name, buftransport.APISubdomain+"."))
+			}
+
+			return fmt.Errorf(msg)
+		}
+		return fmt.Errorf("Failure: %s", connectErr.Message())
 	}
+
+	// Error was not a Connect error
 	return fmt.Errorf("Failure: %w", err)
 }
 
+// asConnectError uses errors.As to unwrap any error and look for a *connect.Error.
+func asConnectError(err error) (*connect.Error, bool) {
+	var connectErr *connect.Error
+	ok := errors.As(err, &connectErr)
+	return connectErr, ok
+}
+
 // isEmptyUnknownError returns true if the given
 // error is non-nil, but has an empty message
 // and an unknown error code.
@@ -190,5 +218,5 @@ func isEmptyUnknownError(err error) bool {
 	if err == nil {
 		return false
 	}
-	return err.Error() == "" && rpc.GetErrorCode(err) == rpc.ErrorCodeUnknown
+	return err.Error() == "" && connect.CodeOf(err) == connect.CodeUnknown
 }

private/buf/cmd/buf/command/alpha/registry/token/tokencreate/tokencreate.go

@@ -19,6 +19,7 @@ import (
 	"time"
 
 	"github.com/bufbuild/buf/private/buf/bufcli"
+	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleref"
 	"github.com/bufbuild/buf/private/pkg/app/appcmd"
 	"github.com/bufbuild/buf/private/pkg/app/appflag"
 	"github.com/bufbuild/buf/private/pkg/prototime"
@@ -84,8 +85,11 @@ func run(
 ) error {
 	bufcli.WarnAlphaCommand(ctx, container)
 	remote := container.Arg(0)
-	if remote == "" {
-		return appcmd.NewInvalidArgumentError("you must specify a remote module")
+	if err := bufmoduleref.ValidateRemoteNotEmpty(remote); err != nil {
+		return err
+	}
+	if err := bufmoduleref.ValidateRemoteHasNoPaths(remote); err != nil {
+		return err
 	}
 	var expireTime *timestamppb.Timestamp
 	var err error

private/buf/cmd/buf/command/alpha/registry/token/tokendelete/tokendelete.go

@@ -18,9 +18,10 @@ import (
 	"context"
 
 	"github.com/bufbuild/buf/private/buf/bufcli"
+	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleref"
 	"github.com/bufbuild/buf/private/pkg/app/appcmd"
 	"github.com/bufbuild/buf/private/pkg/app/appflag"
-	"github.com/bufbuild/buf/private/pkg/rpc"
+	"github.com/bufbuild/connect-go"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -82,8 +83,11 @@ func run(
 ) error {
 	bufcli.WarnAlphaCommand(ctx, container)
 	remote := container.Arg(0)
-	if remote == "" {
-		return appcmd.NewInvalidArgumentError("you must specify a remote module")
+	if err := bufmoduleref.ValidateRemoteNotEmpty(remote); err != nil {
+		return err
+	}
+	if err := bufmoduleref.ValidateRemoteHasNoPaths(remote); err != nil {
+		return err
 	}
 	registryProvider, err := bufcli.NewRegistryProvider(ctx, container)
 	if err != nil {
@@ -99,7 +103,7 @@ func run(
 		}
 	}
 	if err := service.DeleteToken(ctx, flags.TokenID); err != nil {
-		if rpc.GetErrorCode(err) == rpc.ErrorCodeNotFound {
+		if connect.CodeOf(err) == connect.CodeNotFound {
 			return bufcli.NewTokenNotFoundError(flags.TokenID)
 		}
 		return err

private/buf/cmd/buf/command/alpha/registry/token/tokenget/tokenget.go

@@ -20,9 +20,10 @@ import (
 
 	"github.com/bufbuild/buf/private/buf/bufcli"
 	"github.com/bufbuild/buf/private/buf/bufprint"
+	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleref"
 	"github.com/bufbuild/buf/private/pkg/app/appcmd"
 	"github.com/bufbuild/buf/private/pkg/app/appflag"
-	"github.com/bufbuild/buf/private/pkg/rpc"
+	"github.com/bufbuild/connect-go"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -84,8 +85,11 @@ func run(
 ) error {
 	bufcli.WarnAlphaCommand(ctx, container)
 	remote := container.Arg(0)
-	if remote == "" {
-		return appcmd.NewInvalidArgumentError("you must specify a remote module")
+	if err := bufmoduleref.ValidateRemoteNotEmpty(remote); err != nil {
+		return err
+	}
+	if err := bufmoduleref.ValidateRemoteHasNoPaths(remote); err != nil {
+		return err
 	}
 	format, err := bufprint.ParseFormat(flags.Format)
 	if err != nil {
@@ -101,7 +105,7 @@ func run(
 	}
 	token, err := service.GetToken(ctx, flags.TokenID)
 	if err != nil {
-		if rpc.GetErrorCode(err) == rpc.ErrorCodeNotFound {
+		if connect.CodeOf(err) == connect.CodeNotFound {
 			return bufcli.NewTokenNotFoundError(flags.TokenID)
 		}
 		return err

private/buf/cmd/buf/command/alpha/registry/token/tokenlist/tokenlist.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/bufbuild/buf/private/buf/bufcli"
 	"github.com/bufbuild/buf/private/buf/bufprint"
+	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleref"
 	"github.com/bufbuild/buf/private/pkg/app/appcmd"
 	"github.com/bufbuild/buf/private/pkg/app/appflag"
 	"github.com/spf13/cobra"
@@ -95,8 +96,11 @@ func run(
 ) (retErr error) {
 	bufcli.WarnAlphaCommand(ctx, container)
 	remote := container.Arg(0)
-	if remote == "" {
-		return appcmd.NewInvalidArgumentError("you must specify a remote module")
+	if err := bufmoduleref.ValidateRemoteNotEmpty(remote); err != nil {
+		return err
+	}
+	if err := bufmoduleref.ValidateRemoteHasNoPaths(remote); err != nil {
+		return err
 	}
 	format, err := bufprint.ParseFormat(flags.Format)
 	if err != nil {

private/buf/cmd/buf/command/beta/registry/commit/commitget/commitget.go

@@ -23,7 +23,7 @@ import (
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleref"
 	"github.com/bufbuild/buf/private/pkg/app/appcmd"
 	"github.com/bufbuild/buf/private/pkg/app/appflag"
-	"github.com/bufbuild/buf/private/pkg/rpc"
+	"github.com/bufbuild/connect-go"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -97,7 +97,7 @@ func run(
 		moduleReference.Reference(),
 	)
 	if err != nil {
-		if rpc.GetErrorCode(err) == rpc.ErrorCodeNotFound {
+		if connect.CodeOf(err) == connect.CodeNotFound {
 			return bufcli.NewModuleReferenceNotFoundError(moduleReference)
 		}
 		return err

private/buf/cmd/buf/command/beta/registry/commit/commitlist/commitlist.go

@@ -23,7 +23,7 @@ import (
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduleref"
 	"github.com/bufbuild/buf/private/pkg/app/appcmd"
 	"github.com/bufbuild/buf/private/pkg/app/appflag"
-	"github.com/bufbuild/buf/private/pkg/rpc"
+	"github.com/bufbuild/connect-go"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 )
@@ -129,7 +129,7 @@ func run(
 		flags.Reverse,
 	)
 	if err != nil {
-		if rpc.GetErrorCode(err) == rpc.ErrorCodeNotFound {
+		if connect.CodeOf(err) == connect.CodeNotFound {
 			return bufcli.NewModuleReferenceNotFoundError(moduleReference)
 		}
 		return err