From 321bfa8f1fc6e845fcb2f8a29bf85157f21ca036 Mon Sep 17 00:00:00 2001 From: Barnabas Makonda <6409210+makoscafee@users.noreply.github.com> Date: Fri, 8 Apr 2022 17:19:14 +0300 Subject: [PATCH] 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 --- checks/cpuLimitsMissing.yaml | 4 +++ checks/cpuRequestsMissing.yaml | 4 +++ checks/deploymentMissingReplicas.yaml | 4 +++ checks/hostIPCSet.yaml | 3 ++ checks/hostNetworkSet.yaml | 3 ++ checks/hostPIDSet.yaml | 3 ++ checks/hostPortSet.yaml | 2 +- checks/livenessProbeMissing.yaml | 4 +++ checks/memoryLimitsMissing.yaml | 7 ++++ checks/memoryRequestsMissing.yaml | 7 ++++ checks/priorityClassNotSet.yaml | 4 +++ checks/readinessProbeMissing.yaml | 4 +++ checks/runAsPrivileged.yaml | 6 +++- checks/runAsRootAllowed.yaml | 4 +++ pkg/validator/schema.go | 7 ++++ test/checks/cpuLimitsMissing/success.yaml | 2 +- test/checks/cpuRequestsMissing/success.yaml | 2 +- test/checks/livenessProbeMissing/failure.yaml | 2 +- .../checks/memoryRequestsMissing/success.yaml | 2 +- .../checks/readinessProbeMissing/failure.yaml | 2 +- test/mutation_test.go | 32 +++++++++++++------ test/schema_test.go | 16 ++++++++-- 22 files changed, 104 insertions(+), 20 deletions(-) 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) 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 } } }