Have cmd/bash image actually use bash

- Base cmd/bash image on ubuntu, run specified args with `bash -c`
- Fail cmd/bash if extra args are passed -- these would have been ignored previously
- Fix examples for cmd/bash that passed extra args
- Update entrypoint image to be based on busybox, since it only needs `cp`
- Un-export unnecessarily exported types in entrypoint cmd and pkgs

Fixes tektoncd#1445

.ko.yaml

@@ -1,6 +1,6 @@
 baseImageOverrides:
   github.com/tektoncd/pipeline/cmd/creds-init: gcr.io/tekton-nightly/github.com/tektoncd/pipeline/build-base:latest
   github.com/tektoncd/pipeline/cmd/git-init: gcr.io/tekton-nightly/github.com/tektoncd/pipeline/build-base:latest
-  github.com/tektoncd/pipeline/cmd/bash: busybox # image should have shell in $PATH
-  github.com/tektoncd/pipeline/cmd/entrypoint: busybox # image should have shell in $PATH
+  github.com/tektoncd/pipeline/cmd/bash: bash # image must have `bash` in $PATH
+  github.com/tektoncd/pipeline/cmd/entrypoint: busybox  # image must have `cp` in $PATH
   github.com/tektoncd/pipeline/cmd/gsutil: google/cloud-sdk:alpine # image should have gsutil in $PATH

cmd/bash/main.go

@@ -26,16 +26,19 @@ args: ['-args', 'ARGUMENTS_FOR_SHELL_COMMAND']
 For help, run `man sub-command`
 ```
 image: github.com/tektoncd/pipeline/cmd/bash
-args: ['-args', `man`,`mkdir`]
+args: ['-args', 'man mkdir']
 ```
 
 For example:
 Following example executes shell sub-command `mkdir`
 
 ```
 image: github.com/tektoncd/pipeline/cmd/bash
-args:  ['-args', 'mkdir', '-p', '/workspace/gcs-dir']
+args:  ['-args', 'mkdir -p /workspace/gcs-dir']
 ```
+
+Be sure not to pass multiple args to `-args`, only a single quoted string to
+execute. Passing further args will cause the image to raise an error.
 */
 
 package main
@@ -56,7 +59,11 @@ func main() {
 	flag.Parse()
 	logger, _ := logging.NewLogger("", "shell_command")
 	defer logger.Sync()
-	cmd := exec.Command("sh", "-c", *args)
+	if len(flag.Args()) > 0 {
+		logger.Fatalf("Extra args were provided, and would be ignored: %v", flag.Args())
+	}
+
+	cmd := exec.Command("bash", "-c", *args)
 	stdoutStderr, err := cmd.CombinedOutput()
 	if err != nil {
 		logger.Fatalf("Error executing command %q ; error %s; cmd_output %s", strings.Join(cmd.Args, " "), err.Error(), stdoutStderr)

cmd/entrypoint/main.go

@@ -46,9 +46,9 @@ func main() {
 		WaitFileContent: *waitFileContent,
 		PostFile:        *postFile,
 		Args:            flag.Args(),
-		Waiter:          &RealWaiter{},
-		Runner:          &RealRunner{},
-		PostWriter:      &RealPostWriter{},
+		Waiter:          &realWaiter{},
+		Runner:          &realRunner{},
+		PostWriter:      &realPostWriter{},
 	}
 	if err := e.Go(); err != nil {
 		switch t := err.(type) {

cmd/entrypoint/post_writer.go

@@ -7,12 +7,12 @@ import (
 	"github.com/tektoncd/pipeline/pkg/entrypoint"
 )
 
-// RealPostWriter actually writes files.
-type RealPostWriter struct{}
+// realPostWriter actually writes files.
+type realPostWriter struct{}
 
-var _ entrypoint.PostWriter = (*RealPostWriter)(nil)
+var _ entrypoint.PostWriter = (*realPostWriter)(nil)
 
-func (*RealPostWriter) Write(file string) {
+func (*realPostWriter) Write(file string) {
 	if file == "" {
 		return
 	}

cmd/entrypoint/runner.go

@@ -10,12 +10,12 @@ import (
 // TODO(jasonhall): Test that original exit code is propagated and that
 // stdout/stderr are collected -- needs e2e tests.
 
-// RealRunner actually runs commands.
-type RealRunner struct{}
+// realRunner actually runs commands.
+type realRunner struct{}
 
-var _ entrypoint.Runner = (*RealRunner)(nil)
+var _ entrypoint.Runner = (*realRunner)(nil)
 
-func (*RealRunner) Run(args ...string) error {
+func (*realRunner) Run(args ...string) error {
 	if len(args) == 0 {
 		return nil
 	}

cmd/entrypoint/waiter.go

@@ -8,10 +8,10 @@ import (
 	"golang.org/x/xerrors"
 )
 
-// RealWaiter actually waits for files, by polling.
-type RealWaiter struct{}
+// realWaiter actually waits for files, by polling.
+type realWaiter struct{}
 
-var _ entrypoint.Waiter = (*RealWaiter)(nil)
+var _ entrypoint.Waiter = (*realWaiter)(nil)
 
 // Wait watches a file and returns when either a) the file exists and, if
 // the expectContent argument is true, the file has non-zero size or b) there
@@ -22,7 +22,7 @@ var _ entrypoint.Waiter = (*RealWaiter)(nil)
 //
 // If a file of the same name with a ".err" extension exists then this Wait
 // will end with a skipError.
-func (*RealWaiter) Wait(file string, expectContent bool) error {
+func (*realWaiter) Wait(file string, expectContent bool) error {
 	if file == "" {
 		return nil
 	}

cmd/entrypoint/waiter_test.go

@@ -32,7 +32,7 @@ func TestRealWaiterWaitMissingFile(t *testing.T) {
 		t.Errorf("error creating temp file: %v", err)
 	}
 	os.Remove(tmp.Name())
-	rw := RealWaiter{}
+	rw := realWaiter{}
 	doneCh := make(chan struct{})
 	go func() {
 		err := rw.Wait(tmp.Name(), false)
@@ -55,7 +55,7 @@ func TestRealWaiterWaitWithFile(t *testing.T) {
 		t.Errorf("error creating temp file: %v", err)
 	}
 	defer os.Remove(tmp.Name())
-	rw := RealWaiter{}
+	rw := realWaiter{}
 	doneCh := make(chan struct{})
 	go func() {
 		err := rw.Wait(tmp.Name(), false)
@@ -78,7 +78,7 @@ func TestRealWaiterWaitMissingContent(t *testing.T) {
 		t.Errorf("error creating temp file: %v", err)
 	}
 	defer os.Remove(tmp.Name())
-	rw := RealWaiter{}
+	rw := realWaiter{}
 	doneCh := make(chan struct{})
 	go func() {
 		err := rw.Wait(tmp.Name(), true)
@@ -101,7 +101,7 @@ func TestRealWaiterWaitWithContent(t *testing.T) {
 		t.Errorf("error creating temp file: %v", err)
 	}
 	defer os.Remove(tmp.Name())
-	rw := RealWaiter{}
+	rw := realWaiter{}
 	doneCh := make(chan struct{})
 	go func() {
 		err := rw.Wait(tmp.Name(), true)

pkg/reconciler/taskrun/entrypoint/entrypoint.go

@@ -37,24 +37,25 @@ const (
 	// MountName is the name of the pvc being mounted (which
 	// will contain the entrypoint binary and eventually the logs)
 	MountName              = "tools"
-	MountPoint             = "/builder/tools"
+	mountPoint             = "/builder/tools"
 	DownwardMountName      = "downward"
 	DownwardMountPoint     = "/builder/downward"
 	DownwardMountReadyFile = "ready"
-	BinaryLocation         = MountPoint + "/entrypoint"
-	JSONConfigEnvVar       = "ENTRYPOINT_OPTIONS"
+	binaryLocation         = mountPoint + "/entrypoint"
 	InitContainerName      = "place-tools"
 	cacheSize              = 1024
 )
 
-var toolsMount = corev1.VolumeMount{
-	Name:      MountName,
-	MountPath: MountPoint,
-}
-var downwardMount = corev1.VolumeMount{
-	Name:      DownwardMountName,
-	MountPath: DownwardMountPoint,
-}
+var (
+	toolsMount = corev1.VolumeMount{
+		Name:      MountName,
+		MountPath: mountPoint,
+	}
+	downwardMount = corev1.VolumeMount{
+		Name:      DownwardMountName,
+		MountPath: DownwardMountPoint,
+	}
+)
 
 // Cache is a simple caching mechanism allowing for caching the results of
 // getting the Entrypoint of a container image from a remote registry. The
@@ -93,11 +94,9 @@ func AddToEntrypointCache(c *Cache, sha string, ep []string) {
 // subsequent steps and used to capture logs.
 func AddCopyStep(entrypointImage string, spec *v1alpha1.TaskSpec) {
 	cp := corev1.Container{
-		Name:    InitContainerName,
-		Image:   entrypointImage,
-		Command: []string{"/bin/sh"},
-		// based on the ko version, the binary could be in one of two different locations
-		Args:         []string{"-c", fmt.Sprintf("cp /ko-app/entrypoint %s", BinaryLocation)},
+		Name:         InitContainerName,
+		Image:        entrypointImage,
+		Command:      []string{"cp", "/ko-app/entrypoint", binaryLocation},
 		VolumeMounts: []corev1.VolumeMount{toolsMount},
 	}
 	spec.Steps = append([]v1alpha1.Step{{Container: cp}}, spec.Steps...)
@@ -134,7 +133,7 @@ func RedirectStep(cache *Cache, stepNum int, step *v1alpha1.Step, kubeclient kub
 	}
 
 	step.Args = GetArgs(stepNum, step.Command, step.Args)
-	step.Command = []string{BinaryLocation}
+	step.Command = []string{binaryLocation}
 	step.VolumeMounts = append(step.VolumeMounts, toolsMount)
 	// The first step in a Task waits for the existence of a file projected into the Pod
 	// from the Downward API. That file will be populated only when all sidecars are ready,
@@ -175,7 +174,7 @@ func getWaitFile(stepNum int) string {
 		return fmt.Sprintf("%s/%s", DownwardMountPoint, DownwardMountReadyFile)
 	}
 
-	return fmt.Sprintf("%s/%s", MountPoint, strconv.Itoa(stepNum-1))
+	return fmt.Sprintf("%s/%s", mountPoint, strconv.Itoa(stepNum-1))
 }
 
 // GetRemoteEntrypoint accepts a cache of digest lookups, as well as the digest

pkg/reconciler/taskrun/entrypoint/entrypoint_test.go

@@ -74,7 +74,7 @@ func TestRewriteSteps(t *testing.T) {
 		t.Errorf("failed to get resources: %v", err)
 	}
 	for _, input := range inputs {
-		if len(input.Command) == 0 || input.Command[0] != BinaryLocation {
+		if len(input.Command) == 0 || input.Command[0] != binaryLocation {
 			t.Errorf("command incorrectly set: %q", input.Command)
 		}
 		found := false

pkg/reconciler/taskrun/taskrun_test.go

@@ -194,14 +194,12 @@ var (
 		Name: "downward",
 		VolumeSource: corev1.VolumeSource{
 			DownwardAPI: &corev1.DownwardAPIVolumeSource{
-				Items: []corev1.DownwardAPIVolumeFile{
-					{
-						Path: "ready",
-						FieldRef: &corev1.ObjectFieldSelector{
-							FieldPath: "metadata.annotations['tekton.dev/ready']",
-						},
+				Items: []corev1.DownwardAPIVolumeFile{{
+					Path: "ready",
+					FieldRef: &corev1.ObjectFieldSelector{
+						FieldPath: "metadata.annotations['tekton.dev/ready']",
 					},
-				},
+				}},
 			},
 		},
 	}
@@ -245,8 +243,8 @@ var (
 
 	getPlaceToolsInitContainer = func(ops ...tb.ContainerOp) tb.PodSpecOp {
 		actualOps := []tb.ContainerOp{
-			tb.Command("/bin/sh"),
-			tb.Args("-c", fmt.Sprintf("cp /ko-app/entrypoint %s", entrypointLocation)),
+			tb.Command("cp", "/ko-app/entrypoint", entrypointLocation),
+			tb.Args(),
 			tb.WorkingDir(workspaceDir),
 			tb.EnvVar("HOME", "/builder/home"),
 			tb.VolumeMount("tools", "/builder/tools"),