github-community-projects / github-community-projects/safe-settings
Config merge not working as expected for settings files
- Dominant language
- JavaScript
- Stars
- 921
- Forks
- 226
- Avg merge
- 18h 3m
- Merged PRs (30d)
- 14
Description
## Problem Description
As stated in the README
> The precedence order for settings file is repository > suborg > org (.github/repos/.yml > .github/suborgs/.yml > .github/settings.yml)
This implies that if the same configuration(s) are defined at different levels, the settings defined at the repository level should take precedence. However, this is not happening as expected. I discovered that the issue stems from the way `safe-settings` creates the final merged config from the settings files. The merge process does a union of the suborg and repo level settings file, which brings back the deleted configs and also creates duplicate elements. There's also an edge case where the API call fails due to duplicate elements in the merged config.
### What is actually happening
I have configured a ruleset at the suborganization level that applies to multiple repositories and then modify the same ruleset for a particular repository in the repo level settings file.
**suborg/base-rulesets.yml**
```
suborgrepos:
- '*'
rulesets:
- name: Base Rulesets
target: branch
enforcement: active
bypass_actors:
- actor_id: 1
actor_type: OrganizationAdmin
bypass_mode: always
- actor_id: 1097369
actor_type: Integration
bypass_mode: pull_request
- actor_id: 11825178
actor_type: Team
bypass_mode: pull_request
conditions:
ref_name:
include:
- "refs/heads/rhoai-*"
exclude: []
rules:
- type: deletion
- type: non_fast_forward
- type: pull_request
parameters:
required_approving_review_count: 0
dismiss_stale_reviews_on_push: false
require_code_owner_review: false
require_last_push_approval: false
required_review_thread_resolution: false
allowed_merge_methods:
- merge
- squash
- rebase
```
**repos/repo-name.yml**
```
rulesets:
- name: Base Rulesets
target: branch
enforcement: active
bypass_actors:
- actor_id: 11825178
actor_type: Team
bypass_mode: pull_request
conditions:
ref_name:
include:
- "refs/heads/rhoai-*"
exclude: []
rules:
- type: creation
- type: non_fast_forward
- type: pull_request
parameters:
required_approving_review_count: 1
dismiss_stale_reviews_on_push: true
require_code_owner_review: false
require_last_push_approval: false
required_review_thread_resolution: false
allowed_merge_methods:
- merge
- squash
- rebase
```
What's happening is that the configurations from the suborgs and repo level settings files are getting merged as shown below. The merge process does a union of the suborg and repo level settings file, which brings back the deleted configs and also creates duplicate elements(duplicate pull_request object, duplicate ref_name in conditions object).
```
target: "branch"
enforcement: "active"
bypass_actors: [
{
"actor_id": 1,
"actor_type": "OrganizationAdmin",
"bypass_mode": "always"
},
{
"actor_id": 1097369,
"actor_type": "Integration",
"bypass_mode": "pull_request"
},
{
"actor_id": 11825178,
"actor_type": "Team",
"bypass_mode": "pull_request"
},
{
"actor_id": 11825178,
"actor_type": "Team",
"bypass_mode": "pull_request"
}
]
conditions: {
"ref_name": {
"include": [
"refs/heads/rhoai-*",
"refs/heads/rhoai-*"
],
"exclude": []
}
}
rules: [
{
"type": "deletion"
},
{
"type": "non_fast_forward"
},
{
"type": "pull_request",
"parameters": {
"required_approving_review_count": 0,
"dismiss_stale_reviews_on_push": false,
"require_code_owner_review": false,
"require_last_push_approval": false,
"required_review_thread_resolution": false,
"allowed_merge_methods": [
"merge",
"squash",
"rebase"
]
}
},
{
"type": "creation"
},
{
"type": "non_fast_forward"
},
{
"type": "pull_request",
"parameters": {
"required_approving_review_count": 1,
"dismiss_stale_reviews_on_push": true,
"require_code_owner_review": false,
"require_last_push_approval": false,
"required_review_thread_resolution": false,
"allowed_merge_methods": [
"merge",
"squash",
"rebase"
]
}
}
]
```
In the repository-level settings file, I made the below modifications to the rules, but few of them were not applied as expected:
- removed `deletion` rule - _**This rule was brought back due to the merge**_
- added `creation` rule _**Works as expected**_
- changed `required_approving_review_count` from _0_ to _1_ - _**Works as expected**_
- changed `dismiss_stale_reviews_on_push` from _false_ to _true_ - _**Works as expected**_
- removed `actor_id: 1` and `actor_id: 1097369` from _bypass_actors_ list - **_These actors were brought back due to merge_**.
**Apart from this, the merge is also creating DUPLICATE PROPERTIES, which might be resulting in error**
- If the ruleset exists, the app successfully sends a PUT request with the merged config as the request body([full logs](https://github.com/rhoai-rhtap/admin/actions/runs/12546266153/job/34981816239)). However, when the ruleset doesn't exist, the app makes a POST request with the same merged config, which leads to the below error([full logs](https://github.com/rhoai-rhtap/admin/actions/runs/12546352801/job/34982053584)).
> ERROR (HttpError): invalid json response body at https://api.github.com/repos/rhoai-rhtap/rhods-operator/rulesets reason: Unexpected end of JSON input
> HttpError: invalid json response body at https://api.github.com/repos/rhoai-rhtap/rhods-operator/rulesets reason: Unexpected end of JSON input
- include": ["refs/heads/rhoai-*","refs/heads/rhoai-*"] is causing
.
### What is the expected behavior
The Rulesets created for the repository should exactly match the configuration defined in the repository-level settings file.
- Rules removed from the repository-level settings file should not be included in the ruleset.
- Actor IDs removed from the repository-level settings file should not appear in the bypass actors.
In general - the merged config should not have duplicate elements and should not bring back deleted configurations in the repo level settings file. Safe-settings should consider the repository-level settings file as the source of truth and ensure that the precedence order is respected. Hopefully, it will also prevents API calls from failing due to duplicate elements in the request body.
### Error output, if available
Full Logs - https://github.com/rhoai-rhtap/admin/actions/runs/12546352801/job/34982053584
```
ERROR (HttpError): invalid json response body at https://api.github.com/repos/rhoai-rhtap/rhods-operator/rulesets reason: Unexpected end of JSON input
HttpError: invalid json response body at https://api.github.com/repos/rhoai-rhtap/rhods-operator/rulesets reason: Unexpected end of JSON input
at /home/runner/work/admin/admin/.safe-settings-code/node_modules/probot/node_modules/@octokit/request/dist-node/index.js:108:11
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async sendRequestWithRetries (/home/runner/work/admin/admin/.safe-settings-code/node_modules/octokit-auth-probot/node_modules/@octokit/auth-app/dist-node/index.js:398:12)
at async Job.doExecute (/home/runner/work/admin/admin/.safe-settings-code/node_modules/bottleneck/light.js:405:18)
status: 500
request: {
method: "POST"
url: "https://api.github.com/repos/rhoai-rhtap/rhods-operator/rulesets"
headers: {
"accept": "application/vnd.github.v3+json",
"user-agent": "probot/12.3.4 octokit-core.js/3.6.0 Node.js/20.18.1 (linux; x64)",
"x-github-api-version": "2022-11-28",
"authorization": "token [REDACTED]",
"content-type": "application/json; charset=utf-8"
}
body: "{\"name\":\"Base Rulesets\",\"target\":\"branch\",\"enforcement\":\"active\",\"bypass_actors\":[{\"actor_id\":1,\"actor_type\":\"OrganizationAdmin\",\"bypass_mode\":\"always\"},{\"actor_id\":1097369,\"actor_type\":\"Integration\",\"bypass_mode\":\"pull_request\"},{\"actor_id\":11825178,\"actor_type\":\"Team\",\"bypass_mode\":\"pull_request\"},{\"actor_id\":11825178,\"actor_type\":\"Team\",\"bypass_mode\":\"pull_request\"}],\"conditions\":{\"ref_name\":{\"include\":[\"refs/heads/rhoai-*\",\"refs/heads/rhoai-*\"],\"exclude\":[]}},\"rules\":[{\"type\":\"deletion\"},{\"type\":\"non_fast_forward\"},{\"type\":\"pull_request\",\"parameters\":{\"required_approving_review_count\":0,\"dismiss_stale_reviews_on_push\":false,\"require_code_owner_review\":false,\"require_last_push_approval\":false,\"required_review_thread_resolution\":false,\"allowed_merge_methods\":[\"merge\",\"squash\",\"rebase\"]}},{\"type\":\"creation\"},{\"type\":\"non_fast_forward\"},{\"type\":\"pull_request\",\"parameters\":{\"required_approving_review_count\":1,\"dismiss_stale_reviews_on_push\":true,\"require_code_owner_review\":false,\"require_last_push_approval\":false,\"required_review_thread_resolution\":false,\"allowed_merge_methods\":[\"merge\",\"squash\",\"rebase\"]}}]}"
request: {
"retryCount": 3,
"retries": 3,
"retryAfter": 16
}
}
```
## Context
### Are you using the hosted instance of probot/settings or running your own?
Running a full sync using Github Action
#### Version of probot/settings
2.1.14
Contributor guide
Assessment
This issue has not been assessed yet.