From 77e6ae24369e6c7bed85141ae6f7d0c7b0e26c0b Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 5 May 2021 22:51:23 +0200 Subject: [PATCH] Add envars to the generated systemd unit The with --new generated systemd unit loses the environment variables when the create command only contains the key without the value. Since podman tries to lookup those values from the environment the unit can fail. This commits ensures that we will add the environment variables to the unit file when this is the case. The container environment variables are looked up in the container spec. Fixes #10101 Signed-off-by: Paul Holzinger --- pkg/systemd/generate/common.go | 23 ++++++++----- pkg/systemd/generate/containers.go | 31 +++++++++++++++++ pkg/systemd/generate/containers_test.go | 46 +++++++++++++++++++++++++ test/system/250-systemd.bats | 23 +++++++++++++ 4 files changed, 114 insertions(+), 9 deletions(-) diff --git a/pkg/systemd/generate/common.go b/pkg/systemd/generate/common.go index eafd45528b..0f667e2f4f 100644 --- a/pkg/systemd/generate/common.go +++ b/pkg/systemd/generate/common.go @@ -89,19 +89,24 @@ func filterCommonContainerFlags(command []string, argCount int) []string { // see: https://www.freedesktop.org/software/systemd/man/systemd.service.html#Command%20lines func escapeSystemdArguments(command []string) []string { for i := range command { - command[i] = strings.ReplaceAll(command[i], "$", "$$") - command[i] = strings.ReplaceAll(command[i], "%", "%%") - if strings.ContainsAny(command[i], " \t") { - command[i] = strconv.Quote(command[i]) - } else if strings.Contains(command[i], `\`) { - // strconv.Quote also escapes backslashes so - // we should replace only if strconv.Quote was not used - command[i] = strings.ReplaceAll(command[i], `\`, `\\`) - } + command[i] = escapeSystemdArg(command[i]) } return command } +func escapeSystemdArg(arg string) string { + arg = strings.ReplaceAll(arg, "$", "$$") + arg = strings.ReplaceAll(arg, "%", "%%") + if strings.ContainsAny(arg, " \t") { + arg = strconv.Quote(arg) + } else if strings.Contains(arg, `\`) { + // strconv.Quote also escapes backslashes so + // we should replace only if strconv.Quote was not used + arg = strings.ReplaceAll(arg, `\`, `\\`) + } + return arg +} + func removeDetachArg(args []string, argCount int) []string { // "--detach=false" could also be in the container entrypoint // split them off so we do not remove it there diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index e06655a8d5..eb1fb67fff 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -54,6 +54,12 @@ type containerInfo struct { // CreateCommand is the full command plus arguments of the process the // container has been created with. CreateCommand []string + // containerEnv stores the container environment variables + containerEnv []string + // ExtraEnvs contains the container environment variables referenced + // by only the key in the container create command, e.g. --env FOO. + // This is only used with --new + ExtraEnvs []string // EnvVariable is generate.EnvVariable and must not be set. EnvVariable string // ExecStartPre of the unit. @@ -87,6 +93,9 @@ After={{{{- range $index, $value := .BoundToServices -}}}}{{{{if $index}}}} {{{{ [Service] Environment={{{{.EnvVariable}}}}=%n +{{{{- if .ExtraEnvs}}}} +Environment={{{{- range $index, $value := .ExtraEnvs -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}{{{{end}}}} +{{{{- end}}}} Restart={{{{.RestartPolicy}}}} TimeoutStopSec={{{{.TimeoutStopSec}}}} {{{{- if .ExecStartPre}}}} @@ -153,6 +162,8 @@ func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSyste return nil, errors.Errorf("could not lookup container's runroot: got empty string") } + envs := config.Spec.Process.Env + info := containerInfo{ ServiceName: serviceName, ContainerNameOrID: nameOrID, @@ -163,6 +174,7 @@ func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSyste CreateCommand: createCommand, GraphRoot: graphRoot, RunRoot: runRoot, + containerEnv: envs, } return &info, nil @@ -248,6 +260,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst fs.BoolP("detach", "d", false, "") fs.String("name", "", "") fs.Bool("replace", false, "") + fs.StringArrayP("env", "e", nil, "") fs.Parse(remainingCmd) remainingCmd = filterCommonContainerFlags(remainingCmd, fs.NArg()) @@ -304,6 +317,24 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst remainingCmd = removeReplaceArg(remainingCmd, fs.NArg()) } } + + envs, err := fs.GetStringArray("env") + if err != nil { + return "", err + } + for _, env := range envs { + // if env arg does not contain a equal sign we have to add the envar to the unit + // because it does try to red the value from the environment + if !strings.Contains(env, "=") { + for _, containerEnv := range info.containerEnv { + split := strings.SplitN(containerEnv, "=", 2) + if split[0] == env { + info.ExtraEnvs = append(info.ExtraEnvs, escapeSystemdArg(containerEnv)) + } + } + } + } + startCommand = append(startCommand, remainingCmd...) startCommand = escapeSystemdArguments(startCommand) diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go index 899ba6bfa1..75b08526bf 100644 --- a/pkg/systemd/generate/containers_test.go +++ b/pkg/systemd/generate/containers_test.go @@ -442,6 +442,32 @@ ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id PIDFile=%t/jadda-jadda.pid Type=forking +[Install] +WantedBy=multi-user.target default.target +` + + goodNewWithEnvar := `# jadda-jadda.service +# autogenerated by Podman CI + +[Unit] +Description=Podman jadda-jadda.service +Documentation=man:podman-generate-systemd(1) +Wants=network.target +After=network-online.target +RequiresMountsFor=/var/lib/containers/storage /var/run/containers/storage + +[Service] +Environment=PODMAN_SYSTEMD_UNIT=%n +Environment=FOO=abc "BAR=my test" USER=%%a +Restart=always +TimeoutStopSec=70 +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 --env FOO --env=BAR --env=MYENV=2 -e USER awesome-image:latest +ExecStop=/usr/bin/podman stop --ignore --cidfile %t/jadda-jadda.ctr-id -t 10 +ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/jadda-jadda.ctr-id +PIDFile=%t/jadda-jadda.pid +Type=forking + [Install] WantedBy=multi-user.target default.target ` @@ -873,6 +899,26 @@ WantedBy=multi-user.target default.target false, false, }, + {"good with environment variables", + containerInfo{ + Executable: "/usr/bin/podman", + ServiceName: "jadda-jadda", + ContainerNameOrID: "jadda-jadda", + RestartPolicy: "always", + PIDFile: "/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid", + StopTimeout: 10, + PodmanVersion: "CI", + GraphRoot: "/var/lib/containers/storage", + RunRoot: "/var/run/containers/storage", + CreateCommand: []string{"I'll get stripped", "create", "--env", "FOO", "--env=BAR", "--env=MYENV=2", "-e", "USER", "awesome-image:latest"}, + containerEnv: []string{"FOO=abc", "BAR=my test", "USER=%a", "MYENV=2"}, + EnvVariable: define.EnvVariable, + }, + goodNewWithEnvar, + true, + false, + false, + }, } for _, tt := range tests { test := tt diff --git a/test/system/250-systemd.bats b/test/system/250-systemd.bats index b42769409e..4ea1920098 100644 --- a/test/system/250-systemd.bats +++ b/test/system/250-systemd.bats @@ -125,4 +125,27 @@ function service_cleanup() { service_cleanup } +# These tests can fail in dev. environment because of SELinux. +# quick fix: chcon -t container_runtime_exec_t ./bin/podman +@test "podman generate systemd - envar" { + xdg_rootless + + cname=$(random_string) + FOO=value BAR=%s run_podman create --name $cname --env FOO -e BAR --env MYVAR=myval \ + $IMAGE sh -c 'printenv && sleep 100' + + # Start systemd service to run this container + service_setup + + # Give container time to start; make sure output looks top-like + sleep 2 + run_podman logs $cname + is "$output" ".*FOO=value.*" "FOO environment variable set" + is "$output" ".*BAR=%s.*" "BAR environment variable set" + is "$output" ".*MYVAR=myval.*" "MYVAL environment variable set" + + # All good. Stop service, clean up. + service_cleanup +} + # vim: filetype=sh