Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(osutil.Mkdir): add the chmod flag #423

Merged
merged 4 commits into from
Jun 7, 2024
Merged

feat(osutil.Mkdir): add the chmod flag #423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a7e0bb4
feat(osutil): add chmod flag to mkdir
IronCore864 Jun 6, 2024
149f65f
test(osutil): add ut for mkdir chmod
IronCore864 Jun 6, 2024
a7d8805
chore: fix bug
IronCore864 Jun 6, 2024
4bdb0e4
refactor(daemon): api files and tests use chmod flag when mkdir
IronCore864 Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^filesSuite\.TestWriteUserGroupReal$
PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./daemon.test -check.v -check.f ^filesSuite\.TestMakeDirsUserGroupReal$
go test -c ./internals/osutil
PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./osutil.test -check.v -check.f ^mkdirSuite\.TestMakeParentsAndChown$
PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./osutil.test -check.v -check.f ^mkdirSuite\.TestMakeParentsChmodAndChown$

go test -c ./internals/overlord/servstate/
PEBBLE_TEST_USER=runner PEBBLE_TEST_GROUP=runner sudo -E -H ./servstate.test -check.v -check.f ^S.TestUserGroup$
Expand Down
7 changes: 6 additions & 1 deletion internals/daemon/api_files.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ func mkdirAllUserGroup(path string, perm os.FileMode, uid, gid *int) error {
return mkdir(path, perm, &osutil.MkdirOptions{
MakeParents: true,
ExistOK: true,
Chmod: true,
Chown: true,
UserID: sys.UserID(*uid),
GroupID: sys.GroupID(*gid),
Expand All @@ -512,19 +513,23 @@ func mkdirAllUserGroup(path string, perm os.FileMode, uid, gid *int) error {
return mkdir(path, perm, &osutil.MkdirOptions{
MakeParents: true,
ExistOK: true,
Chmod: true,
})
}
}

func mkdirUserGroup(path string, perm os.FileMode, uid, gid *int) error {
if uid != nil && gid != nil {
return mkdir(path, perm, &osutil.MkdirOptions{
Chmod: true,
Chown: true,
UserID: sys.UserID(*uid),
GroupID: sys.GroupID(*gid),
})
} else {
return mkdir(path, perm, nil)
return mkdir(path, perm, &osutil.MkdirOptions{
Chmod: true,
})
}
}

Expand Down
14 changes: 7 additions & 7 deletions internals/daemon/api_files_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,11 +480,11 @@ func (s *filesSuite) TestMakeDirsUserGroupMocked(c *C) {
tmpDir := s.testMakeDirsUserGroup(c, 12, 34, "USER", "GROUP")

c.Assert(mkdirCalls, HasLen, 5)
c.Check(mkdirCalls[0], Equals, args{tmpDir + "/normal", 0o755, osutil.MkdirOptions{}})
c.Check(mkdirCalls[1], Equals, args{tmpDir + "/uid-gid", 0o755, osutil.MkdirOptions{Chown: true, UserID: 12, GroupID: 34}})
c.Check(mkdirCalls[2], Equals, args{tmpDir + "/user-group", 0o755, osutil.MkdirOptions{Chown: true, UserID: 56, GroupID: 78}})
c.Check(mkdirCalls[3], Equals, args{tmpDir + "/nested1/normal", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true}})
c.Check(mkdirCalls[4], Equals, args{tmpDir + "/nested2/user-group", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chown: true, UserID: 56, GroupID: 78}})
c.Check(mkdirCalls[0], Equals, args{tmpDir + "/normal", 0o755, osutil.MkdirOptions{Chmod: true}})
c.Check(mkdirCalls[1], Equals, args{tmpDir + "/uid-gid", 0o755, osutil.MkdirOptions{Chmod: true, Chown: true, UserID: 12, GroupID: 34}})
c.Check(mkdirCalls[2], Equals, args{tmpDir + "/user-group", 0o755, osutil.MkdirOptions{Chmod: true, Chown: true, UserID: 56, GroupID: 78}})
c.Check(mkdirCalls[3], Equals, args{tmpDir + "/nested1/normal", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chmod: true}})
c.Check(mkdirCalls[4], Equals, args{tmpDir + "/nested2/user-group", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chmod: true, Chown: true, UserID: 56, GroupID: 78}})
}

func (s *filesSuite) testMakeDirsUserGroup(c *C, uid, gid int, user, group string) string {
Expand Down Expand Up @@ -984,8 +984,8 @@ func (s *filesSuite) TestWriteUserGroupMocked(c *C) {
c.Check(atomicWriteChownCalls[4], Equals, args{tmpDir + "/nested2/user-group", 0o644, 56, 78})

c.Assert(mkdirCalls, HasLen, 2)
c.Check(mkdirCalls[0], Equals, mkdirArgs{tmpDir + "/nested1", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true}})
c.Check(mkdirCalls[1], Equals, mkdirArgs{tmpDir + "/nested2", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chown: true, UserID: 56, GroupID: 78}})
c.Check(mkdirCalls[0], Equals, mkdirArgs{tmpDir + "/nested1", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chmod: true}})
c.Check(mkdirCalls[1], Equals, mkdirArgs{tmpDir + "/nested2", 0o755, osutil.MkdirOptions{MakeParents: true, ExistOK: true, Chmod: true, Chown: true, UserID: 56, GroupID: 78}})
}

// See .github/workflows/tests.yml for how to run this test as root.
Expand Down
14 changes: 13 additions & 1 deletion internals/osutil/mkdir.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ type MkdirOptions struct {
// it's not a directory).
ExistOK bool

// If false (the default), no explicit chmod is performed. In this case, the permission
// of the created directories will be affected by umask settings.
//
// If true, perform an explicit chmod on any directories created.
Chmod bool

// If false (the default), no explicit chown is performed.
// If true, perform an explicit chown on any directories created, using the UserID
// and GroupID provided.
Expand Down Expand Up @@ -119,7 +125,7 @@ func mkdirAll(path string, perm os.FileMode, options *MkdirOptions) error {
return mkdir(path, perm, options)
}

// Create a single directory and perform chown operations according to options.
// Create a single directory and perform chown/chmod operations according to options.
func mkdir(path string, perm os.FileMode, options *MkdirOptions) error {
cand := path + ".mkdir-new"

Expand All @@ -133,6 +139,12 @@ func mkdir(path string, perm os.FileMode, options *MkdirOptions) error {
}
}

if options.Chmod {
if err := os.Chmod(cand, perm); err != nil {
return err
}
}

if err := os.Rename(cand, path); err != nil {
return err
}
Expand Down
81 changes: 79 additions & 2 deletions internals/osutil/mkdir_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type mkdirSuite struct{}

var _ = check.Suite(&mkdirSuite{})

// Chown requires root, so it's not tested, only test MakeParents, ExistOK,
// Chown requires root, so it's not tested, only test MakeParents, ExistOK, Chmod,
// and the combination of them.
func (mkdirSuite) TestSimpleDir(c *check.C) {
tmpDir := c.MkDir()
Expand Down Expand Up @@ -112,8 +112,82 @@ func (mkdirSuite) TestMakeParentsAndExistNotOK(c *check.C) {
c.Assert(err, check.ErrorMatches, `.*: file exists`)
}

func (mkdirSuite) TestChmod(c *check.C) {
oldmask := syscall.Umask(0022)
IronCore864 marked this conversation as resolved.
Show resolved Hide resolved
defer syscall.Umask(oldmask)

tmpDir := c.MkDir()

err := osutil.Mkdir(tmpDir+"/foo", 0o777, &osutil.MkdirOptions{
Chmod: true,
})
c.Assert(err, check.IsNil)
c.Assert(osutil.IsDir(tmpDir+"/foo"), check.Equals, true)

info, err := os.Stat(tmpDir + "/foo")
c.Assert(err, check.IsNil)
c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o777))
}

func (mkdirSuite) TestNoChmod(c *check.C) {
oldmask := syscall.Umask(0022)
defer syscall.Umask(oldmask)

tmpDir := c.MkDir()

err := osutil.Mkdir(tmpDir+"/foo", 0o777, nil)
c.Assert(err, check.IsNil)
c.Assert(osutil.IsDir(tmpDir+"/foo"), check.Equals, true)

info, err := os.Stat(tmpDir + "/foo")
c.Assert(err, check.IsNil)
c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o755))
}

func (mkdirSuite) TestMakeParentsAndChmod(c *check.C) {
tmpDir := c.MkDir()

err := osutil.Mkdir(tmpDir+"/foo/bar", 0o777, &osutil.MkdirOptions{
MakeParents: true,
Chmod: true,
})
c.Assert(err, check.IsNil)
c.Assert(osutil.IsDir(tmpDir+"/foo"), check.Equals, true)
c.Assert(osutil.IsDir(tmpDir+"/foo/bar"), check.Equals, true)

info, err := os.Stat(tmpDir + "/foo")
c.Assert(err, check.IsNil)
c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o777))

info, err = os.Stat(tmpDir + "/foo/bar")
c.Assert(err, check.IsNil)
c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o777))
}

func (mkdirSuite) TestMakeParentsAndNoChmod(c *check.C) {
oldmask := syscall.Umask(0022)
defer syscall.Umask(oldmask)

tmpDir := c.MkDir()

err := osutil.Mkdir(tmpDir+"/foo/bar", 0o777, &osutil.MkdirOptions{
MakeParents: true,
})
c.Assert(err, check.IsNil)
c.Assert(osutil.IsDir(tmpDir+"/foo"), check.Equals, true)
c.Assert(osutil.IsDir(tmpDir+"/foo/bar"), check.Equals, true)

info, err := os.Stat(tmpDir + "/foo")
c.Assert(err, check.IsNil)
c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o755))

info, err = os.Stat(tmpDir + "/foo/bar")
c.Assert(err, check.IsNil)
c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o755))
}

// See .github/workflows/tests.yml for how to run this test as root.
func (mkdirSuite) TestMakeParentsAndChown(c *check.C) {
func (mkdirSuite) TestMakeParentsChmodAndChown(c *check.C) {
if os.Getuid() != 0 {
c.Skip("requires running as root")
}
Expand All @@ -136,6 +210,7 @@ func (mkdirSuite) TestMakeParentsAndChown(c *check.C) {

err = osutil.Mkdir(tmpDir+"/foo/bar", 0o777, &osutil.MkdirOptions{
MakeParents: true,
Chmod: true,
Chown: true,
UserID: sys.UserID(uid),
GroupID: sys.GroupID(gid),
Expand All @@ -146,13 +221,15 @@ func (mkdirSuite) TestMakeParentsAndChown(c *check.C) {

info, err := os.Stat(tmpDir + "/foo")
c.Assert(err, check.IsNil)
c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o777))
stat, ok := info.Sys().(*syscall.Stat_t)
c.Assert(ok, check.Equals, true)
c.Assert(int(stat.Uid), check.Equals, uid)
c.Assert(int(stat.Gid), check.Equals, gid)

info, err = os.Stat(tmpDir + "/foo/bar")
c.Assert(err, check.IsNil)
c.Assert(info.Mode().Perm(), check.Equals, os.FileMode(0o777))
stat, ok = info.Sys().(*syscall.Stat_t)
c.Assert(ok, check.Equals, true)
c.Assert(int(stat.Uid), check.Equals, uid)
Expand Down
Loading