Skip to content

Commit

Permalink
cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
mwangggg committed Nov 17, 2023
1 parent 859cb05 commit 2f75557
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 46 deletions.
74 changes: 40 additions & 34 deletions internal/controllers/certmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}

Expand All @@ -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
}
Expand Down
4 changes: 3 additions & 1 deletion internal/controllers/clustercryostat_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions internal/controllers/common/finalizer_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
12 changes: 7 additions & 5 deletions internal/controllers/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,6 @@ func (r *Reconciler) reconcile(ctx context.Context, cr *model.CryostatInstance)
return reconcile.Result{}, err
}

err = r.finalizeTLS(ctx, cr)
if err != nil {
return reconcile.Result{}, err
}

err = common.RemoveFinalizer(ctx, r.Client, cr.Object, cryostatFinalizer)
if err != nil {
return reconcile.Result{}, err
Expand Down Expand Up @@ -203,6 +198,13 @@ func (r *Reconciler) reconcile(ctx context.Context, cr *model.CryostatInstance)

// Set up TLS using cert-manager, if available
var tlsConfig *resources.TLSConfig

// Finalizer for CA Cert secrets
err = r.finalizeTLS(ctx, cr)
if err != nil {
return reconcile.Result{}, err
}

if r.IsCertManagerEnabled(cr) {
tlsConfig, err = r.setupTLS(ctx, cr)
if err != nil {
Expand Down
11 changes: 7 additions & 4 deletions internal/controllers/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1865,7 +1865,7 @@ func (c *controllerTest) commonTests() {
})

It("should fail to reconcile", func() {
t.expectAlreadyOwnedError(reconcileErr, "RoleBinding", t.NewRoleBinding(nsInput.Namespace), otherInput)
t.expectAlreadyOwnedError(reconcileErr, "Secret", t.NewCACertSecret(nsInput.Namespace), otherInput)
})

It("should emit a CryostatNameConflict event", func() {
Expand Down Expand Up @@ -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))
}
}

Expand Down
12 changes: 12 additions & 0 deletions internal/test/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit 2f75557

Please sign in to comment.