Skip to content

Commit 50288fe

Browse files
GrigoryPervakovGrigoryPervakov
committed
Validation keeper ref fields in API, minor changes
1 parent 19da5f0 commit 50288fe

11 files changed

Lines changed: 96 additions & 104 deletions

File tree

‎api/v1alpha1/clickhousecluster_types.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ type ClickHouseClusterSpec struct {
3434
// Reference to the KeeperCluster that is used for ClickHouse coordination.
3535
// When namespace is omitted, the ClickHouseCluster namespace is used.
3636
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Keeper Cluster Reference"
37-
KeeperClusterRef *KeeperClusterReference `json:"keeperClusterRef"`
37+
KeeperClusterRef KeeperClusterReference `json:"keeperClusterRef"`
3838

3939
// Parameters passed to the ClickHouse pod spec.
4040
// +optional
@@ -210,9 +210,13 @@ type ClickHouseClusterStatus struct {
210210
// KeeperClusterReference identifies the KeeperCluster used by a ClickHouseCluster.
211211
type KeeperClusterReference struct {
212212
// Name of the KeeperCluster resource.
213+
// +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`
214+
// +kubebuilder:validation:MaxLength=63
213215
Name string `json:"name"`
214216
// Namespace of the KeeperCluster resource.
215217
// When omitted, the ClickHouseCluster namespace is used.
218+
// +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`
219+
// +kubebuilder:validation:MaxLength=63
216220
// +optional
217221
Namespace string `json:"namespace,omitempty"`
218222
}

‎api/v1alpha1/types_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ var _ = Describe("ClickHouseCluster", func() {
6666
Namespace: "clickhouse-ns",
6767
},
6868
Spec: ClickHouseClusterSpec{
69-
KeeperClusterRef: &KeeperClusterReference{
69+
KeeperClusterRef: KeeperClusterReference{
7070
Name: "keeper",
7171
},
7272
},
@@ -85,7 +85,7 @@ var _ = Describe("ClickHouseCluster", func() {
8585
Namespace: "clickhouse-ns",
8686
},
8787
Spec: ClickHouseClusterSpec{
88-
KeeperClusterRef: &KeeperClusterReference{
88+
KeeperClusterRef: KeeperClusterReference{
8989
Name: "keeper",
9090
Namespace: "keeper-ns",
9191
},

‎api/v1alpha1/zz_generated.deepcopy.go‎

Lines changed: 1 addition & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/clickhouse.com_clickhouseclusters.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,11 +1119,15 @@ spec:
11191119
properties:
11201120
name:
11211121
description: Name of the KeeperCluster resource.
1122+
maxLength: 63
1123+
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
11221124
type: string
11231125
namespace:
11241126
description: |-
11251127
Namespace of the KeeperCluster resource.
11261128
When omitted, the ClickHouseCluster namespace is used.
1129+
maxLength: 63
1130+
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
11271131
type: string
11281132
required:
11291133
- name

‎dist/chart/templates/crd/clickhouseclusters.clickhouse.com.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,11 +1122,15 @@ spec:
11221122
properties:
11231123
name:
11241124
description: Name of the KeeperCluster resource.
1125+
maxLength: 63
1126+
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
11251127
type: string
11261128
namespace:
11271129
description: |-
11281130
Namespace of the KeeperCluster resource.
11291131
When omitted, the ClickHouseCluster namespace is used.
1132+
maxLength: 63
1133+
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
11301134
type: string
11311135
required:
11321136
- name

‎docs/styles/config/vocabularies/ClickHouse/accept.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
kubectl
22
hostname
33
CRDs
4-
namespace
4+
[Nn]amespaces?
55
uncomment
66
PVCs
77
PDBs

‎internal/controller/clickhouse/controller_test.go‎

Lines changed: 63 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -40,65 +40,6 @@ func TestControllers(t *testing.T) {
4040
RunSpecs(t, "ClickHouse Controller Suite")
4141
}
4242

43-
var _ = Describe("keeper watch mapping", func() {
44-
It("should enqueue ClickHouse clusters that explicitly reference a keeper in another namespace", func(ctx context.Context) {
45-
testScheme := k8sruntime.NewScheme()
46-
Expect(clientgoscheme.AddToScheme(testScheme)).To(Succeed())
47-
Expect(v1.AddToScheme(testScheme)).To(Succeed())
48-
49-
referencedCluster := &v1.ClickHouseCluster{
50-
ObjectMeta: metav1.ObjectMeta{
51-
Name: "cross-namespace-cluster",
52-
Namespace: "clickhouse-ns",
53-
},
54-
Spec: v1.ClickHouseClusterSpec{
55-
KeeperClusterRef: &v1.KeeperClusterReference{
56-
Name: "keeper",
57-
Namespace: "keeper-ns",
58-
},
59-
},
60-
}
61-
sameNameDifferentNamespace := &v1.ClickHouseCluster{
62-
ObjectMeta: metav1.ObjectMeta{
63-
Name: "same-name-different-namespace",
64-
Namespace: "other-ns",
65-
},
66-
Spec: v1.ClickHouseClusterSpec{
67-
KeeperClusterRef: &v1.KeeperClusterReference{
68-
Name: "keeper",
69-
},
70-
},
71-
}
72-
73-
controller := &ClusterController{
74-
Client: fake.NewClientBuilder().
75-
WithScheme(testScheme).
76-
WithObjects(referencedCluster, sameNameDifferentNamespace).
77-
WithIndex(&v1.ClickHouseCluster{}, keeperClusterReferenceField, func(obj client.Object) []string {
78-
cluster, ok := obj.(*v1.ClickHouseCluster)
79-
if !ok {
80-
return nil
81-
}
82-
83-
return keeperReferenceFieldValue(cluster)
84-
}).
85-
Build(),
86-
}
87-
88-
Expect(controller.clickHouseClustersForKeeper(ctx, &v1.KeeperCluster{
89-
ObjectMeta: metav1.ObjectMeta{
90-
Name: "keeper",
91-
Namespace: "keeper-ns",
92-
},
93-
})).To(ConsistOf(reconcile.Request{
94-
NamespacedName: types.NamespacedName{
95-
Name: referencedCluster.Name,
96-
Namespace: referencedCluster.Namespace,
97-
},
98-
}))
99-
})
100-
})
101-
10243
var _ = When("reconciling ClickHouseCluster", Ordered, func() {
10344
var (
10445
suite testutil.TestSuit
@@ -119,7 +60,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() {
11960
Spec: v1.ClickHouseClusterSpec{
12061
Replicas: new(int32(2)),
12162
Shards: new(int32(2)),
122-
KeeperClusterRef: &v1.KeeperClusterReference{Name: keeperName},
63+
KeeperClusterRef: v1.KeeperClusterReference{Name: keeperName},
12364
Labels: map[string]string{
12465
"test-label": "test-val",
12566
},
@@ -254,7 +195,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() {
254195
Spec: v1.ClickHouseClusterSpec{
255196
Replicas: new(int32(1)),
256197
Shards: new(int32(1)),
257-
KeeperClusterRef: &v1.KeeperClusterReference{
198+
KeeperClusterRef: v1.KeeperClusterReference{
258199
Name: keeper.Name,
259200
Namespace: keeper.Namespace,
260201
},
@@ -565,7 +506,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() {
565506
Spec: v1.ClickHouseClusterSpec{
566507
Replicas: new(int32(2)),
567508
Shards: new(int32(1)),
568-
KeeperClusterRef: &v1.KeeperClusterReference{Name: keeperName},
509+
KeeperClusterRef: v1.KeeperClusterReference{Name: keeperName},
569510
DataVolumeClaimSpec: &corev1.PersistentVolumeClaimSpec{
570511
AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
571512
Resources: corev1.VolumeResourceRequirements{
@@ -669,7 +610,7 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() {
669610
Namespace: "default",
670611
},
671612
Spec: v1.ClickHouseClusterSpec{
672-
KeeperClusterRef: &v1.KeeperClusterReference{Name: keeperName},
613+
KeeperClusterRef: v1.KeeperClusterReference{Name: keeperName},
673614
ExternalSecret: &v1.ExternalSecret{
674615
Name: secret.Name,
675616
},
@@ -729,3 +670,62 @@ var _ = When("reconciling ClickHouseCluster", Ordered, func() {
729670
Expect(secret.Data).To(HaveKey(SecretKeyClusterSecret))
730671
})
731672
})
673+
674+
var _ = Describe("keeper watch mapping", func() {
675+
It("should enqueue ClickHouse clusters that explicitly reference a keeper in another namespace", func(ctx context.Context) {
676+
testScheme := k8sruntime.NewScheme()
677+
Expect(clientgoscheme.AddToScheme(testScheme)).To(Succeed())
678+
Expect(v1.AddToScheme(testScheme)).To(Succeed())
679+
680+
referencedCluster := &v1.ClickHouseCluster{
681+
ObjectMeta: metav1.ObjectMeta{
682+
Name: "cross-namespace-cluster",
683+
Namespace: "clickhouse-ns",
684+
},
685+
Spec: v1.ClickHouseClusterSpec{
686+
KeeperClusterRef: v1.KeeperClusterReference{
687+
Name: "keeper",
688+
Namespace: "keeper-ns",
689+
},
690+
},
691+
}
692+
sameNameDifferentNamespace := &v1.ClickHouseCluster{
693+
ObjectMeta: metav1.ObjectMeta{
694+
Name: "same-name-different-namespace",
695+
Namespace: "other-ns",
696+
},
697+
Spec: v1.ClickHouseClusterSpec{
698+
KeeperClusterRef: v1.KeeperClusterReference{
699+
Name: "keeper",
700+
},
701+
},
702+
}
703+
704+
controller := &ClusterController{
705+
Client: fake.NewClientBuilder().
706+
WithScheme(testScheme).
707+
WithObjects(referencedCluster, sameNameDifferentNamespace).
708+
WithIndex(&v1.ClickHouseCluster{}, keeperClusterReferenceField, func(obj client.Object) []string {
709+
cluster, ok := obj.(*v1.ClickHouseCluster)
710+
if !ok {
711+
return nil
712+
}
713+
714+
return keeperReferenceFieldValue(cluster)
715+
}).
716+
Build(),
717+
}
718+
719+
Expect(controller.clickHouseClustersForKeeper(ctx, &v1.KeeperCluster{
720+
ObjectMeta: metav1.ObjectMeta{
721+
Name: "keeper",
722+
Namespace: "keeper-ns",
723+
},
724+
})).To(ConsistOf(reconcile.Request{
725+
NamespacedName: types.NamespacedName{
726+
Name: referencedCluster.Name,
727+
Namespace: referencedCluster.Namespace,
728+
},
729+
}))
730+
})
731+
})

‎internal/webhook/v1alpha1/clickhousecluster_webhook.go‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@ import (
44
"context"
55
"errors"
66
"fmt"
7-
"strings"
87

9-
"k8s.io/apimachinery/pkg/util/validation"
108
ctrl "sigs.k8s.io/controller-runtime"
119
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
1210

@@ -91,20 +89,6 @@ func (w *ClickHouseClusterWebhook) validateImpl(obj *chv1.ClickHouseCluster) (ad
9189
errs []error
9290
)
9391

94-
if obj.Spec.KeeperClusterRef == nil || obj.Spec.KeeperClusterRef.Name == "" {
95-
errs = append(errs, errors.New("keeperClusterRef name must not be empty"))
96-
}
97-
98-
if obj.Spec.KeeperClusterRef != nil && obj.Spec.KeeperClusterRef.Namespace != "" {
99-
if validationErrs := validation.IsDNS1123Label(obj.Spec.KeeperClusterRef.Namespace); len(validationErrs) > 0 {
100-
errs = append(errs, fmt.Errorf(
101-
"keeperClusterRef namespace %q is invalid: %s",
102-
obj.Spec.KeeperClusterRef.Namespace,
103-
strings.Join(validationErrs, ", "),
104-
))
105-
}
106-
}
107-
10892
if err := obj.Spec.Settings.TLS.Validate(); err != nil {
10993
errs = append(errs, err)
11094
}

‎internal/webhook/v1alpha1/clickhousecluster_webhook_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ var _ = Describe("ClickHouseCluster Webhook", func() {
2424
Name: "test-default",
2525
},
2626
Spec: chv1.ClickHouseClusterSpec{
27-
KeeperClusterRef: &chv1.KeeperClusterReference{
27+
KeeperClusterRef: chv1.KeeperClusterReference{
2828
Name: "some-keeper-cluster",
2929
},
3030
},
@@ -47,7 +47,7 @@ var _ = Describe("ClickHouseCluster Webhook", func() {
4747
Name: "test-default",
4848
},
4949
Spec: chv1.ClickHouseClusterSpec{
50-
KeeperClusterRef: &chv1.KeeperClusterReference{
50+
KeeperClusterRef: chv1.KeeperClusterReference{
5151
Name: "some-keeper-cluster",
5252
},
5353
DataVolumeClaimSpec: &corev1.PersistentVolumeClaimSpec{Resources: corev1.VolumeResourceRequirements{
@@ -73,7 +73,7 @@ var _ = Describe("ClickHouseCluster Webhook", func() {
7373
Name: "test-validate",
7474
},
7575
Spec: chv1.ClickHouseClusterSpec{
76-
KeeperClusterRef: &chv1.KeeperClusterReference{
76+
KeeperClusterRef: chv1.KeeperClusterReference{
7777
Name: "some-keeper-cluster",
7878
},
7979
},
@@ -107,7 +107,7 @@ var _ = Describe("ClickHouseCluster Webhook", func() {
107107

108108
err := k8sClient.Create(ctx, cluster)
109109
Expect(err).To(HaveOccurred())
110-
Expect(err.Error()).To(ContainSubstring("keeperClusterRef namespace"))
110+
Expect(err.Error()).To(ContainSubstring("keeperClusterRef.namespace"))
111111
})
112112

113113
It("Should check certificate passed if TLS enabled", func(ctx context.Context) {

‎test/deploy/deploy_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ func testDeployment(namespace string) {
376376
},
377377
Spec: v1.ClickHouseClusterSpec{
378378
Replicas: new(int32(1)),
379-
KeeperClusterRef: &v1.KeeperClusterReference{
379+
KeeperClusterRef: v1.KeeperClusterReference{
380380
Name: keeper.Name,
381381
},
382382
ContainerTemplate: v1.ContainerTemplateSpec{

0 commit comments

Comments
 (0)