Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions cmd/composition/service/skrwebhook/skr_webhook_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ var (
func ComposeSkrWebhookManager(kcpClient client.Client,
skrContextProvider remote.SkrContextProvider,
repository gateway.IstioGatewayRepository,
certificateRepository certificate.CertificateRepository,
flagVar *flags.FlagVar,
) (*watcher.SkrWebhookManifestManager, error) {
skrCertService, err := setupSKRCertService(kcpClient, flagVar)
skrCertService, err := setupSKRCertService(kcpClient, certificateRepository, flagVar)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -78,7 +79,9 @@ func ComposeSkrWebhookManager(kcpClient client.Client,
watcherMetrics)
}

func setupSKRCertService(kcpClient client.Client, flagVar *flags.FlagVar) (*certificate.Service, error) {
func ComposeCertificateRepository(kcpClient client.Client,
flagVar *flags.FlagVar,
) (certificate.CertificateRepository, error) {
certificateConfig := config.CertificateValues{
Duration: flagVar.SelfSignedCertDuration,
RenewBefore: flagVar.SelfSignedCertRenewBefore,
Expand All @@ -105,6 +108,14 @@ func setupSKRCertService(kcpClient client.Client, flagVar *flags.FlagVar) (*cert
return nil, fmt.Errorf("failed to create certificate repository: %w", err)
}

return certRepoImpl, nil
}

func setupSKRCertService(kcpClient client.Client,
certificateRepository certificate.CertificateRepository,
flagVar *flags.FlagVar,
) (*certificate.Service, error) {

certServiceConfig := certificate.Config{
SkrServiceName: skrwebhookresources.SkrResourceName,
SkrNamespace: flagVar.RemoteSyncNamespace,
Expand All @@ -126,5 +137,5 @@ func setupSKRCertService(kcpClient client.Client, flagVar *flags.FlagVar) (*cert
return nil, errCertificateManagementNotSupported
}

return certificate.NewService(renewalService, certRepoImpl, secretRepository, certServiceConfig), nil
return certificate.NewService(renewalService, certificateRepository, secretRepository, certServiceConfig), nil
}
28 changes: 23 additions & 5 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"net/http"
"net/http/pprof"
"os"
"reflect"
"strings"
"time"

Expand Down Expand Up @@ -88,6 +89,7 @@
skrcrdrepo "github.com/kyma-project/lifecycle-manager/internal/repository/skr/crd"
skrkymarepo "github.com/kyma-project/lifecycle-manager/internal/repository/skr/kyma"
skrkymastatusrepo "github.com/kyma-project/lifecycle-manager/internal/repository/skr/kyma/status"
"github.com/kyma-project/lifecycle-manager/internal/repository/skr/webhook"
resultevent "github.com/kyma-project/lifecycle-manager/internal/result/event"
"github.com/kyma-project/lifecycle-manager/internal/result/kyma/usecase"
"github.com/kyma-project/lifecycle-manager/internal/service/accessmanager"
Expand All @@ -100,6 +102,7 @@
"github.com/kyma-project/lifecycle-manager/internal/service/skrclient"
skrclientcache "github.com/kyma-project/lifecycle-manager/internal/service/skrclient/cache"
"github.com/kyma-project/lifecycle-manager/internal/service/skrsync"
"github.com/kyma-project/lifecycle-manager/internal/service/watcher/certificate"
"github.com/kyma-project/lifecycle-manager/internal/setup"
"github.com/kyma-project/lifecycle-manager/pkg/log"
"github.com/kyma-project/lifecycle-manager/pkg/matcher"
Expand Down Expand Up @@ -215,11 +218,20 @@
accessManagerService,
flagVar.SkrClientQPS,
flagVar.SkrClientBurst)

certificateRepository, err := skrwebhook.ComposeCertificateRepository(kcpClient, flagVar)
t := reflect.TypeOf(certificateRepository)
logger.Info("certificate repository", "type", t)
if err != nil {
logger.Error(err, "failed to setup certificate repository")
os.Exit(bootstrapFailedExitCode)
}
var skrWebhookManager *watcher.SkrWebhookManifestManager
var options ctrlruntime.Options
if flagVar.EnableKcpWatcher {
skrWebhookManager, err = skrwebhook.ComposeSkrWebhookManager(kcpClient, skrContextProvider,
gatewayRepository,
certificateRepository,
flagVar)
if err != nil {
logger.Error(err, "failed to setup SKR webhook manager")
Expand Down Expand Up @@ -280,8 +292,7 @@

setupKymaReconciler(mgr, descriptorProvider, skrContextProvider, eventRecorder, flagVar, options, skrWebhookManager,
kymaMetrics, logger, maintenanceWindow, ociRegistryHost, accessSecretRepository, remoteClientCache,
kymaRepo,
)
certificateRepository, kymaRepo)
setupManifestReconciler(mgr, flagVar, options, sharedMetrics, mandatoryModulesMetrics, accessManagerService, logger,
eventRecorder, kymaRepo)
setupMandatoryModuleReconciler(mgr, descriptorProvider, flagVar, options, mandatoryModulesMetrics, logger,
Expand Down Expand Up @@ -427,11 +438,12 @@
setupLog.V(log.DebugLevel).Info("scheduled job for cleaning up metrics")
}

func setupKymaReconciler(mgr ctrl.Manager, descriptorProvider *provider.CachedDescriptorProvider,

Check failure on line 441 in cmd/main.go

View workflow job for this annotation

GitHub Actions / lint

Function 'setupKymaReconciler' is too long (114 > 80) (funlen)
skrContextFactory remote.SkrContextProvider, event event.Event, flagVar *flags.FlagVar, options ctrlruntime.Options,
skrWebhookManager *watcher.SkrWebhookManifestManager, kymaMetrics *metrics.KymaMetrics,
setupLog logr.Logger, maintenanceWindow maintenancewindows.MaintenanceWindow, ociRegistryHost string,
accessSecretRepository *secretrepo.Repository, skrClientCache *remote.ClientCache, kymaRepo *kymarepo.Repository,
accessSecretRepository *secretrepo.Repository, skrClientCache *remote.ClientCache,
certificateRepository certificate.CertificateRepository, kymaRepo *kymarepo.Repository,
) {
options.RateLimiter = internal.RateLimiter(flagVar.FailureBaseDelay,
flagVar.FailureMaxDelay, flagVar.RateLimiterFrequency, flagVar.RateLimiterBurst)
Expand All @@ -451,10 +463,11 @@
OCIRegistryHost: ociRegistryHost,
SkrImagePullSecretName: flagVar.SkrImagePullSecret,
}
kcpSecretRepo := secretrepo.NewRepository(kcpClient, shared.DefaultControlPlaneNamespace)
syncCrdsUseCase := remote.NewSyncCrdsUseCase(kcpClient, skrContextFactory, nil)
skrSyncService := skrsync.NewService(
skrContextFactory,
secretrepo.NewRepository(kcpClient, shared.DefaultControlPlaneNamespace),
kcpSecretRepo,
&syncCrdsUseCase,
flagVar.SkrImagePullSecret)

Expand Down Expand Up @@ -489,11 +502,16 @@
usecase.DeleteSkrKymaCrd)
deleteMetrics := usecases.NewDeleteMetrics(kymaMetrics)
dropKymaFinalizers := usecases.NewDropKymaFinalizers(kymaRepo)

skrWebhookResourcesRepo := webhook.NewResourceRepository(skrClientCache, shared.DefaultRemoteNamespace,
skrWebhookManager.BaseResources)
removeSkrWebhook := usecases.NewRemoveSkrWebhookResources(skrWebhookResourcesRepo)
certificateCleanup := usecases.NewWatcherCertificateCleanup(certificateRepository, kcpSecretRepo)
kymaDeletionService := kymadeletionsvc.NewService(
setKcpKymaStateDeleting,
setSkrKymaStateDeleting,
deleteSkrKyma,
certificateCleanup,
removeSkrWebhook,
deleteSkrMtCrd,
deleteSkrMrmCrd,
deleteSkrKymaCrd,
Expand Down
7 changes: 3 additions & 4 deletions internal/controller/kyma/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,11 @@ func (r *Reconciler) processDeletion(ctx context.Context, kyma *v1beta2.Kyma) (c
case usecase.SetKcpKymaStateDeleting,
usecase.SetSkrKymaStateDeleting,
usecase.DeleteSkrKyma,
usecase.DeleteSkrWatcher,
usecase.DeleteWatcherCertificateSetup,
usecase.DeleteSkrWebhookResources,
usecase.DeleteSkrModuleTemplateCrd,
usecase.DeleteSkrModuleReleaseMetaCrd,
usecase.DeleteSkrKymaCrd,
usecase.DeleteWatcherCertificate,
usecase.DeleteManifests,
usecase.DeleteMetrics:
// error takes precedence over the RequeueAfter
Expand Down Expand Up @@ -568,8 +568,6 @@ func checkSKRWebhookReadiness(ctx context.Context, skrClient *remote.SkrContext,
}

func (r *Reconciler) handleDeletingState(ctx context.Context, kyma *v1beta2.Kyma) (ctrl.Result, error) {
logger := logf.FromContext(ctx).V(log.InfoLevel)

if r.WatcherEnabled() {
if err := r.SKRWebhookManager.Remove(ctx, kyma); err != nil {
return ctrl.Result{}, err
Expand All @@ -592,6 +590,7 @@ func (r *Reconciler) handleDeletingState(ctx context.Context, kyma *v1beta2.Kyma
return r.requeueWithError(ctx, kyma, err)
}

logger := logf.FromContext(ctx).V(log.InfoLevel)
logger.Info("removed remote finalizers")

if err := r.cleanupManifestCRs(ctx, kyma); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/kyma/deletion/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ func (w *MetricWriter) Write(res result.Result) {
w.metrics.RecordRequeueReason(metrics.RemoteModuleCatalogDeletion, requeueType)
case usecase.DeleteManifests:
w.metrics.RecordRequeueReason(metrics.CleanupManifestCrs, requeueType)
case usecase.DeleteSkrWatcher,
case usecase.DeleteWatcherCertificateSetup,
usecase.DeleteSkrWebhookResources,
usecase.DeleteSkrKymaCrd,
usecase.DeleteWatcherCertificate,
usecase.DeleteMetrics,
usecase.DropKymaFinalizers:
// These use cases are not tracked by metrics.
Expand Down
6 changes: 3 additions & 3 deletions internal/controller/kyma/deletion/metric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func TestMetricWriter_Write(t *testing.T) {
// No calls
{
name: "Nothing for UseCaseDeleteSkrWatcher",
useCase: usecase.DeleteSkrWatcher,
useCase: usecase.DeleteSkrWebhookResources,
err: nil,
expectedCall: false,
},
Expand All @@ -148,8 +148,8 @@ func TestMetricWriter_Write(t *testing.T) {
expectedCall: false,
},
{
name: "Nothing for UseCaseDeleteWatcherCertificate",
useCase: usecase.DeleteWatcherCertificate,
name: "Nothing for DeleteWatcherCertificateSetup",
useCase: usecase.DeleteWatcherCertificateSetup,
err: nil,
expectedCall: false,
},
Expand Down
188 changes: 188 additions & 0 deletions internal/repository/skr/webhook/skrwebhook_repo.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
package webhook

import (
"context"
"fmt"

"golang.org/x/sync/errgroup"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/apis/meta/v1beta1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kyma-project/lifecycle-manager/internal/errors"
skrwebhookresources "github.com/kyma-project/lifecycle-manager/internal/service/watcher/resources"
"github.com/kyma-project/lifecycle-manager/pkg/util"
)

type SkrClientCache interface {
Get(key client.ObjectKey) client.Client
}

//// resourceRef contains the minimal information needed to identify and delete a resource.
//type resourceRef struct {

Check failure on line 26 in internal/repository/skr/webhook/skrwebhook_repo.go

View workflow job for this annotation

GitHub Actions / lint

commentFormatting: put a space between `//` and comment text (gocritic)
// Name string
// APIVersion string
// Kind string
//}

type ResourceRepository struct {
resources []v1beta1.PartialObjectMetadata
skrClientCache SkrClientCache
remoteSyncNamespace string
}

func NewResourceRepository(
skrClientCache SkrClientCache,
remoteSyncNamespace string,
baseResources []*unstructured.Unstructured,
) *ResourceRepository {
resources := make([]v1beta1.PartialObjectMetadata, 0, len(baseResources)+2)
for _, res := range baseResources {
meta := v1beta1.PartialObjectMetadata{
TypeMeta: apimetav1.TypeMeta{
Kind: res.GetKind(),
APIVersion: res.GetAPIVersion(),
},
ObjectMeta: apimetav1.ObjectMeta{
Name: res.GetName(),
Namespace: remoteSyncNamespace,
},
}
resources = append(resources, meta)
}

// Add generated resources that are created dynamically from SkrWebhookManifestManager (not read from resources.yaml)
resources = append(resources,
v1beta1.PartialObjectMetadata{
TypeMeta: apimetav1.TypeMeta{
Kind: "ValidatingWebhookConfiguration",
APIVersion: admissionregistrationv1.SchemeGroupVersion.String(),
},
ObjectMeta: apimetav1.ObjectMeta{
Name: skrwebhookresources.SkrResourceName,
Namespace: remoteSyncNamespace,
},
},
v1beta1.PartialObjectMetadata{
TypeMeta: apimetav1.TypeMeta{
Kind: "Secret",
APIVersion: "v1",
},
ObjectMeta: apimetav1.ObjectMeta{
Name: skrwebhookresources.SkrTLSName,
Namespace: remoteSyncNamespace,
},
},
)

return &ResourceRepository{
resources: resources,
skrClientCache: skrClientCache,
remoteSyncNamespace: remoteSyncNamespace,
}
}

// ResourcesExist checks if any of the skr-webhook resources exist in the SKR cluster for the given Kyma.
func (r *ResourceRepository) ResourcesExist(kymaName string) (bool, error) {
skrClient, err := r.getSkrClient(types.NamespacedName{
Name: kymaName,
Namespace: r.remoteSyncNamespace,
})
if err != nil {
return false, err
}

ctx, cancel := context.WithCancel(context.Background())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the existing context to the ResourcesExist func signature instaed of creating a new background context?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't want to pollute the signature of this function with ctx and cancel .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't have a cancel from outside anyway. So would only be the ctx we receive from the Reconcile func, and we add the cancel within the func

defer cancel()

errGrp, grpCtx := errgroup.WithContext(ctx)
resourceExists := make(chan bool, 1)

for resIdx := range r.resources {
errGrp.Go(func() error {
select {
case <-grpCtx.Done():
// Short-circuit if context is cancelled (resource already found)
return nil
default:
}

ref := r.resources[resIdx]
err := skrClient.Get(grpCtx, client.ObjectKeyFromObject(&ref), &unstructured.Unstructured{})
if err != nil {
if apierrors.IsNotFound(err) {
// Resource does not exist, continue checking others
return nil
}
return fmt.Errorf("failed to check resource %s: %w", ref.Name, err)
}
select {
case resourceExists <- true:
cancel()
default:
}
return nil
})
}

if err := errGrp.Wait(); err != nil {
return false, err
}

select {
case <-resourceExists:
return true, nil
default:
return false, nil
}
}

// DeleteWebhookResources deletes all skr-webhook resources from the SKR cluster for the given Kyma.
func (r *ResourceRepository) DeleteWebhookResources(ctx context.Context, kymaName string) error {
skrClient, err := r.getSkrClient(types.NamespacedName{
Name: kymaName,
Namespace: r.remoteSyncNamespace,
})
if err != nil {
return err
}

errGrp, grpCtx := errgroup.WithContext(ctx)

for resIdx := range r.resources {
errGrp.Go(func() error {
ref := r.resources[resIdx]
//resource := &unstructured.Unstructured{}

Check failure on line 159 in internal/repository/skr/webhook/skrwebhook_repo.go

View workflow job for this annotation

GitHub Actions / lint

commentFormatting: put a space between `//` and comment text (gocritic)
//resource.SetGroupVersionKind(schema.FromAPIVersionAndKind(ref.APIVersion, ref.Kind))
//resource.SetName(ref.Name)
//resource.SetNamespace(r.remoteSyncNamespace)
err := skrClient.Delete(grpCtx, &ref)
if client.IgnoreNotFound(err) != nil {
return fmt.Errorf("failed to delete resource %s: %w", ref.Name, err)
}
return nil
})
}

if err := errGrp.Wait(); err != nil && !util.IsNotFound(err) {
return fmt.Errorf("failed to delete webhook resources: %w", err)
}

return nil
}

func (r *ResourceRepository) getSkrClient(kymaName types.NamespacedName) (client.Client, error) {
skrClient := r.skrClientCache.Get(kymaName)

if skrClient == nil {
// TODO if multiple Repositories are using this error generation, consider creating a shared error instance
// or even provide this get functionality with error on the skrClientCache itself
return nil, fmt.Errorf("%w: Kyma %s", errors.ErrSkrClientNotFound, kymaName.String())
}

return skrClient, nil
}
Loading
Loading