From 57308b01261067f8d3eb7317a866f24d595e5cf0 Mon Sep 17 00:00:00 2001 From: DanHam Date: Fri, 16 Aug 2019 15:42:56 +0100 Subject: [PATCH 01/17] Should return an error with an invalid BuilderId --- .../vagrant-cloud/post-processor_test.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index 5649aca4c..b5a9f5a5d 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -2,10 +2,12 @@ package vagrantcloud import ( "bytes" + "context" "fmt" "net/http" "net/http/httptest" "os" + "strings" "testing" "github.com/hashicorp/packer/packer" @@ -137,6 +139,24 @@ func TestPostProcessor_Configure_Bad(t *testing.T) { } } +func TestPostProcessor_PostProcess_checkArtifactType(t *testing.T) { + artifact := &packer.MockArtifact{ + BuilderIdValue: "invalid.builder", + } + + config := testGoodConfig() + server := newSecureServer("foo", nil) + defer server.Close() + config["vagrant_cloud_url"] = server.URL + var p PostProcessor + + p.Configure(config) + _, _, _, err := p.PostProcess(context.Background(), testUi(), artifact) + if !strings.Contains(err.Error(), "Unknown artifact type") { + t.Fatalf("Should error with message 'Unknown artifact type...' with BuilderId: %s", artifact.BuilderIdValue) + } +} + func testUi() *packer.BasicUi { return &packer.BasicUi{ Reader: new(bytes.Buffer), From 3f4f429c3d4bfe576f35499ae8c3280ffea7d47b Mon Sep 17 00:00:00 2001 From: DanHam Date: Fri, 16 Aug 2019 15:48:28 +0100 Subject: [PATCH 02/17] Should return an error when artifact file does not have .box extension --- .../vagrant-cloud/post-processor_test.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index b5a9f5a5d..b9d8058a1 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -157,6 +157,26 @@ func TestPostProcessor_PostProcess_checkArtifactType(t *testing.T) { } } +func TestPostProcessor_PostProcess_checkArtifactFileIsBox(t *testing.T) { + artifact := &packer.MockArtifact{ + BuilderIdValue: "mitchellh.post-processor.vagrant", // good + FilesValue: []string{"invalid.boxfile"}, // should have .box extension + } + + config := testGoodConfig() + server := newSecureServer("foo", nil) + defer server.Close() + config["vagrant_cloud_url"] = server.URL + var p PostProcessor + + p.Configure(config) + _, _, _, err := p.PostProcess(context.Background(), testUi(), artifact) + if !strings.Contains(err.Error(), "Unknown files in artifact") { + t.Fatalf("Should error with message 'Unknown files in artifact...' with artifact file: %s", + artifact.FilesValue[0]) + } +} + func testUi() *packer.BasicUi { return &packer.BasicUi{ Reader: new(bytes.Buffer), From e8c586175eed5787bcf31513a545d30fd941c74e Mon Sep 17 00:00:00 2001 From: DanHam Date: Tue, 20 Aug 2019 15:44:14 +0100 Subject: [PATCH 03/17] Intention: Allow use of artifice pp with vagrant-cloud pp The Vagrant-Cloud and Vagrant provider (e.g. virtualbox, vmware_desktop etc.) must be determined differently depending on the builder or post-processor supplying the artifact. Adds a wrapper function that: * Uses the original method of determining the provider when the artifact is provided by either the Vagrant builder or Vagrant post-processor * Uses a new (currently empty) function when the artifact is provided via the Artifice post-processor --- .../vagrant-cloud/post-processor.go | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/post-processor/vagrant-cloud/post-processor.go b/post-processor/vagrant-cloud/post-processor.go index 87fe02669..660a2761a 100644 --- a/post-processor/vagrant-cloud/post-processor.go +++ b/post-processor/vagrant-cloud/post-processor.go @@ -19,6 +19,7 @@ import ( var builtins = map[string]string{ "mitchellh.post-processor.vagrant": "vagrant", + "packer.post-processor.artifice": "artifice", "vagrant": "vagrant", } @@ -133,13 +134,14 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact ui.Message("Warning: Using Vagrant Cloud token found in ATLAS_TOKEN. Please make sure it is correct, or set VAGRANT_CLOUD_TOKEN") } - // The name of the provider for vagrant cloud, and vagrant - providerName := providerFromBuilderName(artifact.Id()) + // Determine the name of the provider for Vagrant Cloud, and Vagrant + providerName, err := getProvider(artifact.Id(), artifact.Files()[0], builtins[artifact.BuilderId()]) p.config.ctx.Data = &boxDownloadUrlTemplate{ ArtifactId: artifact.Id(), Provider: providerName, } + boxDownloadUrl, err := interpolate.Render(p.config.BoxDownloadUrl, &p.config.ctx) if err != nil { return nil, false, false, fmt.Errorf("Error processing box_download_url: %s", err) @@ -187,8 +189,21 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact return NewArtifact(providerName, p.config.Tag), true, false, nil } -// converts a packer builder name to the corresponding vagrant -// provider +func getProvider(builderName, boxfile, builderId string) (providerName string, err error) { + if builderId == "artifice" { + // The artifice post processor cannot embed any data in the + // supplied artifact so the provider information must be extracted + // from the box file directly + providerName, err = providerFromVagrantBox(boxfile) + } else { + // For the Vagrant builder and Vagrant post processor the provider can + // be determined from information embedded in the artifact + providerName = providerFromBuilderName(builderName) + } + return providerName, err +} + +// Converts a packer builder name to the corresponding vagrant provider func providerFromBuilderName(name string) string { switch name { case "aws": @@ -207,3 +222,9 @@ func providerFromBuilderName(name string) string { return name } } + +// Returns the Vagrant provider the box is intended for use with by +// reading the metadata file packaged inside the box +func providerFromVagrantBox(boxfile string) (providerName string, err error) { + return "", nil +} From 6b5cf6dcb2475447c230562f434661e6e3487c3c Mon Sep 17 00:00:00 2001 From: DanHam Date: Tue, 20 Aug 2019 16:36:01 +0100 Subject: [PATCH 04/17] Should return an error when the box file is missing --- post-processor/vagrant-cloud/post-processor.go | 6 ++++++ post-processor/vagrant-cloud/post-processor_test.go | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/post-processor/vagrant-cloud/post-processor.go b/post-processor/vagrant-cloud/post-processor.go index 660a2761a..610c402b8 100644 --- a/post-processor/vagrant-cloud/post-processor.go +++ b/post-processor/vagrant-cloud/post-processor.go @@ -226,5 +226,11 @@ func providerFromBuilderName(name string) string { // Returns the Vagrant provider the box is intended for use with by // reading the metadata file packaged inside the box func providerFromVagrantBox(boxfile string) (providerName string, err error) { + f, err := os.Open(boxfile) + if err != nil { + return "", fmt.Errorf("Error attempting to open box file: %s", err) + } + defer f.Close() + return "", nil } diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index b9d8058a1..439f580df 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -197,3 +197,12 @@ func TestProviderFromBuilderName(t *testing.T) { t.Fatal("should convert provider") } } + +func TestProviderFromVagrantBox_missing_box(t *testing.T) { + boxfile := "i_dont_exist.box" + _, err := providerFromVagrantBox(boxfile) + if err == nil { + t.Fatal("Should have error as box file does not exist") + } + t.Logf("%s", err) +} From 35d326de39a61e890efdeb424d57fd4c3b9aa326 Mon Sep 17 00:00:00 2001 From: DanHam Date: Tue, 20 Aug 2019 17:56:24 +0100 Subject: [PATCH 05/17] Add basic workings to function. Return an error if box file is empty --- .../vagrant-cloud/post-processor.go | 34 +++++++++++++++++++ .../vagrant-cloud/post-processor_test.go | 17 ++++++++++ 2 files changed, 51 insertions(+) diff --git a/post-processor/vagrant-cloud/post-processor.go b/post-processor/vagrant-cloud/post-processor.go index 610c402b8..bc63debbc 100644 --- a/post-processor/vagrant-cloud/post-processor.go +++ b/post-processor/vagrant-cloud/post-processor.go @@ -5,8 +5,13 @@ package vagrantcloud import ( + "archive/tar" + "compress/gzip" "context" "fmt" + "io" + "io/ioutil" + "log" "os" "strings" @@ -232,5 +237,34 @@ func providerFromVagrantBox(boxfile string) (providerName string, err error) { } defer f.Close() + // Vagrant boxes are gzipped tar archives + ar, err := gzip.NewReader(f) + if err != nil { + return "", fmt.Errorf("Error unzipping box archive: %s", err) + } + tr := tar.NewReader(ar) + + // Loop through the files in the archive and read the provider + // information from the boxes metadata.json file + for { + hdr, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + return "", fmt.Errorf("%s", err) + } + + if hdr.Name == "metadata.json" { + contents, err := ioutil.ReadAll(tr) + if err != nil { + return "", fmt.Errorf("Error reading contents of metadata.json file from box file: %s", err) + } + // TODO: Parse the json for the provider + log.Printf("Contents: %s", contents) + break + } + } + return "", nil } diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index 439f580df..f42f0c63b 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "io/ioutil" "net/http" "net/http/httptest" "os" @@ -199,6 +200,7 @@ func TestProviderFromBuilderName(t *testing.T) { } func TestProviderFromVagrantBox_missing_box(t *testing.T) { + // Bad: Box does not exist boxfile := "i_dont_exist.box" _, err := providerFromVagrantBox(boxfile) if err == nil { @@ -206,3 +208,18 @@ func TestProviderFromVagrantBox_missing_box(t *testing.T) { } t.Logf("%s", err) } + +func TestProviderFromVagrantBox_empty_box(t *testing.T) { + // Bad: Empty box file + boxfile, err := ioutil.TempFile(os.TempDir(), "test*.box") + if err != nil { + t.Fatalf("Error creating test box file: %s", err) + } + defer os.Remove(boxfile.Name()) + + _, err = providerFromVagrantBox(boxfile.Name()) + if err == nil { + t.Fatal("Should have error as box file is empty") + } + t.Logf("%s", err) +} From 9c6b355088649bc21315042dbf5ed51cae805821 Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 21 Aug 2019 09:44:13 +0100 Subject: [PATCH 06/17] Should return an error if the box is a plain gzip file --- .../vagrant-cloud/post-processor.go | 2 +- .../vagrant-cloud/post-processor_test.go | 35 +++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/post-processor/vagrant-cloud/post-processor.go b/post-processor/vagrant-cloud/post-processor.go index bc63debbc..2d5447124 100644 --- a/post-processor/vagrant-cloud/post-processor.go +++ b/post-processor/vagrant-cloud/post-processor.go @@ -252,7 +252,7 @@ func providerFromVagrantBox(boxfile string) (providerName string, err error) { break } if err != nil { - return "", fmt.Errorf("%s", err) + return "", fmt.Errorf("Error reading header info from box tar archive: %s", err) } if hdr.Name == "metadata.json" { diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index f42f0c63b..d98b4ea7f 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -2,6 +2,7 @@ package vagrantcloud import ( "bytes" + "compress/gzip" "context" "fmt" "io/ioutil" @@ -211,9 +212,9 @@ func TestProviderFromVagrantBox_missing_box(t *testing.T) { func TestProviderFromVagrantBox_empty_box(t *testing.T) { // Bad: Empty box file - boxfile, err := ioutil.TempFile(os.TempDir(), "test*.box") + boxfile, err := newBoxFile() if err != nil { - t.Fatalf("Error creating test box file: %s", err) + t.Fatalf("%s", err) } defer os.Remove(boxfile.Name()) @@ -223,3 +224,33 @@ func TestProviderFromVagrantBox_empty_box(t *testing.T) { } t.Logf("%s", err) } + +func TestProviderFromVagrantBox_gzip_only_box(t *testing.T) { + boxfile, err := newBoxFile() + if err != nil { + t.Fatalf("%s", err) + } + defer os.Remove(boxfile.Name()) + + // Bad: Box is just a plain gzip file + aw := gzip.NewWriter(boxfile) + _, err = aw.Write([]byte("foo content")) + if err != nil { + t.Fatal("Error zipping test box file") + } + aw.Close() // Flush the gzipped contents to file + + _, err = providerFromVagrantBox(boxfile.Name()) + if err == nil { + t.Fatalf("Should have error as box file is a plain gzip file: %s", err) + } + t.Logf("%s", err) +} + +func newBoxFile() (boxfile *os.File, err error) { + boxfile, err = ioutil.TempFile(os.TempDir(), "test*.box") + if err != nil { + return boxfile, fmt.Errorf("Error creating test box file: %s", err) + } + return boxfile, nil +} From 063e4bd3e51a5b73af5ad1c9c59ebc02cad81a23 Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 21 Aug 2019 10:04:32 +0100 Subject: [PATCH 07/17] Should return an error if the box tar archive is empty --- .../vagrant-cloud/post-processor.go | 3 ++ .../vagrant-cloud/post-processor_test.go | 28 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/post-processor/vagrant-cloud/post-processor.go b/post-processor/vagrant-cloud/post-processor.go index 2d5447124..bc04373f6 100644 --- a/post-processor/vagrant-cloud/post-processor.go +++ b/post-processor/vagrant-cloud/post-processor.go @@ -249,6 +249,9 @@ func providerFromVagrantBox(boxfile string) (providerName string, err error) { for { hdr, err := tr.Next() if err == io.EOF { + if providerName == "" { + return "", fmt.Errorf("Error: Provider info was not found in box: %s", boxfile) + } break } if err != nil { diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index d98b4ea7f..dfae6d549 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -1,6 +1,7 @@ package vagrantcloud import ( + "archive/tar" "bytes" "compress/gzip" "context" @@ -247,6 +248,33 @@ func TestProviderFromVagrantBox_gzip_only_box(t *testing.T) { t.Logf("%s", err) } +func TestProviderFromVagrantBox_no_files_in_archive(t *testing.T) { + boxfile, err := newBoxFile() + if err != nil { + t.Fatalf("%s", err) + } + defer os.Remove(boxfile.Name()) + + // Bad: box contains no files in the archive + aw := gzip.NewWriter(boxfile) + tw := tar.NewWriter(aw) + // Flush and close each writer creating an empty gzipped tar archive + err = tw.Close() + if err != nil { + t.Fatalf("Error flushing tar file contents: %s", err) + } + err = aw.Close() + if err != nil { + t.Fatalf("Error flushing gzip file contents: %s", err) + } + + _, err = providerFromVagrantBox(boxfile.Name()) + if err == nil { + t.Fatalf("Should have error as box file has no contents") + } + t.Logf("%s", err) +} + func newBoxFile() (boxfile *os.File, err error) { boxfile, err = ioutil.TempFile(os.TempDir(), "test*.box") if err != nil { From 0bf0e7c0782d672b01a3ca48bb69c24fe11fcacd Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 21 Aug 2019 10:47:08 +0100 Subject: [PATCH 08/17] Should return an error when the metadata file is not in the box tar archive Split out box creation into new helper function --- .../vagrant-cloud/post-processor_test.go | 82 +++++++++++++++---- 1 file changed, 67 insertions(+), 15 deletions(-) diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index dfae6d549..837b07de6 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -16,6 +16,10 @@ import ( "github.com/hashicorp/packer/packer" ) +type tarFiles []struct { + Name, Body string +} + func testGoodConfig() map[string]interface{} { return map[string]interface{}{ "access_token": "foo", @@ -249,25 +253,13 @@ func TestProviderFromVagrantBox_gzip_only_box(t *testing.T) { } func TestProviderFromVagrantBox_no_files_in_archive(t *testing.T) { - boxfile, err := newBoxFile() + // Bad: Box contains no files + boxfile, err := createBox(tarFiles{}) if err != nil { - t.Fatalf("%s", err) + t.Fatalf("Error creating test box: %s", err) } defer os.Remove(boxfile.Name()) - // Bad: box contains no files in the archive - aw := gzip.NewWriter(boxfile) - tw := tar.NewWriter(aw) - // Flush and close each writer creating an empty gzipped tar archive - err = tw.Close() - if err != nil { - t.Fatalf("Error flushing tar file contents: %s", err) - } - err = aw.Close() - if err != nil { - t.Fatalf("Error flushing gzip file contents: %s", err) - } - _, err = providerFromVagrantBox(boxfile.Name()) if err == nil { t.Fatalf("Should have error as box file has no contents") @@ -275,6 +267,25 @@ func TestProviderFromVagrantBox_no_files_in_archive(t *testing.T) { t.Logf("%s", err) } +func TestProviderFromVagrantBox_no_metadata(t *testing.T) { + // Bad: Box contains no metadata/metadata.json file + files := tarFiles{ + {"foo.txt", "This is a foo file"}, + {"bar.txt", "This is a bar file"}, + } + boxfile, err := createBox(files) + if err != nil { + t.Fatalf("Error creating test box: %s", err) + } + defer os.Remove(boxfile.Name()) + + _, err = providerFromVagrantBox(boxfile.Name()) + if err == nil { + t.Fatalf("Should have error as box file does not include metadata.json file") + } + t.Logf("%s", err) +} + func newBoxFile() (boxfile *os.File, err error) { boxfile, err = ioutil.TempFile(os.TempDir(), "test*.box") if err != nil { @@ -282,3 +293,44 @@ func newBoxFile() (boxfile *os.File, err error) { } return boxfile, nil } + +func createBox(files tarFiles) (boxfile *os.File, err error) { + boxfile, err = newBoxFile() + if err != nil { + return boxfile, err + } + + // Box files are gzipped tar archives + aw := gzip.NewWriter(boxfile) + tw := tar.NewWriter(aw) + + // Add each file to the box + for _, file := range files { + // Create and write the tar file header + hdr := &tar.Header{ + Name: file.Name, + Mode: 0644, + Size: int64(len(file.Body)), + } + err = tw.WriteHeader(hdr) + if err != nil { + return boxfile, fmt.Errorf("Error writing box tar file header: %s", err) + } + // Write the file contents + _, err = tw.Write([]byte(file.Body)) + if err != nil { + return boxfile, fmt.Errorf("Error writing box tar file contents: %s", err) + } + } + // Flush and close each writer + err = tw.Close() + if err != nil { + return boxfile, fmt.Errorf("Error flushing tar file contents: %s", err) + } + err = aw.Close() + if err != nil { + return boxfile, fmt.Errorf("Error flushing gzip file contents: %s", err) + } + + return boxfile, nil +} From a9e22a6bb28529334c5e58f3a78d87f194ef02ea Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 21 Aug 2019 12:32:42 +0100 Subject: [PATCH 09/17] Should return the provider by parsing the json in the box metadata file --- .../vagrant-cloud/post-processor.go | 19 +++++++++++------ .../vagrant-cloud/post-processor_test.go | 21 +++++++++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/post-processor/vagrant-cloud/post-processor.go b/post-processor/vagrant-cloud/post-processor.go index bc04373f6..90e885ffc 100644 --- a/post-processor/vagrant-cloud/post-processor.go +++ b/post-processor/vagrant-cloud/post-processor.go @@ -8,10 +8,10 @@ import ( "archive/tar" "compress/gzip" "context" + "encoding/json" "fmt" "io" "io/ioutil" - "log" "os" "strings" @@ -244,12 +244,18 @@ func providerFromVagrantBox(boxfile string) (providerName string, err error) { } tr := tar.NewReader(ar) + // The metadata.json file in the tar archive contains a 'provider' key + type metadata struct { + ProviderName string `json:"provider"` + } + md := metadata{} + // Loop through the files in the archive and read the provider // information from the boxes metadata.json file for { hdr, err := tr.Next() if err == io.EOF { - if providerName == "" { + if md.ProviderName == "" { return "", fmt.Errorf("Error: Provider info was not found in box: %s", boxfile) } break @@ -263,11 +269,12 @@ func providerFromVagrantBox(boxfile string) (providerName string, err error) { if err != nil { return "", fmt.Errorf("Error reading contents of metadata.json file from box file: %s", err) } - // TODO: Parse the json for the provider - log.Printf("Contents: %s", contents) + err = json.Unmarshal(contents, &md) + if err != nil { + return "", fmt.Errorf("Error parsing metadata.json file: %s", err) + } break } } - - return "", nil + return md.ProviderName, nil } diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index 837b07de6..73a8a47e7 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -14,6 +14,7 @@ import ( "testing" "github.com/hashicorp/packer/packer" + "github.com/stretchr/testify/assert" ) type tarFiles []struct { @@ -286,6 +287,26 @@ func TestProviderFromVagrantBox_no_metadata(t *testing.T) { t.Logf("%s", err) } +func TestProviderFromVagrantBox_metadata_ok(t *testing.T) { + // Good: The box contains the metadata.json file with the required + // 'provider' key/value + expectedProvider := "virtualbox" + files := tarFiles{ + {"foo.txt", "This is a foo file"}, + {"bar.txt", "This is a bar file"}, + {"metadata.json", `{"provider":"` + expectedProvider + `"}`}, + } + boxfile, err := createBox(files) + if err != nil { + t.Fatalf("Error creating test box: %s", err) + } + defer os.Remove(boxfile.Name()) + + provider, err := providerFromVagrantBox(boxfile.Name()) + assert.Equal(t, expectedProvider, provider, "Error: Expected provider: '%s'. Got '%s'", expectedProvider, provider) + t.Logf("Expected provider '%s'. Got provider '%s'", expectedProvider, provider) +} + func newBoxFile() (boxfile *os.File, err error) { boxfile, err = ioutil.TempFile(os.TempDir(), "test*.box") if err != nil { From d1327fe422debf032a9fe2e9669859428baa5f90 Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 21 Aug 2019 12:55:28 +0100 Subject: [PATCH 10/17] Should return an error if the metadata file is empty --- .../vagrant-cloud/post-processor_test.go | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index 73a8a47e7..97150dad2 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -287,9 +287,28 @@ func TestProviderFromVagrantBox_no_metadata(t *testing.T) { t.Logf("%s", err) } +func TestProviderFromVagrantBox_metadata_empty(t *testing.T) { + // Bad: Create a box with an empty metadata.json file + files := tarFiles{ + {"foo.txt", "This is a foo file"}, + {"bar.txt", "This is a bar file"}, + {"metadata.json", ""}, + } + boxfile, err := createBox(files) + if err != nil { + t.Fatalf("Error creating test box: %s", err) + } + defer os.Remove(boxfile.Name()) + + _, err = providerFromVagrantBox(boxfile.Name()) + if err == nil { + t.Fatalf("Should have error as box files metadata.json file is empty") + } + t.Logf("%s", err) +} + func TestProviderFromVagrantBox_metadata_ok(t *testing.T) { - // Good: The box contains the metadata.json file with the required - // 'provider' key/value + // Good: The boxes metadata.json file has the 'provider' key/value pair expectedProvider := "virtualbox" files := tarFiles{ {"foo.txt", "This is a foo file"}, From 57137c6e3336a3ec9c5ff65b152dfa6d46e6b5ec Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 21 Aug 2019 13:02:42 +0100 Subject: [PATCH 11/17] Should return an error if the value of the provider key is empty --- .../vagrant-cloud/post-processor.go | 3 +++ .../vagrant-cloud/post-processor_test.go | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/post-processor/vagrant-cloud/post-processor.go b/post-processor/vagrant-cloud/post-processor.go index 90e885ffc..e5b6afe27 100644 --- a/post-processor/vagrant-cloud/post-processor.go +++ b/post-processor/vagrant-cloud/post-processor.go @@ -273,6 +273,9 @@ func providerFromVagrantBox(boxfile string) (providerName string, err error) { if err != nil { return "", fmt.Errorf("Error parsing metadata.json file: %s", err) } + if md.ProviderName == "" { + return "", fmt.Errorf("Error: Could not determine Vagrant provider from box metadata.json file") + } break } } diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index 97150dad2..c63dd910e 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -307,6 +307,26 @@ func TestProviderFromVagrantBox_metadata_empty(t *testing.T) { t.Logf("%s", err) } +func TestProviderFromVagrantBox_metadata_provider_value_empty(t *testing.T) { + // Bad: The boxes metadata.json file 'provider' key has an empty value + files := tarFiles{ + {"foo.txt", "This is a foo file"}, + {"bar.txt", "This is a bar file"}, + {"metadata.json", `{"provider":""}`}, + } + boxfile, err := createBox(files) + if err != nil { + t.Fatalf("Error creating test box: %s", err) + } + defer os.Remove(boxfile.Name()) + + _, err = providerFromVagrantBox(boxfile.Name()) + if err == nil { + t.Fatalf("Should have error as boxes metadata.json file 'provider' key is empty") + } + t.Logf("%s", err) +} + func TestProviderFromVagrantBox_metadata_ok(t *testing.T) { // Good: The boxes metadata.json file has the 'provider' key/value pair expectedProvider := "virtualbox" From a7603f63c7792b66db719bf638bf15214fc3b1fc Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 21 Aug 2019 13:10:43 +0100 Subject: [PATCH 12/17] Should return an error if the metadata file has badly formatted JSON --- .../vagrant-cloud/post-processor_test.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index c63dd910e..b58bee261 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -307,6 +307,26 @@ func TestProviderFromVagrantBox_metadata_empty(t *testing.T) { t.Logf("%s", err) } +func TestProviderFromVagrantBox_metadata_bad_json(t *testing.T) { + // Bad: Create a box with bad JSON in the metadata.json file + files := tarFiles{ + {"foo.txt", "This is a foo file"}, + {"bar.txt", "This is a bar file"}, + {"metadata.json", "{provider: badjson}"}, + } + boxfile, err := createBox(files) + if err != nil { + t.Fatalf("Error creating test box: %s", err) + } + defer os.Remove(boxfile.Name()) + + _, err = providerFromVagrantBox(boxfile.Name()) + if err == nil { + t.Fatalf("Should have error as box files metadata.json file is empty") + } + t.Logf("%s", err) +} + func TestProviderFromVagrantBox_metadata_provider_value_empty(t *testing.T) { // Bad: The boxes metadata.json file 'provider' key has an empty value files := tarFiles{ From aee400836fbd9ca2510a3e739b06b8e6879cde0a Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 21 Aug 2019 13:18:56 +0100 Subject: [PATCH 13/17] Should return an error if the provider kv pair is not in the metadata file --- .../vagrant-cloud/post-processor_test.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index b58bee261..c698db017 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -327,6 +327,26 @@ func TestProviderFromVagrantBox_metadata_bad_json(t *testing.T) { t.Logf("%s", err) } +func TestProviderFromVagrantBox_metadata_no_provider_key(t *testing.T) { + // Bad: Create a box with no 'provider' key in the metadata.json file + files := tarFiles{ + {"foo.txt", "This is a foo file"}, + {"bar.txt", "This is a bar file"}, + {"metadata.json", `{"cows":"moo"}`}, + } + boxfile, err := createBox(files) + if err != nil { + t.Fatalf("Error creating test box: %s", err) + } + defer os.Remove(boxfile.Name()) + + _, err = providerFromVagrantBox(boxfile.Name()) + if err == nil { + t.Fatalf("Should have error as box files metadata.json file is empty") + } + t.Logf("%s", err) +} + func TestProviderFromVagrantBox_metadata_provider_value_empty(t *testing.T) { // Bad: The boxes metadata.json file 'provider' key has an empty value files := tarFiles{ From e8336039d9e393b1a4d4df231c8598af20f3d6dc Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 21 Aug 2019 13:31:29 +0100 Subject: [PATCH 14/17] Should return provider correctly with good box and artifice pp --- .../vagrant-cloud/post-processor_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index c698db017..6a0d9c4cb 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -386,6 +386,24 @@ func TestProviderFromVagrantBox_metadata_ok(t *testing.T) { t.Logf("Expected provider '%s'. Got provider '%s'", expectedProvider, provider) } +func TestGetProvider_artifice(t *testing.T) { + expectedProvider := "virtualbox" + files := tarFiles{ + {"foo.txt", "This is a foo file"}, + {"bar.txt", "This is a bar file"}, + {"metadata.json", `{"provider":"` + expectedProvider + `"}`}, + } + boxfile, err := createBox(files) + if err != nil { + t.Fatalf("Error creating test box: %s", err) + } + defer os.Remove(boxfile.Name()) + + provider, err := getProvider("", boxfile.Name(), "artifice") + assert.Equal(t, expectedProvider, provider, "Error: Expected provider: '%s'. Got '%s'", expectedProvider, provider) + t.Logf("Expected provider '%s'. Got provider '%s'", expectedProvider, provider) +} + func newBoxFile() (boxfile *os.File, err error) { boxfile, err = ioutil.TempFile(os.TempDir(), "test*.box") if err != nil { From e9ab2203ba586177efd934b87529ad0ffc8df47a Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 21 Aug 2019 13:36:50 +0100 Subject: [PATCH 15/17] Should return provider correctly with artifacts from other builders or pp's --- post-processor/vagrant-cloud/post-processor_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/post-processor/vagrant-cloud/post-processor_test.go b/post-processor/vagrant-cloud/post-processor_test.go index 6a0d9c4cb..58e65efaf 100644 --- a/post-processor/vagrant-cloud/post-processor_test.go +++ b/post-processor/vagrant-cloud/post-processor_test.go @@ -404,6 +404,14 @@ func TestGetProvider_artifice(t *testing.T) { t.Logf("Expected provider '%s'. Got provider '%s'", expectedProvider, provider) } +func TestGetProvider_other(t *testing.T) { + expectedProvider := "virtualbox" + + provider, _ := getProvider(expectedProvider, "foo.box", "other") + assert.Equal(t, expectedProvider, provider, "Error: Expected provider: '%s'. Got '%s'", expectedProvider, provider) + t.Logf("Expected provider '%s'. Got provider '%s'", expectedProvider, provider) +} + func newBoxFile() (boxfile *os.File, err error) { boxfile, err = ioutil.TempFile(os.TempDir(), "test*.box") if err != nil { From 7770ade74c406a6a05cfb99c66c4fa0cf0500154 Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 21 Aug 2019 14:07:55 +0100 Subject: [PATCH 16/17] Clearer error message for artifice users. OCD nits and logging --- post-processor/vagrant-cloud/post-processor.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/post-processor/vagrant-cloud/post-processor.go b/post-processor/vagrant-cloud/post-processor.go index e5b6afe27..49c342e05 100644 --- a/post-processor/vagrant-cloud/post-processor.go +++ b/post-processor/vagrant-cloud/post-processor.go @@ -12,6 +12,7 @@ import ( "fmt" "io" "io/ioutil" + "log" "os" "strings" @@ -95,7 +96,7 @@ func (p *PostProcessor) Configure(raws ...interface{}) error { // Accumulate any errors errs := new(packer.MultiError) - // required configuration + // Required configuration templates := map[string]*string{ "box_tag": &p.config.Tag, "version": &p.config.Version, @@ -109,7 +110,7 @@ func (p *PostProcessor) Configure(raws ...interface{}) error { } } - // create the HTTP client + // Create the HTTP client p.client, err = VagrantCloudClient{}.New(p.config.VagrantCloudUrl, p.config.AccessToken, p.insecureSkipTLSVerify) if err != nil { errs = packer.MultiErrorAppend( @@ -132,7 +133,7 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact // We assume that there is only one .box file to upload if !strings.HasSuffix(artifact.Files()[0], ".box") { return nil, false, false, fmt.Errorf( - "Unknown files in artifact, vagrant box is required: %s", artifact.Files()) + "Unknown files in artifact, Vagrant box with .box suffix is required as first artifact file: %s", artifact.Files()) } if p.warnAtlasToken { @@ -231,6 +232,8 @@ func providerFromBuilderName(name string) string { // Returns the Vagrant provider the box is intended for use with by // reading the metadata file packaged inside the box func providerFromVagrantBox(boxfile string) (providerName string, err error) { + log.Println("Attempting to determine provider from metadata in box file. This may take some time...") + f, err := os.Open(boxfile) if err != nil { return "", fmt.Errorf("Error attempting to open box file: %s", err) From 53c59fc486a14b1288314e4b987ab14158e32505 Mon Sep 17 00:00:00 2001 From: DanHam Date: Wed, 21 Aug 2019 16:53:06 +0100 Subject: [PATCH 17/17] Document use of Artifice and Vagrant Cloud pp's. Other changes --- .../post-processors/vagrant-cloud.html.md | 87 ++++++++++++++++--- 1 file changed, 76 insertions(+), 11 deletions(-) diff --git a/website/source/docs/post-processors/vagrant-cloud.html.md b/website/source/docs/post-processors/vagrant-cloud.html.md index d0b490fc1..c3e30fd0e 100644 --- a/website/source/docs/post-processors/vagrant-cloud.html.md +++ b/website/source/docs/post-processors/vagrant-cloud.html.md @@ -1,9 +1,7 @@ --- description: | - The Packer Vagrant Cloud post-processor receives a Vagrant box from the - `vagrant` post-processor or vagrant builder and pushes it to Vagrant Cloud. - Vagrant Cloud hosts and serves boxes to Vagrant, allowing you to version and - distribute boxes to an organization in a simple way. + The Vagrant Cloud post-processor enables the upload of Vagrant boxes to + Vagrant Cloud. layout: docs page_title: 'Vagrant Cloud - Post-Processors' sidebar_current: 'docs-post-processors-vagrant-cloud' @@ -13,12 +11,16 @@ sidebar_current: 'docs-post-processors-vagrant-cloud' Type: `vagrant-cloud` -The Packer Vagrant Cloud post-processor receives a Vagrant box from the -`vagrant` post-processor or vagrant builder and pushes it to Vagrant Cloud. [Vagrant Cloud](https://app.vagrantup.com/boxes/search) hosts and serves boxes to Vagrant, allowing you to version and distribute boxes to an organization in a simple way. +The Vagrant Cloud post-processor enables the upload of Vagrant boxes to Vagrant +Cloud. Currently, the Vagrant Cloud post-processor will accept and upload boxes +supplied to it from the [Vagrant](/docs/post-processors/vagrant.html) or +[Artifice](/docs/post-processors/artifice.html) post-processors and the +[Vagrant](/docs/builders/vagrant.html) builder. + You'll need to be familiar with Vagrant Cloud, have an upgraded account to enable box hosting, and be distributing your box via the [shorthand name](https://docs.vagrantup.com/v2/cli/box.html) configuration. @@ -94,12 +96,18 @@ on Vagrant Cloud, as well as authentication and version information. - `box_download_url` (string) - Optional URL for a self-hosted box. If this is set the box will not be uploaded to the Vagrant Cloud. -## Use with Vagrant Post-Processor +## Use with the Vagrant Post-Processor -You'll need to use the Vagrant post-processor before using this post-processor. -An example configuration is below. Note the use of a doubly-nested array, which -ensures that the Vagrant Cloud post-processor is run after the Vagrant -post-processor. +An example configuration is shown below. Note the use of the nested array that +wraps both the Vagrant and Vagrant Cloud post-processors within the +post-processor section. Chaining the post-processors together in this way tells +Packer that the artifact produced by the Vagrant post-processor should be passed +directly to the Vagrant Cloud Post-Processor. It also sets the order in which +the post-processors should run. + +Failure to chain the post-processors together in this way will result in the +wrong artifact being supplied to the Vagrant Cloud post-processor. This will +likely cause the Vagrant Cloud post-processor to error and fail. ``` json { @@ -108,6 +116,10 @@ post-processor. "version": "1.0.{{timestamp}}" }, "post-processors": [ + { + "type": "shell-local", + "inline": ["echo Doing stuff..."] + }, [ { "type": "vagrant", @@ -125,3 +137,56 @@ post-processor. ] } ``` + +## Use with the Artifice Post-Processor + +An example configuration is shown below. Note the use of the nested array that +wraps both the Artifice and Vagrant Cloud post-processors within the +post-processor section. Chaining the post-processors together in this way tells +Packer that the artifact produced by the Artifice post-processor should be +passed directly to the Vagrant Cloud Post-Processor. It also sets the order in +which the post-processors should run. + +Failure to chain the post-processors together in this way will result in the +wrong artifact being supplied to the Vagrant Cloud post-processor. This will +likely cause the Vagrant Cloud post-processor to error and fail. + +Note that the Vagrant box specified in the Artifice post-processor `files` array +must end in the `.box` extension. It must also be the first file in the array. +Additional files bundled by the Artifice post-processor will be ignored. + +```json +{ + "variables": { + "cloud_token": "{{ env `VAGRANT_CLOUD_TOKEN` }}", + }, + + "builders": [ + { + "type": "null", + "communicator": "none" + } + ], + + "post-processors": [ + { + "type": "shell-local", + "inline": ["echo Doing stuff..."] + }, + [ + { + "type": "artifice", + "files": [ + "./path/to/my.box" + ] + }, + { + "type": "vagrant-cloud", + "box_tag": "myorganisation/mybox", + "access_token": "{{user `cloud_token`}}", + "version": "0.1.0", + } + ] + ] +} +```