Consider migrating to libgit2/git2go

[phillebaba]

Currently the source-controller uses go-git as a dependency to interact with git repositories. go-git is a pure go implementation of the git protocol, which is used by a couple of projects. A long running issue with go-git has been the lack of support for multi_ack and multi_ack_detailed (3 years). As it is a pure go implementation it requires people withing the go community who have know how about the git protocol, something that seems very rare. If maintainer ship of go-git ever becomes an issue I question if there is anyone who can continue supporting the project.

libgit2/git2go was considered as an option before the summer but at the time lacked functionality that was required at the time, which it has now received. A major difference, which is good and bad, is that it is a pure c implementation of git core. The good being that the community around the project is a lot larger, as there are bindings to multiple languages. The bad being that it introduces c dependencies to the source-controller, which some may not like.

My question is if we could consider moving from go-git to libgit2/git2go in the source-controller.

[stefanprodan]

I'm trying to find an active project that uses this library, all I can find are some abandoned experiments on GitHub. Maybe no one uses it due to thread safety concerns: "This may also make SSH connections thread-safe if your copy of libssh2 is linked against OpenSSL."

[phillebaba]

Great! I can try to get a POC working within this or next week, I will keep you guys updated.

[phillebaba]

@stefanprodan @hiddeco I have a working version of source-controller which uses git2go. It works pretty well and does not make things more complex.
https://github.com/phillebaba/source-controller/tree/feature/git2go

I have also verified that it works with Azure DevOps.

flux-system   ssh://git@github.com/phillebaba/fleet-infra              True    Fetched revision: main/6c2ccdfd865c9fbc10243562c781acd0a564d80d     94m
foobar        https://dev.azure.com/philiplaine0831/TEST/_git/foobar   True    Fetched revision: master/1a7635dabced29d8edfc5b28185170989ba5a593   94m
monitoring    https://github.com/fluxcd/flux2                          True    Fetched revision: main/d84297a5b55a405ad2e00b168a8b11a821d17978     94m
webapp        https://github.com/stefanprodan/podinfo                  True    Fetched revision: master/95be17be1dc2103eb5e2c0b0bac50ef692c4657d   94m

The only thing I have not implemented is verifying known_hosts, I cant find any example of anyone doing it with git2go so that still needs to be solved.

If you could look at my code before the next community meeting we can discuss any issues then. Or is it better if I create a PR with my changes?

[stefanprodan]

Great job @phillebaba 🎉

I had a quick look over your changes and I have the following observations:

• we should pull only the last 2 commits, I can't tell for sure but I think we fetch all git history with git2go
• we need to implement pgp signature verification, I guess we can call commit.ExtractSignature() and do the verification on our own with openpgp.CheckArmoredDetachedSignature
• the known_hosts verification could be done in Go using fluxcd/pkg/ssh/knownhosts

[phillebaba]

Thanks for the feedback Stefan.

• I had a look around and it seems like this issue is a bit contentious within the project as it has been a long running issue Support for shallow repositories libgit2/libgit2#3058. Seems like every git lib have their own missing features. There is a PR that is yet to be merged that would implement shallow clones Shallow support v2 libgit2/libgit2#5254.
• I missed that I had commented out the signature verification so I fixed that without any problems.
• Sadly we cant use the the knownhosts package as we are only given sha1 and md5 fingerprints of the returned publickey from the server and not the key itself. I have implemented a check that does work for known host files in the rsa format. I just need to clean it up a bit before pushing it. I have also posted the question in the libgit2 Slack and gotten the response that there may be interest in implementing support for HostKeyCallback in git2go.

[paraskaneriya]

This is good. However, do you have any reference implementation which I can follow to work with Azure DevOps?

[stefanprodan]

I propose we create an interface for our git package and add the git2go implementation. Then in the GitRepository spec users will be able to switch between the two using a field named gitProtocal that accepts v1(go-git) and v2 (git2go), when not specified defaults to v1. We need to document the differences and tell Azure DevOps users to use v2. We should warn users that using git2go means that source-controller will fetch all the git history, for some repos this means downloading GB of data that could fill the disk and also impact their traffic costs by a lot.

[squaremo]

👍

Unless the intent is pin the protocol used to v1 or v2, it might be better to use a term that suggests a trade-off for compatibility: something like GitProtocolV2Compatibility: true|false. Besides giving the user a clue that it's not a straight-forward choice of version, this is forward-compatible with respect to the underlying libraries evolving (or a better alternative being found):

	v1 / v2	GitProtocolV2Compatibility
libgit2 gets shallow clone	people have to change setting	it just works better
go-get implements v2	disable v2 support or break setting	remove alternate implementation

[phillebaba]

Eventually, and hopefully, in the future when libgit2 merges shallow support we could remove go-git as libgit2 would do everything which go-git does. We should not punish those who don't need v2 with poorer performance just because libgit2 needs to clone the whole repository. I would be very happy if we could implement this compromise.

[phillebaba]

There is now a PR that would solve this discussion.

fluxcd/source-controller#213