Skip to content

Commit 41f0232

Browse files
sbueringersbueringer
committed
Fix review findings
1 parent 0f149c2 commit 41f0232

File tree

13 files changed

+152
-28
lines changed

13 files changed

+152
-28
lines changed

‎api/v1beta1/cluster_types.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -546,13 +546,15 @@ type Topology struct {
546546
// +kubebuilder:validation:MaxLength=253
547547
Class string `json:"class"`
548548

549-
// classNamespace is the namespace of the ClusterClass object to create the topology.
550-
// If the namespace is empty or not set, it is defaulted to the namespace of the cluster object.
551-
// Value must follow the DNS1123Subdomain syntax.
549+
// classNamespace is the namespace of the ClusterClass that should be used for the topology.
550+
// If classNamespace is empty or not set, it is defaulted to the namespace of the Cluster object.
551+
// classNamespace must be a valid namespace name and because of that be at most 63 characters in length
552+
// and it must consist only of lower case alphanumeric characters or hyphens (-), and must start
553+
// and end with an alphanumeric character.
552554
// +optional
553555
// +kubebuilder:validation:MinLength=1
554-
// +kubebuilder:validation:MaxLength=253
555-
// +kubebuilder:validation:Pattern=`^[a-z0-9](?:[-a-z0-9]*[a-z0-9])?(?:\.[a-z0-9](?:[-a-z0-9]*[a-z0-9])?)*$`
556+
// +kubebuilder:validation:MaxLength=63
557+
// +kubebuilder:validation:Pattern=`^[a-z0-9](?:[-a-z0-9]*[a-z0-9])$`
556558
ClassNamespace string `json:"classNamespace,omitempty"`
557559

558560
// version is the Kubernetes version of the cluster.

‎api/v1beta1/zz_generated.openapi.go

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

‎api/v1beta2/cluster_types.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -578,18 +578,24 @@ type Topology struct {
578578
// ClusterClassRef is the ref to the ClusterClass that should be used for the topology.
579579
type ClusterClassRef struct {
580580
// name is the name of the ClusterClass that should be used for the topology.
581+
// name must be a valid ClusterClass name and because of that be at most 253 characters in length
582+
// and it must consist only of lower case alphanumeric characters, hyphens (-) and periods (.), and must start
583+
// and end with an alphanumeric character.
581584
// +required
582585
// +kubebuilder:validation:MinLength=1
583586
// +kubebuilder:validation:MaxLength=253
587+
// +kubebuilder:validation:Pattern=`^[a-z0-9](?:[-a-z0-9]*[a-z0-9])?(?:\.[a-z0-9](?:[-a-z0-9]*[a-z0-9])?)*$`
584588
Name string `json:"name"`
585589

586590
// namespace is the namespace of the ClusterClass that should be used for the topology.
587-
// If the namespace is empty or not set, it is defaulted to the namespace of the cluster object.
588-
// Value must follow the DNS1123Subdomain syntax.
591+
// If namespace is empty or not set, it is defaulted to the namespace of the Cluster object.
592+
// namespace must be a valid namespace name and because of that be at most 63 characters in length
593+
// and it must consist only of lower case alphanumeric characters or hyphens (-), and must start
594+
// and end with an alphanumeric character.
589595
// +optional
590596
// +kubebuilder:validation:MinLength=1
591-
// +kubebuilder:validation:MaxLength=253
592-
// +kubebuilder:validation:Pattern=`^[a-z0-9](?:[-a-z0-9]*[a-z0-9])?(?:\.[a-z0-9](?:[-a-z0-9]*[a-z0-9])?)*$`
597+
// +kubebuilder:validation:MaxLength=63
598+
// +kubebuilder:validation:Pattern=`^[a-z0-9](?:[-a-z0-9]*[a-z0-9])$`
593599
Namespace string `json:"namespace,omitempty"`
594600
}
595601

‎api/v1beta2/zz_generated.openapi.go

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

‎config/crd/bases/cluster.x-k8s.io_clusters.yaml

Lines changed: 19 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎docs/book/src/tasks/experimental-features/cluster-class/write-clusterclass.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,8 @@ spec:
612612
In addition to variables specified in the ClusterClass, the following builtin variables can be
613613
referenced in patches:
614614
- `builtin.cluster.{name,namespace,uid,metadata.labels,metadata.annotations}`
615-
- `builtin.cluster.topology.{version,class,classNamespace}`
615+
- `builtin.cluster.topology.{version,classRef.name,classRef.namespace,class,classNamespace}`
616+
- Note: `class` and `classNamespace` are deprecated and will be removed with the next apiVersion.
616617
- `builtin.cluster.network.{serviceDomain,services,pods}`
617618
- `builtin.controlPlane.{replicas,version,name,metadata.labels,metadata.annotations}`
618619
- Please note, these variables are only available when patching control plane or control plane

‎exp/runtime/hooks/api/v1alpha1/topologymutation_variable_types.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,39 @@ type ClusterTopologyBuiltins struct {
7676
// +optional
7777
Version string `json:"version,omitempty"`
7878

79+
// classRef is the ref to the ClusterClass that is used for the topology.
80+
//
81+
// +required
82+
ClassRef ClusterTopologyClusterClassRefBuiltins `json:"classRef"`
83+
7984
// class is the name of the ClusterClass of the Cluster.
85+
//
86+
// Deprecated: Class is deprecated in favor of ClassRef.Name and is going to be removed in the next apiVersion.
87+
//
8088
// +optional
8189
Class string `json:"class,omitempty"`
8290

8391
// classNamespace is the namespace of the ClusterClass of the Cluster.
92+
//
93+
// Deprecated: ClassNamespace is deprecated in favor of ClassRef.Namespace and is going to be removed in the next apiVersion.
94+
//
8495
// +optional
8596
ClassNamespace string `json:"classNamespace,omitempty"`
8697
}
8798

99+
// ClusterTopologyClusterClassRefBuiltins is the ref to the ClusterClass that is used for the topology.
100+
type ClusterTopologyClusterClassRefBuiltins struct {
101+
// name is the name of the ClusterClass that is used for the topology.
102+
//
103+
// +required
104+
Name string `json:"name"`
105+
106+
// namespace is the namespace of the ClusterClass that is used for the topology.
107+
//
108+
// +required
109+
Namespace string `json:"namespace"`
110+
}
111+
88112
// ClusterNetworkBuiltins represents builtin cluster network variables.
89113
type ClusterNetworkBuiltins struct {
90114
// serviceDomain is the domain name for services.

‎exp/runtime/hooks/api/v1alpha1/zz_generated.deepcopy.go

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎exp/runtime/hooks/api/v1alpha1/zz_generated.openapi.go

Lines changed: 43 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎exp/runtime/topologymutation/variables_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ func TestMergeVariables(t *testing.T) {
307307

308308
m, err := MergeVariableMaps(
309309
map[string]apiextensionsv1.JSON{
310-
runtimehooksv1.BuiltinsName: {Raw: []byte(`{"cluster":{"name":"cluster-name","namespace":"default","topology":{"class":"clusterClass1","version":"v1.21.1"}}}`)},
310+
runtimehooksv1.BuiltinsName: {Raw: []byte(`{"cluster":{"name":"cluster-name","namespace":"default","topology":{"classRef":{"name":"clusterClass1","namespace":"default"},"class":"clusterClass1","version":"v1.21.1"}}}`)},
311311
"a": {Raw: []byte("a-different")},
312312
"c": {Raw: []byte("c")},
313313
},
@@ -321,7 +321,7 @@ func TestMergeVariables(t *testing.T) {
321321
)
322322
g.Expect(err).ToNot(HaveOccurred())
323323

324-
g.Expect(m).To(HaveKeyWithValue(runtimehooksv1.BuiltinsName, apiextensionsv1.JSON{Raw: []byte(`{"cluster":{"name":"cluster-name-overwrite","namespace":"default","topology":{"version":"v1.21.1","class":"clusterClass1"}},"controlPlane":{"replicas":3}}`)}))
324+
g.Expect(m).To(HaveKeyWithValue(runtimehooksv1.BuiltinsName, apiextensionsv1.JSON{Raw: []byte(`{"cluster":{"name":"cluster-name-overwrite","namespace":"default","topology":{"version":"v1.21.1","classRef":{"name":"clusterClass1","namespace":"default"},"class":"clusterClass1"}},"controlPlane":{"replicas":3}}`)}))
325325
g.Expect(m).To(HaveKeyWithValue("a", apiextensionsv1.JSON{Raw: []byte("a")}))
326326
g.Expect(m).To(HaveKeyWithValue("b", apiextensionsv1.JSON{Raw: []byte("b")}))
327327
g.Expect(m).To(HaveKeyWithValue("c", apiextensionsv1.JSON{Raw: []byte("c")}))

‎internal/controllers/topology/cluster/patches/variables/variables.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ func Global(clusterTopology *clusterv1.Topology, cluster *clusterv1.Cluster, pat
5959
Version: cluster.Spec.Topology.Version,
6060
Class: cluster.GetClassKey().Name,
6161
ClassNamespace: cluster.GetClassKey().Namespace,
62+
ClassRef: runtimehooksv1.ClusterTopologyClusterClassRefBuiltins{
63+
Name: cluster.GetClassKey().Name,
64+
Namespace: cluster.GetClassKey().Namespace,
65+
},
6266
},
6367
},
6468
}

‎internal/controllers/topology/cluster/patches/variables/variables_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,10 @@ func TestGlobal(t *testing.T) {
109109
"metadata": {"labels":{"foo":"bar"}, "annotations":{"fizz":"buzz"}},
110110
"topology":{
111111
"version": "v1.21.1",
112+
"classRef": {
113+
"name": "clusterClass1",
114+
"namespace": "default"
115+
},
112116
"class": "clusterClass1",
113117
"classNamespace": "default"
114118
},
@@ -183,6 +187,10 @@ func TestGlobal(t *testing.T) {
183187
"uid": "8a35f406-6b9b-4b78-8c93-a7f878d90623",
184188
"topology":{
185189
"version": "v1.21.1",
190+
"classRef": {
191+
"name": "clusterClass1",
192+
"namespace": "default"
193+
},
186194
"class": "clusterClass1",
187195
"classNamespace": "default"
188196
},
@@ -257,6 +265,10 @@ func TestGlobal(t *testing.T) {
257265
"uid": "8a35f406-6b9b-4b78-8c93-a7f878d90623",
258266
"topology":{
259267
"version": "v1.21.1",
268+
"classRef": {
269+
"name": "clusterClass1",
270+
"namespace": "default"
271+
},
260272
"class": "clusterClass1",
261273
"classNamespace": "default"
262274
},
@@ -327,6 +339,10 @@ func TestGlobal(t *testing.T) {
327339
"uid": "8a35f406-6b9b-4b78-8c93-a7f878d90623",
328340
"topology":{
329341
"version": "v1.21.1",
342+
"classRef": {
343+
"name": "clusterClass1",
344+
"namespace": "default"
345+
},
330346
"class": "clusterClass1",
331347
"classNamespace": "default"
332348
},
@@ -392,6 +408,10 @@ func TestGlobal(t *testing.T) {
392408
"uid": "8a35f406-6b9b-4b78-8c93-a7f878d90623",
393409
"topology":{
394410
"version": "v1.21.1",
411+
"classRef": {
412+
"name": "clusterClass1",
413+
"namespace": "default"
414+
},
395415
"class": "clusterClass1",
396416
"classNamespace": "default"
397417
}

0 commit comments

Comments
 (0)