Merge pull request #6593 from hashicorp/iso_inplace

allow to use ISO images in-place v.s. copying them

builder/virtualbox/iso/builder.go

@@ -204,6 +204,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			ResultKey:    "iso_path",
 			TargetPath:   b.config.TargetPath,
 			Url:          b.config.ISOUrls,
+			Inplace:      b.config.Inplace,
 		},
 		&vboxcommon.StepOutputDir{
 			Force: b.config.PackerForce,

builder/vmware/iso/builder.go

@@ -291,6 +291,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 			ResultKey:    "iso_path",
 			TargetPath:   b.config.TargetPath,
 			Url:          b.config.ISOUrls,
+			Inplace:      b.config.Inplace,
 		},
 		&vmwcommon.StepOutputDir{
 			Force: b.config.PackerForce,

common/download.go

@@ -38,10 +38,10 @@ type DownloadConfig struct {
 	// DownloaderMap maps a schema to a Download.
 	DownloaderMap map[string]Downloader
 
-	// If true, this will copy even a local file to the target
+	// Inplace indicates wether to copy file
-	// location. If false, then it will "download" the file by just
+	// versus using it inplace.
-	// returning the local path to the file.
+	// Inplace can only be true when referencing local files.
-	CopyFile bool
+	Inplace bool
 
 	// The hashing implementation to use to checksum the downloaded file.
 	Hash hash.Hash
@@ -155,12 +155,12 @@ func (d *DownloadClient) Get() (string, error) {
 	}
 
 	local, ok := d.downloader.(LocalDownloader)
-	if !ok && !d.config.CopyFile {
+	if !ok && d.config.Inplace {
-		d.config.CopyFile = true
+		d.config.Inplace = false
 	}
 
 	// If we're copying the file, then just use the actual downloader
-	if d.config.CopyFile {
+	if d.config.Inplace == false {
 		var f *os.File
 		finalPath = d.config.TargetPath
 
@@ -192,7 +192,7 @@ func (d *DownloadClient) Get() (string, error) {
 		verify, err = d.VerifyChecksum(finalPath)
 		if err == nil && !verify {
 			// Only delete the file if we made a copy or downloaded it
-			if d.config.CopyFile {
+			if d.config.Inplace == false {
 				os.Remove(finalPath)
 			}
 

common/download_test.go

@@ -58,7 +58,6 @@ func TestDownloadClient_basic(t *testing.T) {
 	client := NewDownloadClient(&DownloadConfig{
 		Url:        ts.URL + "/basic.txt",
 		TargetPath: tf.Name(),
-		CopyFile:   true,
 	})
 
 	path, err := client.Get()
@@ -94,7 +93,6 @@ func TestDownloadClient_checksumBad(t *testing.T) {
 		TargetPath: tf.Name(),
 		Hash:       HashForType("md5"),
 		Checksum:   checksum,
-		CopyFile:   true,
 	})
 
 	if _, err := client.Get(); err == nil {
@@ -120,7 +118,6 @@ func TestDownloadClient_checksumGood(t *testing.T) {
 		TargetPath: tf.Name(),
 		Hash:       HashForType("md5"),
 		Checksum:   checksum,
-		CopyFile:   true,
 	})
 
 	path, err := client.Get()
@@ -152,7 +149,6 @@ func TestDownloadClient_checksumNoDownload(t *testing.T) {
 		TargetPath: "./test-fixtures/root/another.txt",
 		Hash:       HashForType("md5"),
 		Checksum:   checksum,
-		CopyFile:   true,
 	})
 	path, err := client.Get()
 	if err != nil {
@@ -210,7 +206,6 @@ func TestDownloadClient_resume(t *testing.T) {
 	client := NewDownloadClient(&DownloadConfig{
 		Url:        ts.URL,
 		TargetPath: tf.Name(),
-		CopyFile:   true,
 	})
 
 	path, err := client.Get()
@@ -270,7 +265,6 @@ func TestDownloadClient_usesDefaultUserAgent(t *testing.T) {
 	config := &DownloadConfig{
 		Url:        server.URL,
 		TargetPath: tf.Name(),
-		CopyFile:   true,
 	}
 
 	client := NewDownloadClient(config)
@@ -303,7 +297,6 @@ func TestDownloadClient_setsUserAgent(t *testing.T) {
 		Url:        server.URL,
 		TargetPath: tf.Name(),
 		UserAgent:  "fancy user agent",
-		CopyFile:   true,
 	}
 
 	client := NewDownloadClient(config)
@@ -402,7 +395,7 @@ func TestDownloadFileUrl(t *testing.T) {
 		// This should be wrong. We want to make sure we don't delete
 		Checksum: []byte("nope"),
 		Hash:     HashForType("sha256"),
-		CopyFile: false,
+		Inplace:  true,
 	}
 
 	client := NewDownloadClient(config)
@@ -418,6 +411,50 @@ func TestDownloadFileUrl(t *testing.T) {
 	}
 }
 
+// TestDownloadFileUrl_inplace verifies that inplace setting is respected.
+func TestDownloadFileUrl_inplace(t *testing.T) {
+	cwd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("Unable to detect working directory: %s", err)
+	}
+	cwd = filepath.ToSlash(cwd)
+
+	// source_path is a file path and source is a network path
+	sourcePath := fmt.Sprintf("%s/test-fixtures/fileurl/%s", cwd, "cake")
+
+	filePrefix := "file://"
+	if runtime.GOOS == "windows" {
+		filePrefix += "/"
+	}
+
+	source := fmt.Sprintf(filePrefix + sourcePath)
+	t.Logf("Trying to download %s", source)
+
+	config := &DownloadConfig{
+		Url: source,
+		// This is correct. We want to make sure we don't delete
+		Checksum: []byte{96, 111, 25, 69, 248, 26, 2, 45, 14, 208, 189, 153, 237, 253, 79, 153, 8, 28, 28, 177, 249, 127, 174, 8, 114, 145, 238, 20, 233, 69, 230, 8},
+		Hash:     HashForType("sha256"),
+		Inplace:  true,
+	}
+
+	client := NewDownloadClient(config)
+
+	// Verify that we fail to match the checksum
+	url, err := client.Get()
+	if err != nil {
+		t.Fatalf("Unexpected error: \"%v\"", err)
+	}
+
+	if sourcePath != url {
+		t.Errorf("Inplace file get should return same path, expected %s, got %s", sourcePath, url)
+	}
+
+	if _, err = os.Stat(sourcePath); err != nil {
+		t.Errorf("Could not stat source file: %s", sourcePath)
+	}
+}
+
 // SimulateFileUriDownload is a simple utility function that converts a uri
 // into a testable file path whilst ignoring a correct checksum match, stripping
 // UNC path info, and then calling stat to ensure the correct file exists.
@@ -432,7 +469,7 @@ func SimulateFileUriDownload(t *testing.T, uri string) (string, error) {
 		// This should be wrong. We want to make sure we don't delete
 		Checksum: []byte("nope"),
 		Hash:     HashForType("sha256"),
-		CopyFile: false,
+		Inplace:  true,
 	}
 
 	// go go go

common/iso_config.go

@@ -24,6 +24,7 @@ type ISOConfig struct {
 	TargetPath      string   `mapstructure:"iso_target_path"`
 	TargetExtension string   `mapstructure:"iso_target_extension"`
 	RawSingleISOUrl string   `mapstructure:"iso_url"`
+	Inplace         bool     `mapstructure:"iso_inplace"`
 }
 
 func (c *ISOConfig) Prepare(ctx *interpolate.Context) (warnings []string, errs []error) {

common/step_download.go

@@ -45,6 +45,11 @@ type StepDownload struct {
 	// extension on the URL is used. Otherwise, this will be forced
 	// on the downloaded file for every URL.
 	Extension string
+
+	// Inplace indicates wether to copy file
+	// versus using it inplace.
+	// Inplace can only be true when referencing local files.
+	Inplace bool
 }
 
 func (s *StepDownload) Run(_ context.Context, state multistep.StateBag) multistep.StepAction {
@@ -61,7 +66,7 @@ func (s *StepDownload) Run(_ context.Context, state multistep.StateBag) multiste
 		}
 	}
 
-	ui.Say(fmt.Sprintf("Downloading or copying %s", s.Description))
+	ui.Say(fmt.Sprintf("Retrieving %s", s.Description))
 
 	// First try to use any already downloaded file
 	// If it fails, proceed to regular download logic
@@ -89,11 +94,12 @@ func (s *StepDownload) Run(_ context.Context, state multistep.StateBag) multiste
 		config := &DownloadConfig{
 			Url:        url,
 			TargetPath: targetPath,
-			CopyFile:   false,
+			Inplace:    s.Inplace,
 			Hash:       HashForType(s.ChecksumType),
 			Checksum:   checksum,
 			UserAgent:  useragent.String(),
 		}
+
 		downloadConfigs[i] = config
 
 		if match, _ := NewDownloadClient(config).VerifyChecksum(config.TargetPath); match {
@@ -105,7 +111,11 @@ func (s *StepDownload) Run(_ context.Context, state multistep.StateBag) multiste
 
 	if finalPath == "" {
 		for i, url := range s.Url {
-			ui.Message(fmt.Sprintf("Downloading or copying: %s", url))
+			if s.Inplace {
+				ui.Message(fmt.Sprintf("Using file in-place: %s", url))
+			} else {
+				ui.Message(fmt.Sprintf("Transferring file from path: %s", url))
+			}
 
 			config := downloadConfigs[i]