diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index 3c930469d5..b0e1403993 100644 --- a/pkg/operator/encryption/controllers/key_controller.go +++ b/pkg/operator/encryption/controllers/key_controller.go @@ -42,6 +42,7 @@ const ( encryptionSecretMigrationInterval = time.Hour * 24 * 7 // one week kmsEndpointFormat = "unix:///var/run/kmsplugin/kms-%d.sock" defaultKMSTimeout = 10 * time.Second + openshiftConfigNS = "openshift-config" ) // keyController creates new keys if necessary. It @@ -117,6 +118,7 @@ func NewKeyController( 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,24 @@ func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode, }, Plugin: apiServerEncryption.KMS, } + + if secretName, expectedKeys, err := referencedSecretName(apiServerEncryption.KMS); err != nil { + return nil, err + } else if len(secretName) > 0 { + refSecret, err := c.secretClient.Secrets(openshiftConfigNS).Get(ctx, secretName, metav1.GetOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to get secret %s in %s: %w", secretName, openshiftConfigNS, err) + } + for _, key := range expectedKeys { + v, ok := refSecret.Data[key] + if !ok { + return nil, fmt.Errorf("secret %s in %s is missing required key %q", secretName, openshiftConfigNS, key) + } + if err := ks.KMS.PluginSecretData.Set(secretName, key, v); err != nil { + return nil, err + } + } + } } return secrets.FromKeyState(c.instanceName, ks) } @@ -383,6 +403,25 @@ func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, extern return latestKeyID, "rotation-interval-has-passed", time.Since(latestKey.Migrated.Timestamp) > encryptionSecretMigrationInterval } +// referencedSecretName returns the name of the secret referenced by the KMS plugin +// config and the specific data keys to carry from that secret. Only the listed keys +// are copied into the Key Secret; any other data in the referenced secret is ignored. +func referencedSecretName(plugin configv1.KMSPluginConfig) (string, []string, error) { + switch plugin.Type { + case configv1.VaultKMSProvider: + switch plugin.Vault.Authentication.Type { + case configv1.VaultAuthenticationTypeAppRole: + // The Vault AppRole secret must contain "role-id" and "secret-id" keys. + // These are the only keys carried into the encryption key secret. + return plugin.Vault.Authentication.AppRole.Secret.Name, []string{"role-id", "secret-id"}, nil + default: + return "", nil, fmt.Errorf("unsupported Vault authentication type %q", plugin.Vault.Authentication.Type) + } + default: + return "", nil, fmt.Errorf("unsupported KMS provider type %q", plugin.Type) + } +} + // 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 { diff --git a/pkg/operator/encryption/controllers/key_controller_test.go b/pkg/operator/encryption/controllers/key_controller_test.go index 8d636325ee..9fdb0cbd0e 100644 --- a/pkg/operator/encryption/controllers/key_controller_test.go +++ b/pkg/operator/encryption/controllers/key_controller_test.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "errors" "fmt" + "strings" "testing" "time" @@ -336,9 +337,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 +383,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 +428,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 +523,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 { @@ -573,6 +585,65 @@ func TestKeyController(t *testing.T) { }, }, + { + name: "degraded when KMS referenced secret does not exist", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + targetNamespace: "kms", + expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config"}, + initialObjects: []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + }, + apiServerObjects: []runtime.Object{apiServerWithKMS}, + validateOperatorClientFunc: func(ts *testing.T, operatorClient v1helpers.OperatorClient) { + _, status, _, err := operatorClient.GetOperatorState() + if err != nil { + ts.Fatal(err) + } + for _, c := range status.Conditions { + if c.Type == "EncryptionKeyControllerDegraded" && c.Status == "True" { + if !strings.Contains(c.Message, "failed to get secret vault-approle-secret in openshift-config") { + ts.Errorf("unexpected degraded message: %s", c.Message) + } + return + } + } + ts.Fatal("expected EncryptionKeyControllerDegraded condition") + }, + expectedError: fmt.Errorf(`failed to create key: failed to get secret vault-approle-secret in openshift-config: secrets "vault-approle-secret" not found`), + }, + + { + name: "degraded when KMS referenced secret is missing a required key", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + targetNamespace: "kms", + expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "get:secrets:openshift-config"}, + initialObjects: []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "vault-approle-secret", Namespace: "openshift-config"}, + Data: map[string][]byte{ + "role-id": []byte("test-role-id"), + }, + Type: corev1.SecretTypeOpaque, + }, + }, + apiServerObjects: []runtime.Object{apiServerWithKMS}, + validateOperatorClientFunc: func(ts *testing.T, operatorClient v1helpers.OperatorClient) { + expectedCondition := operatorv1.OperatorCondition{ + Type: "EncryptionKeyControllerDegraded", + Status: "True", + Reason: "Error", + Message: `failed to create key: secret vault-approle-secret in openshift-config is missing required key "secret-id"`, + } + encryptiontesting.ValidateOperatorClientConditions(ts, operatorClient, []operatorv1.OperatorCondition{expectedCondition}) + }, + expectedError: errors.New(`failed to create key: secret vault-approle-secret in openshift-config is missing required key "secret-id"`), + }, + { name: "creates a new AESCBC key when switching from KMS to AESCBC", targetGRs: []schema.GroupResource{ @@ -667,7 +738,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...) @@ -708,6 +779,89 @@ func TestKeyController(t *testing.T) { } } +func TestReferencedSecretName(t *testing.T) { + scenarios := []struct { + name string + plugin configv1.KMSPluginConfig + expectedName string + expectedDataKeys []string + expectedError bool + }{ + { + name: "Vault with AppRole authentication returns secret name and keys", + plugin: configv1.KMSPluginConfig{ + Type: configv1.VaultKMSProvider, + Vault: configv1.VaultKMSPluginConfig{ + Authentication: configv1.VaultAuthentication{ + Type: configv1.VaultAuthenticationTypeAppRole, + AppRole: configv1.VaultAppRoleAuthentication{ + Secret: configv1.VaultSecretReference{Name: "my-approle-secret"}, + }, + }, + }, + }, + expectedName: "my-approle-secret", + expectedDataKeys: []string{"role-id", "secret-id"}, + }, + { + name: "Vault with unknown authentication type returns error", + plugin: configv1.KMSPluginConfig{ + Type: configv1.VaultKMSProvider, + Vault: configv1.VaultKMSPluginConfig{ + Authentication: configv1.VaultAuthentication{ + Type: "UnknownAuth", + }, + }, + }, + expectedError: true, + }, + { + name: "Vault with empty authentication type returns error", + plugin: configv1.KMSPluginConfig{ + Type: configv1.VaultKMSProvider, + Vault: configv1.VaultKMSPluginConfig{}, + }, + expectedError: true, + }, + { + name: "unknown KMS provider returns error", + plugin: configv1.KMSPluginConfig{Type: "UnknownProvider"}, + expectedError: true, + }, + { + name: "empty plugin config returns error", + plugin: configv1.KMSPluginConfig{}, + expectedError: true, + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + name, dataKeys, err := referencedSecretName(scenario.plugin) + if scenario.expectedError { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if name != scenario.expectedName { + t.Errorf("expected secret name %q, got %q", scenario.expectedName, name) + } + if len(dataKeys) != len(scenario.expectedDataKeys) { + t.Fatalf("expected %d data keys, got %d", len(scenario.expectedDataKeys), len(dataKeys)) + } + for i, key := range dataKeys { + if key != scenario.expectedDataKeys[i] { + t.Errorf("expected data key[%d] %q, got %q", i, scenario.expectedDataKeys[i], key) + } + } + }) + } +} + var flatEncryptionJSON = ` { "encryption": { diff --git a/pkg/operator/encryption/encryptiondata/config.go b/pkg/operator/encryption/encryptiondata/config.go index 40a855a81f..c4b7706af7 100644 --- a/pkg/operator/encryption/encryptiondata/config.go +++ b/pkg/operator/encryption/encryptiondata/config.go @@ -30,6 +30,48 @@ type Config struct { // KMSPlugins maps keyID to plugin-specific configuration, // carried from Key Secrets into the encryption-config Secret. KMSPlugins map[string]configv1.KMSPluginConfig + // KMSPluginsSecretData maps keyID to secret data carried from + // Key Secrets into the encryption-config Secret. + // Structure: keyID → secretName → dataKey → value. + KMSPluginsSecretData KMSPluginsSecretData +} + +// KMSPluginsSecretData maps keyID to secret data carried from Key Secrets into +// the encryption-config Secret. Structure: keyID → secretName → dataKey → value. +type KMSPluginsSecretData struct { + ByKeyID map[string]state.KMSSecretData +} + +// SetFromRawKey stores a value for the given keyID, splitting rawKey +// on "_" into secretName and dataKey. +func (d *KMSPluginsSecretData) SetFromRawKey(keyID, rawKey string, value []byte) error { + if len(keyID) == 0 { + return fmt.Errorf("keyID must not be empty") + } + if d.ByKeyID == nil { + d.ByKeyID = map[string]state.KMSSecretData{} + } + sd := d.ByKeyID[keyID] + if err := sd.SetFromRawKey(rawKey, value); err != nil { + return err + } + d.ByKeyID[keyID] = sd + return nil +} + +// FlatEntriesByKeyID returns the stored data as a map of keyID to flat entries, +// where each flat entry is keyed by "secretName_dataKey". +func (d *KMSPluginsSecretData) FlatEntriesByKeyID() map[string]map[string][]byte { + if d.ByKeyID == nil { + return nil + } + result := map[string]map[string][]byte{} + for keyID, sd := range d.ByKeyID { + if flat := sd.FlatEntries(); flat != nil { + result[keyID] = flat + } + } + return result } func (c *Config) HasEncryptionConfiguration() bool { @@ -40,6 +82,7 @@ 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 kmsPluginsSecretData KMSPluginsSecretData for gr, grKeys := range encryptionState { resourceConfigs = append(resourceConfigs, apiserverconfigv1.ResourceConfiguration{ @@ -47,11 +90,11 @@ func FromEncryptionState(encryptionState map[schema.GroupResource]state.GroupRes 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 +110,19 @@ func FromEncryptionState(encryptionState map[schema.GroupResource]state.GroupRes kmsPlugins[key.Key.Name] = key.KMS.Plugin } } + if key.HasKMSSecretData() { + if existing, exists := kmsPluginsSecretData.ByKeyID[key.Key.Name]; exists { + if !equality.Semantic.DeepEqual(existing, key.KMS.PluginSecretData) { + return nil, fmt.Errorf("KMS secret data mismatch for keyID %s: secret data from different resources must be identical", key.Key.Name) + } + } else { + for rawKey, value := range key.KMS.PluginSecretData.FlatEntries() { + if err := kmsPluginsSecretData.SetFromRawKey(key.Key.Name, rawKey, value); err != nil { + return nil, fmt.Errorf("failed to copy secret data for keyID %s: %w", key.Key.Name, err) + } + } + } + } } } @@ -76,8 +132,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, + KMSPluginsSecretData: kmsPluginsSecretData, }, nil } diff --git a/pkg/operator/encryption/encryptiondata/config_test.go b/pkg/operator/encryption/encryptiondata/config_test.go index 181fbb0730..4e6f1d3a81 100644 --- a/pkg/operator/encryption/encryptiondata/config_test.go +++ b/pkg/operator/encryption/encryptiondata/config_test.go @@ -798,6 +798,137 @@ func TestFromEncryptionStateKMSPluginConfigValidation(t *testing.T) { } } +func TestFromEncryptionStateKMSSecretDataValidation(t *testing.T) { + tests := []struct { + name string + encryptionState map[schema.GroupResource]state.GroupResourceState + expectedErr string + expectedData encryptiondata.KMSPluginsSecretData + }{ + { + name: "matching secret data across resources", + encryptionState: map[schema.GroupResource]state.GroupResourceState{ + {Resource: "secrets"}: { + ReadKeys: []state.KeyState{{ + Key: apiserverconfigv1.Key{Name: "1", Secret: "AAAAAAAAAAAAAAAAAAAAAA=="}, + Mode: state.KMS, + KMS: &state.KMSState{ + Encryption: &apiserverconfigv1.KMSConfiguration{APIVersion: "v2", Name: "1", Endpoint: "unix:///var/run/kmsplugin/kms-1.sock"}, + Plugin: encryptiontesting.DefaultKMSPluginConfig, + PluginSecretData: state.KMSSecretData{ + Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("test-role-id"), + "secret-id": []byte("test-secret-id"), + }, + }}, + }, + }}, + }, + {Resource: "configmaps"}: { + ReadKeys: []state.KeyState{{ + Key: apiserverconfigv1.Key{Name: "1", Secret: "AAAAAAAAAAAAAAAAAAAAAA=="}, + Mode: state.KMS, + KMS: &state.KMSState{ + Encryption: &apiserverconfigv1.KMSConfiguration{APIVersion: "v2", Name: "1", Endpoint: "unix:///var/run/kmsplugin/kms-1.sock"}, + Plugin: encryptiontesting.DefaultKMSPluginConfig, + PluginSecretData: state.KMSSecretData{ + Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("test-role-id"), + "secret-id": []byte("test-secret-id"), + }, + }}, + }, + }}, + }, + }, + expectedData: encryptiondata.KMSPluginsSecretData{ByKeyID: map[string]state.KMSSecretData{ + "1": {Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("test-role-id"), + "secret-id": []byte("test-secret-id"), + }, + }}, + }}, + }, + { + name: "mismatched secret data across resources", + encryptionState: map[schema.GroupResource]state.GroupResourceState{ + {Resource: "secrets"}: { + ReadKeys: []state.KeyState{{ + Key: apiserverconfigv1.Key{Name: "1", Secret: "AAAAAAAAAAAAAAAAAAAAAA=="}, + Mode: state.KMS, + KMS: &state.KMSState{ + Encryption: &apiserverconfigv1.KMSConfiguration{APIVersion: "v2", Name: "1", Endpoint: "unix:///var/run/kmsplugin/kms-1.sock"}, + Plugin: encryptiontesting.DefaultKMSPluginConfig, + PluginSecretData: state.KMSSecretData{ + Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("role-id-a"), + }, + }}, + }, + }}, + }, + {Resource: "configmaps"}: { + ReadKeys: []state.KeyState{{ + Key: apiserverconfigv1.Key{Name: "1", Secret: "AAAAAAAAAAAAAAAAAAAAAA=="}, + Mode: state.KMS, + KMS: &state.KMSState{ + Encryption: &apiserverconfigv1.KMSConfiguration{APIVersion: "v2", Name: "1", Endpoint: "unix:///var/run/kmsplugin/kms-1.sock"}, + Plugin: encryptiontesting.DefaultKMSPluginConfig, + PluginSecretData: state.KMSSecretData{ + Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("role-id-b"), + }, + }}, + }, + }}, + }, + }, + expectedErr: `KMS secret data mismatch for keyID 1: secret data from different resources must be identical`, + }, + { + name: "no secret data is valid", + encryptionState: map[schema.GroupResource]state.GroupResourceState{ + {Resource: "secrets"}: { + ReadKeys: []state.KeyState{{ + Key: apiserverconfigv1.Key{Name: "1", Secret: "AAAAAAAAAAAAAAAAAAAAAA=="}, + Mode: state.KMS, + KMS: &state.KMSState{ + Encryption: &apiserverconfigv1.KMSConfiguration{APIVersion: "v2", Name: "1", Endpoint: "unix:///var/run/kmsplugin/kms-1.sock"}, + Plugin: encryptiontesting.DefaultKMSPluginConfig, + }, + }}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg, err := encryptiondata.FromEncryptionState(tt.encryptionState) + if tt.expectedErr != "" { + if err == nil { + t.Fatal("expected error, got nil") + } + if err.Error() != tt.expectedErr { + t.Fatalf("unexpected error:\n got: %v\n expected: %v", err, tt.expectedErr) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !cmp.Equal(tt.expectedData, cfg.KMSPluginsSecretData) { + t.Fatalf("unexpected secret data: %s", cmp.Diff(tt.expectedData, cfg.KMSPluginsSecretData)) + } + }) + } +} + func TestSecretRoundtrip(t *testing.T) { tests := []struct { name string @@ -851,6 +982,90 @@ 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, + }, + KMSPluginsSecretData: encryptiondata.KMSPluginsSecretData{ByKeyID: map[string]state.KMSSecretData{ + "1": {Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("test-role-id"), + "secret-id": []byte("test-secret-id"), + }, + }}, + }}, + }, + }, + { + name: "KMS with multiple provider configs 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: "2_secrets", + Endpoint: "unix:///var/run/kmsplugin/kms-2.sock", + Timeout: &metav1.Duration{Duration: 10 * time.Second}, + }, + }, { + 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, + "2": encryptiontesting.DefaultKMSPluginConfig, + }, + KMSPluginsSecretData: encryptiondata.KMSPluginsSecretData{ByKeyID: map[string]state.KMSSecretData{ + "1": {Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("role-id-1"), + "secret-id": []byte("secret-id-1"), + }, + }}, + "2": {Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("role-id-2"), + "secret-id": []byte("secret-id-2"), + }, + }}, + }}, + }, + }, { name: "KMS with multiple provider configs", cfg: &encryptiondata.Config{ @@ -904,3 +1119,192 @@ func TestSecretRoundtrip(t *testing.T) { }) } } + +func TestToSecretSecretDataEdgeCases(t *testing.T) { + baseCfg := &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, + }, + } + + tests := []struct { + name string + secretData encryptiondata.KMSPluginsSecretData + wantErr bool + expectedDataKeys map[string][]byte + }{ + { + name: "empty secret data produces no extra keys", + secretData: encryptiondata.KMSPluginsSecretData{}, + }, + { + name: "valid secret data produces correct data keys", + secretData: encryptiondata.KMSPluginsSecretData{ByKeyID: map[string]state.KMSSecretData{ + "1": {Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("test-role-id"), + "secret-id": []byte("test-secret-id"), + }, + }}, + }}, + expectedDataKeys: map[string][]byte{ + "kms-plugin-secret-vault-approle-secret_role-id-1": []byte("test-role-id"), + "kms-plugin-secret-vault-approle-secret_secret-id-1": []byte("test-secret-id"), + }, + }, + { + name: "multiple secrets within same keyID", + secretData: encryptiondata.KMSPluginsSecretData{ByKeyID: map[string]state.KMSSecretData{ + "1": {Entries: map[string]map[string][]byte{ + "secret-a": { + "key-1": []byte("val-a1"), + }, + "secret-b": { + "key-2": []byte("val-b2"), + }, + }}, + }}, + expectedDataKeys: map[string][]byte{ + "kms-plugin-secret-secret-a_key-1-1": []byte("val-a1"), + "kms-plugin-secret-secret-b_key-2-1": []byte("val-b2"), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &encryptiondata.Config{ + Encryption: baseCfg.Encryption, + KMSPlugins: baseCfg.KMSPlugins, + KMSPluginsSecretData: tt.secretData, + } + + secret, err := encryptiondata.ToSecret("openshift-config-managed", "encryption-config-test", cfg) + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("ToSecret() error: %v", err) + } + + for expectedKey, expectedValue := range tt.expectedDataKeys { + actual, ok := secret.Data[expectedKey] + if !ok { + t.Errorf("expected data key %q not found in secret", expectedKey) + continue + } + if string(actual) != string(expectedValue) { + t.Errorf("data key %q: expected %q, got %q", expectedKey, expectedValue, actual) + } + } + }) + } +} + +func TestFromSecretSecretData(t *testing.T) { + baseCfg := &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, + }, + } + + baseSecret, err := encryptiondata.ToSecret("openshift-config-managed", "encryption-config-test", baseCfg) + if err != nil { + t.Fatalf("failed to create base secret: %v", err) + } + + tests := []struct { + name string + extraData map[string][]byte + expectedData encryptiondata.KMSPluginsSecretData + }{ + { + name: "secret data keys are parsed correctly", + extraData: map[string][]byte{ + "kms-plugin-secret-vault-approle-secret_role-id-1": []byte("test-role-id"), + "kms-plugin-secret-vault-approle-secret_secret-id-1": []byte("test-secret-id"), + }, + expectedData: encryptiondata.KMSPluginsSecretData{ByKeyID: map[string]state.KMSSecretData{ + "1": {Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("test-role-id"), + "secret-id": []byte("test-secret-id"), + }, + }}, + }}, + }, + { + name: "no secret data keys", + extraData: map[string][]byte{}, + }, + { + name: "non-secret-data keys are ignored", + extraData: map[string][]byte{ + "some-other-key": []byte("ignored"), + }, + }, + { + name: "multiple keyIDs are separated correctly", + extraData: map[string][]byte{ + "kms-plugin-secret-vault-approle-secret_role-id-1": []byte("role-id-1"), + "kms-plugin-secret-vault-approle-secret_secret-id-1": []byte("secret-id-1"), + "kms-plugin-secret-vault-approle-secret_role-id-2": []byte("role-id-2"), + "kms-plugin-secret-vault-approle-secret_secret-id-2": []byte("secret-id-2"), + }, + expectedData: encryptiondata.KMSPluginsSecretData{ByKeyID: map[string]state.KMSSecretData{ + "1": {Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("role-id-1"), + "secret-id": []byte("secret-id-1"), + }, + }}, + "2": {Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("role-id-2"), + "secret-id": []byte("secret-id-2"), + }, + }}, + }}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + secret := baseSecret.DeepCopy() + for k, v := range tt.extraData { + secret.Data[k] = v + } + + cfg, err := encryptiondata.FromSecret(secret) + if err != nil { + t.Fatalf("FromSecret() error: %v", err) + } + if !cmp.Equal(tt.expectedData, cfg.KMSPluginsSecretData) { + t.Fatalf("unexpected secret data: %s", cmp.Diff(tt.expectedData, cfg.KMSPluginsSecretData)) + } + }) + } +} diff --git a/pkg/operator/encryption/encryptiondata/secret.go b/pkg/operator/encryption/encryptiondata/secret.go index 7e7b030dc1..b2d025e19f 100644 --- a/pkg/operator/encryption/encryptiondata/secret.go +++ b/pkg/operator/encryption/encryptiondata/secret.go @@ -41,11 +41,15 @@ func keyIDFromPluginConfigSecretDataKey(dataKey string) (string, bool, error) { return keyID, true, nil } -// 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-{secretName}_{dataKey}-{keyID}". + encryptionConfigSecretDataPrefix = "kms-plugin-secret-" +) func FromSecret(encryptionConfigSecret *corev1.Secret) (*Config, error) { data, ok := encryptionConfigSecret.Data[EncryptionConfSecretKey] @@ -80,7 +84,26 @@ 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-{secretName}_{dataKey}-{keyID}" + // (e.g. "kms-plugin-secret-app-role_role-id-1"). keyIDFromSecretDataKey + // returns the keyID (e.g. "1") and the combined key (e.g. "app-role_role-id"), + // which is then split on "_" to recover secretName and dataKey. + var kmsPluginsSecretData KMSPluginsSecretData + for key, value := range encryptionConfigSecret.Data { + keyID, rawKey, found, err := parseSecretDataKey(key) + if err != nil { + return nil, fmt.Errorf("failed to parse secret data key %s: %w", key, err) + } + if !found { + continue + } + if err := kmsPluginsSecretData.SetFromRawKey(keyID, rawKey, value); err != nil { + return nil, fmt.Errorf("failed to set key %s: %w", key, err) + } + } + + return &Config{Encryption: encryptionConfig, KMSPlugins: kmsPlugins, KMSPluginsSecretData: kmsPluginsSecretData}, nil } func ToSecret(ns, name string, secretData *Config) (*corev1.Secret, error) { @@ -124,6 +147,15 @@ func ToSecret(ns, name string, secretData *Config) (*corev1.Secret, error) { s.Data[dataKey] = encodedPlugin } + // Write secret data entries to the encryption-config Secret. + // Each entry from FlatEntries() (e.g. "app-role_role-id") is combined with the keyID + // (e.g. "1") to produce "kms-plugin-secret-app-role_role-id-1". + for keyID, flatEntries := range secretData.KMSPluginsSecretData.FlatEntriesByKeyID() { + for flatKey, value := range flatEntries { + s.Data[encryptionConfigSecretDataPrefix+flatKey+"-"+keyID] = value + } + } + return s, nil } @@ -169,3 +201,19 @@ func ExtractUniqueAndSortedKMSConfigurations(secretData *Config) ([]*apiserverco }) return result, nil } + +func parseSecretDataKey(dataKey string) (keyID, rawKey string, found bool, err 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 +} diff --git a/pkg/operator/encryption/encryptiondata/secret_test.go b/pkg/operator/encryption/encryptiondata/secret_test.go index efd2a84f21..26a4be9279 100644 --- a/pkg/operator/encryption/encryptiondata/secret_test.go +++ b/pkg/operator/encryption/encryptiondata/secret_test.go @@ -354,3 +354,94 @@ func TestToPluginConfigSecretDataKeyFor(t *testing.T) { }) } } + +func TestParseSecretDataKey(t *testing.T) { + tests := []struct { + name string + dataKey string + wantKeyID string + wantRawKey string + wantFound bool + wantErr bool + }{ + { + name: "valid key", + dataKey: "kms-plugin-secret-vault-approle-secret_role-id-1", + wantKeyID: "1", + wantRawKey: "vault-approle-secret_role-id", + wantFound: true, + }, + { + name: "valid key with large keyID", + dataKey: "kms-plugin-secret-vault-approle-secret_secret-id-42", + wantKeyID: "42", + wantRawKey: "vault-approle-secret_secret-id", + wantFound: true, + }, + { + name: "encryption-config key ignored", + dataKey: "encryption-config", + }, + { + name: "plugin config key ignored", + dataKey: "kms-plugin-config-1", + }, + { + name: "empty string", + dataKey: "", + }, + { + name: "prefix only", + dataKey: "kms-plugin-secret-", + }, + { + name: "no dash after prefix", + dataKey: "kms-plugin-secret-nodash", + }, + { + name: "numeric segment in rawKey is not confused with keyID", + dataKey: "kms-plugin-secret-secret-a_port-8080-1", + wantKeyID: "1", + wantRawKey: "secret-a_port-8080", + wantFound: true, + }, + { + name: "flatKey ending with dash produces double dash before keyID", + dataKey: "kms-plugin-secret-secret-a_port-8080--1", + wantKeyID: "1", + wantRawKey: "secret-a_port-8080-", + wantFound: true, + }, + { + name: "non-numeric keyID", + dataKey: "kms-plugin-secret-vault-approle-secret_role-id-abc", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + keyID, rawKey, found, err := parseSecretDataKey(tt.dataKey) + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if found != tt.wantFound { + t.Fatalf("found: got %v, want %v", found, tt.wantFound) + } + if found { + if keyID != tt.wantKeyID { + t.Errorf("keyID: got %q, want %q", keyID, tt.wantKeyID) + } + if rawKey != tt.wantRawKey { + t.Errorf("rawKey: got %q, want %q", rawKey, tt.wantRawKey) + } + } + }) + } +} diff --git a/pkg/operator/encryption/secrets/secrets.go b/pkg/operator/encryption/secrets/secrets.go index e2e59efd6d..341cabbd80 100644 --- a/pkg/operator/encryption/secrets/secrets.go +++ b/pkg/operator/encryption/secrets/secrets.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "strconv" + "strings" "time" corev1 "k8s.io/api/core/v1" @@ -76,16 +77,25 @@ func ToKeyState(s *corev1.Secret) (state.KeyState, error) { // encryption mode. return state.KeyState{}, fmt.Errorf("%s can not be empty, when mode is KMS", EncryptionSecretKMSEncryptionConfig) } - if v, ok := s.Data[EncryptionSecretKMSPluginConfig]; ok && len(v) > 0 { + if v, ok := s.Data[encryptionSecretKMSPluginConfig]; ok && len(v) > 0 { kmsConfig, err := encoding.DecodeKMSPluginConfig(v) if err != nil { - return state.KeyState{}, fmt.Errorf("secret %s/%s has invalid %s data: %w", s.Namespace, s.Name, EncryptionSecretKMSPluginConfig, err) + return state.KeyState{}, fmt.Errorf("secret %s/%s has invalid %s data: %w", s.Namespace, s.Name, encryptionSecretKMSPluginConfig, err) } key.KMS.Plugin = kmsConfig } else { // encryption.apiserver.operator.openshift.io-kms-plugin-config data field is required for KMS // encryption mode. - return state.KeyState{}, fmt.Errorf("%s can not be empty, when mode is KMS", EncryptionSecretKMSPluginConfig) + return state.KeyState{}, fmt.Errorf("%s can not be empty, when mode is KMS", encryptionSecretKMSPluginConfig) + } + for dataKey, value := range s.Data { + rawKey, found := strings.CutPrefix(dataKey, encryptionSecretKMSSecretDataPrefix) + if !found { + continue + } + if err := key.KMS.PluginSecretData.SetFromRawKey(rawKey, value); err != nil { + return state.KeyState{}, fmt.Errorf("secret %s/%s has malformed secret data key %q: %w", s.Namespace, s.Name, dataKey, err) + } } key.Mode = keyMode default: @@ -106,7 +116,7 @@ func FromKeyState(component string, ks state.KeyState) (*corev1.Secret, error) { } if ks.Mode == state.KMS && (!ks.HasKMSEncryption() || !ks.HasKMSPlugin()) { - return nil, fmt.Errorf("%s or %s can not be empty, when mode is KMS", EncryptionSecretKMSEncryptionConfig, EncryptionSecretKMSPluginConfig) + return nil, fmt.Errorf("%s or %s can not be empty, when mode is KMS", EncryptionSecretKMSEncryptionConfig, encryptionSecretKMSPluginConfig) } s := &corev1.Secret{ @@ -156,7 +166,13 @@ func FromKeyState(component string, ks state.KeyState) (*corev1.Secret, error) { if err != nil { return nil, err } - s.Data[EncryptionSecretKMSPluginConfig] = pluginData + s.Data[encryptionSecretKMSPluginConfig] = pluginData + } + + if ks.HasKMSSecretData() { + for flatKey, value := range ks.KMS.PluginSecretData.FlatEntries() { + s.Data[encryptionSecretKMSSecretDataPrefix+flatKey] = value + } } return s, nil diff --git a/pkg/operator/encryption/secrets/secrets_test.go b/pkg/operator/encryption/secrets/secrets_test.go index 1d73ea9cf1..e9522a69e1 100644 --- a/pkg/operator/encryption/secrets/secrets_test.go +++ b/pkg/operator/encryption/secrets/secrets_test.go @@ -182,6 +182,41 @@ func TestRoundtrip(t *testing.T) { }, }, }, + { + name: "full kms with secret data", + component: "kms", + ks: state.KeyState{ + Key: v1.Key{ + Name: "1", + Secret: base64.StdEncoding.EncodeToString(emptyKey), + }, + Backed: true, + Mode: "KMS", + KMS: &state.KMSState{ + Encryption: &v1.KMSConfiguration{ + APIVersion: "v2", + Name: "1", + Endpoint: "unix:///var/run/kmsplugin/kms-1.sock", + Timeout: &metav1.Duration{Duration: 10 * time.Second}, + }, + Plugin: defaultKMSPluginConfig, + PluginSecretData: state.KMSSecretData{Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("test-role-id"), + "secret-id": []byte("test-secret-id"), + }, + }}, + }, + Migrated: state.MigrationState{ + Timestamp: now, + Resources: []schema.GroupResource{ + {Resource: "secrets"}, + }, + }, + InternalReason: "internal", + ExternalReason: "external", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/operator/encryption/secrets/types.go b/pkg/operator/encryption/secrets/types.go index 2795afbc0d..bf152dee31 100644 --- a/pkg/operator/encryption/secrets/types.go +++ b/pkg/operator/encryption/secrets/types.go @@ -54,9 +54,14 @@ const ( // encryption configuration for KMS mode in the encryption-key secret. EncryptionSecretKMSEncryptionConfig = "encryption.apiserver.operator.openshift.io-kms-encryption-config" - // EncryptionSecretKMSPluginConfig is the data field key that stores the serialized KMS plugin + // encryptionSecretKMSPluginConfig is the data field key that stores the serialized KMS plugin // configuration for KMS mode in the encryption-key secret. - EncryptionSecretKMSPluginConfig = "encryption.apiserver.operator.openshift.io-kms-plugin-config" + encryptionSecretKMSPluginConfig = "encryption.apiserver.operator.openshift.io-kms-plugin-config" + + // encryptionSecretKMSSecretDataPrefix is the data field key prefix for secret data values + // fetched from the referenced secret in openshift-config. The full data key is + // constructed as prefix + secretName + separator + dataKey. + encryptionSecretKMSSecretDataPrefix = "encryption.apiserver.operator.openshift.io-kms-plugin-secret-" ) // MigratedGroupResources is the data structured stored in the diff --git a/pkg/operator/encryption/state/types.go b/pkg/operator/encryption/state/types.go index acaf493377..b7163c41e7 100644 --- a/pkg/operator/encryption/state/types.go +++ b/pkg/operator/encryption/state/types.go @@ -1,6 +1,8 @@ package state import ( + "fmt" + "strings" "time" configv1 "github.com/openshift/api/config/v1" @@ -15,6 +17,11 @@ const ( KubernetesDescriptionScaryValue = `WARNING: DO NOT EDIT. Altering of the encryption secrets will render you cluster inaccessible. Catastrophic data loss can occur from the most minor changes.` + + // secretDataKeySeparator separates the secret name from the data key. + // Underscore is used because it is forbidden in Kubernetes secret/configmap + // names, preventing collisions. + secretDataKeySeparator = "_" ) // GroupResourceState represents, for a single group resource, the write and read keys in a @@ -53,6 +60,10 @@ func (k *KeyState) HasKMSPlugin() bool { return k != nil && k.KMS != nil && k.KMS.Plugin != (configv1.KMSPluginConfig{}) } +func (k *KeyState) HasKMSSecretData() bool { + return k != nil && k.KMS != nil && len(k.KMS.PluginSecretData.Entries) > 0 +} + // KMSState stores all KMS encryption mode related configurations type KMSState struct { // Encoded EncryptionConfig that stores the KMS related fields @@ -60,6 +71,58 @@ type KMSState struct { // Plugin stores KMS plugin specific configurations Plugin configv1.KMSPluginConfig + + // PluginSecretData stores data key-value pairs fetched from referenced secrets. + PluginSecretData KMSSecretData +} + +// KMSSecretData stores data key-value pairs fetched from referenced secrets. +// Entries maps secret names to their data key-value pairs. +type KMSSecretData struct { + Entries map[string]map[string][]byte +} + +func (d *KMSSecretData) Set(secretName, dataKey string, value []byte) error { + if len(secretName) == 0 || len(dataKey) == 0 || len(value) == 0 { + return fmt.Errorf("secretName, dataKey, and value must not be empty") + } + if strings.Contains(secretName, "_") { + return fmt.Errorf("secret name %q must not contain underscores", secretName) + } + if d.Entries == nil { + d.Entries = map[string]map[string][]byte{} + } + if d.Entries[secretName] == nil { + d.Entries[secretName] = map[string][]byte{} + } + d.Entries[secretName][dataKey] = value + return nil +} + +// SetFromRawKey splits a combined key of the form "secretName_dataKey" +// and stores the value. +func (d *KMSSecretData) SetFromRawKey(rawKey string, value []byte) error { + parts := strings.SplitN(rawKey, secretDataKeySeparator, 2) + if len(parts) != 2 { + return fmt.Errorf("invalid combined key %q: expected format {secretName}%s{dataKey}", rawKey, secretDataKeySeparator) + } + return d.Set(parts[0], parts[1], value) +} + +// FlatEntries returns the stored data as a flat map keyed by "secretName_dataKey". +// "_" separates secretName from dataKey because "_" is forbidden in +// Kubernetes secret names, making the split unambiguous. +func (d *KMSSecretData) FlatEntries() map[string][]byte { + if d.Entries == nil { + return nil + } + result := map[string][]byte{} + for secretName, keys := range d.Entries { + for dataKey, value := range keys { + result[secretName+secretDataKeySeparator+dataKey] = value + } + } + return result } type MigrationState struct { diff --git a/pkg/operator/encryption/state/types_test.go b/pkg/operator/encryption/state/types_test.go new file mode 100644 index 0000000000..23d5aefda5 --- /dev/null +++ b/pkg/operator/encryption/state/types_test.go @@ -0,0 +1,181 @@ +package state + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestKMSSecretDataSet(t *testing.T) { + tests := []struct { + name string + secretName string + dataKey string + value []byte + wantErr bool + expected KMSSecretData + }{ + { + name: "valid set", + secretName: "vault-approle-secret", + dataKey: "role-id", + value: []byte("test-role-id"), + expected: KMSSecretData{Entries: map[string]map[string][]byte{ + "vault-approle-secret": {"role-id": []byte("test-role-id")}, + }}, + }, + { + name: "secret name with underscore returns error", + secretName: "vault_approle_secret", + dataKey: "role-id", + value: []byte("test-role-id"), + wantErr: true, + }, + { + name: "secret name with trailing underscore returns error", + secretName: "vault-approle-secret_", + dataKey: "role-id", + value: []byte("test-role-id"), + wantErr: true, + }, + { + name: "secret name with leading underscore returns error", + secretName: "_vault-approle-secret", + dataKey: "role-id", + value: []byte("test-role-id"), + wantErr: true, + }, + { + name: "empty secret name returns error", + dataKey: "role-id", + value: []byte("test-role-id"), + wantErr: true, + }, + { + name: "empty data key returns error", + secretName: "vault-approle-secret", + value: []byte("test-role-id"), + wantErr: true, + }, + { + name: "empty value returns error", + secretName: "vault-approle-secret", + dataKey: "role-id", + value: []byte{}, + wantErr: true, + }, + { + name: "nil value returns error", + secretName: "vault-approle-secret", + dataKey: "role-id", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var d KMSSecretData + err := d.Set(tt.secretName, tt.dataKey, tt.value) + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if diff := cmp.Diff(tt.expected, d); diff != "" { + t.Errorf("unexpected result (-want +got):\n%s", diff) + } + }) + } +} + +func TestKMSSecretDataSetFromRawKey(t *testing.T) { + tests := []struct { + name string + rawKey string + value []byte + wantErr bool + expected KMSSecretData + }{ + { + name: "valid raw key", + rawKey: "vault-approle-secret_role-id", + value: []byte("test-role-id"), + expected: KMSSecretData{Entries: map[string]map[string][]byte{ + "vault-approle-secret": {"role-id": []byte("test-role-id")}, + }}, + }, + { + name: "missing separator returns error", + rawKey: "no-separator-here", + value: []byte("v"), + wantErr: true, + }, + { + name: "multiple underscores splits on first only", + rawKey: "vault_approle_secret_role-id", + value: []byte("v"), + expected: KMSSecretData{Entries: map[string]map[string][]byte{ + "vault": {"approle_secret_role-id": []byte("v")}, + }}, + }, + { + name: "empty raw key returns error", + rawKey: "", + value: []byte("v"), + wantErr: true, + }, + { + name: "empty value returns error", + rawKey: "vault-approle-secret_role-id", + value: []byte{}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var d KMSSecretData + err := d.SetFromRawKey(tt.rawKey, tt.value) + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if diff := cmp.Diff(tt.expected, d); diff != "" { + t.Errorf("unexpected result (-want +got):\n%s", diff) + } + }) + } +} + +func TestKMSSecretDataFlatEntries(t *testing.T) { + d := KMSSecretData{Entries: map[string]map[string][]byte{ + "vault-approle-secret": { + "role-id": []byte("test-role-id"), + "secret-id": []byte("test-secret-id"), + }, + }} + + expected := map[string][]byte{ + "vault-approle-secret_role-id": []byte("test-role-id"), + "vault-approle-secret_secret-id": []byte("test-secret-id"), + } + + if diff := cmp.Diff(expected, d.FlatEntries()); diff != "" { + t.Errorf("unexpected result (-want +got):\n%s", diff) + } + + // zero value returns nil + var empty KMSSecretData + if empty.FlatEntries() != nil { + t.Errorf("expected nil, got %v", empty.FlatEntries()) + } +} diff --git a/pkg/operator/encryption/testing/helpers.go b/pkg/operator/encryption/testing/helpers.go index df4865d937..4faa7342e7 100644 --- a/pkg/operator/encryption/testing/helpers.go +++ b/pkg/operator/encryption/testing/helpers.go @@ -149,6 +149,20 @@ func CreateEncryptionKeySecretWithCustomKMSPluginConfig(targetNS string, grs []s return secret } +func CreateVaultAppRoleSecret(name, roleID, secretID string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "openshift-config", + }, + Data: map[string][]byte{ + "role-id": []byte(roleID), + "secret-id": []byte(secretID), + }, + Type: corev1.SecretTypeOpaque, + } +} + func CreateDummyKubeAPIPod(name, namespace string, nodeName string) *corev1.Pod { return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ diff --git a/test/e2e-encryption/encryption_test.go b/test/e2e-encryption/encryption_test.go index 7f56615569..37f558e7bf 100644 --- a/test/e2e-encryption/encryption_test.go +++ b/test/e2e-encryption/encryption_test.go @@ -67,7 +67,7 @@ func TestEncryptionIntegration(tt *testing.T) { // kube clients kubeClient, err := kubernetes.NewForConfig(kubeConfig) require.NoError(t, err) - kubeInformers := v1helpers.NewKubeInformersForNamespaces(kubeClient, "openshift-config-managed") + kubeInformers := v1helpers.NewKubeInformersForNamespaces(kubeClient, "openshift-config-managed", "openshift-config") apiextensionsClient, err := v1.NewForConfig(kubeConfig) require.NoError(t, err) @@ -292,7 +292,7 @@ func TestEncryptionIntegration(tt *testing.T) { keySecret, err := kubeClient.CoreV1().Secrets("openshift-config-managed").Get(ctx, fmt.Sprintf("encryption-key-%s-%s", component, keyID), metav1.GetOptions{}) require.NoError(t, err) - keyPluginData := keySecret.Data[secrets.EncryptionSecretKMSPluginConfig] + keyPluginData := keySecret.Data["encryption.apiserver.operator.openshift.io-kms-plugin-config"] require.NotEmpty(t, keyPluginData, "key secret %s missing kms-plugin-config data", keyID) keyPluginConfig, err := encoding.DecodeKMSPluginConfig(keyPluginData) require.NoError(t, err) @@ -305,6 +305,43 @@ func TestEncryptionIntegration(tt *testing.T) { } } + verifyKMSSecretData := func() { + t.Helper() + encryptionConfigSecret, err := kubeClient.CoreV1().Secrets("openshift-config-managed").Get(ctx, fmt.Sprintf("encryption-config-%s", component), metav1.GetOptions{}) + require.NoError(t, err) + cfg, err := encryptiondata.FromSecret(encryptionConfigSecret) + require.NoError(t, err) + + expectedKeyIDs := map[string]bool{} + for _, rc := range cfg.Encryption.Resources { + for _, p := range rc.Providers { + if p.KMS != nil { + parts := strings.SplitN(p.KMS.Name, "_", 2) + require.Len(t, parts, 2, "unexpected KMS provider name format: %s", p.KMS.Name) + expectedKeyIDs[parts[0]] = true + } + } + } + + for keyID := range expectedKeyIDs { + perKeyData, ok := cfg.KMSPluginsSecretData.ByKeyID[keyID] + require.True(t, ok, "expected secret data for keyID %s in encryption-config secret", keyID) + require.NotEmpty(t, perKeyData.Entries, "expected non-empty secret data for keyID %s", keyID) + + // Verify actual values match the source secret + vaultData, ok := perKeyData.Entries["vault-approle-secret"] + require.True(t, ok, "expected vault-approle-secret data for keyID %s", keyID) + require.Equal(t, "test-role-id", string(vaultData["role-id"]), "role-id secret data mismatch for keyID %s", keyID) + require.Equal(t, "test-secret-id", string(vaultData["secret-id"]), "secret-id secret data mismatch for keyID %s", keyID) + + // Verify Key Secret also carries the secret data + keySecret, err := kubeClient.CoreV1().Secrets("openshift-config-managed").Get(ctx, fmt.Sprintf("encryption-key-%s-%s", component, keyID), metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, "test-role-id", string(keySecret.Data["encryption.apiserver.operator.openshift.io-kms-plugin-secret-vault-approle-secret_role-id"]), "key secret %s role-id secret data mismatch", keyID) + require.Equal(t, "test-secret-id", string(keySecret.Data["encryption.apiserver.operator.openshift.io-kms-plugin-secret-vault-approle-secret_secret-id"]), "key secret %s secret-id secret data mismatch", keyID) + } + } + t.Logf("Wait for initial Encrypted condition") waitForConditionStatus("Encrypted", operatorv1.ConditionFalse) @@ -452,6 +489,18 @@ func TestEncryptionIntegration(tt *testing.T) { ) waitForConditionStatus("Encrypted", operatorv1.ConditionTrue) + t.Logf("Create vault AppRole secret") + _, err = kubeClient.CoreV1().Secrets("openshift-config").Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "vault-approle-secret", Namespace: "openshift-config"}, + Data: map[string][]byte{ + "role-id": []byte("test-role-id"), + "secret-id": []byte("test-secret-id"), + }, + Type: corev1.SecretTypeOpaque, + }, metav1.CreateOptions{}) + require.NoError(t, err) + defer kubeClient.CoreV1().Secrets("openshift-config").Delete(ctx, "vault-approle-secret", metav1.DeleteOptions{}) + t.Logf("Switch to KMS") _, err = fakeApiServerClient.Patch(ctx, "cluster", types.MergePatchType, []byte(`{"spec":{"encryption":{"type":"KMS","kms":{"type":"Vault","vault":{"kmsPluginImage":"registry.example.com/kms-plugin@sha256:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890","vaultAddress":"https://vault.example.com","authentication":{"type":"AppRole","appRole":{"secret":{"name":"vault-approle-secret"}}},"transitKey":"test-transit-key"}}}}}`), metav1.PatchOptions{}) require.NoError(t, err) @@ -466,11 +515,12 @@ func TestEncryptionIntegration(tt *testing.T) { waitForMigration("8") waitForConditionStatus("Encrypted", operatorv1.ConditionTrue) verifyKMSPlugins() + verifyKMSSecretData() t.Logf("Verify KMS key secret contains provider config") kmsKeySecret, err := kubeClient.CoreV1().Secrets("openshift-config-managed").Get(ctx, fmt.Sprintf("encryption-key-%s-8", component), metav1.GetOptions{}) require.NoError(t, err) - kmsPluginConfigData := kmsKeySecret.Data[secrets.EncryptionSecretKMSPluginConfig] + kmsPluginConfigData := kmsKeySecret.Data["encryption.apiserver.operator.openshift.io-kms-plugin-config"] require.NotEmpty(t, kmsPluginConfigData, "expected kms-plugin-config data to be present in key secret") pluginConfig, err := encoding.DecodeKMSPluginConfig(kmsPluginConfigData) require.NoError(t, err) @@ -489,6 +539,7 @@ func TestEncryptionIntegration(tt *testing.T) { ) waitForConditionStatus("Encrypted", operatorv1.ConditionTrue) verifyKMSPlugins() + verifyKMSSecretData() t.Logf("Switch back to KMS") _, err = fakeApiServerClient.Patch(ctx, "cluster", types.MergePatchType, []byte(`{"spec":{"encryption":{"type":"KMS","kms":{"type":"Vault","vault":{"kmsPluginImage":"registry.example.com/kms-plugin@sha256:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890","vaultAddress":"https://vault.example.com","authentication":{"type":"AppRole","appRole":{"secret":{"name":"vault-approle-secret"}}},"transitKey":"test-transit-key"}}}}}`), metav1.PatchOptions{}) @@ -504,6 +555,7 @@ func TestEncryptionIntegration(tt *testing.T) { waitForMigration("10") waitForConditionStatus("Encrypted", operatorv1.ConditionTrue) verifyKMSPlugins() + verifyKMSSecretData() t.Logf("Rotate KMS key via aescbc (KMS->AESCBC->KMS)") _, err = fakeApiServerClient.Patch(ctx, "cluster", types.MergePatchType, []byte(`{"spec":{"encryption":{"type":"aescbc","kms":null}}}`), metav1.PatchOptions{}) @@ -516,6 +568,7 @@ func TestEncryptionIntegration(tt *testing.T) { ) waitForConditionStatus("Encrypted", operatorv1.ConditionTrue) verifyKMSPlugins() + verifyKMSSecretData() t.Logf("Switch back to KMS after rotation") _, err = fakeApiServerClient.Patch(ctx, "cluster", types.MergePatchType, []byte(`{"spec":{"encryption":{"type":"KMS","kms":{"type":"Vault","vault":{"kmsPluginImage":"registry.example.com/kms-plugin@sha256:abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890","vaultAddress":"https://vault.example.com","authentication":{"type":"AppRole","appRole":{"secret":{"name":"vault-approle-secret"}}},"transitKey":"test-transit-key"}}}}}`), metav1.PatchOptions{}) @@ -531,6 +584,7 @@ func TestEncryptionIntegration(tt *testing.T) { waitForMigration("12") waitForConditionStatus("Encrypted", operatorv1.ConditionTrue) verifyKMSPlugins() + verifyKMSSecretData() t.Logf("Delete the encryption-config while in KMS mode") _, err = kubeClient.CoreV1().Secrets("openshift-config-managed").Patch(ctx, fmt.Sprintf("encryption-config-%s", component), types.JSONPatchType, []byte(`[{"op":"remove","path":"/metadata/finalizers"}]`), metav1.PatchOptions{})