diff --git a/builder/vmware/common/driver.go b/builder/vmware/common/driver.go index 8092ec560..167d3bce6 100644 --- a/builder/vmware/common/driver.go +++ b/builder/vmware/common/driver.go @@ -80,6 +80,9 @@ type Driver interface { // Export the vm to ovf or ova format using ovftool Export([]string) error + + // OvfTool + VerifyOvfTool(bool, bool) error } // NewDriver returns a new driver implementation for this operating @@ -628,3 +631,22 @@ func (d *VmwareDriver) Export(args []string) error { return nil } + +func (d *VmwareDriver) VerifyOvfTool(SkipExport, _ bool) error { + if SkipExport { + return nil + } + + log.Printf("Verifying that ovftool exists...") + // Validate that tool exists, but no need to validate credentials. + ovftool := GetOVFTool() + if ovftool != "" { + return nil + } else { + return fmt.Errorf("Couldn't find ovftool in path! Please either " + + "set `skip_export = true` and remove the `format` option " + + "from your template, or make sure ovftool is installed on " + + "your build system. ") + } + +} diff --git a/builder/vmware/common/driver_config.go b/builder/vmware/common/driver_config.go index 35f9d7187..c87ab3c6c 100644 --- a/builder/vmware/common/driver_config.go +++ b/builder/vmware/common/driver_config.go @@ -3,14 +3,8 @@ package common import ( - "bytes" - "context" "fmt" - "net/url" "os" - "os/exec" - "strings" - "time" "github.com/hashicorp/packer/template/interpolate" ) @@ -93,65 +87,5 @@ func (c *DriverConfig) Validate(SkipExport bool) error { "that you set a value for remote_password") } - if c.RemoteType == "" { - // Validate that tool exists, but no need to validate credentials. - ovftool := GetOVFTool() - if ovftool != "" { - return nil - } else { - return fmt.Errorf("Couldn't find ovftool in path! Please either " + - "set `skip_export = true` and remove the `format` option " + - "from your template, or make sure ovftool is installed on " + - "your build system. ") - } - } - - if c.SkipValidateCredentials { - return nil - } - - // check that password is valid by sending a dummy ovftool command - // now, so that we don't fail for a simple mistake after a long - // build - ovftool := GetOVFTool() - - // Generate the uri of the host, with embedded credentials - ovftool_uri := fmt.Sprintf("vi://%s", c.RemoteHost) - u, err := url.Parse(ovftool_uri) - if err != nil { - return fmt.Errorf("Couldn't generate uri for ovftool: %s", err) - } - u.User = url.UserPassword(c.RemoteUser, c.RemotePassword) - - ovfToolArgs := []string{"--noSSLVerify", "--verifyOnly", u.String()} - - var out bytes.Buffer - cmdCtx, cancel := context.WithTimeout(context.Background(), 15*time.Second) - defer cancel() - cmd := exec.CommandContext(cmdCtx, ovftool, ovfToolArgs...) - cmd.Stdout = &out - - // Need to manually close stdin or else the ofvtool call will hang - // forever in a situation where the user has provided an invalid - // password or username - stdin, _ := cmd.StdinPipe() - defer stdin.Close() - - if err := cmd.Run(); err != nil { - outString := out.String() - // The command *should* fail with this error, if it - // authenticates properly. - if !strings.Contains(outString, "Found wrong kind of object") { - err := fmt.Errorf("ovftool validation error: %s; %s", - err, outString) - if strings.Contains(outString, - "Enter login information for source") { - err = fmt.Errorf("The username or password you " + - "provided to ovftool is invalid.") - } - return err - } - } - return nil } diff --git a/builder/vmware/common/driver_esx5.go b/builder/vmware/common/driver_esx5.go index cb4b2352d..dbece3ff9 100644 --- a/builder/vmware/common/driver_esx5.go +++ b/builder/vmware/common/driver_esx5.go @@ -11,7 +11,9 @@ import ( "io" "log" "net" + "net/url" "os" + "os/exec" "path" "path/filepath" "strconv" @@ -263,6 +265,68 @@ func (d *ESX5Driver) Verify() error { return nil } +func (d *ESX5Driver) VerifyOvfTool(SkipExport, skipValidateCredentials bool) error { + err := d.base.VerifyOvfTool(SkipExport, skipValidateCredentials) + if err != nil { + return err + } + + log.Printf("Verifying that ovftool credentials are valid...") + // check that password is valid by sending a dummy ovftool command + // now, so that we don't fail for a simple mistake after a long + // build + ovftool := GetOVFTool() + + if skipValidateCredentials { + return nil + } + + if d.Password == "" { + return fmt.Errorf("exporting the vm from esxi with ovftool requires " + + "that you set a value for remote_password") + } + + // Generate the uri of the host, with embedded credentials + ovftool_uri := fmt.Sprintf("vi://%s", d.Host) + u, err := url.Parse(ovftool_uri) + if err != nil { + return fmt.Errorf("Couldn't generate uri for ovftool: %s", err) + } + u.User = url.UserPassword(d.Username, d.Password) + + ovfToolArgs := []string{"--noSSLVerify", "--verifyOnly", u.String()} + + var out bytes.Buffer + cmdCtx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + cmd := exec.CommandContext(cmdCtx, ovftool, ovfToolArgs...) + cmd.Stdout = &out + + // Need to manually close stdin or else the ofvtool call will hang + // forever in a situation where the user has provided an invalid + // password or username + stdin, _ := cmd.StdinPipe() + defer stdin.Close() + + if err := cmd.Run(); err != nil { + outString := out.String() + // The command *should* fail with this error, if it + // authenticates properly. + if !strings.Contains(outString, "Found wrong kind of object") { + err := fmt.Errorf("ovftool validation error: %s; %s", + err, outString) + if strings.Contains(outString, + "Enter login information for source") { + err = fmt.Errorf("The username or password you " + + "provided to ovftool is invalid.") + } + return err + } + } + + return nil +} + func (d *ESX5Driver) HostIP(multistep.StateBag) (string, error) { conn, err := net.Dial("tcp", fmt.Sprintf("%s:%d", d.Host, d.Port)) if err != nil { diff --git a/builder/vmware/common/driver_mock.go b/builder/vmware/common/driver_mock.go index ba5ecc57f..9ece83841 100644 --- a/builder/vmware/common/driver_mock.go +++ b/builder/vmware/common/driver_mock.go @@ -95,6 +95,8 @@ type DriverMock struct { VerifyCalled bool VerifyErr error + + VerifyOvftoolCalled bool } type NetworkMapperMock struct { @@ -279,3 +281,7 @@ func (d *DriverMock) GetVmwareDriver() VmwareDriver { } return state } + +func (d *DriverMock) VerifyOvfTool(_ bool, _ bool) error { + return nil +} diff --git a/builder/vmware/iso/builder.go b/builder/vmware/iso/builder.go index 353cd0199..624bd8c86 100644 --- a/builder/vmware/iso/builder.go +++ b/builder/vmware/iso/builder.go @@ -35,6 +35,11 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack if err != nil { return nil, fmt.Errorf("Failed creating VMware driver: %s", err) } + // Before we get deep into the build, make sure ovftool is present and + // credentials are valid, if we're going to use ovftool. + if err := driver.VerifyOvfTool(b.config.SkipExport, b.config.SkipValidateCredentials); err != nil { + return nil, err + } // Setup the state bag state := new(multistep.BasicStateBag) diff --git a/builder/vmware/iso/builder_test.go b/builder/vmware/iso/builder_test.go index cd39df5d3..868eb601a 100644 --- a/builder/vmware/iso/builder_test.go +++ b/builder/vmware/iso/builder_test.go @@ -198,6 +198,123 @@ func TestBuilderPrepare_RemoteType(t *testing.T) { } } +func TestBuilderPrepare_Export(t *testing.T) { + type testCase struct { + InputConfigVals map[string]string + ExpectedSkipExportValue bool + ExpectedFormat string + ExpectedErr bool + Reason string + } + testCases := []testCase{ + { + InputConfigVals: map[string]string{ + "remote_type": "", + "format": "", + }, + ExpectedSkipExportValue: true, + ExpectedFormat: "vmx", + ExpectedErr: false, + Reason: "should have defaulted format to vmx.", + }, + { + InputConfigVals: map[string]string{ + "remote_type": "esx5", + "format": "", + "remote_host": "fakehost.com", + "remote_password": "fakepassword", + "remote_username": "fakeuser", + }, + ExpectedSkipExportValue: false, + ExpectedFormat: "ovf", + ExpectedErr: false, + Reason: "should have defaulted format to ovf with remote set to esx5.", + }, + { + InputConfigVals: map[string]string{ + "remote_type": "esx5", + "format": "", + }, + ExpectedSkipExportValue: false, + ExpectedFormat: "ovf", + ExpectedErr: true, + Reason: "should have errored because remote host isn't set for remote build.", + }, + { + InputConfigVals: map[string]string{ + "remote_type": "invalid", + "format": "", + "remote_host": "fakehost.com", + "remote_password": "fakepassword", + "remote_username": "fakeuser", + }, + ExpectedSkipExportValue: false, + ExpectedFormat: "ovf", + ExpectedErr: true, + Reason: "should error with invalid remote type", + }, + { + InputConfigVals: map[string]string{ + "remote_type": "", + "format": "invalid", + }, + ExpectedSkipExportValue: false, + ExpectedFormat: "invalid", + ExpectedErr: true, + Reason: "should error with invalid format", + }, + { + InputConfigVals: map[string]string{ + "remote_type": "", + "format": "ova", + }, + ExpectedSkipExportValue: false, + ExpectedFormat: "ova", + ExpectedErr: false, + Reason: "should set user-given ova format", + }, + { + InputConfigVals: map[string]string{ + "remote_type": "esx5", + "format": "ova", + "remote_host": "fakehost.com", + "remote_password": "fakepassword", + "remote_username": "fakeuser", + }, + ExpectedSkipExportValue: false, + ExpectedFormat: "ova", + ExpectedErr: false, + Reason: "should set user-given ova format", + }, + } + for _, tc := range testCases { + config := testConfig() + for k, v := range tc.InputConfigVals { + config[k] = v + } + config["skip_validate_credentials"] = true + outCfg := &Config{} + warns, errs := (outCfg).Prepare(config) + + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } + + if (errs != nil) != tc.ExpectedErr { + t.Fatalf("received error: \n %s \n but 'expected err' was %t", errs, tc.ExpectedErr) + } + + if outCfg.Format != tc.ExpectedFormat { + t.Fatalf("Expected: %s. Actual: %s. Reason: %s", tc.ExpectedFormat, + outCfg.Format, tc.Reason) + } + if outCfg.SkipExport != tc.ExpectedSkipExportValue { + t.Fatalf("For SkipExport expected %t but recieved %t", + tc.ExpectedSkipExportValue, outCfg.SkipExport) + } + } +} + func TestBuilderPrepare_RemoteExport(t *testing.T) { var b Builder config := testConfig() diff --git a/builder/vmware/iso/config.go b/builder/vmware/iso/config.go index 253b578ff..dcb5d0faf 100644 --- a/builder/vmware/iso/config.go +++ b/builder/vmware/iso/config.go @@ -177,14 +177,14 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) { } if c.Format == "" { - if c.RemoteType != "esx5" { + if c.RemoteType == "" { c.Format = "vmx" } else { c.Format = "ovf" } } - if c.RemoteType != "esx5" && c.Format == "vmx" { + if c.RemoteType == "" && c.Format == "vmx" { // if we're building locally and want a vmx, there's nothing to export. // Set skip export flag here to keep the export step from attempting // an unneded export diff --git a/builder/vmware/vmx/builder.go b/builder/vmware/vmx/builder.go index f9ba881d5..4c303d8bb 100644 --- a/builder/vmware/vmx/builder.go +++ b/builder/vmware/vmx/builder.go @@ -40,6 +40,11 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack if err != nil { return nil, fmt.Errorf("Failed creating VMware driver: %s", err) } + // Before we get deep into the build, make sure ovftool is present and + // credentials are valid, if we're going to use ovftool. + if err := driver.VerifyOvfTool(b.config.SkipExport, b.config.SkipValidateCredentials); err != nil { + return nil, err + } // Set up the state. state := new(multistep.BasicStateBag) diff --git a/builder/vmware/vmx/config.go b/builder/vmware/vmx/config.go index cc7b31531..fc32099e1 100644 --- a/builder/vmware/vmx/config.go +++ b/builder/vmware/vmx/config.go @@ -124,14 +124,14 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) { } if c.Format == "" { - if c.RemoteType != "esx5" { + if c.RemoteType == "" { c.Format = "vmx" } else { c.Format = "ovf" } } - if c.RemoteType != "esx5" && c.Format == "vmx" { + if c.RemoteType == "" && c.Format == "vmx" { // if we're building locally and want a vmx, there's nothing to export. // Set skip export flag here to keep the export step from attempting // an unneded export @@ -143,14 +143,6 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) { errs = packer.MultiErrorAppend(errs, err) } - if c.Format == "" { - if c.RemoteType != "esx5" { - c.Format = "vmx" - } else { - c.Format = "ovf" - } - } - // Warnings var warnings []string if c.ShutdownCommand == "" { diff --git a/builder/vmware/vmx/config_test.go b/builder/vmware/vmx/config_test.go index 4b5c63483..cd386f4a2 100644 --- a/builder/vmware/vmx/config_test.go +++ b/builder/vmware/vmx/config_test.go @@ -58,3 +58,120 @@ func TestNewConfig_sourcePath(t *testing.T) { warns, errs = (&Config{}).Prepare(cfg) testConfigOk(t, warns, errs) } + +func TestNewConfig_exportConfig(t *testing.T) { + type testCase struct { + InputConfigVals map[string]string + ExpectedSkipExportValue bool + ExpectedFormat string + ExpectedErr bool + Reason string + } + testCases := []testCase{ + { + InputConfigVals: map[string]string{ + "remote_type": "", + "format": "", + }, + ExpectedSkipExportValue: true, + ExpectedFormat: "vmx", + ExpectedErr: false, + Reason: "should have defaulted format to vmx.", + }, + { + InputConfigVals: map[string]string{ + "remote_type": "esx5", + "format": "", + "remote_host": "fakehost.com", + "remote_password": "fakepassword", + "remote_username": "fakeuser", + }, + ExpectedSkipExportValue: false, + ExpectedFormat: "ovf", + ExpectedErr: false, + Reason: "should have defaulted format to ovf with remote set to esx5.", + }, + { + InputConfigVals: map[string]string{ + "remote_type": "esx5", + "format": "", + }, + ExpectedSkipExportValue: false, + ExpectedFormat: "ovf", + ExpectedErr: true, + Reason: "should have errored because remote host isn't set for remote build.", + }, + { + InputConfigVals: map[string]string{ + "remote_type": "invalid", + "format": "", + "remote_host": "fakehost.com", + "remote_password": "fakepassword", + "remote_username": "fakeuser", + }, + ExpectedSkipExportValue: false, + ExpectedFormat: "ovf", + ExpectedErr: true, + Reason: "should error with invalid remote type", + }, + { + InputConfigVals: map[string]string{ + "remote_type": "", + "format": "invalid", + }, + ExpectedSkipExportValue: false, + ExpectedFormat: "invalid", + ExpectedErr: true, + Reason: "should error with invalid format", + }, + { + InputConfigVals: map[string]string{ + "remote_type": "", + "format": "ova", + }, + ExpectedSkipExportValue: false, + ExpectedFormat: "ova", + ExpectedErr: false, + Reason: "should set user-given ova format", + }, + { + InputConfigVals: map[string]string{ + "remote_type": "esx5", + "format": "ova", + "remote_host": "fakehost.com", + "remote_password": "fakepassword", + "remote_username": "fakeuser", + }, + ExpectedSkipExportValue: false, + ExpectedFormat: "ova", + ExpectedErr: false, + Reason: "should set user-given ova format", + }, + } + for _, tc := range testCases { + cfg := testConfig(t) + for k, v := range tc.InputConfigVals { + cfg[k] = v + } + cfg["skip_validate_credentials"] = true + outCfg := &Config{} + warns, errs := (outCfg).Prepare(cfg) + + if len(warns) > 0 { + t.Fatalf("bad: %#v", warns) + } + + if (errs != nil) != tc.ExpectedErr { + t.Fatalf("received error: \n %s \n but 'expected err' was %t", errs, tc.ExpectedErr) + } + + if outCfg.Format != tc.ExpectedFormat { + t.Fatalf("Expected: %s. Actual: %s. Reason: %s", tc.ExpectedFormat, + outCfg.Format, tc.Reason) + } + if outCfg.SkipExport != tc.ExpectedSkipExportValue { + t.Fatalf("For SkipExport expected %t but recieved %t", + tc.ExpectedSkipExportValue, outCfg.SkipExport) + } + } +}