diff --git a/pkg/operator/encryption/controllers/condition_controller.go b/pkg/operator/encryption/controllers/condition_controller.go index ee4adb7717..34855d0326 100644 --- a/pkg/operator/encryption/controllers/condition_controller.go +++ b/pkg/operator/encryption/controllers/condition_controller.go @@ -7,6 +7,7 @@ import ( "time" operatorv1 "github.com/openshift/api/operator/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" @@ -16,6 +17,7 @@ import ( applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/encryption/encryptiondata" + "github.com/openshift/library-go/pkg/operator/encryption/secrets" "github.com/openshift/library-go/pkg/operator/encryption/state" "github.com/openshift/library-go/pkg/operator/encryption/statemachine" "github.com/openshift/library-go/pkg/operator/events" @@ -96,7 +98,14 @@ func (c *conditionController) sync(ctx context.Context, _ factory.SyncContext) ( } encryptedGRs := c.provider.EncryptedGRs() - currentConfig, desiredState, foundSecrets, transitioningReason, err := statemachine.GetEncryptionConfigAndState(ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, encryptedGRs) + currentConfig, desiredState, foundSecrets, transitioningReason, err := statemachine.GetEncryptionConfigAndState( + ctx, + c.deployer.DeployedEncryptionConfigSecret, + func(ctx context.Context) ([]*corev1.Secret, error) { + return secrets.ListKeySecrets(ctx, c.secretClient, c.encryptionSecretSelector) + }, + encryptedGRs, + ) if err != nil || len(transitioningReason) > 0 { // do not update the encryption condition (cond). Note: progressing is set elsewhere. cond = nil diff --git a/pkg/operator/encryption/controllers/encryption_computer.go b/pkg/operator/encryption/controllers/encryption_computer.go new file mode 100644 index 0000000000..8dc38017a8 --- /dev/null +++ b/pkg/operator/encryption/controllers/encryption_computer.go @@ -0,0 +1,61 @@ +package controllers + +import ( + "context" + + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/util/workqueue" + + "github.com/openshift/library-go/pkg/controller/factory" +) + +// EncryptionComputer accepts a keyController and a stateController and +// allows computing their outputs without side effects. +type EncryptionComputer struct { + keyController *keyController + stateController *stateController +} + +func NewEncryptionComputer(keyCtrl *keyController, stateCtrl *stateController) *EncryptionComputer { + return &EncryptionComputer{ + keyController: keyCtrl, + stateController: stateCtrl, + } +} + +// ComputeKeySecret returns the key secret that would be created by the +// key controller, or nil if no new key is needed. +func (e *EncryptionComputer) ComputeKeySecret(ctx context.Context, syncCtx factory.SyncContext) (*corev1.Secret, error) { + return e.keyController.computeKeySecret(ctx, syncCtx) +} + +// ComputeEncryptionConfigSecret returns the encryption config secret that +// would be applied by the state controller, or nil if no update is needed. +func (e *EncryptionComputer) ComputeEncryptionConfigSecret(ctx context.Context, queue workqueue.RateLimitingInterface) (*corev1.Secret, []eventWithReason, error) { + return e.stateController.computeEncryptionConfigSecret(ctx, queue) +} + +// ComputeEncryptionConfigSecretWithNewKey computes the key secret that +// would be created by the key controller and propagates it into the state +// controller's computation, returning the encryption config secret that +// would result if the new key had been created. +func (e *EncryptionComputer) ComputeEncryptionConfigSecretWithNewKey(ctx context.Context, syncCtx factory.SyncContext) (*corev1.Secret, []eventWithReason, error) { + newKeySecret, err := e.keyController.computeKeySecret(ctx, syncCtx) + if err != nil { + return nil, nil, err + } + + sc := e.stateController + listKeySecretsFn := sc.listKeySecretsFn + if newKeySecret != nil { + listKeySecretsFn = func(ctx context.Context) ([]*corev1.Secret, error) { + existing, err := sc.listKeySecretsFn(ctx) + if err != nil { + return nil, err + } + return append([]*corev1.Secret{newKeySecret}, existing...), nil + } + } + + return sc.computeEncryptionConfigSecretWithCustomListKeySecretFn(ctx, syncCtx.Queue(), listKeySecretsFn) +} diff --git a/pkg/operator/encryption/controllers/encryption_computer_test.go b/pkg/operator/encryption/controllers/encryption_computer_test.go new file mode 100644 index 0000000000..c60e89c046 --- /dev/null +++ b/pkg/operator/encryption/controllers/encryption_computer_test.go @@ -0,0 +1,245 @@ +package controllers + +import ( + "context" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/diff" + clocktesting "k8s.io/utils/clock/testing" + sigsyaml "sigs.k8s.io/yaml" + + configv1 "github.com/openshift/api/config/v1" + operatorv1 "github.com/openshift/api/operator/v1" + + "github.com/openshift/library-go/pkg/controller/factory" + encryptiontesting "github.com/openshift/library-go/pkg/operator/encryption/testing" + "github.com/openshift/library-go/pkg/operator/events" +) + +var ( + testSecretGR = schema.GroupResource{Resource: "secrets"} + testEncryptedGRs = []schema.GroupResource{testSecretGR} + aescbcRawKey = []byte("61def964fb967f5d7c44a2af8dab6865") + + // testKMSPluginConfig has no external secret or configmap references so the + // expected key secret can be fully expressed as a literal YAML string. + testKMSPluginConfig = configv1.KMSPluginConfig{ + Type: configv1.VaultKMSProvider, + Vault: configv1.VaultKMSPluginConfig{ + KMSPluginImage: "vault-kms-plugin:latest", + VaultAddress: "https://vault.example.com", + Authentication: configv1.VaultAuthentication{ + Type: configv1.VaultAuthenticationTypeAppRole, + AppRole: configv1.VaultAppRoleAuthentication{}, // no secret reference + }, + VaultKeyPath: "transit/keys/mykey", + }, + } +) + +func TestComputeEncryptionConfigSecretWithNewKey(t *testing.T) { + aescbcKey7Secret := encryptiontesting.CreateExpiredMigratedEncryptionKeySecretWithRawKey( + "openshift-config-managed", testEncryptedGRs, 7, aescbcRawKey, + ) + + scenarios := []struct { + name string + existingKeySecrets []*corev1.Secret // inputs: key secrets already in the cluster + + wantNewKeySecret string // expected: YAML of the newly computed key secret + wantEncConfigSecret string // expected: YAML of the computed encryption config secret + }{ + { + name: "fresh KMS setup: no existing keys", + existingKeySecrets: nil, + + wantNewKeySecret: ` +data: + encryption.apiserver.operator.openshift.io-key: AAAAAAAAAAAAAAAAAAAAAA== + encryption.apiserver.operator.openshift.io-kms-encryption-config: eyJraW5kIjoiRW5jcnlwdGlvbkNvbmZpZ3VyYXRpb24iLCJhcGlWZXJzaW9uIjoiYXBpc2VydmVyLmNvbmZpZy5rOHMuaW8vdjEiLCJyZXNvdXJjZXMiOlt7InJlc291cmNlcyI6bnVsbCwicHJvdmlkZXJzIjpbeyJrbXMiOnsiYXBpVmVyc2lvbiI6InYyIiwibmFtZSI6IjEiLCJlbmRwb2ludCI6InVuaXg6Ly8vdmFyL3J1bi9rbXNwbHVnaW4va21zLTEuc29jayIsInRpbWVvdXQiOiIxMHMifX1dfV19Cg== + encryption.apiserver.operator.openshift.io-kms-plugin-config: eyJraW5kIjoiQVBJU2VydmVyIiwiYXBpVmVyc2lvbiI6ImNvbmZpZy5vcGVuc2hpZnQuaW8vdjEiLCJtZXRhZGF0YSI6e30sInNwZWMiOnsic2VydmluZ0NlcnRzIjp7fSwiY2xpZW50Q0EiOnsibmFtZSI6IiJ9LCJlbmNyeXB0aW9uIjp7ImttcyI6eyJ0eXBlIjoiVmF1bHQiLCJ2YXVsdCI6eyJrbXNQbHVnaW5JbWFnZSI6InZhdWx0LWttcy1wbHVnaW46bGF0ZXN0IiwidmF1bHRBZGRyZXNzIjoiaHR0cHM6Ly92YXVsdC5leGFtcGxlLmNvbSIsImF1dGhlbnRpY2F0aW9uIjp7InR5cGUiOiJBcHBSb2xlIn0sInZhdWx0S2V5UGF0aCI6InRyYW5zaXQva2V5cy9teWtleSJ9fX0sImF1ZGl0Ijp7fX0sInN0YXR1cyI6e319Cg== +metadata: + annotations: + encryption.apiserver.operator.openshift.io/external-reason: "" + encryption.apiserver.operator.openshift.io/internal-reason: secrets-key-does-not-exist + encryption.apiserver.operator.openshift.io/mode: KMS + kubernetes.io/description: |- + WARNING: DO NOT EDIT. + Altering of the encryption secrets will render you cluster inaccessible. + Catastrophic data loss can occur from the most minor changes. + finalizers: + - encryption.apiserver.operator.openshift.io/deletion-protection + labels: + encryption.apiserver.operator.openshift.io/component: test-component + name: encryption-key-test-component-1 + namespace: openshift-config-managed +type: Opaque +`, + + // State machine places the new KMS key as a read key on the first + // pass; write key promotion happens after the config converges. + wantEncConfigSecret: ` +apiVersion: v1 +data: + encryption-config: eyJraW5kIjoiRW5jcnlwdGlvbkNvbmZpZ3VyYXRpb24iLCJhcGlWZXJzaW9uIjoiYXBpc2VydmVyLmNvbmZpZy5rOHMuaW8vdjEiLCJyZXNvdXJjZXMiOlt7InJlc291cmNlcyI6WyJzZWNyZXRzIl0sInByb3ZpZGVycyI6W3siaWRlbnRpdHkiOnt9fSx7ImttcyI6eyJhcGlWZXJzaW9uIjoidjIiLCJuYW1lIjoiMV9zZWNyZXRzIiwiZW5kcG9pbnQiOiJ1bml4Oi8vL3Zhci9ydW4va21zcGx1Z2luL2ttcy0xLnNvY2siLCJ0aW1lb3V0IjoiMTBzIn19XX1dfQo= + kms-plugin-config-1: eyJraW5kIjoiQVBJU2VydmVyIiwiYXBpVmVyc2lvbiI6ImNvbmZpZy5vcGVuc2hpZnQuaW8vdjEiLCJtZXRhZGF0YSI6e30sInNwZWMiOnsic2VydmluZ0NlcnRzIjp7fSwiY2xpZW50Q0EiOnsibmFtZSI6IiJ9LCJlbmNyeXB0aW9uIjp7ImttcyI6eyJ0eXBlIjoiVmF1bHQiLCJ2YXVsdCI6eyJrbXNQbHVnaW5JbWFnZSI6InZhdWx0LWttcy1wbHVnaW46bGF0ZXN0IiwidmF1bHRBZGRyZXNzIjoiaHR0cHM6Ly92YXVsdC5leGFtcGxlLmNvbSIsImF1dGhlbnRpY2F0aW9uIjp7InR5cGUiOiJBcHBSb2xlIn0sInZhdWx0S2V5UGF0aCI6InRyYW5zaXQva2V5cy9teWtleSJ9fX0sImF1ZGl0Ijp7fX0sInN0YXR1cyI6e319Cg== +kind: Secret +metadata: + annotations: + kubernetes.io/description: |- + WARNING: DO NOT EDIT. + Altering of the encryption secrets will render you cluster inaccessible. + Catastrophic data loss can occur from the most minor changes. + finalizers: + - encryption.apiserver.operator.openshift.io/deletion-protection + name: encryption-config-test-component + namespace: openshift-config-managed +type: Opaque +`, + }, + { + name: "migrating from AESCBC to KMS: one fully-migrated AESCBC key exists", + existingKeySecrets: []*corev1.Secret{aescbcKey7Secret}, + + wantNewKeySecret: ` +data: + encryption.apiserver.operator.openshift.io-key: AAAAAAAAAAAAAAAAAAAAAA== + encryption.apiserver.operator.openshift.io-kms-encryption-config: eyJraW5kIjoiRW5jcnlwdGlvbkNvbmZpZ3VyYXRpb24iLCJhcGlWZXJzaW9uIjoiYXBpc2VydmVyLmNvbmZpZy5rOHMuaW8vdjEiLCJyZXNvdXJjZXMiOlt7InJlc291cmNlcyI6bnVsbCwicHJvdmlkZXJzIjpbeyJrbXMiOnsiYXBpVmVyc2lvbiI6InYyIiwibmFtZSI6IjgiLCJlbmRwb2ludCI6InVuaXg6Ly8vdmFyL3J1bi9rbXNwbHVnaW4va21zLTguc29jayIsInRpbWVvdXQiOiIxMHMifX1dfV19Cg== + encryption.apiserver.operator.openshift.io-kms-plugin-config: eyJraW5kIjoiQVBJU2VydmVyIiwiYXBpVmVyc2lvbiI6ImNvbmZpZy5vcGVuc2hpZnQuaW8vdjEiLCJtZXRhZGF0YSI6e30sInNwZWMiOnsic2VydmluZ0NlcnRzIjp7fSwiY2xpZW50Q0EiOnsibmFtZSI6IiJ9LCJlbmNyeXB0aW9uIjp7ImttcyI6eyJ0eXBlIjoiVmF1bHQiLCJ2YXVsdCI6eyJrbXNQbHVnaW5JbWFnZSI6InZhdWx0LWttcy1wbHVnaW46bGF0ZXN0IiwidmF1bHRBZGRyZXNzIjoiaHR0cHM6Ly92YXVsdC5leGFtcGxlLmNvbSIsImF1dGhlbnRpY2F0aW9uIjp7InR5cGUiOiJBcHBSb2xlIn0sInZhdWx0S2V5UGF0aCI6InRyYW5zaXQva2V5cy9teWtleSJ9fX0sImF1ZGl0Ijp7fX0sInN0YXR1cyI6e319Cg== +metadata: + annotations: + encryption.apiserver.operator.openshift.io/external-reason: "" + encryption.apiserver.operator.openshift.io/internal-reason: secrets-encryption-mode-changed + encryption.apiserver.operator.openshift.io/mode: KMS + kubernetes.io/description: |- + WARNING: DO NOT EDIT. + Altering of the encryption secrets will render you cluster inaccessible. + Catastrophic data loss can occur from the most minor changes. + finalizers: + - encryption.apiserver.operator.openshift.io/deletion-protection + labels: + encryption.apiserver.operator.openshift.io/component: test-component + name: encryption-key-test-component-8 + namespace: openshift-config-managed +type: Opaque +`, + + // Both the new KMS key (8) and the existing AESCBC key (7) appear + // as read keys; write key promotion happens after convergence. + wantEncConfigSecret: ` +apiVersion: v1 +data: + encryption-config: eyJraW5kIjoiRW5jcnlwdGlvbkNvbmZpZ3VyYXRpb24iLCJhcGlWZXJzaW9uIjoiYXBpc2VydmVyLmNvbmZpZy5rOHMuaW8vdjEiLCJyZXNvdXJjZXMiOlt7InJlc291cmNlcyI6WyJzZWNyZXRzIl0sInByb3ZpZGVycyI6W3siaWRlbnRpdHkiOnt9fSx7ImttcyI6eyJhcGlWZXJzaW9uIjoidjIiLCJuYW1lIjoiOF9zZWNyZXRzIiwiZW5kcG9pbnQiOiJ1bml4Oi8vL3Zhci9ydW4va21zcGx1Z2luL2ttcy04LnNvY2siLCJ0aW1lb3V0IjoiMTBzIn19LHsiYWVzY2JjIjp7ImtleXMiOlt7Im5hbWUiOiI3Iiwic2VjcmV0IjoiTmpGa1pXWTVOalJtWWprMk4yWTFaRGRqTkRSaE1tRm1PR1JoWWpZNE5qVT0ifV19fV19XX0K + kms-plugin-config-8: eyJraW5kIjoiQVBJU2VydmVyIiwiYXBpVmVyc2lvbiI6ImNvbmZpZy5vcGVuc2hpZnQuaW8vdjEiLCJtZXRhZGF0YSI6e30sInNwZWMiOnsic2VydmluZ0NlcnRzIjp7fSwiY2xpZW50Q0EiOnsibmFtZSI6IiJ9LCJlbmNyeXB0aW9uIjp7ImttcyI6eyJ0eXBlIjoiVmF1bHQiLCJ2YXVsdCI6eyJrbXNQbHVnaW5JbWFnZSI6InZhdWx0LWttcy1wbHVnaW46bGF0ZXN0IiwidmF1bHRBZGRyZXNzIjoiaHR0cHM6Ly92YXVsdC5leGFtcGxlLmNvbSIsImF1dGhlbnRpY2F0aW9uIjp7InR5cGUiOiJBcHBSb2xlIn0sInZhdWx0S2V5UGF0aCI6InRyYW5zaXQva2V5cy9teWtleSJ9fX0sImF1ZGl0Ijp7fX0sInN0YXR1cyI6e319Cg== +kind: Secret +metadata: + annotations: + kubernetes.io/description: |- + WARNING: DO NOT EDIT. + Altering of the encryption secrets will render you cluster inaccessible. + Catastrophic data loss can occur from the most minor changes. + finalizers: + - encryption.apiserver.operator.openshift.io/deletion-protection + name: encryption-config-test-component + namespace: openshift-config-managed +type: Opaque +`, + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + computer := newTestEncryptionComputer(scenario.existingKeySecrets) + syncCtx := newTestSyncContext() + + gotNewKeySecret, err := computer.ComputeKeySecret(context.Background(), syncCtx) + if err != nil { + t.Fatalf("ComputeKeySecret: %v", err) + } + if !equality.Semantic.DeepEqual(gotNewKeySecret, mustParseSecret(t, scenario.wantNewKeySecret)) { + t.Errorf("new key secret mismatch:\n%s", diff.Diff(mustParseSecret(t, scenario.wantNewKeySecret), gotNewKeySecret)) + } + + gotEncConfigSecret, _, err := computer.ComputeEncryptionConfigSecretWithNewKey(context.Background(), syncCtx) + if err != nil { + t.Fatalf("ComputeEncryptionConfigSecretWithNewKey: %v", err) + } + if !equality.Semantic.DeepEqual(gotEncConfigSecret, mustParseSecret(t, scenario.wantEncConfigSecret)) { + t.Errorf("encryption config secret mismatch:\n%s", diff.Diff(mustParseSecret(t, scenario.wantEncConfigSecret), gotEncConfigSecret)) + } + }) + } +} + +func mustParseSecret(t *testing.T, yamlStr string) *corev1.Secret { + t.Helper() + s := &corev1.Secret{} + if err := sigsyaml.Unmarshal([]byte(yamlStr), s); err != nil { + t.Fatalf("mustParseSecret: %v", err) + } + return s +} + +func newTestEncryptionComputer(existingKeySecrets []*corev1.Secret) *EncryptionComputer { + instanceName := "test-component" + provider := &fakeProvider{encryptedGRs: testEncryptedGRs} + + noDeployedConfig := func(_ context.Context) (*corev1.Secret, bool, error) { + return nil, true, nil + } + listExistingKeys := func(_ context.Context) ([]*corev1.Secret, error) { + return existingKeySecrets, nil + } + + keyCtrl := &keyController{ + instanceName: instanceName, + provider: provider, + getAPIServerAndOperatorSpecFn: func(_ context.Context) (*configv1.APIServer, *operatorv1.OperatorSpec, error) { + return &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.APIServerSpec{ + Encryption: configv1.APIServerEncryption{ + Type: "KMS", + KMS: testKMSPluginConfig, + }, + }, + }, &operatorv1.OperatorSpec{}, nil + }, + deployedEncryptionConfigSecretFn: noDeployedConfig, + listKeySecretsFn: listExistingKeys, + getKMSPluginSecretFn: func(_ context.Context, _ string) (*corev1.Secret, error) { + return nil, nil // not called: testKMSPluginConfig has no secret reference + }, + getKMSPluginConfigMapFn: func(_ context.Context, _ string) (*corev1.ConfigMap, error) { + return nil, nil // not called: testKMSPluginConfig has no configmap reference + }, + } + + stateCtrl := &stateController{ + instanceName: instanceName, + provider: provider, + deployedEncryptionConfigSecretFn: noDeployedConfig, + listKeySecretsFn: listExistingKeys, + } + + return NewEncryptionComputer(keyCtrl, stateCtrl) +} + +func newTestSyncContext() factory.SyncContext { + recorder := events.NewRecorder(nil, "test", &corev1.ObjectReference{}, clocktesting.NewFakePassiveClock(time.Now())) + return factory.NewSyncContext("test", recorder) +} + +var _ Provider = &fakeProvider{} + +type fakeProvider struct { + encryptedGRs []schema.GroupResource +} + +func (f *fakeProvider) EncryptedGRs() []schema.GroupResource { return f.encryptedGRs } +func (f *fakeProvider) ShouldRunEncryptionControllers() (bool, error) { return true, nil } diff --git a/pkg/operator/encryption/controllers/key_computer.go b/pkg/operator/encryption/controllers/key_computer.go new file mode 100644 index 0000000000..d657675de0 --- /dev/null +++ b/pkg/operator/encryption/controllers/key_computer.go @@ -0,0 +1,25 @@ +package controllers + +import ( + "context" + + corev1 "k8s.io/api/core/v1" + + "github.com/openshift/library-go/pkg/controller/factory" +) + +// KeyComputer uses a keyController to compute what key secret would be +// needed without actually creating it. +type KeyComputer struct { + controller *keyController +} + +func newKeyComputer(controller *keyController) *KeyComputer { + return &KeyComputer{controller: controller} +} + +// ComputeKey returns the key secret that would be created by the key +// controller, or nil if no new key is needed. +func (k *KeyComputer) ComputeKey(ctx context.Context, syncCtx factory.SyncContext) (*corev1.Secret, error) { + return k.controller.computeKeySecret(ctx, syncCtx) +} diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index 653c643c71..23990e97a4 100644 --- a/pkg/operator/encryption/controllers/key_controller.go +++ b/pkg/operator/encryption/controllers/key_controller.go @@ -80,6 +80,12 @@ type keyController struct { preconditionsFulfilledFn preconditionsFulfilled unsupportedConfigPrefix []string + + getAPIServerAndOperatorSpecFn func(context.Context) (*configv1.APIServer, *operatorv1.OperatorSpec, error) + deployedEncryptionConfigSecretFn func(context.Context) (*corev1.Secret, bool, error) + listKeySecretsFn func(context.Context) ([]*corev1.Secret, error) + getKMSPluginSecretFn func(context.Context, string) (*corev1.Secret, error) + getKMSPluginConfigMapFn func(context.Context, string) (*corev1.ConfigMap, error) } func NewKeyController( @@ -113,6 +119,28 @@ func NewKeyController( configMapClient: configMapClient, } + c.getAPIServerAndOperatorSpecFn = func(ctx context.Context) (*configv1.APIServer, *operatorv1.OperatorSpec, error) { + apiServer, err := c.apiServerClient.Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return nil, nil, err + } + operatorSpec, _, _, err := c.operatorClient.GetOperatorState() + if err != nil { + return nil, nil, err + } + return apiServer, operatorSpec, nil + } + c.deployedEncryptionConfigSecretFn = c.deployer.DeployedEncryptionConfigSecret + c.listKeySecretsFn = func(ctx context.Context) ([]*corev1.Secret, error) { + return secrets.ListKeySecrets(ctx, c.secretClient, c.encryptionSecretSelector) + } + c.getKMSPluginSecretFn = func(ctx context.Context, name string) (*corev1.Secret, error) { + return c.secretClient.Secrets(openshiftConfigNS).Get(ctx, name, metav1.GetOptions{}) + } + c.getKMSPluginConfigMapFn = func(ctx context.Context, name string) (*corev1.ConfigMap, error) { + return c.configMapClient.ConfigMaps(openshiftConfigNS).Get(ctx, name, metav1.GetOptions{}) + } + return factory.New(). WithSync(c.sync). WithControllerInstanceName(c.controllerInstanceName). @@ -154,7 +182,19 @@ func (c *keyController) sync(ctx context.Context, syncCtx factory.SyncContext) ( return err // we will get re-kicked when the operator status updates } - err = c.checkAndCreateKeys(ctx, syncCtx, c.provider.EncryptedGRs()) + keySecret, err := c.computeKeySecret(ctx, syncCtx) + if err == nil && keySecret != nil { + keyID, _ := state.NameToKeyID(keySecret.Name) + _, createErr := c.secretClient.Secrets("openshift-config-managed").Create(ctx, keySecret, metav1.CreateOptions{}) + if errors.IsAlreadyExists(createErr) { + err = c.validateExistingSecret(ctx, keySecret, keyID) + } else if createErr != nil { + syncCtx.Recorder().Warningf("EncryptionKeyCreateFailed", "Secret %q failed to create: %v", keySecret.Name, createErr) + err = createErr + } else { + syncCtx.Recorder().Eventf("EncryptionKeyCreated", "Secret %q successfully created", keySecret.Name) + } + } if err != nil { degradedCondition = degradedCondition. WithStatus(operatorv1.ConditionTrue). @@ -168,25 +208,53 @@ func (c *keyController) sync(ctx context.Context, syncCtx factory.SyncContext) ( return err } -func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext factory.SyncContext, encryptedGRs []schema.GroupResource) error { - currentMode, externalReason, apiEncryptionConfiguration, err := c.getCurrentModeReasonAndEncryptionConfig(ctx) +func (c *keyController) computeKeySecret(ctx context.Context, syncContext factory.SyncContext) (*corev1.Secret, error) { + return checkAndCreateKeys( + ctx, syncContext, c.provider.EncryptedGRs(), + c.instanceName, c.unsupportedConfigPrefix, + c.getAPIServerAndOperatorSpecFn, + c.deployedEncryptionConfigSecretFn, + c.listKeySecretsFn, + c.getKMSPluginSecretFn, + c.getKMSPluginConfigMapFn, + ) +} + +func checkAndCreateKeys( + ctx context.Context, + syncContext factory.SyncContext, + encryptedGRs []schema.GroupResource, + instanceName string, + unsupportedConfigPrefix []string, + getAPIServerAndOperatorSpecFn func(context.Context) (*configv1.APIServer, *operatorv1.OperatorSpec, error), + deployedEncryptionConfigSecretFn func(context.Context) (*corev1.Secret, bool, error), + listKeySecretsFn func(context.Context) ([]*corev1.Secret, error), + getKMSPluginSecretFn func(context.Context, string) (*corev1.Secret, error), + getKMSPluginConfigMapFn func(context.Context, string) (*corev1.ConfigMap, error), +) (*corev1.Secret, error) { + currentMode, externalReason, apiEncryptionConfiguration, err := getCurrentModeReasonAndEncryptionConfig(ctx, getAPIServerAndOperatorSpecFn, unsupportedConfigPrefix) if err != nil { - return err + return nil, err } - currentConfig, desiredEncryptionState, secrets, isProgressingReason, err := statemachine.GetEncryptionConfigAndState(ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, encryptedGRs) + currentConfig, desiredEncryptionState, encryptionSecrets, isProgressingReason, err := statemachine.GetEncryptionConfigAndState( + ctx, + deployedEncryptionConfigSecretFn, + listKeySecretsFn, + encryptedGRs, + ) if err != nil { - return err + return nil, err } if len(isProgressingReason) > 0 { syncContext.Queue().AddAfter(syncContext.QueueKey(), 2*time.Minute) - return nil + return nil, nil } // avoid intended start of encryption - hasBeenOnBefore := currentConfig != nil || len(secrets) > 0 + hasBeenOnBefore := currentConfig != nil || len(encryptionSecrets) > 0 if currentMode == state.Identity && !hasBeenOnBefore { - return nil + return nil, nil } var ( @@ -204,7 +272,7 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact var err error desiredProviderCfg, err = newKMSProviderConfig(apiEncryptionConfiguration.KMS) if err != nil { - return err + return nil, err } } @@ -212,7 +280,7 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact for gr, grKeys := range desiredEncryptionState { latestKeyID, internalReason, needed, err := needsNewKey(grKeys, currentMode, externalReason, encryptedGRs, desiredProviderCfg) if err != nil { - return err + return nil, err } if !needed { continue @@ -232,7 +300,7 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact reasons = append(reasons, fmt.Sprintf("%s-%s", gr.Resource, internalReason)) } if !newKeyRequired { - return nil + return nil, nil } if commonReason != nil && len(*commonReason) > 0 && len(reasons) > 1 { reasons = []string{*commonReason} // don't repeat reasons @@ -240,22 +308,11 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact sort.Sort(sort.StringSlice(reasons)) internalReason := strings.Join(reasons, ", ") - keySecret, err := c.generateKeySecret(ctx, newKeyID, currentMode, apiEncryptionConfiguration, desiredProviderCfg, internalReason, externalReason) + keySecret, err := generateKeySecret(ctx, instanceName, newKeyID, currentMode, apiEncryptionConfiguration, desiredProviderCfg, internalReason, externalReason, getKMSPluginSecretFn, getKMSPluginConfigMapFn) if err != nil { - return fmt.Errorf("failed to create key: %v", err) + return nil, fmt.Errorf("failed to create key: %v", err) } - _, createErr := c.secretClient.Secrets("openshift-config-managed").Create(ctx, keySecret, metav1.CreateOptions{}) - if errors.IsAlreadyExists(createErr) { - return c.validateExistingSecret(ctx, keySecret, newKeyID) - } - if createErr != nil { - syncContext.Recorder().Warningf("EncryptionKeyCreateFailed", "Secret %q failed to create: %v", keySecret.Name, err) - return createErr - } - - syncContext.Recorder().Eventf("EncryptionKeyCreated", "Secret %q successfully created: %q", keySecret.Name, reasons) - - return nil + return keySecret, nil } func (c *keyController) validateExistingSecret(ctx context.Context, keySecret *corev1.Secret, keyID uint64) error { @@ -277,7 +334,7 @@ func (c *keyController) validateExistingSecret(ctx context.Context, keySecret *c return nil // we made this key earlier } -func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, currentMode state.Mode, apiServerEncryption configv1.APIServerEncryption, desiredProviderCfg kmsProviderConfig, internalReason, externalReason string) (*corev1.Secret, error) { +func generateKeySecret(ctx context.Context, instanceName string, keyID uint64, currentMode state.Mode, apiServerEncryption configv1.APIServerEncryption, desiredProviderCfg kmsProviderConfig, internalReason, externalReason string, getKMSPluginSecretFn func(context.Context, string) (*corev1.Secret, error), getKMSPluginConfigMapFn func(context.Context, string) (*corev1.ConfigMap, error)) (*corev1.Secret, error) { bs := crypto.ModeToNewKeyFunc[currentMode]() ks := state.KeyState{ Key: apiserverv1.Key{ @@ -302,7 +359,7 @@ func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, cur if secretName, expectedKeys, err := desiredProviderCfg.referencedSecretName(); err != nil { return nil, err } else if len(secretName) > 0 { - refSecret, err := c.secretClient.Secrets(openshiftConfigNS).Get(ctx, secretName, metav1.GetOptions{}) + refSecret, err := getKMSPluginSecretFn(ctx, secretName) if err != nil { return nil, fmt.Errorf("failed to get secret %s in %s: %w", secretName, openshiftConfigNS, err) } @@ -320,7 +377,7 @@ func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, cur if cmName, expectedKeys, err := desiredProviderCfg.referencedConfigMapName(); err != nil { return nil, err } else if len(cmName) > 0 { - refCM, err := c.configMapClient.ConfigMaps(openshiftConfigNS).Get(ctx, cmName, metav1.GetOptions{}) + refCM, err := getKMSPluginConfigMapFn(ctx, cmName) if err != nil { return nil, fmt.Errorf("failed to get configmap %s in %s: %w", cmName, openshiftConfigNS, err) } @@ -335,21 +392,16 @@ func (c *keyController) generateKeySecret(ctx context.Context, keyID uint64, cur } } } - return secrets.FromKeyState(c.instanceName, ks) + return secrets.FromKeyState(instanceName, ks) } -func (c *keyController) getCurrentModeReasonAndEncryptionConfig(ctx context.Context) (state.Mode, string, configv1.APIServerEncryption, error) { - apiServer, err := c.apiServerClient.Get(ctx, "cluster", metav1.GetOptions{}) - if err != nil { - return "", "", configv1.APIServerEncryption{}, err - } - - operatorSpec, _, _, err := c.operatorClient.GetOperatorState() +func getCurrentModeReasonAndEncryptionConfig(ctx context.Context, getAPIServerAndOperatorSpecFn func(context.Context) (*configv1.APIServer, *operatorv1.OperatorSpec, error), unsupportedConfigPrefix []string) (state.Mode, string, configv1.APIServerEncryption, error) { + apiServer, operatorSpec, err := getAPIServerAndOperatorSpecFn(ctx) if err != nil { return "", "", configv1.APIServerEncryption{}, err } - encryptionConfig, err := structuredUnsupportedConfigFrom(operatorSpec.UnsupportedConfigOverrides.Raw, c.unsupportedConfigPrefix) + encryptionConfig, err := structuredUnsupportedConfigFrom(operatorSpec.UnsupportedConfigOverrides.Raw, unsupportedConfigPrefix) if err != nil { return "", "", configv1.APIServerEncryption{}, err } diff --git a/pkg/operator/encryption/controllers/key_controller_test.go b/pkg/operator/encryption/controllers/key_controller_test.go index 76e911c2be..a6e94ac8ec 100644 --- a/pkg/operator/encryption/controllers/key_controller_test.go +++ b/pkg/operator/encryption/controllers/key_controller_test.go @@ -1275,8 +1275,21 @@ func TestGetCurrentModeReasonAndEncryptionConfig(t *testing.T) { fakeApiServerClient := fakeConfigClient.ConfigV1().APIServers() // act - target := keyController{unsupportedConfigPrefix: scenario.prefix, operatorClient: fakeOperatorClient, apiServerClient: fakeApiServerClient} - currentMode, externalReason, encryption, err := target.getCurrentModeReasonAndEncryptionConfig(context.TODO()) + currentMode, externalReason, encryption, err := getCurrentModeReasonAndEncryptionConfig( + context.TODO(), + func(ctx context.Context) (*configv1.APIServer, *operatorv1.OperatorSpec, error) { + apiServer, err := fakeApiServerClient.Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + return nil, nil, err + } + operatorSpec, _, _, err := fakeOperatorClient.GetOperatorState() + if err != nil { + return nil, nil, err + } + return apiServer, operatorSpec, nil + }, + scenario.prefix, + ) // validate if err != nil { diff --git a/pkg/operator/encryption/controllers/migration_controller.go b/pkg/operator/encryption/controllers/migration_controller.go index 7fc649f020..52304fad08 100644 --- a/pkg/operator/encryption/controllers/migration_controller.go +++ b/pkg/operator/encryption/controllers/migration_controller.go @@ -168,7 +168,14 @@ func (c *migrationController) sync(ctx context.Context, syncCtx factory.SyncCont // TODO doc func (c *migrationController) migrateKeysIfNeededAndRevisionStable(ctx context.Context, syncContext factory.SyncContext, encryptedGRs []schema.GroupResource) (migratingResources []schema.GroupResource, err error) { // no storage migration during revision changes - currentEncryptionConfig, desiredEncryptionState, _, isTransitionalReason, err := statemachine.GetEncryptionConfigAndState(ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, encryptedGRs) + currentEncryptionConfig, desiredEncryptionState, _, isTransitionalReason, err := statemachine.GetEncryptionConfigAndState( + ctx, + c.deployer.DeployedEncryptionConfigSecret, + func(ctx context.Context) ([]*corev1.Secret, error) { + return secrets.ListKeySecrets(ctx, c.secretClient, c.encryptionSecretSelector) + }, + encryptedGRs, + ) if err != nil { return nil, err } diff --git a/pkg/operator/encryption/controllers/prune_controller.go b/pkg/operator/encryption/controllers/prune_controller.go index 3078304e0b..09189d00d3 100644 --- a/pkg/operator/encryption/controllers/prune_controller.go +++ b/pkg/operator/encryption/controllers/prune_controller.go @@ -121,7 +121,14 @@ func (c *pruneController) sync(ctx context.Context, syncCtx factory.SyncContext) } func (c *pruneController) deleteOldMigratedSecrets(ctx context.Context, syncContext factory.SyncContext, encryptedGRs []schema.GroupResource) error { - _, desiredEncryptionConfig, _, isProgressingReason, err := statemachine.GetEncryptionConfigAndState(ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, encryptedGRs) + _, desiredEncryptionConfig, _, isProgressingReason, err := statemachine.GetEncryptionConfigAndState( + ctx, + c.deployer.DeployedEncryptionConfigSecret, + func(ctx context.Context) ([]*corev1.Secret, error) { + return secrets.ListKeySecrets(ctx, c.secretClient, c.encryptionSecretSelector) + }, + encryptedGRs, + ) if err != nil { return err } diff --git a/pkg/operator/encryption/controllers/state_controller.go b/pkg/operator/encryption/controllers/state_controller.go index 85b224e19c..1acffcdf48 100644 --- a/pkg/operator/encryption/controllers/state_controller.go +++ b/pkg/operator/encryption/controllers/state_controller.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" @@ -16,6 +17,7 @@ import ( configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/encryption/encryptiondata" + "github.com/openshift/library-go/pkg/operator/encryption/secrets" "github.com/openshift/library-go/pkg/operator/encryption/state" "github.com/openshift/library-go/pkg/operator/encryption/statemachine" "github.com/openshift/library-go/pkg/operator/events" @@ -46,6 +48,9 @@ type stateController struct { deployer statemachine.Deployer provider Provider preconditionsFulfilledFn preconditionsFulfilled + + deployedEncryptionConfigSecretFn func(context.Context) (*corev1.Secret, bool, error) + listKeySecretsFn func(context.Context) ([]*corev1.Secret, error) } func NewStateController( @@ -72,6 +77,11 @@ func NewStateController( preconditionsFulfilledFn: preconditionsFulfilledFn, } + c.deployedEncryptionConfigSecretFn = c.deployer.DeployedEncryptionConfigSecret + c.listKeySecretsFn = func(ctx context.Context) ([]*corev1.Secret, error) { + return secrets.ListKeySecrets(ctx, c.secretClient, c.encryptionSecretSelector) + } + return factory.New().ResyncEvery(time.Minute).WithSync(c.sync).WithControllerInstanceName(c.controllerInstanceName).WithInformers( operatorClient.Informer(), kubeInformersForNamespaces.InformersFor("openshift-config-managed").Core().V1().Secrets().Informer(), @@ -108,7 +118,17 @@ func (c *stateController) sync(ctx context.Context, syncCtx factory.SyncContext) return err // we will get re-kicked when the operator status updates } - configError := c.generateAndApplyCurrentEncryptionConfigSecret(ctx, syncCtx.Queue(), syncCtx.Recorder(), c.provider.EncryptedGRs()) + secretToApply, pendingEvents, configError := c.computeEncryptionConfigSecret(ctx, syncCtx.Queue()) + if configError == nil && secretToApply != nil { + _, changed, applyErr := resourceapply.ApplySecret(ctx, c.secretClient, syncCtx.Recorder(), secretToApply) + if applyErr != nil { + configError = applyErr + } else if changed { + for _, event := range pendingEvents { + syncCtx.Recorder().Eventf(event.reason, "%s", event.message) + } + } + } if configError != nil { degradedCondition = degradedCondition. WithStatus(operatorv1.ConditionTrue). @@ -126,51 +146,57 @@ type eventWithReason struct { message string } -func (c *stateController) generateAndApplyCurrentEncryptionConfigSecret(ctx context.Context, queue workqueue.RateLimitingInterface, recorder events.Recorder, encryptedGRs []schema.GroupResource) error { - currentConfig, desiredEncryptionState, encryptionSecrets, transitioningReason, err := statemachine.GetEncryptionConfigAndState(ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, encryptedGRs) +func (c *stateController) computeEncryptionConfigSecret(ctx context.Context, queue workqueue.RateLimitingInterface) (*corev1.Secret, []eventWithReason, error) { + return c.computeEncryptionConfigSecretWithCustomListKeySecretFn(ctx, queue, c.listKeySecretsFn) +} + +func (c *stateController) computeEncryptionConfigSecretWithCustomListKeySecretFn(ctx context.Context, queue workqueue.RateLimitingInterface, listKeySecretsFn func(context.Context) ([]*corev1.Secret, error)) (*corev1.Secret, []eventWithReason, error) { + return generateEncryptionConfigSecret(ctx, queue, c.provider.EncryptedGRs(), c.instanceName, c.deployedEncryptionConfigSecretFn, listKeySecretsFn) +} + +func generateEncryptionConfigSecret(ctx context.Context, queue workqueue.RateLimitingInterface, encryptedGRs []schema.GroupResource, instanceName string, deployedEncryptionConfigSecretFn func(context.Context) (*corev1.Secret, bool, error), listKeySecretsFn func(context.Context) ([]*corev1.Secret, error)) (*corev1.Secret, []eventWithReason, error) { + currentConfig, desiredEncryptionState, encryptionSecrets, transitioningReason, err := statemachine.GetEncryptionConfigAndState( + ctx, + deployedEncryptionConfigSecretFn, + listKeySecretsFn, + encryptedGRs, + ) if err != nil { - return err + return nil, nil, err } if len(transitioningReason) > 0 { queue.AddAfter(stateWorkKey, 2*time.Minute) - return nil + return nil, nil, nil } if currentConfig == nil && len(encryptionSecrets) == 0 { // we depend on the key controller to create the first key to bootstrap encryption. // Later-on either the config exists or there are keys, even in the case of disabled // encryption via the apiserver config. - return nil + return nil, nil, nil } desiredSecretData, err := encryptiondata.FromEncryptionState(desiredEncryptionState) if err != nil { - return err + return nil, nil, err } - changed, err := c.applyEncryptionConfigSecret(ctx, desiredSecretData, recorder) + secretToApply, err := applyEncryptionConfigSecret(instanceName, desiredSecretData) if err != nil { - return err + return nil, nil, err } - if changed { - currentEncryptionConfig, _ := encryptiondata.ToEncryptionState(currentConfig, encryptionSecrets) - if actionEvents := eventsFromEncryptionConfigChanges(currentEncryptionConfig, desiredEncryptionState); len(actionEvents) > 0 { - for _, event := range actionEvents { - recorder.Eventf(event.reason, "%s", event.message) - } - } - } - return nil + currentEncryptionConfig, _ := encryptiondata.ToEncryptionState(currentConfig, encryptionSecrets) + pendingEvents := eventsFromEncryptionConfigChanges(currentEncryptionConfig, desiredEncryptionState) + + return secretToApply, pendingEvents, nil } -func (c *stateController) applyEncryptionConfigSecret(ctx context.Context, secretData *encryptiondata.Config, recorder events.Recorder) (bool, error) { - s, err := encryptiondata.ToSecret("openshift-config-managed", fmt.Sprintf("%s-%s", encryptiondata.EncryptionConfSecretName, c.instanceName), secretData) +func applyEncryptionConfigSecret(instanceName string, secretData *encryptiondata.Config) (*corev1.Secret, error) { + s, err := encryptiondata.ToSecret("openshift-config-managed", fmt.Sprintf("%s-%s", encryptiondata.EncryptionConfSecretName, instanceName), secretData) if err != nil { - return false, err + return nil, err } - - _, changed, applyErr := resourceapply.ApplySecret(ctx, c.secretClient, recorder, s) - return changed, applyErr + return s, nil } // eventsFromEncryptionConfigChanges return slice of event reasons with messages corresponding to a difference between current and desired encryption state. diff --git a/pkg/operator/encryption/statemachine/transition.go b/pkg/operator/encryption/statemachine/transition.go index 12eb5b8b90..10ae48712f 100644 --- a/pkg/operator/encryption/statemachine/transition.go +++ b/pkg/operator/encryption/statemachine/transition.go @@ -5,14 +5,11 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" "github.com/openshift/library-go/pkg/operator/encryption/encryptiondata" - "github.com/openshift/library-go/pkg/operator/encryption/secrets" "github.com/openshift/library-go/pkg/operator/encryption/state" ) @@ -30,13 +27,12 @@ type Deployer interface { func GetEncryptionConfigAndState( ctx context.Context, - deployer Deployer, - secretClient corev1client.SecretsGetter, - encryptionSecretSelector metav1.ListOptions, + getDeployedEncryptionConfigSecret func(context.Context) (*corev1.Secret, bool, error), + listKeySecrets func(context.Context) ([]*corev1.Secret, error), encryptedGRs []schema.GroupResource, ) (current *encryptiondata.Config, desired map[schema.GroupResource]state.GroupResourceState, encryptionSecrets []*corev1.Secret, transitioningReason string, err error) { // get current config - encryptionConfigSecret, converged, err := deployer.DeployedEncryptionConfigSecret(ctx) + encryptionConfigSecret, converged, err := getDeployedEncryptionConfigSecret(ctx) if err != nil { return nil, nil, nil, "", err } @@ -52,7 +48,7 @@ func GetEncryptionConfigAndState( } // compute desired config - encryptionSecrets, err = secrets.ListKeySecrets(ctx, secretClient, encryptionSecretSelector) + encryptionSecrets, err = listKeySecrets(ctx) if err != nil { return nil, nil, nil, "", err }