Child group settings are not being inherited from the parent group in the output file

[ep-linden]

Describe the bug

A group contains group_settings a subgroup needs as well. However, the subgroup does not inherit those settings. At least in the dry run, we don't see the inheritance.

parent/*:
    group_settings:
      project_creation_level: maintainer
      subgroup_creation_level: owner
      visibility: internal

parent/subgroup/*:
    group_settings:
      project_creation_level: developer

GitLabForm version

🏗 GitLabForm version: 2.11.1 = the latest stable 😊

GitLab version

GitLab Enterprise Edition 14.10.2-ee

[amimas]

@ep-linden - to be clear, in that example, the effective config for parent/subgroup group setting does not configure additional settings mentioned in parent group config. Is that right?

[ep-linden]

Yup, that is correct.

      subgroup_creation_level: owner
      visibility: internal

These two do not get inherited.

[ep-linden]

I checked if the break-inheritance functionality may have broken this, but I tried the exact same config in release 2.10 and it was the same issue.

It could be possible, that it isn't being printed properly. Will double-check.

[gdubicki]

In the tests inheritance works like it should, please check out my latest commit where I specifically made the subgroup effective group-level settings tests use the data from this issue.

So if it doesn't work when generating the dry-run config or when making actual changes, then the bug seems to be someplace else.

[ep-linden]

Hi Greg, the tests pass, and I know why they passed. It is because the effective config is being retrieved by the get_effective_subgroup_config function.

However, if we actually look at how and where get_effective_subgroup_config is being called, you'll see that it is in get_effective_config_for_project where the call is being made and not within groups, so the effective config is not being processed. I think the latest changes to MR #385 should fix that and improve the validation function. Please check it out when you have time.

[gdubicki]

Thanks for the fix, @ep-linden !
It has been pre-released in v3 RC3 released today so it also will be fixed in the final v3 that will be released within a few days. We can close this issue then.

[gdubicki]

Hi @ep-linden!

After we merged #385 I updated the upgrade guide to add info how to use inherit: false to keep the old app behavior.

However, when I tried to apply this recommendation myself I got an error:

 🏗  GitLabForm version: 3.0.0rc3 = pre-release 🤩 (the latest stable is 2.12.0)
 >>> Getting ALL_DEFINED groups and projects...
 Error: The inheritance-break flag set in "delphi/admin-apps" is invalid
 because it has no higher level setting to inherit from.

The relevant config:

# we don't use common-level config

    delphi/*:
      project_settings:
        visibility: internal
      branches:
        develop: *protected_and_devs_CAN_push_and_merge
        patch: *protected_and_devs_CAN_push_and_merge
        gce: *protected_and_devs_CANNOT_push_or_merge
        release: *protected_and_devs_CANNOT_push_or_merge
        master: *protected_and_devs_CANNOT_push_or_merge

    delphi/admin-apps/*:
      inherit: false
      branches:
        master: *protected_and_devs_CANNOT_push_but_CAN_merge
      merge_requests:
        approvals:
          approvals_before_merge: 2
          reset_approvals_on_push: false
          disable_overriding_approvers_per_merge_request: true
          merge_requests_author_approval: false

So I suspected that we actually can't put inherit: false in this location so I added a test in #393 and it did fail ...but not with the error like above, but with a config that did inherit the config. :|

I am a bit confused with what is happening here, can you please check it out?

[ep-linden]

Interesting, I'll take a look.

[ep-linden]

I figured it out. The test and local run results are different because we use 'get_effective_config_for_group' in the test, but the error is caught here, which is from projects_and_groups.py. With the current implementation of inheritance break, I didn't intend on breaking inheritance like how you have set it.

[gdubicki]

Thanks @ep-linden. The current implementation is fine, I have updated the upgrade guide to put the key under each section.

[gdubicki]

Closing as v3 with the fix has been released.