Files that are suffixed with `_linux.go` or `_windows.go` are
already only built on Linux / Windows, so these build-tags
were redundant.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This subtle bug keeps lurking in because error checking for `Mkdir()`
and `MkdirAll()` is slightly different wrt to `EEXIST`/`IsExist`:
- for `Mkdir()`, `IsExist` error should (usually) be ignored
(unless you want to make sure directory was not there before)
as it means "the destination directory was already there"
- for `MkdirAll()`, `IsExist` error should NEVER be ignored.
Mostly, this commit just removes ignoring the IsExist error, as it
should not be ignored.
Also, there are a couple of cases then IsExist is handled as
"directory already exist" which is wrong. As a result, some code
that never worked as intended is now removed.
NOTE that `idtools.MkdirAndChown()` behaves like `os.MkdirAll()`
rather than `os.Mkdir()` -- so its description is amended accordingly,
and its usage is handled as such (i.e. IsExist error is not ignored).
For more details, a quote from my runc commit 6f82d4b (July 2015):
TL;DR: check for IsExist(err) after a failed MkdirAll() is both
redundant and wrong -- so two reasons to remove it.
Quoting MkdirAll documentation:
> MkdirAll creates a directory named path, along with any necessary
> parents, and returns nil, or else returns an error. If path
> is already a directory, MkdirAll does nothing and returns nil.
This means two things:
1. If a directory to be created already exists, no error is
returned.
2. If the error returned is IsExist (EEXIST), it means there exists
a non-directory with the same name as MkdirAll need to use for
directory. Example: we want to MkdirAll("a/b"), but file "a"
(or "a/b") already exists, so MkdirAll fails.
The above is a theory, based on quoted documentation and my UNIX
knowledge.
3. In practice, though, current MkdirAll implementation [1] returns
ENOTDIR in most of cases described in #2, with the exception when
there is a race between MkdirAll and someone else creating the
last component of MkdirAll argument as a file. In this very case
MkdirAll() will indeed return EEXIST.
Because of #1, IsExist check after MkdirAll is not needed.
Because of #2 and #3, ignoring IsExist error is just plain wrong,
as directory we require is not created. It's cleaner to report
the error now.
Note this error is all over the tree, I guess due to copy-paste,
or trying to follow the same usage pattern as for Mkdir(),
or some not quite correct examples on the Internet.
[1] https://github.com/golang/go/blob/f9ed2f75/src/os/path.go
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Standard golang's `os.MkdirAll()` function returns "not a directory" error
in case a directory to be created already exists but is not a directory
(e.g. a file). Our own `idtools.MkdirAs*()` functions do not replicate
the behavior.
This is a bug since all `Mkdir()`-like functions are expected to ensure
the required directory exists and is indeed a directory, and return an
error otherwise.
As the code is using our in-house `system.Stat()` call returning a type
which is incompatible with that of golang's `os.Stat()`, I had to amend
the `system` package with `IsDir()`.
A test case is also provided.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In some cases (e.g. NFS), a chown may technically be a no-op but still
return `EPERM`, so only call `chown` when neccessary.
This is particularly problematic for docker users bind-mounting an NFS
share into a container.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Switch some more usage of the Stat function and the Stat_t type from the
syscall package to golang.org/x/sys. Those were missing in PR #33399.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
There is no case which would resolve in this error. The root user always exists, and if the id maps are empty, the default value of 0 is correct.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
This remove a dependency on `go-check` (and more) when using
`pkg/idtools`. `pkg/integration` should never be called from any other
package then `integration`.
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
When processing the --userns-remap flag, add the
capability to call out to `getent` if the user and
group information is not found via local file
parsing code already in libcontainer/user.
Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com>
Warn the user and fail daemon start if the graphdir path has any
elements which will deny access to the remapped root uid/gid.
Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com>
This should not have been in init() as it causes these lookups to happen
in all reexecs of the Docker binary. The only time it needs to be
resolved is when a user is added, which is extremely rare.
Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com>
Change user/group creation to use flags to adduser/useradd to enforce it
being a system user. Use system user defaults that auto-create a
matching group. These changes allow us to remove all group creation
code, and in doing so we also removed the code that finds available uid,
gid integers and use post-creation query to gather the system-generated
uid and gid.
The only added complexity is that today distros don't auto-create
subordinate ID ranges for a new ID if it is a system ID, so we now need
to handle finding a free range and then calling the `usermod` tool to
add the ranges for that ID. Note that this requires the distro supports
the `-v` and `-w` flags on `usermod` for subordinate ID range additions.
Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)
Since Docker is already skipping newlines in /etc/sub{uid,gid},
this patch skips commented out lines - otherwise Docker fails to start.
Add unit test also.
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
This fixes errors in ownership on directory creation during build that
can cause inaccessible files depending on the paths in the Dockerfile
and non-existing directories in the starting image.
Add tests for the mkdir variants in pkg/idtools
Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)
The `pkg/idtools` package supports the creation of user(s) for
retrieving /etc/sub{u,g}id ranges and creation of the UID/GID mappings
provided to clone() to add support for user namespaces in Docker.
Docker-DCO-1.1-Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (github: estesp)
