Skip to content

CNTRLPLANE-3237:: pkg/operator/encryption/kms: cleanup #2221

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 2 commits into from
May 21, 2026
Merged

CNTRLPLANE-3237:: pkg/operator/encryption/kms: cleanup #2221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7388263
pkg/operator/encryption/kms: cleanup
p0lyn0mial May 21, 2026
d617f82
pkg/operator/encryption/kms: replace require calls with standard test…
p0lyn0mial May 21, 2026
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
30 changes: 27 additions & 3 deletions pkg/operator/encryption/encryptiondata/secret.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"sort"
"strconv"
"strings"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
Expand All @@ -13,10 +14,33 @@ import (
configv1 "github.com/openshift/api/config/v1"

"github.com/openshift/library-go/pkg/operator/encryption/encoding"
"github.com/openshift/library-go/pkg/operator/encryption/kms"
"github.com/openshift/library-go/pkg/operator/encryption/state"
)

const pluginConfigDataKeyPrefix = "kms-plugin-config-"

// toPluginConfigSecretDataKeyFor constructs the data key for storing a KMS plugin config in the encryption-config Secret.
// The keyID must be a valid non-negative integer string.
func toPluginConfigSecretDataKeyFor(keyID string) (string, error) {
if _, err := strconv.ParseUint(keyID, 10, 64); err != nil {
return "", fmt.Errorf("invalid keyID %q: must be a non-negative integer", keyID)
}
return pluginConfigDataKeyPrefix + keyID, nil
}

// keyIDFromPluginConfigSecretDataKey extracts the keyID from a kms-plugin-config data key.
// Returns the keyID and true if the key matches the "kms-plugin-config-<keyID>" pattern.
func keyIDFromPluginConfigSecretDataKey(dataKey string) (string, bool, error) {
keyID, found := strings.CutPrefix(dataKey, pluginConfigDataKeyPrefix)
if !found || len(keyID) == 0 {
return "", false, nil
}
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, true, nil
}

// EncryptionConfSecretName is the name of the final encryption config secret that is revisioned per apiserver rollout.
const EncryptionConfSecretName = "encryption-config"

Expand All @@ -36,7 +60,7 @@ func FromSecret(encryptionConfigSecret *corev1.Secret) (*Config, error) {
for key, value := range encryptionConfigSecret.Data {
// Not all data keys are plugin configs — the Secret also contains the
// encryption-config entry, so skip keys that don't match the pattern.
keyID, found, err := kms.KeyIDFromPluginConfigSecretDataKey(key)
keyID, found, err := keyIDFromPluginConfigSecretDataKey(key)
if err != nil {
return nil, fmt.Errorf("failed to extract keyID from data key %s: %w", key, err)
}
Expand Down Expand Up @@ -93,7 +117,7 @@ func ToSecret(ns, name string, secretData *Config) (*corev1.Secret, error) {
if err != nil {
return nil, fmt.Errorf("failed to encode KMS plugin config for key %s: %w", keyID, err)
}
dataKey, err := kms.ToPluginConfigSecretDataKeyFor(keyID)
dataKey, err := toPluginConfigSecretDataKeyFor(keyID)
if err != nil {
return nil, err
}
Expand Down
138 changes: 125 additions & 13 deletions pkg/operator/encryption/encryptiondata/secret_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package encryptiondata_test
package encryptiondata
Copy link
Copy Markdown
Member

@bertinatto bertinatto May 21, 2026

Choose a reason for hiding this comment

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

+1


import (
"testing"
Expand All @@ -7,16 +7,14 @@ import (
"github.com/google/go-cmp/cmp"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
apiserverconfigv1 "k8s.io/apiserver/pkg/apis/apiserver/v1"

"github.com/openshift/library-go/pkg/operator/encryption/encryptiondata"
)

func TestExtractUniqueAndSortedKMSConfigurations(t *testing.T) {
timeout := &metav1.Duration{Duration: 10 * time.Second}

tests := []struct {
name string
cfg *encryptiondata.Config
cfg *Config
want []*apiserverconfigv1.KMSConfiguration
wantError bool
}{
Expand All @@ -27,12 +25,12 @@ func TestExtractUniqueAndSortedKMSConfigurations(t *testing.T) {
},
{
name: "nil encryption returns error",
cfg: &encryptiondata.Config{},
cfg: &Config{},
wantError: true,
},
{
name: "empty provider list returns empty slice",
cfg: &encryptiondata.Config{
cfg: &Config{
Encryption: &apiserverconfigv1.EncryptionConfiguration{
Resources: []apiserverconfigv1.ResourceConfiguration{{
Resources: []string{"secrets"},
Expand All @@ -44,7 +42,7 @@ func TestExtractUniqueAndSortedKMSConfigurations(t *testing.T) {
},
{
name: "single resource single KMS provider",
cfg: &encryptiondata.Config{
cfg: &Config{
Encryption: &apiserverconfigv1.EncryptionConfiguration{
Resources: []apiserverconfigv1.ResourceConfiguration{{
Resources: []string{"secrets"},
Expand All @@ -68,7 +66,7 @@ func TestExtractUniqueAndSortedKMSConfigurations(t *testing.T) {
},
{
name: "same keyID across resources is deduplicated",
cfg: &encryptiondata.Config{
cfg: &Config{
Encryption: &apiserverconfigv1.EncryptionConfiguration{
Resources: []apiserverconfigv1.ResourceConfiguration{
{
Expand Down Expand Up @@ -105,7 +103,7 @@ func TestExtractUniqueAndSortedKMSConfigurations(t *testing.T) {
},
{
name: "multiple keyIDs sorted descending",
cfg: &encryptiondata.Config{
cfg: &Config{
Encryption: &apiserverconfigv1.EncryptionConfiguration{
Resources: []apiserverconfigv1.ResourceConfiguration{{
Resources: []string{"secrets"},
Expand Down Expand Up @@ -146,7 +144,7 @@ func TestExtractUniqueAndSortedKMSConfigurations(t *testing.T) {
},
{
name: "non-KMS providers are skipped",
cfg: &encryptiondata.Config{
cfg: &Config{
Encryption: &apiserverconfigv1.EncryptionConfiguration{
Resources: []apiserverconfigv1.ResourceConfiguration{{
Resources: []string{"secrets"},
Expand Down Expand Up @@ -174,7 +172,7 @@ func TestExtractUniqueAndSortedKMSConfigurations(t *testing.T) {
},
{
name: "mismatched duplicate keyID errors",
cfg: &encryptiondata.Config{
cfg: &Config{
Encryption: &apiserverconfigv1.EncryptionConfiguration{
Resources: []apiserverconfigv1.ResourceConfiguration{
{
Expand Down Expand Up @@ -206,7 +204,7 @@ func TestExtractUniqueAndSortedKMSConfigurations(t *testing.T) {
},
{
name: "invalid plugin name errors",
cfg: &encryptiondata.Config{
cfg: &Config{
Encryption: &apiserverconfigv1.EncryptionConfiguration{
Resources: []apiserverconfigv1.ResourceConfiguration{{
Resources: []string{"secrets"},
Expand All @@ -226,7 +224,7 @@ func TestExtractUniqueAndSortedKMSConfigurations(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := encryptiondata.ExtractUniqueAndSortedKMSConfigurations(tt.cfg)
got, err := ExtractUniqueAndSortedKMSConfigurations(tt.cfg)
if tt.wantError {
if err == nil {
t.Fatal("expected error but got nil")
Expand All @@ -242,3 +240,117 @@ func TestExtractUniqueAndSortedKMSConfigurations(t *testing.T) {
})
}
}

func TestKeyIDFromPluginConfigSecretDataKey(t *testing.T) {
tests := []struct {
name string
dataKey string
wantKeyID string
wantFound bool
wantError bool
}{
{
name: "valid key",
dataKey: "kms-plugin-config-1",
wantKeyID: "1",
wantFound: true,
},
{
name: "valid key with large ID",
dataKey: "kms-plugin-config-42",
wantKeyID: "42",
wantFound: true,
},
{
name: "encryption-config key",
dataKey: "encryption-config",
},
{
name: "non-integer keyID",
dataKey: "kms-plugin-config-abc",
wantError: true,
},
{
name: "missing keyID",
dataKey: "kms-plugin-config-",
},
{
name: "empty string",
dataKey: "",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
keyID, found, err := keyIDFromPluginConfigSecretDataKey(tt.dataKey)
if tt.wantError {
if err == nil {
t.Fatal("expected error but got nil")
}
return
}
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if found != tt.wantFound {
t.Fatalf("expected found=%v, got %v", tt.wantFound, found)
}
if found && keyID != tt.wantKeyID {
t.Fatalf("expected keyID=%q, got %q", tt.wantKeyID, keyID)
}
})
}
}

func TestToPluginConfigSecretDataKeyFor(t *testing.T) {
tests := []struct {
name string
keyID string
wantKey string
wantError bool
}{
{
name: "valid keyID",
keyID: "1",
wantKey: "kms-plugin-config-1",
},
{
name: "valid large keyID",
keyID: "42",
wantKey: "kms-plugin-config-42",
},
{
name: "non-integer keyID",
keyID: "abc",
wantError: true,
},
{
name: "empty keyID",
keyID: "",
wantError: true,
},
{
name: "negative keyID",
keyID: "-1",
wantError: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := toPluginConfigSecretDataKeyFor(tt.keyID)
if tt.wantError {
if err == nil {
t.Fatal("expected error but got nil")
}
return
}
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got != tt.wantKey {
t.Fatalf("expected key=%q, got %q", tt.wantKey, got)
}
})
}
}
26 changes: 0 additions & 26 deletions pkg/operator/encryption/kms/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,12 @@ package kms

import (
"fmt"
"strconv"
"strings"

"github.com/openshift/api/features"
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
corev1 "k8s.io/api/core/v1"
)

const pluginConfigDataKeyPrefix = "kms-plugin-config-"

// ToPluginConfigSecretDataKeyFor constructs the data key for storing a KMS plugin config in the encryption-config Secret.
// The keyID must be a valid non-negative integer string.
func ToPluginConfigSecretDataKeyFor(keyID string) (string, error) {
if _, err := strconv.ParseUint(keyID, 10, 64); err != nil {
return "", fmt.Errorf("invalid keyID %q: must be a non-negative integer", keyID)
}
return pluginConfigDataKeyPrefix + keyID, nil
}

// KeyIDFromPluginConfigSecretDataKey extracts the keyID from a kms-plugin-config data key.
// Returns the keyID and true if the key matches the "kms-plugin-config-<keyID>" pattern.
func KeyIDFromPluginConfigSecretDataKey(dataKey string) (string, bool, error) {
keyID, found := strings.CutPrefix(dataKey, pluginConfigDataKeyPrefix)
if !found || len(keyID) == 0 {
return "", false, nil
}
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, true, nil
}

// AddKMSPluginVolumeAndMountToPodSpec conditionally adds the KMS plugin volume mount to the specified container.
// It assumes the pod spec does not already contain the KMS volume or mount; no deduplication is performed.
// Deprecated: this is a temporary solution to get KMS TP v1 out. We should come up with a different approach afterwards.
Expand Down
Loading