From d38fcd9390fbfe13f458ca44b3e87fb74a7c676d Mon Sep 17 00:00:00 2001 From: nicholasSSUSE Date: Mon, 3 Jun 2024 17:37:46 -0300 Subject: [PATCH 1/2] making lifecycle code more modular and able to be called from make validate (validateRepo) --- main.go | 3 ++- pkg/lifecycle/lifecycle.go | 10 +++++----- pkg/lifecycle/lifecycle_test.go | 12 ++++++------ pkg/lifecycle/versions.rules.go | 12 +++++++++--- pkg/validate/validate.go | 2 ++ 5 files changed, 24 insertions(+), 15 deletions(-) diff --git a/main.go b/main.go index 46730986..e4c7877c 100644 --- a/main.go +++ b/main.go @@ -559,7 +559,8 @@ func checkRCTagsAndVersions(c *cli.Context) { func lifecycleAssetsClean(c *cli.Context) { // Initialize dependencies with branch-version, current chart and debug mode repoRoot := getRepoRoot() - lifeCycleDep, err := lifecycle.InitDependencies(repoRoot, c.String("branch-version"), CurrentChart, DebugMode) + rootFs := filesystem.GetFilesystem(repoRoot) + lifeCycleDep, err := lifecycle.InitDependencies(rootFs, c.String("branch-version"), CurrentChart, DebugMode) if err != nil { logrus.Fatalf("encountered error while initializing dependencies for lifecycle-assets-clean: %s", err) } diff --git a/pkg/lifecycle/lifecycle.go b/pkg/lifecycle/lifecycle.go index c69797dc..48bc95d0 100644 --- a/pkg/lifecycle/lifecycle.go +++ b/pkg/lifecycle/lifecycle.go @@ -22,7 +22,7 @@ type Asset struct { type Dependencies struct { rootFs billy.Filesystem assetsVersionsMap map[string][]Asset - vr *VersionRules + VR *VersionRules // These wrappers are used to mock the filesystem and git status in the tests walkDirWrapper WalkDirFunc makeRemoveWrapper MakeRemoveFunc @@ -58,7 +58,7 @@ func cycleLog(debugMode bool, msg string, data interface{}) { // InitDependencies will check the filesystem, branch version, // git status, initialize the Dependencies struct and populate it. // If anything fails the operation will be aborted. -func InitDependencies(repoRoot, branchVersion string, currentChart string, debug bool) (*Dependencies, error) { +func InitDependencies(rootFs billy.Filesystem, branchVersion string, currentChart string, debug bool) (*Dependencies, error) { logrus.SetFormatter(&logrus.TextFormatter{ DisableQuote: true, }) @@ -82,13 +82,13 @@ func InitDependencies(repoRoot, branchVersion string, currentChart string, debug cycleLog(debug, "Getting branch version rules for: ", branchVersion) // Initialize and check version rules for the current branch - dep.vr, err = GetVersionRules(branchVersion, debug) + dep.VR, err = GetVersionRules(branchVersion, debug) if err != nil { return nil, fmt.Errorf("encountered error while getting current branch version: %s", err) } // Get the filesystem and index.yaml path for the repository - dep.rootFs = filesystem.GetFilesystem(repoRoot) + dep.rootFs = rootFs // Check if the assets folder and Helm index file exists in the repository exists, err := filesystem.PathExists(dep.rootFs, path.RepositoryAssetsDir) @@ -172,7 +172,7 @@ func (ld *Dependencies) removeAssetsVersions(debug bool) (map[string][]Asset, er // Loop through the versions of the asset and remove the ones that are not in the lifecycle for _, asset := range assetsVersionsMap { - isVersionInLifecycle := ld.vr.checkChartVersionForLifecycle(asset.version) + isVersionInLifecycle := ld.VR.CheckChartVersionForLifecycle(asset.version) if isVersionInLifecycle { logrus.Debugf("Version %s is in lifecycle for %s", asset.version, chartName) continue // Skipping version in lifecycle diff --git a/pkg/lifecycle/lifecycle_test.go b/pkg/lifecycle/lifecycle_test.go index 700bf345..ef22bca3 100644 --- a/pkg/lifecycle/lifecycle_test.go +++ b/pkg/lifecycle/lifecycle_test.go @@ -18,7 +18,7 @@ func Test_removeAssetsVersions(t *testing.T) { checkIfGitIsCleanWrapper: func(debug bool) (bool, error) { return false, nil }, gitAddAndCommitWrapper: func(message string) error { return nil }, assetsVersionsMap: map[string][]Asset{"chart1": {{version: "999.0.0"}}}, - vr: vr, + VR: vr, } // Execute @@ -41,7 +41,7 @@ func Test_removeAssetsVersions(t *testing.T) { }, gitAddAndCommitWrapper: func(message string) error { return nil }, assetsVersionsMap: map[string][]Asset{"chart1": {{version: "999.0.0"}}}, - vr: vr, + VR: vr, } // Execute @@ -64,7 +64,7 @@ func Test_removeAssetsVersions(t *testing.T) { return fmt.Errorf("Some error at gitAddAndCommitWrapper") }, assetsVersionsMap: map[string][]Asset{"chart1": {{version: "999.0.0"}}}, - vr: vr, + VR: vr, } // Execute @@ -98,7 +98,7 @@ func Test_removeAssetsVersions(t *testing.T) { {version: "0.1.0"}, }, }, - vr: vr, + VR: vr, } // Execute @@ -148,7 +148,7 @@ func Test_removeAssetsVersions(t *testing.T) { {version: "101.0.0"}, }, }, - vr: vr, + VR: vr, } // Execute @@ -178,7 +178,7 @@ func Test_removeAssetsVersions(t *testing.T) { assetsVersionsMap: map[string][]Asset{ "chart1": {}, }, - vr: vr, + VR: vr, } // Execute diff --git a/pkg/lifecycle/versions.rules.go b/pkg/lifecycle/versions.rules.go index 6bf35cbd..28a0f1b1 100644 --- a/pkg/lifecycle/versions.rules.go +++ b/pkg/lifecycle/versions.rules.go @@ -76,7 +76,7 @@ func GetVersionRules(branchVersion string, debug bool) (*VersionRules, error) { // Branch can only hold until 2 previous versions of the current branch version. // Branch cannot hold versions from newer branches, only older ones. // -// See checkChartVersionForLifecycle() for more details. +// See CheckChartVersionForLifecycle() for more details. func (vr *VersionRules) getMinMaxVersionInts() { // e.g: 2.9 - 0.2 = 2.7 minVersionStr := vr.rules[(vr.branchVersion - 0.2)].min @@ -100,11 +100,17 @@ func convertBranchVersion(branchVersion string) (float32, error) { return float32(floatVersion), nil } -// checkChartVersionForLifecycle will +// ExtractBranchVersion will extract the branch version from the branch name +func ExtractBranchVersion(branch string) string { + parts := strings.Split(branch, "-v") + return parts[len(parts)-1] +} + +// CheckChartVersionForLifecycle will // Check if the chart version is within the range of the current version: // // If the chart version is within the range, return true, otherwise return false -func (vr *VersionRules) checkChartVersionForLifecycle(chartVersion string) bool { +func (vr *VersionRules) CheckChartVersionForLifecycle(chartVersion string) bool { chartVersionInt, _ := strconv.Atoi(strings.Split(chartVersion, ".")[0]) /** Rule Example: diff --git a/pkg/validate/validate.go b/pkg/validate/validate.go index 98f18b46..199cc989 100644 --- a/pkg/validate/validate.go +++ b/pkg/validate/validate.go @@ -7,6 +7,7 @@ import ( "github.com/go-git/go-billy/v5" "github.com/rancher/charts-build-scripts/pkg/filesystem" + "github.com/rancher/charts-build-scripts/pkg/lifecycle" "github.com/rancher/charts-build-scripts/pkg/options" "github.com/rancher/charts-build-scripts/pkg/path" "github.com/rancher/charts-build-scripts/pkg/puller" @@ -88,6 +89,7 @@ func CompareGeneratedAssets(repoFs billy.Filesystem, u options.UpstreamOptions, if err := standardize.RestructureChartsAndAssets(releaseFs); err != nil { return response, fmt.Errorf("failed to standardize upstream: %s", err) } + // TODO: Add a check bypass here, all assets that do not belong on this lifecycle should be skipped // Walk through directories and execute release logic localOnly := func(fs billy.Filesystem, localPath string, isDir bool) error { if isDir { From b039377d3bd846bfa119f4300450e47f8c701c43 Mon Sep 17 00:00:00 2001 From: nicholasSSUSE Date: Mon, 3 Jun 2024 18:03:21 -0300 Subject: [PATCH 2/2] add assets lifecycle rule check inside make validate --- pkg/validate/validate.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/validate/validate.go b/pkg/validate/validate.go index 199cc989..e41ff79c 100644 --- a/pkg/validate/validate.go +++ b/pkg/validate/validate.go @@ -70,6 +70,13 @@ func CompareGeneratedAssets(repoFs billy.Filesystem, u options.UpstreamOptions, ModifiedPostRelease: options.ReleaseOptions{}, RemovedPostRelease: options.ReleaseOptions{}, } + + // Initialize lifecycle package for validating with assets lifecycle rules + lifeCycleDep, err := lifecycle.InitDependencies(repoFs, lifecycle.ExtractBranchVersion(branch), "", false) + if err != nil { + logrus.Fatalf("encountered error while initializing dependencies for lifecycle-assets-clean: %s", err) + } + // Pull repository logrus.Infof("Pulling upstream repository %s at branch %s", u.URL, branch) releasedChartsRepoBranch, err := puller.GetGithubRepository(u, &branch) @@ -89,7 +96,7 @@ func CompareGeneratedAssets(repoFs billy.Filesystem, u options.UpstreamOptions, if err := standardize.RestructureChartsAndAssets(releaseFs); err != nil { return response, fmt.Errorf("failed to standardize upstream: %s", err) } - // TODO: Add a check bypass here, all assets that do not belong on this lifecycle should be skipped + // Walk through directories and execute release logic localOnly := func(fs billy.Filesystem, localPath string, isDir bool) error { if isDir { @@ -113,7 +120,12 @@ func CompareGeneratedAssets(repoFs billy.Filesystem, u options.UpstreamOptions, } // Chart exists in local and is not tracked by release.yaml logrus.Infof("%s/%s is untracked", chart.Metadata.Name, chart.Metadata.Version) - response.UntrackedInRelease = response.UntrackedInRelease.Append(chart.Metadata.Name, chart.Metadata.Version) + // If the chart exists in local and not on the upstream it may have been removed by the lifecycle rules + isVersionInLifecycle := lifeCycleDep.VR.CheckChartVersionForLifecycle(chart.Metadata.Version) + if isVersionInLifecycle { + // this chart should not be removed + response.UntrackedInRelease = response.UntrackedInRelease.Append(chart.Metadata.Name, chart.Metadata.Version) + } return nil }