Skip to content

Commit 3df5eb4

Browse files
authored
Merge pull request #524 from vshn/fix/collabora-fqdn-required
Make Collabora FQDN nonmandatory
2 parents 701fb00 + 6332604 commit 3df5eb4

File tree

5 files changed

+261
-3
lines changed

5 files changed

+261
-3
lines changed

apis/vshn/v1/vshn_nextcloud.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,6 @@ type CollaboraSpec struct {
227227
//+kubebuilder:default=false
228228
Enabled bool `json:"enabled"`
229229
// FQDN contains the FQDN of the Collabora server. This is used to configure the Collabora server URL in Your Nextcloud instance.
230-
//+kubebuilder:validation:Required
231230
FQDN string `json:"fqdn,omitempty"`
232231
// Version defines the Collabora version to use.
233232
Version string `json:"version,omitempty"`

crds/vshn.appcat.vshn.io_vshnnextclouds.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4631,7 +4631,6 @@ spec:
46314631
type: string
46324632
required:
46334633
- enabled
4634-
- fqdn
46354634
type: object
46364635
existingPGConnectionSecret:
46374636
description: |-

crds/vshn.appcat.vshn.io_xvshnnextclouds.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5360,7 +5360,6 @@ spec:
53605360
type: string
53615361
required:
53625362
- enabled
5363-
- fqdn
53645363
type: object
53655364
existingPGConnectionSecret:
53665365
description: |-

pkg/controller/webhooks/nextcloud.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ func (n *NextcloudWebhookHandler) ValidateCreate(ctx context.Context, obj runtim
6464
}
6565

6666
if nx.Spec.Parameters.Service.Collabora.Enabled {
67+
if nx.Spec.Parameters.Service.Collabora.FQDN == "" {
68+
return nil, fmt.Errorf("Collabora FQDN is required when Collabora is enabled")
69+
}
6770
if err := validateFQDNs([]string{nx.Spec.Parameters.Service.Collabora.FQDN}); err != nil {
6871
return nil, err
6972
}
@@ -93,6 +96,9 @@ func (n *NextcloudWebhookHandler) ValidateUpdate(ctx context.Context, oldObj, ne
9396
}
9497

9598
if nx.Spec.Parameters.Service.Collabora.Enabled {
99+
if nx.Spec.Parameters.Service.Collabora.FQDN == "" {
100+
return nil, fmt.Errorf("Collabora FQDN is required when Collabora is enabled")
101+
}
96102
if err := validateFQDNs([]string{nx.Spec.Parameters.Service.Collabora.FQDN}); err != nil {
97103
return nil, err
98104
}

pkg/controller/webhooks/nextcloud_test.go

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,261 @@ func TestWebhookHandlerWithManager_ValidateUpdate_FQDN(t *testing.T) {
147147
assert.Equal(t, fmt.Errorf("FQDN n€xtcloud.example.tld is not a valid DNS name"), err)
148148
}
149149

150+
func TestNextcloudWebhookHandler_ValidateCreate_CollaboraFQDN(t *testing.T) {
151+
// Given
152+
claimNS := &corev1.Namespace{
153+
ObjectMeta: metav1.ObjectMeta{
154+
Name: "claimns",
155+
Labels: map[string]string{
156+
utils.OrgLabelName: "myorg",
157+
},
158+
},
159+
}
160+
161+
ctx := context.TODO()
162+
163+
fclient := fake.NewClientBuilder().
164+
WithScheme(pkg.SetupScheme()).
165+
WithObjects(claimNS).
166+
Build()
167+
168+
handler := NextcloudWebhookHandler{
169+
DefaultWebhookHandler: DefaultWebhookHandler{
170+
client: fclient,
171+
log: logr.Discard(),
172+
withQuota: true,
173+
obj: &vshnv1.VSHNNextcloud{},
174+
name: "nextcloud",
175+
},
176+
}
177+
178+
// Test 1: Collabora disabled - no FQDN validation should occur
179+
nextcloudCollaboraDisabled := &vshnv1.VSHNNextcloud{
180+
ObjectMeta: metav1.ObjectMeta{
181+
Name: "myinstance",
182+
Namespace: "claimns",
183+
},
184+
Spec: vshnv1.VSHNNextcloudSpec{
185+
Parameters: vshnv1.VSHNNextcloudParameters{
186+
Service: vshnv1.VSHNNextcloudServiceSpec{
187+
FQDN: []string{"mynextcloud.example.tld"},
188+
Collabora: vshnv1.CollaboraSpec{
189+
Enabled: false,
190+
FQDN: "", // Empty FQDN should be fine when disabled
191+
},
192+
},
193+
Size: vshnv1.VSHNSizeSpec{
194+
Requests: vshnv1.VSHNDBaaSSizeRequestsSpec{
195+
CPU: "500m",
196+
},
197+
},
198+
},
199+
},
200+
}
201+
202+
_, err := handler.ValidateCreate(ctx, nextcloudCollaboraDisabled)
203+
assert.NoError(t, err, "Collabora disabled with empty FQDN should pass validation")
204+
205+
// Test 2: Collabora enabled with valid FQDN - should pass
206+
nextcloudCollaboraValid := &vshnv1.VSHNNextcloud{
207+
ObjectMeta: metav1.ObjectMeta{
208+
Name: "myinstance",
209+
Namespace: "claimns",
210+
},
211+
Spec: vshnv1.VSHNNextcloudSpec{
212+
Parameters: vshnv1.VSHNNextcloudParameters{
213+
Service: vshnv1.VSHNNextcloudServiceSpec{
214+
FQDN: []string{"mynextcloud.example.tld"},
215+
Collabora: vshnv1.CollaboraSpec{
216+
Enabled: true,
217+
FQDN: "collabora.example.tld",
218+
},
219+
},
220+
Size: vshnv1.VSHNSizeSpec{
221+
Requests: vshnv1.VSHNDBaaSSizeRequestsSpec{
222+
CPU: "500m",
223+
},
224+
},
225+
},
226+
},
227+
}
228+
229+
_, err = handler.ValidateCreate(ctx, nextcloudCollaboraValid)
230+
assert.NoError(t, err, "Collabora enabled with valid FQDN should pass validation")
231+
232+
// Test 3: Collabora enabled with invalid FQDN - should fail
233+
nextcloudCollaboraInvalid := &vshnv1.VSHNNextcloud{
234+
ObjectMeta: metav1.ObjectMeta{
235+
Name: "myinstance",
236+
Namespace: "claimns",
237+
},
238+
Spec: vshnv1.VSHNNextcloudSpec{
239+
Parameters: vshnv1.VSHNNextcloudParameters{
240+
Service: vshnv1.VSHNNextcloudServiceSpec{
241+
FQDN: []string{"mynextcloud.example.tld"},
242+
Collabora: vshnv1.CollaboraSpec{
243+
Enabled: true,
244+
FQDN: "c€llabora.example.tld", // Invalid characters
245+
},
246+
},
247+
Size: vshnv1.VSHNSizeSpec{
248+
Requests: vshnv1.VSHNDBaaSSizeRequestsSpec{
249+
CPU: "500m",
250+
},
251+
},
252+
},
253+
},
254+
}
255+
256+
_, err = handler.ValidateCreate(ctx, nextcloudCollaboraInvalid)
257+
assert.Error(t, err, "Collabora enabled with invalid FQDN should fail validation")
258+
assert.Equal(t, fmt.Errorf("FQDN c€llabora.example.tld is not a valid DNS name"), err)
259+
260+
// Test 4: Collabora enabled with empty FQDN - should fail
261+
nextcloudCollaboraEmpty := &vshnv1.VSHNNextcloud{
262+
ObjectMeta: metav1.ObjectMeta{
263+
Name: "myinstance",
264+
Namespace: "claimns",
265+
},
266+
Spec: vshnv1.VSHNNextcloudSpec{
267+
Parameters: vshnv1.VSHNNextcloudParameters{
268+
Service: vshnv1.VSHNNextcloudServiceSpec{
269+
FQDN: []string{"mynextcloud.example.tld"},
270+
Collabora: vshnv1.CollaboraSpec{
271+
Enabled: true,
272+
FQDN: "", // Empty FQDN when enabled
273+
},
274+
},
275+
Size: vshnv1.VSHNSizeSpec{
276+
Requests: vshnv1.VSHNDBaaSSizeRequestsSpec{
277+
CPU: "500m",
278+
},
279+
},
280+
},
281+
},
282+
}
283+
284+
_, err = handler.ValidateCreate(ctx, nextcloudCollaboraEmpty)
285+
assert.Error(t, err, "Collabora enabled with empty FQDN should fail validation")
286+
assert.Equal(t, fmt.Errorf("Collabora FQDN is required when Collabora is enabled"), err)
287+
}
288+
289+
func TestNextcloudWebhookHandler_ValidateUpdate_CollaboraFQDN(t *testing.T) {
290+
// Given
291+
claimNS := &corev1.Namespace{
292+
ObjectMeta: metav1.ObjectMeta{
293+
Name: "claimns",
294+
Labels: map[string]string{
295+
utils.OrgLabelName: "myorg",
296+
},
297+
},
298+
}
299+
300+
ctx := context.TODO()
301+
302+
fclient := fake.NewClientBuilder().
303+
WithScheme(pkg.SetupScheme()).
304+
WithObjects(claimNS).
305+
Build()
306+
307+
handler := NextcloudWebhookHandler{
308+
DefaultWebhookHandler: DefaultWebhookHandler{
309+
client: fclient,
310+
log: logr.Discard(),
311+
withQuota: true,
312+
obj: &vshnv1.VSHNNextcloud{},
313+
name: "nextcloud",
314+
},
315+
}
316+
317+
nextcloudOrig := &vshnv1.VSHNNextcloud{
318+
ObjectMeta: metav1.ObjectMeta{
319+
Name: "myinstance",
320+
Namespace: "claimns",
321+
},
322+
Spec: vshnv1.VSHNNextcloudSpec{
323+
Parameters: vshnv1.VSHNNextcloudParameters{
324+
Service: vshnv1.VSHNNextcloudServiceSpec{
325+
FQDN: []string{"mynextcloud.example.tld"},
326+
Collabora: vshnv1.CollaboraSpec{
327+
Enabled: false,
328+
FQDN: "",
329+
},
330+
},
331+
Size: vshnv1.VSHNSizeSpec{
332+
Requests: vshnv1.VSHNDBaaSSizeRequestsSpec{
333+
CPU: "500m",
334+
},
335+
},
336+
},
337+
},
338+
}
339+
340+
// Test 1: Update with Collabora disabled - should pass even with empty FQDN
341+
nextcloudUpdated := nextcloudOrig.DeepCopy()
342+
nextcloudUpdated.Spec.Parameters.Service.Collabora.Enabled = false
343+
nextcloudUpdated.Spec.Parameters.Service.Collabora.FQDN = ""
344+
345+
_, err := handler.ValidateUpdate(ctx, nextcloudOrig, nextcloudUpdated)
346+
assert.NoError(t, err, "Collabora disabled with empty FQDN should pass validation")
347+
348+
// Test 2: Enable Collabora with valid FQDN - should pass
349+
nextcloudEnableValid := nextcloudOrig.DeepCopy()
350+
nextcloudEnableValid.Spec.Parameters.Service.Collabora.Enabled = true
351+
nextcloudEnableValid.Spec.Parameters.Service.Collabora.FQDN = "collabora.example.tld"
352+
353+
_, err = handler.ValidateUpdate(ctx, nextcloudOrig, nextcloudEnableValid)
354+
assert.NoError(t, err, "Enabling Collabora with valid FQDN should pass validation")
355+
356+
// Test 3: Enable Collabora with invalid FQDN - should fail
357+
nextcloudEnableInvalid := nextcloudOrig.DeepCopy()
358+
nextcloudEnableInvalid.Spec.Parameters.Service.Collabora.Enabled = true
359+
nextcloudEnableInvalid.Spec.Parameters.Service.Collabora.FQDN = "c€llabora.example.tld"
360+
361+
_, err = handler.ValidateUpdate(ctx, nextcloudOrig, nextcloudEnableInvalid)
362+
assert.Error(t, err, "Enabling Collabora with invalid FQDN should fail validation")
363+
assert.Equal(t, fmt.Errorf("FQDN c€llabora.example.tld is not a valid DNS name"), err)
364+
365+
// Test 4: Enable Collabora with empty FQDN - should fail
366+
nextcloudEnableEmpty := nextcloudOrig.DeepCopy()
367+
nextcloudEnableEmpty.Spec.Parameters.Service.Collabora.Enabled = true
368+
nextcloudEnableEmpty.Spec.Parameters.Service.Collabora.FQDN = ""
369+
370+
_, err = handler.ValidateUpdate(ctx, nextcloudOrig, nextcloudEnableEmpty)
371+
assert.Error(t, err, "Enabling Collabora with empty FQDN should fail validation")
372+
assert.Equal(t, fmt.Errorf("Collabora FQDN is required when Collabora is enabled"), err)
373+
374+
// Test 5: Update Collabora FQDN to another valid one - should pass
375+
nextcloudWithCollabora := &vshnv1.VSHNNextcloud{
376+
ObjectMeta: metav1.ObjectMeta{
377+
Name: "myinstance",
378+
Namespace: "claimns",
379+
},
380+
Spec: vshnv1.VSHNNextcloudSpec{
381+
Parameters: vshnv1.VSHNNextcloudParameters{
382+
Service: vshnv1.VSHNNextcloudServiceSpec{
383+
FQDN: []string{"mynextcloud.example.tld"},
384+
Collabora: vshnv1.CollaboraSpec{
385+
Enabled: true,
386+
FQDN: "collabora.example.tld",
387+
},
388+
},
389+
Size: vshnv1.VSHNSizeSpec{
390+
Requests: vshnv1.VSHNDBaaSSizeRequestsSpec{
391+
CPU: "500m",
392+
},
393+
},
394+
},
395+
},
396+
}
397+
398+
nextcloudUpdateFQDN := nextcloudWithCollabora.DeepCopy()
399+
nextcloudUpdateFQDN.Spec.Parameters.Service.Collabora.FQDN = "new-collabora.example.tld"
400+
401+
_, err = handler.ValidateUpdate(ctx, nextcloudWithCollabora, nextcloudUpdateFQDN)
402+
assert.NoError(t, err, "Updating Collabora FQDN to another valid one should pass validation")
403+
}
404+
150405
func TestNextcloudWebhookHandler_ValidatePostgreSQLEncryptionChanges(t *testing.T) {
151406
ctx := context.TODO()
152407
fclient := fake.NewClientBuilder().

0 commit comments

Comments
 (0)