fix(provisioner/ansible): Allow for selection of inventory arg based on the ansible-core version

Author: taru-garg-hashicorp

plugins/provisioners/ansible/provisioner/base.rb

@@ -53,16 +53,25 @@ def initialize(machine, config)
           @gathered_version_stdout = nil
           @gathered_version_major = nil
           @gathered_version = nil
+
+          @ansible_package_version_map = {}
         end
 
         def set_and_check_compatibility_mode
           begin
-            set_gathered_ansible_version(gather_ansible_version)
+            set_gathered_ansible_version(gather_ansible_version("ansible"))
           rescue StandardError => e
             # Nothing to do here, as the fallback on safe compatibility_mode is done below
             @logger.error("Error while gathering the ansible version: #{e.to_s}")
           end
 
+          begin
+            set_gathered_ansible_package_version("ansible-core", gather_ansible_version("ansible-core"))
+          rescue StandardError => e
+             @logger.error("Error while gathering the ansible-core version: #{e.to_s}")
+          end
+          
+
           if @gathered_version_major
             if config.compatibility_mode == Ansible::COMPATIBILITY_MODE_AUTO
               detect_compatibility_mode
@@ -164,7 +173,7 @@ def prepare_common_command_arguments
             @command_arguments << "--limit=#{@machine.name}"
           end
 
-          @command_arguments << "--inventory=#{inventory_path}"
+          @command_arguments << get_inventory_argument(inventory_path)
           @command_arguments << "--extra-vars=#{extra_vars_argument}" if config.extra_vars
           @command_arguments << "--#{@lexicon[:become]}" if config.become
           @command_arguments << "--#{@lexicon[:become_user]}=#{config.become_user}" if config.become_user
@@ -175,6 +184,22 @@ def prepare_common_command_arguments
           @command_arguments << "--start-at-task=#{config.start_at_task}" if config.start_at_task
         end
 
+        def get_inventory_argument(inventory_path)
+          ansible_core_version = @ansible_package_version_map["ansible-core"]
+          # Default to --inventory-file if version info is not available
+          return "--inventory-file=#{inventory_path}" if ansible_core_version.nil? || ansible_core_version.empty?
+          
+          major = ansible_core_version[:major].to_i
+          minor = ansible_core_version[:minor].to_i
+          
+          # Use --inventory for ansible-core >= 2.19
+          if major > 2 || (major == 2 && minor >= 19)
+            "--inventory=#{inventory_path}"
+          else
+            "--inventory-file=#{inventory_path}"
+          end
+        end
+
         def prepare_common_environment_variables
           # Ensure Ansible output isn't buffered so that we receive output
           # on a task-by-task basis.
@@ -399,6 +424,26 @@ def set_gathered_ansible_version(stdout_output)
           end
         end
 
+        def set_gathered_ansible_package_version(ansible_package, stdout_output)
+          if !stdout_output.empty?
+            first_line = stdout_output.lines[0]
+            ansible_version_pattern = first_line.match(/(^#{ansible_package}\s+)(.+)$/)
+            if ansible_version_pattern
+              _, gathered_version, _ = ansible_version_pattern.captures
+              gathered_version.strip!
+              if gathered_version
+                gathered_version_major = gathered_version.match(/(\d+)\..+$/).captures[0].to_i
+                gathered_version_minor = gathered_version.match(/\d+\.(\d+)\..+$/).captures[0].to_i
+                @ansible_package_version_map[ansible_package] = {
+                  version: gathered_version,
+                  major: gathered_version_major,
+                  minor: gathered_version_minor
+                }
+              end
+            end
+          end
+        end
+
       end
     end
   end

plugins/provisioners/ansible/provisioner/guest.rb

@@ -74,11 +74,11 @@ def check_and_install_ansible
           end
         end
 
-        def gather_ansible_version
+        def gather_ansible_version(package = 'ansible')
           raw_output = ""
 
           result = @machine.communicate.execute(
-            "python3 -c \"import importlib.metadata; print('ansible ' + importlib.metadata.version('ansible'))\"",
+            "python3 -c \"import importlib.metadata; print('#{package} ' + importlib.metadata.version('#{package}'))\"",
             error_class: Ansible::Errors::AnsibleNotFoundOnGuest,
             error_key: :ansible_not_found_on_guest) do |type, output|
             if type == :stdout && output.lines[0]

plugins/provisioners/ansible/provisioner/host.rb

@@ -110,10 +110,10 @@ def execute_command_from_host(command)
           end
         end
 
-        def gather_ansible_version
+        def gather_ansible_version(package = 'ansible')
           raw_output = ''
           command = ['python3', '-c',
-                     "import importlib.metadata; print('ansible ' + importlib.metadata.version('ansible'))"]
+                     "import importlib.metadata; print('#{package} ' + importlib.metadata.version('#{package}'))"]
 
           command << {
             notify: [:stdout, :stderr]

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

@@ -85,6 +85,17 @@ def self.it_should_check_ansible_version
     end
   end
 
+  def self.it_should_check_ansible_core_version
+    it "execute 'Python ansible-core version check before executing 'ansible-playbook'" do
+      expect(Vagrant::Util::Subprocess).to receive(:execute)
+        .once.with('python3', '-c', "import importlib.metadata; print('ansible-core ' + importlib.metadata.version('ansible-core'))", { notify: %i[
+                     stdout stderr
+                   ] })
+      expect(Vagrant::Util::Subprocess).to receive(:execute)
+        .once.with('ansible-playbook', any_args)
+    end
+  end
+
   def self.it_should_set_arguments_and_environment_variables(
     expected_args_count = 5,
     expected_vars_count = 4,
@@ -96,7 +107,7 @@ def self.it_should_set_arguments_and_environment_variables(
         expect(args[1]).to eq("--connection=ssh")
         expect(args[2]).to eq("--timeout=30")
 
-        inventory_count = args.count { |x| x.match(/^--inventory=.+$/) if x.is_a?(String) }
+        inventory_count = args.count { |x| x.match(/^--inventory-file=.+$/) if x.is_a?(String) }
         expect(inventory_count).to be > 0
 
         expect(args[args.length-2]).to eq("playbook.yml")
@@ -200,9 +211,9 @@ def self.it_should_create_and_use_generated_inventory(with_user = true)
 
     it "sets as ansible inventory the directory containing the auto-generated inventory file" do
       expect(Vagrant::Util::Subprocess).to receive(:execute).with('ansible-playbook', any_args) { |*args|
-        inventory_index = args.rindex("--inventory=#{generated_inventory_dir}")
+        inventory_index = args.rindex("--inventory-file=#{generated_inventory_dir}")
         expect(inventory_index).to be > 0
-        expect(find_last_argument_after(inventory_index, args, /--inventory=\w+/)).to be(false)
+        expect(find_last_argument_after(inventory_index, args, /--inventory-file=\w+/)).to be(false)
       }.and_return(default_execute_result)
     end
   end
@@ -286,6 +297,7 @@ def ensure_that_config_is_valid
 
     describe "with default options" do
       it_should_check_ansible_version
+      it_should_check_ansible_core_version
       it_should_set_arguments_and_environment_variables
       it_should_create_and_use_generated_inventory
 
@@ -383,6 +395,7 @@ def ensure_that_config_is_valid
       end
 
       it_should_check_ansible_version
+      it_should_check_ansible_core_version
       it_should_create_and_use_generated_inventory
 
       it "doesn't warn about compatibility mode auto-detection" do
@@ -621,7 +634,7 @@ def ensure_that_config_is_valid
                                 "-l localhost",
                                 "--limit=foo",
                                 "--limit=bar",
-                                "--inventory=/forget/it/my/friend",
+                                "--inventory-file=/forget/it/my/friend",
                                 "--user=lion",
                                 "--new-arg=yeah"]
       end
@@ -639,7 +652,7 @@ def ensure_that_config_is_valid
       it "sets raw arguments after arguments related to supported options" do
         expect(Vagrant::Util::Subprocess).to receive(:execute).with('ansible-playbook', any_args) { |*args|
           expect(args.index("--user=lion")).to be > args.index("--user=testuser")
-          expect(args.index("--inventory=/forget/it/my/friend")).to be > args.index("--inventory=#{generated_inventory_dir}")
+          expect(args.index("--inventory-file=/forget/it/my/friend")).to be > args.index("--inventory-file=#{generated_inventory_dir}")
           expect(args.index("--limit=bar")).to be > args.index("--limit=all")
           expect(args.index("--skip-tags=ignored")).to be > args.index("--skip-tags=foo,bar")
         }.and_return(default_execute_result)
@@ -736,8 +749,8 @@ def ensure_that_config_is_valid
 
       it "does not generate the inventory and uses given inventory path instead" do
         expect(Vagrant::Util::Subprocess).to receive(:execute).with('ansible-playbook', any_args) { |*args|
-          expect(args).to include("--inventory=#{existing_file}")
-          expect(args).not_to include("--inventory=#{generated_inventory_file}")
+          expect(args).to include("--inventory-file=#{existing_file}")
+          expect(args).not_to include("--inventory-file=#{generated_inventory_file}")
           expect(File.exist?(generated_inventory_file)).to be(false)
         }.and_return(default_execute_result)
       end
@@ -912,7 +925,7 @@ def ensure_that_config_is_valid
 
           it "shows the ansible-playbook command and set verbosity to '-#{verbose_option}' level" do
             expect(machine.env.ui).to receive(:detail)
-              .with("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=#{generated_inventory_dir} -#{verbose_option} playbook.yml")
+              .with("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
 
@@ -926,7 +939,7 @@ def ensure_that_config_is_valid
 
           it "shows the ansible-playbook command and set verbosity to '-#{verbose_option}' level" do
             expect(machine.env.ui).to receive(:detail)
-              .with("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=#{generated_inventory_dir} -#{verbose_option} playbook.yml")
+              .with("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
       end
@@ -941,7 +954,7 @@ def ensure_that_config_is_valid
 
         it "shows the ansible-playbook command and set verbosity to '-v' level" do
           expect(machine.env.ui).to receive(:detail)
-            .with("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=#{generated_inventory_dir} -v playbook.yml")
+            .with("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
 
@@ -1172,7 +1185,7 @@ def ensure_that_config_is_valid
 
       it "shows the ansible-playbook command, with additional quotes when required" do
         expect(machine.env.ui).to receive(:detail)
-          .with(%Q(PYTHONUNBUFFERED=1 ANSIBLE_FORCE_COLOR=true ANSIBLE_ROLES_PATH='/up/to the stars' ANSIBLE_CONFIG='#{existing_file}' 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=#{generated_inventory_dir} --extra-vars=\\{\\"var1\\":\\"string\\ with\\ \\'apo\\$trophe\\$\\',\\ \\\\\\\\,\\ \\\\\\"\\ and\\ \\=\\",\\"var2\\":\\{\\"x\\":42\\}\\} --sudo --sudo-user=deployer -vvv --vault-password-file=#{existing_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))
+          .with(%Q(PYTHONUNBUFFERED=1 ANSIBLE_FORCE_COLOR=true ANSIBLE_ROLES_PATH='/up/to the stars' ANSIBLE_CONFIG='#{existing_file}' 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=\\{\\"var1\\":\\"string\\ with\\ \\'apo\\$trophe\\$\\',\\ \\\\\\\\,\\ \\\\\\"\\ and\\ \\=\\",\\"var2\\":\\{\\"x\\":42\\}\\} --sudo --sudo-user=deployer -vvv --vault-password-file=#{existing_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
 
@@ -1262,5 +1275,52 @@ def ensure_that_config_is_valid
         }.and_return(default_execute_result)
       end
     end
+
+    describe '#get_inventory_argument' do
+      context 'when ansible version is not detected' do
+        before do
+          config.version = nil
+          allow(subject).to receive(:gather_ansible_version).and_return("ansible #{config.version}\n...\n")
+          allow(subject).to receive(:gather_ansible_version).with("ansible-core").and_return("ansible #{config.version}\n...\n")
+        end
+
+        it 'returns the default inventory command' do 
+          expect(Vagrant::Util::Subprocess).to receive(:execute).with('ansible-playbook', any_args) { |*args|
+            expect(args).to include("--inventory-file=#{generated_inventory_dir}")
+          }.and_return(default_execute_result)
+        end
+      end
+
+      context 'when ansible version is detected' do
+        describe 'version >= 2.19' do
+          before do
+            config.version = "2.19.0"
+            allow(subject).to receive(:gather_ansible_version).with("ansible").and_return("ansible #{config.version}\n...\n")
+            allow(subject).to receive(:gather_ansible_version).with("ansible-core").and_return("ansible-core #{config.version}\n...\n")
+          end
+          
+          it 'returns --inventory as the inventory command' do
+            expect(Vagrant::Util::Subprocess).to receive(:execute).with('ansible-playbook', any_args) { |*args|
+              expect(args).to include("--inventory=#{generated_inventory_dir}")
+            }.and_return(default_execute_result)
+          end
+        end
+
+        describe 'version < 2.19' do
+          before do
+            config.version = "2.18.5"
+            allow(subject).to receive(:gather_ansible_version).with("ansible").and_return("ansible #{config.version}\n...\n")
+            allow(subject).to receive(:gather_ansible_version).with("ansible-core").and_return("ansible-core #{config.version}\n...\n")
+          end
+
+          it 'returns --inventory-file as the inventory command' do
+            expect(Vagrant::Util::Subprocess).to receive(:execute).with('ansible-playbook', any_args) { |*args|
+              expect(args).to include("--inventory-file=#{generated_inventory_dir}")
+            }.and_return(default_execute_result)
+          end
+        end
+      end
+    end
+
   end
 end

test/unit/vagrant/action/builtin/box_add_test.rb

@@ -410,7 +410,7 @@ def with_web_server(path, **opts)
           }.and_return(box)
 
           allow(env[:ui]).to receive(:detail).and_call_original
-          expect(env[:ui]).to receive(:detail).with(%r{.*http://(?!#{username}).+?:(?!#{password}).+?@127\.0\.0\.1:#{port}/#{box_path.basename}.*}).
+          expect(env[:ui]).to receive(:detail).with(/.*http:\/\/\*+(?::\*+)?@127\.0\.0\.1:#{port}\/#{box_path.basename}.*/).
             and_call_original
           expect(app).to receive(:call).with(env)