Add a mode parameter to manage repository root directory permission

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org).
 
+## UNRELEASED
+
+### Added
+
+- support for directory modes for repos [\#599](https://github.com/puppetlabs/puppetlabs-vcsrepo/issues/599) ([robbat2](https://github.com/robbat2))
+
 ## [v6.1.0](https://github.com/puppetlabs/puppetlabs-vcsrepo/tree/v6.1.0) - 2023-06-13
 
 [Full Changelog](https://github.com/puppetlabs/puppetlabs-vcsrepo/compare/v6.0.1...v6.1.0)

README.md

@@ -797,37 +797,37 @@ For information on the classes and types, see the [REFERENCE.md](https://github.
 
 Features: `bare_repositories`, `depth`, `multiple_remotes`, `reference_tracking`, `ssh_identity`, `submodules`, `user`
 
-Parameters: `depth`, `ensure`, `excludes`, `force`, `group`, `identity`, `owner`, `path`, `provider`, `remote`, `revision`, `source`, `user`
+Parameters: `depth`, `ensure`, `excludes`, `force`, `group`, `identity`, `owner`, `path`, `provider`, `remote`, `revision`, `source`, `user`, `mode`
 
 ##### `bzr` - Supports the Bazaar VCS.
 
 Features: `reference_tracking`
 
-Parameters: `ensure`, `excludes`, `force`, `group`, `owner`, `path`, `provider`, `revision`, `source`
+Parameters: `ensure`, `excludes`, `force`, `group`, `owner`, `path`, `provider`, `revision`, `source`, `mode`
 
 ##### `cvs` - Supports the CVS VCS.
 
 Features: `cvs_rsh`, `gzip_compression`, `modules`, `reference_tracking`, `user`
 
-Parameters: `compression`, `cvs_rsh`, `ensure`, `excludes`, `force`, `group`, `module`, `owner`, `path`, `provider`
+Parameters: `compression`, `cvs_rsh`, `ensure`, `excludes`, `force`, `group`, `module`, `owner`, `path`, `provider`, `mode`
 
 ##### `hg` - Supports the Mercurial VCS.
 
 Features: `reference_tracking`, `ssh_identity`, `user`
 
-Parameters: `ensure`, `excludes`, `force`, `group`, `identity`, `owner`, `path`, `provider`, `revision`, `source`, `user`
+Parameters: `ensure`, `excludes`, `force`, `group`, `identity`, `owner`, `path`, `provider`, `revision`, `source`, `user`, `mode`
 
 ##### `p4` - Supports the Perforce VCS.
 
 Features: `p4config`, `reference_tracking`
 
-Parameters: `ensure`, `excludes`, `force`, `group`, `owner`, `p4config`, `path`, `provider`, `revision`, `source`
+Parameters: `ensure`, `excludes`, `force`, `group`, `owner`, `p4config`, `path`, `provider`, `revision`, `source`, `mode`
 
 ##### `svn` - Supports the Subversion VCS.
 
 Features: `basic_auth`, `configuration`, `conflict`, `depth`, `filesystem_types`, `reference_tracking`
 
-Parameters: `basic_auth_password`, `basic_auth_username`, `configuration`, `conflict`, `ensure`, `excludes`, `force`, `fstype`, `group`, `includes`, `owner`, `path`, `provider`, `revision`, `source`, `trust_server_cert`
+Parameters: `basic_auth_password`, `basic_auth_username`, `configuration`, `conflict`, `ensure`, `excludes`, `force`, `fstype`, `group`, `includes`, `owner`, `path`, `provider`, `revision`, `source`, `trust_server_cert`, `mode`
 
 <a id="features"></a> 
 #### Features

REFERENCE.md

@@ -154,6 +154,7 @@ The following parameters are available in the `vcsrepo` type.
 * [`trust_server_cert`](#trust_server_cert)
 * [`umask`](#umask)
 * [`user`](#user)
+* [`mode`](#mode)
 
 ##### <a name="basic_auth_password"></a>`basic_auth_password`
 
@@ -280,3 +281,6 @@ Sets the umask to be used for all repo operations
 
 The user to run for repository operations
 
+##### <a name="mode"></a>`mode`
+
+Sets the mode for the repository directory (non-recursive).

lib/puppet/provider/vcsrepo.rb

@@ -16,10 +16,25 @@ def check_force
 
   private
 
+  def set_ownership_and_permissions
+    set_ownership
+    set_permissions
+  end
+
   def set_ownership
+    mode = @resource.value(:mode) || nil
+    # Change the permission on the repo itself.
+    # The VCS should maintain permissions on files within the checkout.
+    FileUtils.chmod(mode, @resource.value(:path)) unless mode.nil?
+  end
+
+  def set_permissions
     owner = @resource.value(:owner) || nil
     group = @resource.value(:group) || nil
     excludes = @resource.value(:excludes) || nil
+    # We might have no work to do, and this makes it easier for the callers.
+    return if owner.nil? && group.nil?
+
     if excludes.nil? || excludes.empty?
       FileUtils.chown_R(owner, group, @resource.value(:path))
     else
@@ -28,6 +43,7 @@ def set_ownership
   end
 
   def files
+    # Expensive on large repos
     excludes = @resource.value(:excludes)
     path = @resource.value(:path)
     Dir["#{path}/**/*"].reject { |f| excludes.any? { |p| f.start_with?("#{path}/#{p}") } }

lib/puppet/provider/vcsrepo/bzr.rb

@@ -102,6 +102,6 @@ def clone_repository(revision)
   end
 
   def update_owner
-    set_ownership if @resource.value(:owner) || @resource.value(:group)
+    set_ownership_and_permissions
   end
 end

lib/puppet/provider/vcsrepo/cvs.rb

@@ -135,7 +135,7 @@ def create_repository(path)
   end
 
   def update_owner
-    set_ownership if @resource.value(:owner) || @resource.value(:group)
+    set_ownership_and_permissions
   end
 
   def runcvs(*args)

lib/puppet/provider/vcsrepo/git.rb

@@ -28,7 +28,7 @@ def create
       init_repository
       self.skip_hooks = @resource.value(:skip_hooks) unless @resource.value(:skip_hooks).nil?
     end
-    update_owner_and_excludes
+    update_owner_permission_and_excludes
   end
 
   def destroy
@@ -88,7 +88,7 @@ def revision=(desired)
     end
     # TODO: Would this ever reach here if it is bare?
     update_submodules if !ensure_bare_or_mirror? && @resource.value(:submodules) == :true
-    update_owner_and_excludes
+    update_owner_permission_and_excludes
   end
 
   def bare_exists?
@@ -228,7 +228,7 @@ def update_references
     at_path do
       git_remote_action('fetch', @resource.value(:remote))
       git_remote_action(*fetch_tags_args, @resource.value(:remote))
-      update_owner_and_excludes
+      update_owner_permission_and_excludes
     end
   end
 
@@ -246,6 +246,8 @@ def convert_working_copy_to_bare
     FileUtils.mv(File.join(@resource.value(:path), '.git'), tempdir)
     FileUtils.rm_rf(@resource.value(:path))
     FileUtils.mv(tempdir, @resource.value(:path))
+    FileUtils.chown(@resource.value(:user), @resource.value(:group), @resource.value(:path)) if @resource.value(:user) || @resource.value(:group)
+    FileUtils.chmod(@resource.value(:mode), @resource.value(:path)) if @resource.value(:mode)
     at_path do
       exec_git('config', '--local', '--bool', 'core.bare', 'true')
       return unless @resource.value(:ensure) == :mirror
@@ -266,13 +268,15 @@ def convert_bare_to_working_copy
     notice 'Converting bare repository to working copy repository'
     FileUtils.mv(@resource.value(:path), tempdir)
     FileUtils.mkdir(@resource.value(:path))
+    FileUtils.chown(@resource.value(:user), @resource.value(:group), @resource.value(:path)) if @resource.value(:user) || @resource.value(:group)
+    FileUtils.chmod(@resource.value(:mode), @resource.value(:path)) if @resource.value(:mode)
     FileUtils.mv(tempdir, File.join(@resource.value(:path), '.git'))
     if commits?
       at_path do
         exec_git('config', '--local', '--bool', 'core.bare', 'false')
         reset('HEAD')
         git_with_identity('checkout', '--force')
-        update_owner_and_excludes
+        update_owner_permission_and_excludes
       end
     end
     set_no_mirror if mirror?
@@ -398,7 +402,8 @@ def init_repository
     else
       # normal init
       FileUtils.mkdir(@resource.value(:path))
-      FileUtils.chown(@resource.value(:user), nil, @resource.value(:path)) if @resource.value(:user)
+      FileUtils.chown(@resource.value(:user), @resource.value(:group), @resource.value(:path)) if @resource.value(:user) || @resource.value(:group)
+      FileUtils.chmod(@resource.value(:mode), @resource.value(:path)) if @resource.value(:mode)
       args = ['init']
       args << '--bare' if @resource.value(:ensure) == :bare
       at_path do
@@ -580,8 +585,8 @@ def get_revision(rev = 'HEAD')
   end
 
   # @!visibility private
-  def update_owner_and_excludes
-    set_ownership if @resource.value(:owner) || @resource.value(:group)
+  def update_owner_permission_and_excludes
+    set_ownership_and_permissions
     set_excludes if @resource.value(:excludes)
   end
 

lib/puppet/provider/vcsrepo/hg.rb

@@ -115,7 +115,7 @@ def clone_repository(revision)
   end
 
   def update_owner
-    set_ownership if @resource.value(:owner) || @resource.value(:group)
+    set_ownership_and_permissions
   end
 
   def sensitive?

lib/puppet/provider/vcsrepo/p4.rb

@@ -101,7 +101,7 @@ def source=(_desired)
   private
 
   def update_owner
-    set_ownership if @resource.value(:owner) || @resource.value(:group)
+    set_ownership_and_permissions
   end
 
   # Sync the client workspace files to head or specified revision.

lib/puppet/provider/vcsrepo/svn.rb

@@ -242,7 +242,7 @@ def create_repository(path)
   end
 
   def update_owner
-    set_ownership if @resource.value(:owner) || @resource.value(:group)
+    set_ownership_and_permissions
   end
 
   def update_includes(paths)

lib/puppet/type/vcsrepo.rb

@@ -213,6 +213,10 @@ def insync?(is)
     desc 'The group/gid that owns the repository files'
   end
 
+  newparam :mode do
+    desc 'The permission for the repository directory itself (not recursive)'
+  end
+
   newparam :user do
     desc 'The user to run for repository operations'
   end

spec/acceptance/clone_repo_spec.rb

@@ -636,4 +636,25 @@
       it { is_expected.to be_mode '664' }
     end
   end
+
+  context 'with mode' do
+    pp = <<-MANIFEST
+      vcsrepo { "#{tmpdir}/testrepo_mode":
+        ensure => present,
+        provider => git,
+        source => "file://#{tmpdir}/testrepo.git",
+        mode => 'u=rwX,g=rX,o=X',
+      }
+    MANIFEST
+    it 'clones a repo' do
+      # Run it twice and test for idempotency
+      idempotent_apply(pp)
+    end
+
+    describe file("#{tmpdir}/testrepo_mode") do
+      # NOTE: '0664' is not supported by 'be_mode'; this must be three digits
+      # unless the first octet is non-zero.
+      it { is_expected.to be_mode '751' }
+    end
+  end
 end

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

@@ -313,7 +313,7 @@ def branch_a_list(include_branch = nil?)
           .with('config', '--local', '--bool', 'core.bare', 'false').and_return(true)
         expect(provider).to receive(:reset).with('HEAD').and_return(true)
         expect(provider).to receive(:git_with_identity).with('checkout', '--force').and_return(true)
-        expect(provider).to receive(:update_owner_and_excludes).and_return(true)
+        expect(provider).to receive(:update_owner_permission_and_excludes).and_return(true)
         expect(provider).to receive(:mirror?).and_return(false)
         provider.instance_eval { convert_bare_to_working_copy }
       end
@@ -330,7 +330,7 @@ def branch_a_list(include_branch = nil?)
           .with('config', '--local', '--bool', 'core.bare', 'false').and_return(true)
         expect(provider).to receive(:reset).with('HEAD').and_return(true)
         expect(provider).to receive(:git_with_identity).with('checkout', '--force').and_return(true)
-        expect(provider).to receive(:update_owner_and_excludes).and_return(true)
+        expect(provider).to receive(:update_owner_permission_and_excludes).and_return(true)
         expect(provider).to receive(:exec_git).with('config', '--unset', 'remote.origin.mirror')
         expect(provider).to receive(:mirror?).and_return(true)
         provider.instance_eval { convert_bare_to_working_copy }