Split out entrypoint resolution and injection, and script conversion

This is the next and largest step of the effort to simplify and separate
MakePod out into digestible chunks (#1605)

Behavioral changes:
- Script->Command conversion now happens before entrypoint rewriting,
  rather than converting the rewritten entrypoint args.
- Image name->digest lookups are cached locally while resolving a single
  TaskRun's steps (with test!)
- Entrypoint lookups also update the step's digest. This was a race
  before: if an image was pushed between resolution and pod start, the
  resolved command might be out-of-date.

Some redundant test cases have been removed from taskrun_test.go -- this
file should test only behavior of taskrun.go, which is now smaller.

Unit tests for individual transformation behavior has been moved into
individual test files in pkg/pod, with some integration tests in
pod_test.go -- some of these could likely be removed as well, if we feel
like it.

Author: imjasonh

cmd/controller/main.go

@@ -54,7 +54,7 @@ var (
 func main() {
 	flag.Parse()
 	images := pipeline.Images{
-		EntryPointImage:          *entrypointImage,
+		EntrypointImage:          *entrypointImage,
 		NopImage:                 *nopImage,
 		GitImage:                 *gitImage,
 		CredsImage:               *credsImage,

examples/taskruns/steps-run-in-order.yaml

@@ -6,8 +6,7 @@ spec:
   taskSpec:
     steps:
     - image: busybox
-      command: ['/bin/sh']
+      # NB: command is not set, so it must be looked up from the registry.
       args: ['-c', 'sleep 3 && touch foo']
     - image: busybox
-      command: ['/bin/sh']
       args: ['-c', 'ls', 'foo']

pkg/apis/pipeline/images.go

@@ -19,8 +19,8 @@ package pipeline
 // Images holds the images reference for a number of container images used
 // across tektoncd pipelines.
 type Images struct {
-	// EntryPointImage is container image containing our entrypoint binary.
-	EntryPointImage string
+	// EntrypointImage is container image containing our entrypoint binary.
+	EntrypointImage string
 	// NopImage is the container image used to kill sidecars.
 	NopImage string
 	// GitImage is the container image with Git that we use to implement the Git source step.

pkg/apis/pipeline/v1alpha1/build_gcs_resource_test.go

@@ -28,7 +28,7 @@ import (
 )
 
 var images = pipeline.Images{
-	EntryPointImage:          "override-with-entrypoint:latest",
+	EntrypointImage:          "override-with-entrypoint:latest",
 	NopImage:                 "tianon/true",
 	GitImage:                 "override-with-git:latest",
 	CredsImage:               "override-with-creds:latest",

pkg/artifacts/artifact_storage_test.go

@@ -34,7 +34,7 @@ import (
 
 var (
 	images = pipeline.Images{
-		EntryPointImage:          "override-with-entrypoint:latest",
+		EntrypointImage:          "override-with-entrypoint:latest",
 		NopImage:                 "tianon/true",
 		GitImage:                 "override-with-git:latest",
 		CredsImage:               "override-with-creds:latest",

pkg/pod/entrypoint.go

@@ -0,0 +1,180 @@
+package pod
+
+import (
+	"errors"
+	"fmt"
+	"path/filepath"
+	"strings"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+)
+
+const (
+	toolsVolumeName  = "tools"
+	mountPoint       = "/builder/tools"
+	entrypointBinary = mountPoint + "/entrypoint"
+
+	downwardVolumeName     = "downward"
+	downwardMountPoint     = "/builder/downward"
+	downwardMountReadyFile = "ready"
+	ReadyAnnotation        = "tekton.dev/ready"
+	ReadyAnnotationValue   = "READY"
+
+	StepPrefix    = "step-"
+	SidecarPrefix = "sidecar-"
+)
+
+var (
+	// TODO(#1605): Generate volumeMount names, to avoid collisions.
+	// TODO(#1605): Unexport these vars when Pod conversion is entirely within
+	// this package.
+	ToolsMount = corev1.VolumeMount{
+		Name:      toolsVolumeName,
+		MountPath: mountPoint,
+	}
+	ToolsVolume = corev1.Volume{
+		Name:         toolsVolumeName,
+		VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
+	}
+
+	// TODO(#1605): Signal sidecar readiness by injecting entrypoint,
+	// remove dependency on Downward API.
+	DownwardVolume = corev1.Volume{
+		Name: downwardVolumeName,
+		VolumeSource: corev1.VolumeSource{
+			DownwardAPI: &corev1.DownwardAPIVolumeSource{
+				Items: []corev1.DownwardAPIVolumeFile{{
+					Path: downwardMountReadyFile,
+					FieldRef: &corev1.ObjectFieldSelector{
+						FieldPath: fmt.Sprintf("metadata.annotations['%s']", ReadyAnnotation),
+					},
+				}},
+			},
+		},
+	}
+	DownwardMount = corev1.VolumeMount{
+		Name:      downwardVolumeName,
+		MountPath: downwardMountPoint,
+	}
+)
+
+// OrderContainers returns the specified steps, modified so that they are
+// executed in order by overriding the entrypoint binary. It also returns the
+// init container that places the entrypoint binary pulled from the
+// entrypointImage.
+//
+// Containers must have Command specified; if the user didn't specify a
+// command, we must have fetched the image's ENTRYPOINT before calling this
+// method, using entrypoint_lookup.go.
+//
+// TODO(#1605): Also use entrypoint injection to order sidecar start/stop.
+func OrderContainers(entrypointImage string, steps []corev1.Container) (corev1.Container, []corev1.Container, error) {
+	toolsInit := corev1.Container{
+		Name:         "place-tools",
+		Image:        entrypointImage,
+		Command:      []string{"cp", "/ko-app/entrypoint", entrypointBinary},
+		VolumeMounts: []corev1.VolumeMount{ToolsMount},
+	}
+
+	if len(steps) == 0 {
+		return corev1.Container{}, nil, errors.New("No steps specified")
+	}
+
+	for i, s := range steps {
+		var argsForEntrypoint []string
+		switch i {
+		case 0:
+			argsForEntrypoint = []string{
+				// First step waits for the Downward volume file.
+				"-wait_file", filepath.Join(downwardMountPoint, downwardMountReadyFile),
+				"-wait_file_content", // Wait for file contents, not just an empty file.
+				// Start next step.
+				"-post_file", filepath.Join(mountPoint, fmt.Sprintf("%d", i)),
+			}
+		default:
+			// All other steps wait for previous file, write next file.
+			argsForEntrypoint = []string{
+				"-wait_file", filepath.Join(mountPoint, fmt.Sprintf("%d", i-1)),
+				"-post_file", filepath.Join(mountPoint, fmt.Sprintf("%d", i)),
+			}
+		}
+
+		cmd, args := s.Command, s.Args
+		if len(cmd) == 0 {
+			return corev1.Container{}, nil, fmt.Errorf("Step %d did not specify command", i)
+		}
+		if len(cmd) > 1 {
+			args = append(cmd[1:], args...)
+			cmd = []string{cmd[0]}
+		}
+		argsForEntrypoint = append(argsForEntrypoint, "-entrypoint", cmd[0], "--")
+		argsForEntrypoint = append(argsForEntrypoint, args...)
+
+		steps[i].Command = []string{entrypointBinary}
+		steps[i].Args = argsForEntrypoint
+		steps[i].VolumeMounts = append(steps[i].VolumeMounts, ToolsMount)
+	}
+	// Mount the Downward volume into the first step container.
+	steps[0].VolumeMounts = append(steps[0].VolumeMounts, DownwardMount)
+
+	return toolsInit, steps, nil
+}
+
+// UpdateReady updates the Pod's annotations to signal the first step to start
+// by projecting the ready annotation via the Downward API.
+func UpdateReady(kubeclient kubernetes.Interface, pod corev1.Pod) error {
+	newPod, err := kubeclient.CoreV1().Pods(pod.Namespace).Get(pod.Name, metav1.GetOptions{})
+	if err != nil {
+		return fmt.Errorf("Error getting Pod %q when updating ready annotation: %w", pod.Name, err)
+	}
+
+	// Update the Pod's "READY" annotation to signal the first step to
+	// start.
+	if newPod.ObjectMeta.Annotations == nil {
+		newPod.ObjectMeta.Annotations = map[string]string{}
+	}
+	if newPod.ObjectMeta.Annotations[ReadyAnnotation] != ReadyAnnotationValue {
+		newPod.ObjectMeta.Annotations[ReadyAnnotation] = ReadyAnnotationValue
+		if _, err := kubeclient.CoreV1().Pods(newPod.Namespace).Update(newPod); err != nil {
+			return fmt.Errorf("Error adding ready annotation to Pod %q: %w", pod.Name, err)
+		}
+	}
+	return nil
+}
+
+// StopSidecars updates sidecar containers in the Pod to a nop image, which
+// exits successfully immediately.
+func StopSidecars(nopImage string, kubeclient kubernetes.Interface, pod corev1.Pod) error {
+	newPod, err := kubeclient.CoreV1().Pods(pod.Namespace).Get(pod.Name, metav1.GetOptions{})
+	if err != nil {
+		return fmt.Errorf("Error getting Pod %q when stopping sidecars: %w", pod.Name, err)
+	}
+
+	updated := false
+	if newPod.Status.Phase == corev1.PodRunning {
+		for _, s := range newPod.Status.ContainerStatuses {
+			if IsContainerSidecar(s.Name) && s.State.Running != nil {
+				for j, c := range newPod.Spec.Containers {
+					if c.Name == s.Name && c.Image != nopImage {
+						updated = true
+						newPod.Spec.Containers[j].Image = nopImage
+					}
+				}
+			}
+		}
+	}
+	if updated {
+		if _, err := kubeclient.CoreV1().Pods(newPod.Namespace).Update(newPod); err != nil {
+			return fmt.Errorf("Error adding ready annotation to Pod %q: %w", pod.Name, err)
+		}
+	}
+	return nil
+}
+
+func IsContainerStep(name string) bool    { return strings.HasPrefix(name, StepPrefix) }
+func IsContainerSidecar(name string) bool { return strings.HasPrefix(name, SidecarPrefix) }
+
+func TrimStepPrefix(name string) string    { return strings.TrimPrefix(name, StepPrefix) }
+func TrimSidecarPrefix(name string) string { return strings.TrimPrefix(name, SidecarPrefix) }

pkg/pod/entrypoint_lookup.go

@@ -0,0 +1,126 @@
+package pod
+
+import (
+	"fmt"
+
+	"github.com/google/go-containerregistry/pkg/authn"
+	"github.com/google/go-containerregistry/pkg/authn/k8schain"
+	"github.com/google/go-containerregistry/pkg/name"
+	"github.com/google/go-containerregistry/pkg/v1/remote"
+	lru "github.com/hashicorp/golang-lru"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/client-go/kubernetes"
+)
+
+// ResolveEntrypoints looks up container image ENTRYPOINTs for all steps that
+// don't specify a Command.
+//
+// Images that are not specified by digest will be specified by digest after
+// lookup in the resulting list of containers.
+func ResolveEntrypoints(cache EntrypointCache, namespace, serviceAccountName string, steps []corev1.Container) ([]corev1.Container, error) {
+	// Keep a local cache of image->digest lookups, just for the scope of
+	// resolving this set of steps. If the image is pushed to, we need to
+	// resolve its digest and entrypoint again, but we can skip lookups
+	// while resolving the same TaskRun.
+	type result struct {
+		digest name.Digest
+		ep     []string
+	}
+	localCache := map[string]result{}
+	for i, s := range steps {
+		if len(s.Command) != 0 {
+			// Nothing to resolve.
+			continue
+		}
+
+		var digest name.Digest
+		var ep []string
+		var err error
+		if r, found := localCache[s.Image]; found {
+			digest = r.digest
+			ep = r.ep
+		} else {
+			// Look it up in the cache, which will also resolve the digest.
+			ep, digest, err = cache.Get(s.Image, namespace, serviceAccountName)
+			if err != nil {
+				return nil, err
+			}
+			localCache[s.Image] = result{digest, ep} // Cache it locally in case another step specifies the same image.
+		}
+
+		steps[i].Image = digest.String() // Specify image by digest, since we know it now.
+		steps[i].Command = ep            // Specify the command explicitly.
+	}
+	return steps, nil
+}
+
+const cacheSize = 1024
+
+type EntrypointCache interface {
+	Get(imageName, namespace, serviceAccountName string) (cmd []string, d name.Digest, err error)
+}
+
+type entrypointCache struct {
+	kubeclient kubernetes.Interface
+	lru        *lru.Cache // cache of digest string -> image entrypoint []string
+}
+
+func NewEntrypointCache(kubeclient kubernetes.Interface) (EntrypointCache, error) {
+	lru, err := lru.New(cacheSize)
+	if err != nil {
+		return nil, err
+	}
+	return &entrypointCache{
+		kubeclient: kubeclient,
+		lru:        lru,
+	}, nil
+}
+
+func (e *entrypointCache) Get(imageName, namespace, serviceAccountName string) (cmd []string, d name.Digest, err error) {
+	ref, err := name.ParseReference(imageName, name.WeakValidation)
+	if err != nil {
+		return nil, name.Digest{}, fmt.Errorf("Error parsing reference %q: %v", imageName, err)
+	}
+
+	// If image is specified by digest, check the local cache.
+	if digest, ok := ref.(name.Digest); ok {
+		if ep, ok := e.lru.Get(digest.String()); ok {
+			return ep.([]string), digest, nil
+		}
+	}
+
+	// If the image wasn't specified by digest, or if the entrypoint
+	// wasn't found, we have to consult the remote registry, using
+	// imagePullSecrets.
+	kc, err := k8schain.New(e.kubeclient, k8schain.Options{
+		Namespace:          namespace,
+		ServiceAccountName: serviceAccountName,
+	})
+	if err != nil {
+		return nil, name.Digest{}, fmt.Errorf("Error creating k8schain: %v", err)
+	}
+	mkc := authn.NewMultiKeychain(kc)
+	img, err := remote.Image(ref, remote.WithAuthFromKeychain(mkc))
+	if err != nil {
+		return nil, name.Digest{}, fmt.Errorf("Error getting image manifest: %v", err)
+	}
+	digest, err := img.Digest()
+	if err != nil {
+		return nil, name.Digest{}, fmt.Errorf("Error getting image digest: %v", err)
+	}
+	cfg, err := img.ConfigFile()
+	if err != nil {
+		return nil, name.Digest{}, fmt.Errorf("Error getting image config: %v", err)
+	}
+	ep := cfg.Config.Entrypoint
+	if len(ep) == 0 {
+		ep = cfg.Config.Cmd
+	}
+	e.lru.Add(digest.String(), ep) // Populate the cache.
+
+	d, err = name.NewDigest(imageName+"@"+digest.String(), name.WeakValidation)
+	if err != nil {
+		return nil, name.Digest{}, fmt.Errorf("Error constructing resulting digest: %v", err)
+	}
+	return ep, d, nil
+}