Skip to content

Commit

Permalink
[validation] check memcached instance names are valid
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.

Also the name of the created memcached instance must match a lowercase
RFC 1123.

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

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

Signed-off-by: Martin Schuppert <[email protected]>
  • Loading branch information
@stuggi
stuggi committed Jul 16, 2024
1 parent 74299b8 commit 869521e
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 12 deletions.
25 changes: 23 additions & 2 deletions apis/core/v1beta1/openstackcontrolplane_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ import (

keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1"
"github.com/openstack-k8s-operators/lib-common/modules/common/route"
common_webhook "github.com/openstack-k8s-operators/lib-common/modules/common/webhook"
mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1"
placementv1 "github.com/openstack-k8s-operators/placement-operator/api/v1beta1"
"golang.org/x/exp/maps"
"golang.org/x/exp/slices"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand All @@ -35,8 +38,6 @@ import (
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
"golang.org/x/exp/maps"
"golang.org/x/exp/slices"

barbicanv1 "github.com/openstack-k8s-operators/barbican-operator/api/v1beta1"
cinderv1 "github.com/openstack-k8s-operators/cinder-operator/api/v1beta1"
Expand Down Expand Up @@ -313,6 +314,16 @@ func (r *OpenStackControlPlane) ValidateCreateServices(basePath *field.Path) (ad
}
}

if r.Spec.Memcached.Enabled {
if r.Spec.Memcached.Templates != nil {
err := common_webhook.ValidateDNS1123Label(
basePath.Child("memcached").Child("template"),
maps.Keys(*r.Spec.Memcached.Templates),
11) // omit issue with statefulset pod label "controller-revision-hash": "<statefulset_name>-<hash>"
errors = append(errors, err...)
}
}

return warnings, errors
}

Expand Down Expand Up @@ -414,6 +425,16 @@ func (r *OpenStackControlPlane) ValidateUpdateServices(old OpenStackControlPlane
errors = append(errors, r.Spec.Designate.Template.ValidateUpdate(*old.Designate.Template, basePath.Child("designate").Child("template"))...)
}

if r.Spec.Memcached.Enabled {
if r.Spec.Memcached.Templates != nil {
err := common_webhook.ValidateDNS1123Label(
basePath.Child("memcached").Child("template"),
maps.Keys(*r.Spec.Memcached.Templates),
memcachedv1.CrMaxLengthCorrection) // omit issue with statefulset pod label "controller-revision-hash": "<statefulset_name>-<hash>"
errors = append(errors, err...)
}
}

return errors
}

Expand Down
4 changes: 3 additions & 1 deletion apis/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ require (
github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240710144010-493348a1e882
github.com/openstack-k8s-operators/ironic-operator/api v0.4.1-0.20240710183123-6fe249a4f5c9
github.com/openstack-k8s-operators/keystone-operator/api v0.4.1-0.20240710140425-fcaeeb81f955
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/storage v0.4.1-0.20240709153313-544a55696489
github.com/openstack-k8s-operators/manila-operator/api v0.4.1-0.20240714132839-ccd10a8e361b
github.com/openstack-k8s-operators/mariadb-operator/api v0.4.1-0.20240709192759-bdbac5dca561
Expand Down Expand Up @@ -116,3 +116,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.20240711055119-1e6b9b8d9bd0 //allow-merging

replace github.com/openstack-k8s-operators/infra-operator/apis => github.com/stuggi/infra-operator/apis v0.0.0-20240716062542-adedab0c37ad
8 changes: 4 additions & 4 deletions apis/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,12 @@ github.com/openstack-k8s-operators/heat-operator/api v0.4.1-0.20240709172131-7d5
github.com/openstack-k8s-operators/heat-operator/api v0.4.1-0.20240709172131-7d58c1869913/go.mod h1:J+o3PC1SVjAeoF0sH3bdmiTlLAAb9VYX2V13bDSFHNk=
github.com/openstack-k8s-operators/horizon-operator/api v0.4.1-0.20240711151742-add443e9e62c h1:N9JmcfK0yF0jOIsPtMNvNEFm+OCsIrtLqVvlCpGLmxA=
github.com/openstack-k8s-operators/horizon-operator/api v0.4.1-0.20240711151742-add443e9e62c/go.mod h1:nNz/9Z9eFlk85Z+D70DeCEWxtIb9FMicQqwOJDydoLE=
github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240710144010-493348a1e882 h1:nCa2Gh8iPtVJZjUblBx/PlZaHmE7WKrr6K0UUb96DZ0=
github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240710144010-493348a1e882/go.mod h1:b898pymkkD1qIsDGOu5BKndVU8t6QQ3TS4Qq4WbI7j4=
github.com/openstack-k8s-operators/ironic-operator/api v0.4.1-0.20240710183123-6fe249a4f5c9 h1:/IVUjj4odR+UuRQ9pex8btZThLYdXoaSttrgHRW2eJs=
github.com/openstack-k8s-operators/ironic-operator/api v0.4.1-0.20240710183123-6fe249a4f5c9/go.mod h1:2UuZV86J1RNXy04nPsGMtk3OPLaZzt68qG2y6ZgKAIg=
github.com/openstack-k8s-operators/keystone-operator/api v0.4.1-0.20240710140425-fcaeeb81f955 h1:wMQ7do6ET1FR2CSXX+2ow8MCShhZ5RIeeFjwHLPCfTY=
github.com/openstack-k8s-operators/keystone-operator/api v0.4.1-0.20240710140425-fcaeeb81f955/go.mod h1:+hE3nW688Fq5FFWHUWmJO3bbP08zda4RDoPYxCxholo=
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/openstack v0.4.1-0.20240709153313-544a55696489 h1:KBs9djvTx4X7c8xjgbjFnRCV81PdSiuroXSiw4/EvNs=
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.4.1-0.20240709153313-544a55696489/go.mod h1:zuPcZ5Kopr15AdfxvA0xqKIIGCZ0XbSe/0VHNKuvbEE=
github.com/openstack-k8s-operators/lib-common/modules/storage v0.4.1-0.20240709153313-544a55696489 h1:T66yBnW6vkgonrkRt9x0ruAUAA6EXkchP/UdUUokD9s=
Expand Down Expand Up @@ -160,6 +158,8 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stuggi/infra-operator/apis v0.0.0-20240716062542-adedab0c37ad h1:nXLkRDJWS2lHCFDdjdwDLjkSWBdJFCC1LozCCkHyyzI=
github.com/stuggi/infra-operator/apis v0.0.0-20240716062542-adedab0c37ad/go.mod h1:5T6w76ZiCHKm29X62vkZM4DfXZhMvs7VWzKGpI+itNc=
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.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
Expand Down
4 changes: 3 additions & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ require (
github.com/openstack-k8s-operators/keystone-operator/api v0.4.1-0.20240710140425-fcaeeb81f955
github.com/openstack-k8s-operators/lib-common/modules/ansible v0.4.1-0.20240709153313-544a55696489
github.com/openstack-k8s-operators/lib-common/modules/certmanager v0.4.1-0.20240709153313-544a55696489
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/storage v0.4.1-0.20240709153313-544a55696489
github.com/openstack-k8s-operators/lib-common/modules/test v0.4.1-0.20240709153313-544a55696489
github.com/openstack-k8s-operators/manila-operator/api v0.4.1-0.20240714132839-ccd10a8e361b
Expand Down Expand Up @@ -130,3 +130,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.20240711055119-1e6b9b8d9bd0 //allow-merging

replace github.com/openstack-k8s-operators/infra-operator/apis => github.com/stuggi/infra-operator/apis v0.0.0-20240716062542-adedab0c37ad
8 changes: 4 additions & 4 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,6 @@ github.com/openstack-k8s-operators/heat-operator/api v0.4.1-0.20240709172131-7d5
github.com/openstack-k8s-operators/heat-operator/api v0.4.1-0.20240709172131-7d58c1869913/go.mod h1:J+o3PC1SVjAeoF0sH3bdmiTlLAAb9VYX2V13bDSFHNk=
github.com/openstack-k8s-operators/horizon-operator/api v0.4.1-0.20240711151742-add443e9e62c h1:N9JmcfK0yF0jOIsPtMNvNEFm+OCsIrtLqVvlCpGLmxA=
github.com/openstack-k8s-operators/horizon-operator/api v0.4.1-0.20240711151742-add443e9e62c/go.mod h1:nNz/9Z9eFlk85Z+D70DeCEWxtIb9FMicQqwOJDydoLE=
github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240710144010-493348a1e882 h1:nCa2Gh8iPtVJZjUblBx/PlZaHmE7WKrr6K0UUb96DZ0=
github.com/openstack-k8s-operators/infra-operator/apis v0.4.1-0.20240710144010-493348a1e882/go.mod h1:b898pymkkD1qIsDGOu5BKndVU8t6QQ3TS4Qq4WbI7j4=
github.com/openstack-k8s-operators/ironic-operator/api v0.4.1-0.20240710183123-6fe249a4f5c9 h1:/IVUjj4odR+UuRQ9pex8btZThLYdXoaSttrgHRW2eJs=
github.com/openstack-k8s-operators/ironic-operator/api v0.4.1-0.20240710183123-6fe249a4f5c9/go.mod h1:2UuZV86J1RNXy04nPsGMtk3OPLaZzt68qG2y6ZgKAIg=
github.com/openstack-k8s-operators/keystone-operator/api v0.4.1-0.20240710140425-fcaeeb81f955 h1:wMQ7do6ET1FR2CSXX+2ow8MCShhZ5RIeeFjwHLPCfTY=
Expand All @@ -120,8 +118,8 @@ github.com/openstack-k8s-operators/lib-common/modules/ansible v0.4.1-0.202407091
github.com/openstack-k8s-operators/lib-common/modules/ansible v0.4.1-0.20240709153313-544a55696489/go.mod h1:tP+nxk95PisCKJaXE/an2igG9lluxuOVhdmV9WtkR2s=
github.com/openstack-k8s-operators/lib-common/modules/certmanager v0.4.1-0.20240709153313-544a55696489 h1:w5m4HtcehbbVsQbsBPJdfj8qAOSre7rzyobbRGMVtZ0=
github.com/openstack-k8s-operators/lib-common/modules/certmanager v0.4.1-0.20240709153313-544a55696489/go.mod h1:GooNi6hM78cOUMjhBy0fXsZIcDTK1dUb1rlay170IJo=
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/openstack v0.4.1-0.20240709153313-544a55696489 h1:KBs9djvTx4X7c8xjgbjFnRCV81PdSiuroXSiw4/EvNs=
github.com/openstack-k8s-operators/lib-common/modules/openstack v0.4.1-0.20240709153313-544a55696489/go.mod h1:zuPcZ5Kopr15AdfxvA0xqKIIGCZ0XbSe/0VHNKuvbEE=
github.com/openstack-k8s-operators/lib-common/modules/storage v0.4.1-0.20240709153313-544a55696489 h1:T66yBnW6vkgonrkRt9x0ruAUAA6EXkchP/UdUUokD9s=
Expand Down Expand Up @@ -183,6 +181,8 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stuggi/infra-operator/apis v0.0.0-20240716062542-adedab0c37ad h1:nXLkRDJWS2lHCFDdjdwDLjkSWBdJFCC1LozCCkHyyzI=
github.com/stuggi/infra-operator/apis v0.0.0-20240716062542-adedab0c37ad/go.mod h1:5T6w76ZiCHKm29X62vkZM4DfXZhMvs7VWzKGpI+itNc=
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.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
Expand Down
74 changes: 74 additions & 0 deletions tests/functional/ctlplane/openstackoperator_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1863,4 +1863,78 @@ var _ = Describe("OpenStackOperator Webhook", func() {
),
)
})

It("Blocks creating ctlplane CRs with to long memcached keys/names", func() {
spec := GetDefaultOpenStackControlPlaneSpec()

memcachedTemplate := map[string]interface{}{
"foo-1234567890-1234567890-1234567890-1234567890-1234567890": map[string]interface{}{
"replicas": 1,
},
}

spec["memcached"] = map[string]interface{}{
"enabled": true,
"templates": memcachedTemplate,
}

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 52 characters"),
)
})

It("Blocks creating ctlplane CRs with wrong memcached keys/names", func() {
spec := GetDefaultOpenStackControlPlaneSpec()

memcachedTemplate := map[string]interface{}{
"foo_bar": map[string]interface{}{
"replicas": 1,
},
}

spec["memcached"] = map[string]interface{}{
"enabled": true,
"templates": memcachedTemplate,
}

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 869521e

Please sign in to comment.