title: TPA-RFC-76: Puppet merge request workflow costs: N/A approval: TPA affected users: TPA deadline: 1 week, 2025-01-16 status: standard discussion: https://gitlab.torproject.org/tpo/tpa/team/-/issues/41971

[[TOC]]

Summary: let's make a mirror of the Puppet repo on GitLab to enable a MR workflow.

Background

In the dynamic environment work, @lavamind found over a dozen branches in the tor-puppet.git repository.

In cleanup branches in tor-puppet.git, we tried to clean them up. I deleted a couple of old branches but there's a solid core of patches that just Must Be Merged eventually, or at least properly discussed. Doing so with the current setup is really and needlessly hard.

The root access review also outlined that our lack of merge request workflow is severely impeding our capacity at accepting outside contributions as well.

Proposal

Mirror the tor-puppet.git repository from the Puppet server (currently pauli) to a new "Puppet code" private and readonly repository the GitLab server.

Project parameters

  1. Path: tpo/tpa/puppet-code (to reflect the Tails convention)

  2. Access: private to TPA, extra "reporter" access granted on a case-by-case basis (at least @hiro)

  3. Merge policy: "fast-forward only", to force developers to merge locally and avoid accidentally trusting GitLab

  4. Branch rules: disallow anyone to "merge" or "push and merge" to the default branch, except a deploy key for the mirror

Rationale

Each setting above brings us the following properties:

  1. Establish a puppet-* namespace in tpo/tpa that is flat (i.e. we do not call this tpo/tpa/puppet/code or have modules named tpo/tpa/puppet/sshd for example, that would be instead tpo/tpa/puppet-sshd

  2. Avoid a long and risky audit of the Puppet codebase for PII while providing ways for contributors outside of TPA (but at least core contributors) to contribute

  3. Not trusting GitLab. By forcing "fast-forward", we make sure we never mistakenly click the "merge" button in GitLab, which makes GitLab create a merge commit which then extends our attack surface to GitLab

  4. Same as (3), another safeguard. This covers the case where someone mistakenly pushes to the production branch. In this case, they are simply not allowed to push at all. The mirror is updated with a deploy key that lives on the Puppet server.

Best practices

In general, the best practice we want to establish here is this:

  • Don't push directly to GitLab, unless for rare exceptions (e.g. if you don't have write access to the repository, in which case you should push to your fork anyways)

  • If you do manage to push to GitLab's production branch (which shouldn't be possible), make sure you sync that branch with the one on the Puppet server, then push everywhere so the mirroring does not break

  • If you push another branch, push it first to the Puppet server and let it mirror to GitLab, then make a Merge Request on GitLab to seek reviews

  • Don't pull from GitLab, again unless exception (external merge requests being an example)

  • If you do pull from GitLab (either by accident or in exceptional cases), do systematically review the patch pulled from GitLab before pushing back to the Puppet server

  • To merge a feature branch, pull it locally, then review the changes in detail, merge locally (i.e. not on GitLab), then push back to the Puppet server. Again, ideally pull from the Puppet server, but if it's on GitLab only, then from GitLab.

Alternatives

Making repository public

Note that this is different from Publish our puppet repository: to fix our immediate issues, we do not have to make the repository public to the world.

We still want to do this eventually, but it feels better to cleanup our act first (and perhaps merge with tails).

Trusting GitLab

The mistake we are trying to avoid is to end up (accidentally) trusting GitLab. It would be easy, for example, to create a merge request, merge it, and have someone pull from GitLab by mistake, updating their default branch with code managed by GitLab.

This would widen the attack surface on the critical Puppet infrastructure too much.

Instead, we forbid merges altogether on that repository.

We might be able to improve on that workflow and start trusting GitLab when we setup commit signing, but this is out of scope for now.

Deadline

Please comment before the end of the coming week, 2025-01-16 AoE (UTC-12).

References