From cb80286a4ac22b9c55ee3d213d980ea73b11cd14 Mon Sep 17 00:00:00 2001
From: Gilles Cornu <git@gilles.cornu.name>
Date: Sat, 5 Mar 2016 16:59:38 +0100
Subject: [PATCH 1/4] 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

From 47c08332773719e913833608f723e52263c0581d Mon Sep 17 00:00:00 2001
From: Gilles Cornu <git@gilles.cornu.name>
Date: Sat, 5 Mar 2016 17:15:28 +0100
Subject: [PATCH 2/4] ansible_local: use double quotes instead of single quotes

Before this minor change, the '--limit' and '--start-at-task'
ansible-playbook command line arguments were enclosed into single
quotes. Using double quotes adds a bit more flexibility, especially
about the task name referred by `start_at_task` option.

It also aligns with the handling of the '--extra-vars' parameter
(see cb80286).
---
 plugins/provisioners/ansible/helpers.rb              |  2 +-
 .../plugins/provisioners/ansible/provisioner_test.rb | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/plugins/provisioners/ansible/helpers.rb b/plugins/provisioners/ansible/helpers.rb
index 9c79891ac..ed6e1d24e 100644
--- a/plugins/provisioners/ansible/helpers.rb
+++ b/plugins/provisioners/ansible/helpers.rb
@@ -16,7 +16,7 @@ module VagrantPlugins
         shell_arg = []
         command.each do |arg|
           if arg =~ /(--start-at-task|--limit)=(.+)/
-            shell_arg << "#{$1}='#{$2}'"
+            shell_arg << %Q(#{$1}="#{$2}")
           elsif arg =~ /(--extra-vars)=(.+)/
             shell_arg << %Q(%s="%s") % [$1, $2.gsub('\\', '\\\\\\').gsub('"', %Q(\\"))]
           else
diff --git a/test/unit/plugins/provisioners/ansible/provisioner_test.rb b/test/unit/plugins/provisioners/ansible/provisioner_test.rb
index f6ee38d7a..4cc0286f8 100644
--- a/test/unit/plugins/provisioners/ansible/provisioner_test.rb
+++ b/test/unit/plugins/provisioners/ansible/provisioner_test.rb
@@ -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
@@ -769,7 +769,7 @@ 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.start_at_task = "joe's awesome task"
         config.raw_arguments = ["--why-not", "--su-user=foot", "--ask-su-pass", "--limit=all", "--private-key=./myself.key"]
 
         # environment variables
@@ -789,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
@@ -804,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(%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))
+          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 playbook.yml))
         }
       end
     end

From ab036ddd0bcb5d844756b48adcaef063a12b5d2f Mon Sep 17 00:00:00 2001
From: Gilles Cornu <git@gilles.cornu.name>
Date: Wed, 20 Apr 2016 18:05:28 +0200
Subject: [PATCH 3/4] provisioners/ansible: don't format raw_arguments

With cb80286a4ac22b9c55ee3d213d980ea73b11cd14, the helper function
stringify_ansible_playbook_command was also applied on the
`raw_arguments` content, which is not wanted. Given that users have used
the `raw_arguments` option as a workaround to avoid the bug GH-6726,
this new change ensure that any `--extra-vars` option passed as a raw
argument won't be additonally enquoted by the ansible_local
provisioner.

This change also improves the ansible remote provisioner verbose output,
but has no impact on its behaviour, which was already correct.

Note that this refactoring introduces some code duplications that are not
very elegant (see ansible_playbook_command_for_shell_execution in
host.rb and execute_ansible_playbook_from_host in base.rb). I hope we
can find a better implementation later, but it is good enough for now
since all these parts are covered by corresponding unit tests (the
`ansible_local` stuff being tested via the verbose output of the ansible
remote provisioner).
---
 plugins/provisioners/ansible/config/base.rb   |  3 ++
 plugins/provisioners/ansible/helpers.rb       | 24 ------------
 .../provisioners/ansible/provisioner/base.rb  | 37 +++++++++++++++++--
 .../provisioners/ansible/provisioner/guest.rb |  3 +-
 .../provisioners/ansible/provisioner/host.rb  | 14 +++++--
 .../provisioners/ansible/provisioner_test.rb  |  6 +--
 6 files changed, 50 insertions(+), 37 deletions(-)

diff --git a/plugins/provisioners/ansible/config/base.rb b/plugins/provisioners/ansible/config/base.rb
index 14ae1a3e6..d6881c70c 100644
--- a/plugins/provisioners/ansible/config/base.rb
+++ b/plugins/provisioners/ansible/config/base.rb
@@ -112,6 +112,9 @@ module VagrantPlugins
             end
           end
 
+          if raw_arguments
+            @raw_arguments = Helpers::as_array(raw_arguments)
+          end
         end
       end
     end
diff --git a/plugins/provisioners/ansible/helpers.rb b/plugins/provisioners/ansible/helpers.rb
index ed6e1d24e..24534c260 100644
--- a/plugins/provisioners/ansible/helpers.rb
+++ b/plugins/provisioners/ansible/helpers.rb
@@ -3,30 +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 << %Q(#{$1}="#{$2}")
-          elsif arg =~ /(--extra-vars)=(.+)/
-            shell_arg << %Q(%s="%s") % [$1, $2.gsub('\\', '\\\\\\').gsub('"', %Q(\\"))]
-          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
diff --git a/plugins/provisioners/ansible/provisioner/base.rb b/plugins/provisioners/ansible/provisioner/base.rb
index 813f96a3e..6092ecfe3 100644
--- a/plugins/provisioners/ansible/provisioner/base.rb
+++ b/plugins/provisioners/ansible/provisioner/base.rb
@@ -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
diff --git a/plugins/provisioners/ansible/provisioner/guest.rb b/plugins/provisioners/ansible/provisioner/guest.rb
index 741b07301..a6dda8131 100644
--- a/plugins/provisioners/ansible/provisioner/guest.rb
+++ b/plugins/provisioners/ansible/provisioner/guest.rb
@@ -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
diff --git a/plugins/provisioners/ansible/provisioner/host.rb b/plugins/provisioners/ansible/provisioner/host.rb
index 245a00499..2ff957bc8 100644
--- a/plugins/provisioners/ansible/provisioner/host.rb
+++ b/plugins/provisioners/ansible/provisioner/host.rb
@@ -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
 
diff --git a/test/unit/plugins/provisioners/ansible/provisioner_test.rb b/test/unit/plugins/provisioners/ansible/provisioner_test.rb
index 4cc0286f8..e662cd959 100644
--- a/test/unit/plugins/provisioners/ansible/provisioner_test.rb
+++ b/test/unit/plugins/provisioners/ansible/provisioner_test.rb
@@ -770,14 +770,14 @@ VF
         config.skip_tags = %w(foo bar)
         config.limit = 'machine*:&vagrant:!that_one'
         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"]
+        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={\"var1\":\"string with 'apostrophes', \\\\, \\\" and =\",\"var2\":{\"x\":42}}",
                                           "sudo"                => "--sudo",
@@ -804,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(%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 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

From b2286388f085135cfdd4a91f4a6d0c2676875050 Mon Sep 17 00:00:00 2001
From: Gilles Cornu <git@gilles.cornu.name>
Date: Wed, 20 Apr 2016 22:27:55 +0200
Subject: [PATCH 4/4] provisioners/ansible: add basic config validators

With this change, the `raw_arguments` and `raw_ssh_args` options are:
- STILL automatically converted as an Array when they are set a String
  (no behaviour change)
- rejected if they are not of Array data type otherwise

Additional Notes:
- the 'as_array' tiny helper has been removed since it was no longer
  used.
- there is for now no deeper validation (i.e. verifying that the Array
  elements are only *String* objects)
---
 plugins/provisioners/ansible/config/base.rb   | 10 ++++-
 plugins/provisioners/ansible/config/host.rb   | 11 +++++
 plugins/provisioners/ansible/helpers.rb       |  4 --
 .../provisioners/ansible/provisioner/host.rb  |  2 +-
 templates/locales/en.yml                      |  4 ++
 .../provisioners/ansible/config_test.rb       | 40 +++++++++++++++++++
 6 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/plugins/provisioners/ansible/config/base.rb b/plugins/provisioners/ansible/config/base.rb
index d6881c70c..00b3f5072 100644
--- a/plugins/provisioners/ansible/config/base.rb
+++ b/plugins/provisioners/ansible/config/base.rb
@@ -113,8 +113,16 @@ module VagrantPlugins
           end
 
           if raw_arguments
-            @raw_arguments = Helpers::as_array(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
diff --git a/plugins/provisioners/ansible/config/host.rb b/plugins/provisioners/ansible/config/host.rb
index 8f9433c63..04d2fac10 100644
--- a/plugins/provisioners/ansible/config/host.rb
+++ b/plugins/provisioners/ansible/config/host.rb
@@ -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
 
diff --git a/plugins/provisioners/ansible/helpers.rb b/plugins/provisioners/ansible/helpers.rb
index 24534c260..ce189b6d2 100644
--- a/plugins/provisioners/ansible/helpers.rb
+++ b/plugins/provisioners/ansible/helpers.rb
@@ -12,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
\ No newline at end of file
diff --git a/plugins/provisioners/ansible/provisioner/host.rb b/plugins/provisioners/ansible/provisioner/host.rb
index 2ff957bc8..1eca11d45 100644
--- a/plugins/provisioners/ansible/provisioner/host.rb
+++ b/plugins/provisioners/ansible/provisioner/host.rb
@@ -243,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.
diff --git a/templates/locales/en.yml b/templates/locales/en.yml
index 3ca78008d..5fd667f48 100755
--- a/templates/locales/en.yml
+++ b/templates/locales/en.yml
@@ -2121,6 +2121,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: |-
diff --git a/test/unit/plugins/provisioners/ansible/config_test.rb b/test/unit/plugins/provisioners/ansible/config_test.rb
index f836382a3..5ba9d638d 100644
--- a/test/unit/plugins/provisioners/ansible/config_test.rb
+++ b/test/unit/plugins/provisioners/ansible/config_test.rb
@@ -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