Skip to content

Commit

Permalink
feat: Prevent unallowed internal/beta module installation
Browse files Browse the repository at this point in the history
  • Loading branch information
c-pius committed Dec 11, 2024
1 parent f4b7091 commit dd4c423
Show file tree
Hide file tree
Showing 12 changed files with 617 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/test-e2e-with-modulereleasemeta.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@
"mandatory-module-metrics",
"misconfigured-kyma-secret",
"ocm-compatible-module-template",
"modulereleasemeta-sync"
"modulereleasemeta-sync",
"modulereleasemeta-not-allowed-installation",
]
},
{
Expand Down
32 changes: 22 additions & 10 deletions internal/remote/remote_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
150 changes: 146 additions & 4 deletions internal/remote/remote_catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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").
Expand Down Expand Up @@ -181,6 +179,123 @@ 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: 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: 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: 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: 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: false, Internal: false}; Kyma{Beta: true, Internal: true}; Expect Installation: true",
moduleBeta: false,
moduleInternal: false,
kymaBeta: true,
kymaInternal: true,
expected: true,
},
}

for _, tc := range testCases {

Check failure on line 288 in internal/remote/remote_catalog_test.go

View workflow job for this annotation

GitHub Actions / lint

variable name 'tc' is too short for the scope of its usage (varnamelen)
t.Run(tc.name, func(t *testing.T) {
mrm := newModuleReleaseMetaBuilder().withBeta(tc.moduleBeta).withInternal(tc.moduleInternal).build()
kyma := newKymaBuilder().withBeta(tc.kymaBeta).withInternal(tc.kymaInternal).build()

result := remote.IsAllowedModuleReleaseMeta(*mrm, kyma)
assert.Equal(t, tc.expected, result)
})
}
}

func moduleReleaseMetas() v1beta2.ModuleReleaseMetaList {
mrm1 := newModuleReleaseMetaBuilder().
withName("regular-module").
Expand Down Expand Up @@ -354,12 +469,21 @@ func (b *moduleReleaseMetaBuilder) withBetaEnabled() *moduleReleaseMetaBuilder {
b.moduleReleaseMeta.Spec.Beta = true
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
}
Expand Down Expand Up @@ -442,11 +566,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
}
Expand Down
25 changes: 23 additions & 2 deletions pkg/templatelookup/regular.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"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"
"github.com/kyma-project/lifecycle-manager/pkg/util"
)
Expand Down Expand Up @@ -58,7 +59,7 @@ func (t *TemplateLookup) GetRegularTemplates(ctx context.Context, kyma *v1beta2.
}

templateInfo := t.PopulateModuleTemplateInfo(ctx, module, kyma.Namespace, kyma.Spec.Channel)
templateInfo = ValidateTemplateMode(templateInfo, kyma)
templateInfo = t.ValidateTemplateMode(ctx, templateInfo, kyma)
if templateInfo.Err != nil {
templates[module.Name] = &templateInfo
continue
Expand Down Expand Up @@ -135,10 +136,21 @@ func (t *TemplateLookup) populateModuleTemplateInfoUsingModuleReleaseMeta(ctx co
return templateInfo
}

func ValidateTemplateMode(template ModuleTemplateInfo, kyma *v1beta2.Kyma) ModuleTemplateInfo {
func (t *TemplateLookup) ValidateTemplateMode(ctx context.Context, template ModuleTemplateInfo, kyma *v1beta2.Kyma) ModuleTemplateInfo {
if template.Err != nil {
return template
}

moduleReleaseMeta, err := GetModuleReleaseMeta(ctx, t, template.Spec.ModuleName, template.Namespace)

if util.IsNotFound(err) {
return validateTemplateModeWithoutModuleReleaseMeta(template, kyma)
}

return validateTemplateModeWithModuleReleaseMeta(template, kyma, moduleReleaseMeta)
}

func validateTemplateModeWithoutModuleReleaseMeta(template ModuleTemplateInfo, kyma *v1beta2.Kyma) ModuleTemplateInfo {
if template.IsInternal() && !kyma.IsInternal() {
template.Err = fmt.Errorf("%w: internal module", ErrTemplateNotAllowed)
return template
Expand All @@ -150,6 +162,15 @@ func ValidateTemplateMode(template ModuleTemplateInfo, kyma *v1beta2.Kyma) Modul
return template
}

func validateTemplateModeWithModuleReleaseMeta(template ModuleTemplateInfo, kyma *v1beta2.Kyma,
moduleReleaseMeta *v1beta2.ModuleReleaseMeta) ModuleTemplateInfo {
if !remote.IsAllowedModuleReleaseMeta(*moduleReleaseMeta, kyma) {
template.Err = fmt.Errorf("%w: module is beta or internal", ErrTemplateNotAllowed)
}

return template
}

func (t *TemplateLookup) getTemplateByVersion(ctx context.Context,
moduleName, moduleVersion, namespace string,
) (*v1beta2.ModuleTemplate, error) {
Expand Down
Loading

0 comments on commit dd4c423

Please sign in to comment.