From 5976929595e3d62cc21b93ac95ebe648f42a6b64 Mon Sep 17 00:00:00 2001 From: "Lane, Larry" Date: Tue, 2 Jun 2020 17:30:03 -0500 Subject: [PATCH] Fix for #9283 --- provisioner/ansible/provisioner.go | 54 ++++++++++++++------ provisioner/ansible/provisioner_test.go | 66 +++++++++++++++++++++---- 2 files changed, 95 insertions(+), 25 deletions(-) diff --git a/provisioner/ansible/provisioner.go b/provisioner/ansible/provisioner.go index 2a200b447..7dbcbd613 100644 --- a/provisioner/ansible/provisioner.go +++ b/provisioner/ansible/provisioner.go @@ -550,6 +550,11 @@ func (p *Provisioner) executeGalaxy(ui packer.Ui, comm packer.Communicator) erro func (p *Provisioner) createCmdArgs(httpAddr, inventory, playbook, privKeyFile string) (args []string, envVars []string) { args = []string{} + //Setting up AnsibleEnvVars at begining so additional checks can take them into account + if len(p.config.AnsibleEnvVars) > 0 { + envVars = append(envVars, p.config.AnsibleEnvVars...) + } + if p.config.PackerBuildName != "" { // HCL configs don't currently have the PakcerBuildName. Don't // cause weirdness with a half-set variable @@ -557,34 +562,36 @@ func (p *Provisioner) createCmdArgs(httpAddr, inventory, playbook, privKeyFile s } args = append(args, "-e", fmt.Sprintf("packer_builder_type=%s", p.config.PackerBuilderType)) - if len(privKeyFile) > 0 { - // "-e ansible_ssh_private_key_file" is preferable to "--private-key" - // because it is a higher priority variable and therefore won't get - // overridden by dynamic variables. See #5852 for more details. - args = append(args, "-e", fmt.Sprintf("ansible_ssh_private_key_file=%s", privKeyFile)) - } // expose packer_http_addr extra variable if httpAddr != "" { args = append(args, "-e", fmt.Sprintf("packer_http_addr=%s", httpAddr)) } - // Add password to ansible call. - if p.config.UseProxy.False() && p.generatedData["ConnType"] == "winrm" { - args = append(args, "-e", fmt.Sprintf("ansible_password=%s", p.generatedData["Password"])) - } - if p.generatedData["ConnType"] == "ssh" { // Add ssh extra args to set IdentitiesOnly - args = append(args, "--ssh-extra-args", "-o IdentitiesOnly=yes") + args = append(args, "--ssh-extra-args", "'-o IdentitiesOnly=yes'") } args = append(args, p.config.ExtraArguments...) - if len(p.config.AnsibleEnvVars) > 0 { - envVars = append(envVars, p.config.AnsibleEnvVars...) + // Add password to ansible call. + if !checkArg("ansible_password", args) && p.config.UseProxy.False() && p.generatedData["ConnType"] == "winrm" { + args = append(args, "-e", fmt.Sprintf("ansible_password=%s", p.generatedData["Password"])) } + if !checkArg("ansible_password", args) && len(privKeyFile) > 0 { + // "-e ansible_ssh_private_key_file" is preferable to "--private-key" + // because it is a higher priority variable and therefore won't get + // overridden by dynamic variables. See #5852 for more details. + args = append(args, "-e", fmt.Sprintf("ansible_ssh_private_key_file=%s", privKeyFile)) + } + + if checkArg("ansible_password", args) && p.generatedData["ConnType"] == "ssh" { + if !checkArg("ansible_host_key_checking", args) && !checkArg("ANSIBLE_HOST_KEY_CHECKING", envVars) { + args = append(args, "-e", "ansible_host_key_checking=False") + } + } // This must be the last arg appended to args args = append(args, "-i", inventory, playbook) return args, envVars @@ -601,7 +608,6 @@ func (p *Provisioner) executeAnsible(ui packer.Ui, comm packer.Communicator, pri return fmt.Errorf("Error executing Ansible Galaxy: %s", err) } } - args, envvars := p.createCmdArgs(httpAddr, inventory, playbook, privKeyFile) cmd := exec.Command(p.config.Command, args...) @@ -652,6 +658,12 @@ func (p *Provisioner) executeAnsible(ui packer.Ui, comm packer.Communicator, pri sanitized = strings.Replace(sanitized, winRMPass.(string), "*****", -1) } + if checkArg("ansible_password", args) { + usePass, ok := p.generatedData["Password"] + if ok && usePass != "" { + sanitized = strings.Replace(sanitized, usePass.(string), "*****", -1) + } + } ui.Say(fmt.Sprintf("Executing Ansible: %s", sanitized)) if err := cmd.Start(); err != nil { @@ -779,3 +791,15 @@ func newSigner(privKeyFile string) (*signer, error) { return signer, nil } + +//checkArg Evaluates if argname is in args +func checkArg(argname string, args []string) bool { + for _, arg := range args { + for _, ansibleArg := range strings.Split(arg, "=") { + if ansibleArg == argname { + return true + } + } + } + return false +} diff --git a/provisioner/ansible/provisioner_test.go b/provisioner/ansible/provisioner_test.go index d5b433684..99bfadefe 100644 --- a/provisioner/ansible/provisioner_test.go +++ b/provisioner/ansible/provisioner_test.go @@ -485,6 +485,7 @@ func basicGenData(input map[string]interface{}) map[string]interface{} { func TestCreateCmdArgs(t *testing.T) { type testcase struct { + TestName string PackerBuildName string PackerBuilderType string UseProxy confighelper.Trilean @@ -498,25 +499,28 @@ func TestCreateCmdArgs(t *testing.T) { TestCases := []testcase{ { // SSH with private key and an extra argument. + TestName: "SSH with private key and an extra argument", PackerBuildName: "packerparty", generatedData: basicGenData(nil), ExtraArguments: []string{"-e", "hello-world"}, AnsibleEnvVars: []string{"ENV_1=pancakes", "ENV_2=bananas"}, callArgs: []string{"", "/var/inventory", "test-playbook.yml", "/path/to/privkey.pem"}, - ExpectedArgs: []string{"-e", "packer_build_name=packerparty", "-e", "packer_builder_type=fakebuilder", "-e", "ansible_ssh_private_key_file=/path/to/privkey.pem", "--ssh-extra-args", "-o IdentitiesOnly=yes", "-i", "/var/inventory", "test-playbook.yml", "-e", "hello-world"}, + ExpectedArgs: []string{"-e", "packer_build_name=packerparty", "-e", "packer_builder_type=fakebuilder", "-e", "ansible_ssh_private_key_file=/path/to/privkey.pem", "--ssh-extra-args", "'-o IdentitiesOnly=yes'", "-e", "hello-world", "-i", "/var/inventory", "test-playbook.yml"}, ExpectedEnvVars: []string{"ENV_1=pancakes", "ENV_2=bananas"}, }, { + TestName: "SSH with private key and an extra argument and UseProxy", PackerBuildName: "packerparty", UseProxy: confighelper.TriTrue, generatedData: basicGenData(nil), ExtraArguments: []string{"-e", "hello-world"}, callArgs: []string{"", "/var/inventory", "test-playbook.yml", "/path/to/privkey.pem"}, - ExpectedArgs: []string{"-e", "packer_build_name=packerparty", "-e", "packer_builder_type=fakebuilder", "-e", "ansible_ssh_private_key_file=/path/to/privkey.pem", "--ssh-extra-args", "-o IdentitiesOnly=yes", "-i", "/var/inventory", "test-playbook.yml", "-e", "hello-world"}, + ExpectedArgs: []string{"-e", "packer_build_name=packerparty", "-e", "packer_builder_type=fakebuilder", "-e", "ansible_ssh_private_key_file=/path/to/privkey.pem", "--ssh-extra-args", "'-o IdentitiesOnly=yes'", "-e", "hello-world", "-i", "/var/inventory", "test-playbook.yml"}, ExpectedEnvVars: []string{}, }, { // Winrm, but no_proxy is unset so we don't do anything with ansible_password. + TestName: "Winrm, but no_proxy is unset so we don't do anything with ansible_password", PackerBuildName: "packerparty", generatedData: basicGenData(map[string]interface{}{ "ConnType": "winrm", @@ -524,20 +528,22 @@ func TestCreateCmdArgs(t *testing.T) { ExtraArguments: []string{"-e", "hello-world"}, AnsibleEnvVars: []string{"ENV_1=pancakes", "ENV_2=bananas"}, callArgs: []string{"", "/var/inventory", "test-playbook.yml", ""}, - ExpectedArgs: []string{"-e", "packer_build_name=packerparty", "-e", "packer_builder_type=fakebuilder", "-i", "/var/inventory", "test-playbook.yml", "-e", "hello-world"}, + ExpectedArgs: []string{"-e", "packer_build_name=packerparty", "-e", "packer_builder_type=fakebuilder", "-e", "hello-world", "-i", "/var/inventory", "test-playbook.yml"}, ExpectedEnvVars: []string{"ENV_1=pancakes", "ENV_2=bananas"}, }, { // HTTPAddr should be set. No env vars. + TestName: "HTTPAddr should be set. No env vars", PackerBuildName: "packerparty", ExtraArguments: []string{"-e", "hello-world"}, generatedData: basicGenData(nil), callArgs: []string{"123.45.67.89", "/var/inventory", "test-playbook.yml", ""}, - ExpectedArgs: []string{"-e", "packer_build_name=packerparty", "-e", "packer_builder_type=fakebuilder", "-e", "packer_http_addr=123.45.67.89", "--ssh-extra-args", "-o IdentitiesOnly=yes", "-i", "/var/inventory", "test-playbook.yml", "-e", "hello-world"}, + ExpectedArgs: []string{"-e", "packer_build_name=packerparty", "-e", "packer_builder_type=fakebuilder", "-e", "packer_http_addr=123.45.67.89", "--ssh-extra-args", "'-o IdentitiesOnly=yes'", "-e", "hello-world", "-i", "/var/inventory", "test-playbook.yml"}, ExpectedEnvVars: []string{}, }, { // Add ansible_password for proxyless winrm connection. + TestName: "Add ansible_password for proxyless winrm connection.", UseProxy: confighelper.TriFalse, generatedData: basicGenData(map[string]interface{}{ "ConnType": "winrm", @@ -550,6 +556,7 @@ func TestCreateCmdArgs(t *testing.T) { }, { // Neither special ssh stuff, nor special windows stuff. This is docker! + TestName: "Neither special ssh stuff, nor special windows stuff. This is docker!", PackerBuildName: "packerparty", generatedData: basicGenData(map[string]interface{}{ "ConnType": "docker", @@ -557,11 +564,12 @@ func TestCreateCmdArgs(t *testing.T) { ExtraArguments: []string{"-e", "hello-world"}, AnsibleEnvVars: []string{"ENV_1=pancakes", "ENV_2=bananas"}, callArgs: []string{"", "/var/inventory", "test-playbook.yml", ""}, - ExpectedArgs: []string{"-e", "packer_build_name=packerparty", "-e", "packer_builder_type=fakebuilder", "-i", "/var/inventory", "test-playbook.yml", "-e", "hello-world"}, + ExpectedArgs: []string{"-e", "packer_build_name=packerparty", "-e", "packer_builder_type=fakebuilder", "-e", "hello-world", "-i", "/var/inventory", "test-playbook.yml"}, ExpectedEnvVars: []string{"ENV_1=pancakes", "ENV_2=bananas"}, }, { // Windows, no proxy, with extra vars. + TestName: "Windows, no proxy, with extra vars.", UseProxy: confighelper.TriFalse, generatedData: basicGenData(map[string]interface{}{ "ConnType": "winrm", @@ -570,14 +578,51 @@ func TestCreateCmdArgs(t *testing.T) { ExtraArguments: []string{"-e", "hello-world"}, AnsibleEnvVars: []string{"ENV_1=pancakes", "ENV_2=bananas"}, callArgs: []string{"123.45.67.89", "/var/inventory", "test-playbook.yml", ""}, - ExpectedArgs: []string{"-e", "packer_builder_type=fakebuilder", "-e", "packer_http_addr=123.45.67.89", "-e", "ansible_password=ilovebananapancakes", "-i", "/var/inventory", "test-playbook.yml", "-e", "hello-world"}, + ExpectedArgs: []string{"-e", "packer_builder_type=fakebuilder", "-e", "packer_http_addr=123.45.67.89", "-e", "ansible_password=ilovebananapancakes", "-e", "hello-world", "-i", "/var/inventory", "test-playbook.yml"}, ExpectedEnvVars: []string{"ENV_1=pancakes", "ENV_2=bananas"}, }, + { + // SSH, use Password. + TestName: "SSH, use ansible_password.", + generatedData: basicGenData(map[string]interface{}{ + "ConnType": "ssh", + "Password": "ilovebananapancakes", + }), + ExtraArguments: []string{"-e", "hello-world", "-e", "ansible_password=ilovebananapancakes"}, + AnsibleEnvVars: []string{"ENV_1=pancakes", "ENV_2=bananas"}, + callArgs: []string{"123.45.67.89", "/var/inventory", "test-playbook.yml", ""}, + ExpectedArgs: []string{"-e", "packer_builder_type=fakebuilder", "-e", "packer_http_addr=123.45.67.89", "--ssh-extra-args", "'-o IdentitiesOnly=yes'", "-e", "hello-world", "-e", "ansible_password=ilovebananapancakes", "-e", "ansible_host_key_checking=False", "-i", "/var/inventory", "test-playbook.yml"}, + ExpectedEnvVars: []string{"ENV_1=pancakes", "ENV_2=bananas"}, + }, + { + // SSH, use Password . + TestName: "SSH, already in ENV ansible_host_key_checking.", + generatedData: basicGenData(map[string]interface{}{ + "ConnType": "ssh", + "Password": "ilovebananapancakes", + }), + ExtraArguments: []string{"-e", "hello-world", "-e", "ansible_password=ilovebananapancakes"}, + AnsibleEnvVars: []string{"ENV_1=pancakes", "ENV_2=bananas", "ANSIBLE_HOST_KEY_CHECKING=False"}, + callArgs: []string{"123.45.67.89", "/var/inventory", "test-playbook.yml", ""}, + ExpectedArgs: []string{"-e", "packer_builder_type=fakebuilder", "-e", "packer_http_addr=123.45.67.89", "--ssh-extra-args", "'-o IdentitiesOnly=yes'", "-e", "hello-world", "-e", "ansible_password=ilovebananapancakes", "-i", "/var/inventory", "test-playbook.yml"}, + ExpectedEnvVars: []string{"ENV_1=pancakes", "ENV_2=bananas", "ANSIBLE_HOST_KEY_CHECKING=False"}, + }, + { + TestName: "Use PrivateKey", + PackerBuildName: "packerparty", + UseProxy: confighelper.TriTrue, + generatedData: basicGenData(nil), + ExtraArguments: []string{"-e", "hello-world"}, + callArgs: []string{"", "/var/inventory", "test-playbook.yml", "/path/to/privkey.pem"}, + ExpectedArgs: []string{"-e", "packer_build_name=packerparty", "-e", "packer_builder_type=fakebuilder", "-e", "ansible_ssh_private_key_file=/path/to/privkey.pem", "--ssh-extra-args", "'-o IdentitiesOnly=yes'", "-e", "hello-world", "-i", "/var/inventory", "test-playbook.yml"}, + ExpectedEnvVars: []string{}, + }, { // No builder name. This shouldn't cause an error, it just shouldn't be set. HCL, yo. + TestName: "No builder name. This shouldn't cause an error, it just shouldn't be set. HCL, yo.", generatedData: basicGenData(nil), callArgs: []string{"", "/var/inventory", "test-playbook.yml", ""}, - ExpectedArgs: []string{"-e", "packer_builder_type=fakebuilder", "--ssh-extra-args", "-o IdentitiesOnly=yes", "-i", "/var/inventory", "test-playbook.yml"}, + ExpectedArgs: []string{"-e", "packer_builder_type=fakebuilder", "--ssh-extra-args", "'-o IdentitiesOnly=yes'", "-i", "/var/inventory", "test-playbook.yml"}, ExpectedEnvVars: []string{}, }, } @@ -595,10 +640,11 @@ func TestCreateCmdArgs(t *testing.T) { args, envVars := p.createCmdArgs(tc.callArgs[0], tc.callArgs[1], tc.callArgs[2], tc.callArgs[3]) assert.ElementsMatch(t, args, tc.ExpectedArgs, - "Args didn't match expected:\n\n expected: \n%s\n; recieved: \n%s\n", tc.ExpectedArgs, args) - assert.ElementsMatch(t, envVars, tc.ExpectedEnvVars, "EnvVars didn't match expected:\n\n expected: \n%s\n; recieved: \n%s\n", tc.ExpectedEnvVars, envVars) + "TestName: %s\nArgs didn't match expected:\nexpected: \n%s\n; recieved: \n%s\n", tc.TestName, tc.ExpectedArgs, args) + assert.ElementsMatch(t, envVars, tc.ExpectedEnvVars, + "TestName: %s\nArgs didn't match expected:\n\nEnvVars didn't match expected:\n\n expected: \n%s\n; recieved: \n%s\n", tc.TestName, tc.ExpectedEnvVars, envVars) assert.EqualValues(t, tc.callArgs[2], args[len(args)-1], - "PlayBook File Not Returned as last element: \nexpected: %s\nrecieved: %s\n", tc.callArgs[2], args[len(args)-1]) + "TestName: %s\nPlayBook File Not Returned as last element: \nexpected: %s\nrecieved: %s\n", tc.TestName, tc.callArgs[2], args[len(args)-1]) } }