Git: Allow to auth with password/token

[tdesveaux]

At the moment, Buildbot allow to provide a ssh key to handle Git authentication.
GitHub and other hosts allow to authenticate with access tokens (and even recommend it).

From a rather quick search, I only found this related issue #3137

I started implementing the feature to authenticate with said token in Buildbot on this branch.
This add the gitCredentials and gitCredentialStoreContent (I'll try to find better names...) attributes to GitStepMixin to enable authentication with said tokens by using the git credential git feature with the store which store credentials in a file allowing to implement in a similar way to the ssh auth.

There is some work left to do in this branch as it's more in a POC state than anything (no tests/doc, only tested source.Git step, make it work for GitPoller, ...).

Is this a feature of interest to the Buildbot project? And is this approach acceptable?

I also saw that there might be some opportunities to refactor some related code that I could tackle while at it.
Two that come to mind:

• Maybe extract the ssh key code from GitStepMixin into a GitStepWithAuthMixin to remove the dummy declarations in GitTag and GitCommit steps.
• Maybe find a way to factorize the code a bit between GitStepMixin and GitPoller? There seems to be a bit of duplication in the ssh key handling.

Let me know if you think of other things I can look at.

[p12tic]

@tdesveaux Thanks for working on this feature. It makes sense to have it in Buildbot.

I've looked into the WIP code and I didn't see any problems in the logic part.

Just before doing a PR please replace all camelCase in code additions with snake_case - Buildbot is gradually transitioning to standard Python snake_case style and all new code should use that.

[tdesveaux]

@p12tic apologize about the ping, but I feel I need to confirm an architectural / code style choice with you.

I started to look into this by refactoring the code a bit (first by trying to move SSH stuff to GitMixin so that it can be used by both GitPoller and GitSteps).

However, I'm feeling more and more that it would be simpler to use composition instead of the Mixin pattern here.
I'd like your input as Mixin seems to be used accross the codebase, and might still be prefered in this case (my issues might be more about my lack of experience with Mixin and multiple inheritance in Python).

I see the following drawbacks to the Mixin inheritance used now:

• it makes the code quite confusing to work with as it uses properties that do not exist in the context of the class but will in the inherited one (eg. here).
• More a multiple inheritance thing, but it makes using method override and super calls quite bothersome and requires that any class call super and get / forward kwargs.

Using composition, I could either:

• create a base abstract class GitSSHAuthBase and two classes GitStepSSHAuth / GitServiceSSHAuth for the specifics
• a simple GitSSHAuth which is provided functions for implementation differences (such as path helpers being simply os.path for GitPoller vs self.build.path_module)

I tend to prefer the 2nd option but it could become unwieldy if there are too many differences between service/worker, but will allow to implement auth by password/token in the same way and abstract both into a larger GitAuth since their behaviour would be similar.

[p12tic]

Myself I prefer composition as well due to the reasons you've outlined. Mixins were preferred by the previous Buildbot development team. I think that your complaint is enough of a reason to change the architecture to composition.

In the composition case I think the first option with inheritance is slightly better as the complexity tends to grow and it will be easier to expand the derived classes with whatever functionality we need.

Whatever you do, it would be great to have the refactor as a separate PR without additional functionality yet.