From 8b5fa0a10b96f5776b8c2b1b989a3f76df9612d7 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Tue, 17 Oct 2023 18:35:55 +0000 Subject: [PATCH 1/5] feat(transport): Log DirectPath misconfiguration --- transport/grpc/dial.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/transport/grpc/dial.go b/transport/grpc/dial.go index e36d7589ee2..966ca477689 100644 --- a/transport/grpc/dial.go +++ b/transport/grpc/dial.go @@ -181,6 +181,7 @@ func dial(ctx context.Context, insecure bool, o *internal.DialSettings) (*grpc.C } // TODO(cbro): add support for system parameters (quota project, request reason) via chained interceptor. } + logDirectPathMisconfig(creds.TokenSource, o) } // Add tracing, but before the other options, so that clients can override the @@ -296,6 +297,27 @@ func checkDirectPathEndPoint(endpoint string) bool { return true } +func logDirectPathMisconfig(ts oauth2.TokenSource, o *internal.DialSettings) { + isDirectPathOptionSet := o.EnableDirectPath + isDirectPathXdsEnvSet := strings.EqualFold(os.Getenv(enableDirectPathXds), "true") + isDirectPathXdsOptionSet := o.EnableDirectPathXds + if isDirectPathXdsEnvSet || isDirectPathXdsOptionSet { + // Case 1: does not enable DirectPath + if !isDirectPathOptionSet { + log.Print("WARNING: DirectPath is misconfigured. Please set the attemptDirectPath option along with the attemptDirectPathXds option.") + } else { + // Case 2: credential is not correctly set + if !isTokenSourceDirectPathCompatible(ts, o) { + log.Print("WARNING: DirectPath is misconfigured. Please make sure the token source is fetched from GCE metadata server and the default service account is used") + } + // Case 3: not running on GCE + if !metadata.OnGCE() { + log.Print("WARNING: DirectPath is misconfigured. DirectPath is only available in a GCE environment.") + } + } + } +} + func processAndValidateOpts(opts []option.ClientOption) (*internal.DialSettings, error) { var o internal.DialSettings for _, opt := range opts { From 8ae04cc9e7e73f54d3def5ba51e1a6be1ea7d4c7 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Thu, 19 Oct 2023 17:55:55 +0000 Subject: [PATCH 2/5] feat(transport): Log DirectPath misconfiguration --- transport/grpc/dial.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/transport/grpc/dial.go b/transport/grpc/dial.go index 966ca477689..76dc5ede180 100644 --- a/transport/grpc/dial.go +++ b/transport/grpc/dial.go @@ -153,6 +153,7 @@ func dial(ctx context.Context, insecure bool, o *internal.DialSettings) (*grpc.C ) // Attempt Direct Path: + logDirectPathMisconfig(endpoint, creds.TokenSource, o) if isDirectPathEnabled(endpoint, o) && isTokenSourceDirectPathCompatible(creds.TokenSource, o) && metadata.OnGCE() { // Overwrite all of the previously specific DialOptions, DirectPath uses its own set of credentials and certificates. grpcOpts = []grpc.DialOption{ @@ -181,7 +182,6 @@ func dial(ctx context.Context, insecure bool, o *internal.DialSettings) (*grpc.C } // TODO(cbro): add support for system parameters (quota project, request reason) via chained interceptor. } - logDirectPathMisconfig(creds.TokenSource, o) } // Add tracing, but before the other options, so that clients can override the @@ -297,18 +297,15 @@ func checkDirectPathEndPoint(endpoint string) bool { return true } -func logDirectPathMisconfig(ts oauth2.TokenSource, o *internal.DialSettings) { - isDirectPathOptionSet := o.EnableDirectPath - isDirectPathXdsEnvSet := strings.EqualFold(os.Getenv(enableDirectPathXds), "true") - isDirectPathXdsOptionSet := o.EnableDirectPathXds - if isDirectPathXdsEnvSet || isDirectPathXdsOptionSet { +func logDirectPathMisconfig(endpoint string, ts oauth2.TokenSource, o *internal.DialSettings) { + if isDirectPathXdsUsed(o) { // Case 1: does not enable DirectPath - if !isDirectPathOptionSet { + if !isDirectPathEnabled(endpoint, o) { log.Print("WARNING: DirectPath is misconfigured. Please set the attemptDirectPath option along with the attemptDirectPathXds option.") } else { // Case 2: credential is not correctly set if !isTokenSourceDirectPathCompatible(ts, o) { - log.Print("WARNING: DirectPath is misconfigured. Please make sure the token source is fetched from GCE metadata server and the default service account is used") + log.Print("WARNING: DirectPath is misconfigured. Please make sure the token source is fetched from GCE metadata server and the default service account is used.") } // Case 3: not running on GCE if !metadata.OnGCE() { From 340abd07dd42318be05d65c83340380663cd95f1 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Fri, 27 Oct 2023 19:16:20 +0000 Subject: [PATCH 3/5] feat(transport): Log DirectPath misconfiguration --- transport/grpc/dial.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/transport/grpc/dial.go b/transport/grpc/dial.go index 76dc5ede180..67867146794 100644 --- a/transport/grpc/dial.go +++ b/transport/grpc/dial.go @@ -301,15 +301,15 @@ func logDirectPathMisconfig(endpoint string, ts oauth2.TokenSource, o *internal. if isDirectPathXdsUsed(o) { // Case 1: does not enable DirectPath if !isDirectPathEnabled(endpoint, o) { - log.Print("WARNING: DirectPath is misconfigured. Please set the attemptDirectPath option along with the attemptDirectPathXds option.") + log.Println("WARNING: DirectPath is misconfigured. Please set the EnableDirectPath option along with the EnableDirectPathXds option.") } else { // Case 2: credential is not correctly set if !isTokenSourceDirectPathCompatible(ts, o) { - log.Print("WARNING: DirectPath is misconfigured. Please make sure the token source is fetched from GCE metadata server and the default service account is used.") + log.Println("WARNING: DirectPath is misconfigured. Please make sure the token source is fetched from GCE metadata server and the default service account is used.") } // Case 3: not running on GCE if !metadata.OnGCE() { - log.Print("WARNING: DirectPath is misconfigured. DirectPath is only available in a GCE environment.") + log.Println("WARNING: DirectPath is misconfigured. DirectPath is only available in a GCE environment.") } } } From 8208f7d6c9814822ded83b0d97cd2de55986cc24 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Thu, 2 Nov 2023 20:53:32 +0000 Subject: [PATCH 4/5] feat(transport): Log DirectPath misconfiguration --- go.mod | 1 + go.sum | 2 ++ transport/grpc/dial.go | 9 ++++++++- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 34ed411f9a7..479b0a01b84 100644 --- a/go.mod +++ b/go.mod @@ -26,6 +26,7 @@ require ( golang.org/x/crypto v0.14.0 // indirect golang.org/x/sys v0.13.0 // indirect golang.org/x/text v0.13.0 // indirect + golang.org/x/time v0.3.0 // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/genproto v0.0.0-20231016165738-49dd2c1f3d0b // indirect google.golang.org/genproto/googleapis/api v0.0.0-20231016165738-49dd2c1f3d0b // indirect diff --git a/go.sum b/go.sum index 50a340d87cd..d472493d391 100644 --- a/go.sum +++ b/go.sum @@ -95,6 +95,8 @@ golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k= golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= +golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4= +golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY= diff --git a/transport/grpc/dial.go b/transport/grpc/dial.go index 67867146794..87a22f75863 100644 --- a/transport/grpc/dial.go +++ b/transport/grpc/dial.go @@ -14,10 +14,12 @@ import ( "net" "os" "strings" + "time" "cloud.google.com/go/compute/metadata" "go.opencensus.io/plugin/ocgrpc" "golang.org/x/oauth2" + "golang.org/x/time/rate" "google.golang.org/api/internal" "google.golang.org/api/option" "google.golang.org/grpc" @@ -38,6 +40,9 @@ const enableDirectPathXds = "GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS" // Set at init time by dial_socketopt.go. If nil, socketopt is not supported. var timeoutDialerOption grpc.DialOption +// Log rate limiter +var logRateLimiter = rate.Sometimes{Interval: 1 * time.Second} + // Dial returns a GRPC connection for use communicating with a Google cloud // service, configured with the given ClientOptions. func Dial(ctx context.Context, opts ...option.ClientOption) (*grpc.ClientConn, error) { @@ -153,7 +158,9 @@ func dial(ctx context.Context, insecure bool, o *internal.DialSettings) (*grpc.C ) // Attempt Direct Path: - logDirectPathMisconfig(endpoint, creds.TokenSource, o) + logRateLimiter.Do(func() { + logDirectPathMisconfig(endpoint, creds.TokenSource, o) + }) if isDirectPathEnabled(endpoint, o) && isTokenSourceDirectPathCompatible(creds.TokenSource, o) && metadata.OnGCE() { // Overwrite all of the previously specific DialOptions, DirectPath uses its own set of credentials and certificates. grpcOpts = []grpc.DialOption{ From 2daa2ab6d582e2ceb9101a84e6e242b2fad39773 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Fri, 3 Nov 2023 23:33:53 +0000 Subject: [PATCH 5/5] feat(transport): Log DirectPath misconfiguration --- transport/grpc/dial_test.go | 78 +++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/transport/grpc/dial_test.go b/transport/grpc/dial_test.go index b017cf6be48..34161b77c84 100644 --- a/transport/grpc/dial_test.go +++ b/transport/grpc/dial_test.go @@ -5,7 +5,15 @@ package grpc import ( + "bytes" + "context" + "log" + "strings" "testing" + + "cloud.google.com/go/compute/metadata" + "golang.org/x/oauth2/google" + "google.golang.org/api/internal" ) func TestCheckDirectPathEndPoint(t *testing.T) { @@ -47,3 +55,73 @@ func TestCheckDirectPathEndPoint(t *testing.T) { }) } } + +func TestLogDirectPathMisconfigAttrempDirectPathNotSet(t *testing.T) { + o := &internal.DialSettings{} + o.EnableDirectPathXds = true + + endpoint := "abc.googleapis.com" + + creds, err := internal.Creds(context.Context(context.Background()), o) + if err != nil { + t.Fatalf("failed to create creds") + } + + buf := bytes.Buffer{} + log.SetOutput(&buf) + + logDirectPathMisconfig(endpoint, creds.TokenSource, o) + + wantedLog := "WARNING: DirectPath is misconfigured. Please set the EnableDirectPath option along with the EnableDirectPathXds option." + if !strings.Contains(buf.String(), wantedLog) { + t.Fatalf("got: %v, want: %v", buf.String(), wantedLog) + } + +} + +func TestLogDirectPathMisconfigWrongCredential(t *testing.T) { + o := &internal.DialSettings{} + o.EnableDirectPath = true + o.EnableDirectPathXds = true + + endpoint := "abc.googleapis.com" + + creds := &google.Credentials{} + + buf := bytes.Buffer{} + log.SetOutput(&buf) + + logDirectPathMisconfig(endpoint, creds.TokenSource, o) + + wantedLog := "WARNING: DirectPath is misconfigured. Please make sure the token source is fetched from GCE metadata server and the default service account is used." + if !strings.Contains(buf.String(), wantedLog) { + t.Fatalf("got: %v, want: %v", buf.String(), wantedLog) + } + +} + +func TestLogDirectPathMisconfigNotOnGCE(t *testing.T) { + o := &internal.DialSettings{} + o.EnableDirectPath = true + o.EnableDirectPathXds = true + + endpoint := "abc.googleapis.com" + + creds, err := internal.Creds(context.Context(context.Background()), o) + if err != nil { + t.Fatalf("failed to create creds") + } + + buf := bytes.Buffer{} + log.SetOutput(&buf) + + logDirectPathMisconfig(endpoint, creds.TokenSource, o) + + if !metadata.OnGCE() { + wantedLog := "WARNING: DirectPath is misconfigured. DirectPath is only available in a GCE environment.." + if !strings.Contains(buf.String(), wantedLog) { + t.Fatalf("got: %v, want: %v", buf.String(), wantedLog) + } + } + +}