EV-6388: L7 logging through Istio Waypoint Proxy

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) ||

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"))
 		})
 

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})
@@ -265,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]
@@ -290,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
@@ -324,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) {