diff --git a/api/go.mod b/api/go.mod index f462f950..6d18e149 100644 --- a/api/go.mod +++ b/api/go.mod @@ -3,8 +3,9 @@ module github.com/openstack-k8s-operators/cinder-operator/api go 1.20 require ( - 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 + golang.org/x/exp v0.0.0-20240213143201-ec583247a57a k8s.io/api v0.28.11 k8s.io/apimachinery v0.28.11 sigs.k8s.io/controller-runtime v0.16.6 @@ -43,7 +44,6 @@ require ( github.com/prometheus/procfs v0.12.0 // indirect github.com/spf13/pflag v1.0.5 // indirect go.uber.org/goleak v1.3.0 // indirect - golang.org/x/exp v0.0.0-20240213143201-ec583247a57a // indirect golang.org/x/net v0.25.0 // indirect golang.org/x/oauth2 v0.16.0 // indirect golang.org/x/sys v0.20.0 // indirect @@ -61,7 +61,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 adae712f..b5776139 100644 --- a/api/go.sum +++ b/api/go.sum @@ -65,8 +65,8 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/onsi/ginkgo/v2 v2.19.0 h1:9Cnnf7UHo57Hy3k6/m5k3dRfGTMXGvxhHFvkDTCTpvA= github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk= -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= @@ -179,8 +179,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/cinder_webhook.go b/api/v1beta1/cinder_webhook.go index 19f066e2..e26a352d 100644 --- a/api/v1beta1/cinder_webhook.go +++ b/api/v1beta1/cinder_webhook.go @@ -24,6 +24,8 @@ package v1beta1 import ( "fmt" + "golang.org/x/exp/maps" + "github.com/openstack-k8s-operators/lib-common/modules/common/service" "github.com/openstack-k8s-operators/lib-common/modules/common/util" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -34,6 +36,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" ) // CinderDefaults - @@ -134,6 +138,24 @@ func (r *Cinder) ValidateCreate() (admission.Warnings, error) { var allErrs field.ErrorList basePath := field.NewPath("spec") + + // Validate cinderVolume name is valid + // CinderVolume name is -volume- + // The CinderVolume controller creates StatefulSet for volume service 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 cinder name and -volume-. So the + // max len also need to be reduced bye the length of those. + err := common_webhook.ValidateDNS1123Label( + basePath.Child("cinderVolumes"), + maps.Keys(r.Spec.CinderVolumes), + GetCrMaxLengthCorrection(r.Name)) // 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...) } @@ -183,6 +205,23 @@ func (r *Cinder) ValidateUpdate(old runtime.Object) (admission.Warnings, error) var allErrs field.ErrorList basePath := field.NewPath("spec") + // Validate cinderVolume name is valid + // CinderVolume name is -volume- + // The CinderVolume controller creates StatefulSet for volume service 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 cinder name and -volume-. So the + // max len also need to be reduced bye the length of those. + err := common_webhook.ValidateDNS1123Label( + basePath.Child("cinderVolumes"), + maps.Keys(r.Spec.CinderVolumes), + GetCrMaxLengthCorrection(r.Name)) // omit issue with statefulset pod label "controller-revision-hash": "-" + allErrs = append(allErrs, err...) + if err := r.Spec.ValidateUpdate(oldCinder.Spec, basePath); err != nil { allErrs = append(allErrs, err...) } @@ -239,12 +278,12 @@ func (spec *CinderSpecCore) SetDefaultRouteAnnotations(annotations map[string]st valHAProxy, okHAProxy := annotations[haProxyAnno] // Human operator set the HAProxy timeout manually - if (!okCinder && okHAProxy) { + if !okCinder && okHAProxy { return } // Human operator modified the HAProxy timeout manually without removing the Cinder flag - if (okCinder && okHAProxy && valCinder != valHAProxy) { + if okCinder && okHAProxy && valCinder != valHAProxy { delete(annotations, cinderAnno) return } @@ -253,3 +292,16 @@ func (spec *CinderSpecCore) SetDefaultRouteAnnotations(annotations map[string]st annotations[cinderAnno] = timeout annotations[haProxyAnno] = timeout } + +// GetCrMaxLengthCorrection - get correction for ValidateDNS1123Label to get the real max string len of the cinder volume key +func GetCrMaxLengthCorrection(name 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 + + // cinder volume name is -volume- with this + // crMaxLengthCorrection = defaultCrMaxLengthCorrection + len() + "-volume-" + + return (defaultCrMaxLengthCorrection + len(name) + 8) +} diff --git a/go.mod b/go.mod index ce56f483..444942b8 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/onsi/gomega v1.33.1 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/storage v0.3.1-0.20240730154700-e526dc22c2bf github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240624132705-6c8da3c0bbfd github.com/openstack-k8s-operators/mariadb-operator/api v0.3.1-0.20240626205513-5c175a95f408 @@ -18,7 +18,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 6baad1e1..4e6816d0 100644 --- a/go.sum +++ b/go.sum @@ -78,8 +78,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= @@ -206,8 +206,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/cinder_controller_test.go b/test/functional/cinder_controller_test.go index 7bd38c9c..e6ec7e9b 100644 --- a/test/functional/cinder_controller_test.go +++ b/test/functional/cinder_controller_test.go @@ -16,6 +16,7 @@ limitations under the License. package functional import ( + "errors" "fmt" "os" @@ -27,6 +28,7 @@ import ( . "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers" corev1 "k8s.io/api/core/v1" + k8s_errors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" @@ -790,4 +792,64 @@ var _ = Describe("Cinder Webhook", func() { "Invalid value: \"wrooong\": invalid endpoint type: wrooong"), ) }) + + It("webhooks reject the request - cinderVolume key too long", func() { + spec := GetDefaultCinderSpec() + raw := map[string]interface{}{ + "apiVersion": "cinder.openstack.org/v1beta1", + "kind": "Cinder", + "metadata": map[string]interface{}{ + "name": cinderTest.Instance.Name, + "namespace": cinderTest.Instance.Namespace, + }, + "spec": spec, + } + + volumeList := map[string]interface{}{ + "foo-1234567890-1234567890-1234567890-1234567890-1234567890": GetDefaultCinderVolumeSpec(), + } + spec["cinderVolumes"] = volumeList + + 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 - cinderVolume wrong key/name", func() { + spec := GetDefaultCinderSpec() + raw := map[string]interface{}{ + "apiVersion": "cinder.openstack.org/v1beta1", + "kind": "Cinder", + "metadata": map[string]interface{}{ + "name": cinderTest.Instance.Name, + "namespace": cinderTest.Instance.Namespace, + }, + "spec": spec, + } + + volumeList := map[string]interface{}{ + "foo_bar": GetDefaultCinderVolumeSpec(), + } + spec["cinderVolumes"] = volumeList + + 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"), + ) + }) })