diff --git a/bootstrap/kubeadm/api/v1beta1/kubeadmconfig_webhook.go b/bootstrap/kubeadm/api/v1beta1/kubeadmconfig_webhook.go index 1f24357766af..99483c28fc88 100644 --- a/bootstrap/kubeadm/api/v1beta1/kubeadmconfig_webhook.go +++ b/bootstrap/kubeadm/api/v1beta1/kubeadmconfig_webhook.go @@ -63,7 +63,7 @@ func (c *KubeadmConfig) ValidateDelete() error { } func (c *KubeadmConfigSpec) validate(name string) error { - allErrs := c.Validate() + allErrs := c.Validate(field.NewPath("spec")) if len(allErrs) == 0 { return nil @@ -73,16 +73,16 @@ func (c *KubeadmConfigSpec) validate(name string) error { } // Validate ensures the KubeadmConfigSpec is valid. -func (c *KubeadmConfigSpec) Validate() field.ErrorList { +func (c *KubeadmConfigSpec) Validate(pathPrefix *field.Path) field.ErrorList { var allErrs field.ErrorList - allErrs = append(allErrs, c.validateFiles()...) - allErrs = append(allErrs, c.validateIgnition()...) + allErrs = append(allErrs, c.validateFiles(pathPrefix)...) + allErrs = append(allErrs, c.validateIgnition(pathPrefix)...) return allErrs } -func (c *KubeadmConfigSpec) validateFiles() field.ErrorList { +func (c *KubeadmConfigSpec) validateFiles(pathPrefix *field.Path) field.ErrorList { var allErrs field.ErrorList knownPaths := map[string]struct{}{} @@ -93,7 +93,7 @@ func (c *KubeadmConfigSpec) validateFiles() field.ErrorList { allErrs = append( allErrs, field.Invalid( - field.NewPath("spec", "files").Index(i), + pathPrefix.Child("files").Index(i), file, conflictingFileSourceMsg, ), @@ -106,9 +106,8 @@ func (c *KubeadmConfigSpec) validateFiles() field.ErrorList { if file.ContentFrom.Secret.Name == "" { allErrs = append( allErrs, - field.Invalid( - field.NewPath("spec", "files").Index(i).Child("contentFrom", "secret", "name"), - file, + field.Required( + pathPrefix.Child("files").Index(i).Child("contentFrom", "secret", "name"), missingSecretNameMsg, ), ) @@ -116,9 +115,8 @@ func (c *KubeadmConfigSpec) validateFiles() field.ErrorList { if file.ContentFrom.Secret.Key == "" { allErrs = append( allErrs, - field.Invalid( - field.NewPath("spec", "files").Index(i).Child("contentFrom", "secret", "key"), - file, + field.Required( + pathPrefix.Child("files").Index(i).Child("contentFrom", "secret", "key"), missingSecretKeyMsg, ), ) @@ -129,7 +127,7 @@ func (c *KubeadmConfigSpec) validateFiles() field.ErrorList { allErrs = append( allErrs, field.Invalid( - field.NewPath("spec", "files").Index(i).Child("path"), + pathPrefix.Child("files").Index(i).Child("path"), file, pathConflictMsg, ), @@ -141,18 +139,18 @@ func (c *KubeadmConfigSpec) validateFiles() field.ErrorList { return allErrs } -func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList { +func (c *KubeadmConfigSpec) validateIgnition(pathPrefix *field.Path) field.ErrorList { var allErrs field.ErrorList if !feature.Gates.Enabled(feature.KubeadmBootstrapFormatIgnition) { if c.Format == Ignition { allErrs = append(allErrs, field.Forbidden( - field.NewPath("spec", "format"), kubeadmBootstrapFormatIgnitionFeatureDisabledMsg)) + pathPrefix.Child("format"), kubeadmBootstrapFormatIgnitionFeatureDisabledMsg)) } if c.Ignition != nil { allErrs = append(allErrs, field.Forbidden( - field.NewPath("spec", "ignition"), kubeadmBootstrapFormatIgnitionFeatureDisabledMsg)) + pathPrefix.Child("ignition"), kubeadmBootstrapFormatIgnitionFeatureDisabledMsg)) } return allErrs @@ -163,7 +161,7 @@ func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList { allErrs = append( allErrs, field.Invalid( - field.NewPath("spec", "format"), + pathPrefix.Child("format"), c.Format, fmt.Sprintf("must be set to %q if spec.ignition is set", Ignition), ), @@ -178,7 +176,7 @@ func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList { allErrs = append( allErrs, field.Forbidden( - field.NewPath("spec", "users").Index(i).Child("inactive"), + pathPrefix.Child("users").Index(i).Child("inactive"), cannotUseWithIgnition, ), ) @@ -189,7 +187,7 @@ func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList { allErrs = append( allErrs, field.Forbidden( - field.NewPath("spec", "useExperimentalRetryJoin"), + pathPrefix.Child("useExperimentalRetryJoin"), cannotUseWithIgnition, ), ) @@ -204,7 +202,7 @@ func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList { allErrs = append( allErrs, field.Invalid( - field.NewPath("spec", "diskSetup", "partitions").Index(i).Child("tableType"), + pathPrefix.Child("diskSetup", "partitions").Index(i).Child("tableType"), *partition.TableType, fmt.Sprintf( "only partition type %q is supported when spec.format is set to %q", @@ -221,7 +219,7 @@ func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList { allErrs = append( allErrs, field.Forbidden( - field.NewPath("spec", "diskSetup", "filesystems").Index(i).Child("replaceFS"), + pathPrefix.Child("diskSetup", "filesystems").Index(i).Child("replaceFS"), cannotUseWithIgnition, ), ) @@ -231,7 +229,7 @@ func (c *KubeadmConfigSpec) validateIgnition() field.ErrorList { allErrs = append( allErrs, field.Forbidden( - field.NewPath("spec", "diskSetup", "filesystems").Index(i).Child("partition"), + pathPrefix.Child("diskSetup", "filesystems").Index(i).Child("partition"), cannotUseWithIgnition, ), ) diff --git a/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook.go b/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook.go index 4459f7fa0bdf..0b1f7bfa0f30 100644 --- a/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook.go +++ b/bootstrap/kubeadm/api/v1beta1/kubeadmconfigtemplate_webhook.go @@ -52,7 +52,7 @@ func (r *KubeadmConfigTemplate) ValidateDelete() error { func (r *KubeadmConfigTemplateSpec) validate(name string) error { var allErrs field.ErrorList - allErrs = append(allErrs, r.Template.Spec.Validate()...) + allErrs = append(allErrs, r.Template.Spec.Validate(field.NewPath("spec", "template", "spec"))...) if len(allErrs) == 0 { return nil diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go index 896ef5d0e36c..093bad87783d 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go @@ -99,7 +99,7 @@ func (in *KubeadmControlPlane) ValidateCreate() error { spec := in.Spec allErrs := validateKubeadmControlPlaneSpec(spec, in.Namespace, field.NewPath("spec")) allErrs = append(allErrs, validateClusterConfiguration(spec.KubeadmConfigSpec.ClusterConfiguration, nil, field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration"))...) - allErrs = append(allErrs, in.Spec.KubeadmConfigSpec.Validate()...) + allErrs = append(allErrs, spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "kubeadmConfigSpec"))...) if len(allErrs) > 0 { return apierrors.NewInvalid(GroupVersion.WithKind("KubeadmControlPlane").GroupKind(), in.Name, allErrs) } @@ -208,7 +208,7 @@ func (in *KubeadmControlPlane) ValidateUpdate(old runtime.Object) error { allErrs = append(allErrs, in.validateVersion(prev.Spec.Version)...) allErrs = append(allErrs, validateClusterConfiguration(in.Spec.KubeadmConfigSpec.ClusterConfiguration, prev.Spec.KubeadmConfigSpec.ClusterConfiguration, field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration"))...) allErrs = append(allErrs, in.validateCoreDNSVersion(prev)...) - allErrs = append(allErrs, in.Spec.KubeadmConfigSpec.Validate()...) + allErrs = append(allErrs, in.Spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "kubeadmConfigSpec"))...) if len(allErrs) > 0 { return apierrors.NewInvalid(GroupVersion.WithKind("KubeadmControlPlane").GroupKind(), in.Name, allErrs) @@ -371,6 +371,18 @@ func validateClusterConfiguration(newClusterConfiguration, oldClusterConfigurati ) } + if newClusterConfiguration.DNS.ImageTag != "" { + if _, err := version.ParseMajorMinorPatchTolerant(newClusterConfiguration.DNS.ImageTag); err != nil { + allErrs = append(allErrs, + field.Invalid( + field.NewPath("dns", "imageTag"), + newClusterConfiguration.DNS.ImageTag, + fmt.Sprintf("failed to parse CoreDNS version: %v", err), + ), + ) + } + } + // TODO: Remove when kubeadm types include OpenAPI validation if newClusterConfiguration.Etcd.Local != nil && !container.ImageTagIsValid(newClusterConfiguration.Etcd.Local.ImageTag) { allErrs = append( @@ -481,9 +493,10 @@ func (in *KubeadmControlPlane) validateCoreDNSVersion(prev *KubeadmControlPlane) fromVersion, err := version.ParseMajorMinorPatchTolerant(prev.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag) if err != nil { allErrs = append(allErrs, - field.InternalError( + field.Invalid( field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration", "dns", "imageTag"), - fmt.Errorf("failed to parse CoreDNS current version: %v", prev.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag), + prev.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag, + fmt.Sprintf("failed to parse current CoreDNS version: %v", err), ), ) return allErrs @@ -495,7 +508,7 @@ func (in *KubeadmControlPlane) validateCoreDNSVersion(prev *KubeadmControlPlane) field.Invalid( field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration", "dns", "imageTag"), targetDNS.ImageTag, - fmt.Sprintf("failed to parse CoreDNS target version: %v", targetDNS.ImageTag), + fmt.Sprintf("failed to parse target CoreDNS version: %v", err), ), ) return allErrs diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go index 9923c95274b9..f63d06d2870a 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go @@ -83,6 +83,9 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { Name: "infraTemplate", }, }, + KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ + ClusterConfiguration: &bootstrapv1.ClusterConfiguration{}, + }, Replicas: pointer.Int32Ptr(1), Version: "v1.19.0", RolloutStrategy: &RolloutStrategy{ @@ -129,6 +132,9 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { invalidVersion2 := valid.DeepCopy() invalidVersion2.Spec.Version = "1.16.6" + invalidCoreDNSVersion := valid.DeepCopy() + invalidCoreDNSVersion.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag = "v1.7" // not a valid semantic version + invalidIgnitionConfiguration := valid.DeepCopy() invalidIgnitionConfiguration.Spec.KubeadmConfigSpec.Ignition = &bootstrapv1.IgnitionSpec{} @@ -187,6 +193,11 @@ func TestKubeadmControlPlaneValidateCreate(t *testing.T) { expectErr: true, kcp: invalidVersion1, }, + { + name: "should return error when given an invalid semantic CoreDNS version", + expectErr: true, + kcp: invalidCoreDNSVersion, + }, { name: "should return error when maxSurge is not 1", expectErr: true, diff --git a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go b/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go index 29de74817e4b..feb0bb3abed4 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_webhook.go @@ -64,6 +64,7 @@ func (r *KubeadmControlPlaneTemplate) ValidateCreate() error { spec := r.Spec.Template.Spec allErrs := validateKubeadmControlPlaneTemplateResourceSpec(spec, field.NewPath("spec", "template", "spec")) allErrs = append(allErrs, validateClusterConfiguration(spec.KubeadmConfigSpec.ClusterConfiguration, nil, field.NewPath("spec", "template", "spec", "kubeadmConfigSpec", "clusterConfiguration"))...) + allErrs = append(allErrs, spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "template", "spec", "kubeadmConfigSpec"))...) if len(allErrs) > 0 { return apierrors.NewInvalid(GroupVersion.WithKind("KubeadmControlPlaneTemplate").GroupKind(), r.Name, allErrs) }