feat: Prevent unallowed internal/beta module installation (kyma-proje…

…ct#2111) * feat: Prevent unallowed internal/beta module installation * fix linting * tests to allow ireturn * refactor mrm fetching * fix tests * revert receiver to normal func * remove unnecessary context * add missing testcases # Conflicts: # pkg/templatelookup/regular.go
medmes · Dec 13, 2024 · 4ef9059 · 4ef9059
1 parent ce71842
commit 4ef9059
Show file tree

Hide file tree

Showing 13 changed files with 707 additions and 35 deletions.
diff --git a/.github/actions/deploy-template-operator-with-modulereleasemeta/action.yml b/.github/actions/deploy-template-operator-with-modulereleasemeta/action.yml
@@ -64,7 +64,8 @@ runs:
         matrix.e2e-test == 'modulereleasemeta-module-upgrade-new-version' ||
         matrix.e2e-test == 'modulereleasemeta-upgrade-under-deletion' ||
         matrix.e2e-test == 'modulereleasemeta-sync' ||
-        matrix.e2e-test == 'module-status-on-skr-connection-lost'
+        matrix.e2e-test == 'module-status-on-skr-connection-lost' ||
+        matrix.e2e-test == 'modulereleasemeta-not-allowed-installation'
         }}
       shell: bash
       run: |

diff --git a/.github/workflows/test-e2e-with-modulereleasemeta.yml b/.github/workflows/test-e2e-with-modulereleasemeta.yml
@@ -65,6 +65,7 @@ jobs:
           - modulereleasemeta-sync
           - module-status-on-skr-connection-lost
           - modulereleasemeta-watch-trigger
+          - modulereleasemeta-not-allowed-installation
 
     runs-on: ubuntu-latest
     timeout-minutes: 20

diff --git a/.vscode/tasks.json b/.vscode/tasks.json
@@ -95,7 +95,8 @@
         "mandatory-module-metrics",
         "misconfigured-kyma-secret",
         "ocm-compatible-module-template",
-        "modulereleasemeta-sync"
+        "modulereleasemeta-sync",
+        "modulereleasemeta-not-allowed-installation",
       ]
     },
     {

diff --git a/internal/remote/remote_catalog.go b/internal/remote/remote_catalog.go
@@ -144,21 +144,30 @@ func (c *RemoteCatalog) GetModuleReleaseMetasToSync(
 
 	moduleReleaseMetas := []v1beta2.ModuleReleaseMeta{}
 	for _, moduleReleaseMeta := range moduleReleaseMetaList.Items {
-		if moduleReleaseMeta.IsBeta() && !kyma.IsBeta() {
-			continue
-		}
-		if moduleReleaseMeta.IsInternal() && !kyma.IsInternal() {
-			continue
+		if IsAllowedModuleReleaseMeta(moduleReleaseMeta, kyma) {
+			moduleReleaseMetas = append(moduleReleaseMetas, moduleReleaseMeta)
 		}
-		moduleReleaseMetas = append(moduleReleaseMetas, moduleReleaseMeta)
 	}
 
 	return moduleReleaseMetas, nil
 }
 
+// IsAllowedModuleReleaseMeta determines whether the given ModuleReleaseMeta is allowed for the given Kyma.
+// If the ModuleReleaseMeta is Beta, it is allowed only if the Kyma is also Beta.
+// If the ModuleReleaseMeta is Internal, it is allowed only if the Kyma is also Internal.
+func IsAllowedModuleReleaseMeta(moduleReleaseMeta v1beta2.ModuleReleaseMeta, kyma *v1beta2.Kyma) bool {
+	if moduleReleaseMeta.IsBeta() && !kyma.IsBeta() {
+		return false
+	}
+	if moduleReleaseMeta.IsInternal() && !kyma.IsInternal() {
+		return false
+	}
+	return true
+}
+
 // GetModuleTemplatesToSync returns a list of ModuleTemplates that should be synced to the SKR.
-// A ModuleTemplate is synced if it is not mandatory and does not have sync disabled. In addition,
-// it must be referenced by a ModuleReleaseMeta that is synced.
+// A ModuleTemplate is synced if it is not mandatory and does not have sync disabled, and if
+// it is referenced by a ModuleReleaseMeta that is synced.
 func (c *RemoteCatalog) GetModuleTemplatesToSync(
 	ctx context.Context,
 	moduleReleaseMetas []v1beta2.ModuleReleaseMeta,
@@ -168,10 +177,13 @@ func (c *RemoteCatalog) GetModuleTemplatesToSync(
 		return nil, fmt.Errorf("failed to list ModuleTemplates: %w", err)
 	}
 
-	return c.FilterModuleTemplatesToSync(moduleTemplateList.Items, moduleReleaseMetas), nil
+	return FilterAllowedModuleTemplates(moduleTemplateList.Items, moduleReleaseMetas), nil
 }
 
-func (c *RemoteCatalog) FilterModuleTemplatesToSync(
+// FilterAllowedModuleTemplates filters out ModuleTemplates that are not allowed.
+// A ModuleTemplate is allowed if it is not mandatory, does not have sync disabled, and if
+// it is referenced by a ModuleReleaseMeta that is synced.
+func FilterAllowedModuleTemplates(
 	moduleTemplates []v1beta2.ModuleTemplate,
 	moduleReleaseMetas []v1beta2.ModuleReleaseMeta,
 ) []v1beta2.ModuleTemplate {

diff --git a/internal/remote/remote_catalog_test.go b/internal/remote/remote_catalog_test.go
@@ -105,10 +105,8 @@ func Test_GetModuleTemplatesToSync_ReturnsMTsThatAreReferencedInMRMAndNotMandato
 	assert.Equal(t, "regular-module-2.0.0", mts[1].ObjectMeta.Name)
 }
 
-func Test_FilterModuleTemplatesToSync_ReturnsMTsThatAreReferencedInMRMAndNotMandatoryNotSyncDisabled(t *testing.T) {
-	remoteCatalog := remote.NewRemoteCatalogFromKyma(fakeClient(), nil, "kyma-system")
-
-	mts := remoteCatalog.FilterModuleTemplatesToSync(moduleTemplates().Items, []v1beta2.ModuleReleaseMeta{
+func Test_FilterAllowedModuleTemplates_ReturnsMTsThatAreReferencedInMRMAndNotMandatoryNotSyncDisabled(t *testing.T) {
+	mts := remote.FilterAllowedModuleTemplates(moduleTemplates().Items, []v1beta2.ModuleReleaseMeta{
 		*newModuleReleaseMetaBuilder().
 			withName("regular-module").
 			withChannelVersion("regular", "1.0.0").
@@ -181,6 +179,163 @@ func Test_GetOldModuleTemplatesToSync_ReturnsBetaInternalNonSyncDisabledNonManda
 	assert.Equal(t, "old-module-regular", mts[3].ObjectMeta.Name)
 }
 
+func Test_IsAllowedModuleReleaseMeta_ReturnsTrue_ForNonBetaNonInternalMRMAndNonBetaNonInternalKyma(t *testing.T) {
+	mrm := newModuleReleaseMetaBuilder().build()
+	kyma := newKymaBuilder().build()
+
+	assert.True(t, remote.IsAllowedModuleReleaseMeta(*mrm, kyma))
+}
+
+func Test_IsAllowedModuleReleaseMeta(t *testing.T) {
+	testCases := []struct {
+		name           string
+		moduleBeta     bool
+		moduleInternal bool
+		kymaBeta       bool
+		kymaInternal   bool
+		expected       bool
+	}{
+		{
+			name:           "Given Module{Beta: false, Internal: false}; Kyma{Beta: false, Internal: false}; Expect Installation: true",
+			moduleBeta:     false,
+			moduleInternal: false,
+			kymaBeta:       false,
+			kymaInternal:   false,
+			expected:       true,
+		},
+		{
+			name:           "Given Module{Beta: true, Internal: false}; Kyma{Beta: false, Internal: false}; Expect Installation:  false",
+			moduleBeta:     true,
+			moduleInternal: false,
+			kymaBeta:       false,
+			kymaInternal:   false,
+			expected:       false,
+		},
+		{
+			name:           "Given Module{Beta: false, Internal: true}; Kyma{Beta: false, Internal: false}; Expect Installation:  false",
+			moduleBeta:     false,
+			moduleInternal: true,
+			kymaBeta:       false,
+			kymaInternal:   false,
+			expected:       false,
+		},
+		{
+			name:           "Given Module{Beta: false, Internal: false}; Kyma{Beta: true, Internal: false}; Expect Installation:  true",
+			moduleBeta:     false,
+			moduleInternal: false,
+			kymaBeta:       true,
+			kymaInternal:   false,
+			expected:       true,
+		},
+		{
+			name:           "Given Module{Beta: false, Internal: false}; Kyma{Beta: false, Internal: true}; Expect Installation:  true",
+			moduleBeta:     false,
+			moduleInternal: false,
+			kymaBeta:       false,
+			kymaInternal:   true,
+			expected:       true,
+		},
+		{
+			name:           "Given Module{Beta: true, Internal: true}; Kyma{Beta: false, Internal: false}; Expect Installation:  false",
+			moduleBeta:     true,
+			moduleInternal: true,
+			kymaBeta:       false,
+			kymaInternal:   false,
+			expected:       false,
+		},
+		{
+			name:           "Given Module{Beta: true, Internal: false}; Kyma{Beta: true, Internal: false}; Expect Installation:  true",
+			moduleBeta:     true,
+			moduleInternal: false,
+			kymaBeta:       true,
+			kymaInternal:   false,
+			expected:       true,
+		},
+		{
+			name:           "Given Module{Beta: true, Internal: false}; Kyma{Beta: false, Internal: true}; Expect Installation:  false",
+			moduleBeta:     true,
+			moduleInternal: false,
+			kymaBeta:       false,
+			kymaInternal:   true,
+			expected:       false,
+		},
+		{
+			name:           "Given Module{Beta: true, Internal: true}; Kyma{Beta: true, Internal: false}; Expect Installation:  false",
+			moduleBeta:     true,
+			moduleInternal: true,
+			kymaBeta:       true,
+			kymaInternal:   false,
+			expected:       false,
+		},
+		{
+			name:           "Given Module{Beta: true, Internal: true}; Kyma{Beta: false, Internal: true}; Expect Installation:  false",
+			moduleBeta:     true,
+			moduleInternal: true,
+			kymaBeta:       false,
+			kymaInternal:   true,
+			expected:       false,
+		},
+		{
+			name:           "Given Module{Beta: true, Internal: true}; Kyma{Beta: true, Internal: true}; Expect Installation:  true",
+			moduleBeta:     true,
+			moduleInternal: true,
+			kymaBeta:       true,
+			kymaInternal:   true,
+			expected:       true,
+		},
+		{
+			name:           "Given Module{Beta: false, Internal: true}; Kyma{Beta: true, Internal: false}; Expect Installation:  false",
+			moduleBeta:     false,
+			moduleInternal: true,
+			kymaBeta:       true,
+			kymaInternal:   false,
+			expected:       false,
+		},
+		{
+			name:           "Given Module{Beta: false, Internal: true}; Kyma{Beta: false, Internal: true}; Expect Installation:  true",
+			moduleBeta:     false,
+			moduleInternal: true,
+			kymaBeta:       false,
+			kymaInternal:   true,
+			expected:       true,
+		},
+		{
+			name:           "Given Module{Beta: false, Internal: false}; Kyma{Beta: true, Internal: true}; Expect Installation:  true",
+			moduleBeta:     false,
+			moduleInternal: false,
+			kymaBeta:       true,
+			kymaInternal:   true,
+			expected:       true,
+		},
+		{
+			name:           "Given Module{Beta: false, Internal: true}; Kyma{Beta: true, Internal: true}; Expect Installation:  true",
+			moduleBeta:     false,
+			moduleInternal: true,
+			kymaBeta:       true,
+			kymaInternal:   true,
+			expected:       true,
+		},
+		{
+			name:           "Given Module{Beta: true, Internal: false}; Kyma{Beta: true, Internal: true}; Expect Installation:  true",
+			moduleBeta:     true,
+			moduleInternal: false,
+			kymaBeta:       true,
+			kymaInternal:   true,
+			expected:       true,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			mrm := newModuleReleaseMetaBuilder().withBeta(testCase.moduleBeta).withInternal(testCase.moduleInternal).build()
+			kyma := newKymaBuilder().withBeta(testCase.kymaBeta).withInternal(testCase.kymaInternal).build()
+
+			result := remote.IsAllowedModuleReleaseMeta(*mrm, kyma)
+			assert.Equal(t, testCase.expected, result)
+		})
+	}
+}
+
 func moduleReleaseMetas() v1beta2.ModuleReleaseMetaList {
 	mrm1 := newModuleReleaseMetaBuilder().
 		withName("regular-module").
@@ -355,11 +510,21 @@ func (b *moduleReleaseMetaBuilder) withBetaEnabled() *moduleReleaseMetaBuilder {
 	return b
 }
 
+func (b *moduleReleaseMetaBuilder) withBeta(beta bool) *moduleReleaseMetaBuilder {
+	b.moduleReleaseMeta.Spec.Beta = beta
+	return b
+}
+
 func (b *moduleReleaseMetaBuilder) withInternalEnabled() *moduleReleaseMetaBuilder {
 	b.moduleReleaseMeta.Spec.Internal = true
 	return b
 }
 
+func (b *moduleReleaseMetaBuilder) withInternal(internal bool) *moduleReleaseMetaBuilder {
+	b.moduleReleaseMeta.Spec.Internal = internal
+	return b
+}
+
 type moduleTemplateBuilder struct {
 	moduleTemplate *v1beta2.ModuleTemplate
 }
@@ -442,11 +607,29 @@ func (b *kymaBuilder) withBetaEnabled() *kymaBuilder {
 	return b
 }
 
+func (b *kymaBuilder) withBeta(beta bool) *kymaBuilder {
+	if beta {
+		b.kyma.Labels[shared.BetaLabel] = shared.EnableLabelValue
+	} else {
+		b.kyma.Labels[shared.BetaLabel] = shared.DisableLabelValue
+	}
+	return b
+}
+
 func (b *kymaBuilder) withInternalEnabled() *kymaBuilder {
 	b.kyma.Labels[shared.InternalLabel] = shared.EnableLabelValue
 	return b
 }
 
+func (b *kymaBuilder) withInternal(internal bool) *kymaBuilder {
+	if internal {
+		b.kyma.Labels[shared.InternalLabel] = shared.EnableLabelValue
+	} else {
+		b.kyma.Labels[shared.InternalLabel] = shared.DisableLabelValue
+	}
+	return b
+}
+
 type errorClient struct {
 	client.Client
 }