Allow breaking configuration inheritence

[amimas]

I've started using the v2 (2.10.0). Lots of improvements have been made from v1. This post is a feature request for being able to break config inheritance.

Below is an example config of what it might look like for a typical group.

config_version: 2
gitlab:
  api_version: 4

projects_and_groups:
  my-group/*:
    # Settings for ALL projects under 'my-group' group and the group itself
    group_settings:
      # Settings of the parent group
      description: Contains projects for example product.
    members:
      # Members for ALL projects.
      enforce: true
      groups:
        user-group-1:
          group_access: 30  # Developer role
        user-group-2:
          group_access: 30  # Developer role
    project_settings:
      # Settings of ALL projects
      visibility: internal
      issues_access_level: enabled
      request_access_enabled: false

  my-group/special-private-project:
    # This is a special project. It should be private and have specific user permission.
    project_settings:
      visibility: private
    members:
      # Only following users should have permission. Do not inherit permissions from common settings
      groups:
        user-group-2:
          group_access: 30
       users:
          john:
            access_level: 40

With the above config, the project my-group/special-private-project gets members from the common settings (configured in my-group/* section) in addition to what's configured in that project directly.

I think it makes sense that this is how it works by default. It'd be nice if I could specify in that project specific config to not inherit common settings so that I could manage "exception" scenarios. In the above example, I want to break the inheritance of the members section only but inherit other settings. I guess it could be perceived that someone might want to break the inheritance in other areas/sections as well

Right now my work-around is following. Only problem with this work-around is that I need to list out every single project in the config just so that I could have exception for a single project. So, I'm not able to take full advantage/power of gitlabform's config.

config_version: 2
gitlab:
  api_version: 4

default_project_access_by_usergroup: &default-project-access-by-usergroup
  user-group1:
    group_access: 30  # Developer
  user-group2:
    group_access: 30  # Developer

projects_and_groups:
  my-group/*:
    # Settings for ALL projects under 'my-group' group and the group itself
    group_settings:
      # Settings of the parent group
      description: Contains projects for example product.
    members:
      # Members for ALL projects.
      # Leave this section empty so that each project can specify what permission to be set.
      # Otherwise we cannot configure exception using gitlabform
      enforce: true
    project_settings:
      # Settings of ALL projects
      visibility: internal
      issues_access_level: enabled
      request_access_enabled: false

  my-group/project1:
    members:
      groups:
        <<: *default-project-access-by-usergroup

  my-group/project2:
    members:
      groups:
        <<: *default-project-access-by-usergroup

  my-group/special-private-project:
    # This is a special project. It should be private and have specific user permission.
    project_settings:
      visibility: private
    members:
      # Only following users should have permission. Do not inherit permissions from common settings
      groups:
        user-group-2:
          group_access: 30
       users:
          john:
            access_level: 40

[gdubicki]

Hi @amimas!

Sorry for a late reply. We actually run into this need too, so I will probably implement it soon. Unless you’d like to give it a try?

[amimas]

Hi @gdubicki - Thanks for acknowledging this request. I believe my colleague @ep-linden can look into implementing it. Can you give us some guidance? What should we name this config and any suggestions how to implement it? I imagine this is something that should be available at every level of the configs (i.e. project_settings, members, merge_requests, etc.)

[gdubicki]

As we already have special keywords like delete: true, enforce: true I suggest another single-word keyword - inherit: false.

So for your example case, the config (shortened to include only the membership) would look like this:

projects_and_groups:
  my-group/*:
    members:
      enforce: true
      groups:
        user-group-1:
          group_access: developer
        user-group-2:
          group_access: developer
 
  my-group/special-private-project:
    members:
      inherit: false
      enforce: true
      groups:
        user-group-2:
          group_access: developer
      users:
        john:
          access_level: maintainer

As you suggested, the inherit: false would apply only to a single section where it is included. If you want to break inheritance for more sections then you should put this key and value explicitly in each one of them.

Also breaking inheritance would only work from the top to the specific level.

Example: imagine that we have a config a for */*, config b for group/* and config c for group/project.
Without any inheritance breaking the group/project has config a+b+c (where + is merging).

If you set inherit: false in the config a, then we should throw an error as it would effectively not do anything as there is nothing to not inherit from and it would be misleading to allow such config.

If you set inherit: false in the config b, then group/project has only config b+c (a is not inherited).

If you set inherit: false in the config c, then group/project has only config c (a and b are not inherited).

Does that make sense, @amimas @ep-linden?

If so, then I think that we only need to update the merge_configs() method to implement this (+ some tests, of course). This should not be hard, just keep in mind that this method operates on a whole config for f.e. group or project, not just a single section.

[amimas]

Thanks for all that details @gdubicki . That makes sense to me and is also what I was thinking how this feature would work.

Question on the config keyword, inherit. Could this be confused with how inheritance work in gitlab (i.e. group -> sub-group/project), etc. For example: in the case of not inheriting member configuration, would it seem like we're trying to break the configuration in gitlab? For members, this is not possible. If a user is added at the group level, you can't remove them from a project.

Maybe I'm over thinking this? Perhaps it's not going to be confusing because the users of gitlabform is expected to have certain knowledge about how gitlab works.

Thoughts?

[gdubicki]

Question on the config keyword, inherit (..)

Yes, there is a slight naming clash here with gitlab's terminology and this is not perfect. But "inheritance" is also a term used in gitlabform from the start and I frankly don't have any alternative ideas.. Do you have?

Anyway, this is something we can agree on later (maybe someone else will chip in?) - it should not block you from creating the PR. :)

[amimas]

But "inheritance" is also a term used in gitlabform from the start

That's a good point. I couldn't think of an alternative either. And I agree this can be improved later, if needed.

[jccl]

Yes, there is a slight naming clash here with gitlab's terminology and this is not perfect. But "inheritance" is also a term used in gitlabform from the start and I frankly don't have any alternative ideas.. Do you have?

In the Maven POM world, the merging of XML behaviour that works similar to what's being requested here is controlled by an attribute called combine.self. By default the attribute value is merge, but you can set it to override to have the desired behaviour of overriding the inheritance.

How's something along the lines of combine_self for the key word to avoid clashing?

Here's how it might look:

projects_and_groups:
  my-group/*:
    members:
      groups:
        user-group-1:
          group_access: developer
 
  my-group/special-private-project:
    members:
      combine_self: override
      enforce: true
      users:
        john:
          access_level: maintainer

[ep-linden]

@gdubicki I have started taking a look at this. I will update here accordingly.

[gdubicki]

Hey @ep-linden, I got some email notify but I don’t see your comment here.. Anyway I think it would be easiest to discuss this further if you would share even a draft PR with your code so please don’t hesitate if you have it even half-baked. :)

[ep-linden]

@gdubicki I had a few design questions, but I thought I'd just test it out first so I deleted it. I'll open up a draft PR today.

[gdubicki]

We have released a pre-release v2.11.0b1 with this feature. 🥳 Thanks again for all your work on this, @ep-linden and your cooperation @amimas!

Please test it out and check if all works without issues so we can release a final release.

We can still change the keyword from inherit: false to something else before the final release. I like the values of @jccl's proposal - merge (default) and override. But I am not a fan of the combine_self keyword, to be frank. 😅

...however as in the following months we plan to release gitlabform v3 where most probably we will use YAML tags for this feature anyway. Together with @jimisola in #331 we discussed having !do-not-inherit and!do-not-allow-inherit-overwrite tags, where the former does what we are discussing here while the latter explicitly disallows doing it for some configurations. So perhaps there is no point in changing this keyword now anyway?

[gdubicki]

I've noticed that when we run gitlabform with ALL_DEFINED it seems that breaking inheritance does not work. :/ I have looked at the code but I can't see why it would make a difference. Have you noticed that too, @amimas @ep-linden ?

[ep-linden]

Oh, interesting, I've been primarily testing the break-inheritance flag running GitLab with ALL_DEFINED and it has been working fine. I'll test it again.

[ep-linden]

I did test it out again, and it's working fine with ALL_DEFINED.

[gdubicki]

There definitely was a bug, I fixed it (or rather worked around it, really :( ) and released v2.11.0b3 with the fix now. See the message of the commit above for more info, @ep-linden.

[ep-linden]

@gdubicki I just saw the changes. Would you be able to provide what you did, so I can reproduce it locally?

[gdubicki]

Sorry @ep-linden, but as I have written in the 652b343 commit message, I was not able to reproduce the bug with unit or acceptance tests, only with my company's 1.6k LOC real-world config and I cannot share that...

[gdubicki]

The final release v2.11.0 with this feature is out. Thanks again for the cooperation, @ep-linden and @amimas!

[ep-linden]

Sorry @ep-linden, but as I have written in the 652b343 commit message, I was not able to reproduce the bug with unit or acceptance tests, only with my company's 1.6k LOC real-world config and I cannot share that...

Oh okay, that's what I was wondering. Obviously won't ask for your company's config haha. Thanks, Greg!

[lfvjimisola]

Is this only working for "members" or other settings as well?
If not, I think that it needs to be more clear for users, e.g. "Allow breaking configuration inheritence for members".

PS. Also, inheritance seems to misspelt in the subject.

[gdubicki]

This should be working for everything, any part of the configuration, @lfvjimisola.

[amimas]

It'd probably be helpful if the inherit flag is used in the sample config.yaml file. I guess that was missed in the PR that added the feature.

@ep-linden - do you think you can add couple examples?

[ep-linden]

Good catch, I'll do that.

[lfvjimisola]

Are there any examples available? I'm hoping that this could help with an issue we have.

We have a bunch of projects under TG/* for which set options related to repositories (project_settings, project_push_rules, merge_requests). However, for some projects under TG/* we've disabled the Git Repositories (we only use the Issue functionality). When we run gitlabform we then get:

'{"message":{"project_feature.merge_requests_access_level":["cannot have higher visibility level than repository access level"]}}'

Can I use the inherit flag explicitly for these projects to avoid the error?

[amimas]

Could you share a snippet of the config being used?

[lfvjimisola]

Interesting that GitHub doesn't allow yml files to be uploaded.

Our config.yml is attached. No imagine that one of the projects under sysdev/... have the repository functionality disabled (as per screenshot).

config.yml.txt