Browse code

vendor: bump runc to 2441732d6fcc0fb0a542671a4372e0c7bc99c19e

Also modify an integration test that hardcoded the error string so it
uses the exported error variable from libcontainer/user.

Signed-off-by: Aleksa Sarai <asarai@suse.de>

Aleksa Sarai authored on 2016/03/31 06:25:51
Showing 13 changed files
... ...
@@ -59,7 +59,7 @@ clone git github.com/miekg/pkcs11 df8ae6ca730422dba20c768ff38ef7d79077a59f
59 59
 clone git github.com/docker/go v1.5.1-1-1-gbaf439e
60 60
 clone git github.com/agl/ed25519 d2b94fd789ea21d12fac1a4443dd3a3f79cda72c
61 61
 
62
-clone git github.com/opencontainers/runc 7b6c4c418d5090f4f11eee949fdf49afd15838c9 # libcontainer
62
+clone git github.com/opencontainers/runc 2441732d6fcc0fb0a542671a4372e0c7bc99c19e # libcontainer
63 63
 clone git github.com/opencontainers/specs 3ce138b1934bf227a418e241ead496c383eaba1c # specs
64 64
 clone git github.com/seccomp/libseccomp-golang 1b506fc7c24eec5a3693cdcbed40d9c226cfc6a1
65 65
 # libcontainer deps (see src/github.com/opencontainers/runc/Godeps/Godeps.json)
... ...
@@ -26,6 +26,7 @@ import (
26 26
 	"github.com/docker/libnetwork/netutils"
27 27
 	"github.com/docker/libnetwork/resolvconf"
28 28
 	"github.com/go-check/check"
29
+	libcontainerUser "github.com/opencontainers/runc/libcontainer/user"
29 30
 )
30 31
 
31 32
 // "test123" should be printed by docker run
... ...
@@ -707,7 +708,7 @@ func (s *DockerSuite) TestRunUserByIDBig(c *check.C) {
707 707
 	if err == nil {
708 708
 		c.Fatal("No error, but must be.", out)
709 709
 	}
710
-	if !strings.Contains(out, "Uids and gids must be in range") {
710
+	if !strings.Contains(out, libcontainerUser.ErrRange.Error()) {
711 711
 		c.Fatalf("expected error about uids range, got %s", out)
712 712
 	}
713 713
 }
... ...
@@ -720,7 +721,7 @@ func (s *DockerSuite) TestRunUserByIDNegative(c *check.C) {
720 720
 	if err == nil {
721 721
 		c.Fatal("No error, but must be.", out)
722 722
 	}
723
-	if !strings.Contains(out, "Uids and gids must be in range") {
723
+	if !strings.Contains(out, libcontainerUser.ErrRange.Error()) {
724 724
 		c.Fatalf("expected error about uids range, got %s", out)
725 725
 	}
726 726
 }
... ...
@@ -9,7 +9,7 @@ import (
9 9
 )
10 10
 
11 11
 type Manager interface {
12
-	// Apply cgroup configuration to the process with the specified pid
12
+	// Applies cgroup configuration to the process with the specified pid
13 13
 	Apply(pid int) error
14 14
 
15 15
 	// Returns the PIDs inside the cgroup set
... ...
@@ -47,13 +47,18 @@ type MemoryStats struct {
47 47
 	// usage of memory + swap
48 48
 	SwapUsage MemoryData `json:"swap_usage,omitempty"`
49 49
 	// usage of kernel memory
50
-	KernelUsage MemoryData        `json:"kernel_usage,omitempty"`
51
-	Stats       map[string]uint64 `json:"stats,omitempty"`
50
+	KernelUsage MemoryData `json:"kernel_usage,omitempty"`
51
+	// usage of kernel TCP memory
52
+	KernelTCPUsage MemoryData `json:"kernel_tcp_usage,omitempty"`
53
+
54
+	Stats map[string]uint64 `json:"stats,omitempty"`
52 55
 }
53 56
 
54 57
 type PidsStats struct {
55 58
 	// number of pids in the cgroup
56 59
 	Current uint64 `json:"current,omitempty"`
60
+	// active pids hard limit
61
+	Limit uint64 `json:"limit,omitempty"`
57 62
 }
58 63
 
59 64
 type BlkioStatEntry struct {
... ...
@@ -326,7 +326,7 @@ func RemovePaths(paths map[string]string) (err error) {
326 326
 			return nil
327 327
 		}
328 328
 	}
329
-	return fmt.Errorf("Failed to remove paths: %s", paths)
329
+	return fmt.Errorf("Failed to remove paths: %v", paths)
330 330
 }
331 331
 
332 332
 func GetHugePageSize() ([]string, error) {
... ...
@@ -56,6 +56,9 @@ type Resources struct {
56 56
 	// Kernel memory limit (in bytes)
57 57
 	KernelMemory int64 `json:"kernel_memory"`
58 58
 
59
+	// Kernel memory limit for TCP use (in bytes)
60
+	KernelMemoryTCP int64 `json:"kernel_memory_tcp"`
61
+
59 62
 	// CPU shares (relative weight vs. other containers)
60 63
 	CpuShares int64 `json:"cpu_shares"`
61 64
 
... ...
@@ -3,7 +3,11 @@ package configs
3 3
 import (
4 4
 	"bytes"
5 5
 	"encoding/json"
6
+	"fmt"
6 7
 	"os/exec"
8
+	"time"
9
+
10
+	"github.com/Sirupsen/logrus"
7 11
 )
8 12
 
9 13
 type Rlimit struct {
... ...
@@ -136,7 +140,7 @@ type Config struct {
136 136
 
137 137
 	// Rlimits specifies the resource limits, such as max open files, to set in the container
138 138
 	// If Rlimits are not set, the container will inherit rlimits from the parent process
139
-	Rlimits []Rlimit `json:"rlimits"`
139
+	Rlimits []Rlimit `json:"rlimits,omitempty"`
140 140
 
141 141
 	// OomScoreAdj specifies the adjustment to be made by the kernel when calculating oom scores
142 142
 	// for a process. Valid values are between the range [-1000, '1000'], where processes with
... ...
@@ -175,8 +179,8 @@ type Config struct {
175 175
 	NoNewPrivileges bool `json:"no_new_privileges,omitempty"`
176 176
 
177 177
 	// Hooks are a collection of actions to perform at various container lifecycle events.
178
-	// Hooks are not able to be marshaled to json but they are also not needed to.
179
-	Hooks *Hooks `json:"-"`
178
+	// CommandHooks are serialized to JSON, but other hooks are not.
179
+	Hooks *Hooks
180 180
 
181 181
 	// Version is the version of opencontainer specification that is supported.
182 182
 	Version string `json:"version"`
... ...
@@ -197,6 +201,52 @@ type Hooks struct {
197 197
 	Poststop []Hook
198 198
 }
199 199
 
200
+func (hooks *Hooks) UnmarshalJSON(b []byte) error {
201
+	var state struct {
202
+		Prestart  []CommandHook
203
+		Poststart []CommandHook
204
+		Poststop  []CommandHook
205
+	}
206
+
207
+	if err := json.Unmarshal(b, &state); err != nil {
208
+		return err
209
+	}
210
+
211
+	deserialize := func(shooks []CommandHook) (hooks []Hook) {
212
+		for _, shook := range shooks {
213
+			hooks = append(hooks, shook)
214
+		}
215
+
216
+		return hooks
217
+	}
218
+
219
+	hooks.Prestart = deserialize(state.Prestart)
220
+	hooks.Poststart = deserialize(state.Poststart)
221
+	hooks.Poststop = deserialize(state.Poststop)
222
+	return nil
223
+}
224
+
225
+func (hooks Hooks) MarshalJSON() ([]byte, error) {
226
+	serialize := func(hooks []Hook) (serializableHooks []CommandHook) {
227
+		for _, hook := range hooks {
228
+			switch chook := hook.(type) {
229
+			case CommandHook:
230
+				serializableHooks = append(serializableHooks, chook)
231
+			default:
232
+				logrus.Warnf("cannot serialize hook of type %T, skipping", hook)
233
+			}
234
+		}
235
+
236
+		return serializableHooks
237
+	}
238
+
239
+	return json.Marshal(map[string]interface{}{
240
+		"prestart":  serialize(hooks.Prestart),
241
+		"poststart": serialize(hooks.Poststart),
242
+		"poststop":  serialize(hooks.Poststop),
243
+	})
244
+}
245
+
200 246
 // HookState is the payload provided to a hook on execution.
201 247
 type HookState struct {
202 248
 	Version string `json:"version"`
... ...
@@ -226,10 +276,11 @@ func (f FuncHook) Run(s HookState) error {
226 226
 }
227 227
 
228 228
 type Command struct {
229
-	Path string   `json:"path"`
230
-	Args []string `json:"args"`
231
-	Env  []string `json:"env"`
232
-	Dir  string   `json:"dir"`
229
+	Path    string         `json:"path"`
230
+	Args    []string       `json:"args"`
231
+	Env     []string       `json:"env"`
232
+	Dir     string         `json:"dir"`
233
+	Timeout *time.Duration `json:"timeout"`
233 234
 }
234 235
 
235 236
 // NewCommandHooks will execute the provided command when the hook is run.
... ...
@@ -254,5 +305,19 @@ func (c Command) Run(s HookState) error {
254 254
 		Env:   c.Env,
255 255
 		Stdin: bytes.NewReader(b),
256 256
 	}
257
-	return cmd.Run()
257
+	errC := make(chan error, 1)
258
+	go func() {
259
+		errC <- cmd.Run()
260
+	}()
261
+	if c.Timeout != nil {
262
+		select {
263
+		case err := <-errC:
264
+			return err
265
+		case <-time.After(*c.Timeout):
266
+			cmd.Process.Kill()
267
+			cmd.Wait()
268
+			return fmt.Errorf("hook ran past specified timeout of %.1fs", c.Timeout.Seconds())
269
+		}
270
+	}
271
+	return <-errC
258 272
 }
... ...
@@ -18,7 +18,7 @@ var namespaceInfo = map[NamespaceType]int{
18 18
 }
19 19
 
20 20
 // CloneFlags parses the container's Namespaces options to set the correct
21
-// flags on clone, unshare. This functions returns flags only for new namespaces.
21
+// flags on clone, unshare. This function returns flags only for new namespaces.
22 22
 func (n *Namespaces) CloneFlags() uintptr {
23 23
 	var flag int
24 24
 	for _, v := range *n {
... ...
@@ -8,7 +8,7 @@ func (n *Namespace) Syscall() int {
8 8
 }
9 9
 
10 10
 // CloneFlags parses the container's Namespaces options to set the correct
11
-// flags on clone, unshare. This functions returns flags only for new namespaces.
11
+// flags on clone, unshare. This function returns flags only for new namespaces.
12 12
 func (n *Namespaces) CloneFlags() uintptr {
13 13
 	panic("No namespace syscall support")
14 14
 	return uintptr(0)
... ...
@@ -48,7 +48,7 @@ func UnreserveLabel(label string) error {
48 48
 	return nil
49 49
 }
50 50
 
51
-// DupSecOpt takes an process label and returns security options that
51
+// DupSecOpt takes a process label and returns security options that
52 52
 // can be used to set duplicate labels on future container processes
53 53
 func DupSecOpt(src string) []string {
54 54
 	return nil
... ...
@@ -11,6 +11,19 @@ import (
11 11
 	"unsafe"
12 12
 )
13 13
 
14
+// If arg2 is nonzero, set the "child subreaper" attribute of the
15
+// calling process; if arg2 is zero, unset the attribute.  When a
16
+// process is marked as a child subreaper, all of the children
17
+// that it creates, and their descendants, will be marked as
18
+// having a subreaper.  In effect, a subreaper fulfills the role
19
+// of init(1) for its descendant processes.  Upon termination of
20
+// a process that is orphaned (i.e., its immediate parent has
21
+// already terminated) and marked as having a subreaper, the
22
+// nearest still living ancestor subreaper will receive a SIGCHLD
23
+// signal and be able to wait(2) on the process to discover its
24
+// termination status.
25
+const PR_SET_CHILD_SUBREAPER = 36
26
+
14 27
 type ParentDeathSignal int
15 28
 
16 29
 func (p ParentDeathSignal) Restore() error {
... ...
@@ -40,6 +53,14 @@ func Execv(cmd string, args []string, env []string) error {
40 40
 	return syscall.Exec(name, args, env)
41 41
 }
42 42
 
43
+func Prlimit(pid, resource int, limit syscall.Rlimit) error {
44
+	_, _, err := syscall.RawSyscall6(syscall.SYS_PRLIMIT64, uintptr(pid), uintptr(resource), uintptr(unsafe.Pointer(&limit)), uintptr(unsafe.Pointer(&limit)), 0, 0)
45
+	if err != 0 {
46
+		return err
47
+	}
48
+	return nil
49
+}
50
+
43 51
 func SetParentDeathSignal(sig uintptr) error {
44 52
 	if _, _, err := syscall.RawSyscall(syscall.SYS_PRCTL, syscall.PR_SET_PDEATHSIG, sig, 0); err != 0 {
45 53
 		return err
... ...
@@ -113,6 +134,11 @@ func RunningInUserNS() bool {
113 113
 	return true
114 114
 }
115 115
 
116
+// SetSubreaper sets the value i as the subreaper setting for the calling process
117
+func SetSubreaper(i int) error {
118
+	return Prctl(PR_SET_CHILD_SUBREAPER, uintptr(i), 0, 0, 0)
119
+}
120
+
116 121
 func Prctl(option int, arg2, arg3, arg4, arg5 uintptr) (err error) {
117 122
 	_, _, e1 := syscall.Syscall6(syscall.SYS_PRCTL, uintptr(option), arg2, arg3, arg4, arg5, 0)
118 123
 	if e1 != 0 {
... ...
@@ -2,13 +2,15 @@ package user
2 2
 
3 3
 import (
4 4
 	"errors"
5
-	"fmt"
6 5
 	"syscall"
7 6
 )
8 7
 
9 8
 var (
10 9
 	// The current operating system does not provide the required data for user lookups.
11 10
 	ErrUnsupported = errors.New("user lookup: operating system does not provide passwd-formatted data")
11
+	// No matching entries found in file.
12
+	ErrNoPasswdEntries = errors.New("no matching entries in passwd file")
13
+	ErrNoGroupEntries  = errors.New("no matching entries in group file")
12 14
 )
13 15
 
14 16
 func lookupUser(filter func(u User) bool) (User, error) {
... ...
@@ -27,7 +29,7 @@ func lookupUser(filter func(u User) bool) (User, error) {
27 27
 
28 28
 	// No user entries found.
29 29
 	if len(users) == 0 {
30
-		return User{}, fmt.Errorf("no matching entries in passwd file")
30
+		return User{}, ErrNoPasswdEntries
31 31
 	}
32 32
 
33 33
 	// Assume the first entry is the "correct" one.
... ...
@@ -75,7 +77,7 @@ func lookupGroup(filter func(g Group) bool) (Group, error) {
75 75
 
76 76
 	// No user entries found.
77 77
 	if len(groups) == 0 {
78
-		return Group{}, fmt.Errorf("no matching entries in group file")
78
+		return Group{}, ErrNoGroupEntries
79 79
 	}
80 80
 
81 81
 	// Assume the first entry is the "correct" one.
... ...
@@ -15,7 +15,7 @@ const (
15 15
 )
16 16
 
17 17
 var (
18
-	ErrRange = fmt.Errorf("Uids and gids must be in range %d-%d", minId, maxId)
18
+	ErrRange = fmt.Errorf("uids and gids must be in range %d-%d", minId, maxId)
19 19
 )
20 20
 
21 21
 type User struct {
... ...
@@ -42,29 +42,30 @@ func parseLine(line string, v ...interface{}) {
42 42
 
43 43
 	parts := strings.Split(line, ":")
44 44
 	for i, p := range parts {
45
+		// Ignore cases where we don't have enough fields to populate the arguments.
46
+		// Some configuration files like to misbehave.
45 47
 		if len(v) <= i {
46
-			// if we have more "parts" than we have places to put them, bail for great "tolerance" of naughty configuration files
47 48
 			break
48 49
 		}
49 50
 
51
+		// Use the type of the argument to figure out how to parse it, scanf() style.
52
+		// This is legit.
50 53
 		switch e := v[i].(type) {
51 54
 		case *string:
52
-			// "root", "adm", "/bin/bash"
53 55
 			*e = p
54 56
 		case *int:
55
-			// "0", "4", "1000"
56
-			// ignore string to int conversion errors, for great "tolerance" of naughty configuration files
57
+			// "numbers", with conversion errors ignored because of some misbehaving configuration files.
57 58
 			*e, _ = strconv.Atoi(p)
58 59
 		case *[]string:
59
-			// "", "root", "root,adm,daemon"
60
+			// Comma-separated lists.
60 61
 			if p != "" {
61 62
 				*e = strings.Split(p, ",")
62 63
 			} else {
63 64
 				*e = []string{}
64 65
 			}
65 66
 		default:
66
-			// panic, because this is a programming/logic error, not a runtime one
67
-			panic("parseLine expects only pointers!  argument " + strconv.Itoa(i) + " is not a pointer!")
67
+			// Someone goof'd when writing code using this function. Scream so they can hear us.
68
+			panic(fmt.Sprintf("parseLine only accepts {*string, *int, *[]string} as arguments! %#v is not a pointer!", e))
68 69
 		}
69 70
 	}
70 71
 }
... ...
@@ -106,8 +107,8 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) {
106 106
 			return nil, err
107 107
 		}
108 108
 
109
-		text := strings.TrimSpace(s.Text())
110
-		if text == "" {
109
+		line := strings.TrimSpace(s.Text())
110
+		if line == "" {
111 111
 			continue
112 112
 		}
113 113
 
... ...
@@ -117,10 +118,7 @@ func ParsePasswdFilter(r io.Reader, filter func(User) bool) ([]User, error) {
117 117
 		//  root:x:0:0:root:/root:/bin/bash
118 118
 		//  adm:x:3:4:adm:/var/adm:/bin/false
119 119
 		p := User{}
120
-		parseLine(
121
-			text,
122
-			&p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell,
123
-		)
120
+		parseLine(line, &p.Name, &p.Pass, &p.Uid, &p.Gid, &p.Gecos, &p.Home, &p.Shell)
124 121
 
125 122
 		if filter == nil || filter(p) {
126 123
 			out = append(out, p)
... ...
@@ -135,6 +133,7 @@ func ParseGroupFile(path string) ([]Group, error) {
135 135
 	if err != nil {
136 136
 		return nil, err
137 137
 	}
138
+
138 139
 	defer group.Close()
139 140
 	return ParseGroup(group)
140 141
 }
... ...
@@ -178,10 +177,7 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) {
178 178
 		//  root:x:0:root
179 179
 		//  adm:x:4:root,adm,daemon
180 180
 		p := Group{}
181
-		parseLine(
182
-			text,
183
-			&p.Name, &p.Pass, &p.Gid, &p.List,
184
-		)
181
+		parseLine(text, &p.Name, &p.Pass, &p.Gid, &p.List)
185 182
 
186 183
 		if filter == nil || filter(p) {
187 184
 			out = append(out, p)
... ...
@@ -192,9 +188,10 @@ func ParseGroupFilter(r io.Reader, filter func(Group) bool) ([]Group, error) {
192 192
 }
193 193
 
194 194
 type ExecUser struct {
195
-	Uid, Gid int
196
-	Sgids    []int
197
-	Home     string
195
+	Uid   int
196
+	Gid   int
197
+	Sgids []int
198
+	Home  string
198 199
 }
199 200
 
200 201
 // GetExecUserPath is a wrapper for GetExecUser. It reads data from each of the
... ...
@@ -235,12 +232,12 @@ func GetExecUserPath(userSpec string, defaults *ExecUser, passwdPath, groupPath
235 235
 //     * "uid:gid
236 236
 //     * "user:gid"
237 237
 //     * "uid:group"
238
+//
239
+// It should be noted that if you specify a numeric user or group id, they will
240
+// not be evaluated as usernames (only the metadata will be filled). So attempting
241
+// to parse a user with user.Name = "1337" will produce the user with a UID of
242
+// 1337.
238 243
 func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) (*ExecUser, error) {
239
-	var (
240
-		userArg, groupArg string
241
-		name              string
242
-	)
243
-
244 244
 	if defaults == nil {
245 245
 		defaults = new(ExecUser)
246 246
 	}
... ...
@@ -258,87 +255,113 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) (
258 258
 		user.Sgids = []int{}
259 259
 	}
260 260
 
261
-	// allow for userArg to have either "user" syntax, or optionally "user:group" syntax
261
+	// Allow for userArg to have either "user" syntax, or optionally "user:group" syntax
262
+	var userArg, groupArg string
262 263
 	parseLine(userSpec, &userArg, &groupArg)
263 264
 
265
+	// Convert userArg and groupArg to be numeric, so we don't have to execute
266
+	// Atoi *twice* for each iteration over lines.
267
+	uidArg, uidErr := strconv.Atoi(userArg)
268
+	gidArg, gidErr := strconv.Atoi(groupArg)
269
+
270
+	// Find the matching user.
264 271
 	users, err := ParsePasswdFilter(passwd, func(u User) bool {
265 272
 		if userArg == "" {
273
+			// Default to current state of the user.
266 274
 			return u.Uid == user.Uid
267 275
 		}
268
-		return u.Name == userArg || strconv.Itoa(u.Uid) == userArg
276
+
277
+		if uidErr == nil {
278
+			// If the userArg is numeric, always treat it as a UID.
279
+			return uidArg == u.Uid
280
+		}
281
+
282
+		return u.Name == userArg
269 283
 	})
284
+
285
+	// If we can't find the user, we have to bail.
270 286
 	if err != nil && passwd != nil {
271 287
 		if userArg == "" {
272 288
 			userArg = strconv.Itoa(user.Uid)
273 289
 		}
274
-		return nil, fmt.Errorf("Unable to find user %v: %v", userArg, err)
290
+		return nil, fmt.Errorf("unable to find user %s: %v", userArg, err)
275 291
 	}
276 292
 
277
-	haveUser := users != nil && len(users) > 0
278
-	if haveUser {
279
-		// if we found any user entries that matched our filter, let's take the first one as "correct"
280
-		name = users[0].Name
293
+	var matchedUserName string
294
+	if len(users) > 0 {
295
+		// First match wins, even if there's more than one matching entry.
296
+		matchedUserName = users[0].Name
281 297
 		user.Uid = users[0].Uid
282 298
 		user.Gid = users[0].Gid
283 299
 		user.Home = users[0].Home
284 300
 	} else if userArg != "" {
285
-		// we asked for a user but didn't find them...  let's check to see if we wanted a numeric user
286
-		user.Uid, err = strconv.Atoi(userArg)
287
-		if err != nil {
288
-			// not numeric - we have to bail
289
-			return nil, fmt.Errorf("Unable to find user %v", userArg)
301
+		// If we can't find a user with the given username, the only other valid
302
+		// option is if it's a numeric username with no associated entry in passwd.
303
+
304
+		if uidErr != nil {
305
+			// Not numeric.
306
+			return nil, fmt.Errorf("unable to find user %s: %v", userArg, ErrNoPasswdEntries)
290 307
 		}
308
+		user.Uid = uidArg
291 309
 
292 310
 		// Must be inside valid uid range.
293 311
 		if user.Uid < minId || user.Uid > maxId {
294 312
 			return nil, ErrRange
295 313
 		}
296 314
 
297
-		// if userArg couldn't be found in /etc/passwd but is numeric, just roll with it - this is legit
315
+		// Okay, so it's numeric. We can just roll with this.
298 316
 	}
299 317
 
300
-	if groupArg != "" || name != "" {
318
+	// On to the groups. If we matched a username, we need to do this because of
319
+	// the supplementary group IDs.
320
+	if groupArg != "" || matchedUserName != "" {
301 321
 		groups, err := ParseGroupFilter(group, func(g Group) bool {
302
-			// Explicit group format takes precedence.
303
-			if groupArg != "" {
304
-				return g.Name == groupArg || strconv.Itoa(g.Gid) == groupArg
322
+			// If the group argument isn't explicit, we'll just search for it.
323
+			if groupArg == "" {
324
+				// Check if user is a member of this group.
325
+				for _, u := range g.List {
326
+					if u == matchedUserName {
327
+						return true
328
+					}
329
+				}
330
+				return false
305 331
 			}
306 332
 
307
-			// Check if user is a member.
308
-			for _, u := range g.List {
309
-				if u == name {
310
-					return true
311
-				}
333
+			if gidErr == nil {
334
+				// If the groupArg is numeric, always treat it as a GID.
335
+				return gidArg == g.Gid
312 336
 			}
313 337
 
314
-			return false
338
+			return g.Name == groupArg
315 339
 		})
316 340
 		if err != nil && group != nil {
317
-			return nil, fmt.Errorf("Unable to find groups for user %v: %v", users[0].Name, err)
341
+			return nil, fmt.Errorf("unable to find groups for spec %v: %v", matchedUserName, err)
318 342
 		}
319 343
 
320
-		haveGroup := groups != nil && len(groups) > 0
344
+		// Only start modifying user.Gid if it is in explicit form.
321 345
 		if groupArg != "" {
322
-			if haveGroup {
323
-				// if we found any group entries that matched our filter, let's take the first one as "correct"
346
+			if len(groups) > 0 {
347
+				// First match wins, even if there's more than one matching entry.
324 348
 				user.Gid = groups[0].Gid
325
-			} else {
326
-				// we asked for a group but didn't find id...  let's check to see if we wanted a numeric group
327
-				user.Gid, err = strconv.Atoi(groupArg)
328
-				if err != nil {
329
-					// not numeric - we have to bail
330
-					return nil, fmt.Errorf("Unable to find group %v", groupArg)
349
+			} else if groupArg != "" {
350
+				// If we can't find a group with the given name, the only other valid
351
+				// option is if it's a numeric group name with no associated entry in group.
352
+
353
+				if gidErr != nil {
354
+					// Not numeric.
355
+					return nil, fmt.Errorf("unable to find group %s: %v", groupArg, ErrNoGroupEntries)
331 356
 				}
357
+				user.Gid = gidArg
332 358
 
333
-				// Ensure gid is inside gid range.
359
+				// Must be inside valid gid range.
334 360
 				if user.Gid < minId || user.Gid > maxId {
335 361
 					return nil, ErrRange
336 362
 				}
337 363
 
338
-				// if groupArg couldn't be found in /etc/group but is numeric, just roll with it - this is legit
364
+				// Okay, so it's numeric. We can just roll with this.
339 365
 			}
340
-		} else if haveGroup {
341
-			// If implicit group format, fill supplementary gids.
366
+		} else if len(groups) > 0 {
367
+			// Supplementary group ids only make sense if in the implicit form.
342 368
 			user.Sgids = make([]int, len(groups))
343 369
 			for i, group := range groups {
344 370
 				user.Sgids[i] = group.Gid