From b85e3e0cc867098c940721a57310391ba12f4cbc Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Tue, 9 Jan 2024 11:59:09 -0500 Subject: [PATCH 1/2] parallels: error when no VM file found in source The post-processor, when consuming a parallels artifact, checks for the presence of a pvm or macvm file from the files of the artifact received, and copies it in the output box for vagrant to use later. However, if the artifact does not contain a compatible file, the post-processor does not error, and instead returns a success, leading to an unusable box being produced. To avoid this, we count the files copied, and if none were, the box will be unusable, so the post-processor errors now. --- post-processor/vagrant/parallels.go | 7 ++ post-processor/vagrant/post-processor_test.go | 66 +++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/post-processor/vagrant/parallels.go b/post-processor/vagrant/parallels.go index b3268866..53029502 100644 --- a/post-processor/vagrant/parallels.go +++ b/post-processor/vagrant/parallels.go @@ -25,6 +25,8 @@ func (p *ParallelsProvider) Process(ui packersdk.Ui, artifact packersdk.Artifact // Create the metadata metadata = map[string]interface{}{"provider": "parallels"} + var copied int + // Copy all of the original contents into the temporary directory for _, path := range artifact.Files() { // If the file isn't critical to the function of the @@ -55,6 +57,11 @@ func (p *ParallelsProvider) Process(ui packersdk.Ui, artifact packersdk.Artifact if err = CopyContents(dstPath, path); err != nil { return } + copied++ + } + + if copied == 0 { + err = fmt.Errorf("No VM file found in source artifact") } return diff --git a/post-processor/vagrant/post-processor_test.go b/post-processor/vagrant/post-processor_test.go index e5a4ee25..abf3e154 100644 --- a/post-processor/vagrant/post-processor_test.go +++ b/post-processor/vagrant/post-processor_test.go @@ -7,7 +7,9 @@ import ( "bytes" "compress/flate" "context" + "fmt" "io/ioutil" + "math/rand" "os" "runtime" "strings" @@ -231,9 +233,48 @@ func TestPostProcessorPostProcess_badId(t *testing.T) { } } +// mockParallelsVMDir creates a fake temp dir for parallels testing +// +// Note: the path to the pvm/macvm dir is returned, the responsibility to remove +// it befalls the caller. +func mockParallelsVMDir() ([]string, error) { + tmpDir := fmt.Sprintf("%s/%d.pvm", os.TempDir(), rand.Uint32()) + err := os.MkdirAll(tmpDir, 0755) + if err != nil { + return nil, err + } + + file1, err := os.CreateTemp(tmpDir, "") + if err != nil { + os.RemoveAll(tmpDir) + return nil, err + } + file1.Close() + + file2, err := os.CreateTemp(tmpDir, "") + if err != nil { + os.RemoveAll(tmpDir) + return nil, err + } + file2.Close() + + return []string{ + tmpDir, + file1.Name(), + file2.Name(), + }, nil +} + func TestPostProcessorPostProcess_vagrantfileUserVariable(t *testing.T) { var p PostProcessor + inputVM, err := mockParallelsVMDir() + if err != nil { + t.Fatalf("failed to create parallels VM directory") + } + dir := inputVM[0] + defer os.RemoveAll(dir) + f, err := ioutil.TempFile("", "packer") if err != nil { t.Fatalf("err: %s", err) @@ -254,6 +295,7 @@ func TestPostProcessorPostProcess_vagrantfileUserVariable(t *testing.T) { a := &packersdk.MockArtifact{ BuilderIdValue: "packer.parallels", + FilesValue: inputVM, } a2, _, _, err := p.PostProcess(context.Background(), testUi(), a) if a2 != nil { @@ -266,6 +308,30 @@ func TestPostProcessorPostProcess_vagrantfileUserVariable(t *testing.T) { } } +func TestPostProcessorPostProcess_parallels_no_file(t *testing.T) { + var p PostProcessor + + c := map[string]interface{}{} + err := p.Configure(c) + if err != nil { + t.Fatalf("err: %s", err) + } + + a := &packersdk.MockArtifact{ + BuilderIdValue: "packer.parallels", + } + a2, _, _, err := p.PostProcess(context.Background(), testUi(), a) + if a2 != nil { + for _, fn := range a2.Files() { + defer os.Remove(fn) + } + } + if err == nil { + t.Fatalf("should have failed without a file to copy, succeeded instead") + } + t.Logf("failed as expected: %s", err) +} + func TestProviderForName(t *testing.T) { if v, ok := providerForName("virtualbox").(*VBoxProvider); !ok { t.Fatalf("bad: %#v", v) From 40abc55ac334ec6f8827f96ae0c5bc428fab890e Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Tue, 16 Jan 2024 16:53:56 -0500 Subject: [PATCH 2/2] Move Parallels test into parallels_test.go --- post-processor/vagrant/parallels_test.go | 106 ++++++++++++++++++ post-processor/vagrant/post-processor_test.go | 101 ----------------- 2 files changed, 106 insertions(+), 101 deletions(-) diff --git a/post-processor/vagrant/parallels_test.go b/post-processor/vagrant/parallels_test.go index c398f560..bd514e24 100644 --- a/post-processor/vagrant/parallels_test.go +++ b/post-processor/vagrant/parallels_test.go @@ -4,9 +4,115 @@ package vagrant import ( + "context" + "fmt" + "io/ioutil" + "math/rand" + "os" "testing" + + packersdk "github.com/hashicorp/packer-plugin-sdk/packer" ) func TestParallelsProvider_impl(t *testing.T) { var _ Provider = new(ParallelsProvider) } + +// mockParallelsVMDir creates a fake temp dir for parallels testing +// +// Note: the path to the pvm/macvm dir is returned, the responsibility to remove +// it befalls the caller. +func mockParallelsVMDir() ([]string, error) { + tmpDir := fmt.Sprintf("%s/%d.pvm", os.TempDir(), rand.Uint32()) + err := os.MkdirAll(tmpDir, 0755) + if err != nil { + return nil, err + } + + file1, err := os.CreateTemp(tmpDir, "") + if err != nil { + os.RemoveAll(tmpDir) + return nil, err + } + file1.Close() + + file2, err := os.CreateTemp(tmpDir, "") + if err != nil { + os.RemoveAll(tmpDir) + return nil, err + } + file2.Close() + + return []string{ + tmpDir, + file1.Name(), + file2.Name(), + }, nil +} + +func TestPostProcessorPostProcessParallels(t *testing.T) { + var p PostProcessor + + inputVM, err := mockParallelsVMDir() + if err != nil { + t.Fatalf("failed to create parallels VM directory") + } + dir := inputVM[0] + defer os.RemoveAll(dir) + + f, err := ioutil.TempFile("", "packer") + if err != nil { + t.Fatalf("err: %s", err) + } + defer os.Remove(f.Name()) + + c := map[string]interface{}{ + "packer_user_variables": map[string]string{ + "foo": f.Name(), + }, + + "vagrantfile_template": "{{user `foo`}}", + } + err = p.Configure(c) + if err != nil { + t.Fatalf("err: %s", err) + } + + a := &packersdk.MockArtifact{ + BuilderIdValue: "packer.parallels", + FilesValue: inputVM, + } + a2, _, _, err := p.PostProcess(context.Background(), testUi(), a) + if a2 != nil { + for _, fn := range a2.Files() { + defer os.Remove(fn) + } + } + if err != nil { + t.Fatalf("err: %s", err) + } +} + +func TestPostProcessorPostProcessParallels_NoFileErrorOnCopy(t *testing.T) { + var p PostProcessor + + c := map[string]interface{}{} + err := p.Configure(c) + if err != nil { + t.Fatalf("err: %s", err) + } + + a := &packersdk.MockArtifact{ + BuilderIdValue: "packer.parallels", + } + a2, _, _, err := p.PostProcess(context.Background(), testUi(), a) + if a2 != nil { + for _, fn := range a2.Files() { + defer os.Remove(fn) + } + } + if err == nil { + t.Fatalf("should have failed without a file to copy, succeeded instead") + } + t.Logf("failed as expected: %s", err) +} diff --git a/post-processor/vagrant/post-processor_test.go b/post-processor/vagrant/post-processor_test.go index abf3e154..030a04f6 100644 --- a/post-processor/vagrant/post-processor_test.go +++ b/post-processor/vagrant/post-processor_test.go @@ -7,9 +7,7 @@ import ( "bytes" "compress/flate" "context" - "fmt" "io/ioutil" - "math/rand" "os" "runtime" "strings" @@ -233,105 +231,6 @@ func TestPostProcessorPostProcess_badId(t *testing.T) { } } -// mockParallelsVMDir creates a fake temp dir for parallels testing -// -// Note: the path to the pvm/macvm dir is returned, the responsibility to remove -// it befalls the caller. -func mockParallelsVMDir() ([]string, error) { - tmpDir := fmt.Sprintf("%s/%d.pvm", os.TempDir(), rand.Uint32()) - err := os.MkdirAll(tmpDir, 0755) - if err != nil { - return nil, err - } - - file1, err := os.CreateTemp(tmpDir, "") - if err != nil { - os.RemoveAll(tmpDir) - return nil, err - } - file1.Close() - - file2, err := os.CreateTemp(tmpDir, "") - if err != nil { - os.RemoveAll(tmpDir) - return nil, err - } - file2.Close() - - return []string{ - tmpDir, - file1.Name(), - file2.Name(), - }, nil -} - -func TestPostProcessorPostProcess_vagrantfileUserVariable(t *testing.T) { - var p PostProcessor - - inputVM, err := mockParallelsVMDir() - if err != nil { - t.Fatalf("failed to create parallels VM directory") - } - dir := inputVM[0] - defer os.RemoveAll(dir) - - f, err := ioutil.TempFile("", "packer") - if err != nil { - t.Fatalf("err: %s", err) - } - defer os.Remove(f.Name()) - - c := map[string]interface{}{ - "packer_user_variables": map[string]string{ - "foo": f.Name(), - }, - - "vagrantfile_template": "{{user `foo`}}", - } - err = p.Configure(c) - if err != nil { - t.Fatalf("err: %s", err) - } - - a := &packersdk.MockArtifact{ - BuilderIdValue: "packer.parallels", - FilesValue: inputVM, - } - a2, _, _, err := p.PostProcess(context.Background(), testUi(), a) - if a2 != nil { - for _, fn := range a2.Files() { - defer os.Remove(fn) - } - } - if err != nil { - t.Fatalf("err: %s", err) - } -} - -func TestPostProcessorPostProcess_parallels_no_file(t *testing.T) { - var p PostProcessor - - c := map[string]interface{}{} - err := p.Configure(c) - if err != nil { - t.Fatalf("err: %s", err) - } - - a := &packersdk.MockArtifact{ - BuilderIdValue: "packer.parallels", - } - a2, _, _, err := p.PostProcess(context.Background(), testUi(), a) - if a2 != nil { - for _, fn := range a2.Files() { - defer os.Remove(fn) - } - } - if err == nil { - t.Fatalf("should have failed without a file to copy, succeeded instead") - } - t.Logf("failed as expected: %s", err) -} - func TestProviderForName(t *testing.T) { if v, ok := providerForName("virtualbox").(*VBoxProvider); !ok { t.Fatalf("bad: %#v", v)