Skip to content

Commit f32ebfc

Browse files
officialasishkumarcpuguy83
authored andcommitted
fix: merge User field in MergeSpecImage
MergeSpecImage merges all ImageConfig fields from the target-level config into the spec-level config, but the User field was omitted. This caused the user field set under targets.<target>.image to be silently ignored when building a container image. Add the User field merge following the same override-if-non-empty pattern used for all other string fields (Entrypoint, Cmd, WorkingDir, StopSignal, Base). Also add comprehensive unit tests for MergeSpecImage covering all field merge behaviors. Fixes #1013 Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
1 parent 00477ac commit f32ebfc

File tree

2 files changed

+187
-0
lines changed

2 files changed

+187
-0
lines changed

imgconfig.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ func MergeSpecImage(spec *Spec, targetKey string) *ImageConfig {
5757
if i.Base != "" {
5858
cfg.Base = i.Base
5959
}
60+
61+
if i.User != "" {
62+
cfg.User = i.User
63+
}
6064
}
6165

6266
return &cfg

imgconfig_test.go

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
package dalec
2+
3+
import (
4+
"testing"
5+
6+
"gotest.tools/v3/assert"
7+
"gotest.tools/v3/assert/cmp"
8+
)
9+
10+
func TestMergeSpecImage(t *testing.T) {
11+
t.Run("nil spec image returns empty config", func(t *testing.T) {
12+
spec := &Spec{}
13+
cfg := MergeSpecImage(spec, "foo")
14+
assert.Check(t, cmp.DeepEqual(cfg, &ImageConfig{}))
15+
})
16+
17+
t.Run("spec image with no target returns spec image", func(t *testing.T) {
18+
spec := &Spec{
19+
Image: &ImageConfig{
20+
Entrypoint: "/bin/sh",
21+
Cmd: "-c",
22+
User: "1000",
23+
WorkingDir: "/app",
24+
StopSignal: "SIGTERM",
25+
},
26+
}
27+
cfg := MergeSpecImage(spec, "foo")
28+
assert.Check(t, cmp.Equal(cfg.Entrypoint, "/bin/sh"))
29+
assert.Check(t, cmp.Equal(cfg.Cmd, "-c"))
30+
assert.Check(t, cmp.Equal(cfg.User, "1000"))
31+
assert.Check(t, cmp.Equal(cfg.WorkingDir, "/app"))
32+
assert.Check(t, cmp.Equal(cfg.StopSignal, "SIGTERM"))
33+
})
34+
35+
t.Run("target overrides user", func(t *testing.T) {
36+
spec := &Spec{
37+
Image: &ImageConfig{
38+
User: "1000",
39+
},
40+
Targets: map[string]Target{
41+
"azlinux3": {
42+
Image: &ImageConfig{
43+
User: "999",
44+
},
45+
},
46+
},
47+
}
48+
cfg := MergeSpecImage(spec, "azlinux3")
49+
assert.Check(t, cmp.Equal(cfg.User, "999"))
50+
})
51+
52+
t.Run("target sets user when spec has none", func(t *testing.T) {
53+
spec := &Spec{
54+
Targets: map[string]Target{
55+
"azlinux3": {
56+
Image: &ImageConfig{
57+
User: "999",
58+
},
59+
},
60+
},
61+
}
62+
cfg := MergeSpecImage(spec, "azlinux3")
63+
assert.Check(t, cmp.Equal(cfg.User, "999"))
64+
})
65+
66+
t.Run("spec user preserved when target does not set it", func(t *testing.T) {
67+
spec := &Spec{
68+
Image: &ImageConfig{
69+
User: "1000",
70+
},
71+
Targets: map[string]Target{
72+
"azlinux3": {
73+
Image: &ImageConfig{
74+
Cmd: "echo hello",
75+
},
76+
},
77+
},
78+
}
79+
cfg := MergeSpecImage(spec, "azlinux3")
80+
assert.Check(t, cmp.Equal(cfg.User, "1000"))
81+
})
82+
83+
t.Run("target overrides all string fields", func(t *testing.T) {
84+
spec := &Spec{
85+
Image: &ImageConfig{
86+
Entrypoint: "/bin/old",
87+
Cmd: "old",
88+
WorkingDir: "/old",
89+
StopSignal: "SIGINT",
90+
Base: "old:latest",
91+
User: "root",
92+
},
93+
Targets: map[string]Target{
94+
"t1": {
95+
Image: &ImageConfig{
96+
Entrypoint: "/bin/new",
97+
Cmd: "new",
98+
WorkingDir: "/new",
99+
StopSignal: "SIGTERM",
100+
Base: "new:latest",
101+
User: "nobody",
102+
},
103+
},
104+
},
105+
}
106+
cfg := MergeSpecImage(spec, "t1")
107+
assert.Check(t, cmp.Equal(cfg.Entrypoint, "/bin/new"))
108+
assert.Check(t, cmp.Equal(cfg.Cmd, "new"))
109+
assert.Check(t, cmp.Equal(cfg.WorkingDir, "/new"))
110+
assert.Check(t, cmp.Equal(cfg.StopSignal, "SIGTERM"))
111+
assert.Check(t, cmp.Equal(cfg.Base, "new:latest"))
112+
assert.Check(t, cmp.Equal(cfg.User, "nobody"))
113+
})
114+
115+
t.Run("target env appends to spec env", func(t *testing.T) {
116+
spec := &Spec{
117+
Image: &ImageConfig{
118+
Env: []string{"A=1"},
119+
},
120+
Targets: map[string]Target{
121+
"t1": {
122+
Image: &ImageConfig{
123+
Env: []string{"B=2"},
124+
},
125+
},
126+
},
127+
}
128+
cfg := MergeSpecImage(spec, "t1")
129+
assert.Check(t, cmp.DeepEqual(cfg.Env, []string{"A=1", "B=2"}))
130+
})
131+
132+
t.Run("target volumes merge with spec volumes", func(t *testing.T) {
133+
spec := &Spec{
134+
Image: &ImageConfig{
135+
Volumes: map[string]struct{}{"/data": {}},
136+
},
137+
Targets: map[string]Target{
138+
"t1": {
139+
Image: &ImageConfig{
140+
Volumes: map[string]struct{}{"/logs": {}},
141+
},
142+
},
143+
},
144+
}
145+
cfg := MergeSpecImage(spec, "t1")
146+
assert.Check(t, cmp.Len(cfg.Volumes, 2))
147+
_, hasData := cfg.Volumes["/data"]
148+
_, hasLogs := cfg.Volumes["/logs"]
149+
assert.Check(t, hasData)
150+
assert.Check(t, hasLogs)
151+
})
152+
153+
t.Run("target labels merge with spec labels", func(t *testing.T) {
154+
spec := &Spec{
155+
Image: &ImageConfig{
156+
Labels: map[string]string{"a": "1"},
157+
},
158+
Targets: map[string]Target{
159+
"t1": {
160+
Image: &ImageConfig{
161+
Labels: map[string]string{"b": "2"},
162+
},
163+
},
164+
},
165+
}
166+
cfg := MergeSpecImage(spec, "t1")
167+
assert.Check(t, cmp.Len(cfg.Labels, 2))
168+
assert.Check(t, cmp.Equal(cfg.Labels["a"], "1"))
169+
assert.Check(t, cmp.Equal(cfg.Labels["b"], "2"))
170+
})
171+
172+
t.Run("nonexistent target key returns spec image unchanged", func(t *testing.T) {
173+
spec := &Spec{
174+
Image: &ImageConfig{
175+
User: "1000",
176+
Entrypoint: "/bin/sh",
177+
},
178+
}
179+
cfg := MergeSpecImage(spec, "nonexistent")
180+
assert.Check(t, cmp.Equal(cfg.User, "1000"))
181+
assert.Check(t, cmp.Equal(cfg.Entrypoint, "/bin/sh"))
182+
})
183+
}

0 commit comments

Comments
 (0)