Merge pull request #2620 from fluxcd/bug/hr-multi-element-img

Parse image refs in HelmReleases with >2 elements

pkg/cluster/kubernetes/resource/helmrelease.go

@@ -240,46 +240,52 @@ func interpretAsContainer(m mapper) (image.Ref, ImageSetter, bool) {
 	case string:
 		// container:
 		//   image: 'repo/image:tag'
-		imageRef, err := image.ParseRef(img)
-		if err == nil {
-			var reggy bool
-			if registry, ok := m.get("registry"); ok {
-				// container:
-				//   registry: registry.com
-				//	 image: repo/foo
-				if registryStr, ok := registry.(string); ok {
-					reggy = true
-					imageRef.Domain = registryStr
-				}
+		fullImgStr := img
+
+		var reggy bool
+		// container:
+		//   registry: registry.com
+		//	 image: repo/foo
+		if registry, ok := m.get("registry"); ok {
+			if registryStr, ok := registry.(string); ok {
+				reggy = true
+				fullImgStr = registryStr + "/" + fullImgStr
 			}
-			var taggy bool
-			if tag, ok := m.get("tag"); ok {
-				// container:
-				//   image: repo/foo
-				//   tag: v1
-				if tagStr, ok := tag.(string); ok {
-					taggy = true
-					imageRef.Tag = tagStr
-				}
+		}
+
+		var taggy bool
+		// container:
+		//   image: repo/foo
+		//   tag: v1
+		if tag, ok := m.get("tag"); ok {
+			if tagStr, ok := tag.(string); ok {
+				taggy = true
+				fullImgStr = fullImgStr + ":" + tagStr
 			}
-			return imageRef, func(ref image.Ref) {
-				switch {
-				case (reggy && taggy):
-					m.set("registry", ref.Domain)
-					m.set("image", ref.Image)
-					m.set("tag", ref.Tag)
-					return
-				case reggy:
-					m.set("registry", ref.Domain)
-					m.set("image", ref.Name.Image+":"+ref.Tag)
-				case taggy:
-					m.set("image", ref.Name.String())
-					m.set("tag", ref.Tag)
-				default:
-					m.set("image", ref.String())
-				}
-			}, true
 		}
+
+		imageRef, err := image.ParseRef(fullImgStr)
+		if err != nil {
+			return image.Ref{}, nil, false
+		}
+
+		return imageRef, func(ref image.Ref) {
+			switch {
+			case reggy && taggy:
+				m.set("registry", ref.Domain)
+				m.set("image", ref.Image)
+				m.set("tag", ref.Tag)
+				return
+			case reggy:
+				m.set("registry", ref.Domain)
+				m.set("image", ref.Name.Image+":"+ref.Tag)
+			case taggy:
+				m.set("image", ref.Name.String())
+				m.set("tag", ref.Tag)
+			default:
+				m.set("image", ref.String())
+			}
+		}, true
 	case map[string]interface{}:
 		return interpretAsImage(stringMap(img))
 	case map[interface{}]interface{}:
@@ -300,46 +306,52 @@ func interpretAsImage(m mapper) (image.Ref, ImageSetter, bool) {
 	// image:
 	//   repository: repo/foo
 	if imgStr, ok := imgRepo.(string); ok {
-		imageRef, err := image.ParseRef(imgStr)
-		if err == nil {
-			var reggy bool
-			// image:
-			//   registry: registry.com
-			//   repository: repo/foo
-			if registry, ok := m.get("registry"); ok {
-				if registryStr, ok := registry.(string); ok {
-					reggy = true
-					imageRef.Domain = registryStr
-				}
+		fullImgStr := imgStr
+
+		var reggy bool
+		// image:
+		//   registry: registry.com
+		//   repository: repo/foo
+		if registry, ok := m.get("registry"); ok {
+			if registryStr, ok := registry.(string); ok {
+				reggy = ok
+				fullImgStr = registryStr + "/" + fullImgStr
 			}
-			var taggy bool
-			// image:
-			//   repository: repo/foo
-			//   tag: v1
-			if tag, ok := m.get("tag"); ok {
-				if tagStr, ok := tag.(string); ok {
-					taggy = true
-					imageRef.Tag = tagStr
-				}
+		}
+
+		var taggy bool
+		// image:
+		//   repository: repo/foo
+		//   tag: v1
+		if tag, ok := m.get("tag"); ok {
+			if tagStr, ok := tag.(string); ok {
+				taggy = ok
+				fullImgStr = fullImgStr + ":" + tagStr
 			}
-			return imageRef, func(ref image.Ref) {
-				switch {
-				case (reggy && taggy):
-					m.set("registry", ref.Domain)
-					m.set("repository", ref.Image)
-					m.set("tag", ref.Tag)
-					return
-				case reggy:
-					m.set("registry", ref.Domain)
-					m.set("repository", ref.Name.Image+":"+ref.Tag)
-				case taggy:
-					m.set("repository", ref.Name.String())
-					m.set("tag", ref.Tag)
-				default:
-					m.set("repository", ref.String())
-				}
-			}, true
 		}
+
+		imageRef, err := image.ParseRef(fullImgStr)
+		if err != nil {
+			return image.Ref{}, nil, false
+		}
+
+		return imageRef, func(ref image.Ref) {
+			switch {
+			case reggy && taggy:
+				m.set("registry", ref.Domain)
+				m.set("repository", ref.Image)
+				m.set("tag", ref.Tag)
+				return
+			case reggy:
+				m.set("registry", ref.Domain)
+				m.set("repository", ref.Name.Image+":"+ref.Tag)
+			case taggy:
+				m.set("repository", ref.Name.String())
+				m.set("tag", ref.Tag)
+			default:
+				m.set("repository", ref.String())
+			}
+		}, true
 	}
 
 	return image.Ref{}, nil, false

pkg/cluster/kubernetes/resource/helmrelease_test.go

@@ -716,6 +716,83 @@ spec:
 	}
 }
 
+func TestParseNamedImageObjectFormatWithRegistryAndMultiElementImage(t *testing.T) {
+	expectedContainer := "db"
+	expectedRegistry := "registry.com"
+	expectedImageName := "public/bitnami/ghost"
+	expectedImageTag := "1.21.5-r0"
+	expectedImage := expectedRegistry + "/" + expectedImageName + ":" + expectedImageTag
+
+	doc := `---
+apiVersion: helm.fluxcd.io/v1
+kind: HelmRelease
+metadata:
+  name: ghost
+  namespace: ghost
+  labels:
+    chart: ghost
+spec:
+  chart:
+    git: [email protected]:fluxcd/flux-get-started
+    ref: master
+    path: charts/ghost
+  values:
+    other:
+      not: "containing image"
+    ` + expectedContainer + `:
+      first: post
+      image:
+        registry: ` + expectedRegistry + `
+        repository: ` + expectedImageName + `
+        tag: ` + expectedImageTag + `
+      persistence:
+        enabled: false
+`
+
+	resources, err := ParseMultidoc([]byte(doc), "test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	res, ok := resources["ghost:helmrelease/ghost"]
+	if !ok {
+		t.Fatalf("expected resource not found; instead got %#v", resources)
+	}
+	hr, ok := res.(resource.Workload)
+	if !ok {
+		t.Fatalf("expected resource to be a Workload, instead got %#v", res)
+	}
+
+	containers := hr.Containers()
+	if len(containers) != 1 {
+		t.Fatalf("expected 1 container; got %#v", containers)
+	}
+	image := containers[0].Image.String()
+	if image != expectedImage {
+		t.Errorf("expected container image %q, got %q", expectedImage, image)
+	}
+	if containers[0].Name != expectedContainer {
+		t.Errorf("expected container name %q, got %q", expectedContainer, containers[0].Name)
+	}
+
+	newImage := containers[0].Image.WithNewTag("some-other-tag")
+	newImage.Domain = "someotherregistry.com"
+	if err := hr.SetContainerImage(expectedContainer, newImage); err != nil {
+		t.Error(err)
+	}
+
+	containers = hr.Containers()
+	if len(containers) != 1 {
+		t.Fatalf("expected 1 container; got %#v", containers)
+	}
+	image = containers[0].Image.String()
+	if image != newImage.String() {
+		t.Errorf("expected container image %q, got %q", newImage.String(), image)
+	}
+	if containers[0].Name != expectedContainer {
+		t.Errorf("expected container name %q, got %q", expectedContainer, containers[0].Name)
+	}
+}
+
 func TestParseNamedImageObjectFormatWithRegistryWitoutTag(t *testing.T) {
 	expectedContainer := "db"
 	expectedRegistry := "registry.com"