fix: added edge case tests for regex and reject for source

kubernetes-sigs · Jun 21, 2024 · f4441a8 · f4441a8
1 parent 4210131
commit f4441a8
Show file tree

Hide file tree

Showing 3 changed files with 250 additions and 5 deletions.
diff --git a/api/filters/replacement/replacement.go b/api/filters/replacement/replacement.go
@@ -74,6 +74,13 @@ func selectSourceNode(nodes []*yaml.RNode, selector *types.SourceSelector) (*yam
 		if err != nil {
 			return nil, fmt.Errorf("error getting node IDs: %w", err)
 		}
+		selectByAnnoAndLabel, err := rejectByAnnoAndLabel(n, selector.Reject)
+		if err != nil {
+			return nil, err
+		}
+		if !selectByAnnoAndLabel {
+			continue
+		}
 		for _, id := range ids {
 			if id.IsSelectedBy(selector.ResId) {
 				if len(matches) > 0 {
@@ -139,7 +146,7 @@ func applyReplacement(nodes []*yaml.RNode, value *yaml.RNode, targetSelectors []
 			}
 
 			// filter targets by label and annotation selectors
-			selectByAnnoAndLabel, err := selectByAnnoAndLabel(possibleTarget, selector)
+			selectByAnnoAndLabel, err := selectByAnnoAndLabel(possibleTarget, selector.Select, selector.Reject)
 			if err != nil {
 				return nil, err
 			}
@@ -162,11 +169,15 @@ func applyReplacement(nodes []*yaml.RNode, value *yaml.RNode, targetSelectors []
 	return nodes, nil
 }
 
-func selectByAnnoAndLabel(n *yaml.RNode, t *types.TargetSelector) (bool, error) {
-	if matchesSelect, err := matchesAnnoAndLabelSelector(n, t.Select); !matchesSelect || err != nil {
+func selectByAnnoAndLabel(n *yaml.RNode, s *types.Selector, r []*types.Selector) (bool, error) {
+	if matchesSelect, err := matchesAnnoAndLabelSelector(n, s); !matchesSelect || err != nil {
 		return false, err
 	}
-	for _, reject := range t.Reject {
+	return rejectByAnnoAndLabel(n, r)
+}
+
+func rejectByAnnoAndLabel(n *yaml.RNode, r []*types.Selector) (bool, error) {
+	for _, reject := range r {
 		if reject.AnnotationSelector == "" && reject.LabelSelector == "" {
 			continue
 		}
@@ -177,6 +188,7 @@ func selectByAnnoAndLabel(n *yaml.RNode, t *types.TargetSelector) (bool, error)
 	return true, nil
 }
 
+
 func matchesAnnoAndLabelSelector(n *yaml.RNode, selector *types.Selector) (bool, error) {
 	r := resource.Resource{RNode: *n}
 	annoMatch, err := r.MatchesAnnotationSelector(selector.AnnotationSelector)
@@ -283,6 +295,10 @@ func getByDelimiter(delimiter string, target string, source string, index int) s
 }
 
 func getByRegex(regex string, target string, source string, index int) string {
+	_, err := regexp.Compile(pattern)
+	if err != nil {
+		return nil, fmt.Errorf("the regex: %s is not valid.", regex)
+	}
 	re := regexp.MustCompile(regex)
 	counter := 0
 	res := re.ReplaceAllStringFunc(target, func(str string) string {
@@ -293,6 +309,6 @@ func getByRegex(regex string, target string, source string, index int) string {
 		counter++
 		return re.ReplaceAllString(str, source)
 	})
-	return res
+	return res, nil
 }
 
diff --git a/api/filters/replacement/replacement_test.go b/api/filters/replacement/replacement_test.go
@@ -3465,6 +3465,155 @@ spec:
         name: postgresdb
 `,
 		},
+    "multiple matches for source select with reject": {
+			input: `apiVersion: v1
+kind: Deployment
+metadata:
+  name: deploy1
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx:1.7.9
+        name: nginx-tagged
+      - image: postgres:1.8.0
+        name: postgresdb
+---
+apiVersion: v1
+kind: Deployment
+metadata:
+  name: deploy2
+  labels:
+    foo: bar
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx:1.7.9
+        name: nginx-tagged
+      - image: postgres:1.8.0
+        name: postgresdb
+---
+apiVersion: v1
+kind: Deployment
+metadata:
+  name: deploy3
+  labels:
+    foo: bar
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx:1.7.9
+        name: nginx-tagged
+      - image: postgres:1.8.0
+        name: postgresdb
+`,
+			replacements: `replacements:
+- source:
+    kind: Deployment
+    reject:
+      - labelSelector: foo=bar
+  targets:
+  - select:
+      kind: Deployment
+`,
+			expected: `apiVersion: v1
+kind: Deployment
+metadata:
+  name: deploy1
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx:1.7.9
+        name: nginx-tagged
+      - image: postgres:1.8.0
+        name: postgresdb
+---
+apiVersion: v1
+kind: Deployment
+metadata:
+  name: deploy1
+  labels:
+    foo: bar
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx:1.7.9
+        name: nginx-tagged
+      - image: postgres:1.8.0
+        name: postgresdb
+---
+apiVersion: v1
+kind: Deployment
+metadata:
+  name: deploy1
+  labels:
+    foo: bar
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx:1.7.9
+        name: nginx-tagged
+      - image: postgres:1.8.0
+        name: postgresdb`,
+		},
+    "multiple matches for source select with reject but still too many matches": {
+			input: `apiVersion: v1
+kind: Deployment
+metadata:
+  name: deploy1
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx:1.7.9
+        name: nginx-tagged
+      - image: postgres:1.8.0
+        name: postgresdb
+---
+apiVersion: v1
+kind: Deployment
+metadata:
+  name: deploy2
+  labels:
+    foo: bar
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx:1.7.9
+        name: nginx-tagged
+      - image: postgres:1.8.0
+        name: postgresdb
+---
+apiVersion: v1
+kind: Deployment
+metadata:
+  name: deploy3
+spec:
+  template:
+    spec:
+      containers:
+      - image: nginx:1.7.9
+        name: nginx-tagged
+      - image: postgres:1.8.0
+        name: postgresdb
+`,
+			replacements: `replacements:
+- source:
+    kind: Deployment
+    reject:
+      - labelSelector: foo=bar
+  targets:
+  - select:
+      kind: Deployment
+`,
+			expectedErr: `multiple matches for selector Deployment.[noVer].[noGrp]/[noName].[noNs]`,
+		},
 	}
 
 	for tn, tc := range testCases {
@@ -3490,3 +3639,80 @@ spec:
 		})
 	}
 }
+
+func TestGetByRegex(t *testing.T) {
+  testCases := map[string]struct {
+		regex        string
+		target       string
+		source       string
+		index        int
+    want         string
+	}{ "simple": {
+    regex: `THIS`,
+    target: `replace THIS word`,
+    source: `REPLACED`,
+    index: 0,
+    want: `replace REPLACED word`,
+  },
+  "simple replace all": {
+    regex: `THIS`,
+    target: `replace THIS word THIS`,
+    source: `REPLACED`,
+    index: -1,
+    want: `replace REPLACED word REPLACED`,
+  },
+  "simple replace with index": {
+    regex: `THIS`,
+    target: `replace THIS word THIS`,
+    source: `REPLACED`,
+    index: 0,
+    want: `replace REPLACED word THIS`,
+  },
+  "simple replace target has no matching regex": {
+    regex: `foo`,
+    target: `this is a target`,
+    source: `bar`,
+    index: 0,
+    want: `this is a target`,
+  },
+  "simple replace source is empty": {
+    regex: `this`,
+    target: `this is a target`,
+    source: ``,
+    index: 0,
+    want: ` is a target`,
+  },
+  "simple replace source with point": {
+    regex: `.`,
+    target: `this is a target`,
+    source: `a`,
+    index: 0,
+    want: `ahis is a target`,
+  },"simple replace all alphanumeric character": {
+    regex: `\w`,
+    target: `this is a target`,
+    source: `a`,
+    index: -1,
+    want: `aaaa aa a aaaaaa`,
+  },"simple replace index out of bounds": {
+    regex: `this`,
+    target: `this is a target`,
+    source: `bar`,
+    index: 5,
+    want: `this is a target`,
+  },
+  "simple replace with star": {
+    regex: `this*`,
+    target: `this is a target`,
+    source: `bar`,
+    index: 5,
+    want: `this is a target`,
+  },
+  }
+  for _, tc := range testCases {
+    res := getByRegex(tc.regex, tc.target, tc.source, tc.index)
+    if !assert.Equal(t, tc.want, res) {
+      t.FailNow()
+    }
+  }
+}
diff --git a/api/types/replacement.go b/api/types/replacement.go
@@ -32,6 +32,9 @@ type SourceSelector struct {
 
 	// Structured field path expected in the allowed object.
 	FieldPath string `json:"fieldPath,omitempty" yaml:"fieldPath,omitempty"`
+
+	// From the allowed set, remove objects that match this.
+	Reject []*Selector `json:"reject,omitempty" yaml:"reject,omitempty"`
 
 	// Used to refine the interpretation of the field.
 	Options *FieldOptions `json:"options,omitempty" yaml:"options,omitempty"`