Skip to content

Commit

Permalink
[memcached] add CR name length validation
Browse files Browse the repository at this point in the history
The memcached controller creates StatefulSet for the memcached
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. With this the max name
must not exceed 52 chars.

Depends-On: openstack-k8s-operators/lib-common#532

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

Signed-off-by: Martin Schuppert <[email protected]>
  • Loading branch information
stuggi committed Jul 16, 2024
1 parent 446730f commit adedab0
Show file tree
Hide file tree
Showing 9 changed files with 140 additions and 8 deletions.
2 changes: 1 addition & 1 deletion apis/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
github.com/go-logr/logr v1.4.2
github.com/onsi/ginkgo/v2 v2.19.0
github.com/onsi/gomega v1.33.1
github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240709153313-544a55696489
github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240715111029-358ba8041494
golang.org/x/exp v0.0.0-20240213143201-ec583247a57a
k8s.io/api v0.28.11
k8s.io/apimachinery v0.28.11
Expand Down
4 changes: 2 additions & 2 deletions apis/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk=
github.com/onsi/gomega v1.33.1/go.mod h1:U4R44UsT+9eLIaYRB2a5qajjtQYn0hauxvRm16AVYg0=
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.4.1-0.20240709153313-544a55696489 h1:4X/xF7z62rCHEui1yUVaE3/zdCleOcHsrsg6++oOrkY=
github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240709153313-544a55696489/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ=
github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240715111029-358ba8041494 h1:JZnRj8RIIsFpsJ5oonvqUEAKNDEkD9C3aeclu1V8JrA=
github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240715111029-358ba8041494/go.mod h1:s6ANiH9MoDBntRxnBzMP5fWBq2+NxFSl9CoXilGbLFY=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
Expand Down
5 changes: 5 additions & 0 deletions apis/memcached/v1beta1/memcached_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ const (

// MemcachedContainerImage is the fall-back container image for Memcached
MemcachedContainerImage = "quay.io/podified-antelope-centos9/openstack-memcached:current-podified"

// CrMaxLengthCorrection - 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
CrMaxLengthCorrection = 11
)

// MemcachedSpec defines the desired state of Memcached
Expand Down
22 changes: 20 additions & 2 deletions apis/memcached/v1beta1/memcached_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,16 @@ limitations under the License.
package v1beta1

import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
ctrl "sigs.k8s.io/controller-runtime"
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"
)

// MemcachedDefaults -
Expand Down Expand Up @@ -86,8 +91,21 @@ var _ webhook.Validator = &Memcached{}
func (r *Memcached) ValidateCreate() (admission.Warnings, error) {
memcachedlog.Info("validate create", "name", r.Name)

// TODO(user): fill in your validation logic upon object creation.
return nil, nil
var allErrs field.ErrorList
var allWarn []string

allErrs = common_webhook.ValidateDNS1123Label(
field.NewPath("metadata").Child("name"),
[]string{r.Name},
CrMaxLengthCorrection) // omit issue with statefulset pod label "controller-revision-hash": "<statefulset_name>-<hash>"

if len(allErrs) != 0 {
return allWarn, apierrors.NewInvalid(
schema.GroupKind{Group: "memcached.openstack.org", Kind: "Memcached"},
r.Name, allErrs)
}

return allWarn, nil
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
github.com/onsi/ginkgo/v2 v2.19.0
github.com/onsi/gomega v1.33.1
github.com/openstack-k8s-operators/infra-operator/apis v0.3.0
github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240709153313-544a55696489
github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240715111029-358ba8041494
github.com/openstack-k8s-operators/lib-common/modules/test v0.4.1-0.20240709153313-544a55696489
github.com/rabbitmq/cluster-operator/v2 v2.9.0
go.uber.org/zap v1.27.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk=
github.com/onsi/gomega v1.33.1/go.mod h1:U4R44UsT+9eLIaYRB2a5qajjtQYn0hauxvRm16AVYg0=
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.4.1-0.20240709153313-544a55696489 h1:4X/xF7z62rCHEui1yUVaE3/zdCleOcHsrsg6++oOrkY=
github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240709153313-544a55696489/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ=
github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240715111029-358ba8041494 h1:JZnRj8RIIsFpsJ5oonvqUEAKNDEkD9C3aeclu1V8JrA=
github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240715111029-358ba8041494/go.mod h1:s6ANiH9MoDBntRxnBzMP5fWBq2+NxFSl9CoXilGbLFY=
github.com/openstack-k8s-operators/lib-common/modules/test v0.4.1-0.20240709153313-544a55696489 h1:3yhphtEQ55lNxiIrZze+Fgy6g/s68sUB5MZMnJyH5zU=
github.com/openstack-k8s-operators/lib-common/modules/test v0.4.1-0.20240709153313-544a55696489/go.mod h1:0h76CxD9g0z2Hk7fGFOZcjnzT1tQQ/yRNv3OXng+S/A=
github.com/openstack-k8s-operators/rabbitmq-cluster-operator/v2 v2.6.1-0.20240711055119-1e6b9b8d9bd0 h1:xtcdlh6o8xhzrv3BvEK79Y8b/CJhBZvp6Fqt8TBV6RA=
Expand Down
31 changes: 31 additions & 0 deletions tests/functional/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

memcachedv1 "github.com/openstack-k8s-operators/infra-operator/apis/memcached/v1beta1"
networkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1"
condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
rabbitmqclusterv2 "github.com/rabbitmq/cluster-operator/v2/api/v1beta1"
Expand Down Expand Up @@ -652,3 +653,33 @@ func GetReservationFromNet(ipsetName types.NamespacedName, netName string) netwo

return res
}

func CreateMemcachedConfig(namespace string, spec map[string]interface{}) client.Object {
name := uuid.New().String()

raw := map[string]interface{}{
"apiVersion": "memcached.openstack.org/v1beta1",
"kind": "Memcached",
"metadata": map[string]interface{}{
"name": name,
"namespace": namespace,
},
"spec": spec,
}

return th.CreateUnstructured(raw)
}

func GetDefaultMemcachedSpec() map[string]interface{} {
return map[string]interface{}{
"replicas": 1,
}
}

func GetMemcached(name types.NamespacedName) *memcachedv1.Memcached {
instance := &memcachedv1.Memcached{}
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(ctx, name, instance)).Should(Succeed())
}, timeout, interval).Should(Succeed())
return instance
}
65 changes: 65 additions & 0 deletions tests/functional/memcached_webhook_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
Copyright 2023.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package functional_test

import (
. "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports
. "github.com/onsi/gomega" //revive:disable:dot-imports
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/types"
)

var _ = Describe("Memcached webhook", func() {
var memcacheName types.NamespacedName

When("a default Memcache gets created", func() {
BeforeEach(func() {
memcache := CreateMemcachedConfig(namespace, GetDefaultMemcachedSpec())
memcacheName.Name = memcache.GetName()
memcacheName.Namespace = memcache.GetNamespace()
DeferCleanup(th.DeleteInstance, memcache)
})

It("should have created a Memcache", func() {
Eventually(func(_ Gomega) {
GetMemcached(memcacheName)
}, timeout, interval).Should(Succeed())
})
})

When("a Memcache gets created with a name longer then 52 chars", func() {
It("gets blocked by the webhook and fail", func() {

raw := map[string]interface{}{
"apiVersion": "memcached.openstack.org/v1beta1",
"kind": "Memcached",
"metadata": map[string]interface{}{
"name": "foo-1234567890-1234567890-1234567890-1234567890-1234567890",
"namespace": namespace,
},
"spec": GetDefaultMemcachedSpec(),
}

unstructuredObj := &unstructured.Unstructured{Object: raw}
_, err := controllerutil.CreateOrPatch(
th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil })
Expect(err).To(HaveOccurred())
})
})
})
13 changes: 13 additions & 0 deletions tests/functional/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@ import (

metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"

memcachedv1 "github.com/openstack-k8s-operators/infra-operator/apis/memcached/v1beta1"
networkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1"
rabbitmqv1 "github.com/openstack-k8s-operators/infra-operator/apis/rabbitmq/v1beta1"
rabbitmqclusterv2 "github.com/rabbitmq/cluster-operator/v2/api/v1beta1"

memcached_ctrl "github.com/openstack-k8s-operators/infra-operator/controllers/memcached"
network_ctrl "github.com/openstack-k8s-operators/infra-operator/controllers/network"
rabbitmq_ctrl "github.com/openstack-k8s-operators/infra-operator/controllers/rabbitmq"

Expand Down Expand Up @@ -114,6 +116,8 @@ var _ = BeforeSuite(func() {
Expect(err).NotTo(HaveOccurred())
err = rabbitmqclusterv2.AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred())
err = memcachedv1.AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred())
//+kubebuilder:scaffold:scheme

logger = ctrl.Log.WithName("---Test---")
Expand Down Expand Up @@ -159,6 +163,8 @@ var _ = BeforeSuite(func() {
Expect(err).NotTo(HaveOccurred())
err = (&networkv1.DNSMasq{}).SetupWebhookWithManager(k8sManager)
Expect(err).NotTo(HaveOccurred())
err = (&memcachedv1.Memcached{}).SetupWebhookWithManager(k8sManager)
Expect(err).NotTo(HaveOccurred())

err = (&network_ctrl.DNSMasqReconciler{
Client: k8sManager.GetClient(),
Expand Down Expand Up @@ -195,6 +201,13 @@ var _ = BeforeSuite(func() {
}).SetupWithManager(k8sManager)
Expect(err).ToNot(HaveOccurred())

err = (&memcached_ctrl.Reconciler{
Client: k8sManager.GetClient(),
Scheme: k8sManager.GetScheme(),
Kclient: kclient,
}).SetupWithManager(k8sManager)
Expect(err).ToNot(HaveOccurred())

go func() {
defer GinkgoRecover()
err = k8sManager.Start(ctx)
Expand Down

0 comments on commit adedab0

Please sign in to comment.