diff --git a/controlplane/kubeadm/controllers/controller.go b/controlplane/kubeadm/controllers/controller.go index 4705f08aa97a..a737347992a3 100644 --- a/controlplane/kubeadm/controllers/controller.go +++ b/controlplane/kubeadm/controllers/controller.go @@ -265,7 +265,7 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster * } // Generate Cluster Kubeconfig if needed - if err := r.reconcileKubeconfig(ctx, util.ObjectKey(cluster), cluster.Spec.ControlPlaneEndpoint, kcp); err != nil { + if err := r.reconcileKubeconfig(ctx, cluster, kcp); err != nil { log.Error(err, "failed to reconcile Kubeconfig") return ctrl.Result{}, err } diff --git a/controlplane/kubeadm/controllers/helpers.go b/controlplane/kubeadm/controllers/helpers.go index 9287e56647e8..907875730e2d 100644 --- a/controlplane/kubeadm/controllers/helpers.go +++ b/controlplane/kubeadm/controllers/helpers.go @@ -19,7 +19,6 @@ package controllers import ( "context" "encoding/json" - "sigs.k8s.io/cluster-api/util/conditions" "strings" "github.com/pkg/errors" @@ -37,21 +36,23 @@ import ( capierrors "sigs.k8s.io/cluster-api/errors" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/certs" + "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/kubeconfig" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/cluster-api/util/secret" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" ) -func (r *KubeadmControlPlaneReconciler) reconcileKubeconfig(ctx context.Context, clusterName client.ObjectKey, endpoint clusterv1.APIEndpoint, kcp *controlplanev1.KubeadmControlPlane) error { +func (r *KubeadmControlPlaneReconciler) reconcileKubeconfig(ctx context.Context, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane) error { log := ctrl.LoggerFrom(ctx) + endpoint := cluster.Spec.ControlPlaneEndpoint if endpoint.IsZero() { return nil } controllerOwnerRef := *metav1.NewControllerRef(kcp, controlplanev1.GroupVersion.WithKind("KubeadmControlPlane")) + clusterName := util.ObjectKey(cluster) configSecret, err := secret.GetFromNamespacedName(ctx, r.Client, clusterName, secret.Kubeconfig) switch { case apierrors.IsNotFound(err): @@ -72,6 +73,14 @@ func (r *KubeadmControlPlaneReconciler) reconcileKubeconfig(ctx context.Context, return errors.Wrap(err, "failed to retrieve kubeconfig Secret") } + // check if the kubeconfig secret was created by v1alpha2 controllers, and thus it has the Cluster as the owner instead of KCP; + // if yes, adopt it. + if util.IsOwnedByObject(configSecret, cluster) && !util.IsControlledBy(configSecret, kcp) { + if err := r.adoptKubeconfigSecret(ctx, cluster, configSecret, controllerOwnerRef); err != nil { + return err + } + } + // only do rotation on owned secrets if !util.IsControlledBy(configSecret, kcp) { return nil @@ -92,6 +101,27 @@ func (r *KubeadmControlPlaneReconciler) reconcileKubeconfig(ctx context.Context, return nil } +func (r *KubeadmControlPlaneReconciler) adoptKubeconfigSecret(ctx context.Context, cluster *clusterv1.Cluster, configSecret *corev1.Secret, controllerOwnerRef metav1.OwnerReference) error { + log := ctrl.LoggerFrom(ctx) + log.Info("Adopting KubeConfig secret created by v1alpha2 controllers", "Name", configSecret.Name) + + patch, err := patch.NewHelper(configSecret, r.Client) + if err != nil { + return errors.Wrap(err, "failed to create patch helper for the kubeconfig secret") + } + configSecret.OwnerReferences = util.RemoveOwnerRef(configSecret.OwnerReferences, metav1.OwnerReference{ + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: cluster.Name, + UID: cluster.UID, + }) + configSecret.OwnerReferences = util.EnsureOwnerRef(configSecret.OwnerReferences, controllerOwnerRef) + if err := patch.Patch(ctx, configSecret); err != nil { + return errors.Wrap(err, "failed to patch the kubeconfig secret") + } + return nil +} + func (r *KubeadmControlPlaneReconciler) reconcileExternalReference(ctx context.Context, cluster *clusterv1.Cluster, ref corev1.ObjectReference) error { if !strings.HasSuffix(ref.Kind, external.TemplateSuffix) { return nil diff --git a/controlplane/kubeadm/controllers/helpers_test.go b/controlplane/kubeadm/controllers/helpers_test.go index 0509b959e77d..3ab122ab2fe3 100644 --- a/controlplane/kubeadm/controllers/helpers_test.go +++ b/controlplane/kubeadm/controllers/helpers_test.go @@ -32,7 +32,6 @@ import ( "sigs.k8s.io/cluster-api/controllers/external" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha4" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" - "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/kubeconfig" "sigs.k8s.io/cluster-api/util/secret" @@ -42,7 +41,25 @@ import ( func TestReconcileKubeconfigEmptyAPIEndpoints(t *testing.T) { g := NewWithT(t) + cluster := &clusterv1.Cluster{ + TypeMeta: metav1.TypeMeta{ + Kind: "Cluster", + APIVersion: clusterv1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "test", + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneEndpoint: clusterv1.APIEndpoint{}, + }, + } + kcp := &controlplanev1.KubeadmControlPlane{ + TypeMeta: metav1.TypeMeta{ + Kind: "KubeadmControlPlane", + APIVersion: controlplanev1.GroupVersion.String(), + }, ObjectMeta: metav1.ObjectMeta{ Name: "foo", Namespace: "test", @@ -59,7 +76,7 @@ func TestReconcileKubeconfigEmptyAPIEndpoints(t *testing.T) { recorder: record.NewFakeRecorder(32), } - g.Expect(r.reconcileKubeconfig(ctx, clusterName, clusterv1.APIEndpoint{}, kcp)).To(Succeed()) + g.Expect(r.reconcileKubeconfig(ctx, cluster, kcp)).To(Succeed()) kubeconfigSecret := &corev1.Secret{} secretName := client.ObjectKey{ @@ -72,7 +89,25 @@ func TestReconcileKubeconfigEmptyAPIEndpoints(t *testing.T) { func TestReconcileKubeconfigMissingCACertificate(t *testing.T) { g := NewWithT(t) + cluster := &clusterv1.Cluster{ + TypeMeta: metav1.TypeMeta{ + Kind: "Cluster", + APIVersion: clusterv1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "test", + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneEndpoint: clusterv1.APIEndpoint{Host: "test.local", Port: 8443}, + }, + } + kcp := &controlplanev1.KubeadmControlPlane{ + TypeMeta: metav1.TypeMeta{ + Kind: "KubeadmControlPlane", + APIVersion: controlplanev1.GroupVersion.String(), + }, ObjectMeta: metav1.ObjectMeta{ Name: "foo", Namespace: "test", @@ -81,8 +116,6 @@ func TestReconcileKubeconfigMissingCACertificate(t *testing.T) { Version: "v1.16.6", }, } - clusterName := client.ObjectKey{Namespace: "test", Name: "foo"} - endpoint := clusterv1.APIEndpoint{Host: "test.local", Port: 8443} fakeClient := newFakeClient(g, kcp.DeepCopy()) r := &KubeadmControlPlaneReconciler{ @@ -90,27 +123,38 @@ func TestReconcileKubeconfigMissingCACertificate(t *testing.T) { recorder: record.NewFakeRecorder(32), } - g.Expect(r.reconcileKubeconfig(ctx, clusterName, endpoint, kcp)).NotTo(Succeed()) + g.Expect(r.reconcileKubeconfig(ctx, cluster, kcp)).NotTo(Succeed()) kubeconfigSecret := &corev1.Secret{} secretName := client.ObjectKey{ Namespace: "test", - Name: secret.Name(clusterName.Name, secret.Kubeconfig), + Name: secret.Name(cluster.Name, secret.Kubeconfig), } g.Expect(r.Client.Get(ctx, secretName, kubeconfigSecret)).To(MatchError(ContainSubstring("not found"))) } -func TestReconcileKubeconfigSecretAlreadyExists(t *testing.T) { +func TestReconcileKubeconfigSecretAdoptsV1alpha2Secrets(t *testing.T) { g := NewWithT(t) cluster := &clusterv1.Cluster{ + TypeMeta: metav1.TypeMeta{ + Kind: "Cluster", + APIVersion: clusterv1.GroupVersion.String(), + }, ObjectMeta: metav1.ObjectMeta{ Name: "foo", Namespace: "test", }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneEndpoint: clusterv1.APIEndpoint{Host: "test.local", Port: 8443}, + }, } kcp := &controlplanev1.KubeadmControlPlane{ + TypeMeta: metav1.TypeMeta{ + Kind: "KubeadmControlPlane", + APIVersion: controlplanev1.GroupVersion.String(), + }, ObjectMeta: metav1.ObjectMeta{ Name: "foo", Namespace: "test", @@ -119,13 +163,16 @@ func TestReconcileKubeconfigSecretAlreadyExists(t *testing.T) { Version: "v1.16.6", }, } - clusterName := util.ObjectKey(cluster) - endpoint := clusterv1.APIEndpoint{Host: "test.local", Port: 8443} existingKubeconfigSecret := kubeconfig.GenerateSecretWithOwner( client.ObjectKey{Name: "foo", Namespace: "test"}, []byte{}, - *metav1.NewControllerRef(cluster, clusterv1.GroupVersion.WithKind("Cluster")), + metav1.OwnerReference{ + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: cluster.Name, + UID: cluster.UID, + }, // the Cluster ownership defines v1alpha2 controlled secrets ) fakeClient := newFakeClient(g, kcp.DeepCopy(), existingKubeconfigSecret.DeepCopy()) @@ -134,31 +181,103 @@ func TestReconcileKubeconfigSecretAlreadyExists(t *testing.T) { recorder: record.NewFakeRecorder(32), } - g.Expect(r.reconcileKubeconfig(ctx, clusterName, endpoint, kcp)).To(Succeed()) + g.Expect(r.reconcileKubeconfig(ctx, cluster, kcp)).To(Succeed()) kubeconfigSecret := &corev1.Secret{} secretName := client.ObjectKey{ Namespace: "test", - Name: secret.Name(clusterName.Name, secret.Kubeconfig), + Name: secret.Name(cluster.Name, secret.Kubeconfig), } g.Expect(r.Client.Get(ctx, secretName, kubeconfigSecret)).To(Succeed()) g.Expect(kubeconfigSecret.Labels).To(Equal(existingKubeconfigSecret.Labels)) g.Expect(kubeconfigSecret.Data).To(Equal(existingKubeconfigSecret.Data)) - g.Expect(kubeconfigSecret.OwnerReferences).NotTo(ContainElement(*metav1.NewControllerRef(kcp, controlplanev1.GroupVersion.WithKind("KubeadmControlPlane")))) + g.Expect(kubeconfigSecret.OwnerReferences).ToNot(ContainElement(metav1.OwnerReference{ + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: cluster.Name, + UID: cluster.UID, + })) + g.Expect(kubeconfigSecret.OwnerReferences).To(ContainElement(*metav1.NewControllerRef(kcp, controlplanev1.GroupVersion.WithKind("KubeadmControlPlane")))) +} + +func TestReconcileKubeconfigSecretDoesNotAdoptsUserSecrets(t *testing.T) { + g := NewWithT(t) + + cluster := &clusterv1.Cluster{ + TypeMeta: metav1.TypeMeta{ + Kind: "Cluster", + APIVersion: clusterv1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "test", + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneEndpoint: clusterv1.APIEndpoint{Host: "test.local", Port: 8443}, + }, + } + + kcp := &controlplanev1.KubeadmControlPlane{ + TypeMeta: metav1.TypeMeta{ + Kind: "KubeadmControlPlane", + APIVersion: controlplanev1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "test", + }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + Version: "v1.16.6", + }, + } + + existingKubeconfigSecret := kubeconfig.GenerateSecretWithOwner( + client.ObjectKey{Name: "foo", Namespace: "test"}, + []byte{}, + metav1.OwnerReference{}, // user defined secrets are not owned by the cluster. + ) + fakeClient := newFakeClient(g, kcp.DeepCopy(), existingKubeconfigSecret.DeepCopy()) + r := &KubeadmControlPlaneReconciler{ + Client: fakeClient, + recorder: record.NewFakeRecorder(32), + } + + g.Expect(r.reconcileKubeconfig(ctx, cluster, kcp)).To(Succeed()) + + kubeconfigSecret := &corev1.Secret{} + secretName := client.ObjectKey{ + Namespace: "test", + Name: secret.Name(cluster.Name, secret.Kubeconfig), + } + g.Expect(r.Client.Get(ctx, secretName, kubeconfigSecret)).To(Succeed()) + g.Expect(kubeconfigSecret.Labels).To(Equal(existingKubeconfigSecret.Labels)) + g.Expect(kubeconfigSecret.Data).To(Equal(existingKubeconfigSecret.Data)) + g.Expect(kubeconfigSecret.OwnerReferences).ToNot(ContainElement(*metav1.NewControllerRef(kcp, controlplanev1.GroupVersion.WithKind("KubeadmControlPlane")))) } func TestKubeadmControlPlaneReconciler_reconcileKubeconfig(t *testing.T) { g := NewWithT(t) cluster := &clusterv1.Cluster{ + TypeMeta: metav1.TypeMeta{ + Kind: "Cluster", + APIVersion: clusterv1.GroupVersion.String(), + }, ObjectMeta: metav1.ObjectMeta{ Name: "foo", Namespace: "test", }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneEndpoint: clusterv1.APIEndpoint{Host: "test.local", Port: 8443}, + }, } kcp := &controlplanev1.KubeadmControlPlane{ + TypeMeta: metav1.TypeMeta{ + Kind: "KubeadmControlPlane", + APIVersion: controlplanev1.GroupVersion.String(), + }, ObjectMeta: metav1.ObjectMeta{ Name: "foo", Namespace: "test", @@ -167,8 +286,6 @@ func TestKubeadmControlPlaneReconciler_reconcileKubeconfig(t *testing.T) { Version: "v1.16.6", }, } - clusterName := util.ObjectKey(cluster) - endpoint := clusterv1.APIEndpoint{Host: "test.local", Port: 8443} clusterCerts := secret.NewCertificatesForInitialControlPlane(&kubeadmv1.ClusterConfiguration{}) g.Expect(clusterCerts.Generate()).To(Succeed()) @@ -183,17 +300,17 @@ func TestKubeadmControlPlaneReconciler_reconcileKubeconfig(t *testing.T) { Client: fakeClient, recorder: record.NewFakeRecorder(32), } - g.Expect(r.reconcileKubeconfig(ctx, clusterName, endpoint, kcp)).To(Succeed()) + g.Expect(r.reconcileKubeconfig(ctx, cluster, kcp)).To(Succeed()) kubeconfigSecret := &corev1.Secret{} secretName := client.ObjectKey{ Namespace: "test", - Name: secret.Name(clusterName.Name, secret.Kubeconfig), + Name: secret.Name(cluster.Name, secret.Kubeconfig), } g.Expect(r.Client.Get(ctx, secretName, kubeconfigSecret)).To(Succeed()) g.Expect(kubeconfigSecret.OwnerReferences).NotTo(BeEmpty()) g.Expect(kubeconfigSecret.OwnerReferences).To(ContainElement(*metav1.NewControllerRef(kcp, controlplanev1.GroupVersion.WithKind("KubeadmControlPlane")))) - g.Expect(kubeconfigSecret.Labels).To(HaveKeyWithValue(clusterv1.ClusterLabelName, clusterName.Name)) + g.Expect(kubeconfigSecret.Labels).To(HaveKeyWithValue(clusterv1.ClusterLabelName, cluster.Name)) } func TestCloneConfigsAndGenerateMachine(t *testing.T) {