On cloud-vps puppetservers the puppet checkouts are owned by the 'gitpuppet' user and kept in /srv/git.
Puppet 7 has some new ownership constraints which means that we can no longer investigate these repos as root, for example:
root@metricsinfra-puppetserver-1:/srv/git/operations/puppet# git branch fatal: detected dubious ownership in repository at '/srv/git/operations/puppet' To add an exception for this directory, call: git config --global --add safe.directory /srv/git/operations/puppet
This produces several problems:
- puppetserver-deploy-code doesn't work. I recently modified it to check the branch before deploying, but since it runs as root it can't check the branch and errors out.
- Users (including me) are likely to run the suggested '--add safe' command and get on with things. That appears to work but breaks future runs of git-sync-upstream (something the user may not notice until weeks or months later).
Arturo points out that this has been a chronic issue even before puppet 7
T325280: cloud puppetmasters fail to run git-sync-upstream - https://phabricator.wikimedia.org/T325280
T184259: labspuppetmaster1001: have consistency in owner of git repos - https://phabricator.wikimedia.org/T184259
T152060: Inconsistent groups for Git repositories with role::puppetmaster::standalone - https://phabricator.wikimedia.org/T152060
I think the right solution here is to be like prod and just have /srv/git things owned by root. The gitpuppet user was initially created to avoid running git-sync-upstream but that script has since been rewritten to run as root and setuid to gitpuppet for select actions which is arguably just as risky as leaving things root-owned.
The alternative is refactoring puppetserver-deploy-code to run as an arbitrary user... I went down that road a bit and it started to get messy. OR, we could rip out the branch check in puppetserver-deploy-code and let it run blindly, but that was what produced our parent task T364047