From af80197b3fa4644ecdcf8770434b1e48d2603d66 Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Wed, 19 Jan 2022 03:08:36 +0000 Subject: [PATCH] use podtemplate imagepullsecrets to resolve entrypoint --- docs/podtemplates.md | 34 +++++ pkg/pod/entrypoint_lookup.go | 6 +- pkg/pod/entrypoint_lookup_impl.go | 8 +- pkg/pod/entrypoint_lookup_impl_test.go | 179 +++++++++++++++++++++++++ pkg/pod/entrypoint_lookup_test.go | 4 +- pkg/pod/pod.go | 16 +-- 6 files changed, 233 insertions(+), 14 deletions(-) create mode 100644 pkg/pod/entrypoint_lookup_impl_test.go diff --git a/docs/podtemplates.md b/docs/podtemplates.md index f200bf2c79e..31fa98e093f 100644 --- a/docs/podtemplates.md +++ b/docs/podtemplates.md @@ -95,6 +95,40 @@ Pod templates support fields listed in the table below. +## Use `imagePullSecrets` to lookup entrypoint + +If no command is configured in `task` and `imagePullSecrets` is configured in `podTemplate`, the Tekton Controller will look up the entrypoint of image with `imagePullSecrets`. The Tekton controller's service account is given access to secrets by default. See [this](https://github.com/tektoncd/pipeline/blob/main/config/200-clusterrole.yaml) for reference. If the Tekton controller's service account is not granted the access to secrets in different namespace, you need to grant the access via `RoleBinding`: + +```yaml +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: creds-getter + namespace: my-ns +rules: +- apiGroups: [""] + resources: ["secrets"] + resourceNames: ["creds"] + verbs: ["get"] +``` + +```yaml +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: creds-getter-binding + namespace: my-ns +subjects: +- kind: ServiceAccount + name: tekton-pipelines-controller + namespace: tekton-pipelines + apiGroup: rbac.authorization.k8s.io +roleRef: + kind: Role + name: creds-getter + apiGroup: rbac.authorization.k8s.io +``` + --- Except as otherwise noted, the content of this page is licensed under the diff --git a/pkg/pod/entrypoint_lookup.go b/pkg/pod/entrypoint_lookup.go index 84db0dfc378..80dba700400 100644 --- a/pkg/pod/entrypoint_lookup.go +++ b/pkg/pod/entrypoint_lookup.go @@ -36,7 +36,7 @@ type EntrypointCache interface { // the reference referred to an index, the returned digest will be the // index's digest, not any platform-specific image contained by the // index. - get(ctx context.Context, ref name.Reference, namespace, serviceAccountName string) (*imageData, error) + get(ctx context.Context, ref name.Reference, namespace, serviceAccountName string, imagePullSecrets []corev1.LocalObjectReference) (*imageData, error) } // imageData contains information looked up about an image or multi-platform image index. @@ -50,7 +50,7 @@ type imageData struct { // // Images that are not specified by digest will be specified by digest after // lookup in the resulting list of containers. -func resolveEntrypoints(ctx context.Context, cache EntrypointCache, namespace, serviceAccountName string, steps []corev1.Container) ([]corev1.Container, error) { +func resolveEntrypoints(ctx context.Context, cache EntrypointCache, namespace, serviceAccountName string, imagePullSecrets []corev1.LocalObjectReference, steps []corev1.Container) ([]corev1.Container, error) { // Keep a local cache of name->imageData lookups, just for the scope of // resolving this set of steps. If the image is pushed to before the // next run, we need to resolve its digest and commands again, but we @@ -72,7 +72,7 @@ func resolveEntrypoints(ctx context.Context, cache EntrypointCache, namespace, s id = cid } else { // Look it up for real. - lid, err := cache.get(ctx, ref, namespace, serviceAccountName) + lid, err := cache.get(ctx, ref, namespace, serviceAccountName, imagePullSecrets) if err != nil { return nil, err } diff --git a/pkg/pod/entrypoint_lookup_impl.go b/pkg/pod/entrypoint_lookup_impl.go index ba618e12018..176a7a0dbd6 100644 --- a/pkg/pod/entrypoint_lookup_impl.go +++ b/pkg/pod/entrypoint_lookup_impl.go @@ -28,6 +28,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote" lru "github.com/hashicorp/golang-lru" specs "github.com/opencontainers/image-spec/specs-go/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes" ) @@ -56,7 +57,7 @@ func NewEntrypointCache(kubeclient kubernetes.Interface) (EntrypointCache, error // It also returns the digest associated with the given reference. If the // reference referred to an index, the returned digest will be the index's // digest, not any platform-specific image contained by the index. -func (e *entrypointCache) get(ctx context.Context, ref name.Reference, namespace, serviceAccountName string) (*imageData, error) { +func (e *entrypointCache) get(ctx context.Context, ref name.Reference, namespace, serviceAccountName string, imagePullSecrets []corev1.LocalObjectReference) (*imageData, error) { // If image is specified by digest, check the local cache. if digest, ok := ref.(name.Digest); ok { if id, ok := e.lru.Get(digest.String()); ok { @@ -64,10 +65,15 @@ func (e *entrypointCache) get(ctx context.Context, ref name.Reference, namespace } } + pullSecretsNames := make([]string, 0, len(imagePullSecrets)) + for _, ps := range imagePullSecrets { + pullSecretsNames = append(pullSecretsNames, ps.Name) + } // Consult the remote registry, using imagePullSecrets. kc, err := k8schain.New(ctx, e.kubeclient, k8schain.Options{ Namespace: namespace, ServiceAccountName: serviceAccountName, + ImagePullSecrets: pullSecretsNames, }) if err != nil { return nil, fmt.Errorf("error creating k8schain: %v", err) diff --git a/pkg/pod/entrypoint_lookup_impl_test.go b/pkg/pod/entrypoint_lookup_impl_test.go new file mode 100644 index 00000000000..5f6d2029216 --- /dev/null +++ b/pkg/pod/entrypoint_lookup_impl_test.go @@ -0,0 +1,179 @@ +/* +Copyright 2022 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package pod + +import ( + "context" + "encoding/base64" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/registry" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + remotetest "github.com/tektoncd/pipeline/test" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + fakeclient "k8s.io/client-go/kubernetes/fake" +) + +const ( + username = "foo" + password = "bar" + imagePullSecretsName = "secret" + nameSpace = "ns" +) + +type fakeHTTP struct { + reg http.Handler +} + +func newfakeHTTP() fakeHTTP { + reg := registry.New() + return fakeHTTP{ + reg: reg, + } +} + +func (f *fakeHTTP) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // Request authentication for ping request. + // For further reference see https://docs.docker.com/registry/spec/api/#api-version-check. + if r.URL.Path == "/v2/" && r.Method == http.MethodGet { + w.Header().Add("WWW-Authenticate", "basic") + w.WriteHeader(http.StatusUnauthorized) + return + } + // Check auth if we've fetching the image. + if strings.HasPrefix(r.URL.Path, "/v2/task") && r.Method == "GET" { + u, p, ok := r.BasicAuth() + if !ok || username != u || password != p { + w.WriteHeader(http.StatusUnauthorized) + return + } + } + // Default to open. + f.reg.ServeHTTP(w, r) +} + +func generateSecret(host string, username string, password string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: imagePullSecretsName, + Namespace: nameSpace, + }, + Type: corev1.SecretTypeDockercfg, + Data: map[string][]byte{ + corev1.DockerConfigKey: []byte( + fmt.Sprintf(`{%q: {"auth": %q}}`, + host, + base64.StdEncoding.EncodeToString([]byte(username+":"+password)), + ), + ), + }, + } +} + +func TestGetImageWithImagePullSecrets(t *testing.T) { + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + ftp := newfakeHTTP() + s := httptest.NewServer(&ftp) + defer s.Close() + + u, err := url.Parse(s.URL) + if err != nil { + t.Errorf("Parsing url with an error: %v", err) + } + + task := &v1beta1.Task{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "tekton.dev/v1beta1", + Kind: "Task"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-create-image"}, + } + + ref, err := remotetest.CreateImageWithAnnotations(u.Host+"/task/test-create-image", remotetest.DefaultObjectAnnotationMapper, task) + if err != nil { + t.Errorf("uploading image failed unexpectedly with an error: %v", err) + } + + imgRef, err := name.ParseReference(ref) + if err != nil { + t.Errorf("digest %s is not a valid reference: %v", ref, err) + } + + for _, tc := range []struct { + name string + basicSecret *corev1.Secret + imagePullSecrets []corev1.LocalObjectReference + wantErr bool + }{{ + name: "correct secret", + basicSecret: generateSecret(u.Host, username, password), + imagePullSecrets: []corev1.LocalObjectReference{{Name: imagePullSecretsName}}, + wantErr: false, + }, { + name: "unauthorized secret", + basicSecret: generateSecret(u.Host, username, "wrong password"), + imagePullSecrets: []corev1.LocalObjectReference{{Name: imagePullSecretsName}}, + wantErr: true, + }, { + name: "empty secret", + basicSecret: &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}, + imagePullSecrets: []corev1.LocalObjectReference{{Name: imagePullSecretsName}}, + wantErr: true, + }, { + name: "no basic secret", + basicSecret: &corev1.Secret{}, + imagePullSecrets: []corev1.LocalObjectReference{{Name: imagePullSecretsName}}, + wantErr: true, + }, { + name: "no imagePullSecrets", + basicSecret: generateSecret(u.Host, username, password), + imagePullSecrets: nil, + wantErr: true, + }} { + t.Run(tc.name, func(t *testing.T) { + client := fakeclient.NewSimpleClientset(&corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: nameSpace, + }, + }, tc.basicSecret) + + entrypointCache, err := NewEntrypointCache(client) + if err != nil { + t.Errorf("Creating entrypointCache with an error: %v", err) + } + + i, err := entrypointCache.get(ctx, imgRef, nameSpace, "", tc.imagePullSecrets) + if (err != nil) != tc.wantErr { + t.Fatalf("get() = %+v, %v, wantErr %t", i, err, tc.wantErr) + } + + }) + + } + +} diff --git a/pkg/pod/entrypoint_lookup_test.go b/pkg/pod/entrypoint_lookup_test.go index 694ec56b1e7..76d89d20a40 100644 --- a/pkg/pod/entrypoint_lookup_test.go +++ b/pkg/pod/entrypoint_lookup_test.go @@ -66,7 +66,7 @@ func TestResolveEntrypoints(t *testing.T) { "reg.io/multi/arch:latest": &data{id: multi}, } - got, err := resolveEntrypoints(ctx, cache, "namespace", "serviceAccountName", []corev1.Container{{ + got, err := resolveEntrypoints(ctx, cache, "namespace", "serviceAccountName", []corev1.LocalObjectReference{{Name: "imageSecret"}}, []corev1.Container{{ // This step specifies its command, so there's nothing to // resolve. Image: "fully-specified", @@ -143,7 +143,7 @@ type data struct { seen bool // Whether the image has been looked up before. } -func (f fakeCache) get(ctx context.Context, ref name.Reference, _, _ string) (*imageData, error) { +func (f fakeCache) get(ctx context.Context, ref name.Reference, _, _ string, _ []corev1.LocalObjectReference) (*imageData, error) { if d, ok := ref.(name.Digest); ok { if data, found := f[d.String()]; found { return data.id, nil diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 7fa2417a1b3..984142abc0a 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -160,8 +160,15 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec initContainers = append(initContainers, *workingDirInit) } + // By default, use an empty pod template and take the one defined in the task run spec if any + podTemplate := pod.Template{} + + if taskRun.Spec.PodTemplate != nil { + podTemplate = *taskRun.Spec.PodTemplate + } + // Resolve entrypoint for any steps that don't specify command. - stepContainers, err = resolveEntrypoints(ctx, b.EntrypointCache, taskRun.Namespace, taskRun.Spec.ServiceAccountName, stepContainers) + stepContainers, err = resolveEntrypoints(ctx, b.EntrypointCache, taskRun.Namespace, taskRun.Spec.ServiceAccountName, podTemplate.ImagePullSecrets, stepContainers) if err != nil { return nil, err } @@ -261,13 +268,6 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec stepContainers[i].Name = names.SimpleNameGenerator.RestrictLength(StepName(s.Name, i)) } - // By default, use an empty pod template and take the one defined in the task run spec if any - podTemplate := pod.Template{} - - if taskRun.Spec.PodTemplate != nil { - podTemplate = *taskRun.Spec.PodTemplate - } - // Add podTemplate Volumes to the explicitly declared use volumes volumes = append(volumes, taskSpec.Volumes...) volumes = append(volumes, podTemplate.Volumes...)