Merge pull request #12609 from sbueringer/pr-fix-kcfg-validate

k8s-ci-robot · web-flow · commit 0958e05275cd · 2025-08-11T10:15:08.000-07:00
🐛 Fix ControlPlaneComponentHealthCheckSeconds validation in KubeadmConfigSpec.Validate
diff --git a/api/bootstrap/kubeadm/v1beta2/kubeadmconfig_types.go b/api/bootstrap/kubeadm/v1beta2/kubeadmconfig_types.go
@@ -143,7 +143,7 @@ type KubeadmConfigSpec struct {
 }
 
 // Validate ensures the KubeadmConfigSpec is valid.
-func (c *KubeadmConfigSpec) Validate(pathPrefix *field.Path) field.ErrorList {
+func (c *KubeadmConfigSpec) Validate(isKCP bool, pathPrefix *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
 
 	allErrs = append(allErrs, c.validateFiles(pathPrefix)...)
@@ -166,29 +166,34 @@ func (c *KubeadmConfigSpec) Validate(pathPrefix *field.Path) field.ErrorList {
 		}
 	}
 
-	// Validate timeouts
-	// Note: When v1beta1 will be removed, we can drop this limitation.
-	tInit := "unset"
-	if c.InitConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds != nil {
-		tInit = fmt.Sprintf("%d", *c.InitConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds)
-	}
-	tJoin := "unset"
-	if c.JoinConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds != nil {
-		tJoin = fmt.Sprintf("%d", *c.JoinConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds)
-	}
-	if tInit != tJoin {
-		allErrs = append(allErrs,
-			field.Invalid(
-				pathPrefix.Child("initConfiguration", "timeouts", "controlPlaneComponentHealthCheckSeconds"),
-				tInit,
-				fmt.Sprintf("controlPlaneComponentHealthCheckSeconds must be set to the same value both in initConfiguration.timeouts (%s) and in joinConfiguration.timeouts (%s)", tInit, tJoin),
-			),
-			field.Invalid(
-				pathPrefix.Child("joinConfiguration", "timeouts", "controlPlaneComponentHealthCheckSeconds"),
-				tJoin,
-				fmt.Sprintf("controlPlaneComponentHealthCheckSeconds must be set to the same value both in initConfiguration.timeouts (%s) and in joinConfiguration.timeouts (%s)", tInit, tJoin),
-			),
-		)
+	// Only ensure ControlPlaneComponentHealthCheckSeconds fields are equal for KubeadmControlPlane and KubeadmControlPlaneTemplate.
+	// In KubeadmConfig objects usually only one of InitConfiguration or JoinConfiguration is defined as a Machine uses
+	// either kubeadm init or kubeadm join, but not both.
+	if isKCP {
+		// Validate timeouts
+		// Note: When v1beta1 will be removed, we can drop this limitation.
+		tInit := "unset"
+		if c.InitConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds != nil {
+			tInit = fmt.Sprintf("%d", *c.InitConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds)
+		}
+		tJoin := "unset"
+		if c.JoinConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds != nil {
+			tJoin = fmt.Sprintf("%d", *c.JoinConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds)
+		}
+		if tInit != tJoin {
+			allErrs = append(allErrs,
+				field.Invalid(
+					pathPrefix.Child("initConfiguration", "timeouts", "controlPlaneComponentHealthCheckSeconds"),
+					tInit,
+					fmt.Sprintf("controlPlaneComponentHealthCheckSeconds must be set to the same value both in initConfiguration.timeouts (%s) and in joinConfiguration.timeouts (%s)", tInit, tJoin),
+				),
+				field.Invalid(
+					pathPrefix.Child("joinConfiguration", "timeouts", "controlPlaneComponentHealthCheckSeconds"),
+					tJoin,
+					fmt.Sprintf("controlPlaneComponentHealthCheckSeconds must be set to the same value both in initConfiguration.timeouts (%s) and in joinConfiguration.timeouts (%s)", tInit, tJoin),
+				),
+			)
+		}
 	}
 
 	return allErrs
diff --git a/bootstrap/kubeadm/internal/webhooks/kubeadmconfig.go b/bootstrap/kubeadm/internal/webhooks/kubeadmconfig.go
@@ -70,7 +70,7 @@ func (webhook *KubeadmConfig) ValidateDelete(_ context.Context, _ runtime.Object
 }
 
 func (webhook *KubeadmConfig) validate(c bootstrapv1.KubeadmConfigSpec, name string) error {
-	allErrs := c.Validate(field.NewPath("spec"))
+	allErrs := c.Validate(false, field.NewPath("spec"))
 
 	if len(allErrs) == 0 {
 		return nil
diff --git a/bootstrap/kubeadm/internal/webhooks/kubeadmconfig_test.go b/bootstrap/kubeadm/internal/webhooks/kubeadmconfig_test.go
@@ -450,7 +450,7 @@ func TestKubeadmConfigValidate(t *testing.T) {
 				},
 			},
 		},
-		"invalid ControlPlaneComponentHealthCheckSeconds": {
+		"valid ControlPlaneComponentHealthCheckSeconds (JoinConfiguration not defined)": {
 			in: &bootstrapv1.KubeadmConfig{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "baz",
@@ -464,7 +464,23 @@ func TestKubeadmConfigValidate(t *testing.T) {
 					},
 				},
 			},
-			expectErr: true,
+			expectErr: false,
+		},
+		"valid ControlPlaneComponentHealthCheckSeconds (InitConfiguration not defined)": {
+			in: &bootstrapv1.KubeadmConfig{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "baz",
+					Namespace: metav1.NamespaceDefault,
+				},
+				Spec: bootstrapv1.KubeadmConfigSpec{
+					JoinConfiguration: bootstrapv1.JoinConfiguration{
+						Timeouts: bootstrapv1.Timeouts{
+							ControlPlaneComponentHealthCheckSeconds: ptr.To[int32](10),
+						},
+					},
+				},
+			},
+			expectErr: false,
 		},
 		"valid ControlPlaneComponentHealthCheckSeconds": {
 			in: &bootstrapv1.KubeadmConfig{
diff --git a/bootstrap/kubeadm/internal/webhooks/kubeadmconfigtemplate.go b/bootstrap/kubeadm/internal/webhooks/kubeadmconfigtemplate.go
@@ -94,7 +94,7 @@ func (webhook *KubeadmConfigTemplate) ValidateDelete(_ context.Context, _ runtim
 func (webhook *KubeadmConfigTemplate) validate(r *bootstrapv1.KubeadmConfigTemplateSpec, name string) error {
 	var allErrs field.ErrorList
 
-	allErrs = append(allErrs, r.Template.Spec.Validate(field.NewPath("spec", "template", "spec"))...)
+	allErrs = append(allErrs, r.Template.Spec.Validate(false, field.NewPath("spec", "template", "spec"))...)
 	// Validate the metadata of the template.
 	allErrs = append(allErrs, r.Template.ObjectMeta.Validate(field.NewPath("spec", "template", "metadata"))...)
 
diff --git a/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go b/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go
@@ -99,7 +99,7 @@ func (webhook *KubeadmControlPlane) ValidateCreate(_ context.Context, obj runtim
 	spec := k.Spec
 	allErrs := validateKubeadmControlPlaneSpec(spec, field.NewPath("spec"))
 	allErrs = append(allErrs, validateClusterConfiguration(nil, &spec.KubeadmConfigSpec.ClusterConfiguration, field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration"))...)
-	allErrs = append(allErrs, spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "kubeadmConfigSpec"))...)
+	allErrs = append(allErrs, spec.KubeadmConfigSpec.Validate(true, field.NewPath("spec", "kubeadmConfigSpec"))...)
 	if len(allErrs) > 0 {
 		return nil, apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("KubeadmControlPlane").GroupKind(), k.Name, allErrs)
 	}
@@ -261,7 +261,7 @@ func (webhook *KubeadmControlPlane) ValidateUpdate(_ context.Context, oldObj, ne
 	allErrs = append(allErrs, webhook.validateVersion(oldK, newK)...)
 	allErrs = append(allErrs, validateClusterConfiguration(&oldK.Spec.KubeadmConfigSpec.ClusterConfiguration, &newK.Spec.KubeadmConfigSpec.ClusterConfiguration, field.NewPath("spec", "kubeadmConfigSpec", "clusterConfiguration"))...)
 	allErrs = append(allErrs, webhook.validateCoreDNSVersion(oldK, newK)...)
-	allErrs = append(allErrs, newK.Spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "kubeadmConfigSpec"))...)
+	allErrs = append(allErrs, newK.Spec.KubeadmConfigSpec.Validate(true, field.NewPath("spec", "kubeadmConfigSpec"))...)
 
 	if len(allErrs) > 0 {
 		return nil, apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("KubeadmControlPlane").GroupKind(), newK.Name, allErrs)
diff --git a/controlplane/kubeadm/internal/webhooks/kubeadmcontrolplanetemplate.go b/controlplane/kubeadm/internal/webhooks/kubeadmcontrolplanetemplate.go
@@ -69,7 +69,7 @@ func (webhook *KubeadmControlPlaneTemplate) ValidateCreate(_ context.Context, ob
 	spec := k.Spec.Template.Spec
 	allErrs := validateKubeadmControlPlaneTemplateResourceSpec(spec, field.NewPath("spec", "template", "spec"))
 	allErrs = append(allErrs, validateClusterConfiguration(nil, &spec.KubeadmConfigSpec.ClusterConfiguration, field.NewPath("spec", "template", "spec", "kubeadmConfigSpec", "clusterConfiguration"))...)
-	allErrs = append(allErrs, spec.KubeadmConfigSpec.Validate(field.NewPath("spec", "template", "spec", "kubeadmConfigSpec"))...)
+	allErrs = append(allErrs, spec.KubeadmConfigSpec.Validate(true, field.NewPath("spec", "template", "spec", "kubeadmConfigSpec"))...)
 	// Validate the metadata of the KubeadmControlPlaneTemplateResource
 	allErrs = append(allErrs, k.Spec.Template.ObjectMeta.Validate(field.NewPath("spec", "template", "metadata"))...)
 	if len(allErrs) > 0 {
diff --git a/test/e2e/data/infrastructure-docker/main/clusterclass-quick-start-runtimesdk-v1beta1.yaml b/test/e2e/data/infrastructure-docker/main/clusterclass-quick-start-runtimesdk-v1beta1.yaml
@@ -98,6 +98,7 @@ spec:
       kubeadmConfigSpec:
         clusterConfiguration:
           apiServer:
+            timeoutForControlPlane: 20m
             # host.docker.internal is required by kubetest when running on MacOS because of the way ports are proxied.
             certSANs: [localhost, 127.0.0.1, 0.0.0.0, host.docker.internal]
         initConfiguration:

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ func (webhook *KubeadmConfig) ValidateDelete(_ context.Context, _ runtime.Object`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`func (webhook *KubeadmConfig) validate(c bootstrapv1.KubeadmConfigSpec, name string) error {`
`73`		`- allErrs := c.Validate(field.NewPath("spec"))`
	`73`	`+ allErrs := c.Validate(false, field.NewPath("spec"))`
`74`	`74`
`75`	`75`	`if len(allErrs) == 0 {`
`76`	`76`	`return nil`