From 6eb1a5976e083203379b687049bd0e1f90064669 Mon Sep 17 00:00:00 2001 From: Anurag Rajawat Date: Tue, 27 Feb 2024 15:51:50 +0530 Subject: [PATCH 1/3] fix(core): Handle SecurityIntent update and deletion Signed-off-by: Anurag Rajawat --- ...clustersecurityintentbinding_controller.go | 196 ++++++++++++--- .../controller/securityintent_controller.go | 7 +- .../securityintentbinding_controller.go | 233 ++++++++++++++---- internal/controller/util.go | 46 +++- pkg/processor/errors/errors.go | 12 + .../clusternimbuspolicy_builder.go | 5 +- .../policybuilder/nimbuspolicy_builder.go | 4 +- 7 files changed, 412 insertions(+), 91 deletions(-) create mode 100644 pkg/processor/errors/errors.go diff --git a/internal/controller/clustersecurityintentbinding_controller.go b/internal/controller/clustersecurityintentbinding_controller.go index dc7cddc9..46e872df 100644 --- a/internal/controller/clustersecurityintentbinding_controller.go +++ b/internal/controller/clustersecurityintentbinding_controller.go @@ -5,6 +5,7 @@ package controller import ( "context" + "errors" "github.com/go-logr/logr" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -15,10 +16,13 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" v1 "github.com/5GSEC/nimbus/api/v1" + processorerrors "github.com/5GSEC/nimbus/pkg/processor/errors" "github.com/5GSEC/nimbus/pkg/processor/policybuilder" ) @@ -43,21 +47,27 @@ func (r *ClusterSecurityIntentBindingReconciler) Reconcile(ctx context.Context, if err != nil { if apierrors.IsNotFound(err) { logger.Info("ClusterSecurityIntentBinding not found. Ignoring since object must be deleted") - logger.Info("ClusterNimbusPolicy deleted due to ClusterSecurityIntentBinding deletion", - "ClusterNimbusPolicy.Name", req.Name, "ClusterSecurityIntentBinding.Name", req.Name, - ) return doNotRequeue() } logger.Error(err, "failed to get ClusterSecurityIntentBinding", "ClusterSecurityIntentBinding.Name", csib.Name) return requeueWithError(err) } - logger.Info("ClusterSecurityIntentBinding found", "ClusterSecurityIntentBinding.Name", req.Name) + + if csib.GetGeneration() == 1 { + logger.Info("ClusterSecurityIntentBinding found", "ClusterSecurityIntentBinding.Name", req.Name) + } else { + logger.Info("ClusterSecurityIntentBinding configured", "ClusterSecurityIntentBinding.Name", req.Name) + } + + if err = r.updateCsibStatus(ctx, logger, req); err != nil { + return requeueWithError(err) + } if err = r.createOrUpdateCwnp(ctx, logger, req); err != nil { return requeueWithError(err) } - if err = r.updateStatus(ctx, logger, req); err != nil { + if err = r.updateCSibStatusWithBoundSisAndCwnpInfo(ctx, logger, req); err != nil { return requeueWithError(err) } @@ -76,6 +86,9 @@ func (r *ClusterSecurityIntentBindingReconciler) SetupWithManager(mgr ctrl.Manag DeleteFunc: r.deleteFn, }, ). + Watches(&v1.SecurityIntent{}, + handler.EnqueueRequestsFromMapFunc(r.findCsibsForSi), + ). Complete(r) } @@ -97,6 +110,9 @@ func (r *ClusterSecurityIntentBindingReconciler) deleteFn(deleteEvent event.Dele if _, ok := obj.(*v1.ClusterSecurityIntentBinding); ok { return true } + if _, ok := obj.(*v1.SecurityIntent); ok { + return true + } return ownerExists(r.Client, obj) } @@ -124,6 +140,14 @@ func (r *ClusterSecurityIntentBindingReconciler) createOrUpdateCwnp(ctx context. func (r *ClusterSecurityIntentBindingReconciler) createCwnp(ctx context.Context, logger logr.Logger, csib v1.ClusterSecurityIntentBinding) error { clusterNp, err := policybuilder.BuildClusterNimbusPolicy(ctx, logger, r.Client, r.Scheme, csib) if err != nil { + if errors.Is(err, processorerrors.ErrSecurityIntentsNotFound) { + // Since the SecurityIntent(s) referenced in ClusterSecurityIntentBinding spec do not + // exist, so delete ClusterNimbusPolicy if it exists. + if err := r.deleteCwnp(ctx, csib.GetName()); err != nil { + return err + } + return nil + } logger.Error(err, "failed to build ClusterNimbusPolicy") return err } @@ -134,7 +158,11 @@ func (r *ClusterSecurityIntentBindingReconciler) createCwnp(ctx context.Context, } logger.Info("ClusterNimbusPolicy created", "ClusterNimbusPolicy.Name", clusterNp.Name) - return nil + return r.updateCwnpStatus(ctx, logger, ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: csib.Name, + }}, + ) } func (r *ClusterSecurityIntentBindingReconciler) updateCwnp(ctx context.Context, logger logr.Logger, csib v1.ClusterSecurityIntentBinding) error { @@ -146,6 +174,14 @@ func (r *ClusterSecurityIntentBindingReconciler) updateCwnp(ctx context.Context, clusterNp, err := policybuilder.BuildClusterNimbusPolicy(ctx, logger, r.Client, r.Scheme, csib) if err != nil { + if errors.Is(err, processorerrors.ErrSecurityIntentsNotFound) { + // Since the SecurityIntent(s) referenced in ClusterSecurityIntentBinding spec do not + // exist, so delete ClusterNimbusPolicy if it exists. + if err := r.deleteCwnp(ctx, csib.GetName()); err != nil { + return err + } + return nil + } logger.Error(err, "failed to build ClusterNimbusPolicy") return err } @@ -157,18 +193,74 @@ func (r *ClusterSecurityIntentBindingReconciler) updateCwnp(ctx context.Context, } logger.Info("ClusterNimbusPolicy configured", "ClusterNimbusPolicy.Name", clusterNp.Name) + return r.updateCwnpStatus(ctx, logger, ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: csib.Name, + }}, + ) +} + +func (r *ClusterSecurityIntentBindingReconciler) findCsibsForSi(ctx context.Context, si client.Object) []reconcile.Request { + logger := log.FromContext(ctx) + + csibs := &v1.ClusterSecurityIntentBindingList{} + if err := r.List(ctx, csibs); err != nil { + logger.Error(err, "failed to list ClusterSecurityIntentBindings") + return []reconcile.Request{} + } + + requests := make([]reconcile.Request, len(csibs.Items)) + + for idx, csib := range csibs.Items { + for _, intent := range csib.Spec.Intents { + if intent.Name == si.GetName() { + requests[idx] = ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: csib.GetNamespace(), + Name: csib.GetName(), + }, + } + break + } + } + } + + return requests +} + +func (r *ClusterSecurityIntentBindingReconciler) updateCsibStatus(ctx context.Context, logger logr.Logger, req ctrl.Request) error { + if retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + latestCsib := &v1.ClusterSecurityIntentBinding{} + if err := r.Get(ctx, req.NamespacedName, latestCsib); err != nil && !apierrors.IsNotFound(err) { + logger.Error(err, "failed to fetch ClusterSecurityIntentBinding", "clusterSecurityIntentBindingName", req.Name) + return err + } + + latestCsib.Status.Status = StatusCreated + latestCsib.Status.LastUpdated = metav1.Now() + if err := r.Status().Update(ctx, latestCsib); err != nil { + return err + } + + return nil + }); retryErr != nil { + logger.Error(retryErr, "failed to update ClusterSecurityIntentBinding status", "ClusterSecurityIntentBinding.Name", req.Name) + return retryErr + } + return nil } -func (r *ClusterSecurityIntentBindingReconciler) updateStatus(ctx context.Context, logger logr.Logger, req ctrl.Request) error { - // To handle potential latency issues with the Kubernetes API server, we - // implement an exponential backoff strategy when fetching the ClusterNimbusPolicy - // custom resource. This enhances resilience by retrying failed requests with - // increasing intervals, preventing excessive retries in case of persistent 'Not - // Found' errors. +func (r *ClusterSecurityIntentBindingReconciler) updateCwnpStatus(ctx context.Context, logger logr.Logger, req ctrl.Request) error { + cwnp := &v1.ClusterNimbusPolicy{} + + // To handle potential latency or outdated cache issues with the Kubernetes API + // server, we implement an exponential backoff strategy when fetching the + // ClusterNimbusPolicy custom resource. This enhances resilience by retrying + // failed requests with increasing intervals, preventing excessive retries in + // case of persistent 'Not Found' errors. if retryErr := retry.OnError(retry.DefaultRetry, apierrors.IsNotFound, func() error { - np := &v1.ClusterNimbusPolicy{} - if err := r.Get(ctx, req.NamespacedName, np); err != nil { + if err := r.Get(ctx, req.NamespacedName, cwnp); err != nil { return err } return nil @@ -184,15 +276,12 @@ func (r *ClusterSecurityIntentBindingReconciler) updateStatus(ctx context.Contex // potential issues while preventing indefinite retries in case of persistent // conflicts. if retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { - latestCwnp := &v1.ClusterNimbusPolicy{} - if err := r.Get(ctx, req.NamespacedName, latestCwnp); err != nil { + if err := r.Get(ctx, req.NamespacedName, cwnp); err != nil { return err } - latestCwnp.Status = v1.ClusterNimbusPolicyStatus{ - Status: StatusCreated, - LastUpdated: metav1.Now(), - } - if err := r.Status().Update(ctx, latestCwnp); err != nil { + cwnp.Status.Status = StatusCreated + cwnp.Status.LastUpdated = metav1.Now() + if err := r.Status().Update(ctx, cwnp); err != nil { return err } return nil @@ -201,21 +290,66 @@ func (r *ClusterSecurityIntentBindingReconciler) updateStatus(ctx context.Contex return retryErr } - // Fetch the latest SecurityIntentBinding so that we have the latest state - // on the cluster. + return nil +} + +func (r *ClusterSecurityIntentBindingReconciler) deleteCwnp(ctx context.Context, name string) error { + logger := log.FromContext(ctx) + + var cwnp v1.ClusterNimbusPolicy + err := r.Get(ctx, types.NamespacedName{Name: name}, &cwnp) + if err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return err + } + + logger.Info("Deleting ClusterNimbusPolicy since no SecurityIntents found", "clusterNimbusPolicyName", name) + logger.Info("ClusterNimbusPolicy deleted", "clusterNimbusPolicyName", name) + if err = r.Delete(context.Background(), &cwnp); err != nil { + logger.Error(err, "failed to delete ClusterNimbusPolicy", "clusterNimbusPolicyName", name) + return err + } + + return nil +} + +func (r *ClusterSecurityIntentBindingReconciler) updateCSibStatusWithBoundSisAndCwnpInfo(ctx context.Context, logger logr.Logger, req ctrl.Request) error { latestCsib := &v1.ClusterSecurityIntentBinding{} - if err := r.Get(ctx, req.NamespacedName, latestCsib); err != nil { + if err := r.Get(ctx, req.NamespacedName, latestCsib); err != nil && !apierrors.IsNotFound(err) { logger.Error(err, "failed to fetch ClusterSecurityIntentBinding", "ClusterSecurityIntentBinding.Name", req.Name) return err } - count, boundIntents := extractBoundIntentsInfo(latestCsib.Spec.Intents) - latestCsib.Status = v1.ClusterSecurityIntentBindingStatus{ - Status: StatusCreated, - LastUpdated: metav1.Now(), - NumberOfBoundIntents: count, - BoundIntents: boundIntents, - ClusterNimbusPolicy: req.Name, + + latestCwnp := &v1.ClusterNimbusPolicy{} + if retryErr := retry.OnError(retry.DefaultRetry, apierrors.IsNotFound, func() error { + if err := r.Get(ctx, req.NamespacedName, latestCwnp); err != nil { + return err + } + return nil + }); retryErr != nil { + if !apierrors.IsNotFound(retryErr) { + logger.Error(retryErr, "failed to fetch ClusterNimbusPolicy", "ClusterNimbusPolicy.Name", req.Name) + return retryErr + } + + // Remove outdated SecurityIntent(s) and ClusterNimbusPolicy info + latestCsib.Status.NumberOfBoundIntents = 0 + latestCsib.Status.BoundIntents = nil + latestCsib.Status.ClusterNimbusPolicy = "" + if err := r.Status().Update(ctx, latestCsib); err != nil { + logger.Error(err, "failed to update ClusterSecurityIntentBinding status", "ClusterSecurityIntentBinding.Name", latestCsib.Name) + return err + } + return nil } + + // Update ClusterSecurityIntentBinding status with bound SecurityIntent(s) and NimbusPolicy. + latestCsib.Status.NumberOfBoundIntents = int32(len(latestCwnp.Spec.NimbusRules)) + latestCsib.Status.BoundIntents = extractBoundIntentsNameFromCSib(ctx, r.Client, req.Name) + latestCsib.Status.ClusterNimbusPolicy = req.Name + if err := r.Status().Update(ctx, latestCsib); err != nil { logger.Error(err, "failed to update ClusterSecurityIntentBinding status", "ClusterSecurityIntentBinding.Name", latestCsib.Name) return err diff --git a/internal/controller/securityintent_controller.go b/internal/controller/securityintent_controller.go index 4976c72a..15a5f543 100644 --- a/internal/controller/securityintent_controller.go +++ b/internal/controller/securityintent_controller.go @@ -41,11 +41,16 @@ func (r *SecurityIntentReconciler) Reconcile(ctx context.Context, req ctrl.Reque return requeueWithError(err) } + if si.GetGeneration() == 1 { + logger.Info("SecurityIntent found", "SecurityIntent.Name", si.Name) + } else { + logger.Info("SecurityIntent configured", "SecurityIntent.Name", si.Name) + } + if err = r.updateStatus(ctx, req.Name); err != nil { logger.Error(err, "failed to update SecurityIntent status", "SecurityIntent.Name", req.Name) return requeueWithError(err) } - logger.Info("SecurityIntent found", "SecurityIntent.Name", si.Name) return doNotRequeue() } diff --git a/internal/controller/securityintentbinding_controller.go b/internal/controller/securityintentbinding_controller.go index 2673fe3c..64c4a119 100644 --- a/internal/controller/securityintentbinding_controller.go +++ b/internal/controller/securityintentbinding_controller.go @@ -5,6 +5,7 @@ package controller import ( "context" + "errors" "strings" "github.com/go-logr/logr" @@ -16,10 +17,13 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" v1 "github.com/5GSEC/nimbus/api/v1" + processorerrors "github.com/5GSEC/nimbus/pkg/processor/errors" "github.com/5GSEC/nimbus/pkg/processor/policybuilder" ) @@ -44,23 +48,27 @@ func (r *SecurityIntentBindingReconciler) Reconcile(ctx context.Context, req ctr if err != nil { if apierrors.IsNotFound(err) { logger.Info("SecurityIntentBinding not found. Ignoring since object must be deleted") - logger.Info("NimbusPolicy deleted due to SecurityIntentBinding deletion", - "NimbusPolicy.Name", req.Name, "NimbusPolicy.Namespace", req.Namespace, - "SecurityIntentBinding.Name", req.Name, "SecurityIntentBinding.Namespace", req.Namespace) return doNotRequeue() } logger.Error(err, "failed to fetch SecurityIntentBinding", "SecurityIntentBinding.Name", req.Name, "SecurityIntentBinding.Namespace", req.Namespace) return requeueWithError(err) } - logger.Info("SecurityIntentBinding found", "SecurityIntentBinding.Name", req.Name, "SecurityIntentBinding.Namespace", req.Namespace) - if np, err := r.createOrUpdateNp(ctx, logger, req); err != nil { + if sib.GetGeneration() == 1 { + logger.Info("SecurityIntentBinding found", "SecurityIntentBinding.Name", req.Name, "SecurityIntentBinding.Namespace", req.Namespace) + } else { + logger.Info("SecurityIntentBinding configured", "SecurityIntentBinding.Name", req.Name, "SecurityIntentBinding.Namespace", req.Namespace) + } + + if err = r.updateSibStatus(ctx, logger, req); err != nil { + return requeueWithError(err) + } + + if 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 { + if err = r.updateSibStatusWithBoundSisAndNpInfo(ctx, logger, req); err != nil { return requeueWithError(err) } @@ -79,6 +87,9 @@ func (r *SecurityIntentBindingReconciler) SetupWithManager(mgr ctrl.Manager) err DeleteFunc: r.deleteFn, }, ). + Watches(&v1.SecurityIntent{}, + handler.EnqueueRequestsFromMapFunc(r.findSibsForSi), + ). Complete(r) } @@ -100,16 +111,20 @@ func (r *SecurityIntentBindingReconciler) deleteFn(deleteEvent event.DeleteEvent if _, ok := obj.(*v1.SecurityIntentBinding); ok { return true } + if _, ok := obj.(*v1.SecurityIntent); ok { + return true + } return ownerExists(r.Client, obj) } -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 +func (r *SecurityIntentBindingReconciler) createOrUpdateNp(ctx context.Context, logger logr.Logger, req ctrl.Request) error { + // Always fetch the 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 nil, err + return err } var np v1.NimbusPolicy @@ -119,78 +134,155 @@ 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 nil, err + return err } return r.updateNp(ctx, logger, sib) } -func (r *SecurityIntentBindingReconciler) createNp(ctx context.Context, logger logr.Logger, sib v1.SecurityIntentBinding) (*v1.NimbusPolicy, error) { +func (r *SecurityIntentBindingReconciler) createNp(ctx context.Context, logger logr.Logger, sib v1.SecurityIntentBinding) 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. + // Error is caused due to CEL, so don't retry to build NimbusPolicy. if strings.Contains(err.Error(), "error processing CEL") { logger.Error(err, "failed to build NimbusPolicy") - return nil, nil + return nil + } + if errors.Is(err, processorerrors.ErrSecurityIntentsNotFound) { + // Since the SecurityIntent(s) referenced in SecurityIntentBinding spec do not + // exist, so delete NimbusPolicy if it exists. + if err := r.deleteNp(ctx, sib.GetName(), sib.GetNamespace()); err != nil { + return err + } + return nil } logger.Error(err, "failed to build NimbusPolicy") - return nil, err + return err } if nimbusPolicy == nil { logger.Info("Abort NimbusPolicy creation as no labels matched the CEL expressions") - return nil, nil + return 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 nil, err + return err } logger.Info("NimbusPolicy created", "NimbusPolicy.Name", nimbusPolicy.Name, "NimbusPolicy.Namespace", nimbusPolicy.Namespace) - return nimbusPolicy, nil + return r.updateNpStatus(ctx, logger, ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: sib.Namespace, + Name: sib.Name, + }}, + ) } -func (r *SecurityIntentBindingReconciler) updateNp(ctx context.Context, logger logr.Logger, sib v1.SecurityIntentBinding) (*v1.NimbusPolicy, error) { +func (r *SecurityIntentBindingReconciler) updateNp(ctx context.Context, logger logr.Logger, sib v1.SecurityIntentBinding) 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 nil, err + return err } 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. + // Error is caused due to CEL, so don't retry to build NimbusPolicy. if strings.Contains(err.Error(), "error processing CEL") { logger.Error(err, "failed to build NimbusPolicy") - return nil, nil + return nil + } + if errors.Is(err, processorerrors.ErrSecurityIntentsNotFound) { + // Since the SecurityIntent(s) referenced in SecurityIntentBinding spec do not + // exist, so delete NimbusPolicy if it exists. + if err := r.deleteNp(ctx, sib.GetName(), sib.GetNamespace()); err != nil { + return err + } + return nil } logger.Error(err, "failed to build NimbusPolicy") - return nil, err + return err } if nimbusPolicy == nil { logger.Info("Abort NimbusPolicy creation as no labels matched the CEL expressions") - return nil, nil + return 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 nil, err + return err } logger.Info("NimbusPolicy configured", "NimbusPolicy.Name", nimbusPolicy.Name, "NimbusPolicy.Namespace", nimbusPolicy.Namespace) - return nimbusPolicy, nil + return r.updateNpStatus(ctx, logger, ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: sib.Namespace, + Name: sib.Name, + }}, + ) +} + +func (r *SecurityIntentBindingReconciler) findSibsForSi(ctx context.Context, si client.Object) []reconcile.Request { + logger := log.FromContext(ctx) + + sibs := &v1.SecurityIntentBindingList{} + if err := r.List(ctx, sibs); err != nil { + logger.Error(err, "failed to list SecurityIntentBindings") + return []reconcile.Request{} + } + + requests := make([]reconcile.Request, len(sibs.Items)) + + for idx, sib := range sibs.Items { + for _, intent := range sib.Spec.Intents { + if intent.Name == si.GetName() { + requests[idx] = ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: sib.GetNamespace(), + Name: sib.GetName(), + }, + } + break + } + } + } + + return requests +} + +func (r *SecurityIntentBindingReconciler) deleteNp(ctx context.Context, name, namespace string) error { + logger := log.FromContext(ctx) + + var np v1.NimbusPolicy + err := r.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, &np) + if err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return err + } + + logger.Info("Deleting NimbusPolicy since no SecurityIntents found", "nimbusPolicyName", name, "nimbusPolicyNamespace", namespace) + logger.Info("NimbusPolicy deleted", "nimbusPolicyName", name, "nimbusPolicyNamespace", namespace) + if err = r.Delete(context.Background(), &np); err != nil { + logger.Error(err, "failed to delete NimbusPolicy", "nimbusPolicyName", name, "nimbusPolicyNamespace", namespace) + return err + } + + return nil } -func (r *SecurityIntentBindingReconciler) updateStatus(ctx context.Context, logger logr.Logger, req ctrl.Request) error { - // To handle potential latency issues with the Kubernetes API server, we - // implement an exponential backoff strategy when fetching the NimbusPolicy - // custom resource. This enhances resilience by retrying failed requests with - // increasing intervals, preventing excessive retries in case of persistent 'Not - // Found' errors. +func (r *SecurityIntentBindingReconciler) updateNpStatus(ctx context.Context, logger logr.Logger, req ctrl.Request) error { + np := &v1.NimbusPolicy{} + + // To handle potential latency or outdated cache issues with the Kubernetes API + // server, we implement an exponential backoff strategy when fetching the + // NimbusPolicy custom resource. This enhances resilience by retrying failed + // requests with increasing intervals, preventing excessive retries in case of + // persistent 'Not Found' errors. if retryErr := retry.OnError(retry.DefaultRetry, apierrors.IsNotFound, func() error { - np := &v1.NimbusPolicy{} if err := r.Get(ctx, req.NamespacedName, np); err != nil { return err } @@ -207,16 +299,13 @@ func (r *SecurityIntentBindingReconciler) updateStatus(ctx context.Context, logg // potential issues while preventing indefinite retries in case of persistent // conflicts. if retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { - latestNp := &v1.NimbusPolicy{} - if err := r.Get(ctx, req.NamespacedName, latestNp); err != nil { + if err := r.Get(ctx, req.NamespacedName, np); err != nil { return err } - latestNp.Status = v1.NimbusPolicyStatus{ - Status: StatusCreated, - LastUpdated: metav1.Now(), - } - if err := r.Status().Update(ctx, latestNp); err != nil { + np.Status.Status = StatusCreated + np.Status.LastUpdated = metav1.Now() + if err := r.Status().Update(ctx, np); err != nil { return err } return nil @@ -224,22 +313,68 @@ func (r *SecurityIntentBindingReconciler) updateStatus(ctx context.Context, logg logger.Error(retryErr, "failed to update NimbusPolicy status", "NimbusPolicy.Name", req.Name, "NimbusPolicy.Namespace", req.Namespace) return retryErr } + return nil +} + +func (r *SecurityIntentBindingReconciler) updateSibStatus(ctx context.Context, logger logr.Logger, req ctrl.Request) error { + if retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + latestSib := &v1.SecurityIntentBinding{} + if err := r.Get(ctx, req.NamespacedName, latestSib); err != nil { + logger.Error(err, "failed to fetch SecurityIntentBinding", "securityIntentBindingName", req.Name, "securityIntentBindingNamespace", req.Namespace) + return err + } + + latestSib.Status.Status = StatusCreated + latestSib.Status.LastUpdated = metav1.Now() + + if err := r.Status().Update(ctx, latestSib); err != nil { + return err + } + return nil + }); retryErr != nil { + logger.Error(retryErr, "failed to update SecurityIntentBinding status", "SecurityIntentBinding.Name", req.Name, "SecurityIntentBinding.Namespace", req.Namespace) + return retryErr + } + + return nil +} - // Fetch the latest SecurityIntentBinding so that we have the latest state - // on the cluster. +func (r *SecurityIntentBindingReconciler) updateSibStatusWithBoundSisAndNpInfo(ctx context.Context, logger logr.Logger, req ctrl.Request) error { latestSib := &v1.SecurityIntentBinding{} if err := r.Get(ctx, req.NamespacedName, latestSib); err != nil { logger.Error(err, "failed to fetch SecurityIntentBinding", "SecurityIntentBinding.Name", req.Name, "SecurityIntentBinding.Namespace", req.Namespace) return err } - count, boundIntents := extractBoundIntentsInfo(latestSib.Spec.Intents) - latestSib.Status = v1.SecurityIntentBindingStatus{ - Status: StatusCreated, - LastUpdated: metav1.Now(), - NumberOfBoundIntents: count, - BoundIntents: boundIntents, - NimbusPolicy: req.Name, + + latestNp := &v1.NimbusPolicy{} + if retryErr := retry.OnError(retry.DefaultRetry, apierrors.IsNotFound, func() error { + if err := r.Get(ctx, req.NamespacedName, latestNp); err != nil { + return err + } + return nil + }); retryErr != nil { + if !apierrors.IsNotFound(retryErr) { + logger.Error(retryErr, "failed to fetch NimbusPolicy", "NimbusPolicy.Name", req.Name, "NimbusPolicy.Namespace", req.Namespace) + return retryErr + } + + // Remove outdated SecurityIntent(s) and NimbusPolicy info + latestSib.Status.NumberOfBoundIntents = 0 + latestSib.Status.BoundIntents = nil + latestSib.Status.NimbusPolicy = "" + if err := r.Status().Update(ctx, latestSib); err != nil { + logger.Error(err, "failed to update SecurityIntentBinding status", "SecurityIntentBinding.Name", req.Name, "SecurityIntentBinding.Namespace", req.Namespace) + return err + } + + return nil } + + // Update SecurityIntentBinding status with bound SecurityIntent(s) and NimbusPolicy. + latestSib.Status.NumberOfBoundIntents = int32(len(latestNp.Spec.NimbusRules)) + latestSib.Status.BoundIntents = extractBoundIntentsNameFromSib(ctx, r.Client, req.Name, req.Namespace) + latestSib.Status.NimbusPolicy = req.Name + if err := r.Status().Update(ctx, latestSib); err != nil { logger.Error(err, "failed to update SecurityIntentBinding status", "SecurityIntentBinding.Name", req.Name, "SecurityIntentBinding.Namespace", req.Namespace) return err diff --git a/internal/controller/util.go b/internal/controller/util.go index 416fd374..8ff00ea7 100644 --- a/internal/controller/util.go +++ b/internal/controller/util.go @@ -10,6 +10,7 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" v1 "github.com/5GSEC/nimbus/api/v1" ) @@ -29,14 +30,45 @@ func requeueWithError(err error) (ctrl.Result, error) { return ctrl.Result{}, err } -func extractBoundIntentsInfo(intents []v1.MatchIntent) (int32, []string) { - var count int32 - var names []string - for _, intent := range intents { - count++ - names = append(names, intent.Name) +func extractBoundIntentsNameFromSib(ctx context.Context, c client.Client, name, namespace string) []string { + logger := log.FromContext(ctx) + + var boundIntentsName []string + + var sib v1.SecurityIntentBinding + if err := c.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, &sib); err != nil { + logger.Error(err, "failed to fetch SecurityIntentBinding", "securityIntentBindingName", name, "securityIntentBindingNamespace", namespace) + return boundIntentsName + } + + for _, intent := range sib.Spec.Intents { + var si v1.SecurityIntent + if err := c.Get(ctx, types.NamespacedName{Name: intent.Name}, &si); err == nil { + boundIntentsName = append(boundIntentsName, intent.Name) + } + } + + return boundIntentsName +} +func extractBoundIntentsNameFromCSib(ctx context.Context, c client.Client, name string) []string { + logger := log.FromContext(ctx) + + var boundIntentsName []string + + var csib v1.ClusterSecurityIntentBinding + if err := c.Get(ctx, types.NamespacedName{Name: name}, &csib); err != nil { + logger.Error(err, "failed to fetch ClusterSecurityIntentBinding", "ClusterSecurityIntentBinding", name) + return boundIntentsName } - return count, names + + for _, intent := range csib.Spec.Intents { + var si v1.SecurityIntent + if err := c.Get(ctx, types.NamespacedName{Name: intent.Name}, &si); err == nil { + boundIntentsName = append(boundIntentsName, intent.Name) + } + } + + return boundIntentsName } func ownerExists(c client.Client, controllee client.Object) bool { diff --git a/pkg/processor/errors/errors.go b/pkg/processor/errors/errors.go new file mode 100644 index 00000000..7e070cae --- /dev/null +++ b/pkg/processor/errors/errors.go @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2023 Authors of Nimbus + +package errors + +import ( + "errors" +) + +var ( + ErrSecurityIntentsNotFound = errors.New("no SecurityIntents found") +) diff --git a/pkg/processor/policybuilder/clusternimbuspolicy_builder.go b/pkg/processor/policybuilder/clusternimbuspolicy_builder.go index bde69035..b7968c09 100644 --- a/pkg/processor/policybuilder/clusternimbuspolicy_builder.go +++ b/pkg/processor/policybuilder/clusternimbuspolicy_builder.go @@ -5,7 +5,6 @@ package policybuilder import ( "context" - "fmt" "github.com/go-logr/logr" "github.com/pkg/errors" @@ -15,6 +14,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" v1 "github.com/5GSEC/nimbus/api/v1" + processorerrors "github.com/5GSEC/nimbus/pkg/processor/errors" "github.com/5GSEC/nimbus/pkg/processor/intentbinder" ) @@ -24,7 +24,8 @@ func BuildClusterNimbusPolicy(ctx context.Context, logger logr.Logger, k8sClient logger.Info("Building ClusterNimbusPolicy") intents := intentbinder.ExtractIntents(ctx, k8sClient, &csib) if len(intents) == 0 { - return nil, fmt.Errorf("no SecurityIntents found in the cluster") + logger.Info("ClusterNimbusPolicy creation aborted since no SecurityIntents found") + return nil, processorerrors.ErrSecurityIntentsNotFound } var nimbusRules []v1.NimbusRules diff --git a/pkg/processor/policybuilder/nimbuspolicy_builder.go b/pkg/processor/policybuilder/nimbuspolicy_builder.go index 5ad5a131..f57a037a 100644 --- a/pkg/processor/policybuilder/nimbuspolicy_builder.go +++ b/pkg/processor/policybuilder/nimbuspolicy_builder.go @@ -15,6 +15,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" v1 "github.com/5GSEC/nimbus/api/v1" + processorerrors "github.com/5GSEC/nimbus/pkg/processor/errors" "github.com/5GSEC/nimbus/pkg/processor/intentbinder" ) @@ -25,7 +26,8 @@ func BuildNimbusPolicy(ctx context.Context, logger logr.Logger, k8sClient client intents := intentbinder.ExtractIntents(ctx, k8sClient, &sib) if len(intents) == 0 { - return nil, fmt.Errorf("no SecurityIntents found in the cluster") + logger.Info("NimbusPolicy creation aborted since no SecurityIntents found") + return nil, processorerrors.ErrSecurityIntentsNotFound } var nimbusRules []v1.NimbusRules From 5d342a72575e141f14cbfd26980a2ba13e243dff Mon Sep 17 00:00:00 2001 From: Anurag Rajawat Date: Wed, 28 Feb 2024 18:18:08 +0530 Subject: [PATCH 2/3] tests(core): Add tests for SecurityIntent update and deletion Signed-off-by: Anurag Rajawat --- .../nimbuspolicy/delete/chainsaw-test.yaml | 4 +- .../nimbuspolicy/update/chainsaw-test.yaml | 4 +- ...ation-si.yaml => dns-manipulation-si.yaml} | 0 ...ion-sib.yaml => dns-manipulation-sib.yaml} | 0 .../resources/namespaced/multiple-sis.yaml | 29 ++++++ .../namespaced/sib-for-multiple-sis.yaml | 19 ++++ .../securityintent/chainsaw-test.yaml | 2 +- .../create/chainsaw-test.yaml | 4 +- .../delete/chainsaw-test.yaml | 4 +- .../update/chainsaw-test.yaml | 4 +- .../controllers/sis-and-sibs/create/README.md | 76 ++++++++++++++ .../sis-and-sibs/create/chainsaw-test.yaml | 45 +++++++++ .../controllers/sis-and-sibs/delete/README.md | 76 ++++++++++++++ .../sis-and-sibs/delete/chainsaw-test.yaml | 58 +++++++++++ .../sib-status-after-si-deletion-assert.yaml | 11 +++ .../sis-and-sibs/nimbus-policy-assert.yaml | 24 +++++ .../sis-and-sibs/np-status-assert.yaml | 10 ++ .../sis-and-sibs/si-status-assert.yaml | 11 +++ .../sis-and-sibs/sib-status-assert.yaml | 13 +++ .../controllers/sis-and-sibs/update/README.md | 98 +++++++++++++++++++ .../sis-and-sibs/update/chainsaw-test.yaml | 58 +++++++++++ .../nimbus-policy-after-deleting-one-si.yaml | 24 +++++ .../nimbus-policy-after-updating-one-si.yaml | 27 +++++ .../nimbus-policy-for-multiple-sis.yaml | 27 +++++ .../sib-status-after-si-deletion-assert.yaml | 14 +++ .../sis-and-sibs/update/updated-sib.yaml | 18 ++++ .../update/updated-unauth-sa-si.yaml | 11 +++ 27 files changed, 660 insertions(+), 11 deletions(-) rename tests/controllers/resources/namespaced/{1-dns-manipulation-si.yaml => dns-manipulation-si.yaml} (100%) rename tests/controllers/resources/namespaced/{1-dns-manipulation-sib.yaml => dns-manipulation-sib.yaml} (100%) create mode 100644 tests/controllers/resources/namespaced/multiple-sis.yaml create mode 100644 tests/controllers/resources/namespaced/sib-for-multiple-sis.yaml create mode 100644 tests/controllers/sis-and-sibs/create/README.md create mode 100644 tests/controllers/sis-and-sibs/create/chainsaw-test.yaml create mode 100644 tests/controllers/sis-and-sibs/delete/README.md create mode 100644 tests/controllers/sis-and-sibs/delete/chainsaw-test.yaml create mode 100644 tests/controllers/sis-and-sibs/delete/sib-status-after-si-deletion-assert.yaml create mode 100644 tests/controllers/sis-and-sibs/nimbus-policy-assert.yaml create mode 100644 tests/controllers/sis-and-sibs/np-status-assert.yaml create mode 100644 tests/controllers/sis-and-sibs/si-status-assert.yaml create mode 100644 tests/controllers/sis-and-sibs/sib-status-assert.yaml create mode 100644 tests/controllers/sis-and-sibs/update/README.md create mode 100644 tests/controllers/sis-and-sibs/update/chainsaw-test.yaml create mode 100644 tests/controllers/sis-and-sibs/update/nimbus-policy-after-deleting-one-si.yaml create mode 100644 tests/controllers/sis-and-sibs/update/nimbus-policy-after-updating-one-si.yaml create mode 100644 tests/controllers/sis-and-sibs/update/nimbus-policy-for-multiple-sis.yaml create mode 100644 tests/controllers/sis-and-sibs/update/sib-status-after-si-deletion-assert.yaml create mode 100644 tests/controllers/sis-and-sibs/update/updated-sib.yaml create mode 100644 tests/controllers/sis-and-sibs/update/updated-unauth-sa-si.yaml diff --git a/tests/controllers/nimbuspolicy/delete/chainsaw-test.yaml b/tests/controllers/nimbuspolicy/delete/chainsaw-test.yaml index 170c3366..923029bd 100644 --- a/tests/controllers/nimbuspolicy/delete/chainsaw-test.yaml +++ b/tests/controllers/nimbuspolicy/delete/chainsaw-test.yaml @@ -13,12 +13,12 @@ spec: - name: "Create a SecurityIntent" try: - apply: - file: ../../resources/namespaced/1-dns-manipulation-si.yaml + file: ../../resources/namespaced/dns-manipulation-si.yaml - name: "Create a SecurityIntentBinding" try: - apply: - file: ../../resources/namespaced/1-dns-manipulation-sib.yaml + file: ../../resources/namespaced/dns-manipulation-sib.yaml - name: "Verity NimbusPolicy creation" try: diff --git a/tests/controllers/nimbuspolicy/update/chainsaw-test.yaml b/tests/controllers/nimbuspolicy/update/chainsaw-test.yaml index 580f4738..5e872d96 100644 --- a/tests/controllers/nimbuspolicy/update/chainsaw-test.yaml +++ b/tests/controllers/nimbuspolicy/update/chainsaw-test.yaml @@ -13,12 +13,12 @@ spec: - name: "Create a SecurityIntent" try: - apply: - file: ../../resources/namespaced/1-dns-manipulation-si.yaml + file: ../../resources/namespaced/dns-manipulation-si.yaml - name: "Create a SecurityIntentBinding" try: - apply: - file: ../../resources/namespaced/1-dns-manipulation-sib.yaml + file: ../../resources/namespaced/dns-manipulation-sib.yaml - name: "Verity NimbusPolicy creation" try: diff --git a/tests/controllers/resources/namespaced/1-dns-manipulation-si.yaml b/tests/controllers/resources/namespaced/dns-manipulation-si.yaml similarity index 100% rename from tests/controllers/resources/namespaced/1-dns-manipulation-si.yaml rename to tests/controllers/resources/namespaced/dns-manipulation-si.yaml diff --git a/tests/controllers/resources/namespaced/1-dns-manipulation-sib.yaml b/tests/controllers/resources/namespaced/dns-manipulation-sib.yaml similarity index 100% rename from tests/controllers/resources/namespaced/1-dns-manipulation-sib.yaml rename to tests/controllers/resources/namespaced/dns-manipulation-sib.yaml diff --git a/tests/controllers/resources/namespaced/multiple-sis.yaml b/tests/controllers/resources/namespaced/multiple-sis.yaml new file mode 100644 index 00000000..e669655a --- /dev/null +++ b/tests/controllers/resources/namespaced/multiple-sis.yaml @@ -0,0 +1,29 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: SecurityIntent +metadata: + name: pkg-mgr-exec-multiple +spec: + intent: + id: swDeploymentTools + action: Block +--- +apiVersion: intent.security.nimbus.com/v1 +kind: SecurityIntent +metadata: + name: unauthorized-sa-token-access-multiple +spec: + intent: + id: unAuthorizedSaTokenAccess + action: Audit +--- +apiVersion: intent.security.nimbus.com/v1 +kind: SecurityIntent +metadata: + name: dns-manipulation-multiple +spec: + intent: + id: dnsManipulation + action: Block diff --git a/tests/controllers/resources/namespaced/sib-for-multiple-sis.yaml b/tests/controllers/resources/namespaced/sib-for-multiple-sis.yaml new file mode 100644 index 00000000..3eaf9db9 --- /dev/null +++ b/tests/controllers/resources/namespaced/sib-for-multiple-sis.yaml @@ -0,0 +1,19 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: SecurityIntentBinding +metadata: + name: multiple-sis-binding +spec: + intents: + - name: pkg-mgr-exec-multiple + - name: unauthorized-sa-token-access-multiple + - name: dns-manipulation-multiple + selector: + any: + - resources: + kind: Pod + namespace: default + matchLabels: + app: nginx diff --git a/tests/controllers/securityintent/chainsaw-test.yaml b/tests/controllers/securityintent/chainsaw-test.yaml index 6744b156..20688a42 100644 --- a/tests/controllers/securityintent/chainsaw-test.yaml +++ b/tests/controllers/securityintent/chainsaw-test.yaml @@ -13,7 +13,7 @@ spec: - name: "Create a SecurityIntent" try: - apply: - file: ../resources/namespaced/1-dns-manipulation-si.yaml + file: ../resources/namespaced/dns-manipulation-si.yaml - name: "Verify status of created SecurityIntent" try: diff --git a/tests/controllers/securityintentbinding/create/chainsaw-test.yaml b/tests/controllers/securityintentbinding/create/chainsaw-test.yaml index fd0b1597..7f034fb3 100644 --- a/tests/controllers/securityintentbinding/create/chainsaw-test.yaml +++ b/tests/controllers/securityintentbinding/create/chainsaw-test.yaml @@ -13,12 +13,12 @@ spec: - name: "Create a SecurityIntent" try: - apply: - file: ../../resources/namespaced/1-dns-manipulation-si.yaml + file: ../../resources/namespaced/dns-manipulation-si.yaml - name: "Create a SecurityIntentBinding" try: - apply: - file: ../../resources/namespaced/1-dns-manipulation-sib.yaml + file: ../../resources/namespaced/dns-manipulation-sib.yaml - name: "Verity NimbusPolicy creation" try: diff --git a/tests/controllers/securityintentbinding/delete/chainsaw-test.yaml b/tests/controllers/securityintentbinding/delete/chainsaw-test.yaml index eb457c31..21e7e688 100644 --- a/tests/controllers/securityintentbinding/delete/chainsaw-test.yaml +++ b/tests/controllers/securityintentbinding/delete/chainsaw-test.yaml @@ -13,12 +13,12 @@ spec: - name: "Create a SecurityIntent" try: - apply: - file: ../../resources/namespaced/1-dns-manipulation-si.yaml + file: ../../resources/namespaced/dns-manipulation-si.yaml - name: "Create a SecurityIntentBinding" try: - apply: - file: ../../resources/namespaced/1-dns-manipulation-sib.yaml + file: ../../resources/namespaced/dns-manipulation-sib.yaml # This resource is intentionally left undeleted by chainsaw to avoid unnecessary errors during its cleanup phase, as it will be explicitly deleted in the following step. skipDelete: true diff --git a/tests/controllers/securityintentbinding/update/chainsaw-test.yaml b/tests/controllers/securityintentbinding/update/chainsaw-test.yaml index 1d358b13..dce45134 100644 --- a/tests/controllers/securityintentbinding/update/chainsaw-test.yaml +++ b/tests/controllers/securityintentbinding/update/chainsaw-test.yaml @@ -11,12 +11,12 @@ spec: - name: "Create a SecurityIntent" try: - apply: - file: ../../resources/namespaced/1-dns-manipulation-si.yaml + file: ../../resources/namespaced/dns-manipulation-si.yaml - name: "Create a SecurityIntentBinding" try: - apply: - file: ../../resources/namespaced/1-dns-manipulation-sib.yaml + file: ../../resources/namespaced/dns-manipulation-sib.yaml - name: "Update existing SecurityIntentBinding" try: diff --git a/tests/controllers/sis-and-sibs/create/README.md b/tests/controllers/sis-and-sibs/create/README.md new file mode 100644 index 00000000..a4315f51 --- /dev/null +++ b/tests/controllers/sis-and-sibs/create/README.md @@ -0,0 +1,76 @@ +# Test: `securityintentbinding-and-securityintent-independent-creation` + +This test verifies the independent creation of SecurityIntent and SecurityIntentBinding custom resources. It ensures users can create these custom resources individually without requiring one to exist beforehand. + + +### Steps + +| # | Name | Try | Catch | Finally | +|:-:|---|:-:|:-:|:-:| +| 1 | [Create a SecurityIntentBinding](#step-Create a SecurityIntentBinding) | 1 | 0 | 0 | +| 2 | [Create a SecurityIntent](#step-Create a SecurityIntent) | 1 | 0 | 0 | +| 3 | [Verity NimbusPolicy creation](#step-Verity NimbusPolicy creation) | 1 | 0 | 0 | +| 4 | [Verify status of created SecurityIntentBinding](#step-Verify status of created SecurityIntentBinding) | 1 | 0 | 0 | +| 5 | [Verify status of created SecurityIntent](#step-Verify status of created SecurityIntent) | 1 | 0 | 0 | +| 6 | [Verify status of created NimbusPolicy](#step-Verify status of created NimbusPolicy) | 1 | 0 | 0 | + +## Step: `Create a SecurityIntentBinding` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `apply` | *No description* | + +## Step: `Create a SecurityIntent` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `apply` | *No description* | + +## Step: `Verity NimbusPolicy creation` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `assert` | *No description* | + +## Step: `Verify status of created SecurityIntentBinding` + +Verify the created SecurityIntentBinding status subresource includes the number and names of bound intents, along with the generated NimbusPolicy name. + + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `assert` | *No description* | + +## Step: `Verify status of created SecurityIntent` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `assert` | *No description* | + +## Step: `Verify status of created NimbusPolicy` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `assert` | *No description* | diff --git a/tests/controllers/sis-and-sibs/create/chainsaw-test.yaml b/tests/controllers/sis-and-sibs/create/chainsaw-test.yaml new file mode 100644 index 00000000..0942e023 --- /dev/null +++ b/tests/controllers/sis-and-sibs/create/chainsaw-test.yaml @@ -0,0 +1,45 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: securityintentbinding-and-securityintent-independent-creation +spec: + description: > + This test verifies the independent creation of SecurityIntent and SecurityIntentBinding custom resources. + It ensures users can create these custom resources individually without requiring one to exist beforehand. + + steps: + - name: "Create a SecurityIntentBinding" + try: + - apply: + file: ../../resources/namespaced/dns-manipulation-sib.yaml + + - name: "Create a SecurityIntent" + try: + - apply: + file: ../../resources/namespaced/dns-manipulation-si.yaml + + - name: "Verity NimbusPolicy creation" + try: + - assert: + file: ../nimbus-policy-assert.yaml + + - name: "Verify status of created SecurityIntentBinding" + description: > + Verify the created SecurityIntentBinding status subresource includes the number and names of bound intents, + along with the generated NimbusPolicy name. + try: + - assert: + file: ../sib-status-assert.yaml + + - name: "Verify status of created SecurityIntent" + try: + - assert: + file: ../si-status-assert.yaml + + - name: "Verify status of created NimbusPolicy" + try: + - assert: + file: ../np-status-assert.yaml diff --git a/tests/controllers/sis-and-sibs/delete/README.md b/tests/controllers/sis-and-sibs/delete/README.md new file mode 100644 index 00000000..99a45920 --- /dev/null +++ b/tests/controllers/sis-and-sibs/delete/README.md @@ -0,0 +1,76 @@ +# Test: `securityintent-deletion-after-creation-of-nimbuspolicy` + +This test verifies that when a SecurityIntent is the only one referenced by a SecurityIntentBinding, and that SecurityIntent is deleted, the corresponding NimbusPolicy is also automatically deleted. + + +### Steps + +| # | Name | Try | Catch | Finally | +|:-:|---|:-:|:-:|:-:| +| 1 | [Create a SecurityIntentBinding](#step-Create a SecurityIntentBinding) | 1 | 0 | 0 | +| 2 | [Create a SecurityIntent](#step-Create a SecurityIntent) | 1 | 0 | 0 | +| 3 | [Verify NimbusPolicy creation](#step-Verify NimbusPolicy creation) | 1 | 0 | 0 | +| 4 | [Delete previously created SecurityIntent](#step-Delete previously created SecurityIntent) | 1 | 0 | 0 | +| 5 | [Verify the NimbusPolicy deletion](#step-Verify the NimbusPolicy deletion) | 1 | 0 | 0 | +| 6 | [Verify status of SecurityIntentBinding](#step-Verify status of SecurityIntentBinding) | 1 | 0 | 0 | + +## Step: `Create a SecurityIntentBinding` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `apply` | *No description* | + +## Step: `Create a SecurityIntent` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `apply` | *No description* | + +## Step: `Verify NimbusPolicy creation` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `assert` | *No description* | + +## Step: `Delete previously created SecurityIntent` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `delete` | *No description* | + +## Step: `Verify the NimbusPolicy deletion` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `script` | *No description* | + +## Step: `Verify status of SecurityIntentBinding` + +This verifies that upon deletion of a NimbusPolicy, the corresponding SecurityIntentBinding's status subresource is updated to reflect the current information. + + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `assert` | *No description* | diff --git a/tests/controllers/sis-and-sibs/delete/chainsaw-test.yaml b/tests/controllers/sis-and-sibs/delete/chainsaw-test.yaml new file mode 100644 index 00000000..87a2d128 --- /dev/null +++ b/tests/controllers/sis-and-sibs/delete/chainsaw-test.yaml @@ -0,0 +1,58 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: securityintent-deletion-after-creation-of-nimbuspolicy +spec: + description: > + This test verifies that when a SecurityIntent is the only one referenced by a SecurityIntentBinding, and that + SecurityIntent is deleted, the corresponding NimbusPolicy is also automatically deleted. + + steps: + - name: "Create a SecurityIntentBinding" + try: + - apply: + file: ../../resources/namespaced/dns-manipulation-sib.yaml + + - name: "Create a SecurityIntent" + try: + - apply: + file: ../../resources/namespaced/dns-manipulation-si.yaml + skipDelete: true + + - name: "Verify NimbusPolicy creation" + try: + - assert: + file: ../nimbus-policy-assert.yaml + + - name: "Delete previously created SecurityIntent" + try: + - delete: + ref: + apiVersion: intent.security.nimbus.com/v1 + kind: SecurityIntent + name: dns-manipulation + expect: + - match: + apiVersion: intent.security.nimbus.com/v1 + kind: SecurityIntent + name: dns-manipulation + check: + ($error != null): true + + - name: "Verify the NimbusPolicy deletion" + try: + - script: + content: kubectl get np -n $NAMESPACE dns-manipulation-binding + check: + ($error != null): true + + - name: "Verify status of SecurityIntentBinding" + description: > + This verifies that upon deletion of a NimbusPolicy, the corresponding SecurityIntentBinding's status subresource is + updated to reflect the current information. + try: + - assert: + file: sib-status-after-si-deletion-assert.yaml diff --git a/tests/controllers/sis-and-sibs/delete/sib-status-after-si-deletion-assert.yaml b/tests/controllers/sis-and-sibs/delete/sib-status-after-si-deletion-assert.yaml new file mode 100644 index 00000000..dfe72f22 --- /dev/null +++ b/tests/controllers/sis-and-sibs/delete/sib-status-after-si-deletion-assert.yaml @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: SecurityIntentBinding +metadata: + name: dns-manipulation-binding +status: + nimbusPolicy: "" + numberOfBoundIntents: 0 + status: Created diff --git a/tests/controllers/sis-and-sibs/nimbus-policy-assert.yaml b/tests/controllers/sis-and-sibs/nimbus-policy-assert.yaml new file mode 100644 index 00000000..4cb5c161 --- /dev/null +++ b/tests/controllers/sis-and-sibs/nimbus-policy-assert.yaml @@ -0,0 +1,24 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: NimbusPolicy +metadata: + name: dns-manipulation-binding + ownerReferences: + - apiVersion: intent.security.nimbus.com/v1 + blockOwnerDeletion: true + controller: true + kind: SecurityIntentBinding + name: dns-manipulation-binding + # Since UID is not predictable so ignore it. +spec: + rules: + - description: An adversary can manipulate DNS requests to redirect network traffic + and potentially reveal end user activity. + id: dnsManipulation + rule: + action: Block + selector: + matchLabels: + app: nginx diff --git a/tests/controllers/sis-and-sibs/np-status-assert.yaml b/tests/controllers/sis-and-sibs/np-status-assert.yaml new file mode 100644 index 00000000..793d1972 --- /dev/null +++ b/tests/controllers/sis-and-sibs/np-status-assert.yaml @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: NimbusPolicy +metadata: + name: dns-manipulation-binding +status: + numberOfAdapterPolicies: 0 + status: Created diff --git a/tests/controllers/sis-and-sibs/si-status-assert.yaml b/tests/controllers/sis-and-sibs/si-status-assert.yaml new file mode 100644 index 00000000..9b74be8b --- /dev/null +++ b/tests/controllers/sis-and-sibs/si-status-assert.yaml @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: SecurityIntent +metadata: + name: dns-manipulation +status: + action: Block + id: dnsManipulation + status: Created diff --git a/tests/controllers/sis-and-sibs/sib-status-assert.yaml b/tests/controllers/sis-and-sibs/sib-status-assert.yaml new file mode 100644 index 00000000..fc6d1f14 --- /dev/null +++ b/tests/controllers/sis-and-sibs/sib-status-assert.yaml @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: SecurityIntentBinding +metadata: + name: dns-manipulation-binding +status: + boundIntents: + - dns-manipulation + nimbusPolicy: dns-manipulation-binding + numberOfBoundIntents: 1 + status: Created diff --git a/tests/controllers/sis-and-sibs/update/README.md b/tests/controllers/sis-and-sibs/update/README.md new file mode 100644 index 00000000..c6959bc3 --- /dev/null +++ b/tests/controllers/sis-and-sibs/update/README.md @@ -0,0 +1,98 @@ +# Test: `update` + +This test verifies that modifying a SecurityIntent triggers the desired updates in corresponding SecurityIntentBinding's status subresource and related NimbusPolicy. + + +### Steps + +| # | Name | Try | Catch | Finally | +|:-:|---|:-:|:-:|:-:| +| 1 | [Create a SecurityIntentBinding for multiple SecurityIntents](#step-Create a SecurityIntentBinding for multiple SecurityIntents) | 1 | 0 | 0 | +| 2 | [Create multiple SecurityIntents](#step-Create multiple SecurityIntents) | 1 | 0 | 0 | +| 3 | [Verity NimbusPolicy creation](#step-Verity NimbusPolicy creation) | 1 | 0 | 0 | +| 4 | [Update one SecurityIntent](#step-Update one SecurityIntent) | 1 | 0 | 0 | +| 5 | [Verify NimbusPolicy update](#step-Verify NimbusPolicy update) | 1 | 0 | 0 | +| 6 | [Update SecurityIntentBinding to remove one SecurityIntent](#step-Update SecurityIntentBinding to remove one SecurityIntent) | 1 | 0 | 0 | +| 7 | [Verify the NimbusPolicy update after removal of SecurityIntent](#step-Verify the NimbusPolicy update after removal of SecurityIntent) | 1 | 0 | 0 | +| 8 | [Verify status of SecurityIntentBinding after update](#step-Verify status of SecurityIntentBinding after update) | 1 | 0 | 0 | + +## Step: `Create a SecurityIntentBinding for multiple SecurityIntents` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `apply` | *No description* | + +## Step: `Create multiple SecurityIntents` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `apply` | *No description* | + +## Step: `Verity NimbusPolicy creation` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `assert` | *No description* | + +## Step: `Update one SecurityIntent` + +Update the action of one of the previously created SecurityIntents + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `apply` | *No description* | + +## Step: `Verify NimbusPolicy update` + +Verify the update of rule.action for corresponding SecurityIntent update + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `assert` | *No description* | + +## Step: `Update SecurityIntentBinding to remove one SecurityIntent` + +Remove one of the previously created SecurityIntents from the SecurityIntentBinding + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `apply` | *No description* | + +## Step: `Verify the NimbusPolicy update after removal of SecurityIntent` + +*No description* + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `assert` | *No description* | + +## Step: `Verify status of SecurityIntentBinding after update` + +This verifies that upon deletion of a NimbusPolicy, the corresponding SecurityIntentBinding's status subresource is updated to reflect the current information. + + +### Try + +| # | Operation | Description | +|:-:|---|---| +| 1 | `assert` | *No description* | diff --git a/tests/controllers/sis-and-sibs/update/chainsaw-test.yaml b/tests/controllers/sis-and-sibs/update/chainsaw-test.yaml new file mode 100644 index 00000000..01a731f8 --- /dev/null +++ b/tests/controllers/sis-and-sibs/update/chainsaw-test.yaml @@ -0,0 +1,58 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: update +spec: + description: > + This test verifies that modifying a SecurityIntent triggers the desired updates in corresponding SecurityIntentBinding's + status subresource and related NimbusPolicy. + + steps: + - name: "Create a SecurityIntentBinding for multiple SecurityIntents" + try: + - apply: + file: ../../resources/namespaced/sib-for-multiple-sis.yaml + + - name: "Create multiple SecurityIntents" + try: + - apply: + file: ../../resources/namespaced/multiple-sis.yaml + + - name: "Verity NimbusPolicy creation" + try: + - assert: + file: nimbus-policy-for-multiple-sis.yaml + + - name: "Update one SecurityIntent" + description: "Update the action of one of the previously created SecurityIntents" + try: + - apply: + file: updated-unauth-sa-si.yaml + + - name: "Verify NimbusPolicy update" + description: "Verify the update of rule.action for corresponding SecurityIntent update" + try: + - assert: + file: nimbus-policy-after-updating-one-si.yaml + + - name: "Update SecurityIntentBinding to remove one SecurityIntent" + description: "Remove one of the previously created SecurityIntents from the SecurityIntentBinding" + try: + - apply: + file: updated-sib.yaml + + - name: "Verify the NimbusPolicy update after removal of SecurityIntent" + try: + - assert: + file: nimbus-policy-after-deleting-one-si.yaml + + - name: "Verify status of SecurityIntentBinding after update" + description: > + This verifies that upon deletion of a NimbusPolicy, the corresponding SecurityIntentBinding's status subresource is + updated to reflect the current information. + try: + - assert: + file: sib-status-after-si-deletion-assert.yaml diff --git a/tests/controllers/sis-and-sibs/update/nimbus-policy-after-deleting-one-si.yaml b/tests/controllers/sis-and-sibs/update/nimbus-policy-after-deleting-one-si.yaml new file mode 100644 index 00000000..efe68a2f --- /dev/null +++ b/tests/controllers/sis-and-sibs/update/nimbus-policy-after-deleting-one-si.yaml @@ -0,0 +1,24 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: NimbusPolicy +metadata: + name: multiple-sis-binding + ownerReferences: + - apiVersion: intent.security.nimbus.com/v1 + blockOwnerDeletion: true + controller: true + kind: SecurityIntentBinding + name: multiple-sis-binding +spec: + rules: + - id: unAuthorizedSaTokenAccess + rule: + action: Block + - id: dnsManipulation + rule: + action: Block + selector: + matchLabels: + app: nginx diff --git a/tests/controllers/sis-and-sibs/update/nimbus-policy-after-updating-one-si.yaml b/tests/controllers/sis-and-sibs/update/nimbus-policy-after-updating-one-si.yaml new file mode 100644 index 00000000..9b3da3c7 --- /dev/null +++ b/tests/controllers/sis-and-sibs/update/nimbus-policy-after-updating-one-si.yaml @@ -0,0 +1,27 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: NimbusPolicy +metadata: + name: multiple-sis-binding + ownerReferences: + - apiVersion: intent.security.nimbus.com/v1 + blockOwnerDeletion: true + controller: true + kind: SecurityIntentBinding + name: multiple-sis-binding +spec: + rules: + - id: swDeploymentTools + rule: + action: Block + - id: unAuthorizedSaTokenAccess + rule: + action: Block + - id: dnsManipulation + rule: + action: Block + selector: + matchLabels: + app: nginx diff --git a/tests/controllers/sis-and-sibs/update/nimbus-policy-for-multiple-sis.yaml b/tests/controllers/sis-and-sibs/update/nimbus-policy-for-multiple-sis.yaml new file mode 100644 index 00000000..87762d56 --- /dev/null +++ b/tests/controllers/sis-and-sibs/update/nimbus-policy-for-multiple-sis.yaml @@ -0,0 +1,27 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: NimbusPolicy +metadata: + name: multiple-sis-binding + ownerReferences: + - apiVersion: intent.security.nimbus.com/v1 + blockOwnerDeletion: true + controller: true + kind: SecurityIntentBinding + name: multiple-sis-binding +spec: + rules: + - id: swDeploymentTools + rule: + action: Block + - id: unAuthorizedSaTokenAccess + rule: + action: Audit + - id: dnsManipulation + rule: + action: Block + selector: + matchLabels: + app: nginx diff --git a/tests/controllers/sis-and-sibs/update/sib-status-after-si-deletion-assert.yaml b/tests/controllers/sis-and-sibs/update/sib-status-after-si-deletion-assert.yaml new file mode 100644 index 00000000..5cebb0bf --- /dev/null +++ b/tests/controllers/sis-and-sibs/update/sib-status-after-si-deletion-assert.yaml @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: SecurityIntentBinding +metadata: + name: multiple-sis-binding +status: + boundIntents: + - unauthorized-sa-token-access-multiple + - dns-manipulation-multiple + nimbusPolicy: multiple-sis-binding + numberOfBoundIntents: 2 + status: Created diff --git a/tests/controllers/sis-and-sibs/update/updated-sib.yaml b/tests/controllers/sis-and-sibs/update/updated-sib.yaml new file mode 100644 index 00000000..20fe3267 --- /dev/null +++ b/tests/controllers/sis-and-sibs/update/updated-sib.yaml @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: SecurityIntentBinding +metadata: + name: multiple-sis-binding +spec: + intents: + - name: unauthorized-sa-token-access-multiple + - name: dns-manipulation-multiple + selector: + any: + - resources: + kind: Pod + namespace: default + matchLabels: + app: nginx diff --git a/tests/controllers/sis-and-sibs/update/updated-unauth-sa-si.yaml b/tests/controllers/sis-and-sibs/update/updated-unauth-sa-si.yaml new file mode 100644 index 00000000..e83e913f --- /dev/null +++ b/tests/controllers/sis-and-sibs/update/updated-unauth-sa-si.yaml @@ -0,0 +1,11 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2023 Authors of Nimbus + +apiVersion: intent.security.nimbus.com/v1 +kind: SecurityIntent +metadata: + name: unauthorized-sa-token-access-multiple +spec: + intent: + id: unAuthorizedSaTokenAccess + action: Block From 4e5b0a8f52033d9b0d9f89412816498a60edba96 Mon Sep 17 00:00:00 2001 From: Anurag Rajawat Date: Wed, 28 Feb 2024 18:23:18 +0530 Subject: [PATCH 3/3] fix(adapters): Handle Policies on SecurityIntent update and deletion Signed-off-by: Anurag Rajawat --- .../nimbus-kubearmor/manager/manager.go | 82 ++++++++++++------- .../nimbus-netpol/manager/netpols_manager.go | 81 ++++++++++++++---- pkg/adapter/util/nimbuspolicy_util.go | 22 ++++- 3 files changed, 136 insertions(+), 49 deletions(-) diff --git a/pkg/adapter/nimbus-kubearmor/manager/manager.go b/pkg/adapter/nimbus-kubearmor/manager/manager.go index c2794a2a..7effd747 100644 --- a/pkg/adapter/nimbus-kubearmor/manager/manager.go +++ b/pkg/adapter/nimbus-kubearmor/manager/manager.go @@ -6,6 +6,7 @@ package manager import ( "context" "fmt" + "strings" "github.com/go-logr/logr" kubearmorv1 "github.com/kubearmor/KubeArmor/pkg/KubeArmorController/api/security.kubearmor.com/v1" @@ -89,9 +90,9 @@ func reconcileKsp(ctx context.Context, kspName, namespace string, deleted bool) return } if deleted { - logger.Info("Reconciling deleted KubeArmorPolicy", "KubeArmorPolicy.Name", kspName, "KubeArmorPolicy.Namespace", namespace) + logger.V(2).Info("Reconciling deleted KubeArmorPolicy", "KubeArmorPolicy.Name", kspName, "KubeArmorPolicy.Namespace", namespace) } else { - logger.Info("Reconciling modified KubeArmorPolicy", "KubeArmorPolicy.Name", kspName, "KubeArmorPolicy.Namespace", namespace) + logger.V(2).Info("Reconciling modified KubeArmorPolicy", "KubeArmorPolicy.Name", kspName, "KubeArmorPolicy.Namespace", namespace) } createOrUpdateKsp(ctx, npName, namespace) } @@ -109,11 +110,9 @@ func createOrUpdateKsp(ctx context.Context, npName, npNamespace string) { return } + deleteDanglingKsps(ctx, np, logger) ksps := processor.BuildKspsFrom(logger, &np) - // TODO: Fix loop due to unnecessary KSPs deletion - //deleteUnnecessaryKsps(ctx, ksps, npNamespace, logger) - // Iterate using a separate index variable to avoid aliasing for idx := range ksps { ksp := ksps[idx] @@ -130,12 +129,14 @@ func createOrUpdateKsp(ctx context.Context, npName, npNamespace string) { logger.Error(err, "failed to get existing KubeArmorPolicy", "KubeArmorPolicy.Name", ksp.Name, "KubeArmorPolicy.Namespace", ksp.Namespace) return } - if err != nil && errors.IsNotFound(err) { - if err = k8sClient.Create(ctx, &ksp); err != nil { - logger.Error(err, "failed to create KubeArmorPolicy", "KubeArmorPolicy.Name", ksp.Name, "KubeArmorPolicy.Namespace", ksp.Namespace) - return + if err != nil { + if errors.IsNotFound(err) { + if err = k8sClient.Create(ctx, &ksp); err != nil { + logger.Error(err, "failed to create KubeArmorPolicy", "KubeArmorPolicy.Name", ksp.Name, "KubeArmorPolicy.Namespace", ksp.Namespace) + return + } + logger.Info("KubeArmorPolicy created", "KubeArmorPolicy.Name", ksp.Name, "KubeArmorPolicy.Namespace", ksp.Namespace) } - logger.Info("KubeArmorPolicy created", "KubeArmorPolicy.Name", ksp.Name, "KubeArmorPolicy.Namespace", ksp.Namespace) } else { ksp.ObjectMeta.ResourceVersion = existingKsp.ObjectMeta.ResourceVersion if err = k8sClient.Update(ctx, &ksp); err != nil { @@ -145,14 +146,12 @@ func createOrUpdateKsp(ctx context.Context, npName, npNamespace string) { logger.Info("KubeArmorPolicy configured", "KubeArmorPolicy.Name", existingKsp.Name, "KubeArmorPolicy.Namespace", existingKsp.Namespace) } - // Every adapter is responsible for updating the status field of the - // corresponding NimbusPolicy with the number and names of successfully created - // policies by calling the 'adapterutil.UpdateNpStatus' API. This provides - // feedback to users about the translation and deployment of their security - // intent. - if err = adapterutil.UpdateNpStatus(ctx, k8sClient, "KubeArmorPolicy/"+ksp.Name, np.Name, np.Namespace); err != nil { - logger.Error(err, "failed to update KubeArmorPolicies status in NimbusPolicy") - } + //TODO: Due to adapters' dependency on nimbus module, the docker image build is + // failing. The relevant code is commented out below (lines 153-155). We shall + // uncomment this code in a subsequent PR. + //if err = adapterutil.UpdateNpStatus(ctx, k8sClient, "KubeArmorPolicy/"+ksp.Name, np.Name, np.Namespace, false); err != nil { + // logger.Error(err, "failed to update KubeArmorPolicies status in NimbusPolicy") + //} } } @@ -177,24 +176,49 @@ func deleteKsp(ctx context.Context, npName, npNamespace string) { } } -func deleteUnnecessaryKsps(ctx context.Context, currentKsps []kubearmorv1.KubeArmorPolicy, namespace string, logger logr.Logger) { +func deleteDanglingKsps(ctx context.Context, np intentv1.NimbusPolicy, logger logr.Logger) { var existingKsps kubearmorv1.KubeArmorPolicyList - if err := k8sClient.List(ctx, &existingKsps, client.InNamespace(namespace)); err != nil { + if err := k8sClient.List(ctx, &existingKsps, client.InNamespace(np.Namespace)); err != nil { logger.Error(err, "failed to list KubeArmorPolicies for cleanup") return } - currentKspNames := make(map[string]bool) - for _, ksp := range currentKsps { - currentKspNames[ksp.Name] = true + var kspsOwnedByNp []kubearmorv1.KubeArmorPolicy + for _, ksp := range existingKsps.Items { + ownerRef := ksp.OwnerReferences[0] + if ownerRef.Name == np.Name && ownerRef.UID == np.UID { + kspsOwnedByNp = append(kspsOwnedByNp, ksp) + } + } + if len(kspsOwnedByNp) == 0 { + return } - for i := range existingKsps.Items { - 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) - } + kspsToDelete := make(map[string]kubearmorv1.KubeArmorPolicy) + + // Populate owned KSPs + for _, kspOwnedByNp := range kspsOwnedByNp { + kspsToDelete[kspOwnedByNp.Name] = kspOwnedByNp + } + + for _, nimbusRule := range np.Spec.NimbusRules { + kspName := np.Name + "-" + strings.ToLower(nimbusRule.ID) + delete(kspsToDelete, kspName) + } + + for kspName := range kspsToDelete { + ksp := kspsToDelete[kspName] + if err := k8sClient.Delete(ctx, &ksp); err != nil { + logger.Error(err, "failed to delete dangling KubeArmorPolicy", "KubeArmorPolicy.Name", ksp.Namespace, "KubeArmorPolicy.Namespace", ksp.Namespace) + continue } + + //TODO: Due to adapters' dependency on nimbus module, the docker image build is + // failing. The relevant code is commented out below (lines 217-219). We shall + // uncomment this code in a subsequent PR. + //if err := adapterutil.UpdateNpStatus(ctx, k8sClient, "KubeArmorPolicy/"+ksp.Name, np.Name, np.Namespace, true); err != nil { + // logger.Error(err, "failed to update KubeArmorPolicy status in NimbusPolicy") + //} + logger.Info("Dangling KubeArmorPolicy deleted", "KubeArmorPolicy.Name", ksp.Name, "KubeArmorPolicy.Namespace", ksp.Namespace) } } diff --git a/pkg/adapter/nimbus-netpol/manager/netpols_manager.go b/pkg/adapter/nimbus-netpol/manager/netpols_manager.go index b3dddf87..aae8a006 100644 --- a/pkg/adapter/nimbus-netpol/manager/netpols_manager.go +++ b/pkg/adapter/nimbus-netpol/manager/netpols_manager.go @@ -6,7 +6,9 @@ package manager import ( "context" "fmt" + "strings" + "github.com/go-logr/logr" netv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -88,9 +90,9 @@ func reconcileNetPol(ctx context.Context, netpolName, namespace string, deleted return } if deleted { - logger.Info("Reconciling deleted NetworkPolicy", "NetworkPolicy.Name", netpolName, "NetworkPolicy.Namespace", namespace) + logger.V(2).Info("Reconciling deleted NetworkPolicy", "NetworkPolicy.Name", netpolName, "NetworkPolicy.Namespace", namespace) } else { - logger.Info("Reconciling modified NetworkPolicy", "NetworkPolicy.Name", netpolName, "NetworkPolicy.Namespace", namespace) + logger.V(2).Info("Reconciling modified NetworkPolicy", "NetworkPolicy.Name", netpolName, "NetworkPolicy.Namespace", namespace) } createOrUpdateNetworkPolicy(ctx, npName, namespace) } @@ -108,6 +110,7 @@ func createOrUpdateNetworkPolicy(ctx context.Context, npName, npNamespace string return } + deleteDanglingNetpols(ctx, np, logger) netPols := processor.BuildNetPolsFrom(logger, np) // Iterate using a separate index variable to avoid aliasing for idx := range netPols { @@ -125,12 +128,14 @@ func createOrUpdateNetworkPolicy(ctx context.Context, npName, npNamespace string logger.Error(err, "failed to get existing NetworkPolicy", "NetworkPolicy.Name", netpol.Name, "NetworkPolicy.Namespace", netpol.Namespace) return } - if errors.IsNotFound(err) { - if err = k8sClient.Create(ctx, &netpol); err != nil { - logger.Error(err, "failed to create NetworkPolicy", "NetworkPolicy.Name", netpol.Name, "NetworkPolicy.Namespace", netpol.Namespace) - return + if err != nil { + if errors.IsNotFound(err) { + if err = k8sClient.Create(ctx, &netpol); err != nil { + logger.Error(err, "failed to create NetworkPolicy", "NetworkPolicy.Name", netpol.Name, "NetworkPolicy.Namespace", netpol.Namespace) + return + } + logger.Info("NetworkPolicy created", "NetworkPolicy.Name", netpol.Name, "NetworkPolicy.Namespace", netpol.Namespace) } - logger.Info("NetworkPolicy created", "NetworkPolicy.Name", netpol.Name, "NetworkPolicy.Namespace", netpol.Namespace) } else { netpol.ObjectMeta.ResourceVersion = existingNetpol.ObjectMeta.ResourceVersion if err = k8sClient.Update(ctx, &netpol); err != nil { @@ -139,15 +144,12 @@ func createOrUpdateNetworkPolicy(ctx context.Context, npName, npNamespace string } logger.Info("NetworkPolicy configured", "NetworkPolicy.Name", netpol.Name, "NetworkPolicy.Namespace", netpol.Namespace) } - - // Every adapter is responsible for updating the status field of the - // corresponding NimbusPolicy with the number and names of successfully created - // policies by calling the 'adapterutil.UpdateNpStatus' API. This provides - // feedback to users about the translation and deployment of their security - // intent. - if err = adapterutil.UpdateNpStatus(ctx, k8sClient, "NetworkPolicy/"+netpol.Name, np.Name, np.Namespace); err != nil { - logger.Error(err, "failed to update NetworkPolicies status in NimbusPolicy") - } + //TODO: Due to adapters' dependency on nimbus module, the docker image build is + // failing. The relevant code is commented out below (lines 150-152). We shall + // uncomment this code in a subsequent PR. + //if err = adapterutil.UpdateNpStatus(ctx, k8sClient, "NetworkPolicy/"+netpol.Name, np.Name, np.Namespace, false); err != nil { + // logger.Error(err, "failed to update NetworkPolicies status in NimbusPolicy") + //} } } @@ -171,3 +173,50 @@ func deleteNetworkPolicy(ctx context.Context, npName, npNamespace string) { ) } } + +func deleteDanglingNetpols(ctx context.Context, np intentv1.NimbusPolicy, logger logr.Logger) { + var existingNetpols netv1.NetworkPolicyList + if err := k8sClient.List(ctx, &existingNetpols, client.InNamespace(np.Namespace)); err != nil { + logger.Error(err, "failed to list NetworkPolicies for cleanup") + return + } + + var netpolsOwnedByNp []netv1.NetworkPolicy + for _, netpol := range existingNetpols.Items { + ownerRef := netpol.OwnerReferences[0] + if ownerRef.Name == np.Name && ownerRef.UID == np.UID { + netpolsOwnedByNp = append(netpolsOwnedByNp, netpol) + } + } + + if len(netpolsOwnedByNp) == 0 { + return + } + + netpolsToDelete := make(map[string]netv1.NetworkPolicy) + + // Populate owned Netpols + for _, netpolOwnedByNp := range netpolsOwnedByNp { + netpolsToDelete[netpolOwnedByNp.Name] = netpolOwnedByNp + } + + for _, nimbusRule := range np.Spec.NimbusRules { + netpolName := np.Name + "-" + strings.ToLower(nimbusRule.ID) + delete(netpolsToDelete, netpolName) + } + + for netpolName := range netpolsToDelete { + netpol := netpolsToDelete[netpolName] + if err := k8sClient.Delete(ctx, &netpol); err != nil { + logger.Error(err, "failed to delete dangling NetworkPolicy", "NetworkPolicy.Name", netpol.Namespace, "NetworkPolicy.Namespace", netpol.Namespace) + continue + } + //TODO: Due to adapters' dependency on nimbus module, the docker image build is + // failing. The relevant code is commented out below (lines 215-217). We shall + // uncomment this code in a subsequent PR. + //if err := adapterutil.UpdateNpStatus(ctx, k8sClient, "NetworkPolicy/"+netpol.Name, np.Name, np.Namespace, true); err != nil { + // logger.Error(err, "failed to update NetworkPolicy status in NimbusPolicy") + //} + logger.Info("Dangling NetworkPolicy deleted", "NetworkPolicy.Name", netpol.Name, "NetworkPolicy.Namespace", netpol.Namespace) + } +} diff --git a/pkg/adapter/util/nimbuspolicy_util.go b/pkg/adapter/util/nimbuspolicy_util.go index 2579971a..5987e46e 100644 --- a/pkg/adapter/util/nimbuspolicy_util.go +++ b/pkg/adapter/util/nimbuspolicy_util.go @@ -22,8 +22,12 @@ func ExtractNpName(policyName string) string { } // UpdateNpStatus updates the provided NimbusPolicy status with the number and -// names of its descendant policies that were created. -func UpdateNpStatus(ctx context.Context, k8sClient client.Client, currPolicyFullName, npName, namespace string) error { +// names of its descendant policies that were created. Every adapter is +// responsible for updating the status field of the corresponding NimbusPolicy +// with the number and names of successfully created policies by calling this +// API. This provides feedback to users about the translation and deployment of +// their security intent +func UpdateNpStatus(ctx context.Context, k8sClient client.Client, currPolicyFullName, npName, namespace string, decrement bool) error { // Since multiple adapters may attempt to update the NimbusPolicy status // concurrently, potentially leading to conflicts. To ensure data consistency, // retry on write failures. On conflict, the update is retried with an @@ -35,7 +39,7 @@ func UpdateNpStatus(ctx context.Context, k8sClient client.Client, currPolicyFull return nil } - updateCountAndPoliciesName(latestNp, currPolicyFullName) + updateCountAndPoliciesName(latestNp, currPolicyFullName, decrement) if err := k8sClient.Status().Update(ctx, latestNp); err != nil { return err } @@ -47,11 +51,21 @@ func UpdateNpStatus(ctx context.Context, k8sClient client.Client, currPolicyFull return nil } -func updateCountAndPoliciesName(latestNp *intentv1.NimbusPolicy, currPolicyFullName string) { +func updateCountAndPoliciesName(latestNp *intentv1.NimbusPolicy, currPolicyFullName string, decrement bool) { if !contains(latestNp.Status.Policies, currPolicyFullName) { latestNp.Status.NumberOfAdapterPolicies++ latestNp.Status.Policies = append(latestNp.Status.Policies, currPolicyFullName) } + + if decrement { + latestNp.Status.NumberOfAdapterPolicies-- + for idx, existingPolicyName := range latestNp.Status.Policies { + if existingPolicyName == currPolicyFullName { + latestNp.Status.Policies = append(latestNp.Status.Policies[:idx], latestNp.Status.Policies[idx+1:]...) + return + } + } + } } func contains(existingPolicies []string, policy string) bool {