xds: Add env var protection for RBAC HTTP Filter

internal/xds/env/env.go

@@ -43,6 +43,7 @@ const (
 	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_ENABLE_RBAC"
 
 	c2pResolverSupportEnv                    = "GRPC_EXPERIMENTAL_GOOGLE_C2P_RESOLVER"
 	c2pResolverTestOnlyTrafficDirectorURIEnv = "GRPC_TEST_ONLY_GOOGLE_C2P_RESOLVER_TRAFFIC_DIRECTOR_URI"
@@ -82,6 +83,9 @@ var (
 	// RetrySupport indicates whether xDS retry is enabled.
 	RetrySupport = strings.EqualFold(os.Getenv(retrySupportEnv), "true")
 
+	// RBACSupport indicates whether xDS configured RBAC HTTP Filter is enabled.
+	RBACSupport = strings.EqualFold(os.Getenv(rbacSupportEnv), "true")
+
 	// 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".

xds/internal/httpfilter/httpfilter.go

@@ -94,6 +94,11 @@ func Register(b Filter) {
 	}
 }
 
+// UnregisterForTesting unregisters the HTTP Filter for testing purposes.
+func UnregisterForTesting(typeURL string) {
+	delete(m, typeURL)
+}
+
 // Get returns the HTTPFilter registered with typeURL.
 //
 // If no filter is register with typeURL, nil will be returned.

xds/internal/httpfilter/rbac/rbac.go

@@ -28,6 +28,7 @@ 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"
@@ -37,9 +38,27 @@ 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,6 +35,7 @@ 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"
 )
@@ -272,34 +273,37 @@ func (l *listenerWrapper) Accept() (net.Conn, error) {
 			conn.Close()
 			continue
 		}
-		var rc xdsclient.RouteConfigUpdate
-		if fc.InlineRouteConfig != nil {
-			rc = *fc.InlineRouteConfig
-		} else {
-			rcPtr := atomic.LoadPointer(&l.rdsUpdates)
-			rcuPtr := (*map[string]xdsclient.RouteConfigUpdate)(rcPtr)
-			// This shouldn't happen, but this error protects against a panic.
-			if rcuPtr == nil {
-				return nil, errors.New("route configuration pointer is nil")
+		var vhswi []xdsclient.VirtualHostWithInterceptors
+		if env.RBACSupport {
+			var rc xdsclient.RouteConfigUpdate
+			if fc.InlineRouteConfig != nil {
+				rc = *fc.InlineRouteConfig
+			} else {
+				rcPtr := atomic.LoadPointer(&l.rdsUpdates)
+				rcuPtr := (*map[string]xdsclient.RouteConfigUpdate)(rcPtr)
+				// This shouldn't happen, but this error protects against a panic.
+				if rcuPtr == nil {
+					return nil, errors.New("route configuration pointer is nil")
+				}
+				rcu := *rcuPtr
+				rc = rcu[fc.RouteConfigName]
+			}
+			// The filter chain will construct a usuable route table on each
+			// connection accept. This is done because preinstantiating every route
+			// table before it is needed for a connection would potentially lead to
+			// a lot of cpu time and memory allocated for route tables that will
+			// never be used. There was also a thought to cache this configuration,
+			// and reuse it for the next accepted connection. However, this would
+			// lead to a lot of code complexity (RDS Updates for a given route name
+			// can come it at any time), and connections aren't accepted too often,
+			// so this reinstantation of the Route Configuration is an acceptable
+			// tradeoff for simplicity.
+			vhswi, err = fc.ConstructUsableRouteConfiguration(rc)
+			if err != nil {
+				l.logger.Warningf("route configuration construction: %v", err)
+				conn.Close()
+				continue
 			}
-			rcu := *rcuPtr
-			rc = rcu[fc.RouteConfigName]
-		}
-		// The filter chain will construct a usuable route table on each
-		// connection accept. This is done because preinstantiating every route
-		// table before it is needed for a connection would potentially lead to
-		// a lot of cpu time and memory allocated for route tables that will
-		// never be used. There was also a thought to cache this configuration,
-		// and reuse it for the next accepted connection. However, this would
-		// lead to a lot of code complexity (RDS Updates for a given route name
-		// can come it at any time), and connections aren't accepted too often,
-		// so this reinstantation of the Route Configuration is an acceptable
-		// tradeoff for simplicity.
-		vhswi, err := fc.ConstructUsableRouteConfiguration(rc)
-		if err != nil {
-			l.logger.Warningf("route configuration construction: %v", err)
-			conn.Close()
-			continue
 		}
 		return &connWrapper{Conn: conn, filterChain: fc, parent: l, virtualHosts: vhswi}, nil
 	}
@@ -410,8 +414,10 @@ 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 l.drainCallback != nil {
-		l.drainCallback(l.Listener.Addr())
+	if env.RBACSupport {
+		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,6 +34,7 @@ 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"
@@ -325,6 +326,11 @@ 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()