Skip to content

Commit 53cd1a2

Browse files
laz-canvaampcode-com
authored andcommitted
xds: Support deprecated xDS TLS fields for Istio compat (#12435)
## Problem When using xDS with Istio's grpc-agent in proxyless mode, Java gRPC fails with: ``` LDS response Listener validation error: tls_certificate_provider_instance is required in downstream-tls-context ``` **Root Cause:** Istio sends deprecated certificate provider fields for backward compatibility with older Envoy versions. Java gRPC currently only reads the current fields, causing validation failures. Specifically, Istio uses these deprecated fields: 1. **Field 11**: `tls_certificate_certificate_provider_instance` (deprecated) instead of field 14 (`tls_certificate_provider_instance`) 2. **Field 4**: `validation_context_certificate_provider_instance` in `CombinedValidationContext` (deprecated) instead of `ca_certificate_provider_instance` in `default_validation_context` ## Fix Istio is adding support for the new fields in istio/istio#58257. Add fallback logic to support deprecated certificate provider fields before that is rolled out: **For identity certificates:** 1. Try current field 14 (`tls_certificate_provider_instance`) first 2. Fall back to deprecated field 11 (`tls_certificate_certificate_provider_instance`) **For validation context in CombinedValidationContext:** 1. Try `ca_certificate_provider_instance` in `default_validation_context` first 2. Fall back to deprecated field 4 (`validation_context_certificate_provider_instance`) This matches the behavior of [grpc-cpp](https://github.com/grpc/grpc/blob/master/src/core/xds/grpc/xds_common_types_parser.cc#L435-L474) and [grpc-go](https://github.com/grpc/grpc-go/blob/master/internal/xds/xdsclient/xdsresource/unmarshal_cds.go#L310-L344) implementations. ## Testing * Added new tests for both deprecated field paths (field 11 and field 4) * All existing tests pass * Manual local testing with Istio in proxyless mode verified the compatibility fix works --------- Co-authored-by: Amp <amp@ampcode.com>
1 parent 6fc3fd0 commit 53cd1a2

File tree

6 files changed

+148
-5
lines changed

6 files changed

+148
-5
lines changed

xds/src/main/java/io/grpc/xds/XdsClusterResource.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,12 @@ private static String getIdentityCertInstanceName(CommonTlsContext commonTlsCont
541541
if (commonTlsContext.hasTlsCertificateProviderInstance()) {
542542
return commonTlsContext.getTlsCertificateProviderInstance().getInstanceName();
543543
}
544-
return null;
544+
// Fall back to deprecated field (field 11) for backward compatibility with Istio
545+
@SuppressWarnings("deprecation")
546+
String instanceName = commonTlsContext.hasTlsCertificateCertificateProviderInstance()
547+
? commonTlsContext.getTlsCertificateCertificateProviderInstance().getInstanceName()
548+
: null;
549+
return instanceName;
545550
}
546551

547552
private static String getRootCertInstanceName(CommonTlsContext commonTlsContext) {
@@ -559,6 +564,16 @@ private static String getRootCertInstanceName(CommonTlsContext commonTlsContext)
559564
return combinedCertificateValidationContext.getDefaultValidationContext()
560565
.getCaCertificateProviderInstance().getInstanceName();
561566
}
567+
// Fall back to deprecated field (field 4) in CombinedValidationContext
568+
@SuppressWarnings("deprecation")
569+
String instanceName = combinedCertificateValidationContext
570+
.hasValidationContextCertificateProviderInstance()
571+
? combinedCertificateValidationContext.getValidationContextCertificateProviderInstance()
572+
.getInstanceName()
573+
: null;
574+
if (instanceName != null) {
575+
return instanceName;
576+
}
562577
}
563578
return null;
564579
}

xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ public static boolean hasCertProviderInstance(CommonTlsContext commonTlsContext)
2828
if (commonTlsContext == null) {
2929
return false;
3030
}
31+
@SuppressWarnings("deprecation")
32+
boolean hasDeprecatedField = commonTlsContext.hasTlsCertificateCertificateProviderInstance();
3133
return commonTlsContext.hasTlsCertificateProviderInstance()
34+
|| hasDeprecatedField
3235
|| hasValidationProviderInstance(commonTlsContext);
3336
}
3437

@@ -37,9 +40,19 @@ private static boolean hasValidationProviderInstance(CommonTlsContext commonTlsC
3740
.hasCaCertificateProviderInstance()) {
3841
return true;
3942
}
40-
return commonTlsContext.hasCombinedValidationContext()
41-
&& commonTlsContext.getCombinedValidationContext().getDefaultValidationContext()
42-
.hasCaCertificateProviderInstance();
43+
if (commonTlsContext.hasCombinedValidationContext()) {
44+
CommonTlsContext.CombinedCertificateValidationContext combined =
45+
commonTlsContext.getCombinedValidationContext();
46+
if (combined.hasDefaultValidationContext()
47+
&& combined.getDefaultValidationContext().hasCaCertificateProviderInstance()) {
48+
return true;
49+
}
50+
// Check deprecated field (field 4) in CombinedValidationContext
51+
@SuppressWarnings("deprecation")
52+
boolean hasDeprecatedField = combined.hasValidationContextCertificateProviderInstance();
53+
return hasDeprecatedField;
54+
}
55+
return false;
4356
}
4457

4558
/**

xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,13 @@ protected static CertificateProviderInstance getCertProviderInstance(
113113
if (commonTlsContext.hasTlsCertificateProviderInstance()) {
114114
return CommonTlsContextUtil.convert(commonTlsContext.getTlsCertificateProviderInstance());
115115
}
116-
return null;
116+
// Fall back to deprecated field for backward compatibility with Istio
117+
@SuppressWarnings("deprecation")
118+
CertificateProviderInstance deprecatedInstance =
119+
commonTlsContext.hasTlsCertificateCertificateProviderInstance()
120+
? commonTlsContext.getTlsCertificateCertificateProviderInstance()
121+
: null;
122+
return deprecatedInstance;
117123
}
118124

119125
@Nullable

xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3134,6 +3134,18 @@ public void validateCommonTlsContext_tlsNewCertificateProviderInstance()
31343134
.validateCommonTlsContext(commonTlsContext, ImmutableSet.of("name1", "name2"), true);
31353135
}
31363136

3137+
@Test
3138+
@SuppressWarnings("deprecation")
3139+
public void validateCommonTlsContext_tlsDeprecatedCertificateProviderInstance()
3140+
throws ResourceInvalidException {
3141+
CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder()
3142+
.setTlsCertificateCertificateProviderInstance(
3143+
CommonTlsContext.CertificateProviderInstance.newBuilder().setInstanceName("name1"))
3144+
.build();
3145+
XdsClusterResource
3146+
.validateCommonTlsContext(commonTlsContext, ImmutableSet.of("name1", "name2"), true);
3147+
}
3148+
31373149
@Test
31383150
public void validateCommonTlsContext_tlsCertificateProviderInstance()
31393151
throws ResourceInvalidException {
@@ -3218,6 +3230,24 @@ public void validateCommonTlsContext_combinedValidationContextSystemRootCerts()
32183230
.validateCommonTlsContext(commonTlsContext, ImmutableSet.of(), false);
32193231
}
32203232

3233+
@Test
3234+
@SuppressWarnings("deprecation")
3235+
public void validateCommonTlsContext_combinedValidationContextDeprecatedCertProvider()
3236+
throws ResourceInvalidException {
3237+
CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder()
3238+
.setTlsCertificateProviderInstance(
3239+
CertificateProviderPluginInstance.newBuilder().setInstanceName("cert1"))
3240+
.setCombinedValidationContext(
3241+
CommonTlsContext.CombinedCertificateValidationContext.newBuilder()
3242+
.setValidationContextCertificateProviderInstance(
3243+
CommonTlsContext.CertificateProviderInstance.newBuilder()
3244+
.setInstanceName("root1"))
3245+
.build())
3246+
.build();
3247+
XdsClusterResource
3248+
.validateCommonTlsContext(commonTlsContext, ImmutableSet.of("cert1", "root1"), true);
3249+
}
3250+
32213251
@Test
32223252
public void validateCommonTlsContext_validationContextSystemRootCerts_envVarNotSet_throws() {
32233253
XdsClusterResource.enableSystemRootCerts = false;

xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,33 @@ private static CommonTlsContext buildCommonTlsContextForCertProviderInstance(
232232
return builder.build();
233233
}
234234

235+
/** Helper method to build CommonTlsContext using deprecated certificate provider field. */
236+
@SuppressWarnings("deprecation")
237+
public static CommonTlsContext buildCommonTlsContextWithDeprecatedCertProviderInstance(
238+
String certInstanceName,
239+
String certName,
240+
String rootInstanceName,
241+
String rootCertName,
242+
Iterable<String> alpnProtocols,
243+
CertificateValidationContext staticCertValidationContext) {
244+
CommonTlsContext.Builder builder = CommonTlsContext.newBuilder();
245+
if (certInstanceName != null) {
246+
// Use deprecated field (field 11) instead of current field (field 14)
247+
builder =
248+
builder.setTlsCertificateCertificateProviderInstance(
249+
CommonTlsContext.CertificateProviderInstance.newBuilder()
250+
.setInstanceName(certInstanceName)
251+
.setCertificateName(certName));
252+
}
253+
builder =
254+
addCertificateValidationContext(
255+
builder, rootInstanceName, rootCertName, staticCertValidationContext);
256+
if (alpnProtocols != null) {
257+
builder.addAllAlpnProtocols(alpnProtocols);
258+
}
259+
return builder.build();
260+
}
261+
235262
private static CommonTlsContext buildNewCommonTlsContextForCertProviderInstance(
236263
String certInstanceName,
237264
String certName,

xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderTest.java

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,58 @@ public void testProviderForClient_rootInstanceNull_but_isUsingSystemRootCerts_va
470470
.build(), false);
471471
}
472472

473+
@Test
474+
public void testProviderForClient_deprecatedCertProviderField() throws Exception {
475+
final CertificateProvider.DistributorWatcher[] watcherCaptor =
476+
new CertificateProvider.DistributorWatcher[1];
477+
TestCertificateProvider.createAndRegisterProviderProvider(
478+
certificateProviderRegistry, watcherCaptor, "testca", 0);
479+
480+
// Build UpstreamTlsContext using deprecated field
481+
EnvoyServerProtoData.UpstreamTlsContext upstreamTlsContext =
482+
new EnvoyServerProtoData.UpstreamTlsContext(
483+
CommonTlsContextTestsUtil.buildCommonTlsContextWithDeprecatedCertProviderInstance(
484+
"gcp_id",
485+
"cert-default",
486+
"gcp_id",
487+
"root-default",
488+
/* alpnProtocols= */ null,
489+
/* staticCertValidationContext= */ null));
490+
491+
Bootstrapper.BootstrapInfo bootstrapInfo = CommonBootstrapperTestUtils.getTestBootstrapInfo();
492+
CertProviderClientSslContextProvider provider =
493+
(CertProviderClientSslContextProvider)
494+
certProviderClientSslContextProviderFactory.getProvider(
495+
upstreamTlsContext,
496+
bootstrapInfo.node().toEnvoyProtoNode(),
497+
bootstrapInfo.certProviders());
498+
499+
assertThat(provider.savedKey).isNull();
500+
assertThat(provider.savedCertChain).isNull();
501+
assertThat(provider.savedTrustedRoots).isNull();
502+
assertThat(provider.getSslContextAndTrustManager()).isNull();
503+
504+
// Generate cert update
505+
watcherCaptor[0].updateCertificate(
506+
CommonCertProviderTestUtils.getPrivateKey(CLIENT_KEY_FILE),
507+
ImmutableList.of(getCertFromResourceName(CLIENT_PEM_FILE)));
508+
assertThat(provider.savedKey).isNotNull();
509+
assertThat(provider.savedCertChain).isNotNull();
510+
assertThat(provider.getSslContextAndTrustManager()).isNull();
511+
512+
// Generate root cert update
513+
watcherCaptor[0].updateTrustedRoots(ImmutableList.of(getCertFromResourceName(CA_PEM_FILE)));
514+
assertThat(provider.getSslContextAndTrustManager()).isNotNull();
515+
assertThat(provider.savedKey).isNull();
516+
assertThat(provider.savedCertChain).isNull();
517+
assertThat(provider.savedTrustedRoots).isNull();
518+
519+
TestCallback testCallback =
520+
CommonTlsContextTestsUtil.getValueThruCallback(provider);
521+
522+
doChecksOnSslContext(false, testCallback.updatedSslContext, /* expectedApnProtos= */ null);
523+
}
524+
473525
static class QueuedExecutor implements Executor {
474526
/** A list of Runnables to be run in order. */
475527
@VisibleForTesting final Queue<Runnable> runQueue = new ConcurrentLinkedQueue<>();

0 commit comments

Comments
 (0)