Remove secrets encryption controller (#10612)

* Remove secrets encryption controller

Signed-off-by: Derek Nola <derek.nola@suse.com>

pkg/daemons/config/types.go

@@ -227,8 +227,6 @@ type Control struct {
 	ClusterInit              bool
 	ClusterReset             bool
 	ClusterResetRestorePath  string
-	EncryptForce             bool
-	EncryptSkip              bool
 	MinTLSVersion            string
 	CipherSuites             []string
 	TLSMinVersion            uint16   `json:"-"`

pkg/daemons/control/server.go

@@ -14,7 +14,6 @@ import (
 	"github.com/k3s-io/k3s/pkg/daemons/config"
 	"github.com/k3s-io/k3s/pkg/daemons/control/deps"
 	"github.com/k3s-io/k3s/pkg/daemons/executor"
-	"github.com/k3s-io/k3s/pkg/secretsencrypt"
 	"github.com/k3s-io/k3s/pkg/util"
 	"github.com/k3s-io/k3s/pkg/version"
 	"github.com/pkg/errors"
@@ -61,18 +60,6 @@ func Server(ctx context.Context, cfg *config.Control) error {
 		if err := apiServer(ctx, cfg); err != nil {
 			return err
 		}
-		if cfg.EncryptSecrets {
-			controllerName := "reencrypt-secrets"
-			cfg.Runtime.ClusterControllerStarts[controllerName] = func(ctx context.Context) {
-				// cfg.Runtime.Core is populated before this callback is triggered
-				if err := secretsencrypt.Register(ctx,
-					controllerName,
-					cfg,
-					cfg.Runtime.Core.Core().V1().Node()); err != nil {
-					logrus.Errorf("Failed to register %s controller: %v", controllerName, err)
-				}
-			}
-		}
 	}
 
 	// Wait for an apiserver to become available before starting additional controllers,

pkg/secretsencrypt/config.go

@@ -27,13 +27,19 @@ import (
 )
 
 const (
-	EncryptionStart             string = "start"
+	EncryptionStart             string  = "start"
-	EncryptionPrepare           string = "prepare"
+	EncryptionPrepare           string  = "prepare"
-	EncryptionRotate            string = "rotate"
+	EncryptionRotate            string  = "rotate"
-	EncryptionRotateKeys        string = "rotate_keys"
+	EncryptionRotateKeys        string  = "rotate_keys"
-	EncryptionReencryptRequest  string = "reencrypt_request"
+	EncryptionReencryptRequest  string  = "reencrypt_request"
-	EncryptionReencryptActive   string = "reencrypt_active"
+	EncryptionReencryptActive   string  = "reencrypt_active"
-	EncryptionReencryptFinished string = "reencrypt_finished"
+	EncryptionReencryptFinished string  = "reencrypt_finished"
+	SecretListPageSize          int64   = 20
+	SecretQPS                   float32 = 200
+	SecretBurst                 int     = 200
+	SecretsUpdateErrorEvent     string  = "SecretsUpdateError"
+	SecretsProgressEvent        string  = "SecretsProgress"
+	SecretsUpdateCompleteEvent  string  = "SecretsUpdateComplete"
 )
 
 var EncryptionHashAnnotation = version.Program + ".io/encryption-config-hash"
@@ -178,7 +184,9 @@ func BootstrapEncryptionHashAnnotation(node *corev1.Node, runtime *config.Contro
 	return nil
 }
 
-func WriteEncryptionHashAnnotation(runtime *config.ControlRuntime, node *corev1.Node, stage string) error {
+// WriteEncryptionHashAnnotation writes the encryption hash to the node annotation and optionally to a file.
+// The file is used to track the last stage of the reencryption process.
+func WriteEncryptionHashAnnotation(runtime *config.ControlRuntime, node *corev1.Node, skipFile bool, stage string) error {
 	encryptionConfigHash, err := GenEncryptionConfigHash(runtime)
 	if err != nil {
 		return err
@@ -192,6 +200,9 @@ func WriteEncryptionHashAnnotation(runtime *config.ControlRuntime, node *corev1.
 		return err
 	}
 	logrus.Debugf("encryption hash annotation set successfully on node: %s\n", node.ObjectMeta.Name)
+	if skipFile {
+		return nil
+	}
 	return os.WriteFile(runtime.EncryptionHash, []byte(ann), 0600)
 }
 

pkg/secretsencrypt/controller.go

@@ -1,246 +0,0 @@
-package secretsencrypt
-
-import (
-	"context"
-	"errors"
-	"fmt"
-	"strings"
-
-	"github.com/k3s-io/k3s/pkg/cluster"
-	"github.com/k3s-io/k3s/pkg/daemons/config"
-	"github.com/k3s-io/k3s/pkg/util"
-	coreclient "github.com/rancher/wrangler/v3/pkg/generated/controllers/core/v1"
-	"github.com/sirupsen/logrus"
-	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/tools/clientcmd"
-	"k8s.io/client-go/tools/pager"
-	"k8s.io/client-go/tools/record"
-	"k8s.io/client-go/util/retry"
-)
-
-const (
-	controllerAgentName        string = "reencrypt-controller"
-	secretsUpdateStartEvent    string = "SecretsUpdateStart"
-	secretsProgressEvent       string = "SecretsProgress"
-	secretsUpdateCompleteEvent string = "SecretsUpdateComplete"
-	secretsUpdateErrorEvent    string = "SecretsUpdateError"
-
-	secretListPageSize = 20
-)
-
-type handler struct {
-	ctx           context.Context
-	controlConfig *config.Control
-	nodes         coreclient.NodeController
-	k8s           *kubernetes.Clientset
-	recorder      record.EventRecorder
-}
-
-func Register(
-	ctx context.Context,
-	controllerName string,
-	controlConfig *config.Control,
-	nodes coreclient.NodeController,
-) error {
-	restConfig, err := clientcmd.BuildConfigFromFlags("", controlConfig.Runtime.KubeConfigSupervisor)
-	if err != nil {
-		return err
-	}
-	// For secrets we need a much higher QPS than what wrangler provides, so we create a new clientset
-	restConfig.QPS = 200
-	restConfig.Burst = 200
-	k8s, err := kubernetes.NewForConfig(restConfig)
-	if err != nil {
-		return err
-	}
-
-	h := &handler{
-		ctx:           ctx,
-		controlConfig: controlConfig,
-		nodes:         nodes,
-		k8s:           k8s,
-		recorder:      util.BuildControllerEventRecorder(k8s, controllerAgentName, metav1.NamespaceDefault),
-	}
-
-	nodes.OnChange(ctx, controllerName, h.onChangeNode)
-	return nil
-}
-
-// onChangeNode handles changes to Nodes. We are looking for a specific annotation change
-func (h *handler) onChangeNode(nodeName string, node *corev1.Node) (*corev1.Node, error) {
-	if node == nil {
-		return nil, nil
-	}
-
-	ann, ok := node.Annotations[EncryptionHashAnnotation]
-	if !ok {
-		return node, nil
-	}
-
-	// This is consistent with events attached to the node generated by the kubelet
-	// https://github.com/kubernetes/kubernetes/blob/612130dd2f4188db839ea5c2dea07a96b0ad8d1c/pkg/kubelet/kubelet.go#L479-L485
-	nodeRef := &corev1.ObjectReference{
-		Kind:      "Node",
-		Name:      node.Name,
-		UID:       types.UID(node.Name),
-		Namespace: "",
-	}
-
-	if valid, err := h.validateReencryptStage(node, ann); err != nil {
-		h.recorder.Event(nodeRef, corev1.EventTypeWarning, secretsUpdateErrorEvent, err.Error())
-		return node, err
-	} else if !valid {
-		return node, nil
-	}
-
-	reencryptHash, err := GenReencryptHash(h.controlConfig.Runtime, EncryptionReencryptActive)
-	if err != nil {
-		h.recorder.Event(nodeRef, corev1.EventTypeWarning, secretsUpdateErrorEvent, err.Error())
-		return node, err
-	}
-	ann = EncryptionReencryptActive + "-" + reencryptHash
-
-	err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
-		node, err = h.nodes.Get(nodeName, metav1.GetOptions{})
-		if err != nil {
-			return err
-		}
-		node.Annotations[EncryptionHashAnnotation] = ann
-		_, err = h.nodes.Update(node)
-		return err
-	})
-	if err != nil {
-		h.recorder.Event(nodeRef, corev1.EventTypeWarning, secretsUpdateErrorEvent, err.Error())
-		return node, err
-	}
-
-	if err := h.updateSecrets(nodeRef); err != nil {
-		h.recorder.Event(nodeRef, corev1.EventTypeWarning, secretsUpdateErrorEvent, err.Error())
-		return node, err
-	}
-
-	// If skipping, revert back to the previous stage
-	if h.controlConfig.EncryptSkip {
-		err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
-			node, err = h.nodes.Get(nodeName, metav1.GetOptions{})
-			if err != nil {
-				return err
-			}
-			BootstrapEncryptionHashAnnotation(node, h.controlConfig.Runtime)
-			_, err = h.nodes.Update(node)
-			return err
-		})
-		return node, err
-	}
-
-	// Remove last key
-	curKeys, err := GetEncryptionKeys(h.controlConfig.Runtime, false)
-	if err != nil {
-		h.recorder.Event(nodeRef, corev1.EventTypeWarning, secretsUpdateErrorEvent, err.Error())
-		return node, err
-	}
-
-	logrus.Infoln("Removing key: ", curKeys[len(curKeys)-1])
-	curKeys = curKeys[:len(curKeys)-1]
-	if err = WriteEncryptionConfig(h.controlConfig.Runtime, curKeys, true); err != nil {
-		h.recorder.Event(nodeRef, corev1.EventTypeWarning, secretsUpdateErrorEvent, err.Error())
-		return node, err
-	}
-
-	err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
-		node, err = h.nodes.Get(nodeName, metav1.GetOptions{})
-		if err != nil {
-			return err
-		}
-		return WriteEncryptionHashAnnotation(h.controlConfig.Runtime, node, EncryptionReencryptFinished)
-	})
-	if err != nil {
-		h.recorder.Event(nodeRef, corev1.EventTypeWarning, secretsUpdateErrorEvent, err.Error())
-		return node, err
-	}
-	if err := cluster.Save(h.ctx, h.controlConfig, true); err != nil {
-		h.recorder.Event(nodeRef, corev1.EventTypeWarning, secretsUpdateErrorEvent, err.Error())
-		return node, err
-	}
-	return node, nil
-}
-
-// validateReencryptStage ensures that the request for reencryption is valid and
-// that there is only one active reencryption at a time
-func (h *handler) validateReencryptStage(node *corev1.Node, annotation string) (bool, error) {
-	split := strings.Split(annotation, "-")
-	if len(split) != 2 {
-		err := fmt.Errorf("invalid annotation %s found on node %s", annotation, node.ObjectMeta.Name)
-		return false, err
-	}
-	stage := split[0]
-	hash := split[1]
-
-	// Validate the specific stage and the request via sha256 hash
-	if stage != EncryptionReencryptRequest {
-		return false, nil
-	}
-	if reencryptRequestHash, err := GenReencryptHash(h.controlConfig.Runtime, EncryptionReencryptRequest); err != nil {
-		return false, err
-	} else if reencryptRequestHash != hash {
-		err = fmt.Errorf("invalid hash: %s found on node %s", hash, node.ObjectMeta.Name)
-		return false, err
-	}
-	reencryptActiveHash, err := GenReencryptHash(h.controlConfig.Runtime, EncryptionReencryptActive)
-	if err != nil {
-		return false, err
-	}
-	labelSelector := labels.Set{util.ControlPlaneRoleLabelKey: "true"}.String()
-	nodes, err := h.nodes.List(metav1.ListOptions{LabelSelector: labelSelector})
-	if err != nil {
-		return false, err
-	}
-	for _, node := range nodes.Items {
-		if ann, ok := node.Annotations[EncryptionHashAnnotation]; ok {
-			split := strings.Split(ann, "-")
-			if len(split) != 2 {
-				return false, fmt.Errorf("invalid annotation %s found on node %s", ann, node.ObjectMeta.Name)
-			}
-			stage := split[0]
-			hash := split[1]
-			if stage == EncryptionReencryptActive && hash == reencryptActiveHash {
-				return false, fmt.Errorf("another reencrypt is already active")
-			}
-		}
-	}
-	return true, nil
-}
-
-func (h *handler) updateSecrets(nodeRef *corev1.ObjectReference) error {
-	secretPager := pager.New(pager.SimplePageFunc(func(opts metav1.ListOptions) (runtime.Object, error) {
-		return h.k8s.CoreV1().Secrets(metav1.NamespaceAll).List(h.ctx, opts)
-	}))
-	secretPager.PageSize = secretListPageSize
-
-	i := 0
-	if err := secretPager.EachListItem(h.ctx, metav1.ListOptions{}, func(obj runtime.Object) error {
-		secret, ok := obj.(*corev1.Secret)
-		if !ok {
-			return errors.New("failed to convert object to Secret")
-		}
-		if _, err := h.k8s.CoreV1().Secrets(secret.Namespace).Update(h.ctx, secret, metav1.UpdateOptions{}); err != nil && !apierrors.IsConflict(err) {
-			return fmt.Errorf("failed to update secret: %v", err)
-		}
-		if i != 0 && i%50 == 0 {
-			h.recorder.Eventf(nodeRef, corev1.EventTypeNormal, secretsProgressEvent, "reencrypted %d secrets", i)
-		}
-		i++
-		return nil
-	}); err != nil {
-		return err
-	}
-
-	h.recorder.Eventf(nodeRef, corev1.EventTypeNormal, secretsUpdateCompleteEvent, "completed reencrypt of %d secrets", i)
-	return nil
-}

pkg/server/secrets-encrypt.go

@@ -17,11 +17,19 @@ import (
 	"github.com/k3s-io/k3s/pkg/daemons/config"
 	"github.com/k3s-io/k3s/pkg/secretsencrypt"
 	"github.com/k3s-io/k3s/pkg/util"
+	"github.com/pkg/errors"
 	"github.com/rancher/wrangler/v3/pkg/generated/controllers/core"
 	"github.com/sirupsen/logrus"
+	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	apiserverconfigv1 "k8s.io/apiserver/pkg/apis/apiserver/v1"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/tools/clientcmd"
+	"k8s.io/client-go/tools/pager"
 	"k8s.io/client-go/util/retry"
 	"k8s.io/utils/ptr"
 )
@@ -150,8 +158,7 @@ func encryptionEnable(ctx context.Context, server *config.Control, enable bool)
 	if err := cluster.Save(ctx, server, true); err != nil {
 		return err
 	}
-	server.EncryptSkip = true
+	return reencryptAndRemoveKey(ctx, server, true, os.Getenv("NODE_NAME"))
-	return setReencryptAnnotation(server)
 }
 
 func encryptionConfigHandler(ctx context.Context, server *config.Control) http.Handler {
@@ -219,7 +226,7 @@ func encryptionPrepare(ctx context.Context, server *config.Control, force bool)
 		if err != nil {
 			return err
 		}
-		return secretsencrypt.WriteEncryptionHashAnnotation(server.Runtime, node, secretsencrypt.EncryptionPrepare)
+		return secretsencrypt.WriteEncryptionHashAnnotation(server.Runtime, node, false, secretsencrypt.EncryptionPrepare)
 	})
 	if err != nil {
 		return err
@@ -250,7 +257,7 @@ func encryptionRotate(ctx context.Context, server *config.Control, force bool) e
 		if err != nil {
 			return err
 		}
-		return secretsencrypt.WriteEncryptionHashAnnotation(server.Runtime, node, secretsencrypt.EncryptionRotate)
+		return secretsencrypt.WriteEncryptionHashAnnotation(server.Runtime, node, false, secretsencrypt.EncryptionRotate)
 	})
 	if err != nil {
 		return err
@@ -262,25 +269,20 @@ func encryptionReencrypt(ctx context.Context, server *config.Control, force bool
 	if err := verifyEncryptionHashAnnotation(server.Runtime, server.Runtime.Core.Core(), secretsencrypt.EncryptionRotate); err != nil && !force {
 		return err
 	}
-	server.EncryptForce = force
+	// Set the reencrypt-active annotation so other nodes know we are in the process of reencrypting.
-	server.EncryptSkip = skip
+	// As this stage is not persisted, we do not write the annotation to file
 	nodeName := os.Getenv("NODE_NAME")
-	node, err := server.Runtime.Core.Core().V1().Node().Get(nodeName, metav1.GetOptions{})
+	if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
-	if err != nil {
+		node, err := server.Runtime.Core.Core().V1().Node().Get(nodeName, metav1.GetOptions{})
+		if err != nil {
+			return err
+		}
+		return secretsencrypt.WriteEncryptionHashAnnotation(server.Runtime, node, true, secretsencrypt.EncryptionReencryptActive)
+	}); err != nil {
 		return err
 	}
 
-	reencryptHash, err := secretsencrypt.GenReencryptHash(server.Runtime, secretsencrypt.EncryptionReencryptRequest)
+	return reencryptAndRemoveKey(ctx, server, skip, nodeName)
-	if err != nil {
-		return err
-	}
-	ann := secretsencrypt.EncryptionReencryptRequest + "-" + reencryptHash
-	node.Annotations[secretsencrypt.EncryptionHashAnnotation] = ann
-	if _, err = server.Runtime.Core.Core().V1().Node().Update(node); err != nil {
-		return err
-	}
-	logrus.Debugf("encryption hash annotation set successfully on node: %s\n", node.ObjectMeta.Name)
-	return nil
 }
 
 func addAndRotateKeys(server *config.Control) error {
@@ -321,6 +323,19 @@ func encryptionRotateKeys(ctx context.Context, server *config.Control) error {
 		return err
 	}
 
+	// Set the reencrypt-active annotation so other nodes know we are in the process of reencrypting.
+	// As this stage is not persisted, we do not write the annotation to file
+	nodeName := os.Getenv("NODE_NAME")
+	if err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		node, err := server.Runtime.Core.Core().V1().Node().Get(nodeName, metav1.GetOptions{})
+		if err != nil {
+			return err
+		}
+		return secretsencrypt.WriteEncryptionHashAnnotation(server.Runtime, node, true, secretsencrypt.EncryptionReencryptActive)
+	}); err != nil {
+		return err
+	}
+
 	if err := addAndRotateKeys(server); err != nil {
 		return err
 	}
@@ -329,26 +344,100 @@ func encryptionRotateKeys(ctx context.Context, server *config.Control) error {
 		return err
 	}
 
-	return setReencryptAnnotation(server)
+	return reencryptAndRemoveKey(ctx, server, false, nodeName)
 }
 
-func setReencryptAnnotation(server *config.Control) error {
+func reencryptAndRemoveKey(ctx context.Context, server *config.Control, skip bool, nodeName string) error {
-	nodeName := os.Getenv("NODE_NAME")
+	if err := updateSecrets(ctx, server, nodeName); err != nil {
-	node, err := server.Runtime.Core.Core().V1().Node().Get(nodeName, metav1.GetOptions{})
+		return err
+	}
+
+	// If skipping, revert back to the previous stage and do not remove the key
+	if skip {
+		err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+			node, err := server.Runtime.Core.Core().V1().Node().Get(nodeName, metav1.GetOptions{})
+			if err != nil {
+				return err
+			}
+			secretsencrypt.BootstrapEncryptionHashAnnotation(node, server.Runtime)
+			_, err = server.Runtime.Core.Core().V1().Node().Update(node)
+			return err
+		})
+		return err
+	}
+
+	// Remove last key
+	curKeys, err := secretsencrypt.GetEncryptionKeys(server.Runtime, false)
 	if err != nil {
 		return err
 	}
 
-	reencryptHash, err := secretsencrypt.GenReencryptHash(server.Runtime, secretsencrypt.EncryptionReencryptRequest)
+	logrus.Infoln("Removing key: ", curKeys[len(curKeys)-1])
+	curKeys = curKeys[:len(curKeys)-1]
+	if err = secretsencrypt.WriteEncryptionConfig(server.Runtime, curKeys, true); err != nil {
+		return err
+	}
+
+	if err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		node, err := server.Runtime.Core.Core().V1().Node().Get(nodeName, metav1.GetOptions{})
+		if err != nil {
+			return err
+		}
+		return secretsencrypt.WriteEncryptionHashAnnotation(server.Runtime, node, false, secretsencrypt.EncryptionReencryptFinished)
+	}); err != nil {
+		return err
+	}
+
+	return cluster.Save(ctx, server, true)
+}
+
+func updateSecrets(ctx context.Context, server *config.Control, nodeName string) error {
+	restConfig, err := clientcmd.BuildConfigFromFlags("", server.Runtime.KubeConfigSupervisor)
 	if err != nil {
 		return err
 	}
-	ann := secretsencrypt.EncryptionReencryptRequest + "-" + reencryptHash
+	// For secrets we need a much higher QPS than default
-	node.Annotations[secretsencrypt.EncryptionHashAnnotation] = ann
+	restConfig.QPS = secretsencrypt.SecretQPS
-	if _, err = server.Runtime.Core.Core().V1().Node().Update(node); err != nil {
+	restConfig.Burst = secretsencrypt.SecretBurst
+	k8s, err := kubernetes.NewForConfig(restConfig)
+	if err != nil {
 		return err
 	}
-	logrus.Debugf("encryption hash annotation set successfully on node: %s\n", node.ObjectMeta.Name)
+
+	nodeRef := &corev1.ObjectReference{
+		Kind:      "Node",
+		Name:      nodeName,
+		UID:       types.UID(nodeName),
+		Namespace: "",
+	}
+
+	// For backwards compatibility with the old controller, we use an event recorder instead of logrus
+	recorder := util.BuildControllerEventRecorder(k8s, "secrets-reencrypt", metav1.NamespaceDefault)
+
+	secretPager := pager.New(pager.SimplePageFunc(func(opts metav1.ListOptions) (runtime.Object, error) {
+		return k8s.CoreV1().Secrets(metav1.NamespaceAll).List(ctx, opts)
+	}))
+	secretPager.PageSize = secretsencrypt.SecretListPageSize
+
+	i := 0
+	if err := secretPager.EachListItem(ctx, metav1.ListOptions{}, func(obj runtime.Object) error {
+		secret, ok := obj.(*corev1.Secret)
+		if !ok {
+			return errors.New("failed to convert object to Secret")
+		}
+		if _, err := k8s.CoreV1().Secrets(secret.Namespace).Update(ctx, secret, metav1.UpdateOptions{}); err != nil && !apierrors.IsConflict(err) {
+			recorder.Eventf(nodeRef, corev1.EventTypeWarning, secretsencrypt.SecretsUpdateErrorEvent, "failed to update secret: %v", err)
+			return fmt.Errorf("failed to update secret: %v", err)
+		}
+		if i != 0 && i%50 == 0 {
+			recorder.Eventf(nodeRef, corev1.EventTypeNormal, secretsencrypt.SecretsProgressEvent, "reencrypted %d secrets", i)
+		}
+		i++
+		return nil
+	}); err != nil {
+		return err
+	}
+	recorder.Eventf(nodeRef, corev1.EventTypeNormal, secretsencrypt.SecretsUpdateCompleteEvent, "reencrypted %d secrets", i)
 	return nil
 }