From 0d3dd37df1e4aa158492523eb0145a6895cdd30d Mon Sep 17 00:00:00 2001 From: Martin Schuppert Date: Thu, 4 Jul 2024 15:32:56 +0200 Subject: [PATCH] [validation] check memcached instance names are valid The memcached controller creates StatefulSet for the memcached to run. This adds a StatefulSet pod's label "controller-revision-hash": "-" to the pod. The kubernetes label is restricted under 63 char and the revision hash is an int32, 10 chars + the hyphen. With this the max name must not exceed 52 chars. Also the name of the created memcached instance must match a lowercase RFC 1123. Depends-On: https://github.com/openstack-k8s-operators/lib-common/pull/532 --- .../v1beta1/openstackcontrolplane_webhook.go | 25 ++++++- apis/go.mod | 4 + apis/go.sum | 8 +- go.mod | 4 + go.sum | 8 +- .../openstackoperator_controller_test.go | 74 +++++++++++++++++++ 6 files changed, 113 insertions(+), 10 deletions(-) diff --git a/apis/core/v1beta1/openstackcontrolplane_webhook.go b/apis/core/v1beta1/openstackcontrolplane_webhook.go index 12053b883..2197d948d 100644 --- a/apis/core/v1beta1/openstackcontrolplane_webhook.go +++ b/apis/core/v1beta1/openstackcontrolplane_webhook.go @@ -23,8 +23,11 @@ import ( keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1" "github.com/openstack-k8s-operators/lib-common/modules/common/route" + common_webhook "github.com/openstack-k8s-operators/lib-common/modules/common/webhook" mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" placementv1 "github.com/openstack-k8s-operators/placement-operator/api/v1beta1" + "golang.org/x/exp/maps" + "golang.org/x/exp/slices" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -35,8 +38,6 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "golang.org/x/exp/maps" - "golang.org/x/exp/slices" barbicanv1 "github.com/openstack-k8s-operators/barbican-operator/api/v1beta1" cinderv1 "github.com/openstack-k8s-operators/cinder-operator/api/v1beta1" @@ -313,6 +314,16 @@ func (r *OpenStackControlPlane) ValidateCreateServices(basePath *field.Path) (ad } } + if r.Spec.Memcached.Enabled { + if r.Spec.Memcached.Templates != nil { + err := common_webhook.ValidateDNS1123Label( + basePath.Child("memcached").Child("template"), + maps.Keys(*r.Spec.Memcached.Templates), + 11) // omit issue with statefulset pod label "controller-revision-hash": "-" + errors = append(errors, err...) + } + } + return warnings, errors } @@ -414,6 +425,16 @@ func (r *OpenStackControlPlane) ValidateUpdateServices(old OpenStackControlPlane errors = append(errors, r.Spec.Designate.Template.ValidateUpdate(*old.Designate.Template, basePath.Child("designate").Child("template"))...) } + if r.Spec.Memcached.Enabled { + if r.Spec.Memcached.Templates != nil { + err := common_webhook.ValidateDNS1123Label( + basePath.Child("memcached").Child("template"), + maps.Keys(*r.Spec.Memcached.Templates), + 11) // omit issue with statefulset pod label "controller-revision-hash": "-" + errors = append(errors, err...) + } + } + return errors } diff --git a/apis/go.mod b/apis/go.mod index b70411e83..c961ed0e9 100644 --- a/apis/go.mod +++ b/apis/go.mod @@ -116,3 +116,7 @@ replace github.com/openshift/api => github.com/openshift/api v0.0.0-202304141430 // custom RabbitmqClusterSpecCore for OpenStackControlplane (v2.6.0_patches_tag) replace github.com/rabbitmq/cluster-operator/v2 => github.com/openstack-k8s-operators/rabbitmq-cluster-operator/v2 v2.6.1-0.20240626194327-e7df1b654cb7 //allow-merging + +replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/stuggi/lib-common/modules/common v0.0.0-20240711103924-a6adaedc7ed1 + +replace github.com/openstack-k8s-operators/infra-operator/apis => github.com/stuggi/infra-operator/apis v0.0.0-20240711124315-e4428864f742 diff --git a/apis/go.sum b/apis/go.sum index 25e898087..51798d5a5 100644 --- a/apis/go.sum +++ b/apis/go.sum @@ -102,14 +102,10 @@ github.com/openstack-k8s-operators/heat-operator/api v0.3.1-0.20240704231515-a1b github.com/openstack-k8s-operators/heat-operator/api v0.3.1-0.20240704231515-a1bb7731fd73/go.mod h1:pgGJi04RNzrElpZm3qgUSx1LpEKRzw0hFjWdqFOwgWA= github.com/openstack-k8s-operators/horizon-operator/api v0.3.1-0.20240704220914-63a9fd21e69b h1:HEOyzCIxJBLjDYYoZC+dAQUS8fOu2PbX9OrlPH1Z7sg= github.com/openstack-k8s-operators/horizon-operator/api v0.3.1-0.20240704220914-63a9fd21e69b/go.mod h1:WSpTrx0ry9ziJqILYYQD56m/uyMD7oh9Yr/PkGoDOR8= -github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240704231227-c7ee9d3de988 h1:in8msReo1uisAwdyjjWDJ1moIItt1jNRq5pLZy9+bbE= -github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240704231227-c7ee9d3de988/go.mod h1:gdc2zBX7iz+6vWjD2dhi+rtEmTulb91aybjU6bJyFVQ= github.com/openstack-k8s-operators/ironic-operator/api v0.3.1-0.20240704220915-746cf7c339c5 h1:7fmf9hOPG68ppqTCIYjGKDo1UM6egpTuGhknCXOopFk= github.com/openstack-k8s-operators/ironic-operator/api v0.3.1-0.20240704220915-746cf7c339c5/go.mod h1:t9jIbObwq9/kswhmUcdZBUH+L9xLtm0+5flPcPSBH8M= github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240704220917-e65444cf5f26 h1:rPemtm9yElXfCD3uKvUQWwxdPwpEXhJpUq5cxyKnQR4= github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240704220917-e65444cf5f26/go.mod h1:Pg+s5VIUvZNec600X7GtlGTAUD2vafi9GZ7V0guaujI= -github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240709153313-544a55696489 h1:4X/xF7z62rCHEui1yUVaE3/zdCleOcHsrsg6++oOrkY= -github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240709153313-544a55696489/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240624132705-6c8da3c0bbfd h1:solheCgtkDXK3AKTwZYmwoeQqXiwjB8K6T/YuxDjNDc= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240624132705-6c8da3c0bbfd/go.mod h1:zuPcZ5Kopr15AdfxvA0xqKIIGCZ0XbSe/0VHNKuvbEE= github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240624132705-6c8da3c0bbfd h1:MY3MDe11c9R/kp0ALVeaWHIdRpbQh9Xs3ym/Z/KBBlU= @@ -160,6 +156,10 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stuggi/infra-operator/apis v0.0.0-20240711124315-e4428864f742 h1:tBUdMP3uJheXS62hdxAOir3BKDDYuIYHdoWMq92mVVY= +github.com/stuggi/infra-operator/apis v0.0.0-20240711124315-e4428864f742/go.mod h1:eknPOejgiEKrI1dUmBnSAwKj2AMx7Ai52kKuVdglkzo= +github.com/stuggi/lib-common/modules/common v0.0.0-20240711103924-a6adaedc7ed1 h1:ZPLqxyJntbNhPYPyAnkTJvjDGgf6vA8aG/4fIm2+mMI= +github.com/stuggi/lib-common/modules/common v0.0.0-20240711103924-a6adaedc7ed1/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= diff --git a/go.mod b/go.mod index c9599ef2e..c8979afef 100644 --- a/go.mod +++ b/go.mod @@ -130,3 +130,7 @@ replace github.com/openshift/api => github.com/openshift/api v0.0.0-202304141430 // custom RabbitmqClusterSpecCore for OpenStackControlplane (v2.6.0_patches_tag) replace github.com/rabbitmq/cluster-operator/v2 => github.com/openstack-k8s-operators/rabbitmq-cluster-operator/v2 v2.6.1-0.20240626194327-e7df1b654cb7 //allow-merging + +replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/stuggi/lib-common/modules/common v0.0.0-20240711103924-a6adaedc7ed1 + +replace github.com/openstack-k8s-operators/infra-operator/apis => github.com/stuggi/infra-operator/apis v0.0.0-20240711124315-e4428864f742 diff --git a/go.sum b/go.sum index 61108c2ce..7a471eaac 100644 --- a/go.sum +++ b/go.sum @@ -110,8 +110,6 @@ github.com/openstack-k8s-operators/heat-operator/api v0.3.1-0.20240704231515-a1b github.com/openstack-k8s-operators/heat-operator/api v0.3.1-0.20240704231515-a1bb7731fd73/go.mod h1:pgGJi04RNzrElpZm3qgUSx1LpEKRzw0hFjWdqFOwgWA= github.com/openstack-k8s-operators/horizon-operator/api v0.3.1-0.20240704220914-63a9fd21e69b h1:HEOyzCIxJBLjDYYoZC+dAQUS8fOu2PbX9OrlPH1Z7sg= github.com/openstack-k8s-operators/horizon-operator/api v0.3.1-0.20240704220914-63a9fd21e69b/go.mod h1:WSpTrx0ry9ziJqILYYQD56m/uyMD7oh9Yr/PkGoDOR8= -github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240704231227-c7ee9d3de988 h1:in8msReo1uisAwdyjjWDJ1moIItt1jNRq5pLZy9+bbE= -github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240704231227-c7ee9d3de988/go.mod h1:gdc2zBX7iz+6vWjD2dhi+rtEmTulb91aybjU6bJyFVQ= github.com/openstack-k8s-operators/ironic-operator/api v0.3.1-0.20240704220915-746cf7c339c5 h1:7fmf9hOPG68ppqTCIYjGKDo1UM6egpTuGhknCXOopFk= github.com/openstack-k8s-operators/ironic-operator/api v0.3.1-0.20240704220915-746cf7c339c5/go.mod h1:t9jIbObwq9/kswhmUcdZBUH+L9xLtm0+5flPcPSBH8M= github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240704220917-e65444cf5f26 h1:rPemtm9yElXfCD3uKvUQWwxdPwpEXhJpUq5cxyKnQR4= @@ -120,8 +118,6 @@ github.com/openstack-k8s-operators/lib-common/modules/ansible v0.3.1-0.202406241 github.com/openstack-k8s-operators/lib-common/modules/ansible v0.3.1-0.20240624132705-6c8da3c0bbfd/go.mod h1:tP+nxk95PisCKJaXE/an2igG9lluxuOVhdmV9WtkR2s= github.com/openstack-k8s-operators/lib-common/modules/certmanager v0.0.0-20240624132705-6c8da3c0bbfd h1:NuDKT4KCTJNun1glbc6ULOoaO6hRY4Ct22gYGrZd6Eg= github.com/openstack-k8s-operators/lib-common/modules/certmanager v0.0.0-20240624132705-6c8da3c0bbfd/go.mod h1:GooNi6hM78cOUMjhBy0fXsZIcDTK1dUb1rlay170IJo= -github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240709153313-544a55696489 h1:4X/xF7z62rCHEui1yUVaE3/zdCleOcHsrsg6++oOrkY= -github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240709153313-544a55696489/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240624132705-6c8da3c0bbfd h1:solheCgtkDXK3AKTwZYmwoeQqXiwjB8K6T/YuxDjNDc= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240624132705-6c8da3c0bbfd/go.mod h1:zuPcZ5Kopr15AdfxvA0xqKIIGCZ0XbSe/0VHNKuvbEE= github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240624132705-6c8da3c0bbfd h1:MY3MDe11c9R/kp0ALVeaWHIdRpbQh9Xs3ym/Z/KBBlU= @@ -183,6 +179,10 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stuggi/infra-operator/apis v0.0.0-20240711124315-e4428864f742 h1:tBUdMP3uJheXS62hdxAOir3BKDDYuIYHdoWMq92mVVY= +github.com/stuggi/infra-operator/apis v0.0.0-20240711124315-e4428864f742/go.mod h1:eknPOejgiEKrI1dUmBnSAwKj2AMx7Ai52kKuVdglkzo= +github.com/stuggi/lib-common/modules/common v0.0.0-20240711103924-a6adaedc7ed1 h1:ZPLqxyJntbNhPYPyAnkTJvjDGgf6vA8aG/4fIm2+mMI= +github.com/stuggi/lib-common/modules/common v0.0.0-20240711103924-a6adaedc7ed1/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= diff --git a/tests/functional/ctlplane/openstackoperator_controller_test.go b/tests/functional/ctlplane/openstackoperator_controller_test.go index 35b390f2d..4725481cf 100644 --- a/tests/functional/ctlplane/openstackoperator_controller_test.go +++ b/tests/functional/ctlplane/openstackoperator_controller_test.go @@ -1863,4 +1863,78 @@ var _ = Describe("OpenStackOperator Webhook", func() { ), ) }) + + It("Blocks creating ctlplane CRs with to long memcached keys/names", func() { + spec := GetDefaultOpenStackControlPlaneSpec() + + memcachedTemplate := map[string]interface{}{ + "foo-1234567890-1234567890-1234567890-1234567890-1234567890": map[string]interface{}{ + "replicas": 1, + }, + } + + spec["memcached"] = map[string]interface{}{ + "enabled": true, + "templates": memcachedTemplate, + } + + raw := map[string]interface{}{ + "apiVersion": "core.openstack.org/v1beta1", + "kind": "OpenStackControlPlane", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": namespace, + }, + "spec": spec, + } + + unstructuredObj := &unstructured.Unstructured{Object: raw} + _, err := controllerutil.CreateOrPatch( + th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil }) + Expect(err).Should(HaveOccurred()) + var statusError *k8s_errors.StatusError + Expect(errors.As(err, &statusError)).To(BeTrue()) + Expect(statusError.ErrStatus.Details.Kind).To(Equal("OpenStackControlPlane")) + Expect(statusError.ErrStatus.Message).To( + ContainSubstring( + "Invalid value: \"foo-1234567890-1234567890-1234567890-1234567890-1234567890\": must be no more than 52 characters"), + ) + }) + + It("Blocks creating ctlplane CRs with wrong memcached keys/names", func() { + spec := GetDefaultOpenStackControlPlaneSpec() + + memcachedTemplate := map[string]interface{}{ + "foo_bar": map[string]interface{}{ + "replicas": 1, + }, + } + + spec["memcached"] = map[string]interface{}{ + "enabled": true, + "templates": memcachedTemplate, + } + + raw := map[string]interface{}{ + "apiVersion": "core.openstack.org/v1beta1", + "kind": "OpenStackControlPlane", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": namespace, + }, + "spec": spec, + } + + unstructuredObj := &unstructured.Unstructured{Object: raw} + _, err := controllerutil.CreateOrPatch( + th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil }) + Expect(err).Should(HaveOccurred()) + var statusError *k8s_errors.StatusError + Expect(errors.As(err, &statusError)).To(BeTrue()) + Expect(statusError.ErrStatus.Details.Kind).To(Equal("OpenStackControlPlane")) + Expect(statusError.ErrStatus.Message).To( + ContainSubstring( + "Invalid value: \"foo_bar\": a lowercase RFC 1123 label must consist"), + ) + }) })