From f025813b80f16f126d2c18f5142b3cbf6da5f0b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20Schw=C3=A4gerl?= Date: Thu, 23 Jan 2025 12:48:53 +0100 Subject: [PATCH 1/3] chore: Add version and name helpers to ModuleTemplate (#2201) * chore: Add version and name helpers to ModuleTemplate * fix lint * fix lint ++ --- api/v1beta2/moduletemplate_types.go | 50 ++++--- api/v1beta2/moduletemplate_types_test.go | 131 ++++++++++++++++-- internal/descriptor/cache/key.go | 2 +- .../maintenancewindows/maintenance_window.go | 5 +- 4 files changed, 147 insertions(+), 41 deletions(-) diff --git a/api/v1beta2/moduletemplate_types.go b/api/v1beta2/moduletemplate_types.go index 2fc4b1360d..9c758ce316 100644 --- a/api/v1beta2/moduletemplate_types.go +++ b/api/v1beta2/moduletemplate_types.go @@ -223,39 +223,27 @@ func (m *ModuleTemplate) IsInternal() bool { return false } -var ErrInvalidVersion = errors.New("can't find valid semantic version") - -// getVersionLegacy() returns the version of the ModuleTemplate from the annotation on the object. -// Remove once shared.ModuleVersionAnnotation is removed. -func (m *ModuleTemplate) getVersionLegacy() (string, error) { - if m.Annotations != nil { - moduleVersion, found := m.Annotations[shared.ModuleVersionAnnotation] - if found { - return moduleVersion, nil - } +// https://github.com/kyma-project/lifecycle-manager/issues/2096 +// Refactor this function to drop the label fallback after the migration to the new ModuleTemplate format is completed. +func (m *ModuleTemplate) GetVersion() string { + version := m.Spec.Version + if version == "" { + version = m.Annotations[shared.ModuleVersionAnnotation] } - return "", ErrInvalidVersion + return version } -// GetVersion returns the declared version of the ModuleTemplate from it's Spec. -func (m *ModuleTemplate) GetVersion() (*semver.Version, error) { - var versionValue string - var err error - - if m.Spec.Version == "" { - versionValue, err = m.getVersionLegacy() - if err != nil { - return nil, err - } - } else { - versionValue = m.Spec.Version - } +var ErrInvalidVersion = errors.New("can't find valid semantic version") - version, err := semver.NewVersion(versionValue) +// GetSemanticVersion returns the declared version of the ModuleTemplate as semantic version. +func (m *ModuleTemplate) GetSemanticVersion() (*semver.Version, error) { + version := m.GetVersion() + + semanticVersion, err := semver.NewVersion(version) if err != nil { return nil, fmt.Errorf("%w: %s", ErrInvalidVersion, err.Error()) } - return version, nil + return semanticVersion, nil } // https://github.com/kyma-project/lifecycle-manager/issues/2096 @@ -279,3 +267,13 @@ func (m *ModuleTemplate) HasSyncDisabled() bool { } return false } + +// https://github.com/kyma-project/lifecycle-manager/issues/2096 +// Refactor this function to drop the label fallback after the migration to the new ModuleTemplate format is completed. +func (m *ModuleTemplate) GetModuleName() string { + moduleName := m.Spec.ModuleName + if moduleName == "" { + moduleName = m.Labels[shared.ModuleName] + } + return moduleName +} diff --git a/api/v1beta2/moduletemplate_types_test.go b/api/v1beta2/moduletemplate_types_test.go index b02b375e20..afaeafe7b9 100644 --- a/api/v1beta2/moduletemplate_types_test.go +++ b/api/v1beta2/moduletemplate_types_test.go @@ -4,13 +4,14 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" apimetav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" ) -func Test_GetVersion(t *testing.T) { +func Test_GetSemanticVersion(t *testing.T) { const testVersion = "1.0.1" const otherVersion = "0.0.1" tests := []struct { @@ -20,7 +21,7 @@ func Test_GetVersion(t *testing.T) { expectedErr string }{ { - name: "Test GetVersion() by annotation (legacy)", + name: "Test GetSemanticVersion() by annotation (legacy)", m: &v1beta2.ModuleTemplate{ ObjectMeta: apimetav1.ObjectMeta{ Annotations: map[string]string{ @@ -31,7 +32,7 @@ func Test_GetVersion(t *testing.T) { expectedVersion: testVersion, }, { - name: "Test GetVersion() by explicit version in Spec", + name: "Test GetSemanticVersion() by explicit version in Spec", m: &v1beta2.ModuleTemplate{ Spec: v1beta2.ModuleTemplateSpec{ Version: testVersion, @@ -40,7 +41,7 @@ func Test_GetVersion(t *testing.T) { expectedVersion: testVersion, }, { - name: "Test GetVersion() with both version in Spec and annotation", + name: "Test GetSemanticVersion() with both version in Spec and annotation", m: &v1beta2.ModuleTemplate{ ObjectMeta: apimetav1.ObjectMeta{ Annotations: map[string]string{ @@ -75,32 +76,140 @@ func Test_GetVersion(t *testing.T) { } for _, testCase := range tests { t.Run(testCase.name, func(t *testing.T) { - actualVersion, err := testCase.m.GetVersion() + actualVersion, err := testCase.m.GetSemanticVersion() if err != nil { if actualVersion != nil { - t.Errorf("GetVersion(): Returned version should be nil when error is not nil") + t.Errorf("GetSemanticVersion(): Returned version should be nil when error is not nil") } if testCase.expectedErr == "" { - t.Errorf("GetVersion(): Unexpected error: %v", err) + t.Errorf("GetSemanticVersion(): Unexpected error: %v", err) } if !strings.Contains(err.Error(), testCase.expectedErr) { - t.Errorf("GetVersion(): Actual error = %v, expected error: %v", err, testCase.expectedErr) + t.Errorf("GetSemanticVersion(): Actual error = %v, expected error: %v", err, testCase.expectedErr) } return } if actualVersion == nil { - t.Errorf("GetVersion(): Returned version should not be nil when error is nil") + t.Errorf("GetSemanticVersion(): Returned version should not be nil when error is nil") } if testCase.expectedVersion == "" { - t.Errorf("GetVersion(): Expected version is empty but non-nil version is returned") + t.Errorf("GetSemanticVersion(): Expected version is empty but non-nil version is returned") } if actualVersion != nil && actualVersion.String() != testCase.expectedVersion { - t.Errorf("GetVersion(): actual version = %v, expected version: %v", actualVersion.String(), + t.Errorf("GetSemanticVersion(): actual version = %v, expected version: %v", actualVersion.String(), testCase.expectedVersion) } }) } } + +//nolint:dupl // similar but not duplicate +func Test_GetVersion(t *testing.T) { + tests := []struct { + name string + m *v1beta2.ModuleTemplate + expectedVersion string + }{ + { + name: "Test GetVersion() by annotation (legacy)", + m: &v1beta2.ModuleTemplate{ + ObjectMeta: apimetav1.ObjectMeta{ + Annotations: map[string]string{ + shared.ModuleVersionAnnotation: "1.0.0-annotated", + }, + }, + Spec: v1beta2.ModuleTemplateSpec{}, + }, + expectedVersion: "1.0.0-annotated", + }, + { + name: "Test GetVersion() by spec.version", + m: &v1beta2.ModuleTemplate{ + ObjectMeta: apimetav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + Spec: v1beta2.ModuleTemplateSpec{ + Version: "2.0.0-spec", + }, + }, + expectedVersion: "2.0.0-spec", + }, + { + name: "Test GetVersion() spec.moduleName has priority over annotation", + m: &v1beta2.ModuleTemplate{ + ObjectMeta: apimetav1.ObjectMeta{ + Annotations: map[string]string{ + shared.ModuleVersionAnnotation: "1.0.0-annotated", + }, + }, + Spec: v1beta2.ModuleTemplateSpec{ + Version: "2.0.0-spec", + }, + }, + expectedVersion: "2.0.0-spec", + }, + } + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + actualVersion := testCase.m.GetVersion() + assert.Equal(t, testCase.expectedVersion, actualVersion) + }) + } +} + +//nolint:dupl // similar but not duplicate +func Test_GetModuleName(t *testing.T) { + tests := []struct { + name string + m *v1beta2.ModuleTemplate + expectedName string + }{ + { + name: "Test GetModuleName() by label", + m: &v1beta2.ModuleTemplate{ + ObjectMeta: apimetav1.ObjectMeta{ + Labels: map[string]string{ + shared.ModuleName: "labelled-module", + }, + }, + Spec: v1beta2.ModuleTemplateSpec{}, + }, + expectedName: "labelled-module", + }, + { + name: "Test GetModuleName() by spec.moduleName", + m: &v1beta2.ModuleTemplate{ + ObjectMeta: apimetav1.ObjectMeta{ + Labels: map[string]string{}, + }, + Spec: v1beta2.ModuleTemplateSpec{ + ModuleName: "spec-module", + }, + }, + expectedName: "spec-module", + }, + { + name: "Test GetModuleName() spec.moduleName has priority over label", + m: &v1beta2.ModuleTemplate{ + ObjectMeta: apimetav1.ObjectMeta{ + Labels: map[string]string{ + shared.ModuleName: "labelled-module", + }, + }, + Spec: v1beta2.ModuleTemplateSpec{ + ModuleName: "spec-module", + }, + }, + expectedName: "spec-module", + }, + } + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + actualName := testCase.m.GetModuleName() + assert.Equal(t, testCase.expectedName, actualName) + }) + } +} diff --git a/internal/descriptor/cache/key.go b/internal/descriptor/cache/key.go index f5d7cf90e2..4a6472f663 100644 --- a/internal/descriptor/cache/key.go +++ b/internal/descriptor/cache/key.go @@ -9,7 +9,7 @@ import ( type DescriptorKey string func GenerateDescriptorKey(template *v1beta2.ModuleTemplate) DescriptorKey { - version, err := template.GetVersion() + version, err := template.GetSemanticVersion() if err == nil { return DescriptorKey(fmt.Sprintf("%s:%s:%d:%s", template.Name, template.Spec.Channel, template.Generation, version)) diff --git a/internal/maintenancewindows/maintenance_window.go b/internal/maintenancewindows/maintenance_window.go index 2f1a5389f4..ce358ebdce 100644 --- a/internal/maintenancewindows/maintenance_window.go +++ b/internal/maintenancewindows/maintenance_window.go @@ -73,14 +73,13 @@ func (MaintenanceWindow) IsRequired(moduleTemplate *v1beta2.ModuleTemplate, kyma } // module not installed yet => no need for maintenance window - moduleStatus := kyma.Status.GetModuleStatus(moduleTemplate.Spec.ModuleName) + moduleStatus := kyma.Status.GetModuleStatus(moduleTemplate.GetModuleName()) if moduleStatus == nil { return false } // module already installed in this version => no need for maintenance window - installedVersion := moduleStatus.Version - return installedVersion != moduleTemplate.Spec.Version + return moduleStatus.Version != moduleTemplate.GetVersion() } // IsActive determines if a maintenance window is currently active. From 3d85fd00b777da75c75ab18de728a3f1de9ce97e Mon Sep 17 00:00:00 2001 From: Mohammed Mesaoudi Date: Thu, 23 Jan 2025 15:16:19 +0100 Subject: [PATCH 2/3] =?UTF-8?q?feat:=20Removed=20skr=20permission=20=20and?= =?UTF-8?q?=20cache=20config=20for=20remote=20kyma=20&=20ModuleTemp?= =?UTF-8?q?=E2=80=A6=20(#2198)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Removed skr permission and cache config for remote kyma & ModuleTemplate resources * Removed e2e test for klm-controller-manager serviceAcount on kyma-system namespace. * Remove remoteNamespace field as it will be not used for the cache. --------- Co-authored-by: Benjamin Lindner <50365642+lindnerby@users.noreply.github.com> --- cmd/main.go | 2 +- .../namespace_bindings/kustomization.yaml | 2 - config/rbac/namespace_bindings/skr_role.yaml | 43 ------------------- .../namespace_bindings/skr_role_binding.yaml | 12 ------ internal/cache_options.go | 20 ++++----- tests/e2e/rbac_privileges_test.go | 32 -------------- .../controller/eventfilters/suite_test.go | 2 +- .../integration/controller/kcp/suite_test.go | 2 +- .../integration/controller/kyma/suite_test.go | 2 +- .../mandatorymodule/deletion/suite_test.go | 2 +- .../installation/suite_test.go | 2 +- .../custom_resource_check/suite_test.go | 2 +- .../controller/manifest/suite_test.go | 2 +- .../controller/moduletemplate/suite_test.go | 2 +- .../controller/purge/suite_test.go | 2 +- .../controller/withwatcher/suite_test.go | 2 +- 16 files changed, 19 insertions(+), 112 deletions(-) delete mode 100644 config/rbac/namespace_bindings/skr_role.yaml delete mode 100644 config/rbac/namespace_bindings/skr_role_binding.yaml diff --git a/cmd/main.go b/cmd/main.go index d2b10cfada..578f1cf49f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -116,7 +116,7 @@ func main() { } cacheOptions := internal.GetCacheOptions(flagVar.IsKymaManaged, flagVar.IstioNamespace, - flagVar.IstioGatewayNamespace, flagVar.RemoteSyncNamespace) + flagVar.IstioGatewayNamespace) setupManager(flagVar, cacheOptions, scheme, setupLog) } diff --git a/config/rbac/namespace_bindings/kustomization.yaml b/config/rbac/namespace_bindings/kustomization.yaml index d1b8329d73..8e9a9d4f54 100644 --- a/config/rbac/namespace_bindings/kustomization.yaml +++ b/config/rbac/namespace_bindings/kustomization.yaml @@ -13,6 +13,4 @@ resources: - role_binding.yaml # Comment the following to disable manifest integration - watcher_certmanager_role.yaml - - skr_role.yaml - watcher_certmanager_role_binding.yaml - - skr_role_binding.yaml diff --git a/config/rbac/namespace_bindings/skr_role.yaml b/config/rbac/namespace_bindings/skr_role.yaml deleted file mode 100644 index 27bdb53aa4..0000000000 --- a/config/rbac/namespace_bindings/skr_role.yaml +++ /dev/null @@ -1,43 +0,0 @@ ---- -# Give controller-manager permissions to the resources residing in kyma-system namespace on the SKR cluster -apiVersion: rbac.authorization.k8s.io/v1 -kind: Role -metadata: - name: controller-manager-skr - namespace: kyma-system -rules: -- apiGroups: - - operator.kyma-project.io - resources: - - kymas - verbs: - - list - - watch - - delete - - get - - create - - patch - - update -- apiGroups: - - operator.kyma-project.io - resources: - - kymas/finalizers - verbs: - - update -- apiGroups: - - operator.kyma-project.io - resources: - - kymas/status - verbs: - - get - - patch - - update - - watch -- apiGroups: - - operator.kyma-project.io - resources: - - moduletemplates - verbs: - - list - - watch - - delete \ No newline at end of file diff --git a/config/rbac/namespace_bindings/skr_role_binding.yaml b/config/rbac/namespace_bindings/skr_role_binding.yaml deleted file mode 100644 index 2772232b08..0000000000 --- a/config/rbac/namespace_bindings/skr_role_binding.yaml +++ /dev/null @@ -1,12 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding -metadata: - name: controller-manager-skr - namespace: kyma-system -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: Role - name: controller-manager-skr -subjects: - - kind: ServiceAccount - name: controller-manager diff --git a/internal/cache_options.go b/internal/cache_options.go index cc7a64a017..0af1f02130 100644 --- a/internal/cache_options.go +++ b/internal/cache_options.go @@ -19,10 +19,9 @@ type DefaultCacheOptions struct { } type KcpCacheOptions struct { - CacheOptions cache.Options - istioNamespace string - kcpNamespace string - remoteNamespace string + CacheOptions cache.Options + istioNamespace string + kcpNamespace string } func (c *DefaultCacheOptions) GetCacheOptions() cache.Options { @@ -47,14 +46,12 @@ func (c *KcpCacheOptions) GetCacheOptions() cache.Options { }, &v1beta2.Kyma{}: { Namespaces: map[string]cache.Config{ - c.remoteNamespace: {}, - c.kcpNamespace: {}, + c.kcpNamespace: {}, }, }, &v1beta2.ModuleTemplate{}: { Namespaces: map[string]cache.Config{ - c.remoteNamespace: {}, - c.kcpNamespace: {}, + c.kcpNamespace: {}, }, }, &v1beta2.ModuleReleaseMeta{}: { @@ -88,12 +85,11 @@ func (c *KcpCacheOptions) GetCacheOptions() cache.Options { } } -func GetCacheOptions(isKymaManaged bool, istioNamespace, kcpNamespace, remoteNamespace string) cache.Options { +func GetCacheOptions(isKymaManaged bool, istioNamespace, kcpNamespace string) cache.Options { if isKymaManaged { options := &KcpCacheOptions{ - istioNamespace: istioNamespace, - kcpNamespace: kcpNamespace, - remoteNamespace: remoteNamespace, + istioNamespace: istioNamespace, + kcpNamespace: kcpNamespace, } return options.GetCacheOptions() } diff --git a/tests/e2e/rbac_privileges_test.go b/tests/e2e/rbac_privileges_test.go index 1014f0fe90..588d2d1a34 100644 --- a/tests/e2e/rbac_privileges_test.go +++ b/tests/e2e/rbac_privileges_test.go @@ -199,38 +199,6 @@ var _ = Describe("RBAC Privileges", func() { Expect(GetRoleBindingRolePolicyRules(ctx, kcpClient, "klm-controller-manager-watcher-certmanager", "istio-system", istioSystemKlmRoleBindings)).To(Equal(istioNamespaceRoleRules)) - - By("And KLM Service Account has the correct RoleBindings in kyma-system namespace") - remoteNamespaceRoleRules := []apirbacv1.PolicyRule{ - { - APIGroups: []string{"operator.kyma-project.io"}, - Resources: []string{"kymas"}, - Verbs: []string{"list", "watch", "delete", "get", "create", "patch", "update"}, - }, - { - APIGroups: []string{"operator.kyma-project.io"}, - Resources: []string{"kymas/finalizers"}, - Verbs: []string{"update"}, - }, - { - APIGroups: []string{"operator.kyma-project.io"}, - Resources: []string{"kymas/status"}, - Verbs: []string{"get", "patch", "update", "watch"}, - }, - { - APIGroups: []string{"operator.kyma-project.io"}, - Resources: []string{"moduletemplates"}, - Verbs: []string{"list", "watch", "delete"}, - }, - } - kymaSystemKlmRoleBindings, err := ListKlmRoleBindings(kcpClient, ctx, "klm-controller-manager", - "kyma-system") - Expect(err).ToNot(HaveOccurred()) - Expect(kymaSystemKlmRoleBindings.Items).To(HaveLen(1)) - - Expect(GetRoleBindingRolePolicyRules(ctx, kcpClient, - "klm-controller-manager-skr", "kyma-system", - kymaSystemKlmRoleBindings)).To(Equal(remoteNamespaceRoleRules)) }) }) }) diff --git a/tests/integration/controller/eventfilters/suite_test.go b/tests/integration/controller/eventfilters/suite_test.go index 73d8354129..30ccfe248c 100644 --- a/tests/integration/controller/eventfilters/suite_test.go +++ b/tests/integration/controller/eventfilters/suite_test.go @@ -124,7 +124,7 @@ var _ = BeforeSuite(func() { BindAddress: randomPort, }, Scheme: k8sclientscheme.Scheme, - Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace, RemoteNamespace), + Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace), }) Expect(err).ToNot(HaveOccurred()) diff --git a/tests/integration/controller/kcp/suite_test.go b/tests/integration/controller/kcp/suite_test.go index 784209be82..e1d7429ec4 100644 --- a/tests/integration/controller/kcp/suite_test.go +++ b/tests/integration/controller/kcp/suite_test.go @@ -124,7 +124,7 @@ var _ = BeforeSuite(func() { BindAddress: UseRandomPort, }, Scheme: k8sclientscheme.Scheme, - Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace, RemoteNamespace), + Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace), }) Expect(err).ToNot(HaveOccurred()) diff --git a/tests/integration/controller/kyma/suite_test.go b/tests/integration/controller/kyma/suite_test.go index 0e43ca2a3f..bd780f071f 100644 --- a/tests/integration/controller/kyma/suite_test.go +++ b/tests/integration/controller/kyma/suite_test.go @@ -122,7 +122,7 @@ var _ = BeforeSuite(func() { BindAddress: randomPort, }, Scheme: k8sclientscheme.Scheme, - Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace, RemoteNamespace), + Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace), }) Expect(err).ToNot(HaveOccurred()) diff --git a/tests/integration/controller/mandatorymodule/deletion/suite_test.go b/tests/integration/controller/mandatorymodule/deletion/suite_test.go index 368fe36e80..2ed0011137 100644 --- a/tests/integration/controller/mandatorymodule/deletion/suite_test.go +++ b/tests/integration/controller/mandatorymodule/deletion/suite_test.go @@ -106,7 +106,7 @@ var _ = BeforeSuite(func() { BindAddress: useRandomPort, }, Scheme: k8sclientscheme.Scheme, - Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace, RemoteNamespace), + Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace), }) Expect(err).ToNot(HaveOccurred()) diff --git a/tests/integration/controller/mandatorymodule/installation/suite_test.go b/tests/integration/controller/mandatorymodule/installation/suite_test.go index 6a77125541..38fb3eec1d 100644 --- a/tests/integration/controller/mandatorymodule/installation/suite_test.go +++ b/tests/integration/controller/mandatorymodule/installation/suite_test.go @@ -97,7 +97,7 @@ var _ = BeforeSuite(func() { BindAddress: useRandomPort, }, Scheme: k8sclientscheme.Scheme, - Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace, RemoteNamespace), + Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace), }) Expect(err).ToNot(HaveOccurred()) diff --git a/tests/integration/controller/manifest/custom_resource_check/suite_test.go b/tests/integration/controller/manifest/custom_resource_check/suite_test.go index 4d4059fb30..5073fede57 100644 --- a/tests/integration/controller/manifest/custom_resource_check/suite_test.go +++ b/tests/integration/controller/manifest/custom_resource_check/suite_test.go @@ -114,7 +114,7 @@ var _ = BeforeSuite(func() { if !found { metricsBindAddress = ":0" } - cacheOpts := internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace, RemoteNamespace) + cacheOpts := internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace) syncPeriod := 2 * time.Second cacheOpts.SyncPeriod = &syncPeriod diff --git a/tests/integration/controller/manifest/suite_test.go b/tests/integration/controller/manifest/suite_test.go index c79bbc1cba..a6967ac2ee 100644 --- a/tests/integration/controller/manifest/suite_test.go +++ b/tests/integration/controller/manifest/suite_test.go @@ -125,7 +125,7 @@ var _ = BeforeSuite(func() { BindAddress: metricsBindAddress, }, Scheme: k8sclientscheme.Scheme, - Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace, RemoteNamespace), + Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace), }, ) Expect(err).ToNot(HaveOccurred()) diff --git a/tests/integration/controller/moduletemplate/suite_test.go b/tests/integration/controller/moduletemplate/suite_test.go index b0932cb05b..52fcd7f364 100644 --- a/tests/integration/controller/moduletemplate/suite_test.go +++ b/tests/integration/controller/moduletemplate/suite_test.go @@ -90,7 +90,7 @@ var _ = BeforeSuite(func() { BindAddress: randomPort, }, Scheme: k8sclientscheme.Scheme, - Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace, RemoteNamespace), + Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace), }) Expect(err).ToNot(HaveOccurred()) diff --git a/tests/integration/controller/purge/suite_test.go b/tests/integration/controller/purge/suite_test.go index f028acd694..a117dbaef7 100644 --- a/tests/integration/controller/purge/suite_test.go +++ b/tests/integration/controller/purge/suite_test.go @@ -101,7 +101,7 @@ var _ = BeforeSuite(func() { BindAddress: useRandomPort, }, Scheme: k8sclientscheme.Scheme, - Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace, RemoteNamespace), + Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace), }) Expect(err).ToNot(HaveOccurred()) diff --git a/tests/integration/controller/withwatcher/suite_test.go b/tests/integration/controller/withwatcher/suite_test.go index d481c9d137..2b9c96f205 100644 --- a/tests/integration/controller/withwatcher/suite_test.go +++ b/tests/integration/controller/withwatcher/suite_test.go @@ -145,7 +145,7 @@ var _ = BeforeSuite(func() { BindAddress: metricsBindAddress, }, Scheme: k8sclientscheme.Scheme, - Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace, RemoteNamespace), + Cache: internal.GetCacheOptions(false, "istio-system", ControlPlaneNamespace), }) Expect(err).ToNot(HaveOccurred()) From d4ab0597b0761f6229feaee88446af5424eed5a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20Schw=C3=A4gerl?= Date: Thu, 23 Jan 2025 15:48:34 +0100 Subject: [PATCH 3/3] feat: Refactor template lookup to use strategy pattern (#2200) * feat: Refactor template lookup to use strategy pattern * fix deletion test * fix linting * slim interface signature * add coverage entry * adapt expected coverage * fix e2e tests * fix deletion test * add comments to new types * rename strategies * add test cases for strategies --------- Co-authored-by: Benjamin Lindner <50365642+lindnerby@users.noreply.github.com> --- cmd/main.go | 11 +- pkg/module/sync/runner.go | 3 +- .../by_channel_strategy.go | 135 ++++++++ .../by_channel_strategy_test.go | 69 +++++ .../by_module_release_meta_strategy.go | 52 ++++ .../by_module_release_meta_strategy_test.go | 80 +++++ .../by_version_strategy.go | 82 +++++ .../by_version_strategy_test.go | 106 +++++++ .../moduletemplateinfolookup/common.go | 59 ++++ .../moduletemplateinfolookup/strategies.go | 50 +++ .../strategies_test.go | 91 ++++++ pkg/templatelookup/regular.go | 287 ++---------------- pkg/templatelookup/regular_test.go | 190 ++++++------ pkg/testutils/moduletemplate.go | 57 ++-- tests/e2e/module_deletion_test.go | 10 +- tests/e2e/module_upgrade_new_version_test.go | 2 +- ...maintenance_window_module_downtime_test.go | 2 +- ...asemeta_module_upgrade_new_version_test.go | 2 +- ...leasemeta_not_allowed_installation_test.go | 2 +- tests/e2e/modulereleasemeta_sync_test.go | 39 ++- .../integration/controller/kcp/helper_test.go | 5 +- .../controller/kcp/remote_sync_test.go | 13 +- .../integration/controller/kcp/suite_test.go | 9 +- .../controller/kyma/helper_test.go | 2 +- .../integration/controller/kyma/kyma_test.go | 5 +- .../controller/kyma/manifest_test.go | 26 +- .../integration/controller/kyma/suite_test.go | 9 +- unit-test-coverage.yaml | 3 +- 28 files changed, 950 insertions(+), 451 deletions(-) create mode 100644 pkg/templatelookup/moduletemplateinfolookup/by_channel_strategy.go create mode 100644 pkg/templatelookup/moduletemplateinfolookup/by_channel_strategy_test.go create mode 100644 pkg/templatelookup/moduletemplateinfolookup/by_module_release_meta_strategy.go create mode 100644 pkg/templatelookup/moduletemplateinfolookup/by_module_release_meta_strategy_test.go create mode 100644 pkg/templatelookup/moduletemplateinfolookup/by_version_strategy.go create mode 100644 pkg/templatelookup/moduletemplateinfolookup/by_version_strategy_test.go create mode 100644 pkg/templatelookup/moduletemplateinfolookup/common.go create mode 100644 pkg/templatelookup/moduletemplateinfolookup/strategies.go create mode 100644 pkg/templatelookup/moduletemplateinfolookup/strategies_test.go diff --git a/cmd/main.go b/cmd/main.go index 578f1cf49f..f66ac2b3ad 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -65,6 +65,7 @@ import ( "github.com/kyma-project/lifecycle-manager/pkg/matcher" "github.com/kyma-project/lifecycle-manager/pkg/queue" "github.com/kyma-project/lifecycle-manager/pkg/templatelookup" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup" "github.com/kyma-project/lifecycle-manager/pkg/watcher" _ "k8s.io/client-go/plugin/pkg/client/auth" @@ -280,13 +281,19 @@ func scheduleMetricsCleanup(kymaMetrics *metrics.KymaMetrics, cleanupIntervalInM func setupKymaReconciler(mgr ctrl.Manager, descriptorProvider *provider.CachedDescriptorProvider, skrContextFactory remote.SkrContextProvider, event event.Event, flagVar *flags.FlagVar, options ctrlruntime.Options, skrWebhookManager *watcher.SKRWebhookManifestManager, kymaMetrics *metrics.KymaMetrics, - setupLog logr.Logger, maintenanceWindow templatelookup.MaintenanceWindow, + setupLog logr.Logger, _ *maintenancewindows.MaintenanceWindow, ) { options.RateLimiter = internal.RateLimiter(flagVar.FailureBaseDelay, flagVar.FailureMaxDelay, flagVar.RateLimiterFrequency, flagVar.RateLimiterBurst) options.CacheSyncTimeout = flagVar.CacheSyncTimeout options.MaxConcurrentReconciles = flagVar.MaxConcurrentKymaReconciles + moduleTemplateInfoLookupStrategies := moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + moduletemplateinfolookup.NewByVersionStrategy(mgr.GetClient()), + moduletemplateinfolookup.NewByChannelStrategy(mgr.GetClient()), + moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(mgr.GetClient()), + }) + if err := (&kyma.Reconciler{ Client: mgr.GetClient(), SkrContextFactory: skrContextFactory, @@ -306,7 +313,7 @@ func setupKymaReconciler(mgr ctrl.Manager, descriptorProvider *provider.CachedDe Metrics: kymaMetrics, RemoteCatalog: remote.NewRemoteCatalogFromKyma(mgr.GetClient(), skrContextFactory, flagVar.RemoteSyncNamespace), - TemplateLookup: templatelookup.NewTemplateLookup(mgr.GetClient(), descriptorProvider, maintenanceWindow), + TemplateLookup: templatelookup.NewTemplateLookup(mgr.GetClient(), descriptorProvider, moduleTemplateInfoLookupStrategies), }).SetupWithManager( mgr, options, kyma.SetupOptions{ ListenerAddr: flagVar.KymaListenerAddr, diff --git a/pkg/module/sync/runner.go b/pkg/module/sync/runner.go index db2106f900..4c4bbccb78 100644 --- a/pkg/module/sync/runner.go +++ b/pkg/module/sync/runner.go @@ -21,6 +21,7 @@ import ( "github.com/kyma-project/lifecycle-manager/pkg/log" "github.com/kyma-project/lifecycle-manager/pkg/module/common" "github.com/kyma-project/lifecycle-manager/pkg/templatelookup" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup" "github.com/kyma-project/lifecycle-manager/pkg/util" ) @@ -275,7 +276,7 @@ func generateModuleStatus(module *common.Module, existStatus *v1beta2.ModuleStat newModuleStatus.Message = module.Template.Err.Error() return *newModuleStatus } - if errors.Is(module.Template.Err, templatelookup.ErrNoTemplatesInListResult) { + if errors.Is(module.Template.Err, moduletemplateinfolookup.ErrNoTemplatesInListResult) { return v1beta2.ModuleStatus{ Name: module.ModuleName, Channel: module.Template.DesiredChannel, diff --git a/pkg/templatelookup/moduletemplateinfolookup/by_channel_strategy.go b/pkg/templatelookup/moduletemplateinfolookup/by_channel_strategy.go new file mode 100644 index 0000000000..27293e9ada --- /dev/null +++ b/pkg/templatelookup/moduletemplateinfolookup/by_channel_strategy.go @@ -0,0 +1,135 @@ +package moduletemplateinfolookup + +import ( + "context" + "errors" + "fmt" + + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/pkg/log" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup" +) + +var ErrNotDefaultChannelAllowed = errors.New("specifying no default channel is not allowed") + +// ByChannelStrategy looks up the module template for a given channel-based installation. +type ByChannelStrategy struct { + client client.Reader +} + +func NewByChannelStrategy(client client.Reader) ByChannelStrategy { + return ByChannelStrategy{client: client} +} + +func (ByChannelStrategy) IsResponsible(moduleInfo *templatelookup.ModuleInfo, moduleReleaseMeta *v1beta2.ModuleReleaseMeta) bool { + if moduleReleaseMeta != nil { + return false + } + + if moduleInfo.IsInstalledByVersion() { + return false + } + + return true +} + +func (s ByChannelStrategy) Lookup(ctx context.Context, + moduleInfo *templatelookup.ModuleInfo, + kyma *v1beta2.Kyma, + _ *v1beta2.ModuleReleaseMeta, +) templatelookup.ModuleTemplateInfo { + desiredChannel := getDesiredChannel(moduleInfo.Channel, kyma.Spec.Channel) + info := templatelookup.ModuleTemplateInfo{ + DesiredChannel: desiredChannel, + } + + template, err := s.filterTemplatesByChannel(ctx, moduleInfo.Name, desiredChannel) + if err != nil { + info.Err = err + return info + } + + actualChannel := template.Spec.Channel + if actualChannel == "" { + info.Err = fmt.Errorf( + "no channel found on template for module: %s: %w", + moduleInfo.Name, ErrNotDefaultChannelAllowed, + ) + return info + } + + logUsedChannel(ctx, moduleInfo.Name, actualChannel, kyma.Spec.Channel) + info.ModuleTemplate = template + return info +} + +func (s ByChannelStrategy) filterTemplatesByChannel(ctx context.Context, name, desiredChannel string) ( + *v1beta2.ModuleTemplate, error, +) { + templateList := &v1beta2.ModuleTemplateList{} + err := s.client.List(ctx, templateList) + if err != nil { + return nil, fmt.Errorf("failed to list module templates on lookup: %w", err) + } + + var filteredTemplates []*v1beta2.ModuleTemplate + for _, template := range templateList.Items { + if TemplateNameMatch(&template, name) && template.Spec.Channel == desiredChannel { + filteredTemplates = append(filteredTemplates, &template) + continue + } + } + + if len(filteredTemplates) > 1 { + return nil, newMoreThanOneTemplateCandidateErr(name, templateList.Items) + } + + if len(filteredTemplates) == 0 { + return nil, fmt.Errorf("%w: for module %s in channel %s ", + ErrNoTemplatesInListResult, name, desiredChannel) + } + + if filteredTemplates[0].Spec.Mandatory { + return nil, fmt.Errorf("%w: for module %s in channel %s", + ErrTemplateMarkedAsMandatory, name, desiredChannel) + } + + return filteredTemplates[0], nil +} + +func getDesiredChannel(moduleChannel, globalChannel string) string { + var desiredChannel string + + switch { + case moduleChannel != "": + desiredChannel = moduleChannel + case globalChannel != "": + desiredChannel = globalChannel + default: + desiredChannel = v1beta2.DefaultChannel + } + + return desiredChannel +} + +func logUsedChannel(ctx context.Context, name string, actualChannel string, defaultChannel string) { + logger := logf.FromContext(ctx) + if actualChannel != defaultChannel { + logger.V(log.DebugLevel).Info( + fmt.Sprintf( + "using %s (instead of %s) for module %s", + actualChannel, defaultChannel, name, + ), + ) + } else { + logger.V(log.DebugLevel).Info( + fmt.Sprintf( + "using %s for module %s", + actualChannel, name, + ), + ) + } +} diff --git a/pkg/templatelookup/moduletemplateinfolookup/by_channel_strategy_test.go b/pkg/templatelookup/moduletemplateinfolookup/by_channel_strategy_test.go new file mode 100644 index 0000000000..1f04802ea6 --- /dev/null +++ b/pkg/templatelookup/moduletemplateinfolookup/by_channel_strategy_test.go @@ -0,0 +1,69 @@ +package moduletemplateinfolookup_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup" + "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" +) + +func Test_ByChannelStrategy_IsResponsible_ReturnsTrue(t *testing.T) { + moduleInfo := newModuleInfoBuilder().WithChannel("regular").Enabled().Build() + var moduleReleaseMeta *v1beta2.ModuleReleaseMeta = nil + byChannelStrategy := moduletemplateinfolookup.NewByChannelStrategy(nil) + + responsible := byChannelStrategy.IsResponsible(moduleInfo, moduleReleaseMeta) + + assert.True(t, responsible) +} + +func Test_ByChannelStrategy_IsResponsible_ReturnsFalse_WhenModuleReleaseMetaIsNotNil(t *testing.T) { + moduleInfo := newModuleInfoBuilder().WithChannel("regular").Enabled().Build() + moduleReleaseMeta := builder.NewModuleReleaseMetaBuilder().Build() + byChannelStrategy := moduletemplateinfolookup.NewByChannelStrategy(nil) + + responsible := byChannelStrategy.IsResponsible(moduleInfo, moduleReleaseMeta) + + assert.False(t, responsible) +} + +func Test_ByChannelStrategy_IsResponsible_ReturnsFalse_WhenInstalledByVersion(t *testing.T) { + moduleInfo := newModuleInfoBuilder().WithVersion("1.0.0").Enabled().Build() + var moduleReleaseMeta *v1beta2.ModuleReleaseMeta = nil + byChannelStrategy := moduletemplateinfolookup.NewByChannelStrategy(nil) + + responsible := byChannelStrategy.IsResponsible(moduleInfo, moduleReleaseMeta) + + assert.False(t, responsible) +} + +func Test_ByChannelStrategy_Lookup_ReturnsModuleTemplateInfo(t *testing.T) { + moduleInfo := newModuleInfoBuilder().WithName("test-module").WithChannel("regular").Enabled().Build() + kyma := builder.NewKymaBuilder().Build() + var moduleReleaseMeta *v1beta2.ModuleReleaseMeta = nil + moduleTemplate := builder.NewModuleTemplateBuilder(). + WithName("test-module-regular"). + WithModuleName("test-module"). + WithVersion(""). + WithChannel("regular"). + Build() + byChannelStrategy := moduletemplateinfolookup.NewByChannelStrategy(fakeClient( + &v1beta2.ModuleTemplateList{ + Items: []v1beta2.ModuleTemplate{ + *moduleTemplate, + }, + }, + )) + + moduleTemplateInfo := byChannelStrategy.Lookup(context.Background(), moduleInfo, kyma, moduleReleaseMeta) + + assert.NotNil(t, moduleTemplateInfo) + assert.Equal(t, moduleTemplate.Name, moduleTemplateInfo.ModuleTemplate.Name) + assert.Equal(t, moduleTemplate.Spec.ModuleName, moduleTemplateInfo.ModuleTemplate.Spec.ModuleName) + assert.Equal(t, moduleTemplate.Spec.Version, moduleTemplateInfo.ModuleTemplate.Spec.Version) + assert.Equal(t, moduleTemplate.Spec.Channel, moduleTemplateInfo.ModuleTemplate.Spec.Channel) +} diff --git a/pkg/templatelookup/moduletemplateinfolookup/by_module_release_meta_strategy.go b/pkg/templatelookup/moduletemplateinfolookup/by_module_release_meta_strategy.go new file mode 100644 index 0000000000..579a5373c6 --- /dev/null +++ b/pkg/templatelookup/moduletemplateinfolookup/by_module_release_meta_strategy.go @@ -0,0 +1,52 @@ +package moduletemplateinfolookup + +import ( + "context" + + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup" +) + +// ByModuleReleaseMetaStrategy looks up the module template via the module release meta. +// It only supports channel-based installation. +type ByModuleReleaseMetaStrategy struct { + client client.Reader +} + +func NewByModuleReleaseMetaStrategy(client client.Reader) ByModuleReleaseMetaStrategy { + return ByModuleReleaseMetaStrategy{client: client} +} + +func (ByModuleReleaseMetaStrategy) IsResponsible(_ *templatelookup.ModuleInfo, moduleReleaseMeta *v1beta2.ModuleReleaseMeta) bool { + return moduleReleaseMeta != nil +} + +func (s ByModuleReleaseMetaStrategy) Lookup(ctx context.Context, + moduleInfo *templatelookup.ModuleInfo, + kyma *v1beta2.Kyma, + moduleReleaseMeta *v1beta2.ModuleReleaseMeta, +) templatelookup.ModuleTemplateInfo { + moduleTemplateInfo := templatelookup.ModuleTemplateInfo{} + + moduleTemplateInfo.DesiredChannel = getDesiredChannel(moduleInfo.Channel, kyma.Spec.Channel) + desiredModuleVersion, err := templatelookup.GetChannelVersionForModule(moduleReleaseMeta, moduleTemplateInfo.DesiredChannel) + if err != nil { + moduleTemplateInfo.Err = err + return moduleTemplateInfo + } + + template, err := getTemplateByVersion(ctx, + s.client, + moduleInfo.Name, + desiredModuleVersion, + kyma.Namespace) + if err != nil { + moduleTemplateInfo.Err = err + return moduleTemplateInfo + } + + moduleTemplateInfo.ModuleTemplate = template + return moduleTemplateInfo +} diff --git a/pkg/templatelookup/moduletemplateinfolookup/by_module_release_meta_strategy_test.go b/pkg/templatelookup/moduletemplateinfolookup/by_module_release_meta_strategy_test.go new file mode 100644 index 0000000000..ba6f1469ee --- /dev/null +++ b/pkg/templatelookup/moduletemplateinfolookup/by_module_release_meta_strategy_test.go @@ -0,0 +1,80 @@ +package moduletemplateinfolookup_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + machineryruntime "k8s.io/apimachinery/pkg/runtime" + machineryutilruntime "k8s.io/apimachinery/pkg/util/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/kyma-project/lifecycle-manager/api" + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup" + "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" +) + +func Test_ByModuleReleaseMetaStrategy_IsResponsible_ReturnsTrue(t *testing.T) { + moduleInfo := newModuleInfoBuilder().WithChannel("regular").Enabled().Build() + moduleReleaseMeta := builder.NewModuleReleaseMetaBuilder().Build() + byMRMStrategy := moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(nil) + + responsible := byMRMStrategy.IsResponsible(moduleInfo, moduleReleaseMeta) + + assert.True(t, responsible) +} + +func Test_ByModuleReleaseMetaStrategy_IsResponsible_ReturnsFalse_WhenModuleReleaseMetaIsNotNil(t *testing.T) { + moduleInfo := newModuleInfoBuilder().WithVersion("regular").Enabled().Build() + var moduleReleaseMeta *v1beta2.ModuleReleaseMeta = nil + byMRMStrategy := moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(nil) + + responsible := byMRMStrategy.IsResponsible(moduleInfo, moduleReleaseMeta) + + assert.False(t, responsible) +} + +func Test_ByModuleReleaseMeta_Strategy_Lookup_ReturnsModuleTemplateInfo(t *testing.T) { + moduleInfo := newModuleInfoBuilder().WithName("test-module").WithChannel("regular").Enabled().Build() + kyma := builder.NewKymaBuilder().Build() + moduleReleaseMeta := builder.NewModuleReleaseMetaBuilder(). + WithModuleName("test-module"). + WithName("test-module"). + WithModuleChannelAndVersions([]v1beta2.ChannelVersionAssignment{ + { + Channel: "regular", + Version: "1.0.0", + }, + }). + Build() + moduleTemplate := builder.NewModuleTemplateBuilder(). + WithName("test-module-1.0.0"). + WithModuleName("test-module"). + WithVersion("1.0.0"). + WithChannel("none"). + Build() + byMRMStrategy := moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(fakeClient( + &v1beta2.ModuleTemplateList{ + Items: []v1beta2.ModuleTemplate{ + *moduleTemplate, + }, + }, + )) + + moduleTemplateInfo := byMRMStrategy.Lookup(context.Background(), moduleInfo, kyma, moduleReleaseMeta) + + assert.NotNil(t, moduleTemplateInfo) + assert.Equal(t, moduleTemplate.Name, moduleTemplateInfo.ModuleTemplate.Name) + assert.Equal(t, moduleTemplate.Spec.ModuleName, moduleTemplateInfo.ModuleTemplate.Spec.ModuleName) + assert.Equal(t, moduleTemplate.Spec.Version, moduleTemplateInfo.ModuleTemplate.Spec.Version) + assert.Equal(t, moduleTemplate.Spec.Channel, moduleTemplateInfo.ModuleTemplate.Spec.Channel) +} + +func fakeClient(mts *v1beta2.ModuleTemplateList) client.Client { + scheme := machineryruntime.NewScheme() + machineryutilruntime.Must(api.AddToScheme(scheme)) + + return fake.NewClientBuilder().WithScheme(scheme).WithLists(mts).Build() +} diff --git a/pkg/templatelookup/moduletemplateinfolookup/by_version_strategy.go b/pkg/templatelookup/moduletemplateinfolookup/by_version_strategy.go new file mode 100644 index 0000000000..95bd6c3b62 --- /dev/null +++ b/pkg/templatelookup/moduletemplateinfolookup/by_version_strategy.go @@ -0,0 +1,82 @@ +package moduletemplateinfolookup + +import ( + "context" + "fmt" + + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup" +) + +// ByVersionStrategy looks up the module template for a given version-based installation. +type ByVersionStrategy struct { + client client.Reader +} + +func NewByVersionStrategy(client client.Reader) ByVersionStrategy { + return ByVersionStrategy{client: client} +} + +func (ByVersionStrategy) IsResponsible(moduleInfo *templatelookup.ModuleInfo, moduleReleaseMeta *v1beta2.ModuleReleaseMeta) bool { + if moduleReleaseMeta != nil { + return false + } + + if !moduleInfo.IsInstalledByVersion() { + return false + } + + return true +} + +func (s ByVersionStrategy) Lookup(ctx context.Context, + moduleInfo *templatelookup.ModuleInfo, + _ *v1beta2.Kyma, + _ *v1beta2.ModuleReleaseMeta, +) templatelookup.ModuleTemplateInfo { + info := templatelookup.ModuleTemplateInfo{ + DesiredChannel: string(shared.NoneChannel), + } + template, err := s.filterTemplatesByVersion(ctx, moduleInfo.Name, moduleInfo.Version) + if err != nil { + info.Err = err + return info + } + + info.ModuleTemplate = template + return info +} + +func (s ByVersionStrategy) filterTemplatesByVersion(ctx context.Context, name, version string) ( + *v1beta2.ModuleTemplate, error, +) { + templateList := &v1beta2.ModuleTemplateList{} + err := s.client.List(ctx, templateList) + if err != nil { + return nil, fmt.Errorf("failed to list module templates on lookup: %w", err) + } + + var filteredTemplates []*v1beta2.ModuleTemplate + for _, template := range templateList.Items { + if TemplateNameMatch(&template, + name) && shared.NoneChannel.Equals(template.Spec.Channel) && template.Spec.Version == version { + filteredTemplates = append(filteredTemplates, &template) + continue + } + } + if len(filteredTemplates) > 1 { + return nil, newMoreThanOneTemplateCandidateErr(name, templateList.Items) + } + if len(filteredTemplates) == 0 { + return nil, fmt.Errorf("%w: for module %s in version %s", + ErrNoTemplatesInListResult, name, version) + } + if filteredTemplates[0].Spec.Mandatory { + return nil, fmt.Errorf("%w: for module %s in version %s", + ErrTemplateMarkedAsMandatory, name, version) + } + return filteredTemplates[0], nil +} diff --git a/pkg/templatelookup/moduletemplateinfolookup/by_version_strategy_test.go b/pkg/templatelookup/moduletemplateinfolookup/by_version_strategy_test.go new file mode 100644 index 0000000000..243d0abd16 --- /dev/null +++ b/pkg/templatelookup/moduletemplateinfolookup/by_version_strategy_test.go @@ -0,0 +1,106 @@ +package moduletemplateinfolookup_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup" + "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" +) + +func Test_ByVersionStrategy_IsResponsible_ReturnsTrue(t *testing.T) { + moduleInfo := newModuleInfoBuilder().WithVersion("1.0.0").Enabled().Build() + var moduleReleaseMeta *v1beta2.ModuleReleaseMeta = nil + byVersionStrategy := moduletemplateinfolookup.NewByVersionStrategy(nil) + + responsible := byVersionStrategy.IsResponsible(moduleInfo, moduleReleaseMeta) + + assert.True(t, responsible) +} + +func Test_ByVersionStrategy_IsResponsible_ReturnsFalse_WhenModuleReleaseMetaIsNotNil(t *testing.T) { + moduleInfo := newModuleInfoBuilder().WithVersion("1.0.0").Enabled().Build() + moduleReleaseMeta := builder.NewModuleReleaseMetaBuilder().Build() + byVersionStrategy := moduletemplateinfolookup.NewByVersionStrategy(nil) + + responsible := byVersionStrategy.IsResponsible(moduleInfo, moduleReleaseMeta) + + assert.False(t, responsible) +} + +func Test_ByVersionStrategy_IsResponsible_ReturnsFalse_WhenNotInstalledByVersion(t *testing.T) { + moduleInfo := newModuleInfoBuilder().WithVersion("").WithChannel("regular").Enabled().Build() + var moduleReleaseMeta *v1beta2.ModuleReleaseMeta = nil + byVersionStrategy := moduletemplateinfolookup.NewByVersionStrategy(nil) + + responsible := byVersionStrategy.IsResponsible(moduleInfo, moduleReleaseMeta) + + assert.False(t, responsible) +} + +func Test_ByVersion_Strategy_Lookup_ReturnsModuleTemplateInfo(t *testing.T) { + moduleInfo := newModuleInfoBuilder().WithName("test-module").WithVersion("1.0.0").Enabled().Build() + var kyma *v1beta2.Kyma = nil + var moduleReleaseMeta *v1beta2.ModuleReleaseMeta = nil + moduleTemplate := builder.NewModuleTemplateBuilder(). + WithName("test-module-1.0.0"). + WithModuleName("test-module"). + WithVersion("1.0.0"). + WithChannel("none"). + Build() + byVersionStrategy := moduletemplateinfolookup.NewByVersionStrategy(fakeClient( + &v1beta2.ModuleTemplateList{ + Items: []v1beta2.ModuleTemplate{ + *moduleTemplate, + }, + }, + )) + + moduleTemplateInfo := byVersionStrategy.Lookup(context.Background(), moduleInfo, kyma, moduleReleaseMeta) + + assert.NotNil(t, moduleTemplateInfo) + assert.Equal(t, moduleTemplate.Name, moduleTemplateInfo.ModuleTemplate.Name) + assert.Equal(t, moduleTemplate.Spec.ModuleName, moduleTemplateInfo.ModuleTemplate.Spec.ModuleName) + assert.Equal(t, moduleTemplate.Spec.Version, moduleTemplateInfo.ModuleTemplate.Spec.Version) + assert.Equal(t, moduleTemplate.Spec.Channel, moduleTemplateInfo.ModuleTemplate.Spec.Channel) +} + +type moduleInfoBuilder struct { + moduleInfo *templatelookup.ModuleInfo +} + +func newModuleInfoBuilder() moduleInfoBuilder { + return moduleInfoBuilder{ + moduleInfo: &templatelookup.ModuleInfo{ + Module: v1beta2.Module{}, + }, + } +} + +func (b moduleInfoBuilder) WithName(name string) moduleInfoBuilder { + b.moduleInfo.Module.Name = name + return b +} + +func (b moduleInfoBuilder) WithVersion(version string) moduleInfoBuilder { + b.moduleInfo.Module.Version = version + return b +} + +func (b moduleInfoBuilder) WithChannel(channel string) moduleInfoBuilder { + b.moduleInfo.Module.Channel = channel + return b +} + +func (b moduleInfoBuilder) Enabled() moduleInfoBuilder { + b.moduleInfo.Enabled = true + return b +} + +func (b moduleInfoBuilder) Build() *templatelookup.ModuleInfo { + return b.moduleInfo +} diff --git a/pkg/templatelookup/moduletemplateinfolookup/common.go b/pkg/templatelookup/moduletemplateinfolookup/common.go new file mode 100644 index 0000000000..21f4d2a7c5 --- /dev/null +++ b/pkg/templatelookup/moduletemplateinfolookup/common.go @@ -0,0 +1,59 @@ +package moduletemplateinfolookup + +import ( + "context" + "errors" + "fmt" + + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kyma-project/lifecycle-manager/api/shared" + "github.com/kyma-project/lifecycle-manager/api/v1beta2" +) + +var ( + ErrNoTemplatesInListResult = errors.New("no templates were found") + ErrTemplateMarkedAsMandatory = errors.New("template marked as mandatory") + ErrTemplateNotIdentified = errors.New("no unique template could be identified") +) + +func TemplateNameMatch(template *v1beta2.ModuleTemplate, name string) bool { + if len(template.Spec.ModuleName) > 0 { + return template.Spec.ModuleName == name + } + + // Drop the legacyCondition once the label 'shared.ModuleName' is removed: https://github.com/kyma-project/lifecycle-manager/issues/1796 + if template.Labels == nil { + return false + } + return template.Labels[shared.ModuleName] == name +} + +func newMoreThanOneTemplateCandidateErr(moduleName string, + candidateTemplates []v1beta2.ModuleTemplate, +) error { + candidates := make([]string, len(candidateTemplates)) + for i, candidate := range candidateTemplates { + candidates[i] = candidate.GetName() + } + + return fmt.Errorf("%w: more than one module template found for module: %s, candidates: %v", + ErrTemplateNotIdentified, moduleName, candidates) +} + +func getTemplateByVersion(ctx context.Context, + clnt client.Reader, + moduleName, moduleVersion, namespace string, +) (*v1beta2.ModuleTemplate, error) { + moduleTemplate := &v1beta2.ModuleTemplate{} + + moduleTemplateName := fmt.Sprintf("%s-%s", moduleName, moduleVersion) + if err := clnt.Get(ctx, client.ObjectKey{ + Name: moduleTemplateName, + Namespace: namespace, + }, moduleTemplate); err != nil { + return nil, fmt.Errorf("failed to get module template: %w", err) + } + + return moduleTemplate, nil +} diff --git a/pkg/templatelookup/moduletemplateinfolookup/strategies.go b/pkg/templatelookup/moduletemplateinfolookup/strategies.go new file mode 100644 index 0000000000..441240b8fe --- /dev/null +++ b/pkg/templatelookup/moduletemplateinfolookup/strategies.go @@ -0,0 +1,50 @@ +package moduletemplateinfolookup + +import ( + "context" + "errors" + + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup" +) + +var ErrNoResponsibleStrategy = errors.New("failed to find responsible module template lookup strategy") + +type ModuleTemplateInfoLookupStrategy interface { + // IsResponsible checks if the strategy is responsible for the given module installation. + IsResponsible(moduleInfo *templatelookup.ModuleInfo, + moduleReleaseMeta *v1beta2.ModuleReleaseMeta, + ) bool + // Lookup looks up the required module template. + Lookup(ctx context.Context, + moduleInfo *templatelookup.ModuleInfo, + kyma *v1beta2.Kyma, + moduleReleaseMeta *v1beta2.ModuleReleaseMeta, + ) templatelookup.ModuleTemplateInfo +} + +// ModuleTemplateInfoLookupStrategies is a strategy that aggregates multiple ModuleTemplateInfoLookupStrategies. +// It iterates over the strategies and uses the first one that is responsible for the given module info. +type ModuleTemplateInfoLookupStrategies struct { + strategies []ModuleTemplateInfoLookupStrategy +} + +func NewModuleTemplateInfoLookupStrategies(strategies []ModuleTemplateInfoLookupStrategy) ModuleTemplateInfoLookupStrategies { + return ModuleTemplateInfoLookupStrategies{strategies: strategies} +} + +func (s ModuleTemplateInfoLookupStrategies) Lookup(ctx context.Context, + moduleInfo *templatelookup.ModuleInfo, + kyma *v1beta2.Kyma, + moduleReleaseMeta *v1beta2.ModuleReleaseMeta, +) templatelookup.ModuleTemplateInfo { + for _, strategy := range s.strategies { + if strategy.IsResponsible(moduleInfo, moduleReleaseMeta) { + return strategy.Lookup(ctx, moduleInfo, kyma, moduleReleaseMeta) + } + } + + return templatelookup.ModuleTemplateInfo{ + Err: ErrNoResponsibleStrategy, + } +} diff --git a/pkg/templatelookup/moduletemplateinfolookup/strategies_test.go b/pkg/templatelookup/moduletemplateinfolookup/strategies_test.go new file mode 100644 index 0000000000..8ca6399f6e --- /dev/null +++ b/pkg/templatelookup/moduletemplateinfolookup/strategies_test.go @@ -0,0 +1,91 @@ +package moduletemplateinfolookup_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/kyma-project/lifecycle-manager/api/v1beta2" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup" +) + +func Test_ModuleTemplateInfoLookupStrategies_Lookup_CallsResponsibleStrategy(t *testing.T) { + nonResponsibleStrategy := newLookupStrategyStub(false) + responsibleStrategy := newLookupStrategyStub(true) + strategies := moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + &nonResponsibleStrategy, + &responsibleStrategy, + }) + + moduleTemplateInfo := strategies.Lookup(context.Background(), nil, nil, nil) + + assert.True(t, responsibleStrategy.called) + assert.False(t, nonResponsibleStrategy.called) + require.NoError(t, moduleTemplateInfo.Err) +} + +func Test_ModuleTemplateInfoLookupStrategies_Lookup_CallsFirstResponsibleStrategy(t *testing.T) { + nonResponsibleStrategy := newLookupStrategyStub(false) + responsibleStrategy := newLookupStrategyStub(true) + responsibleStrategy2 := newLookupStrategyStub(true) + strategies := moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + &nonResponsibleStrategy, + &responsibleStrategy, + &responsibleStrategy2, + }) + + moduleTemplateInfo := strategies.Lookup(context.Background(), nil, nil, nil) + + assert.True(t, responsibleStrategy.called) + assert.False(t, responsibleStrategy2.called) + assert.False(t, nonResponsibleStrategy.called) + require.NoError(t, moduleTemplateInfo.Err) +} + +func Test_ModuleTemplateInfoLookupStrategies_Lookup_ReturnsFailureWhenNoStrategyResponsible(t *testing.T) { + nonResponsibleStrategy := newLookupStrategyStub(false) + strategies := moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + &nonResponsibleStrategy, + }) + + moduleTemplateInfo := strategies.Lookup(context.Background(), nil, nil, nil) + + assert.False(t, nonResponsibleStrategy.called) + require.ErrorIs(t, moduleTemplateInfo.Err, moduletemplateinfolookup.ErrNoResponsibleStrategy) +} + +func Test_ModuleTemplateInfoLookupStrategies_Lookup_ReturnsFailureWhenNoStrategies(t *testing.T) { + strategies := moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{}) + + moduleTemplateInfo := strategies.Lookup(context.Background(), nil, nil, nil) + + require.ErrorIs(t, moduleTemplateInfo.Err, moduletemplateinfolookup.ErrNoResponsibleStrategy) +} + +func newLookupStrategyStub(responsible bool) LookupStrategyStub { + return LookupStrategyStub{ + responsible: responsible, + } +} + +type LookupStrategyStub struct { + responsible bool + called bool +} + +func (s *LookupStrategyStub) Lookup(ctx context.Context, + _ *templatelookup.ModuleInfo, + _ *v1beta2.Kyma, + _ *v1beta2.ModuleReleaseMeta, +) templatelookup.ModuleTemplateInfo { + s.called = true + return templatelookup.ModuleTemplateInfo{} +} + +func (s *LookupStrategyStub) IsResponsible(_ *templatelookup.ModuleInfo, _ *v1beta2.ModuleReleaseMeta, +) bool { + return s.responsible +} diff --git a/pkg/templatelookup/regular.go b/pkg/templatelookup/regular.go index f617215507..c13703c6ee 100644 --- a/pkg/templatelookup/regular.go +++ b/pkg/templatelookup/regular.go @@ -9,27 +9,16 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/kyma-project/lifecycle-manager/api/shared" "github.com/kyma-project/lifecycle-manager/api/v1beta2" "github.com/kyma-project/lifecycle-manager/internal/descriptor/provider" "github.com/kyma-project/lifecycle-manager/internal/remote" - "github.com/kyma-project/lifecycle-manager/pkg/log" ) var ( - ErrTemplateNotIdentified = errors.New("no unique template could be identified") - ErrNotDefaultChannelAllowed = errors.New("specifying no default channel is not allowed") - ErrNoTemplatesInListResult = errors.New("no templates were found") - ErrTemplateMarkedAsMandatory = errors.New("template marked as mandatory") - ErrTemplateNotAllowed = errors.New("module template not allowed") - ErrTemplateUpdateNotAllowed = errors.New("module template update not allowed") + ErrTemplateNotAllowed = errors.New("module template not allowed") + ErrTemplateUpdateNotAllowed = errors.New("module template update not allowed") ) -type MaintenanceWindow interface { - IsRequired(moduleTemplate *v1beta2.ModuleTemplate, kyma *v1beta2.Kyma) bool - IsActive(kyma *v1beta2.Kyma) (bool, error) -} - type ModuleTemplateInfo struct { *v1beta2.ModuleTemplate Err error @@ -37,57 +26,69 @@ type ModuleTemplateInfo struct { DesiredChannel string } +type ModuleTemplateInfoLookupStrategy interface { + Lookup(ctx context.Context, + moduleInfo *ModuleInfo, + kyma *v1beta2.Kyma, + moduleReleaseMeta *v1beta2.ModuleReleaseMeta, + ) ModuleTemplateInfo +} + func NewTemplateLookup(reader client.Reader, descriptorProvider *provider.CachedDescriptorProvider, - maintenanceWindow MaintenanceWindow, + moduleTemplateInfoLookupStrategy ModuleTemplateInfoLookupStrategy, ) *TemplateLookup { return &TemplateLookup{ - Reader: reader, - descriptorProvider: descriptorProvider, - maintenanceWindow: maintenanceWindow, + Reader: reader, + descriptorProvider: descriptorProvider, + moduleTemplateInfoLookupStrategy: moduleTemplateInfoLookupStrategy, } } type TemplateLookup struct { client.Reader - descriptorProvider *provider.CachedDescriptorProvider - maintenanceWindow MaintenanceWindow + descriptorProvider *provider.CachedDescriptorProvider + moduleTemplateInfoLookupStrategy ModuleTemplateInfoLookupStrategy } type ModuleTemplatesByModuleName map[string]*ModuleTemplateInfo func (t *TemplateLookup) GetRegularTemplates(ctx context.Context, kyma *v1beta2.Kyma) ModuleTemplatesByModuleName { templates := make(ModuleTemplatesByModuleName) - for _, module := range FetchModuleInfo(kyma) { - _, found := templates[module.Name] + for _, moduleInfo := range FetchModuleInfo(kyma) { + _, found := templates[moduleInfo.Name] if found { continue } - if module.ValidationError != nil { - templates[module.Name] = &ModuleTemplateInfo{Err: module.ValidationError} + if moduleInfo.ValidationError != nil { + templates[moduleInfo.Name] = &ModuleTemplateInfo{Err: moduleInfo.ValidationError} continue } - moduleReleaseMeta, err := GetModuleReleaseMeta(ctx, t, module.Name, kyma.Namespace) + moduleReleaseMeta, err := GetModuleReleaseMeta(ctx, t, moduleInfo.Name, kyma.Namespace) if client.IgnoreNotFound(err) != nil { - templates[module.Name] = &ModuleTemplateInfo{Err: err} + templates[moduleInfo.Name] = &ModuleTemplateInfo{Err: err} continue } - templateInfo := t.PopulateModuleTemplateInfo(ctx, module, kyma.Namespace, kyma.Spec.Channel, moduleReleaseMeta) + templateInfo := t.moduleTemplateInfoLookupStrategy.Lookup(ctx, + &moduleInfo, + kyma, + moduleReleaseMeta) + templateInfo = ValidateTemplateMode(templateInfo, kyma, moduleReleaseMeta) if templateInfo.Err != nil { - templates[module.Name] = &templateInfo + templates[moduleInfo.Name] = &templateInfo continue } if err := t.descriptorProvider.Add(templateInfo.ModuleTemplate); err != nil { templateInfo.Err = fmt.Errorf("failed to get descriptor: %w", err) - templates[module.Name] = &templateInfo + templates[moduleInfo.Name] = &templateInfo continue } for i := range kyma.Status.Modules { moduleStatus := &kyma.Status.Modules[i] - if moduleMatch(moduleStatus, module.Name) { + if moduleMatch(moduleStatus, moduleInfo.Name) { descriptor, err := t.descriptorProvider.GetDescriptor(templateInfo.ModuleTemplate) if err != nil { msg := "could not handle channel skew as descriptor from template cannot be fetched" @@ -97,56 +98,11 @@ func (t *TemplateLookup) GetRegularTemplates(ctx context.Context, kyma *v1beta2. markInvalidSkewUpdate(ctx, &templateInfo, moduleStatus, descriptor.Version) } } - templates[module.Name] = &templateInfo + templates[moduleInfo.Name] = &templateInfo } return templates } -func (t *TemplateLookup) PopulateModuleTemplateInfo(ctx context.Context, - module ModuleInfo, namespace, kymaChannel string, moduleReleaseMeta *v1beta2.ModuleReleaseMeta, -) ModuleTemplateInfo { - if moduleReleaseMeta == nil { - return t.populateModuleTemplateInfoWithoutModuleReleaseMeta(ctx, module, kymaChannel) - } - - return t.populateModuleTemplateInfoUsingModuleReleaseMeta(ctx, module, moduleReleaseMeta, kymaChannel, namespace) -} - -func (t *TemplateLookup) populateModuleTemplateInfoWithoutModuleReleaseMeta(ctx context.Context, - module ModuleInfo, kymaChannel string, -) ModuleTemplateInfo { - var templateInfo ModuleTemplateInfo - if module.IsInstalledByVersion() { - templateInfo = t.GetAndValidateByVersion(ctx, module.Name, module.Version) - } else { - templateInfo = t.GetAndValidateByChannel(ctx, module.Name, module.Channel, kymaChannel) - } - return templateInfo -} - -func (t *TemplateLookup) populateModuleTemplateInfoUsingModuleReleaseMeta(ctx context.Context, - module ModuleInfo, - moduleReleaseMeta *v1beta2.ModuleReleaseMeta, kymaChannel, namespace string, -) ModuleTemplateInfo { - var templateInfo ModuleTemplateInfo - templateInfo.DesiredChannel = getDesiredChannel(module.Channel, kymaChannel) - desiredModuleVersion, err := GetChannelVersionForModule(moduleReleaseMeta, templateInfo.DesiredChannel) - if err != nil { - templateInfo.Err = err - return templateInfo - } - - template, err := t.getTemplateByVersion(ctx, module.Name, desiredModuleVersion, namespace) - if err != nil { - templateInfo.Err = err - return templateInfo - } - - templateInfo.ModuleTemplate = template - - return templateInfo -} - func ValidateTemplateMode(template ModuleTemplateInfo, kyma *v1beta2.Kyma, moduleReleaseMeta *v1beta2.ModuleReleaseMeta, @@ -184,83 +140,6 @@ func validateTemplateModeWithModuleReleaseMeta(template ModuleTemplateInfo, kyma return template } -func (t *TemplateLookup) getTemplateByVersion(ctx context.Context, - moduleName, moduleVersion, namespace string, -) (*v1beta2.ModuleTemplate, error) { - moduleTemplate := &v1beta2.ModuleTemplate{} - - moduleTemplateName := fmt.Sprintf("%s-%s", moduleName, moduleVersion) - if err := t.Get(ctx, client.ObjectKey{ - Name: moduleTemplateName, - Namespace: namespace, - }, moduleTemplate); err != nil { - return nil, fmt.Errorf("failed to get module template: %w", err) - } - - return moduleTemplate, nil -} - -func (t *TemplateLookup) GetAndValidateByChannel(ctx context.Context, - name, channel, defaultChannel string, -) ModuleTemplateInfo { - desiredChannel := getDesiredChannel(channel, defaultChannel) - info := ModuleTemplateInfo{ - DesiredChannel: desiredChannel, - } - - template, err := t.filterTemplatesByChannel(ctx, name, desiredChannel) - if err != nil { - info.Err = err - return info - } - - actualChannel := template.Spec.Channel - if actualChannel == "" { - info.Err = fmt.Errorf( - "no channel found on template for module: %s: %w", - name, ErrNotDefaultChannelAllowed, - ) - return info - } - - logUsedChannel(ctx, name, actualChannel, defaultChannel) - info.ModuleTemplate = template - return info -} - -func (t *TemplateLookup) GetAndValidateByVersion(ctx context.Context, name, version string) ModuleTemplateInfo { - info := ModuleTemplateInfo{ - DesiredChannel: string(shared.NoneChannel), - } - template, err := t.filterTemplatesByVersion(ctx, name, version) - if err != nil { - info.Err = err - return info - } - - info.ModuleTemplate = template - return info -} - -func logUsedChannel(ctx context.Context, name string, actualChannel string, defaultChannel string) { - logger := logf.FromContext(ctx) - if actualChannel != defaultChannel { - logger.V(log.DebugLevel).Info( - fmt.Sprintf( - "using %s (instead of %s) for module %s", - actualChannel, defaultChannel, name, - ), - ) - } else { - logger.V(log.DebugLevel).Info( - fmt.Sprintf( - "using %s for module %s", - actualChannel, name, - ), - ) - } -} - func moduleMatch(moduleStatus *v1beta2.ModuleStatus, moduleName string) bool { return moduleStatus.Name == moduleName } @@ -328,107 +207,3 @@ func filterVersion(version *semver.Version) *semver.Version { version.Major(), version.Minor(), version.Patch())) return filteredVersion } - -func getDesiredChannel(moduleChannel, globalChannel string) string { - var desiredChannel string - - switch { - case moduleChannel != "": - desiredChannel = moduleChannel - case globalChannel != "": - desiredChannel = globalChannel - default: - desiredChannel = v1beta2.DefaultChannel - } - - return desiredChannel -} - -func (t *TemplateLookup) filterTemplatesByChannel(ctx context.Context, name, desiredChannel string) ( - *v1beta2.ModuleTemplate, error, -) { - templateList := &v1beta2.ModuleTemplateList{} - err := t.List(ctx, templateList) - if err != nil { - return nil, fmt.Errorf("failed to list module templates on lookup: %w", err) - } - - var filteredTemplates []*v1beta2.ModuleTemplate - for _, template := range templateList.Items { - if TemplateNameMatch(&template, name) && template.Spec.Channel == desiredChannel { - filteredTemplates = append(filteredTemplates, &template) - continue - } - } - - if len(filteredTemplates) > 1 { - return nil, NewMoreThanOneTemplateCandidateErr(name, templateList.Items) - } - - if len(filteredTemplates) == 0 { - return nil, fmt.Errorf("%w: for module %s in channel %s ", - ErrNoTemplatesInListResult, name, desiredChannel) - } - - if filteredTemplates[0].Spec.Mandatory { - return nil, fmt.Errorf("%w: for module %s in channel %s", - ErrTemplateMarkedAsMandatory, name, desiredChannel) - } - - return filteredTemplates[0], nil -} - -func (t *TemplateLookup) filterTemplatesByVersion(ctx context.Context, name, version string) ( - *v1beta2.ModuleTemplate, error, -) { - templateList := &v1beta2.ModuleTemplateList{} - err := t.List(ctx, templateList) - if err != nil { - return nil, fmt.Errorf("failed to list module templates on lookup: %w", err) - } - - var filteredTemplates []*v1beta2.ModuleTemplate - for _, template := range templateList.Items { - if TemplateNameMatch(&template, - name) && shared.NoneChannel.Equals(template.Spec.Channel) && template.Spec.Version == version { - filteredTemplates = append(filteredTemplates, &template) - continue - } - } - if len(filteredTemplates) > 1 { - return nil, NewMoreThanOneTemplateCandidateErr(name, templateList.Items) - } - if len(filteredTemplates) == 0 { - return nil, fmt.Errorf("%w: for module %s in version %s", - ErrNoTemplatesInListResult, name, version) - } - if filteredTemplates[0].Spec.Mandatory { - return nil, fmt.Errorf("%w: for module %s in version %s", - ErrTemplateMarkedAsMandatory, name, version) - } - return filteredTemplates[0], nil -} - -func TemplateNameMatch(template *v1beta2.ModuleTemplate, name string) bool { - if len(template.Spec.ModuleName) > 0 { - return template.Spec.ModuleName == name - } - - // Drop the legacyCondition once the label 'shared.ModuleName' is removed: https://github.com/kyma-project/lifecycle-manager/issues/1796 - if template.Labels == nil { - return false - } - return template.Labels[shared.ModuleName] == name -} - -func NewMoreThanOneTemplateCandidateErr(moduleName string, - candidateTemplates []v1beta2.ModuleTemplate, -) error { - candidates := make([]string, len(candidateTemplates)) - for i, candidate := range candidateTemplates { - candidates[i] = candidate.GetName() - } - - return fmt.Errorf("%w: more than one module template found for module: %s, candidates: %v", - ErrTemplateNotIdentified, moduleName, candidates) -} diff --git a/pkg/templatelookup/regular_test.go b/pkg/templatelookup/regular_test.go index 98a9175309..5f55d33782 100644 --- a/pkg/templatelookup/regular_test.go +++ b/pkg/templatelookup/regular_test.go @@ -21,6 +21,7 @@ import ( "github.com/kyma-project/lifecycle-manager/internal/descriptor/provider" "github.com/kyma-project/lifecycle-manager/internal/descriptor/types" "github.com/kyma-project/lifecycle-manager/pkg/templatelookup" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup" "github.com/kyma-project/lifecycle-manager/pkg/testutils" "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" ) @@ -332,7 +333,11 @@ func Test_GetRegularTemplates_WhenInvalidModuleProvided(t *testing.T) { for _, tt := range tests { test := tt t.Run(tt.name, func(t *testing.T) { - lookup := templatelookup.NewTemplateLookup(nil, provider.NewCachedDescriptorProvider(), maintenanceWindowStub{}) + lookup := templatelookup.NewTemplateLookup(nil, provider.NewCachedDescriptorProvider(), moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + moduletemplateinfolookup.NewByVersionStrategy(nil), + moduletemplateinfolookup.NewByChannelStrategy(nil), + moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(nil), + })) kyma := &v1beta2.Kyma{ Spec: test.KymaSpec, Status: test.KymaStatus, @@ -464,10 +469,14 @@ func TestTemplateLookup_GetRegularTemplates_WhenSwitchModuleChannel(t *testing.T for _, testCase := range tests { t.Run(testCase.name, func(t *testing.T) { - lookup := templatelookup.NewTemplateLookup(NewFakeModuleTemplateReader(testCase.availableModuleTemplate, - testCase.availableModuleReleaseMeta), + reader := NewFakeModuleTemplateReader(testCase.availableModuleTemplate, testCase.availableModuleReleaseMeta) + lookup := templatelookup.NewTemplateLookup(reader, provider.NewCachedDescriptorProvider(), - maintenanceWindowStub{}) + moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + moduletemplateinfolookup.NewByVersionStrategy(reader), + moduletemplateinfolookup.NewByChannelStrategy(reader), + moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(reader), + })) got := lookup.GetRegularTemplates(context.TODO(), testCase.kyma) assert.Equal(t, len(got), len(testCase.want)) for key, module := range got { @@ -494,13 +503,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenSwitchBetweenModuleVersions(t *t availableModuleReleaseMetas := v1beta2.ModuleReleaseMetaList{} - tests := []struct { - name string - kyma *v1beta2.Kyma - wantVersion string - wantChannel string - wantErrContains string - }{ + tests := getRegularTemplatesTestCases{ { name: "When upgrade version, then result contains no error", kyma: builder.NewKymaBuilder(). @@ -536,25 +539,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenSwitchBetweenModuleVersions(t *t }, } - for _, testCase := range tests { - t.Run(testCase.name, func(t *testing.T) { - lookup := templatelookup.NewTemplateLookup(NewFakeModuleTemplateReader(availableModuleTemplates, - availableModuleReleaseMetas), - provider.NewCachedDescriptorProvider(), - maintenanceWindowStub{}) - got := lookup.GetRegularTemplates(context.TODO(), testCase.kyma) - assert.Len(t, got, 1) - for key, module := range got { - assert.Equal(t, key, moduleToInstall.Name) - if testCase.wantErrContains != "" { - assert.Contains(t, module.Err.Error(), testCase.wantErrContains) - } else { - assert.Equal(t, testCase.wantChannel, module.DesiredChannel) - assert.Equal(t, testCase.wantVersion, module.ModuleTemplate.Spec.Version) - } - } - }) - } + executeGetRegularTemplatesTestCases(t, tests, availableModuleTemplates, availableModuleReleaseMetas, moduleToInstall) } func TestTemplateLookup_GetRegularTemplates_WhenSwitchFromChannelToVersion(t *testing.T) { @@ -570,13 +555,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenSwitchFromChannelToVersion(t *te availableModuleReleaseMetas := v1beta2.ModuleReleaseMetaList{} - tests := []struct { - name string - kyma *v1beta2.Kyma - wantVersion string - wantChannel string - wantErrContains string - }{ + tests := getRegularTemplatesTestCases{ { name: "When staying with the same version, then result contains no error", kyma: builder.NewKymaBuilder(). @@ -629,25 +608,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenSwitchFromChannelToVersion(t *te }, } - for _, testCase := range tests { - t.Run(testCase.name, func(t *testing.T) { - lookup := templatelookup.NewTemplateLookup(NewFakeModuleTemplateReader(availableModuleTemplates, - availableModuleReleaseMetas), - provider.NewCachedDescriptorProvider(), - maintenanceWindowStub{}) - got := lookup.GetRegularTemplates(context.TODO(), testCase.kyma) - assert.Len(t, got, 1) - for key, module := range got { - assert.Equal(t, key, moduleToInstall.Name) - if testCase.wantErrContains != "" { - assert.Contains(t, module.Err.Error(), testCase.wantErrContains) - } else { - assert.Equal(t, testCase.wantChannel, module.DesiredChannel) - assert.Equal(t, testCase.wantVersion, module.ModuleTemplate.Spec.Version) - } - } - }) - } + executeGetRegularTemplatesTestCases(t, tests, availableModuleTemplates, availableModuleReleaseMetas, moduleToInstall) } func TestTemplateLookup_GetRegularTemplates_WhenSwitchFromVersionToChannel(t *testing.T) { @@ -663,13 +624,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenSwitchFromVersionToChannel(t *te availableModuleReleaseMetas := v1beta2.ModuleReleaseMetaList{} - tests := []struct { - name string - kyma *v1beta2.Kyma - wantVersion string - wantChannel string - wantErrContains string - }{ + tests := getRegularTemplatesTestCases{ { name: "When staying with the same version, then result contains no error", kyma: builder.NewKymaBuilder(). @@ -722,25 +677,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenSwitchFromVersionToChannel(t *te }, } - for _, testCase := range tests { - t.Run(testCase.name, func(t *testing.T) { - lookup := templatelookup.NewTemplateLookup(NewFakeModuleTemplateReader(availableModuleTemplates, - availableModuleReleaseMetas), - provider.NewCachedDescriptorProvider(), - maintenanceWindowStub{}) - got := lookup.GetRegularTemplates(context.TODO(), testCase.kyma) - assert.Len(t, got, 1) - for key, module := range got { - assert.Equal(t, key, moduleToInstall.Name) - if testCase.wantErrContains != "" { - assert.Contains(t, module.Err.Error(), testCase.wantErrContains) - } else { - assert.Equal(t, testCase.wantChannel, module.DesiredChannel) - assert.Equal(t, testCase.wantVersion, module.ModuleTemplate.Spec.Version) - } - } - }) - } + executeGetRegularTemplatesTestCases(t, tests, availableModuleTemplates, availableModuleReleaseMetas, moduleToInstall) } func TestNewTemplateLookup_GetRegularTemplates_WhenModuleTemplateContainsInvalidDescriptor(t *testing.T) { @@ -838,10 +775,15 @@ func TestNewTemplateLookup_GetRegularTemplates_WhenModuleTemplateContainsInvalid }).Build()) } } - lookup := templatelookup.NewTemplateLookup(NewFakeModuleTemplateReader(*givenTemplateList, - moduleReleaseMetas), + reader := NewFakeModuleTemplateReader(*givenTemplateList, + moduleReleaseMetas) + lookup := templatelookup.NewTemplateLookup(reader, provider.NewCachedDescriptorProvider(), - maintenanceWindowStub{}) + moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + moduletemplateinfolookup.NewByVersionStrategy(reader), + moduletemplateinfolookup.NewByChannelStrategy(reader), + moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(reader), + })) got := lookup.GetRegularTemplates(context.TODO(), testCase.kyma) assert.Equal(t, len(got), len(testCase.want)) for key, module := range got { @@ -874,7 +816,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateNotFound(t *testin want: templatelookup.ModuleTemplatesByModuleName{ testModule.Name: &templatelookup.ModuleTemplateInfo{ DesiredChannel: testModule.Channel, - Err: templatelookup.ErrNoTemplatesInListResult, + Err: moduletemplateinfolookup.ErrNoTemplatesInListResult, }, }, }, @@ -893,7 +835,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateNotFound(t *testin want: templatelookup.ModuleTemplatesByModuleName{ testModule.Name: &templatelookup.ModuleTemplateInfo{ DesiredChannel: testModule.Channel, - Err: templatelookup.ErrNoTemplatesInListResult, + Err: moduletemplateinfolookup.ErrNoTemplatesInListResult, }, }, }, @@ -901,10 +843,15 @@ func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateNotFound(t *testin for _, testCase := range tests { t.Run(testCase.name, func(t *testing.T) { givenTemplateList := &v1beta2.ModuleTemplateList{} - lookup := templatelookup.NewTemplateLookup(NewFakeModuleTemplateReader(*givenTemplateList, - v1beta2.ModuleReleaseMetaList{}), + reader := NewFakeModuleTemplateReader(*givenTemplateList, + v1beta2.ModuleReleaseMetaList{}) + lookup := templatelookup.NewTemplateLookup(reader, provider.NewCachedDescriptorProvider(), - maintenanceWindowStub{}) + moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + moduletemplateinfolookup.NewByVersionStrategy(reader), + moduletemplateinfolookup.NewByChannelStrategy(reader), + moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(reader), + })) got := lookup.GetRegularTemplates(context.TODO(), testCase.kyma) assert.Equal(t, len(got), len(testCase.want)) for key, module := range got { @@ -1039,10 +986,15 @@ func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateExists(t *testing. WithOCM(compdescv2.SchemaVersion).Build()) } } - lookup := templatelookup.NewTemplateLookup(NewFakeModuleTemplateReader(*givenTemplateList, - moduleReleaseMetas), + reader := NewFakeModuleTemplateReader(*givenTemplateList, + moduleReleaseMetas) + lookup := templatelookup.NewTemplateLookup(reader, provider.NewCachedDescriptorProvider(), - maintenanceWindowStub{}) + moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + moduletemplateinfolookup.NewByVersionStrategy(reader), + moduletemplateinfolookup.NewByChannelStrategy(reader), + moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(reader), + })) got := lookup.GetRegularTemplates(context.TODO(), testCase.kyma) assert.Equal(t, len(got), len(testCase.want)) for key, module := range got { @@ -1139,13 +1091,53 @@ func TestTemplateNameMatch(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := templatelookup.TemplateNameMatch(&tt.template, targetName); got != tt.want { + if got := moduletemplateinfolookup.TemplateNameMatch(&tt.template, targetName); got != tt.want { assert.Equal(t, tt.want, got) } }) } } +type getRegularTemplatesTestCases []struct { + name string + kyma *v1beta2.Kyma + wantVersion string + wantChannel string + wantErrContains string +} + +func executeGetRegularTemplatesTestCases(t *testing.T, + testCases getRegularTemplatesTestCases, + availableModuleTemplates v1beta2.ModuleTemplateList, + availableModuleReleaseMetas v1beta2.ModuleReleaseMetaList, + moduleToInstall v1beta2.Module, +) { + t.Helper() + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + reader := NewFakeModuleTemplateReader(availableModuleTemplates, availableModuleReleaseMetas) + lookup := templatelookup.NewTemplateLookup(reader, + provider.NewCachedDescriptorProvider(), + moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + moduletemplateinfolookup.NewByVersionStrategy(reader), + moduletemplateinfolookup.NewByChannelStrategy(reader), + moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(reader), + })) + got := lookup.GetRegularTemplates(context.TODO(), testCase.kyma) + assert.Len(t, got, 1) + for key, module := range got { + assert.Equal(t, key, moduleToInstall.Name) + if testCase.wantErrContains != "" { + assert.Contains(t, module.Err.Error(), testCase.wantErrContains) + } else { + assert.Equal(t, testCase.wantChannel, module.DesiredChannel) + assert.Equal(t, testCase.wantVersion, module.ModuleTemplate.Spec.Version) + } + } + }) + } +} + func generateModuleTemplateListWithModule(moduleName, moduleChannel, moduleVersion string) v1beta2.ModuleTemplateList { templateList := v1beta2.ModuleTemplateList{} templateList.Items = append(templateList.Items, *builder.NewModuleTemplateBuilder(). @@ -1199,13 +1191,3 @@ func (mtlb *ModuleTemplateListBuilder) Build() v1beta2.ModuleTemplateList { func moduleToInstallByVersion(moduleName, moduleVersion string) v1beta2.Module { return testutils.NewTestModuleWithChannelVersion(moduleName, "", moduleVersion) } - -type maintenanceWindowStub struct{} - -func (m maintenanceWindowStub) IsRequired(moduleTemplate *v1beta2.ModuleTemplate, kyma *v1beta2.Kyma) bool { - return false -} - -func (m maintenanceWindowStub) IsActive(kyma *v1beta2.Kyma) (bool, error) { - return false, nil -} diff --git a/pkg/testutils/moduletemplate.go b/pkg/testutils/moduletemplate.go index 707dcb8a07..a649e4cde0 100644 --- a/pkg/testutils/moduletemplate.go +++ b/pkg/testutils/moduletemplate.go @@ -12,6 +12,7 @@ import ( "github.com/kyma-project/lifecycle-manager/api/v1beta2" "github.com/kyma-project/lifecycle-manager/internal/descriptor/provider" "github.com/kyma-project/lifecycle-manager/pkg/templatelookup" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup" "github.com/kyma-project/lifecycle-manager/pkg/util" ) @@ -29,23 +30,23 @@ func CreateModuleTemplate(ctx context.Context, func GetModuleTemplate(ctx context.Context, clnt client.Client, module v1beta2.Module, - defaultChannel string, - namespace string, + kyma *v1beta2.Kyma, ) (*v1beta2.ModuleTemplate, error) { - descriptorProvider := provider.NewCachedDescriptorProvider() - // replace maintenancePolicyHandlerStub with proper implementation for tests - templateLookup := templatelookup.NewTemplateLookup(clnt, descriptorProvider, maintenanceWindowStub{}) + moduleTemplateInfoLookupStrategies := moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + moduletemplateinfolookup.NewByVersionStrategy(clnt), + moduletemplateinfolookup.NewByChannelStrategy(clnt), + moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(clnt), + }) availableModule := templatelookup.ModuleInfo{ Module: module, } - moduleReleaseMeta, err := GetModuleReleaseMeta(ctx, module.Name, namespace, clnt) + moduleReleaseMeta, err := GetModuleReleaseMeta(ctx, module.Name, kyma.Namespace, clnt) if !meta.IsNoMatchError(err) && client.IgnoreNotFound(err) != nil { return nil, fmt.Errorf("failed to get ModuleReleaseMeta: %w", err) } - templateInfo := templateLookup.PopulateModuleTemplateInfo(ctx, availableModule, namespace, - defaultChannel, moduleReleaseMeta) + templateInfo := moduleTemplateInfoLookupStrategies.Lookup(ctx, &availableModule, kyma, moduleReleaseMeta) if templateInfo.Err != nil { return nil, fmt.Errorf("get module template: %w", templateInfo.Err) @@ -56,11 +57,10 @@ func GetModuleTemplate(ctx context.Context, func ModuleTemplateExists(ctx context.Context, clnt client.Client, module v1beta2.Module, - defaultChannel string, - namespace string, + kyma *v1beta2.Kyma, ) error { - moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, defaultChannel, namespace) - if moduleTemplate == nil || errors.Is(err, templatelookup.ErrNoTemplatesInListResult) { + moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kyma) + if moduleTemplate == nil || errors.Is(err, moduletemplateinfolookup.ErrNoTemplatesInListResult) { return ErrNotFound } @@ -85,7 +85,7 @@ func ModuleTemplateExistsByName(ctx context.Context, func AllModuleTemplatesExists(ctx context.Context, clnt client.Client, kyma *v1beta2.Kyma) error { for _, module := range kyma.Spec.Modules { - if err := ModuleTemplateExists(ctx, clnt, module, kyma.Spec.Channel, kyma.Namespace); err != nil { + if err := ModuleTemplateExists(ctx, clnt, module, kyma); err != nil { return err } } @@ -97,11 +97,10 @@ func UpdateModuleTemplateSpec(ctx context.Context, clnt client.Client, module v1beta2.Module, key, - newValue, - kymaChannel string, - namespace string, + newValue string, + kyma *v1beta2.Kyma, ) error { - moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kymaChannel, namespace) + moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kyma) if err != nil { return err } @@ -141,9 +140,11 @@ func MandatoryModuleTemplateHasExpectedLabel(ctx context.Context, clnt client.Cl } func DeleteModuleTemplate(ctx context.Context, - clnt client.Client, module v1beta2.Module, kymaChannel string, namespace string, + clnt client.Client, + module v1beta2.Module, + kyma *v1beta2.Kyma, ) error { - moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kymaChannel, namespace) + moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kyma) if util.IsNotFound(err) { return nil } @@ -155,10 +156,12 @@ func DeleteModuleTemplate(ctx context.Context, return nil } -func ReadModuleVersionFromModuleTemplate(ctx context.Context, clnt client.Client, module v1beta2.Module, - channel string, namespace string, +func ReadModuleVersionFromModuleTemplate(ctx context.Context, + clnt client.Client, + module v1beta2.Module, + kyma *v1beta2.Kyma, ) (string, error) { - moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, channel, namespace) + moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kyma) if err != nil { return "", fmt.Errorf("failed to fetch ModuleTemplate: %w", err) } @@ -171,13 +174,3 @@ func ReadModuleVersionFromModuleTemplate(ctx context.Context, clnt client.Client return ocmDesc.Version, nil } - -type maintenanceWindowStub struct{} - -func (m maintenanceWindowStub) IsRequired(moduleTemplate *v1beta2.ModuleTemplate, kyma *v1beta2.Kyma) bool { - return false -} - -func (m maintenanceWindowStub) IsActive(kyma *v1beta2.Kyma) (bool, error) { - return false, nil -} diff --git a/tests/e2e/module_deletion_test.go b/tests/e2e/module_deletion_test.go index 663b1ebab9..8387af78c6 100644 --- a/tests/e2e/module_deletion_test.go +++ b/tests/e2e/module_deletion_test.go @@ -10,6 +10,7 @@ import ( . "github.com/onsi/gomega" . "github.com/kyma-project/lifecycle-manager/pkg/testutils" + "github.com/kyma-project/lifecycle-manager/pkg/testutils/builder" . "github.com/kyma-project/lifecycle-manager/tests/e2e/commontestutils" ) @@ -272,14 +273,19 @@ var _ = Describe("Non Blocking Kyma Module Deletion", Ordered, func() { It("When ModuleTemplate is removed from KCP Cluster", func() { Eventually(DeleteModuleTemplate). WithContext(ctx). - WithArguments(kcpClient, module, kyma.Spec.Channel, ControlPlaneNamespace). + WithArguments(kcpClient, module, kyma). Should(Succeed()) }) It("Then ModuleTemplate is no longer in SKR Cluster", func() { Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(skrClient, module, kyma.Spec.Channel, RemoteNamespace). + WithArguments(skrClient, + module, + builder.NewKymaBuilder(). + WithName(defaultRemoteKymaName). + WithNamespace(shared.DefaultRemoteNamespace). + Build()). Should(Equal(ErrNotFound)) }) }) diff --git a/tests/e2e/module_upgrade_new_version_test.go b/tests/e2e/module_upgrade_new_version_test.go index 525ae06263..edf1aaa8da 100644 --- a/tests/e2e/module_upgrade_new_version_test.go +++ b/tests/e2e/module_upgrade_new_version_test.go @@ -79,7 +79,7 @@ var _ = Describe("Module Upgrade By New Version", Ordered, func() { By("And Kyma Module Version in Kyma Status is updated") newModuleTemplateVersion, err := ReadModuleVersionFromModuleTemplate(ctx, kcpClient, module, - kyma.Spec.Channel, ControlPlaneNamespace) + kyma) Expect(err).ToNot(HaveOccurred()) Eventually(ModuleVersionInKymaStatusIsCorrect). diff --git a/tests/e2e/modulereleasemeta_maintenance_window_module_downtime_test.go b/tests/e2e/modulereleasemeta_maintenance_window_module_downtime_test.go index c6e8972959..5df61ed8eb 100644 --- a/tests/e2e/modulereleasemeta_maintenance_window_module_downtime_test.go +++ b/tests/e2e/modulereleasemeta_maintenance_window_module_downtime_test.go @@ -119,7 +119,7 @@ var _ = Describe("Maintenance Window With ModuleReleaseMeta and Module Downtime" By("And Kyma Module Version in Kyma Status is updated") newModuleTemplateVersion, err := ReadModuleVersionFromModuleTemplate(ctx, kcpClient, module, - kyma.Spec.Channel, ControlPlaneNamespace) + kyma) Expect(err).ToNot(HaveOccurred()) Eventually(ModuleVersionInKymaStatusIsCorrect). diff --git a/tests/e2e/modulereleasemeta_module_upgrade_new_version_test.go b/tests/e2e/modulereleasemeta_module_upgrade_new_version_test.go index fc81f3f0a0..3613f52dba 100644 --- a/tests/e2e/modulereleasemeta_module_upgrade_new_version_test.go +++ b/tests/e2e/modulereleasemeta_module_upgrade_new_version_test.go @@ -79,7 +79,7 @@ var _ = Describe("Module with ModuleReleaseMeta Upgrade By New Version", Ordered By("And Kyma Module Version in Kyma Status is updated") newModuleTemplateVersion, err := ReadModuleVersionFromModuleTemplate(ctx, kcpClient, module, - kyma.Spec.Channel, ControlPlaneNamespace) + kyma) Expect(err).ToNot(HaveOccurred()) Eventually(ModuleVersionInKymaStatusIsCorrect). diff --git a/tests/e2e/modulereleasemeta_not_allowed_installation_test.go b/tests/e2e/modulereleasemeta_not_allowed_installation_test.go index dda391ca4f..af5ea1ba3f 100644 --- a/tests/e2e/modulereleasemeta_not_allowed_installation_test.go +++ b/tests/e2e/modulereleasemeta_not_allowed_installation_test.go @@ -20,7 +20,7 @@ var _ = Describe("ModuleReleaseMeta Not Allowed Installation", Ordered, func() { By("The the ModuleTemplate exists in the KCP Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(kcpClient, module, v1beta2.DefaultChannel, ControlPlaneNamespace). + WithArguments(kcpClient, module, kyma). Should(Succeed()) By("And the ModuleReleaseMeta exists on the KCP Cluster") diff --git a/tests/e2e/modulereleasemeta_sync_test.go b/tests/e2e/modulereleasemeta_sync_test.go index 7f3c84862e..b6a8ee449a 100644 --- a/tests/e2e/modulereleasemeta_sync_test.go +++ b/tests/e2e/modulereleasemeta_sync_test.go @@ -14,17 +14,26 @@ import ( var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { kyma := NewKymaWithSyncLabel("kyma-sample", ControlPlaneNamespace, v1beta2.DefaultChannel) + var skrKyma *v1beta2.Kyma module := NewTemplateOperator(v1beta2.DefaultChannel) v1Version := "1.1.1-e2e-test" v2Version := "2.4.2-e2e-test" InitEmptyKymaBeforeAll(kyma) - Context("Given SKR Cluster with ModuleTemplate", func() { + Context("Given SKR Cluster", func() { + It("When Kyma can be fetched from SKR Cluster", func() { + var err error + skrKyma, err = GetKyma(ctx, skrClient, shared.DefaultRemoteKymaName, RemoteNamespace) + if err != nil { + Fail("Failed to get SKR Kyma") + } + }) + It("When Template Operator v1 ModuleTemplate is applied in the KCP Cluster with ModuleReleaseMeta", func() { By("Then the Template Operator v1 ModuleTemplate exists in the KCP Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(kcpClient, module, v1beta2.DefaultChannel, ControlPlaneNamespace). + WithArguments(kcpClient, module, kyma). Should(Succeed()) Eventually(ImmediatelyRequeueKyma). @@ -35,7 +44,7 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { By("And the Template Operator v1 ModuleTemplate exists in the SKR Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(skrClient, module, v1beta2.DefaultChannel, RemoteNamespace). + WithArguments(skrClient, module, skrKyma). Should(Succeed()) By("And the ModuleReleaseMeta exists on the KCP Cluster with the correct channel-version") @@ -119,7 +128,7 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { By("And the Template Operator v1 ModuleTemplate no longer exists in the SKR Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(skrClient, module, v1beta2.DefaultChannel, RemoteNamespace). + WithArguments(skrClient, module, skrKyma). Should(Equal(ErrNotFound)) }) @@ -138,7 +147,7 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { By("And the Template Operator v1 ModuleTemplate exists in the SKR Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(skrClient, module, v1beta2.DefaultChannel, RemoteNamespace). + WithArguments(skrClient, module, skrKyma). Should(Succeed()) }) @@ -161,7 +170,7 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { By("And the Template Operator v1 ModuleTemplate no longer exists in the SKR Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(skrClient, module, v1beta2.DefaultChannel, RemoteNamespace). + WithArguments(skrClient, module, skrKyma). Should(Equal(ErrNotFound)) }) @@ -180,26 +189,26 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { By("And the Template Operator v1 ModuleTemplate exists in the SKR Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(skrClient, module, v1beta2.DefaultChannel, RemoteNamespace). + WithArguments(skrClient, module, skrKyma). Should(Succeed()) }) It("When Template Operator v1 ModuleTemplate is removed from the KCP Cluster", func() { Eventually(DeleteModuleTemplate). WithContext(ctx). - WithArguments(kcpClient, module, v1beta2.DefaultChannel, ControlPlaneNamespace). + WithArguments(kcpClient, module, kyma). Should(Succeed()) By("Then Template Operator v1 ModuleTemplate no longer exists on the KCP Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(kcpClient, module, v1beta2.DefaultChannel, ControlPlaneNamespace). + WithArguments(kcpClient, module, kyma). Should(Equal(ErrNotFound)) By("Then Template Operator v1 ModuleTemplate no longer exists on the SKR Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(skrClient, module, v1beta2.DefaultChannel, RemoteNamespace). + WithArguments(skrClient, module, skrKyma). Should(Equal(ErrNotFound)) }) @@ -207,7 +216,7 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { By("And ModuleReleaseMeta is updated with the correct channel-version") Eventually(UpdateChannelVersionInModuleReleaseMeta). WithContext(ctx). - WithArguments(kcpClient, module.Name, ControlPlaneNamespace, v1beta2.DefaultChannel, NewerVersion). + WithArguments(kcpClient, module.Name, ControlPlaneNamespace, v1beta2.DefaultChannel, v2Version). Should(Succeed()) Eventually(ImmediatelyRequeueKyma). WithContext(ctx). @@ -217,13 +226,13 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { By("Then the Template Operator v2 ModuleTemplate exists in the KCP Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(kcpClient, module, v1beta2.DefaultChannel, ControlPlaneNamespace). + WithArguments(kcpClient, module, kyma). Should(Succeed()) By("And the Template Operator v2 ModuleTemplate exists in the SKR Cluster") Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(skrClient, module, v1beta2.DefaultChannel, RemoteNamespace). + WithArguments(skrClient, module, skrKyma). Should(Succeed()) By("And the ModuleReleaseMeta exists on the KCP Cluster with the correct channel-version") @@ -234,7 +243,7 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { Eventually(ModuleReleaseMetaContainsCorrectChannelVersion). WithContext(ctx). - WithArguments(module.Name, ControlPlaneNamespace, v1beta2.DefaultChannel, NewerVersion, kcpClient). + WithArguments(module.Name, ControlPlaneNamespace, v1beta2.DefaultChannel, v2Version, kcpClient). Should(Succeed()) By("And the ModuleReleaseMeta exists on the SKR Cluster with the correct channel-version") @@ -245,7 +254,7 @@ var _ = Describe("ModuleReleaseMeta Sync", Ordered, func() { Eventually(ModuleReleaseMetaContainsCorrectChannelVersion). WithContext(ctx). - WithArguments(module.Name, RemoteNamespace, v1beta2.DefaultChannel, NewerVersion, skrClient). + WithArguments(module.Name, RemoteNamespace, v1beta2.DefaultChannel, v2Version, skrClient). Should(Succeed()) }) diff --git a/tests/integration/controller/kcp/helper_test.go b/tests/integration/controller/kcp/helper_test.go index 5088a62103..57e4457a25 100644 --- a/tests/integration/controller/kcp/helper_test.go +++ b/tests/integration/controller/kcp/helper_test.go @@ -111,10 +111,9 @@ func watcherLabelsAnnotationsExist(clnt client.Client, remoteKyma *v1beta2.Kyma, func expectModuleTemplateSpecGetReset( clnt client.Client, module v1beta2.Module, - kymaChannel string, - namespace string, + kyma *v1beta2.Kyma, ) error { - moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kymaChannel, namespace) + moduleTemplate, err := GetModuleTemplate(ctx, clnt, module, kyma) if err != nil { return err } diff --git a/tests/integration/controller/kcp/remote_sync_test.go b/tests/integration/controller/kcp/remote_sync_test.go index edb0a79ff0..bcee3a7c93 100644 --- a/tests/integration/controller/kcp/remote_sync_test.go +++ b/tests/integration/controller/kcp/remote_sync_test.go @@ -103,11 +103,11 @@ var _ = Describe("Kyma sync into Remote Cluster", Ordered, func() { It("ModuleTemplates should be synchronized in both clusters", func() { By("ModuleTemplate exists in KCP cluster") Eventually(ModuleTemplateExists, Timeout, Interval). - WithArguments(ctx, kcpClient, moduleInKCP, kyma.Spec.Channel, ControlPlaneNamespace). + WithArguments(ctx, kcpClient, moduleInKCP, kyma). Should(Succeed()) By("ModuleTemplate exists in SKR cluster") - Eventually(ModuleTemplateExists, Timeout, Interval).WithArguments(ctx, skrClient, moduleInKCP, - kyma.Spec.Channel, RemoteNamespace).Should(Succeed()) + Eventually(ModuleTemplateExists, Timeout, Interval). + WithArguments(ctx, skrClient, moduleInKCP, skrKyma).Should(Succeed()) By("No module synced to remote Kyma") Eventually(NotContainsModuleInSpec, Timeout, Interval). @@ -117,7 +117,7 @@ var _ = Describe("Kyma sync into Remote Cluster", Ordered, func() { By("Remote Module Catalog created") Eventually(ModuleTemplateExists, Timeout, Interval). - WithArguments(ctx, skrClient, moduleInSKR, kyma.Spec.Channel, RemoteNamespace). + WithArguments(ctx, skrClient, moduleInSKR, skrKyma). Should(Succeed()) Eventually(containsModuleTemplateCondition, Timeout, Interval). WithArguments(skrClient, skrKyma.GetName(), flags.DefaultRemoteSyncNamespace). @@ -181,13 +181,12 @@ var _ = Describe("Kyma sync into Remote Cluster", Ordered, func() { By("Update SKR Module Template spec.data.spec field") Eventually(UpdateModuleTemplateSpec, Timeout, Interval). WithContext(ctx). - WithArguments(skrClient, moduleInSKR, InitSpecKey, "valueUpdated", kyma.Spec.Channel, RemoteNamespace). + WithArguments(skrClient, moduleInSKR, InitSpecKey, "valueUpdated", skrKyma). Should(Succeed()) By("Expect SKR Module Template spec.data.spec field get reset") Eventually(expectModuleTemplateSpecGetReset, 2*Timeout, Interval). - WithArguments(skrClient, - moduleInSKR, kyma.Spec.Channel, RemoteNamespace). + WithArguments(skrClient, moduleInSKR, skrKyma). Should(Succeed()) }) diff --git a/tests/integration/controller/kcp/suite_test.go b/tests/integration/controller/kcp/suite_test.go index e1d7429ec4..fda586c6ed 100644 --- a/tests/integration/controller/kcp/suite_test.go +++ b/tests/integration/controller/kcp/suite_test.go @@ -42,13 +42,13 @@ import ( "github.com/kyma-project/lifecycle-manager/internal/crd" "github.com/kyma-project/lifecycle-manager/internal/descriptor/provider" "github.com/kyma-project/lifecycle-manager/internal/event" - "github.com/kyma-project/lifecycle-manager/internal/maintenancewindows" "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/templatelookup" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup" . "github.com/kyma-project/lifecycle-manager/pkg/testutils" "github.com/kyma-project/lifecycle-manager/tests/integration" testskrcontext "github.com/kyma-project/lifecycle-manager/tests/integration/commontestutils/skrcontextimpl" @@ -143,7 +143,6 @@ var _ = BeforeSuite(func() { testSkrContextFactory = testskrcontext.NewDualClusterFactory(kcpClient.Scheme(), testEventRec) descriptorProvider = provider.NewCachedDescriptorProvider() crdCache = crd.NewCache(nil) - maintenanceWindow, _ := maintenancewindows.InitializeMaintenanceWindow(logr, "/not-required", "not-required") err = (&kyma.Reconciler{ Client: kcpClient, SkrContextFactory: testSkrContextFactory, @@ -156,7 +155,11 @@ var _ = BeforeSuite(func() { IsManagedKyma: true, Metrics: metrics.NewKymaMetrics(metrics.NewSharedMetrics()), RemoteCatalog: remote.NewRemoteCatalogFromKyma(kcpClient, testSkrContextFactory, flags.DefaultRemoteSyncNamespace), - TemplateLookup: templatelookup.NewTemplateLookup(kcpClient, descriptorProvider, maintenanceWindow), + TemplateLookup: templatelookup.NewTemplateLookup(kcpClient, descriptorProvider, moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + moduletemplateinfolookup.NewByVersionStrategy(kcpClient), + moduletemplateinfolookup.NewByChannelStrategy(kcpClient), + moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(kcpClient), + })), }).SetupWithManager(mgr, ctrlruntime.Options{}, kyma.SetupOptions{ListenerAddr: UseRandomPort}) Expect(err).ToNot(HaveOccurred()) diff --git a/tests/integration/controller/kyma/helper_test.go b/tests/integration/controller/kyma/helper_test.go index 3606ddb2cf..f2ed775f37 100644 --- a/tests/integration/controller/kyma/helper_test.go +++ b/tests/integration/controller/kyma/helper_test.go @@ -86,7 +86,7 @@ func DeployModuleTemplates(ctx context.Context, kcpClient client.Client, kyma *v Should(Succeed()) managedModule := NewTestModuleWithFixName(module.Name, module.Channel, "") Eventually(ModuleTemplateExists, Timeout, Interval). - WithArguments(ctx, kcpClient, managedModule, module.Channel, ControlPlaneNamespace). + WithArguments(ctx, kcpClient, managedModule, kyma). Should(Succeed()) } } diff --git a/tests/integration/controller/kyma/kyma_test.go b/tests/integration/controller/kyma/kyma_test.go index 35fc20c719..ed728b0c8b 100644 --- a/tests/integration/controller/kyma/kyma_test.go +++ b/tests/integration/controller/kyma/kyma_test.go @@ -162,8 +162,7 @@ var _ = Describe("Kyma enable one Module", Ordered, func() { if len(modulesStatus) != 1 { return ErrWrongModulesStatus } - template, err := GetModuleTemplate(ctx, kcpClient, module, v1beta2.DefaultChannel, - ControlPlaneNamespace) + template, err := GetModuleTemplate(ctx, kcpClient, module, createdKyma) if err != nil { return err } @@ -518,7 +517,7 @@ func updateKCPModuleTemplateSpecData(kymaName, valueUpdated string) func() error } for _, activeModule := range createdKyma.Spec.Modules { return UpdateModuleTemplateSpec(ctx, kcpClient, - activeModule, InitSpecKey, valueUpdated, createdKyma.Spec.Channel, ControlPlaneNamespace) + activeModule, InitSpecKey, valueUpdated, createdKyma) } return nil } diff --git a/tests/integration/controller/kyma/manifest_test.go b/tests/integration/controller/kyma/manifest_test.go index d6e12d1799..0f89f87e43 100644 --- a/tests/integration/controller/kyma/manifest_test.go +++ b/tests/integration/controller/kyma/manifest_test.go @@ -127,7 +127,7 @@ var _ = Describe("Manifest.Spec is rendered correctly", Ordered, func() { RegisterDefaultLifecycleForKyma(kyma) It("validate Manifest", func() { - moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kyma.Spec.Channel, ControlPlaneNamespace) + moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kyma) Expect(err).NotTo(HaveOccurred()) expectManifest := expectManifestFor(kyma) @@ -197,7 +197,7 @@ var _ = Describe("Manifest.Spec is reset after manual update", Ordered, func() { }) It("validate Manifest", func() { - moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kyma.Spec.Channel, ControlPlaneNamespace) + moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kyma) Expect(err).NotTo(HaveOccurred()) expectManifest := expectManifestFor(kyma) @@ -266,8 +266,8 @@ var _ = Describe("Update Module Template Version", Ordered, func() { { newVersionAndLayerDigest := updateModuleTemplateVersion updatedVersionAndLayerDigest := validateModuleTemplateVersionUpdated - updateModuleTemplateWith := funWrap(updateKCPModuleTemplate(module, kyma.Spec.Channel)) - validateModuleTemplateWith := funWrap(validateKCPModuleTemplate(module, kyma.Spec.Channel)) + updateModuleTemplateWith := funWrap(updateKCPModuleTemplate(module, kyma)) + validateModuleTemplateWith := funWrap(validateKCPModuleTemplate(module, kyma)) updateModuleTemplateVersionAndLayerDigest := updateModuleTemplateWith(newVersionAndLayerDigest) validateVersionAndLayerDigestAreUpdated := validateModuleTemplateWith(updatedVersionAndLayerDigest) @@ -333,11 +333,10 @@ var _ = Describe("Modules can only be referenced via module name", Ordered, func It("returns the expected operator", func() { Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(kcpClient, moduleReferencedWithLabel, v1beta2.DefaultChannel, ControlPlaneNamespace). + WithArguments(kcpClient, moduleReferencedWithLabel, kyma). Should(Succeed()) - moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, moduleReferencedWithLabel, v1beta2.DefaultChannel, - ControlPlaneNamespace) + moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, moduleReferencedWithLabel, kyma) Expect(err).ToNot(HaveOccurred()) foundModuleName := moduleTemplate.Labels[shared.ModuleName] Expect(foundModuleName).To(Equal(moduleReferencedWithLabel.Name)) @@ -348,8 +347,7 @@ var _ = Describe("Modules can only be referenced via module name", Ordered, func It("cannot find the operator", func() { Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(kcpClient, moduleReferencedWithNamespacedName, v1beta2.DefaultChannel, - ControlPlaneNamespace). + WithArguments(kcpClient, moduleReferencedWithNamespacedName, kyma). Should(Equal(ErrNotFound)) }) }) @@ -358,7 +356,7 @@ var _ = Describe("Modules can only be referenced via module name", Ordered, func It("cannot find the operator", func() { Eventually(ModuleTemplateExists). WithContext(ctx). - WithArguments(kcpClient, moduleReferencedWithFQDN, v1beta2.DefaultChannel, ControlPlaneNamespace). + WithArguments(kcpClient, moduleReferencedWithFQDN, kyma). Should(Equal(ErrNotFound)) }) }) @@ -506,9 +504,9 @@ func validateManifestSpecResource(manifestResource, moduleTemplateData *unstruct } // getKCPModuleTemplate is a generic ModuleTemplate validation function. -func validateKCPModuleTemplate(module v1beta2.Module, kymaChannel string) func(moduleTemplateFn) error { +func validateKCPModuleTemplate(module v1beta2.Module, kyma *v1beta2.Kyma) func(moduleTemplateFn) error { return func(validateFunc moduleTemplateFn) error { - moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kymaChannel, ControlPlaneNamespace) + moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kyma) if err != nil { return err } @@ -523,9 +521,9 @@ func validateKCPModuleTemplate(module v1beta2.Module, kymaChannel string) func(m } // updateKCPModuleTemplate is a generic ModuleTemplate update function. -func updateKCPModuleTemplate(module v1beta2.Module, kymaChannel string) func(moduleTemplateFn) error { +func updateKCPModuleTemplate(module v1beta2.Module, kyma *v1beta2.Kyma) func(moduleTemplateFn) error { return func(updateFunc moduleTemplateFn) error { - moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kymaChannel, ControlPlaneNamespace) + moduleTemplate, err := GetModuleTemplate(ctx, kcpClient, module, kyma) if err != nil { return err } diff --git a/tests/integration/controller/kyma/suite_test.go b/tests/integration/controller/kyma/suite_test.go index bd780f071f..8ce38b9a6a 100644 --- a/tests/integration/controller/kyma/suite_test.go +++ b/tests/integration/controller/kyma/suite_test.go @@ -41,13 +41,13 @@ import ( "github.com/kyma-project/lifecycle-manager/internal/controller/kyma" "github.com/kyma-project/lifecycle-manager/internal/descriptor/provider" "github.com/kyma-project/lifecycle-manager/internal/event" - "github.com/kyma-project/lifecycle-manager/internal/maintenancewindows" "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/templatelookup" + "github.com/kyma-project/lifecycle-manager/pkg/templatelookup/moduletemplateinfolookup" "github.com/kyma-project/lifecycle-manager/tests/integration" testskrcontext "github.com/kyma-project/lifecycle-manager/tests/integration/commontestutils/skrcontextimpl" @@ -137,7 +137,6 @@ var _ = BeforeSuite(func() { kcpClient = mgr.GetClient() testEventRec := event.NewRecorderWrapper(mgr.GetEventRecorderFor(shared.OperatorName)) testSkrContextFactory := testskrcontext.NewSingleClusterFactory(kcpClient, mgr.GetConfig(), testEventRec) - maintenanceWindow, _ := maintenancewindows.InitializeMaintenanceWindow(logr, "/not-required", "/not-required") err = (&kyma.Reconciler{ Client: kcpClient, Event: testEventRec, @@ -148,7 +147,11 @@ var _ = BeforeSuite(func() { InKCPMode: false, RemoteSyncNamespace: flags.DefaultRemoteSyncNamespace, Metrics: metrics.NewKymaMetrics(metrics.NewSharedMetrics()), - TemplateLookup: templatelookup.NewTemplateLookup(kcpClient, descriptorProvider, maintenanceWindow), + TemplateLookup: templatelookup.NewTemplateLookup(kcpClient, descriptorProvider, moduletemplateinfolookup.NewModuleTemplateInfoLookupStrategies([]moduletemplateinfolookup.ModuleTemplateInfoLookupStrategy{ + moduletemplateinfolookup.NewByVersionStrategy(kcpClient), + moduletemplateinfolookup.NewByChannelStrategy(kcpClient), + moduletemplateinfolookup.NewByModuleReleaseMetaStrategy(kcpClient), + })), }).SetupWithManager(mgr, ctrlruntime.Options{ RateLimiter: internal.RateLimiter( 1*time.Second, 5*time.Second, diff --git a/unit-test-coverage.yaml b/unit-test-coverage.yaml index 1e1b9a09f3..4bfc67663c 100644 --- a/unit-test-coverage.yaml +++ b/unit-test-coverage.yaml @@ -19,4 +19,5 @@ packages: internal/pkg/resources: 91.7 internal/remote: 20.2 internal/util/collections: 86 - pkg/templatelookup: 83.3 + pkg/templatelookup: 88 + pkg/templatelookup/moduletemplateinfolookup: 72