update after review

Signed-off-by: lou <alex1988@outlook.com>

internal/resourcemodifiers/resource_modifiers.go

@@ -97,9 +97,12 @@ func (p *ResourceModifiers) ApplyResourceModifierRules(obj *unstructured.Unstruc
 }
 
 func (r *ResourceModifierRule) apply(obj *unstructured.Unstructured, groupResource string, scheme *runtime.Scheme, log logrus.FieldLogger) error {
-	namespaceInclusion := collections.NewIncludesExcludes().Includes(r.Conditions.Namespaces...)
-	if !namespaceInclusion.ShouldInclude(obj.GetNamespace()) {
-		return nil
+	ns := obj.GetNamespace()
+	if ns != "" {
+		namespaceInclusion := collections.NewIncludesExcludes().Includes(r.Conditions.Namespaces...)
+		if !namespaceInclusion.ShouldInclude(ns) {
+			return nil
+		}
 	}
 
 	g, err := glob.Compile(r.Conditions.GroupResource)
@@ -148,17 +151,21 @@ func (r *ResourceModifierRule) apply(obj *unstructured.Unstructured, groupResour
 	return nil
 }
 
-func matchConditions(u *unstructured.Unstructured, patches []MatchRule, _ logrus.FieldLogger) (bool, error) {
-	if len(patches) == 0 {
+func matchConditions(u *unstructured.Unstructured, rules []MatchRule, _ logrus.FieldLogger) (bool, error) {
+	if len(rules) == 0 {
 		return true, nil
 	}
 
 	var fixed []JSONPatch
-	for _, patch := range patches {
+	for _, rule := range rules {
+		if rule.Path == "" {
+			return false, fmt.Errorf("path is required for match rule")
+		}
+
 		fixed = append(fixed, JSONPatch{
 			Operation: "test",
-			Path:      patch.Path,
-			Value:     patch.Value,
+			Path:      rule.Path,
+			Value:     rule.Value,
 		})
 	}
 

internal/resourcemodifiers/resource_modifiers_test.go

@@ -141,7 +141,7 @@ func TestGetResourceModifiersFromConfig(t *testing.T) {
 			Namespace: "test-namespace",
 		},
 		Data: map[string]string{
-			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupResource: pods\n    namespaces:\n    - ns1\n  mergePatches:\n  - patchData: |\n      metadata:\n        annotations:\n          foo: null",
+			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupResource: pods\n    namespaces:\n    - ns1\n    matches:\n    - path: /metadata/annotations/foo\n      value: bar\n  mergePatches:\n  - patchData: |\n      metadata:\n        annotations:\n          foo: null",
 		},
 	}
 
@@ -154,6 +154,12 @@ func TestGetResourceModifiersFromConfig(t *testing.T) {
 					Namespaces: []string{
 						"ns1",
 					},
+					Matches: []MatchRule{
+						{
+							Path:  "/metadata/annotations/foo",
+							Value: "bar",
+						},
+					},
 				},
 				MergePatches: []JSONMergePatch{
 					{
@@ -341,7 +347,7 @@ func TestGetResourceModifiersFromConfig(t *testing.T) {
 				return
 			}
 			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("GetResourceModifiersFromConfig() = %#v, want %#v", got, tt.want)
+				t.Errorf("GetResourceModifiersFromConfig() = %v, want %v", got, tt.want)
 			}
 		})
 	}
@@ -654,6 +660,38 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 			wantErr: false,
 			wantObj: deployNginxTwoReplica.DeepCopy(),
 		},
+		{
+			name: "nginx deployment: Empty Resource Regex",
+			fields: fields{
+				Version: "v1",
+				ResourceModifierRules: []ResourceModifierRule{
+					{
+						Conditions: Conditions{
+							GroupResource: "deployments.apps",
+							Namespaces:    []string{"foo"},
+						},
+						Patches: []JSONPatch{
+							{
+								Operation: "test",
+								Path:      "/spec/replicas",
+								Value:     "1",
+							},
+							{
+								Operation: "replace",
+								Path:      "/spec/replicas",
+								Value:     "2",
+							},
+						},
+					},
+				},
+			},
+			args: args{
+				obj:           deployNginxOneReplica.DeepCopy(),
+				groupResource: "deployments.apps",
+			},
+			wantErr: false,
+			wantObj: deployNginxTwoReplica.DeepCopy(),
+		},
 		{
 			name: "nginx deployment: Empty Resource Regex  and namespaces list",
 			fields: fields{

site/content/docs/main/restore-resource-modifiers.md

@@ -105,3 +105,81 @@ resourceModifierRules:
 -  Update a container's image using a json patch with positional arrays
 kubectl patch pod valid-pod -type='json' -p='[{"op": "replace", "path": "/spec/containers/0/image", "value":"new image"}]'
 - Before creating the resource modifier yaml, you can try it out using kubectl patch command. The same commands should work as it is.
+
+### New features introduced in 1.13
+
+#### JSON Merge Patch
+you can modify a resource using JSON Merge Patch
+```yaml
+version: v1
+resourceModifierRules:
+- conditions:
+    groupResource: pods
+    namespaces:
+    - ns1
+  mergePatches:
+  - patchData: |
+      {
+        "metadata": {
+          "annotations": {
+            "foo": null
+          }
+        }
+      }
+```
+- The above configmap will apply the Merge Patch to all the pods in namespace ns1 and remove the annotation `foo` from the pods.
+- Both json and yaml format are supported for the patchData.
+
+#### Strategic Merge Patch
+you can modify a resource using Strategic Merge Patch
+```yaml
+version: v1
+resourceModifierRules:
+- conditions:
+    groupResource: pods
+    resourceNameRegex: "^my-pod$"
+    namespaces:
+    - ns1
+  strategicPatches:
+  - patchData: |
+      {
+        "spec": {
+          "containers": [
+            {
+              "name": "nginx",
+              "image": "repo2/nginx"
+            }
+          ]
+        }
+      }
+```
+- The above configmap will apply the Strategic Merge Patch to the pod with name my-pod in namespace ns1 and update the image of container nginx to `repo2/nginx`.
+- Both json and yaml format are supported for the patchData.
+
+### Conditional Patches in ALL Patch Types
+Since JSON Merge Patch and Strategic Merge Patch do not support conditional patches, we use the `test` operation of JSON Patch to support conditional patches in all patch types by adding it to `Conditions` struct in `ResourceModifierRule`.
+
+Example of test in conditions
+```yaml
+version: v1
+resourceModifierRules:
+- conditions:
+    groupResource: persistentvolumeclaims.storage.k8s.io
+    matches:
+    - path: "/spec/storageClassName"
+      value: "premium"
+  mergePatches:
+  - patchData: |
+      {
+        "metadata": {
+          "annotations": {
+            "foo": null
+          }
+        }
+      }
+```
+- The above configmap will apply the Merge Patch to all the PVCs in all namespaces with storageClassName premium and remove the annotation `foo` from the PVCs.
+- You can specify multiple rules in the `matches` list. The patch will be applied only if all the matches are satisfied.
+
+### Wildcard Support for GroupResource
+The user can specify a wildcard for groupResource in the conditions' struct. This will allow the user to apply the patches for all the resources of a particular group or all resources in all groups. For example, `*.apps` will apply to all the resources in the `apps` group, `*` will apply to all the resources in all groups.