Skip to content

xds: Enable deprecation warnings #11998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions xds/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 7 additions & 2 deletions xds/src/main/java/io/grpc/xds/RbacFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
23 changes: 3 additions & 20 deletions xds/src/main/java/io/grpc/xds/XdsClusterResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -451,8 +451,7 @@ static StructOrError<RouteAction> 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();
}
Expand Down
17 changes: 13 additions & 4 deletions xds/src/main/java/io/grpc/xds/internal/MatcherParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,12 @@
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 {
Expand All @@ -49,14 +52,20 @@
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();

Check warning on line 66 in xds/src/main/java/io/grpc/xds/internal/MatcherParser.java

View check run for this annotation

Codecov / codecov/patch

xds/src/main/java/io/grpc/xds/internal/MatcherParser.java#L66

Added line #L66 was not covered by tests
return Matchers.HeaderMatcher.forContains(
proto.getName(), proto.getContainsMatch(), proto.getInvertMatch());
proto.getName(), containsMatch, proto.getInvertMatch());

Check warning on line 68 in xds/src/main/java/io/grpc/xds/internal/MatcherParser.java

View check run for this annotation

Codecov / codecov/patch

xds/src/main/java/io/grpc/xds/internal/MatcherParser.java#L68

Added line #L68 was not covered by tests
case STRING_MATCH:
return Matchers.HeaderMatcher.forString(
proto.getName(), parseStringMatcher(proto.getStringMatch()), proto.getInvertMatch());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -29,30 +28,18 @@ public static boolean hasCertProviderInstance(CommonTlsContext commonTlsContext)
if (commonTlsContext == null) {
return false;
}
return hasIdentityCertificateProviderInstance(commonTlsContext)
|| hasCertProviderValidationContext(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();
|| hasValidationProviderInstance(commonTlsContext);
}

private static boolean hasValidationProviderInstance(CommonTlsContext commonTlsContext) {
if (commonTlsContext.hasValidationContext() && commonTlsContext.getValidationContext()
.hasCaCertificateProviderInstance()) {
return true;
}
return commonTlsContext.hasValidationContextCertificateProviderInstance();
return commonTlsContext.hasCombinedValidationContext()
&& commonTlsContext.getCombinedValidationContext().getDefaultValidationContext()
.hasCaCertificateProviderInstance();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<StringMatcher> verifyList = certContext.getMatchSubjectAltNamesList();
if (verifyList.isEmpty()) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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");
}

Expand Down
Loading