Merge branch 'gildegoma/ansible-force-ssh-user'

Resolved conflicts in
  plugins/provisioners/ansible/config.rb
  plugins/provisioners/ansible/provisioner.rb
  test/unit/plugins/provisioners/ansible/provisioner_test.rb
  website/docs/source/v2/provisioning/ansible.html.md

ref #6348

CHANGELOG.md

@@ -9,8 +9,22 @@ FEATURES:
   - **IPv6 Private Networks**: Private networking now supports IPv6. This
     only works with VirtualBox and VMware at this point. [GH-6342]
 
+BREAKING CHANGES:
+
+  - the `ansible` provisioner now can override the effective ansible remote user
+    (i.e. `ansible_ssh_user` setting) to always correspond to the vagrant ssh
+    username. This change is enabled by default, but we expect this to affect
+    only a tiny number of people as it corresponds to the common usage.
+    If you however use different remote usernames in your Ansible plays, tasks,
+    or custom inventories, you can simply set the option `force_remote_user` to
+    false to make Vagrant behave the same as before.
+
+
 IMPROVEMENTS:
 
+  - provisioners/ansible: add new `force_remote_user` option to control whether
+    `ansible_ssh_user` parameter should be applied or not [GH-6348]
+
 BUG FIXES:
 
   - communicator/winrm: respect `boot_timeout` setting [GH-6229]

plugins/provisioners/ansible/config/host.rb

@@ -7,6 +7,7 @@ module VagrantPlugins
 
         attr_accessor :ask_sudo_pass
         attr_accessor :ask_vault_pass
+        attr_accessor :force_remote_user
         attr_accessor :host_key_checking
         attr_accessor :raw_ssh_args
 
@@ -15,6 +16,7 @@ module VagrantPlugins
 
           @ask_sudo_pass       = false
           @ask_vault_pass      = false
+          @force_remote_user   = true
           @host_key_checking   = false
           @raw_ssh_args        = UNSET_VALUE
         end
@@ -24,6 +26,7 @@ module VagrantPlugins
 
           @ask_sudo_pass       = false if @ask_sudo_pass     != true
           @ask_vault_pass      = false if @ask_vault_pass    != true
+          @force_remote_user   = true  if @force_remote_user != false
           @host_key_checking   = false if @host_key_checking != true
           @raw_ssh_args        = nil   if @raw_ssh_args      == UNSET_VALUE
         end

plugins/provisioners/ansible/provisioner/host.rb

@@ -33,9 +33,6 @@ module VagrantPlugins
         end
 
         def prepare_command_arguments
-          # By default, connect with Vagrant SSH username
-          @command_arguments << "--user=#{@ssh_info[:username]}"
-
           # Connect with native OpenSSH client
           # Other modes (e.g. paramiko) are not officially supported,
           # but can be enabled via raw_arguments option.
@@ -47,6 +44,16 @@ module VagrantPlugins
           # is not controlled during vagrant boot process.
           @command_arguments << "--timeout=30"
 
+          if !config.force_remote_user
+            # Pass the vagrant ssh username as Ansible default remote user, because
+            # the ansible_ssh_user parameter won't be added to the auto-generated inventory.
+            @command_arguments << "--user=#{@ssh_info[:username]}"
+          elsif config.inventory_path
+            # Using an extra variable is the only way to ensure that the Ansible remote user
+            # is overridden (as the ansible inventory is not under vagrant control)
+            @command_arguments << "--extra-vars=ansible_ssh_user='#{@ssh_info[:username]}'"
+          end
+
           @command_arguments << "--ask-sudo-pass" if config.ask_sudo_pass
           @command_arguments << "--ask-vault-pass" if config.ask_vault_pass
 
@@ -118,7 +125,11 @@ module VagrantPlugins
               m = @machine.env.machine(*am)
               m_ssh_info = m.ssh_info
               if !m_ssh_info.nil?
-                machines += "#{m.name} ansible_ssh_host=#{m_ssh_info[:host]} ansible_ssh_port=#{m_ssh_info[:port]} ansible_ssh_private_key_file='#{m_ssh_info[:private_key_path][0]}'\n"
+                forced_ssh_user = ""
+                if config.force_remote_user
+                  forced_ssh_user = "ansible_ssh_user='#{m_ssh_info[:username]}' "
+                end
+                machines += "#{m.name} ansible_ssh_host=#{m_ssh_info[:host]} ansible_ssh_port=#{m_ssh_info[:port]} #{forced_ssh_user}ansible_ssh_private_key_file='#{m_ssh_info[:private_key_path][0]}'\n"
                 @inventory_machines[m.name] = m
               else
                 @logger.error("Auto-generated inventory: Impossible to get SSH information for machine '#{m.name} (#{m.provider_name})'. This machine should be recreated.")

test/unit/plugins/provisioners/ansible/config_test.rb

@@ -19,6 +19,7 @@ describe VagrantPlugins::Ansible::Config::Host do
     supported_options = %w( ask_sudo_pass
                             ask_vault_pass
                             extra_vars
+                            force_remote_user
                             groups
                             host_key_checking
                             inventory_path
@@ -42,6 +43,7 @@ describe VagrantPlugins::Ansible::Config::Host do
 
     expect(subject.playbook).to be_nil
     expect(subject.extra_vars).to be_nil
+    expect(subject.force_remote_user).to be_true
     expect(subject.ask_sudo_pass).to be_false
     expect(subject.ask_vault_pass).to be_false
     expect(subject.vault_password_file).to be_nil
@@ -58,6 +60,9 @@ describe VagrantPlugins::Ansible::Config::Host do
     expect(subject.raw_ssh_args).to be_nil
   end
 
+  describe "force_remote_user option" do
+    it_behaves_like "any VagrantConfigProvisioner strict boolean attribute", :force_remote_user, true
+  end
   describe "host_key_checking option" do
     it_behaves_like "any VagrantConfigProvisioner strict boolean attribute", :host_key_checking, false
   end

test/unit/plugins/provisioners/ansible/provisioner_test.rb

@@ -67,15 +67,17 @@ VF
   #
 
   def self.it_should_set_arguments_and_environment_variables(
-    expected_args_count = 6, expected_vars_count = 4, expected_host_key_checking = false, expected_transport_mode = "ssh")
+    expected_args_count = 5,
+    expected_vars_count = 4,
+    expected_host_key_checking = false,
+    expected_transport_mode = "ssh")
 
     it "sets implicit arguments in a specific order" do
       expect(Vagrant::Util::Subprocess).to receive(:execute).with { |*args|
 
         expect(args[0]).to eq("ansible-playbook")
-        expect(args[1]).to eq("--user=#{machine.ssh_info[:username]}")
+        expect(args[1]).to eq("--connection=ssh")
-        expect(args[2]).to eq("--connection=ssh")
+        expect(args[2]).to eq("--timeout=30")
-        expect(args[3]).to eq("--timeout=30")
 
         inventory_count = args.count { |x| x =~ /^--inventory-file=.+$/ }
         expect(inventory_count).to be > 0
@@ -162,13 +164,17 @@ VF
     end
   end
 
-  def self.it_should_create_and_use_generated_inventory
+  def self.it_should_create_and_use_generated_inventory(with_ssh_user = true)
     it "generates an inventory with all active machines" do
       expect(Vagrant::Util::Subprocess).to receive(:execute).with { |*args|
         expect(config.inventory_path).to be_nil
         expect(File.exists?(generated_inventory_file)).to be_true
         inventory_content = File.read(generated_inventory_file)
+        if with_ssh_user
+          expect(inventory_content).to include("#{machine.name} ansible_ssh_host=#{machine.ssh_info[:host]} ansible_ssh_port=#{machine.ssh_info[:port]} ansible_ssh_user='#{machine.ssh_info[:username]}' ansible_ssh_private_key_file='#{machine.ssh_info[:private_key_path][0]}'\n")
+        else
           expect(inventory_content).to include("#{machine.name} ansible_ssh_host=#{machine.ssh_info[:host]} ansible_ssh_port=#{machine.ssh_info[:port]} ansible_ssh_private_key_file='#{machine.ssh_info[:private_key_path][0]}'\n")
+        end
         expect(inventory_content).to include("# MISSING: '#{iso_env.machine_names[1]}' machine was probably removed without using Vagrant. This machine should be recreated.\n")
       }
     end
@@ -273,7 +279,7 @@ VF
         config.host_key_checking = true
       end
 
-      it_should_set_arguments_and_environment_variables 6, 4, true
+      it_should_set_arguments_and_environment_variables 5, 4, true
     end
 
     describe "with boolean (flag) options disabled" do
@@ -285,7 +291,7 @@ VF
         config.sudo_user = 'root'
       end
 
-      it_should_set_arguments_and_environment_variables 7
+      it_should_set_arguments_and_environment_variables 6
       it_should_set_optional_arguments({ "sudo_user" => "--sudo-user=root" })
 
       it "it does not set boolean flag when corresponding option is set to false" do
@@ -300,6 +306,7 @@ VF
     describe "with raw_arguments option" do
       before do
         config.sudo = false
+        config.force_remote_user = false
         config.skip_tags = %w(foo bar)
         config.limit = "all"
         config.raw_arguments = ["--connection=paramiko",
@@ -349,12 +356,29 @@ VF
       it_should_set_arguments_and_environment_variables
     end
 
+    context "with force_remote_user option disabled" do
+      before do
+        config.force_remote_user = false
+      end
+
+      it_should_create_and_use_generated_inventory false # i.e. without setting ansible_ssh_user in inventory
+
+      it_should_set_arguments_and_environment_variables 6
+
+      it "uses a --user argument to set a default remote user" do
+        expect(Vagrant::Util::Subprocess).to receive(:execute).with { |*args|
+          expect(args).not_to include("--extra-vars=ansible_ssh_user='#{machine.ssh_info[:username]}'")
+          expect(args).to include("--user=#{machine.ssh_info[:username]}")
+        }
+      end
+    end
+
     describe "with inventory_path option" do
       before do
         config.inventory_path = existing_file
       end
 
-      it_should_set_arguments_and_environment_variables
+      it_should_set_arguments_and_environment_variables 6
 
       it "does not generate the inventory and uses given inventory path instead" do
         expect(Vagrant::Util::Subprocess).to receive(:execute).with { |*args|
@@ -363,6 +387,26 @@ VF
           expect(File.exists?(generated_inventory_file)).to be_false
         }
       end
+
+      it "uses an --extra-vars argument to force ansible_ssh_user parameter" do
+        expect(Vagrant::Util::Subprocess).to receive(:execute).with { |*args|
+          expect(args).not_to include("--user=#{machine.ssh_info[:username]}")
+          expect(args).to include("--extra-vars=ansible_ssh_user='#{machine.ssh_info[:username]}'")
+        }
+      end
+
+      describe "with force_remote_user option disabled" do
+        before do
+          config.force_remote_user = false
+        end
+
+        it "uses a --user argument to set a default remote user" do
+          expect(Vagrant::Util::Subprocess).to receive(:execute).with { |*args|
+            expect(args).not_to include("--extra-vars=ansible_ssh_user='#{machine.ssh_info[:username]}'")
+            expect(args).to include("--user=#{machine.ssh_info[:username]}")
+          }
+        end
+      end
     end
 
     describe "with ask_vault_pass option" do
@@ -370,7 +414,7 @@ VF
         config.ask_vault_pass = true
       end
 
-      it_should_set_arguments_and_environment_variables 7
+      it_should_set_arguments_and_environment_variables 6
 
       it "should ask the vault password" do
         expect(Vagrant::Util::Subprocess).to receive(:execute).with { |*args|
@@ -384,7 +428,7 @@ VF
         config.vault_password_file = existing_file
       end
 
-      it_should_set_arguments_and_environment_variables 7
+      it_should_set_arguments_and_environment_variables 6
 
       it "uses the given vault password file" do
         expect(Vagrant::Util::Subprocess).to receive(:execute).with { |*args|
@@ -398,7 +442,7 @@ VF
         config.raw_ssh_args = ['-o ControlMaster=no', '-o ForwardAgent=no']
       end
 
-      it_should_set_arguments_and_environment_variables 6, 4
+      it_should_set_arguments_and_environment_variables
       it_should_explicitly_enable_ansible_ssh_control_persist_defaults
 
       it "passes custom SSH options via ANSIBLE_SSH_ARGS with the highest priority" do
@@ -432,7 +476,7 @@ VF
         ssh_info[:private_key_path] = ['/path/to/my/key', '/an/other/identity', '/yet/an/other/key']
       end
 
-      it_should_set_arguments_and_environment_variables 6, 4
+      it_should_set_arguments_and_environment_variables
       it_should_explicitly_enable_ansible_ssh_control_persist_defaults
 
       it "passes additional Identity Files via ANSIBLE_SSH_ARGS" do
@@ -449,7 +493,7 @@ VF
         ssh_info[:forward_agent] = true
       end
 
-      it_should_set_arguments_and_environment_variables 6, 4
+      it_should_set_arguments_and_environment_variables
       it_should_explicitly_enable_ansible_ssh_control_persist_defaults
 
       it "enables SSH-Forwarding via ANSIBLE_SSH_ARGS" do
@@ -468,12 +512,12 @@ VF
             config.verbose = verbose_option
           end
 
-          it_should_set_arguments_and_environment_variables 7
+          it_should_set_arguments_and_environment_variables 6
           it_should_set_optional_arguments({ "verbose" => "-#{verbose_option}" })
 
           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 --user=testuser --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
@@ -483,12 +527,12 @@ VF
             config.verbose = "-#{verbose_option}"
           end
 
-          it_should_set_arguments_and_environment_variables 7
+          it_should_set_arguments_and_environment_variables 6
           it_should_set_optional_arguments({ "verbose" => "-#{verbose_option}" })
 
           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 --user=testuser --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
@@ -499,12 +543,12 @@ VF
           config.verbose = "wrong"
         end
 
-        it_should_set_arguments_and_environment_variables 7
+        it_should_set_arguments_and_environment_variables 6
         it_should_set_optional_arguments({ "verbose" => "-v" })
 
         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 --user=testuser --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
@@ -566,7 +610,7 @@ VF
         config.raw_ssh_args = ['-o ControlMaster=no']
       end
 
-      it_should_set_arguments_and_environment_variables 21, 4, true
+      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__)}",
                                           "sudo"                => "--sudo",
@@ -593,7 +637,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 -o IdentityFile=/my/key1 -o IdentityFile=/my/key2 -o ForwardAgent=yes -o ControlMaster=no -o ControlMaster=auto -o ControlPersist=60s' ansible-playbook --user=testuser --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("PYTHONUNBUFFERED=1 ANSIBLE_FORCE_COLOR=true ANSIBLE_HOST_KEY_CHECKING=true ANSIBLE_SSH_ARGS='-o IdentitiesOnly=yes -o IdentityFile=/my/key1 -o IdentityFile=/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")
         }
       end
     end

website/docs/source/v2/provisioning/ansible.html.md

@@ -59,6 +59,14 @@ This section lists the specific options for the Ansible (remote) provisioner. In
 
   The default value is `false`.
 
+- `force_remote_user` (boolean) - require Vagrant to set the `ansible_ssh_user` setting in the generated inventory, or as an extra variable when a static inventory is used. All the Ansible `remote_user` parameters will then be overridden by the value of `config.ssh.username` of the [Vagrant SSH Settings](/v2/vagrantfile/ssh_settings.html).
+
+  If this option is set to `false` Vagrant will set the Vagrant SSH username as a default Ansible remote user, but `remote_user` parameters of your Ansible plays or tasks will still be taken into account and thus override the Vagrant configuration.
+
+  The default value is `true`.
+
+  **Note:** This option was introduced in Vagrant 1.8.0. Previous Vagrant versions behave like if this option was set to `false`.
+
 - `host_key_checking` (boolean) - require Ansible to [enable SSH host key checking](http://docs.ansible.com/intro_getting_started.html#host-key-checking).
 
   The default value is `false`.
@@ -113,31 +121,6 @@ end
 
 If you apply this parallel provisioning pattern with a static Ansible inventory, you'll have to organize the things so that [all the relevant private keys are provided to the `ansible-playbook` command](https://github.com/mitchellh/vagrant/pull/5765#issuecomment-120247738). The same kind of considerations applies if you are using multiple private keys for a same machine (see [`config.ssh.private_key_path` SSH setting](/v2/vagrantfile/ssh_settings.html)).
 
-### Troubleshooting SSH Connection Errors
-
-It is good to know that the following Ansible settings always override the `config.ssh.username` option defined in [Vagrant SSH Settings](/v2/vagrantfile/ssh_settings.html):
-
-* `ansible_ssh_user` variable
-* `remote_user` (or `user`) play attribute
-* `remote_user` task attribute
-
-Be aware that copying snippets from the Ansible documentation might lead to this problem, as `root` is used as the remote user in many [examples](http://docs.ansible.com/playbooks_intro.html#hosts-and-users).
-
-Example of an SSH error (with `vvv` log level), where an undefined remote user `xyz` has replaced `vagrant`:
-
-```
-TASK: [my_role | do something] *****************
-<127.0.0.1> ESTABLISH CONNECTION FOR USER: xyz
-<127.0.0.1> EXEC ['ssh', '-tt', '-vvv', '-o', 'ControlMaster=auto',...
-fatal: [ansible-devbox] => SSH encountered an unknown error. We recommend you re-run the command using -vvvv, which will enable SSH debugging output to help diagnose the issue.
-```
-
-In a situation like the above, to override the `remote_user` specified in a play you can use the following line in your Vagrantfile `vm.provision` block:
-
-```
-ansible.extra_vars = { ansible_ssh_user: 'vagrant' }
-```
-
 ### Force Paramiko Connection Mode
 
 The Ansible provisioner is implemented with native OpenSSH support in mind, and there is no official support for [paramiko](https://github.com/paramiko/paramiko/) (A native Python SSHv2 protocol library).

website/docs/source/v2/provisioning/ansible_intro.html.md

@@ -91,7 +91,7 @@ The first and simplest option is to not provide one to Vagrant at all. Vagrant w
 ```
 # Generated by Vagrant
 
-default ansible_ssh_host=127.0.0.1 ansible_ssh_port=2200 ansible_ssh_private_key_file='/home/.../.vagrant/machines/default/virtualbox/private_key'
+default ansible_ssh_host=127.0.0.1 ansible_ssh_port=2200 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/default/virtualbox/private_key'
 ```
 
 Note that the generated inventory file is stored as part of your local Vagrant environment in
@@ -137,8 +137,8 @@ Vagrant would generate an inventory file that might look like:
 ```
 # Generated by Vagrant
 
-machine1 ansible_ssh_host=127.0.0.1 ansible_ssh_port=2200 ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine1/virtualbox/private_key'
+machine1 ansible_ssh_host=127.0.0.1 ansible_ssh_port=2200 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine1/virtualbox/private_key'
-machine2 ansible_ssh_host=127.0.0.1 ansible_ssh_port=2222 ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine2/virtualbox/private_key'
+machine2 ansible_ssh_host=127.0.0.1 ansible_ssh_port=2222 ansible_ssh_user='vagrant' ansible_ssh_private_key_file='/home/.../.vagrant/machines/machine2/virtualbox/private_key'
 
 [group1]
 machine1