diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index df2058c..c42f9ad 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -61,8 +61,8 @@ practice rather than asserted from habit — see Further reading. Same testability goal as #3, applied to the specific ways this program reaches outside itself. A consistent shape beats ad hoc mocking invented per call site. - **Already in force**: `GithubEndpoints` (checker/fetcher/verifier), - `repo_add_bin` and the pacman.conf path (publisher), `PKGWATCH_REPO_DIR` + **Already in force**: `source::Endpoints` (checker), the plain API root + it hands to fetcher/verifier, `repo_add_bin` and the pacman.conf path (publisher), `PKGWATCH_REPO_DIR` (main, for manual dry runs against a scratch repo instead of the real one). A new external call follows the same shape: production code calls a thin wrapper with the real default; tests call the parameterized diff --git a/docs/SPEC.md b/docs/SPEC.md index 842d338..220d05c 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -382,18 +382,14 @@ Open questions on the schema: GitHub's confirm-each-tag step. `check_interval`/per-package cadence not wired up yet — checks are a fixed hourly tick.)* - Both hosts' HTTP is hand-rolled on the `reqwest` already in the tree, - not an API-client crate: what pkgwatch needs is two `GET`s - (latest-release, release-by-tag), GitHub's check deliberately uses the - Atom feed that no API crate covers (to stay off the rate-limited REST - API), and the release-by-tag call is shared verbatim between the two - hosts, which two per-host crates would split in two. `octocrab` is - async/tokio/hyper against this project's blocking `reqwest`, and its - default tree alone (217 crates) is larger than all of pkgwatch's today - (143); `forgejo-api` has a `sync` feature but is a generated binding of - the whole Forgejo API for one endpoint. Revisit if pkgwatch ever needs - authenticated or write API calls (e.g. publishing its own releases from - code rather than CI). + The HTTP is hand-rolled on the `reqwest` already in the tree, not an + API-client crate: pkgwatch needs two `GET`s, GitHub's check deliberately + uses an Atom feed no API crate covers (to stay off the rate-limited REST + API), and the release-by-tag call is shared verbatim by both hosts. + `octocrab` is async against our blocking `reqwest` with a default tree + larger than pkgwatch's whole current one; `forgejo-api` is a generated + binding of the entire API for one endpoint. Revisit if pkgwatch needs + authenticated or write API calls. - **Fetcher**: downloads the artifact (and any checksum/signature/ attestation companion) for a resolved version. *(Implemented — `src/fetcher.rs`, via the GitHub or Forgejo releases API — same diff --git a/src/config.rs b/src/config.rs index 5123dd7..3119eaa 100644 --- a/src/config.rs +++ b/src/config.rs @@ -34,7 +34,7 @@ pub struct Package { /// (the API lives under `/api/v1`). Required for, and only meaningful /// with, `source = "forgejo-release"`. pub base_url: Option, - /// Exact GitHub release asset name (still not a glob — see + /// Exact release asset name (still not a glob — see /// docs/SPEC.md > Architecture > Fetcher), optionally containing a /// `{version}` placeholder for projects whose asset names embed the /// version (e.g. `scaleway-cli_{version}_linux_amd64`). Substituted via diff --git a/src/pipeline.rs b/src/pipeline.rs index c9f523e..88fa9e7 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -347,6 +347,7 @@ fn approve(state_dir: &Path, work_dir: &Path, name: &str, pkg: &Package, tag: &s mod tests { use super::*; use crate::github::GithubEndpoints; + use crate::test_support::same_origin_package; #[test] fn decide_tier_action_failed_verification_overrides_everything() { @@ -414,22 +415,6 @@ mod tests { ] } - /// `pkg_toml_extra` is spliced in before the `[verification]` table, so - /// it can carry top-level keys like `source`. - fn same_origin_package(pkg_toml_extra: &str) -> Package { - toml::from_str(&format!( - r#" - repo = "o/r" - asset_pattern = "thing.tar.gz" - {pkg_toml_extra} - [verification] - method = "same-origin-sha256" - checksum_asset_pattern = "SHA256SUMS" - "# - )) - .unwrap() - } - /// Runs `process_package` for a package whose checksum is wrong and /// asserts it errors with "verification failed" while leaving no trace /// in state. @@ -497,9 +482,8 @@ mod tests { .create(); let _release_mocks = mock_release_with_bad_checksum(&mut server); - let pkg = same_origin_package( - "source = \"forgejo-release\"\nbase_url = \"https://forge.example.com\"", - ); - assert_verification_failure_is_an_error(&endpoints, &pkg); + // The package's own `source` is irrelevant here: `endpoints` is + // hand-built to point at the mock server, bypassing `for_package`. + assert_verification_failure_is_an_error(&endpoints, &same_origin_package("")); } } diff --git a/src/source.rs b/src/source.rs index 03bd059..c014dba 100644 --- a/src/source.rs +++ b/src/source.rs @@ -1,6 +1,6 @@ //! Maps a package's configured `source` to the endpoints the pipeline -//! stages talk to. The only module that knows how each hosting service -//! lays out its URLs; `checker` and `fetcher` take what it hands them. +//! stages talk to. `checker` and `fetcher` take what it hands them and +//! build their own paths under it. use crate::config::{Package, Source}; use crate::github::GithubEndpoints; @@ -16,10 +16,8 @@ pub enum Endpoints { } impl Endpoints { - /// Errors only if a `forgejo-release` package has no `base_url`, which - /// `config::load_packages_dir` already rejects — this is the same check - /// again for a `Package` built some other way, not a second source of - /// truth. + /// Defensive: errors only if a `forgejo-release` package has no + /// `base_url`, which `config::load_packages_dir` already guarantees. pub fn for_package(pkg: &Package) -> Result { match pkg.source { Source::GithubRelease => Ok(Endpoints::Github(GithubEndpoints::default())), @@ -50,20 +48,7 @@ impl Endpoints { #[cfg(test)] mod tests { use super::*; - - fn package(extra: &str) -> Package { - toml::from_str(&format!( - r#" - repo = "o/r" - asset_pattern = "x" - {extra} - [verification] - method = "same-origin-sha256" - checksum_asset_pattern = "SUMS" - "# - )) - .unwrap() - } + use crate::test_support::same_origin_package as package; #[test] fn github_source_uses_real_github() { diff --git a/src/test_support.rs b/src/test_support.rs index d53c80b..09d72f4 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -1,10 +1,11 @@ //! Test-only fixture helpers shared across modules' `#[cfg(test)]` code -//! (`publisher`, `sanity`, `notifier`) — not production code, and not built outside -//! `cargo test`. See docs/ARCHITECTURE.md > "organize by pipeline stage, not -//! by layer": this exists to remove one specific piece of duplication -//! (two near-identical copies of "write an executable shell script"), not -//! as a general test-utils dump. +//! — not production code, and not built outside `cargo test`. See +//! docs/ARCHITECTURE.md > "organize by pipeline stage, not by layer": this +//! exists to remove specific pieces of duplication (near-identical copies +//! of "write an executable shell script" and of "build a same-origin +//! `Package` from TOML"), not as a general test-utils dump. +use crate::config::Package; use std::path::{Path, PathBuf}; use std::process::Command; use std::time::{Duration, Instant}; @@ -52,3 +53,22 @@ fn wait_until_executable(path: &Path) { } } } + +/// A `same-origin-sha256` `Package` for repo `o/r`, asset `thing.tar.gz`, +/// checksum asset `SHA256SUMS`. `extra` is spliced in as top-level keys +/// before the `[verification]` table (e.g. `source`/`base_url`), and is +/// parsed directly rather than through `config::load_packages_dir`, so it +/// skips load-time validation. +pub(crate) fn same_origin_package(extra: &str) -> Package { + toml::from_str(&format!( + r#" + repo = "o/r" + asset_pattern = "thing.tar.gz" + {extra} + [verification] + method = "same-origin-sha256" + checksum_asset_pattern = "SHA256SUMS" + "# + )) + .unwrap() +}