Skip to content

Commit a478026

Browse files
warango4warango4
committed
fix: Revert "fix: Update subpath from DeepCopy instead of pointer (vmware-tanzu#580)"
This reverts commit 3cedc29.
1 parent 8783fd7 commit a478026

6 files changed

+79
-582
lines changed

‎pkg/apis/cartographer/v1alpha1/workload_helpers.go

+14-20
Original file line numberDiff line numberDiff line change
@@ -219,28 +219,17 @@ func (w *WorkloadSpec) Merge(updates *WorkloadSpec) {
219219
for _, p := range updates.Params {
220220
w.MergeParams(p.Name, p.Value)
221221
}
222-
sp := ""
223-
if w.Source != nil {
224-
sp = w.Source.Subpath
225-
}
226222
if updates.Image != "" {
227223
w.MergeImage(updates.Image)
228224
}
229-
if updates.Source != nil && (updates.Source.Git != nil || updates.Source.Image != "") && sp != "" {
230-
if w.Source == nil {
231-
w.Source = &Source{}
232-
}
233-
w.Source.Subpath = sp
234-
}
235-
if updates.Source != nil {
236-
s := updates.Source.DeepCopy()
225+
if s := updates.Source; s != nil {
237226
if s.Git != nil {
238227
w.MergeGit(*s.Git)
239228
}
240229
if s.Image != "" {
241230
w.MergeSourceImage(s.Image)
242231
}
243-
if s.Subpath != "" && w.Source != nil && (w.Source.Git != nil || w.Source.Image != "") {
232+
if s.Subpath != "" {
244233
w.MergeSubPath(s.Subpath)
245234
}
246235
}
@@ -337,7 +326,7 @@ func (w *WorkloadSpec) ResetSource() {
337326
}
338327

339328
func (w *WorkloadSpec) MergeGit(git GitSource) {
340-
stash := w.Source.DeepCopy()
329+
stash := w.Source
341330
image := w.Image
342331
w.ResetSource()
343332

@@ -352,30 +341,35 @@ func (w *WorkloadSpec) MergeGit(git GitSource) {
352341
w.Source = &Source{
353342
Git: &git,
354343
}
355-
if stash != nil && stash.Subpath != "" {
344+
if stash != nil && stash.Git != nil {
356345
w.Source.Subpath = stash.Subpath
357346
}
358347
}
359348
}
360349

361350
func (w *WorkloadSpec) MergeSourceImage(image string) {
362-
src := &Source{Image: image}
363-
if w.Source != nil {
364-
src.Subpath = w.Source.Subpath
365-
}
351+
stash := w.Source
366352
w.ResetSource()
367-
w.Source = src
353+
354+
w.Source = &Source{
355+
Image: image,
356+
}
357+
if stash != nil && stash.Image != "" {
358+
w.Source.Subpath = stash.Subpath
359+
}
368360
}
369361

370362
func (w *WorkloadSpec) MergeSubPath(subPath string) {
371363
if w.Source == nil {
372364
w.Source = &Source{}
373365
}
366+
374367
w.Source.Subpath = subPath
375368
}
376369

377370
func (w *WorkloadSpec) MergeImage(image string) {
378371
w.ResetSource()
372+
379373
w.Image = image
380374
}
381375

‎pkg/apis/cartographer/v1alpha1/workload_test.go

+50-129
Original file line numberDiff line numberDiff line change
@@ -1001,134 +1001,6 @@ func TestWorkload_Merge(t *testing.T) {
10011001
Image: "ubuntu:bionic",
10021002
},
10031003
},
1004-
}, {
1005-
name: "image without source",
1006-
seed: &Workload{},
1007-
update: &Workload{
1008-
Spec: WorkloadSpec{
1009-
Image: "ubuntu:bionic",
1010-
},
1011-
},
1012-
want: &Workload{
1013-
Spec: WorkloadSpec{
1014-
Image: "ubuntu:bionic",
1015-
},
1016-
},
1017-
}, {
1018-
name: "image with subpath",
1019-
seed: &Workload{
1020-
Spec: WorkloadSpec{
1021-
Image: "alpine:latest",
1022-
Source: &Source{
1023-
Subpath: "/sys",
1024-
},
1025-
},
1026-
},
1027-
update: &Workload{
1028-
Spec: WorkloadSpec{
1029-
Image: "ubuntu:bionic",
1030-
},
1031-
},
1032-
want: &Workload{
1033-
Spec: WorkloadSpec{
1034-
Image: "ubuntu:bionic",
1035-
},
1036-
},
1037-
}, {
1038-
name: "image with sources nil",
1039-
seed: &Workload{
1040-
Spec: WorkloadSpec{
1041-
Image: "alpine:latest",
1042-
},
1043-
},
1044-
update: &Workload{
1045-
Spec: WorkloadSpec{
1046-
Image: "ubuntu:bionic",
1047-
Source: &Source{Subpath: "my-subpath"},
1048-
},
1049-
},
1050-
want: &Workload{
1051-
Spec: WorkloadSpec{
1052-
Image: "ubuntu:bionic",
1053-
},
1054-
},
1055-
}, {
1056-
name: "workload with multiple sources and subpath",
1057-
seed: &Workload{
1058-
Spec: WorkloadSpec{
1059-
Image: "alpine:latest",
1060-
},
1061-
},
1062-
update: &Workload{
1063-
Spec: WorkloadSpec{
1064-
Image: "ubuntu:bionic",
1065-
Source: &Source{
1066-
Image: "ubuntu:bionic",
1067-
Subpath: "my-subpath",
1068-
},
1069-
},
1070-
},
1071-
want: &Workload{
1072-
Spec: WorkloadSpec{
1073-
Source: &Source{
1074-
Image: "ubuntu:bionic",
1075-
Subpath: "my-subpath",
1076-
},
1077-
},
1078-
},
1079-
}, {
1080-
name: "image update sources source image and subpath",
1081-
seed: &Workload{
1082-
Spec: WorkloadSpec{
1083-
Image: "alpine:latest",
1084-
Source: &Source{
1085-
Subpath: "new-subpath",
1086-
},
1087-
},
1088-
},
1089-
update: &Workload{
1090-
Spec: WorkloadSpec{
1091-
Image: "ubuntu:bionic",
1092-
Source: &Source{
1093-
Image: "ubuntu:bionic",
1094-
Subpath: "my-subpath",
1095-
},
1096-
},
1097-
},
1098-
want: &Workload{
1099-
Spec: WorkloadSpec{
1100-
Source: &Source{
1101-
Image: "ubuntu:bionic",
1102-
Subpath: "my-subpath",
1103-
},
1104-
},
1105-
},
1106-
}, {
1107-
name: "local source image with subpath update source image and subpath",
1108-
seed: &Workload{
1109-
Spec: WorkloadSpec{
1110-
Source: &Source{
1111-
Image: "ubuntu:bionic",
1112-
Subpath: "/opt",
1113-
},
1114-
},
1115-
},
1116-
update: &Workload{
1117-
Spec: WorkloadSpec{
1118-
Source: &Source{
1119-
Image: "ubuntu:bionic",
1120-
Subpath: "/sys",
1121-
},
1122-
},
1123-
},
1124-
want: &Workload{
1125-
Spec: WorkloadSpec{
1126-
Source: &Source{
1127-
Image: "ubuntu:bionic",
1128-
Subpath: "/sys",
1129-
},
1130-
},
1131-
},
11321004
}, {
11331005
name: "git",
11341006
seed: &Workload{
@@ -1187,7 +1059,13 @@ func TestWorkload_Merge(t *testing.T) {
11871059
},
11881060
},
11891061
},
1190-
want: &Workload{},
1062+
want: &Workload{
1063+
Spec: WorkloadSpec{
1064+
Source: &Source{
1065+
Subpath: "test-path",
1066+
},
1067+
},
1068+
},
11911069
}, {
11921070
name: "env",
11931071
seed: &Workload{
@@ -1889,6 +1767,30 @@ func TestWorkloadSpec_MergeGit(t *testing.T) {
18891767
Subpath: "my-subpath",
18901768
},
18911769
},
1770+
}, {
1771+
name: "update to git source deleting subpath",
1772+
seed: &WorkloadSpec{
1773+
Source: &Source{
1774+
Image: "my-registry.nip.io/my-folder/my-image:latest@sha:my-sha1234567890",
1775+
Subpath: "my-subpath",
1776+
},
1777+
},
1778+
git: GitSource{
1779+
URL: "[email protected]:example/repo.git",
1780+
Ref: GitRef{
1781+
Branch: "main",
1782+
},
1783+
},
1784+
want: &WorkloadSpec{
1785+
Source: &Source{
1786+
Git: &GitSource{
1787+
URL: "[email protected]:example/repo.git",
1788+
Ref: GitRef{
1789+
Branch: "main",
1790+
},
1791+
},
1792+
},
1793+
},
18921794
}, {
18931795
name: "delete source when setting repo to empty string",
18941796
seed: &WorkloadSpec{
@@ -1982,6 +1884,25 @@ func TestWorkloadSpec_MergeSourceImage(t *testing.T) {
19821884
Subpath: "my-subpath",
19831885
},
19841886
},
1887+
}, {
1888+
name: "update deleting subpath",
1889+
seed: &WorkloadSpec{
1890+
Source: &Source{
1891+
Git: &GitSource{
1892+
URL: "[email protected]:example/repo.git",
1893+
Ref: GitRef{
1894+
Branch: "main",
1895+
},
1896+
},
1897+
Subpath: "my-subpath",
1898+
},
1899+
},
1900+
sourceImage: "my-registry.nip.io/my-folder/my-image:latest@sha:my-sha1234567890",
1901+
want: &WorkloadSpec{
1902+
Source: &Source{
1903+
Image: "my-registry.nip.io/my-folder/my-image:latest@sha:my-sha1234567890",
1904+
},
1905+
},
19851906
}}
19861907

19871908
for _, test := range tests {

‎pkg/commands/testdata/workload-lsp-image-non-subPath.yaml

-31
This file was deleted.

‎pkg/commands/testdata/workload-lsp-non-subPath.yaml

-30
This file was deleted.

‎pkg/commands/testdata/workload-lsp-subPath.yaml

-31
This file was deleted.

0 commit comments

Comments
 (0)