Address reveiw comments

Author: Karthik-K-N

api/bootstrap/kubeadm/v1beta1/conversion.go

@@ -83,6 +83,18 @@ func RestoreKubeadmConfigSpec(restored *bootstrapv1.KubeadmConfigSpec, dst *boot
 		}
 		dst.JoinConfiguration.Timeouts = restored.JoinConfiguration.Timeouts
 	}
+	if restored.ClusterConfiguration != nil &&
+		(restored.ClusterConfiguration.CertificateValidityPeriodDays != nil || restored.ClusterConfiguration.CACertificateValidityPeriodDays != nil) {
+		if dst.ClusterConfiguration == nil {
+			dst.ClusterConfiguration = &bootstrapv1.ClusterConfiguration{}
+		}
+		if restored.ClusterConfiguration.CertificateValidityPeriodDays != nil {
+			dst.ClusterConfiguration.CertificateValidityPeriodDays = restored.ClusterConfiguration.CertificateValidityPeriodDays
+		}
+		if restored.ClusterConfiguration.CACertificateValidityPeriodDays != nil {
+			dst.ClusterConfiguration.CACertificateValidityPeriodDays = restored.ClusterConfiguration.CACertificateValidityPeriodDays
+		}
+	}
 }
 
 func RestoreBoolIntentKubeadmConfigSpec(src *KubeadmConfigSpec, dst *bootstrapv1.KubeadmConfigSpec, hasRestored bool, restored *bootstrapv1.KubeadmConfigSpec) error {

api/bootstrap/kubeadm/v1beta1/conversion_test.go

@@ -128,12 +128,6 @@ func hubKubeadmConfigSpec(in *bootstrapv1.KubeadmConfigSpec, c randfill.Continue
 		}
 		in.JoinConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds = initControlPlaneComponentHealthCheckSeconds
 	}
-
-	// Drop the field to avoid round trip errors as it exists only in v1beta2.
-	if in.ClusterConfiguration != nil {
-		in.ClusterConfiguration.CertificateValidityPeriodSeconds = nil
-		in.ClusterConfiguration.CACertificateValidityPeriodSeconds = nil
-	}
 }
 
 func hubNodeRegistrationOptions(in *bootstrapv1.NodeRegistrationOptions, c randfill.Continue) {

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

@@ -181,15 +181,15 @@ type ClusterConfiguration struct {
 	// +optional
 	FeatureGates map[string]bool `json:"featureGates,omitempty"`
 
-	// certificateValidityPeriodSeconds specifies the validity period for a non-CA certificate generated by kubeadm.
-	// Default value: 756,864,000 seconds -> 8760h (365 days * 24 hours = 1 year)
+	// certificateValidityPeriodDays specifies the validity period for a non-CA certificate generated by kubeadm.
+	// Default value: 3650 days (10 years) set by kubeadm.
 	// +optional
-	CertificateValidityPeriodSeconds *int32 `json:"certificateValidityPeriodSeconds,omitempty"`
+	CertificateValidityPeriodDays *int32 `json:"certificateValidityPeriodDays,omitempty"`
 
-	// caCertificateValidityPeriodSeconds specifies the validity period for a CA certificate generated by kubeadm.
-	// Default value: 756,864,000 seconds -> 87600h (365 days * 24 hours * 10 = 10 years)
+	// caCertificateValidityPeriodDays specifies the validity period for a CA certificate generated by kubeadm.
+	// Default value: 3650 days (10 years) set by kubeadm.
 	// +optional
-	CACertificateValidityPeriodSeconds *int32 `json:"caCertificateValidityPeriodSeconds,omitempty"`
+	CACertificateValidityPeriodDays *int32 `json:"caCertificateValidityPeriodDays,omitempty"`
 }
 
 // APIServer holds settings necessary for API server deployments in the cluster.

api/bootstrap/kubeadm/v1beta2/zz_generated.deepcopy.go

@@ -123,8 +123,8 @@ func hubKubeadmConfigSpec(in *bootstrapv1.KubeadmConfigSpec, c randfill.Continue
 		in.JoinConfiguration.Timeouts.ControlPlaneComponentHealthCheckSeconds = initControlPlaneComponentHealthCheckSeconds
 	}
 	if in.ClusterConfiguration != nil {
-		in.ClusterConfiguration.CertificateValidityPeriodSeconds = nil
-		in.ClusterConfiguration.CACertificateValidityPeriodSeconds = nil
+		in.ClusterConfiguration.CertificateValidityPeriodDays = nil
+		in.ClusterConfiguration.CACertificateValidityPeriodDays = nil
 	}
 }
 

api/controlplane/kubeadm/v1beta1/conversion_test.go

@@ -104,3 +104,26 @@ func Convert_Duration_To_Pointer_int32(in metav1.Duration, hasRestored bool, res
 	// Otherwise, if the value is not 0, convert to *value.
 	*out = ConvertToSeconds(&in)
 }
+
+// ConvertToDays takes *metav1.Duration and returns a *int32.
+// Durations longer than MaxInt32 are capped.
+// NOTE: this is a util function intended only for usage in API conversions.
+func ConvertToDays(in *metav1.Duration) *int32 {
+	if in == nil {
+		return nil
+	}
+	hours := math.Trunc(in.Hours())
+	if hours > math.MaxInt32 {
+		return ptr.To[int32](math.MaxInt32)
+	}
+	return ptr.To(int32(hours / 24))
+}
+
+// ConvertFromDays takes *int32 and returns a *metav1.Duration.
+// NOTE: this is a util function intended only for usage in API conversions.
+func ConvertFromDays(in *int32) *metav1.Duration {
+	if in == nil {
+		return nil
+	}
+	return ptr.To(metav1.Duration{Duration: time.Duration(*in) * time.Hour * 24})
+}

api/core/v1beta2/conversion.go

@@ -232,6 +232,6 @@ func hubBootstrapTokenDiscoveryFuzzer(obj *bootstrapv1.BootstrapTokenDiscovery,
 func hubClusterConfigurationFuzzer(obj *bootstrapv1.ClusterConfiguration, c randfill.Continue) {
 	c.FillNoCustom(obj)
 
-	obj.CertificateValidityPeriodSeconds = nil
-	obj.CACertificateValidityPeriodSeconds = nil
+	obj.CertificateValidityPeriodDays = nil
+	obj.CACertificateValidityPeriodDays = nil
 }

bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigs.yaml

@@ -65,12 +65,21 @@ func Convert_upstreamv1beta4_ClusterConfiguration_To_v1beta2_ClusterConfiguratio
 	// Following fields do not exist in CABPK v1beta1 version:
 	// - Proxy (Not supported yet)
 	// - EncryptionAlgorithm (Not supported yet)
-	// - CACertificateValidityPeriod (Not supported yet)
 	if err := autoConvert_upstreamv1beta4_ClusterConfiguration_To_v1beta2_ClusterConfiguration(in, out, s); err != nil {
 		return err
 	}
-	out.CertificateValidityPeriodSeconds = clusterv1.ConvertToSeconds(in.CertificateValidityPeriod)
-	out.CACertificateValidityPeriodSeconds = clusterv1.ConvertToSeconds(in.CACertificateValidityPeriod)
+	out.CertificateValidityPeriodDays = clusterv1.ConvertToDays(in.CertificateValidityPeriod)
+	out.CACertificateValidityPeriodDays = clusterv1.ConvertToSeconds(in.CACertificateValidityPeriod)
+	return nil
+}
+
+// Convert_v1beta2_ClusterConfiguration_To_upstreamv1beta4_ClusterConfiguration is an autogenerated conversion function.
+func Convert_v1beta2_ClusterConfiguration_To_upstreamv1beta4_ClusterConfiguration(in *bootstrapv1.ClusterConfiguration, out *ClusterConfiguration, s apimachineryconversion.Scope) error {
+	if err := autoConvert_v1beta2_ClusterConfiguration_To_upstreamv1beta4_ClusterConfiguration(in, out, s); err != nil {
+		return err
+	}
+	out.CertificateValidityPeriod = clusterv1.ConvertFromDays(in.CertificateValidityPeriodDays)
+	out.CACertificateValidityPeriod = clusterv1.ConvertFromSeconds(in.CACertificateValidityPeriodDays)
 	return nil
 }
 
@@ -307,13 +316,3 @@ func (src *ClusterConfiguration) GetAdditionalData(data *upstream.AdditionalData
 	// NOTE: for kubeadm v1beta4 types we are not reading ControlPlaneComponentHealthCheckSeconds into additional data
 	// because Cluster API types are aligned with kubeadm's v1beta4 API version.
 }
-
-// Convert_v1beta2_ClusterConfiguration_To_upstreamv1beta4_ClusterConfiguration is an autogenerated conversion function.
-func Convert_v1beta2_ClusterConfiguration_To_upstreamv1beta4_ClusterConfiguration(in *bootstrapv1.ClusterConfiguration, out *ClusterConfiguration, s apimachineryconversion.Scope) error {
-	if err := autoConvert_v1beta2_ClusterConfiguration_To_upstreamv1beta4_ClusterConfiguration(in, out, s); err != nil {
-		return err
-	}
-	out.CertificateValidityPeriod = clusterv1.ConvertFromSeconds(in.CertificateValidityPeriodSeconds)
-	out.CACertificateValidityPeriod = clusterv1.ConvertFromSeconds(in.CACertificateValidityPeriodSeconds)
-	return nil
-}