Skip to content

Commit

Permalink
[validation] check cinderVolume names are valid
Browse files Browse the repository at this point in the history 
CinderVolume name is <cinder name>-volume-<volume name>
The CinderVolume controller creates StatefulSet for volume service 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 cinder name and -volume-. So the max len also need to
be reduced bye the length of those.

Also the name of the created cinderVolume name 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 1b94c78)
  • Loading branch information
@stuggi
stuggi committed Sep 13, 2024
1 parent e705048 commit 5c1a068
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 17 deletions.
6 changes: 3 additions & 3 deletions api/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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-20240119083558-1b970713d09a
k8s.io/api v0.28.11
k8s.io/apimachinery v0.28.11
sigs.k8s.io/controller-runtime v0.16.6
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
12 changes: 6 additions & 6 deletions api/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -98,8 +98,8 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/exp v0.0.0-20240213143201-ec583247a57a h1:HinSgX1tJRX3KsL//Gxynpw5CTOAIPhgL4W8PNiIpVE=
golang.org/x/exp v0.0.0-20240213143201-ec583247a57a/go.mod h1:CxmFvTBINI24O/j8iY7H1xHzx2i4OsyguNBmN/uPtqc=
golang.org/x/exp v0.0.0-20240119083558-1b970713d09a h1:Q8/wZp0KX97QFTc2ywcOE0YRjZPVIx+MXInMzdvQqcA=
golang.org/x/exp v0.0.0-20240119083558-1b970713d09a/go.mod h1:idGWGoKP1toJGkd5/ig9ZLuPcZBC3ewk7SzmH0uou08=
golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
Expand Down Expand Up @@ -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=
Expand Down
56 changes: 54 additions & 2 deletions api/v1beta1/cinder_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 -
Expand Down Expand Up @@ -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 <cinder name>-volume-<volume name>
// The CinderVolume controller creates StatefulSet for volume service 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 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": "<statefulset_name>-<hash>"
allErrs = append(allErrs, err...)

if err := r.Spec.ValidateCreate(basePath); err != nil {
allErrs = append(allErrs, err...)
}
Expand Down Expand Up @@ -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 <cinder name>-volume-<volume name>
// The CinderVolume controller creates StatefulSet for volume service 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 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": "<statefulset_name>-<hash>"
allErrs = append(allErrs, err...)

if err := r.Spec.ValidateUpdate(oldCinder.Spec, basePath); err != nil {
allErrs = append(allErrs, err...)
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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": "<statefulset_name>-<hash>"
// Int32 is a 10 character + hyphen = 11
defaultCrMaxLengthCorrection := 11

// cinder volume name is <cinder name>-volume-<volume name> with this
// crMaxLengthCorrection = defaultCrMaxLengthCorrection + len(<cinder name>) + "-volume-"

return (defaultCrMaxLengthCorrection + len(name) + 8)
}
4 changes: 2 additions & 2 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ 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
golang.org/x/exp v0.0.0-20240213143201-ec583247a57a
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 @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
62 changes: 62 additions & 0 deletions test/functional/cinder_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.
package functional

import (
"errors"
"fmt"
"os"

Expand All @@ -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"
Expand Down Expand Up @@ -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"),
)
})
})

0 comments on commit 5c1a068

Please sign in to comment.