From b83422318f96ede176d36e6658556d4a15bfabab Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Thu, 9 Nov 2023 16:36:35 -0500 Subject: [PATCH] cleanup --- internal/controllers/certmanager.go | 74 ++++++++++--------- .../clustercryostat_controller_test.go | 4 +- .../controllers/common/finalizer_utils.go | 4 +- internal/controllers/reconciler.go | 14 ++-- internal/controllers/reconciler_test.go | 9 ++- internal/test/resources.go | 12 +++ 6 files changed, 71 insertions(+), 46 deletions(-) diff --git a/internal/controllers/certmanager.go b/internal/controllers/certmanager.go index f5be84296..f2a0bda59 100644 --- a/internal/controllers/certmanager.go +++ b/internal/controllers/certmanager.go @@ -122,32 +122,42 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) ( return nil, err } + secret, err := r.GetCertificateSecret(ctx, caCert) + if err != nil { + return nil, err + } // Copy Cryostat CA secret in each target namespace for _, ns := range cr.TargetNamespaces { - secret, err := r.GetCertificateSecret(ctx, caCert) - if err != nil { - return nil, err - } - namespaceSecret := secret.DeepCopy() - namespaceSecret.Namespace = ns - err = r.createOrUpdateSecret(ctx, namespaceSecret, cr.Object, func() error { - if secret.StringData == nil { - secret.StringData = map[string]string{} + if ns != cr.InstallNamespace { + namespaceSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: caCert.Spec.SecretName, + Namespace: ns, + }, + Type: secret.Type, } - return nil - }) + err = r.createOrUpdateSecret(ctx, namespaceSecret, cr.Object, func() error { + namespaceSecret.Data = secret.Data + return nil + }) + if err != nil { + return nil, err + } + } } // Delete any Cryostat CA secrets in target namespaces that are no longer requested for _, ns := range toDelete(cr) { - namespaceSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: caCert.Spec.SecretName, - Namespace: ns, - }, - } - err = r.deleteSecret(ctx, namespaceSecret) - if err != nil { - return nil, err + if ns != cr.InstallNamespace { + namespaceSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: caCert.Spec.SecretName, + Namespace: ns, + }, + } + err = r.deleteSecret(ctx, namespaceSecret) + if err != nil { + return nil, err + } } } @@ -163,22 +173,18 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) ( func (r *Reconciler) finalizeTLS(ctx context.Context, cr *model.CryostatInstance) error { caCert := resources.NewCryostatCACert(cr) for _, ns := range cr.TargetNamespaces { - namespaceSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: caCert.Spec.SecretName, - Namespace: ns, - }, - } - err := r.deleteSecret(ctx, namespaceSecret) - if err != nil { - if kerrors.IsNotFound(err) { - r.Log.Info("secret not found, proceeding with deletion", "name", namespaceSecret.Name, "namespace", namespaceSecret.Namespace) - return nil + if ns != cr.InstallNamespace { + namespaceSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: caCert.Spec.SecretName, + Namespace: ns, + }, + } + err := r.deleteSecret(ctx, namespaceSecret) + if err != nil { + return err } - r.Log.Error(err, "failed to delete secret", "name", namespaceSecret.Name, "namespace", namespaceSecret.Namespace) - return err } - r.Log.Info("deleted secret", "name", namespaceSecret.Name, "namespace", namespaceSecret.Namespace) } return nil } diff --git a/internal/controllers/clustercryostat_controller_test.go b/internal/controllers/clustercryostat_controller_test.go index 90e0dd090..a5227600a 100644 --- a/internal/controllers/clustercryostat_controller_test.go +++ b/internal/controllers/clustercryostat_controller_test.go @@ -85,7 +85,9 @@ var _ = Describe("ClusterCryostatController", func() { *cr.TargetNamespaceStatus = targetNamespaces t.objs = append(t.objs, cr.Object, t.NewRoleBinding(targetNamespaces[0]), - t.NewRoleBinding(targetNamespaces[1])) + t.NewRoleBinding(targetNamespaces[1]), + t.NewCACertSecret(targetNamespaces[0]), + t.NewCACertSecret(targetNamespaces[1])) }) It("should create the expected main deployment", func() { t.expectMainDeployment() diff --git a/internal/controllers/common/finalizer_utils.go b/internal/controllers/common/finalizer_utils.go index dc36890fa..02bd1d671 100644 --- a/internal/controllers/common/finalizer_utils.go +++ b/internal/controllers/common/finalizer_utils.go @@ -22,7 +22,7 @@ import ( ) // AddFinalizer adds the provided finalizer to the object and updates it in the cluster -func AddFinalizer(ctx context.Context, client client.Client, obj controllerutil.Object, finalizer string) error { +func AddFinalizer(ctx context.Context, client client.Client, obj client.Object, finalizer string) error { log.Info("adding finalizer to object", "namespace", obj.GetNamespace(), "name", obj.GetName()) controllerutil.AddFinalizer(obj, finalizer) err := client.Update(ctx, obj) @@ -35,7 +35,7 @@ func AddFinalizer(ctx context.Context, client client.Client, obj controllerutil. } // RemoveFinalizer removes the provided finalizer from the object and updates it in the cluster -func RemoveFinalizer(ctx context.Context, client client.Client, obj controllerutil.Object, finalizer string) error { +func RemoveFinalizer(ctx context.Context, client client.Client, obj client.Object, finalizer string) error { log.Info("removing finalizer from object", "namespace", obj.GetNamespace(), "name", obj.GetName()) controllerutil.RemoveFinalizer(obj, finalizer) err := client.Update(ctx, obj) diff --git a/internal/controllers/reconciler.go b/internal/controllers/reconciler.go index 7cff16efd..4404b660e 100644 --- a/internal/controllers/reconciler.go +++ b/internal/controllers/reconciler.go @@ -161,6 +161,7 @@ func (r *Reconciler) reconcile(ctx context.Context, cr *model.CryostatInstance) return reconcile.Result{}, err } + // Finalizer for CA Cert secrets err = r.finalizeTLS(ctx, cr) if err != nil { return reconcile.Result{}, err @@ -203,6 +204,13 @@ func (r *Reconciler) reconcile(ctx context.Context, cr *model.CryostatInstance) // Set up TLS using cert-manager, if available var tlsConfig *resources.TLSConfig + + // Reconcile RBAC resources for Cryostat + err = r.reconcileRBAC(ctx, cr) + if err != nil { + return reconcile.Result{}, err + } + if r.IsCertManagerEnabled(cr) { tlsConfig, err = r.setupTLS(ctx, cr) if err != nil { @@ -235,12 +243,6 @@ func (r *Reconciler) reconcile(ctx context.Context, cr *model.CryostatInstance) } } - // Reconcile RBAC resources for Cryostat - err = r.reconcileRBAC(ctx, cr) - if err != nil { - return reconcile.Result{}, err - } - serviceSpecs := &resources.ServiceSpecs{ InsightsURL: r.InsightsProxy, } diff --git a/internal/controllers/reconciler_test.go b/internal/controllers/reconciler_test.go index e98ee66a9..c40f68222 100644 --- a/internal/controllers/reconciler_test.go +++ b/internal/controllers/reconciler_test.go @@ -2336,14 +2336,17 @@ func (t *cryostatTestInput) expectCertificates() { err := t.Client.Get(context.Background(), types.NamespacedName{Name: expectedSecret.Name, Namespace: expectedSecret.Namespace}, secret) Expect(err).ToNot(HaveOccurred()) t.checkMetadata(secret, expectedSecret) - Expect(secret.StringData).To(Equal(secret.StringData)) + Expect(secret.StringData).To(Equal(expectedSecret.StringData)) + // Check CA Cert secrets in each target namespace Expect(t.TargetNamespaces).ToNot(BeEmpty()) for _, ns := range t.TargetNamespaces { - caCert := t.NewCACert() + namespaceSecret := t.NewCACertSecret(ns) secret := &corev1.Secret{} - err := t.Client.Get(context.Background(), types.NamespacedName{Name: caCert.Name, Namespace: ns}, secret) + err := t.Client.Get(context.Background(), types.NamespacedName{Name: namespaceSecret.Name, Namespace: ns}, secret) Expect(err).ToNot(HaveOccurred()) + t.checkMetadata(secret, namespaceSecret) + Expect(secret.Data).To(Equal(namespaceSecret.Data)) } } diff --git a/internal/test/resources.go b/internal/test/resources.go index 79c957263..6ef0ada2d 100644 --- a/internal/test/resources.go +++ b/internal/test/resources.go @@ -880,6 +880,18 @@ func (r *TestResources) NewTestService() *corev1.Service { } } +func (r *TestResources) NewCACertSecret(ns string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: r.Name + "-ca", + Namespace: ns, + }, + Data: map[string][]byte{ + corev1.TLSCertKey: []byte(r.Name + "-ca-bytes"), + }, + } +} + func (r *TestResources) NewGrafanaSecret() *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{