provisioners/ansible: don't format raw_arguments

With cb80286a4a, 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

@@ -112,6 +112,9 @@ module VagrantPlugins
             end
           end
 
+          if raw_arguments
+            @raw_arguments = Helpers::as_array(raw_arguments)
+          end
         end
       end
     end

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

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

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

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
 

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