Skip to content

Commit

Permalink
Add envars to the generated systemd unit
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
Paul Holzinger committed May 10, 2021
1 parent 54bed10 commit 77e6ae2
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 9 deletions.
23 changes: 14 additions & 9 deletions pkg/systemd/generate/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions pkg/systemd/generate/containers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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}}}}
Expand Down Expand Up @@ -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,
Expand All @@ -163,6 +174,7 @@ func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSyste
CreateCommand: createCommand,
GraphRoot: graphRoot,
RunRoot: runRoot,
containerEnv: envs,
}

return &info, nil
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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)

Expand Down
46 changes: 46 additions & 0 deletions pkg/systemd/generate/containers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
`
Expand Down Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions test/system/250-systemd.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 77e6ae2

Please sign in to comment.