From 43e8fd4f69b65fd51d72578df4afa5c0519ca2b5 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Fri, 10 Sep 2021 10:59:25 -0700 Subject: [PATCH] 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 | 26 ++++++++++++++-- xds/internal/xdsclient/cds_test.go | 48 ++++++++++++++++++++++++++++++ xds/internal/xdsclient/xds.go | 12 +++++--- 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/internal/xds/env/env.go b/internal/xds/env/env.go index 9d5d47ff2a8..7f879b44da0 100644 --- a/internal/xds/env/env.go +++ b/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") + } +} diff --git a/xds/internal/xdsclient/cds_test.go b/xds/internal/xdsclient/cds_test.go index a72322c1ba0..f03cbe7efb4 100644 --- a/xds/internal/xdsclient/cds_test.go +++ b/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" diff --git a/xds/internal/xdsclient/xds.go b/xds/internal/xdsclient/xds.go index bd223ff7aa8..236d11f3731 100644 --- a/xds/internal/xdsclient/xds.go +++ b/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{