351 lines
12 KiB
Diff
351 lines
12 KiB
Diff
From a3638072985a0cb71ff561ad5e5bbc2454f81c1f Mon Sep 17 00:00:00 2001
|
|
From: DCCooper <1866858@gmail.com>
|
|
Date: Wed, 8 Dec 2021 12:51:20 +0800
|
|
Subject: [PATCH 2/2] isula-build: fix problems found by code review
|
|
|
|
Signed-off-by: DCCooper <1866858@gmail.com>
|
|
---
|
|
daemon/load.go | 67 ++++++++++++++++++++++++-------------------------
|
|
daemon/save.go | 79 ++++++++++++++++++++--------------------------------------
|
|
image/image.go | 3 ++-
|
|
util/cipher.go | 8 +++++-
|
|
4 files changed, 69 insertions(+), 88 deletions(-)
|
|
|
|
diff --git a/daemon/load.go b/daemon/load.go
|
|
index 378325c..894159b 100644
|
|
--- a/daemon/load.go
|
|
+++ b/daemon/load.go
|
|
@@ -69,9 +69,9 @@ type separatorLoad struct {
|
|
}
|
|
|
|
type loadOptions struct {
|
|
+ logEntry *logrus.Entry
|
|
path string
|
|
format string
|
|
- logEntry *logrus.Entry
|
|
sep separatorLoad
|
|
}
|
|
|
|
@@ -355,7 +355,7 @@ func (s *separatorLoad) getTarballInfo() error {
|
|
return errors.Wrap(err, "join manifest file path failed")
|
|
}
|
|
|
|
- var t = make(map[string]tarballInfo)
|
|
+ var t = make(map[string]tarballInfo, 1)
|
|
if err = util.LoadJSONFile(manifest, &t); err != nil {
|
|
return errors.Wrap(err, "load manifest file failed")
|
|
}
|
|
@@ -370,7 +370,7 @@ func (s *separatorLoad) getTarballInfo() error {
|
|
}
|
|
|
|
func (s *separatorLoad) constructTarballInfo() (err error) {
|
|
- s.log.Infof("construct image tarball info for %s", s.appName)
|
|
+ s.log.Infof("Construct image tarball info for %s", s.appName)
|
|
// fill up path for separator
|
|
// this case should not happened since client side already check this flag
|
|
if len(s.appName) == 0 {
|
|
@@ -408,26 +408,25 @@ func (s *separatorLoad) tarballCheckSum() error {
|
|
return nil
|
|
}
|
|
|
|
- // app image tarball can not be empty
|
|
- if len(s.appPath) == 0 {
|
|
- return errors.New("app image tarball path can not be empty")
|
|
- }
|
|
- if err := util.CheckSum(s.appPath, s.info.AppHash); err != nil {
|
|
- return errors.Wrapf(err, "check sum for file %q failed", s.appPath)
|
|
- }
|
|
-
|
|
- // base image tarball can not be empty
|
|
- if len(s.basePath) == 0 {
|
|
- return errors.New("base image tarball path can not be empty")
|
|
- }
|
|
- if err := util.CheckSum(s.basePath, s.info.BaseHash); err != nil {
|
|
- return errors.Wrapf(err, "check sum for file %q failed", s.basePath)
|
|
- }
|
|
-
|
|
- // lib image may be empty image
|
|
- if len(s.libPath) != 0 {
|
|
- if err := util.CheckSum(s.libPath, s.info.LibHash); err != nil {
|
|
- return errors.Wrapf(err, "check sum for file %q failed", s.libPath)
|
|
+ type checkInfo struct {
|
|
+ path string
|
|
+ hash string
|
|
+ str string
|
|
+ canBeEmpty bool
|
|
+ }
|
|
+ checkLen := 3
|
|
+ var checkList = make([]checkInfo, 0, checkLen)
|
|
+ checkList = append(checkList, checkInfo{path: s.basePath, hash: s.info.BaseHash, canBeEmpty: false, str: "base image"})
|
|
+ checkList = append(checkList, checkInfo{path: s.libPath, hash: s.info.LibHash, canBeEmpty: true, str: "lib image"})
|
|
+ checkList = append(checkList, checkInfo{path: s.appPath, hash: s.info.AppHash, canBeEmpty: false, str: "app image"})
|
|
+ for _, p := range checkList {
|
|
+ if len(p.path) == 0 && !p.canBeEmpty {
|
|
+ return errors.Errorf("%s tarball path can not be empty", p.str)
|
|
+ }
|
|
+ if len(p.path) != 0 {
|
|
+ if err := util.CheckSum(p.path, p.hash); err != nil {
|
|
+ return errors.Wrapf(err, "check sum for file %q failed", p.path)
|
|
+ }
|
|
}
|
|
}
|
|
|
|
@@ -457,18 +456,18 @@ func (s *separatorLoad) unpackTarballs() error {
|
|
return errors.Wrap(err, "failed to make temporary directories")
|
|
}
|
|
|
|
- // unpack base first and the later images will be moved here
|
|
- if err := util.UnpackFile(s.basePath, s.tmpDir.base, archive.Gzip, false); err != nil {
|
|
- return errors.Wrapf(err, "unpack base tarball %q failed", s.basePath)
|
|
- }
|
|
-
|
|
- if err := util.UnpackFile(s.appPath, s.tmpDir.app, archive.Gzip, false); err != nil {
|
|
- return errors.Wrapf(err, "unpack app tarball %q failed", s.appPath)
|
|
- }
|
|
+ type unpackInfo struct{ path, dir, str string }
|
|
+ unpackLen := 3
|
|
+ var unpackList = make([]unpackInfo, 0, unpackLen)
|
|
+ unpackList = append(unpackList, unpackInfo{path: s.basePath, dir: s.tmpDir.base, str: "base image"})
|
|
+ unpackList = append(unpackList, unpackInfo{path: s.appPath, dir: s.tmpDir.app, str: "app image"})
|
|
+ unpackList = append(unpackList, unpackInfo{path: s.libPath, dir: s.tmpDir.lib, str: "lib image"})
|
|
|
|
- if len(s.libPath) != 0 {
|
|
- if err := util.UnpackFile(s.libPath, s.tmpDir.lib, archive.Gzip, false); err != nil {
|
|
- return errors.Wrapf(err, "unpack lib tarball %q failed", s.libPath)
|
|
+ for _, p := range unpackList {
|
|
+ if len(p.path) != 0 {
|
|
+ if err := util.UnpackFile(p.path, p.dir, archive.Gzip, false); err != nil {
|
|
+ return errors.Wrapf(err, "unpack %s tarball %q failed", p.str, p.path)
|
|
+ }
|
|
}
|
|
}
|
|
|
|
diff --git a/daemon/save.go b/daemon/save.go
|
|
index f14a485..7a110bd 100644
|
|
--- a/daemon/save.go
|
|
+++ b/daemon/save.go
|
|
@@ -77,9 +77,9 @@ type saveOptions struct {
|
|
}
|
|
|
|
type separatorSave struct {
|
|
+ log *logrus.Entry
|
|
renameData []renames
|
|
tmpDir imageTmpDir
|
|
- log *logrus.Entry
|
|
base string
|
|
lib string
|
|
dest string
|
|
@@ -190,7 +190,7 @@ func (b *Backend) Save(req *pb.SaveRequest, stream pb.Control_SaveServer) (err e
|
|
}).Info("SaveRequest received")
|
|
|
|
opts := b.getSaveOptions(req)
|
|
- if err = opts.check(); err != nil {
|
|
+ if err = opts.manage(); err != nil {
|
|
return errors.Wrap(err, "check save options failed")
|
|
}
|
|
|
|
@@ -278,17 +278,17 @@ func messageHandler(stream pb.Control_SaveServer, cliLogger *logger.Logger) func
|
|
}
|
|
}
|
|
|
|
-func (opts *saveOptions) check() error {
|
|
+func (opts *saveOptions) manage() error {
|
|
if err := opts.checkImageNameIsID(); err != nil {
|
|
return err
|
|
}
|
|
- if err := opts.checkFormat(); err != nil {
|
|
+ if err := opts.setFormat(); err != nil {
|
|
return err
|
|
}
|
|
if err := opts.filterImageName(); err != nil {
|
|
return err
|
|
}
|
|
- if err := opts.checkRenameFile(); err != nil {
|
|
+ if err := opts.loadRenameFile(); err != nil {
|
|
return err
|
|
}
|
|
|
|
@@ -318,7 +318,7 @@ func (opts *saveOptions) checkImageNameIsID() error {
|
|
return nil
|
|
}
|
|
|
|
-func (opts *saveOptions) checkFormat() error {
|
|
+func (opts *saveOptions) setFormat() error {
|
|
switch opts.format {
|
|
case constant.DockerTransport:
|
|
opts.format = constant.DockerArchiveTransport
|
|
@@ -337,7 +337,7 @@ func (opts *saveOptions) filterImageName() error {
|
|
return nil
|
|
}
|
|
|
|
- visitedImage := make(map[string]bool)
|
|
+ visitedImage := make(map[string]bool, 1)
|
|
for _, imageName := range opts.oriImgList {
|
|
if _, exists := visitedImage[imageName]; exists {
|
|
continue
|
|
@@ -351,8 +351,7 @@ func (opts *saveOptions) filterImageName() error {
|
|
|
|
finalImage, ok := opts.finalImageSet[img.ID]
|
|
if !ok {
|
|
- finalImage = &savedImage{exist: true}
|
|
- finalImage.tags = []reference.NamedTagged{}
|
|
+ finalImage = &savedImage{exist: true, tags: []reference.NamedTagged{}}
|
|
opts.finalImageOrdered = append(opts.finalImageOrdered, img.ID)
|
|
}
|
|
|
|
@@ -369,7 +368,7 @@ func (opts *saveOptions) filterImageName() error {
|
|
return nil
|
|
}
|
|
|
|
-func (opts *saveOptions) checkRenameFile() error {
|
|
+func (opts *saveOptions) loadRenameFile() error {
|
|
if len(opts.sep.renameFile) != 0 {
|
|
var reName []renames
|
|
if err := util.LoadJSONFile(opts.sep.renameFile, &reName); err != nil {
|
|
@@ -494,12 +493,11 @@ func (s *separatorSave) adjustLayers() ([]imageManifest, error) {
|
|
return man, nil
|
|
}
|
|
|
|
-func separateImage(opt saveOptions) error {
|
|
+func separateImage(opt saveOptions) (err error) {
|
|
s := &opt.sep
|
|
s.log.Infof("Start saving separated images %v", opt.oriImgList)
|
|
- var errList []error
|
|
|
|
- if err := os.MkdirAll(s.dest, constant.DefaultRootDirMode); err != nil {
|
|
+ if err = os.MkdirAll(s.dest, constant.DefaultRootDirMode); err != nil {
|
|
return err
|
|
}
|
|
|
|
@@ -507,30 +505,26 @@ func separateImage(opt saveOptions) error {
|
|
if tErr := os.RemoveAll(s.tmpDir.root); tErr != nil && !os.IsNotExist(tErr) {
|
|
s.log.Warnf("Removing save tmp directory %q failed: %v", s.tmpDir.root, tErr)
|
|
}
|
|
- if len(errList) != 0 {
|
|
+ if err != nil {
|
|
if rErr := os.RemoveAll(s.dest); rErr != nil && !os.IsNotExist(rErr) {
|
|
s.log.Warnf("Removing save dest directory %q failed: %v", s.dest, rErr)
|
|
}
|
|
}
|
|
}()
|
|
- if err := util.UnpackFile(opt.outputPath, s.tmpDir.untar, archive.Gzip, true); err != nil {
|
|
- errList = append(errList, err)
|
|
+ if err = util.UnpackFile(opt.outputPath, s.tmpDir.untar, archive.Gzip, true); err != nil {
|
|
return errors.Wrapf(err, "unpack %q failed", opt.outputPath)
|
|
}
|
|
- manifest, err := s.adjustLayers()
|
|
- if err != nil {
|
|
- errList = append(errList, err)
|
|
- return errors.Wrap(err, "adjust layers failed")
|
|
+ manifest, aErr := s.adjustLayers()
|
|
+ if aErr != nil {
|
|
+ return errors.Wrap(aErr, "adjust layers failed")
|
|
}
|
|
|
|
- imgInfos, err := s.constructImageInfos(manifest, opt.localStore)
|
|
- if err != nil {
|
|
- errList = append(errList, err)
|
|
- return errors.Wrap(err, "process image infos failed")
|
|
+ imgInfos, cErr := s.constructImageInfos(manifest, opt.localStore)
|
|
+ if cErr != nil {
|
|
+ return errors.Wrap(cErr, "process image infos failed")
|
|
}
|
|
|
|
- if err := s.processImageLayers(imgInfos); err != nil {
|
|
- errList = append(errList, err)
|
|
+ if err = s.processImageLayers(imgInfos); err != nil {
|
|
return err
|
|
}
|
|
|
|
@@ -552,7 +546,7 @@ func (s *separatorSave) processImageLayers(imgInfos map[string]imageInfo) error
|
|
sort.Strings(sortedKey)
|
|
for _, k := range sortedKey {
|
|
info := imgInfos[k]
|
|
- if err := s.clearDirs(true); err != nil {
|
|
+ if err := s.clearTempDirs(); err != nil {
|
|
return errors.Wrap(err, "clear tmp dirs failed")
|
|
}
|
|
var t tarballInfo
|
|
@@ -584,32 +578,13 @@ func (s *separatorSave) processImageLayers(imgInfos map[string]imageInfo) error
|
|
return nil
|
|
}
|
|
|
|
-func (s *separatorSave) clearDirs(reCreate bool) error {
|
|
- tmpDir := s.tmpDir
|
|
- dirs := []string{tmpDir.base, tmpDir.app, tmpDir.lib}
|
|
- var mkTmpDirs = func(dirs []string) error {
|
|
- for _, dir := range dirs {
|
|
- if err := os.MkdirAll(dir, constant.DefaultRootDirMode); err != nil {
|
|
- return err
|
|
- }
|
|
- }
|
|
- return nil
|
|
- }
|
|
-
|
|
- var rmTmpDirs = func(dirs []string) error {
|
|
- for _, dir := range dirs {
|
|
- if err := os.RemoveAll(dir); err != nil {
|
|
- return err
|
|
- }
|
|
+func (s *separatorSave) clearTempDirs() error {
|
|
+ dirs := []string{s.tmpDir.base, s.tmpDir.app, s.tmpDir.lib}
|
|
+ for _, dir := range dirs {
|
|
+ if err := os.RemoveAll(dir); err != nil {
|
|
+ return err
|
|
}
|
|
- return nil
|
|
- }
|
|
-
|
|
- if err := rmTmpDirs(dirs); err != nil {
|
|
- return err
|
|
- }
|
|
- if reCreate {
|
|
- if err := mkTmpDirs(dirs); err != nil {
|
|
+ if err := os.MkdirAll(dir, constant.DefaultRootDirMode); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
diff --git a/image/image.go b/image/image.go
|
|
index b24cb41..37cd7fa 100644
|
|
--- a/image/image.go
|
|
+++ b/image/image.go
|
|
@@ -626,7 +626,8 @@ func GetNamedTaggedReference(image string) (reference.NamedTagged, string, error
|
|
return nil, "", nil
|
|
}
|
|
|
|
- if slashLastIndex, sepLastIndex := strings.LastIndex(image, "/"), strings.LastIndex(image, ":"); sepLastIndex == -1 || (sepLastIndex < slashLastIndex) {
|
|
+ slashLastIndex, sepLastIndex := strings.LastIndex(image, "/"), strings.LastIndex(image, ":")
|
|
+ if sepLastIndex == -1 || (sepLastIndex < slashLastIndex) {
|
|
image = fmt.Sprintf("%s:%s", image, constant.DefaultTag)
|
|
}
|
|
|
|
diff --git a/util/cipher.go b/util/cipher.go
|
|
index a5e3125..67cb52b 100644
|
|
--- a/util/cipher.go
|
|
+++ b/util/cipher.go
|
|
@@ -212,6 +212,9 @@ func GenRSAPublicKeyFile(key *rsa.PrivateKey, path string) error {
|
|
if err := pem.Encode(file, block); err != nil {
|
|
return err
|
|
}
|
|
+ if cErr := file.Close(); cErr != nil {
|
|
+ return cErr
|
|
+ }
|
|
|
|
return nil
|
|
}
|
|
@@ -230,7 +233,10 @@ func ReadPublicKey(path string) (rsa.PublicKey, error) {
|
|
if err != nil {
|
|
return rsa.PublicKey{}, err
|
|
}
|
|
- key := pubInterface.(*rsa.PublicKey)
|
|
+ key, ok := pubInterface.(*rsa.PublicKey)
|
|
+ if !ok {
|
|
+ return rsa.PublicKey{}, errors.New("failed to find public key type")
|
|
+ }
|
|
|
|
return *key, nil
|
|
}
|
|
--
|
|
1.8.3.1
|
|
|