-
Notifications
You must be signed in to change notification settings - Fork 157
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
Implementing support for suborg level rulesets #597
Comments
If rulesets are defined in the suborg yaml, we could create repo rulesets for each repo in the suborg. But the ruleset team took a different approach by providing a way to create a ruleset at the org level and yet target it to a subset of repos using repo properties. With that option in mind, first, it would be nice if we could support repository properties as a way to define sub-orgs. Second, if the suborg is defined using repo properties and has a ruleset in the yaml, we could create the ruleset as an org ruleset instead of multiple repo rulesets. These is what I was thinking but haven't started designing anything yet. |
Thanks @decyjphr, that makes sense although it seems like defining sub-orgs by properties is in tension with the usual business of safe-settings being the source-of-truth for other settings. I realised after posting that it is currently possible to define rulesets in suborgs which create repo rulesets. Is it best not to rely on this undocumented feature? |
@stevoland It doesn't seem to work for me. You can create a ruleset but not modify it and the app won't revert changes that are made manually, even after subscribing to |
@JakubBiel Fair, I haven't tested that webhook myself. It looks like it's supposed to work Lines 295 to 304 in 9128ea8
|
Maybe because it doesn't detect any changes? I wasn't able to modify the ruleset (only create it) after all so something is definitely not working. |
We haven't had any issues modifying rulesets so far fwiw |
So just to confirm, you were able to change ruleset under suborg settings and the diff appeared in the PR comment and then it modified the ruleset. If so, would you mind checking if the webhook works for you? Maybe I'm doing something wrong here |
That's correct. We do have some fixes that aren't pushed upstream. This one might be relevant eeveebank@db7ff86. From memory if there's a ruleset that is unchanged an exception is thrown and swallowed and the diff will be empty |
Also, empty diff if there's multiple commits on the PR branch eeveebank@bc83d9f |
safe-settings/docs/sample-settings/settings.yml Lines 278 to 279 in 777a97f
This comment in combination with this issue confuses me a bit. Are rulesets for suborgs supported or not? Because given that the above comment, it suggests one only has to leave out UPDATE: I just tried it myself. When placed in a suborg YAML, then the ruleset is created on the repository (not organization) level. |
I got this working with the following config https://github.com/UCL-MIRSG/.github/blob/main/safe-settings/suborgs/rulesets.yaml |
New Feature
We're investigating implementing support for rulesets in suborgs.
It seems that, with this missing piece, rulesets can completely replace branch protections (and their limitations such as the api not supporting branch name patterns)
It would be good to get some more detail on this comment @decyjphr
I note that
repository_property
is recently available in org ruleset conditions but am wondering how this might affect the implementation of suborg rulesets?Wouldn't suborg rulesets be implemented as repo rulesets in the same way as eg: branch protections?
It would be great if you could give a sketch of the requirements here or indicate if this is on the immediate roadmap, thanks
The text was updated successfully, but these errors were encountered: