From 98f08f09e01a276851f1d97e5eae8d41ff31dc7a Mon Sep 17 00:00:00 2001 From: Jonathan Knight Date: Fri, 15 Sep 2023 13:20:47 +0300 Subject: [PATCH] Validate image names in the web-hook (#619) --- api/v1/coherence_webhook.go | 53 +++++- api/v1/coherence_webhook_image_test.go | 252 +++++++++++++++++++++++++ api/v1/coherence_webhook_job.go | 15 +- go.mod | 2 + go.sum | 4 + 5 files changed, 317 insertions(+), 9 deletions(-) create mode 100644 api/v1/coherence_webhook_image_test.go diff --git a/api/v1/coherence_webhook.go b/api/v1/coherence_webhook.go index 61f3c627..9fe5f144 100644 --- a/api/v1/coherence_webhook.go +++ b/api/v1/coherence_webhook.go @@ -8,8 +8,10 @@ package v1 import ( "fmt" + "github.com/distribution/reference" "github.com/go-test/deep" "github.com/oracle/coherence-operator/pkg/operator" + "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime" @@ -132,16 +134,19 @@ var commonWebHook = CommonWebHook{} // The optional warnings will be added to the response as warning messages. // Return an error if the object is invalid. func (in *Coherence) ValidateCreate() (admission.Warnings, error) { - var err error var warnings admission.Warnings webhookLogger.Info("validate create", "name", in.Name) - err = commonWebHook.validateReplicas(in) - if err != nil { + if err := commonWebHook.validateReplicas(in); err != nil { + return warnings, err + } + if err := commonWebHook.validateImages(in); err != nil { return warnings, err } - err = commonWebHook.validateNodePorts(in) - return warnings, err + if err := commonWebHook.validateNodePorts(in); err != nil { + return warnings, err + } + return warnings, nil } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type @@ -154,6 +159,9 @@ func (in *Coherence) ValidateUpdate(previous runtime.Object) (admission.Warnings if err := commonWebHook.validateReplicas(in); err != nil { return warnings, err } + if err := commonWebHook.validateImages(in); err != nil { + return warnings, err + } prev := previous.(*Coherence) if err := commonWebHook.validatePersistence(in, prev); err != nil { @@ -211,6 +219,41 @@ func (in *Coherence) validateVolumeClaimTemplates(previous *Coherence) error { type CommonWebHook struct { } +// validateImages validates image names +func (in *CommonWebHook) validateImages(c CoherenceResource) error { + var err error + spec := c.GetSpec() + if spec != nil { + img := spec.GetCoherenceImage() + if img != nil { + _, err = reference.Parse(*img) + if err != nil { + return errors.Errorf("invalid spec.image field, %s", err.Error()) + } + } + img = spec.GetCoherenceOperatorImage() + if img != nil { + _, err = reference.Parse(*img) + if err != nil { + return errors.Errorf("invalid spec.coherenceUtils.image field, %s", err.Error()) + } + } + for _, c := range spec.InitContainers { + _, err = reference.Parse(c.Image) + if err != nil { + return errors.Errorf("invalid image name in init-container %s, %s", c.Name, err.Error()) + } + } + for _, c := range spec.SideCars { + _, err = reference.Parse(c.Image) + if err != nil { + return errors.Errorf("invalid image name in side-car container %s, %s", c.Name, err.Error()) + } + } + } + return err +} + // validateReplicas validates that spec.replicas >= 0 func (in *CommonWebHook) validateReplicas(c CoherenceResource) error { replicas := c.GetReplicas() diff --git a/api/v1/coherence_webhook_image_test.go b/api/v1/coherence_webhook_image_test.go new file mode 100644 index 00000000..acd40f02 --- /dev/null +++ b/api/v1/coherence_webhook_image_test.go @@ -0,0 +1,252 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. + * Licensed under the Universal Permissive License v 1.0 as shown at + * http://oss.oracle.com/licenses/upl. + */ + +package v1_test + +import ( + . "github.com/onsi/gomega" + coh "github.com/oracle/coherence-operator/api/v1" + corev1 "k8s.io/api/core/v1" + "testing" +) + +// Tests for image name validation + +func TestCoherenceWithNoImageNames(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.Coherence{} + _, err := c.ValidateCreate() + g.Expect(err).NotTo(HaveOccurred()) +} + +func TestCoherenceCreateWithValidImageName(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.Coherence{ + Spec: coh.CoherenceStatefulSetResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + Image: stringPtr("test/coherence:1.0"), + }, + }, + } + _, err := c.ValidateCreate() + g.Expect(err).NotTo(HaveOccurred()) +} + +func TestCoherenceCreateWithInvalidImageName(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.Coherence{ + Spec: coh.CoherenceStatefulSetResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + Image: stringPtr("test/bad image name:1.0"), + }, + }, + } + _, err := c.ValidateCreate() + g.Expect(err).To(HaveOccurred()) +} + +func TestCoherenceCreateWithImageNameWithTrailingSpace(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.Coherence{ + Spec: coh.CoherenceStatefulSetResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + Image: stringPtr("test/coherence:1.0 "), + }, + }, + } + _, err := c.ValidateCreate() + g.Expect(err).To(HaveOccurred()) +} + +func TestCoherenceCreateWithValidOperatorImageName(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.Coherence{ + Spec: coh.CoherenceStatefulSetResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + CoherenceUtils: &coh.ImageSpec{ + Image: stringPtr("test/coherence:1.0"), + }, + }, + }, + } + _, err := c.ValidateCreate() + g.Expect(err).NotTo(HaveOccurred()) +} + +func TestCoherenceCreateWithInvalidOperatorImageName(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.Coherence{ + Spec: coh.CoherenceStatefulSetResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + CoherenceUtils: &coh.ImageSpec{ + Image: stringPtr("test/bad image name:1.0"), + }, + }, + }, + } + _, err := c.ValidateCreate() + g.Expect(err).To(HaveOccurred()) +} + +func TestCoherenceUpdateWithInvalidImageName(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.Coherence{ + Spec: coh.CoherenceStatefulSetResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + Image: stringPtr("test/bad image name:1.0"), + }, + }, + } + _, err := c.ValidateUpdate(&c) + g.Expect(err).To(HaveOccurred()) +} + +func TestCoherenceCreateWithValidInitContainerImageName(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.Coherence{ + Spec: coh.CoherenceStatefulSetResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + InitContainers: []corev1.Container{ + { + Name: "side-one", + Image: "test/coherence:1.0", + }, + }, + }, + }, + } + _, err := c.ValidateCreate() + g.Expect(err).NotTo(HaveOccurred()) +} + +func TestCoherenceCreateWithInvalidInitContainerImageName(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.Coherence{ + Spec: coh.CoherenceStatefulSetResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + InitContainers: []corev1.Container{ + { + Name: "side-one", + Image: "test/bad image name:1.0", + }, + }, + }, + }, + } + _, err := c.ValidateCreate() + g.Expect(err).To(HaveOccurred()) +} + +func TestCoherenceCreateWithValidSidecarImageName(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.Coherence{ + Spec: coh.CoherenceStatefulSetResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + SideCars: []corev1.Container{ + { + Name: "side-one", + Image: "test/coherence:1.0", + }, + }, + }, + }, + } + _, err := c.ValidateCreate() + g.Expect(err).NotTo(HaveOccurred()) +} + +func TestCoherenceCreateWithInvalidSidecarImageName(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.Coherence{ + Spec: coh.CoherenceStatefulSetResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + SideCars: []corev1.Container{ + { + Name: "side-one", + Image: "test/bad image name:1.0", + }, + }, + }, + }, + } + _, err := c.ValidateCreate() + g.Expect(err).To(HaveOccurred()) +} + +func TestJobWithNoImageNames(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.CoherenceJob{} + _, err := c.ValidateCreate() + g.Expect(err).NotTo(HaveOccurred()) +} + +func TestJobCreateWithInvalidImageName(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.CoherenceJob{ + Spec: coh.CoherenceJobResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + Image: stringPtr("test/bad image name:1.0"), + }, + }, + } + _, err := c.ValidateCreate() + g.Expect(err).To(HaveOccurred()) +} + +func TestJobCreateWithValidImageDigest(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.CoherenceJob{ + Spec: coh.CoherenceJobResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + Image: stringPtr("ghcr.io@sha256:f8a592ee6d31c02feea037c269a87564ae666f91480d1d6be24ff9dd1675c7d0"), + }, + }, + } + _, err := c.ValidateCreate() + g.Expect(err).NotTo(HaveOccurred()) +} + +func TestJobCreateWithInvalidImageDigest(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.CoherenceJob{ + Spec: coh.CoherenceJobResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + Image: stringPtr("test@sha256:1234"), + }, + }, + } + _, err := c.ValidateCreate() + g.Expect(err).To(HaveOccurred()) +} + +func TestJobUpdateWithInvalidImageName(t *testing.T) { + g := NewGomegaWithT(t) + + c := coh.CoherenceJob{ + Spec: coh.CoherenceJobResourceSpec{ + CoherenceResourceSpec: coh.CoherenceResourceSpec{ + Image: stringPtr("test/bad image name:1.0"), + }, + }, + } + _, err := c.ValidateUpdate(&c) + g.Expect(err).To(HaveOccurred()) +} diff --git a/api/v1/coherence_webhook_job.go b/api/v1/coherence_webhook_job.go index b9295f5b..607b653c 100644 --- a/api/v1/coherence_webhook_job.go +++ b/api/v1/coherence_webhook_job.go @@ -85,12 +85,16 @@ func (in *CoherenceJob) ValidateCreate() (admission.Warnings, error) { var warnings admission.Warnings webhookLogger.Info("validate create", "name", in.Name) - err = commonWebHook.validateReplicas(in) - if err != nil { + if err = commonWebHook.validateReplicas(in); err != nil { return warnings, err } - err = commonWebHook.validateNodePorts(in) - return warnings, err + if err = commonWebHook.validateImages(in); err != nil { + return warnings, err + } + if err = commonWebHook.validateNodePorts(in); err != nil { + return warnings, err + } + return warnings, nil } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type @@ -101,6 +105,9 @@ func (in *CoherenceJob) ValidateUpdate(previous runtime.Object) (admission.Warni if err := commonWebHook.validateReplicas(in); err != nil { return warnings, err } + if err := commonWebHook.validateImages(in); err != nil { + return warnings, err + } prev := previous.(*CoherenceJob) if err := commonWebHook.validatePersistence(in, prev); err != nil { diff --git a/go.mod b/go.mod index 98a0b00e..aa912a7e 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.20 require ( github.com/davecgh/go-spew v1.1.1 + github.com/distribution/reference v0.5.0 github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 github.com/go-logr/logr v1.2.4 github.com/go-test/deep v1.1.0 @@ -56,6 +57,7 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/opencontainers/go-digest v1.0.0 // indirect github.com/pelletier/go-toml/v2 v2.0.8 // indirect github.com/prometheus/client_golang v1.16.0 // indirect github.com/prometheus/client_model v0.4.0 // indirect diff --git a/go.sum b/go.sum index 42f2a0e4..6eb2f758 100644 --- a/go.sum +++ b/go.sum @@ -58,6 +58,8 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/distribution/reference v0.5.0 h1:/FUIFXtfc/x2gpa5/VGfiGLuOIdYa1t65IKK2OFGvA0= +github.com/distribution/reference v0.5.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= github.com/emicklei/go-restful/v3 v3.9.0 h1:XwGDlfxEnQZzuopoqxwSEllNcCOM9DhhFyhFIIGKwxE= github.com/emicklei/go-restful/v3 v3.9.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= @@ -216,6 +218,8 @@ github.com/onsi/ginkgo/v2 v2.11.0 h1:WgqUCUt/lT6yXoQ8Wef0fsNn5cAuMK7+KT9UFRz2tcU github.com/onsi/gomega v1.3.0/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.27.10 h1:naR28SdDFlqrG6kScpT8VWpu1xWY5nJRCF3XaYyBjhI= github.com/onsi/gomega v1.27.10/go.mod h1:RsS8tutOdbdgzbPtzzATp12yT7kM5I5aElG3evPbQ0M= +github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= +github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/pelletier/go-toml/v2 v2.0.8 h1:0ctb6s9mE31h0/lhu+J6OPmVeDxJn+kYnJc2jZR9tGQ= github.com/pelletier/go-toml/v2 v2.0.8/go.mod h1:vuYfssBdrU2XDZ9bYydBu6t+6a6PYNcZljzZR9VXg+4= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=