Add support for GitHub teams and team-repos #1167
Conversation
cartography/intel/github/__init__.py
Outdated
| auth_data['token'], | ||
| auth_data['url'], | ||
| auth_data['name'], | ||
| config.update_tag, |
There was a problem hiding this comment.
You should modify users & repos sync to add "config.update_tag" parameter to keep all sync function a similar definition.
There was a problem hiding this comment.
I think we can take away this parameter instead, since the update tag is within common_job_parameters.
| lastupdated: PropertyRef = PropertyRef('lastupdated', set_in_kwargs=True) | ||
|
|
||
|
|
||
| @dataclass(frozen=True) |
There was a problem hiding this comment.
May be you could add an extra label "GITHUB_PERMISSION" or similar to allow generic graph queries like "does team X has access to repository Y" with a simple query:
| MATCH (a:GitHubTeam)-[r:GITHUB_PERMISSION]-(b:GitHubRepository)
This will allow much cleaner queries, and if any new kind of role appears you will not break your existing queries.
There was a problem hiding this comment.
There are 5 possible roles, but I'm not against the idea. I usually would just do something like match (a:MyNode)--(b:OtherNode). If we do this, I think a good name is ACCESS.
https://docs.github.com/en/organizations/managing-user-access-to-your-organizations-repositories/repository-roles-for-an-organization
| lastupdated: PropertyRef = PropertyRef('lastupdated', set_in_kwargs=True) | ||
|
|
||
|
|
||
| @dataclass(frozen=True) |
There was a problem hiding this comment.
There are 5 possible roles, but I'm not against the idea. I usually would just do something like match (a:MyNode)--(b:OtherNode). If we do this, I think a good name is ACCESS.
https://docs.github.com/en/organizations/managing-user-access-to-your-organizations-repositories/repository-roles-for-an-organization
|
|
||
| @dataclass(frozen=True) | ||
| class GitHubTeamNodeProperties(CartographyNodeProperties): | ||
| id: PropertyRef = PropertyRef('url') |
There was a problem hiding this comment.
nonblocker: I feel like id should maybe be {org}/{name}, but i see that url was also used for the id of Repos
| @patch.object(cartography.intel.github.teams, 'get_teams', return_value=GH_TEAM_DATA) | ||
| def test_sync_github_teams(mock_teams, mock_team_repos, neo4j_session): | ||
| # Arrange | ||
| _ensure_local_neo4j_has_test_data(neo4j_session) |
There was a problem hiding this comment.
Non blocker: add one more test org to make sure teams aren't accidentally added to that other org.
cartography/intel/github/__init__.py
Outdated
| auth_data['token'], | ||
| auth_data['url'], | ||
| auth_data['name'], | ||
| config.update_tag, |
There was a problem hiding this comment.
I think we can take away this parameter instead, since the update tag is within common_job_parameters.
Testing performed