Skip to content

Commit 0954d21

Browse files
committed
Address review comments
1 parent 905f389 commit 0954d21

File tree

8 files changed

+83
-16
lines changed

8 files changed

+83
-16
lines changed

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,12 +182,12 @@ type ClusterConfiguration struct {
182182
FeatureGates map[string]bool `json:"featureGates,omitempty"`
183183

184184
// certificateValidityPeriodDays specifies the validity period for a non-CA certificate generated by kubeadm.
185-
// Default value: 3650 days (10 years) set by kubeadm.
185+
// If not specified, kubeadm will use a default of 3650 days (10 years).
186186
// +optional
187187
CertificateValidityPeriodDays *int32 `json:"certificateValidityPeriodDays,omitempty"`
188188

189189
// caCertificateValidityPeriodDays specifies the validity period for a CA certificate generated by kubeadm.
190-
// Default value: 3650 days (10 years) set by kubeadm.
190+
// If not specified, kubeadm will use a default of 3650 days (10 years).
191191
// +optional
192192
CACertificateValidityPeriodDays *int32 `json:"caCertificateValidityPeriodDays,omitempty"`
193193
}

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

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bootstrap/kubeadm/types/upstreamv1beta4/conversion.go

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@ limitations under the License.
1717
package upstreamv1beta4
1818

1919
import (
20+
"math"
21+
"time"
2022
"unsafe"
2123

24+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2225
apimachineryconversion "k8s.io/apimachinery/pkg/conversion"
2326
"k8s.io/utils/ptr"
2427
"sigs.k8s.io/controller-runtime/pkg/conversion"
@@ -68,8 +71,8 @@ func Convert_upstreamv1beta4_ClusterConfiguration_To_v1beta2_ClusterConfiguratio
6871
if err := autoConvert_upstreamv1beta4_ClusterConfiguration_To_v1beta2_ClusterConfiguration(in, out, s); err != nil {
6972
return err
7073
}
71-
out.CertificateValidityPeriodDays = clusterv1.ConvertToDays(in.CertificateValidityPeriod)
72-
out.CACertificateValidityPeriodDays = clusterv1.ConvertToSeconds(in.CACertificateValidityPeriod)
74+
out.CertificateValidityPeriodDays = convertToDays(in.CertificateValidityPeriod)
75+
out.CACertificateValidityPeriodDays = convertToDays(in.CACertificateValidityPeriod)
7376
return nil
7477
}
7578

@@ -78,8 +81,8 @@ func Convert_v1beta2_ClusterConfiguration_To_upstreamv1beta4_ClusterConfiguratio
7881
if err := autoConvert_v1beta2_ClusterConfiguration_To_upstreamv1beta4_ClusterConfiguration(in, out, s); err != nil {
7982
return err
8083
}
81-
out.CertificateValidityPeriod = clusterv1.ConvertFromDays(in.CertificateValidityPeriodDays)
82-
out.CACertificateValidityPeriod = clusterv1.ConvertFromSeconds(in.CACertificateValidityPeriodDays)
84+
out.CertificateValidityPeriod = convertFromDays(in.CertificateValidityPeriodDays)
85+
out.CACertificateValidityPeriod = convertFromDays(in.CACertificateValidityPeriodDays)
8386
return nil
8487
}
8588

@@ -316,3 +319,26 @@ func (src *ClusterConfiguration) GetAdditionalData(data *upstream.AdditionalData
316319
// NOTE: for kubeadm v1beta4 types we are not reading ControlPlaneComponentHealthCheckSeconds into additional data
317320
// because Cluster API types are aligned with kubeadm's v1beta4 API version.
318321
}
322+
323+
// convertToDays takes *metav1.Duration and returns a *int32.
324+
// Durations longer than MaxInt32 are capped.
325+
// NOTE: this is a util function intended only for usage in API conversions.
326+
func convertToDays(in *metav1.Duration) *int32 {
327+
if in == nil {
328+
return nil
329+
}
330+
days := math.Trunc(in.Hours() / 24)
331+
if days > math.MaxInt32 {
332+
return ptr.To[int32](math.MaxInt32)
333+
}
334+
return ptr.To(int32(days))
335+
}
336+
337+
// convertFromDays takes *int32 and returns a *metav1.Duration.
338+
// NOTE: this is a util function intended only for usage in API conversions.
339+
func convertFromDays(in *int32) *metav1.Duration {
340+
if in == nil {
341+
return nil
342+
}
343+
return ptr.To(metav1.Duration{Duration: time.Duration(*in) * time.Hour * 24})
344+
}

controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanetemplates.yaml

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

util/secret/certificates.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ func NewCertificatesForInitialControlPlane(config *bootstrapv1.ClusterConfigurat
7171
if config.CertificatesDir != "" {
7272
certificatesDir = config.CertificatesDir
7373
}
74-
if config.CertificateValidityPeriodDays != nil {
75-
validityPeriodDays = config.CertificateValidityPeriodDays
74+
if config.CACertificateValidityPeriodDays != nil {
75+
validityPeriodDays = config.CACertificateValidityPeriodDays
7676
}
7777
}
7878

util/secret/certificates_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,13 @@ package secret_test
1818

1919
import (
2020
"testing"
21+
"time"
2122

2223
. "github.com/onsi/gomega"
24+
"k8s.io/utils/ptr"
2325

2426
bootstrapv1 "sigs.k8s.io/cluster-api/api/bootstrap/kubeadm/v1beta2"
27+
"sigs.k8s.io/cluster-api/util/certs"
2528
"sigs.k8s.io/cluster-api/util/secret"
2629
)
2730

@@ -53,3 +56,41 @@ func TestNewControlPlaneJoinCertsAsFilesNotPanicsWhenEmpty(t *testing.T) {
5356
certs := secret.NewControlPlaneJoinCerts(config)
5457
g.Expect(certs.AsFiles()).To(BeEmpty())
5558
}
59+
60+
func TestNewCertificatesForInitialControlPlane(t *testing.T) {
61+
tests := []struct {
62+
name string
63+
expectedExpiry time.Time
64+
caCertificateValidityPeriodDays *int32
65+
}{
66+
{
67+
name: "should return default expiry if caCertificateValidityPeriodDays not set",
68+
expectedExpiry: time.Now().UTC().Add(time.Hour * 24 * 365 * 10), // 10 years.
69+
},
70+
{
71+
name: "should return expiry if caCertificateValidityPeriodDays set",
72+
expectedExpiry: time.Now().UTC().Add(time.Hour * 24 * 10), // 10 days.
73+
caCertificateValidityPeriodDays: ptr.To[int32](10),
74+
},
75+
}
76+
for _, test := range tests {
77+
t.Run(test.name, func(t *testing.T) {
78+
g := NewWithT(t)
79+
80+
clusterCerts := secret.NewCertificatesForInitialControlPlane(&bootstrapv1.ClusterConfiguration{
81+
CACertificateValidityPeriodDays: test.caCertificateValidityPeriodDays,
82+
})
83+
g.Expect(clusterCerts.Generate()).To(Succeed())
84+
caCert := clusterCerts.GetByPurpose(secret.ClusterCA)
85+
86+
g.Expect(caCert.KeyPair).NotTo(BeNil())
87+
g.Expect(caCert.KeyPair.Cert).NotTo(BeNil())
88+
89+
// Decode the cert
90+
decodedCert, err := certs.DecodeCertPEM(caCert.KeyPair.Cert)
91+
g.Expect(err).ToNot(HaveOccurred())
92+
93+
g.Expect(decodedCert.NotAfter).Should(BeTemporally("~", test.expectedExpiry, 1*time.Minute))
94+
})
95+
}
96+
}

0 commit comments

Comments
 (0)