Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

chore: Refactor available module #2120

Merged
merged 44 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
13190c4
refactor available module
medmes Dec 13, 2024
42fbe5d
feat: Prevent unallowed internal/beta module installation (#2111)
medmes Dec 13, 2024
f2ef866
feat: Prevent unallowed internal/beta module installation (#2111)
medmes Dec 13, 2024
701bdc3
add empty spec and status unit test
medmes Dec 13, 2024
dd5893d
rename old function into FetchModuleStatusInfo
medmes Dec 13, 2024
ab2cae9
remove import line
medmes Dec 13, 2024
70f76cd
remove aut new line - gci complain
medmes Dec 13, 2024
087f2d9
Merge branch 'main' into refactor_available_module
medmes Dec 15, 2024
a3108eb
remove import line
medmes Dec 15, 2024
226bbb2
revert go.mod
medmes Dec 16, 2024
6fca9e8
revert go.mod
medmes Dec 16, 2024
fb1bf87
Merge branch 'main' into refactor_available_module
medmes Dec 16, 2024
dd8d7a8
rename all into "ModuleInfo" instead
medmes Dec 16, 2024
46d1ab5
added method comment
medmes Dec 16, 2024
7169967
refactor available module
medmes Dec 13, 2024
b9574fa
feat: Prevent unallowed internal/beta module installation (#2111)
medmes Dec 13, 2024
f733532
feat: Prevent unallowed internal/beta module installation (#2111)
medmes Dec 13, 2024
fd52c9d
add empty spec and status unit test
medmes Dec 13, 2024
7f5bb30
rename old function into FetchModuleStatusInfo
medmes Dec 13, 2024
ce5dd91
remove import line
medmes Dec 13, 2024
cb5adf7
remove aut new line - gci complain
medmes Dec 13, 2024
3a0bc8a
remove import line
medmes Dec 15, 2024
79e8a07
revert go.mod
medmes Dec 16, 2024
6620d37
revert go.mod
medmes Dec 16, 2024
a938248
rename all into "ModuleInfo" instead
medmes Dec 16, 2024
868a81b
added method comment
medmes Dec 16, 2024
4adb34f
Merge branch 'refactor_available_module' of github.com:medmes/lifecyc…
medmes Dec 17, 2024
85e41a7
Merge branch 'main' of github.com:kyma-project/lifecycle-manager into…
medmes Dec 17, 2024
cf12cb6
Merge branch 'main' into refactor_available_module
medmes Dec 18, 2024
70c1d6b
Refactor code and algorithm optimization to O(n+m)
medmes Dec 18, 2024
8340c19
Merge branch 'main' into refactor_available_module
medmes Dec 18, 2024
7042d7f
revert back to Enabled and Managed
medmes Dec 18, 2024
092ff82
Revert "Refactor code and algorithm optimization to O(n+m)"
medmes Dec 18, 2024
ec45b25
Merge branch 'main' into refactor_available_module
medmes Dec 18, 2024
0c00db5
delete unused code
medmes Dec 18, 2024
13f68de
revert to old state
medmes Dec 18, 2024
30507dd
revert ide new line added
medmes Dec 18, 2024
812c99e
revert ide new line added
medmes Dec 18, 2024
fc483f6
delete unused code
medmes Dec 18, 2024
621e59b
Update moduleinfo_test.go
medmes Dec 18, 2024
31dbec5
Revert "Update moduleinfo_test.go"
medmes Dec 18, 2024
1510aab
revert ide new line added
medmes Dec 18, 2024
46faf45
Apply suggestions from code review
medmes Dec 19, 2024
f05472e
add comment to fetchModuleInfo function
medmes Dec 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions internal/manifest/parser/template_to_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (p *Parser) GenerateModulesFromTemplates(kyma *v1beta2.Kyma, templates temp
// (since we do not know which module we are dealing with)
modules := make(common.Modules, 0)

for _, module := range templatelookup.FindAvailableModules(kyma) {
for _, module := range templatelookup.FetchModuleInfo(kyma) {
template := templates[module.Name]
modules = p.appendModuleWithInformation(module, kyma, template, modules)
}
Expand All @@ -67,7 +67,7 @@ func (p *Parser) GenerateMandatoryModulesFromTemplates(ctx context.Context,
moduleName = template.Name
}

modules = p.appendModuleWithInformation(templatelookup.AvailableModule{
modules = p.appendModuleWithInformation(templatelookup.ModuleInfo{
Module: v1beta2.Module{
Name: moduleName,
CustomResourcePolicy: v1beta2.CustomResourcePolicyCreateAndDelete,
Expand All @@ -79,7 +79,7 @@ func (p *Parser) GenerateMandatoryModulesFromTemplates(ctx context.Context,
return modules
}

func (p *Parser) appendModuleWithInformation(module templatelookup.AvailableModule, kyma *v1beta2.Kyma,
func (p *Parser) appendModuleWithInformation(module templatelookup.ModuleInfo, kyma *v1beta2.Kyma,
template *templatelookup.ModuleTemplateInfo, modules common.Modules,
) common.Modules {
if template.Err != nil && !errors.Is(template.Err, templatelookup.ErrTemplateNotAllowed) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,36 @@ var (
ErrInvalidModuleInStatus = errors.New("invalid module entry in Kyma status")
)

type AvailableModule struct {
type ModuleInfo struct {
v1beta2.Module
Enabled bool
ValidationError error
Unmanaged bool
}

func (a AvailableModule) IsInstalledByVersion() bool {
func (a ModuleInfo) IsInstalledByVersion() bool {
return a.configuredWithVersionInSpec() || a.installedwithVersionInStatus()
}

// configuredWithVersionInSpec returns true if the Module is enabled in Spec using a specific version instead of a channel.
func (a AvailableModule) configuredWithVersionInSpec() bool {
func (a ModuleInfo) configuredWithVersionInSpec() bool {
return a.Enabled && a.Version != "" && a.Channel == ""
}

// installedwithVersionInStatus returns true if the Module installed using a specific version (instead of a channel) is reported in Status.
func (a AvailableModule) installedwithVersionInStatus() bool {
func (a ModuleInfo) installedwithVersionInStatus() bool {
return !a.Enabled && shared.NoneChannel.Equals(a.Channel) && a.Version != ""
}

// FindAvailableModules returns a list of AvailableModule objects based on the Kyma CR Spec and Status.
func FindAvailableModules(kyma *v1beta2.Kyma) []AvailableModule {
// FetchModuleInfo returns a list of ModuleInfo objects containing information about modules referenced by the Kyma CR.
// This includes modules that are enabled in `.spec.modules[]` and modules that are not enabled in `.spec.modules[]` but still contain an entry in `.status.modules[]`.
func FetchModuleInfo(kyma *v1beta2.Kyma) []ModuleInfo {
moduleMap := make(map[string]bool)
modules := make([]AvailableModule, 0)
modules := make([]ModuleInfo, 0)
for _, module := range kyma.Spec.Modules {
moduleMap[module.Name] = true
if shared.NoneChannel.Equals(module.Channel) {
modules = append(modules, AvailableModule{
modules = append(modules, ModuleInfo{
Module: module,
Enabled: true,
ValidationError: fmt.Errorf("%w for module %s: Channel \"none\" is not allowed", ErrInvalidModuleInSpec, module.Name),
Expand All @@ -50,15 +51,15 @@ func FindAvailableModules(kyma *v1beta2.Kyma) []AvailableModule {
continue
}
if module.Version != "" && module.Channel != "" {
modules = append(modules, AvailableModule{
modules = append(modules, ModuleInfo{
Module: module,
Enabled: true,
ValidationError: fmt.Errorf("%w for module %s: Version and channel are mutually exclusive options", ErrInvalidModuleInSpec, module.Name),
Unmanaged: !module.Managed,
})
continue
}
modules = append(modules, AvailableModule{Module: module, Enabled: true, Unmanaged: !module.Managed})
modules = append(modules, ModuleInfo{Module: module, Enabled: true, Unmanaged: !module.Managed})
}

for _, moduleInStatus := range kyma.Status.Modules {
Expand All @@ -67,7 +68,7 @@ func FindAvailableModules(kyma *v1beta2.Kyma) []AvailableModule {
continue
}

modules = append(modules, AvailableModule{
modules = append(modules, ModuleInfo{
Module: v1beta2.Module{
Name: moduleInStatus.Name,
Channel: moduleInStatus.Channel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,46 @@ import (
"github.com/kyma-project/lifecycle-manager/pkg/templatelookup"
)

func Test_GetAvailableModules_When_ModuleInSpecOnly(t *testing.T) {
func Test_FetchModuleInfo_When_EmptySpecAndStatus(t *testing.T) {
tests := []struct {
name string
KymaSpec v1beta2.KymaSpec
KymaStatus v1beta2.KymaStatus
want []moduleInfo
}{
{
name: "When KymaSpec and KymaStatus are both empty, the output should be empty",
KymaSpec: v1beta2.KymaSpec{},
KymaStatus: v1beta2.KymaStatus{},
want: []moduleInfo{}, // Expect empty result
},
}
for ti := range tests {
testcase := tests[ti]
t.Run(testcase.name, func(t *testing.T) {
kyma := &v1beta2.Kyma{
Spec: testcase.KymaSpec,
Status: testcase.KymaStatus,
}

got := templatelookup.FetchModuleInfo(kyma)
if len(got) != len(testcase.want) {
t.Errorf("FetchModuleInfo() = %v, want %v", got, testcase.want)
}
for gi := range got {
if !testcase.want[gi].Equals(got[gi]) {
t.Errorf("FetchModuleInfo() = %v, want %v", got, testcase.want)
}
}
})
}
}

func Test_FetchModuleInfo_When_ModuleInSpecOnly(t *testing.T) {
tests := []struct {
name string
KymaSpec v1beta2.KymaSpec
want []availableModuleDescription
want []moduleInfo
}{
{
name: "When Channel \"none\" is used, then the module is invalid",
Expand All @@ -24,7 +59,7 @@ func Test_GetAvailableModules_When_ModuleInSpecOnly(t *testing.T) {
{Name: "Module1", Channel: "none"},
},
},
want: []availableModuleDescription{
want: []moduleInfo{
{
Module: v1beta2.Module{Name: "Module1", Channel: "none"},
Enabled: true,
Expand All @@ -39,7 +74,7 @@ func Test_GetAvailableModules_When_ModuleInSpecOnly(t *testing.T) {
{Name: "Module1", Channel: "regular", Version: "v1.0"},
},
},
want: []availableModuleDescription{
want: []moduleInfo{
{
Module: v1beta2.Module{Name: "Module1", Channel: "regular", Version: "v1.0"},
Enabled: true,
Expand All @@ -54,7 +89,7 @@ func Test_GetAvailableModules_When_ModuleInSpecOnly(t *testing.T) {
{Name: "Module1", Channel: "regular"},
},
},
want: []availableModuleDescription{
want: []moduleInfo{
{Module: v1beta2.Module{Name: "Module1", Channel: "regular"}, Enabled: true},
},
},
Expand All @@ -65,7 +100,7 @@ func Test_GetAvailableModules_When_ModuleInSpecOnly(t *testing.T) {
{Name: "Module1", Version: "v1.0"},
},
},
want: []availableModuleDescription{
want: []moduleInfo{
{Module: v1beta2.Module{Name: "Module1", Version: "v1.0"}, Enabled: true},
},
},
Expand All @@ -77,24 +112,24 @@ func Test_GetAvailableModules_When_ModuleInSpecOnly(t *testing.T) {
Spec: testcase.KymaSpec,
}

got := templatelookup.FindAvailableModules(kyma)
got := templatelookup.FetchModuleInfo(kyma)
if len(got) != len(testcase.want) {
t.Errorf("GetAvailableModules() = %v, want %v", got, testcase.want)
t.Errorf("FetchModuleInfo() = %v, want %v", got, testcase.want)
}
for gi := range got {
if !testcase.want[gi].Equals(got[gi]) {
t.Errorf("GetAvailableModules() = %v, want %v", got, testcase.want)
t.Errorf("FetchModuleInfo() = %v, want %v", got, testcase.want)
}
}
})
}
}

func Test_GetAvailableModules_When_ModuleInStatusOnly(t *testing.T) {
func Test_FetchModuleInfo_When_ModuleInStatusOnly(t *testing.T) {
tests := []struct {
name string
KymaStatus v1beta2.KymaStatus
want []availableModuleDescription
want []moduleInfo
}{
{
name: "When Template exists, then the module is valid",
Expand All @@ -108,7 +143,7 @@ func Test_GetAvailableModules_When_ModuleInStatusOnly(t *testing.T) {
},
},
},
want: []availableModuleDescription{
want: []moduleInfo{
{Module: v1beta2.Module{Name: "Module1", Channel: "regular", Version: "v1.0"}, Enabled: false},
},
},
Expand All @@ -124,7 +159,7 @@ func Test_GetAvailableModules_When_ModuleInStatusOnly(t *testing.T) {
},
},
},
want: []availableModuleDescription{
want: []moduleInfo{
{
Module: v1beta2.Module{Name: "Module1", Channel: "regular", Version: "v1.0"},
Enabled: false,
Expand All @@ -140,25 +175,25 @@ func Test_GetAvailableModules_When_ModuleInStatusOnly(t *testing.T) {
Status: testcase.KymaStatus,
}

got := templatelookup.FindAvailableModules(kyma)
got := templatelookup.FetchModuleInfo(kyma)
if len(got) != len(testcase.want) {
t.Errorf("GetAvailableModules() = %v, want %v", got, testcase.want)
t.Errorf("FetchModuleInfo() = %v, want %v", got, testcase.want)
}
for gi := range got {
if !testcase.want[gi].Equals(got[gi]) {
t.Errorf("GetAvailableModules() = %v, want %v", got, testcase.want)
t.Errorf("FetchModuleInfo() = %v, want %v", got, testcase.want)
}
}
})
}
}

func Test_GetAvailableModules_When_ModuleExistsInSpecAndStatus(t *testing.T) {
func Test_FetchModuleInfo_When_ModuleExistsInSpecAndStatus(t *testing.T) {
tests := []struct {
name string
KymaSpec v1beta2.KymaSpec
KymaStatus v1beta2.KymaStatus
want []availableModuleDescription
want []moduleInfo
}{
{
name: "When Module have different version between Spec and Status, the output should be based on Spec",
Expand All @@ -175,7 +210,7 @@ func Test_GetAvailableModules_When_ModuleExistsInSpecAndStatus(t *testing.T) {
},
},
},
want: []availableModuleDescription{
want: []moduleInfo{
{Module: v1beta2.Module{Name: "Module1", Version: "v1.1"}, Enabled: true},
},
},
Expand All @@ -187,50 +222,50 @@ func Test_GetAvailableModules_When_ModuleExistsInSpecAndStatus(t *testing.T) {
Spec: testcase.KymaSpec,
Status: testcase.KymaStatus,
}
got := templatelookup.FindAvailableModules(kyma)
got := templatelookup.FetchModuleInfo(kyma)
if len(got) != len(testcase.want) {
t.Errorf("GetAvailableModules() = %v, want %v", got, testcase.want)
t.Errorf("FetchModuleInfo() = %v, want %v", got, testcase.want)
}
for gi := range got {
if !testcase.want[gi].Equals(got[gi]) {
t.Errorf("GetAvailableModules() = %v, want %v", got, testcase.want)
t.Errorf("FetchModuleInfo() = %v, want %v", got, testcase.want)
}
}
})
}
}

type availableModuleDescription struct {
type moduleInfo struct {
Module v1beta2.Module
Enabled bool
ValidationErrorContains string
ExpectedError error
}
medmes marked this conversation as resolved.
Show resolved Hide resolved

func (amd availableModuleDescription) Equals(other templatelookup.AvailableModule) bool {
if amd.Module.Name != other.Name {
func (m moduleInfo) Equals(other templatelookup.ModuleInfo) bool {
if m.Module.Name != other.Name {
return false
}
if amd.Module.Channel != other.Channel {
if m.Module.Channel != other.Channel {
return false
}
if amd.Module.Version != other.Version {
if m.Module.Version != other.Version {
return false
}
if amd.Enabled != other.Enabled {
if m.Enabled != other.Enabled {
return false
}
if amd.ExpectedError != nil && other.ValidationError == nil {
if m.ExpectedError != nil && other.ValidationError == nil {
return false
}
if amd.ExpectedError == nil && other.ValidationError != nil {
if m.ExpectedError == nil && other.ValidationError != nil {
return false
}
if amd.ExpectedError != nil && other.ValidationError != nil {
if !errors.Is(other.ValidationError, amd.ExpectedError) {
if m.ExpectedError != nil && other.ValidationError != nil {
if !errors.Is(other.ValidationError, m.ExpectedError) {
return false
}
if !strings.Contains(other.ValidationError.Error(), amd.ValidationErrorContains) {
if !strings.Contains(other.ValidationError.Error(), m.ValidationErrorContains) {
return false
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/templatelookup/regular.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type ModuleTemplatesByModuleName map[string]*ModuleTemplateInfo

func (t *TemplateLookup) GetRegularTemplates(ctx context.Context, kyma *v1beta2.Kyma) ModuleTemplatesByModuleName {
templates := make(ModuleTemplatesByModuleName)
for _, module := range FindAvailableModules(kyma) {
for _, module := range FetchModuleInfo(kyma) {
_, found := templates[module.Name]
if found {
continue
Expand Down Expand Up @@ -92,7 +92,7 @@ func (t *TemplateLookup) GetRegularTemplates(ctx context.Context, kyma *v1beta2.
}

func (t *TemplateLookup) PopulateModuleTemplateInfo(ctx context.Context,
module AvailableModule, namespace, kymaChannel string, moduleReleaseMeta *v1beta2.ModuleReleaseMeta,
module ModuleInfo, namespace, kymaChannel string, moduleReleaseMeta *v1beta2.ModuleReleaseMeta,
) ModuleTemplateInfo {
if moduleReleaseMeta == nil {
return t.populateModuleTemplateInfoWithoutModuleReleaseMeta(ctx, module, kymaChannel)
Expand All @@ -102,7 +102,7 @@ func (t *TemplateLookup) PopulateModuleTemplateInfo(ctx context.Context,
}

func (t *TemplateLookup) populateModuleTemplateInfoWithoutModuleReleaseMeta(ctx context.Context,
module AvailableModule, kymaChannel string,
module ModuleInfo, kymaChannel string,
) ModuleTemplateInfo {
var templateInfo ModuleTemplateInfo
if module.IsInstalledByVersion() {
Expand All @@ -114,7 +114,7 @@ func (t *TemplateLookup) populateModuleTemplateInfoWithoutModuleReleaseMeta(ctx
}

func (t *TemplateLookup) populateModuleTemplateInfoUsingModuleReleaseMeta(ctx context.Context,
module AvailableModule,
module ModuleInfo,
moduleReleaseMeta *v1beta2.ModuleReleaseMeta, kymaChannel, namespace string,
) ModuleTemplateInfo {
var templateInfo ModuleTemplateInfo
Expand Down
4 changes: 2 additions & 2 deletions pkg/templatelookup/regular_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ func TestNewTemplateLookup_GetRegularTemplates_WhenModuleTemplateContainsInvalid
t.Run(testCase.name, func(t *testing.T) {
givenTemplateList := &v1beta2.ModuleTemplateList{}
moduleReleaseMetas := v1beta2.ModuleReleaseMetaList{}
for _, module := range templatelookup.FindAvailableModules(testCase.kyma) {
for _, module := range templatelookup.FetchModuleInfo(testCase.kyma) {
givenTemplateList.Items = append(givenTemplateList.Items, *builder.NewModuleTemplateBuilder().
WithName(fmt.Sprintf("%s-%s", module.Name, testModule.Version)).
WithModuleName(module.Name).
Expand Down Expand Up @@ -1010,7 +1010,7 @@ func TestTemplateLookup_GetRegularTemplates_WhenModuleTemplateExists(t *testing.
t.Run(testCase.name, func(t *testing.T) {
givenTemplateList := &v1beta2.ModuleTemplateList{}
moduleReleaseMetas := v1beta2.ModuleReleaseMetaList{}
for _, module := range templatelookup.FindAvailableModules(testCase.kyma) {
for _, module := range templatelookup.FetchModuleInfo(testCase.kyma) {
if testCase.mrmExist {
givenTemplateList.Items = append(givenTemplateList.Items, *builder.NewModuleTemplateBuilder().
WithName(fmt.Sprintf("%s-%s", module.Name, testModule.Version)).
Expand Down
2 changes: 1 addition & 1 deletion pkg/testutils/moduletemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func GetModuleTemplate(ctx context.Context,
) (*v1beta2.ModuleTemplate, error) {
descriptorProvider := provider.NewCachedDescriptorProvider()
templateLookup := templatelookup.NewTemplateLookup(clnt, descriptorProvider)
availableModule := templatelookup.AvailableModule{
availableModule := templatelookup.ModuleInfo{
Module: module,
}

Expand Down
Loading