From 11314a3d714390e5ad5b6fbb23652ebc18549f74 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Tue, 12 May 2015 10:37:38 +0200 Subject: [PATCH] Tweaking a few minor things according to the feedback on GH --- builtin/provisioners/chef/resource_provisioner.go | 7 ++++--- builtin/provisioners/chef/resource_provisioner_test.go | 1 + builtin/provisioners/chef/ssh_provisioner.go | 4 ++-- builtin/provisioners/chef/ssh_provisioner_test.go | 4 ++++ builtin/provisioners/chef/winrm_provisioner_test.go | 10 ++++------ website/source/docs/provisioners/chef.html.markdown | 2 +- 6 files changed, 16 insertions(+), 12 deletions(-) diff --git a/builtin/provisioners/chef/resource_provisioner.go b/builtin/provisioners/chef/resource_provisioner.go index 12071d6f4..e546588a5 100644 --- a/builtin/provisioners/chef/resource_provisioner.go +++ b/builtin/provisioners/chef/resource_provisioner.go @@ -74,6 +74,7 @@ type Provisioner struct { installChefClient func(terraform.UIOutput, communicator.Communicator) error createConfigFiles func(terraform.UIOutput, communicator.Communicator) error runChefClient func(terraform.UIOutput, communicator.Communicator) error + useSudo bool } // ResourceProvisioner represents a generic chef provisioner @@ -93,15 +94,15 @@ func (r *ResourceProvisioner) Apply( // Set some values based on the targeted OS switch s.Ephemeral.ConnInfo["type"] { case "ssh", "": // The default connection type is ssh, so if the type is empty use ssh - p.PreventSudo = p.PreventSudo || s.Ephemeral.ConnInfo["user"] == "root" p.installChefClient = p.sshInstallChefClient p.createConfigFiles = p.sshCreateConfigFiles p.runChefClient = p.runChefClientFunc(linuxConfDir) + p.useSudo = !p.PreventSudo && s.Ephemeral.ConnInfo["user"] != "root" case "winrm": - p.PreventSudo = true p.installChefClient = p.winrmInstallChefClient p.createConfigFiles = p.winrmCreateConfigFiles p.runChefClient = p.runChefClientFunc(windowsConfDir) + p.useSudo = false default: return fmt.Errorf("Unsupported connection type: %s", s.Ephemeral.ConnInfo["type"]) } @@ -363,7 +364,7 @@ func (p *Provisioner) runCommand( var err error // Unless prevented, prefix the command with sudo - if !p.PreventSudo { + if p.useSudo { command = "sudo " + command } diff --git a/builtin/provisioners/chef/resource_provisioner_test.go b/builtin/provisioners/chef/resource_provisioner_test.go index 191b5a620..84a343513 100644 --- a/builtin/provisioners/chef/resource_provisioner_test.go +++ b/builtin/provisioners/chef/resource_provisioner_test.go @@ -126,6 +126,7 @@ func TestResourceProvider_runChefClient(t *testing.T) { } p.runChefClient = p.runChefClientFunc(tc.ConfDir) + p.useSudo = !p.PreventSudo err = p.runChefClient(o, c) if err != nil { diff --git a/builtin/provisioners/chef/ssh_provisioner.go b/builtin/provisioners/chef/ssh_provisioner.go index e8f42c3fb..c6d0fb011 100644 --- a/builtin/provisioners/chef/ssh_provisioner.go +++ b/builtin/provisioners/chef/ssh_provisioner.go @@ -50,7 +50,7 @@ func (p *Provisioner) sshCreateConfigFiles( } // Make sure we have enough rights to upload the files if using sudo - if !p.PreventSudo { + if p.useSudo { if err := p.runCommand(o, comm, "chmod 777 "+linuxConfDir); err != nil { return err } @@ -61,7 +61,7 @@ func (p *Provisioner) sshCreateConfigFiles( } // When done copying the files restore the rights and make sure root is owner - if !p.PreventSudo { + if p.useSudo { if err := p.runCommand(o, comm, "chmod 755 "+linuxConfDir); err != nil { return err } diff --git a/builtin/provisioners/chef/ssh_provisioner_test.go b/builtin/provisioners/chef/ssh_provisioner_test.go index 5673a4f4e..90e7b7d91 100644 --- a/builtin/provisioners/chef/ssh_provisioner_test.go +++ b/builtin/provisioners/chef/ssh_provisioner_test.go @@ -116,6 +116,8 @@ func TestResourceProvider_sshInstallChefClient(t *testing.T) { t.Fatalf("Error: %v", err) } + p.useSudo = !p.PreventSudo + err = p.sshInstallChefClient(o, c) if err != nil { t.Fatalf("Test %q failed: %v", k, err) @@ -254,6 +256,8 @@ func TestResourceProvider_sshCreateConfigFiles(t *testing.T) { t.Fatalf("Error: %v", err) } + p.useSudo = !p.PreventSudo + err = p.sshCreateConfigFiles(o, c) if err != nil { t.Fatalf("Test %q failed: %v", k, err) diff --git a/builtin/provisioners/chef/winrm_provisioner_test.go b/builtin/provisioners/chef/winrm_provisioner_test.go index 03edc11a5..075e261bd 100644 --- a/builtin/provisioners/chef/winrm_provisioner_test.go +++ b/builtin/provisioners/chef/winrm_provisioner_test.go @@ -17,7 +17,6 @@ func TestResourceProvider_winrmInstallChefClient(t *testing.T) { "Default": { Config: testConfig(t, map[string]interface{}{ "node_name": "nodename1", - "prevent_sudo": true, // Needs to be set for ALL WinRM tests! "run_list": []interface{}{"cookbook::recipe"}, "server_url": "https://chef.local", "validation_client_name": "validator", @@ -38,7 +37,6 @@ func TestResourceProvider_winrmInstallChefClient(t *testing.T) { "http_proxy": "http://proxy.local", "no_proxy": []interface{}{"http://local.local", "http://local.org"}, "node_name": "nodename1", - "prevent_sudo": true, // Needs to be set for ALL WinRM tests! "run_list": []interface{}{"cookbook::recipe"}, "server_url": "https://chef.local", "validation_client_name": "validator", @@ -57,7 +55,6 @@ func TestResourceProvider_winrmInstallChefClient(t *testing.T) { "Version": { Config: testConfig(t, map[string]interface{}{ "node_name": "nodename1", - "prevent_sudo": true, // Needs to be set for ALL WinRM tests! "run_list": []interface{}{"cookbook::recipe"}, "server_url": "https://chef.local", "validation_client_name": "validator", @@ -88,6 +85,8 @@ func TestResourceProvider_winrmInstallChefClient(t *testing.T) { t.Fatalf("Error: %v", err) } + p.useSudo = false + err = p.winrmInstallChefClient(o, c) if err != nil { t.Fatalf("Test %q failed: %v", k, err) @@ -104,7 +103,6 @@ func TestResourceProvider_winrmCreateConfigFiles(t *testing.T) { "Default": { Config: testConfig(t, map[string]interface{}{ "node_name": "nodename1", - "prevent_sudo": true, // Needs to be set for ALL WinRM tests! "run_list": []interface{}{"cookbook::recipe"}, "server_url": "https://chef.local", "validation_client_name": "validator", @@ -128,7 +126,6 @@ func TestResourceProvider_winrmCreateConfigFiles(t *testing.T) { "https_proxy": "https://proxy.local", "no_proxy": []interface{}{"http://local.local", "https://local.local"}, "node_name": "nodename1", - "prevent_sudo": true, // Needs to be set for ALL WinRM tests! "run_list": []interface{}{"cookbook::recipe"}, "server_url": "https://chef.local", "validation_client_name": "validator", @@ -170,7 +167,6 @@ func TestResourceProvider_winrmCreateConfigFiles(t *testing.T) { }, }, "node_name": "nodename1", - "prevent_sudo": true, // Needs to be set for ALL WinRM tests! "run_list": []interface{}{"cookbook::recipe"}, "server_url": "https://chef.local", "validation_client_name": "validator", @@ -203,6 +199,8 @@ func TestResourceProvider_winrmCreateConfigFiles(t *testing.T) { t.Fatalf("Error: %v", err) } + p.useSudo = false + err = p.winrmCreateConfigFiles(o, c) if err != nil { t.Fatalf("Test %q failed: %v", k, err) diff --git a/website/source/docs/provisioners/chef.html.markdown b/website/source/docs/provisioners/chef.html.markdown index b56fa4f73..fb7bc4416 100644 --- a/website/source/docs/provisioners/chef.html.markdown +++ b/website/source/docs/provisioners/chef.html.markdown @@ -48,7 +48,7 @@ resource "aws_instance" "web" { The following arguments are supported: -* `attributes (hash)` - (Optional) A hash with initial node attributes for the new node. +* `attributes (map)` - (Optional) A map with initial node attributes for the new node. See example. * `environment (string)` - (Optional) The Chef environment the new node will be joining