Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/concepts/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ your-org/engineering-assets

`plan` and `distribute` run the same pipeline; `plan` stops before writing, with the
read account. `distribute --dry-run` runs it with the write account and also stops
before writing, and its report is the same as `plan`'s.
before writing. Its report is `plan`'s, except where the write account knows more: its
permissions, and on GitLab, Gitea and Forgejo the protected branches it may not push to,
which block a target before any write (`rules:protected-branch`).

| Phase | What happens |
|---|---|
Expand Down
5 changes: 5 additions & 0 deletions docs/guide/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ A run whose hub commit is no longer the tip of the default branch ends `supersed

`plan` runs with the reader, which cannot see what the writer may bypass: `plan` can
predict a rule block that `distribute` will not hit. `doctor` checks with the writer.
`distribute --dry-run` and `distribute` read, with the writer, the protected branches
that keep it from pushing to the sync branch (GitLab's protected branches; on Gitea and
Forgejo, the protection of a sync branch that exists already) and block the target
before any write; other rules show at the first push, which they refuse before anything
is written.

## A target is deferred or failed

Expand Down
3 changes: 2 additions & 1 deletion docs/project/e2e.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ The script also sets `TOUCHMARK_E2E_REQUIRED=1`: the tests then fail instead of
- Each step also checks which writes each target got, by the report's journal.
- Before it checks that a second run writes nothing, the test waits for the server's own work after a push. The server writes the push event into the pull request's timeline from a queue, seconds after the push, and that changes the pull request's `updated_at`. For a person's pull request the comparison leaves `updated_at` out: touchmark never writes to one (its state, head, text, labels and comments tell), and the person's own push may land in its timeline late.
- **`TestDoctor`**: `doctor` through `cli.Main` with the writer's token, over a fresh organisation: a writable opted-in repository (access ok; rules unknown while no sync branch exists, since the writer reads the protection of existing branches only), a read-only one (access fails), one not opted in (skipped), one whose sync branch a rule `touchmark/*` without pushes protects (rules fail), and one whose pull requests on the sync branch carry another hub's marker with the same id and a person's copy of this hub's (markers warn). The hub is a repository the writer reads (`hub-hidden` warns), then writes (fails). The token's scopes come from `GET /api/v1/token` where the forge has it; `--hub-token` with the admin's token reads the hub's Actions secrets, and a write key there fails. No token reaches any output.
- **`TestProtectedBranch`**: branch protection `touchmark/*` without pushes, which the writer reads for existing branches only. On a target whose pull request is open, a pack change is blocked `blocked:rules:protected-branch` before any write, in a dry run and in `distribute`, with a warning naming the branch (the forge does not show the writer the rule's name), the pull request as it was. On a target protected before its sync branch exists, the first push meets the protection and is refused with the same reason, nothing written.
- **`TestFacts`**: forge behavior touchmark relies on that the documentation leaves open, checked through the API and git alone, without the driver:
- flavor detection;
- the head filter of the pull request list (and whether it matches forks), `poster`, `pulls/{base}/{head}`;
Expand Down Expand Up @@ -188,7 +189,7 @@ Secrets never appear on a command line: curl reads its `PRIVATE-TOKEN` header fr
That group access token is revoked before `migrate` runs: the bot of a revoked token must still be found and its merge request adopted.
- **`TestDoctor`**: `doctor` through `cli.Main` with the writer's token over a fresh subgroup: the token's expiry and scopes (`/personal_access_tokens/self`), a bot or service account (2fa ok); a Developer's access; protected branches over the sync branch, for Maintainers (rules fail) and for Developers without force pushes (warn); another hub's marker with the same id and a person's copy of this hub's (markers warn); a hub the writer does not see (`hub-hidden` ok), then one in its subgroup (fail). `--hub-token` with root's token reads the hub's variables and protected tags: a protected write key scoped to `touchmark-distribute` is ok, an unprotected one fails, a protected tag Developers may create fails.
- **`TestSetup`**: `touchmark setup gitlab` through `cli.Main` as a maintainer runs it, with the person's token: an Owner of the seeded group, whose fresh subgroup holds the targets (one project opted in), and an Owner of a separate top-level group where the hub lives (GitLab shows a group's variables, which the final check reads, to its Owners only; a Maintainer gets that check as `unknown` and exit code 3). The hub starts as GitLab makes projects (the default branch protected for Maintainers) and keeps a stale write key, a personal access token of the person that root mints, in an unprotected variable of every environment. The first run must end with no failed, manual or unknown step, and root then reads: the reader and the writer (service accounts where the instance lets the person create them, else group access tokens; the `FINDING` line says which) with Reporter and Developer in the group and no access to the hub; `TOUCHMARK_READ_TOKEN` masked and not protected with scope `*`; `TOUCHMARK_WRITE_TOKEN` protected, masked and hidden with scope `touchmark-distribute`, and the stale copy gone, its token revoked (`GET /personal_access_tokens/self` with it answers 401); the environment; `main` with push No one, merge Maintainers and no force push; no pipeline variables; `protect_merge_request_pipelines` false; the two schedules. A second run and a dry run have only `ok` steps and rotate no token (the active tokens are the same before and after). `doctor --hub-token` with the person's token grades the hub with no `fail` or `warn`. With the runner, a merge request (merged by the person: push to `main` is No one now) names the writer in `hub.yml` and adds a pipeline whose jobs on `main` must succeed: one without an environment runs `probe` (the write key is not visible there) and `plan` (the reader's token reads the group), one with `environment: {name: touchmark-distribute, action: prepare}` runs `doctor` with the writer's token. No token appears in any output or job log.
- **`TestProtectedBranch`**: a wildcard protected branch. One target protects `touchmark/*` for Maintainers alone: the first push is refused and the target is `blocked:rules:protected-branch` with no write. Another lets Developers push but not force-push: the first run opens a merge request, and after a pack change the rebuilt branch needs a force push, which is refused, so the target is `blocked:rules:protected-branch` and its merge request stays as it was.
- **`TestProtectedBranch`**: wildcard protected branches, which the writer, a Developer, reads before it pushes. One target protects `touchmark/*` for Maintainers alone: a dry run and the first run block it `blocked:rules:protected-branch` before any write, with a warning naming the rule. Another lets Developers push but not force-push: the first run opens a merge request, and after a pack change the rebuilt branch needs a force push, which is refused, so the target is `blocked:rules:protected-branch` and its merge request stays as it was. A third has two rules on the sync branch, `touchmark/*` for Maintainers alone and the branch's own name for Developers with force pushes: GitLab applies the most permissive rule that matches a branch, so it is delivered and updated as if unprotected.
- **`TestFacts`**: GitLab behavior touchmark relies on that the documentation leaves open, through the API and git alone: 409 on a duplicate merge request; draft title prefixes and a `Draft:` commit; labels created by name and `add_labels`; quick actions in a person's description on create and update, against touchmark-shaped text; `closed_by` for a person and for a bot, and `merged_by`; a deleted source branch closes the merge request, who GitLab names as the closer (`closed_by` and `resource_state_events`), and what a reopen without the branch does; a force push keeps it open; how soon after a git push the merge request API takes the new branch (`push-then-create`, see below); `repository_object_format` and sha256 projects; git over HTTP with Basic `oauth2:<token>` and other user names; git with a group access token that has the scope `api` alone; descriptions of 200 000 bytes and around 1 MiB; the reader's `members/all` of the writer; rate-limit headers of a default instance; the bot of a revoked group access token, watched for 90 s (the user, and the author of its merge request); which users API view carries the `bot` field; what a fork of a project the reader cannot read shows; `GET /personal_access_tokens/self`; `push_rule` on CE; the pagination headers.

`push-then-create` is why a merge request is opened again after a pause: GitLab checks a new merge request's source branch against the project's cached branch names, which the push's background job refreshes. On CE 19.4.1 under load a `POST /merge_requests` right after a push got `400 {"source_branch":["does not exist"]}` in 8 of 9 tries over three runs, and the merge request opened 1 to 10 s later; `HEAD /repository/branches/:branch` (the cached view) lagged `GET` of the same (Gitaly) by as much. When the branches API finds the branch, the driver's `CreatePR` returns `platform.ErrNotYet` (transient, with a `RetryAfter`), and the core sends the POST again after growing pauses, for at most 30 s.
Expand Down
4 changes: 4 additions & 0 deletions internal/decide/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ const (
// because it would force the branch (GitHub's non_fast_forward): a
// report reason "rules:<rule>".
ReasonRulesNoForce = "rules:non-fast-forward"
// ReasonRulesProtected is the reason of a push to a branch a
// protection rule keeps the writer from pushing to at all (GitLab's
// protected branches, Gitea's branch protection, GitHub's).
ReasonRulesProtected = "rules:protected-branch"
)

// CommentAutoDeclined is the Reason of the StepComment that follows the
Expand Down
59 changes: 59 additions & 0 deletions internal/distribute/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,14 @@ func (r *run) inspectFull(ctx context.Context, t *target, optIn *config.OptIn, s
if hasKept && (len(kept.Marker.Key) > 0 || touches(w.Decision, kept.PR.Number)) {
w.Open = &kept
}
if pushes(w.Decision) {
protected, err := r.readNoPush(ctx, w)
if err != nil {
r.fail(ctx, t, "read the protection of the target's branches", err, "internal")
return nil
}
r.blockProtected(w, protected)
}
r.checkSigning(w)
w.NeedPerms = needPerms(w.Decision.Steps)
r.fillBody(w, sel, reads)
Expand Down Expand Up @@ -769,6 +777,57 @@ func (r *run) readRules(ctx context.Context, w *Work) (platform.Rules, error) {
return rules, nil
}

// readNoPush reads which of the branches w's decision pushes to a
// protection rule keeps the writer from pushing to, when the provider's
// identity is a platform.PushGuard (GitLab's and Gitea's writers; a plan's
// reader never is one). Without one it returns none: such a push meets the
// rule at run time, refused before it writes. A transient failure is tried
// three times.
func (r *run) readNoPush(ctx context.Context, w *Work) ([]platform.Protected, error) {
g, ok := w.t.prov.reader.(platform.PushGuard)
if !ok {
return nil, nil
}
var branches []string
for _, s := range w.Decision.Steps {
if s.Kind == decide.StepPush || s.Kind == decide.StepRecreateBranch {
branches = uniqueNonEmpty(append(branches, s.Branch))
}
}
var out []platform.Protected
err := r.retry(ctx, w.t.prov, func() error {
var err error
out, err = g.NoPush(ctx, w.t.repo, branches)
return err
})
return out, err
}

// blockProtected blocks w's decision as rules:protected-branch when a
// protection rule keeps the writer from a branch it pushes to, with a
// warning that names the rule, before any write: what a push would end in.
// Memory upkeep, which pushes nothing, stays.
func (r *run) blockProtected(w *Work, protected []platform.Protected) {
if len(protected) == 0 {
return
}
d := w.Decision
var upkeep []decide.Step
for _, s := range d.Steps {
if memoryUpkeep(s) {
upkeep = append(upkeep, s)
}
}
w.Decision = decide.TargetDecision{Outcome: decide.OutcomeBlocked, Reason: decide.ReasonRulesProtected, PR: d.PR, Branch: d.Branch, Steps: upkeep}
for _, p := range protected {
rule := ""
if p.Rule != "" {
rule = " (" + p.Rule + ")"
}
w.t.res.Warnings = append(w.t.res.Warnings, fmt.Sprintf("branch %s is protected%s, and the writer may not push to it", p.Branch, rule))
}
}

// checkSigning decides how a push of w is signed. A signature is needed with
// providers[].sign always, and under sign: auto when a rule of the default
// or the sync branch requires signed commits (Rules.SignedCommits from the
Expand Down
6 changes: 5 additions & 1 deletion internal/distribute/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,11 @@ type WriteDeps struct {
// force pushes are decide's NoForcePush (blocked:rules:non-fast-forward
// under our open pull request, a branch created afresh without one).
// Rules the identity cannot read (Rules.Known false) change nothing.
// Then the signature (checkSigning): providers[].sign "always", or a
// A decision that still pushes asks the writer, when it is a
// platform.PushGuard (GitLab, Gitea and Forgejo), which branches it
// pushes to a protection rule keeps it from: one is blocked
// rules:protected-branch before any write (blockProtected), memory
// upkeep aside. Then the signature (checkSigning): providers[].sign "always", or a
// rule requiring signed commits, needs one; the signing key makes it,
// else the platform's API commit (Caps.Commit.API: phase F commits
// through the stage ref), else, outside ModePlan, which never holds
Expand Down
92 changes: 92 additions & 0 deletions internal/distribute/run_protected_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package distribute

import (
"slices"
"testing"

"github.com/bedrock-python/touchmark/internal/platform/fake"
"github.com/bedrock-python/touchmark/internal/report"
)

// protect makes branch patterns of r protected branches the writer may not
// push to.
func (g *gitWorld) protect(r string, patterns ...string) {
g.t.Helper()
g.p.AddRuleset(r, fake.Ruleset{Branches: patterns, NoPush: true})
g.ok()
}

// TestRunProtectedBranch: a protected branch the writer may not push to
// (GitLab's, Gitea's), which the writer as a platform.PushGuard reports,
// blocks the target rules:protected-branch before any write, in a dry run
// and in distribute, with a warning naming the rule; a plan, whose reader
// is no PushGuard, plans the pull request. An open pull request stays as
// it is. Without the guard, or with the rules hidden, the push meets the
// protection and is refused with the same reason, nothing written.
func TestRunProtectedBranch(t *testing.T) {
t.Parallel()
t.Run("guard", func(t *testing.T) {
t.Parallel()
w := newGitWorld(t, fake.WithPushGuard())
x := w.optedIn("acme/x", nil)
w.protect(x.ID, "touchmark/*")
w.optedIn("acme/y", nil)

plan := w.run(w.deps(ModePlan), ModePlan)
want(t, plan, "gh:acme/x", report.OutcomeOpened, "", 0)
for _, mode := range []Mode{ModeDryRun, ModeDistribute} {
rep := w.run(w.deps(mode), mode)
tg := want(t, rep, "gh:acme/x", report.OutcomeBlocked, "rules:protected-branch", 0)
if !hasWarningLike(tg, "branch "+branch+" is protected (touchmark/*)") {
t.Errorf("%v: warnings %q", mode, tg.Warnings)
}
if mode == ModeDistribute {
want(t, rep, "gh:acme/y", report.OutcomeOpened, "", 1)
}
}
if writes := w.writesOn("acme/x"); len(writes) > 0 || w.p.Branch(x.ID, branch) != "" {
t.Errorf("wrote to acme/x: %q, branch at %q", writes, w.p.Branch(x.ID, branch))
}
})

t.Run("open pull request", func(t *testing.T) {
t.Parallel()
w := newGitWorld(t, fake.WithPushGuard())
x := w.optedIn("acme/x", nil)
want(t, w.distribute(nil), "gh:acme/x", report.OutcomeOpened, "", 1)
head := w.p.Branch(x.ID, branch)
w.protect(x.ID, branch)
w.pack(slices.Concat(baseFiles, []string{"docs/new.md", version("docs/new.md", 1)})...)

dry := w.run(w.deps(ModeDryRun), ModeDryRun)
want(t, dry, "gh:acme/x", report.OutcomeBlocked, "rules:protected-branch", 1)
rep := w.run(w.deps(ModeDistribute), ModeDistribute)
tg := want(t, rep, "gh:acme/x", report.OutcomeBlocked, "rules:protected-branch", 1)
if !hasWarningLike(tg, "branch "+branch+" is protected ("+branch+")") {
t.Errorf("warnings %q", tg.Warnings)
}
if writes := w.writesOn("acme/x"); len(writes) > 0 || w.p.Branch(x.ID, branch) != head {
t.Errorf("wrote to acme/x: %q, branch moved from %s to %s", writes, head, w.p.Branch(x.ID, branch))
}
})

for name, opts := range map[string][]fake.Option{"no guard": nil, "hidden": {fake.WithPushGuard()}} {
t.Run(name, func(t *testing.T) {
t.Parallel()
w := newGitWorld(t, opts...)
x := w.optedIn("acme/x", nil)
w.protect(x.ID, "touchmark/*")
w.p.HideRules(x.ID, true)
w.ok()
want(t, w.both(nil), "gh:acme/x", report.OutcomeOpened, "", 0)
rep := w.run(w.deps(ModeDistribute), ModeDistribute)
tg := want(t, rep, "gh:acme/x", report.OutcomeBlocked, "rules:protected-branch", 0)
if tg.Writes != 0 || w.p.Branch(x.ID, branch) != "" {
t.Errorf("%d writes, branch at %q", tg.Writes, w.p.Branch(x.ID, branch))
}
if !hasWarningLike(tg, "not allowed to push code to protected branches") {
t.Errorf("warnings %q", tg.Warnings)
}
})
}
}
Loading
Loading