MR approvers user/group has no effect (REST API change?)

[gitlabform]

MR approvers user/group has no effect (REST API change?)

[smcgregor83]

I am seeing the same behaviour running against 12.4.2-ee.

[gdubicki]

I have come up with a temporary workaround for this bug:

1. Create GitLab group such as "approvers_group_a" and in GitLabForm assign them members.
2. Assign ONLY THIS GROUP to your appropriate projects as approver groups with GitLabForm.

Example:

# (...)
group_settings:
  approver_group_a:
    enforce_group_members: true
    group_members:
      owner:
        access_level: 50 # you have to have at least 1 Owner per group, GitLab requires that
      user1: &developer_access
        access_level: 30
      user2: *developer_access
      user3: *developer_access

project_settings:
  some_group/project_a:
    merge_requests:
      approvals:
        approvals_before_merge: 1
        reset_approvals_on_push: false
        disable_overriding_approvers_per_merge_request: true
      approver_groups:
        - approver_group_a

I know that this is not pretty but it should work until this bug is resolved.

UPDATE: wait a moment, I forgot that there is no support for managing group members in GitLabForm yet! 😅 However implementing it now will be very simple, I will do it really soon. 😉 Stay tuned!

UPDATE 2: I updated the syntax in the example above to match the syntax added in v1.11.0 / #83.

UPDATE 3: I updated the syntax in the example above to match the syntax added in v1.13.0.

[weakcamel]

Many many thanks @gdubicki

I can confirm that the latest version works within limitations caused by https://gitlab.com/gitlab-org/gitlab/issues/198770 (when I tried, I got the lovely warning about it.

I've voted for the issue and now commented on it as well, fingers crossed it gets some interest from Gitlab people soon.

[weakcamel]

Works like a charm!

👍 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍

[weakcamel]

The new API - if powerful - is sadly much more complex and involves creating/updating approval rules (which have their own ids).

https://docs.gitlab.com/ee/api/merge_request_approvals.html#create-project-level-rule

[weakcamel]

On a related note: https://gitlab.com/gitlab-org/gitlab/issues/4816

TL;DR: with MR approvals enabled for a project, Gitlab assumes that all Developers should be notified on any MR.

Gitlabform could be the most feasible workaround for this spammy behaviour.

[gdubicki]

Hi, thank you for reporting this and sorry for the trouble. I will try to get this fixed in the next 2-3 weeks.

[gdubicki]

I have tested this some time ago on 12.5.4-ee and have not noticed this issue.

I have also tested this with the current 12.6.2-ee in https://travis-ci.com/egnyte/gitlabform/jobs/272436690 (done locally for now, will finish on Travis within a few minutes) and the problem does not exist there anymore.

Please see my test setup, configs and assertions in https://github.com/egnyte/gitlabform/pull/75/files#diff-c92a09cbae0c182205d0cbe7f414ff33 .

Can you please verify if this problem persists with GitLab >= 12.5.4, @weakcamel and @smcgregor83 ?

Update: sorry, it turns out that the issue does appear in my new integration tests, but I can’t reproduce it manually yet. I will get back to you when I have more info.

[gdubicki]

Ok, so my latests test confirm that there is an issue if you have both users AND groups set as approvers and change any of these lists, for both 12.5.4 and 12.6.2.

(There is NO such problem if you ONLY have users OR only groups as approvers and change them.)

We need to report this to GitLab as even if this API is deprecated it should not have such bugs.

[weakcamel]

I've noticed a very inconsistent behaviour here:

• if there is no validation rule, gitlabform run doesn't apply anything, no matter what
  • the way out from here is to create (I used python-gitlab from PyPI) a dummy validation rule with a user id and a group id
• if I add a validation rule manually and it contains a group only - my update has no effect, which confirms your observations @gdubicki
• occasionally by re-running gitlabform on the project and manually tweaking the same validation rule - repeated a few times - seems to unblock it sometimes, although I've not managed to make it repeatable yet

So yes, I suspect that Gitlab implemented something which used to be consistent with their previous design (or most common setup) for the deprecated API...

[gdubicki]

Sorry for the delay.

I started to work on play with (as I am NOT working overhours here ;) implementing the new API but I have very strange results for now.

I suspect that I may be hitting https://gitlab.com/gitlab-org/gitlab/issues/36014 , https://gitlab.com/gitlab-org/gitlab/issues/195793 or some similar GitLab bug.

[gdubicki]

According to my tests on the latest GitLab version, calls to the new API only set the first LAST user and the first LAST group from the lists provided in https://docs.gitlab.com/ee/api/merge_request_approvals.html#update-project-level-rule requests .

However I will check how it is done in the GitLab provider for Terraform, which almost has this feature now too (https://github.com/terraform-providers/terraform-provider-gitlab/pull/208). Maybe I am doing something wrong?

[gdubicki]

I think that the GitLab provider for Terraform’s PR tests do not cover the case where I detected the problem. I reported that in that PR already.

I also reported the bug to GitLab, here: https://gitlab.com/gitlab-org/gitlab/issues/198770

[gdubicki]

I have read GitLab code and if I got it right then the tests for the API there do NOT cover case with more than 1 user or more than 1 group set as approvers: https://gitlab.com/gitlab-org/gitlab/blob/master/ee/spec/requests/api/project_approval_rules_spec.rb

Setting more users or groups in the GitLab web UI DOES work but it is using a different code than that API - there is a ticket open to switch it to use the public API here: https://gitlab.com/gitlab-org/gitlab/issues/13574

But I find it hard to believe that it could be possible that this bug in fact exists and no one noticed it for such a long time since the API is available, so I think that I rather have a bug in my code. 😞

[gdubicki]

Unfortunately python-gitlab does not have this implemented so I can’t check how they have dealt with it: python-gitlab/python-gitlab#877

[gdubicki]

Version 1.11.0 is just being built and pushed to PyPI. I contains the group members adding/changing access level (I am trying to be precise here as we cannot yet remove members with gitlabform). So the above workaround should work now. :)

[gdubicki]

Version 1.12.0 is just being built and pushed to PyPI. It contains new Merge Request Approvals API implementation, so the code above should work now.

[gdubicki]

I am just releasing 1.13.0 with full group membership management, including removing users from a group. But note that with that you HAVE TO have at least one Owner in each group, GitLab requires that! Please see updated syntax in the comment above.

[gdubicki]

@weakcamel and everyone involved: I am happy to report that the issue is resolved in v1.14.0 that has been released just now.

(But less-than-happy to learn that it was caused by something that I consider a GitLab issue more than a GitLabForm issue - see #90 and the linked GitLab issue for more info.)

[weakcamel]

@gdubicki this is amazing! I'm definitely going to give it a try as soon as I can :)