auth: grpc.NewClient dns scheme incompatible with secure forward-proxies

[erezrokah]

Client

cloud.google.com/go/auth

Environment

go version go1.23.2 darwin/arm64

Code and Dependencies

This issue impacts all consumers of the auth package which I believe are all the different packages in this repo and also https://github.com/googleapis/google-api-go-client/blob/3347fa1ef84d6de6d5f0ce651bf6da6577791a9e/transport/grpc/dial.go#L22

go.mod

module modname

go 1.22.4

require (
	cloud.google.com/go/accessapproval v1.8.2
	cloud.google.com/go/aiplatform v1.68.0
	cloud.google.com/go/apigateway v1.7.2
	cloud.google.com/go/apikeys v0.6.0
	cloud.google.com/go/appengine v1.9.2
	cloud.google.com/go/artifactregistry v1.15.2
	cloud.google.com/go/asset v1.20.3
	cloud.google.com/go/baremetalsolution v1.3.2
	cloud.google.com/go/batch v1.11.2
	cloud.google.com/go/beyondcorp v1.1.2
	cloud.google.com/go/bigquery v1.63.1
	cloud.google.com/go/bigtable v1.33.0
	cloud.google.com/go/billing v1.19.2
	cloud.google.com/go/binaryauthorization v1.9.2
	cloud.google.com/go/certificatemanager v1.9.2
	cloud.google.com/go/cloudtasks v1.13.2
	cloud.google.com/go/compute v1.28.2
	cloud.google.com/go/container v1.41.0
	cloud.google.com/go/containeranalysis v0.13.2
	cloud.google.com/go/dataproc/v2 v2.10.0
	cloud.google.com/go/deploy v1.23.1
	cloud.google.com/go/domains v0.10.2
	cloud.google.com/go/errorreporting v0.3.1
	cloud.google.com/go/filestore v1.9.2
	cloud.google.com/go/firestore v1.17.0
	cloud.google.com/go/functions v1.19.2
	cloud.google.com/go/iam v1.2.2
	cloud.google.com/go/kms v1.20.1
	cloud.google.com/go/logging v1.12.0
	cloud.google.com/go/longrunning v0.6.2
	cloud.google.com/go/monitoring v1.21.2
	cloud.google.com/go/networkmanagement v1.15.0
	cloud.google.com/go/networkservices v0.2.2
	cloud.google.com/go/osconfig v1.14.2
	cloud.google.com/go/pubsub v1.45.1
	cloud.google.com/go/redis v1.17.2
	cloud.google.com/go/resourcemanager v1.10.2
	cloud.google.com/go/run v1.6.1
	cloud.google.com/go/scheduler v1.11.2
	cloud.google.com/go/secretmanager v1.14.2
	cloud.google.com/go/securitycenter v1.35.2
	cloud.google.com/go/serviceusage v1.9.2
	cloud.google.com/go/storage v1.45.0
	cloud.google.com/go/storagetransfer v1.11.2
	cloud.google.com/go/trace v1.11.2
	cloud.google.com/go/translate v1.12.2
	cloud.google.com/go/video v1.23.2
	cloud.google.com/go/vision/v2 v2.9.2
	cloud.google.com/go/vmmigration v1.8.2
	cloud.google.com/go/vpcaccess v1.8.2
	cloud.google.com/go/websecurityscanner v1.7.2
	cloud.google.com/go/workflows v1.13.2
	github.com/apache/arrow/go/v17 v17.0.0
	github.com/cdfmlr/ellipsis v0.0.1
	github.com/cloudquery/codegen v0.3.19
	github.com/cloudquery/plugin-sdk/v4 v4.68.2
	github.com/golang/mock v1.6.0
	github.com/googleapis/gax-go/v2 v2.13.0
	github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.1.0
	github.com/iancoleman/strcase v0.3.0
	github.com/invopop/jsonschema v0.12.0
	github.com/julienschmidt/httprouter v1.3.0
	github.com/rs/zerolog v1.33.0
	github.com/spf13/cast v1.5.0
	github.com/stretchr/testify v1.9.0
	github.com/thoas/go-funk v0.9.3
	github.com/wk8/go-ordered-map/v2 v2.1.8
	golang.org/x/exp v0.0.0-20241009180824-f66d83c29e7c
	golang.org/x/sync v0.8.0
	google.golang.org/api v0.203.0
	google.golang.org/genproto v0.0.0-20241021214115-324edc3d5d38
	google.golang.org/grpc v1.67.1
	google.golang.org/protobuf v1.35.1
)

Expected behavior

Resolving scheme should not change without communicating the breaking change

Actual behavior

Resolving scheme changed in a chore: #10780

Additional context

Started after upgrading to v0.9.2.

Setting GOOGLE_API_GO_EXPERIMENTAL_DISABLE_NEW_AUTH_LIB to true seems to fix the issue as it changes the logic here to use DialContext instead of NewClient from the auth package

Related to grpc/grpc-go#7556

Updated

Added a reproduction scenario via https://github.com/cloudquery/gcp-client-breakage

[quartzmo]

@erezrokah Can you please update this issue to include actual program output in the Actual behavior section demonstrating how the change from passthrough to dns is breaking your usage of the library? And if it is not obvious, maybe describe in Expected behavior how your program worked before the change? These sections are not intended for commentary, but instead for concrete demos. Thanks!

[quartzmo]

See discussion and possible workaround in grpc/grpc-go#7556.

[erezrokah]

Thanks for the reply @quartzmo 👍

@erezrokah Can you please update this issue to include actual program output in the Actual behavior section demonstrating how the change from passthrough to dns is breaking your usage of the library? And if it is not obvious, maybe describe in Expected behavior how your program worked before the change? These sections are not intended for commentary, but instead for concrete demos. Thanks!

I'll add sample code but in order to observe the behavior you'd need put up a forward proxy like envoy and see it break.

See discussion and possible workaround in grpc/grpc-go#7556.

Won't this workaround disable DSN resolving altogether even when explicitly set by the scheme?

[quartzmo]

in order to observe the behavior you'd need put up a forward proxy like envoy and see it break.

We are asking you to provide this program output, thank you!

[quartzmo]

Another possible workaround tactic is to use option.WithEndpoint to set the service endpoint with the passthrough scheme. This was done in the handwritten Spanner client for emulator usage in /pull/10947.

[erezrokah]

We are asking you to provide this program output, thank you!

Let me set up a repo that better shows how this is a breaking change and I'll update the issue with it.

Another possible workaround tactic is to use option.WithEndpoint to set the service endpoint with the passthrough scheme. This was done in the handwritten Spanner client for emulator usage in /pull/10947.

Is there a way of doing that without knowing each service endpoint in advance? We're using almost every module in this repo to interact with GCP so adding it manually will require a bit of work, and also we'd have to maintain the list of endpoints up to date

To clarify I'm not asking to revert the change, only a way to get the previous behavior that can be applied generically to all modules

[quartzmo]

To clarify I'm not asking to revert the change, only a way to get the previous behavior that can be applied generically to all modules

Sounds good. I agree that this is really important. As I have been doing, please link any tactical workarounds you come across, as one of these might offer an solution to be used either directly by users, or indirectly via some change to the client libraries or the auth library.

Note that grpc.Dial (where passthrough is the default) has been deprecated, and that there are considerable performance advantages with client-side load balancing in some environments for using its replacement grpc.NewClient (where dns is the default). These were the motivations for the change. We did not immediately see the potential for breakage, apologies for that.

[erezrokah]

Hi @quartzmo I've created a reproduction in the repo here https://github.com/cloudquery/gcp-client-breakage.
It shows how moving to DNS resolution to the client can break existing use cases when the client is behind a proxy with access control that's enforced via the domain of the request.

[quartzmo]

Per the recommendation of @arjan-bal on grpc/grpc-go#7556 (comment), we are in agreement that we should temporarily revert usages of NewClient to Dial in the Go client libraries, pending a fix by gRPC for forward proxies.

[quartzmo]

Closed by #11118