Moving from Go module v4 to v5 prepares us for public releases.
Move done using gomove [1] as with the v3 and v4 moves.
[1] https://github.com/KSubedi/gomove
Signed-off-by: Matt Heon <mheon@redhat.com>
The fedora minimal 39 image has been updated on the fedora registry and
removed the `useradd` binary. Since we were pulling by tag and not by
digest, updates to images outside of our control always entail a certain
risk - and now it bit us.
To fix it, try to move as many users of `useradd` to _our_ CITEST_IMAGE
and migrate the code where necessary to this Alpine-based tooling.
However, the Alpine-based `adduser` binary (not useradd!) doesn't work
well when being executed as a non-root user and will just error out.
Hence, move the fedora minimal image back to version 34 which is still
including the `useradd` binary.
Ultimately, all images on public registries should be pulled via digest
to make sure we pin them down. I refrain from doing this now to make
sure we can cherry-pick this PR to older branches and get things back
into a working state ASAP.
Fixes: #20119
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
Commit 1 of 2.
More easy ones: test files that either work with ExitCleanly()
or require very, very simple tweaks.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Motivation
===========
This feature aims to make --uidmap and --gidmap easier to use, especially in rootless podman setups.
(I will focus here on the --gidmap option, although the same applies for --uidmap.)
In rootless podman, the user namespace mapping happens in two steps, through an intermediate mapping.
See https://docs.podman.io/en/latest/markdown/podman-run.1.html#uidmap-container-uid-from-uid-amount
for further detail, here is a summary:
First the user GID is mapped to 0 (root), and all subordinate GIDs (defined at /etc/subgid, and
usually >100000) are mapped starting at 1.
One way to customize the mapping is through the `--gidmap` option, that maps that intermediate mapping
to the final mapping that will be seen by the container.
As an example, let's say we have as main GID the group 1000, and we also belong to the additional GID 2000,
that we want to make accessible inside the container.
We first ask the sysadmin to subordinate the group to us, by adding "$user:2000:1" to /etc/subgid.
Then we need to use --gidmap to specify that we want to map GID 2000 into some GID inside the container.
And here is the first trouble:
Since the --gidmap option operates on the intermediate mapping, we first need to figure out where has
podman placed our GID 2000 in that intermediate mapping using:
podman unshare cat /proc/self/gid_map
Then, we may see that GID 2000 was mapped to intermediate GID 5. So our --gidmap option should include:
--gidmap 20000:5:1
This intermediate mapping may change in the future if further groups are subordinated to us (or we stop
having its subordination), so we are forced to verify the mapping with
`podman unshare cat /proc/self/gid_map` every time, and parse it if we want to script it.
**The first usability improvement** we agreed on #18333 is to be able to use:
--gidmap 20000:@2000:1
so podman does this lookup in the parent user namespace for us.
But this is only part of the problem. We must specify a **full** gidmap and not only what we want:
--gidmap 0:0:5 --gidmap 5:6:15000 --gidmap 20000:5:1
This is becoming complicated. We had to break the gidmap at 5, because the intermediate 5 had to
be mapped to another value (20000), and then we had to keep mapping all other subordinate ids... up to
close to the maximum number of subordinate ids that we have (or some reasonable value). This is hard
to explain to someone who does not understand how the mappings work internally.
To simplify this, **the second usability improvement** is to be able to use:
--gidmap "+20000:@2000:1"
where the plus flag (`+`) states that the given mapping should extend any previous/default mapping,
overriding any previous conflicting assignment.
Podman will set that mapping and fill the rest of mapped gids with all other subordinated gids, leading
to the same (or an equivalent) full gidmap that we were specifying before.
One final usability improvement related to this is the following:
By default, when podman gets a --gidmap argument but not a --uidmap argument, it copies the mapping.
This is convenient in many scenarios, since usually subordinated uids and gids are assigned in chunks
simultaneously, and the subordinated IDs in /etc/subuid and /etc/subgid for a given user match.
For scenarios with additional subordinated GIDs, this map copying is annoying, since it forces the user
to provide a --uidmap, to prevent the copy from being made. This means, that when the user wants:
--gidmap 0:0:5 --gidmap 5:6:15000 --gidmap 20000:5:1
The user has to include a uidmap as well:
--gidmap 0:0:5 --gidmap 5:6:15000 --gidmap 20000:5:1 --uidmap 0:0:65000
making everything even harder to understand without proper context.
For this reason, besides the "+" flag, we introduce the "u" and "g" flags. Those flags applied to a
mapping tell podman that the mapping should only apply to users or groups, and ignored otherwise.
Therefore we can use:
--gidmap "+g20000:@2000:1"
So the mapping only applies to groups and is ignored for uidmaps. If no "u" nor "g" flag is assigned
podman assumes the mapping applies to both users and groups as before, so we preserve backwards compatibility.
Co-authored-by: Tom Sweeney <tsweeney@redhat.com>
Signed-off-by: Sergio Oller <sergioller@gmail.com>
To silence my find-obsolete-skips script, remove the '#'
from the following issues in skip messages:
#11784#15013#15025#17433#17436#17456
Also update the messages to reflect the fact that the issues
will never be fixed.
Also remove ubuntu skips: we no longer test ubuntu.
Also remove one buildah skip that is no longer applicable:
Fixes: #17520
Signed-off-by: Ed Santiago <santiago@redhat.com>
the combination --pod and --userns is already blocked. Ignore the
PODMAN_USERNS variable when a pod is used, since it would cause to
create a new user namespace for the container.
Ideally a container should be able to do that, but its user namespace
must be a child of the pod user namespace, not a sibling. Since
nested user namespaces are not allowed in the OCI runtime specs,
disallow this case, since the end result is just confusing for the
user.
Closes: https://github.com/containers/podman/issues/18580
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
There is no reason to define the same code every time in each file, just
use global nodes. This diff should speak for itself.
CleanupSecrets()/Volume() no longer call Cleanup() directly, as the
global AfterEach node will always call Cleanup() this is no longer
necessary. If one AfterEach() node fails it will still run the others.
Also always unset the CONTAINERS_CONF env vars. This prevents people
from forgetting to unset it. And fix the special CONTAINERS_CONF logic
in the system connection tests, we do not want to preserve
CONTAINERS_CONF anyway so just remove this logic.
Ginkgo orders the BeforeEach and AfterEach nodes. They will be executed
from the outer-most defined to inner-most. This means our global
BeforeEach is always first. Only then the inner one (in the Describe()
function in each file). For AfterEach it is inverted, from the inner to
the outer.
Also see https://onsi.github.io/ginkgo/#organizing-specs-with-container-nodes
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
...at least as many as possible. "run/exec -it" make no sense
in a CI environment; I believe the vast majority of these are
the result of fingers typing on autopilot, then copy/pasting
cascades from those. This PR gets rid of as many -it/-ti as
possible. Some are still needed for testing purposes.
Y'all have no idea how much I hate #10927 (the "no logs from conmon"
flake). This does not fix the underlying problem, nor does it even
eliminate the flake (The "exec terminal doesn't hang" test needs
to keep the -ti flag, and that's one of the most popular flakers).
But this at least reduces the scope of the problem. It also removes
a ton of nasty orange "input device is not a TTY" warnings from logs.
Signed-off-by: Ed Santiago <santiago@redhat.com>
always use the direct mapping when writing the mappings for an
idmapped mount. crun was previously using the reverse mapping, which
is not correct and it is being addressed here:
https://github.com/containers/crun/pull/1147
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
copy the current mapping into a new user namespace, and run into a
separate user namespace.
Closes: https://github.com/containers/podman/issues/17337
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Ha ha. This was supposed to be a trivial little followup to #17174:
https://github.com/containers/podman/pull/17174#discussion_r1085963780
(safer username check when --userns=keep-id)
It got complicated. TL;DR we need to use User.Username, not User.Name.
The latter is GECOS! Tests were working because, on Fedora, GECOS
for root is "root". Found and fixed all 'u.Name' instances, but
if there are any references with a variable other than 'u', they
still need looking into.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Unify the functions used to detect rootless to "isRootless()".
This function can detect to join the user namespace by mistake.
Signed-off-by: Toshiki Sonoda <sonoda.toshiki@fujitsu.com>
I found the ginkgolinter[1] by accident, this looks for not optimal
matching and suggest how to do it better.
Overall these fixes seem to be all correct and they will give much
better error messages when something fails.
Check out the repo to see what the linter reports.
[1] https://github.com/nunnatsa/ginkgolinter
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
honor eventual options set in the containers.userns setting in the
containers.conf file, e.g.:
[containers]
userns = "auto:size=8192"
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
idmap is documented as supported for volumes, but it was not added to
the getNamedVolume() function.
Fixes: e83d36665 ("volumes: add new option idmap")
Signed-off-by: Kristian Klausen <kristian@klausen.dk>
Package `io/ioutil` was deprecated in golang 1.16, preventing podman from
building under Fedora 37. Fortunately, functionality identical
replacements are provided by the packages `io` and `os`. Replace all
usage of all `io/ioutil` symbols with appropriate substitutions
according to the golang docs.
Signed-off-by: Chris Evich <cevich@redhat.com>
add two new options to the keep-id user namespace option:
- uid: allow to override the UID used inside the container.
- gid: allow to override the GID used inside the container.
For example, the following command will map the rootless user (that
has UID=0 inside the rootless user namespace) to the UID=11 inside the
container user namespace:
$ podman run --userns=keep-id:uid=11 --rm -ti fedora cat /proc/self/uid_map
0 1 11
11 0 1
12 12 65525
Closes: https://github.com/containers/podman/issues/15294
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
`podman-remote` and Libpod API does not supports build with
`--userns=auto` since `IDMappingOptions` were not implemented for API
and bindings, following PR implements passing `IDMappingOptions` via
bindings to API.
Closes: https://github.com/containers/podman/issues/15476
Signed-off-by: Aditya R <arajan@redhat.com>
if an explicit mapping is specified, do not accept `--userns` since it
overriden to "private".
Closes: https://github.com/containers/podman/issues/15233
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
The errcheck linter makes sure that errors are always check and not
ignored by accident. It spotted a lot of unchecked errors, mostly in the
tests but also some real problem in the code.
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
From a security point of view, it would be nice to be able to map a
rootless usernamespace that does not use your own UID within the
container.
This would add protection against a hostile process escapping the
container and reading content in your homedir.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Commit 2 of 2: there were (still are?) a bunch of string
checks that didn't have a corresponding Expect(). IIUC
that means they were NOPs. Try to identify and fix those.
The first few were caught by Go linting, "ok is defined
but not used". When I realized the problem, I looked for
more using:
$ ack -A2 LineInOutputStartsWith
...and tediously eyeballing the results, looking for
matches in which the next line was not Expect(). If
test was wrong (e.g. "server" should've been "nameserver"),
fix that.
Also: remove the remove-betrue script. We don't need it
in the repo, I just wanted to preserve it for posterity.
Signed-off-by: Ed Santiago <santiago@redhat.com>
Many ginkgo tests have been written to use this evil form:
GrepString("foo")
Expect(that to BeTrue())
...which yields horrible useless messages on failure:
false is not true
Identify those (automatically, via script) and convert to:
Expect(output to ContainSubstring("foo"))
...which yields:
"this output" does not contain substring "foo"
There are still many BeTrue()s left. This is just a start.
This is commit 1 of 2. It includes the script I used, and
all changes to *.go are those computed by the script.
Commit 2 will apply some manual fixes.
Signed-off-by: Ed Santiago <santiago@redhat.com>
e2e test failures are rife with messages like:
Expected 1 to equal 0
These make me cry. They're anti-helpful, requiring the reader
to dive into the source code to figure out what those numbers
mean.
Solution: Go tests have a '.Should(Exit(NNN))' mechanism. I
don't know if it spits out a better diagnostic (I have no way
to run e2e tests on my laptop), but I have to fantasize that
it will, and given the state of our flakes I assume that at
least one test will fail and give me the opportunity to see
what the error message looks like.
THIS IS NOT REVIEWABLE CODE. There is no way for a human
to review it. Don't bother. Maybe look at a few random
ones for sanity. If you want to really review, here is
a reproducer of what I did:
cd test/e2e
! positive assertions. The second is the same as the first,
! with the addition of (unnecessary) parentheses because
! some invocations were written that way. The third is BeZero().
perl -pi -e 's/Expect\((\S+)\.ExitCode\(\)\)\.To\(Equal\((\d+)\)\)/Expect($1).Should(Exit($2))/' *_test.go
perl -pi -e 's/Expect\((\S+)\.ExitCode\(\)\)\.To\(\(Equal\((\d+)\)\)\)/Expect($1).Should(Exit($2))/' *_test.go
perl -pi -e 's/Expect\((\S+)\.ExitCode\(\)\)\.To\(BeZero\(\)\)/Expect($1).Should(Exit(0))/' *_test.go
! Same as above, but handles three non-numeric exit codes
! in run_exit_test.go
perl -pi -e 's/Expect\((\S+)\.ExitCode\(\)\)\.To\(Equal\((\S+)\)\)/Expect($1).Should(Exit($2))/' *_test.go
! negative assertions. Difference is the spelling of 'To(Not)',
! 'ToNot', and 'NotTo'. I assume those are all the same.
perl -pi -e 's/Expect\((\S+)\.ExitCode\(\)\)\.To\(Not\(Equal\((0)\)\)\)/Expect($1).To(ExitWithError())/' *_test.go
perl -pi -e 's/Expect\((\S+)\.ExitCode\(\)\)\.ToNot\(Equal\((0)\)\)/Expect($1).To(ExitWithError())/' *_test.go
perl -pi -e 's/Expect\((\S+)\.ExitCode\(\)\)\.NotTo\(Equal\((0)\)\)/Expect($1).To(ExitWithError())/' *_test.go
! negative, old use of BeZero()
perl -pi -e 's/Expect\((\S+)\.ExitCode\(\)\)\.ToNot\(BeZero\(\)\)/Expect($1).Should(ExitWithError())/' *_test.go
Run those on a clean copy of main branch (at the same branch
point as my PR, of course), then diff against a checked-out
copy of my PR. There should be no differences. Then all you
have to review is that my replacements above are sane.
UPDATE: nope, that's not enough, you also need to add gomega/gexec
to the files that don't have it:
perl -pi -e '$_ .= "$1/gexec\"\n" if m!^(.*/onsi/gomega)"!' $(grep -L gomega/gexec $(git log -1 --stat | awk '$1 ~ /test\/e2e\// { print $1}'))
UPDATE 2: hand-edit run_volume_test.go
UPDATE 3: sigh, add WaitWithDefaultTimeout() to a couple of places
UPDATE 4: skip a test due to bug #10935 (race condition)
Signed-off-by: Ed Santiago <santiago@redhat.com>
We missed bumping the go module, so let's do it now :)
* Automated go code with github.com/sirkon/go-imports-rename
* Manually via `vgrep podman/v2` the rest
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
when joining an existing container user namespace, read the existing
mappings so the storage can be created with the correct ownership.
Closes: https://github.com/containers/podman/issues/7547
Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>
Bind-mounting /etc/passwd into the container is problematic
becuase of how system utilities like `useradd` work. They want
to make a copy and then rename to try to prevent breakage; this
is, unfortunately, impossible when the file they want to rename
is a bind mount. The current behavior is fine for read-only
containers, though, because we expect useradd to fail in those
cases.
Instead of bind-mounting, we can edit /etc/passwd in the
container's rootfs. This is kind of gross, because the change
will show up in `podman diff` and similar tools, and will be
included in images made by `podman commit`. However, it's a lot
better than breaking important system tools.
Fixes#6953
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
If I enter a continer with --userns keep-id, my UID will be present
inside of the container, but most likely my user will not be defined.
This patch will take information about the user and stick it into the
container.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
With the advent of Podman 2.0.0 we crossed the magical barrier of go
modules. While we were able to continue importing all packages inside
of the project, the project could not be vendored anymore from the
outside.
Move the go module to new major version and change all imports to
`github.com/containers/libpod/v2`. The renaming of the imports
was done via `gomove` [1].
[1] https://github.com/KSubedi/gomove
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
specify the mappings in the container configuration to the storage
when creating the container so that the correct mappings can be
configured.
Regression introduced with Podman 2.0.
Closes: https://github.com/containers/libpod/issues/6735
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Currently podman run --userns keep-id --user root:root fedora id
The --user flag is ignored. Removing this makes the code work correctly.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
Matt Heon
Paul Holzinger
Valentin Rothberg
Ed Santiago
Sergio Oller
Giuseppe Scrivano
Chris Evich
Toshiki Sonoda
Erik Sjölund
Kristian Klausen
Aditya R
Daniel J Walsh
Valentin Rothberg
Giuseppe Scrivano
OpenShift Merge Robot
Matthew Heon
Brent Baude