packages/isula-build
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
isula-build/patch/0070-clean-code-all-latest-tag-checks-take-the-FindImage-.patch
DCCooper 36a8e66c74 isula-build:Sync patch from upstream 
reason: sync patches from upstream, including
relocate export package, clean code for tests and golint

Signed-off-by: DCCooper <1866858@gmail.com>
2021-10-25 17:27:38 +08:00

392 lines
12 KiB
Diff
Raw Blame History

From d6c6a81dd3cb73685c5cdf029cf9dd5602d3d44d Mon Sep 17 00:00:00 2001
From: xingweizheng <xingweizheng@huawei.com>
Date: Sun, 29 Aug 2021 01:07:58 +0800
Subject: [PATCH 4/5] clean code: all latest tag checks take the FindImage as
the entrance
---
builder/dockerfile/builder.go | 2 +-
constant.go | 2 +
daemon/push.go | 6 ---
daemon/save.go | 24 ++++------
exporter/docker/archive/archive.go | 56 +++++++++++++---------
image/image.go | 40 ++++++++--------
image/image_test.go | 96 ++++++++++++++++++++++++++++++++++++++
7 files changed, 160 insertions(+), 66 deletions(-)
diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go
index 1e1e24d..e28fac9 100644
--- a/builder/dockerfile/builder.go
+++ b/builder/dockerfile/builder.go
@@ -646,7 +646,7 @@ func CheckAndExpandTag(tag string) (reference.Named, string, error) {
if sepLastIndex == -1 || (sepLastIndex < slashLastIndex) {
// isula
// localhost:5000/isula
- newTag += ":latest"
+ newTag = fmt.Sprintf("%s:%s", newTag, constant.DefaultTag)
}
const longestTagFieldsLen = 3
diff --git a/constant.go b/constant.go
index bfe399b..4d1596a 100644
--- a/constant.go
+++ b/constant.go
@@ -89,6 +89,8 @@ const (
IsuladTransport = "isulad"
// ManifestTransport used to export manifest list
ManifestTransport = "manifest"
+ // DefaultTag is latest
+ DefaultTag = "latest"
)
var (
diff --git a/daemon/push.go b/daemon/push.go
index d3f5571..ac05383 100644
--- a/daemon/push.go
+++ b/daemon/push.go
@@ -68,12 +68,6 @@ func (b *Backend) Push(req *pb.PushRequest, stream pb.Control_PushServer) error
return err
}
- imageName, err := image.CheckAndAddDefaultTag(opt.imageName, opt.localStore)
- if err != nil {
- return err
- }
- opt.imageName = imageName
-
manifestType, gErr := exporter.GetManifestType(opt.format)
if gErr != nil {
return gErr
diff --git a/daemon/save.go b/daemon/save.go
index 35b67de..ee70691 100644
--- a/daemon/save.go
+++ b/daemon/save.go
@@ -16,6 +16,7 @@ package daemon
import (
"context"
"os"
+ "strings"
"github.com/containers/image/v5/docker/reference"
"github.com/containers/image/v5/types"
@@ -76,7 +77,7 @@ func (b *Backend) Save(req *pb.SaveRequest, stream pb.Control_SaveServer) error
var err error
opts := b.getSaveOptions(req)
- if err = checkFormatAndExpandTag(&opts); err != nil {
+ if err = checkFormat(&opts); err != nil {
return err
}
if err = filterImageName(&opts); err != nil {
@@ -159,7 +160,7 @@ func messageHandler(stream pb.Control_SaveServer, cliLogger *logger.Logger) func
}
}
-func checkFormatAndExpandTag(opts *saveOptions) error {
+func checkFormat(opts *saveOptions) error {
switch opts.format {
case constant.DockerTransport:
opts.format = constant.DockerArchiveTransport
@@ -169,14 +170,6 @@ func checkFormatAndExpandTag(opts *saveOptions) error {
return errors.New("wrong image format provided")
}
- for i, imageName := range opts.oriImgList {
- nameWithTag, err := image.CheckAndAddDefaultTag(imageName, opts.localStore)
- if err != nil {
- return errors.Wrapf(err, "check format and expand tag failed with image name %q", imageName)
- }
- opts.oriImgList[i] = nameWithTag
- }
-
return nil
}
@@ -205,12 +198,11 @@ func filterImageName(opts *saveOptions) error {
opts.finalImageOrdered = append(opts.finalImageOrdered, img.ID)
}
- ref, err := reference.Parse(imageName)
- if err != nil {
- return errors.Wrapf(err, "filter image name failed when parsing name %q", imageName)
- }
- tagged, withTag := ref.(reference.NamedTagged)
- if withTag {
+ if !strings.HasPrefix(img.ID, imageName) {
+ tagged, _, err := image.GetNamedTaggedReference(imageName)
+ if err != nil {
+ return errors.Wrapf(err, "get named tagged reference failed when saving image %q", imageName)
+ }
finalImage.tags = append(finalImage.tags, tagged)
}
opts.finalImageSet[img.ID] = finalImage
diff --git a/exporter/docker/archive/archive.go b/exporter/docker/archive/archive.go
index 36a2881..60e67fd 100644
--- a/exporter/docker/archive/archive.go
+++ b/exporter/docker/archive/archive.go
@@ -19,7 +19,6 @@ import (
"sync"
"github.com/containers/image/v5/docker/archive"
- "github.com/containers/image/v5/transports/alltransports"
"github.com/containers/image/v5/types"
"github.com/pkg/errors"
@@ -50,24 +49,47 @@ func (d *dockerArchiveExporter) Name() string {
}
func (d *dockerArchiveExporter) Init(opts exporter.ExportOptions, src, destSpec string, localStore *store.Store) error {
- var (
- srcReference types.ImageReference
- destReference types.ImageReference
- err error
- )
+ var archiveFilePath string
+
const partsNum = 2
- // src could be form of ImageID digest or name[:tag]
+
+ // src is an imageid digest
// destSpec could be "file:name:tag" or "file:name" or just "file" with transport "docker-archive", such as docker-archive:output.tar:name:tag
- // When more than two parts, build must be called
+ if parts := strings.Split(destSpec, ":"); len(parts) < partsNum {
+ return errors.Errorf("image name %q is invalid", destSpec)
+ } else if len(parts) == partsNum {
+ archiveFilePath = strings.SplitN(destSpec, ":", partsNum)[1]
+ } else {
+ fileNameTag := strings.SplitN(destSpec, ":", partsNum)[1]
+ archiveFilePath = strings.SplitN(fileNameTag, ":", partsNum)[0]
+ }
+
+ if DockerArchiveExporter.GetArchiveWriter(opts.ExportID) == nil {
+ archWriter, err := archive.NewWriter(opts.SystemContext, archiveFilePath)
+ if err != nil {
+ return errors.Wrapf(err, "create archive writer failed")
+ }
+ DockerArchiveExporter.InitArchiveWriter(opts.ExportID, archWriter)
+ }
+
+ // when destSpec is more than two parts, build operation must be called
if parts := strings.Split(destSpec, ":"); len(parts) > partsNum {
- srcReference, _, err = image.FindImage(localStore, src)
+ srcReference, _, err := image.FindImage(localStore, src)
if err != nil {
return errors.Wrapf(err, "find src image: %q failed with transport %q", src, d.Name())
}
- destReference, err = alltransports.ParseImageName(destSpec)
+
+ // removing docker.io/library/ prefix by not using alltransports.ParseImageName
+ namedTagged, _, err := image.GetNamedTaggedReference(strings.Join(parts[2:], ":"))
+ if err != nil {
+ return errors.Wrapf(err, "get named tagged reference of image %q failed", strings.Join(parts[2:], ":"))
+ }
+ archiveWriter := DockerArchiveExporter.GetArchiveWriter(opts.ExportID)
+ destReference, err := archiveWriter.NewReference(namedTagged)
if err != nil {
return errors.Wrapf(err, "parse dest spec: %q failed with transport %q", destSpec, d.Name())
}
+
d.Lock()
d.items[opts.ExportID] = exporter.Bus{
SrcRef: srcReference,
@@ -79,23 +101,13 @@ func (d *dockerArchiveExporter) Init(opts exporter.ExportOptions, src, destSpec
}
// from build or save, we can get path from the other part
- archiveFilePath := strings.SplitN(destSpec, ":", partsNum)[1]
-
- if DockerArchiveExporter.GetArchiveWriter(opts.ExportID) == nil {
- archWriter, wErr := archive.NewWriter(opts.SystemContext, archiveFilePath)
- if wErr != nil {
- return errors.Wrapf(wErr, "create archive writer failed")
- }
- DockerArchiveExporter.InitArchiveWriter(opts.ExportID, archWriter)
- }
-
- srcReference, _, err = image.FindImage(localStore, src)
+ srcReference, _, err := image.FindImage(localStore, src)
if err != nil {
return errors.Wrapf(err, "find src image: %q failed with transport %q", src, d.Name())
}
archiveWriter := DockerArchiveExporter.GetArchiveWriter(opts.ExportID)
- destReference, err = archiveWriter.NewReference(nil)
+ destReference, err := archiveWriter.NewReference(nil)
if err != nil {
return errors.Wrapf(err, "parse dest spec: %q failed", destSpec)
}
diff --git a/image/image.go b/image/image.go
index b6210f2..5dda185 100644
--- a/image/image.go
+++ b/image/image.go
@@ -482,7 +482,7 @@ func FindImage(store *store.Store, image string) (types.ImageReference, *storage
if err != nil {
return nil, nil, errors.Wrapf(err, "failed to parse image %q in local store", localName)
}
-
+
return ref, img, nil
}
@@ -528,9 +528,8 @@ func tryResolveNameInStore(name string, store *store.Store) string {
return img.ID
}
- defaultTag := "latest"
- logrus.Infof("Try to find image: %s:%s in local storage", name, defaultTag)
- img, err = store.Image(fmt.Sprintf("%s:%s", name, defaultTag))
+ logrus.Infof("Try to find image: %s:%s in local storage", name, constant.DefaultTag)
+ img, err = store.Image(fmt.Sprintf("%s:%s", name, constant.DefaultTag))
if err != nil {
return ""
}
@@ -621,25 +620,24 @@ func tryResolveNameInRegistries(name string, sc *types.SystemContext) ([]string,
return candidates, constant.DockerTransport
}
-// CheckAndAddDefaultTag checks if src is format of repository[:tag], add default tag if src without tag
-func CheckAndAddDefaultTag(imageName string, store *store.Store) (string, error) {
- _, img, err := FindImage(store, imageName)
- if err != nil {
- return "", errors.Wrapf(err, "find src image: %q failed", imageName)
+// GetNamedTaggedReference checks an image name, if it does not include a tag, default tag "latest" will be added to it.
+func GetNamedTaggedReference(image string) (reference.NamedTagged, string, error) {
+ if image == "" {
+ return nil, "", nil
}
- defaultTag := "latest"
- for _, name := range img.Names {
- // for imageName is the format of repository[:tag]
- if imageName == name {
- return imageName, nil
- }
- // for name is the format of repository
- if fmt.Sprintf("%s:%s", imageName, defaultTag) == name {
- return name, nil
- }
+ if slashLastIndex, sepLastIndex := strings.LastIndex(image, "/"), strings.LastIndex(image, ":"); sepLastIndex == -1 || (sepLastIndex < slashLastIndex) {
+ image = fmt.Sprintf("%s:%s", image, constant.DefaultTag)
+ }
+
+ ref, err := reference.Parse(image)
+ if err != nil {
+ return nil, "", errors.Wrapf(err, "filter image name failed when parsing name %q", image)
+ }
+ tagged, withTag := ref.(reference.NamedTagged)
+ if !withTag {
+ return nil, "", errors.Errorf("image %q does not contain a tag even though the default tag is added", image)
}
- // for imageName is the format of imageID
- return imageName, nil
+ return tagged, image, nil
}
diff --git a/image/image_test.go b/image/image_test.go
index 43d936f..15b13e1 100644
--- a/image/image_test.go
+++ b/image/image_test.go
@@ -123,3 +123,99 @@ registries = []
assert.Assert(t, cmp.Contains(candidates, "localhost/busybox:latest"))
assert.Equal(t, transport, constant.DockerTransport)
}
+
+func TestGetNamedTaggedReference(t *testing.T) {
+ type testcase struct {
+ name string
+ tag string
+ output string
+ wantErr bool
+ errString string
+ }
+ testcases := []testcase{
+ {
+ name: "test 1",
+ tag: "isula/test",
+ output: "isula/test:latest",
+ wantErr: false,
+ },
+ {
+ name: "test 2",
+ tag: "localhost:5000/test",
+ output: "localhost:5000/test:latest",
+ wantErr: false,
+ },
+ {
+ name: "test 3",
+ tag: "isula/test:latest",
+ output: "isula/test:latest",
+ wantErr: false,
+ },
+ {
+ name: "test 4",
+ tag: "localhost:5000/test:latest",
+ output: "localhost:5000/test:latest",
+ wantErr: false,
+ },
+ {
+ name: "test 5",
+ tag: "localhost:5000:aaa/test:latest",
+ output: "",
+ wantErr: true,
+ errString: "invalid reference format",
+ },
+ {
+ name: "test 6",
+ tag: "localhost:5000:aaa/test",
+ output: "",
+ wantErr: true,
+ errString: "invalid reference format",
+ },
+ {
+ name: "test 7",
+ tag: "localhost:5000/test:latest:latest",
+ output: "",
+ wantErr: true,
+ errString: "invalid reference format",
+ },
+ {
+ name: "test 8",
+ tag: "test:latest:latest",
+ output: "",
+ wantErr: true,
+ errString: "invalid reference format",
+ },
+ {
+ name: "test 9",
+ tag: "",
+ output: "",
+ wantErr: false,
+ },
+ {
+ name: "test 10",
+ tag: "abc efg:latest",
+ output: "",
+ wantErr: true,
+ errString: "invalid reference format",
+ },
+ {
+ name: "test 11",
+ tag: "abc!@#:latest",
+ output: "",
+ wantErr: true,
+ errString: "invalid reference format",
+ },
+ }
+ for _, tc := range testcases {
+ t.Run(tc.name, func(t *testing.T) {
+ _, tag, err := GetNamedTaggedReference(tc.tag)
+ if !tc.wantErr {
+ assert.Equal(t, tag, tc.output, tc.name)
+ }
+ if tc.wantErr {
+ assert.ErrorContains(t, err, tc.errString)
+ }
+ })
+
+ }
+}
--
1.8.3.1
Reference in New Issue View Git Blame Copy Permalink