From e9212d8a7a5221d49dd4d928eb27f532c6e67f63 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Fri, 10 Nov 2023 00:11:04 +0000 Subject: [PATCH 1/6] feat: carve out experimental universe option This PR adds the first step in plumbing a universe option into clients by first exposing it as part of the option package. It propagates that onto the internal.DialSettings which receives the applied options, but no further. --- internal/settings.go | 1 + option/option.go | 13 +++++++++++++ option/option_test.go | 2 ++ 3 files changed, 16 insertions(+) diff --git a/internal/settings.go b/internal/settings.go index 84f9302dcfa..2dcac2fe96f 100644 --- a/internal/settings.go +++ b/internal/settings.go @@ -55,6 +55,7 @@ type DialSettings struct { EnableDirectPathXds bool EnableNewAuthLibrary bool AllowNonDefaultServiceAccount bool + UniverseDomain string // Google API system parameters. For more information please read: // https://cloud.google.com/apis/docs/system-parameters diff --git a/option/option.go b/option/option.go index b2085a1949a..c882c1eb482 100644 --- a/option/option.go +++ b/option/option.go @@ -343,3 +343,16 @@ func (w *withCreds) Apply(o *internal.DialSettings) { func WithCredentials(creds *google.Credentials) ClientOption { return (*withCreds)(creds) } + +// WithUniverseDomain returns a ClientOption that sets the universe domain. +// +// This is an EXPERIMENTAL API and may be changed or removed in the future. +func WithUniverseDomain(ud string) ClientOption { + return withUniverseDomain(ud) +} + +type withUniverseDomain string + +func (w withUniverseDomain) Apply(o *internal.DialSettings) { + o.UniverseDomain = string(w) +} diff --git a/option/option_test.go b/option/option_test.go index bbd673eb0af..93129307c27 100644 --- a/option/option_test.go +++ b/option/option_test.go @@ -73,6 +73,7 @@ func TestApply(t *testing.T) { WithQuotaProject("user-project"), WithRequestReason("Request Reason"), WithTelemetryDisabled(), + WithUniverseDomain("universe.com"), } var got internal.DialSettings for _, opt := range opts { @@ -91,6 +92,7 @@ func TestApply(t *testing.T) { QuotaProject: "user-project", RequestReason: "Request Reason", TelemetryDisabled: true, + UniverseDomain: "universe.com", } if !cmp.Equal(got, want, cmpopts.IgnoreUnexported(grpc.ClientConn{}), cmpopts.IgnoreFields(google.Credentials{}, "universeDomain")) { t.Errorf(cmp.Diff(got, want, cmpopts.IgnoreUnexported(grpc.ClientConn{}), cmpopts.IgnoreFields(google.Credentials{}, "universeDomain"))) From a49f296c6618f9cd4a0e243942854ee3427c107b Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 16 Nov 2023 00:09:48 +0000 Subject: [PATCH 2/6] add default option as well --- internal/settings.go | 1 + option/option.go | 13 +++++++++++++ option/option_test.go | 28 +++++++++++++++------------- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/internal/settings.go b/internal/settings.go index 2dcac2fe96f..3356fa97b8f 100644 --- a/internal/settings.go +++ b/internal/settings.go @@ -56,6 +56,7 @@ type DialSettings struct { EnableNewAuthLibrary bool AllowNonDefaultServiceAccount bool UniverseDomain string + DefaultUniverseDomain string // Google API system parameters. For more information please read: // https://cloud.google.com/apis/docs/system-parameters diff --git a/option/option.go b/option/option.go index c882c1eb482..dbb16002bbf 100644 --- a/option/option.go +++ b/option/option.go @@ -356,3 +356,16 @@ type withUniverseDomain string func (w withUniverseDomain) Apply(o *internal.DialSettings) { o.UniverseDomain = string(w) } + +// WithDefaultUniverseDomain returns a ClientOption that sets the universe domain. +// +// This is an EXPERIMENTAL API and may be changed or removed in the future. +func WithDefaultUniverseDomain(ud string) ClientOption { + return withDefaultUniverseDomain(ud) +} + +type withDefaultUniverseDomain string + +func (w withDefaultUniverseDomain) Apply(o *internal.DialSettings) { + o.UniverseDomain = string(w) +} diff --git a/option/option_test.go b/option/option_test.go index 93129307c27..df2863df2a6 100644 --- a/option/option_test.go +++ b/option/option_test.go @@ -74,25 +74,27 @@ func TestApply(t *testing.T) { WithRequestReason("Request Reason"), WithTelemetryDisabled(), WithUniverseDomain("universe.com"), + WithDefaultUniverseDomain("googleapis.com"), } var got internal.DialSettings for _, opt := range opts { opt.Apply(&got) } want := internal.DialSettings{ - Scopes: []string{"https://example.com/auth/helloworld", "https://example.com/auth/otherthing"}, - UserAgent: "ua", - Endpoint: "https://example.com:443", - GRPCConn: conn, - Credentials: &google.DefaultCredentials{ProjectID: "p"}, - CredentialsFile: "service-account.json", - CredentialsJSON: []byte(`{some: "json"}`), - APIKey: "api-key", - Audiences: []string{"https://example.com/"}, - QuotaProject: "user-project", - RequestReason: "Request Reason", - TelemetryDisabled: true, - UniverseDomain: "universe.com", + Scopes: []string{"https://example.com/auth/helloworld", "https://example.com/auth/otherthing"}, + UserAgent: "ua", + Endpoint: "https://example.com:443", + GRPCConn: conn, + Credentials: &google.DefaultCredentials{ProjectID: "p"}, + CredentialsFile: "service-account.json", + CredentialsJSON: []byte(`{some: "json"}`), + APIKey: "api-key", + Audiences: []string{"https://example.com/"}, + QuotaProject: "user-project", + RequestReason: "Request Reason", + TelemetryDisabled: true, + UniverseDomain: "universe.com", + DefaultUniverseDomain: "googleapis.com", } if !cmp.Equal(got, want, cmpopts.IgnoreUnexported(grpc.ClientConn{}), cmpopts.IgnoreFields(google.Credentials{}, "universeDomain")) { t.Errorf(cmp.Diff(got, want, cmpopts.IgnoreUnexported(grpc.ClientConn{}), cmpopts.IgnoreFields(google.Credentials{}, "universeDomain"))) From 3a8dca7243265279afba688269889983560ffe5c Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Fri, 17 Nov 2023 03:53:10 +0000 Subject: [PATCH 3/6] fix default universe --- option/option.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/option/option.go b/option/option.go index dbb16002bbf..9b65a53ad2e 100644 --- a/option/option.go +++ b/option/option.go @@ -367,5 +367,5 @@ func WithDefaultUniverseDomain(ud string) ClientOption { type withDefaultUniverseDomain string func (w withDefaultUniverseDomain) Apply(o *internal.DialSettings) { - o.UniverseDomain = string(w) + o.DefaultUniverseDomain = string(w) } From 66320c3af778910435c78043af57bb2c97e0ff1e Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 30 Nov 2023 20:46:39 +0000 Subject: [PATCH 4/6] move default option internal --- option/internaloption/internaloption.go | 13 +++++++++ option/internaloption/internaloption_test.go | 28 ++++++++++++++++++++ option/option_test.go | 28 +++++++++----------- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/option/internaloption/internaloption.go b/option/internaloption/internaloption.go index b2b249eec68..c32f80321f3 100644 --- a/option/internaloption/internaloption.go +++ b/option/internaloption/internaloption.go @@ -126,6 +126,19 @@ func (w withDefaultScopes) Apply(o *internal.DialSettings) { copy(o.DefaultScopes, w) } +// WithDefaultUniverseDomain returns a ClientOption that sets the default universe domain. +// +// It should only be used internally by generated clients. +func WithDefaultUniverseDomain(ud string) option.ClientOption { + return withDefaultUniverseDomain(ud) +} + +type withDefaultUniverseDomain string + +func (w withDefaultUniverseDomain) Apply(o *internal.DialSettings) { + o.DefaultUniverseDomain = string(w) +} + // EnableJwtWithScope returns a ClientOption that specifies if scope can be used // with self-signed JWT. func EnableJwtWithScope() option.ClientOption { diff --git a/option/internaloption/internaloption_test.go b/option/internaloption/internaloption_test.go index 0ad09f8c236..8f322f9815c 100644 --- a/option/internaloption/internaloption_test.go +++ b/option/internaloption/internaloption_test.go @@ -7,9 +7,13 @@ package internaloption import ( "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "golang.org/x/oauth2" "golang.org/x/oauth2/google" "google.golang.org/api/internal" + "google.golang.org/api/option" + "google.golang.org/grpc" ) func TestWithCredentials(t *testing.T) { @@ -27,3 +31,27 @@ func TestWithCredentials(t *testing.T) { t.Errorf("ts.Token() = %q, want %q", tok.AccessToken, "") } } + +func TestDefaultApply(t *testing.T) { + opts := []option.ClientOption{ + WithDefaultEndpoint("https://example.com:443"), + WithDefaultMTLSEndpoint("http://mtls.example.com:445"), + WithDefaultScopes("a"), + WithDefaultUniverseDomain("foo.com"), + WithDefaultAudience("audience"), + } + var got internal.DialSettings + for _, opt := range opts { + opt.Apply(&got) + } + want := internal.DialSettings{ + DefaultScopes: []string{"a"}, + DefaultEndpoint: "https://example.com:443", + DefaultUniverseDomain: "foo.com", + DefaultAudience: "audience", + DefaultMTLSEndpoint: "http://mtls.example.com:445", + } + if !cmp.Equal(got, want, cmpopts.IgnoreUnexported(grpc.ClientConn{}), cmpopts.IgnoreFields(google.Credentials{}, "universeDomain")) { + t.Errorf(cmp.Diff(got, want, cmpopts.IgnoreUnexported(grpc.ClientConn{}), cmpopts.IgnoreFields(google.Credentials{}, "universeDomain"))) + } +} diff --git a/option/option_test.go b/option/option_test.go index df2863df2a6..93129307c27 100644 --- a/option/option_test.go +++ b/option/option_test.go @@ -74,27 +74,25 @@ func TestApply(t *testing.T) { WithRequestReason("Request Reason"), WithTelemetryDisabled(), WithUniverseDomain("universe.com"), - WithDefaultUniverseDomain("googleapis.com"), } var got internal.DialSettings for _, opt := range opts { opt.Apply(&got) } want := internal.DialSettings{ - Scopes: []string{"https://example.com/auth/helloworld", "https://example.com/auth/otherthing"}, - UserAgent: "ua", - Endpoint: "https://example.com:443", - GRPCConn: conn, - Credentials: &google.DefaultCredentials{ProjectID: "p"}, - CredentialsFile: "service-account.json", - CredentialsJSON: []byte(`{some: "json"}`), - APIKey: "api-key", - Audiences: []string{"https://example.com/"}, - QuotaProject: "user-project", - RequestReason: "Request Reason", - TelemetryDisabled: true, - UniverseDomain: "universe.com", - DefaultUniverseDomain: "googleapis.com", + Scopes: []string{"https://example.com/auth/helloworld", "https://example.com/auth/otherthing"}, + UserAgent: "ua", + Endpoint: "https://example.com:443", + GRPCConn: conn, + Credentials: &google.DefaultCredentials{ProjectID: "p"}, + CredentialsFile: "service-account.json", + CredentialsJSON: []byte(`{some: "json"}`), + APIKey: "api-key", + Audiences: []string{"https://example.com/"}, + QuotaProject: "user-project", + RequestReason: "Request Reason", + TelemetryDisabled: true, + UniverseDomain: "universe.com", } if !cmp.Equal(got, want, cmpopts.IgnoreUnexported(grpc.ClientConn{}), cmpopts.IgnoreFields(google.Credentials{}, "universeDomain")) { t.Errorf(cmp.Diff(got, want, cmpopts.IgnoreUnexported(grpc.ClientConn{}), cmpopts.IgnoreFields(google.Credentials{}, "universeDomain"))) From bc27a0e6cc24f1732b289cc858116f0d5397773f Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 30 Nov 2023 20:58:55 +0000 Subject: [PATCH 5/6] redact default from public --- option/option.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/option/option.go b/option/option.go index 9b65a53ad2e..c882c1eb482 100644 --- a/option/option.go +++ b/option/option.go @@ -356,16 +356,3 @@ type withUniverseDomain string func (w withUniverseDomain) Apply(o *internal.DialSettings) { o.UniverseDomain = string(w) } - -// WithDefaultUniverseDomain returns a ClientOption that sets the universe domain. -// -// This is an EXPERIMENTAL API and may be changed or removed in the future. -func WithDefaultUniverseDomain(ud string) ClientOption { - return withDefaultUniverseDomain(ud) -} - -type withDefaultUniverseDomain string - -func (w withDefaultUniverseDomain) Apply(o *internal.DialSettings) { - o.DefaultUniverseDomain = string(w) -} From 76466e701c8752fe732aad1cef883c3e990ba485 Mon Sep 17 00:00:00 2001 From: Seth Hollyman Date: Thu, 30 Nov 2023 21:22:48 +0000 Subject: [PATCH 6/6] address reviewer comment feedback --- option/internaloption/internaloption.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/option/internaloption/internaloption.go b/option/internaloption/internaloption.go index c32f80321f3..3fdee095c1b 100644 --- a/option/internaloption/internaloption.go +++ b/option/internaloption/internaloption.go @@ -129,6 +129,9 @@ func (w withDefaultScopes) Apply(o *internal.DialSettings) { // WithDefaultUniverseDomain returns a ClientOption that sets the default universe domain. // // It should only be used internally by generated clients. +// +// This is similar to the public WithUniverse, but allows us to determine whether the user has +// overridden the default universe. func WithDefaultUniverseDomain(ud string) option.ClientOption { return withDefaultUniverseDomain(ud) }