carvel-dev / kbld

kbld seamlessly incorporates image building and image pushing into your development and deployment workflows
https://carvel.dev/kbld
Apache License 2.0
294 stars 41 forks source link

Can't relocate images containing tag *and* digest #72

Open cirocosta opened 4 years ago

cirocosta commented 4 years ago

Hi! šŸ‘‹

I recently hit what seems to be a bug in how kbld keeps track of processed images:

$ kbld relocate --repository $registry -f https://storage.googleapis.com/tekton-releases/pipeline/previous/v0.18.1/release.yaml

relocate | exporting 2 images...
relocate | will export gcr.io/tekton-releases/github.com/tektoncd/pipeline/cmd/controller:v0.18.1@sha256:0d1dcd40d032e940da112460dfbff8da9a423cfcb2397388f2703548b7d406ef
relocate | will export gcr.io/tekton-releases/github.com/tektoncd/pipeline/cmd/webhook:v0.18.1@sha256:96cebd8a17eb204aee86a4ec8cd576fd0d2bbdefc74bdfe17d46b2bdf0bc5e6c
relocate | exported 2 images
relocate | importing 2 images...
relocate | importing gcr.io/tekton-releases/github.com/tektoncd/pipeline/cmd/controller@sha256:0d1dcd40d032e940da112460dfbff8da9a423cfcb2397388f2703548b7d406ef -> <blabla>/tekton@sha256:0d1dcd40d032e940da112460dfbff8da9a423cfcb2397388f2703548b7d406ef...
relocate | importing gcr.io/tekton-releases/github.com/tektoncd/pipeline/cmd/webhook@sha256:96cebd8a17eb204aee86a4ec8cd576fd0d2bbdefc74bdfe17d46b2bdf0bc5e6c -> <blabla>/tekton@sha256:96cebd8a17eb204aee86a4ec8cd576fd0d2bbdefc74bdfe17d46b2bdf0bc5e6c...

relocate | imported 2 images
kbld: Error:
- Expected to find image for 'gcr.io/tekton-releases/github.com/tektoncd/pipeline/cmd/controller:v0.18.1@sha256:0d1dcd40d032e940da112460dfbff8da9a423cfcb2397388f2703548b7d406ef'
- Expected to find image for 'gcr.io/tekton-releases/github.com/tektoncd/pipeline/cmd/webhook:v0.18.1@sha256:96cebd8a17eb204aee86a4ec8cd576fd0d2bbdefc74bdfe17d46b2bdf0bc5e6c'

it appears to me that the possible bug is somewhere around

https://github.com/vmware-tanzu/carvel-kbld/blob/371a1f845d6c2c0e1e1a8d45061a6b05447969c6/pkg/kbld/cmd/relocate.go#L123-L128

where FindByURL is passing the URL containing the tag ($repo:$tag@$digest) but we're adding to the map of processed images the non-tagged form

https://github.com/vmware-tanzu/carvel-kbld/blob/371a1f845d6c2c0e1e1a8d45061a6b05447969c6/pkg/kbld/cmd/processed_images.go#L25-L30

Am I reading it correctly? happy to help in any form :))

thanks!

cirocosta commented 4 years ago

the following patch works but, I imagine there might be a better approach:

diff --git a/pkg/kbld/cmd/relocate.go b/pkg/kbld/cmd/relocate.go
index 863feca..d41eaaa 100644
--- a/pkg/kbld/cmd/relocate.go
+++ b/pkg/kbld/cmd/relocate.go
@@ -120,6 +120,13 @@ func (o *RelocateOptions) updateRefsInResources(
                imageRefs := ctlser.NewImageRefs(resContents, conf.SearchRules())

                imageRefs.Visit(func(imgURL string) (string, bool) {
+                       ref, err := regname.NewDigest(imgURL)
+                       if err != nil {
+                               panic(err)
+                       }
+
+                       imgURL = ref.Name()
+
                        outputImg, found := resolvedImages.FindByURL(UnprocessedImageURL{imgURL})
                        if found {
                                return outputImg.URL, true
danielhelfand commented 4 years ago

Thanks for reporting this @cirocosta. I have been able to reproduce the issue and can confirm your patch resolves the issue. Thank you for debugging this and pointing to where the error occurred.

In debugging this myself locally, I am also seeing that the imgURL is not found as shown below:

image

danielhelfand commented 4 years ago

With the patch, the imgURL is properly found and the relocation goes through properly:

image

cppforlife commented 4 years ago

as a workaround you should be able to do this for now:

kbld -f https://storage.googleapis.com/tekton-releases/pipeline/previous/v0.18.1/release.yaml > manifest.yml
kbld relocate --repository $registry -f manifest.yml

first kbld "normalizes" it to pure digest form. we'll report back with a fix a bit later.