From cb80286a4ac22b9c55ee3d213d980ea73b11cd14 Mon Sep 17 00:00:00 2001 From: Gilles Cornu Date: Sat, 5 Mar 2016 16:59:38 +0100 Subject: [PATCH] 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 --- plugins/provisioners/ansible/helpers.rb | 2 ++ .../provisioners/ansible/provisioner_test.rb | 24 ++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/plugins/provisioners/ansible/helpers.rb b/plugins/provisioners/ansible/helpers.rb index 9795ab923..9c79891ac 100644 --- a/plugins/provisioners/ansible/helpers.rb +++ b/plugins/provisioners/ansible/helpers.rb @@ -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 diff --git a/test/unit/plugins/provisioners/ansible/provisioner_test.rb b/test/unit/plugins/provisioners/ansible/provisioner_test.rb index 31354a79c..f6ee38d7a 100644 --- a/test/unit/plugins/provisioners/ansible/provisioner_test.rb +++ b/test/unit/plugins/provisioners/ansible/provisioner_test.rb @@ -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