From 2fe4b844b3ac15e5ff58c6871bdef1ce8f0a6084 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 3 Apr 2025 15:00:29 -0700 Subject: [PATCH 1/5] xds: Enable deprecation warnings The security code referenced fields removed from gRFC A29 before it was finalized. --- xds/build.gradle | 2 -- xds/src/main/java/io/grpc/xds/RbacFilter.java | 9 ++++++-- .../java/io/grpc/xds/XdsClusterResource.java | 23 +++---------------- .../grpc/xds/XdsRouteConfigureResource.java | 3 +-- .../io/grpc/xds/internal/MatcherParser.java | 17 ++++++++++---- .../security/CommonTlsContextUtil.java | 15 +++--------- .../CertProviderSslContextProvider.java | 11 --------- .../security/trust/XdsX509TrustManager.java | 1 + 8 files changed, 28 insertions(+), 53 deletions(-) diff --git a/xds/build.gradle b/xds/build.gradle index cdd4924cab3..90ba3709d14 100644 --- a/xds/build.gradle +++ b/xds/build.gradle @@ -133,8 +133,6 @@ tasks.named("checkstyleThirdparty").configure { tasks.named("compileJava").configure { it.options.compilerArgs += [ - // TODO: remove - "-Xlint:-deprecation", // only has AutoValue annotation processor "-Xlint:-processing", ] diff --git a/xds/src/main/java/io/grpc/xds/RbacFilter.java b/xds/src/main/java/io/grpc/xds/RbacFilter.java index d91884735e9..91df1e68802 100644 --- a/xds/src/main/java/io/grpc/xds/RbacFilter.java +++ b/xds/src/main/java/io/grpc/xds/RbacFilter.java @@ -276,8 +276,13 @@ private static Matcher parsePrincipal(Principal principal) { return createSourceIpMatcher(principal.getDirectRemoteIp()); case REMOTE_IP: return createSourceIpMatcher(principal.getRemoteIp()); - case SOURCE_IP: - return createSourceIpMatcher(principal.getSourceIp()); + case SOURCE_IP: { + // gRFC A41 has identical handling of source_ip as remote_ip and direct_remote_ip and + // pre-dates the deprecation. + @SuppressWarnings("deprecation") + CidrRange sourceIp = principal.getSourceIp(); + return createSourceIpMatcher(sourceIp); + } case HEADER: return parseHeaderMatcher(principal.getHeader()); case NOT_ID: diff --git a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java index cfc74f3ca70..0d9274e2869 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java @@ -450,15 +450,6 @@ static void validateCommonTlsContext( throw new ResourceInvalidException( "common-tls-context with validation_context_sds_secret_config is not supported"); } - if (commonTlsContext.hasValidationContextCertificateProvider()) { - throw new ResourceInvalidException( - "common-tls-context with validation_context_certificate_provider is not supported"); - } - if (commonTlsContext.hasValidationContextCertificateProviderInstance()) { - throw new ResourceInvalidException( - "common-tls-context with validation_context_certificate_provider_instance is not" - + " supported"); - } String certInstanceName = getIdentityCertInstanceName(commonTlsContext); if (certInstanceName == null) { if (server) { @@ -473,10 +464,6 @@ static void validateCommonTlsContext( throw new ResourceInvalidException( "tls_certificate_provider_instance is unset"); } - if (commonTlsContext.hasTlsCertificateCertificateProvider()) { - throw new ResourceInvalidException( - "tls_certificate_provider_instance is unset"); - } } else if (certProviderInstances == null || !certProviderInstances.contains(certInstanceName)) { throw new ResourceInvalidException( "CertificateProvider instance name '" + certInstanceName @@ -505,7 +492,9 @@ static void validateCommonTlsContext( .getDefaultValidationContext(); } if (certificateValidationContext != null) { - if (certificateValidationContext.getMatchSubjectAltNamesCount() > 0 && server) { + @SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names + int matchSubjectAltNamesCount = certificateValidationContext.getMatchSubjectAltNamesCount(); + if (matchSubjectAltNamesCount > 0 && server) { throw new ResourceInvalidException( "match_subject_alt_names only allowed in upstream_tls_context"); } @@ -536,8 +525,6 @@ static void validateCommonTlsContext( private static String getIdentityCertInstanceName(CommonTlsContext commonTlsContext) { if (commonTlsContext.hasTlsCertificateProviderInstance()) { return commonTlsContext.getTlsCertificateProviderInstance().getInstanceName(); - } else if (commonTlsContext.hasTlsCertificateCertificateProviderInstance()) { - return commonTlsContext.getTlsCertificateCertificateProviderInstance().getInstanceName(); } return null; } @@ -556,10 +543,6 @@ private static String getRootCertInstanceName(CommonTlsContext commonTlsContext) .hasCaCertificateProviderInstance()) { return combinedCertificateValidationContext.getDefaultValidationContext() .getCaCertificateProviderInstance().getInstanceName(); - } else if (combinedCertificateValidationContext - .hasValidationContextCertificateProviderInstance()) { - return combinedCertificateValidationContext - .getValidationContextCertificateProviderInstance().getInstanceName(); } } return null; diff --git a/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java b/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java index 80a77cbb1d4..2ee326435c4 100644 --- a/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java @@ -451,8 +451,7 @@ static StructOrError parseRouteAction( config.getHeader(); Pattern regEx = null; String regExSubstitute = null; - if (headerCfg.hasRegexRewrite() && headerCfg.getRegexRewrite().hasPattern() - && headerCfg.getRegexRewrite().getPattern().hasGoogleRe2()) { + if (headerCfg.hasRegexRewrite() && headerCfg.getRegexRewrite().hasPattern()) { regEx = Pattern.compile(headerCfg.getRegexRewrite().getPattern().getRegex()); regExSubstitute = headerCfg.getRegexRewrite().getSubstitution(); } diff --git a/xds/src/main/java/io/grpc/xds/internal/MatcherParser.java b/xds/src/main/java/io/grpc/xds/internal/MatcherParser.java index 39b80bbcc03..fb291efc461 100644 --- a/xds/src/main/java/io/grpc/xds/internal/MatcherParser.java +++ b/xds/src/main/java/io/grpc/xds/internal/MatcherParser.java @@ -26,9 +26,12 @@ public static Matchers.HeaderMatcher parseHeaderMatcher( io.envoyproxy.envoy.config.route.v3.HeaderMatcher proto) { switch (proto.getHeaderMatchSpecifierCase()) { case EXACT_MATCH: + @SuppressWarnings("deprecation") // gRFC A63: support indefinitely + String exactMatch = proto.getExactMatch(); return Matchers.HeaderMatcher.forExactValue( - proto.getName(), proto.getExactMatch(), proto.getInvertMatch()); + proto.getName(), exactMatch, proto.getInvertMatch()); case SAFE_REGEX_MATCH: + @SuppressWarnings("deprecation") // gRFC A63: support indefinitely String rawPattern = proto.getSafeRegexMatch().getRegex(); Pattern safeRegExMatch; try { @@ -49,14 +52,20 @@ public static Matchers.HeaderMatcher parseHeaderMatcher( return Matchers.HeaderMatcher.forPresent( proto.getName(), proto.getPresentMatch(), proto.getInvertMatch()); case PREFIX_MATCH: + @SuppressWarnings("deprecation") // gRFC A63: support indefinitely + String prefixMatch = proto.getPrefixMatch(); return Matchers.HeaderMatcher.forPrefix( - proto.getName(), proto.getPrefixMatch(), proto.getInvertMatch()); + proto.getName(), prefixMatch, proto.getInvertMatch()); case SUFFIX_MATCH: + @SuppressWarnings("deprecation") // gRFC A63: support indefinitely + String suffixMatch = proto.getSuffixMatch(); return Matchers.HeaderMatcher.forSuffix( - proto.getName(), proto.getSuffixMatch(), proto.getInvertMatch()); + proto.getName(), suffixMatch, proto.getInvertMatch()); case CONTAINS_MATCH: + @SuppressWarnings("deprecation") // gRFC A63: support indefinitely + String containsMatch = proto.getContainsMatch(); return Matchers.HeaderMatcher.forContains( - proto.getName(), proto.getContainsMatch(), proto.getInvertMatch()); + proto.getName(), containsMatch, proto.getInvertMatch()); case STRING_MATCH: return Matchers.HeaderMatcher.forString( proto.getName(), parseStringMatcher(proto.getStringMatch()), proto.getInvertMatch()); 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 e5a8c115361..4e4cb1b35be 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 @@ -34,25 +34,16 @@ public static boolean hasCertProviderInstance(CommonTlsContext commonTlsContext) } private static boolean hasCertProviderValidationContext(CommonTlsContext commonTlsContext) { - if (commonTlsContext.hasCombinedValidationContext()) { - CombinedCertificateValidationContext combinedCertificateValidationContext = - commonTlsContext.getCombinedValidationContext(); - return combinedCertificateValidationContext.hasValidationContextCertificateProviderInstance(); - } return hasValidationProviderInstance(commonTlsContext); } private static boolean hasIdentityCertificateProviderInstance(CommonTlsContext commonTlsContext) { - return commonTlsContext.hasTlsCertificateProviderInstance() - || commonTlsContext.hasTlsCertificateCertificateProviderInstance(); + return commonTlsContext.hasTlsCertificateProviderInstance(); } private static boolean hasValidationProviderInstance(CommonTlsContext commonTlsContext) { - if (commonTlsContext.hasValidationContext() && commonTlsContext.getValidationContext() - .hasCaCertificateProviderInstance()) { - return true; - } - return commonTlsContext.hasValidationContextCertificateProviderInstance(); + return commonTlsContext.hasValidationContext() && commonTlsContext.getValidationContext() + .hasCaCertificateProviderInstance(); } /** 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 f9cd14f2efd..801dabeecb7 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 @@ -99,8 +99,6 @@ protected static CertificateProviderInstance getCertProviderInstance( CommonTlsContext commonTlsContext) { if (commonTlsContext.hasTlsCertificateProviderInstance()) { return CommonTlsContextUtil.convert(commonTlsContext.getTlsCertificateProviderInstance()); - } else if (commonTlsContext.hasTlsCertificateCertificateProviderInstance()) { - return commonTlsContext.getTlsCertificateCertificateProviderInstance(); } return null; } @@ -128,15 +126,6 @@ protected static CommonTlsContext.CertificateProviderInstance getRootCertProvide if (certValidationContext != null && certValidationContext.hasCaCertificateProviderInstance()) { return CommonTlsContextUtil.convert(certValidationContext.getCaCertificateProviderInstance()); } - if (commonTlsContext.hasCombinedValidationContext()) { - CommonTlsContext.CombinedCertificateValidationContext combinedValidationContext = - commonTlsContext.getCombinedValidationContext(); - if (combinedValidationContext.hasValidationContextCertificateProviderInstance()) { - return combinedValidationContext.getValidationContextCertificateProviderInstance(); - } - } else if (commonTlsContext.hasValidationContextCertificateProviderInstance()) { - return commonTlsContext.getValidationContextCertificateProviderInstance(); - } return null; } 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 dcd3f2006a1..1ecfe378d29 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 @@ -207,6 +207,7 @@ void verifySubjectAltNameInChain(X509Certificate[] peerCertChain) throws Certifi if (certContext == null) { return; } + @SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names List verifyList = certContext.getMatchSubjectAltNamesList(); if (verifyList.isEmpty()) { return; From 0473f1eb49a383dbf578a7ace0531d05f7b3a65e Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Wed, 9 Apr 2025 09:22:36 -0700 Subject: [PATCH 2/5] Use non-deprecated accessor --- xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java index 0fb7cf15909..080760303bf 100644 --- a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java @@ -673,7 +673,7 @@ public Status onResult2(final ResolutionResult resolutionResult) { resolutionResult.getAddressesOrError(); if (addressesOrError.hasValue()) { backoffPolicy = null; // reset backoff sequence if succeeded - for (EquivalentAddressGroup eag : resolutionResult.getAddresses()) { + for (EquivalentAddressGroup eag : addressesOrError.getValue()) { // No weight attribute is attached, all endpoint-level LB policy should be able // to handle such it. String localityName = localityName(LOGICAL_DNS_CLUSTER_LOCALITY); From 813e35d97462013221c14fe8d1ca819a9ac0a619 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Wed, 9 Apr 2025 09:23:22 -0700 Subject: [PATCH 3/5] Remove unused import --- .../java/io/grpc/xds/internal/security/CommonTlsContextUtil.java | 1 - 1 file changed, 1 deletion(-) 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 4e4cb1b35be..31242e6a531 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 @@ -18,7 +18,6 @@ import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateProviderPluginInstance; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext; -import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext.CombinedCertificateValidationContext; /** Class for utility functions for {@link CommonTlsContext}. */ public final class CommonTlsContextUtil { From d6132c194326c0dff4377970326b7b4e156bba50 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Wed, 9 Apr 2025 16:26:14 -0700 Subject: [PATCH 4/5] Fix tests Note that this fixes a bug in CommonTlsContextUtil where CombinedValidationContext was not checked. I believe this was the only location with such a bug as I audited all non-test usages of has/getValidationContext() and confirmed they have have a cooresponding has/getCombinedValidationContext(). --- .../security/CommonTlsContextUtil.java | 21 ++- ...rChainMatchingProtocolNegotiatorsTest.java | 3 +- .../grpc/xds/GrpcXdsClientImplDataTest.java | 147 ++++++------------ .../grpc/xds/GrpcXdsClientImplTestBase.java | 9 +- .../io/grpc/xds/GrpcXdsClientImplV3Test.java | 12 +- .../ClientSslContextProviderFactoryTest.java | 6 +- .../security/CommonTlsContextTestsUtil.java | 101 +++++------- .../CertificateProviderStoreTest.java | 3 - 8 files changed, 97 insertions(+), 205 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 31242e6a531..50fa18b64f9 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,21 +28,18 @@ public static boolean hasCertProviderInstance(CommonTlsContext commonTlsContext) if (commonTlsContext == null) { return false; } - return hasIdentityCertificateProviderInstance(commonTlsContext) - || hasCertProviderValidationContext(commonTlsContext); - } - - private static boolean hasCertProviderValidationContext(CommonTlsContext commonTlsContext) { - return hasValidationProviderInstance(commonTlsContext); - } - - private static boolean hasIdentityCertificateProviderInstance(CommonTlsContext commonTlsContext) { - return commonTlsContext.hasTlsCertificateProviderInstance(); + return commonTlsContext.hasTlsCertificateProviderInstance() + || hasValidationProviderInstance(commonTlsContext); } private static boolean hasValidationProviderInstance(CommonTlsContext commonTlsContext) { - return commonTlsContext.hasValidationContext() && commonTlsContext.getValidationContext() - .hasCaCertificateProviderInstance(); + if (commonTlsContext.hasValidationContext() && commonTlsContext.getValidationContext() + .hasCaCertificateProviderInstance()) { + return true; + } + return commonTlsContext.hasCombinedValidationContext() + && commonTlsContext.getCombinedValidationContext().getDefaultValidationContext() + .hasCaCertificateProviderInstance(); } /** diff --git a/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java b/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java index c3d006a6003..722f915dbea 100644 --- a/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java +++ b/xds/src/test/java/io/grpc/xds/FilterChainMatchingProtocolNegotiatorsTest.java @@ -1125,7 +1125,6 @@ public void filterChain_5stepMatch() throws Exception { } @Test - @SuppressWarnings("deprecation") public void filterChainMatch_unsupportedMatchers() throws Exception { EnvoyServerProtoData.DownstreamTlsContext tlsContext1 = CommonTlsContextTestsUtil.buildTestInternalDownstreamTlsContext("CERT1", "ROOTCA"); @@ -1194,7 +1193,7 @@ public void filterChainMatch_unsupportedMatchers() throws Exception { assertThat(sslSet.get()).isEqualTo(defaultFilterChain.sslContextProviderSupplier()); assertThat(routingSettable.get()).isEqualTo(noopConfig); assertThat(sslSet.get().getTlsContext().getCommonTlsContext() - .getTlsCertificateCertificateProviderInstance() + .getTlsCertificateProviderInstance() .getCertificateName()).isEqualTo("CERT3"); } diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java index 0ea58c974bb..337e210d66f 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java @@ -98,7 +98,6 @@ import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateProviderPluginInstance; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext; -import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext.CertificateProviderInstance; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext.CombinedCertificateValidationContext; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.DownstreamTlsContext; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.SdsSecretConfig; @@ -3042,35 +3041,6 @@ public void validateCommonTlsContext_validationContextSdsSecretConfig() XdsClusterResource.validateCommonTlsContext(commonTlsContext, null, false); } - @Test - @SuppressWarnings("deprecation") - public void validateCommonTlsContext_validationContextCertificateProvider() - throws ResourceInvalidException { - CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() - .setValidationContextCertificateProvider( - CommonTlsContext.CertificateProvider.getDefaultInstance()) - .build(); - thrown.expect(ResourceInvalidException.class); - thrown.expectMessage( - "common-tls-context with validation_context_certificate_provider is not supported"); - XdsClusterResource.validateCommonTlsContext(commonTlsContext, null, false); - } - - @Test - @SuppressWarnings("deprecation") - public void validateCommonTlsContext_validationContextCertificateProviderInstance() - throws ResourceInvalidException { - CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() - .setValidationContextCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) - .build(); - thrown.expect(ResourceInvalidException.class); - thrown.expectMessage( - "common-tls-context with validation_context_certificate_provider_instance is not " - + "supported"); - XdsClusterResource.validateCommonTlsContext(commonTlsContext, null, false); - } - @Test public void validateCommonTlsContext_tlsCertificateProviderInstance_isRequiredForServer() throws ResourceInvalidException { @@ -3083,36 +3053,33 @@ public void validateCommonTlsContext_tlsCertificateProviderInstance_isRequiredFo } @Test - @SuppressWarnings("deprecation") public void validateCommonTlsContext_tlsNewCertificateProviderInstance() throws ResourceInvalidException { CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setTlsCertificateProviderInstance( - CertificateProviderPluginInstance.newBuilder().setInstanceName("name1").build()) + CertificateProviderPluginInstance.newBuilder().setInstanceName("name1")) .build(); XdsClusterResource .validateCommonTlsContext(commonTlsContext, ImmutableSet.of("name1", "name2"), true); } @Test - @SuppressWarnings("deprecation") public void validateCommonTlsContext_tlsCertificateProviderInstance() throws ResourceInvalidException { CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() - .setTlsCertificateCertificateProviderInstance( - CertificateProviderInstance.newBuilder().setInstanceName("name1").build()) + .setTlsCertificateProviderInstance( + CertificateProviderPluginInstance.newBuilder().setInstanceName("name1")) .build(); XdsClusterResource .validateCommonTlsContext(commonTlsContext, ImmutableSet.of("name1", "name2"), true); } @Test - @SuppressWarnings("deprecation") public void validateCommonTlsContext_tlsCertificateProviderInstance_absentInBootstrapFile() throws ResourceInvalidException { CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() - .setTlsCertificateCertificateProviderInstance( - CertificateProviderInstance.newBuilder().setInstanceName("bad-name").build()) + .setTlsCertificateProviderInstance( + CertificateProviderPluginInstance.newBuilder().setInstanceName("bad-name")) .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage( @@ -3122,15 +3089,17 @@ public void validateCommonTlsContext_tlsCertificateProviderInstance_absentInBoot } @Test - @SuppressWarnings("deprecation") public void validateCommonTlsContext_validationContextProviderInstance() throws ResourceInvalidException { CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setCombinedValidationContext( CommonTlsContext.CombinedCertificateValidationContext.newBuilder() - .setValidationContextCertificateProviderInstance( - CertificateProviderInstance.newBuilder().setInstanceName("name1").build()) + .setDefaultValidationContext(CertificateValidationContext.newBuilder() + .setCaCertificateProviderInstance(CertificateProviderPluginInstance.newBuilder() + .setInstanceName("name1") + .build()) .build()) + .build()) .build(); XdsClusterResource .validateCommonTlsContext(commonTlsContext, ImmutableSet.of("name1", "name2"), false); @@ -3218,15 +3187,14 @@ public void validateCommonTlsContext_validationContextSystemRootCerts() } @Test - @SuppressWarnings("deprecation") public void validateCommonTlsContext_validationContextProviderInstance_absentInBootstrapFile() throws ResourceInvalidException { CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setCombinedValidationContext( CommonTlsContext.CombinedCertificateValidationContext.newBuilder() - .setValidationContextCertificateProviderInstance( - CertificateProviderInstance.newBuilder().setInstanceName("bad-name").build()) - .build()) + .setDefaultValidationContext(CertificateValidationContext.newBuilder() + .setCaCertificateProviderInstance(CertificateProviderPluginInstance.newBuilder() + .setInstanceName("bad-name")))) .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage( @@ -3258,20 +3226,6 @@ public void validateCommonTlsContext_tlsCertificateSdsSecretConfigsCount() XdsClusterResource.validateCommonTlsContext(commonTlsContext, null, false); } - @Test - @SuppressWarnings("deprecation") - public void validateCommonTlsContext_tlsCertificateCertificateProvider() - throws ResourceInvalidException { - CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() - .setTlsCertificateCertificateProvider( - CommonTlsContext.CertificateProvider.getDefaultInstance()) - .build(); - thrown.expect(ResourceInvalidException.class); - thrown.expectMessage( - "tls_certificate_provider_instance is unset"); - XdsClusterResource.validateCommonTlsContext(commonTlsContext, null, false); - } - @Test public void validateCommonTlsContext_combinedValidationContext_isRequiredForClient() throws ResourceInvalidException { @@ -3304,13 +3258,13 @@ public void validateCommonTlsContext_combinedValContextWithDefaultValContextForS CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setCombinedValidationContext( CombinedCertificateValidationContext.newBuilder() - .setValidationContextCertificateProviderInstance( - CertificateProviderInstance.getDefaultInstance()) .setDefaultValidationContext(CertificateValidationContext.newBuilder() + .setCaCertificateProviderInstance( + CertificateProviderPluginInstance.getDefaultInstance()) .addMatchSubjectAltNames(StringMatcher.newBuilder().setExact("foo.com").build()) .build())) - .setTlsCertificateCertificateProviderInstance( - CertificateProviderInstance.getDefaultInstance()) + .setTlsCertificateProviderInstance( + CertificateProviderPluginInstance.getDefaultInstance()) .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage("match_subject_alt_names only allowed in upstream_tls_context"); @@ -3318,18 +3272,16 @@ public void validateCommonTlsContext_combinedValContextWithDefaultValContextForS } @Test - @SuppressWarnings("deprecation") public void validateCommonTlsContext_combinedValContextWithDefaultValContextVerifyCertSpki() throws ResourceInvalidException { CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setCombinedValidationContext( CommonTlsContext.CombinedCertificateValidationContext.newBuilder() - .setValidationContextCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) - .setDefaultValidationContext( - CertificateValidationContext.newBuilder().addVerifyCertificateSpki("foo"))) - .setTlsCertificateCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) + .setDefaultValidationContext(CertificateValidationContext.newBuilder() + .setCaCertificateProviderInstance( + CertificateProviderPluginInstance.getDefaultInstance()) + .addVerifyCertificateSpki("foo"))) + .setTlsCertificateProviderInstance(CertificateProviderPluginInstance.getDefaultInstance()) .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage("verify_certificate_spki in default_validation_context is not " @@ -3338,18 +3290,16 @@ public void validateCommonTlsContext_combinedValContextWithDefaultValContextVeri } @Test - @SuppressWarnings("deprecation") public void validateCommonTlsContext_combinedValContextWithDefaultValContextVerifyCertHash() throws ResourceInvalidException { CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setCombinedValidationContext( CommonTlsContext.CombinedCertificateValidationContext.newBuilder() - .setValidationContextCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) - .setDefaultValidationContext( - CertificateValidationContext.newBuilder().addVerifyCertificateHash("foo"))) - .setTlsCertificateCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) + .setDefaultValidationContext(CertificateValidationContext.newBuilder() + .setCaCertificateProviderInstance( + CertificateProviderPluginInstance.getDefaultInstance()) + .addVerifyCertificateHash("foo"))) + .setTlsCertificateProviderInstance(CertificateProviderPluginInstance.getDefaultInstance()) .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage("verify_certificate_hash in default_validation_context is not " @@ -3358,18 +3308,17 @@ public void validateCommonTlsContext_combinedValContextWithDefaultValContextVeri } @Test - @SuppressWarnings("deprecation") public void validateCommonTlsContext_combinedValContextDfltValContextRequireSignedCertTimestamp() throws ResourceInvalidException { CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setCombinedValidationContext( CommonTlsContext.CombinedCertificateValidationContext.newBuilder() - .setValidationContextCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) .setDefaultValidationContext(CertificateValidationContext.newBuilder() + .setCaCertificateProviderInstance( + CertificateProviderPluginInstance.getDefaultInstance()) .setRequireSignedCertificateTimestamp(BoolValue.of(true)))) - .setTlsCertificateCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) + .setTlsCertificateProviderInstance( + CertificateProviderPluginInstance.getDefaultInstance()) .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage( @@ -3379,18 +3328,16 @@ public void validateCommonTlsContext_combinedValContextDfltValContextRequireSign } @Test - @SuppressWarnings("deprecation") public void validateCommonTlsContext_combinedValidationContextWithDefaultValidationContextCrl() throws ResourceInvalidException { CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setCombinedValidationContext( CommonTlsContext.CombinedCertificateValidationContext.newBuilder() - .setValidationContextCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) .setDefaultValidationContext(CertificateValidationContext.newBuilder() + .setCaCertificateProviderInstance( + CertificateProviderPluginInstance.getDefaultInstance()) .setCrl(DataSource.getDefaultInstance()))) - .setTlsCertificateCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) + .setTlsCertificateProviderInstance(CertificateProviderPluginInstance.getDefaultInstance()) .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage("crl in default_validation_context is not supported"); @@ -3398,18 +3345,16 @@ public void validateCommonTlsContext_combinedValidationContextWithDefaultValidat } @Test - @SuppressWarnings("deprecation") public void validateCommonTlsContext_combinedValContextWithDfltValContextCustomValidatorConfig() throws ResourceInvalidException { CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setCombinedValidationContext( CommonTlsContext.CombinedCertificateValidationContext.newBuilder() - .setValidationContextCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) .setDefaultValidationContext(CertificateValidationContext.newBuilder() + .setCaCertificateProviderInstance( + CertificateProviderPluginInstance.getDefaultInstance()) .setCustomValidatorConfig(TypedExtensionConfig.getDefaultInstance()))) - .setTlsCertificateCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) + .setTlsCertificateProviderInstance(CertificateProviderPluginInstance.getDefaultInstance()) .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage("custom_validator_config in default_validation_context is not " @@ -3426,15 +3371,14 @@ public void validateDownstreamTlsContext_noCommonTlsContext() throws ResourceInv } @Test - @SuppressWarnings("deprecation") public void validateDownstreamTlsContext_hasRequireSni() throws ResourceInvalidException { CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setCombinedValidationContext( CommonTlsContext.CombinedCertificateValidationContext.newBuilder() - .setValidationContextCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance())) - .setTlsCertificateCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) + .setDefaultValidationContext(CertificateValidationContext.newBuilder() + .setCaCertificateProviderInstance( + CertificateProviderPluginInstance.getDefaultInstance()))) + .setTlsCertificateProviderInstance(CertificateProviderPluginInstance.getDefaultInstance()) .build(); DownstreamTlsContext downstreamTlsContext = DownstreamTlsContext.newBuilder() .setCommonTlsContext(commonTlsContext) @@ -3446,15 +3390,14 @@ public void validateDownstreamTlsContext_hasRequireSni() throws ResourceInvalidE } @Test - @SuppressWarnings("deprecation") public void validateDownstreamTlsContext_hasOcspStaplePolicy() throws ResourceInvalidException { CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .setCombinedValidationContext( CommonTlsContext.CombinedCertificateValidationContext.newBuilder() - .setValidationContextCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance())) - .setTlsCertificateCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.getDefaultInstance()) + .setDefaultValidationContext(CertificateValidationContext.newBuilder() + .setCaCertificateProviderInstance( + CertificateProviderPluginInstance.getDefaultInstance()))) + .setTlsCertificateProviderInstance(CertificateProviderPluginInstance.getDefaultInstance()) .build(); DownstreamTlsContext downstreamTlsContext = DownstreamTlsContext.newBuilder() .setCommonTlsContext(commonTlsContext) diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java index 36131464d08..369763a21b7 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java @@ -47,7 +47,6 @@ import io.envoyproxy.envoy.config.route.v3.WeightedCluster; import io.envoyproxy.envoy.extensions.filters.http.router.v3.Router; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateProviderPluginInstance; -import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext; import io.grpc.BindableService; import io.grpc.ChannelCredentials; import io.grpc.Context; @@ -2245,7 +2244,6 @@ public void cdsResponseWithCircuitBreakers() { * CDS response containing UpstreamTlsContext for a cluster. */ @Test - @SuppressWarnings("deprecation") public void cdsResponseWithUpstreamTlsContext() { DiscoveryRpcCall call = startResourceWatcher(XdsClusterResource.getInstance(), CDS_RESOURCE, cdsResourceWatcher); @@ -2269,9 +2267,9 @@ public void cdsResponseWithUpstreamTlsContext() { verify(cdsResourceWatcher, times(1)) .onChanged(cdsUpdateCaptor.capture()); CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); - CommonTlsContext.CertificateProviderInstance certificateProviderInstance = + CertificateProviderPluginInstance certificateProviderInstance = cdsUpdate.upstreamTlsContext().getCommonTlsContext().getCombinedValidationContext() - .getValidationContextCertificateProviderInstance(); + .getDefaultValidationContext().getCaCertificateProviderInstance(); assertThat(certificateProviderInstance.getInstanceName()).isEqualTo("cert-instance-name"); assertThat(certificateProviderInstance.getCertificateName()).isEqualTo("cert1"); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, clusterEds, VERSION_1, TIME_INCREMENT); @@ -2282,7 +2280,6 @@ public void cdsResponseWithUpstreamTlsContext() { * CDS response containing new UpstreamTlsContext for a cluster. */ @Test - @SuppressWarnings("deprecation") public void cdsResponseWithNewUpstreamTlsContext() { DiscoveryRpcCall call = startResourceWatcher(XdsClusterResource.getInstance(), CDS_RESOURCE, cdsResourceWatcher); @@ -2344,7 +2341,6 @@ public void cdsResponseErrorHandling_badUpstreamTlsContext() { * CDS response containing OutlierDetection for a cluster. */ @Test - @SuppressWarnings("deprecation") public void cdsResponseWithOutlierDetection() { DiscoveryRpcCall call = startResourceWatcher(XdsClusterResource.getInstance(), CDS_RESOURCE, cdsResourceWatcher); @@ -2413,7 +2409,6 @@ public void cdsResponseWithOutlierDetection() { * CDS response containing OutlierDetection for a cluster. */ @Test - @SuppressWarnings("deprecation") public void cdsResponseWithInvalidOutlierDetectionNacks() { DiscoveryRpcCall call = startResourceWatcher(XdsClusterResource.getInstance(), CDS_RESOURCE, diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplV3Test.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplV3Test.java index af34c7232d0..3966fae7f20 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplV3Test.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplV3Test.java @@ -613,18 +613,15 @@ protected Message buildLeastRequestLbConfig(int choiceCount) { } @Override - @SuppressWarnings("deprecation") protected Message buildUpstreamTlsContext(String instanceName, String certName) { CommonTlsContext.Builder commonTlsContextBuilder = CommonTlsContext.newBuilder(); if (instanceName != null && certName != null) { - CommonTlsContext.CertificateProviderInstance providerInstance = - CommonTlsContext.CertificateProviderInstance.newBuilder() - .setInstanceName(instanceName) - .setCertificateName(certName) - .build(); CommonTlsContext.CombinedCertificateValidationContext combined = CommonTlsContext.CombinedCertificateValidationContext.newBuilder() - .setValidationContextCertificateProviderInstance(providerInstance) + .setDefaultValidationContext(CertificateValidationContext.newBuilder() + .setCaCertificateProviderInstance(CertificateProviderPluginInstance.newBuilder() + .setInstanceName(instanceName) + .setCertificateName(certName))) .build(); commonTlsContextBuilder.setCombinedValidationContext(combined); } @@ -751,7 +748,6 @@ protected Message buildDropOverload(String category, int dropPerMillion) { .build(); } - @SuppressWarnings("deprecation") @Override protected FilterChain buildFilterChain( List alpn, Message tlsContext, String transportSocketName, diff --git a/xds/src/test/java/io/grpc/xds/internal/security/ClientSslContextProviderFactoryTest.java b/xds/src/test/java/io/grpc/xds/internal/security/ClientSslContextProviderFactoryTest.java index 9c8340123da..397fe01e0f5 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/ClientSslContextProviderFactoryTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/ClientSslContextProviderFactoryTest.java @@ -317,7 +317,6 @@ static void verifyWatcher( .isSameInstanceAs(sslContextProvider); } - @SuppressWarnings("deprecation") static CommonTlsContext.Builder addFilenames( CommonTlsContext.Builder builder, String certChain, String privateKey, String trustCa) { TlsCertificate tlsCert = @@ -329,13 +328,10 @@ static CommonTlsContext.Builder addFilenames( CertificateValidationContext.newBuilder() .setTrustedCa(DataSource.newBuilder().setFilename(trustCa)) .build(); - CommonTlsContext.CertificateProviderInstance certificateProviderInstance = - builder.getValidationContextCertificateProviderInstance(); CommonTlsContext.CombinedCertificateValidationContext.Builder combinedBuilder = CommonTlsContext.CombinedCertificateValidationContext.newBuilder(); combinedBuilder - .setDefaultValidationContext(certContext) - .setValidationContextCertificateProviderInstance(certificateProviderInstance); + .setDefaultValidationContext(certContext); return builder .addTlsCertificates(tlsCert) .setCombinedValidationContext(combinedBuilder.build()); 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 a6cf2c52a15..48814dece1d 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 @@ -23,7 +23,6 @@ import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateProviderPluginInstance; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext; -import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext.CertificateProviderInstance; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext.CombinedCertificateValidationContext; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.DownstreamTlsContext; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext; @@ -63,48 +62,26 @@ public class CommonTlsContextTestsUtil { public static final String BAD_CLIENT_KEY_FILE = "badclient.key"; /** takes additional values and creates CombinedCertificateValidationContext as needed. */ - @SuppressWarnings("deprecation") - static CommonTlsContext buildCommonTlsContextWithAdditionalValues( + private static CommonTlsContext buildCommonTlsContextWithAdditionalValues( String certInstanceName, String certName, String validationContextCertInstanceName, String validationContextCertName, Iterable matchSubjectAltNames, Iterable alpnNames) { - - CommonTlsContext.Builder builder = CommonTlsContext.newBuilder(); - - CertificateProviderInstance certificateProviderInstance = CertificateProviderInstance - .newBuilder().setInstanceName(certInstanceName).setCertificateName(certName).build(); - if (certificateProviderInstance != null) { - builder.setTlsCertificateCertificateProviderInstance(certificateProviderInstance); - } - CertificateProviderInstance validationCertificateProviderInstance = - CertificateProviderInstance.newBuilder().setInstanceName(validationContextCertInstanceName) - .setCertificateName(validationContextCertName).build(); - CertificateValidationContext certValidationContext = - matchSubjectAltNames == null - ? null - : CertificateValidationContext.newBuilder() - .addAllMatchSubjectAltNames(matchSubjectAltNames) - .build(); - if (validationCertificateProviderInstance != null) { - CombinedCertificateValidationContext.Builder combinedBuilder = - CombinedCertificateValidationContext.newBuilder() - .setValidationContextCertificateProviderInstance( - validationCertificateProviderInstance); - if (certValidationContext != null) { - combinedBuilder = combinedBuilder.setDefaultValidationContext(certValidationContext); - } - builder.setCombinedValidationContext(combinedBuilder); - } else if (validationCertificateProviderInstance != null) { - builder - .setValidationContextCertificateProviderInstance(validationCertificateProviderInstance); - } else if (certValidationContext != null) { - builder.setValidationContext(certValidationContext); - } - if (alpnNames != null) { - builder.addAllAlpnProtocols(alpnNames); - } - return builder.build(); + @SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names + CertificateValidationContext.Builder certificateValidationContextBuilder + = CertificateValidationContext.newBuilder() + .addAllMatchSubjectAltNames(matchSubjectAltNames); + return CommonTlsContext.newBuilder() + .setTlsCertificateProviderInstance(CertificateProviderPluginInstance.newBuilder() + .setInstanceName(certInstanceName) + .setCertificateName(certName)) + .setCombinedValidationContext(CombinedCertificateValidationContext.newBuilder() + .setDefaultValidationContext(certificateValidationContextBuilder + .setCaCertificateProviderInstance(CertificateProviderPluginInstance.newBuilder() + .setInstanceName(validationContextCertInstanceName) + .setCertificateName(validationContextCertName)))) + .addAllAlpnProtocols(alpnNames) + .build(); } /** Helper method to build DownstreamTlsContext for multiple test classes. */ @@ -152,7 +129,7 @@ public static DownstreamTlsContext buildTestDownstreamTlsContext( useSans ? Arrays.asList( StringMatcher.newBuilder() .setExact("spiffe://grpc-sds-testing.svc.id.goog/ns/default/sa/bob") - .build()) : null, + .build()) : Arrays.asList(), Arrays.asList("managed-tls")); } return buildDownstreamTlsContext(commonTlsContext, /* requireClientCert= */ false); @@ -199,7 +176,6 @@ public static X509Certificate getCertFromResourceName(String resourceName) } } - @SuppressWarnings("deprecation") private static CommonTlsContext buildCommonTlsContextForCertProviderInstance( String certInstanceName, String certName, @@ -210,10 +186,10 @@ private static CommonTlsContext buildCommonTlsContextForCertProviderInstance( CommonTlsContext.Builder builder = CommonTlsContext.newBuilder(); if (certInstanceName != null) { builder = - builder.setTlsCertificateCertificateProviderInstance( - CommonTlsContext.CertificateProviderInstance.newBuilder() - .setInstanceName(certInstanceName) - .setCertificateName(certName)); + builder.setTlsCertificateProviderInstance( + CertificateProviderPluginInstance.newBuilder() + .setInstanceName(certInstanceName) + .setCertificateName(certName)); } builder = addCertificateValidationContext( @@ -248,35 +224,28 @@ private static CommonTlsContext buildNewCommonTlsContextForCertProviderInstance( return builder.build(); } - @SuppressWarnings("deprecation") private static CommonTlsContext.Builder addCertificateValidationContext( CommonTlsContext.Builder builder, String rootInstanceName, String rootCertName, CertificateValidationContext staticCertValidationContext) { - CertificateProviderInstance providerInstance = null; - if (rootInstanceName != null) { - providerInstance = CertificateProviderInstance.newBuilder() - .setInstanceName(rootInstanceName) - .setCertificateName(rootCertName) - .build(); - } - if (providerInstance != null) { - builder = builder.setValidationContextCertificateProviderInstance(providerInstance); + if (staticCertValidationContext == null && rootInstanceName == null) { + return builder; } - CombinedCertificateValidationContext.Builder combined = - CombinedCertificateValidationContext.newBuilder(); - if (providerInstance != null) { - combined = combined.setValidationContextCertificateProviderInstance(providerInstance); - } - if (staticCertValidationContext != null) { - combined = combined.setDefaultValidationContext(staticCertValidationContext); + CertificateValidationContext.Builder contextBuilder; + if (staticCertValidationContext == null) { + contextBuilder = CertificateValidationContext.newBuilder(); + } else { + contextBuilder = staticCertValidationContext.toBuilder(); } - if (combined.hasValidationContextCertificateProviderInstance() - || combined.hasDefaultValidationContext()) { - builder = builder.setCombinedValidationContext(combined.build()); + if (rootInstanceName != null) { + contextBuilder.setCaCertificateProviderInstance(CertificateProviderPluginInstance.newBuilder() + .setInstanceName(rootInstanceName) + .setCertificateName(rootCertName)); + builder.setValidationContext(contextBuilder.build()); } - return builder; + return builder.setCombinedValidationContext(CombinedCertificateValidationContext.newBuilder() + .setDefaultValidationContext(contextBuilder)); } private static CommonTlsContext.Builder addNewCertificateValidationContext( diff --git a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertificateProviderStoreTest.java b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertificateProviderStoreTest.java index 8f77de7b5e2..c0bc095eab6 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertificateProviderStoreTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertificateProviderStoreTest.java @@ -123,7 +123,6 @@ public void notifyCertUpdatesNotSupported_expectExceptionOnSecondCall() { } @Test - @SuppressWarnings("deprecation") public void onePluginSameConfig_sameInstance() { registerPlugin("plugin1"); CertificateProvider.Watcher mockWatcher1 = mock(CertificateProvider.Watcher.class); @@ -167,7 +166,6 @@ public void onePluginSameConfig_sameInstance() { } @Test - @SuppressWarnings("deprecation") public void onePluginSameConfig_secondWatcherAfterFirstNotify() { registerPlugin("plugin1"); CertificateProvider.Watcher mockWatcher1 = mock(CertificateProvider.Watcher.class); @@ -275,7 +273,6 @@ public void twoPlugins_differentInstance() { mockWatcher1, handle1, certProviderProvider1, mockWatcher2, handle2, certProviderProvider2); } - @SuppressWarnings("deprecation") private static void checkDifferentInstances( CertificateProvider.Watcher mockWatcher1, CertificateProviderStore.Handle handle1, From 3b8d0cf5cb57682aaf0ccc774a6a7cd34c542403 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 10 Apr 2025 15:34:00 -0700 Subject: [PATCH 5/5] Remove unnecessary build()s --- xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java index 337e210d66f..e5502463db0 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java @@ -3096,10 +3096,7 @@ public void validateCommonTlsContext_validationContextProviderInstance() CommonTlsContext.CombinedCertificateValidationContext.newBuilder() .setDefaultValidationContext(CertificateValidationContext.newBuilder() .setCaCertificateProviderInstance(CertificateProviderPluginInstance.newBuilder() - .setInstanceName("name1") - .build()) - .build()) - .build()) + .setInstanceName("name1")))) .build(); XdsClusterResource .validateCommonTlsContext(commonTlsContext, ImmutableSet.of("name1", "name2"), false);