From 4a8ef78d333dc07d9f62fc4480b556a7d10089fd Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 10 Jul 2015 12:56:27 +0200 Subject: [PATCH] Fixes #2676 by prefixing all Windows commands By prefixing them with `cmd /c` it will work with both `winner` and `ssh` connection types. This PR also reverts some bad stringer changes made in PR #2673 --- .../provisioners/chef/resource_provisioner.go | 9 +++++--- .../chef/resource_provisioner_test.go | 23 +++++++++++++++---- .../provisioners/chef/windows_provisioner.go | 4 ++-- .../chef/windows_provisioner_test.go | 8 +++---- command/counthookaction_string.go | 2 +- helper/schema/valuetype_string.go | 2 +- terraform/graphnodeconfigtype_string.go | 2 +- terraform/instancetype_string.go | 2 +- terraform/walkoperation_string.go | 2 +- 9 files changed, 36 insertions(+), 18 deletions(-) diff --git a/builtin/provisioners/chef/resource_provisioner.go b/builtin/provisioners/chef/resource_provisioner.go index 13a0e03b5..8ff5a255b 100644 --- a/builtin/provisioners/chef/resource_provisioner.go +++ b/builtin/provisioners/chef/resource_provisioner.go @@ -27,9 +27,11 @@ const ( defaultEnv = "_default" firstBoot = "first-boot.json" logfileDir = "logfiles" + linuxChefCmd = "chef-client" linuxConfDir = "/etc/chef" secretKey = "encrypted_data_bag_secret" validationKey = "validation.pem" + windowsChefCmd = "cmd /c chef-client" windowsConfDir = "C:/chef" ) @@ -112,12 +114,12 @@ func (r *ResourceProvisioner) Apply( case "linux": p.installChefClient = p.linuxInstallChefClient p.createConfigFiles = p.linuxCreateConfigFiles - p.runChefClient = p.runChefClientFunc(linuxConfDir) + p.runChefClient = p.runChefClientFunc(linuxChefCmd, linuxConfDir) p.useSudo = !p.PreventSudo && s.Ephemeral.ConnInfo["user"] != "root" case "windows": p.installChefClient = p.windowsInstallChefClient p.createConfigFiles = p.windowsCreateConfigFiles - p.runChefClient = p.runChefClientFunc(windowsConfDir) + p.runChefClient = p.runChefClientFunc(windowsChefCmd, windowsConfDir) p.useSudo = false default: return fmt.Errorf("Unsupported os type: %s", p.OSType) @@ -289,10 +291,11 @@ func retryFunc(timeout time.Duration, f func() error) error { } func (p *Provisioner) runChefClientFunc( + chefCmd string, confDir string) func(terraform.UIOutput, communicator.Communicator) error { return func(o terraform.UIOutput, comm communicator.Communicator) error { fb := path.Join(confDir, firstBoot) - cmd := fmt.Sprintf("chef-client -j %q -E %q", fb, p.Environment) + cmd := fmt.Sprintf("%s -j %q -E %q", chefCmd, fb, p.Environment) if p.LogToFile { if err := os.MkdirAll(logfileDir, 0755); err != nil { diff --git a/builtin/provisioners/chef/resource_provisioner_test.go b/builtin/provisioners/chef/resource_provisioner_test.go index 45fc8a211..78c44c7ea 100644 --- a/builtin/provisioners/chef/resource_provisioner_test.go +++ b/builtin/provisioners/chef/resource_provisioner_test.go @@ -1,6 +1,8 @@ package chef import ( + "fmt" + "path" "testing" "github.com/hashicorp/terraform/communicator" @@ -58,6 +60,7 @@ func testConfig(t *testing.T, c map[string]interface{}) *terraform.ResourceConfi func TestResourceProvider_runChefClient(t *testing.T) { cases := map[string]struct { Config *terraform.ResourceConfig + ChefCmd string ConfDir string Commands map[string]bool }{ @@ -70,10 +73,14 @@ func TestResourceProvider_runChefClient(t *testing.T) { "validation_key_path": "test-fixtures/validator.pem", }), + ChefCmd: linuxChefCmd, + ConfDir: linuxConfDir, Commands: map[string]bool{ - `sudo chef-client -j "/etc/chef/first-boot.json" -E "_default"`: true, + fmt.Sprintf(`sudo %s -j %q -E "_default"`, + linuxChefCmd, + path.Join(linuxConfDir, "first-boot.json")): true, }, }, @@ -87,10 +94,14 @@ func TestResourceProvider_runChefClient(t *testing.T) { "validation_key_path": "test-fixtures/validator.pem", }), + ChefCmd: linuxChefCmd, + ConfDir: linuxConfDir, Commands: map[string]bool{ - `chef-client -j "/etc/chef/first-boot.json" -E "_default"`: true, + fmt.Sprintf(`%s -j %q -E "_default"`, + linuxChefCmd, + path.Join(linuxConfDir, "first-boot.json")): true, }, }, @@ -105,10 +116,14 @@ func TestResourceProvider_runChefClient(t *testing.T) { "validation_key_path": "test-fixtures/validator.pem", }), + ChefCmd: windowsChefCmd, + ConfDir: windowsConfDir, Commands: map[string]bool{ - `chef-client -j "C:/chef/first-boot.json" -E "production"`: true, + fmt.Sprintf(`%s -j %q -E "production"`, + windowsChefCmd, + path.Join(windowsConfDir, "first-boot.json")): true, }, }, } @@ -125,7 +140,7 @@ func TestResourceProvider_runChefClient(t *testing.T) { t.Fatalf("Error: %v", err) } - p.runChefClient = p.runChefClientFunc(tc.ConfDir) + p.runChefClient = p.runChefClientFunc(tc.ChefCmd, tc.ConfDir) p.useSudo = !p.PreventSudo err = p.runChefClient(o, c) diff --git a/builtin/provisioners/chef/windows_provisioner.go b/builtin/provisioners/chef/windows_provisioner.go index 236bc6a9f..953734021 100644 --- a/builtin/provisioners/chef/windows_provisioner.go +++ b/builtin/provisioners/chef/windows_provisioner.go @@ -66,7 +66,7 @@ func (p *Provisioner) windowsCreateConfigFiles( o terraform.UIOutput, comm communicator.Communicator) error { // Make sure the config directory exists - cmd := fmt.Sprintf("if not exist %q mkdir %q", windowsConfDir, windowsConfDir) + cmd := fmt.Sprintf("cmd /c if not exist %q mkdir %q", windowsConfDir, windowsConfDir) if err := p.runCommand(o, comm, cmd); err != nil { return err } @@ -74,7 +74,7 @@ func (p *Provisioner) windowsCreateConfigFiles( if len(p.OhaiHints) > 0 { // Make sure the hits directory exists hintsDir := path.Join(windowsConfDir, "ohai/hints") - cmd := fmt.Sprintf("if not exist %q mkdir %q", hintsDir, hintsDir) + cmd := fmt.Sprintf("cmd /c if not exist %q mkdir %q", hintsDir, hintsDir) if err := p.runCommand(o, comm, cmd); err != nil { return err } diff --git a/builtin/provisioners/chef/windows_provisioner_test.go b/builtin/provisioners/chef/windows_provisioner_test.go index 442d6a723..a01599a30 100644 --- a/builtin/provisioners/chef/windows_provisioner_test.go +++ b/builtin/provisioners/chef/windows_provisioner_test.go @@ -113,8 +113,8 @@ func TestResourceProvider_windowsCreateConfigFiles(t *testing.T) { }), Commands: map[string]bool{ - fmt.Sprintf("if not exist %q mkdir %q", windowsConfDir, windowsConfDir): true, - fmt.Sprintf("if not exist %q mkdir %q", + fmt.Sprintf("cmd /c if not exist %q mkdir %q", windowsConfDir, windowsConfDir): true, + fmt.Sprintf("cmd /c if not exist %q mkdir %q", path.Join(windowsConfDir, "ohai/hints"), path.Join(windowsConfDir, "ohai/hints")): true, }, @@ -142,7 +142,7 @@ func TestResourceProvider_windowsCreateConfigFiles(t *testing.T) { }), Commands: map[string]bool{ - fmt.Sprintf("if not exist %q mkdir %q", windowsConfDir, windowsConfDir): true, + fmt.Sprintf("cmd /c if not exist %q mkdir %q", windowsConfDir, windowsConfDir): true, }, Uploads: map[string]string{ @@ -185,7 +185,7 @@ func TestResourceProvider_windowsCreateConfigFiles(t *testing.T) { }), Commands: map[string]bool{ - fmt.Sprintf("if not exist %q mkdir %q", windowsConfDir, windowsConfDir): true, + fmt.Sprintf("cmd /c if not exist %q mkdir %q", windowsConfDir, windowsConfDir): true, }, Uploads: map[string]string{ diff --git a/command/counthookaction_string.go b/command/counthookaction_string.go index 6aeffc3eb..8b90dc50b 100644 --- a/command/counthookaction_string.go +++ b/command/counthookaction_string.go @@ -9,7 +9,7 @@ const _countHookAction_name = "countHookActionAddcountHookActionChangecountHookA var _countHookAction_index = [...]uint8{0, 18, 39, 60} func (i countHookAction) String() string { - if i+1 >= countHookAction(len(_countHookAction_index)) { + if i >= countHookAction(len(_countHookAction_index)-1) { return fmt.Sprintf("countHookAction(%d)", i) } return _countHookAction_name[_countHookAction_index[i]:_countHookAction_index[i+1]] diff --git a/helper/schema/valuetype_string.go b/helper/schema/valuetype_string.go index fec00944e..42442a46b 100644 --- a/helper/schema/valuetype_string.go +++ b/helper/schema/valuetype_string.go @@ -9,7 +9,7 @@ const _ValueType_name = "TypeInvalidTypeBoolTypeIntTypeFloatTypeStringTypeListTy var _ValueType_index = [...]uint8{0, 11, 19, 26, 35, 45, 53, 60, 67, 77} func (i ValueType) String() string { - if i < 0 || i+1 >= ValueType(len(_ValueType_index)) { + if i < 0 || i >= ValueType(len(_ValueType_index)-1) { return fmt.Sprintf("ValueType(%d)", i) } return _ValueType_name[_ValueType_index[i]:_ValueType_index[i+1]] diff --git a/terraform/graphnodeconfigtype_string.go b/terraform/graphnodeconfigtype_string.go index de24c2dcd..d8c1724f4 100644 --- a/terraform/graphnodeconfigtype_string.go +++ b/terraform/graphnodeconfigtype_string.go @@ -9,7 +9,7 @@ const _GraphNodeConfigType_name = "GraphNodeConfigTypeInvalidGraphNodeConfigType var _GraphNodeConfigType_index = [...]uint8{0, 26, 53, 80, 105, 130, 157} func (i GraphNodeConfigType) String() string { - if i < 0 || i+1 >= GraphNodeConfigType(len(_GraphNodeConfigType_index)) { + if i < 0 || i >= GraphNodeConfigType(len(_GraphNodeConfigType_index)-1) { return fmt.Sprintf("GraphNodeConfigType(%d)", i) } return _GraphNodeConfigType_name[_GraphNodeConfigType_index[i]:_GraphNodeConfigType_index[i+1]] diff --git a/terraform/instancetype_string.go b/terraform/instancetype_string.go index fc8697644..3114bc157 100644 --- a/terraform/instancetype_string.go +++ b/terraform/instancetype_string.go @@ -9,7 +9,7 @@ const _InstanceType_name = "TypeInvalidTypePrimaryTypeTaintedTypeDeposed" var _InstanceType_index = [...]uint8{0, 11, 22, 33, 44} func (i InstanceType) String() string { - if i < 0 || i+1 >= InstanceType(len(_InstanceType_index)) { + if i < 0 || i >= InstanceType(len(_InstanceType_index)-1) { return fmt.Sprintf("InstanceType(%d)", i) } return _InstanceType_name[_InstanceType_index[i]:_InstanceType_index[i+1]] diff --git a/terraform/walkoperation_string.go b/terraform/walkoperation_string.go index ddd894f53..423793c3c 100644 --- a/terraform/walkoperation_string.go +++ b/terraform/walkoperation_string.go @@ -9,7 +9,7 @@ const _walkOperation_name = "walkInvalidwalkInputwalkApplywalkPlanwalkPlanDestro var _walkOperation_index = [...]uint8{0, 11, 20, 29, 37, 52, 63, 75} func (i walkOperation) String() string { - if i+1 >= walkOperation(len(_walkOperation_index)) { + if i >= walkOperation(len(_walkOperation_index)-1) { return fmt.Sprintf("walkOperation(%d)", i) } return _walkOperation_name[_walkOperation_index[i]:_walkOperation_index[i+1]]