From 7514ec05d67d1ad03e926d39daaef4b4287b15ea Mon Sep 17 00:00:00 2001 From: Alex Harford Date: Fri, 17 Apr 2026 13:56:54 -0700 Subject: [PATCH 1/2] EV-6388: L7 logging through Istio Waypoint Proxy Adds automatic L7 logging for every Gateway using the istio-waypoint GatewayClass. The istio controller now creates three static resources in the Istio root namespace (calico-system) and Istio's deployment controller applies them as class-level defaults to all waypoints cluster-wide: - tigera-waypoint-l7-defaults ConfigMap injects the l7-collector sidecar (with --mode=waypoint on the existing ComponentL7Collector image) and the shared emptyDir + Felix CSI volumes into every waypoint pod. - tigera-waypoint-l7-als EnvoyFilter enables gRPC ALS on main_internal. - tigera-waypoint-l7-srcport EnvoyFilter captures the Forwarded header on connect_terminate and propagates the client IP as filter state. A small typed EnvoyFilter struct is introduced so the component handler (which casts to metav1.ObjectMetaAccessor) can manage the resources without taking on the networking.istio.io client-go dependency. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/controller/istio/istio_controller.go | 12 + pkg/controller/istio/istio_controller_test.go | 2 + pkg/render/istio/envoyfilter.go | 97 +++++ pkg/render/istio/istio.go | 12 + pkg/render/istio/istio_test.go | 45 +++ pkg/render/istio/l7waypoint.go | 340 ++++++++++++++++++ pkg/render/istio/l7waypoint_test.go | 234 ++++++++++++ 7 files changed, 742 insertions(+) create mode 100644 pkg/render/istio/envoyfilter.go create mode 100644 pkg/render/istio/l7waypoint.go create mode 100644 pkg/render/istio/l7waypoint_test.go diff --git a/pkg/controller/istio/istio_controller.go b/pkg/controller/istio/istio_controller.go index 12dd8996c5..3fc5130b5d 100644 --- a/pkg/controller/istio/istio_controller.go +++ b/pkg/controller/istio/istio_controller.go @@ -19,6 +19,7 @@ import ( "fmt" "strconv" + autoscalingv2 "k8s.io/api/autoscaling/v2" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -60,6 +61,17 @@ var ( // Start Watches within the Add function for any resources that this controller creates or monitors. This will trigger // calls to Reconcile() when an instance of one of the watched resources is modified. func Add(mgr manager.Manager, opts options.ControllerOptions) error { + // Register the typed EnvoyFilter so the L7 waypoint resources rendered by + // pkg/render/istio can be created/updated via the controller-runtime client. + istio.AddEnvoyFilterToScheme(mgr.GetScheme()) + + // The Istio helm charts we render include a HorizontalPodAutoscaler, so + // the scheme must know about autoscaling/v2 for resources.go's universal + // deserializer to decode those manifests. + if err := autoscalingv2.AddToScheme(mgr.GetScheme()); err != nil { + return fmt.Errorf("failed to register autoscaling/v2 with scheme: %w", err) + } + r := newReconciler(mgr, opts) c, err := ctrlruntime.NewController("istio-controller", mgr, controller.Options{Reconciler: r}) diff --git a/pkg/controller/istio/istio_controller_test.go b/pkg/controller/istio/istio_controller_test.go index fb6a05f8ff..bc838e1ac4 100644 --- a/pkg/controller/istio/istio_controller_test.go +++ b/pkg/controller/istio/istio_controller_test.go @@ -67,6 +67,7 @@ var _ = Describe("Istio controller tests", func() { Expect(rbacv1.SchemeBuilder.AddToScheme(scheme)).ShouldNot(HaveOccurred()) Expect(admregv1.SchemeBuilder.AddToScheme(scheme)).ShouldNot(HaveOccurred()) Expect(autoscalingv2.SchemeBuilder.AddToScheme(scheme)).ShouldNot(HaveOccurred()) + istio.AddEnvoyFilterToScheme(scheme) ctx = context.Background() objTrackerWithCalls = test.NewObjectTrackerWithCalls(scheme) @@ -789,6 +790,7 @@ var _ = Describe("Istio controller tests", func() { {Image: "tigera/istio-install-cni", Digest: "sha256:cni123"}, {Image: "tigera/istio-ztunnel", Digest: "sha256:ztunnel123"}, {Image: "tigera/istio-proxyv2", Digest: "sha256:proxyv2123"}, + {Image: "tigera/l7-collector", Digest: "sha256:l7collector123"}, }, }, } diff --git a/pkg/render/istio/envoyfilter.go b/pkg/render/istio/envoyfilter.go new file mode 100644 index 0000000000..ccc248ed0e --- /dev/null +++ b/pkg/render/istio/envoyfilter.go @@ -0,0 +1,97 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package istio + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +// envoyFilterGV is the GroupVersion for Istio EnvoyFilter resources. +var envoyFilterGV = schema.GroupVersion{Group: "networking.istio.io", Version: "v1alpha3"} + +// EnvoyFilter is a typed wrapper around Istio's networking.istio.io/v1alpha3 +// EnvoyFilter, used to avoid the full istio.io/client-go dependency while +// still letting the operator's component handler treat it as a +// metav1.ObjectMetaAccessor. Only the fields the operator needs to manage are +// represented; the Spec is an opaque map so we do not have to mirror the full +// EnvoyFilter schema. +type EnvoyFilter struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + Spec map[string]interface{} `json:"spec,omitempty"` +} + +// DeepCopyObject implements runtime.Object. +func (e *EnvoyFilter) DeepCopyObject() runtime.Object { + if e == nil { + return nil + } + out := &EnvoyFilter{ + TypeMeta: e.TypeMeta, + ObjectMeta: *e.DeepCopy(), + } + if e.Spec != nil { + out.Spec = runtime.DeepCopyJSON(e.Spec) + } + return out +} + +// EnvoyFilterList is the list form of EnvoyFilter. Controller-runtime's +// caching client requires the List kind to be registered alongside the item +// kind for watches and List calls to work. +type EnvoyFilterList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []EnvoyFilter `json:"items"` +} + +// DeepCopyObject implements runtime.Object. +func (l *EnvoyFilterList) DeepCopyObject() runtime.Object { + if l == nil { + return nil + } + out := &EnvoyFilterList{ + TypeMeta: l.TypeMeta, + ListMeta: *l.DeepCopy(), + } + if l.Items != nil { + out.Items = make([]EnvoyFilter, len(l.Items)) + for i := range l.Items { + l.Items[i].DeepCopyInto(&out.Items[i]) + } + } + return out +} + +// DeepCopyInto copies the EnvoyFilter fields into the provided destination. +func (e *EnvoyFilter) DeepCopyInto(out *EnvoyFilter) { + out.TypeMeta = e.TypeMeta + e.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + if e.Spec != nil { + out.Spec = runtime.DeepCopyJSON(e.Spec) + } +} + +// AddEnvoyFilterToScheme registers the EnvoyFilter type with the given +// runtime.Scheme so the operator's client can create, update, and delete it. +// Callers (controller setup and tests) must invoke this before using the +// waypoint L7 render output. +func AddEnvoyFilterToScheme(s *runtime.Scheme) { + s.AddKnownTypeWithName(envoyFilterGV.WithKind("EnvoyFilter"), &EnvoyFilter{}) + s.AddKnownTypeWithName(envoyFilterGV.WithKind("EnvoyFilterList"), &EnvoyFilterList{}) + metav1.AddToGroupVersion(s, envoyFilterGV) +} diff --git a/pkg/render/istio/istio.go b/pkg/render/istio/istio.go index 881889ff26..95f6d6f4c5 100644 --- a/pkg/render/istio/istio.go +++ b/pkg/render/istio/istio.go @@ -52,6 +52,7 @@ type IstioComponent struct { IstioInstallCNIImage string IstioZTunnelImage string IstioProxyv2Image string + L7CollectorImage string resources *IstioResources } @@ -211,6 +212,10 @@ func (c *IstioComponent) ResolveImages(is *operatorv1.ImageSet) error { if err != nil { return err } + c.L7CollectorImage, err = components.GetReference(components.ComponentL7Collector, reg, path, prefix, is) + if err != nil { + return err + } } else { c.IstioPilotImage, err = components.GetReference(components.ComponentCalicoIstioPilot, reg, path, prefix, is) if err != nil { @@ -308,6 +313,13 @@ func (c *IstioComponent) Objects() ([]client.Object, []client.Object) { objs = append(objs, res.CNI...) objs = append(objs, res.ZTunnel...) + // Waypoint L7 logging is Enterprise-only. The three resources live in the + // Istio system namespace so Istio's deployment controller applies them as + // class defaults to every Gateway using the istio-waypoint GatewayClass. + if c.cfg.Installation.Variant.IsEnterprise() { + objs = append(objs, L7WaypointObjects(c.cfg.IstioNamespace, c.L7CollectorImage)...) + } + return objs, toDelete } diff --git a/pkg/render/istio/istio_test.go b/pkg/render/istio/istio_test.go index 5cd8eb7f78..39ff7dbd44 100644 --- a/pkg/render/istio/istio_test.go +++ b/pkg/render/istio/istio_test.go @@ -66,6 +66,7 @@ func getEnterpriseTestImageSet() *operatorv1.ImageSet { {Image: "tigera/istio-install-cni", Digest: "sha256:test-cni-digest"}, {Image: "tigera/istio-ztunnel", Digest: "sha256:test-ztunnel-digest"}, {Image: "tigera/istio-proxyv2", Digest: "sha256:test-proxyv2-digest"}, + {Image: "tigera/l7-collector", Digest: "sha256:test-l7-collector-digest"}, }, }, } @@ -846,6 +847,50 @@ var _ = Describe("Istio Component Rendering", func() { Expect(data).NotTo(ContainSubstring("fake.io/fakeimg/proxyv2:faketag")) } }) + + It("should emit waypoint L7 logging resources only for Enterprise variant", func() { + // Enterprise: all three L7 waypoint resources must appear in the + // Istio root namespace, with the l7-collector image resolved onto + // the defaults ConfigMap. + cfg.Installation.Variant = operatorv1.CalicoEnterprise + _, component, err := istio.Istio(cfg) + Expect(err).ShouldNot(HaveOccurred()) + Expect(component.ResolveImages(getEnterpriseTestImageSet())).To(Succeed()) + + objsToCreate, _ := component.Objects() + + defaults, err := rtest.GetResourceOfType[*corev1.ConfigMap]( + objsToCreate, istio.L7WaypointDefaultsConfigMapName, istio.IstioNamespace) + Expect(err).ShouldNot(HaveOccurred()) + Expect(defaults.Labels).To(HaveKeyWithValue( + "gateway.istio.io/defaults-for-class", istio.IstioWaypointGatewayClass)) + expectedImage, _ := components.GetReference(components.ComponentL7Collector, + cfg.Installation.Registry, cfg.Installation.ImagePath, cfg.Installation.ImagePrefix, + getEnterpriseTestImageSet()) + Expect(defaults.Data["deployment"]).To(ContainSubstring(expectedImage)) + Expect(defaults.Data["deployment"]).To(ContainSubstring("--mode=waypoint")) + + _, err = rtest.GetResourceOfType[*istio.EnvoyFilter]( + objsToCreate, istio.L7WaypointALSFilterName, istio.IstioNamespace) + Expect(err).ShouldNot(HaveOccurred()) + _, err = rtest.GetResourceOfType[*istio.EnvoyFilter]( + objsToCreate, istio.L7WaypointSrcPortFilterName, istio.IstioNamespace) + Expect(err).ShouldNot(HaveOccurred()) + + // Calico (OSS) variant: none of the three resources should appear, + // regardless of image resolution outcome. + cfg.Installation.Variant = operatorv1.Calico + _, component, err = istio.Istio(cfg) + Expect(err).ShouldNot(HaveOccurred()) + Expect(component.ResolveImages(getCalicoTestImageSet())).To(Succeed()) + + objsToCreate, _ = component.Objects() + for _, o := range objsToCreate { + Expect(o.GetName()).NotTo(Equal(istio.L7WaypointDefaultsConfigMapName)) + Expect(o.GetName()).NotTo(Equal(istio.L7WaypointALSFilterName)) + Expect(o.GetName()).NotTo(Equal(istio.L7WaypointSrcPortFilterName)) + } + }) }) Describe("GKE Platform Configuration", func() { diff --git a/pkg/render/istio/l7waypoint.go b/pkg/render/istio/l7waypoint.go new file mode 100644 index 0000000000..60c12ffcf4 --- /dev/null +++ b/pkg/render/istio/l7waypoint.go @@ -0,0 +1,340 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package istio + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" + + "github.com/tigera/operator/pkg/render/common/securitycontext" +) + +const ( + // L7WaypointDefaultsConfigMapName is the class-level defaults ConfigMap that + // Istio's deployment controller applies to every Gateway using the + // istio-waypoint GatewayClass. + L7WaypointDefaultsConfigMapName = "tigera-waypoint-l7-defaults" + + // L7WaypointALSFilterName is the EnvoyFilter that enables gRPC ALS access + // logging on the waypoint's main_internal listener. + L7WaypointALSFilterName = "tigera-waypoint-l7-als" + + // L7WaypointSrcPortFilterName is the EnvoyFilter that captures the original + // client IP from the Forwarded header on the connect_terminate listener and + // propagates it as filter state to main_internal. + L7WaypointSrcPortFilterName = "tigera-waypoint-l7-srcport" + + // IstioWaypointGatewayClass is the standard Istio-provided GatewayClass for + // waypoint proxies. Every Gateway using this class automatically receives + // L7 logging via the class-level defaults ConfigMap. + IstioWaypointGatewayClass = "istio-waypoint" + + // gatewayClassDefaultsLabel is the label Istio's deployment controller uses + // to find class-level defaults ConfigMaps for a given GatewayClass. + gatewayClassDefaultsLabel = "gateway.istio.io/defaults-for-class" + + // forwardedFilterStateKey is the filter state key used to propagate the + // original client IP from connect_terminate to main_internal. + forwardedFilterStateKey = "io.tigera.forwarded_header" + + l7CollectorSocketMountPath = "/var/run/l7-collector" + l7CollectorSocketURI = "unix:///var/run/l7-collector/l7-collector.sock" + felixSyncMountPath = "/var/run/felix" + felixDialTarget = "/var/run/felix/nodeagent/socket" + + socketVolumeName = "l7-collector-socket" + felixVolumeName = "felix-sync" +) + +// EnvoyFilterGVK returns the GroupVersionKind for Istio EnvoyFilter resources. +func EnvoyFilterGVK() schema.GroupVersionKind { + return envoyFilterGV.WithKind("EnvoyFilter") +} + +// L7WaypointObjects returns the three resources the operator manages to enable +// L7 logging on every Gateway using the istio-waypoint GatewayClass: +// +// - A defaults ConfigMap (gateway.istio.io/defaults-for-class=istio-waypoint) +// that Istio applies as a strategic merge patch to every waypoint +// Deployment, injecting the l7-collector sidecar and its shared volumes. +// - An EnvoyFilter enabling gRPC ALS on main_internal. +// - An EnvoyFilter capturing the Forwarded header on connect_terminate and +// propagating it as filter state to main_internal. +// +// All three are created in the Istio system namespace (the root namespace +// Istiod reads class-level defaults and mesh-wide EnvoyFilters from). +func L7WaypointObjects(namespace, l7CollectorImage string) []client.Object { + return []client.Object{ + renderL7DefaultsConfigMap(namespace, l7CollectorImage), + renderALSEnvoyFilter(namespace), + renderSrcPortEnvoyFilter(namespace), + } +} + +// renderL7DefaultsConfigMap builds the class-level defaults ConfigMap. Istio's +// deployment controller reads the `deployment` key as a strategic merge patch +// onto every waypoint Deployment's PodSpec. +func renderL7DefaultsConfigMap(namespace, image string) *corev1.ConfigMap { + return &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: L7WaypointDefaultsConfigMapName, + Namespace: namespace, + Labels: map[string]string{ + gatewayClassDefaultsLabel: IstioWaypointGatewayClass, + }, + }, + Data: map[string]string{ + "deployment": waypointDeploymentOverlay(image), + }, + } +} + +// waypointDeploymentOverlay produces the YAML strategic merge patch that gets +// applied to every waypoint Deployment, injecting the l7-collector sidecar, +// the shared unix socket volume, and the Felix CSI volume. An additional +// volumeMount is applied to the existing istio-proxy container so it can write +// access logs to the shared socket. +func waypointDeploymentOverlay(image string) string { + sc := securitycontext.NewNonRootContext() + sc.ReadOnlyRootFilesystem = ptr.To(true) + + sidecar := corev1.Container{ + Name: "l7-collector", + Image: image, + Args: []string{"--mode=waypoint"}, + Env: []corev1.EnvVar{ + {Name: "FELIX_DIAL_TARGET", Value: felixDialTarget}, + {Name: "LOG_LEVEL", Value: "Info"}, + {Name: "OWNING_GATEWAY_NAME", ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.labels['gateway.networking.k8s.io/gateway-name']", + }, + }}, + {Name: "OWNING_GATEWAY_NAMESPACE", ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{FieldPath: "metadata.namespace"}, + }}, + }, + VolumeMounts: []corev1.VolumeMount{ + {Name: socketVolumeName, MountPath: l7CollectorSocketMountPath}, + {Name: felixVolumeName, MountPath: felixSyncMountPath}, + }, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("50m"), + corev1.ResourceMemory: resource.MustParse("64Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("200m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + }, + SecurityContext: sc, + } + + // Only specify the istio-proxy container's new volumeMount; strategic merge + // on containers is keyed by name, so existing fields are preserved. + istioProxyPatch := corev1.Container{ + Name: "istio-proxy", + VolumeMounts: []corev1.VolumeMount{ + {Name: socketVolumeName, MountPath: l7CollectorSocketMountPath}, + }, + } + + volumes := []corev1.Volume{ + { + Name: socketVolumeName, + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, + }, + { + Name: felixVolumeName, + VolumeSource: corev1.VolumeSource{ + CSI: &corev1.CSIVolumeSource{Driver: "csi.tigera.io"}, + }, + }, + } + + overlay := map[string]interface{}{ + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "volumes": volumes, + "containers": []corev1.Container{istioProxyPatch, sidecar}, + }, + }, + }, + } + + out, err := yaml.Marshal(overlay) + if err != nil { + // yaml.Marshal on well-formed core types is not expected to fail. + panic(fmt.Errorf("failed to marshal waypoint deployment overlay: %w", err)) + } + return string(out) +} + +// renderALSEnvoyFilter builds the EnvoyFilter that enables gRPC ALS access +// logging on the waypoint proxy's main_internal listener, streaming logs to +// the l7-collector sidecar via the shared unix socket. +func renderALSEnvoyFilter(namespace string) *EnvoyFilter { + return &EnvoyFilter{ + TypeMeta: metav1.TypeMeta{ + APIVersion: envoyFilterGV.String(), + Kind: "EnvoyFilter", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: L7WaypointALSFilterName, + Namespace: namespace, + }, + Spec: map[string]interface{}{ + // Attach mesh-wide to all waypoints. Istio's policy attachment + // (pilot/pkg/model/policyattachment.go) only matches waypoint + // proxies when the EnvoyFilter lives in the root namespace and + // carries a targetRef of kind GatewayClass with name + // "istio-waypoint". workloadSelector with the class-name label + // is silently ignored for waypoint internal listeners. + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "GatewayClass", + "group": "gateway.networking.k8s.io", + "name": IstioWaypointGatewayClass, + }, + }, + "configPatches": []interface{}{ + map[string]interface{}{ + "applyTo": "NETWORK_FILTER", + "match": map[string]interface{}{ + "listener": map[string]interface{}{ + "name": "main_internal", + "filterChain": map[string]interface{}{ + "filter": map[string]interface{}{ + "name": "envoy.filters.network.http_connection_manager", + }, + }, + }, + }, + "patch": map[string]interface{}{ + "operation": "MERGE", + "value": map[string]interface{}{ + "typed_config": map[string]interface{}{ + "@type": "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager", + "access_log": []interface{}{ + map[string]interface{}{ + "name": "envoy.access_loggers.http_grpc", + "typed_config": map[string]interface{}{ + "@type": "type.googleapis.com/envoy.extensions.access_loggers.grpc.v3.HttpGrpcAccessLogConfig", + "common_config": map[string]interface{}{ + "log_name": "tigera_l7", + "grpc_service": map[string]interface{}{ + "google_grpc": map[string]interface{}{ + "target_uri": l7CollectorSocketURI, + "stat_prefix": "l7_waypoint_als", + }, + }, + "transport_api_version": "V3", + "filter_state_objects_to_log": []interface{}{ + forwardedFilterStateKey, + }, + }, + "additional_request_headers_to_log": []interface{}{ + "x-forwarded-for", + "x-envoy-external-address", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } +} + +// renderSrcPortEnvoyFilter builds the EnvoyFilter that captures the original +// client IP from the Forwarded header (set by ztunnel on the HBONE CONNECT +// request) on the connect_terminate listener and propagates it as filter +// state to main_internal. +func renderSrcPortEnvoyFilter(namespace string) *EnvoyFilter { + return &EnvoyFilter{ + TypeMeta: metav1.TypeMeta{ + APIVersion: envoyFilterGV.String(), + Kind: "EnvoyFilter", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: L7WaypointSrcPortFilterName, + Namespace: namespace, + }, + Spec: map[string]interface{}{ + // See renderALSEnvoyFilter — waypoints require a targetRef of + // kind GatewayClass in the root namespace; workloadSelector does + // not reach the waypoint's connect_terminate listener. + "targetRefs": []interface{}{ + map[string]interface{}{ + "kind": "GatewayClass", + "group": "gateway.networking.k8s.io", + "name": IstioWaypointGatewayClass, + }, + }, + "configPatches": []interface{}{ + map[string]interface{}{ + "applyTo": "HTTP_FILTER", + "match": map[string]interface{}{ + "listener": map[string]interface{}{ + "name": "connect_terminate", + "filterChain": map[string]interface{}{ + "filter": map[string]interface{}{ + "name": "envoy.filters.network.http_connection_manager", + "subFilter": map[string]interface{}{ + "name": "connect_authority", + }, + }, + }, + }, + }, + "patch": map[string]interface{}{ + "operation": "INSERT_AFTER", + "value": map[string]interface{}{ + "name": "tigera.forwarded_header", + "typed_config": map[string]interface{}{ + "@type": "type.googleapis.com/envoy.extensions.filters.http.set_filter_state.v3.Config", + "on_request_headers": []interface{}{ + map[string]interface{}{ + "object_key": forwardedFilterStateKey, + "format_string": map[string]interface{}{ + "text_format_source": map[string]interface{}{ + "inline_string": "%REQ(forwarded)%", + }, + }, + "shared_with_upstream": "ONCE", + "factory_key": "envoy.string", + }, + }, + }, + }, + }, + }, + }, + }, + } +} diff --git a/pkg/render/istio/l7waypoint_test.go b/pkg/render/istio/l7waypoint_test.go new file mode 100644 index 0000000000..fb5096bc97 --- /dev/null +++ b/pkg/render/istio/l7waypoint_test.go @@ -0,0 +1,234 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package istio_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/yaml" + + "github.com/tigera/operator/pkg/render/istio" +) + +var _ = Describe("L7 Waypoint render", func() { + const ( + ns = "calico-system" + image = "my-registry.example.com/tigera/l7-collector:v0.0.0" + ) + + It("EnvoyFilterGVK returns networking.istio.io/v1alpha3", func() { + gvk := istio.EnvoyFilterGVK() + Expect(gvk.Group).To(Equal("networking.istio.io")) + Expect(gvk.Version).To(Equal("v1alpha3")) + Expect(gvk.Kind).To(Equal("EnvoyFilter")) + }) + + Context("L7WaypointObjects", func() { + It("returns exactly three resources in the requested namespace", func() { + objs := istio.L7WaypointObjects(ns, image) + Expect(objs).To(HaveLen(3)) + for _, o := range objs { + Expect(o.GetNamespace()).To(Equal(ns), "object %s/%s in wrong namespace", o.GetObjectKind().GroupVersionKind().Kind, o.GetName()) + } + }) + + It("returns the three expected resource names", func() { + objs := istio.L7WaypointObjects(ns, image) + names := map[string]bool{} + for _, o := range objs { + names[o.GetName()] = true + } + Expect(names).To(HaveKey(istio.L7WaypointDefaultsConfigMapName)) + Expect(names).To(HaveKey(istio.L7WaypointALSFilterName)) + Expect(names).To(HaveKey(istio.L7WaypointSrcPortFilterName)) + }) + }) + + Context("defaults ConfigMap", func() { + var cm *corev1.ConfigMap + BeforeEach(func() { + objs := istio.L7WaypointObjects(ns, image) + var ok bool + cm, ok = objs[0].(*corev1.ConfigMap) + Expect(ok).To(BeTrue(), "first object should be the defaults ConfigMap") + }) + + It("is labelled for the istio-waypoint GatewayClass", func() { + Expect(cm.Labels).To(HaveKeyWithValue( + "gateway.istio.io/defaults-for-class", istio.IstioWaypointGatewayClass)) + }) + + It("embeds the provided l7-collector image with --mode=waypoint", func() { + raw, ok := cm.Data["deployment"] + Expect(ok).To(BeTrue(), "ConfigMap must contain a `deployment` key") + + var overlay map[string]interface{} + Expect(yaml.Unmarshal([]byte(raw), &overlay)).To(Succeed()) + + containers := diveContainers(overlay) + var found bool + for _, c := range containers { + m := c.(map[string]interface{}) + if m["name"] == "l7-collector" { + found = true + Expect(m["image"]).To(Equal(image)) + args := m["args"].([]interface{}) + Expect(args).To(ContainElement("--mode=waypoint")) + } + } + Expect(found).To(BeTrue(), "overlay must contain an l7-collector sidecar container") + }) + + It("adds the shared socket volumeMount to the istio-proxy container", func() { + var overlay map[string]interface{} + Expect(yaml.Unmarshal([]byte(cm.Data["deployment"]), &overlay)).To(Succeed()) + containers := diveContainers(overlay) + + var found bool + for _, c := range containers { + m := c.(map[string]interface{}) + if m["name"] != "istio-proxy" { + continue + } + found = true + mounts := m["volumeMounts"].([]interface{}) + Expect(mounts).To(HaveLen(1)) + mount := mounts[0].(map[string]interface{}) + Expect(mount["name"]).To(Equal("l7-collector-socket")) + Expect(mount["mountPath"]).To(Equal("/var/run/l7-collector")) + } + Expect(found).To(BeTrue(), "overlay must patch the istio-proxy container") + }) + + It("declares the emptyDir and Felix CSI volumes", func() { + var overlay map[string]interface{} + Expect(yaml.Unmarshal([]byte(cm.Data["deployment"]), &overlay)).To(Succeed()) + volumes := diveVolumes(overlay) + + var hasSocket, hasFelix bool + for _, v := range volumes { + m := v.(map[string]interface{}) + switch m["name"] { + case "l7-collector-socket": + hasSocket = true + Expect(m).To(HaveKey("emptyDir")) + case "felix-sync": + hasFelix = true + csi := m["csi"].(map[string]interface{}) + Expect(csi["driver"]).To(Equal("csi.tigera.io")) + } + } + Expect(hasSocket).To(BeTrue(), "overlay must declare the l7-collector-socket emptyDir volume") + Expect(hasFelix).To(BeTrue(), "overlay must declare the felix-sync CSI volume") + }) + }) + + Context("ALS EnvoyFilter", func() { + var ef *istio.EnvoyFilter + BeforeEach(func() { + objs := istio.L7WaypointObjects(ns, image) + var ok bool + ef, ok = objs[1].(*istio.EnvoyFilter) + Expect(ok).To(BeTrue()) + Expect(ef.Kind).To(Equal("EnvoyFilter")) + Expect(ef.Name).To(Equal(istio.L7WaypointALSFilterName)) + }) + + It("attaches to the istio-waypoint GatewayClass", func() { + refs := ef.Spec["targetRefs"].([]interface{}) + Expect(refs).To(HaveLen(1)) + ref := refs[0].(map[string]interface{}) + Expect(ref).To(HaveKeyWithValue("kind", "GatewayClass")) + Expect(ref).To(HaveKeyWithValue("group", "gateway.networking.k8s.io")) + Expect(ref).To(HaveKeyWithValue("name", istio.IstioWaypointGatewayClass)) + }) + + It("patches the main_internal listener", func() { + patch := firstConfigPatch(ef) + listener := patch["match"].(map[string]interface{})["listener"].(map[string]interface{}) + Expect(listener["name"]).To(Equal("main_internal")) + }) + + It("configures the l7-collector unix socket as gRPC ALS target", func() { + patch := firstConfigPatch(ef) + value := patch["patch"].(map[string]interface{})["value"].(map[string]interface{}) + typedConfig := value["typed_config"].(map[string]interface{}) + accessLog := typedConfig["access_log"].([]interface{})[0].(map[string]interface{}) + common := accessLog["typed_config"].(map[string]interface{})["common_config"].(map[string]interface{}) + grpc := common["grpc_service"].(map[string]interface{})["google_grpc"].(map[string]interface{}) + Expect(grpc["target_uri"]).To(Equal("unix:///var/run/l7-collector/l7-collector.sock")) + }) + }) + + Context("SrcPort EnvoyFilter", func() { + var ef *istio.EnvoyFilter + BeforeEach(func() { + objs := istio.L7WaypointObjects(ns, image) + var ok bool + ef, ok = objs[2].(*istio.EnvoyFilter) + Expect(ok).To(BeTrue()) + Expect(ef.Kind).To(Equal("EnvoyFilter")) + Expect(ef.Name).To(Equal(istio.L7WaypointSrcPortFilterName)) + }) + + It("attaches to the istio-waypoint GatewayClass", func() { + refs := ef.Spec["targetRefs"].([]interface{}) + Expect(refs).To(HaveLen(1)) + ref := refs[0].(map[string]interface{}) + Expect(ref).To(HaveKeyWithValue("kind", "GatewayClass")) + Expect(ref).To(HaveKeyWithValue("group", "gateway.networking.k8s.io")) + Expect(ref).To(HaveKeyWithValue("name", istio.IstioWaypointGatewayClass)) + }) + + It("inserts after connect_authority on the connect_terminate listener", func() { + patch := firstConfigPatch(ef) + listener := patch["match"].(map[string]interface{})["listener"].(map[string]interface{}) + Expect(listener["name"]).To(Equal("connect_terminate")) + subFilter := listener["filterChain"].(map[string]interface{})["filter"].(map[string]interface{})["subFilter"].(map[string]interface{}) + Expect(subFilter["name"]).To(Equal("connect_authority")) + Expect(patch["patch"].(map[string]interface{})["operation"]).To(Equal("INSERT_AFTER")) + }) + + It("propagates io.tigera.forwarded_header to upstream", func() { + patch := firstConfigPatch(ef) + value := patch["patch"].(map[string]interface{})["value"].(map[string]interface{}) + typedConfig := value["typed_config"].(map[string]interface{}) + onReq := typedConfig["on_request_headers"].([]interface{})[0].(map[string]interface{}) + Expect(onReq["object_key"]).To(Equal("io.tigera.forwarded_header")) + Expect(onReq["shared_with_upstream"]).To(Equal("ONCE")) + }) + }) +}) + +// diveContainers returns the containers slice from a strategic merge overlay +// shaped like {spec: {template: {spec: {containers: [...]}}}}. +func diveContainers(overlay map[string]interface{}) []interface{} { + return overlay["spec"].(map[string]interface{})["template"].(map[string]interface{})["spec"].(map[string]interface{})["containers"].([]interface{}) +} + +// diveVolumes returns the volumes slice from the same overlay shape. +func diveVolumes(overlay map[string]interface{}) []interface{} { + return overlay["spec"].(map[string]interface{})["template"].(map[string]interface{})["spec"].(map[string]interface{})["volumes"].([]interface{}) +} + +// firstConfigPatch returns the first entry in spec.configPatches of an +// EnvoyFilter. +func firstConfigPatch(ef *istio.EnvoyFilter) map[string]interface{} { + patches := ef.Spec["configPatches"].([]interface{}) + Expect(patches).NotTo(BeEmpty()) + return patches[0].(map[string]interface{}) +} From 739a32a1c27de9f3edb979d20078828b1dacb7a3 Mon Sep 17 00:00:00 2001 From: Alex Harford Date: Mon, 4 May 2026 10:35:53 -0700 Subject: [PATCH 2/2] EV-6388: auto-program FelixConfiguration.policySyncPathPrefix The L7 ambient waypoint pod's l7-collector sidecar dials Felix's nodeagent socket, which Felix only opens when policySyncPathPrefix is set. Have the istio controller program the field as part of its reconcile, mirroring the applicationlayer controller's existing patching path. The two controllers coordinate explicitly via shared predicates in pkg/controller/utils/policy_sync.go: each side's deletion path consults the other's CR before clearing the field, so neither strands the other. The customer-override branch (any non-default value) is preserved verbatim by both. Bonus: setIstioFelixConfiguration and the two existing configurators now return (changed, error) so utils.PatchFelixConfiguration's no-op short- circuit fires when nothing changed, instead of churning the FC on every reconcile. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../applicationlayer_controller.go | 55 ++++---- .../applicationlayer_controller_test.go | 45 ++++++- pkg/controller/istio/istio_controller.go | 106 +++++++++++---- pkg/controller/istio/istio_controller_test.go | 123 ++++++++++++++++++ pkg/controller/utils/policy_sync.go | 85 ++++++++++++ pkg/controller/utils/policy_sync_test.go | 99 ++++++++++++++ pkg/controller/utils/utils.go | 15 +++ 7 files changed, 477 insertions(+), 51 deletions(-) create mode 100644 pkg/controller/utils/policy_sync.go create mode 100644 pkg/controller/utils/policy_sync_test.go diff --git a/pkg/controller/applicationlayer/applicationlayer_controller.go b/pkg/controller/applicationlayer/applicationlayer_controller.go index 045581584b..aa4e6dee0b 100644 --- a/pkg/controller/applicationlayer/applicationlayer_controller.go +++ b/pkg/controller/applicationlayer/applicationlayer_controller.go @@ -50,9 +50,10 @@ const ResourceName = "applicationlayer" var log = logf.Log.WithName("controller_applicationlayer") -const ( - DefaultPolicySyncPrefix string = "/var/run/nodeagent" -) +// DefaultPolicySyncPrefix is the operator-managed value for +// FelixConfiguration.policySyncPathPrefix. The single source of truth lives +// in the utils package so the istio controller can write the same value. +const DefaultPolicySyncPrefix = utils.DefaultPolicySyncPrefix // Add creates a new ApplicationLayer Controller and adds it to the Manager. // The Manager will set fields on the Controller and Start it when the Manager is Started. @@ -475,26 +476,9 @@ func (r *ReconcileApplicationLayer) isSidecarInjectionEnabled(applicationLayerSp *applicationLayerSpec.SidecarInjection == operatorv1.SidecarEnabled } -func (r *ReconcileApplicationLayer) getPolicySyncPathPrefix(fcSpec *v3.FelixConfigurationSpec, al *operatorv1.ApplicationLayer) string { - // Respect existing policySyncPathPrefix if it's already set (e.g. EGW) - // This will cause policySyncPathPrefix value to remain when ApplicationLayer is disabled. - existing := fcSpec.PolicySyncPathPrefix - if existing != "" { - return existing - } - - // There's no existing value, nor is ApplicationLayer enabled - if al == nil { - return "" - } - - // No existing value. However, at least one of the applicationLayer features are enabled - spec := &al.Spec - if r.isALPEnabled(spec) || r.isWAFEnabled(spec) || r.isLogsCollectionEnabled(spec) || - r.isSidecarInjectionEnabled(spec) { - return DefaultPolicySyncPrefix - } - return "" +func (r *ReconcileApplicationLayer) getPolicySyncPathPrefix(fcSpec *v3.FelixConfigurationSpec, al *operatorv1.ApplicationLayer, istioNeeds bool) string { + alNeeds := utils.ApplicationLayerRequiresPolicySync(al) + return utils.DesiredPolicySyncPathPrefix(fcSpec.PolicySyncPathPrefix, alNeeds, istioNeeds) } func (r *ReconcileApplicationLayer) getTProxyMode(al *operatorv1.ApplicationLayer) (bool, string) { @@ -516,7 +500,28 @@ func (r *ReconcileApplicationLayer) getTProxyMode(al *operatorv1.ApplicationLaye // patchFelixConfiguration takes all application layer specs as arguments and patches felix config. // If at least one of the specs requires TPROXYMode as "Enabled" it'll be patched as "Enabled" otherwise it is "Disabled". func (r *ReconcileApplicationLayer) patchFelixConfiguration(ctx context.Context, al *operatorv1.ApplicationLayer) error { - _, err := utils.PatchFelixConfiguration(ctx, r.client, func(fc *v3.FelixConfiguration) (bool, error) { + // Symmetric coordination on policySyncPathPrefix: the istio controller + // also writes this field for the L7 ambient waypoint flow. Fetch the + // Istio CR and Installation variant so DesiredPolicySyncPathPrefix can + // see whether the istio side still needs the field. Both reads tolerate + // NotFound — the istio side has no claim if either is absent. + istioCR, err := utils.GetIstio(ctx, r.client) + if err != nil { + return err + } + // Use Spec.Variant (via the second return of GetInstallationSpec) so the + // gate matches the renderer's decision to ship the L7 waypoint sidecar. + var variant operatorv1.ProductVariant + if _, spec, ierr := utils.GetInstallationSpec(ctx, r.client); ierr != nil { + if !apierrors.IsNotFound(ierr) { + return ierr + } + } else if spec != nil { + variant = spec.Variant + } + istioNeeds := utils.IstioRequiresPolicySync(istioCR, variant) + + _, err = utils.PatchFelixConfiguration(ctx, r.client, func(fc *v3.FelixConfiguration) (bool, error) { var tproxyMode string if ok, v := r.getTProxyMode(al); ok { tproxyMode = v @@ -538,7 +543,7 @@ func (r *ReconcileApplicationLayer) patchFelixConfiguration(ctx context.Context, tproxyMode = "Disabled" } - policySyncPrefix := r.getPolicySyncPathPrefix(&fc.Spec, al) + policySyncPrefix := r.getPolicySyncPathPrefix(&fc.Spec, al, istioNeeds) policySyncPrefixSetDesired := fc.Spec.PolicySyncPathPrefix == policySyncPrefix tproxyModeSetDesired := fc.Spec.TPROXYMode != "" && fc.Spec.TPROXYMode == string(tproxyMode) wafEventLogsFileEnabled := al != nil && ((al.Spec.SidecarInjection != nil && *al.Spec.SidecarInjection == operatorv1.SidecarEnabled) || diff --git a/pkg/controller/applicationlayer/applicationlayer_controller_test.go b/pkg/controller/applicationlayer/applicationlayer_controller_test.go index 36d7ca5d90..2e7dd6ea9e 100644 --- a/pkg/controller/applicationlayer/applicationlayer_controller_test.go +++ b/pkg/controller/applicationlayer/applicationlayer_controller_test.go @@ -148,13 +148,56 @@ var _ = Describe("Application layer controller tests", func() { _, err = r.Reconcile(ctx, reconcile.Request{}) Expect(err).ShouldNot(HaveOccurred()) - By("ensuring that felix configuration PolicySyncPathPrefix is left as is, even after ALP deletion") + By("ensuring that PolicySyncPathPrefix is cleared after ALP deletion when nothing else needs it") f2 := v3.FelixConfiguration{ ObjectMeta: metav1.ObjectMeta{ Name: "default", }, } Expect(test.GetResource(c, &f2)).To(BeNil()) + Expect(f2.Spec.PolicySyncPathPrefix).To(Equal("")) + }) + + It("should leave PolicySyncPathPrefix set on AL deletion when Istio CR still needs it", func() { + // Symmetric coordination: AL cleanup must consult Istio state + // before clearing policySyncPathPrefix. With an Istio CR present + // on an Enterprise install, the istio side claims the field. + mockStatus.On("AddDaemonsets", mock.Anything).Return() + mockStatus.On("AddDeployments", mock.Anything).Return() + mockStatus.On("IsAvailable").Return(true) + mockStatus.On("AddStatefulSets", mock.Anything).Return() + mockStatus.On("AddCronJobs", mock.Anything) + mockStatus.On("OnCRNotFound").Return() + mockStatus.On("ClearDegraded") + mockStatus.On("ReadyToMonitor") + mockStatus.On("SetMetaData", mock.Anything).Return() + Expect(c.Create(ctx, installation)).NotTo(HaveOccurred()) + Expect(c.Create(ctx, &operatorv1.Istio{ObjectMeta: metav1.ObjectMeta{Name: "default"}})).NotTo(HaveOccurred()) + + enabled := operatorv1.ApplicationLayerPolicyEnabled + alSpec := &operatorv1.ApplicationLayer{ + ObjectMeta: metav1.ObjectMeta{Name: "tigera-secure"}, + Spec: operatorv1.ApplicationLayerSpec{ + ApplicationLayerPolicy: &enabled, + }, + } + Expect(c.Create(ctx, alSpec)).NotTo(HaveOccurred()) + + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + + f1 := v3.FelixConfiguration{ObjectMeta: metav1.ObjectMeta{Name: "default"}} + Expect(test.GetResource(c, &f1)).To(BeNil()) + Expect(f1.Spec.PolicySyncPathPrefix).To(Equal("/var/run/nodeagent")) + + Expect(c.Delete(ctx, alSpec)).NotTo(HaveOccurred()) + + _, err = r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + + By("ensuring AL deletion does not clear policySyncPathPrefix while Istio still needs it") + f2 := v3.FelixConfiguration{ObjectMeta: metav1.ObjectMeta{Name: "default"}} + Expect(test.GetResource(c, &f2)).To(BeNil()) Expect(f2.Spec.PolicySyncPathPrefix).To(Equal("/var/run/nodeagent")) }) diff --git a/pkg/controller/istio/istio_controller.go b/pkg/controller/istio/istio_controller.go index 3fc5130b5d..4f67851c15 100644 --- a/pkg/controller/istio/istio_controller.go +++ b/pkg/controller/istio/istio_controller.go @@ -277,20 +277,22 @@ func updateDefaults(istio *operatorv1.Istio) { } func (r *ReconcileIstio) setIstioFelixConfiguration(ctx context.Context, instance *operatorv1.Istio, fc *v3.FelixConfiguration, remove bool) (bool, error) { - // Handle Istio Ambient Mode configuration - if err := r.configureIstioAmbientMode(fc, remove); err != nil { + ambientChanged, err := r.configureIstioAmbientMode(fc, remove) + if err != nil { return false, err } - - // Handle Istio DSCP Mark configuration - if err := r.configureIstioDSCPMark(instance, fc, remove); err != nil { + dscpChanged, err := r.configureIstioDSCPMark(instance, fc, remove) + if err != nil { return false, err } - - return true, nil + policySyncChanged, err := r.configurePolicySyncPathPrefix(ctx, instance, fc, remove) + if err != nil { + return false, err + } + return ambientChanged || dscpChanged || policySyncChanged, nil } -func (r *ReconcileIstio) configureIstioAmbientMode(fc *v3.FelixConfiguration, remove bool) error { +func (r *ReconcileIstio) configureIstioAmbientMode(fc *v3.FelixConfiguration, remove bool) (bool, error) { var annotationMode *string if fc.Annotations[istio.IstioOperatorAnnotationMode] != "" { value := fc.Annotations[istio.IstioOperatorAnnotationMode] @@ -302,30 +304,37 @@ func (r *ReconcileIstio) configureIstioAmbientMode(fc *v3.FelixConfiguration, re annotationMode != nil && fc.Spec.IstioAmbientMode != nil && *annotationMode == string(*fc.Spec.IstioAmbientMode) if !match { - return fmt.Errorf("felixconfig IstioAmbientMode modified by user") + return false, fmt.Errorf("felixconfig IstioAmbientMode modified by user") } if remove { + if annotationMode == nil && fc.Spec.IstioAmbientMode == nil { + return false, nil + } delete(fc.Annotations, istio.IstioOperatorAnnotationMode) fc.Spec.IstioAmbientMode = nil - } else { - istioModeDesired := v3.IstioAmbientModeEnabled - fc.Spec.IstioAmbientMode = &istioModeDesired - if fc.Annotations == nil { - fc.Annotations = make(map[string]string) - } - fc.Annotations[istio.IstioOperatorAnnotationMode] = string(istioModeDesired) + return true, nil } - return nil + istioModeDesired := v3.IstioAmbientModeEnabled + if fc.Spec.IstioAmbientMode != nil && *fc.Spec.IstioAmbientMode == istioModeDesired && + annotationMode != nil && *annotationMode == string(istioModeDesired) { + return false, nil + } + fc.Spec.IstioAmbientMode = &istioModeDesired + if fc.Annotations == nil { + fc.Annotations = make(map[string]string) + } + fc.Annotations[istio.IstioOperatorAnnotationMode] = string(istioModeDesired) + return true, nil } -func (r *ReconcileIstio) configureIstioDSCPMark(instance *operatorv1.Istio, fc *v3.FelixConfiguration, remove bool) error { +func (r *ReconcileIstio) configureIstioDSCPMark(instance *operatorv1.Istio, fc *v3.FelixConfiguration, remove bool) (bool, error) { var annotationDSCP *numorstring.DSCP if fc.Annotations[istio.IstioOperatorAnnotationDSCP] != "" { value, err := strconv.ParseUint(fc.Annotations[istio.IstioOperatorAnnotationDSCP], 10, 6) if err != nil { - return err + return false, err } dscp := numorstring.DSCPFromInt(uint8(value)) annotationDSCP = &dscp @@ -336,19 +345,66 @@ func (r *ReconcileIstio) configureIstioDSCPMark(instance *operatorv1.Istio, fc * annotationDSCP != nil && fc.Spec.IstioDSCPMark != nil && annotationDSCP.ToUint8() == fc.Spec.IstioDSCPMark.ToUint8() if !match { - return fmt.Errorf("felixconfig IstioDSCPMark modified by user") + return false, fmt.Errorf("felixconfig IstioDSCPMark modified by user") } if remove || instance.Spec.DSCPMark == nil { + if annotationDSCP == nil && fc.Spec.IstioDSCPMark == nil { + return false, nil + } delete(fc.Annotations, istio.IstioOperatorAnnotationDSCP) fc.Spec.IstioDSCPMark = nil - } else { - istioDSCPMarkDesired := *instance.Spec.DSCPMark - fc.Spec.IstioDSCPMark = &istioDSCPMarkDesired - fc.Annotations[istio.IstioOperatorAnnotationDSCP] = strconv.FormatUint(uint64(istioDSCPMarkDesired.ToUint8()), 10) + return true, nil } - return nil + istioDSCPMarkDesired := *instance.Spec.DSCPMark + if fc.Spec.IstioDSCPMark != nil && annotationDSCP != nil && + fc.Spec.IstioDSCPMark.ToUint8() == istioDSCPMarkDesired.ToUint8() && + annotationDSCP.ToUint8() == istioDSCPMarkDesired.ToUint8() { + return false, nil + } + fc.Spec.IstioDSCPMark = &istioDSCPMarkDesired + fc.Annotations[istio.IstioOperatorAnnotationDSCP] = strconv.FormatUint(uint64(istioDSCPMarkDesired.ToUint8()), 10) + return true, nil +} + +// configurePolicySyncPathPrefix reconciles FelixConfiguration.policySyncPathPrefix +// for the Istio side. The L7 ambient waypoint pod's l7-collector sidecar +// dials Felix's nodeagent socket, which Felix only opens when this field +// is set. The applicationlayer controller writes this same field for the +// Dikastes/sidecar/WAF flow; both controllers consult each other's state +// (via utils.{ApplicationLayerRequiresPolicySync,IstioRequiresPolicySync}) +// so that deleting one CR does not strand the other. +func (r *ReconcileIstio) configurePolicySyncPathPrefix(ctx context.Context, instance *operatorv1.Istio, fc *v3.FelixConfiguration, remove bool) (bool, error) { + var istioNeeds bool + if !remove { + // Mirror the renderer gate at pkg/render/istio/istio.go: it reads + // installationSpec.Variant (i.e. Installation.Spec.Variant), so the + // policy-sync field tracks the renderer's decision to ship the L7 + // waypoint sidecar even before Status.Variant catches up. + _, installationSpec, err := utils.GetInstallationSpec(ctx, r.Client) + if err != nil && !errors.IsNotFound(err) { + return false, err + } + var variant operatorv1.ProductVariant + if installationSpec != nil { + variant = installationSpec.Variant + } + istioNeeds = utils.IstioRequiresPolicySync(instance, variant) + } + + al, err := utils.GetApplicationLayer(ctx, r.Client) + if err != nil { + return false, err + } + alNeeds := utils.ApplicationLayerRequiresPolicySync(al) + + desired := utils.DesiredPolicySyncPathPrefix(fc.Spec.PolicySyncPathPrefix, alNeeds, istioNeeds) + if fc.Spec.PolicySyncPathPrefix == desired { + return false, nil + } + fc.Spec.PolicySyncPathPrefix = desired + return true, nil } func (r *ReconcileIstio) maintainFinalizer(ctx context.Context, instance *operatorv1.Istio, reqLogger logr.Logger) (res reconcile.Result, err error, finalized bool) { diff --git a/pkg/controller/istio/istio_controller_test.go b/pkg/controller/istio/istio_controller_test.go index bc838e1ac4..41f313a23a 100644 --- a/pkg/controller/istio/istio_controller_test.go +++ b/pkg/controller/istio/istio_controller_test.go @@ -499,6 +499,129 @@ var _ = Describe("Istio controller tests", func() { Expect(err.Error()).To(ContainSubstring("felixconfig IstioDSCPMark modified by user")) }) + Context("policySyncPathPrefix coordination", func() { + // All tests in this block run with Enterprise variant: the L7 + // waypoint feature that makes Istio claim policySyncPathPrefix + // is Enterprise-only. The parent Context's BeforeEach has + // already created the Installation as Calico, so we re-read and + // patch it to Enterprise here. + BeforeEach(func() { + inst := &operatorv1.Installation{} + Expect(cli.Get(ctx, types.NamespacedName{Name: "default"}, inst)).NotTo(HaveOccurred()) + inst.Spec.Variant = operatorv1.CalicoEnterprise + inst.Status.Variant = operatorv1.CalicoEnterprise + Expect(cli.Update(ctx, inst)).NotTo(HaveOccurred()) + }) + + It("sets policySyncPathPrefix to the operator default when no AL is present", func() { + fc := &v3.FelixConfiguration{ObjectMeta: metav1.ObjectMeta{Name: "default"}} + Expect(cli.Create(ctx, fc)).NotTo(HaveOccurred()) + + r := &ReconcileIstio{Client: cli, scheme: scheme, provider: operatorv1.ProviderNone, status: mockStatus} + _, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "default"}}) + Expect(err).ShouldNot(HaveOccurred()) + + patched := &v3.FelixConfiguration{} + Expect(cli.Get(ctx, types.NamespacedName{Name: "default"}, patched)).NotTo(HaveOccurred()) + Expect(patched.Spec.PolicySyncPathPrefix).To(Equal("/var/run/nodeagent")) + }) + + It("does not stomp a customer override on policySyncPathPrefix", func() { + fc := &v3.FelixConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "default"}, + Spec: v3.FelixConfigurationSpec{PolicySyncPathPrefix: "/var/run/customer"}, + } + Expect(cli.Create(ctx, fc)).NotTo(HaveOccurred()) + + r := &ReconcileIstio{Client: cli, scheme: scheme, provider: operatorv1.ProviderNone, status: mockStatus} + _, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "default"}}) + Expect(err).ShouldNot(HaveOccurred()) + + patched := &v3.FelixConfiguration{} + Expect(cli.Get(ctx, types.NamespacedName{Name: "default"}, patched)).NotTo(HaveOccurred()) + Expect(patched.Spec.PolicySyncPathPrefix).To(Equal("/var/run/customer")) + }) + + It("leaves policySyncPathPrefix set on Istio deletion when ApplicationLayer still needs it", func() { + // AL with logsCollection enabled — the symmetric coordination + // case the user called out: Istio deletion must not clear the + // field while AL is still actively using it. + enabled := operatorv1.L7LogCollectionEnabled + al := &operatorv1.ApplicationLayer{ + ObjectMeta: metav1.ObjectMeta{Name: "tigera-secure"}, + Spec: operatorv1.ApplicationLayerSpec{ + LogCollection: &operatorv1.LogCollectionSpec{CollectLogs: &enabled}, + }, + } + Expect(cli.Create(ctx, al)).NotTo(HaveOccurred()) + + fc := &v3.FelixConfiguration{ObjectMeta: metav1.ObjectMeta{Name: "default"}} + Expect(cli.Create(ctx, fc)).NotTo(HaveOccurred()) + + r := &ReconcileIstio{Client: cli, scheme: scheme, provider: operatorv1.ProviderNone, status: mockStatus} + _, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "default"}}) + Expect(err).ShouldNot(HaveOccurred()) + + // Now delete the Istio CR and reconcile the finalizer cleanup. + updated := &operatorv1.Istio{} + Expect(cli.Get(ctx, types.NamespacedName{Name: "default"}, updated)).NotTo(HaveOccurred()) + Expect(cli.Delete(ctx, updated)).NotTo(HaveOccurred()) + _, err = r.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "default"}}) + Expect(err).ShouldNot(HaveOccurred()) + + cleaned := &v3.FelixConfiguration{} + Expect(cli.Get(ctx, types.NamespacedName{Name: "default"}, cleaned)).NotTo(HaveOccurred()) + Expect(cleaned.Spec.PolicySyncPathPrefix).To(Equal("/var/run/nodeagent")) + }) + + It("clears policySyncPathPrefix on Istio deletion when ApplicationLayer features are all disabled", func() { + disabled := operatorv1.L7LogCollectionDisabled + al := &operatorv1.ApplicationLayer{ + ObjectMeta: metav1.ObjectMeta{Name: "tigera-secure"}, + Spec: operatorv1.ApplicationLayerSpec{ + LogCollection: &operatorv1.LogCollectionSpec{CollectLogs: &disabled}, + }, + } + Expect(cli.Create(ctx, al)).NotTo(HaveOccurred()) + + fc := &v3.FelixConfiguration{ObjectMeta: metav1.ObjectMeta{Name: "default"}} + Expect(cli.Create(ctx, fc)).NotTo(HaveOccurred()) + + r := &ReconcileIstio{Client: cli, scheme: scheme, provider: operatorv1.ProviderNone, status: mockStatus} + _, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "default"}}) + Expect(err).ShouldNot(HaveOccurred()) + + updated := &operatorv1.Istio{} + Expect(cli.Get(ctx, types.NamespacedName{Name: "default"}, updated)).NotTo(HaveOccurred()) + Expect(cli.Delete(ctx, updated)).NotTo(HaveOccurred()) + _, err = r.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "default"}}) + Expect(err).ShouldNot(HaveOccurred()) + + cleaned := &v3.FelixConfiguration{} + Expect(cli.Get(ctx, types.NamespacedName{Name: "default"}, cleaned)).NotTo(HaveOccurred()) + Expect(cleaned.Spec.PolicySyncPathPrefix).To(Equal("")) + }) + + It("clears policySyncPathPrefix on Istio deletion when ApplicationLayer is absent", func() { + fc := &v3.FelixConfiguration{ObjectMeta: metav1.ObjectMeta{Name: "default"}} + Expect(cli.Create(ctx, fc)).NotTo(HaveOccurred()) + + r := &ReconcileIstio{Client: cli, scheme: scheme, provider: operatorv1.ProviderNone, status: mockStatus} + _, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "default"}}) + Expect(err).ShouldNot(HaveOccurred()) + + updated := &operatorv1.Istio{} + Expect(cli.Get(ctx, types.NamespacedName{Name: "default"}, updated)).NotTo(HaveOccurred()) + Expect(cli.Delete(ctx, updated)).NotTo(HaveOccurred()) + _, err = r.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Name: "default"}}) + Expect(err).ShouldNot(HaveOccurred()) + + cleaned := &v3.FelixConfiguration{} + Expect(cli.Get(ctx, types.NamespacedName{Name: "default"}, cleaned)).NotTo(HaveOccurred()) + Expect(cleaned.Spec.PolicySyncPathPrefix).To(Equal("")) + }) + }) + It("should clear FelixConfiguration on deletion", func() { // Create empty FelixConfiguration fc := &v3.FelixConfiguration{ diff --git a/pkg/controller/utils/policy_sync.go b/pkg/controller/utils/policy_sync.go new file mode 100644 index 0000000000..e9418ab86e --- /dev/null +++ b/pkg/controller/utils/policy_sync.go @@ -0,0 +1,85 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package utils + +import ( + operatorv1 "github.com/tigera/operator/api/v1" +) + +// DefaultPolicySyncPrefix is the operator-managed value for +// FelixConfiguration.policySyncPathPrefix. The applicationlayer and istio +// controllers both write this value when their respective features need a +// running policy-sync gRPC server on the host (Dikastes sidecar, Istio +// ambient waypoint l7-collector, EGW). +const DefaultPolicySyncPrefix = "/var/run/nodeagent" + +// ApplicationLayerRequiresPolicySync reports whether the given +// ApplicationLayer CR has any feature enabled that requires +// policySyncPathPrefix to be set on FelixConfiguration. A nil receiver +// returns false (the AL CR is absent or being deleted). +func ApplicationLayerRequiresPolicySync(al *operatorv1.ApplicationLayer) bool { + if al == nil { + return false + } + spec := &al.Spec + if spec.LogCollection != nil && spec.LogCollection.CollectLogs != nil && + *spec.LogCollection.CollectLogs == operatorv1.L7LogCollectionEnabled { + return true + } + if spec.WebApplicationFirewall != nil && + *spec.WebApplicationFirewall == operatorv1.WAFEnabled { + return true + } + if spec.ApplicationLayerPolicy != nil && + *spec.ApplicationLayerPolicy == operatorv1.ApplicationLayerPolicyEnabled { + return true + } + if spec.SidecarInjection != nil && + *spec.SidecarInjection == operatorv1.SidecarEnabled { + return true + } + return false +} + +// IstioRequiresPolicySync reports whether an Istio CR is active in a way +// that requires policySyncPathPrefix to be set. The L7 ambient waypoint +// resources (l7-collector sidecar + EnvoyFilter) are rendered when the +// installation variant is Enterprise; this predicate mirrors that gate so +// the FelixConfiguration field tracks the renderer. +func IstioRequiresPolicySync(istio *operatorv1.Istio, variant operatorv1.ProductVariant) bool { + return istio != nil && variant.IsEnterprise() +} + +// DesiredPolicySyncPathPrefix returns the value FelixConfiguration's +// policySyncPathPrefix should hold given the currently set value and +// whether either the applicationlayer or istio controllers need it. +// +// - A non-empty existing value that does not match the operator-managed +// default is treated as a customer override and preserved verbatim. +// - If either controller needs the field, the operator-managed default +// is returned. +// - Otherwise the field is cleared. +// +// Both the applicationlayer and istio controllers call this from their +// set and cleanup paths to keep coordination explicit and symmetric. +func DesiredPolicySyncPathPrefix(existing string, alNeeds, istioNeeds bool) string { + if existing != "" && existing != DefaultPolicySyncPrefix { + return existing + } + if alNeeds || istioNeeds { + return DefaultPolicySyncPrefix + } + return "" +} diff --git a/pkg/controller/utils/policy_sync_test.go b/pkg/controller/utils/policy_sync_test.go new file mode 100644 index 0000000000..90f2499762 --- /dev/null +++ b/pkg/controller/utils/policy_sync_test.go @@ -0,0 +1,99 @@ +// Copyright (c) 2026 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package utils_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + operatorv1 "github.com/tigera/operator/api/v1" + "github.com/tigera/operator/pkg/controller/utils" +) + +var _ = Describe("policySyncPathPrefix coordination predicates", func() { + Describe("ApplicationLayerRequiresPolicySync", func() { + It("returns false for a nil receiver", func() { + Expect(utils.ApplicationLayerRequiresPolicySync(nil)).To(BeFalse()) + }) + + It("returns false when no feature is enabled", func() { + Expect(utils.ApplicationLayerRequiresPolicySync(&operatorv1.ApplicationLayer{})).To(BeFalse()) + }) + + It("returns true when LogCollection is enabled", func() { + enabled := operatorv1.L7LogCollectionEnabled + al := &operatorv1.ApplicationLayer{ + Spec: operatorv1.ApplicationLayerSpec{ + LogCollection: &operatorv1.LogCollectionSpec{CollectLogs: &enabled}, + }, + } + Expect(utils.ApplicationLayerRequiresPolicySync(al)).To(BeTrue()) + }) + + It("returns true when WAF is enabled", func() { + enabled := operatorv1.WAFEnabled + Expect(utils.ApplicationLayerRequiresPolicySync(&operatorv1.ApplicationLayer{ + Spec: operatorv1.ApplicationLayerSpec{WebApplicationFirewall: &enabled}, + })).To(BeTrue()) + }) + + It("returns true when ApplicationLayerPolicy is enabled", func() { + enabled := operatorv1.ApplicationLayerPolicyEnabled + Expect(utils.ApplicationLayerRequiresPolicySync(&operatorv1.ApplicationLayer{ + Spec: operatorv1.ApplicationLayerSpec{ApplicationLayerPolicy: &enabled}, + })).To(BeTrue()) + }) + + It("returns true when SidecarInjection is enabled", func() { + enabled := operatorv1.SidecarEnabled + Expect(utils.ApplicationLayerRequiresPolicySync(&operatorv1.ApplicationLayer{ + Spec: operatorv1.ApplicationLayerSpec{SidecarInjection: &enabled}, + })).To(BeTrue()) + }) + }) + + Describe("IstioRequiresPolicySync", func() { + It("returns false when the Istio CR is absent", func() { + Expect(utils.IstioRequiresPolicySync(nil, operatorv1.CalicoEnterprise)).To(BeFalse()) + }) + + It("returns false on a non-Enterprise variant", func() { + Expect(utils.IstioRequiresPolicySync(&operatorv1.Istio{}, operatorv1.Calico)).To(BeFalse()) + }) + + It("returns true when an Istio CR is present on Enterprise", func() { + Expect(utils.IstioRequiresPolicySync(&operatorv1.Istio{}, operatorv1.CalicoEnterprise)).To(BeTrue()) + }) + }) + + Describe("DesiredPolicySyncPathPrefix", func() { + It("preserves a customer override regardless of need flags", func() { + Expect(utils.DesiredPolicySyncPathPrefix("/var/run/customer", false, false)).To(Equal("/var/run/customer")) + Expect(utils.DesiredPolicySyncPathPrefix("/var/run/customer", true, true)).To(Equal("/var/run/customer")) + }) + + It("returns the operator default when either side needs it", func() { + Expect(utils.DesiredPolicySyncPathPrefix("", true, false)).To(Equal("/var/run/nodeagent")) + Expect(utils.DesiredPolicySyncPathPrefix("", false, true)).To(Equal("/var/run/nodeagent")) + }) + + It("clears when neither side needs it and there is no override", func() { + Expect(utils.DesiredPolicySyncPathPrefix("", false, false)).To(Equal("")) + // The operator-managed default is treated as operator-owned, not + // as a customer override — so it gets cleared in this case. + Expect(utils.DesiredPolicySyncPathPrefix("/var/run/nodeagent", false, false)).To(Equal("")) + }) + }) +}) diff --git a/pkg/controller/utils/utils.go b/pkg/controller/utils/utils.go index 0325af56f8..2eb9084e07 100644 --- a/pkg/controller/utils/utils.go +++ b/pkg/controller/utils/utils.go @@ -463,6 +463,21 @@ func GetApplicationLayer(ctx context.Context, c client.Client) (*operatorv1.Appl return applicationLayer, nil } +// Return the Istio CR if present. No error is returned if it was not found. +func GetIstio(ctx context.Context, c client.Client) (*operatorv1.Istio, error) { + istio := &operatorv1.Istio{} + + err := c.Get(ctx, DefaultInstanceKey, istio) + if err != nil { + if errors.IsNotFound(err) { + return nil, nil + } + return nil, err + } + + return istio, nil +} + // Return the ManagementCluster CR if present. No error is returned if it was not found. func GetManagementCluster(ctx context.Context, c client.Client) (*operatorv1.ManagementCluster, error) { managementCluster := &operatorv1.ManagementCluster{}