Skip to content

Commit

Permalink
[validation] check glanceAPI names are valid
Browse files Browse the repository at this point in the history 
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#562
Depends-On: openstack-k8s-operators/glance-operator#622

Jira: https://issues.redhat.com/browse/OSPRH-8063

Signed-off-by: Martin Schuppert <[email protected]>
(cherry picked from commit c0b082c)
  • Loading branch information
@stuggi
stuggi committed Sep 16, 2024
1 parent d63bfd9 commit 12c0693
Show file tree
Hide file tree
Showing 8 changed files with 173 additions and 7 deletions.
16 changes: 16 additions & 0 deletions apis/core/v1beta1/openstackcontrolplane_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -973,3 +978,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
}
16 changes: 16 additions & 0 deletions apis/core/v1beta1/openstackcontrolplane_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"))...)
}

Expand Down Expand Up @@ -400,6 +408,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"))...)
}

Expand Down
2 changes: 1 addition & 1 deletion apis/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/openstack-k8s-operators/barbican-operator/api v0.0.0-20240731032705-4a6f7bdc7202
github.com/openstack-k8s-operators/cinder-operator/api v0.3.1-0.20240731063935-e70504866b5d
github.com/openstack-k8s-operators/designate-operator/api v0.0.0-20240731063935-7d1a5bfcadb6
github.com/openstack-k8s-operators/glance-operator/api v0.3.1-0.20240731084039-931d948c6ce3
github.com/openstack-k8s-operators/glance-operator/api v0.3.1-0.20240913114408-6cb58c0ce9cd
github.com/openstack-k8s-operators/heat-operator/api v0.3.1-0.20240731072333-75659acdc7fc
github.com/openstack-k8s-operators/horizon-operator/api v0.3.1-0.20240731065850-f38ddc99c0af
github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240913164928-5ae3f1f5134a
Expand Down
2 changes: 2 additions & 0 deletions apis/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ github.com/openstack-k8s-operators/designate-operator/api v0.0.0-20240731063935-
github.com/openstack-k8s-operators/designate-operator/api v0.0.0-20240731063935-7d1a5bfcadb6/go.mod h1:HV4kdO67NTxjQvAiC+TUSnaJRW7IAS1dSXOAwGF6snA=
github.com/openstack-k8s-operators/glance-operator/api v0.3.1-0.20240731084039-931d948c6ce3 h1:W6nY8DeafgFBOjryLQaUMiaa6KV5UZoBd+SV78nJ48Q=
github.com/openstack-k8s-operators/glance-operator/api v0.3.1-0.20240731084039-931d948c6ce3/go.mod h1:Yoe5Jvp6GO3AfaJ6NykkfSE4B5mTYQg73Y3klmnxm0I=
github.com/openstack-k8s-operators/glance-operator/api v0.3.1-0.20240913114408-6cb58c0ce9cd h1:iw+bSSiJIvXSO1zW/V9jueKfp1QbtmgnVHYOtIfqzv0=
github.com/openstack-k8s-operators/glance-operator/api v0.3.1-0.20240913114408-6cb58c0ce9cd/go.mod h1:knSh0sVIBUU2gTZeKcn6MqQsfokbc/EfO4uXrg9MuZQ=
github.com/openstack-k8s-operators/heat-operator/api v0.3.1-0.20240731072333-75659acdc7fc h1:MlDw/dfOBSVZqk5hXYiD2NPnghEa9dRfKcQmoQHHl1w=
github.com/openstack-k8s-operators/heat-operator/api v0.3.1-0.20240731072333-75659acdc7fc/go.mod h1:XlhxSa3x7u2/q5sFwbv6OLDBf9A/pBOhVYxbkEhvKvs=
github.com/openstack-k8s-operators/horizon-operator/api v0.3.1-0.20240731065850-f38ddc99c0af h1:SgwslYuKikEO3s7QjmX5dJmhhYlB8NFO32wYEEawlvU=
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/openstack-k8s-operators/barbican-operator/api v0.0.0-20240731032705-4a6f7bdc7202
github.com/openstack-k8s-operators/cinder-operator/api v0.3.1-0.20240731063935-e70504866b5d
github.com/openstack-k8s-operators/designate-operator/api v0.0.0-20240731063935-7d1a5bfcadb6
github.com/openstack-k8s-operators/glance-operator/api v0.3.1-0.20240731084039-931d948c6ce3
github.com/openstack-k8s-operators/glance-operator/api v0.3.1-0.20240913114408-6cb58c0ce9cd
github.com/openstack-k8s-operators/heat-operator/api v0.3.1-0.20240731072333-75659acdc7fc
github.com/openstack-k8s-operators/horizon-operator/api v0.3.1-0.20240731065850-f38ddc99c0af
github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240913164928-5ae3f1f5134a
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ github.com/openstack-k8s-operators/designate-operator/api v0.0.0-20240731063935-
github.com/openstack-k8s-operators/designate-operator/api v0.0.0-20240731063935-7d1a5bfcadb6/go.mod h1:HV4kdO67NTxjQvAiC+TUSnaJRW7IAS1dSXOAwGF6snA=
github.com/openstack-k8s-operators/glance-operator/api v0.3.1-0.20240731084039-931d948c6ce3 h1:W6nY8DeafgFBOjryLQaUMiaa6KV5UZoBd+SV78nJ48Q=
github.com/openstack-k8s-operators/glance-operator/api v0.3.1-0.20240731084039-931d948c6ce3/go.mod h1:Yoe5Jvp6GO3AfaJ6NykkfSE4B5mTYQg73Y3klmnxm0I=
github.com/openstack-k8s-operators/glance-operator/api v0.3.1-0.20240913114408-6cb58c0ce9cd h1:iw+bSSiJIvXSO1zW/V9jueKfp1QbtmgnVHYOtIfqzv0=
github.com/openstack-k8s-operators/glance-operator/api v0.3.1-0.20240913114408-6cb58c0ce9cd/go.mod h1:knSh0sVIBUU2gTZeKcn6MqQsfokbc/EfO4uXrg9MuZQ=
github.com/openstack-k8s-operators/heat-operator/api v0.3.1-0.20240731072333-75659acdc7fc h1:MlDw/dfOBSVZqk5hXYiD2NPnghEa9dRfKcQmoQHHl1w=
github.com/openstack-k8s-operators/heat-operator/api v0.3.1-0.20240731072333-75659acdc7fc/go.mod h1:XlhxSa3x7u2/q5sFwbv6OLDBf9A/pBOhVYxbkEhvKvs=
github.com/openstack-k8s-operators/horizon-operator/api v0.3.1-0.20240731065850-f38ddc99c0af h1:SgwslYuKikEO3s7QjmX5dJmhhYlB8NFO32wYEEawlvU=
Expand Down
6 changes: 1 addition & 5 deletions pkg/openstack/glance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
134 changes: 134 additions & 0 deletions tests/functional/ctlplane/openstackoperator_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2085,4 +2085,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"),
)
})
})

0 comments on commit 12c0693

Please sign in to comment.