ansible_local: put json extra-vars in double quotes

Without this change, the JSON string generated from the `extra_vars`
Ruby hash is passed without enclosing quotes and is then not parseable
by the ansible-playbook command when exectuted in a usual shell context.

In this changeset, the ansible (remote) unit test coverage is improved
to cover both usage of `extra_vars` (ansible_local unit tests are still
missing).

Additional Notes:

 - Double quotes are favored to single quotes in order to allow usage of
   any character for the variable values. For this reason additional
   escaping is appended to JSON-inner double quotes and backslashes.

 - This problem was not affecting the `ansible` remote provisioner
   (which is running the ansible-playbook command via the childprocess
   Ruby library). But with this change, the `verbose` output will also
   now be correct for a copy-paste reuse.

 - After this change, all the "--extra-vars" arguments (also a var
   file passed with the @-syntax or anything coming via the
   `raw_arguments` option) are "blindly" and systematically enclosed
   in double quoted and double-escaped.
   This is not optimal and can potentially break with peculiar values
   (e.g. a double quote character (") cannot be used in a json value
   when using `raw_arguments`). That said, I think that the current
   solution is a reasonable trade-off, since the official `extra_vars`
   option should now be able to cover a great majority of use cases.

Fix #6726
This commit is contained in:
Gilles Cornu 2016-03-05 16:59:38 +01:00
parent ffe0ef71ad
commit cb80286a4a
2 changed files with 23 additions and 3 deletions

View File

@ -17,6 +17,8 @@ module VagrantPlugins
command.each do |arg|
if arg =~ /(--start-at-task|--limit)=(.+)/
shell_arg << "#{$1}='#{$2}'"
elsif arg =~ /(--extra-vars)=(.+)/
shell_arg << %Q(%s="%s") % [$1, $2.gsub('\\', '\\\\\\').gsub('"', %Q(\\"))]
else
shell_arg << arg
end

View File

@ -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"
@ -761,7 +779,7 @@ VF
it_should_set_arguments_and_environment_variables 20, 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",
@ -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='an awesome task' --why-not --su-user=foot --ask-su-pass --limit='all' --private-key=./myself.key playbook.yml))
}
end
end