feature: add repositories rules/rulesets

[electriquo]

Prerequisites:

Following Introducing Repository Rules Public Beta, GitHub also introduced a Rules REST API endpoint, but it seems to still be missing from Octokit (please do verify).

New Feature

Settings app to support the repository rules endpoint

[travi]

initial support has been released in https://github.com/repository-settings/app/releases/tag/v4.1.0. please note that this is considered a pre-release and may have breaking changes applied before it is considered fully released. feedback is welcome if you try it out in the meantime.

since this is not yet fully released, i'm going to keep this issue open and will follow up with more detail about the details i'm still considering further iteration on

[loozhengyuan]

@travi Thanks for adding support for this! I have tried this and thus far it is working great for ruleset creations but faced issues when updating it.

I noted that adding a new rule to a ruleset does not update the rule. Initially, I thought that it might be an issue the request schema but the desired changes were able to be applied if I were to change the ruleset name. This meant that the schema is working fine but probably faced issues when updating it.

[loozhengyuan]

I also noticed that the strict_required_status_checks_policy value must be defined, which seems to tally with the API docs. I am not sure if this should be solved in code or via documentation, but the example in the documentation does not work for me without making the following change:

rulesets:
  - name: verification must pass
    target: branch
    enforcement: active
    conditions:
      ref_name:
        include:
          - "~DEFAULT_BRANCH"
    rules:
      - type: required_status_checks
        parameters:
+         strict_required_status_checks_policy: false
          required_status_checks:
            - context: test
              integration_id: 123456

[travi]

thanks for testing it out, @loozhengyuan!

I noted that adding a new rule to a ruleset does not update the rule

i've noticed that i missed some details for the update scenario as well. i think i've identified the primary issue and have a partial solution that i'm working through already, so i think the results will be better once i get that finished up. unfortunately, i can't promise a timeline right now because i'm preparing to travel for work this week and it is questionable how much time i will find before returning home. this is high on my list once i have availability, though.

I also noticed that the strict_required_status_checks_policy value must be defined, which seems to tally with the API docs. I am not sure if this should be solved in code or via documentation, but the example in the documentation does not work for me

i spotted this problem while debugging the update issue. i agree that it being required was missed in the documentation. i havent decided yet how much we should work around these sorts of details in the app, but would accept a PR to update the docs for now if you'd be willing to tackle that detail

[loozhengyuan]

@travi Sure, that's fine. I will at least fix the docs so others are aware of this detail in the meantime; we can figure this out another time. Thanks!

[travi]

@loozhengyuan i worked through the problems that i had discovered so far, but i wont claim that i've tested all combinations against the real api yet. please let me know if you encounter more scenarios that dont appear to be working as you would expect

[loozhengyuan]

@travi Thanks for the fixes, I am able to confirm that the ruleset updates are working fine for me. I will report back if I encounter any other issues, but they are looking great for now. Many thanks!

[travi]

will follow up with more detail about the details i'm still considering further iteration on

ok, now that we've worked through those initial bugs, i'm finally getting a chance to circle back here and capture the thoughts that are in my mind before considering this the stable implementation.

mainly, i think the shape of the data is a bit awkward for the config file when presenting to human team members to understand and populate. for example, the use of IDs is totally reasonable for the api as it is expected to be populated programmatically by a system that can do lookups and translate to more human understandable representations. normally, we try hard to keep the shape of the config as close to the api representation as possible, but i'm leaning toward that not being appropriate for this particular plugin.

when comparing to the branch-protection plugin, defining a team uses the slug in that case, but that is also what the api expects. the slug is far more human understandable than the actor-id. similarly, repository roles are also represented as actor-ids in the rulesets api, but the word that represents roles, like "admin" could make the config more approachable. same for the name of a github app for things like bypass-actors or github actions for where a status must be reported from for required checks.

if we do go as far as mapping the actor-ids to more human consumable representations, i think that potentially also opens the door to make a few other things more ergonomic.

• maybe represent the default branch in a simpler way than ~DEFAULT_BRANCH. i like that there is a special way to specify the default rather than a branch name that could change over time, but that particular representation is an awkward way to present to humans
• could the include or exclude lists be optional in the config, even though they are required in the api?
• could other properties be considered optional, like maybe defaulting strict_required_status_checks_policy to false when not provided? is it better to keep these as explicit to avoid mistakes (might be a different situation if we provided better feedback for failures within the settings app than we currently do)?

[travi]

also worth noting: github is considering the bypass_actors property as sensitive information according to https://docs.github.com/en/rest/repos/rules#get-a-repository-ruleset and does not expose them through the api unless called with a token that has write access to the ruleset. defining those details in the config file make them more visible than that. we probably need to at least make that detail very visible in the docs, at the very least