Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: Correct gRFC A1 implementation #7881

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
619dcd4
Change proxy behaviour
eshitachandwani Dec 3, 2024
65b33bd
change delegating resolver
eshitachandwani Dec 4, 2024
94364e2
Improving code
eshitachandwani Dec 5, 2024
9f6a067
correct import
eshitachandwani Dec 5, 2024
5ce86d0
improve tests
eshitachandwani Dec 5, 2024
fbef3a4
address comments
eshitachandwani Dec 5, 2024
342c332
add warning and httpfunc test
eshitachandwani Dec 5, 2024
8726188
add warning and httpfunc test
eshitachandwani Dec 5, 2024
1cfb089
from delegating_pr
eshitachandwani Dec 7, 2024
32d9de5
improve code
eshitachandwani Dec 8, 2024
34b902f
improve code
eshitachandwani Dec 8, 2024
11c8fb1
improve
eshitachandwani Dec 9, 2024
b4c9980
correct tests
eshitachandwani Dec 18, 2024
93fc3e1
trying tests
eshitachandwani Dec 18, 2024
e22ad1d
tests
eshitachandwani Dec 19, 2024
b515565
test
eshitachandwani Dec 19, 2024
7ae2797
correct test environment
eshitachandwani Dec 20, 2024
5ce4658
Correct pr
eshitachandwani Dec 23, 2024
a292b63
Merge branch 'master' into proxy_pr2
eshitachandwani Dec 23, 2024
4483e95
rebase
eshitachandwani Dec 23, 2024
d4f6215
comment
eshitachandwani Dec 23, 2024
333a68c
proxy testutils refactor
eshitachandwani Dec 23, 2024
cacf058
correct vet.sh
eshitachandwani Dec 23, 2024
7c5b1b3
correct vet
eshitachandwani Dec 23, 2024
cb6b09c
Merge branch 'grpc:master' into proxy_pr2
eshitachandwani Dec 27, 2024
ff05ee3
something
eshitachandwani Dec 27, 2024
104cd18
working tests manual resolver
eshitachandwani Dec 27, 2024
fb23cca
working tests manual resolver
eshitachandwani Dec 27, 2024
ef927ce
test
eshitachandwani Dec 27, 2024
bc5efc3
trying test without manual resolver
eshitachandwani Jan 2, 2025
876f09e
e2e tests
eshitachandwani Jan 3, 2025
3e20010
correct vet
eshitachandwani Jan 3, 2025
6827a62
correct e2e tests
eshitachandwani Jan 3, 2025
9acd8de
correct vet
eshitachandwani Jan 3, 2025
8f5055e
vet
eshitachandwani Jan 3, 2025
bbd7f02
vet
eshitachandwani Jan 3, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clientconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ 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.
opts = append([]DialOption{withDefaultScheme("passthrough")}, opts...)
opts = append([]DialOption{withDefaultScheme("passthrough"), WithTargetResolutionEnabled()}, opts...)
arjan-bal marked this conversation as resolved.
Show resolved Hide resolved
arjan-bal marked this conversation as resolved.
Show resolved Hide resolved
cc, err := NewClient(target, opts...)
if err != nil {
return nil, err
Expand Down
26 changes: 20 additions & 6 deletions dialoptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ type dialOptions struct {
idleTimeout time.Duration
defaultScheme string
maxCallAttempts int
// TargetResolutionEnabled specifies if the target resolution on client is
// enabled even when proxy is enabled.
TargetResolutionEnabled bool
// UseProxy specifies if a proxy should be used.
UseProxy bool
}

// DialOption configures how we set up the connection.
Expand Down Expand Up @@ -377,7 +382,15 @@ func WithInsecure() DialOption {
// later release.
func WithNoProxy() DialOption {
return newFuncDialOption(func(o *dialOptions) {
o.copts.UseProxy = false
arjan-bal marked this conversation as resolved.
Show resolved Hide resolved
o.UseProxy = false
})
}

// WithTargetResolutionEnabled returns a DialOption which enables target
// resolution on client.
func WithTargetResolutionEnabled() DialOption {
arjan-bal marked this conversation as resolved.
Show resolved Hide resolved
return newFuncDialOption(func(o *dialOptions) {
o.TargetResolutionEnabled = true
})
}

Expand Down Expand Up @@ -662,14 +675,15 @@ func defaultDialOptions() dialOptions {
copts: transport.ConnectOptions{
ReadBufferSize: defaultReadBufSize,
WriteBufferSize: defaultWriteBufSize,
UseProxy: true,
UserAgent: grpcUA,
BufferPool: mem.DefaultBufferPool(),
},
bs: internalbackoff.DefaultExponential,
idleTimeout: 30 * time.Minute,
defaultScheme: "dns",
maxCallAttempts: defaultMaxCallAttempts,
bs: internalbackoff.DefaultExponential,
idleTimeout: 30 * time.Minute,
defaultScheme: "dns",
maxCallAttempts: defaultMaxCallAttempts,
UseProxy: true,
TargetResolutionEnabled: false,
}
}

Expand Down
5 changes: 5 additions & 0 deletions internal/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,11 @@ var (
// SetBufferPoolingThresholdForTesting updates the buffer pooling threshold, for
// testing purposes.
SetBufferPoolingThresholdForTesting any // func(int)

// HTTPSProxyFromEnvironmentForTesting returns the URL of the proxy to use
// for testing purposes. It is used to override the `http.ProxyFromEnvironment`
// function for testing purposes.
HTTPSProxyFromEnvironmentForTesting any // func(*http.Request) (*url.URL, error)
arjan-bal marked this conversation as resolved.
Show resolved Hide resolved
)

// HealthChecker defines the signature of the client-side LB channel health
Expand Down
64 changes: 64 additions & 0 deletions internal/proxyattributes/proxyattributes.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
*
* Copyright 2024 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

// Package proxyattributes contains functions for getting and setting proxy
// attributes like the CONNECT address and user info.
package proxyattributes

import (
"net/url"

"google.golang.org/grpc/resolver"
)

type keyType string

const userAndConnectAddrKey = keyType("grpc.resolver.delegatingresolver.userAndConnectAddr")

type attr struct {
user *url.Userinfo
addr string
}

// Populate returns a copy of addr with attributes containing the
// provided user and connect address, which are needed during the CONNECT
// handshake for a proxy connection.
func Populate(resAddr resolver.Address, user *url.Userinfo, addr string) resolver.Address {
resAddr.Attributes = resAddr.Attributes.WithValue(userAndConnectAddrKey, attr{user: user, addr: addr})
return resAddr
}

// ProxyConnectAddr returns the proxy connect address in resolver.Address, or nil
// if not present. The returned data should not be mutated.
func ProxyConnectAddr(addr resolver.Address) string {
attribute := addr.Attributes.Value(userAndConnectAddrKey)
if attribute != nil {
return attribute.(attr).addr
}
return ""
}

// User returns the user info in the resolver.Address, or nil if not present.
// The returned data should not be mutated.
func User(addr resolver.Address) *url.Userinfo {
attribute := addr.Attributes.Value(userAndConnectAddrKey)
if attribute != nil {
return attribute.(attr).user
}
return nil
}
103 changes: 103 additions & 0 deletions internal/proxyattributes/proxyattributes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional: I haven't completed the entire review. If you can test the setting of proxy attributes in an e2e style test, you can get rid of these tests.

*
* Copyright 2024 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

package proxyattributes

import (
"net/url"
"testing"

"google.golang.org/grpc/attributes"
"google.golang.org/grpc/internal/grpctest"
"google.golang.org/grpc/resolver"
)

type s struct {
grpctest.Tester
}

func Test(t *testing.T) {
grpctest.RunSubTests(t, s{})
}

// TestProxyConnectAddr tests ProxyConnectAddr returns the coorect connect
// address in the attribute.
func (s) TestProxyConnectAddr(t *testing.T) {
addr := resolver.Address{
Addr: "test-address",
Attributes: attributes.New(userAndConnectAddrKey, attr{user: nil, addr: "proxy-address"}),
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nix new line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// Validate ProxyConnectAddr returns empty string for missing attributes
if got, want := ProxyConnectAddr(addr), "proxy-address"; got != want {
t.Errorf("Unexpected ConnectAddr proxy atrribute = %v, want : %v", got, want)
}
}

// TestUser tests User returns the correct user in the attribute.
func (s) TestUser(t *testing.T) {
user := url.UserPassword("username", "password")
addr := resolver.Address{
Addr: "test-address",
Attributes: attributes.New(userAndConnectAddrKey, attr{user: user, addr: ""}),
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nix new line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// Validate User returns nil for missing attributes
if got, want := User(addr), user; got != want {
t.Errorf("unexpected User proxy attribute = %v, want %v", got, want)
}
}

// TestEmptyProxyAttribute tests ProxyConnectAddr and User return empty string
// and nil respectively when not set.
func (s) TestEmptyProxyAttribute(t *testing.T) {
addr := resolver.Address{
Addr: "test-address",
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nix new line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// Validate ProxyConnectAddr returns empty string for missing attributes
if got := ProxyConnectAddr(addr); got != "" {
t.Errorf("Unexpected ConnectAddr proxy atrribute = %v, want empty string", got)
}
// Validate User returns nil for missing attributes
if got := User(addr); got != nil {
t.Errorf("unexpected User proxy attribute = %v, want nil", got)
}
}

// TestPopulate tests Populate returns a copy of addr with attributes
// containing correct user and connect address.
func (s) TestPopulate(t *testing.T) {
addr := resolver.Address{
Addr: "test-address",
}
user := url.UserPassword("username", "password")
connectAddr := "proxy-address"

// Call Populate and validate attributes
populatedAddr := Populate(addr, user, connectAddr)

// Verify that the returned address is updated correctly
if got, want := ProxyConnectAddr(populatedAddr), connectAddr; got != want {
t.Errorf("Unexpected ConnectAddr proxy atrribute = %v, want %v", got, want)
}

if got, want := User(populatedAddr), user; got != want {
t.Errorf("unexpected User proxy attribute = %v, want %v", got, want)
}
}
Loading
Loading