Refactor Auth to make GitHub Enterprise work

[joan38]

Apology, this is a huge refactoring of the options to enforce the necessary options for each forge type.

We run scala-steward @ Netflix and we recently switched from Bitbucket to Github Enterprise. However Basic Auth is disabled in favor of JWT for all API calls. scala-steward currently does not support JWT only credentials. This PR switches all Github API calls to JWT.

[exoego]

Thanks for working on this.

However, this PR seems to contain

• lots of unrelated refactorings
• unrelated changes (e.g. removing Java 21 from workflow) with reason unexplained

It's much easier to review if such things are extracted into small PRs

[joan38]

@exoego

Java 21 change was due to a rebase I guess.

I'm trying to reduce the change set but changing the config is touching practically all files.

Listing all the changes:

Refactor configuration

The current configuration has forge specific configs:

    bitbucketCfg: BitbucketCfg,
    bitbucketServerCfg: BitbucketServerCfg,
    gitLabCfg: GitLabCfg,
    azureReposCfg: AzureReposCfg,

and some mandatory config for Github that are optional:

    githubApp: Option[GitHubApp],

This makes impossible to handle auth without doing some .get or .getOrElse(throw ...)
We need a way to enforce options like --github-app-id when Github is used.

In this change, instead of doing --forge-type <forge type> we use the mutual-exclusion feature of decline to make sure some options are provided for some forge but not in other forges (first time using decline for me and it's actually pretty cool).
The change unfortunately introduces a breaking cli change since we would need to pass for example --gitlab instead of --forge-type gitlab. I tried to think more about how I could make this back compatible but I don't think we can.
Also --github is not necessary since it's the default one.
Another thing I would like to propose is to remove the forge name prefix in front of forge specific options like --gitlab-merge-when-pipeline-succeeds could just be --merge-when-pipeline-succeeds

ForgeType goes away and gets replaced by both ForgeRepo (more on this below) and Forge that contains all the forge specific config. Forge also replaces all GitHubApp, BitbucketCfg, GitLabCfg... This means we can model correctly mandatory fields like appKeyFile: File for GitHub.

Expand ForgeRepo to take the non config part of ForgeType

Now that Forge focuses on forge specific config, we have to move some forge specific behavior and ForgeRepo that was already there, make sense to take on the full implementation for diffUrlFor and fileUrlFor.

Introduction of the ForgeAuthAlg trait

We currently have a GitHubAuthAlg to generate the JWT tokens and GitHubAppApiAlg to access repositories and other, but auth on GitHub is much more complex. We need to ask for short lived tokens and use them for both the entire API and Git password. Hence why ForgeAuthAlg exposes authenticateApi and authenticateGit that can be used to auth requests the way the forge wants:

  def authenticateApi(req: Request[F]): F[Request[F]]
  def authenticateGit(uri: Uri): F[Uri]

Other forge can use BasicAuthAlg that extends ForgeAuthAlg to use Basic Auth or BitbucketServerAuthAlg or GitLabAuthAlg since they use slightly different headers for their tokens.

getGitHubAppRepos moves under the ForgeAuthAlg trait

Since we now have to use ForgeAuthAlg to get the GitHub repos, I added accessibleRepos which is returning empty list for all other forge but GitHub will be using the auth mechanism to list accessible repos just like before.

ForgeSelection is now Forge

Since all the auth stuff in there moved into ForgeAuthAlg, the creation of a Forge made sense to be moved in the Forge.create function, just like any other Alg (btw, what does Alg even mean?).

UrlChecker.create is not delayed

That I'm not sure how I can make it an F and declare it as implicit. So I just remove the F.

I'm ok to open a PR for each of the above listed changes. And the reason I did not in the first place is because a config change on it's own is not worth it. But with the auth change, it is necessary.
Also I'm doing it as part of my job and don't have much time to dedicate to this.

Anyhow, let me know what's the best way to proceed. I'm also available for a call if it helps.

[joan38]

@exoego Let me know if the overall plan sounds good and I can make multiple based on the above changes?

[joan38]

@exoego Let me know if the general idea of this PR is good to you and I can do multiple PRs.
We currently have Scala-Steward down for all Scala repos at Netflix and I'm eager to make this work again.
Thanks

[joan38]

cc @mzuehlke @alejandrohdezma
Really looking to move forward with Github Enterprise support: https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/authenticating-as-a-github-app

[joan38]

I was trying to understand how is Scala-Steward working on Github.com and not working for Github Enterprise, and I found out that's because the user @scala-steward is used to authenticate, whereas in Github Enterprise there is no service account. So we have to use Github Apps instead and the authentication for Github Apps is only though JWT. No Basic Auth with token is allowed.

So in essence, Scala-Steward does not work with Github Enterprise as of today.

[alejandrohdezma]

Hey @joan38 doesn't it work if you use the Scala GitHub Action? I'm quite sure it should work with GitHub enterprise

[joan38]

@alejandrohdezma GitHub Actions are not enabled for our Enterprise instance

[alejandrohdezma]

oh, I see, then I guess it would be good to follow @exoego suggestion on splitting the changes done in this PR to smaller reviewable ones

[joan38]

Sounds good, I just wanted to make sure I'm not doing this in void.
PRs incoming.
Thanks @alejandrohdezma

[mzuehlke]

I really like the idea. I started contributing some years ago with the same aim 😄
But then I discovered that Github Action are a working solution for me.

Breaking the CLI is always an unpleasant thing.....
Without delving into this too deep, could we maybe have 2 entry points?
The default one breaks and used the better structure arguments, but you can (for some time) use a legagy entry point. that print a lot of warning and works like now ?
But not sure if this is even worth the effort......

[joan38]

Thanks @mzuehlke
We don't have GitHub Actions enabled at Netflix.
I actually didn't want to break the CLI too, but stuff like login, github-key, ask-pass... Have to all be optional and then we need to throw exceptions here and there when the options are not specified.

[mzuehlke]

Can this be closed ?

[joan38]

Should I do a rebase? Or breaking the CLI is out of the question?

[mzuehlke]

If rebasing isn't a big effort, it would be easier to see what changes are left from these.
But I wouldn't break the CLI if not needed.