Branch protection is not enabling codeowners approval

[amimas]

I'm currently working on setting up GitlabForm to configure groups/projects in a GitLab Premium edition. I have the following snippet for GitlabForm configuration:

  ## API: https://docs.gitlab.com/ee/api/protected_branches.html
  ### GitLab UI: Projct -> Settings -> Repository -> Proected Branches
  branches:
    main:
      protected: true
      unprotect_access_level: 40 # Maintainer
      push_access_level: 0 # No access
      merge_access_level: 30 # Developer access
      code_owner_approval_required: true

GitlabForm is processing the above configuration and setting up branch protection on a branch named main but it's not processing code_owner_approval_required parameter. I understand this feature is not available in CE edition but thought this will probably be okay because of GitlabForm's raw parameter processing concept. But, it seems that's not happening in this case.

Unfortunately that puts me in a difficult situation. I was thinking of enabling "codeowners approval required" setting in branch protection manually. But GitLab doesn't have an api to edit a branch protection setting. You'll need to remove existing branch protection and then create a new one. So, even if I configure "codeowners approval required" setting for a branch, when GitlabForm runs, it overwrites my setting (due to the need for removing branch protection and adding a new one).

I'm not fully familiar with the GitlabForm's code base. I suspect changes are needed in the following function. But, I'm not sure how to make changes without impacting others using the CE edition. Any suggestions?

https://github.com/egnyte/gitlabform/blob/6190377542826a0cc1fec0026b78b0661187b68f/gitlabform/gitlab/branches.py#L22-L42

https://docs.gitlab.com/ee/api/protected_branches.html#protect-repository-branches

[gdubicki]

Hey, thanks for reporting this!

Our code for Protected branches API is a bit messy right now as we still support the deprecated API for it (for compatibility with the existing config syntax) as well as the new API, so it's not trivial to do it right... (We should clean this up in GitLabForm 2.x, when we will be able to drop the old config syntax. cc @kowpatryk )

However I did write a draft PR that should add support for your config example without breaking anything else: https://github.com/egnyte/gitlabform/pull/155/files

Could you test if it works for you, @amimas ? You would have to edit your GitLabForm installed app files. (I would release a pre-release version to PyPI for you, but we have a problem with Travis right now (see #156) and I won't fix it today. :( )

[amimas]

Hi @gdubicki - Thank you so much for the quick reply to this and the draft PR during this holiday time! I really appreciate it. I can manage with manually patching my environment. So, no worries about releasing to PyPI.

I'm giving feedback here in the issue instead of the PR because I think it will be easier to follow the discussions.

The changes in your draft PR worked great! I can now enable the option to require codeowners approval in branch protection. I did run into a different issue, which I wasn't able to test before. Below is my configuration snippet.

project_settings:
  my-group/regular-project-example:
    <<: *default-settings
  my-group/deploy-project-example:
    <<: *default-settings

    ## Customize/Overwrite default setting
    project_settings:
      auto_cancel_pending_pipelines: disabled
      ci_forward_deployment_enabled: false

    ## API: https://docs.gitlab.com/ee/api/protected_branches.html
    ### GitLab UI: Projct -> Settings -> Repository -> Proected Branches
    branches:
      production:
        protected: true
        unprotect_access_level: 40 # Maintainer access
        push_access_level: 0 # No access
        allowed_to_merge:
          - group_id: 27 # id of the user group named 'users-ops' who are allowed to merge
        code_owner_approval_required: true

      integration:
        protected: true
        unprotect_access_level: 40 # Maintainer access
        push_access_level: 30 # Developer access
        merge_access_level: 30 # Developer access

      staging:
        protected: true
        unprotect_access_level: 40 # Maintainer access
        push_access_level: 30 # Developer access
        merge_access_level: 30 # Developer access

I'm getting the following error when running Gitlabform.

GitLabForm version: 1.18.3.2 (the latest)
>>> Processing ALL groups and projects defined in config
*** # of groups to process: 1
*** # of projects to process: 3
> (1/1) Processing: my-group/playground
* [1/3] Processing: my-group/deploy-project-example
+++ Error while processing 'my-group/deploy-project-example'
Traceback (most recent call last):
  File "/gitlabform/gitlabform/gitlabform/core.py", line 315, in process_all
    self.project_processors.process_project(
  File "/gitlabform/gitlabform/gitlabform/processors/project/__init__.py", line 57, in process_project
    processor.process(project_and_group, configuration, dry_run)
  File "/gitlabform/gitlabform/gitlabform/processors/util/decorators.py", line 42, in method_wrapper
    return method(self, project_and_group, SafeDict(configuration), dry_run)
  File "/gitlabform/gitlabform/gitlabform/processors/abstract_processor.py", line 34, in process
    self._process_configuration(project_or_project_and_group, configuration)
  File "/gitlabform/gitlabform/gitlabform/processors/project/branches_processor.py", line 13, in _process_configuration
    self.__branch_protector.protect_branch(
  File "/gitlabform/gitlabform/gitlabform/processors/util/branch_protector.py", line 42, in protect_branch
    configuration["branches"][branch]["merge_access_level"],
KeyError: 'merge_access_level'

I believe the error is coming up when Gitlabform tries to process the branch protection for branch named production. The error message is not very clear butt I beleive it's expecting merge_access_level key in the configuration. For this branch, my workflow requires that only certain users (those in the users-ops group) will be able to merge. That's why I have allowed_to_merge with specific group id.

I think similar to codeowners approval option, the above config is not being recognized because branch protection does not have the raw parameter processing feature. Please let me know if I should open a separate issue for this.

Thanks again for your help and happy holidays!

[amimas]

I just found an existing issue #99, which seems to match the problem I mentioned in the previous comment (allowing merge/push based on user or group). This feature currently is not available.

So, I think we can close this issue once #155 is merged and released. That PR adds the ability to specify the option of requiring code owners approval for branch protection and worked fine in my test.

Thanks again!!

[pa-eps]

@gdubicki - I noticed the corresponding PR (#155 ) has been merged. Thanks so much!! Does that mean the CI issue has been resolved? Just wondering if the update is released or only the change has been merged.

[pa-eps]

Just saw the Release page of this repo. Should've checked this first.

Looks like v1.19.0 has this feature. Couldn't find the corresponding docker image in docker hub. Is that a separate release process?

[gdubicki]

Sorry @pa-eps , our CI system was incomplete during v.1.19.0 release. See https://github.com/egnyte/gitlabform/releases/tag/v1.19.0 for more info.

But we have just released v.1.20.0 that has this feature AND a proper DockerHub image.

So I am finally closing this as resolved, @amimas . :)

[amimas]

Thanks again for adding this feature quickly. It's been extremely useful.