pkg/operator/encryption: Make key_controller KMS config aware

Signed-off-by: Swarup Ghosh <swghosh@redhat.com>

Author: swghosh

pkg/operator/encryption/controllers/key_controller.go

@@ -20,13 +20,15 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/utils/ptr"
 
+	configv1 "github.com/openshift/api/config/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
 	configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1"
 	applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
 
 	"github.com/openshift/library-go/pkg/controller/factory"
 	"github.com/openshift/library-go/pkg/operator/encryption/crypto"
+	"github.com/openshift/library-go/pkg/operator/encryption/encryptionconfig"
 	"github.com/openshift/library-go/pkg/operator/encryption/secrets"
 	"github.com/openshift/library-go/pkg/operator/encryption/state"
 	"github.com/openshift/library-go/pkg/operator/encryption/statemachine"
@@ -159,7 +161,7 @@ func (c *keyController) sync(ctx context.Context, syncCtx factory.SyncContext) (
 }
 
 func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext factory.SyncContext, encryptedGRs []schema.GroupResource) error {
-	currentMode, externalReason, err := c.getCurrentModeAndExternalReason(ctx)
+	currentKeyState, err := c.getCurrentEncryptionModeWithExternalReason(ctx)
 	if err != nil {
 		return err
 	}
@@ -175,13 +177,15 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact
 
 	// avoid intended start of encryption
 	hasBeenOnBefore := currentConfig != nil || len(secrets) > 0
-	if currentMode == state.Identity && !hasBeenOnBefore {
+	if currentKeyState.Mode == state.Identity && !hasBeenOnBefore {
 		return nil
 	}
 
 	var (
 		newKeyRequired bool
 		newKeyID       uint64
+		latestKeyID    uint64
+		ok             bool
 		reasons        []string
 	)
 
@@ -191,23 +195,34 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact
 
 	var commonReason *string
 	for gr, grKeys := range desiredEncryptionState {
-		latestKeyID, internalReason, needed := needsNewKey(grKeys, currentMode, externalReason, encryptedGRs)
+		// if kmsKeyID in GR ReadKey is not the same as current kmsKeyID, needed is true.
+		ks, needed := needsNewKeyWithInternalReason(grKeys, currentKeyState.Mode, currentKeyState.KMSKeyID, currentKeyState.ExternalReason, encryptedGRs)
 		if !needed {
 			continue
 		}
 
+		if ks.Mode != state.KMS {
+			latestKeyID, ok = state.NameToKeyID(ks.Key.Name)
+			if !ok {
+				latestKeyID = 0
+			}
+		}
+
 		if commonReason == nil {
-			commonReason = &internalReason
-		} else if *commonReason != internalReason {
+			commonReason = &ks.InternalReason
+		} else if *commonReason != ks.InternalReason {
 			commonReason = ptr.To("") // this means we have no common reason
 		}
 
 		newKeyRequired = true
+
+		// tracking the newKeyID is only required for non-KMS
 		nextKeyID := latestKeyID + 1
-		if newKeyID < nextKeyID {
+		if ks.Mode != state.KMS && newKeyID < nextKeyID {
 			newKeyID = nextKeyID
 		}
-		reasons = append(reasons, fmt.Sprintf("%s-%s", gr.Resource, internalReason))
+
+		reasons = append(reasons, fmt.Sprintf("%s-%s", gr.Resource, ks.InternalReason))
 	}
 	if !newKeyRequired {
 		return nil
@@ -216,12 +231,20 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact
 		reasons = []string{*commonReason} // don't repeat reasons
 	}
 
-	sort.Sort(sort.StringSlice(reasons))
-	internalReason := strings.Join(reasons, ", ")
-	keySecret, err := c.generateKeySecret(newKeyID, currentMode, internalReason, externalReason)
+	sort.Strings(reasons)
+	currentKeyState.InternalReason = strings.Join(reasons, ", ")
+
+	var keySecret *corev1.Secret
+	if currentKeyState.Mode == state.KMS {
+		keySecret, err = c.generateKMSKeySecret(currentKeyState.KMSConfig, currentKeyState.InternalReason, currentKeyState.ExternalReason)
+	} else {
+		keySecret, err = c.generateLocalKeySecret(newKeyID, currentKeyState.Mode, currentKeyState.InternalReason, currentKeyState.ExternalReason)
+	}
+
 	if err != nil {
 		return fmt.Errorf("failed to create key: %v", err)
 	}
+
 	_, createErr := c.secretClient.Secrets("openshift-config-managed").Create(ctx, keySecret, metav1.CreateOptions{})
 	if errors.IsAlreadyExists(createErr) {
 		return c.validateExistingSecret(ctx, keySecret, newKeyID)
@@ -242,20 +265,31 @@ func (c *keyController) validateExistingSecret(ctx context.Context, keySecret *c
 		return err
 	}
 
+	ks, err := secrets.ToKeyState(actualKeySecret)
+	if err != nil {
+		return fmt.Errorf("secret %s/%s is invalid, new keys cannot be created for encryption target", keySecret.Namespace, keySecret.Name)
+	}
+
+	if ks.Mode == state.KMS && ks.KMSKeyID != "" {
+		return nil
+	}
+
+	if ks.Mode == state.KMS && ks.KMSKeyID == "" {
+		// kmsKeyID is mandatory in case of KMS
+		return fmt.Errorf("secret %s/%s is invalid, new KMS keys cannot be created for encryption target", keySecret.Namespace, keySecret.Name)
+	}
+
+	// checks for local aes (non-KMS) keys only
 	actualKeyID, ok := state.NameToKeyID(actualKeySecret.Name)
 	if !ok || actualKeyID != keyID {
 		// TODO we can just get stuck in degraded here ...
-		return fmt.Errorf("secret %s has an invalid name, new keys cannot be created for encryption target", keySecret.Name)
-	}
-
-	if _, err := secrets.ToKeyState(actualKeySecret); err != nil {
-		return fmt.Errorf("secret %s is invalid, new keys cannot be created for encryption target", keySecret.Name)
+		return fmt.Errorf("secret %s/%s has an invalid name, new keys cannot be created for encryption target", keySecret.Namespace, keySecret.Name)
 	}
 
 	return nil // we made this key earlier
 }
 
-func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode, internalReason, externalReason string) (*corev1.Secret, error) {
+func (c *keyController) generateLocalKeySecret(keyID uint64, currentMode state.Mode, internalReason, externalReason string) (*corev1.Secret, error) {
 	bs := crypto.ModeToNewKeyFunc[currentMode]()
 	ks := state.KeyState{
 		Key: apiserverv1.Key{
@@ -269,50 +303,85 @@ func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode,
 	return secrets.FromKeyState(c.instanceName, ks)
 }
 
-func (c *keyController) getCurrentModeAndExternalReason(ctx context.Context) (state.Mode, string, error) {
+func (c *keyController) generateKMSKeySecret(kmsConfig *configv1.KMSConfig, internalReason, externalReason string) (*corev1.Secret, error) {
+	kmsConfig = kmsConfig.DeepCopy()
+
+	kmsKeyID, err := encryptionconfig.HashKMSConfig(*kmsConfig)
+	if err != nil {
+		return nil, err
+	}
+
+	ks := state.KeyState{
+		Mode:           state.KMS,
+		InternalReason: internalReason,
+		ExternalReason: externalReason,
+		KMSKeyID:       kmsKeyID,
+		KMSConfig:      kmsConfig,
+	}
+	return secrets.FromKeyState(c.instanceName, ks)
+}
+
+func (c *keyController) getCurrentEncryptionModeWithExternalReason(ctx context.Context) (state.KeyState, error) {
 	apiServer, err := c.apiServerClient.Get(ctx, "cluster", metav1.GetOptions{})
 	if err != nil {
-		return "", "", err
+		return state.KeyState{}, err
 	}
 
 	operatorSpec, _, _, err := c.operatorClient.GetOperatorState()
 	if err != nil {
-		return "", "", err
+		return state.KeyState{}, err
 	}
 
 	encryptionConfig, err := structuredUnsupportedConfigFrom(operatorSpec.UnsupportedConfigOverrides.Raw, c.unsupportedConfigPrefix)
 	if err != nil {
-		return "", "", err
+		return state.KeyState{}, err
 	}
 
 	reason := encryptionConfig.Encryption.Reason
 	switch currentMode := state.Mode(apiServer.Spec.Encryption.Type); currentMode {
 	case state.AESCBC, state.AESGCM, state.Identity: // secretbox is disabled for now
-		return currentMode, reason, nil
+		return state.KeyState{Mode: currentMode, ExternalReason: reason}, nil
+	case state.KMS:
+		kmsConfig := apiServer.Spec.Encryption.KMS.DeepCopy()
+
+		kmsKeyID, err := encryptionconfig.HashKMSConfig(*kmsConfig)
+		if err != nil {
+			return state.KeyState{}, fmt.Errorf("encryption mode configured: %s, but provided kms config could not generate required kms key id %v", currentMode, err)
+		}
+
+		ks := state.KeyState{
+			Mode:           state.KMS,
+			KMSKeyID:       kmsKeyID,
+			KMSConfig:      kmsConfig,
+			ExternalReason: reason,
+		}
+		return ks, nil
 	case "": // unspecified means use the default (which can change over time)
-		return state.DefaultMode, reason, nil
+		return state.KeyState{Mode: state.DefaultMode, ExternalReason: reason}, nil
 	default:
-		return "", "", fmt.Errorf("unknown encryption mode configured: %s", currentMode)
+		return state.KeyState{}, fmt.Errorf("unknown encryption mode configured: %s", currentMode)
 	}
 }
 
-// needsNewKey checks whether a new key must be created for the given resource. If true, it also returns the latest
+// needsNewKeyWithInternalReason checks whether a new key must be created for the given resource. If true, it also returns the latest
 // used key ID and a reason string.
-func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, externalReason string, encryptedGRs []schema.GroupResource) (uint64, string, bool) {
+func needsNewKeyWithInternalReason(grKeys state.GroupResourceState, currentMode state.Mode, optionalCurrentKMSKeyID string, externalReason string, encryptedGRs []schema.GroupResource) (state.KeyState, bool) {
 	// we always need to have some encryption keys unless we are turned off
 	if len(grKeys.ReadKeys) == 0 {
-		return 0, "key-does-not-exist", currentMode != state.Identity
+		return state.KeyState{InternalReason: "key-does-not-exist"}, currentMode != state.Identity
 	}
 
 	latestKey := grKeys.ReadKeys[0]
 	latestKeyID, ok := state.NameToKeyID(latestKey.Key.Name)
 	if !ok {
-		return latestKeyID, fmt.Sprintf("key-secret-%d-is-invalid", latestKeyID), true
+		latestKey.InternalReason = fmt.Sprintf("key-secret-%d-is-invalid", latestKeyID)
+		return latestKey, true
 	}
 
 	// if latest secret has been deleted, we will never be able to migrate to that key.
 	if !latestKey.Backed {
-		return latestKeyID, fmt.Sprintf("encryption-config-key-%d-not-backed-by-secret", latestKeyID), true
+		latestKey.InternalReason = fmt.Sprintf("encryption-config-key-%d-not-backed-by-secret", latestKeyID)
+		return latestKey, true
 	}
 
 	// check that we have pruned read-keys: the write-keys, plus at most one more backed read-key (potentially some unbacked once before)
@@ -323,32 +392,41 @@ func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, extern
 		}
 	}
 	if backedKeys > 2 {
-		return 0, "", false
+		return state.KeyState{}, false
 	}
 
 	// we have not migrated the latest key, do nothing until that is complete
 	if allMigrated, _, _ := state.MigratedFor(encryptedGRs, latestKey); !allMigrated {
-		return 0, "", false
+		return state.KeyState{}, false
 	}
 
 	// if the most recent secret was encrypted in a mode different than the current mode, we need to generate a new key
 	if latestKey.Mode != currentMode {
-		return latestKeyID, "encryption-mode-changed", true
+		latestKey.InternalReason = "encryption-mode-changed"
+		return latestKey, true
 	}
 
 	// if the most recent secret turned off encryption and we want to keep it that way, do nothing
 	if latestKey.Mode == state.Identity && currentMode == state.Identity {
-		return 0, "", false
+		return state.KeyState{}, false
+	}
+
+	// if the hash of the kms config (kmsKeyID) has updated, we need a new KMS backing secret
+	if currentMode == state.KMS && latestKey.KMSKeyID != optionalCurrentKMSKeyID {
+		latestKey.InternalReason = "kms-config-changed"
+		return latestKey, true
 	}
 
 	// if the most recent secret has a different external reason than the current reason, we need to generate a new key
 	if latestKey.ExternalReason != externalReason && len(externalReason) != 0 {
-		return latestKeyID, "external-reason-changed", true
+		latestKey.InternalReason = "external-reason-changed"
+		return latestKey, true
 	}
 
 	// we check for encryptionSecretMigratedTimestamp set by migration controller to determine when migration completed
 	// this also generates back pressure for key rotation when migration takes a long time or was recently completed
-	return latestKeyID, "rotation-interval-has-passed", time.Since(latestKey.Migrated.Timestamp) > encryptionSecretMigrationInterval
+	latestKey.InternalReason = "rotation-interval-has-passed"
+	return latestKey, time.Since(latestKey.Migrated.Timestamp) > encryptionSecretMigrationInterval
 }
 
 // TODO make this un-settable once set

pkg/operator/encryption/controllers/key_controller_test.go

@@ -5,10 +5,11 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
-	clocktesting "k8s.io/utils/clock/testing"
 	"testing"
 	"time"
 
+	clocktesting "k8s.io/utils/clock/testing"
+
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -281,11 +282,11 @@ func TestKeyController(t *testing.T) {
 					Type:    "EncryptionKeyControllerDegraded",
 					Status:  "True",
 					Reason:  "Error",
-					Message: "secret encryption-key-kms-1 is invalid, new keys cannot be created for encryption target",
+					Message: "secret openshift-config-managed/encryption-key-kms-1 is invalid, new keys cannot be created for encryption target",
 				}
 				encryptiontesting.ValidateOperatorClientConditions(ts, operatorClient, []operatorv1.OperatorCondition{expectedCondition})
 			},
-			expectedError: errors.New("secret encryption-key-kms-1 is invalid, new keys cannot be created for encryption target"),
+			expectedError: errors.New("secret openshift-config-managed/encryption-key-kms-1 is invalid, new keys cannot be created for encryption target"),
 		},
 
 		{
@@ -494,7 +495,8 @@ func TestGetCurrentModeAndExternalReason(t *testing.T) {
 
 			// act
 			target := keyController{unsupportedConfigPrefix: scenario.prefix, operatorClient: fakeOperatorClient, apiServerClient: fakeApiServerClient}
-			_, externalReason, err := target.getCurrentModeAndExternalReason(context.TODO())
+			ks, err := target.getCurrentEncryptionModeWithExternalReason(context.TODO())
+			externalReason := ks.ExternalReason
 
 			// validate
 			if err != nil {

pkg/operator/encryption/state/helpers.go

@@ -51,6 +51,7 @@ func KeysWithPotentiallyPersistedDataAndNextReadKey(grs []schema.GroupResource,
 	return recentFirstSortedKeys
 }
 
+// TODO: make sure sort takes into account for KMS keys
 func SortRecentFirst(unsorted []KeyState) []KeyState {
 	ret := make([]KeyState, len(unsorted))
 	copy(ret, unsorted)