Commit graph

123 commits

Author SHA1 Message Date
Matthew Heon
484bd0af1d Ensure pod infra containers have an exit command
Most Libpod containers are made via `pkg/specgen/generate` which
includes code to generate an appropriate exit command which will
handle unmounting the container's storage, cleaning up the
container's network, etc. There is one notable exception: pod
infra containers, which are made entirely within Libpod and do
not touch pkg/specgen. As such, no cleanup process, network never
cleaned up, bad things can happen.

There is good news, though - it's not that difficult to add this,
and it's done in this PR. Generally speaking, we don't allow
passing options directly to the infra container at create time,
but we do (optionally) proxy a pre-approved set of options into
it when we create it. Add ExitCommand to these options, and set
it at time of pod creation using the same code we use to generate
exit commands for normal containers.

Fixes #7103

Signed-off-by: Matthew Heon <mheon@redhat.com>

<MH: Fixed cherry-pick conflicts>

Signed-off-by: Matthew Heon <mheon@redhat.com>
2020-08-24 11:31:37 -04:00
Brent Baude
23251149ab error when adding container to pod with network information
because a pod's network information is dictated by the infra container at creation, a container cannot be created with network attributes.  this has been difficult for users to understand.  we now return an error when a container is being created inside a pod and passes any of the following attributes:

* static IP (v4 and v6)
* static mac
* ports -p (i.e. -p 8080:80)
* exposed ports (i.e. 222-225)
* publish ports from image -P

Signed-off-by: Brent Baude <bbaude@redhat.com>

<MH: Fixed cherry pick conflicts and compile>

Signed-off-by: Matthew Heon <mheon@redhat.com>
2020-08-24 11:31:37 -04:00
Daniel J Walsh
7ed653804c
In podman 1.* regression on --cap-add
In podman 1.0 if  you executed a command like:

podman run --user dwalsh --cap-add net_bind_service alpine nc -l 80

It would work, and the user dwalsh would get the capability,  in
podman 2.0, only root and the binding set gets the capability.

This change restores us back to the way podman 1.0 worked.
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-08-21 10:55:22 -04:00
Paul Holzinger
0ef6688785 fix podman create/run UTS NS docs
Add better error message when using `--pod` and `--hostname`.
Improve the docs to better explain the uts hostname relation.
Add more valid options for the `--uts` flag.

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
2020-08-20 12:25:55 -04:00
Matthew Heon
23348e7f31 Ensure DefaultEnvVariables is used in Specgen
When we rewrote Podman's pkg/spec, one of the things that was
lost was our use of a set of default environment variables, that
ensure all containers have at least $PATH and $TERM set.

While we're in the process of re-adding it, change it from a
variable to a function, so we can ensure the Join function does
not overwrite it and corrupt the defaults.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-08-20 12:16:53 -04:00
Daniel J Walsh
fc24c0cc10 Fix handling of working dir
Buildah and podman build can create images without a working dir.

FROM fedora
WORKDIR /test

If you build this image with caching twice, the second time the image
will not have a working dir.

Similarly if you execute

podman run --workdir /foobar fedora

It blows up since the workingdir is not created automatically.

Finally there was duplicated code for getting the workingdir
out of an image, that this PR removes.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-08-20 12:16:53 -04:00
Matthew Heon
32f0c8f624 Do not use image CMD if user gave ENTRYPOINT
This matches Docker behavior, and seems to make sense - the CMD
may have been specific to the original entrypoint and probably
does not make sense if it was changed.

While we're in here, greatly simplify the logic for populating
the SpecGen's Command. We create the full command when making the
OCI spec, so the client should not be doing any more than setting
it to the Command the user passed in, and completely ignoring
ENTRYPOINT.

Fixes #7115

Signed-off-by: Matthew Heon <mheon@redhat.com>
2020-08-20 12:16:53 -04:00
Matthew Heon
d4c3365454 Ensure WORKDIR from images is created
A recent crun change stopped the creation of the container's
working directory if it does not exist. This is arguably correct
for user-specified directories, to protect against typos; it is
definitely not correct for image WORKDIR, where the image author
definitely intended for the directory to be used.

This makes Podman create the working directory and chown it to
container root, if it does not already exist, and only if it was
specified by an image, not the user.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-08-20 12:16:53 -04:00
Sascha Grunert
66fcafa4d4 Allow specifying seccomp profiles for privileged containers
To sync the behavior between AppArmor and seccomp it is now possible to
also specify seccomp profiles for privileged containers.

Signed-off-by: Sascha Grunert <sgrunert@suse.com>
2020-08-20 12:16:53 -04:00
Christian Heimes
7fb53bc240 Use set for systemd commands
Signed-off-by: Christian Heimes <cheimes@redhat.com>
2020-08-20 12:16:52 -04:00
Christian Heimes
76ae0c9075 Enable systemd mode for /usr/local/sbin/init
Podman 1.6.2 changed systemd mode auto-detection from commands ending in
``init`` to hard-coded paths ``/sbin/init`` and ``/usr/sbin/init``. This
broke FreeIPA container. ``podman run`` and ``podman create`` now
activate systemd mode when the command is ``/usr/local/sbin/init``.

Fixes: https://github.com/containers/podman/issues/7287
Signed-off-by: Christian Heimes <cheimes@redhat.com>
2020-08-20 12:16:52 -04:00
Daniel J Walsh
3dfd8630a5 Add username to /etc/passwd inside of container if --userns keep-id
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>
2020-08-11 14:58:29 +02:00
Paul Holzinger
895e0d0e2e fix pod creation with "new:" syntax
When you execute podman create/run with the --pod new:<name> syntax
the pod was created but the namespaces where not shared and
therefore containers could not communicate over localhost.

Add the default namespaces and pass the network options to the
pod create options.

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
2020-08-11 13:53:23 +02:00
Matthew Heon
da752a7ed3 Binding the same container port to >1 host port is OK
The initial version of the new port code mistakenly restricted
this, so un-restrict it. We still need to maintain the map of
container ports, unfortunately (need to verify if the port in
question is a duplicate, for example).

Fixes #7062

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-07-31 11:41:05 -04:00
Daniel J Walsh
4d7626cc89 Fix handling of entrypoint
If a user specifies an entrypoint of "" then we should not use the images
entrypoint.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-07-22 14:53:26 -04:00
Matthew Heon
0630d19b34 Fix container and pod create commands for remote create
In `podman inspect` output for containers and pods, we include
the command that was used to create the container. This is also
used by `podman generate systemd --new` to generate unit files.

With remote podman, the generated create commands were incorrect
since we sourced directly from os.Args on the server side, which
was guaranteed to be `podman system service` (or some variant
thereof). The solution is to pass the command along in the
Specgen or PodSpecgen, where we can source it from the client's
os.Args.

This will still be VERY iffy for mixed local/remote use (doing a
`podman --remote run ...` on a remote client then a
`podman generate systemd --new` on the server on the same
container will not work, because the `--remote` flag will slip
in) but at the very least the output of `podman inspect` will be
correct. We can look into properly handling `--remote` (parsing
it out would be a little iffy) in a future PR.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

<MH: Fixed build after cherry-pick>

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-07-22 14:39:41 -04:00
Daniel J Walsh
800595a0a3 Mask out /sys/dev to prevent information leak from the host
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-07-22 14:35:30 -04:00
Matthew Heon
db81bc2c83 When determining systemd mode, use full command
We were only using the Command field in specgen when determining
whether to enable systemd if systemd=true (the default) was used.
This does not include the entrypoint, and does not include any
entrypoint/command sourced from the image - so an image could be
running systemd and we'd not correctly detect this. Using the
full, final command resolves this and matches Podman v1.9.x
behavior.

Fixes #6920

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

<MH: Fixed compile after backport>

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-07-22 14:28:42 -04:00
Akihiro Suda
9c8ffc4615 Fix "Error: unrecognized protocol \"TCP\" in port mapping"
"TCP" in upper characters was not recognized as a valid protocol name.

Fix #6948

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
2020-07-22 14:15:30 -04:00
Brent Baude
c6852ee516 Error on rootless mac and ip addresses
When creating a pod or container where a static MAC or IP address is provided, we should return a proper error and exit as 125.

Fixes: #6972

Signed-off-by: Brent Baude <bbaude@redhat.com>

<MH: Fixed build after cherry-pick>

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-07-22 14:13:43 -04:00
Giuseppe Scrivano
e21a6368f9 abi: set default umask and rlimits
the code got lost in the migration to podman 2.0, reintroduce it.

Closes: https://github.com/containers/podman/issues/6989

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

<MH: Fixed build>

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-07-22 14:05:20 -04:00
Daniel J Walsh
cb603b8a3e Support default profile for apparmor
Currently you can not apply an ApparmorProfile if you specify
--privileged.  This patch will allow both to be specified
simultaniosly.

By default Apparmor should be disabled if the user
specifies --privileged, but if the user specifies --security apparmor:PROFILE,
with --privileged, we should do both.

Added e2e run_apparmor_test.go

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-07-22 13:14:15 -04:00
OpenShift Merge Robot
e3e2b1e659
Merge pull request #6871 from mheon/202_backports
Backports for v2.0.2
2020-07-06 15:55:15 -04:00
Ralf Haferkamp
626aeffc55 specgen: fix order for setting rlimits
Also make sure that the limits we set for rootless are not higher than
what we'd set for root containers.

Rootless containers failed to start when the calling user already
had ulimit (e.g. on NOFILE) set.

This is basically a cherry-pick of 76f8efc0d0 into specgen

Signed-off-by: Ralf Haferkamp <rhafer@suse.com>
2020-07-06 14:21:53 -04:00
Daniel J Walsh
c4023a9302
Pids-limit should only be set if the user set it
Currently we are sending over pids-limits from the user even if they
never modified the defaults.  The pids limit should be set at the server
side unless modified by the user.

This issue has led to failures on systems that were running with cgroups V1.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-07-06 14:14:48 -04:00
Matthew Heon
1c02d5ab89 Allow empty host port in --publish flag
I didn't believe that this was actually legal, but it looks like
it is. And, unlike our previous understanding (host port being
empty means just use container port), empty host port actually
carries the same meaning as `--expose` + `--publish-all` (that
is, assign a random host port to the given container port). This
requires a significant rework of our port handling code to handle
this new case. I don't foresee this being commonly used, so I
optimized having a fixed port number as fast path, which this
random assignment code running after the main port handling code
only if necessary.

Fixes #6806

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-07-06 13:37:27 -04:00
Valentin Rothberg
2d74811520 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:52:17 +02:00
Matthew Heon
2ad9dcc07e Set syslog for exit commands on log-level=debug
We have a flag, --syslog, for telling logrus to log to syslog as
well as to the terminal. Previously, this flag also set the exit
command for containers to use `--syslog` (otherwise all output
from exit commands is lost). I attempted to replicate this with
Podman v2.0, but quickly ran into circular import hell (the flag
is defined in cmd/podman, I needed it in cmd/podman/containers,
cmd/podman imports cmd/podman/containers already, etc). Instead,
let's just set the syslog flag automatically on
`--log-level=debug` so we log exit commands automatically when
debug-level logs are requested. This is consistent with Conmon
and seems to make sense.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-06-25 15:42:32 -04:00
Matthew Heon
0b4dcc7a36 Fix conflicts between privileged and other flags
The `--privileged` flag does not conflict with `--group-add`
(this one was breaking Toolbox) and does not conflict with most
parts of `--security-opt` (this was breaking Openstack).

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-06-24 14:37:35 -04:00
OpenShift Merge Robot
6472b44c34
Merge pull request #6654 from rhatdan/keepid
Don't ignore --user flag in rootless --userns keepid
2020-06-17 21:22:06 -04:00
Daniel J Walsh
fe69aa9ba3
Handle dropping capabilties correctly when running as non root user
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-06-17 17:20:53 -04:00
Daniel J Walsh
c385e6d363
Don't ignore --user flag in rootless --userns keepid
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>
2020-06-17 17:13:58 -04:00
Matthew Heon
6f1440a3ec Add support for the unless-stopped restart policy
We initially believed that implementing this required support for
restarting containers after reboot, but this is not the case.
The unless-stopped restart policy acts identically to the always
restart policy except in cases related to reboot (which we do not
support yet), but it does not require that support for us to
implement it.

Changes themselves are quite simple, we need a new restart policy
constant, we need to remove existing checks that block creation
of containers when unless-stopped was used, and we need to update
the manpages.

Fixes #6508

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-06-17 11:16:12 -04:00
OpenShift Merge Robot
78ad57ebd3
Merge pull request #6636 from mheon/add_warnings
Re-add resource limit warnings to Specgen
2020-06-17 16:51:51 +02:00
Matthew Heon
c51c593ff6 Re-add resource limit warnings to Specgen
These were part of Podman v1.9, but were lost in the transition
to using Specgen to create containers. Most resource limits are
checked via the sysinfo package to ensure they are safe to use
(the cgroup is mounted, kernel support is present, etc) and
removed if not safe. Further, bounds checks are performed to
ensure that values are valid.

Ensure these warnings are printed client-side when they occur.
This part is a little bit gross, as it happens in pkg/infra and
not cmd/podman, which is largely down to how we implemented
`podman run` - all the work is done in pkg/infra and it returns
only once the container has exited, and we need warnings to print
*before* the container runs. The solution here, while inelegant,
avoid the need to extensively refactor our handling of run.

Should fix blkio-limit warnings that were identified by the FCOS
test suite.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-06-16 17:32:01 -04:00
Joseph Gooch
eb8bfdad3e Fix --init and --init-path
Init properly passed into specgen
Allow --init with --systemd=true but not --systemd=always.

Signed-off-by: Joseph Gooch <mrwizard@dok.org>
2020-06-16 17:37:39 +00:00
Valentin Rothberg
402c68b41d pod create: add --infra-conmon-pidfile
Add an `--infra-conmon-pidfile` flag to `podman-pod-create` to write the
infra container's conmon process ID to a specified path.  Several
container sub-commands already support `--conmon-pidfile` which is
especially helpful to allow for systemd to access and track the conmon
processes.  This allows for easily tracking the conmon process of a
pod's infra container.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
2020-06-11 11:01:13 +02:00
Valentin Rothberg
636881ece5 pod config: add a CreateCommand field
Add a `CreateCommand` field to the pod config which includes the entire
`os.Args` at pod-creation.  Similar to the already existing field in a
container config, we need this information to properly generate generic
systemd unit files for pods.  It's a prerequisite to support the `--new`
flag for pods.

Also add the `CreateCommand` to the pod-inspect data, which can come in
handy for debugging, general inspection and certainly for the tests that
are added along with the other changes.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
2020-06-11 11:01:13 +02:00
Daniel J Walsh
4bb43b898d
Fixup issues found by golint
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-06-10 05:49:41 -04:00
Daniel J Walsh
c8f57b71a4
Fix handling of systemd.
Systemd enablement has to happen on the server side, since we need
check if the image is running systemd.

Also need to make sure user setting the StopSignal is not overriden on the
server side. But if not set and using systemd, we set it correctly.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-06-05 13:01:57 -04:00
OpenShift Merge Robot
f2432335ef
Merge pull request #6498 from mheon/fix_pod_hostname
Ensure that containers in pods properly set hostname
2020-06-05 09:41:26 -04:00
Daniel J Walsh
10ace87ca9
Fix handling of ThrottleWriteIOPSDevice
This is causing the UBuntu tests to fail.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-06-05 08:11:37 -04:00
Matthew Heon
644a7b78ff Ensure that containers in pods properly set hostname
When we moved to the new Namespace types in Specgen, we made a
distinction between taking a namespace from a pod, and taking it
from another container. Due to this new distinction, some code
that previously worked for both `--pod=$ID` and
`--uts=container:$ID` has accidentally become conditional on only
the latter case. This happened for Hostname - we weren't properly
setting it in cases where the container joined a pod.
Fortunately, this is an easy fix once we know to check the
condition.

Also, ensure that `podman pod inspect` actually prints hostname.

Fixes #6494

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-06-04 16:32:10 -04:00
Matthew Heon
e0d9404634 Enable detached exec for remote
The biggest obstacle here was cleanup - we needed a way to remove
detached exec sessions after they exited, but there's no way to
tell if an exec session will be attached or detached when it's
created, and that's when we must add the exit command that would
do the removal. The solution was adding a delay to the exit
command (5 minutes), which gives sufficient time for attached
exec sessions to retrieve the exit code of the session after it
exits, but still guarantees that they will be removed, even for
detached sessions. This requires Conmon 2.0.17, which has the new
`--exit-delay` flag.

As part of the exit command rework, we can drop the hack we were
using to clean up exec sessions (remove them as part of inspect).
This is a lot cleaner, and I'm a lot happier about it.

Otherwise, this is just plumbing - we need a bindings call for
detached exec, and that needed to be added to the tunnel mode
backend for entities.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-06-02 15:30:42 -04:00
Jhon Honce
670f7c271f Add invalid value to error message
Signed-off-by: Jhon Honce <jhonce@redhat.com>
2020-06-01 09:23:10 -07:00
Peter Hunt
339ffd8ade specgen: fix segfault
we should not access the devices without checking if the resources are there

Signed-off-by: Peter Hunt <pehunt@redhat.com>
2020-05-27 14:52:17 -04:00
Daniel J Walsh
e41089244b
Attempt to turn on build_without_cgo tests
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-05-22 12:56:19 -04:00
Daniel J Walsh
2e7d2c2f47
Start testing with cross compilation
Add missing man page links for podman-image-search and
podman-image-diff

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
2020-05-21 11:58:15 -04:00
Matthew Heon
6b9e9610d8 Enable cleanup processes for detached exec
The cleanup command creation logic is made public as part of this
and wired such that we can call it both within SpecGen (to make
container exit commands) and from the ABI detached exec handler.
Exit commands are presently only used for detached exec, but
theoretically could be turned on for all exec sessions if we
wanted (I'm declining to do this because of potential overhead).

I also forgot to copy the exit command from the exec config into
the ExecOptions struct used by the OCI runtime, so it was not
being added.

There are also two significant bugfixes for exec in here. One is
for updating the status of running exec sessions - this was
always failing as I had coded it to remove the exit file *before*
reading it, instead of after (oops). The second was that removing
a running exec session would always fail because I inverted the
check to see if it was running.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-05-20 16:11:05 -04:00
Matthew Heon
26f48139ce Add remaining annotations for podman inspect
This should finish support for `podman inspect` in APIv2.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
2020-05-08 14:45:32 -04:00