Skip to content

Only enumerate commits in pre-receive check if push came from Web

What does this MR do and why?

In GitLab 17.0, !150466 (merged) introduced a push rule check that attempts to validate that a signed commit doesn't have a customized author. This check requires calling ListAllCommitsRequest to enumerate all the commits in the quarantined repository.

This merge request introduced Gitlab::Checks::CommitChecks that enumerates over all the commits behind the skip_committer_email_check feature flag. However, regardless of the state of this feature flag, ListAllCommitsRequest is still called. This RPC can timeout for pushes with large number of commits, resulting in a 500 error in the /api/v4/internal/allowed endpoint and ultimately causing pushes to fail.

As it stands, the current feature only checks commits that were:

  1. Signed by GitLab itself
  2. Pushed via a UI change

We can short-circuit the commits enumeration in the common case by checking whether the push came from the UI.

Relates to #467556 (closed)

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

How to set up and validate locally

  1. Create a new project.
  2. Attempt to push a branch of the Linux kernel:
git clone -b master https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git remote add gitlab <your remote>
git push -u gitlab master

On master, I see:

% git push -u gitlab master
Enumerating objects: 8573144, done.
Counting objects: 100% (8573144/8573144), done.
Delta compression using up to 10 threads
Compressing objects: 100% (1404972/1404972), done.
Writing objects: 100% (8573144/8573144), 3.43 GiB | 32.76 MiB/s, done.
Total 8573144 (delta 7116859), reused 8573144 (delta 7116859), pack-reused 0 (from 0)
remote: warning: object d924c0ed349b7f92c528df97708cbe85d4b18f48: badFilemode: contains bad file modes
remote: warning: object acafbc6b9b8ce5a90e40c673dd03abd898876929: badFilemode: contains bad file modes
remote: warning: object 8e9223ef4deb8137fd5a7722da13d753745c06aa: badFilemode: contains bad file modes
remote: warning: object 8264a1b0eeebd9cda625822a1ac6df22d631e8dc: badFilemode: contains bad file modes
remote: warning: object 956b25bd6ec3f1254affb2a47e059497e9c9d030: badFilemode: contains bad file modes
remote: warning: object b2a05c3c518908735ce88038c0043065ef6df11e: badFilemode: contains bad file modes
remote: warning: object ac816417fd510b93e6c95016a42400ef68a810ad: badFilemode: contains bad file modes
remote: warning: object 349794521329ca4029bdc25af77ff7a7e8aa9f83: badFilemode: contains bad file modes
remote: warning: object 602207596acf39400a6bb96fdee83b9bb8b9e0b1: badFilemode: contains bad file modes
remote: warning: object 2b44639ced114a28737daa6cc4812f20e26ed96e: badFilemode: contains bad file modes
remote: warning: object 8ff31038cc910c71b00524b5f38bd0ab24c55217: badFilemode: contains bad file modes
remote: warning: object b2758f55614503d51c777c25faeff549faf6c26c: badFilemode: contains bad file modes
remote: warning: object 313d78fa8a105153164c39c96d494c8ccabd8c7f: badFilemode: contains bad file modes
remote: Resolving deltas: 100% (7116859/7116859), done.
remote: Checking connectivity: 8573144, done.
remote: GitLab: 500 Internal Server Error
To gitlab.example.com:gitlab-org/linux-test.git
 ! [remote rejected]           master -> master (pre-receive hook declined)
error: failed to push some refs to 'gitlab.example.com:gitlab-org/linux-test.git'

With this change, I get:

% git push -u test master
Enumerating objects: 8573144, done.
Counting objects: 100% (8573144/8573144), done.
Delta compression using up to 10 threads
Compressing objects: 100% (1404972/1404972), done.
Writing objects: 100% (8573144/8573144), 3.43 GiB | 32.55 MiB/s, done.
Total 8573144 (delta 7116859), reused 8573144 (delta 7116859), pack-reused 0 (from 0)
remote: warning: object d924c0ed349b7f92c528df97708cbe85d4b18f48: badFilemode: contains bad file modes
remote: warning: object acafbc6b9b8ce5a90e40c673dd03abd898876929: badFilemode: contains bad file modes
remote: warning: object 8e9223ef4deb8137fd5a7722da13d753745c06aa: badFilemode: contains bad file modes
remote: warning: object 8264a1b0eeebd9cda625822a1ac6df22d631e8dc: badFilemode: contains bad file modes
remote: warning: object 956b25bd6ec3f1254affb2a47e059497e9c9d030: badFilemode: contains bad file modes
remote: warning: object b2a05c3c518908735ce88038c0043065ef6df11e: badFilemode: contains bad file modes
remote: warning: object 349794521329ca4029bdc25af77ff7a7e8aa9f83: badFilemode: contains bad file modes
remote: warning: object 602207596acf39400a6bb96fdee83b9bb8b9e0b1: badFilemode: contains bad file modes
remote: warning: object 8ff31038cc910c71b00524b5f38bd0ab24c55217: badFilemode: contains bad file modes
remote: warning: object 2b44639ced114a28737daa6cc4812f20e26ed96e: badFilemode: contains bad file modes
remote: warning: object ac816417fd510b93e6c95016a42400ef68a810ad: badFilemode: contains bad file modes
remote: warning: object b2758f55614503d51c777c25faeff549faf6c26c: badFilemode: contains bad file modes
remote: warning: object 313d78fa8a105153164c39c96d494c8ccabd8c7f: badFilemode: contains bad file modes
remote: Resolving deltas: 100% (7116859/7116859), done.
remote: Checking connectivity: 8573144, done.
To gitlab.example.com:gitlab-org/linux-test.git
 * [new branch]                master -> master
branch 'master' set up to track 'test/master'.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

  • This MR is backporting a bug fix, documentation update, or spec fix, previously merged in the default branch.
  • The MR that fixed the bug on the default branch has been deployed to GitLab.com (not applicable for documentation or spec changes).
  • This MR has a severity label assigned (if applicable).
  • Set the milestone of the merge request to match the target backport branch version.
  • This MR has been approved by a maintainer (only one approval is required).
  • Ensure the e2e:package-and-test-ee job has either succeeded or been approved by a Software Engineer in Test.

Note to the merge request author and maintainer

If you have questions about the patch release process, please:

Edited by Joe Woodward

Merge request reports

Loading