From f772bcda885fc0df1c06eb44362e477d32b10eeb Mon Sep 17 00:00:00 2001 From: David Phillips Date: Tue, 9 Sep 2025 05:02:40 +1200 Subject: [PATCH] gitlab_protected_branch: refactor, add `allow_force_push`, `code_owner_approval_required` (#10795) * gitlab_protected_branch: fix typo * gitlab_protected_branch: lump parameters into options dictionary Hardcoding parameter lists gets repetitive. Refactor this module to use an options dictionary like many other gitlab_* modules. This makes it cleaner to add new options. * gitlab_protected_branch: update when possible Until now, the module deletes and re-creates the protected branch if any change is detected. This makes sense for the access level parameters, as these are not easily mutated after creation. However, in order to add further options which _can_ easily be updated, we should support updating by default, unless known-immutable parameters are changing. * gitlab_protected_branch: add `allow_force_push` option * gitlab_protected_branch: add `code_owner_approval_required` option * gitlab_protected_branch: add issues to changelog * Update changelog. --------- Co-authored-by: Felix Fontein --- ...orce_push-code_owner_approval_required.yml | 3 + plugins/modules/gitlab_protected_branch.py | 87 ++++++++++++------- .../modules/test_gitlab_protected_branch.py | 44 ++++++++-- 3 files changed, 99 insertions(+), 35 deletions(-) create mode 100644 changelogs/fragments/10795-gitlab_protected_branch-add-allow_force_push-code_owner_approval_required.yml diff --git a/changelogs/fragments/10795-gitlab_protected_branch-add-allow_force_push-code_owner_approval_required.yml b/changelogs/fragments/10795-gitlab_protected_branch-add-allow_force_push-code_owner_approval_required.yml new file mode 100644 index 0000000000..ed4d4d78e8 --- /dev/null +++ b/changelogs/fragments/10795-gitlab_protected_branch-add-allow_force_push-code_owner_approval_required.yml @@ -0,0 +1,3 @@ +minor_changes: + - gitlab_protected_branch - add ``allow_force_push``, ``code_owner_approval_required`` (https://github.com/ansible-collections/community.general/pull/10795, https://github.com/ansible-collections/community.general/issues/6432, https://github.com/ansible-collections/community.general/issues/10289, https://github.com/ansible-collections/community.general/issues/10765). + - gitlab_protected_branch - update protected branches if possible instead of recreating them (https://github.com/ansible-collections/community.general/pull/10795). diff --git a/plugins/modules/gitlab_protected_branch.py b/plugins/modules/gitlab_protected_branch.py index 4a3b7177ee..9baa9d393e 100644 --- a/plugins/modules/gitlab_protected_branch.py +++ b/plugins/modules/gitlab_protected_branch.py @@ -58,6 +58,16 @@ options: default: maintainer type: str choices: ["maintainer", "developer", "nobody"] + allow_force_push: + description: + - Whether or not to allow force pushes to the protected branch. + type: bool + version_added: '11.3.0' + code_owner_approval_required: + description: + - Whether or not to require code owner approval to push. + type: bool + version_added: '11.3.0' """ @@ -106,27 +116,43 @@ class GitlabProtectedBranch(object): except Exception as e: return False - def create_protected_branch(self, name, merge_access_levels, push_access_level): - if self._module.check_mode: - return True - merge = self.ACCESS_LEVEL[merge_access_levels] - push = self.ACCESS_LEVEL[push_access_level] - self.project.protectedbranches.create({ + def create_or_update_protected_branch(self, name, options): + protected_branch_options = { 'name': name, - 'merge_access_level': merge, - 'push_access_level': push - }) + 'allow_force_push': options['allow_force_push'], + 'code_owner_approval_required': options['code_owner_approval_required'], + } + protected_branch = self.protected_branch_exist(name=name) + changed = False + if protected_branch and self.can_update(protected_branch, options): + for arg_key, arg_value in protected_branch_options.items(): + if arg_value is not None: + if getattr(protected_branch, arg_key) != arg_value: + setattr(protected_branch, arg_key, arg_value) + changed = True + if changed and not self._module.check_mode: + protected_branch.save() + else: + # Set immutable options only on (re)creation + protected_branch_options['merge_access_level'] = options['merge_access_levels'] + protected_branch_options['push_access_level'] = options['push_access_level'] + if protected_branch: + # Exists, but couldn't update. So, delete first + self.delete_protected_branch(name) + if not self._module.check_mode: + self.project.protectedbranches.create(protected_branch_options) + changed = True - def compare_protected_branch(self, name, merge_access_levels, push_access_level): - configured_merge = self.ACCESS_LEVEL[merge_access_levels] - configured_push = self.ACCESS_LEVEL[push_access_level] - current = self.protected_branch_exist(name=name) - current_merge = current.merge_access_levels[0]['access_level'] - current_push = current.push_access_levels[0]['access_level'] - if current: - if current.name == name and current_merge == configured_merge and current_push == configured_push: - return True - return False + return changed + + def can_update(self, protected_branch, options): + # these keys are not set on update the same way they are on creation + configured_merge = options['merge_access_levels'] + configured_push = options['push_access_level'] + current_merge = protected_branch.merge_access_levels[0]['access_level'] + current_push = protected_branch.push_access_levels[0]['access_level'] + return ((configured_merge is None or current_merge == configured_merge) and + (configured_push is None or current_push == configured_push)) def delete_protected_branch(self, name): if self._module.check_mode: @@ -142,6 +168,8 @@ def main(): name=dict(type='str', required=True), merge_access_levels=dict(type='str', default="maintainer", choices=["maintainer", "developer", "nobody"]), push_access_level=dict(type='str', default="maintainer", choices=["maintainer", "developer", "nobody"]), + allow_force_push=dict(type='bool'), + code_owner_approval_required=dict(type='bool'), state=dict(type='str', default="present", choices=["absent", "present"]), ) @@ -174,23 +202,24 @@ def main(): gitlab_version = gitlab.__version__ if LooseVersion(gitlab_version) < LooseVersion('2.3.0'): - module.fail_json(msg="community.general.gitlab_proteched_branch requires python-gitlab Python module >= 2.3.0 (installed version: [%s])." + module.fail_json(msg="community.general.gitlab_protected_branch requires python-gitlab Python module >= 2.3.0 (installed version: [%s])." " Please upgrade python-gitlab to version 2.3.0 or above." % gitlab_version) this_gitlab = GitlabProtectedBranch(module=module, project=project, gitlab_instance=gitlab_instance) p_branch = this_gitlab.protected_branch_exist(name=name) - if not p_branch and state == "present": - this_gitlab.create_protected_branch(name=name, merge_access_levels=merge_access_levels, push_access_level=push_access_level) - module.exit_json(changed=True, msg="Created the proteched branch.") - elif p_branch and state == "present": - if not this_gitlab.compare_protected_branch(name, merge_access_levels, push_access_level): - this_gitlab.delete_protected_branch(name=name) - this_gitlab.create_protected_branch(name=name, merge_access_levels=merge_access_levels, push_access_level=push_access_level) - module.exit_json(changed=True, msg="Recreated the proteched branch.") + options = { + "merge_access_levels": this_gitlab.ACCESS_LEVEL[merge_access_levels], + "push_access_level": this_gitlab.ACCESS_LEVEL[push_access_level], + "allow_force_push": module.params["allow_force_push"], + "code_owner_approval_required": module.params["code_owner_approval_required"], + } + if state == "present": + changed = this_gitlab.create_or_update_protected_branch(name, options) + module.exit_json(changed=changed, msg="Created or updated the protected branch.") elif p_branch and state == "absent": this_gitlab.delete_protected_branch(name=name) - module.exit_json(changed=True, msg="Deleted the proteched branch.") + module.exit_json(changed=True, msg="Deleted the protected branch.") module.exit_json(changed=False, msg="No changes are needed.") diff --git a/tests/unit/plugins/modules/test_gitlab_protected_branch.py b/tests/unit/plugins/modules/test_gitlab_protected_branch.py index 1162d8a351..8141782fbc 100644 --- a/tests/unit/plugins/modules/test_gitlab_protected_branch.py +++ b/tests/unit/plugins/modules/test_gitlab_protected_branch.py @@ -47,6 +47,12 @@ except ImportError: with_httmock = _dummy +class MockProtectedBranch(): + def __init__(self, merge_access_levels, push_access_levels): + self.merge_access_levels = merge_access_levels + self.push_access_levels = push_access_levels + + class TestGitlabProtectedBranch(GitlabModuleTestCase): @with_httmock(resp_get_project_by_name) @with_httmock(resp_get_user) @@ -66,14 +72,40 @@ class TestGitlabProtectedBranch(GitlabModuleTestCase): rvalue = self.moduleUtil.protected_branch_exist(name="master") self.assertEqual(rvalue, False) - @with_httmock(resp_get_protected_branch) - def test_compare_protected_branch(self): - rvalue = self.moduleUtil.compare_protected_branch(name="master", merge_access_levels="maintainer", push_access_level="maintainer") + def test_can_update_zero_delta(self): + protected_branch = MockProtectedBranch( + merge_access_levels=[{"access_level": 40}], + push_access_levels=[{"access_level": 40}], + ) + options = { + "merge_access_levels": 40, + "push_access_level": 40 + } + rvalue = self.moduleUtil.can_update(protected_branch, options) self.assertEqual(rvalue, True) - @with_httmock(resp_get_protected_branch) - def test_compare_protected_branch_different_settings(self): - rvalue = self.moduleUtil.compare_protected_branch(name="master", merge_access_levels="developer", push_access_level="maintainer") + def test_can_update_no_configured(self): + protected_branch = MockProtectedBranch( + merge_access_levels=[{"access_level": 40}], + push_access_levels=[{"access_level": 40}], + ) + options = { + "merge_access_levels": None, + "push_access_level": None + } + rvalue = self.moduleUtil.can_update(protected_branch, options) + self.assertEqual(rvalue, True) + + def test_can_update_different_settings(self): + protected_branch = MockProtectedBranch( + merge_access_levels=[{"access_level": 40}], + push_access_levels=[{"access_level": 40}], + ) + options = { + "merge_access_levels": 40, + "push_access_level": 30 + } + rvalue = self.moduleUtil.can_update(protected_branch, options) self.assertEqual(rvalue, False) @with_httmock(resp_get_protected_branch)