Merge pull request #7103 from mitchellh/gildegoma/fix-6726

ansible_local: use double quoting for 'extra-vars', 'limit' and 'start-at-task' options (except if defined via `raw_arguments` option)
This commit is contained in:
Gilles Cornu 2016-04-21 00:29:43 +02:00
commit b6a3f0e8f1
9 changed files with 139 additions and 47 deletions

View File

@ -112,6 +112,17 @@ module VagrantPlugins
end
end
if raw_arguments
if raw_arguments.kind_of?(String)
@raw_arguments = [raw_arguments]
elsif !raw_arguments.kind_of?(Array)
@errors << I18n.t(
"vagrant.provisioners.ansible.errors.raw_arguments_invalid",
type: raw_arguments.class.to_s,
value: raw_arguments.to_s)
end
end
end
end
end

View File

@ -34,6 +34,17 @@ module VagrantPlugins
def validate(machine)
super
if raw_ssh_args
if raw_ssh_args.kind_of?(String)
@raw_ssh_args = [raw_ssh_args]
elsif !raw_ssh_args.kind_of?(Array)
@errors << I18n.t(
"vagrant.provisioners.ansible.errors.raw_ssh_args_invalid",
type: raw_ssh_args.class.to_s,
value: raw_ssh_args.to_s)
end
end
{ "ansible remote provisioner" => @errors }
end

View File

@ -3,28 +3,6 @@ require "vagrant"
module VagrantPlugins
module Ansible
class Helpers
def self.stringify_ansible_playbook_command(env, command)
shell_command = ''
env.each_pair do |k, v|
if k == 'ANSIBLE_SSH_ARGS'
shell_command += "#{k}='#{v}' "
else
shell_command += "#{k}=#{v} "
end
end
shell_arg = []
command.each do |arg|
if arg =~ /(--start-at-task|--limit)=(.+)/
shell_arg << "#{$1}='#{$2}'"
else
shell_arg << arg
end
end
shell_command += shell_arg.join(' ')
end
def self.expand_path_in_unix_style(path, base_dir)
# Remove the possible drive letter, which is added
# by `File.expand_path` when running on a Windows host
@ -34,10 +12,6 @@ module VagrantPlugins
def self.as_list_argument(v)
v.kind_of?(Array) ? v.join(',') : v
end
def self.as_array(v)
v.kind_of?(Array) ? v : [v]
end
end
end
end

View File

@ -25,6 +25,39 @@ module VagrantPlugins
@inventory_path = nil
end
def ansible_playbook_command_for_shell_execution
shell_command = []
@environment_variables.each_pair do |k, v|
if k == 'ANSIBLE_SSH_ARGS'
shell_command << "#{k}='#{v}'"
else
shell_command << "#{k}=#{v}"
end
end
shell_command << "ansible-playbook"
shell_args = []
@command_arguments.each do |arg|
if arg =~ /(--start-at-task|--limit)=(.+)/
shell_args << %Q(#{$1}="#{$2}")
elsif arg =~ /(--extra-vars)=(.+)/
shell_args << %Q(%s="%s") % [$1, $2.gsub('\\', '\\\\\\').gsub('"', %Q(\\"))]
else
shell_args << arg
end
end
shell_command << shell_args
# Add the raw arguments at the end, to give them the highest precedence
shell_command << config.raw_arguments if config.raw_arguments
shell_command << config.playbook
shell_command.flatten.join(' ')
end
def prepare_common_command_arguments
# By default we limit by the current machine,
# but this can be overridden by the `limit` option.
@ -43,10 +76,6 @@ module VagrantPlugins
@command_arguments << "--tags=#{Helpers::as_list_argument(config.tags)}" if config.tags
@command_arguments << "--skip-tags=#{Helpers::as_list_argument(config.skip_tags)}" if config.skip_tags
@command_arguments << "--start-at-task=#{config.start_at_task}" if config.start_at_task
# Finally, add the raw configuration options, which has the highest precedence
# and can therefore potentially override any other options of this provisioner.
@command_arguments.concat(Helpers::as_array(config.raw_arguments)) if config.raw_arguments
end
def prepare_common_environment_variables

View File

@ -78,8 +78,7 @@ module VagrantPlugins
prepare_common_command_arguments
prepare_common_environment_variables
command = (%w(ansible-playbook) << @command_arguments << config.playbook).flatten
remote_command = "cd #{config.provisioning_path} && #{Helpers::stringify_ansible_playbook_command(@environment_variables, command)}"
remote_command = "cd #{config.provisioning_path} && #{ansible_playbook_command_for_shell_execution}"
execute_ansible_command_on_guest "playbook", remote_command
end

View File

@ -94,8 +94,6 @@ module VagrantPlugins
command_template = config.galaxy_command.gsub(' ', VAGRANT_ARG_SEPARATOR)
str_command = command_template % command_values
ui_running_ansible_command "galaxy", str_command.gsub(VAGRANT_ARG_SEPARATOR, ' ')
command = str_command.split(VAGRANT_ARG_SEPARATOR)
command << {
# Write stdout and stderr data, since it's the regular Ansible output
@ -103,6 +101,8 @@ module VagrantPlugins
workdir: @machine.env.root_path.to_s
}
ui_running_ansible_command "galaxy", str_command.gsub(VAGRANT_ARG_SEPARATOR, ' ')
execute_command_from_host command
end
@ -111,9 +111,13 @@ module VagrantPlugins
prepare_environment_variables
# Assemble the full ansible-playbook command
command = (%w(ansible-playbook) << @command_arguments << config.playbook).flatten
command = %w(ansible-playbook) << @command_arguments
ui_running_ansible_command "playbook", Helpers::stringify_ansible_playbook_command(@environment_variables, command)
# Add the raw arguments at the end, to give them the highest precedence
command << config.raw_arguments if config.raw_arguments
command << config.playbook
command = command.flatten
command << {
env: @environment_variables,
@ -122,6 +126,8 @@ module VagrantPlugins
workdir: @machine.env.root_path.to_s
}
ui_running_ansible_command "playbook", ansible_playbook_command_for_shell_execution
execute_command_from_host command
end
@ -237,7 +243,7 @@ module VagrantPlugins
ssh_options << "-o ForwardAgent=yes" if @ssh_info[:forward_agent]
# Unchecked SSH Parameters
ssh_options.concat(Helpers::as_array(config.raw_ssh_args)) if config.raw_ssh_args
ssh_options.concat(config.raw_ssh_args) if config.raw_ssh_args
# Re-enable ControlPersist Ansible defaults,
# which are lost when ANSIBLE_SSH_ARGS is defined.

View File

@ -2123,6 +2123,10 @@ en:
for more information.
extra_vars_invalid: |-
`extra_vars` must be a hash or a path to an existing file. Received: %{value} (as %{type})
raw_arguments_invalid: |-
`raw_arguments` must be an array of strings. Received: %{value} (as %{type})
raw_ssh_args_invalid: |-
`raw_ssh_args` must be an array of strings. Received: %{value} (as %{type})
galaxy_role_file_invalid: |-
`galaxy_role_file` does not exist on the %{system}: %{path}
inventory_path_invalid: |-

View File

@ -200,6 +200,46 @@ describe VagrantPlugins::Ansible::Config::Host do
])
end
it "returns an error if the raw_arguments is of the wrong data type" do
subject.raw_arguments = { arg1: 1, arg2: "foo" }
subject.finalize!
result = subject.validate(machine)
expect(result["ansible remote provisioner"]).to eql([
I18n.t("vagrant.provisioners.ansible.errors.raw_arguments_invalid",
type: subject.raw_arguments.class.to_s,
value: subject.raw_arguments.to_s)
])
end
it "converts a raw_arguments option defined as a String into an Array" do
subject.raw_arguments = "--foo=bar"
subject.finalize!
result = subject.validate(machine)
expect(subject.raw_arguments).to eql(%w(--foo=bar))
end
it "returns an error if the raw_ssh_args is of the wrong data type" do
subject.raw_ssh_args = { arg1: 1, arg2: "foo" }
subject.finalize!
result = subject.validate(machine)
expect(result["ansible remote provisioner"]).to eql([
I18n.t("vagrant.provisioners.ansible.errors.raw_ssh_args_invalid",
type: subject.raw_ssh_args.class.to_s,
value: subject.raw_ssh_args.to_s)
])
end
it "converts a raw_ssh_args option defined as a String into an Array" do
subject.raw_arguments = "-o ControlMaster=no"
subject.finalize!
result = subject.validate(machine)
expect(subject.raw_arguments).to eql(["-o ControlMaster=no"])
end
it "it collects and returns all detected errors" do
subject.playbook = non_existing_file
subject.inventory_path = non_existing_file

View File

@ -639,7 +639,7 @@ VF
it "shows the ansible-playbook command and set verbosity to '-#{verbose_option}' level" do
expect(machine.env.ui).to receive(:detail).with { |full_command|
expect(full_command).to eq("PYTHONUNBUFFERED=1 ANSIBLE_FORCE_COLOR=true ANSIBLE_HOST_KEY_CHECKING=false ANSIBLE_SSH_ARGS='-o UserKnownHostsFile=/dev/null -o IdentitiesOnly=yes -o ControlMaster=auto -o ControlPersist=60s' ansible-playbook --connection=ssh --timeout=30 --limit='machine1' --inventory-file=#{generated_inventory_dir} -#{verbose_option} playbook.yml")
expect(full_command).to eq("PYTHONUNBUFFERED=1 ANSIBLE_FORCE_COLOR=true ANSIBLE_HOST_KEY_CHECKING=false ANSIBLE_SSH_ARGS='-o UserKnownHostsFile=/dev/null -o IdentitiesOnly=yes -o ControlMaster=auto -o ControlPersist=60s' ansible-playbook --connection=ssh --timeout=30 --limit=\"machine1\" --inventory-file=#{generated_inventory_dir} -#{verbose_option} playbook.yml")
}
end
end
@ -654,7 +654,7 @@ VF
it "shows the ansible-playbook command and set verbosity to '-#{verbose_option}' level" do
expect(machine.env.ui).to receive(:detail).with { |full_command|
expect(full_command).to eq("PYTHONUNBUFFERED=1 ANSIBLE_FORCE_COLOR=true ANSIBLE_HOST_KEY_CHECKING=false ANSIBLE_SSH_ARGS='-o UserKnownHostsFile=/dev/null -o IdentitiesOnly=yes -o ControlMaster=auto -o ControlPersist=60s' ansible-playbook --connection=ssh --timeout=30 --limit='machine1' --inventory-file=#{generated_inventory_dir} -#{verbose_option} playbook.yml")
expect(full_command).to eq("PYTHONUNBUFFERED=1 ANSIBLE_FORCE_COLOR=true ANSIBLE_HOST_KEY_CHECKING=false ANSIBLE_SSH_ARGS='-o UserKnownHostsFile=/dev/null -o IdentitiesOnly=yes -o ControlMaster=auto -o ControlPersist=60s' ansible-playbook --connection=ssh --timeout=30 --limit=\"machine1\" --inventory-file=#{generated_inventory_dir} -#{verbose_option} playbook.yml")
}
end
end
@ -670,7 +670,7 @@ VF
it "shows the ansible-playbook command and set verbosity to '-v' level" do
expect(machine.env.ui).to receive(:detail).with { |full_command|
expect(full_command).to eq("PYTHONUNBUFFERED=1 ANSIBLE_FORCE_COLOR=true ANSIBLE_HOST_KEY_CHECKING=false ANSIBLE_SSH_ARGS='-o UserKnownHostsFile=/dev/null -o IdentitiesOnly=yes -o ControlMaster=auto -o ControlPersist=60s' ansible-playbook --connection=ssh --timeout=30 --limit='machine1' --inventory-file=#{generated_inventory_dir} -v playbook.yml")
expect(full_command).to eq("PYTHONUNBUFFERED=1 ANSIBLE_FORCE_COLOR=true ANSIBLE_HOST_KEY_CHECKING=false ANSIBLE_SSH_ARGS='-o UserKnownHostsFile=/dev/null -o IdentitiesOnly=yes -o ControlMaster=auto -o ControlPersist=60s' ansible-playbook --connection=ssh --timeout=30 --limit=\"machine1\" --inventory-file=#{generated_inventory_dir} -v playbook.yml")
}
end
end
@ -732,6 +732,24 @@ VF
end
end
context "with extra_vars option defined" do
describe "with a hash value" do
before do
config.extra_vars = { var1: %Q(string with 'apostrophes', \\, " and =), var2: { x: 42 } }
end
it_should_set_optional_arguments({ "extra_vars" => "--extra-vars={\"var1\":\"string with 'apostrophes', \\\\, \\\" and =\",\"var2\":{\"x\":42}}" })
end
describe "with a string value referring to file path (with the '@' prefix)" do
before do
config.extra_vars = "@#{existing_file}"
end
it_should_set_optional_arguments({ "extra_vars" => "--extra-vars=@#{File.expand_path(__FILE__)}" })
end
end
# The Vagrant Ansible provisioner does not validate the coherency of
# argument combinations, and let ansible-playbook complain.
describe "with a maximum of options" do
@ -741,7 +759,7 @@ VF
ssh_info[:private_key_path] = ['/my/key1', '/my/key2']
# command line arguments
config.extra_vars = "@#{existing_file}"
config.extra_vars = { var1: %Q(string with 'apostrophes', \\, " and =), var2: { x: 42 } }
config.sudo = true
config.sudo_user = 'deployer'
config.verbose = "vvv"
@ -751,17 +769,17 @@ VF
config.tags = %w(db www)
config.skip_tags = %w(foo bar)
config.limit = 'machine*:&vagrant:!that_one'
config.start_at_task = 'an awesome task'
config.raw_arguments = ["--why-not", "--su-user=foot", "--ask-su-pass", "--limit=all", "--private-key=./myself.key"]
config.start_at_task = "joe's awesome task"
config.raw_arguments = ["--why-not", "--su-user=foot", "--ask-su-pass", "--limit=all", "--private-key=./myself.key", "--extra-vars='{\"var3\":\"foo\"}'"]
# environment variables
config.host_key_checking = true
config.raw_ssh_args = ['-o ControlMaster=no']
end
it_should_set_arguments_and_environment_variables 20, 4, true
it_should_set_arguments_and_environment_variables 21, 4, true
it_should_explicitly_enable_ansible_ssh_control_persist_defaults
it_should_set_optional_arguments({ "extra_vars" => "--extra-vars=@#{File.expand_path(__FILE__)}",
it_should_set_optional_arguments({ "extra_vars" => "--extra-vars={\"var1\":\"string with 'apostrophes', \\\\, \\\" and =\",\"var2\":{\"x\":42}}",
"sudo" => "--sudo",
"sudo_user" => "--sudo-user=deployer",
"verbose" => "-vvv",
@ -771,7 +789,7 @@ VF
"tags" => "--tags=db,www",
"skip_tags" => "--skip-tags=foo,bar",
"limit" => "--limit=machine*:&vagrant:!that_one",
"start_at_task" => "--start-at-task=an awesome task",
"start_at_task" => "--start-at-task=joe's awesome task",
})
it "also includes given raw arguments" do
@ -786,7 +804,7 @@ VF
it "shows the ansible-playbook command, with additional quotes when required" do
expect(machine.env.ui).to receive(:detail).with { |full_command|
expect(full_command).to eq("PYTHONUNBUFFERED=1 ANSIBLE_FORCE_COLOR=true ANSIBLE_HOST_KEY_CHECKING=true ANSIBLE_SSH_ARGS='-o IdentitiesOnly=yes -i '/my/key1' -i '/my/key2' -o ForwardAgent=yes -o ControlMaster=no -o ControlMaster=auto -o ControlPersist=60s' ansible-playbook --connection=ssh --timeout=30 --ask-sudo-pass --ask-vault-pass --limit='machine*:&vagrant:!that_one' --inventory-file=#{generated_inventory_dir} --extra-vars=@#{File.expand_path(__FILE__)} --sudo --sudo-user=deployer -vvv --vault-password-file=#{File.expand_path(__FILE__)} --tags=db,www --skip-tags=foo,bar --start-at-task='an awesome task' --why-not --su-user=foot --ask-su-pass --limit='all' --private-key=./myself.key playbook.yml")
expect(full_command).to eq(%Q(PYTHONUNBUFFERED=1 ANSIBLE_FORCE_COLOR=true ANSIBLE_HOST_KEY_CHECKING=true ANSIBLE_SSH_ARGS='-o IdentitiesOnly=yes -i '/my/key1' -i '/my/key2' -o ForwardAgent=yes -o ControlMaster=no -o ControlMaster=auto -o ControlPersist=60s' ansible-playbook --connection=ssh --timeout=30 --ask-sudo-pass --ask-vault-pass --limit="machine*:&vagrant:!that_one" --inventory-file=#{generated_inventory_dir} --extra-vars="{\\"var1\\":\\"string with 'apostrophes', \\\\\\\\, \\\\\\" and =\\",\\"var2\\":{\\"x\\":42}}" --sudo --sudo-user=deployer -vvv --vault-password-file=#{File.expand_path(__FILE__)} --tags=db,www --skip-tags=foo,bar --start-at-task="joe's awesome task" --why-not --su-user=foot --ask-su-pass --limit=all --private-key=./myself.key --extra-vars='{\"var3\":\"foo\"}' playbook.yml))
}
end
end