Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Cleanup certificates when kyma with skip-reconciliation gets deleted after the label removal #2162

Merged
merged 9 commits into from
Jan 8, 2025
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
}
nesmabadr marked this conversation as resolved.
Show resolved Hide resolved
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)
nesmabadr marked this conversation as resolved.
Show resolved Hide resolved
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
Loading