Skip to content

Commit

Permalink
fix: Cleanup certificates when kyma with skip-reconciliation gets del…
Browse files Browse the repository at this point in the history
…eted after the label removal (#2162)

* Add E2E test

* Fix test

* test with foreground deletion

* test with foreground deletion

* deletion logic

* fix cleanup
  • Loading branch information
nesmabadr authored Jan 8, 2025
1 parent f606b59 commit a4c34ce
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 11 deletions.
11 changes: 11 additions & 0 deletions internal/controller/kyma/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
if util.IsNotFound(err) {
logger.V(log.DebugLevel).Info(fmt.Sprintf("Kyma %s not found, probably already deleted",
req.NamespacedName))
if err = r.handleOrphanedResourcesDeletion(ctx, req.Name); err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{Requeue: false}, nil
}
r.Metrics.RecordRequeueReason(metrics.KymaRetrieval, queue.UnexpectedRequeue)
Expand Down Expand Up @@ -624,6 +627,14 @@ func (r *Reconciler) UpdateModuleTemplatesIfNeeded(ctx context.Context) error {
return nil
}

func (r *Reconciler) handleOrphanedResourcesDeletion(ctx context.Context, kymaName string) error {
if err := r.SKRWebhookManager.RemoveKCPCertificate(ctx, kymaName); err != nil {
return err
}

return nil
}

func needUpdateForMandatoryModuleLabel(moduleTemplate v1beta2.ModuleTemplate) bool {
if moduleTemplate.Labels == nil {
moduleTemplate.Labels = make(map[string]string)
Expand Down
19 changes: 19 additions & 0 deletions pkg/testutils/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kyma-project/lifecycle-manager/api/shared"
"github.com/kyma-project/lifecycle-manager/pkg/util"
)

var (
ErrSecretNotFound = errors.New("secret does not exist")
ErrCertificateNotFound = errors.New("certificate does not exist")
errOldCreationTime = errors.New("certificate has an old creation timestamp")
errNotSyncedSecret = errors.New("secrets are not synced")
errTLSSecretNotRotated = errors.New("tls secret did not rotated")
Expand All @@ -26,13 +29,29 @@ var (
func CertificateSecretExists(ctx context.Context, secretName types.NamespacedName, k8sClient client.Client) error {
certificateSecret := &apicorev1.Secret{}
err := k8sClient.Get(ctx, secretName, certificateSecret)
if util.IsNotFound(err) {
return ErrSecretNotFound
}
if err != nil {
return fmt.Errorf("failed to get certificate secret %w", err)
}

return nil
}

func CertificateExists(ctx context.Context, certificateName types.NamespacedName, k8sClient client.Client) error {
certificate := &certmanagerv1.Certificate{}
err := k8sClient.Get(ctx, certificateName, certificate)
if util.IsNotFound(err) {
return ErrCertificateNotFound
}
if err != nil {
return fmt.Errorf("failed to get certificate %w", err)
}

return nil
}

func CertificateSecretIsCreatedAfter(ctx context.Context,
secretName types.NamespacedName, k8sClient client.Client, notBeforeTime *apimetav1.Time,
) error {
Expand Down
4 changes: 2 additions & 2 deletions pkg/watcher/certificate_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,15 @@ func (c *CertificateManager) CreateSelfSignedCert(ctx context.Context, kyma *v1b

// Remove removes the certificate including its certificate secret.
func (c *CertificateManager) Remove(ctx context.Context) error {
err := c.RemoveCertificate(ctx)
err := c.removeCertificate(ctx)
if err != nil {
return err
}

return c.removeSecret(ctx)
}

func (c *CertificateManager) RemoveCertificate(ctx context.Context) error {
func (c *CertificateManager) removeCertificate(ctx context.Context) error {
certificate := &certmanagerv1.Certificate{}
err := c.kcpClient.Get(ctx, client.ObjectKey{
Name: c.certificateName,
Expand Down
16 changes: 13 additions & 3 deletions pkg/watcher/skr_webhook_manifest_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,11 @@ func (m *SKRWebhookManifestManager) Remove(ctx context.Context, kyma *v1beta2.Ky
if err != nil {
return fmt.Errorf("failed to get skrContext: %w", err)
}
certificate := NewCertificateManager(m.kcpClient, kyma.Name,
m.certificateConfig)
if err = certificate.Remove(ctx); err != nil {

if err = m.RemoveKCPCertificate(ctx, kyma.Name); err != nil {
return err
}

skrClientObjects := m.getBaseClientObjects()
genClientObjects := getGeneratedClientObjects(&unstructuredResourcesConfig{}, []v1beta2.Watcher{},
m.config.RemoteSyncNamespace)
Expand All @@ -172,6 +172,16 @@ func (m *SKRWebhookManifestManager) Remove(ctx context.Context, kyma *v1beta2.Ky
return nil
}

func (m *SKRWebhookManifestManager) RemoveKCPCertificate(ctx context.Context, kymaName string) error {
certificate := NewCertificateManager(m.kcpClient, kymaName,
m.certificateConfig)
if err := certificate.Remove(ctx); err != nil {
return err
}

return nil
}

func (m *SKRWebhookManifestManager) getSKRClientObjectsForInstall(ctx context.Context,
kymaObjKey client.ObjectKey, remoteNs string, gatewaySecret *apicorev1.Secret, logger logr.Logger,
) ([]client.Object, error) {
Expand Down
10 changes: 5 additions & 5 deletions tests/e2e/deprovision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ package e2e_test
import (
"os/exec"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

. "github.com/kyma-project/lifecycle-manager/pkg/testutils"

"github.com/kyma-project/lifecycle-manager/api/shared"
"github.com/kyma-project/lifecycle-manager/api/v1beta2"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

. "github.com/kyma-project/lifecycle-manager/pkg/testutils"
)

func RunDeletionTest(deletionPropagation apimetav1.DeletionPropagation) {
Expand Down
61 changes: 60 additions & 1 deletion tests/e2e/watcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"os/exec"
"time"

apiappsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -34,7 +35,6 @@ var _ = Describe("Enqueue Event from Watcher", Ordered, func() {
kyma.GetNamespace(), kyma.GetName())

InitEmptyKymaBeforeAll(kyma)
CleanupKymaAfterAll(kyma)
secretName := types.NamespacedName{
Name: watcher.SkrTLSName,
Namespace: RemoteNamespace,
Expand Down Expand Up @@ -118,6 +118,65 @@ var _ = Describe("Enqueue Event from Watcher", Ordered, func() {
kcpClient, patchingTimestamp).
Should(Succeed())
})

It("When SKR Cluster is removed", func() {
cmd := exec.Command("sh", "../../scripts/tests/remove_skr_host_from_coredns.sh")
out, err := cmd.CombinedOutput()
Expect(err).NotTo(HaveOccurred())
GinkgoWriter.Printf(string(out))
cmd = exec.Command("k3d", "cluster", "rm", "skr")
out, err = cmd.CombinedOutput()
Expect(err).NotTo(HaveOccurred())
GinkgoWriter.Printf(string(out))

By("And skip-reconciliation label is added to KCP Kyma CR")
Eventually(UpdateKymaLabel).
WithContext(ctx).
WithArguments(kcpClient, kyma.GetName(), kyma.GetNamespace(), shared.SkipReconcileLabel,
shared.EnableLabelValue).
Should(Succeed())

By("And KCP Kyma CR is deleted")
Eventually(DeleteKyma).
WithContext(ctx).
WithArguments(kcpClient, kyma, apimetav1.DeletePropagationBackground).
Should(Succeed())

By("And Kubeconfig Secret is deleted")
Eventually(DeleteKymaSecret).
WithContext(ctx).
WithArguments(kyma.GetName(), kyma.GetNamespace(), kcpClient).
Should(Succeed())

By("And skip-reconciliation label is removed from KCP Kyma CR")
Eventually(UpdateKymaLabel).
WithContext(ctx).
WithArguments(kcpClient, kyma.GetName(), kyma.GetNamespace(), shared.SkipReconcileLabel,
shared.DisableLabelValue).
Should(Succeed())
})

It("Then KCP Kyma CR is deleted", func() {
Eventually(KymaDeleted).
WithContext(ctx).
WithArguments(kyma.GetName(), kyma.GetNamespace(), kcpClient).
Should(Succeed())

By("And KCP TLS Certificate and Secret are deleted")
secretNamespacedName := types.NamespacedName{
Name: watcher.ResolveTLSCertName(kyma.Name),
Namespace: IstioNamespace,
}
Eventually(CertificateSecretExists).
WithContext(ctx).
WithArguments(secretNamespacedName, kcpClient).
Should(Equal(ErrSecretNotFound))

Eventually(CertificateExists).
WithContext(ctx).
WithArguments(secretNamespacedName, kcpClient).
Should(Equal(ErrCertificateNotFound))
})
})
})

Expand Down

0 comments on commit a4c34ce

Please sign in to comment.