fix(meshpassthrough): don't remove all filters chains (#11540)

While debugging the tests, I noticed a log entry: error adding listener 'outbound:passthrough:ipv6': no filter chains specified This leads to the case where the MeshPassthrough policy might consist of only IPv4 or IPv6-specific entries. In such cases, the current logic removes all filter chains and initializes a new, empty filter chain. If there’s no match for the specific IP protocol, the list could end up empty, which may cause Envoy to reject the configuration. I've added a change to check if there are rules for a specific IP protocol, and only trigger the logic if rules exist. --------- Signed-off-by: Lukasz Dziedziak <[email protected]>
kumahq · Sep 24, 2024 · a53c594 · a53c594
1 parent d61af00
commit a53c594
Show file tree

Hide file tree

Showing 4 changed files with 139 additions and 4 deletions.
diff --git a/pkg/plugins/policies/meshpassthrough/plugin/v1alpha1/plugin_test.go b/pkg/plugins/policies/meshpassthrough/plugin/v1alpha1/plugin_test.go
@@ -229,6 +229,51 @@ var _ = Describe("MeshPassthrough", func() {
 			listenersGolden: "basic.listener.golden.yaml",
 			clustersGolden:  "basic.clusters.golden.yaml",
 		}),
+		Entry("only ipv4 rules", testCase{
+			resources: []*core_xds.Resource{
+				{
+					Name:   "outbound:passthrough:ipv4",
+					Origin: generator.OriginTransparent,
+					Resource: NewListenerBuilder(envoy_common.APIV3, "outbound:passthrough:ipv4").
+						Configure(OutboundListener("0.0.0.0", 15001, core_xds.SocketAddressProtocolTCP)).
+						Configure(FilterChain(NewFilterChainBuilder(envoy_common.APIV3, envoy_common.AnonymousResource).
+							Configure(TCPProxy("outbound_passthrough_ipv4", []envoy_common.Split{
+								plugins_xds.NewSplitBuilder().WithClusterName("outbound:passthrough:ipv4").WithWeight(100).Build(),
+							}...)),
+						)).MustBuild(),
+				},
+				{
+					Name:   "outbound:passthrough:ipv6",
+					Origin: generator.OriginTransparent,
+					Resource: NewListenerBuilder(envoy_common.APIV3, "outbound:passthrough:ipv6").
+						Configure(OutboundListener("::", 15001, core_xds.SocketAddressProtocolTCP)).
+						Configure(FilterChain(NewFilterChainBuilder(envoy_common.APIV3, envoy_common.AnonymousResource).
+							Configure(TCPProxy("outbound_passthrough_ipv6", []envoy_common.Split{
+								plugins_xds.NewSplitBuilder().WithClusterName("outbound:passthrough:ipv6").WithWeight(100).Build(),
+							}...)),
+						)).MustBuild(),
+				},
+			},
+			singleItemRules: core_rules.SingleItemRules{
+				Rules: []*core_rules.Rule{
+					{
+						Subset: []core_rules.Tag{},
+						Conf: api.Conf{
+							AppendMatch: []api.Match{
+								{
+									Type:     api.MatchType("IP"),
+									Value:    "192.168.0.0",
+									Port:     pointer.To[int](80),
+									Protocol: api.ProtocolType("tcp"),
+								},
+							},
+						},
+					},
+				},
+			},
+			listenersGolden: "only-ipv4-rules.listener.golden.yaml",
+			clustersGolden:  "only-ipv4-rules.clusters.golden.yaml",
+		}),
 		Entry("simple policy", testCase{
 			resources: []*core_xds.Resource{
 				{

diff --git a/...ns/policies/meshpassthrough/plugin/v1alpha1/testdata/only-ipv4-rules.clusters.golden.yaml b/...ns/policies/meshpassthrough/plugin/v1alpha1/testdata/only-ipv4-rules.clusters.golden.yaml
@@ -0,0 +1,9 @@
+resources:
+- name: meshpassthrough_192.168.0.0_80
+  resource:
+    '@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
+    altStatName: meshpassthrough_192_168_0_0_80
+    connectTimeout: 5s
+    lbPolicy: CLUSTER_PROVIDED
+    name: meshpassthrough_192.168.0.0_80
+    type: ORIGINAL_DST
diff --git a/...ns/policies/meshpassthrough/plugin/v1alpha1/testdata/only-ipv4-rules.listener.golden.yaml b/...ns/policies/meshpassthrough/plugin/v1alpha1/testdata/only-ipv4-rules.listener.golden.yaml
@@ -0,0 +1,47 @@
+resources:
+- name: outbound:passthrough:ipv4
+  resource:
+    '@type': type.googleapis.com/envoy.config.listener.v3.Listener
+    address:
+      socketAddress:
+        address: 0.0.0.0
+        portValue: 15001
+    filterChains:
+    - filterChainMatch:
+        destinationPort: 80
+        prefixRanges:
+        - addressPrefix: 192.168.0.0
+          prefixLen: 32
+        transportProtocol: raw_buffer
+      filters:
+      - name: envoy.filters.network.tcp_proxy
+        typedConfig:
+          '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
+          cluster: meshpassthrough_192.168.0.0_80
+          statPrefix: meshpassthrough_192_168_0_0_80
+      name: meshpassthrough_192.168.0.0_80
+    listenerFilters:
+    - name: envoy.filters.listener.tls_inspector
+      typedConfig:
+        '@type': type.googleapis.com/envoy.extensions.filters.listener.tls_inspector.v3.TlsInspector
+    - name: envoy.filters.listener.http_inspector
+      typedConfig:
+        '@type': type.googleapis.com/envoy.extensions.filters.listener.http_inspector.v3.HttpInspector
+    name: outbound:passthrough:ipv4
+    trafficDirection: OUTBOUND
+- name: outbound:passthrough:ipv6
+  resource:
+    '@type': type.googleapis.com/envoy.config.listener.v3.Listener
+    address:
+      socketAddress:
+        address: '::'
+        portValue: 15001
+    filterChains:
+    - filters:
+      - name: envoy.filters.network.tcp_proxy
+        typedConfig:
+          '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy
+          cluster: outbound:passthrough:ipv6
+          statPrefix: outbound_passthrough_ipv6
+    name: outbound:passthrough:ipv6
+    trafficDirection: OUTBOUND
diff --git a/pkg/plugins/policies/meshpassthrough/plugin/xds/configurer.go b/pkg/plugins/policies/meshpassthrough/plugin/xds/configurer.go
@@ -24,12 +24,18 @@ func (c Configurer) Configure(ipv4 *envoy_listener.Listener, ipv6 *envoy_listene
 	if err != nil {
 		return err
 	}
-	if err := c.configureListener(orderedMatchers, ipv4, clustersAccumulator, false); err != nil {
-		return err
+
+	if hasIPv4Matches(orderedMatchers) {
+		if err := c.configureListener(orderedMatchers, ipv4, clustersAccumulator, false); err != nil {
+			return err
+		}
 	}
-	if err := c.configureListener(orderedMatchers, ipv6, clustersAccumulator, true); err != nil {
-		return err
+	if hasIPv6Matches(orderedMatchers) {
+		if err := c.configureListener(orderedMatchers, ipv6, clustersAccumulator, true); err != nil {
+			return err
+		}
 	}
+
 	for name, protocol := range clustersAccumulator {
 		config, err := CreateCluster(c.APIVersion, name, protocol)
 		if err != nil {
@@ -99,3 +105,31 @@ func (c Configurer) configureListenerFilter(listener *envoy_listener.Listener) e
 	}
 	return err
 }
+
+func hasIPv4Matches(orderedMatchers []FilterChainMatcher) bool {
+	for _, matcher := range orderedMatchers {
+		for _, route := range matcher.Routes {
+			if route.MatchType == Domain ||
+				route.MatchType == WildcardDomain ||
+				route.MatchType == CIDR ||
+				route.MatchType == IP {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+func hasIPv6Matches(orderedMatchers []FilterChainMatcher) bool {
+	for _, matcher := range orderedMatchers {
+		for _, route := range matcher.Routes {
+			if route.MatchType == Domain ||
+				route.MatchType == WildcardDomain ||
+				route.MatchType == CIDRV6 ||
+				route.MatchType == IPV6 {
+				return true
+			}
+		}
+	}
+	return false
+}