Merge pull request #605 from david22swan/GH-585--CONT-998/main/unsafe…

…_directoryfix

(GH-585/CONT-998) Fix for safe_directory logic

README.md

@@ -864,6 +864,8 @@ For example, setting the `owner` parameter on a resource would cause Puppet runs
 Impacted users are now advised to use the new `safe_directory` parameter on Git resources.
 Explicitily setting the value to `true` will add the current path specified on the resource to the `safe.directory` git configuration for the current user (global scope) allowing the Puppet run to continue without error.
 
+Safe directory configuration will be stored within the system wide configuration file `/etc/gitconfig`.
+
 <a id="development"></a> 
 ## Development
 

lib/puppet/provider/vcsrepo/git.rb

@@ -591,7 +591,7 @@ def git_version
 
   # @!visibility private
   def safe_directories
-    args = ['config', '--global', '--get-all', 'safe.directory']
+    args = ['config', '--system', '--get-all', 'safe.directory']
     begin
       d = git_with_identity(*args) || ''
       d.split('\n')
@@ -609,15 +609,15 @@ def update_safe_directory
 
     if should_add_safe_directory?
       add_safe_directory
-    else
+    elsif should_remove_safe_directory?
       remove_safe_directory
     end
   end
 
   # @!visibility private
   def add_safe_directory
     notice("Adding '#{@resource.value(:path)}' to safe directory list")
-    args = ['config', '--global', '--add', 'safe.directory', @resource.value(:path)]
+    args = ['config', '--system', '--add', 'safe.directory', @resource.value(:path)]
     git_with_identity(*args)
   end
 
@@ -626,7 +626,7 @@ def remove_safe_directory
     return unless safe_directories.include?(@resource.value(:path))
 
     notice("Removing '#{@resource.value(:path)}' from safe directory list")
-    args = ['config', '--global', '--unset', 'safe.directory', @resource.value(:path)]
+    args = ['config', '--system', '--unset', 'safe.directory', @resource.value(:path)]
     git_with_identity(*args)
   end
 
@@ -637,6 +637,12 @@ def should_add_safe_directory?
       !safe_directories.include?(@resource.value(:path)) # directory should not already be in the list
   end
 
+  # @!visibility private
+  def should_remove_safe_directory?
+    !@resource.value(:safe_directory) && # safe_directory should be false
+      safe_directories.include?(@resource.value(:path)) # directory should be in the list
+  end
+
   # @!visibility private
   def git_remote_action(*args)
     proxy = @resource.value(:http_proxy)

spec/acceptance/clone_repo_spec.rb

@@ -238,6 +238,12 @@
       it { is_expected.to be_directory }
       it { is_expected.to be_owned_by 'vagrant' }
     end
+
+    describe file('/etc/gitconfig') do
+      subject { super().content }
+
+      it { is_expected.to match %r{directory = /tmp/vcsrepo/testrepo_owner} }
+    end
   end
 
   context 'with with a group' do

spec/unit/puppet/provider/vcsrepo/git_spec.rb

@@ -253,7 +253,7 @@ def branch_a_list(include_branch = nil?)
       expect(provider).to receive(:path_exists?).and_return(true)
       expect(provider).to receive(:path_empty?).and_return(false)
       provider.destroy
-      expect(provider).to receive(:exec_git).with('config', '--global', '--get-all', 'safe.directory')
+      expect(provider).to receive(:exec_git).with('config', '--system', '--get-all', 'safe.directory')
       expect(provider).to receive(:exec_git).with('clone', resource.value(:source), resource.value(:path))
       expect(provider).to receive(:update_submodules)
       expect(provider).to receive(:update_remote_url).with('origin', resource.value(:source)).and_return false