Skip to content

Commit ea96d78

Browse files
authored
Merge pull request #6591 from taskcluster/issue6590
Generic Worker: fix osGroups feature on Linux
2 parents 10e1af1 + 38956f2 commit ea96d78

File tree

7 files changed

+66
-33
lines changed

7 files changed

+66
-33
lines changed

changelog/issue-6590.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
audience: users
2+
level: patch
3+
reference: issue 6590
4+
---
5+
Generic Worker osGroups feature on Linux has been fixed. It never worked on this platform.

workers/generic-worker/helper_posix_test.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"path/filepath"
88
"strconv"
9+
"strings"
910
)
1011

1112
func helloGoodbye() [][]string {
@@ -154,8 +155,17 @@ func listGroups() [][]string {
154155
return [][]string{
155156
{
156157
"bash",
157-
"-c",
158-
`USER="$(whoami)"; for group in $(id -nG "${USER}"); do echo "*${group}"; done`,
158+
"-ce",
159+
strings.Join(
160+
[]string{
161+
`# make sure listing groups fails if process does not have same permissions as user`,
162+
`[ "$(id -nG)" == "$(id -nG $(whoami))" ]`,
163+
`for group in $(id -nG); do`,
164+
` echo "*${group}"`,
165+
`done`,
166+
},
167+
"\n",
168+
),
159169
},
160170
}
161171
}

workers/generic-worker/os_groups_multiuser.go

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,50 +4,57 @@ package main
44

55
import (
66
"fmt"
7+
"os/user"
78
)
89

910
// one instance per task
1011
type OSGroups struct {
1112
Task *TaskRun
1213
// keep track of which groups we successfully update
13-
AddedGroups []string
14+
AddedGroups []*user.Group
1415
}
1516

1617
func (osGroups *OSGroups) Start() *CommandExecutionError {
17-
groups := osGroups.Task.Payload.OSGroups
18-
if len(groups) == 0 {
18+
groupNames := osGroups.Task.Payload.OSGroups
19+
if len(groupNames) == 0 {
1920
return nil
2021
}
2122
if config.RunTasksAsCurrentUser {
22-
osGroups.Task.Infof("Not adding task user to group(s) %v since we are running as current user.", groups)
23+
osGroups.Task.Infof("Not adding task user to group(s) %v since we are running as current user.", groupNames)
2324
return nil
2425
}
25-
notAddedGroups := []string{}
26-
for _, group := range groups {
27-
err := addUserToGroup(taskContext.User.Name, group)
28-
if err == nil {
29-
osGroups.AddedGroups = append(osGroups.AddedGroups, group)
30-
} else {
31-
notAddedGroups = append(notAddedGroups, group)
32-
osGroups.Task.Errorf("[osGroups] Could not add task user to OS group %v: %v", group, err)
26+
notAddedGroupNames := []string{}
27+
for _, groupName := range groupNames {
28+
err := addUserToGroup(taskContext.User.Name, groupName)
29+
if err != nil {
30+
notAddedGroupNames = append(notAddedGroupNames, groupName)
31+
osGroups.Task.Errorf("[osGroups] Could not add task user to OS group %v: %v", groupName, err)
32+
continue
3333
}
34+
group, err := user.LookupGroup(groupName)
35+
if err != nil {
36+
notAddedGroupNames = append(notAddedGroupNames, groupName)
37+
osGroups.Task.Errorf("[osGroups] Could not look up group ID for OS group %v: %v", groupName, err)
38+
continue
39+
}
40+
osGroups.AddedGroups = append(osGroups.AddedGroups, group)
3441
}
35-
if len(notAddedGroups) > 0 {
36-
return MalformedPayloadError(fmt.Errorf("Could not add task user to OS group(s) %v", notAddedGroups))
42+
if len(notAddedGroupNames) > 0 {
43+
return MalformedPayloadError(fmt.Errorf("Could not add task user to OS group(s) %v", notAddedGroupNames))
3744
}
3845
return osGroups.refreshTaskCommands()
3946
}
4047

4148
func (osGroups *OSGroups) Stop(err *ExecutionErrors) {
42-
notRemovedGroups := []string{}
49+
notRemovedGroupNames := []string{}
4350
for _, group := range osGroups.AddedGroups {
44-
e := removeUserFromGroup(taskContext.User.Name, group)
51+
e := removeUserFromGroup(taskContext.User.Name, group.Name)
4552
if e != nil {
46-
notRemovedGroups = append(notRemovedGroups, group)
53+
notRemovedGroupNames = append(notRemovedGroupNames, group.Name)
4754
osGroups.Task.Errorf("[osGroups] Could not remove task user from OS group %v: %v", group, e)
4855
}
4956
}
50-
if len(notRemovedGroups) > 0 {
51-
err.add(executionError(internalError, errored, fmt.Errorf("Could not remove task user from OS group(s) %v", notRemovedGroups)))
57+
if len(notRemovedGroupNames) > 0 {
58+
err.add(executionError(internalError, errored, fmt.Errorf("Could not remove task user from OS group(s) %v", notRemovedGroupNames)))
5259
}
5360
}

workers/generic-worker/os_groups_multiuser_darwin.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,3 @@ func addUserToGroup(user, group string) error {
1313
func removeUserFromGroup(user, group string) error {
1414
return host.Run("/usr/sbin/dseditgroup", "-o", "edit", "-d", taskContext.User.Name, "-t", "user", group)
1515
}
16-
17-
func (osGroups *OSGroups) refreshTaskCommands() (err *CommandExecutionError) {
18-
return
19-
}

workers/generic-worker/os_groups_multiuser_freebsd.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,3 @@ func removeUserFromGroup(user, group string) error {
1515
// TODO copied from Linux version, need to find out what to do for FreeBSD
1616
return host.Run("/usr/bin/gpasswd", "-d", taskContext.User.Name, group)
1717
}
18-
19-
func (osGroups *OSGroups) refreshTaskCommands() (err *CommandExecutionError) {
20-
return
21-
}

workers/generic-worker/os_groups_multiuser_linux.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,3 @@ func addUserToGroup(user, group string) error {
1313
func removeUserFromGroup(user, group string) error {
1414
return host.Run("/usr/bin/gpasswd", "-d", taskContext.User.Name, group)
1515
}
16-
17-
func (osGroups *OSGroups) refreshTaskCommands() (err *CommandExecutionError) {
18-
return
19-
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//go:build multiuser && (darwin || linux || freebsd)
2+
3+
package main
4+
5+
import (
6+
"fmt"
7+
"strconv"
8+
)
9+
10+
func (osGroups *OSGroups) refreshTaskCommands() (err *CommandExecutionError) {
11+
gids := make([]uint32, len(osGroups.AddedGroups))
12+
for i, group := range osGroups.AddedGroups {
13+
gid, err := strconv.Atoi(group.Gid)
14+
if err != nil {
15+
panic(fmt.Sprintf("Group ID for %q is %q which isn't an int: %v", group.Name, group.Gid, err))
16+
}
17+
gids[i] = uint32(gid)
18+
}
19+
for _, command := range osGroups.Task.Commands {
20+
command.SysProcAttr.Credential.Groups = gids
21+
}
22+
return
23+
}

0 commit comments

Comments
 (0)