Repository: FairwindsOps/polaris
Pull request #712 (2022-02-24 14:52:10)

Add mutation field to `imagePolicyNotAlways` 
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?
The goal of the PR is to add a mutation field for one check and try to update the `YAML` based on the mutation definition.
### What changes did you make?

- [x] Add a mutation  field for pullPolicyNotAlways
- [x] Add a test to the [checks tests](https://github.com/FairwindsOps/polaris/tree/master/test/checks) to verify that the mutation works
  - [x] Apply the mutation to failure.yaml
  - [x] Make sure the result matches success.yaml

### What alternative solution should we consider, if any?



rbren: Looking good Barnie! Mostly nits

Patch:
diff --git a/checks/pullPolicyNotAlways.yaml b/checks/pullPolicyNotAlways.yaml
index 25a50aac..2921c7a1 100644
--- a/checks/pullPolicyNotAlways.yaml
+++ b/checks/pullPolicyNotAlways.yaml
@@ -9,3 +9,7 @@ schema:
   properties:
     imagePullPolicy:
       const: Always
+mutations:
+  - op: add
+    path: /imagePullPolicy
+    value: Always
\ No newline at end of file
diff --git a/cmd/polaris/audit.go b/cmd/polaris/audit.go
index 3ffb5e68..eeeb9b52 100644
--- a/cmd/polaris/audit.go
+++ b/cmd/polaris/audit.go
@@ -41,6 +41,7 @@ var resourceToAudit string
 var useColor bool
 var helmChart string
 var helmValues string
+var checks string
 
 func init() {
 	rootCmd.AddCommand(auditCmd)
@@ -56,6 +57,7 @@ func init() {
 	auditCmd.PersistentFlags().StringVar(&resourceToAudit, "resource", "", "Audit a specific resource, in the format namespace/kind/version/name, e.g. nginx-ingress/Deployment.apps/v1/default-backend.")
 	auditCmd.PersistentFlags().StringVar(&helmChart, "helm-chart", "", "Will fill out Helm template")
 	auditCmd.PersistentFlags().StringVar(&helmValues, "helm-values", "", "Optional flag to add helm values")
+	auditCmd.PersistentFlags().StringVar(&checks, "checks", "", "Optional flag to specify specific checks to check")
 }
 
 var auditCmd = &cobra.Command{
diff --git a/cmd/polaris/root.go b/cmd/polaris/root.go
index dc1b3afc..77da760e 100644
--- a/cmd/polaris/root.go
+++ b/cmd/polaris/root.go
@@ -25,7 +25,7 @@ import (
 )
 
 var configPath string
-var disallowExemptions, disallowConfigExemptions, disallowAnnotationExemptions bool
+var disallowExemptions, disallowConfigExemptions, disallowAnnotationExemptions, fixChecks bool
 var logLevel string
 var auditPath string
 var displayName string
diff --git a/examples/config-full.yaml b/examples/config-full.yaml
index e050d0a6..0578b71c 100644
--- a/examples/config-full.yaml
+++ b/examples/config-full.yaml
@@ -26,6 +26,7 @@ checks:
   resourceLimits: warning
   imageRegistry: danger
 
+  
 exemptions:
   - controllerNames:
     - my-network-controller
diff --git a/examples/config.yaml b/examples/config.yaml
index 9b5a3af3..a06a2dac 100644
--- a/examples/config.yaml
+++ b/examples/config.yaml
@@ -28,6 +28,7 @@ checks:
   hostPortSet: warning
   tlsSettingsMissing: warning
 
+
 exemptions:
   - namespace: kube-system
     controllerNames:
diff --git a/go.mod b/go.mod
index 3739bdb7..e588aa02 100644
--- a/go.mod
+++ b/go.mod
@@ -3,6 +3,8 @@ module github.com/fairwindsops/polaris
 go 1.15
 
 require (
+	github.com/evanphx/json-patch v5.6.0+incompatible // indirect
+	github.com/evanphx/json-patch/v5 v5.6.0
 	github.com/fatih/color v1.13.0
 	github.com/gobuffalo/packr/v2 v2.8.1
 	github.com/google/gofuzz v1.2.0 // indirect
diff --git a/go.sum b/go.sum
index f3550c92..ced06543 100644
--- a/go.sum
+++ b/go.sum
@@ -158,8 +158,11 @@ github.com/envoyproxy/go-control-plane v0.10.1/go.mod h1:AY7fTTXNdv/aJ2O5jwpxAPO
 github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
 github.com/envoyproxy/protoc-gen-validate v0.6.2/go.mod h1:2t7qjJNvHPx8IjnBOzl9E9/baC+qXE/TeeyBRzgJDws=
 github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ=
-github.com/evanphx/json-patch v4.11.0+incompatible h1:glyUF9yIYtMHzn8xaKw5rMhdWcwsYV8dZHIq5567/xs=
 github.com/evanphx/json-patch v4.11.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
+github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U=
+github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
+github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww=
+github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4=
 github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
 github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=
 github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 1988c791..aee3dd0f 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -36,6 +36,7 @@ type Configuration struct {
 	DisallowExemptions           bool                   `json:"disallowExemptions"`
 	DisallowConfigExemptions     bool                   `json:"disallowConfigExemptions"`
 	DisallowAnnotationExemptions bool                   `json:"disallowAnnotationExemptions"`
+	Mutations                    []string               `json:"mutations"`
 	KubeContext                  string                 `json:"kubeContext"`
 }
 
diff --git a/pkg/config/schema.go b/pkg/config/schema.go
index 22e3d4c6..d6d3b520 100644
--- a/pkg/config/schema.go
+++ b/pkg/config/schema.go
@@ -51,6 +51,7 @@ type SchemaCheck struct {
 	AdditionalSchemas       map[string]map[string]interface{} `yaml:"additionalSchemas" json:"additionalSchemas"`
 	AdditionalSchemaStrings map[string]string                 `yaml:"additionalSchemaStrings" json:"additionalSchemaStrings"`
 	AdditionalValidators    map[string]jsonschema.RootSchema  `yaml:"-" json:"-"`
+	Mutations               []map[string]interface{}          `yaml:"mutations" json:"mutations"`
 }
 
 type resourceMinimum string
diff --git a/pkg/mutation/mutate.go b/pkg/mutation/mutate.go
new file mode 100644
index 00000000..7668ef71
--- /dev/null
+++ b/pkg/mutation/mutate.go
@@ -0,0 +1,78 @@
+package mutation
+
+import (
+	"encoding/json"
+	"fmt"
+
+	jsonpatch "github.com/evanphx/json-patch/v5"
+	"github.com/fairwindsops/polaris/pkg/config"
+	"github.com/fairwindsops/polaris/pkg/kube"
+	"github.com/fairwindsops/polaris/pkg/validator"
+)
+
+// ApplyAllSchemaMutations applies available mutation to a single resource
+func ApplyAllSchemaMutations(conf *config.Configuration, resourceProvider *kube.ResourceProvider, resource kube.GenericResource, mutations []map[string]interface{}) (kube.GenericResource, error) {
+	resByte := resource.OriginalObjectJSON
+	var jsonByte []byte
+	mutationByte, err := json.Marshal(mutations)
+	if err != nil {
+		return resource, err
+	}
+
+	patch, err := jsonpatch.DecodePatch(mutationByte)
+	if err != nil {
+		return resource, err
+	}
+	jsonByte, err = patch.Apply(resByte)
+	if err != nil {
+		return resource, err
+	}
+	mutated, err := kube.NewGenericResourceFromBytes(jsonByte)
+	if err != nil {
+		return resource, err
+	}
+
+	return mutated, nil
+}
+
+// GetMutationsFromResults returns all mutations from results
+func GetMutationsFromResults(conf *config.Configuration, results []validator.Result) map[string][]map[string]interface{} {
+	allMutationsFromResults := make(map[string][]map[string]interface{})
+	for _, result := range results {
+		key := fmt.Sprintf("%s/%s/%s", result.Kind, result.Name, result.Namespace)
+
+		for _, resultMessage := range result.Results {
+			if len(resultMessage.Mutations) > 0 {
+				mutations, ok := allMutationsFromResults[key]
+				if !ok {
+					mutations = make([]map[string]interface{}, 0)
+				}
+				allMutationsFromResults[key] = append(mutations, resultMessage.Mutations...)
+			}
+		}
+
+		for _, resultMessage := range result.PodResult.Results {
+			if len(resultMessage.Mutations) > 0 {
+				mutations, ok := allMutationsFromResults[key]
+				if !ok {
+					mutations = make([]map[string]interface{}, 0)
+				}
+				allMutationsFromResults[key] = append(mutations, resultMessage.Mutations...)
+			}
+		}
+
+		for _, containerResult := range result.PodResult.ContainerResults {
+			for _, resultMessage := range containerResult.Results {
+				if len(resultMessage.Mutations) > 0 {
+					mutations, ok := allMutationsFromResults[key]
+					if !ok {
+						mutations = make([]map[string]interface{}, 0)
+					}
+					allMutationsFromResults[key] = append(mutations, resultMessage.Mutations...)
+				}
+			}
+		}
+
+	}
+	return allMutationsFromResults
+}
diff --git a/pkg/validator/output.go b/pkg/validator/output.go
index bb0365de..94b476c8 100644
--- a/pkg/validator/output.go
+++ b/pkg/validator/output.go
@@ -72,12 +72,13 @@ type ClusterInfo struct {
 
 // ResultMessage is the result of a given check
 type ResultMessage struct {
-	ID       string
-	Message  string
-	Details  []string
-	Success  bool
-	Severity config.Severity
-	Category string
+	ID        string
+	Message   string
+	Details   []string
+	Success   bool
+	Severity  config.Severity
+	Category  string
+	Mutations []map[string]interface{}
 }
 
 // ResultSet contiains the results for a set of checks
diff --git a/pkg/validator/schema.go b/pkg/validator/schema.go
index 9beec693..fb06e568 100644
--- a/pkg/validator/schema.go
+++ b/pkg/validator/schema.go
@@ -3,9 +3,11 @@ package validator
 import (
 	"fmt"
 	"sort"
+	"strconv"
 	"strings"
 
 	"github.com/qri-io/jsonschema"
+	"github.com/sirupsen/logrus"
 	"github.com/thoas/go-funk"
 	corev1 "k8s.io/api/core/v1"
 	metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -239,6 +241,7 @@ func applySchemaCheck(conf *config.Configuration, checkID string, test schemaTes
 	}
 	var passes bool
 	var issues []jsonschema.ValError
+	var prefix string
 	if check.SchemaTarget != "" {
 		if check.SchemaTarget == config.TargetPod && check.Target == config.TargetContainer {
 			podCopy := *test.Resource.PodSpec
@@ -250,7 +253,15 @@ func applySchemaCheck(conf *config.Configuration, checkID string, test schemaTes
 		}
 	} else if check.Target == config.TargetPod {
 		passes, issues, err = check.CheckPod(test.Resource.PodSpec)
+		prefix = getJSONSchemaPrefix(test.Resource.Kind)
 	} else if check.Target == config.TargetContainer {
+		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.CheckContainer(test.Container)
 	} else {
 		passes, issues, err = check.CheckObject(test.Resource.Resource.Object)
@@ -279,6 +290,16 @@ func applySchemaCheck(conf *config.Configuration, checkID string, test schemaTes
 		}
 	}
 	result := makeResult(conf, check, passes, issues)
+	if !passes {
+		if funk.Contains(conf.Mutations, checkID) {
+			mutations := funk.Map(check.Mutations, func(mutation map[string]interface{}) map[string]interface{} {
+				mutationCopy := deepCopyMutation(mutation)
+				mutationCopy["path"] = prefix + mutationCopy["path"].(string)
+				return mutationCopy
+			}).([]map[string]interface{})
+			result.Mutations = mutations
+		}
+	}
 	return &result, nil
 }
 
@@ -290,3 +311,25 @@ func getSortedKeys(m map[string]config.Severity) []string {
 	sort.Strings(keys)
 	return keys
 }
+
+func deepCopyMutation(source map[string]interface{}) map[string]interface{} {
+	destination := map[string]interface{}{}
+	for key, value := range source {
+		destination[key] = value
+	}
+	return destination
+}
+
+func getJSONSchemaPrefix(kind string) (prefix string) {
+	if kind == "CronJob" {
+		prefix = "/spec/jobTemplate/spec/template/spec"
+	} else if kind == "Pod" {
+		prefix = "/spec"
+	} else if (kind == "Deployment") || (kind == "Daemonset") ||
+		(kind == "Statefulset") || (kind == "Job") || (kind == "ReplicationController") {
+		prefix = "/spec/template/spec"
+	} else {
+		logrus.Warningf("Mutation for this this resource (%s) is not supported", kind)
+	}
+	return prefix
+}


Test patch:
diff --git a/test/checks/pullPolicyNotAlways/failure.cron-job.yaml b/test/checks/pullPolicyNotAlways/failure.cron-job.yaml
new file mode 100644
index 00000000..0dfc8700
--- /dev/null
+++ b/test/checks/pullPolicyNotAlways/failure.cron-job.yaml
@@ -0,0 +1,18 @@
+apiVersion: batch/v1
+kind: CronJob
+metadata:
+  name: hello
+spec:
+  schedule: "* * * * *"
+  jobTemplate:
+    spec:
+      template:
+        spec:
+          containers:
+          - name: hello
+            image: busybox
+            command:
+            - /bin/sh
+            - -c
+            - date; echo Hello from the Kubernetes cluster
+          restartPolicy: OnFailure
\ No newline at end of file
diff --git a/test/checks/pullPolicyNotAlways/failure.deployment.yaml b/test/checks/pullPolicyNotAlways/failure.deployment.yaml
new file mode 100644
index 00000000..3649247c
--- /dev/null
+++ b/test/checks/pullPolicyNotAlways/failure.deployment.yaml
@@ -0,0 +1,21 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+  labels:
+    app: nginx
+spec:
+  replicas: 3
+  selector:
+    matchLabels:
+      app: nginx
+  template:
+    metadata:
+      labels:
+        app: nginx
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:1.14.2
+        ports:
+        - containerPort: 80
\ No newline at end of file
diff --git a/test/checks/pullPolicyNotAlways/success.cron-job.yaml b/test/checks/pullPolicyNotAlways/success.cron-job.yaml
new file mode 100644
index 00000000..e9c291c6
--- /dev/null
+++ b/test/checks/pullPolicyNotAlways/success.cron-job.yaml
@@ -0,0 +1,19 @@
+apiVersion: batch/v1
+kind: CronJob
+metadata:
+  name: hello
+spec:
+  schedule: "* * * * *"
+  jobTemplate:
+    spec:
+      template:
+        spec:
+          containers:
+          - name: hello
+            image: busybox
+            imagePullPolicy: Always
+            command:
+            - /bin/sh
+            - -c
+            - date; echo Hello from the Kubernetes cluster
+          restartPolicy: OnFailure
\ No newline at end of file
diff --git a/test/checks/pullPolicyNotAlways/success.deployment.yaml b/test/checks/pullPolicyNotAlways/success.deployment.yaml
new file mode 100644
index 00000000..26a08dd9
--- /dev/null
+++ b/test/checks/pullPolicyNotAlways/success.deployment.yaml
@@ -0,0 +1,22 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+  labels:
+    app: nginx
+spec:
+  replicas: 3
+  selector:
+    matchLabels:
+      app: nginx
+  template:
+    metadata:
+      labels:
+        app: nginx
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:1.14.2
+        imagePullPolicy: Always
+        ports:
+        - containerPort: 80
\ No newline at end of file
diff --git a/test/mutation_test.go b/test/mutation_test.go
new file mode 100644
index 00000000..6f954144
--- /dev/null
+++ b/test/mutation_test.go
@@ -0,0 +1,50 @@
+package test
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/fairwindsops/polaris/pkg/config"
+	"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
+`
+
+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) {
+			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)
+			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 {
+				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)
+			}
+		}
+	}
+}
diff --git a/test/schema_test.go b/test/schema_test.go
index 0dd3f070..b9216e8e 100644
--- a/test/schema_test.go
+++ b/test/schema_test.go
@@ -24,6 +24,8 @@ type testCase struct {
 	failure   bool
 }
 
+var successResourceMap = map[string]*kube.ResourceProvider{}
+
 func init() {
 	_, baseDir, _, _ := runtime.Caller(0)
 	baseDir = filepath.Dir(baseDir) + "/checks"
@@ -49,6 +51,10 @@ func init() {
 				resources: resources,
 				failure:   strings.Contains(tc.Name(), "failure"),
 			})
+			if !strings.Contains(tc.Name(), "failure") {
+				key := fmt.Sprintf("%s/%s", check, tc.Name())
+				successResourceMap[key] = resources
+			}
 		}
 	}
 }