diff --git a/README.md b/README.md index 1316b27..6de3437 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,11 @@ Three ways in, one version number: - **Review, never merge** — every change arrives as a pull request a person merges; there is no auto-merge option - **Opt-in** — a repository receives nothing until it adds `.engineering-assets.yml`, which - can add packs and `ignore` paths + can add packs and `ignore` paths; a hub that owns the decision subscribes repositories + itself (`opt_in: assumed`), and `enabled: false` in the file still opts one out +- **Targets as you name them** — a repository, an organisation or a group by path or by + web URL, narrowed by topics and path globs (`acme/svc-*`), minus `exclude` globs + (`corp:platform/legacy/**`) - **Four platforms, one hub** — GitHub (github.com, GHE.com, Enterprise Server), GitLab (gitlab.com and self-managed), Gitea and Forgejo; one hub can deliver to several at once - **Declines remembered** — content in a pull request closed without merging is not @@ -222,9 +226,9 @@ every flag. | File | Lives in | What it holds | |---|---|---| | `hub.yml` | the hub | the hub's id, the platforms it delivers to with their writers, commit and pull request settings, security settings, pack descriptions, `requires` and `formerly` | -| `targets.yml` | the hub | which repositories get which packs: by repository, organisation or group, filtered by topic | +| `targets.yml` | the hub | which repositories get which packs: by repository (its path or web URL), organisation or group, filtered by topic and path pattern, minus `exclude` patterns; and whether the hub subscribes them without an opt-in file (`opt_in: assumed`) | | `.touchmark/operations.yml` | the hub | one-off overrides of a safeguard, merged through review: rebuild a paused branch, propose declined content again, allow a mass close | -| `.engineering-assets.yml` | each target | the repository's consent; packs to add and paths to `ignore` | +| `.engineering-assets.yml` | each target | the repository's consent, or with `enabled: false` its refusal; packs to add and paths to `ignore` | Credentials come from the environment, never from flags or files: `TOUCHMARK_[_]READ_TOKEN` (or `…_READ_APP_ID` with `…_READ_APP_KEY` for a GitHub App), diff --git a/action.yml b/action.yml index 4c1d565..30f4f78 100644 --- a/action.yml +++ b/action.yml @@ -56,7 +56,7 @@ inputs: required: false default: "false" assume-opt-in: - description: "plan: report as if every target had opted in (--assume-opt-in). true or false." + description: "plan: report as if every target without an opt-in file had opted in (--assume-opt-in); enabled: false still opts a target out. true or false." required: false default: "false" dry-run: diff --git a/docs/agents.md b/docs/agents.md index d160ca1..64317dd 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -45,7 +45,8 @@ file is copied byte for byte. It does not merge a team's edits into a pack's new an edited file is the team's and is never touched again. It keeps no state in targets — no lock file, no header, no marker in their files. It never checks a target out in CI, and never runs a script, hook, filter or LFS from the hub or a target. It delivers -nothing to a repository without the opt-in file. There is no pull mode (targets do not +nothing to a repository without the opt-in file, unless `targets.yml` subscribes it +(`opt_in: assumed`). There is no pull mode (targets do not fetch from the hub), no remote pack sources, and no Bitbucket or Azure DevOps driver. ## Mental model @@ -75,7 +76,17 @@ configures it: `hub.yml`, `targets.yml`, `.touchmark/operations.yml`, the CI fil later wins. The hub sets the minimum; a target can add packs and `ignore` paths, never remove a pack the hub assigns. * **Opt-in.** A target receives nothing until `.engineering-assets.yml` exists at its - root. An empty file is consent. + root. An empty file is consent. An entry of `targets.yml` with `opt_in: assumed` + subscribes the repositories it selects instead: they count as opted in without the + file, as if it were empty; one such entry is enough. The file still wins: + `enabled: false` in it opts any repository out (`skipped:opted-out`, its open sync pull + request closed), and deleting it returns a subscribed repository to the hub's + subscription rather than opting it out. +* **Targets** are named `[:]` or by web URL (`https://host/owner/name`), + whose provider is the one in `hub.yml` at that url; output and `--only` always use + `:`. `exclude` entries and the `match` filter of `org`/`group` entries + are globs: `*` within a segment, `**` across segments, `?` one character, case + ignored; YAML needs a glob that starts with `*` quoted (`"**/legacy"`). * **Accounts.** A **reader** (read-only) runs `plan` on hub pull requests. A **writer**, a different account, runs `distribute` and `doctor` from the hub's default branch only. The **hub channel**, the CI's own token (`GITHUB_TOKEN`, `CI_JOB_TOKEN`), reaches the hub @@ -253,14 +264,22 @@ ten `::error` and ten `::warning` annotations. | `packs..description`, `.requires`, `.formerly` | none | metadata; `requires` adds and orders dependencies; `formerly` keeps a renamed pack's history | `targets.yml` (in the hub; `touchmark schema targets`): `defaults.provider`, -`defaults.packs`; `targets[]` entries with exactly one of `repo` (`owner/name`, or -`provider:path`), `org` or `group` (a namespace; with `topics` that must all match, -`subgroups` default `true`, `forks` default `false`), each with optional `provider` and -`packs`; `exclude[]`. +`defaults.packs`, `defaults.opt_in`; `targets[]` entries with exactly one of `repo` +(`owner/name`, `provider:path`, or a web URL), `org` or `group` (a namespace, by path or +web URL; with `topics` that must all match, `subgroups` default `true`, `forks` default +`false`, `match` a list of globs over the full path of which one must match), each with +optional `provider`, `packs` and `opt_in` (`required`, the default, or `assumed`); +`exclude[]` (repositories, globs or web URLs; it wins over every entry). A URL resolves to +the one provider whose `url` it lies under (`github` defaults to `https://github.com`, +`gitlab` to `https://gitlab.com`); none fails `check`, and so do several unless the +entry's `provider` or `defaults.provider` picks one of them, credentials, a query, a +fragment, and any scheme but `https` (`http` for loopback only). On GitHub, Gitea and +Forgejo a repository URL, and an `exclude` URL without `**`, names `owner/name`. `.engineering-assets.yml` (in each target; `touchmark schema opt-in`): `version`, -`packs` (added to the hub's), `ignore` (paths or globs, `**` for any depth, at most -1000; a plain path covers everything under it). At most 64 KiB. +`enabled` (default `true`; `false` opts out whatever the hub says), `packs` (added to +the hub's), `ignore` (paths or globs, `**` for any depth, at most 1000; a plain path +covers everything under it). At most 64 KiB. `.touchmark/operations.yml` (in the hub; `touchmark schema operations`): `recreate[]` (`target`, `head`: acts while the branch head is that commit), @@ -301,8 +320,10 @@ from its environment once read: 5. **The writer has no access to the hub.** Install the Apps on targets only; keep the hub outside the GitLab groups the accounts belong to. `doctor` fails when the writer can push to the hub, and warns when it can see a private one. -6. **Nothing reaches a repository without its opt-in file.** `.engineering-assets.yml` - at the root, possibly empty. Never ship that file in a pack: `check` rejects it. +6. **Nothing reaches a repository without its consent.** `.engineering-assets.yml` at + the root, possibly empty, or an entry with `opt_in: assumed` when the hub owns the + decision; `enabled: false` in the file overrides the hub. Never ship that file in a + pack: `check` rejects it. 7. **Only committed packs ship.** `distribute` reads the hub's HEAD commit and refuses `--worktree`; it runs only when HEAD is the tip of the default branch (otherwise the run is `superseded` and writes nothing). @@ -436,7 +457,7 @@ Each target ends with one outcome, and a reason for most: |---|---|---| | `opened`, `updated`, `unchanged`, `closed` | `updated`: `content`, `rebase`, `recreate`, `body`, `title`, `base-renamed`; `closed`: `no-diff`, `opted-out`, `target-dropped`, `duplicate` | nothing | | `declined` | the declined pull request | nothing; see [memory](concepts/memory.md) to propose again | -| `skipped` | `not-opted-in`, `archived`, `disabled`, `empty`, `mirror`, `pending-deletion`, `prs-disabled`, `sha256`, `unsafe-opt-in`, `private-in-public-hub`, `superseded` | add the opt-in file, or nothing | +| `skipped` | `not-opted-in`, `opted-out` (the opt-in file says `enabled: false`), `archived`, `disabled`, `empty`, `mirror`, `pending-deletion`, `prs-disabled`, `sha256`, `unsafe-opt-in`, `private-in-public-hub`, `superseded` | add the opt-in file (or `opt_in: assumed` in the hub), or nothing | | `blocked` | `edited` (someone pushed to the sync branch), `branch-taken`, `branch-in-use`, `opt-in-invalid`, `marker-invalid`, `rules:`, `permission:`, `cannot-sign`, `archived`, `mass-close` | tick *Rebuild this branch* or add a `recreate` entry; fix the opt-in file; grant the permission; add a signing key; `allow_mass_close` | | `deferred` | `rate-limit`, `deadline`, `rollout-limit`, `provider-down`, `interrupted`, `cooldown` | nothing: the next run continues | | `failed` | `transient`, `auth`, `access`, `git`, `integrity`, `race`, `secret-exposure`, `internal` | read the report's reason; `auth` and `access` are the credential | @@ -457,10 +478,10 @@ Fetch a page when the task is the one named beside it. | [A hub on GitHub](getting-started/github.md) | creating a hub with GitHub Apps and the `touchmark-distribute` environment | | [A hub on GitLab](getting-started/gitlab.md) | creating a hub with service accounts and protected variables | | [A hub on Gitea or Forgejo](getting-started/gitea-forgejo.md) | delivering to or hosting on Gitea and Forgejo | -| [Opt a repository in](getting-started/opt-in.md) | writing a target's `.engineering-assets.yml` | +| [Opt a repository in](getting-started/opt-in.md) | writing a target's `.engineering-assets.yml`; a hub that subscribes repositories (`opt_in: assumed`) and how they opt out | | [How it works](concepts/overview.md) | the flow from a hub pull request to a sync pull request | | [Ownership by provenance](concepts/ownership.md) | the file states, `local`, `--adopt`, CRLF, `orphaned` | -| [Packs and opt-in](concepts/packs.md) | pack selection order, `requires`, `formerly`, seed files | +| [Packs and opt-in](concepts/packs.md) | pack selection order, `requires`, `formerly`, seed files, consent by file or by the hub | | [Delivery and the sync branch](concepts/delivery.md) | the branch, the commit, signing, pull request fields, stale pull requests | | [Memory of declined pull requests](concepts/memory.md) | why a pull request did not come back, and how to bring it back | | [Security model](concepts/security.md) | the accounts, the probe, write isolation, what a target can and cannot do | diff --git a/docs/concepts/delivery.md b/docs/concepts/delivery.md index 72ca914..69a2c38 100644 --- a/docs/concepts/delivery.md +++ b/docs/concepts/delivery.md @@ -85,7 +85,7 @@ One pull request per target, from the sync branch to the default branch. | Field | Belongs to | touchmark writes it | |---|---|---| -| body | touchmark | when the changes it proposes change, and to record a decline, a close or a rebuild | +| body | touchmark | when the changes it proposes change, and to record a decline, a close or a rebuild; also when a repository the hub subscribed (`opt_in: assumed`) gains or loses its opt-in file, which adds or drops the subscription paragraph | | title | people, after creation | at creation; later only if `pr.title` changed and nobody edited the title | | draft | people, after creation | at creation (`pr.draft`) | | labels | shared | at creation (`pr.labels`); later only labels it never set; never removes one | @@ -94,6 +94,9 @@ One pull request per target, from the sync branch to the default branch. The body, in English: 1. the text of `pr.intro_file`, then the hub, the hub commit and the packs; + for a repository the hub subscribed without an opt-in file, a paragraph on how to + choose packs or keep files out (add the opt-in file with `packs` or `ignore`) and how + to opt out (`enabled: false` in it). Never cut; 2. **⚠ Sensitive paths**: changes to workflows, CI configuration, agent settings, skills, subagents, hooks, CODEOWNERS, `.gitattributes` and executable files (plus `sensitive_paths` from `hub.yml`). Never cut; @@ -122,7 +125,7 @@ other pull request. | Reason | When | |---|---| | `no-diff` | the target already has everything; the branch is deleted | -| `opted-out` | the opt-in file was deleted | +| `opted-out` | the repository is no longer opted in: its opt-in file says `enabled: false`, or it has no opt-in file and the hub does not subscribe it (the file was deleted, or the hub dropped `opt_in: assumed`); the comment says which | | `target-dropped` | the target left `targets.yml` or entered `exclude` | | `duplicate` | two of touchmark's pull requests are open, on the branch and on an alias | diff --git a/docs/concepts/memory.md b/docs/concepts/memory.md index 191094b..cc8abfa 100644 --- a/docs/concepts/memory.md +++ b/docs/concepts/memory.md @@ -33,6 +33,7 @@ covered by the target's declines. It does open one when anything is new: | #7: A created, B created | B was edited in the target (it is `local`) | a new pull request with A | | #7: A created, B created | B added to `ignore` | a new pull request with A | | #7: A created | a comment changed in the opt-in file | `declined`: the parsed file is the same | +| #7: A created, in a repository the hub subscribed without an opt-in file | an empty opt-in file added | `declined`: the file is parsed the same as the empty one assumed | | #7: A created; #9: B created | nothing | `declined`: both pairs are covered | | #7: A created, B created | the pack changes B | a new pull request with A and the new B, saying *A was declined in #7* | | #7: A at v1 | A at v2 was merged, then the hub went back to v1 | a new pull request: `v2 → v1` is another pair | @@ -67,7 +68,8 @@ To undo a decline: - **from the hub**, add a `forget_declines` entry to `.touchmark/operations.yml` and merge it through review (see [One-off operations](../guide/operations.md)). -To refuse a file for good instead, add it to `ignore`, or delete the opt-in file to stop +To refuse a file for good instead, add it to `ignore`, or set `enabled: false` in the opt-in +file (deleting it works too, unless the hub subscribed the repository) to stop receiving anything. ## Limits diff --git a/docs/concepts/overview.md b/docs/concepts/overview.md index 74ed95c..d32727f 100644 --- a/docs/concepts/overview.md +++ b/docs/concepts/overview.md @@ -22,9 +22,9 @@ your-org/engineering-assets reports what each would receive: pull requests to open, update and close, the sensitive paths, the API writes it would cost. Nothing is written. 2. **A maintainer merges it.** On the default branch, `touchmark distribute` runs with - the write account. For every target that has opted in and has a difference, it - builds one commit on the sync branch `touchmark/` and opens or updates one pull - request. A second run on the same inputs writes nothing. + the write account. For every target that has opted in (or that the hub subscribes) + and has a difference, it builds one commit on the sync branch `touchmark/` and + opens or updates one pull request. A second run on the same inputs writes nothing. 3. **A person in each target reviews it.** They merge it, or edit files (which makes them the repository's own), or close it, which touchmark remembers as a decline. @@ -40,8 +40,8 @@ before writing, and its report is the same as `plan`'s. | Phase | What happens | |---|---| | guards | git version, configuration, `check`, the write isolation probe, the hub's fingerprint; the hub's HEAD must be the tip of its default branch, or the run is `superseded` and writes nothing | -| resolve | `targets.yml` becomes a list of repositories: organisations and groups listed, topics filtered, `exclude` applied, duplicates merged by repository id | -| inspect | per target, in parallel: archived, empty or mirror repositories skipped; the opt-in file read; the target's tree fetched without blobs or a checkout; the decision made | +| resolve | `targets.yml` becomes a list of repositories: web URLs matched to providers, organisations and groups listed, `topics` and `match` filtered, `exclude` patterns applied, duplicates merged by repository id | +| inspect | per target, in parallel: archived, empty or mirror repositories skipped; the opt-in file read (an empty one assumed where `targets.yml` subscribes the target; `enabled: false` skips it); the target's tree fetched without blobs or a checkout; the decision made | | sweep | sync pull requests of this hub in repositories that are no longer targets are found, to be closed | | gate | the guards against mass actions: at most `limits.max_new_prs_per_run` new pull requests, and no mass close | | execute | `distribute` only: closes, then updates, then new pull requests, under each provider's write budget | diff --git a/docs/concepts/packs.md b/docs/concepts/packs.md index 85239fc..f3eec04 100644 --- a/docs/concepts/packs.md +++ b/docs/concepts/packs.md @@ -51,7 +51,8 @@ exception is visible in the target itself. ## Opt-in A target receives nothing until it has the opt-in file, `.engineering-assets.yml` at its -root (`opt_in_file` in `hub.yml` renames it). An empty file is consent. +root (`opt_in_file` in `hub.yml` renames it), or the hub subscribes it (below). An empty +file is consent. ```yaml version: 1 @@ -68,10 +69,31 @@ ignore: directories. - Editing `packs` or `ignore` is a new choice of the team, so it lifts every decline the repository made: see [memory](memory.md). +- `enabled: false` opts the repository out, whatever the hub says. The opt-in file belongs to each repository: `touchmark check` rejects a pack that ships it. +### When the hub subscribes + +Consent by file suits a hub that offers packs to teams. A hub that keeps its own +organisation's repositories in line can subscribe them instead: `opt_in: assumed` on an +entry of `targets.yml` (or in its `defaults`) makes every repository the entry selects +count as opted in without the file, as if it had an empty one. + +| The repository has | It gets | +|---|---| +| no opt-in file, and no entry that selects it says `assumed` | nothing (`skipped:not-opted-in`) | +| no opt-in file, and an entry that selects it says `assumed` | the packs of `targets.yml`, nothing ignored | +| an opt-in file | the packs of `targets.yml` and of the file, minus what it ignores | +| an opt-in file with `enabled: false` | nothing (`skipped:opted-out`); its open sync pull request is closed | + +One entry with `assumed` is enough: entries add up, as their packs do. The consent moves, +the choice does not: the first sync pull request of a subscribed repository says the hub +subscribed it and how to opt out, and closing it is a decline like any other. Deleting +the opt-in file of a subscribed repository returns it to the hub's subscription; only +`enabled: false` opts it out. See [targets.yml](../reference/targets.md#opt-in). + ## Renaming and retiring - **Rename** a pack by moving its directory and listing the old name under `formerly`. diff --git a/docs/getting-started/opt-in.md b/docs/getting-started/opt-in.md index 5bce067..597d5aa 100644 --- a/docs/getting-started/opt-in.md +++ b/docs/getting-started/opt-in.md @@ -1,8 +1,9 @@ # Opt a repository in Nothing happens to a repository until it has `.engineering-assets.yml` at its root. The -file can be empty: its presence is the consent. It can also ask for more packs and keep -some paths for itself: +file can be empty: its presence is the consent. (A hub can also subscribe repositories +itself: see [when the hub subscribes a repository](#when-the-hub-subscribes-a-repository).) +The file can also ask for more packs and keep some paths for itself: ```yaml # .engineering-assets.yml @@ -27,7 +28,8 @@ runs [`touchmark status`](../guide/local.md) in a checkout. - **Change its mind.** Editing `packs` or `ignore` lifts the memory of declined pull requests: content the team declined by closing a sync pull request is proposed again. Comments and formatting don't count. -- **Opt out.** Deleting the file stops delivery; the open sync pull request is closed +- **Opt out.** `enabled: false` in the file stops delivery, and so does deleting the + file, unless the hub subscribed the repository; the open sync pull request is closed with the reason `opted-out`. Files already merged stay. The file is read through the platform's API from the default branch: at most 64 KiB, a @@ -50,8 +52,10 @@ targets: topics: [python-library] packs: [python-library] - repo: corp:platform/api # a repository on another provider from hub.yml + - repo: https://gitlab.example.com/platform/web # a web URL: the provider at that url exclude: - your-org/legacy-monolith + - your-org/*-archive # a glob: * within a segment, ** across segments ``` A repository's packs, in order: `defaults.packs`, then the `packs` of every entry that @@ -59,10 +63,51 @@ matches it (in file order), then the `packs` of its opt-in file. Duplicates are and each pack's `requires` from `hub.yml` comes before it. When two packs ship the same path, the later one wins. See [Packs and opt-in](../concepts/packs.md). +## When the hub subscribes a repository + +A hub that keeps its own repositories in sync, rather than offering packs to teams that +ask, marks entries with `opt_in: assumed`: + +```yaml +# targets.yml in the hub +version: 1 +defaults: + packs: [agents] +targets: + - org: your-org + match: [your-org/svc-*] # only the services + opt_in: assumed # no opt-in file needed +``` + +A repository such an entry selects counts as opted in without the file: it gets the packs +of `targets.yml`, and its first sync pull request says the hub subscribed it. The team +still decides: + +- **Merge it**, or **close it**: closing is a decline, and the same content is not + proposed again. +- **Choose packs or keep files**: add the opt-in file with `packs` or `ignore`; it works + as for any target. +- **Opt out**: add the opt-in file with `enabled: false`: + + ```yaml + # .engineering-assets.yml + version: 1 + enabled: false + ``` + + The open sync pull request is closed, and nothing comes until the file says otherwise. + Deleting the opt-in file of a subscribed repository does not opt it out: it returns it + to the hub's subscription. + +See [targets.yml](../reference/targets.md#opt-in) for the rules. + ## What the team sees A sync pull request from the hub's writer on the branch `touchmark/`: +- when the hub subscribed the repository without an opt-in file (`opt_in: assumed`), a + paragraph that says so, with how to choose packs or keep files out, and how to opt + out; - a table of every change, by file and pack; - a **⚠ Sensitive paths** section listing changes to workflows, CI configuration, agent settings, skills and subagents, CODEOWNERS and executable files, which is never cut; diff --git a/docs/guide/doctor.md b/docs/guide/doctor.md index 12f837d..1295284 100644 --- a/docs/guide/doctor.md +++ b/docs/guide/doctor.md @@ -28,7 +28,9 @@ Each check has a name, a status and a detail. The names you will see: | `write-isolation` | the hub | `security.write_isolation` is `none`: a warning with its reason | A check touchmark cannot read is `unknown`, never `ok`. Targets it does not check say why -(`skipped`: not opted in, archived, deferred, …). +(`skipped`: not opted in, opted out, archived, deferred, …). A repository `targets.yml` +subscribes (`opt_in: assumed`) is checked without an opt-in file, as `distribute` writes +to it. ## Where the hub keeps its write key: `--hub-token` diff --git a/docs/guide/faq.md b/docs/guide/faq.md index db52398..1f8722c 100644 --- a/docs/guide/faq.md +++ b/docs/guide/faq.md @@ -50,8 +50,18 @@ Run `touchmark apply` in a checkout with the hub next to it, and commit the resu propose and is closed as `no-diff`. **How do we leave entirely?** -Delete `.engineering-assets.yml`. touchmark closes its open pull request as `opted-out` -and delivers nothing more. Files already merged stay. +Put `enabled: false` in `.engineering-assets.yml`, or delete the file. touchmark closes +its open pull request as `opted-out` and delivers nothing more. Files already merged +stay. If the hub subscribed your repository (the sync pull request says so), deleting +the file is not enough: the hub's subscription applies again, so add the file with +`enabled: false`. + +**We never added an opt-in file. Why did we get a sync pull request?** +The hub subscribed your repository (`opt_in: assumed` in its `targets.yml`), and the +pull request's description says so. Treat it like any other: merge it, or close it and +the same content won't come back. To choose packs or keep files, add +`.engineering-assets.yml` with `packs` or `ignore`; to stop them, add it with +`enabled: false`. ## For hub maintainers @@ -59,7 +69,21 @@ and delivers nothing more. Files already merged stay. A public hub's CI logs are public, so touchmark skips private and internal targets there and prints only how many. A hub whose visibility the CI does not report counts as public. Set `security.private_targets_in_public_hub: deliver` to deliver to them anyway; their -names will then appear in the logs. Repositories without the opt-in file are skipped too. +names will then appear in the logs. Repositories without the opt-in file are skipped too +(`not-opted-in`), unless an entry that selects them has `opt_in: assumed`, and so are +repositories whose opt-in file says `enabled: false` (`opted-out`). + +**Can the hub subscribe repositories without asking each team?** +Yes: `opt_in: assumed` on an entry of `targets.yml`, or in `defaults`. The repositories it +selects get their packs without an opt-in file, and the first sync pull request tells the +team how to choose packs or opt out. Narrow an organisation with `match` (`acme/svc-*`) +and leave repositories out with `exclude` patterns (`corp:platform/legacy/**`). See +[targets.yml](../reference/targets.md#opt-in). + +**Can targets.yml name a repository by its URL?** +Yes: `repo: https://gitlab.example.com/platform/api` works, and so do `org`, `group` and +`exclude`. touchmark finds the provider in `hub.yml` whose `url` the URL lies under and +reports the target as `:`. `check` fails on a URL no provider serves. **Can two packs ship the same path?** Yes. Packs are layered in order, and the later one wins. diff --git a/docs/guide/local.md b/docs/guide/local.md index a9a77dc..5058d19 100644 --- a/docs/guide/local.md +++ b/docs/guide/local.md @@ -65,6 +65,22 @@ that might match, `status` warns, and `apply` refuses until you name the packs: touchmark apply --hub ~/src/engineering-assets --packs agents,claude ``` +The same goes for a hub that subscribes repositories (`opt_in: assumed`). In a checkout +without the opt-in file: + +- a `repo:` entry with `assumed` that names the target opts it in: `status` prints + `opted in by targets.yml (opt_in: assumed)`, and `apply` writes the packs of + `targets.yml`; +- an `org:` or `group:` entry with `assumed` that might hold the target cannot be + resolved here: the target counts as not opted in, and `status` warns that `plan` has + the answer; +- without `--repo` or an `origin` remote, touchmark cannot tell, and warns when + `targets.yml` subscribes anything. + +An opt-in file in the checkout settles it either way; with `enabled: false` in it, +`status` prints `opted out` and `apply` writes nothing. In `--format json` the target's +`opt_in` says which: `file`, `assumed`, `opted-out` or `none`. + ## Try a pack before committing it ```sh diff --git a/docs/guide/migrate.md b/docs/guide/migrate.md index 455bb0b..a383952 100644 --- a/docs/guide/migrate.md +++ b/docs/guide/migrate.md @@ -75,8 +75,12 @@ cannot express, and 1 when an account it checked does not exist. on the same GitLab instance ([A hub on GitLab](../getting-started/gitlab.md)). 2. Save the three files, resolve every `TODO`, move the files multi-gitter's script copied into packs under `packs/`, and assign them in `targets.yml`. -3. Make sure every target has its opt-in file: multi-gitter delivered without consent, - touchmark does not. +3. Decide how targets opt in. multi-gitter delivered without asking; touchmark waits + for each repository's opt-in file unless `targets.yml` subscribes it. + `defaults.opt_in: assumed` keeps multi-gitter's behaviour: every target gets its + packs without the file, and a team opts out with `enabled: false` in it. Without it, + add the opt-in file to every target first: the first `distribute` skips the others + (`not-opted-in`) and takes none of their multi-gitter merge requests over. 4. Run `touchmark check`, open a merge request and read `plan`: it shows what `distribute` would do with each multi-gitter merge request, and the effect of the `adopt_unmarked` entry. @@ -100,7 +104,7 @@ token's bot stays the author of its merge requests, and stays in `known_authors` | multi-gitter | touchmark | |---|---| -| every selected repository gets the change | only repositories with `.engineering-assets.yml` | +| every selected repository gets the change | repositories with `.engineering-assets.yml`, or those `targets.yml` subscribes (`opt_in: assumed`) | | a file a team changed is overwritten, or the script decides | a changed file is the team's, never touched again | | a closed merge request comes back on the next run | a declined merge request is remembered | | one body for the whole run | a body per target: what changes there, what is sensitive | diff --git a/docs/guide/providers.md b/docs/guide/providers.md index ee00aa6..3e0c928 100644 --- a/docs/guide/providers.md +++ b/docs/guide/providers.md @@ -80,17 +80,25 @@ defaults: targets: - repo: acme/billing # gh, the default - repo: corp:platform/api # : + - repo: https://gitlab.example.com/platform/web # corp: the URL is under its url - provider: corp group: platform # with its subgroups; subgroups: false turns them off topics: [python] - - org: cb:acme + - org: https://codeberg.org/acme # cb:acme exclude: - corp:platform/sandbox + - corp:platform/legacy/** # a subgroup and everything beneath it ``` A target's provider is, in order: the entry's `provider`, the `:` prefix, `defaults.provider`, the only provider. When that leaves a choice, `check` fails. +A target written as a web URL takes the provider whose `url` it lies under, matched by +scheme, host, port and path (`https://example.com/gitlab/platform/api` is under a +provider at `https://example.com/gitlab`); exactly one must match. touchmark rewrites +it as `:` before anything else reads it. See +[targets.yml](../reference/targets.md#web-urls). + Everywhere a single repository is named — in output, in `--only`, in `.touchmark/operations.yml` — it is `:`. Inside a run, a repository is identified by its host and immutable id, so one repository named by two entries, or by diff --git a/docs/guide/troubleshooting.md b/docs/guide/troubleshooting.md index 6bc55e3..03a5c62 100644 --- a/docs/guide/troubleshooting.md +++ b/docs/guide/troubleshooting.md @@ -16,6 +16,9 @@ These exit with code 2 and write nothing. | `hub checkout is shallow: full history is required` | `actions/checkout` without `fetch-depth: 0`, or `GIT_DEPTH` not `"0"` | fetch the whole history | | `hub checkout is a partial clone` | a clone with `--filter` | clone without it | | `id: still the template placeholder "change-me"` | a new hub | set `id` in `hub.yml` | +| `targets.yml: targets[N].repo: is not under the url of any provider` | a target written as a web URL whose host or path no provider in `hub.yml` has | add the provider, fix its `url`, or write the target as `:` | +| `… is under the url of providers …` | two providers share the instance of a URL | name one with `provider:` on the entry, or write the target as `:` | +| `targets.yml: its web URLs are matched with the providers' urls, which are unknown` | a hub without `providers` that names targets by URL, run outside its own CI: an `origin` remote names the provider only on `github.com`, a `*.ghe.com` host or `gitlab.com` | list the instance under `providers` in `hub.yml`; on one of those hosts, adding the `origin` remote is enough | | `… looks like a secret …; remove it from the file and revoke it` | a token pasted into a configuration file | remove it, and revoke the token: it is in the history | | `TOUCHMARK_… is visible in this job, which any branch of the hub can run` | the probe saw a write key | keep write keys only in the protected environment or variable | | a write key visible outside the environment, or the probe not configured | GitHub: `TOUCHMARK_KEY_EXPOSED` is `true`, or unset | remove the repository or organisation secret; pass the probe job's output | @@ -71,12 +74,17 @@ predict a rule block that `distribute` will not hit. `doctor` checks with the wr ## Nothing happens to a repository -- It has no `.engineering-assets.yml` at its root (`skipped:not-opted-in`). +- It has no `.engineering-assets.yml` at its root (`skipped:not-opted-in`), and no entry + that selects it has `opt_in: assumed`. +- Its `.engineering-assets.yml` says `enabled: false` (`skipped:opted-out`). +- An `exclude` entry covers it: a pattern such as `acme/*` or `platform/**` covers more + than the repositories it names. `check` warns when one covers a `repo:` entry. - It is archived, disabled, empty, a mirror, pending deletion, has pull requests turned off, or uses SHA-256 objects (`skipped` with that reason). - The hub is public and the repository is not (`private-in-public-hub`, only counted). -- An `org` or `group` entry did not select it: `topics` must all match, forks are left out - unless `forks: true`, nested groups need `subgroups` (on by default). +- An `org` or `group` entry did not select it: `topics` must all match, its full path must + match one of the `match` patterns (`acme/svc-*` leaves out `acme/sub/svc-a`), forks are + left out unless `forks: true`, nested groups need `subgroups` (on by default). - It declined this content before (`declined`): see [memory](../concepts/memory.md). - It already has everything (`unchanged`). diff --git a/docs/project/threat-model.md b/docs/project/threat-model.md index 957077d..a12aa13 100644 --- a/docs/project/threat-model.md +++ b/docs/project/threat-model.md @@ -57,7 +57,7 @@ How to read the tests column: | The HTTP client sends `Authorization` only to its account's host, never over a redirect, never over plain http but to loopback; the hub channel only to the hub's repository; every request goes through `httpx` | `httpx`, `hubch` | `TestAuthToAllowedHost`, `TestAuthRefusedForOtherHost`, `TestAuthCaseInsensitiveHosts`, `TestRefusedRequestsNeverReachTheTransport`, `TestNoRedirectWithAuth`, `TestPlainHTTPToLoopback`, `TestRESTChannelTokenGoesOnlyToTheAPIHost`, `TestGitLabChannelIsolation`, `TestOnlyHTTPXSpeaksHTTP` | — | | In a hub pull request, `providers` and `ca_file` come from the default branch, a new or moved provider is planned without credentials, and the sensitive paths are the union of both branches' | `cli.planProviders`, `cli.unionSensitive` | `TestPlanHubPullRequestProviders`, `TestPlanDefaultBranchProviders`, `TestUntrustedRun`, `TestPlanSensitivePathsUnion` *(git 2.45)* | Where the default branch names no writer for a provider (a new hub's first pull request, which names it), plan takes the pull request's: the writer decides no destination of a credential, only which pull requests the plan's report counts as touchmark's (`TestPlanHubPullRequestProviders/added_writer`) | | The redaction registry: every token and JWT in its forms, secrets drivers mint registered, `::add-mask::` on GitHub Actions, merged automata so that masking stays linear in the targets | `redact`, the drivers, `cli.maskNewSecrets` | `TestAddForms`, `TestOnAdd`, `TestManySecrets`, `TestGitHubMask`, `TestMaskNewSecrets`, `TestJWT`, `TestInstallationTokens`, `FuzzWriter` | — | -| A secret in a body, title, label, comment, commit message or branch name stops the target before its first write: `failed:secret-exposure` | `distribute` (`targetExec.leak`); config files refuse token-like strings | `TestLeakEveryText`, `TestExecuteSecretExposure`, `TestExecuteRefusesBeforeWriting` *(git 2.45)*, `TestExecuteSecretInPath` *(git 2.45)*, `TestParseRejectsSecrets`, `TestParseOperationsSecrets` | — | +| A secret in a body, title, label, comment, commit message or branch name stops the target before its first write: `failed:secret-exposure` | `distribute` (`targetExec.leak`); config files refuse token-like strings, and a URL with credentials (`user:password@`), which `targets.yml` refuses, has them cut from every message | `TestLeakEveryText`, `TestExecuteSecretExposure`, `TestExecuteRefusesBeforeWriting` *(git 2.45)*, `TestExecuteSecretInPath` *(git 2.45)*, `TestParseRejectsSecrets`, `TestParseOperationsSecrets`, `TestDisplayURL` | — | | No touchmark token, `GITHUB_TOKEN` or `CI_JOB_TOKEN` in the environment of a target's git; the environment keeps `PATH`, system and temporary directories, proxies and `SSL_CERT_*`; `GIT_CONFIG_NOSYSTEM`, an empty global config, a temporary `HOME`, `GIT_ATTR_NOSYSTEM`, `GIT_TERMINAL_PROMPT=0`, empty `credential.helper` and askpass | `gitx.Isolation`, `gitx.Git` | `TestTargetIsolation`, `TestInitTargetDirAndHome`, `TestRunIgnoresInheritedEnv`, `TestRunInherit`, `TestCanaries` | — | | A timeout or cancel kills git's whole process tree (on Windows the launcher's children too); stale `shallow.lock` files go before the next fetch | `gitx` (`configureCancel`, `removeStaleLocks`) | `TestCancelledFetch`, `TestNetworkTimeout`, `TestStaleLocksRemoved`, `TestRunCancelled` | — | | git's settings in a target: hooks off, `core.fsmonitor=false`, `protocol.allow=never` but https (and http to a loopback host), `http.followRedirects=false`, `transfer.fsckObjects=true`, `submodule.recurse=false`, `GIT_NO_LAZY_FETCH=1` | `gitx.newIsolated` | `TestTargetIsolation`, `TestFetchRefusesMalformedObjects`, `TestReadBlobNeverFetches`, `TestInitTargetRemotes`, `TestCheckRemoteScope` | `core.hooksPath` points at an empty directory of the isolation home, not at `/dev/null`: the same effect on every OS. | diff --git a/docs/reference/action.md b/docs/reference/action.md index 10086a7..b5db7a4 100644 --- a/docs/reference/action.md +++ b/docs/reference/action.md @@ -31,7 +31,7 @@ Each input is one flag; there is no free-form argument. | `strict` | `false` | plan, distribute, doctor: exit 3 on a blocked or deferred target, an incomplete resolve or a check that warns (--strict). true or false. | | `all` | `false` | plan: in a hub pull request, plan every target, not only those of the packs it changes (--all). true or false. | | `comment` | `false` | plan: keep the report in one comment of the hub pull request, through GITHUB_TOKEN (--comment). true or false. | -| `assume-opt-in` | `false` | plan: report as if every target had opted in (--assume-opt-in). true or false. | +| `assume-opt-in` | `false` | plan: report as if every target without an opt-in file had opted in (--assume-opt-in); enabled: false still opts a target out. true or false. | | `dry-run` | `false` | distribute: check everything with the write account and write nothing (--dry-run). true or false. | | `only` | — | plan, distribute, doctor: only these targets, comma-separated [PROVIDER:]PATH (--only). | | `deadline` | — | distribute: start no target after this duration, such as 50m (--deadline). Default: 5h30m on GitHub Actions. | diff --git a/docs/reference/glossary.md b/docs/reference/glossary.md index c5bcf6a..1450c99 100644 --- a/docs/reference/glossary.md +++ b/docs/reference/glossary.md @@ -41,7 +41,11 @@ history. `touchmark manifest` prints it. fingerprint and what the pull request carries. **opt-in file** — `.engineering-assets.yml` at a target's root. Its presence is consent; -it can add packs and `ignore` paths. +it can add packs and `ignore` paths, and `enabled: false` in it opts the target out. + +**`opt_in: assumed`** — an entry of `targets.yml` that subscribes the repositories it +selects: they count as opted in without an opt-in file. See +[targets.yml](targets.md#opt-in). **`orphaned`** — the state of a file shipped only by a pack the target no longer gets, and still unchanged: left in place and listed. @@ -71,7 +75,8 @@ default branch. **sync pull request** — touchmark's pull request (merge request on GitLab) from the sync branch. -**target** — a repository `targets.yml` selects. +**target** — a repository `targets.yml` selects: by path, by web URL, or through an +organisation or group, narrowed by `topics` and `match` and minus `exclude`. **write isolation** — keeping the write key visible to the default branch only: `security.write_isolation` (`platform`, `external`, `none`). diff --git a/docs/reference/hub.md b/docs/reference/hub.md index dd4769e..aa07484 100644 --- a/docs/reference/hub.md +++ b/docs/reference/hub.md @@ -105,7 +105,7 @@ In a hub pull request, `providers` and `ca_file` are read from the default branc | `providers` | list of objects | Platforms this hub delivers to. Targets refer to them by id. | | `providers[].id` | string, required | Name targets.yml uses for this provider, and the <ID> of its TOUCHMARK_<ID>_\* variables. | | `providers[].type` | one of `github`, `gitlab`, `gitea`, `forgejo`, required | — | -| `providers[].url` | string | Web URL of the instance. Default for github: https://github.com; for gitlab: https://gitlab.com. Required for gitea and forgejo. | +| `providers[].url` | string | Web URL of the instance. Default for github: https://github.com; for gitlab: https://gitlab.com. Required for gitea and forgejo. A target targets.yml names by web URL belongs to the provider whose url it lies under. | | `providers[].api_url` | string | API URL, when it cannot be derived from url. | | `providers[].ca_file` | string | Hub file with extra CA certificates for this instance. | | `providers[].writer` | string, up to 255 characters | The account that writes to targets, as the platform names it: on GitHub the App's bot, <slug>[bot]; on GitLab, Gitea and Forgejo the username of the service account or bot user. | diff --git a/docs/reference/opt-in.md b/docs/reference/opt-in.md index 2fec703..5cd21bc 100644 --- a/docs/reference/opt-in.md +++ b/docs/reference/opt-in.md @@ -2,7 +2,9 @@ `.engineering-assets.yml` at the root of a target repository (`opt_in_file` in `hub.yml` renames it). Its presence is the repository's consent: nothing is delivered without it, -and an empty file is enough. `touchmark schema opt-in` prints the JSON Schema. +and an empty file is enough, unless the hub subscribes the repository with `opt_in: +assumed` in [`targets.yml`](targets.md#opt-in). `touchmark schema opt-in` prints the +JSON Schema. ```yaml version: 1 @@ -12,6 +14,12 @@ ignore: - docs/guidelines/** ``` +- `enabled: false` opts the repository out, whatever `targets.yml` says: it is skipped + (`opted-out`), and its open sync pull request is closed as `opted-out`. Files already + merged stay. The default is `true`. +- **Deleting the file** opts a repository out too, unless the hub subscribes it + (`opt_in: assumed`): such a repository returns to what the hub assigns, so only + `enabled: false` opts it out. - `packs` adds packs; it cannot remove one the hub assigns. A pack the hub does not have blocks the target (`opt-in-invalid`). - `ignore` patterns: a path without glob characters covers everything beneath it; `*` @@ -32,6 +40,7 @@ ignore: | Key | Type | Description | |---|---|---| | `version` | `1` | Format version. Only 1 exists. | +| `enabled` | boolean | false opts the repository out, even when the hub subscribes it (opt_in: assumed in targets.yml): it gets nothing, and the hub's open pull requests here are closed. Default: true. | | `packs` | list of strings, each up to 64 characters | Packs to add to those the hub assigns. Packs the hub assigns cannot be removed here. | | `ignore` | list of strings, at most 1000 | Paths the hub must never create, update or delete here. A path without glob characters covers everything beneath it; \*\* matches any number of directories. At most 1000 patterns. | diff --git a/docs/reference/output.md b/docs/reference/output.md index 9c95e5a..d0494c8 100644 --- a/docs/reference/output.md +++ b/docs/reference/output.md @@ -25,6 +25,7 @@ touchmark plan · hub acme-eng (github.com/712345678) @ 3f2a1c9 (PR #41) · touc gh github.com read acme-read[bot] write acme-write[bot]: not checked resolve complete corp gitlab.example.com read tm-reader write tm-writer: not checked resolve complete scope: packs base, python changed → 11 of 11 targets (--all for every target) +opt_in: assumed: 2 targets without an opt-in file are opted in by targets.yml; an opt-in file with enabled: false opts one out open 4 corp:platform/api, gh:acme/billing, gh:acme/docs and 1 more update 1 gh:acme/sdk #1 (content) @@ -48,7 +49,7 @@ text says *would*. Every target has one outcome: | `unchanged` | — | | `closed` | `no-diff`, `opted-out`, `target-dropped`, `duplicate` | | `declined` | — (the declined pull request is in `pr`) | -| `skipped` | `not-opted-in`, `archived`, `disabled`, `empty`, `mirror`, `pending-deletion`, `prs-disabled`, `sha256`, `unsafe-opt-in`, `private-in-public-hub`, `superseded` | +| `skipped` | `not-opted-in`, `opted-out`, `archived`, `disabled`, `empty`, `mirror`, `pending-deletion`, `prs-disabled`, `sha256`, `unsafe-opt-in`, `private-in-public-hub`, `superseded` | | `blocked` | `edited`, `branch-taken`, `branch-in-use`, `opt-in-invalid`, `marker-invalid`, `rules:`, `permission:`, `cannot-sign`, `archived`, `mass-close` | | `deferred` | `rate-limit`, `deadline`, `rollout-limit`, `provider-down`, `interrupted`, `cooldown` | | `failed` | `transient`, `auth`, `access`, `git`, `integrity`, `race`, `secret-exposure`, `internal` | @@ -79,7 +80,7 @@ and the branch head before and after. | `scope.base` | string | Full commit id: 40 or 64 lowercase hex digits. | | `scope.processed` | integer ≥ 0, required | The targets the plan processed in full. | | `scope.total` | integer ≥ 0, required | The targets the resolve found. | -| `assume_opt_in` | boolean | plan --assume-opt-in: every target counts as opted in, for this report only. | +| `assume_opt_in` | boolean | plan --assume-opt-in: for this report only, every target without an opt-in file, or whose opt-in file is not a regular file or is too large, counts as opted in; one whose opt-in file says enabled: false stays opted out. | | `providers` | list of objects, required | The hub's providers, in hub.yml order. | | `providers[].id` | string, required | — | | `providers[].type` | one of `github`, `gitlab`, `gitea`, `forgejo`, required | — | @@ -103,7 +104,8 @@ and the branch head before and after. | `targets[].pr.state` | one of `open`, `closed`, `merged` | — | | `targets[].key` | string | Content key of the changes; absent when there are none. | | `targets[].packs` | list of strings | The packs the target gets, in layering order. | -| `targets[].opt_in_assumed` | boolean | plan --assume-opt-in planned the target as if it had an empty opt-in file: it has none, or one that is not a regular file. | +| `targets[].opt_in_assumed` | boolean | The target was processed as if it had an empty opt-in file: it has none, or, with plan --assume-opt-in, one that is not a regular file or is too large. opt_in_assumed_by says why. | +| `targets[].opt_in_assumed_by` | one of `targets.yml`, `--assume-opt-in` | Why opt_in_assumed is set: targets.yml (an entry with opt_in: assumed selects the target, for real) or --assume-opt-in (plan's flag, for the report only). | | `targets[].changes` | object, required | The changes D holds, by action. | | `targets[].changes.create` | integer ≥ 0, required | — | | `targets[].changes.update` | integer ≥ 0, required | — | @@ -209,7 +211,7 @@ hub, for each provider's write identity, and for each target. See | `targets[].host` | string, not empty, required | — | | `targets[].repo_id` | string, required | The repository's immutable id on the platform; empty for a target the report does not name. | | `targets[].path` | string, required | The repository's canonical path; empty for a target the report does not name. | -| `targets[].skipped` | string, not empty | Why the target was not checked: archived, disabled, empty, mirror, pending-deletion, prs-disabled, sha256, not-opted-in, unsafe-opt-in, or deferred:<reason> (the run could not check the target; an unknown check says why). | +| `targets[].skipped` | string, not empty | Why the target was not checked: archived, disabled, empty, mirror, pending-deletion, prs-disabled, sha256, not-opted-in, opted-out, unsafe-opt-in, or deferred:<reason> (the run could not check the target; an unknown check says why). | | `targets[].checks` | list of objects, required | — | | `targets[].checks[].check` | string, required | What was checked, e.g. access, token-expiry, rules, markers, hub-hidden, write-isolation, key-location. | | `targets[].checks[].status` | one of `ok`, `warn`, `fail`, `unknown`, required | — | @@ -267,7 +269,10 @@ appears in it. See [Set up a hub's platform](../guide/setup.md). ## status and apply `--format json` prints the hub (its directory, `id` and commit), the target (its -directory, its reference in `targets.yml`, its opt-in file and whether it opted in), the +directory, its reference in `targets.yml`, its opt-in file, whether it opted in, and +`opt_in`, which says why: `file`, the opt-in file is there; `assumed`, there is none and +a `repo:` entry with `opt_in: assumed` subscribes it; `opted-out`, the file says +`enabled: false`; `none`, neither), the pack selection and where each pack came from, one entry per managed path with its `state`, `action`, `pack` and blob ids, a `summary` counting each state, and `warnings`: @@ -276,7 +281,8 @@ pack selection and where each pack came from, one entry per managed path with it "command": "status", "dry_run": false, "hub": { "dir": "/src/engineering-assets", "id": "acme-eng", "commit": "ffae0953c140298cf045e600f15710104e9d5d0d" }, - "target": { "root": "/src/billing", "ref": "acme/billing", "opt_in_file": ".engineering-assets.yml", "opted_in": true }, + "target": { "root": "/src/billing", "ref": "acme/billing", "opt_in_file": ".engineering-assets.yml", "opted_in": true, + "opt_in": "file" }, "selection": { "packs": ["agents"], "complete": true, "unresolved": [], "sources": { "agents": ["defaults"] } }, "entries": [ { "path": "AGENTS.md", "state": "local", "action": "keep", "pack": "agents", diff --git a/docs/reference/targets.md b/docs/reference/targets.md index 1611731..4d780cf 100644 --- a/docs/reference/targets.md +++ b/docs/reference/targets.md @@ -8,10 +8,12 @@ version: 1 defaults: provider: gh # needed only when hub.yml lists several providers packs: [agents] # every target gets these + opt_in: required # the default: a target waits for its opt-in file targets: - repo: acme/billing # one repository, on the default provider packs: [python-service] - repo: corp:platform/api # : + - repo: https://gitlab.example.com/platform/web # a web URL: the provider at that url - provider: corp group: platform # GitLab: with subgroups unless subgroups: false topics: [python] # all of them must match @@ -20,9 +22,14 @@ targets: topics: [library] forks: false # the default; an explicit repo: entry is never skipped as a fork packs: [python-library] + - org: acme + match: [acme/svc-*] # only the repositories whose path matches + opt_in: assumed # opted in by the hub, without an opt-in file exclude: - acme/legacy-monolith - corp:platform/sandbox + - corp:platform/legacy/** # a subgroup, with every project beneath it + - acme/*-archive ``` - **A target's packs**, in order: `defaults.packs`, the `packs` of every matching entry @@ -30,10 +37,10 @@ exclude: `requires` before it. - **A target's provider**, in order: the entry's `provider`, the `:` prefix, `defaults.provider`, the only provider. `check` fails when that leaves a choice. -- **Identity.** Output, `--only` and `operations.yml` name a target `:`. - Inside a run a target is its host and immutable repository id, so a repository two - entries select is delivered to once, with the packs of both. A repository whose path - changed gets a `renamed` warning. +- **Identity.** Output, `--only` and `operations.yml` name a target `:`, + also when `targets.yml` names it by its URL. Inside a run a target is its host and + immutable repository id, so a repository two entries select is delivered to once, with + the packs of both. A repository whose path changed gets a `renamed` warning. - **Always skipped**, with the reason in the report: archived, disabled, empty, mirror, pending deletion, pull requests turned off, SHA-256 repositories, and, in a public hub, non-public ones. @@ -46,6 +53,127 @@ or `apply` cannot resolve them. When an `org` or `group` entry with `packs` migh the target, `status` warns, and `apply` wants `--packs`; entries without `packs` add nothing a local run could miss. +## Web URLs + +`repo`, `org`, `group` and `exclude` also take the web URL of a repository or a +namespace, as a browser shows it: `https://github.com/acme/billing`, +`https://gitlab.example.com/platform/api`, with or without a trailing `/`, and for a +repository with or without `.git`. + +- **The provider** is the one in `hub.yml` whose `url` the URL lies under: the same + scheme, host and port, and the provider's path as a prefix when the instance lives + under one (`https://example.com/gitlab`). A `github` provider without `url` is at + `https://github.com`, a `gitlab` one at `https://gitlab.com`. A hub with `writer` at the + top level of `hub.yml` instead of `providers` has the url of the CI's own platform + (`GITHUB_SERVER_URL`, `CI_SERVER_URL`). Outside CI, the hub's `origin` remote tells it + only on `github.com`, a `*.ghe.com` host or `gitlab.com`: a hub on any other instance + that names targets by URL lists the instance under `providers`, or its local `check`, + `status --hub` and `apply --hub` fail, and so does a run in another platform's CI (a + target's own GitHub Actions job reading a hub on GitLab). `check` in CI warns about it. +- **Exactly one provider** must match. A URL under no provider's `url` fails `check`. + When two providers share an instance, the entry's `provider` picks one, else + `defaults.provider` when it is one of them; an `exclude` URL takes `defaults.provider` + only, or is written as `:`. +- **The rest of the path** is the target's path: with a provider `corp` at + `https://gitlab.example.com`, `https://gitlab.example.com/platform/api` is + `corp:platform/api`. On GitHub, Gitea and Forgejo a repository URL names `owner/name`, + and so does an `exclude` URL without `**` (a page inside a repository, such as + `/tree/main`, would exclude nothing); an organisation URL names one owner. A GitLab URL + with a `/-/` segment (a file, a merge request) points inside a project and is refused. + `.git` is dropped from a repository URL, but a pattern ending in `.git` is refused: it + would widen (`acme/*.git` to `acme/*`). +- **A namespace URL in `exclude`** names one repository at that path, not what lies + beneath it: browsers show groups and projects alike, so to leave out a group write its + URL with `/**` (`https://gitlab.example.com/platform/legacy/**`). `plan` and + `distribute` warn about an `exclude` entry that excludes nothing while targets lie + beneath it. +- **Refused:** credentials (`user:password@`), a query (`?tab=readme`), a fragment + (`#readme`), and any scheme but `https`; `http` is accepted for `localhost`, + `127.0.0.1` and `[::1]` only, as in `hub.yml`. + +touchmark rewrites every URL as `:` as soon as it reads `targets.yml`: +reports, `--only` and `operations.yml` keep that form. + +## Patterns + +`exclude` entries and the `match` patterns of `org` and `group` entries are glob +patterns over a repository's full path: + +| Pattern | Matches | +|---|---| +| `*` | any characters within one path segment: `acme/legacy-*` | +| `**` | as a whole segment, any number of segments, none included: `corp:platform/legacy/**` is every project of the subgroup and of the groups beneath it | +| `?` | one character: `acme/svc-?` | + +Paths compare without regard to case. A pattern covers a whole repository path, so it +has at least two segments; `**` inside a longer segment (`svc-**`) is `*`. An entry +without glob characters names one repository, as before: `acme/legacy` does not exclude +`acme/legacy-api`, nor anything beneath `acme/legacy`. `[` is not a glob character and +is refused; in a URL `?` would start a query, so a URL pattern takes `*` and `**` only. + +In YAML a value that starts with `*` reads as an alias, so quote such a pattern: +`- "**/legacy"`, `match: ["*/svc-*"]`. + +- **`exclude`**: `[:]`, or a URL. The provider is found as for an + entry: the prefix, else `defaults.provider`, else the only provider. A pattern that + covers no `repo:` entry is fine: it is there for the repositories of `org` and `group` + entries. One that covers a `repo:` entry is a warning: the exclude wins. So is one of + more than `owner/name` without `**` on GitHub, Gitea or Forgejo, which have no nested + namespaces: it can cover no repository. +- **`match`**, on `org` and `group` entries only: patterns over the full path, without + a provider (`acme/svc-*`, `platform/**/api`). A repository of the namespace is selected + when its path matches at least one of them. `check` warns about a pattern that no + repository of the namespace can match: outside it, or deeper than `subgroups: false` + allows, or than a GitHub, Gitea or Forgejo organisation holds (`owner/name`). An empty + list, a URL, a provider prefix, and `match` on a `repo:` entry are errors. + +## Opt-in + +A repository receives nothing until it has [the opt-in file](opt-in.md), unless the hub +subscribes it: `opt_in` on an entry, or in `defaults` for every entry that sets none. + +- **`required`** (the default): a repository the entry selects is skipped + (`not-opted-in`) until it adds the opt-in file. +- **`assumed`**: a repository the entry selects counts as opted in without the file, as + if it had an empty one. It gets the packs of `defaults` and of its entries, and ignores + nothing. Its sync pull request says that the hub subscribed it, how to choose packs or + ignore files (add the opt-in file), and how to opt out (`enabled: false` in it). + +**One entry is enough.** A repository is subscribed when at least one entry that selects +it has `assumed`, its own or through `defaults.opt_in`, whatever the other entries say. +Entries add up, as their packs do: a `repo:` entry that gives one repository of a +subscribed organisation more packs must not unsubscribe it in passing. To let a +repository of that organisation opt in by itself, keep it out of the `assumed` entry +with `match`. `check` warns about a `repo:` entry that says `opt_in: required` while an +`assumed` entry may select the same repository. + +**The repository decides last.** Its opt-in file always wins: with one, its `packs` and +`ignore` apply, as for any target; with `enabled: false` in it, the repository is opted +out whatever `targets.yml` says (`skipped:opted-out`), and its open sync pull request is +closed as `opted-out`. Deleting the opt-in file of a subscribed repository returns it to +the hub's subscription; it does not opt it out. + +**Withdrawing a subscription** (dropping `assumed` from an entry or from `defaults`) +makes the repositories it subscribed without an opt-in file `not-opted-in` again: their +open sync pull requests are closed as `opted-out`, with a comment that says the hub no +longer subscribes the repository and that the opt-in file brings the changes back. + +**Declines work as usual.** The first sync pull request of a subscribed repository is a +proposal: closing it without merging is a decline, and the same content is not proposed +again. An opt-in file that changes `packs` or `ignore` lifts the decline, like any edit +of those keys; an empty one does not. + +In reports, a subscribed target without the opt-in file has `opt_in_assumed: true` and +`opt_in_assumed_by: targets.yml`, and the text and Markdown outputs count such targets. +`plan --assume-opt-in` is another thing: it takes an empty opt-in file for every target +that has none, for that report only (`opt_in_assumed_by: --assume-opt-in`). + +A local `status` or `apply` knows the target by its path only. A `repo:` entry with +`assumed` that names it opts it in (`opted in by targets.yml`). An `org` or `group` entry +with `assumed` cannot be resolved locally: the target counts as not opted in, and +`status` warns that `plan` has the answer. An opt-in file in the checkout settles it +either way. + ## Keys @@ -56,16 +184,19 @@ nothing a local run could miss. | `defaults` | object | Applies to every target. | | `defaults.provider` | string | Provider of entries that name none. Needed when hub.yml has several providers. | | `defaults.packs` | list of strings, each up to 64 characters | Packs every target gets, before the packs of its entries and its opt-in file. | +| `defaults.opt_in` | one of `required`, `assumed` | The opt_in of every entry that sets none: required (the default) or assumed. | | `targets` | list of objects | Repositories, organisations and groups to deliver to, in delivery order. | -| `targets[].repo` | string, up to 512 characters | One repository: owner/name or group/sub/project, optionally prefixed with a provider id and a colon. An entry has exactly one of `repo`, `org` and `group`. | +| `targets[].repo` | string, up to 512 characters | One repository: owner/name or group/sub/project, optionally prefixed with a provider id and a colon, or its web URL, such as https://gitlab.example.com/platform/api, whose provider is the one hub.yml lists at that url. An entry has exactly one of `repo`, `org` and `group`. | | `targets[].provider` | string | Id of a provider in hub.yml. | | `targets[].packs` | list of strings, each up to 64 characters | — | -| `targets[].org` | string, up to 512 characters | Every repository in a GitHub organisation or a GitLab, Gitea or Forgejo namespace (a synonym of group). An entry has exactly one of `repo`, `org` and `group`. | +| `targets[].opt_in` | one of `required`, `assumed` | required (the default): a repository the entry selects gets nothing until it adds the opt-in file. assumed: it counts as opted in without one, with the packs targets.yml gives it, until its opt-in file says enabled: false. One entry with assumed is enough to subscribe a repository. | +| `targets[].org` | string, up to 512 characters | Every repository in a GitHub organisation or a GitLab, Gitea or Forgejo namespace (a synonym of group), named by its path or its web URL. An entry has exactly one of `repo`, `org` and `group`. | | `targets[].topics` | list of strings | Only repositories with all of these topics. Only with `org` or `group`. | | `targets[].subgroups` | boolean | Include repositories in nested groups. Default: true. Only with `org` or `group`. | | `targets[].forks` | boolean | Include forks. Default: false. Only with `org` or `group`. | -| `targets[].group` | string, up to 512 characters | Every repository in a GitLab group, with its subgroups unless subgroups is false (a synonym of org). An entry has exactly one of `repo`, `org` and `group`. | -| `exclude` | list of strings, each up to 512 characters | Repositories never delivered to, even when an entry selects them. | +| `targets[].match` | list of strings, each up to 512 characters | Only repositories whose full path, without the provider, matches at least one of these patterns: \* within one segment, \*\* any number of segments, ? one character, compared ignoring case, such as acme/svc-\* or platform/\*\*/api. Quote a pattern that starts with \*, which YAML reads as an alias. Only with `org` or `group`. | +| `targets[].group` | string, up to 512 characters | Every repository in a GitLab group, with its subgroups unless subgroups is false (a synonym of org), named by its path or its web URL. An entry has exactly one of `repo`, `org` and `group`. | +| `exclude` | list of strings, each up to 512 characters | Repositories never delivered to, even when an entry selects them: a repository as repo names one, or a pattern of repository paths in which \* matches within one segment, \*\* any number of segments and ? one character (corp:platform/legacy/\*\* covers a subgroup at any depth), compared ignoring case. A web URL works too, with \* and \*\* but not ?. Quote a pattern that starts with \*, which YAML reads as an alias. | | `repos` | list of strings, each up to 512 characters | Deprecated. The pre-v1 format: a bare list of repository paths, read as repo: entries. | diff --git a/internal/cli/check.go b/internal/cli/check.go index 1a5e0ec..112d44d 100644 --- a/internal/cli/check.go +++ b/internal/cli/check.go @@ -34,13 +34,19 @@ func runCheck(ctx context.Context, e *env, o *options) error { Errors: []string{}, Warnings: []string{}, }} - for _, err := range h.readConfigs(ctx) { + for _, err := range h.readConfigs(ctx, e) { c.addErrors(flatten(err)...) } if err := h.readOperations(ctx); err != nil { c.addErrors(flatten(err)...) } c.addWarnings(h.warnings...) + if h.ciOnlyURLs { + c.addWarnings(config.TargetsFile + ": its web URLs resolve against the implicit provider this CI names, as " + config.HubFile + + " lists no providers; outside CI that provider comes from the hub's origin remote only on github.com, a *.ghe.com host or gitlab.com, " + + "so a local check, status --hub or apply --hub of this hub fails, and so does a run in another platform's CI; list the instance under providers in " + + config.HubFile) + } c.rep.Hub = h.report() if err := h.readPacks(ctx); err != nil { return err diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index c377366..de3e1a8 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -201,7 +201,7 @@ func loadDoctorHub(ctx context.Context, e *env, o *options) (*hub, error) { if err != nil { return nil, err } - if errs := h.readConfigs(ctx); len(errs) > 0 { + if errs := h.readConfigs(ctx, e); len(errs) > 0 { return nil, configErrorf("%w\nrun touchmark check for details", joinedError(errors.Join(errs...))) } if h.cfg.Legacy || h.cfg.ID == "" { diff --git a/internal/cli/hub.go b/internal/cli/hub.go index 188ef2a..6320b3d 100644 --- a/internal/cli/hub.go +++ b/internal/cli/hub.go @@ -19,6 +19,7 @@ import ( "github.com/bedrock-python/touchmark/internal/apply" "github.com/bedrock-python/touchmark/internal/config" "github.com/bedrock-python/touchmark/internal/gitx" + "github.com/bedrock-python/touchmark/internal/hubch" "github.com/bedrock-python/touchmark/internal/provenance" "github.com/bedrock-python/touchmark/internal/report" ) @@ -45,6 +46,11 @@ type hub struct { // only check reads it. ops *config.Operations warnings []string + // ciOnlyURLs is set when the web URLs of targets.yml resolved against + // the implicit provider the CI's environment names, on an instance an + // origin remote cannot name outside CI (config.OriginTellsProvider): + // local runs cannot resolve them. + ciOnlyURLs bool manifest *provenance.Manifest skipped []string @@ -124,9 +130,11 @@ func existingDir(dir string) (string, error) { return abs, nil } -// readConfigs reads and parses hub.yml and targets.yml. It returns every -// problem found; cfg and targets stay nil for a file that failed. -func (h *hub) readConfigs(ctx context.Context) []error { +// readConfigs reads and parses hub.yml and targets.yml, and rewrites the +// web URLs of targets.yml as : (resolveURLs). It returns +// every problem found; cfg and targets stay nil for a file that failed, +// targets too when a URL did not resolve. +func (h *hub) readConfigs(ctx context.Context, e *env) []error { var errs []error data, err := h.readConfig(ctx, config.HubFile) if err == nil { @@ -146,9 +154,34 @@ func (h *hub) readConfigs(ctx context.Context) []error { if err != nil { errs = append(errs, err) } + if h.cfg != nil && h.targets.HasURLs() { + if err := h.resolveURLs(ctx, e.getenv); err != nil { + errs = append(errs, err) + h.targets = nil + } + } return errs } +// resolveURLs rewrites the web URLs of targets.yml as : +// (config.ResolveURLs) against the providers plan and distribute resolve: +// those of hub.yml, or the implicit one, which the CI's environment or, +// outside CI, the hub's origin remote names (on github.com, a *.ghe.com +// host or gitlab.com only). +func (h *hub) resolveURLs(ctx context.Context, getenv func(string) string) error { + rps, err := h.resolveProviders(ctx, hubch.Detect(getenv, os.ReadFile), getenv) + if err != nil { + return fmt.Errorf("%s: its web URLs are matched with the providers' urls, which are unknown: %w", config.TargetsFile, err) + } + targets, err := config.ResolveURLs(h.targets, rps) + if err != nil { + return err + } + h.targets = targets + h.ciOnlyURLs = len(rps) == 1 && rps[0].Implicit && !config.OriginTellsProvider(rps[0].Host) + return nil +} + // readOperations reads and parses .touchmark/operations.yml like // readConfigs reads hub.yml: from HEAD or the work tree, absent meaning no // operations. ops stays nil when the file failed. diff --git a/internal/cli/migrate.go b/internal/cli/migrate.go index 5085e92..fef4a4e 100644 --- a/internal/cli/migrate.go +++ b/internal/cli/migrate.go @@ -1121,6 +1121,7 @@ func (p *migration) targetsYML() string { w.line(1, "- repo: "+yamlString(r)) } w.comment(0, "TODO: packs: which packs every target gets (defaults.packs, packs per entry); the opt-in files add theirs.") + w.comment(0, "Targets need their own opt-in file, as they did with multi-gitter. To subscribe repositories without one, add opt_in: assumed under defaults or to an entry; a repository opts out with enabled: false in its opt-in file.") if len(p.exclude) > 0 { w.line(0, "exclude:") for _, r := range p.exclude { diff --git a/internal/cli/plan.go b/internal/cli/plan.go index e0e8393..bc6d8b4 100644 --- a/internal/cli/plan.go +++ b/internal/cli/plan.go @@ -106,11 +106,12 @@ func planFlags() (*flagSet, *options) { // In a hub pull request the plan processes only the targets of the packs // the pull request changes (planScope; every target with --all), and with // --comment keeps its report in one comment of the pull request -// (postPlanComment). --assume-opt-in plans every target as opted in. In CI -// it leaves the outputs distribute does (publish): the -// step summary on GitHub Actions and Gitea, annotations of the first failed -// and blocked targets on GitHub Actions, and touchmark-report.{json,md} -// in the working directory for the job's artifact. +// (postPlanComment). --assume-opt-in plans every target without an opt-in +// file as opted in. In CI it leaves the outputs distribute does (publish): +// the step summary on GitHub Actions and Gitea, annotations of the first +// failed and blocked targets on GitHub Actions, and +// touchmark-report.{json,md} in the working directory for the job's +// artifact. func runPlan(ctx context.Context, e *env, o *options, p *planOptions) (err error) { h, err := loadPlanHub(ctx, e, o) if err != nil { @@ -261,7 +262,7 @@ func loadPlanHub(ctx context.Context, e *env, o *options) (*hub, error) { if err != nil { return nil, err } - if errs := h.readConfigs(ctx); len(errs) > 0 { + if errs := h.readConfigs(ctx, e); len(errs) > 0 { return nil, configErrorf("%w\nrun touchmark check for details", joinedError(errors.Join(errs...))) } if h.cfg.Legacy || h.cfg.ID == "" { diff --git a/internal/cli/plan_comment_test.go b/internal/cli/plan_comment_test.go index 0b66282..723ca5a 100644 --- a/internal/cli/plan_comment_test.go +++ b/internal/cli/plan_comment_test.go @@ -187,8 +187,8 @@ func TestPlanCommentGitLab(t *testing.T) { } } -// plan --assume-opt-in plans every target as opted in; the output says so -// in every format. +// plan --assume-opt-in plans every target without an opt-in file as opted +// in; the output says so in every format. func TestPlanAssumeOptInOutput(t *testing.T) { h := planHub(t) w := newPlanWorld(t) diff --git a/internal/cli/plan_scope.go b/internal/cli/plan_scope.go index cab91c5..e3b06ef 100644 --- a/internal/cli/plan_scope.go +++ b/internal/cli/plan_scope.go @@ -26,7 +26,8 @@ type planOptions struct { all bool // comment keeps plan's comment in the hub pull request (--comment). comment bool - // assumeOptIn plans every target as opted in (--assume-opt-in). + // assumeOptIn plans every target without an opt-in file as opted in + // (--assume-opt-in). assumeOptIn bool } diff --git a/internal/cli/sync.go b/internal/cli/sync.go index 47f07dc..ae0bdfa 100644 --- a/internal/cli/sync.go +++ b/internal/cli/sync.go @@ -98,7 +98,7 @@ func (s *syncRun) load(ctx context.Context) error { if err != nil { return err } - if errs := h.readConfigs(ctx); len(errs) > 0 { + if errs := h.readConfigs(ctx, s.e); len(errs) > 0 { return configErrorf("%w\nrun touchmark check for details", joinedError(errors.Join(errs...))) } t, err := openTarget(ctx, s.e, s.o.dir) @@ -111,6 +111,9 @@ func (s *syncRun) load(ctx context.Context) error { if err := t.resolveRef(ctx, s.o.repo, h); err != nil { return err } + if err := t.assume(h); err != nil { + return err + } s.hub, s.tgt = h, t return nil } diff --git a/internal/cli/target.go b/internal/cli/target.go index d97966d..40ca3b0 100644 --- a/internal/cli/target.go +++ b/internal/cli/target.go @@ -9,6 +9,7 @@ import ( "net/url" "os" "path/filepath" + "slices" "strings" "github.com/bedrock-python/touchmark/internal/config" @@ -30,10 +31,14 @@ type target struct { // --repo nor the origin remote gave one. ref config.Ref refKnown bool - // optInName is the opt-in file; optIn is nil when it is absent. - optInName string - optIn *config.OptIn - warnings []string + // optInName is the opt-in file; optIn is nil when the target is not + // opted in: the file is absent and targets.yml does not subscribe the + // target, or it says enabled: false. optInState says which + // (report.OptIn*). + optInName string + optIn *config.OptIn + optInState string + warnings []string } // openTarget finds the target root: the top of the git work tree containing @@ -75,11 +80,12 @@ func openTarget(ctx context.Context, e *env, dir string) (*target, error) { return t, nil } -// readOptIn reads and parses the opt-in file called name at the target root. An -// absent file leaves optIn nil. The file comes from the target, so it is +// readOptIn reads and parses the opt-in file called name at the target +// root. An absent file, or one that says enabled: false, leaves optIn nil +// (optInState none or opted-out). The file comes from the target, so it is // read without following symlinks and with a size limit. func (t *target) readOptIn(name string) error { - t.optInName = name + t.optInName, t.optInState = name, report.OptInNone data, found, err := readRegular(t.root, name, maxOptInRead) if err != nil { return configErrorf("opt-in file %s: %w", name, err) @@ -94,7 +100,44 @@ func (t *target) readOptIn(name string) error { for _, w := range warns { t.warnings = append(t.warnings, name+": "+w.Message) } - t.optIn = o + if o.Disabled() { + t.optInState = report.OptInDisabled + return nil + } + t.optIn, t.optInState = o, report.OptInFile + return nil +} + +// assume opts in a target without an opt-in file that targets.yml +// subscribes (config.Assume): a repo: entry with opt_in: assumed names it. +// It then gets the packs of targets.yml, as if it had an empty opt-in +// file. An org or group entry with opt_in: assumed that may hold the +// target is a warning: only the platform tells whether it selects it, so +// the target stays not opted in here, and plan has the answer. +func (t *target) assume(h *hub) error { + if t.optInState != report.OptInNone { + return nil + } + if !t.refKnown { + if h.targets != nil && (h.targets.Defaults.OptIn == config.OptInAssumed || + slices.ContainsFunc(h.targets.Targets, func(e config.Entry) bool { return e.OptIn == config.OptInAssumed })) { + t.warnings = append(t.warnings, "targets.yml subscribes some repositories without an opt-in file (opt_in: assumed), "+ + "and this one's name is unknown (no --repo and no origin remote): pass --repo to tell whether it is one of them") + } + return nil + } + a, err := config.Assume(h.cfg, h.targets, t.ref) + if err != nil { + return configError(err) + } + if a.Assumed { + t.optIn, t.optInState = &config.OptIn{Version: 1}, report.OptInAssumed + return nil + } + if len(a.Unresolved) > 0 { + t.warnings = append(t.warnings, fmt.Sprintf("targets.yml may subscribe this repository without an opt-in file: %s with opt_in: assumed "+ + "cannot be resolved here, so it counts as not opted in; plan tells, and an opt-in file decides it here", strings.Join(a.Unresolved, ", "))) + } return nil } @@ -252,7 +295,7 @@ func remoteProvider(host string, cfg *config.Hub, targets *config.Targets) strin // report returns the target part of a report. func (t *target) report() report.Target { - r := report.Target{Root: t.root, OptInFile: t.optInName, OptedIn: t.optIn != nil} + r := report.Target{Root: t.root, OptInFile: t.optInName, OptedIn: t.optIn != nil, OptIn: t.optInState} if t.refKnown { r.Ref = t.ref.String() } diff --git a/internal/cli/targets_test.go b/internal/cli/targets_test.go new file mode 100644 index 0000000..d2fa0d0 --- /dev/null +++ b/internal/cli/targets_test.go @@ -0,0 +1,141 @@ +package cli + +import ( + "strings" + "testing" + + "github.com/bedrock-python/touchmark/internal/report" +) + +// assumedTargets subscribes acme/svc by name and the libraries of acme by a +// pattern, without an opt-in file. +const assumedTargets = `version: 1 +defaults: + packs: [base] +targets: + - repo: acme/svc + opt_in: assumed + - org: acme + match: [acme/lib-*] + opt_in: assumed +` + +// A target without the opt-in file that a repo: entry with opt_in: assumed +// names is opted in locally, with the packs of targets.yml; one only an org +// entry may subscribe stays not opted in, with a warning, since a local run +// cannot resolve the org. +func TestAssumedOptIn(t *testing.T) { + t.Parallel() + h := baseHub(t) + h.write("targets.yml", assumedTargets) + h.commit("subscribe acme/svc") + tg := newRepo(t, false) + s := newScenario(t, "assumed", h, tg, "--repo", "acme/svc") + s.golden("status.txt", s.text("status")) + s.golden("apply.json", s.json("apply")) + s.golden("tree.txt", tg.tree()) + + lib := newScenario(t, "assumed", h, newRepo(t, false), "--repo", "acme/lib-x") + rep := lib.report("status") + if rep.Target.OptedIn || rep.Target.OptIn != report.OptInNone || len(rep.Warnings) != 1 || + !strings.Contains(rep.Warnings[0], "org: acme with opt_in: assumed cannot be resolved here") { + t.Errorf("acme/lib-x: %+v, warnings %q", rep.Target, rep.Warnings) + } + web := newScenario(t, "assumed", h, newRepo(t, false), "--repo", "acme/web") + if rep := web.report("status"); rep.Target.OptedIn || len(rep.Warnings) != 0 { + t.Errorf("acme/web: %+v, warnings %q", rep.Target, rep.Warnings) + } + unknown := newScenario(t, "assumed", h, newPlainDir(t)) + if rep := unknown.report("status"); rep.Target.OptedIn || len(rep.Warnings) != 1 || !strings.Contains(rep.Warnings[0], "pass --repo") { + t.Errorf("a target without a name: %+v, warnings %q", rep.Target, rep.Warnings) + } +} + +// An opt-in file with enabled: false opts the target out, whatever +// targets.yml says: status says so, and apply writes nothing. +func TestOptedOut(t *testing.T) { + t.Parallel() + h := baseHub(t) + h.write("targets.yml", assumedTargets) + h.commit("subscribe acme/svc") + tg := optedIn(t, "version: 1\nenabled: false\npacks: [base]\n") + s := newScenario(t, "opted-out", h, tg, "--repo", "acme/svc") + s.golden("status.txt", s.text("status")) + s.golden("apply.json", s.json("apply")) + if tree := tg.tree(); strings.Contains(tree, "AGENTS.md") { + t.Errorf("apply wrote into a target that opted out:\n%s", tree) + } +} + +// Web URLs in targets.yml resolve against the providers of hub.yml, so the +// local commands see :; a URL no provider serves fails +// check, and so does a hub whose only provider is implicit when nothing +// tells its url. +func TestTargetURLs(t *testing.T) { + t.Parallel() + h := baseHub(t) + h.write("hub.yml", `version: 1 +id: acme-eng +providers: + - id: gh + type: github + - id: corp + type: gitlab + url: https://gitlab.example.com +`) + h.write("targets.yml", `version: 1 +defaults: + packs: [base] +targets: + - repo: https://github.com/acme/svc + opt_in: assumed + - group: https://gitlab.example.com/platform/ + match: [platform/**/api] +exclude: + - https://gitlab.example.com/platform/legacy/** +`) + h.commit("targets by URL") + if res := runWith(t, nil, "check", "--hub", h.dir); res.code != 0 || !strings.Contains(res.stdout, "ok") { + t.Fatalf("check: exit %d\n%s%s", res.code, res.stdout, res.stderr) + } + tg := newRepo(t, false) + res := runWith(t, nil, "status", "--hub", h.dir, "--dir", tg.dir, "--repo", "gh:acme/svc", "--format", "json") + if rep := decodeSync(t, res.stdout); res.code != 0 || rep.Target.OptIn != report.OptInAssumed || !rep.Target.OptedIn { + t.Errorf("status of gh:acme/svc: exit %d, %+v\n%s", res.code, rep.Target, res.stderr) + } + + h.write("targets.yml", "version: 1\ntargets:\n - repo: https://bitbucket.org/acme/svc\n") + h.commit("a URL of no provider") + res = runWith(t, nil, "check", "--hub", h.dir) + if res.code != exitUsage || !strings.Contains(res.stdout, "targets.yml: targets[0].repo: https://bitbucket.org/acme/svc is not under the url of any provider") { + t.Errorf("check of a URL of no provider: exit %d\n%s", res.code, res.stdout) + } + res = runWith(t, nil, "status", "--hub", h.dir, "--dir", tg.dir, "--repo", "gh:acme/svc") + if res.code != exitUsage || !strings.Contains(res.stderr, "is not under the url of any provider") { + t.Errorf("status with a URL of no provider: exit %d\n%s", res.code, res.stderr) + } + + // The implicit provider: the CI names its url; nothing does locally + // without an origin remote. + h.write("hub.yml", hubYML) + h.write("targets.yml", "version: 1\ntargets:\n - repo: https://gitlab.example.com/platform/api\n") + h.commit("the implicit provider") + res = runWith(t, nil, "check", "--hub", h.dir) + if res.code != exitUsage || !strings.Contains(res.stdout, "its web URLs are matched with the providers' urls, which are unknown") { + t.Errorf("check without a provider url: exit %d\n%s", res.code, res.stdout) + } + // In CI they resolve, with a warning that a local run cannot: an origin + // remote names a provider on public instances only. + ci := map[string]string{"GITLAB_CI": "true", "CI_SERVER_URL": "https://gitlab.example.com"} + res = runWith(t, ci, "check", "--hub", h.dir) + if res.code != 0 || !strings.Contains(res.stdout, "list the instance under providers in hub.yml") { + t.Errorf("check in GitLab CI: exit %d\n%s%s", res.code, res.stdout, res.stderr) + } + h.write("targets.yml", "version: 1\ntargets:\n - repo: https://gitlab.com/platform/api\n") + h.commit("a target on gitlab.com") + ci["CI_SERVER_URL"] = "https://gitlab.com" + res = runWith(t, ci, "check", "--hub", h.dir) + if res.code != 0 || strings.Contains(res.stdout, "list the instance under providers") { + t.Errorf("check in GitLab CI on gitlab.com: exit %d\n%s%s", res.code, res.stdout, res.stderr) + } +} diff --git a/internal/cli/testdata/golden/assumed/apply.json b/internal/cli/testdata/golden/assumed/apply.json new file mode 100644 index 0000000..e8e542a --- /dev/null +++ b/internal/cli/testdata/golden/assumed/apply.json @@ -0,0 +1,102 @@ +{ + "command": "apply", + "dry_run": false, + "hub": { + "dir": "$HUB", + "id": "acme-eng", + "commit": "$COMMIT" + }, + "target": { + "root": "$TARGET", + "ref": "acme/svc", + "opt_in_file": ".engineering-assets.yml", + "opted_in": true, + "opt_in": "assumed" + }, + "selection": { + "packs": [ + "base" + ], + "complete": true, + "unresolved": [], + "sources": { + "base": [ + "defaults" + ] + } + }, + "entries": [ + { + "path": ".github/pull_request_template.md", + "state": "missing", + "action": "create", + "pack": "base", + "from": "", + "to": "da9823e8fe7223ced65fd1d888c4086c5663d588", + "mode": "100644", + "detail": "", + "after_deletes": false + }, + { + "path": "AGENTS.md", + "state": "missing", + "action": "create", + "pack": "base", + "from": "", + "to": "f14d69790c939769881dcc505507364b7d1f2f55", + "mode": "100644", + "detail": "", + "after_deletes": false + }, + { + "path": "docs/guide.md", + "state": "missing", + "action": "create", + "pack": "base", + "from": "", + "to": "b05b6d090e03924f92073d05c2f7471dfc33dc66", + "mode": "100644", + "detail": "", + "after_deletes": false + } + ], + "results": [ + { + "path": ".github/pull_request_template.md", + "action": "create", + "state": "missing", + "outcome": "done", + "detail": "" + }, + { + "path": "AGENTS.md", + "action": "create", + "state": "missing", + "outcome": "done", + "detail": "" + }, + { + "path": "docs/guide.md", + "action": "create", + "state": "missing", + "outcome": "done", + "detail": "" + } + ], + "summary": { + "missing": 3, + "current": 0, + "outdated": 0, + "local": 0, + "ignored": 0, + "retired": 0, + "retired_local": 0, + "unsafe": 0, + "orphaned": 0, + "changes": 3, + "done": 3, + "skipped": 0, + "failed": 0 + }, + "warnings": [] +} diff --git a/internal/cli/testdata/golden/assumed/status.txt b/internal/cli/testdata/golden/assumed/status.txt new file mode 100644 index 0000000..94c06ac --- /dev/null +++ b/internal/cli/testdata/golden/assumed/status.txt @@ -0,0 +1,10 @@ +touchmark status · hub acme-eng @ $COMMIT · target acme/svc +opted in by targets.yml (opt_in: assumed): .engineering-assets.yml not found in $TARGET +packs: base (defaults) + + create missing .github/pull_request_template.md base + create missing AGENTS.md base + create missing docs/guide.md base + +missing 3 +3 changes to apply: run touchmark apply diff --git a/internal/cli/testdata/golden/assumed/tree.txt b/internal/cli/testdata/golden/assumed/tree.txt new file mode 100644 index 0000000..8528f56 --- /dev/null +++ b/internal/cli/testdata/golden/assumed/tree.txt @@ -0,0 +1,3 @@ +.github/pull_request_template.md base PR template v1 +AGENTS.md base AGENTS.md v1 +docs/guide.md base guide v1 diff --git a/internal/cli/testdata/golden/crlf-committed/apply-adopt.json b/internal/cli/testdata/golden/crlf-committed/apply-adopt.json index e1987b2..75c2edb 100644 --- a/internal/cli/testdata/golden/crlf-committed/apply-adopt.json +++ b/internal/cli/testdata/golden/crlf-committed/apply-adopt.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/crlf-committed/apply.json b/internal/cli/testdata/golden/crlf-committed/apply.json index a0518b7..aacac75 100644 --- a/internal/cli/testdata/golden/crlf-committed/apply.json +++ b/internal/cli/testdata/golden/crlf-committed/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/crlf/apply.json b/internal/cli/testdata/golden/crlf/apply.json index fe99cfd..30f9bec 100644 --- a/internal/cli/testdata/golden/crlf/apply.json +++ b/internal/cli/testdata/golden/crlf/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/file-directory/apply.json b/internal/cli/testdata/golden/file-directory/apply.json index 0adbb48..4c04a25 100644 --- a/internal/cli/testdata/golden/file-directory/apply.json +++ b/internal/cli/testdata/golden/file-directory/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/file-directory/status.json b/internal/cli/testdata/golden/file-directory/status.json index 5ff324e..80553bb 100644 --- a/internal/cli/testdata/golden/file-directory/status.json +++ b/internal/cli/testdata/golden/file-directory/status.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/formerly/apply.json b/internal/cli/testdata/golden/formerly/apply.json index a8c438b..8a1a6fa 100644 --- a/internal/cli/testdata/golden/formerly/apply.json +++ b/internal/cli/testdata/golden/formerly/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/fresh-opt-in/apply-again.json b/internal/cli/testdata/golden/fresh-opt-in/apply-again.json index 6451635..cb43382 100644 --- a/internal/cli/testdata/golden/fresh-opt-in/apply-again.json +++ b/internal/cli/testdata/golden/fresh-opt-in/apply-again.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/fresh-opt-in/apply.json b/internal/cli/testdata/golden/fresh-opt-in/apply.json index 0682248..aecc57d 100644 --- a/internal/cli/testdata/golden/fresh-opt-in/apply.json +++ b/internal/cli/testdata/golden/fresh-opt-in/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/fresh-opt-in/status.json b/internal/cli/testdata/golden/fresh-opt-in/status.json index 7479e5e..57d3362 100644 --- a/internal/cli/testdata/golden/fresh-opt-in/status.json +++ b/internal/cli/testdata/golden/fresh-opt-in/status.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/ignore-subtree/apply.json b/internal/cli/testdata/golden/ignore-subtree/apply.json index 766a0a0..9ca877b 100644 --- a/internal/cli/testdata/golden/ignore-subtree/apply.json +++ b/internal/cli/testdata/golden/ignore-subtree/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/in-sync/apply.json b/internal/cli/testdata/golden/in-sync/apply.json index ab8dc54..cbfc415 100644 --- a/internal/cli/testdata/golden/in-sync/apply.json +++ b/internal/cli/testdata/golden/in-sync/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/incomplete-selection/apply-packs.json b/internal/cli/testdata/golden/incomplete-selection/apply-packs.json index eb40974..786bd06 100644 --- a/internal/cli/testdata/golden/incomplete-selection/apply-packs.json +++ b/internal/cli/testdata/golden/incomplete-selection/apply-packs.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/incomplete-selection/status.json b/internal/cli/testdata/golden/incomplete-selection/status.json index 0c4e07c..8145e13 100644 --- a/internal/cli/testdata/golden/incomplete-selection/status.json +++ b/internal/cli/testdata/golden/incomplete-selection/status.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/legacy/apply.json b/internal/cli/testdata/golden/legacy/apply.json index f487f24..afe0c79 100644 --- a/internal/cli/testdata/golden/legacy/apply.json +++ b/internal/cli/testdata/golden/legacy/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/local-edit/apply-adopt.json b/internal/cli/testdata/golden/local-edit/apply-adopt.json index ce418d1..6a5be93 100644 --- a/internal/cli/testdata/golden/local-edit/apply-adopt.json +++ b/internal/cli/testdata/golden/local-edit/apply-adopt.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/local-edit/apply.json b/internal/cli/testdata/golden/local-edit/apply.json index 635db84..1ec5986 100644 --- a/internal/cli/testdata/golden/local-edit/apply.json +++ b/internal/cli/testdata/golden/local-edit/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/migrate/multi-gitter.txt b/internal/cli/testdata/golden/migrate/multi-gitter.txt index afb6a07..dda025a 100644 --- a/internal/cli/testdata/golden/migrate/multi-gitter.txt +++ b/internal/cli/testdata/golden/migrate/multi-gitter.txt @@ -53,6 +53,7 @@ targets: # multi-gitter took these projects only with one of the topics "python", "go"; repo entries take no topics. - repo: acme/tools/cli # TODO: packs: which packs every target gets (defaults.packs, packs per entry); the opt-in files add theirs. +# Targets need their own opt-in file, as they did with multi-gitter. To subscribe repositories without one, add opt_in: assumed under defaults or to an entry; a repository opts out with enabled: false in its opt-in file. exclude: - platform/legacy diff --git a/internal/cli/testdata/golden/not-opted-in/apply.json b/internal/cli/testdata/golden/not-opted-in/apply.json index 5cc3e04..3659133 100644 --- a/internal/cli/testdata/golden/not-opted-in/apply.json +++ b/internal/cli/testdata/golden/not-opted-in/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": false + "opted_in": false, + "opt_in": "none" }, "selection": null, "entries": [], diff --git a/internal/cli/testdata/golden/opted-out/apply.json b/internal/cli/testdata/golden/opted-out/apply.json new file mode 100644 index 0000000..f0e0588 --- /dev/null +++ b/internal/cli/testdata/golden/opted-out/apply.json @@ -0,0 +1,35 @@ +{ + "command": "apply", + "dry_run": false, + "hub": { + "dir": "$HUB", + "id": "acme-eng", + "commit": "$COMMIT" + }, + "target": { + "root": "$TARGET", + "ref": "acme/svc", + "opt_in_file": ".engineering-assets.yml", + "opted_in": false, + "opt_in": "opted-out" + }, + "selection": null, + "entries": [], + "results": [], + "summary": { + "missing": 0, + "current": 0, + "outdated": 0, + "local": 0, + "ignored": 0, + "retired": 0, + "retired_local": 0, + "unsafe": 0, + "orphaned": 0, + "changes": 0, + "done": 0, + "skipped": 0, + "failed": 0 + }, + "warnings": [] +} diff --git a/internal/cli/testdata/golden/opted-out/status.txt b/internal/cli/testdata/golden/opted-out/status.txt new file mode 100644 index 0000000..05ecaf7 --- /dev/null +++ b/internal/cli/testdata/golden/opted-out/status.txt @@ -0,0 +1,2 @@ +touchmark status · hub acme-eng @ $COMMIT · target acme/svc +opted out: .engineering-assets.yml says enabled: false diff --git a/internal/cli/testdata/golden/orphaned/apply.json b/internal/cli/testdata/golden/orphaned/apply.json index 8f4590d..a1ec4d0 100644 --- a/internal/cli/testdata/golden/orphaned/apply.json +++ b/internal/cli/testdata/golden/orphaned/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/outdated/apply.json b/internal/cli/testdata/golden/outdated/apply.json index 09c7ee3..464e26e 100644 --- a/internal/cli/testdata/golden/outdated/apply.json +++ b/internal/cli/testdata/golden/outdated/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/pack-override/apply.json b/internal/cli/testdata/golden/pack-override/apply.json index bd46996..f4136ea 100644 --- a/internal/cli/testdata/golden/pack-override/apply.json +++ b/internal/cli/testdata/golden/pack-override/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/plain-directory/apply.json b/internal/cli/testdata/golden/plain-directory/apply.json index 4b457bb..b95cbb6 100644 --- a/internal/cli/testdata/golden/plain-directory/apply.json +++ b/internal/cli/testdata/golden/plain-directory/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/plan-assume/plan.json b/internal/cli/testdata/golden/plan-assume/plan.json index a37309f..41f830a 100644 --- a/internal/cli/testdata/golden/plan-assume/plan.json +++ b/internal/cli/testdata/golden/plan-assume/plan.json @@ -65,6 +65,7 @@ "base" ], "opt_in_assumed": true, + "opt_in_assumed_by": "--assume-opt-in", "changes": { "create": 2, "update": 0, diff --git a/internal/cli/testdata/golden/plan-assume/plan.md b/internal/cli/testdata/golden/plan-assume/plan.md index 041b4ae..db8b4fa 100644 --- a/internal/cli/testdata/golden/plan-assume/plan.md +++ b/internal/cli/testdata/golden/plan-assume/plan.md @@ -7,7 +7,7 @@ Hub `acme-eng` (`github.com/712345678`) at `$COMMIT` · touchmark dev | `gh` | `github.com` | `acme-read[bot]` | `acme-write[bot]`: not checked | resolve complete | | `corp` | `gitlab.example.com` | `tm-reader` | `tm-writer`: not checked | resolve complete | -**\-\-assume\-opt\-in:** every target counts as opted in, for this report only; 1 target without an opt\-in file is planned as if it had an empty one +**\-\-assume\-opt\-in:** for this report only, a target without an opt\-in file counts as opted in \(one whose file says enabled\: false stays opted out\); 1 target is planned as if it had an empty one | Outcome | Targets | |---|---:| diff --git a/internal/cli/testdata/golden/plan-assume/plan.txt b/internal/cli/testdata/golden/plan-assume/plan.txt index f4f36c3..aca55ab 100644 --- a/internal/cli/testdata/golden/plan-assume/plan.txt +++ b/internal/cli/testdata/golden/plan-assume/plan.txt @@ -1,7 +1,7 @@ touchmark plan · hub acme-eng (github.com/712345678) @ $COMMIT · touchmark dev gh github.com read acme-read[bot] write acme-write[bot]: not checked resolve complete corp gitlab.example.com read tm-reader write tm-writer: not checked resolve complete ---assume-opt-in: every target counts as opted in, for this report only; 1 target without an opt-in file is planned as if it had an empty one +--assume-opt-in: for this report only, a target without an opt-in file counts as opted in (one whose file says enabled: false stays opted out); 1 target is planned as if it had an empty one open 1 gh:acme/web unchanged 1 gh:acme/api #1 diff --git a/internal/cli/testdata/golden/requires/apply.json b/internal/cli/testdata/golden/requires/apply.json index 5383d59..88b3186 100644 --- a/internal/cli/testdata/golden/requires/apply.json +++ b/internal/cli/testdata/golden/requires/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/retire/apply.json b/internal/cli/testdata/golden/retire/apply.json index 72aaf4c..6c3cade 100644 --- a/internal/cli/testdata/golden/retire/apply.json +++ b/internal/cli/testdata/golden/retire/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/tiny-version/apply.json b/internal/cli/testdata/golden/tiny-version/apply.json index e1ee1e4..58750e7 100644 --- a/internal/cli/testdata/golden/tiny-version/apply.json +++ b/internal/cli/testdata/golden/tiny-version/apply.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/cli/testdata/golden/worktree/apply-worktree.json b/internal/cli/testdata/golden/worktree/apply-worktree.json index 694b374..99eb4e9 100644 --- a/internal/cli/testdata/golden/worktree/apply-worktree.json +++ b/internal/cli/testdata/golden/worktree/apply-worktree.json @@ -10,7 +10,8 @@ "root": "$TARGET", "ref": "acme/svc", "opt_in_file": ".engineering-assets.yml", - "opted_in": true + "opted_in": true, + "opt_in": "file" }, "selection": { "packs": [ diff --git a/internal/config/assume_test.go b/internal/config/assume_test.go new file mode 100644 index 0000000..79a36ad --- /dev/null +++ b/internal/config/assume_test.go @@ -0,0 +1,130 @@ +package config + +import ( + "reflect" + "testing" +) + +// TestTargetsAssumed: an entry's opt_in is its own, else defaults.opt_in, +// else required; one entry with assumed among those that select a target +// is enough, whatever the others say. +func TestTargetsAssumed(t *testing.T) { + targets := &Targets{ + Defaults: Defaults{OptIn: OptInAssumed}, + Targets: []Entry{ + {Org: "acme"}, + {Repo: "acme/x", OptIn: OptInRequired}, + {Repo: "acme/y", OptIn: OptInAssumed}, + }, + } + for _, tc := range []struct { + entries []int + want bool + }{ + {nil, false}, + {[]int{0}, true}, + {[]int{1}, false}, + {[]int{1, 0}, true}, + {[]int{2}, true}, + {[]int{1, 7, -1}, false}, + } { + if got := targets.Assumed(tc.entries); got != tc.want { + t.Errorf("Assumed(%v) = %v, want %v", tc.entries, got, tc.want) + } + } + targets.Defaults.OptIn = "" + if targets.Assumed([]int{0, 1}) || !targets.Assumed([]int{2}) { + t.Error("without defaults.opt_in, only an entry's own assumed counts") + } + var none *Targets + if none.Assumed([]int{0}) { + t.Error("nil targets assume something") + } +} + +// TestAssume: what a local run can tell about a target without an opt-in +// file. +func TestAssume(t *testing.T) { + hub := selectHub("gh", "corp") + targets := &Targets{ + Defaults: Defaults{Provider: "gh"}, + Targets: []Entry{ + {Repo: "acme/billing", OptIn: OptInAssumed}, + {Repo: "acme/web"}, + {Org: "acme", OptIn: OptInAssumed, Match: []string{"acme/svc-*"}}, + {Group: "corp:platform", OptIn: OptInAssumed}, + {Org: "acme", Packs: []string{"claude"}}, + {Repo: "acme/legacy", OptIn: OptInAssumed}, + {Repo: "acme/old-1", OptIn: OptInAssumed}, + }, + Exclude: []string{"acme/legacy", "acme/old-*"}, + } + for _, tc := range []struct { + target string + want Assumption + }{ + {"acme/billing", Assumption{Assumed: true}}, + {"gh:ACME/Billing", Assumption{Assumed: true}}, + {"corp:acme/billing", Assumption{}}, + {"acme/web", Assumption{}}, + {"acme/svc-api", Assumption{Unresolved: []string{"org: acme"}}}, + {"corp:platform/team/api", Assumption{Unresolved: []string{"group: corp:platform"}}}, + {"acme/legacy", Assumption{}}, + {"acme/old-1", Assumption{}}, + {"acme/svc-old-1", Assumption{Unresolved: []string{"org: acme"}}}, + } { + got, err := Assume(hub, targets, ref(t, tc.target)) + if err != nil || !reflect.DeepEqual(got, tc.want) { + t.Errorf("Assume(%s) = %+v, %v; want %+v", tc.target, got, err, tc.want) + } + } + // An explicit entry decides: the org entries need not be resolved. + targets.Targets = append(targets.Targets, Entry{Repo: "acme/svc-api", OptIn: OptInAssumed}) + if got, _ := Assume(hub, targets, ref(t, "acme/svc-api")); !reflect.DeepEqual(got, Assumption{Assumed: true}) { + t.Errorf("explicit and org entry: %+v", got) + } + // defaults.opt_in applies to every entry. + plain := &Targets{Defaults: Defaults{OptIn: OptInAssumed}, Targets: []Entry{{Repo: "acme/x"}}} + if got, _ := Assume(hub, plain, ref(t, "acme/x")); !got.Assumed { + t.Error("defaults.opt_in: assumed does not reach the repo entry") + } + if got, err := Assume(nil, nil, ref(t, "acme/x")); err != nil || got.Assumed || got.Unresolved != nil { + t.Errorf("Assume without targets = %+v, %v", got, err) + } + if _, err := Assume(hub, &Targets{Exclude: []string{"acme/[x]"}}, ref(t, "acme/x")); err == nil { + t.Error("a malformed exclude entry is no error") + } +} + +// TestOptInEnabled: enabled: false opts out; an absent or null enabled, or +// true, does not; enabled does not count in the hash, which lifts declines +// only for packs and ignore. +func TestOptInEnabled(t *testing.T) { + parse := func(s string) *OptIn { + t.Helper() + o, _, err := ParseOptIn([]byte(s)) + if err != nil { + t.Fatal(err) + } + return o + } + for content, want := range map[string]bool{ + "": false, + "version: 1\n": false, + "version: 1\nenabled: true\n": false, + "version: 1\nenabled:\n": false, + "version: 1\nenabled: false\n": true, + "enabled: false\npacks: [x-y]\n": true, + } { + if got := parse(content).Disabled(); got != want { + t.Errorf("%q: Disabled() = %v, want %v", content, got, want) + } + } + var none *OptIn + if none.Disabled() { + t.Error("a nil opt-in is disabled") + } + if a, b := parse("version: 1\n").Hash(), parse("version: 1\nenabled: false\n").Hash(); a != b { + t.Errorf("enabled changes the hash: %s, %s", a, b) + } +} diff --git a/internal/config/check.go b/internal/config/check.go index f766e51..870346a 100644 --- a/internal/config/check.go +++ b/internal/config/check.go @@ -38,9 +38,15 @@ func check(hub *Hub, targets *Targets, known map[string]bool) ([]Warning, []erro c.packMeta() c.formerly() c.cycles() + if targets.HasURLs() { + c.errorf(TargetsFile, "holds web URLs that were not resolved to providers (ResolveURLs)") + return c.warns, c.errs + } c.targetPacks() c.providers() c.exclude() + c.match() + c.optIn() return c.warns, c.errs } @@ -163,31 +169,127 @@ func (c *checker) providers() { } } for i, ex := range c.targets.Exclude { - if r, err := ParseRef(ex); err == nil { + if r, err := parsePattern(ex); err == nil { ref(fmt.Sprintf("exclude[%d]", i), r.Provider) } } } // exclude checks that exclude entries parse and warns about targets that are -// both listed and excluded. +// both listed and excluded. A pattern that covers no repository listed +// explicitly is fine: it is there for the repositories of org and group +// entries. On a platform without nested namespaces, an entry of more than +// owner/name without a "**" (which may span nothing) can cover no +// repository: a warning. func (c *checker) exclude() { res := resolver{hub: c.hub, targets: c.targets} for i, ex := range c.targets.Exclude { - exRef, err := ParseRef(ex) + pat, err := parsePattern(ex) if err != nil { c.errorf(TargetsFile, "exclude[%d]: %v", i, err) continue } + segs := strings.Split(pat.Path, "/") + if typ := c.flatType(res.provider(pat.Provider)); typ != "" && len(segs) > 2 && !slices.Contains(segs, "**") { + c.warnf(TargetsFile, "exclude[%d]: %s has %d path segments, and a %s repository path is owner/name, so it excludes nothing", i, ex, len(segs), typ) + } for j := range c.targets.Targets { e := &c.targets.Targets[j] if entryKind(e) != "repo" { continue } r, err := ParseRef(e.Repo) - if err == nil && res.matchRepo(r, e.Provider, exRef) { + if err != nil { + continue + } + r.Provider = firstNonEmpty(e.Provider, r.Provider) + if !res.matchPattern(pat, r) { + continue + } + if isGlob(pat.Path) { + c.warnf(TargetsFile, "exclude[%d]: %s also covers targets[%d] (%s); exclude wins", i, ex, j, e.Repo) + } else { c.warnf(TargetsFile, "exclude[%d]: %s is also listed as targets[%d]; exclude wins", i, ex, j) } } } } + +// match warns about a pattern of an org or group entry's match that no +// repository of its namespace can match: it selects nothing. A namespace +// on a platform without nested namespaces (flatType) holds no subgroups, +// whatever subgroups says. +func (c *checker) match() { + res := resolver{hub: c.hub, targets: c.targets} + for i := range c.targets.Targets { + e := &c.targets.Targets[i] + if entryKind(e) == "repo" { + continue + } + ns, err := entryRef(e) + if err != nil { + continue + } + typ := c.flatType(res.provider(firstNonEmpty(e.Provider, ns.Provider))) + for j, pat := range e.Match { + if checkMatchPattern(pat) == nil && !globUnder(pat, ns.Path, includeSubgroups(e) && typ == "") { + where := "under " + ns.Path + switch { + case !includeSubgroups(e): + where = "directly under " + ns.Path + " (subgroups: false)" + case typ != "" && globUnder(pat, ns.Path, true): + where = "directly under " + ns.Path + " (a " + typ + " namespace has no subgroups)" + } + c.warnf(TargetsFile, "targets[%d].match[%d]: %s matches no repository %s, so it selects nothing", i, j, pat, where) + } + } + } +} + +// flatType returns the type of the provider with id when its platform has no +// nested namespaces (github, gitea, forgejo): a repository path there is +// owner/name. It returns "" for any other type, and for a provider hub.yml +// does not list (the implicit one, whose type the CI tells). +func (c *checker) flatType(id string) string { + for _, p := range c.hub.Providers { + if p.ID != id { + continue + } + switch p.Type { + case "github", "gitea", "forgejo": + return p.Type + } + return "" + } + return "" +} + +// optIn warns about a repo entry whose opt_in: required says nothing: an +// org or group entry with opt_in: assumed may select the same repository, +// and one entry with assumed is enough to subscribe it (Targets.Assumed). +func (c *checker) optIn() { + res := resolver{hub: c.hub, targets: c.targets} + for i := range c.targets.Targets { + e := &c.targets.Targets[i] + if entryKind(e) != "repo" || e.OptIn != OptInRequired { + continue + } + r, err := ParseRef(e.Repo) + if err != nil { + continue + } + r.Provider = firstNonEmpty(e.Provider, r.Provider) + for j := range c.targets.Targets { + o := &c.targets.Targets[j] + if entryKind(o) == "repo" || c.targets.effectiveOptIn(o) != OptInAssumed { + continue + } + ns, err := entryRef(o) + if err == nil && res.mayContain(o, ns, r) { + c.warnf(TargetsFile, "targets[%d]: opt_in: required does not hold %s back: targets[%d] (%s: %s) has opt_in: assumed and may select it, "+ + "and one entry with assumed subscribes a repository; to have it opt in itself, leave it out of targets[%d] with match", i, e.Repo, j, entryKind(o), selectorValue(o), j) + break + } + } + } +} diff --git a/internal/config/clean.go b/internal/config/clean.go index b2f0cb4..0a1483a 100644 --- a/internal/config/clean.go +++ b/internal/config/clean.go @@ -2,6 +2,7 @@ package config import ( "errors" + "regexp" "strconv" "strings" "unicode" @@ -15,11 +16,19 @@ import ( // terminals and CI logs. Every error and warning the parsers return is // cleaned first: control characters (newlines included), bidirectional // overrides and invalid UTF-8 are escaped, so a value cannot forge a log -// line, a CI annotation (::error ...) or a terminal escape; and whatever -// looks like a credential is cut to its prefix. +// line, a CI annotation (::error ...) or a terminal escape; whatever +// looks like a credential is cut to its prefix; and the user information +// of a URL (user:password@) is cut. // cleanMessage returns s safe to print. -func cleanMessage(s string) string { return redactSecrets(escapeUnprintable(s)) } +func cleanMessage(s string) string { return redactUserinfo(redactSecrets(escapeUnprintable(s))) } + +// userinfoRe matches the scheme and user information of a URL: everything +// between "://" and the "@" that ends the user information. +var userinfoRe = regexp.MustCompile(`([A-Za-z][A-Za-z0-9+.-]*://)[^/?#@\s"'\x60]*@`) + +// redactUserinfo cuts the user information of every URL in s. +func redactUserinfo(s string) string { return userinfoRe.ReplaceAllString(s, "${1}…@") } // escapeUnprintable escapes, Go style, every rune of s that could change // how the text around it is displayed. @@ -103,11 +112,17 @@ func cleanWarnings(warns []Warning) []Warning { } // yamlError describes an error of the YAML decoder in one line. A type -// error lists one problem per line; they are joined with "; ". +// error lists one problem per line; they are joined with "; ". The errors +// of a bad alias get a hint: a glob pattern that starts with "*" (**/legacy) +// is read as one unless quoted. func yamlError(err error) string { var te *yaml.TypeError if errors.As(err, &te) { return "yaml: " + strings.Join(te.Errors, "; ") } - return err.Error() + msg := err.Error() + if strings.Contains(msg, "did not find expected alphabetic or numeric character") || strings.Contains(msg, "unknown anchor") { + msg += ` (a value that starts with * is read as a YAML alias: quote it, as in "**/legacy")` + } + return msg } diff --git a/internal/config/config.go b/internal/config/config.go index 5d262a1..bc35256 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -174,10 +174,26 @@ type Targets struct { type Defaults struct { Provider string `yaml:"provider"` Packs []string `yaml:"packs"` + // OptIn is the opt_in of every entry that sets none: OptInRequired + // (the default when empty) or OptInAssumed. + OptIn string `yaml:"opt_in"` } +// Values of opt_in in targets.yml. +const ( + // OptInRequired: a target the entry selects gets nothing until it has + // an opt-in file. + OptInRequired = "required" + // OptInAssumed: a target the entry selects counts as opted in without + // an opt-in file, with the packs targets.yml gives it. + OptInAssumed = "assumed" +) + // Entry is one item of targets.yml `targets:`. Exactly one of Repo, Org and // Group is set. Org and Group are synonyms (a namespace). +// +// Repo, Org and Group may be web URLs as written in the file; ResolveURLs +// turns them into :, which is what everything else reads. type Entry struct { Repo string `yaml:"repo"` Org string `yaml:"org"` @@ -186,7 +202,12 @@ type Entry struct { Topics []string `yaml:"topics"` Subgroups *bool `yaml:"subgroups"` Forks bool `yaml:"forks"` - Packs []string `yaml:"packs"` + // Match keeps, of an org or group entry, the repositories whose full + // path matches one of these glob patterns. + Match []string `yaml:"match"` + Packs []string `yaml:"packs"` + // OptIn is OptInRequired, OptInAssumed, or "" for defaults.opt_in. + OptIn string `yaml:"opt_in"` } // OptIn is the target's opt-in file. Its presence is the consent; it may be @@ -195,10 +216,17 @@ type OptIn struct { Version int `yaml:"version"` Packs []string `yaml:"packs"` Ignore []string `yaml:"ignore"` + // Enabled false opts the repository out, whatever targets.yml says; + // absent means true. + Enabled *bool `yaml:"enabled"` Legacy bool `yaml:"-"` // no version key } +// Disabled reports whether the opt-in file says enabled: false. A nil +// opt-in is not disabled. +func (o *OptIn) Disabled() bool { return o != nil && o.Enabled != nil && !*o.Enabled } + // ParseHub decodes and validates hub.yml. data == nil means the file is // absent (legacy hub). // @@ -230,6 +258,12 @@ func ParseHub(data []byte) (*Hub, []Warning, error) { // ParseTargets decodes and validates targets.yml. data == nil means absent: // no targets, no defaults. // +// A value of repo, org, group or exclude may be a web URL: its form is +// checked here, and ResolveURLs, which knows the providers, rewrites it as +// :. Exclude entries are patterns that may hold the glob +// characters * and ? (glob.go), as are the patterns of an org or group +// entry's match; opt_in is required or assumed. +// // The pre-v1 format, a bare `repos:` list without version, is converted // to Targets entries with Legacy set and a warning. func ParseTargets(data []byte) (*Targets, []Warning, error) { @@ -240,7 +274,8 @@ func ParseTargets(data []byte) (*Targets, []Warning, error) { // ParseOptIn decodes and validates an opt-in file. An empty file is valid. // // Version is 1 after parsing; Legacy records that the file had no version -// key (the pre-v1 format, or an empty file). +// key (the pre-v1 format, or an empty file). enabled: false opts the +// repository out (Disabled). func ParseOptIn(data []byte) (*OptIn, []Warning, error) { o, warns, err := parseOptIn(data) return o, cleanWarnings(warns), cleanErr(err) @@ -295,12 +330,15 @@ type Selection struct { // yields an empty, complete selection with a warning. Unknown packs and // requires cycles are errors (known lists the packs the hub ships now). // -// Only org/group entries that carry packs and whose namespace could contain -// target count as unresolved; the others cannot change the result. A repo -// entry or an exclude names target when the paths are equal ignoring case -// and the providers are equal. Each side's provider is its own (entry -// provider, then ref prefix), else targets.Defaults.Provider, else the hub's -// only provider; when either side stays unknown, the path alone decides. +// Only org/group entries that carry packs and whose namespace and match +// patterns could contain target count as unresolved; the others cannot +// change the result. A repo entry names target when the paths are equal +// ignoring case and the providers are equal; an exclude entry covers it +// when its pattern matches the path (an entry without glob characters is +// an equal path) and the providers are equal. Each side's provider is its +// own (entry provider, then ref prefix), else targets.Defaults.Provider, +// else the hub's only provider; when either side stays unknown, the path +// alone decides. targets must hold no URL: ResolveURLs rewrites them first. // A pack's former name (formerly) selects the pack, with a warning, when // that pack exists; a former name of a pack that is gone is unknown. func Select(hub *Hub, targets *Targets, optIn *OptIn, target Ref, known map[string]bool) (Selection, []Warning, error) { @@ -340,8 +378,13 @@ func (h *Hub) OptInName() string { // // It also rejects the template's placeholder id, and requires every entry // to name a provider (itself or through defaults.provider) when the hub has -// more than one. Former pack names in references, metadata for packs that -// do not exist, and targets both listed and excluded are warnings. +// more than one. targets must hold no URL: ResolveURLs rewrites them first, +// and one left is an error. Former pack names in references, metadata for +// packs that do not exist, targets both listed and excluded (by name or by +// pattern), a match pattern no repository of its namespace can match, and +// a repo entry's opt_in: required that an org or group entry with opt_in: +// assumed overrides are warnings. An exclude pattern that covers no listed +// repository is fine: it is there for org and group entries. func Check(hub *Hub, targets *Targets, known map[string]bool) ([]Warning, []error) { return check(hub, targets, known) } diff --git a/internal/config/glob.go b/internal/config/glob.go new file mode 100644 index 0000000..47ec350 --- /dev/null +++ b/internal/config/glob.go @@ -0,0 +1,131 @@ +package config + +import ( + "fmt" + "strings" + + "github.com/bedrock-python/touchmark/internal/pathx" +) + +// Glob patterns over repository paths: the entries of exclude and the +// patterns of an org or group entry's match. +// +// A pattern is a repository path whose segments may hold glob characters: +// "*" matches any run of characters within one segment, "?" any one +// character, and a segment that is "**" spans any number of segments, +// none included. "**" inside a longer segment is "*" twice. Patterns +// compare ignoring case, as repository paths do. A pattern without glob +// characters names one repository and matches its path only, never the +// paths beneath it: an exclude entry without them behaves as a plain +// repository reference. + +// globChars are the characters that make a pattern a glob. +const globChars = "*?" + +func isGlobChar(c rune) bool { return strings.ContainsRune(globChars, c) } + +// isGlob reports whether pattern holds glob characters. +func isGlob(pattern string) bool { return strings.ContainsAny(pattern, globChars) } + +// matchPath reports whether the repository path p matches pattern. +func matchPath(pattern, p string) bool { + if !isGlob(pattern) { + return strings.EqualFold(pattern, p) + } + return pathx.MatchSegments(pathx.Fold(pattern), pathx.Fold(p)) +} + +// matchAnyPath reports whether p matches one of patterns. +func matchAnyPath(patterns []string, p string) bool { + for _, pat := range patterns { + if matchPath(pat, p) { + return true + } + } + return false +} + +// checkMatchPattern validates a pattern of match: a full repository path +// with glob characters, without a provider prefix. +func checkMatchPattern(s string) error { + switch { + case len(s) > maxRefLen: + return fmt.Errorf("%.40q…: longer than %d bytes", displayURL(s), maxRefLen) + case isURL(s): + return fmt.Errorf("%q: a URL is not accepted here; match takes repository paths such as acme/svc-*", displayURL(s)) + case strings.Contains(s, ":"): + return fmt.Errorf("%q: match takes repository paths without a provider, such as acme/svc-*", s) + } + if err := checkRefPath(s, 2, true); err != nil { + return fmt.Errorf("%q: %w", s, err) + } + return nil +} + +// globUnder reports whether a repository under namespace ns can match +// pattern: directly under it, or, with subgroups, at any depth. It is +// what check uses to warn about a match pattern that selects nothing. +func globUnder(pattern, ns string, subgroups bool) bool { + pat := strings.Split(pathx.Fold(pattern), "/") + segs := strings.Split(pathx.Fold(ns), "/") + // at[i]: pat[:i] matches the whole of segs. + at := globPrefixes(pat, segs) + for i, ok := range at { + if !ok { + continue + } + rest := pat[i:] + // A "**" that took the last namespace segment can take the + // repository's segments too, when the rest matches nothing. + if i > 0 && pat[i-1] == "**" && onlyStars(rest) { + return true + } + literal := 0 + for _, s := range rest { + if s != "**" { + literal++ + } + } + switch { + case len(rest) == 0: + case subgroups: + return true + case literal <= 1: + return true + } + } + return false +} + +// globPrefixes returns, for each i from 0 to len(pat), whether pat[:i] +// matches segs as a whole. +func globPrefixes(pat, segs []string) []bool { + n := len(segs) + prev := make([]bool, n+1) // pat[:i-1] matches segs[:j] + prev[0] = true + out := make([]bool, len(pat)+1) + out[0] = n == 0 + for i := 1; i <= len(pat); i++ { + cur := make([]bool, n+1) + for j := 0; j <= n; j++ { + if pat[i-1] == "**" { + cur[j] = prev[j] || (j > 0 && cur[j-1]) + } else { + cur[j] = j > 0 && prev[j-1] && pathx.MatchSegments(pat[i-1], segs[j-1]) + } + } + out[i] = cur[n] + prev = cur + } + return out +} + +// onlyStars reports whether every segment of pat is "**". +func onlyStars(pat []string) bool { + for _, s := range pat { + if s != "**" { + return false + } + } + return true +} diff --git a/internal/config/glob_test.go b/internal/config/glob_test.go new file mode 100644 index 0000000..271b85c --- /dev/null +++ b/internal/config/glob_test.go @@ -0,0 +1,291 @@ +package config + +import ( + "reflect" + "strings" + "testing" +) + +func TestMatchPath(t *testing.T) { + for _, tc := range []struct { + pattern, path string + want bool + }{ + // Without glob characters: one repository, ignoring case, nothing + // beneath it. + {"acme/legacy", "acme/legacy", true}, + {"acme/legacy", "ACME/Legacy", true}, + {"acme/legacy", "acme/legacy/sub", false}, + {"acme/legacy", "acme/legacy-x", false}, + // * stays within one segment. + {"acme/legacy-*", "acme/legacy-api", true}, + {"acme/legacy-*", "acme/legacy-", true}, + {"acme/legacy-*", "acme/legacy", false}, + {"acme/*", "acme/x/y", false}, + {"*/billing", "acme/billing", true}, + {"ACME/Legacy-*", "acme/LEGACY-web", true}, + // ? is one character. + {"acme/?ld", "acme/old", true}, + {"acme/?ld", "acme/bold", false}, + {"acme/?ld", "acme/ld", false}, + // ** spans segments, none included. + {"platform/legacy/**", "platform/legacy/api", true}, + {"platform/legacy/**", "platform/legacy", true}, + {"platform/legacy/**", "platform/legacy/a/b/c", true}, + {"platform/legacy/**", "platform/legacyx/api", false}, + {"platform/legacy/**", "platform/api", false}, + {"**/sandbox", "acme/sandbox", true}, + {"**/sandbox", "a/b/c/sandbox", true}, + {"**/sandbox", "acme/sandbox-x", false}, + {"acme/**/api", "acme/api", true}, + {"acme/**/api", "acme/x/y/api", true}, + {"acme/**/api", "acme/x/api/y", false}, + {"**", "any/thing/at/all", true}, + // ** inside a longer segment is * twice. + {"acme/svc-**", "acme/svc-a", true}, + {"acme/svc-**", "acme/svc-a/b", false}, + // A dot is an ordinary character. + {"acme/.*", "acme/.github", true}, + {"acme/*.go", "acme/x.go", true}, + } { + if got := matchPath(tc.pattern, tc.path); got != tc.want { + t.Errorf("matchPath(%q, %q) = %v, want %v", tc.pattern, tc.path, got, tc.want) + } + } +} + +func TestGlobUnder(t *testing.T) { + for _, tc := range []struct { + pattern, ns string + subgroups bool + want bool + }{ + {"acme/svc-*", "acme", true, true}, + {"ACME/svc-*", "acme", false, true}, + {"other/*", "acme", true, false}, + {"acme", "acme", true, false}, + {"acme/*", "acme/sub", true, false}, + {"acme/**", "acme/sub", true, true}, + {"acme/**", "acme", false, true}, + {"**/api", "platform", false, true}, + {"**/api", "platform/team", true, true}, + {"platform/*/api", "platform", false, false}, + {"platform/*/api", "platform", true, true}, + {"platform/team/**", "platform", false, true}, + {"*/x", "a/b", true, false}, + {"*/*/x", "a/b", true, true}, + {"a/**/b/**", "a", false, true}, + {"a/?", "a", false, true}, + } { + if got := globUnder(tc.pattern, tc.ns, tc.subgroups); got != tc.want { + t.Errorf("globUnder(%q, %q, subgroups %v) = %v, want %v", tc.pattern, tc.ns, tc.subgroups, got, tc.want) + } + } +} + +// TestExcludedPatterns: exclude patterns resolve their provider as a plain +// entry does, and a plain entry still names one repository only. +func TestExcludedPatterns(t *testing.T) { + hub := selectHub("gh", "corp") + targets := &Targets{ + Defaults: Defaults{Provider: "gh"}, + Exclude: []string{"acme/legacy", "corp:platform/legacy/**", "acme/sandbox-*", "**/archive-??"}, + } + for _, tc := range []struct { + provider, path string + index int + ok bool + }{ + {"gh", "acme/legacy", 0, true}, + {"gh", "acme/legacy/x", 0, false}, + {"corp", "platform/legacy/api", 1, true}, + {"corp", "platform/legacy/team/api", 1, true}, + {"gh", "platform/legacy/api", 0, false}, // corp's pattern + {"gh", "acme/sandbox-1", 2, true}, + {"corp", "acme/sandbox-1", 0, false}, // a bare pattern means defaults.provider + {"gh", "a/b/archive-17", 3, true}, + {"gh", "acme/archive-7", 0, false}, + } { + index, ok := Excluded(hub, targets, tc.provider, tc.path) + if index != tc.index || ok != tc.ok { + t.Errorf("Excluded(%q, %q) = %d, %v; want %d, %v", tc.provider, tc.path, index, ok, tc.index, tc.ok) + } + } +} + +// TestSelectorsMatchAndOptIn: an org or group entry's match patterns and +// every entry's opt_in, its own or defaults.opt_in, reach the selectors. +func TestSelectorsMatchAndOptIn(t *testing.T) { + targets, _, err := ParseTargets([]byte(`version: 1 +defaults: + opt_in: assumed +targets: + - org: acme + match: [acme/svc-*, acme/api] + - repo: acme/billing + opt_in: required + - group: platform + opt_in: assumed +`)) + if err != nil { + t.Fatal(err) + } + got, err := Selectors(selectHub("gh"), targets) + if err != nil { + t.Fatal(err) + } + want := []Selector{ + {Entry: 0, Provider: "gh", Namespace: "acme", Subgroups: true, Match: []string{"acme/svc-*", "acme/api"}, Assumed: true}, + {Entry: 1, Provider: "gh", Repo: "acme/billing"}, + {Entry: 2, Provider: "gh", Namespace: "platform", Subgroups: true, Assumed: true}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("Selectors =\n%+v\nwant\n%+v", got, want) + } + for path, want := range map[string]bool{"acme/svc-a": true, "ACME/API": true, "acme/apix": false, "acme/web": false} { + if got := got[0].Selects(path); got != want { + t.Errorf("Selects(%q) = %v, want %v", path, got, want) + } + } + if !got[2].Selects("platform/anything") { + t.Error("a selector without match does not select") + } + got[0].Match[0] = "changed" + if targets.Targets[0].Match[0] != "acme/svc-*" { + t.Error("Selectors shares the match patterns of targets.yml") + } +} + +// TestSelectExcludePatternAndMatch: local mode applies exclude patterns, +// and an org entry whose match leaves the target out cannot select it. +func TestSelectExcludePatternAndMatch(t *testing.T) { + hub := selectHub("gh") + targets := &Targets{ + Targets: []Entry{ + {Org: "acme", Match: []string{"acme/svc-*"}, Packs: []string{"python-service"}}, + {Org: "acme", Match: []string{"acme/lib-*"}, Packs: []string{"python-library"}}, + }, + Exclude: []string{"acme/svc-legacy*"}, + } + sel, _, err := Select(hub, targets, &OptIn{}, ref(t, "acme/svc-billing"), knownPacks()) + if err != nil { + t.Fatal(err) + } + if sel.Complete || !reflect.DeepEqual(sel.Unresolved, []string{"org: acme"}) { + t.Errorf("svc-billing: %+v, want only the svc entry unresolved", sel) + } + sel, warns, err := Select(hub, targets, &OptIn{}, ref(t, "acme/svc-legacy-1"), knownPacks()) + if err != nil || !sel.Complete || len(sel.Packs) != 0 || len(warns) != 1 || !strings.Contains(warns[0].Message, "excluded by exclude[0] (acme/svc-legacy*)") { + t.Errorf("svc-legacy-1: %+v, %v, %v", sel, warns, err) + } + sel, _, err = Select(hub, targets, &OptIn{}, ref(t, "acme/web"), knownPacks()) + if err != nil || !sel.Complete { + t.Errorf("web: %+v, %v; want complete: no entry's match takes it", sel, err) + } +} + +// TestParseTargetsUnquotedPattern: a pattern that starts with * is a YAML +// alias unless quoted; the parse error says so. +func TestParseTargetsUnquotedPattern(t *testing.T) { + for _, yml := range []string{ + "version: 1\nexclude:\n - **/legacy\n", + "version: 1\ntargets:\n - org: acme\n match: [*/svc-*]\n", + "version: 1\nexclude:\n - *-archive\n", + } { + _, _, err := ParseTargets([]byte(yml)) + if err == nil || !strings.Contains(err.Error(), `quote it, as in "**/legacy"`) { + t.Errorf("%q: %v", yml, err) + } + } + targets, _, err := ParseTargets([]byte("version: 1\nexclude:\n - \"**/legacy\"\n - '*-archive/x'\n")) + if err != nil || !reflect.DeepEqual(targets.Exclude, []string{"**/legacy", "*-archive/x"}) { + t.Errorf("quoted patterns: %+v, %v", targets, err) + } +} + +func TestCheckPatterns(t *testing.T) { + hub := selectHub("gh", "corp") + for _, tc := range []struct { + name string + targets *Targets + errors []string + warnings []string + }{ + { + name: "an exclude pattern that covers no listed repository is fine", + targets: &Targets{Defaults: Defaults{Provider: "gh"}, Targets: []Entry{{Org: "acme"}}, + Exclude: []string{"acme/legacy-*", "corp:platform/**"}}, + }, + { + name: "an exclude pattern that covers a listed repository", + targets: &Targets{Defaults: Defaults{Provider: "gh"}, + Targets: []Entry{{Repo: "acme/legacy-api"}, {Repo: "corp:acme/legacy-x"}, {Repo: "acme/billing"}}, + Exclude: []string{"acme/legacy-*"}}, + warnings: []string{"exclude[0]: acme/legacy-* also covers targets[0] (acme/legacy-api); exclude wins"}, + }, + { + name: "an exclude pattern with an unknown provider", + targets: &Targets{Defaults: Defaults{Provider: "gh"}, Exclude: []string{"lab:platform/**"}}, + errors: []string{`targets.yml: exclude[0]: unknown provider "lab"`}, + }, + { + name: "a malformed exclude pattern built in code", + targets: &Targets{Defaults: Defaults{Provider: "gh"}, Exclude: []string{"acme/[x]"}}, + errors: []string{`targets.yml: exclude[0]: "acme/[x]": character '['`}, + }, + { + name: "match patterns that select nothing", + targets: &Targets{Defaults: Defaults{Provider: "gh"}, Targets: []Entry{ + {Org: "acme", Match: []string{"acme/svc-*", "other/*"}}, + {Group: "corp:platform", Subgroups: boolPtr(false), Match: []string{"platform/*/api"}}, + }}, + warnings: []string{ + "targets[0].match[1]: other/* matches no repository under acme, so it selects nothing", + "targets[1].match[0]: platform/*/api matches no repository directly under platform (subgroups: false)", + }, + }, + { + name: "deeper than a platform without nested namespaces allows", + targets: &Targets{Defaults: Defaults{Provider: "gh"}, Targets: []Entry{ + {Org: "acme", Match: []string{"acme/a/b", "acme/svc-*", "acme/**/api", "other/x/y"}}, + }, Exclude: []string{"acme/legacy/tree/main", "acme/**/legacy", "acme/legacy-*", "corp:a/b/c/**"}}, + warnings: []string{ + "exclude[0]: acme/legacy/tree/main has 4 path segments, and a github repository path is owner/name, so it excludes nothing", + "targets[0].match[0]: acme/a/b matches no repository directly under acme (a github namespace has no subgroups)", + "targets[0].match[3]: other/x/y matches no repository under acme, so it selects nothing", + }, + }, + { + name: "opt_in: required under an assumed org", + targets: &Targets{Defaults: Defaults{Provider: "gh", OptIn: OptInAssumed}, Targets: []Entry{ + {Org: "acme", Match: []string{"acme/svc-*"}}, + {Repo: "acme/svc-billing", OptIn: OptInRequired}, + {Repo: "acme/web", OptIn: OptInRequired}, + {Repo: "corp:acme/svc-x", OptIn: OptInRequired}, + {Group: "corp:platform", OptIn: OptInRequired}, + {Repo: "corp:platform/api", OptIn: OptInRequired}, + }}, + warnings: []string{"targets[1]: opt_in: required does not hold acme/svc-billing back: targets[0] (org: acme) has opt_in: assumed and may select it"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + warns, errs := Check(hub, tc.targets, knownPacks()) + if len(errs) != len(tc.errors) { + t.Errorf("errors = %q, want %d", errs, len(tc.errors)) + } + for i := 0; i < len(errs) && i < len(tc.errors); i++ { + if !strings.Contains(errs[i].Error(), tc.errors[i]) { + t.Errorf("error %d = %q, want %q", i, errs[i], tc.errors[i]) + } + } + if len(warns) != len(tc.warnings) { + t.Errorf("warnings = %v, want %d", warns, len(tc.warnings)) + } + for i := 0; i < len(warns) && i < len(tc.warnings); i++ { + if !strings.Contains(warns[i].String(), tc.warnings[i]) { + t.Errorf("warning %d = %q, want %q", i, warns[i], tc.warnings[i]) + } + } + }) + } +} diff --git a/internal/config/providers.go b/internal/config/providers.go index f9a9b9a..105d3c2 100644 --- a/internal/config/providers.go +++ b/internal/config/providers.go @@ -195,6 +195,14 @@ func implicitProvider(h *Hub, env ciEnv, originHost string) (Provider, error) { return p, nil } +// OriginTellsProvider reports whether a hub without providers can take its +// implicit provider outside CI from an origin remote on host: only a public +// instance (github.com, a *.ghe.com host, gitlab.com) tells its type. +func OriginTellsProvider(host string) bool { + _, ok := originProvider(host) + return ok +} + // originProvider returns the provider of a public instance at host, the // host of a hub's origin remote: only there does the host tell the type. func originProvider(host string) (Provider, bool) { @@ -388,7 +396,9 @@ func SelectFor(hub *Hub, targets *Targets, optIn *OptIn, matched []int, known ma // // Patterns that normalize to "" match nothing and are left out; pack names // are taken as written. Legacy does not count: adding "version: 1" to a -// file without it changes nothing. For example, an empty file hashes +// file without it changes nothing. Nor does Enabled: a file that says +// enabled: false is never compared, and one that says true chooses +// nothing new. For example, an empty file hashes // "touchmark-optin/v1\nversion 1\n", and "packs: [claude]" appends // "pack 6:claude\n". func (o *OptIn) Hash() string { diff --git a/internal/config/schema_test.go b/internal/config/schema_test.go index 43f45b9..78d6c06 100644 --- a/internal/config/schema_test.go +++ b/internal/config/schema_test.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "regexp" + "strconv" "strings" "testing" @@ -348,14 +349,24 @@ func TestSchemaPatterns(t *testing.T) { "/$defs/pattern/pattern": patternPattern, }, "targets": { - "/$defs/packName/pattern": packNamePattern, - "/$defs/packName/maxLength": maxPackNameLen, - "/$defs/providerId/pattern": providerIDPattern, - "/$defs/ref/pattern": refPattern, - "/$defs/ref/maxLength": maxRefLen, - "/$defs/namespace/pattern": namespacePattern, - "/$defs/namespace/maxLength": maxRefLen, - "/$defs/topics/items/pattern": nonBlankPattern, + "/$defs/packName/pattern": packNamePattern, + "/$defs/packName/maxLength": maxPackNameLen, + "/$defs/providerId/pattern": providerIDPattern, + "/$defs/ref/pattern": refPattern, + "/$defs/ref/maxLength": maxRefLen, + "/$defs/repo/anyOf/0/pattern": refPattern, + "/$defs/repo/anyOf/1/pattern": repoURLPattern, + "/$defs/repo/maxLength": maxRefLen, + "/$defs/namespace/anyOf/0/pattern": namespacePattern, + "/$defs/namespace/anyOf/1/pattern": namespaceURLPattern, + "/$defs/namespace/maxLength": maxRefLen, + "/$defs/excludeEntry/anyOf/0/pattern": excludePattern, + "/$defs/excludeEntry/anyOf/1/pattern": excludeURLPattern, + "/$defs/excludeEntry/maxLength": maxRefLen, + "/$defs/match/items/pattern": matchPattern, + "/$defs/match/items/maxLength": maxRefLen, + "/$defs/optIn/enum": optInModes, + "/$defs/topics/items/pattern": nonBlankPattern, }, "opt-in": { "/properties/packs/items/pattern": packNamePattern, @@ -396,11 +407,19 @@ func TestSchemaPatterns(t *testing.T) { func lookup(doc any, ptr string) (any, bool) { for _, tok := range strings.Split(strings.TrimPrefix(ptr, "/"), "/") { - m, ok := doc.(map[string]any) - if !ok { - return nil, false - } - if doc, ok = m[tok]; !ok { + switch v := doc.(type) { + case map[string]any: + var ok bool + if doc, ok = v[tok]; !ok { + return nil, false + } + case []any: + i, err := strconv.Atoi(tok) + if err != nil || i < 0 || i >= len(v) { + return nil, false + } + doc = v[i] + default: return nil, false } } @@ -433,6 +452,53 @@ func TestRefPatternsMatchParser(t *testing.T) { } } +// TestTargetPatternsMatchParser checks that the schema's patterns of web +// URLs, exclude entries and match patterns accept exactly what targets.yml +// parsing accepts. +func TestTargetPatternsMatchParser(t *testing.T) { + urls := []string{ + "https://github.com/acme/billing", "https://github.com/acme/billing/", "https://github.com/acme/billing.git", + "https://gitlab.example.com:8443/platform/sub/api", "http://localhost:3000/acme/x", "http://127.0.0.1/acme/x", + "http://[::1]:3000/acme/x", "https://github.com/acme", "https://github.com", "https://github.com/", + "http://github.com/acme/x", "ftp://github.com/acme/x", "https://user:pass@github.com/acme/x", + "https://github.com/acme/x?tab=readme", "https://github.com/acme/x#readme", "https://github.com/acme/x?", + "https://github.com/acme//x", "https://github.com/acme/./x", "https://github.com/acme/../x", + "https://github.com/acme/my%20repo", "https://github.com/acme/my repo", "https://github.com:99999/acme/x", + "https://github.com/acme/*", "https://github.com/acme/legacy-*", "https://gitlab.example.com/platform/legacy/**", + "https://github.com/acme/x?y", "https://github.com/acme/[x]", "HTTPS://github.com/acme/x", + "https://github.com/acme/x//", "https:/github.com/acme/x", "https://gith_ub.com/acme/x", + "https://" + strings.Repeat("a", 480) + ".com/acme/x", + } + patterns := []string{ + "acme/legacy", "acme/legacy-*", "acme/*", "corp:platform/legacy/**", "**/sandbox", "acme/?ld", + "acme/**/x", "acme", "acme/*/", "/acme/*", "acme//x", "acme/./x", "acme/../*", "acme/[ab]", "acme/x y", + "gh:acme/*", "*:acme/x", "GH:acme/*", "acme/.*", "acme/..*", "acme/x:y", `acme\*`, + strings.Repeat("a", 256) + "/" + strings.Repeat("*", 256), + strings.Repeat("a", 255) + "/" + strings.Repeat("*", 256), + } + repoURLs, nsURLs, exURLs := mustCompile(t, repoURLPattern), mustCompile(t, namespaceURLPattern), mustCompile(t, excludeURLPattern) + excludeRe, matchRe := mustCompile(t, excludePattern), mustCompile(t, matchPattern) + fits := func(re *regexp.Regexp, s string) bool { return re.MatchString(s) && len(s) <= maxRefLen } + for _, s := range urls { + for _, c := range []struct { + k urlKind + re *regexp.Regexp + }{{urlRepo, repoURLs}, {urlNamespace, nsURLs}, {urlExclude, exURLs}} { + if got, want := checkTargetURL(s, c.k) == nil, fits(c.re, s); got != want { + t.Errorf("URL %q of kind %d: parser ok=%v, schema ok=%v", s, c.k, got, want) + } + } + } + for _, s := range append(patterns, urls...) { + if got, want := checkExclude(s) == nil, fits(excludeRe, s) || fits(exURLs, s); got != want { + t.Errorf("exclude %q: parser ok=%v, schema ok=%v (%v)", s, got, want, checkExclude(s)) + } + if got, want := checkMatchPattern(s) == nil, fits(matchRe, s); got != want { + t.Errorf("match %q: parser ok=%v, schema ok=%v (%v)", s, got, want, checkMatchPattern(s)) + } + } +} + func mustCompile(t *testing.T, pattern string) *regexp.Regexp { t.Helper() re, err := regexp.Compile(pattern) diff --git a/internal/config/select.go b/internal/config/select.go index a444aee..43a477b 100644 --- a/internal/config/select.go +++ b/internal/config/select.go @@ -47,9 +47,20 @@ func (r resolver) matchRepo(ref Ref, override string, target Ref) bool { return sameProvider(r.provider(firstNonEmpty(override, ref.Provider)), r.provider(target.Provider)) } +// matchPattern reports whether the exclude pattern pat (parsePattern) +// covers target: its path matches (glob.go) and the providers are +// compatible, as in matchRepo. +func (r resolver) matchPattern(pat Ref, target Ref) bool { + if !matchPath(pat.Path, target.Path) { + return false + } + return sameProvider(r.provider(pat.Provider), r.provider(target.Provider)) +} + // mayContain reports whether the org or group entry e, whose namespace is // ns, could select target: the target lies in the namespace (directly, or -// in a nested one when subgroups are included) on a compatible provider. +// in a nested one when subgroups are included) on a compatible provider, +// and its path passes the entry's match patterns. func (r resolver) mayContain(e *Entry, ns Ref, target Ref) bool { if !sameProvider(r.provider(firstNonEmpty(e.Provider, ns.Provider)), r.provider(target.Provider)) { return false @@ -59,6 +70,9 @@ func (r resolver) mayContain(e *Entry, ns Ref, target Ref) bool { if !strings.HasPrefix(path, prefix) { return false } + if len(e.Match) > 0 && !matchAnyPath(e.Match, target.Path) { + return false + } return includeSubgroups(e) || !strings.Contains(path[len(prefix):], "/") } @@ -221,11 +235,11 @@ func selectPacks(hub *Hub, targets *Targets, optIn *OptIn, target Ref, known map } r := resolver{hub: hub, targets: targets} for i, ex := range targets.Exclude { - ref, err := ParseRef(ex) + pat, err := parsePattern(ex) if err != nil { return Selection{}, nil, fmt.Errorf("%s: exclude[%d]: %w", TargetsFile, i, err) } - if r.matchRepo(ref, "", target) { + if r.matchPattern(pat, target) { w := Warning{File: TargetsFile, Message: fmt.Sprintf("%s is excluded by exclude[%d] (%s)", target, i, ex)} return Selection{Packs: []string{}, Complete: true, Sources: map[string][]string{}}, []Warning{w}, nil } @@ -269,6 +283,65 @@ func selectPacks(hub *Hub, targets *Targets, optIn *OptIn, target Ref, known map }, s.warns, nil } +// Assumption is what targets.yml says, as a local run can tell, about a +// target that has no opt-in file (Assume). +type Assumption struct { + // Assumed is set when a repo: entry that names the target has opt_in: + // assumed (its own, or defaults.opt_in) and no exclude entry covers the + // target: it counts as opted in, as if it had an empty opt-in file. + Assumed bool + // Unresolved lists the org and group entries with opt_in: assumed whose + // namespace may hold the target, as "org: acme": only the platform's API + // tells whether they select it. + Unresolved []string +} + +// Assume tells whether targets.yml subscribes target, which has no opt-in +// file, in local mode: a repo: entry that names it (as Select matches +// them) with opt_in: assumed makes it Assumed; an org or group entry with +// opt_in: assumed whose namespace and match patterns may hold it is listed +// in Unresolved, since a local run cannot resolve it. An excluded target is +// neither. Nil hub and targets are a legacy hub and an empty targets.yml. +func Assume(hub *Hub, targets *Targets, target Ref) (Assumption, error) { + if hub == nil { + hub = &Hub{Legacy: true} + } + var out Assumption + if targets == nil { + return out, nil + } + r := resolver{hub: hub, targets: targets} + for i, ex := range targets.Exclude { + pat, err := parsePattern(ex) + if err != nil { + return out, fmt.Errorf("%s: exclude[%d]: %w", TargetsFile, i, err) + } + if r.matchPattern(pat, target) { + return out, nil + } + } + for i := range targets.Targets { + e := &targets.Targets[i] + if targets.effectiveOptIn(e) != OptInAssumed { + continue + } + ref, err := entryRef(e) + if err != nil { + return Assumption{}, fmt.Errorf("%s: targets[%d]: %w", TargetsFile, i, err) + } + switch kind := entryKind(e); { + case kind == "repo": + out.Assumed = out.Assumed || r.matchRepo(ref, e.Provider, target) + case r.mayContain(e, ref, target): + out.Unresolved = append(out.Unresolved, kind+": "+selectorValue(e)) + } + } + if out.Assumed { + out.Unresolved = nil + } + return out, nil +} + func selectorValue(e *Entry) string { return firstNonEmpty(e.Repo, firstNonEmpty(e.Org, e.Group)) } diff --git a/internal/config/selectors.go b/internal/config/selectors.go index 62b4fb9..a80cce6 100644 --- a/internal/config/selectors.go +++ b/internal/config/selectors.go @@ -3,6 +3,7 @@ package config import ( "fmt" "slices" + "strings" ) // Selector is one targets.yml entry as delivery resolves it through a @@ -27,6 +28,22 @@ type Selector struct { // Topics must all be present on a repository; Forks includes forks. Topics []string Forks bool + // Match are the glob patterns of an org or group entry: a repository + // of the namespace is selected when its full path matches one of them + // (Selects); none selects every repository. + Match []string + // Assumed is set when the entry's opt_in is assumed (its own, or + // defaults.opt_in): the targets it selects count as opted in without an + // opt-in file. + Assumed bool +} + +// Selects reports whether the repository at path, which the platform +// listed for an org or group selector, passes its match patterns: it +// matches one of them, ignoring case, or there are none. A repo selector +// selects its repository. +func (s Selector) Selects(path string) bool { + return len(s.Match) == 0 || matchAnyPath(s.Match, path) } // Selectors returns the entries of targets.yml as selectors, in file order. @@ -51,7 +68,8 @@ func Selectors(hub *Hub, targets *Targets) ([]Selector, error) { if err != nil { return nil, fmt.Errorf("%s: targets[%d]: %w", TargetsFile, i, err) } - s := Selector{Entry: i, Provider: r.provider(firstNonEmpty(e.Provider, ref.Provider))} + s := Selector{Entry: i, Provider: r.provider(firstNonEmpty(e.Provider, ref.Provider)), + Assumed: targets.effectiveOptIn(e) == OptInAssumed} if entryKind(e) == "repo" { s.Repo = ref.Path } else { @@ -59,6 +77,7 @@ func Selectors(hub *Hub, targets *Targets) ([]Selector, error) { s.Subgroups = includeSubgroups(e) s.Topics = slices.Clone(e.Topics) s.Forks = e.Forks + s.Match = slices.Clone(e.Match) } out = append(out, s) } @@ -84,18 +103,51 @@ func Names(hub *Hub, targets *Targets, ref Ref, provider, path string) bool { } // Excluded returns the index of the first exclude entry of targets.yml -// that names the repository at path on provider (see Names); ok is false -// when none does. Entries that do not parse are skipped: Check reports -// them. +// that covers the repository at path on provider; ok is false when none +// does. An entry is a pattern (glob.go) whose provider is resolved as in +// Names: an entry without glob characters names one repository, as Names +// matches it. Entries that do not parse, and URLs ResolveURLs did not +// rewrite, are skipped: Check reports them. func Excluded(hub *Hub, targets *Targets, provider, path string) (index int, ok bool) { if targets == nil { return 0, false } + if hub == nil { + hub = &Hub{Legacy: true} + } + r := resolver{hub: hub, targets: targets} for i, ex := range targets.Exclude { - ref, err := ParseRef(ex) - if err == nil && Names(hub, targets, ref, provider, path) { + pat, err := parsePattern(ex) + if err == nil && r.matchPattern(pat, Ref{Provider: provider, Path: path}) { return i, true } } return 0, false } + +// ExcludedBeneath returns the indexes of the exclude entries of targets.yml +// that name one repository (no glob characters) at a path the repository at +// path on provider lies beneath, ignoring case, on a compatible provider +// (resolved as in Excluded). Such an entry excludes a repository at its +// own path only, never what lies beneath it: written for a namespace, as a +// browser shows group and project URLs alike, it wants "/**". +func ExcludedBeneath(hub *Hub, targets *Targets, provider, path string) []int { + if targets == nil { + return nil + } + if hub == nil { + hub = &Hub{Legacy: true} + } + r := resolver{hub: hub, targets: targets} + var out []int + for i, ex := range targets.Exclude { + pat, err := parsePattern(ex) + if err != nil || isGlob(pat.Path) || len(path) <= len(pat.Path) || path[len(pat.Path)] != '/' { + continue + } + if strings.EqualFold(path[:len(pat.Path)], pat.Path) && sameProvider(r.provider(pat.Provider), r.provider(provider)) { + out = append(out, i) + } + } + return out +} diff --git a/internal/config/targets.go b/internal/config/targets.go index 5ee673b..44f0689 100644 --- a/internal/config/targets.go +++ b/internal/config/targets.go @@ -69,16 +69,30 @@ func validateTargets(t *Targets, doc document, p *problems) { } } checkPackNames(p, "defaults.packs", t.Defaults.Packs) + checkEnum(p, doc, "defaults.opt_in", t.Defaults.OptIn, optInModes) for i := range t.Targets { checkEntry(&t.Targets[i], fmt.Sprintf("targets[%d]", i), doc, p) } for i, ex := range t.Exclude { - if _, err := ParseRef(ex); err != nil { + if err := checkExclude(ex); err != nil { p.errorf(fmt.Sprintf("exclude[%d]", i), "%v", err) } } } +// optInModes are the values of opt_in. +var optInModes = []string{OptInRequired, OptInAssumed} + +// checkExclude validates an exclude entry: a reference or a glob pattern, +// [:], or a web URL of either. +func checkExclude(ex string) error { + if isURL(ex) { + return checkTargetURL(ex, urlExclude) + } + _, err := parsePattern(ex) + return err +} + func checkEntry(e *Entry, field string, doc document, p *problems) { var kinds []string for _, k := range []string{"repo", "org", "group"} { @@ -92,6 +106,7 @@ func checkEntry(e *Entry, field string, doc document, p *problems) { } } checkPackNames(p, field+".packs", e.Packs) + checkEnum(p, doc, field+".opt_in", e.OptIn, optInModes) switch len(kinds) { case 0: p.errorf(field, "needs one of repo, org or group") @@ -102,14 +117,21 @@ func checkEntry(e *Entry, field string, doc document, p *problems) { return } kind := kinds[0] - ref, err := selectorRef(e, kind) - if err != nil { + if v := selectorValue(e); isURL(v) { + k := urlNamespace + if kind == "repo" { + k = urlRepo + } + if err := checkTargetURL(v, k); err != nil { + p.errorf(field+"."+kind, "%v", err) + } + } else if ref, err := selectorRef(e, kind); err != nil { p.errorf(field+"."+kind, "%v", err) } else if ref.Provider != "" && e.Provider != "" && ref.Provider != e.Provider { p.errorf(field, "provider %q contradicts the %q prefix of %s", e.Provider, ref.Provider, kind) } if kind == "repo" { - for _, k := range []string{"topics", "subgroups", "forks"} { + for _, k := range []string{"topics", "subgroups", "forks", "match"} { if doc.has(field + "." + k) { p.errorf(field+"."+k, "only with org or group") } @@ -121,12 +143,51 @@ func checkEntry(e *Entry, field string, doc document, p *problems) { p.errorf(fmt.Sprintf("%s.topics[%d]", field, i), "must not be blank") } } + if e.Match != nil && len(e.Match) == 0 { + p.errorf(field+".match", "must not be empty; leave match out to select every repository of the %s", kind) + } + for i, pat := range e.Match { + if err := checkMatchPattern(pat); err != nil { + p.errorf(fmt.Sprintf("%s.match[%d]", field, i), "%v", err) + } + } } // entryRef parses the selector of e: a repository for repo, a namespace for -// org and group. +// org and group. A URL is an error: ResolveURLs rewrites it first. func entryRef(e *Entry) (Ref, error) { return selectorRef(e, entryKind(e)) } +// effectiveOptIn returns the opt_in of entry e: its own, else +// defaults.opt_in, else OptInRequired. +func (t *Targets) effectiveOptIn(e *Entry) string { + switch { + case e.OptIn != "": + return e.OptIn + case t.Defaults.OptIn != "": + return t.Defaults.OptIn + } + return OptInRequired +} + +// Assumed reports whether targets.yml subscribes a target that the entries +// at indexes select (Selector.Entry; indexes outside Targets are left +// out): one of them has opt_in: assumed, its own or through +// defaults.opt_in. Any one is enough, whatever the others say and whatever +// their kind: entries add up, as their packs do, and a repository a hub +// subscribes through its organisation stays subscribed when another entry +// names it to give it more packs. A nil targets assumes nothing. +func (t *Targets) Assumed(indexes []int) bool { + if t == nil { + return false + } + for _, i := range indexes { + if i >= 0 && i < len(t.Targets) && t.effectiveOptIn(&t.Targets[i]) == OptInAssumed { + return true + } + } + return false +} + func selectorRef(e *Entry, kind string) (Ref, error) { switch kind { case "repo": diff --git a/internal/config/testdata/opt-in/invalid/enabled-string.yml b/internal/config/testdata/opt-in/invalid/enabled-string.yml new file mode 100644 index 0000000..ffb6145 --- /dev/null +++ b/internal/config/testdata/opt-in/invalid/enabled-string.yml @@ -0,0 +1,2 @@ +# error: enabled: expected true or false, got a string +enabled: "no" diff --git a/internal/config/testdata/opt-in/valid/disabled.yml b/internal/config/testdata/opt-in/valid/disabled.yml new file mode 100644 index 0000000..2523813 --- /dev/null +++ b/internal/config/testdata/opt-in/valid/disabled.yml @@ -0,0 +1,4 @@ +# The repository opts out: enabled: false, its packs kept for later. +version: 1 +enabled: false +packs: [claude] diff --git a/internal/config/testdata/opt-in/valid/enabled-null.yml b/internal/config/testdata/opt-in/valid/enabled-null.yml new file mode 100644 index 0000000..87f7fae --- /dev/null +++ b/internal/config/testdata/opt-in/valid/enabled-null.yml @@ -0,0 +1,3 @@ +# A null enabled is absent: the repository stays opted in. +version: 1 +enabled: diff --git a/internal/config/testdata/targets/invalid/defaults-opt-in-bad.yml b/internal/config/testdata/targets/invalid/defaults-opt-in-bad.yml new file mode 100644 index 0000000..965fec2 --- /dev/null +++ b/internal/config/testdata/targets/invalid/defaults-opt-in-bad.yml @@ -0,0 +1,4 @@ +# error: defaults.opt_in: "Assumed" must be one of required, assumed +version: 1 +defaults: + opt_in: Assumed diff --git a/internal/config/testdata/targets/invalid/exclude-bracket.yml b/internal/config/testdata/targets/invalid/exclude-bracket.yml new file mode 100644 index 0000000..507edd2 --- /dev/null +++ b/internal/config/testdata/targets/invalid/exclude-bracket.yml @@ -0,0 +1,4 @@ +# error: exclude[0]: "acme/[ab]": character '[' is not allowed in a repository path +version: 1 +exclude: + - acme/[ab] diff --git a/internal/config/testdata/targets/invalid/exclude-glob-one-segment.yml b/internal/config/testdata/targets/invalid/exclude-glob-one-segment.yml new file mode 100644 index 0000000..63a03ab --- /dev/null +++ b/internal/config/testdata/targets/invalid/exclude-glob-one-segment.yml @@ -0,0 +1,4 @@ +# error: exclude[0]: "acme*": a pattern covers a whole repository path +version: 1 +exclude: + - acme* diff --git a/internal/config/testdata/targets/invalid/group-glob.yml b/internal/config/testdata/targets/invalid/group-glob.yml new file mode 100644 index 0000000..0bb9581 --- /dev/null +++ b/internal/config/testdata/targets/invalid/group-glob.yml @@ -0,0 +1,4 @@ +# error: glob patterns go in exclude and match +version: 1 +targets: + - group: platform/? diff --git a/internal/config/testdata/targets/invalid/match-empty.yml b/internal/config/testdata/targets/invalid/match-empty.yml new file mode 100644 index 0000000..841a5f4 --- /dev/null +++ b/internal/config/testdata/targets/invalid/match-empty.yml @@ -0,0 +1,5 @@ +# error: targets[0].match: must not be empty +version: 1 +targets: + - org: acme + match: [] diff --git a/internal/config/testdata/targets/invalid/match-null-item.yml b/internal/config/testdata/targets/invalid/match-null-item.yml new file mode 100644 index 0000000..4a22282 --- /dev/null +++ b/internal/config/testdata/targets/invalid/match-null-item.yml @@ -0,0 +1,5 @@ +# error: targets[0].match[0]: expected a string, got null +version: 1 +targets: + - org: acme + match: [null] diff --git a/internal/config/testdata/targets/invalid/match-on-repo.yml b/internal/config/testdata/targets/invalid/match-on-repo.yml new file mode 100644 index 0000000..ceee8ce --- /dev/null +++ b/internal/config/testdata/targets/invalid/match-on-repo.yml @@ -0,0 +1,5 @@ +# error: targets[0].match: only with org or group +version: 1 +targets: + - repo: acme/billing + match: [acme/*] diff --git a/internal/config/testdata/targets/invalid/match-one-segment.yml b/internal/config/testdata/targets/invalid/match-one-segment.yml new file mode 100644 index 0000000..8e9f5a0 --- /dev/null +++ b/internal/config/testdata/targets/invalid/match-one-segment.yml @@ -0,0 +1,5 @@ +# error: targets[0].match[0]: "svc-*": a pattern covers a whole repository path +version: 1 +targets: + - org: acme + match: [svc-*] diff --git a/internal/config/testdata/targets/invalid/match-provider.yml b/internal/config/testdata/targets/invalid/match-provider.yml new file mode 100644 index 0000000..89bb1ae --- /dev/null +++ b/internal/config/testdata/targets/invalid/match-provider.yml @@ -0,0 +1,5 @@ +# error: targets[0].match[0]: "gh:acme/*": match takes repository paths without a provider +version: 1 +targets: + - org: gh:acme + match: [gh:acme/*] diff --git a/internal/config/testdata/targets/invalid/match-url.yml b/internal/config/testdata/targets/invalid/match-url.yml new file mode 100644 index 0000000..14ccb9a --- /dev/null +++ b/internal/config/testdata/targets/invalid/match-url.yml @@ -0,0 +1,5 @@ +# error: targets[0].match[0]: "https://github.com/acme/*": a URL is not accepted here +version: 1 +targets: + - org: acme + match: [https://github.com/acme/*] diff --git a/internal/config/testdata/targets/invalid/opt-in-bad.yml b/internal/config/testdata/targets/invalid/opt-in-bad.yml new file mode 100644 index 0000000..daccfa8 --- /dev/null +++ b/internal/config/testdata/targets/invalid/opt-in-bad.yml @@ -0,0 +1,5 @@ +# error: targets[0].opt_in: "optional" must be one of required, assumed +version: 1 +targets: + - repo: acme/billing + opt_in: optional diff --git a/internal/config/testdata/targets/invalid/opt-in-null.yml b/internal/config/testdata/targets/invalid/opt-in-null.yml new file mode 100644 index 0000000..a0d284e --- /dev/null +++ b/internal/config/testdata/targets/invalid/opt-in-null.yml @@ -0,0 +1,5 @@ +# error: targets[0].opt_in: expected a string, got null +version: 1 +targets: + - repo: acme/billing + opt_in: diff --git a/internal/config/testdata/targets/invalid/repo-glob.yml b/internal/config/testdata/targets/invalid/repo-glob.yml new file mode 100644 index 0000000..e0a29f3 --- /dev/null +++ b/internal/config/testdata/targets/invalid/repo-glob.yml @@ -0,0 +1,4 @@ +# error: targets[0].repo: "acme/*": character '*' is not allowed in a repository path; glob patterns go in exclude and match +version: 1 +targets: + - repo: acme/* diff --git a/internal/config/testdata/targets/invalid/url-credentials.yml b/internal/config/testdata/targets/invalid/url-credentials.yml new file mode 100644 index 0000000..81f1d0e --- /dev/null +++ b/internal/config/testdata/targets/invalid/url-credentials.yml @@ -0,0 +1,4 @@ +# error: targets[0].repo: "https://…@github.com/acme/x" is not a repository URL +version: 1 +targets: + - repo: https://jdoe:hunter2@github.com/acme/x diff --git a/internal/config/testdata/targets/invalid/url-exclude-question.yml b/internal/config/testdata/targets/invalid/url-exclude-question.yml new file mode 100644 index 0000000..d91a2eb --- /dev/null +++ b/internal/config/testdata/targets/invalid/url-exclude-question.yml @@ -0,0 +1,4 @@ +# error: exclude[0]: "https://github.com/acme/x?" is not a repository URL like https://github.com/acme/legacy or a pattern +version: 1 +exclude: + - https://github.com/acme/x? diff --git a/internal/config/testdata/targets/invalid/url-fragment.yml b/internal/config/testdata/targets/invalid/url-fragment.yml new file mode 100644 index 0000000..05a12a0 --- /dev/null +++ b/internal/config/testdata/targets/invalid/url-fragment.yml @@ -0,0 +1,4 @@ +# error: is not a repository URL +version: 1 +targets: + - repo: "https://github.com/acme/x#readme" diff --git a/internal/config/testdata/targets/invalid/url-ftp.yml b/internal/config/testdata/targets/invalid/url-ftp.yml new file mode 100644 index 0000000..63f1426 --- /dev/null +++ b/internal/config/testdata/targets/invalid/url-ftp.yml @@ -0,0 +1,4 @@ +# error: targets[0].org: "ftp://github.com/acme" is not an organisation or group URL +version: 1 +targets: + - org: ftp://github.com/acme diff --git a/internal/config/testdata/targets/invalid/url-http.yml b/internal/config/testdata/targets/invalid/url-http.yml new file mode 100644 index 0000000..03d227a --- /dev/null +++ b/internal/config/testdata/targets/invalid/url-http.yml @@ -0,0 +1,4 @@ +# error: targets[0].repo: "http://github.com/acme/x" is not a repository URL +version: 1 +targets: + - repo: http://github.com/acme/x diff --git a/internal/config/testdata/targets/invalid/url-no-path.yml b/internal/config/testdata/targets/invalid/url-no-path.yml new file mode 100644 index 0000000..36e3e8d --- /dev/null +++ b/internal/config/testdata/targets/invalid/url-no-path.yml @@ -0,0 +1,4 @@ +# error: targets[0].org: "https://github.com/" is not an organisation or group URL +version: 1 +targets: + - org: https://github.com/ diff --git a/internal/config/testdata/targets/invalid/url-org-glob.yml b/internal/config/testdata/targets/invalid/url-org-glob.yml new file mode 100644 index 0000000..b0b5795 --- /dev/null +++ b/internal/config/testdata/targets/invalid/url-org-glob.yml @@ -0,0 +1,4 @@ +# error: targets[0].org: "https://github.com/acme-*" is not an organisation or group URL +version: 1 +targets: + - org: https://github.com/acme-* diff --git a/internal/config/testdata/targets/invalid/url-query.yml b/internal/config/testdata/targets/invalid/url-query.yml new file mode 100644 index 0000000..6985131 --- /dev/null +++ b/internal/config/testdata/targets/invalid/url-query.yml @@ -0,0 +1,4 @@ +# error: targets[0].repo: "https://github.com/acme/x?tab=readme" is not a repository URL +version: 1 +targets: + - repo: https://github.com/acme/x?tab=readme diff --git a/internal/config/testdata/targets/valid/match-null.yml b/internal/config/testdata/targets/valid/match-null.yml new file mode 100644 index 0000000..e89cff5 --- /dev/null +++ b/internal/config/testdata/targets/valid/match-null.yml @@ -0,0 +1,5 @@ +# A null match selects every repository, as an absent one does. +version: 1 +targets: + - org: acme + match: diff --git a/internal/config/testdata/targets/valid/patterns.yml b/internal/config/testdata/targets/valid/patterns.yml new file mode 100644 index 0000000..dd72d39 --- /dev/null +++ b/internal/config/testdata/targets/valid/patterns.yml @@ -0,0 +1,17 @@ +# Glob patterns in exclude and match, and opt_in in defaults and entries. +version: 1 +defaults: + opt_in: assumed +targets: + - org: acme + match: [acme/svc-*, "acme/api-??"] + opt_in: required + - group: corp:platform + match: + - platform/**/api + - repo: acme/billing + opt_in: assumed +exclude: + - acme/legacy-* + - corp:platform/legacy/** + - "**/sandbox" diff --git a/internal/config/testdata/targets/valid/urls.yml b/internal/config/testdata/targets/valid/urls.yml new file mode 100644 index 0000000..38c993b --- /dev/null +++ b/internal/config/testdata/targets/valid/urls.yml @@ -0,0 +1,14 @@ +# Web URLs of repositories and namespaces, with a trailing slash or .git; +# ResolveURLs finds their providers later. +version: 1 +targets: + - repo: https://github.com/acme/billing + - repo: https://gitlab.example.com/platform/api.git + packs: [base] + - repo: http://localhost:3000/acme/tools/ + - org: https://github.com/acme + - provider: corp + group: https://gitlab.example.com:8443/gitlab/platform/ +exclude: + - https://github.com/acme/legacy + - https://gitlab.example.com/platform/legacy/** diff --git a/internal/config/urls.go b/internal/config/urls.go new file mode 100644 index 0000000..9a9804f --- /dev/null +++ b/internal/config/urls.go @@ -0,0 +1,331 @@ +package config + +import ( + "errors" + "fmt" + "net/url" + "slices" + "strconv" + "strings" +) + +// A value of targets[].repo, org or group, or of exclude, may be the web URL +// of a repository or a namespace, as a browser shows it: +// https://gitlab.example.com/platform/api, with or without a trailing +// slash or .git. ParseTargets checks its form; ResolveURLs finds its +// provider among the hub's by the provider's url and rewrites it as +// :, the form every other part of touchmark reads. + +// isURL reports whether a targets.yml value is a web URL rather than a +// [:] reference. +func isURL(s string) bool { return strings.Contains(s, "://") } + +// urlKind is what a URL in targets.yml names. +type urlKind int + +const ( + urlRepo urlKind = iota // targets[].repo: one repository + urlNamespace // targets[].org and group: a namespace + urlExclude // exclude: a repository or a pattern +) + +// checkTargetURL checks the form of a URL of kind k: https (http only for +// localhost), no credentials, query or fragment, path segments a repository +// path may hold (and, in exclude, the glob character *). +func checkTargetURL(s string, k urlKind) error { + if len(s) > maxRefLen { + return fmt.Errorf("%.40q…: longer than %d bytes", displayURL(s), maxRefLen) + } + re, like, chars := repoURLRe, "a repository URL like https://github.com/acme/billing", "'.', '-' and '_'" + switch k { + case urlNamespace: + re, like = namespaceURLRe, "an organisation or group URL like https://gitlab.example.com/platform" + case urlExclude: + re, like = excludeURLRe, "a repository URL like https://github.com/acme/legacy or a pattern like https://gitlab.example.com/platform/legacy/**" + chars = "'.', '-', '_' and the glob character *" + } + if re.MatchString(s) { + return nil + } + return fmt.Errorf("%q is not %s: https:// (http:// only for localhost), without credentials, query or fragment, "+ + "and a path of letters, digits, %s", displayURL(s), like, chars) +} + +// displayURL returns s with the user information of a URL in it (user:pass@) +// cut, so that a message never shows a credential written in targets.yml. +func displayURL(s string) string { + scheme, rest, ok := strings.Cut(s, "://") + if !ok { + return s + } + authority := rest + if i := strings.IndexAny(rest, "/?#"); i >= 0 { + authority = rest[:i] + } + at := strings.LastIndex(authority, "@") + if at < 0 { + return s + } + return scheme + "://…@" + rest[at+1:] +} + +// site is where a URL points: its scheme and host (with a port other than +// the scheme's default) and the segments of its path. +type site struct { + origin string + segs []string +} + +// siteOf splits a URL that passed checkURL or checkTargetURL. +func siteOf(raw string) (site, bool) { + u, err := url.Parse(raw) + if err != nil || u.Hostname() == "" { + return site{}, false + } + scheme := strings.ToLower(u.Scheme) + host := strings.ToLower(u.Hostname()) + if strings.Contains(host, ":") { + host = "[" + host + "]" + } + if p := u.Port(); p != "" { + n, err := strconv.Atoi(p) + if err != nil { + return site{}, false + } + defaultPort := (scheme == "https" && n == 443) || (scheme == "http" && n == 80) + if !defaultPort { + host += ":" + strconv.Itoa(n) + } + } + var segs []string + for _, seg := range strings.Split(u.Path, "/") { + if seg != "" { + segs = append(segs, seg) + } + } + return site{origin: scheme + "://" + host, segs: segs}, true +} + +// under reports whether s lies strictly beneath base: the same origin, and +// base's path segments, compared ignoring case, followed by at least one +// more. +func (s site) under(base site) bool { + if s.origin != base.origin || len(s.segs) <= len(base.segs) { + return false + } + for i, seg := range base.segs { + if !strings.EqualFold(seg, s.segs[i]) { + return false + } + } + return true +} + +// HasURLs reports whether a value of targets[].repo, org or group, or of +// exclude, is a web URL, which ResolveURLs resolves. A nil targets has +// none. +func (t *Targets) HasURLs() bool { + if t == nil { + return false + } + for i := range t.Targets { + if isURL(selectorValue(&t.Targets[i])) { + return true + } + } + return slices.ContainsFunc(t.Exclude, isURL) +} + +// ResolveURLs returns targets with every web URL among the values of +// targets[].repo, org and group and of exclude replaced by +// :; the rest of touchmark reads that form only, so reports, +// --only and operations.yml keep naming targets :. +// +// A URL's provider is the one of providers whose url (scheme, host, port and +// path) the URL lies beneath; the rest of the URL's path, without a +// trailing slash and, for a repository or an exclude entry, without a +// trailing .git, is the target's path. Exactly one provider must match: +// none, or several (providers that share a url), is an error, unless the +// entry names its provider with provider:, which then must be one of them. +// An implicit provider (the CI's own platform, for a hub.yml without +// providers) is the hub's only one: its targets are written as bare paths. +// On GitHub, Gitea and Forgejo, which have no nested namespaces, a +// repository URL must name owner/name and an organisation URL one +// namespace; on GitLab a URL with a /-/ segment points inside a project and +// is refused. +// +// The result is a copy: targets is left as it is, and returned as it is +// when it holds no URL. Errors name the field of each URL that did not +// resolve, joined, safe to print. A nil targets is returned as it is. +func ResolveURLs(targets *Targets, providers []ResolvedProvider) (*Targets, error) { + if !targets.HasURLs() { + return targets, nil + } + out := *targets + out.Targets = slices.Clone(targets.Targets) + out.Exclude = slices.Clone(targets.Exclude) + sites := make([]site, len(providers)) + for i, p := range providers { + sites[i], _ = siteOf(p.URL) + } + r := urlResolver{providers: providers, sites: sites} + var errs []error + for i := range out.Targets { + e := &out.Targets[i] + kind := entryKind(e) + v := selectorValue(e) + if !isURL(v) { + continue + } + k := urlNamespace + if kind == "repo" { + k = urlRepo + } + ref, err := r.resolve(v, k, e.Provider, out.Defaults.Provider) + if err != nil { + errs = append(errs, fmt.Errorf("%s: targets[%d].%s: %w", TargetsFile, i, kind, err)) + continue + } + switch kind { + case "repo": + e.Repo = ref + case "org": + e.Org = ref + default: + e.Group = ref + } + } + for i, ex := range out.Exclude { + if !isURL(ex) { + continue + } + ref, err := r.resolve(ex, urlExclude, "", out.Defaults.Provider) + if err != nil { + errs = append(errs, fmt.Errorf("%s: exclude[%d]: %w", TargetsFile, i, err)) + continue + } + out.Exclude[i] = ref + } + if len(errs) > 0 { + return nil, cleanErr(errors.Join(errs...)) + } + return &out, nil +} + +// urlResolver finds the provider of a URL. +type urlResolver struct { + providers []ResolvedProvider + sites []site +} + +// resolve returns the URL s of kind k as [:]; want is the +// entry's provider field ("" for none), and fallback defaults.provider, +// which picks among several matching providers when want is "" and it is +// one of them. +func (r urlResolver) resolve(s string, k urlKind, want, fallback string) (string, error) { + if err := checkTargetURL(s, k); err != nil { + return "", err + } + u, ok := siteOf(s) + if !ok { + return "", fmt.Errorf("%q is not a URL touchmark can read", displayURL(s)) + } + var match []int + for i, ps := range r.sites { + if ps.origin != "" && u.under(ps) { + match = append(match, i) + } + } + if want != "" { + i := slices.IndexFunc(r.providers, func(p ResolvedProvider) bool { return p.ID == want }) + if i >= 0 && !slices.Contains(match, i) { + return "", fmt.Errorf("%s is not under the url of provider %s (%s)", s, want, r.providers[i].URL) + } + if i >= 0 { + match = []int{i} + } + } + if want == "" && fallback != "" && len(match) > 1 { + i := slices.IndexFunc(r.providers, func(p ResolvedProvider) bool { return p.ID == fallback }) + if slices.Contains(match, i) { + match = []int{i} + } + } + switch { + case len(match) == 0: + return "", fmt.Errorf("%s is not under the url of any provider of %s (%s); add the provider there, or write the target as :", + s, HubFile, r.list(nil)) + case len(match) > 1: + return "", fmt.Errorf("%s is under the url of providers %s; name one with provider: or defaults.provider, or write the target as :", + s, r.list(match)) + } + p := r.providers[match[0]] + segs := slices.Clone(u.segs[len(r.sites[match[0]].segs):]) + if last := len(segs) - 1; k != urlNamespace && len(segs[last]) > len(".git") && strings.HasSuffix(strings.ToLower(segs[last]), ".git") { + // A repository URL may end in .git; dropping it from a pattern + // would widen it (acme/*.git to acme/*). + if isGlob(segs[last]) { + return "", fmt.Errorf("%s: a pattern whose last segment ends in .git; write it without .git", s) + } + segs[last] = segs[last][:len(segs[last])-len(".git")] + } + path := strings.Join(segs, "/") + min := 2 + if k == urlNamespace { + min = 1 + } + if err := checkRefPath(path, min, k == urlExclude); err != nil { + return "", fmt.Errorf("%s: %w", s, err) + } + if err := checkURLShape(p.Type, segs, k); err != nil { + return "", fmt.Errorf("%s: %w", s, err) + } + if p.Implicit { + return path, nil + } + return p.ID + ":" + path, nil +} + +// list names the providers at indexes (every provider when nil) with their +// urls. +func (r urlResolver) list(indexes []int) string { + if indexes == nil { + for i := range r.providers { + indexes = append(indexes, i) + } + } + if len(indexes) == 0 { + return "it has none" + } + parts := make([]string, 0, len(indexes)) + for _, i := range indexes { + parts = append(parts, r.providers[i].ID+" at "+r.providers[i].URL) + } + return strings.Join(parts, ", ") +} + +// checkURLShape checks the path segments a URL of kind k names on a +// platform of type typ: GitHub, Gitea and Forgejo have no nested +// namespaces, so an exclude URL of more than owner/name, without a "**" +// that could span nothing, points inside a repository (/tree/main) and +// would exclude nothing; a GitLab URL with a /-/ segment points inside a +// project (a file, a merge request), not at it. +func checkURLShape(typ string, segs []string, k urlKind) error { + switch typ { + case "github", "gitea", "forgejo": + switch { + case k == urlRepo && len(segs) != 2: + return fmt.Errorf("a %s repository URL names owner/name, like https://example.com/acme/billing; this one has %d path segments after the provider's url", typ, len(segs)) + case k == urlExclude && len(segs) > 2 && !slices.Contains(segs, "**"): + return fmt.Errorf("a %s repository URL names owner/name, like https://example.com/acme/legacy; this one has %d path segments after the provider's url, "+ + "so it points inside a repository and would exclude nothing", typ, len(segs)) + case k == urlNamespace && len(segs) != 1: + return fmt.Errorf("a %s organisation URL names one owner, like https://example.com/acme; this one has %d path segments after the provider's url", typ, len(segs)) + } + case "gitlab": + if slices.Contains(segs, "-") { + return errors.New("the /-/ segment points inside a GitLab project; use the URL of the project or group itself") + } + } + return nil +} diff --git a/internal/config/urls_test.go b/internal/config/urls_test.go new file mode 100644 index 0000000..ef500d6 --- /dev/null +++ b/internal/config/urls_test.go @@ -0,0 +1,339 @@ +package config + +import ( + "reflect" + "strings" + "testing" +) + +// urlHub declares providers at the root of a host, under a path, on a +// port, and on the loopback host over http. +const urlHub = `version: 1 +id: acme-eng +providers: + - id: gh + type: github + - id: corp + type: gitlab + url: https://gitlab.example.com + - id: sub + type: gitlab + url: https://example.com/GitLab/ + - id: tea + type: gitea + url: http://localhost:3000 + - id: ghe + type: github + url: https://ghe.example.com:8443 +` + +// resolvedProviders parses hub.yml and resolves its providers without an +// environment. +func resolvedProviders(t *testing.T, yml string) []ResolvedProvider { + t.Helper() + hub := mustParseHub(t, []byte(yml)) + rps, err := hub.ResolveProviders(func(string) string { return "" }) + if err != nil { + t.Fatal(err) + } + return rps +} + +func TestResolveURLs(t *testing.T) { + provs := resolvedProviders(t, urlHub) + for _, tc := range []struct { + entry Entry + want string // the resolved value, or a substring of the error after "!" + }{ + {Entry{Repo: "https://github.com/acme/billing"}, "gh:acme/billing"}, + {Entry{Repo: "https://GitHub.com:443/Acme/Billing.git/"}, "gh:Acme/Billing"}, + {Entry{Repo: "https://github.com/acme/billing.git.git"}, "gh:acme/billing.git"}, + {Entry{Org: "https://github.com/acme/"}, "gh:acme"}, + {Entry{Repo: "https://gitlab.example.com/platform/sub/api"}, "corp:platform/sub/api"}, + {Entry{Group: "https://gitlab.example.com/platform/sub/"}, "corp:platform/sub"}, + {Entry{Group: "https://gitlab.example.com/x.git"}, "corp:x.git"}, + {Entry{Repo: "https://example.com/gitlab/team/app"}, "sub:team/app"}, + {Entry{Repo: "http://localhost:3000/acme/tools"}, "tea:acme/tools"}, + {Entry{Repo: "https://ghe.example.com:8443/acme/x"}, "ghe:acme/x"}, + {Entry{Repo: "https://github.com/acme/x", Provider: "gh"}, "gh:acme/x"}, + // No provider there: another host, port, scheme or path. + {Entry{Repo: "https://bitbucket.org/acme/x"}, "!is not under the url of any provider of hub.yml (gh at https://github.com, corp at https://gitlab.example.com"}, + {Entry{Repo: "https://ghe.example.com/acme/x"}, "!is not under the url of any provider"}, + {Entry{Repo: "https://localhost:3000/acme/x"}, "!is not under the url of any provider"}, + {Entry{Repo: "https://example.com/gitlabx/team/app"}, "!is not under the url of any provider"}, + // The provider's own URL names no repository. + {Entry{Org: "https://example.com/gitlab/"}, "!is not under the url of any provider"}, + // The path after the provider's url is too short, or has the wrong shape. + {Entry{Repo: "https://example.com/gitlab/team"}, "!a repository path needs an owner and a name"}, + {Entry{Repo: "https://github.com/acme/billing/tree/main"}, "!a github repository URL names owner/name"}, + {Entry{Org: "https://github.com/acme/billing"}, "!a github organisation URL names one owner"}, + {Entry{Org: "http://localhost:3000/acme/x"}, "!a gitea organisation URL names one owner"}, + {Entry{Repo: "https://gitlab.example.com/platform/api/-/merge_requests"}, "!the /-/ segment points inside a GitLab project"}, + // The entry's provider must be where the URL points. + {Entry{Repo: "https://github.com/acme/x", Provider: "corp"}, "!is not under the url of provider corp (https://gitlab.example.com)"}, + // A URL in the wrong form. + {Entry{Repo: "http://github.com/acme/x"}, `!"http://github.com/acme/x" is not a repository URL`}, + } { + in := &Targets{Targets: []Entry{tc.entry}} + got, err := ResolveURLs(in, provs) + if want, ok := strings.CutPrefix(tc.want, "!"); ok { + if err == nil || !strings.Contains(err.Error(), want) { + t.Errorf("%+v: error %v, want one with %q", tc.entry, err, want) + } + continue + } + if err != nil { + t.Errorf("%+v: %v", tc.entry, err) + continue + } + if v := selectorValue(&got.Targets[0]); v != tc.want { + t.Errorf("%+v: %q, want %q", tc.entry, v, tc.want) + } + if !reflect.DeepEqual(in.Targets[0], tc.entry) { + t.Errorf("%+v: the input changed to %+v", tc.entry, in.Targets[0]) + } + } +} + +// TestResolveURLsExclude: exclude entries, patterns included; the values +// that are no URLs stay as they are, and every URL that fails is reported +// with its field. +func TestResolveURLsExclude(t *testing.T) { + provs := resolvedProviders(t, urlHub) + in := &Targets{ + Defaults: Defaults{Provider: "gh", Packs: []string{"agents"}}, + Targets: []Entry{{Repo: "acme/billing"}, {Org: "https://github.com/acme", Match: []string{"acme/svc-*"}}}, + Exclude: []string{ + "acme/legacy", + "https://gitlab.example.com/platform/legacy/**", + "https://github.com/acme/legacy-*", + "https://github.com/acme/old/", + }, + } + got, err := ResolveURLs(in, provs) + if err != nil { + t.Fatal(err) + } + want := &Targets{ + Defaults: Defaults{Provider: "gh", Packs: []string{"agents"}}, + Targets: []Entry{{Repo: "acme/billing"}, {Org: "gh:acme", Match: []string{"acme/svc-*"}}}, + Exclude: []string{"acme/legacy", "corp:platform/legacy/**", "gh:acme/legacy-*", "gh:acme/old"}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("ResolveURLs =\n%+v\nwant\n%+v", got, want) + } + if got.HasURLs() || !in.HasURLs() { + t.Error("HasURLs is wrong for the result or the input") + } + if _, errs := Check(&Hub{ID: "acme-eng", Providers: []Provider{{ID: "gh"}, {ID: "corp"}}}, got, knownPacks()); len(errs) > 0 { + t.Errorf("Check of the result: %v", errs) + } + + bad := &Targets{ + Targets: []Entry{{Repo: "https://bitbucket.org/a/b"}, {Repo: "acme/x"}, {Group: "https://gitlab.example.com/a/-/b"}}, + Exclude: []string{"https://github.com/a/*/-", "https://jdoe:hunter2@github.com/acme/x", "https://gitlab.example.com/a/-/*"}, + } + _, err = ResolveURLs(bad, provs) + msgs := flattenErr(err) + wantFields := []string{"targets.yml: targets[0].repo:", "targets.yml: targets[2].group:", "targets.yml: exclude[0]:", "targets.yml: exclude[1]:", "targets.yml: exclude[2]:"} + if len(msgs) != len(wantFields) { + t.Fatalf("errors %q, want %d", msgs, len(wantFields)) + } + for i, f := range wantFields { + if !strings.HasPrefix(msgs[i], f) { + t.Errorf("error %q, want it to start with %q", msgs[i], f) + } + } + if strings.Contains(err.Error(), "hunter2") || strings.Contains(err.Error(), "jdoe") { + t.Errorf("the error shows the credential of a URL: %v", err) + } + + // Nothing to resolve: the same targets come back. + plain := &Targets{Targets: []Entry{{Repo: "acme/x"}}} + if got, err := ResolveURLs(plain, nil); got != plain || err != nil { + t.Errorf("ResolveURLs without URLs = %p, %v; want the input", got, err) + } + if got, err := ResolveURLs(nil, provs); got != nil || err != nil { + t.Errorf("ResolveURLs(nil) = %v, %v", got, err) + } +} + +// TestResolveURLsExcludeShape: an exclude URL copied from a page inside a +// repository of a platform without nested namespaces (GitHub's /tree/main, +// Gitea's /src/branch/main) names no repository and would exclude nothing, +// so it is refused, as for repo:; a "**" may span nothing, so it may stand +// in a longer path. A pattern ending in .git is refused: dropping .git, as +// a repository URL may end in it, would widen the pattern. +func TestResolveURLsExcludeShape(t *testing.T) { + provs := resolvedProviders(t, urlHub) + for _, tc := range []struct{ url, want string }{ + {"https://github.com/acme/legacy/tree/main", "!a github repository URL names owner/name"}, + {"https://github.com/acme/legacy/tree/main/**", "gh:acme/legacy/tree/main/**"}, + {"https://github.com/acme/**/legacy", "gh:acme/**/legacy"}, + {"http://localhost:3000/acme/legacy/src/branch/main", "!so it points inside a repository and would exclude nothing"}, + {"https://gitlab.example.com/platform/legacy/api", "corp:platform/legacy/api"}, + {"https://github.com/acme/legacy.git", "gh:acme/legacy"}, + {"https://github.com/acme/*.git", "!a pattern whose last segment ends in .git; write it without .git"}, + {"https://gitlab.example.com/platform/**/*.GIT", "!ends in .git"}, + } { + got, err := ResolveURLs(&Targets{Exclude: []string{tc.url}}, provs) + if want, ok := strings.CutPrefix(tc.want, "!"); ok { + if err == nil || !strings.Contains(err.Error(), want) { + t.Errorf("%s: error %v, want one with %q", tc.url, err, want) + } + continue + } + if err != nil || got.Exclude[0] != tc.want { + t.Errorf("%s: %v, %v; want %q", tc.url, got, err, tc.want) + } + } +} + +// flattenErr splits a joined error into its messages. +func flattenErr(err error) []string { + if err == nil { + return nil + } + if j, ok := err.(interface{ Unwrap() []error }); ok { + var out []string + for _, e := range j.Unwrap() { + out = append(out, flattenErr(e)...) + } + return out + } + return []string{err.Error()} +} + +// TestResolveURLsAmbiguous: providers whose urls a URL lies beneath alike +// are an error, unless the entry names one, or defaults.provider is one of +// them (for exclude entries too). +func TestResolveURLsAmbiguous(t *testing.T) { + provs := resolvedProviders(t, `version: 1 +id: acme-eng +providers: + - {id: gh, type: github} + - {id: gh-app, type: github, url: https://github.com/} + - {id: tea, type: gitea, url: https://example.com} + - {id: lab, type: gitlab, url: https://example.com/gitlab} +`) + for _, tc := range []struct { + targets *Targets + want string + }{ + {&Targets{Targets: []Entry{{Repo: "https://github.com/acme/x"}}}, "is under the url of providers gh at https://github.com, gh-app at https://github.com"}, + {&Targets{Exclude: []string{"https://github.com/acme/x"}}, "name one with provider: or defaults.provider, or write the target as :"}, + // defaults.provider picks only among the providers the URL lies under. + {&Targets{Defaults: Defaults{Provider: "tea"}, Targets: []Entry{{Repo: "https://github.com/acme/x"}}}, "is under the url of providers gh at"}, + {&Targets{Targets: []Entry{{Repo: "https://example.com/gitlab/team/app"}}}, "providers tea at https://example.com, lab at https://example.com/gitlab"}, + } { + if _, err := ResolveURLs(tc.targets, provs); err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("%+v: error %v, want one with %q", tc.targets, err, tc.want) + } + } + got, err := ResolveURLs(&Targets{Targets: []Entry{ + {Repo: "https://github.com/acme/x", Provider: "gh-app"}, + {Repo: "https://example.com/gitlab/team/app", Provider: "lab"}, + {Repo: "https://example.com/acme/tools", Provider: "tea"}, + }}, provs) + if err != nil { + t.Fatal(err) + } + for i, want := range []string{"gh-app:acme/x", "lab:team/app", "tea:acme/tools"} { + if got.Targets[i].Repo != want { + t.Errorf("targets[%d] = %q, want %q", i, got.Targets[i].Repo, want) + } + } + // defaults.provider picks one for entries that name none, and for + // exclude entries; an entry's provider still wins. + got, err = ResolveURLs(&Targets{ + Defaults: Defaults{Provider: "gh-app"}, + Targets: []Entry{{Repo: "https://github.com/acme/x"}, {Repo: "https://github.com/acme/y", Provider: "gh"}, {Repo: "https://example.com/acme/tools"}}, + Exclude: []string{"https://github.com/acme/legacy-*"}, + }, provs) + if err != nil { + t.Fatal(err) + } + for i, want := range []string{"gh-app:acme/x", "gh:acme/y", "tea:acme/tools"} { + if got.Targets[i].Repo != want { + t.Errorf("with defaults.provider: targets[%d] = %q, want %q", i, got.Targets[i].Repo, want) + } + } + if got.Exclude[0] != "gh-app:acme/legacy-*" { + t.Errorf("with defaults.provider: exclude[0] = %q", got.Exclude[0]) + } + // gitea has no nested namespaces: tea cannot hold gitlab/team/app. + if _, err := ResolveURLs(&Targets{Targets: []Entry{{Repo: "https://example.com/gitlab/team/app", Provider: "tea"}}}, provs); err == nil || + !strings.Contains(err.Error(), "a gitea repository URL names owner/name") { + t.Errorf("tea with three segments: %v", err) + } +} + +// TestResolveURLsImplicit: the implicit provider, from the CI or the hub's +// origin, is the hub's only one: its targets are bare paths, which Check +// accepts with no providers in hub.yml. +func TestResolveURLsImplicit(t *testing.T) { + hub := mustParseHub(t, []byte("version: 1\nid: acme-eng\nwriter: acme-write[bot]\n")) + env := map[string]string{"GITLAB_CI": "true", "CI_SERVER_URL": "https://gitlab.example.com/"} + rps, err := hub.ResolveProviders(func(k string) string { return env[k] }) + if err != nil { + t.Fatal(err) + } + targets, _, err := ParseTargets([]byte("version: 1\ntargets:\n - group: https://gitlab.example.com/platform\nexclude:\n - https://gitlab.example.com/platform/legacy/**\n")) + if err != nil { + t.Fatal(err) + } + got, err := ResolveURLs(targets, rps) + if err != nil { + t.Fatal(err) + } + if got.Targets[0].Group != "platform" || got.Exclude[0] != "platform/legacy/**" { + t.Errorf("resolved %+v", got) + } + if warns, errs := Check(hub, got, knownPacks()); len(errs) > 0 || len(warns) > 0 { + t.Errorf("Check: %v, %v", errs, warns) + } + if _, errs := Check(hub, targets, knownPacks()); len(errs) != 1 || !strings.Contains(errs[0].Error(), "not resolved to providers") { + t.Errorf("Check of unresolved URLs: %v", errs) + } +} + +func TestDisplayURL(t *testing.T) { + for in, want := range map[string]string{ + "https://github.com/acme/x": "https://github.com/acme/x", + "https://jdoe:hunter2@github.com/acme/x": "https://…@github.com/acme/x", + "https://a@b@github.com": "https://…@github.com", + "https://github.com/acme/x?mail=me@x.com": "https://github.com/acme/x?mail=me@x.com", + "acme/x": "acme/x", + "gh:acme/x": "gh:acme/x", + "ssh://git@github.com:acme/x.git": "ssh://…@github.com:acme/x.git", + "https://token@github.com#frag@not-userinfo": "https://…@github.com#frag@not-userinfo", + } { + if got := displayURL(in); got != want { + t.Errorf("displayURL(%q) = %q, want %q", in, got, want) + } + } + // Every message of the parsers cuts it too, wherever the URL is. + if got := cleanMessage(`hub.yml: url: "https://jdoe:hunter2@gitlab.example.com" must be`); strings.Contains(got, "hunter2") { + t.Errorf("cleanMessage kept the password: %q", got) + } + if _, _, err := ParseHub([]byte("version: 1\nid: acme-eng\nproviders:\n - {id: corp, type: gitlab, url: \"https://jdoe:hunter2@gitlab.example.com\"}\n")); err == nil || + strings.Contains(err.Error(), "hunter2") { + t.Errorf("ParseHub error shows the password: %v", err) + } + if _, _, err := ParseTargets([]byte("version: 1\ntargets:\n - repo: https://jdoe:hunter2@github.com/acme/x\n")); err == nil || + strings.Contains(err.Error(), "hunter2") { + t.Errorf("ParseTargets error shows the password: %v", err) + } +} + +// TestRefRefusesURL: where a URL is not accepted (operations.yml, --only, +// a URL left unresolved), the message says so. +func TestRefRefusesURL(t *testing.T) { + if _, err := ParseRef("https://github.com/acme/x"); err == nil || !strings.Contains(err.Error(), "a URL is not accepted here") { + t.Errorf("ParseRef of a URL: %v", err) + } + if _, err := Selectors(nil, &Targets{Targets: []Entry{{Repo: "https://github.com/acme/x"}}}); err == nil || + !strings.Contains(err.Error(), "a URL is not accepted here") { + t.Errorf("Selectors of a URL: %v", err) + } +} diff --git a/internal/config/validate.go b/internal/config/validate.go index 260ea5c..f507277 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -30,16 +30,38 @@ const ( providerIDPattern = `^[a-z][a-z0-9-]{0,31}$` // refSegment is one path segment of a repository or namespace: the // characters platforms allow in names, and never "." or "..". - refSegment = `([A-Za-z0-9_-][A-Za-z0-9_.-]*|\.[A-Za-z0-9_-][A-Za-z0-9_.-]*|\.\.[A-Za-z0-9_.-]+)` - refPrefix = `^([a-z][a-z0-9-]{0,31}:)?` - refPattern = refPrefix + refSegment + `(/` + refSegment + `)+$` - namespacePattern = refPrefix + refSegment + `(/` + refSegment + `)*$` - urlPattern = `^(https://[A-Za-z0-9.-]+(:[0-9]{1,5})?|http://(localhost|127\.0\.0\.1|\[::1\])(:[0-9]{1,5})?)(/[^?#` + space + `]*)?$` - fingerprintPattern = `^[A-Za-z0-9.-]+(:[0-9]{1,5})?/[0-9]+$` - cooldownPattern = `^[1-9][0-9]{0,4}[dh]$` - accountPattern = `^[^` + space + `]+$` - labelPattern = `^[^,]*[^,` + space + `][^,]*$` - nonBlankPattern = `[^` + space + `]` + refSegment = `([A-Za-z0-9_-][A-Za-z0-9_.-]*|\.[A-Za-z0-9_-][A-Za-z0-9_.-]*|\.\.[A-Za-z0-9_.-]+)` + refPrefix = `^([a-z][a-z0-9-]{0,31}:)?` + refPattern = refPrefix + refSegment + `(/` + refSegment + `)+$` + namespacePattern = refPrefix + refSegment + `(/` + refSegment + `)*$` + // urlBase is the scheme, host and port of a URL touchmark accepts: + // https, or http for the loopback host only. + urlBase = `(https://[A-Za-z0-9.-]+(:[0-9]{1,5})?|http://(localhost|127\.0\.0\.1|\[::1\])(:[0-9]{1,5})?)` + urlPattern = `^` + urlBase + `(/[^?#` + space + `]*)?$` + // globSegment is one segment of an exclude or match pattern: a + // repository path segment that may hold the glob characters * and ?. + // A segment of "**" spans segments. + globSegment = `([A-Za-z0-9_*?-][A-Za-z0-9_.*?-]*|\.[A-Za-z0-9_*?-][A-Za-z0-9_.*?-]*|\.\.[A-Za-z0-9_.*?-]+)` + // urlGlobSegment is globSegment without ?, which starts a URL's query. + urlGlobSegment = `([A-Za-z0-9_*-][A-Za-z0-9_.*-]*|\.[A-Za-z0-9_*-][A-Za-z0-9_.*-]*|\.\.[A-Za-z0-9_.*-]+)` + // excludePattern is an exclude entry written as [:], + // matchPattern a pattern of match: both cover a repository path, two + // segments or more. + excludePattern = refPrefix + globSegment + `(/` + globSegment + `)+$` + matchPattern = `^` + globSegment + `(/` + globSegment + `)+$` + // The web URLs targets.yml accepts: a repository's (repo), a + // namespace's (org and group), and an exclude pattern's. A provider may + // live under a path, so how many segments name the repository is known + // only once the URL meets the providers (ResolveURLs); a trailing slash + // and, for a repository, a trailing .git are allowed. + repoURLPattern = `^` + urlBase + `(/` + refSegment + `){2,}/?$` + namespaceURLPattern = `^` + urlBase + `(/` + refSegment + `)+/?$` + excludeURLPattern = `^` + urlBase + `(/` + urlGlobSegment + `){2,}/?$` + fingerprintPattern = `^[A-Za-z0-9.-]+(:[0-9]{1,5})?/[0-9]+$` + cooldownPattern = `^[1-9][0-9]{0,4}[dh]$` + accountPattern = `^[^` + space + `]+$` + labelPattern = `^[^,]*[^,` + space + `][^,]*$` + nonBlankPattern = `[^` + space + `]` // patternPattern requires an ignore or sensitive_paths pattern to hold // something besides spaces, slashes and backslashes; the parser checks // that it is not empty after trimming them and "./". @@ -63,15 +85,18 @@ const ( ) var ( - packNameRe = regexp.MustCompile(packNamePattern) - providerIDRe = regexp.MustCompile(providerIDPattern) - urlRe = regexp.MustCompile(urlPattern) - fingerprintRe = regexp.MustCompile(fingerprintPattern) - cooldownRe = regexp.MustCompile(cooldownPattern) - accountRe = regexp.MustCompile(accountPattern) - labelRe = regexp.MustCompile(labelPattern) - nonBlankRe = regexp.MustCompile(nonBlankPattern) - draftRe = regexp.MustCompile(draftPattern) + packNameRe = regexp.MustCompile(packNamePattern) + providerIDRe = regexp.MustCompile(providerIDPattern) + urlRe = regexp.MustCompile(urlPattern) + repoURLRe = regexp.MustCompile(repoURLPattern) + namespaceURLRe = regexp.MustCompile(namespaceURLPattern) + excludeURLRe = regexp.MustCompile(excludeURLPattern) + fingerprintRe = regexp.MustCompile(fingerprintPattern) + cooldownRe = regexp.MustCompile(cooldownPattern) + accountRe = regexp.MustCompile(accountPattern) + labelRe = regexp.MustCompile(labelPattern) + nonBlankRe = regexp.MustCompile(nonBlankPattern) + draftRe = regexp.MustCompile(draftPattern) ) // secretRe matches strings that look like credentials: GitHub and GitLab @@ -234,9 +259,23 @@ func checkBranchName(s string) error { // parseRef parses "path" or "provider:path" where path has at least // minSegments segments. -func parseRef(s string, minSegments int) (Ref, error) { +func parseRef(s string, minSegments int) (Ref, error) { return parseRefOf(s, minSegments, false) } + +// parsePattern parses an exclude entry that is not a URL: +// "[provider:]pattern", where pattern covers two segments or more and may +// hold the glob characters * and ? (glob.go). An entry without them is a +// reference, which ParseRef reads. +func parsePattern(s string) (Ref, error) { return parseRefOf(s, 2, isGlob(s)) } + +// parseRefOf is parseRef, with the glob characters * and ? allowed in the +// path when glob is set. A URL is refused: only targets.yml takes one, and +// ResolveURLs rewrites it first. +func parseRefOf(s string, minSegments int, glob bool) (Ref, error) { if len(s) > maxRefLen { - return Ref{}, fmt.Errorf("%.40q…: longer than %d bytes", s, maxRefLen) + return Ref{}, fmt.Errorf("%.40q…: longer than %d bytes", displayURL(s), maxRefLen) + } + if isURL(s) { + return Ref{}, fmt.Errorf("%q: a URL is not accepted here; write the repository as :", displayURL(s)) } var r Ref path := s @@ -246,14 +285,16 @@ func parseRef(s string, minSegments int) (Ref, error) { } r.Provider, path = prov, rest } - if err := checkRefPath(path, minSegments); err != nil { + if err := checkRefPath(path, minSegments, glob); err != nil { return Ref{}, fmt.Errorf("%q: %w", s, err) } r.Path = path return r, nil } -func checkRefPath(p string, minSegments int) error { +// checkRefPath checks a repository or namespace path of at least +// minSegments segments; with glob set, a segment may hold * and ?. +func checkRefPath(p string, minSegments int, glob bool) error { if p == "" { return errors.New("empty path") } @@ -269,12 +310,19 @@ func checkRefPath(p string, minSegments int) error { return fmt.Errorf("%q path segment", seg) } for _, c := range seg { - if !isRefChar(c) { + switch { + case isRefChar(c), glob && isGlobChar(c): + case isGlobChar(c): + return fmt.Errorf("character %q is not allowed in a repository path; glob patterns go in exclude and match", c) + default: return fmt.Errorf("character %q is not allowed in a repository path", c) } } } if len(segs) < minSegments { + if glob { + return errors.New("a pattern covers a whole repository path, like acme/legacy-* or platform/legacy/**") + } return errors.New("a repository path needs an owner and a name, like acme/billing") } return nil diff --git a/internal/decide/sweep.go b/internal/decide/sweep.go index ad3348c..307f369 100644 --- a/internal/decide/sweep.go +++ b/internal/decide/sweep.go @@ -47,9 +47,11 @@ type SweepInput struct { // so the target is missing under its old path while the sweep finds // its pull requests under the new one. Unresolved map[string]bool - // OptedOut holds "host/id" of the Active targets whose opt-in file is - // gone (skipped:not-opted-in): their own open PRs are closed with - // ReasonOptedOut. DecideTarget never runs for them. + // OptedOut holds "host/id" of the Active targets that are not opted in + // (skipped:not-opted-in: no opt-in file, and targets.yml does not + // subscribe them; skipped:opted-out: the file says enabled: false): + // their own open PRs are closed with ReasonOptedOut. DecideTarget never + // runs for them. OptedOut map[string]bool // Complete is set when the provider's resolve and the OpenPRsBy // listing were both complete; otherwise nothing is swept. diff --git a/internal/distribute/body.go b/internal/distribute/body.go index 0071bab..b0a1be9 100644 --- a/internal/distribute/body.go +++ b/internal/distribute/body.go @@ -43,7 +43,8 @@ func (r *run) content(w *Work, reads branchReads) (pairs []decide.Pair, hubCommi // fillBody fills the body input (Work.Body: the rows of what the branch // holds, and while paused what a rebuild would bring, D − C; the local // files; the hub's name, link and sensitive paths; the target's packs and -// platform) and the marker data of D (Work.Marker). A paused branch without +// platform, and whether targets.yml subscribed it without an opt-in file) +// and the marker data of D (Work.Marker). A paused branch without // a commit of touchmark's (its Hc is gone: foreign) has no C: the body // claims nothing about what it holds, shows no table, and lists all of D as // what a rebuild would bring. @@ -83,6 +84,7 @@ func (r *run) fillBody(w *Work, sel config.Selection, reads branchReads) { Local: local, Sensitive: slices.Clone(r.hub.SensitivePaths), OptInFile: r.optIn, + Assumed: t.res.Assumed && t.res.AssumedBy == report.AssumedByHub, GiteaWorkflows: giteaWorkflows(t.prov.cfg.Type, w.Tree, append(slices.Clone(rows), pending...)), Caps: t.prov.caps, } diff --git a/internal/distribute/distribute.go b/internal/distribute/distribute.go index cf2dd35..e35be00 100644 --- a/internal/distribute/distribute.go +++ b/internal/distribute/distribute.go @@ -118,8 +118,8 @@ type Deps struct { InCI bool // Scope limits a plan in a hub pull request to the targets the pull // request touches (scope.go); nil plans every target. - // AssumeOptIn plans every target as opted in (plan --assume-opt-in, - // optInAndTree). Only ModePlan reads them. + // AssumeOptIn plans every target without an opt-in file as opted in + // (plan --assume-opt-in, optInAndTree). Only ModePlan reads them. Scope *Scope AssumeOptIn bool // Now is the clock; time.Now when nil. @@ -219,9 +219,12 @@ func HeadGuard(c hubch.Context) bool { // known_authors and automation_accounts (their stable ids feed // decide.Identity and the classes of closes); every targets.yml entry whose // provider is this one becomes a platform.Selector (repo: → Reader.Repo; -// org/group → Resolve with topics, subgroups, forks); exclude removes -// targets (same matching as config, by the canonical path and the path a -// repo: entry wrote; a repo: entry excluded as written is never looked up); +// org/group → Resolve with topics, subgroups, forks, keeping the +// repositories whose path matches one of the entry's match patterns); +// exclude removes targets (same matching as config, patterns included, by +// the canonical path and the path a repo: entry wrote; a repo: entry +// excluded as written is never looked up); web URLs in targets.yml are +// resolved against the run's providers first (config.ResolveURLs); // targets are deduplicated by (Host, ID), and a repository two providers // reach is handled by the provider of its first entry in targets.yml, with // the target warning "duplicate-provider"; every target remembers the @@ -269,10 +272,13 @@ func HeadGuard(c hubch.Context) bool { // a chunk of 50 targets at once where the reader is a // platform.BatchReader (optin.go; a file the batch cannot settle is read // on its own): not found (platform.ClassOf gives ClassNotFound) → -// skipped:not-opted-in; ErrNotRegular or ErrTooLarge → -// skipped:unsafe-opt-in; parse error → blocked:opt-in-invalid. With -// AssumeOptIn a missing or irregular file is taken for an empty one -// (DeliveryTarget.Assumed). +// skipped:not-opted-in, unless an entry that selects the target has +// opt_in: assumed, which takes an empty file for it (DeliveryTarget +// Assumed, AssumedBy targets.yml); ErrNotRegular or ErrTooLarge → +// skipped:unsafe-opt-in; parse error → blocked:opt-in-invalid; +// enabled: false → skipped:opted-out, whose pull requests the sweep +// closes as a missing file's. With AssumeOptIn a missing or irregular +// file is taken for an empty one (AssumedBy --assume-opt-in). // 3. packs: config.SelectFor with the matched entries; unknown packs → // blocked:opt-in-invalid with the message as a warning. A plan limited // by Scope stops here for a target whose final pack list holds no pack @@ -487,9 +493,12 @@ type target struct { // written are the paths repo: entries wrote for it. written []string // entries are the targets.yml entries that matched it, providers the - // ids of every provider that found it (prov first). + // ids of every provider that found it (prov first). assumed is set when + // one of the entries has opt_in: assumed: the target counts as opted in + // without an opt-in file. entries []int providers []string + assumed bool // first is the first entry that found it and its position in that // entry's listing: its place in targets.yml order. first [2]int @@ -512,6 +521,10 @@ type target struct { // dropped marks a repository the sweep added: it is no target of the // run any more. dropped bool + // optOut is why the sweep closes the pull requests of a target that is + // not opted in (prbody.CauseDisabled, prbody.CauseNoOptIn), for the + // comment after each close; "" for any other target. + optOut string // outOfScope marks a target a limited plan leaves out (scope.go): it // stays a target of the run, without a report line. optIn is its opt-in // file as a batch read it, until phase C takes it (optin.go). @@ -629,6 +642,19 @@ func newRun(d Deps, mode Mode) (*run, error) { if r.targets == nil { r.targets = &config.Targets{} } + if r.targets.HasURLs() { + // The CLI resolves them as it reads targets.yml; a caller that did + // not gets them resolved against the run's providers. + rps := make([]config.ResolvedProvider, len(d.Providers)) + for i, p := range d.Providers { + rps[i] = p.Config + } + targets, err := config.ResolveURLs(r.targets, rps) + if err != nil { + return nil, err + } + r.targets = targets + } r.affected = r.scopePacks() r.rep.Strict = d.Strict r.rep.Assumed = mode == ModePlan && d.AssumeOptIn diff --git a/internal/distribute/doctor.go b/internal/distribute/doctor.go index 400afcf..f5b3f49 100644 --- a/internal/distribute/doctor.go +++ b/internal/distribute/doctor.go @@ -60,8 +60,10 @@ type DoctorDeps struct { // visible to it (it is a member). // // Per target: skipped with the reason of delivery (archived, disabled, -// empty, mirror, pending-deletion, prs-disabled, sha256), not-opted-in or -// unsafe-opt-in (the opt-in file read through the API), or +// empty, mirror, pending-deletion, prs-disabled, sha256), not-opted-in +// (no opt-in file, and targets.yml does not subscribe the target), +// opted-out (the opt-in file says enabled: false) or unsafe-opt-in (the +// opt-in file read through the API), or // deferred: with an unknown check when the provider's circuit is // open or the run ended (notChecked); else the checks // of platform.Checker for the repository and the sync branches (access, @@ -340,14 +342,24 @@ func (r *run) doctorTarget(ctx context.Context, t *target) (dt report.DoctorTarg return notChecked(dt, "interrupted", "the run was interrupted") } rd := &doctorReads{r: r, p: p} + var file platform.File err := rd.read(ctx, func() error { - _, err := p.reader.ReadFile(ctx, t.repo, "", r.optIn, maxOptIn) + var err error + file, err = p.reader.ReadFile(ctx, t.repo, "", r.optIn, maxOptIn) return err }) switch { case err == nil: + // A file that does not parse is delivery's business + // (blocked:opt-in-invalid); the writer's access is checked anyway. + if o, _, perr := config.ParseOptIn(file.Content); perr == nil && o.Disabled() { + dt.Skipped = reasonOptedOut + return dt + } + case isNotFound(err) && ctx.Err() == nil && t.assumed: + // targets.yml subscribes it: delivery writes to it without the file. case isNotFound(err) && ctx.Err() == nil: - dt.Skipped = "not-opted-in" + dt.Skipped = reasonNotOptedIn return dt case errors.Is(err, platform.ErrNotRegular) || errors.Is(err, platform.ErrTooLarge): dt.Skipped = "unsafe-opt-in" diff --git a/internal/distribute/doctor_test.go b/internal/distribute/doctor_test.go index 2e8f159..bfae692 100644 --- a/internal/distribute/doctor_test.go +++ b/internal/distribute/doctor_test.go @@ -136,6 +136,31 @@ func TestDoctor(t *testing.T) { } } +// TestDoctorOptIn: doctor checks a repository targets.yml subscribes +// without an opt-in file, as delivery writes to it, and skips one whose +// opt-in file says enabled: false, whatever targets.yml says. +func TestDoctorOptIn(t *testing.T) { + w := newWorld(t) + w.targetsYML = assumedTargetsYML + svc := w.repo("acme/svc-a", nil, "README.md", "subscribed by the hub\n") + w.p.GrantWrite(svc.ID, w.writer) + w.repo("acme/svc-off", nil, optInName, "version: 1\nenabled: false\n") + w.repo("acme/web", topics("python"), "README.md", "wants an opt-in file\n") + broken := w.repo("acme/svc-broken", nil, optInName, "version: 2\n") + w.p.GrantWrite(broken.ID, w.writer) + doc := w.doctor(w.doctorDeps(w.writer)) + wantCheck(t, "svc-a", doctorTarget(t, doc, "gh:acme/svc-a").Checks, "access", report.StatusOK, "may push") + if got := doctorTarget(t, doc, "gh:acme/svc-off"); got.Skipped != "opted-out" || len(got.Checks) != 0 { + t.Errorf("svc-off: %+v", got) + } + if got := doctorTarget(t, doc, "gh:acme/web"); got.Skipped != "not-opted-in" { + t.Errorf("web: %+v", got) + } + // An opt-in file that does not parse is delivery's business: the + // writer's access is checked anyway. + wantCheck(t, "svc-broken", doctorTarget(t, doc, "gh:acme/svc-broken").Checks, "access", report.StatusOK, "may push") +} + // TestDoctorHub: whether the writer may write to the hub, by what it sees. func TestDoctorHub(t *testing.T) { for _, tc := range []struct { diff --git a/internal/distribute/inspect.go b/internal/distribute/inspect.go index 8da6cf3..846a7b4 100644 --- a/internal/distribute/inspect.go +++ b/internal/distribute/inspect.go @@ -171,26 +171,37 @@ func (r *run) inspect(ctx context.Context, t *target, by time.Time) *Work { // is decided, or a limited plan leaves the target out (scope.go): then no // snapshot is taken. // +// A target that targets.yml subscribes (an entry that selects it has +// opt_in: assumed) and that has no opt-in file is processed as if it had an +// empty one: the packs of defaults and of its entries, nothing ignored. An +// opt-in file that says enabled: false opts any target out: +// skipped:opted-out, and the sweep closes its pull requests. When the API +// found the opt-in file of a subscribed target and the tree does not hold +// it, the file is read again at the tree's commit too: its deletion returns +// the target to the subscription, and must not make it look not opted in, +// which the sweep would take for an opt-out. +// // A plan with Deps.AssumeOptIn plans a target without an opt-in file, or // with one that is not a regular file or is too large, as if it had an empty -// one (for the report only); its report line says so (Assumed). Only a file -// the API did not find and the tree holds is read again at the tree's -// commit: an unsafe one stays assumed. +// one (for the report only). Either way the report line says so (Assumed, +// AssumedBy). Only a file the API did not find and the tree holds is read +// again at the tree's commit: an unsafe one stays assumed. func (r *run) optInAndTree(ctx context.Context, t *target) (optIn *config.OptIn, sel config.Selection, tree *snapshot.Tree, ok bool) { res := &t.res assume := r.mode == ModePlan && r.d.AssumeOptIn ref := "" for attempt := 0; ; attempt++ { file, err := r.readOptIn(ctx, t, ref) - // assumed: the plan takes an empty opt-in file; unsafe: because the - // API found one that is not a regular file or is too large. + // assumed: the run takes an empty opt-in file; unsafe: because the + // API found one that is not a regular file or is too large (plan + // --assume-opt-in only). assumed, unsafe := false, false switch { case err == nil: - case isNotFound(err) && ctx.Err() == nil && assume: + case isNotFound(err) && ctx.Err() == nil && (t.assumed || assume): assumed = true case isNotFound(err) && ctx.Err() == nil: - res.Outcome, res.Reason = report.OutcomeSkipped, "not-opted-in" + res.Outcome, res.Reason = report.OutcomeSkipped, reasonNotOptedIn r.leaveByHub(t, nil) return nil, sel, nil, false case errors.Is(err, platform.ErrNotRegular) || errors.Is(err, platform.ErrTooLarge): @@ -218,6 +229,11 @@ func (r *run) optInAndTree(ctx context.Context, t *target) (optIn *config.OptIn, r.leaveByHub(t, nil) return nil, sel, nil, false } + if o.Disabled() { + res.Outcome, res.Reason = report.OutcomeSkipped, reasonOptedOut + r.leaveByHub(t, nil) + return nil, sel, nil, false + } s, warns, err := config.SelectFor(r.hub, r.targets, o, t.entries, r.known) if err != nil { res.Outcome, res.Reason = report.OutcomeBlocked, "opt-in-invalid" @@ -249,8 +265,21 @@ func (r *run) optInAndTree(ctx context.Context, t *target) (optIn *config.OptIn, res.Warnings = append(res.Warnings, err.Error()) return nil, sel, nil, false } + case !inTree && assumed: + // Subscribed by targets.yml, and no opt-in file on either side. + case !inTree && t.assumed && attempt == 0 && tree.Commit != "": + // The file of a subscribed target went away in between: read it + // again at the snapshot's commit, where the subscription holds. + // (Without targets.yml's subscription, the snapshot's word is + // enough: the target is not opted in.) + ref = tree.Commit + continue + case !inTree && t.assumed: + res.Outcome, res.Reason = report.OutcomeFailed, "race" + res.Warnings = append(res.Warnings, fmt.Sprintf("%s read through the API is not in the snapshot", r.optIn)) + return nil, sel, nil, false case !inTree: - res.Outcome, res.Reason = report.OutcomeSkipped, "not-opted-in" + res.Outcome, res.Reason = report.OutcomeSkipped, reasonNotOptedIn return nil, sel, nil, false case !regular: res.Outcome, res.Reason = report.OutcomeSkipped, "unsafe-opt-in" @@ -273,11 +302,26 @@ func (r *run) optInAndTree(ctx context.Context, t *target) (optIn *config.OptIn, for _, w := range warns { res.Warnings = append(res.Warnings, w.String()) } - res.Assumed = assumed + res.Assumed, res.AssumedBy = assumed, "" + switch { + case assumed && t.assumed && !unsafe && !inTree: + res.AssumedBy = report.AssumedByHub + case assumed: + res.AssumedBy = report.AssumedByFlag + } return o, s, tree, true } } +// Skip reasons of the opt-in step. +const ( + // reasonNotOptedIn: the target has no opt-in file, and targets.yml does + // not subscribe it. + reasonNotOptedIn = "not-opted-in" + // reasonOptedOut: the opt-in file says enabled: false. + reasonOptedOut = "opted-out" +) + // snapshot takes the snapshot of the target's default branch. func (r *run) snapshot(ctx context.Context, t *target) (*snapshot.Tree, bool) { var remote platform.Remote diff --git a/internal/distribute/plan_test.go b/internal/distribute/plan_test.go index 4e8f16c..ceb96a3 100644 --- a/internal/distribute/plan_test.go +++ b/internal/distribute/plan_test.go @@ -1170,6 +1170,61 @@ providers: } } +// An exclude entry without glob characters names one project: written for +// a subgroup, it excludes none of the projects beneath it, and the run +// warns, counting them; with /** it excludes them. +func TestPlanExcludeNamespace(t *testing.T) { + p := fake.New("gitlab.example.com", fake.WithFlavor(fake.GitLab)) + reader := p.AddAccount("tm-reader", platform.KindServiceAccount) + p.AddAccount("tm-writer", platform.KindServiceAccount) + for _, path := range []string{"platform/api", "platform/legacy/a", "platform/legacy/b", "platform/legacy-web"} { + r := p.AddRepo(platform.Repo{Path: path}) + p.SetFile(r.ID, optInName, []byte("version: 1\n"), "") + } + if err := p.Err(); err != nil { + t.Fatal(err) + } + w := newWorld(t) + w.hubYML = `version: 1 +id: acme-eng +providers: + - id: corp + type: gitlab + url: https://gitlab.example.com + writer: tm-writer +` + hub, _, err := config.ParseHub([]byte(w.hubYML)) + if err != nil { + t.Fatal(err) + } + plan := func(exclude string) *report.Delivery { + t.Helper() + w.targetsYML = "version: 1\ndefaults:\n packs: [base]\ntargets:\n - group: platform\nexclude:\n - " + exclude + "\n" + d := w.deps() + d.Providers = w.providers(hub, map[string]platform.Reader{"gitlab.example.com": p.Reader(reader)}) + d.Snapshots = sources{"gitlab.example.com": p.Snapshots()} + return w.plan(d) + } + rep := plan("corp:platform/legacy") + if len(rep.Targets) != 4 { + t.Errorf("%d targets, want 4", len(rep.Targets)) + } + warn := "targets.yml: exclude[0] (corp:platform/legacy) names one repository and excludes nothing beneath it, where 2 targets lie; " + + "to exclude a namespace, write corp:platform/legacy/**" + if !slices.Contains(rep.Warnings, warn) { + t.Errorf("warnings %q, want %q", rep.Warnings, warn) + } + rep = plan("corp:platform/legacy/**") + if len(rep.Targets) != 2 || slices.ContainsFunc(rep.Warnings, func(s string) bool { return strings.Contains(s, "exclude[0]") }) { + t.Errorf("with /**: %d targets, warnings %q", len(rep.Targets), rep.Warnings) + } + // A project that the entry does name is excluded, without a warning. + rep = plan("corp:platform/legacy-web") + if len(rep.Targets) != 3 || slices.ContainsFunc(rep.Warnings, func(s string) bool { return strings.Contains(s, "exclude[0]") }) { + t.Errorf("a project: %d targets, warnings %q", len(rep.Targets), rep.Warnings) + } +} + // The output does not depend on how many targets are inspected at once. func TestPlanDeterministic(t *testing.T) { w := newWorld(t) diff --git a/internal/distribute/resolve.go b/internal/distribute/resolve.go index d80ea08..41f489d 100644 --- a/internal/distribute/resolve.go +++ b/internal/distribute/resolve.go @@ -42,6 +42,7 @@ func (r *run) resolve(ctx context.Context) ([]*target, error) { if !slices.Contains(t.entries, s.Entry) { t.entries = append(t.entries, s.Entry) } + t.assumed = t.assumed || s.Assumed if !slices.Contains(t.providers, p.cfg.ID) { t.providers = append(t.providers, p.cfg.ID) } @@ -389,8 +390,9 @@ func (r *run) resolveRepo(ctx context.Context, p *provider, s config.Selector, a platform.ClassOf(err) == platform.ClassAuth) } -// resolveNamespace resolves an org: or group: entry and reports whether the -// provider may go on resolving. +// resolveNamespace resolves an org: or group: entry, keeping the +// repositories its match patterns select, and reports whether the provider +// may go on resolving. func (r *run) resolveNamespace(ctx context.Context, p *provider, s config.Selector, add addFunc) bool { var res platform.Resolved err := r.retry(ctx, p, func() error { @@ -416,21 +418,28 @@ func (r *run) resolveNamespace(ctx context.Context, p *provider, s config.Select r.warnf("provider %s: the listing of targets.yml targets[%d] (%s) is incomplete%s", p.cfg.ID, s.Entry, s.Namespace, why) } for i, repo := range res.Repos { - add(p, s, repo, i) + if s.Selects(repo.Path) { + add(p, s, repo, i) + } } return true } // filter drops excluded targets and, with Only, the targets it does not // name; marks the targets a public hub must not name; and adds the -// warnings about duplicates and renames. +// warnings about duplicates and renames, and about exclude entries that +// name one repository while targets lie beneath them (beneathExcludes). func (r *run) filter(list []*target) []*target { matched := make([]bool, len(r.d.Only)) var kept []*target + beneath := map[int]int{} for _, t := range list { if r.excluded(t) { continue } + for _, i := range config.ExcludedBeneath(r.hub, r.targets, t.prov.cfg.ID, t.repo.Path) { + beneath[i]++ + } if len(r.d.Only) > 0 { hit := false for i, ref := range r.d.Only { @@ -450,6 +459,7 @@ func (r *run) filter(list []*target) []*target { r.warnf("--only %s matches no target", ref) } } + r.beneathExcludes(beneath) for _, t := range kept { if len(t.providers) > 1 { others := strings.Join(t.providers[1:], ", ") @@ -471,6 +481,27 @@ func (r *run) filter(list []*target) []*target { return kept } +// beneathExcludes warns about each exclude entry that names one repository +// while n targets lie beneath it (beneath: entry index → n): it excludes +// none of them, and a namespace takes "/**". The warning names the entry, +// which targets.yml shows anyway, and counts the targets without naming +// them. +func (r *run) beneathExcludes(beneath map[int]int) { + for i := range r.targets.Exclude { + n := beneath[i] + if n == 0 { + continue + } + what := "1 target lies" + if n > 1 { + what = fmt.Sprintf("%d targets lie", n) + } + ex := r.targets.Exclude[i] + r.warnf("%s: exclude[%d] (%s) names one repository and excludes nothing beneath it, where %s; to exclude a namespace, write %s/**", + config.TargetsFile, i, ex, what, ex) + } +} + // excluded reports whether exclude names t on any provider that found it, // by its canonical path or a path a repo: entry wrote. func (r *run) excluded(t *target) bool { diff --git a/internal/distribute/run_optin_test.go b/internal/distribute/run_optin_test.go new file mode 100644 index 0000000..4eb2c68 --- /dev/null +++ b/internal/distribute/run_optin_test.go @@ -0,0 +1,386 @@ +package distribute + +import ( + "bytes" + "context" + "slices" + "strings" + "sync" + "testing" + "time" + + "github.com/bedrock-python/touchmark/internal/platform" + "github.com/bedrock-python/touchmark/internal/report" +) + +// assumedTargetsYML subscribes the services of acme (opt_in: assumed, +// narrowed by match) and other/tool, and lists every repository of acme +// with the python topic as an entry that wants an opt-in file; the legacy +// services are excluded by a pattern. +const assumedTargetsYML = `version: 1 +defaults: + packs: [base] +targets: + - org: acme + match: [acme/svc-*] + opt_in: assumed + - org: acme + topics: [python] + packs: [python] + - repo: other/tool + opt_in: assumed +exclude: + - acme/svc-legacy* +` + +// TestPlanAssumedOptIn: a repository an entry with opt_in: assumed +// selects counts as opted in without an opt-in file, with the packs of +// defaults and of its entries, and the report says targets.yml assumed +// it; one selected only by an entry that wants the file is not opted in; +// enabled: false opts any repository out; an opt-in file of its own wins +// over the assumption. plan --assume-opt-in marks what it assumes apart. +func TestPlanAssumedOptIn(t *testing.T) { + w := newWorld(t) + w.targetsYML = assumedTargetsYML + w.repo("acme/svc-a", nil, "README.md", "service a\n") + w.repo("acme/svc-b", topics("python"), "README.md", "service b\n") + w.repo("acme/web", topics("python"), "README.md", "the web site\n") + w.repo("acme/svc-off", nil, optInName, "version: 1\nenabled: false\n") + w.repo("acme/svc-legacy-1", nil, "README.md", "excluded\n") + w.repo("acme/lib", topics("python"), optInName, "version: 1\n") + w.repo("other/tool", nil, "README.md", "a tool\n") + w.repo("acme/svc-own", nil, optInName, "version: 1\npacks: [python]\n") + w.repo("acme/batch", nil, "README.md", "selected by no entry\n") + + rep := w.plan(w.deps()) + check := func(rep *report.Delivery, ref string, outcome report.Outcome, reason, by string, packs ...string) { + t.Helper() + tg := want(t, rep, ref, outcome, reason, 0) + if tg.Assumed != (by != "") || tg.AssumedBy != by { + t.Errorf("%s: assumed %v by %q, want %q", ref, tg.Assumed, tg.AssumedBy, by) + } + if packs != nil && !slices.Equal(tg.Packs, packs) { + t.Errorf("%s: packs %v, want %v", ref, tg.Packs, packs) + } + } + check(rep, "gh:acme/svc-a", report.OutcomeOpened, "", report.AssumedByHub, "base") + check(rep, "gh:acme/svc-b", report.OutcomeOpened, "", report.AssumedByHub, "base", "python") + check(rep, "gh:acme/web", report.OutcomeSkipped, "not-opted-in", "") + check(rep, "gh:acme/svc-off", report.OutcomeSkipped, "opted-out", "") + check(rep, "gh:acme/lib", report.OutcomeOpened, "", "", "base", "python") + check(rep, "gh:other/tool", report.OutcomeOpened, "", report.AssumedByHub, "base") + check(rep, "gh:acme/svc-own", report.OutcomeOpened, "", "", "base", "python") + for _, tg := range rep.Targets { + if tg.Path == "acme/svc-legacy-1" || tg.Path == "acme/batch" { + t.Errorf("%s is a target: %+v", tg.Path, tg) + } + } + if len(rep.Targets) != 7 { + t.Errorf("%d targets, want 7", len(rep.Targets)) + } + var text, md bytes.Buffer + if err := rep.WriteText(&text); err != nil { + t.Fatal(err) + } + if err := rep.WriteMarkdown(&md); err != nil { + t.Fatal(err) + } + line := "3 targets without an opt-in file are opted in by targets.yml" + if !strings.Contains(text.String(), "opt_in: assumed: "+line) || + !strings.Contains(md.String(), `**opt\_in\: assumed:** 3 targets without an opt\-in file are opted in by targets\.yml`) { + t.Errorf("no line %q in\n%s\n%s", line, text.String(), md.String()) + } + + // --assume-opt-in takes the rest for opted in, apart; enabled: false + // still opts out, and the hub's own assumption stays the hub's. + rep = w.plan(func() Deps { d := w.deps(); d.AssumeOptIn = true; return d }()) + check(rep, "gh:acme/web", report.OutcomeOpened, "", report.AssumedByFlag, "base", "python") + check(rep, "gh:acme/svc-a", report.OutcomeOpened, "", report.AssumedByHub, "base") + check(rep, "gh:acme/svc-off", report.OutcomeSkipped, "opted-out", "") + text.Reset() + if err := rep.WriteText(&text); err != nil { + t.Fatal(err) + } + if !strings.Contains(text.String(), "--assume-opt-in: for this report only, a target without an opt-in file counts as opted in (one whose file says enabled: false stays opted out); 1 target is planned") { + t.Errorf("--assume-opt-in line:\n%s", text.String()) + } +} + +// TestPlanTargetsByURL: entries and exclude entries written as web URLs +// select the same targets as their : forms, and the report +// names the targets :. +func TestPlanTargetsByURL(t *testing.T) { + w := newWorld(t) + w.targetsYML = `version: 1 +defaults: + packs: [base] +targets: + - org: https://github.com/acme/ + match: [acme/svc-*] + - repo: https://github.com/other/tool.git +exclude: + - https://github.com/acme/svc-legacy-* +` + w.optedIn("acme/svc-a", nil) + w.optedIn("acme/svc-legacy-1", nil) + w.optedIn("acme/web", nil) + w.optedIn("other/tool", nil) + rep := w.plan(w.deps()) + var refs []string + for _, tg := range rep.Targets { + refs = append(refs, tg.Provider+":"+tg.Path) + } + if !slices.Equal(refs, []string{"gh:acme/svc-a", "gh:other/tool"}) { + t.Errorf("targets %q", refs) + } + // A URL no provider serves stops the run before any target is read. + w.targetsYML = "version: 1\ntargets:\n - repo: https://gitlab.com/acme/x\n" + if _, err := Plan(t.Context(), w.deps()); err == nil || !strings.Contains(err.Error(), "is not under the url of any provider") { + t.Errorf("Plan with a URL of no provider: %v", err) + } +} + +// TestRunAssumedOptIn: the life of a repository the hub subscribes +// (opt_in: assumed) through distribute. Without an opt-in file it gets a +// pull request whose description says how to choose packs and how to opt +// out; closing it is a decline, remembered, which an empty opt-in file +// keeps and a change of ignore lifts. An open pull request of a subscribed +// repository is never swept for its missing file, while one of a +// repository selected only by an entry that wants the file is. enabled: +// false closes the open pull request as opted-out; deleting the file again +// returns the repository to the hub's subscription. +func TestRunAssumedOptIn(t *testing.T) { + t.Parallel() + w := newGitWorld(t) + w.targetsYML = assumedTargetsYML + billing := w.repo("acme/svc-billing", nil, "README.md", "the billing service\n") + api := w.repo("acme/svc-api", nil, "README.md", "the api\n") + kept := w.repo("acme/svc-kept", nil, "README.md", "a service with a sync pull request\n") + w.syncCommit(kept, branch, "", baseFiles...) + nKept := w.openOwn(kept) + w.written(kept, nKept) + web := w.repo("acme/web", topics("python"), "README.md", "the web site\n") + nWeb := w.openOwn(web) + + rep := w.distribute(nil) + tg := targetOf(t, rep, "gh:acme/svc-billing") + if tg.Outcome != report.OutcomeOpened || tg.PR == nil || tg.AssumedBy != report.AssumedByHub { + t.Fatalf("svc-billing: %+v", tg) + } + nBilling := tg.PR.Number + pr := w.p.PR(billing.ID, nBilling) + if !strings.Contains(pr.Body, "The hub subscribed this repository, which has no `.engineering-assets.yml`.") || + !strings.Contains(pr.Body, "add it with `enabled: false`") { + t.Errorf("the description does not explain the subscription:\n%s", pr.Body) + } + nAPI := want(t, rep, "gh:acme/svc-api", report.OutcomeOpened, "", 1).PR.Number + want(t, rep, "gh:acme/svc-kept", report.OutcomeUnchanged, "", nKept) + want(t, rep, "gh:acme/web", report.OutcomeClosed, "opted-out", nWeb) + if c := w.p.Comments(web.ID, nWeb); len(c) == 0 || !strings.Contains(c[len(c)-1].Body, "has no opt-in file, and the hub does not subscribe it") { + t.Errorf("close comment of a repository without the file: %+v", c) + } + if got := w.p.PR(kept.ID, nKept).State; got != platform.Open { + t.Errorf("the subscribed repository's pull request is %s", got) + } + + // A person closes the pull request: a decline, acked once, then quiet. + w.p.SetPRState(billing.ID, nBilling, platform.Closed, &w.person, time.Now()) + w.ok() + rep = w.distribute(nil) + if tg := want(t, rep, "gh:acme/svc-billing", report.OutcomeDeclined, "", nBilling); tg.Writes != 2 { + t.Errorf("first sight of the decline: %d writes, want 2 (ack, comment)", tg.Writes) + } + // An empty opt-in file chooses nothing new: the decline holds. + w.push(billing, "main", w.person, optInName, "version: 1\n") + rep = w.run(w.deps(ModeDistribute), ModeDistribute) + if tg := want(t, rep, "gh:acme/svc-billing", report.OutcomeDeclined, "", nBilling); tg.Writes != 0 || tg.Assumed { + t.Errorf("with an empty opt-in file: %+v", tg) + } + // A change of ignore is a new choice: the content comes again, without + // the paragraph, as the repository has its file now. + w.push(billing, "main", w.person, optInName, "version: 1\nignore: [notes/**]\n") + rep = w.distribute(nil) + tg = targetOf(t, rep, "gh:acme/svc-billing") + if tg.Outcome != report.OutcomeOpened || tg.PR == nil || tg.PR.Number == nBilling || tg.Assumed { + t.Fatalf("after the change of ignore: %+v", tg) + } + if body := w.p.PR(billing.ID, tg.PR.Number).Body; strings.Contains(body, "subscribed") { + t.Errorf("a repository with its opt-in file is told it was subscribed:\n%s", body) + } + + // enabled: false opts the repository out: its pull request is closed. + w.push(api, "main", w.person, optInName, "version: 1\nenabled: false\n") + rep = w.distribute(nil) + if tg := want(t, rep, "gh:acme/svc-api", report.OutcomeClosed, "opted-out", nAPI); tg.Writes != 2 { + t.Errorf("opt-out: %d writes, want 2 (close, comment)", tg.Writes) + } + if got := w.p.PR(api.ID, nAPI).State; got != platform.Closed { + t.Errorf("the pull request of the opted-out repository is %s", got) + } + comments := w.p.Comments(api.ID, nAPI) + if len(comments) == 0 || !strings.Contains(comments[len(comments)-1].Body, "says `enabled: false`") { + t.Errorf("close comment %+v", comments) + } + rep = w.run(w.deps(ModeDistribute), ModeDistribute) + want(t, rep, "gh:acme/svc-api", report.OutcomeSkipped, "opted-out", 0) + + // Deleting the file returns it to the hub's subscription: the content + // comes again, as touchmark's own close was no decline. + w.push(api, "main", w.person, optInName, "") + rep = w.distribute(nil) + tg = targetOf(t, rep, "gh:acme/svc-api") + if tg.Outcome != report.OutcomeOpened || tg.PR == nil || tg.PR.Number == nAPI || tg.AssumedBy != report.AssumedByHub { + t.Errorf("after the file is gone again: %+v", tg) + } +} + +// TestRunAssumedOptInWithdrawn: the hub withdraws its subscription (the +// entry drops opt_in: assumed). The open pull request of a subscribed +// repository that never had an opt-in file is closed as opted-out, with a +// comment that blames no deletion and no enabled: false. +func TestRunAssumedOptInWithdrawn(t *testing.T) { + t.Parallel() + w := newGitWorld(t) + w.targetsYML = `version: 1 +defaults: + packs: [base] +targets: + - org: acme + opt_in: assumed +` + svc := w.repo("acme/svc", nil, "README.md", "a service\n") + rep := w.distribute(nil) + tg := targetOf(t, rep, "gh:acme/svc") + if tg.Outcome != report.OutcomeOpened || tg.PR == nil { + t.Fatalf("subscribed: %+v", tg) + } + n := tg.PR.Number + + w.targetsYML = `version: 1 +defaults: + packs: [base] +targets: + - org: acme +` + rep = w.distribute(nil) + want(t, rep, "gh:acme/svc", report.OutcomeClosed, "opted-out", n) + c := w.p.Comments(svc.ID, n) + if len(c) == 0 { + t.Fatal("no close comment") + } + body := c[len(c)-1].Body + if !strings.Contains(body, "has no opt-in file, and the hub does not subscribe it") || + !strings.Contains(body, "add the opt-in file `"+optInName+"`") || + strings.Contains(body, "enabled") || strings.Contains(body, " back") { + t.Errorf("close comment:\n%s", body) + } +} + +// TestPlanAssumedOptInDeleted: the opt-in file of a subscribed repository +// is deleted between the API read and the snapshot. It is read again at the +// snapshot's commit, where the subscription holds, so the repository is +// planned as subscribed, not as not opted in (which the sweep would take +// for an opt-out); a reader that keeps answering the file fails the +// target. A repository targets.yml does not subscribe takes the snapshot's +// word: not opted in. +func TestPlanAssumedOptInDeleted(t *testing.T) { + w := newWorld(t) + w.targetsYML = assumedTargetsYML + svc := w.repo("acme/svc-a", nil, "README.md", "service a\n") + web := w.repo("acme/web", topics("python"), "README.md", "the web site\n") + d := w.deps() + stale := &staleOptInOf{Reader: d.Providers[0].Reader, repos: []string{svc.ID, web.ID}} + d.Providers[0].Reader = stale + rep := w.plan(d) + if tg := want(t, rep, "gh:acme/svc-a", report.OutcomeOpened, "", 0); tg.AssumedBy != report.AssumedByHub { + t.Errorf("svc-a: assumed by %q, want %q", tg.AssumedBy, report.AssumedByHub) + } + if got := stale.reads(svc.ID); !slices.Equal(got, []string{"", w.p.Head(svc.ID)}) { + t.Errorf("svc-a: opt-in reads at %q, want the default branch, then %s", got, w.p.Head(svc.ID)) + } + want(t, rep, "gh:acme/web", report.OutcomeSkipped, "not-opted-in", 0) + if got := stale.reads(web.ID); !slices.Equal(got, []string{""}) { + t.Errorf("web: opt-in reads at %q, want the default branch only", got) + } + + d.Providers[0].Reader = &staleOptInOf{Reader: w.deps().Providers[0].Reader, repos: []string{svc.ID}, always: true} + rep = w.plan(d) + tg := want(t, rep, "gh:acme/svc-a", report.OutcomeFailed, "race", 0) + if !slices.ContainsFunc(tg.Warnings, func(s string) bool { return strings.Contains(s, "is not in the snapshot") }) { + t.Errorf("svc-a: warnings %q", tg.Warnings) + } +} + +// TestRunAssumedOptInDeleted: distribute over the race of +// TestPlanAssumedOptInDeleted keeps the open pull request of the subscribed +// repository: it is no opt-out. +func TestRunAssumedOptInDeleted(t *testing.T) { + t.Parallel() + w := newGitWorld(t) + w.targetsYML = assumedTargetsYML + kept := w.repo("acme/svc-kept", nil, "README.md", "a service with a sync pull request\n") + w.syncCommit(kept, branch, "", baseFiles...) + n := w.openOwn(kept) + w.written(kept, n) + var stale *staleOptInOf + rep := w.distribute(func(d *Deps) { + stale = &staleOptInOf{Reader: d.Providers[0].Writer, repos: []string{kept.ID}} + d.Providers[0].Writer = staleOptInWriter{Writer: d.Providers[0].Writer, stale: stale} + }) + if tg := want(t, rep, "gh:acme/svc-kept", report.OutcomeUnchanged, "", n); tg.AssumedBy != report.AssumedByHub { + t.Errorf("svc-kept: assumed by %q", tg.AssumedBy) + } + if got := w.p.PR(kept.ID, n).State; got != platform.Open { + t.Errorf("the pull request of the subscribed repository is %s", got) + } + if got := stale.reads(kept.ID); len(got) < 2 || got[0] != "" || got[1] != w.p.Head(kept.ID) { + t.Errorf("opt-in reads at %q, want the default branch, then %s", got, w.p.Head(kept.ID)) + } +} + +// staleOptInOf answers the first read of the opt-in file of each of repos +// (every read with always) with a file the default branch no longer holds. +type staleOptInOf struct { + platform.Reader + repos []string + always bool + + mu sync.Mutex + refs map[string][]string +} + +// staleOptInWriter is a writer, which reads in distribute, whose opt-in +// reads go through stale (whose Reader is the writer). +type staleOptInWriter struct { + platform.Writer + stale *staleOptInOf +} + +func (s staleOptInWriter) ReadFile(ctx context.Context, r platform.Repo, ref, path string, max int64) (platform.File, error) { + return s.stale.ReadFile(ctx, r, ref, path, max) +} + +func (s *staleOptInOf) ReadFile(ctx context.Context, r platform.Repo, ref, path string, max int64) (platform.File, error) { + if path != optInName || !slices.Contains(s.repos, r.ID) { + return s.Reader.ReadFile(ctx, r, ref, path, max) + } + s.mu.Lock() + if s.refs == nil { + s.refs = map[string][]string{} + } + s.refs[r.ID] = append(s.refs[r.ID], ref) + first := len(s.refs[r.ID]) == 1 + s.mu.Unlock() + if s.always || first { + content := "version: 1\n" + return platform.File{Path: path, Mode: "100644", OID: oid(content), Content: []byte(content)}, nil + } + return s.Reader.ReadFile(ctx, r, ref, path, max) +} + +// reads returns the refs the opt-in file of repo was read at. +func (s *staleOptInOf) reads(repo string) []string { + s.mu.Lock() + defer s.mu.Unlock() + return slices.Clone(s.refs[repo]) +} diff --git a/internal/distribute/scope_test.go b/internal/distribute/scope_test.go index 00edc27..7de9c6f 100644 --- a/internal/distribute/scope_test.go +++ b/internal/distribute/scope_test.go @@ -309,7 +309,7 @@ func TestPlanAssumeOptIn(t *testing.T) { if err := rep.WriteText(&text); err != nil { t.Fatal(err) } - if !strings.Contains(text.String(), "--assume-opt-in: every target counts as opted in, for this report only; 2 targets without an opt-in file") { + if !strings.Contains(text.String(), "--assume-opt-in: for this report only, a target without an opt-in file counts as opted in (one whose file says enabled: false stays opted out); 2 targets are planned") { t.Errorf("text:\n%s", text.String()) } diff --git a/internal/distribute/steps.go b/internal/distribute/steps.go index 3ab5c9f..b8e670b 100644 --- a/internal/distribute/steps.go +++ b/internal/distribute/steps.go @@ -317,7 +317,7 @@ func (p *preparer) comment(s decide.Step) ([]writeAct, error) { var text string switch s.Reason { case decide.OutcomeClosed: - text = prbody.ClosedComment(cmp.Or(p.closed[s.PR], x.w.Decision.Reason, x.res.Reason)) + text = prbody.ClosedComment(cmp.Or(p.closed[s.PR], x.w.Decision.Reason, x.res.Reason), x.t.optOut, x.r.optIn) case decide.OutcomeDeclined: text = prbody.DeclinedComment(s.PR, x.declinedPaths(s.PR), x.r.optIn) case decide.CommentAutoDeclined: diff --git a/internal/distribute/sweep.go b/internal/distribute/sweep.go index 8181e12..f9b83e7 100644 --- a/internal/distribute/sweep.go +++ b/internal/distribute/sweep.go @@ -8,14 +8,18 @@ import ( "github.com/bedrock-python/touchmark/internal/decide" "github.com/bedrock-python/touchmark/internal/platform" + "github.com/bedrock-python/touchmark/internal/prbody" "github.com/bedrock-python/touchmark/internal/report" "github.com/bedrock-python/touchmark/internal/throttle" ) // sweepAll runs phase D: per provider, the own open pull requests of -// repositories that are no longer targets (target-dropped) or whose opt-in -// file is gone (opted-out) become sweep closes. It returns them as works, in -// decide.Sweep's order, provider by provider, and fills the report's Sweep. +// repositories that are no longer targets (target-dropped), or that are no +// longer opted in (opted-out: the target has no opt-in file and targets.yml +// does not subscribe it, or the file says enabled: false; target.optOut +// keeps which, for the comment), become sweep closes. It returns them as +// works, in decide.Sweep's order, provider by provider, and fills the +// report's Sweep. // // A plan limited to the targets of some packs (scope.go) keeps only the // closes of the targets it processes: the targets it leaves out are active @@ -45,8 +49,15 @@ func (r *run) sweepAll(ctx context.Context, kept []*target) []*Work { for _, t := range kept { key := t.host + "/" + t.repo.ID active[key] = true - if !t.outOfScope && t.res.Outcome == report.OutcomeSkipped && t.res.Reason == "not-opted-in" { - optedOut[key] = t + if t.outOfScope || t.res.Outcome != report.OutcomeSkipped { + continue + } + // The skip reason says why, before the first close replaces it. + switch t.res.Reason { + case reasonNotOptedIn: + optedOut[key], t.optOut = t, prbody.CauseNoOptIn + case reasonOptedOut: + optedOut[key], t.optOut = t, prbody.CauseDisabled } } var works []*Work diff --git a/internal/pathx/pathx.go b/internal/pathx/pathx.go index 79c31f5..41d7dd7 100644 --- a/internal/pathx/pathx.go +++ b/internal/pathx/pathx.go @@ -204,6 +204,17 @@ func Match(pattern, p string) bool { return false } +// MatchSegments reports whether the path p matches pattern segment by +// segment: "*", "?" and "[...]" work inside one segment as in path.Match, +// and a "**" segment matches zero or more segments. Unlike Match, it takes +// pattern and p as they are: nothing is trimmed, and a pattern without +// glob characters matches only the path it spells, not what lies beneath +// it. Matching takes time proportional to the number of pattern segments +// times the number of path segments. +func MatchSegments(pattern, p string) bool { + return matchSegments(strings.Split(pattern, "/"), strings.Split(p, "/")) +} + // MatchAny reports whether p matches any of patterns. func MatchAny(patterns []string, p string) bool { for _, pat := range patterns { diff --git a/internal/pathx/pathx_test.go b/internal/pathx/pathx_test.go index 7e93c10..b7874ac 100644 --- a/internal/pathx/pathx_test.go +++ b/internal/pathx/pathx_test.go @@ -450,6 +450,35 @@ func TestMatchSegmentsExhaustive(t *testing.T) { } } +// TestMatchSegments: MatchSegments takes its pattern as written, without +// Match's trimming and without its subtree rule. +func TestMatchSegments(t *testing.T) { + for _, tc := range []struct { + pattern, p string + want bool + }{ + {"docs", "docs", true}, + {"docs", "docs/a.md", false}, + {"./docs", "docs", false}, + {"docs/", "docs", false}, + {"a/**", "a/b/c", true}, + {"**/c", "a/b/c", true}, + {"a/*/c", "a/b/c", true}, + {"a/?", "a/bb", false}, + } { + if got := MatchSegments(tc.pattern, tc.p); got != tc.want { + t.Errorf("MatchSegments(%q, %q) = %v, want %v", tc.pattern, tc.p, got, tc.want) + } + } + for _, pat := range sequences([]string{"a", "*", "**"}, 3) { + for _, segs := range sequences([]string{"a", "b"}, 3) { + if got, want := MatchSegments(strings.Join(pat, "/"), strings.Join(segs, "/")), matchSegmentsRef(pat, segs); got != want { + t.Fatalf("MatchSegments(%q, %q) = %v, want %v", pat, segs, got, want) + } + } + } +} + func FuzzMatch(f *testing.F) { f.Add("docs", "docs/a.md") f.Add("**/x", "a/b/x") diff --git a/internal/prbody/comments.go b/internal/prbody/comments.go index 1a789c5..8497588 100644 --- a/internal/prbody/comments.go +++ b/internal/prbody/comments.go @@ -106,13 +106,31 @@ const ( reasonDuplicate = "duplicate" ) +// Why a repository is no longer opted in, for an opted-out close +// (ClosedComment's cause). +const ( + // CauseDisabled: its opt-in file says enabled: false. + CauseDisabled = "disabled" + // CauseNoOptIn: it has no opt-in file, and targets.yml does not + // subscribe it (the file was deleted, or the hub withdrew opt_in: + // assumed). + CauseNoOptIn = "no-opt-in" +) + // ClosedComment is the comment after touchmark closes its own PR, for reason -// (decide.Reason* close reasons). +// (decide.Reason* close reasons). An opted-out close says why the +// repository is no longer opted in by its cause (CauseDisabled, +// CauseNoOptIn; any other value covers both), naming the opt-in file +// optInFile ("" for "the opt-in file"). // // It says why and what comes next, without naming the request "pull" or // "merge" (it has no platform) and without a mention or a line that starts // with "/". An unknown reason is shown in a code span. -func ClosedComment(reason string) string { +func ClosedComment(reason, cause, optInFile string) string { + optIn := "the opt-in file" + if optInFile != "" { + optIn = "the opt-in file " + Code(optInFile) + } var why, next string switch reason { case reasonNoDiff: @@ -121,8 +139,19 @@ func ClosedComment(reason string) string { why = "there is nothing left to sync (the default branch has the hub's versions of these files, or they are ignored or changed here)" next = "This is not a decline: nothing is remembered." case reasonOptedOut: - why = "the opt-in file of this repository is gone" - next = "To get engineering assets again, add the opt-in file back." + switch cause { + case CauseDisabled: + why = "this repository opted out, as " + optIn + " says `enabled: false`" + next = "To get engineering assets again, remove `enabled: false` from it." + case CauseNoOptIn: + // The repository may never have had the file: the hub may + // have withdrawn its subscription. + why = "this repository has no opt-in file, and the hub does not subscribe it (the file was deleted, or the hub stopped subscribing it)" + next = "To get engineering assets again, add " + optIn + "." + default: + why = "this repository is no longer opted in (it has no opt-in file and the hub does not subscribe it, or its opt-in file says `enabled: false`)" + next = "To get engineering assets again, add " + optIn + ", or remove `enabled: false` from it." + } case reasonTargetDropped: why = "the hub no longer syncs this repository" next = "This is not a decline: if the hub adds this repository back, a new one comes." diff --git a/internal/prbody/comments_test.go b/internal/prbody/comments_test.go index f6349bc..4729121 100644 --- a/internal/prbody/comments_test.go +++ b/internal/prbody/comments_test.go @@ -193,14 +193,29 @@ func TestCommitMessageMatchesCheck(t *testing.T) { func TestClosedComment(t *testing.T) { for _, reason := range []string{"no-diff", "opted-out", "target-dropped", "duplicate"} { - got := ClosedComment(reason) + got := ClosedComment(reason, "", optInName) golden(t, "comments/closed-"+reason+".md", got) checkSafe(t, got, 1) if strings.Contains(got, "reason `") { t.Errorf("%s: explained as an unknown reason", reason) } } - got := ClosedComment("@x\n/close`") + // An opt-out by its cause: the file says enabled: false, or the + // repository has none and the hub does not subscribe it (it may never + // have had one, when the hub withdrew its subscription). + for _, cause := range []string{CauseDisabled, CauseNoOptIn} { + got := ClosedComment("opted-out", cause, optInName) + golden(t, "comments/closed-opted-out-"+cause+".md", got) + checkSafe(t, got, 1) + } + if got := ClosedComment("opted-out", CauseNoOptIn, ""); strings.Contains(got, "enabled") || strings.Contains(got, "back") || + !strings.Contains(got, "add the opt-in file.") { + t.Errorf("no opt-in file: %q", got) + } + if got := ClosedComment("no-diff", CauseDisabled, optInName); got != ClosedComment("no-diff", "", "") { + t.Errorf("a cause changes the comment of another reason: %q", got) + } + got := ClosedComment("@x\n/close`", "", "") if want := "touchmark closed this: reason `` @x/close` ``."; got != want { t.Errorf("unknown reason: %q, want %q", got, want) } @@ -211,7 +226,7 @@ func TestClosedComment(t *testing.T) { // are decide's. func TestDecideNames(t *testing.T) { for _, reason := range []string{decide.ReasonNoDiff, decide.ReasonOptedOut, decide.ReasonTargetDropped, decide.ReasonDuplicate} { - if strings.Contains(ClosedComment(reason), "reason `") { + if strings.Contains(ClosedComment(reason, "", ""), "reason `") { t.Errorf("close reason %q has no text of its own", reason) } } diff --git a/internal/prbody/inert_test.go b/internal/prbody/inert_test.go index 4586eac..c6c5cf6 100644 --- a/internal/prbody/inert_test.go +++ b/internal/prbody/inert_test.go @@ -66,7 +66,7 @@ func TestCheckText(t *testing.T) { if Inert(body) != body { t.Errorf("Inert changes a rendered body:\n%s", body) } - for _, c := range []string{ClosedComment("no-diff"), DeclinedComment(3, []string{"@team/notes.md", "a/b.md"}, ".engineering-assets.yml"), + for _, c := range []string{ClosedComment("no-diff", "", ""), ClosedComment("opted-out", CauseDisabled, ".engineering-assets.yml"), DeclinedComment(3, []string{"@team/notes.md", "a/b.md"}, ".engineering-assets.yml"), AutoDeclinedComment(4, []string{"@x.md"}, "", []string{"engineering-assets"})} { if err := CheckText(c); err != nil { t.Errorf("a comment fails CheckText: %v\n%s", err, c) diff --git a/internal/prbody/prbody.go b/internal/prbody/prbody.go index 4f99dc9..04e1414 100644 --- a/internal/prbody/prbody.go +++ b/internal/prbody/prbody.go @@ -136,6 +136,10 @@ type Input struct { // OptInFile is the target's opt-in file name (hub.yml opt_in_file), used // in the hints about ignore; "" reads "the opt-in file". OptInFile string + // Assumed is set when the target has no opt-in file and the hub + // subscribes it (opt_in: assumed in targets.yml): a paragraph after the + // hub line says how to choose packs or ignore files, and how to opt out. + Assumed bool // Blocks. PreviouslyDeclined []int64 Paused bool @@ -180,7 +184,8 @@ const ( numLists ) -// Render returns the body (in order: intro and hub line; ⚠ sensitive paths, +// Render returns the body (in order: intro and hub line; the paragraph of a +// target the hub subscribed without an opt-in file; ⚠ sensitive paths, // never cut; the changes table ordered sensitive, deletes, updates, creates, // at most 100 rows then "and N more"; local files; blocks; controls; // footnote; marker last). It fits the body into Caps.MaxBody by shortening @@ -246,7 +251,7 @@ type renderer struct { declined []int64 // Fixed parts. - intro, head, sensitive, declinedBlock, updateBlock, nothingBlock, controls, footnote string + intro, head, assumed, sensitive, declinedBlock, updateBlock, nothingBlock, controls, footnote string // lists are indexed by listChanges, listLocal and listPending; nil for // a section the body does not have. lists [numLists]*cuttable @@ -286,6 +291,7 @@ func newRenderer(in Input) (*renderer, error) { r.declined = prNumbers(in.PreviouslyDeclined) r.head = r.hubLine(hub) + r.assumed = r.assumedBlock() r.sensitive = r.sensitiveSection() r.declinedBlock = r.previouslyDeclined() r.updateBlock = r.updateBranch() @@ -388,10 +394,11 @@ func (p piece) render() string { } // pieces are the parts of the body in order, each list showing its n rows. -func (r *renderer) pieces(n [numLists]int) [12]piece { +func (r *renderer) pieces(n [numLists]int) [13]piece { return [...]piece{ {text: r.intro}, {text: r.head}, + {text: r.assumed}, {text: r.sensitive}, {list: r.lists[listChanges], rows: n[listChanges]}, {list: r.lists[listLocal], rows: n[listLocal]}, @@ -556,6 +563,21 @@ func (r *renderer) hubLine(hub hubRef) string { return b.String() } +// assumedBlock is the paragraph of a target the hub subscribed without an +// opt-in file: how to choose packs or ignore files, and how to opt out. +func (r *renderer) assumedBlock() string { + if !r.in.Assumed { + return "" + } + name, file, it := "opt-in file", "an opt-in file", "one" + if r.in.OptInFile != "" { + name, file, it = Code(r.in.OptInFile), Code(r.in.OptInFile), "it" + } + return "The hub subscribed this repository, which has no " + name + ". " + + "To add packs or keep files out of the sync, add " + file + " with `packs` or `ignore`; " + + "to stop these " + r.noun + "s, add " + it + " with `enabled: false`." +} + // sensitiveSection is the ⚠ section: the sensitive rows of the table and of // what a rebuild would bring, by path, and the Gitea workflows warning. func (r *renderer) sensitiveSection() string { diff --git a/internal/prbody/render_test.go b/internal/prbody/render_test.go index 5e66b58..f1d5c05 100644 --- a/internal/prbody/render_test.go +++ b/internal/prbody/render_test.go @@ -79,6 +79,14 @@ var scenarios = []struct { in.Local = append(in.Local, fmt.Sprintf("docs/handbook/chapter-%02d/local-%03d.md", i%12, i)) } }}, + {"assumed", func(in *Input) { + // No opt-in file: the hub subscribed the repository. + in.Assumed = true + in.Changes = []Change{ + {Path: "AGENTS.md", Pack: "agents", Action: ActionCreate, Mode: "100644"}, + {Path: "prompts/review.md", Pack: "agents", Action: ActionCreate, Mode: "100644"}, + } + }}, {"sensitive-only", func(in *Input) { in.Sensitive = []string{"deploy/**"} in.GiteaWorkflows = true @@ -287,6 +295,7 @@ func TestRenderOrder(t *testing.T) { in.PreviouslyDeclined = []int64{4} in.Paused, in.UpdateBranchNeeded, in.NothingMore = true, true, true in.ShowRecreate, in.ShowRepropose = true, true + in.Assumed = true body, err := Render(in) if err != nil { t.Fatal(err) @@ -294,6 +303,7 @@ func TestRenderOrder(t *testing.T) { parts := []string{ "Acme keeps its shared", "touchmark syncs packs", + "The hub subscribed this repository", "### ⚠ Sensitive paths", "### Changes", "
", @@ -477,6 +487,45 @@ func TestRenderBudget(t *testing.T) { }) } +// TestRenderAssumed: the paragraph of a repository the hub subscribed names +// the opt-in file, or reads "an opt-in file" without its name, and stays +// when the budget cuts the lists. +func TestRenderAssumed(t *testing.T) { + in := scenarioInput(t, "assumed", "gitlab") + body, err := Render(in) + if err != nil { + t.Fatal(err) + } + want := "The hub subscribed this repository, which has no `.engineering-assets.yml`. " + + "To add packs or keep files out of the sync, add `.engineering-assets.yml` with `packs` or `ignore`; " + + "to stop these merge requests, add it with `enabled: false`." + if !strings.Contains(body, "\n\n"+want+"\n\n") { + t.Errorf("no paragraph %q in\n%s", want, body) + } + in.OptInFile = "" + want = "The hub subscribed this repository, which has no opt-in file. " + + "To add packs or keep files out of the sync, add an opt-in file with `packs` or `ignore`; " + + "to stop these merge requests, add one with `enabled: false`." + if body, err = Render(in); err != nil || !strings.Contains(body, want) { + t.Errorf("without a file name: %v\n%s", err, body) + } + in = scenarioInput(t, "assumed", "github") + in.Changes = hugeChanges(400) + r, err := newRenderer(in) + if err != nil { + t.Fatal(err) + } + in.Caps.MaxBody = len(r.body([numLists]int{10, 0, 0})) + if body, err = Render(in); err != nil || !strings.Contains(body, "The hub subscribed this repository") { + t.Errorf("under a budget: %v", err) + } + checkBody(t, in, body) + in.Assumed = false + if body, err = Render(in); err != nil || strings.Contains(body, "subscribed") { + t.Errorf("a repository with its opt-in file: %v", err) + } +} + // TestSize checks that the computed size of a body is its length, for // every number of rows of every list. func TestSize(t *testing.T) { diff --git a/internal/prbody/testdata/assumed/gitea.md b/internal/prbody/testdata/assumed/gitea.md new file mode 100644 index 0000000..26dc842 --- /dev/null +++ b/internal/prbody/testdata/assumed/gitea.md @@ -0,0 +1,19 @@ +Acme keeps its shared engineering files (agent instructions, editor and CI settings) in one hub. +Review this like any other change; questions go to the platform team's channel. + +touchmark syncs packs `agents`, `claude` and `base` from the hub `acme-eng` ([`github.com/acme/engineering-assets`](https://redirect.github.com/acme/engineering-assets)) at commit [`3f2c1ab9d8e7`](https://redirect.github.com/acme/engineering-assets/commit/3f2c1ab9d8e7f6a5b4c3d2e1f0a9b8c7d6e5f4a3). + +The hub subscribed this repository, which has no `.engineering-assets.yml`. To add packs or keep files out of the sync, add `.engineering-assets.yml` with `packs` or `ignore`; to stop these pull requests, add it with `enabled: false`. + +### Changes + +| Change | File | Pack | +|---|---|---| +| create | `AGENTS.md` | `agents` | +| create | `prompts/review.md` | `agents` | + +--- + +If this pull request is closed without merging, touchmark remembers it and does not propose the same changes again; a new one comes when the hub changes these files. Editing `packs` or `ignore` in `.engineering-assets.yml` resets that memory, and `ignore` opts a file out for good. touchmark rewrites this description, so please comment instead of editing it. + + \ No newline at end of file diff --git a/internal/prbody/testdata/assumed/github.md b/internal/prbody/testdata/assumed/github.md new file mode 100644 index 0000000..26dc842 --- /dev/null +++ b/internal/prbody/testdata/assumed/github.md @@ -0,0 +1,19 @@ +Acme keeps its shared engineering files (agent instructions, editor and CI settings) in one hub. +Review this like any other change; questions go to the platform team's channel. + +touchmark syncs packs `agents`, `claude` and `base` from the hub `acme-eng` ([`github.com/acme/engineering-assets`](https://redirect.github.com/acme/engineering-assets)) at commit [`3f2c1ab9d8e7`](https://redirect.github.com/acme/engineering-assets/commit/3f2c1ab9d8e7f6a5b4c3d2e1f0a9b8c7d6e5f4a3). + +The hub subscribed this repository, which has no `.engineering-assets.yml`. To add packs or keep files out of the sync, add `.engineering-assets.yml` with `packs` or `ignore`; to stop these pull requests, add it with `enabled: false`. + +### Changes + +| Change | File | Pack | +|---|---|---| +| create | `AGENTS.md` | `agents` | +| create | `prompts/review.md` | `agents` | + +--- + +If this pull request is closed without merging, touchmark remembers it and does not propose the same changes again; a new one comes when the hub changes these files. Editing `packs` or `ignore` in `.engineering-assets.yml` resets that memory, and `ignore` opts a file out for good. touchmark rewrites this description, so please comment instead of editing it. + + \ No newline at end of file diff --git a/internal/prbody/testdata/assumed/gitlab.md b/internal/prbody/testdata/assumed/gitlab.md new file mode 100644 index 0000000..7f9c296 --- /dev/null +++ b/internal/prbody/testdata/assumed/gitlab.md @@ -0,0 +1,19 @@ +Acme keeps its shared engineering files (agent instructions, editor and CI settings) in one hub. +Review this like any other change; questions go to the platform team's channel. + +touchmark syncs packs `agents`, `claude` and `base` from the hub `acme-eng` ([`github.com/acme/engineering-assets`](https://redirect.github.com/acme/engineering-assets)) at commit [`3f2c1ab9d8e7`](https://redirect.github.com/acme/engineering-assets/commit/3f2c1ab9d8e7f6a5b4c3d2e1f0a9b8c7d6e5f4a3). + +The hub subscribed this repository, which has no `.engineering-assets.yml`. To add packs or keep files out of the sync, add `.engineering-assets.yml` with `packs` or `ignore`; to stop these merge requests, add it with `enabled: false`. + +### Changes + +| Change | File | Pack | +|---|---|---| +| create | `AGENTS.md` | `agents` | +| create | `prompts/review.md` | `agents` | + +--- + +If this merge request is closed without merging, touchmark remembers it and does not propose the same changes again; a new one comes when the hub changes these files. Editing `packs` or `ignore` in `.engineering-assets.yml` resets that memory, and `ignore` opts a file out for good. touchmark rewrites this description, so please comment instead of editing it. + + \ No newline at end of file diff --git a/internal/prbody/testdata/comments/closed-opted-out-disabled.md b/internal/prbody/testdata/comments/closed-opted-out-disabled.md new file mode 100644 index 0000000..22011b7 --- /dev/null +++ b/internal/prbody/testdata/comments/closed-opted-out-disabled.md @@ -0,0 +1,3 @@ +touchmark closed this: this repository opted out, as the opt-in file `.engineering-assets.yml` says `enabled: false`. + +To get engineering assets again, remove `enabled: false` from it. \ No newline at end of file diff --git a/internal/prbody/testdata/comments/closed-opted-out-no-opt-in.md b/internal/prbody/testdata/comments/closed-opted-out-no-opt-in.md new file mode 100644 index 0000000..839607a --- /dev/null +++ b/internal/prbody/testdata/comments/closed-opted-out-no-opt-in.md @@ -0,0 +1,3 @@ +touchmark closed this: this repository has no opt-in file, and the hub does not subscribe it (the file was deleted, or the hub stopped subscribing it). + +To get engineering assets again, add the opt-in file `.engineering-assets.yml`. \ No newline at end of file diff --git a/internal/prbody/testdata/comments/closed-opted-out.md b/internal/prbody/testdata/comments/closed-opted-out.md index 0ab173e..8ed4f46 100644 --- a/internal/prbody/testdata/comments/closed-opted-out.md +++ b/internal/prbody/testdata/comments/closed-opted-out.md @@ -1,3 +1,3 @@ -touchmark closed this: the opt-in file of this repository is gone. +touchmark closed this: this repository is no longer opted in (it has no opt-in file and the hub does not subscribe it, or its opt-in file says `enabled: false`). -To get engineering assets again, add the opt-in file back. \ No newline at end of file +To get engineering assets again, add the opt-in file `.engineering-assets.yml`, or remove `enabled: false` from it. \ No newline at end of file diff --git a/internal/report/delivery.go b/internal/report/delivery.go index 21febd0..a0f6cd8 100644 --- a/internal/report/delivery.go +++ b/internal/report/delivery.go @@ -44,12 +44,22 @@ var Reasons = map[Outcome][]string{ OutcomeUnchanged: nil, OutcomeClosed: {"no-diff", "opted-out", "target-dropped", "duplicate"}, OutcomeDeclined: nil, - OutcomeSkipped: {"not-opted-in", "archived", "disabled", "empty", "mirror", "pending-deletion", "prs-disabled", "sha256", "unsafe-opt-in", "private-in-public-hub", "superseded"}, + OutcomeSkipped: {"not-opted-in", "opted-out", "archived", "disabled", "empty", "mirror", "pending-deletion", "prs-disabled", "sha256", "unsafe-opt-in", "private-in-public-hub", "superseded"}, OutcomeBlocked: {"edited", "branch-taken", "branch-in-use", "opt-in-invalid", "marker-invalid", "rules:*", "permission:*", "cannot-sign", "archived", "mass-close"}, OutcomeDeferred: {"rate-limit", "deadline", "rollout-limit", "provider-down", "interrupted", "cooldown"}, OutcomeFailed: {"transient", "auth", "access", "git", "integrity", "race", "secret-exposure", "internal"}, } +// Why a target counts as opted in without an opt-in file +// (DeliveryTarget.AssumedBy). +const ( + // AssumedByHub: an entry of targets.yml that selects it has opt_in: + // assumed. + AssumedByHub = "targets.yml" + // AssumedByFlag: plan --assume-opt-in, for the report only. + AssumedByFlag = "--assume-opt-in" +) + // ReasonPrivate is the skip reason of a non-public target of a public hub. // Such a target is only counted: its report entry has no path and no // repository id, and no output names it: a public hub's CI logs are @@ -128,12 +138,18 @@ type DeliveryTarget struct { PR *PRRef `json:"pr,omitempty"` // Key is the content key of D, the hash of its (path, from, mode, to) // changes, "" when D is empty. - Key string `json:"key,omitempty"` - Packs []string `json:"packs,omitempty"` - Assumed bool `json:"opt_in_assumed,omitempty"` // plan --assume-opt-in took an empty opt-in file for a missing or unsafe one - Changes ChangeCounts `json:"changes"` - Orphaned []string `json:"orphaned,omitempty"` - Warnings []string `json:"warnings,omitempty"` + Key string `json:"key,omitempty"` + Packs []string `json:"packs,omitempty"` + // Assumed is set when the target was processed without an opt-in file, + // as if it had an empty one; AssumedBy says why: AssumedByHub (an entry + // of targets.yml with opt_in: assumed subscribes it) or AssumedByFlag + // (plan --assume-opt-in took an empty opt-in file for a missing or + // unsafe one). + Assumed bool `json:"opt_in_assumed,omitempty"` + AssumedBy string `json:"opt_in_assumed_by,omitempty"` + Changes ChangeCounts `json:"changes"` + Orphaned []string `json:"orphaned,omitempty"` + Warnings []string `json:"warnings,omitempty"` // Writes is the estimated (plan) or actual (distribute) number of writes // for this target: HTTP writes of the API and git pushes, as the // throttle counts them, for the mutations that went diff --git a/internal/report/plan.go b/internal/report/plan.go index f024ac2..fe4990a 100644 --- a/internal/report/plan.go +++ b/internal/report/plan.go @@ -86,8 +86,9 @@ type ProviderEstimate struct { TotalSeconds int64 `json:"total_seconds,omitempty"` } -// planLines are the lines a plan's text output shows after the providers: -// its scope and --assume-opt-in; none for other reports. +// planLines are the lines the text output shows after the providers: a +// plan's scope and --assume-opt-in, and how many targets targets.yml +// subscribes without an opt-in file (opt_in: assumed). func (d *Delivery) planLines() []string { var out []string if s := d.Scope; s != nil { @@ -96,9 +97,28 @@ func (d *Delivery) planLines() []string { if d.Assumed { out = append(out, "--assume-opt-in: "+d.assumeText()) } + if text := d.subscribedText(); text != "" { + out = append(out, "opt_in: assumed: "+text) + } return out } +// subscribedText says how many targets without an opt-in file targets.yml +// subscribes (AssumedByHub); "" for none. +func (d *Delivery) subscribedText() string { + n := 0 + for _, t := range d.Targets { + if t.Assumed && t.AssumedBy == AssumedByHub { + n++ + } + } + if n == 0 { + return "" + } + return fmt.Sprintf("%s without an opt-in file %s opted in by targets.yml; an opt-in file with enabled: false opts one out", + plural(n, "target"), pluralVerb(n)) +} + // scopeText describes a plan's scope in one line. func scopeText(s *Scope) string { more := " (--all for every target)" @@ -124,16 +144,17 @@ func pluralWord(n int, word string) string { return word + "s" } -// assumeText describes --assume-opt-in: every target counts as opted in, -// for this report only. +// assumeText describes --assume-opt-in: for this report only, a target +// without an opt-in file (or with one that is not a regular file or is too +// large) counts as opted in; one whose file says enabled: false does not. func (d *Delivery) assumeText() string { n := 0 for _, t := range d.Targets { - if t.Assumed { + if t.Assumed && t.AssumedBy != AssumedByHub { n++ } } - return fmt.Sprintf("every target counts as opted in, for this report only; %s without an opt-in file %s planned as if %s had an empty one", + return fmt.Sprintf("for this report only, a target without an opt-in file counts as opted in (one whose file says enabled: false stays opted out); %s %s planned as if %s had an empty one", plural(n, "target"), pluralVerb(n), pluralPronoun(n)) } @@ -210,8 +231,8 @@ func approxDuration(seconds int64) string { return fmt.Sprintf("~%d h %d min", h, m) } -// mdPlanLines writes a plan's scope and --assume-opt-in to the Markdown -// output. +// mdPlanLines writes a plan's scope and --assume-opt-in, and the targets +// targets.yml subscribes, to the Markdown output. func (d *Delivery) mdPlanLines(m *mdWriter) { if s := d.Scope; s != nil { m.line("") @@ -221,6 +242,10 @@ func (d *Delivery) mdPlanLines(m *mdWriter) { m.line("") m.line("**" + mdText("--assume-opt-in") + ":** " + mdText(d.assumeText())) } + if text := d.subscribedText(); text != "" { + m.line("") + m.line("**" + mdText("opt_in: assumed") + ":** " + mdText(text)) + } } // Comment limits (plan --comment keeps one comment in the hub pull diff --git a/internal/report/plan_test.go b/internal/report/plan_test.go index 143b1cb..c13370a 100644 --- a/internal/report/plan_test.go +++ b/internal/report/plan_test.go @@ -37,7 +37,7 @@ func TestPlanLines(t *testing.T) { } for _, want := range []string{ "\nscope: pack python changed → 12 of 57 targets (--all for every target)\n", - "\n--assume-opt-in: every target counts as opted in, for this report only; 1 target without an opt-in file is planned as if it had an empty one\n", + "\n--assume-opt-in: for this report only, a target without an opt-in file counts as opted in (one whose file says enabled: false stays opted out); 1 target is planned as if it had an empty one\n", "\nThis hub pull request changes, across the targets it affects\n" + " update .claude/settings.json SENSITIVE 15 targets\n" + " add prompts/review.md 12 targets\n" + @@ -51,7 +51,7 @@ func TestPlanLines(t *testing.T) { } for _, want := range []string{ "\n**Scope:** pack python changed → 12 of 57 targets \\(\\-\\-all for every target\\)\n", - "\n**\\-\\-assume\\-opt\\-in:** every target counts as opted in", + "\n**\\-\\-assume\\-opt\\-in:** for this report only, a target without an opt\\-in file counts as opted in", "\n**Cost:** gh ≈ 400 writes \\(\\~45 min\\)", "
This hub pull request changes, across the targets it affects (3)\n\n| Change | Path | Targets |\n|---|---|---:|\n" + "| update ⚠ sensitive | `.claude/settings.json` | 15 |\n| add | `prompts/review.md` | 12 |\n| add | `docs/a\\|b.md` | 1 |\n", diff --git a/internal/report/report.go b/internal/report/report.go index 213f174..0b3f6bd 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -36,9 +36,28 @@ type Target struct { // when it could not be determined. Ref string `json:"ref"` OptInFile string `json:"opt_in_file"` - OptedIn bool `json:"opted_in"` + // OptedIn is set when the target gets packs: OptIn is OptInFile or + // OptInAssumed. + OptedIn bool `json:"opted_in"` + // OptIn says why the target is opted in or not: one of the OptIn* + // values. + OptIn string `json:"opt_in"` } +// Values of Target.OptIn. +const ( + // OptInFile: the target has the opt-in file. + OptInFile = "file" + // OptInAssumed: the target has no opt-in file, and a repo: entry of + // targets.yml with opt_in: assumed subscribes it. + OptInAssumed = "assumed" + // OptInDisabled: the opt-in file says enabled: false. + OptInDisabled = "opted-out" + // OptInNone: the target has no opt-in file, and no repo: entry + // subscribes it. + OptInNone = "none" +) + // Selection is the resolved pack list. type Selection struct { Packs []string `json:"packs"` diff --git a/internal/report/text.go b/internal/report/text.go index 7f80b7f..f61167d 100644 --- a/internal/report/text.go +++ b/internal/report/text.go @@ -53,10 +53,17 @@ func (p *printer) table(rows [][]string) { func (s *Sync) WriteText(w io.Writer) error { p := &printer{w: w} p.linef("%s · hub %s · target %s", s.title(), hubLabel(s.Hub), targetLabel(s.Target)) - if !s.Target.OptedIn { + switch { + case s.Target.OptIn == OptInDisabled: + p.linef("opted out: %s says enabled: false", s.Target.OptInFile) + s.writeWarnings(p) + return p.err + case !s.Target.OptedIn: p.linef("not opted in: %s not found in %s", s.Target.OptInFile, s.Target.Root) s.writeWarnings(p) return p.err + case s.Target.OptIn == OptInAssumed: + p.linef("opted in by targets.yml (opt_in: assumed): %s not found in %s", s.Target.OptInFile, s.Target.Root) } if s.Selection != nil { p.linef("packs: %s", packsLine(s.Selection)) diff --git a/schemas/doctor.schema.json b/schemas/doctor.schema.json index 941717c..7e46c08 100644 --- a/schemas/doctor.schema.json +++ b/schemas/doctor.schema.json @@ -163,7 +163,7 @@ "type": "string" }, "skipped": { - "description": "Why the target was not checked: archived, disabled, empty, mirror, pending-deletion, prs-disabled, sha256, not-opted-in, unsafe-opt-in, or deferred: (the run could not check the target; an unknown check says why).", + "description": "Why the target was not checked: archived, disabled, empty, mirror, pending-deletion, prs-disabled, sha256, not-opted-in, opted-out, unsafe-opt-in, or deferred: (the run could not check the target; an unknown check says why).", "type": "string", "minLength": 1 }, diff --git a/schemas/hub.schema.json b/schemas/hub.schema.json index 3f994f1..a673bed 100644 --- a/schemas/hub.schema.json +++ b/schemas/hub.schema.json @@ -279,7 +279,7 @@ }, "type": { "$ref": "#/$defs/providerType" }, "url": { - "description": "Web URL of the instance. Default for github: https://github.com; for gitlab: https://gitlab.com. Required for gitea and forgejo.", + "description": "Web URL of the instance. Default for github: https://github.com; for gitlab: https://gitlab.com. Required for gitea and forgejo. A target targets.yml names by web URL belongs to the provider whose url it lies under.", "$ref": "#/$defs/url" }, "api_url": { diff --git a/schemas/opt-in.schema.json b/schemas/opt-in.schema.json index 6b06b63..c980bbd 100644 --- a/schemas/opt-in.schema.json +++ b/schemas/opt-in.schema.json @@ -2,7 +2,7 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "https://raw.githubusercontent.com/bedrock-python/touchmark/master/schemas/opt-in.schema.json", "title": "touchmark opt-in file", - "description": "The opt-in file of a target repository (.engineering-assets.yml by default). Its presence is the repository's consent; it may be empty. Format v1.", + "description": "The opt-in file of a target repository (.engineering-assets.yml by default). Its presence is the repository's consent, and its enabled: false the repository's refusal; it may be empty. Format v1.", "type": "object", "additionalProperties": false, "properties": { @@ -11,6 +11,10 @@ "type": "integer", "const": 1 }, + "enabled": { + "description": "false opts the repository out, even when the hub subscribes it (opt_in: assumed in targets.yml): it gets nothing, and the hub's open pull requests here are closed. Default: true.", + "type": ["boolean", "null"] + }, "packs": { "description": "Packs to add to those the hub assigns. Packs the hub assigns cannot be removed here.", "type": ["array", "null"], diff --git a/schemas/report.schema.json b/schemas/report.schema.json index 25281d2..e2ea28a 100644 --- a/schemas/report.schema.json +++ b/schemas/report.schema.json @@ -34,7 +34,7 @@ }, "scope": { "$ref": "#/$defs/scope" }, "assume_opt_in": { - "description": "plan --assume-opt-in: every target counts as opted in, for this report only.", + "description": "plan --assume-opt-in: for this report only, every target without an opt-in file, or whose opt-in file is not a regular file or is too large, counts as opted in; one whose opt-in file says enabled: false stays opted out.", "type": "boolean" }, "providers": { @@ -226,9 +226,13 @@ "items": { "type": "string" } }, "opt_in_assumed": { - "description": "plan --assume-opt-in planned the target as if it had an empty opt-in file: it has none, or one that is not a regular file.", + "description": "The target was processed as if it had an empty opt-in file: it has none, or, with plan --assume-opt-in, one that is not a regular file or is too large. opt_in_assumed_by says why.", "type": "boolean" }, + "opt_in_assumed_by": { + "description": "Why opt_in_assumed is set: targets.yml (an entry with opt_in: assumed selects the target, for real) or --assume-opt-in (plan's flag, for the report only).", + "enum": ["targets.yml", "--assume-opt-in"] + }, "changes": { "$ref": "#/$defs/changes" }, "orphaned": { "description": "Files a pack the target no longer gets shipped, still unchanged in the target.", @@ -275,7 +279,7 @@ "if": { "properties": { "outcome": { "const": "skipped" } } }, "then": { "required": ["reason"], - "properties": { "reason": { "enum": ["not-opted-in", "archived", "disabled", "empty", "mirror", "pending-deletion", "prs-disabled", "sha256", "unsafe-opt-in", "private-in-public-hub", "superseded"] } } + "properties": { "reason": { "enum": ["not-opted-in", "opted-out", "archived", "disabled", "empty", "mirror", "pending-deletion", "prs-disabled", "sha256", "unsafe-opt-in", "private-in-public-hub", "superseded"] } } } }, { diff --git a/schemas/targets.schema.json b/schemas/targets.schema.json index 2d3a29f..e09ffba 100644 --- a/schemas/targets.schema.json +++ b/schemas/targets.schema.json @@ -30,6 +30,10 @@ "packs": { "description": "Packs every target gets, before the packs of its entries and its opt-in file.", "$ref": "#/$defs/packList" + }, + "opt_in": { + "description": "The opt_in of every entry that sets none: required (the default) or assumed.", + "$ref": "#/$defs/optIn" } } }, @@ -39,9 +43,9 @@ "items": { "$ref": "#/$defs/entry" } }, "exclude": { - "description": "Repositories never delivered to, even when an entry selects them.", + "description": "Repositories never delivered to, even when an entry selects them: a repository as repo names one, or a pattern of repository paths in which * matches within one segment, ** any number of segments and ? one character (corp:platform/legacy/** covers a subgroup at any depth), compared ignoring case. A web URL works too, with * and ** but not ?. Quote a pattern that starts with *, which YAML reads as an alias.", "type": ["array", "null"], - "items": { "$ref": "#/$defs/ref" } + "items": { "$ref": "#/$defs/excludeEntry" } } } }, @@ -71,11 +75,12 @@ "required": ["repo"], "properties": { "repo": { - "description": "One repository: owner/name or group/sub/project, optionally prefixed with a provider id and a colon.", - "$ref": "#/$defs/ref" + "description": "One repository: owner/name or group/sub/project, optionally prefixed with a provider id and a colon, or its web URL, such as https://gitlab.example.com/platform/api, whose provider is the one hub.yml lists at that url.", + "$ref": "#/$defs/repo" }, "provider": { "$ref": "#/$defs/providerId" }, - "packs": { "$ref": "#/$defs/packList" } + "packs": { "$ref": "#/$defs/packList" }, + "opt_in": { "$ref": "#/$defs/optIn" } } }, "orgEntry": { @@ -84,14 +89,16 @@ "required": ["org"], "properties": { "org": { - "description": "Every repository in a GitHub organisation or a GitLab, Gitea or Forgejo namespace (a synonym of group).", + "description": "Every repository in a GitHub organisation or a GitLab, Gitea or Forgejo namespace (a synonym of group), named by its path or its web URL.", "$ref": "#/$defs/namespace" }, "provider": { "$ref": "#/$defs/providerId" }, "topics": { "$ref": "#/$defs/topics" }, "subgroups": { "$ref": "#/$defs/subgroups" }, "forks": { "$ref": "#/$defs/forks" }, - "packs": { "$ref": "#/$defs/packList" } + "match": { "$ref": "#/$defs/match" }, + "packs": { "$ref": "#/$defs/packList" }, + "opt_in": { "$ref": "#/$defs/optIn" } } }, "groupEntry": { @@ -100,14 +107,30 @@ "required": ["group"], "properties": { "group": { - "description": "Every repository in a GitLab group, with its subgroups unless subgroups is false (a synonym of org).", + "description": "Every repository in a GitLab group, with its subgroups unless subgroups is false (a synonym of org), named by its path or its web URL.", "$ref": "#/$defs/namespace" }, "provider": { "$ref": "#/$defs/providerId" }, "topics": { "$ref": "#/$defs/topics" }, "subgroups": { "$ref": "#/$defs/subgroups" }, "forks": { "$ref": "#/$defs/forks" }, - "packs": { "$ref": "#/$defs/packList" } + "match": { "$ref": "#/$defs/match" }, + "packs": { "$ref": "#/$defs/packList" }, + "opt_in": { "$ref": "#/$defs/optIn" } + } + }, + "optIn": { + "description": "required (the default): a repository the entry selects gets nothing until it adds the opt-in file. assumed: it counts as opted in without one, with the packs targets.yml gives it, until its opt-in file says enabled: false. One entry with assumed is enough to subscribe a repository.", + "enum": ["required", "assumed"] + }, + "match": { + "description": "Only repositories whose full path, without the provider, matches at least one of these patterns: * within one segment, ** any number of segments, ? one character, compared ignoring case, such as acme/svc-* or platform/**/api. Quote a pattern that starts with *, which YAML reads as an alias.", + "type": ["array", "null"], + "minItems": 1, + "items": { + "type": "string", + "maxLength": 512, + "pattern": "^([A-Za-z0-9_*?-][A-Za-z0-9_.*?-]*|\\.[A-Za-z0-9_*?-][A-Za-z0-9_.*?-]*|\\.\\.[A-Za-z0-9_.*?-]+)(/([A-Za-z0-9_*?-][A-Za-z0-9_.*?-]*|\\.[A-Za-z0-9_*?-][A-Za-z0-9_.*?-]*|\\.\\.[A-Za-z0-9_.*?-]+))+$" } }, "topics": { @@ -142,10 +165,29 @@ "maxLength": 512, "pattern": "^([a-z][a-z0-9-]{0,31}:)?([A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.[A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.\\.[A-Za-z0-9_.-]+)(/([A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.[A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.\\.[A-Za-z0-9_.-]+))+$" }, + "repo": { + "type": "string", + "maxLength": 512, + "anyOf": [ + { "pattern": "^([a-z][a-z0-9-]{0,31}:)?([A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.[A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.\\.[A-Za-z0-9_.-]+)(/([A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.[A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.\\.[A-Za-z0-9_.-]+))+$" }, + { "pattern": "^(https://[A-Za-z0-9.-]+(:[0-9]{1,5})?|http://(localhost|127\\.0\\.0\\.1|\\[::1\\])(:[0-9]{1,5})?)(/([A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.[A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.\\.[A-Za-z0-9_.-]+)){2,}/?$" } + ] + }, "namespace": { "type": "string", "maxLength": 512, - "pattern": "^([a-z][a-z0-9-]{0,31}:)?([A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.[A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.\\.[A-Za-z0-9_.-]+)(/([A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.[A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.\\.[A-Za-z0-9_.-]+))*$" + "anyOf": [ + { "pattern": "^([a-z][a-z0-9-]{0,31}:)?([A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.[A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.\\.[A-Za-z0-9_.-]+)(/([A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.[A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.\\.[A-Za-z0-9_.-]+))*$" }, + { "pattern": "^(https://[A-Za-z0-9.-]+(:[0-9]{1,5})?|http://(localhost|127\\.0\\.0\\.1|\\[::1\\])(:[0-9]{1,5})?)(/([A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.[A-Za-z0-9_-][A-Za-z0-9_.-]*|\\.\\.[A-Za-z0-9_.-]+))+/?$" } + ] + }, + "excludeEntry": { + "type": "string", + "maxLength": 512, + "anyOf": [ + { "pattern": "^([a-z][a-z0-9-]{0,31}:)?([A-Za-z0-9_*?-][A-Za-z0-9_.*?-]*|\\.[A-Za-z0-9_*?-][A-Za-z0-9_.*?-]*|\\.\\.[A-Za-z0-9_.*?-]+)(/([A-Za-z0-9_*?-][A-Za-z0-9_.*?-]*|\\.[A-Za-z0-9_*?-][A-Za-z0-9_.*?-]*|\\.\\.[A-Za-z0-9_.*?-]+))+$" }, + { "pattern": "^(https://[A-Za-z0-9.-]+(:[0-9]{1,5})?|http://(localhost|127\\.0\\.0\\.1|\\[::1\\])(:[0-9]{1,5})?)(/([A-Za-z0-9_*-][A-Za-z0-9_.*-]*|\\.[A-Za-z0-9_*-][A-Za-z0-9_.*-]*|\\.\\.[A-Za-z0-9_.*-]+)){2,}/?$" } + ] } } }