From 98e5ec25d1209a91267a31842b22480bb06e8eca Mon Sep 17 00:00:00 2001 From: Kannan J Date: Wed, 5 Nov 2025 12:23:33 +0000 Subject: [PATCH 1/5] Remove feature guarding of the env vars for Cloud run CSM: GRPC_EXPERIMENTAL_XDS_SNI GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE GRPC_EXPERIMENTAL_XDS_SYSTEM_ROOT_CERTS GRPC_EXPERIMENTAL_XDS_GCP_AUTHENTICATION_FILTER --- .../main/java/io/grpc/xds/FilterRegistry.java | 10 +- .../io/grpc/xds/GcpAuthenticationFilter.java | 5 - .../java/io/grpc/xds/XdsClusterResource.java | 6 +- .../grpc/xds/XdsRouteConfigureResource.java | 9 +- .../security/SecurityProtocolNegotiators.java | 19 +- .../security/trust/XdsX509TrustManager.java | 2 +- .../grpc/xds/GcpAuthenticationFilterTest.java | 5 - .../grpc/xds/GrpcXdsClientImplDataTest.java | 173 +++-------- .../grpc/xds/XdsSecurityClientServerTest.java | 6 - .../SecurityProtocolNegotiatorsTest.java | 283 ++++++++---------- 10 files changed, 172 insertions(+), 346 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/FilterRegistry.java b/xds/src/main/java/io/grpc/xds/FilterRegistry.java index 1fbccea8000..da3a59fe8c1 100644 --- a/xds/src/main/java/io/grpc/xds/FilterRegistry.java +++ b/xds/src/main/java/io/grpc/xds/FilterRegistry.java @@ -17,7 +17,6 @@ package io.grpc.xds; import com.google.common.annotations.VisibleForTesting; -import io.grpc.internal.GrpcUtil; import java.util.HashMap; import java.util.Map; import javax.annotation.Nullable; @@ -33,18 +32,13 @@ final class FilterRegistry { private FilterRegistry() {} - static boolean isEnabledGcpAuthnFilter = - GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_GCP_AUTHENTICATION_FILTER", false); - static synchronized FilterRegistry getDefaultRegistry() { if (instance == null) { instance = newRegistry().register( new FaultFilter.Provider(), new RouterFilter.Provider(), - new RbacFilter.Provider()); - if (isEnabledGcpAuthnFilter) { - instance.register(new GcpAuthenticationFilter.Provider()); - } + new RbacFilter.Provider(), + new GcpAuthenticationFilter.Provider()); } return instance; } diff --git a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java index dc133eaaf1a..8ec02f4f809 100644 --- a/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java +++ b/xds/src/main/java/io/grpc/xds/GcpAuthenticationFilter.java @@ -17,7 +17,6 @@ package io.grpc.xds; import static com.google.common.base.Preconditions.checkNotNull; -import static io.grpc.xds.FilterRegistry.isEnabledGcpAuthnFilter; import static io.grpc.xds.XdsNameResolver.CLUSTER_SELECTION_KEY; import static io.grpc.xds.XdsNameResolver.XDS_CONFIG_CALL_OPTION_KEY; @@ -313,10 +312,6 @@ public String getTypeUrl() { public AudienceWrapper parse(Any any) throws ResourceInvalidException { Audience audience; try { - if (!isEnabledGcpAuthnFilter) { - throw new InvalidProtocolBufferException("Environment variable for GCP Authentication " - + "Filter is Not Set"); - } audience = any.unpack(Audience.class); } catch (InvalidProtocolBufferException ex) { throw new ResourceInvalidException("Invalid Resource in address proto", ex); diff --git a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java index 54089491671..a1c1c67a177 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java @@ -64,9 +64,6 @@ class XdsClusterResource extends XdsResourceType { ? Boolean.parseBoolean(System.getenv("GRPC_EXPERIMENTAL_ENABLE_LEAST_REQUEST")) : Boolean.parseBoolean( System.getProperty("io.grpc.xds.experimentalEnableLeastRequest", "true")); - @VisibleForTesting - public static boolean enableSystemRootCerts = - GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_SYSTEM_ROOT_CERTS", false); static boolean isEnabledXdsHttpConnect = GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_HTTP_CONNECT", false); @@ -486,8 +483,7 @@ static void validateCommonTlsContext( } String rootCaInstanceName = getRootCertInstanceName(commonTlsContext); if (rootCaInstanceName == null) { - if (!server && (!enableSystemRootCerts - || !CommonTlsContextUtil.isUsingSystemRootCerts(commonTlsContext))) { + if (!server && !CommonTlsContextUtil.isUsingSystemRootCerts(commonTlsContext)) { throw new ResourceInvalidException( "ca_certificate_provider_instance or system_root_certs is required in " + "upstream-tls-context"); diff --git a/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java b/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java index 2ee326435c4..c974f9dc475 100644 --- a/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java @@ -475,8 +475,7 @@ static StructOrError parseRouteAction( case CLUSTER: return StructOrError.fromStruct(RouteAction.forCluster( proto.getCluster(), hashPolicies, timeoutNano, retryPolicy, - GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE, false) - && args.getServerInfo().isTrustedXdsServer() && proto.getAutoHostRewrite().getValue())); + args.getServerInfo().isTrustedXdsServer() && proto.getAutoHostRewrite().getValue())); case CLUSTER_HEADER: return null; case WEIGHTED_CLUSTERS: @@ -510,8 +509,7 @@ static StructOrError parseRouteAction( } return StructOrError.fromStruct(VirtualHost.Route.RouteAction.forWeightedClusters( weightedClusters, hashPolicies, timeoutNano, retryPolicy, - GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE, false) - && args.getServerInfo().isTrustedXdsServer() && proto.getAutoHostRewrite().getValue())); + args.getServerInfo().isTrustedXdsServer() && proto.getAutoHostRewrite().getValue())); case CLUSTER_SPECIFIER_PLUGIN: if (enableRouteLookup) { String pluginName = proto.getClusterSpecifierPlugin(); @@ -527,8 +525,7 @@ static StructOrError parseRouteAction( NamedPluginConfig namedPluginConfig = NamedPluginConfig.create(pluginName, pluginConfig); return StructOrError.fromStruct(VirtualHost.Route.RouteAction.forClusterSpecifierPlugin( namedPluginConfig, hashPolicies, timeoutNano, retryPolicy, - GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE, false) - && args.getServerInfo().isTrustedXdsServer() + args.getServerInfo().isTrustedXdsServer() && proto.getAutoHostRewrite().getValue())); } else { return null; diff --git a/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java b/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java index 0da06b3a753..f512c0e3057 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java @@ -215,21 +215,16 @@ public void handlerAdded(ChannelHandlerContext ctx) throws Exception { this.sslContextProviderSupplier = sslContextProviderSupplier; EnvoyServerProtoData.BaseTlsContext tlsContext = sslContextProviderSupplier.getTlsContext(); UpstreamTlsContext upstreamTlsContext = ((UpstreamTlsContext) tlsContext); - if (CertificateUtils.isXdsSniEnabled) { - String sniToUse = upstreamTlsContext.getAutoHostSni() - && !Strings.isNullOrEmpty(endpointHostname) - ? endpointHostname : upstreamTlsContext.getSni(); - if (sniToUse.isEmpty() && CertificateUtils.useChannelAuthorityIfNoSniApplicable) { - sniToUse = grpcHandler.getAuthority(); - autoSniSanValidationDoesNotApply = true; - } else { - autoSniSanValidationDoesNotApply = false; - } - sni = sniToUse; + String sniToUse = upstreamTlsContext.getAutoHostSni() + && !Strings.isNullOrEmpty(endpointHostname) + ? endpointHostname : upstreamTlsContext.getSni(); + if (sniToUse.isEmpty() && CertificateUtils.useChannelAuthorityIfNoSniApplicable) { + sniToUse = grpcHandler.getAuthority(); + autoSniSanValidationDoesNotApply = true; } else { - sni = grpcHandler.getAuthority(); autoSniSanValidationDoesNotApply = false; } + sni = sniToUse; } @VisibleForTesting diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java index e8e7243ce0e..01f25dda6c7 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java @@ -308,7 +308,7 @@ public void checkServerTrusted(X509Certificate[] chain, String authType) private List getAutoSniSanMatchers(SSLParameters sslParams) { List sniNamesToMatch = new ArrayList<>(); - if (CertificateUtils.isXdsSniEnabled && autoSniSanValidation) { + if (autoSniSanValidation) { List serverNames = sslParams.getServerNames(); if (serverNames != null) { for (SNIServerName serverName : serverNames) { diff --git a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java index 7f203e036b7..4f8f2ed8f29 100644 --- a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java +++ b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java @@ -88,11 +88,6 @@ public class GcpAuthenticationFilterTest { private static final RdsUpdate rdsUpdate = getRdsUpdate(); private static final CdsUpdate cdsUpdate = getCdsUpdate(); - @Before - public void setUp() { - System.setProperty("GRPC_EXPERIMENTAL_XDS_GCP_AUTHENTICATION_FILTER", "true"); - } - @Test public void testNewFilterInstancesPerFilterName() { assertThat(new GcpAuthenticationFilter("FILTER_INSTANCE_NAME1", 10)) diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java index 2c75ee04d91..79718722207 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java @@ -172,26 +172,21 @@ public class GrpcXdsClientImplDataTest { private static final ServerInfo LRS_SERVER_INFO = ServerInfo.create("lrs.googleapis.com", InsecureChannelCredentials.create()); - private static final String GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE = - "GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE"; private final FilterRegistry filterRegistry = FilterRegistry.getDefaultRegistry(); private boolean originalEnableRouteLookup; private boolean originalEnableLeastRequest; - private boolean originalEnableUseSystemRootCerts; @Before public void setUp() { originalEnableRouteLookup = XdsRouteConfigureResource.enableRouteLookup; originalEnableLeastRequest = XdsClusterResource.enableLeastRequest; - originalEnableUseSystemRootCerts = XdsClusterResource.enableSystemRootCerts; } @After public void tearDown() { XdsRouteConfigureResource.enableRouteLookup = originalEnableRouteLookup; XdsClusterResource.enableLeastRequest = originalEnableLeastRequest; - XdsClusterResource.enableSystemRootCerts = originalEnableUseSystemRootCerts; } @Test @@ -536,23 +531,18 @@ public void parseRouteAction_withCluster() { @Test public void parseRouteAction_withCluster_autoHostRewriteEnabled() { - System.setProperty(GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE, "true"); - try { - io.envoyproxy.envoy.config.route.v3.RouteAction proto = - io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder() - .setCluster("cluster-foo") - .setAutoHostRewrite(BoolValue.of(true)) - .build(); - StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, - ImmutableMap.of(), ImmutableSet.of(), getXdsResourceTypeArgs(true)); - assertThat(struct.getErrorDetail()).isNull(); - assertThat(struct.getStruct().cluster()).isEqualTo("cluster-foo"); - assertThat(struct.getStruct().weightedClusters()).isNull(); - assertThat(struct.getStruct().autoHostRewrite()).isTrue(); - } finally { - System.clearProperty(GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE); - } + io.envoyproxy.envoy.config.route.v3.RouteAction proto = + io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder() + .setCluster("cluster-foo") + .setAutoHostRewrite(BoolValue.of(true)) + .build(); + StructOrError struct = + XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, + ImmutableMap.of(), ImmutableSet.of(), getXdsResourceTypeArgs(true)); + assertThat(struct.getErrorDetail()).isNull(); + assertThat(struct.getStruct().cluster()).isEqualTo("cluster-foo"); + assertThat(struct.getStruct().weightedClusters()).isNull(); + assertThat(struct.getStruct().autoHostRewrite()).isTrue(); } @Test @@ -600,35 +590,30 @@ public void parseRouteAction_withWeightedCluster() { @Test public void parseRouteAction_withWeightedCluster_autoHostRewriteEnabled() { - System.setProperty(GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE, "true"); - try { - io.envoyproxy.envoy.config.route.v3.RouteAction proto = - io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder() - .setWeightedClusters( - WeightedCluster.newBuilder() - .addClusters( - WeightedCluster.ClusterWeight - .newBuilder() - .setName("cluster-foo") - .setWeight(UInt32Value.newBuilder().setValue(30))) - .addClusters(WeightedCluster.ClusterWeight - .newBuilder() - .setName("cluster-bar") - .setWeight(UInt32Value.newBuilder().setValue(70)))) - .setAutoHostRewrite(BoolValue.of(true)) - .build(); - StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, - ImmutableMap.of(), ImmutableSet.of(), getXdsResourceTypeArgs(true)); - assertThat(struct.getErrorDetail()).isNull(); - assertThat(struct.getStruct().cluster()).isNull(); - assertThat(struct.getStruct().weightedClusters()).containsExactly( - ClusterWeight.create("cluster-foo", 30, ImmutableMap.of()), - ClusterWeight.create("cluster-bar", 70, ImmutableMap.of())); - assertThat(struct.getStruct().autoHostRewrite()).isTrue(); - } finally { - System.clearProperty(GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE); - } + io.envoyproxy.envoy.config.route.v3.RouteAction proto = + io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder() + .setWeightedClusters( + WeightedCluster.newBuilder() + .addClusters( + WeightedCluster.ClusterWeight + .newBuilder() + .setName("cluster-foo") + .setWeight(UInt32Value.newBuilder().setValue(30))) + .addClusters(WeightedCluster.ClusterWeight + .newBuilder() + .setName("cluster-bar") + .setWeight(UInt32Value.newBuilder().setValue(70)))) + .setAutoHostRewrite(BoolValue.of(true)) + .build(); + StructOrError struct = + XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, + ImmutableMap.of(), ImmutableSet.of(), getXdsResourceTypeArgs(true)); + assertThat(struct.getErrorDetail()).isNull(); + assertThat(struct.getStruct().cluster()).isNull(); + assertThat(struct.getStruct().weightedClusters()).containsExactly( + ClusterWeight.create("cluster-foo", 30, ImmutableMap.of()), + ClusterWeight.create("cluster-bar", 70, ImmutableMap.of())); + assertThat(struct.getStruct().autoHostRewrite()).isTrue(); } @Test @@ -1004,28 +989,6 @@ public void parseRouteAction_clusterSpecifier() { @Test public void parseRouteAction_clusterSpecifier_autoHostRewriteEnabled() { - System.setProperty(GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE, "true"); - try { - XdsRouteConfigureResource.enableRouteLookup = true; - io.envoyproxy.envoy.config.route.v3.RouteAction proto = - io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder() - .setClusterSpecifierPlugin(CLUSTER_SPECIFIER_PLUGIN.name()) - .setAutoHostRewrite(BoolValue.of(true)) - .build(); - StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, - ImmutableMap.of(CLUSTER_SPECIFIER_PLUGIN.name(), RlsPluginConfig.create( - ImmutableMap.of("lookupService", "rls-cbt.googleapis.com"))), ImmutableSet.of(), - getXdsResourceTypeArgs(true)); - assertThat(struct.getStruct()).isNotNull(); - assertThat(struct.getStruct().autoHostRewrite()).isTrue(); - } finally { - System.clearProperty(GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE); - } - } - - @Test - public void parseRouteAction_clusterSpecifier_flagDisabled_autoHostRewriteDisabled() { XdsRouteConfigureResource.enableRouteLookup = true; io.envoyproxy.envoy.config.route.v3.RouteAction proto = io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder() @@ -1038,7 +1001,7 @@ public void parseRouteAction_clusterSpecifier_flagDisabled_autoHostRewriteDisabl ImmutableMap.of("lookupService", "rls-cbt.googleapis.com"))), ImmutableSet.of(), getXdsResourceTypeArgs(true)); assertThat(struct.getStruct()).isNotNull(); - assertThat(struct.getStruct().autoHostRewrite()).isFalse(); + assertThat(struct.getStruct().autoHostRewrite()).isTrue(); } @Test @@ -2447,7 +2410,6 @@ public Object parse(Any value) { @Test public void processCluster_parsesAudienceMetadata() throws Exception { - FilterRegistry.isEnabledGcpAuthnFilter = true; MetadataRegistry.getInstance(); Audience audience = Audience.newBuilder() @@ -2491,14 +2453,10 @@ public void processCluster_parsesAudienceMetadata() throws Exception { "FILTER_METADATA", ImmutableMap.of( "key1", "value1", "key2", 42.0)); - try { - assertThat(update.parsedMetadata().get("FILTER_METADATA")) - .isEqualTo(expectedParsedMetadata.get("FILTER_METADATA")); - assertThat(update.parsedMetadata().get("AUDIENCE_METADATA")) - .isInstanceOf(AudienceWrapper.class); - } finally { - FilterRegistry.isEnabledGcpAuthnFilter = false; - } + assertThat(update.parsedMetadata().get("FILTER_METADATA")) + .isEqualTo(expectedParsedMetadata.get("FILTER_METADATA")); + assertThat(update.parsedMetadata().get("AUDIENCE_METADATA")) + .isInstanceOf(AudienceWrapper.class); } @Test @@ -3177,36 +3135,9 @@ public void validateCommonTlsContext_validationContextProviderInstance() .validateCommonTlsContext(commonTlsContext, ImmutableSet.of("name1", "name2"), false); } - @Test - public void - validateCommonTlsContext_combinedValidationContextSystemRootCerts_envVarNotSet_throws() { - XdsClusterResource.enableSystemRootCerts = false; - CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() - .setCombinedValidationContext( - CommonTlsContext.CombinedCertificateValidationContext.newBuilder() - .setDefaultValidationContext( - CertificateValidationContext.newBuilder() - .setSystemRootCerts( - CertificateValidationContext.SystemRootCerts.newBuilder().build()) - .build() - ) - .build()) - .build(); - try { - XdsClusterResource - .validateCommonTlsContext(commonTlsContext, ImmutableSet.of(), false); - fail("Expected exception"); - } catch (ResourceInvalidException ex) { - assertThat(ex.getMessage()).isEqualTo( - "ca_certificate_provider_instance or system_root_certs is required in" - + " upstream-tls-context"); - } - } - @Test public void validateCommonTlsContext_combinedValidationContextSystemRootCerts() throws ResourceInvalidException { - XdsClusterResource.enableSystemRootCerts = true; CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setCombinedValidationContext( CommonTlsContext.CombinedCertificateValidationContext.newBuilder() @@ -3222,31 +3153,9 @@ public void validateCommonTlsContext_combinedValidationContextSystemRootCerts() .validateCommonTlsContext(commonTlsContext, ImmutableSet.of(), false); } - @Test - public void validateCommonTlsContext_validationContextSystemRootCerts_envVarNotSet_throws() { - XdsClusterResource.enableSystemRootCerts = false; - CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() - .setValidationContext( - CertificateValidationContext.newBuilder() - .setSystemRootCerts( - CertificateValidationContext.SystemRootCerts.newBuilder().build()) - .build()) - .build(); - try { - XdsClusterResource - .validateCommonTlsContext(commonTlsContext, ImmutableSet.of(), false); - fail("Expected exception"); - } catch (ResourceInvalidException ex) { - assertThat(ex.getMessage()).isEqualTo( - "ca_certificate_provider_instance or system_root_certs is required in " - + "upstream-tls-context"); - } - } - @Test public void validateCommonTlsContext_validationContextSystemRootCerts() throws ResourceInvalidException { - XdsClusterResource.enableSystemRootCerts = true; CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setValidationContext( CertificateValidationContext.newBuilder() diff --git a/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java b/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java index 9141147702f..4df7f0ec4f7 100644 --- a/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java @@ -317,7 +317,6 @@ public void tlsClientServer_noAutoSniValidation_failureToMatchSubjAltNames() @Test public void tlsClientServer_autoSniValidation_sniInUtc() throws Exception { - CertificateUtils.isXdsSniEnabled = true; Path trustStoreFilePath = getCacertFilePathForTestCa(); try { setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString()); @@ -341,14 +340,12 @@ public void tlsClientServer_autoSniValidation_sniInUtc() } finally { Files.deleteIfExists(trustStoreFilePath); clearTrustStoreSystemProperties(); - CertificateUtils.isXdsSniEnabled = false; } } @Test public void tlsClientServer_autoSniValidation_sniFromHostname() throws Exception { - CertificateUtils.isXdsSniEnabled = true; Path trustStoreFilePath = getCacertFilePathForTestCa(); try { setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString()); @@ -375,14 +372,12 @@ public void tlsClientServer_autoSniValidation_sniFromHostname() } finally { Files.deleteIfExists(trustStoreFilePath); clearTrustStoreSystemProperties(); - CertificateUtils.isXdsSniEnabled = false; } } @Test public void tlsClientServer_autoSniValidation_noSniApplicable_usesMatcherFromCmnVdnCtx() throws Exception { - CertificateUtils.isXdsSniEnabled = true; Path trustStoreFilePath = getCacertFilePathForTestCa(); try { setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString()); @@ -406,7 +401,6 @@ public void tlsClientServer_autoSniValidation_noSniApplicable_usesMatcherFromCmn } finally { Files.deleteIfExists(trustStoreFilePath); clearTrustStoreSystemProperties(); - CertificateUtils.isXdsSniEnabled = false; } } diff --git a/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java b/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java index dcb2fa051a3..b27042f7a00 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java @@ -151,32 +151,27 @@ public void clientSecurityProtocolNegotiatorNewHandler_withTlsContextAttribute() @Test public void clientSecurityProtocolNegotiator_autoHostSni_hostnamePassedToClientSecurityHandlr() { - CertificateUtils.isXdsSniEnabled = true; - try { - UpstreamTlsContext upstreamTlsContext = - CommonTlsContextTestsUtil.buildUpstreamTlsContext( - CommonTlsContext.newBuilder().build(), "", true, false); - ClientSecurityProtocolNegotiator pn = - new ClientSecurityProtocolNegotiator(InternalProtocolNegotiators.plaintext()); - GrpcHttp2ConnectionHandler mockHandler = mock(GrpcHttp2ConnectionHandler.class); - ChannelLogger logger = mock(ChannelLogger.class); - doNothing().when(logger).log(any(ChannelLogLevel.class), anyString()); - when(mockHandler.getNegotiationLogger()).thenReturn(logger); - TlsContextManager mockTlsContextManager = mock(TlsContextManager.class); - when(mockHandler.getEagAttributes()) - .thenReturn( - Attributes.newBuilder() - .set(SecurityProtocolNegotiators.ATTR_SSL_CONTEXT_PROVIDER_SUPPLIER, - new SslContextProviderSupplier(upstreamTlsContext, mockTlsContextManager)) - .set(XdsInternalAttributes.ATTR_ADDRESS_NAME, FAKE_AUTHORITY) - .build()); - ChannelHandler newHandler = pn.newHandler(mockHandler); - assertThat(newHandler).isNotNull(); - assertThat(newHandler).isInstanceOf(ClientSecurityHandler.class); - assertThat(((ClientSecurityHandler) newHandler).getSni()).isEqualTo(FAKE_AUTHORITY); - } finally { - CertificateUtils.isXdsSniEnabled = false; - } + UpstreamTlsContext upstreamTlsContext = + CommonTlsContextTestsUtil.buildUpstreamTlsContext( + CommonTlsContext.newBuilder().build(), "", true, false); + ClientSecurityProtocolNegotiator pn = + new ClientSecurityProtocolNegotiator(InternalProtocolNegotiators.plaintext()); + GrpcHttp2ConnectionHandler mockHandler = mock(GrpcHttp2ConnectionHandler.class); + ChannelLogger logger = mock(ChannelLogger.class); + doNothing().when(logger).log(any(ChannelLogLevel.class), anyString()); + when(mockHandler.getNegotiationLogger()).thenReturn(logger); + TlsContextManager mockTlsContextManager = mock(TlsContextManager.class); + when(mockHandler.getEagAttributes()) + .thenReturn( + Attributes.newBuilder() + .set(SecurityProtocolNegotiators.ATTR_SSL_CONTEXT_PROVIDER_SUPPLIER, + new SslContextProviderSupplier(upstreamTlsContext, mockTlsContextManager)) + .set(XdsInternalAttributes.ATTR_ADDRESS_NAME, FAKE_AUTHORITY) + .build()); + ChannelHandler newHandler = pn.newHandler(mockHandler); + assertThat(newHandler).isNotNull(); + assertThat(newHandler).isInstanceOf(ClientSecurityHandler.class); + assertThat(((ClientSecurityHandler) newHandler).getSni()).isEqualTo(FAKE_AUTHORITY); } @Test @@ -235,110 +230,71 @@ protected void onException(Throwable throwable) { @Test public void sniInClientSecurityHandler_autoHostSniIsTrue_usesEndpointHostname() { - CertificateUtils.isXdsSniEnabled = true; - try { - Bootstrapper.BootstrapInfo bootstrapInfoForClient = CommonBootstrapperTestUtils - .buildBootstrapInfo("google_cloud_private_spiffe-client", CLIENT_KEY_FILE, - CLIENT_PEM_FILE, CA_PEM_FILE, null, null, null, null, null); - UpstreamTlsContext upstreamTlsContext = - CommonTlsContextTestsUtil - .buildUpstreamTlsContext("google_cloud_private_spiffe-client", true, "", true); - SslContextProviderSupplier sslContextProviderSupplier = - new SslContextProviderSupplier(upstreamTlsContext, - new TlsContextManagerImpl(bootstrapInfoForClient)); - - ClientSecurityHandler clientSecurityHandler = - new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, HOSTNAME); - - assertThat(clientSecurityHandler.getSni()).isEqualTo(HOSTNAME); - } finally { - CertificateUtils.isXdsSniEnabled = false; - } + Bootstrapper.BootstrapInfo bootstrapInfoForClient = CommonBootstrapperTestUtils + .buildBootstrapInfo("google_cloud_private_spiffe-client", CLIENT_KEY_FILE, + CLIENT_PEM_FILE, CA_PEM_FILE, null, null, null, null, null); + UpstreamTlsContext upstreamTlsContext = + CommonTlsContextTestsUtil + .buildUpstreamTlsContext("google_cloud_private_spiffe-client", true, "", true); + SslContextProviderSupplier sslContextProviderSupplier = + new SslContextProviderSupplier(upstreamTlsContext, + new TlsContextManagerImpl(bootstrapInfoForClient)); + + ClientSecurityHandler clientSecurityHandler = + new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, HOSTNAME); + + assertThat(clientSecurityHandler.getSni()).isEqualTo(HOSTNAME); } @Test public void sniInClientSecurityHandler_autoHostSni_endpointHostnameIsEmpty_usesSniFromUtc() { - CertificateUtils.isXdsSniEnabled = true; - try { - Bootstrapper.BootstrapInfo bootstrapInfoForClient = CommonBootstrapperTestUtils - .buildBootstrapInfo("google_cloud_private_spiffe-client", CLIENT_KEY_FILE, - CLIENT_PEM_FILE, CA_PEM_FILE, null, null, null, null, null); - UpstreamTlsContext upstreamTlsContext = CommonTlsContextTestsUtil.buildUpstreamTlsContext( - "google_cloud_private_spiffe-client", true, SNI_IN_UTC, true); - SslContextProviderSupplier sslContextProviderSupplier = - new SslContextProviderSupplier(upstreamTlsContext, - new TlsContextManagerImpl(bootstrapInfoForClient)); - - ClientSecurityHandler clientSecurityHandler = - new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, ""); - - assertThat(clientSecurityHandler.getSni()).isEqualTo(SNI_IN_UTC); - } finally { - CertificateUtils.isXdsSniEnabled = false; - } + Bootstrapper.BootstrapInfo bootstrapInfoForClient = CommonBootstrapperTestUtils + .buildBootstrapInfo("google_cloud_private_spiffe-client", CLIENT_KEY_FILE, + CLIENT_PEM_FILE, CA_PEM_FILE, null, null, null, null, null); + UpstreamTlsContext upstreamTlsContext = CommonTlsContextTestsUtil.buildUpstreamTlsContext( + "google_cloud_private_spiffe-client", true, SNI_IN_UTC, true); + SslContextProviderSupplier sslContextProviderSupplier = + new SslContextProviderSupplier(upstreamTlsContext, + new TlsContextManagerImpl(bootstrapInfoForClient)); + + ClientSecurityHandler clientSecurityHandler = + new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, ""); + + assertThat(clientSecurityHandler.getSni()).isEqualTo(SNI_IN_UTC); } @Test public void sniInClientSecurityHandler_autoHostSni_endpointHostnameIsNull_usesSniFromUtc() { - CertificateUtils.isXdsSniEnabled = true; - try { - Bootstrapper.BootstrapInfo bootstrapInfoForClient = CommonBootstrapperTestUtils - .buildBootstrapInfo("google_cloud_private_spiffe-client", CLIENT_KEY_FILE, - CLIENT_PEM_FILE, CA_PEM_FILE, null, null, null, null, null); - UpstreamTlsContext upstreamTlsContext = CommonTlsContextTestsUtil.buildUpstreamTlsContext( - "google_cloud_private_spiffe-client", true, SNI_IN_UTC, true); - SslContextProviderSupplier sslContextProviderSupplier = - new SslContextProviderSupplier(upstreamTlsContext, - new TlsContextManagerImpl(bootstrapInfoForClient)); - - ClientSecurityHandler clientSecurityHandler = - new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, null); - - assertThat(clientSecurityHandler.getSni()).isEqualTo(SNI_IN_UTC); - } finally { - CertificateUtils.isXdsSniEnabled = false; - } - } + Bootstrapper.BootstrapInfo bootstrapInfoForClient = CommonBootstrapperTestUtils + .buildBootstrapInfo("google_cloud_private_spiffe-client", CLIENT_KEY_FILE, + CLIENT_PEM_FILE, CA_PEM_FILE, null, null, null, null, null); + UpstreamTlsContext upstreamTlsContext = CommonTlsContextTestsUtil.buildUpstreamTlsContext( + "google_cloud_private_spiffe-client", true, SNI_IN_UTC, true); + SslContextProviderSupplier sslContextProviderSupplier = + new SslContextProviderSupplier(upstreamTlsContext, + new TlsContextManagerImpl(bootstrapInfoForClient)); - @Test - public void sniInClientSecurityHandler_autoHostSniIsFalse_usesSniFromUpstreamTlsContext() { - CertificateUtils.isXdsSniEnabled = true; - try { - Bootstrapper.BootstrapInfo bootstrapInfoForClient = CommonBootstrapperTestUtils - .buildBootstrapInfo("google_cloud_private_spiffe-client", CLIENT_KEY_FILE, - CLIENT_PEM_FILE, CA_PEM_FILE, null, null, null, null, null); - UpstreamTlsContext upstreamTlsContext = CommonTlsContextTestsUtil.buildUpstreamTlsContext( - "google_cloud_private_spiffe-client", true, SNI_IN_UTC, false); - SslContextProviderSupplier sslContextProviderSupplier = - new SslContextProviderSupplier(upstreamTlsContext, - new TlsContextManagerImpl(bootstrapInfoForClient)); - - ClientSecurityHandler clientSecurityHandler = - new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, HOSTNAME); - - assertThat(clientSecurityHandler.getSni()).isEqualTo(SNI_IN_UTC); - } finally { - CertificateUtils.isXdsSniEnabled = false; - } + ClientSecurityHandler clientSecurityHandler = + new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, null); + + assertThat(clientSecurityHandler.getSni()).isEqualTo(SNI_IN_UTC); } @Test - public void sniFeatureNotEnabled_usesChannelAuthorityForSni() { - CertificateUtils.isXdsSniEnabled = false; + public void sniInClientSecurityHandler_autoHostSniIsFalse_usesSniFromUpstreamTlsContext() { Bootstrapper.BootstrapInfo bootstrapInfoForClient = CommonBootstrapperTestUtils - .buildBootstrapInfo("google_cloud_private_spiffe-client", CLIENT_KEY_FILE, - CLIENT_PEM_FILE, CA_PEM_FILE, null, null, null, null, null); - UpstreamTlsContext upstreamTlsContext = - CommonTlsContextTestsUtil - .buildUpstreamTlsContext("google_cloud_private_spiffe-client", true); + .buildBootstrapInfo("google_cloud_private_spiffe-client", CLIENT_KEY_FILE, + CLIENT_PEM_FILE, CA_PEM_FILE, null, null, null, null, null); + UpstreamTlsContext upstreamTlsContext = CommonTlsContextTestsUtil.buildUpstreamTlsContext( + "google_cloud_private_spiffe-client", true, SNI_IN_UTC, false); SslContextProviderSupplier sslContextProviderSupplier = - new SslContextProviderSupplier(upstreamTlsContext, - new TlsContextManagerImpl(bootstrapInfoForClient)); + new SslContextProviderSupplier(upstreamTlsContext, + new TlsContextManagerImpl(bootstrapInfoForClient)); ClientSecurityHandler clientSecurityHandler = - new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, HOSTNAME); + new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, HOSTNAME); - assertThat(clientSecurityHandler.getSni()).isEqualTo(FAKE_AUTHORITY); + assertThat(clientSecurityHandler.getSni()).isEqualTo(SNI_IN_UTC); } @Test @@ -504,59 +460,54 @@ public void nullTlsContext_nullFallbackProtocolNegotiator_expectException() { @Test public void clientSecurityProtocolNegotiatorNewHandler_fireProtocolNegotiationEvent() throws InterruptedException, TimeoutException, ExecutionException { - CertificateUtils.isXdsSniEnabled = true; - try { - FakeClock executor = new FakeClock(); - CommonCertProviderTestUtils.register(executor); - Bootstrapper.BootstrapInfo bootstrapInfoForClient = CommonBootstrapperTestUtils - .buildBootstrapInfo("google_cloud_private_spiffe-client", CLIENT_KEY_FILE, - CLIENT_PEM_FILE, CA_PEM_FILE, null, null, null, null, null); - UpstreamTlsContext upstreamTlsContext = - CommonTlsContextTestsUtil - .buildUpstreamTlsContext("google_cloud_private_spiffe-client", true); - - SslContextProviderSupplier sslContextProviderSupplier = - new SslContextProviderSupplier(upstreamTlsContext, - new TlsContextManagerImpl(bootstrapInfoForClient)); - ClientSecurityHandler clientSecurityHandler = - new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, HOSTNAME); - - pipeline.addLast(clientSecurityHandler); - channelHandlerCtx = pipeline.context(clientSecurityHandler); - assertNotNull(channelHandlerCtx); // non-null since we just added it - - // kick off protocol negotiation. - pipeline.fireUserEventTriggered(InternalProtocolNegotiationEvent.getDefault()); - final SettableFuture future = SettableFuture.create(); - sslContextProviderSupplier - .updateSslContext(new SslContextProvider.Callback(MoreExecutors.directExecutor()) { - @Override - public void updateSslContextAndExtendedX509TrustManager( - AbstractMap.SimpleImmutableEntry sslContextAndTm) { - future.set(sslContextAndTm); - } - - @Override - protected void onException(Throwable throwable) { - future.set(throwable); - } - }, false); - executor.runDueTasks(); - channel.runPendingTasks(); // need this for tasks to execute on eventLoop - Object fromFuture = future.get(5, TimeUnit.SECONDS); - assertThat(fromFuture).isInstanceOf(AbstractMap.SimpleImmutableEntry.class); - channel.runPendingTasks(); - channelHandlerCtx = pipeline.context(clientSecurityHandler); - assertThat(channelHandlerCtx).isNull(); - Object sslEvent = SslHandshakeCompletionEvent.SUCCESS; - - pipeline.fireUserEventTriggered(sslEvent); - channel.runPendingTasks(); // need this for tasks to execute on eventLoop - assertTrue(channel.isOpen()); - CommonCertProviderTestUtils.register0(); - } finally { - CertificateUtils.isXdsSniEnabled = false; - } + FakeClock executor = new FakeClock(); + CommonCertProviderTestUtils.register(executor); + Bootstrapper.BootstrapInfo bootstrapInfoForClient = CommonBootstrapperTestUtils + .buildBootstrapInfo("google_cloud_private_spiffe-client", CLIENT_KEY_FILE, + CLIENT_PEM_FILE, CA_PEM_FILE, null, null, null, null, null); + UpstreamTlsContext upstreamTlsContext = + CommonTlsContextTestsUtil + .buildUpstreamTlsContext("google_cloud_private_spiffe-client", true); + + SslContextProviderSupplier sslContextProviderSupplier = + new SslContextProviderSupplier(upstreamTlsContext, + new TlsContextManagerImpl(bootstrapInfoForClient)); + ClientSecurityHandler clientSecurityHandler = + new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, HOSTNAME); + + pipeline.addLast(clientSecurityHandler); + channelHandlerCtx = pipeline.context(clientSecurityHandler); + assertNotNull(channelHandlerCtx); // non-null since we just added it + + // kick off protocol negotiation. + pipeline.fireUserEventTriggered(InternalProtocolNegotiationEvent.getDefault()); + final SettableFuture future = SettableFuture.create(); + sslContextProviderSupplier + .updateSslContext(new SslContextProvider.Callback(MoreExecutors.directExecutor()) { + @Override + public void updateSslContextAndExtendedX509TrustManager( + AbstractMap.SimpleImmutableEntry sslContextAndTm) { + future.set(sslContextAndTm); + } + + @Override + protected void onException(Throwable throwable) { + future.set(throwable); + } + }, false); + executor.runDueTasks(); + channel.runPendingTasks(); // need this for tasks to execute on eventLoop + Object fromFuture = future.get(5, TimeUnit.SECONDS); + assertThat(fromFuture).isInstanceOf(AbstractMap.SimpleImmutableEntry.class); + channel.runPendingTasks(); + channelHandlerCtx = pipeline.context(clientSecurityHandler); + assertThat(channelHandlerCtx).isNull(); + Object sslEvent = SslHandshakeCompletionEvent.SUCCESS; + + pipeline.fireUserEventTriggered(sslEvent); + channel.runPendingTasks(); // need this for tasks to execute on eventLoop + assertTrue(channel.isOpen()); + CommonCertProviderTestUtils.register0(); } @Test From 5859f9bca66a01050b77e1e3c71c0bbc81bf2752 Mon Sep 17 00:00:00 2001 From: Kannan J Date: Wed, 5 Nov 2025 12:29:57 +0000 Subject: [PATCH 2/5] Remove unused var. --- xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java b/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java index c974f9dc475..fda22343ee2 100644 --- a/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java @@ -69,8 +69,6 @@ class XdsRouteConfigureResource extends XdsResourceType { - private static final String GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE = - "GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE"; @VisibleForTesting static boolean enableRouteLookup = GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_RLS_LB", true); From faa3dc06c1d6b2c2b8edc1ed79a3e66b4e068e63 Mon Sep 17 00:00:00 2001 From: Kannan J Date: Wed, 5 Nov 2025 16:45:50 +0000 Subject: [PATCH 3/5] Remove feature guarding of the env vars for Cloud run CSM: GRPC_EXPERIMENTAL_XDS_SNI GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE GRPC_EXPERIMENTAL_XDS_SYSTEM_ROOT_CERTS GRPC_EXPERIMENTAL_XDS_GCP_AUTHENTICATION_FILTER --- .../grpc/xds/GcpAuthenticationFilterTest.java | 4 +- .../grpc/xds/GrpcXdsClientImplDataTest.java | 48 +------------------ .../grpc/xds/XdsSecurityClientServerTest.java | 1 - .../test/java/io/grpc/xds/XdsTestUtils.java | 33 +++++++++++-- .../SecurityProtocolNegotiatorsTest.java | 1 - 5 files changed, 34 insertions(+), 53 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java index 4f8f2ed8f29..ae99374be3a 100644 --- a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java +++ b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java @@ -65,6 +65,7 @@ import io.grpc.xds.XdsEndpointResource.EdsUpdate; import io.grpc.xds.XdsListenerResource.LdsUpdate; import io.grpc.xds.XdsRouteConfigureResource.RdsUpdate; +import io.grpc.xds.client.Bootstrapper; import io.grpc.xds.client.Locality; import io.grpc.xds.client.XdsResourceType; import io.grpc.xds.client.XdsResourceType.ResourceInvalidException; @@ -72,7 +73,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -463,7 +463,7 @@ private static LdsUpdate getLdsUpdate() { private static RdsUpdate getRdsUpdate() { RouteConfiguration routeConfiguration = buildRouteConfiguration("my-server", RDS_NAME, CLUSTER_NAME); - XdsResourceType.Args args = new XdsResourceType.Args(null, "0", "0", null, null, null); + XdsResourceType.Args args = new XdsResourceType.Args(XdsTestUtils.EMPTY_BOOTSTRAPPER_SERVER_INFO, "0", "0", null, null, null); try { return XdsRouteConfigureResource.getInstance().doParse(args, routeConfiguration); } catch (ResourceInvalidException ex) { diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java index 79718722207..af0cdbb4951 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java @@ -530,7 +530,7 @@ public void parseRouteAction_withCluster() { } @Test - public void parseRouteAction_withCluster_autoHostRewriteEnabled() { + public void parseRouteAction_withCluster_autoHostRewrite() { io.envoyproxy.envoy.config.route.v3.RouteAction proto = io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder() .setCluster("cluster-foo") @@ -545,22 +545,6 @@ public void parseRouteAction_withCluster_autoHostRewriteEnabled() { assertThat(struct.getStruct().autoHostRewrite()).isTrue(); } - @Test - public void parseRouteAction_withCluster_flagDisabled_autoHostRewriteNotEnabled() { - io.envoyproxy.envoy.config.route.v3.RouteAction proto = - io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder() - .setCluster("cluster-foo") - .setAutoHostRewrite(BoolValue.of(true)) - .build(); - StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, - ImmutableMap.of(), ImmutableSet.of(), getXdsResourceTypeArgs(true)); - assertThat(struct.getErrorDetail()).isNull(); - assertThat(struct.getStruct().cluster()).isEqualTo("cluster-foo"); - assertThat(struct.getStruct().weightedClusters()).isNull(); - assertThat(struct.getStruct().autoHostRewrite()).isFalse(); - } - @Test public void parseRouteAction_withWeightedCluster() { io.envoyproxy.envoy.config.route.v3.RouteAction proto = @@ -589,7 +573,7 @@ public void parseRouteAction_withWeightedCluster() { } @Test - public void parseRouteAction_withWeightedCluster_autoHostRewriteEnabled() { + public void parseRouteAction_withWeightedCluster_autoHostRewrite() { io.envoyproxy.envoy.config.route.v3.RouteAction proto = io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder() .setWeightedClusters( @@ -616,34 +600,6 @@ public void parseRouteAction_withWeightedCluster_autoHostRewriteEnabled() { assertThat(struct.getStruct().autoHostRewrite()).isTrue(); } - @Test - public void parseRouteAction_withWeightedCluster_flagDisabled_autoHostRewriteDisabled() { - io.envoyproxy.envoy.config.route.v3.RouteAction proto = - io.envoyproxy.envoy.config.route.v3.RouteAction.newBuilder() - .setWeightedClusters( - WeightedCluster.newBuilder() - .addClusters( - WeightedCluster.ClusterWeight - .newBuilder() - .setName("cluster-foo") - .setWeight(UInt32Value.newBuilder().setValue(30))) - .addClusters(WeightedCluster.ClusterWeight - .newBuilder() - .setName("cluster-bar") - .setWeight(UInt32Value.newBuilder().setValue(70)))) - .setAutoHostRewrite(BoolValue.of(true)) - .build(); - StructOrError struct = - XdsRouteConfigureResource.parseRouteAction(proto, filterRegistry, - ImmutableMap.of(), ImmutableSet.of(), getXdsResourceTypeArgs(true)); - assertThat(struct.getErrorDetail()).isNull(); - assertThat(struct.getStruct().cluster()).isNull(); - assertThat(struct.getStruct().weightedClusters()).containsExactly( - ClusterWeight.create("cluster-foo", 30, ImmutableMap.of()), - ClusterWeight.create("cluster-bar", 70, ImmutableMap.of())); - assertThat(struct.getStruct().autoHostRewrite()).isFalse(); - } - @Test public void parseRouteAction_weightedClusterSum() { io.envoyproxy.envoy.config.route.v3.RouteAction proto = diff --git a/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java b/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java index 4df7f0ec4f7..c8ad9f1c670 100644 --- a/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java @@ -77,7 +77,6 @@ import io.grpc.xds.internal.security.SslContextProviderSupplier; import io.grpc.xds.internal.security.TlsContextManagerImpl; import io.grpc.xds.internal.security.certprovider.FileWatcherCertificateProviderProvider; -import io.grpc.xds.internal.security.trust.CertificateUtils; import io.netty.handler.ssl.NotSslRecordException; import java.io.File; import java.io.FileOutputStream; diff --git a/xds/src/test/java/io/grpc/xds/XdsTestUtils.java b/xds/src/test/java/io/grpc/xds/XdsTestUtils.java index 63bd139c2cd..fa118c18bc5 100644 --- a/xds/src/test/java/io/grpc/xds/XdsTestUtils.java +++ b/xds/src/test/java/io/grpc/xds/XdsTestUtils.java @@ -86,6 +86,33 @@ public class XdsTestUtils { + ".HttpConnectionManager"; public static final String ENDPOINT_HOSTNAME = "data-host"; public static final int ENDPOINT_PORT = 1234; + static final Bootstrapper.ServerInfo EMPTY_BOOTSTRAPPER_SERVER_INFO = + new Bootstrapper.ServerInfo() { + @Override + public String target() { + return null; + } + + @Override + public Object implSpecificConfig() { + return null; + } + + @Override + public boolean ignoreResourceDeletion() { + return false; + } + + @Override + public boolean isTrustedXdsServer() { + return false; + } + + @Override + public boolean resourceTimerIsTransientError() { + return false; + } + }; static BindableService createLrsService(AtomicBoolean lrsEnded, Queue loadReportCalls) { @@ -247,8 +274,8 @@ static XdsConfig getDefaultXdsConfig(String serverHostName) RouteConfiguration routeConfiguration = buildRouteConfiguration(serverHostName, RDS_NAME, CLUSTER_NAME); - Bootstrapper.ServerInfo serverInfo = null; - XdsResourceType.Args args = new XdsResourceType.Args(serverInfo, "0", "0", null, null, null); + XdsResourceType.Args args = new XdsResourceType.Args( + EMPTY_BOOTSTRAPPER_SERVER_INFO, "0", "0", null, null, null); XdsRouteConfigureResource.RdsUpdate rdsUpdate = XdsRouteConfigureResource.getInstance().doParse(args, routeConfiguration); @@ -268,7 +295,7 @@ static XdsConfig getDefaultXdsConfig(String serverHostName) XdsEndpointResource.EdsUpdate edsUpdate = new XdsEndpointResource.EdsUpdate( EDS_NAME, lbEndpointsMap, Collections.emptyList()); XdsClusterResource.CdsUpdate cdsUpdate = XdsClusterResource.CdsUpdate.forEds( - CLUSTER_NAME, EDS_NAME, serverInfo, null, null, null, false, null) + CLUSTER_NAME, EDS_NAME, EMPTY_BOOTSTRAPPER_SERVER_INFO, null, null, null, false, null) .lbPolicyConfig(getWrrLbConfigAsMap()).build(); XdsConfig.XdsClusterConfig clusterConfig = new XdsConfig.XdsClusterConfig( CLUSTER_NAME, cdsUpdate, new EndpointConfig(StatusOr.fromValue(edsUpdate))); diff --git a/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java b/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java index b27042f7a00..6c272b7c1d4 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java @@ -54,7 +54,6 @@ import io.grpc.xds.internal.security.SecurityProtocolNegotiators.ClientSecurityHandler; import io.grpc.xds.internal.security.SecurityProtocolNegotiators.ClientSecurityProtocolNegotiator; import io.grpc.xds.internal.security.certprovider.CommonCertProviderTestUtils; -import io.grpc.xds.internal.security.trust.CertificateUtils; import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPipeline; From 2ba9fd9da9520d5a7f178b8a4df9ae4e3dc245c3 Mon Sep 17 00:00:00 2001 From: Kannan J Date: Thu, 6 Nov 2025 09:55:24 +0000 Subject: [PATCH 4/5] fix tests. --- xds/src/test/java/io/grpc/xds/XdsDependencyManagerTest.java | 2 -- xds/src/test/java/io/grpc/xds/XdsTestUtils.java | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/XdsDependencyManagerTest.java b/xds/src/test/java/io/grpc/xds/XdsDependencyManagerTest.java index fca7b5220e6..603fc4cb707 100644 --- a/xds/src/test/java/io/grpc/xds/XdsDependencyManagerTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsDependencyManagerTest.java @@ -105,8 +105,6 @@ @RunWith(JUnit4.class) public class XdsDependencyManagerTest { private static final Logger log = Logger.getLogger(XdsDependencyManagerTest.class.getName()); - public static final String CLUSTER_TYPE_NAME = XdsClusterResource.getInstance().typeName(); - public static final String ENDPOINT_TYPE_NAME = XdsEndpointResource.getInstance().typeName(); private final SynchronizationContext syncContext = new SynchronizationContext((t, e) -> { diff --git a/xds/src/test/java/io/grpc/xds/XdsTestUtils.java b/xds/src/test/java/io/grpc/xds/XdsTestUtils.java index fa118c18bc5..4f3b4daad0e 100644 --- a/xds/src/test/java/io/grpc/xds/XdsTestUtils.java +++ b/xds/src/test/java/io/grpc/xds/XdsTestUtils.java @@ -105,7 +105,7 @@ public boolean ignoreResourceDeletion() { @Override public boolean isTrustedXdsServer() { - return false; + return true; } @Override @@ -295,7 +295,7 @@ static XdsConfig getDefaultXdsConfig(String serverHostName) XdsEndpointResource.EdsUpdate edsUpdate = new XdsEndpointResource.EdsUpdate( EDS_NAME, lbEndpointsMap, Collections.emptyList()); XdsClusterResource.CdsUpdate cdsUpdate = XdsClusterResource.CdsUpdate.forEds( - CLUSTER_NAME, EDS_NAME, EMPTY_BOOTSTRAPPER_SERVER_INFO, null, null, null, false, null) + CLUSTER_NAME, EDS_NAME, null, null, null, null, false, null) .lbPolicyConfig(getWrrLbConfigAsMap()).build(); XdsConfig.XdsClusterConfig clusterConfig = new XdsConfig.XdsClusterConfig( CLUSTER_NAME, cdsUpdate, new EndpointConfig(StatusOr.fromValue(edsUpdate))); From b605ab57b3b27915d5eda7a5faf92f317a5464e4 Mon Sep 17 00:00:00 2001 From: Kannan J Date: Thu, 6 Nov 2025 10:34:16 +0000 Subject: [PATCH 5/5] Fix style --- .../grpc/xds/GcpAuthenticationFilterTest.java | 4 +- .../test/java/io/grpc/xds/XdsTestUtils.java | 50 +++++++++---------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java index ae99374be3a..b58c4ed2aa5 100644 --- a/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java +++ b/xds/src/test/java/io/grpc/xds/GcpAuthenticationFilterTest.java @@ -65,7 +65,6 @@ import io.grpc.xds.XdsEndpointResource.EdsUpdate; import io.grpc.xds.XdsListenerResource.LdsUpdate; import io.grpc.xds.XdsRouteConfigureResource.RdsUpdate; -import io.grpc.xds.client.Bootstrapper; import io.grpc.xds.client.Locality; import io.grpc.xds.client.XdsResourceType; import io.grpc.xds.client.XdsResourceType.ResourceInvalidException; @@ -463,7 +462,8 @@ private static LdsUpdate getLdsUpdate() { private static RdsUpdate getRdsUpdate() { RouteConfiguration routeConfiguration = buildRouteConfiguration("my-server", RDS_NAME, CLUSTER_NAME); - XdsResourceType.Args args = new XdsResourceType.Args(XdsTestUtils.EMPTY_BOOTSTRAPPER_SERVER_INFO, "0", "0", null, null, null); + XdsResourceType.Args args = new XdsResourceType.Args( + XdsTestUtils.EMPTY_BOOTSTRAPPER_SERVER_INFO, "0", "0", null, null, null); try { return XdsRouteConfigureResource.getInstance().doParse(args, routeConfiguration); } catch (ResourceInvalidException ex) { diff --git a/xds/src/test/java/io/grpc/xds/XdsTestUtils.java b/xds/src/test/java/io/grpc/xds/XdsTestUtils.java index 4f3b4daad0e..d31187756cd 100644 --- a/xds/src/test/java/io/grpc/xds/XdsTestUtils.java +++ b/xds/src/test/java/io/grpc/xds/XdsTestUtils.java @@ -88,31 +88,31 @@ public class XdsTestUtils { public static final int ENDPOINT_PORT = 1234; static final Bootstrapper.ServerInfo EMPTY_BOOTSTRAPPER_SERVER_INFO = new Bootstrapper.ServerInfo() { - @Override - public String target() { - return null; - } - - @Override - public Object implSpecificConfig() { - return null; - } - - @Override - public boolean ignoreResourceDeletion() { - return false; - } - - @Override - public boolean isTrustedXdsServer() { - return true; - } - - @Override - public boolean resourceTimerIsTransientError() { - return false; - } - }; + @Override + public String target() { + return null; + } + + @Override + public Object implSpecificConfig() { + return null; + } + + @Override + public boolean ignoreResourceDeletion() { + return false; + } + + @Override + public boolean isTrustedXdsServer() { + return true; + } + + @Override + public boolean resourceTimerIsTransientError() { + return false; + } + }; static BindableService createLrsService(AtomicBoolean lrsEnded, Queue loadReportCalls) {