chore: Refactor NewCachedDescriptorProvider (#1695)

* remove parameter for NewCachedDescriptorProvider

* fix dead link

* adjust unit test coverage

* fix flaky test

.golangci.yaml

@@ -190,7 +190,7 @@ issues:
       linters: [ gci ] # Disable gci due to the test utilities dot import.
     - path: tests/integration/declarative/declarative_test.go
       linters: [ gci ] # Disable gci due to the test utilities dot import.
-    - path: tests/integration/controller/(controlplane|eventfilters|kyma|withwatcher|purge|mandatorymodule)/(.*)_test.go
+    - path: tests/integration/controller/(eventfilters|kyma|withwatcher|purge|mandatorymodule|kcp)/(.*)_test.go
       linters: [ gci ] # Disable gci due to the test utilities dot import.
     - linters:
       - importas

cmd/main.go

@@ -177,10 +177,11 @@ func setupManager(flagVar *flags.FlagVar, cacheOptions cache.Options, scheme *ma
 	}
 
 	sharedMetrics := metrics.NewSharedMetrics()
-	descriptorProvider := provider.NewCachedDescriptorProvider(nil)
+	descriptorProvider := provider.NewCachedDescriptorProvider()
 	kymaMetrics := metrics.NewKymaMetrics(sharedMetrics)
 	mandatoryModulesMetrics := metrics.NewMandatoryModulesMetrics()
-	setupKymaReconciler(mgr, descriptorProvider, skrContextProvider, eventRecorder, flagVar, options, skrWebhookManager, kymaMetrics,
+	setupKymaReconciler(mgr, descriptorProvider, skrContextProvider, eventRecorder, flagVar, options, skrWebhookManager,
+		kymaMetrics,
 		setupLog)
 	setupManifestReconciler(mgr, flagVar, options, sharedMetrics, mandatoryModulesMetrics, setupLog)
 	setupMandatoryModuleReconciler(mgr, descriptorProvider, flagVar, options, mandatoryModulesMetrics, setupLog)
@@ -315,7 +316,9 @@ func setupKymaReconciler(mgr ctrl.Manager,
 	}
 }
 
-func createSkrWebhookManager(mgr ctrl.Manager, skrContextFactory remote.SkrContextProvider, flagVar *flags.FlagVar) (*watcher.SKRWebhookManifestManager, error) {
+func createSkrWebhookManager(mgr ctrl.Manager, skrContextFactory remote.SkrContextProvider,
+	flagVar *flags.FlagVar,
+) (*watcher.SKRWebhookManifestManager, error) {
 	caCertificateCache := watcher.NewCACertificateCache(flagVar.CaCertCacheTTL)
 	config := watcher.SkrWebhookManagerConfig{
 		SKRWatcherPath:         flagVar.WatcherResourcesPath,

docs/user-tutorials/01-10-control-plane-quick-start.md

@@ -39,7 +39,9 @@ To use Lifecycle Manager in a local setup, you need the following prerequisites:
    kubectl apply -f https://raw.githubusercontent.com/prometheus-community/helm-charts/main/charts/kube-prometheus-stack/charts/crds/crds/crd-servicemonitors.yaml
    ```
 
-You can also follow the official [Prometheus Operator quick start](https://prometheus-operator.dev/docs/getting-started/quick-start/) guide to deploy a full set of Prometheus Operator features into your cluster.
+You can also follow the
+official [Prometheus Operator quick start](https://prometheus-operator.dev/docs/getting-started/) guide to deploy a full
+set of Prometheus Operator features into your cluster.
 
 ### Deploy Lifecycle Manager
 

internal/descriptor/provider/provider.go

@@ -17,17 +17,12 @@ var (
 )
 
 type CachedDescriptorProvider struct {
-	descriptorCache *cache.DescriptorCache
+	DescriptorCache *cache.DescriptorCache
 }
 
-func NewCachedDescriptorProvider(descriptorCache *cache.DescriptorCache) *CachedDescriptorProvider {
-	if descriptorCache != nil {
-		return &CachedDescriptorProvider{
-			descriptorCache: descriptorCache,
-		}
-	}
+func NewCachedDescriptorProvider() *CachedDescriptorProvider {
 	return &CachedDescriptorProvider{
-		descriptorCache: cache.NewDescriptorCache(),
+		DescriptorCache: cache.NewDescriptorCache(),
 	}
 }
 
@@ -49,7 +44,7 @@ func (c *CachedDescriptorProvider) GetDescriptor(template *v1beta2.ModuleTemplat
 		return desc, nil
 	}
 	key := cache.GenerateDescriptorKey(template)
-	descriptor := c.descriptorCache.Get(key)
+	descriptor := c.DescriptorCache.Get(key)
 	if descriptor != nil {
 		return descriptor, nil
 	}
@@ -75,15 +70,15 @@ func (c *CachedDescriptorProvider) Add(template *v1beta2.ModuleTemplate) error {
 		return ErrTemplateNil
 	}
 	key := cache.GenerateDescriptorKey(template)
-	descriptor := c.descriptorCache.Get(key)
+	descriptor := c.DescriptorCache.Get(key)
 	if descriptor != nil {
 		return nil
 	}
 
 	if template.Spec.Descriptor.Object != nil {
 		desc, ok := template.Spec.Descriptor.Object.(*v1beta2.Descriptor)
 		if ok && desc != nil {
-			c.descriptorCache.Set(key, desc)
+			c.DescriptorCache.Set(key, desc)
 			return nil
 		}
 	}
@@ -101,6 +96,6 @@ func (c *CachedDescriptorProvider) Add(template *v1beta2.ModuleTemplate) error {
 		return ErrTypeAssert
 	}
 
-	c.descriptorCache.Set(key, descriptor)
+	c.DescriptorCache.Set(key, descriptor)
 	return nil
 }

internal/descriptor/provider/provider_test.go

@@ -14,7 +14,7 @@ import (
 )
 
 func TestGetDescriptor_OnEmptySpec_ReturnsErrDecode(t *testing.T) {
-	descriptorProvider := provider.NewCachedDescriptorProvider(nil) // assuming it handles nil cache internally
+	descriptorProvider := provider.NewCachedDescriptorProvider()
 	template := &v1beta2.ModuleTemplate{}
 
 	_, err := descriptorProvider.GetDescriptor(template)
@@ -24,7 +24,7 @@ func TestGetDescriptor_OnEmptySpec_ReturnsErrDecode(t *testing.T) {
 }
 
 func TestAdd_OnNilTemplate_ReturnsErrTemplateNil(t *testing.T) {
-	descriptorProvider := provider.NewCachedDescriptorProvider(nil)
+	descriptorProvider := provider.NewCachedDescriptorProvider()
 
 	err := descriptorProvider.Add(nil)
 
@@ -33,7 +33,7 @@ func TestAdd_OnNilTemplate_ReturnsErrTemplateNil(t *testing.T) {
 }
 
 func TestGetDescriptor_OnNilTemplate_ReturnsErrTemplateNil(t *testing.T) {
-	descriptorProvider := provider.NewCachedDescriptorProvider(nil)
+	descriptorProvider := provider.NewCachedDescriptorProvider()
 
 	_, err := descriptorProvider.GetDescriptor(nil)
 
@@ -42,7 +42,7 @@ func TestGetDescriptor_OnNilTemplate_ReturnsErrTemplateNil(t *testing.T) {
 }
 
 func TestGetDescriptor_OnInvalidRawDescriptor_ReturnsErrDescriptorNil(t *testing.T) {
-	descriptorProvider := provider.NewCachedDescriptorProvider(nil)
+	descriptorProvider := provider.NewCachedDescriptorProvider()
 	template := builder.NewModuleTemplateBuilder().WithRawDescriptor([]byte("invalid descriptor")).WithDescriptor(nil).Build()
 
 	_, err := descriptorProvider.GetDescriptor(template)
@@ -52,8 +52,7 @@ func TestGetDescriptor_OnInvalidRawDescriptor_ReturnsErrDescriptorNil(t *testing
 }
 
 func TestGetDescriptor_OnEmptyCache_ReturnsParsedDescriptor(t *testing.T) {
-	descriptorCache := cache.NewDescriptorCache()
-	descriptorProvider := provider.NewCachedDescriptorProvider(descriptorCache)
+	descriptorProvider := provider.NewCachedDescriptorProvider()
 	template := builder.NewModuleTemplateBuilder().Build()
 
 	_, err := descriptorProvider.GetDescriptor(template)
@@ -62,7 +61,7 @@ func TestGetDescriptor_OnEmptyCache_ReturnsParsedDescriptor(t *testing.T) {
 }
 
 func TestAdd_OnInvalidRawDescriptor_ReturnsErrDecode(t *testing.T) {
-	descriptorProvider := provider.NewCachedDescriptorProvider(nil)
+	descriptorProvider := provider.NewCachedDescriptorProvider()
 	template := builder.NewModuleTemplateBuilder().WithRawDescriptor([]byte("invalid descriptor")).WithDescriptor(nil).Build()
 
 	err := descriptorProvider.Add(template)
@@ -72,7 +71,7 @@ func TestAdd_OnInvalidRawDescriptor_ReturnsErrDecode(t *testing.T) {
 }
 
 func TestAdd_OnDescriptorTypeButNull_ReturnsNoError(t *testing.T) {
-	descriptorProvider := provider.NewCachedDescriptorProvider(nil)
+	descriptorProvider := provider.NewCachedDescriptorProvider()
 	template := builder.NewModuleTemplateBuilder().WithDescriptor(&v1beta2.Descriptor{}).Build()
 
 	err := descriptorProvider.Add(template)
@@ -82,12 +81,14 @@ func TestAdd_OnDescriptorTypeButNull_ReturnsNoError(t *testing.T) {
 
 func TestGetDescriptor_OnEmptyCache_AddsDescriptorFromTemplate(t *testing.T) {
 	descriptorCache := cache.NewDescriptorCache()
-	descriptorProvider := provider.NewCachedDescriptorProvider(descriptorCache)
+	descriptorProvider := provider.CachedDescriptorProvider{DescriptorCache: descriptorCache}
 
 	expected := &v1beta2.Descriptor{
-		ComponentDescriptor: &compdesc.ComponentDescriptor{Metadata: compdesc.Metadata{
-			ConfiguredVersion: "v2",
-		}},
+		ComponentDescriptor: &compdesc.ComponentDescriptor{
+			Metadata: compdesc.Metadata{
+				ConfiguredVersion: "v2",
+			},
+		},
 	}
 	template := builder.NewModuleTemplateBuilder().WithDescriptor(expected).Build()
 

pkg/testutils/kyma.go

@@ -139,6 +139,11 @@ func EnableModule(ctx context.Context,
 	if err != nil {
 		return err
 	}
+	for _, enabledModule := range kyma.Spec.Modules {
+		if enabledModule.Name == module.Name {
+			return nil
+		}
+	}
 	kyma.Spec.Modules = append(
 		kyma.Spec.Modules, module)
 	err = clnt.Update(ctx, kyma)

pkg/testutils/moduletemplate.go

@@ -18,7 +18,7 @@ func GetModuleTemplate(ctx context.Context,
 	module v1beta2.Module,
 	defaultChannel string,
 ) (*v1beta2.ModuleTemplate, error) {
-	descriptorProvider := provider.NewCachedDescriptorProvider(nil)
+	descriptorProvider := provider.NewCachedDescriptorProvider()
 	templateLookup := templatelookup.NewTemplateLookup(clnt, descriptorProvider)
 	templateInfo := templateLookup.GetAndValidate(ctx, module.Name, module.Channel, defaultChannel)
 	if templateInfo.Err != nil {
@@ -94,7 +94,7 @@ func ReadModuleVersionFromModuleTemplate(ctx context.Context, clnt client.Client
 		return "", fmt.Errorf("failed to fetch ModuleTemplate: %w", err)
 	}
 
-	descriptorProvider := provider.NewCachedDescriptorProvider(nil)
+	descriptorProvider := provider.NewCachedDescriptorProvider()
 	ocmDesc, err := descriptorProvider.GetDescriptor(moduleTemplate)
 	if err != nil {
 		return "", fmt.Errorf("failed to get descriptor: %w", err)

tests/integration/apiwebhook/moduletemplate_crd_validation_test.go

@@ -77,7 +77,7 @@ var _ = Describe("Webhook ValidationCreate Strict", Ordered, func() {
 			WithChannel(v1beta2.DefaultChannel).
 			WithOCM(compdescv2.SchemaVersion).Build()
 		Expect(k8sClient.Create(webhookServerContext, template)).Should(Succeed())
-		descriptorProvider := provider.NewCachedDescriptorProvider(nil)
+		descriptorProvider := provider.NewCachedDescriptorProvider()
 		descriptor, err := descriptorProvider.GetDescriptor(template)
 		Expect(err).ToNot(HaveOccurred())
 		version, err := semver.NewVersion(descriptor.Version)

tests/integration/controller/eventfilters/suite_test.go

@@ -34,6 +34,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
 	"github.com/kyma-project/lifecycle-manager/api"
 	"github.com/kyma-project/lifecycle-manager/api/shared"
 	"github.com/kyma-project/lifecycle-manager/internal"
@@ -47,7 +49,6 @@ import (
 	"github.com/kyma-project/lifecycle-manager/pkg/queue"
 	testskrcontext "github.com/kyma-project/lifecycle-manager/pkg/testutils/skrcontextimpl"
 	"github.com/kyma-project/lifecycle-manager/tests/integration"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	_ "github.com/open-component-model/ocm/pkg/contexts/ocm"
 
@@ -142,7 +143,7 @@ var _ = BeforeSuite(func() {
 		SkrContextFactory:   testSkrContextFactory,
 		Event:               testEventRec,
 		RequeueIntervals:    intervals,
-		DescriptorProvider:  provider.NewCachedDescriptorProvider(nil),
+		DescriptorProvider:  provider.NewCachedDescriptorProvider(),
 		SyncRemoteCrds:      remote.NewSyncCrdsUseCase(kcpClient, testSkrContextFactory, nil),
 		InKCPMode:           false,
 		RemoteSyncNamespace: flags.DefaultRemoteSyncNamespace,

tests/integration/controller/kcp/remote_sync_test.go

@@ -15,9 +15,10 @@ import (
 	"github.com/kyma-project/lifecycle-manager/internal/pkg/flags"
 	"github.com/kyma-project/lifecycle-manager/pkg/testutils/builder"
 
-	. "github.com/kyma-project/lifecycle-manager/pkg/testutils"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+
+	. "github.com/kyma-project/lifecycle-manager/pkg/testutils"
 )
 
 var (
@@ -223,7 +224,7 @@ func buildSkrKyma() *v1beta2.Kyma {
 
 func IsDescriptorCached(template *v1beta2.ModuleTemplate) bool {
 	key := cache.GenerateDescriptorKey(template)
-	result := descriptorCache.Get(key)
+	result := descriptorProvider.DescriptorCache.Get(key)
 	return result != nil
 }
 

tests/integration/controller/kcp/suite_test.go

@@ -40,20 +40,19 @@ import (
 	"github.com/kyma-project/lifecycle-manager/internal"
 	"github.com/kyma-project/lifecycle-manager/internal/controller/kyma"
 	"github.com/kyma-project/lifecycle-manager/internal/crd"
-	"github.com/kyma-project/lifecycle-manager/internal/descriptor/cache"
 	"github.com/kyma-project/lifecycle-manager/internal/descriptor/provider"
 	"github.com/kyma-project/lifecycle-manager/internal/event"
 	"github.com/kyma-project/lifecycle-manager/internal/pkg/flags"
 	"github.com/kyma-project/lifecycle-manager/internal/pkg/metrics"
 	"github.com/kyma-project/lifecycle-manager/internal/remote"
 	"github.com/kyma-project/lifecycle-manager/pkg/log"
 	"github.com/kyma-project/lifecycle-manager/pkg/queue"
+	"github.com/kyma-project/lifecycle-manager/pkg/testutils"
 	testskrcontext "github.com/kyma-project/lifecycle-manager/pkg/testutils/skrcontextimpl"
 	"github.com/kyma-project/lifecycle-manager/tests/integration"
 
 	_ "github.com/open-component-model/ocm/pkg/contexts/ocm"
 
-	. "github.com/kyma-project/lifecycle-manager/pkg/testutils"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
@@ -71,7 +70,6 @@ var (
 	ctx                   context.Context
 	cancel                context.CancelFunc
 	cfg                   *rest.Config
-	descriptorCache       *cache.DescriptorCache
 	descriptorProvider    *provider.CachedDescriptorProvider
 	crdCache              *crd.Cache
 )
@@ -88,7 +86,7 @@ var _ = BeforeSuite(func() {
 	var err error
 	By("bootstrapping test environment")
 
-	externalCRDs, err := AppendExternalCRDs(
+	externalCRDs, err := testutils.AppendExternalCRDs(
 		filepath.Join(integration.GetProjectRoot(), "config", "samples", "tests", "crds"),
 		"cert-manager-v1.10.1.crds.yaml",
 		"istio-v1.17.1.crds.yaml")
@@ -140,8 +138,7 @@ var _ = BeforeSuite(func() {
 
 	testEventRec := event.NewRecorderWrapper(mgr.GetEventRecorderFor(shared.OperatorName))
 	testSkrContextFactory = testskrcontext.NewDualClusterFactory(kcpClient.Scheme(), testEventRec)
-	descriptorCache = cache.NewDescriptorCache()
-	descriptorProvider = provider.NewCachedDescriptorProvider(descriptorCache)
+	descriptorProvider = provider.NewCachedDescriptorProvider()
 	crdCache = crd.NewCache(nil)
 	err = (&kyma.Reconciler{
 		Client:              kcpClient,

tests/integration/controller/kyma/suite_test.go

@@ -130,7 +130,7 @@ var _ = BeforeSuite(func() {
 		Warning: 100 * time.Millisecond,
 	}
 
-	descriptorProvider = provider.NewCachedDescriptorProvider(nil)
+	descriptorProvider = provider.NewCachedDescriptorProvider()
 	kcpClient = mgr.GetClient()
 	testEventRec := event.NewRecorderWrapper(mgr.GetEventRecorderFor(shared.OperatorName))
 	testSkrContextFactory := testskrcontext.NewSingleClusterFactory(kcpClient, mgr.GetConfig(), testEventRec)

tests/integration/controller/mandatorymodule/deletion/suite_test.go

@@ -115,7 +115,7 @@ var _ = BeforeSuite(func() {
 		Warning: 100 * time.Millisecond,
 	}
 
-	descriptorProvider := provider.NewCachedDescriptorProvider(nil)
+	descriptorProvider := provider.NewCachedDescriptorProvider()
 	reconciler = &mandatorymodule.DeletionReconciler{
 		Client:             mgr.GetClient(),
 		Event:              event.NewRecorderWrapper(mgr.GetEventRecorderFor(shared.OperatorName)),

tests/integration/controller/mandatorymodule/installation/suite_test.go

@@ -106,7 +106,7 @@ var _ = BeforeSuite(func() {
 		Warning: 100 * time.Millisecond,
 	}
 
-	descriptorProvider := provider.NewCachedDescriptorProvider(nil)
+	descriptorProvider := provider.NewCachedDescriptorProvider()
 	reconciler = &mandatorymodule.InstallationReconciler{
 		Client:              mgr.GetClient(),
 		DescriptorProvider:  descriptorProvider,

tests/integration/controller/withwatcher/suite_test.go

@@ -213,7 +213,7 @@ var _ = BeforeSuite(func() {
 		Event:               testEventRec,
 		RequeueIntervals:    intervals,
 		SKRWebhookManager:   skrWebhookChartManager,
-		DescriptorProvider:  provider.NewCachedDescriptorProvider(nil),
+		DescriptorProvider:  provider.NewCachedDescriptorProvider(),
 		SyncRemoteCrds:      remote.NewSyncCrdsUseCase(kcpClient, testSkrContextFactory, nil),
 		RemoteSyncNamespace: flags.DefaultRemoteSyncNamespace,
 		InKCPMode:           true,

unit-test-coverage.yaml

@@ -1,7 +1,7 @@
 packages:
   internal/crd: 92
   internal/descriptor/cache: 93
-  internal/descriptor/provider: 68
+  internal/descriptor/provider: 66
   internal/event: 100
   internal/manifest/filemutex: 100
   internal/istio: 63