fix: Ensure IAR and KAC ImagePullSecrets are updated during reconciles

internal/controller/admission/falconadmission_controller.go

@@ -225,7 +225,8 @@ func (r *FalconAdmissionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		}
 	}
 
-	if err := r.reconcileServiceAccount(ctx, req, log, falconAdmission); err != nil {
+	serviceAccountUpdateRequiresRestart, err := r.reconcileServiceAccount(ctx, req, log, falconAdmission)
+	if err != nil {
 		return ctrl.Result{}, err
 	}
 
@@ -291,7 +292,7 @@ func (r *FalconAdmissionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return ctrl.Result{RequeueAfter: 5 * time.Second}, nil
 	}
 
-	if configUpdated || clusterNameConfigUpdated || serviceUpdated || webhookUpdated {
+	if configUpdated || clusterNameConfigUpdated || serviceUpdated || webhookUpdated || serviceAccountUpdateRequiresRestart {
 		err = r.admissionDeploymentUpdate(ctx, req, log, falconAdmission)
 		if err != nil {
 			return ctrl.Result{}, err

internal/controller/admission/falconadmission_controller_test.go

@@ -19,6 +19,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
@@ -73,6 +74,10 @@ var _ = Describe("FalconAdmission controller", func() {
 			Expect(err).To(Not(HaveOccurred()))
 
 			falconAdmission = &falconv1alpha1.FalconAdmission{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "falcon.crowdstrike.com/v1alpha1",
+					Kind:       "FalconAdmission",
+				},
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      controllerName,
 					Namespace: namespaceName,
@@ -492,5 +497,346 @@ var _ = Describe("FalconAdmission controller", func() {
 			Expect(falconClientContainer.Resources.Requests.Memory().String()).To(Equal("128Mi"))
 			Expect(falconClientContainer.Resources.Limits.Memory().String()).To(Equal("128Mi"))
 		})
+
+		// Testing reconcileServiceAccount return value
+		It("should return false when creating a new service account", func() {
+			log := zap.New(zap.UseDevMode(true))
+
+			By("Creating the custom resource for the Kind FalconAdmission")
+			err := k8sClient.Create(ctx, falconAdmission)
+			Expect(err).To(Not(HaveOccurred()))
+
+			By("Checking if the custom resource was successfully created")
+			Eventually(func() error {
+				found := &falconv1alpha1.FalconAdmission{}
+				return k8sClient.Get(ctx, admissionNamespacedName, found)
+			}, 20*time.Second, time.Second).Should(Succeed())
+
+			By("Creating the reconciler")
+			falconAdmissionReconciler := &FalconAdmissionReconciler{
+				Client: k8sClient,
+				Reader: k8sReader,
+				Scheme: k8sClient.Scheme(),
+			}
+
+			By("Calling reconcileServiceAccount when no service account exists")
+			updated, err := falconAdmissionReconciler.reconcileServiceAccount(ctx, reconcile.Request{
+				NamespacedName: admissionNamespacedName,
+			}, log, falconAdmission)
+
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(updated).To(BeFalse(), "reconcileServiceAccount should return false when creating a new service account")
+
+			By("Verifying the service account was created")
+			Eventually(func() error {
+				found := &corev1.ServiceAccount{}
+				return k8sClient.Get(ctx, types.NamespacedName{
+					Name:      common.AdmissionServiceAccountName,
+					Namespace: namespaceName,
+				}, found)
+			}, 5*time.Second, time.Second).Should(Succeed())
+		})
+
+		It("should return false when only annotations are updated", func() {
+			log := zap.New(zap.UseDevMode(true))
+
+			By("Creating the custom resource for the Kind FalconAdmission")
+			err := k8sClient.Create(ctx, falconAdmission)
+			Expect(err).To(Not(HaveOccurred()))
+
+			By("Checking if the custom resource was successfully created")
+			Eventually(func() error {
+				found := &falconv1alpha1.FalconAdmission{}
+				return k8sClient.Get(ctx, admissionNamespacedName, found)
+			}, 20*time.Second, time.Second).Should(Succeed())
+
+			By("Creating the reconciler")
+			falconAdmissionReconciler := &FalconAdmissionReconciler{
+				Client: k8sClient,
+				Reader: k8sReader,
+				Scheme: k8sClient.Scheme(),
+			}
+
+			By("Creating initial service account")
+			updated, err := falconAdmissionReconciler.reconcileServiceAccount(ctx, reconcile.Request{
+				NamespacedName: admissionNamespacedName,
+			}, log, falconAdmission)
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(updated).To(BeFalse())
+
+			By("Updating annotations in FalconAdmission spec")
+			falconAdmission.Spec.AdmissionConfig.ServiceAccount.Annotations = map[string]string{
+				"new-annotation": "new-value",
+			}
+
+			By("Calling reconcileServiceAccount with updated annotations")
+			updated, err = falconAdmissionReconciler.reconcileServiceAccount(ctx, reconcile.Request{
+				NamespacedName: admissionNamespacedName,
+			}, log, falconAdmission)
+
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(updated).To(BeFalse(), "reconcileServiceAccount should return false when only annotations are updated")
+
+			By("Verifying annotations were updated")
+			Eventually(func() map[string]string {
+				found := &corev1.ServiceAccount{}
+				_ = k8sClient.Get(ctx, types.NamespacedName{
+					Name:      common.AdmissionServiceAccountName,
+					Namespace: namespaceName,
+				}, found)
+				return found.ObjectMeta.Annotations
+			}, 5*time.Second, time.Second).Should(HaveKey("new-annotation"))
+		})
+
+		It("should return true when imagePullSecrets are updated", func() {
+			log := zap.New(zap.UseDevMode(true))
+
+			By("Creating the custom resource for the Kind FalconAdmission")
+			err := k8sClient.Create(ctx, falconAdmission)
+			Expect(err).To(Not(HaveOccurred()))
+
+			By("Checking if the custom resource was successfully created")
+			Eventually(func() error {
+				found := &falconv1alpha1.FalconAdmission{}
+				return k8sClient.Get(ctx, admissionNamespacedName, found)
+			}, 20*time.Second, time.Second).Should(Succeed())
+
+			By("Creating the reconciler")
+			falconAdmissionReconciler := &FalconAdmissionReconciler{
+				Client: k8sClient,
+				Reader: k8sReader,
+				Scheme: k8sClient.Scheme(),
+			}
+
+			By("Creating initial service account with default imagePullSecrets")
+			updated, err := falconAdmissionReconciler.reconcileServiceAccount(ctx, reconcile.Request{
+				NamespacedName: admissionNamespacedName,
+			}, log, falconAdmission)
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(updated).To(BeFalse())
+
+			By("Verifying initial imagePullSecrets")
+			Eventually(func() []corev1.LocalObjectReference {
+				found := &corev1.ServiceAccount{}
+				_ = k8sClient.Get(ctx, types.NamespacedName{
+					Name:      common.AdmissionServiceAccountName,
+					Namespace: namespaceName,
+				}, found)
+				return found.ImagePullSecrets
+			}, 5*time.Second, time.Second).Should(Equal([]corev1.LocalObjectReference{
+				{Name: common.FalconPullSecretName},
+			}))
+
+			By("Adding a new imagePullSecret to FalconAdmission spec")
+			falconAdmission.Spec.AdmissionConfig.ImagePullSecrets = []corev1.LocalObjectReference{
+				{Name: "additional-secret"},
+			}
+
+			By("Calling reconcileServiceAccount with updated imagePullSecrets")
+			updated, err = falconAdmissionReconciler.reconcileServiceAccount(ctx, reconcile.Request{
+				NamespacedName: admissionNamespacedName,
+			}, log, falconAdmission)
+
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(updated).To(BeTrue(), "reconcileServiceAccount should return true when imagePullSecrets are updated")
+
+			By("Verifying imagePullSecrets were updated")
+			Eventually(func() []corev1.LocalObjectReference {
+				found := &corev1.ServiceAccount{}
+				_ = k8sClient.Get(ctx, types.NamespacedName{
+					Name:      common.AdmissionServiceAccountName,
+					Namespace: namespaceName,
+				}, found)
+				return found.ImagePullSecrets
+			}, 5*time.Second, time.Second).Should(Equal([]corev1.LocalObjectReference{
+				{Name: common.FalconPullSecretName},
+				{Name: "additional-secret"},
+			}))
+		})
+
+		It("should return false when no changes are made to service account", func() {
+			log := zap.New(zap.UseDevMode(true))
+
+			By("Creating the custom resource for the Kind FalconAdmission")
+			err := k8sClient.Create(ctx, falconAdmission)
+			Expect(err).To(Not(HaveOccurred()))
+
+			By("Checking if the custom resource was successfully created")
+			Eventually(func() error {
+				found := &falconv1alpha1.FalconAdmission{}
+				return k8sClient.Get(ctx, admissionNamespacedName, found)
+			}, 20*time.Second, time.Second).Should(Succeed())
+
+			By("Creating the reconciler")
+			falconAdmissionReconciler := &FalconAdmissionReconciler{
+				Client: k8sClient,
+				Reader: k8sReader,
+				Scheme: k8sClient.Scheme(),
+			}
+
+			By("Creating initial service account")
+			updated, err := falconAdmissionReconciler.reconcileServiceAccount(ctx, reconcile.Request{
+				NamespacedName: admissionNamespacedName,
+			}, log, falconAdmission)
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(updated).To(BeFalse())
+
+			By("Calling reconcileServiceAccount again without any changes")
+			updated, err = falconAdmissionReconciler.reconcileServiceAccount(ctx, reconcile.Request{
+				NamespacedName: admissionNamespacedName,
+			}, log, falconAdmission)
+
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(updated).To(BeFalse(), "reconcileServiceAccount should return false when no changes are made")
+		})
+
+		It("should return true when imagePullSecrets are removed", func() {
+			log := zap.New(zap.UseDevMode(true))
+
+			By("Creating the custom resource for the Kind FalconAdmission with extra imagePullSecret")
+			falconAdmission.Spec.AdmissionConfig.ImagePullSecrets = []corev1.LocalObjectReference{
+				{Name: "extra-secret"},
+			}
+			err := k8sClient.Create(ctx, falconAdmission)
+			Expect(err).To(Not(HaveOccurred()))
+
+			By("Checking if the custom resource was successfully created")
+			Eventually(func() error {
+				found := &falconv1alpha1.FalconAdmission{}
+				return k8sClient.Get(ctx, admissionNamespacedName, found)
+			}, 20*time.Second, time.Second).Should(Succeed())
+
+			By("Creating the reconciler")
+			falconAdmissionReconciler := &FalconAdmissionReconciler{
+				Client: k8sClient,
+				Reader: k8sReader,
+				Scheme: k8sClient.Scheme(),
+			}
+
+			By("Creating initial service account with multiple imagePullSecrets")
+			updated, err := falconAdmissionReconciler.reconcileServiceAccount(ctx, reconcile.Request{
+				NamespacedName: admissionNamespacedName,
+			}, log, falconAdmission)
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(updated).To(BeFalse())
+
+			By("Verifying initial imagePullSecrets include extra secret")
+			Eventually(func() int {
+				found := &corev1.ServiceAccount{}
+				_ = k8sClient.Get(ctx, types.NamespacedName{
+					Name:      common.AdmissionServiceAccountName,
+					Namespace: namespaceName,
+				}, found)
+				return len(found.ImagePullSecrets)
+			}, 5*time.Second, time.Second).Should(Equal(2))
+
+			By("Removing extra imagePullSecret from FalconAdmission spec")
+			falconAdmission.Spec.AdmissionConfig.ImagePullSecrets = []corev1.LocalObjectReference{}
+
+			By("Calling reconcileServiceAccount with removed imagePullSecret")
+			updated, err = falconAdmissionReconciler.reconcileServiceAccount(ctx, reconcile.Request{
+				NamespacedName: admissionNamespacedName,
+			}, log, falconAdmission)
+
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(updated).To(BeTrue(), "reconcileServiceAccount should return true when imagePullSecrets are removed")
+
+			By("Verifying imagePullSecrets were reduced to default only")
+			Eventually(func() []corev1.LocalObjectReference {
+				found := &corev1.ServiceAccount{}
+				_ = k8sClient.Get(ctx, types.NamespacedName{
+					Name:      common.AdmissionServiceAccountName,
+					Namespace: namespaceName,
+				}, found)
+				return found.ImagePullSecrets
+			}, 5*time.Second, time.Second).Should(Equal([]corev1.LocalObjectReference{
+				{Name: common.FalconPullSecretName},
+			}))
+		})
+
+		It("should not trigger reconciliation loop when external annotations are added", func() {
+			log := zap.New(zap.UseDevMode(true))
+
+			By("Creating the custom resource for the Kind FalconAdmission with operator-managed annotations")
+			falconAdmission.Spec.AdmissionConfig.ServiceAccount.Annotations = map[string]string{
+				"operator-managed-annotation": "value1",
+			}
+			err := k8sClient.Create(ctx, falconAdmission)
+			Expect(err).To(Not(HaveOccurred()))
+
+			By("Checking if the custom resource was successfully created")
+			Eventually(func() error {
+				found := &falconv1alpha1.FalconAdmission{}
+				return k8sClient.Get(ctx, admissionNamespacedName, found)
+			}, 20*time.Second, time.Second).Should(Succeed())
+
+			By("Creating the reconciler")
+			falconAdmissionReconciler := &FalconAdmissionReconciler{
+				Client: k8sClient,
+				Reader: k8sReader,
+				Scheme: k8sClient.Scheme(),
+			}
+
+			By("Creating initial service account with operator-managed annotations")
+			updated, err := falconAdmissionReconciler.reconcileServiceAccount(ctx, reconcile.Request{
+				NamespacedName: admissionNamespacedName,
+			}, log, falconAdmission)
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(updated).To(BeFalse())
+
+			By("Verifying service account was created with operator-managed annotation")
+			Eventually(func() map[string]string {
+				found := &corev1.ServiceAccount{}
+				_ = k8sClient.Get(ctx, types.NamespacedName{
+					Name:      common.AdmissionServiceAccountName,
+					Namespace: namespaceName,
+				}, found)
+				return found.Annotations
+			}, 5*time.Second, time.Second).Should(HaveKeyWithValue("operator-managed-annotation", "value1"))
+
+			By("Simulating external system (like OpenShift) adding annotations to the service account")
+			serviceAccount := &corev1.ServiceAccount{}
+			err = k8sClient.Get(ctx, types.NamespacedName{
+				Name:      common.AdmissionServiceAccountName,
+				Namespace: namespaceName,
+			}, serviceAccount)
+			Expect(err).To(Not(HaveOccurred()))
+
+			serviceAccount.Annotations["openshift.io/sa.scc.mcs"] = "s0:c27,c4"
+			serviceAccount.Annotations["openshift.io/sa.scc.supplemental-groups"] = "1000710000/10000"
+			serviceAccount.Annotations["openshift.io/sa.scc.uid-range"] = "1000710000/10000"
+			err = k8sClient.Update(ctx, serviceAccount)
+			Expect(err).To(Not(HaveOccurred()))
+
+			By("Verifying external annotations were added")
+			Eventually(func() bool {
+				found := &corev1.ServiceAccount{}
+				_ = k8sClient.Get(ctx, types.NamespacedName{
+					Name:      common.AdmissionServiceAccountName,
+					Namespace: namespaceName,
+				}, found)
+				return len(found.Annotations) == 4 // 1 operator-managed + 3 openshift
+			}, 5*time.Second, time.Second).Should(BeTrue())
+
+			By("Calling reconcileServiceAccount again - should not trigger update for external annotations")
+			updated, err = falconAdmissionReconciler.reconcileServiceAccount(ctx, reconcile.Request{
+				NamespacedName: admissionNamespacedName,
+			}, log, falconAdmission)
+
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(updated).To(BeFalse(), "reconcileServiceAccount should return false when only external annotations exist")
+
+			By("Verifying external annotations are preserved after reconciliation")
+			serviceAccount = &corev1.ServiceAccount{}
+			err = k8sClient.Get(ctx, types.NamespacedName{
+				Name:      common.AdmissionServiceAccountName,
+				Namespace: namespaceName,
+			}, serviceAccount)
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(serviceAccount.Annotations).To(HaveKeyWithValue("openshift.io/sa.scc.mcs", "s0:c27,c4"))
+			Expect(serviceAccount.Annotations).To(HaveKeyWithValue("openshift.io/sa.scc.supplemental-groups", "1000710000/10000"))
+			Expect(serviceAccount.Annotations).To(HaveKeyWithValue("openshift.io/sa.scc.uid-range", "1000710000/10000"))
+			Expect(serviceAccount.Annotations).To(HaveKeyWithValue("operator-managed-annotation", "value1"))
+		})
 	})
 })