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.

Sign up for GitHub

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

Fix #59: Fix CEL bug #72

Merged
merged 1 commit into from
Feb 26, 2024
Merged

Fix #59: Fix CEL bug #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 25 additions & 16 deletions internal/controller/securityintentbinding_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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 {
Expand Down
6 changes: 2 additions & 4 deletions pkg/adapter/nimbus-kubearmor/manager/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/processor/policybuilder/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading