config: remove retry and RBAC disable via environment variables

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,

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")
 )

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".

test/retry_test.go

@@ -33,22 +33,14 @@ 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"
 	"google.golang.org/grpc/status"
 	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)

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 {
 }
 

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

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()
 

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{

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))
-	}
-}

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 {