Skip to content

Commit

Permalink
github: test snapdusergo tag (#14550)
Browse files Browse the repository at this point in the history
* github: test snapdusergo tag

* osutil/user: cover cases of missing group/user

* osutil/user: fix error type when missing user/group

* osutil: fix tests when using snapdusergo

* osutil/user: test with more realistic getent exit codes

* osutil/strace: fix tests when built with snapdusergo
  • Loading branch information
valentindavid authored Oct 1, 2024
1 parent 7446f59 commit 0d7c868
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 38 deletions.
7 changes: 7 additions & 0 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ jobs:
- nosecboot
- faultinject
- race
- snapdusergo

steps:
- name: Checkout code
Expand Down Expand Up @@ -491,6 +492,12 @@ jobs:
cd ${{ github.workspace }}/src/github.com/snapcore/snapd || exit 1
SKIP_DIRTY_CHECK=1 GO_TEST_RACE=1 SKIP_COVERAGE=1 ./run-checks --unit
- name: Test Go (snapdusergo)
if: ${{ matrix.unit-scenario == 'snapdusergo' }}
run: |
cd ${{ github.workspace }}/src/github.com/snapcore/snapd || exit 1
SKIP_DIRTY_CHECK=1 GO_BUILD_TAGS=snapdusergo ./run-checks --unit
- name: Upload the coverage results
if: ${{ matrix.gochannel != 'latest/stable' && matrix.unit-scenario != 'race' }}
uses: actions/upload-artifact@v4
Expand Down
114 changes: 86 additions & 28 deletions osutil/group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,21 @@ var _ = check.Suite(&findUserGroupSuite{})

func (s *findUserGroupSuite) SetUpTest(c *check.C) {
// exit 2 is not found
s.mockGetent = testutil.MockCommand(c, "getent", "exit 2")
if user.GetentBased {
s.mockGetent = testutil.MockCommand(c, "getent", `
if [ "${1}" == "passwd" ] && [ "${2}" == "root" ]; then
echo 'root:x:0:0:root:/root:/bin/bash'
exit 0
fi
if [ "${1}" == "group" ] && [ "${2}" == "root" ]; then
echo 'root:x:0:'
exit 0
fi
exit 2`)
} else {
s.mockGetent = testutil.MockCommand(c, "getent", `
exit 2`)
}
}

func (s *findUserGroupSuite) TearDownTest(c *check.C) {
Expand All @@ -49,36 +63,53 @@ func (s *findUserGroupSuite) TestFindUidNoGetentFallback(c *check.C) {
uid, err := osutil.FindUidNoGetentFallback("root")
c.Assert(err, check.IsNil)
c.Assert(uid, check.Equals, uint64(0))
// getent shouldn't have been called with FindUidNoGetentFallback()
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string(nil))
if !user.GetentBased {
// getent shouldn't have been called with FindUidNoGetentFallback()
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string(nil))
}
}

func (s *findUserGroupSuite) TestFindUidNonexistent(c *check.C) {
_, err := osutil.FindUidNoGetentFallback("lakatos")
c.Assert(err, check.ErrorMatches, "user: unknown user lakatos")
_, ok := err.(user.UnknownUserError)
c.Assert(ok, check.Equals, true)
// getent shouldn't have been called with FindUidNoGetentFallback()
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string(nil))
if user.GetentBased {
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "passwd", "lakatos"},
})
} else {
// getent shouldn't have been called with FindUidNoGetentFallback()
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string(nil))
}
}

func (s *findUserGroupSuite) TestFindUidWithGetentFallback(c *check.C) {
uid, err := osutil.FindUidWithGetentFallback("root")
c.Assert(err, check.IsNil)
c.Assert(uid, check.Equals, uint64(0))
// getent shouldn't have been called since 'root' is in /etc/passwd
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string(nil))
if !user.GetentBased {
// getent shouldn't have been called since 'root' is in /etc/passwd
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string(nil))
}
}

func (s *findUserGroupSuite) TestFindUidGetentNonexistent(c *check.C) {
_, err := osutil.FindUidWithGetentFallback("lakatos")
c.Assert(err, check.ErrorMatches, "user: unknown user lakatos")
_, ok := err.(user.UnknownUserError)
c.Assert(ok, check.Equals, true)
// getent should've have been called
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "passwd", "lakatos"},
})
if user.GetentBased {
// getent should've have been called
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "passwd", "lakatos"},
{"getent", "passwd", "lakatos"},
})
} else {
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "passwd", "lakatos"},
})
}
}

func (s *findUserGroupSuite) TestFindUidGetentFoundFromGetent(c *check.C) {
Expand Down Expand Up @@ -110,10 +141,18 @@ func (s *findUserGroupSuite) TestFindUidGetentMockedOtherError(c *check.C) {
uid, err := osutil.FindUidWithGetentFallback("lakatos")
c.Assert(err, check.ErrorMatches, "getent failed with: exit status 3")
c.Check(uid, check.Equals, uint64(0))
// getent should've have been called
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "passwd", "lakatos"},
})
if user.GetentBased {
// getent should've have been called
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "passwd", "lakatos"},
{"getent", "passwd", "lakatos"},
})
} else {
// getent should've have been called
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "passwd", "lakatos"},
})
}
}

func (s *findUserGroupSuite) TestFindUidGetentMocked(c *check.C) {
Expand All @@ -136,10 +175,12 @@ func (s *findUserGroupSuite) TestFindUidGetentMockedMalformated(c *check.C) {

func (s *findUserGroupSuite) TestFindGidNoGetentFallback(c *check.C) {
gid, err := osutil.FindGidNoGetentFallback("root")
c.Assert(err, check.IsNil)
c.Assert(gid, check.Equals, uint64(0))
// getent shouldn't have been called with FindGidNoGetentFallback()
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string(nil))
if !user.GetentBased {
c.Assert(err, check.IsNil)
c.Assert(gid, check.Equals, uint64(0))
// getent shouldn't have been called with FindGidNoGetentFallback()
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string(nil))
}
}

func (s *findUserGroupSuite) TestFindGidNonexistent(c *check.C) {
Expand Down Expand Up @@ -176,19 +217,29 @@ func (s *findUserGroupSuite) TestFindGidWithGetentFallback(c *check.C) {
gid, err := osutil.FindGidWithGetentFallback("root")
c.Assert(err, check.IsNil)
c.Assert(gid, check.Equals, uint64(0))
// getent shouldn't have been called since 'root' is in /etc/group
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string(nil))
if !user.GetentBased {
// getent shouldn't have been called since 'root' is in /etc/group
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string(nil))
}
}

func (s *findUserGroupSuite) TestFindGidGetentNonexistent(c *check.C) {
_, err := osutil.FindGidWithGetentFallback("lakatos")
c.Assert(err, check.ErrorMatches, "group: unknown group lakatos")
_, ok := err.(user.UnknownGroupError)
c.Assert(ok, check.Equals, true)
// getent should've have been called
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "group", "lakatos"},
})
if user.GetentBased {
// getent should've have been called
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "group", "lakatos"},
{"getent", "group", "lakatos"},
})
} else {
// getent should've have been called
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "group", "lakatos"},
})
}
}

func (s *findUserGroupSuite) TestFindGidGetentMockedOtherError(c *check.C) {
Expand All @@ -198,9 +249,16 @@ func (s *findUserGroupSuite) TestFindGidGetentMockedOtherError(c *check.C) {
c.Assert(err, check.ErrorMatches, "getent failed with: exit status 3")
c.Check(gid, check.Equals, uint64(0))
// getent should've have been called
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "group", "lakatos"},
})
if user.GetentBased {
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "group", "lakatos"},
{"getent", "group", "lakatos"},
})
} else {
c.Check(s.mockGetent.Calls(), check.DeepEquals, [][]string{
{"getent", "group", "lakatos"},
})
}
}

func (s *findUserGroupSuite) TestFindGidGetentMocked(c *check.C) {
Expand Down
22 changes: 20 additions & 2 deletions osutil/strace/strace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package strace_test

import (
"os"
"os/exec"
"path/filepath"
"testing"

Expand Down Expand Up @@ -98,19 +99,36 @@ func (s *straceSuite) TestStraceCommandHappyFromSnap(c *C) {
}

func (s *straceSuite) TestStraceCommandNoSudo(c *C) {
tmp := c.MkDir()

if user.GetentBased {
getEntPath, err := exec.LookPath("getent")
c.Assert(err, IsNil)
err = os.Symlink(getEntPath, filepath.Join(tmp, "getent"))
c.Assert(err, IsNil)
}

origPath := os.Getenv("PATH")
defer func() { os.Setenv("PATH", origPath) }()
os.Setenv("PATH", tmp)

os.Setenv("PATH", "/not-exists")
_, err := strace.Command(nil, "foo")
c.Assert(err, ErrorMatches, `cannot use strace without sudo: exec: "sudo": executable file not found in \$PATH`)
}

func (s *straceSuite) TestStraceCommandNoStrace(c *C) {
tmp := c.MkDir()

if user.GetentBased {
getEntPath, err := exec.LookPath("getent")
c.Assert(err, IsNil)
err = os.Symlink(getEntPath, filepath.Join(tmp, "getent"))
c.Assert(err, IsNil)
}

origPath := os.Getenv("PATH")
defer func() { os.Setenv("PATH", origPath) }()

tmp := c.MkDir()
os.Setenv("PATH", tmp)
err := os.WriteFile(filepath.Join(tmp, "sudo"), nil, 0755)
c.Assert(err, IsNil)
Expand Down
3 changes: 3 additions & 0 deletions osutil/user/getent.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ func getEnt(params ...string) ([]byte, error) {
if err != nil {
var exitError *exec.ExitError
if errors.As(err, &exitError) {
if exitError.ExitCode() == 2 {
return nil, nil
}
return nil, fmt.Errorf("getent returned an error: %q", exitError.Stderr)
}
return nil, fmt.Errorf("getent could not be executed: %w", err)
Expand Down
51 changes: 43 additions & 8 deletions osutil/user/getent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"fmt"
"os"
"path/filepath"
"strconv"
"testing"

. "gopkg.in/check.v1"
Expand All @@ -46,12 +47,17 @@ func (s *getentSuite) SetUpTest(c *C) {
s.getentDir = c.MkDir()

s.mockGetent = testutil.MockCommand(c, "getent", fmt.Sprintf(`
cat '%s'/"${1}${2:+/}${2-}"
set -eu
base='%s'/"${1}${2:+/}${2-}"
cat "${base}"
if [ -f "${base}.exit" ]; then
exit "$(cat "${base}.exit")"
fi
`, s.getentDir))
s.AddCleanup(s.mockGetent.Restore)
}

func (s *getentSuite) mockGetentOutput(c *C, value string, params ...string) {
func (s *getentSuite) mockGetentOutput(c *C, value string, exit int, params ...string) {
path := []string{s.getentDir}
path = append(path, params...)
resultPath := filepath.Join(path...)
Expand All @@ -60,11 +66,16 @@ func (s *getentSuite) mockGetentOutput(c *C, value string, params ...string) {
b := []byte(value)
err = os.WriteFile(resultPath, b, 0644)
c.Assert(err, IsNil)
if exit != 0 {
exitBytes := []byte(strconv.Itoa(exit))
err = os.WriteFile(resultPath+".exit", exitBytes, 0644)
c.Assert(err, IsNil)
}
}

func (s *getentSuite) TestLookupGroupByName(c *C) {
s.mockGetentOutput(c, `mygroup:x:60000:myuser,someuser
`, "group", "mygroup")
`, 0, "group", "mygroup")

grp, err := user.LookupGroupFromGetent(user.GroupMatchGroupname("mygroup"))
c.Assert(err, IsNil)
Expand All @@ -79,7 +90,7 @@ func (s *getentSuite) TestLookupGroupByNameError(c *C) {
}

func (s *getentSuite) TestLookupGroupByNameDoesNotExist(c *C) {
s.mockGetentOutput(c, ``, "group", "mygroup")
s.mockGetentOutput(c, ``, 2, "group", "mygroup")

grp, err := user.LookupGroupFromGetent(user.GroupMatchGroupname("mygroup"))
c.Assert(err, IsNil)
Expand All @@ -90,7 +101,7 @@ func (s *getentSuite) TestLookupGroupByNumericalName(c *C) {
// This is probably not valid
s.mockGetentOutput(c, `mygroup:x:60001:myuser,someuser
1mygroup:x:60000:myuser,someuser
`, "group")
`, 0, "group")

grp, err := user.LookupGroupFromGetent(user.GroupMatchGroupname("1mygroup"))
c.Assert(err, IsNil)
Expand All @@ -101,7 +112,7 @@ func (s *getentSuite) TestLookupGroupByNumericalName(c *C) {

func (s *getentSuite) TestLookupUserByName(c *C) {
s.mockGetentOutput(c, `johndoe:x:60000:60000:John Doe:/home/johndoe:/bin/bash
`, "passwd", "johndoe")
`, 0, "passwd", "johndoe")

usr, err := user.LookupUserFromGetent(user.UserMatchUsername("johndoe"))
c.Assert(err, IsNil)
Expand All @@ -115,7 +126,7 @@ func (s *getentSuite) TestLookupUserByName(c *C) {

func (s *getentSuite) TestLookupUserByUid(c *C) {
s.mockGetentOutput(c, `johndoe:x:60000:60000:John Doe:/home/johndoe:/bin/bash
`, "passwd", "60000")
`, 0, "passwd", "60000")

usr, err := user.LookupUserFromGetent(user.UserMatchUid(60000))
c.Assert(err, IsNil)
Expand All @@ -131,7 +142,7 @@ func (s *getentSuite) TestLookupUserByNumericalName(c *C) {
// This is probably not valid
s.mockGetentOutput(c, `johndoe:x:60001:60001:John Doe2:/home/johndoe2:/bin/bash
1johndoe:x:60000:60000:John Doe:/home/johndoe:/bin/bash
`, "passwd")
`, 0, "passwd")

usr, err := user.LookupUserFromGetent(user.UserMatchUsername("1johndoe"))
c.Assert(err, IsNil)
Expand All @@ -142,3 +153,27 @@ func (s *getentSuite) TestLookupUserByNumericalName(c *C) {
c.Check(usr.Name, Equals, "John Doe")
c.Check(usr.HomeDir, Equals, "/home/johndoe")
}

func (s *getentSuite) TestLookupUserByNameMissing(c *C) {
s.mockGetentOutput(c, ``, 2, "passwd", "johndoe")

usr, err := user.LookupUserFromGetent(user.UserMatchUsername("johndoe"))
c.Assert(err, IsNil)
c.Assert(usr, IsNil)
}

func (s *getentSuite) TestLookupUserUidMissing(c *C) {
s.mockGetentOutput(c, ``, 2, "passwd", "60000")

usr, err := user.LookupUserFromGetent(user.UserMatchUid(60000))
c.Assert(err, IsNil)
c.Assert(usr, IsNil)
}

func (s *getentSuite) TestLookupGroupByNameMissing(c *C) {
s.mockGetentOutput(c, ``, 2, "group", "mygroup")

grp, err := user.LookupGroupFromGetent(user.GroupMatchGroupname("mygroup"))
c.Assert(err, IsNil)
c.Assert(grp, IsNil)
}
2 changes: 2 additions & 0 deletions osutil/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ type (
UnknownGroupError = osuser.UnknownGroupError
)

const GetentBased = false

// Current returns the current user
//
// This is a wrapper for (os/user).Current
Expand Down
2 changes: 2 additions & 0 deletions osutil/user/user_from_snap.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ type (
UnknownGroupError = osuser.UnknownGroupError
)

const GetentBased = true

// Current returns the current user
func Current() (*User, error) {
u, err := lookupUserFromGetent(userMatchUid(os.Getuid()))
Expand Down

0 comments on commit 0d7c868

Please sign in to comment.