Skip to content

Commit

Permalink
Correct pr
Browse files Browse the repository at this point in the history
  • Loading branch information
@eshitachandwani
eshitachandwani committed Dec 23, 2024
1 parent 7ae2797 commit 5ce4658
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 92 deletions.
7 changes: 7 additions & 0 deletions clientconn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,13 @@ func Dial(target string, opts ...DialOption) (*ClientConn, error) {
func DialContext(ctx context.Context, target string, opts ...DialOption) (conn *ClientConn, err error) {
// At the end of this method, we kick the channel out of idle, rather than
// waiting for the first rpc.
//
// WithTargetResolutionEnabled in `grpc.Dial` ensures that it preserves
// behavior: when default scheme passthrough is used, skip hostname
// resolution, when any other scheme like "dns" is used for resolution,
// perform resolution on the client as expected.
opts = append([]DialOption{withDefaultScheme("passthrough"), WithTargetResolutionEnabled()}, opts...)

opts = append([]DialOption{withDefaultScheme("passthrough"), WithTargetResolutionEnabled()}, opts...)
cc, err := NewClient(target, opts...)
if err != nil {
Expand Down
7 changes: 2 additions & 5 deletions dialoptions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,8 @@ type dialOptions struct {
idleTimeout time.Duration
defaultScheme string
maxCallAttempts int
// targetResolutionEnabled indicates whether the client should perform
// target resolution even when a proxy is enabled.
targetResolutionEnabled bool
// useProxy specifies if a proxy should be used.
useProxy bool
targetResolutionEnabled bool // Specifies if client should perform target resolution when proxy is enabled.
useProxy bool // Specifies if a proxy should be used.
}

// DialOption configures how we set up the connection.
Expand Down
15 changes: 9 additions & 6 deletions internal/testutils/proxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ const defaultTestTimeout = 10 * time.Second
// ProxyServer represents a test proxy server.
type ProxyServer struct {
lis net.Listener
in net.Conn // The connection from the client to the proxy.
out net.Conn // The connection from the proxy to the backend.
requestCheck func(*http.Request) error // The function to check the request sent to proxy.
in net.Conn // Connection from the client to the proxy.
out net.Conn // Connection from the proxy to the backend.
requestCheck func(*http.Request) error // Function to check the request sent to proxy.
}

// Stop closes the ProxyServer and its connectionsto client and server.
// Stop closes the ProxyServer and its connections to client and server.
func (p *ProxyServer) Stop() {
p.lis.Close()
if p.in != nil {
Expand All @@ -63,6 +63,7 @@ func NewProxyServer(lis net.Listener, reqCheck func(*http.Request) error, errCh
return
}
p.in = in
// This will be used in tests to check if the proxy server is started.
if proxyStarted != nil {
proxyStarted()
}
Expand All @@ -87,19 +88,21 @@ func NewProxyServer(lis net.Listener, reqCheck func(*http.Request) error, errCh
} else {
out, err = net.Dial("tcp", backendAddr)
}

if err != nil {
errCh <- fmt.Errorf("failed to dial to server: %v", err)
return
}
out.SetDeadline(time.Now().Add(defaultTestTimeout))

// Response OK to client
resp := http.Response{StatusCode: http.StatusOK, Proto: "HTTP/1.0"}
var buf bytes.Buffer
resp.Write(&buf)
p.in.Write(buf.Bytes())
p.out = out
// Perform the proxy function, i.e pass the data from client to server and server to client.

// Perform the proxy function, i.e pass the data from client to server
// and server to client.
go io.Copy(p.in, p.out)
go io.Copy(p.out, p.in)
close(doneCh)
Expand Down
6 changes: 3 additions & 3 deletions internal/transport/proxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ func doHTTPConnectHandshake(ctx context.Context, conn net.Conn, addr resolver.Ad
}
}()

a, _ := proxyattributes.ExtractOptions(addr)
opts, _ := proxyattributes.ExtractOptions(addr)
req := &http.Request{
Method: http.MethodConnect,
URL: &url.URL{Host: a.ConnectAddr},
URL: &url.URL{Host: opts.ConnectAddr},
Header: map[string][]string{"User-Agent": {grpcUA}},
}

if user := a.User; user != (url.Userinfo{}) {
if user := opts.User; user != (url.Userinfo{}) {
u := user.Username()
p, pSet := user.Password()
if !pSet {
Expand Down
1 change: 0 additions & 1 deletion resolver_wrapper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ func (ccr *ccResolverWrapper) start() error {
// The delegating resolver is used unless:
// - A custom dialer is provided via WithContextDialer dialoption.
// - Proxy usage is disabled through WithNoProxy dialoption.
// - Client-side resolution is enforced with WithTargetResolutionEnabled.
// In these cases, the resolver is built based on the scheme of target,
// using the appropriate resolver builder.
if ccr.cc.dopts.copts.Dialer != nil || !ccr.cc.dopts.useProxy {
Expand Down
95 changes: 18 additions & 77 deletions test/proxy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ const (
unresolvedProxyURI = "proxyexample.com"
)

// createAndStartBackendServer creates and starts a test backend server,
// registering a cleanup to stop it.
func createAndStartBackendServer(t *testing.T) string {
t.Helper()
backend := &stubserver.StubServer{
Expand Down Expand Up @@ -121,11 +119,11 @@ func (s) TestGRPCDialWithProxy(t *testing.T) {
os.Setenv("HTTPS_PROXY", pLis.Addr().String())
defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }()

// Use the httpproxy package instead of http.ProxyFromEnvironment because
// the latter reads proxy-related environment variables only once at
// Use the httpproxy package functions instead of `http.ProxyFromEnvironment`
// because the latter reads proxy-related environment variables only once at
// initialization. This behavior causes issues when running multiple tests
// with different proxy configurations, as changes to environment variables
// during tests would be ignored. By using httpproxy.FromEnvironment(), we
// during tests would be ignored. By using `httpproxy.FromEnvironment()`, we
// ensure proxy settings are read dynamically in each test execution.
origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment
delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) {
Expand All @@ -143,6 +141,7 @@ func (s) TestGRPCDialWithProxy(t *testing.T) {
}
defer conn.Close()

// Send an empty RPC to the backend through the proxy.
client := testgrpc.NewTestServiceClient(conn)
if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil {
t.Errorf("EmptyCall failed: %v", err)
Expand Down Expand Up @@ -171,12 +170,6 @@ func (s) TestGRPCDialWithDNSAndProxy(t *testing.T) {
os.Setenv("HTTPS_PROXY", unresolvedProxyURI)
defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }()

// Use the httpproxy package instead of http.ProxyFromEnvironment because
// the latter reads proxy-related environment variables only once at
// initialization. This behavior causes issues when running multiple tests
// with different proxy configurations, as changes to environment variables
// during tests would be ignored. By using httpproxy.FromEnvironment(), we
// ensure proxy settings are read dynamically in each test execution.
origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment
delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
Expand Down Expand Up @@ -206,7 +199,7 @@ func (s) TestGRPCDialWithDNSAndProxy(t *testing.T) {
}
defer conn.Close()

// Send an RPC to the backend through the proxy.
// Send an empty RPC to the backend through the proxy.
client := testgrpc.NewTestServiceClient(conn)
if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil {
t.Errorf("EmptyCall failed: %v", err)
Expand Down Expand Up @@ -235,12 +228,6 @@ func (s) TestGRPCNewClientWithProxy(t *testing.T) {
os.Setenv("HTTPS_PROXY", unresolvedProxyURI)
defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }()

// Use the httpproxy package instead of http.ProxyFromEnvironment because
// the latter reads proxy-related environment variables only once at
// initialization. This behavior causes issues when running multiple tests
// with different proxy configurations, as changes to environment variables
// during tests would be ignored. By using httpproxy.FromEnvironment(), we
// ensure proxy settings are read dynamically in each test execution.
origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment
delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
Expand All @@ -251,13 +238,8 @@ func (s) TestGRPCNewClientWithProxy(t *testing.T) {

// Set up and update a manual resolver for proxy resolution.
proxyResolver := setupDNS(t)
proxyResolver.InitialState(resolver.State{
Addresses: []resolver.Address{
{Addr: pLis.Addr().String()},
},
})
proxyResolver.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: pLis.Addr().String()}}})

// Dial to the proxy server.
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
conn, err := grpc.NewClient(unresolvedTargetURI, grpc.WithTransportCredentials(insecure.NewCredentials()))
Expand All @@ -266,7 +248,7 @@ func (s) TestGRPCNewClientWithProxy(t *testing.T) {
}
defer conn.Close()

// Send an RPC to the backend through the proxy.
// Send an empty RPC to the backend through the proxy.
client := testgrpc.NewTestServiceClient(conn)
if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil {
t.Errorf("EmptyCall failed: %v", err)
Expand All @@ -288,20 +270,13 @@ func (s) TestGRPCNewClientWithProxy(t *testing.T) {
// establishes a connection to the backend server.
func (s) TestGRPCNewClientWithProxyAndCustomResolver(t *testing.T) {
backendAddr := createAndStartBackendServer(t)
// Set up and start the proxy server.
pLis, errCh, doneCh, _ := setupProxy(t, backendAddr, true, requestCheck(backendAddr))

// Overwrite the proxy environment and restore it after the test.
proxyEnv := os.Getenv("HTTPS_PROXY")
os.Setenv("HTTPS_PROXY", unresolvedProxyURI)
defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }()

// Use the httpproxy package instead of http.ProxyFromEnvironment because
// the latter reads proxy-related environment variables only once at
// initialization. This behavior causes issues when running multiple tests
// with different proxy configurations, as changes to environment variables
// during tests would be ignored. By using httpproxy.FromEnvironment(), we
// ensure proxy settings are read dynamically in each test execution.
origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment
delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
Expand All @@ -328,13 +303,13 @@ func (s) TestGRPCNewClientWithProxyAndCustomResolver(t *testing.T) {
}
t.Cleanup(func() { conn.Close() })

// Create a test service client and make an RPC call.
// Send an empty RPC to the backend through the proxy.
client := testgrpc.NewTestServiceClient(conn)
if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil {
t.Errorf("EmptyCall() failed: %v", err)
}

// Check if the proxy encountered any errors.
// Verify if the proxy encountered any errors.
select {
case err := <-errCh:
t.Fatalf("proxy server encountered an error: %v", err)
Expand All @@ -350,20 +325,13 @@ func (s) TestGRPCNewClientWithProxyAndCustomResolver(t *testing.T) {
// HTTP CONNECT request, and establishes a connection to the backend server.
func (s) TestGRPCNewClientWithProxyAndCustomResolverWithEndpoints(t *testing.T) {
backendAddr := createAndStartBackendServer(t)
// Set up and start the proxy server.
pLis, errCh, doneCh, _ := setupProxy(t, backendAddr, true, requestCheck(backendAddr))

// Overwrite the proxy environment and restore it after the test.
proxyEnv := os.Getenv("HTTPS_PROXY")
os.Setenv("HTTPS_PROXY", unresolvedProxyURI)
defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }()

// Use the httpproxy package instead of http.ProxyFromEnvironment because
// the latter reads proxy-related environment variables only once at
// initialization. This behavior causes issues when running multiple tests
// with different proxy configurations, as changes to environment variables
// during tests would be ignored. By using httpproxy.FromEnvironment(), we
// ensure proxy settings are read dynamically in each test execution.
origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment
delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
Expand All @@ -376,6 +344,7 @@ func (s) TestGRPCNewClientWithProxyAndCustomResolverWithEndpoints(t *testing.T)
targetResolver := manual.NewBuilderWithScheme("test")
resolver.Register(targetResolver)
targetResolver.InitialState(resolver.State{Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: backendAddr}}}}})

// Set up and update a manual resolver for proxy resolution.
proxyResolver := setupDNS(t)
proxyResolver.InitialState(resolver.State{Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: pLis.Addr().String()}}}}})
Expand All @@ -389,7 +358,7 @@ func (s) TestGRPCNewClientWithProxyAndCustomResolverWithEndpoints(t *testing.T)
}
t.Cleanup(func() { conn.Close() })

// Create a test service client and make an RPC call.
// Send an empty RPC to the backend through the proxy.
client := testgrpc.NewTestServiceClient(conn)
if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil {
t.Errorf("EmptyCall() failed: %v", err)
Expand Down Expand Up @@ -419,12 +388,6 @@ func (s) TestGRPCNewClientWithProxyAndTargetResolutionEnabled(t *testing.T) {
os.Setenv("HTTPS_PROXY", unresolvedProxyURI)
defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }()

// Use the httpproxy package instead of http.ProxyFromEnvironment because
// the latter reads proxy-related environment variables only once at
// initialization. This behavior causes issues when running multiple tests
// with different proxy configurations, as changes to environment variables
// during tests would be ignored. By using httpproxy.FromEnvironment(), we
// ensure proxy settings are read dynamically in each test execution.
origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment
delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
Expand All @@ -446,12 +409,10 @@ func (s) TestGRPCNewClientWithProxyAndTargetResolutionEnabled(t *testing.T) {
resolver.Register(proxyResolver)
proxyResolver.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: pLis.Addr().String()}}})

// Dial options with target resolution enabled.
dopts := []grpc.DialOption{
grpc.WithTransportCredentials(insecure.NewCredentials()),
grpc.WithTargetResolutionEnabled(),
grpc.WithTargetResolutionEnabled(), // Target resolution on client enabled.
}
// Dial to the proxy server.
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
conn, err := grpc.NewClient(unresolvedTargetURI, dopts...)
Expand All @@ -460,7 +421,7 @@ func (s) TestGRPCNewClientWithProxyAndTargetResolutionEnabled(t *testing.T) {
}
t.Cleanup(func() { conn.Close() })

// Create a test service client and make an RPC call.
// Send an empty RPC to the backend through the proxy.
client := testgrpc.NewTestServiceClient(conn)
if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil {
t.Errorf("EmptyCall() failed: %v", err)
Expand Down Expand Up @@ -491,12 +452,6 @@ func (s) TestGRPCNewClientWithNoProxy(t *testing.T) {
os.Setenv("HTTPS_PROXY", unresolvedProxyURI)
defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }()

// Use the httpproxy package instead of http.ProxyFromEnvironment because
// the latter reads proxy-related environment variables only once at
// initialization. This behavior causes issues when running multiple tests
// with different proxy configurations, as changes to environment variables
// during tests would be ignored. By using httpproxy.FromEnvironment(), we
// ensure proxy settings are read dynamically in each test execution.
origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment
delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
Expand All @@ -508,10 +463,9 @@ func (s) TestGRPCNewClientWithNoProxy(t *testing.T) {
targetResolver := setupDNS(t)
targetResolver.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: backendAddr}}})

// Dial options with proxy explicitly disabled.
dopts := []grpc.DialOption{
grpc.WithTransportCredentials(insecure.NewCredentials()),
grpc.WithNoProxy(), // Disable proxy.
grpc.WithNoProxy(), // Diable proxy.
}
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
Expand All @@ -527,7 +481,7 @@ func (s) TestGRPCNewClientWithNoProxy(t *testing.T) {
t.Errorf("EmptyCall() failed: %v", err)
}

// Verify that the proxy was not dialed.
// Verify that the proxy server was not dialed.
select {
case <-proxyStartedCh:
t.Fatal("unexpected dial to proxy server")
Expand All @@ -552,12 +506,6 @@ func (s) TestGRPCNewClientWithContextDialer(t *testing.T) {
os.Setenv("HTTPS_PROXY", unresolvedProxyURI)
defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }()

// Use the httpproxy package instead of http.ProxyFromEnvironment because
// the latter reads proxy-related environment variables only once at
// initialization. This behavior causes issues when running multiple tests
// with different proxy configurations, as changes to environment variables
// during tests would be ignored. By using httpproxy.FromEnvironment(), we
// ensure proxy settings are read dynamically in each test execution.
origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment
delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
Expand All @@ -566,8 +514,7 @@ func (s) TestGRPCNewClientWithContextDialer(t *testing.T) {
delegatingresolver.HTTPSProxyFromEnvironment = origHTTPSProxyFromEnvironment
}()

// Create a custom dialer that directly dials the backend. We'll use this
// to bypass any proxy logic.
// Create a custom dialer that directly dials the backend.ß
dialerCalled := make(chan struct{})
customDialer := func(_ context.Context, backendAddr string) (net.Conn, error) {
close(dialerCalled)
Expand All @@ -579,7 +526,7 @@ func (s) TestGRPCNewClientWithContextDialer(t *testing.T) {

dopts := []grpc.DialOption{
grpc.WithTransportCredentials(insecure.NewCredentials()),
grpc.WithContextDialer(customDialer),
grpc.WithContextDialer(customDialer), // Use a custom dialer.
}
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
Expand Down Expand Up @@ -646,12 +593,6 @@ func (s) TestBasicAuthInGrpcNewClientWithProxy(t *testing.T) {
os.Setenv("HTTPS_PROXY", user+":"+password+"@"+unresolvedProxyURI)
defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }()

// Use the httpproxy package instead of http.ProxyFromEnvironment because
// the latter reads proxy-related environment variables only once at
// initialization. This behavior causes issues when running multiple tests
// with different proxy configurations, as changes to environment variables
// during tests would be ignored. By using httpproxy.FromEnvironment(), we
// ensure proxy settings are read dynamically in each test execution.
origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment
delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) {
return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
Expand All @@ -676,7 +617,7 @@ func (s) TestBasicAuthInGrpcNewClientWithProxy(t *testing.T) {
}
defer conn.Close()

// Send an RPC to the backend through the proxy.
// Send an empty RPC to the backend through the proxy.
client := testgrpc.NewTestServiceClient(conn)
if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil {
t.Errorf("EmptyCall failed: %v", err)
Expand Down

0 comments on commit 5ce4658

Please sign in to comment.