integrate github.com/opencontainers/runc/libcontainer/user (alternative) #134
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… implements proper parsing of /etc/passwd and /etc/group, and use that to add support for "docker run -u user:group" and for getting supplementary groups (if ":group" is not specified) Docker-DCO-1.1-Signed-off-by: Andrew Page <[email protected]> (github: tianon)
Docker-DCO-1.1-Signed-off-by: Michael Crosby <[email protected]> (github: crosbymichael)
Fixes #5647 Docker-DCO-1.1-Signed-off-by: Alexandr Morozov <[email protected]> (github: LK4D4)
…plementary to return "Home" too Docker-DCO-1.1-Signed-off-by: Andrew Page <[email protected]> (github: tianon)
This patch refactors most of GetUserGroupSupplementaryHome and its signature, to make using it much simpler. The private parsing ftunctions have also been exposed (parsePasswdFile, parseGroupFile) to allow custom data source to be used (increasing the versatility of the user/ tools). In addition, file path wrappers around the formerly private API functions have been added to make usage of the API for callers easier if the files that are being parsed are on the filesystem (while the io.Reader APIs are exposed for non-traditional usecases). Signed-off-by: Aleksa Sarai <[email protected]> (github: cyphar)
This patch adds an os/user-like user lookup API, implemented in pure Go. It also has some features not present in the standard library implementation (such as group lookups). Signed-off-by: Aleksa Sarai <[email protected]> (github: cyphar)
Signed-off-by: Aleksa Sarai <[email protected]> (github: cyphar)
Signed-off-by: Andrey Vagin <[email protected]>
Signed-off-by: Mrunal Patel <[email protected]>
This parses group file only once to process a list of groups instead of parsing once for each group. Also added an unit test for GetAdditionalGroupsPath Signed-off-by: Daniel, Dao Quang Minh <[email protected]>
The old GetAdditionalGroups* API didn't match the rest of
libcontainer/user, we make functions that take io.Readers and then make
wrappers around them. Otherwise we have to do dodgy stuff when testing
our code.
Fixes: b6df94ee65436 ("refactor GetAdditionalGroupsPath")
Signed-off-by: Aleksa Sarai <[email protected]>
Update the tests to use the test-friendly GetAdditionalGroups API, rather than making random files for no good reason. Signed-off-by: Aleksa Sarai <[email protected]>
/etc/groups is not needed when specifying numeric group ids. This change allows containers without /etc/groups to specify numeric supplemental groups. Signed-off-by: Sami Wagiaalla <[email protected]>
Export errors as variables when no matching entries are found in passwd or group file. Signed-off-by: Thomas LE ROUX <[email protected]>
Most shadow-related tools don't treat numeric ids as potential usernames, so change our behaviour to match that. Previously, using an explicit specification like 111:222 could result in the UID and GID not being 111 and 222 respectively (which is confusing). Signed-off-by: Aleksa Sarai <[email protected]>
Signed-off-by: Aleksa Sarai <[email protected]>
Some of the code was quite confusing inside libcontainer/user, so refactor and comment it so future maintainers can understand what's going and what edge cases we have to deal with. Signed-off-by: Aleksa Sarai <[email protected]>
Fixes: #941 Signed-off-by: Qiang Huang <[email protected]>
Signed-off-by: Lei Jitang <[email protected]>
The parameters passed to `GetExecUser` is not correct.
Consider the following code:
```
package main
import (
"fmt"
"io"
"os"
)
func main() {
passwd, err := os.Open("/etc/passwd1")
if err != nil {
passwd = nil
} else {
defer passwd.Close()
}
err = GetUserPasswd(passwd)
if err != nil {
fmt.Printf("%#v\n", err)
}
}
func GetUserPasswd(r io.Reader) error {
if r == nil {
return fmt.Errorf("nil source for passwd-formatted
data")
} else {
fmt.Printf("r = %#v\n", r)
}
return nil
}
```
If the file `/etc/passwd1` is not exist, we expect to return
`nil source for passwd-formatted data` error, and in fact, the func
`GetUserPasswd` return nil.
The same logic exists in runc code. this patch fix it.
Signed-off-by: Wang Long <[email protected]>
Since syscall is outdated and broken for some architectures, use x/sys/unix instead. There are still some dependencies on the syscall package that will remain in syscall for the forseeable future: Errno Signal SysProcAttr Additionally: - os still uses syscall, so it needs to be kept for anything returning *os.ProcessState, such as process.Wait. Signed-off-by: Christy Perez <[email protected]>
Signed-off-by: Valentin Rothberg <[email protected]>
Signed-off-by: Kenfe-Mickael Laventure <[email protected]>
This reverts commit be16efd31c1748d9203905dffe449c655791c6a9, reversing changes made to 51b501dab1889ca609db9c536ac976f0f53e7021. Signed-off-by: Kenfe-Mickael Laventure <[email protected]>
runc currently only support Linux platform, and since we dont intend to expose the support to other platform, removing all other platforms placeholder code. `libcontainer/configs` still being used in https://github.com/moby/moby/blob/master/daemon/daemon_windows.go so keeping it for now. After this, we probably should also rename files to drop linux suffices if possible. Signed-off-by: Daniel Dao <[email protected]>
This rearranges a bit of the user and group lookup, such that only a basic subset is exposed. Signed-off-by: Vincent Batts <[email protected]>
Signed-off-by: Akihiro Suda <[email protected]>
This fixes the following compilation error on 32bit ARM: ``` $ GOARCH=arm GOARCH=6 go build ./libcontainer/system/ libcontainer/system/linux.go:119:89: constant 4294967295 overflows int ``` Signed-off-by: Tibor Vass <[email protected]>
subgid is defined per user, not group (see subgid(5)) This commit also adds support for specifying subuid owner with a numeric UID. Signed-off-by: Akihiro Suda <[email protected]>
The Err() method should be called after the Scan() loop, not inside it. Found by git grep -A3 -F '.Scan()' | grep Err Signed-off-by: Kir Kolyshkin <[email protected]>
Commit f1e605bf added these two functions, but they are only used from Windows code. The v1 of this patch moved these functions to _windows.go file, but after some discussion we decided to drop windows code altogether, so this is what this patch now does. This fixes > libcontainer/user/user.go:64:6: func `groupFromOS` is unused (unused) > libcontainer/user/user.go:35:6: func `userFromOS` is unused (unused) Signed-off-by: Kir Kolyshkin <[email protected]>
Move the unix-specific code to a file that's not compiled on Windows. Some of the errors (ErrUnsupported, ErrNoPasswdEntries, ErrNoGroupEntries) are used in other parts of the code, so are moved to a non-platform specific file. Most of "user" is probably not useful on Windows, although it's possible that Windows code may have to parse a passwd file, so leaving that code for now. Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
gofumpt (mvdan.cc/gofumpt) is a fork of gofmt with stricter rules. Brought to you by git ls-files \*.go | grep -v ^vendor/ | xargs gofumpt -s -w Looking at the diff, all these changes make sense. Also, replace gofmt with gofumpt in golangci.yml. Signed-off-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Using fmt.Errorf for errors that do not have %-style formatting directives is an overkill. Switch to errors.New. Found by git grep fmt.Errorf | grep -v ^vendor | grep -v '%' Signed-off-by: Kir Kolyshkin <[email protected]>
This should result in no change when the error is printed, but make the errors returned unwrappable, meaning errors.As and errors.Is will work. Signed-off-by: Kir Kolyshkin <[email protected]>
Every []byte to string conversion results in a new allocation. Avoid some by using []byte more. Signed-off-by: Kir Kolyshkin <[email protected]>
Same as in other places (other parsers here, as well as golang os/user parser and glibc parser all tolerate extra space at BOL and EOL). Signed-off-by: Kir Kolyshkin <[email protected]>
Lines in /etc/group longer than 64 characters breaks the current implementation of group parser. This is caused by bufio.Scanner buffer limit. Fix by re-using the fix for a similar problem in golang os/user, namely https://go-review.googlesource.com/c/go/+/283601. Add some tests. Co-authored-by: Andrey Bokhanko <[email protected]> Signed-off-by: Kir Kolyshkin <[email protected]>
Go 1.17 introduce this new (and better) way to specify build tags. For more info, see https://golang.org/design/draft-gobuild. As a way to seamlessly switch from old to new build tags, gofmt (and gopls) from go 1.17 adds the new tags along with the old ones. Later, when go < 1.17 is no longer supported, the old build tags can be removed. Now, as I started to use latest gopls (v0.7.1), it adds these tags while I edit. Rather than to randomly add new build tags, I guess it is better to do it once for all files. Mind that previous commits removed some tags that were useless, so this one only touches packages that can at least be built on non-linux. Brought to you by go1.17 fmt ./... Signed-off-by: Kir Kolyshkin <[email protected]>
Since Go 1.19, godoc recognizes lists, code blocks, headings etc. It also reformats the sources making it more apparent that these features are used. Fix a few places where it misinterpreted the formatting (such as indented vs unindented), and format the result using the gofumpt from HEAD, which already incorporates gofmt 1.19 changes. Some more fixes (and enhancements) might be required. Signed-off-by: Kir Kolyshkin <[email protected]>
The exception was fixed by polyfloyd/go-errorlint#12 which eventually made its way into golangci-lint. Signed-off-by: Kir Kolyshkin <[email protected]>
This integrate the github.com/opencontainers/runc/libcontainer/user package at commit [opencontainers/runc@a3a0ec4]. This package was originally authored in a "utils" package in the Docker repository, after which moved to "libcontainer", which became part of runc. Some commits were not included in history, due to them being applied in the Docker repository before moving into "libcontainer". Leaving links to those for future reference: - [moby/moby@eb38750] - [moby/moby@e41507b] - [moby/moby@b07314e] [opencontainers/runc@a3a0ec4]: opencontainers/runc@a3a0ec4 [moby/moby@eb38750]: moby/moby@eb38750 [moby/moby@e41507b]: moby/moby@e41507b [moby/moby@b07314e]: moby/moby@b07314e Co-authored-by: Tianon Gravi <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
|
Posting for posterity (marked it to "pass" manually); looks like there's some more commits with an "incorrect" DCO now; I think it's due to the "old" format; Commit sha: ef3397c, Author: Aleksa Sarai, Committer: Tianon Gravi; The sign-off is missing. |
AkihiroSuda
approved these changes
Sep 17, 2023
|
@cyphar @kolyshkin ptal 😄 |
tianon
approved these changes
Sep 18, 2023
|
(happy to see a revert or a follow up if there are any serious concerns, but I think this is fine 🙏) |
|
❤️ I was also considering "should I merge?" or "should I first have all runc maintainers on board"? (because I don't want it to become "just a fork") I'll update my runc PR, and let's use that for the "final decision" (then I'll create a tag) |
This was referenced Oct 24, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
integrate github.com/opencontainers/runc/libcontainer/user
This integrate the github.com/opencontainers/runc/libcontainer/user package
at commit opencontainers/runc@a3a0ec48c464fe6458d2ba067f6707c027e5384e.
This package was originally authored in a "utils" package in the Docker
repository, after which moved to "libcontainer", which became part of runc.
Some commits were not included in history, due to them being applied in
the Docker repository before moving into "libcontainer". Leaving links
to those for future reference:
Difference with other branch
This branch is identical, except fo
Co-authored-byin the merge commit (although GitHub doesn't seem to show merge-commit messages 🐼 😢)LICENSEandNOTICEnot being includedmain, so it includes the.gitignorechanges from add _build to gitignore #133;Diff below: