From f1b050d56d804b245cab048c2980d32b0eaceb4e Mon Sep 17 00:00:00 2001 From: Cody Oss <6331106+codyoss@users.noreply.github.com> Date: Fri, 30 Aug 2024 12:28:19 -0500 Subject: [PATCH 1/3] fix(auth): make sure quota option takes precedence over env/file (#10797) Also align header keys set from old library. Example: https://togithub.com/googleapis/google-api-go-client/blob/21f10edb1d9d6a3d3baa5b1edcef6588da99def8/transport/grpc/dial.go#L212 Eventually we should consider pushing all quota project resolution down the the credentials package, but for now this will work. Fixes: #10795 --- auth/grpctransport/grpctransport.go | 7 +++- auth/grpctransport/grpctransport_test.go | 50 ++++++++++++++++++++++++ auth/httptransport/httptransport_test.go | 31 +++++++++++++++ auth/httptransport/transport.go | 7 +++- 4 files changed, 91 insertions(+), 4 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index b275e1b15d22..21488e29f0b7 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -40,7 +40,7 @@ const ( // Check env to decide if using google-c2p resolver for DirectPath traffic. enableDirectPathXdsEnvVar = "GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS" - quotaProjectHeaderKey = "X-Goog-User-Project" + quotaProjectHeaderKey = "X-goog-user-project" ) var ( @@ -273,7 +273,10 @@ func dial(ctx context.Context, secure bool, opts *Options) (*grpc.ClientConn, er if metadata == nil { metadata = make(map[string]string, 1) } - metadata[quotaProjectHeaderKey] = qp + // Don't overwrite user specified quota + if _, ok := metadata[quotaProjectHeaderKey]; !ok { + metadata[quotaProjectHeaderKey] = qp + } } grpcOpts = append(grpcOpts, grpc.WithPerRPCCredentials(&grpcCredentialsProvider{ diff --git a/auth/grpctransport/grpctransport_test.go b/auth/grpctransport/grpctransport_test.go index 78e1b84f4aea..a3251852fadc 100644 --- a/auth/grpctransport/grpctransport_test.go +++ b/auth/grpctransport/grpctransport_test.go @@ -411,6 +411,56 @@ func TestGRPCKeyProvider_GetRequestMetadata(t *testing.T) { } } +func TestNewClient_QuotaPrecedence(t *testing.T) { + testQuota := "testquotaWins" + t.Setenv(internal.QuotaProjectEnvVar, "testquotaLoses") + l, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatal(err) + } + gsrv := grpc.NewServer() + defer gsrv.Stop() + echo.RegisterEchoerServer(gsrv, &fakeEchoService{ + Fn: func(ctx context.Context, _ *echo.EchoRequest) (*echo.EchoReply, error) { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + t.Error("unable to extract metadata") + return nil, errors.New("oops") + } + if got := md.Get(quotaProjectHeaderKey); len(got) != 1 || got[0] != testQuota { + t.Errorf("got %q, want %q", got, testQuota) + } + return &echo.EchoReply{}, nil + }, + }) + go func() { + if err := gsrv.Serve(l); err != nil { + panic(err) + } + }() + + pool, err := Dial(context.Background(), false, &Options{ + Metadata: map[string]string{quotaProjectHeaderKey: "testquotaWins"}, + InternalOptions: &InternalOptions{ + DefaultEndpointTemplate: l.Addr().String(), + }, + DetectOpts: &credentials.DetectOptions{ + Audience: l.Addr().String(), + CredentialsFile: "../internal/testdata/sa_universe_domain.json", + UseSelfSignedJWT: true, + }, + GRPCDialOpts: []grpc.DialOption{grpc.WithTransportCredentials(insecure.NewCredentials())}, + UniverseDomain: "example.com", // Also configured in sa_universe_domain.json + }) + if err != nil { + t.Fatalf("NewClient() = %v", err) + } + client := echo.NewEchoerClient(pool) + if _, err := client.Echo(context.Background(), &echo.EchoRequest{}); err != nil { + t.Fatalf("client.Echo() = %v", err) + } +} + type staticTP struct { tok *auth.Token } diff --git a/auth/httptransport/httptransport_test.go b/auth/httptransport/httptransport_test.go index 85c908fb0ef3..381f0652cf09 100644 --- a/auth/httptransport/httptransport_test.go +++ b/auth/httptransport/httptransport_test.go @@ -372,6 +372,37 @@ func TestNewClient_APIKey(t *testing.T) { } } +func TestNewClient_QuotaPrecedence(t *testing.T) { + testQuota := "testquotaWins" + t.Setenv(internal.QuotaProjectEnvVar, "testquotaLoses") + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get(quotaProjectHeaderKey); got != testQuota { + t.Errorf("got %q, want %q", got, testQuota) + } + })) + defer ts.Close() + + h := make(http.Header) + h.Set(quotaProjectHeaderKey, testQuota) + client, err := NewClient(&Options{ + InternalOptions: &InternalOptions{ + DefaultEndpointTemplate: ts.URL, + }, + DetectOpts: &credentials.DetectOptions{ + Audience: ts.URL, + CredentialsFile: "../internal/testdata/sa.json", + UseSelfSignedJWT: true, + }, + Headers: h, + }) + if err != nil { + t.Fatalf("NewClient() = %v", err) + } + if _, err := client.Get(ts.URL); err != nil { + t.Fatalf("client.Get() = %v", err) + } +} + func TestNewClient_BaseRoundTripper(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { got := r.Header.Get("Foo") diff --git a/auth/httptransport/transport.go b/auth/httptransport/transport.go index 414b9bfde9b9..274bb01254c9 100644 --- a/auth/httptransport/transport.go +++ b/auth/httptransport/transport.go @@ -31,7 +31,7 @@ import ( ) const ( - quotaProjectHeaderKey = "X-Goog-User-Project" + quotaProjectHeaderKey = "X-goog-user-project" ) func newTransport(base http.RoundTripper, opts *Options) (http.RoundTripper, error) { @@ -76,7 +76,10 @@ func newTransport(base http.RoundTripper, opts *Options) (http.RoundTripper, err if headers == nil { headers = make(map[string][]string, 1) } - headers.Set(quotaProjectHeaderKey, qp) + // Don't overwrite user specified quota + if v := headers.Get(quotaProjectHeaderKey); v == "" { + headers.Set(quotaProjectHeaderKey, qp) + } } creds.TokenProvider = auth.NewCachedTokenProvider(creds.TokenProvider, nil) trans = &authTransport{ From 16317a81f6dd7f3b55befb21ab9bd46943c61f18 Mon Sep 17 00:00:00 2001 From: "release-please[bot]" <55107282+release-please[bot]@users.noreply.github.com> Date: Fri, 30 Aug 2024 13:36:08 -0400 Subject: [PATCH 2/3] chore(main): release auth 0.9.2 (#10754) --- .release-please-manifest-individual.json | 2 +- auth/CHANGES.md | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/.release-please-manifest-individual.json b/.release-please-manifest-individual.json index 6a506dcf4896..63d16efb19d3 100644 --- a/.release-please-manifest-individual.json +++ b/.release-please-manifest-individual.json @@ -1,7 +1,7 @@ { "ai": "0.8.2", "aiplatform": "1.68.0", - "auth": "0.9.1", + "auth": "0.9.2", "auth/oauth2adapt": "0.2.4", "bigquery": "1.62.0", "bigtable": "1.31.0", diff --git a/auth/CHANGES.md b/auth/CHANGES.md index ea6df0cafa7a..b1ca0d02f6ef 100644 --- a/auth/CHANGES.md +++ b/auth/CHANGES.md @@ -1,5 +1,18 @@ # Changelog +## [0.9.2](https://github.com/googleapis/google-cloud-go/compare/auth/v0.9.1...auth/v0.9.2) (2024-08-30) + + +### Bug Fixes + +* **auth:** Handle non-Transport DefaultTransport ([#10733](https://github.com/googleapis/google-cloud-go/issues/10733)) ([98d91dc](https://github.com/googleapis/google-cloud-go/commit/98d91dc8316b247498fab41ab35e57a0446fe556)), refs [#10742](https://github.com/googleapis/google-cloud-go/issues/10742) +* **auth:** Make sure quota option takes precedence over env/file ([#10797](https://github.com/googleapis/google-cloud-go/issues/10797)) ([f1b050d](https://github.com/googleapis/google-cloud-go/commit/f1b050d56d804b245cab048c2980d32b0eaceb4e)), refs [#10795](https://github.com/googleapis/google-cloud-go/issues/10795) + + +### Documentation + +* **auth:** Fix Go doc comment link ([#10751](https://github.com/googleapis/google-cloud-go/issues/10751)) ([015acfa](https://github.com/googleapis/google-cloud-go/commit/015acfab4d172650928bb1119bc2cd6307b9a437)) + ## [0.9.1](https://github.com/googleapis/google-cloud-go/compare/auth/v0.9.0...auth/v0.9.1) (2024-08-22) From 633dc86f615ee32c1ad1ec704bf6bc858a4d535c Mon Sep 17 00:00:00 2001 From: Ron Gal <125445217+ron-gal@users.noreply.github.com> Date: Fri, 30 Aug 2024 14:23:25 -0400 Subject: [PATCH 3/3] test(bigtable): Fix TestIntegration_Aggregates (#10789) * Update integration_test.go * Update integration_test.go * Update integration_test.go * Update inmem.go --------- Co-authored-by: Baha Aiman --- bigtable/bttest/inmem.go | 2 +- bigtable/integration_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bigtable/bttest/inmem.go b/bigtable/bttest/inmem.go index b2da28599ebf..26f672c2af89 100644 --- a/bigtable/bttest/inmem.go +++ b/bigtable/bttest/inmem.go @@ -339,7 +339,7 @@ func (s *server) ModifyColumnFamilies(ctx context.Context, req *btapb.ModifyColu return nil, fmt.Errorf("no such family %q", mod.Id) } if cf.valueType != newcf.valueType { - return nil, status.Errorf(codes.InvalidArgument, "Immutable fields 'value_type' cannot be updated") + return nil, status.Errorf(codes.InvalidArgument, "Immutable fields 'value_type.aggregate_type' cannot be updated") } // assume that we ALWAYS want to replace by the new setting diff --git a/bigtable/integration_test.go b/bigtable/integration_test.go index 692a4dae9312..98d106f19942 100644 --- a/bigtable/integration_test.go +++ b/bigtable/integration_test.go @@ -347,11 +347,11 @@ func TestIntegration_Aggregates(t *testing.T) { t.Fatalf("Read row mismatch.\n got %#v\nwant %#v", row, wantRow) } - err = ac.UpdateFamily(ctx, tableName, family, Family{ValueType: Int64Type{}}) + err = ac.UpdateFamily(ctx, tableName, family, Family{ValueType: StringType{}}) if err == nil { t.Fatalf("Expected UpdateFamily to fail, but it didn't") } - wantError := "Immutable fields 'value_type' cannot be updated" + wantError := "Immutable fields 'value_type.aggregate_type' cannot be updated" if !strings.Contains(err.Error(), wantError) { t.Errorf("Wrong error. Expected to containt %q but was %v", wantError, err) }