Added more mutations and refactor test to test each mutation separately (#734)

* added more mutations and refactor test to test each mutation separately

* added more mutation definitions

* update spec for controller

* added mutations for cpu and memory request and limits

* update request memory mutation

* added liveness and probes

* rmeove hostport mutation

* added multiple mutations for request and limits memory

Co-authored-by: Robert Brennan <accounts@rbren.io>

checks/cpuLimitsMissing.yaml

@@ -25,3 +25,7 @@ schema:
               type: string
               not:
                 const: ''
+mutations:
+  - op: add
+    path: /resources/limits/cpu
+    value: 100m

checks/cpuRequestsMissing.yaml

@@ -25,3 +25,7 @@ schema:
               type: string
               not:
                 const: ''
+mutations:
+  - op: add
+    path: /resources/requests/cpu
+    value: 100m

checks/deploymentMissingReplicas.yaml

@@ -19,3 +19,7 @@ schema:
         replicas:
           type: integer
           minimum: 2
+mutations:
+  - op: add
+    path: /spec/replicas
+    value: 3

checks/hostIPCSet.yaml

@@ -9,3 +9,6 @@ schema:
     hostIPC:
       not:
         const: true
+mutations:
+  - op: remove
+    path: /hostIPC

checks/hostNetworkSet.yaml

@@ -9,3 +9,6 @@ schema:
     hostNetwork:
       not:
         const: true
+mutations:
+  - op: remove
+    path: /hostNetwork

checks/hostPIDSet.yaml

@@ -9,3 +9,6 @@ schema:
     hostPID:
       not:
         const: true
+mutations:
+  - op: remove
+    path: /hostPID

checks/hostPortSet.yaml

@@ -12,4 +12,4 @@ schema:
       items:
         properties:
           hostPort:
-            const: 0
+            const: 0

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 }

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"

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"

checks/priorityClassNotSet.yaml

@@ -7,3 +7,7 @@ schema:
   type: object
   required:
   - priorityClassName
+mutations:
+  - op: add
+    path: /priorityClassName
+    value: high-priority

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 }

checks/runAsPrivileged.yaml

@@ -21,4 +21,8 @@ schema:
       items:
         properties:
           securityContext:
-            $ref: "#/definitions/notBadSecurityContext"
+            $ref: "#/definitions/notBadSecurityContext"
+mutations:
+  - op: add
+    path: /securityContext/privileged
+    value: false

checks/runAsRootAllowed.yaml

@@ -50,3 +50,7 @@ schema:
           properties:
             securityContext:
               $ref: "#/definitions/goodSecurityContext"
+mutations:
+  - op: replace
+    path: /securityContext/runAsNonRoot
+    value: true

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/checks/cpuLimitsMissing/success.yaml

@@ -10,5 +10,5 @@ spec:
     image: nginx
     resources: 
       limits: 
-        memory: 256Mi
+        memory: 128Mi
         cpu: 100m

test/checks/cpuRequestsMissing/success.yaml

@@ -10,5 +10,5 @@ spec:
     image: nginx
     resources: 
       requests: 
-        memory: 256Mi
+        memory: 128Mi
         cpu: 100m

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.
+    - touch /tmp/healthy; sleep 30; rm -rf /tmp/healthy; sleep 600

test/checks/memoryRequestsMissing/success.yaml

@@ -10,4 +10,4 @@ spec:
     image: nginx
     resources:
       requests:
-        memory: "128Mi"
+        memory: "512Mi"

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.
+    - touch /tmp/healthy; sleep 30; rm -rf /tmp/healthy; sleep 600

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))
 			}
 		}
 	}

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
 			}
 		}
 	}