feat: resource selection struct and selection related functions (kubefleet-dev#418)

Author: britaniar

pkg/controllers/placement/resource_selector.go

@@ -294,7 +294,7 @@ func (r *Reconciler) shouldPropagateObj(namespace, placementName string, obj run
 		return false, nil
 	}
 
-	shouldInclude, err := utils.ShouldPropagateObj(r.InformerManager, uObj, r.EnableWorkload)
+	shouldInclude, err := controller.ShouldPropagateObj(r.InformerManager, uObj, r.EnableWorkload)
 	if err != nil {
 		klog.ErrorS(err, "Cannot determine if we should propagate an object", "namespace", namespace, "placement", placementName, "object", uObjKObj)
 		return false, err

pkg/resourcewatcher/change_dector.go

@@ -168,7 +168,7 @@ func (d *ChangeDetector) dynamicResourceFilter(obj interface{}) bool {
 	}
 
 	if unstructuredObj, ok := obj.(*unstructured.Unstructured); ok {
-		shouldPropagate, err := utils.ShouldPropagateObj(d.InformerManager, unstructuredObj.DeepCopy(), d.EnableWorkload)
+		shouldPropagate, err := controller.ShouldPropagateObj(d.InformerManager, unstructuredObj.DeepCopy(), d.EnableWorkload)
 		if err != nil || !shouldPropagate {
 			klog.V(5).InfoS("Skip watching resource in namespace", "namespace", cwKey.Namespace,
 				"group", cwKey.Group, "version", cwKey.Version, "kind", cwKey.Kind, "object", cwKey.Name)

pkg/utils/common.go

@@ -27,18 +27,14 @@ import (
 	appv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
-	discoveryv1 "k8s.io/api/discovery/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	policyv1 "k8s.io/api/policy/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	schedulingv1 "k8s.io/api/scheduling/v1"
 	storagev1 "k8s.io/api/storage/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/util/retry"
@@ -50,8 +46,6 @@ import (
 	placementv1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1"
 	placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1"
 	"github.com/kubefleet-dev/kubefleet/pkg/utils/condition"
-	"github.com/kubefleet-dev/kubefleet/pkg/utils/controller"
-	"github.com/kubefleet-dev/kubefleet/pkg/utils/informer"
 )
 
 const (
@@ -505,71 +499,6 @@ func CheckCRDInstalled(discoveryClient discovery.DiscoveryInterface, gvk schema.
 	return err
 }
 
-// ShouldPropagateObj decides if one should propagate the object.
-// PVCs are only propagated when enableWorkload is false (workloads not allowed on hub).
-func ShouldPropagateObj(informerManager informer.Manager, uObj *unstructured.Unstructured, enableWorkload bool) (bool, error) {
-	// TODO:  add more special handling for different resource kind
-	switch uObj.GroupVersionKind() {
-	case appv1.SchemeGroupVersion.WithKind(ReplicaSetKind):
-		// Skip ReplicaSets if they are managed by Deployments (have owner references).
-		// Standalone ReplicaSets (without owners) can be propagated.
-		if len(uObj.GetOwnerReferences()) > 0 {
-			return false, nil
-		}
-	case appv1.SchemeGroupVersion.WithKind("ControllerRevision"):
-		// Skip ControllerRevisions if they are managed by DaemonSets/StatefulSets (have owner references).
-		// Standalone ControllerRevisions (without owners) can be propagated.
-		if len(uObj.GetOwnerReferences()) > 0 {
-			return false, nil
-		}
-	case corev1.SchemeGroupVersion.WithKind(ConfigMapKind):
-		// Skip the built-in custom CA certificate created in the namespace.
-		if uObj.GetName() == "kube-root-ca.crt" {
-			return false, nil
-		}
-	case corev1.SchemeGroupVersion.WithKind("ServiceAccount"):
-		// Skip the default service account created in the namespace.
-		if uObj.GetName() == "default" {
-			return false, nil
-		}
-	case corev1.SchemeGroupVersion.WithKind("Secret"):
-		// The secret, with type 'kubernetes.io/service-account-token', is created along with `ServiceAccount` should be
-		// prevented from propagating.
-		var secret corev1.Secret
-		if err := runtime.DefaultUnstructuredConverter.FromUnstructured(uObj.Object, &secret); err != nil {
-			return false, controller.NewUnexpectedBehaviorError(fmt.Errorf("failed to convert a secret object %s in namespace %s: %w", uObj.GetName(), uObj.GetNamespace(), err))
-		}
-		if secret.Type == corev1.SecretTypeServiceAccountToken {
-			return false, nil
-		}
-	case corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"):
-		// Skip PersistentVolumeClaims by default to avoid conflicts with the PVCs created by statefulset controller.
-		// This only happens if the workloads are allowed to run on the hub cluster.
-		if enableWorkload {
-			return false, nil
-		}
-	case corev1.SchemeGroupVersion.WithKind("Endpoints"):
-		// we assume that all endpoints with the same name of a service is created by the service controller
-		if _, err := informerManager.Lister(ServiceGVR).ByNamespace(uObj.GetNamespace()).Get(uObj.GetName()); err != nil {
-			if apierrors.IsNotFound(err) {
-				// there is no service of the same name as the end point,
-				// we assume that this endpoint is created by the user
-				return true, nil
-			}
-			return false, controller.NewAPIServerError(true, fmt.Errorf("failed to get the service %s in namespace %s: %w", uObj.GetName(), uObj.GetNamespace(), err))
-		}
-		// we find a service of the same name as the endpoint, we assume it's created by the service
-		return false, nil
-	case discoveryv1.SchemeGroupVersion.WithKind("EndpointSlice"):
-		// all EndpointSlice created by the EndpointSlice controller has a managed by label
-		if _, exist := uObj.GetLabels()[discoveryv1.LabelManagedBy]; exist {
-			// do not propagate hub cluster generated endpoint slice
-			return false, nil
-		}
-	}
-	return true, nil
-}
-
 // IsReservedNamespace indicates if an argued namespace is reserved.
 func IsReservedNamespace(namespace string) bool {
 	return strings.HasPrefix(namespace, fleetPrefix) || strings.HasPrefix(namespace, kubePrefix)
@@ -774,7 +703,7 @@ var LessFuncDiffedResourcePlacementsV1 = func(a, b placementv1.DiffedResourcePla
 	return aStr < bStr
 }
 
-// LessFuncCondition is a less function for sorting conditions based on its types.
+// LessFuncConditionByType is a less function for sorting conditions based on its types.
 var LessFuncConditionByType = func(a, b metav1.Condition) bool {
 	return a.Type < b.Type
 }

pkg/utils/common_test.go

@@ -6,7 +6,6 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/utils/ptr"
 
 	fleetv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1"
@@ -1190,221 +1189,3 @@ func TestIsDiffedResourcePlacementEqual(t *testing.T) {
 		})
 	}
 }
-
-func TestShouldPropagateObj(t *testing.T) {
-	tests := []struct {
-		name            string
-		obj             map[string]interface{}
-		ownerReferences []metav1.OwnerReference
-		enableWorkload  bool
-		want            bool
-	}{
-		{
-			name: "standalone replicaset without ownerReferences should propagate",
-			obj: map[string]interface{}{
-				"apiVersion": "apps/v1",
-				"kind":       "ReplicaSet",
-				"metadata": map[string]interface{}{
-					"name":      "standalone-rs",
-					"namespace": "default",
-				},
-			},
-			ownerReferences: nil,
-			enableWorkload:  true,
-			want:            true,
-		},
-		{
-			name: "standalone replicaset without ownerReferences should propagate if workload is disabled",
-			obj: map[string]interface{}{
-				"apiVersion": "apps/v1",
-				"kind":       "ReplicaSet",
-				"metadata": map[string]interface{}{
-					"name":      "standalone-rs",
-					"namespace": "default",
-				},
-			},
-			ownerReferences: nil,
-			enableWorkload:  false,
-			want:            true,
-		},
-		{
-			name: "standalone pod without ownerReferences should propagate",
-			obj: map[string]interface{}{
-				"apiVersion": "v1",
-				"kind":       "Pod",
-				"metadata": map[string]interface{}{
-					"name":      "standalone-pod",
-					"namespace": "default",
-				},
-			},
-			ownerReferences: nil,
-			enableWorkload:  true,
-			want:            true,
-		},
-		{
-			name: "replicaset with deployment owner should NOT propagate",
-			obj: map[string]interface{}{
-				"apiVersion": "apps/v1",
-				"kind":       "ReplicaSet",
-				"metadata": map[string]interface{}{
-					"name":      "test-deploy-abc123",
-					"namespace": "default",
-				},
-			},
-			ownerReferences: []metav1.OwnerReference{
-				{
-					APIVersion: "apps/v1",
-					Kind:       "Deployment",
-					Name:       "test-deploy",
-					UID:        "12345",
-				},
-			},
-			enableWorkload: true,
-			want:           false,
-		},
-		{
-			name: "pod owned by replicaset - passes ShouldPropagateObj but filtered by resource config",
-			obj: map[string]interface{}{
-				"apiVersion": "v1",
-				"kind":       "Pod",
-				"metadata": map[string]interface{}{
-					"name":      "test-deploy-abc123-xyz",
-					"namespace": "default",
-				},
-			},
-			ownerReferences: []metav1.OwnerReference{
-				{
-					APIVersion: "apps/v1",
-					Kind:       "ReplicaSet",
-					Name:       "test-deploy-abc123",
-					UID:        "67890",
-				},
-			},
-			enableWorkload: false,
-			want:           true, // ShouldPropagateObj doesn't filter Pods - they're filtered by NewResourceConfig
-		},
-		{
-			name: "controllerrevision owned by daemonset should NOT propagate",
-			obj: map[string]interface{}{
-				"apiVersion": "apps/v1",
-				"kind":       "ControllerRevision",
-				"metadata": map[string]interface{}{
-					"name":      "test-ds-7b9848797f",
-					"namespace": "default",
-				},
-			},
-			ownerReferences: []metav1.OwnerReference{
-				{
-					APIVersion: "apps/v1",
-					Kind:       "DaemonSet",
-					Name:       "test-ds",
-					UID:        "abcdef",
-				},
-			},
-			enableWorkload: false,
-			want:           false,
-		},
-		{
-			name: "controllerrevision owned by statefulset should NOT propagate",
-			obj: map[string]interface{}{
-				"apiVersion": "apps/v1",
-				"kind":       "ControllerRevision",
-				"metadata": map[string]interface{}{
-					"name":      "test-ss-7878b4b446",
-					"namespace": "default",
-				},
-			},
-			ownerReferences: []metav1.OwnerReference{
-				{
-					APIVersion: "apps/v1",
-					Kind:       "StatefulSet",
-					Name:       "test-ss",
-					UID:        "fedcba",
-				},
-			},
-			enableWorkload: false,
-			want:           false,
-		},
-		{
-			name: "standalone controllerrevision without owner should propagate",
-			obj: map[string]interface{}{
-				"apiVersion": "apps/v1",
-				"kind":       "ControllerRevision",
-				"metadata": map[string]interface{}{
-					"name":      "custom-revision",
-					"namespace": "default",
-				},
-			},
-			ownerReferences: nil,
-			enableWorkload:  false,
-			want:            true,
-		},
-		{
-			name: "PVC should propagate when workload is disabled",
-			obj: map[string]interface{}{
-				"apiVersion": "v1",
-				"kind":       "PersistentVolumeClaim",
-				"metadata": map[string]interface{}{
-					"name":      "test-pvc",
-					"namespace": "default",
-				},
-			},
-			ownerReferences: nil,
-			enableWorkload:  false,
-			want:            true,
-		},
-		{
-			name: "PVC should NOT propagate when workload is enabled",
-			obj: map[string]interface{}{
-				"apiVersion": "v1",
-				"kind":       "PersistentVolumeClaim",
-				"metadata": map[string]interface{}{
-					"name":      "test-pvc",
-					"namespace": "default",
-				},
-			},
-			ownerReferences: nil,
-			enableWorkload:  true,
-			want:            false,
-		},
-		{
-			name: "PVC with ownerReferences should NOT propagate when workload is enabled",
-			obj: map[string]interface{}{
-				"apiVersion": "v1",
-				"kind":       "PersistentVolumeClaim",
-				"metadata": map[string]interface{}{
-					"name":      "data-statefulset-0",
-					"namespace": "default",
-				},
-			},
-			ownerReferences: []metav1.OwnerReference{
-				{
-					APIVersion: "apps/v1",
-					Kind:       "StatefulSet",
-					Name:       "statefulset",
-					UID:        "sts-uid",
-				},
-			},
-			enableWorkload: true,
-			want:           false,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			uObj := &unstructured.Unstructured{Object: tt.obj}
-			if tt.ownerReferences != nil {
-				uObj.SetOwnerReferences(tt.ownerReferences)
-			}
-
-			got, err := ShouldPropagateObj(nil, uObj, tt.enableWorkload)
-			if err != nil {
-				t.Errorf("ShouldPropagateObj() error = %v", err)
-				return
-			}
-			if got != tt.want {
-				t.Errorf("ShouldPropagateObj() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}