diff --git a/controllers/common/common.go b/controllers/common/common.go index af470152..046316f0 100644 --- a/controllers/common/common.go +++ b/controllers/common/common.go @@ -10,7 +10,6 @@ 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" @@ -150,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) + + for _, item := range list.Items { + for _, cluster := range item.Status.Decisions { + decisions = append(decisions, appsv1.PlacementDecision{ + ClusterName: cluster.ClusterName, + ClusterNamespace: cluster.ClusterName, + }) + } + } -// 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{} + 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) + } - 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 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 diff --git a/controllers/policyset/policyset_controller.go b/controllers/policyset/policyset_controller.go index 9c6cb1fe..96d066da 100644 --- a/controllers/policyset/policyset_controller.go +++ b/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). diff --git a/controllers/propagator/propagation.go b/controllers/propagator/propagation.go index 538e34e5..99f22525 100644 --- a/controllers/propagator/propagation.go +++ b/controllers/propagator/propagation.go @@ -289,45 +289,88 @@ func (set decisionSet) namespaces() []string { // getPolicyPlacementDecisions retrieves the placement decisions for a input // placement binding when the policy is bound within it. func (r *PolicyReconciler) getPolicyPlacementDecisions( - instance *policiesv1.Policy, pb policiesv1.PlacementBinding, -) ([]appsv1.PlacementDecision, []*policiesv1.Placement, error) { - log := log.WithValues( - "policyName", instance.GetName(), - "policyNamespace", instance.GetNamespace(), - "placementBindingName", pb.GetName(), - "placementBidningNamespace", pb.GetNamespace(), - ) - var decisions []appsv1.PlacementDecision - var placements []*policiesv1.Placement - var err error - - subjects := pb.Subjects - for _, subject := range subjects { - if !(subject.APIGroup == policiesv1.SchemeGroupVersion.Group && - subject.Kind == policiesv1.Kind && - subject.Name == instance.GetName()) && !r.isPolicySetSubject(instance, subject) { + instance *policiesv1.Policy, pb *policiesv1.PlacementBinding, +) (decisions []appsv1.PlacementDecision, placements []*policiesv1.Placement, err error) { + if !common.HasValidPlacementRef(pb) { + return nil, nil, fmt.Errorf("placement binding %s/%s reference is not valid", pb.Name, pb.Namespace) + } + + policySubjectFound := false + policySetSubjects := make(map[string]struct{}) // a set, to prevent duplicates + + for _, subject := range pb.Subjects { + if subject.APIGroup != policiesv1.SchemeGroupVersion.Group { continue } - decisions, placements, err = getPlacementDecisions(r.Client, pb, instance) - if err != nil { - return nil, nil, err + switch subject.Kind { + case policiesv1.Kind: + if !policySubjectFound && subject.Name == instance.GetName() { + policySubjectFound = true + + placements = append(placements, &policiesv1.Placement{ + PlacementBinding: pb.GetName(), + }) + } + case policiesv1.PolicySetKind: + if _, exists := policySetSubjects[subject.Name]; !exists { + policySetSubjects[subject.Name] = struct{}{} + + if r.isPolicyInPolicySet(instance.GetName(), subject.Name, pb.GetNamespace()) { + placements = append(placements, &policiesv1.Placement{ + PlacementBinding: pb.GetName(), + PolicySet: subject.Name, + }) + } + } } + } + + if len(placements) == 0 { + // None of the subjects in the PlacementBinding were relevant to this Policy. + return nil, nil, nil + } + + // If the placementRef exists, then it needs to be added to the placement item + refNN := types.NamespacedName{ + Namespace: pb.GetNamespace(), + Name: pb.PlacementRef.Name, + } - if instance.Spec.Disabled { - // Only handle the first match in pb.spec.subjects - return nil, placements, nil + switch pb.PlacementRef.Kind { + case "PlacementRule": + plr := &appsv1.PlacementRule{} + if err := r.Get(context.TODO(), refNN, plr); err != nil && !k8serrors.IsNotFound(err) { + return nil, nil, fmt.Errorf("failed to check for PlacementRule '%v': %w", pb.PlacementRef.Name, err) } - if len(decisions) == 0 { - log.Info("No placement decisions to process on this policy") + for i := range placements { + placements[i].PlacementRule = plr.Name // will be empty if the PlacementRule was not found + } + case "Placement": + pl := &clusterv1beta1.Placement{} + if err := r.Get(context.TODO(), refNN, pl); err != nil && !k8serrors.IsNotFound(err) { + return nil, nil, fmt.Errorf("failed to check for Placement '%v': %w", pb.PlacementRef.Name, err) } - // Only handle the first match in pb.spec.subjects - break + for i := range placements { + placements[i].Placement = pl.Name // will be empty if the Placement was not found + } } - return decisions, placements, nil + // If there are no placements, then the PlacementBinding is not for this Policy. + if len(placements) == 0 { + return nil, nil, nil + } + + // If the policy is disabled, don't return any decisions, so that the policy isn't put on any clusters + if instance.Spec.Disabled { + return nil, placements, nil + } + + decisions, err = common.GetDecisions(r.Client, pb) + + return decisions, placements, err } // getAllClusterDecisions retrieves all cluster decisions for the input policy, taking into @@ -351,23 +394,24 @@ func (r *PolicyReconciler) getAllClusterDecisions( ) ( allClusterDecisions []clusterDecision, placements []*policiesv1.Placement, err error, ) { - var pbsWithSubFilter []policiesv1.PlacementBinding - allClusterDecisionsMap := map[appsv1.PlacementDecision]policiesv1.BindingOverrides{} // Process all placement bindings without subFilter for _, pb := range pbList.Items { if pb.SubFilter == policiesv1.Restricted { - pbsWithSubFilter = append(pbsWithSubFilter, pb) - continue } - plcDecisions, plcPlacements, err := r.getPolicyPlacementDecisions(instance, pb) + plcDecisions, plcPlacements, err := r.getPolicyPlacementDecisions(instance, &pb) if err != nil { return nil, nil, err } + if len(plcDecisions) == 0 { + log.Info("No placement decisions to process for this policy from this binding", + "policyName", instance.GetName(), "bindingName", pb.GetName()) + } + for _, decision := range plcDecisions { if overrides, ok := allClusterDecisionsMap[decision]; ok { // Found cluster in the decision map @@ -388,14 +432,23 @@ func (r *PolicyReconciler) getAllClusterDecisions( } // Process all placement bindings with subFilter:restricted - for _, pb := range pbsWithSubFilter { + for _, pb := range pbList.Items { + if pb.SubFilter != policiesv1.Restricted { + continue + } + foundInDecisions := false - plcDecisions, plcPlacements, err := r.getPolicyPlacementDecisions(instance, pb) + plcDecisions, plcPlacements, err := r.getPolicyPlacementDecisions(instance, &pb) if err != nil { return nil, nil, err } + if len(plcDecisions) == 0 { + log.Info("No placement decisions to process for this policy from this binding", + "policyName", instance.GetName(), "bindingName", pb.GetName()) + } + for _, decision := range plcDecisions { if overrides, ok := allClusterDecisionsMap[decision]; ok { // Found cluster in the decision map @@ -422,7 +475,7 @@ func (r *PolicyReconciler) getAllClusterDecisions( allClusterDecisions = append(allClusterDecisions, decision) } - return + return allClusterDecisions, placements, nil } // handleDecisions will get all the placement decisions based on the input policy and placement @@ -686,186 +739,6 @@ func (r *PolicyReconciler) handleRootPolicy(instance *policiesv1.Policy) error { return nil } -// getApplicationPlacements return the placements from an application -// lifecycle placementrule -func getApplicationPlacements( - c client.Client, pb policiesv1.PlacementBinding, instance *policiesv1.Policy, -) ([]*policiesv1.Placement, error) { - plr := &appsv1.PlacementRule{} - - 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 nil, err - } - - var placements []*policiesv1.Placement - - plcPlacementAdded := false - - for _, subject := range pb.Subjects { - if subject.Kind == policiesv1.PolicySetKind { - // retrieve policyset to see if policy is part of it - plcset := &policiesv1beta1.PolicySet{} - err := c.Get(context.TODO(), types.NamespacedName{ - Namespace: instance.GetNamespace(), - Name: subject.Name, - }, plcset) - // no error when not found - if err != nil && !k8serrors.IsNotFound(err) { - log.Error( - err, - "Failed to get the policyset", - "namespace", instance.GetNamespace(), - "name", subject.Name, - ) - - continue - } - - for _, plcName := range plcset.Spec.Policies { - if plcName == policiesv1beta1.NonEmptyString(instance.Name) { - // found matching policy in policyset, add placement to it - placement := &policiesv1.Placement{ - PlacementBinding: pb.GetName(), - PlacementRule: plr.GetName(), - PolicySet: subject.Name, - } - placements = append(placements, placement) - - break - } - } - } else if subject.Kind == policiesv1.Kind && subject.Name == instance.GetName() && !plcPlacementAdded { - placement := &policiesv1.Placement{ - PlacementBinding: pb.GetName(), - PlacementRule: plr.GetName(), - } - placements = append(placements, placement) - // should only add policy placement once in case placement binding subjects contains duplicated policies - plcPlacementAdded = true - } - } - - return placements, nil -} - -// getClusterPlacements return the placement decisions from an application -// lifecycle placementrule -func getClusterPlacements( - c client.Client, pb policiesv1.PlacementBinding, instance *policiesv1.Policy, -) ([]*policiesv1.Placement, error) { - log := log.WithValues("name", pb.PlacementRef.Name, "namespace", instance.GetNamespace()) - pl := &clusterv1beta1.Placement{} - - 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") - - return nil, err - } - - var placements []*policiesv1.Placement - - plcPlacementAdded := false - - for _, subject := range pb.Subjects { - if subject.Kind == policiesv1.PolicySetKind { - // retrieve policyset to see if policy is part of it - plcset := &policiesv1beta1.PolicySet{} - err := c.Get(context.TODO(), types.NamespacedName{ - Namespace: instance.GetNamespace(), - Name: subject.Name, - }, plcset) - // no error when not found - if err != nil && !k8serrors.IsNotFound(err) { - log.Error( - err, - "Failed to get the policyset", - "namespace", instance.GetNamespace(), - "name", subject.Name, - ) - - continue - } - - for _, plcName := range plcset.Spec.Policies { - if plcName == policiesv1beta1.NonEmptyString(instance.Name) { - // found matching policy in policyset, add placement to it - placement := &policiesv1.Placement{ - PlacementBinding: pb.GetName(), - Placement: pl.GetName(), - PolicySet: subject.Name, - } - placements = append(placements, placement) - - break - } - } - } else if subject.Kind == policiesv1.Kind && subject.Name == instance.GetName() && !plcPlacementAdded { - // add current Placement to placement, if not found no decisions will be found - placement := &policiesv1.Placement{ - PlacementBinding: pb.GetName(), - Placement: pl.GetName(), - } - placements = append(placements, placement) - // should only add policy placement once in case placement binding subjects contains duplicated policies - plcPlacementAdded = true - } - } - - return placements, nil -} - -// getPlacementDecisions gets the PlacementDecisions for a PlacementBinding -func getPlacementDecisions(c client.Client, pb policiesv1.PlacementBinding, - instance *policiesv1.Policy, -) ([]appsv1.PlacementDecision, []*policiesv1.Placement, 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, nil, err - } - - placement, err := getApplicationPlacements(c, pb, instance) - if err != nil { - return nil, nil, err - } - - return d, placement, 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, nil, err - } - - placement, err := getClusterPlacements(c, pb, instance) - if err != nil { - return nil, nil, err - } - - return d, placement, nil - } - - return nil, nil, fmt.Errorf("placement binding %s/%s reference is not valid", pb.Name, pb.Namespace) -} - // handleDecision puts the policy on the cluster, creating it or updating it as required, // including resolving hub templates. It will return an error if an API call fails; no // internal states will result in errors (eg invalid templates don't cause errors here) @@ -1231,30 +1104,24 @@ func isConfigurationPolicy(policyT *policiesv1.PolicyTemplate) bool { return jsonDef != nil && jsonDef["kind"] == "ConfigurationPolicy" } -func (r *PolicyReconciler) isPolicySetSubject(instance *policiesv1.Policy, subject policiesv1.Subject) bool { - log := log.WithValues("policyName", instance.GetName(), "policyNamespace", instance.GetNamespace()) - - if subject.APIGroup == policiesv1.SchemeGroupVersion.Group && - subject.Kind == policiesv1.PolicySetKind { - policySetNamespacedName := types.NamespacedName{ - Name: subject.Name, - Namespace: instance.GetNamespace(), - } +func (r *PolicyReconciler) isPolicyInPolicySet(policyName, policySetName, namespace string) bool { + log := log.WithValues("policyName", policyName, "policySetName", policySetName, "policyNamespace", namespace) - policySet := &policiesv1beta1.PolicySet{} + policySet := policiesv1beta1.PolicySet{} + setNN := types.NamespacedName{ + Name: policySetName, + Namespace: namespace, + } - err := r.Get(context.TODO(), policySetNamespacedName, policySet) - if err != nil { - log.Error(err, "Failed to get the policyset", "policySetName", subject.Name, "policyName", - instance.GetName(), "policyNamespace", instance.GetNamespace()) + if err := r.Get(context.TODO(), setNN, &policySet); err != nil { + log.Error(err, "Failed to get the policyset") - return false - } + return false + } - for _, plc := range policySet.Spec.Policies { - if string(plc) == instance.GetName() { - return true - } + for _, plc := range policySet.Spec.Policies { + if string(plc) == policyName { + return true } }