envoy: Configure internal address config based on IP family

[upstream be7e55e] There is upstream issue which might cause IP malformed error, this commit is to make sure that ::1/128 is added to internal address config if IPv6 is enabled in Cilium. Relates: envoyproxy/envoy#29902 Fixes: cilium#35943 Fixes: cilium#36691 Signed-off-by: Tam Mach <[email protected]>
DataDog · Jan 9, 2025 · 286fdfe · 286fdfe
1 parent f85b07f
commit 286fdfe
Show file tree

Hide file tree

Showing 21 changed files with 121 additions and 58 deletions.
diff --git a/install/kubernetes/cilium/files/cilium-envoy/configmap/bootstrap-config.yaml b/install/kubernetes/cilium/files/cilium-envoy/configmap/bootstrap-config.yaml
@@ -33,6 +33,7 @@ staticResources:
               "@type": "type.googleapis.com/envoy.extensions.filters.http.router.v3.Router"
           internalAddressConfig:
             cidrRanges:
+            {{- if .Values.ipv4.enabled }}
             - addressPrefix: "10.0.0.0"
               prefixLen: 8
             - addressPrefix: "172.16.0.0"
@@ -41,8 +42,11 @@ staticResources:
               prefixLen: 16
             - addressPrefix: "127.0.0.1"
               prefixLen: 32
+            {{- end }}
+            {{- if .Values.ipv6.enabled }}
             - addressPrefix: "::1"
               prefixLen: 128
+            {{- end }}
           streamIdleTimeout: "0s"
   {{- end }}
   - name: "envoy-health-listener"
@@ -81,6 +85,7 @@ staticResources:
               "@type": "type.googleapis.com/envoy.extensions.filters.http.router.v3.Router"
           internalAddressConfig:
             cidrRanges:
+            {{- if .Values.ipv4.enabled }}
             - addressPrefix: "10.0.0.0"
               prefixLen: 8
             - addressPrefix: "172.16.0.0"
@@ -89,8 +94,11 @@ staticResources:
               prefixLen: 16
             - addressPrefix: "127.0.0.1"
               prefixLen: 32
+            {{- end }}
+            {{- if .Values.ipv6.enabled }}
             - addressPrefix: "::1"
               prefixLen: 128
+            {{- end }}
           streamIdleTimeout: "0s"
   clusters:
   - name: "ingress-cluster"

diff --git a/operator/cmd/root.go b/operator/cmd/root.go
@@ -53,6 +53,7 @@ import (
 	"github.com/cilium/cilium/pkg/metrics"
 	features "github.com/cilium/cilium/pkg/metrics/features/operator"
 	"github.com/cilium/cilium/pkg/option"
+	agentOption "github.com/cilium/cilium/pkg/option"
 	"github.com/cilium/cilium/pkg/pprof"
 	"github.com/cilium/cilium/pkg/rand"
 	"github.com/cilium/cilium/pkg/version"
@@ -705,6 +706,8 @@ func (legacy *legacyOnLeader) onStart(_ cell.HookContext) error {
 			ingress.WithDefaultSecretNamespace(operatorOption.Config.IngressDefaultSecretNamespace),
 			ingress.WithDefaultSecretName(operatorOption.Config.IngressDefaultSecretName),
 			ingress.WithIdleTimeoutSeconds(operatorOption.Config.ProxyIdleTimeoutSeconds),
+			ingress.EnabledIPv4(agentOption.Config.EnableIPv4),
+			ingress.EnabledIPv6(agentOption.Config.EnableIPv6),
 		)
 		if err != nil {
 			log.WithError(err).WithField(logfields.LogSubsys, ingress.Subsys).Fatal(
@@ -718,6 +721,8 @@ func (legacy *legacyOnLeader) onStart(_ cell.HookContext) error {
 			operatorOption.Config.EnableGatewayAPISecretsSync,
 			operatorOption.Config.GatewayAPISecretsNamespace,
 			operatorOption.Config.ProxyIdleTimeoutSeconds,
+			agentOption.Config.EnableIPv4,
+			agentOption.Config.EnableIPv6,
 		)
 		if err != nil {
 			log.WithError(err).WithField(logfields.LogSubsys, gatewayapi.Subsys).Fatal(
@@ -732,6 +737,8 @@ func (legacy *legacyOnLeader) onStart(_ cell.HookContext) error {
 			operatorOption.Config.LoadBalancerL7Ports,
 			operatorOption.Config.LoadBalancerL7Algorithm,
 			operatorOption.Config.ProxyIdleTimeoutSeconds,
+			agentOption.Config.EnableIPv4,
+			agentOption.Config.EnableIPv6,
 		)
 	}
 

diff --git a/operator/pkg/ciliumenvoyconfig/envoy_config.go b/operator/pkg/ciliumenvoyconfig/envoy_config.go
@@ -285,13 +285,7 @@ func (m *Manager) getConnectionManager(svc *slim_corev1.Service) (ciliumv2.XDSRe
 			UnixSockets: false,
 			// only RFC1918 IP addresses will be considered internal
 			// https://datatracker.ietf.org/doc/html/rfc1918
-			CidrRanges: []*envoy_config_core_v3.CidrRange{
-				{AddressPrefix: "10.0.0.0", PrefixLen: &wrapperspb.UInt32Value{Value: 8}},
-				{AddressPrefix: "172.16.0.0", PrefixLen: &wrapperspb.UInt32Value{Value: 12}},
-				{AddressPrefix: "192.168.0.0", PrefixLen: &wrapperspb.UInt32Value{Value: 16}},
-				{AddressPrefix: "127.0.0.1", PrefixLen: &wrapperspb.UInt32Value{Value: 32}},
-				{AddressPrefix: "::1", PrefixLen: &wrapperspb.UInt32Value{Value: 128}},
-			},
+			CidrRanges: envoy.GetInternalListenerCIDRs(m.enableIpv4, m.enableIpv6),
 		},
 	}
 

diff --git a/operator/pkg/ciliumenvoyconfig/manager.go b/operator/pkg/ciliumenvoyconfig/manager.go
@@ -38,10 +38,13 @@ type Manager struct {
 	serviceStore cache.Store
 	ports        []string
 	algorithm    string
+
+	enableIpv4 bool
+	enableIpv6 bool
 }
 
 // New returns a new Manager for CiliumEnvoyConfig
-func New(ctx context.Context, client client.Clientset, indexer cache.Store, ports []string, algorithm string, idleTimeoutSeconds int) (*Manager, error) {
+func New(ctx context.Context, client client.Clientset, indexer cache.Store, ports []string, algorithm string, idleTimeoutSeconds int, enableIpv4 bool, enableIpv6 bool) (*Manager, error) {
 	manager := &Manager{
 		queue:              workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()),
 		client:             client,
@@ -50,6 +53,8 @@ func New(ctx context.Context, client client.Clientset, indexer cache.Store, port
 		idleTimeoutSeconds: idleTimeoutSeconds,
 		ports:              ports,
 		algorithm:          algorithm,
+		enableIpv4:         enableIpv4,
+		enableIpv6:         enableIpv6,
 	}
 
 	envoyConfigManager, err := newEnvoyConfigManager(ctx, client, manager.maxRetries, manager.idleTimeoutSeconds)

diff --git a/operator/pkg/gateway-api/controller.go b/operator/pkg/gateway-api/controller.go
@@ -56,7 +56,7 @@ type Controller struct {
 
 // NewController returns a new gateway controller, which is implemented
 // using the controller-runtime library.
-func NewController(enableSecretSync bool, secretsNamespace string, idleTimeoutSeconds int) (*Controller, error) {
+func NewController(enableSecretSync bool, secretsNamespace string, idleTimeoutSeconds int, enableIPv4 bool, enableIPv6 bool) (*Controller, error) {
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Scheme: scheme,
 		// Disable controller metrics server in favour of cilium's metrics server.
@@ -85,6 +85,8 @@ func NewController(enableSecretSync bool, secretsNamespace string, idleTimeoutSe
 		Model:              m,
 		controllerName:     controllerName,
 		IdleTimeoutSeconds: idleTimeoutSeconds,
+		EnableIPv4:         enableIPv4,
+		EnableIPv6:         enableIPv6,
 	}
 	if err = gwReconciler.SetupWithManager(mgr); err != nil {
 		return nil, err

diff --git a/operator/pkg/gateway-api/gateway.go b/operator/pkg/gateway-api/gateway.go
@@ -37,6 +37,8 @@ type gatewayReconciler struct {
 	Scheme             *runtime.Scheme
 	SecretsNamespace   string
 	IdleTimeoutSeconds int
+	EnableIPv4         bool
+	EnableIPv6         bool
 
 	controllerName string
 	Model          *internalModel

diff --git a/operator/pkg/gateway-api/gateway_reconcile.go b/operator/pkg/gateway-api/gateway_reconcile.go
@@ -120,7 +120,8 @@ func (r *gatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	setGatewayAccepted(gw, true, "Gateway successfully scheduled")
 
 	// Step 3: Translate the listeners into Cilium model
-	cec, svc, ep, err := translation.NewTranslator(r.SecretsNamespace, r.IdleTimeoutSeconds).Translate(&model.Model{HTTP: httpListeners, TLS: tlsListeners})
+	cec, svc, ep, err := translation.NewTranslator(r.SecretsNamespace, r.IdleTimeoutSeconds, r.EnableIPv4, r.EnableIPv6).
+		Translate(&model.Model{HTTP: httpListeners, TLS: tlsListeners})
 	if err != nil {
 		scopedLog.WithError(err).Error("Unable to translate resources")
 		setGatewayAccepted(gw, false, "Unable to translate resources")

diff --git a/operator/pkg/ingress/ingress.go b/operator/pkg/ingress/ingress.go
@@ -127,8 +127,8 @@ func NewController(
 		defaultLoadbalancerMode: opts.DefaultLoadbalancerMode,
 		defaultSecretNamespace:  opts.DefaultSecretNamespace,
 		defaultSecretName:       opts.DefaultSecretName,
-		sharedTranslator:        ingressTranslation.NewSharedIngressTranslator(opts.SharedLBServiceName, opts.CiliumNamespace, opts.SecretsNamespace, opts.EnforcedHTTPS, opts.IdleTimeoutSeconds),
-		dedicatedTranslator:     ingressTranslation.NewDedicatedIngressTranslator(opts.SecretsNamespace, opts.EnforcedHTTPS, opts.IdleTimeoutSeconds),
+		sharedTranslator:        ingressTranslation.NewSharedIngressTranslator(opts.SharedLBServiceName, opts.CiliumNamespace, opts.SecretsNamespace, opts.EnforcedHTTPS, opts.IdleTimeoutSeconds, opts.EnableIPv4, opts.EnableIPv6),
+		dedicatedTranslator:     ingressTranslation.NewDedicatedIngressTranslator(opts.SecretsNamespace, opts.EnforcedHTTPS, opts.IdleTimeoutSeconds, opts.EnableIPv4, opts.EnableIPv6),
 	}
 	ic.ingressStore, ic.ingressInformer = informer.NewInformer(
 		utils.ListerWatcherFromTyped[*slim_networkingv1.IngressList](clientset.Slim().NetworkingV1().Ingresses(corev1.NamespaceAll)),

diff --git a/operator/pkg/ingress/ingress_options.go b/operator/pkg/ingress/ingress_options.go
@@ -3,6 +3,10 @@
 
 package ingress
 
+import (
+	"github.com/cilium/cilium/pkg/defaults"
+)
+
 // Options stores all the configurations values for cilium ingress controller.
 type Options struct {
 	MaxRetries              int
@@ -16,6 +20,8 @@ type Options struct {
 	DefaultSecretNamespace  string
 	DefaultSecretName       string
 	IdleTimeoutSeconds      int
+	EnableIPv4              bool
+	EnableIPv6              bool
 }
 
 // DefaultIngressOptions specifies default values for cilium ingress controller.
@@ -28,6 +34,8 @@ var DefaultIngressOptions = Options{
 	CiliumNamespace:         "kube-system",
 	DefaultLoadbalancerMode: "shared",
 	IdleTimeoutSeconds:      60,
+	EnableIPv4:              defaults.EnableIPv4,
+	EnableIPv6:              defaults.EnableIPv6,
 }
 
 // Option customizes the configuration of cilium ingress controller
@@ -120,3 +128,17 @@ func WithIdleTimeoutSeconds(idleTimeoutSeconds int) Option {
 		return nil
 	}
 }
+
+func EnabledIPv4(enableIPv4 bool) Option {
+	return func(o *Options) error {
+		o.EnableIPv4 = enableIPv4
+		return nil
+	}
+}
+
+func EnabledIPv6(enableIPv6 bool) Option {
+	return func(o *Options) error {
+		o.EnableIPv6 = enableIPv6
+		return nil
+	}
+}
diff --git a/operator/pkg/model/translation/envoy_http_connection_manager.go b/operator/pkg/model/translation/envoy_http_connection_manager.go
@@ -4,7 +4,6 @@
 package translation
 
 import (
-	envoy_config_core "github.com/cilium/proxy/go/envoy/config/core/v3"
 	httpRouterv3 "github.com/cilium/proxy/go/envoy/extensions/filters/http/router/v3"
 	httpConnectionManagerv3 "github.com/cilium/proxy/go/envoy/extensions/filters/network/http_connection_manager/v3"
 	"google.golang.org/protobuf/proto"
@@ -17,6 +16,16 @@ import (
 
 type HttpConnectionManagerMutator func(*httpConnectionManagerv3.HttpConnectionManager) *httpConnectionManagerv3.HttpConnectionManager
 
+func WithInternalAddressConfig(enableIpv4, enableIpv6 bool) HttpConnectionManagerMutator {
+	return func(hcm *httpConnectionManagerv3.HttpConnectionManager) *httpConnectionManagerv3.HttpConnectionManager {
+		hcm.InternalAddressConfig = &httpConnectionManagerv3.HttpConnectionManager_InternalAddressConfig{
+			UnixSockets: false,
+			CidrRanges:  envoy.GetInternalListenerCIDRs(enableIpv4, enableIpv6),
+		}
+		return hcm
+	}
+}
+
 // NewHTTPConnectionManager returns a new HTTP connection manager filter with the given name and route.
 // Mutation functions can be passed to modify the filter based on the caller's needs.
 func NewHTTPConnectionManager(name, routeName string, mutationFunc ...HttpConnectionManagerMutator) (ciliumv2.XDSResource, error) {
@@ -35,18 +44,6 @@ func NewHTTPConnectionManager(name, routeName string, mutationFunc ...HttpConnec
 				},
 			},
 		},
-		InternalAddressConfig: &httpConnectionManagerv3.HttpConnectionManager_InternalAddressConfig{
-			UnixSockets: false,
-			// only RFC1918 IP addresses will be considered internal
-			// https://datatracker.ietf.org/doc/html/rfc1918
-			CidrRanges: []*envoy_config_core.CidrRange{
-				{AddressPrefix: "10.0.0.0", PrefixLen: &wrapperspb.UInt32Value{Value: 8}},
-				{AddressPrefix: "172.16.0.0", PrefixLen: &wrapperspb.UInt32Value{Value: 12}},
-				{AddressPrefix: "192.168.0.0", PrefixLen: &wrapperspb.UInt32Value{Value: 16}},
-				{AddressPrefix: "127.0.0.1", PrefixLen: &wrapperspb.UInt32Value{Value: 32}},
-				{AddressPrefix: "::1", PrefixLen: &wrapperspb.UInt32Value{Value: 128}},
-			},
-		},
 		UpgradeConfigs: []*httpConnectionManagerv3.HttpConnectionManager_UpgradeConfig{
 			{UpgradeType: "websocket"},
 		},

diff --git a/operator/pkg/model/translation/envoy_listener.go b/operator/pkg/model/translation/envoy_listener.go
@@ -85,25 +85,26 @@ func WithSocketOption(tcpKeepAlive, tcpKeepIdleInSeconds, tcpKeepAliveProbeInter
 }
 
 // NewHTTPListenerWithDefaults same as NewListener but with default mutators applied.
-func NewHTTPListenerWithDefaults(name string, ciliumSecretNamespace string, tls map[model.TLSSecret][]string, mutatorFunc ...ListenerMutator) (ciliumv2.XDSResource, error) {
+func NewHTTPListenerWithDefaults(name string, ciliumSecretNamespace string, tls map[model.TLSSecret][]string, enableIpv4 bool, enableIpv6 bool, mutatorFunc ...ListenerMutator) (ciliumv2.XDSResource, error) {
 	fns := append(mutatorFunc,
 		WithSocketOption(
 			defaultTCPKeepAlive,
 			defaultTCPKeepAliveIdleTimeInSeconds,
 			defaultTCPKeepAliveProbeIntervalInSeconds,
 			defaultTCPKeepAliveMaxFailures),
 	)
-	return NewHTTPListener(name, ciliumSecretNamespace, tls, fns...)
+	return NewHTTPListener(name, ciliumSecretNamespace, tls, enableIpv4, enableIpv6, fns...)
 }
 
 // NewHTTPListener creates a new Envoy listener with the given name.
 // The listener will have both secure and insecure filters.
 // Secret Discovery Service (SDS) is used to fetch the TLS certificates.
-func NewHTTPListener(name string, ciliumSecretNamespace string, tls map[model.TLSSecret][]string, mutatorFunc ...ListenerMutator) (ciliumv2.XDSResource, error) {
+func NewHTTPListener(name string, ciliumSecretNamespace string, tls map[model.TLSSecret][]string, enableIpv4 bool, enableIpv6 bool, mutatorFunc ...ListenerMutator) (ciliumv2.XDSResource, error) {
 	var filterChains []*envoy_config_listener.FilterChain
 
 	insecureHttpConnectionManagerName := fmt.Sprintf("%s-insecure", name)
-	insecureHttpConnectionManager, err := NewHTTPConnectionManager(insecureHttpConnectionManagerName, insecureHttpConnectionManagerName)
+	insecureHttpConnectionManager, err := NewHTTPConnectionManager(insecureHttpConnectionManagerName, insecureHttpConnectionManagerName,
+		WithInternalAddressConfig(enableIpv4, enableIpv6))
 	if err != nil {
 		return ciliumv2.XDSResource{}, err
 	}
@@ -122,7 +123,8 @@ func NewHTTPListener(name string, ciliumSecretNamespace string, tls map[model.TL
 
 	for secret, hostNames := range tls {
 		secureHttpConnectionManagerName := fmt.Sprintf("%s-secure", name)
-		secureHttpConnectionManager, err := NewHTTPConnectionManager(secureHttpConnectionManagerName, secureHttpConnectionManagerName)
+		secureHttpConnectionManager, err := NewHTTPConnectionManager(secureHttpConnectionManagerName, secureHttpConnectionManagerName,
+			WithInternalAddressConfig(enableIpv4, enableIpv6))
 		if err != nil {
 			return ciliumv2.XDSResource{}, err
 		}

diff --git a/operator/pkg/model/translation/envoy_listener_test.go b/operator/pkg/model/translation/envoy_listener_test.go
@@ -18,7 +18,7 @@ import (
 
 func TestNewHTTPListener(t *testing.T) {
 	t.Run("without TLS", func(t *testing.T) {
-		res, err := NewHTTPListener("dummy-name", "dummy-secret-namespace", nil)
+		res, err := NewHTTPListener("dummy-name", "dummy-secret-namespace", nil, true, true)
 		require.Nil(t, err)
 
 		listener := &envoy_config_listener.Listener{}
@@ -34,7 +34,7 @@ func TestNewHTTPListener(t *testing.T) {
 		res, err := NewHTTPListener("dummy-name", "dummy-secret-namespace", map[model.TLSSecret][]string{
 			{Name: "dummy-secret-1", Namespace: "dummy-namespace"}: {"dummy.server.com"},
 			{Name: "dummy-secret-2", Namespace: "dummy-namespace"}: {"dummy.anotherserver.com"},
-		})
+		}, true, true)
 		require.Nil(t, err)
 
 		listener := &envoy_config_listener.Listener{}

diff --git a/...ay-api/testdata/conformance/httproute_backend_protocol_h_2_c_app_protocol/cec-output.yaml b/...ay-api/testdata/conformance/httproute_backend_protocol_h_2_c_app_protocol/cec-output.yaml
@@ -36,8 +36,6 @@ spec:
               prefixLen: 16
             - addressPrefix: 127.0.0.1
               prefixLen: 32
-            - addressPrefix: ::1
-              prefixLen: 128
           rds:
             routeConfigName: listener-insecure
           statPrefix: listener-insecure

diff --git a/operator/pkg/model/translation/gateway-api/translator.go b/operator/pkg/model/translation/gateway-api/translator.go
@@ -28,13 +28,17 @@ type translator struct {
 	secretsNamespace string
 
 	idleTimeoutSeconds int
+	enableIpv4         bool
+	enableIpv6         bool
 }
 
 // NewTranslator returns a new translator for Gateway API.
-func NewTranslator(secretsNamespace string, idleTimeoutSeconds int) translation.Translator {
+func NewTranslator(secretsNamespace string, idleTimeoutSeconds int, enableIpv4 bool, enableIpv6 bool) translation.Translator {
 	return &translator{
 		secretsNamespace:   secretsNamespace,
 		idleTimeoutSeconds: idleTimeoutSeconds,
+		enableIpv4:         enableIpv4,
+		enableIpv6:         enableIpv6,
 	}
 }
 
@@ -56,7 +60,7 @@ func (t *translator) Translate(m *model.Model) (*ciliumv2.CiliumEnvoyConfig, *co
 		return nil, nil, nil, fmt.Errorf("model source name can't be empty")
 	}
 
-	trans := translation.NewTranslator(ciliumGatewayPrefix+source.Name, source.Namespace, t.secretsNamespace, false, true, t.idleTimeoutSeconds)
+	trans := translation.NewTranslator(ciliumGatewayPrefix+source.Name, source.Namespace, t.secretsNamespace, false, true, t.idleTimeoutSeconds, t.enableIpv4, t.enableIpv6)
 	cec, _, _, err := trans.Translate(m)
 	if err != nil {
 		return nil, nil, nil, err

diff --git a/operator/pkg/model/translation/gateway-api/translator_test.go b/operator/pkg/model/translation/gateway-api/translator_test.go
@@ -50,6 +50,8 @@ func Test_translator_Translate(t *testing.T) {
 			trans := &translator{
 				idleTimeoutSeconds: 60,
 				secretsNamespace:   "cilium-secrets",
+				enableIpv4:         true,
+				enableIpv6:         true,
 			}
 
 			input := &model.Model{}
@@ -80,6 +82,7 @@ func Test_translator_Translate_AppProtocol(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			trans := &translator{
 				idleTimeoutSeconds: 60,
+				enableIpv4:         true,
 			}
 
 			input := &model.Model{}

diff --git a/operator/pkg/model/translation/ingress/dedicated_ingress.go b/operator/pkg/model/translation/ingress/dedicated_ingress.go
@@ -28,13 +28,17 @@ type DedicatedIngressTranslator struct {
 	secretsNamespace   string
 	enforceHTTPs       bool
 	idleTimeoutSeconds int
+	enableIpv4         bool
+	enableIpv6         bool
 }
 
-func NewDedicatedIngressTranslator(secretsNamespace string, enforceHTTPs bool, idleTimeoutSeconds int) *DedicatedIngressTranslator {
+func NewDedicatedIngressTranslator(secretsNamespace string, enforceHTTPs bool, idleTimeoutSeconds int, enableIpv4 bool, enableIpv6 bool) *DedicatedIngressTranslator {
 	return &DedicatedIngressTranslator{
 		secretsNamespace:   secretsNamespace,
 		enforceHTTPs:       enforceHTTPs,
 		idleTimeoutSeconds: idleTimeoutSeconds,
+		enableIpv4:         enableIpv4,
+		enableIpv6:         enableIpv6,
 	}
 }
 
@@ -48,7 +52,7 @@ func (d *DedicatedIngressTranslator) Translate(m *model.Model) (*ciliumv2.Cilium
 
 	// The logic is same as what we have with default translator, but with a different model
 	// (i.e. the HTTP listeners are just belonged to one Ingress resource).
-	translator := translation.NewTranslator(name, namespace, d.secretsNamespace, d.enforceHTTPs, false, d.idleTimeoutSeconds)
+	translator := translation.NewTranslator(name, namespace, d.secretsNamespace, d.enforceHTTPs, false, d.idleTimeoutSeconds, d.enableIpv4, d.enableIpv6)
 	cec, _, _, err := translator.Translate(m)
 	if err != nil {
 		return nil, nil, nil, err