51 Commits

Author SHA1 Message Date
ec1f350ee5 container: resolve workdir after all the mounts happen.
There are use-cases where users would want to use overlay-mounts as
workdir. For such cases workdir should be resolved after all the mounts
are completed during the container init process.

Signed-off-by: Aditya Rajan <arajan@redhat.com>
2021-08-30 19:49:26 +05:30
547fff2703 e2e tests: use Should(Exit()) and ExitWithError()
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>
2021-07-15 05:06:33 -06:00
ad3b56c62f Fix volumes with uid and gid options
Podman uses the volume option map to check if it has to mount the volume
or not when the container is started. Commit 28138dafcc39 added to uid
and gid options to this map, however when only uid/gid is set we cannot
mount this volume because there is no filesystem or device specified.
Make sure we do not try to mount the volume when only the uid/gid option
is set since this is a simple chown operation.

Also when a uid/gid is explicity set, do not chown the volume based on
the container user when the volume is used for the first time.

Fixes #10620

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
2021-06-11 13:27:08 +02:00
4d51995377 Fix podman build --pull-never
Currently pull policy is set incorrectly when users set --pull-never.

Also pull-policy is not being translated correctly when using
podman-remote.

Fixes: #9573

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2021-03-27 05:33:30 -04:00
452decf8a4 Ensure manually-created volumes have correct ownership
As part of a fix for an earlier bug (#5698) we added the ability
for Podman to chown volumes to correctly match the user running
in the container, even in adverse circumstances (where we don't
know the right UID/GID until very late in the process). However,
we only did this for volumes created automatically by a
`podman run` or `podman create`. Volumes made by
`podman volume create` do not get this chown, so their
permissions may not be correct. I've looked, and I don't think
there's a good reason not to do this chwon for all volumes the
first time the container is started.

I would prefer to do this as part of volume copy-up, but I don't
think that's really possible (copy-up happens earlier in the
process and we don't have a spec). There is a small chance, as
things stand, that a copy-up happens for one container and then
a chown for a second, unrelated container, but the odds of this
are astronomically small (we'd need a very close race between two
starting containers).

Fixes #9608

Signed-off-by: Matthew Heon <mheon@redhat.com>
2021-03-24 14:24:47 -04:00
874f2327e6 Add U volume flag to chown source volumes
Signed-off-by: Eduardo Vega <edvegavalerio@gmail.com>
2021-02-22 22:55:19 -06:00
5dded6fae7 bump go module to v3
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>
2021-02-22 09:03:51 +01:00
a9d548bf77 Change source path resolution for volume copy-up
Instead of using the container's mountpoint as the base of the
chroot and indexing from there by the volume directory, instead
use the full path of what we want to copy as the base of the
chroot and copy everything in it. This resolves the bug, ends up
being a bit simpler code-wise (no string concatenation, as we
already have the full path calculated for other checks), and
seems more understandable than trying to resolve things on the
destination side of the copy-up.

Fixes #9354

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2021-02-17 14:57:06 -05:00
759fc93343 Fix an issue where copyup could fail with ENOENT
This one is rather bizarre because it triggers only on some
systems. I've included a CI test, for example, but I'm 99% sure
we use images in CI that have volumes over empty directories, and
the earlier patch to change copy-up implementation passed CI
without complaint.

I can reproduce this on a stock F33 VM, but that's the only place
I have been able to see it.

Regardless, the issue: under certain as-yet-unidentified
environmental conditions, the copier.Get method will return an
ENOENT attempting to stream a directory that is empty. Work
around this by avoiding the copy altogether in this case.

Signed-off-by: Matthew Heon <mheon@redhat.com>
2021-02-16 09:21:49 -05:00
ea910fc535 Rewrite copy-up to use buildah Copier
The old copy-up implementation was very unhappy with symlinks,
which could cause containers to fail to start for unclear reasons
when a directory we wanted to copy-up contained one. Rewrite to
use the Buildah Copier, which is more recent and should be both
safer and less likely to blow up over links.

At the same time, fix a deadlock in copy-up for volumes requiring
mounting - the Mountpoint() function tried to take the
already-acquired volume lock.

Fixes #6003

Signed-off-by: Matthew Heon <mheon@redhat.com>
2021-02-10 14:21:37 -05:00
4a6d042c28 Docker ignores mount flags that begin with constency
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1915332

```
According to the Docker docs, the consistency option should be ignored on Linux.

the possible values are 'cached', 'delegated', and 'consistent', but they should be ignored equally.

This is a widely used option in scripts run by developer machines, as this makes file I/O less horribly slow on MacOS.
```

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2021-01-30 06:50:18 -05:00
0f637e09da Ensure we do not double-lock the same volume in create
When making containers, we want to lock all named volumes we are
adding the container to, to ensure they aren't removed from under
us while we are working. Unfortunately, this code did not account
for a container having the same volume mounted in multiple places
so it could deadlock. Add a map to ensure that we don't lock the
same name more than once to resolve this.

Fixes #8221

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-11-11 16:06:03 -05:00
15345ce4c3 podman create doesn't support creating detached containers
Detached containers and detach keys are only created with the podman run, i
exec, and start commands.  We do not store the detach key sequence or the
detach flags in the database, nor does Docker. The current code was ignoreing
these fields but documenting that they can be used.

Fix podman create man page and --help output to no longer indicate that
--detach and --detach-keys works.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-10-21 21:00:50 -04:00
b496802413 Make all Skips specify a reason
Always use CGROUPV2 rather then reading from system all the time.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-09-29 16:01:26 -04:00
007c0ecd50 Remove SkipIfRootless if possible, document other calls
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-09-27 07:55:16 -04:00
b7147afde9 e2e tests: SkipIfRemote(): add a reason
Now that Dan has added helpful comments to each SkipIfRemote,
let's take the next step and include those messages in the
Skip() output so someone viewing test results can easily
see if a remote test is skipped for a real reason or for
a FIXME.

This commit is the result of a simple:

   perl -pi -e 's;(SkipIfRemote)\(\)(\s+//\s+(.*))?;$1("$3");' *.go

in the test/e2e directory, with a few minor (manual) changes
in wording.

Signed-off-by: Ed Santiago <santiago@redhat.com>
2020-09-23 08:09:31 -06:00
a277b7eb0b Examine all SkipIfRemote functions
Remove ones that are not needed.
Document those that should be there.
Document those that should be fixed.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-09-22 16:55:37 -04:00
fef3e2da6a Remove some unnecessary []byte to string conversions
Some calls to `Sprintf("%s")` can be avoided by using direct string
type assertions.

Signed-off-by: Sascha Grunert <sgrunert@suse.com>
2020-08-03 09:13:04 +02:00
a5e37ad280 Switch all references to github.com/containers/libpod -> podman
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-07-28 08:23:45 -04:00
a75f96ae90 Turn on a bunch more remote tests
We need to be more specific about the remote tests we turn off.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-07-22 13:13:28 -04:00
020d81f113 Add support for overlay volume mounts in podman.
Add support -v for overlay volume mounts in podman.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

Signed-off-by: Qi Wang <qiwan@redhat.com>
2020-07-20 09:48:55 -04:00
a10d5b42ab Change buildtag for remoteclient to remote for testing
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-07-06 15:22:24 -04:00
8489dc4345 move go module to v2
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>
2020-07-06 15:50:12 +02:00
e26f9eda64 Add support for readonly option to --mount
This is just an alias to the `ro` option, but it's already in the
manpages (and Docker) so we might as well add support for it.

Fixes #6379

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-05-26 16:05:11 -04:00
cc65430145 Turn off 'noexec' option by default for named volumes
We previously enforced this for security reasons, but as Dan has
explained on several occasions, it's not very valuable there
(it's trivially easy to bypass) and it does seriously annoy folks
trying to use named volumes. Flip the default from 'on' to 'off'.

Signed-off-by: Matthew Heon <mheon@redhat.com>
2020-05-20 16:48:20 -04:00
e78e66c5b9 enable volume integration tests
enabled integration tests for volumes.  there are two exceptions that still need work because of something not yet implemented.

also, add code to deal with the fact that containers conf appears to set a local volume driver where it used to be simply blank.

Signed-off-by: Brent Baude <bbaude@redhat.com>
2020-04-27 16:12:06 -05:00
02671a103f Add support for volumes-from, image volumes, init
This should complete Podmanv2's support for volume-related flags.
Most code was sourced from the old pkg/spec implementation with
modifications to account for the split between frontend flags
(volume, mount, tmpfs) and the backend flags implemented here.

Also enables tests for podman run with volumes

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-04-27 13:13:21 -04:00
5c968b7693 Force integration tests to pass
Failing tests are now skipped and we should work from this.

Signed-off-by: Brent Baude <bbaude@redhat.com>
2020-04-21 13:48:50 -05:00
e39cf4cf59 test/e2e/run_volume_test: use unique mount point
For volume and bind mount tests, use the in-container mount point path
that has no common ancestor with any host path (except for root).

This might help to uncover bugs like [1]. Even if not, it seems
lile a good cleanup regardless.

[1] https://github.com/containers/libpod/pull/5676

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-04-02 10:19:40 -07:00
504f9082ee test/e2e/run_volume_test.go: mv dockerfile decl
Move declaration of a dockerfile closer to its use.
Since it is used only once, there's no sense in having it declared
globally.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-04-02 10:17:10 -07:00
ce38be7959 test/e2e/run_volume_test: only create dir once
Remove repeated mountPath directory creation.

 * For the first two hunks it is the same dir ("secrets") that was
   already created before.

 * For the last hunk ("scratchpad") it is not used at all.

Add an empty line after Mkdir for cases where dir is used more than once.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-04-02 10:17:10 -07:00
3449b27cd1 Switch to using --time as opposed to --timeout to better match Docker.
We need to consistently use --time rather then --timeout throughout the code.
Fix locations where timeout defaults are not set correctly as well.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-03-31 08:50:32 -04:00
40fa7e9931 Use cleaned destination path for indexing image volumes
We use filepath.Clean() to remove trailing slashes to ensure that
when we supercede image mounts with mounts from --volume and
--mount, paths are consistent when we compare. Unfortunately,
while we used the cleaned path for the destination in the mount,
it was accidentally not used to index the maps that we use to
identify what to supercede, so our comparisons might be thrown
off by trailing slashes and similar.

Fixes #5219

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-02-17 16:28:36 -05:00
c140ecdc9b Do not copy up when volume is not empty
When Docker performs a copy up, it first verifies that the volume
being copied into is empty; thus, for volumes that have been
modified elsewhere (e.g. manually copying into then), the copy up
will not be performed at all. Duplicate this behavior in Podman
by checking if the volume is empty before copying.

Furthermore, move setting copyup to false further up. This will
prevent a potential race where copy up could happen more than
once if Podman was killed after some files had been copied but
before the DB was updated.

This resolves CVE-2020-1726.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-02-12 10:58:42 -05:00
4e5e9dbec2 mount: add new options nocopyup|copyup for tmpfs
add a way to disable tmpcopyup for tmpfs.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
2019-11-07 18:24:02 +01:00
3e891c1b60 Wait for mount command to finish when mounting volume
command.Start() just starts the command. That catches some
errors, but the nasty ones - bad options and similar - happen
when the command runs. Use CombinedOutput() instead - it waits
for the command to exit, and thus catches non-0 exit of the
`mount` command (invalid options, for example).

STDERR from the `mount` command is directly used, which isn't
necessarily the best, but we can't really get much more info on
what went wrong.

Fixes #4303

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-10-30 14:41:16 -04:00
57eaea9539 Image volumes should not be mounted noexec
This matches Docker more closely, but retains the more important
protections of nosuid/nodev.

Fixes #4318

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-10-23 12:09:22 -04:00
0d623914d0 Add support for anonymous volumes to podman run -v
Previously, when `podman run` encountered a volume mount without
separate source and destination (e.g. `-v /run`) we would assume
that both were the same - a bind mount of `/run` on the host to
`/run` in the container. However, this does not match Docker's
behavior - in Docker, this makes an anonymous named volume that
will be mounted at `/run`.

We already have (more limited) support for these anonymous
volumes in the form of image volumes. Extend this support to
allow it to be used with user-created volumes coming in from the
`-v` flag.

This change also affects how named volumes created by the
container but given names are treated by `podman run --rm` and
`podman rm -v`. Previously, they would be removed with the
container in these cases, but this did not match Docker's
behaviour. Docker only removed anonymous volumes. With this patch
we move to that model as well; `podman run -v testvol:/test` will
not have `testvol` survive the container being removed by `podman
rm -v`.

The sum total of these changes let us turn on volume removal in
`--rm` by default.

Fixes: #4276

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-10-17 13:18:17 -04:00
60d0be17fc Refactor tests when checking for error exit codes
Rather than checking for non-zero, we need to check for >0 to
distinguish between timeouts and error exit codes.

Signed-off-by: Jhon Honce <jhonce@redhat.com>
2019-10-16 08:10:07 -07:00
d89414b1f0 Handle conflict between volumes and --read-only-tmpfs
When a named volume is mounted on any of the tmpfs filesystems
created by read-only tmpfs, it caused a conflict that was not
resolved prior to this.

Fixes BZ1755119

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-09-24 15:57:17 -04:00
b6106341fb When first mounting any named volume, copy up
Previously, we only did this for volumes created at the same time
as the container. However, this is not correct behavior - Docker
does so for all named volumes, even those made with
'podman volume create' and mounted into a container later.

Fixes #3945

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-09-09 17:17:39 -04:00
a760e325f3 Add ability for volumes with options to mount/umount
When volume options and the local volume driver are specified,
the volume is intended to be mounted using the 'mount' command.
Supported options will be used to volume the volume before the
first container using it starts, and unmount the volume after the
last container using it dies.

This should work for any local filesystem, though at present I've
only tested with tmpfs and btrfs.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-09-05 17:12:27 -04:00
5f15501578 Add test to verify noexec works with volume mounts
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-09-04 13:19:19 -04:00
c13a52cfde Add a test for the new suid/exec/dev options
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-08-28 14:28:52 -04:00
e563f41116 Re-add locks to volumes.
This will require a 'podman system renumber' after being applied
to get lock numbers for existing volumes.

Add the DB backend code for rewriting volume configs and use it
for updating lock numbers as part of 'system renumber'.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-08-28 11:35:00 -04:00
c0a124ea89 Allow --ro=[true|false] with mount flag
The 'podman run --mount' flag previously allowed the 'ro' option
to be specified, but was missing the ability to set it to a bool
(as is allowed by docker). Add that. While we're at it, allow
setting 'rw' explicitly as well.

Fixes #2980

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-08-07 10:03:01 -04:00
30f24bb760 Add tests for cached and delegated mounts
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-06-19 09:57:33 -04:00
f610a485c1 use imagecaches for local tests
when doing localized tests (not varlink), we can use secondary image
stores as read-only image caches.  this cuts down on test time
significantly because each test does not need to restore the images from
a tarball anymore.

Signed-off-by: baude <bbaude@redhat.com>
2019-05-29 15:12:05 -05:00
43f28cdc16 Add test for image volume conflict with user volume
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-05-21 12:36:48 -04:00
bb564b68e1 Fix podman-in-podman volume test
When running inside Podman, we get an extra `nodev` mount option.
It doesn't seem to be a bug, more an artifact of running in a
somewhat locked-down container. So instead of checking explicitly
for a set of mount options, just verify the ones we set are
present.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2019-05-02 15:17:03 -04:00