feat: add allowedPropagatingAPIs option for resource selection

pkg/controllers/clusterresourceplacement/placement_controllerv1alpha1.go

@@ -49,7 +49,7 @@ type Reconciler struct {
 	// It's only needed by v1beta1 APIs.
 	UncachedReader client.Reader
 
-	// ResourceConfig contains all the api resources that we won't select based on allowed or skipped propagating apis option.
+	// ResourceConfig contains all the API resources that we won't select based on allowed or skipped propagating APIs ption.
 	ResourceConfig *utils.ResourceConfig
 
 	// SkippedNamespaces contains the namespaces that we should not propagate.

pkg/controllers/clusterresourceplacement/resource_selector.go

@@ -289,7 +289,7 @@ func (r *Reconciler) fetchAllResourcesInOneNamespace(namespaceName string, place
 
 // shouldSelectResource returns whether a resource should be selected for propagation.
 func (r *Reconciler) shouldSelectResource(gvr schema.GroupVersionResource) bool {
-	if r.ResourceConfig.IsEmpty() {
+	if r.ResourceConfig == nil {
 		return true
 	}
 	gvks, err := r.RestMapper.KindsFor(gvr)

pkg/resourcewatcher/change_dector.go

@@ -64,7 +64,7 @@ type ChangeDetector struct {
 	// InformerManager manages all the dynamic informers created by the discovery client
 	InformerManager informer.Manager
 
-	// ResourceConfig contains all the api resources that we won't select based on the allowed or skipped propagating apis option.
+	// ResourceConfig contains all the API resources that we won't select based on the allowed or skipped propagating APIs option.
 	ResourceConfig *utils.ResourceConfig
 
 	// SkippedNamespaces contains all the namespaces that we won't select
@@ -186,7 +186,7 @@ func (d *ChangeDetector) discoverResources(dynamicResourceEventHandler cache.Res
 
 // gvrDisabled returns whether GroupVersionResource is disabled.
 func (d *ChangeDetector) shouldWatchResource(gvr schema.GroupVersionResource) bool {
-	if d.ResourceConfig.IsEmpty() {
+	if d.ResourceConfig == nil {
 		return true
 	}
 

pkg/utils/apiresources.go

@@ -58,7 +58,7 @@ type ResourceConfig struct {
 }
 
 // NewResourceConfig creates an empty ResourceConfig with an allow list flag.
-// If the resourceConfig is not an allowlist, it creates a default skipped propagation APIs list.
+// If the resourceConfig is not an allowlist, it creates a default skipped propagating APIs list.
 func NewResourceConfig(isAllowList bool) *ResourceConfig {
 	r := &ResourceConfig{
 		groups:            map[string]struct{}{},
@@ -171,7 +171,7 @@ func (r *ResourceConfig) parseSingle(token string) error {
 }
 
 // IsResourceDisabled returns whether a given GroupVersionKind is disabled.
-// A gkv is disabled if its group or group version is disabled.
+// A gvk is disabled if its group or group version is disabled.
 func (r *ResourceConfig) IsResourceDisabled(gvk schema.GroupVersionKind) bool {
 	isConfigured := r.isResourceConfigured(gvk)
 	if r.isAllowList {
@@ -198,22 +198,17 @@ func (r *ResourceConfig) isResourceConfigured(gvk schema.GroupVersionKind) bool
 	return false
 }
 
-// AddGroup to store group in the resource config.
+// AddGroup stores a group in the resource config.
 func (r *ResourceConfig) AddGroup(g string) {
 	r.groups[g] = struct{}{}
 }
 
-// AddGroupVersion to store group version in the resource config.
+// AddGroupVersion stores a group version in the resource config.
 func (r *ResourceConfig) AddGroupVersion(gv schema.GroupVersion) {
 	r.groupVersions[gv] = struct{}{}
 }
 
-// AddGroupVersionKind to store GroupVersionKind in the resource config.
+// AddGroupVersionKind stores a GroupVersionKind in the resource config.
 func (r *ResourceConfig) AddGroupVersionKind(gvk schema.GroupVersionKind) {
 	r.groupVersionKinds[gvk] = struct{}{}
 }
-
-// IsEmpty returns whether the ResourceConfig is empty.
-func (r *ResourceConfig) IsEmpty() bool {
-	return len(r.groups) == 0 && len(r.groupVersions) == 0 && len(r.groupVersionKinds) == 0
-}

pkg/utils/apiresources_test.go

@@ -71,22 +71,9 @@ func TestResourceConfigGVKParse(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		r := NewResourceConfig(false)
-		if err := r.Parse(test.input); err != nil {
-			t.Fatalf("Unexpected error: %v", err)
-		}
-		for i, o := range test.disabled {
-			ok := r.IsResourceDisabled(o)
-			if !ok {
-				t.Errorf("%d: unexpected error: %v", i, o)
-			}
-		}
-		for i, o := range test.enabled {
-			ok := r.IsResourceDisabled(o)
-			if ok {
-				t.Errorf("%d: unexpected error: %v", i, o)
-			}
-		}
+		r := newTestResourceConfig(t, false, test.input)
+		checkIfResourcesAreDisabledInConfig(t, r, test.disabled)
+		checkIfResourcesAreEnabledInConfig(t, r, test.enabled)
 	}
 }
 
@@ -140,22 +127,9 @@ func TestResourceConfigGVParse(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		r := NewResourceConfig(false)
-		if err := r.Parse(test.input); err != nil {
-			t.Fatalf("Unexpected error: %v", err)
-		}
-		for i, o := range test.disabled {
-			ok := r.IsResourceDisabled(o)
-			if !ok {
-				t.Errorf("%d: unexpected error: %v", i, o)
-			}
-		}
-		for i, o := range test.enabled {
-			ok := r.IsResourceDisabled(o)
-			if ok {
-				t.Errorf("%d: unexpected error: %v", i, o)
-			}
-		}
+		r := newTestResourceConfig(t, false, test.input)
+		checkIfResourcesAreDisabledInConfig(t, r, test.disabled)
+		checkIfResourcesAreEnabledInConfig(t, r, test.enabled)
 	}
 }
 
@@ -209,22 +183,9 @@ func TestResourceConfigGroupParse(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		r := NewResourceConfig(false)
-		if err := r.Parse(test.input); err != nil {
-			t.Fatalf("Unexpected error: %v", err)
-		}
-		for i, o := range test.disabled {
-			ok := r.IsResourceDisabled(o)
-			if !ok {
-				t.Errorf("%d: unexpected error: %v", i, o)
-			}
-		}
-		for i, o := range test.enabled {
-			ok := r.IsResourceDisabled(o)
-			if ok {
-				t.Errorf("%d: unexpected error: %v", i, o)
-			}
-		}
+		r := newTestResourceConfig(t, false, test.input)
+		checkIfResourcesAreDisabledInConfig(t, r, test.disabled)
+		checkIfResourcesAreEnabledInConfig(t, r, test.enabled)
 	}
 }
 
@@ -302,22 +263,9 @@ func TestResourceConfigMixedParse(t *testing.T) {
 	}
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			r := NewResourceConfig(test.isAllowList)
-			if err := r.Parse(input); err != nil {
-				t.Fatalf("Parse() returned error: %v", err)
-			}
-
-			for i, o := range test.disabled {
-				if ok := r.IsResourceDisabled(o); !ok {
-					t.Errorf("%d: expected resource to be disabled : %v", i, o)
-				}
-			}
-
-			for i, o := range test.enabled {
-				if ok := r.IsResourceDisabled(o); ok {
-					t.Errorf("%d: expected resource to be enabled : %v", i, o)
-				}
-			}
+			r := newTestResourceConfig(t, test.isAllowList, input)
+			checkIfResourcesAreDisabledInConfig(t, r, test.disabled)
+			checkIfResourcesAreEnabledInConfig(t, r, test.enabled)
 		})
 	}
 }
@@ -378,46 +326,40 @@ func TestDefaultResourceConfigGroupVersionKindParse(t *testing.T) {
 	}
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			r := NewResourceConfig(test.isAllowList)
-			if err := r.Parse(""); err != nil {
-				t.Fatalf("Parse() returned error: %v", err)
-			}
-			for i, o := range test.disabled {
-				if ok := r.IsResourceDisabled(o); !ok {
-					t.Errorf("%d: expected resource to be disabled : %v", i, o)
-				}
-			}
-
-			for i, o := range test.enabled {
-				if ok := r.IsResourceDisabled(o); ok {
-					t.Errorf("%d: expected resource to be enabled : %v", i, o)
-				}
-			}
+			r := newTestResourceConfig(t, test.isAllowList, "")
+			checkIfResourcesAreDisabledInConfig(t, r, test.disabled)
+			checkIfResourcesAreEnabledInConfig(t, r, test.enabled)
 		})
 	}
 }
 
-func TestResourceConfigIsEmpty(t *testing.T) {
-	tests := []struct {
-		input string
-		want  bool
-	}{
-		{
-			input: "",
-			want:  true,
-		},
-		{
-			input: "v1/Node,Pod;networking.k8s.io;apps/v1;authorization.k8s.io/v1/SelfSubjectRulesReview",
-			want:  false,
-		},
+// newTestResourceConfig creates a new ResourceConfig for either allow or disable list
+// for testing with resources parsed from the input string. If the input string is not
+// valid, it will fail the test.
+func newTestResourceConfig(t *testing.T, isAllowList bool, input string) *ResourceConfig {
+	r := NewResourceConfig(isAllowList)
+	if err := r.Parse(input); err != nil {
+		t.Fatalf("Parse() returned error: %v", err)
 	}
-	for _, test := range tests {
-		r := NewResourceConfig(true)
-		if err := r.Parse(test.input); err != nil {
-			t.Fatalf("Unexpected error: %v", err)
+	return r
+}
+
+// checkIfResourcesAreDisabledInConfig checks if the resources are disabled in the ResourceConfig.
+// If the check fails, it will fail the test.
+func checkIfResourcesAreDisabledInConfig(t *testing.T, r *ResourceConfig, resources []schema.GroupVersionKind) {
+	for _, o := range resources {
+		if ok := r.IsResourceDisabled(o); !ok {
+			t.Errorf("IsResourceDisabled(%v) = false, want true", o)
 		}
-		if got := r.IsEmpty(); got != test.want {
-			t.Errorf("Unexpected result: %v", got)
+	}
+}
+
+// checkIfResourcesAreEnabledInConfig checks if the resources are enabled in the ResourceConfig.
+// If the check fails, it will fail the test.
+func checkIfResourcesAreEnabledInConfig(t *testing.T, r *ResourceConfig, resources []schema.GroupVersionKind) {
+	for _, o := range resources {
+		if ok := r.IsResourceDisabled(o); ok {
+			t.Errorf("IsResourceDisabled(%v) = true, want false", o)
 		}
 	}
 }