envoy: reject listeners with duplicate filter chains

Perform a check for duplicate filter chains at listener validation time.

Envoy will reject listeners that contain duplicate filter chains. This
can result in a runaway stream of errors from Envoy if we allow such
CECs to enter the agent cache. This patch prevents many of these cases.

This check directly compares protobuf data, so is a strictly weaker
check than the validation performed on the Envoy side.

Signed-off-by: Andrew Sauber <andrew.sauber@isovalent.com>

Author: asauber

pkg/ciliumenvoyconfig/cec_resource_parser.go

@@ -534,6 +534,9 @@ func (r *CECResourceParser) ParseResources(cecNamespace string, cecName string,
 			if err := listener.Validate(); err != nil {
 				return xds.Resources{}, fmt.Errorf("failed to validate Listener %q (%w): %s", listener.Name, err, listener.String())
 			}
+			if listenerHasDuplicateFilterChainMatch(listener) {
+				return xds.Resources{}, fmt.Errorf("Listener %q contains filter chains with duplicate matching rules", listener.Name)
+			}
 		}
 	}
 
@@ -598,6 +601,23 @@ func (r *CECResourceParser) ParseResources(cecNamespace string, cecName string,
 	return resources, nil
 }
 
+// listenerHasDuplicateFilterChainMatch reports whether the listener contains
+// filter chains with identical matching rules. Rejection here is order
+// sensitive, so some configs pass here and Envoy will later reject them.
+func listenerHasDuplicateFilterChainMatch(listener *envoy_config_listener.Listener) bool {
+	seenMatches := make([]*envoy_config_listener.FilterChainMatch, 0, len(listener.GetFilterChains()))
+	for _, filterChain := range listener.GetFilterChains() {
+		match := filterChain.GetFilterChainMatch()
+		for _, seenMatch := range seenMatches {
+			if proto.Equal(match, seenMatch) {
+				return true
+			}
+		}
+		seenMatches = append(seenMatches, match)
+	}
+	return false
+}
+
 // 'l7lb' triggers the upstream mark to embed source pod EndpointID instead of source security ID
 func (r *CECResourceParser) getBPFMetadataListenerFilter(useOriginalSourceAddr bool, l7lb bool, proxyPort uint16, isHTTPListener bool) *envoy_config_listener.ListenerFilter {
 	conf := &cilium.BpfMetadata{

pkg/ciliumenvoyconfig/duplicate_filter_chain_test.go

@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Authors of Cilium
+
+package ciliumenvoyconfig
+
+import (
+	"testing"
+
+	envoy_config_listener "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestListenerHasDuplicateFilterChainMatch(t *testing.T) {
+	tests := []struct {
+		name            string
+		listener        *envoy_config_listener.Listener
+		expectDuplicate bool
+	}{
+		{
+			name:     "no filter chains",
+			listener: &envoy_config_listener.Listener{},
+		},
+		{
+			name:     "single filter chain",
+			listener: listenerWithFilterChains(t, &envoy_config_listener.FilterChainMatch{ServerNames: []string{"one.example.com"}}),
+		},
+		{
+			name: "unique filter chain matches",
+			listener: listenerWithFilterChains(t,
+				&envoy_config_listener.FilterChainMatch{ServerNames: []string{"one.example.com"}},
+				&envoy_config_listener.FilterChainMatch{ServerNames: []string{"two.example.com"}},
+			),
+		},
+		{
+			name: "duplicate filter chain match",
+			listener: listenerWithFilterChains(t,
+				&envoy_config_listener.FilterChainMatch{ServerNames: []string{"same.example.com"}},
+				&envoy_config_listener.FilterChainMatch{ServerNames: []string{"same.example.com"}},
+			),
+			expectDuplicate: true,
+		},
+		{
+			name: "duplicate empty filter chain matches",
+			listener: listenerWithFilterChains(t,
+				&envoy_config_listener.FilterChainMatch{},
+				&envoy_config_listener.FilterChainMatch{},
+			),
+			expectDuplicate: true,
+		},
+		{
+			name: "duplicate nil filter chain matches",
+			listener: listenerWithFilterChains(t,
+				nil,
+				nil,
+			),
+			expectDuplicate: true,
+		},
+		{
+			name: "differing transport protocol is not a duplicate",
+			listener: listenerWithFilterChains(t,
+				&envoy_config_listener.FilterChainMatch{ServerNames: []string{"same.example.com"}, TransportProtocol: "tls"},
+				&envoy_config_listener.FilterChainMatch{ServerNames: []string{"same.example.com"}, TransportProtocol: "raw_buffer"},
+			),
+		},
+		{
+			name: "same server names with matching transport protocol is a duplicate",
+			listener: listenerWithFilterChains(t,
+				&envoy_config_listener.FilterChainMatch{ServerNames: []string{"same.example.com"}, TransportProtocol: "tls"},
+				&envoy_config_listener.FilterChainMatch{ServerNames: []string{"same.example.com"}, TransportProtocol: "tls"},
+			),
+			expectDuplicate: true,
+		},
+		{
+			// Known limitation: the validator is order-sensitive, so it does
+			// not flag these as duplicates. Envoy will later reject them.
+			name: "server name order difference not detected (known limitation vs Envoy)",
+			listener: listenerWithFilterChains(t,
+				&envoy_config_listener.FilterChainMatch{ServerNames: []string{"a.example.com", "b.example.com"}},
+				&envoy_config_listener.FilterChainMatch{ServerNames: []string{"b.example.com", "a.example.com"}},
+			),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.expectDuplicate, listenerHasDuplicateFilterChainMatch(tt.listener))
+		})
+	}
+}
+
+func listenerWithFilterChains(t *testing.T, matches ...*envoy_config_listener.FilterChainMatch) *envoy_config_listener.Listener {
+	t.Helper()
+
+	listener := &envoy_config_listener.Listener{}
+	for _, match := range matches {
+		listener.FilterChains = append(listener.FilterChains, &envoy_config_listener.FilterChain{FilterChainMatch: match})
+	}
+	return listener
+}