xds: don't remove env var protection for security on the client yet (#…

…4752)

Set the value to true by default, and remove it one release later.

internal/xds/env/env.go

@@ -39,9 +39,10 @@ const (
 	// When both bootstrap FileName and FileContent are set, FileName is used.
 	BootstrapFileContentEnv = "GRPC_XDS_BOOTSTRAP_CONFIG"
 
-	ringHashSupportEnv        = "GRPC_XDS_EXPERIMENTAL_ENABLE_RING_HASH"
-	aggregateAndDNSSupportEnv = "GRPC_XDS_EXPERIMENTAL_ENABLE_AGGREGATE_AND_LOGICAL_DNS_CLUSTER"
-	retrySupportEnv           = "GRPC_XDS_EXPERIMENTAL_ENABLE_RETRY"
+	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"
 
 	c2pResolverSupportEnv                    = "GRPC_EXPERIMENTAL_GOOGLE_C2P_RESOLVER"
 	c2pResolverTestOnlyTrafficDirectorURIEnv = "GRPC_TEST_ONLY_GOOGLE_C2P_RESOLVER_TRAFFIC_DIRECTOR_URI"
@@ -64,6 +65,13 @@ var (
 	// be enabled by setting the environment variable
 	// "GRPC_XDS_EXPERIMENTAL_ENABLE_RING_HASH" to "true".
 	RingHashSupport = strings.EqualFold(os.Getenv(ringHashSupportEnv), "true")
+	// ClientSideSecuritySupport 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.
+	ClientSideSecuritySupport = strings.EqualFold(os.Getenv(clientSideSecuritySupportEnv), "true")
 	// AggregateAndDNSSupportEnv indicates whether processing of aggregated
 	// cluster and DNS cluster is enabled, which can be enabled by setting the
 	// environment variable
@@ -81,3 +89,15 @@ var (
 	// C2PResolverTestOnlyTrafficDirectorURI is the TD URI for testing.
 	C2PResolverTestOnlyTrafficDirectorURI = os.Getenv(c2pResolverTestOnlyTrafficDirectorURIEnv)
 )
+
+func init() {
+	// Set the env var used to control processing of security configuration on
+	// the client-side to true by default.
+	// TODO(easwars): Remove this env var completely in 1.42.x release.
+	//
+	// If the env var is set explicitly, honor it.
+	ClientSideSecuritySupport = true
+	if val, ok := os.LookupEnv(clientSideSecuritySupportEnv); ok {
+		ClientSideSecuritySupport = strings.EqualFold(val, "true")
+	}
+}

xds/internal/xdsclient/cds_test.go

@@ -432,6 +432,54 @@ func (s) TestValidateCluster_Success(t *testing.T) {
 	}
 }
 
+func (s) TestValidateClusterWithSecurityConfig_EnvVarOff(t *testing.T) {
+	// Turn off the env var protection for client-side security.
+	origClientSideSecurityEnvVar := env.ClientSideSecuritySupport
+	env.ClientSideSecuritySupport = false
+	defer func() { env.ClientSideSecuritySupport = origClientSideSecurityEnvVar }()
+
+	cluster := &v3clusterpb.Cluster{
+		Name:                 clusterName,
+		ClusterDiscoveryType: &v3clusterpb.Cluster_Type{Type: v3clusterpb.Cluster_EDS},
+		EdsClusterConfig: &v3clusterpb.Cluster_EdsClusterConfig{
+			EdsConfig: &v3corepb.ConfigSource{
+				ConfigSourceSpecifier: &v3corepb.ConfigSource_Ads{
+					Ads: &v3corepb.AggregatedConfigSource{},
+				},
+			},
+			ServiceName: serviceName,
+		},
+		LbPolicy: v3clusterpb.Cluster_ROUND_ROBIN,
+		TransportSocket: &v3corepb.TransportSocket{
+			Name: "envoy.transport_sockets.tls",
+			ConfigType: &v3corepb.TransportSocket_TypedConfig{
+				TypedConfig: testutils.MarshalAny(&v3tlspb.UpstreamTlsContext{
+					CommonTlsContext: &v3tlspb.CommonTlsContext{
+						ValidationContextType: &v3tlspb.CommonTlsContext_ValidationContextCertificateProviderInstance{
+							ValidationContextCertificateProviderInstance: &v3tlspb.CommonTlsContext_CertificateProviderInstance{
+								InstanceName:    "rootInstance",
+								CertificateName: "rootCert",
+							},
+						},
+					},
+				}),
+			},
+		},
+	}
+	wantUpdate := ClusterUpdate{
+		ClusterName:    clusterName,
+		EDSServiceName: serviceName,
+		EnableLRS:      false,
+	}
+	gotUpdate, err := validateClusterAndConstructClusterUpdate(cluster)
+	if err != nil {
+		t.Errorf("validateClusterAndConstructClusterUpdate() failed: %v", err)
+	}
+	if diff := cmp.Diff(wantUpdate, gotUpdate); diff != "" {
+		t.Errorf("validateClusterAndConstructClusterUpdate() returned unexpected diff (-want, got):\n%s", diff)
+	}
+}
+
 func (s) TestValidateClusterWithSecurityConfig(t *testing.T) {
 	const (
 		identityPluginInstance = "identityPluginInstance"

xds/internal/xdsclient/xds.go

@@ -698,10 +698,14 @@ func validateClusterAndConstructClusterUpdate(cluster *v3clusterpb.Cluster) (Clu
 		return ClusterUpdate{}, fmt.Errorf("unexpected lbPolicy %v in response: %+v", cluster.GetLbPolicy(), cluster)
 	}
 
-	// Process security configuration received from the control plane .
-	sc, err := securityConfigFromCluster(cluster)
-	if err != nil {
-		return ClusterUpdate{}, err
+	// Process security configuration received from the control plane iff the
+	// corresponding environment variable is set.
+	var sc *SecurityConfig
+	if env.ClientSideSecuritySupport {
+		var err error
+		if sc, err = securityConfigFromCluster(cluster); err != nil {
+			return ClusterUpdate{}, err
+		}
 	}
 
 	ret := ClusterUpdate{