From e05a2288ef2e07946db0b4d51c98030ce541e65d Mon Sep 17 00:00:00 2001 From: Nahshon Unna-Tsameret Date: Mon, 30 Oct 2023 12:10:03 +0200 Subject: [PATCH] KubeVirt: create the etcd encryption key secret, if missing To allow creating of KubeVirt hosted cluster using the hosted cluster API (rather than using the cli). When creating the hosted cluster using the cli, the cli also creates the secret. But when creating the hosted cluster using the hosted cluster API, the secret is not created. This PR changes hypershift so it now creates the etcd encryption key secret, if it is not already exist. Signed-off-by: Nahshon Unna-Tsameret --- .../hostedcluster/hostedcluster_controller.go | 67 ++- .../hostedcluster_controller_test.go | 400 +++++++++++++++++- 2 files changed, 453 insertions(+), 14 deletions(-) diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 5f3c682f1e..8325373921 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -15,6 +15,7 @@ package hostedcluster import ( "bytes" "context" + "crypto/rand" "crypto/x509" "crypto/x509/pkix" "errors" @@ -122,6 +123,8 @@ const ( controlPlaneOperatorManagesMachineAutoscaler = "io.openshift.hypershift.control-plane-operator-manages.cluster-autoscaler" controlPlaneOperatorAppliesManagementKASNetworkPolicyLabel = "io.openshift.hypershift.control-plane-operator-applies-management-kas-network-policy-label" useRestrictedPodSecurityLabel = "io.openshift.hypershift.restricted-psa" + + etcdEncKeyPostfix = "-etcd-encryption-key" ) var ( @@ -491,8 +494,10 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques } } + createOrUpdate := r.createOrUpdate(req) + // Reconcile platform defaults - if err := r.reconcilePlatformDefaultSettings(ctx, hcluster); err != nil { + if err := r.reconcilePlatformDefaultSettings(ctx, hcluster, createOrUpdate, log); err != nil { return ctrl.Result{}, err } @@ -1029,8 +1034,6 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques } } - createOrUpdate := r.createOrUpdate(req) - var pullSecret corev1.Secret if err := r.Client.Get(ctx, types.NamespacedName{Namespace: hcluster.Namespace, Name: hcluster.Spec.PullSecret.Name}, &pullSecret); err != nil { return ctrl.Result{}, fmt.Errorf("failed to get pull secret: %w", err) @@ -4548,7 +4551,7 @@ func (r *HostedClusterReconciler) serviceAccountSigningKeyBytes(ctx context.Cont return privateKeyPEMBytes, publicKeyPEMBytes, nil } -func (r *HostedClusterReconciler) reconcileKubevirtPlatformDefaultSettings(ctx context.Context, hc *hyperv1.HostedCluster) error { +func (r *HostedClusterReconciler) reconcileKubevirtPlatformDefaultSettings(ctx context.Context, hc *hyperv1.HostedCluster, createOrUpdate upsert.CreateOrUpdateFN, logger logr.Logger) error { if hc.Spec.Platform.Kubevirt == nil { hc.Spec.Platform.Kubevirt = &hyperv1.KubevirtPlatformSpec{} } @@ -4606,14 +4609,64 @@ func (r *HostedClusterReconciler) reconcileKubevirtPlatformDefaultSettings(ctx c } } - return nil + if hc.Spec.SecretEncryption == nil || + len(hc.Spec.SecretEncryption.Type) == 0 || + (hc.Spec.SecretEncryption.Type == hyperv1.AESCBC && + (hc.Spec.SecretEncryption.AESCBC == nil || len(hc.Spec.SecretEncryption.AESCBC.ActiveKey.Name) == 0)) { + + logger.Info("no etcd encryption key configuration found; adding", "hostedCluster name", hc.Name, "hostedCluster namespace", hc.Namespace) + etcdEncSec := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: hc.Namespace, + Name: hc.Name + etcdEncKeyPostfix, + }, + } + + _, err := createOrUpdate(ctx, r.Client, etcdEncSec, func() error { + // don't override existing key just in case something weird happened + _, exists := etcdEncSec.Data[hyperv1.AESCBCKeySecretKey] + if exists { + return nil + } + + generatedKey := make([]byte, 32) + _, err := rand.Read(generatedKey) + if err != nil { + return fmt.Errorf("failed to generate the etcd encryption key; %w", err) + } + + if etcdEncSec.Data == nil { + etcdEncSec.Data = map[string][]byte{} + } + etcdEncSec.Data[hyperv1.AESCBCKeySecretKey] = generatedKey + etcdEncSec.Type = corev1.SecretTypeOpaque + + ownerRef := config.OwnerRefFrom(hc) + ownerRef.ApplyTo(etcdEncSec) + return nil + }) + if err != nil { + return fmt.Errorf("failed to create ETCD SecretEncryption key for KubeVirt platform HostedCluster: %w", err) + } + + hc.Spec.SecretEncryption = &hyperv1.SecretEncryptionSpec{ + Type: hyperv1.AESCBC, + AESCBC: &hyperv1.AESCBCSpec{ + ActiveKey: corev1.LocalObjectReference{ + Name: etcdEncSec.Name, + }, + }, + } + } + + return nil } -func (r *HostedClusterReconciler) reconcilePlatformDefaultSettings(ctx context.Context, hc *hyperv1.HostedCluster) error { +func (r *HostedClusterReconciler) reconcilePlatformDefaultSettings(ctx context.Context, hc *hyperv1.HostedCluster, createOrUpdate upsert.CreateOrUpdateFN, logger logr.Logger) error { switch hc.Spec.Platform.Type { case hyperv1.KubevirtPlatform: - return r.reconcileKubevirtPlatformDefaultSettings(ctx, hc) + return r.reconcileKubevirtPlatformDefaultSettings(ctx, hc, createOrUpdate, logger) } return nil } diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go index 17c5b3f685..876829ea8e 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go @@ -831,7 +831,10 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) { releaseImage, _ := version.LookupDefaultOCPVersion("") hostedClusters := []*hyperv1.HostedCluster{ { - ObjectMeta: metav1.ObjectMeta{Name: "agent"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "agent", + Namespace: "test", + }, Spec: hyperv1.HostedClusterSpec{ Platform: hyperv1.PlatformSpec{ Type: hyperv1.AgentPlatform, @@ -846,7 +849,10 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) { }, }, { - ObjectMeta: metav1.ObjectMeta{Name: "aws"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "aws", + Namespace: "test", + }, Spec: hyperv1.HostedClusterSpec{ Platform: hyperv1.PlatformSpec{ Type: hyperv1.AWSPlatform, @@ -869,7 +875,10 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) { }, }, { - ObjectMeta: metav1.ObjectMeta{Name: "none"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "none", + Namespace: "test", + }, Spec: hyperv1.HostedClusterSpec{ Platform: hyperv1.PlatformSpec{ Type: hyperv1.NonePlatform, @@ -880,7 +889,10 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) { }, }, { - ObjectMeta: metav1.ObjectMeta{Name: "ibm"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "ibm", + Namespace: "test", + }, Spec: hyperv1.HostedClusterSpec{ Platform: hyperv1.PlatformSpec{ Type: hyperv1.IBMCloudPlatform, @@ -893,7 +905,8 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) { }, { ObjectMeta: metav1.ObjectMeta{ - Name: "kubevirt", + Name: "kubevirt", + Namespace: "test", Annotations: map[string]string{ hyperv1.AllowUnsupportedKubeVirtRHCOSVariantsAnnotation: "true", }, @@ -912,6 +925,14 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) { }, }, }, + SecretEncryption: &hyperv1.SecretEncryptionSpec{ + Type: hyperv1.AESCBC, + AESCBC: &hyperv1.AESCBCSpec{ + ActiveKey: corev1.LocalObjectReference{ + Name: "kubevirt" + etcdEncKeyPostfix, + }, + }, + }, Release: hyperv1.Release{ Image: releaseImage.PullSpec, }, @@ -922,7 +943,8 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) { objects := []crclient.Object{ &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "secret", + Name: "secret", + Namespace: "test", }, Data: map[string][]byte{ "credentials": []byte("creds"), @@ -944,6 +966,15 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) { &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ibm"}}, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "kubevirt"}}, &corev1.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: "kubernetes", Namespace: "default"}}, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "kubevirt" + etcdEncKeyPostfix, Namespace: "test"}, + Data: map[string][]byte{ + hyperv1.AESCBCKeySecretKey: { + 0, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, + 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, + }, + }, + }, } for _, cluster := range hostedClusters { cluster.Spec.Services = []hyperv1.ServicePublishingStrategyMapping{ @@ -986,7 +1017,8 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) { for _, hc := range hostedClusters { t.Run(hc.Name, func(t *testing.T) { - if _, err := r.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Namespace: hc.Namespace, Name: hc.Name}}); err != nil { + _, err := r.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Namespace: hc.Namespace, Name: hc.Name}}) + if err != nil { t.Fatalf("Reconcile failed: %v", err) } }) @@ -3669,3 +3701,357 @@ func TestReconcileCAPIProviderDeployment(t *testing.T) { }) } } + +func TestKubevirtETCDEncKey(t *testing.T) { + for _, testCase := range []struct { + name string + hc *hyperv1.HostedCluster + secretName string + secretExpected bool + objects []crclient.Object + }{ + { + name: "secret encryption already defined", + hc: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kubevirt", + Namespace: "test", + Annotations: map[string]string{ + hyperv1.AllowUnsupportedKubeVirtRHCOSVariantsAnnotation: "true", + }, + }, + Spec: hyperv1.HostedClusterSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.KubevirtPlatform, + Kubevirt: &hyperv1.KubevirtPlatformSpec{ + GenerateID: "123456789", + Credentials: &hyperv1.KubevirtPlatformCredentials{ + InfraNamespace: "kubevirt-kubevirt", + InfraKubeConfigSecret: &hyperv1.KubeconfigSecretRef{ + Name: "secret", + Key: "key", + }, + }, + }, + }, + SecretEncryption: &hyperv1.SecretEncryptionSpec{ + Type: hyperv1.AESCBC, + AESCBC: &hyperv1.AESCBCSpec{ + ActiveKey: corev1.LocalObjectReference{ + Name: "kubevirt" + etcdEncKeyPostfix, + }, + }, + }, + Networking: hyperv1.ClusterNetworking{ + APIServer: &hyperv1.APIServerNetworking{ + AdvertiseAddress: pointer.String("1.2.3.4"), + }, + }, + }, + }, + secretName: "kubevirt" + etcdEncKeyPostfix, + secretExpected: true, + objects: []crclient.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "kubevirt" + etcdEncKeyPostfix, Namespace: "test"}, + Data: map[string][]byte{ + hyperv1.AESCBCKeySecretKey: {1, 2, 3, 4, 5, 6, 7, 8, 9, 0}, + }, + }, + }, + }, + { + name: "secret encryption not defined", + hc: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kubevirt", + Namespace: "test", + Annotations: map[string]string{ + hyperv1.AllowUnsupportedKubeVirtRHCOSVariantsAnnotation: "true", + }, + }, + Spec: hyperv1.HostedClusterSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.KubevirtPlatform, + Kubevirt: &hyperv1.KubevirtPlatformSpec{ + GenerateID: "123456789", + Credentials: &hyperv1.KubevirtPlatformCredentials{ + InfraNamespace: "kubevirt-kubevirt", + InfraKubeConfigSecret: &hyperv1.KubeconfigSecretRef{ + Name: "secret", + Key: "key", + }, + }, + }, + }, + Networking: hyperv1.ClusterNetworking{ + APIServer: &hyperv1.APIServerNetworking{ + AdvertiseAddress: pointer.String("1.2.3.4"), + }, + }, + }, + }, + secretName: "kubevirt" + etcdEncKeyPostfix, + secretExpected: true, + }, + { + name: "secret encryption with no type", + hc: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kubevirt", + Namespace: "test", + Annotations: map[string]string{ + hyperv1.AllowUnsupportedKubeVirtRHCOSVariantsAnnotation: "true", + }, + }, + Spec: hyperv1.HostedClusterSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.KubevirtPlatform, + Kubevirt: &hyperv1.KubevirtPlatformSpec{ + GenerateID: "123456789", + Credentials: &hyperv1.KubevirtPlatformCredentials{ + InfraNamespace: "kubevirt-kubevirt", + InfraKubeConfigSecret: &hyperv1.KubeconfigSecretRef{ + Name: "secret", + Key: "key", + }, + }, + }, + }, + SecretEncryption: &hyperv1.SecretEncryptionSpec{}, + Networking: hyperv1.ClusterNetworking{ + APIServer: &hyperv1.APIServerNetworking{ + AdvertiseAddress: pointer.String("1.2.3.4"), + }, + }, + }, + }, + secretName: "kubevirt" + etcdEncKeyPostfix, + secretExpected: true, + }, + { + name: "secret encryption with no details", + hc: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kubevirt", + Namespace: "test", + Annotations: map[string]string{ + hyperv1.AllowUnsupportedKubeVirtRHCOSVariantsAnnotation: "true", + }, + }, + Spec: hyperv1.HostedClusterSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.KubevirtPlatform, + Kubevirt: &hyperv1.KubevirtPlatformSpec{ + GenerateID: "123456789", + Credentials: &hyperv1.KubevirtPlatformCredentials{ + InfraNamespace: "kubevirt-kubevirt", + InfraKubeConfigSecret: &hyperv1.KubeconfigSecretRef{ + Name: "secret", + Key: "key", + }, + }, + }, + }, + SecretEncryption: &hyperv1.SecretEncryptionSpec{ + Type: hyperv1.AESCBC, + }, + Networking: hyperv1.ClusterNetworking{ + APIServer: &hyperv1.APIServerNetworking{ + AdvertiseAddress: pointer.String("1.2.3.4"), + }, + }, + }, + }, + secretName: "kubevirt" + etcdEncKeyPostfix, + secretExpected: true, + }, + { + name: "secret encryption with no name", + hc: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kubevirt", + Namespace: "test", + Annotations: map[string]string{ + hyperv1.AllowUnsupportedKubeVirtRHCOSVariantsAnnotation: "true", + }, + }, + Spec: hyperv1.HostedClusterSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.KubevirtPlatform, + Kubevirt: &hyperv1.KubevirtPlatformSpec{ + GenerateID: "123456789", + Credentials: &hyperv1.KubevirtPlatformCredentials{ + InfraNamespace: "kubevirt-kubevirt", + InfraKubeConfigSecret: &hyperv1.KubeconfigSecretRef{ + Name: "secret", + Key: "key", + }, + }, + }, + }, + SecretEncryption: &hyperv1.SecretEncryptionSpec{ + Type: hyperv1.AESCBC, + AESCBC: &hyperv1.AESCBCSpec{}, + }, + Networking: hyperv1.ClusterNetworking{ + APIServer: &hyperv1.APIServerNetworking{ + AdvertiseAddress: pointer.String("1.2.3.4"), + }, + }, + }, + }, + secretName: "kubevirt" + etcdEncKeyPostfix, + secretExpected: true, + }, + { + name: "secret encryption with custom name", + hc: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kubevirt", + Namespace: "test", + Annotations: map[string]string{ + hyperv1.AllowUnsupportedKubeVirtRHCOSVariantsAnnotation: "true", + }, + }, + Spec: hyperv1.HostedClusterSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.KubevirtPlatform, + Kubevirt: &hyperv1.KubevirtPlatformSpec{ + GenerateID: "123456789", + Credentials: &hyperv1.KubevirtPlatformCredentials{ + InfraNamespace: "kubevirt-kubevirt", + InfraKubeConfigSecret: &hyperv1.KubeconfigSecretRef{ + Name: "secret", + Key: "key", + }, + }, + }, + }, + SecretEncryption: &hyperv1.SecretEncryptionSpec{ + Type: hyperv1.AESCBC, + AESCBC: &hyperv1.AESCBCSpec{ + ActiveKey: corev1.LocalObjectReference{ + Name: "custom-name", + }, + }, + }, + Networking: hyperv1.ClusterNetworking{ + APIServer: &hyperv1.APIServerNetworking{ + AdvertiseAddress: pointer.String("1.2.3.4"), + }, + }, + }, + }, + secretName: "custom-name", + secretExpected: false, + }, + { + name: "secret encryption not defined and secret exists with no key", + hc: &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kubevirt", + Namespace: "test", + Annotations: map[string]string{ + hyperv1.AllowUnsupportedKubeVirtRHCOSVariantsAnnotation: "true", + }, + }, + Spec: hyperv1.HostedClusterSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.KubevirtPlatform, + Kubevirt: &hyperv1.KubevirtPlatformSpec{ + GenerateID: "123456789", + Credentials: &hyperv1.KubevirtPlatformCredentials{ + InfraNamespace: "kubevirt-kubevirt", + InfraKubeConfigSecret: &hyperv1.KubeconfigSecretRef{ + Name: "secret", + Key: "key", + }, + }, + }, + }, + Networking: hyperv1.ClusterNetworking{ + APIServer: &hyperv1.APIServerNetworking{ + AdvertiseAddress: pointer.String("1.2.3.4"), + }, + }, + }, + }, + secretName: "kubevirt" + etcdEncKeyPostfix, + secretExpected: true, + objects: []crclient.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "kubevirt" + etcdEncKeyPostfix, Namespace: "test"}, + }, + }, + }, + } { + t.Run(testCase.name, func(tt *testing.T) { + testCase.objects = append(testCase.objects, testCase.hc) + client := &createTypeTrackingClient{Client: fake.NewClientBuilder(). + WithScheme(api.Scheme). + WithObjects(testCase.objects...). + Build()} + + r := &HostedClusterReconciler{ + Client: client, + Clock: clock.RealClock{}, + ManagementClusterCapabilities: fakecapabilities.NewSupportAllExcept( + capabilities.CapabilityInfrastructure, + capabilities.CapabilityIngress, + capabilities.CapabilityProxy, + ), + createOrUpdate: func(reconcile.Request) upsert.CreateOrUpdateFN { return ctrl.CreateOrUpdate }, + ReleaseProvider: &fakereleaseprovider.FakeReleaseProvider{}, + ImageMetadataProvider: &fakeimagemetadataprovider.FakeImageMetadataProvider{Result: &dockerv1client.DockerImageConfig{}}, + now: metav1.Now, + } + + if _, err := r.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Namespace: testCase.hc.Namespace, Name: testCase.hc.Name}}); err != nil { + tt.Fatalf("Reconcile failed: %v", err) + } + + if testCase.secretExpected { + secList := &corev1.SecretList{} + err := client.List(context.Background(), secList) + if err != nil { + tt.Fatalf("should create etcd encryptiuon key secret, but no secret found") + } + + if numSec := len(secList.Items); numSec != 1 { + tt.Fatalf("should create 1 secret, but found %d", numSec) + } + + sec := secList.Items[0] + if sec.Name != testCase.secretName { + tt.Errorf("secret should be with name of %q, but it's %q", testCase.secretName, secList.Items[0].Name) + } + + if _, keyExist := sec.Data[hyperv1.AESCBCKeySecretKey]; !keyExist { + tt.Errorf("the secret should contain the %q key", hyperv1.AESCBCKeySecretKey) + } + } + + hcFromTest := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: testCase.hc.Name, + Namespace: testCase.hc.Namespace, + }, + } + + err := client.Get(context.Background(), crclient.ObjectKeyFromObject(hcFromTest), hcFromTest) + if err != nil { + tt.Fatalf("should read the hosted cluster but got error; %v", err) + } + + if hcFromTest.Spec.SecretEncryption == nil || + hcFromTest.Spec.SecretEncryption.Type != hyperv1.AESCBC || + hcFromTest.Spec.SecretEncryption.AESCBC == nil || + hcFromTest.Spec.SecretEncryption.AESCBC.ActiveKey.Name != testCase.secretName { + + tt.Errorf("wrong SecretEncryption %#v", hcFromTest.Spec.SecretEncryption) + } + }, + ) + } +}