Skip to content

Explicitly put timestamp into Gitaly requests

Patrick Steinhardt requested to merge pks-gitaly-determenistic-rpcs into master

What does this MR do?

When creating git commits, the resulting commit hash is computed over all contents of the commit. This not only includes hashes of referenced objects like e.g. its tree, but also soft information like the author's and committer's timestamps. As a result, commits generated at different points in time have different object IDs.

This hasn't been a problem for Gitaly until now, a standalone Gitaly instance still doesn't care. But this is slowly changing with the not-so-recent introduction of Praefect and reference transactions. If any RPC is called under a transaction, then the result will only be committed if all Gitaly nodes taking part in the transaction arrive at the same result. If any node tries to update a git reference to something else than the others, then it will abort the transaction and not modify the on-disk state.

This is where the problem is: when for example two Gitalies are part of a transaction where the RPC shall compute a commit and update a branch to point to that commit, then both Gitaly instances need to compute the exact same commit, including the date. If there is time drift between both nodes or if they didn't compute the the commit at the exact same point in time, then chances are high they arrive at different commits and thus fail the transaction.

Gitaly has thus introduced a new "timestamp" field for RPCs which create commits. If it is set, its date will be used for the author and/or committer signature. Given that this same request is then distrisbuted to all Gitalies, they stop relying on their own local time and only use what was provided.

The only remaining piece is that there needs to be a central location where this time is reliably set, and the GitLab app is the natural location to do so. So this commit starts injecting timestamps into the requests to make transactions work reliably.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Patrick Steinhardt

Merge request reports