fix: Revert "Update subpath from DeepCopy instead of pointer" (#598)

This reverts commit 3cedc29.

pkg/apis/cartographer/v1alpha1/workload_helpers.go

@@ -219,28 +219,17 @@ func (w *WorkloadSpec) Merge(updates *WorkloadSpec) {
 	for _, p := range updates.Params {
 		w.MergeParams(p.Name, p.Value)
 	}
-	sp := ""
-	if w.Source != nil {
-		sp = w.Source.Subpath
-	}
 	if updates.Image != "" {
 		w.MergeImage(updates.Image)
 	}
-	if updates.Source != nil && (updates.Source.Git != nil || updates.Source.Image != "") && sp != "" {
-		if w.Source == nil {
-			w.Source = &Source{}
-		}
-		w.Source.Subpath = sp
-	}
-	if updates.Source != nil {
-		s := updates.Source.DeepCopy()
+	if s := updates.Source; s != nil {
 		if s.Git != nil {
 			w.MergeGit(*s.Git)
 		}
 		if s.Image != "" {
 			w.MergeSourceImage(s.Image)
 		}
-		if s.Subpath != "" && w.Source != nil && (w.Source.Git != nil || w.Source.Image != "") {
+		if s.Subpath != "" {
 			w.MergeSubPath(s.Subpath)
 		}
 	}
@@ -337,7 +326,7 @@ func (w *WorkloadSpec) ResetSource() {
 }
 
 func (w *WorkloadSpec) MergeGit(git GitSource) {
-	stash := w.Source.DeepCopy()
+	stash := w.Source
 	image := w.Image
 	w.ResetSource()
 
@@ -352,30 +341,35 @@ func (w *WorkloadSpec) MergeGit(git GitSource) {
 		w.Source = &Source{
 			Git: &git,
 		}
-		if stash != nil && stash.Subpath != "" {
+		if stash != nil && stash.Git != nil {
 			w.Source.Subpath = stash.Subpath
 		}
 	}
 }
 
 func (w *WorkloadSpec) MergeSourceImage(image string) {
-	src := &Source{Image: image}
-	if w.Source != nil {
-		src.Subpath = w.Source.Subpath
-	}
+	stash := w.Source
 	w.ResetSource()
-	w.Source = src
+
+	w.Source = &Source{
+		Image: image,
+	}
+	if stash != nil && stash.Image != "" {
+		w.Source.Subpath = stash.Subpath
+	}
 }
 
 func (w *WorkloadSpec) MergeSubPath(subPath string) {
 	if w.Source == nil {
 		w.Source = &Source{}
 	}
+
 	w.Source.Subpath = subPath
 }
 
 func (w *WorkloadSpec) MergeImage(image string) {
 	w.ResetSource()
+
 	w.Image = image
 }
 

pkg/apis/cartographer/v1alpha1/workload_test.go

@@ -1001,134 +1001,6 @@ func TestWorkload_Merge(t *testing.T) {
 				Image: "ubuntu:bionic",
 			},
 		},
-	}, {
-		name: "image without source",
-		seed: &Workload{},
-		update: &Workload{
-			Spec: WorkloadSpec{
-				Image: "ubuntu:bionic",
-			},
-		},
-		want: &Workload{
-			Spec: WorkloadSpec{
-				Image: "ubuntu:bionic",
-			},
-		},
-	}, {
-		name: "image with subpath",
-		seed: &Workload{
-			Spec: WorkloadSpec{
-				Image: "alpine:latest",
-				Source: &Source{
-					Subpath: "/sys",
-				},
-			},
-		},
-		update: &Workload{
-			Spec: WorkloadSpec{
-				Image: "ubuntu:bionic",
-			},
-		},
-		want: &Workload{
-			Spec: WorkloadSpec{
-				Image: "ubuntu:bionic",
-			},
-		},
-	}, {
-		name: "image with sources nil",
-		seed: &Workload{
-			Spec: WorkloadSpec{
-				Image: "alpine:latest",
-			},
-		},
-		update: &Workload{
-			Spec: WorkloadSpec{
-				Image:  "ubuntu:bionic",
-				Source: &Source{Subpath: "my-subpath"},
-			},
-		},
-		want: &Workload{
-			Spec: WorkloadSpec{
-				Image: "ubuntu:bionic",
-			},
-		},
-	}, {
-		name: "workload with multiple sources and subpath",
-		seed: &Workload{
-			Spec: WorkloadSpec{
-				Image: "alpine:latest",
-			},
-		},
-		update: &Workload{
-			Spec: WorkloadSpec{
-				Image: "ubuntu:bionic",
-				Source: &Source{
-					Image:   "ubuntu:bionic",
-					Subpath: "my-subpath",
-				},
-			},
-		},
-		want: &Workload{
-			Spec: WorkloadSpec{
-				Source: &Source{
-					Image:   "ubuntu:bionic",
-					Subpath: "my-subpath",
-				},
-			},
-		},
-	}, {
-		name: "image update sources source image and subpath",
-		seed: &Workload{
-			Spec: WorkloadSpec{
-				Image: "alpine:latest",
-				Source: &Source{
-					Subpath: "new-subpath",
-				},
-			},
-		},
-		update: &Workload{
-			Spec: WorkloadSpec{
-				Image: "ubuntu:bionic",
-				Source: &Source{
-					Image:   "ubuntu:bionic",
-					Subpath: "my-subpath",
-				},
-			},
-		},
-		want: &Workload{
-			Spec: WorkloadSpec{
-				Source: &Source{
-					Image:   "ubuntu:bionic",
-					Subpath: "my-subpath",
-				},
-			},
-		},
-	}, {
-		name: "local source image with subpath update source image and subpath",
-		seed: &Workload{
-			Spec: WorkloadSpec{
-				Source: &Source{
-					Image:   "ubuntu:bionic",
-					Subpath: "/opt",
-				},
-			},
-		},
-		update: &Workload{
-			Spec: WorkloadSpec{
-				Source: &Source{
-					Image:   "ubuntu:bionic",
-					Subpath: "/sys",
-				},
-			},
-		},
-		want: &Workload{
-			Spec: WorkloadSpec{
-				Source: &Source{
-					Image:   "ubuntu:bionic",
-					Subpath: "/sys",
-				},
-			},
-		},
 	}, {
 		name: "git",
 		seed: &Workload{
@@ -1187,7 +1059,13 @@ func TestWorkload_Merge(t *testing.T) {
 				},
 			},
 		},
-		want: &Workload{},
+		want: &Workload{
+			Spec: WorkloadSpec{
+				Source: &Source{
+					Subpath: "test-path",
+				},
+			},
+		},
 	}, {
 		name: "env",
 		seed: &Workload{
@@ -1889,6 +1767,30 @@ func TestWorkloadSpec_MergeGit(t *testing.T) {
 				Subpath: "my-subpath",
 			},
 		},
+	}, {
+		name: "update to git source deleting subpath",
+		seed: &WorkloadSpec{
+			Source: &Source{
+				Image:   "my-registry.nip.io/my-folder/my-image:latest@sha:my-sha1234567890",
+				Subpath: "my-subpath",
+			},
+		},
+		git: GitSource{
+			URL: "git@github.com:example/repo.git",
+			Ref: GitRef{
+				Branch: "main",
+			},
+		},
+		want: &WorkloadSpec{
+			Source: &Source{
+				Git: &GitSource{
+					URL: "git@github.com:example/repo.git",
+					Ref: GitRef{
+						Branch: "main",
+					},
+				},
+			},
+		},
 	}, {
 		name: "delete source when setting repo to empty string",
 		seed: &WorkloadSpec{
@@ -1982,6 +1884,25 @@ func TestWorkloadSpec_MergeSourceImage(t *testing.T) {
 				Subpath: "my-subpath",
 			},
 		},
+	}, {
+		name: "update deleting subpath",
+		seed: &WorkloadSpec{
+			Source: &Source{
+				Git: &GitSource{
+					URL: "git@github.com:example/repo.git",
+					Ref: GitRef{
+						Branch: "main",
+					},
+				},
+				Subpath: "my-subpath",
+			},
+		},
+		sourceImage: "my-registry.nip.io/my-folder/my-image:latest@sha:my-sha1234567890",
+		want: &WorkloadSpec{
+			Source: &Source{
+				Image: "my-registry.nip.io/my-folder/my-image:latest@sha:my-sha1234567890",
+			},
+		},
 	}}
 
 	for _, test := range tests {