From f142fa19ddf43b1919f7d82341e8f84656ba8351 Mon Sep 17 00:00:00 2001 From: adam lazur Date: Wed, 22 Oct 2025 22:34:56 +0900 Subject: [PATCH 1/5] xds: Support deprecated tls_certificate_certificate_provider_instance field Add backward compatibility for deprecated certificate provider field 11 (tls_certificate_certificate_provider_instance) by falling back to it when field 14 (tls_certificate_provider_instance) is not present. This matches the behavior of grpc-go and grpc-cpp, enabling compatibility with Istio which sends the deprecated field for backward compatibility with older Envoy versions. Amp-Thread-ID: https://ampcode.com/threads/T-a71beee4-6f09-48fb-a8f8-9f2e09c1623f Co-authored-by: Amp --- .../java/io/grpc/xds/XdsClusterResource.java | 7 ++- .../security/CommonTlsContextUtil.java | 15 ++++++ .../CertProviderSslContextProvider.java | 9 +++- .../security/CommonTlsContextTestsUtil.java | 26 ++++++++++ ...tProviderClientSslContextProviderTest.java | 52 +++++++++++++++++++ 5 files changed, 107 insertions(+), 2 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java index 54089491671..26577c464d8 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java @@ -541,7 +541,12 @@ private static String getIdentityCertInstanceName(CommonTlsContext commonTlsCont if (commonTlsContext.hasTlsCertificateProviderInstance()) { return commonTlsContext.getTlsCertificateProviderInstance().getInstanceName(); } - return null; + // Fall back to deprecated field (field 11) for backward compatibility with Istio + @SuppressWarnings("deprecation") + String instanceName = commonTlsContext.hasTlsCertificateCertificateProviderInstance() + ? commonTlsContext.getTlsCertificateCertificateProviderInstance().getInstanceName() + : null; + return instanceName; } private static String getRootCertInstanceName(CommonTlsContext commonTlsContext) { diff --git a/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java b/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java index 50fa18b64f9..7cece77b470 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java @@ -28,7 +28,10 @@ public static boolean hasCertProviderInstance(CommonTlsContext commonTlsContext) if (commonTlsContext == null) { return false; } + @SuppressWarnings("deprecation") + boolean hasDeprecatedField = commonTlsContext.hasTlsCertificateCertificateProviderInstance(); return commonTlsContext.hasTlsCertificateProviderInstance() + || hasDeprecatedField || hasValidationProviderInstance(commonTlsContext); } @@ -53,6 +56,18 @@ public static CommonTlsContext.CertificateProviderInstance convert( .setCertificateName(pluginInstance.getCertificateName()).build(); } + /** + * Converts deprecated {@link CommonTlsContext.CertificateProviderInstance} (field 11) to + * internal {@link CommonTlsContext.CertificateProviderInstance}. + * This supports a deprecated field for backward compatibility (primarily for Istio) + */ + public static CommonTlsContext.CertificateProviderInstance convertDeprecated( + CommonTlsContext.CertificateProviderInstance deprecatedInstance) { + return CommonTlsContext.CertificateProviderInstance.newBuilder() + .setInstanceName(deprecatedInstance.getInstanceName()) + .setCertificateName(deprecatedInstance.getCertificateName()).build(); + } + public static boolean isUsingSystemRootCerts(CommonTlsContext commonTlsContext) { if (commonTlsContext.hasCombinedValidationContext()) { return commonTlsContext.getCombinedValidationContext().getDefaultValidationContext() diff --git a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java index 2570dcb731b..4244b51652c 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java @@ -113,7 +113,14 @@ protected static CertificateProviderInstance getCertProviderInstance( if (commonTlsContext.hasTlsCertificateProviderInstance()) { return CommonTlsContextUtil.convert(commonTlsContext.getTlsCertificateProviderInstance()); } - return null; + // Fall back to deprecated field for backward compatibility with Istio + @SuppressWarnings("deprecation") + CertificateProviderInstance deprecatedInstance = + commonTlsContext.hasTlsCertificateCertificateProviderInstance() + ? CommonTlsContextUtil.convertDeprecated( + commonTlsContext.getTlsCertificateCertificateProviderInstance()) + : null; + return deprecatedInstance; } @Nullable diff --git a/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java b/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java index 80a8083fb27..65429754dfd 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java @@ -232,6 +232,32 @@ private static CommonTlsContext buildCommonTlsContextForCertProviderInstance( return builder.build(); } + /** Helper method to build CommonTlsContext using deprecated certificate provider field. */ + public static CommonTlsContext buildCommonTlsContextWithDeprecatedCertProviderInstance( + String certInstanceName, + String certName, + String rootInstanceName, + String rootCertName, + Iterable alpnProtocols, + CertificateValidationContext staticCertValidationContext) { + CommonTlsContext.Builder builder = CommonTlsContext.newBuilder(); + if (certInstanceName != null) { + // Use deprecated field (field 11) instead of current field (field 14) + builder = + builder.setTlsCertificateCertificateProviderInstance( + CommonTlsContext.CertificateProviderInstance.newBuilder() + .setInstanceName(certInstanceName) + .setCertificateName(certName)); + } + builder = + addCertificateValidationContext( + builder, rootInstanceName, rootCertName, staticCertValidationContext); + if (alpnProtocols != null) { + builder.addAllAlpnProtocols(alpnProtocols); + } + return builder.build(); + } + private static CommonTlsContext buildNewCommonTlsContextForCertProviderInstance( String certInstanceName, String certName, diff --git a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderTest.java b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderTest.java index 875a57b8f3d..91f02863ca4 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderTest.java @@ -470,6 +470,58 @@ public void testProviderForClient_rootInstanceNull_but_isUsingSystemRootCerts_va .build(), false); } + @Test + public void testProviderForClient_deprecatedCertProviderField() throws Exception { + final CertificateProvider.DistributorWatcher[] watcherCaptor = + new CertificateProvider.DistributorWatcher[1]; + TestCertificateProvider.createAndRegisterProviderProvider( + certificateProviderRegistry, watcherCaptor, "testca", 0); + + // Build UpstreamTlsContext using deprecated field + EnvoyServerProtoData.UpstreamTlsContext upstreamTlsContext = + new EnvoyServerProtoData.UpstreamTlsContext( + CommonTlsContextTestsUtil.buildCommonTlsContextWithDeprecatedCertProviderInstance( + "gcp_id", + "cert-default", + "gcp_id", + "root-default", + /* alpnProtocols= */ null, + /* staticCertValidationContext= */ null)); + + Bootstrapper.BootstrapInfo bootstrapInfo = CommonBootstrapperTestUtils.getTestBootstrapInfo(); + CertProviderClientSslContextProvider provider = + (CertProviderClientSslContextProvider) + certProviderClientSslContextProviderFactory.getProvider( + upstreamTlsContext, + bootstrapInfo.node().toEnvoyProtoNode(), + bootstrapInfo.certProviders()); + + assertThat(provider.savedKey).isNull(); + assertThat(provider.savedCertChain).isNull(); + assertThat(provider.savedTrustedRoots).isNull(); + assertThat(provider.getSslContextAndTrustManager()).isNull(); + + // Generate cert update + watcherCaptor[0].updateCertificate( + CommonCertProviderTestUtils.getPrivateKey(CLIENT_KEY_FILE), + ImmutableList.of(getCertFromResourceName(CLIENT_PEM_FILE))); + assertThat(provider.savedKey).isNotNull(); + assertThat(provider.savedCertChain).isNotNull(); + assertThat(provider.getSslContextAndTrustManager()).isNull(); + + // Generate root cert update + watcherCaptor[0].updateTrustedRoots(ImmutableList.of(getCertFromResourceName(CA_PEM_FILE))); + assertThat(provider.getSslContextAndTrustManager()).isNotNull(); + assertThat(provider.savedKey).isNull(); + assertThat(provider.savedCertChain).isNull(); + assertThat(provider.savedTrustedRoots).isNull(); + + TestCallback testCallback = + CommonTlsContextTestsUtil.getValueThruCallback(provider); + + doChecksOnSslContext(false, testCallback.updatedSslContext, /* expectedApnProtos= */ null); + } + static class QueuedExecutor implements Executor { /** A list of Runnables to be run in order. */ @VisibleForTesting final Queue runQueue = new ConcurrentLinkedQueue<>(); From d2986a142c5dd7c46a30ac869784785f433b2157 Mon Sep 17 00:00:00 2001 From: adam lazur Date: Fri, 24 Oct 2025 00:20:09 +0900 Subject: [PATCH 2/5] drop un-necessary conversion --- .../xds/internal/security/CommonTlsContextUtil.java | 12 ------------ .../certprovider/CertProviderSslContextProvider.java | 3 +-- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java b/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java index 7cece77b470..ca291215a43 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java @@ -56,18 +56,6 @@ public static CommonTlsContext.CertificateProviderInstance convert( .setCertificateName(pluginInstance.getCertificateName()).build(); } - /** - * Converts deprecated {@link CommonTlsContext.CertificateProviderInstance} (field 11) to - * internal {@link CommonTlsContext.CertificateProviderInstance}. - * This supports a deprecated field for backward compatibility (primarily for Istio) - */ - public static CommonTlsContext.CertificateProviderInstance convertDeprecated( - CommonTlsContext.CertificateProviderInstance deprecatedInstance) { - return CommonTlsContext.CertificateProviderInstance.newBuilder() - .setInstanceName(deprecatedInstance.getInstanceName()) - .setCertificateName(deprecatedInstance.getCertificateName()).build(); - } - public static boolean isUsingSystemRootCerts(CommonTlsContext commonTlsContext) { if (commonTlsContext.hasCombinedValidationContext()) { return commonTlsContext.getCombinedValidationContext().getDefaultValidationContext() diff --git a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java index 4244b51652c..cb99ca6ad95 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java @@ -117,8 +117,7 @@ protected static CertificateProviderInstance getCertProviderInstance( @SuppressWarnings("deprecation") CertificateProviderInstance deprecatedInstance = commonTlsContext.hasTlsCertificateCertificateProviderInstance() - ? CommonTlsContextUtil.convertDeprecated( - commonTlsContext.getTlsCertificateCertificateProviderInstance()) + ? commonTlsContext.getTlsCertificateCertificateProviderInstance() : null; return deprecatedInstance; } From 3b532a3218a79f85c870efaf214e1f941edf474c Mon Sep 17 00:00:00 2001 From: adam lazur Date: Fri, 24 Oct 2025 10:58:25 +0900 Subject: [PATCH 3/5] xds: Suppress deprecation warning in test helper Add @SuppressWarnings("deprecation") to test helper that intentionally uses deprecated field to verify backward compatibility. --- .../io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java | 1 + 1 file changed, 1 insertion(+) diff --git a/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java b/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java index 65429754dfd..abacd2038f8 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java @@ -233,6 +233,7 @@ private static CommonTlsContext buildCommonTlsContextForCertProviderInstance( } /** Helper method to build CommonTlsContext using deprecated certificate provider field. */ + @SuppressWarnings("deprecation") public static CommonTlsContext buildCommonTlsContextWithDeprecatedCertProviderInstance( String certInstanceName, String certName, From cf4a2a1142521abd95cc3ba233eb233dde282738 Mon Sep 17 00:00:00 2001 From: adam lazur Date: Fri, 24 Oct 2025 23:51:37 +0900 Subject: [PATCH 4/5] code coverage --- .../java/io/grpc/xds/GrpcXdsClientImplDataTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java index 2c75ee04d91..1e75c09aa16 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java @@ -3138,6 +3138,18 @@ public void validateCommonTlsContext_tlsNewCertificateProviderInstance() .validateCommonTlsContext(commonTlsContext, ImmutableSet.of("name1", "name2"), true); } + @Test + @SuppressWarnings("deprecation") + public void validateCommonTlsContext_tlsDeprecatedCertificateProviderInstance() + throws ResourceInvalidException { + CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() + .setTlsCertificateCertificateProviderInstance( + CommonTlsContext.CertificateProviderInstance.newBuilder().setInstanceName("name1")) + .build(); + XdsClusterResource + .validateCommonTlsContext(commonTlsContext, ImmutableSet.of("name1", "name2"), true); + } + @Test public void validateCommonTlsContext_tlsCertificateProviderInstance() throws ResourceInvalidException { From 6dbd3bf1d08882fc522ee19de19effc0a44f829e Mon Sep 17 00:00:00 2001 From: adam lazur Date: Sat, 25 Oct 2025 19:47:18 +0900 Subject: [PATCH 5/5] xds: Support deprecated field in CombinedValidationContext Add fallback to deprecated validation_context_certificate_provider_instance (field 4) in CombinedValidationContext for Istio compatibility. --- .../java/io/grpc/xds/XdsClusterResource.java | 10 ++++++++++ .../security/CommonTlsContextUtil.java | 16 +++++++++++++--- .../io/grpc/xds/GrpcXdsClientImplDataTest.java | 18 ++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java index 26577c464d8..e1472d493d4 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java @@ -564,6 +564,16 @@ private static String getRootCertInstanceName(CommonTlsContext commonTlsContext) return combinedCertificateValidationContext.getDefaultValidationContext() .getCaCertificateProviderInstance().getInstanceName(); } + // Fall back to deprecated field (field 4) in CombinedValidationContext + @SuppressWarnings("deprecation") + String instanceName = combinedCertificateValidationContext + .hasValidationContextCertificateProviderInstance() + ? combinedCertificateValidationContext.getValidationContextCertificateProviderInstance() + .getInstanceName() + : null; + if (instanceName != null) { + return instanceName; + } } return null; } diff --git a/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java b/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java index ca291215a43..bd8a423e683 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java @@ -40,9 +40,19 @@ private static boolean hasValidationProviderInstance(CommonTlsContext commonTlsC .hasCaCertificateProviderInstance()) { return true; } - return commonTlsContext.hasCombinedValidationContext() - && commonTlsContext.getCombinedValidationContext().getDefaultValidationContext() - .hasCaCertificateProviderInstance(); + if (commonTlsContext.hasCombinedValidationContext()) { + CommonTlsContext.CombinedCertificateValidationContext combined = + commonTlsContext.getCombinedValidationContext(); + if (combined.hasDefaultValidationContext() + && combined.getDefaultValidationContext().hasCaCertificateProviderInstance()) { + return true; + } + // Check deprecated field (field 4) in CombinedValidationContext + @SuppressWarnings("deprecation") + boolean hasDeprecatedField = combined.hasValidationContextCertificateProviderInstance(); + return hasDeprecatedField; + } + return false; } /** diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java index 1e75c09aa16..30d37f59d39 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java @@ -3234,6 +3234,24 @@ public void validateCommonTlsContext_combinedValidationContextSystemRootCerts() .validateCommonTlsContext(commonTlsContext, ImmutableSet.of(), false); } + @Test + @SuppressWarnings("deprecation") + public void validateCommonTlsContext_combinedValidationContextDeprecatedCertProvider() + throws ResourceInvalidException { + CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() + .setTlsCertificateProviderInstance( + CertificateProviderPluginInstance.newBuilder().setInstanceName("cert1")) + .setCombinedValidationContext( + CommonTlsContext.CombinedCertificateValidationContext.newBuilder() + .setValidationContextCertificateProviderInstance( + CommonTlsContext.CertificateProviderInstance.newBuilder() + .setInstanceName("root1")) + .build()) + .build(); + XdsClusterResource + .validateCommonTlsContext(commonTlsContext, ImmutableSet.of("cert1", "root1"), true); + } + @Test public void validateCommonTlsContext_validationContextSystemRootCerts_envVarNotSet_throws() { XdsClusterResource.enableSystemRootCerts = false;