github-community-projects / github-community-projects/safe-settings

Config merge not working as expected for settings files

Open
#723 9 comments 2 reactions 0 assignees View on GitHub
bug
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
Screenshot 2024-12-30 at 4 26 21 PM.

### 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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.