From dd038cce0a560af2dce28bceb9a988d43166a4f5 Mon Sep 17 00:00:00 2001 From: Martin Schuppert Date: Thu, 18 Jul 2024 12:56:46 +0200 Subject: [PATCH] [validation] check glanceAPI names are valid The glanceAPI controller creates StatefulSet for glanceapi 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. Which results in a default statefulset max len of 52 chars. The statefulset name also contain the glance name and the glanceAPI type + 2 hyphens. So the max len also need to be reduced bye the length of those. Also the name of the created rabbitmq instance must match a lowercase RFC 1123. Depends-On: openstack-k8s-operators/lib-common#532 Depends-On: https://github.com/openstack-k8s-operators/glance-operator/pull/587 Jira: https://issues.redhat.com/browse/OSPRH-8063 Signed-off-by: Martin Schuppert --- .../v1beta1/openstackcontrolplane_types.go | 14 ++ .../v1beta1/openstackcontrolplane_webhook.go | 16 +++ apis/go.mod | 2 + apis/go.sum | 4 +- go.mod | 2 + go.sum | 4 +- .../openstackoperator_controller_test.go | 134 ++++++++++++++++++ 7 files changed, 172 insertions(+), 4 deletions(-) diff --git a/apis/core/v1beta1/openstackcontrolplane_types.go b/apis/core/v1beta1/openstackcontrolplane_types.go index 01bb67a5f..6b87c1fdf 100644 --- a/apis/core/v1beta1/openstackcontrolplane_types.go +++ b/apis/core/v1beta1/openstackcontrolplane_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta1 import ( + "fmt" + barbicanv1 "github.com/openstack-k8s-operators/barbican-operator/api/v1beta1" cinderv1 "github.com/openstack-k8s-operators/cinder-operator/api/v1beta1" designatev1 "github.com/openstack-k8s-operators/designate-operator/api/v1beta1" @@ -974,3 +976,15 @@ func (c CertConfig) GetRenewBeforeHours() string { return "" } + +// GetGlanceName - returns the glanceName and altGlanceName depending if +// UniquePodNames is configured +func (instance OpenStackControlPlane) GetGlanceName() (string, string) { + glanceName := "glance" + altGlanceName := fmt.Sprintf("glance-%s", instance.UID[:5]) + if instance.Spec.Glance.UniquePodNames { + glanceName, altGlanceName = altGlanceName, glanceName + } + + return glanceName, altGlanceName +} diff --git a/apis/core/v1beta1/openstackcontrolplane_webhook.go b/apis/core/v1beta1/openstackcontrolplane_webhook.go index e93930548..295dd6b82 100644 --- a/apis/core/v1beta1/openstackcontrolplane_webhook.go +++ b/apis/core/v1beta1/openstackcontrolplane_webhook.go @@ -279,6 +279,14 @@ func (r *OpenStackControlPlane) ValidateCreateServices(basePath *field.Path) (ad } if r.Spec.Glance.Enabled { + glanceName, _ := r.GetGlanceName() + for key, glanceAPI := range r.Spec.Glance.Template.GlanceAPIs { + err := common_webhook.ValidateDNS1123Label( + basePath.Child("glance").Child("template").Child("glanceAPIs"), + []string{key}, + glancev1.GetCrMaxLengthCorrection(glanceName, glanceAPI.Type)) // omit issue with statefulset pod label "controller-revision-hash": "-" + errors = append(errors, err...) + } errors = append(errors, r.Spec.Glance.Template.ValidateCreate(basePath.Child("glance").Child("template"))...) } @@ -390,6 +398,14 @@ func (r *OpenStackControlPlane) ValidateUpdateServices(old OpenStackControlPlane if old.Glance.Template == nil { old.Glance.Template = &glancev1.GlanceSpecCore{} } + glanceName, _ := r.GetGlanceName() + for key, glanceAPI := range r.Spec.Glance.Template.GlanceAPIs { + err := common_webhook.ValidateDNS1123Label( + basePath.Child("glance").Child("template").Child("glanceAPIs"), + []string{key}, + glancev1.GetCrMaxLengthCorrection(glanceName, glanceAPI.Type)) // omit issue with statefulset pod label "controller-revision-hash": "-" + errors = append(errors, err...) + } errors = append(errors, r.Spec.Glance.Template.ValidateUpdate(*old.Glance.Template, basePath.Child("glance").Child("template"))...) } diff --git a/apis/go.mod b/apis/go.mod index 0d3b1da31..298eb620f 100644 --- a/apis/go.mod +++ b/apis/go.mod @@ -116,3 +116,5 @@ 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.20240711055119-1e6b9b8d9bd0 //allow-merging + +replace github.com/openstack-k8s-operators/glance-operator/api => github.com/stuggi/glance-operator/api v0.0.0-20240719082250-7d471da0bed6 diff --git a/apis/go.sum b/apis/go.sum index 399d40acf..21ac79fe1 100644 --- a/apis/go.sum +++ b/apis/go.sum @@ -96,8 +96,6 @@ github.com/openstack-k8s-operators/cinder-operator/api v0.4.1-0.20240709170501-8 github.com/openstack-k8s-operators/cinder-operator/api v0.4.1-0.20240709170501-8cdb9c68d2b2/go.mod h1:ALiHsN2QOlIMbeYT5zBT6y0T01qscpPqIlKoC4k2hf4= github.com/openstack-k8s-operators/designate-operator/api v0.1.1-0.20240715084339-770038b9b19c h1:6swKdKjtAOiCxm/Zn1r/zr+37QnqIDGIS4p3a4PJFyA= github.com/openstack-k8s-operators/designate-operator/api v0.1.1-0.20240715084339-770038b9b19c/go.mod h1:zEDytGmq4nHOsx/35YoYNxjGwCSg29/dUquVpTrWupQ= -github.com/openstack-k8s-operators/glance-operator/api v0.4.1-0.20240714131453-8248cbbd71b0 h1:0iXf/3mFndmXIQehVeQq1AXYqX6ueZn9wxOR9vaZIg4= -github.com/openstack-k8s-operators/glance-operator/api v0.4.1-0.20240714131453-8248cbbd71b0/go.mod h1:d5jIUULDakLoT0lM+wyR9ppdq/IrkWH97+mMufoDQcQ= github.com/openstack-k8s-operators/heat-operator/api v0.4.1-0.20240715084339-1e2113ac57b7 h1:uzSSMeDv/6izNbym2t2MUuUQ91/UN76yHA6blM08Rb8= 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= @@ -160,6 +158,8 @@ 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/glance-operator/api v0.0.0-20240719082250-7d471da0bed6 h1:TtMRbMveDZzggxp1BOYXDoQ2gylNB49soK+uTOw1wXg= +github.com/stuggi/glance-operator/api v0.0.0-20240719082250-7d471da0bed6/go.mod h1:7TSiTVXd2YruGwLJxqMyoqEzqilwi5PWcNqcGxt8AiA= 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 f69f7e628..3532439ce 100644 --- a/go.mod +++ b/go.mod @@ -130,3 +130,5 @@ 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.20240711055119-1e6b9b8d9bd0 //allow-merging + +replace github.com/openstack-k8s-operators/glance-operator/api => github.com/stuggi/glance-operator/api v0.0.0-20240719082250-7d471da0bed6 diff --git a/go.sum b/go.sum index 80246ed91..3eabed2f4 100644 --- a/go.sum +++ b/go.sum @@ -104,8 +104,6 @@ github.com/openstack-k8s-operators/cinder-operator/api v0.4.1-0.20240709170501-8 github.com/openstack-k8s-operators/cinder-operator/api v0.4.1-0.20240709170501-8cdb9c68d2b2/go.mod h1:ALiHsN2QOlIMbeYT5zBT6y0T01qscpPqIlKoC4k2hf4= github.com/openstack-k8s-operators/designate-operator/api v0.1.1-0.20240715084339-770038b9b19c h1:6swKdKjtAOiCxm/Zn1r/zr+37QnqIDGIS4p3a4PJFyA= github.com/openstack-k8s-operators/designate-operator/api v0.1.1-0.20240715084339-770038b9b19c/go.mod h1:zEDytGmq4nHOsx/35YoYNxjGwCSg29/dUquVpTrWupQ= -github.com/openstack-k8s-operators/glance-operator/api v0.4.1-0.20240714131453-8248cbbd71b0 h1:0iXf/3mFndmXIQehVeQq1AXYqX6ueZn9wxOR9vaZIg4= -github.com/openstack-k8s-operators/glance-operator/api v0.4.1-0.20240714131453-8248cbbd71b0/go.mod h1:d5jIUULDakLoT0lM+wyR9ppdq/IrkWH97+mMufoDQcQ= github.com/openstack-k8s-operators/heat-operator/api v0.4.1-0.20240715084339-1e2113ac57b7 h1:uzSSMeDv/6izNbym2t2MUuUQ91/UN76yHA6blM08Rb8= 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= @@ -183,6 +181,8 @@ 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/glance-operator/api v0.0.0-20240719082250-7d471da0bed6 h1:TtMRbMveDZzggxp1BOYXDoQ2gylNB49soK+uTOw1wXg= +github.com/stuggi/glance-operator/api v0.0.0-20240719082250-7d471da0bed6/go.mod h1:7TSiTVXd2YruGwLJxqMyoqEzqilwi5PWcNqcGxt8AiA= 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 5af882b9e..c6277f2b9 100644 --- a/tests/functional/ctlplane/openstackoperator_controller_test.go +++ b/tests/functional/ctlplane/openstackoperator_controller_test.go @@ -2011,4 +2011,138 @@ var _ = Describe("OpenStackOperator Webhook", func() { "Invalid value: \"foo_bar\": a lowercase RFC 1123 label must consist"), ) }) + + It("Blocks creating ctlplane CRs with to long glanceapi keys/names", func() { + spec := GetDefaultOpenStackControlPlaneSpec() + + apiList := map[string]interface{}{ + "foo-1234567890-1234567890-1234567890-1234567890-1234567890": map[string]interface{}{ + "replicas": 1, + }, + } + + glanceTemplate := map[string]interface{}{ + "databaseInstance": "openstack", + "secret": "secret", + "databaseAccount": "account", + "glanceAPIs": apiList, + } + + spec["glance"] = map[string]interface{}{ + "enabled": true, + "uniquePodNames": false, + "template": glanceTemplate, + } + + 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 39 characters"), + ) + }) + + It("Blocks creating ctlplane CRs with to long glanceapi keys/names (uniquePodNames)", func() { + spec := GetDefaultOpenStackControlPlaneSpec() + + apiList := map[string]interface{}{ + "foo-1234567890-1234567890-1234567890-1234567890-1234567890": map[string]interface{}{ + "replicas": 1, + }, + } + + glanceTemplate := map[string]interface{}{ + "databaseInstance": "openstack", + "secret": "secret", + "databaseAccount": "account", + "glanceAPIs": apiList, + } + + spec["glance"] = map[string]interface{}{ + "enabled": true, + "uniquePodNames": true, + "template": glanceTemplate, + } + + 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 33 characters"), + ) + }) + + It("Blocks creating ctlplane CRs with wrong glanceapi keys/names", func() { + spec := GetDefaultOpenStackControlPlaneSpec() + + apiList := map[string]interface{}{ + "foo_bar": map[string]interface{}{ + "replicas": 1, + }, + } + + glanceTemplate := map[string]interface{}{ + "databaseInstance": "openstack", + "secret": "secret", + "databaseAccount": "account", + "glanceAPIs": apiList, + } + + spec["glance"] = map[string]interface{}{ + "enabled": true, + "template": glanceTemplate, + } + + 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"), + ) + }) })