feat: add github_repo_permission module and test #10767

danielino · 2025-08-30T12:49:17Z

SUMMARY

Add a new Ansible module community.general.github_repo_permission to manage repository permissions for an organization team (by slug) or a user (collaborator) on GitHub. The module supports both built-in permissions (pull, triage, push, maintain, admin) and custom repository roles (GitHub Enterprise Server). It is idempotent, supports check_mode, and includes verification with small retries to account for eventual consistency on GHE.

ISSUE TYPE

New Module/Plugin Pull Request

COMPONENT NAME

github_repo_permission

ADDITIONAL INFORMATION

Motivation:
- Provide a Terraform-like capability (similar to github_team_repository) directly within Ansible to grant/revoke repo permissions for teams and users.
Key features:
- Subject can be a team (team_slug) or user (username) for a given repository (owner/repo).
- Built-in permissions and custom org roles (GHE). Accepts synonyms read→pull, write→push.
- Idempotency across built-ins and custom roles; case-insensitive team slug matching.
- Verification with short retries; relaxed verification for custom roles when GHE APIs do not expose role_name.
- check_mode fully supported; access_token marked no_log.
API notes:
- Team access:
  - PUT/DELETE /orgs/{org}/teams/{team_slug}/repos/{owner}/{repo}
  - GET /orgs/{org}/teams/{team_slug}/repos/{owner}/{repo}
  - GET /repos/{owner}/{repo}/teams
- User access:
  - PUT/DELETE /repos/{owner}/{repo}/collaborators/{username}
  - GET /repos/{owner}/{repo}/collaborators/{username}/permission
- Custom roles discovery:
  - GET /orgs/{org}/custom-repository-roles
- Headers: Accept: application/vnd.github+json, X-GitHub-Api-Version: 2022-11-28
GHE guidance:
- Use api_url ending with /api/v3. A warning is emitted if /api/v4 (GitLab-style) is detected.

Examples:

Grant push to a team:

- community.general.github_repo_permission:
    access_token: "{{ github_token }}"
    repository: myorg/myrepo
    team_slug: backend
    permission: push
    state: present

Ensure a user has triage:

- community.general.github_repo_permission:
    access_token: "{{ github_token }}"
    repository: myorg/myrepo
    username: octocat
    permission: triage
    state: present

Revoke team access:

- community.general.github_repo_permission:
    access_token: "{{ github_token }}"
    repository: myorg/myrepo
    team_slug: contractors
    state: absent

Signed-off-by: Daniele Marcocci <[email protected]>

Copilot

Pull Request Overview

This PR introduces a new Ansible module github_repo_permission for managing team and user permissions on GitHub repositories, providing Terraform-like capabilities within Ansible to grant/revoke repository access.

Adds comprehensive GitHub API integration with support for both built-in permissions and custom roles
Implements idempotent operations with proper verification and retry logic for eventual consistency
Includes full test coverage for teams, users, built-in permissions, and custom roles

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
plugins/modules/github_repo_permission.py	Main module implementation with GitHub API wrapper, permission management logic, and support for both teams and users
tests/unit/plugins/modules/test_github_repo_permission.py	Comprehensive unit tests covering idempotency, updates, custom roles, and user/team scenarios
changelogs/fragments/gh-repo-permission-module.yml	Changelog entry documenting the new module

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-30T12:49:51Z

plugins/modules/github_repo_permission.py

+    permission:
+        description:
+            - Permission to grant when O(state=present).
+            - Supports built-ins C(pull), C(triage), C(push), C(maintain), C(admin) and custom repository roles (GitHub Enterprise).
+            - Synonyms C(read) -> C(pull) and C(write) -> C(push) are accepted.
+        type: str


The indentation for the 'permission' option is incorrect. It should be at the same level as other top-level options like 'team_slug' and 'username'.

Suggested change

permission:

description:

- Permission to grant when O(state=present).

- Supports built-ins C(pull), C(triage), C(push), C(maintain), C(admin) and custom repository roles (GitHub Enterprise).

- Synonyms C(read) -> C(pull) and C(write) -> C(push) are accepted.

type: str

permission:

description:

- Permission to grant when O(state=present).

- Supports built-ins C(pull), C(triage), C(push), C(maintain), C(admin) and custom repository roles (GitHub Enterprise).

- Synonyms C(read) -> C(pull) and C(write) -> C(push) are accepted.

type: str

Copilot · 2025-08-30T12:49:52Z

plugins/modules/github_repo_permission.py

+
+def ensure_team_permission(module, session, owner, repo, team_slug, desired_permission, state):
+    # Get current teams on repo and find team's current permission
+    role_name, current_builtin = get_team_repo_permission(session, owner, team_slug, owner, repo)


The parameters are incorrect - 'owner' is passed twice. The first parameter should be 'org' (organization), not 'owner'. This should be: get_team_repo_permission(session, owner, team_slug, owner, repo)

ansibullbot · 2025-08-30T12:52:29Z

cc @stpierre
click here for bot help

ansibullbot · 2025-08-30T12:58:06Z

The test ansible-test sanity --test pep8 [explain] failed with 4 errors:

plugins/modules/github_repo_permission.py:340:161: E501: line too long (167 > 160 characters)
plugins/modules/github_repo_permission.py:482:161: E501: line too long (169 > 160 characters)
tests/unit/plugins/modules/test_github_repo_permission.py:155:5: E122: continuation line missing indentation or outdented
tests/unit/plugins/modules/test_github_repo_permission.py:155:6: E202: whitespace before ')'

The test ansible-test sanity --test pep8 [explain] failed with 4 errors:

plugins/modules/github_repo_permission.py:340:161: E501: line too long (167 > 160 characters)
plugins/modules/github_repo_permission.py:482:161: E501: line too long (169 > 160 characters)
tests/unit/plugins/modules/test_github_repo_permission.py:155:5: E122: continuation line missing indentation or outdented
tests/unit/plugins/modules/test_github_repo_permission.py:155:6: E202: whitespace before ')'

The test ansible-test sanity --test pylint [explain] failed with 3 errors:

tests/unit/plugins/modules/test_github_repo_permission.py:75:41: disallowed-name: Disallowed name "_"
tests/unit/plugins/modules/test_github_repo_permission.py:159:13: undefined-variable: Undefined variable 'ctx'
tests/unit/plugins/modules/test_github_repo_permission.py:216:41: disallowed-name: Disallowed name "_"

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

plugins/modules/github_repo_permission.py:0:0: invalid-documentation: DOCUMENTATION.options.username.permission: extra keys not allowed @ data['options']['username']['permission']. Got {'description': ['Permission to grant when O(state=present).', 'Supports built-ins C(pull), C(triage), C(push), C(maintain), C(admin) and custom repository roles (GitHub Enterprise).', 'Synonyms C(read) -> C(pull) and C(write) -> C(push) are accepted.'], 'type': 'str'}
plugins/modules/github_repo_permission.py:0:0: parameter-type-not-in-doc: Argument 'permission' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/github_repo_permission.py:0:0: undocumented-parameter: Argument 'permission' is listed in the argument_spec, but not documented in the module documentation

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/github_repo_permission.py:404:23: disallowed-name: Disallowed name "_"

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

plugins/modules/github_repo_permission.py:0:0: invalid-documentation: DOCUMENTATION.options.username.permission: extra keys not allowed @ data['options']['username']['permission']. Got {'description': ['Permission to grant when O(state=present).', 'Supports built-ins C(pull), C(triage), C(push), C(maintain), C(admin) and custom repository roles (GitHub Enterprise).', 'Synonyms C(read) -> C(pull) and C(write) -> C(push) are accepted.'], 'type': 'str'}
plugins/modules/github_repo_permission.py:0:0: parameter-type-not-in-doc: Argument 'permission' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/github_repo_permission.py:0:0: undocumented-parameter: Argument 'permission' is listed in the argument_spec, but not documented in the module documentation

The test ansible-test sanity --test pep8 [explain] failed with 4 errors:

plugins/modules/github_repo_permission.py:340:161: E501: line too long (167 > 160 characters)
plugins/modules/github_repo_permission.py:482:161: E501: line too long (169 > 160 characters)
tests/unit/plugins/modules/test_github_repo_permission.py:155:5: E122: continuation line missing indentation or outdented
tests/unit/plugins/modules/test_github_repo_permission.py:155:6: E202: whitespace before ')'

The test ansible-test sanity --test pep8 [explain] failed with 4 errors:

plugins/modules/github_repo_permission.py:340:161: E501: line too long (167 > 160 characters)
plugins/modules/github_repo_permission.py:482:161: E501: line too long (169 > 160 characters)
tests/unit/plugins/modules/test_github_repo_permission.py:155:5: E122: continuation line missing indentation or outdented
tests/unit/plugins/modules/test_github_repo_permission.py:155:6: E202: whitespace before ')'

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/github_repo_permission.py:404:23: disallowed-name: Disallowed name "_"

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

plugins/modules/github_repo_permission.py:0:0: invalid-documentation: DOCUMENTATION.options.username.permission: extra keys not allowed @ data['options']['username']['permission']. Got {'description': ['Permission to grant when O(state=present).', 'Supports built-ins C(pull), C(triage), C(push), C(maintain), C(admin) and custom repository roles (GitHub Enterprise).', 'Synonyms C(read) -> C(pull) and C(write) -> C(push) are accepted.'], 'type': 'str'}
plugins/modules/github_repo_permission.py:0:0: parameter-type-not-in-doc: Argument 'permission' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/github_repo_permission.py:0:0: undocumented-parameter: Argument 'permission' is listed in the argument_spec, but not documented in the module documentation

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/github_repo_permission.py:404:23: disallowed-name: Disallowed name "_"

The test ansible-test sanity --test pylint [explain] failed with 3 errors:

tests/unit/plugins/modules/test_github_repo_permission.py:75:41: disallowed-name: Disallowed name "_"
tests/unit/plugins/modules/test_github_repo_permission.py:159:13: undefined-variable: Undefined variable 'ctx'
tests/unit/plugins/modules/test_github_repo_permission.py:216:41: disallowed-name: Disallowed name "_"

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

plugins/modules/github_repo_permission.py:0:0: invalid-documentation: DOCUMENTATION.options.username.permission: extra keys not allowed @ data['options']['username']['permission']. Got {'description': ['Permission to grant when O(state=present).', 'Supports built-ins C(pull), C(triage), C(push), C(maintain), C(admin) and custom repository roles (GitHub Enterprise).', 'Synonyms C(read) -> C(pull) and C(write) -> C(push) are accepted.'], 'type': 'str'}
plugins/modules/github_repo_permission.py:0:0: parameter-type-not-in-doc: Argument 'permission' in argument_spec defines type as 'str' but documentation doesn't define type
plugins/modules/github_repo_permission.py:0:0: undocumented-parameter: Argument 'permission' is listed in the argument_spec, but not documented in the module documentation

The test ansible-test sanity --test pylint [explain] failed with 3 errors:

tests/unit/plugins/modules/test_github_repo_permission.py:75:41: disallowed-name: Disallowed name "_"
tests/unit/plugins/modules/test_github_repo_permission.py:159:13: undefined-variable: Undefined variable 'ctx'
tests/unit/plugins/modules/test_github_repo_permission.py:216:41: disallowed-name: Disallowed name "_"

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/github_repo_permission.py:404:23: disallowed-name: Disallowed name "_"

The test ansible-test sanity --test pylint [explain] failed with 3 errors:

tests/unit/plugins/modules/test_github_repo_permission.py:75:41: disallowed-name: Disallowed name "_"
tests/unit/plugins/modules/test_github_repo_permission.py:159:13: undefined-variable: Undefined variable 'ctx'
tests/unit/plugins/modules/test_github_repo_permission.py:216:41: disallowed-name: Disallowed name "_"

click here for bot help

Signed-off-by: Daniele Marcocci <[email protected]>

russoz

Hi @danielino Thanks for your contribution.

This PR seems to have been vibe-coded. Sigh. The code is super bloated, confused, and it looks like something that will be a nightmare to maintain - one year from now, no one will have any idea what was in there. Even now, this over-verbose code is hard to read and make sense of. After a couple of interactions, Copilot (or any other LLM) will lose track of the objectives.

I strongly suggest you perform a thorough review of this module and improve its quality. There are many redundancies, and things being done in over-engineered ways. There are (possibly, probably) parts that could be refactored to make it better.

I have pointed some things that popped up immediately, but not everything.

russoz · 2025-08-31T02:56:03Z

changelogs/fragments/gh-repo-permission-module.yml

Changelog fragments are not used when adding new modules, please remove this file.

russoz · 2025-08-31T02:57:52Z

plugins/modules/github_repo_permission.py

+      - Supports built-ins C(pull), C(triage), C(push), C(maintain), C(admin) and custom repository roles (GitHub Enterprise).
+      - Synonyms C(read) -> C(pull) and C(write) -> C(push) are accepted.


Suggested change

- Supports built-ins C(pull), C(triage), C(push), C(maintain), C(admin) and custom repository roles (GitHub Enterprise).

- Synonyms C(read) -> C(pull) and C(write) -> C(push) are accepted.

- Supports built-ins V(pull), V(triage), V(push), V(maintain), V(admin) and custom repository roles (GitHub Enterprise).

- Synonyms V(read) -> V(pull) and V(write) -> V(push) are accepted.

Please use semantic markup as documented in:
https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_documenting.html#semantic-markup-within-module-documentation

russoz · 2025-08-31T03:04:39Z

tests/unit/plugins/modules/test_github_repo_permission.py

+try:  # Python 3
+    from unittest.mock import patch
+except Exception:  # Python 2 fallback
+    try:
+        from mock import patch  # type: ignore
+    except Exception:
+        patch = None  # type: ignore


See other tests for examples.

Suggested change

try: # Python 3

from unittest.mock import patch

except Exception: # Python 2 fallback

try:

from mock import patch # type: ignore

except Exception:

patch = None # type: ignore

from ansible_collections.community.internal_test_tools.tests.unit.compat.mock import patch

russoz · 2025-08-31T03:05:22Z

tests/unit/plugins/modules/test_github_repo_permission.py

+# Skip the entire module on Python 2.7 if 'mock' isn't available
+if sys.version_info[0] < 3 and patch is None:  # type: ignore
+    import pytest  # type: ignore
+    pytest.skip("mock not available on Python 2.7 test env", allow_module_level=True)
+


that is unnecessary

russoz · 2025-08-31T03:09:49Z

plugins/modules/github_repo_permission.py

+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+
+# Copyright (c) 2025, community.general contributors


This is meant to have your name on it, not this generic text. Copilot is cheating on you :-P

russoz · 2025-08-31T03:21:08Z

plugins/modules/github_repo_permission.py

+            # Use the recommended media type that supports newer fields and features
+            'Accept': 'application/vnd.github+json',
+            # Pin to a stable API version when available
+            'X-GitHub-Api-Version': '2022-11-28',


This hardcoded magic value here is not a good practice. Why specifically this version? Would different users want to use different versions? Should this be a parameter? If it must be fixed, please make a constant (by the beginning of the file) and add a note in the documentation about it, for the benefit of the module users.

russoz · 2025-08-31T03:22:33Z

plugins/modules/github_repo_permission.py

+  sample:
+    subject: team
+    subject_identifier: backend
+    repository: myorg/myrepo
+    permission: push
+    state: present


This is not a valid sample for that RV. Please see other modules.

russoz · 2025-08-31T03:26:02Z

plugins/modules/github_repo_permission.py

+        return 'pull'
+    if low == 'write':
+        return 'push'
+    if low in BUILTIN_PERMISSIONS:


if this constant was a dict, mapping the desired permission to the permission name effectively used, this entire function could be made much simpler. Something like:

return PERM_MAP.get(p.lower(), p)

(Assuming if the builtin are in the map, mapping to themselves).

russoz · 2025-08-31T03:28:11Z

plugins/modules/github_repo_permission.py

+    # Attempt to match custom role by case-insensitive name
+    roles = session.get_custom_repo_roles(org)
+    match = None
+    desired_lower = str(desired).lower()


why would it ever not be a string to begin with?

russoz · 2025-08-31T03:30:20Z

plugins/modules/github_repo_permission.py

+    if perm == 'read':
+        return 'pull'
+    if perm == 'write':
+        return 'push'


why are you normalizing "manually" here if there is a function to normalize it?

feat: add github_repo_permission module and test

284d3d3

Signed-off-by: Daniele Marcocci <[email protected]>

Copilot AI review requested due to automatic review settings August 30, 2025 12:49

Copilot AI reviewed Aug 30, 2025

View reviewed changes

ansibullbot added module module new_contributor Help guide this first time contributor new_plugin New plugin plugins plugin (any type) tests tests unit tests/unit labels Aug 30, 2025

ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Aug 30, 2025

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-11 Automatically create a backport for the stable-10 branch labels Aug 30, 2025

danielino added 2 commits August 30, 2025 15:16

fix: copilot suggestion and sanity test output

7a907aa

Signed-off-by: Daniele Marcocci <[email protected]>

fix: failed tests

c6015f0

Signed-off-by: Daniele Marcocci <[email protected]>

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Aug 30, 2025

fix: python2 fallback import for mock library. add maintainer to BOTMETA

60df55e

Signed-off-by: Daniele Marcocci <[email protected]>

ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Aug 30, 2025

russoz requested changes Aug 31, 2025

View reviewed changes

ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Sep 8, 2025

felixfontein removed the backport-11 Automatically create a backport for the stable-10 branch label Oct 13, 2025

		- Supports built-ins C(pull), C(triage), C(push), C(maintain), C(admin) and custom repository roles (GitHub Enterprise).
		- Synonyms C(read) -> C(pull) and C(write) -> C(push) are accepted.

feat: add github_repo_permission module and test #10767

Are you sure you want to change the base?

feat: add github_repo_permission module and test #10767

Conversation

danielino commented Aug 30, 2025

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

Uh oh!

ansibullbot commented Aug 30, 2025

Uh oh!

ansibullbot commented Aug 30, 2025

Uh oh!

russoz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants