From 9a9f8963b02ef3a32184636468cc9bcf4dcd462b Mon Sep 17 00:00:00 2001 From: Scott Date: Wed, 28 Apr 2021 15:00:08 -0400 Subject: [PATCH] Fix issue with "$$" in Script blocks 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. --- examples/v1alpha1/taskruns/step-script.yaml | 19 +++++++ examples/v1beta1/taskruns/step-script.yaml | 19 +++++++ pkg/pod/pod_test.go | 63 +++++++++++++++++++++ pkg/pod/script.go | 6 ++ 4 files changed, 107 insertions(+) diff --git a/examples/v1alpha1/taskruns/step-script.yaml b/examples/v1alpha1/taskruns/step-script.yaml index fd63efd08ec..b95e707313f 100644 --- a/examples/v1alpha1/taskruns/step-script.yaml +++ b/examples/v1alpha1/taskruns/step-script.yaml @@ -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) + 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! :)') diff --git a/examples/v1beta1/taskruns/step-script.yaml b/examples/v1beta1/taskruns/step-script.yaml index 964b222cc74..087ddf572c8 100644 --- a/examples/v1beta1/taskruns/step-script.yaml +++ b/examples/v1beta1/taskruns/step-script.yaml @@ -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! :)') diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 9489a7a2d44..6a302e64b22 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -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{ diff --git a/pkg/pod/script.go b/pkg/pod/script.go index 288c6875cc1..b2077cacbe3 100644 --- a/pkg/pod/script.go +++ b/pkg/pod/script.go @@ -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)))