Skip to content

Commit e052e63

Browse files
fmountfmount
committed
Improve envTest for validation webhooks
This patch fixes the isValidEndpoint function provided by the validation webhook and introduces envTests for Validation webhooks, that are one of the most important part of the operator. Signed-off-by: Francesco Pantano <[email protected]>
1 parent 47e8a3c commit e052e63

File tree

5 files changed

+191
-28
lines changed

5 files changed

+191
-28
lines changed

‎api/v1beta1/glance_webhook.go‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,9 @@ func (r *Glance) Default() {
7272
// Check if the KeystoneEndpoint matches with a deployed glanceAPI
7373
func (spec *GlanceSpec) isValidKeystoneEP() bool {
7474
for name, api := range spec.GlanceAPIs {
75-
// If it's an Edge API is not a valid endpoint
76-
if api.Type == APIEdge {
77-
return false
78-
}
79-
if spec.KeystoneEndpoint == name {
75+
// A valid keystoneEndpoint can either be applied to
76+
// a single API or split type, but not to an EdgeAPI
77+
if api.Type != APIEdge && spec.KeystoneEndpoint == name {
8078
return true
8179
}
8280
}
@@ -184,11 +182,6 @@ var _ webhook.Validator = &Glance{}
184182
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
185183
func (r *Glance) ValidateCreate() (admission.Warnings, error) {
186184
glancelog.Info("validate create", "name", r.Name)
187-
// At creation time, if the CR has an invalid keystoneEndpoint value (that
188-
// doesn't match with any defined backend), return an error.
189-
if !r.Spec.isValidKeystoneEP() {
190-
return nil, errors.New("KeystoneEndpoint is assigned to an invalid glanceAPI instance")
191-
}
192185
// Check if the top-level CR has a "customServiceConfig" with an explicit
193186
// "backend:file || empty string" and save the result into topLevel var.
194187
// If it's empty it should be ignored and having a file backend depends
@@ -205,6 +198,11 @@ func (r *Glance) ValidateCreate() (admission.Warnings, error) {
205198
return nil, errors.New("Invalid backend configuration detected")
206199
}
207200
}
201+
// At creation time, if the CR has an invalid keystoneEndpoint value (that
202+
// doesn't match with any defined backend), return an error.
203+
if !r.Spec.isValidKeystoneEP() {
204+
return nil, errors.New("KeystoneEndpoint is assigned to an invalid glanceAPI instance")
205+
}
208206
return nil, nil
209207
}
210208

@@ -234,12 +232,13 @@ func (r *Glance) ValidateUpdate(old runtime.Object) (admission.Warnings, error)
234232
if r.isInvalidBackend(glanceAPI, topLevelFileBackend) {
235233
return nil, errors.New("Invalid backend configuration detected")
236234
}
237-
// At update time, if the CR has an invalid keystoneEndpoint set
238-
// (e.g. an Edge GlanceAPI instance that can't be registered in keystone)
239-
// return an error message
240-
if !r.Spec.isValidKeystoneEP() {
241-
return nil, errors.New("KeystoneEndpoint is assigned to an invalid glanceAPI instance")
242-
}
235+
}
236+
// At update time, if the CR has an invalid keystoneEndpoint set
237+
// (e.g. an Edge GlanceAPI instance that can't be registered in keystone)
238+
// return an error message
239+
if !r.Spec.isValidKeystoneEP() {
240+
return nil, errors.New(
241+
"KeystoneEndpoint is assigned to an invalid glanceAPI instance")
243242
}
244243
return nil, nil
245244
}

‎config/samples/layout/edge/glance_v1beta1_glance.yaml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,15 @@ spec:
1111
databaseAccount: glance
1212
keystoneEndpoint: central
1313
glanceAPIs:
14-
central:
15-
preserveJobs: false
16-
replicas: 1
17-
type: single
1814
edge1:
1915
preserveJobs: false
2016
replicas: 1
21-
type: single
17+
type: edge
2218
edge2:
19+
preserveJobs: false
20+
replicas: 1
21+
type: edge
22+
central:
2323
preserveJobs: false
2424
replicas: 1
2525
type: single

‎test/functional/base_test.go‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package functional
1818

1919
import (
20+
"fmt"
2021
"golang.org/x/exp/maps"
2122
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
2223
"k8s.io/apimachinery/pkg/types"
@@ -169,6 +170,7 @@ func CreateGlanceSecret(namespace string, name string) *corev1.Secret {
169170
)
170171
}
171172

173+
// GetDefaultGlanceSpec - It returns a default API built for testing purposes
172174
func GetDefaultGlanceSpec() map[string]interface{} {
173175
return map[string]interface{}{
174176
"databaseInstance": "openstack",
@@ -178,6 +180,7 @@ func GetDefaultGlanceSpec() map[string]interface{} {
178180
}
179181
}
180182

183+
// CreateGlanceAPISpec -
181184
func CreateGlanceAPISpec(apiType APIType) map[string]interface{} {
182185
spec := map[string]interface{}{
183186
"replicas": 1,
@@ -193,6 +196,7 @@ func CreateGlanceAPISpec(apiType APIType) map[string]interface{} {
193196
return spec
194197
}
195198

199+
// GetDefaultGlanceAPISpec -
196200
func GetDefaultGlanceAPISpec(apiType APIType) map[string]interface{} {
197201
spec := map[string]interface{}{
198202
"replicas": 1,
@@ -208,6 +212,7 @@ func GetDefaultGlanceAPISpec(apiType APIType) map[string]interface{} {
208212
return spec
209213
}
210214

215+
// GetTLSGlanceAPISpec -
211216
func GetTLSGlanceAPISpec(apiType APIType) map[string]interface{} {
212217
spec := CreateGlanceAPISpec(apiType)
213218
maps.Copy(spec, map[string]interface{}{
@@ -229,6 +234,7 @@ func GetTLSGlanceAPISpec(apiType APIType) map[string]interface{} {
229234
return spec
230235
}
231236

237+
// GlanceAPINotExists - Asserts the GlanceAPI does not exist
232238
func GlanceAPINotExists(name types.NamespacedName) {
233239
Consistently(func(g Gomega) {
234240
instance := &glancev1.GlanceAPI{}
@@ -237,6 +243,7 @@ func GlanceAPINotExists(name types.NamespacedName) {
237243
}, timeout, interval).Should(Succeed())
238244
}
239245

246+
// GlanceAPIExists - Asserts the GlanceAPI exist
240247
func GlanceAPIExists(name types.NamespacedName) {
241248
Consistently(func(g Gomega) {
242249
instance := &glancev1.GlanceAPI{}
@@ -245,7 +252,8 @@ func GlanceAPIExists(name types.NamespacedName) {
245252
}, timeout, interval).Should(Succeed())
246253
}
247254

248-
// AssertPVCDoesNotExist ensures the local PVC resource does not exist in a k8s cluster.
255+
// AssertPVCDoesNotExist ensures the local PVC resource does not exist in a k8s
256+
// cluster.
249257
func AssertPVCDoesNotExist(name types.NamespacedName) {
250258
instance := &corev1.PersistentVolumeClaim{}
251259
Eventually(func(g Gomega) {
@@ -263,11 +271,20 @@ func AssertPVCExist(name types.NamespacedName) {
263271
}, th.Timeout, th.Interval).Should(Succeed())
264272
}
265273

266-
// AssertCronJobDoesNotExist ensures the CronJob resource does not exist in a k8s cluster.
274+
// AssertCronJobDoesNotExist ensures the CronJob resource does not exist in a
275+
// k8s cluster.
267276
func AssertCronJobDoesNotExist(name types.NamespacedName) {
268277
instance := &batchv1.CronJob{}
269278
Eventually(func(g Gomega) {
270279
err := th.K8sClient.Get(th.Ctx, name, instance)
271280
g.Expect(k8s_errors.IsNotFound(err)).To(BeTrue())
272281
}, th.Timeout, th.Interval).Should(Succeed())
273282
}
283+
284+
// GetDummyBackend - Utility function that simulates a customServiceConfig
285+
// where a Ceph backend has been set
286+
func GetDummyBackend() string {
287+
section := "[DEFAULT]"
288+
dummyBackend := "enabled_backends=backend1:rbd"
289+
return fmt.Sprintf("%s\n%s", section, dummyBackend)
290+
}

‎test/functional/glance_controller_test.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -234,16 +234,12 @@ var _ = Describe("Glance controller", func() {
234234
condition.DBReadyCondition,
235235
corev1.ConditionTrue,
236236
)
237-
th.ExpectCondition(
238-
glanceTest.Instance,
237+
th.ExpectCondition(glanceTest.Instance,
239238
ConditionGetterFunc(GlanceConditionGetter),
240239
condition.DBSyncReadyCondition,
241240
corev1.ConditionTrue,
242241
)
243242
})
244-
It("GlanceAPI CR is created", func() {
245-
GlanceAPIExists(glanceTest.GlanceSingle)
246-
})
247243
})
248244
When("Glance CR is created without container images defined", func() {
249245
BeforeEach(func() {

‎test/functional/validation_webhook_test.go‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/*
2+
Copyright 2024.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
package functional
17+
18+
import (
19+
"errors"
20+
21+
. "github.com/onsi/ginkgo/v2"
22+
. "github.com/onsi/gomega"
23+
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
24+
25+
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
26+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
27+
)
28+
29+
var _ = Describe("Glance validation", func() {
30+
It("webhooks reject the request - invalid keystoneEndpoint", func() {
31+
// GlanceEmptySpec is used to provide a standard Glance CR where no
32+
// field is customized: we can inject our parameters to test webhooks
33+
spec := GetGlanceDefaultSpec()
34+
spec["keystoneEndpoint"] = "foo"
35+
raw := map[string]interface{}{
36+
"apiVersion": "glance.openstack.org/v1beta1",
37+
"kind": "Glance",
38+
"metadata": map[string]interface{}{
39+
"name": glanceTest.Instance.Name,
40+
"namespace": glanceTest.Instance.Namespace,
41+
},
42+
"spec": spec,
43+
}
44+
unstructuredObj := &unstructured.Unstructured{Object: raw}
45+
_, err := controllerutil.CreateOrPatch(
46+
ctx, k8sClient, unstructuredObj, func() error { return nil })
47+
48+
Expect(err).Should(HaveOccurred())
49+
var statusError *k8s_errors.StatusError
50+
Expect(errors.As(err, &statusError)).To(BeTrue())
51+
Expect(statusError.ErrStatus.Message).To(
52+
ContainSubstring(
53+
"KeystoneEndpoint is assigned to an invalid glanceAPI instance"),
54+
)
55+
})
56+
57+
It("webhooks reject the request - invalid backend", func() {
58+
spec := GetGlanceDefaultSpec()
59+
60+
gapis := map[string]interface{}{
61+
"glanceAPIs": map[string]interface{}{
62+
"default": map[string]interface{}{
63+
"replicas": 1,
64+
"type": "split",
65+
},
66+
"edge1": map[string]interface{}{
67+
"replicas": 1,
68+
"type": "edge",
69+
},
70+
},
71+
}
72+
73+
spec["keystoneEndpoint"] = "edge1"
74+
spec["glanceAPIs"] = gapis
75+
76+
raw := map[string]interface{}{
77+
"apiVersion": "glance.openstack.org/v1beta1",
78+
"kind": "Glance",
79+
"metadata": map[string]interface{}{
80+
"name": glanceTest.Instance.Name,
81+
"namespace": glanceTest.Instance.Namespace,
82+
},
83+
"spec": spec,
84+
}
85+
unstructuredObj := &unstructured.Unstructured{Object: raw}
86+
_, err := controllerutil.CreateOrPatch(
87+
ctx, k8sClient, unstructuredObj, func() error { return nil })
88+
89+
Expect(err).Should(HaveOccurred())
90+
var statusError *k8s_errors.StatusError
91+
Expect(errors.As(err, &statusError)).To(BeTrue())
92+
// Webhooks catch that no backend is set before even realize that an
93+
// invalid endpoint has been set
94+
Expect(statusError.ErrStatus.Message).To(
95+
ContainSubstring(
96+
"Invalid backend configuration detected"),
97+
)
98+
})
99+
100+
It("webhooks reject the request - invalid instance", func() {
101+
spec := GetGlanceDefaultSpec()
102+
103+
gapis := map[string]interface{}{
104+
"edge2": map[string]interface{}{
105+
"replicas": 1,
106+
"type": "edge",
107+
// inject a valid Ceph backend
108+
"customServiceConfig": GetDummyBackend(),
109+
},
110+
"default": map[string]interface{}{
111+
"replicas": 1,
112+
"type": "split",
113+
// inject a valid Ceph backend
114+
"customServiceConfig": GetDummyBackend(),
115+
},
116+
"edge1": map[string]interface{}{
117+
"replicas": 1,
118+
"type": "edge",
119+
// inject a valid Ceph backend
120+
"customServiceConfig": GetDummyBackend(),
121+
},
122+
}
123+
// Set the KeystoneEndpoint to the wrong instance
124+
spec["keystoneEndpoint"] = "edge1"
125+
// Deploy multiple GlanceAPIs
126+
spec["glanceAPIs"] = gapis
127+
128+
raw := map[string]interface{}{
129+
"apiVersion": "glance.openstack.org/v1beta1",
130+
"kind": "Glance",
131+
"metadata": map[string]interface{}{
132+
"name": glanceTest.Instance.Name,
133+
"namespace": glanceTest.Instance.Namespace,
134+
},
135+
"spec": spec,
136+
}
137+
unstructuredObj := &unstructured.Unstructured{Object: raw}
138+
_, err := controllerutil.CreateOrPatch(
139+
ctx, k8sClient, unstructuredObj, func() error { return nil })
140+
141+
Expect(err).Should(HaveOccurred())
142+
var statusError *k8s_errors.StatusError
143+
Expect(errors.As(err, &statusError)).To(BeTrue())
144+
// We shouldn't fail again for the backend, but because the endpoint is
145+
// not valid
146+
Expect(statusError.ErrStatus.Message).To(
147+
ContainSubstring(
148+
"KeystoneEndpoint is assigned to an invalid glanceAPI instance"),
149+
)
150+
})
151+
})

0 commit comments

Comments
 (0)