From 6efcb2e785e452505894b0e1e589e72487439e17 Mon Sep 17 00:00:00 2001 From: xingweizheng Date: Wed, 27 Oct 2021 18:27:11 +0800 Subject: [PATCH] bugfix: loaded images cover existing images name and tag --- daemon/load.go | 114 +++++++++++++++++++++++++++++++------------- daemon/load_test.go | 26 +++++----- image/image.go | 66 ++++++++++++------------- 3 files changed, 129 insertions(+), 77 deletions(-) diff --git a/daemon/load.go b/daemon/load.go index 41690ab..b6d675b 100644 --- a/daemon/load.go +++ b/daemon/load.go @@ -17,6 +17,8 @@ import ( "io/ioutil" "os" "path/filepath" + "context" + "strings" "github.com/containers/image/v5/docker/tarfile" ociarchive "github.com/containers/image/v5/oci/archive" @@ -25,6 +27,7 @@ import ( "github.com/containers/storage" "github.com/containers/storage/pkg/archive" securejoin "github.com/cyphar/filepath-securejoin" + digest "github.com/opencontainers/go-digest" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -72,6 +75,12 @@ type loadOptions struct { sep separatorLoad } +type singleImage struct { + index int + id string + nameTag []string +} + func (b *Backend) getLoadOptions(req *pb.LoadRequest) (loadOptions, error) { var opt = loadOptions{ path: req.GetPath(), @@ -119,10 +128,8 @@ func (b *Backend) Load(req *pb.LoadRequest, stream pb.Control_LoadServer) error "LoadID": req.GetLoadID(), }).Info("LoadRequest received") - var ( - si *storage.Image - repoTags [][]string - ) + var si *storage.Image + opts, err := b.getLoadOptions(req) if err != nil { return errors.Wrap(err, "process load options failed") @@ -142,7 +149,7 @@ func (b *Backend) Load(req *pb.LoadRequest, stream pb.Control_LoadServer) error } } - repoTags, err = tryToParseImageFormatFromTarball(b.daemon.opts.DataRoot, &opts) + imagesInTar, err := tryToParseImageFormatFromTarball(b.daemon.opts.DataRoot, &opts) if err != nil { return err } @@ -163,24 +170,30 @@ func (b *Backend) Load(req *pb.LoadRequest, stream pb.Control_LoadServer) error eg.Go(func() error { defer log.CloseContent() - for index, nameAndTag := range repoTags { + for _, singleImage := range imagesInTar { _, si, err = image.ResolveFromImage(&image.PrepareImageOptions{ Ctx: ctx, FromImage: exporter.FormatTransport(opts.format, opts.path), + ToImage: singleImage.id, SystemContext: image.GetSystemContext(), Store: b.daemon.localStore, Reporter: log, - ManifestIndex: index, + ManifestIndex: singleImage.index, }) if err != nil { return err } - if sErr := b.daemon.localStore.SetNames(si.ID, nameAndTag); sErr != nil { - return sErr + originalNames, err := b.daemon.localStore.Names(si.ID) + if err != nil { + return err + } + if err = b.daemon.localStore.SetNames(si.ID, append(originalNames, singleImage.nameTag...)); err != nil { + return err } log.Print("Loaded image as %s\n", si.ID) + logrus.Infof("Loaded image as %s", si.ID) } return nil @@ -189,17 +202,11 @@ func (b *Backend) Load(req *pb.LoadRequest, stream pb.Control_LoadServer) error if wErr := eg.Wait(); wErr != nil { return wErr } - logrus.Infof("Loaded image as %s", si.ID) return nil } -func tryToParseImageFormatFromTarball(dataRoot string, opts *loadOptions) ([][]string, error) { - var ( - allRepoTags [][]string - err error - ) - +func tryToParseImageFormatFromTarball(dataRoot string, opts *loadOptions) ([]singleImage, error) { // tmp dir will be removed after NewSourceFromFileWithContext tmpDir, err := securejoin.SecureJoin(dataRoot, dataRootTmpDirPrefix) if err != nil { @@ -208,19 +215,21 @@ func tryToParseImageFormatFromTarball(dataRoot string, opts *loadOptions) ([][]s systemContext := image.GetSystemContext() systemContext.BigFilesTemporaryDir = tmpDir - allRepoTags, err = getDockerRepoTagFromImageTar(systemContext, opts.path) + // try docker format loading + imagesInTar, err := getDockerRepoTagFromImageTar(systemContext, opts.path) if err == nil { logrus.Infof("Parse image successful with %q format", constant.DockerTransport) opts.format = constant.DockerArchiveTransport - return allRepoTags, nil + return imagesInTar, nil } logrus.Warnf("Try to Parse image of docker format failed with error: %v", err) - allRepoTags, err = getOCIRepoTagFromImageTar(systemContext, opts.path) + // try oci format loading + imagesInTar, err = getOCIRepoTagFromImageTar(systemContext, opts.path) if err == nil { logrus.Infof("Parse image successful with %q format", constant.OCITransport) opts.format = constant.OCIArchiveTransport - return allRepoTags, nil + return imagesInTar, nil } logrus.Warnf("Try to parse image of oci format failed with error: %v", err) @@ -228,7 +237,7 @@ func tryToParseImageFormatFromTarball(dataRoot string, opts *loadOptions) ([][]s return nil, errors.Wrap(err, "wrong image format detected from local tarball") } -func getDockerRepoTagFromImageTar(systemContext *types.SystemContext, path string) ([][]string, error) { +func getDockerRepoTagFromImageTar(systemContext *types.SystemContext, path string) ([]singleImage, error) { // tmp dir will be removed after NewSourceFromFileWithContext tarfileSource, err := tarfile.NewSourceFromFileWithContext(systemContext, path) if err != nil { @@ -245,35 +254,74 @@ func getDockerRepoTagFromImageTar(systemContext *types.SystemContext, path strin return nil, errors.Errorf("failed to get the top level image manifest: %v", err) } - var allRepoTags [][]string - for _, manifestItem := range topLevelImageManifest { - allRepoTags = append(allRepoTags, manifestItem.RepoTags) + imagesInTar := make([]singleImage, 0, len(topLevelImageManifest)) + for i, manifestItem := range topLevelImageManifest { + imageID, err := parseConfigID(manifestItem.Config) + if err != nil { + return nil, err + } + imagesInTar = append(imagesInTar, singleImage{index: i, id: imageID, nameTag: manifestItem.RepoTags}) } - return allRepoTags, nil + return imagesInTar, nil } -func getOCIRepoTagFromImageTar(systemContext *types.SystemContext, path string) ([][]string, error) { - var ( - err error - ) - +func getOCIRepoTagFromImageTar(systemContext *types.SystemContext, path string) ([]singleImage, error) { srcRef, err := alltransports.ParseImageName(exporter.FormatTransport(constant.OCIArchiveTransport, path)) if err != nil { return nil, errors.Wrap(err, "failed to parse image name of oci image format") } + imageID, err := getLoadedImageID(srcRef, systemContext) + if err != nil { + return nil, err + } tarManifest, err := ociarchive.LoadManifestDescriptorWithContext(systemContext, srcRef) if err != nil { return nil, errors.Wrap(err, "failed to load manifest descriptor of oci image format") } - // For now, we only support load single image in archive file + // For now, we only support loading oci-archive file with one single image if _, ok := tarManifest.Annotations[imgspecv1.AnnotationRefName]; ok { - return [][]string{{tarManifest.Annotations[imgspecv1.AnnotationRefName]}}, nil + return []singleImage{{0, imageID, []string{tarManifest.Annotations[imgspecv1.AnnotationRefName]}}}, nil + } + return []singleImage{{0, imageID, []string{}}}, nil +} + +func parseConfigID(configID string) (string, error) { + parts := strings.SplitN(configID, ".", 2) + if len(parts) != 2 { + return "", errors.New("wrong config info of manifest.json") + } + + configDigest := "sha256:" + digest.Digest(parts[0]) + if err := configDigest.Validate(); err != nil { + return "", errors.Wrapf(err, "failed to get config info") + } + + return "@" + configDigest.Encoded(), nil +} + +func getLoadedImageID(imageRef types.ImageReference, systemContext *types.SystemContext) (string, error) { + if imageRef == nil || systemContext == nil { + return "", errors.New("nil image reference or system context when loading image") + } + + newImage, err := imageRef.NewImage(context.TODO(), systemContext) + if err != nil { + return "", err + } + defer func() { + if err = newImage.Close(); err != nil { + logrus.Errorf("failed to close image: %v", err) + } + }() + imageDigest := newImage.ConfigInfo().Digest + if err = imageDigest.Validate(); err != nil { + return "", errors.Wrap(err, "failed to get config info") } - return [][]string{{}}, nil + return "@" + imageDigest.Encoded(), nil } func loadSeparatedImage(opt *loadOptions) error { diff --git a/daemon/load_test.go b/daemon/load_test.go index cbcb5d8..860b897 100644 --- a/daemon/load_test.go +++ b/daemon/load_test.go @@ -31,9 +31,9 @@ import ( ) const ( - loadedTarFile = "load.tar" + loadedTarFile = "load.tar" manifestJSONFile = "manifest.json" - indexJSONFile = "index.json" + indexJSONFile = "index.json" ) var ( @@ -167,8 +167,10 @@ func TestLoadSingleImage(t *testing.T) { } ] }`, - format: "oci", - withTag: true, + format: "oci", + withTag: true, + wantErr: true, + errString: "no such file or directory", }, { name: "TC3 normal case load docker tar with no RepoTags", @@ -197,8 +199,10 @@ func TestLoadSingleImage(t *testing.T) { } ] }`, - format: "oci", - withTag: false, + format: "oci", + withTag: false, + wantErr: true, + errString: "no such file or directory", }, { name: "TC5 abnormal case load docker tar with wrong manifestJSON", @@ -217,7 +221,7 @@ func TestLoadSingleImage(t *testing.T) { format: "docker", withTag: true, wantErr: true, - errString: "error loading index", + errString: "no such file or directory", }, { name: "TC6 abnormal case with wrong tar path", @@ -312,10 +316,10 @@ func TestLoadMultipleImages(t *testing.T) { path := dir.Join(loadedTarFile) repoTags, err := tryToParseImageFormatFromTarball(daemon.opts.DataRoot, &loadOptions{path: path}) assert.NilError(t, err) - assert.Equal(t, repoTags[0][0], "registry.example.com/sayhello:first") - assert.Equal(t, repoTags[1][0], "registry.example.com/sayhello:second") - assert.Equal(t, repoTags[1][1], "registry.example.com/sayhello:third") - assert.Equal(t, len(repoTags[2]), 0) + assert.Equal(t, repoTags[0].nameTag[0], "registry.example.com/sayhello:first") + assert.Equal(t, repoTags[1].nameTag[0], "registry.example.com/sayhello:second") + assert.Equal(t, repoTags[1].nameTag[1], "registry.example.com/sayhello:third") + assert.Equal(t, len(repoTags[2].nameTag), 0) req := &pb.LoadRequest{Path: path} stream := &controlLoadServer{} diff --git a/image/image.go b/image/image.go index 5dda185..b24cb41 100644 --- a/image/image.go +++ b/image/image.go @@ -37,7 +37,6 @@ import ( "github.com/containers/image/v5/transports/alltransports" "github.com/containers/image/v5/types" "github.com/containers/storage" - "github.com/containers/storage/pkg/stringid" "github.com/opencontainers/go-digest" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -55,6 +54,7 @@ type PrepareImageOptions struct { SystemContext *types.SystemContext Ctx context.Context FromImage string + ToImage string Store *store.Store Reporter io.Writer ManifestIndex int @@ -125,20 +125,17 @@ func PullAndGetImageInfo(opt *PrepareImageOptions) (types.ImageReference, *stora } if transport == "" { - // if the image can be obtained from the local storage by image id, - // then only one image can be obtained. + // if the image can be obtained from the local storage by image id, then only one image can be obtained. if len(candidates) != 1 { return nil, nil, errors.New("transport is empty and multi or no image be found") } - img, err := opt.Store.Image(candidates[0]) - if err != nil { - pLog.Errorf("Failed to find the image %q in local store: %v", candidates[0], err) - return nil, nil, err - } - ref, err := is.Transport.ParseStoreReference(opt.Store, img.ID) - if err != nil { - return nil, nil, errors.Wrapf(err, "failed to get the ref in store by %q", candidates[0]) + + ref, img, fErr := FindImage(opt.Store, candidates[0]) + if fErr != nil { + pLog.Errorf("Failed to find the image %q in local store: %v", candidates[0], fErr) + return nil, nil, fErr } + pLog.Infof("Get image from local store first search by %q", opt.FromImage) return ref, img, nil } @@ -147,25 +144,33 @@ func PullAndGetImageInfo(opt *PrepareImageOptions) (types.ImageReference, *stora var errPull error for _, strImage := range candidates { var ( - srcRef types.ImageReference - pErr error + srcRef types.ImageReference + destImage string ) imageName := exporter.FormatTransport(transport, strImage) - if transport == constant.DockerArchiveTransport { - srcRef, pErr = alltransports.ParseImageName(imageName + ":@" + strconv.Itoa(opt.ManifestIndex)) - } else { - srcRef, pErr = alltransports.ParseImageName(imageName) - } - if pErr != nil { - pLog.Debugf("Failed to parse the image %q: %v", imageName, pErr) - continue - } - - destImage, err := getLocalImageNameFromRef(opt.Store, srcRef) - if err != nil { - pLog.Debugf("Failed to get local image name for %q: %v", imageName, err) - continue + switch transport { + case constant.DockerArchiveTransport: + if srcRef, err = alltransports.ParseImageName(imageName + ":@" + strconv.Itoa(opt.ManifestIndex)); err != nil { + pLog.Debugf("Failed to parse the image %q with %q transport: %v", imageName, constant.DockerArchiveTransport, err) + continue + } + destImage = opt.ToImage + case constant.OCIArchiveTransport: + if srcRef, err = alltransports.ParseImageName(imageName); err != nil { + pLog.Debugf("Failed to parse the image %q with %q transport: %v", imageName, constant.OCIArchiveTransport, err) + continue + } + destImage = opt.ToImage + default: + if srcRef, err = alltransports.ParseImageName(imageName); err != nil { + pLog.Debugf("Failed to get local image name for %q: %v", imageName, err) + continue + } + if destImage, err = getLocalImageNameFromRef(srcRef); err != nil { + pLog.Debugf("Failed to parse store reference for %q: %v", destImage, err) + continue + } } destRef, err := is.Transport.ParseStoreReference(opt.Store, destImage) @@ -173,7 +178,6 @@ func PullAndGetImageInfo(opt *PrepareImageOptions) (types.ImageReference, *stora pLog.Debugf("Failed to parse store reference for %q: %v", destImage, err) continue } - img, err := is.Transport.GetStoreImage(opt.Store, destRef) if err == nil { // find the unique image in local store by name or digest @@ -246,14 +250,10 @@ func instantiatingImage(ctx context.Context, sc *types.SystemContext, ref types. return baseImg, nil } -func getLocalImageNameFromRef(store storage.Store, srcRef types.ImageReference) (string, error) { +func getLocalImageNameFromRef(srcRef types.ImageReference) (string, error) { if srcRef == nil { return "", errors.Errorf("reference to image is empty") } - - if err := exporter.CheckArchiveFormat(srcRef.Transport().Name()); err == nil { - return stringid.GenerateRandomID() + ":" + stringid.GenerateRandomID(), nil - } if srcRef.Transport().Name() != constant.DockerTransport { return "", errors.Errorf("the %s transport is not supported yet", srcRef.Transport().Name()) } -- 2.27.0