Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependency on buildpack/imgutil #133

Merged
merged 3 commits into from
Sep 17, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 51 additions & 136 deletions Gopkg.lock

Large diffs are not rendered by default.

5 changes: 0 additions & 5 deletions Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ required = [
"k8s.io/code-generator/cmd/informer-gen",
"github.com/knative/test-infra/scripts",
"github.com/knative/test-infra/tools/dep-collector",
"github.com/buildpack/imgutil",
]

[[override]]
Expand Down Expand Up @@ -99,10 +98,6 @@ required = [
name = "golang.org/x/net"
revision = "3b0461eec859c4b73bb64fdc8285971fd33e3938"

[[override]]
name = "github.com/buildpack/imgutil"
revision = "1f31ed20483a84a33484db37c125df7778692ed6"

[[override]]
name = "contrib.go.opencensus.io/exporter/stackdriver"
# HEAD as of 2019-02-11
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/build/v1alpha1/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ func (b *Build) ServiceAccount() string {
return b.Spec.ServiceAccount
}

func (b *Build) Identifier() string {
return b.Tag()
}

func (b *Build) Tag() string {
return b.Spec.Tags[0]
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/build/v1alpha1/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (b *Builder) Namespace() string {
return b.ObjectMeta.Namespace
}

func (b *Builder) Tag() string {
func (b *Builder) Identifier() string {
return b.Spec.Image
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/build/v1alpha1/cluster_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ func (in *ClusterBuilder) Namespace() string {
return ""
}

func (in *ClusterBuilder) Tag() string {
func (in *ClusterBuilder) Identifier() string {
return in.Spec.Image
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/cnb/cnb_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (r *RemoteMetadataRetriever) GetBuilderImage(repo registry.ImageRef) (Build

return BuilderImage{
BuilderBuildpackMetadata: metadata.Buildpacks,
Identifier: identifier.String(),
Identifier: identifier,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be Image too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is an actual built image identifier. Not the same thing as an image because it always includes the digest

}, nil
}

Expand Down Expand Up @@ -90,7 +90,7 @@ func (r *RemoteMetadataRetriever) GetBuiltImage(ref registry.ImageRef) (BuiltIma
}

return BuiltImage{
Identifier: identifier.String(),
Identifier: identifier,
CompletedAt: imageCreatedAt,
BuildpackMetadata: metadata.Buildpacks,
}, nil
Expand Down
20 changes: 4 additions & 16 deletions pkg/cnb/cnb_metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ package cnb_test
import (
"testing"

"github.com/buildpack/imgutil/fakes"
"github.com/buildpack/imgutil/remote"
"github.com/google/go-containerregistry/pkg/name"
"github.com/sclevine/spec"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -27,12 +24,8 @@ func testMetadataRetriever(t *testing.T, when spec.G, it spec.S) {
when("RemoteMetadataRetriever", func() {
when("retrieving from a builder image", func() {
it("gets buildpacks from a local image", func() {
digest, err := name.NewDigest("builder/image:tag@sha256:2bc85afc0ee0aec012b3889cf5f2e9690bb504c9d19ce90add2f415b85990895")
require.NoError(t, err)
fakeImage := fakes.NewImage("builder/image:tag", "topLayerSha", remote.DigestIdentifier{
Digest: digest,
})
err = fakeImage.SetLabel("io.buildpacks.builder.metadata", `{"buildpacks": [{"id": "test.id", "version": "1.2.3"}]}`)
fakeImage := registryfakes.NewFakeRemoteImage("index.docker.io/builder/image", "sha256:2bc85afc0ee0aec012b3889cf5f2e9690bb504c9d19ce90add2f415b85990895")
err := fakeImage.SetLabel("io.buildpacks.builder.metadata", `{"buildpacks": [{"id": "test.id", "version": "1.2.3"}]}`)
assert.NoError(t, err)

imageRef := registry.NewNoAuthImageRef("test-repo-name")
Expand All @@ -53,13 +46,8 @@ func testMetadataRetriever(t *testing.T, when spec.G, it spec.S) {

when("GetBuiltImage", func() {
it("retrieves the metadata from the registry", func() {
digest, err := name.NewDigest("built/image:tag@sha256:dc7e5e790001c71c2cfb175854dd36e65e0b71c58294b331a519be95bdec4ef4")
require.NoError(t, err)

fakeImage := fakes.NewImage("built/image:tag", "topLayerSha", remote.DigestIdentifier{
Digest: digest,
})
err = fakeImage.SetLabel("io.buildpacks.lifecycle.metadata", `{"buildpacks": [{"key": "test.id", "version": "1.2.3"}]}`)
fakeImage := registryfakes.NewFakeRemoteImage("index.docker.io/built/image", "sha256:dc7e5e790001c71c2cfb175854dd36e65e0b71c58294b331a519be95bdec4ef4")
err := fakeImage.SetLabel("io.buildpacks.lifecycle.metadata", `{"buildpacks": [{"key": "test.id", "version": "1.2.3"}]}`)
assert.NoError(t, err)

fakeImageRef := registry.NewNoAuthImageRef("built/image:tag")
Expand Down
16 changes: 6 additions & 10 deletions pkg/cnb/file_permission_setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@ import (
"os"
"testing"

"github.com/buildpack/imgutil/fakes"
"github.com/google/go-containerregistry/pkg/name"
"github.com/pivotal/kpack/pkg/cnb"
"github.com/pivotal/kpack/pkg/registry"
"github.com/pivotal/kpack/pkg/registry/registryfakes"
"github.com/sclevine/spec"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/pivotal/kpack/pkg/cnb"
"github.com/pivotal/kpack/pkg/registry"
"github.com/pivotal/kpack/pkg/registry/registryfakes"
)

func TestFilePermissionSetup(t *testing.T) {
Expand All @@ -38,10 +37,7 @@ func testFilePermissionSetup(t *testing.T, when spec.G, it spec.S) {

when("#setup", func() {
it("sets the owner of all requested", func() {
digest, err := name.NewDigest("some/builder:tag@sha256:2bc85afc0ee0aec012b3889cf5f2e9690bb504c9d19ce90add2f415b85990895")
require.NoError(t, err)

fakeImage := fakes.NewImage("some/builder", "topLayerSha", digest)
fakeImage := registryfakes.NewFakeRemoteImage("some/builder", "2bc85afc0ee0aec012b3889cf5f2e9690bb504c9d19ce90add2f415b85990895")
require.NoError(t, fakeImage.SetEnv("CNB_USER_ID", "1234"))
require.NoError(t, fakeImage.SetEnv("CNB_GROUP_ID", "5678"))

Expand All @@ -55,7 +51,7 @@ func testFilePermissionSetup(t *testing.T, when spec.G, it spec.S) {
RemoteImageFactory: fakeRemoteImageFactory,
Chowner: chowner,
}
err = filePermissionSetup.Setup("builder/builder", testVolume)
err := filePermissionSetup.Setup("builder/builder", testVolume)
require.NoError(t, err)

require.Equal(t, chowner.chowned[testVolume], "1234:5678")
Expand Down
109 changes: 109 additions & 0 deletions pkg/registry/ggcr_image.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package registry

import (
"fmt"
"net/http"
"strings"
"time"

"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/remote"
"github.com/pkg/errors"
)

type GoContainerRegistryImage struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename this file/struct ... maybe RemoteRegistryImage?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thought is the consumers of this object will consume it through the RemoteImage interface. What makes this struct unique is that it implements that interface using GoContainerRegistry library.

image v1.Image
repoName string
}

func NewGoContainerRegistryImage(repoName string, keychain authn.Keychain) (*GoContainerRegistryImage, error) {
image, err := newV1Image(keychain, repoName)
if err != nil {
return nil, err
}

ri := &GoContainerRegistryImage{
repoName: repoName,
image: image,
}

return ri, nil
}

func newV1Image(keychain authn.Keychain, repoName string) (v1.Image, error) {
var auth authn.Authenticator
ref, err := name.ParseReference(repoName, name.WeakValidation)
if err != nil {
return nil, errors.Wrapf(err, "parse reference '%s'", repoName)
}

auth, err = keychain.Resolve(ref.Context().Registry)
if err != nil {
return nil, errors.Wrapf(err, "resolving keychain for '%s'", ref.Context().Registry)
}

image, err := remote.Image(ref, remote.WithAuth(auth), remote.WithTransport(http.DefaultTransport))
if err != nil {
return nil, errors.Wrapf(err, "connect to registry store '%s'", repoName)
}

return image, nil
}

func (i *GoContainerRegistryImage) CreatedAt() (time.Time, error) {
cfg, err := i.configFile()
if err != nil {
return time.Time{}, err
}
return cfg.Created.UTC(), nil
}

func (i *GoContainerRegistryImage) Env(key string) (string, error) {
cfg, err := i.configFile()
if err != nil {
return "", err
}
for _, envVar := range cfg.Config.Env {
parts := strings.Split(envVar, "=")
if parts[0] == key {
return parts[1], nil
}
}
return "", nil
}

func (i *GoContainerRegistryImage) Label(key string) (string, error) {
cfg, err := i.configFile()
if err != nil {
return "", err
}
labels := cfg.Config.Labels
return labels[key], nil
}

func (i *GoContainerRegistryImage) Identifier() (string, error) {
ref, err := name.ParseReference(i.repoName, name.WeakValidation)
if err != nil {
return "", err
}

digest, err := i.image.Digest()
if err != nil {
return "", errors.Wrapf(err, "failed to get digest for image '%s'", i.repoName)
}

return fmt.Sprintf("%s@%s", ref.Context().Name(), digest), nil
}

func (i *GoContainerRegistryImage) configFile() (*v1.ConfigFile, error) {
cfg, err := i.image.ConfigFile()
if err != nil {
return nil, errors.Wrapf(err, "failed to get config for image '%s'", i.repoName)
} else if cfg == nil {
return nil, errors.Errorf("failed to get config for image '%s'", i.repoName)
}

return cfg, nil
}
75 changes: 75 additions & 0 deletions pkg/registry/ggcr_image_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package registry_test

import (
"testing"
"time"

"github.com/google/go-containerregistry/pkg/authn"
"github.com/sclevine/spec"
"github.com/stretchr/testify/require"

"github.com/pivotal/kpack/pkg/registry"
)

func TestGGCRImage(t *testing.T) {
spec.Run(t, "GGCR Image", testGGCRImage)
}

func testGGCRImage(t *testing.T, when spec.G, it spec.S) {
when("#CreatedAt", func() {
it("returns created at from the image", func() {
image, err := registry.NewGoContainerRegistryImage("cloudfoundry/cnb:bionic@sha256:33c3ad8676530f864d51d78483b510334ccc4f03368f7f5bb9d517ff4cbd630f", authn.DefaultKeychain)
require.NoError(t, err)

createdAt, err := image.CreatedAt()
require.NoError(t, err)

require.NotEqual(t, time.Time{}, createdAt)
})
})

when("#Label", func() {
it("returns created at from the image", func() {
image, err := registry.NewGoContainerRegistryImage("cloudfoundry/cnb:bionic@sha256:33c3ad8676530f864d51d78483b510334ccc4f03368f7f5bb9d517ff4cbd630f", authn.DefaultKeychain)
require.NoError(t, err)

metadata, err := image.Label("io.buildpacks.builder.metadata")
require.NoError(t, err)

require.NotEmpty(t, metadata)
})
})

when("#Env", func() {
it("returns created at from the image", func() {
image, err := registry.NewGoContainerRegistryImage("cloudfoundry/cnb:bionic@sha256:33c3ad8676530f864d51d78483b510334ccc4f03368f7f5bb9d517ff4cbd630f", authn.DefaultKeychain)
require.NoError(t, err)

cnbUserId, err := image.Env("CNB_USER_ID")
require.NoError(t, err)

require.NotEmpty(t, cnbUserId)
})
})

when("#identifer", func() {
it("includes digest if repoName does not have a digest", func() {
image, err := registry.NewGoContainerRegistryImage("cloudfoundry/cnb:bionic", authn.DefaultKeychain)
require.NoError(t, err)

identifier, err := image.Identifier()
require.NoError(t, err)
require.Len(t, identifier, 104)
require.Equal(t, identifier[0:40], "index.docker.io/cloudfoundry/cnb@sha256:")
})

it("includes digest if repoName already has a digest", func() {
image, err := registry.NewGoContainerRegistryImage("cloudfoundry/cnb:bionic@sha256:33c3ad8676530f864d51d78483b510334ccc4f03368f7f5bb9d517ff4cbd630f", authn.DefaultKeychain)
require.NoError(t, err)

identifier, err := image.Identifier()
require.NoError(t, err)
require.Equal(t, identifier, "index.docker.io/cloudfoundry/cnb@sha256:33c3ad8676530f864d51d78483b510334ccc4f03368f7f5bb9d517ff4cbd630f")
})
})
}
21 changes: 9 additions & 12 deletions pkg/registry/image_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,16 @@ package registry
import (
"time"

"github.com/buildpack/imgutil"
"github.com/buildpack/imgutil/remote"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/pkg/errors"
)

type ImageFactory struct {
KeychainFactory KeychainFactory
}

func (f *ImageFactory) NewRemote(imageRef ImageRef) (RemoteImage, error) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an extension of our previous comment, can we update this as well to say Get/FetchRemote because it will never be our intent on actually creating a new remote image

remote, err := remote.NewImage(imageRef.Tag(), f.KeychainFactory.KeychainForImageRef(imageRef), remote.FromBaseImage(imageRef.Tag()))
return remote, errors.Wrapf(err, "could not create remote image from ref %s", imageRef.Tag())
remoteImage, err := NewGoContainerRegistryImage(imageRef.Identifier(), f.KeychainFactory.KeychainForImageRef(imageRef))
return remoteImage, err
}

type KeychainFactory interface {
Expand All @@ -25,25 +22,25 @@ type KeychainFactory interface {
type ImageRef interface {
ServiceAccount() string
Namespace() string
Tag() string
Identifier() string
HasSecret() bool
SecretName() string
}

type noAuthImageRef struct {
repoName string
identifier string
}

func (na *noAuthImageRef) SecretName() string {
return ""
}

func NewNoAuthImageRef(repoName string) *noAuthImageRef {
return &noAuthImageRef{repoName: repoName}
func NewNoAuthImageRef(identifier string) *noAuthImageRef {
return &noAuthImageRef{identifier: identifier}
}

func (na *noAuthImageRef) Tag() string {
return na.repoName
func (na *noAuthImageRef) Identifier() string {
return na.identifier
}

func (noAuthImageRef) ServiceAccount() string {
Expand All @@ -60,7 +57,7 @@ func (noAuthImageRef) Namespace() string {

type RemoteImage interface {
CreatedAt() (time.Time, error)
Identifier() (imgutil.Identifier, error)
Identifier() (string, error)
Label(labelName string) (string, error)
Env(key string) (string, error)
}
Expand Down
Loading