Skip to content

CNTRLPLANE-3237: Add new KMS foundational mechanism in encryption controller #2161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Apr 20, 2026
Merged

CNTRLPLANE-3237: Add new KMS foundational mechanism in encryption controller #2161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/operator/encryption/controllers/key_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import (
// greater than the last key's ID (the first key has a key ID of 1).
const (
encryptionSecretMigrationInterval = time.Hour * 24 * 7 // one week
defaultKMSEndpoint = "unix:///var/run/kmsplugin/kms-1.sock"
kmsEndpointFormat = "unix:///var/run/kmsplugin/kms-%d.sock"
defaultKMSTimeout = 10 * time.Second
)

Expand Down Expand Up @@ -271,10 +271,10 @@ func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode,
ExternalReason: externalReason,
}
if currentMode == state.KMS {
ks.KMSConfiguration = &apiserverv1.KMSConfiguration{
ks.KMSEncryptionConfig = &apiserverv1.KMSConfiguration{
APIVersion: "v2",
Name: fmt.Sprintf("%d", keyID),
Endpoint: defaultKMSEndpoint,
Endpoint: fmt.Sprintf(kmsEndpointFormat, keyID),
Timeout: &metav1.Duration{Duration: defaultKMSTimeout},
}
}
Expand Down
34 changes: 17 additions & 17 deletions pkg/operator/encryption/controllers/key_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,13 +351,13 @@ func TestKeyController(t *testing.T) {
ts.Errorf("expected mode to be KMS, got %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"])
}

// Verify KMS config annotation exists
kmsConfig := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config"]
if kmsConfig == "" {
ts.Error("expected kms-config annotation to be present")
// Verify KMS config is in data field
kmsConfigData := actualSecret.Data["encryption.apiserver.operator.openshift.io-kms-encryption-config"]
if len(kmsConfigData) == 0 {
ts.Error("expected kms-encryption-config data to be present")
}
if kmsConfig != `{"apiVersion":"v2","name":"1","endpoint":"unix:///var/run/kmsplugin/kms-1.sock","timeout":"10s"}` {
ts.Errorf("unexpected kms-config: %s", kmsConfig)
if string(kmsConfigData) != `{"apiVersion":"v2","name":"1","endpoint":"unix:///var/run/kmsplugin/kms-1.sock","timeout":"10s"}` {
ts.Errorf("unexpected kms-encryption-config: %s", kmsConfigData)
}

// Verify internal reason
Expand Down Expand Up @@ -413,10 +413,10 @@ func TestKeyController(t *testing.T) {
ts.Errorf("expected mode to be KMS, got %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"])
}

// Verify KMS config annotation exists
kmsConfig := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config"]
if kmsConfig != `{"apiVersion":"v2","name":"6","endpoint":"unix:///var/run/kmsplugin/kms-1.sock","timeout":"10s"}` {
ts.Errorf("unexpected kms-config: %s", kmsConfig)
// Verify KMS config is in data field
kmsConfigData := actualSecret.Data["encryption.apiserver.operator.openshift.io-kms-encryption-config"]
if string(kmsConfigData) != `{"apiVersion":"v2","name":"6","endpoint":"unix:///var/run/kmsplugin/kms-6.sock","timeout":"10s"}` {
ts.Errorf("unexpected kms-encryption-config: %s", kmsConfigData)
}

// Verify internal reason is mode changed
Expand Down Expand Up @@ -488,10 +488,10 @@ func TestKeyController(t *testing.T) {
ts.Errorf("expected mode to be KMS, got %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"])
}

// Verify KMS config annotation exists
kmsConfig := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config"]
if kmsConfig != `{"apiVersion":"v2","name":"6","endpoint":"unix:///var/run/kmsplugin/kms-1.sock","timeout":"10s"}` {
ts.Errorf("unexpected kms-config: %s", kmsConfig)
// Verify KMS config is in data field
kmsConfigData := actualSecret.Data["encryption.apiserver.operator.openshift.io-kms-encryption-config"]
if string(kmsConfigData) != `{"apiVersion":"v2","name":"6","endpoint":"unix:///var/run/kmsplugin/kms-6.sock","timeout":"10s"}` {
ts.Errorf("unexpected kms-encryption-config: %s", kmsConfigData)
}

// Verify internal reason is mode changed
Expand Down Expand Up @@ -538,9 +538,9 @@ func TestKeyController(t *testing.T) {
ts.Errorf("expected mode to be aescbc, got %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"])
}

// Verify KMS config annotation is removed (not present for AESCBC)
if kmsConfig, exists := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config"]; exists {
ts.Errorf("expected kms-config annotation to be absent, got: %s", kmsConfig)
// Verify KMS config data is not present for AESCBC
if kmsConfigData, exists := actualSecret.Data["encryption.apiserver.operator.openshift.io-kms-encryption-config"]; exists {
ts.Errorf("expected kms-encryption-config data to be absent, got: %s", kmsConfigData)
}

// Verify internal reason is mode changed
Expand Down
8 changes: 4 additions & 4 deletions pkg/operator/encryption/controllers/state_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ func TestStateController(t *testing.T) {
KMS: &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: "2_secrets",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: "unix:///var/run/kmsplugin/kms-2.sock",
Timeout: &metav1.Duration{Duration: 10 * time.Second},
},
}, {
Expand All @@ -945,7 +945,7 @@ func TestStateController(t *testing.T) {
KMS: &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: "2_secrets",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: "unix:///var/run/kmsplugin/kms-2.sock",
Timeout: &metav1.Duration{Duration: 10 * time.Second},
},
}, {
Expand All @@ -969,7 +969,7 @@ func TestStateController(t *testing.T) {
KMS: &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: "2_secrets",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: "unix:///var/run/kmsplugin/kms-2.sock",
Timeout: &metav1.Duration{Duration: 10 * time.Second},
},
}, {
Expand Down Expand Up @@ -1072,7 +1072,7 @@ func TestStateController(t *testing.T) {
KMS: &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: "2_secrets",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: "unix:///var/run/kmsplugin/kms-2.sock",
Timeout: &metav1.Duration{Duration: 10 * time.Second},
},
}, {
Expand Down
6 changes: 3 additions & 3 deletions pkg/operator/encryption/encryptionconfig/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,12 @@ func stateToProviders(resource string, desired state.GroupResourceState) []apise
},
})
case state.KMS:
if key.KMSConfiguration == nil {
klog.Infof("skipping key %s for %s in KMS mode as its KMSConfiguration is nil", key.Key.Name, resource)
if key.KMSEncryptionConfig == nil {
klog.Infof("skipping key %s for %s in KMS mode as its KMSEncryptionConfig is nil", key.Key.Name, resource)
continue // this should never happen
}
// In order to preserve the uniqueness, we should insert resource name
kmsCopy := key.KMSConfiguration.DeepCopy()
kmsCopy := key.KMSEncryptionConfig.DeepCopy()
kmsCopy.Name = createKMSProviderName(key.Key.Name, resource)
provider := apiserverconfigv1.ProviderConfiguration{
KMS: kmsCopy,
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/encryption/encryptionconfig/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ func keyToKMSConfiguration(key *corev1.Secret, resource string) *apiserverconfig
return &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: fmt.Sprintf("%d_%s", keyID, resource),
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: fmt.Sprintf("unix:///var/run/kmsplugin/kms-%d.sock", keyID),
Timeout: &metav1.Duration{
Duration: 10 * time.Second,
},
Expand Down
26 changes: 13 additions & 13 deletions pkg/operator/encryption/secrets/secrets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,21 @@ func ToKeyState(s *corev1.Secret) (state.KeyState, error) {
key.ExternalReason = v
}

if v, ok := s.Annotations[EncryptionSecretKMSConfig]; ok && len(v) > 0 {
kmsConfiguration := &apiserverconfigv1.KMSConfiguration{}
if err := json.Unmarshal([]byte(v), kmsConfiguration); err != nil {
return state.KeyState{}, fmt.Errorf("secret %s/%s has invalid %s annotation: %v", s.Namespace, s.Name, EncryptionSecretKMSConfig, err)
}
key.KMSConfiguration = kmsConfiguration
}

keyMode := state.Mode(s.Annotations[encryptionSecretMode])
switch keyMode {
case state.AESCBC, state.AESGCM, state.SecretBox, state.Identity:
key.Mode = keyMode
case state.KMS:
if key.KMSConfiguration == nil {
return state.KeyState{}, fmt.Errorf("KMSConfiguration can not be nil, when mode is KMS")
if v, ok := s.Data[EncryptionSecretKMSEncryptionConfig]; ok && len(v) > 0 {
kmsConfiguration := &apiserverconfigv1.KMSConfiguration{}
if err := json.Unmarshal(v, kmsConfiguration); err != nil {
return state.KeyState{}, fmt.Errorf("secret %s/%s has invalid %s data: %w", s.Namespace, s.Name, EncryptionSecretKMSEncryptionConfig, err)
}
key.KMSEncryptionConfig = kmsConfiguration
} else {
// encryption.apiserver.operator.openshift.io-kms-encryption-config data field is required for KMS
// encryption mode.
return state.KeyState{}, fmt.Errorf("%s can not be empty, when mode is KMS", EncryptionSecretKMSEncryptionConfig)
}
key.Mode = keyMode
default:
Expand Down Expand Up @@ -126,12 +126,12 @@ func FromKeyState(component string, ks state.KeyState) (*corev1.Secret, error) {
s.Annotations[EncryptionSecretMigratedResources] = string(bs)
}

if ks.KMSConfiguration != nil {
ksJSON, err := json.Marshal(ks.KMSConfiguration)
if ks.KMSEncryptionConfig != nil {
kmsEncCfgJSON, err := json.Marshal(ks.KMSEncryptionConfig)
if err != nil {
return nil, err
}
s.Annotations[EncryptionSecretKMSConfig] = string(ksJSON)
s.Data[EncryptionSecretKMSEncryptionConfig] = kmsEncCfgJSON
}

return s, nil
Expand Down
6 changes: 3 additions & 3 deletions pkg/operator/encryption/secrets/secrets_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func TestRoundtrip(t *testing.T) {
},
Backed: true,
Mode: "KMS",
KMSConfiguration: &v1.KMSConfiguration{
KMSEncryptionConfig: &v1.KMSConfiguration{
APIVersion: "v2",
Name: "1",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Expand All @@ -150,10 +150,10 @@ func TestRoundtrip(t *testing.T) {
},
Backed: true,
Mode: "KMS",
KMSConfiguration: &v1.KMSConfiguration{
KMSEncryptionConfig: &v1.KMSConfiguration{
APIVersion: "v2",
Name: "2",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: "unix:///var/run/kmsplugin/kms-2.sock",
},
},
},
Expand Down
5 changes: 3 additions & 2 deletions pkg/operator/encryption/secrets/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ const (
// deletion of secrets by enforcing a two phase delete.
EncryptionSecretFinalizer = "encryption.apiserver.operator.openshift.io/deletion-protection"

// EncryptionSecretKMSConfig is the annotation that stores the encoded KMS configuration.
EncryptionSecretKMSConfig = "encryption.apiserver.operator.openshift.io/kms-config"
// EncryptionSecretKMSEncryptionConfig is the data field key that stores the serialized KMS
// encryption configuration for KMS mode in the encryption-key secret.
EncryptionSecretKMSEncryptionConfig = "encryption.apiserver.operator.openshift.io-kms-encryption-config"

@p0lyn0mial p0lyn0mial Apr 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be kms-encryption-config ?

@ardaguclu ardaguclu Apr 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All annotations and data fields in this area uses encryption.apiserver.operator.openshift. prefix, I think we should still have it to distinguish.

@ardaguclu ardaguclu Apr 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a flexibility to just use kms-encrypyion-config as data key, though.

@p0lyn0mial p0lyn0mial Apr 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if that is the convention then we should stick to it. thanks.

@p0lyn0mial p0lyn0mial Apr 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not encryption.apiserver.operator.openshift.io/kms-encryption-config ?

@ardaguclu ardaguclu Apr 20, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to use it and got an error. After the investigations, I found out the reason https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/api/core/v1/types.go#L7949-L7954 that secret data key can not contain / character.

library-go/pkg/operator/encryption/secrets/types.go

Line 46 in e2aa54b

EncryptionSecretKeyDataKey = "encryption.apiserver.operator.openshift.io-key"
is in the same format due to this reason.

)

// MigratedGroupResources is the data structured stored in the
Expand Down
4 changes: 2 additions & 2 deletions pkg/operator/encryption/state/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ type KeyState struct {
InternalReason string
// the user via unsupportConfigOverrides.encryption.reason triggered this key.
ExternalReason string
// Encoded KMSConfiguration that stores the KMS related fields
KMSConfiguration *apiserverconfigv1.KMSConfiguration
// Encoded KMSEncryptionConfig that stores the KMS related fields
KMSEncryptionConfig *apiserverconfigv1.KMSConfiguration
}

type MigrationState struct {
Expand Down
14 changes: 7 additions & 7 deletions pkg/operator/encryption/statemachine/transition_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,7 @@ func TestGetDesiredEncryptionState(t *testing.T) {
KMS: &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: "2_secrets",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: "unix:///var/run/kmsplugin/kms-2.sock",
Timeout: &metav1.Duration{Duration: 10 * time.Second},
},
}, {
Expand All @@ -1079,7 +1079,7 @@ func TestGetDesiredEncryptionState(t *testing.T) {
KMS: &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: "2_secrets",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: "unix:///var/run/kmsplugin/kms-2.sock",
Timeout: &metav1.Duration{Duration: 10 * time.Second},
},
}, {
Expand All @@ -1102,7 +1102,7 @@ func TestGetDesiredEncryptionState(t *testing.T) {
KMS: &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: "2_secrets",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: "unix:///var/run/kmsplugin/kms-2.sock",
Timeout: &metav1.Duration{Duration: 10 * time.Second},
},
}, {
Expand All @@ -1129,7 +1129,7 @@ func TestGetDesiredEncryptionState(t *testing.T) {
KMS: &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: "2_secrets",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: "unix:///var/run/kmsplugin/kms-2.sock",
Timeout: &metav1.Duration{Duration: 10 * time.Second},
},
}, {
Expand Down Expand Up @@ -1158,7 +1158,7 @@ func TestGetDesiredEncryptionState(t *testing.T) {
KMS: &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: "2_secrets",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: "unix:///var/run/kmsplugin/kms-2.sock",
Timeout: &metav1.Duration{Duration: 10 * time.Second},
},
}, {
Expand All @@ -1184,7 +1184,7 @@ func TestGetDesiredEncryptionState(t *testing.T) {
KMS: &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: "2_secrets",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: "unix:///var/run/kmsplugin/kms-2.sock",
Timeout: &metav1.Duration{Duration: 10 * time.Second},
},
}, {
Expand Down Expand Up @@ -1213,7 +1213,7 @@ func TestGetDesiredEncryptionState(t *testing.T) {
KMS: &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: "2_secrets",
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: "unix:///var/run/kmsplugin/kms-2.sock",
Timeout: &metav1.Duration{Duration: 10 * time.Second},
},
}, {
Expand Down
14 changes: 7 additions & 7 deletions pkg/operator/encryption/testing/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ import (
)

const (
encryptionSecretKeyDataForTest = "encryption.apiserver.operator.openshift.io-key"
encryptionSecretMigratedTimestampForTest = "encryption.apiserver.operator.openshift.io/migrated-timestamp"
encryptionSecretMigratedResourcesForTest = "encryption.apiserver.operator.openshift.io/migrated-resources"
encryptionSecretKMSConfigForTest = "encryption.apiserver.operator.openshift.io/kms-config"
encryptionSecretKeyDataForTest = "encryption.apiserver.operator.openshift.io-key"
encryptionSecretMigratedTimestampForTest = "encryption.apiserver.operator.openshift.io/migrated-timestamp"
encryptionSecretMigratedResourcesForTest = "encryption.apiserver.operator.openshift.io/migrated-resources"
encryptionSecretKMSEncryptionConfigForTest = "encryption.apiserver.operator.openshift.io-kms-encryption-config"

@p0lyn0mial p0lyn0mial Apr 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be kms-encryption-config ?

@ardaguclu ardaguclu Apr 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #2161 (comment)

)

func CreateEncryptionKeySecretNoData(targetNS string, grs []schema.GroupResource, keyID uint64) *corev1.Secret {
Expand Down Expand Up @@ -101,11 +101,11 @@ func CreateEncryptionKeySecretWithKMSConfig(targetNS string, grs []schema.GroupR
kmsConfig := &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: fmt.Sprintf("%d", keyID),
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: fmt.Sprintf("unix:///var/run/kmsplugin/kms-%d.sock", keyID),
Timeout: &metav1.Duration{Duration: 10 * time.Second},
}
kmsConfigJSON, _ := json.Marshal(kmsConfig)
secret.Annotations[encryptionSecretKMSConfigForTest] = string(kmsConfigJSON)
secret.Data[encryptionSecretKMSEncryptionConfigForTest] = kmsConfigJSON
return secret
}

Expand Down Expand Up @@ -273,7 +273,7 @@ func createProviderCfg(mode string, resource string, key apiserverconfigv1.Key)
KMS: &apiserverconfigv1.KMSConfiguration{
APIVersion: "v2",
Name: fmt.Sprintf("%s_%s", key.Name, resource),
Endpoint: "unix:///var/run/kmsplugin/kms-1.sock",
Endpoint: fmt.Sprintf("unix:///var/run/kmsplugin/kms-%s.sock", key.Name),
Timeout: &metav1.Duration{Duration: 10 * time.Second},
},
}
Expand Down