fix: move responsibility for managing k3s token to control plane cont…

…roller (#71)

* Move responsibility for creating the token required by nodes to join the cluster to the KThreesControlPlane controller

.golangci.yml

@@ -86,6 +86,8 @@ linters-settings:
           - sigs.k8s.io/cluster-api
 
           - github.com/cluster-api-provider-k3s/cluster-api-k3s
+
+          - github.com/google/uuid
   gci:
     sections:
       - standard

bootstrap/controllers/kthreesconfig_controller.go

@@ -223,13 +223,13 @@ func (r *KThreesConfigReconciler) joinControlplane(ctx context.Context, scope *S
 
 	serverURL := fmt.Sprintf("https://%s", scope.Cluster.Spec.ControlPlaneEndpoint.String())
 
-	tokn, err := r.retrieveToken(ctx, scope)
+	tokn, err := token.Lookup(ctx, r.Client, client.ObjectKeyFromObject(scope.Cluster))
 	if err != nil {
 		conditions.MarkFalse(scope.Config, bootstrapv1.DataSecretAvailableCondition, bootstrapv1.DataSecretGenerationFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
 		return err
 	}
 
-	configStruct := k3s.GenerateJoinControlPlaneConfig(serverURL, tokn,
+	configStruct := k3s.GenerateJoinControlPlaneConfig(serverURL, *tokn,
 		scope.Cluster.Spec.ControlPlaneEndpoint.Host,
 		scope.Config.Spec.ServerConfig,
 		scope.Config.Spec.AgentConfig)
@@ -284,13 +284,13 @@ func (r *KThreesConfigReconciler) joinWorker(ctx context.Context, scope *Scope)
 
 	serverURL := fmt.Sprintf("https://%s", scope.Cluster.Spec.ControlPlaneEndpoint.String())
 
-	tokn, err := r.retrieveToken(ctx, scope)
+	tokn, err := token.Lookup(ctx, r.Client, client.ObjectKeyFromObject(scope.Cluster))
 	if err != nil {
 		conditions.MarkFalse(scope.Config, bootstrapv1.DataSecretAvailableCondition, bootstrapv1.DataSecretGenerationFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
 		return err
 	}
 
-	configStruct := k3s.GenerateWorkerConfig(serverURL, tokn, scope.Config.Spec.ServerConfig, scope.Config.Spec.AgentConfig)
+	configStruct := k3s.GenerateWorkerConfig(serverURL, *tokn, scope.Config.Spec.ServerConfig, scope.Config.Spec.AgentConfig)
 
 	b, err := kubeyaml.Marshal(configStruct)
 	if err != nil {
@@ -424,7 +424,7 @@ func (r *KThreesConfigReconciler) handleClusterNotInitialized(ctx context.Contex
 	}
 	conditions.MarkTrue(scope.Config, bootstrapv1.CertificatesAvailableCondition)
 
-	token, err := r.generateAndStoreToken(ctx, scope)
+	token, err := token.Lookup(ctx, r.Client, client.ObjectKeyFromObject(scope.Cluster))
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -433,7 +433,7 @@ func (r *KThreesConfigReconciler) handleClusterNotInitialized(ctx context.Contex
 	// For now just use the etcd option
 	configStruct := k3s.GenerateInitControlPlaneConfig(
 		scope.Cluster.Spec.ControlPlaneEndpoint.Host,
-		token,
+		*token,
 		scope.Config.Spec.ServerConfig,
 		scope.Config.Spec.AgentConfig)
 
@@ -480,64 +480,6 @@ func (r *KThreesConfigReconciler) handleClusterNotInitialized(ctx context.Contex
 	return r.reconcileKubeconfig(ctx, scope)
 }
 
-func (r *KThreesConfigReconciler) generateAndStoreToken(ctx context.Context, scope *Scope) (string, error) {
-	tokn, err := token.Random(16)
-	if err != nil {
-		return "", err
-	}
-
-	secret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      token.Name(scope.Cluster.Name),
-			Namespace: scope.Config.Namespace,
-			Labels: map[string]string{
-				clusterv1.ClusterNameLabel: scope.Cluster.Name,
-			},
-			OwnerReferences: []metav1.OwnerReference{
-				{
-					APIVersion: clusterv1.GroupVersion.String(),
-					Kind:       "Cluster",
-					Name:       scope.Cluster.Name,
-					UID:        scope.Cluster.UID,
-					Controller: pointer.Bool(true),
-				},
-			},
-		},
-		Data: map[string][]byte{
-			"value": []byte(tokn),
-		},
-		Type: clusterv1.ClusterSecretType,
-	}
-
-	// as secret creation and scope.Config status patch are not atomic operations
-	// it is possible that secret creation happens but the config.Status patches are not applied
-	if err := r.Client.Create(ctx, secret); err != nil {
-		if !apierrors.IsAlreadyExists(err) {
-			return "", fmt.Errorf("failed to create token for KThreesConfig %s/%s: %w", scope.Config.Namespace, scope.Config.Name, err)
-		}
-		// r.Log.Info("bootstrap data secret for KThreesConfig already exists, updating", "secret", secret.Name, "KThreesConfig", scope.Config.Name)
-		if err := r.Client.Update(ctx, secret); err != nil {
-			return "", fmt.Errorf("failed to update bootstrap token secret for KThreesConfig %s/%s: %w", scope.Config.Namespace, scope.Config.Name, err)
-		}
-	}
-
-	return tokn, nil
-}
-
-func (r *KThreesConfigReconciler) retrieveToken(ctx context.Context, scope *Scope) (string, error) {
-	secret := &corev1.Secret{}
-	obj := client.ObjectKey{
-		Namespace: scope.Config.Namespace,
-		Name:      token.Name(scope.Cluster.Name),
-	}
-
-	if err := r.Client.Get(ctx, obj, secret); err != nil {
-		return "", fmt.Errorf("failed to get token for KThreesConfig %s/%s: %w", scope.Config.Namespace, scope.Config.Name, err)
-	}
-
-	return string(secret.Data["value"]), nil
-}
-
 func (r *KThreesConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	if r.KThreesInitLock == nil {
 		r.KThreesInitLock = locking.NewControlPlaneInitMutex(ctrl.Log.WithName("init-locker"), mgr.GetClient())

controlplane/api/v1beta1/condition_consts.go

@@ -120,3 +120,11 @@ const (
 	// EtcdMemberUnhealthyReason (Severity=Error) documents a Machine's etcd member is unhealthy.
 	EtcdMemberUnhealthyReason = "EtcdMemberUnhealthy"
 )
+
+const (
+	// TokenAvailableCondition documents whether the token required for nodes to join the cluster is available.
+	TokenAvailableCondition clusterv1.ConditionType = "TokenAvailable"
+
+	// TokenGenerationFailedReason documents that the token required for nodes to join the cluster could not be generated.
+	TokenGenerationFailedReason = "TokenGenerationFailed"
+)

controlplane/controllers/kthreescontrolplane_controller.go

@@ -50,6 +50,7 @@ import (
 	"github.com/cluster-api-provider-k3s/cluster-api-k3s/pkg/kubeconfig"
 	"github.com/cluster-api-provider-k3s/cluster-api-k3s/pkg/machinefilters"
 	"github.com/cluster-api-provider-k3s/cluster-api-k3s/pkg/secret"
+	"github.com/cluster-api-provider-k3s/cluster-api-k3s/pkg/token"
 )
 
 // KThreesControlPlaneReconciler reconciles a KThreesControlPlane object.
@@ -244,6 +245,7 @@ func patchKThreesControlPlane(ctx context.Context, patchHelper *patch.Helper, kc
 			controlplanev1.MachinesReadyCondition,
 			controlplanev1.AvailableCondition,
 			controlplanev1.CertificatesAvailableCondition,
+			controlplanev1.TokenAvailableCondition,
 		),
 	)
 
@@ -258,6 +260,7 @@ func patchKThreesControlPlane(ctx context.Context, patchHelper *patch.Helper, kc
 			controlplanev1.MachinesReadyCondition,
 			controlplanev1.AvailableCondition,
 			controlplanev1.CertificatesAvailableCondition,
+			controlplanev1.TokenAvailableCondition,
 		}},
 	)
 }
@@ -408,6 +411,12 @@ func (r *KThreesControlPlaneReconciler) reconcile(ctx context.Context, cluster *
 	}
 	conditions.MarkTrue(kcp, controlplanev1.CertificatesAvailableCondition)
 
+	if err := token.Reconcile(ctx, r.Client, client.ObjectKeyFromObject(cluster), kcp); err != nil {
+		conditions.MarkFalse(kcp, controlplanev1.TokenAvailableCondition, controlplanev1.TokenGenerationFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
+		return reconcile.Result{}, err
+	}
+	conditions.MarkTrue(kcp, controlplanev1.TokenAvailableCondition)
+
 	// If ControlPlaneEndpoint is not set, return early
 	if !cluster.Spec.ControlPlaneEndpoint.IsValid() {
 		logger.Info("Cluster does not yet have a ControlPlaneEndpoint defined")

go.mod

@@ -26,6 +26,7 @@ require (
 	github.com/coredns/caddy v1.1.0 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/emicklei/go-restful/v3 v3.9.0 // indirect
+	github.com/evanphx/json-patch v5.6.0+incompatible // indirect
 	github.com/evanphx/json-patch/v5 v5.6.0 // indirect
 	github.com/fsnotify/fsnotify v1.6.0 // indirect
 	github.com/go-logr/zapr v1.2.3 // indirect

go.sum

@@ -102,6 +102,7 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20210512163311-63b5d3c536b0/go.m
 github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
 github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ=
 github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U=
+github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
 github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww=
 github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4=
 github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=

pkg/token/token.go

@@ -1,12 +1,61 @@
 package token
 
 import (
+	"context"
 	cryptorand "crypto/rand"
 	"encoding/hex"
 	"fmt"
+
+	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
-func Random(size int) (string, error) {
+func Lookup(ctx context.Context, ctrlclient client.Client, clusterKey client.ObjectKey) (*string, error) {
+	var s *corev1.Secret
+	var err error
+
+	if s, err = getSecret(ctx, ctrlclient, clusterKey); err != nil {
+		return nil, fmt.Errorf("failed to lookup token: %v", err)
+	}
+	if val, ok := s.Data["value"]; ok {
+		ret := string(val)
+		return &ret, nil
+	}
+
+	return nil, fmt.Errorf("found token secret without value")
+}
+
+func Reconcile(ctx context.Context, ctrlclient client.Client, clusterKey client.ObjectKey, owner client.Object) error {
+	var s *corev1.Secret
+	var err error
+
+	// Find the token secret
+	if s, err = getSecret(ctx, ctrlclient, clusterKey); err != nil {
+		if apierrors.IsNotFound(err) {
+			// Secret does not exist, create it
+			_, err = generateAndStore(ctx, ctrlclient, clusterKey, owner)
+			return err
+		}
+	}
+
+	// Secret exists
+	// Ensure the secret has correct ownership; this is necessary because at one point, the secret was owned by KThreesConfig
+	if !metav1.IsControlledBy(s, owner) {
+		upsertControllerRef(s, owner)
+		if err := ctrlclient.Update(ctx, s); err != nil {
+			return fmt.Errorf("failed to update ownership of token: %v", err)
+		}
+	}
+
+	return nil
+}
+
+// randomB64 generates a cryptographically secure random byte slice of length size and returns its base64 encoding.
+func randomB64(size int) (string, error) {
 	token := make([]byte, size)
 	_, err := cryptorand.Read(token)
 	if err != nil {
@@ -15,6 +64,81 @@ func Random(size int) (string, error) {
 	return hex.EncodeToString(token), err
 }
 
-func Name(clusterName string) string {
+// name returns the name of the token secret, computed by convention using the name of the cluster.
+func name(clusterName string) string {
 	return fmt.Sprintf("%s-token", clusterName)
 }
+
+func getSecret(ctx context.Context, ctrlclient client.Client, clusterKey client.ObjectKey) (*corev1.Secret, error) {
+	s := &corev1.Secret{}
+	key := client.ObjectKey{
+		Name:      name(clusterKey.Name),
+		Namespace: clusterKey.Namespace,
+	}
+	if err := ctrlclient.Get(ctx, key, s); err != nil {
+		return nil, err
+	}
+
+	return s, nil
+}
+
+func generateAndStore(ctx context.Context, ctrlclient client.Client, clusterKey client.ObjectKey, owner client.Object) (*string, error) {
+	tokn, err := randomB64(16)
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate token: %v", err)
+	}
+
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name(clusterKey.Name),
+			Namespace: clusterKey.Namespace,
+			Labels: map[string]string{
+				clusterv1.ClusterNameLabel: clusterKey.Name,
+			},
+		},
+		Data: map[string][]byte{
+			"value": []byte(tokn),
+		},
+		Type: clusterv1.ClusterSecretType,
+	}
+
+	//nolint:errcheck
+	controllerutil.SetControllerReference(owner, secret, ctrlclient.Scheme())
+
+	// as secret creation and scope.Config status patch are not atomic operations
+	// it is possible that secret creation happens but the config.Status patches are not applied
+	if err := ctrlclient.Create(ctx, secret); err != nil {
+		return nil, fmt.Errorf("failed to store token: %v", err)
+	}
+
+	return &tokn, nil
+}
+
+// upsertControllerRef takes controllee and controller objects, either replaces the existing controller ref
+// if one exists or appends the new controller ref if one does not exist, and returns the updated controllee
+// This is meant to be used in place of controllerutil.SetControllerReference(...), which would throw an error
+// if there were already an existing controller ref.
+func upsertControllerRef(controllee client.Object, controller client.Object) {
+	newControllerRef := metav1.NewControllerRef(controller, controller.GetObjectKind().GroupVersionKind())
+
+	// Iterate through existing owner references
+	var updatedOwnerReferences []metav1.OwnerReference
+	var controllerRefUpdated bool
+	for _, ownerRef := range controllee.GetOwnerReferences() {
+		// Identify and replace the controlling owner reference
+		if ownerRef.Controller != nil && *ownerRef.Controller {
+			updatedOwnerReferences = append(updatedOwnerReferences, *newControllerRef)
+			controllerRefUpdated = true
+		} else {
+			// Keep non-controlling owner references intact
+			updatedOwnerReferences = append(updatedOwnerReferences, ownerRef)
+		}
+	}
+
+	// If the controlling owner reference was not found, add the new one
+	if !controllerRefUpdated {
+		updatedOwnerReferences = append(updatedOwnerReferences, *newControllerRef)
+	}
+
+	controllee.SetOwnerReferences(updatedOwnerReferences)
+}