Introduce kms to kms migration

Author: ardaguclu

pkg/operator/encryption/controllers/key_controller.go

@@ -201,7 +201,7 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact
 
 	var commonReason *string
 	for gr, grKeys := range desiredEncryptionState {
-		latestKeyID, internalReason, needed := needsNewKey(grKeys, currentMode, externalReason, encryptedGRs)
+		latestKeyID, internalReason, needed := needsNewKey(grKeys, currentMode, externalReason, encryptedGRs, apiEncryptionConfiguration)
 		if !needed {
 			continue
 		}
@@ -265,6 +265,63 @@ func (c *keyController) validateExistingSecret(ctx context.Context, keySecret *c
 	return nil // we made this key earlier
 }
 
+// kmsProviderConfig abstracts provider-specific KMS logic so that every
+// provider-type switch lives in a single factory (newKMSProviderConfig).
+type kmsProviderConfig interface {
+	referencedSecretName() (string, []string, error)
+	referencedConfigMapName() (string, []string, error)
+	// migrationRequired reports whether switching from latest (stored in
+	// the key secret) to this provider config requires a new encryption key.
+	migrationRequired(latest configv1.KMSPluginConfig) bool
+}
+
+func newKMSProviderConfig(plugin configv1.KMSPluginConfig) (kmsProviderConfig, error) {
+	switch plugin.Type {
+	case configv1.VaultKMSProvider:
+		return &vaultProviderConfig{plugin.Vault}, nil
+	default:
+		return nil, fmt.Errorf("unsupported KMS provider type %q", plugin.Type)
+	}
+}
+
+type vaultProviderConfig struct {
+	vault configv1.VaultKMSPluginConfig
+}
+
+// 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 (v *vaultProviderConfig) referencedSecretName() (string, []string, error) {
+	switch v.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 v.vault.Authentication.AppRole.Secret.Name, []string{"role-id", "secret-id"}, nil
+	default:
+		return "", nil, fmt.Errorf("unsupported Vault authentication type %q", v.vault.Authentication.Type)
+	}
+}
+
+// referencedConfigMapName returns the name of the configmap referenced by the KMS plugin
+// config and the specific data keys to carry from that configmap. Only the listed keys
+// are copied into the Key Secret; any other data in the referenced configmap is ignored.
+func (v *vaultProviderConfig) referencedConfigMapName() (string, []string, error) {
+	if v.vault.TLS.CABundle.Name == "" {
+		return "", nil, nil
+	}
+	return v.vault.TLS.CABundle.Name, []string{"ca-bundle.crt"}, nil
+}
+
+func (v *vaultProviderConfig) migrationRequired(latest configv1.KMSPluginConfig) bool {
+	if latest.Type != configv1.VaultKMSProvider {
+		return true
+	}
+	return v.vault.VaultAddress != latest.Vault.VaultAddress ||
+		v.vault.VaultNamespace != latest.Vault.VaultNamespace ||
+		v.vault.TransitMount != latest.Vault.TransitMount ||
+		v.vault.TransitKey != latest.Vault.TransitKey
+}
+
 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{
@@ -287,7 +344,12 @@ func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, cur
 			Plugin: apiServerEncryption.KMS,
 		}
 
-		if secretName, expectedKeys, err := referencedSecretName(apiServerEncryption.KMS); err != nil {
+		providerCfg, err := newKMSProviderConfig(apiServerEncryption.KMS)
+		if err != nil {
+			return nil, err
+		}
+
+		if secretName, expectedKeys, err := providerCfg.referencedSecretName(); err != nil {
 			return nil, err
 		} else if len(secretName) > 0 {
 			refSecret, err := c.secretClient.Secrets(openshiftConfigNS).Get(ctx, secretName, metav1.GetOptions{})
@@ -305,7 +367,7 @@ func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, cur
 			}
 		}
 
-		if cmName, expectedKeys, err := referencedConfigMapName(apiServerEncryption.KMS); err != nil {
+		if cmName, expectedKeys, err := providerCfg.referencedConfigMapName(); err != nil {
 			return nil, err
 		} else if len(cmName) > 0 {
 			refCM, err := c.configMapClient.ConfigMaps(openshiftConfigNS).Get(ctx, cmName, metav1.GetOptions{})
@@ -358,7 +420,7 @@ func (c *keyController) getCurrentModeReasonAndEncryptionConfig(ctx context.Cont
 
 // needsNewKey checks whether a new key must be created for the given resource. If true, it also returns the latest
 // used key ID and a reason string.
-func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, externalReason string, encryptedGRs []schema.GroupResource) (uint64, string, bool) {
+func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, externalReason string, encryptedGRs []schema.GroupResource, apiServerEncryption configv1.APIServerEncryption) (uint64, string, bool) {
 	// we always need to have some encryption keys unless we are turned off
 	if len(grKeys.ReadKeys) == 0 {
 		return 0, "key-does-not-exist", currentMode != state.Identity
@@ -403,13 +465,18 @@ func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, extern
 
 	if currentMode == state.KMS {
 		// We are here because Encryption Mode is not changed
+		// However, we need to create a new key if migration-triggering fields
+		// in the KMS provider configuration have changed.
+		currentProviderCfg, err := newKMSProviderConfig(apiServerEncryption.KMS)
+		if err != nil {
+			return latestKeyID, fmt.Sprintf("unsupported-kms-provider-%s", apiServerEncryption.KMS.Type), true
+		}
+		if currentProviderCfg.migrationRequired(latestKey.KMS.Plugin) {
+			return latestKeyID, "kms-provider-changed", true
+		}
 
-		// For now in Tech Preview v1, we don't support configurational changes. Therefore,
-		// it is pointless comparing the secrets.
-
-		// For KMS mode, we don't do time-based rotation. Therefore, we shortcut here
-		// KMS keys are rotated externally by the KMS system.
-		// Moreover, we don't trigger new key when external reason is changed.
+		// For KMS mode, we don't do time-based rotation. KMS keys are rotated
+		// externally by the KMS provider. Moreover, we don't trigger new key when external reason is changed.
 		// Because it would lead to duplicate providers which is not allowed.
 		return 0, "", false
 	}
@@ -424,40 +491,6 @@ 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)
-	}
-}
-
-// referencedConfigMapName returns the name of the configmap referenced by the KMS plugin
-// config and the specific data keys to carry from that configmap. Only the listed keys
-// are copied into the Key Secret; any other data in the referenced configmap is ignored.
-func referencedConfigMapName(plugin configv1.KMSPluginConfig) (string, []string, error) {
-	switch plugin.Type {
-	case configv1.VaultKMSProvider:
-		if plugin.Vault.TLS.CABundle.Name == "" {
-			return "", nil, nil
-		}
-		return plugin.Vault.TLS.CABundle.Name, []string{"ca-bundle.crt"}, nil
-	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 {