Merge pull request #622 from stuggi/8063-18.0.2

[18.0.0-proposed] [validation] check glanceAPI names are valid

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

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=

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)

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 <glance name>-<api name>-<api type>
+		// The glanceAPI controller creates StatefulSet for glanceapi to run.
+		// This adds a StatefulSet pod's label
+		// "controller-revision-hash": "<statefulset_name>-<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": "<statefulset_name>-<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 <glance name>-<api name>-<api type>
+		// The glanceAPI controller creates StatefulSet for glanceapi to run.
+		// This adds a StatefulSet pod's label
+		// "controller-revision-hash": "<statefulset_name>-<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": "<statefulset_name>-<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": "<statefulset_name>-<hash>"
+	// Int32 is a 10 character + hyphen = 11
+	defaultCrMaxLengthCorrection := 11
+
+	// GlanceAPI name is <glance name>-<api name>-<api type> with this
+	// crMaxLengthCorrection = defaultCrMaxLengthCorrection + len(<glance name>) + "-" + "-" + len(<api type>)
+
+	return (defaultCrMaxLengthCorrection + len(name) + len(apiType) + 2)
+}

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
 )
 

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=

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"),
+		)
+	})
 })