From b7ef58f2b00c94b4c007d43f01d4cbea34472c0d 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 Depends-On: https://github.com/openstack-k8s-operators/infra-operator/pull/245 Jira: https://issues.redhat.com/browse/OSPRH-8063 Signed-off-by: Martin Schuppert --- .../v1beta1/openstackcontrolplane_webhook.go | 25 ++++++- apis/go.mod | 2 +- apis/go.sum | 4 +- go.mod | 2 +- go.sum | 4 +- .../openstackoperator_controller_test.go | 74 +++++++++++++++++++ 6 files changed, 103 insertions(+), 8 deletions(-) diff --git a/apis/core/v1beta1/openstackcontrolplane_webhook.go b/apis/core/v1beta1/openstackcontrolplane_webhook.go index 12053b883..22105854e 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), + memcachedv1.CrMaxLengthCorrection) // 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), + memcachedv1.CrMaxLengthCorrection) // 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 aedd29581..8aaaea3ac 100644 --- a/apis/go.mod +++ b/apis/go.mod @@ -13,7 +13,7 @@ require ( github.com/openstack-k8s-operators/glance-operator/api v0.4.1-0.20240714131453-8248cbbd71b0 github.com/openstack-k8s-operators/heat-operator/api v0.4.1-0.20240715084339-1e2113ac57b7 github.com/openstack-k8s-operators/horizon-operator/api v0.4.1-0.20240715101812-a40a0701a6f4 - github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240715063952-446730fefacf + github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240716081244-6191389eafd5 github.com/openstack-k8s-operators/ironic-operator/api v0.4.1-0.20240710183123-6fe249a4f5c9 github.com/openstack-k8s-operators/keystone-operator/api v0.4.1-0.20240715064816-751c29ed6ef1 github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240715111029-358ba8041494 diff --git a/apis/go.sum b/apis/go.sum index 37b1593ee..eb2e295ce 100644 --- a/apis/go.sum +++ b/apis/go.sum @@ -102,8 +102,8 @@ github.com/openstack-k8s-operators/heat-operator/api v0.4.1-0.20240715084339-1e2 github.com/openstack-k8s-operators/heat-operator/api v0.4.1-0.20240715084339-1e2113ac57b7/go.mod h1:VdfRsAYy7pAPH71QM5DPWk9t/Y+l+w+7n651zm9XnvU= github.com/openstack-k8s-operators/horizon-operator/api v0.4.1-0.20240715101812-a40a0701a6f4 h1:0tV5GXv9vmJdybZQxq15MQP+rO1LKNrdv92z2ra0+tg= github.com/openstack-k8s-operators/horizon-operator/api v0.4.1-0.20240715101812-a40a0701a6f4/go.mod h1:DEG5iAP0G1LhWnszDJ3e63EAgVi+xqKLHwCnQVajJbA= -github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240715063952-446730fefacf h1:ArEuU0sTGh/DcPrkfAzlcGQe/f926uv64SwMJiVL0qs= -github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240715063952-446730fefacf/go.mod h1:Dyp7Ug8BetGwy/Xl3fy55+A1wU0KuFZ6mFaKLecbHwM= +github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240716081244-6191389eafd5 h1:GukL/mPmB9OqxEaw6trcQjXwKQx1uKq0gNqhIqVFx94= +github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240716081244-6191389eafd5/go.mod h1:5T6w76ZiCHKm29X62vkZM4DfXZhMvs7VWzKGpI+itNc= github.com/openstack-k8s-operators/ironic-operator/api v0.4.1-0.20240710183123-6fe249a4f5c9 h1:/IVUjj4odR+UuRQ9pex8btZThLYdXoaSttrgHRW2eJs= github.com/openstack-k8s-operators/ironic-operator/api v0.4.1-0.20240710183123-6fe249a4f5c9/go.mod h1:2UuZV86J1RNXy04nPsGMtk3OPLaZzt68qG2y6ZgKAIg= github.com/openstack-k8s-operators/keystone-operator/api v0.4.1-0.20240715064816-751c29ed6ef1 h1:D3XPXH3qXSpLBwUInnnNfKcZ1IhmUlWqRyXGLbdCUTE= diff --git a/go.mod b/go.mod index 2f1a58e83..b09e3b7b9 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/openstack-k8s-operators/glance-operator/api v0.4.1-0.20240714131453-8248cbbd71b0 github.com/openstack-k8s-operators/heat-operator/api v0.4.1-0.20240715084339-1e2113ac57b7 github.com/openstack-k8s-operators/horizon-operator/api v0.4.1-0.20240715101812-a40a0701a6f4 - github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240715063952-446730fefacf + github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240716081244-6191389eafd5 github.com/openstack-k8s-operators/ironic-operator/api v0.4.1-0.20240710183123-6fe249a4f5c9 github.com/openstack-k8s-operators/keystone-operator/api v0.4.1-0.20240715064816-751c29ed6ef1 github.com/openstack-k8s-operators/lib-common/modules/ansible v0.4.1-0.20240715111029-358ba8041494 diff --git a/go.sum b/go.sum index 3737e1891..7c891c9ff 100644 --- a/go.sum +++ b/go.sum @@ -110,8 +110,8 @@ github.com/openstack-k8s-operators/heat-operator/api v0.4.1-0.20240715084339-1e2 github.com/openstack-k8s-operators/heat-operator/api v0.4.1-0.20240715084339-1e2113ac57b7/go.mod h1:VdfRsAYy7pAPH71QM5DPWk9t/Y+l+w+7n651zm9XnvU= github.com/openstack-k8s-operators/horizon-operator/api v0.4.1-0.20240715101812-a40a0701a6f4 h1:0tV5GXv9vmJdybZQxq15MQP+rO1LKNrdv92z2ra0+tg= github.com/openstack-k8s-operators/horizon-operator/api v0.4.1-0.20240715101812-a40a0701a6f4/go.mod h1:DEG5iAP0G1LhWnszDJ3e63EAgVi+xqKLHwCnQVajJbA= -github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240715063952-446730fefacf h1:ArEuU0sTGh/DcPrkfAzlcGQe/f926uv64SwMJiVL0qs= -github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240715063952-446730fefacf/go.mod h1:Dyp7Ug8BetGwy/Xl3fy55+A1wU0KuFZ6mFaKLecbHwM= +github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240716081244-6191389eafd5 h1:GukL/mPmB9OqxEaw6trcQjXwKQx1uKq0gNqhIqVFx94= +github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240716081244-6191389eafd5/go.mod h1:5T6w76ZiCHKm29X62vkZM4DfXZhMvs7VWzKGpI+itNc= github.com/openstack-k8s-operators/ironic-operator/api v0.4.1-0.20240710183123-6fe249a4f5c9 h1:/IVUjj4odR+UuRQ9pex8btZThLYdXoaSttrgHRW2eJs= github.com/openstack-k8s-operators/ironic-operator/api v0.4.1-0.20240710183123-6fe249a4f5c9/go.mod h1:2UuZV86J1RNXy04nPsGMtk3OPLaZzt68qG2y6ZgKAIg= github.com/openstack-k8s-operators/keystone-operator/api v0.4.1-0.20240715064816-751c29ed6ef1 h1:D3XPXH3qXSpLBwUInnnNfKcZ1IhmUlWqRyXGLbdCUTE= 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"), + ) + }) })