Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SLK-78181 - Fix nil pointer dereference starboard #235

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Set up Go
uses: actions/setup-go@v1
uses: actions/setup-go@v4
with:
go-version: 1.19
go-version: '1.21'
id: go
- name: Set up docker
run: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
32 changes: 6 additions & 26 deletions controllers/operator/aquacsp/aquacsp_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
22 changes: 8 additions & 14 deletions controllers/operator/aqualightning/aquaLightningHelper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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
}

Expand Down
64 changes: 18 additions & 46 deletions controllers/operator/aqualightning/aqualightning_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
Loading