CNTRLPLANE-3237: Carry referenced Secret data from API to plugin lifecycle for KMS

pkg/operator/encryption/controllers/key_controller.go

@@ -116,7 +116,9 @@ func NewKeyController(
 		WithInformers(
 			apiServerInformer.Informer(),
 			operatorClient.Informer(),
+
 			kubeInformersForNamespaces.InformersFor("openshift-config-managed").Core().V1().Secrets().Informer(),
+			// TODO: add informer for openshift-config namespace to watch referenced Secrets for KMS plugin secret data changes
 			deployer,
 		).ToController(
 		c.controllerInstanceName,
@@ -223,7 +225,7 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact
 
 	sort.Sort(sort.StringSlice(reasons))
 	internalReason := strings.Join(reasons, ", ")
-	keySecret, err := c.generateKeySecret(newKeyID, currentMode, apiEncryptionConfiguration, internalReason, externalReason)
+	keySecret, err := c.generateKeySecret(ctx, newKeyID, currentMode, apiEncryptionConfiguration, internalReason, externalReason)
 	if err != nil {
 		return fmt.Errorf("failed to create key: %v", err)
 	}
@@ -260,7 +262,7 @@ func (c *keyController) validateExistingSecret(ctx context.Context, keySecret *c
 	return nil // we made this key earlier
 }
 
-func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode, apiServerEncryption configv1.APIServerEncryption, internalReason, externalReason string) (*corev1.Secret, error) {
+func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, currentMode state.Mode, apiServerEncryption configv1.APIServerEncryption, internalReason, externalReason string) (*corev1.Secret, error) {
 	bs := crypto.ModeToNewKeyFunc[currentMode]()
 	ks := state.KeyState{
 		Key: apiserverv1.Key{
@@ -281,6 +283,22 @@ func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode,
 			},
 			Plugin: apiServerEncryption.KMS,
 		}
+
+		if secretName, expectedKeys := referencedSecretName(apiServerEncryption.KMS); len(secretName) > 0 {
+			refSecret, err := c.secretClient.Secrets("openshift-config").Get(ctx, secretName, metav1.GetOptions{})
+			if err != nil {
+				return nil, fmt.Errorf("failed to get secret %s in openshift-config: %w", secretName, err)
+			}
+			sd := make(map[string][]byte, len(expectedKeys))
+			for _, key := range expectedKeys {
+				v, ok := refSecret.Data[key]
+				if !ok {
+					return nil, fmt.Errorf("secret %s in openshift-config is missing required key %q", secretName, key)
+				}
+				sd[secretName+"-"+key] = v
+			}
+			ks.KMS.SecretData = sd
+		}
 	}
 	return secrets.FromKeyState(c.instanceName, ks)
 }
@@ -383,6 +401,15 @@ func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, extern
 	return latestKeyID, "rotation-interval-has-passed", time.Since(latestKey.Migrated.Timestamp) > encryptionSecretMigrationInterval
 }
 
+func referencedSecretName(plugin configv1.KMSPluginConfig) (string, []string) {
+	switch plugin.Type {
+	case configv1.VaultKMSProvider:
+		return plugin.Vault.Authentication.AppRole.Secret.Name, []string{"role-id", "secret-id"}
+	default:
+		return "", nil
+	}
+}
+
 // TODO make this un-settable once set
 // ex: we could require the tech preview no upgrade flag to be set before we will honor this field
 type unsupportedEncryptionConfig struct {

pkg/operator/encryption/controllers/key_controller_test.go

@@ -336,9 +336,10 @@ func TestKeyController(t *testing.T) {
 				{Group: "", Resource: "secrets"},
 			},
 			targetNamespace: "kms",
-			expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "create:secrets:openshift-config-managed", "create:events:kms"},
+			expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "create:secrets:openshift-config-managed", "create:events:kms"},
 			initialObjects: []runtime.Object{
 				encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
+				encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"),
 			},
 			apiServerObjects: []runtime.Object{apiServerWithKMS},
 			validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) {
@@ -381,6 +382,14 @@ func TestKeyController(t *testing.T) {
 							ts.Errorf("unexpected kms-plugin-config: %s", kmsPluginConfigData)
 						}
 
+						// Verify secret data is carried
+						if roleID := string(actualSecret.Data["encryption.apiserver.operator.openshift.io-kms-plugin-secret-vault-approle-secret-role-id"]); roleID != "test-role-id" {
+							ts.Errorf("expected role-id secret data to be 'test-role-id', got %q", roleID)
+						}
+						if secretID := string(actualSecret.Data["encryption.apiserver.operator.openshift.io-kms-plugin-secret-vault-approle-secret-secret-id"]); secretID != "test-secret-id" {
+							ts.Errorf("expected secret-id secret data to be 'test-secret-id', got %q", secretID)
+						}
+
 						// Verify internal reason
 						if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"] != "secrets-key-does-not-exist" {
 							ts.Errorf("unexpected internal reason: %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"])
@@ -418,10 +427,11 @@ func TestKeyController(t *testing.T) {
 			initialObjects: []runtime.Object{
 				encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
 				encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5, []byte("61def964fb967f5d7c44a2af8dab6865"), "aescbc"),
+				encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"),
 			},
 			apiServerObjects: []runtime.Object{apiServerWithKMS},
 			targetNamespace:  "kms",
-			expectedActions:  []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "create:secrets:openshift-config-managed", "create:events:kms"},
+			expectedActions:  []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "create:secrets:openshift-config-managed", "create:events:kms"},
 			validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) {
 				wasSecretValidated := false
 				for _, action := range actions {
@@ -512,10 +522,11 @@ func TestKeyController(t *testing.T) {
 			initialObjects: []runtime.Object{
 				encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
 				encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5, []byte("identity-key"), "identity"),
+				encryptiontesting.CreateVaultAppRoleSecret("vault-approle-secret", "test-role-id", "test-secret-id"),
 			},
 			apiServerObjects: []runtime.Object{apiServerWithKMS},
 			targetNamespace:  "kms",
-			expectedActions:  []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "create:secrets:openshift-config-managed", "create:events:kms"},
+			expectedActions:  []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config", "create:secrets:openshift-config-managed", "create:events:kms"},
 			validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) {
 				wasSecretValidated := false
 				for _, action := range actions {
@@ -667,7 +678,7 @@ func TestKeyController(t *testing.T) {
 			// - target namespace: pods and secrets
 			// - openshift-config-managed: secrets
 			// note that the informer factory is not used in the test - it's only needed to create the controller
-			kubeInformers := v1helpers.NewKubeInformersForNamespaces(fakeKubeClient, "openshift-config-managed", scenario.targetNamespace)
+			kubeInformers := v1helpers.NewKubeInformersForNamespaces(fakeKubeClient, "openshift-config-managed", "openshift-config", scenario.targetNamespace)
 			fakeSecretClient := fakeKubeClient.CoreV1()
 			fakePodClient := fakeKubeClient.CoreV1()
 			fakeConfigClient := configv1clientfake.NewSimpleClientset(scenario.apiServerObjects...)

pkg/operator/encryption/encryptiondata/config.go

@@ -30,6 +30,10 @@ type Config struct {
 	// KMSPlugins maps keyID to plugin-specific configuration,
 	// carried from Key Secrets into the encryption-config Secret.
 	KMSPlugins map[string]configv1.KMSPluginConfig
+	// KMSSecretData maps keyID to secret data carried from
+	// Key Secrets into the encryption-config Secret.
+	// Inner map keys use the short form (e.g. "app-role-role-id").
+	KMSSecretData map[string]map[string][]byte
 }
 
 func (c *Config) HasEncryptionConfiguration() bool {
@@ -40,18 +44,19 @@ func (c *Config) HasEncryptionConfiguration() bool {
 func FromEncryptionState(encryptionState map[schema.GroupResource]state.GroupResourceState) (*Config, error) {
 	resourceConfigs := make([]apiserverconfigv1.ResourceConfiguration, 0, len(encryptionState))
 	var kmsPlugins map[string]configv1.KMSPluginConfig
+	var kmsSecretData map[string]map[string][]byte
 
 	for gr, grKeys := range encryptionState {
 		resourceConfigs = append(resourceConfigs, apiserverconfigv1.ResourceConfiguration{
 			Resources: []string{gr.String()}, // we are forced to lose data here because this API is broken
 			Providers: stateToProviders(gr.Resource, grKeys),
 		})
 
-		// Collect KMS plugin configs from read keys (which already include the write key).
-		// We iterate over encryptionState which is keyed by GroupResource, so the same
-		// keyID is seen once per resource (e.g. key "1" for secrets and key "1" for configmaps).
-		// Since all resources share the same Key Secret, the plugin config is identical
-		// across duplicates and we only need to keep the first occurrence.
+		// Collect KMS plugin configs and secret data from read keys (which already
+		// include the write key). We iterate over encryptionState which is keyed by
+		// GroupResource, so the same keyID is seen once per resource. Since all
+		// resources share the same Key Secret, the plugin config and secret data are
+		// identical across duplicates and we only need to keep the first occurrence.
 		for _, key := range grKeys.ReadKeys {
 			if key.HasKMSPlugin() {
 				if kmsPlugins == nil {
@@ -67,6 +72,18 @@ func FromEncryptionState(encryptionState map[schema.GroupResource]state.GroupRes
 					kmsPlugins[key.Key.Name] = key.KMS.Plugin
 				}
 			}
+			if key.HasKMSSecretData() {
+				if kmsSecretData == nil {
+					kmsSecretData = map[string]map[string][]byte{}
+				}
+				if existing, exists := kmsSecretData[key.Key.Name]; exists {
+					if !equality.Semantic.DeepEqual(existing, key.KMS.SecretData) {
+						return nil, fmt.Errorf("KMS secret data mismatch for keyID %s: secret data from different resources must be identical", key.Key.Name)
+					}
+				} else {
+					kmsSecretData[key.Key.Name] = key.KMS.SecretData
+				}
+			}
 		}
 	}
 
@@ -76,8 +93,9 @@ func FromEncryptionState(encryptionState map[schema.GroupResource]state.GroupRes
 	})
 
 	return &Config{
-		Encryption: &apiserverconfigv1.EncryptionConfiguration{Resources: resourceConfigs},
-		KMSPlugins: kmsPlugins,
+		Encryption:    &apiserverconfigv1.EncryptionConfiguration{Resources: resourceConfigs},
+		KMSPlugins:    kmsPlugins,
+		KMSSecretData: kmsSecretData,
 	}, nil
 }
 

pkg/operator/encryption/encryptiondata/config_test.go

@@ -851,6 +851,39 @@ func TestSecretRoundtrip(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "KMS with provider config and secret data",
+			cfg: &encryptiondata.Config{
+				Encryption: &apiserverconfigv1.EncryptionConfiguration{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "EncryptionConfiguration",
+						APIVersion: "apiserver.config.k8s.io/v1",
+					},
+					Resources: []apiserverconfigv1.ResourceConfiguration{{
+						Resources: []string{"secrets"},
+						Providers: []apiserverconfigv1.ProviderConfiguration{{
+							KMS: &apiserverconfigv1.KMSConfiguration{
+								APIVersion: "v2",
+								Name:       "1_secrets",
+								Endpoint:   "unix:///var/run/kmsplugin/kms-1.sock",
+								Timeout:    &metav1.Duration{Duration: 10 * time.Second},
+							},
+						}, {
+							Identity: &apiserverconfigv1.IdentityConfiguration{},
+						}},
+					}},
+				},
+				KMSPlugins: map[string]configv1.KMSPluginConfig{
+					"1": encryptiontesting.DefaultKMSPluginConfig,
+				},
+				KMSSecretData: map[string]map[string][]byte{
+					"1": {
+						"vault-approle-secret-role-id":   []byte("test-role-id"),
+						"vault-approle-secret-secret-id": []byte("test-secret-id"),
+					},
+				},
+			},
+		},
 		{
 			name: "KMS with multiple provider configs",
 			cfg: &encryptiondata.Config{

pkg/operator/encryption/encryptiondata/secret.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"sort"
 	"strconv"
+	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -17,11 +18,15 @@ import (
 	"github.com/openshift/library-go/pkg/operator/encryption/state"
 )
 
-// EncryptionConfSecretName is the name of the final encryption config secret that is revisioned per apiserver rollout.
-const EncryptionConfSecretName = "encryption-config"
-
-// EncryptionConfSecretKey is the map data key used to store the raw bytes of the final encryption config.
-const EncryptionConfSecretKey = "encryption-config"
+const (
+	// EncryptionConfSecretName is the name of the final encryption config secret that is revisioned per apiserver rollout.
+	EncryptionConfSecretName = "encryption-config"
+	// EncryptionConfSecretKey is the map data key used to store the raw bytes of the final encryption config.
+	EncryptionConfSecretKey = "encryption-config"
+	// encryptionConfigSecretDataPrefix is the data key prefix for KMS plugin secret
+	// data entries in the encryption-config Secret. Full key: "kms-plugin-secret-{secretDataKey}-{keyID}".
+	encryptionConfigSecretDataPrefix = "kms-plugin-secret-"
+)
 
 func FromSecret(encryptionConfigSecret *corev1.Secret) (*Config, error) {
 	data, ok := encryptionConfigSecret.Data[EncryptionConfSecretKey]
@@ -56,7 +61,29 @@ func FromSecret(encryptionConfigSecret *corev1.Secret) (*Config, error) {
 		kmsPlugins[keyID] = pluginConfig
 	}
 
-	return &Config{Encryption: encryptionConfig, KMSPlugins: kmsPlugins}, nil
+	// Extract secret data entries from the encryption-config Secret.
+	// Data keys follow the format "kms-plugin-secret-{secretDataKey}-{keyID}"
+	// (e.g. "kms-plugin-secret-app-role-role-id-1"). KeyIDFromSecretDataKey
+	// returns the keyID (e.g. "1") and the secretDataKey (e.g. "app-role-role-id").
+	var kmsSecretData map[string]map[string][]byte
+	for key, value := range encryptionConfigSecret.Data {
+		keyID, secretDataKey, found, err := keyIDFromSecretDataKey(key)
+		if err != nil {
+			return nil, fmt.Errorf("failed to extract keyID from secret data key %s: %w", key, err)
+		}
+		if !found {
+			continue
+		}
+		if kmsSecretData == nil {
+			kmsSecretData = map[string]map[string][]byte{}
+		}
+		if kmsSecretData[keyID] == nil {
+			kmsSecretData[keyID] = map[string][]byte{}
+		}
+		kmsSecretData[keyID][secretDataKey] = value
+	}
+
+	return &Config{Encryption: encryptionConfig, KMSPlugins: kmsPlugins, KMSSecretData: kmsSecretData}, nil
 }
 
 func ToSecret(ns, name string, secretData *Config) (*corev1.Secret, error) {
@@ -100,6 +127,19 @@ func ToSecret(ns, name string, secretData *Config) (*corev1.Secret, error) {
 		s.Data[dataKey] = encodedPlugin
 	}
 
+	// Write secret data entries to the encryption-config Secret.
+	// Each secret dataKey (e.g. "app-role-role-id") is combined with the keyID
+	// (e.g. "1") to produce the data key "kms-plugin-secret-app-role-role-id-1".
+	for keyID, keySecretData := range secretData.KMSSecretData {
+		for secretDataKey, value := range keySecretData {
+			dataKey, err := toSecretDataKeyFor(secretDataKey, keyID)
+			if err != nil {
+				return nil, err
+			}
+			s.Data[dataKey] = value
+		}
+	}
+
 	return s, nil
 }
 
@@ -145,3 +185,26 @@ func ExtractUniqueAndSortedKMSConfigurations(secretData *Config) ([]*apiserverco
 	})
 	return result, nil
 }
+
+func toSecretDataKeyFor(secretDataKey, keyID string) (string, error) {
+	if _, err := strconv.ParseUint(keyID, 10, 64); err != nil {
+		return "", fmt.Errorf("invalid keyID %q: must be a non-negative integer", keyID)
+	}
+	return encryptionConfigSecretDataPrefix + secretDataKey + "-" + keyID, nil
+}
+
+func keyIDFromSecretDataKey(dataKey string) (string, string, bool, error) {
+	rest, found := strings.CutPrefix(dataKey, encryptionConfigSecretDataPrefix)
+	if !found {
+		return "", "", false, nil
+	}
+	i := strings.LastIndex(rest, "-")
+	if i < 1 {
+		return "", "", false, nil
+	}
+	keyID := rest[i+1:]
+	if _, err := strconv.ParseUint(keyID, 10, 64); err != nil {
+		return "", "", false, fmt.Errorf("invalid keyID %q: must be a non-negative integer", keyID)
+	}
+	return keyID, rest[:i], true, nil
+}

pkg/operator/encryption/secrets/secrets.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"strconv"
+	"strings"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
@@ -87,6 +88,14 @@ func ToKeyState(s *corev1.Secret) (state.KeyState, error) {
 			// encryption mode.
 			return state.KeyState{}, fmt.Errorf("%s can not be empty, when mode is KMS", EncryptionSecretKMSPluginConfig)
 		}
+		for dataKey, value := range s.Data {
+			if rawKey, found := strings.CutPrefix(dataKey, EncryptionSecretKMSSecretDataPrefix); found && len(rawKey) > 0 {
+				if key.KMS.SecretData == nil {
+					key.KMS.SecretData = map[string][]byte{}
+				}
+				key.KMS.SecretData[rawKey] = value
+			}
+		}
 		key.Mode = keyMode
 	default:
 		return state.KeyState{}, fmt.Errorf("secret %s/%s has invalid mode: %s", s.Namespace, s.Name, keyMode)
@@ -159,6 +168,12 @@ func FromKeyState(component string, ks state.KeyState) (*corev1.Secret, error) {
 		s.Data[EncryptionSecretKMSPluginConfig] = pluginData
 	}
 
+	if ks.HasKMSSecretData() {
+		for dataKey, value := range ks.KMS.SecretData {
+			s.Data[EncryptionSecretKMSSecretDataPrefix+dataKey] = value
+		}
+	}
+
 	return s, nil
 }