[release-1.4] 🐛 controlplane: prevent nil pointer exception in kcp controller when in…

controlplane/kubeadm/internal/controllers/controller.go

@@ -588,35 +588,40 @@ func (r *KubeadmControlPlaneReconciler) syncMachines(ctx context.Context, contro
 			{"f:metadata", "f:annotations"},
 			{"f:metadata", "f:labels"},
 		}
-		infraMachine := controlPlane.InfraResources[machineName]
-		// Cleanup managed fields of all InfrastructureMachines to drop ownership of labels and annotations
-		// from "manager". We do this so that InfrastructureMachines that are created using the Create method
-		// can also work with SSA. Otherwise, labels and annotations would be co-owned by our "old" "manager"
-		// and "capi-kubeadmcontrolplane" and then we would not be able to e.g. drop labels and annotations.
-		if err := ssa.DropManagedFields(ctx, r.Client, infraMachine, kcpManagerName, labelsAndAnnotationsManagedFieldPaths); err != nil {
-			return errors.Wrapf(err, "failed to clean up managedFields of InfrastructureMachine %s", klog.KObj(infraMachine))
-		}
-		// Update in-place mutating fields on InfrastructureMachine.
-		if err := r.updateExternalObject(ctx, infraMachine, controlPlane.KCP, controlPlane.Cluster); err != nil {
-			return errors.Wrapf(err, "failed to update InfrastructureMachine %s", klog.KObj(infraMachine))
-		}
-
-		kubeadmConfig, ok := controlPlane.GetKubeadmConfig(machineName)
-		if !ok || kubeadmConfig == nil {
-			return errors.Wrapf(err, "failed to retrieve KubeadmConfig for machine %s", machineName)
-		}
-		// Note: Set the GroupVersionKind because updateExternalObject depends on it.
-		kubeadmConfig.SetGroupVersionKind(m.Spec.Bootstrap.ConfigRef.GroupVersionKind())
-		// Cleanup managed fields of all KubeadmConfigs to drop ownership of labels and annotations
-		// from "manager". We do this so that KubeadmConfigs that are created using the Create method
-		// can also work with SSA. Otherwise, labels and annotations would be co-owned by our "old" "manager"
-		// and "capi-kubeadmcontrolplane" and then we would not be able to e.g. drop labels and annotations.
-		if err := ssa.DropManagedFields(ctx, r.Client, kubeadmConfig, kcpManagerName, labelsAndAnnotationsManagedFieldPaths); err != nil {
-			return errors.Wrapf(err, "failed to clean up managedFields of KubeadmConfig %s", klog.KObj(kubeadmConfig))
-		}
-		// Update in-place mutating fields on BootstrapConfig.
-		if err := r.updateExternalObject(ctx, kubeadmConfig, controlPlane.KCP, controlPlane.Cluster); err != nil {
-			return errors.Wrapf(err, "failed to update KubeadmConfig %s", klog.KObj(kubeadmConfig))
+		infraMachine, infraMachineFound := controlPlane.InfraResources[machineName]
+		// Only update the InfraMachine if it is already found, otherwise just skip it.
+		// This could happen e.g. if the cache is not up-to-date yet.
+		if infraMachineFound {
+			// Cleanup managed fields of all InfrastructureMachines to drop ownership of labels and annotations
+			// from "manager". We do this so that InfrastructureMachines that are created using the Create method
+			// can also work with SSA. Otherwise, labels and annotations would be co-owned by our "old" "manager"
+			// and "capi-kubeadmcontrolplane" and then we would not be able to e.g. drop labels and annotations.
+			if err := ssa.DropManagedFields(ctx, r.Client, infraMachine, kcpManagerName, labelsAndAnnotationsManagedFieldPaths); err != nil {
+				return errors.Wrapf(err, "failed to clean up managedFields of InfrastructureMachine %s", klog.KObj(infraMachine))
+			}
+			// Update in-place mutating fields on InfrastructureMachine.
+			if err := r.updateExternalObject(ctx, infraMachine, controlPlane.KCP, controlPlane.Cluster); err != nil {
+				return errors.Wrapf(err, "failed to update InfrastructureMachine %s", klog.KObj(infraMachine))
+			}
+		}
+
+		kubeadmConfig, kubeadmConfigFound := controlPlane.KubeadmConfigs[machineName]
+		// Only update the KubeadmConfig if it is already found, otherwise just skip it.
+		// This could happen e.g. if the cache is not up-to-date yet.
+		if kubeadmConfigFound {
+			// Note: Set the GroupVersionKind because updateExternalObject depends on it.
+			kubeadmConfig.SetGroupVersionKind(m.Spec.Bootstrap.ConfigRef.GroupVersionKind())
+			// Cleanup managed fields of all KubeadmConfigs to drop ownership of labels and annotations
+			// from "manager". We do this so that KubeadmConfigs that are created using the Create method
+			// can also work with SSA. Otherwise, labels and annotations would be co-owned by our "old" "manager"
+			// and "capi-kubeadmcontrolplane" and then we would not be able to e.g. drop labels and annotations.
+			if err := ssa.DropManagedFields(ctx, r.Client, kubeadmConfig, kcpManagerName, labelsAndAnnotationsManagedFieldPaths); err != nil {
+				return errors.Wrapf(err, "failed to clean up managedFields of KubeadmConfig %s", klog.KObj(kubeadmConfig))
+			}
+			// Update in-place mutating fields on BootstrapConfig.
+			if err := r.updateExternalObject(ctx, kubeadmConfig, controlPlane.KCP, controlPlane.Cluster); err != nil {
+				return errors.Wrapf(err, "failed to update KubeadmConfig %s", klog.KObj(kubeadmConfig))
+			}
 		}
 	}
 	// Update the patch helpers.

controlplane/kubeadm/internal/controllers/controller_test.go

@@ -1512,6 +1512,32 @@ func TestKubeadmControlPlaneReconciler_syncMachines(t *testing.T) {
 		return !deletingMachine.DeletionTimestamp.IsZero()
 	}, 30*time.Second).Should(BeTrue())
 
+	// Existing machine that has a InfrastructureRef which does not exist.
+	nilInfraMachineMachine := &clusterv1.Machine{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: clusterv1.GroupVersion.String(),
+			Kind:       "Machine",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "nil-infra-machine-machine",
+			Namespace:   namespace.Name,
+			Labels:      map[string]string{},
+			Annotations: map[string]string{},
+			Finalizers:  []string{"testing-finalizer"},
+		},
+		Spec: clusterv1.MachineSpec{
+			ClusterName: testCluster.Name,
+			InfrastructureRef: corev1.ObjectReference{
+				Namespace: namespace.Name,
+			},
+			Bootstrap: clusterv1.Bootstrap{
+				DataSecretName: pointer.String("machine-bootstrap-secret"),
+			},
+		},
+	}
+	g.Expect(env.Create(ctx, nilInfraMachineMachine, client.FieldOwner(classicManager))).To(Succeed())
+	// Delete the machine to put it in the deleting state
+
 	kcp := &controlplanev1.KubeadmControlPlane{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "KubeadmControlPlane",
@@ -1557,6 +1583,7 @@ func TestKubeadmControlPlaneReconciler_syncMachines(t *testing.T) {
 		Machines: collections.Machines{
 			inPlaceMutatingMachine.Name: inPlaceMutatingMachine,
 			deletingMachine.Name:        deletingMachine,
+			nilInfraMachineMachine.Name: nilInfraMachineMachine,
 		},
 		KubeadmConfigs: map[string]*bootstrapv1.KubeadmConfig{
 			inPlaceMutatingMachine.Name: existingKubeadmConfig,