add label selector in resource modifier

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

changelogs/unreleased/6704-27149chen

@@ -0,0 +1,3 @@
+This pr made some improvements in Resource Modifiers:
+1. add label selector
+2. change the field name from groupKind to groupResource

design/Implemented/json-substitution-action-design.md

@@ -26,7 +26,7 @@ Currently velero supports substituting certain values in the K8s resources durin
 <!-- ## Background -->
 
 ## Goals
-- Allow the user to specify a GroupKind, Name(optional), JSON patch for modification.
+- Allow the user to specify a GroupResource, Name(optional), JSON patch for modification.
 - Allow the user to specify multiple JSON patch.
 
 ## Non Goals
@@ -74,7 +74,7 @@ velero restore create --from-backup backup-1 --resource-modifier-configmap resou
 
 ### Resource Modifier ConfigMap Structure
 - User first needs to provide details on which resources the JSON Substitutions need to be applied. 
-    - For this the user will provide 4 inputs - Namespaces(for NS Scoped resources), GroupKind (kind.group format similar to includeResources field in velero) and Name Regex(optional).
+    - For this the user will provide 4 inputs - Namespaces(for NS Scoped resources), GroupResource (resource.group format similar to includeResources field in velero) and Name Regex(optional).
     - If the user does not provide the Name, the JSON Substitutions will be applied to all the resources of the given Group and Kind under the given namespaces.
 
 - Further the use will specify the JSON Patch using the structure of kubectl's "JSON Patch" based inputs.
@@ -83,7 +83,7 @@ velero restore create --from-backup backup-1 --resource-modifier-configmap resou
 version: v1
 resourceModifierRules:
 - conditions:
-    groupKind: persistentvolumeclaims
+    groupResource: persistentvolumeclaims
     resourceNameRegex: "mysql.*"
     namespaces:
     - bar
@@ -119,7 +119,7 @@ kubectl create cm <configmap-name> --from-file <yaml-file> -n velero
 version: v1
 resourceModifierRules:
 - conditions:
-    groupKind: persistentvolumeclaims.storage.k8s.io
+    groupResource: persistentvolumeclaims.storage.k8s.io
     resourceNameRegex: ".*"
     namespaces:
     - bar

internal/resourcemodifiers/resource_modifiers.go

@@ -2,17 +2,18 @@ package resourcemodifiers
 
 import (
 	"fmt"
-	"io"
 	"regexp"
 	"strconv"
 	"strings"
 
 	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	"gopkg.in/yaml.v3"
 	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/labels"
+	"sigs.k8s.io/yaml"
 
 	"github.com/vmware-tanzu/velero/pkg/util/collections"
 )
@@ -23,26 +24,27 @@ const (
 )
 
 type JSONPatch struct {
-	Operation string `yaml:"operation"`
-	From      string `yaml:"from,omitempty"`
-	Path      string `yaml:"path"`
-	Value     string `yaml:"value,omitempty"`
+	Operation string `json:"operation"`
+	From      string `json:"from,omitempty"`
+	Path      string `json:"path"`
+	Value     string `json:"value,omitempty"`
 }
 
 type Conditions struct {
-	Namespaces        []string `yaml:"namespaces,omitempty"`
-	GroupKind         string   `yaml:"groupKind"`
-	ResourceNameRegex string   `yaml:"resourceNameRegex"`
+	Namespaces        []string              `json:"namespaces,omitempty"`
+	GroupResource     string                `json:"groupResource"`
+	ResourceNameRegex string                `json:"resourceNameRegex,omitempty"`
+	LabelSelector     *metav1.LabelSelector `json:"labelSelector,omitempty"`
 }
 
 type ResourceModifierRule struct {
-	Conditions Conditions  `yaml:"conditions"`
-	Patches    []JSONPatch `yaml:"patches"`
+	Conditions Conditions  `json:"conditions"`
+	Patches    []JSONPatch `json:"patches"`
 }
 
 type ResourceModifiers struct {
-	Version               string                 `yaml:"version"`
-	ResourceModifierRules []ResourceModifierRule `yaml:"resourceModifierRules"`
+	Version               string                 `json:"version"`
+	ResourceModifierRules []ResourceModifierRule `json:"resourceModifierRules"`
 }
 
 func GetResourceModifiersFromConfig(cm *v1.ConfigMap) (*ResourceModifiers, error) {
@@ -58,7 +60,7 @@ func GetResourceModifiersFromConfig(cm *v1.ConfigMap) (*ResourceModifiers, error
 		yamlData = v
 	}
 
-	resModifiers, err := unmarshalResourceModifiers(&yamlData)
+	resModifiers, err := unmarshalResourceModifiers([]byte(yamlData))
 	if err != nil {
 		return nil, errors.WithStack(err)
 	}
@@ -83,9 +85,11 @@ func (r *ResourceModifierRule) Apply(obj *unstructured.Unstructured, groupResour
 	if !namespaceInclusion.ShouldInclude(obj.GetNamespace()) {
 		return nil
 	}
-	if !strings.EqualFold(groupResource, r.Conditions.GroupKind) {
+
+	if !strings.EqualFold(groupResource, r.Conditions.GroupResource) {
 		return nil
 	}
+
 	if r.Conditions.ResourceNameRegex != "" {
 		match, err := regexp.MatchString(r.Conditions.ResourceNameRegex, obj.GetName())
 		if err != nil {
@@ -95,6 +99,17 @@ func (r *ResourceModifierRule) Apply(obj *unstructured.Unstructured, groupResour
 			return nil
 		}
 	}
+
+	if r.Conditions.LabelSelector != nil {
+		selector, err := metav1.LabelSelectorAsSelector(r.Conditions.LabelSelector)
+		if err != nil {
+			return errors.Errorf("error in creating label selector %s", err.Error())
+		}
+		if !selector.Matches(labels.Set(obj.GetLabels())) {
+			return nil
+		}
+	}
+
 	patches, err := r.PatchArrayToByteArray()
 	if err != nil {
 		return err
@@ -107,7 +122,7 @@ func (r *ResourceModifierRule) Apply(obj *unstructured.Unstructured, groupResour
 	return nil
 }
 
-// convert all JsonPatch to string array with the format of jsonpatch.Patch and then convert it to byte array
+// PatchArrayToByteArray converts all JsonPatch to string array with the format of jsonpatch.Patch and then convert it to byte array
 func (r *ResourceModifierRule) PatchArrayToByteArray() ([]byte, error) {
 	var patches []string
 	for _, patch := range r.Patches {
@@ -148,22 +163,15 @@ func ApplyPatch(patch []byte, obj *unstructured.Unstructured, log logrus.FieldLo
 	return nil
 }
 
-func unmarshalResourceModifiers(yamlData *string) (*ResourceModifiers, error) {
+func unmarshalResourceModifiers(yamlData []byte) (*ResourceModifiers, error) {
 	resModifiers := &ResourceModifiers{}
-	err := decodeStruct(strings.NewReader(*yamlData), resModifiers)
+	err := yaml.UnmarshalStrict(yamlData, resModifiers)
 	if err != nil {
 		return nil, fmt.Errorf("failed to decode yaml data into resource modifiers  %v", err)
 	}
 	return resModifiers, nil
 }
 
-// decodeStruct restrict validate the keys in decoded mappings to exist as fields in the struct being decoded into
-func decodeStruct(r io.Reader, s interface{}) error {
-	dec := yaml.NewDecoder(r)
-	dec.KnownFields(true)
-	return dec.Decode(s)
-}
-
 func addQuotes(value string) bool {
 	if value == "" {
 		return true

internal/resourcemodifiers/resource_modifiers_test.go

@@ -18,7 +18,7 @@ func TestGetResourceModifiersFromConfig(t *testing.T) {
 			Namespace: "test-namespace",
 		},
 		Data: map[string]string{
-			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupKind: persistentvolumeclaims\n    resourceNameRegex: \".*\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: replace\n    path: \"/spec/storageClassName\"\n    value: \"premium\"\n  - operation: remove\n    path: \"/metadata/labels/test\"\n\n\n",
+			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupResource: persistentvolumeclaims\n    resourceNameRegex: \".*\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: replace\n    path: \"/spec/storageClassName\"\n    value: \"premium\"\n  - operation: remove\n    path: \"/metadata/labels/test\"\n\n\n",
 		},
 	}
 
@@ -27,7 +27,7 @@ func TestGetResourceModifiersFromConfig(t *testing.T) {
 		ResourceModifierRules: []ResourceModifierRule{
 			{
 				Conditions: Conditions{
-					GroupKind:         "persistentvolumeclaims",
+					GroupResource:     "persistentvolumeclaims",
 					ResourceNameRegex: ".*",
 					Namespaces:        []string{"bar", "foo"},
 				},
@@ -51,7 +51,7 @@ func TestGetResourceModifiersFromConfig(t *testing.T) {
 			Namespace: "test-namespace",
 		},
 		Data: map[string]string{
-			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupKind: deployments.apps\n    resourceNameRegex: \"^test-.*$\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: add\n    path: \"/spec/template/spec/containers/0\"\n    value: \"{\\\"name\\\": \\\"nginx\\\", \\\"image\\\": \\\"nginx:1.14.2\\\", \\\"ports\\\": [{\\\"containerPort\\\": 80}]}\"\n  - operation: copy\n    from: \"/spec/template/spec/containers/0\"\n    path: \"/spec/template/spec/containers/1\"\n\n\n",
+			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupResource: deployments.apps\n    resourceNameRegex: \"^test-.*$\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: add\n    path: \"/spec/template/spec/containers/0\"\n    value: \"{\\\"name\\\": \\\"nginx\\\", \\\"image\\\": \\\"nginx:1.14.2\\\", \\\"ports\\\": [{\\\"containerPort\\\": 80}]}\"\n  - operation: copy\n    from: \"/spec/template/spec/containers/0\"\n    path: \"/spec/template/spec/containers/1\"\n\n\n",
 		},
 	}
 
@@ -60,7 +60,7 @@ func TestGetResourceModifiersFromConfig(t *testing.T) {
 		ResourceModifierRules: []ResourceModifierRule{
 			{
 				Conditions: Conditions{
-					GroupKind:         "deployments.apps",
+					GroupResource:     "deployments.apps",
 					ResourceNameRegex: "^test-.*$",
 					Namespaces:        []string{"bar", "foo"},
 				},
@@ -86,7 +86,7 @@ func TestGetResourceModifiersFromConfig(t *testing.T) {
 			Namespace: "test-namespace",
 		},
 		Data: map[string]string{
-			"sub.yml": "version1: v1\nresourceModifierRules:\n- conditions:\n    groupKind: deployments.apps\n    resourceNameRegex: \"^test-.*$\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: add\n    path: \"/spec/template/spec/containers/0\"\n    value: \"{\\\"name\\\": \\\"nginx\\\", \\\"image\\\": \\\"nginx:1.14.2\\\", \\\"ports\\\": [{\\\"containerPort\\\": 80}]}\"\n  - operation: copy\n    from: \"/spec/template/spec/containers/0\"\n    path: \"/spec/template/spec/containers/1\"\n\n\n",
+			"sub.yml": "version1: v1\nresourceModifierRules:\n- conditions:\n    groupResource: deployments.apps\n    resourceNameRegex: \"^test-.*$\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: add\n    path: \"/spec/template/spec/containers/0\"\n    value: \"{\\\"name\\\": \\\"nginx\\\", \\\"image\\\": \\\"nginx:1.14.2\\\", \\\"ports\\\": [{\\\"containerPort\\\": 80}]}\"\n  - operation: copy\n    from: \"/spec/template/spec/containers/0\"\n    path: \"/spec/template/spec/containers/1\"\n\n\n",
 		},
 	}
 
@@ -287,7 +287,7 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind:         "persistentvolumeclaims",
+							GroupResource:     "persistentvolumeclaims",
 							ResourceNameRegex: "[a-z",
 							Namespaces:        []string{"foo"},
 						},
@@ -320,7 +320,7 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind:         "persistentvolumeclaims",
+							GroupResource:     "persistentvolumeclaims",
 							ResourceNameRegex: ".*",
 							Namespaces:        []string{"foo"},
 						},
@@ -353,7 +353,7 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind:         "deployments.apps",
+							GroupResource:     "deployments.apps",
 							ResourceNameRegex: "^test-.*$",
 							Namespaces:        []string{"foo"},
 						},
@@ -386,7 +386,7 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind:         "deployments.apps",
+							GroupResource:     "deployments.apps",
 							ResourceNameRegex: "^test-.*$",
 							Namespaces:        []string{"foo"},
 						},
@@ -419,8 +419,8 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind:  "deployments.apps",
-							Namespaces: []string{"foo"},
+							GroupResource: "deployments.apps",
+							Namespaces:    []string{"foo"},
 						},
 						Patches: []JSONPatch{
 							{
@@ -451,7 +451,7 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind: "deployments.apps",
+							GroupResource: "deployments.apps",
 						},
 						Patches: []JSONPatch{
 							{
@@ -482,7 +482,7 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind:         "deployments.apps",
+							GroupResource:     "deployments.apps",
 							ResourceNameRegex: ".*",
 							Namespaces:        []string{"bar"},
 						},
@@ -515,7 +515,7 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind:         "deployments.apps",
+							GroupResource:     "deployments.apps",
 							ResourceNameRegex: "^test-.*$",
 							Namespaces:        []string{"foo"},
 						},
@@ -543,7 +543,7 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) {
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind:         "deployments.apps",
+							GroupResource:     "deployments.apps",
 							ResourceNameRegex: "^test-.*$",
 							Namespaces:        []string{"foo"},
 						},

internal/resourcemodifiers/resource_modifiers_validator.go

@@ -1,9 +1,8 @@
 package resourcemodifiers
 
 import (
-	"strings"
-
 	"fmt"
+	"strings"
 )
 
 func (r *ResourceModifierRule) Validate() error {
@@ -48,8 +47,8 @@ func (p *JSONPatch) Validate() error {
 }
 
 func (c *Conditions) Validate() error {
-	if c.GroupKind == "" {
-		return fmt.Errorf("groupkind cannot be empty")
+	if c.GroupResource == "" {
+		return fmt.Errorf("groupkResource cannot be empty")
 	}
 	return nil
 }

internal/resourcemodifiers/resource_modifiers_validator_test.go

@@ -21,7 +21,7 @@ func TestResourceModifiers_Validate(t *testing.T) {
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind:         "persistentvolumeclaims",
+							GroupResource:     "persistentvolumeclaims",
 							ResourceNameRegex: ".*",
 							Namespaces:        []string{"bar", "foo"},
 						},
@@ -44,7 +44,7 @@ func TestResourceModifiers_Validate(t *testing.T) {
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind:         "persistentvolumeclaims",
+							GroupResource:     "persistentvolumeclaims",
 							ResourceNameRegex: ".*",
 							Namespaces:        []string{"bar", "foo"},
 						},
@@ -75,7 +75,7 @@ func TestResourceModifiers_Validate(t *testing.T) {
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind:         "persistentvolumeclaims",
+							GroupResource:     "persistentvolumeclaims",
 							ResourceNameRegex: ".*",
 							Namespaces:        []string{"bar", "foo"},
 						},
@@ -92,13 +92,13 @@ func TestResourceModifiers_Validate(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			name: "Condition has empty GroupKind",
+			name: "Condition has empty GroupResource",
 			fields: fields{
 				Version: "v1",
 				ResourceModifierRules: []ResourceModifierRule{
 					{
 						Conditions: Conditions{
-							GroupKind:         "",
+							GroupResource:     "",
 							ResourceNameRegex: ".*",
 							Namespaces:        []string{"bar", "foo"},
 						},

pkg/controller/restore_controller_test.go

@@ -760,7 +760,7 @@ func TestValidateAndCompleteWithResourceModifierSpecified(t *testing.T) {
 			Namespace: velerov1api.DefaultNamespace,
 		},
 		Data: map[string]string{
-			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupKind: persistentvolumeclaims\n    resourceNameRegex: \".*\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: replace\n    path: \"/spec/storageClassName\"\n    value: \"premium\"\n  - operation: remove\n    path: \"/metadata/labels/test\"\n\n\n",
+			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupResource: persistentvolumeclaims\n    resourceNameRegex: \".*\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: replace\n    path: \"/spec/storageClassName\"\n    value: \"premium\"\n  - operation: remove\n    path: \"/metadata/labels/test\"\n\n\n",
 		},
 	}
 	require.NoError(t, r.kbClient.Create(context.Background(), cm1))
@@ -788,7 +788,7 @@ func TestValidateAndCompleteWithResourceModifierSpecified(t *testing.T) {
 			Namespace: velerov1api.DefaultNamespace,
 		},
 		Data: map[string]string{
-			"sub.yml": "version1: v1\nresourceModifierRules:\n- conditions:\n    groupKind: persistentvolumeclaims\n    resourceNameRegex: \".*\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: replace\n    path: \"/spec/storageClassName\"\n    value: \"premium\"\n  - operation: remove\n    path: \"/metadata/labels/test\"\n\n\n",
+			"sub.yml": "version1: v1\nresourceModifierRules:\n- conditions:\n    groupResource: persistentvolumeclaims\n    resourceNameRegex: \".*\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: replace\n    path: \"/spec/storageClassName\"\n    value: \"premium\"\n  - operation: remove\n    path: \"/metadata/labels/test\"\n\n\n",
 		},
 	}
 	require.NoError(t, r.kbClient.Create(context.Background(), invalidVersionCm))
@@ -816,7 +816,7 @@ func TestValidateAndCompleteWithResourceModifierSpecified(t *testing.T) {
 			Namespace: velerov1api.DefaultNamespace,
 		},
 		Data: map[string]string{
-			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupKind: persistentvolumeclaims\n    resourceNameRegex: \".*\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: invalid\n    path: \"/spec/storageClassName\"\n    value: \"premium\"\n  - operation: remove\n    path: \"/metadata/labels/test\"\n\n\n",
+			"sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n    groupResource: persistentvolumeclaims\n    resourceNameRegex: \".*\"\n    namespaces:\n    - bar\n    - foo\n  patches:\n  - operation: invalid\n    path: \"/spec/storageClassName\"\n    value: \"premium\"\n  - operation: remove\n    path: \"/metadata/labels/test\"\n\n\n",
 		},
 	}
 	require.NoError(t, r.kbClient.Create(context.Background(), invalidOperatorCm))