Disable git hooks for git-annex pushes

Customer in https://gitlab.zendesk.com/agent/tickets/7394 suggested disabling git hooks validation for git-annex pushes branches.

They've ran into issues pushing big files because their git hook that checks commit messages triggered when they were pushing file using git annex.

***
* [new branch] git-annex -> origin/git-annex 
ok 
***
*remote: GitLab: Commit message does not follow the pattern 
'(([Tt]icket|[Rr]efs|[Ff]ix|[Ff]erme) #\d+|[Mm]erge).*'* 
To ***
! [remote rejected] git-annex -> synced/git-annex (pre-receive hook 
declined) 
! [remote rejected] master -> synced/master (pre-receive hook declined) 

This is also the case for the customer in https://gitlab.zendesk.com/agent/tickets/9339 where they have email validation that is failing.

cc @dzaporozhets @patricio @dblessing @JobV

I think it is a good idea to skip git hooks for git annex pushes.

I also think is a good idea, as git-annex pushes seem to also be failing if the Check whether author is GitLab user hook is active. => https://gitlab.zendesk.com/agent/tickets/9339

@marin amount of work?

+1 for disabling it

Milestone changed to 8.2

Milestone changed to 8.3

@JobV 8.3 came and went and this was not worked on. It is becoming a problem for a new customer. They have a hook set up to disallow committing binary files through normal Git by denying certain file extensions. The hook is firing on a Git annex commit as well, completely defeating the purpose of git annex. I guess there is no time to get it in on 8.4, right?

Added ~113221 label

Report back here as well: https://gitlab.zendesk.com/agent/tickets/14704

Milestone changed to 8.5

I asked @marin to have a quick look. Let's see. Maybe we should put it in a patch.

Looking at the code in git_access.rb I don't understand why the git hooks even get triggered. We have separate methods for git-annex commands that already skip all the hooks. My guess is that initial git annex command gets picked correctly and approved and then once the payload is received git-annex sends another regular push which then triggers the hooks. Will have to setup an instance to do a bit of debugging.

Reassigned to @marin

If I remember correctly, git-annex does at least 3 different pushes (from what I've seen the git logs). I'm not sure why or what they are needed for, but one of these 3 must be hitting the hooks.

Thanks for looking into this @marin.

Thanks @patricio, indeed it does.

It first uses the git-annex-shell commands to store the object and then it uses git to push to master:synced/master. This is where the hooks get triggered.

After the object is pushed upstream and git-annex made sure that the branches it uses are up to date(synced/BRANCHNAME and synced/git-annex), it then triggers git-annex-shell commit DIR. This will do the final merge into BRANCHNAME.

So the problem is at the step of pushing synced/BRANCHNAME and synced/git-annex. The fix is simple, right?; just check if the branches are named certain way and skip git hooks. Well that is wrong: What if git-annex is not enabled and a user pushed synced/master? We cannot just skip hooks, that would be just giving blank permissions.

This is where things get complicated: gitlab-rails doesn't know if git-annex is enabled because this setting is in gitlab-shell/config.yml.

I see couple of possible solutions:

1. Add another same setting in gitlab.yml and tell all users if they want to use git-annex, they need to set these two config options. For omnibus-gitlab that is not a problem, for source installations this might be a problem. (we already do this btw with various paths, we have a subsection in gitlab.yml called gitlab_shell). Then it is as simple as if config.git_annex_enabled && annex_branches? skip_hooks end.
2. Add a database field to Project table, eg. supports_large_files. On first push of git-annex command we would set this field to true and then we can just do if project.supports_large_files && annex_branches? skip_hooks end.
3. ????

I would personally opt for option 1. because we already require some config duplication between gitlab-shell and gitlab-rails. If you use the omnibus-gitlab package nothing would change for the end user. Installations from source would have to update their configuration. Second option feels icky, push to set a database field(eeek).

@dzaporozhets Need your help with this one. Do you see any other option?

@marin I propose we go with option 1. Especially because

If you use the omnibus-gitlab package nothing would change for the end user

mentioned in merge request !129 (merged)

Status changed to closed by commit fb71b01c

Status changed to closed by merge request !129 (merged)

mentioned in commit 2bc69854

mentioned in commit fb71b01c

Status changed to reopened

mentioned in merge request omnibus-gitlab!612 (merged)

Added omnibus-gitlab configuration omnibus-gitlab!612 (merged) .

Need to update the docs. => gitlab-org/gitlab-ee!132

@patricio this will be in 8.4.

mentioned in commit omnibus-gitlab@796a0d98

thanks @marin! I've reported back, so we can close this issue.

Status changed to closed

mentioned in merge request !132 (merged)

@marin the customer replied back that after upgrading to 8.4.3-ee it is still triggering the file name restriction git hook. https://gitlab.zendesk.com/agent/tickets/15565

Any ideas?

Status changed to reopened

@patricio it is unclear from that ticket what kind of installation do they have. Can you confirm that they have in gitlab.yml under gitlab_shell: git_annex_enabled: true ?

@marin thanks for checking. They are using Omnibus and have had it enabled for quite a while now. If they remove the file name check hook, it works as expected. Git Annex completes the requests and the files are added, but as soon as they add the hook, it fails. They need the hook to force their users to use annex for their large files. This customer was one of the first to report this problem. 8.4 should have solved that, but for some reason it still does not work for them.

@patricio I tried reproducing this failure on gitlab.com in a test repo but I am getting a expected behaviour. I did this:

• Add a git hook \.(jar|ear|exe|war|rar|tar|gz|gzip|zip|bin|so|swf|class|o|dmg)$
• Add a .dmg file and added it with git annex add FILE.dmg
• git annex sync --content pushed the file correctly to the repository
• Pushed a tar.gz file using git annex, it worked
• Added a tar.gz file but using regular git, push was refused because of the git hook

I've added you to my test repo so you can confirm. It is also possible that I am set in the ways I test this stuff since I did it number of times so I might be missing a step somewhere.

Milestone changed to 8.6

Assignee removed

@marin sorry it has taken this long to get back to you. I have heard from the customer. they tried against GitLab.com and it is also failing here. Can you please look at his last reply and tell me what you think? Thank you!

https://gitlab.zendesk.com/agent/tickets/15565

@marin can you help us with the ZD ticket I mentioned above?

@patricio I am sorry, I am really struggling keeping up with things. I will see if I can take a look today or tomorrow but if someone else can also jump in it would be great.

Reassigned to @jameslopez

I believe I reproduced the issue, thought it had to do with https://gitlab.com/gitlab-org/gitlab-ee/issues/279 but now I'm not very sure. I'll dig more into it

mentioned in merge request !272 (merged)

OK so this was a bit tricky, basically the issue as pointed in https://gitlab.zendesk.com/agent/tickets/15565 happens after we successfully sync and then push to the normal branch.

Setup: Added filename Git hook \.(jar|ear|exe|war|rar|tar|gz|gzip|zip|bin|so|swf|class|o|pyc)$

Enabled Annex on the server and installed server and client.

Locally:

$ git clone ssh://git@IP:2222/root/tiny-repo.git
$ touch README.md && git add README.md && git commit -m "add README" && git push -u origin master #fine
$ git-annex init
$ dd if=/dev/urandom of=file2.bin bs=1 count=2048000
$ git push
Everything up-to-date
$ git annex add file2.bin
$ git annex sync --content #fine
$ git push 
Total 0 (delta 0), reused 0 (delta 0)
remote: GitLab: File name "file2.bin" is prohibited by the pattern '\.(jar|ear|exe|war|rar|tar|gz|gzip|zip|bin|so|swf|class|o|pyc)$'
To ssh://git@IP:2222/root/tiny-repo.git
 ! [remote rejected] master -> master (pre-receive hook declined)
error: failed to push some refs to 'ssh://git@IP:2222/root/tiny-repo.git'

Server:

# Debugging git_access.rb#git_annex_branch_sync?(changes)
changes = "e86d115e2f3af17566b8f4e385790236b22f4081 5924ed3558e596e839f05419be5590142a5e9700 refs/heads/master\n"

As this push goes to master, we can’t detect this goes to synced/git-annex or synced/master.

Same commit locally:

git show
commit 5924ed3558e596e839f05419be5590142a5e9700
Author: James Lopez <james@blah>
Date:   Wed Mar 16 12:45:22 2016 +0100

    git-annex in James@blah.local:~/Development/test/tiny-repo

diff --git a/file2.bin b/file2.bin
new file mode 120000
index 0000000..44ddfb4
--- /dev/null
+++ b/file2.bin
@@ -0,0 +1 @@
+.git/annex/objects/p0/8g/SHA256E-s2048000--cd6332098bf62b6f852ef87bbbd18cf76a79a8765b5f2a1e71278fd517b12ed3.bin/SHA256E-s2048000--cd6332098bf62b6f852ef87bbbd18cf76a79a8765b5f2a1e71278fd517b12ed3.bin
\ No newline at end of file

So we can see it comes from git-annex because of the message.

Attempting a fix on https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/272

Feel free to offer other suggestions /cc @marin @DouweM @rspeicher

Status changed to closed by commit d69d43818651cdc081abcddbbe52ee3f25ba148d

mentioned in commit d69d4381

mentioned in commit 023a315d

Mentioned in commit pfjason/gitlab-ce@fb71b01c

Mentioned in commit pfjason/gitlab-ce@d69d4381

mentioned in commit a9907682

mentioned in commit 08d0a28e

mentioned in commit ce2890cb

mentioned in commit 33d6233e

mentioned in commit e8eb9f41