diff --git a/apis/placement/v1alpha1/override_types.go b/apis/placement/v1alpha1/override_types.go index 65993fe63..0343cb9fa 100644 --- a/apis/placement/v1alpha1/override_types.go +++ b/apis/placement/v1alpha1/override_types.go @@ -76,7 +76,7 @@ type OverrideRule struct { // OverrideType defines the type of the override rules. // +kubebuilder:validation:Enum=JSONPatch;Delete - // +kubebuilder:default:JSONPatch + // +kubebuilder:default=JSONPatch // +optional OverrideType OverrideType `json:"overrideType,omitempty"` diff --git a/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverrides.yaml b/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverrides.yaml index 6bf99ed41..c047bcfee 100644 --- a/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverrides.yaml +++ b/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverrides.yaml @@ -303,8 +303,9 @@ spec: - clusterSelectorTerms type: object jsonPatchOverrides: - description: JSONPatchOverrides defines a list of JSON patch - override rules. + description: |- + JSONPatchOverrides defines a list of JSON patch override rules. + This field is only allowed when OverrideType is JSONPatch. items: description: JSONPatchOverride applies a JSON patch on the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902). @@ -334,8 +335,14 @@ spec: maxItems: 20 minItems: 1 type: array - required: - - jsonPatchOverrides + overrideType: + default: JSONPatch + description: OverrideType defines the type of the override + rules. + enum: + - JSONPatch + - Delete + type: string type: object maxItems: 20 minItems: 1 diff --git a/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverridesnapshots.yaml b/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverridesnapshots.yaml index 10c5770e3..4c9fa8682 100644 --- a/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverridesnapshots.yaml +++ b/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverridesnapshots.yaml @@ -317,8 +317,9 @@ spec: - clusterSelectorTerms type: object jsonPatchOverrides: - description: JSONPatchOverrides defines a list of JSON - patch override rules. + description: |- + JSONPatchOverrides defines a list of JSON patch override rules. + This field is only allowed when OverrideType is JSONPatch. items: description: JSONPatchOverride applies a JSON patch on the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902). @@ -348,8 +349,14 @@ spec: maxItems: 20 minItems: 1 type: array - required: - - jsonPatchOverrides + overrideType: + default: JSONPatch + description: OverrideType defines the type of the override + rules. + enum: + - JSONPatch + - Delete + type: string type: object maxItems: 20 minItems: 1 diff --git a/config/crd/bases/placement.kubernetes-fleet.io_resourceoverrides.yaml b/config/crd/bases/placement.kubernetes-fleet.io_resourceoverrides.yaml index 5ad116e78..a61b71d69 100644 --- a/config/crd/bases/placement.kubernetes-fleet.io_resourceoverrides.yaml +++ b/config/crd/bases/placement.kubernetes-fleet.io_resourceoverrides.yaml @@ -218,8 +218,9 @@ spec: - clusterSelectorTerms type: object jsonPatchOverrides: - description: JSONPatchOverrides defines a list of JSON patch - override rules. + description: |- + JSONPatchOverrides defines a list of JSON patch override rules. + This field is only allowed when OverrideType is JSONPatch. items: description: JSONPatchOverride applies a JSON patch on the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902). @@ -249,8 +250,14 @@ spec: maxItems: 20 minItems: 1 type: array - required: - - jsonPatchOverrides + overrideType: + default: JSONPatch + description: OverrideType defines the type of the override + rules. + enum: + - JSONPatch + - Delete + type: string type: object maxItems: 20 minItems: 1 diff --git a/config/crd/bases/placement.kubernetes-fleet.io_resourceoverridesnapshots.yaml b/config/crd/bases/placement.kubernetes-fleet.io_resourceoverridesnapshots.yaml index 2073826cd..33cbe2b12 100644 --- a/config/crd/bases/placement.kubernetes-fleet.io_resourceoverridesnapshots.yaml +++ b/config/crd/bases/placement.kubernetes-fleet.io_resourceoverridesnapshots.yaml @@ -232,8 +232,9 @@ spec: - clusterSelectorTerms type: object jsonPatchOverrides: - description: JSONPatchOverrides defines a list of JSON - patch override rules. + description: |- + JSONPatchOverrides defines a list of JSON patch override rules. + This field is only allowed when OverrideType is JSONPatch. items: description: JSONPatchOverride applies a JSON patch on the selected resources following [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902). @@ -263,8 +264,14 @@ spec: maxItems: 20 minItems: 1 type: array - required: - - jsonPatchOverrides + overrideType: + default: JSONPatch + description: OverrideType defines the type of the override + rules. + enum: + - JSONPatch + - Delete + type: string type: object maxItems: 20 minItems: 1 diff --git a/pkg/controllers/workgenerator/controller.go b/pkg/controllers/workgenerator/controller.go index cc3f05315..3a981ac9c 100644 --- a/pkg/controllers/workgenerator/controller.go +++ b/pkg/controllers/workgenerator/controller.go @@ -425,17 +425,22 @@ func (r *Reconciler) syncAllWork(ctx context.Context, resourceBinding *fleetv1be } var simpleManifests []fleetv1beta1.Manifest for j := range snapshot.Spec.SelectedResources { - selectedResource := snapshot.Spec.SelectedResources[j] - if err := r.applyOverrides(&selectedResource, cluster, croMap, roMap); err != nil { - return false, false, err + selectedResource := snapshot.Spec.SelectedResources[j].DeepCopy() + // TODO: override the content of the wrapped resource instead of the envelope itself + resourceDeleted, overrideErr := r.applyOverrides(selectedResource, cluster, croMap, roMap) + if overrideErr != nil { + return false, false, overrideErr + } + if resourceDeleted { + klog.V(2).InfoS("The resource is deleted by the override rules", "snapshot", klog.KObj(snapshot), "selectedResource", snapshot.Spec.SelectedResources[j]) + continue } - // we need to special treat configMap with envelopeConfigMapAnnotation annotation, // so we need to check the GVK and annotation of the selected resource var uResource unstructured.Unstructured - if err := uResource.UnmarshalJSON(selectedResource.Raw); err != nil { - klog.ErrorS(err, "work has invalid content", "snapshot", klog.KObj(snapshot), "selectedResource", selectedResource.Raw) - return true, false, controller.NewUnexpectedBehaviorError(err) + if unMarshallErr := uResource.UnmarshalJSON(selectedResource.Raw); unMarshallErr != nil { + klog.ErrorS(unMarshallErr, "work has invalid content", "snapshot", klog.KObj(snapshot), "selectedResource", selectedResource.Raw) + return true, false, controller.NewUnexpectedBehaviorError(unMarshallErr) } if uResource.GetObjectKind().GroupVersionKind() == utils.ConfigMapGVK && len(uResource.GetAnnotations()[fleetv1beta1.EnvelopeConfigMapAnnotation]) != 0 { @@ -447,11 +452,11 @@ func (r *Reconciler) syncAllWork(ctx context.Context, resourceBinding *fleetv1be activeWork[work.Name] = work newWork = append(newWork, work) } else { - simpleManifests = append(simpleManifests, fleetv1beta1.Manifest(selectedResource)) + simpleManifests = append(simpleManifests, fleetv1beta1.Manifest(*selectedResource)) } } if len(simpleManifests) == 0 { - klog.V(2).InfoS("the snapshot contains enveloped resource only", "snapshot", klog.KObj(snapshot)) + klog.V(2).InfoS("the snapshot contains no resource to apply either because of override or enveloped resources", "snapshot", klog.KObj(snapshot)) } // generate a work object for the manifests even if there is nothing to place // to allow CRP to collect the status of the placement diff --git a/pkg/controllers/workgenerator/override.go b/pkg/controllers/workgenerator/override.go index a3eedeb02..2fcc93b4e 100644 --- a/pkg/controllers/workgenerator/override.go +++ b/pkg/controllers/workgenerator/override.go @@ -92,15 +92,21 @@ func (r *Reconciler) fetchResourceOverrideSnapshots(ctx context.Context, resourc return roMap, nil } -func (r *Reconciler) applyOverrides(resource *placementv1beta1.ResourceContent, cluster clusterv1beta1.MemberCluster, croMap map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot, roMap map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot) error { +// applyOverrides applies the overrides on the selected resources. +// The resource could be selected by both ClusterResourceOverride and ResourceOverride. +// It returns +// - true if the resource is deleted by the overrides. +// - an error if the override rules are invalid. +func (r *Reconciler) applyOverrides(resource *placementv1beta1.ResourceContent, cluster clusterv1beta1.MemberCluster, + croMap map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot, roMap map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot) (bool, error) { if len(croMap) == 0 && len(roMap) == 0 { - return nil + return false, nil } var uResource unstructured.Unstructured if err := uResource.UnmarshalJSON(resource.Raw); err != nil { klog.ErrorS(err, "Work has invalid content", "selectedResource", resource.Raw) - return controller.NewUnexpectedBehaviorError(err) + return false, controller.NewUnexpectedBehaviorError(err) } gvk := uResource.GetObjectKind().GroupVersionKind() key := placementv1beta1.ResourceIdentifier{ @@ -131,13 +137,12 @@ func (r *Reconciler) applyOverrides(resource *placementv1beta1.ResourceContent, } if err := applyOverrideRules(resource, cluster, snapshot.Spec.OverrideSpec.Policy.OverrideRules); err != nil { klog.ErrorS(err, "Failed to apply the override rules", "clusterResourceOverrideSnapshot", klog.KObj(snapshot)) - return err + return false, err } } klog.V(2).InfoS("Applied clusterResourceOverrideSnapshots", "resource", klog.KObj(&uResource), "numberOfOverrides", len(croMap[key])) - // If the resource is selected by both ClusterResourceOverride and ResourceOverride, ResourceOverride will win when - // resolving conflicts. + // If the resource is selected by both ClusterResourceOverride and ResourceOverride, ResourceOverride will win when resolving conflicts. // Apply ResourceOverrideSnapshots. if !isClusterScopeResource { key = placementv1beta1.ResourceIdentifier{ @@ -155,12 +160,12 @@ func (r *Reconciler) applyOverrides(resource *placementv1beta1.ResourceContent, } if err := applyOverrideRules(resource, cluster, snapshot.Spec.OverrideSpec.Policy.OverrideRules); err != nil { klog.ErrorS(err, "Failed to apply the override rules", "resourceOverrideSnapshot", klog.KObj(snapshot)) - return err + return false, err } } klog.V(2).InfoS("Applied resourceOverrideSnapshots", "resource", klog.KObj(&uResource), "numberOfOverrides", len(roMap[key])) } - return nil + return resource.Raw == nil, nil } func applyOverrideRules(resource *placementv1beta1.ResourceContent, cluster clusterv1beta1.MemberCluster, rules []placementv1alpha1.OverrideRule) error { @@ -173,7 +178,12 @@ func applyOverrideRules(resource *placementv1beta1.ResourceContent, cluster clus if !matched { continue } - + if rule.OverrideType == placementv1alpha1.DeleteOverrideType { + // Delete the resource + resource.Raw = nil + return nil + } + // Apply JSONPatchOverrides by default if err := applyJSONPatchOverride(resource, rule.JSONPatchOverrides); err != nil { klog.ErrorS(err, "Failed to apply JSON patch override") return controller.NewUserError(err) diff --git a/pkg/controllers/workgenerator/override_test.go b/pkg/controllers/workgenerator/override_test.go index 7e63497b8..e12adb977 100644 --- a/pkg/controllers/workgenerator/override_test.go +++ b/pkg/controllers/workgenerator/override_test.go @@ -442,6 +442,7 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) { croMap map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot wantClusterRole rbacv1.ClusterRole wantErr error + wantDeleted bool }{ { name: "empty overrides", @@ -520,7 +521,7 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) { }, }, { - name: "selected by clusterResourceOverride", + name: "selected by clusterResourceOverride but only one rule matched the cluster", clusterRole: rbacv1.ClusterRole{ TypeMeta: clusterRoleType, ObjectMeta: metav1.ObjectMeta{ @@ -552,6 +553,7 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) { Policy: &placementv1alpha1.OverridePolicy{ OverrideRules: []placementv1alpha1.OverrideRule{ { + // matching rule ClusterSelector: &placementv1beta1.ClusterSelector{ ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ { @@ -572,6 +574,7 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) { }, }, { + // non matching rule ClusterSelector: &placementv1beta1.ClusterSelector{ ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ { @@ -609,6 +612,282 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) { }, }, }, + { + name: "selected by clusterResourceOverride with two rules that don't conflict", + clusterRole: rbacv1.ClusterRole{ + TypeMeta: clusterRoleType, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterrole-name", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"authorization.k8s.io"}, + Resources: []string{"selfsubjectaccessreviews", "selfsubjectrulesreviews"}, + Verbs: []string{"create"}, + }, + }, + }, + cluster: clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + Labels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + }, + croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{ + { + Group: "rbac.authorization.k8s.io", + Version: "v1", + Kind: "ClusterRole", + Name: "clusterrole-name", + }: { + { + Spec: placementv1alpha1.ClusterResourceOverrideSnapshotSpec{ + OverrideSpec: placementv1alpha1.ClusterResourceOverrideSpec{ + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key1": "value1", + }, + }, + }, + }, + }, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpAdd, + Path: "/rules/0/verbs/1", + Value: apiextensionsv1.JSON{Raw: []byte(`"read"`)}, + }, + }, + }, + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key2": "value2", + }, + }, + }, + }, + }, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpRemove, + Path: "/rules/0/verbs/0", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + wantClusterRole: rbacv1.ClusterRole{ + TypeMeta: clusterRoleType, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterrole-name", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"authorization.k8s.io"}, + Resources: []string{"selfsubjectaccessreviews", "selfsubjectrulesreviews"}, + Verbs: []string{"read"}, + }, + }, + }, + }, + { + name: "selected by clusterResourceOverride with two rules that conflict but still a valid patch", + clusterRole: rbacv1.ClusterRole{ + TypeMeta: clusterRoleType, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterrole-name", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"authorization.k8s.io"}, + Resources: []string{"selfsubjectaccessreviews", "selfsubjectrulesreviews"}, + Verbs: []string{"create"}, + }, + }, + }, + cluster: clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + Labels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + }, + croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{ + { + Group: "rbac.authorization.k8s.io", + Version: "v1", + Kind: "ClusterRole", + Name: "clusterrole-name", + }: { + { + Spec: placementv1alpha1.ClusterResourceOverrideSnapshotSpec{ + OverrideSpec: placementv1alpha1.ClusterResourceOverrideSpec{ + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key1": "value1", + }, + }, + }, + }, + }, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpAdd, + Path: "/rules/0/verbs/1", + Value: apiextensionsv1.JSON{Raw: []byte(`"read"`)}, + }, + }, + }, + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key2": "value2", + }, + }, + }, + }, + }, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpRemove, + Path: "/rules/0/verbs/1", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + wantClusterRole: rbacv1.ClusterRole{ + TypeMeta: clusterRoleType, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterrole-name", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"authorization.k8s.io"}, + Resources: []string{"selfsubjectaccessreviews", "selfsubjectrulesreviews"}, + Verbs: []string{"create"}, + }, + }, + }, + }, + { + name: "selected by clusterResourceOverride with two rules that conflict and result in error", + clusterRole: rbacv1.ClusterRole{ + TypeMeta: clusterRoleType, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterrole-name", + }, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{"authorization.k8s.io"}, + Resources: []string{"selfsubjectaccessreviews", "selfsubjectrulesreviews"}, + Verbs: []string{"create"}, + }, + }, + }, + cluster: clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + Labels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + }, + croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{ + { + Group: "rbac.authorization.k8s.io", + Version: "v1", + Kind: "ClusterRole", + Name: "clusterrole-name", + }: { + { + Spec: placementv1alpha1.ClusterResourceOverrideSnapshotSpec{ + OverrideSpec: placementv1alpha1.ClusterResourceOverrideSpec{ + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key1": "value1", + }, + }, + }, + }, + }, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpRemove, + Path: "/rules/0/verbs", + }, + }, + }, + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key2": "value2", + }, + }, + }, + }, + }, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpAdd, + Path: "/rules/0/verbs/1", + Value: apiextensionsv1.JSON{Raw: []byte(`"read"`)}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: controller.ErrUserError, + }, { name: "invalid json patch of clusterResourceOverride", clusterRole: rbacv1.ClusterRole{ @@ -658,13 +937,65 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) { }, }, }, + }, + }, + }, + }, + }, + }, + }, + wantErr: controller.ErrUserError, + }, + { + name: "delete during the clusterResourceOverride", + clusterRole: rbacv1.ClusterRole{ + TypeMeta: clusterRoleType, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterrole-name", + }, + }, + cluster: clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + Labels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + }, + croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{ + { + Group: "rbac.authorization.k8s.io", + Version: "v1", + Kind: "ClusterRole", + Name: "clusterrole-name", + }: { + { + Spec: placementv1alpha1.ClusterResourceOverrideSnapshotSpec{ + OverrideSpec: placementv1alpha1.ClusterResourceOverrideSpec{ + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key1": "value1", + }, + }, + }, + }, + }, + OverrideType: placementv1alpha1.DeleteOverrideType, + }, { ClusterSelector: &placementv1beta1.ClusterSelector{ ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ { LabelSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - "key2": "value1", + "key1": "value1", }, }, }, @@ -673,7 +1004,7 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) { JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ { Operator: placementv1alpha1.JSONPatchOverrideOpReplace, - Path: "/metadata/labels/new-label", + Path: "/metadata/labels/key1", Value: apiextensionsv1.JSON{Raw: []byte(`"new-value1"`)}, }, }, @@ -685,7 +1016,83 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) { }, }, }, - wantErr: controller.ErrUserError, + wantDeleted: true, + }, + { + name: "delete after patching the clusterResourceOverride", + clusterRole: rbacv1.ClusterRole{ + TypeMeta: clusterRoleType, + ObjectMeta: metav1.ObjectMeta{ + Name: "clusterrole-name", + Labels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + }, + cluster: clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + Labels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + }, + croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{ + { + Group: "rbac.authorization.k8s.io", + Version: "v1", + Kind: "ClusterRole", + Name: "clusterrole-name", + }: { + { + Spec: placementv1alpha1.ClusterResourceOverrideSnapshotSpec{ + OverrideSpec: placementv1alpha1.ClusterResourceOverrideSpec{ + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key2": "value2", + }, + }, + }, + }, + }, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpReplace, + Path: "/metadata/labels/key1", + Value: apiextensionsv1.JSON{Raw: []byte(`"new-value1"`)}, + }, + }, + }, + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key1": "value1", + }, + }, + }, + }, + }, + OverrideType: placementv1alpha1.DeleteOverrideType, + }, + }, + }, + }, + }, + }, + }, + }, + wantDeleted: true, }, } @@ -695,14 +1102,19 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) { InformerManager: &fakeInformer, } rc := resource.CreateResourceContentForTest(t, tc.clusterRole) - err := r.applyOverrides(rc, tc.cluster, tc.croMap, nil) + gotDeleted, err := r.applyOverrides(rc, tc.cluster, tc.croMap, nil) if gotErr, wantErr := err != nil, tc.wantErr != nil; gotErr != wantErr || !errors.Is(err, tc.wantErr) { t.Fatalf("applyOverrides() got error %v, want error %v", err, tc.wantErr) } - + if gotDeleted != tc.wantDeleted { + t.Fatalf("applyOverrides() gotDeleted %v, want %v", gotDeleted, tc.wantDeleted) + } if tc.wantErr != nil { return } + if tc.wantDeleted { + return + } var u unstructured.Unstructured if err := u.UnmarshalJSON(rc.Raw); err != nil { @@ -711,7 +1123,7 @@ func TestApplyOverrides_clusterScopedResource(t *testing.T) { var clusterRole rbacv1.ClusterRole if err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, &clusterRole); err != nil { - t.Fatalf("Failed to convert the result to deployment: %v, want nil", err) + t.Fatalf("Failed to convert the result to clusterole: %v, want nil", err) } if diff := cmp.Diff(tc.wantClusterRole, clusterRole); diff != "" { @@ -725,16 +1137,16 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { fakeInformer := informer.FakeManager{ APIResources: map[schema.GroupVersionKind]bool{ { - Group: "", - Version: "v1", - Kind: "Deployment", + Group: utils.DeploymentGVK.Group, + Version: utils.DeploymentGVK.Version, + Kind: utils.DeploymentGVK.Kind, }: true, }, IsClusterScopedResource: false, } deploymentType := metav1.TypeMeta{ - APIVersion: "v1", - Kind: "Deployment", + APIVersion: utils.DeploymentGVK.GroupVersion().String(), + Kind: utils.DeploymentGVK.Kind, } tests := []struct { @@ -745,6 +1157,7 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { roMap map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot wantDeployment appsv1.Deployment wantErr error + wantDelete bool }{ { name: "empty overrides", @@ -758,13 +1171,82 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { }, }, }, - cluster: clusterv1beta1.MemberCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster-1", - }, - }, - croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{}, - roMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot{}, + cluster: clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + }, + }, + croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{}, + roMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot{}, + wantDeployment: appsv1.Deployment{ + TypeMeta: deploymentType, + ObjectMeta: metav1.ObjectMeta{ + Name: "deployment-name", + Namespace: "deployment-namespace", + Labels: map[string]string{ + "app": "nginx", + }, + }, + }, + }, + { + name: "no matched overrides on clusters", + deployment: appsv1.Deployment{ + TypeMeta: deploymentType, + ObjectMeta: metav1.ObjectMeta{ + Name: "deployment-name", + Namespace: "deployment-namespace", + Labels: map[string]string{ + "app": "nginx", + }, + }, + }, + cluster: clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + }, + }, + croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{ + { + Group: utils.NamespaceMetaGVK.Group, + Version: utils.NamespaceMetaGVK.Version, + Kind: utils.NamespaceMetaGVK.Kind, + Name: "invalid-namespace", + }: { + { + Spec: placementv1alpha1.ClusterResourceOverrideSnapshotSpec{ + OverrideSpec: placementv1alpha1.ClusterResourceOverrideSpec{ + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key1": "value1", + }, + }, + }, + }, + }, + OverrideType: placementv1alpha1.JSONPatchOverrideType, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpAdd, + Path: "/metadata/labels/new-label", + Value: apiextensionsv1.JSON{Raw: []byte(`"new-value"`)}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + roMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot{}, wantDeployment: appsv1.Deployment{ TypeMeta: deploymentType, ObjectMeta: metav1.ObjectMeta{ @@ -777,7 +1259,7 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { }, }, { - name: "no matched overrides", + name: "no matched overrides on resources", deployment: appsv1.Deployment{ TypeMeta: deploymentType, ObjectMeta: metav1.ObjectMeta{ @@ -809,6 +1291,7 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { }, }, }, + OverrideType: placementv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ { Operator: placementv1alpha1.JSONPatchOverrideOpAdd, @@ -839,6 +1322,7 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { OverrideRules: []placementv1alpha1.OverrideRule{ { ClusterSelector: nil, // matching all the clusters + OverrideType: placementv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ { Operator: placementv1alpha1.JSONPatchOverrideOpReplace, @@ -927,6 +1411,7 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { }, }, }, + OverrideType: placementv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ { Operator: placementv1alpha1.JSONPatchOverrideOpReplace, @@ -977,9 +1462,9 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { }, roMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot{ { - Group: "", - Version: "v1", - Kind: "Deployment", + Group: utils.DeploymentGVK.Group, + Version: utils.DeploymentGVK.Version, + Kind: utils.DeploymentGVK.Kind, Name: "deployment-name", Namespace: "deployment-namespace", }: { @@ -1000,6 +1485,7 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { }, }, }, + OverrideType: placementv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ { Operator: placementv1alpha1.JSONPatchOverrideOpAdd, @@ -1010,6 +1496,7 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { }, { ClusterSelector: &placementv1beta1.ClusterSelector{}, // selecting all the clusters + OverrideType: placementv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ { Operator: placementv1alpha1.JSONPatchOverrideOpAdd, @@ -1082,6 +1569,7 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { }, }, }, + OverrideType: placementv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ { Operator: placementv1alpha1.JSONPatchOverrideOpReplace, @@ -1099,9 +1587,9 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { }, roMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot{ { - Group: "", - Version: "v1", - Kind: "Deployment", + Group: utils.DeploymentGVK.Group, + Version: utils.DeploymentGVK.Version, + Kind: utils.DeploymentGVK.Kind, Name: "deployment-name", Namespace: "deployment-namespace", }: { @@ -1112,6 +1600,7 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { OverrideRules: []placementv1alpha1.OverrideRule{ { ClusterSelector: &placementv1beta1.ClusterSelector{}, // matching all the clusters + OverrideType: placementv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ { Operator: placementv1alpha1.JSONPatchOverrideOpReplace, @@ -1183,6 +1672,7 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { }, }, }, + OverrideType: placementv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ { Operator: placementv1alpha1.JSONPatchOverrideOpReplace, @@ -1223,9 +1713,9 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { }, roMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot{ { - Group: "", - Version: "v1", - Kind: "Deployment", + Group: utils.DeploymentGVK.Group, + Version: utils.DeploymentGVK.Version, + Kind: utils.DeploymentGVK.Kind, Name: "deployment-name", Namespace: "deployment-namespace", }: { @@ -1236,10 +1726,11 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { OverrideRules: []placementv1alpha1.OverrideRule{ { ClusterSelector: &placementv1beta1.ClusterSelector{}, // matching all the clusters + OverrideType: placementv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ { Operator: placementv1alpha1.JSONPatchOverrideOpReplace, - Path: "/metadata/label/app", + Path: "/metadata/spec", Value: apiextensionsv1.JSON{Raw: []byte(`"app3"`)}, }, }, @@ -1253,6 +1744,226 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { }, wantErr: controller.ErrUserError, }, + { + name: "delete type of resourceOverride", + deployment: appsv1.Deployment{ + TypeMeta: deploymentType, + ObjectMeta: metav1.ObjectMeta{ + Name: "deployment-name", + Namespace: "deployment-namespace", + Labels: map[string]string{ + "app": "app1", + }, + }, + }, + cluster: clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + Labels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + }, + roMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot{ + { + Group: utils.DeploymentGVK.Group, + Version: utils.DeploymentGVK.Version, + Kind: utils.DeploymentGVK.Kind, + Name: "deployment-name", + Namespace: "deployment-namespace", + }: { + { + Spec: placementv1alpha1.ResourceOverrideSnapshotSpec{ + OverrideSpec: placementv1alpha1.ResourceOverrideSpec{ + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{}, // matching all the clusters + OverrideType: placementv1alpha1.DeleteOverrideType, + }, + }, + }, + }, + }, + }, + }, + }, + wantDelete: true, + }, + { + name: "resourceOverride delete the cro override", + deployment: appsv1.Deployment{ + TypeMeta: deploymentType, + ObjectMeta: metav1.ObjectMeta{ + Name: "deployment-name", + Namespace: "deployment-namespace", + Labels: map[string]string{ + "app": "app1", + }, + }, + }, + cluster: clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + Labels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + }, + croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{ + { + Group: utils.NamespaceMetaGVK.Group, + Version: utils.NamespaceMetaGVK.Version, + Kind: utils.NamespaceMetaGVK.Kind, + Name: "deployment-namespace", + }: { + { + Spec: placementv1alpha1.ClusterResourceOverrideSnapshotSpec{ + OverrideSpec: placementv1alpha1.ClusterResourceOverrideSpec{ + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key1": "value1", + }, + }, + }, + }, + }, + OverrideType: placementv1alpha1.JSONPatchOverrideType, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpReplace, + Path: "/metadata/labels/app", + Value: apiextensionsv1.JSON{Raw: []byte(`"app2"`)}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + roMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot{ + { + Group: utils.DeploymentGVK.Group, + Version: utils.DeploymentGVK.Version, + Kind: utils.DeploymentGVK.Kind, + Name: "deployment-name", + Namespace: "deployment-namespace", + }: { + { + Spec: placementv1alpha1.ResourceOverrideSnapshotSpec{ + OverrideSpec: placementv1alpha1.ResourceOverrideSpec{ + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{}, // matching all the clusters + OverrideType: placementv1alpha1.DeleteOverrideType, + }, + }, + }, + }, + }, + }, + }, + }, + wantDelete: true, + }, + { + name: "resourceOverride no-op when the cro delete", + deployment: appsv1.Deployment{ + TypeMeta: deploymentType, + ObjectMeta: metav1.ObjectMeta{ + Name: "deployment-name", + Namespace: "deployment-namespace", + Labels: map[string]string{ + "app": "app1", + }, + }, + }, + cluster: clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-1", + Labels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + }, + croMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ClusterResourceOverrideSnapshot{ + { + Group: utils.NamespaceMetaGVK.Group, + Version: utils.NamespaceMetaGVK.Version, + Kind: utils.NamespaceMetaGVK.Kind, + Name: "deployment-namespace", + }: { + { + Spec: placementv1alpha1.ClusterResourceOverrideSnapshotSpec{ + OverrideSpec: placementv1alpha1.ClusterResourceOverrideSpec{ + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key1": "value1", + }, + }, + }, + }, + }, + OverrideType: placementv1alpha1.DeleteOverrideType, + }, + }, + }, + }, + }, + }, + }, + }, + roMap: map[placementv1beta1.ResourceIdentifier][]*placementv1alpha1.ResourceOverrideSnapshot{ + { + Group: utils.DeploymentGVK.Group, + Version: utils.DeploymentGVK.Version, + Kind: utils.DeploymentGVK.Kind, + Name: "deployment-name", + Namespace: "deployment-namespace", + }: { + { + Spec: placementv1alpha1.ResourceOverrideSnapshotSpec{ + OverrideSpec: placementv1alpha1.ResourceOverrideSpec{ + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{}, // matching all the clusters + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpAdd, + Path: "/metadata/labels/new-label", + Value: apiextensionsv1.JSON{Raw: []byte(`"new-value"`)}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + wantDelete: true, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -1260,14 +1971,19 @@ func TestApplyOverrides_namespacedScopeResource(t *testing.T) { InformerManager: &fakeInformer, } rc := resource.CreateResourceContentForTest(t, tc.deployment) - err := r.applyOverrides(rc, tc.cluster, tc.croMap, tc.roMap) + gotDeleted, err := r.applyOverrides(rc, tc.cluster, tc.croMap, tc.roMap) if gotErr, wantErr := err != nil, tc.wantErr != nil; gotErr != wantErr || !errors.Is(err, tc.wantErr) { t.Fatalf("applyOverrides() got error %v, want error %v", err, tc.wantErr) } - + if gotDeleted != tc.wantDelete { + t.Fatalf("applyOverrides() gotDeleted %v, want %v", gotDeleted, tc.wantDelete) + } if tc.wantErr != nil { return } + if tc.wantDelete { + return + } var u unstructured.Unstructured if err := u.UnmarshalJSON(rc.Raw); err != nil { diff --git a/pkg/utils/validator/clusterresourceoverride_test.go b/pkg/utils/validator/clusterresourceoverride_test.go index 088c11424..7040fe5df 100644 --- a/pkg/utils/validator/clusterresourceoverride_test.go +++ b/pkg/utils/validator/clusterresourceoverride_test.go @@ -540,6 +540,21 @@ func TestValidateClusterResourceOverride(t *testing.T) { }, wantErrMsg: nil, }, + "valid cluster resource override - delete policy": { + cro: fleetv1alpha1.ClusterResourceOverride{ + Spec: fleetv1alpha1.ClusterResourceOverrideSpec{ + Policy: &fleetv1alpha1.OverridePolicy{ + OverrideRules: []fleetv1alpha1.OverrideRule{ + { + ClusterSelector: validClusterSelector, + OverrideType: fleetv1alpha1.DeleteOverrideType, + }, + }, + }, + }, + }, + wantErrMsg: nil, + }, "valid cluster resource override - policy with all label selectors": { cro: fleetv1alpha1.ClusterResourceOverride{ Spec: fleetv1alpha1.ClusterResourceOverrideSpec{ @@ -564,6 +579,7 @@ func TestValidateClusterResourceOverride(t *testing.T) { }, }, }, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: validJSONPatchOverrides, }, { @@ -585,6 +601,7 @@ func TestValidateClusterResourceOverride(t *testing.T) { }, }, }, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: validJSONPatchOverrides, }, }, @@ -627,6 +644,7 @@ func TestValidateClusterResourceOverride(t *testing.T) { Policy: &fleetv1alpha1.OverridePolicy{ OverrideRules: []fleetv1alpha1.OverrideRule{ { + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: validJSONPatchOverrides, }, }, @@ -693,6 +711,7 @@ func TestValidateClusterResourceOverride(t *testing.T) { {}, }, }, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: validJSONPatchOverrides, }, }, @@ -718,6 +737,7 @@ func TestValidateClusterResourceOverride(t *testing.T) { }, }, }, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []fleetv1alpha1.JSONPatchOverride{ { Operator: fleetv1alpha1.JSONPatchOverrideOpRemove, diff --git a/pkg/utils/validator/resourceoverride.go b/pkg/utils/validator/resourceoverride.go index 362deb62d..67d16fcf4 100644 --- a/pkg/utils/validator/resourceoverride.go +++ b/pkg/utils/validator/resourceoverride.go @@ -102,9 +102,16 @@ func validateOverridePolicy(policy *fleetv1alpha1.OverridePolicy) error { } } } + switch rule.OverrideType { + case fleetv1alpha1.DeleteOverrideType: + if len(rule.JSONPatchOverrides) != 0 { + return errors.New("invalid JSONPatchOverrides: JSONPatchOverrides cannot be set when the override type is Delete") + } - if err := validateJSONPatchOverride(rule.JSONPatchOverrides); err != nil { - allErr = append(allErr, err) + case fleetv1alpha1.JSONPatchOverrideType: + if err := validateJSONPatchOverride(rule.JSONPatchOverrides); err != nil { + allErr = append(allErr, err) + } } } return apierrors.NewAggregate(allErr) diff --git a/pkg/utils/validator/resourceoverride_test.go b/pkg/utils/validator/resourceoverride_test.go index 869d4eee3..e870aa6c6 100644 --- a/pkg/utils/validator/resourceoverride_test.go +++ b/pkg/utils/validator/resourceoverride_test.go @@ -348,6 +348,30 @@ func TestValidateResourceOverride(t *testing.T) { roList: nil, wantErrMsg: nil, }, + "valid resource override - delete nil": { + ro: fleetv1alpha1.ResourceOverride{ + Spec: fleetv1alpha1.ResourceOverrideSpec{ + ResourceSelectors: []fleetv1alpha1.ResourceSelector{ + { + Group: "rbac.authorization.k8s.io", + Version: "v1", + Kind: "ClusterRole", + Name: "test-cluster-role", + }, + }, + Policy: &fleetv1alpha1.OverridePolicy{ + OverrideRules: []fleetv1alpha1.OverrideRule{ + { + ClusterSelector: validClusterSelector, + OverrideType: fleetv1alpha1.DeleteOverrideType, + }, + }, + }, + }, + }, + roList: nil, + wantErrMsg: nil, + }, "invalid resource override - fail validateResourceOverridePolicy with unsupported type ": { ro: fleetv1alpha1.ResourceOverride{ Spec: fleetv1alpha1.ResourceOverrideSpec{ @@ -404,6 +428,7 @@ func TestValidateResourceOverride(t *testing.T) { OverrideRules: []fleetv1alpha1.OverrideRule{ { ClusterSelector: &fleetv1beta1.ClusterSelector{}, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: validJSONPatchOverrides, }, }, @@ -421,6 +446,7 @@ func TestValidateResourceOverride(t *testing.T) { ClusterSelector: &fleetv1beta1.ClusterSelector{ ClusterSelectorTerms: []fleetv1beta1.ClusterSelectorTerm{}, }, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: validJSONPatchOverrides, }, }, @@ -442,6 +468,7 @@ func TestValidateResourceOverride(t *testing.T) { }, }, }, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: validJSONPatchOverrides, }, { @@ -452,6 +479,7 @@ func TestValidateResourceOverride(t *testing.T) { }, }, }, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: validJSONPatchOverrides, }, }, @@ -485,6 +513,7 @@ func TestValidateResourceOverride(t *testing.T) { }, }, }, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []fleetv1alpha1.JSONPatchOverride{ { Operator: fleetv1alpha1.JSONPatchOverrideOpRemove, @@ -670,13 +699,14 @@ func TestValidateOverridePolicy(t *testing.T) { }, }, }, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: nil, }, }, }, wantErrMsg: errors.New("JSONPatchOverrides cannot be empty"), }, - "empty JSONPatchOverrides": { + "empty JSONPatchOverrides with jsonPatch override type": { policy: &fleetv1alpha1.OverridePolicy{ OverrideRules: []fleetv1alpha1.OverrideRule{ { @@ -691,17 +721,41 @@ func TestValidateOverridePolicy(t *testing.T) { }, }, }, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []fleetv1alpha1.JSONPatchOverride{}, }, }, }, wantErrMsg: errors.New("JSONPatchOverrides cannot be empty"), }, + "JSONPatchOverrides with delete override type": { + policy: &fleetv1alpha1.OverridePolicy{ + OverrideRules: []fleetv1alpha1.OverrideRule{ + { + ClusterSelector: &fleetv1beta1.ClusterSelector{ + ClusterSelectorTerms: []fleetv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "key": "value", + }, + }, + }, + }, + }, + OverrideType: fleetv1alpha1.DeleteOverrideType, + JSONPatchOverrides: validJSONPatchOverrides, + }, + }, + }, + wantErrMsg: errors.New("JSONPatchOverrides cannot be set when the override type is Delete"), + }, "invalid JSONPatchOverridesPath": { policy: &fleetv1alpha1.OverridePolicy{ OverrideRules: []fleetv1alpha1.OverrideRule{ { ClusterSelector: &fleetv1beta1.ClusterSelector{}, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []fleetv1alpha1.JSONPatchOverride{ { Operator: fleetv1alpha1.JSONPatchOverrideOpReplace, @@ -719,6 +773,7 @@ func TestValidateOverridePolicy(t *testing.T) { OverrideRules: []fleetv1alpha1.OverrideRule{ { ClusterSelector: &fleetv1beta1.ClusterSelector{}, + OverrideType: fleetv1alpha1.JSONPatchOverrideType, JSONPatchOverrides: []fleetv1alpha1.JSONPatchOverride{ { Operator: fleetv1alpha1.JSONPatchOverrideOpRemove, diff --git a/test/e2e/placement_cro_test.go b/test/e2e/placement_cro_test.go index 904afdf85..e89a30b25 100644 --- a/test/e2e/placement_cro_test.go +++ b/test/e2e/placement_cro_test.go @@ -9,8 +9,11 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" placementv1alpha1 "go.goms.io/fleet/apis/placement/v1alpha1" placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" @@ -431,3 +434,119 @@ var _ = Describe("creating clusterResourceOverride with and resource becomes inv // This check will ignore the annotation of resources. It("should not place the selected resources on member clusters", checkIfRemovedWorkResourcesFromAllMemberClusters) }) + +var _ = Describe("creating clusterResourceOverride with delete rules for one cluster", Ordered, func() { + crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) + croName := fmt.Sprintf(croNameTemplate, GinkgoParallelProcess()) + + BeforeAll(func() { + By("creating work resources") + createWorkResources() + + // Create the cro before crp so that the observed resource index is predictable. + cro := &placementv1alpha1.ClusterResourceOverride{ + ObjectMeta: metav1.ObjectMeta{ + Name: croName, + }, + Spec: placementv1alpha1.ClusterResourceOverrideSpec{ + ClusterResourceSelectors: workResourceSelector(), + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{regionLabelName: regionLabelValue1}, + }, + }, + }, + }, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpAdd, + Path: "/metadata/annotations", + Value: apiextensionsv1.JSON{Raw: []byte(fmt.Sprintf(`{"%s": "%s"}`, croTestAnnotationKey1, croTestAnnotationValue1))}, + }, + }, + }, + // Delete the resources on the region2 cluster. + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{regionLabelName: regionLabelValue2}, + }, + }, + }, + }, + OverrideType: placementv1alpha1.DeleteOverrideType, + }, + }, + }, + }, + } + By(fmt.Sprintf("creating clusterResourceOverride %s", croName)) + Expect(hubClient.Create(ctx, cro)).To(Succeed(), "Failed to create clusterResourceOverride %s", croName) + + // Create the CRP. + crp := &placementv1beta1.ClusterResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: crpName, + // Add a custom finalizer; this would allow us to better observe + // the behavior of the controllers. + Finalizers: []string{customDeletionBlockerFinalizer}, + }, + Spec: placementv1beta1.ClusterResourcePlacementSpec{ + ResourceSelectors: workResourceSelector(), + }, + } + By(fmt.Sprintf("creating placement %s", crpName)) + Expect(hubClient.Create(ctx, crp)).To(Succeed(), "Failed to create CRP %s", crpName) + }) + + AfterAll(func() { + By(fmt.Sprintf("deleting placement %s and related resources", crpName)) + ensureCRPAndRelatedResourcesDeletion(crpName, allMemberClusters) + + By(fmt.Sprintf("deleting clusterResourceOverride %s", croName)) + cleanupClusterResourceOverride(croName) + }) + + It("should update CRP status as expected", func() { + wantCRONames := []string{fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, croName, 0)} + crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", wantCRONames, nil) + Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) + }) + + // This check will ignore the annotation of resources. + It("should place the selected resources on the member clusters that are patched", func() { + for idx := 0; idx < 2; idx++ { + memberCluster := allMemberClusters[idx] + workResourcesPlacedActual := workNamespaceAndConfigMapPlacedOnClusterActual(memberCluster) + Eventually(workResourcesPlacedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to place work resources on member cluster %s", memberCluster.ClusterName) + } + }) + + It("should have override annotations on the member clusters that are patched", func() { + for idx := 0; idx < 2; idx++ { + cluster := allMemberClusters[idx] + wantAnnotations := map[string]string{croTestAnnotationKey1: croTestAnnotationValue1} + Expect(validateAnnotationOfWorkNamespaceOnCluster(cluster, wantAnnotations)).Should(Succeed(), "Failed to override the annotation of work namespace on %s", cluster.ClusterName) + Expect(validateOverrideAnnotationOfConfigMapOnCluster(cluster, wantAnnotations)).Should(Succeed(), "Failed to override the annotation of configmap on %s", cluster.ClusterName) + } + }) + + It("should not place the selected resources on the member clusters that are deleted", func() { + memberCluster := allMemberClusters[2] + Consistently(func() bool { + ns := &corev1.Namespace{} + workNamespaceName := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess()) + if err := memberCluster.KubeClient.Get(ctx, types.NamespacedName{Name: workNamespaceName}, ns); err != nil { + return errors.IsNotFound(err) + } + return false + }, consistentlyDuration, eventuallyInterval).Should(BeTrue(), "Failed to delete work resources on member cluster %s", memberCluster.ClusterName) + }) +}) diff --git a/test/e2e/placement_ro_test.go b/test/e2e/placement_ro_test.go index 704e1ad65..5cabd278b 100644 --- a/test/e2e/placement_ro_test.go +++ b/test/e2e/placement_ro_test.go @@ -9,8 +9,11 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" placementv1alpha1 "go.goms.io/fleet/apis/placement/v1alpha1" placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" @@ -567,3 +570,128 @@ var _ = Describe("creating resourceOverride and resource becomes invalid after o // This check will ignore the annotation of resources. It("should not place the selected resources on member clusters", checkIfRemovedWorkResourcesFromAllMemberClusters) }) + +var _ = Describe("creating resourceOverride with delete configMap", Ordered, func() { + crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) + roName := fmt.Sprintf(roNameTemplate, GinkgoParallelProcess()) + roNamespace := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess()) + + BeforeAll(func() { + By("creating work resources") + createWorkResources() + + // Create the ro before crp so that the observed resource index is predictable. + ro := &placementv1alpha1.ResourceOverride{ + ObjectMeta: metav1.ObjectMeta{ + Name: roName, + Namespace: roNamespace, + }, + Spec: placementv1alpha1.ResourceOverrideSpec{ + ResourceSelectors: configMapSelector(), + Policy: &placementv1alpha1.OverridePolicy{ + OverrideRules: []placementv1alpha1.OverrideRule{ + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{regionLabelName: regionLabelValue1}, + }, + }, + }, + }, + JSONPatchOverrides: []placementv1alpha1.JSONPatchOverride{ + { + Operator: placementv1alpha1.JSONPatchOverrideOpAdd, + Path: "/metadata/annotations", + Value: apiextensionsv1.JSON{Raw: []byte(fmt.Sprintf(`{"%s": "%s"}`, roTestAnnotationKey, roTestAnnotationValue))}, + }, + }, + }, + { + ClusterSelector: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{regionLabelName: regionLabelValue2}, + }, + }, + }, + }, + OverrideType: placementv1alpha1.DeleteOverrideType, + }, + }, + }, + }, + } + By(fmt.Sprintf("creating resourceOverride %s", roName)) + Expect(hubClient.Create(ctx, ro)).To(Succeed(), "Failed to create resourceOverride %s", roName) + + // Create the CRP. + crp := &placementv1beta1.ClusterResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: crpName, + // Add a custom finalizer; this would allow us to better observe + // the behavior of the controllers. + Finalizers: []string{customDeletionBlockerFinalizer}, + }, + Spec: placementv1beta1.ClusterResourcePlacementSpec{ + ResourceSelectors: workResourceSelector(), + }, + } + By(fmt.Sprintf("creating placement %s", crpName)) + Expect(hubClient.Create(ctx, crp)).To(Succeed(), "Failed to create CRP %s", crpName) + }) + + AfterAll(func() { + By(fmt.Sprintf("deleting placement %s and related resources", crpName)) + ensureCRPAndRelatedResourcesDeletion(crpName, allMemberClusters) + + By(fmt.Sprintf("deleting resourceOverride %s", roName)) + cleanupResourceOverride(roName, roNamespace) + }) + + It("should update CRP status as expected", func() { + wantRONames := []placementv1beta1.NamespacedName{ + {Namespace: roNamespace, Name: fmt.Sprintf(placementv1alpha1.OverrideSnapshotNameFmt, roName, 0)}, + } + crpStatusUpdatedActual := crpStatusWithOverrideUpdatedActual(workResourceIdentifiers(), allMemberClusterNames, "0", nil, wantRONames) + Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) + }) + + It("should place the namespaces on all member clusters", func() { + for idx := 0; idx < 3; idx++ { + memberCluster := allMemberClusters[idx] + workResourcesPlacedActual := workNamespacePlacedOnClusterActual(memberCluster) + Eventually(workResourcesPlacedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to place work resources on member cluster %s", memberCluster.ClusterName) + } + }) + + // This check will ignore the annotation of resources. + It("should place the configmap on member clusters that are patched", func() { + for idx := 0; idx < 2; idx++ { + memberCluster := allMemberClusters[idx] + workResourcesPlacedActual := workNamespaceAndConfigMapPlacedOnClusterActual(memberCluster) + Eventually(workResourcesPlacedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to place work resources on member cluster %s", memberCluster.ClusterName) + } + }) + + It("should have override annotations on the configmap on the member clusters that are patched", func() { + for idx := 0; idx < 2; idx++ { + cluster := allMemberClusters[idx] + wantAnnotations := map[string]string{roTestAnnotationKey: roTestAnnotationValue} + Expect(validateOverrideAnnotationOfConfigMapOnCluster(cluster, wantAnnotations)).Should(Succeed(), "Failed to override the annotation of configmap on %s", cluster.ClusterName) + } + }) + + It("should not place the configmap on the member clusters that are deleted", func() { + memberCluster := allMemberClusters[2] + Consistently(func() bool { + namespaceName := fmt.Sprintf(workNamespaceNameTemplate, GinkgoParallelProcess()) + configMapName := fmt.Sprintf(appConfigMapNameTemplate, GinkgoParallelProcess()) + configMap := corev1.ConfigMap{} + err := memberCluster.KubeClient.Get(ctx, types.NamespacedName{Name: configMapName, Namespace: namespaceName}, &configMap) + return errors.IsNotFound(err) + }, consistentlyDuration, consistentlyInterval).Should(BeTrue(), "Failed to delete work resources on member cluster %s", memberCluster.ClusterName) + }) +})