diff --git a/plugins/provisioners/ansible/config/base.rb b/plugins/provisioners/ansible/config/base.rb index 14ae1a3e6..00b3f5072 100644 --- a/plugins/provisioners/ansible/config/base.rb +++ b/plugins/provisioners/ansible/config/base.rb @@ -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 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 9795ab923..ce189b6d2 100644 --- a/plugins/provisioners/ansible/helpers.rb +++ b/plugins/provisioners/ansible/helpers.rb @@ -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 \ No newline at end of file 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..1eca11d45 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 @@ -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. diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 4929d9a38..9db662f4b 100755 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -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: |- 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 diff --git a/test/unit/plugins/provisioners/ansible/provisioner_test.rb b/test/unit/plugins/provisioners/ansible/provisioner_test.rb index 31354a79c..e662cd959 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 @@ -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