Skip to content

Commit 34f1117

Browse files
ardagucluardaguclu
committed
Mock KMS status call and add extensive KMS tests
1 parent 15729b4 commit 34f1117

File tree

2 files changed

+266
-2
lines changed

2 files changed

+266
-2
lines changed

‎pkg/operator/encryption/controllers/key_controller.go‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ import (
4141
// greater than the last key's ID (the first key has a key ID of 1).
4242
const encryptionSecretMigrationInterval = time.Hour * 24 * 7 // one week
4343

44+
// kmsHashesGetter is a function type for getting KMS config and key ID hashes
45+
type kmsHashesGetter func(ctx context.Context, kmsConfig *configv1.KMSConfig) (configHash string, keyIDHash string, err error)
46+
47+
var kmsHashesGetterFunc kmsHashesGetter
48+
4449
// keyController creates new keys if necessary. It
4550
// * watches
4651
// - secrets in openshift-config-managed
@@ -106,6 +111,8 @@ func NewKeyController(
106111
secretClient: secretClient,
107112
}
108113

114+
kmsHashesGetterFunc = defaultGetKMSHashes
115+
109116
return factory.New().
110117
WithSync(c.sync).
111118
WithControllerInstanceName(c.controllerInstanceName).
@@ -169,7 +176,7 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact
169176
// Compute KMS hashes if using KMS mode
170177
var kmsConfigHash, kmsKeyIDHash string
171178
if currentMode == state.KMS && kmsConfig != nil {
172-
kmsConfigHash, kmsKeyIDHash, err = c.getKMSHashes(ctx, kmsConfig)
179+
kmsConfigHash, kmsKeyIDHash, err = kmsHashesGetterFunc(ctx, kmsConfig)
173180
if err != nil {
174181
return err
175182
}
@@ -311,7 +318,9 @@ func (c *keyController) getCurrentModeAndExternalReason(ctx context.Context) (st
311318
}
312319
}
313320

314-
func (c *keyController) getKMSHashes(ctx context.Context, kmsConfig *configv1.KMSConfig) (string, string, error) {
321+
// defaultGetKMSHashes is the default implementation of getting KMS hashes
322+
// It calls the real KMS client to get the status and compute hashes
323+
func defaultGetKMSHashes(ctx context.Context, kmsConfig *configv1.KMSConfig) (string, string, error) {
315324
// Generate unix socket path from KMS config and get the hash
316325
socketPath, configHash, err := kms.GenerateUnixSocketPath(kmsConfig)
317326
if err != nil {

‎pkg/operator/encryption/controllers/key_controller_test.go‎

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ func TestKeyController(t *testing.T) {
5353
validateFunc func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource)
5454
validateOperatorClientFunc func(ts *testing.T, operatorClient v1helpers.OperatorClient)
5555
expectedError error
56+
kmsConfigHash string
57+
kmsKeyIDHash string
5658
}{
5759
{
5860
name: "no apiservers config",
@@ -324,6 +326,253 @@ func TestKeyController(t *testing.T) {
324326
}
325327
},
326328
},
329+
330+
// KMS mode test cases
331+
{
332+
name: "KMS: creates first encryption key when none exists",
333+
targetGRs: []schema.GroupResource{
334+
{Group: "", Resource: "secrets"},
335+
},
336+
kmsConfigHash: "config-hash-12345678",
337+
kmsKeyIDHash: "key-id-hash-abcdefgh",
338+
initialObjects: []runtime.Object{
339+
encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
340+
},
341+
apiServerObjects: []runtime.Object{
342+
func() *configv1.APIServer {
343+
apiServer := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}}
344+
apiServer.Spec.Encryption = configv1.APIServerEncryption{
345+
Type: "kms",
346+
KMS: &configv1.KMSConfig{
347+
Type: configv1.AWSKMSProvider,
348+
AWS: &configv1.AWSKMSConfig{
349+
KeyARN: "arn:aws:kms:us-east-1:123456789012:key/test-key",
350+
Region: "us-east-1",
351+
},
352+
},
353+
}
354+
return apiServer
355+
}(),
356+
},
357+
targetNamespace: "kms",
358+
expectedActions: []string{
359+
"list:pods:kms",
360+
"get:secrets:kms",
361+
"list:secrets:openshift-config-managed",
362+
"create:secrets:openshift-config-managed",
363+
"create:events:kms",
364+
},
365+
validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) {
366+
wasSecretValidated := false
367+
for _, action := range actions {
368+
if action.Matches("create", "secrets") {
369+
createAction := action.(clientgotesting.CreateAction)
370+
actualSecret := createAction.GetObject().(*corev1.Secret)
371+
372+
// Verify KMS annotations are set
373+
if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] == "" {
374+
ts.Error("expected KMS config hash annotation to be set")
375+
}
376+
if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] == "" {
377+
ts.Error("expected KMS key ID hash annotation to be set")
378+
}
379+
if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"] != "kms" {
380+
ts.Errorf("expected mode to be 'kms', got '%s'", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"])
381+
}
382+
383+
wasSecretValidated = true
384+
break
385+
}
386+
}
387+
if !wasSecretValidated {
388+
ts.Errorf("the secret wasn't created and validated")
389+
}
390+
},
391+
},
392+
393+
{
394+
name: "KMS: creates new key when KMS config hash changes",
395+
targetGRs: []schema.GroupResource{
396+
{Group: "", Resource: "secrets"},
397+
},
398+
kmsConfigHash: "new-config-hash-xyz", // Different hash for new key ARN
399+
kmsKeyIDHash: "key-id-hash-abcd1234",
400+
initialObjects: []runtime.Object{
401+
encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
402+
func() *corev1.Secret {
403+
s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, "kms")
404+
s.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] = "old-config-hash-1234"
405+
s.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] = "key-id-hash-abcd1234"
406+
return s
407+
}(),
408+
},
409+
apiServerObjects: []runtime.Object{
410+
func() *configv1.APIServer {
411+
apiServer := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}}
412+
apiServer.Spec.Encryption = configv1.APIServerEncryption{
413+
Type: "kms",
414+
KMS: &configv1.KMSConfig{
415+
Type: configv1.AWSKMSProvider,
416+
AWS: &configv1.AWSKMSConfig{
417+
KeyARN: "arn:aws:kms:us-east-1:123456789012:key/new-key", // Different key
418+
Region: "us-east-1",
419+
},
420+
},
421+
}
422+
return apiServer
423+
}(),
424+
},
425+
targetNamespace: "kms",
426+
expectedActions: []string{
427+
"list:pods:kms",
428+
"get:secrets:kms",
429+
"list:secrets:openshift-config-managed",
430+
"create:secrets:openshift-config-managed",
431+
"create:events:kms",
432+
},
433+
validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) {
434+
wasSecretValidated := false
435+
for _, action := range actions {
436+
if action.Matches("create", "secrets") {
437+
createAction := action.(clientgotesting.CreateAction)
438+
actualSecret := createAction.GetObject().(*corev1.Secret)
439+
440+
// Verify new KMS config hash is set and different from old
441+
newConfigHash := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"]
442+
if newConfigHash == "" || newConfigHash == "old-config-hash-1234" {
443+
ts.Errorf("expected new KMS config hash, got '%s'", newConfigHash)
444+
}
445+
446+
// Verify internal reason mentions config change
447+
internalReason := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"]
448+
if internalReason != "secrets-kms-config-changed" {
449+
ts.Errorf("expected internal reason 'secrets-kms-config-changed', got '%s'", internalReason)
450+
}
451+
452+
wasSecretValidated = true
453+
break
454+
}
455+
}
456+
if !wasSecretValidated {
457+
ts.Errorf("the secret wasn't created and validated")
458+
}
459+
},
460+
},
461+
462+
{
463+
name: "KMS: creates new key when KMS key ID hash changes (key rotation)",
464+
targetGRs: []schema.GroupResource{
465+
{Group: "", Resource: "secrets"},
466+
},
467+
kmsConfigHash: "config-hash-12345678",
468+
kmsKeyIDHash: "new-key-id-hash-xyz",
469+
initialObjects: []runtime.Object{
470+
encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
471+
func() *corev1.Secret {
472+
s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, "kms")
473+
s.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] = "config-hash-12345678"
474+
s.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] = "old-key-id-hash-abc"
475+
return s
476+
}(),
477+
},
478+
apiServerObjects: []runtime.Object{
479+
func() *configv1.APIServer {
480+
apiServer := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}}
481+
apiServer.Spec.Encryption = configv1.APIServerEncryption{
482+
Type: "kms",
483+
KMS: &configv1.KMSConfig{
484+
Type: configv1.AWSKMSProvider,
485+
AWS: &configv1.AWSKMSConfig{
486+
KeyARN: "arn:aws:kms:us-east-1:123456789012:key/test-key",
487+
Region: "us-east-1",
488+
},
489+
},
490+
}
491+
return apiServer
492+
}(),
493+
},
494+
targetNamespace: "kms",
495+
expectedActions: []string{
496+
"list:pods:kms",
497+
"get:secrets:kms",
498+
"list:secrets:openshift-config-managed",
499+
"create:secrets:openshift-config-managed",
500+
"create:events:kms",
501+
},
502+
validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) {
503+
wasSecretValidated := false
504+
for _, action := range actions {
505+
if action.Matches("create", "secrets") {
506+
createAction := action.(clientgotesting.CreateAction)
507+
actualSecret := createAction.GetObject().(*corev1.Secret)
508+
509+
// Verify new KMS key ID hash is set and different from old
510+
newKeyIDHash := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"]
511+
if newKeyIDHash == "" || newKeyIDHash == "old-key-id-hash-abc" {
512+
ts.Errorf("expected new KMS key ID hash, got '%s'", newKeyIDHash)
513+
}
514+
515+
// Verify config hash stays the same
516+
configHash := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"]
517+
if configHash != "config-hash-12345678" {
518+
ts.Errorf("expected config hash to remain 'config-hash-12345678', got '%s'", configHash)
519+
}
520+
521+
// Verify internal reason mentions key ID change
522+
internalReason := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"]
523+
if internalReason != "secrets-kms-key-id-changed" {
524+
ts.Errorf("expected internal reason 'secrets-kms-key-id-changed', got '%s'", internalReason)
525+
}
526+
527+
wasSecretValidated = true
528+
break
529+
}
530+
}
531+
if !wasSecretValidated {
532+
ts.Errorf("the secret wasn't created and validated")
533+
}
534+
},
535+
},
536+
537+
{
538+
name: "KMS: no-op when hashes match and key is migrated",
539+
targetGRs: []schema.GroupResource{
540+
{Group: "", Resource: "secrets"},
541+
},
542+
kmsConfigHash: "config-hash-12345678",
543+
kmsKeyIDHash: "key-id-hash-abcdefgh",
544+
initialObjects: []runtime.Object{
545+
encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
546+
func() *corev1.Secret {
547+
s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, "kms")
548+
s.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] = "config-hash-12345678"
549+
s.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] = "key-id-hash-abcdefgh"
550+
return s
551+
}(),
552+
},
553+
apiServerObjects: []runtime.Object{
554+
func() *configv1.APIServer {
555+
apiServer := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}}
556+
apiServer.Spec.Encryption = configv1.APIServerEncryption{
557+
Type: "kms",
558+
KMS: &configv1.KMSConfig{
559+
Type: configv1.AWSKMSProvider,
560+
AWS: &configv1.AWSKMSConfig{
561+
KeyARN: "arn:aws:kms:us-east-1:123456789012:key/test-key",
562+
Region: "us-east-1",
563+
},
564+
},
565+
}
566+
return apiServer
567+
}(),
568+
},
569+
targetNamespace: "kms",
570+
expectedActions: []string{
571+
"list:pods:kms",
572+
"get:secrets:kms",
573+
"list:secrets:openshift-config-managed",
574+
},
575+
},
327576
}
328577

329578
for _, scenario := range scenarios {
@@ -375,6 +624,12 @@ func TestKeyController(t *testing.T) {
375624

376625
target := NewKeyController(scenario.targetNamespace, nil, provider, deployer, alwaysFulfilledPreconditions, fakeOperatorClient, fakeApiServerClient, fakeApiServerInformer, kubeInformers, fakeSecretClient, scenario.encryptionSecretSelector, eventRecorder)
377626

627+
if scenario.kmsConfigHash != "" || scenario.kmsKeyIDHash != "" {
628+
kmsHashesGetterFunc = func(ctx context.Context, kmsConfig *configv1.KMSConfig) (string, string, error) {
629+
return scenario.kmsConfigHash, scenario.kmsKeyIDHash, nil
630+
}
631+
}
632+
378633
// act
379634
err = target.Sync(context.TODO(), factory.NewSyncContext("test", eventRecorder))
380635

0 commit comments

Comments
 (0)