Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for repository rulesets #7650

Merged
merged 25 commits into from Jul 10, 2023
Merged

Conversation

vaindil
Copy link
Contributor

@vaindil vaindil commented Jul 1, 2023

Repository rulesets are a new GitHub feature, essentially the evolution of branch protections. This PR adds support for them via a new command (ruleset/rs) with 3 subcommands.

  • rs list: Lists rulesets for the provided repo or org
  • rs view <id>: View info about a specific ruleset by ID, or choose interactively
  • rs check <branch>: List the rules that apply to the given branch in a repo
Sample output

List:

Showing 2 of 2 rulesets in my-org/repo-name and its parents

ID    NAME            SOURCE                   STATUS  RULES
1234  My Org Ruleset  my-org (org)             active  3
5678  Test Ruleset    my-org/repo-name (repo)  active  3

View:

Test Ruleset
ID: 5678
Source: my-org/repo-name (Repository)
Enforceument: Active

Bypass List
- OrganizationAdmin (ID: 1), mode: always
- RepositoryRole (ID: 5), mode: always

Conditions
- ref_name: [exclude: []] [include: [~ALL]] 

Rules
- commit_author_email_pattern: [name: ] [negate: false] [operator: ends_with] [pattern: @example.com] 
- commit_message_pattern: [name: ] [negate: false] [operator: contains] [pattern: asdf] 
- creation

Check:

6 rules apply to branch foo in repo my-org/repo-name

- commit_author_email_pattern: [name: ] [negate: false] [operator: ends_with] [pattern: @example.com] 
  (configured in ruleset 1234 from organization my-org)

- commit_author_email_pattern: [name: ] [negate: false] [operator: ends_with] [pattern: @example.me] 
  (configured in ruleset 5678 from repository my-org/repo-name)

- commit_message_pattern: [name: ] [negate: false] [operator: starts_with] [pattern: fff] 
  (configured in ruleset 1234 from organization my-org)

- commit_message_pattern: [name: ] [negate: false] [operator: contains] [pattern: asdf] 
  (configured in ruleset 5678 from repository my-org/repo-name)

- creation
  (configured in ruleset 5678 from repository my-org/repo-name)

- required_signatures
  (configured in ruleset 1234 from organization my-org)

Notes

  • The list subcommand and interactive mode for the view subcommand both use GraphQL, but the others use the REST API. This is because each rule type within the GraphQL response is its own type, so if everything were to use the GraphQL API, then the CLI would have to be updated each time a new rule is added to rulesets. We don't want to tie the CLI logic to the API shape like that.
    • I could standardize both on the REST API if desired, but I already had the GraphQL logic written when I figured that out so I just left it.
  • None of the commands support the --json flag for that same reason, as it seems to require using the GraphQL API and providing a list of supported fields, which would continue to tie them together. I'd love to add support if it's possible!

@vaindil vaindil marked this pull request as ready for review July 5, 2023 19:24
@vaindil vaindil requested a review from a team as a code owner July 5, 2023 19:24
@vaindil vaindil requested review from samcoe and removed request for a team July 5, 2023 19:24
@cliAutomation cliAutomation added the external pull request originating outside of the CLI core team label Jul 5, 2023
@cliAutomation
Copy link
Collaborator

Hi! Thanks for the pull request. Please ensure that this change is linked to an issue by mentioning an issue number in the description of the pull request. If this pull request would close the issue, please put the word 'Fixes' before the issue number somewhere in the pull request body. If this is a tiny change like fixing a typo, feel free to ignore this message.

@cliAutomation cliAutomation added this to Needs review 🤔 in The GitHub CLI Jul 5, 2023
Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested some very minor changes; otherwise it's looking great. thank you for the awesome work.

pkg/cmd/ruleset/list/list.go Outdated Show resolved Hide resolved
pkg/cmd/ruleset/view/view.go Outdated Show resolved Hide resolved
pkg/cmd/ruleset/view/view_test.go Show resolved Hide resolved
@vilmibm
Copy link
Contributor

vilmibm commented Jul 5, 2023

the CLI would have to be updated each time a new rule is added to rulesets

This is frustrating but might be worth it in order to support --json.

Alternatively, can we support --json but just treat actual rule definitions as raw json? in other words, the only keys you can select are top level to a ruleset.

@vaindil
Copy link
Contributor Author

vaindil commented Jul 6, 2023

This is frustrating but might be worth it in order to support --json.

Alternatively, can we support --json but just treat actual rule definitions as raw json? in other words, the only keys you can select are top level to a ruleset.

@vilmibm This is theoretically possible, but several of the fields are paginated, and many of them require more fields underneath that may change:

  • bypassActors
    • paginated
    • future additions would require a CLI update
  • conditions
    • future additions would require a CLI update
  • rules
    • paginated
    • future additions would require CLI update
  • source
    • can be multiple response types (either Repository or Organization)
    • future additions would require CLI update

Based on this, I think the only feasible way to add support would be to use the REST API, since those top-level fields will rarely change. Nothing else in the CLI uses the REST API for that command though. There's just too much weird behavior with this GraphQL setup because rulesets are a bit unique, it's not really feasible.

@vilmibm vilmibm merged commit 4ba2f2f into cli:trunk Jul 10, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external pull request originating outside of the CLI core team
Projects
No open projects
The GitHub CLI
  
Needs review 🤔
Development

Successfully merging this pull request may close these issues.

None yet

3 participants