Skip to content

Commit a427c01

Browse files
igorcostaigorcosta
authored
Merge pull request #741 from jitran/jitran/custom-pr-status-checks
Feature: Allow PR status checks to be managed external to safe settings
2 parents a8a27e7 + fa8d2d7 commit a427c01

File tree

7 files changed

+824
-3
lines changed

7 files changed

+824
-3
lines changed

‎README.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,46 @@ overridevalidators:
123123
124124
A sample of `deployment-settings` file is found [here](docs/sample-settings/sample-deployment-settings.yml).
125125

126+
### Custom Status Checks
127+
For branch protection rules and rulesets, you can allow for status checks to be defined outside of safe-settings together with your usual safe settings.
128+
129+
This can be defined at the org, sub-org, and repo level.
130+
131+
To configure this for branch protection rules, specify `{{EXTERNALLY_DEFINED}}` under the `contexts` keyword:
132+
```yaml
133+
branches:
134+
- name: main
135+
protection:
136+
...
137+
required_status_checks:
138+
contexts:
139+
- "{{EXTERNALLY_DEFINED}}"
140+
```
141+
142+
For rulesets, specify `{{EXTERNALLY_DEFINED}}` under the `required_status_checks` keyword:
143+
```yaml
144+
rulesets:
145+
- name: Status Checks
146+
...
147+
rules:
148+
- type: required_status_checks
149+
parameters:
150+
required_status_checks:
151+
- context: "{{EXTERNALLY_DEFINED}}"
152+
```
153+
154+
Notes:
155+
- For the same branch that is covered by multi-level branch protection rules, contexts defined at the org level are merged into the sub-org and repo level contexts, while contexts defined at the sub-org level are merged into the repo level contexts.
156+
- Rules from the sub-org level are merged into the repo level when their ruleset share the same name. Becareful not to define the same rule type in both levels as it will be rejected by GitHub.
157+
- When `{{EXTERNALLY_DEFINED}}` is defined for a new branch protection rule or ruleset configuration, they will be deployed with no status checks.
158+
- When an existing branch protection rule or ruleset configuration is amended with `{{EXTERNALLY_DEFINED}}`, the status checks in the existing rules in GitHub will remain as is.
159+
160+
> ⚠️ **Warning:**
161+
When `{{EXTERNALLY_DEFINED}}` is removed from an existing branch protection rule or ruleset configuration, the status checks in the existing rules in GitHub will revert to the checks that are defined in safe-settings. From this point onwards, all status checks configured through the GitHub UI will be reverted back to the safe-settings configuration.
162+
163+
#### Status checks inheritance across scopes
164+
Refer to [Status checks](docs/status-checks.md).
165+
126166
### Performance
127167
When there are 1000s of repos to be managed -- and there is a global settings change -- safe-settings will have to work efficiently and only make the necessary API calls.
128168

‎docs/status-checks.md

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
## Status checks inheritance across scopes
2+
3+
### Rulesets
4+
5+
The status checks between organisation and repository rulesets are independent of each together.
6+
7+
In the following examples, a common ruleset name is used at all levels. Repo1 and Repo2 are managed at the Sub-org level.
8+
9+
#### No custom checks
10+
```
11+
Org checks:
12+
Org Check
13+
Sub-org checks:
14+
Sub-org Check
15+
Repo checks for Repo2:
16+
Repo Check
17+
```
18+
19+
Status checks:
20+
- Newly deployed rules:
21+
- Org: Org Check
22+
- Repo1: Sub-org Check
23+
- Repo2: _Failed to deploy as required_status_checks can't be defined twice in both sub-org and repo level_
24+
- Updating status checks via GitHub UI:
25+
- Org: Status checks reverted back to safe settings
26+
- Repo1: Status checks reverted back to safe settings
27+
- Repo2: NA
28+
29+
#### No custom checks 2
30+
```
31+
Org checks:
32+
Org Check
33+
Sub-org checks:
34+
Sub-org Check
35+
Repo checks for Repo2:
36+
_NONE_
37+
```
38+
39+
Status checks:
40+
- Newly deployed rules:
41+
- Org: Org Check
42+
- Repo1: Sub-org Check
43+
- Repo2: _NONE_
44+
- Updating status checks via GitHub UI:
45+
- Org: Status checks reverted back to safe settings
46+
- Repo1: Status checks reverted back to safe settings
47+
- Repo2: Custom status checks are retained
48+
49+
**The remaining tests will leave Repo2 out of the Sub-org.**
50+
51+
#### Custom checks enabled at the Org and Sub-org level
52+
```
53+
Org:
54+
Org Check
55+
{{EXTERNALLY_DEFINED}}
56+
Sub-org checks:
57+
Sub-org Check
58+
{{EXTERNALLY_DEFINED}}
59+
Repo checks for Repo2:
60+
Repo Check
61+
```
62+
63+
Status checks:
64+
- Newly deployed rules:
65+
- Org: []
66+
- Repo1: []
67+
- Repo2: Repo Check
68+
- Updating status checks via GitHub UI:
69+
- Org: Custom status checks are retained
70+
- Repo1: Custom status checks are retained
71+
- Repo2: Status checks reverted back to safe settings
72+
73+
#### Custom checks enabled at the Repo level
74+
```
75+
Org:
76+
Org Check
77+
Sub-org checks:
78+
Sub-org Check
79+
Repo checks for Repo2:
80+
Repo Check
81+
{{EXTERNALLY_DEFINED}}
82+
```
83+
84+
Status checks:
85+
- Newly deployed rules:
86+
- Org: Org Check
87+
- Repo1: Sub-org Check
88+
- Repo2: []
89+
- Updating status checks via GitHub UI:
90+
- Org: Status checks reverted back to safe settings
91+
- Repo1: Status checks reverted back to safe settings
92+
- Repo2: Custom status checks are retained
93+
94+
95+
### Branch protection rules
96+
97+
In the following examples the `main` branch is protected at all levels. Repo1 and Repo2 are managed at the Sub-org level.
98+
99+
#### No custom checks
100+
```
101+
Org checks:
102+
Org Check
103+
Sub-org checks:
104+
Sub-org Check
105+
Repo checks for Repo2:
106+
Repo Check
107+
```
108+
109+
Status checks:
110+
- Newly deployed rules:
111+
- Repo1: Org Check, Sub-org Check
112+
- Repo2: Org Check, Sub-org Check, Repo Check
113+
- Updating status checks via GitHub UI:
114+
- Repo1: Status checks reverted back to safe settings
115+
- Repo2: Status checks reverted back to safe settings
116+
117+
#### Custom checks enabled at the Org level
118+
```
119+
Org:
120+
Org Check
121+
{{EXTERNALLY_DEFINED}}
122+
Sub-org checks:
123+
Sub-org Check
124+
Repo checks for Repo2:
125+
Repo Check
126+
```
127+
128+
Status checks:
129+
- Newly deployed rules:
130+
- Repo1: []
131+
- Repo2: []
132+
- Updating status checks via GitHub UI:
133+
- Repo1: Custom status checks are retained
134+
- Repo2: Custom status checks are retained
135+
136+
#### Custom checks enabled at the Sub-org level
137+
```
138+
Org:
139+
Org Check
140+
Sub-org checks:
141+
Sub-org Check
142+
{{EXTERNALLY_DEFINED}}
143+
Repo checks for Repo2:
144+
Repo Check
145+
```
146+
147+
Status checks:
148+
- Newly deployed rules:
149+
- Repo1: []
150+
- Repo2: []
151+
- Updating status checks via GitHub UI:
152+
- Repo1: Custom status checks are retained
153+
- Repo2: Custom status checks are retained
154+
155+
#### Custom checks enabled at the Repo level
156+
```
157+
Org:
158+
Org Check
159+
Sub-org checks:
160+
Sub-org Check
161+
Repo checks for Repo2:
162+
Repo Check
163+
{{EXTERNALLY_DEFINED}}
164+
```
165+
166+
Status checks:
167+
- Newly deployed rules:
168+
- Repo1: Org Check, Sub-org Check
169+
- Repo2: []
170+
- Updating status checks via GitHub UI:
171+
- Repo1: Status checks reverted back to safe settings
172+
- Repo2: Custom status checks are retained

‎lib/plugins/branches.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,22 @@
11
const ErrorStash = require('./errorStash')
22
const NopCommand = require('../nopcommand')
33
const MergeDeep = require('../mergeDeep')
4+
const Overrides = require('./overrides')
45
const ignorableFields = []
56
const previewHeaders = { accept: 'application/vnd.github.hellcat-preview+json,application/vnd.github.luke-cage-preview+json,application/vnd.github.zzzax-preview+json' }
7+
const overrides = {
8+
'contexts': {
9+
'action': 'reset',
10+
'type': 'array'
11+
},
12+
}
613

714
module.exports = class Branches extends ErrorStash {
815
constructor (nop, github, repo, settings, log, errors) {
916
super(errors)
1017
this.github = github
1118
this.repo = repo
12-
this.branches = settings
19+
this.branches = structuredClone(settings)
1320
this.log = log
1421
this.nop = nop
1522
}
@@ -49,7 +56,7 @@ module.exports = class Branches extends ErrorStash {
4956
const params = Object.assign({}, p)
5057
return this.github.repos.getBranchProtection(params).then((result) => {
5158
const mergeDeep = new MergeDeep(this.log, this.github, ignorableFields)
52-
const changes = mergeDeep.compareDeep({ branch: { protection: this.reformatAndReturnBranchProtection(result.data) } }, { branch: { protection: branch.protection } })
59+
const changes = mergeDeep.compareDeep({ branch: { protection: this.reformatAndReturnBranchProtection(result.data) } }, { branch: { protection: Overrides.removeOverrides(overrides, branch.protection, result.data) } })
5360
const results = { msg: `Followings changes will be applied to the branch protection for ${params.branch.name} branch`, additions: changes.additions, modifications: changes.modifications, deletions: changes.deletions }
5461
this.log.debug(`Result of compareDeep = ${results}`)
5562

@@ -76,7 +83,7 @@ module.exports = class Branches extends ErrorStash {
7683
return this.github.repos.updateBranchProtection(params).then(res => this.log(`Branch protection applied successfully ${JSON.stringify(res.url)}`)).catch(e => { this.logError(`Error applying branch protection ${JSON.stringify(e)}`); return [] })
7784
}).catch((e) => {
7885
if (e.status === 404) {
79-
Object.assign(params, branch.protection, { headers: previewHeaders })
86+
Object.assign(params, Overrides.removeOverrides(overrides, branch.protection, {}), { headers: previewHeaders })
8087
if (this.nop) {
8188
resArray.push(new NopCommand(this.constructor.name, this.repo, this.github.repos.updateBranchProtection.endpoint(params), 'Add Branch Protection'))
8289
return Promise.resolve(resArray)

‎lib/plugins/overrides.js

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
const ErrorStash = require('./errorStash')
2+
3+
module.exports = class Overrides extends ErrorStash {
4+
// Find all object references for a given key from the source.
5+
static getObjectRef (source, dataKey) {
6+
const results = []
7+
const traverse = (obj) => {
8+
for (const key in obj) {
9+
if (key === dataKey) {
10+
results.push(obj)
11+
} else if (Array.isArray(obj[key])) {
12+
obj[key].forEach(element => traverse(element))
13+
} else if (typeof obj[key] === 'object' && obj[key]) {
14+
traverse(obj[key])
15+
}
16+
}
17+
}
18+
traverse(source)
19+
return results
20+
}
21+
22+
// Find the parent object reference for a given child object and
23+
// allow the option to remove the parent object from the source.
24+
static getParentObjectRef (source, child, remove = false) {
25+
let parent = null
26+
const traverse = (obj, parentObj = null, parentKey = '') => {
27+
for (const key in obj) {
28+
if (obj[key] === child) {
29+
parent = obj
30+
if (remove && parentObj && parentKey) {
31+
delete parentObj[parentKey]
32+
}
33+
} else if (Array.isArray(obj[key])) {
34+
obj[key].forEach((element, index) => {
35+
if (element === child) {
36+
parent = obj[key]
37+
if (remove) {
38+
obj[key].splice(index, 1)
39+
}
40+
return
41+
}
42+
traverse(element)
43+
})
44+
} else if (typeof obj[key] === 'object' && obj[key]) {
45+
traverse(obj[key], obj, key)
46+
}
47+
}
48+
}
49+
traverse(source)
50+
return parent
51+
}
52+
53+
// Traverse the source and remove the top level parent object
54+
static removeTopLevelParent (source, child, levels) {
55+
let parent = child
56+
for (let i = 0; i < levels; i++) {
57+
if (i + 1 === levels) {
58+
parent = Overrides.getParentObjectRef(source, parent, true)
59+
} else {
60+
parent = Overrides.getParentObjectRef(source, parent, false)
61+
}
62+
}
63+
}
64+
65+
// When {{EXTERNALLY_DEFINED}} is found in the override value, retain the
66+
// existing value from GitHub. If GitHub does not have a value, then
67+
// - A/ If the action is delete, then remove the top level parent object
68+
// and the override value from the source.
69+
// - B/ Otherwise, initialise the value to an appropriate value.
70+
// Note:
71+
// - The admin settings could define multiple status check rules for a
72+
// ruleset, but the GitHub API retains one only, i.e. the last one.
73+
// - The PUT method for rulesets (update) allows for multiple overrides.
74+
// - The POST method for rulesets (create) allows for one override only.
75+
static removeOverrides (overrides, source, existing) {
76+
Object.entries(overrides).forEach(([override, props]) => {
77+
let sourceRefs = Overrides.getObjectRef(source, override)
78+
let data = JSON.stringify(sourceRefs)
79+
80+
if (data.includes('{{EXTERNALLY_DEFINED}}')) {
81+
let existingRefs = Overrides.getObjectRef(existing, override)
82+
sourceRefs.forEach(sourceRef => {
83+
if (existingRefs[0]) {
84+
sourceRef[override] = existingRefs[0][override]
85+
} else if (props['action'] === 'delete') {
86+
Overrides.removeTopLevelParent(source, sourceRef[override], props['parents'])
87+
delete sourceRef[override]
88+
} else if (props['type'] === 'array') {
89+
sourceRef[override] = []
90+
} else if (props['type'] === 'dict') {
91+
sourceRef[override] = {}
92+
} else {
93+
throw new Error(`Unknown type ${props['type']} for ${override}`)
94+
}
95+
})
96+
}
97+
})
98+
return source
99+
}
100+
}

0 commit comments

Comments
 (0)