From fc0219d477aff0310bc9057bdbd775928fe426b5 Mon Sep 17 00:00:00 2001 From: Fabio Bertinatto Date: Wed, 22 Jul 2026 08:08:12 -0400 Subject: [PATCH 1/3] kms: export GetDesiredEncryptionState in operator/encryption/statemachine Export the desired encryption state helper so other encryption controllers can reuse the same state transition logic instead of reimplementing it. --- pkg/operator/encryption/statemachine/transition.go | 6 +++--- pkg/operator/encryption/statemachine/transition_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/operator/encryption/statemachine/transition.go b/pkg/operator/encryption/statemachine/transition.go index 12eb5b8b90..b8f143938f 100644 --- a/pkg/operator/encryption/statemachine/transition.go +++ b/pkg/operator/encryption/statemachine/transition.go @@ -56,12 +56,12 @@ func GetEncryptionConfigAndState( if err != nil { return nil, nil, nil, "", err } - desiredEncryptionState := getDesiredEncryptionState(secretData, encryptionSecrets, encryptedGRs) + desiredEncryptionState := GetDesiredEncryptionState(secretData, encryptionSecrets, encryptedGRs) return secretData, desiredEncryptionState, encryptionSecrets, "", nil } -// getDesiredEncryptionState returns the desired state of encryption for all resources. +// GetDesiredEncryptionState returns the desired state of encryption for all resources. // To do this it compares the current state against the available secrets and to-be-encrypted resources. // oldEncryptionConfig can be nil if there is no config yet. // If there are no secrets, the identity is set for all resources as write key. @@ -73,7 +73,7 @@ func GetEncryptionConfigAndState( // 2. every GR must have all the read-keys (existing as secrets) since last complete migration. // 3. if (2) is the case, the write-key must be the most recent key. // 4. if (2) and (3) are the case, all non-write keys should be removed. -func getDesiredEncryptionState(oldSecretData *encryptiondata.Config, encryptionSecrets []*corev1.Secret, toBeEncryptedGRs []schema.GroupResource) map[schema.GroupResource]state.GroupResourceState { +func GetDesiredEncryptionState(oldSecretData *encryptiondata.Config, encryptionSecrets []*corev1.Secret, toBeEncryptedGRs []schema.GroupResource) map[schema.GroupResource]state.GroupResourceState { // // STEP 0: start with old encryption config, and alter it towards the desired state in the following STEPs. // diff --git a/pkg/operator/encryption/statemachine/transition_test.go b/pkg/operator/encryption/statemachine/transition_test.go index 11f0beed5e..ffb0072583 100644 --- a/pkg/operator/encryption/statemachine/transition_test.go +++ b/pkg/operator/encryption/statemachine/transition_test.go @@ -1302,7 +1302,7 @@ func TestGetDesiredEncryptionState(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := getDesiredEncryptionState(tt.args.oldEncryptionConfig, tt.args.encryptionSecrets, tt.args.toBeEncryptedGRs) + got := GetDesiredEncryptionState(tt.args.oldEncryptionConfig, tt.args.encryptionSecrets, tt.args.toBeEncryptedGRs) if tt.validate != nil { tt.validate(t, &tt.args, got) } From 1f6cfbf3882a06cdc96e8dac2e072db38c6d5c82 Mon Sep 17 00:00:00 2001 From: Fabio Bertinatto Date: Thu, 6 Aug 2026 13:49:05 -0400 Subject: [PATCH 2/3] kms: share encryption key planning helpers Move KMS key planning and key secret construction into reusable helpers so the key controller and preflight flow can build the same next-key shape from one implementation. --- .../controllers/encryption_key_helpers.go | 204 ++++++++++++++++++ .../encryption_key_helpers_test.go | 99 +++++++++ .../encryption/controllers/key_controller.go | 135 +++--------- 3 files changed, 330 insertions(+), 108 deletions(-) create mode 100644 pkg/operator/encryption/controllers/encryption_key_helpers.go create mode 100644 pkg/operator/encryption/controllers/encryption_key_helpers_test.go diff --git a/pkg/operator/encryption/controllers/encryption_key_helpers.go b/pkg/operator/encryption/controllers/encryption_key_helpers.go new file mode 100644 index 0000000000..fa650d57db --- /dev/null +++ b/pkg/operator/encryption/controllers/encryption_key_helpers.go @@ -0,0 +1,204 @@ +package controllers + +import ( + "context" + "encoding/base64" + "fmt" + "sort" + "strings" + + configv1 "github.com/openshift/api/config/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + apiserverv1 "k8s.io/apiserver/pkg/apis/apiserver/v1" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + + "github.com/openshift/library-go/pkg/operator/encryption/crypto" + "github.com/openshift/library-go/pkg/operator/encryption/secrets" + "github.com/openshift/library-go/pkg/operator/encryption/state" +) + +type encryptionKeyPlan struct { + needed bool + keyID uint64 + reasons []string + internalReason string +} + +func planNextEncryptionKey( + desiredEncryptionState map[schema.GroupResource]state.GroupResourceState, + currentMode state.Mode, + externalReason string, + encryptedGRs []schema.GroupResource, + desiredProviderCfg kmsProviderConfig, +) (*encryptionKeyPlan, error) { + plan := &encryptionKeyPlan{} + reasons := []string{} + + var ( + commonReason string + hasCommonReason bool + commonReasonDiffers bool + ) + + for gr, grKeys := range desiredEncryptionState { + latestKeyID, internalReason, needed, err := needsNewKey(grKeys, currentMode, externalReason, encryptedGRs, desiredProviderCfg) + if err != nil { + return nil, err + } + if !needed { + continue + } + + if !hasCommonReason { + commonReason = internalReason + hasCommonReason = true + } else if commonReason != internalReason { + commonReasonDiffers = true + } + + plan.needed = true + nextKeyID := latestKeyID + 1 + if plan.keyID < nextKeyID { + plan.keyID = nextKeyID + } + reasons = append(reasons, fmt.Sprintf("%s-%s", gr.Resource, internalReason)) + } + + if !plan.needed { + return plan, nil + } + if hasCommonReason && !commonReasonDiffers && len(reasons) > 1 { + reasons = []string{commonReason} + } + + sort.Strings(reasons) + plan.reasons = reasons + plan.internalReason = strings.Join(reasons, ", ") + return plan, nil +} + +// encryptionKeyBuildResult is the in-memory key material plus any referenced +// Secret/ConfigMap fetched while building it. Callers that also need to hash +// the KMS config (key controller) reuse the refs to avoid a second API round-trip. +type encryptionKeyBuildResult struct { + keyState state.KeyState + refSecret *corev1.Secret + refCM *corev1.ConfigMap +} + +func buildEncryptionKeyState( + ctx context.Context, + keyID uint64, + currentMode state.Mode, + apiServerEncryption configv1.APIServerEncryption, + desiredProviderCfg kmsProviderConfig, + secretClient corev1client.SecretsGetter, + configMapClient corev1client.ConfigMapsGetter, + internalReason string, + externalReason string, + kmsEndpointOverride string, +) (*encryptionKeyBuildResult, error) { + bs := crypto.ModeToNewKeyFunc[currentMode]() + result := &encryptionKeyBuildResult{ + keyState: state.KeyState{ + Key: apiserverv1.Key{ + Name: fmt.Sprintf("%d", keyID), + Secret: base64.StdEncoding.EncodeToString(bs), + }, + Mode: currentMode, + InternalReason: internalReason, + ExternalReason: externalReason, + }, + } + + if currentMode != state.KMS { + return result, nil + } + + endpoint := kmsEndpointOverride + if len(endpoint) == 0 { + endpoint = fmt.Sprintf(kmsEndpointFormat, keyID) + } + result.keyState.KMS = &state.KMSState{ + Encryption: &apiserverv1.KMSConfiguration{ + APIVersion: "v2", + Name: fmt.Sprintf("%d", keyID), + Endpoint: endpoint, + Timeout: &metav1.Duration{Duration: defaultKMSTimeout}, + }, + Plugin: apiServerEncryption.KMS, + } + + if secretName, expectedKeys, err := desiredProviderCfg.referencedSecretName(); err != nil { + return nil, err + } else if len(secretName) > 0 { + refSecret, err := 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) + } + result.refSecret = refSecret + 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 := result.keyState.KMS.PluginSecretData.Set(secretName, key, v); err != nil { + return nil, err + } + } + } + + if cmName, expectedKeys, err := desiredProviderCfg.referencedConfigMapName(); err != nil { + return nil, err + } else if len(cmName) > 0 { + refCM, err := configMapClient.ConfigMaps(openshiftConfigNS).Get(ctx, cmName, metav1.GetOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to get configmap %s in %s: %w", cmName, openshiftConfigNS, err) + } + result.refCM = refCM + for _, key := range expectedKeys { + v, ok := refCM.Data[key] + if !ok { + return nil, fmt.Errorf("configmap %s in %s is missing required key %q", cmName, openshiftConfigNS, key) + } + if err := result.keyState.KMS.PluginConfigMapData.Set(cmName, key, []byte(v)); err != nil { + return nil, err + } + } + } + + return result, nil +} + +func buildEncryptionKeySecret( + ctx context.Context, + instanceName string, + keyID uint64, + currentMode state.Mode, + apiServerEncryption configv1.APIServerEncryption, + desiredProviderCfg kmsProviderConfig, + secretClient corev1client.SecretsGetter, + configMapClient corev1client.ConfigMapsGetter, + internalReason string, + externalReason string, + kmsEndpointOverride string, +) (*corev1.Secret, error) { + result, err := buildEncryptionKeyState( + ctx, + keyID, + currentMode, + apiServerEncryption, + desiredProviderCfg, + secretClient, + configMapClient, + internalReason, + externalReason, + kmsEndpointOverride, + ) + if err != nil { + return nil, err + } + return secrets.FromKeyState(instanceName, result.keyState) +} diff --git a/pkg/operator/encryption/controllers/encryption_key_helpers_test.go b/pkg/operator/encryption/controllers/encryption_key_helpers_test.go new file mode 100644 index 0000000000..dbd44aca22 --- /dev/null +++ b/pkg/operator/encryption/controllers/encryption_key_helpers_test.go @@ -0,0 +1,99 @@ +package controllers + +import ( + "context" + "strings" + "testing" + + configv1 "github.com/openshift/api/config/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + + "github.com/openshift/library-go/pkg/operator/encryption/state" +) + +func TestBuildEncryptionKeyStateMissingRefs(t *testing.T) { + apiServerEncryption := configv1.APIServerEncryption{ + Type: configv1.EncryptionTypeKMS, + KMS: configv1.KMSPluginConfig{ + Type: configv1.VaultKMSProvider, + Vault: wellKnownBaseVaultConfig, + }, + } + providerCfg, err := newKMSProviderConfig(apiServerEncryption.KMS) + if err != nil { + t.Fatalf("failed to create provider config: %v", err) + } + + t.Run("missing referenced secret", func(t *testing.T) { + client := fake.NewSimpleClientset(&wellKnownBaseConfigMap) + _, err := buildEncryptionKeyState( + context.TODO(), + 1, + state.KMS, + apiServerEncryption, + providerCfg, + client.CoreV1(), + client.CoreV1(), + "test-reason", + "", + "", + ) + if err == nil { + t.Fatal("expected an error") + } + if !strings.Contains(err.Error(), "vault-approle") { + t.Fatalf("expected error mentioning the missing secret, got: %v", err) + } + }) + + t.Run("missing referenced configmap", func(t *testing.T) { + client := fake.NewSimpleClientset(&wellKnownBaseSecret) + _, err := buildEncryptionKeyState( + context.TODO(), + 1, + state.KMS, + apiServerEncryption, + providerCfg, + client.CoreV1(), + client.CoreV1(), + "test-reason", + "", + "", + ) + if err == nil { + t.Fatal("expected an error") + } + if !strings.Contains(err.Error(), "vault-ca-bundle") { + t.Fatalf("expected error mentioning the missing configmap, got: %v", err) + } + }) + + t.Run("returns fetched refs for hasher reuse", func(t *testing.T) { + client := fake.NewSimpleClientset([]runtime.Object{&wellKnownBaseSecret, &wellKnownBaseConfigMap}...) + result, err := buildEncryptionKeyState( + context.TODO(), + 1, + state.KMS, + apiServerEncryption, + providerCfg, + client.CoreV1(), + client.CoreV1(), + "test-reason", + "", + "unix:///var/run/kmsplugin/kms.sock", + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result.refSecret == nil || result.refSecret.Name != "vault-approle" { + t.Fatalf("expected refSecret vault-approle, got %+v", result.refSecret) + } + if result.refCM == nil || result.refCM.Name != "vault-ca-bundle" { + t.Fatalf("expected refCM vault-ca-bundle, got %+v", result.refCM) + } + if result.keyState.KMS == nil || result.keyState.KMS.Encryption.Endpoint != "unix:///var/run/kmsplugin/kms.sock" { + t.Fatalf("expected endpoint override, got %+v", result.keyState.KMS) + } + }) +} diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index 6f75f6d075..6da602fb11 100644 --- a/pkg/operator/encryption/controllers/key_controller.go +++ b/pkg/operator/encryption/controllers/key_controller.go @@ -3,11 +3,8 @@ package controllers import ( "bytes" "context" - "encoding/base64" "encoding/json" "fmt" - "sort" - "strings" "time" corev1 "k8s.io/api/core/v1" @@ -15,10 +12,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - apiserverv1 "k8s.io/apiserver/pkg/apis/apiserver/v1" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/klog/v2" - "k8s.io/utils/ptr" configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" @@ -27,7 +22,6 @@ import ( 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/kms" "github.com/openshift/library-go/pkg/operator/encryption/secrets" "github.com/openshift/library-go/pkg/operator/encryption/state" @@ -204,13 +198,7 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact return nil } - var ( - newKeyRequired bool - newKeyID uint64 - reasons []string - ) - - // note here that desiredEncryptionState is never empty because getDesiredEncryptionState + // note here that desiredEncryptionState is never empty because GetDesiredEncryptionState // fills up the state with all resources and set identity write key if write key secrets // are missing. @@ -223,40 +211,14 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact } } - var commonReason *string - for gr, grKeys := range desiredEncryptionState { - latestKeyID, internalReason, needed, err := needsNewKey(grKeys, currentMode, externalReason, encryptedGRs, desiredProviderCfg) - if err != nil { - return err - } - if !needed { - continue - } - - if commonReason == nil { - commonReason = &internalReason - } else if *commonReason != internalReason { - commonReason = ptr.To("") // this means we have no common reason - } - - newKeyRequired = true - nextKeyID := latestKeyID + 1 - if newKeyID < nextKeyID { - newKeyID = nextKeyID - } - reasons = append(reasons, fmt.Sprintf("%s-%s", gr.Resource, internalReason)) + keyPlan, err := planNextEncryptionKey(desiredEncryptionState, currentMode, externalReason, encryptedGRs, desiredProviderCfg) + if err != nil { + return err } - if !newKeyRequired { + if !keyPlan.needed { return nil } - - if commonReason != nil && len(*commonReason) > 0 && len(reasons) > 1 { - reasons = []string{*commonReason} // don't repeat reasons - } - - sort.Sort(sort.StringSlice(reasons)) - internalReason := strings.Join(reasons, ", ") - keySecret, preconditionMet, err := c.generateKeySecret(ctx, newKeyID, currentMode, apiEncryptionConfiguration, desiredProviderCfg, internalReason, externalReason) + keySecret, preconditionMet, err := c.generateKeySecret(ctx, keyPlan.keyID, currentMode, apiEncryptionConfiguration, desiredProviderCfg, keyPlan.internalReason, externalReason) if err != nil { return fmt.Errorf("failed to create key: %v", err) } @@ -266,14 +228,14 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact } _, createErr := c.secretClient.Secrets("openshift-config-managed").Create(ctx, keySecret, metav1.CreateOptions{}) if errors.IsAlreadyExists(createErr) { - return c.validateExistingSecret(ctx, keySecret, newKeyID) + return c.validateExistingSecret(ctx, keySecret, keyPlan.keyID) } if createErr != nil { syncContext.Recorder().Warningf("EncryptionKeyCreateFailed", "Secret %q failed to create: %v", keySecret.Name, err) return createErr } - syncContext.Recorder().Eventf("EncryptionKeyCreated", "Secret %q successfully created: %q", keySecret.Name, reasons) + syncContext.Recorder().Eventf("EncryptionKeyCreated", "Secret %q successfully created: %q", keySecret.Name, keyPlan.reasons) return nil } @@ -307,69 +269,26 @@ func (c *keyController) validateExistingSecret(ctx context.Context, keySecret *c // - (nil, false, nil) — preflight still in progress; caller should back off. // - (nil, false, err) — preflight failed or transient error; caller should surface it. func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, currentMode state.Mode, apiServerEncryption configv1.APIServerEncryption, desiredProviderCfg kmsProviderConfig, internalReason, externalReason string) (*corev1.Secret, bool, error) { - bs := crypto.ModeToNewKeyFunc[currentMode]() - ks := state.KeyState{ - Key: apiserverv1.Key{ - Name: fmt.Sprintf("%d", keyID), - Secret: base64.StdEncoding.EncodeToString(bs), - }, - Mode: currentMode, - InternalReason: internalReason, - ExternalReason: externalReason, + result, err := buildEncryptionKeyState( + ctx, + keyID, + currentMode, + apiServerEncryption, + desiredProviderCfg, + c.secretClient, + c.configMapClient, + internalReason, + externalReason, + "", // use the real operand endpoint format + ) + if err != nil { + return nil, false, err } - if currentMode == state.KMS { - ks.KMS = &state.KMSState{ - Encryption: &apiserverv1.KMSConfiguration{ - APIVersion: "v2", - Name: fmt.Sprintf("%d", keyID), - Endpoint: fmt.Sprintf(kmsEndpointFormat, keyID), - Timeout: &metav1.Duration{Duration: defaultKMSTimeout}, - }, - Plugin: apiServerEncryption.KMS, - } - // Fetch the referenced Secret and ConfigMap, copying their data into the - // key state. The fetched objects are reused by prefetchedKMSConfigHasherResourceProvider - // to compute the config hash without a second API round-trip. - var refSecret *corev1.Secret - if secretName, expectedKeys, err := desiredProviderCfg.referencedSecretName(); err != nil { - return nil, false, err - } else if len(secretName) > 0 { - refSecret, err = c.secretClient.Secrets(openshiftConfigNS).Get(ctx, secretName, metav1.GetOptions{}) - if err != nil { - return nil, false, 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, false, 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, false, err - } - } - } - - var refCM *corev1.ConfigMap - if cmName, expectedKeys, err := desiredProviderCfg.referencedConfigMapName(); err != nil { - return nil, false, err - } else if len(cmName) > 0 { - refCM, err = c.configMapClient.ConfigMaps(openshiftConfigNS).Get(ctx, cmName, metav1.GetOptions{}) - if err != nil { - return nil, false, fmt.Errorf("failed to get configmap %s in %s: %w", cmName, openshiftConfigNS, err) - } - for _, key := range expectedKeys { - v, ok := refCM.Data[key] - if !ok { - return nil, false, fmt.Errorf("configmap %s in %s is missing required key %q", cmName, openshiftConfigNS, key) - } - if err := ks.KMS.PluginConfigMapData.Set(cmName, key, []byte(v)); err != nil { - return nil, false, err - } - } - } - - resources := &prefetchedKMSConfigHasherResourceProvider{secret: refSecret, configMap: refCM} + if currentMode == state.KMS { + // Reuse the Secret/ConfigMap already fetched by buildEncryptionKeyState + // so hashing does not require a second API round-trip. + resources := &prefetchedKMSConfigHasherResourceProvider{secret: result.refSecret, configMap: result.refCM} hasher, err := newKMSConfigHasher(desiredProviderCfg, resources, openshiftConfigNS) if err != nil { return nil, false, fmt.Errorf("failed to create KMS config hasher: %w", err) @@ -387,7 +306,7 @@ func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, cur } } - secret, err := secrets.FromKeyState(c.instanceName, ks) + secret, err := secrets.FromKeyState(c.instanceName, result.keyState) if err != nil { return nil, false, err } From 96361f8bff5710a327a0e47c9f0fcf89af488034 Mon Sep 17 00:00:00 2001 From: Fabio Bertinatto Date: Thu, 6 Aug 2026 13:49:11 -0400 Subject: [PATCH 3/3] kms: simulate next KMS config for preflight Build the candidate encryption config that would result from the next KMS key before launching preflight so the checker validates the exact secret and plugin data that rollout will use. --- pkg/operator/encryption/controllers.go | 3 + .../controllers/kms_preflight_controller.go | 112 ++++- .../kms_preflight_controller_test.go | 421 +++++++++++++++++- 3 files changed, 513 insertions(+), 23 deletions(-) diff --git a/pkg/operator/encryption/controllers.go b/pkg/operator/encryption/controllers.go index 3b876b76d4..ec45bd60a1 100644 --- a/pkg/operator/encryption/controllers.go +++ b/pkg/operator/encryption/controllers.go @@ -136,11 +136,14 @@ func NewControllers( provider, encryptionEnabledChecker.PreconditionFulfilled, preflightDeployer, + deployer, operatorClient, apiServerClient, apiServerInformer, + kubeInformersForNamespaces, secretsClient, configMapClient, + encryptionSecretSelector, encryptionStatusProvider, eventRecorder, )) diff --git a/pkg/operator/encryption/controllers/kms_preflight_controller.go b/pkg/operator/encryption/controllers/kms_preflight_controller.go index 4d089bf81e..58f2fc86aa 100644 --- a/pkg/operator/encryption/controllers/kms_preflight_controller.go +++ b/pkg/operator/encryption/controllers/kms_preflight_controller.go @@ -23,7 +23,10 @@ import ( 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/encryptiondata" "github.com/openshift/library-go/pkg/operator/encryption/kms" + "github.com/openshift/library-go/pkg/operator/encryption/state" + "github.com/openshift/library-go/pkg/operator/encryption/statemachine" "github.com/openshift/library-go/pkg/operator/events" operatorv1helpers "github.com/openshift/library-go/pkg/operator/v1helpers" ) @@ -198,6 +201,7 @@ type KMSPreflightDeployer interface { type kmsPreflightController struct { controllerInstanceName string + instanceName string operatorClient operatorv1helpers.OperatorClient apiServerClient configv1client.APIServerInterface @@ -214,7 +218,12 @@ type kmsPreflightController struct { // An alternative would be to check a cached lister before issuing Delete // calls, but the encryption controllers do not use informers and have no // shared cache available. - dirtyDeployer bool + dirtyDeployer bool + // encryptionDeployer is used to compute the exact encryption config secret that + // will be deployed once the key-controller creates the key this preflight is + // validating for. See computeEncryptionConfigSecret. + encryptionDeployer statemachine.Deployer + encryptionSecretSelector metav1.ListOptions provider Provider preconditionsFulfilledFn preconditionsFulfilled encryptionStatusProvider kms.EncryptionStatusProvider @@ -295,20 +304,30 @@ func NewKMSPreflightController( provider Provider, preconditionsFulfilledFn preconditionsFulfilled, deployer KMSPreflightDeployer, + // encryptionDeployer is the same statemachine.Deployer passed to the other + // encryption controllers (key, state, prune, migration). It is used to read the + // currently deployed encryption config secret when computing the exact config + // that will result from the key the key-controller is about to create. + encryptionDeployer statemachine.Deployer, operatorClient operatorv1helpers.OperatorClient, apiServerClient configv1client.APIServerInterface, apiServerInformer configv1informers.APIServerInformer, + kubeInformersForNamespaces operatorv1helpers.KubeInformersForNamespaces, // secretsClient and configMapsClient read referenced Secrets and ConfigMaps in - // openshift-config for hash computation. No informer is needed: the key-controller - // detects config changes and updates ObservedConfigHash, which triggers this - // controller via the operatorClient informer. The minute-based resync covers the rest. + // openshift-config for hash computation, and list key secrets in + // openshift-config-managed for encryption-config simulation. No openshift-config + // informer is needed: the key-controller detects config changes and updates + // ObservedConfigHash, which triggers this controller via the operatorClient + // informer. The minute-based resync covers the rest. secretsClient corev1client.SecretsGetter, configMapsClient corev1client.ConfigMapsGetter, + encryptionSecretSelector metav1.ListOptions, encryptionStatusProvider kms.EncryptionStatusProvider, eventRecorder events.Recorder, ) factory.Controller { c := &kmsPreflightController{ controllerInstanceName: factory.ControllerInstanceName(instanceName, "EncryptionKMSPreflight"), + instanceName: instanceName, operatorClient: operatorClient, apiServerClient: apiServerClient, @@ -318,6 +337,8 @@ func NewKMSPreflightController( deployer: deployer, // assume resources may exist from a previous process run dirtyDeployer: true, + encryptionDeployer: encryptionDeployer, + encryptionSecretSelector: encryptionSecretSelector, provider: provider, preconditionsFulfilledFn: preconditionsFulfilledFn, encryptionStatusProvider: encryptionStatusProvider, @@ -329,6 +350,8 @@ func NewKMSPreflightController( ResyncEvery(time.Minute). WithInformers( operatorClient.Informer(), + kubeInformersForNamespaces.InformersFor("openshift-config-managed").Core().V1().Secrets().Informer(), + encryptionDeployer, ).ToController( c.controllerInstanceName, eventRecorder.WithComponentSuffix("encryption-kms-preflight-controller"), @@ -502,8 +525,14 @@ func (c *kmsPreflightController) runPreflightChecks(ctx context.Context) (requeu } } c.dirtyDeployer = true - // TODO: compute the encryption configuration and pass it to the deployer - if err := c.deployer.Deploy(ctx, requiredHash, nil); err != nil { + requeueForConvergence, encryptionSecret, err := c.computeEncryptionConfigSecret(ctx) + if err != nil { + return false, "", "", fmt.Errorf("failed to compute encryption config for preflight: %w", err) + } + if requeueForConvergence { + return true, "RunningPreflightCheck", fmt.Sprintf("Waiting for encryption config to converge before deploying preflight pod for hash %s", requiredHash), nil + } + if err := c.deployer.Deploy(ctx, requiredHash, encryptionSecret); err != nil { return true, "", "", err } return true, "RunningPreflightCheck", fmt.Sprintf("Deploying preflight pod for hash %s", requiredHash), nil @@ -599,6 +628,77 @@ func (c *kmsPreflightController) cleanupDeployer(ctx context.Context) error { return nil } +// computeEncryptionConfigSecret computes the encryption config secret consumed by the preflight pod for the next KMS key rollout. +func (c *kmsPreflightController) computeEncryptionConfigSecret(ctx context.Context) (requeue bool, secret *corev1.Secret, err error) { + encryptedGRs := c.provider.EncryptedGRs() + + // Step 1: load the currently deployed encryption config and existing key secrets + // via the same shared path used by the key/state controllers. + currentConfig, desiredStateBeforeNewKey, existingKeySecrets, transitioningReason, err := statemachine.GetEncryptionConfigAndState( + ctx, c.encryptionDeployer, c.secretsClient, c.encryptionSecretSelector, encryptedGRs, + ) + if err != nil { + return false, nil, fmt.Errorf("failed to get encryption config and state: %w", err) + } + if len(transitioningReason) > 0 { + return true, nil, nil + } + + apiServer, err := c.apiServerClient.Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return false, nil, fmt.Errorf("failed to get apiserver config: %w", err) + } + providerCfg, err := newKMSProviderConfig(apiServer.Spec.Encryption.KMS) + if err != nil { + return false, nil, fmt.Errorf("failed to create KMS provider config: %w", err) + } + + // Step 2: determine the next key using the same shared planning logic as the key controller. + // TODO: pass externalReason from UnsupportedConfigOverrides like the key controller does + keyPlan, err := planNextEncryptionKey(desiredStateBeforeNewKey, state.KMS, "", encryptedGRs, providerCfg) + if err != nil { + return false, nil, fmt.Errorf("failed to plan next KMS key: %w", err) + } + if !keyPlan.needed { + return false, nil, fmt.Errorf("preflight required but no new KMS key is needed") + } + + // Step 3: build the simulated key secret using the same shared helper as the key controller. + simulatedKeySecret, err := buildEncryptionKeySecret( + ctx, + c.instanceName, + keyPlan.keyID, + state.KMS, + apiServer.Spec.Encryption, + providerCfg, + c.secretsClient, + c.configMapsClient, + keyPlan.internalReason, + "", + "unix:///var/run/kmsplugin/kms.sock", // TODO: override the KMS endpoint for the preflight checker + ) + if err != nil { + return false, nil, fmt.Errorf("failed to create simulated key secret: %w", err) + } + + // Step 4: run the state machine with the simulated key included, exactly as the state-controller will once the key-controller actually creates this key. + allKeySecrets := append(existingKeySecrets, simulatedKeySecret) + desiredState := statemachine.GetDesiredEncryptionState(currentConfig, allKeySecrets, encryptedGRs) + + // Step 5: convert the desired state into the encryption config secret, exactly as the state-controller does. + cfg, err := encryptiondata.FromEncryptionState(desiredState) + if err != nil { + return false, nil, fmt.Errorf("failed to build encryption config: %w", err) + } + + secretName := fmt.Sprintf("%s-%s", encryptiondata.EncryptionConfSecretName, c.instanceName) + secret, err = encryptiondata.ToSecret("openshift-config-managed", secretName, cfg) + if err != nil { + return false, nil, err + } + return false, secret, nil +} + // ensurePreflightResult writes result to KMSEncryptionStatus.Preflight.Result // if it has not already been written for this hash. It is a no-op when // existingResult is non-nil and its ConfigHash already matches result.ConfigHash, diff --git a/pkg/operator/encryption/controllers/kms_preflight_controller_test.go b/pkg/operator/encryption/controllers/kms_preflight_controller_test.go index bcf283811c..c4c158f97f 100644 --- a/pkg/operator/encryption/controllers/kms_preflight_controller_test.go +++ b/pkg/operator/encryption/controllers/kms_preflight_controller_test.go @@ -2,28 +2,34 @@ package controllers import ( "context" + "encoding/base64" "fmt" "strings" "testing" "time" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + apiserverconfigv1 "k8s.io/apiserver/pkg/apis/apiserver/v1" "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/cache" clocktesting "k8s.io/utils/clock/testing" - "k8s.io/apimachinery/pkg/api/equality" - configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" configv1clientfake "github.com/openshift/client-go/config/clientset/versioned/fake" 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/encryptiondata" "github.com/openshift/library-go/pkg/operator/encryption/kms" + "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" encryptiontesting "github.com/openshift/library-go/pkg/operator/encryption/testing" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/v1helpers" @@ -313,17 +319,19 @@ func TestKMSConfigHasher(t *testing.T) { } type fakeDeployer struct { - deployed bool - cleaned bool - cleanupCount int - deployErr error - statusErr error - cleanupErr error - podStatus corev1.PodStatus + deployed bool + cleaned bool + cleanupCount int + deployErr error + statusErr error + cleanupErr error + podStatus corev1.PodStatus + encryptionSecret *corev1.Secret } -func (f *fakeDeployer) Deploy(_ context.Context, _ string, _ *corev1.Secret) error { +func (f *fakeDeployer) Deploy(_ context.Context, _ string, encryptionSecret *corev1.Secret) error { f.deployed = true + f.encryptionSecret = encryptionSecret return f.deployErr } @@ -392,6 +400,7 @@ func TestKMSPreflightController(t *testing.T) { scenarios := []struct { name string deployer KMSPreflightDeployer + encryptionDeployer statemachine.Deployer encryptionStatusProvider *fakeEncryptionStatusProvider apiServerObjects []runtime.Object coreObjects []runtime.Object @@ -402,6 +411,7 @@ func TestKMSPreflightController(t *testing.T) { expectedConditions []operatorv1.OperatorCondition expectedKMSPreflightResult *operatorv1.KMSPreflightResult expectedEncryptionStatusProviderUpdateCalls int + expectDeployedEncryptionSecret bool }{ { name: "preconditions not met, clears degraded and progressing", @@ -466,13 +476,15 @@ func TestKMSPreflightController(t *testing.T) { }, { // Scenario 2b: deploying — progressing. - name: "hashes match, no pod exists, deploys and returns", - deployer: &fakeDeployer{statusErr: apierrors.NewNotFound(schema.GroupResource{Resource: "pods"}, "kms-preflight")}, - encryptionStatusProvider: &fakeEncryptionStatusProvider{observedConfigHash: wellKnownMatchingHashForBaseVaultConfig}, - apiServerObjects: []runtime.Object{apiServerWithKMS}, - coreObjects: []runtime.Object{&wellKnownBaseSecret, &wellKnownBaseConfigMap}, - initialDirtyDeployer: true, - preconditionsMet: true, + name: "hashes match, no pod exists, deploys and returns", + deployer: &fakeDeployer{statusErr: apierrors.NewNotFound(schema.GroupResource{Resource: "pods"}, "kms-preflight")}, + encryptionDeployer: &fakeEncryptionDeployer{converged: true}, + encryptionStatusProvider: &fakeEncryptionStatusProvider{observedConfigHash: wellKnownMatchingHashForBaseVaultConfig}, + apiServerObjects: []runtime.Object{apiServerWithKMS}, + coreObjects: []runtime.Object{&wellKnownBaseSecret, &wellKnownBaseConfigMap}, + initialDirtyDeployer: true, + preconditionsMet: true, + expectDeployedEncryptionSecret: true, expectedConditions: []operatorv1.OperatorCondition{ {Type: "EncryptionKMSPreflightControllerDegraded", Status: "False"}, {Type: "EncryptionKMSPreflightControllerProgressing", Status: "True", Reason: "RunningPreflightCheck", Message: "Deploying preflight pod for hash cuZm_g=="}, @@ -1029,14 +1041,22 @@ func TestKMSPreflightController(t *testing.T) { deployer = &fakeDeployer{} } + encryptionDeployer := scenario.encryptionDeployer + if encryptionDeployer == nil { + encryptionDeployer = &fakeEncryptionDeployer{converged: true} + } + c := &kmsPreflightController{ controllerInstanceName: factory.ControllerInstanceName("test", "EncryptionKMSPreflight"), + instanceName: "test", operatorClient: fakeOperatorClient, apiServerClient: fakeApiServerClient, secretsClient: fakeKubeClient.CoreV1(), configMapsClient: fakeKubeClient.CoreV1(), deployer: deployer, dirtyDeployer: scenario.initialDirtyDeployer, + encryptionDeployer: encryptionDeployer, + encryptionSecretSelector: metav1.ListOptions{}, provider: provider, preconditionsFulfilledFn: preconditionsFn, encryptionStatusProvider: scenario.encryptionStatusProvider, @@ -1074,8 +1094,375 @@ func TestKMSPreflightController(t *testing.T) { t.Errorf("expected dirtyDeployer=false, got true") } } + if scenario.expectDeployedEncryptionSecret { + if fakeDeployerInstance.encryptionSecret == nil { + t.Fatalf("expected Deploy to receive a non-nil encryption config secret") + } + cfg, err := encryptiondata.FromSecret(fakeDeployerInstance.encryptionSecret) + if err != nil { + t.Fatalf("Deploy received an invalid encryption config secret: %v", err) + } + kmsConfigs, err := encryptiondata.ExtractUniqueAndSortedKMSConfigurations(cfg) + if err != nil { + t.Fatalf("failed to extract KMS configurations from deployed secret: %v", err) + } + if len(kmsConfigs) == 0 { + t.Fatalf("expected deployed encryption config to contain at least one KMS configuration") + } + if _, ok := cfg.KMSPluginsSecretData.Get(kmsConfigs[0].Name); !ok { + t.Fatalf("expected deployed encryption config to carry KMS secret data for key %s", kmsConfigs[0].Name) + } + } encryptiontesting.ValidateOperatorClientConditions(t, fakeOperatorClient, scenario.expectedConditions) }) } } + +type fakeEncryptionDeployer struct { + secret *corev1.Secret + converged bool + err error +} + +func (f *fakeEncryptionDeployer) DeployedEncryptionConfigSecret(_ context.Context) (*corev1.Secret, bool, error) { + return f.secret, f.converged, f.err +} + +func (f *fakeEncryptionDeployer) AddEventHandler(_ cache.ResourceEventHandler) (cache.ResourceEventHandlerRegistration, error) { + return nil, nil +} + +func (f *fakeEncryptionDeployer) HasSynced() bool { return true } + +var _ statemachine.Deployer = &fakeEncryptionDeployer{} + +func TestComputeEncryptionConfigSecret(t *testing.T) { + apiServerWithKMS := &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.APIServerSpec{ + Encryption: configv1.APIServerEncryption{ + Type: configv1.EncryptionTypeKMS, + KMS: configv1.KMSPluginConfig{ + Type: configv1.VaultKMSProvider, + Vault: wellKnownBaseVaultConfig, + }, + }, + }, + } + encryptedGRs := []schema.GroupResource{{Group: "", Resource: "secrets"}} + + newExistingKeySecret := func(t *testing.T, keyID string) *corev1.Secret { + t.Helper() + // Existing keys use a different VaultKeyPath than the current apiserver config so + // needsNewKey reports kms-provider-changed (and thus needed=true). They are also + // marked migrated: needsNewKey refuses to create a new key until migration completes. + oldPlugin := apiServerWithKMS.Spec.Encryption.KMS + oldPlugin.Vault.VaultKeyPath = "transit/keys/old-key" + ks := state.KeyState{ + Key: apiserverconfigv1.Key{Name: keyID, Secret: base64.StdEncoding.EncodeToString(make([]byte, 16))}, + Mode: state.KMS, + Migrated: state.MigrationState{ + Resources: encryptedGRs, + }, + KMS: &state.KMSState{ + Encryption: &apiserverconfigv1.KMSConfiguration{ + APIVersion: "v2", + Name: keyID, + Endpoint: fmt.Sprintf("unix:///var/run/kmsplugin/kms-%s.sock", keyID), + Timeout: &metav1.Duration{Duration: 10 * time.Second}, + }, + Plugin: oldPlugin, + }, + } + if err := ks.KMS.PluginSecretData.Set("vault-approle", "role-id", []byte("old-role-id")); err != nil { + t.Fatalf("failed to set plugin secret data: %v", err) + } + if err := ks.KMS.PluginSecretData.Set("vault-approle", "secret-id", []byte("old-secret-id")); err != nil { + t.Fatalf("failed to set plugin secret data: %v", err) + } + if err := ks.KMS.PluginConfigMapData.Set("vault-ca-bundle", "ca-bundle.crt", []byte("old-ca-cert")); err != nil { + t.Fatalf("failed to set plugin configmap data: %v", err) + } + s, err := secrets.FromKeyState("test", ks) + if err != nil { + t.Fatalf("failed to build existing key secret: %v", err) + } + return s + } + + // newDeployedEncryptionConfig builds a converged encryption-config secret as the + // state controller would after the given key secrets are write keys. + newDeployedEncryptionConfig := func(t *testing.T, keySecrets ...*corev1.Secret) *corev1.Secret { + t.Helper() + desired := statemachine.GetDesiredEncryptionState(nil, keySecrets, encryptedGRs) + cfg, err := encryptiondata.FromEncryptionState(desired) + if err != nil { + t.Fatalf("failed to build intermediate encryption config: %v", err) + } + // Second pass promotes the write key once read keys are present. + desired = statemachine.GetDesiredEncryptionState(cfg, keySecrets, encryptedGRs) + cfg, err = encryptiondata.FromEncryptionState(desired) + if err != nil { + t.Fatalf("failed to build deployed encryption config: %v", err) + } + secret, err := encryptiondata.ToSecret("openshift-config-managed", "encryption-config-test", cfg) + if err != nil { + t.Fatalf("failed to serialize deployed encryption config: %v", err) + } + return secret + } + + newController := func(coreObjects []runtime.Object, encryptionDeployer statemachine.Deployer) *kmsPreflightController { + fakeKubeClient := fake.NewSimpleClientset(coreObjects...) + fakeConfigClient := configv1clientfake.NewSimpleClientset(apiServerWithKMS) + return &kmsPreflightController{ + instanceName: "test", + apiServerClient: fakeConfigClient.ConfigV1().APIServers(), + secretsClient: fakeKubeClient.CoreV1(), + configMapsClient: fakeKubeClient.CoreV1(), + encryptionDeployer: encryptionDeployer, + encryptionSecretSelector: metav1.ListOptions{}, + provider: newTestProvider(encryptedGRs), + } + } + + assertKeyCredentials := func(t *testing.T, cfg *encryptiondata.Config, keyID, roleID, secretID, caBundle string) { + t.Helper() + if _, ok := cfg.KMSPlugins[keyID]; !ok { + t.Fatalf("expected plugin config for keyID %s", keyID) + } + secretData, ok := cfg.KMSPluginsSecretData.Get(keyID) + if !ok { + t.Fatalf("expected secret data for keyID %s", keyID) + } + if v, ok := secretData.Get("vault-approle", "role-id"); !ok || string(v) != roleID { + t.Errorf("key %s: expected role-id %q, got %q (found=%v)", keyID, roleID, v, ok) + } + if v, ok := secretData.Get("vault-approle", "secret-id"); !ok || string(v) != secretID { + t.Errorf("key %s: expected secret-id %q, got %q (found=%v)", keyID, secretID, v, ok) + } + cmData, ok := cfg.KMSPluginsConfigMapData.Get(keyID) + if !ok { + t.Fatalf("expected configmap data for keyID %s", keyID) + } + if v, ok := cmData.Get("vault-ca-bundle", "ca-bundle.crt"); !ok || string(v) != caBundle { + t.Errorf("key %s: expected ca-bundle %q, got %q (found=%v)", keyID, caBundle, v, ok) + } + } + + t.Run("first key, no existing secrets, produces key ID 1", func(t *testing.T) { + c := newController([]runtime.Object{&wellKnownBaseSecret, &wellKnownBaseConfigMap}, &fakeEncryptionDeployer{converged: true}) + + requeue, secret, err := c.computeEncryptionConfigSecret(context.TODO()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if requeue { + t.Fatalf("expected no requeue") + } + if secret == nil { + t.Fatalf("expected a secret, got nil") + } + + cfg, err := encryptiondata.FromSecret(secret) + if err != nil { + t.Fatalf("failed to parse produced secret: %v", err) + } + kmsConfigs, err := encryptiondata.ExtractUniqueAndSortedKMSConfigurations(cfg) + if err != nil { + t.Fatalf("failed to extract KMS configurations: %v", err) + } + if len(kmsConfigs) != 1 { + t.Fatalf("expected 1 KMS configuration, got %d: %+v", len(kmsConfigs), kmsConfigs) + } + if kmsConfigs[0].Name != "1" { + t.Errorf("expected key ID 1, got %s", kmsConfigs[0].Name) + } + if kmsConfigs[0].Endpoint != "unix:///var/run/kmsplugin/kms.sock" { + t.Errorf("unexpected endpoint: %s", kmsConfigs[0].Endpoint) + } + assertKeyCredentials(t, cfg, "1", "role-123", "secret-456", "test-ca-cert") + + // Must match the state-controller shape for key 1, except that the + // simulated candidate key uses the preflight socket endpoint. + simulatedKey, err := secrets.FromKeyState("test", state.KeyState{ + Key: apiserverconfigv1.Key{Name: "1", Secret: base64.StdEncoding.EncodeToString(make([]byte, 16))}, + Mode: state.KMS, + KMS: &state.KMSState{ + Encryption: &apiserverconfigv1.KMSConfiguration{ + APIVersion: "v2", + Name: "1", + Endpoint: "unix:///var/run/kmsplugin/kms.sock", + Timeout: &metav1.Duration{Duration: 10 * time.Second}, + }, + Plugin: apiServerWithKMS.Spec.Encryption.KMS, + }, + }) + if err != nil { + t.Fatalf("failed to build comparison key: %v", err) + } + // Copy credentials into the comparison key the same way generateKeySecret would. + ks, err := secrets.ToKeyState(simulatedKey) + if err != nil { + t.Fatalf("failed to parse comparison key: %v", err) + } + _ = ks.KMS.PluginSecretData.Set("vault-approle", "role-id", []byte("role-123")) + _ = ks.KMS.PluginSecretData.Set("vault-approle", "secret-id", []byte("secret-456")) + _ = ks.KMS.PluginConfigMapData.Set("vault-ca-bundle", "ca-bundle.crt", []byte("test-ca-cert")) + simulatedKey, err = secrets.FromKeyState("test", ks) + if err != nil { + t.Fatalf("failed to rebuild comparison key: %v", err) + } + desired := statemachine.GetDesiredEncryptionState(nil, []*corev1.Secret{simulatedKey}, encryptedGRs) + wantCfg, err := encryptiondata.FromEncryptionState(desired) + if err != nil { + t.Fatalf("failed to build state-controller golden config: %v", err) + } + wantSecret, err := encryptiondata.ToSecret("openshift-config-managed", "encryption-config-test", wantCfg) + if err != nil { + t.Fatalf("failed to serialize golden config: %v", err) + } + if !equality.Semantic.DeepEqual(secret.Data, wantSecret.Data) { + t.Errorf("preflight secret data diverges from expected candidate config after key creation") + } + }) + + t.Run("existing key with deployed config, uses next key ID and retains credentials", func(t *testing.T) { + existingKeySecret := newExistingKeySecret(t, "3") + deployed := newDeployedEncryptionConfig(t, existingKeySecret) + c := newController( + []runtime.Object{&wellKnownBaseSecret, &wellKnownBaseConfigMap, existingKeySecret}, + &fakeEncryptionDeployer{converged: true, secret: deployed}, + ) + + requeue, secret, err := c.computeEncryptionConfigSecret(context.TODO()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if requeue { + t.Fatalf("expected no requeue") + } + + cfg, err := encryptiondata.FromSecret(secret) + if err != nil { + t.Fatalf("failed to parse produced secret: %v", err) + } + kmsConfigs, err := encryptiondata.ExtractUniqueAndSortedKMSConfigurations(cfg) + if err != nil { + t.Fatalf("failed to extract KMS configurations: %v", err) + } + + // Key 3 remains the write key (STEP 2 early return); key 4 is the new read key + // the key-controller is about to create. Both must be present. + var found3, found4 bool + for _, kc := range kmsConfigs { + switch kc.Name { + case "3": + found3 = true + case "4": + found4 = true + if kc.Endpoint != "unix:///var/run/kmsplugin/kms.sock" { + t.Errorf("unexpected endpoint for key 4: %s", kc.Endpoint) + } + } + } + if !found3 || !found4 { + t.Fatalf("expected key IDs 3 and 4 among KMS configurations, got %+v", kmsConfigs) + } + assertKeyCredentials(t, cfg, "3", "old-role-id", "old-secret-id", "old-ca-cert") + assertKeyCredentials(t, cfg, "4", "role-123", "secret-456", "test-ca-cert") + + // Golden: must match the expected candidate config once key 4 is + // simulated for preflight, with key 4 rewritten to the preflight socket + // and carrying the current apiserver plugin config (not the old key's). + newKey := newExistingKeySecret(t, "4") + ks, err := secrets.ToKeyState(newKey) + if err != nil { + t.Fatalf("failed to parse new key: %v", err) + } + ks.KMS.Encryption.Endpoint = "unix:///var/run/kmsplugin/kms.sock" + ks.KMS.Plugin = apiServerWithKMS.Spec.Encryption.KMS + _ = ks.KMS.PluginSecretData.Set("vault-approle", "role-id", []byte("role-123")) + _ = ks.KMS.PluginSecretData.Set("vault-approle", "secret-id", []byte("secret-456")) + _ = ks.KMS.PluginConfigMapData.Set("vault-ca-bundle", "ca-bundle.crt", []byte("test-ca-cert")) + newKey, err = secrets.FromKeyState("test", ks) + if err != nil { + t.Fatalf("failed to rebuild new key: %v", err) + } + deployedCfg, err := encryptiondata.FromSecret(deployed) + if err != nil { + t.Fatalf("failed to parse deployed config: %v", err) + } + desired := statemachine.GetDesiredEncryptionState(deployedCfg, []*corev1.Secret{existingKeySecret, newKey}, encryptedGRs) + wantCfg, err := encryptiondata.FromEncryptionState(desired) + if err != nil { + t.Fatalf("failed to build golden config: %v", err) + } + wantSecret, err := encryptiondata.ToSecret("openshift-config-managed", "encryption-config-test", wantCfg) + if err != nil { + t.Fatalf("failed to serialize golden config: %v", err) + } + if !equality.Semantic.DeepEqual(secret.Data, wantSecret.Data) { + t.Errorf("preflight secret data diverges from expected candidate config after key 4 creation") + } + }) + + t.Run("new candidate key uses preflight socket while existing key keeps original endpoint", func(t *testing.T) { + existingKeySecret := newExistingKeySecret(t, "3") + deployed := newDeployedEncryptionConfig(t, existingKeySecret) + c := newController( + []runtime.Object{&wellKnownBaseSecret, &wellKnownBaseConfigMap, existingKeySecret}, + &fakeEncryptionDeployer{converged: true, secret: deployed}, + ) + + _, secret, err := c.computeEncryptionConfigSecret(context.TODO()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + cfg, err := encryptiondata.FromSecret(secret) + if err != nil { + t.Fatalf("failed to parse produced secret: %v", err) + } + kmsConfigs, err := encryptiondata.ExtractUniqueAndSortedKMSConfigurations(cfg) + if err != nil { + t.Fatalf("failed to extract KMS configurations: %v", err) + } + + endpointsByKey := map[string]string{} + for _, kc := range kmsConfigs { + endpointsByKey[kc.Name] = kc.Endpoint + } + if endpointsByKey["3"] != "unix:///var/run/kmsplugin/kms-3.sock" { + t.Fatalf("expected existing key 3 to keep its original endpoint, got %q", endpointsByKey["3"]) + } + if endpointsByKey["4"] != "unix:///var/run/kmsplugin/kms.sock" { + t.Fatalf("expected candidate key 4 to use preflight endpoint %q, got %q", "unix:///var/run/kmsplugin/kms.sock", endpointsByKey["4"]) + } + }) + + t.Run("API server revisions not converged, requeues without error or secret", func(t *testing.T) { + c := newController([]runtime.Object{&wellKnownBaseSecret, &wellKnownBaseConfigMap}, &fakeEncryptionDeployer{converged: false}) + + requeue, secret, err := c.computeEncryptionConfigSecret(context.TODO()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !requeue { + t.Fatalf("expected requeue") + } + if secret != nil { + t.Fatalf("expected no secret, got %+v", secret) + } + }) + + t.Run("encryption deployer error is propagated", func(t *testing.T) { + c := newController(nil, &fakeEncryptionDeployer{err: fmt.Errorf("boom")}) + + _, _, err := c.computeEncryptionConfigSecret(context.TODO()) + if err == nil || !strings.Contains(err.Error(), "boom") { + t.Fatalf("expected error containing %q, got: %v", "boom", err) + } + }) +}