From d7d00d8069fa3bd3f65b0caba26222e3f8eb052e Mon Sep 17 00:00:00 2001
From: Pit <pitkley@googlemail.com>
Date: Mon, 13 Jan 2020 11:41:52 +0100
Subject: [PATCH] Fix regression in docker-tag post-processor (#8593)

GH-8392 introduced the feature that multiple tags can be specified. This
(presumably inadvertently) broke the option to tag an image without the
actual tag specified -- Docker handles this by assuming `latest` as the
tag.

In addition, use-cases exist for the `repository` field containing the
tag already, which was also broken.
---
 CHANGELOG.md                                  |  1 +
 post-processor/docker-tag/post-processor.go   | 23 +++++++----
 .../docker-tag/post-processor_test.go         | 39 +++++++++++++++++++
 3 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 135cdeb89..6dc68da7a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@
 * builder/virtualbox-vm: use config as a non pointer to avoid a panic [GH-8576]
 * core: Fix crash when build.sources is set to an invalid name [GH-8569]
 * core: Fix loading of external plugins. GH-8543]
+* post-processor/docker-tag: Fix regression if no tags were specified. [GH-8593]
 * post-processor/vagrant: correctly handle the diskSize property as a qemu size
     string [GH-8567]
 * provisioner/ansible: Fix password sanitization to account for empty string
diff --git a/post-processor/docker-tag/post-processor.go b/post-processor/docker-tag/post-processor.go
index 4c1f03054..b4af223e6 100644
--- a/post-processor/docker-tag/post-processor.go
+++ b/post-processor/docker-tag/post-processor.go
@@ -68,17 +68,26 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact
 
 	importRepo := p.config.Repository
 	var lastTaggedRepo = importRepo
-	for _, tag := range p.config.Tag {
-		local := importRepo + ":" + tag
-		ui.Message("Tagging image: " + artifact.Id())
-		ui.Message("Repository: " + local)
+	if len(p.config.Tag) > 0 {
+		for _, tag := range p.config.Tag {
+			local := importRepo + ":" + tag
+			ui.Message("Tagging image: " + artifact.Id())
+			ui.Message("Repository: " + local)
 
-		err := driver.TagImage(artifact.Id(), local, p.config.Force)
+			err := driver.TagImage(artifact.Id(), local, p.config.Force)
+			if err != nil {
+				return nil, false, true, err
+			}
+
+			lastTaggedRepo = local
+		}
+	} else {
+		ui.Message("Tagging image: " + artifact.Id())
+		ui.Message("Repository: " + importRepo)
+		err := driver.TagImage(artifact.Id(), importRepo, p.config.Force)
 		if err != nil {
 			return nil, false, true, err
 		}
-
-		lastTaggedRepo = local
 	}
 
 	// Build the artifact
diff --git a/post-processor/docker-tag/post-processor_test.go b/post-processor/docker-tag/post-processor_test.go
index 0c4fa9857..883eeb331 100644
--- a/post-processor/docker-tag/post-processor_test.go
+++ b/post-processor/docker-tag/post-processor_test.go
@@ -127,3 +127,42 @@ func TestPostProcessor_PostProcess_Force(t *testing.T) {
 		t.Fatal("bad force")
 	}
 }
+
+func TestPostProcessor_PostProcess_NoTag(t *testing.T) {
+	driver := &docker.MockDriver{}
+	p := &PostProcessor{Driver: driver}
+	c := testConfig()
+	delete(c, "tag")
+	if err := p.Configure(c); err != nil {
+		t.Fatalf("err %s", err)
+	}
+
+	artifact := &packer.MockArtifact{BuilderIdValue: dockerimport.BuilderId, IdValue: "1234567890abcdef"}
+
+	result, keep, forceOverride, err := p.PostProcess(context.Background(), testUi(), artifact)
+	if _, ok := result.(packer.Artifact); !ok {
+		t.Fatal("should be instance of Artifact")
+	}
+	if !keep {
+		t.Fatal("should keep")
+	}
+	if !forceOverride {
+		t.Fatal("Should force keep no matter what user sets.")
+	}
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	if driver.TagImageCalled != 1 {
+		t.Fatal("should call TagImage")
+	}
+	if driver.TagImageImageId != "1234567890abcdef" {
+		t.Fatal("bad image id")
+	}
+	if driver.TagImageRepo[0] != "foo" {
+		t.Fatal("bad repo")
+	}
+	if driver.TagImageForce {
+		t.Fatal("bad force")
+	}
+}