From 7541df6fe22bd252ddfb1e361ac5de01ca22edea Mon Sep 17 00:00:00 2001 From: Martin Schuppert Date: Wed, 17 Jul 2024 12:39:00 +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: https://github.com/openstack-k8s-operators/lib-common/pull/562 Jira: https://issues.redhat.com/browse/OSPRH-8063 Signed-off-by: Martin Schuppert (cherry picked from commit 7d471da0bed691acd28c72c16b776a61c447a2dd) --- api/go.mod | 4 +- api/go.sum | 8 +-- api/v1beta1/common_types.go | 10 ++-- api/v1beta1/glance_webhook.go | 60 +++++++++++++++++++- go.mod | 4 +- go.sum | 8 +-- test/functional/validation_webhook_test.go | 64 ++++++++++++++++++++++ 7 files changed, 138 insertions(+), 20 deletions(-) diff --git a/api/go.mod b/api/go.mod index 92621386..28ade387 100644 --- a/api/go.mod +++ b/api/go.mod @@ -4,7 +4,7 @@ go 1.20 require ( github.com/google/go-cmp v0.6.0 - github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240730154700-e526dc22c2bf + github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240913084932-eb5ec1aa13b5 github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240730154700-e526dc22c2bf k8s.io/api v0.28.11 k8s.io/apimachinery v0.28.11 @@ -62,7 +62,7 @@ require ( k8s.io/component-base v0.28.11 // indirect k8s.io/klog/v2 v2.120.1 // indirect k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect - k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 // indirect + k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect sigs.k8s.io/yaml v1.4.0 // indirect diff --git a/api/go.sum b/api/go.sum index b32d643c..788a0fb0 100644 --- a/api/go.sum +++ b/api/go.sum @@ -67,8 +67,8 @@ github.com/onsi/ginkgo/v2 v2.19.0 h1:9Cnnf7UHo57Hy3k6/m5k3dRfGTMXGvxhHFvkDTCTpvA github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk= github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 h1:rncLxJBpFGqBztyxCMwNRnMjhhIDOWHJowi6q8G6koI= github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4= -github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240730154700-e526dc22c2bf h1:VDxaGely5T2NBt7HtCMYMMd7yR6w7tKrHFdQgAus/wY= -github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240730154700-e526dc22c2bf/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ= +github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240913084932-eb5ec1aa13b5 h1:1PCgFxZUaIlkAqBNgl18xtvnX0CdGZm1SYISKawu+oU= +github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240913084932-eb5ec1aa13b5/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ= github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240730154700-e526dc22c2bf h1:TdE6ccP5WWxv2UAWjOpugiGnwyeXk0NszqVdHmyZqXs= github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240730154700-e526dc22c2bf/go.mod h1:v9iFrR8J5fZACS9W5pZau/4lwyWs/YmO4ezpDeoEFKU= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= @@ -181,8 +181,8 @@ k8s.io/klog/v2 v2.120.1 h1:QXU6cPEOIslTGvZaXvFWiP9VKyeet3sawzTOvdXb4Vw= k8s.io/klog/v2 v2.120.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7FjZpUb45WallggurYhKGag= k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98= -k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 h1:jgGTlFYnhF1PM1Ax/lAlxUPE+KfCIXHaathvJg1C3ak= -k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= +k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 h1:pUdcCO1Lk/tbT5ztQWOBi5HBgbBP1J8+AsQnQCKsi8A= +k8s.io/utils v0.0.0-20240711033017-18e509b52bc8/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= sigs.k8s.io/controller-runtime v0.16.6 h1:FiXwTuFF5ZJKmozfP2Z0j7dh6kmxP4Ou1KLfxgKKC3I= sigs.k8s.io/controller-runtime v0.16.6/go.mod h1:+dQzkZxnylD0u49e0a+7AR+vlibEBaThmPca7lTyUsI= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= diff --git a/api/v1beta1/common_types.go b/api/v1beta1/common_types.go index 29613f7c..c8106ffc 100644 --- a/api/v1beta1/common_types.go +++ b/api/v1beta1/common_types.go @@ -148,11 +148,11 @@ func SetupDefaults() { // Acquire environmental defaults and initialize Glance defaults with them glanceDefaults := GlanceDefaults{ ContainerImageURL: util.GetEnvVar("RELATED_IMAGE_GLANCE_API_IMAGE_URL_DEFAULT", GlanceAPIContainerImage), - DBPurgeAge: DBPurgeDefaultAge, - DBPurgeSchedule: DBPurgeDefaultSchedule, - CleanerSchedule: CleanerDefaultSchedule, - PrunerSchedule: PrunerDefaultSchedule, - APITimeout: APIDefaultTimeout, + DBPurgeAge: DBPurgeDefaultAge, + DBPurgeSchedule: DBPurgeDefaultSchedule, + CleanerSchedule: CleanerDefaultSchedule, + PrunerSchedule: PrunerDefaultSchedule, + APITimeout: APIDefaultTimeout, } SetupGlanceDefaults(glanceDefaults) diff --git a/api/v1beta1/glance_webhook.go b/api/v1beta1/glance_webhook.go index 8286a954..dec604c1 100644 --- a/api/v1beta1/glance_webhook.go +++ b/api/v1beta1/glance_webhook.go @@ -30,6 +30,8 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + common_webhook "github.com/openstack-k8s-operators/lib-common/modules/common/webhook" ) // GlanceDefaults - @@ -39,7 +41,7 @@ type GlanceDefaults struct { DBPurgeSchedule string CleanerSchedule string PrunerSchedule string - APITimeout int + APITimeout int } var glanceDefaults GlanceDefaults @@ -204,6 +206,26 @@ func (r *Glance) ValidateCreate() (admission.Warnings, error) { glancelog.Info("validate create", "name", r.Name) var allErrs field.ErrorList basePath := field.NewPath("spec") + + for key, glanceAPI := range r.Spec.GlanceAPIs { + // Validate glanceapi name is valid + // GlanceAPI name is -- + // 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. + err := common_webhook.ValidateDNS1123Label( + basePath.Child("glanceAPIs"), + []string{key}, + GetCrMaxLengthCorrection(r.Name, glanceAPI.Type)) // omit issue with statefulset pod label "controller-revision-hash": "-" + allErrs = append(allErrs, err...) + } + if err := r.Spec.ValidateCreate(basePath); err != nil { allErrs = append(allErrs, err...) } @@ -275,6 +297,25 @@ func (r *Glance) ValidateUpdate(old runtime.Object) (admission.Warnings, error) var allErrs field.ErrorList basePath := field.NewPath("spec") + for key, glanceAPI := range r.Spec.GlanceAPIs { + // Validate glanceapi name is valid + // GlanceAPI name is -- + // 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. + err := common_webhook.ValidateDNS1123Label( + basePath.Child("glanceAPIs"), + []string{key}, + GetCrMaxLengthCorrection(r.Name, glanceAPI.Type)) // omit issue with statefulset pod label "controller-revision-hash": "-" + allErrs = append(allErrs, err...) + } + if err := r.Spec.ValidateUpdate(o.Spec, basePath); err != nil { allErrs = append(allErrs, err...) } @@ -357,12 +398,12 @@ func (glanceAPI *GlanceAPITemplate) SetDefaultRouteAnnotations(annotations map[s valHAProxy, okHAProxy := annotations[haProxyAnno] // Human operator set the HAProxy timeout manually - if (!okGlance && okHAProxy) { + if !okGlance && okHAProxy { return } // Human operator modified the HAProxy timeout manually without removing the Glance flag - if (okGlance && okHAProxy && valGlance != valHAProxy) { + if okGlance && okHAProxy && valGlance != valHAProxy { delete(annotations, glanceAnno) return } @@ -371,3 +412,16 @@ func (glanceAPI *GlanceAPITemplate) SetDefaultRouteAnnotations(annotations map[s annotations[glanceAnno] = timeout annotations[haProxyAnno] = timeout } + +// GetCrMaxLengthCorrection - get correction for ValidateDNS1123Label to get the real max string len of the glance API key +func GetCrMaxLengthCorrection(name string, apiType string) int { + // defaultCrMaxLengthCorrection - DNS1123LabelMaxLength (63) - CrMaxLengthCorrection used in validation to + // omit issue with statefulset pod label "controller-revision-hash": "-" + // Int32 is a 10 character + hyphen = 11 + defaultCrMaxLengthCorrection := 11 + + // GlanceAPI name is -- with this + // crMaxLengthCorrection = defaultCrMaxLengthCorrection + len() + "-" + "-" + len() + + return (defaultCrMaxLengthCorrection + len(name) + len(apiType) + 2) +} diff --git a/go.mod b/go.mod index b0e4161d..f2180127 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/openstack-k8s-operators/glance-operator/api v0.0.0-00010101000000-000000000000 github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240701051058-e23cdfd81161 github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240703053916-a2bf14020ef4 - github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240730154700-e526dc22c2bf + github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240913084932-eb5ec1aa13b5 github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240730154700-e526dc22c2bf github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240730154700-e526dc22c2bf github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240730154700-e526dc22c2bf @@ -23,7 +23,7 @@ require ( k8s.io/api v0.28.11 k8s.io/apimachinery v0.28.11 k8s.io/client-go v0.28.11 - k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 + k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 sigs.k8s.io/controller-runtime v0.16.6 ) diff --git a/go.sum b/go.sum index c547a6f3..4a037129 100644 --- a/go.sum +++ b/go.sum @@ -80,8 +80,8 @@ github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240701051058-e github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240701051058-e23cdfd81161/go.mod h1:gdc2zBX7iz+6vWjD2dhi+rtEmTulb91aybjU6bJyFVQ= github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240703053916-a2bf14020ef4 h1:MZqkT7o29H2zrCdWFo7ZgmpipCRO0HUvIgUTv9jus2s= github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240703053916-a2bf14020ef4/go.mod h1:Pg+s5VIUvZNec600X7GtlGTAUD2vafi9GZ7V0guaujI= -github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240730154700-e526dc22c2bf h1:VDxaGely5T2NBt7HtCMYMMd7yR6w7tKrHFdQgAus/wY= -github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240730154700-e526dc22c2bf/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ= +github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240913084932-eb5ec1aa13b5 h1:1PCgFxZUaIlkAqBNgl18xtvnX0CdGZm1SYISKawu+oU= +github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240913084932-eb5ec1aa13b5/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240730154700-e526dc22c2bf h1:5IxujAEi+Fk3DyOY83CegcNe+Lcqg/RwEPaPEhQiQ3E= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240730154700-e526dc22c2bf/go.mod h1:zuPcZ5Kopr15AdfxvA0xqKIIGCZ0XbSe/0VHNKuvbEE= github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240730154700-e526dc22c2bf h1:TdE6ccP5WWxv2UAWjOpugiGnwyeXk0NszqVdHmyZqXs= @@ -208,8 +208,8 @@ k8s.io/klog/v2 v2.120.1 h1:QXU6cPEOIslTGvZaXvFWiP9VKyeet3sawzTOvdXb4Vw= k8s.io/klog/v2 v2.120.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7FjZpUb45WallggurYhKGag= k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98= -k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 h1:jgGTlFYnhF1PM1Ax/lAlxUPE+KfCIXHaathvJg1C3ak= -k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= +k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 h1:pUdcCO1Lk/tbT5ztQWOBi5HBgbBP1J8+AsQnQCKsi8A= +k8s.io/utils v0.0.0-20240711033017-18e509b52bc8/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= sigs.k8s.io/controller-runtime v0.16.6 h1:FiXwTuFF5ZJKmozfP2Z0j7dh6kmxP4Ou1KLfxgKKC3I= sigs.k8s.io/controller-runtime v0.16.6/go.mod h1:+dQzkZxnylD0u49e0a+7AR+vlibEBaThmPca7lTyUsI= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= diff --git a/test/functional/validation_webhook_test.go b/test/functional/validation_webhook_test.go index 8710435d..322a7e54 100644 --- a/test/functional/validation_webhook_test.go +++ b/test/functional/validation_webhook_test.go @@ -185,4 +185,68 @@ var _ = Describe("Glance validation", func() { "Invalid value: \"wrooong\": invalid endpoint type: wrooong"), ) }) + + It("webhooks reject the request - glanceAPI key too long", func() { + // GlanceEmptySpec is used to provide a standard Glance CR where no + // field is customized: we can inject our parameters to test webhooks + spec := GetGlanceDefaultSpec() + raw := map[string]interface{}{ + "apiVersion": "glance.openstack.org/v1beta1", + "kind": "Glance", + "metadata": map[string]interface{}{ + "name": glanceTest.Instance.Name, + "namespace": glanceTest.Instance.Namespace, + }, + "spec": spec, + } + + apiList := map[string]interface{}{ + "foo-1234567890-1234567890-1234567890-1234567890-1234567890": GetDefaultGlanceAPISpec(GlanceAPITypeSingle), + } + spec["glanceAPIs"] = apiList + + unstructuredObj := &unstructured.Unstructured{Object: raw} + _, err := controllerutil.CreateOrPatch( + ctx, 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.Message).To( + ContainSubstring( + "Invalid value: \"foo-1234567890-1234567890-1234567890-1234567890-1234567890\": must be no more than 32 characters"), + ) + }) + + It("webhooks reject the request - glanceAPI wrong key/name", func() { + // GlanceEmptySpec is used to provide a standard Glance CR where no + // field is customized: we can inject our parameters to test webhooks + spec := GetGlanceDefaultSpec() + raw := map[string]interface{}{ + "apiVersion": "glance.openstack.org/v1beta1", + "kind": "Glance", + "metadata": map[string]interface{}{ + "name": glanceTest.Instance.Name, + "namespace": glanceTest.Instance.Namespace, + }, + "spec": spec, + } + + apiList := map[string]interface{}{ + "foo_bar": GetDefaultGlanceAPISpec(GlanceAPITypeSingle), + } + spec["glanceAPIs"] = apiList + + unstructuredObj := &unstructured.Unstructured{Object: raw} + _, err := controllerutil.CreateOrPatch( + ctx, 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.Message).To( + ContainSubstring( + "Invalid value: \"foo_bar\": a lowercase RFC 1123 label must consist of lower case alphanumeric characters"), + ) + }) })