envconfig: remove env vars for on-by-default features (grpc#6749)

Author: dfawley

internal/envconfig/envconfig.go

@@ -28,17 +28,11 @@ import (
 var (
 	// TXTErrIgnore is set if TXT errors should be ignored ("GRPC_GO_IGNORE_TXT_ERRORS" is not "false").
 	TXTErrIgnore = boolFromEnv("GRPC_GO_IGNORE_TXT_ERRORS", true)
-	// AdvertiseCompressors is set if registered compressor should be advertised
-	// ("GRPC_GO_ADVERTISE_COMPRESSORS" is not "false").
-	AdvertiseCompressors = boolFromEnv("GRPC_GO_ADVERTISE_COMPRESSORS", true)
 	// RingHashCap indicates the maximum ring size which defaults to 4096
 	// entries but may be overridden by setting the environment variable
 	// "GRPC_RING_HASH_CAP".  This does not override the default bounds
 	// checking which NACKs configs specifying ring sizes > 8*1024*1024 (~8M).
 	RingHashCap = uint64FromEnv("GRPC_RING_HASH_CAP", 4096, 1, 8*1024*1024)
-	// PickFirstLBConfig is set if we should support configuration of the
-	// pick_first LB policy.
-	PickFirstLBConfig = boolFromEnv("GRPC_EXPERIMENTAL_PICKFIRST_LB_CONFIG", true)
 	// LeastRequestLB is set if we should support the least_request_experimental
 	// LB policy, which can be enabled by setting the environment variable
 	// "GRPC_EXPERIMENTAL_ENABLE_LEAST_REQUEST" to "true".

internal/envconfig/xds.go

@@ -50,46 +50,7 @@ var (
 	//
 	// When both bootstrap FileName and FileContent are set, FileName is used.
 	XDSBootstrapFileContent = os.Getenv(XDSBootstrapFileContentEnv)
-	// XDSRingHash indicates whether ring hash support is enabled, which can be
-	// disabled by setting the environment variable
-	// "GRPC_XDS_EXPERIMENTAL_ENABLE_RING_HASH" to "false".
-	XDSRingHash = boolFromEnv("GRPC_XDS_EXPERIMENTAL_ENABLE_RING_HASH", true)
-	// XDSClientSideSecurity is used to control processing of security
-	// configuration on the client-side.
-	//
-	// Note that there is no env var protection for the server-side because we
-	// have a brand new API on the server-side and users explicitly need to use
-	// the new API to get security integration on the server.
-	XDSClientSideSecurity = boolFromEnv("GRPC_XDS_EXPERIMENTAL_SECURITY_SUPPORT", true)
-	// XDSAggregateAndDNS indicates whether processing of aggregated cluster and
-	// DNS cluster is enabled, which can be disabled by setting the environment
-	// variable "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"
-	// to "false".
-	XDSAggregateAndDNS = boolFromEnv("GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER", true)
-
-	// XDSRBAC indicates whether xDS configured RBAC HTTP Filter is enabled,
-	// which can be disabled by setting the environment variable
-	// "GRPC_XDS_EXPERIMENTAL_RBAC" to "false".
-	XDSRBAC = boolFromEnv("GRPC_XDS_EXPERIMENTAL_RBAC", true)
-	// XDSOutlierDetection indicates whether outlier detection support is
-	// enabled, which can be disabled by setting the environment variable
-	// "GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION" to "false".
-	XDSOutlierDetection = boolFromEnv("GRPC_EXPERIMENTAL_ENABLE_OUTLIER_DETECTION", true)
-	// XDSFederation indicates whether federation support is enabled, which can
-	// be enabled by setting the environment variable
-	// "GRPC_EXPERIMENTAL_XDS_FEDERATION" to "true".
-	XDSFederation = boolFromEnv("GRPC_EXPERIMENTAL_XDS_FEDERATION", true)
-
-	// XDSRLS indicates whether processing of Cluster Specifier plugins and
-	// support for the RLS CLuster Specifier is enabled, which can be disabled by
-	// setting the environment variable "GRPC_EXPERIMENTAL_XDS_RLS_LB" to
-	// "false".
-	XDSRLS = boolFromEnv("GRPC_EXPERIMENTAL_XDS_RLS_LB", true)
 
 	// C2PResolverTestOnlyTrafficDirectorURI is the TD URI for testing.
 	C2PResolverTestOnlyTrafficDirectorURI = os.Getenv("GRPC_TEST_ONLY_GOOGLE_C2P_RESOLVER_TRAFFIC_DIRECTOR_URI")
-	// XDSCustomLBPolicy indicates whether Custom LB Policies are enabled, which
-	// can be disabled by setting the environment variable
-	// "GRPC_EXPERIMENTAL_XDS_CUSTOM_LB_CONFIG" to "false".
-	XDSCustomLBPolicy = boolFromEnv("GRPC_EXPERIMENTAL_XDS_CUSTOM_LB_CONFIG", true)
 )

internal/grpcutil/compressor.go

@@ -20,8 +20,6 @@ package grpcutil
 
 import (
 	"strings"
-
-	"google.golang.org/grpc/internal/envconfig"
 )
 
 // RegisteredCompressorNames holds names of the registered compressors.
@@ -40,8 +38,5 @@ func IsCompressorNameRegistered(name string) bool {
 // RegisteredCompressors returns a string of registered compressor names
 // separated by comma.
 func RegisteredCompressors() string {
-	if !envconfig.AdvertiseCompressors {
-		return ""
-	}
 	return strings.Join(RegisteredCompressorNames, ",")
 }

internal/grpcutil/compressor_test.go

@@ -20,27 +20,13 @@ package grpcutil
 
 import (
 	"testing"
-
-	"google.golang.org/grpc/internal/envconfig"
 )
 
 func TestRegisteredCompressors(t *testing.T) {
 	defer func(c []string) { RegisteredCompressorNames = c }(RegisteredCompressorNames)
-	defer func(v bool) { envconfig.AdvertiseCompressors = v }(envconfig.AdvertiseCompressors)
 	RegisteredCompressorNames = []string{"gzip", "snappy"}
-	tests := []struct {
-		desc    string
-		enabled bool
-		want    string
-	}{
-		{desc: "compressor_ad_disabled", enabled: false, want: ""},
-		{desc: "compressor_ad_enabled", enabled: true, want: "gzip,snappy"},
-	}
-	for _, tt := range tests {
-		envconfig.AdvertiseCompressors = tt.enabled
-		compressors := RegisteredCompressors()
-		if compressors != tt.want {
-			t.Fatalf("Unexpected compressors got:%s, want:%s", compressors, tt.want)
-		}
+	if got, want := RegisteredCompressors(), "gzip,snappy"; got != want {
+		t.Fatalf("Unexpected compressors got:%s, want:%s", got, want)
+
 	}
 }

internal/testutils/xds/bootstrap/bootstrap.go

@@ -116,12 +116,9 @@ func Contents(opts Options) ([]byte, error) {
 		cfg.XdsServers[0].ServerFeatures = append(cfg.XdsServers[0].ServerFeatures, "ignore_resource_deletion")
 	}
 
-	auths := make(map[string]authority)
-	if envconfig.XDSFederation {
-		// This will end up using the top-level server list for new style
-		// resources with empty authority.
-		auths[""] = authority{}
-	}
+	// This will end up using the top-level server list for new style
+	// resources with empty authority.
+	auths := map[string]authority{"": {}}
 	for n, auURI := range opts.Authorities {
 		auths[n] = authority{XdsServers: []server{{
 			ServerURI:      auURI,

pickfirst.go

@@ -25,7 +25,6 @@ import (
 
 	"google.golang.org/grpc/balancer"
 	"google.golang.org/grpc/connectivity"
-	"google.golang.org/grpc/internal/envconfig"
 	internalgrpclog "google.golang.org/grpc/internal/grpclog"
 	"google.golang.org/grpc/internal/grpcrand"
 	"google.golang.org/grpc/internal/pretty"
@@ -65,19 +64,6 @@ type pfConfig struct {
 }
 
 func (*pickfirstBuilder) ParseConfig(js json.RawMessage) (serviceconfig.LoadBalancingConfig, error) {
-	if !envconfig.PickFirstLBConfig {
-		// Prior to supporting loadbalancing configuration, the pick_first LB
-		// policy did not implement the balancer.ConfigParser interface. This
-		// meant that if a non-empty configuration was passed to it, the service
-		// config unmarshaling code would throw a warning log, but would
-		// continue using the pick_first LB policy. The code below ensures the
-		// same behavior is retained if the env var is not set.
-		if string(js) != "{}" {
-			logger.Warningf("Ignoring non-empty balancer configuration %q for the pick_first LB policy", string(js))
-		}
-		return nil, nil
-	}
-
 	var cfg pfConfig
 	if err := json.Unmarshal(js, &cfg); err != nil {
 		return nil, fmt.Errorf("pickfirst: unable to unmarshal LB policy config: %s, error: %v", string(js), err)

test/compressor_test.go

@@ -31,7 +31,6 @@ import (
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/encoding"
-	"google.golang.org/grpc/internal/envconfig"
 	"google.golang.org/grpc/internal/stubserver"
 	"google.golang.org/grpc/metadata"
 	"google.golang.org/grpc/status"
@@ -408,23 +407,6 @@ func (s) TestUnregisteredSetSendCompressorFailure(t *testing.T) {
 	})
 }
 
-func (s) TestUnadvertisedSetSendCompressorFailure(t *testing.T) {
-	// Disable client compressor advertisement.
-	defer func(b bool) { envconfig.AdvertiseCompressors = b }(envconfig.AdvertiseCompressors)
-	envconfig.AdvertiseCompressors = false
-
-	resCompressor := "gzip"
-	wantErr := status.Error(codes.Unknown, "unable to set send compressor: client does not support compressor \"gzip\"")
-
-	t.Run("unary", func(t *testing.T) {
-		testUnarySetSendCompressorFailure(t, resCompressor, wantErr)
-	})
-
-	t.Run("stream", func(t *testing.T) {
-		testStreamSetSendCompressorFailure(t, resCompressor, wantErr)
-	})
-}
-
 func testUnarySetSendCompressorFailure(t *testing.T, resCompressor string, wantErr error) {
 	ss := &stubserver.StubServer{
 		EmptyCallF: func(ctx context.Context, in *testpb.Empty) (*testpb.Empty, error) {

test/pickfirst_test.go

@@ -33,7 +33,6 @@ import (
 	"google.golang.org/grpc/credentials/insecure"
 	"google.golang.org/grpc/internal"
 	"google.golang.org/grpc/internal/channelz"
-	"google.golang.org/grpc/internal/envconfig"
 	"google.golang.org/grpc/internal/grpcrand"
 	"google.golang.org/grpc/internal/stubserver"
 	"google.golang.org/grpc/internal/testutils"
@@ -376,8 +375,6 @@ func (s) TestPickFirst_StickyTransientFailure(t *testing.T) {
 
 // Tests the PF LB policy with shuffling enabled.
 func (s) TestPickFirst_ShuffleAddressList(t *testing.T) {
-	defer func(old bool) { envconfig.PickFirstLBConfig = old }(envconfig.PickFirstLBConfig)
-	envconfig.PickFirstLBConfig = true
 	const serviceConfig = `{"loadBalancingConfig": [{"pick_first":{ "shuffleAddressList": true }}]}`
 
 	// Install a shuffler that always reverses two entries.
@@ -429,45 +426,6 @@ func (s) TestPickFirst_ShuffleAddressList(t *testing.T) {
 	}
 }
 
-// Tests the PF LB policy with the environment variable support of address list
-// shuffling disabled.
-func (s) TestPickFirst_ShuffleAddressListDisabled(t *testing.T) {
-	defer func(old bool) { envconfig.PickFirstLBConfig = old }(envconfig.PickFirstLBConfig)
-	envconfig.PickFirstLBConfig = false
-	const serviceConfig = `{"loadBalancingConfig": [{"pick_first":{ "shuffleAddressList": true }}]}`
-
-	// Install a shuffler that always reverses two entries.
-	origShuf := grpcrand.Shuffle
-	defer func() { grpcrand.Shuffle = origShuf }()
-	grpcrand.Shuffle = func(n int, f func(int, int)) {
-		if n != 2 {
-			t.Errorf("Shuffle called with n=%v; want 2", n)
-			return
-		}
-		f(0, 1) // reverse the two addresses
-	}
-
-	// Set up our backends.
-	cc, r, backends := setupPickFirst(t, 2)
-	addrs := stubBackendsToResolverAddrs(backends)
-
-	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
-	defer cancel()
-
-	// Send a config with shuffling enabled.  This will reverse the addresses,
-	// so we should connect to backend 1 if shuffling is supported.  However
-	// with it disabled at the start of the test, we will connect to backend 0
-	// instead.
-	shufState := resolver.State{
-		ServiceConfig: parseServiceConfig(t, r, serviceConfig),
-		Addresses:     []resolver.Address{addrs[0], addrs[1]},
-	}
-	r.UpdateState(shufState)
-	if err := pickfirst.CheckRPCsToBackend(ctx, cc, addrs[0]); err != nil {
-		t.Fatal(err)
-	}
-}
-
 // Test config parsing with the env var turned on and off for various scenarios.
 func (s) TestPickFirst_ParseConfig_Success(t *testing.T) {
 	// Install a shuffler that always reverses two entries.
@@ -483,49 +441,23 @@ func (s) TestPickFirst_ParseConfig_Success(t *testing.T) {
 
 	tests := []struct {
 		name          string
-		envVar        bool
 		serviceConfig string
 		wantFirstAddr bool
 	}{
 		{
-			name:          "env var disabled with empty pickfirst config",
-			envVar:        false,
-			serviceConfig: `{"loadBalancingConfig": [{"pick_first":{}}]}`,
-			wantFirstAddr: true,
-		},
-		{
-			name:          "env var disabled with non-empty good pickfirst config",
-			envVar:        false,
-			serviceConfig: `{"loadBalancingConfig": [{"pick_first":{ "shuffleAddressList": true }}]}`,
-			wantFirstAddr: true,
-		},
-		{
-			name:          "env var disabled with non-empty bad pickfirst config",
-			envVar:        false,
-			serviceConfig: `{"loadBalancingConfig": [{"pick_first":{ "shuffleAddressList": 666 }}]}`,
-			wantFirstAddr: true,
-		},
-		{
-			name:          "env var enabled with empty pickfirst config",
-			envVar:        true,
+			name:          "empty pickfirst config",
 			serviceConfig: `{"loadBalancingConfig": [{"pick_first":{}}]}`,
 			wantFirstAddr: true,
 		},
 		{
-			name:          "env var enabled with empty good pickfirst config",
-			envVar:        true,
+			name:          "empty good pickfirst config",
 			serviceConfig: `{"loadBalancingConfig": [{"pick_first":{ "shuffleAddressList": true }}]}`,
 			wantFirstAddr: false,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			// Set the env var as specified by the test table.
-			origPickFirstLBConfig := envconfig.PickFirstLBConfig
-			envconfig.PickFirstLBConfig = test.envVar
-			defer func() { envconfig.PickFirstLBConfig = origPickFirstLBConfig }()
-
 			// Set up our backends.
 			cc, r, backends := setupPickFirst(t, 2)
 			addrs := stubBackendsToResolverAddrs(backends)
@@ -555,10 +487,6 @@ func (s) TestPickFirst_ParseConfig_Success(t *testing.T) {
 
 // Test config parsing for a bad service config.
 func (s) TestPickFirst_ParseConfig_Failure(t *testing.T) {
-	origPickFirstLBConfig := envconfig.PickFirstLBConfig
-	envconfig.PickFirstLBConfig = true
-	defer func() { envconfig.PickFirstLBConfig = origPickFirstLBConfig }()
-
 	// Service config should fail with the below config. Name resolvers are
 	// expected to perform this parsing before they push the parsed service
 	// config to the channel.

test/xds/xds_client_affinity_test.go

@@ -25,7 +25,6 @@ import (
 
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
-	"google.golang.org/grpc/internal/envconfig"
 	"google.golang.org/grpc/internal/stubserver"
 	"google.golang.org/grpc/internal/testutils"
 	"google.golang.org/grpc/internal/testutils/xds/e2e"
@@ -84,12 +83,6 @@ func ringhashCluster(clusterName, edsServiceName string) *v3clusterpb.Cluster {
 // propagated to pick the ring_hash policy. It doesn't test the affinity
 // behavior in ring_hash policy.
 func (s) TestClientSideAffinitySanityCheck(t *testing.T) {
-	defer func() func() {
-		old := envconfig.XDSRingHash
-		envconfig.XDSRingHash = true
-		return func() { envconfig.XDSRingHash = old }
-	}()()
-
 	managementServer, nodeID, _, resolver, cleanup1 := e2e.SetupManagementServer(t, e2e.ManagementServerOptions{})
 	defer cleanup1()
 

test/xds/xds_client_custom_lb_test.go

@@ -93,11 +93,6 @@ func clusterWithLBConfiguration(t *testing.T, clusterName, edsServiceName string
 // first) child load balancing policy, and asserts the correct distribution
 // based on the locality weights and the endpoint picking policy specified.
 func (s) TestWrrLocality(t *testing.T) {
-	oldCustomLBSupport := envconfig.XDSCustomLBPolicy
-	envconfig.XDSCustomLBPolicy = true
-	defer func() {
-		envconfig.XDSCustomLBPolicy = oldCustomLBSupport
-	}()
 	oldLeastRequestLBSupport := envconfig.LeastRequestLB
 	envconfig.LeastRequestLB = true
 	defer func() {