[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": "<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. Also the name of the created rabbitmq instance must match a lowercase RFC 1123. Depends-On: openstack-k8s-operators/lib-common#532 Depends-On: openstack-k8s-operators/glance-operator#587 Jira: https://issues.redhat.com/browse/OSPRH-8063 Signed-off-by: Martin Schuppert <[email protected]>
stuggi · Jul 23, 2024 · 7440130 · 7440130
1 parent d5a74fd
commit 7440130
Show file tree

Hide file tree

Showing 4 changed files with 167 additions and 5 deletions.
diff --git 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"
@@ -62,6 +64,9 @@ const (
 	OvnDbCaName = tls.DefaultCAPrefix + "ovn"
 	// LibvirtCaName -
 	LibvirtCaName = tls.DefaultCAPrefix + "libvirt"
+
+	// GlanceName - Default Glance name
+	GlanceName = "glance"
 )
 
 // OpenStackControlPlaneSpec defines the desired state of OpenStackControlPlane
@@ -974,3 +979,14 @@ func (c CertConfig) GetRenewBeforeHours() string {
 
 	return ""
 }
+
+// GetServiceName - returns the name and altName depending if
+// UniquePodNames is configured
+func (instance OpenStackControlPlane) GetServiceName(name string, uniquePodNames bool) (string, string) {
+	altName := fmt.Sprintf("%s-%s", name, instance.UID[:5])
+	if uniquePodNames {
+		name, altName = altName, name
+	}
+
+	return name, altName
+}
diff --git 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.GetServiceName(GlanceName, r.Spec.Glance.UniquePodNames)
+		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": "<statefulset_name>-<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.GetServiceName(GlanceName, r.Spec.Glance.UniquePodNames)
+		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": "<statefulset_name>-<hash>"
+			errors = append(errors, err...)
+		}
 		errors = append(errors, r.Spec.Glance.Template.ValidateUpdate(*old.Glance.Template, basePath.Child("glance").Child("template"))...)
 	}
 

diff --git a/pkg/openstack/glance.go b/pkg/openstack/glance.go
@@ -28,11 +28,7 @@ const (
 
 // ReconcileGlance -
 func ReconcileGlance(ctx context.Context, instance *corev1beta1.OpenStackControlPlane, version *corev1beta1.OpenStackVersion, helper *helper.Helper) (ctrl.Result, error) {
-	glanceName := "glance"
-	altGlanceName := fmt.Sprintf("glance-%s", instance.UID[:5])
-	if instance.Spec.Glance.UniquePodNames {
-		glanceName, altGlanceName = altGlanceName, glanceName
-	}
+	glanceName, altGlanceName := instance.GetServiceName(corev1beta1.GlanceName, instance.Spec.Glance.UniquePodNames)
 	// Ensure the alternate cinder CR doesn't exist, as the ramdomPodNames flag may have been toggled
 	glance := &glancev1.Glance{
 		ObjectMeta: metav1.ObjectMeta{

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