Remove dependency on buildpack/imgutil #133
Conversation
matthewmcnew
commented
Sep 16, 2019
•
matthewmcnew
commented
- Create GGCRImage that implements RemoteImage interface
- Fixes Builder.Spec.Image does not support images with digest #128
- Upgrades lifecycle to 0.0.4
matthewmcnew
requested review from
joaopapereira,
djoyahoy and
thisisnotashwin
|
We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story. The labels on this github issue will be updated when the story is started. |
pkg/registry/ggcr_image.go
Outdated
| "github.com/pkg/errors" | ||
| ) | ||
|
|
||
| type GGCRImage struct { |
There was a problem hiding this comment.
Not sure if GGCR is how the community refers to the library. We could call it RegistryImage possibly? That or even RemoteImage 🤷♂
pkg/registry/ggcr_image.go
Outdated
| repoName string | ||
| } | ||
|
|
||
| func NewGGCRImage(repoName string, keychain authn.Keychain) (*GGCRImage, error) { |
There was a problem hiding this comment.
Do we want to call it GetRemoteImage or something similar, because we do not want to create an image in case one doesn't exist right? We only want to fetch an already existing one.
| "github.com/google/go-containerregistry/pkg/authn" | ||
| "github.com/pkg/errors" | ||
| ) | ||
|
|
||
| type ImageFactory struct { | ||
| KeychainFactory KeychainFactory | ||
| } | ||
|
|
||
| func (f *ImageFactory) NewRemote(imageRef ImageRef) (RemoteImage, error) { |
There was a problem hiding this comment.
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
- Create GGCRImage that implements RemoteImage interface - Fixes #128
matthewmcnew
force-pushed
the
remove-imgutil
branch
from
d25bfe9 to
6115a29
Compare
- Use 'io.buildpacks.build.metadata' to fetch built image metadata - Bump Go Container Registry
| @@ -57,23 +57,23 @@ func (r *RemoteMetadataRetriever) GetBuilderImage(repo registry.ImageRef) (Build | |||
|
|
|||
| return BuilderImage{ | |||
| BuilderBuildpackMetadata: metadata.Buildpacks, | |||
| Identifier: identifier.String(), | |||
| Identifier: identifier, | |||
There was a problem hiding this comment.
That is an actual built image identifier. Not the same thing as an image because it always includes the digest
pkg/dockercreds/access_checker.go
Outdated
| @@ -2,27 +2,31 @@ package dockercreds | |||
|
|
|||
| import ( | |||
| "fmt" | |||
| "github.com/google/go-containerregistry/pkg/name" | |||
matthewmcnew
force-pushed
the
remove-imgutil
branch
from
619d075 to
42d5238
Compare
out.yaml
Outdated
| @@ -0,0 +1,217 @@ | |||
| apiVersion: v1 | |||
There was a problem hiding this comment.
I finally accidentally did. Gone now :)
| @@ -60,21 +64,3 @@ func HasWriteAccess(tagName string) (bool, error) { | |||
|
|
|||
| return true, nil | |||
| } | |||
|
|
|||
| type unAuthorizedWithoutErrorCodeTransportChecker struct { | |||
There was a problem hiding this comment.
does the new version of ggcr make this obsolete?
There was a problem hiding this comment.
Sort of. We can get the status code now directly on the transport error.
| "github.com/pkg/errors" | ||
| ) | ||
|
|
||
| type GoContainerRegistryImage struct { |
There was a problem hiding this comment.
can we rename this file/struct ... maybe RemoteRegistryImage?
There was a problem hiding this comment.
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.
- Prevents confusing Identifier object on builder/build types
matthewmcnew
force-pushed
the
remove-imgutil
branch
from
42d5238 to
3ee8d25
Compare
