From 1ca44dd49de54b1e15dd733cadb675354a1a61e0 Mon Sep 17 00:00:00 2001 From: Jose Luis Vazquez Gonzalez Date: Tue, 19 Apr 2022 18:38:26 +0200 Subject: [PATCH 1/6] Enforce hierarchical chart changes ordering Signed-off-by: Jose Luis Vazquez Gonzalez --- pkg/mover/chart.go | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/pkg/mover/chart.go b/pkg/mover/chart.go index f6cb6bcf..84adc4d5 100644 --- a/pkg/mover/chart.go +++ b/pkg/mover/chart.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "regexp" + "sort" "github.com/google/go-containerregistry/pkg/authn" @@ -153,6 +154,11 @@ type ChartMover struct { rawHints []byte } +type ChartChanges struct { + chart *chart.Chart + changes []*internal.RewriteAction +} + // NewChartMover creates a ChartMover to relocate a chart following the given // imagePatters and rules. func NewChartMover(req *ChartMoveRequest, opts ...Option) (*ChartMover, error) { @@ -399,20 +405,20 @@ func (cm *ChartMover) printMove() { src, change.RewrittenReference.Name(), change.Digest, pushRequiredTxt) } - for chartToModify, changes := range groupChangesByChart(cm.chartChanges, cm.chart) { - log.Printf("\nChanges to be applied to %s/values.yaml:\n", chartToModify.ChartFullPath()) - for _, change := range changes { + for _, chartChanges := range orderedChangesByChart(cm.chartChanges, cm.chart) { + log.Printf("\nChanges to be applied to %s/values.yaml:\n", chartChanges.chart.ChartFullPath()) + for _, change := range chartChanges.changes { // Remove chart name from the path since we are already indicating what values.yaml file we are changing - log.Printf(" %s: %s\n", namespacedPath(change.Path, chartToModify.Name()), change.Value) + log.Printf(" %s: %s\n", namespacedPath(change.Path, chartChanges.chart.Name()), change.Value) } log.Println() } } -// Return the grouped set of changes by Helm Chart. +// Return the ordered set of changes grouped by Helm Chart. // Meaning that changes to be performed to a given helm chart will be returned under the same map key -func groupChangesByChart(changes []*internal.RewriteAction, rootChart *chart.Chart) map[*chart.Chart][]*internal.RewriteAction { +func orderedChangesByChart(changes []*internal.RewriteAction, rootChart *chart.Chart) []ChartChanges { groupedChanges := make(map[*chart.Chart][]*internal.RewriteAction) for _, change := range changes { @@ -424,7 +430,20 @@ func groupChangesByChart(changes []*internal.RewriteAction, rootChart *chart.Cha } } - return groupedChanges + return orderByChartHierarchy(groupedChanges) +} + +func orderByChartHierarchy(groupedChanges map[*chart.Chart][]*internal.RewriteAction) []ChartChanges { + allChanges := make([]ChartChanges, 0, len(groupedChanges)) + + for chart, changes := range groupedChanges { + allChanges = append(allChanges, ChartChanges{chart: chart, changes: changes}) + } + + sort.Slice(allChanges, func(i, j int) bool { + return allChanges[i].chart.ChartFullPath() < allChanges[j].chart.ChartFullPath() + }) + return allChanges } // namespacedPath removes the chartName from the beginning of the full path From 82a50a1ae33d5e7abae7672cbc8b19b10cd6dc22 Mon Sep 17 00:00:00 2001 From: Jose Luis Vazquez Gonzalez Date: Tue, 19 Apr 2022 18:47:05 +0200 Subject: [PATCH 2/6] Fix go fmt Signed-off-by: Jose Luis Vazquez Gonzalez --- pkg/mover/chart.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/mover/chart.go b/pkg/mover/chart.go index 84adc4d5..7052168c 100644 --- a/pkg/mover/chart.go +++ b/pkg/mover/chart.go @@ -155,7 +155,7 @@ type ChartMover struct { } type ChartChanges struct { - chart *chart.Chart + chart *chart.Chart changes []*internal.RewriteAction } @@ -440,8 +440,8 @@ func orderByChartHierarchy(groupedChanges map[*chart.Chart][]*internal.RewriteAc allChanges = append(allChanges, ChartChanges{chart: chart, changes: changes}) } - sort.Slice(allChanges, func(i, j int) bool { - return allChanges[i].chart.ChartFullPath() < allChanges[j].chart.ChartFullPath() + sort.Slice(allChanges, func(i, j int) bool { + return allChanges[i].chart.ChartFullPath() < allChanges[j].chart.ChartFullPath() }) return allChanges } From e81709b4ab254c484a4e4af58301687cb9c95558 Mon Sep 17 00:00:00 2001 From: Jose Luis Vazquez Gonzalez Date: Tue, 19 Apr 2022 19:08:32 +0200 Subject: [PATCH 3/6] Fix tests and ordering Signed-off-by: Jose Luis Vazquez Gonzalez --- pkg/mover/chart.go | 4 +++- pkg/mover/chart_test.go | 19 +++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/pkg/mover/chart.go b/pkg/mover/chart.go index 7052168c..2ab868cb 100644 --- a/pkg/mover/chart.go +++ b/pkg/mover/chart.go @@ -441,7 +441,9 @@ func orderByChartHierarchy(groupedChanges map[*chart.Chart][]*internal.RewriteAc } sort.Slice(allChanges, func(i, j int) bool { - return allChanges[i].chart.ChartFullPath() < allChanges[j].chart.ChartFullPath() + a := allChanges[i].chart.ChartFullPath() + b := allChanges[j].chart.ChartFullPath() + return len(a) < len(b) || a < b }) return allChanges } diff --git a/pkg/mover/chart_test.go b/pkg/mover/chart_test.go index 3d704b67..db48e6af 100644 --- a/pkg/mover/chart_test.go +++ b/pkg/mover/chart_test.go @@ -714,9 +714,9 @@ func TestGroupChangesByChart(t *testing.T) { rewrites := []*internal.RewriteAction{r1, subchart1R1, subchart1R2, subchart1Subchart3, subchart2R1} // Expected output - want := make(map[*chart.Chart][]*internal.RewriteAction) + want := make([]ChartChanges, 4) // parent chart - want[rootChart] = []*internal.RewriteAction{r1} + want[0] = ChartChanges{chart: rootChart, changes: []*internal.RewriteAction{r1}} firstLevelDeps := rootChart.Dependencies() // Sort dependencies since they come in arbitrary order @@ -725,16 +725,23 @@ func TestGroupChangesByChart(t *testing.T) { }) // Subchart1 - want[firstLevelDeps[0]] = []*internal.RewriteAction{subchart1R1, subchart1R2} + want[1] = ChartChanges{chart: firstLevelDeps[0], changes: []*internal.RewriteAction{subchart1R1, subchart1R2}} // Subchart2 - want[firstLevelDeps[1]] = []*internal.RewriteAction{subchart2R1} + want[2] = ChartChanges{chart: firstLevelDeps[1], changes: []*internal.RewriteAction{subchart2R1}} // Subchart1.Subchart3 - want[firstLevelDeps[0].Dependencies()[0]] = []*internal.RewriteAction{subchart1Subchart3} + want[3] = ChartChanges{chart: firstLevelDeps[0].Dependencies()[0], changes: []*internal.RewriteAction{subchart1Subchart3}} + + for i, w := range want { + fmt.Printf("w[%d] = %+v\n", i, w) + } // Compare output - if got := groupChangesByChart(rewrites, rootChart); !reflect.DeepEqual(got, want) { + if got := orderedChangesByChart(rewrites, rootChart); !reflect.DeepEqual(got, want) { + for i, g := range got { + fmt.Printf("g[%d] = %+v\n", i, g) + } t.Errorf("got=%v; want=%v", got, want) } } From 3331dcf830a62ee222fe588847ab411e97d3d75d Mon Sep 17 00:00:00 2001 From: Jose Luis Vazquez Gonzalez Date: Tue, 19 Apr 2022 19:22:02 +0200 Subject: [PATCH 4/6] Make ordering really hierarchical Signed-off-by: Jose Luis Vazquez Gonzalez --- pkg/mover/chart.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/mover/chart.go b/pkg/mover/chart.go index 2ab868cb..8376adb0 100644 --- a/pkg/mover/chart.go +++ b/pkg/mover/chart.go @@ -11,6 +11,7 @@ import ( "path/filepath" "regexp" "sort" + "strings" "github.com/google/go-containerregistry/pkg/authn" @@ -443,7 +444,9 @@ func orderByChartHierarchy(groupedChanges map[*chart.Chart][]*internal.RewriteAc sort.Slice(allChanges, func(i, j int) bool { a := allChanges[i].chart.ChartFullPath() b := allChanges[j].chart.ChartFullPath() - return len(a) < len(b) || a < b + pathA := strings.Split(a, "/") + pathB := strings.Split(b, "/") + return len(pathA) < len(pathB) || a < b }) return allChanges } From 79f7493ccd76e2967beede129f1221d7005a5bf1 Mon Sep 17 00:00:00 2001 From: Jose Luis Vazquez Gonzalez Date: Tue, 19 Apr 2022 19:42:15 +0200 Subject: [PATCH 5/6] Update bogus linter Signed-off-by: Jose Luis Vazquez Gonzalez --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index db0d3b16..214b01f8 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -20,6 +20,6 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v2 with: - version: v1.43 + version: v1.45.2 # Optional: show only new issues if it's a pull request. The default value is `false`. only-new-issues: true From a3c19c91bf9e6e976d1c767e81c82cca86825d82 Mon Sep 17 00:00:00 2001 From: Jose Luis Vazquez Gonzalez Date: Tue, 19 Apr 2022 23:04:24 +0200 Subject: [PATCH 6/6] Remove traces in test Signed-off-by: Jose Luis Vazquez Gonzalez --- pkg/mover/chart_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pkg/mover/chart_test.go b/pkg/mover/chart_test.go index db48e6af..b105fdd5 100644 --- a/pkg/mover/chart_test.go +++ b/pkg/mover/chart_test.go @@ -733,15 +733,8 @@ func TestGroupChangesByChart(t *testing.T) { // Subchart1.Subchart3 want[3] = ChartChanges{chart: firstLevelDeps[0].Dependencies()[0], changes: []*internal.RewriteAction{subchart1Subchart3}} - for i, w := range want { - fmt.Printf("w[%d] = %+v\n", i, w) - } - // Compare output if got := orderedChangesByChart(rewrites, rootChart); !reflect.DeepEqual(got, want) { - for i, g := range got { - fmt.Printf("g[%d] = %+v\n", i, g) - } t.Errorf("got=%v; want=%v", got, want) } }