change hold file contents and selection approach

Signed-off-by: Richard Gee <[email protected]>
alexellis · Nov 21, 2024 · 65ca4d2 · 65ca4d2
1 parent 35817a0
commit 65ca4d2
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 71 deletions.
diff --git a/cmd/chart/upgrade.go b/cmd/chart/upgrade.go
@@ -93,7 +93,7 @@ Otherwise, it returns a non-zero exit code and the updated values.yaml file.`,
 			return err
 		}
 
-		filtered := helm.FilterImagesUptoDepth(values, depth)
+		filtered := helm.FilterImagesUptoDepth(values, depth, 0, "")
 		if len(filtered) == 0 {
 			return fmt.Errorf("no images found in %s", file)
 		}
@@ -144,8 +144,8 @@ Otherwise, it returns a non-zero exit code and the updated values.yaml file.`,
 			}()
 		}
 
-		for k := range filtered {
-			workChan <- k
+		for _, v := range filtered {
+			workChan <- v
 		}
 
 		close(workChan)
@@ -297,8 +297,9 @@ func readFileLines(filename string) ([]string, error) {
 func removeHoldImages(fullset map[string]string, held []string) map[string]string {
 
 	for _, h := range held {
+		serviceName := strings.TrimSuffix(h, ".image")
 		for k := range fullset {
-			if strings.EqualFold(h, k[len(k)-len(h):]) {
+			if strings.EqualFold(serviceName, k) {
 				delete(fullset, k)
 			}
 		}

diff --git a/cmd/chart/upgrade_test.go b/cmd/chart/upgrade_test.go
@@ -384,91 +384,47 @@ func TestRemoveHoldImages(t *testing.T) {
 		{
 			name: "Basic exclusion",
 			fullset: map[string]string{
-				"registry/img1:16": "going",
-				"registry/img1:17": "staying",
-				"registry/img1:18": "staying",
+				"going":    "registry/img1:16",
+				"staying1": "registry/img1:17",
+				"staying2": "registry/img1:18",
 			},
 			held: []string{
-				"img1:16",
+				"going.image",
 			},
 			expected: map[string]string{
-				"registry/img1:17": "staying",
-				"registry/img1:18": "staying",
+				"staying1": "registry/img1:17",
+				"staying2": "registry/img1:18",
 			},
 		},
 		{
 			name: "Basic exclusion / muli-match",
 			fullset: map[string]string{
-				"registry/img1:16": "going",
-				"registry/img1:17": "going",
-				"registry/img1:18": "staying",
+				"going1":  "registry/img1:16",
+				"going2":  "registry/img1:17",
+				"staying": "registry/img1:18",
 			},
 			held: []string{
-				"img1:16",
-				"img1:17",
+				"going1.image",
+				"going2.image",
 			},
 			expected: map[string]string{
-				"registry/img1:18": "staying",
+				"staying": "registry/img1:18",
 			},
 		},
 		{
 			name: "No match",
 			fullset: map[string]string{
-				"registry/img1:17": "staying",
-				"registry/img1:18": "staying",
-				"registry/img1:19": "staying",
+				"staying1": "registry/img1:17",
+				"staying2": "registry/img1:18",
+				"staying3": "registry/img1:19",
 			},
 			held: []string{
-				"img1:16",
+				"going.image",
 			},
 			expected: map[string]string{
-				"registry/img1:17": "staying",
-				"registry/img1:18": "staying",
-				"registry/img1:19": "staying",
-			},
-		},
-		{
-			name: "Different Repos images match",
-			fullset: map[string]string{
-				"registry/repo/img1:16":  "going",
-				"registry/repo2/img1:16": "going",
-				"registry/repo3/img1:18": "staying",
-			},
-			held: []string{
-				"img1:16",
-			},
-			expected: map[string]string{
-				"registry/repo3/img1:18": "staying",
-			},
-		},
-		{
-			name: "Different Repos images match / full path exclude",
-			fullset: map[string]string{
-				"registry/repo/img1:16":  "going",
-				"registry/repo2/img1:16": "staying",
-				"registry/repo3/img1:18": "staying",
-			},
-			held: []string{
-				"registry/repo/img1:16",
-			},
-			expected: map[string]string{
-				"registry/repo2/img1:16": "staying",
-				"registry/repo3/img1:18": "staying",
-			},
-		},
-		{
-			name: "Different Repos images match / two exclude",
-			fullset: map[string]string{
-				"registry/repo/img1:16":  "going",
-				"registry/repo2/img1:16": "going",
-				"registry/repo3/img1:18": "staying",
-			},
-			held: []string{
-				"registry/repo/img1:16",
-				"img1:16",
-			},
-			expected: map[string]string{
-				"registry/repo3/img1:18": "staying",
+				"staying1": "registry/img1:17",
+				"staying2": "registry/img1:18",
+				"staying3": "registry/img1:19",
 			},
 		},
 		{

diff --git a/cmd/chart/verify.go b/cmd/chart/verify.go
@@ -72,7 +72,7 @@ autoscaler          ghcr.io/openfaasltd/autoscaler:0.2.5
 			return err
 		}
 
-		filtered := helm.FilterImagesUptoDepth(values, depth)
+		filtered := helm.FilterImagesUptoDepth(values, depth, 0, "")
 		if len(filtered) == 0 {
 			return fmt.Errorf("no images found in %s", file)
 		}

diff --git a/pkg/helm/io.go b/pkg/helm/io.go
@@ -59,18 +59,22 @@ func ReplaceValuesInHelmValuesFile(values map[string]string, yamlPath string) (s
 
 // FilterImagesUptoDepth takes a ValuesMap and returns a map of images that
 // were found upto max level
-func FilterImagesUptoDepth(values ValuesMap, depth int) map[string]string {
+func FilterImagesUptoDepth(values ValuesMap, depth int, level int, component string) map[string]string {
 	images := map[string]string{}
 
 	for k, v := range values {
 
+		if level == 1 {
+			component = k
+		}
+
 		if k == "image" && reflect.TypeOf(v).Kind() == reflect.String {
 			imageUrl := v.(string)
-			images[imageUrl] = imageUrl
+			images[component] = imageUrl
 		}
 
 		if c, ok := v.(ValuesMap); ok && depth > 0 {
-			images = mergeMaps(images, FilterImagesUptoDepth(c, depth-1))
+			images = mergeMaps(images, FilterImagesUptoDepth(c, depth-1, level+1, component))
 		}
 	}
 	return images