Skip to content

Commit

Permalink
Merge pull request #9178 from Luap99/fix-9176
Browse files Browse the repository at this point in the history
Fix podman generate systemd --new special char handling
  • Loading branch information
openshift-merge-robot authored Feb 1, 2021
2 parents 2018334 + 5352df2 commit 182e841
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 9 deletions.
14 changes: 11 additions & 3 deletions pkg/systemd/generate/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,21 @@ func filterPodFlags(command []string) []string {
return processed
}

// quoteArguments makes sure that all arguments with at least one whitespace
// escapeSystemdArguments 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 {
// multiple ones. Also make sure to escape all characters which have a special
// meaning to systemd -> $,% and \
// 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], `\`, `\\`)
}
}
return command
Expand Down
40 changes: 38 additions & 2 deletions pkg/systemd/generate/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestFilterPodFlags(t *testing.T) {
}
}

func TestQuoteArguments(t *testing.T) {
func TestEscapeSystemdArguments(t *testing.T) {
tests := []struct {
input []string
output []string
Expand All @@ -46,10 +46,46 @@ func TestQuoteArguments(t *testing.T) {
[]string{"foo", "bar=\"arg with\ttab\""},
[]string{"foo", "\"bar=\\\"arg with\\ttab\\\"\""},
},
{
[]string{"$"},
[]string{"$$"},
},
{
[]string{"foo", "command with dollar sign $"},
[]string{"foo", "\"command with dollar sign $$\""},
},
{
[]string{"foo", "command with two dollar signs $$"},
[]string{"foo", "\"command with two dollar signs $$$$\""},
},
{
[]string{"%"},
[]string{"%%"},
},
{
[]string{"foo", "command with percent sign %"},
[]string{"foo", "\"command with percent sign %%\""},
},
{
[]string{"foo", "command with two percent signs %%"},
[]string{"foo", "\"command with two percent signs %%%%\""},
},
{
[]string{`\`},
[]string{`\\`},
},
{
[]string{"foo", `command with backslash \`},
[]string{"foo", `"command with backslash \\"`},
},
{
[]string{"foo", `command with two backslashs \\`},
[]string{"foo", `"command with two backslashs \\\\"`},
},
}

for _, test := range tests {
quoted := quoteArguments(test.input)
quoted := escapeSystemdArguments(test.input)
assert.Equal(t, test.output, quoted)
}
}
4 changes: 2 additions & 2 deletions pkg/systemd/generate/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
startCommand := []string{info.Executable}
if index > 2 {
// include root flags
info.RootFlags = strings.Join(quoteArguments(info.CreateCommand[1:index-1]), " ")
info.RootFlags = strings.Join(escapeSystemdArguments(info.CreateCommand[1:index-1]), " ")
startCommand = append(startCommand, info.CreateCommand[1:index-1]...)
}
startCommand = append(startCommand,
Expand Down Expand Up @@ -279,7 +279,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
}
}
startCommand = append(startCommand, remainingCmd...)
startCommand = quoteArguments(startCommand)
startCommand = escapeSystemdArguments(startCommand)

info.ExecStartPre = "/bin/rm -f {{{{.PIDFile}}}} {{{{.ContainerIDFile}}}}"
info.ExecStart = strings.Join(startCommand, " ")
Expand Down
40 changes: 40 additions & 0 deletions pkg/systemd/generate/containers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,30 @@ 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
`

goodNewWithSpecialChars := `# 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
[Service]
Environment=PODMAN_SYSTEMD_UNIT=%n
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 --replace --name test awesome-image:latest sh -c "kill $$$$ && echo %%\\"
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 @@ -647,6 +671,22 @@ WantedBy=multi-user.target default.target
true,
false,
},
{"good with special chars",
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",
CreateCommand: []string{"I'll get stripped", "create", "--name", "test", "awesome-image:latest", "sh", "-c", "kill $$ && echo %\\"},
EnvVariable: EnvVariable,
},
goodNewWithSpecialChars,
true,
false,
},
}
for _, tt := range tests {
test := tt
Expand Down
4 changes: 2 additions & 2 deletions pkg/systemd/generate/pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
return "", errors.Errorf("pod does not appear to be created via `podman pod create`: %v", info.CreateCommand)
}
podRootArgs = info.CreateCommand[1 : podCreateIndex-1]
info.RootFlags = strings.Join(quoteArguments(podRootArgs), " ")
info.RootFlags = strings.Join(escapeSystemdArguments(podRootArgs), " ")
podCreateArgs = filterPodFlags(info.CreateCommand[podCreateIndex+1:])
}
// We're hard-coding the first five arguments and append the
Expand Down Expand Up @@ -306,7 +306,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
}

startCommand = append(startCommand, podCreateArgs...)
startCommand = quoteArguments(startCommand)
startCommand = escapeSystemdArguments(startCommand)

info.ExecStartPre1 = "/bin/rm -f {{{{.PIDFile}}}} {{{{.PodIDFile}}}}"
info.ExecStartPre2 = strings.Join(startCommand, " ")
Expand Down

0 comments on commit 182e841

Please sign in to comment.