diff --git a/pkg/operator/encryption/controllers/migration_controller.go b/pkg/operator/encryption/controllers/migration_controller.go index 09413f363f..7fc649f020 100644 --- a/pkg/operator/encryption/controllers/migration_controller.go +++ b/pkg/operator/encryption/controllers/migration_controller.go @@ -182,7 +182,10 @@ func (c *migrationController) migrateKeysIfNeededAndRevisionStable(ctx context.C return nil, err } currentState, _ := encryptiondata.ToEncryptionState(currentEncryptionConfig, encryptionSecrets) - desiredEncryptedSecretData := encryptiondata.FromEncryptionState(desiredEncryptionState) + desiredEncryptedSecretData, err := encryptiondata.FromEncryptionState(desiredEncryptionState) + if err != nil { + return nil, err + } // no storage migration until config is stable if !reflect.DeepEqual(currentEncryptionConfig.Encryption.Resources, desiredEncryptedSecretData.Encryption.Resources) { diff --git a/pkg/operator/encryption/controllers/state_controller.go b/pkg/operator/encryption/controllers/state_controller.go index c0485e6ed7..a0b7f1f677 100644 --- a/pkg/operator/encryption/controllers/state_controller.go +++ b/pkg/operator/encryption/controllers/state_controller.go @@ -143,7 +143,10 @@ func (c *stateController) generateAndApplyCurrentEncryptionConfigSecret(ctx cont return nil } - desiredSecretData := encryptiondata.FromEncryptionState(desiredEncryptionState) + desiredSecretData, err := encryptiondata.FromEncryptionState(desiredEncryptionState) + if err != nil { + return err + } changed, err := c.applyEncryptionConfigSecret(ctx, desiredSecretData, recorder) if err != nil { return err diff --git a/pkg/operator/encryption/encryptiondata/config.go b/pkg/operator/encryption/encryptiondata/config.go index 4c5c0553be..e51837a5cb 100644 --- a/pkg/operator/encryption/encryptiondata/config.go +++ b/pkg/operator/encryption/encryptiondata/config.go @@ -7,6 +7,7 @@ import ( "strings" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime/schema" apiserverconfigv1 "k8s.io/apiserver/pkg/apis/apiserver/v1" "k8s.io/klog/v2" @@ -36,7 +37,7 @@ func (c *Config) HasEncryptionConfiguration() bool { } // FromEncryptionState converts encryption state to Config. -func FromEncryptionState(encryptionState map[schema.GroupResource]state.GroupResourceState) *Config { +func FromEncryptionState(encryptionState map[schema.GroupResource]state.GroupResourceState) (*Config, error) { resourceConfigs := make([]apiserverconfigv1.ResourceConfiguration, 0, len(encryptionState)) var kmsProviders map[string]*configv1.KMSConfig @@ -56,7 +57,13 @@ func FromEncryptionState(encryptionState map[schema.GroupResource]state.GroupRes if kmsProviders == nil { kmsProviders = map[string]*configv1.KMSConfig{} } - if _, exists := kmsProviders[key.Key.Name]; !exists { + if provider, exists := kmsProviders[key.Key.Name]; exists { + // Sanity check: the same keyID seen from a different resource must carry + // an identical provider config, since they originate from the same Key Secret. + if !equality.Semantic.DeepEqual(provider, key.KMSConfig.Provider) { + return nil, fmt.Errorf("KMS provider config mismatch for keyID %s: configs from different resources must be identical", key.Key.Name) + } + } else { kmsProviders[key.Key.Name] = key.KMSConfig.Provider } } @@ -71,7 +78,7 @@ func FromEncryptionState(encryptionState map[schema.GroupResource]state.GroupRes return &Config{ Encryption: &apiserverconfigv1.EncryptionConfiguration{Resources: resourceConfigs}, KMSProviders: kmsProviders, - } + }, nil } // ToEncryptionState converts config to state. diff --git a/pkg/operator/encryption/encryptiondata/config_test.go b/pkg/operator/encryption/encryptiondata/config_test.go index 1f112466e8..e5bf4329aa 100644 --- a/pkg/operator/encryption/encryptiondata/config_test.go +++ b/pkg/operator/encryption/encryptiondata/config_test.go @@ -657,7 +657,10 @@ func TestFromEncryptionState(t *testing.T) { } grState[gr] = ks } - actualOutput := encryptiondata.FromEncryptionState(grState) + actualOutput, err := encryptiondata.FromEncryptionState(grState) + if err != nil { + t.Fatalf("unexpected error from FromEncryptionState: %v", err) + } expectedOutput := scenario.makeOutput(scenario.writeKeyIn, scenario.readKeysIn) if !cmp.Equal(expectedOutput, actualOutput.Encryption.Resources) { @@ -705,6 +708,96 @@ func newFakeIdentityKeyForTest() []byte { return make([]byte, 16) } +func TestFromEncryptionStateKMSProviderConfigValidation(t *testing.T) { + tests := []struct { + name string + encryptionState map[schema.GroupResource]state.GroupResourceState + expectedErr string + }{ + { + name: "matching provider configs across resources", + encryptionState: map[schema.GroupResource]state.GroupResourceState{ + {Resource: "secrets"}: { + ReadKeys: []state.KeyState{{ + Key: apiserverconfigv1.Key{Name: "1", Secret: "AAAAAAAAAAAAAAAAAAAAAA=="}, + Mode: state.KMS, + KMSConfig: &state.KMSConfig{ + Encryption: &apiserverconfigv1.KMSConfiguration{APIVersion: "v2", Name: "1", Endpoint: "unix:///var/run/kmsplugin/kms-1.sock"}, + Provider: encryptiontesting.DefaultKMSProviderConfig, + }, + }}, + }, + {Resource: "configmaps"}: { + ReadKeys: []state.KeyState{{ + Key: apiserverconfigv1.Key{Name: "1", Secret: "AAAAAAAAAAAAAAAAAAAAAA=="}, + Mode: state.KMS, + KMSConfig: &state.KMSConfig{ + Encryption: &apiserverconfigv1.KMSConfiguration{APIVersion: "v2", Name: "1", Endpoint: "unix:///var/run/kmsplugin/kms-1.sock"}, + Provider: encryptiontesting.DefaultKMSProviderConfig, + }, + }}, + }, + }, + }, + { + name: "mismatched provider configs across resources", + encryptionState: map[schema.GroupResource]state.GroupResourceState{ + {Resource: "secrets"}: { + ReadKeys: []state.KeyState{{ + Key: apiserverconfigv1.Key{Name: "1", Secret: "AAAAAAAAAAAAAAAAAAAAAA=="}, + Mode: state.KMS, + KMSConfig: &state.KMSConfig{ + Encryption: &apiserverconfigv1.KMSConfiguration{APIVersion: "v2", Name: "1", Endpoint: "unix:///var/run/kmsplugin/kms-1.sock"}, + Provider: &configv1.KMSConfig{ + Type: configv1.VaultKMSProvider, + Vault: configv1.VaultKMSConfig{ + VaultAddress: "https://vault-a.example.com", + TransitKey: "key-a", + }, + }, + }, + }}, + }, + {Resource: "configmaps"}: { + ReadKeys: []state.KeyState{{ + Key: apiserverconfigv1.Key{Name: "1", Secret: "AAAAAAAAAAAAAAAAAAAAAA=="}, + Mode: state.KMS, + KMSConfig: &state.KMSConfig{ + Encryption: &apiserverconfigv1.KMSConfiguration{APIVersion: "v2", Name: "1", Endpoint: "unix:///var/run/kmsplugin/kms-1.sock"}, + Provider: &configv1.KMSConfig{ + Type: configv1.VaultKMSProvider, + Vault: configv1.VaultKMSConfig{ + VaultAddress: "https://vault-b.example.com", + TransitKey: "key-b", + }, + }, + }, + }}, + }, + }, + expectedErr: `KMS provider config mismatch for keyID 1: configs from different resources must be identical`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := encryptiondata.FromEncryptionState(tt.encryptionState) + if tt.expectedErr != "" { + if err == nil { + t.Fatal("expected error, got nil") + } + if err.Error() != tt.expectedErr { + t.Fatalf("unexpected error:\n got: %v\n expected: %v", err, tt.expectedErr) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} + func TestSecretRoundtrip(t *testing.T) { tests := []struct { name string diff --git a/pkg/operator/encryption/statemachine/transition_test.go b/pkg/operator/encryption/statemachine/transition_test.go index 266bc006cf..62a97bd619 100644 --- a/pkg/operator/encryption/statemachine/transition_test.go +++ b/pkg/operator/encryption/statemachine/transition_test.go @@ -49,7 +49,10 @@ func TestGetDesiredEncryptionState(t *testing.T) { } expected := expected.DeepCopy() expected.TypeMeta = metav1.TypeMeta{} - secretData := encryptiondata.FromEncryptionState(state) + secretData, err := encryptiondata.FromEncryptionState(state) + if err != nil { + ts.Fatalf("unexpected error from FromEncryptionState: %v", err) + } if !reflect.DeepEqual(expected, secretData.Encryption) { ts.Errorf("unexpected encryption config (A: expected, B: got):\n%s", diff.ObjectDiff(expected, secretData.Encryption)) }