Skip to content

Commit e7d75c8

Browse files
thaJeztahthaJeztah
committed
api/types/container: fix validation for UTSMode, UsernsMode, PidMode
The IPCMode type was added in 497fc88, and from that patch, the intent was to allow `host` (without `:`), `""` (empty, default) or `container:<container ID>`, but the `Valid()` function seems to be too relaxed and accepting both `:`, as well as `host:<anything>`. No unit-tests were added in that patch, and integration-tests only tested for valid values. Later on, `PidMode`, and `UTSMode` were added in 23feaaa and f2e5207, both of which were implemented as a straight copy of the `IPCMode` implementation, copying the same bug. Finally, commit d4aec5f implemented unit-tests for these types, but testing for the wrong behavior of the implementation. This patch updates the validation to correctly invalidate `host[:<anything>]` and empty (`:`) types. Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent 1bd4866 commit e7d75c8

File tree

2 files changed

+25
-32
lines changed

2 files changed

+25
-32
lines changed

‎api/types/container/hostconfig.go‎

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -175,21 +175,12 @@ func (n UsernsMode) IsHost() bool {
175175

176176
// IsPrivate indicates whether the container uses the a private userns.
177177
func (n UsernsMode) IsPrivate() bool {
178-
return !(n.IsHost())
178+
return !n.IsHost()
179179
}
180180

181181
// Valid indicates whether the userns is valid.
182182
func (n UsernsMode) Valid() bool {
183-
if string(n) == "" {
184-
return true
185-
}
186-
187-
switch mode, _, _ := strings.Cut(string(n), ":"); mode {
188-
case "host":
189-
return true
190-
default:
191-
return false
192-
}
183+
return n == "" || n.IsHost()
193184
}
194185

195186
// CgroupSpec represents the cgroup to use for the container.
@@ -217,7 +208,7 @@ type UTSMode string
217208

218209
// IsPrivate indicates whether the container uses its private UTS namespace.
219210
func (n UTSMode) IsPrivate() bool {
220-
return !(n.IsHost())
211+
return !n.IsHost()
221212
}
222213

223214
// IsHost indicates whether the container uses the host's UTS namespace.
@@ -227,13 +218,7 @@ func (n UTSMode) IsHost() bool {
227218

228219
// Valid indicates whether the UTS namespace is valid.
229220
func (n UTSMode) Valid() bool {
230-
parts := strings.Split(string(n), ":")
231-
switch mode := parts[0]; mode {
232-
case "", "host":
233-
default:
234-
return false
235-
}
236-
return true
221+
return n == "" || n.IsHost()
237222
}
238223

239224
// PidMode represents the pid namespace of the container.
@@ -257,15 +242,7 @@ func (n PidMode) IsContainer() bool {
257242

258243
// Valid indicates whether the pid namespace is valid.
259244
func (n PidMode) Valid() bool {
260-
mode, v, ok := strings.Cut(string(n), ":")
261-
switch mode {
262-
case "", "host":
263-
return true
264-
case "container":
265-
return ok && v != ""
266-
default:
267-
return false
268-
}
245+
return n == "" || n.IsHost() || n.IsContainer()
269246
}
270247

271248
// Container returns the name of the container whose pid namespace is going to be used.

‎api/types/container/hostconfig_unix_test.go‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ func TestCgroupnsMode(t *testing.T) {
1515
"": {private: false, host: false, empty: true, valid: true},
1616
"something:weird": {private: false, host: false, empty: false, valid: false},
1717
"host": {private: false, host: true, empty: false, valid: true},
18-
"host:name": {private: false, host: false, empty: false, valid: false},
18+
"host:": {valid: false},
19+
"host:name": {valid: false},
20+
":name": {valid: false},
21+
":": {valid: false},
1922
"private": {private: true, host: false, empty: false, valid: true},
2023
"private:name": {private: false, host: false, empty: false, valid: false},
2124
}
@@ -70,6 +73,10 @@ func TestIpcMode(t *testing.T) {
7073
"something:weird": {},
7174
":weird": {},
7275
"host": {host: true, valid: true},
76+
"host:": {valid: false},
77+
"host:name": {valid: false},
78+
":name": {valid: false},
79+
":": {valid: false},
7380
"container": {},
7481
"container:": {container: true, valid: true, ctrName: ""},
7582
"container:name": {container: true, valid: true, ctrName: "name"},
@@ -94,7 +101,10 @@ func TestUTSMode(t *testing.T) {
94101
"": {private: true, host: false, valid: true},
95102
"something:weird": {private: true, host: false, valid: false},
96103
"host": {private: false, host: true, valid: true},
97-
"host:name": {private: true, host: false, valid: true},
104+
"host:": {private: true, valid: false},
105+
"host:name": {private: true, valid: false},
106+
":name": {private: true, valid: false},
107+
":": {private: true, valid: false},
98108
}
99109
for mode, expected := range modes {
100110
t.Run("mode="+string(mode), func(t *testing.T) {
@@ -111,7 +121,10 @@ func TestUsernsMode(t *testing.T) {
111121
"": {private: true, host: false, valid: true},
112122
"something:weird": {private: true, host: false, valid: false},
113123
"host": {private: false, host: true, valid: true},
114-
"host:name": {private: true, host: false, valid: true},
124+
"host:": {private: true, valid: false},
125+
"host:name": {private: true, valid: false},
126+
":name": {private: true, valid: false},
127+
":": {private: true, valid: false},
115128
}
116129
for mode, expected := range modes {
117130
t.Run("mode="+string(mode), func(t *testing.T) {
@@ -127,7 +140,10 @@ func TestPidMode(t *testing.T) {
127140
"": {private: true, host: false, valid: true},
128141
"something:weird": {private: true, host: false, valid: false},
129142
"host": {private: false, host: true, valid: true},
130-
"host:name": {private: true, host: false, valid: true},
143+
"host:": {private: true, valid: false},
144+
"host:name": {private: true, valid: false},
145+
":name": {private: true, valid: false},
146+
":": {private: true, valid: false},
131147
}
132148
for mode, expected := range modes {
133149
t.Run("mode="+string(mode), func(t *testing.T) {

0 commit comments

Comments
 (0)