Repository: FairwindsOps/polaris
Pull request #734 (2022-03-29 15:18:31)

Added more mutations and refactor test to test each mutation separately
This PR fixes #

## Checklist
* [x] I have signed the CLA
* [x] I have updated/added any relevant documentation

## Description
### What's the goal of this PR?

### What changes did you make?

### What alternative solution should we consider, if any?




Patch:
diff --git a/checks/cpuLimitsMissing.yaml b/checks/cpuLimitsMissing.yaml
index 6601e03d..2e495524 100644
--- a/checks/cpuLimitsMissing.yaml
+++ b/checks/cpuLimitsMissing.yaml
@@ -25,3 +25,7 @@ schema:
               type: string
               not:
                 const: ''
+mutations:
+  - op: add
+    path: /resources/limits/cpu
+    value: 100m
\ No newline at end of file
diff --git a/checks/cpuRequestsMissing.yaml b/checks/cpuRequestsMissing.yaml
index f63ffdd4..cd1a56fb 100644
--- a/checks/cpuRequestsMissing.yaml
+++ b/checks/cpuRequestsMissing.yaml
@@ -25,3 +25,7 @@ schema:
               type: string
               not:
                 const: ''
+mutations:
+  - op: add
+    path: /resources/requests/cpu
+    value: 100m
\ No newline at end of file
diff --git a/checks/deploymentMissingReplicas.yaml b/checks/deploymentMissingReplicas.yaml
index 6c2a921d..19f561a9 100644
--- a/checks/deploymentMissingReplicas.yaml
+++ b/checks/deploymentMissingReplicas.yaml
@@ -19,3 +19,7 @@ schema:
         replicas:
           type: integer
           minimum: 2
+mutations:
+  - op: add
+    path: /spec/replicas
+    value: 3
\ No newline at end of file
diff --git a/checks/hostIPCSet.yaml b/checks/hostIPCSet.yaml
index db62a24f..7a3607e1 100644
--- a/checks/hostIPCSet.yaml
+++ b/checks/hostIPCSet.yaml
@@ -9,3 +9,6 @@ schema:
     hostIPC:
       not:
         const: true
+mutations:
+  - op: remove
+    path: /hostIPC
\ No newline at end of file
diff --git a/checks/hostNetworkSet.yaml b/checks/hostNetworkSet.yaml
index c8574874..29965475 100644
--- a/checks/hostNetworkSet.yaml
+++ b/checks/hostNetworkSet.yaml
@@ -9,3 +9,6 @@ schema:
     hostNetwork:
       not:
         const: true
+mutations:
+  - op: remove
+    path: /hostNetwork
\ No newline at end of file
diff --git a/checks/hostPIDSet.yaml b/checks/hostPIDSet.yaml
index d2b0a65c..48336f37 100644
--- a/checks/hostPIDSet.yaml
+++ b/checks/hostPIDSet.yaml
@@ -9,3 +9,6 @@ schema:
     hostPID:
       not:
         const: true
+mutations:
+  - op: remove
+    path: /hostPID
\ No newline at end of file
diff --git a/checks/hostPortSet.yaml b/checks/hostPortSet.yaml
index ea4da62d..508bbdbd 100644
--- a/checks/hostPortSet.yaml
+++ b/checks/hostPortSet.yaml
@@ -12,4 +12,4 @@ schema:
       items:
         properties:
           hostPort:
-            const: 0
+            const: 0
\ No newline at end of file
diff --git a/checks/livenessProbeMissing.yaml b/checks/livenessProbeMissing.yaml
index 2456bd6a..0b9ed043 100644
--- a/checks/livenessProbeMissing.yaml
+++ b/checks/livenessProbeMissing.yaml
@@ -19,3 +19,7 @@ schema:
       type: object
       not:
         const: null
+mutations:
+  - op: add
+    path: /livenessProbe
+    value: {"exec": { "command": [ "cat", "/tmp/healthy" ] }, "initialDelaySeconds": 5, "periodSeconds": 5 }
\ No newline at end of file
diff --git a/checks/memoryLimitsMissing.yaml b/checks/memoryLimitsMissing.yaml
index 53ffb25b..431885a9 100644
--- a/checks/memoryLimitsMissing.yaml
+++ b/checks/memoryLimitsMissing.yaml
@@ -25,3 +25,10 @@ schema:
               type: string
               not:
                 const: ''
+mutations:
+  - op: add
+    path: /resources/limits
+    value: {}
+  - op: add
+    path: /resources/limits/memory
+    value: "512Mi"
\ No newline at end of file
diff --git a/checks/memoryRequestsMissing.yaml b/checks/memoryRequestsMissing.yaml
index 3fce46ab..9051acfb 100644
--- a/checks/memoryRequestsMissing.yaml
+++ b/checks/memoryRequestsMissing.yaml
@@ -25,3 +25,10 @@ schema:
               type: string
               not:
                 const: ''
+mutations:
+  - op: add
+    path: /resources/requests
+    value: {}
+  - op: add
+    path: /resources/requests/memory
+    value: "512Mi"
\ No newline at end of file
diff --git a/checks/priorityClassNotSet.yaml b/checks/priorityClassNotSet.yaml
index 3516a3fe..49690027 100644
--- a/checks/priorityClassNotSet.yaml
+++ b/checks/priorityClassNotSet.yaml
@@ -7,3 +7,7 @@ schema:
   type: object
   required:
   - priorityClassName
+mutations:
+  - op: add
+    path: /priorityClassName
+    value: high-priority
\ No newline at end of file
diff --git a/checks/readinessProbeMissing.yaml b/checks/readinessProbeMissing.yaml
index c655e13c..576370ef 100644
--- a/checks/readinessProbeMissing.yaml
+++ b/checks/readinessProbeMissing.yaml
@@ -19,3 +19,7 @@ schema:
       type: object
       not:
         const: null
+mutations:
+  - op: add
+    path: /readinessProbe
+    value: {"exec": { "command": [ "cat", "/tmp/healthy" ] }, "initialDelaySeconds": 5, "periodSeconds": 5 }
\ No newline at end of file
diff --git a/checks/runAsPrivileged.yaml b/checks/runAsPrivileged.yaml
index a0f77169..d641c752 100644
--- a/checks/runAsPrivileged.yaml
+++ b/checks/runAsPrivileged.yaml
@@ -21,4 +21,8 @@ schema:
       items:
         properties:
           securityContext:
-            $ref: "#/definitions/notBadSecurityContext"
\ No newline at end of file
+            $ref: "#/definitions/notBadSecurityContext"
+mutations:
+  - op: add
+    path: /securityContext/privileged
+    value: false
\ No newline at end of file
diff --git a/checks/runAsRootAllowed.yaml b/checks/runAsRootAllowed.yaml
index 3ae2fe49..a56fe02c 100644
--- a/checks/runAsRootAllowed.yaml
+++ b/checks/runAsRootAllowed.yaml
@@ -50,3 +50,7 @@ schema:
           properties:
             securityContext:
               $ref: "#/definitions/goodSecurityContext"
+mutations:
+  - op: replace
+    path: /securityContext/runAsNonRoot
+    value: true
\ No newline at end of file
diff --git a/pkg/validator/schema.go b/pkg/validator/schema.go
index ea167815..6b9b9409 100644
--- a/pkg/validator/schema.go
+++ b/pkg/validator/schema.go
@@ -261,6 +261,13 @@ func applySchemaCheck(conf *config.Configuration, checkID string, test schemaTes
 			podCopy := *test.Resource.PodSpec
 			podCopy.InitContainers = []corev1.Container{}
 			podCopy.Containers = []corev1.Container{*test.Container}
+			containerIndex := funk.IndexOf(test.Resource.PodSpec.Containers, func(value corev1.Container) bool {
+				return value.Name == test.Container.Name
+			})
+			prefix = getJSONSchemaPrefix(test.Resource.Kind)
+			if prefix != "" {
+				prefix += "/containers/" + strconv.Itoa(containerIndex)
+			}
 			passes, issues, err = check.CheckPod(&podCopy)
 		} else {
 			return nil, fmt.Errorf("Unknown combination of target (%s) and schema target (%s)", check.Target, check.SchemaTarget)


Test patch:
diff --git a/test/checks/cpuLimitsMissing/success.yaml b/test/checks/cpuLimitsMissing/success.yaml
index 7abd72b7..259c44f0 100644
--- a/test/checks/cpuLimitsMissing/success.yaml
+++ b/test/checks/cpuLimitsMissing/success.yaml
@@ -10,5 +10,5 @@ spec:
     image: nginx
     resources: 
       limits: 
-        memory: 256Mi
+        memory: 128Mi
         cpu: 100m
\ No newline at end of file
diff --git a/test/checks/cpuRequestsMissing/success.yaml b/test/checks/cpuRequestsMissing/success.yaml
index 50d0625b..2a6b20bc 100644
--- a/test/checks/cpuRequestsMissing/success.yaml
+++ b/test/checks/cpuRequestsMissing/success.yaml
@@ -10,5 +10,5 @@ spec:
     image: nginx
     resources: 
       requests: 
-        memory: 256Mi
+        memory: 128Mi
         cpu: 100m
\ No newline at end of file
diff --git a/test/checks/livenessProbeMissing/failure.yaml b/test/checks/livenessProbeMissing/failure.yaml
index 258eff81..b7264b8c 100644
--- a/test/checks/livenessProbeMissing/failure.yaml
+++ b/test/checks/livenessProbeMissing/failure.yaml
@@ -11,4 +11,4 @@ spec:
     args:
     - /bin/sh
     - -c
-    - touch /tmp/healthy; sleep 30; rm -rf /tmp/healthy; sleep 600.
\ No newline at end of file
+    - touch /tmp/healthy; sleep 30; rm -rf /tmp/healthy; sleep 600
\ No newline at end of file
diff --git a/test/checks/memoryRequestsMissing/success.yaml b/test/checks/memoryRequestsMissing/success.yaml
index f1feafe4..952db9f6 100644
--- a/test/checks/memoryRequestsMissing/success.yaml
+++ b/test/checks/memoryRequestsMissing/success.yaml
@@ -10,4 +10,4 @@ spec:
     image: nginx
     resources:
       requests:
-        memory: "128Mi"
\ No newline at end of file
+        memory: "512Mi"
\ No newline at end of file
diff --git a/test/checks/readinessProbeMissing/failure.yaml b/test/checks/readinessProbeMissing/failure.yaml
index 0bd88d3a..40e6f979 100644
--- a/test/checks/readinessProbeMissing/failure.yaml
+++ b/test/checks/readinessProbeMissing/failure.yaml
@@ -11,4 +11,4 @@ spec:
     args:
     - /bin/sh
     - -c
-    - touch /tmp/healthy; sleep 30; rm -rf /tmp/healthy; sleep 600.
\ No newline at end of file
+    - touch /tmp/healthy; sleep 30; rm -rf /tmp/healthy; sleep 600
\ No newline at end of file
diff --git a/test/mutation_test.go b/test/mutation_test.go
index 6f954144..ba8d39da 100644
--- a/test/mutation_test.go
+++ b/test/mutation_test.go
@@ -9,41 +9,53 @@ import (
 	"github.com/fairwindsops/polaris/pkg/mutation"
 	"github.com/fairwindsops/polaris/pkg/validator"
 	"github.com/stretchr/testify/assert"
-	"github.com/thoas/go-funk"
 )
 
 var configYaml = `
 checks:
   pullPolicyNotAlways: warning
-mutations:
-  - pullPolicyNotAlways
+  hostIPCSet: danger
+  hostPIDSet: danger
+  hostNetworkSet: danger
+  deploymentMissingReplicas: warning
+  priorityClassNotSet: ignore
+  runAsRootAllowed: danger
+  cpuRequestsMissing: warning
+  cpuLimitsMissing: warning
+  memoryRequestsMissing: warning
+  memoryLimitsMissing: warning
+  readinessProbeMissing: warning
+  livenessProbeMissing: warning
 `
 
 func TestMutations(t *testing.T) {
 	c, err := config.Parse([]byte(configYaml))
 	assert.NoError(t, err)
-	assert.Len(t, c.Mutations, 1)
-
-	for _, tc := range testCases {
-		if tc.failure && funk.Contains(c.Mutations, tc.check) {
+	assert.Len(t, c.Mutations, 0)
+	mutations := []string{"hostIPCSet", "pullPolicyNotAlways", "hostPIDSet", "hostNetworkSet", "deploymentMissingReplicas", "runAsRootAllowed", "cpuRequestsMissing", "cpuLimitsMissing", "memoryRequestsMissing", "memoryLimitsMissing", "livenessProbeMissing", "readinessProbeMissing"}
+	for _, mutationStr := range mutations {
+		for _, tc := range failureTestCasesMap[mutationStr] {
+			newConfig := c
 			key := fmt.Sprintf("%s/%s", tc.check, strings.ReplaceAll(tc.filename, "failure", "success"))
 			successResources, ok := successResourceMap[key]
 			assert.True(t, ok)
 			assert.Len(t, tc.resources.Resources, 1)
 			assert.Len(t, successResources.Resources, 1)
-			results, err := validator.ApplyAllSchemaChecksToResourceProvider(&c, tc.resources)
+			newConfig.Mutations = []string{mutationStr}
+			results, err := validator.ApplyAllSchemaChecksToResourceProvider(&newConfig, tc.resources)
 			assert.NoError(t, err)
 			assert.Len(t, results, 1)
 			allMutations := mutation.GetMutationsFromResults(&c, results)
 			assert.Len(t, allMutations, 1)
 			for kind, resources := range tc.resources.Resources {
+				assert.Len(t, resources, 1)
 				key := fmt.Sprintf("%s/%s/%s", resources[0].Kind, resources[0].Resource.GetName(), resources[0].Resource.GetNamespace())
 				mutations := allMutations[key]
-				assert.Len(t, mutations, 1)
 				mutated, err := mutation.ApplyAllSchemaMutations(&c, tc.resources, resources[0], mutations)
 				assert.NoError(t, err)
 				expected := successResources.Resources[kind][0]
-				assert.Equal(t, expected.Resource.Object, mutated.Resource.Object)
+				// use fmt.sprint incase type casting issues for int and float.
+				assert.EqualValues(t, fmt.Sprint(expected.Resource.Object), fmt.Sprint(mutated.Resource.Object))
 			}
 		}
 	}
diff --git a/test/schema_test.go b/test/schema_test.go
index 9fee950c..ed9ba000 100644
--- a/test/schema_test.go
+++ b/test/schema_test.go
@@ -39,6 +39,7 @@ type testCase struct {
 }
 
 var successResourceMap = map[string]*kube.ResourceProvider{}
+var failureTestCasesMap = map[string][]testCase{}
 
 func init() {
 	_, baseDir, _, _ := runtime.Caller(0)
@@ -59,15 +60,24 @@ func init() {
 			if err != nil {
 				panic(err)
 			}
-			testCases = append(testCases, testCase{
+			testcase := testCase{
 				filename:  tc.Name(),
 				check:     check,
 				resources: resources,
 				failure:   strings.Contains(tc.Name(), "failure"),
-			})
-			if !strings.Contains(tc.Name(), "failure") {
+			}
+			testCases = append(testCases, testcase)
+
+			if strings.Contains(tc.Name(), "success") {
 				key := fmt.Sprintf("%s/%s", check, tc.Name())
 				successResourceMap[key] = resources
+			} else {
+				testCases, ok := failureTestCasesMap[check]
+				if !ok {
+					testCases = []testCase{}
+				}
+				testCases = append(testCases, testcase)
+				failureTestCasesMap[check] = testCases
 			}
 		}
 	}