SLK-78181 - Fix nil pointer dereference starboard

aquasecurity · Feb 28, 2024 · c594dc8 · c594dc8
1 parent 61cf816
commit c594dc8
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 111 deletions.
diff --git a/controllers/aquasecurity/aquastarboard/aquastarboard_controller.go b/controllers/aquasecurity/aquastarboard/aquastarboard_controller.go
@@ -352,8 +352,9 @@ func (r *AquaStarboardReconciler) addStarboardRole(ro *aquasecurityv1alpha1.Aqua
 		reqLogger.Info("Aqua Starboard: Creating a New Role", "Role.Namespace", role.Namespace, "Role.Name", role.Name)
 		err = r.Client.Create(context.TODO(), role)
 		if err != nil {
-			return reconcile.Result{Requeue: true}, err
+			return reconcile.Result{Requeue: true}, nil
 		}
+
 		return reconcile.Result{}, nil
 	} else if err != nil {
 		return reconcile.Result{}, err
@@ -367,19 +368,20 @@ func (r *AquaStarboardReconciler) addStarboardRole(ro *aquasecurityv1alpha1.Aqua
 	}
 
 	if !equal {
-		found.Rules = role.Rules // Update the existing Role's rules
-		reqLogger.Info("Aqua Starboard: Updating Role", "Role.Namespace", found.Namespace, "Role.Name", found.Name)
+		found = role
+		log.Info("Aqua Starboard: Updating Role", "Role.Namespace", found.Namespace, "Role.Name", found.Name)
 		err := r.Client.Update(context.TODO(), found)
 		if err != nil {
-			reqLogger.Error(err, "Failed to update Role", "Role.Namespace", found.Namespace, "Role.Name", found.Name)
+			log.Error(err, "Failed to update Role", "Role.Namespace", found.Namespace, "Role.Name", found.Name)
 			return reconcile.Result{}, err
 		}
+
 		return reconcile.Result{Requeue: true}, nil
 	}
 
-	// Role already exists and is up-to-date - don't requeue
+	// Role already exists - don't requeue
 	reqLogger.Info("Skip reconcile: Aqua Role Exists", "Role.Namespace", found.Namespace, "Role.Name", found.Name)
-	return reconcile.Result{}, nil
+	return reconcile.Result{Requeue: true}, nil
 }
 
 func (r *AquaStarboardReconciler) createAquaStarboardServiceAccount(cr *aquasecurityv1alpha1.AquaStarboard) (reconcile.Result, error) {

diff --git a/controllers/operator/aquacsp/aquacsp_controller.go b/controllers/operator/aquacsp/aquacsp_controller.go
@@ -672,36 +672,16 @@ func (r *AquaCspReconciler) InstallAquaKubeEnforcer(cr *v1alpha1.AquaCsp) (recon
 
 		reqLogger.Info("Checking for AquaKubeEnforcer Upgrade", "kube-enforcer", enforcer.Spec, "found", found.Spec, "update bool", update)
 		if update {
-			// Retry loop with backoff
-			retryCount := 0
-			maxRetries := 3
-			retryDelay := time.Second * 5
-
-			for {
-				// Increment retry count
-				retryCount++
-
-				// Attempt to update AquaKubeEnforcer
-				err = r.Client.Update(context.Background(), found)
-				if err == nil {
-					// Update successful, break out of the loop
-					break
-				}
-
-				// Check if maximum retries reached
-				if retryCount >= maxRetries {
-					reqLogger.Error(err, "Max retries reached. Failed to update AquaKubeEnforcer.")
-					return reconcile.Result{}, err
-				}
-
-				// Log the error and retry after delay
-				reqLogger.Info("Error updating AquaKubeEnforcer. Retrying...", "RetryCount", retryCount, "MaxRetries", maxRetries)
-				time.Sleep(retryDelay)
+			found.Spec = *(enforcer.Spec.DeepCopy())
+			err = r.Client.Update(context.Background(), found)
+			if err != nil {
+				reqLogger.Error(err, "Aqua CSP: Failed to update AquaKubeEnforcer.", "Deployment.Namespace", found.Namespace, "Deployment.Name", found.Name)
+				return reconcile.Result{}, err
 			}
-
 			// Spec updated - return and requeue
 			return reconcile.Result{Requeue: true}, nil
 		}
+
 	}
 
 	reqLogger.Info("Skip reconcile: Aqua KubeEnforcer Exists", "AquaKubeEnforcer.Namespace", found.Namespace, "AquaKubeEnforcer.Name", found.Name)

diff --git a/controllers/operator/aquakubeenforcer/aquaKubeEnforcerHelper.go b/controllers/operator/aquakubeenforcer/aquaKubeEnforcerHelper.go
@@ -977,12 +977,6 @@ func (ebf *AquaKubeEnforcerHelper) getEnvVars(cr *operatorv1alpha1.AquaKubeEnfor
 // Starboard functions
 
 func (ebf *AquaKubeEnforcerHelper) newStarboard(cr *operatorv1alpha1.AquaKubeEnforcer) *v1alpha1.AquaStarboard {
-	if cr == nil {
-		log.Error(fmt.Errorf("AquaKubeEnforcer object is nil"), "AquaKubeEnforcer object is nil")
-		return nil
-	}
-
-	log.Info("Creating new AquaStarboard instance")
 
 	_, registry, repository, tag := extra.GetImageData("kube-enforcer", cr.Spec.Infrastructure.Version, cr.Spec.KubeEnforcerService.ImageData, cr.Spec.AllowAnyVersion)
 

diff --git a/controllers/operator/aquakubeenforcer/aquakubeenforcer_controller.go b/controllers/operator/aquakubeenforcer/aquakubeenforcer_controller.go
@@ -105,6 +105,7 @@ func (r *AquaKubeEnforcerReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		reqLogger.Error(syserrors.New("Unable to create KubeEnforcer Certificates"), "Unable to create KubeEnforcer Certificates")
 		return reconcile.Result{}, nil
 	}
+
 	// Fetch the AquaKubeEnforcer instance
 	instance := &operatorv1alpha1.AquaKubeEnforcer{}
 	err := r.Client.Get(context.TODO(), req.NamespacedName, instance)
@@ -196,6 +197,7 @@ func (r *AquaKubeEnforcerReconciler) Reconcile(ctx context.Context, req ctrl.Req
 
 	instance.Spec.KubeEnforcerService = r.updateKubeEnforcerServerObject(instance.Spec.KubeEnforcerService, instance.Spec.ImageData)
 
+	sbSpec := instance.DeepCopy()
 	_, err = r.addKEClusterRoleBinding(instance)
 	if err != nil {
 		return reconcile.Result{}, err
@@ -247,7 +249,7 @@ func (r *AquaKubeEnforcerReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	}
 
 	if instance.Spec.DeployStarboard != nil {
-		r.installAquaStarboard(instance)
+		r.installAquaStarboard(sbSpec)
 	}
 
 	return ctrl.Result{}, nil
@@ -430,17 +432,10 @@ func (r *AquaKubeEnforcerReconciler) updateKubeEnforcerObject(cr *operatorv1alph
 func (r *AquaKubeEnforcerReconciler) addKEDeployment(cr *operatorv1alpha1.AquaKubeEnforcer) (reconcile.Result, error) {
 	reqLogger := log.WithValues("KubeEnforcer Deployment Phase", "Create Deployment")
 	reqLogger.Info("Start creating deployment")
-
 	pullPolicy, registry, repository, tag := extra.GetImageData("kube-enforcer", cr.Spec.Infrastructure.Version, cr.Spec.KubeEnforcerService.ImageData, cr.Spec.AllowAnyVersion)
 
 	enforcerHelper := newAquaKubeEnforcerHelper(cr)
-	deployment := enforcerHelper.CreateKEDeployment(cr,
-		consts.AquaKubeEnforcerClusterRoleBidingName,
-		"ke-deployment",
-		registry,
-		tag,
-		pullPolicy,
-		repository)
+	deployment := enforcerHelper.CreateKEDeployment(cr, consts.AquaKubeEnforcerClusterRoleBidingName, "ke-deployment", registry, tag, pullPolicy, repository)
 
 	// Set AquaKubeEnforcer instance as the owner and controller
 	if err := controllerutil.SetControllerReference(cr, deployment, r.Scheme); err != nil {
@@ -486,10 +481,12 @@ func (r *AquaKubeEnforcerReconciler) addKEDeployment(cr *operatorv1alpha1.AquaKu
 				return reconcile.Result{}, err
 			}
 			// Spec updated - return and requeue
+
 			return reconcile.Result{Requeue: true}, nil
 		} else if update && !updateEnforcerApproved {
 			cr.Status.State = operatorv1alpha1.AquaEnforcerUpdatePendingApproval
 			_ = r.Client.Status().Update(context.Background(), cr)
+
 		} else {
 			currentState := cr.Status.State
 			if !k8s.IsDeploymentReady(found, 1) {
@@ -506,6 +503,7 @@ func (r *AquaKubeEnforcerReconciler) addKEDeployment(cr *operatorv1alpha1.AquaKu
 	}
 
 	// object already exists - don't requeue
+
 	reqLogger.Info("Skip reconcile: Aqua KubeEnforcer Deployment Exists", "Deployment.Namespace", found.Namespace, "Deployment.Name", found.Name)
 	return reconcile.Result{}, nil
 }
@@ -999,10 +997,7 @@ func (r *AquaKubeEnforcerReconciler) addKEService(cr *operatorv1alpha1.AquaKubeE
 	reqLogger.Info("Start creating service")
 
 	enforcerHelper := newAquaKubeEnforcerHelper(cr)
-	service := enforcerHelper.CreateKEService(cr.Name,
-		cr.Namespace,
-		consts.AquaKubeEnforcerClusterRoleBidingName,
-		"ke-service")
+	service := enforcerHelper.CreateKEService(cr.Name, cr.Namespace, consts.AquaKubeEnforcerClusterRoleBidingName, "ke-service")
 
 	// Set AquaKubeEnforcer instance as the owner and controller
 	if err := controllerutil.SetControllerReference(cr, service, r.Scheme); err != nil {

diff --git a/controllers/operator/aqualightning/aquaLightningHelper.go b/controllers/operator/aqualightning/aquaLightningHelper.go
@@ -33,16 +33,6 @@ func newAquaLightningHelper(cr *v1alpha1.AquaLightning) *AquaLightningHelper {
 }
 
 func (lightning *AquaLightningHelper) newAquaKubeEnforcer(cr *v1alpha1.AquaLightning) *v1alpha1.AquaKubeEnforcer {
-	// Step 1: Check if cr or cr.Spec is nil
-	if cr == nil {
-		return nil
-	}
-
-	// Step 2: Check if cr.Spec.KubeEnforcer is nil
-	if cr.Spec.KubeEnforcer == nil {
-		return nil
-	}
-
 	registry := consts.Registry
 	if cr.Spec.KubeEnforcer.RegistryData != nil {
 		if len(cr.Spec.KubeEnforcer.RegistryData.URL) > 0 {
@@ -201,10 +191,6 @@ func (lightning *AquaLightningHelper) newAquaEnforcer(cr *v1alpha1.AquaLightning
 				Port: gatewayPort,
 			},
 			Token: cr.Spec.Enforcer.Token,
-			Secret: &v1alpha1.AquaSecret{
-				Name: cr.Spec.Enforcer.Secret.Name,
-				Key:  cr.Spec.Enforcer.Secret.Key,
-			},
 			EnforcerService: &v1alpha1.AquaService{
 				ImageData: &v1alpha1.AquaImage{
 					Registry:   registry,
@@ -219,6 +205,14 @@ func (lightning *AquaLightningHelper) newAquaEnforcer(cr *v1alpha1.AquaLightning
 		},
 	}
 
+	// Check if Mtls is enabled for enforcer
+	if cr.Spec.Enforcer.Mtls {
+		aquaenf.Spec.Secret = &v1alpha1.AquaSecret{
+			Name: cr.Spec.Enforcer.Secret.Name,
+			Key:  cr.Spec.Enforcer.Secret.Key,
+		}
+	}
+
 	return aquaenf
 }
 

diff --git a/controllers/operator/aqualightning/aqualightning_controller.go b/controllers/operator/aqualightning/aqualightning_controller.go
@@ -24,7 +24,6 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/pem"
-	stderrors "errors"
 	"fmt"
 	"github.com/aquasecurity/aqua-operator/apis/operator/v1alpha1"
 	"github.com/aquasecurity/aqua-operator/controllers/common"
@@ -49,9 +48,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-const maxRetries = 3
-const retryDelay = 1 * time.Second
-
 var log = logf.Log.WithName("controller_aqualightning")
 
 // AquaLightningReconciler reconciles a AquaKubeEnforcer object
@@ -69,29 +65,10 @@ type KubeEnforcerCertificates struct {
 }
 
 func (r *AquaLightningReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	for attempt := 0; attempt < maxRetries; attempt++ {
-		result, err := r.reconcileOnce(ctx, req)
-		if err == nil {
-			return result, nil
-		}
-
-		if errors.IsConflict(err) {
-			// Conflict error encountered, retry after delay
-			time.Sleep(retryDelay)
-			continue
-		}
-
-		return result, err
-	}
-
-	return reconcile.Result{}, stderrors.New("exhausted max retries")
-}
-
-func (r *AquaLightningReconciler) reconcileOnce(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	reqLogger := log.WithValues("Request.Namespace", req.Namespace, "Request.Name", req.Name)
 	reqLogger.Info("Reconciling AquaLightning")
 
-	// Fetch the AquaCsp instance
+	// Fetch the AquaLightning instance
 	instance := &v1alpha1.AquaLightning{}
 	err := r.Client.Get(context.TODO(), req.NamespacedName, instance)
 	if err != nil {
@@ -120,8 +97,7 @@ func (r *AquaLightningReconciler) reconcileOnce(ctx context.Context, req ctrl.Re
 	waitForEnforcer := true
 	waitForKubeEnforcer := true
 
-	if !reflect.DeepEqual(v1alpha1.AquaDeploymentUpdateInProgress, instance.Status.State) &&
-		(waitForKubeEnforcer || waitForEnforcer) {
+	if !reflect.DeepEqual(v1alpha1.AquaDeploymentUpdateInProgress, instance.Status.State) && (waitForKubeEnforcer || waitForEnforcer) {
 		crStatus := r.WaitForEnforcersReady(instance, waitForEnforcer, waitForKubeEnforcer)
 		if !reflect.DeepEqual(instance.Status.State, crStatus) {
 			instance.Status.State = crStatus
@@ -179,24 +155,24 @@ func (r *AquaLightningReconciler) updateLightningObject(cr *v1alpha1.AquaLightni
 }
 
 func (r *AquaLightningReconciler) InstallAquaKubeEnforcer(cr *v1alpha1.AquaLightning) (reconcile.Result, error) {
-	reqLogger := log.WithValues("CSP - AquaKubeEnforcer Phase", "Install Aqua Enforcer")
+	reqLogger := log.WithValues("Lightning - AquaKubeEnforcer Phase", "Install Aqua Enforcer")
 	reqLogger.Info("Start installing AquaKubeEnforcer")
 
 	// Define a new AquaEnforcer object
 	lightningHelper := newAquaLightningHelper(cr)
-	enforcer := lightningHelper.newAquaKubeEnforcer(cr)
+	kubeEnforcer := lightningHelper.newAquaKubeEnforcer(cr)
 
-	// Set AquaCsp instance as the owner and controller
-	if err := controllerutil.SetControllerReference(cr, enforcer, r.Scheme); err != nil {
+	// Set AquaLightning instance as the owner and controller
+	if err := controllerutil.SetControllerReference(cr, kubeEnforcer, r.Scheme); err != nil {
 		return reconcile.Result{}, err
 	}
 
 	// Check if this AquaKubeEnforcer already exists
 	found := &v1alpha1.AquaKubeEnforcer{}
-	err := r.Client.Get(context.TODO(), types.NamespacedName{Name: enforcer.Name, Namespace: enforcer.Namespace}, found)
+	err := r.Client.Get(context.TODO(), types.NamespacedName{Name: kubeEnforcer.Name, Namespace: kubeEnforcer.Namespace}, found)
 	if err != nil && errors.IsNotFound(err) {
-		reqLogger.Info("Creating a New Aqua KubeEnforcer", "AquaKubeEnforcer.Namespace", enforcer.Namespace, "AquaKubeEnforcer.Name", enforcer.Name)
-		err = r.Client.Create(context.TODO(), enforcer)
+		reqLogger.Info("Creating a New Aqua KubeEnforcer", "AquaKubeEnforcer.Namespace", kubeEnforcer.Namespace, "AquaKubeEnforcer.Name", kubeEnforcer.Name)
+		err = r.Client.Create(context.TODO(), kubeEnforcer)
 		if err != nil {
 			return reconcile.Result{Requeue: true, RequeueAfter: time.Duration(0)}, err
 		}
@@ -208,11 +184,11 @@ func (r *AquaLightningReconciler) InstallAquaKubeEnforcer(cr *v1alpha1.AquaLight
 	// AquaEnforcer already exists - don't requeue
 
 	if found != nil {
-		update := !reflect.DeepEqual(enforcer.Spec, found.Spec)
+		update := !reflect.DeepEqual(kubeEnforcer.Spec, found.Spec)
 
-		reqLogger.Info("Checking for AquaKubeEnforcer Upgrade", "kube-enforcer", enforcer.Spec, "found", found.Spec, "update bool", update)
+		reqLogger.Info("Checking for AquaKubeEnforcer Upgrade", "kube-enforcer", kubeEnforcer.Spec, "found", found.Spec, "update bool", update)
 		if update {
-			found.Spec = *(enforcer.Spec.DeepCopy())
+			found.Spec = *(kubeEnforcer.Spec.DeepCopy())
 			err = r.Client.Update(context.Background(), found)
 			if err != nil {
 				reqLogger.Error(err, "Aqua CSP: Failed to update AquaKubeEnforcer.", "Deployment.Namespace", found.Namespace, "Deployment.Name", found.Name)
@@ -221,7 +197,6 @@ func (r *AquaLightningReconciler) InstallAquaKubeEnforcer(cr *v1alpha1.AquaLight
 			// Spec updated - return and requeue
 			return reconcile.Result{Requeue: true}, nil
 		}
-
 	}
 
 	reqLogger.Info("Skip reconcile: Aqua KubeEnforcer Exists", "AquaKubeEnforcer.Namespace", found.Namespace, "AquaKubeEnforcer.Name", found.Name)
@@ -236,7 +211,7 @@ func (r *AquaLightningReconciler) InstallAquaEnforcer(cr *v1alpha1.AquaLightning
 	lightningHelper := newAquaLightningHelper(cr)
 	enforcer := lightningHelper.newAquaEnforcer(cr)
 
-	// Set AquaCsp instance as the owner and controller
+	// Set AquaLightning instance as the owner and controller
 	if err := controllerutil.SetControllerReference(cr, enforcer, r.Scheme); err != nil {
 		return reconcile.Result{}, err
 	}
@@ -265,7 +240,7 @@ func (r *AquaLightningReconciler) InstallAquaEnforcer(cr *v1alpha1.AquaLightning
 			found.Spec = *(enforcer.Spec.DeepCopy())
 			err = r.Client.Update(context.Background(), found)
 			if err != nil {
-				reqLogger.Error(err, "Aqua CSP: Failed to update AquaEnforcer.", "Deployment.Namespace", found.Namespace, "Deployment.Name", found.Name)
+				reqLogger.Error(err, "Aqua Lightning: Failed to update AquaEnforcer.", "Deployment.Namespace", found.Namespace, "Deployment.Name", found.Name)
 				return reconcile.Result{}, err
 			}
 			// Spec updated - return and requeue
@@ -278,7 +253,7 @@ func (r *AquaLightningReconciler) InstallAquaEnforcer(cr *v1alpha1.AquaLightning
 }
 
 func (r *AquaLightningReconciler) WaitForEnforcersReady(cr *v1alpha1.AquaLightning, validateEnforcer, validateKubeEnforcer bool) v1alpha1.AquaDeploymentState {
-	reqLogger := log.WithValues("CSP - AquaEnforcers Phase", "Wait For Aqua Enforcer and KubeEnforcer")
+	reqLogger := log.WithValues("Lightning - AquaEnforcers Phase", "Wait For Aqua Enforcer and KubeEnforcer")
 	reqLogger.Info("Start waiting to aqua enforcer and kube-enforcer")
 
 	enforcerStatus := v1alpha1.AquaDeploymentStateRunning
@@ -309,14 +284,11 @@ func (r *AquaLightningReconciler) WaitForEnforcersReady(cr *v1alpha1.AquaLightni
 
 	returnStatus := v1alpha1.AquaDeploymentStateRunning
 
-	if reflect.DeepEqual(v1alpha1.AquaDeploymentStatePending, enforcerStatus) ||
-		reflect.DeepEqual(v1alpha1.AquaDeploymentStatePending, kubeEnforcerStatus) {
+	if reflect.DeepEqual(v1alpha1.AquaDeploymentStatePending, enforcerStatus) || reflect.DeepEqual(v1alpha1.AquaDeploymentStatePending, kubeEnforcerStatus) {
 		returnStatus = v1alpha1.AquaEnforcerWaiting
-	} else if reflect.DeepEqual(v1alpha1.AquaEnforcerUpdateInProgress, enforcerStatus) ||
-		reflect.DeepEqual(v1alpha1.AquaEnforcerUpdateInProgress, kubeEnforcerStatus) {
+	} else if reflect.DeepEqual(v1alpha1.AquaEnforcerUpdateInProgress, enforcerStatus) || reflect.DeepEqual(v1alpha1.AquaEnforcerUpdateInProgress, kubeEnforcerStatus) {
 		returnStatus = v1alpha1.AquaEnforcerUpdateInProgress
-	} else if reflect.DeepEqual(v1alpha1.AquaEnforcerUpdatePendingApproval, enforcerStatus) ||
-		reflect.DeepEqual(v1alpha1.AquaEnforcerUpdatePendingApproval, kubeEnforcerStatus) {
+	} else if reflect.DeepEqual(v1alpha1.AquaEnforcerUpdatePendingApproval, enforcerStatus) || reflect.DeepEqual(v1alpha1.AquaEnforcerUpdatePendingApproval, kubeEnforcerStatus) {
 		returnStatus = v1alpha1.AquaEnforcerUpdatePendingApproval
 	}