Skip to content

Commit

Permalink
xds: implement filter-chain uniqueness check as per grfc A36 (grpc#8295)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sanjaypujare @YifeiZhuang
sanjaypujare authored and YifeiZhuang committed Jul 25, 2021
1 parent e710ddb commit 392d8b3
Show file tree
Hide file tree
Showing 2 changed files with 293 additions and 10 deletions.
141 changes: 137 additions & 4 deletions xds/src/main/java/io/grpc/xds/ClientXdsClient.java
View file
Expand Up @@ -85,6 +85,7 @@
import java.net.InetSocketAddress;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -286,15 +287,17 @@ static EnvoyServerProtoData.Listener parseServerSideListener(
}

List<FilterChain> filterChains = new ArrayList<>();
Set<FilterChainMatch> uniqueSet = new HashSet<>();
for (io.envoyproxy.envoy.config.listener.v3.FilterChain fc : proto.getFilterChainsList()) {
filterChains.add(
parseFilterChain(fc, rdsResources, tlsContextManager, filterRegistry, parseHttpFilter));
parseFilterChain(fc, rdsResources, tlsContextManager, filterRegistry, uniqueSet,
parseHttpFilter));
}
FilterChain defaultFilterChain = null;
if (proto.hasDefaultFilterChain()) {
defaultFilterChain = parseFilterChain(
proto.getDefaultFilterChain(), rdsResources, tlsContextManager, filterRegistry,
parseHttpFilter);
null, parseHttpFilter);
}

return new EnvoyServerProtoData.Listener(
Expand All @@ -304,7 +307,8 @@ static EnvoyServerProtoData.Listener parseServerSideListener(
@VisibleForTesting
static FilterChain parseFilterChain(
io.envoyproxy.envoy.config.listener.v3.FilterChain proto, Set<String> rdsResources,
TlsContextManager tlsContextManager, FilterRegistry filterRegistry, boolean parseHttpFilters)
TlsContextManager tlsContextManager, FilterRegistry filterRegistry,
Set<FilterChainMatch> uniqueSet, boolean parseHttpFilters)
throws ResourceInvalidException {
io.grpc.xds.HttpConnectionManager httpConnectionManager = null;
HashSet<String> uniqueNames = new HashSet<>();
Expand Down Expand Up @@ -361,15 +365,144 @@ static FilterChain parseFilterChain(
if (name.isEmpty()) {
name = UUID.randomUUID().toString();
}
FilterChainMatch filterChainMatch = parseFilterChainMatch(proto.getFilterChainMatch());
checkForUniqueness(uniqueSet, filterChainMatch);
return new FilterChain(
name,
parseFilterChainMatch(proto.getFilterChainMatch()),
filterChainMatch,
httpConnectionManager,
downstreamTlsContext,
tlsContextManager
);
}

private static void checkForUniqueness(Set<FilterChainMatch> uniqueSet,
FilterChainMatch filterChainMatch) throws ResourceInvalidException {
if (uniqueSet != null) {
List<FilterChainMatch> crossProduct = getCrossProduct(filterChainMatch);
for (FilterChainMatch cur : crossProduct) {
if (!uniqueSet.add(cur)) {
throw new ResourceInvalidException("Found duplicate matcher: " + cur);
}
}
}
}

private static List<FilterChainMatch> getCrossProduct(FilterChainMatch filterChainMatch) {
// repeating fields to process:
// prefixRanges, applicationProtocols, sourcePrefixRanges, sourcePorts, serverNames
List<FilterChainMatch> expandedList = expandOnPrefixRange(filterChainMatch);
expandedList = expandOnApplicationProtocols(expandedList);
expandedList = expandOnSourcePrefixRange(expandedList);
expandedList = expandOnSourcePorts(expandedList);
return expandOnServerNames(expandedList);
}

private static List<FilterChainMatch> expandOnPrefixRange(FilterChainMatch filterChainMatch) {
ArrayList<FilterChainMatch> expandedList = new ArrayList<>();
if (filterChainMatch.getPrefixRanges().isEmpty()) {
expandedList.add(filterChainMatch);
} else {
for (EnvoyServerProtoData.CidrRange cidrRange : filterChainMatch.getPrefixRanges()) {
expandedList.add(new FilterChainMatch(filterChainMatch.getDestinationPort(),
Arrays.asList(cidrRange),
Collections.unmodifiableList(filterChainMatch.getApplicationProtocols()),
Collections.unmodifiableList(filterChainMatch.getSourcePrefixRanges()),
filterChainMatch.getConnectionSourceType(),
Collections.unmodifiableList(filterChainMatch.getSourcePorts()),
Collections.unmodifiableList(filterChainMatch.getServerNames()),
filterChainMatch.getTransportProtocol()));
}
}
return expandedList;
}

private static List<FilterChainMatch> expandOnApplicationProtocols(
Collection<FilterChainMatch> set) {
ArrayList<FilterChainMatch> expandedList = new ArrayList<>();
for (FilterChainMatch filterChainMatch : set) {
if (filterChainMatch.getApplicationProtocols().isEmpty()) {
expandedList.add(filterChainMatch);
} else {
for (String applicationProtocol : filterChainMatch.getApplicationProtocols()) {
expandedList.add(new FilterChainMatch(filterChainMatch.getDestinationPort(),
Collections.unmodifiableList(filterChainMatch.getPrefixRanges()),
Arrays.asList(applicationProtocol),
Collections.unmodifiableList(filterChainMatch.getSourcePrefixRanges()),
filterChainMatch.getConnectionSourceType(),
Collections.unmodifiableList(filterChainMatch.getSourcePorts()),
Collections.unmodifiableList(filterChainMatch.getServerNames()),
filterChainMatch.getTransportProtocol()));
}
}
}
return expandedList;
}

private static List<FilterChainMatch> expandOnSourcePrefixRange(
Collection<FilterChainMatch> set) {
ArrayList<FilterChainMatch> expandedList = new ArrayList<>();
for (FilterChainMatch filterChainMatch : set) {
if (filterChainMatch.getSourcePrefixRanges().isEmpty()) {
expandedList.add(filterChainMatch);
} else {
for (EnvoyServerProtoData.CidrRange cidrRange : filterChainMatch.getSourcePrefixRanges()) {
expandedList.add(new FilterChainMatch(filterChainMatch.getDestinationPort(),
Collections.unmodifiableList(filterChainMatch.getPrefixRanges()),
Collections.unmodifiableList(filterChainMatch.getApplicationProtocols()),
Arrays.asList(cidrRange),
filterChainMatch.getConnectionSourceType(),
Collections.unmodifiableList(filterChainMatch.getSourcePorts()),
Collections.unmodifiableList(filterChainMatch.getServerNames()),
filterChainMatch.getTransportProtocol()));
}
}
}
return expandedList;
}

private static List<FilterChainMatch> expandOnSourcePorts(Collection<FilterChainMatch> set) {
ArrayList<FilterChainMatch> expandedList = new ArrayList<>();
for (FilterChainMatch filterChainMatch : set) {
if (filterChainMatch.getSourcePorts().isEmpty()) {
expandedList.add(filterChainMatch);
} else {
for (Integer sourcePort : filterChainMatch.getSourcePorts()) {
expandedList.add(new FilterChainMatch(filterChainMatch.getDestinationPort(),
Collections.unmodifiableList(filterChainMatch.getPrefixRanges()),
Collections.unmodifiableList(filterChainMatch.getApplicationProtocols()),
Collections.unmodifiableList(filterChainMatch.getSourcePrefixRanges()),
filterChainMatch.getConnectionSourceType(),
Arrays.asList(sourcePort),
Collections.unmodifiableList(filterChainMatch.getServerNames()),
filterChainMatch.getTransportProtocol()));
}
}
}
return expandedList;
}

private static List<FilterChainMatch> expandOnServerNames(Collection<FilterChainMatch> set) {
ArrayList<FilterChainMatch> expandedList = new ArrayList<>();
for (FilterChainMatch filterChainMatch : set) {
if (filterChainMatch.getServerNames().isEmpty()) {
expandedList.add(filterChainMatch);
} else {
for (String serverName : filterChainMatch.getServerNames()) {
expandedList.add(new FilterChainMatch(filterChainMatch.getDestinationPort(),
Collections.unmodifiableList(filterChainMatch.getPrefixRanges()),
Collections.unmodifiableList(filterChainMatch.getApplicationProtocols()),
Collections.unmodifiableList(filterChainMatch.getSourcePrefixRanges()),
filterChainMatch.getConnectionSourceType(),
Collections.unmodifiableList(filterChainMatch.getSourcePorts()),
Arrays.asList(serverName),
filterChainMatch.getTransportProtocol()));
}
}
}
return expandedList;
}

private static FilterChainMatch parseFilterChainMatch(
io.envoyproxy.envoy.config.listener.v3.FilterChainMatch proto)
throws ResourceInvalidException {
Expand Down
162 changes: 156 additions & 6 deletions xds/src/test/java/io/grpc/xds/ClientXdsClientDataTest.java
View file
Expand Up @@ -34,6 +34,7 @@
import io.envoyproxy.envoy.config.cluster.v3.Cluster.RingHashLbConfig.HashFunction;
import io.envoyproxy.envoy.config.core.v3.Address;
import io.envoyproxy.envoy.config.core.v3.AggregatedConfigSource;
import io.envoyproxy.envoy.config.core.v3.CidrRange;
import io.envoyproxy.envoy.config.core.v3.ConfigSource;
import io.envoyproxy.envoy.config.core.v3.HttpProtocolOptions;
import io.envoyproxy.envoy.config.core.v3.Locality;
Expand Down Expand Up @@ -1038,6 +1039,153 @@ public void parseServerSideListener_useOriginalDst() throws ResourceInvalidExcep
listener, new HashSet<String>(), null, filterRegistry, true /* does not matter */);
}

@Test
public void parseServerSideListener_nonUniqueFilterChainMatch() throws ResourceInvalidException {
Filter filter1 = buildHttpConnectionManagerFilter(
HttpFilter.newBuilder().setName("http-filter-1").setIsOptional(true).build());
FilterChainMatch filterChainMatch1 =
FilterChainMatch.newBuilder()
.addAllSourcePorts(Arrays.asList(80, 8080))
.addAllPrefixRanges(Arrays.asList(CidrRange.newBuilder().setAddressPrefix("192.168.0.0")
.setPrefixLen(UInt32Value.of(16)).build(),
CidrRange.newBuilder().setAddressPrefix("10.0.0.0").setPrefixLen(UInt32Value.of(8))
.build()))
.build();
FilterChain filterChain1 =
FilterChain.newBuilder()
.setName("filter-chain-1")
.setFilterChainMatch(filterChainMatch1)
.setTransportSocket(TransportSocket.getDefaultInstance())
.addFilters(filter1)
.build();
Filter filter2 = buildHttpConnectionManagerFilter(
HttpFilter.newBuilder().setName("http-filter-2").setIsOptional(true).build());
FilterChainMatch filterChainMatch2 =
FilterChainMatch.newBuilder()
.addAllSourcePorts(Arrays.asList(443, 8080))
.addAllPrefixRanges(Arrays.asList(
CidrRange.newBuilder().setAddressPrefix("2001:DB8::8:800:200C:417A")
.setPrefixLen(UInt32Value.of(60)).build(),
CidrRange.newBuilder().setAddressPrefix("192.168.0.0")
.setPrefixLen(UInt32Value.of(16)).build()))
.build();
FilterChain filterChain2 =
FilterChain.newBuilder()
.setName("filter-chain-2")
.setFilterChainMatch(filterChainMatch2)
.setTransportSocket(TransportSocket.getDefaultInstance())
.addFilters(filter2)
.build();
Listener listener =
Listener.newBuilder()
.setName("listener1")
.setTrafficDirection(TrafficDirection.INBOUND)
.addAllFilterChains(Arrays.asList(filterChain1, filterChain2))
.build();
thrown.expect(ResourceInvalidException.class);
thrown.expectMessage("Found duplicate matcher:");
ClientXdsClient.parseServerSideListener(
listener, new HashSet<String>(), null, filterRegistry, true /* does not matter */);
}

@Test
public void parseServerSideListener_nonUniqueFilterChainMatch_sameFilter()
throws ResourceInvalidException {
Filter filter1 = buildHttpConnectionManagerFilter(
HttpFilter.newBuilder().setName("http-filter-1").setIsOptional(true).build());
FilterChainMatch filterChainMatch1 =
FilterChainMatch.newBuilder()
.addAllSourcePorts(Arrays.asList(80, 8080))
.addAllPrefixRanges(Arrays.asList(
CidrRange.newBuilder().setAddressPrefix("10.0.0.0").setPrefixLen(UInt32Value.of(8))
.build()))
.build();
FilterChain filterChain1 =
FilterChain.newBuilder()
.setName("filter-chain-1")
.setFilterChainMatch(filterChainMatch1)
.setTransportSocket(TransportSocket.getDefaultInstance())
.addFilters(filter1)
.build();
Filter filter2 = buildHttpConnectionManagerFilter(
HttpFilter.newBuilder().setName("http-filter-2").setIsOptional(true).build());
FilterChainMatch filterChainMatch2 =
FilterChainMatch.newBuilder()
.addAllSourcePorts(Arrays.asList(443, 8080))
.addAllPrefixRanges(Arrays.asList(
CidrRange.newBuilder().setAddressPrefix("192.168.0.0")
.setPrefixLen(UInt32Value.of(16)).build(),
CidrRange.newBuilder().setAddressPrefix("192.168.0.0")
.setPrefixLen(UInt32Value.of(16)).build()))
.build();
FilterChain filterChain2 =
FilterChain.newBuilder()
.setName("filter-chain-2")
.setFilterChainMatch(filterChainMatch2)
.setTransportSocket(TransportSocket.getDefaultInstance())
.addFilters(filter2)
.build();
Listener listener =
Listener.newBuilder()
.setName("listener1")
.setTrafficDirection(TrafficDirection.INBOUND)
.addAllFilterChains(Arrays.asList(filterChain1, filterChain2))
.build();
thrown.expect(ResourceInvalidException.class);
thrown.expectMessage("Found duplicate matcher:");
ClientXdsClient.parseServerSideListener(
listener, new HashSet<String>(), null, filterRegistry, true /* does not matter */);
}

@Test
public void parseServerSideListener_uniqueFilterChainMatch() throws ResourceInvalidException {
Filter filter1 = buildHttpConnectionManagerFilter(
HttpFilter.newBuilder().setName("http-filter-1").setIsOptional(true).build());
FilterChainMatch filterChainMatch1 =
FilterChainMatch.newBuilder()
.addAllSourcePorts(Arrays.asList(80, 8080))
.addAllPrefixRanges(Arrays.asList(CidrRange.newBuilder().setAddressPrefix("192.168.0.0")
.setPrefixLen(UInt32Value.of(16)).build(),
CidrRange.newBuilder().setAddressPrefix("10.0.0.0").setPrefixLen(UInt32Value.of(8))
.build()))
.setSourceType(FilterChainMatch.ConnectionSourceType.EXTERNAL)
.build();
FilterChain filterChain1 =
FilterChain.newBuilder()
.setName("filter-chain-1")
.setFilterChainMatch(filterChainMatch1)
.setTransportSocket(TransportSocket.getDefaultInstance())
.addFilters(filter1)
.build();
Filter filter2 = buildHttpConnectionManagerFilter(
HttpFilter.newBuilder().setName("http-filter-2").setIsOptional(true).build());
FilterChainMatch filterChainMatch2 =
FilterChainMatch.newBuilder()
.addAllSourcePorts(Arrays.asList(443, 8080))
.addAllPrefixRanges(Arrays.asList(
CidrRange.newBuilder().setAddressPrefix("2001:DB8::8:800:200C:417A")
.setPrefixLen(UInt32Value.of(60)).build(),
CidrRange.newBuilder().setAddressPrefix("192.168.0.0")
.setPrefixLen(UInt32Value.of(16)).build()))
.setSourceType(FilterChainMatch.ConnectionSourceType.ANY)
.build();
FilterChain filterChain2 =
FilterChain.newBuilder()
.setName("filter-chain-2")
.setFilterChainMatch(filterChainMatch2)
.setTransportSocket(TransportSocket.getDefaultInstance())
.addFilters(filter2)
.build();
Listener listener =
Listener.newBuilder()
.setName("listener1")
.setTrafficDirection(TrafficDirection.INBOUND)
.addAllFilterChains(Arrays.asList(filterChain1, filterChain2))
.build();
ClientXdsClient.parseServerSideListener(
listener, new HashSet<String>(), null, filterRegistry, true /* does not matter */);
}

@Test
public void parseFilterChain_noHcm() throws ResourceInvalidException {
FilterChain filterChain =
Expand All @@ -1050,7 +1198,7 @@ public void parseFilterChain_noHcm() throws ResourceInvalidException {
thrown.expectMessage(
"FilterChain filter-chain-foo missing required HttpConnectionManager filter");
ClientXdsClient.parseFilterChain(
filterChain, new HashSet<String>(), null, filterRegistry, true /* does not matter */);
filterChain, new HashSet<String>(), null, filterRegistry, null, true /* does not matter */);
}

@Test
Expand All @@ -1068,7 +1216,7 @@ public void parseFilterChain_duplicateFilter() throws ResourceInvalidException {
thrown.expectMessage(
"FilterChain filter-chain-foo with duplicated filter: envoy.http_connection_manager");
ClientXdsClient.parseFilterChain(
filterChain, new HashSet<String>(), null, filterRegistry, true /* does not matter */);
filterChain, new HashSet<String>(), null, filterRegistry, null, true /* does not matter */);
}

@Test
Expand All @@ -1086,7 +1234,7 @@ public void parseFilterChain_filterMissingTypedConfig() throws ResourceInvalidEx
"FilterChain filter-chain-foo contains filter envoy.http_connection_manager "
+ "without typed_config");
ClientXdsClient.parseFilterChain(
filterChain, new HashSet<String>(), null, filterRegistry, true /* does not matter */);
filterChain, new HashSet<String>(), null, filterRegistry, null, true /* does not matter */);
}

@Test
Expand All @@ -1108,7 +1256,7 @@ public void parseFilterChain_unsupportedFilter() throws ResourceInvalidException
"FilterChain filter-chain-foo contains filter unsupported with unsupported "
+ "typed_config type unsupported-type-url");
ClientXdsClient.parseFilterChain(
filterChain, new HashSet<String>(), null, filterRegistry, true /* does not matter */);
filterChain, new HashSet<String>(), null, filterRegistry, null, true /* does not matter */);
}

@Test
Expand All @@ -1133,9 +1281,11 @@ public void parseFilterChain_noName_generatedUuid() throws ResourceInvalidExcept
.build();

EnvoyServerProtoData.FilterChain parsedFilterChain1 = ClientXdsClient.parseFilterChain(
filterChain1, new HashSet<String>(), null, filterRegistry, true /* does not matter */);
filterChain1, new HashSet<String>(), null, filterRegistry, null,
true /* does not matter */);
EnvoyServerProtoData.FilterChain parsedFilterChain2 = ClientXdsClient.parseFilterChain(
filterChain2, new HashSet<String>(), null, filterRegistry, true /* does not matter */);
filterChain2, new HashSet<String>(), null, filterRegistry, null,
true /* does not matter */);
assertThat(parsedFilterChain1.getName()).isNotEqualTo(parsedFilterChain2.getName());
}

Expand Down

0 comments on commit 392d8b3

Please sign in to comment.