Skip to content

Commit acb8479

Browse files
committed
In-place field update for KMS mode regardless of the convergence
When a KMS sidecar revision is stuck (e.g. wrong container image), the cluster-admin needs to update APIServer.spec.encryption.kms with corrected values. Previously, all encryption controllers blocked on revision convergence, so the fix could never be applied. This change allows the key_controller to update the latest key secret's KMS plugin config in-place for non-migration fields (image, TLS, AppRole) regardless of convergence state. If there is a change in migration-triggering fields, we should wait until convergence happens. State controller carries in-place fields updates only when there is no change in the encryption-config data key.
1 parent b0f5231 commit acb8479

6 files changed

Lines changed: 777 additions & 23 deletions

File tree

pkg/operator/encryption/controllers/key_controller.go

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"time"
1212

1313
corev1 "k8s.io/api/core/v1"
14+
"k8s.io/apimachinery/pkg/api/equality"
1415
"k8s.io/apimachinery/pkg/api/errors"
1516
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1617
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -28,6 +29,7 @@ import (
2829

2930
"github.com/openshift/library-go/pkg/controller/factory"
3031
"github.com/openshift/library-go/pkg/operator/encryption/crypto"
32+
"github.com/openshift/library-go/pkg/operator/encryption/encoding"
3133
"github.com/openshift/library-go/pkg/operator/encryption/secrets"
3234
"github.com/openshift/library-go/pkg/operator/encryption/state"
3335
"github.com/openshift/library-go/pkg/operator/encryption/statemachine"
@@ -174,6 +176,16 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact
174176
return err
175177
}
176178

179+
// Apply in-place KMS plugin config updates (e.g. image, TLS) to the latest key
180+
// secret regardless of convergence. This unblocks stuck revisions and propagates
181+
// operational fixes like CVE image updates. Changes to migration-triggering fields
182+
// (transit-key, vault-address, transit-mount) are skipped via kmsMigrationRequired.
183+
if currentMode == state.KMS {
184+
if err := c.maybeUpdateKMSPluginConfigInPlace(ctx, syncContext, apiEncryptionConfiguration); err != nil {
185+
return err
186+
}
187+
}
188+
177189
currentConfig, desiredEncryptionState, secrets, isProgressingReason, err := statemachine.GetEncryptionConfigAndState(ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, encryptedGRs)
178190
if err != nil {
179191
return err
@@ -265,6 +277,103 @@ func (c *keyController) validateExistingSecret(ctx context.Context, keySecret *c
265277
return nil // we made this key earlier
266278
}
267279

280+
// maybeUpdateKMSPluginConfigInPlace updates the latest key secret's KMS plugin
281+
// config when only in-place-safe fields changed (image, TLS, authentication).
282+
func (c *keyController) maybeUpdateKMSPluginConfigInPlace(ctx context.Context, syncContext factory.SyncContext, apiServerEncryption configv1.APIServerEncryption) error {
283+
keySecrets, err := secrets.ListKeySecrets(ctx, c.secretClient, c.encryptionSecretSelector)
284+
if err != nil {
285+
return err
286+
}
287+
if len(keySecrets) == 0 {
288+
return nil
289+
}
290+
// Sort by key ID descending so [0] is the newest.
291+
// Parse the newest secret directly — fail fast if it is malformed
292+
// instead of silently falling back to an older key.
293+
sort.Slice(keySecrets, func(i, j int) bool {
294+
iKeyID, _ := state.NameToKeyID(keySecrets[i].Name)
295+
jKeyID, _ := state.NameToKeyID(keySecrets[j].Name)
296+
return iKeyID > jKeyID
297+
})
298+
// We only focus on the latest backed key.
299+
latest, err := secrets.ToKeyState(keySecrets[0])
300+
if err != nil {
301+
return fmt.Errorf("latest key secret %s is invalid: %w", keySecrets[0].Name, err)
302+
}
303+
304+
// Any mode mismatch (e.g. KMS <-> AESCBC) requires a migration, not an in-place
305+
// update. The normal needsNewKey path handles this after convergence.
306+
if latest.Mode != state.KMS {
307+
return nil
308+
}
309+
// This should never happen under normal operation because ToKeyState enforces
310+
// that KMS mode keys have a plugin config. This can only occur if someone
311+
// manually edited the key secret and removed the kms-plugin-config data field.
312+
// To mitigate, re-add the removed kms-plugin-config data to the key secret.
313+
if !latest.HasKMSPlugin() {
314+
return fmt.Errorf("latest KMS key %s is missing plugin config", latest.Key.Name)
315+
}
316+
// Skip when the plugin config is already up-to-date or when
317+
if equality.Semantic.DeepEqual(latest.KMS.Plugin, apiServerEncryption.KMS) {
318+
return nil
319+
}
320+
321+
migrationRequired, err := kmsMigrationRequired(latest.KMS.Plugin, apiServerEncryption.KMS)
322+
if err != nil {
323+
return err
324+
}
325+
// migration-triggering fields changed (needs a new key, not an in-place update).
326+
if migrationRequired {
327+
return nil
328+
}
329+
330+
keySecret, err := secrets.FromKeyState(c.instanceName, latest)
331+
if err != nil {
332+
return err
333+
}
334+
s, err := c.secretClient.Secrets(keySecret.Namespace).Get(ctx, keySecret.Name, metav1.GetOptions{})
335+
if err != nil {
336+
return fmt.Errorf("failed to get key secret %s/%s: %v", keySecret.Namespace, keySecret.Name, err)
337+
}
338+
pluginData, err := encoding.EncodeKMSPluginConfig(apiServerEncryption.KMS)
339+
if err != nil {
340+
return fmt.Errorf("failed to encode KMS plugin config: %v", err)
341+
}
342+
s.Data["encryption.apiserver.operator.openshift.io-kms-plugin-config"] = pluginData
343+
_, updateErr := c.secretClient.Secrets(s.Namespace).Update(ctx, s, metav1.UpdateOptions{})
344+
if errors.IsConflict(updateErr) {
345+
return nil
346+
}
347+
if updateErr == nil {
348+
syncContext.Recorder().Eventf("EncryptionKeyKMSPluginConfigUpdated", "Updated KMS plugin config on key secret %q in-place", s.Name)
349+
}
350+
return updateErr
351+
}
352+
353+
// kmsMigrationRequired reports whether the KMS config change between latest
354+
// (stored in the key secret) and current (from the APIServer CR) involves
355+
// migration-triggering fields that require a new encryption key.
356+
// Returns false when only in-place-safe fields differ (image, TLS, authentication)
357+
// or when configs are identical.
358+
func kmsMigrationRequired(latest, current configv1.KMSPluginConfig) (bool, error) {
359+
if equality.Semantic.DeepEqual(latest, current) {
360+
return false, nil
361+
}
362+
if latest.Type != configv1.VaultKMSProvider || current.Type != configv1.VaultKMSProvider {
363+
return false, fmt.Errorf("KMS plugin config has an invalid type: %q", latest.Type)
364+
}
365+
if latest.Type != current.Type {
366+
return true, nil
367+
}
368+
if latest.Vault.VaultAddress != current.Vault.VaultAddress ||
369+
latest.Vault.VaultNamespace != current.Vault.VaultNamespace ||
370+
latest.Vault.TransitMount != current.Vault.TransitMount ||
371+
latest.Vault.TransitKey != current.Vault.TransitKey {
372+
return true, nil
373+
}
374+
return false, nil
375+
}
376+
268377
func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, currentMode state.Mode, apiServerEncryption configv1.APIServerEncryption, internalReason, externalReason string) (*corev1.Secret, error) {
269378
bs := crypto.ModeToNewKeyFunc[currentMode]()
270379
ks := state.KeyState{

0 commit comments

Comments
 (0)