Skip to content

Commit

Permalink
feat: support uid only form in NormalizeUidGid (#553)
Browse files Browse the repository at this point in the history
This makes it valid to supply just the uid for operations setting file
ownership, like mkdir and push.
  • Loading branch information
james-garner-canonical authored Jan 10, 2025
1 parent 004d32d commit 05ea5bc
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 9 deletions.
6 changes: 6 additions & 0 deletions internals/osutil/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ func FakeUserLookup(f func(name string) (*user.User, error)) func() {
return func() { userLookup = oldUserLookup }
}

func FakeUserLookupId(f func(name string) (*user.User, error)) func() {
oldUserLookupId := userLookupId
userLookupId = f
return func() { userLookupId = oldUserLookupId }
}

func FakeUserLookupGroup(f func(name string) (*user.Group, error)) func() {
oldUserLookupGroup := userLookupGroup
userLookupGroup = f
Expand Down
23 changes: 15 additions & 8 deletions internals/osutil/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
var (
userCurrent = user.Current
userLookup = user.Lookup
userLookupId = user.LookupId
userLookupGroup = user.LookupGroup

enoentMessage = syscall.ENOENT.Error()
Expand Down Expand Up @@ -112,11 +113,6 @@ func NormalizeUidGid(uid, gid *int, username, group string) (*int, *int, error)
username, n, *uid)
}
uid = &n
if gid == nil && group == "" {
// Group not specified; use user's primary group ID
gidVal, _ := strconv.Atoi(u.Gid)
gid = &gidVal
}
}
if group != "" {
g, err := userLookupGroup(group)
Expand All @@ -134,12 +130,23 @@ func NormalizeUidGid(uid, gid *int, username, group string) (*int, *int, error)
}
gid = &n
}
if gid == nil {
// Neither gid nor group was specified
// Either uid or user must have been specified; use user's primary group ID
uidInfo, err := userLookupId(strconv.Itoa(*uid))
if err != nil {
if strings.Contains(err.Error(), enoentMessage) {
// Better error message to work around https://github.com/golang/go/issues/67912
return nil, nil, user.UnknownUserIdError(*uid)
}
return nil, nil, err
}
gidVal, _ := strconv.Atoi(uidInfo.Gid)
gid = &gidVal
}
if uid == nil && gid != nil {
return nil, nil, fmt.Errorf("must specify user, not just group")
}
if uid != nil && gid == nil {
return nil, nil, fmt.Errorf("must specify group, not just UID")
}
return uid, gid, nil
}

Expand Down
9 changes: 8 additions & 1 deletion internals/osutil/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,13 @@ func (s *userSuite) TestNormalizeUidGid(c *check.C) {
})
defer restoreUser()

var userIdErr error
restoreUserId := osutil.FakeUserLookupId(func(uid string) (*user.User, error) {
c.Check(uid, check.Equals, "10")
return &user.User{Uid: "10", Gid: "20"}, userIdErr
})
defer restoreUserId()

var groupErr error
restoreGroup := osutil.FakeUserLookupGroup(func(name string) (*user.Group, error) {
c.Check(name, check.Equals, "GROUP")
Expand All @@ -127,14 +134,14 @@ func (s *userSuite) TestNormalizeUidGid(c *check.C) {
test(nil, nil, "", "", nil, nil, "")
test(nil, nil, "", "GROUP", nil, nil, "must specify user, not just group")
test(nil, nil, "USER", "", ptr(10), ptr(20), "")
test(ptr(10), nil, "", "", ptr(10), ptr(20), "")
test(nil, nil, "USER", "GROUP", ptr(10), ptr(30), "")

test(nil, ptr(2), "", "", nil, nil, "must specify user, not just group")
test(nil, ptr(2), "", "GROUP", nil, nil, `group "GROUP" GID \(30\) does not match group-id \(2\)`)
test(nil, ptr(2), "USER", "", ptr(10), ptr(2), "")
test(nil, ptr(2), "USER", "GROUP", nil, nil, `group "GROUP" GID \(30\) does not match group-id \(2\)`)

test(ptr(1), nil, "", "", nil, nil, "must specify group, not just UID")
test(ptr(1), nil, "", "GROUP", ptr(1), ptr(30), "")
test(ptr(1), nil, "USER", "", nil, nil, `user "USER" UID \(10\) does not match user-id \(1\)`)
test(ptr(1), nil, "USER", "GROUP", nil, nil, `user "USER" UID \(10\) does not match user-id \(1\)`)
Expand Down

0 comments on commit 05ea5bc

Please sign in to comment.