opensource/podman
1
0
Fork 0
mirror of https://github.com/containers/podman.git synced 2025-08-06 11:32:07 +08:00
Code Issues Packages Projects Releases Wiki Activity

libpod: pass entire environment to conmon

Browse Source 
Pass the _entire_ environment to conmon instead of selectively enabling
only specific variables.  The main reasoning is to make sure that conmon
and the podman-cleanup callback process operate in the exact same
environment than the initial podman process.  Some configuration files
may be passed via environment variables.  Podman not passing those down
to conmon has led to subtle and hard to debug issues in the past, so
passing all down will avoid such kinds of issues in the future.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
This commit is contained in:
Valentin Rothberg
2023-09-26 09:41:00 +02:00
parent a018fe7c1b
commit 7ade972102
3 changed files with 25 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files

73
libpod/oci_conmon_common.go
View File

@ -35,7 +35,6 @@ import (
"github.com/containers/podman/v4/pkg/specgenutil"
"github.com/containers/podman/v4/pkg/util"
"github.com/containers/podman/v4/utils"
"github.com/containers/storage/pkg/homedir"
spec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
@ -1042,11 +1041,6 @@ func (r *ConmonOCIRuntime) getLogTag(ctr *Container) (string, error) {
func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *ContainerCheckpointOptions) (int64, error) {
var stderrBuf bytes.Buffer
runtimeDir, err := util.GetRuntimeDir()
if err != nil {
return 0, err
}
parentSyncPipe, childSyncPipe, err := newPipe()
if err != nil {
return 0, fmt.Errorf("creating socket pair: %w", err)
@ -1189,7 +1183,10 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
}
// 0, 1 and 2 are stdin, stdout and stderr
conmonEnv := r.configureConmonEnv(runtimeDir)
conmonEnv, err := r.configureConmonEnv()
if err != nil {
return 0, fmt.Errorf("configuring conmon env: %w", err)
}
var filesToClose []*os.File
if preserveFDs > 0 {
@ -1251,7 +1248,7 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
if restoreOptions != nil {
runtimeRestoreStarted = time.Now()
}
err = startCommand(cmd, ctr)
err = cmd.Start()
// regardless of whether we errored or not, we no longer need the children pipes
childSyncPipe.Close()
@ -1311,38 +1308,23 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
}
// configureConmonEnv gets the environment values to add to conmon's exec struct
// TODO this may want to be less hardcoded/more configurable in the future
func (r *ConmonOCIRuntime) configureConmonEnv(runtimeDir string) []string {
var env []string
for _, e := range os.Environ() {
if strings.HasPrefix(e, "LC_") {
env = append(env, e)
}
if strings.HasPrefix(e, "LANG=") {
env = append(env, e)
func (r *ConmonOCIRuntime) configureConmonEnv() ([]string, error) {
env := os.Environ()
res := make([]string, 0, len(env))
for _, v := range env {
if strings.HasPrefix(v, "NOTIFY_SOCKET=") {
// The NOTIFY_SOCKET must not leak into the environment.
continue
}
res = append(res, v)
}
if path, ok := os.LookupEnv("PATH"); ok {
env = append(env, fmt.Sprintf("PATH=%s", path))
}
if conf, ok := os.LookupEnv("CONTAINERS_CONF"); ok {
env = append(env, fmt.Sprintf("CONTAINERS_CONF=%s", conf))
}
if conf, ok := os.LookupEnv("CONTAINERS_CONF_OVERRIDE"); ok {
env = append(env, fmt.Sprintf("CONTAINERS_CONF_OVERRIDE=%s", conf))
}
if conf, ok := os.LookupEnv("CONTAINERS_HELPER_BINARY_DIR"); ok {
env = append(env, fmt.Sprintf("CONTAINERS_HELPER_BINARY_DIR=%s", conf))
}
env = append(env, fmt.Sprintf("XDG_RUNTIME_DIR=%s", runtimeDir))
env = append(env, fmt.Sprintf("_CONTAINERS_USERNS_CONFIGURED=%s", os.Getenv("_CONTAINERS_USERNS_CONFIGURED")))
env = append(env, fmt.Sprintf("_CONTAINERS_ROOTLESS_UID=%s", os.Getenv("_CONTAINERS_ROOTLESS_UID")))
home := homedir.Get()
if home != "" {
env = append(env, fmt.Sprintf("HOME=%s", home))
runtimeDir, err := util.GetRuntimeDir()
if err != nil {
return nil, err
}
return env
res = append(res, "XDG_RUNTIME_DIR="+runtimeDir)
return res, nil
}
// sharedConmonArgs takes common arguments for exec and create/restore and formats them for the conmon CLI
@ -1422,25 +1404,6 @@ func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, p
return args
}
func startCommand(cmd *exec.Cmd, ctr *Container) error {
// Make sure to unset the NOTIFY_SOCKET and reset it afterwards if needed.
switch ctr.config.SdNotifyMode {
case define.SdNotifyModeContainer, define.SdNotifyModeIgnore:
if prev := os.Getenv("NOTIFY_SOCKET"); prev != "" {
if err := os.Unsetenv("NOTIFY_SOCKET"); err != nil {
logrus.Warnf("Error unsetting NOTIFY_SOCKET %v", err)
}
defer func() {
if err := os.Setenv("NOTIFY_SOCKET", prev); err != nil {
logrus.Errorf("Resetting NOTIFY_SOCKET=%s", prev)
}
}()
}
}
return cmd.Start()
}
// newPipe creates a unix socket pair for communication.
// Returns two files - first is parent, second is child.
func newPipe() (*os.File, *os.File, error) {

13
libpod/oci_conmon_exec_common.go
View File

@ -17,7 +17,6 @@ import (
"github.com/containers/podman/v4/libpod/define"
"github.com/containers/podman/v4/pkg/errorhandling"
"github.com/containers/podman/v4/pkg/lookup"
"github.com/containers/podman/v4/pkg/util"
spec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
@ -374,11 +373,6 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex
}
}()
runtimeDir, err := util.GetRuntimeDir()
if err != nil {
return nil, nil, err
}
finalEnv := make([]string, 0, len(options.Env))
for k, v := range options.Env {
finalEnv = append(finalEnv, fmt.Sprintf("%s=%s", k, v))
@ -438,7 +432,10 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex
// }
// }
conmonEnv := r.configureConmonEnv(runtimeDir)
conmonEnv, err := r.configureConmonEnv()
if err != nil {
return nil, nil, fmt.Errorf("configuring conmon env: %w", err)
}
var filesToClose []*os.File
if options.PreserveFDs > 0 {
@ -461,7 +458,7 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex
Setpgid: true,
}
err = startCommand(execCmd, c)
err = execCmd.Start()
// We don't need children pipes on the parent side
errorhandling.CloseQuiet(childSyncPipe)

3
test/system/030-run.bats
View File

@ -1288,7 +1288,7 @@ search | $IMAGE |
done < <(parse_table "$tests")
}
@test "podman --syslog passed to conmon" {
@test "podman --syslog and environment passed to conmon" {
skip_if_remote "--syslog is not supported for remote clients"
skip_if_journald_unavailable
@ -1298,6 +1298,7 @@ search | $IMAGE |
run_podman container inspect $cid --format "{{ .State.ConmonPid }}"
conmon_pid="$output"
is "$(< /proc/$conmon_pid/cmdline)" ".*--exit-command-arg--syslog.*" "conmon's exit-command has --syslog set"
assert "$(< /proc/$conmon_pid/environ)" =~ "BATS_TEST_TMPDIR" "entire env is passed down to conmon (incl. BATS variables)"
run_podman rm -f -t0 $cid
}