From f71ca999c986edc7ecd49f08a2aa07408a6f5e36 Mon Sep 17 00:00:00 2001 From: Robert Brennan Date: Tue, 7 Jun 2022 09:37:25 -0400 Subject: [PATCH] Change `target: Pod` to `target: PodSpec` (#726) * change target pod to target pod spec * add checks * update docs * fix tests Co-authored-by: MAKOSCAFEE --- checks/hostIPCSet.yaml | 2 +- checks/hostNetworkSet.yaml | 2 +- checks/hostPIDSet.yaml | 2 +- checks/notReadOnlyRootFilesystem.yaml | 2 +- checks/priorityClassNotSet.yaml | 2 +- checks/privilegeEscalationAllowed.yaml | 2 +- checks/runAsPrivileged.yaml | 2 +- checks/runAsRootAllowed.yaml | 2 +- docs/customization/custom-checks.md | 4 ++-- pkg/config/schema.go | 10 +++++----- pkg/validator/schema.go | 22 +++++++++++++++++----- test/checks/nakedPod/check.yaml | 17 +++++++++++++++++ test/checks/nakedPod/failure.yaml | 13 +++++++++++++ test/checks/nakedPod/success.yaml | 13 +++++++++++++ test/checks/oneController/check.yaml | 17 +++++++++++++++++ test/checks/oneController/failure.yaml | 13 +++++++++++++ test/checks/oneController/success.yaml | 13 +++++++++++++ test/schema_test.go | 26 +++++++++++++++++++++----- 18 files changed, 139 insertions(+), 25 deletions(-) create mode 100644 test/checks/nakedPod/check.yaml create mode 100644 test/checks/nakedPod/failure.yaml create mode 100644 test/checks/nakedPod/success.yaml create mode 100644 test/checks/oneController/check.yaml create mode 100644 test/checks/oneController/failure.yaml create mode 100644 test/checks/oneController/success.yaml diff --git a/checks/hostIPCSet.yaml b/checks/hostIPCSet.yaml index 7a3607e1..88845c89 100644 --- a/checks/hostIPCSet.yaml +++ b/checks/hostIPCSet.yaml @@ -1,7 +1,7 @@ successMessage: Host IPC is not configured failureMessage: Host IPC should not be configured category: Security -target: Pod +target: PodSpec schema: '$schema': http://json-schema.org/draft-07/schema type: object diff --git a/checks/hostNetworkSet.yaml b/checks/hostNetworkSet.yaml index 29965475..53728aa1 100644 --- a/checks/hostNetworkSet.yaml +++ b/checks/hostNetworkSet.yaml @@ -1,7 +1,7 @@ successMessage: Host network is not configured failureMessage: Host network should not be configured category: Security -target: Pod +target: PodSpec schema: '$schema': http://json-schema.org/draft-07/schema type: object diff --git a/checks/hostPIDSet.yaml b/checks/hostPIDSet.yaml index 48336f37..fe1f5c7b 100644 --- a/checks/hostPIDSet.yaml +++ b/checks/hostPIDSet.yaml @@ -1,7 +1,7 @@ successMessage: Host PID is not configured failureMessage: Host PID should not be configured category: Security -target: Pod +target: PodSpec schema: '$schema': http://json-schema.org/draft-07/schema type: object diff --git a/checks/notReadOnlyRootFilesystem.yaml b/checks/notReadOnlyRootFilesystem.yaml index 54152900..d614089d 100644 --- a/checks/notReadOnlyRootFilesystem.yaml +++ b/checks/notReadOnlyRootFilesystem.yaml @@ -2,7 +2,7 @@ successMessage: Filesystem is read only failureMessage: Filesystem should be read only category: Security target: Container -schemaTarget: Pod +schemaTarget: PodSpec schema: '$schema': http://json-schema.org/draft-07/schema definitions: diff --git a/checks/priorityClassNotSet.yaml b/checks/priorityClassNotSet.yaml index 49690027..dc9ee056 100644 --- a/checks/priorityClassNotSet.yaml +++ b/checks/priorityClassNotSet.yaml @@ -1,7 +1,7 @@ successMessage: Priority class has been set failureMessage: Priority class should be set category: Security -target: Pod +target: PodSpec schema: '$schema': http://json-schema.org/draft-07/schema type: object diff --git a/checks/privilegeEscalationAllowed.yaml b/checks/privilegeEscalationAllowed.yaml index 8d2933d9..396f93c8 100644 --- a/checks/privilegeEscalationAllowed.yaml +++ b/checks/privilegeEscalationAllowed.yaml @@ -2,7 +2,7 @@ successMessage: Privilege escalation not allowed failureMessage: Privilege escalation should not be allowed category: Security target: Container -schemaTarget: Pod +schemaTarget: PodSpec schema: '$schema': http://json-schema.org/draft-07/schema definitions: diff --git a/checks/runAsPrivileged.yaml b/checks/runAsPrivileged.yaml index d641c752..fc4e59e6 100644 --- a/checks/runAsPrivileged.yaml +++ b/checks/runAsPrivileged.yaml @@ -2,7 +2,7 @@ successMessage: Not running as privileged failureMessage: Should not be running as privileged category: Security target: Container -schemaTarget: Pod +schemaTarget: PodSpec schema: '$schema': http://json-schema.org/draft-07/schema definitions: diff --git a/checks/runAsRootAllowed.yaml b/checks/runAsRootAllowed.yaml index a56fe02c..7039820f 100644 --- a/checks/runAsRootAllowed.yaml +++ b/checks/runAsRootAllowed.yaml @@ -2,7 +2,7 @@ successMessage: Is not allowed to run as root failureMessage: Should not be allowed to run as root category: Security target: Container -schemaTarget: Pod +schemaTarget: PodSpec schema: '$schema': http://json-schema.org/draft-07/schema definitions: diff --git a/docs/customization/custom-checks.md b/docs/customization/custom-checks.md index eac66d85..45b3742a 100644 --- a/docs/customization/custom-checks.md +++ b/docs/customization/custom-checks.md @@ -43,8 +43,8 @@ check ID. Note that you'll also have to set its severity in the `checks` section * `category` - one of `Security`, `Efficiency`, or `Reliability` * `target` - specifies the type of resource to check. This can be: * a group and kind, e.g. `apps/Deployment` or `networking.k8s.io/Ingress` - * `Controller`, to check _any_ resource that contains a pod spec (e.g. Deployments, CronJobs, StatefulSets), as well as naked Pods - * `Pod`, same as `Controller`, but the schema applies to the Pod spec rather than the top-level controller + * `Controller`, to check _any_ resource that creates Pods (e.g. Deployments, CronJobs, StatefulSets), as well as naked Pods + * `PodSpec`, same as `Controller`, but the schema applies to the Pod spec rather than the top-level controller * `Container` same as `Controller`, but the schema applies to all Container specs rather than the top-level controller * `controllers` - if `target` is `Controller`, `Pod` or `Container`, you can use this to change which types of controllers are checked * `controllers.include` - _only_ check these controllers diff --git a/pkg/config/schema.go b/pkg/config/schema.go index b80887d5..c9623dfb 100644 --- a/pkg/config/schema.go +++ b/pkg/config/schema.go @@ -39,15 +39,15 @@ const ( TargetController TargetKind = "Controller" // TargetContainer points to the container spec TargetContainer TargetKind = "Container" - // TargetPod points to the pod spec - TargetPod TargetKind = "Pod" + // TargetPodSpec points to the pod spec + TargetPodSpec TargetKind = "PodSpec" ) // HandledTargets is a list of target names that are explicitly handled var HandledTargets = []TargetKind{ TargetController, TargetContainer, - TargetPod, + TargetPodSpec, } // MutationComment is the comments added to a mutated file @@ -253,8 +253,8 @@ func (check SchemaCheck) TemplateForResource(res interface{}) (*SchemaCheck, err return &newCheck, err } -// CheckPod checks a pod spec against the schema -func (check SchemaCheck) CheckPod(pod *corev1.PodSpec) (bool, []jsonschema.ValError, error) { +// CheckPodSpec checks a pod spec against the schema +func (check SchemaCheck) CheckPodSpec(pod *corev1.PodSpec) (bool, []jsonschema.ValError, error) { return check.CheckObject(pod) } diff --git a/pkg/validator/schema.go b/pkg/validator/schema.go index 1056101b..258819ac 100644 --- a/pkg/validator/schema.go +++ b/pkg/validator/schema.go @@ -15,6 +15,7 @@ package validator import ( + "errors" "fmt" "sort" "strconv" @@ -165,6 +166,17 @@ func applyControllerSchemaChecks(conf *config.Configuration, resourceProvider *k } finalResult.Results = resultSet + nonControllerResults, err := applyTopLevelSchemaChecks(conf, resourceProvider, resource, false) + if err != nil { + return finalResult, err + } + for key, val := range nonControllerResults { + if _, ok := finalResult.Results[key]; ok { + return finalResult, errors.New("Duplicate finding for check " + key) + } + finalResult.Results[key] = val + } + podRS, err := applyPodSchemaChecks(conf, resourceProvider, resource) if err != nil { return finalResult, err @@ -214,7 +226,7 @@ func applyTopLevelSchemaChecks(conf *config.Configuration, resources *kube.Resou func applyPodSchemaChecks(conf *config.Configuration, resources *kube.ResourceProvider, controller kube.GenericResource) (ResultSet, error) { test := schemaTestCase{ - Target: config.TargetPod, + Target: config.TargetPodSpec, ResourceProvider: resources, Resource: controller, } @@ -258,7 +270,7 @@ func applySchemaCheck(conf *config.Configuration, checkID string, test schemaTes var issues []jsonschema.ValError var prefix string if check.SchemaTarget != "" { - if check.SchemaTarget == config.TargetPod && check.Target == config.TargetContainer { + if check.SchemaTarget == config.TargetPodSpec && check.Target == config.TargetContainer { podCopy := *test.Resource.PodSpec podCopy.InitContainers = []corev1.Container{} podCopy.Containers = []corev1.Container{*test.Container} @@ -269,12 +281,12 @@ func applySchemaCheck(conf *config.Configuration, checkID string, test schemaTes if prefix != "" { prefix += "/containers/" + strconv.Itoa(containerIndex) } - passes, issues, err = check.CheckPod(&podCopy) + passes, issues, err = check.CheckPodSpec(&podCopy) } else { return nil, fmt.Errorf("Unknown combination of target (%s) and schema target (%s)", check.Target, check.SchemaTarget) } - } else if check.Target == config.TargetPod { - passes, issues, err = check.CheckPod(test.Resource.PodSpec) + } else if check.Target == config.TargetPodSpec { + passes, issues, err = check.CheckPodSpec(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 { diff --git a/test/checks/nakedPod/check.yaml b/test/checks/nakedPod/check.yaml new file mode 100644 index 00000000..066889d6 --- /dev/null +++ b/test/checks/nakedPod/check.yaml @@ -0,0 +1,17 @@ +successMessage: example FP success +failureMessage: example FP fail +target: Pod +schema: + '$schema': http://json-schema.org/draft-07/schema + type: object + properties: + metadata: + type: object + required: ["labels"] + properties: + labels: + type: object + required: ["app.kubernetes.io/name"] + properties: + app.kubernetes.io/name: + const: "{{ .metadata.name }}" diff --git a/test/checks/nakedPod/failure.yaml b/test/checks/nakedPod/failure.yaml new file mode 100644 index 00000000..b89334be --- /dev/null +++ b/test/checks/nakedPod/failure.yaml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: Pod +metadata: + name: nginx + labels: + app.kubernetes.io/name: nginxIDONTMATCH +spec: + containers: + - name: nginx + image: nginx + resources: + limits: + memory: "512Mi" diff --git a/test/checks/nakedPod/success.yaml b/test/checks/nakedPod/success.yaml new file mode 100644 index 00000000..63ed1f65 --- /dev/null +++ b/test/checks/nakedPod/success.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: + memory: "512Mi" diff --git a/test/checks/oneController/check.yaml b/test/checks/oneController/check.yaml new file mode 100644 index 00000000..6f964ce3 --- /dev/null +++ b/test/checks/oneController/check.yaml @@ -0,0 +1,17 @@ +successMessage: example FP success +failureMessage: example FP fail +target: StatefulSet +schema: + '$schema': http://json-schema.org/draft-07/schema + type: object + properties: + metadata: + type: object + required: ["labels"] + properties: + labels: + type: object + required: ["app.kubernetes.io/name"] + properties: + app.kubernetes.io/name: + const: "{{ .metadata.name }}" diff --git a/test/checks/oneController/failure.yaml b/test/checks/oneController/failure.yaml new file mode 100644 index 00000000..e5399f07 --- /dev/null +++ b/test/checks/oneController/failure.yaml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: StatefulSet +metadata: + name: nginx + labels: + app.kubernetes.io/name: nginxIDONTMATCH +spec: + containers: + - name: nginx + image: nginx + resources: + limits: + memory: "512Mi" diff --git a/test/checks/oneController/success.yaml b/test/checks/oneController/success.yaml new file mode 100644 index 00000000..693db747 --- /dev/null +++ b/test/checks/oneController/success.yaml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: StatefulSet +metadata: + name: nginx + labels: + app.kubernetes.io/name: nginx +spec: + containers: + - name: nginx + image: nginx + resources: + limits: + memory: "512Mi" diff --git a/test/schema_test.go b/test/schema_test.go index 2aca7cae..647ab317 100644 --- a/test/schema_test.go +++ b/test/schema_test.go @@ -37,6 +37,7 @@ type testCase struct { filename string resources *kube.ResourceProvider failure bool + config config.Configuration } var mutatedYamlContentMap = map[string]string{} @@ -56,7 +57,25 @@ func init() { if err != nil { panic(err) } + configString := "checks:\n " + check + ": danger" + checkPath := checkDir + "/check.yaml" + customCheckContent, err := ioutil.ReadFile(checkPath) + if err == nil { + lines := strings.Split(string(customCheckContent), "\n") + for idx := range lines { + lines[idx] = " " + lines[idx] + } + configString += "\ncustomChecks:\n " + check + ":\n" + configString += strings.Join(lines, "\n") + } + c, err := config.Parse([]byte(configString)) + if err != nil { + panic(err) + } for _, tc := range cases { + if tc.Name() == "check.yaml" { + continue + } resources, err := kube.CreateResourceProviderFromPath(checkDir + "/" + tc.Name()) if err != nil { panic(err) @@ -75,6 +94,7 @@ func init() { check: check, resources: resources, failure: strings.Contains(tc.Name(), "failure"), + config: c, } testCases = append(testCases, testcase) @@ -93,11 +113,7 @@ func init() { func TestChecks(t *testing.T) { for _, tc := range testCases { - c, err := config.Parse([]byte("checks:\n " + tc.check + ": danger")) - if err != nil { - panic(err) - } - results, err := validator.ApplyAllSchemaChecksToResourceProvider(&c, tc.resources) + results, err := validator.ApplyAllSchemaChecksToResourceProvider(&tc.config, tc.resources) if err != nil { panic(err) }