Merge pull request #6 from reactiveops/rs/better-resource-checks

Improving resource validation, lots of restructuring to add ContainerValidation type

.dockerignore

@@ -1,5 +1,6 @@
 deploy
 .gitignore
+.git/*
 Dockerfile
 LICENSE
 Tiltfile

.gitignore

@@ -12,3 +12,4 @@
 *.out
 
 Tiltfile
+main

deploy/all.yaml

@@ -1,19 +1,5 @@
 ---
 apiVersion: v1
-kind: ConfigMap
-metadata:
-  name: fairwinds-config
-data:
-  config.yml: |
-    resources:
-      cpu:
-        min: 100m
-        max: 500m
-      memory:
-        min: 100M
-        max: 200M
----
-apiVersion: v1
 kind: Namespace
 metadata:
   name: fairwinds

pkg/config/config.go

@@ -11,11 +11,11 @@ type ResourceMinMax struct {
 
 type ResourceList map[corev1.ResourceName]ResourceMinMax
 
-type ResourceRequestsAndLimits struct {
+type RequestsAndLimits struct {
 	Requests ResourceList
 	Limits   ResourceList
 }
 
 type Configuration struct {
-	Resources ResourceRequestsAndLimits
+	Resources RequestsAndLimits
 }

pkg/types/types.go

@@ -11,15 +11,6 @@ type Failure struct {
 	Actual   string
 }
 
-// NewFailure is a factory function for a Failure.
-func NewFailure(name, expected, actual string) *Failure {
-	return &Failure{
-		Name:     name,
-		Expected: expected,
-		Actual:   actual,
-	}
-}
-
 // Reason returns a string that describes the reason for a Failure.
 func (f *Failure) Reason() string {
 	return fmt.Sprintf("- %s: Expected: %s, Actual: %s.\n",
@@ -28,15 +19,3 @@ func (f *Failure) Reason() string {
 		f.Actual,
 	)
 }
-
-// ContainerResults has the results of the validation checks for containers.
-type ContainerResults struct {
-	Name     string
-	Failures []Failure
-}
-
-// AddFailure creates a new Failure and adds it to ContainerResults.
-func (c *ContainerResults) AddFailure(name, expected, actual string) {
-	f := NewFailure(name, expected, actual)
-	c.Failures = append(c.Failures, *f)
-}

pkg/validator/container.go

@@ -15,76 +15,88 @@
 package validator
 
 import (
-	"strings"
+	"fmt"
 
 	conf "github.com/reactiveops/fairwinds/pkg/config"
 	"github.com/reactiveops/fairwinds/pkg/types"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 )
 
-func validateContainer(conf conf.Configuration, container corev1.Container) types.ContainerResults {
-	results := types.ContainerResults{
-		Name: container.Name,
-	}
-	results = resources(conf.Resources, container, results)
-	results = probes(conf.Resources, container, results)
-	results = tag(conf.Resources, container, results)
-
-	return results
+// ContainerValidation tracks validation failures associated with a Container
+type ContainerValidation struct {
+	Container corev1.Container
+	Failures  []types.Failure
 }
 
-func resources(conf conf.ResourceRequestsAndLimits, c corev1.Container, results types.ContainerResults) types.ContainerResults {
-	confCPUmin, err := resource.ParseQuantity(conf.Requests["cpu"].Min)
-	if err != nil {
-		log.Error(err, "cpu min parse quan")
+func validateContainer(conf conf.Configuration, container corev1.Container) ContainerValidation {
+	cv := ContainerValidation{
+		Container: container,
 	}
-	// CPUmax, err := resource.ParseQuantity(conf["cpu"].Max)
-	// if err != nil {
-	// 	log.Error(err, "cpu max parse quan")
-	// }
 
-	containerRequests := c.Resources.Requests.Cpu()
-	if containerRequests.MilliValue() < confCPUmin.MilliValue() {
-		results.AddFailure("CPU requests", confCPUmin.String(), containerRequests.String())
-	}
+	cv.validateResources(conf.Resources)
+	// cv.validateHealthChecks(conf.HealthChecks)
+	// cv.validateTags(conf.Image)
 
-	if c.Resources.Requests.Memory().IsZero() {
-		results.AddFailure("Memory requests", "placeholder", "placeholder")
-	}
-	if c.Resources.Limits.Cpu().IsZero() {
-		results.AddFailure("CPU limits", "placeholder", "placeholder")
-	}
-	if c.Resources.Limits.Memory().IsZero() {
-		results.AddFailure("Memory limits", "placeholder", "placeholder")
-	}
-	return results
+	return cv
 }
 
-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) addFailure(name, expected, actual string) {
+	cv.Failures = append(cv.Failures, types.Failure{
+		Name:     name,
+		Expected: expected,
+		Actual:   actual,
+	})
+}
 
-	if c.LivenessProbe == nil {
-		results.AddFailure("Liveness Probe", "placeholder", "placeholder")
-	}
-	return results
+func (cv *ContainerValidation) validateResources(conf conf.RequestsAndLimits) {
+	actualRes := cv.Container.Resources
+	cv.withinRange("requests.cpu", conf.Requests["cpu"], actualRes.Requests.Cpu())
+	cv.withinRange("requests.memory", conf.Requests["memory"], actualRes.Requests.Memory())
+	cv.withinRange("limits.cpu", conf.Limits["cpu"], actualRes.Limits.Cpu())
+	cv.withinRange("limits.memory", conf.Limits["memory"], actualRes.Limits.Memory())
 }
 
-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")
+func (cv *ContainerValidation) withinRange(resourceName string, expectedRange conf.ResourceMinMax, actual *resource.Quantity) {
+	expectedMin, err := resource.ParseQuantity(expectedRange.Min)
+	if err != nil {
+		log.Error(err, fmt.Sprintf("Error parsing min quantity for %s", resourceName))
+	} else if expectedMin.MilliValue() > actual.MilliValue() {
+		cv.addFailure(resourceName, expectedMin.String(), actual.String())
 	}
-	return results
-}
 
-func hostPort(conf conf.ResourceRequestsAndLimits, c corev1.Container, results types.ContainerResults) types.ContainerResults {
-	for _, port := range c.Ports {
-		if port.HostPort != 0 {
-			results.AddFailure("Host port", "placeholder", "placeholder")
-		}
+	expectedMax, err := resource.ParseQuantity(expectedRange.Max)
+	if err != nil {
+		log.Error(err, fmt.Sprintf("Error parsing max quantity for %s", resourceName))
+	} else if expectedMax.MilliValue() < actual.MilliValue() {
+		cv.addFailure(resourceName, expectedMax.String(), actual.String())
 	}
-	return results
 }
+
+// func probes(conf conf.ResourceRequestsAndLimits, c corev1.Container, results types.ContainerResults) types.ContainerResults {
+// 	if c.ReadinessProbe == nil {
+// 		results.AddFailure("Readiness Probe", "placeholder", "placeholder")
+// 	}
+
+// 	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 hostPort(conf conf.ResourceRequestsAndLimits, c corev1.Container, results types.ContainerResults) types.ContainerResults {
+// 	for _, port := range c.Ports {
+// 		if port.HostPort != 0 {
+// 			results.AddFailure("Host port", "placeholder", "placeholder")
+// 		}
+// 	}
+// 	return results
+// }

pkg/validator/pod.go

@@ -19,7 +19,6 @@ import (
 	"net/http"
 
 	conf "github.com/reactiveops/fairwinds/pkg/config"
-	"github.com/reactiveops/fairwinds/pkg/report"
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
@@ -49,23 +48,23 @@ func (v *PodValidator) Handle(ctx context.Context, req types.Request) types.Resp
 		return admission.ErrorResponse(http.StatusBadRequest, err)
 	}
 
-	results := validatePods(v.Config, pod, report.Results{})
+	results := validatePods(v.Config, pod, Results{})
 	allowed, reason := results.Format()
 
 	return admission.ValidationResponse(allowed, reason)
 }
 
-func validatePods(conf conf.Configuration, pod *corev1.Pod, results report.Results) report.Results {
+func validatePods(conf conf.Configuration, pod *corev1.Pod, results Results) Results {
 	for _, container := range pod.Spec.InitContainers {
-		results.InitContainers = append(
-			results.InitContainers,
+		results.InitContainerValidations = append(
+			results.InitContainerValidations,
 			validateContainer(conf, container),
 		)
 	}
 
 	for _, container := range pod.Spec.Containers {
-		results.Containers = append(
-			results.Containers,
+		results.ContainerValidations = append(
+			results.ContainerValidations,
 			validateContainer(conf, container),
 		)
 	}

pkg/validator/report.go

@@ -0,0 +1,48 @@
+package validator
+
+import (
+	"fmt"
+	"strings"
+)
+
+// Results contains the validation check results.
+type Results struct {
+	Pass                     bool
+	FailMsg                  string
+	ContainerValidations     []ContainerValidation
+	InitContainerValidations []ContainerValidation
+}
+
+// Format structures the validation results to return back to k8s API.
+func (r *Results) Format() (bool, string) {
+	var sb strings.Builder
+	r.Pass = true
+
+	for _, cv := range r.ContainerValidations {
+		if !validContainer(&sb, &cv) {
+			r.Pass = false
+		}
+	}
+
+	for _, cv := range r.InitContainerValidations {
+		if !validContainer(&sb, &cv) {
+			r.Pass = false
+		}
+	}
+
+	r.FailMsg = sb.String()
+	return r.Pass, r.FailMsg
+}
+
+func validContainer(sb *strings.Builder, cv *ContainerValidation) bool {
+	if len(cv.Failures) == 0 {
+		return true
+	}
+
+	s := fmt.Sprintf("\nContainer: %s\n Failure/s:\n", cv.Container.Name)
+	sb.WriteString(s)
+	for _, failure := range cv.Failures {
+		sb.WriteString(failure.Reason())
+	}
+	return false
+}