From 2b845fdb3e3c9d23b0fec856bcd5ce8ced868683 Mon Sep 17 00:00:00 2001
From: DCCooper <1866858@gmail.com>
Date: Thu, 4 Nov 2021 14:38:20 +0800
Subject: [PATCH] isula-build:fix panic when using image ID to save separated
 image

Signed-off-by: DCCooper <1866858@gmail.com>
---
 daemon/save.go                                     | 120 +++++++++++++--------
 .../src/cover_test_save_separated_image_failed.sh  |  28 +++++
 .../src/cover_test_save_separated_image_success.sh |  24 ++++-
 3 files changed, 127 insertions(+), 45 deletions(-)

diff --git a/daemon/save.go b/daemon/save.go
index 9ad4e03..9c5e563 100644
--- a/daemon/save.go
+++ b/daemon/save.go
@@ -83,6 +83,7 @@ type separatorSave struct {
 	base       string
 	lib        string
 	dest       string
+	renameFile string
 	enabled    bool
 }
 
@@ -141,15 +142,7 @@ type tarballInfo struct {
 	BaseLayers    []string `json:"baseLayer"`
 }
 
-func (b *Backend) getSaveOptions(req *pb.SaveRequest) (saveOptions, error) {
-	var sep = separatorSave{
-		base:    req.GetSep().GetBase(),
-		lib:     req.GetSep().GetLib(),
-		dest:    req.GetSep().GetDest(),
-		log:     logrus.WithFields(logrus.Fields{"SaveID": req.GetSaveID()}),
-		enabled: req.GetSep().GetEnabled(),
-	}
-
+func (b *Backend) getSaveOptions(req *pb.SaveRequest) saveOptions {
 	var opt = saveOptions{
 		sysCtx:            image.GetSystemContext(),
 		localStore:        b.daemon.localStore,
@@ -161,11 +154,19 @@ func (b *Backend) getSaveOptions(req *pb.SaveRequest) (saveOptions, error) {
 		outputPath:        req.GetPath(),
 		logger:            logger.NewCliLogger(constant.CliLogBufferLen),
 		logEntry:          logrus.WithFields(logrus.Fields{"SaveID": req.GetSaveID(), "Format": req.GetFormat()}),
-		sep:               sep,
 	}
 	// normal save
-	if !sep.enabled {
-		return opt, nil
+	if !req.GetSep().GetEnabled() {
+		return opt
+	}
+
+	opt.sep = separatorSave{
+		base:       req.GetSep().GetBase(),
+		lib:        req.GetSep().GetLib(),
+		dest:       req.GetSep().GetDest(),
+		log:        logrus.WithFields(logrus.Fields{"SaveID": req.GetSaveID()}),
+		enabled:    req.GetSep().GetEnabled(),
+		renameFile: req.GetSep().GetRename(),
 	}
 
 	// save separated image
@@ -175,44 +176,22 @@ func (b *Backend) getSaveOptions(req *pb.SaveRequest) (saveOptions, error) {
 	baseDir := filepath.Join(tmpRoot, baseUntarTempDirName)
 	libDir := filepath.Join(tmpRoot, libUntarTempDirName)
 
-	opt.sep.tmpDir = imageTmpDir{
-		app:   appDir,
-		base:  baseDir,
-		lib:   libDir,
-		untar: untar,
-		root:  tmpRoot,
-	}
+	opt.sep.tmpDir = imageTmpDir{app: appDir, base: baseDir, lib: libDir, untar: untar, root: tmpRoot}
 	opt.outputPath = filepath.Join(untar, unionTarName)
-	renameFile := req.GetSep().GetRename()
-	if len(renameFile) != 0 {
-		var reName []renames
-		if err := util.LoadJSONFile(renameFile, &reName); err != nil {
-			return saveOptions{}, err
-		}
-		opt.sep.renameData = reName
-	}
 
-	return opt, nil
+	return opt
 }
 
 // Save receives a save request and save the image(s) into tarball
-func (b *Backend) Save(req *pb.SaveRequest, stream pb.Control_SaveServer) error {
+func (b *Backend) Save(req *pb.SaveRequest, stream pb.Control_SaveServer) (err error) {
 	logrus.WithFields(logrus.Fields{
 		"SaveID": req.GetSaveID(),
 		"Format": req.GetFormat(),
 	}).Info("SaveRequest received")
 
-	var err error
-	opts, err := b.getSaveOptions(req)
-	if err != nil {
-		return errors.Wrap(err, "process save options failed")
-	}
-
-	if err = checkFormat(&opts); err != nil {
-		return err
-	}
-	if err = filterImageName(&opts); err != nil {
-		return err
+	opts := b.getSaveOptions(req)
+	if err = opts.check(); err != nil {
+		return errors.Wrap(err, "check save options failed")
 	}
 
 	defer func() {
@@ -299,7 +278,47 @@ func messageHandler(stream pb.Control_SaveServer, cliLogger *logger.Logger) func
 	}
 }
 
-func checkFormat(opts *saveOptions) error {
+func (opts *saveOptions) check() error {
+	if err := opts.checkImageNameIsID(); err != nil {
+		return err
+	}
+	if err := opts.checkFormat(); err != nil {
+		return err
+	}
+	if err := opts.filterImageName(); err != nil {
+		return err
+	}
+	if err := opts.checkRenameFile(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (opts *saveOptions) checkImageNameIsID() error {
+	imageNames := opts.oriImgList
+	if opts.sep.enabled {
+		if len(opts.sep.base) != 0 {
+			imageNames = append(imageNames, opts.sep.base)
+		}
+		if len(opts.sep.lib) != 0 {
+			imageNames = append(imageNames, opts.sep.lib)
+		}
+	}
+	for _, name := range imageNames {
+		_, img, err := image.FindImage(opts.localStore, name)
+		if err != nil {
+			return errors.Wrapf(err, "check image name failed when finding image name %q", name)
+		}
+		if strings.HasPrefix(img.ID, name) && opts.sep.enabled {
+			return errors.Errorf("using image ID %q as image name to save separated image is not allowed", name)
+		}
+	}
+
+	return nil
+}
+
+func (opts *saveOptions) checkFormat() error {
 	switch opts.format {
 	case constant.DockerTransport:
 		opts.format = constant.DockerArchiveTransport
@@ -312,7 +331,7 @@ func checkFormat(opts *saveOptions) error {
 	return nil
 }
 
-func filterImageName(opts *saveOptions) error {
+func (opts *saveOptions) filterImageName() error {
 	if opts.format == constant.OCIArchiveTransport {
 		opts.finalImageOrdered = opts.oriImgList
 		return nil
@@ -350,6 +369,18 @@ func filterImageName(opts *saveOptions) error {
 	return nil
 }
 
+func (opts *saveOptions) checkRenameFile() error {
+	if len(opts.sep.renameFile) != 0 {
+		var reName []renames
+		if err := util.LoadJSONFile(opts.sep.renameFile, &reName); err != nil {
+			return errors.Wrap(err, "check rename file failed")
+		}
+		opts.sep.renameData = reName
+	}
+
+	return nil
+}
+
 func getLayerHashFromStorage(store *store.Store, name string) ([]string, error) {
 	if len(name) == 0 {
 		return nil, nil
@@ -770,6 +801,11 @@ func getLayersID(layer []string) []string {
 
 func (s *separatorSave) constructSingleImgInfo(mani imageManifest, store *store.Store) (imageInfo, error) {
 	var libLayers, appLayers []string
+	// image name should not be empty here
+	if len(mani.RepoTags) == 0 {
+		return imageInfo{}, errors.New("image name and tag is empty")
+	}
+	// if there is more than one repoTag, will use first one as image name
 	imageRepoFields := strings.Split(mani.RepoTags[0], ":")
 	imageLayers := getLayersID(mani.Layers)
 
diff --git a/tests/src/cover_test_save_separated_image_failed.sh b/tests/src/cover_test_save_separated_image_failed.sh
index c64dcf5..66db580 100644
--- a/tests/src/cover_test_save_separated_image_failed.sh
+++ b/tests/src/cover_test_save_separated_image_failed.sh
@@ -89,6 +89,33 @@ function test_run6() {
     rm -rf "${workspace}"/Images
 }
 
+# using image id to save
+function test_run7() {
+    base_image_id=$(isula-build ctr-img images ${base_image_name} | tail -n 2 | head -n 1 | awk '{print $3}')
+    lib_image_id=$(isula-build ctr-img images ${lib_image_name} | tail -n 2 | head -n 1 | awk '{print $3}')
+    app_image_id=$(isula-build ctr-img images ${app1_image_name} | tail -n 2 | head -n 1 | awk '{print $3}')
+    app1_image_id=$(isula-build ctr-img images ${app2_image_name} | tail -n 2 | head -n 1 | awk '{print $3}')
+    # all name is image id
+    isula-build ctr-img save -b "${base_image_id}" -l "${lib_image_id}" -d "${workspace}"/Images "${app1_image_id}" "${app2_image_id}"
+    check_result_not_equal $? 0
+    rm -rf "${workspace}"/Images
+
+    # app name is image id
+    isula-build ctr-img save -b "${base_image_name}" -l "${lib_image_name}" -d "${workspace}"/Images "${app1_image_id}" "${app2_image_name}"
+    check_result_not_equal $? 0
+    rm -rf "${workspace}"/Images
+
+    # lib name is image id
+    isula-build ctr-img save -b "${base_image_name}" -l "${lib_image_id}" -d "${workspace}"/Images "${app1_image_name}" "${app2_image_name}"
+    check_result_not_equal $? 0
+    rm -rf "${workspace}"/Images
+
+    # base name is image id
+    isula-build ctr-img save -b "${base_image_id}" -l "${lib_image_name}" -d "${workspace}"/Images "${app1_image_name}" "${app2_image_name}"
+    check_result_not_equal $? 0
+    rm -rf "${workspace}"/Images
+}
+
 function cleanup() {
     rm -rf "${workspace}"
     isula-build ctr-img rm "${bad_lib_image_name}" "${bad_app1_image_name}" "${bad_app2_image_name}" "${lib_image_name}" "${app1_image_name}" "${app2_image_name}"
@@ -102,6 +129,7 @@ test_run3
 test_run4
 test_run5
 test_run6
+test_run7
 cleanup
 # shellcheck disable=SC2154
 exit "${exit_flag}"
diff --git a/tests/src/cover_test_save_separated_image_success.sh b/tests/src/cover_test_save_separated_image_success.sh
index 2095bd3..68d2cae 100644
--- a/tests/src/cover_test_save_separated_image_success.sh
+++ b/tests/src/cover_test_save_separated_image_success.sh
@@ -26,6 +26,11 @@ function pre_run() {
     lib_image_name="lib:latest"
     app1_image_name="app1:latest"
     app2_image_name="app2:latest"
+    base_image_short_name="b:latest"
+    lib_image_short_name="l:latest"
+    app1_image_short_name="a:latest"
+    app2_image_short_name="c:latest"
+
     lib_layer_number=5
     app1_layer_number=4
     app2_layer_number=3
@@ -37,18 +42,31 @@ function pre_run() {
     build_image "${app2_image_name}" "${workspace}"
 }
 
-function test_run() {
+function test_run1() {
     isula-build ctr-img save -b "${base_image_name}" -l "${lib_image_name}" -d "${workspace}"/Images "${app1_image_name}" "${app2_image_name}"
     check_result_equal $? 0
+    rm -rf "${workspace}"
+}
+
+# use short image name
+function test_run2() {
+    isula-build ctr-img tag "${base_image_name}" "${base_image_short_name}"
+    isula-build ctr-img tag "${lib_image_name}" "${lib_image_short_name}"
+    isula-build ctr-img tag "${app1_image_name}" "${app1_image_short_name}"
+    isula-build ctr-img tag "${app2_image_name}" "${app2_image_short_name}"
+    isula-build ctr-img save -b "${base_image_short_name}" -l "${lib_image_short_name}" -d "${workspace}"/Images "${app1_image_short_name}" "${app2_image_short_name}"
+    check_result_equal $? 0
+    rm -rf "${workspace}"
 }
 
 function cleanup() {
     rm -rf "${workspace}"
-    isula-build ctr-img rm "${lib_image_name}" "${app1_image_name}" "${app2_image_name}"
+    isula-build ctr-img rm "${lib_image_name}" "${app1_image_name}" "${app2_image_name}" "${base_image_short_name}" "${lib_image_short_name}" "${app1_image_short_name}" "${app2_image_short_name}"
 }
 
 pre_run
-test_run
+test_run1
+test_run2
 cleanup
 # shellcheck disable=SC2154
 exit "${exit_flag}"
-- 
1.8.3.1