diff --git a/dialoptions.go b/dialoptions.go index 40d8ba6596ab..0b19764c01c3 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -29,7 +29,6 @@ import ( "google.golang.org/grpc/credentials" "google.golang.org/grpc/internal" internalbackoff "google.golang.org/grpc/internal/backoff" - "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/internal/transport" "google.golang.org/grpc/keepalive" "google.golang.org/grpc/resolver" @@ -580,7 +579,6 @@ func withHealthCheckFunc(f internal.HealthChecker) DialOption { func defaultDialOptions() dialOptions { return dialOptions{ - disableRetry: !envconfig.Retry, healthCheckFunc: internal.HealthCheckFunc, copts: transport.ConnectOptions{ WriteBufferSize: defaultWriteBufSize, diff --git a/internal/envconfig/envconfig.go b/internal/envconfig/envconfig.go index 9f25a67fc6bd..6f0272543110 100644 --- a/internal/envconfig/envconfig.go +++ b/internal/envconfig/envconfig.go @@ -22,20 +22,14 @@ package envconfig import ( "os" "strings" - - xdsenv "google.golang.org/grpc/internal/xds/env" ) const ( prefix = "GRPC_GO_" - retryStr = prefix + "RETRY" txtErrIgnoreStr = prefix + "IGNORE_TXT_ERRORS" ) var ( - // Retry is enabled unless explicitly disabled via "GRPC_GO_RETRY=off" or - // if XDS retry support is explicitly disabled. - Retry = !strings.EqualFold(os.Getenv(retryStr), "off") && xdsenv.RetrySupport // TXTErrIgnore is set if TXT errors should be ignored ("GRPC_GO_IGNORE_TXT_ERRORS" is not "false"). TXTErrIgnore = !strings.EqualFold(os.Getenv(txtErrIgnoreStr), "false") ) diff --git a/internal/xds/env/env.go b/internal/xds/env/env.go index 87d3c2433a4f..235feb8e0154 100644 --- a/internal/xds/env/env.go +++ b/internal/xds/env/env.go @@ -42,8 +42,6 @@ const ( ringHashSupportEnv = "GRPC_XDS_EXPERIMENTAL_ENABLE_RING_HASH" clientSideSecuritySupportEnv = "GRPC_XDS_EXPERIMENTAL_SECURITY_SUPPORT" aggregateAndDNSSupportEnv = "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER" - retrySupportEnv = "GRPC_XDS_EXPERIMENTAL_ENABLE_RETRY" - rbacSupportEnv = "GRPC_XDS_EXPERIMENTAL_RBAC" c2pResolverSupportEnv = "GRPC_EXPERIMENTAL_GOOGLE_C2P_RESOLVER" c2pResolverTestOnlyTrafficDirectorURIEnv = "GRPC_TEST_ONLY_GOOGLE_C2P_RESOLVER_TRAFFIC_DIRECTOR_URI" @@ -80,14 +78,6 @@ var ( // "true". AggregateAndDNSSupportEnv = strings.EqualFold(os.Getenv(aggregateAndDNSSupportEnv), "true") - // RetrySupport indicates whether xDS retry is enabled. - RetrySupport = !strings.EqualFold(os.Getenv(retrySupportEnv), "false") - - // RBACSupport indicates whether xDS configured RBAC HTTP Filter is enabled, - // which can be disabled by setting the environment variable - // "GRPC_XDS_EXPERIMENTAL_RBAC" to "false". - RBACSupport = !strings.EqualFold(os.Getenv(rbacSupportEnv), "false") - // C2PResolverSupport indicates whether support for C2P resolver is enabled. // This can be enabled by setting the environment variable // "GRPC_EXPERIMENTAL_GOOGLE_C2P_RESOLVER" to "true". diff --git a/test/retry_test.go b/test/retry_test.go index 7f068d79f44d..1bd866add606 100644 --- a/test/retry_test.go +++ b/test/retry_test.go @@ -33,7 +33,6 @@ import ( "github.com/golang/protobuf/proto" "google.golang.org/grpc" "google.golang.org/grpc/codes" - "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/internal/stubserver" "google.golang.org/grpc/metadata" "google.golang.org/grpc/stats" @@ -41,14 +40,7 @@ import ( testpb "google.golang.org/grpc/test/grpc_testing" ) -func enableRetry() func() { - old := envconfig.Retry - envconfig.Retry = true - return func() { envconfig.Retry = old } -} - func (s) TestRetryUnary(t *testing.T) { - defer enableRetry()() i := -1 ss := &stubserver.StubServer{ EmptyCallF: func(context.Context, *testpb.Empty) (*testpb.Empty, error) { @@ -116,7 +108,6 @@ func (s) TestRetryUnary(t *testing.T) { } func (s) TestRetryThrottling(t *testing.T) { - defer enableRetry()() i := -1 ss := &stubserver.StubServer{ EmptyCallF: func(context.Context, *testpb.Empty) (*testpb.Empty, error) { @@ -192,7 +183,6 @@ func (s) TestRetryThrottling(t *testing.T) { } func (s) TestRetryStreaming(t *testing.T) { - defer enableRetry()() req := func(b byte) *testpb.StreamingOutputCallRequest { return &testpb.StreamingOutputCallRequest{Payload: &testpb.Payload{Body: []byte{b}}} } @@ -510,7 +500,6 @@ func (*retryStatsHandler) TagConn(ctx context.Context, _ *stats.ConnTagInfo) con func (*retryStatsHandler) HandleConn(context.Context, stats.ConnStats) {} func (s) TestRetryStats(t *testing.T) { - defer enableRetry()() lis, err := net.Listen("tcp", "localhost:0") if err != nil { t.Fatalf("Failed to listen. Err: %v", err) diff --git a/xds/internal/httpfilter/rbac/rbac.go b/xds/internal/httpfilter/rbac/rbac.go index e92e2e64421b..0ba54a18df67 100644 --- a/xds/internal/httpfilter/rbac/rbac.go +++ b/xds/internal/httpfilter/rbac/rbac.go @@ -28,7 +28,6 @@ import ( "github.com/golang/protobuf/proto" "github.com/golang/protobuf/ptypes" "google.golang.org/grpc/internal/resolver" - "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/internal/xds/rbac" "google.golang.org/grpc/xds/internal/httpfilter" "google.golang.org/protobuf/types/known/anypb" @@ -38,27 +37,9 @@ import ( ) func init() { - if env.RBACSupport { - httpfilter.Register(builder{}) - } -} - -// RegisterForTesting registers the RBAC HTTP Filter for testing purposes, regardless -// of the RBAC environment variable. This is needed because there is no way to set the RBAC -// environment variable to true in a test before init() in this package is run. -func RegisterForTesting() { httpfilter.Register(builder{}) } -// UnregisterForTesting unregisters the RBAC HTTP Filter for testing purposes. This is needed because -// there is no way to unregister the HTTP Filter after registering it solely for testing purposes using -// rbac.RegisterForTesting() -func UnregisterForTesting() { - for _, typeURL := range builder.TypeURLs(builder{}) { - httpfilter.UnregisterForTesting(typeURL) - } -} - type builder struct { } diff --git a/xds/internal/server/listener_wrapper.go b/xds/internal/server/listener_wrapper.go index 99c9a7532307..0d1173324bb6 100644 --- a/xds/internal/server/listener_wrapper.go +++ b/xds/internal/server/listener_wrapper.go @@ -35,7 +35,6 @@ import ( internalbackoff "google.golang.org/grpc/internal/backoff" internalgrpclog "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/internal/grpcsync" - "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" ) @@ -273,9 +272,6 @@ func (l *listenerWrapper) Accept() (net.Conn, error) { conn.Close() continue } - if !env.RBACSupport { - return &connWrapper{Conn: conn, filterChain: fc, parent: l}, nil - } var rc xdsclient.RouteConfigUpdate if fc.InlineRouteConfig != nil { rc = *fc.InlineRouteConfig @@ -414,10 +410,8 @@ func (l *listenerWrapper) handleLDSUpdate(update ldsUpdateWithError) { // Server's state to ServingModeNotServing. That prevents new connections // from being accepted, whereas here we simply want the clients to reconnect // to get the updated configuration. - if env.RBACSupport { - if l.drainCallback != nil { - l.drainCallback(l.Listener.Addr()) - } + if l.drainCallback != nil { + l.drainCallback(l.Listener.Addr()) } l.rdsHandler.updateRouteNamesToWatch(ilc.FilterChains.RouteConfigNames) // If there are no dynamic RDS Configurations still needed to be received diff --git a/xds/internal/server/listener_wrapper_test.go b/xds/internal/server/listener_wrapper_test.go index 383729363665..010b2044d405 100644 --- a/xds/internal/server/listener_wrapper_test.go +++ b/xds/internal/server/listener_wrapper_test.go @@ -34,7 +34,6 @@ import ( wrapperspb "github.com/golang/protobuf/ptypes/wrappers" "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/internal/testutils" - "google.golang.org/grpc/internal/xds/env" _ "google.golang.org/grpc/xds/internal/httpfilter/router" "google.golang.org/grpc/xds/internal/testutils/e2e" "google.golang.org/grpc/xds/internal/testutils/fakeclient" @@ -326,11 +325,6 @@ func (s) TestNewListenerWrapper(t *testing.T) { // the update from the rds handler should it move the server to // ServingModeServing. func (s) TestNewListenerWrapperWithRouteUpdate(t *testing.T) { - oldRBAC := env.RBACSupport - env.RBACSupport = true - defer func() { - env.RBACSupport = oldRBAC - }() _, readyCh, xdsC, _, cleanup := newListenerWrapper(t) defer cleanup() diff --git a/xds/internal/test/xds_client_integration_test.go b/xds/internal/test/xds_client_integration_test.go index e26e3e08f4c9..6a9a8c9688f0 100644 --- a/xds/internal/test/xds_client_integration_test.go +++ b/xds/internal/test/xds_client_integration_test.go @@ -32,7 +32,6 @@ import ( "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/internal/stubserver" "google.golang.org/grpc/internal/testutils" - "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/status" "google.golang.org/grpc/xds/internal/testutils/e2e" @@ -105,11 +104,6 @@ func (s) TestClientSideXDS(t *testing.T) { } func (s) TestClientSideRetry(t *testing.T) { - if !env.RetrySupport { - // Skip this test if retry is not enabled. - return - } - ctr := 0 errs := []codes.Code{codes.ResourceExhausted} ss := &stubserver.StubServer{ diff --git a/xds/internal/test/xds_server_integration_test.go b/xds/internal/test/xds_server_integration_test.go index 6641a678db3c..7decbc05cc57 100644 --- a/xds/internal/test/xds_server_integration_test.go +++ b/xds/internal/test/xds_server_integration_test.go @@ -34,10 +34,8 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/internal/testutils" - "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/status" "google.golang.org/grpc/xds" - "google.golang.org/grpc/xds/internal/httpfilter/rbac" "google.golang.org/grpc/xds/internal/testutils/e2e" v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" @@ -374,11 +372,6 @@ func (s) TestServerSideXDS_SecurityConfigChange(t *testing.T) { // (NonForwardingAction), and the RPC's matching those routes should proceed as // normal. func (s) TestServerSideXDS_RouteConfiguration(t *testing.T) { - oldRBAC := env.RBACSupport - env.RBACSupport = true - defer func() { - env.RBACSupport = oldRBAC - }() managementServer, nodeID, bootstrapContents, resolver, cleanup1 := setupManagementServer(t) defer cleanup1() @@ -722,13 +715,6 @@ func serverListenerWithRBACHTTPFilters(host string, port uint32, rbacCfg *rpb.RB // as normal and certain RPC's are denied by the RBAC HTTP Filter which gets // called by hooked xds interceptors. func (s) TestRBACHTTPFilter(t *testing.T) { - oldRBAC := env.RBACSupport - env.RBACSupport = true - defer func() { - env.RBACSupport = oldRBAC - }() - rbac.RegisterForTesting() - defer rbac.UnregisterForTesting() tests := []struct { name string rbacCfg *rpb.RBAC @@ -967,18 +953,6 @@ func (s) TestRBACHTTPFilter(t *testing.T) { if _, err := client.UnaryCall(ctx, &testpb.SimpleRequest{}); status.Code(err) != test.wantStatusUnaryCall { t.Fatalf("UnaryCall() returned err with status: %v, wantStatusUnaryCall: %v", err, test.wantStatusUnaryCall) } - - // Toggle the RBAC Env variable off, this should disable RBAC and allow any RPC"s through (will not go through - // routing or processed by HTTP Filters and thus will never get denied by RBAC). - env.RBACSupport = false - if _, err := client.EmptyCall(ctx, &testpb.Empty{}); status.Code(err) != codes.OK { - t.Fatalf("EmptyCall() returned err with status: %v, once RBAC is disabled all RPC's should proceed as normal", status.Code(err)) - } - if _, err := client.UnaryCall(ctx, &testpb.SimpleRequest{}); status.Code(err) != codes.OK { - t.Fatalf("UnaryCall() returned err with status: %v, once RBAC is disabled all RPC's should proceed as normal", status.Code(err)) - } - // Toggle RBAC back on for next iterations. - env.RBACSupport = true }() }) } @@ -1102,13 +1076,6 @@ func serverListenerWithBadRouteConfiguration(host string, port uint32) *v3listen } func (s) TestRBACToggledOn_WithBadRouteConfiguration(t *testing.T) { - // Turn RBAC support on. - oldRBAC := env.RBACSupport - env.RBACSupport = true - defer func() { - env.RBACSupport = oldRBAC - }() - managementServer, nodeID, bootstrapContents, resolver, cleanup1 := setupManagementServer(t) defer cleanup1() @@ -1157,60 +1124,3 @@ func (s) TestRBACToggledOn_WithBadRouteConfiguration(t *testing.T) { t.Fatalf("UnaryCall() returned err with status: %v, if RBAC is disabled all RPC's should proceed as normal", status.Code(err)) } } - -func (s) TestRBACToggledOff_WithBadRouteConfiguration(t *testing.T) { - // Turn RBAC support off. - oldRBAC := env.RBACSupport - env.RBACSupport = false - defer func() { - env.RBACSupport = oldRBAC - }() - - managementServer, nodeID, bootstrapContents, resolver, cleanup1 := setupManagementServer(t) - defer cleanup1() - - lis, cleanup2 := setupGRPCServer(t, bootstrapContents) - defer cleanup2() - - host, port, err := hostPortFromListener(lis) - if err != nil { - t.Fatalf("failed to retrieve host and port of server: %v", err) - } - const serviceName = "my-service-fallback" - - // The inbound listener needs a route table that will never match on a VH, - // and thus shouldn't allow incoming RPC's to proceed. - resources := e2e.DefaultClientResources(e2e.ResourceParams{ - DialTarget: serviceName, - NodeID: nodeID, - Host: host, - Port: port, - SecLevel: e2e.SecurityLevelNone, - }) - // This bad route configuration shouldn't affect incoming RPC's from - // proceeding as normal, as the configuration shouldn't be parsed due to the - // RBAC Environment variable not being set to true. - inboundLis := serverListenerWithBadRouteConfiguration(host, port) - resources.Listeners = append(resources.Listeners, inboundLis) - - ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) - defer cancel() - // Setup the management server with client and server-side resources. - if err := managementServer.Update(ctx, resources); err != nil { - t.Fatal(err) - } - - cc, err := grpc.DialContext(ctx, fmt.Sprintf("xds:///%s", serviceName), grpc.WithInsecure(), grpc.WithResolvers(resolver)) - if err != nil { - t.Fatalf("failed to dial local test server: %v", err) - } - defer cc.Close() - - client := testpb.NewTestServiceClient(cc) - if _, err := client.EmptyCall(ctx, &testpb.Empty{}); status.Code(err) != codes.OK { - t.Fatalf("EmptyCall() returned err with status: %v, if RBAC is disabled all RPC's should proceed as normal", status.Code(err)) - } - if _, err := client.UnaryCall(ctx, &testpb.SimpleRequest{}); status.Code(err) != codes.OK { - t.Fatalf("UnaryCall() returned err with status: %v, if RBAC is disabled all RPC's should proceed as normal", status.Code(err)) - } -} diff --git a/xds/internal/xdsclient/filter_chain.go b/xds/internal/xdsclient/filter_chain.go index 7503e0e48761..cb8db0ce54ab 100644 --- a/xds/internal/xdsclient/filter_chain.go +++ b/xds/internal/xdsclient/filter_chain.go @@ -29,7 +29,6 @@ import ( "github.com/golang/protobuf/proto" "github.com/golang/protobuf/ptypes" "google.golang.org/grpc/internal/resolver" - "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/xds/internal/httpfilter" "google.golang.org/grpc/xds/internal/version" ) @@ -611,9 +610,6 @@ func processNetworkFilters(filters []*v3listenerpb.Filter) (*FilterChain, error) // TODO: Implement terminal filter logic, as per A36. filterChain.HTTPFilters = filters seenHCM = true - if !env.RBACSupport { - continue - } switch hcm.RouteSpecifier.(type) { case *v3httppb.HttpConnectionManager_Rds: if hcm.GetRds().GetConfigSource().GetAds() == nil { diff --git a/xds/internal/xdsclient/filter_chain_test.go b/xds/internal/xdsclient/filter_chain_test.go index ae1035e76409..433fc2d341fa 100644 --- a/xds/internal/xdsclient/filter_chain_test.go +++ b/xds/internal/xdsclient/filter_chain_test.go @@ -40,7 +40,6 @@ import ( iresolver "google.golang.org/grpc/internal/resolver" "google.golang.org/grpc/internal/testutils" - "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/xds/internal/httpfilter" "google.golang.org/grpc/xds/internal/httpfilter/router" "google.golang.org/grpc/xds/internal/testutils/e2e" @@ -520,11 +519,6 @@ func TestNewFilterChainImpl_Failure_BadSecurityConfig(t *testing.T) { // TestNewFilterChainImpl_Success_RouteUpdate tests the construction of the // filter chain with valid HTTP Filters present. func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { - oldRBAC := env.RBACSupport - env.RBACSupport = true - defer func() { - env.RBACSupport = oldRBAC - }() tests := []struct { name string lis *v3listenerpb.Listener @@ -760,11 +754,6 @@ func TestNewFilterChainImpl_Success_RouteUpdate(t *testing.T) { // TestNewFilterChainImpl_Failure_BadRouteUpdate verifies cases where the Route // Update in the filter chain are invalid. func TestNewFilterChainImpl_Failure_BadRouteUpdate(t *testing.T) { - oldRBAC := env.RBACSupport - env.RBACSupport = true - defer func() { - env.RBACSupport = oldRBAC - }() tests := []struct { name string lis *v3listenerpb.Listener @@ -972,11 +961,6 @@ func TestNewFilterChainImpl_Failure_BadHTTPFilters(t *testing.T) { // TestNewFilterChainImpl_Success_HTTPFilters tests the construction of the // filter chain with valid HTTP Filters present. func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { - oldRBAC := env.RBACSupport - env.RBACSupport = true - defer func() { - env.RBACSupport = oldRBAC - }() tests := []struct { name string lis *v3listenerpb.Listener @@ -1295,11 +1279,6 @@ func TestNewFilterChainImpl_Success_HTTPFilters(t *testing.T) { // TestNewFilterChainImpl_Success_SecurityConfig verifies cases where the // security configuration in the filter chain contains valid data. func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { - oldRBAC := env.RBACSupport - env.RBACSupport = true - defer func() { - env.RBACSupport = oldRBAC - }() tests := []struct { desc string lis *v3listenerpb.Listener @@ -1527,11 +1506,6 @@ func TestNewFilterChainImpl_Success_SecurityConfig(t *testing.T) { // success at config validation time and the filter chains which contains // unsupported match fields will be skipped at lookup time. func TestNewFilterChainImpl_Success_UnsupportedMatchFields(t *testing.T) { - oldRBAC := env.RBACSupport - env.RBACSupport = true - defer func() { - env.RBACSupport = oldRBAC - }() unspecifiedEntry := &destPrefixEntry{ srcTypeArr: [3]*sourcePrefixes{ { @@ -1697,11 +1671,6 @@ func TestNewFilterChainImpl_Success_UnsupportedMatchFields(t *testing.T) { // TestNewFilterChainImpl_Success_AllCombinations verifies different // combinations of the supported match criteria. func TestNewFilterChainImpl_Success_AllCombinations(t *testing.T) { - oldRBAC := env.RBACSupport - env.RBACSupport = true - defer func() { - env.RBACSupport = oldRBAC - }() tests := []struct { desc string lis *v3listenerpb.Listener @@ -2348,11 +2317,6 @@ func TestLookup_Failures(t *testing.T) { } func TestLookup_Successes(t *testing.T) { - oldRBAC := env.RBACSupport - env.RBACSupport = true - defer func() { - env.RBACSupport = oldRBAC - }() lisWithDefaultChain := &v3listenerpb.Listener{ FilterChains: []*v3listenerpb.FilterChain{ { diff --git a/xds/internal/xdsclient/lds_test.go b/xds/internal/xdsclient/lds_test.go index f889e380eab3..74d3ab07044d 100644 --- a/xds/internal/xdsclient/lds_test.go +++ b/xds/internal/xdsclient/lds_test.go @@ -28,7 +28,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "google.golang.org/grpc/internal/testutils" - "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/xds/internal/httpfilter" _ "google.golang.org/grpc/xds/internal/httpfilter/router" "google.golang.org/grpc/xds/internal/testutils/e2e" @@ -699,11 +698,6 @@ func (s) TestUnmarshalListener_ClientSide(t *testing.T) { } func (s) TestUnmarshalListener_ServerSide(t *testing.T) { - oldRBAC := env.RBACSupport - env.RBACSupport = true - defer func() { - env.RBACSupport = oldRBAC - }() const ( v3LDSTarget = "grpc/server?xds.resource.listening_address=0.0.0.0:9999" testVersion = "test-version-lds-server" diff --git a/xds/internal/xdsclient/rds_test.go b/xds/internal/xdsclient/rds_test.go index 8b419244d672..c865bf36cfa7 100644 --- a/xds/internal/xdsclient/rds_test.go +++ b/xds/internal/xdsclient/rds_test.go @@ -100,10 +100,6 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) { } } goodUpdateWithRetryPolicy = func(vhrc *RetryConfig, rrc *RetryConfig) RouteConfigUpdate { - if !env.RetrySupport { - vhrc = nil - rrc = nil - } return RouteConfigUpdate{ VirtualHosts: []*VirtualHost{{ Domains: []string{ldsTarget}, @@ -117,13 +113,7 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) { }}, } } - defaultRetryBackoff = RetryBackoff{BaseInterval: 25 * time.Millisecond, MaxInterval: 250 * time.Millisecond} - goodUpdateIfRetryDisabled = func() RouteConfigUpdate { - if env.RetrySupport { - return RouteConfigUpdate{} - } - return goodUpdateWithRetryPolicy(nil, nil) - } + defaultRetryBackoff = RetryBackoff{BaseInterval: 25 * time.Millisecond, MaxInterval: 250 * time.Millisecond} ) tests := []struct { @@ -555,26 +545,26 @@ func (s) TestRDSGenerateRDSUpdateFromRouteConfiguration(t *testing.T) { { name: "bad-retry-policy-0-retries", rc: goodRouteConfigWithRetryPolicy(&v3routepb.RetryPolicy{RetryOn: "cancelled", NumRetries: &wrapperspb.UInt32Value{Value: 0}}, nil), - wantUpdate: goodUpdateIfRetryDisabled(), - wantError: env.RetrySupport, + wantUpdate: RouteConfigUpdate{}, + wantError: true, }, { name: "bad-retry-policy-0-base-interval", rc: goodRouteConfigWithRetryPolicy(&v3routepb.RetryPolicy{RetryOn: "cancelled", RetryBackOff: &v3routepb.RetryPolicy_RetryBackOff{BaseInterval: durationpb.New(0)}}, nil), - wantUpdate: goodUpdateIfRetryDisabled(), - wantError: env.RetrySupport, + wantUpdate: RouteConfigUpdate{}, + wantError: true, }, { name: "bad-retry-policy-negative-max-interval", rc: goodRouteConfigWithRetryPolicy(&v3routepb.RetryPolicy{RetryOn: "cancelled", RetryBackOff: &v3routepb.RetryPolicy_RetryBackOff{MaxInterval: durationpb.New(-time.Second)}}, nil), - wantUpdate: goodUpdateIfRetryDisabled(), - wantError: env.RetrySupport, + wantUpdate: RouteConfigUpdate{}, + wantError: true, }, { name: "bad-retry-policy-negative-max-interval-no-known-retry-on", rc: goodRouteConfigWithRetryPolicy(&v3routepb.RetryPolicy{RetryOn: "something", RetryBackOff: &v3routepb.RetryPolicy_RetryBackOff{MaxInterval: durationpb.New(-time.Second)}}, nil), - wantUpdate: goodUpdateIfRetryDisabled(), - wantError: env.RetrySupport, + wantUpdate: RouteConfigUpdate{}, + wantError: true, }, } for _, test := range tests { diff --git a/xds/internal/xdsclient/xds.go b/xds/internal/xdsclient/xds.go index 4b4f0680de67..a838eb6b9950 100644 --- a/xds/internal/xdsclient/xds.go +++ b/xds/internal/xdsclient/xds.go @@ -405,7 +405,7 @@ func generateRDSUpdateFromRouteConfiguration(rc *v3routepb.RouteConfiguration, l } func generateRetryConfig(rp *v3routepb.RetryPolicy) (*RetryConfig, error) { - if !env.RetrySupport || rp == nil { + if rp == nil { return nil, nil } diff --git a/xds/server.go b/xds/server.go index b36fa64b5008..33a490957995 100644 --- a/xds/server.go +++ b/xds/server.go @@ -37,7 +37,6 @@ import ( "google.golang.org/grpc/internal/grpcsync" iresolver "google.golang.org/grpc/internal/resolver" "google.golang.org/grpc/internal/transport" - "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" "google.golang.org/grpc/xds/internal/server" @@ -382,10 +381,8 @@ func routeAndProcess(ctx context.Context) error { // xdsUnaryInterceptor is the unary interceptor added to the gRPC server to // perform any xDS specific functionality on unary RPCs. func xdsUnaryInterceptor(ctx context.Context, req interface{}, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp interface{}, err error) { - if env.RBACSupport { - if err := routeAndProcess(ctx); err != nil { - return nil, err - } + if err := routeAndProcess(ctx); err != nil { + return nil, err } return handler(ctx, req) } @@ -393,10 +390,8 @@ func xdsUnaryInterceptor(ctx context.Context, req interface{}, _ *grpc.UnaryServ // xdsStreamInterceptor is the stream interceptor added to the gRPC server to // perform any xDS specific functionality on streaming RPCs. func xdsStreamInterceptor(srv interface{}, ss grpc.ServerStream, _ *grpc.StreamServerInfo, handler grpc.StreamHandler) error { - if env.RBACSupport { - if err := routeAndProcess(ss.Context()); err != nil { - return err - } + if err := routeAndProcess(ss.Context()); err != nil { + return err } return handler(srv, ss) }