Repository: FairwindsOps/polaris
Pull request #790 (2022-07-07 14:03:08)

Add more mutations, fix mutation tests
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?
* Add a few more good mutations
* Fix the mutation tests

### What changes did you make?

### What alternative solution should we consider, if any?



fairwinds-insights[bot]: ## Fairwinds Insights CI Report
> [View the Full Report](https://insights.fairwinds.com/gh/FairwindsOps/polaris/rb%2Ffix-tests)

:white_check_mark: No new Action Items detected!

Patch:
diff --git a/checks/dangerousCapabilities.yaml b/checks/dangerousCapabilities.yaml
index f2bd6d65..30e68e55 100644
--- a/checks/dangerousCapabilities.yaml
+++ b/checks/dangerousCapabilities.yaml
@@ -24,3 +24,8 @@ schema:
                 - not:
                     contains:
                       pattern: '^(?i)NET_ADMIN$'
+
+
+mutations:
+  - op: remove
+    path: /securityContext/capabilities
diff --git a/checks/insecureCapabilities.yaml b/checks/insecureCapabilities.yaml
index d9ec436d..1d694840 100644
--- a/checks/insecureCapabilities.yaml
+++ b/checks/insecureCapabilities.yaml
@@ -54,3 +54,9 @@ schema:
                     pattern: '^(?i)KILL$'
                 - contains:
                     pattern: '^(?i)AUDIT_WRITE$'
+mutations:
+  - op: remove
+    path: /securityContext/capabilities
+  - op: add
+    path: /securityContext/capabilities
+    value: {"drop": ["ALL"]}
diff --git a/checks/notReadOnlyRootFilesystem.yaml b/checks/notReadOnlyRootFilesystem.yaml
index d614089d..c77cf2b5 100644
--- a/checks/notReadOnlyRootFilesystem.yaml
+++ b/checks/notReadOnlyRootFilesystem.yaml
@@ -40,4 +40,9 @@ schema:
           - securityContext
           properties:
             securityContext:
-              $ref: "#/definitions/goodSecurityContext"
\ No newline at end of file
+              $ref: "#/definitions/goodSecurityContext"
+mutations:
+  - op: add
+    path: /securityContext/readOnlyRootFilesystem
+    value: true
+
diff --git a/cmd/polaris/fix.go b/cmd/polaris/fix.go
index 53a2efe3..41a04386 100644
--- a/cmd/polaris/fix.go
+++ b/cmd/polaris/fix.go
@@ -33,11 +33,16 @@ import (
 	"sigs.k8s.io/yaml"
 )
 
-var filesPath string
+var (
+	filesPath   string
+	checksToFix []string
+	fixAll      bool
+)
 
 func init() {
 	rootCmd.AddCommand(fixCommand)
 	fixCommand.PersistentFlags().StringVar(&filesPath, "files-path", "", "mutate and fix one or more YAML files in a specified folder")
+	fixCommand.PersistentFlags().StringSliceVar(&checksToFix, "checks", []string{}, "Optional flag to specify specific checks to fix eg. checks=hostIPCSet,hostPIDSet and checks=all applies fix to all defined checks mutations")
 }
 
 var fixCommand = &cobra.Command{
@@ -73,6 +78,19 @@ var fixCommand = &cobra.Command{
 		}
 		var contentStr string
 		isFirstResource := true
+
+		if len(checksToFix) > 0 {
+			if len(checksToFix) == 1 && checksToFix[0] == "all" {
+				allchecks := []string{}
+				for key := range config.Checks {
+					allchecks = append(allchecks, key)
+				}
+				config.Mutations = allchecks
+			} else {
+				config.Mutations = checksToFix
+			}
+		}
+
 		for _, fullFilePath := range yamlFiles {
 
 			yamlFile, err := ioutil.ReadFile(fullFilePath)
diff --git a/examples/config.yaml b/examples/config.yaml
index dadec568..5ea55016 100644
--- a/examples/config.yaml
+++ b/examples/config.yaml
@@ -27,7 +27,10 @@ checks:
   hostNetworkSet: danger
   hostPortSet: warning
   tlsSettingsMissing: warning
- 
+
+mutations:
+  - pullPolicyNotAlways
+
 exemptions:
   - namespace: kube-system
     controllerNames:
diff --git a/pkg/validator/schema.go b/pkg/validator/schema.go
index 258819ac..fc3b1f95 100644
--- a/pkg/validator/schema.go
+++ b/pkg/validator/schema.go
@@ -325,7 +325,7 @@ func applySchemaCheck(conf *config.Configuration, checkID string, test schemaTes
 	}
 	result := makeResult(conf, check, passes, issues)
 	if !passes {
-		if funk.Contains(conf.Mutations, checkID) {
+		if funk.Contains(conf.Mutations, checkID) && len(check.Mutations) > 0 {
 			mutations := funk.Map(check.Mutations, func(mutation jsonpatch.Operation) jsonpatch.Operation {
 				mutationCopy := deepCopyMutation(mutation)
 				mutationCopy.Path = prefix + mutationCopy.Path


Test patch:
diff --git a/test/checks/cpuLimitsMissing/mutated.yaml b/test/checks/cpuLimitsMissing/mutated.yaml
index b56c750f..9e693d88 100644
--- a/test/checks/cpuLimitsMissing/mutated.yaml
+++ b/test/checks/cpuLimitsMissing/mutated.yaml
@@ -10,5 +10,5 @@ spec:
     name: nginx
     resources:
       limits:
-        cpu: 100m #TODO: Set this to the amount of CPU you want to reserve for your workload
+        cpu: 100m #TODO: Set this to the maximum amount of CPU you want your workload to use
         memory: 128Mi
diff --git a/test/checks/dangerousCapabilities/failure.yaml b/test/checks/dangerousCapabilities/failure.yaml
index 28fe5ee7..cadd83f8 100644
--- a/test/checks/dangerousCapabilities/failure.yaml
+++ b/test/checks/dangerousCapabilities/failure.yaml
@@ -6,9 +6,9 @@ metadata:
     app.kubernetes.io/name: nginx
 spec:
   containers:
-  - name: nginx 
+  - name: nginx
     image: nginx
     securityContext:
       capabilities:
         add:
-        - NET_ADMIN
\ No newline at end of file
+        - NET_ADMIN
diff --git a/test/checks/dangerousCapabilities/mutated.yaml b/test/checks/dangerousCapabilities/mutated.yaml
new file mode 100644
index 00000000..5c978f18
--- /dev/null
+++ b/test/checks/dangerousCapabilities/mutated.yaml
@@ -0,0 +1,12 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    app.kubernetes.io/name: nginx
+  name: nginx
+spec:
+  containers:
+  - image: nginx
+    name: nginx
+    resources: {}
+    securityContext: {}
diff --git a/test/checks/insecureCapabilities/mutated.drop-most.yaml b/test/checks/insecureCapabilities/mutated.drop-most.yaml
new file mode 100644
index 00000000..2222f047
--- /dev/null
+++ b/test/checks/insecureCapabilities/mutated.drop-most.yaml
@@ -0,0 +1,15 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    env: test
+  name: nginx
+spec:
+  containers:
+  - image: nginx
+    name: nginx
+    resources: {}
+    securityContext:
+      capabilities:
+        drop:
+        - ALL
diff --git a/test/checks/insecureCapabilities/mutated.yaml b/test/checks/insecureCapabilities/mutated.yaml
new file mode 100644
index 00000000..2222f047
--- /dev/null
+++ b/test/checks/insecureCapabilities/mutated.yaml
@@ -0,0 +1,15 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    env: test
+  name: nginx
+spec:
+  containers:
+  - image: nginx
+    name: nginx
+    resources: {}
+    securityContext:
+      capabilities:
+        drop:
+        - ALL
diff --git a/test/checks/memoryLimitsMissing/failure.cpu.yaml b/test/checks/memoryLimitsMissing/failure.cpu.yaml
new file mode 100644
index 00000000..7666baba
--- /dev/null
+++ b/test/checks/memoryLimitsMissing/failure.cpu.yaml
@@ -0,0 +1,13 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: nginx
+  labels:
+    app.kubernetes.io/name: nginx
+spec:
+  containers:
+  - name: nginx
+    image: nginx
+    resources:
+      limits:
+        cpu: 100m
diff --git a/test/checks/memoryLimitsMissing/mutated.cpu.yaml b/test/checks/memoryLimitsMissing/mutated.cpu.yaml
new file mode 100644
index 00000000..2a4659f0
--- /dev/null
+++ b/test/checks/memoryLimitsMissing/mutated.cpu.yaml
@@ -0,0 +1,14 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    app.kubernetes.io/name: nginx
+  name: nginx
+spec:
+  containers:
+  - image: nginx
+    name: nginx
+    resources:
+      limits:
+        cpu: 100m
+        memory: 512Mi #TODO: Set this to the maximum amount of memory you want your workload to use
diff --git a/test/checks/memoryLimitsMissing/mutated.yaml b/test/checks/memoryLimitsMissing/mutated.yaml
index fbba0769..67200ce6 100644
--- a/test/checks/memoryLimitsMissing/mutated.yaml
+++ b/test/checks/memoryLimitsMissing/mutated.yaml
@@ -10,4 +10,4 @@ spec:
     name: nginx
     resources:
       limits:
-        memory: 512Mi #TODO: Set this to the amount of Memory you want to reserve for your workload
+        memory: 512Mi #TODO: Set this to the maximum amount of memory you want your workload to use
diff --git a/test/mutation_test.go b/test/mutation_test.go
index a2212981..92bee9eb 100644
--- a/test/mutation_test.go
+++ b/test/mutation_test.go
@@ -33,14 +33,18 @@ func TestMutations(t *testing.T) {
 	c, err := config.Parse([]byte(configYaml))
 	assert.NoError(t, err)
 	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] {
+	for mutationStr := range mutationTestCasesMap {
+		if len(mutationTestCasesMap[mutationStr]) == 0 {
+			panic("No test cases found for " + mutationStr)
+		}
+		for _, tc := range mutationTestCasesMap[mutationStr] {
 			newConfig := c
 			key := fmt.Sprintf("%s/%s", tc.check, strings.ReplaceAll(tc.filename, "failure", "mutated"))
 			mutatedYamlContent, ok := mutatedYamlContentMap[key]
 			assert.True(t, ok)
 			assert.Len(t, tc.resources.Resources, 1)
+			newConfig.Checks = map[string]config.Severity{}
+			newConfig.Checks[mutationStr] = config.SeverityDanger
 			newConfig.Mutations = []string{mutationStr}
 			results, err := validator.ApplyAllSchemaChecksToResourceProvider(&newConfig, tc.resources)
 			assert.NoError(t, err)
@@ -56,7 +60,7 @@ func TestMutations(t *testing.T) {
 				yamlContent, err := yaml.JSONToYAML(mutated.OriginalObjectJSON)
 				assert.NoError(t, err)
 				contentStr := mutation.UpdateMutatedContentWithComments(string(yamlContent), comments)
-				assert.EqualValues(t, mutatedYamlContent, contentStr)
+				assert.EqualValues(t, mutatedYamlContent, contentStr, "Mutation test case for " + tc.check + "/" + tc.filename + " failed")
 			}
 		}
 	}
diff --git a/test/schema_test.go b/test/schema_test.go
index 647ab317..3c0a1372 100644
--- a/test/schema_test.go
+++ b/test/schema_test.go
@@ -41,9 +41,10 @@ type testCase struct {
 }
 
 var mutatedYamlContentMap = map[string]string{}
-var failureTestCasesMap = map[string][]testCase{}
+var mutationTestCasesMap = map[string][]testCase{}
 
 func init() {
+	checkToTest := os.Getenv("POLARIS_CHECK_TEST")
 	_, baseDir, _, _ := runtime.Caller(0)
 	baseDir = filepath.Dir(baseDir) + "/checks"
 	dirs, err := ioutil.ReadDir(baseDir)
@@ -52,6 +53,9 @@ func init() {
 	}
 	for _, dir := range dirs {
 		check := dir.Name()
+		if checkToTest != "" && checkToTest != check {
+			continue
+		}
 		checkDir := baseDir + "/" + check
 		cases, err := ioutil.ReadDir(checkDir)
 		if err != nil {
@@ -76,10 +80,19 @@ func init() {
 			if tc.Name() == "check.yaml" {
 				continue
 			}
-			resources, err := kube.CreateResourceProviderFromPath(checkDir + "/" + tc.Name())
+			resourceFilename := strings.Replace(tc.Name(), "mutated", "failure", -1)
+
+			resources, err := kube.CreateResourceProviderFromPath(checkDir + "/" + resourceFilename)
 			if err != nil {
 				panic(err)
 			}
+			testcase := testCase{
+				filename:  tc.Name(),
+				check:     check,
+				resources: resources,
+				failure:   strings.Contains(resourceFilename, "failure"),
+				config:    c,
+			}
 
 			if strings.Contains(tc.Name(), "mutated") {
 				yamlContent, err := os.ReadFile(checkDir + "/" + tc.Name())
@@ -88,24 +101,14 @@ func init() {
 				}
 				key := fmt.Sprintf("%s/%s", check, tc.Name())
 				mutatedYamlContentMap[key] = string(yamlContent)
-			} else {
-				testcase := testCase{
-					filename:  tc.Name(),
-					check:     check,
-					resources: resources,
-					failure:   strings.Contains(tc.Name(), "failure"),
-					config:    c,
+				testCases, ok := mutationTestCasesMap[check]
+				if !ok {
+					testCases = []testCase{}
 				}
 				testCases = append(testCases, testcase)
-
-				if strings.Contains(tc.Name(), "mutated") {
-					testCases, ok := failureTestCasesMap[check]
-					if !ok {
-						testCases = []testCase{}
-					}
-					testCases = append(testCases, testcase)
-					failureTestCasesMap[check] = testCases
-				}
+				mutationTestCasesMap[check] = testCases
+			} else {
+				testCases = append(testCases, testcase)
 			}
 		}
 	}