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

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

Signed-off-by: Martin Schuppert <[email protected]>
(cherry picked from commit 7d471da)
  • Loading branch information
@stuggi
stuggi committed Sep 13, 2024
1 parent 931d948 commit 7541df6
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 20 deletions.
4 changes: 2 additions & 2 deletions api/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions api/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
10 changes: 5 additions & 5 deletions api/v1beta1/common_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
60 changes: 57 additions & 3 deletions api/v1beta1/glance_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 -
Expand All @@ -39,7 +41,7 @@ type GlanceDefaults struct {
DBPurgeSchedule string
CleanerSchedule string
PrunerSchedule string
APITimeout int
APITimeout int
}

var glanceDefaults GlanceDefaults
Expand Down Expand Up @@ -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...)
}
Expand Down Expand Up @@ -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...)
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
)

Expand Down
8 changes: 4 additions & 4 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
64 changes: 64 additions & 0 deletions test/functional/validation_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
)
})
})

0 comments on commit 7541df6

Please sign in to comment.