Skip to content

Commit

Permalink
Fix bug in entrypoint lookup for steps by digest
Browse files Browse the repository at this point in the history
Before this change, any resolved digests would be simply appended to the
input image name, which would result in invalid duplicate digests when
the input was specified by digest.

This change uses ref.Context() instead, which omits the digest (or tag),
and appends the resolved digest to the end of that.

This also slightly refactors entrypoint_lookup_impl.go to make it
slightly easier to test, and adds a simple unit test for this specific
error condition.

More tests will be included in future changes.
  • Loading branch information
imjasonh authored and tekton-robot committed Dec 4, 2019
1 parent d478e7d commit d3dc419
Show file tree
Hide file tree
Showing 12 changed files with 1,469 additions and 7 deletions.
7 changes: 4 additions & 3 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions cmd/pullrequest-init/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@ import (
"context"
"flag"
"fmt"
"go.uber.org/zap"
"os"

"github.com/tektoncd/pipeline/pkg/pullrequest"

"go.uber.org/zap"
"knative.dev/pkg/logging"
)

Expand Down
11 changes: 11 additions & 0 deletions examples/taskruns/step-by-digest.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
apiVersion: tekton.dev/v1alpha1
kind: TaskRun
metadata:
generateName: step-by-digest-
spec:
taskSpec:
steps:
# Step images can be specified by digest.
- image: busybox@sha256:1303dbf110c57f3edf68d9f5a16c082ec06c4cf7604831669faf2c712260b5a0
# NB: command is not set, so it must be looked up from the registry.
args: ['-c', 'echo hello']
13 changes: 11 additions & 2 deletions pkg/pod/entrypoint_lookup_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/authn/k8schain"
"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/remote"
lru "github.com/hashicorp/golang-lru"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -75,6 +76,15 @@ func (e *entrypointCache) Get(imageName, namespace, serviceAccountName string) (
if err != nil {
return nil, name.Digest{}, fmt.Errorf("Error getting image manifest: %v", err)
}
ep, digest, err := imageData(ref, img)
if err != nil {
return nil, name.Digest{}, err
}
e.lru.Add(digest.String(), ep) // Populate the cache.
return ep, digest, err
}

func imageData(ref name.Reference, img v1.Image) ([]string, name.Digest, error) {
digest, err := img.Digest()
if err != nil {
return nil, name.Digest{}, fmt.Errorf("Error getting image digest: %v", err)
Expand All @@ -87,9 +97,8 @@ func (e *entrypointCache) Get(imageName, namespace, serviceAccountName string) (
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)
d, err := name.NewDigest(ref.Context().String()+"@"+digest.String(), name.WeakValidation)
if err != nil {
return nil, name.Digest{}, fmt.Errorf("Error constructing resulting digest: %v", err)
}
Expand Down
62 changes: 62 additions & 0 deletions pkg/pod/entrypoint_lookup_impl_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
Copyright 2019 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 (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/mutate"
"github.com/google/go-containerregistry/pkg/v1/random"
)

func TestImageData(t *testing.T) {
// Generate an Image with an Entrypoint configured.
img, err := random.Image(1, 1)
if err != nil {
t.Fatalf("random.Image: %v", err)
}
img, err = mutate.Config(img, v1.Config{
Entrypoint: []string{"my", "entrypoint"},
})
if err != nil {
t.Fatalf("mutate.Config: %v", err)
}
// Get the generated image's digest.
dig, err := img.Digest()
if err != nil {
t.Fatalf("Digest: %v", err)
}
ref, err := name.ParseReference("ubuntu@"+dig.String(), name.WeakValidation)
if err != nil {
t.Fatalf("ParseReference(%q): %v", dig, err)
}

// Get the image data (entrypoint and digest) for the image.
gotEP, gotDig, err := imageData(ref, img)
if err != nil {
t.Fatalf("imageData: %v", err)
}
if d := cmp.Diff([]string{"my", "entrypoint"}, gotEP); d != "" {
t.Errorf("Diff(-want, +got): %s", d)
}
if d := cmp.Diff("index.docker.io/library/ubuntu@"+dig.String(), gotDig.String()); d != "" {
t.Errorf("Diff(-want, +got): %s", d)
}
}
16 changes: 16 additions & 0 deletions vendor/github.com/google/go-containerregistry/pkg/v1/mutate/doc.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit d3dc419

Please sign in to comment.