diff --git a/controllers/common/common.go b/controllers/common/common.go index 304ff60e..046316f0 100644 --- a/controllers/common/common.go +++ b/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 diff --git a/controllers/policyset/placementBindingPredicate.go b/controllers/policyset/placementBindingPredicate.go index 5c8a1d26..9b602bdb 100644 --- a/controllers/policyset/placementBindingPredicate.go +++ b/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) }, } 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/placementBindingMapper.go b/controllers/propagator/placementBindingMapper.go index 96b56715..9d68c301 100644 --- a/controllers/propagator/placementBindingMapper.go +++ b/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) } } diff --git a/controllers/propagator/placementBindingPredicate.go b/controllers/propagator/placementBindingPredicate.go index 4c3cd78a..04682da9 100644 --- a/controllers/propagator/placementBindingPredicate.go +++ b/controllers/propagator/placementBindingPredicate.go @@ -19,19 +19,18 @@ var pbPredicateFuncs = predicate.Funcs{ //nolint:forcetypeassert pbObjOld := e.ObjectOld.(*policiesv1.PlacementBinding) - return common.IsPbForPoicy(pbObjNew) || common.IsPbForPoicy(pbObjOld) || - common.IsPbForPoicySet(pbObjNew) || common.IsPbForPoicySet(pbObjOld) + return common.IsForPolicyOrPolicySet(pbObjNew) || common.IsForPolicyOrPolicySet(pbObjOld) }, CreateFunc: func(e event.CreateEvent) bool { //nolint:forcetypeassert pbObj := e.Object.(*policiesv1.PlacementBinding) - return common.IsPbForPoicy(pbObj) || common.IsPbForPoicySet(pbObj) + return common.IsForPolicyOrPolicySet(pbObj) }, DeleteFunc: func(e event.DeleteEvent) bool { //nolint:forcetypeassert pbObj := e.Object.(*policiesv1.PlacementBinding) - return common.IsPbForPoicy(pbObj) || common.IsPbForPoicySet(pbObj) + return common.IsForPolicyOrPolicySet(pbObj) }, } diff --git a/controllers/propagator/placementDecisionMapper.go b/controllers/propagator/placementDecisionMapper.go index 9a1e36ae..21f4e35c 100644 --- a/controllers/propagator/placementDecisionMapper.go +++ b/controllers/propagator/placementDecisionMapper.go @@ -6,14 +6,13 @@ package propagator import ( "context" - "k8s.io/apimachinery/pkg/types" clusterv1beta1 "open-cluster-management.io/api/cluster/v1beta1" "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 placementDecisionMapper(c client.Client) handler.MapFunc { @@ -40,52 +39,14 @@ func placementDecisionMapper(c client.Client) handler.MapFunc { } var result []reconcile.Request - // loop through pb to find if current placement is used for policy + // loop through pbs and collect policies from each matching one. for _, pb := range pbList.Items { if pb.PlacementRef.APIGroup != clusterv1beta1.SchemeGroupVersion.Group || pb.PlacementRef.Kind != "Placement" || pb.PlacementRef.Name != placementName { continue } - // found matching placement in pb -- check if it is for policy or policyset - subjects := pb.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 a placement decision", - "policyName", subject.Name) - // generate reconcile request for policy referenced by pb - 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, "placementBindingName", pb.GetName(), "error", err) - - continue - } - policies := policySet.Spec.Policies - for _, plc := range policies { - log.V(2).Info("Found reconciliation request from a policyset placement decision", - "policySetName", subject.Name, "policyName", string(plc)) - request := reconcile.Request{NamespacedName: types.NamespacedName{ - Name: string(plc), - Namespace: object.GetNamespace(), - }} - result = append(result, request) - } - } - } - } + result = append(result, common.GetPoliciesInPlacementBinding(ctx, c, &pb)...) } return result diff --git a/controllers/propagator/placementRuleMapper.go b/controllers/propagator/placementRuleMapper.go index 5900b2f9..bf5f1d75 100644 --- a/controllers/propagator/placementRuleMapper.go +++ b/controllers/propagator/placementRuleMapper.go @@ -6,14 +6,13 @@ package propagator import ( "context" - "k8s.io/apimachinery/pkg/types" 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/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 placementRuleMapper(c client.Client) handler.MapFunc { @@ -32,51 +31,14 @@ func placementRuleMapper(c client.Client) handler.MapFunc { } var result []reconcile.Request - // loop through pb to find if current placementrule is used for policy + // loop through pbs and collect policies from each matching one. for _, pb := range pbList.Items { - // found matching placement rule in pb - if pb.PlacementRef.APIGroup == appsv1.SchemeGroupVersion.Group && - pb.PlacementRef.Kind == "PlacementRule" && pb.PlacementRef.Name == object.GetName() { - // check if it is for policy - subjects := pb.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 rule", "policyName", - subject.Name) - // generate reconcile request for policy referenced by pb - 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, "placementBindingName", pb.GetName(), "error", err) - - continue - } - policies := policySet.Spec.Policies - for _, plc := range policies { - log.V(2).Info("Found reconciliation request from a policyset placement rule", - "policySetName", subject.Name, "policyName", string(plc)) - request := reconcile.Request{NamespacedName: types.NamespacedName{ - Name: string(plc), - Namespace: object.GetNamespace(), - }} - result = append(result, request) - } - } - } - } + if pb.PlacementRef.APIGroup != appsv1.SchemeGroupVersion.Group || + pb.PlacementRef.Kind != "PlacementRule" || pb.PlacementRef.Name != object.GetName() { + continue } + + result = append(result, common.GetPoliciesInPlacementBinding(ctx, c, &pb)...) } return result diff --git a/controllers/propagator/policy_controller.go b/controllers/propagator/policy_controller.go index 5b91c93c..f841906d 100644 --- a/controllers/propagator/policy_controller.go +++ b/controllers/propagator/policy_controller.go @@ -102,7 +102,7 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request ctrl.Request) lock, _ := r.RootPolicyLocks.LoadOrStore(request.NamespacedName, &sync.Mutex{}) lock.(*sync.Mutex).Lock() - defer func() { lock.(*sync.Mutex).Unlock() }() + defer lock.(*sync.Mutex).Unlock() // Set the hub template watch metric after reconcile defer func() { diff --git a/controllers/propagator/propagation.go b/controllers/propagator/propagation.go index ae93b2fe..65a5937e 100644 --- a/controllers/propagator/propagation.go +++ b/controllers/propagator/propagation.go @@ -262,9 +262,7 @@ func handleDecisionWrapper( ) log.V(1).Info("Handling the decision") - instanceCopy := *instance.DeepCopy() - - templateRefObjs, err := decisionHandler.handleDecision(&instanceCopy, decision) + templateRefObjs, err := decisionHandler.handleDecision(instance, decision) if err == nil { log.V(1).Info("Replicated the policy") } @@ -290,45 +288,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 instance.Spec.Disabled { - // Only handle the first match in pb.spec.subjects - return nil, placements, 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, + } + + 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 + } + } + + // If there are no placements, then the PlacementBinding is not for this Policy. + if len(placements) == 0 { + return nil, nil, nil } - return decisions, placements, 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 @@ -352,23 +393,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 @@ -389,14 +431,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 @@ -423,7 +474,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 @@ -683,186 +734,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) @@ -876,7 +747,6 @@ func (r *PolicyReconciler) handleDecision( log := log.WithValues( "policyName", rootPlc.GetName(), "policyNamespace", rootPlc.GetNamespace(), - "replicatePolicyName", common.FullNameForPolicy(rootPlc), "replicatedPolicyNamespace", decision.ClusterNamespace, ) // retrieve replicated policy in cluster namespace @@ -1229,30 +1099,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 } } diff --git a/controllers/rootpolicystatus/root_policy_status_controller.go b/controllers/rootpolicystatus/root_policy_status_controller.go index ac186c8f..353bfe6c 100644 --- a/controllers/rootpolicystatus/root_policy_status_controller.go +++ b/controllers/rootpolicystatus/root_policy_status_controller.go @@ -74,7 +74,7 @@ func (r *RootPolicyStatusReconciler) Reconcile(ctx context.Context, request ctrl lock, _ := r.RootPolicyLocks.LoadOrStore(request.NamespacedName, &sync.Mutex{}) lock.(*sync.Mutex).Lock() - defer func() { lock.(*sync.Mutex).Unlock() }() + defer lock.(*sync.Mutex).Unlock() rootPolicy := &policiesv1.Policy{}