diff --git a/builder/vmware/common/driver_config.go b/builder/vmware/common/driver_config.go index 7ff7ac7fa..d72320451 100644 --- a/builder/vmware/common/driver_config.go +++ b/builder/vmware/common/driver_config.go @@ -99,10 +99,16 @@ func (c *DriverConfig) Validate(SkipExport bool) error { // now, so that we don't fail for a simple mistake after a long // build ovftool := GetOVFTool() - ovfToolArgs := []string{"--noSSLVerify", "--verifyOnly", fmt.Sprintf("vi://%s:%s@%s", - url.QueryEscape(c.RemoteUser), - url.QueryEscape(c.RemotePassword), - c.RemoteHost)} + + // 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) diff --git a/builder/vmware/common/step_export.go b/builder/vmware/common/step_export.go index 320f00b1a..e60e0ea84 100644 --- a/builder/vmware/common/step_export.go +++ b/builder/vmware/common/step_export.go @@ -38,22 +38,28 @@ func GetOVFTool() string { return ovftool } -func (s *StepExport) generateArgs(c *DriverConfig, displayName string, hidePassword bool) []string { - password := url.QueryEscape(c.RemotePassword) - username := url.QueryEscape(c.RemoteUser) +func (s *StepExport) generateArgs(c *DriverConfig, displayName string, hidePassword bool) ([]string, error) { - if hidePassword { - password = "****" + ovftool_uri := fmt.Sprintf("vi://%s/%s", c.RemoteHost, displayName) + u, err := url.Parse(ovftool_uri) + if err != nil { + return []string{}, err } + + password := c.RemotePassword + if hidePassword { + password = "" + } + u.User = url.UserPassword(c.RemoteUser, password) + args := []string{ "--noSSLVerify=true", "--skipManifestCheck", "-tt=" + s.Format, - - "vi://" + username + ":" + password + "@" + c.RemoteHost + "/" + displayName, + u.String(), s.OutputDir, } - return append(s.OVFToolOptions, args...) + return append(s.OVFToolOptions, args...), nil } func (s *StepExport) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { @@ -91,9 +97,23 @@ func (s *StepExport) Run(ctx context.Context, state multistep.StateBag) multiste if v, ok := state.GetOk("display_name"); ok { displayName = v.(string) } - ui.Message(fmt.Sprintf("Executing: %s %s", ovftool, strings.Join(s.generateArgs(c, displayName, true), " "))) + ui_args, err := s.generateArgs(c, displayName, true) + if err != nil { + err := fmt.Errorf("Couldn't generate ovftool uri: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + ui.Message(fmt.Sprintf("Executing: %s %s", ovftool, strings.Join(ui_args, " "))) var out bytes.Buffer - cmd := exec.Command(ovftool, s.generateArgs(c, displayName, false)...) + args, err := s.generateArgs(c, displayName, false) + if err != nil { + err := fmt.Errorf("Couldn't generate ovftool uri: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + cmd := exec.Command(ovftool, args...) cmd.Stdout = &out if err := cmd.Run(); err != nil { err := fmt.Errorf("Error exporting virtual machine: %s\n%s\n", err, out.String()) diff --git a/post-processor/vsphere/post-processor.go b/post-processor/vsphere/post-processor.go index 2d7c4c084..eede07c0d 100644 --- a/post-processor/vsphere/post-processor.go +++ b/post-processor/vsphere/post-processor.go @@ -115,6 +115,35 @@ func (p *PostProcessor) Configure(raws ...interface{}) error { return nil } +func (p *PostProcessor) generateURI() (*url.URL, error) { + // use net/url lib to encode and escape url elements + ovftool_uri := fmt.Sprintf("vi://%s/%s/host/%s", + p.config.Host, + p.config.Datacenter, + p.config.Cluster) + + if p.config.ResourcePool != "" { + ovftool_uri += "/Resources/" + p.config.ResourcePool + } + + u, err := url.Parse(ovftool_uri) + if err != nil { + return nil, fmt.Errorf("Couldn't generate uri for ovftool: %s", err) + } + u.User = url.UserPassword(p.config.Username, p.config.Password) + + if p.config.ESXiHost != "" { + q := u.Query() + if ipv4Regex.MatchString(p.config.ESXiHost) { + q.Add("ip", p.config.ESXiHost) + } else if hostnameRegex.MatchString(p.config.ESXiHost) { + q.Add("dns", p.config.ESXiHost) + } + u.RawQuery = q.Encode() + } + return u, nil +} + func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact packer.Artifact) (packer.Artifact, bool, bool, error) { source := "" for _, path := range artifact.Files() { @@ -128,27 +157,12 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact return nil, false, false, fmt.Errorf("VMX, OVF or OVA file not found") } - password := escapeWithSpaces(p.config.Password) - ovftool_uri := fmt.Sprintf("vi://%s:%s@%s/%s/host/%s", - escapeWithSpaces(p.config.Username), - password, - p.config.Host, - p.config.Datacenter, - p.config.Cluster) - - if p.config.ResourcePool != "" { - ovftool_uri += "/Resources/" + p.config.ResourcePool + ovftool_uri, err := p.generateURI() + if err != nil { + return nil, false, false, err } - if p.config.ESXiHost != "" { - if ipv4Regex.MatchString(p.config.ESXiHost) { - ovftool_uri += "/?ip=" + p.config.ESXiHost - } else if hostnameRegex.MatchString(p.config.ESXiHost) { - ovftool_uri += "/?dns=" + p.config.ESXiHost - } - } - - args, err := p.BuildArgs(source, ovftool_uri) + args, err := p.BuildArgs(source, ovftool_uri.String()) if err != nil { ui.Message(fmt.Sprintf("Failed: %s\n", err)) } @@ -156,11 +170,7 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact ui.Message(fmt.Sprintf("Uploading %s to vSphere", source)) log.Printf("Starting ovftool with parameters: %s", - strings.Replace( - strings.Join(args, " "), - password, - "", - -1)) + filterLog(strings.Join(args, " "), ovftool_uri)) var errWriter io.Writer var errOut bytes.Buffer @@ -170,20 +180,24 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact cmd.Stderr = errWriter if err := cmd.Run(); err != nil { - err := fmt.Errorf("Error uploading virtual machine: %s\n%s\n", err, p.filterLog(errOut.String())) + err := fmt.Errorf("Error uploading virtual machine: %s\n%s\n", err, filterLog(errOut.String(), ovftool_uri)) return nil, false, false, err } - ui.Message(p.filterLog(errOut.String())) + ui.Message(filterLog(errOut.String(), ovftool_uri)) artifact = NewArtifact(p.config.Datastore, p.config.VMFolder, p.config.VMName, artifact.Files()) return artifact, false, false, nil } -func (p *PostProcessor) filterLog(s string) string { - password := escapeWithSpaces(p.config.Password) - return strings.Replace(s, password, "", -1) +func filterLog(s string, u *url.URL) string { + password, passwordSet := u.User.Password() + if passwordSet && password != "" { + return strings.Replace(s, password, "", -1) + } + + return s } func (p *PostProcessor) BuildArgs(source, ovftool_uri string) ([]string, error) { @@ -222,10 +236,3 @@ func (p *PostProcessor) BuildArgs(source, ovftool_uri string) ([]string, error) return args, nil } - -// Encode everything except for + signs -func escapeWithSpaces(stringToEscape string) string { - escapedString := url.QueryEscape(stringToEscape) - escapedString = strings.Replace(escapedString, "+", `%20`, -1) - return escapedString -} diff --git a/post-processor/vsphere/post-processor_test.go b/post-processor/vsphere/post-processor_test.go index 65b99f9a7..8a14357d3 100644 --- a/post-processor/vsphere/post-processor_test.go +++ b/post-processor/vsphere/post-processor_test.go @@ -7,19 +7,25 @@ import ( "testing" ) +func getTestConfig() Config { + return Config{ + Username: "me", + Password: "notpassword", + Host: "myhost", + Datacenter: "mydc", + Cluster: "mycluster", + VMName: "my vm", + Datastore: "my datastore", + Insecure: true, + DiskMode: "thin", + VMFolder: "my folder", + } +} + func TestArgs(t *testing.T) { var p PostProcessor - p.config.Username = "me" - p.config.Password = "notpassword" - p.config.Host = "myhost" - p.config.Datacenter = "mydc" - p.config.Cluster = "mycluster" - p.config.VMName = "my vm" - p.config.Datastore = "my datastore" - p.config.Insecure = true - p.config.DiskMode = "thin" - p.config.VMFolder = "my folder" + p.config = getTestConfig() source := "something.vmx" ovftool_uri := fmt.Sprintf("vi://%s:%s@%s/%s/host/%s", @@ -41,7 +47,22 @@ func TestArgs(t *testing.T) { t.Logf("ovftool %s", strings.Join(args, " ")) } -func TestEscaping(t *testing.T) { +func TestGenerateURI_Basic(t *testing.T) { + var p PostProcessor + + p.config = getTestConfig() + + uri, err := p.generateURI() + if err != nil { + t.Fatalf("had error: %s", err) + } + expected_uri := "vi://me:notpassword@myhost/mydc/host/mycluster" + if uri.String() != expected_uri { + t.Fatalf("URI did not match. Recieved: %s. Expected: %s", uri, expected_uri) + } +} + +func TestGenerateURI_PasswordEscapes(t *testing.T) { type escapeCases struct { Input string Expected string @@ -50,24 +71,33 @@ func TestEscaping(t *testing.T) { cases := []escapeCases{ {`this has spaces`, `this%20has%20spaces`}, {`exclaimation_!`, `exclaimation_%21`}, - {`hash_#_dollar_$`, `hash_%23_dollar_%24`}, - {`ampersand_&awesome`, `ampersand_%26awesome`}, + {`hash_#_dollar_$`, `hash_%23_dollar_$`}, + {`ampersand_&awesome`, `ampersand_&awesome`}, {`single_quote_'_and_another_'`, `single_quote_%27_and_another_%27`}, {`open_paren_(_close_paren_)`, `open_paren_%28_close_paren_%29`}, - {`asterisk_*_plus_+`, `asterisk_%2A_plus_%2B`}, - {`comma_,slash_/`, `comma_%2Cslash_%2F`}, - {`colon_:semicolon_;`, `colon_%3Asemicolon_%3B`}, - {`equal_=question_?`, `equal_%3Dquestion_%3F`}, + {`asterisk_*_plus_+`, `asterisk_%2A_plus_+`}, + {`comma_,slash_/`, `comma_,slash_%2F`}, + {`colon_:semicolon_;`, `colon_%3Asemicolon_;`}, + {`equal_=question_?`, `equal_=question_%3F`}, {`at_@`, `at_%40`}, {`open_bracket_[closed_bracket]`, `open_bracket_%5Bclosed_bracket%5D`}, - {`user:password with $paces@host/name.foo`, `user%3Apassword%20with%20%24paces%40host%2Fname.foo`}, + {`user:password with $paces@host/name.foo`, `user%3Apassword%20with%20$paces%40host%2Fname.foo`}, } - for _, escapeCase := range cases { - received := escapeWithSpaces(escapeCase.Input) - if escapeCase.Expected != received { - t.Errorf("Error escaping URL; expected %s got %s", escapeCase.Expected, received) + for _, escapeCase := range cases { + var p PostProcessor + + p.config = getTestConfig() + p.config.Password = escapeCase.Input + + uri, err := p.generateURI() + if err != nil { + t.Fatalf("had error: %s", err) + } + expected_uri := fmt.Sprintf("vi://me:%s@myhost/mydc/host/mycluster", escapeCase.Expected) + + if uri.String() != expected_uri { + t.Fatalf("URI did not match. Recieved: %s. Expected: %s", uri, expected_uri) } } - }