From 90bdcf58bd19eabb68839bd13c32118690e4cc8b Mon Sep 17 00:00:00 2001
From: Megan Marsh <megan@hashicorp.com>
Date: Tue, 7 Jan 2020 10:45:24 -0800
Subject: [PATCH] update step_download to return an ActionContinue if the URls
 field is empty. this allows us to simplify the hyperv builder, and is still
 safe because all other builders and uses of step_download already validate
 that the iso url is not empty if that's what they need, most of them inside
 of the IsoConfig prepare function.

---
 builder/hyperv/vmcx/builder.go | 30 +++++++++++-------------------
 common/step_download.go        |  6 ++++++
 common/step_download_test.go   |  5 +++++
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/builder/hyperv/vmcx/builder.go b/builder/hyperv/vmcx/builder.go
index dc1fcea2b..c7c840f40 100644
--- a/builder/hyperv/vmcx/builder.go
+++ b/builder/hyperv/vmcx/builder.go
@@ -236,23 +236,15 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			Force: b.config.PackerForce,
 			Path:  b.config.OutputDir,
 		},
-	}
-
-	if b.config.RawSingleISOUrl != "" || len(b.config.ISOUrls) > 0 {
-		steps = append(steps,
-			&common.StepDownload{
-				Checksum:     b.config.ISOChecksum,
-				ChecksumType: b.config.ISOChecksumType,
-				Description:  "ISO",
-				ResultKey:    "iso_path",
-				Url:          b.config.ISOUrls,
-				Extension:    b.config.TargetExtension,
-				TargetPath:   b.config.TargetPath,
-			},
-		)
-	}
-
-	steps = append(steps,
+		&common.StepDownload{
+			Checksum:     b.config.ISOChecksum,
+			ChecksumType: b.config.ISOChecksumType,
+			Description:  "ISO",
+			ResultKey:    "iso_path",
+			Url:          b.config.ISOUrls,
+			Extension:    b.config.TargetExtension,
+			TargetPath:   b.config.TargetPath,
+		},
 		&common.StepCreateFloppy{
 			Files:       b.config.FloppyFiles,
 			Directories: b.config.FloppyConfig.FloppyDirectories,
@@ -367,9 +359,9 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			OutputDir:  b.config.OutputDir,
 			SkipExport: b.config.SkipExport,
 		},
+	}
 
-		// the clean up actions for each step will be executed reverse order
-	)
+	// the clean up actions for each step will be executed reverse order
 
 	// Run the steps.
 	b.runner = common.NewRunner(steps, b.config.PackerConfig, ui)
diff --git a/common/step_download.go b/common/step_download.go
index a1fa6db89..3b0b84932 100644
--- a/common/step_download.go
+++ b/common/step_download.go
@@ -53,12 +53,18 @@ type StepDownload struct {
 }
 
 func (s *StepDownload) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
+	if len(s.Url) == 0 {
+		log.Printf("No URLs were provided to Step Download. Continuing...")
+		return multistep.ActionContinue
+	}
+
 	defer log.Printf("Leaving retrieve loop for %s", s.Description)
 
 	ui := state.Get("ui").(packer.Ui)
 	ui.Say(fmt.Sprintf("Retrieving %s", s.Description))
 
 	var errs []error
+
 	for _, source := range s.Url {
 		if ctx.Err() != nil {
 			state.Put("error", fmt.Errorf("Download cancelled: %v", errs))
diff --git a/common/step_download_test.go b/common/step_download_test.go
index 6a5a70bd8..cffed33d8 100644
--- a/common/step_download_test.go
+++ b/common/step_download_test.go
@@ -63,6 +63,11 @@ func TestStepDownload_Run(t *testing.T) {
 		want      multistep.StepAction
 		wantFiles []string
 	}{
+		{"Empty URL field passes",
+			fields{Url: []string{}},
+			multistep.ActionContinue,
+			nil,
+		},
 		{"not passing a checksum passes",
 			fields{Url: []string{abs(t, "./test-fixtures/root/another.txt")}},
 			multistep.ActionContinue,