Infrastructure

David Ostrovsky wrote:

New Gerrit versions has fixed critical bugs […]

Critical bugs that haven't been backported to other supported branches?

Guilhem Moulin wrote:

David Ostrovsky wrote:

New Gerrit versions has fixed critical bugs […]

Critical bugs that haven't been backported to other supported branches?

Let's call it just bugs. Only severe problems and major security vulnerabilities are backported to very old stable branches. Also Gerrit transitive dependencies: JGit, Apache SSHD, Eclipse Jetty and Apache Lucene only updated in recent versions and those updates are not back ported.

David Ostrovsky wrote:

[...] and introduced many new features, like new backend NoteDb (replacement of ReviewDb), new Polymer based PolyGerrit UI (replacement of GWT UI) and new build tool: Bazel (replacement of Buck).

I would also add that PolyGerrit feature: "Diff on single page for all changed files" was always requested by so many users.

IIRC, there was an upgrade planned in the near future, also because of a required plugin
Guilhem, can you share the details on what's planned and when?
There's some urgent things in between, but sharing upfront here what's planned and when is a good idea, I think!

Florian Effenberger wrote:

IIRC, there was an upgrade planned in the near future, also because of a required plugin
Guilhem, can you share the details on what's planned and when?

Would have been good (meaning saved a few mails) to have the request here rather than on hostmaster then :-)

Mike and Olivier would like a 2.16 test instance for the CodeMirror plugin. While I was able to build the plugin against 2.13, I was unable to make it work, I guess to changes in the JS API. I'll deploy a 2.16 test instance on vm178 along with our (2.13) stage instance. Upgrading the production instance to >2.13, given the sweat we have throw at upgrade paths, is unlikely to happen in 2019. (This was mentioned to Mike, in the infra call minutes, and on IRC several times already.) Unless an EOL is announced for 2.13, that is.

Mike and Olivier would like a 2.16 test instance for the CodeMirror
plugin. While I was able to build the plugin against 2.13, I was unable
to make it work, I guess to changes in the JS API. I'll deploy a 2.16
test instance on vm178 along with our (2.13) stage instance.
Upgrading the production instance to >2.13, given the sweat we have
throw at upgrade paths, is unlikely to happen in 2019. (This was
mentioned to Mike, in the infra call minutes, and on IRC several times
already.) Unless an EOL is announced for 2.13, that is.

Sorry for the confusion then, seems I got this one wrong. What we should
discuss then however is when the CodeMirror plugin should go live, if
that's bound to 2.16 - IMHO the plan was to have that in 2019 too, but
let's talk this through in the next infra call or team meeting, before I
do more wild guesses here that end up wrong ;-)

FWIW, OpenStack infrastructure members attended recent Gerrit
Hackathon and User Conference and here their report: [1],[2],
also related to the upgrade sequence from:
2.13 -> 2.14 -> 2.15 -> 2.16 -> Migration from ReviewDb to NoteDb happens here -> 3.0.

[1] https://groups.google.com/d/topic/repo-discuss/Hqe2S6-Ex_E/discussion
[2] http://lists.openstack.org/pipermail/openstack-infra/2019-September/006475.html

Interesting timing, I tested that same upgrade path (up to 2.16) earlier this week on the stage instance. Unsurprisingly the fact that accounts are now (since 2.15) stored in NoteDb breaks our user management scripts… We can of course change the script to make it push to All-Users's ‘refs/meta/external-ids’, but that's not so smooth because the migration didn't resolve conflicts nor collisions. MediaWiki folks stumbled upon the same problem: https://phabricator.wikimedia.org/T197192 .

At this point I'd say that it makes sense to sync the upgrade path with the migration to TDF's IdP for Single-Sign On authentication, and deprecate all other authentication methods to Gerrit's web frontend. On our 2.16 stage I forked your OAuth2 plugin to make it work with our provider (LemonLDAP::NG — like for other IdP it's only a matter of choosing the right paths to the various endpoints and then map the JSON object to an account).

At this point I'd say that it makes sense to sync the upgrade path with the migration to TDF's IdP for Single-Sign On authentication, and deprecate all other authentication methods to Gerrit's web frontend.

I totally agree on this, and expected that gerrit is going to be moved to LDAP auth scheme, before upgrading to 2.15. I even had a discussion with migration steps with Norbert years ago, from OpenID/OAuth to LDAP.

On our 2.16 stage I forked your OAuth2 plugin to make it work with our provider (LemonLDAP::NG — like for other IdP it's only a matter of choosing the right paths to the various endpoints and then map the JSON object to an account).

I would happily review your changes and would merge them upstream. Could you please upload your changes to oauth-plugin project in gerrit-review: [1]?

Another option would be to use: saml plugin, that I recently moved to gerrit-review and adapted to work against most recent gerrit versions: [2]. With saml plugin the real SSO should work out of the box and some enterprise customers already successful use this plugin in production.

[1] https://gerrit-review.googlesource.com/admin/repos/plugins/oauth
[2] https://gerrit-review.googlesource.com/admin/repos/plugins/saml

David Ostrovsky wrote:

At this point I'd say that it makes sense to sync the upgrade path with the migration to TDF's IdP for Single-Sign On authentication, and deprecate all other authentication methods to Gerrit's web frontend.

I totally agree on this, and expected that gerrit is going to be moved to LDAP auth scheme, before upgrading to 2.15. I even had a discussion with migration steps with Norbert years ago, from OpenID/OAuth to LDAP.

Changing the authentication method to LDAP provides shared credentials not Single Sign-On. It also increases the attack surface as the service can access said shared credentials. Like for other services at TDF I have no plan to use LDAP authentication but instead use a proper WebSSO solution using for instance SAML 2.0 or Auth2 (we're using both).

On our 2.16 stage I forked your OAuth2 plugin to make it work with our provider (LemonLDAP::NG — like for other IdP it's only a matter of choosing the right paths to the various endpoints and then map the JSON object to an account).

I would happily review your changes and would merge them upstream. Could you please upload your changes to oauth-plugin project in gerrit-review: [1]?

Another option would be to use: saml plugin, that I recently moved to gerrit-review and adapted to work against most recent gerrit versions: [2]. With saml plugin the real SSO should work out of the box and some enterprise customers already successful use this plugin in production.

Ah cool, I'll test the SAML plugin also, LemonLDAP::NG has a SAML IdP and this shouldn't require fiddling around with the code.

This is done now and can be closed.

Also, adding the reference to the question about generic OAuth2 provider using
gerrit-oauth plugin: [1]. And of course I am still interested to add support for the
LemonLDAP::NG to the gerrit-oauth plugin. Is the source available in any publicly
accessible repository? I could pick it up myself and upload to the canonical plugin
repository: [2].

[1] https://github.com/davido/gerrit-oauth-provider/issues/134
[2] https://gerrit.googlesource.com/plugins/oauth

Closing, and attaching patch against v2.16.1.

Thanks! Unrelated to this issue, but could you also upload gitiles-plugin patch for marking diff chunks you applied to gitiles-plugin on gerrit.libreoffice.org site?

Closing, and attaching patch against v2.16.1. The new LL::NG provider is trivially derived from your code for GitLab.

I uploaded this CL for review: [1]. Note, that oauth plugin was
bumped to the latest version of scribejava library to address
this issue: [2].

I don't know whether or not LL::NG provider supports bearer token
signature and basic authentication scheme, so that I commented out
new auth and signature ways in the code. If you can figure out,
whether or not new auth schemes are supported (or not), we can fix
those comments: [3], [4].

I guess LEMONLDAP_PROVIDER_PREFIX should be changed to "llng-oauth:" though.

Right, I've changed the prefix as you have suggested to: "llng-oauth:".

Unrelated to this issue, but could you also upload gitiles-plugin patch for marking diff chunks you applied to gitiles-plugin on gerrit.libreoffice.org site?

Moreover, there was another request for highlighting of in line
differences in Gitiles in this thread: [5] and I commented, that
TDF infra team patched Gitiles and it would be great if this patch
could be upstreamed. That's how truly open source driven projects
work. So, please, consider to give your valuable contribution back
to the Gerrit community. Thank you in advance.

[1] https://gerrit-review.googlesource.com/c/plugins/oauth/+/256231
[2] https://github.com/davido/gerrit-oauth-provider/issues/135
[3] https://gerrit-review.googlesource.com/c/plugins/oauth/+/256231/1/src/main/java/com/googlesource/gerrit/plugins/oauth/LemonLDAPApi.java#42
[4] https://gerrit-review.googlesource.com/c/plugins/oauth/+/256231/1/src/main/java/com/googlesource/gerrit/plugins/oauth/LemonLDAPApi.java#49
[5] https://groups.google.com/d/msg/repo-discuss/TJmjeelXG1E/7f5ndn6HGAAJ

(And if you wonder, yes I just created https://git.libreoffice.org/gitiles .)

You're welcome to pick it up at https://git.libreoffice.org/gitiles/commit/e08d0507ae31c38db1d8302ab4d4d15141d909ef .

Thanks. I conducted a custom release: [1] for the diff-match-patch
library and adapted the patch to fetch it on the fly using
Bazel's http_file() rule: [2]. It would be great, if you could
create account on gerrit-review and sign the ACL, I would change
the author in this CL: [2] to your name.

[1] https://github.com/davido/diff-match-patch/releases/tag/v1.0.0
[2] https://gerrit-review.googlesource.com/c/gitiles/+/256512

I conducted a custom release: [1] for the diff-match-patch library and adapted the patch to fetch it on the fly using Bazel's http_file() rule: [2].

Neat, thanks!

It would be great, if you could create account on gerrit-review and sign the ACL

I'm afraid I don't have a Google account :-P I suppose they don't offer non-Googley authentication methods?

David Ostrovsky wrote:

I don't know whether or not LL::NG provider supports bearer token
signature and basic authentication scheme, so that I commented out
new auth and signature ways in the code. If you can figure out,
whether or not new auth schemes are supported (or not), we can fix
those comments: [3], [4].

I guess LEMONLDAP_PROVIDER_PREFIX should be changed to "llng-oauth:" though.

Right, I've changed the prefix as you have suggested to: "llng-oauth:".

Unrelated to this issue but I didn't forget to back after the LL::NG resp. gerrit upgrade :-)

The version we're running at the moment can be found at https://git.libreoffice.org/infra/gerrit-oauth/+log/refs/tags/v3.1.3+tdf . There are some minors modifications to your code:

57e5804 LemonLDAP::NG: Remove getBearerSignature() override
1367d2b LemonLDAP::NG: Fix default scope name
420273f LemonLDAP::NG: Set username claim name in accordance with specs

(420273f allows automatic username provisioning for SSH and HTTP.)

Guilhem Moulin wrote:

David Ostrovsky wrote:

I don't know whether or not LL::NG provider supports bearer token
signature and basic authentication scheme, so that I commented out
new auth and signature ways in the code. If you can figure out,
whether or not new auth schemes are supported (or not), we can fix
those comments: [3], [4].

I guess LEMONLDAP_PROVIDER_PREFIX should be changed to "llng-oauth:" though.

Right, I've changed the prefix as you have suggested to: "llng-oauth:".

Unrelated to this issue but I didn't forget to back after the LL::NG resp. gerrit upgrade :-)

The version we're running at the moment can be found at https://git.libreoffice.org/infra/gerrit-oauth/+log/refs/tags/v3.1.3+tdf . There are some minors modifications to your code:

57e5804 LemonLDAP::NG: Remove getBearerSignature() override
1367d2b LemonLDAP::NG: Fix default scope name
420273f LemonLDAP::NG: Set username claim name in accordance with specs

Cool, I have uploaded all your fixes:

https://gerrit-review.googlesource.com/c/plugins/oauth/+/274444 LemonLDAP::NG: Set username claim name in accordance with specs [NEW]
  https://gerrit-review.googlesource.com/c/plugins/oauth/+/274445 LemonLDAP::NG: Fix default scope 
  https://gerrit-review.googlesource.com/c/plugins/oauth/+/274446 LemonLDAP::NG: Remove getBearerSignature() override [NEW]

Hopefully, you could switch to upstream once they are merged ;-)

Also note, that you wouldn't have to build them yourself, but could download from the GerritForge CI: [1].

[1] https://gerrit-ci.gerritforge.com/view/Plugins-stable-3.1/job/plugin-cfoauth-bazel-master-stable-3.1

David Ostrovsky wrote:

Cool, I have uploaded all your fixes:

Thanks!

Hopefully, you could switch to upstream once they are merged ;-)

Also note, that you wouldn't have to build them yourself, but could download from the GerritForge CI: [1].

Yup that's what we did before the switch to our own IdP :-) Eliminating the delta between the upstream version and what we run would indeed be welcome.

Guilhem Moulin wrote:

David Ostrovsky wrote:

Cool, I have uploaded all your fixes:

Thanks!

Hopefully, you could switch to upstream once they are merged ;-)

Also note, that you wouldn't have to build them yourself, but could download from the GerritForge CI: [1].

Yup that's what we did before the switch to our own IdP :-) Eliminating the delta between the upstream version and what we run would indeed be welcome.

All CLs are merged now, and CI build is green as well: [1].

[1] https://gerrit-ci.gerritforge.com/view/Plugins-stable-3.1/job/plugin-oauth-bazel-stable-3.1/19