Make generate systemd --new robust against double curly braces

If the container create command contains an argument with double
curly braces the golang template parsing can fail since it tries
to interpret the value as variable. To fix this change the default
delimiter for the internal template to `{{{{`.

Fixes #9034

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
This commit is contained in:
Paul Holzinger
2021-01-20 14:30:42 +01:00
parent 7d024a2fc8
commit c3cbaa355c
6 changed files with 159 additions and 56 deletions

View File

@ -30,14 +30,14 @@ func validateRestartPolicy(restart string) error {
return errors.Errorf("%s is not a valid restart policy", restart) return errors.Errorf("%s is not a valid restart policy", restart)
} }
const headerTemplate = `# {{.ServiceName}}.service const headerTemplate = `# {{{{.ServiceName}}}}.service
# autogenerated by Podman {{.PodmanVersion}} # autogenerated by Podman {{{{.PodmanVersion}}}}
{{- if .TimeStamp}} {{{{- if .TimeStamp}}}}
# {{.TimeStamp}} # {{{{.TimeStamp}}}}
{{- end}} {{{{- end}}}}
[Unit] [Unit]
Description=Podman {{.ServiceName}}.service Description=Podman {{{{.ServiceName}}}}.service
Documentation=man:podman-generate-systemd(1) Documentation=man:podman-generate-systemd(1)
Wants=network.target Wants=network.target
After=network-online.target After=network-online.target

View File

@ -72,22 +72,22 @@ type containerInfo struct {
} }
const containerTemplate = headerTemplate + ` const containerTemplate = headerTemplate + `
{{- if .BoundToServices}} {{{{- if .BoundToServices}}}}
BindsTo={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} BindsTo={{{{- range $index, $value := .BoundToServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
After={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} After={{{{- range $index, $value := .BoundToServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
{{- end}} {{{{- end}}}}
[Service] [Service]
Environment={{.EnvVariable}}=%n Environment={{{{.EnvVariable}}}}=%n
Restart={{.RestartPolicy}} Restart={{{{.RestartPolicy}}}}
TimeoutStopSec={{.TimeoutStopSec}} TimeoutStopSec={{{{.TimeoutStopSec}}}}
{{- if .ExecStartPre}} {{{{- if .ExecStartPre}}}}
ExecStartPre={{.ExecStartPre}} ExecStartPre={{{{.ExecStartPre}}}}
{{- end}} {{{{- end}}}}
ExecStart={{.ExecStart}} ExecStart={{{{.ExecStart}}}}
ExecStop={{.ExecStop}} ExecStop={{{{.ExecStop}}}}
ExecStopPost={{.ExecStopPost}} ExecStopPost={{{{.ExecStopPost}}}}
PIDFile={{.PIDFile}} PIDFile={{{{.PIDFile}}}}
Type=forking Type=forking
[Install] [Install]
@ -173,9 +173,9 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
} }
info.EnvVariable = EnvVariable info.EnvVariable = EnvVariable
info.ExecStart = "{{.Executable}} start {{.ContainerNameOrID}}" info.ExecStart = "{{{{.Executable}}}} start {{{{.ContainerNameOrID}}}}"
info.ExecStop = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}}" info.ExecStop = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.ContainerNameOrID}}}}"
info.ExecStopPost = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.ContainerNameOrID}}" info.ExecStopPost = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.ContainerNameOrID}}}}"
// Assemble the ExecStart command when creating a new container. // Assemble the ExecStart command when creating a new container.
// //
@ -209,8 +209,8 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
} }
startCommand = append(startCommand, startCommand = append(startCommand,
"run", "run",
"--conmon-pidfile", "{{.PIDFile}}", "--conmon-pidfile", "{{{{.PIDFile}}}}",
"--cidfile", "{{.ContainerIDFile}}", "--cidfile", "{{{{.ContainerIDFile}}}}",
"--cgroups=no-conmon", "--cgroups=no-conmon",
) )
// If the container is in a pod, make sure that the // If the container is in a pod, make sure that the
@ -281,10 +281,10 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
startCommand = append(startCommand, remainingCmd...) startCommand = append(startCommand, remainingCmd...)
startCommand = quoteArguments(startCommand) startCommand = quoteArguments(startCommand)
info.ExecStartPre = "/bin/rm -f {{.PIDFile}} {{.ContainerIDFile}}" info.ExecStartPre = "/bin/rm -f {{{{.PIDFile}}}} {{{{.ContainerIDFile}}}}"
info.ExecStart = strings.Join(startCommand, " ") info.ExecStart = strings.Join(startCommand, " ")
info.ExecStop = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}stop --ignore --cidfile {{.ContainerIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}" info.ExecStop = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}stop --ignore --cidfile {{{{.ContainerIDFile}}}} {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}}"
info.ExecStopPost = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}rm --ignore -f --cidfile {{.ContainerIDFile}}" info.ExecStopPost = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}rm --ignore -f --cidfile {{{{.ContainerIDFile}}}}"
} }
info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout
@ -307,7 +307,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
// generation. That's especially needed for embedding the PID and ID // generation. That's especially needed for embedding the PID and ID
// files in other fields which will eventually get replaced in the 2nd // files in other fields which will eventually get replaced in the 2nd
// template execution. // template execution.
templ, err := template.New("container_template").Parse(containerTemplate) templ, err := template.New("container_template").Delims("{{{{", "}}}}").Parse(containerTemplate)
if err != nil { if err != nil {
return "", errors.Wrap(err, "error parsing systemd service template") return "", errors.Wrap(err, "error parsing systemd service template")
} }
@ -318,7 +318,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
} }
// Now parse the generated template (i.e., buf) and execute it. // Now parse the generated template (i.e., buf) and execute it.
templ, err = template.New("container_template").Parse(buf.String()) templ, err = template.New("container_template").Delims("{{{{", "}}}}").Parse(buf.String())
if err != nil { if err != nil {
return "", errors.Wrap(err, "error parsing systemd service template") return "", errors.Wrap(err, "error parsing systemd service template")
} }

View File

@ -329,6 +329,29 @@ Type=forking
WantedBy=multi-user.target default.target WantedBy=multi-user.target default.target
` `
goodNewWithJournaldTag := `# 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 --log-driver=journald --log-opt=tag={{.Name}} 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
`
tests := []struct { tests := []struct {
name string name string
info containerInfo info containerInfo
@ -608,6 +631,22 @@ WantedBy=multi-user.target default.target
true, true,
false, false,
}, },
{"good with journald log tag (see #9034)",
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", "--log-driver=journald", "--log-opt=tag={{.Name}}", "awesome-image:latest"},
EnvVariable: EnvVariable,
},
goodNewWithJournaldTag,
true,
false,
},
} }
for _, tt := range tests { for _, tt := range tests {
test := tt test := tt

View File

@ -72,23 +72,23 @@ type podInfo struct {
ExecStopPost string ExecStopPost string
} }
const podTemplate = headerTemplate + `Requires={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} const podTemplate = headerTemplate + `Requires={{{{- range $index, $value := .RequiredServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
Before={{- range $index, $value := .RequiredServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}} Before={{{{- range $index, $value := .RequiredServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
[Service] [Service]
Environment={{.EnvVariable}}=%n Environment={{{{.EnvVariable}}}}=%n
Restart={{.RestartPolicy}} Restart={{{{.RestartPolicy}}}}
TimeoutStopSec={{.TimeoutStopSec}} TimeoutStopSec={{{{.TimeoutStopSec}}}}
{{- if .ExecStartPre1}} {{{{- if .ExecStartPre1}}}}
ExecStartPre={{.ExecStartPre1}} ExecStartPre={{{{.ExecStartPre1}}}}
{{- end}} {{{{- end}}}}
{{- if .ExecStartPre2}} {{{{- if .ExecStartPre2}}}}
ExecStartPre={{.ExecStartPre2}} ExecStartPre={{{{.ExecStartPre2}}}}
{{- end}} {{{{- end}}}}
ExecStart={{.ExecStart}} ExecStart={{{{.ExecStart}}}}
ExecStop={{.ExecStop}} ExecStop={{{{.ExecStop}}}}
ExecStopPost={{.ExecStopPost}} ExecStopPost={{{{.ExecStopPost}}}}
PIDFile={{.PIDFile}} PIDFile={{{{.PIDFile}}}}
Type=forking Type=forking
[Install] [Install]
@ -236,9 +236,9 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
} }
info.EnvVariable = EnvVariable info.EnvVariable = EnvVariable
info.ExecStart = "{{.Executable}} start {{.InfraNameOrID}}" info.ExecStart = "{{{{.Executable}}}} start {{{{.InfraNameOrID}}}}"
info.ExecStop = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.InfraNameOrID}}" info.ExecStop = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.InfraNameOrID}}}}"
info.ExecStopPost = "{{.Executable}} stop {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}} {{.InfraNameOrID}}" info.ExecStopPost = "{{{{.Executable}}}} stop {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} {{{{.InfraNameOrID}}}}"
// Assemble the ExecStart command when creating a new pod. // Assemble the ExecStart command when creating a new pod.
// //
@ -278,8 +278,8 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
startCommand = append(startCommand, podRootArgs...) startCommand = append(startCommand, podRootArgs...)
startCommand = append(startCommand, startCommand = append(startCommand,
[]string{"pod", "create", []string{"pod", "create",
"--infra-conmon-pidfile", "{{.PIDFile}}", "--infra-conmon-pidfile", "{{{{.PIDFile}}}}",
"--pod-id-file", "{{.PodIDFile}}"}...) "--pod-id-file", "{{{{.PodIDFile}}}}"}...)
// Presence check for certain flags/options. // Presence check for certain flags/options.
fs := pflag.NewFlagSet("args", pflag.ContinueOnError) fs := pflag.NewFlagSet("args", pflag.ContinueOnError)
@ -308,11 +308,11 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
startCommand = append(startCommand, podCreateArgs...) startCommand = append(startCommand, podCreateArgs...)
startCommand = quoteArguments(startCommand) startCommand = quoteArguments(startCommand)
info.ExecStartPre1 = "/bin/rm -f {{.PIDFile}} {{.PodIDFile}}" info.ExecStartPre1 = "/bin/rm -f {{{{.PIDFile}}}} {{{{.PodIDFile}}}}"
info.ExecStartPre2 = strings.Join(startCommand, " ") info.ExecStartPre2 = strings.Join(startCommand, " ")
info.ExecStart = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}pod start --pod-id-file {{.PodIDFile}}" info.ExecStart = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod start --pod-id-file {{{{.PodIDFile}}}}"
info.ExecStop = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}pod stop --ignore --pod-id-file {{.PodIDFile}} {{if (ge .StopTimeout 0)}}-t {{.StopTimeout}}{{end}}" info.ExecStop = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod stop --ignore --pod-id-file {{{{.PodIDFile}}}} {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}}"
info.ExecStopPost = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}pod rm --ignore -f --pod-id-file {{.PodIDFile}}" info.ExecStopPost = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod rm --ignore -f --pod-id-file {{{{.PodIDFile}}}}"
} }
info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout
@ -334,7 +334,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
// generation. That's especially needed for embedding the PID and ID // generation. That's especially needed for embedding the PID and ID
// files in other fields which will eventually get replaced in the 2nd // files in other fields which will eventually get replaced in the 2nd
// template execution. // template execution.
templ, err := template.New("pod_template").Parse(podTemplate) templ, err := template.New("pod_template").Delims("{{{{", "}}}}").Parse(podTemplate)
if err != nil { if err != nil {
return "", errors.Wrap(err, "error parsing systemd service template") return "", errors.Wrap(err, "error parsing systemd service template")
} }
@ -345,7 +345,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
} }
// Now parse the generated template (i.e., buf) and execute it. // Now parse the generated template (i.e., buf) and execute it.
templ, err = template.New("pod_template").Parse(buf.String()) templ, err = template.New("pod_template").Delims("{{{{", "}}}}").Parse(buf.String())
if err != nil { if err != nil {
return "", errors.Wrap(err, "error parsing systemd service template") return "", errors.Wrap(err, "error parsing systemd service template")
} }

View File

@ -139,6 +139,33 @@ ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-
PIDFile=%t/pod-123abc.pid PIDFile=%t/pod-123abc.pid
Type=forking Type=forking
[Install]
WantedBy=multi-user.target default.target
`
podNewLabelWithCurlyBraces := `# pod-123abc.service
# autogenerated by Podman CI
[Unit]
Description=Podman pod-123abc.service
Documentation=man:podman-generate-systemd(1)
Wants=network.target
After=network-online.target
Requires=container-1.service container-2.service
Before=container-1.service container-2.service
[Service]
Environment=PODMAN_SYSTEMD_UNIT=%n
Restart=on-failure
TimeoutStopSec=70
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 --label key={{someval}} --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
PIDFile=%t/pod-123abc.pid
Type=forking
[Install] [Install]
WantedBy=multi-user.target default.target WantedBy=multi-user.target default.target
` `
@ -230,6 +257,22 @@ WantedBy=multi-user.target default.target
true, true,
false, false,
}, },
{"pod --new with double curly braces",
podInfo{
Executable: "/usr/bin/podman",
ServiceName: "pod-123abc",
InfraNameOrID: "jadda-jadda-infra",
RestartPolicy: "on-failure",
PIDFile: "/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
StopTimeout: 10,
PodmanVersion: "CI",
RequiredServices: []string{"container-1", "container-2"},
CreateCommand: []string{"podman", "pod", "create", "--name", "foo", "--label", "key={{someval}}"},
},
podNewLabelWithCurlyBraces,
true,
false,
},
} }
for _, tt := range tests { for _, tt := range tests {

View File

@ -355,4 +355,25 @@ var _ = Describe("Podman generate systemd", func() {
Expect(session.ExitCode()).To(Equal(0)) Expect(session.ExitCode()).To(Equal(0))
Expect(session.IsJSONOutputValid()).To(BeTrue()) Expect(session.IsJSONOutputValid()).To(BeTrue())
}) })
It("podman generate systemd --new create command with double curly braces", func() {
// Regression test for #9034
session := podmanTest.Podman([]string{"create", "--name", "foo", "--log-driver=journald", "--log-opt=tag={{.Name}}", ALPINE})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
session = podmanTest.Podman([]string{"generate", "systemd", "--new", "foo"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expect(session.OutputToString()).To(ContainSubstring(" --log-opt=tag={{.Name}} "))
session = podmanTest.Podman([]string{"pod", "create", "--name", "pod", "--label", "key={{someval}}"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
session = podmanTest.Podman([]string{"generate", "systemd", "--new", "pod"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expect(session.OutputToString()).To(ContainSubstring(" --label key={{someval}}"))
})
}) })