Merge pull request #3356 from fabriziopandini/fix-matchClusterConfigu…

…ration

🐛 KCP: fix problem in rollout detection logic when ClusterConfiguration is nil

controlplane/kubeadm/internal/machinefilters/machine_filters.go

@@ -274,17 +274,31 @@ func MatchesKubeadmBootstrapConfig(ctx context.Context, c client.Client, kcp *co
 // Users should use KCP.Spec.UpgradeAfter field to force a rollout in this case.
 func matchClusterConfiguration(kcp *controlplanev1.KubeadmControlPlane, machine *clusterv1.Machine) bool {
 	machineClusterConfigStr, ok := machine.GetAnnotations()[controlplanev1.KubeadmClusterConfigurationAnnotation]
-	if ok {
-		machineClusterConfig := &kubeadmv1.ClusterConfiguration{}
-		// ClusterConfiguration annotation is not correct, only solution is to rollout.
-		if err := json.Unmarshal([]byte(machineClusterConfigStr), machineClusterConfig); err != nil {
-			return false
-		}
-		return reflect.DeepEqual(machineClusterConfig, kcp.Spec.KubeadmConfigSpec.ClusterConfiguration)
+	if !ok {
+		// We don't have enough information to make a decision; don't' trigger a roll out.
+		return true
+	}
+
+	machineClusterConfig := &kubeadmv1.ClusterConfiguration{}
+	// ClusterConfiguration annotation is not correct, only solution is to rollout.
+	// The call to json.Unmarshal has to take a pointer to the pointer struct defined above,
+	// otherwise we won't be able to handle a nil ClusterConfiguration (that is serialized into "null").
+	// See https://github.com/kubernetes-sigs/cluster-api/issues/3353.
+	if err := json.Unmarshal([]byte(machineClusterConfigStr), &machineClusterConfig); err != nil {
+		return false
+	}
+
+	// If any of the compared values are nil, treat them the same as an empty ClusterConfiguration.
+	if machineClusterConfig == nil {
+		machineClusterConfig = &kubeadmv1.ClusterConfiguration{}
+	}
+	kcpLocalClusterConfiguration := kcp.Spec.KubeadmConfigSpec.ClusterConfiguration
+	if kcpLocalClusterConfiguration == nil {
+		kcpLocalClusterConfiguration = &kubeadmv1.ClusterConfiguration{}
 	}
 
-	// We don't have enough information to make a decision; don't' trigger a roll out.
-	return true
+	// Compare and return.
+	return reflect.DeepEqual(machineClusterConfig, kcpLocalClusterConfiguration)
 }
 
 // matchInitOrJoinConfiguration verifies if KCP and machine InitConfiguration or JoinConfiguration matches.

controlplane/kubeadm/internal/machinefilters/util_test.go

@@ -90,6 +90,22 @@ func TestMatchClusterConfiguration(t *testing.T) {
 		}
 		g.Expect(matchClusterConfiguration(kcp, m)).To(gomega.BeFalse())
 	})
+	t.Run("Return true if cluster configuration is nil (special case)", func(t *testing.T) {
+		g := gomega.NewWithT(t)
+		kcp := &controlplanev1.KubeadmControlPlane{
+			Spec: controlplanev1.KubeadmControlPlaneSpec{
+				KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{},
+			},
+		}
+		m := &clusterv1.Machine{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{
+					controlplanev1.KubeadmClusterConfigurationAnnotation: "null",
+				},
+			},
+		}
+		g.Expect(matchClusterConfiguration(kcp, m)).To(gomega.BeTrue())
+	})
 }
 
 func TestGetAdjustedKcpConfig(t *testing.T) {