Trust a module's trunk for who can merge into it, not for a required status (ADR 0534 §6)
The forge requires no status to merge any more (ADR 0534 §1); a trunk is trusted when it takes no push, its merge whitelist names mesh-tools alone and no administrator merges past its rule, so only mesh-delivery lands a commit there, on a passing mesh/check and the operator's Approve. The old rule failed the 11:51 delivery plan once mesh-catalog's status was unticked.
This commit is contained in:
@@ -205,8 +205,9 @@ func (f *sourceForms) buildsFrom(entries []inventory.Entry, repository, seat str
|
||||
type forgeFacts struct {
|
||||
// ID is the forge's own id of the repository: what tells it from one deleted and made again by its name.
|
||||
ID int64
|
||||
// Guarded is whether the branch is protected as a module's trunk must be: no direct push, at least one
|
||||
// required status, and no administrator merging past one. Why says what is missing when it is not.
|
||||
// Guarded is whether the branch is protected as a module's trunk must be: no direct push, a merge whitelist naming
|
||||
// the tools' account alone, and no administrator merging past the rule (novox/hq ADR 0534 §6; no required status,
|
||||
// which the forge no longer asks, §1). Why says what is missing when it is not.
|
||||
Guarded bool
|
||||
Why string
|
||||
}
|
||||
@@ -361,27 +362,39 @@ func askTheForgeOnce(ctx context.Context, owner, repo, branch string) (forgeFact
|
||||
return facts, nil
|
||||
}
|
||||
|
||||
// forgeToolsAccount is the forge tools' account, the only one main's merge whitelist names (novox/hq ADR 0398 §2, the
|
||||
// gitea module's tools-account.ts TOOLS_ACCOUNT): only mesh-delivery, through the merge tool, lands a commit on a trunk.
|
||||
const forgeToolsAccount = "mesh-tools"
|
||||
|
||||
// protectionRule is a branch protection rule as the forge's tool summarises it.
|
||||
type protectionRule struct {
|
||||
Rule string `json:"rule"`
|
||||
Push bool `json:"push"`
|
||||
RequiredStatuses []string `json:"required_statuses"`
|
||||
AdminMayOverride bool `json:"admin_may_override"`
|
||||
// MergeWhitelist is who may merge into the branch; nil when the rule keeps no merge whitelist (anyone who writes).
|
||||
MergeWhitelist []string `json:"merge_whitelist"`
|
||||
}
|
||||
|
||||
// judgedRule judges the one rule the forge applies to a branch, as the forge picks it: the rule named for the
|
||||
// branch, else the first glob rule, in the forge's order, that covers it (gitea's first matching rule). Never a
|
||||
// later rule that happens to be stronger: the forge does not read it.
|
||||
// later rule that happens to be stronger: the forge does not read it. A trunk is trusted for who can land a commit on
|
||||
// it, not for a status it requires (novox/hq ADR 0534 §6): no push, only the tools' account merges, and no administrator
|
||||
// merging past the rule; then only mesh-delivery lands a commit there, through the merge tool, on a passing mesh/check
|
||||
// and the operator's Approve (ADR 0534 §4).
|
||||
func judgedRule(rules []protectionRule, branch string) forgeFacts {
|
||||
switch r := appliedRule(rules, branch); {
|
||||
case r == nil:
|
||||
return forgeFacts{Why: fmt.Sprintf("no protection rule covers %s", branch)}
|
||||
case r.Push:
|
||||
return forgeFacts{Why: fmt.Sprintf("the rule %s lets a person push to %s directly", r.Rule, branch)}
|
||||
case len(r.RequiredStatuses) == 0:
|
||||
return forgeFacts{Why: fmt.Sprintf("the rule %s requires no status before a merge into %s", r.Rule, branch)}
|
||||
case r.MergeWhitelist == nil || len(r.MergeWhitelist) == 0:
|
||||
return forgeFacts{Why: fmt.Sprintf("the rule %s keeps no merge whitelist naming %s alone for %s", r.Rule, forgeToolsAccount, branch)}
|
||||
case len(r.MergeWhitelist) != 1 || r.MergeWhitelist[0] != forgeToolsAccount:
|
||||
return forgeFacts{Why: fmt.Sprintf("the rule %s lets %s merge into %s, not %s alone", r.Rule,
|
||||
strings.Join(r.MergeWhitelist, ", "), branch, forgeToolsAccount)}
|
||||
case r.AdminMayOverride:
|
||||
return forgeFacts{Why: fmt.Sprintf("the rule %s lets an administrator merge into %s past a status", r.Rule, branch)}
|
||||
return forgeFacts{Why: fmt.Sprintf("the rule %s lets an administrator merge into %s past it", r.Rule, branch)}
|
||||
}
|
||||
return forgeFacts{Guarded: true}
|
||||
}
|
||||
|
||||
@@ -102,16 +102,17 @@ func init() {
|
||||
// The forge applies one rule to a branch: the rule named for it, else the first glob that covers it. A stronger
|
||||
// rule later in the list is not what guards the branch, and is not read as if it were (the confirmation review).
|
||||
func TestTheRuleJudgedIsTheOneTheForgeApplies(t *testing.T) {
|
||||
guarded := protectionRule{Rule: "*", RequiredStatuses: []string{"mesh/merge-gate"}}
|
||||
open := protectionRule{Rule: "main", Push: true, RequiredStatuses: []string{"mesh/merge-gate"}}
|
||||
tools := []string{forgeToolsAccount}
|
||||
guarded := protectionRule{Rule: "*", MergeWhitelist: tools}
|
||||
open := protectionRule{Rule: "main", Push: true, MergeWhitelist: tools}
|
||||
if f := judgedRule([]protectionRule{guarded, open}, "main"); f.Guarded {
|
||||
t.Error("an exact rule letting pushes was passed over for a glob that guards")
|
||||
}
|
||||
if f := judgedRule([]protectionRule{{Rule: "ma*", RequiredStatuses: nil}, {Rule: "*", RequiredStatuses: []string{"x"}}},
|
||||
if f := judgedRule([]protectionRule{{Rule: "ma*", MergeWhitelist: nil}, {Rule: "*", MergeWhitelist: tools}},
|
||||
"main"); f.Guarded || !strings.Contains(f.Why, "ma*") {
|
||||
t.Errorf("the first glob covering the branch was not the one judged: %+v", f)
|
||||
}
|
||||
if f := judgedRule([]protectionRule{{Rule: "release/*"}, {Rule: "main", RequiredStatuses: []string{"x"}}}, "main"); !f.Guarded {
|
||||
if f := judgedRule([]protectionRule{{Rule: "release/*"}, {Rule: "main", MergeWhitelist: tools}}, "main"); !f.Guarded {
|
||||
t.Errorf("the rule named for the branch was not judged: %+v", f)
|
||||
}
|
||||
if f := judgedRule(nil, "main"); f.Guarded {
|
||||
@@ -119,6 +120,36 @@ func TestTheRuleJudgedIsTheOneTheForgeApplies(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// **A module's trunk is trusted when it takes no push, only the tools' account merges into it, and no administrator
|
||||
// merges past its rule** (novox/hq ADR 0534 §6): the forge requires no status to merge (§1), so a rule requiring none is
|
||||
// trusted; one with no merge whitelist, another name in it, a push allowed or an administrator's override is not.
|
||||
func TestATrunkIsTrustedForItsMergersNotForAStatus(t *testing.T) {
|
||||
tools := []string{forgeToolsAccount}
|
||||
for _, c := range []struct {
|
||||
name string
|
||||
rule protectionRule
|
||||
want bool
|
||||
why string
|
||||
}{
|
||||
{"no status, no push, the tools alone merge", protectionRule{Rule: "main", MergeWhitelist: tools}, true, ""},
|
||||
{"a status required as well", protectionRule{Rule: "main", MergeWhitelist: tools, RequiredStatuses: []string{"mesh/check"}}, true, ""},
|
||||
{"no merge whitelist", protectionRule{Rule: "main"}, false, "merge whitelist"},
|
||||
{"an empty merge whitelist", protectionRule{Rule: "main", MergeWhitelist: []string{}}, false, "merge whitelist"},
|
||||
{"another name in the whitelist", protectionRule{Rule: "main", MergeWhitelist: []string{forgeToolsAccount, "agent-ace"}}, false, "agent-ace"},
|
||||
{"another name alone", protectionRule{Rule: "main", MergeWhitelist: []string{"agent-ace"}}, false, "agent-ace"},
|
||||
{"a push allowed", protectionRule{Rule: "main", Push: true, MergeWhitelist: tools}, false, "push"},
|
||||
{"an administrator's override", protectionRule{Rule: "main", MergeWhitelist: tools, AdminMayOverride: true}, false, "administrator"},
|
||||
} {
|
||||
f := judgedRule([]protectionRule{c.rule}, "main")
|
||||
if f.Guarded != c.want || !strings.Contains(f.Why, c.why) {
|
||||
t.Errorf("%s: %+v, want guarded %v saying %q", c.name, f, c.want, c.why)
|
||||
}
|
||||
if strings.Contains(f.Why, "requires no status") {
|
||||
t.Errorf("%s: a trunk is still judged on a required status: %s", c.name, f.Why)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// An id the forge could not give keeps the id already recorded.
|
||||
|
||||
// **The operator's Approve of a reviewed group registers a moved module from its new repository** (novox/hq ADR 0420 §7,
|
||||
|
||||
Reference in New Issue
Block a user