From cb16bed4bf24fabc5f3ab8fd6384d4a38e08c333 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Wed, 13 Dec 2023 10:19:57 +0100 Subject: [PATCH] KCP: Allow mutation of all fields that should be mutable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stefan Büringer buringerst@vmware.com --- .../v1beta1/kubeadm_control_plane_webhook.go | 24 ++++++++++- .../kubeadm_control_plane_webhook_test.go | 42 ++++--------------- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go index 862ffd568984..9ba8768eb95d 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go @@ -139,12 +139,20 @@ func (in *KubeadmControlPlane) ValidateUpdate(old runtime.Object) (admission.War // 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"}, @@ -154,16 +162,27 @@ func (in *KubeadmControlPlane) ValidateUpdate(old runtime.Object) (admission.War {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}, @@ -177,6 +196,8 @@ func (in *KubeadmControlPlane) ValidateUpdate(old runtime.Object) (admission.War {spec, kubeadmConfigSpec, diskSetup, "*"}, {spec, kubeadmConfigSpec, "format"}, {spec, kubeadmConfigSpec, "mounts"}, + {spec, kubeadmConfigSpec, "useExperimentalRetryJoin"}, + // spec.machineTemplate {spec, "machineTemplate", "metadata"}, {spec, "machineTemplate", "metadata", "*"}, {spec, "machineTemplate", "infrastructureRef", "apiVersion"}, @@ -185,6 +206,7 @@ func (in *KubeadmControlPlane) ValidateUpdate(old runtime.Object) (admission.War {spec, "machineTemplate", "nodeDrainTimeout"}, {spec, "machineTemplate", "nodeVolumeDetachTimeout"}, {spec, "machineTemplate", "nodeDeletionTimeout"}, + // spec {spec, "replicas"}, {spec, "version"}, {spec, "remediationStrategy"}, diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go index b5d678d899b6..fb4ab68b9ac8 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go @@ -353,18 +353,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{} @@ -559,8 +553,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{ @@ -689,12 +681,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, @@ -707,12 +693,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, @@ -875,20 +855,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, }, @@ -899,8 +879,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, }, @@ -910,12 +890,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,