From 8b20fd9dcf79454db2a26e965307fb82920c1e52 Mon Sep 17 00:00:00 2001 From: Robert Brennan Date: Fri, 20 Dec 2019 20:15:45 +0000 Subject: [PATCH] migrate health checks to schemas --- checks/livenessProbe.yaml | 23 +++++++++ checks/readinessProbe.yaml | 23 +++++++++ pkg/validator/container.go | 33 +++---------- pkg/validator/container_test.go | 16 ++++--- pkg/validator/pod.go | 6 ++- pkg/validator/schema.go | 82 ++++++++++++++++++++++++++++++++- 6 files changed, 148 insertions(+), 35 deletions(-) create mode 100644 checks/livenessProbe.yaml create mode 100644 checks/readinessProbe.yaml diff --git a/checks/livenessProbe.yaml b/checks/livenessProbe.yaml new file mode 100644 index 00000000..a60d719f --- /dev/null +++ b/checks/livenessProbe.yaml @@ -0,0 +1,23 @@ +name: LivenessProbeMissing +id: livenessProbeMissing +successMessage: Liveness probe is configured +failureMessage: Liveness probe should be configured +category: Health Checks +controllers: + exclude: + - Job + - CronJob +containers: + exclude: + - initContainer +target: Container +schema: + '$schema': http://json-schema.org/draft-07/schema + type: object + required: + - livenessProbe + properties: + livenessProbe: + type: object + not: + const: null diff --git a/checks/readinessProbe.yaml b/checks/readinessProbe.yaml new file mode 100644 index 00000000..c9d0dba8 --- /dev/null +++ b/checks/readinessProbe.yaml @@ -0,0 +1,23 @@ +name: ReadinessProbeMissing +id: readinessProbeMissing +successMessage: Readiness probe is configured +failureMessage: Readiness probe should be configured +category: Health Checks +controllers: + exclude: + - Job + - CronJob +containers: + exclude: + - initContainer +target: Container +schema: + '$schema': http://json-schema.org/draft-07/schema + type: object + required: + - readinessProbe + properties: + readinessProbe: + type: object + not: + const: null diff --git a/pkg/validator/container.go b/pkg/validator/container.go index 22117203..da3d5031 100644 --- a/pkg/validator/container.go +++ b/pkg/validator/container.go @@ -58,9 +58,13 @@ func ValidateContainer(container *corev1.Container, parentPodResult *PodResult, } cv.validateResources(conf, controllerName) - if !isInit && controllerType != config.Jobs && controllerType != config.CronJobs { - cv.validateHealthChecks(conf, controllerName) + + err := applyContainerSchemaChecks(conf, container, controllerName, controllerType, isInit, &cv) + // FIXME: don't panic + if err != nil { + panic(err) } + cv.validateImage(conf, controllerName) cv.validateNetworking(conf, controllerName) cv.validateSecurity(conf, controllerName) @@ -153,31 +157,6 @@ func (cv *ContainerValidation) validateResourceRange(id, resourceName string, ra } } -func (cv *ContainerValidation) validateHealthChecks(conf *config.Configuration, controllerName string) { - category := messages.CategoryHealthChecks - - name := "ReadinessProbeMissing" - // Don't validate readiness probes on init containers - if !cv.IsInitContainer && conf.IsActionable(conf.HealthChecks, name, controllerName) { - id := config.GetIDFromField(conf.HealthChecks, name) - if cv.Container.ReadinessProbe == nil { - cv.addFailure(messages.ReadinessProbeFailure, conf.HealthChecks.ReadinessProbeMissing, category, id) - } else { - cv.addSuccess(messages.ReadinessProbeSuccess, category, id) - } - } - - name = "LivenessProbeMissing" - if conf.IsActionable(conf.HealthChecks, name, controllerName) { - id := config.GetIDFromField(conf.HealthChecks, "LivenessProbeMissing") - if cv.Container.LivenessProbe == nil { - cv.addFailure(messages.LivenessProbeFailure, conf.HealthChecks.LivenessProbeMissing, category, id) - } else { - cv.addSuccess(messages.LivenessProbeSuccess, category, id) - } - } -} - func (cv *ContainerValidation) validateImage(conf *config.Configuration, controllerName string) { category := messages.CategoryImages diff --git a/pkg/validator/container_test.go b/pkg/validator/container_test.go index 7d5da0c8..2cf4f12e 100644 --- a/pkg/validator/container_test.go +++ b/pkg/validator/container_test.go @@ -409,17 +409,21 @@ func TestValidateHealthChecks(t *testing.T) { {name: "probes configured, but not required", probes: p2, cv: goodCV, errors: &f1}, } - for _, tt := range testCases { + for idx, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - tt.cv.validateHealthChecks(&conf.Configuration{HealthChecks: tt.probes}, "") + err := applyContainerSchemaChecks(&conf.Configuration{HealthChecks: tt.probes}, tt.cv.Container, "", conf.Deployments, tt.cv.IsInitContainer, &tt.cv) + if err != nil { + panic(err) + } + message := fmt.Sprintf("test case %d", idx) if tt.warnings != nil { - assert.Len(t, tt.cv.Warnings, len(*tt.warnings)) - assert.ElementsMatch(t, tt.cv.Warnings, *tt.warnings) + assert.Len(t, tt.cv.Warnings, len(*tt.warnings), message) + assert.ElementsMatch(t, tt.cv.Warnings, *tt.warnings, message) } - assert.Len(t, tt.cv.Errors, len(*tt.errors)) - assert.ElementsMatch(t, tt.cv.Errors, *tt.errors) + assert.Len(t, tt.cv.Errors, len(*tt.errors), message) + assert.ElementsMatch(t, tt.cv.Errors, *tt.errors, message) }) } } diff --git a/pkg/validator/pod.go b/pkg/validator/pod.go index d3b455e8..c9130a9c 100644 --- a/pkg/validator/pod.go +++ b/pkg/validator/pod.go @@ -32,7 +32,11 @@ func ValidatePod(conf config.Configuration, pod *corev1.PodSpec, controllerName ResourceValidation: &ResourceValidation{}, } - applyPodSchemaChecks(&conf, pod, controllerName, &pv) + err := applyPodSchemaChecks(&conf, pod, controllerName, controllerType, &pv) + // FIXME: don't panic + if err != nil { + panic(err) + } pRes := PodResult{ Messages: pv.messages(), diff --git a/pkg/validator/schema.go b/pkg/validator/schema.go index d9e8ea81..5bf167a3 100644 --- a/pkg/validator/schema.go +++ b/pkg/validator/schema.go @@ -34,6 +34,7 @@ type SchemaCheck struct { SuccessMessage string `yaml:"success_message"` FailureMessage string `yaml:"failure_message"` Controllers IncludeExcludeList `yaml:"controllers"` + Containers IncludeExcludeList `yaml:"containers"` Target Target `yaml:"target"` Schema jsonschema.RootSchema `yaml:"schema"` } @@ -44,10 +45,16 @@ var ( TargetContainer: []SchemaCheck{}, TargetPod: []SchemaCheck{}, } + // We explicitly set the order to avoid thrash in the + // tests as we migrate toward JSON schema checkOrder = []string{ + // Pod checks "hostIPC", "hostPID", "hostNetwork", + // Container checks + "readinessProbe", + "livenessProbe", } ) @@ -109,11 +116,62 @@ func (check SchemaCheck) checkPod(pod *corev1.PodSpec) (bool, error) { return len(errors) == 0, err } -func applyPodSchemaChecks(conf *config.Configuration, pod *corev1.PodSpec, controllerName string, pv *PodValidation) error { +func (check SchemaCheck) checkContainer(container *corev1.Container) (bool, error) { + bytes, err := json.Marshal(container) + if err != nil { + return false, err + } + errors, err := check.Schema.ValidateBytes(bytes) + return len(errors) == 0, err +} + +func (check SchemaCheck) isActionable(target Target, controllerType config.SupportedController, isInit bool) bool { + if check.Target != target { + return false + } + isIncluded := len(check.Controllers.Include) == 0 + for _, inclusion := range check.Controllers.Include { + if config.GetSupportedControllerFromString(inclusion) == controllerType { + isIncluded = true + break + } + } + if !isIncluded { + return false + } + for _, exclusion := range check.Controllers.Exclude { + if config.GetSupportedControllerFromString(exclusion) == controllerType { + return false + } + } + if check.Target == TargetContainer { + isIncluded := len(check.Containers.Include) == 0 + for _, inclusion := range check.Containers.Include { + if (inclusion == "initContainer" && isInit) || (inclusion == "container" && !isInit) { + isIncluded = true + break + } + } + if !isIncluded { + return false + } + for _, exclusion := range check.Containers.Exclude { + if (exclusion == "initContainer" && isInit) || (exclusion == "container" && !isInit) { + return false + } + } + } + return true +} + +func applyPodSchemaChecks(conf *config.Configuration, pod *corev1.PodSpec, controllerName string, controllerType config.SupportedController, pv *PodValidation) error { for _, check := range checks[TargetPod] { if !conf.IsActionable(check.Category, check.Name, controllerName) { continue } + if !check.isActionable(TargetPod, controllerType, false) { + continue + } severity := conf.GetSeverity(check.Category, check.Name) passes, err := check.checkPod(pod) if err != nil { @@ -127,3 +185,25 @@ func applyPodSchemaChecks(conf *config.Configuration, pod *corev1.PodSpec, contr } return nil } + +func applyContainerSchemaChecks(conf *config.Configuration, container *corev1.Container, controllerName string, controllerType config.SupportedController, isInit bool, cv *ContainerValidation) error { + for _, check := range checks[TargetContainer] { + if !conf.IsActionable(check.Category, check.Name, controllerName) { + continue + } + if !check.isActionable(TargetContainer, controllerType, isInit) { + continue + } + severity := conf.GetSeverity(check.Category, check.Name) + passes, err := check.checkContainer(container) + if err != nil { + return err + } + if passes { + cv.addSuccess(check.SuccessMessage, check.Category, check.ID) + } else { + cv.addFailure(check.FailureMessage, severity, check.Category, check.ID) + } + } + return nil +}