Skip to content

Commit

Permalink
Fix issue with "$$" in Script blocks
Browse files Browse the repository at this point in the history
Prior to this commit Steps with Scripts that include two
dollar signs resulted in only one dollar sign ending up
in the final executed script. This is because the script
is passed into an init container via its "args" field and
Kubernetes replaces instances of the literal string
"$$" with "$". This is a documented behaviour of the
"args" field (sort of) but it doesn't make sense to have
those replacements happen for our "script" blocks. "$$"
in bash scripts is the current process id so replacing this
character sequence can be problematic.

This commit replaces instances of two dollar signs in a script
with four dollar signs. Kubernetes then converts these back
to two dollar signs. This replacement behaviour only exists
for dollar symbols so hard-coding this replacement feels like
an acceptable workaround that will have the least impact
on backwards compatibility.
  • Loading branch information
Scott authored and tekton-robot committed May 6, 2021
1 parent 18f337d commit 9a9f896
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 0 deletions.
19 changes: 19 additions & 0 deletions examples/v1alpha1/taskruns/step-script.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,22 @@ spec:
[[ $# == 2 ]]
[[ $1 == "hello" ]]
[[ $2 == "world" ]]
# Test that multiple dollar signs next to each other are not replaced by Kubernetes
- name: dollar-signs-allowed
image: python
script: |
#!/usr/bin/env python3
if '$' != '\u0024':
print('single dollar signs ($) are not passed through as expected :(')
exit(1)
if '$$' != '\u0024\u0024':
print('double dollar signs ($$) are not passed through as expected :(')
exit(2)

This comment has been minimized.

Copy link
@chmouel

chmouel May 6, 2021

Member

FYI, a tiny neat for maybe next time @sbwsg exit in python is for the interactive repl/shell, sys.exit is what is used for non interactive,

if '$$$' != '\u0024\u0024\u0024':
print('three dollar signs ($$$) are not passed through as expected :(')
exit(3)
if '$$$$' != '\u0024\u0024\u0024\u0024':
print('four dollar signs ($$$$) are not passed through as expected :(')
exit(3)
print('dollar signs appear to be handled correctly! :)')
19 changes: 19 additions & 0 deletions examples/v1beta1/taskruns/step-script.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,22 @@ spec:
[[ $# == 2 ]]
[[ $1 == "hello" ]]
[[ $2 == "world" ]]
# Test that multiple dollar signs next to each other are not replaced by Kubernetes
- name: dollar-signs-allowed
image: python
script: |
#!/usr/bin/env python3
if '$' != '\u0024':
print('single dollar signs ($) are not passed through as expected :(')
exit(1)
if '$$' != '\u0024\u0024':
print('double dollar signs ($$) are not passed through as expected :(')
exit(2)
if '$$$' != '\u0024\u0024\u0024':
print('three dollar signs ($$$) are not passed through as expected :(')
exit(3)
if '$$$$' != '\u0024\u0024\u0024\u0024':
print('four dollar signs ($$$$) are not passed through as expected :(')
exit(3)
print('dollar signs appear to be handled correctly! :)')
63 changes: 63 additions & 0 deletions pkg/pod/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,69 @@ script-heredoc-randomly-generated-78c5n
VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}},
}),
},
}, {
desc: "step with script that uses two dollar signs",
ts: v1beta1.TaskSpec{
Steps: []v1beta1.Step{{
Container: corev1.Container{
Name: "one",
Image: "image",
},
Script: "#!/bin/sh\n$$",
}},
},
want: &corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyNever,
InitContainers: []corev1.Container{
{
Name: "place-scripts",
Image: images.ShellImage,
Command: []string{"sh"},
Args: []string{"-c", `tmpfile="/tekton/scripts/script-0-9l9zj"
touch ${tmpfile} && chmod +x ${tmpfile}
cat > ${tmpfile} << 'script-heredoc-randomly-generated-mz4c7'
#!/bin/sh
$$$$
script-heredoc-randomly-generated-mz4c7
`},
VolumeMounts: []corev1.VolumeMount{scriptsVolumeMount},
},
{
Name: "place-tools",
Image: images.EntrypointImage,
Command: []string{"/ko-app/entrypoint", "cp", "/ko-app/entrypoint", "/tekton/tools/entrypoint"},
VolumeMounts: []corev1.VolumeMount{toolsMount},
}},
Containers: []corev1.Container{{
Name: "step-one",
Image: "image",
Command: []string{"/tekton/tools/entrypoint"},
Args: []string{
"-wait_file",
"/tekton/downward/ready",
"-wait_file_content",
"-post_file",
"/tekton/tools/0",
"-termination_path",
"/tekton/termination",
"-entrypoint",
"/tekton/scripts/script-0-9l9zj",
"--",
},
Env: append(implicitEnvVars),
VolumeMounts: append([]corev1.VolumeMount{scriptsVolumeMount, toolsMount, downwardMount, {
Name: "tekton-creds-init-home-0",
MountPath: "/tekton/creds",
}}, implicitVolumeMounts...),
WorkingDir: pipeline.WorkspaceDir,
Resources: corev1.ResourceRequirements{Requests: allZeroQty()},
TerminationMessagePath: "/tekton/termination",
}},
Volumes: append(implicitVolumes, scriptsVolume, toolsVolume, downwardVolume, corev1.Volume{
Name: "tekton-creds-init-home-0",
VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}},
}),
},
}, {
desc: "using another scheduler",
ts: v1beta1.TaskSpec{
Expand Down
6 changes: 6 additions & 0 deletions pkg/pod/script.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ func convertListOfSteps(steps []v1beta1.Step, initContainer *corev1.Container, p
// non-nil init container.
*placeScripts = true

// Kubernetes replaces instances of "$$" with "$". So we double-up
// on these instances in our args and Kubernetes reduces them back down
// to the expected number of dollar signs. This is a workaround for
// https://github.com/kubernetes/kubernetes/issues/101137
script = strings.Replace(script, "$$", "$$$$", -1)

// Append to the place-scripts script to place the
// script file in a known location in the scripts volume.
tmpFile := filepath.Join(scriptsDir, names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("%s-%d", namePrefix, i)))
Expand Down

0 comments on commit 9a9f896

Please sign in to comment.