From e8223d9204ecf1619f87295d5850539e19a83a0d Mon Sep 17 00:00:00 2001 From: Varsha Varadarajan Date: Thu, 20 Jun 2019 15:13:43 -0400 Subject: [PATCH] Separate messages for malformed image and fully qualified image. --- checks/basic/fully_qualified_image.go | 29 +++++++----- checks/basic/fully_qualified_image_test.go | 51 ++++++++++++++++++---- checks/basic/helper_test.go | 6 +-- checks/basic/latest_tag_test.go | 24 +++++----- 4 files changed, 76 insertions(+), 34 deletions(-) diff --git a/checks/basic/fully_qualified_image.go b/checks/basic/fully_qualified_image.go index 43948df..ef9a6c0 100644 --- a/checks/basic/fully_qualified_image.go +++ b/checks/basic/fully_qualified_image.go @@ -34,27 +34,36 @@ func (fq *fullyQualifiedImageCheck) Description() string { // Run runs this check on a set of Kubernetes objects. It can return warnings // (low-priority problems) and errors (high-priority problems) as well as an // error value indicating that the check failed to run. -func (fq *fullyQualifiedImageCheck) Run(objects *kube.Objects) (warnings []error, errors []error, err error) { - var w []error +func (fq *fullyQualifiedImageCheck) Run(objects *kube.Objects) ([]error, []error, error) { + var warnings, errors []error for _, pod := range objects.Pods.Items { podName := pod.GetName() namespace := pod.GetNamespace() - w = append(w, checkImage(pod.Spec.Containers, podName, namespace)...) - w = append(w, checkImage(pod.Spec.InitContainers, podName, namespace)...) + w, e := checkImage(pod.Spec.Containers, podName, namespace) + warnings = append(warnings, w...) + errors = append(errors, e...) + w, e = checkImage(pod.Spec.InitContainers, podName, namespace) + warnings = append(warnings, w...) + errors = append(errors, e...) } - return w, nil, nil + return warnings, errors, nil } // checkImage checks if the image name is fully qualified // Adds a warning if the container does not use a fully qualified image name -func checkImage(containers []corev1.Container, podName string, namespace string) []error { - var w []error +func checkImage(containers []corev1.Container, podName string, namespace string) ([]error, []error) { + var w, e []error for _, container := range containers { - if value, err := reference.ParseAnyReference(container.Image); err != nil || value.String() != container.Image { - w = append(w, fmt.Errorf("[Best Practice] Use fully qualified image for container '%s' in pod '%s' in namespace '%s'", container.Name, podName, namespace)) + value, err := reference.ParseAnyReference(container.Image) + if err != nil { + e = append(e, fmt.Errorf("[Error] Malformed image name for container '%s' in pod '%s' in namespace '%s'", container.Name, podName, namespace)) + } else { + if value.String() != container.Image { + w = append(w, fmt.Errorf("[Best Practice] Use fully qualified image for container '%s' in pod '%s' in namespace '%s'", container.Name, podName, namespace)) + } } } - return w + return w, e } diff --git a/checks/basic/fully_qualified_image_test.go b/checks/basic/fully_qualified_image_test.go index 6122ed0..ba9e415 100644 --- a/checks/basic/fully_qualified_image_test.go +++ b/checks/basic/fully_qualified_image_test.go @@ -43,7 +43,7 @@ func TestFullyQualifiedImageWarning(t *testing.T) { { name: "pod with container image - busybox:latest", arg: container("busybox:latest"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - k8s.gcr.io/busybox", @@ -53,7 +53,7 @@ func TestFullyQualifiedImageWarning(t *testing.T) { { name: "pod with container image - busybox", arg: container("busybox"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - test:5000/repo/image@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", @@ -63,7 +63,7 @@ func TestFullyQualifiedImageWarning(t *testing.T) { { name: "pod with container image - repo/image@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", arg: container("repo/image@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - test:5000/repo/image:ignore-tag@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", @@ -73,7 +73,7 @@ func TestFullyQualifiedImageWarning(t *testing.T) { { name: "pod with container image - repo/image:ignore-tag@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", arg: container("repo/image:ignore-tag@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - k8s.gcr.io/busybox:latest", @@ -83,7 +83,7 @@ func TestFullyQualifiedImageWarning(t *testing.T) { { name: "pod with container image - busybox:latest", arg: initContainer("busybox:latest"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - k8s.gcr.io/busybox", @@ -93,7 +93,7 @@ func TestFullyQualifiedImageWarning(t *testing.T) { { name: "pod with container image - busybox", arg: initContainer("busybox"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - test:5000/repo/image@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", @@ -103,7 +103,7 @@ func TestFullyQualifiedImageWarning(t *testing.T) { { name: "pod with container image - repo/image@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", arg: initContainer("repo/image@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - test:5000/repo/image:ignore-tag@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", @@ -113,7 +113,7 @@ func TestFullyQualifiedImageWarning(t *testing.T) { { name: "pod with container image - repo/image:ignore-tag@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", arg: initContainer("repo/image:ignore-tag@sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"), - expected: warnings(warning), + expected: issues(warning), }, } @@ -122,9 +122,42 @@ func TestFullyQualifiedImageWarning(t *testing.T) { for _, scenario := range scenarios { t.Run(scenario.name, func(t *testing.T) { w, e, err := fullyQualifiedImageCheck.Run(scenario.arg) + assert.NoError(t, err) assert.ElementsMatch(t, scenario.expected, w) assert.Empty(t, e) - assert.Nil(t, err) + + }) + } +} + +func TestMalformedImageError(t *testing.T) { + const e string = "[Error] Malformed image name for container 'bar' in pod 'pod_foo' in namespace 'k8s'" + + scenarios := []struct { + name string + arg *kube.Objects + expected []error + }{ + { + name: "container with image : test:5000/repo/image@sha256:digest", + arg: container("test:5000/repo/image@sha256:digest"), + expected: issues(e), + }, + { + name: "init container with image : test:5000/repo/image@sha256:digest", + arg: initContainer("test:5000/repo/image@sha256:digest"), + expected: issues(e), + }, + } + fullyQualifiedImageCheck := fullyQualifiedImageCheck{} + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + w, e, err := fullyQualifiedImageCheck.Run(scenario.arg) + assert.NoError(t, err) + assert.ElementsMatch(t, scenario.expected, e) + assert.Empty(t, w) + }) } } diff --git a/checks/basic/helper_test.go b/checks/basic/helper_test.go index e0f9bb3..ecbd90a 100644 --- a/checks/basic/helper_test.go +++ b/checks/basic/helper_test.go @@ -45,9 +45,9 @@ func initContainer(image string) *kube.Objects { return objs } -func warnings(s string) []error { - w := []error{ +func issues(s string) []error { + issue := []error{ fmt.Errorf(s), } - return w + return issue } diff --git a/checks/basic/latest_tag_test.go b/checks/basic/latest_tag_test.go index d896e7e..4ffc2b9 100644 --- a/checks/basic/latest_tag_test.go +++ b/checks/basic/latest_tag_test.go @@ -37,32 +37,32 @@ func TestLatestTagWarning(t *testing.T) { { name: "pod with container image - k8s.gcr.io/busybox:latest", arg: container("k8s.gcr.io/busybox:latest"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - busybox:latest", arg: container("busybox:latest"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - k8s.gcr.io/busybox", arg: container("k8s.gcr.io/busybox"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - busybox", arg: container("busybox"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - private:5000/repo/busybox", arg: container("http://private:5000/repo/busybox"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - private:5000/repo/busybox:latest", arg: container("http://private:5000/repo/busybox:latest"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - test:5000/repo@sha256:digest", @@ -88,32 +88,32 @@ func TestLatestTagWarning(t *testing.T) { { name: "pod with init container image - k8s.gcr.io/busybox:latest", arg: initContainer("k8s.gcr.io/busybox:latest"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with init container image - busybox:latest", arg: initContainer("busybox:latest"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with init container image - k8s.gcr.io/busybox", arg: initContainer("k8s.gcr.io/busybox"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with init container image - busybox", arg: initContainer("busybox"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - http://private:5000/repo/busybox", arg: container("http://private:5000/repo/busybox"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - http://private:5000/repo/busybox:latest", arg: container("http://private:5000/repo/busybox:latest"), - expected: warnings(warning), + expected: issues(warning), }, { name: "pod with container image - test:5000/repo@sha256:digest",