From 61369deaa93965aaeeea832159cc80c45467e6e8 Mon Sep 17 00:00:00 2001 From: Sally O'Malley Date: Mon, 20 Apr 2020 10:44:59 -0400 Subject: [PATCH 1/3] Bug 1823143: Try user-given registry/repository during extract before defaulting to image reference When working with mirrored release payloads, a release from a mirrored registry, mylocalregistry/ocp/release:4.5.0-0.nightly-2020-04-18-093630 mirrored from registry.svc.ci.openshift.org/ocp/release:4.5.0-0.nightly-2020-04-18-093630 - Both reference 'quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:2eb0a51...'. When extracting from the mirrored registry, try extracting from the user-given registry/repo/name If this fails, then try the release image-references. In disconnected environments, currently, oc adm release extract fails. Different from mirrored, when working with a tagged release that was uploaded to another repository, such as `quay.io/sallyom/release:new`, since that release wasn't mirrored, the underlying images still point to the original tag. In this case, `oc adm release extract --command openshift-install` will try the sallyom/release image but will fail, issue an info message, and then proceed to use the underlying tagged installer image - that will succeed as long as the user has access to the original release image. --- pkg/cli/admin/release/info.go | 51 +++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/pkg/cli/admin/release/info.go b/pkg/cli/admin/release/info.go index fc0d5b2f0a..d6f172d960 100644 --- a/pkg/cli/admin/release/info.go +++ b/pkg/cli/admin/release/info.go @@ -775,6 +775,28 @@ func (o *InfoOptions) LoadReleaseInfo(image string, retrieveImages bool) (*Relea errs = append(errs, err) return true, nil } + // try to verify user-passed image first, in case of mirrored images this will + // exist, and if so, use this instead of the release image-reference. If this doesn't + // exist, proceed with release image-reference from the readReleaseImageReference above + newRef := ref.Ref.AsRepository() + // Try the registry/repo passed from the user first. If image does not exist, + // proceed with the release info from the release image-references, from readReleaseImageReference above + for _, tag := range is.Spec.Tags { + // tagRef is the sha of each component in the release image-reference. + // If can't get digest ID, skip this tag, this happens when user has built a payload by + // replacing component images in the release with a new image + tagRef, err := imagereference.Parse(tag.From.Name) + if err != nil { + continue + } + newRef.ID = tagRef.ID + // If the user-given registry/repo/name:digest exists, replace with that, if not keep the + // is.Spec.Tag from release image-reference + if verifyImageExists(opts, newRef) { + tag.From.Name = newRef.String() + } + } + release.References = is case "release-metadata": data, err := ioutil.ReadAll(r) @@ -875,6 +897,35 @@ func (o *InfoOptions) LoadReleaseInfo(image string, retrieveImages bool) (*Relea return release, nil } +func verifyImageExists(opts *extract.Options, ref imagereference.DockerImageReference) bool { + from := imagesource.TypedImageReference{Type: imagesource.DestinationRegistry, Ref: ref} + ctx := context.Background() + fromContext, err := opts.SecurityOptions.Context() + if err != nil { + return false + } + fromOptions := &imagesource.Options{ + FileDir: opts.FileDir, + Insecure: opts.SecurityOptions.Insecure, + RegistryContext: fromContext, + } + + repo, err := fromOptions.Repository(ctx, from) + if err != nil { + klog.V(2).Infof("unable to connect to image repository %s: %v", from.String(), err) + return false + } + _, _, err = imagemanifest.FirstManifest(ctx, from.Ref, repo, opts.FilterOptions.Include) + if err != nil { + if imagemanifest.IsImageNotFound(err) { + return false + } + klog.V(2).Infof("unable to read image %s: %v", from.String(), err) + return false + } + return true +} + func readComponentVersions(is *imageapi.ImageStream) (ComponentVersions, []error) { var errs []error combined := make(map[string]sets.String) From 36bb2484d5d89b1220edd3c3bb705ff671ba2ebb Mon Sep 17 00:00:00 2001 From: Sally O'Malley Date: Fri, 8 May 2020 22:43:46 -0400 Subject: [PATCH 2/3] Try user-given registry/repository in 'oc adm release mirror' before defaulting to image reference --- pkg/cli/admin/release/mirror.go | 48 ++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/pkg/cli/admin/release/mirror.go b/pkg/cli/admin/release/mirror.go index dfb1c0b2a8..5d8822b382 100644 --- a/pkg/cli/admin/release/mirror.go +++ b/pkg/cli/admin/release/mirror.go @@ -377,6 +377,35 @@ func (o *MirrorOptions) handleSignatures(context context.Context, signaturesByDi return nil } +func verifySrcImageExists(o *mirror.MirrorImageOptions, ref imagereference.DockerImageReference) bool { + from := imagesource.TypedImageReference{Type: imagesource.DestinationRegistry, Ref: ref} + ctx := context.Background() + fromContext, err := o.SecurityOptions.Context() + if err != nil { + return false + } + fromOptions := &imagesource.Options{ + FileDir: o.FileDir, + Insecure: o.SecurityOptions.Insecure, + RegistryContext: fromContext, + } + + repo, err := fromOptions.Repository(ctx, from) + if err != nil { + klog.V(2).Infof("unable to connect to image repository %s: %v", from.String(), err) + return false + } + _, _, err = imagemanifest.FirstManifest(ctx, from.Ref, repo, o.FilterOptions.Include) + if err != nil { + if imagemanifest.IsImageNotFound(err) { + return false + } + klog.V(2).Infof("unable to read image %s: %v", from.String(), err) + return false + } + return true +} + func (o *MirrorOptions) Run() error { var recreateRequired bool var hasPrefix bool @@ -517,10 +546,12 @@ func (o *MirrorOptions) Run() error { if err := imageVerifier.Verify(ctx, releaseDigest); err != nil { fmt.Fprintf(o.ErrOut, "warning: An image was retrieved that failed verification: %v\n", err) } + var srcRef imagesource.TypedImageReference var mappings []mirror.Mapping if len(o.From) > 0 { + var err error src := o.From - srcRef, err := imagesource.ParseReference(src) + srcRef, err = imagesource.ParseReference(src) if err != nil { return fmt.Errorf("invalid --from: %v", err) } @@ -579,6 +610,7 @@ func (o *MirrorOptions) Run() error { repositories := make(map[string]struct{}) + newRef := srcRef.Ref.AsRepository() // build the mapping list for mirroring and rewrite if necessary for i := range is.Spec.Tags { tag := &is.Spec.Tags[i] @@ -593,9 +625,23 @@ func (o *MirrorOptions) Run() error { return fmt.Errorf("image-references should only contain pointers to images by digest: %s", tag.From.Name) } + opts := mirror.NewMirrorImageOptions(genericclioptions.IOStreams{Out: o.Out, ErrOut: o.ErrOut}) + opts.SecurityOptions = o.SecurityOptions + opts.ParallelOptions = o.ParallelOptions + opts.FileDir = o.ToDir // Allow mirror refs to be sourced locally srcMirrorRef := imagesource.TypedImageReference{Ref: from, Type: imagesource.DestinationRegistry} srcMirrorRef = sourceFn(srcMirrorRef) + // Try the registry/repo passed from the user first. If image does not exist, + // proceed with the release info from the release image-references + // ID is the sha of each component in the release image-reference. + newRef.ID = from.ID + // If the user-given registry/repo/name:digest exists, replace with that, if not keep the + // is.Spec.Tag from release image-reference + if verifySrcImageExists(opts, newRef) { + srcMirrorRef = imagesource.TypedImageReference{Ref: newRef, Type: imagesource.DestinationRegistry} + srcMirrorRef = sourceFn(srcMirrorRef) + } // Create a unique map of repos as keys currentRepo := from.AsRepository().String() From 396f3d7d3e928fbdb90ee6532fd1591a0a6d1265 Mon Sep 17 00:00:00 2001 From: Sally O'Malley Date: Mon, 18 May 2020 13:02:41 -0400 Subject: [PATCH 3/3] condense verifyImage --- pkg/cli/admin/release/helpers.go | 37 ++++++++++++++++++++++++++++++++ pkg/cli/admin/release/info.go | 35 +++++------------------------- pkg/cli/admin/release/mirror.go | 35 +++++------------------------- 3 files changed, 47 insertions(+), 60 deletions(-) create mode 100644 pkg/cli/admin/release/helpers.go diff --git a/pkg/cli/admin/release/helpers.go b/pkg/cli/admin/release/helpers.go new file mode 100644 index 0000000000..a6d7068e70 --- /dev/null +++ b/pkg/cli/admin/release/helpers.go @@ -0,0 +1,37 @@ +package release + +import ( + "context" + + "k8s.io/klog" + + "github.com/openshift/library-go/pkg/image/reference" + "github.com/openshift/library-go/pkg/image/registryclient" + "github.com/openshift/oc/pkg/cli/image/imagesource" + imagemanifest "github.com/openshift/oc/pkg/cli/image/manifest" +) + +func verifyImageExists(fromContext *registryclient.Context, fileDir string, insecure bool, include imagemanifest.FilterFunc, ref reference.DockerImageReference) bool { + from := imagesource.TypedImageReference{Type: imagesource.DestinationRegistry, Ref: ref} + ctx := context.Background() + fromOptions := &imagesource.Options{ + FileDir: fileDir, + Insecure: insecure, + RegistryContext: fromContext, + } + + repo, err := fromOptions.Repository(ctx, from) + if err != nil { + klog.V(2).Infof("unable to connect to image repository %s: %v", from.String(), err) + return false + } + _, _, err = imagemanifest.FirstManifest(ctx, from.Ref, repo, include) + if err != nil { + if imagemanifest.IsImageNotFound(err) { + return false + } + klog.V(2).Infof("unable to read image %s: %v", from.String(), err) + return false + } + return true +} diff --git a/pkg/cli/admin/release/info.go b/pkg/cli/admin/release/info.go index d6f172d960..501322a061 100644 --- a/pkg/cli/admin/release/info.go +++ b/pkg/cli/admin/release/info.go @@ -792,7 +792,11 @@ func (o *InfoOptions) LoadReleaseInfo(image string, retrieveImages bool) (*Relea newRef.ID = tagRef.ID // If the user-given registry/repo/name:digest exists, replace with that, if not keep the // is.Spec.Tag from release image-reference - if verifyImageExists(opts, newRef) { + fromContext, err := opts.SecurityOptions.Context() + if err != nil { + return true, nil + } + if verifyImageExists(fromContext, opts.FileDir, o.SecurityOptions.Insecure, opts.FilterOptions.Include, newRef) { tag.From.Name = newRef.String() } } @@ -897,35 +901,6 @@ func (o *InfoOptions) LoadReleaseInfo(image string, retrieveImages bool) (*Relea return release, nil } -func verifyImageExists(opts *extract.Options, ref imagereference.DockerImageReference) bool { - from := imagesource.TypedImageReference{Type: imagesource.DestinationRegistry, Ref: ref} - ctx := context.Background() - fromContext, err := opts.SecurityOptions.Context() - if err != nil { - return false - } - fromOptions := &imagesource.Options{ - FileDir: opts.FileDir, - Insecure: opts.SecurityOptions.Insecure, - RegistryContext: fromContext, - } - - repo, err := fromOptions.Repository(ctx, from) - if err != nil { - klog.V(2).Infof("unable to connect to image repository %s: %v", from.String(), err) - return false - } - _, _, err = imagemanifest.FirstManifest(ctx, from.Ref, repo, opts.FilterOptions.Include) - if err != nil { - if imagemanifest.IsImageNotFound(err) { - return false - } - klog.V(2).Infof("unable to read image %s: %v", from.String(), err) - return false - } - return true -} - func readComponentVersions(is *imageapi.ImageStream) (ComponentVersions, []error) { var errs []error combined := make(map[string]sets.String) diff --git a/pkg/cli/admin/release/mirror.go b/pkg/cli/admin/release/mirror.go index 5d8822b382..a22ceac3a2 100644 --- a/pkg/cli/admin/release/mirror.go +++ b/pkg/cli/admin/release/mirror.go @@ -377,35 +377,6 @@ func (o *MirrorOptions) handleSignatures(context context.Context, signaturesByDi return nil } -func verifySrcImageExists(o *mirror.MirrorImageOptions, ref imagereference.DockerImageReference) bool { - from := imagesource.TypedImageReference{Type: imagesource.DestinationRegistry, Ref: ref} - ctx := context.Background() - fromContext, err := o.SecurityOptions.Context() - if err != nil { - return false - } - fromOptions := &imagesource.Options{ - FileDir: o.FileDir, - Insecure: o.SecurityOptions.Insecure, - RegistryContext: fromContext, - } - - repo, err := fromOptions.Repository(ctx, from) - if err != nil { - klog.V(2).Infof("unable to connect to image repository %s: %v", from.String(), err) - return false - } - _, _, err = imagemanifest.FirstManifest(ctx, from.Ref, repo, o.FilterOptions.Include) - if err != nil { - if imagemanifest.IsImageNotFound(err) { - return false - } - klog.V(2).Infof("unable to read image %s: %v", from.String(), err) - return false - } - return true -} - func (o *MirrorOptions) Run() error { var recreateRequired bool var hasPrefix bool @@ -638,7 +609,11 @@ func (o *MirrorOptions) Run() error { newRef.ID = from.ID // If the user-given registry/repo/name:digest exists, replace with that, if not keep the // is.Spec.Tag from release image-reference - if verifySrcImageExists(opts, newRef) { + fromContext, err := opts.SecurityOptions.Context() + if err != nil { + return err + } + if verifyImageExists(fromContext, opts.FileDir, opts.SecurityOptions.Insecure, opts.FilterOptions.Include, newRef) { srcMirrorRef = imagesource.TypedImageReference{Ref: newRef, Type: imagesource.DestinationRegistry} srcMirrorRef = sourceFn(srcMirrorRef) }