Skip to content

Commit 2f4474a

Browse files
sbueringersbueringer
committed
KCP: improve validation webhooks
Signed-off-by: Stefan Büringer [email protected]
1 parent 691e5c4 commit 2f4474a

File tree

5 files changed

+49
-24
lines changed

5 files changed

+49
-24
lines changed

‎bootstrap/kubeadm/api/v1beta1/kubeadmconfig_webhook.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func (c *KubeadmConfig) ValidateDelete() error {
6363
}
6464

6565
func (c *KubeadmConfigSpec) validate(name string) error {
66-
allErrs := c.Validate()
66+
allErrs := c.Validate(field.NewPath("spec"))
6767

6868
if len(allErrs) == 0 {
6969
return nil
@@ -73,16 +73,16 @@ func (c *KubeadmConfigSpec) validate(name string) error {
7373
}
7474

7575
// Validate ensures the KubeadmConfigSpec is valid.
76-
func (c *KubeadmConfigSpec) Validate() field.ErrorList {
76+
func (c *KubeadmConfigSpec) Validate(pathPrefix *field.Path) field.ErrorList {
7777
var allErrs field.ErrorList
7878

79-
allErrs = append(allErrs, c.validateFiles()...)
80-
allErrs = append(allErrs, c.validateIgnition()...)
79+
allErrs = append(allErrs, c.validateFiles(pathPrefix)...)
80+
allErrs = append(allErrs, c.validateIgnition(pathPrefix)...)
8181

8282
return allErrs
8383
}
8484

85-
func (c *KubeadmConfigSpec) validateFiles() field.ErrorList {
85+
func (c *KubeadmConfigSpec) validateFiles(pathPrefix *field.Path) field.ErrorList {
8686
var allErrs field.ErrorList
8787

8888
knownPaths := map[string]struct{}{}
@@ -93,7 +93,7 @@ func (c *KubeadmConfigSpec) validateFiles() field.ErrorList {
9393
allErrs = append(
9494
allErrs,
9595
field.Invalid(
96-
field.NewPath("spec", "files").Index(i),
96+
pathPrefix.Child("files").Index(i),
9797
file,
9898
conflictingFileSourceMsg,
9999
),
@@ -107,7 +107,7 @@ func (c *KubeadmConfigSpec) validateFiles() field.ErrorList {
107107
allErrs = append(
108108
allErrs,
109109
field.Invalid(
110-
field.NewPath("spec", "files").Index(i).Child("contentFrom", "secret", "name"),
110+
pathPrefix.Child("files").Index(i).Child("contentFrom", "secret", "name"),
111111
file,
112112
missingSecretNameMsg,
113113
),
@@ -117,7 +117,7 @@ func (c *KubeadmConfigSpec) validateFiles() field.ErrorList {
117117
allErrs = append(
118118
allErrs,
119119
field.Invalid(
120-
field.NewPath("spec", "files").Index(i).Child("contentFrom", "secret", "key"),
120+
pathPrefix.Child("files").Index(i).Child("contentFrom", "secret", "key"),
121121
file,
122122
missingSecretKeyMsg,
123123
),
@@ -129,7 +129,7 @@ func (c *KubeadmConfigSpec) validateFiles() field.ErrorList {
129129
allErrs = append(
130130
allErrs,
131131
field.Invalid(
132-
field.NewPath("spec", "files").Index(i).Child("path"),
132+
pathPrefix.Child("files").Index(i).Child("path"),
133133
file,
134134
pathConflictMsg,
135135
),
@@ -141,18 +141,18 @@ func (c *KubeadmConfigSpec) validateFiles() field.ErrorList {
141141
return allErrs
142142
}
143143

144-
func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList {
144+
func (c *KubeadmConfigSpec) validateIgnition(pathPrefix *field.Path) field.ErrorList {
145145
var allErrs field.ErrorList
146146

147147
if !feature.Gates.Enabled(feature.KubeadmBootstrapFormatIgnition) {
148148
if c.Format == Ignition {
149149
allErrs = append(allErrs, field.Forbidden(
150-
field.NewPath("spec", "format"), kubeadmBootstrapFormatIgnitionFeatureDisabledMsg))
150+
pathPrefix.Child("format"), kubeadmBootstrapFormatIgnitionFeatureDisabledMsg))
151151
}
152152

153153
if c.Ignition != nil {
154154
allErrs = append(allErrs, field.Forbidden(
155-
field.NewPath("spec", "ignition"), kubeadmBootstrapFormatIgnitionFeatureDisabledMsg))
155+
pathPrefix.Child("ignition"), kubeadmBootstrapFormatIgnitionFeatureDisabledMsg))
156156
}
157157

158158
return allErrs
@@ -163,7 +163,7 @@ func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList {
163163
allErrs = append(
164164
allErrs,
165165
field.Invalid(
166-
field.NewPath("spec", "format"),
166+
pathPrefix.Child("format"),
167167
c.Format,
168168
fmt.Sprintf("must be set to %q if spec.ignition is set", Ignition),
169169
),
@@ -178,7 +178,7 @@ func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList {
178178
allErrs = append(
179179
allErrs,
180180
field.Forbidden(
181-
field.NewPath("spec", "users").Index(i).Child("inactive"),
181+
pathPrefix.Child("users").Index(i).Child("inactive"),
182182
cannotUseWithIgnition,
183183
),
184184
)
@@ -189,7 +189,7 @@ func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList {
189189
allErrs = append(
190190
allErrs,
191191
field.Forbidden(
192-
field.NewPath("spec", "useExperimentalRetryJoin"),
192+
pathPrefix.Child("useExperimentalRetryJoin"),
193193
cannotUseWithIgnition,
194194
),
195195
)
@@ -204,7 +204,7 @@ func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList {
204204
allErrs = append(
205205
allErrs,
206206
field.Invalid(
207-
field.NewPath("spec", "diskSetup", "partitions").Index(i).Child("tableType"),
207+
pathPrefix.Child("diskSetup", "partitions").Index(i).Child("tableType"),
208208
*partition.TableType,
209209
fmt.Sprintf(
210210
"only partition type %q is supported when spec.format is set to %q",
@@ -221,7 +221,7 @@ func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList {
221221
allErrs = append(
222222
allErrs,
223223
field.Forbidden(
224-
field.NewPath("spec", "diskSetup", "filesystems").Index(i).Child("replaceFS"),
224+
pathPrefix.Child("diskSetup", "filesystems").Index(i).Child("replaceFS"),
225225
cannotUseWithIgnition,
226226
),
227227
)
@@ -231,7 +231,7 @@ func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList {
231231
allErrs = append(
232232
allErrs,
233233
field.Forbidden(
234-
field.NewPath("spec", "diskSetup", "filesystems").Index(i).Child("partition"),
234+
pathPrefix.Child("diskSetup", "filesystems").Index(i).Child("partition"),
235235
cannotUseWithIgnition,
236236
),
237237
)

‎bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func (r *KubeadmConfigTemplate) ValidateDelete() error {
5252
func (r *KubeadmConfigTemplateSpec) validate(name string) error {
5353
var allErrs field.ErrorList
5454

55-
allErrs = append(allErrs, r.Template.Spec.Validate()...)
55+
allErrs = append(allErrs, r.Template.Spec.Validate(field.NewPath("spec", "template", "spec"))...)
5656

5757
if len(allErrs) == 0 {
5858
return nil

‎controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func (in *KubeadmControlPlane) ValidateCreate() error {
9999
spec := in.Spec
100100
allErrs := validateKubeadmControlPlaneSpec(spec, in.Namespace, field.NewPath("spec"))
101101
allErrs = append(allErrs, validateClusterConfiguration(spec.KubeadmConfigSpec.ClusterConfiguration, nil, field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration"))...)
102-
allErrs = append(allErrs, in.Spec.KubeadmConfigSpec.Validate()...)
102+
allErrs = append(allErrs, spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "kubeadmConfigSpec"))...)
103103
if len(allErrs) > 0 {
104104
return apierrors.NewInvalid(GroupVersion.WithKind("KubeadmControlPlane").GroupKind(), in.Name, allErrs)
105105
}
@@ -208,7 +208,7 @@ func (in *KubeadmControlPlane) ValidateUpdate(old runtime.Object) error {
208208
allErrs = append(allErrs, in.validateVersion(prev.Spec.Version)...)
209209
allErrs = append(allErrs, validateClusterConfiguration(in.Spec.KubeadmConfigSpec.ClusterConfiguration, prev.Spec.KubeadmConfigSpec.ClusterConfiguration, field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration"))...)
210210
allErrs = append(allErrs, in.validateCoreDNSVersion(prev)...)
211-
allErrs = append(allErrs, in.Spec.KubeadmConfigSpec.Validate()...)
211+
allErrs = append(allErrs, in.Spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "kubeadmConfigSpec"))...)
212212

213213
if len(allErrs) > 0 {
214214
return apierrors.NewInvalid(GroupVersion.WithKind("KubeadmControlPlane").GroupKind(), in.Name, allErrs)
@@ -371,6 +371,18 @@ func validateClusterConfiguration(newClusterConfiguration, oldClusterConfigurati
371371
)
372372
}
373373

374+
if newClusterConfiguration.DNS.ImageTag != "" {
375+
if _, err := version.ParseMajorMinorPatchTolerant(newClusterConfiguration.DNS.ImageTag); err != nil {
376+
allErrs = append(allErrs,
377+
field.Invalid(
378+
field.NewPath("dns", "imageTag"),
379+
newClusterConfiguration.DNS.ImageTag,
380+
fmt.Sprintf("failed to parse CoreDNS version: %v", err),
381+
),
382+
)
383+
}
384+
}
385+
374386
// TODO: Remove when kubeadm types include OpenAPI validation
375387
if newClusterConfiguration.Etcd.Local != nil && !container.ImageTagIsValid(newClusterConfiguration.Etcd.Local.ImageTag) {
376388
allErrs = append(
@@ -481,9 +493,10 @@ func (in *KubeadmControlPlane) validateCoreDNSVersion(prev *KubeadmControlPlane)
481493
fromVersion, err := version.ParseMajorMinorPatchTolerant(prev.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag)
482494
if err != nil {
483495
allErrs = append(allErrs,
484-
field.InternalError(
496+
field.Invalid(
485497
field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration", "dns", "imageTag"),
486-
fmt.Errorf("failed to parse CoreDNS current version: %v", prev.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag),
498+
prev.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag,
499+
fmt.Sprintf("failed to parse current CoreDNS version: %v", err),
487500
),
488501
)
489502
return allErrs
@@ -495,7 +508,7 @@ func (in *KubeadmControlPlane) validateCoreDNSVersion(prev *KubeadmControlPlane)
495508
field.Invalid(
496509
field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration", "dns", "imageTag"),
497510
targetDNS.ImageTag,
498-
fmt.Sprintf("failed to parse CoreDNS target version: %v", targetDNS.ImageTag),
511+
fmt.Sprintf("failed to parse target CoreDNS version: %v", err),
499512
),
500513
)
501514
return allErrs

‎controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) {
8383
Name: "infraTemplate",
8484
},
8585
},
86+
KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{
87+
ClusterConfiguration: &bootstrapv1.ClusterConfiguration{},
88+
},
8689
Replicas: pointer.Int32Ptr(1),
8790
Version: "v1.19.0",
8891
RolloutStrategy: &RolloutStrategy{
@@ -129,6 +132,9 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) {
129132
invalidVersion2 := valid.DeepCopy()
130133
invalidVersion2.Spec.Version = "1.16.6"
131134

135+
invalidCoreDNSVersion := valid.DeepCopy()
136+
invalidCoreDNSVersion.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag = "v1.7" // not a valid semantic version
137+
132138
invalidIgnitionConfiguration := valid.DeepCopy()
133139
invalidIgnitionConfiguration.Spec.KubeadmConfigSpec.Ignition = &bootstrapv1.IgnitionSpec{}
134140

@@ -187,6 +193,11 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) {
187193
expectErr: true,
188194
kcp: invalidVersion1,
189195
},
196+
{
197+
name: "should return error when given an invalid semantic CoreDNS version",
198+
expectErr: true,
199+
kcp: invalidCoreDNSVersion,
200+
},
190201
{
191202
name: "should return error when maxSurge is not 1",
192203
expectErr: true,

‎controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ func (r *KubeadmControlPlaneTemplate) ValidateCreate() error {
6464
spec := r.Spec.Template.Spec
6565
allErrs := validateKubeadmControlPlaneTemplateResourceSpec(spec, field.NewPath("spec", "template", "spec"))
6666
allErrs = append(allErrs, validateClusterConfiguration(spec.KubeadmConfigSpec.ClusterConfiguration, nil, field.NewPath("spec", "template", "spec", "kubeadmConfigSpec", "clusterConfiguration"))...)
67+
allErrs = append(allErrs, spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "template", "spec", "kubeadmConfigSpec"))...)
6768
if len(allErrs) > 0 {
6869
return apierrors.NewInvalid(GroupVersion.WithKind("KubeadmControlPlaneTemplate").GroupKind(), r.Name, allErrs)
6970
}

0 commit comments

Comments
 (0)