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 11, 2024
1 parent 493348a commit c894351
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 6 deletions.
2 changes: 2 additions & 0 deletions apis/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,5 @@ require (
)

replace github.com/openshift/api => github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 //allow-merging

replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/stuggi/lib-common/modules/common v0.0.0-20240711103924-a6adaedc7ed1
4 changes: 2 additions & 2 deletions apis/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ 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.20240709142659-5a0b4d5c6176 h1:K2/qVoIkgF48y580yjtW4Ciyc3WFZ8TLY41T8WbYnwg=
github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240709142659-5a0b4d5c6176/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ=
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 All @@ -112,6 +110,8 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stuggi/lib-common/modules/common v0.0.0-20240711103924-a6adaedc7ed1 h1:ZPLqxyJntbNhPYPyAnkTJvjDGgf6vA8aG/4fIm2+mMI=
github.com/stuggi/lib-common/modules/common v0.0.0-20240711103924-a6adaedc7ed1/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
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},
11) // 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: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,5 @@ replace github.com/openshift/api => github.com/openshift/api v0.0.0-202304141430

// custom RabbitmqClusterSpecCore for OpenStackControlplane (v2.6.0_patches_tag)
replace github.com/rabbitmq/cluster-operator/v2 => github.com/openstack-k8s-operators/rabbitmq-cluster-operator/v2 v2.6.1-0.20240626194327-e7df1b654cb7 //allow-merging

replace github.com/openstack-k8s-operators/lib-common/modules/common => github.com/stuggi/lib-common/modules/common v0.0.0-20240711103924-a6adaedc7ed1
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ 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.20240709142659-5a0b4d5c6176 h1:K2/qVoIkgF48y580yjtW4Ciyc3WFZ8TLY41T8WbYnwg=
github.com/openstack-k8s-operators/lib-common/modules/common v0.4.1-0.20240709142659-5a0b4d5c6176/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ=
github.com/openstack-k8s-operators/lib-common/modules/test v0.4.1-0.20240709142659-5a0b4d5c6176 h1:uNnIlMD0uYqmnoaiq24vp9T9qQ4cDS0LQV68aajw934=
github.com/openstack-k8s-operators/lib-common/modules/test v0.4.1-0.20240709142659-5a0b4d5c6176/go.mod h1:0h76CxD9g0z2Hk7fGFOZcjnzT1tQQ/yRNv3OXng+S/A=
github.com/openstack-k8s-operators/rabbitmq-cluster-operator/v2 v2.6.1-0.20240626194327-e7df1b654cb7 h1:PcirbCDZzVx75torXit86b7fph8+HKkO0qtOkyn/+C8=
Expand Down Expand Up @@ -116,6 +114,8 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stuggi/lib-common/modules/common v0.0.0-20240711103924-a6adaedc7ed1 h1:ZPLqxyJntbNhPYPyAnkTJvjDGgf6vA8aG/4fIm2+mMI=
github.com/stuggi/lib-common/modules/common v0.0.0-20240711103924-a6adaedc7ed1/go.mod h1:k9KuWN2LBtLbKHgcyh/0lrwk3Kr0cOAhiR3hi/mrwOQ=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
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 c894351

Please sign in to comment.