Skip to content

Commit

Permalink
fix(transport): remove conditional App Engine gen 1 Go hooks (#2158)
Browse files Browse the repository at this point in the history
* Remove gRPC hook for google.golang.org/appengine/socket
* Remove HTTP hook for google.golang.org/appengine/urlfetch

Both hooks load packages that are no longer intended for use in the App Engine Go second generation runtime (>= Go 1.11).

closes: #2128
  • Loading branch information
quartzmo committed Sep 12, 2023
1 parent 2114d8d commit c2fa93e
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 122 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ require (
golang.org/x/net v0.15.0
golang.org/x/oauth2 v0.12.0
golang.org/x/sync v0.3.0
google.golang.org/appengine v1.6.7
google.golang.org/genproto/googleapis/bytestream v0.0.0-20230911183012-2d3300fd4832
google.golang.org/genproto/googleapis/rpc v0.0.0-20230911183012-2d3300fd4832
google.golang.org/grpc v1.57.0
Expand All @@ -27,6 +26,7 @@ require (
golang.org/x/crypto v0.13.0 // indirect
golang.org/x/sys v0.12.0 // indirect
golang.org/x/text v0.13.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20230803162519-f966b187b2e5 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20230803162519-f966b187b2e5 // indirect
)
9 changes: 0 additions & 9 deletions transport/grpc/dial.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ const disableDirectPath = "GOOGLE_CLOUD_DISABLE_DIRECT_PATH"
// Check env to decide if using google-c2p resolver for DirectPath traffic.
const enableDirectPathXds = "GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS"

// Set at init time by dial_appengine.go. If nil, we're not on App Engine.
var appengineDialerHook func(context.Context) grpc.DialOption

// Set at init time by dial_socketopt.go. If nil, socketopt is not supported.
var timeoutDialerOption grpc.DialOption

Expand Down Expand Up @@ -186,12 +183,6 @@ func dial(ctx context.Context, insecure bool, o *internal.DialSettings) (*grpc.C
}
}

if appengineDialerHook != nil {
// Use the Socket API on App Engine.
// appengine dialer will override socketopt dialer
grpcOpts = append(grpcOpts, appengineDialerHook(ctx))
}

// Add tracing, but before the other options, so that clients can override the
// gRPC stats handler.
// This assumes that gRPC options are processed in order, left to right.
Expand Down
32 changes: 0 additions & 32 deletions transport/grpc/dial_appengine.go

This file was deleted.

46 changes: 0 additions & 46 deletions transport/grpc/dial_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,55 +5,9 @@
package grpc

import (
"context"
"errors"
"net"
"testing"
"time"

"golang.org/x/oauth2"
"google.golang.org/api/option"
"google.golang.org/grpc"
)

// Check that user optioned grpc.WithDialer option overwrites App Engine dialer
func TestGRPCHook(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
expected := false

appengineDialerHook = (func(ctx context.Context) grpc.DialOption {
return grpc.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) {
t.Error("did not expect a call to appengine dialer, got one")
cancel()
return nil, errors.New("not expected")
})
})
defer func() {
appengineDialerHook = nil
}()

expectedDialer := grpc.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) {
expected = true
cancel()
return nil, errors.New("expected")
})

conn, err := Dial(ctx,
option.WithTokenSource(oauth2.StaticTokenSource(nil)), // No creds.
option.WithGRPCDialOption(expectedDialer),
option.WithGRPCDialOption(grpc.WithBlock()))
if err != context.Canceled {
t.Errorf("got %v, want %v", err, context.Canceled)
}
if conn != nil {
conn.Close()
t.Error("got valid conn, want nil")
}
if !expected {
t.Error("expected a call to expected dialer, didn't get one")
}
}

func TestCheckDirectPathEndPoint(t *testing.T) {
for _, testcase := range []struct {
name string
Expand Down
17 changes: 4 additions & 13 deletions transport/http/dial.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,22 +145,13 @@ func (t *parameterTransport) RoundTrip(req *http.Request) (*http.Response, error
return rt.RoundTrip(&newReq)
}

// Set at init time by dial_appengine.go. If nil, we're not on App Engine.
var appengineUrlfetchHook func(context.Context) http.RoundTripper

// defaultBaseTransport returns the base HTTP transport.
// On App Engine, this is urlfetch.Transport.
// Otherwise, use a default transport, taking most defaults from
// http.DefaultTransport.
// defaultBaseTransport returns the base HTTP transport. It uses a default
// transport, taking most defaults from http.DefaultTransport.
// If TLSCertificate is available, set TLSClientConfig as well.
func defaultBaseTransport(ctx context.Context, clientCertSource cert.Source, dialTLSContext func(context.Context, string, string) (net.Conn, error)) http.RoundTripper {
if appengineUrlfetchHook != nil {
return appengineUrlfetchHook(ctx)
}

// Copy http.DefaultTransport except for MaxIdleConnsPerHost setting,
// which is increased due to reported performance issues under load in the GCS
// client. Transport.Clone is only available in Go 1.13 and up.
// which is increased due to reported performance issues under load in the
// GCS client. Transport.Clone is only available in Go 1.13 and up.
trans := clonedTransport(http.DefaultTransport)
if trans == nil {
trans = fallbackBaseTransport()
Expand Down
21 changes: 0 additions & 21 deletions transport/http/dial_appengine.go

This file was deleted.

0 comments on commit c2fa93e

Please sign in to comment.