Skip to content

Commit 8f184b4

Browse files
committed
resolving comments
1 parent c3355c8 commit 8f184b4

9 files changed

Lines changed: 154 additions & 99 deletions

File tree

internal/envconfig/envconfig.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ var (
7878
// - The DNS resolver is being used.
7979
EnableDefaultPortForProxyTarget = boolFromEnv("GRPC_EXPERIMENTAL_ENABLE_DEFAULT_PORT_FOR_PROXY_TARGET", true)
8080

81-
// XDSAuthorityRewrite is set if xDS authority rewriting is enabled,
82-
// according to gRFC A81. It can be enabled by setting
83-
// GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE to true.
81+
// XDSAuthorityRewrite indicates whether xDS authority rewriting is enabled.
82+
// This feature is defined in gRFC A81 and is enabled by setting the
83+
// environment variable GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE to "true".
8484
XDSAuthorityRewrite = boolFromEnv("GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE", false)
8585
)
8686

internal/xds/xdsclient/xdsresource/filter_chain.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,7 @@ func processNetworkFilters(filters []*v3listenerpb.Filter) (*FilterChain, error)
680680
// server-side." - A36
681681
// Can specify v3 here, as will never get to this function
682682
// if v2.
683-
routeU, err := generateRDSUpdateFromRouteConfiguration(nil, hcm.GetRouteConfig())
683+
routeU, err := generateRDSUpdateFromRouteConfiguration(hcm.GetRouteConfig(), nil)
684684
if err != nil {
685685
return nil, fmt.Errorf("failed to parse inline RDS resp: %v", err)
686686
}

internal/xds/xdsclient/xdsresource/listener_resource_type.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ type listenerResourceDecoder struct {
3535
bootstrapConfig *bootstrap.Config
3636
}
3737

38-
func (d *listenerResourceDecoder) Decode(resource *xdsclient.AnyProto, _ xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) {
39-
name, listener, err := unmarshalListenerResource(resource.ToAny())
38+
func (d *listenerResourceDecoder) Decode(resource *xdsclient.AnyProto, opts xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) {
39+
name, listener, err := unmarshalListenerResource(resource.ToAny(), &opts)
4040
if name == "" {
4141
// Name is unset only when protobuf deserialization fails.
4242
return nil, err

internal/xds/xdsclient/xdsresource/route_config_resource_type.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ type routeConfigResourceDecoder struct {
3838
}
3939

4040
func (d *routeConfigResourceDecoder) Decode(resource *xdsclient.AnyProto, opts xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) {
41-
name, rc, err := unmarshalRouteConfigResource(&opts, resource.ToAny())
41+
name, rc, err := unmarshalRouteConfigResource(resource.ToAny(), &opts)
4242
if name == "" {
4343
// Name is unset only when protobuf deserialization fails.
4444
return nil, err

internal/xds/xdsclient/xdsresource/unmarshal_eds_test.go

Lines changed: 101 additions & 55 deletions
Large diffs are not rendered by default.

internal/xds/xdsclient/xdsresource/unmarshal_lds.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,13 @@ import (
2727
v3listenerpb "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
2828
v3routepb "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
2929
v3httppb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
30+
"google.golang.org/grpc/internal/xds/clients/xdsclient"
3031
"google.golang.org/grpc/internal/xds/httpfilter"
3132
"google.golang.org/protobuf/proto"
3233
"google.golang.org/protobuf/types/known/anypb"
3334
)
3435

35-
func unmarshalListenerResource(r *anypb.Any) (string, ListenerUpdate, error) {
36+
func unmarshalListenerResource(r *anypb.Any, opts *xdsclient.DecodeOptions) (string, ListenerUpdate, error) {
3637
r, err := UnwrapResource(r)
3738
if err != nil {
3839
return "", ListenerUpdate{}, fmt.Errorf("failed to unwrap resource: %v", err)
@@ -46,24 +47,24 @@ func unmarshalListenerResource(r *anypb.Any) (string, ListenerUpdate, error) {
4647
return "", ListenerUpdate{}, fmt.Errorf("failed to unmarshal resource: %v", err)
4748
}
4849

49-
lu, err := processListener(lis)
50+
lu, err := processListener(lis, opts)
5051
if err != nil {
5152
return lis.GetName(), ListenerUpdate{}, err
5253
}
5354
lu.Raw = r
5455
return lis.GetName(), *lu, nil
5556
}
5657

57-
func processListener(lis *v3listenerpb.Listener) (*ListenerUpdate, error) {
58+
func processListener(lis *v3listenerpb.Listener, opts *xdsclient.DecodeOptions) (*ListenerUpdate, error) {
5859
if lis.GetApiListener() != nil {
59-
return processClientSideListener(lis)
60+
return processClientSideListener(lis, opts)
6061
}
6162
return processServerSideListener(lis)
6263
}
6364

6465
// processClientSideListener checks if the provided Listener proto meets
6566
// the expected criteria. If so, it returns a non-empty routeConfigName.
66-
func processClientSideListener(lis *v3listenerpb.Listener) (*ListenerUpdate, error) {
67+
func processClientSideListener(lis *v3listenerpb.Listener, opts *xdsclient.DecodeOptions) (*ListenerUpdate, error) {
6768
update := &ListenerUpdate{}
6869

6970
apiLisAny := lis.GetApiListener().GetApiListener()
@@ -95,7 +96,7 @@ func processClientSideListener(lis *v3listenerpb.Listener) (*ListenerUpdate, err
9596
}
9697
update.RouteConfigName = name
9798
case *v3httppb.HttpConnectionManager_RouteConfig:
98-
routeU, err := generateRDSUpdateFromRouteConfiguration(nil, apiLis.GetRouteConfig())
99+
routeU, err := generateRDSUpdateFromRouteConfiguration(apiLis.GetRouteConfig(), opts)
99100
if err != nil {
100101
return nil, fmt.Errorf("failed to parse inline RDS resp: %v", err)
101102
}

internal/xds/xdsclient/xdsresource/unmarshal_lds_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) {
580580

581581
for _, test := range tests {
582582
t.Run(test.name, func(t *testing.T) {
583-
name, update, err := unmarshalListenerResource(test.resource)
583+
name, update, err := unmarshalListenerResource(test.resource, nil)
584584
if (err != nil) != test.wantErr {
585585
t.Errorf("unmarshalListenerResource(%s), got err: %v, wantErr: %v", pretty.ToJSON(test.resource), err, test.wantErr)
586586
}
@@ -1707,7 +1707,7 @@ func (s) TestUnmarshalListener_ServerSide(t *testing.T) {
17071707

17081708
for _, test := range tests {
17091709
t.Run(test.name, func(t *testing.T) {
1710-
name, update, err := unmarshalListenerResource(test.resource)
1710+
name, update, err := unmarshalListenerResource(test.resource, nil)
17111711
if err != nil && !strings.Contains(err.Error(), test.wantErr) {
17121712
t.Errorf("unmarshalListenerResource(%s) = %v wantErr: %q", pretty.ToJSON(test.resource), err, test.wantErr)
17131713
}

internal/xds/xdsclient/xdsresource/unmarshal_rds.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import (
3636
v3typepb "github.com/envoyproxy/go-control-plane/envoy/type/v3"
3737
)
3838

39-
func unmarshalRouteConfigResource(opts *xdsclient.DecodeOptions, r *anypb.Any) (string, RouteConfigUpdate, error) {
39+
func unmarshalRouteConfigResource(r *anypb.Any, opts *xdsclient.DecodeOptions) (string, RouteConfigUpdate, error) {
4040
r, err := UnwrapResource(r)
4141
if err != nil {
4242
return "", RouteConfigUpdate{}, fmt.Errorf("failed to unwrap resource: %v", err)
@@ -50,7 +50,7 @@ func unmarshalRouteConfigResource(opts *xdsclient.DecodeOptions, r *anypb.Any) (
5050
return "", RouteConfigUpdate{}, fmt.Errorf("failed to unmarshal resource: %v", err)
5151
}
5252

53-
u, err := generateRDSUpdateFromRouteConfiguration(opts, rc)
53+
u, err := generateRDSUpdateFromRouteConfiguration(rc, opts)
5454
if err != nil {
5555
return rc.GetName(), RouteConfigUpdate{}, err
5656
}
@@ -74,7 +74,7 @@ func unmarshalRouteConfigResource(opts *xdsclient.DecodeOptions, r *anypb.Any) (
7474
// field must be empty and whose route field must be set. Inside that route
7575
// message, the cluster field will contain the clusterName or weighted clusters
7676
// we are looking for.
77-
func generateRDSUpdateFromRouteConfiguration(opts *xdsclient.DecodeOptions, rc *v3routepb.RouteConfiguration) (RouteConfigUpdate, error) {
77+
func generateRDSUpdateFromRouteConfiguration(rc *v3routepb.RouteConfiguration, opts *xdsclient.DecodeOptions) (RouteConfigUpdate, error) {
7878
vhs := make([]*VirtualHost, 0, len(rc.GetVirtualHosts()))
7979
csps, err := processClusterSpecifierPlugins(rc.ClusterSpecifierPlugins)
8080
if err != nil {
@@ -85,7 +85,7 @@ func generateRDSUpdateFromRouteConfiguration(opts *xdsclient.DecodeOptions, rc *
8585
// ignored and not emitted by the xdsclient.
8686
var cspNames = make(map[string]bool)
8787
for _, vh := range rc.GetVirtualHosts() {
88-
routes, cspNs, err := routesProtoToSlice(opts, vh.Routes, csps)
88+
routes, cspNs, err := routesProtoToSlice(vh.Routes, csps, opts)
8989
if err != nil {
9090
return RouteConfigUpdate{}, fmt.Errorf("received route is invalid: %v", err)
9191
}
@@ -208,7 +208,7 @@ func generateRetryConfig(rp *v3routepb.RetryPolicy) (*RetryConfig, error) {
208208
return cfg, nil
209209
}
210210

211-
func routesProtoToSlice(opts *xdsclient.DecodeOptions, routes []*v3routepb.Route, csps map[string]clusterspecifier.BalancerConfig) ([]*Route, map[string]bool, error) {
211+
func routesProtoToSlice(routes []*v3routepb.Route, csps map[string]clusterspecifier.BalancerConfig, opts *xdsclient.DecodeOptions) ([]*Route, map[string]bool, error) {
212212
var routesRet []*Route
213213
var cspNames = make(map[string]bool)
214214
for _, r := range routes {

internal/xds/xdsclient/xdsresource/unmarshal_rds_test.go

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) {
712712
}
713713
for _, test := range tests {
714714
t.Run(test.name, func(t *testing.T) {
715-
gotUpdate, gotError := generateRDSUpdateFromRouteConfiguration(nil, test.rc)
715+
gotUpdate, gotError := generateRDSUpdateFromRouteConfiguration(test.rc, nil)
716716
if (gotError != nil) != test.wantError ||
717717
!cmp.Equal(gotUpdate, test.wantUpdate, cmpopts.EquateEmpty(),
718718
cmp.Transformer("FilterConfig", func(fc httpfilter.FilterConfig) string {
@@ -730,52 +730,46 @@ func (s) TestGenerateRDSUpdateFromRouteConfigurationWithAutoHostRewrite(t *testi
730730
ldsTarget = "lds.target.good:1111"
731731
)
732732

733-
buildRouteConfig := func() *v3routepb.RouteConfiguration {
734-
return &v3routepb.RouteConfiguration{
735-
Name: "routeName",
736-
VirtualHosts: []*v3routepb.VirtualHost{{
737-
Domains: []string{ldsTarget},
738-
Routes: []*v3routepb.Route{{
739-
Match: &v3routepb.RouteMatch{PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/"}},
740-
Action: &v3routepb.Route_Route{
741-
Route: &v3routepb.RouteAction{
742-
ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName},
743-
HostRewriteSpecifier: &v3routepb.RouteAction_AutoHostRewrite{AutoHostRewrite: &wrapperspb.BoolValue{Value: true}},
744-
},
745-
},
746-
}},
747-
}},
748-
}
749-
}
750-
751733
tests := []struct {
752734
name string
753735
isTrusted bool // Corresponds to ServerConfig
754736
envConfigRewrite bool // Corresponds to envconfig.XDSAuthorityRewrite
737+
autoHostRewrite bool
755738
wantResult bool
756739
}{
757740
{
758741
name: "envConfigOn_Trusted",
759742
isTrusted: true,
760743
envConfigRewrite: true,
744+
autoHostRewrite: true,
761745
wantResult: true,
762746
},
747+
{
748+
name: "envConfigOn_Trusted_AutoHostRewriteFalse",
749+
isTrusted: true,
750+
envConfigRewrite: true,
751+
autoHostRewrite: false,
752+
wantResult: false,
753+
},
763754
{
764755
name: "envConfigOff_Trusted",
765756
isTrusted: true,
766757
envConfigRewrite: false,
758+
autoHostRewrite: true,
767759
wantResult: false,
768760
},
769761
{
770762
name: "envConfigOn_Untrusted",
771763
isTrusted: false,
772764
envConfigRewrite: true,
765+
autoHostRewrite: false,
773766
wantResult: false,
774767
},
775768
{
776769
name: "envConfigOff_Untrusted",
777770
isTrusted: false,
778771
envConfigRewrite: false,
772+
autoHostRewrite: true,
779773
wantResult: false,
780774
},
781775
}
@@ -795,17 +789,31 @@ func (s) TestGenerateRDSUpdateFromRouteConfigurationWithAutoHostRewrite(t *testi
795789
},
796790
}
797791

798-
update, err := generateRDSUpdateFromRouteConfiguration(opts, buildRouteConfig())
792+
routeConfig := &v3routepb.RouteConfiguration{
793+
Name: "routeName",
794+
VirtualHosts: []*v3routepb.VirtualHost{{
795+
Domains: []string{ldsTarget},
796+
Routes: []*v3routepb.Route{{
797+
Match: &v3routepb.RouteMatch{PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/"}},
798+
Action: &v3routepb.Route_Route{
799+
Route: &v3routepb.RouteAction{
800+
ClusterSpecifier: &v3routepb.RouteAction_Cluster{Cluster: clusterName},
801+
HostRewriteSpecifier: &v3routepb.RouteAction_AutoHostRewrite{AutoHostRewrite: &wrapperspb.BoolValue{Value: test.autoHostRewrite}},
802+
},
803+
},
804+
}},
805+
}},
806+
}
807+
808+
update, err := generateRDSUpdateFromRouteConfiguration(routeConfig, opts)
799809
if err != nil {
800810
t.Errorf("generateRDSUpdateFromRouteConfiguration() failed, got : %v, want: <nil>", err)
801811
}
802-
803812
if len(update.VirtualHosts) == 0 || len(update.VirtualHosts[0].Routes) == 0 {
804813
t.Errorf("Unexpected parsed routes from generateRDSUpdateFromRouteConfiguration(), got : 0, want: 1")
805814
}
806815

807816
got := update.VirtualHosts[0].Routes[0].AutoHostRewrite
808-
809817
if got != test.wantResult {
810818
t.Errorf("AutoHostRewrite = %v, want %v", got, test.wantResult)
811819
}
@@ -965,7 +973,7 @@ func (s) TestUnmarshalRouteConfig(t *testing.T) {
965973
}
966974
for _, test := range tests {
967975
t.Run(test.name, func(t *testing.T) {
968-
name, update, err := unmarshalRouteConfigResource(nil, test.resource)
976+
name, update, err := unmarshalRouteConfigResource(test.resource, nil)
969977
if (err != nil) != test.wantErr {
970978
t.Errorf("unmarshalRouteConfigResource(%s), got err: %v, wantErr: %v", pretty.ToJSON(test.resource), err, test.wantErr)
971979
}
@@ -1596,7 +1604,7 @@ func (s) TestRoutesProtoToSlice(t *testing.T) {
15961604
}
15971605
for _, tt := range tests {
15981606
t.Run(tt.name, func(t *testing.T) {
1599-
got, _, err := routesProtoToSlice(nil, tt.routes, nil)
1607+
got, _, err := routesProtoToSlice(tt.routes, nil, nil)
16001608
if (err != nil) != tt.wantErr {
16011609
t.Fatalf("routesProtoToSlice() error = %v, wantErr %v", err, tt.wantErr)
16021610
}

0 commit comments

Comments
 (0)