Merge pull request #7355 from sbueringer/pr-detect-cert-expiry-from-apiserver

🌱 Detect certificate expiry from kube-apiserver serving cert

Author: k8s-ci-robot

bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go

@@ -66,15 +66,8 @@ const (
 const (
 	// DefaultTokenTTL is the default TTL used for tokens.
 	DefaultTokenTTL = 15 * time.Minute
-
-	// This hard-coded duration matches the hard-coded value used by kubeadm certificate generation.
-	certificateExpiryDuration = 365 * 24 * time.Hour
 )
 
-// now returns the current time.
-// This is defined as a variable so that it can be overridden in unit tests.
-var now = time.Now
-
 // InitLocker is a lock that is used around kubeadm init.
 type InitLocker interface {
 	Lock(ctx context.Context, cluster *clusterv1.Cluster, machine *clusterv1.Machine) bool
@@ -511,10 +504,6 @@ func (r *KubeadmConfigReconciler) handleClusterNotInitialized(ctx context.Contex
 		return ctrl.Result{}, err
 	}
 
-	// Update the certificate expiration time in the config.
-	// This annotation will be used by KCP to trigger control plane machines rollout before the certificate generated on the machine are going to expire.
-	r.addCertificateExpiryAnnotation(scope.Config)
-
 	return ctrl.Result{}, nil
 }
 
@@ -716,9 +705,6 @@ func (r *KubeadmConfigReconciler) joinControlplane(ctx context.Context, scope *S
 		return ctrl.Result{}, err
 	}
 
-	// Update the certificate expiration time in the config.
-	r.addCertificateExpiryAnnotation(scope.Config)
-
 	return ctrl.Result{}, nil
 }
 
@@ -1036,19 +1022,3 @@ func (r *KubeadmConfigReconciler) storeBootstrapData(ctx context.Context, scope
 	conditions.MarkTrue(scope.Config, bootstrapv1.DataSecretAvailableCondition)
 	return nil
 }
-
-// addCertificateExpiryAnnotation sets the certificate expiration time as an
-// annotation on KubeadmConfig, if it doesn't exist already.
-// NOTE: the certificate expiry date stored in the annotation will be slightly different from the one
-// actually used in the certificates - that depends on the exact time kubeadm runs on the machine-,
-// but this approximation is acceptable given that it happens before the actual expiration date.
-func (r *KubeadmConfigReconciler) addCertificateExpiryAnnotation(config *bootstrapv1.KubeadmConfig) {
-	annotations := config.GetAnnotations()
-	if annotations == nil {
-		annotations = map[string]string{}
-	}
-	if _, ok := annotations[clusterv1.MachineCertificatesExpiryDateAnnotation]; !ok {
-		annotations[clusterv1.MachineCertificatesExpiryDateAnnotation] = now().Add(certificateExpiryDuration).Format(time.RFC3339)
-		config.SetAnnotations(annotations)
-	}
-}

bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller_test.go

@@ -2069,48 +2069,6 @@ func TestKubeadmConfigReconciler_ResolveUsers(t *testing.T) {
 	}
 }
 
-func TestKubeadmConfigReconciler_ReconcileCertificateExpiryTime(t *testing.T) {
-	fakeNow, _ := time.Parse(time.RFC3339, "2022-01-01T00:00:00Z")
-	now = func() time.Time {
-		return fakeNow
-	}
-	oneYearFromNow := "2023-01-01T00:00:00Z"
-	time2 := "2023-10-01T00:00:00Z"
-
-	tests := []struct {
-		name     string
-		cfg      *bootstrapv1.KubeadmConfig
-		wantTime string
-	}{
-		{
-			name:     "set the expiry time to one year from now if the expiry time is not set",
-			cfg:      &bootstrapv1.KubeadmConfig{},
-			wantTime: oneYearFromNow,
-		},
-		{
-			name: "do not change the expiry time if it is already set",
-			cfg: &bootstrapv1.KubeadmConfig{
-				ObjectMeta: metav1.ObjectMeta{
-					Annotations: map[string]string{
-						clusterv1.MachineCertificatesExpiryDateAnnotation: time2,
-					},
-				},
-			},
-			wantTime: time2,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			g := NewWithT(t)
-			k := &KubeadmConfigReconciler{}
-			k.addCertificateExpiryAnnotation(tt.cfg)
-			annotations := tt.cfg.GetAnnotations()
-			g.Expect(annotations[clusterv1.MachineCertificatesExpiryDateAnnotation]).To(Equal(tt.wantTime))
-		})
-	}
-}
-
 // test utils.
 
 // newWorkerMachineForCluster returns a Machine with the passed Cluster's information and a pre-configured name.

controlplane/kubeadm/internal/cluster.go

@@ -160,6 +160,7 @@ func (m *Management) GetWorkloadCluster(ctx context.Context, clusterKey client.O
 	}
 	tlsConfig.InsecureSkipVerify = true
 	return &Workload{
+		restConfig:          restConfig,
 		Client:              c,
 		CoreDNSMigrator:     &CoreDNSMigrator{},
 		etcdClientGenerator: NewEtcdClientGenerator(restConfig, tlsConfig, m.EtcdDialTimeout),

controlplane/kubeadm/internal/control_plane.go

@@ -244,6 +244,12 @@ func (c *ControlPlane) HasDeletingMachine() bool {
 	return len(c.Machines.Filter(collections.HasDeletionTimestamp)) > 0
 }
 
+// GetKubeadmConfig returns the KubeadmConfig of a given machine.
+func (c *ControlPlane) GetKubeadmConfig(machineName string) (*bootstrapv1.KubeadmConfig, bool) {
+	kubeadmConfig, ok := c.kubeadmConfigs[machineName]
+	return kubeadmConfig, ok
+}
+
 // MachinesNeedingRollout return a list of machines that need to be rolled out.
 func (c *ControlPlane) MachinesNeedingRollout() collections.Machines {
 	// Ignore machines to be deleted.

controlplane/kubeadm/internal/controllers/controller.go

@@ -333,6 +333,11 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster *
 		return result, err
 	}
 
+	// Reconcile certificate expiry for machines that don't have the expiry annotation on KubeadmConfig yet.
+	if result, err := r.reconcileCertificateExpiries(ctx, controlPlane); err != nil || !result.IsZero() {
+		return result, err
+	}
+
 	// Control plane machines rollout due to configuration changes (e.g. upgrades) takes precedence over other operations.
 	needRollout := controlPlane.MachinesNeedingRollout()
 	switch {
@@ -578,6 +583,71 @@ func (r *KubeadmControlPlaneReconciler) reconcileEtcdMembers(ctx context.Context
 	return ctrl.Result{}, nil
 }
 
+func (r *KubeadmControlPlaneReconciler) reconcileCertificateExpiries(ctx context.Context, controlPlane *internal.ControlPlane) (ctrl.Result, error) {
+	log := ctrl.LoggerFrom(ctx)
+
+	// Return if there are no KCP-owned control-plane machines.
+	if controlPlane.Machines.Len() == 0 {
+		return ctrl.Result{}, nil
+	}
+
+	// Ignore machines which are being deleted.
+	machines := controlPlane.Machines.Filter(collections.Not(collections.HasDeletionTimestamp))
+
+	workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(controlPlane.Cluster))
+	if err != nil {
+		return ctrl.Result{}, errors.Wrap(err, "failed to reconcile certificate expiries: cannot get remote client to workload cluster")
+	}
+
+	for _, m := range machines {
+		log = log.WithValues("Machine", klog.KObj(m))
+
+		kubeadmConfig, ok := controlPlane.GetKubeadmConfig(m.Name)
+		if !ok {
+			// Skip if the Machine doesn't have a KubeadmConfig.
+			continue
+		}
+
+		annotations := kubeadmConfig.GetAnnotations()
+		if _, ok := annotations[clusterv1.MachineCertificatesExpiryDateAnnotation]; ok {
+			// Skip if annotation is already set.
+			continue
+		}
+
+		if m.Status.NodeRef == nil {
+			// Skip if the Machine is still provisioning.
+			continue
+		}
+		nodeName := m.Status.NodeRef.Name
+		log = log.WithValues("Node", klog.KRef("", nodeName))
+
+		log.V(3).Info("Reconciling certificate expiry")
+		certificateExpiry, err := workloadCluster.GetAPIServerCertificateExpiry(ctx, kubeadmConfig, nodeName)
+		if err != nil {
+			return ctrl.Result{}, errors.Wrapf(err, "failed to reconcile certificate expiry for Machine/%s", m.Name)
+		}
+		expiry := certificateExpiry.Format(time.RFC3339)
+
+		log.V(2).Info(fmt.Sprintf("Setting certificate expiry to %s", expiry))
+		patchHelper, err := patch.NewHelper(kubeadmConfig, r.Client)
+		if err != nil {
+			return ctrl.Result{}, errors.Wrapf(err, "failed to reconcile certificate expiry for Machine/%s: failed to create PatchHelper for KubeadmConfig/%s", m.Name, kubeadmConfig.Name)
+		}
+
+		if annotations == nil {
+			annotations = map[string]string{}
+		}
+		annotations[clusterv1.MachineCertificatesExpiryDateAnnotation] = expiry
+		kubeadmConfig.SetAnnotations(annotations)
+
+		if err := patchHelper.Patch(ctx, kubeadmConfig); err != nil {
+			return ctrl.Result{}, errors.Wrapf(err, "failed to reconcile certificate expiry for Machine/%s: failed to patch KubeadmConfig/%s", m.Name, kubeadmConfig.Name)
+		}
+	}
+
+	return ctrl.Result{}, nil
+}
+
 func (r *KubeadmControlPlaneReconciler) adoptMachines(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, machines collections.Machines, cluster *clusterv1.Cluster) error {
 	// We do an uncached full quorum read against the KCP to avoid re-adopting Machines the garbage collector just intentionally orphaned
 	// See https://github.com/kubernetes/kubernetes/issues/42639