Keep only one resource config

cmd/hubagent/workload/setup.go

@@ -105,14 +105,12 @@ func SetupControllers(ctx context.Context, wg *sync.WaitGroup, mgr ctrl.Manager,
 		}
 	}
 
-	disabledResourceConfig := utils.NewResourceConfig(true)
-	if err := disabledResourceConfig.Parse(opts.SkippedPropagatingAPIs); err != nil {
-		// The program will never go here because the parameters have been checked
+	disabledResourceConfig := utils.NewResourceConfig(opts.AllowedPropagatingAPIs != "")
+	if err := disabledResourceConfig.Parse(opts.AllowedPropagatingAPIs); err != nil {
 		return err
 	}
-
-	allowedResourceConfig := utils.NewResourceConfig(false)
-	if err := allowedResourceConfig.Parse(opts.AllowedPropagatingAPIs); err != nil {
+	if err := disabledResourceConfig.Parse(opts.SkippedPropagatingAPIs); err != nil {
+		// The program will never go here because the parameters have been checked
 		return err
 	}
 
@@ -138,7 +136,6 @@ func SetupControllers(ctx context.Context, wg *sync.WaitGroup, mgr ctrl.Manager,
 		RestMapper:             mgr.GetRESTMapper(),
 		InformerManager:        dynamicInformerManager,
 		DisabledResourceConfig: disabledResourceConfig,
-		AllowedResourceConfig:  allowedResourceConfig,
 		SkippedNamespaces:      skippedNamespaces,
 		Scheme:                 mgr.GetScheme(),
 		UncachedReader:         mgr.GetAPIReader(),
@@ -276,7 +273,6 @@ func SetupControllers(ctx context.Context, wg *sync.WaitGroup, mgr ctrl.Manager,
 		MemberClusterPlacementController:           memberClusterPlacementController,
 		InformerManager:                            dynamicInformerManager,
 		DisabledResourceConfig:                     disabledResourceConfig,
-		AllowedResourceConfig:                      allowedResourceConfig,
 		SkippedNamespaces:                          skippedNamespaces,
 		ConcurrentClusterPlacementWorker:           opts.ConcurrentClusterPlacementSyncs,
 		ConcurrentResourceChangeWorker:             opts.ConcurrentResourceChangeSyncs,

pkg/controllers/clusterresourceplacement/placement_controllerv1alpha1.go

@@ -52,10 +52,6 @@ type Reconciler struct {
 	// DisabledResourceConfig contains all the api resources that we won't select.
 	DisabledResourceConfig *utils.ResourceConfig
 
-	// AllowedResourceConfig contains all the api resources that are selected for
-	// propagation. This is mutually exclusive with DisabledResourceConfig
-	AllowedResourceConfig *utils.ResourceConfig
-
 	// SkippedNamespaces contains the namespaces that we should not propagate.
 	SkippedNamespaces map[string]bool
 

pkg/controllers/clusterresourceplacement/resource_selector.go

@@ -79,7 +79,7 @@ func (r *Reconciler) gatherSelectedResource(placement string, selectors []fleetv
 			Kind:    selector.Kind,
 		}
 
-		if r.DisabledResourceConfig.IsResourceConfigured(gvk) {
+		if r.DisabledResourceConfig.IsResourceDisabled(gvk) {
 			klog.V(2).InfoS("Skip select resource", "group version kind", gvk.String())
 			continue
 		}
@@ -287,29 +287,18 @@ func (r *Reconciler) fetchAllResourcesInOneNamespace(namespaceName string, place
 	return resources, nil
 }
 
-// shouldSelectResource returns whether a resource should be selected for propagation
+// shouldSelectResource returns whether a resource should be selected for propagation.
 func (r *Reconciler) shouldSelectResource(gvr schema.GroupVersionResource) bool {
+	if r.DisabledResourceConfig.IsEmpty() {
+		return true
+	}
 	gvks, err := r.RestMapper.KindsFor(gvr)
 	if err != nil {
 		klog.ErrorS(err, "gvr(%s) transform failed: %v", gvr.String(), err)
 		return false
 	}
-
-	if !r.AllowedResourceConfig.IsEmpty() {
-		for _, gvk := range gvks {
-			if !r.AllowedResourceConfig.IsResourceConfigured(gvk) {
-				return false
-			}
-			klog.V(2).InfoS("Selecting Allowed Resource", "GVK", gvk.String())
-		}
-	}
-
-	if r.DisabledResourceConfig.IsEmpty() {
-		return true
-	}
-
 	for _, gvk := range gvks {
-		if r.DisabledResourceConfig.IsResourceConfigured(gvk) {
+		if r.DisabledResourceConfig.IsResourceDisabled(gvk) {
 			klog.V(2).InfoS("Skip watch resource", "group version kind", gvk.String())
 			return false
 		}

pkg/controllers/clusterresourceplacement/suite_test.go

@@ -102,8 +102,7 @@ var _ = BeforeSuite(func() {
 		Recorder:               mgr.GetEventRecorderFor(controllerName),
 		RestMapper:             mgr.GetRESTMapper(),
 		InformerManager:        informer.NewInformerManager(dynamicClient, 5*time.Minute, ctx.Done()),
-		DisabledResourceConfig: utils.NewResourceConfig(true),
-		AllowedResourceConfig:  utils.NewResourceConfig(false),
+		DisabledResourceConfig: utils.NewResourceConfig(false),
 		SkippedNamespaces: map[string]bool{
 			"default": true,
 		},

pkg/resourcewatcher/change_dector.go

@@ -67,9 +67,6 @@ type ChangeDetector struct {
 	// DisabledResourceConfig contains all the api resources that we won't select
 	DisabledResourceConfig *utils.ResourceConfig
 
-	// AllowedResourceConfig contains all the api resources that are watched for changes
-	AllowedResourceConfig *utils.ResourceConfig
-
 	// SkippedNamespaces contains all the namespaces that we won't select
 	SkippedNamespaces map[string]bool
 
@@ -187,31 +184,19 @@ func (d *ChangeDetector) discoverResources(dynamicResourceEventHandler cache.Res
 	d.InformerManager.Start()
 }
 
-// shouldWatchResource returns whether GroupVersionResource should be watched
-// If the resource is not in the AllowedResourceConfig, it will be skipped.
-// If the resource is in the DisabledResourceConfig, it will be skipped.
+// gvrDisabled returns whether GroupVersionResource is disabled.
 func (d *ChangeDetector) shouldWatchResource(gvr schema.GroupVersionResource) bool {
+	if d.DisabledResourceConfig.IsEmpty() {
+		return true
+	}
+
 	gvks, err := d.RESTMapper.KindsFor(gvr)
 	if err != nil {
 		klog.ErrorS(err, "gvr transform failed", "gvr", gvr.String())
 		return false
 	}
-
-	if !d.AllowedResourceConfig.IsEmpty() {
-		for _, gvk := range gvks {
-			if !d.AllowedResourceConfig.IsResourceConfigured(gvk) {
-				return false
-			}
-			klog.InfoS("Watching Allowed Resource", "GVK", gvk.String())
-		}
-	}
-
-	if d.DisabledResourceConfig.IsEmpty() {
-		return true
-	}
-
 	for _, gvk := range gvks {
-		if d.DisabledResourceConfig.IsResourceConfigured(gvk) {
+		if d.DisabledResourceConfig.IsResourceDisabled(gvk) {
 			klog.V(4).InfoS("Skip watch resource", "group version kind", gvk.String())
 			return false
 		}

pkg/utils/apiresources.go

@@ -53,43 +53,39 @@ type ResourceConfig struct {
 	groupVersions map[schema.GroupVersion]struct{}
 	// groupVersionKinds holds a collection of resource that should be considered.
 	groupVersionKinds map[schema.GroupVersionKind]struct{}
+	// isAllowList indicates whether the ResourceConfig is an allow list or not.
+	isAllowList bool
 }
 
-// NewResourceConfig creates an empty ResourceConfig with an optional
-// flag to add default resources that we would like to consider.
-// In case of DisableResourceConfig, we add fleet and built-in resources to the config.
-func NewResourceConfig(addDefaultResources bool) *ResourceConfig {
+// NewResourceConfig creates an empty ResourceConfig with an allow list flag.
+// if the resourceConfig is not an allowlist, we add fleet related resources
+// and default built-in resources to the config.
+func NewResourceConfig(isAllowList bool) *ResourceConfig {
 	r := &ResourceConfig{
 		groups:            map[string]struct{}{},
 		groupVersions:     map[schema.GroupVersion]struct{}{},
 		groupVersionKinds: map[schema.GroupVersionKind]struct{}{},
 	}
-	if addDefaultResources {
-		r.AddFleetRelatedResources()
-		r.AddBuiltInResources()
+	r.isAllowList = isAllowList
+	if r.isAllowList {
+		return r
 	}
+	// disable fleet related resource by default
+	r.DisableGroup(fleetv1alpha1.GroupVersion.Group)
+	r.DisableGroup(placementv1beta1.GroupVersion.Group)
+	r.DisableGroup(clusterv1beta1.GroupVersion.Group)
+	r.DisableGroupVersionKind(WorkGVK)
+
+	// disable the below built-in resources
+	r.DisableGroup(eventsv1.GroupName)
+	r.DisableGroup(coordv1.GroupName)
+	r.DisableGroup(metricsV1beta1.GroupName)
+	r.DisableGroupVersionKind(corev1PodGVK)
+	r.DisableGroupVersionKind(corev1NodeGVK)
+	r.DisableGroupVersionKind(serviceImportGVK)
 	return r
 }
 
-// AddFleetRelatedResources configures the ResourceConfig with fleet related resource.
-func (r *ResourceConfig) AddFleetRelatedResources() {
-	r.AddGroup(fleetv1alpha1.GroupVersion.Group)
-	r.AddGroup(placementv1beta1.GroupVersion.Group)
-	r.AddGroup(clusterv1beta1.GroupVersion.Group)
-	r.AddGroupVersionKind(WorkGVK)
-}
-
-// AddBuiltInResources configures the ResourceConfig with a set of built-in resources
-// such as events, coordination, metrics, corev1/pod, corev1/node, etc.
-func (r *ResourceConfig) AddBuiltInResources() {
-	r.AddGroup(eventsv1.GroupName)
-	r.AddGroup(coordv1.GroupName)
-	r.AddGroup(metricsV1beta1.GroupName)
-	r.AddGroupVersionKind(corev1PodGVK)
-	r.AddGroupVersionKind(corev1NodeGVK)
-	r.AddGroupVersionKind(serviceImportGVK)
-}
-
 // Parse parses the user inputs that provides apis as GVK, GV or Group.
 func (r *ResourceConfig) Parse(c string) error {
 	// default(empty) input
@@ -175,9 +171,19 @@ func (r *ResourceConfig) parseSingle(token string) error {
 	return nil
 }
 
-// IsResourceConfigured returns whether a given GroupVersionKind is found in the ResourceConfig.
+// IsResourceDisabled returns whether a given GroupVersionKind is disabled.
+// a gkv 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 {
+		return !isConfigured
+	}
+	return isConfigured
+}
+
+// isResourceConfigured returns whether a given GroupVersionKind is found in the ResourceConfig.
 // a gvk is configured if its group or group version is configured
-func (r *ResourceConfig) IsResourceConfigured(gvk schema.GroupVersionKind) bool {
+func (r *ResourceConfig) isResourceConfigured(gvk schema.GroupVersionKind) bool {
 	if _, ok := r.groups[gvk.Group]; ok {
 		return true
 	}
@@ -193,18 +199,18 @@ func (r *ResourceConfig) IsResourceConfigured(gvk schema.GroupVersionKind) bool
 	return false
 }
 
-// AddGroup adds a Group to ResourceConfig.
-func (r *ResourceConfig) AddGroup(g string) {
+// DisableGroup to disable group.
+func (r *ResourceConfig) DisableGroup(g string) {
 	r.groups[g] = struct{}{}
 }
 
-// AddGroupVersion adds a GroupVersion to ResourceConfig
-func (r *ResourceConfig) AddGroupVersion(gv schema.GroupVersion) {
+// DisableGroupVersion to disable group version.
+func (r *ResourceConfig) DisableGroupVersion(gv schema.GroupVersion) {
 	r.groupVersions[gv] = struct{}{}
 }
 
-// AddGroupVersionKind adds a GroupVersionKind to ResourceConfig
-func (r *ResourceConfig) AddGroupVersionKind(gvk schema.GroupVersionKind) {
+// DisableGroupVersionKind to disable GroupVersionKind.
+func (r *ResourceConfig) DisableGroupVersionKind(gvk schema.GroupVersionKind) {
 	r.groupVersionKinds[gvk] = struct{}{}
 }
 

pkg/utils/apiresources_test.go

@@ -76,13 +76,13 @@ func TestResourceConfigGVKParse(t *testing.T) {
 			t.Fatalf("Unexpected error: %v", err)
 		}
 		for i, o := range test.disabled {
-			ok := r.IsResourceConfigured(o)
+			ok := r.IsResourceDisabled(o)
 			if !ok {
 				t.Errorf("%d: unexpected error: %v", i, o)
 			}
 		}
 		for i, o := range test.enabled {
-			ok := r.IsResourceConfigured(o)
+			ok := r.IsResourceDisabled(o)
 			if ok {
 				t.Errorf("%d: unexpected error: %v", i, o)
 			}
@@ -145,13 +145,13 @@ func TestResourceConfigGVParse(t *testing.T) {
 			t.Fatalf("Unexpected error: %v", err)
 		}
 		for i, o := range test.disabled {
-			ok := r.IsResourceConfigured(o)
+			ok := r.IsResourceDisabled(o)
 			if !ok {
 				t.Errorf("%d: unexpected error: %v", i, o)
 			}
 		}
 		for i, o := range test.enabled {
-			ok := r.IsResourceConfigured(o)
+			ok := r.IsResourceDisabled(o)
 			if ok {
 				t.Errorf("%d: unexpected error: %v", i, o)
 			}
@@ -214,21 +214,21 @@ func TestResourceConfigGroupParse(t *testing.T) {
 			t.Fatalf("Unexpected error: %v", err)
 		}
 		for i, o := range test.disabled {
-			ok := r.IsResourceConfigured(o)
+			ok := r.IsResourceDisabled(o)
 			if !ok {
 				t.Errorf("%d: unexpected error: %v", i, o)
 			}
 		}
 		for i, o := range test.enabled {
-			ok := r.IsResourceConfigured(o)
+			ok := r.IsResourceDisabled(o)
 			if ok {
 				t.Errorf("%d: unexpected error: %v", i, o)
 			}
 		}
 	}
 }
 
-func TestResourceConfigMixedParse(t *testing.T) {
+func TestDisabledResourceConfigMixedParse(t *testing.T) {
 	tests := []struct {
 		input    string
 		disabled []schema.GroupVersionKind
@@ -293,13 +293,13 @@ func TestResourceConfigMixedParse(t *testing.T) {
 			t.Fatalf("Unexpected error: %v", err)
 		}
 		for i, o := range test.disabled {
-			ok := r.IsResourceConfigured(o)
+			ok := r.IsResourceDisabled(o)
 			if !ok {
 				t.Errorf("%d: unexpected error: %v", i, o)
 			}
 		}
 		for i, o := range test.enabled {
-			ok := r.IsResourceConfigured(o)
+			ok := r.IsResourceDisabled(o)
 			if ok {
 				t.Errorf("%d: unexpected error: %v", i, o)
 			}
@@ -351,18 +351,18 @@ func TestDefaultDisabledResourceConfigGroupVersionKindParse(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		r := NewResourceConfig(true)
+		r := NewResourceConfig(false)
 		if err := r.Parse(""); err != nil {
 			t.Fatalf("Unexpected error: %v", err)
 		}
 		for i, o := range test.disabled {
-			ok := r.IsResourceConfigured(o)
+			ok := r.IsResourceDisabled(o)
 			if !ok {
 				t.Errorf("%d: unexpected error: %v", i, o)
 			}
 		}
 		for i, o := range test.enabled {
-			ok := r.IsResourceConfigured(o)
+			ok := r.IsResourceDisabled(o)
 			if ok {
 				t.Errorf("%d: unexpected error: %v", i, o)
 			}
@@ -385,7 +385,7 @@ func TestResourceConfigIsEmpty(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		r := NewResourceConfig(false)
+		r := NewResourceConfig(true)
 		if err := r.Parse(test.input); err != nil {
 			t.Fatalf("Unexpected error: %v", err)
 		}