Skip to content

Commit

Permalink
Make generate systemd --new robust against double curly braces
Browse files Browse the repository at this point in the history 
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 containers#9034

Signed-off-by: Paul Holzinger <[email protected]>
  • Loading branch information
@mheon
Paul Holzinger authored and mheon committed Jan 29, 2021
1 parent 07f2df4 commit b111370
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 56 deletions.
12 changes: 6 additions & 6 deletions pkg/systemd/generate/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ func validateRestartPolicy(restart string) error {
return errors.Errorf("%s is not a valid restart policy", restart)
}

const headerTemplate = `# {{.ServiceName}}.service
# autogenerated by Podman {{.PodmanVersion}}
{{- if .TimeStamp}}
# {{.TimeStamp}}
{{- end}}
const headerTemplate = `# {{{{.ServiceName}}}}.service
# autogenerated by Podman {{{{.PodmanVersion}}}}
{{{{- if .TimeStamp}}}}
# {{{{.TimeStamp}}}}
{{{{- end}}}}
[Unit]
Description=Podman {{.ServiceName}}.service
Description=Podman {{{{.ServiceName}}}}.service
Documentation=man:podman-generate-systemd(1)
Wants=network.target
After=network-online.target
Expand Down
48 changes: 24 additions & 24 deletions pkg/systemd/generate/containers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,22 @@ type containerInfo struct {
}

const containerTemplate = headerTemplate + `
{{- if .BoundToServices}}
BindsTo={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}}
After={{- range $index, $value := .BoundToServices -}}{{if $index}} {{end}}{{ $value }}.service{{end}}
{{- end}}
{{{{- if .BoundToServices}}}}
BindsTo={{{{- range $index, $value := .BoundToServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
After={{{{- range $index, $value := .BoundToServices -}}}}{{{{if $index}}}} {{{{end}}}}{{{{ $value }}}}.service{{{{end}}}}
{{{{- end}}}}
[Service]
Environment={{.EnvVariable}}=%n
Restart={{.RestartPolicy}}
TimeoutStopSec={{.TimeoutStopSec}}
{{- if .ExecStartPre}}
ExecStartPre={{.ExecStartPre}}
{{- end}}
ExecStart={{.ExecStart}}
ExecStop={{.ExecStop}}
ExecStopPost={{.ExecStopPost}}
PIDFile={{.PIDFile}}
Environment={{{{.EnvVariable}}}}=%n
Restart={{{{.RestartPolicy}}}}
TimeoutStopSec={{{{.TimeoutStopSec}}}}
{{{{- if .ExecStartPre}}}}
ExecStartPre={{{{.ExecStartPre}}}}
{{{{- end}}}}
ExecStart={{{{.ExecStart}}}}
ExecStop={{{{.ExecStop}}}}
ExecStopPost={{{{.ExecStopPost}}}}
PIDFile={{{{.PIDFile}}}}
Type=forking
[Install]
Expand Down Expand Up @@ -173,9 +173,9 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
}

info.EnvVariable = EnvVariable
info.ExecStart = "{{.Executable}} start {{.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.ExecStart = "{{{{.Executable}}}} start {{{{.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}}}}"

// Assemble the ExecStart command when creating a new container.
//
Expand Down Expand Up @@ -209,8 +209,8 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
}
startCommand = append(startCommand,
"run",
"--conmon-pidfile", "{{.PIDFile}}",
"--cidfile", "{{.ContainerIDFile}}",
"--conmon-pidfile", "{{{{.PIDFile}}}}",
"--cidfile", "{{{{.ContainerIDFile}}}}",
"--cgroups=no-conmon",
)
// If the container is in a pod, make sure that the
Expand Down Expand Up @@ -281,10 +281,10 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
startCommand = append(startCommand, remainingCmd...)
startCommand = quoteArguments(startCommand)

info.ExecStartPre = "/bin/rm -f {{.PIDFile}} {{.ContainerIDFile}}"
info.ExecStartPre = "/bin/rm -f {{{{.PIDFile}}}} {{{{.ContainerIDFile}}}}"
info.ExecStart = strings.Join(startCommand, " ")
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.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.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout
Expand All @@ -307,7 +307,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
// generation. That's especially needed for embedding the PID and ID
// files in other fields which will eventually get replaced in the 2nd
// template execution.
templ, err := template.New("container_template").Parse(containerTemplate)
templ, err := template.New("container_template").Delims("{{{{", "}}}}").Parse(containerTemplate)
if err != nil {
return "", errors.Wrap(err, "error parsing systemd service template")
}
Expand All @@ -318,7 +318,7 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst
}

// 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 {
return "", errors.Wrap(err, "error parsing systemd service template")
}
Expand Down
39 changes: 39 additions & 0 deletions pkg/systemd/generate/containers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,29 @@ Type=forking
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 {
name string
info containerInfo
Expand Down Expand Up @@ -608,6 +631,22 @@ WantedBy=multi-user.target default.target
true,
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 {
test := tt
Expand Down
52 changes: 26 additions & 26 deletions pkg/systemd/generate/pods.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,23 +72,23 @@ type podInfo struct {
ExecStopPost string
}

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}}
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}}}}
[Service]
Environment={{.EnvVariable}}=%n
Restart={{.RestartPolicy}}
TimeoutStopSec={{.TimeoutStopSec}}
{{- if .ExecStartPre1}}
ExecStartPre={{.ExecStartPre1}}
{{- end}}
{{- if .ExecStartPre2}}
ExecStartPre={{.ExecStartPre2}}
{{- end}}
ExecStart={{.ExecStart}}
ExecStop={{.ExecStop}}
ExecStopPost={{.ExecStopPost}}
PIDFile={{.PIDFile}}
Environment={{{{.EnvVariable}}}}=%n
Restart={{{{.RestartPolicy}}}}
TimeoutStopSec={{{{.TimeoutStopSec}}}}
{{{{- if .ExecStartPre1}}}}
ExecStartPre={{{{.ExecStartPre1}}}}
{{{{- end}}}}
{{{{- if .ExecStartPre2}}}}
ExecStartPre={{{{.ExecStartPre2}}}}
{{{{- end}}}}
ExecStart={{{{.ExecStart}}}}
ExecStop={{{{.ExecStop}}}}
ExecStopPost={{{{.ExecStopPost}}}}
PIDFile={{{{.PIDFile}}}}
Type=forking
[Install]
Expand Down Expand Up @@ -236,9 +236,9 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
}

info.EnvVariable = EnvVariable
info.ExecStart = "{{.Executable}} start {{.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.ExecStart = "{{{{.Executable}}}} start {{{{.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}}}}"

// Assemble the ExecStart command when creating a new pod.
//
Expand Down Expand Up @@ -278,8 +278,8 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
startCommand = append(startCommand, podRootArgs...)
startCommand = append(startCommand,
[]string{"pod", "create",
"--infra-conmon-pidfile", "{{.PIDFile}}",
"--pod-id-file", "{{.PodIDFile}}"}...)
"--infra-conmon-pidfile", "{{{{.PIDFile}}}}",
"--pod-id-file", "{{{{.PodIDFile}}}}"}...)

// Presence check for certain flags/options.
fs := pflag.NewFlagSet("args", pflag.ContinueOnError)
Expand Down Expand Up @@ -308,11 +308,11 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
startCommand = append(startCommand, podCreateArgs...)
startCommand = quoteArguments(startCommand)

info.ExecStartPre1 = "/bin/rm -f {{.PIDFile}} {{.PodIDFile}}"
info.ExecStartPre1 = "/bin/rm -f {{{{.PIDFile}}}} {{{{.PodIDFile}}}}"
info.ExecStartPre2 = strings.Join(startCommand, " ")
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.ExecStopPost = "{{.Executable}} {{if .RootFlags}}{{ .RootFlags}} {{end}}pod rm --ignore -f --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.ExecStopPost = "{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod rm --ignore -f --pod-id-file {{{{.PodIDFile}}}}"
}
info.TimeoutStopSec = minTimeoutStopSec + info.StopTimeout

Expand All @@ -334,7 +334,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
// generation. That's especially needed for embedding the PID and ID
// files in other fields which will eventually get replaced in the 2nd
// template execution.
templ, err := template.New("pod_template").Parse(podTemplate)
templ, err := template.New("pod_template").Delims("{{{{", "}}}}").Parse(podTemplate)
if err != nil {
return "", errors.Wrap(err, "error parsing systemd service template")
}
Expand All @@ -345,7 +345,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions)
}

// 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 {
return "", errors.Wrap(err, "error parsing systemd service template")
}
Expand Down
43 changes: 43 additions & 0 deletions pkg/systemd/generate/pods_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,33 @@ ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-123abc.pod-
PIDFile=%t/pod-123abc.pid
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]
WantedBy=multi-user.target default.target
`
Expand Down Expand Up @@ -230,6 +257,22 @@ WantedBy=multi-user.target default.target
true,
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 {
Expand Down
21 changes: 21 additions & 0 deletions test/e2e/generate_systemd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,4 +355,25 @@ var _ = Describe("Podman generate systemd", func() {
Expect(session.ExitCode()).To(Equal(0))
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}}"))
})
})

0 comments on commit b111370

Please sign in to comment.