Merge pull request #72 from seungsoo-lee/fix_cel_2

Fix #59: Fix CEL bug
5GSEC · Feb 26, 2024 · fcd5679 · fcd5679
2 parents 703a154 + 9960029
commit fcd5679
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 21 deletions.
diff --git a/internal/controller/securityintentbinding_controller.go b/internal/controller/securityintentbinding_controller.go
@@ -54,8 +54,10 @@ func (r *SecurityIntentBindingReconciler) Reconcile(ctx context.Context, req ctr
 	}
 	logger.Info("SecurityIntentBinding found", "SecurityIntentBinding.Name", req.Name, "SecurityIntentBinding.Namespace", req.Namespace)
 
-	if err = r.createOrUpdateNp(ctx, logger, req); err != nil {
+	if np, err := r.createOrUpdateNp(ctx, logger, req); err != nil {
 		return requeueWithError(err)
+	} else if np == nil {
+		return doNotRequeue()
 	}
 
 	if err = r.updateStatus(ctx, logger, req); err != nil {
@@ -101,14 +103,13 @@ func (r *SecurityIntentBindingReconciler) deleteFn(deleteEvent event.DeleteEvent
 	return ownerExists(r.Client, obj)
 }
 
-func (r *SecurityIntentBindingReconciler) createOrUpdateNp(ctx context.Context, logger logr.Logger, req ctrl.Request) error {
-	// Always fetch the latest CRs so that we have the latest state of the CRs on the
+func (r *SecurityIntentBindingReconciler) createOrUpdateNp(ctx context.Context, logger logr.Logger, req ctrl.Request) (*v1.NimbusPolicy, error) { // Always fetch the latest CRs so that we have the latest state of the CRs on the
 	// cluster.
 
 	var sib v1.SecurityIntentBinding
 	if err := r.Get(ctx, req.NamespacedName, &sib); err != nil {
 		logger.Error(err, "failed to fetch SecurityIntentBinding", "SecurityIntentBinding.Name", req.Name, "SecurityIntentBinding.Namespace", req.Namespace)
-		return err
+		return nil, err
 	}
 
 	var np v1.NimbusPolicy
@@ -118,38 +119,42 @@ func (r *SecurityIntentBindingReconciler) createOrUpdateNp(ctx context.Context,
 			return r.createNp(ctx, logger, sib)
 		}
 		logger.Error(err, "failed to fetch NimbusPolicy", "NimbusPolicy.Name", req.Name, "NimbusPolicy.Namespace", req.Namespace)
-		return err
+		return nil, err
 	}
 	return r.updateNp(ctx, logger, sib)
 }
 
-func (r *SecurityIntentBindingReconciler) createNp(ctx context.Context, logger logr.Logger, sib v1.SecurityIntentBinding) error {
+func (r *SecurityIntentBindingReconciler) createNp(ctx context.Context, logger logr.Logger, sib v1.SecurityIntentBinding) (*v1.NimbusPolicy, error) {
 	nimbusPolicy, err := policybuilder.BuildNimbusPolicy(ctx, logger, r.Client, r.Scheme, sib)
 	// TODO: Improve error handling for CEL
 	if err != nil {
 		// If error is caused due to CEL then we don't retry to build NimbusPolicy.
 		if strings.Contains(err.Error(), "error processing CEL") {
 			logger.Error(err, "failed to build NimbusPolicy")
-			return nil
+			return nil, nil
 		}
 		logger.Error(err, "failed to build NimbusPolicy")
-		return err
+		return nil, err
+	}
+	if nimbusPolicy == nil {
+		logger.Info("Abort NimbusPolicy creation as no labels matched the CEL expressions")
+		return nil, nil
 	}
 
 	if err := r.Create(ctx, nimbusPolicy); err != nil {
 		logger.Error(err, "failed to create NimbusPolicy", "NimbusPolicy.Name", nimbusPolicy.Name, "NimbusPolicy.Namespace", nimbusPolicy.Namespace)
-		return err
+		return nil, err
 	}
 	logger.Info("NimbusPolicy created", "NimbusPolicy.Name", nimbusPolicy.Name, "NimbusPolicy.Namespace", nimbusPolicy.Namespace)
 
-	return nil
+	return nimbusPolicy, nil
 }
 
-func (r *SecurityIntentBindingReconciler) updateNp(ctx context.Context, logger logr.Logger, sib v1.SecurityIntentBinding) error {
+func (r *SecurityIntentBindingReconciler) updateNp(ctx context.Context, logger logr.Logger, sib v1.SecurityIntentBinding) (*v1.NimbusPolicy, error) {
 	var existingNp v1.NimbusPolicy
 	if err := r.Get(ctx, types.NamespacedName{Name: sib.Name, Namespace: sib.Namespace}, &existingNp); err != nil {
 		logger.Error(err, "failed to fetch NimbusPolicy", "NimbusPolicy.Name", sib.Name, "NimbusPolicy.Namespace", sib.Namespace)
-		return err
+		return nil, err
 	}
 
 	nimbusPolicy, err := policybuilder.BuildNimbusPolicy(ctx, logger, r.Client, r.Scheme, sib)
@@ -158,20 +163,24 @@ func (r *SecurityIntentBindingReconciler) updateNp(ctx context.Context, logger l
 		// If error is caused due to CEL then we don't retry to build NimbusPolicy.
 		if strings.Contains(err.Error(), "error processing CEL") {
 			logger.Error(err, "failed to build NimbusPolicy")
-			return nil
+			return nil, nil
 		}
 		logger.Error(err, "failed to build NimbusPolicy")
-		return err
+		return nil, err
+	}
+	if nimbusPolicy == nil {
+		logger.Info("Abort NimbusPolicy creation as no labels matched the CEL expressions")
+		return nil, nil
 	}
 
 	nimbusPolicy.ObjectMeta.ResourceVersion = existingNp.ObjectMeta.ResourceVersion
 	if err := r.Update(ctx, nimbusPolicy); err != nil {
 		logger.Error(err, "failed to configure NimbusPolicy", "NimbusPolicy.Name", nimbusPolicy.Name, "NimbusPolicy.Namespace", nimbusPolicy.Namespace)
-		return err
+		return nil, err
 	}
 	logger.Info("NimbusPolicy configured", "NimbusPolicy.Name", nimbusPolicy.Name, "NimbusPolicy.Namespace", nimbusPolicy.Namespace)
 
-	return nil
+	return nimbusPolicy, nil
 }
 
 func (r *SecurityIntentBindingReconciler) updateStatus(ctx context.Context, logger logr.Logger, req ctrl.Request) error {

diff --git a/pkg/adapter/nimbus-kubearmor/manager/manager.go b/pkg/adapter/nimbus-kubearmor/manager/manager.go
@@ -180,7 +180,7 @@ func deleteKsp(ctx context.Context, npName, npNamespace string) {
 func deleteUnnecessaryKsps(ctx context.Context, currentKsps []kubearmorv1.KubeArmorPolicy, namespace string, logger logr.Logger) {
 	var existingKsps kubearmorv1.KubeArmorPolicyList
 	if err := k8sClient.List(ctx, &existingKsps, client.InNamespace(namespace)); err != nil {
-		logger.Error(err, "Failed to list KubeArmorPolicies for cleanup")
+		logger.Error(err, "failed to list KubeArmorPolicies for cleanup")
 		return
 	}
 
@@ -193,9 +193,7 @@ func deleteUnnecessaryKsps(ctx context.Context, currentKsps []kubearmorv1.KubeAr
 		existingKsp := existingKsps.Items[i]
 		if _, needed := currentKspNames[existingKsp.Name]; !needed {
 			if err := k8sClient.Delete(ctx, &existingKsp); err != nil {
-				logger.Error(err, "Failed to delete unnecessary KubeArmorPolicy", "KubeArmorPolicy.Name", existingKsp.Name)
-			} else {
-				logger.Info("Deleted unnecessary KubeArmorPolicy", "KubeArmorPolicy.Name", existingKsp.Name)
+				logger.Error(err, "failed to delete unnecessary KubeArmorPolicy", "KubeArmorPolicy.Name", existingKsp.Name)
 			}
 		}
 	}

diff --git a/pkg/processor/policybuilder/common.go b/pkg/processor/policybuilder/common.go
@@ -65,7 +65,7 @@ func ProcessCEL(ctx context.Context, k8sClient client.Client, namespace string,
 				"labels": resource["labels"],
 			})
 			if err != nil {
-				logger.Error(err, "Error evaluating CEL expression for pod", "PodName", pod.Name)
+				logger.Info("Error evaluating CEL expression for pod", "PodName", pod.Name, "error", err.Error())
 				// Instead of returning an error immediately, we log the error and continue.
 				break
 			}