Skip to content

Commit

Permalink
Load builtin entrypoint while redirecting steps
Browse files Browse the repository at this point in the history
Currently, any steps in a Pipeline that rely on the built in entrypoint
of a container will not have the expected behaviour while running due to
the entrypoint being overridden at runtime. This fixes #175.

A major side effect of this work is that the override step will now
possibly make HTTP calls every time a step is overridden. There is very
rudimentary caching in place, however this could likely be improved if
performance becomes an issue.
  • Loading branch information
Tanner Bruce committed Oct 24, 2018
1 parent a183a8a commit bf8be01
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 9 deletions.
45 changes: 42 additions & 3 deletions pkg/reconciler/v1alpha1/taskrun/resources/entrypoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ package resources
import (
"encoding/json"
"fmt"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/name"
"github.com/google/go-containerregistry/pkg/v1/remote"

corev1 "k8s.io/api/core/v1"
)
Expand Down Expand Up @@ -59,9 +62,15 @@ type entrypointArgs struct {
MarkerFile string `json:"marker_file"`
}

func getEnvVar(cmd, args []string) (string, error) {
func getEnvVar(baseCmd, cmd, args []string) (string, error) {
// use the default entrypoint for the container
as := append(baseCmd, args...)
if len(cmd) > 0 {
// unless the step actually overrides the entrypoint
as = append(cmd, args...)
}
entrypointArgs := entrypointArgs{
Args: append(cmd, args...),
Args: as,
ProcessLog: "/tools/process-log.txt",
MarkerFile: "/tools/marker-file.txt",
}
Expand All @@ -84,9 +93,39 @@ func getEnvVar(cmd, args []string) (string, error) {
// we can update the controller to inspect the image's `Entrypoint`
// and use that if required.
func AddEntrypoint(steps []corev1.Container) error {
cache := make(map[string][]string)
getEntrypoint := func(image string) ([]string, error) {
// check if we've seen this image before
if ep, ok := cache[image]; ok {
return ep, nil
}
// verify the image name, then download the remote config file
ref, err := name.ParseReference(image, name.WeakValidation)
if err != nil {
return nil, fmt.Errorf("couldn't parse image %s: %v", image, err)
}
img, err := remote.Image(ref, remote.WithAuthFromKeychain(authn.DefaultKeychain))
if err != nil {
return nil, fmt.Errorf("couldn't get remote image info for %s: %v", image, err)
}
cfg, err := img.ConfigFile()
if err != nil {
return nil, fmt.Errorf("couldn't get config for image %s: %v", image, err)
}

// add the image to the cache
cache[image] = cfg.ContainerConfig.Entrypoint
return cfg.ContainerConfig.Entrypoint, nil
}

for i := range steps {
step := &steps[i]
e, err := getEnvVar(step.Command, step.Args)
ep, err := getEntrypoint(step.Image)
if err != nil {
return fmt.Errorf("couldn't get entrypoint for %s: %v", step.Image, err)
}

e, err := getEnvVar(ep, step.Command, step.Args)
if err != nil {
return fmt.Errorf("couldn't get env var for entrypoint: %s", err)
}
Expand Down
69 changes: 69 additions & 0 deletions pkg/reconciler/v1alpha1/taskrun/resources/entrypoint_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package resources_test

import (
"github.com/knative/build-pipeline/pkg/reconciler/v1alpha1/taskrun/resources"
"k8s.io/api/core/v1"
"testing"
)
const (
kanikoImage = "gcr.io/kaniko-project/executor"
knativeEntrypoint = "/tools/entrypoint"
entrypointJSONConfigEnvVar = "ENTRYPOINT_OPTIONS"
)

func TestAddEntrypoint(t *testing.T) {
inputs := []v1.Container{
{
Image: kanikoImage,
},
{
Image: kanikoImage,
Args: []string{"abcd"},
},
{
Image: kanikoImage,
Command: []string{"abcd"},
Args: []string{"efgh"},
},
}
// The first test case showcases the downloading of the entrypoint for the
// image. the second test shows downloading the image as well as the args
// being passed in. The third command shows a set Command overriding the
// remote one.
envVarStrings := []string{
`{"args":["/kaniko/executor"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`,
`{"args":["/kaniko/executor","abcd"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`,
`{"args":["abcd","efgh"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`,

}
err := resources.AddEntrypoint(inputs)
if err != nil {
t.Errorf("failed to get resources: %v", err)
}
for i, input := range inputs {
if len(input.Command) == 0 || input.Command[0] != knativeEntrypoint {
t.Errorf("command incorrectly set: %q", input.Command)
}
if len(input.Args) > 0 {
t.Errorf("containers should have no args")
}
if len(input.Env) == 0 {
t.Error("there should be atleast one envvar")
}
for _, e := range input.Env {
if e.Name == entrypointJSONConfigEnvVar && e.Value != envVarStrings[i] {
t.Errorf("envvar \n%s\n does not match \n%s", e.Value, envVarStrings[i])
}
}
found := false
for _, vm := range input.VolumeMounts {
if vm.Name == resources.MountName {
found = true
break
}
}
if !found {
t.Error("could not find tools volume mount")
}
}
}
4 changes: 0 additions & 4 deletions test/helm_task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ func getCreateImageTask(namespace string, t *testing.T) *v1alpha1.Task {
Steps: []corev1.Container{{
Name: "kaniko",
Image: "gcr.io/kaniko-project/executor",
Command: []string{"/kaniko/executor"},
Args: []string{"--dockerfile=/workspace/test/gohelloworld/Dockerfile",
fmt.Sprintf("--destination=%s", imageName),
},
Expand Down Expand Up @@ -231,12 +230,10 @@ func getHelmDeployTask(namespace string) *v1alpha1.Task {
Steps: []corev1.Container{{
Name: "helm-init",
Image: "alpine/helm",
Command: []string{"helm"},
Args: []string{"init", "--wait"},
}, {
Name: "helm-deploy",
Image: "alpine/helm",
Command: []string{"helm"},
Args: []string{"install",
"--debug",
"--name=${inputs.params.chartname}",
Expand Down Expand Up @@ -488,7 +485,6 @@ func removeHelmFromCluster(c *clients, t *testing.T, namespace string, logger *l
Steps: []corev1.Container{{
Name: "helm-reset",
Image: "alpine/helm",
Command: []string{"helm"},
Args: []string{"reset", "--force"},
},
},
Expand Down
1 change: 0 additions & 1 deletion test/kaniko_task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ func getTask(repo, namespace string, withSecretConfig bool) *v1alpha1.Task {
step := corev1.Container{
Name: "kaniko",
Image: "gcr.io/kaniko-project/executor",
Command: []string{"/kaniko/executor"},
Args: []string{"--dockerfile=/workspace/Dockerfile",
fmt.Sprintf("--destination=%s", repo),
},
Expand Down
1 change: 0 additions & 1 deletion test/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ func TestPipelineRun_WithServiceAccount(t *testing.T) {
Name: "config-docker",
Image: "gcr.io/cloud-builders/docker",
// Private docker image for Build CRD testing
Command: []string{"docker"},
Args: []string{"pull", "gcr.io/build-crd-testing/secret-sauce"},
VolumeMounts: []corev1.VolumeMount{{
Name: "docker-socket",
Expand Down

0 comments on commit bf8be01

Please sign in to comment.