From acc1a70e0cbe6feb93efb2bef451ea993236bf70 Mon Sep 17 00:00:00 2001
From: TomSweeneyRedHat <tsweeney@redhat.com>
Date: Thu, 14 Jan 2021 15:12:08 -0500
Subject: [PATCH] Cherry pick localhost fix and update CI configuration for
 release-1.19 branch

Cherry pick @vrothberg's "use local image name for pull policy checks" #2908
and update the cirrus and git validations so the test will run on this new(ish)
branch.  From @vrothberg:

Some pull policies require to first look up a local image and compare
that to the remote counter part. When looking up the remote image, we
need to make sure to use the name of the local image, if it exists.

This fixes a bug where a short name resolved to an image with the
"localhost/" prefix. This prefix is only used for local image look ups
via shortnames.ResolveLocally. Hence, when looking up the remote
counter part, we must preserve this prefix.

Fixes: #2904

Signed-off-by: TomSweeneyRedHat <tsweeney@redhat.com>
---
 .cirrus.yml                      |  2 +-
 new.go                           | 24 +++++++++++++++---------
 tests/pull.bats                  | 15 +++++++++++++++
 tests/validate/git-validation.sh |  2 +-
 4 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 799766532ed..ed2fac11c02 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -6,7 +6,7 @@ env:
     #### Global variables used for all tasks
     ####
     # Name of the ultimate destination branch for this CI run, PR or post-merge.
-    DEST_BRANCH: "master"
+    DEST_BRANCH: "release-1.19"
     GOPATH: "/var/tmp/go"
     GOSRC: "${GOPATH}/src/github.com/containers/buildah"
     # Overrides default location (/tmp/cirrus) for repo clone
diff --git a/new.go b/new.go
index aab17fea270..4d70e014638 100644
--- a/new.go
+++ b/new.go
@@ -103,27 +103,27 @@ func newContainerIDMappingOptions(idmapOptions *IDMappingOptions) storage.IDMapp
 	return options
 }
 
-func resolveLocalImage(systemContext *types.SystemContext, store storage.Store, options BuilderOptions) (types.ImageReference, string, *storage.Image, error) {
+func resolveLocalImage(systemContext *types.SystemContext, store storage.Store, options BuilderOptions) (types.ImageReference, string, string, *storage.Image, error) {
 	candidates, _, _, err := util.ResolveName(options.FromImage, options.Registry, systemContext, store)
 	if err != nil {
-		return nil, "", nil, errors.Wrapf(err, "error resolving local image %q", options.FromImage)
+		return nil, "", "", nil, errors.Wrapf(err, "error resolving local image %q", options.FromImage)
 	}
-	for _, image := range candidates {
-		img, err := store.Image(image)
+	for _, imageName := range candidates {
+		img, err := store.Image(imageName)
 		if err != nil {
 			if errors.Cause(err) == storage.ErrImageUnknown {
 				continue
 			}
-			return nil, "", nil, err
+			return nil, "", "", nil, err
 		}
 		ref, err := is.Transport.ParseStoreReference(store, img.ID)
 		if err != nil {
-			return nil, "", nil, errors.Wrapf(err, "error parsing reference to image %q", img.ID)
+			return nil, "", "", nil, errors.Wrapf(err, "error parsing reference to image %q", img.ID)
 		}
-		return ref, ref.Transport().Name(), img, nil
+		return ref, ref.Transport().Name(), imageName, img, nil
 	}
 
-	return nil, "", nil, nil
+	return nil, "", "", nil, nil
 }
 
 func resolveImage(ctx context.Context, systemContext *types.SystemContext, store storage.Store, options BuilderOptions) (types.ImageReference, string, *storage.Image, error) {
@@ -145,7 +145,7 @@ func resolveImage(ctx context.Context, systemContext *types.SystemContext, store
 		}
 	}
 
-	localImageRef, _, localImage, err := resolveLocalImage(systemContext, store, options)
+	localImageRef, _, localImageName, localImage, err := resolveLocalImage(systemContext, store, options)
 	if err != nil {
 		return nil, "", nil, err
 	}
@@ -166,6 +166,12 @@ func resolveImage(ctx context.Context, systemContext *types.SystemContext, store
 		}
 	}
 
+	// If we found a local image, we must use it's name.
+	// See #2904.
+	if localImageRef != nil {
+		fromImage = localImageName
+	}
+
 	resolved, err := shortnames.Resolve(systemContext, fromImage)
 	if err != nil {
 		return nil, "", nil, err
diff --git a/tests/pull.bats b/tests/pull.bats
index 1466d56346e..288341302a6 100644
--- a/tests/pull.bats
+++ b/tests/pull.bats
@@ -2,6 +2,21 @@
 
 load helpers
 
+# Regression test for #2904
+@test "local-image resolution" {
+  run_buildah pull -q busybox
+  iid=$output
+  run_buildah tag ${iid} localhost/image
+
+  # We want to make sure that "image" will always resolve to "localhost/image"
+  # (given a local image with that name exists).  The trick we're using is to
+  # force a failed pull and look at the error message which *must* include the
+  # the resolved image name (localhost/image:latest).
+  run_buildah 125 pull --policy=always image
+  [[ "$output" == *"Error initializing source docker://localhost/image:latest"* ]]
+  run_buildah rmi localhost/image ${iid}
+}
+
 @test "pull-flags-order-verification" {
   run_buildah 125 pull image1 --tls-verify
   check_options_flag_err "--tls-verify"
diff --git a/tests/validate/git-validation.sh b/tests/validate/git-validation.sh
index 9012cc1f348..dd2f669ffe6 100755
--- a/tests/validate/git-validation.sh
+++ b/tests/validate/git-validation.sh
@@ -11,7 +11,7 @@ if [[ -z "$(type -P git-validation)" ]]; then
 	exit 1
 fi
 
-GITVALIDATE_EPOCH="${GITVALIDATE_EPOCH:-1f8bf4dba27d9a157f966dad3a1e0f58091091d8}"
+GITVALIDATE_EPOCH="${GITVALIDATE_EPOCH:-474febf0dcb3c76c23a88f3b83834c81f20a7e5c}"
 
 OUTPUT_OPTIONS="-q"
 if [[ "$CI" == 'true' ]]; then