Skip to content

Commit c083042

Browse files
Shreyas-s14shreyas-s-raorenormalizeunmarshall
authored
CRD Validation for Etcd resource using CEL expressions (#995)
* validation markers without cron regex fix and update rules * Added integration test framework for validating CRD fields * used common function to generate etcd object for tests + consistent cron expression * Upgraded env-test k8s version, and fixed corresponding bugs. * Added `itTestEnv.StartManager()` to start the manager in `setupTestEnvironment()`. * Upgraded teh env-test k8s version to `1.30`. * Changed the timeout to `20s` in `assertReconcileSuspensionEventRecorded` to prevent the test timing out. * Removed an unnecessary `time.Sleep` in `TestValidateSpecReplicas`. * 1) reduced timeout to 10s 2) added version compatibility docs 3) reduced code reuse in specUpdate_test.go Reverted the timeout value in test/it/helper.go which was increased to handle failure of prow jobs 1) Renamed "test/it/crd-validation" to "test/crdvalidation" 2) Removed Camel casing from the file names in test/crdvalidation/etcd 3) Changed version-compatibility matrix to 1.29 since CEL expressions are valid from this version onwards 4) Removed config/crd/bases/crd-druid.gardener.cloud_etcds.yaml * generate both crds based on k8s version 1) make prepare-helm-charts now takes an additional parameter to deploy the crd either with or without the CEL expressions based on the k8s version. 2) modified api/hack/generate.sh to generate 2 versions of the etcd crd. One with and one without CEL expressions 3) added deploy.sh which is called when make deploy is called. Additional functionality: get k8s version and deploy accordingly. * changed the crds API to accept a k8sversion and added unit test * removed deploy.sh, changed Makefile targets and refactored prepare-chart-resources * modified generate.sh to conditionally generate crd without cel validations * added missing license headers * changed it tests to skip test cases based on k8s version + removed validation for requests field in etcd 1) Changed the Makefile target for deploy to install the dependecies if not present 2) Removed existing validations for fields of type resources where the request field was mandatory. 3) reverted the cron expressions in examples/etcd/druid_v1alpha1_etcd_localstack.yaml 4) exported the kubernetes version variable. 5) removed duplicate variable in hack/tools.mk 6) Added path to etcd crd without CEL as a return type in assets.go if the k8s version is < 1.29. Added the function to get k8s version as well 7) Changes to test/it/controller/etcd/reconciler_test.go and test/it/crdvalidation/etcd/validation_test.go based on changes to test/it/assets/assets.go 8) Skip test cases based on k8s versions * renamed function skipTestBasedOnK8SVersion to skipCELTestsForOlderK8sVersions + added comment(description) to exported function * fixed incorrect invocation of prepare-chart-resources for e2e tests resulting in incorrect SANs * Updated the version-compatibility-matrix to remove supported versions of k8s for v0.28 * prepare chart resources will regenerate PKI artifacts when namespace changes * added check for unspecified k8s version --------- Co-authored-by: Shreyas Rao <[email protected]> Co-authored-by: Saketh Kalaga <[email protected]> Co-authored-by: madhav bhargava <[email protected]>
1 parent 1e47845 commit c083042

33 files changed

+3410
-151
lines changed

‎Makefile

+8-2
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ test-cov-clean:
107107

108108
.PHONY: test-e2e
109109
test-e2e: $(KUBECTL) $(HELM) $(SKAFFOLD) $(KUSTOMIZE) $(GINKGO)
110-
@$(HACK_DIR)/prepare-chart-resources.sh $(BUCKET_NAME) $(CERT_EXPIRY_DAYS)
110+
@$(HACK_DIR)/prepare-chart-resources.sh -n $(BUCKET_NAME) -e $(CERT_EXPIRY_DAYS)
111111
@VERSION=$(VERSION) GIT_SHA=$(GIT_SHA) $(HACK_DIR)/e2e-test/run-e2e-test.sh $(PROVIDERS)
112112

113113
.PHONY: ci-e2e-kind
@@ -177,9 +177,15 @@ ifndef NAMESPACE
177177
override NAMESPACE = default
178178
endif
179179

180+
# While preparing helm charts, it will by default attempt to get the k8s version by invoking kubectl command assuming that you are already connected to a k8s cluster.
181+
# If you wish to specify a specific k8s version, then set K8S_VERSION environment variable to a value of your choice.
180182
.PHONY: prepare-helm-charts
181183
prepare-helm-charts:
182-
@$(HACK_DIR)/prepare-chart-resources.sh $(NAMESPACE) $(CERT_EXPIRY_DAYS)
184+
ifeq ($(strip $(K8S_VERSION)),)
185+
@$(HACK_DIR)/prepare-chart-resources.sh --namespce $(NAMESPACE) --cert-expiry $(CERT_EXPIRY_DAYS)
186+
else
187+
@$(HACK_DIR)/prepare-chart-resources.sh --namespce $(NAMESPACE) --cert-expiry $(CERT_EXPIRY_DAYS) --k8s-version $(K8S_VERSION)
188+
endif
183189

184190
.PHONY: deploy
185191
deploy: $(SKAFFOLD) $(HELM) prepare-helm-charts

‎api/core/crds/crd.go

+40-6
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,56 @@
55
package crds
66

77
import (
8+
"k8s.io/apimachinery/pkg/util/version"
9+
810
_ "embed"
911
)
1012

1113
var (
1214
//go:embed druid.gardener.cloud_etcds.yaml
1315
etcdCRD string
16+
//go:embed druid.gardener.cloud_etcds_without_cel.yaml
17+
etcdCRDWithoutCEL string
1418
//go:embed druid.gardener.cloud_etcdcopybackupstasks.yaml
1519
etcdCopyBackupsTaskCRD string
1620
)
1721

18-
// GetEtcdCRD returns the etcd CRD.
19-
func GetEtcdCRD() string {
20-
return etcdCRD
22+
const (
23+
// KindEtcd is the kind of the etcd CRD.
24+
KindEtcd = "Etcd"
25+
// KindEtcdCopyBackupsTask is the kind of the etcd-copy-backup-task CRD.
26+
KindEtcdCopyBackupsTask = "EtcdCopyBackupsTask"
27+
)
28+
29+
// GetAll returns all CRDs for the given k8s version.
30+
// There are currently two sets of CRDs maintained.
31+
// 1. One set is for Kubernetes version 1.29 and above. These CRDs contain embedded CEL validation expressions. CEL expressions are only GA since Kubernetes 1.29 onwards.
32+
// 2. The other set is for Kubernetes versions below 1.29. These CRDs do not contain embedded CEL validation expressions.
33+
func GetAll(k8sVersion string) (map[string]string, error) {
34+
k8sVersionAbove129, err := IsK8sVersionEqualToOrAbove129(k8sVersion)
35+
if err != nil {
36+
return nil, err
37+
}
38+
var selectedEtcdCRD string
39+
if k8sVersionAbove129 {
40+
selectedEtcdCRD = etcdCRD
41+
} else {
42+
selectedEtcdCRD = etcdCRDWithoutCEL
43+
}
44+
return map[string]string{
45+
KindEtcd: selectedEtcdCRD,
46+
KindEtcdCopyBackupsTask: etcdCopyBackupsTaskCRD,
47+
}, nil
2148
}
2249

23-
// GetEtcdCopyBackupsTaskCRD returns the etcd-copy-backup-task CRD.
24-
func GetEtcdCopyBackupsTaskCRD() string {
25-
return etcdCopyBackupsTaskCRD
50+
// IsK8sVersionEqualToOrAbove129 returns true if the K8s version is 1.29 or higher, otherwise false.
51+
func IsK8sVersionEqualToOrAbove129(k8sVersion string) (bool, error) {
52+
v, err := version.ParseMajorMinor(k8sVersion)
53+
if err != nil {
54+
return false, err
55+
}
56+
if v.LessThan(version.MajorMinor(1, 29)) {
57+
return false, nil
58+
}
59+
return true, nil
2660
}

‎api/core/crds/crd_test.go

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// SPDX-FileCopyrightText: 2025 SAP SE or an SAP affiliate company and Gardener contributors
2+
//
3+
// SPDX-License-Identifier: Apache-2.0
4+
5+
package crds
6+
7+
import (
8+
"testing"
9+
10+
. "github.com/onsi/gomega"
11+
)
12+
13+
func TestGetAll(t *testing.T) {
14+
testCases := []struct {
15+
name string
16+
k8sVersion string
17+
expectedCRDs map[string]string
18+
}{
19+
{
20+
name: "k8s version is 1.29",
21+
k8sVersion: "1.29",
22+
expectedCRDs: map[string]string{
23+
KindEtcd: etcdCRD,
24+
KindEtcdCopyBackupsTask: etcdCopyBackupsTaskCRD,
25+
},
26+
},
27+
{
28+
name: "k8s version is v1.29.0",
29+
k8sVersion: "v1.29.0",
30+
expectedCRDs: map[string]string{
31+
KindEtcd: etcdCRD,
32+
KindEtcdCopyBackupsTask: etcdCopyBackupsTaskCRD,
33+
},
34+
},
35+
{
36+
name: "k8s version is below 1.29",
37+
k8sVersion: "1.28",
38+
expectedCRDs: map[string]string{
39+
KindEtcd: etcdCRDWithoutCEL,
40+
KindEtcdCopyBackupsTask: etcdCopyBackupsTaskCRD,
41+
},
42+
},
43+
{
44+
name: "k8s version is below v1.29",
45+
k8sVersion: "v1.28.3",
46+
expectedCRDs: map[string]string{
47+
KindEtcd: etcdCRDWithoutCEL,
48+
KindEtcdCopyBackupsTask: etcdCopyBackupsTaskCRD,
49+
},
50+
},
51+
{
52+
name: "k8s version is above 1.29",
53+
k8sVersion: "1.30",
54+
expectedCRDs: map[string]string{
55+
KindEtcd: etcdCRD,
56+
KindEtcdCopyBackupsTask: etcdCopyBackupsTaskCRD,
57+
},
58+
},
59+
{
60+
name: "k8s version is above v1.29",
61+
k8sVersion: "v1.30.1",
62+
expectedCRDs: map[string]string{
63+
KindEtcd: etcdCRD,
64+
KindEtcdCopyBackupsTask: etcdCopyBackupsTaskCRD,
65+
},
66+
},
67+
}
68+
69+
g := NewWithT(t)
70+
t.Parallel()
71+
for _, tc := range testCases {
72+
t.Run(tc.name, func(t *testing.T) {
73+
t.Parallel()
74+
crds, err := GetAll(tc.k8sVersion)
75+
g.Expect(err).To(BeNil())
76+
g.Expect(crds).To(HaveLen(len(tc.expectedCRDs)))
77+
g.Expect(crds).To(Equal(tc.expectedCRDs))
78+
})
79+
}
80+
81+
}

‎api/core/crds/druid.gardener.cloud_etcds.yaml

+37-2
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,13 @@ spec:
162162
deltaSnapshotPeriod:
163163
description: DeltaSnapshotPeriod defines the period after which
164164
delta snapshots will be taken
165+
pattern: ^([0-9]+([.][0-9]+)?h)?([0-9]+([.][0-9]+)?m)?([0-9]+([.][0-9]+)?s)?([0-9]+([.][0-9]+)?d)?$
165166
type: string
166167
deltaSnapshotRetentionPeriod:
167168
description: |-
168169
DeltaSnapshotRetentionPeriod defines the duration for which delta snapshots will be retained, excluding the latest snapshot set.
169170
The value should be a string formatted as a duration (e.g., '1s', '2m', '3h', '4d')
170-
pattern: ^([0-9][0-9]*([.][0-9]+)?(s|m|h|d))+$
171+
pattern: ^([0-9]+([.][0-9]+)?h)?([0-9]+([.][0-9]+)?m)?([0-9]+([.][0-9]+)?s)?([0-9]+([.][0-9]+)?d)?$
171172
type: string
172173
enableProfiling:
173174
description: EnableProfiling defines if profiling should be enabled
@@ -176,14 +177,17 @@ spec:
176177
etcdSnapshotTimeout:
177178
description: EtcdSnapshotTimeout defines the timeout duration
178179
for etcd FullSnapshot operation
180+
pattern: ^([0-9]+([.][0-9]+)?h)?([0-9]+([.][0-9]+)?m)?([0-9]+([.][0-9]+)?s)?([0-9]+([.][0-9]+)?d)?$
179181
type: string
180182
fullSnapshotSchedule:
181183
description: FullSnapshotSchedule defines the cron standard schedule
182184
for full snapshots.
185+
pattern: ^(\*|[1-5]?[0-9]|[1-5]?[0-9]-[1-5]?[0-9]|(?:[1-9]|[1-4][0-9]|5[0-9])\/(?:[1-9]|[1-4][0-9]|5[0-9]|60)|\*\/(?:[1-9]|[1-4][0-9]|5[0-9]|60))\s+(\*|[0-9]|1[0-9]|2[0-3]|[0-9]-(?:[0-9]|1[0-9]|2[0-3])|1[0-9]-(?:1[0-9]|2[0-3])|2[0-3]-2[0-3]|(?:[1-9]|1[0-9]|2[0-3])\/(?:[1-9]|1[0-9]|2[0-4])|\*\/(?:[1-9]|1[0-9]|2[0-4]))\s+(\*|[1-9]|[12][0-9]|3[01]|[1-9]-(?:[1-9]|[12][0-9]|3[01])|[12][0-9]-(?:[12][0-9]|3[01])|3[01]-3[01]|(?:[1-9]|[12][0-9]|30)\/(?:[1-9]|[12][0-9]|3[01])|\*\/(?:[1-9]|[12][0-9]|3[01]))\s+(\*|[1-9]|1[0-2]|[1-9]-(?:[1-9]|1[0-2])|1[0-2]-1[0-2]|(?:[1-9]|1[0-2])\/(?:[1-9]|1[0-2])|\*\/(?:[1-9]|1[0-2]))\s+(\*|[1-7]|[1-6]-[1-7]|[1-6]\/[1-7]|\*\/[1-7])$
183186
type: string
184187
garbageCollectionPeriod:
185188
description: GarbageCollectionPeriod defines the period for garbage
186189
collecting old backups
190+
pattern: ^([0-9]+([.][0-9]+)?h)?([0-9]+([.][0-9]+)?m)?([0-9]+([.][0-9]+)?s)?([0-9]+([.][0-9]+)?d)?$
187191
type: string
188192
garbageCollectionPolicy:
189193
description: GarbageCollectionPolicy defines the policy for garbage
@@ -202,10 +206,12 @@ spec:
202206
etcdConnectionTimeout:
203207
description: EtcdConnectionTimeout defines the timeout duration
204208
for etcd client connection during leader election.
209+
pattern: ^([0-9]+([.][0-9]+)?h)?([0-9]+([.][0-9]+)?m)?([0-9]+([.][0-9]+)?s)?([0-9]+([.][0-9]+)?d)?$
205210
type: string
206211
reelectionPeriod:
207212
description: ReelectionPeriod defines the Period after which
208213
leadership status of corresponding etcd is checked.
214+
pattern: ^([0-9]+([.][0-9]+)?h)?([0-9]+([.][0-9]+)?m)?([0-9]+([.][0-9]+)?s)?([0-9]+([.][0-9]+)?d)?$
209215
type: string
210216
type: object
211217
maxBackupsLimitBasedGC:
@@ -366,6 +372,11 @@ spec:
366372
- tlsCASecretRef
367373
type: object
368374
type: object
375+
x-kubernetes-validations:
376+
- message: etcd.spec.backup.garbageCollectionPeriod must be greater
377+
than etcd.spec.backup.deltaSnapshotPeriod
378+
rule: '!(has(self.deltaSnapshotPeriod) && has(self.garbageCollectionPeriod))
379+
|| duration(self.deltaSnapshotPeriod).getSeconds() < duration(self.garbageCollectionPeriod).getSeconds()'
369380
etcd:
370381
description: EtcdConfig defines the configuration for the etcd cluster
371382
to be deployed.
@@ -470,14 +481,17 @@ spec:
470481
defragmentationSchedule:
471482
description: DefragmentationSchedule defines the cron standard
472483
schedule for defragmentation of etcd.
484+
pattern: ^(\*|[1-5]?[0-9]|[1-5]?[0-9]-[1-5]?[0-9]|(?:[1-9]|[1-4][0-9]|5[0-9])\/(?:[1-9]|[1-4][0-9]|5[0-9]|60)|\*\/(?:[1-9]|[1-4][0-9]|5[0-9]|60))\s+(\*|[0-9]|1[0-9]|2[0-3]|[0-9]-(?:[0-9]|1[0-9]|2[0-3])|1[0-9]-(?:1[0-9]|2[0-3])|2[0-3]-2[0-3]|(?:[1-9]|1[0-9]|2[0-3])\/(?:[1-9]|1[0-9]|2[0-4])|\*\/(?:[1-9]|1[0-9]|2[0-4]))\s+(\*|[1-9]|[12][0-9]|3[01]|[1-9]-(?:[1-9]|[12][0-9]|3[01])|[12][0-9]-(?:[12][0-9]|3[01])|3[01]-3[01]|(?:[1-9]|[12][0-9]|30)\/(?:[1-9]|[12][0-9]|3[01])|\*\/(?:[1-9]|[12][0-9]|3[01]))\s+(\*|[1-9]|1[0-2]|[1-9]-(?:[1-9]|1[0-2])|1[0-2]-1[0-2]|(?:[1-9]|1[0-2])\/(?:[1-9]|1[0-2])|\*\/(?:[1-9]|1[0-2]))\s+(\*|[1-7]|[1-6]-[1-7]|[1-6]\/[1-7]|\*\/[1-7])$
473485
type: string
474486
etcdDefragTimeout:
475487
description: EtcdDefragTimeout defines the timeout duration for
476488
etcd defrag call
489+
pattern: ^([0-9]+([.][0-9]+)?h)?([0-9]+([.][0-9]+)?m)?([0-9]+([.][0-9]+)?s)?([0-9]+([.][0-9]+)?d)?$
477490
type: string
478491
heartbeatDuration:
479492
description: HeartbeatDuration defines the duration for members
480493
to send heartbeats. The default value is 10s.
494+
pattern: ^([0-9]+([.][0-9]+)?h)?([0-9]+([.][0-9]+)?m)?([0-9]+([.][0-9]+)?s)?$
481495
type: string
482496
image:
483497
description: Image defines the etcd container image and tag
@@ -634,6 +648,9 @@ spec:
634648
replicas:
635649
format: int32
636650
type: integer
651+
x-kubernetes-validations:
652+
- message: Replicas can either be increased or be downscaled to 0.
653+
rule: 'self==0 ? true : self < oldSelf ? false : true'
637654
schedulingConstraints:
638655
description: |-
639656
SchedulingConstraints defines the different scheduling constraints that must be applied to the
@@ -1753,6 +1770,7 @@ spec:
17531770
selector is a label query over pods that should match the replica count.
17541771
It must match the pod template's labels.
17551772
More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors
1773+
Deprecated: this field will be removed in the future.
17561774
properties:
17571775
matchExpressions:
17581776
description: matchExpressions is a list of label selector requirements.
@@ -1822,22 +1840,39 @@ spec:
18221840
description: StorageCapacity defines the size of persistent volume.
18231841
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
18241842
x-kubernetes-int-or-string: true
1843+
x-kubernetes-validations:
1844+
- message: etcd.spec.storageCapacity is an immutable field
1845+
rule: self == oldSelf
18251846
storageClass:
18261847
description: |-
18271848
StorageClass defines the name of the StorageClass required by the claim.
18281849
More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#class-1
18291850
type: string
1851+
x-kubernetes-validations:
1852+
- message: etcd.spec.storageClass is an immutable field
1853+
rule: self == oldSelf
18301854
volumeClaimTemplate:
18311855
description: VolumeClaimTemplate defines the volume claim template
18321856
to be created
18331857
type: string
1858+
x-kubernetes-validations:
1859+
- message: etcd.spec.VolumeClaimTemplate is an immutable field
1860+
rule: self == oldSelf
18341861
required:
18351862
- backup
18361863
- etcd
18371864
- labels
18381865
- replicas
1839-
- selector
18401866
type: object
1867+
x-kubernetes-validations:
1868+
- message: If backups are enabled, then value of etcd.spec.storageCapacity
1869+
must be 3 times the value of etcd.spec.etcd.quota or more. If backups
1870+
are disabled, then value of etcd.spec.storageCapacity must be the
1871+
value of etcd.spec.etcd.quota or more.
1872+
rule: 'has(self.storageCapacity) && has(self.etcd.quota) ? (has(self.backup.store)
1873+
? ((quantity(self.storageCapacity).isLessThan(quantity(self.etcd.quota).add(quantity(self.etcd.quota)).add(quantity(self.etcd.quota)))
1874+
) ? false : true): (quantity(self.storageCapacity).isLessThan(quantity(self.etcd.quota))
1875+
? false : true) ): true'
18411876
status:
18421877
description: EtcdStatus defines the observed state of Etcd.
18431878
properties:

0 commit comments

Comments
 (0)