On a review of change to git::clone ( Gerrit #927750) I am wondering whether the umask parameter needs to be exposed or even used at all.
The first implementation from March 2014 at 74cd7ade251549fa89b3280ae237ad47c95011ae which originally used a umask parameter and git config core.sharedRepository=umask. Then some hours after on the same day 884c794e160b6be7c43006d6ddaef1b4c1c2df3c introduced core.sharedRepository=group .
Followed up in January 2015 with some more umask fix via 032c07b7c0060200bd1f5816f575d1f0465fd524
If git::clone has 'shared => true` then the previously cloned repository has in its config core.sharedRepository=group which cause git to set files and objects group-writable. A local test using git -c core.sharedRepository=group shows all files/directories are group writable.
Thus I think umask is a left over and does not serve any purpose since beside a normal clone, the sole other use case I imagine is a group shared repo.
From a grep in Puppet I found TWO usage:
git::clone { 'releng/dev-images': directory => '/srv/dev-images', owner => 'root', group => 'wikidev', mode => '0775', umask => '002', origin => 'https://gitlab.wikimedia.org/releng/dev-images.git' }
It comes from 3a8854054c1c6ae409f776c076d1f28cc2b980e5 and I think should instead use shared => true.
git::clone { 'integration/config': directory => '/etc/zuul/wikimedia', owner => 'zuul', group => 'zuul', mode => '0775', umask => '002', origin => 'https://gerrit.wikimedia.org/r/integration/config.git', branch => $conf_server['config_git_branch'], }
Umask comes from the previously mentioned January 2015 commit 032c07b7c0060200bd1f5816f575d1f0465fd524. Pretty sure it can be changed to shared => true as well. That repository is very old and probably even predate git::clone.
Both repositories are managed and primarily used by Release-Engineering-Team so I guess the impact is rather limited.
So we should:
- switch both to use shared => true
- remove the umask parameter from git::clone