[release-2.9] 🤖 Sync from open-cluster-management-io/governance-policy-propagator: #127

controllers/common/common.go

@@ -10,15 +10,16 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/go-logr/logr"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	clusterv1 "open-cluster-management.io/api/cluster/v1"
 	clusterv1beta1 "open-cluster-management.io/api/cluster/v1beta1"
 	appsv1 "open-cluster-management.io/multicloud-operators-subscription/pkg/apis/apps/placementrule/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	policiesv1 "open-cluster-management.io/governance-policy-propagator/api/v1"
+	policiesv1beta1 "open-cluster-management.io/governance-policy-propagator/api/v1beta1"
 )
 
 const (
@@ -60,36 +61,79 @@ func IsReplicatedPolicy(c client.Client, policy client.Object) (bool, error) {
 	return IsInClusterNamespace(c, policy.GetNamespace())
 }
 
-// IsPbForPoicy compares group and kind with policy group and kind for given pb
-func IsPbForPoicy(pb *policiesv1.PlacementBinding) bool {
-	found := false
-
-	subjects := pb.Subjects
-	for _, subject := range subjects {
-		if subject.Kind == policiesv1.Kind && subject.APIGroup == policiesv1.SchemeGroupVersion.Group {
-			found = true
+// IsForPolicyOrPolicySet returns true if any of the subjects of the PlacementBinding are Policies
+// or PolicySets.
+func IsForPolicyOrPolicySet(pb *policiesv1.PlacementBinding) bool {
+	if pb == nil {
+		return false
+	}
 
-			break
+	for _, subject := range pb.Subjects {
+		if subject.APIGroup == policiesv1.SchemeGroupVersion.Group &&
+			(subject.Kind == policiesv1.Kind || subject.Kind == policiesv1.PolicySetKind) {
+			return true
 		}
 	}
 
-	return found
+	return false
 }
 
-// IsPbForPoicySet compares group and kind with policyset group and kind for given pb
-func IsPbForPoicySet(pb *policiesv1.PlacementBinding) bool {
-	found := false
+// IsPbForPolicySet compares group and kind with policyset group and kind for given pb
+func IsPbForPolicySet(pb *policiesv1.PlacementBinding) bool {
+	if pb == nil {
+		return false
+	}
 
 	subjects := pb.Subjects
 	for _, subject := range subjects {
 		if subject.Kind == policiesv1.PolicySetKind && subject.APIGroup == policiesv1.SchemeGroupVersion.Group {
-			found = true
+			return true
+		}
+	}
+
+	return false
+}
+
+// GetPoliciesInPlacementBinding returns a list of the Policies that are either direct subjects of
+// the given PlacementBinding, or are in PolicySets that are subjects of the PlacementBinding.
+// The list items are not guaranteed to be unique (for example if a policy is in multiple sets).
+func GetPoliciesInPlacementBinding(
+	ctx context.Context, c client.Client, pb *policiesv1.PlacementBinding,
+) []reconcile.Request {
+	result := make([]reconcile.Request, 0)
+
+	for _, subject := range pb.Subjects {
+		if subject.APIGroup != policiesv1.SchemeGroupVersion.Group {
+			continue
+		}
+
+		switch subject.Kind {
+		case policiesv1.Kind:
+			result = append(result, reconcile.Request{NamespacedName: types.NamespacedName{
+				Name:      subject.Name,
+				Namespace: pb.GetNamespace(),
+			}})
+		case policiesv1.PolicySetKind:
+			setNN := types.NamespacedName{
+				Name:      subject.Name,
+				Namespace: pb.GetNamespace(),
+			}
+
+			policySet := policiesv1beta1.PolicySet{}
+			if err := c.Get(ctx, setNN, &policySet); err != nil {
+				continue
+			}
 
-			break
+			for _, plc := range policySet.Spec.Policies {
+				result = append(result, reconcile.Request{NamespacedName: types.NamespacedName{
+					Name:      string(plc),
+					Namespace: pb.GetNamespace(),
+				}})
+			}
 		}
 	}
 
-	return found
+	return result
 }
 
 // FindNonCompliantClustersForPolicy returns cluster in noncompliant status with given policy
@@ -105,80 +149,76 @@ func FindNonCompliantClustersForPolicy(plc *policiesv1.Policy) []string {
 	return clusterList
 }
 
-// GetClusterPlacementDecisions return the placement decisions from cluster
-// placement decisions
-func GetClusterPlacementDecisions(
-	c client.Client, pb policiesv1.PlacementBinding, instance *policiesv1.Policy, log logr.Logger,
-) ([]appsv1.PlacementDecision, error) {
-	log = log.WithValues("name", pb.PlacementRef.Name, "namespace", instance.GetNamespace())
-	pl := &clusterv1beta1.Placement{}
+func HasValidPlacementRef(pb *policiesv1.PlacementBinding) bool {
+	switch pb.PlacementRef.Kind {
+	case "PlacementRule":
+		return pb.PlacementRef.APIGroup == appsv1.SchemeGroupVersion.Group
+	case "Placement":
+		return pb.PlacementRef.APIGroup == clusterv1beta1.SchemeGroupVersion.Group
+	default:
+		return false
+	}
+}
 
-	err := c.Get(context.TODO(), types.NamespacedName{
-		Namespace: instance.GetNamespace(),
-		Name:      pb.PlacementRef.Name,
-	}, pl)
-	// no error when not found
-	if err != nil && !k8serrors.IsNotFound(err) {
-		log.Error(err, "Failed to get the Placement")
+// GetDecisions returns the placement decisions from the Placement or PlacementRule referred to by
+// the PlacementBinding
+func GetDecisions(c client.Client, pb *policiesv1.PlacementBinding) ([]appsv1.PlacementDecision, error) {
+	if !HasValidPlacementRef(pb) {
+		return nil, fmt.Errorf("placement binding %s/%s reference is not valid", pb.Name, pb.Namespace)
+	}
 
-		return nil, err
+	refNN := types.NamespacedName{
+		Namespace: pb.GetNamespace(),
+		Name:      pb.PlacementRef.Name,
 	}
 
-	list := &clusterv1beta1.PlacementDecisionList{}
-	lopts := &client.ListOptions{Namespace: instance.GetNamespace()}
+	switch pb.PlacementRef.Kind {
+	case "Placement":
+		pl := &clusterv1beta1.Placement{}
 
-	opts := client.MatchingLabels{"cluster.open-cluster-management.io/placement": pl.GetName()}
-	opts.ApplyToList(lopts)
-	err = c.List(context.TODO(), list, lopts)
+		err := c.Get(context.TODO(), refNN, pl)
+		if err != nil && !k8serrors.IsNotFound(err) {
+			return nil, fmt.Errorf("failed to get Placement '%v': %w", pb.PlacementRef.Name, err)
+		}
 
-	// do not error out if not found
-	if err != nil && !k8serrors.IsNotFound(err) {
-		log.Error(err, "Failed to get the PlacementDecision")
+		if k8serrors.IsNotFound(err) {
+			return nil, nil
+		}
 
-		return nil, err
-	}
+		list := &clusterv1beta1.PlacementDecisionList{}
+		lopts := &client.ListOptions{Namespace: pb.GetNamespace()}
 
-	var decisions []appsv1.PlacementDecision
-	decisions = make([]appsv1.PlacementDecision, 0, len(list.Items))
+		opts := client.MatchingLabels{"cluster.open-cluster-management.io/placement": pl.GetName()}
+		opts.ApplyToList(lopts)
 
-	for _, item := range list.Items {
-		for _, cluster := range item.Status.Decisions {
-			decided := &appsv1.PlacementDecision{
-				ClusterName:      cluster.ClusterName,
-				ClusterNamespace: cluster.ClusterName,
-			}
-			decisions = append(decisions, *decided)
+		err = c.List(context.TODO(), list, lopts)
+		if err != nil && !k8serrors.IsNotFound(err) {
+			return nil, fmt.Errorf("failed to list the PlacementDecisions for '%v', %w", pb.PlacementRef.Name, err)
 		}
-	}
 
-	return decisions, nil
-}
+		decisions := make([]appsv1.PlacementDecision, 0)
 
-// GetApplicationPlacementDecisions return the placement decisions from an application
-// lifecycle placementrule
-func GetApplicationPlacementDecisions(
-	c client.Client, pb policiesv1.PlacementBinding, instance *policiesv1.Policy, log logr.Logger,
-) ([]appsv1.PlacementDecision, error) {
-	log = log.WithValues("name", pb.PlacementRef.Name, "namespace", instance.GetNamespace())
-	plr := &appsv1.PlacementRule{}
+		for _, item := range list.Items {
+			for _, cluster := range item.Status.Decisions {
+				decisions = append(decisions, appsv1.PlacementDecision{
+					ClusterName:      cluster.ClusterName,
+					ClusterNamespace: cluster.ClusterName,
+				})
+			}
+		}
 
-	err := c.Get(context.TODO(), types.NamespacedName{
-		Namespace: instance.GetNamespace(),
-		Name:      pb.PlacementRef.Name,
-	}, plr)
-	// no error when not found
-	if err != nil && !k8serrors.IsNotFound(err) {
-		log.Error(
-			err,
-			"Failed to get the PlacementRule",
-			"namespace", instance.GetNamespace(),
-			"name", pb.PlacementRef.Name,
-		)
+		return decisions, nil
+	case "PlacementRule":
+		plr := &appsv1.PlacementRule{}
+		if err := c.Get(context.TODO(), refNN, plr); err != nil && !k8serrors.IsNotFound(err) {
+			return nil, fmt.Errorf("failed to get PlacementRule '%v': %w", pb.PlacementRef.Name, err)
+		}
 
-		return nil, err
+		// if the PlacementRule was not found, the decisions will be empty
+		return plr.Status.Decisions, nil
 	}
 
-	return plr.Status.Decisions, nil
+	return nil, fmt.Errorf("placement binding %s/%s reference is not valid", pb.Name, pb.Namespace)
 }
 
 // GetNumWorkers is a helper function to return the number of workers to handle concurrent tasks

controllers/policyset/placementBindingPredicate.go

@@ -19,18 +19,18 @@ var pbPredicateFuncs = predicate.Funcs{
 		//nolint:forcetypeassert
 		pbObjOld := e.ObjectOld.(*policiesv1.PlacementBinding)
 
-		return common.IsPbForPoicySet(pbObjNew) || common.IsPbForPoicySet(pbObjOld)
+		return common.IsPbForPolicySet(pbObjNew) || common.IsPbForPolicySet(pbObjOld)
 	},
 	CreateFunc: func(e event.CreateEvent) bool {
 		//nolint:forcetypeassert
 		pbObj := e.Object.(*policiesv1.PlacementBinding)
 
-		return common.IsPbForPoicySet(pbObj)
+		return common.IsPbForPolicySet(pbObj)
 	},
 	DeleteFunc: func(e event.DeleteEvent) bool {
 		//nolint:forcetypeassert
 		pbObj := e.Object.(*policiesv1.PlacementBinding)
 
-		return common.IsPbForPoicySet(pbObj)
+		return common.IsPbForPolicySet(pbObj)
 	},
 }

controllers/policyset/policyset_controller.go

@@ -190,7 +190,7 @@ func (r *PolicySetReconciler) processPolicySet(ctx context.Context, plcSet *poli
 				}
 
 				var decisions []appsv1.PlacementDecision
-				decisions, err = getDecisions(r.Client, *pb, childPlc)
+				decisions, err = common.GetDecisions(r.Client, pb)
 				if err != nil {
 					log.Error(err, "Error getting placement decisions for binding "+pbName)
 				}
@@ -297,31 +297,6 @@ func showCompliance(compliancesFound []string, unknown []string, pending []strin
 	return false
 }
 
-// getDecisions gets the PlacementDecisions for a PlacementBinding
-func getDecisions(c client.Client, pb policyv1.PlacementBinding,
-	instance *policyv1.Policy,
-) ([]appsv1.PlacementDecision, error) {
-	if pb.PlacementRef.APIGroup == appsv1.SchemeGroupVersion.Group &&
-		pb.PlacementRef.Kind == "PlacementRule" {
-		d, err := common.GetApplicationPlacementDecisions(c, pb, instance, log)
-		if err != nil {
-			return nil, err
-		}
-
-		return d, nil
-	} else if pb.PlacementRef.APIGroup == clusterv1beta1.SchemeGroupVersion.Group &&
-		pb.PlacementRef.Kind == "Placement" {
-		d, err := common.GetClusterPlacementDecisions(c, pb, instance, log)
-		if err != nil {
-			return nil, err
-		}
-
-		return d, nil
-	}
-
-	return nil, fmt.Errorf("placement binding %s/%s reference is not valid", pb.Name, pb.Namespace)
-}
-
 // SetupWithManager sets up the controller with the Manager.
 func (r *PolicySetReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).

controllers/propagator/placementBindingMapper.go

@@ -6,64 +6,22 @@ package propagator
 import (
 	"context"
 
-	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	policiesv1 "open-cluster-management.io/governance-policy-propagator/api/v1"
-	policiesv1beta1 "open-cluster-management.io/governance-policy-propagator/api/v1beta1"
+	"open-cluster-management.io/governance-policy-propagator/controllers/common"
 )
 
 func placementBindingMapper(c client.Client) handler.MapFunc {
 	return func(ctx context.Context, obj client.Object) []reconcile.Request {
 		//nolint:forcetypeassert
-		object := obj.(*policiesv1.PlacementBinding)
-		var result []reconcile.Request
-
-		log := log.WithValues("placementBindingName", object.GetName(), "namespace", object.GetNamespace())
+		pb := obj.(*policiesv1.PlacementBinding)
 
+		log := log.WithValues("placementBindingName", pb.GetName(), "namespace", pb.GetNamespace())
 		log.V(2).Info("Reconcile request for a PlacementBinding")
 
-		subjects := object.Subjects
-		for _, subject := range subjects {
-			if subject.APIGroup == policiesv1.SchemeGroupVersion.Group {
-				if subject.Kind == policiesv1.Kind {
-					log.V(2).Info("Found reconciliation request from policy placement binding",
-						"policyName", subject.Name)
-
-					request := reconcile.Request{NamespacedName: types.NamespacedName{
-						Name:      subject.Name,
-						Namespace: object.GetNamespace(),
-					}}
-					result = append(result, request)
-				} else if subject.Kind == policiesv1.PolicySetKind {
-					policySetNamespacedName := types.NamespacedName{
-						Name:      subject.Name,
-						Namespace: object.GetNamespace(),
-					}
-					policySet := &policiesv1beta1.PolicySet{}
-					err := c.Get(ctx, policySetNamespacedName, policySet)
-					if err != nil {
-						log.V(2).Info("Failed to retrieve policyset referenced in placementbinding",
-							"policySetName", subject.Name, "error", err)
-
-						continue
-					}
-					policies := policySet.Spec.Policies
-					for _, plc := range policies {
-						log.V(2).Info("Found reconciliation request from policyset placement bindng",
-							"policySetName", subject.Name, "policyName", string(plc))
-						request := reconcile.Request{NamespacedName: types.NamespacedName{
-							Name:      string(plc),
-							Namespace: object.GetNamespace(),
-						}}
-						result = append(result, request)
-					}
-				}
-			}
-		}
-
-		return result
+		return common.GetPoliciesInPlacementBinding(ctx, c, pb)
 	}
 }