Merge pull request #9871 from sbueringer/pr-kcp-mutable-fields

✨ KCP: Allow mutation of all fields that should be mutable
kubernetes-sigs · Dec 15, 2023 · 2d58e1c · 2d58e1c
2 parents 32b2f3d + be30b77
commit 2d58e1c
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 35 deletions.
diff --git a/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go b/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go
@@ -159,12 +159,20 @@ func (webhook *KubeadmControlPlane) ValidateUpdate(_ context.Context, oldObj, ne
 	// add a * to indicate everything beneath is ok.
 	// For example, {"spec", "*"} will allow any path under "spec" to change.
 	allowedPaths := [][]string{
+		// metadata
 		{"metadata", "*"},
-		{spec, kubeadmConfigSpec, "useExperimentalRetryJoin"},
+		// spec.kubeadmConfigSpec.clusterConfiguration
 		{spec, kubeadmConfigSpec, clusterConfiguration, "etcd", "local", "imageRepository"},
 		{spec, kubeadmConfigSpec, clusterConfiguration, "etcd", "local", "imageTag"},
 		{spec, kubeadmConfigSpec, clusterConfiguration, "etcd", "local", "extraArgs"},
 		{spec, kubeadmConfigSpec, clusterConfiguration, "etcd", "local", "extraArgs", "*"},
+		{spec, kubeadmConfigSpec, clusterConfiguration, "etcd", "local", "dataDir"},
+		{spec, kubeadmConfigSpec, clusterConfiguration, "etcd", "local", "peerCertSANs"},
+		{spec, kubeadmConfigSpec, clusterConfiguration, "etcd", "local", "serverCertSANs"},
+		{spec, kubeadmConfigSpec, clusterConfiguration, "etcd", "external", "endpoints"},
+		{spec, kubeadmConfigSpec, clusterConfiguration, "etcd", "external", "caFile"},
+		{spec, kubeadmConfigSpec, clusterConfiguration, "etcd", "external", "certFile"},
+		{spec, kubeadmConfigSpec, clusterConfiguration, "etcd", "external", "keyFile"},
 		{spec, kubeadmConfigSpec, clusterConfiguration, "dns", "imageRepository"},
 		{spec, kubeadmConfigSpec, clusterConfiguration, "dns", "imageTag"},
 		{spec, kubeadmConfigSpec, clusterConfiguration, "imageRepository"},
@@ -174,16 +182,27 @@ func (webhook *KubeadmControlPlane) ValidateUpdate(_ context.Context, oldObj, ne
 		{spec, kubeadmConfigSpec, clusterConfiguration, controllerManager, "*"},
 		{spec, kubeadmConfigSpec, clusterConfiguration, scheduler},
 		{spec, kubeadmConfigSpec, clusterConfiguration, scheduler, "*"},
+		// spec.kubeadmConfigSpec.initConfiguration
 		{spec, kubeadmConfigSpec, initConfiguration, nodeRegistration},
 		{spec, kubeadmConfigSpec, initConfiguration, nodeRegistration, "*"},
 		{spec, kubeadmConfigSpec, initConfiguration, patches, directory},
 		{spec, kubeadmConfigSpec, initConfiguration, patches},
 		{spec, kubeadmConfigSpec, initConfiguration, skipPhases},
+		{spec, kubeadmConfigSpec, initConfiguration, "bootstrapTokens"},
+		{spec, kubeadmConfigSpec, initConfiguration, "localAPIEndpoint"},
+		{spec, kubeadmConfigSpec, initConfiguration, "localAPIEndpoint", "*"},
+		// spec.kubeadmConfigSpec.joinConfiguration
 		{spec, kubeadmConfigSpec, joinConfiguration, nodeRegistration},
 		{spec, kubeadmConfigSpec, joinConfiguration, nodeRegistration, "*"},
 		{spec, kubeadmConfigSpec, joinConfiguration, patches, directory},
 		{spec, kubeadmConfigSpec, joinConfiguration, patches},
 		{spec, kubeadmConfigSpec, joinConfiguration, skipPhases},
+		{spec, kubeadmConfigSpec, joinConfiguration, "caCertPath"},
+		{spec, kubeadmConfigSpec, joinConfiguration, "controlPlane"},
+		{spec, kubeadmConfigSpec, joinConfiguration, "controlPlane", "*"},
+		{spec, kubeadmConfigSpec, joinConfiguration, "discovery"},
+		{spec, kubeadmConfigSpec, joinConfiguration, "discovery", "*"},
+		// spec.kubeadmConfigSpec
 		{spec, kubeadmConfigSpec, preKubeadmCommands},
 		{spec, kubeadmConfigSpec, postKubeadmCommands},
 		{spec, kubeadmConfigSpec, files},
@@ -197,6 +216,8 @@ func (webhook *KubeadmControlPlane) ValidateUpdate(_ context.Context, oldObj, ne
 		{spec, kubeadmConfigSpec, diskSetup, "*"},
 		{spec, kubeadmConfigSpec, "format"},
 		{spec, kubeadmConfigSpec, "mounts"},
+		{spec, kubeadmConfigSpec, "useExperimentalRetryJoin"},
+		// spec.machineTemplate
 		{spec, "machineTemplate", "metadata"},
 		{spec, "machineTemplate", "metadata", "*"},
 		{spec, "machineTemplate", "infrastructureRef", "apiVersion"},
@@ -205,6 +226,7 @@ func (webhook *KubeadmControlPlane) ValidateUpdate(_ context.Context, oldObj, ne
 		{spec, "machineTemplate", "nodeDrainTimeout"},
 		{spec, "machineTemplate", "nodeVolumeDetachTimeout"},
 		{spec, "machineTemplate", "nodeDeletionTimeout"},
+		// spec
 		{spec, "replicas"},
 		{spec, "version"},
 		{spec, "remediationStrategy"},

diff --git a/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane_test.go b/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane_test.go
@@ -380,18 +380,12 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
 	wrongReplicaCountForScaleIn := before.DeepCopy()
 	wrongReplicaCountForScaleIn.Spec.RolloutStrategy.RollingUpdate.MaxSurge.IntVal = int32(0)
 
-	invalidUpdateKubeadmConfigInit := before.DeepCopy()
-	invalidUpdateKubeadmConfigInit.Spec.KubeadmConfigSpec.InitConfiguration = &bootstrapv1.InitConfiguration{}
-
 	validUpdateKubeadmConfigInit := before.DeepCopy()
 	validUpdateKubeadmConfigInit.Spec.KubeadmConfigSpec.InitConfiguration.NodeRegistration = bootstrapv1.NodeRegistrationOptions{}
 
 	invalidUpdateKubeadmConfigCluster := before.DeepCopy()
 	invalidUpdateKubeadmConfigCluster.Spec.KubeadmConfigSpec.ClusterConfiguration = &bootstrapv1.ClusterConfiguration{}
 
-	invalidUpdateKubeadmConfigJoin := before.DeepCopy()
-	invalidUpdateKubeadmConfigJoin.Spec.KubeadmConfigSpec.JoinConfiguration = &bootstrapv1.JoinConfiguration{}
-
 	validUpdateKubeadmConfigJoin := before.DeepCopy()
 	validUpdateKubeadmConfigJoin.Spec.KubeadmConfigSpec.JoinConfiguration.NodeRegistration = bootstrapv1.NodeRegistrationOptions{}
 
@@ -586,8 +580,6 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
 	localDataDir.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local = &bootstrapv1.LocalEtcd{
 		DataDir: "some local data dir",
 	}
-	modifyLocalDataDir := localDataDir.DeepCopy()
-	modifyLocalDataDir.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local.DataDir = "a different local data dir"
 
 	localPeerCertSANs := before.DeepCopy()
 	localPeerCertSANs.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local = &bootstrapv1.LocalEtcd{
@@ -726,12 +718,6 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
 			before:    before,
 			kcp:       validUpdate,
 		},
-		{
-			name:      "should return error when trying to mutate the kubeadmconfigspec initconfiguration",
-			expectErr: true,
-			before:    before,
-			kcp:       invalidUpdateKubeadmConfigInit,
-		},
 		{
 			name:      "should not return an error when trying to mutate the kubeadmconfigspec initconfiguration noderegistration",
 			expectErr: false,
@@ -744,12 +730,6 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
 			before:    before,
 			kcp:       invalidUpdateKubeadmConfigCluster,
 		},
-		{
-			name:      "should return error when trying to mutate the kubeadmconfigspec joinconfiguration",
-			expectErr: true,
-			before:    before,
-			kcp:       invalidUpdateKubeadmConfigJoin,
-		},
 		{
 			name:      "should not return an error when trying to mutate the kubeadmconfigspec joinconfiguration noderegistration",
 			expectErr: false,
@@ -912,20 +892,20 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
 			kcp:       featureGates,
 		},
 		{
-			name:      "should fail when making a change to the cluster config's local etcd's configuration localDataDir field",
-			expectErr: true,
+			name:      "should succeed when making a change to the cluster config's local etcd's configuration localDataDir field",
+			expectErr: false,
 			before:    before,
 			kcp:       localDataDir,
 		},
 		{
-			name:      "should fail when making a change to the cluster config's local etcd's configuration localPeerCertSANs field",
-			expectErr: true,
+			name:      "should succeed when making a change to the cluster config's local etcd's configuration localPeerCertSANs field",
+			expectErr: false,
 			before:    before,
 			kcp:       localPeerCertSANs,
 		},
 		{
-			name:      "should fail when making a change to the cluster config's local etcd's configuration localServerCertSANs field",
-			expectErr: true,
+			name:      "should succeed when making a change to the cluster config's local etcd's configuration localServerCertSANs field",
+			expectErr: false,
 			before:    before,
 			kcp:       localServerCertSANs,
 		},
@@ -936,8 +916,8 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
 			kcp:       localExtraArgs,
 		},
 		{
-			name:      "should fail when making a change to the cluster config's external etcd's configuration",
-			expectErr: true,
+			name:      "should succeed when making a change to the cluster config's external etcd's configuration",
+			expectErr: false,
 			before:    before,
 			kcp:       externalEtcd,
 		},
@@ -947,12 +927,6 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) {
 			before:    etcdLocalImageTag,
 			kcp:       unsetEtcd,
 		},
-		{
-			name:      "should fail when modifying a field that is not the local etcd image metadata",
-			expectErr: true,
-			before:    localDataDir,
-			kcp:       modifyLocalDataDir,
-		},
 		{
 			name:      "should fail if both local and external etcd are set",
 			expectErr: true,