diff --git a/libcontainer/user/user.go b/libcontainer/user/user.go index 964e31bf..e6375ea4 100644 --- a/libcontainer/user/user.go +++ b/libcontainer/user/user.go @@ -349,21 +349,26 @@ func GetExecUser(userSpec string, defaults *ExecUser, passwd, group io.Reader) ( return user, nil } -// GetAdditionalGroups looks up a list of groups by name or group id against -// against the given /etc/group formatted data. If a group name cannot be found, -// an error will be returned. If a group id cannot be found, it will be returned -// as-is. +// GetAdditionalGroups looks up a list of groups by name or group id +// against the given /etc/group formatted data. If a group name cannot +// be found, an error will be returned. If a group id cannot be found, +// or the given group data is nil, the id will be returned as-is +// provided it is in the legal range. func GetAdditionalGroups(additionalGroups []string, group io.Reader) ([]int, error) { - groups, err := ParseGroupFilter(group, func(g Group) bool { - for _, ag := range additionalGroups { - if g.Name == ag || strconv.Itoa(g.Gid) == ag { - return true + var groups = []Group{} + if group != nil { + var err error + groups, err = ParseGroupFilter(group, func(g Group) bool { + for _, ag := range additionalGroups { + if g.Name == ag || strconv.Itoa(g.Gid) == ag { + return true + } } + return false + }) + if err != nil { + return nil, fmt.Errorf("Unable to find additional groups %v: %v", additionalGroups, err) } - return false - }) - if err != nil { - return nil, fmt.Errorf("Unable to find additional groups %v: %v", additionalGroups, err) } gidMap := make(map[int]struct{}) @@ -401,13 +406,13 @@ func GetAdditionalGroups(additionalGroups []string, group io.Reader) ([]int, err return gids, nil } -// Wrapper around GetAdditionalGroups that opens the groupPath given and gives -// it as an argument to GetAdditionalGroups. +// GetAdditionalGroupsPath is a wrapper around GetAdditionalGroups +// that opens the groupPath given and gives it as an argument to +// GetAdditionalGroups. func GetAdditionalGroupsPath(additionalGroups []string, groupPath string) ([]int, error) { group, err := os.Open(groupPath) - if err != nil { - return nil, fmt.Errorf("Failed to open group file: %v", err) + if err == nil { + defer group.Close() } - defer group.Close() return GetAdditionalGroups(additionalGroups, group) } diff --git a/libcontainer/user/user_test.go b/libcontainer/user/user_test.go index 0e37ac3d..53b2289b 100644 --- a/libcontainer/user/user_test.go +++ b/libcontainer/user/user_test.go @@ -434,3 +434,39 @@ this is just some garbage data } } } + +func TestGetAdditionalGroupsNumeric(t *testing.T) { + tests := []struct { + groups []string + expected []int + hasError bool + }{ + { + // numeric groups only + groups: []string{"1234", "5678"}, + expected: []int{1234, 5678}, + }, + { + // numeric and alphabetic + groups: []string{"1234", "fake"}, + expected: nil, + hasError: true, + }, + } + + for _, test := range tests { + gids, err := GetAdditionalGroups(test.groups, nil) + if test.hasError && err == nil { + t.Errorf("Parse(%#v) expects error but has none", test) + continue + } + if !test.hasError && err != nil { + t.Errorf("Parse(%#v) has error %v", test, err) + continue + } + sort.Sort(sort.IntSlice(gids)) + if !reflect.DeepEqual(gids, test.expected) { + t.Errorf("Gids(%v), expect %v from groups %v", gids, test.expected, test.groups) + } + } +}