Merge tag 'v0.3.12' into spectro-master

v0.3.12

Author: abhinavnagaraj

OWNERS

@@ -7,9 +7,13 @@ approvers:
   - cluster-api-maintainers
 
 emeritus_approvers:
-  - roberthbailey
-  - kris-nova
   - chuckha
+  - kris-nova
+  - ncdc
+  - roberthbailey
+
+emeritus_maintainers:
+  - ncdc
 
 reviewers:
   - cluster-api-maintainers

OWNERS_ALIASES

@@ -23,7 +23,6 @@ aliases:
   cluster-api-maintainers:
   - justinsb
   - detiber
-  - ncdc
   - vincepri
   - CecileRobertMichon
 
@@ -48,20 +47,18 @@ aliases:
   # -----------------------------------------------------------
 
   cluster-api-provider-docker-maintainers:
-  - ncdc
+  - fabriziopandini
 
   # -----------------------------------------------------------
   # OWNER_ALIASES for bootstrap/kubeadm
   # -----------------------------------------------------------
 
   cluster-api-bootstrap-provider-kubeadm-maintainers:
   - fabriziopandini
-  - ncdc
   - SataQiu
 
   cluster-api-bootstrap-provider-kubeadm-reviewers:
   - fabriziopandini
-  - ncdc
 
   # -----------------------------------------------------------
   # OWNER_ALIASES for cmd/clusterctl

bootstrap/kubeadm/controllers/kubeadmconfig_controller.go

@@ -148,7 +148,7 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 
 	// Look up the owner of this KubeConfig if there is one
 	configOwner, err := bsutil.GetConfigOwner(ctx, r.Client, config)
-	if apierrors.IsNotFound(err) {
+	if apierrors.IsNotFound(errors.Cause(err)) {
 		// Could not find the owner yet, this is not an error and will rereconcile when the owner gets set.
 		return ctrl.Result{}, nil
 	}
@@ -169,7 +169,7 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 			return ctrl.Result{}, nil
 		}
 
-		if apierrors.IsNotFound(err) {
+		if apierrors.IsNotFound(errors.Cause(err)) {
 			log.Info("Cluster does not exist yet, waiting until it is created")
 			return ctrl.Result{}, nil
 		}
@@ -225,10 +225,7 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 		return ctrl.Result{}, nil
 	// Migrate plaintext data to secret.
 	case config.Status.BootstrapData != nil && config.Status.DataSecretName == nil:
-		if err := r.storeBootstrapData(ctx, scope, config.Status.BootstrapData); err != nil {
-			return ctrl.Result{}, err
-		}
-		return ctrl.Result{}, nil
+		return ctrl.Result{}, r.storeBootstrapData(ctx, scope, config.Status.BootstrapData)
 	// Reconcile status for machines that already have a secret reference, but our status isn't up to date.
 	// This case solves the pivoting scenario (or a backup restore) which doesn't preserve the status subresource on objects.
 	case configOwner.DataSecretName() != nil && (!config.Status.Ready || config.Status.DataSecretName == nil):
@@ -240,26 +237,14 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 	case config.Status.Ready:
 		// If the BootstrapToken has been generated for a join and the infrastructure is not ready.
 		// This indicates the token in the join config has not been consumed and it may need a refresh.
-		if (config.Spec.JoinConfiguration != nil && config.Spec.JoinConfiguration.Discovery.BootstrapToken != nil) && !configOwner.IsInfrastructureReady() {
-
-			token := config.Spec.JoinConfiguration.Discovery.BootstrapToken.Token
-
-			remoteClient, err := r.remoteClientGetter(ctx, r.Client, util.ObjectKey(cluster), r.scheme)
-			if err != nil {
-				log.Error(err, "Error creating remote cluster client")
-				return ctrl.Result{}, err
-			}
-
-			log.Info("Refreshing token until the infrastructure has a chance to consume it")
-			err = refreshToken(remoteClient, token)
-			if err != nil {
-				// It would be nice to re-create the bootstrap token if the error was "not found", but we have no way to update the Machine's bootstrap data
-				return ctrl.Result{}, errors.Wrapf(err, "failed to refresh bootstrap token")
+		if config.Spec.JoinConfiguration != nil && config.Spec.JoinConfiguration.Discovery.BootstrapToken != nil {
+			if !configOwner.IsInfrastructureReady() {
+				return r.refreshBootstrapToken(ctx, log, cluster, config)
+			} else if (config.Spec.JoinConfiguration != nil && config.Spec.JoinConfiguration.Discovery.BootstrapToken != nil) && configOwner.IsMachinePool() {
+				// If the BootstrapToken has been generated and infrastructure is ready but the configOwner is a MachinePool,
+				// we rotate the token to keep it fresh for future scale ups.
+				return r.rotateMachinePoolBootstrapToken(ctx, log, cluster, config, scope)
 			}
-			// NB: this may not be sufficient to keep the token live if we don't see it before it expires, but when we generate a config we will set the status to "ready" which should generate an update event
-			return ctrl.Result{
-				RequeueAfter: DefaultTokenTTL / 2,
-			}, nil
 		}
 		// In any other case just return as the config is already generated and need not be generated again.
 		return ctrl.Result{}, nil
@@ -290,6 +275,54 @@ func (r *KubeadmConfigReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, re
 	return r.joinWorker(ctx, scope)
 }
 
+func (r *KubeadmConfigReconciler) refreshBootstrapToken(ctx context.Context, log logr.Logger, cluster *clusterv1.Cluster, config *bootstrapv1.KubeadmConfig) (ctrl.Result, error) {
+	token := config.Spec.JoinConfiguration.Discovery.BootstrapToken.Token
+
+	remoteClient, err := r.remoteClientGetter(ctx, r.Client, util.ObjectKey(cluster), r.scheme)
+	if err != nil {
+		log.Error(err, "Error creating remote cluster client")
+		return ctrl.Result{}, err
+	}
+
+	log.Info("Refreshing token until the infrastructure has a chance to consume it")
+	if err := refreshToken(remoteClient, token); err != nil {
+		return ctrl.Result{}, errors.Wrapf(err, "failed to refresh bootstrap token")
+	}
+	return ctrl.Result{
+		RequeueAfter: DefaultTokenTTL / 2,
+	}, nil
+}
+
+func (r *KubeadmConfigReconciler) rotateMachinePoolBootstrapToken(ctx context.Context, log logr.Logger, cluster *clusterv1.Cluster, config *bootstrapv1.KubeadmConfig, scope *Scope) (ctrl.Result, error) {
+	log.V(2).Info("Config is owned by a MachinePool, checking if token should be rotated")
+	remoteClient, err := r.remoteClientGetter(ctx, r.Client, util.ObjectKey(cluster), r.scheme)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+
+	token := config.Spec.JoinConfiguration.Discovery.BootstrapToken.Token
+	shouldRotate, err := shouldRotate(remoteClient, token)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+	if shouldRotate {
+		log.V(2).Info("Creating new bootstrap token")
+		token, err := createToken(remoteClient)
+		if err != nil {
+			return ctrl.Result{}, errors.Wrapf(err, "failed to create new bootstrap token")
+		}
+
+		config.Spec.JoinConfiguration.Discovery.BootstrapToken.Token = token
+		log.Info("Altering JoinConfiguration.Discovery.BootstrapToken", "Token", token)
+
+		// update the bootstrap data
+		return r.joinWorker(ctx, scope)
+	}
+	return ctrl.Result{
+		RequeueAfter: DefaultTokenTTL / 3,
+	}, nil
+}
+
 func (r *KubeadmConfigReconciler) handleClusterNotInitialized(ctx context.Context, scope *Scope) (_ ctrl.Result, reterr error) {
 	// initialize the DataSecretAvailableCondition if missing.
 	// this is required in order to avoid the condition's LastTransitionTime to flicker in case of errors surfacing
@@ -839,7 +872,10 @@ func (r *KubeadmConfigReconciler) storeBootstrapData(ctx context.Context, scope
 		if !apierrors.IsAlreadyExists(err) {
 			return errors.Wrapf(err, "failed to create bootstrap data secret for KubeadmConfig %s/%s", scope.Config.Namespace, scope.Config.Name)
 		}
-		r.Log.Info("bootstrap data secret for KubeadmConfig already exists", "secret", secret.Name, "KubeadmConfig", scope.Config.Name)
+		r.Log.Info("bootstrap data secret for KubeadmConfig already exists, updating", "secret", secret.Name, "KubeadmConfig", scope.Config.Name)
+		if err := r.Client.Update(ctx, secret); err != nil {
+			return errors.Wrapf(err, "failed to update bootstrap data secret for KubeadmConfig %s/%s", scope.Config.Namespace, scope.Config.Name)
+		}
 	}
 	scope.Config.Status.DataSecretName = pointer.StringPtr(secret.Name)
 	scope.Config.Status.Ready = true

bootstrap/kubeadm/controllers/kubeadmconfig_controller_test.go

@@ -134,7 +134,7 @@ func TestKubeadmConfigReconciler_Reconcile_ReturnEarlyIfKubeadmConfigIsReady(t *
 }
 
 // Reconcile returns an error in this case because the owning machine should not go away before the things it owns.
-func TestKubeadmConfigReconciler_Reconcile_ReturnErrorIfReferencedMachineIsNotFound(t *testing.T) {
+func TestKubeadmConfigReconciler_Reconcile_ReturnNilIfReferencedMachineIsNotFound(t *testing.T) {
 	g := NewWithT(t)
 
 	machine := newMachine(nil, "machine")
@@ -158,7 +158,7 @@ func TestKubeadmConfigReconciler_Reconcile_ReturnErrorIfReferencedMachineIsNotFo
 		},
 	}
 	_, err := k.Reconcile(request)
-	g.Expect(err).To(HaveOccurred())
+	g.Expect(err).To(BeNil())
 }
 
 // If the machine has bootstrap data secret reference, there is no need to generate more bootstrap data.
@@ -944,6 +944,148 @@ func TestBootstrapTokenTTLExtension(t *testing.T) {
 	}
 }
 
+func TestBootstrapTokenRotationMachinePool(t *testing.T) {
+	_ = feature.MutableGates.Set("MachinePool=true")
+	g := NewWithT(t)
+
+	cluster := newCluster("cluster")
+	cluster.Status.InfrastructureReady = true
+	cluster.Status.ControlPlaneInitialized = true
+	cluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{Host: "100.105.150.1", Port: 6443}
+
+	controlPlaneInitMachine := newControlPlaneMachine(cluster, "control-plane-init-machine")
+	initConfig := newControlPlaneInitKubeadmConfig(controlPlaneInitMachine, "control-plane-init-config")
+	workerMachinePool := newWorkerMachinePool(cluster)
+	workerJoinConfig := newWorkerPoolJoinKubeadmConfig(workerMachinePool)
+	objects := []runtime.Object{
+		cluster,
+		workerMachinePool,
+		workerJoinConfig,
+	}
+
+	objects = append(objects, createSecrets(t, cluster, initConfig)...)
+	myclient := helpers.NewFakeClientWithScheme(setupScheme(), objects...)
+	k := &KubeadmConfigReconciler{
+		Log:                log.Log,
+		Client:             myclient,
+		KubeadmInitLock:    &myInitLocker{},
+		remoteClientGetter: fakeremote.NewClusterClient,
+	}
+	request := ctrl.Request{
+		NamespacedName: client.ObjectKey{
+			Namespace: "default",
+			Name:      "workerpool-join-cfg",
+		},
+	}
+	result, err := k.Reconcile(request)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(result.Requeue).To(BeFalse())
+	g.Expect(result.RequeueAfter).To(Equal(time.Duration(0)))
+
+	cfg, err := getKubeadmConfig(myclient, "workerpool-join-cfg")
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(cfg.Status.Ready).To(BeTrue())
+	g.Expect(cfg.Status.DataSecretName).NotTo(BeNil())
+	g.Expect(cfg.Status.ObservedGeneration).NotTo(BeNil())
+
+	l := &corev1.SecretList{}
+	err = myclient.List(context.Background(), l, client.ListOption(client.InNamespace(metav1.NamespaceSystem)))
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(len(l.Items)).To(Equal(1))
+
+	// ensure that the token is refreshed...
+	tokenExpires := make([][]byte, len(l.Items))
+
+	for i, item := range l.Items {
+		tokenExpires[i] = item.Data[bootstrapapi.BootstrapTokenExpirationKey]
+	}
+
+	<-time.After(1 * time.Second)
+
+	for _, req := range []ctrl.Request{
+		{
+			NamespacedName: client.ObjectKey{
+				Namespace: "default",
+				Name:      "workerpool-join-cfg",
+			},
+		},
+	} {
+
+		result, err := k.Reconcile(req)
+		g.Expect(err).NotTo(HaveOccurred())
+		g.Expect(result.RequeueAfter).NotTo(BeNumerically(">=", DefaultTokenTTL))
+	}
+
+	l = &corev1.SecretList{}
+	err = myclient.List(context.Background(), l, client.ListOption(client.InNamespace(metav1.NamespaceSystem)))
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(len(l.Items)).To(Equal(1))
+
+	for i, item := range l.Items {
+		g.Expect(bytes.Equal(tokenExpires[i], item.Data[bootstrapapi.BootstrapTokenExpirationKey])).To(BeFalse())
+		tokenExpires[i] = item.Data[bootstrapapi.BootstrapTokenExpirationKey]
+	}
+
+	// ...until the infrastructure is marked "ready"
+	workerMachinePool.Status.InfrastructureReady = true
+	err = myclient.Update(context.Background(), workerMachinePool)
+	g.Expect(err).NotTo(HaveOccurred())
+
+	<-time.After(1 * time.Second)
+
+	request = ctrl.Request{
+		NamespacedName: client.ObjectKey{
+			Namespace: "default",
+			Name:      "workerpool-join-cfg",
+		},
+	}
+	result, err = k.Reconcile(request)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(result.RequeueAfter).To(Equal(DefaultTokenTTL / 3))
+
+	l = &corev1.SecretList{}
+	err = myclient.List(context.Background(), l, client.ListOption(client.InNamespace(metav1.NamespaceSystem)))
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(len(l.Items)).To(Equal(1))
+
+	for i, item := range l.Items {
+		g.Expect(bytes.Equal(tokenExpires[i], item.Data[bootstrapapi.BootstrapTokenExpirationKey])).To(BeTrue())
+	}
+
+	// before token expires, it should rotate it
+	tokenExpires[0] = []byte(time.Now().UTC().Add(DefaultTokenTTL / 5).Format(time.RFC3339))
+	l.Items[0].Data[bootstrapapi.BootstrapTokenExpirationKey] = tokenExpires[0]
+	err = myclient.Update(context.TODO(), &l.Items[0])
+	g.Expect(err).NotTo(HaveOccurred())
+
+	request = ctrl.Request{
+		NamespacedName: client.ObjectKey{
+			Namespace: "default",
+			Name:      "workerpool-join-cfg",
+		},
+	}
+	result, err = k.Reconcile(request)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(result.RequeueAfter).To(Equal(time.Duration(0)))
+
+	l = &corev1.SecretList{}
+	err = myclient.List(context.Background(), l, client.ListOption(client.InNamespace(metav1.NamespaceSystem)))
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(len(l.Items)).To(Equal(2))
+	foundOld := false
+	foundNew := true
+	for _, item := range l.Items {
+		if bytes.Equal(item.Data[bootstrapapi.BootstrapTokenExpirationKey], tokenExpires[0]) {
+			foundOld = true
+		} else {
+			g.Expect(string(item.Data[bootstrapapi.BootstrapTokenExpirationKey])).To(Equal(time.Now().UTC().Add(DefaultTokenTTL).Format(time.RFC3339)))
+			foundNew = true
+		}
+	}
+	g.Expect(foundOld).To(BeTrue())
+	g.Expect(foundNew).To(BeTrue())
+}
+
 // Ensure the discovery portion of the JoinConfiguration gets generated correctly.
 func TestKubeadmConfigReconciler_Reconcile_DiscoveryReconcileBehaviors(t *testing.T) {
 	k := &KubeadmConfigReconciler{

bootstrap/kubeadm/controllers/token.go

@@ -71,24 +71,47 @@ func createToken(c client.Client) (string, error) {
 	return token, nil
 }
 
-// refreshToken extends the TTL for an existing token
-func refreshToken(c client.Client, token string) error {
+// getToken fetches the token Secret and returns an error if it is invalid.
+func getToken(c client.Client, token string) (*v1.Secret, error) {
 	substrs := bootstraputil.BootstrapTokenRegexp.FindStringSubmatch(token)
 	if len(substrs) != 3 {
-		return errors.Errorf("the bootstrap token %q was not of the form %q", token, bootstrapapi.BootstrapTokenPattern)
+		return nil, errors.Errorf("the bootstrap token %q was not of the form %q", token, bootstrapapi.BootstrapTokenPattern)
 	}
 	tokenID := substrs[1]
 
 	secretName := bootstraputil.BootstrapTokenSecretName(tokenID)
 	secret := &v1.Secret{}
 	if err := c.Get(context.TODO(), client.ObjectKey{Name: secretName, Namespace: metav1.NamespaceSystem}, secret); err != nil {
-		return err
+		return secret, err
 	}
 
 	if secret.Data == nil {
-		return errors.Errorf("Invalid bootstrap secret %q, remove the token from the kubadm config to re-create", secretName)
+		return nil, errors.Errorf("Invalid bootstrap secret %q, remove the token from the kubadm config to re-create", secretName)
+	}
+	return secret, nil
+}
+
+// refreshToken extends the TTL for an existing token.
+func refreshToken(c client.Client, token string) error {
+	secret, err := getToken(c, token)
+	if err != nil {
+		return err
 	}
 	secret.Data[bootstrapapi.BootstrapTokenExpirationKey] = []byte(time.Now().UTC().Add(DefaultTokenTTL).Format(time.RFC3339))
 
 	return c.Update(context.TODO(), secret)
 }
+
+// shouldRotate returns true if an existing token is past half of its TTL and should to be rotated.
+func shouldRotate(c client.Client, token string) (bool, error) {
+	secret, err := getToken(c, token)
+	if err != nil {
+		return false, err
+	}
+
+	expiration, err := time.Parse(time.RFC3339, string(secret.Data[bootstrapapi.BootstrapTokenExpirationKey]))
+	if err != nil {
+		return false, err
+	}
+	return expiration.Before(time.Now().UTC().Add(DefaultTokenTTL / 2)), nil
+}