A repository holding 3rd party component(s) enables by default the GitLab user check setting

Summary

When a project maintains upstream forks, these forks contain commits with users that are not users of the Elipse Gitlab instance. This kind of repositories requires regular upstream merges that pull external commits. Pushing this kind of an update (merge) to the Eclipse Gitlab instance fails due to the Check whether the commit author is a GitLab user being enabled by default. These are so-called third-party repositories and the solution is to simply disable this setting. The issue though persists and even if disabled, all the forks of it will enable this setting in the newly forked repository. We require user forks so we can handle merge requests through the common fork and merge request development flow.

Steps to reproduce

Fork https://gitlab.eclipse.org/eclipse/oniro-core/meta-zephyr repository in the namespace of a user. The newly created repository has the Check whether the commit author is a GitLab user setting enabled even though the parent (forked) repository doesn't.

What is the current bug behavior?

Check whether the commit author is a GitLab user is enabled for repositories hosting 3rd party components.

What is the expected correct behavior?

Repositories holding 3rd party components/forks should have the Check whether the commit author is a GitLab user disabled including for the repositories that fork further these projects.

Relevant logs and/or screenshots

remote: GitLab: Committer 'ABC' is not a member of team

• where ABC is a commiter from the forked repository.

AFAICT, the push rule setting Check whether the commit author is a GitLab user can be set on a project, group and GitLab-wide level.

In a forked repo the owner should be able to change the setting themself. So this sounds like an inconvenient extra step. I'm not sure if it justifies disabling the check on a group or GitLab-wide level.

It would be nice to change this setting specifically for forked repos. Maybe worth a feature request at gitlab.com?

@wbeaton any thoughts on this?

Thanks for looking into this @fgurr. Before we talk about how to deal with this, I'd like to stress a bit more the impact as this is a bit more than just an inconvenience. Also, I'm wondering if this restriction/configuration is needed at all.

The fork and merge development flow is standard in open-source making it a very common operation. As a matter of fact, only yesterday I got 5 requests from developers not knowing how to deal with this. The fact that the logs are unclear (to say the least) and their mapping to the GitLab configuration, make it especially confusing. Also, the fact that this check can be from GitLab configuration or from a custom git hook check (for example ECA) adds to the confusion making the debug process cumbersomely.

Now, this check basically adds a verify step in GitLab so that all commits have a GitLab account. Now we also have an ECA check which verifies that every commit/author has a signed ECA which means it has an Eclipse account. Given that the Eclipse account is used as SSO for GitLab, wouldn't that imply that the author/commit also has an associated Gitlab account?

I agree that unclear error messages or logs can make this confusing. As a quick fix I'd recommend to add instructions to the contributing section of the README.txt. It could also be added as a note to the repo description.

I don't know if the ECA check alone is enough and the Check whether the commit author is a GitLab user is more of a burden than a benefit.

@wbeaton please advise

assigned to @wbeaton

We currently have, I believe, two projects (Adoptium and Oniro) that maintain forks of third party content that, at least theoretically require periodic updates from upstream (in case you're wondering, we believe that this is consistent with the IP Policy). I believe that at least one project in our proposal queue will do something similar. The TL;DR: is that this is quickly becoming something that we're going to need to support.

So... if the question is "should we permit forks of third-party content to periodically pull/fetch from upstream?", the answer is yes. Both Oniro and Adoptium are good-to-go, but as other projects ask for this, we'll need approval from the EMO.

Note that this is related, I believe, to #42 (closed) in that keeping these forks separate from project content will reduce the risks associated with confusing who owns what (but that's another matter).

I opened a related issue a few months back, but for reasons that I forget, I decided to make that issue confidential. I'm thinking that we should move Infrazilla 468 here. It's concerned with adding metadata to ensure that we correctly track these third-party repository "live forks".

I think that I've answered the question. Let me know if I've missed something.

I don't know if the ECA check alone is enough and the Check whether the commit author is a GitLab user is more of a burden than a benefit.

@wbeaton please comment on the implied technical question "Do we want/need this GitLab feature to be enabled or is the ECA check good enough?"

So my explicit questions would be:

1. Is ECA check enough to cover the GitLab user check? See my rationale above.
2. Can we drop the GitLab user existence check as the default repository configuration?

@wbeaton please comment on the implied technical question "Do we want/need this GitLab feature to be enabled or is the ECA check good enough?"

The requirement is that contributions be covered by an ECA. In the absence of any other information, I don't think I care whether or not the user is known to GitLab.

I am concerned that focusing on this specific feature might make us miss the bigger picture. At least in the case of these third party forks, we need to be able to support the "fork and merge development flow" that @agherzan describes.

The end goal is indeed to support third-party components but this specific check is one that takes us a step closer to that. And by that, I mean that disabling the check will allow us to push code from third-party components as the history would not pass the check otherwise.

changed the description

The Check whether the commit author is a GitLab user is definitely not a requirement. It's totally possible that a user has a valid ECA on file without a Gitlab account.

I typically disable this setting for projects that I manage.

So if that is the case, could we disable this as a default configuration? If most of us (maybe everyone) disables it at the level of the repositories, we should make this the default and let only the exceptions (I'm still not sure what those are) enable it. Otherwise again, every single fork needs to manually disable this to work on 3rd party components.

I've disabled the check on the oniro group level and for all projects where it was not disabled already. New projects will no longer have this setting enabled.

It might still be enabled for forked projects due to the GitLab instance-wide setting.

@fgurr well, that's the most painful part of it. The oniro repositories don't change that much so creating new ones usually is a one-off while the forks are things that are dynamic based on dynamic contributors. That is the part that I'm more interested in.

It might still be enabled for forked projects due to the GitLab instance-wide setting.

Fred, do you know if we can disable this instance-wide setting? I don't see why we would need this with the ECA validator.

Fred, do you know if we can disable this instance-wide setting?

Yes, we can. I've disabled it now (https://gitlab.eclipse.org/admin/push_rule)

This setting is most likely still active for groups and projects that were created before this change and were not changed manually.

That is great. I have checked and the new repositories don't have this restriction anymore. Indeed, the already created one will but that is a one-off action we can deal with. Thanks once again! We can close this issue.

Thanks @fgurr!

added to epic &12 (closed)

removed from epic &12 (closed)

mentioned in issue #1017 (closed)

closed

mentioned in epic &12 (closed)

mentioned in issue #1525 (closed)

mentioned in issue #2081 (closed)