From 5e50ba3ecbdd738679849d7a86fef0c4ab7f109d Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Fri, 14 Aug 2020 11:19:02 +0200 Subject: podman support for IPv6 networks podman containers using IPv6 were missing the default route, breaking deployments trying to use them. The problem is that the default route was hardcoded to IPv4, this takes into consideration the podman subnet IP family to generate the corresponding default route. Signed-off-by: Antonio Ojea --- pkg/domain/infra/abi/network.go | 2 +- pkg/network/ip.go | 5 +++++ pkg/network/netconflist.go | 15 ++++++++++++--- pkg/network/netconflist_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 pkg/network/netconflist_test.go (limited to 'pkg') diff --git a/pkg/domain/infra/abi/network.go b/pkg/domain/infra/abi/network.go index 26383129c..fd63fc80f 100644 --- a/pkg/domain/infra/abi/network.go +++ b/pkg/domain/infra/abi/network.go @@ -191,7 +191,7 @@ func createBridge(r *libpod.Runtime, name string, options entities.NetworkCreate var plugins []network.CNIPlugins var routes []network.IPAMRoute - defaultRoute, err := network.NewIPAMDefaultRoute() + defaultRoute, err := network.NewIPAMDefaultRoute(network.IsIPv6(subnet.IP)) if err != nil { return "", err } diff --git a/pkg/network/ip.go b/pkg/network/ip.go index 1798cd939..ba93a0d05 100644 --- a/pkg/network/ip.go +++ b/pkg/network/ip.go @@ -12,3 +12,8 @@ func CalcGatewayIP(ipn *net.IPNet) net.IP { nid := ipn.IP.Mask(ipn.Mask) return ip.NextIP(nid) } + +// IsIPv6 returns if netIP is IPv6. +func IsIPv6(netIP net.IP) bool { + return netIP != nil && netIP.To4() == nil +} diff --git a/pkg/network/netconflist.go b/pkg/network/netconflist.go index 4271d3f54..8187fdb39 100644 --- a/pkg/network/netconflist.go +++ b/pkg/network/netconflist.go @@ -6,6 +6,11 @@ import ( "path/filepath" ) +const ( + defaultIPv4Route = "0.0.0.0/0" + defaultIPv6Route = "::/0" +) + // NcList describes a generic map type NcList map[string]interface{} @@ -86,9 +91,13 @@ func NewIPAMRoute(r *net.IPNet) IPAMRoute { //nolint:interfacer } // NewIPAMDefaultRoute creates a new IPAMDefault route of -// 0.0.0.0/0 -func NewIPAMDefaultRoute() (IPAMRoute, error) { - _, n, err := net.ParseCIDR("0.0.0.0/0") +// 0.0.0.0/0 for IPv4 or ::/0 for IPv6 +func NewIPAMDefaultRoute(isIPv6 bool) (IPAMRoute, error) { + route := defaultIPv4Route + if isIPv6 { + route = defaultIPv6Route + } + _, n, err := net.ParseCIDR(route) if err != nil { return IPAMRoute{}, err } diff --git a/pkg/network/netconflist_test.go b/pkg/network/netconflist_test.go new file mode 100644 index 000000000..a82a0140a --- /dev/null +++ b/pkg/network/netconflist_test.go @@ -0,0 +1,38 @@ +package network + +import ( + "reflect" + "testing" +) + +func TestNewIPAMDefaultRoute(t *testing.T) { + + tests := []struct { + name string + isIPv6 bool + want IPAMRoute + }{ + { + name: "IPv4 default route", + isIPv6: false, + want: IPAMRoute{defaultIPv4Route}, + }, + { + name: "IPv6 default route", + isIPv6: true, + want: IPAMRoute{defaultIPv6Route}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + got, err := NewIPAMDefaultRoute(tt.isIPv6) + if err != nil { + t.Errorf("no errorr expected: %v", err) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("NewIPAMDefaultRoute() = %v, want %v", got, tt.want) + } + }) + } +} -- cgit v1.2.3-54-g00ecf From cb4c5fc9c262f14c935e843224ee80e098d58224 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 13 Aug 2020 12:53:28 +0200 Subject: podman.service: use sdnotiy Commit 2b6dd3fb4384 set the killmode of the podman.service to the systemd default which ultimately lead to the problem that systemd will kill *all* processes inside the unit's cgroup and hence kill all containers whenever the service is stopped. Fix it by setting the type to sdnotify and the killmode to process. `podman system service` will send the necessary notify messages when the NOTIFY_SOCKET is set and unset it right after to prevent the backend and container runtimes from jumping in between and send messages as well. Fixes: #7294 Signed-off-by: Valentin Rothberg --- contrib/systemd/system/podman.service | 3 +- pkg/api/server/server.go | 27 ++++++- .../coreos/go-systemd/v22/daemon/sdnotify.go | 84 ++++++++++++++++++++++ .../coreos/go-systemd/v22/daemon/watchdog.go | 73 +++++++++++++++++++ vendor/modules.txt | 1 + 5 files changed, 186 insertions(+), 2 deletions(-) create mode 100644 vendor/github.com/coreos/go-systemd/v22/daemon/sdnotify.go create mode 100644 vendor/github.com/coreos/go-systemd/v22/daemon/watchdog.go (limited to 'pkg') diff --git a/contrib/systemd/system/podman.service b/contrib/systemd/system/podman.service index c8751168d..e14bbe078 100644 --- a/contrib/systemd/system/podman.service +++ b/contrib/systemd/system/podman.service @@ -6,5 +6,6 @@ Documentation=man:podman-system-service(1) StartLimitIntervalSec=0 [Service] -Type=simple +Type=notify +KillMode=process ExecStart=/usr/bin/podman system service diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go index 1c6007745..bfa5fcdce 100644 --- a/pkg/api/server/server.go +++ b/pkg/api/server/server.go @@ -2,6 +2,7 @@ package server import ( "context" + "fmt" "log" "net" "net/http" @@ -17,6 +18,7 @@ import ( "github.com/containers/libpod/v2/pkg/api/handlers" "github.com/containers/libpod/v2/pkg/api/server/idletracker" "github.com/coreos/go-systemd/v22/activation" + "github.com/coreos/go-systemd/v22/daemon" "github.com/gorilla/mux" "github.com/gorilla/schema" "github.com/pkg/errors" @@ -147,8 +149,31 @@ func newServer(runtime *libpod.Runtime, duration time.Duration, listener *net.Li return &server, nil } -// Serve starts responding to HTTP requests +// If the NOTIFY_SOCKET is set, communicate the PID and readiness, and +// further unset NOTIFY_SOCKET to prevent containers from sending +// messages and unset INVOCATION_ID so conmon and containers are in +// the correct cgroup. +func setupSystemd() { + if len(os.Getenv("NOTIFY_SOCKET")) == 0 { + return + } + payload := fmt.Sprintf("MAINPID=%d", os.Getpid()) + payload += "\n" + payload += daemon.SdNotifyReady + if sent, err := daemon.SdNotify(true, payload); err != nil { + logrus.Errorf("Error notifying systemd of Conmon PID: %s", err.Error()) + } else if sent { + logrus.Debugf("Notify sent successfully") + } + + if err := os.Unsetenv("INVOCATION_ID"); err != nil { + logrus.Errorf("Error unsetting INVOCATION_ID: %s", err.Error()) + } +} + +// Serve starts responding to HTTP requests. func (s *APIServer) Serve() error { + setupSystemd() sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) errChan := make(chan error, 1) diff --git a/vendor/github.com/coreos/go-systemd/v22/daemon/sdnotify.go b/vendor/github.com/coreos/go-systemd/v22/daemon/sdnotify.go new file mode 100644 index 000000000..ba4ae31f1 --- /dev/null +++ b/vendor/github.com/coreos/go-systemd/v22/daemon/sdnotify.go @@ -0,0 +1,84 @@ +// Copyright 2014 Docker, Inc. +// Copyright 2015-2018 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// Package daemon provides a Go implementation of the sd_notify protocol. +// It can be used to inform systemd of service start-up completion, watchdog +// events, and other status changes. +// +// https://www.freedesktop.org/software/systemd/man/sd_notify.html#Description +package daemon + +import ( + "net" + "os" +) + +const ( + // SdNotifyReady tells the service manager that service startup is finished + // or the service finished loading its configuration. + SdNotifyReady = "READY=1" + + // SdNotifyStopping tells the service manager that the service is beginning + // its shutdown. + SdNotifyStopping = "STOPPING=1" + + // SdNotifyReloading tells the service manager that this service is + // reloading its configuration. Note that you must call SdNotifyReady when + // it completed reloading. + SdNotifyReloading = "RELOADING=1" + + // SdNotifyWatchdog tells the service manager to update the watchdog + // timestamp for the service. + SdNotifyWatchdog = "WATCHDOG=1" +) + +// SdNotify sends a message to the init daemon. It is common to ignore the error. +// If `unsetEnvironment` is true, the environment variable `NOTIFY_SOCKET` +// will be unconditionally unset. +// +// It returns one of the following: +// (false, nil) - notification not supported (i.e. NOTIFY_SOCKET is unset) +// (false, err) - notification supported, but failure happened (e.g. error connecting to NOTIFY_SOCKET or while sending data) +// (true, nil) - notification supported, data has been sent +func SdNotify(unsetEnvironment bool, state string) (bool, error) { + socketAddr := &net.UnixAddr{ + Name: os.Getenv("NOTIFY_SOCKET"), + Net: "unixgram", + } + + // NOTIFY_SOCKET not set + if socketAddr.Name == "" { + return false, nil + } + + if unsetEnvironment { + if err := os.Unsetenv("NOTIFY_SOCKET"); err != nil { + return false, err + } + } + + conn, err := net.DialUnix(socketAddr.Net, nil, socketAddr) + // Error connecting to NOTIFY_SOCKET + if err != nil { + return false, err + } + defer conn.Close() + + if _, err = conn.Write([]byte(state)); err != nil { + return false, err + } + return true, nil +} diff --git a/vendor/github.com/coreos/go-systemd/v22/daemon/watchdog.go b/vendor/github.com/coreos/go-systemd/v22/daemon/watchdog.go new file mode 100644 index 000000000..7a0e0d3a5 --- /dev/null +++ b/vendor/github.com/coreos/go-systemd/v22/daemon/watchdog.go @@ -0,0 +1,73 @@ +// Copyright 2016 CoreOS, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package daemon + +import ( + "fmt" + "os" + "strconv" + "time" +) + +// SdWatchdogEnabled returns watchdog information for a service. +// Processes should call daemon.SdNotify(false, daemon.SdNotifyWatchdog) every +// time / 2. +// If `unsetEnvironment` is true, the environment variables `WATCHDOG_USEC` and +// `WATCHDOG_PID` will be unconditionally unset. +// +// It returns one of the following: +// (0, nil) - watchdog isn't enabled or we aren't the watched PID. +// (0, err) - an error happened (e.g. error converting time). +// (time, nil) - watchdog is enabled and we can send ping. +// time is delay before inactive service will be killed. +func SdWatchdogEnabled(unsetEnvironment bool) (time.Duration, error) { + wusec := os.Getenv("WATCHDOG_USEC") + wpid := os.Getenv("WATCHDOG_PID") + if unsetEnvironment { + wusecErr := os.Unsetenv("WATCHDOG_USEC") + wpidErr := os.Unsetenv("WATCHDOG_PID") + if wusecErr != nil { + return 0, wusecErr + } + if wpidErr != nil { + return 0, wpidErr + } + } + + if wusec == "" { + return 0, nil + } + s, err := strconv.Atoi(wusec) + if err != nil { + return 0, fmt.Errorf("error converting WATCHDOG_USEC: %s", err) + } + if s <= 0 { + return 0, fmt.Errorf("error WATCHDOG_USEC must be a positive number") + } + interval := time.Duration(s) * time.Microsecond + + if wpid == "" { + return interval, nil + } + p, err := strconv.Atoi(wpid) + if err != nil { + return 0, fmt.Errorf("error converting WATCHDOG_PID: %s", err) + } + if os.Getpid() != p { + return 0, nil + } + + return interval, nil +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 17c195326..a3d21b273 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -200,6 +200,7 @@ github.com/containers/storage/pkg/unshare github.com/coreos/go-iptables/iptables # github.com/coreos/go-systemd/v22 v22.1.0 github.com/coreos/go-systemd/v22/activation +github.com/coreos/go-systemd/v22/daemon github.com/coreos/go-systemd/v22/dbus github.com/coreos/go-systemd/v22/internal/dlopen github.com/coreos/go-systemd/v22/journal -- cgit v1.2.3-54-g00ecf From ceae3a99d54154fac6073f30bba2618768d05362 Mon Sep 17 00:00:00 2001 From: Jonathan Dieter Date: Tue, 11 Aug 2020 15:51:16 +0100 Subject: Fix hang when `path` doesn't exist I'm not sure if this is an OS-specific issue, but on CentOS 8, if `path` doesn't exist, this hangs while waiting to read from this socket, even though the socket is closed by the `reexec_in_user_namespace`. Switching to a pipe fixes the problem, and pipes shouldn't be an issue since this is Linux-specific code. Signed-off-by: Jonathan Dieter --- pkg/rootless/rootless_linux.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'pkg') diff --git a/pkg/rootless/rootless_linux.go b/pkg/rootless/rootless_linux.go index 2e580347d..fb9f3e156 100644 --- a/pkg/rootless/rootless_linux.go +++ b/pkg/rootless/rootless_linux.go @@ -363,14 +363,12 @@ func TryJoinFromFilePaths(pausePidPath string, needNewNamespace bool, paths []st lastErr = nil break } else { - fds, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_DGRAM, 0) + r, w, err := os.Pipe() if err != nil { lastErr = err continue } - r, w := os.NewFile(uintptr(fds[0]), "read file"), os.NewFile(uintptr(fds[1]), "write file") - defer errorhandling.CloseQuiet(r) if _, _, err := becomeRootInUserNS("", path, w); err != nil { -- cgit v1.2.3-54-g00ecf From 09bd563e2bc544aa987e90c5d831f146198eebb5 Mon Sep 17 00:00:00 2001 From: zhangguanzhang Date: Sun, 9 Aug 2020 22:05:26 +0800 Subject: Add parameter verification for api creation network Signed-off-by: zhangguanzhang --- pkg/network/network.go | 9 +++++++++ test/apiv2/35-networks.at | 28 +++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/network/network.go b/pkg/network/network.go index 37f3f721a..4da6bc969 100644 --- a/pkg/network/network.go +++ b/pkg/network/network.go @@ -137,6 +137,15 @@ func networkIntersect(n1, n2 *net.IPNet) bool { // ValidateUserNetworkIsAvailable returns via an error if a network is available // to be used func ValidateUserNetworkIsAvailable(config *config.Config, userNet *net.IPNet) error { + if len(userNet.IP) == 0 || len(userNet.Mask) == 0 { + return errors.Errorf("network %s's ip or mask cannot be empty", userNet.String()) + } + + ones, bit := userNet.Mask.Size() + if ones == 0 || bit == 0 { + return errors.Errorf("network %s's mask is invalid", userNet.String()) + } + networks, err := GetNetworksFromFilesystem(config) if err != nil { return err diff --git a/test/apiv2/35-networks.at b/test/apiv2/35-networks.at index fff3f3b1f..4c032c072 100644 --- a/test/apiv2/35-networks.at +++ b/test/apiv2/35-networks.at @@ -3,6 +3,32 @@ # network-related tests # -t GET /networks/non-existing-network 404 +t GET networks/non-existing-network 404 \ + .cause='network not found' + +if root; then + t POST libpod/networks/create?name=network1 '' 200 \ + .Filename~.*/network1\\.conflist + + # --data '{"Subnet":{"IP":"10.10.254.0","Mask":[255,255,255,0]}}' + t POST libpod/networks/create?name=network2 '"Subnet":{"IP":"10.10.254.0","Mask":[255,255,255,0]}' 200 \ + .Filename~.*/network2\\.conflist + + # test for empty mask + t POST libpod/networks/create '"Subnet":{"IP":"10.10.1.0","Mask":[]}' 500 \ + .cause~'.*cannot be empty' + # test for invalid mask + t POST libpod/networks/create '"Subnet":{"IP":"10.10.1.0","Mask":[0,255,255,0]}' 500 \ + .cause~'.*mask is invalid' + + # clean the network + t DELETE libpod/networks/network1 200 \ + .[0].Name~network1 \ + .[0].Err=null + t DELETE libpod/networks/network2 200 \ + .[0].Name~network2 \ + .[0].Err=null + +fi # vim: filetype=sh -- cgit v1.2.3-54-g00ecf From c539091bb0346cca036f0b7c4c4fe4423dcc66ee Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Thu, 6 Aug 2020 14:24:09 -0500 Subject: Replace deepcopy on history results the deepcopy in the remote history code path was throwing an uncaught error on a type mismatch. we now manually do the conversion and fix the type mismatch on the fly. Fixes: #7122 Signed-off-by: Brent Baude --- pkg/domain/infra/tunnel/images.go | 13 +++++++++++-- test/system/110-history.bats | 2 -- 2 files changed, 11 insertions(+), 4 deletions(-) (limited to 'pkg') diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index 2e30621c5..2e027a6e1 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/pkg/domain/infra/tunnel/images.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "strings" + "time" "github.com/containers/common/pkg/config" "github.com/containers/image/v5/docker/reference" @@ -65,8 +66,16 @@ func (ir *ImageEngine) History(ctx context.Context, nameOrID string, opts entiti } for i, layer := range results { - hold := entities.ImageHistoryLayer{} - _ = utils.DeepCopy(&hold, layer) + // Created time comes over as an int64 so needs conversion to time.time + t := time.Unix(layer.Created, 0) + hold := entities.ImageHistoryLayer{ + ID: layer.ID, + Created: t.UTC(), + CreatedBy: layer.CreatedBy, + Tags: layer.Tags, + Size: layer.Size, + Comment: layer.Comment, + } history.Layers[i] = hold } return &history, nil diff --git a/test/system/110-history.bats b/test/system/110-history.bats index b83e90fe4..5dc221d61 100644 --- a/test/system/110-history.bats +++ b/test/system/110-history.bats @@ -3,8 +3,6 @@ load helpers @test "podman history - basic tests" { - skip_if_remote "FIXME: pending #7122" - tests=" | .*[0-9a-f]\\\{12\\\} .* CMD .* LABEL --format '{{.ID}} {{.Created}}' | .*[0-9a-f]\\\{12\\\} .* ago -- cgit v1.2.3-54-g00ecf From 76ae0c907591b60374febdd8bd41f90cbc5444b6 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Tue, 11 Aug 2020 11:29:49 +0200 Subject: 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 --- docs/source/markdown/podman-create.1.md | 4 ++-- docs/source/markdown/podman-run.1.md | 4 ++-- pkg/specgen/generate/container_create.go | 3 ++- pkg/varlinkapi/create.go | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) (limited to 'pkg') diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md index 715d6c1c9..ffce338b4 100644 --- a/docs/source/markdown/podman-create.1.md +++ b/docs/source/markdown/podman-create.1.md @@ -777,8 +777,8 @@ Run container in systemd mode. The default is *true*. The value *always* enforces the systemd mode is enforced without looking at the executable name. Otherwise, if set to true and the -command you are running inside the container is systemd, /usr/sbin/init -or /sbin/init. +command you are running inside the container is systemd, /usr/sbin/init, +/sbin/init or /usr/local/sbin/init. If the command you are running inside of the container is systemd, Podman will setup tmpfs mount points in the following directories: diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md index fab648593..a500bfc41 100644 --- a/docs/source/markdown/podman-run.1.md +++ b/docs/source/markdown/podman-run.1.md @@ -809,8 +809,8 @@ Run container in systemd mode. The default is **true**. The value *always* enforces the systemd mode is enforced without looking at the executable name. Otherwise, if set to **true** and the -command you are running inside the container is systemd, _/usr/sbin/init_ -or _/sbin/init_. +command you are running inside the container is systemd, _/usr/sbin/init_, +_/sbin/init_ or _/usr/local/sbin/init_. If the command you are running inside of the container is systemd Podman will setup tmpfs mount points in the following directories: diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index be1e3b48e..630e59854 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -153,13 +153,14 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen. } if len(command) > 0 { - if command[0] == "/usr/sbin/init" || command[0] == "/sbin/init" || (filepath.Base(command[0]) == "systemd") { + if command[0] == "/usr/sbin/init" || command[0] == "/sbin/init" || command[0] == "/usr/local/sbin/init" || (filepath.Base(command[0]) == "systemd") { useSystemd = true } } default: return nil, errors.Wrapf(err, "invalid value %q systemd option requires 'true, false, always'", s.Systemd) } + logrus.Debugf("using systemd mode: %t", useSystemd) if useSystemd { // is StopSignal was not set by the user then set it to systemd // expected StopSigal diff --git a/pkg/varlinkapi/create.go b/pkg/varlinkapi/create.go index ac93939d9..249505486 100644 --- a/pkg/varlinkapi/create.go +++ b/pkg/varlinkapi/create.go @@ -704,7 +704,7 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod. if err != nil { return nil, errors.Wrapf(err, "cannot parse bool %s", c.String("systemd")) } - if x && (command[0] == "/usr/sbin/init" || command[0] == "/sbin/init" || (filepath.Base(command[0]) == "systemd")) { + if x && (command[0] == "/usr/sbin/init" || command[0] == "/sbin/init" || command[0] == "/usr/local/sbin/init" || (filepath.Base(command[0]) == "systemd")) { systemd = true } } -- cgit v1.2.3-54-g00ecf From 7fb53bc240cea153fb054bcd307d3b1a8945a435 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Tue, 11 Aug 2020 13:29:17 +0200 Subject: Use set for systemd commands Signed-off-by: Christian Heimes --- pkg/specgen/generate/container_create.go | 7 ++++++- pkg/varlinkapi/create.go | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) (limited to 'pkg') diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index 630e59854..b31bc91e0 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -153,7 +153,12 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen. } if len(command) > 0 { - if command[0] == "/usr/sbin/init" || command[0] == "/sbin/init" || command[0] == "/usr/local/sbin/init" || (filepath.Base(command[0]) == "systemd") { + useSystemdCommands := map[string]bool{ + "/sbin/init": true, + "/usr/sbin/init": true, + "/usr/local/sbin/init": true, + } + if useSystemdCommands[command[0]] || (filepath.Base(command[0]) == "systemd") { useSystemd = true } } diff --git a/pkg/varlinkapi/create.go b/pkg/varlinkapi/create.go index 249505486..7661173b8 100644 --- a/pkg/varlinkapi/create.go +++ b/pkg/varlinkapi/create.go @@ -704,7 +704,12 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod. if err != nil { return nil, errors.Wrapf(err, "cannot parse bool %s", c.String("systemd")) } - if x && (command[0] == "/usr/sbin/init" || command[0] == "/sbin/init" || command[0] == "/usr/local/sbin/init" || (filepath.Base(command[0]) == "systemd")) { + useSystemdCommands := map[string]bool{ + "/sbin/init": true, + "/usr/sbin/init": true, + "/usr/local/sbin/init": true, + } + if x && (useSystemdCommands[command[0]] || (filepath.Base(command[0]) == "systemd")) { systemd = true } } -- cgit v1.2.3-54-g00ecf From 66fcafa4d45a26b59ad3662419cd3c778e23c39c Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Mon, 10 Aug 2020 10:16:28 +0200 Subject: 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 --- pkg/specgen/generate/security.go | 5 +++-- test/e2e/run_test.go | 30 +++++++++++++++++++++++++++--- 2 files changed, 30 insertions(+), 5 deletions(-) (limited to 'pkg') diff --git a/pkg/specgen/generate/security.go b/pkg/specgen/generate/security.go index fcd1622f9..840dcb72d 100644 --- a/pkg/specgen/generate/security.go +++ b/pkg/specgen/generate/security.go @@ -158,8 +158,9 @@ func securityConfigureGenerator(s *specgen.SpecGenerator, g *generate.Generator, configSpec.Linux.Seccomp = seccompConfig } - // Clear default Seccomp profile from Generator for privileged containers - if s.SeccompProfilePath == "unconfined" || s.Privileged { + // Clear default Seccomp profile from Generator for unconfined containers + // and privileged containers which do not specify a seccomp profile. + if s.SeccompProfilePath == "unconfined" || (s.Privileged && (s.SeccompProfilePath == config.SeccompOverridePath || s.SeccompProfilePath == config.SeccompDefaultPath)) { configSpec.Linux.Seccomp = nil } diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 95eecd042..13f0db550 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -183,22 +183,46 @@ var _ = Describe("Podman run", func() { Expect(conData[0].Config.Annotations["io.podman.annotations.init"]).To(Equal("FALSE")) }) - It("podman run seccomp test", func() { - + forbidGetCWDSeccompProfile := func() string { in := []byte(`{"defaultAction":"SCMP_ACT_ALLOW","syscalls":[{"name":"getcwd","action":"SCMP_ACT_ERRNO"}]}`) jsonFile, err := podmanTest.CreateSeccompJson(in) if err != nil { fmt.Println(err) Skip("Failed to prepare seccomp.json for test.") } + return jsonFile + } + + It("podman run seccomp test", func() { + session := podmanTest.Podman([]string{"run", "-it", "--security-opt", strings.Join([]string{"seccomp=", forbidGetCWDSeccompProfile()}, ""), ALPINE, "pwd"}) + session.WaitWithDefaultTimeout() + Expect(session).To(ExitWithError()) + match, _ := session.GrepString("Operation not permitted") + Expect(match).Should(BeTrue()) + }) - session := podmanTest.Podman([]string{"run", "-it", "--security-opt", strings.Join([]string{"seccomp=", jsonFile}, ""), ALPINE, "pwd"}) + It("podman run seccomp test --privileged", func() { + session := podmanTest.Podman([]string{"run", "-it", "--privileged", "--security-opt", strings.Join([]string{"seccomp=", forbidGetCWDSeccompProfile()}, ""), ALPINE, "pwd"}) session.WaitWithDefaultTimeout() Expect(session).To(ExitWithError()) match, _ := session.GrepString("Operation not permitted") Expect(match).Should(BeTrue()) }) + It("podman run seccomp test --privileged no profile should be unconfined", func() { + session := podmanTest.Podman([]string{"run", "-it", "--privileged", ALPINE, "grep", "Seccomp", "/proc/self/status"}) + session.WaitWithDefaultTimeout() + Expect(session.OutputToString()).To(ContainSubstring("0")) + Expect(session.ExitCode()).To(Equal(0)) + }) + + It("podman run seccomp test no profile should be default", func() { + session := podmanTest.Podman([]string{"run", "-it", ALPINE, "grep", "Seccomp", "/proc/self/status"}) + session.WaitWithDefaultTimeout() + Expect(session.OutputToString()).To(ContainSubstring("2")) + Expect(session.ExitCode()).To(Equal(0)) + }) + It("podman run capabilities test", func() { session := podmanTest.Podman([]string{"run", "--rm", "--cap-add", "all", ALPINE, "cat", "/proc/self/status"}) session.WaitWithDefaultTimeout() -- cgit v1.2.3-54-g00ecf From d4c3365454d903077ece3c1a31367f639ee24900 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Fri, 31 Jul 2020 17:08:06 -0400 Subject: 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 --- libpod/container.go | 4 ++++ libpod/container_internal_linux.go | 27 ++++++++++++++++++++++++++- libpod/options.go | 13 +++++++++++++ pkg/specgen/generate/container_create.go | 11 +++++++++++ test/e2e/run_test.go | 10 ++++++++++ 5 files changed, 64 insertions(+), 1 deletion(-) (limited to 'pkg') diff --git a/libpod/container.go b/libpod/container.go index 9ad938a5c..644647bc9 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -261,6 +261,10 @@ type ContainerConfig struct { Mounts []string `json:"mounts,omitempty"` // NamedVolumes lists the named volumes to mount into the container. NamedVolumes []*ContainerNamedVolume `json:"namedVolumes,omitempty"` + // CreateWorkingDir indicates that Libpod should create the container's + // working directory if it does not exist. Some OCI runtimes do this by + // default, but others do not. + CreateWorkingDir bool `json:"createWorkingDir,omitempty"` // Security Config diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index b9e4f9a93..0d9a1c824 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -157,7 +157,32 @@ func (c *Container) prepare() error { } // Save changes to container state - return c.save() + if err := c.save(); err != nil { + return err + } + + // Ensure container entrypoint is created (if required) + if c.config.CreateWorkingDir { + workdir, err := securejoin.SecureJoin(c.state.Mountpoint, c.WorkingDir()) + if err != nil { + return errors.Wrapf(err, "error creating path to container %s working dir", c.ID()) + } + rootUID := c.RootUID() + rootGID := c.RootGID() + + if err := os.MkdirAll(workdir, 0755); err != nil { + if os.IsExist(err) { + return nil + } + return errors.Wrapf(err, "error creating container %s working dir", c.ID()) + } + + if err := os.Chown(workdir, rootUID, rootGID); err != nil { + return errors.Wrapf(err, "error chowning container %s working directory to container root", c.ID()) + } + } + + return nil } // cleanupNetwork unmounts and cleans up the container's network diff --git a/libpod/options.go b/libpod/options.go index 560b406e2..a4e4b99e9 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1395,6 +1395,19 @@ func WithCreateCommand(cmd []string) CtrCreateOption { } } +// WithCreateWorkingDir tells Podman to create the container's working directory +// if it does not exist. +func WithCreateWorkingDir() CtrCreateOption { + return func(ctr *Container) error { + if ctr.valid { + return define.ErrCtrFinalized + } + + ctr.config.CreateWorkingDir = true + return nil + } +} + // Volume Creation Options // WithVolumeName sets the name of the volume. diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index b31bc91e0..42be5e812 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -215,6 +215,17 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen. if s.Entrypoint != nil { options = append(options, libpod.WithEntrypoint(s.Entrypoint)) } + // If the user did not set an workdir but the image did, ensure it is + // created. + if s.WorkDir == "" && img != nil { + newWD, err := img.WorkingDir(ctx) + if err != nil { + return nil, err + } + if newWD != "" { + options = append(options, libpod.WithCreateWorkingDir()) + } + } if s.StopSignal != nil { options = append(options, libpod.WithStopSignal(*s.StopSignal)) } diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 13f0db550..ef275b32e 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -1060,4 +1060,14 @@ USER mail` Expect(session.ExitCode()).To(Equal(0)) Expect(session.OutputToString()).To(ContainSubstring(limit)) }) + + It("podman run makes entrypoint from image", func() { + dockerfile := `FROM busybox +WORKDIR /madethis` + podmanTest.BuildImage(dockerfile, "test", "false") + session := podmanTest.Podman([]string{"run", "--rm", "test", "pwd"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(ContainSubstring("/madethis")) + }) }) -- cgit v1.2.3-54-g00ecf From 32f0c8f624e0400a6decf219d6fe889f12272729 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Fri, 7 Aug 2020 13:57:02 -0400 Subject: 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 --- cmd/podman/common/specgen.go | 18 +----------------- pkg/specgen/generate/oci.go | 4 +++- test/e2e/run_test.go | 11 +++++------ 3 files changed, 9 insertions(+), 24 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go index 2333f2f7e..48a2069ff 100644 --- a/cmd/podman/common/specgen.go +++ b/cmd/podman/common/specgen.go @@ -387,8 +387,6 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string s.Annotations = annotations s.WorkDir = c.Workdir - userCommand := []string{} - var command []string if c.Entrypoint != nil { entrypoint := []string{} if ep := *c.Entrypoint; len(ep) > 0 { @@ -398,27 +396,13 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string } } s.Entrypoint = entrypoint - // Build the command - // If we have an entry point, it goes first - command = entrypoint } // Include the command used to create the container. s.ContainerCreateCommand = os.Args if len(inputCommand) > 0 { - // User command overrides data CMD - command = append(command, inputCommand...) - userCommand = append(userCommand, inputCommand...) - } - - switch { - case len(inputCommand) > 0: - s.Command = userCommand - case c.Entrypoint != nil: - s.Command = []string{} - default: - s.Command = command + s.Command = inputCommand } // SHM Size diff --git a/pkg/specgen/generate/oci.go b/pkg/specgen/generate/oci.go index f279aac1c..f1c9f2a1a 100644 --- a/pkg/specgen/generate/oci.go +++ b/pkg/specgen/generate/oci.go @@ -96,8 +96,10 @@ func makeCommand(ctx context.Context, s *specgen.SpecGenerator, img *image.Image finalCommand = append(finalCommand, entrypoint...) + // Only use image command if the user did not manually set an + // entrypoint. command := s.Command - if command == nil && img != nil { + if command == nil && img != nil && s.Entrypoint == nil { newCmd, err := img.Cmd(ctx) if err != nil { return nil, err diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index ef275b32e..681498ea1 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -1061,13 +1061,12 @@ USER mail` Expect(session.OutputToString()).To(ContainSubstring(limit)) }) - It("podman run makes entrypoint from image", func() { - dockerfile := `FROM busybox -WORKDIR /madethis` - podmanTest.BuildImage(dockerfile, "test", "false") - session := podmanTest.Podman([]string{"run", "--rm", "test", "pwd"}) + It("podman run --entrypoint does not use image command", func() { + session := podmanTest.Podman([]string{"run", "--entrypoint", "/bin/echo", ALPINE}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) - Expect(session.OutputToString()).To(ContainSubstring("/madethis")) + // We can't guarantee the output is completely empty, some + // nonprintables seem to work their way in. + Expect(session.OutputToString()).To(Not(ContainSubstring("/bin/sh"))) }) }) -- cgit v1.2.3-54-g00ecf From fc24c0cc102de911387ed6b768173604fecb1eee Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 5 Aug 2020 16:28:47 -0400 Subject: 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 --- pkg/specgen/container_validate.go | 5 --- pkg/specgen/generate/container.go | 15 ++++--- pkg/specgen/generate/container_create.go | 8 +--- test/e2e/run_working_dir.go | 69 ++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 18 deletions(-) create mode 100644 test/e2e/run_working_dir.go (limited to 'pkg') diff --git a/pkg/specgen/container_validate.go b/pkg/specgen/container_validate.go index 57dd2aba7..a979a7f4a 100644 --- a/pkg/specgen/container_validate.go +++ b/pkg/specgen/container_validate.go @@ -135,11 +135,6 @@ func (s *SpecGenerator) Validate() error { return err } - // The following are defaults as needed by container creation - if len(s.WorkDir) < 1 { - s.WorkDir = "/" - } - // Set defaults if network info is not provided if s.NetNS.NSMode == "" { s.NetNS.NSMode = Bridge diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go index f0d52d0c3..59fee80f1 100644 --- a/pkg/specgen/generate/container.go +++ b/pkg/specgen/generate/container.go @@ -135,15 +135,18 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat s.Annotations = annotations // workdir - if newImage != nil { - workingDir, err := newImage.WorkingDir(ctx) - if err != nil { - return nil, err - } - if len(s.WorkDir) < 1 && len(workingDir) > 1 { + if s.WorkDir == "" { + if newImage != nil { + workingDir, err := newImage.WorkingDir(ctx) + if err != nil { + return nil, err + } s.WorkDir = workingDir } } + if s.WorkDir == "" { + s.WorkDir = "/" + } if len(s.SeccompProfilePath) < 1 { p, err := libpod.DefaultSeccompPath() diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index 42be5e812..6c0a702d6 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -218,13 +218,7 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen. // If the user did not set an workdir but the image did, ensure it is // created. if s.WorkDir == "" && img != nil { - newWD, err := img.WorkingDir(ctx) - if err != nil { - return nil, err - } - if newWD != "" { - options = append(options, libpod.WithCreateWorkingDir()) - } + options = append(options, libpod.WithCreateWorkingDir()) } if s.StopSignal != nil { options = append(options, libpod.WithStopSignal(*s.StopSignal)) diff --git a/test/e2e/run_working_dir.go b/test/e2e/run_working_dir.go new file mode 100644 index 000000000..93330deba --- /dev/null +++ b/test/e2e/run_working_dir.go @@ -0,0 +1,69 @@ +package integration + +import ( + "os" + "strings" + + . "github.com/containers/podman/v2/test/utils" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Podman run", func() { + var ( + tempdir string + err error + podmanTest *PodmanTestIntegration + ) + + BeforeEach(func() { + tempdir, err = CreateTempDirInTempDir() + if err != nil { + os.Exit(1) + } + podmanTest = PodmanTestCreate(tempdir) + podmanTest.Setup() + podmanTest.SeedImages() + }) + + AfterEach(func() { + podmanTest.Cleanup() + f := CurrentGinkgoTestDescription() + processTestResult(f) + + }) + + It("podman run a container without workdir", func() { + session := podmanTest.Podman([]string{"run", ALPINE, "pwd"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(Equal("/")) + }) + + It("podman run a container using non existing --workdir", func() { + if !strings.Contains(podmanTest.OCIRuntime, "crun") { + Skip("Test only works on crun") + } + session := podmanTest.Podman([]string{"run", "--workdir", "/home/foobar", ALPINE, "pwd"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(127)) + }) + + It("podman run a container on an image with a workdir", func() { + SkipIfRemote() + dockerfile := `FROM alpine +RUN mkdir -p /home/foobar +WORKDIR /etc/foobar` + podmanTest.BuildImage(dockerfile, "test", "false") + + session := podmanTest.Podman([]string{"run", "test", "pwd"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(Equal("/etc/foobar")) + + session = podmanTest.Podman([]string{"run", "--workdir", "/home/foobar", "test", "pwd"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(session.OutputToString()).To(Equal("/home/foobar")) + }) +}) -- cgit v1.2.3-54-g00ecf From 4f349554adf4811d1081f55c2ba90952c57475e6 Mon Sep 17 00:00:00 2001 From: Parker Van Roy Date: Mon, 10 Aug 2020 14:37:12 -0400 Subject: Error pass through for more accurate error reporting Included old error + wrapped Signed-off-by: Parker Van Roy --- pkg/domain/infra/abi/images.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go index 5f19f416a..a4dc92820 100644 --- a/pkg/domain/infra/abi/images.go +++ b/pkg/domain/infra/abi/images.go @@ -108,7 +108,7 @@ func (ir *ImageEngine) Pull(ctx context.Context, rawImage string, options entiti if err != nil { imageRef, err = alltransports.ParseImageName(fmt.Sprintf("%s%s", dockerPrefix, rawImage)) if err != nil { - return nil, errors.Errorf("invalid image reference %q", rawImage) + return nil, errors.Wrapf(err, "invalid image reference %q", rawImage) } } -- cgit v1.2.3-54-g00ecf From eff0c29098fe988327112c37884d57a7b244ac40 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 4 Aug 2020 15:51:13 -0400 Subject: Unconditionally retrieve pod names via API The ListContainers API previously had a Pod parameter, which determined if pod name was returned (but, notably, not Pod ID, which was returned unconditionally). This was fairly confusing, so we decided to deprecate/remove the parameter and return it unconditionally. To do this without serious performance implications, we need to avoid expensive JSON decodes of pod configuration in the DB. The way our Bolt tables are structured, retrieving name given ID is actually quite cheap, but we did not expose this via the Libpod API. Add a new GetName API to do this. Fixes #7214 Signed-off-by: Matthew Heon --- libpod/boltdb_state.go | 55 +++++++++++++++++++++++++++++++++++ libpod/in_memory_state.go | 30 +++++++++++++++++++ libpod/runtime.go | 16 ++++++++++ libpod/state.go | 6 ++++ pkg/api/handlers/libpod/containers.go | 3 +- pkg/api/server/register_containers.go | 3 +- pkg/bindings/containers/containers.go | 5 +--- pkg/bindings/test/containers_test.go | 19 ++++++++++-- pkg/domain/infra/tunnel/containers.go | 2 +- pkg/domain/infra/tunnel/helpers.go | 2 +- pkg/ps/ps.go | 7 +++-- 11 files changed, 134 insertions(+), 14 deletions(-) (limited to 'pkg') diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 38881d3e4..4e2630526 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -424,6 +424,61 @@ func (s *BoltState) SetNamespace(ns string) error { return nil } +// GetName returns the name associated with a given ID. Since IDs are globally +// unique, it works for both containers and pods. +// Returns ErrNoSuchCtr if the ID does not exist. +func (s *BoltState) GetName(id string) (string, error) { + if id == "" { + return "", define.ErrEmptyID + } + + if !s.valid { + return "", define.ErrDBClosed + } + + idBytes := []byte(id) + + db, err := s.getDBCon() + if err != nil { + return "", err + } + defer s.deferredCloseDBCon(db) + + name := "" + + err = db.View(func(tx *bolt.Tx) error { + idBkt, err := getIDBucket(tx) + if err != nil { + return err + } + + nameBytes := idBkt.Get(idBytes) + if nameBytes == nil { + return define.ErrNoSuchCtr + } + + if s.namespaceBytes != nil { + nsBkt, err := getNSBucket(tx) + if err != nil { + return err + } + + idNs := nsBkt.Get(idBytes) + if !bytes.Equal(idNs, s.namespaceBytes) { + return define.ErrNoSuchCtr + } + } + + name = string(nameBytes) + return nil + }) + if err != nil { + return "", err + } + + return name, nil +} + // Container retrieves a single container from the state by its full ID func (s *BoltState) Container(id string) (*Container, error) { if id == "" { diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go index 794212bf0..b0eae0992 100644 --- a/libpod/in_memory_state.go +++ b/libpod/in_memory_state.go @@ -106,6 +106,36 @@ func (s *InMemoryState) SetNamespace(ns string) error { return nil } +// GetName retrieves the name associated with a given ID. +// Works with both Container and Pod IDs. +func (s *InMemoryState) GetName(id string) (string, error) { + if id == "" { + return "", define.ErrEmptyID + } + + var idIndex *truncindex.TruncIndex + if s.namespace != "" { + nsIndex, ok := s.namespaceIndexes[s.namespace] + if !ok { + // We have no containers in the namespace + // Return false + return "", define.ErrNoSuchCtr + } + idIndex = nsIndex.idIndex + } else { + idIndex = s.idIndex + } + + fullID, err := idIndex.Get(id) + if err != nil { + if err == truncindex.ErrNotExist { + return "", define.ErrNoSuchCtr + } + return "", errors.Wrapf(err, "error performing truncindex lookup for ID %s", id) + } + return fullID, nil +} + // Container retrieves a container from its full ID func (s *InMemoryState) Container(id string) (*Container, error) { if id == "" { diff --git a/libpod/runtime.go b/libpod/runtime.go index 24370d50e..867644be7 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -732,6 +732,22 @@ func (r *Runtime) GetStore() storage.Store { return r.store } +// GetName retrieves the name associated with a given full ID. +// This works for both containers and pods, and does not distinguish between the +// two. +// If the given ID does not correspond to any existing Pod or Container, +// ErrNoSuchCtr is returned. +func (r *Runtime) GetName(id string) (string, error) { + r.lock.RLock() + defer r.lock.RUnlock() + + if !r.valid { + return "", define.ErrRuntimeStopped + } + + return r.state.GetName(id) +} + // DBConfig is a set of Libpod runtime configuration settings that are saved in // a State when it is first created, and can subsequently be retrieved. type DBConfig struct { diff --git a/libpod/state.go b/libpod/state.go index 6206a2994..44632b02f 100644 --- a/libpod/state.go +++ b/libpod/state.go @@ -43,6 +43,12 @@ type State interface { // containers and pods in all namespaces will be returned. SetNamespace(ns string) error + // Resolve an ID into a Name. Since Podman names and IDs are globally + // unique between Pods and Containers, the ID may belong to either a pod + // or container. Despite this, we will always return ErrNoSuchCtr if the + // ID does not exist. + GetName(id string) (string, error) + // Return a container from the database from its full ID. // If the container is not in the set namespace, an error will be // returned. diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go index 2303ff17a..abfc79a0b 100644 --- a/pkg/api/handlers/libpod/containers.go +++ b/pkg/api/handlers/libpod/containers.go @@ -39,7 +39,6 @@ func ListContainers(w http.ResponseWriter, r *http.Request) { Filters map[string][]string `schema:"filters"` Last int `schema:"last"` Namespace bool `schema:"namespace"` - Pod bool `schema:"pod"` Size bool `schema:"size"` Sync bool `schema:"sync"` }{ @@ -59,7 +58,7 @@ func ListContainers(w http.ResponseWriter, r *http.Request) { Size: query.Size, Sort: "", Namespace: query.Namespace, - Pod: query.Pod, + Pod: true, Sync: query.Sync, } pss, err := ps.GetContainerLists(runtime, opts) diff --git a/pkg/api/server/register_containers.go b/pkg/api/server/register_containers.go index 18ff2f423..a0c9ac574 100644 --- a/pkg/api/server/register_containers.go +++ b/pkg/api/server/register_containers.go @@ -661,11 +661,10 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error { // type: boolean // description: Include namespace information // default: false - // - in: query // name: pod // type: boolean // default: false - // description: Include Pod ID and Name if applicable + // description: Ignored. Previously included details on pod name and ID that are currently included by default. // - in: query // name: size // type: boolean diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go index c479e5dcb..d7e67567b 100644 --- a/pkg/bindings/containers/containers.go +++ b/pkg/bindings/containers/containers.go @@ -24,7 +24,7 @@ var ( // the most recent number of containers. The pod and size booleans indicate that pod information and rootfs // size information should also be included. Finally, the sync bool synchronizes the OCI runtime and // container state. -func List(ctx context.Context, filters map[string][]string, all *bool, last *int, pod, size, sync *bool) ([]entities.ListContainer, error) { // nolint:typecheck +func List(ctx context.Context, filters map[string][]string, all *bool, last *int, size, sync *bool) ([]entities.ListContainer, error) { // nolint:typecheck conn, err := bindings.GetClient(ctx) if err != nil { return nil, err @@ -37,9 +37,6 @@ func List(ctx context.Context, filters map[string][]string, all *bool, last *int if last != nil { params.Set("last", strconv.Itoa(*last)) } - if pod != nil { - params.Set("pod", strconv.FormatBool(*pod)) - } if size != nil { params.Set("size", strconv.FormatBool(*size)) } diff --git a/pkg/bindings/test/containers_test.go b/pkg/bindings/test/containers_test.go index 4b2c78353..0685a3377 100644 --- a/pkg/bindings/test/containers_test.go +++ b/pkg/bindings/test/containers_test.go @@ -509,7 +509,7 @@ var _ = Describe("Podman containers ", func() { Expect(err).To(BeNil()) _, err = bt.RunTopContainer(&name2, bindings.PFalse, nil) Expect(err).To(BeNil()) - containerLatestList, err := containers.List(bt.conn, nil, nil, &latestContainers, nil, nil, nil) + containerLatestList, err := containers.List(bt.conn, nil, nil, &latestContainers, nil, nil) Expect(err).To(BeNil()) err = containers.Kill(bt.conn, containerLatestList[0].Names[0], "SIGTERM") Expect(err).To(BeNil()) @@ -754,8 +754,23 @@ var _ = Describe("Podman containers ", func() { // Validate list container with id filter filters := make(map[string][]string) filters["id"] = []string{cid} - c, err := containers.List(bt.conn, filters, bindings.PTrue, nil, nil, nil, nil) + c, err := containers.List(bt.conn, filters, bindings.PTrue, nil, nil, nil) Expect(err).To(BeNil()) Expect(len(c)).To(Equal(1)) }) + + It("List containers always includes pod information", func() { + podName := "testpod" + ctrName := "testctr" + bt.Podcreate(&podName) + _, err := bt.RunTopContainer(&ctrName, bindings.PTrue, &podName) + Expect(err).To(BeNil()) + + lastNum := 1 + + c, err := containers.List(bt.conn, nil, bindings.PTrue, &lastNum, nil, nil) + Expect(err).To(BeNil()) + Expect(len(c)).To(Equal(1)) + Expect(c[0].PodName).To(Equal(podName)) + }) }) diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 8835248ca..dd72064a3 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -496,7 +496,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri } func (ic *ContainerEngine) ContainerList(ctx context.Context, options entities.ContainerListOptions) ([]entities.ListContainer, error) { - return containers.List(ic.ClientCxt, options.Filters, &options.All, &options.Last, &options.Pod, &options.Size, &options.Sync) + return containers.List(ic.ClientCxt, options.Filters, &options.All, &options.Last, &options.Size, &options.Sync) } func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.ContainerRunOptions) (*entities.ContainerRunReport, error) { diff --git a/pkg/domain/infra/tunnel/helpers.go b/pkg/domain/infra/tunnel/helpers.go index 9974c4d1d..90e52c5c0 100644 --- a/pkg/domain/infra/tunnel/helpers.go +++ b/pkg/domain/infra/tunnel/helpers.go @@ -20,7 +20,7 @@ func getContainersByContext(contextWithConnection context.Context, all bool, nam if all && len(namesOrIDs) > 0 { return nil, errors.New("cannot lookup containers and all") } - c, err := containers.List(contextWithConnection, nil, bindings.PTrue, nil, nil, nil, bindings.PTrue) + c, err := containers.List(contextWithConnection, nil, bindings.PTrue, nil, nil, bindings.PTrue) if err != nil { return nil, err } diff --git a/pkg/ps/ps.go b/pkg/ps/ps.go index 2b81311af..d4fd96596 100644 --- a/pkg/ps/ps.go +++ b/pkg/ps/ps.go @@ -175,11 +175,14 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities State: conState.String(), } if opts.Pod && len(conConfig.Pod) > 0 { - pod, err := rt.GetPod(conConfig.Pod) + podName, err := rt.GetName(conConfig.Pod) if err != nil { + if errors.Cause(err) == define.ErrNoSuchCtr { + return entities.ListContainer{}, errors.Wrapf(define.ErrNoSuchPod, "could not find container %s pod (id %s) in state", conConfig.ID, conConfig.Pod) + } return entities.ListContainer{}, err } - ps.PodName = pod.Name() + ps.PodName = podName } if opts.Namespace { -- cgit v1.2.3-54-g00ecf From a8a3325445fe2120896be997f74e426b36f11b9c Mon Sep 17 00:00:00 2001 From: TomSweeneyRedHat Date: Thu, 23 Jul 2020 19:27:33 -0400 Subject: [CI:DOCS] BZ1860126 - Fix userns defaults in run man page Addresses the multiple "default" userns values found in the podman-run(1) man page: http://docs.podman.io/en/latest/markdown/podman-run.1.html. This in response to: https://bugzilla.redhat.com/show_bug.cgi?id=1860126 which this PR wil fix. Signed-off-by: TomSweeneyRedHat --- docs/source/markdown/podman-run.1.md | 10 ++++++---- pkg/namespaces/namespaces.go | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'pkg') diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md index a500bfc41..25bb44c06 100644 --- a/docs/source/markdown/podman-run.1.md +++ b/docs/source/markdown/podman-run.1.md @@ -875,20 +875,22 @@ Ulimit options. You can use **host** to copy the current configuration from the Sets the username or UID used and optionally the groupname or GID for the specified command. -Without this argument the command will be run as root in the container. +Without this argument, the command will run as the user specified in the container image. Unless overridden by a `USER` command in the Containerfile or by a value passed to this option, this user generally defaults to root. + +When a user namespace is not in use, the UID and GID used within the container and on the host will match. When user namespaces are in use, however, the UID and GID in the container may correspond to another UID and GID on the host. In rootless containers, for example, a user namespace is always used, and root in the container will by default correspond to the UID and GID of the user invoking Podman. **--userns**=**auto**|**host**|**keep-id**|**container:**_id_|**ns:**_namespace_ -Set the user namespace mode for the container. It defaults to the **PODMAN_USERNS** environment variable. An empty value means user namespaces are disabled. +Set the user namespace mode for the container. It defaults to the **PODMAN_USERNS** environment variable. An empty value ("") means user namespaces are disabled unless an explicit mapping is set with they `--uidmapping` and `--gidmapping` options. - **auto**: automatically create a namespace. It is possible to specify other options to `auto`. The supported options are **size=SIZE** to specify an explicit size for the automatic user namespace. e.g. `--userns=auto:size=8192`. If `size` is not specified, `auto` will guess a size for the user namespace. **uidmapping=HOST_UID:CONTAINER_UID:SIZE** to force a UID mapping to be present in the user namespace. **gidmapping=HOST_UID:CONTAINER_UID:SIZE** to force a GID mapping to be present in the user namespace. -- **host**: run in the user namespace of the caller. This is the default if no user namespace options are set. The processes running in the container will have the same privileges on the host as any other process launched by the calling user. +- **host**: run in the user namespace of the caller. The processes running in the container will have the same privileges on the host as any other process launched by the calling user (default). - **keep-id**: creates a user namespace where the current rootless user's UID:GID are mapped to the same values in the container. This option is ignored for containers created by the root user. - **ns**: run the container in the given existing user namespace. -- **private**: create a new namespace for the container (default) +- **private**: create a new namespace for the container. - **container**: join the user namespace of the specified container. This option is incompatible with **--gidmap**, **--uidmap**, **--subuid** and **--subgid**. diff --git a/pkg/namespaces/namespaces.go b/pkg/namespaces/namespaces.go index 2ffbde977..c2f255530 100644 --- a/pkg/namespaces/namespaces.go +++ b/pkg/namespaces/namespaces.go @@ -91,7 +91,7 @@ func (n UsernsMode) IsHost() bool { return n == hostType } -// IsKeepID indicates whether container uses a mapping where the (uid, gid) on the host is lept inside of the namespace. +// IsKeepID indicates whether container uses a mapping where the (uid, gid) on the host is kept inside of the namespace. func (n UsernsMode) IsKeepID() bool { return n == "keep-id" } -- cgit v1.2.3-54-g00ecf From 23348e7f313a570c513772a88db7f2741c8242ba Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Fri, 31 Jul 2020 13:52:14 -0400 Subject: 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 --- cmd/podman/common/specgen.go | 5 ++--- pkg/env/env.go | 14 ++++++++------ pkg/spec/spec.go | 4 ++-- pkg/specgen/generate/container.go | 9 +++++++++ pkg/specgen/generate/oci.go | 1 - 5 files changed, 21 insertions(+), 12 deletions(-) (limited to 'pkg') diff --git a/cmd/podman/common/specgen.go b/cmd/podman/common/specgen.go index 48a2069ff..2074ed4fa 100644 --- a/cmd/podman/common/specgen.go +++ b/cmd/podman/common/specgen.go @@ -308,9 +308,8 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *ContainerCLIOpts, args []string // // Precedence order (higher index wins): // 1) env-host, 2) image data, 3) env-file, 4) env - env := map[string]string{ - "container": "podman", - } + env := make(map[string]string) + env["container"] = "podman" // First transform the os env into a map. We need it for the labels later in // any case. diff --git a/pkg/env/env.go b/pkg/env/env.go index a16007a50..0d55e5560 100644 --- a/pkg/env/env.go +++ b/pkg/env/env.go @@ -12,14 +12,16 @@ import ( "github.com/pkg/errors" ) -// DefaultEnvVariables sets $PATH and $TERM. -var DefaultEnvVariables = map[string]string{ - "PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", - "TERM": "xterm", -} - const whiteSpaces = " \t" +// DefaultEnvVariables returns a default environment, with $PATH and $TERM set. +func DefaultEnvVariables() map[string]string { + return map[string]string{ + "PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "TERM": "xterm", + } +} + // Slice transforms the specified map of environment variables into a // slice. If a value is non-empty, the key and value are joined with '='. func Slice(m map[string]string) []string { diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index b974772d5..0dbdc76bb 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -321,13 +321,13 @@ func (config *CreateConfig) createConfigToOCISpec(runtime *libpod.Runtime, userM // config. var defaultEnv map[string]string if runtimeConfig == nil { - defaultEnv = env.DefaultEnvVariables + defaultEnv = env.DefaultEnvVariables() } else { defaultEnv, err = env.ParseSlice(runtimeConfig.Containers.Env) if err != nil { return nil, errors.Wrap(err, "Env fields in containers.conf failed ot parse") } - defaultEnv = env.Join(env.DefaultEnvVariables, defaultEnv) + defaultEnv = env.Join(env.DefaultEnvVariables(), defaultEnv) } if err := addRlimits(config, &g); err != nil { diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go index 59fee80f1..06ffa3df6 100644 --- a/pkg/specgen/generate/container.go +++ b/pkg/specgen/generate/container.go @@ -86,6 +86,15 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat s.Env = envLib.Join(envLib.Join(defaultEnvs, envs), s.Env) + // Ensure that default environment variables are populated. + // Container must have PATH and TERM set, even if nothing else set them. + baseEnv := envLib.DefaultEnvVariables() + for k, v := range baseEnv { + if _, ok := s.Env[k]; !ok { + s.Env[k] = v + } + } + // Labels and Annotations annotations := make(map[string]string) if newImage != nil { diff --git a/pkg/specgen/generate/oci.go b/pkg/specgen/generate/oci.go index f1c9f2a1a..aefc7204c 100644 --- a/pkg/specgen/generate/oci.go +++ b/pkg/specgen/generate/oci.go @@ -260,7 +260,6 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt for key, val := range s.Annotations { g.AddAnnotation(key, val) } - g.AddProcessEnv("container", "podman") g.Config.Linux.Resources = s.ResourceLimits -- cgit v1.2.3-54-g00ecf From b5b782f2584e59534ae1d66551de0e04fc3c0038 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 18 Aug 2020 11:28:46 +0200 Subject: generate systemd: quote arguments with whitespace Make sure that arguments with whitespace are properly quoted so they are interpreted as one (and not multiple ones) by systemd. Now `-e tz="america/new york"` will be generated as `-e "tz=america/new york"`. The quotes are moving but the argument is still correct. Fixes: #7285 Signed-off-by: Valentin Rothberg --- pkg/systemd/generate/common.go | 13 +++++++++++++ pkg/systemd/generate/common_test.go | 25 +++++++++++++++++++++++++ pkg/systemd/generate/containers.go | 1 + pkg/systemd/generate/containers_test.go | 4 ++-- pkg/systemd/generate/pods.go | 1 + pkg/systemd/generate/pods_test.go | 4 ++-- 6 files changed, 44 insertions(+), 4 deletions(-) (limited to 'pkg') diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go index d6d18a810..1fc4479ff 100644 --- a/pkg/systemd/generate/common.go +++ b/pkg/systemd/generate/common.go @@ -1,6 +1,7 @@ package generate import ( + "strconv" "strings" "github.com/pkg/errors" @@ -53,3 +54,15 @@ func filterPodFlags(command []string) []string { } return processed } + +// quoteArguments makes sure that all arguments with at least one whitespace +// are quoted to make sure those are interpreted as one argument instead of +// multiple ones. +func quoteArguments(command []string) []string { + for i := range command { + if strings.ContainsAny(command[i], " \t") { + command[i] = strconv.Quote(command[i]) + } + } + return command +} diff --git a/pkg/systemd/generate/common_test.go b/pkg/systemd/generate/common_test.go index 389c30f59..d0ec5637c 100644 --- a/pkg/systemd/generate/common_test.go +++ b/pkg/systemd/generate/common_test.go @@ -28,3 +28,28 @@ func TestFilterPodFlags(t *testing.T) { } } } + +func TestQuoteArguments(t *testing.T) { + tests := []struct { + input []string + output []string + }{ + { + []string{"foo", "bar=\"arg\""}, + []string{"foo", "bar=\"arg\""}, + }, + { + []string{"foo", "bar=\"arg with space\""}, + []string{"foo", "\"bar=\\\"arg with space\\\"\""}, + }, + { + []string{"foo", "bar=\"arg with\ttab\""}, + []string{"foo", "\"bar=\\\"arg with\\ttab\\\"\""}, + }, + } + + for _, test := range tests { + quoted := quoteArguments(test.input) + assert.Equal(t, test.output, quoted) + } +} diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index b77fb5e11..068bb3c82 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -241,6 +241,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst startCommand = append(startCommand, "--replace") } startCommand = append(startCommand, info.CreateCommand[index:]...) + startCommand = quoteArguments(startCommand) info.ExecStartPre = "/bin/rm -f {{.PIDFile}} {{.ContainerIDFile}}" info.ExecStart = strings.Join(startCommand, " ") diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go index 13138c0cd..73fc5e856 100644 --- a/pkg/systemd/generate/containers_test.go +++ b/pkg/systemd/generate/containers_test.go @@ -117,7 +117,7 @@ After=network-online.target Environment=PODMAN_SYSTEMD_UNIT=%n Restart=always ExecStartPre=/bin/rm -f %t/jadda-jadda.pid %t/jadda-jadda.ctr-id -ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon -d --replace --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN +ExecStart=/usr/bin/podman run --conmon-pidfile %t/jadda-jadda.pid --cidfile %t/jadda-jadda.ctr-id --cgroups=no-conmon -d --replace --name jadda-jadda --hostname hello-world awesome-image:latest command arg1 ... argN "foo=arg \"with \" space" ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 42 ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id PIDFile=%t/jadda-jadda.pid @@ -296,7 +296,7 @@ WantedBy=multi-user.target default.target` PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", StopTimeout: 42, PodmanVersion: "CI", - CreateCommand: []string{"I'll get stripped", "container", "run", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN"}, + CreateCommand: []string{"I'll get stripped", "container", "run", "--name", "jadda-jadda", "--hostname", "hello-world", "awesome-image:latest", "command", "arg1", "...", "argN", "foo=arg \"with \" space"}, EnvVariable: EnvVariable, }, goodWithNameAndGeneric, diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go index 1c7ef7792..df807585a 100644 --- a/pkg/systemd/generate/pods.go +++ b/pkg/systemd/generate/pods.go @@ -292,6 +292,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions) } startCommand = append(startCommand, podCreateArgs...) + startCommand = quoteArguments(startCommand) info.ExecStartPre1 = "/bin/rm -f {{.PIDFile}} {{.PodIDFile}}" info.ExecStartPre2 = strings.Join(startCommand, " ") diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go index 4089b44e5..9754d3215 100644 --- a/pkg/systemd/generate/pods_test.go +++ b/pkg/systemd/generate/pods_test.go @@ -75,7 +75,7 @@ Before=container-1.service container-2.service Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure ExecStartPre=/bin/rm -f %t/pod-123abc.pid %t/pod-123abc.pod-id -ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --name foo --replace +ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-123abc.pid --pod-id-file %t/pod-123abc.pod-id --name foo "bar=arg with space" --replace ExecStart=/usr/bin/podman pod start --pod-id-file %t/pod-123abc.pod-id ExecStop=/usr/bin/podman pod stop --ignore --pod-id-file %t/pod-123abc.pod-id -t 10 ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-id @@ -118,7 +118,7 @@ WantedBy=multi-user.target default.target` StopTimeout: 10, PodmanVersion: "CI", RequiredServices: []string{"container-1", "container-2"}, - CreateCommand: []string{"podman", "pod", "create", "--name", "foo"}, + CreateCommand: []string{"podman", "pod", "create", "--name", "foo", "bar=arg with space"}, }, podGoodNamedNew, true, -- cgit v1.2.3-54-g00ecf From ce1389bde7c62ef4eaf9ed8b2f0a68a4c14bbb9d Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 17 Aug 2020 15:58:16 +0200 Subject: abi: fix detection for systemd create a scope everytime we don't own the current cgroup and we are running on systemd. Closes: https://github.com/containers/podman/issues/6734 Signed-off-by: Giuseppe Scrivano --- pkg/domain/infra/abi/system.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'pkg') diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go index 435902ded..043dfe49e 100644 --- a/pkg/domain/infra/abi/system.go +++ b/pkg/domain/infra/abi/system.go @@ -70,8 +70,13 @@ func (ic *ContainerEngine) SetupRootless(_ context.Context, cmd *cobra.Command) if err != nil { return err } + + initCommand, err := ioutil.ReadFile("/proc/1/comm") + // On errors, default to systemd + runsUnderSystemd := err != nil || string(initCommand) == "systemd" + unitName := fmt.Sprintf("podman-%d.scope", os.Getpid()) - if conf.Engine.CgroupManager == config.SystemdCgroupsManager { + if runsUnderSystemd || conf.Engine.CgroupManager == config.SystemdCgroupsManager { if err := utils.RunUnderSystemdScope(os.Getpid(), "user.slice", unitName); err != nil { logrus.Warnf("Failed to add podman to systemd sandbox cgroup: %v", err) } -- cgit v1.2.3-54-g00ecf From 0ef668878572986951df57fb38596683e5750d07 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 18 Aug 2020 12:19:28 +0200 Subject: 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 --- docs/source/markdown/podman-create.1.md | 14 ++++++++------ docs/source/markdown/podman-run.1.md | 9 ++++----- pkg/specgen/container_validate.go | 3 +++ 3 files changed, 15 insertions(+), 11 deletions(-) (limited to 'pkg') diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md index ffce338b4..00f16d0a3 100644 --- a/docs/source/markdown/podman-create.1.md +++ b/docs/source/markdown/podman-create.1.md @@ -336,7 +336,7 @@ value can be expressed in a time format such as `1m22s`. The default value is ` Container host name -Sets the container host name that is available inside the container. +Sets the container host name that is available inside the container. Can only be used with a private UTS namespace `--uts=private` (default). If `--pod` is specified and the pod shares the UTS namespace (default) the pods hostname will be used. **--help** @@ -862,12 +862,14 @@ Set the user namespace mode for the container. It defaults to the **PODMAN_USER This option is incompatible with --gidmap, --uidmap, --subuid and --subgid -**--uts**=*host* +**--uts**=*mode* -Set the UTS mode for the container - **host**: use the host's UTS namespace inside the container. - **ns**: specify the user namespace to use. - Note: the host mode gives the container access to changing the host's hostname and is therefore considered insecure. +Set the UTS namespace mode for the container. The following values are supported: + +- **host**: use the host's UTS namespace inside the container. +- **private**: create a new namespace for the container (default). +- **ns:[path]**: run the container in the given existing UTS namespace. +- **container:[container]**: join the UTS namespace of the specified container. **--volume**, **-v**[=*[[SOURCE-VOLUME|HOST-DIR:]CONTAINER-DIR[:OPTIONS]]*] diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md index 25bb44c06..5febf1c66 100644 --- a/docs/source/markdown/podman-run.1.md +++ b/docs/source/markdown/podman-run.1.md @@ -356,7 +356,7 @@ Print usage statement Container host name -Sets the container host name that is available inside the container. +Sets the container host name that is available inside the container. Can only be used with a private UTS namespace `--uts=private` (default). If `--pod` is specified and the pod shares the UTS namespace (default) the pods hostname will be used. **--http-proxy**=**true**|**false** @@ -900,10 +900,9 @@ This option is incompatible with **--gidmap**, **--uidmap**, **--subuid** and ** Set the UTS namespace mode for the container. The following values are supported: - **host**: use the host's UTS namespace inside the container. -- **private**: create a new namespace for the container (default) -- **ns**: use own UTS namespace. - -**NOTE**: the host mode gives the container access to changing the host's hostname and is therefore considered insecure. +- **private**: create a new namespace for the container (default). +- **ns:[path]**: run the container in the given existing UTS namespace. +- **container:[container]**: join the UTS namespace of the specified container. **--volume**, **-v**[=[[_source-volume_|_host-dir_:]_container-dir_[:_options_]]] diff --git a/pkg/specgen/container_validate.go b/pkg/specgen/container_validate.go index a979a7f4a..4dd2ab0b3 100644 --- a/pkg/specgen/container_validate.go +++ b/pkg/specgen/container_validate.go @@ -43,6 +43,9 @@ func (s *SpecGenerator) Validate() error { } // Cannot set hostname and utsns if len(s.ContainerBasicConfig.Hostname) > 0 && !s.ContainerBasicConfig.UtsNS.IsPrivate() { + if s.ContainerBasicConfig.UtsNS.IsPod() { + return errors.Wrap(ErrInvalidSpecConfig, "cannot set hostname when joining the pod UTS namespace") + } return errors.Wrap(ErrInvalidSpecConfig, "cannot set hostname when running in the host UTS namespace") } // systemd values must be true, false, or always -- cgit v1.2.3-54-g00ecf