From c3bbbde631a00dc02868b3abe7652c9ce6285ff8 Mon Sep 17 00:00:00 2001 From: jessicagreben Date: Mon, 31 Dec 2018 11:21:46 -0800 Subject: [PATCH 1/7] add image and healthcheck config and checks --- deploy/all.yaml | 9 ++++----- pkg/config/config.go | 25 +++++++++++++++++++++++- pkg/config/config_test.go | 4 ++-- pkg/validator/container.go | 39 +++++++++++++++++++------------------- 4 files changed, 50 insertions(+), 27 deletions(-) diff --git a/deploy/all.yaml b/deploy/all.yaml index 248c881a..e7b2809c 100644 --- a/deploy/all.yaml +++ b/deploy/all.yaml @@ -81,16 +81,15 @@ data: whitelist: - '*.example.com' prevent_overlaps: true - health_checks: + healthchecks: readiness: require: true liveness: require: true images: - require_tag: true - repos: - whitelist: - - gcr.io + tagRequired: true + whitelistRepos: + - gcr.io namespaces: require_labels: true security_context: diff --git a/pkg/config/config.go b/pkg/config/config.go index 44629992..e6de771e 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -5,25 +5,32 @@ import ( "fmt" "io" "io/ioutil" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/yaml" ) +// ResourceMinMax sets a range for a min and max setting for a resource. type ResourceMinMax struct { Min *resource.Quantity Max *resource.Quantity } +// ResourceList does x. type ResourceList map[corev1.ResourceName]ResourceMinMax +// RequestsAndLimits contains config for resource requests and limits. type RequestsAndLimits struct { Requests ResourceList Limits ResourceList } +// Configuration contains all of the config for the validation checks. type Configuration struct { - Resources RequestsAndLimits + Resources RequestsAndLimits + Healthchecks Probes + Images Images } // ParseFile parses config from a file @@ -49,3 +56,19 @@ func Parse(rawBytes []byte) (Configuration, error) { } } } + +// Probes contains config for the readiness and liveness probes. +type Probes struct { + Readiness resourceRequire + Liveness resourceRequire +} + +type resourceRequire map[require]bool + +type require string + +// Images contains the config for images. +type Images struct { + TagRequired bool + WhitelistRepos []string +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index d35bf714..96e290e0 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -41,7 +41,7 @@ resources: max: 4G ` -var resourceConfJson1 = `{ +var resourceConfJSON1 = `{ "resources": { "requests": { "cpu": { @@ -89,7 +89,7 @@ func TestParseYaml(t *testing.T) { } func TestParseJson(t *testing.T) { - parsedConf, err := Parse([]byte(resourceConfJson1)) + parsedConf, err := Parse([]byte(resourceConfJSON1)) assert.NoError(t, err, "Expected no error when parsing config") requests := parsedConf.Resources.Requests diff --git a/pkg/validator/container.go b/pkg/validator/container.go index 305f4e35..259fafd9 100644 --- a/pkg/validator/container.go +++ b/pkg/validator/container.go @@ -15,6 +15,8 @@ package validator import ( + "strings" + conf "github.com/reactiveops/fairwinds/pkg/config" "github.com/reactiveops/fairwinds/pkg/types" corev1 "k8s.io/api/core/v1" @@ -33,8 +35,8 @@ func validateContainer(conf conf.Configuration, container corev1.Container) Cont } cv.validateResources(conf.Resources) - // cv.validateHealthChecks(conf.HealthChecks) - // cv.validateTags(conf.Image) + cv.validateHealthChecks(conf.Healthchecks) + cv.validateImage(conf.Images) return cv } @@ -65,24 +67,23 @@ func (cv *ContainerValidation) withinRange(resourceName string, expectedRange co } } -// func probes(conf conf.ResourceRequestsAndLimits, c corev1.Container, results types.ContainerResults) types.ContainerResults { -// if c.ReadinessProbe == nil { -// results.AddFailure("Readiness Probe", "placeholder", "placeholder") -// } +func (cv *ContainerValidation) validateHealthChecks(conf conf.Probes) { + if conf.Readiness["require"] && cv.Container.ReadinessProbe == nil { + cv.addFailure("readiness", "probe needs to be configured", "nil") + } + if conf.Liveness["require"] && cv.Container.LivenessProbe == nil { + cv.addFailure("liveness", "probe needs to be configured", "nil") + } +} -// if c.LivenessProbe == nil { -// results.AddFailure("Liveness Probe", "placeholder", "placeholder") -// } -// return results -// } - -// func tag(conf conf.ResourceRequestsAndLimits, c corev1.Container, results types.ContainerResults) types.ContainerResults { -// img := strings.Split(c.Image, ":") -// if len(img) == 1 || img[1] == "latest" { -// results.AddFailure("Image Tag", "not latest", "latest") -// } -// return results -// } +func (cv *ContainerValidation) validateImage(conf conf.Images) { + if conf.TagRequired { + img := strings.Split(cv.Container.Image, ":") + if len(img) == 1 || img[1] == "latest" { + cv.addFailure("Image Tag", "not latest", "latest") + } + } +} // func hostPort(conf conf.ResourceRequestsAndLimits, c corev1.Container, results types.ContainerResults) types.ContainerResults { // for _, port := range c.Ports { From fcf9b20ab68393b22531560f696532ed1d134f2d Mon Sep 17 00:00:00 2001 From: jessicagreben Date: Mon, 31 Dec 2018 11:34:04 -0800 Subject: [PATCH 2/7] fix comment --- pkg/config/config.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index e6de771e..6dc70dab 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -17,7 +17,7 @@ type ResourceMinMax struct { Max *resource.Quantity } -// ResourceList does x. +// ResourceList maps the resource name to a range on min and max values. type ResourceList map[corev1.ResourceName]ResourceMinMax // RequestsAndLimits contains config for resource requests and limits. @@ -33,7 +33,7 @@ type Configuration struct { Images Images } -// ParseFile parses config from a file +// ParseFile parses config from a file. func ParseFile(path string) (Configuration, error) { rawBytes, err := ioutil.ReadFile(path) if err != nil { @@ -42,7 +42,7 @@ func ParseFile(path string) (Configuration, error) { return Parse(rawBytes) } -// Parse parses config from a byte array +// Parse parses config from a byte array. func Parse(rawBytes []byte) (Configuration, error) { reader := bytes.NewReader(rawBytes) conf := Configuration{} From 6648cedef48731ac43f143d4807ce5a82a049757 Mon Sep 17 00:00:00 2001 From: jessicagreben Date: Mon, 31 Dec 2018 12:18:55 -0800 Subject: [PATCH 3/7] add go lint and go to circle ci tests --- .circleci/config.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index fb5c9786..11f4aec1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -17,6 +17,9 @@ jobs: steps: - checkout - run: dep ensure + - run: go get -u github.com/golang/lint/golint + - run: go list ./... | grep -v vendor | xargs golint + - run: go list ./... | grep -v vendor | xargs go vet - run: go test ./pkg/... -coverprofile cover.out build: From 21e7d2b0f8fd5e50511275f456b1db39096c39a4 Mon Sep 17 00:00:00 2001 From: jessicagreben Date: Thu, 3 Jan 2019 07:33:20 -0800 Subject: [PATCH 4/7] make PR comment changes --- deploy/all.yaml | 14 +------------- pkg/config/config.go | 13 +++++++------ pkg/config/config_test.go | 4 ++-- pkg/validator/container.go | 6 +++--- 4 files changed, 13 insertions(+), 24 deletions(-) diff --git a/deploy/all.yaml b/deploy/all.yaml index e7b2809c..14226e51 100644 --- a/deploy/all.yaml +++ b/deploy/all.yaml @@ -77,11 +77,7 @@ data: memory: min: 10m max: 2000M - ingresses: - whitelist: - - '*.example.com' - prevent_overlaps: true - healthchecks: + healthChecks: readiness: require: true liveness: @@ -90,14 +86,6 @@ data: tagRequired: true whitelistRepos: - gcr.io - namespaces: - require_labels: true - security_context: - capabilities: - whitelist: - - 'CAP_SYS_ADMIN' - prevent_privileged: true - read_only_file_system: true --- apiVersion: extensions/v1beta1 kind: Deployment diff --git a/pkg/config/config.go b/pkg/config/config.go index 6dc70dab..f8831ec3 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -29,7 +29,7 @@ type RequestsAndLimits struct { // Configuration contains all of the config for the validation checks. type Configuration struct { Resources RequestsAndLimits - Healthchecks Probes + HealthChecks Probes Images Images } @@ -59,13 +59,14 @@ func Parse(rawBytes []byte) (Configuration, error) { // Probes contains config for the readiness and liveness probes. type Probes struct { - Readiness resourceRequire - Liveness resourceRequire + Readiness ResourceRequire + Liveness ResourceRequire } -type resourceRequire map[require]bool - -type require string +// ResourceRequire indicates if this resource should be validated. +type ResourceRequire struct { + Require bool +} // Images contains the config for images. type Images struct { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 96e290e0..5aa7cc0e 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -23,7 +23,7 @@ import ( var resourceConfInvalid1 = `test` -var resourceConfYaml1 = `--- +var resourceConfYAML1 = `--- resources: requests: cpu: @@ -72,7 +72,7 @@ func TestParseError(t *testing.T) { } func TestParseYaml(t *testing.T) { - parsedConf, err := Parse([]byte(resourceConfYaml1)) + parsedConf, err := Parse([]byte(resourceConfYAML1)) assert.NoError(t, err, "Expected no error when parsing config") requests := parsedConf.Resources.Requests diff --git a/pkg/validator/container.go b/pkg/validator/container.go index 259fafd9..0838cb65 100644 --- a/pkg/validator/container.go +++ b/pkg/validator/container.go @@ -35,7 +35,7 @@ func validateContainer(conf conf.Configuration, container corev1.Container) Cont } cv.validateResources(conf.Resources) - cv.validateHealthChecks(conf.Healthchecks) + cv.validateHealthChecks(conf.HealthChecks) cv.validateImage(conf.Images) return cv @@ -68,10 +68,10 @@ func (cv *ContainerValidation) withinRange(resourceName string, expectedRange co } func (cv *ContainerValidation) validateHealthChecks(conf conf.Probes) { - if conf.Readiness["require"] && cv.Container.ReadinessProbe == nil { + if conf.Readiness.Require && cv.Container.ReadinessProbe == nil { cv.addFailure("readiness", "probe needs to be configured", "nil") } - if conf.Liveness["require"] && cv.Container.LivenessProbe == nil { + if conf.Liveness.Require && cv.Container.LivenessProbe == nil { cv.addFailure("liveness", "probe needs to be configured", "nil") } } From 0e4f8657ce075f524af8e2b05e812d72107e5ce7 Mon Sep 17 00:00:00 2001 From: jessicagreben Date: Thu, 3 Jan 2019 08:30:52 -0800 Subject: [PATCH 5/7] add tests --- pkg/validator/container_test.go | 83 +++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/pkg/validator/container_test.go b/pkg/validator/container_test.go index f30d85a4..ba01805c 100644 --- a/pkg/validator/container_test.go +++ b/pkg/validator/container_test.go @@ -165,3 +165,86 @@ func testValidateResources(t *testing.T, container *corev1.Container, resourceCo assert.Len(t, cv.Failures, len(*expectedFailures)) assert.ElementsMatch(t, cv.Failures, *expectedFailures) } + +func TestVaidateHealthChecks(t *testing.T) { + + // Test setup. + p1 := conf.Probes{} + p2 := conf.Probes{ + Readiness: conf.ResourceRequire{Require: false}, + Liveness: conf.ResourceRequire{Require: false}, + } + p3 := conf.Probes{ + Readiness: conf.ResourceRequire{Require: true}, + Liveness: conf.ResourceRequire{Require: true}, + } + + probe := corev1.Probe{} + cv1 := ContainerValidation{Container: corev1.Container{Name: ""}} + cv2 := ContainerValidation{Container: corev1.Container{Name: "", LivenessProbe: &probe, ReadinessProbe: &probe}} + + l := types.Failure{Name: "liveness", Expected: "probe needs to be configured", Actual: "nil"} + r := types.Failure{Name: "readiness", Expected: "probe needs to be configured", Actual: "nil"} + f1 := []types.Failure{} + f2 := []types.Failure{r, l} + + var testCases = []struct { + name string + probes conf.Probes + cv ContainerValidation + expected []types.Failure + }{ + {name: "probes not configured", probes: p1, cv: cv1, expected: f1}, + {name: "probes not required", probes: p2, cv: cv1, expected: f1}, + {name: "probes required & configured", probes: p3, cv: cv2, expected: f1}, + {name: "probes required & not configured", probes: p3, cv: cv1, expected: f2}, + {name: "probes configured, but not required", probes: p2, cv: cv2, expected: f1}, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + tt.cv.validateHealthChecks(tt.probes) + assert.Len(t, tt.cv.Failures, len(tt.expected)) + assert.ElementsMatch(t, tt.cv.Failures, tt.expected) + }) + } +} + +func TestVaidateImage(t *testing.T) { + + // Test setup. + i1 := conf.Images{} + i2 := conf.Images{TagRequired: false} + i3 := conf.Images{TagRequired: true} + + cv1 := ContainerValidation{Container: corev1.Container{Name: ""}} + cv2 := ContainerValidation{Container: corev1.Container{Name: "", Image: "test:tag"}} + cv3 := ContainerValidation{Container: corev1.Container{Name: "", Image: "test:latest"}} + cv4 := ContainerValidation{Container: corev1.Container{Name: "", Image: "test"}} + + f := types.Failure{Name: "Image Tag", Expected: "not latest", Actual: "latest"} + f1 := []types.Failure{} + f2 := []types.Failure{f} + + var testCases = []struct { + name string + image conf.Images + cv ContainerValidation + expected []types.Failure + }{ + {name: "image not configured", image: i1, cv: cv1, expected: f1}, + {name: "image not required ", image: i2, cv: cv1, expected: f1}, + {name: "image tag required and configured", image: i3, cv: cv2, expected: f1}, + {name: "image tag required, but not configured", image: i3, cv: cv1, expected: f2}, + {name: "image tag required, but is latest", image: i3, cv: cv3, expected: f2}, + {name: "image tag required, but is empty", image: i3, cv: cv4, expected: f2}, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + tt.cv.validateImage(tt.image) + assert.Len(t, tt.cv.Failures, len(tt.expected)) + assert.ElementsMatch(t, tt.cv.Failures, tt.expected) + }) + } +} From 135f33fe7cab0f8e5fd812ae0128b30cf3176589 Mon Sep 17 00:00:00 2001 From: jessicagreben Date: Thu, 3 Jan 2019 08:32:09 -0800 Subject: [PATCH 6/7] fix spelling err --- pkg/validator/container_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/validator/container_test.go b/pkg/validator/container_test.go index ba01805c..328292a5 100644 --- a/pkg/validator/container_test.go +++ b/pkg/validator/container_test.go @@ -166,7 +166,7 @@ func testValidateResources(t *testing.T, container *corev1.Container, resourceCo assert.ElementsMatch(t, cv.Failures, *expectedFailures) } -func TestVaidateHealthChecks(t *testing.T) { +func TestValidateHealthChecks(t *testing.T) { // Test setup. p1 := conf.Probes{} @@ -210,7 +210,7 @@ func TestVaidateHealthChecks(t *testing.T) { } } -func TestVaidateImage(t *testing.T) { +func TestValidateImage(t *testing.T) { // Test setup. i1 := conf.Images{} From 718c0901eb1e1e170c5a7b072df031c8d6bcb1d1 Mon Sep 17 00:00:00 2001 From: jessicagreben Date: Thu, 3 Jan 2019 09:20:41 -0800 Subject: [PATCH 7/7] remove unneeded white space --- pkg/validator/container_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/validator/container_test.go b/pkg/validator/container_test.go index 328292a5..b0f5bc48 100644 --- a/pkg/validator/container_test.go +++ b/pkg/validator/container_test.go @@ -233,7 +233,7 @@ func TestValidateImage(t *testing.T) { expected []types.Failure }{ {name: "image not configured", image: i1, cv: cv1, expected: f1}, - {name: "image not required ", image: i2, cv: cv1, expected: f1}, + {name: "image not required", image: i2, cv: cv1, expected: f1}, {name: "image tag required and configured", image: i3, cv: cv2, expected: f1}, {name: "image tag required, but not configured", image: i3, cv: cv1, expected: f2}, {name: "image tag required, but is latest", image: i3, cv: cv3, expected: f2},