diff --git a/.agents/skills/check-changes/SKILL.md b/.agents/skills/check-changes/SKILL.md index acfcd22..de74730 100644 --- a/.agents/skills/check-changes/SKILL.md +++ b/.agents/skills/check-changes/SKILL.md @@ -30,9 +30,30 @@ of three buckets it falls into: ## What a good entry says +**Twenty-five words at most per entry.** Count them, and count wrapped lines +too -- a bullet spilling over three lines is over the limit however it looks in +the file. An entry that needs more is either two entries or an explanation that +belongs elsewhere. + Say what changed for the reader, not what was edited. "Rejects a request path containing `..`" tells an operator something; "hardened `join_upstream`" does -not. Where a behaviour changed rather than appeared, say what it was before. +not. + +Add as few entries as the change honestly needs. A changelog is scanned, not +read: every line that could have been left out costs the reader attention on the +lines that could not. + +Where to put what the fifteen words cannot hold: + +| The reasoning, the measurements, the rejected alternative | the commit message | +| How the thing works and how to configure it | `docs/` | +| Why the code is shaped that way | a comment next to it | + +None of that belongs in `CHANGES.md`. A reader wanting it has `git log` and the +documentation; a reader wanting to know whether to care has one line. + +Where a behaviour changed rather than appeared, one clause on what it was before +is worth the words -- inside the fifteen, not in addition to them. Keep the existing Added / Changed / Fixed / Notes grouping. Entries are written as part of the change that caused them; if you are adding several at once @@ -47,3 +68,7 @@ diff when the subject is not enough. Do not treat a green checklist as the goal. If the Development section is accurate and short because little user-visible changed, say so. + +Do not restore length that was cut. An entry trimmed to fifteen words has not +lost anything a reader of a changelog wanted: check that what was cut is +recorded in the commit message or the documentation, and leave the entry short. diff --git a/.agents/skills/pre-release-check/SKILL.md b/.agents/skills/pre-release-check/SKILL.md index 2c036d5..dd3a004 100644 --- a/.agents/skills/pre-release-check/SKILL.md +++ b/.agents/skills/pre-release-check/SKILL.md @@ -9,18 +9,23 @@ A gate, not a fixer. Anything it finds gets fixed in its own commit before the release proceeds; do not fold repairs into the release commit, which should stay a mechanical, reviewable change. -## Run these four first +## Run these five first In this order, because a later one is pointless if an earlier one fails: 1. `run-tests-and-linters` -- the workspace is green, with captured output. 2. `check-licenses` -- every direct dependency is compliant and justified. -3. `check-changes` -- the Development section reflects what landed. -4. `check-docs` -- the reference, CLI surface, error codes and rule table match +3. `regenerate-config-schema` -- `doppel-config.schema.json` matches the types + and every field still describes itself. It is a release asset and is what + editors fetch, so a stale one ships. +4. `check-changes` -- the Development section reflects what landed. +5. `check-docs` -- the reference, CLI surface, error codes and rule table match the code. `bump-version` is deliberately not in this list. This skill checks; that one -changes things. +changes things. `regenerate-config-schema` is the one exception, and only +because the thing it writes is generated: if it produces a diff, that diff is a +commit of its own before the release, not part of the release commit. ## Then check the version diff --git a/.agents/skills/regenerate-config-schema/SKILL.md b/.agents/skills/regenerate-config-schema/SKILL.md new file mode 100644 index 0000000..90ed2bd --- /dev/null +++ b/.agents/skills/regenerate-config-schema/SKILL.md @@ -0,0 +1,95 @@ +--- +name: regenerate-config-schema +description: Use after adding, removing or renaming any field in crates/doppel-core/src/config/, and before cutting a release. Regenerates doppel-config.schema.json and checks that every field still describes itself. +--- + +# Regenerate the configuration schema + +`doppel-config.schema.json` at the repository root is generated, checked in, +attached to every release, and fetched by editors through a +`yaml-language-server` modeline. A stale copy is worse than none: it reports +mistakes that are not mistakes and accepts fields that no longer exist. + +## Regenerate + +```bash +uv run scripts/config_schema.py +uv run scripts/config_schema.py --check +``` + +The script runs `doppel config schema` and writes its output. Python tooling +here is driven by `uv`, never `pip`. + +Two things already fail when the checked-in copy falls behind, so this skill is +about the cases they cannot see: + +- `cargo test -p doppel-core --lib config::schema` compares the file to what the + code produces, for whoever runs the suite locally; +- a CI step runs `--check`, so a forgotten regeneration cannot merge. + +## Where the schema comes from + +The same `utoipa::ToSchema` derives the admin API's OpenAPI document uses. There +is no second description of the types to keep in step, and that is the point -- +so **do not** add `schemars`, a hand-written schema, or a second derive set. + +A new type reachable from `Config` needs `utoipa::ToSchema` on it or the build +fails. Two standard-library types have no `utoipa` schema and are annotated +where they appear: `IpAddr` and `PathBuf` both carry +`#[schema(value_type = String)]`. + +## Every field must describe itself + +The schema is read in an editor. A field with no description is a tooltip that +says nothing, which is the whole reason it is generated from doc comments. + +```bash +cargo test -p doppel-core --lib every_field_carries_a_description +``` + +That test enumerates rather than samples, so a field added without a `///` fails +it. Write the doc comment for the person editing YAML, not for the person +reading Rust: what the field is for, its unit, its default, and what happens +when it is left out. + +`utoipa` puts a doc comment where you would not expect for an `Option` whose +`T` has its own schema: the description lands *inside* the `oneOf` branch beside +the `$ref` rather than on the property. `config::schema::hoist_descriptions` +lifts it back out. If a field's description goes missing from the generated +file, that hoist is the first place to look -- not the doc comment. + +## When the URL changes + +`config::schema::URL` is the `$id` and is also the URL in +`main.example.yaml`'s modeline. They are compared by +`the_example_configs_modeline_names_this_url`, so changing one without the other +fails. It points at the raw file on `main` deliberately: a reader copies that +line once and keeps it, and a version-pinned URL would leave them validating +next year's configuration against an old schema. The per-release asset is there +for anyone who wants the pin. + +## Check what it actually rejects + +The generated file being current says nothing about it being useful. Validate a +document against it, and include a mistake: + +```bash +uv run --with jsonschema --with pyyaml python - <<'EOF' +import json, yaml, jsonschema +schema = json.load(open("doppel-config.schema.json")) +jsonschema.Draft202012Validator.check_schema(schema) +doc = yaml.safe_load(open("main.example.yaml")) +print("example errors:", len(list(jsonschema.Draft202012Validator(schema).iter_errors(doc)))) +doc["proxies"][0]["loss"]["percentage"] = 45 # a fraction was meant +print("with a bad percentage:", len(list(jsonschema.Draft202012Validator(schema).iter_errors(doc)))) +EOF +``` + +The first count has to be zero and the second has to not be. A schema that +accepts everything passes every other check in this file. + +## Report + +Say whether the file changed, and name the fields whose descriptions you added +or reworded. "Regenerated, no diff" is a useful result; "the schema is fine" is +not. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3c82fa0..df46591 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -91,6 +91,25 @@ jobs: - name: Test workspace run: cargo test --workspace + # Pinned to an exact release: astral-sh publishes `v9.0.0` but stopped + # publishing the sliding major tag after `v7`, so `@v9` resolves to + # nothing and the job fails before it runs. + - name: Install uv + uses: astral-sh/setup-uv@v9.0.0 + + # After the tests, so the workspace is already compiled and this only has + # to run the binary. A stale schema is worse than none -- editors report + # mistakes that are not mistakes and accept fields that no longer exist -- + # and it is generated, so nothing but a check catches a forgotten + # regeneration. + # + # `cargo test` covers the same ground through a drift test in + # `config::schema`, deliberately: that one fails for whoever runs the + # suite locally, this one names the fix in a step whose title says what + # broke. + - name: Configuration schema is up to date + run: uv run scripts/config_schema.py --check + docs: name: Documentation builds runs-on: ubuntu-latest diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 86b6156..afe9811 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -326,13 +326,25 @@ jobs: # flatten them into one directory. merge-multiple: true + # Attached so a configuration can be validated against the exact release + # it will run under, rather than against whatever `main` says today. The + # checked-in file is taken as-is: CI has already refused a stale one + # (`scripts/config_schema.py --check`), so regenerating here would only + # add a Rust build to a job that otherwise needs none. + - name: Stage the configuration schema + run: cp doppel-config.schema.json dist/ + - name: Write checksums run: | set -euo pipefail cd dist # Relative names, so `shasum -c` works from whatever directory the # verifier downloaded into. - sha256sum -- *.tar.gz > checksums.txt + # + # The schema is signed alongside the archives: it is a release asset + # people will fetch over the network, and one that could be swapped + # without anyone noticing is worth as little as an unsigned binary. + sha256sum -- *.tar.gz doppel-config.schema.json > checksums.txt cat checksums.txt - name: Import the release key diff --git a/CHANGES.md b/CHANGES.md index b470639..d4a8996 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,58 +8,50 @@ release promotes it to a version heading; the `bump-version` skill does that. ## Development +## 0.4.0 -- 2026-08-11 + +### Added + +- A JSON Schema for the configuration, checked in and attached to each release. +- `doppel config schema` prints it; every field carries a description. + +### Changed + +- An empty or absent `proxies` list is accepted; requests get `503 + NO_PROXIES_CONFIGURED`. +- `type: tcp` is now refused while parsing rather than by a validation rule. +- `admin.groups` bounds which names `access` may reference; rule V36 checks it. +- Names may no longer contain `.`, and are capped at 64 characters, 32 for a + proxy. + +## 0.3.0 -- 2026-08-10 + +### Added + +- `X-Forwarded-Host` and `X-Forwarded-Proto` are now sent upstream. +- `proxies[].rewrite_redirects`, default `true`. + ### Changed -- The documentation site is versioned with `mike`: one built copy per release on - the `gh-pages` branch, a switcher in the header, and the site root redirecting - to the newest release. It was a single unversioned site, so a reader on 0.1.0 - had no way to reach the documentation for the release they were running, and - publishing 0.2.0 silently replaced it. Pre-release tags publish nothing; a push - to `main` publishes `dev`. +- A redirect into the proxied space now points back at Doppel, not the upstream. +- The documentation site is versioned with `mike`, one built copy per release. ## 0.2.0 -- 2026-08-03 ### Changed -- A matching mock is now decided before `loss` and `latency`, not after, so - `replace` is the share of matching requests a mock answers rather than the - share of those that survived a loss roll. Previously `loss: 0.5` halved every - `replace` in the proxy, and no configuration could ask a mock to answer half - of its matching requests while any loss was set. The proxy's `loss` and - `latency` no longer apply to a request a mock answered: they describe the real - backend, and a mock replaces it. -- A run of slashes at the start of a request path is collapsed to one before - mocks are matched, so `//api/v1/index/` matches a mock declared - `^/api/v1/index/$`. Clients produce the doubled form by joining a base URL - ending in `/` to a path beginning with `/`; it is legal HTTP, nothing rejected - it, and the only symptom was an anchored mock silently not firing. Empty - segments elsewhere in the path are left alone. -- An injected `latency` is now a target for the whole response rather than an - addition to it: the time the upstream really took is subtracted, and only the - remainder is waited out. A 500ms latency in front of a backend answering in - 120ms delays by 380ms, where before it delayed by 500 and produced 620ms - total -- so the number written in the configuration was unreachable by - construction, and moved with whatever the upstream happened to be doing. An - upstream slower than the target leaves no remainder and is passed straight - through; the setting is a floor, never a ceiling. `latency_injected_ms` in the - log line is the wait actually taken and reads `0` in that case, while - `doppel_latency_injected_total` still counts the request. +- A matching mock is decided before `loss` and `latency`, so `replace` no longer + shrinks with loss. +- The proxy's `loss` no longer applies to a request a mock answered; its + `latency` still does. +- Leading slashes in a request path are collapsed before mocks are matched. +- An injected `latency` is a target for the whole response: the upstream's real + time is subtracted. ### Fixed -- A mock's `proxy.loss` and `proxy.latency` are applied. They were parsed, - validated and compiled into the runtime, and then never read, so a mock - declaring either was silently answering every request it matched. They now - apply to the requests the mock answers, after it has won its `replace` roll, - and go through the same `decide` as the proxy's -- so loss short-circuits - latency there too. -- What a mock inherits from its proxy is now settled per setting rather than by - accident: `replace` and `latency` fall back to the proxy's, `loss` does not. - `latency` describes how slow the proxy is to answer, which holds whatever - answers, so a mocked response is delayed like any other and a mock's own value - overrides rather than adds to it. `loss` is excluded because a mock inheriting - it would be dropped by the proxy's loss, which is the coupling between `loss` - and `replace` the ordering above exists to remove. +- A mock's own `proxy.loss` and `proxy.latency` are applied; they were parsed and + then ignored. ## 0.1.0 -- 2026-08-02 diff --git a/Cargo.lock b/Cargo.lock index 3b86671..d278c41 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -591,7 +591,7 @@ dependencies = [ [[package]] name = "doppel-admin" -version = "0.2.0" +version = "0.4.0" dependencies = [ "async-trait", "axum", @@ -610,7 +610,7 @@ dependencies = [ [[package]] name = "doppel-cli" -version = "0.2.0" +version = "0.4.0" dependencies = [ "anyhow", "clap", @@ -631,7 +631,7 @@ dependencies = [ [[package]] name = "doppel-core" -version = "0.2.0" +version = "0.4.0" dependencies = [ "arc-swap", "async-trait", @@ -654,7 +654,7 @@ dependencies = [ [[package]] name = "doppel-proxy" -version = "0.2.0" +version = "0.4.0" dependencies = [ "axum", "doppel-core", @@ -672,7 +672,7 @@ dependencies = [ [[package]] name = "doppel-render" -version = "0.2.0" +version = "0.4.0" dependencies = [ "doppel-core", "minijinja", @@ -681,7 +681,7 @@ dependencies = [ [[package]] name = "doppel-store-postgres" -version = "0.2.0" +version = "0.4.0" dependencies = [ "async-trait", "doppel-core", @@ -696,7 +696,7 @@ dependencies = [ [[package]] name = "doppel-telemetry" -version = "0.2.0" +version = "0.4.0" dependencies = [ "doppel-core", "sentry", diff --git a/Cargo.toml b/Cargo.toml index 36e5845..e4d2c00 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = ["crates/*"] resolver = "3" [workspace.package] -version = "0.2.0" +version = "0.4.0" edition = "2024" rust-version = "1.94" license = "Apache-2.0" diff --git a/crates/doppel-admin/tests/proxies.rs b/crates/doppel-admin/tests/proxies.rs index 51adf84..922d29f 100644 --- a/crates/doppel-admin/tests/proxies.rs +++ b/crates/doppel-admin/tests/proxies.rs @@ -435,25 +435,28 @@ async fn delete_drops_the_proxys_templates() { assert!(kept.exists(), "another proxy's templates must survive"); } +/// Rule V5 used to refuse this, so emptying a Doppel over the API meant +/// deleting every proxy but one and then editing the file by hand. An empty +/// proxy list is a legal configuration now: the delete goes through, and a +/// request afterwards is answered `503 NO_PROXIES_CONFIGURED` rather than the +/// deletion being blocked to keep that from happening. #[tokio::test] -async fn deleting_the_last_proxy_is_refused_by_validation() { +async fn deleting_the_last_proxy_is_allowed_and_leaves_none() { let (only_alpha, _) = common::BASE_CONFIG .split_once(" - name: beta") .expect("BASE_CONFIG defines beta"); let harness = Harness::with_config(only_alpha); + harness.write_template("alpha", "body.json.j2", "{}"); + let reply = Call::delete("/api/v1/proxies/alpha") .token(ROOT) .send(harness.router()) .await; - // The rule set runs on the result of every write, delete included: a - // configuration with no proxies is not one this process can serve. - assert_eq!(reply.status, 400, "{}", reply.body); - assert_eq!(reply.error_code(), "CONFIG_INVALID"); - assert_eq!(harness.stored().proxies.len(), 1); - // The templates of a proxy whose deletion was refused must still be - // there: the config write is what authorises dropping them. - assert!(harness.templates_dir.exists()); + assert_eq!(reply.status, 204, "{}", reply.body); + assert!(harness.stored().proxies.is_empty()); + // The write authorises dropping them, and the write happened. + assert_absent(&harness.template_path("alpha", "body.json.j2")); } #[tokio::test] diff --git a/crates/doppel-cli/src/cli.rs b/crates/doppel-cli/src/cli.rs index dce909c..66059bf 100644 --- a/crates/doppel-cli/src/cli.rs +++ b/crates/doppel-cli/src/cli.rs @@ -83,6 +83,13 @@ pub enum ConfigCommand { /// alters a shared schema when it boots turns a rollback into data loss, /// and the operator who rolled back is the one least expecting it. Migrate(MigrateArgs), + /// Print the configuration's JSON Schema on stdout. + /// + /// Takes no arguments and reads nothing: the schema describes the shape a + /// configuration may have, which is a property of this binary rather than + /// of any file, so pointing it at a `--config` would only invite the + /// question of whether the answer depended on it. + Schema, } #[derive(Args)] diff --git a/crates/doppel-cli/src/main.rs b/crates/doppel-cli/src/main.rs index 8bb8003..221d9e7 100644 --- a/crates/doppel-cli/src/main.rs +++ b/crates/doppel-cli/src/main.rs @@ -40,6 +40,14 @@ fn run(command: Command) -> u8 { Command::Config { command: ConfigCommand::Reload(args), } => run_on_light_runtime(async move { commands::reload::reload(&args).await }), + // No runtime: this reads no file, opens no socket and touches no + // database, so there is nothing to await. + Command::Config { + command: ConfigCommand::Schema, + } => { + print!("{}", doppel_core::config::schema::json_schema_document()); + 0 + } Command::Config { command: ConfigCommand::Push(args), } => run_on_light_runtime(async move { report(commands::transfer::push(&args).await) }), diff --git a/crates/doppel-cli/tests/migrate.rs b/crates/doppel-cli/tests/migrate.rs index 5b054c2..e8460b1 100644 --- a/crates/doppel-cli/tests/migrate.rs +++ b/crates/doppel-cli/tests/migrate.rs @@ -49,11 +49,22 @@ async fn a_migrated_database_reports_its_version_and_exits_zero() { let (code, text) = status(&schema.url()); assert_eq!(code, 0, "{text}"); - assert!(text.contains("schema version 1"), "{text}"); assert!(text.contains("up to date"), "{text}"); - // The version, not the row count, is what identifies the schema. Both - // happen to be 1 today, so the assertion has to name the word. - assert!(text.contains("schema version"), "{text}"); + + // Read from the embedded migrations rather than written down. Hardcoding + // `1` here meant the first migration ever added broke this test for no + // reason -- it is about `--status` reporting the version it found, not about + // which version that happens to be today. + // + // The version, not the count, is what identifies the schema. They were the + // same number while there was one migration, so the assertion names the + // word to stay honest once they diverge. + let newest = doppel_store_postgres::MIGRATOR + .iter() + .map(|m| m.version) + .max() + .expect("the crate embeds at least one migration"); + assert!(text.contains(&format!("schema version {newest}")), "{text}"); schema.drop().await; } diff --git a/crates/doppel-core/src/config/admin.rs b/crates/doppel-core/src/config/admin.rs index 61cd14b..eec143c 100644 --- a/crates/doppel-core/src/config/admin.rs +++ b/crates/doppel-core/src/config/admin.rs @@ -5,7 +5,93 @@ use std::net::IpAddr; use serde::de::{self, SeqAccess, Visitor}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +/// A name `access` is allowed to reference, or `*` for any of them. +/// +/// Its own type rather than a `String`, because `*` and a name are different +/// things and that difference is the whole content of the setting. Not a `Name` +/// either: `*` is not a legal name and should never become one. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AllowedGroup { + /// `*`: any group or token name may be referenced. + Any, + /// Exactly this name may be referenced. + Named(super::Name), +} + +impl AllowedGroup { + /// Whether this entry permits `name`. + #[must_use] + pub fn permits(&self, name: &str) -> bool { + match self { + Self::Any => true, + Self::Named(allowed) => allowed == name, + } + } +} + +impl std::fmt::Display for AllowedGroup { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + Self::Any => f.write_str("*"), + Self::Named(name) => f.write_str(name.as_str()), + } + } +} + +impl Serialize for AllowedGroup { + fn serialize(&self, s: S) -> Result { + match self { + Self::Any => s.serialize_str("*"), + Self::Named(name) => s.serialize_str(name.as_str()), + } + } +} + +impl<'de> Deserialize<'de> for AllowedGroup { + fn deserialize>(d: D) -> Result { + let value = String::deserialize(d)?; + if value == "*" { + return Ok(Self::Any); + } + super::Name::parse(value) + .map(Self::Named) + .map_err(de::Error::custom) + } +} + +impl utoipa::PartialSchema for AllowedGroup { + fn schema() -> utoipa::openapi::RefOr { + utoipa::openapi::schema::ObjectBuilder::new() + .schema_type(utoipa::openapi::schema::Type::String) + // `*` or a name, built from the name rules rather than restating + // them: without a pattern here the schema accepts `"not a name!"`, + // and an editor's whole job is to say so before Doppel is run. + .pattern(Some(format!( + r"^(\*|{}{{{},{}}})$", + super::name::CHARACTERS, + super::name::MIN, + super::name::MAX + ))) + .description(Some( + "A token or group name `access` may reference, or `*` for any of them.", + )) + .examples([serde_json::json!("*"), serde_json::json!("admin")]) + .into() + } +} + +impl utoipa::ToSchema for AllowedGroup {} + +/// What an absent `admin.groups` means: any name may be referenced. +/// +/// The permissive default is the opposite choice from `access` itself, on +/// purpose. `access` defaults to `admin` because the cost of getting it wrong is +/// unauthenticated writes. This defaults to `*` because the cost of getting it +/// wrong is an operator locked out of naming their own groups -- an allow-list +/// nobody asked for only ever surprises. +const ANY_GROUP: [AllowedGroup; 1] = [AllowedGroup::Any]; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct AdminConfig { /// Whether to run the admin listener at all. @@ -19,20 +105,73 @@ pub struct AdminConfig { /// listener on later, and they will not re-read the rules first. #[serde(default = "enabled")] pub enable: bool, + /// An IP address, not a hostname: a name would have to be resolved, + /// and which address it resolves to is not the configuration's to + /// decide. `utoipa` has no schema for `IpAddr`, so it is described + /// here as the string it is written as. + #[schema(value_type = String, examples("127.0.0.1"))] pub host: IpAddr, + /// The TCP port the admin API listens on. Must differ from + /// `server.port`. pub port: super::Port, + /// Which header carries the bearer token. #[serde(default)] pub auth: AuthConfig, + /// The tokens that may call the admin API. Names and token values are + /// each unique. `DOPPEL_ADMIN_TOKENS` can supply these instead. #[serde(default)] pub tokens: Vec, + /// Which names `access` may reference, here and in a proxy's overrides. + /// + /// `["*"]`, the default, allows any. A concrete list allows exactly those: + /// `["admin", "ci"]` permits `admin` and `ci` and refuses `user`. + /// + /// `[]` allows none, and that is stronger than it looks: every action + /// defaults to the `admin` group, so an empty list makes the defaults + /// themselves violations and every one of the six actions has to be written + /// out as `public`. + /// + /// `public` is never governed by this. It is the absence of a subject rather + /// than a name, so an allow-list has nothing to say about it. + /// + /// Checked by rule V36, not by this type: it compares one field against + /// another, which is what is left for the rule set once the types have taken + /// everything they can decide alone. + /// + /// An `Option` rather than a `Vec` defaulting to `["*"]`, and skipped when + /// absent, so that adding this field did not change the canonical YAML of + /// every configuration written before it existed. The revision is derived + /// from that YAML, so a materialised default would have made every stored + /// configuration fail its own revision check on the first load after the + /// upgrade. Read it through [`AdminConfig::allowed_groups`]. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub groups: Option>, + /// Who may perform each admin action. Every action defaults to the + /// `admin` group, reads included. #[serde(default)] pub access: AccessConfig, + /// Bounds an uploaded template file. pub upload: UploadConfig, } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +impl AdminConfig { + /// The names `access` may reference, with an absent `groups` resolved to its + /// default. + /// + /// One place resolves it, so no caller has to remember that absent and + /// `["*"]` mean the same thing while `[]` means the opposite -- which is + /// exactly the confusion an `Option>` invites. + #[must_use] + pub fn allowed_groups(&self) -> &[AllowedGroup] { + self.groups.as_deref().unwrap_or(&ANY_GROUP) + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct AuthConfig { + /// The header a caller presents its token in, as `Bearer `. + /// Defaults to `X-Proxy-Authorization`. #[serde(default = "default_auth_header")] pub header: super::HeaderName, } @@ -49,11 +188,17 @@ impl Default for AuthConfig { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct TokenConfig { + /// What to call this token in `access` lists and in logs. Never the + /// secret itself. pub name: super::Name, + /// The group it belongs to. `admin` and `user` are predefined; any other + /// name must be carried by at least one token. pub group: super::Name, + /// The secret the caller sends. A version 4 UUID is the recommended + /// shape. pub token: super::Token, } @@ -133,19 +278,25 @@ impl<'de> Deserialize<'de> for Subjects { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct AccessConfig { + /// List the proxies. A listing exposes upstream URLs and injected headers. #[serde(default = "admin_only")] pub list: Subjects, + /// Read one proxy document, credentials in its `url` included. #[serde(default = "admin_only")] pub read: Subjects, + /// Add a proxy. Refused for `public` by rule V34. #[serde(default = "admin_only")] pub create: Subjects, + /// Replace a proxy. Refused for `public` by rule V34. #[serde(default = "admin_only")] pub update: Subjects, + /// Remove a proxy. Refused for `public` by rule V34. #[serde(default = "admin_only")] pub delete: Subjects, + /// Upload or delete a template file. Refused for `public` by rule V34. #[serde(default = "admin_only")] pub upload: Subjects, } @@ -192,19 +343,25 @@ impl Default for AccessConfig { #[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct ProxyAccessConfig { + /// Who may read this proxy's document. Absent leaves the global rule. #[serde(default, skip_serializing_if = "Option::is_none")] pub read: Option, + /// Who may replace this proxy. Absent leaves the global rule. #[serde(default, skip_serializing_if = "Option::is_none")] pub update: Option, + /// Who may remove this proxy. Absent leaves the global rule. #[serde(default, skip_serializing_if = "Option::is_none")] pub delete: Option, + /// Who may upload templates for this proxy. Absent leaves the global rule. #[serde(default, skip_serializing_if = "Option::is_none")] pub upload: Option, } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct UploadConfig { + /// Largest template file the admin API accepts. A larger upload is + /// refused with `413`. pub limit: super::ByteSize, } diff --git a/crates/doppel-core/src/config/mock.rs b/crates/doppel-core/src/config/mock.rs index 4b026e6..224ee29 100644 --- a/crates/doppel-core/src/config/mock.rs +++ b/crates/doppel-core/src/config/mock.rs @@ -9,17 +9,25 @@ use super::proxy::{LatencyConfig, LossConfig}; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct MockConfig { + /// Names this mock in the `mock` log field and the hit counter. Unique + /// within the proxy. pub name: crate::config::Name, + /// What this mock matches, and what it takes out of the request. pub request: MockRequest, + /// What it answers with. pub response: MockResponse, #[serde(default, skip_serializing_if = "Option::is_none")] + /// Per-mock overrides of the proxy's `replace`, `loss` and `latency`. pub proxy: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct MockRequest { + /// The method this mock answers, matched exactly and upper case. pub method: crate::config::HttpMethod, + /// A regex matched against the request path, unanchored. Named capture + /// groups become template variables. pub url: crate::config::Pattern, /// Variable name -> request header name. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] @@ -35,11 +43,19 @@ pub struct MockRequest { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct MockResponse { + /// The status to answer with. pub status: crate::config::HttpStatus, + /// A template rendered and sent as `text/plain`. Exclusive with `json` and + /// `template`. #[serde(default, skip_serializing_if = "Option::is_none")] pub body: Option, + /// A template whose rendered output must be valid JSON, sent as + /// `application/json`. Exclusive with `body` and `template`. #[serde(default, skip_serializing_if = "Option::is_none")] pub json: Option, + /// A template file under this proxy's template directory. Read per request, + /// so it may be uploaded after the configuration was loaded. Exclusive with + /// `body` and `json`. #[serde(default, skip_serializing_if = "Option::is_none")] pub template: Option, /// Header name -> template producing the value. @@ -65,10 +81,15 @@ impl MockResponse { #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct MockProxyOverride { + /// Overrides the proxy's `replace` for requests this mock matches. #[serde(default, skip_serializing_if = "Option::is_none")] pub replace: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + /// Drops a share of the requests this mock would have answered. Not + /// inherited from the proxy: a mock without it is never dropped. pub loss: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + /// Replaces the proxy's `latency` for this mock's responses, rather than + /// adding to it. pub latency: Option, } diff --git a/crates/doppel-core/src/config/mod.rs b/crates/doppel-core/src/config/mod.rs index ad81637..d37f5af 100644 --- a/crates/doppel-core/src/config/mod.rs +++ b/crates/doppel-core/src/config/mod.rs @@ -10,6 +10,7 @@ pub mod pattern; pub mod port; pub mod proxy; pub mod ratio; +pub mod schema; pub mod selector; pub mod server; pub mod size; @@ -24,13 +25,14 @@ use serde::{Deserialize, Serialize}; pub use crate::method::{HttpMethod, MethodError}; pub use admin::{ - AccessConfig, AdminConfig, AuthConfig, ProxyAccessConfig, Subjects, TokenConfig, UploadConfig, + AccessConfig, AdminConfig, AllowedGroup, AuthConfig, ProxyAccessConfig, Subjects, TokenConfig, + UploadConfig, }; pub use duration::{Seconds, SecondsError, TimeoutError, TimeoutSeconds}; pub use env::{EnvTokens, EnvTokensError}; pub use header::{HeaderName, HeaderNameError, HeaderValue, HeaderValueError}; pub use mock::{MockConfig, MockProxyOverride, MockRequest, MockResponse}; -pub use name::{Name, NameError}; +pub use name::{MAX_PROXY, Name, NameError, ProxyName}; pub use pattern::{Pattern, PatternError}; pub use port::{Port, PortError}; pub use proxy::{LatencyConfig, LossConfig, ProxyConfig, ProxyKind, ResolveConfig, ResolveKind}; @@ -45,19 +47,30 @@ pub use template::{TemplateName, TemplateNameError}; pub use token::{Token, TokenError}; pub use url::{UpstreamUrl, UrlError}; -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct Config { + /// Where the proxy listens for the traffic being forwarded or mocked. pub server: ServerConfig, + /// Log level and output format. #[serde(default)] pub logging: LoggingConfig, + /// The Unix socket `doppel config reload` talks to. #[serde(default)] pub control: ControlConfig, + /// Where mock template files are read from and uploaded to. #[serde(default)] pub templates: TemplatesConfig, + /// Optional error reporting. Absent, or an empty DSN, disables it. #[serde(default, skip_serializing_if = "Option::is_none")] pub sentry: Option, + /// The admin API's listener, its tokens, and who may do what. pub admin: AdminConfig, + /// The proxies this instance serves, in the order they are tried. + /// + /// May be empty or left out: Doppel then starts and serves the admin API, + /// and a request is answered `503 NO_PROXIES_CONFIGURED` until a proxy is + /// added by reload or over that API. #[serde(default)] pub proxies: Vec, } @@ -227,11 +240,24 @@ proxies: assert_eq!(parse_subjects("[]"), Subjects::Public); } + /// `tcp` is refused while the document is read, not by a rule afterwards. + /// The message has to say it is unimplemented rather than only that the + /// value was not accepted -- someone writing `type: tcp` has a plan, not a + /// typo, and deserves to be told the plan will not work. #[test] - fn tcp_type_deserializes_so_validation_can_reject_it_with_a_good_message() { + fn tcp_type_is_refused_while_parsing_and_says_why() { let text = MINIMAL.replace("type: http", "type: tcp"); - let config = load_from_str(&text).unwrap(); - assert_eq!(config.proxies[0].kind, ProxyKind::Tcp); + let err = load_from_str(&text).unwrap_err().to_string(); + assert!(err.contains("not implemented"), "got {err}"); + assert!(err.contains("tcp"), "got {err}"); + } + + #[test] + fn an_unknown_proxy_type_names_the_only_accepted_one() { + let text = MINIMAL.replace("type: http", "type: grpc"); + let err = load_from_str(&text).unwrap_err().to_string(); + assert!(err.contains("grpc"), "got {err}"); + assert!(err.contains("http"), "got {err}"); } #[test] diff --git a/crates/doppel-core/src/config/name.rs b/crates/doppel-core/src/config/name.rs index ee5b8c3..5271f4c 100644 --- a/crates/doppel-core/src/config/name.rs +++ b/crates/doppel-core/src/config/name.rs @@ -22,23 +22,47 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// nothing legitimate is shorter than `p1` or `ci`. Not four, which was the /// first proposal -- it refuses `ops`, and a group nobody can name for being /// three letters long is a rule getting in the way of its own purpose. -const MIN: usize = 2; +pub const MIN: usize = 2; -/// The longest name accepted. +/// The longest name accepted by default. /// -/// A name becomes a path component, and 128 leaves room under the 255-byte -/// limit both target platforms impose even once a prefix and an extension are -/// added around it. -const MAX: usize = 128; +/// A name becomes a path component, and 64 leaves generous room under the +/// 255-byte limit both target platforms impose even once a prefix and an +/// extension are added around it. It was 128, which was the platform limit +/// reasoned about rather than a length anyone would type. +pub const MAX: usize = 64; + +/// The longest proxy name accepted. +/// +/// Tighter than the rest, because a proxy name travels further than any other: +/// it is a directory under `templates.dir`, a `proxy` label on every metric, a +/// field in every log line, and the value a client writes into a resolution +/// header on every request. Each of those is somewhere a long name is paid for +/// repeatedly rather than once. +pub const MAX_PROXY: usize = 32; + +/// The character class a name is built from, as a regex fragment. +/// +/// Public so `AllowedGroup`'s schema can compose its own pattern from it rather +/// than restating the class -- two spellings of one rule drift, and the schema is +/// the copy nobody compiles. +pub const CHARACTERS: &str = "[A-Za-z0-9_-]"; /// A validated name. /// -/// Letters, digits, `.`, `-` and `_`, between 2 and 128 characters. The dot is -/// allowed because the reference configuration already documents names like -/// `Billing.API.v2`, and removing a spelling the documentation teaches is a -/// cost with no matching benefit. +/// Letters, digits, `-` and `_`, between [`MIN`] and `MAX_LEN` characters -- +/// [`MAX`] by default, [`MAX_PROXY`] for a [`ProxyName`]. +/// +/// The dot used to be allowed, for names like `Billing.API.v2`. It is not any +/// more, and dropping it removed two rules with it: a name becomes a directory +/// component, so `.hidden` and `..` each had to be refused separately. Without +/// the dot neither shape can be written, and the type is a character set and a +/// length again rather than a character set, a length and two exceptions. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct Name(String); +pub struct Name(String); + +/// A proxy name: the same rules, capped at [`MAX_PROXY`]. +pub type ProxyName = Name; /// Why a name was refused. /// @@ -49,21 +73,19 @@ pub struct Name(String); pub enum NameError { #[error("a name must be at least {MIN} characters, `{0}` is {len}", len = .0.chars().count())] TooShort(String), - #[error("a name must be at most {MAX} characters, this one is {0}")] - TooLong(usize), - #[error("a name may contain letters, digits, `.`, `-` and `_`; `{0}` contains {1:?}")] + #[error("a name must be at most {1} characters, this one is {0}")] + TooLong(usize, usize), + /// The dot gets its own message. It was accepted until 0.3.0, and the + /// reference configuration taught it, so somebody hitting this is more + /// likely to be carrying an old name forward than to have mistyped -- and + /// "contains '.'" alone reads like the character set is being restated. + #[error("a name may no longer contain `.`; `{0}` does -- use `-` or `_` instead")] + Dot(String), + #[error("a name may contain letters, digits, `-` and `_`; `{0}` contains {1:?}")] BadCharacter(String, char), - /// A name becomes a directory component, so the shapes that stop it being - /// one are refused here rather than at the moment a file is written -- - /// which is a path error reported for a configuration mistake, and much - /// later. - #[error("a name must not start with a dot: `{0}`")] - LeadingDot(String), - #[error("a name must not contain `..`: `{0}`")] - DotDot(String), } -impl Name { +impl Name { /// Check a string and keep it, or say why not. pub fn parse(value: impl Into) -> Result { let value = value.into(); @@ -78,27 +100,25 @@ impl Name { if length < MIN { return Err(NameError::TooShort(value)); } - if length > MAX { - return Err(NameError::TooLong(length)); + if length > MAX_LEN { + return Err(NameError::TooLong(length, MAX_LEN)); + } + // Checked before the general character test so the dot gets its own + // message rather than being reported as just another rejected + // character. + if value.contains('.') { + return Err(NameError::Dot(value)); } if let Some(bad) = value .chars() - .find(|c| !c.is_ascii_alphanumeric() && !matches!(c, '.' | '-' | '_')) + .find(|c| !c.is_ascii_alphanumeric() && !matches!(c, '-' | '_')) { return Err(NameError::BadCharacter(value, bad)); } - // The character set alone would admit `..` and `.hidden`, and a name - // is a directory component. Catching them here is what lets the - // separate validation rule that used to do it go away: one check, at - // the moment the name comes into existence. - if value.starts_with('.') { - return Err(NameError::LeadingDot(value)); - } - if value.contains("..") { - return Err(NameError::DotDot(value)); - } - + // No `.hidden` or `..` check is needed: neither can be written without a + // dot, so a name is a safe directory component by construction rather + // than by a further rule. Ok(Self(value)) } @@ -113,13 +133,13 @@ impl Name { } } -impl fmt::Display for Name { +impl fmt::Display for Name { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str(&self.0) } } -impl FromStr for Name { +impl FromStr for Name { type Err = NameError; fn from_str(value: &str) -> Result { @@ -127,7 +147,7 @@ impl FromStr for Name { } } -impl AsRef for Name { +impl AsRef for Name { fn as_ref(&self) -> &str { &self.0 } @@ -135,63 +155,91 @@ impl AsRef for Name { /// So a `BTreeMap` or a `Vec` can be looked up by `&str` /// without building a `Name` to throw away. -impl Borrow for Name { +impl Borrow for Name { fn borrow(&self) -> &str { &self.0 } } -impl PartialEq for Name { +impl PartialEq for Name { fn eq(&self, other: &str) -> bool { self.0 == other } } -impl PartialEq<&str> for Name { +impl PartialEq<&str> for Name { fn eq(&self, other: &&str) -> bool { self.0 == *other } } -impl PartialEq for str { - fn eq(&self, other: &Name) -> bool { +impl PartialEq> for str { + fn eq(&self, other: &Name) -> bool { self == other.0 } } -impl Serialize for Name { +impl Serialize for Name { fn serialize(&self, s: S) -> Result { s.serialize_str(&self.0) } } -impl<'de> Deserialize<'de> for Name { +impl<'de, const MAX_LEN: usize> Deserialize<'de> for Name { fn deserialize>(d: D) -> Result { let value = String::deserialize(d)?; Self::parse(value).map_err(serde::de::Error::custom) } } -impl utoipa::PartialSchema for Name { +impl utoipa::PartialSchema for Name { fn schema() -> utoipa::openapi::RefOr { utoipa::openapi::schema::ObjectBuilder::new() .schema_type(utoipa::openapi::schema::Type::String) - .pattern(Some(r"^[A-Za-z0-9._-]+$")) + .pattern(Some(format!("^{CHARACTERS}{{{MIN},{MAX_LEN}}}$"))) .min_length(Some(MIN)) - .max_length(Some(MAX)) - .description(Some( - "Letters, digits, `.`, `-` and `_`, between 2 and 128 characters.", - )) + // The cap that actually applies, so a proxy name's schema says 32 + // and every other name's says 64 rather than both restating one + // number that is right for neither. + .max_length(Some(MAX_LEN)) + .description(Some(format!( + "Letters, digits, `-` and `_`, between {MIN} and {MAX_LEN} characters." + ))) .into() } } -impl utoipa::ToSchema for Name {} +impl utoipa::ToSchema for Name { + /// One schema name per cap. + /// + /// `utoipa` derives a component name from the type name alone, so + /// `Name<64>` and `Name<32>` would both be `Name` and the second would + /// silently overwrite the first -- leaving the generated schema claiming a + /// proxy name may be 64 characters when the type refuses 33. `utoipa`'s own + /// documentation warns about exactly this for generic types. + fn name() -> std::borrow::Cow<'static, str> { + match MAX_LEN { + MAX_PROXY => std::borrow::Cow::Borrowed("ProxyName"), + MAX => std::borrow::Cow::Borrowed("Name"), + // No other cap exists today; if one is added it gets a name of its + // own rather than colliding with these two. + _ => std::borrow::Cow::Owned(format!("Name{MAX_LEN}")), + } + } +} #[cfg(test)] mod tests { use super::*; + /// The default cap, spelled out: `Name` alone leaves the const parameter to + /// be inferred, and in a test there is nothing to infer it from. + /// + /// Not called `Default`, which it was: that shadows the trait of the same + /// name inside this module, and a reader meeting `AnyName::parse` has to + /// work out which of the two they are looking at. + type AnyName = Name; + #[test] fn the_names_people_actually_write_are_accepted() { for name in [ @@ -200,10 +248,13 @@ mod tests { "alpha", "billing-api", "billing_api", - "Billing.API.v2", + "BillingAPIv2", "a".repeat(MAX).as_str(), ] { - assert!(Name::parse(name).is_ok(), "`{name}` should be a legal name"); + assert!( + AnyName::parse(name).is_ok(), + "`{name}` should be a legal name" + ); } } @@ -211,28 +262,74 @@ mod tests { fn a_single_character_is_refused_but_two_are_not() { // Two, not four. Four refuses `ops`, and a group nobody can name for // being three letters long is a rule obstructing its own purpose. - assert!(matches!(Name::parse("a"), Err(NameError::TooShort(_)))); - assert!(Name::parse("ci").is_ok()); - assert!(Name::parse("ops").is_ok()); + assert!(matches!(AnyName::parse("a"), Err(NameError::TooShort(_)))); + assert!(AnyName::parse("ci").is_ok()); + assert!(AnyName::parse("ops").is_ok()); + } + + /// The dot was legal until 0.3.0 and the reference configuration taught + /// `Billing.API.v2`, so it gets a message that says it was removed and what + /// to write instead -- not one that reads like the character set being + /// restated. + #[test] + fn a_dot_is_refused_and_the_message_offers_a_replacement() { + let err = AnyName::parse("Billing.API.v2").unwrap_err(); + assert!(matches!(err, NameError::Dot(_)), "{err:?}"); + let text = err.to_string(); + assert!(text.contains("no longer"), "{text}"); + assert!(text.contains('-') && text.contains('_'), "{text}"); + } + + /// Without the dot there is no `..` and no `.hidden` to check for + /// separately: a name is a usable directory component by construction. + #[test] + fn the_shapes_that_needed_their_own_rules_are_unwritable_now() { + for name in ["..", ".hidden", "a..b", "..a"] { + let err = AnyName::parse(name).unwrap_err(); + assert!( + matches!(err, NameError::Dot(_) | NameError::TooShort(_)), + "`{name}` -> {err:?}" + ); + } } #[test] fn the_message_says_what_is_wrong_not_just_that_something_is() { // The whole point of one variant per rule. A reader who sees the rule // restated still has to work out which part they broke. - let short = Name::parse("a").unwrap_err().to_string(); + let short = AnyName::parse("a").unwrap_err().to_string(); assert!(short.contains("at least 2"), "{short}"); assert!(short.contains("is 1"), "{short}"); - let bad = Name::parse("a/b").unwrap_err().to_string(); + let bad = AnyName::parse("a/b").unwrap_err().to_string(); assert!( bad.contains('/'), "the offending character must be named: {bad}" ); - let long = Name::parse("a".repeat(MAX + 1)).unwrap_err().to_string(); - assert!(long.contains("at most 128"), "{long}"); - assert!(long.contains("129"), "{long}"); + let long = AnyName::parse("a".repeat(MAX + 1)).unwrap_err().to_string(); + assert!(long.contains("at most 64"), "{long}"); + assert!(long.contains("65"), "{long}"); + } + + /// A proxy name travels further than any other -- a directory, a metric + /// label, a log field, and a header value on every request -- so it is + /// capped tighter. The message has to quote the cap that was applied, not + /// the default one. + #[test] + fn a_proxy_name_is_capped_shorter_than_other_names() { + let thirty_three = "a".repeat(MAX_PROXY + 1); + assert!( + AnyName::parse(&thirty_three).is_ok(), + "still fine for a token or a group" + ); + + let err = ProxyName::parse(&thirty_three).unwrap_err(); + assert!(matches!(err, NameError::TooLong(33, 32)), "{err:?}"); + let text = err.to_string(); + assert!(text.contains("at most 32"), "{text}"); + + assert!(ProxyName::parse("a".repeat(MAX_PROXY)).is_ok()); } #[test] @@ -240,8 +337,8 @@ mod tests { // A name becomes a path component. These are the shapes that stop it // being one, and the reason the store's `sanitize` and this type agree // about what a name is. - for name in ["..", "a/b", "a\\b", "a b", "a\u{0}b", ".."] { - assert!(Name::parse(name).is_err(), "`{name}` must be refused"); + for name in ["..", "a/b", "a\\b", "a b", "a\u{0}b"] { + assert!(AnyName::parse(name).is_err(), "`{name}` must be refused"); } } @@ -250,12 +347,12 @@ mod tests { // Two accented letters are two characters and four bytes. The length // check runs before the character set check, so its message has to be // right even for input the next check will reject. - let err = Name::parse("é").unwrap_err(); + let err = AnyName::parse("é").unwrap_err(); assert!( matches!(err, NameError::TooShort(_)), "one character is short regardless of its byte length: {err:?}" ); - let two = Name::parse("éé").unwrap_err(); + let two = AnyName::parse("éé").unwrap_err(); assert!( matches!(two, NameError::BadCharacter(..)), "two characters are long enough, and then refused for the character: {two:?}" @@ -264,14 +361,24 @@ mod tests { #[test] fn a_name_round_trips_through_yaml() { - let name = Name::parse("billing-api").unwrap(); + let name = AnyName::parse("billing-api").unwrap(); let yaml = serde_norway::to_string(&name).unwrap(); - assert_eq!(serde_norway::from_str::(&yaml).unwrap(), name); + assert_eq!(serde_norway::from_str::(&yaml).unwrap(), name); } #[test] fn deserializing_a_bad_name_fails_with_the_reason() { - let err = serde_norway::from_str::("\"a/b\"").unwrap_err(); + let err = serde_norway::from_str::("\"a/b\"").unwrap_err(); assert!(err.to_string().contains('/'), "{err}"); } + + /// The cap belongs to the type, so it has to survive deserialization rather + /// than only being applied by a direct `parse`. + #[test] + fn the_proxy_cap_applies_when_deserializing_too() { + let long = format!("\"{}\"", "a".repeat(MAX_PROXY + 1)); + assert!(serde_norway::from_str::(&long).is_ok()); + let err = serde_norway::from_str::(&long).unwrap_err(); + assert!(err.to_string().contains("at most 32"), "{err}"); + } } diff --git a/crates/doppel-core/src/config/proxy.rs b/crates/doppel-core/src/config/proxy.rs index 1b13b4d..5f1774e 100644 --- a/crates/doppel-core/src/config/proxy.rs +++ b/crates/doppel-core/src/config/proxy.rs @@ -8,11 +8,34 @@ use super::admin::ProxyAccessConfig; use super::mock::MockConfig; use super::size::ByteSize; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] +/// What a proxy forwards. Only `http` exists. +/// +/// There was a `Tcp` variant, admitted by the parser so that rule V7 could +/// reject it afterwards with a message better than serde's. Both are gone: the +/// variant meant every layer downstream -- the runtime, the store, this schema -- +/// had to carry a case that could never be reached, and the good message is +/// available without it. `Deserialize` below is written by hand for exactly that +/// reason, so `type: tcp` still says *why* rather than only that the value is +/// not one of the accepted ones. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, utoipa::ToSchema)] #[serde(rename_all = "lowercase")] pub enum ProxyKind { + /// Forward HTTP, the only kind Doppel implements. Http, - Tcp, +} + +impl<'de> Deserialize<'de> for ProxyKind { + fn deserialize>(deserializer: D) -> Result { + match String::deserialize(deserializer)?.as_str() { + "http" => Ok(Self::Http), + "tcp" => Err(serde::de::Error::custom( + "`tcp` proxying is not implemented; `http` is the only proxy type", + )), + other => Err(serde::de::Error::custom(format!( + "`{other}` is not a proxy type; `http` is the only one" + ))), + } + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] @@ -25,8 +48,12 @@ pub enum ResolveKind { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct ResolveConfig { + /// `default` takes anything unclaimed; `header` takes requests naming + /// this proxy in `header`. #[serde(rename = "type", default = "default_resolve_kind")] pub kind: ResolveKind, + /// The header carrying the proxy name. Required when `type: header`, and + /// meaningless otherwise. #[serde(default, skip_serializing_if = "Option::is_none")] pub header: Option, } @@ -47,44 +74,83 @@ impl Default for ResolveConfig { #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct LossConfig { + /// The share of requests to drop, as a fraction. `0.1` is one in ten. pub percentage: super::Ratio, + /// The status a dropped request is answered with, rather than being left + /// to hang. pub status: super::HttpStatus, } #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct LatencyConfig { + /// The share of requests to delay, as a fraction. pub percentage: super::Ratio, + /// Lower bound of the delay, in seconds. The delay is a target for the + /// whole response: time the upstream already spent is subtracted. pub min: super::Seconds, + /// Upper bound of the delay, in seconds. Must be at least `min`. pub max: super::Seconds, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct ProxyConfig { - pub name: crate::config::Name, + /// Names this proxy in `X-Proxy-Name`, in metrics labels, in log lines + /// and as its template subdirectory. Unique within the document. + pub name: crate::config::ProxyName, + /// What it forwards. `http` is the only value. #[serde(rename = "type")] pub kind: ProxyKind, + /// The upstream base. A request path is grafted underneath it, and the + /// result can never escape it -- so a base with a path confines the proxy + /// to that subtree. pub url: super::UpstreamUrl, + /// Bounds the whole upstream exchange, in seconds. Exceeding it is + /// `504 UPSTREAM_TIMEOUT`. Defaults to 30. #[serde(default, skip_serializing_if = "Option::is_none")] pub timeout: Option, + /// How a request is matched to this proxy: by header, or as the default. #[serde(default)] pub resolve: ResolveConfig, + /// Overrides the admin `access` rules for this proxy alone. #[serde(default, skip_serializing_if = "Option::is_none")] pub access: Option, + /// Headers injected into every outbound request, overriding whatever the + /// client sent by the same name. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub headers: BTreeMap, + /// Drops a share of requests rather than forwarding them. Not applied to + /// a request a mock answered. #[serde(default, skip_serializing_if = "Option::is_none")] pub loss: Option, + /// Makes a share of requests take a chosen time. Applies to mocked + /// responses too; a mock may override the figure. #[serde(default, skip_serializing_if = "Option::is_none")] pub latency: Option, + /// What share of requests a matching mock actually answers; the rest go + /// upstream. Defaults to 1.0, so a matching mock answers. #[serde(default, skip_serializing_if = "Option::is_none")] pub replace: Option, + /// Whether a redirect whose `Location` points back into the space this + /// proxy forwards is rewritten to point at Doppel instead. Absent means + /// enabled. + /// + /// On by default because the alternative is a silent failure: `Host` is + /// replaced with the upstream's authority, so the upstream's `Location` + /// names the upstream, and a client following it leaves Doppel -- along with + /// every fault and every mock -- with nothing reported. Turn it off to have + /// the response relayed byte for byte, which is what a client being tested + /// *against redirect handling itself* needs. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub rewrite_redirects: Option, /// Bounds the request body a matched mock is allowed to buffer in order /// to extract from it; phase 1 streams bodies deliberately, and reading /// `.content.items` needs the whole thing in hand. See rule V33. #[serde(default = "default_body_limit")] pub body_limit: ByteSize, + /// Mocks in the order they are tried. First match wins, and patterns are + /// unanchored, so a general one placed first shadows the rest. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub mocks: Vec, } diff --git a/crates/doppel-core/src/config/schema.rs b/crates/doppel-core/src/config/schema.rs new file mode 100644 index 0000000..c4b277b --- /dev/null +++ b/crates/doppel-core/src/config/schema.rs @@ -0,0 +1,283 @@ +//! The configuration document described as a JSON Schema, for editors and for +//! anything that validates a file before Doppel sees it. +//! +//! Derived from the same `utoipa::ToSchema` implementations the admin API's +//! OpenAPI document uses, rather than from a second set of derives or a +//! hand-written file. Two descriptions of one type drift, and the one nobody +//! runs drifts first; this way a field added to `Config` appears here or the +//! drift test in `tests` below fails. +//! +//! `utoipa` 5 emits OpenAPI 3.1 schema objects, and 3.1 aligned its schema +//! dialect with JSON Schema 2020-12 -- so the objects need no translation, only +//! rehousing: OpenAPI keeps its definitions under `#/components/schemas/` and +//! JSON Schema under `#/$defs/`, so every `$ref` is rewritten. + +use serde_json::{Value, json}; + +/// Where the checked-in copy of this schema is fetched from. +/// +/// The raw file on `main`, not a release asset: this URL goes into a `$schema` +/// comment that a reader copies once and keeps, and pinning it to whichever +/// version happened to be current would leave them validating next year's file +/// against an old schema. Every release also attaches the file, for anyone who +/// wants the pin. +pub const URL: &str = + "https://raw.githubusercontent.com/lorem-dev/doppel/main/doppel-config.schema.json"; + +/// The whole configuration document as a JSON Schema 2020-12 object. +#[must_use] +pub fn json_schema() -> Value { + let mut defs = Vec::new(); + ::schemas(&mut defs); + + let mut root = serde_json::to_value(::schema()) + .expect("a utoipa schema serializes"); + rehouse_refs(&mut root); + hoist_descriptions(&mut root); + + let mut definitions = serde_json::Map::new(); + for (name, schema) in defs { + let mut value = serde_json::to_value(schema).expect("a utoipa schema serializes"); + rehouse_refs(&mut value); + hoist_descriptions(&mut value); + definitions.insert(name, value); + } + + let mut out = json!({ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": URL, + "title": "Doppel configuration", + "description": concat!( + "The `main.yaml` Doppel reads. Generated from the Rust types by ", + "`doppel config schema`; do not edit by hand." + ), + }); + let object = out.as_object_mut().expect("built from a json! object"); + // Insertion order does not survive: `serde_json`'s map is a `BTreeMap` + // unless `preserve_order` is on, so the emitted file is sorted by key + // whatever order things are added in. That is worth having for a generated + // file -- the diff between two versions is the change and nothing else -- + // and it is why the metadata keys appear among the schema's own rather than + // above them. + if let Value::Object(root) = root { + object.extend(root); + } + object.insert("$defs".to_owned(), Value::Object(definitions)); + out +} + +/// The schema as the file on disk holds it: pretty-printed, one trailing +/// newline. Shared by the CLI and the drift test so neither can disagree about +/// formatting. +#[must_use] +pub fn json_schema_document() -> String { + let mut text = serde_json::to_string_pretty(&json_schema()).expect("a json value serializes"); + text.push('\n'); + text +} + +/// Lifts a field's description out of the `oneOf` branch `utoipa` buries it in, +/// up to the field itself. +/// +/// An `Option` where `T` is its own schema is emitted as +/// `{"oneOf": [{"type": "null"}, {"$ref": "...", "description": "..."}]}` -- the +/// doc comment is there, one level below where anything looks for it. An editor +/// showing a tooltip for `timeout:` reads the property, finds no description and +/// shows nothing, which is the whole reason those doc comments were written. +/// +/// Only the description moves; the `oneOf` stays, so the field is still +/// nullable. JSON Schema 2020-12 allows keywords beside a `$ref`, which +/// draft-07 did not -- worth knowing, because it is why this can be a hoist +/// rather than a restructuring. +fn hoist_descriptions(value: &mut Value) { + match value { + Value::Object(map) => { + if !map.contains_key("description") + && let Some(Value::Array(branches)) = map.get_mut("oneOf") + { + let lifted = branches.iter_mut().find_map(|branch| { + branch + .as_object_mut() + .filter(|b| b.contains_key("$ref")) + .and_then(|b| b.remove("description")) + }); + if let Some(description) = lifted { + map.insert("description".to_owned(), description); + } + } + for nested in map.values_mut() { + hoist_descriptions(nested); + } + } + Value::Array(items) => { + for item in items { + hoist_descriptions(item); + } + } + _ => {} + } +} + +/// Rewrites every `$ref` from OpenAPI's location to JSON Schema's, in place and +/// at any depth. +fn rehouse_refs(value: &mut Value) { + const OPENAPI: &str = "#/components/schemas/"; + match value { + Value::Object(map) => { + if let Some(Value::String(reference)) = map.get_mut("$ref") + && let Some(name) = reference.strip_prefix(OPENAPI) + { + *reference = format!("#/$defs/{name}"); + } + for nested in map.values_mut() { + rehouse_refs(nested); + } + } + Value::Array(items) => { + for item in items { + rehouse_refs(item); + } + } + _ => {} + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The checked-in file is what editors and the release fetch, so it has to + /// be what the code produces. Regenerate with: + /// + /// ```text + /// uv run scripts/config_schema.py + /// ``` + #[test] + fn the_checked_in_schema_is_what_the_code_generates() { + let on_disk = include_str!("../../../../doppel-config.schema.json"); + assert_eq!( + on_disk, + json_schema_document(), + "doppel-config.schema.json is stale; regenerate it with \ + `uv run scripts/config_schema.py`" + ); + } + + #[test] + fn no_ref_still_points_at_the_openapi_component_path() { + let text = json_schema_document(); + assert!( + !text.contains("#/components/schemas/"), + "a $ref was left pointing into an OpenAPI document" + ); + assert!(text.contains("#/$defs/"), "no $ref was rehoused at all"); + } + + /// Every `$ref` has to resolve, or an editor reports the file as broken + /// rather than reporting the mistake in the configuration. + #[test] + fn every_ref_resolves_to_a_definition() { + let schema = json_schema(); + let defs = schema["$defs"].as_object().expect("$defs is an object"); + + let mut refs = Vec::new(); + collect_refs(&schema, &mut refs); + assert!(!refs.is_empty(), "the schema has no $refs to check"); + + let dangling: Vec<_> = refs + .iter() + .filter_map(|r| r.strip_prefix("#/$defs/")) + .filter(|name| !defs.contains_key(*name)) + .collect(); + assert!(dangling.is_empty(), "unresolved $refs: {dangling:?}"); + } + + /// The sections a reader edits first. Named individually rather than + /// counted, so adding a section cannot quietly satisfy the assertion. + #[test] + fn the_top_level_sections_are_all_described() { + let schema = json_schema(); + let properties = schema["properties"] + .as_object() + .expect("the root describes properties"); + for section in [ + "server", + "logging", + "control", + "templates", + "sentry", + "admin", + "proxies", + ] { + assert!( + properties.contains_key(section), + "`{section}` is missing from the schema root: {:?}", + properties.keys().collect::>() + ); + } + } + + /// The schema exists to be read in an editor, and a field with no + /// description is a tooltip that says nothing. Enumerated rather than + /// spot-checked: a field added without a doc comment fails here, which is + /// the only moment anyone would notice. + #[test] + fn every_field_carries_a_description() { + let schema = json_schema(); + let mut bare = Vec::new(); + + let mut check = |owner: &str, node: &Value| { + if let Some(properties) = node.get("properties").and_then(Value::as_object) { + for (field, spec) in properties { + if spec.get("description").is_none() { + bare.push(format!("{owner}.{field}")); + } + } + } + }; + check("Config", &schema); + for (name, node) in schema["$defs"].as_object().expect("$defs is an object") { + check(name, node); + } + + assert!( + bare.is_empty(), + "these fields would show an empty tooltip; give them a doc comment: {bare:?}" + ); + } + + /// `main.example.yaml` carries the URL in a `yaml-language-server` modeline, + /// and a reader copies that line into their own file. If the two disagree, + /// every copy points somewhere wrong. + #[test] + fn the_example_configs_modeline_names_this_url() { + let example = include_str!("../../../../main.example.yaml"); + let modeline = example + .lines() + .find(|line| line.contains("yaml-language-server")) + .expect("main.example.yaml must carry a $schema modeline"); + assert!( + modeline.contains(URL), + "the modeline names a different URL than `schema::URL`:\n {modeline}" + ); + } + + fn collect_refs(value: &Value, out: &mut Vec) { + match value { + Value::Object(map) => { + if let Some(Value::String(reference)) = map.get("$ref") { + out.push(reference.clone()); + } + for nested in map.values() { + collect_refs(nested, out); + } + } + Value::Array(items) => { + for item in items { + collect_refs(item, out); + } + } + _ => {} + } + } +} diff --git a/crates/doppel-core/src/config/server.rs b/crates/doppel-core/src/config/server.rs index a7d3bf7..a670a33 100644 --- a/crates/doppel-core/src/config/server.rs +++ b/crates/doppel-core/src/config/server.rs @@ -5,7 +5,7 @@ use std::path::PathBuf; use serde::{Deserialize, Serialize}; -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] /// No `workers` here. It sizes the tokio runtime, and a database-backed /// store cannot be opened before that runtime exists -- so the value has to @@ -13,11 +13,18 @@ use serde::{Deserialize, Serialize}; /// of the boundary as the connection settings. It is `--workers` / /// `DOPPEL_WORKERS`. pub struct ServerConfig { + /// An IP address, not a hostname: a name would have to be resolved, + /// and which address it resolves to is not the configuration's to + /// decide. `utoipa` has no schema for `IpAddr`, so it is described + /// here as the string it is written as. + #[schema(value_type = String, examples("127.0.0.1"))] pub host: IpAddr, + /// The TCP port proxied traffic arrives on. Must differ from + /// `admin.port`. pub port: super::Port, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(rename_all = "lowercase")] pub enum LogLevel { Trace, @@ -40,18 +47,21 @@ impl LogLevel { } } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(rename_all = "lowercase")] pub enum LogFormat { Json, Text, } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct LoggingConfig { + /// The lowest level that is logged. `RUST_LOG` overrides it when set and + /// non-empty. #[serde(default = "default_level")] pub level: LogLevel, + /// `json` for machines, `text` for a terminal. #[serde(default = "default_format")] pub format: LogFormat, } @@ -73,10 +83,15 @@ impl Default for LoggingConfig { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct ControlConfig { + /// Path to the control socket, created with mode 0600 and removed on + /// shutdown. Its parent directory must already exist. #[serde(default = "default_socket")] + /// A filesystem path. `utoipa` has no schema for `PathBuf`, so it is + /// described as the string it is written as. + #[schema(value_type = String)] pub socket: PathBuf, } @@ -92,10 +107,15 @@ impl Default for ControlConfig { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct TemplatesConfig { + /// Directory holding mock templates, one subdirectory per proxy. Created + /// at startup if absent. #[serde(default = "default_templates_dir")] + /// A filesystem path. `utoipa` has no schema for `PathBuf`, so it is + /// described as the string it is written as. + #[schema(value_type = String)] pub dir: PathBuf, } @@ -111,8 +131,10 @@ impl Default for TemplatesConfig { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, utoipa::ToSchema)] #[serde(deny_unknown_fields)] pub struct SentryConfig { + /// The Sentry DSN to report to. Empty disables reporting, so a deployment + /// can blank it without removing the section. pub dsn: String, } diff --git a/crates/doppel-core/src/conformance.rs b/crates/doppel-core/src/conformance.rs index 4443f1d..03b03c8 100644 --- a/crates/doppel-core/src/conformance.rs +++ b/crates/doppel-core/src/conformance.rs @@ -35,6 +35,11 @@ proxies: - name: alpha type: http url: "https://alpha.example.com/api/" + # Present, and deliberately `false` rather than `true`: a store that + # dropped the column, or materialised the default instead of the absence, + # would round-trip a different document and `load_returns_what_save_wrote` + # would catch it. `true` would have been indistinguishable from the default. + rewrite_redirects: false "#; fn base() -> Config { diff --git a/crates/doppel-core/src/error.rs b/crates/doppel-core/src/error.rs index f5caeb2..199f5b1 100644 --- a/crates/doppel-core/src/error.rs +++ b/crates/doppel-core/src/error.rs @@ -7,6 +7,9 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ErrorCode { ProxyNotResolved, + /// The configuration names no proxies at all, so there was never + /// anything for the request to resolve to. + NoProxiesConfigured, TemplateRenderError, TemplateNotFound, BodyExtractionError, @@ -32,6 +35,7 @@ impl ErrorCode { pub fn status(self) -> u16 { match self { Self::ProxyNotResolved | Self::NotFound => 404, + Self::NoProxiesConfigured => 503, Self::TemplateRenderError | Self::TemplateNotFound | Self::BodyExtractionError @@ -77,6 +81,7 @@ impl ErrorCode { pub fn as_str(self) -> &'static str { match self { Self::ProxyNotResolved => "PROXY_NOT_RESOLVED", + Self::NoProxiesConfigured => "NO_PROXIES_CONFIGURED", Self::TemplateRenderError => "TEMPLATE_RENDER_ERROR", Self::TemplateNotFound => "TEMPLATE_NOT_FOUND", Self::BodyExtractionError => "BODY_EXTRACTION_ERROR", @@ -113,6 +118,7 @@ impl<'de> Deserialize<'de> for ErrorCode { let value = String::deserialize(d)?; match value.as_str() { "PROXY_NOT_RESOLVED" => Ok(Self::ProxyNotResolved), + "NO_PROXIES_CONFIGURED" => Ok(Self::NoProxiesConfigured), "TEMPLATE_RENDER_ERROR" => Ok(Self::TemplateRenderError), "TEMPLATE_NOT_FOUND" => Ok(Self::TemplateNotFound), "BODY_EXTRACTION_ERROR" => Ok(Self::BodyExtractionError), @@ -200,6 +206,7 @@ mod tests { /// (rather than never added) still fails, at test time. const ALL_CODES: &[(ErrorCode, &str, u16)] = &[ (ErrorCode::ProxyNotResolved, "PROXY_NOT_RESOLVED", 404), + (ErrorCode::NoProxiesConfigured, "NO_PROXIES_CONFIGURED", 503), (ErrorCode::TemplateRenderError, "TEMPLATE_RENDER_ERROR", 500), (ErrorCode::TemplateNotFound, "TEMPLATE_NOT_FOUND", 500), (ErrorCode::BodyExtractionError, "BODY_EXTRACTION_ERROR", 500), @@ -225,6 +232,7 @@ mod tests { fn assert_listed_exactly_once(code: ErrorCode) { match code { ErrorCode::ProxyNotResolved + | ErrorCode::NoProxiesConfigured | ErrorCode::TemplateRenderError | ErrorCode::TemplateNotFound | ErrorCode::BodyExtractionError diff --git a/crates/doppel-core/src/runtime.rs b/crates/doppel-core/src/runtime.rs index fc95193..8d26dd0 100644 --- a/crates/doppel-core/src/runtime.rs +++ b/crates/doppel-core/src/runtime.rs @@ -24,6 +24,10 @@ pub struct CompiledProxy { pub loss: Option, pub latency: Option, pub replace: f64, + /// Whether a redirect pointing back into this proxy's space is rewritten to + /// point at Doppel. The `Option` in the configuration resolves to its + /// default here, so the hot path never has to know what that default was. + pub rewrite_redirects: bool, pub resolve_header: Option, /// Mocks in configuration order. Matching is first-wins, so this order /// is load-bearing, not incidental. @@ -185,6 +189,7 @@ fn compile_proxy(proxy: &ProxyConfig) -> Result { loss: proxy.loss, latency: proxy.latency, replace: proxy.replace.map_or(1.0, crate::config::Ratio::get), + rewrite_redirects: proxy.rewrite_redirects.unwrap_or(true), resolve_header: match proxy.resolve.kind { ResolveKind::Header => proxy .resolve @@ -334,6 +339,39 @@ proxies: assert_eq!(rt.proxies[1].name, "p2"); } + /// Neither proxy in the fixture mentions `rewrite_redirects`, so this is the + /// default being resolved, not a value being carried through. + /// + /// It exists because the default was once quietly flipped to `false` and the + /// whole suite stayed green: every other test builds a `CompiledProxy` + /// literal with the field set explicitly, so nothing went through + /// `compile_proxy` to notice. A default nothing asserts is a comment. + #[test] + fn a_proxy_that_says_nothing_rewrites_redirects() { + let rt = compile(TWO_PROXIES); + assert!( + rt.proxies.iter().all(|p| p.rewrite_redirects), + "rewrite_redirects defaults to true; got {:?}", + rt.proxies + .iter() + .map(|p| (p.name.as_str(), p.rewrite_redirects)) + .collect::>() + ); + } + + /// And the setting is carried through when it *is* written down, so the test + /// above cannot pass by the field being hardcoded. + #[test] + fn a_proxy_can_turn_redirect_rewriting_off() { + let text = TWO_PROXIES.replace( + " url: \"https://two.example.com/\"", + " url: \"https://two.example.com/\"\n rewrite_redirects: false", + ); + let rt = compile(&text); + assert!(rt.proxy_by_name("p1").unwrap().rewrite_redirects); + assert!(!rt.proxy_by_name("p2").unwrap().rewrite_redirects); + } + #[test] fn records_the_default_proxy() { let rt = compile(TWO_PROXIES); diff --git a/crates/doppel-core/src/store/mod.rs b/crates/doppel-core/src/store/mod.rs index c756d76..af6d14c 100644 --- a/crates/doppel-core/src/store/mod.rs +++ b/crates/doppel-core/src/store/mod.rs @@ -279,6 +279,7 @@ proxies: loss: None, latency: None, replace: None, + rewrite_redirects: None, body_limit: ByteSize::parse(1024 * 1024).unwrap(), mocks: Vec::new(), } diff --git a/crates/doppel-core/src/validate/access.rs b/crates/doppel-core/src/validate/access.rs index 71792d2..27a5247 100644 --- a/crates/doppel-core/src/validate/access.rs +++ b/crates/doppel-core/src/validate/access.rs @@ -1,4 +1,4 @@ -//! Rules V26, V27 and V34. V28 is enforced by `ProxyAccessConfig`, and V29 +//! Rules V26, V27, V34 and V36. V28 is enforced by `ProxyAccessConfig`, and V29 //! by `ByteSize`, which refuses a limit of zero for every field that uses //! it rather than once per field. @@ -10,10 +10,6 @@ use crate::config::{Config, Subjects}; /// Groups that always exist, whether or not a token carries them. const PREDEFINED_GROUPS: [&str; 2] = ["admin", "user"]; -fn access_of(config: &Config) -> &crate::config::AccessConfig { - &config.admin.access -} - pub(super) fn check(config: &Config, v: &mut Violations) { // V26 let mut seen_names = BTreeSet::new(); @@ -34,11 +30,12 @@ pub(super) fn check(config: &Config, v: &mut Violations) { // rewrite the proxy set. That is far more often a mistake than an intent, // and startup is the last cheap moment to catch it. Reads stay allowed to // be public -- `/status` and a proxy listing give nothing away. + let access = &config.admin.access; for (action, subjects) in [ - ("create", &access_of(config).create), - ("update", &access_of(config).update), - ("delete", &access_of(config).delete), - ("upload", &access_of(config).upload), + ("create", &access.create), + ("update", &access.update), + ("delete", &access.delete), + ("upload", &access.upload), ] { if matches!(subjects, Subjects::Public) { v.push( @@ -51,19 +48,35 @@ pub(super) fn check(config: &Config, v: &mut Violations) { } } - // V27 + // V27 and V36 read the same places, so the walk happens once and both + // rules run per site. They used to be one loop each, which is how a rule + // added later ends up covering the admin block and quietly forgetting the + // per-proxy overrides. let known = known_subjects(config); + for (path, subjects) in access_sites(config) { + // V27 + check_subjects(subjects, &known, &path, v); + // V36 + check_allowed_groups(subjects, config.admin.allowed_groups(), &path, v); + } +} + +/// Every place a configuration names who may do something, with the path to +/// report it under: the admin block's six actions, then each proxy's four +/// overrides. +fn access_sites(config: &Config) -> Vec<(String, &Subjects)> { let access = &config.admin.access; - for (action, subjects) in [ + let mut sites: Vec<(String, &Subjects)> = [ ("list", &access.list), ("read", &access.read), ("create", &access.create), ("update", &access.update), ("delete", &access.delete), ("upload", &access.upload), - ] { - check_subjects(subjects, &known, &format!("admin.access.{action}"), v); - } + ] + .into_iter() + .map(|(action, subjects)| (format!("admin.access.{action}"), subjects)) + .collect(); for (i, proxy) in config.proxies.iter().enumerate() { let Some(overrides) = &proxy.access else { @@ -76,15 +89,52 @@ pub(super) fn check(config: &Config, v: &mut Violations) { ("upload", &overrides.upload), ] { if let Some(subjects) = subjects { - check_subjects( - subjects, - &known, - &format!("proxies[{i}].access.{action}"), - v, - ); + sites.push((format!("proxies[{i}].access.{action}"), subjects)); } } } + sites +} + +/// V36: a name `access` references has to be one `admin.groups` allows. +/// +/// `Subjects::Public` is never checked. `public` is the absence of a subject +/// rather than a name, so an allow-list has nothing to say about it -- and a +/// configuration reduced to `groups: []` still has to be able to express "anyone +/// may read this". +fn check_allowed_groups( + subjects: &Subjects, + allowed: &[crate::config::AllowedGroup], + path: &str, + v: &mut Violations, +) { + let Subjects::Names(names) = subjects else { + return; + }; + for name in names { + if allowed.iter().any(|entry| entry.permits(name.as_str())) { + continue; + } + // The message names the list rather than only the rejection: the reader + // has to decide whether to change the reference or widen the list, and + // cannot do either without seeing what is currently permitted. + let permitted = if allowed.is_empty() { + "`admin.groups` is empty, so only `public` may be used".to_owned() + } else { + format!( + "`admin.groups` allows only {}", + allowed + .iter() + .map(|entry| format!("`{entry}`")) + .collect::>() + .join(", ") + ) + }; + v.push( + path, + format!("`{name}` is not an allowed group: {permitted}"), + ); + } } fn known_subjects(config: &Config) -> BTreeSet { diff --git a/crates/doppel-core/src/validate/mod.rs b/crates/doppel-core/src/validate/mod.rs index 3b92054..91627a8 100644 --- a/crates/doppel-core/src/validate/mod.rs +++ b/crates/doppel-core/src/validate/mod.rs @@ -186,6 +186,85 @@ proxies: ); } + /// The default is `["*"]`, and an allow-list nobody asked for would only + /// surprise -- so a configuration that never mentions `groups` is unaffected. + #[test] + fn v36_an_absent_groups_list_allows_anything() { + let text = good().replace("read: public", r#"read: ["admin", "user"]"#); + assert_eq!(validate(&load_from_str(&text).unwrap()), Ok(())); + } + + #[test] + fn v36_a_concrete_list_refuses_a_name_it_does_not_carry() { + let text = good() + .replace(" access:", " groups: [\"admin\", \"user1\"]\n access:") + .replace("read: public", r#"read: ["admin", "user"]"#); + assert_violation(&text, "admin.access.read", "`user` is not an allowed group"); + } + + /// The message has to name what *is* permitted: the reader has to choose + /// between changing the reference and widening the list, and cannot do + /// either without seeing the list. + #[test] + fn v36_the_message_names_the_permitted_entries() { + let text = good() + .replace(" access:", " groups: [\"admin\", \"ci\"]\n access:") + .replace("read: public", "read: user"); + assert_violation(&text, "admin.access.read", "allows only `admin`, `ci`"); + } + + /// `[]` is the deliberate lockdown: no name may be referenced at all, and + /// the message says so rather than listing nothing. + #[test] + fn v36_an_empty_list_permits_no_name_and_says_why() { + let text = good() + .replace(" access:", " groups: []\n access:") + .replace("update: user1", "update: admin"); + assert_violation( + &text, + "admin.access.update", + "`admin.groups` is empty, so only `public` may be used", + ); + } + + /// `public` is the absence of a subject rather than a name, so an + /// allow-list has nothing to say about it -- and a configuration locked down + /// to `groups: []` still has to be able to express "anyone may read this". + #[test] + fn v36_public_is_never_governed_by_the_list() { + let text = good() + .replace(" access:", " groups: []\n access:") + .replace("update: user1", "update: public\n create: public"); + let config = load_from_str(&text).unwrap(); + // V34 refuses a public *write*, which is a different rule and still + // applies; `read: public` is what is being asserted here. + let violations = validate(&config).unwrap_err(); + assert!( + violations + .iter() + .all(|violation| violation.path != "admin.access.read"), + "`read: public` must not be a V36 violation: {violations:?}" + ); + } + + /// A proxy's overrides are checked too. V27 and V36 walk one shared list of + /// access sites precisely so a rule cannot cover the admin block and forget + /// these. + #[test] + fn v36_governs_a_proxys_overrides_as_well() { + let text = good() + .replace(" access:", " groups: [\"admin\"]\n access:") + .replace( + " url: \"https://example.com/\"", + " url: \"https://example.com/\"\n access:\n read: user", + ); + assert_violation( + &text, + "proxies[0].access.read", + "`user` is not an allowed group", + ); + } + #[test] fn v27_predefined_groups_are_always_valid() { let text = good().replace("read: public", r#"read: ["admin", "user"]"#); @@ -342,22 +421,28 @@ proxies: /// /// Written out rather than counted, because it is the thing the tests /// below compare the source and the documentation against. - const LIVE: [u8; 16] = [1, 5, 6, 7, 10, 11, 14, 16, 19, 20, 21, 25, 26, 27, 30, 34]; + const LIVE: [u8; 15] = [1, 6, 10, 11, 14, 16, 19, 20, 21, 25, 26, 27, 30, 34, 36]; /// Every rule that has been retired, and is therefore never reused. - const RETIRED: [u8; 19] = [ - 2, 3, 4, 8, 9, 12, 13, 15, 17, 18, 22, 23, 24, 28, 29, 31, 32, 33, 35, + const RETIRED: [u8; 21] = [ + 2, 3, 4, 5, 7, 8, 9, 12, 13, 15, 17, 18, 22, 23, 24, 28, 29, 31, 32, 33, 35, ]; #[test] - fn every_number_from_v1_to_v35_is_accounted_for() { + fn every_number_up_to_the_highest_is_accounted_for() { // The two lists are the map. If a rule is retired and dropped from // `LIVE` without being added to `RETIRED`, its number silently // becomes available for reuse -- and a message quoted in an old // issue would then mean two different things. + // + // The range runs to whichever number is highest rather than to a + // literal: this asserted `1..=35` and had to be edited to add V36, + // which is one more thing to remember at exactly the moment a rule + // number is being chosen. A gap still fails, which is the point. let mut all: Vec = LIVE.iter().chain(RETIRED.iter()).copied().collect(); all.sort_unstable(); - assert_eq!(all, (1..=35).collect::>()); + let highest = *all.last().expect("there is at least one rule"); + assert_eq!(all, (1..=highest).collect::>()); } #[test] diff --git a/crates/doppel-core/src/validate/proxy.rs b/crates/doppel-core/src/validate/proxy.rs index b8ad15e..2380e7b 100644 --- a/crates/doppel-core/src/validate/proxy.rs +++ b/crates/doppel-core/src/validate/proxy.rs @@ -20,16 +20,9 @@ use std::collections::BTreeSet; use super::{Violations, mock}; -use crate::config::{Config, ProxyKind, ResolveKind}; +use crate::config::{Config, ResolveKind}; pub(super) fn check(config: &Config, v: &mut Violations) { - // V5 - v.require( - !config.proxies.is_empty(), - "proxies", - "at least one proxy is required", - ); - let mut seen_names = BTreeSet::new(); let mut default_seen = false; @@ -44,14 +37,6 @@ pub(super) fn check(config: &Config, v: &mut Violations) { ); } - // V7 - if proxy.kind == ProxyKind::Tcp { - v.push( - format!("{path}.type"), - "TCP proxying is not implemented yet", - ); - } - // V10 and V11 match proxy.resolve.kind { ResolveKind::Default => { @@ -148,10 +133,21 @@ proxies: assert_eq!(validate(&load_from_str(&good()).unwrap()), Ok(())); } + /// V5 used to refuse this. It is legal now: Doppel starts, binds, serves the + /// admin API and waits for a proxy to arrive by reload or over that API. A + /// request meanwhile is answered `503 NO_PROXIES_CONFIGURED`, which is the + /// only place the state can be reported without blaming the client. #[test] - fn v5_proxies_must_not_be_empty() { + fn an_empty_proxy_list_is_accepted() { let text = good().split("proxies:").next().unwrap().to_owned() + "proxies: []\n"; - assert_violation(&text, "proxies", "at least one proxy"); + assert_eq!(validate(&load_from_str(&text).unwrap()), Ok(())); + } + + /// And an absent list, which is a different document from an empty one. + #[test] + fn an_absent_proxy_list_is_accepted() { + let text = good().split("proxies:").next().unwrap().to_owned(); + assert_eq!(validate(&load_from_str(&text).unwrap()), Ok(())); } #[test] @@ -170,10 +166,17 @@ proxies: // document, not merely fail later at a template write. for (name, expected) in [ ("a", "at least 2"), - ("..", "must not start with a dot"), + // Every dotted shape now fails on the dot itself. `..` and + // `.hidden` used to need a rule each; without the dot in the + // character set neither can be written at all. + ("..", "no longer contain `.`"), ("a/b", "contains"), - ("a..b", "must not contain `..`"), - (".hidden", "must not start with a dot"), + ("a..b", "no longer contain `.`"), + (".hidden", "no longer contain `.`"), + ("Billing.API.v2", "no longer contain `.`"), + // A proxy name is capped at 32, tighter than the 64 a token or a + // group gets. + (&"a".repeat(33), "at most 32"), ] { let text = good().replace("name: p1", &format!("name: '{name}'")); let err = load_from_str(&text) @@ -185,7 +188,7 @@ proxies: #[test] fn the_names_people_actually_use_still_load() { - for name in ["p1", "ops", "billing-api", "billing_api", "Billing.API.v2"] { + for name in ["p1", "ops", "billing-api", "billing_api", "BillingAPIv2"] { let text = good().replace("name: p1", &format!("name: '{name}'")); assert_eq!( validate(&load_from_str(&text).unwrap()), @@ -195,15 +198,6 @@ proxies: } } - #[test] - fn v7_tcp_is_rejected_with_a_specific_message() { - assert_violation( - &good().replace("type: http", "type: tcp"), - "proxies[0].type", - "TCP proxying is not implemented yet", - ); - } - #[test] fn an_upstream_url_doppel_cannot_forward_over_fails_at_load() { // V8 and V32, now `config::UpstreamUrl`. The last of them is the diff --git a/crates/doppel-proxy/src/mock.rs b/crates/doppel-proxy/src/mock.rs index 5b20d88..0561bd7 100644 --- a/crates/doppel-proxy/src/mock.rs +++ b/crates/doppel-proxy/src/mock.rs @@ -182,6 +182,7 @@ mod tests { loss: None, latency: None, replace: 1.0, + rewrite_redirects: true, resolve_header: None, mocks, body_limit: 1024 * 1024, diff --git a/crates/doppel-proxy/src/resolve.rs b/crates/doppel-proxy/src/resolve.rs index 120ad09..d2f8c89 100644 --- a/crates/doppel-proxy/src/resolve.rs +++ b/crates/doppel-proxy/src/resolve.rs @@ -22,12 +22,35 @@ pub fn resolve<'a>(runtime: &'a Runtime, headers: &HeaderMap) -> Result<&'a Comp } } - runtime.default().ok_or_else(|| { - Error::new( - ErrorCode::ProxyNotResolved, - "no proxy matched the request and no default proxy is configured", - ) - }) + if let Some(proxy) = runtime.default() { + return Ok(proxy); + } + + // Two different failures, told apart because they are somebody else's + // problem in each case. + // + // An empty proxy list is a configuration that has not been finished. It is + // legal -- Doppel starts, binds, serves the admin API and waits for a proxy + // to be added over it or by reload -- so this is the only place it can be + // reported, and the client did nothing wrong. `503` says the service is not + // in a position to answer and invites a retry, which is exactly right: the + // operator adds a proxy, reloads, and the next attempt works, with no + // restart. A `404` here would tell the caller their path was wrong and send + // whoever is debugging the client into the client. + if runtime.proxies.is_empty() { + return Err(Error::new( + ErrorCode::NoProxiesConfigured, + "no proxies are configured; add one and reload", + )); + } + + // Proxies exist and none of them wanted this request: either its resolution + // header named nothing, or it carried none and there is no default. That is + // about this request, so it stays a `404`. + Err(Error::new( + ErrorCode::ProxyNotResolved, + "no proxy matched the request and no default proxy is configured", + )) } #[cfg(test)] @@ -163,4 +186,40 @@ proxies: ); assert_eq!(resolve(&rt, &map).unwrap().name, "fallback"); } + + /// The configuration is unfinished rather than the request being wrong, so + /// `503` and its own code. A `404` here would send whoever is debugging the + /// client into the client. + #[test] + fn no_proxies_at_all_is_a_503_of_its_own() { + let text = CONFIG.split("proxies:").next().unwrap().to_owned() + "proxies: []\n"; + let err = resolve(&runtime(&text), &HeaderMap::new()).unwrap_err(); + assert_eq!(err.code, ErrorCode::NoProxiesConfigured); + assert_eq!(err.status(), 503); + } + + /// And an absent list, which is a different document from an empty one and + /// reaches the same place. + #[test] + fn an_absent_proxy_list_resolves_the_same_way_as_an_empty_one() { + let text = CONFIG.split("proxies:").next().unwrap().to_owned(); + let err = resolve(&runtime(&text), &HeaderMap::new()).unwrap_err(); + assert_eq!(err.code, ErrorCode::NoProxiesConfigured); + } + + /// The distinction the two codes exist for: proxies are configured, and none + /// of them wanted this request. That is about the request, so it stays a + /// `404`. + #[test] + fn proxies_that_none_match_stays_proxy_not_resolved() { + // Every proxy in this fixture resolves by header, so a request carrying + // none has nothing to fall back to. + let text = CONFIG.replace( + " type: default", + " type: header\n header: X-Other", + ); + let err = resolve(&runtime(&text), &HeaderMap::new()).unwrap_err(); + assert_eq!(err.code, ErrorCode::ProxyNotResolved); + assert_eq!(err.status(), 404); + } } diff --git a/crates/doppel-proxy/src/upstream.rs b/crates/doppel-proxy/src/upstream.rs index fbf0867..ecfc878 100644 --- a/crates/doppel-proxy/src/upstream.rs +++ b/crates/doppel-proxy/src/upstream.rs @@ -153,12 +153,26 @@ pub async fn forward( let mut headers = sanitize_headers(&parts.headers); // reqwest derives Host from the URL; relaying the client's would send the - // wrong authority upstream. + // wrong authority upstream. What the client asked for is not thrown away + // though -- it goes on as `X-Forwarded-Host` below. + // + // Read before the removal, and from `parts.headers` rather than from the + // sanitized copy. HTTP/2 carries the authority in `:authority` rather than + // in a `Host` header, which `axum` surfaces on the URI, so both are tried. + let client_authority = parts + .headers + .get("host") + .and_then(|value| value.to_str().ok()) + .map(str::to_owned) + .or_else(|| parts.uri.authority().map(ToString::to_string)); headers.remove("host"); for name in resolve_headers { headers.remove(name.as_str()); } apply_forwarded_for(&mut headers, &parts.headers, peer); + if let Some(authority) = client_authority.as_deref() { + apply_forwarded_host_and_proto(&mut headers, &parts.headers, authority); + } if let Ok(value) = HeaderValue::from_str(request_id) { headers.insert("x-request-id", value); } @@ -178,7 +192,10 @@ pub async fn forward( let started = Instant::now(); let response = client - .request(parts.method, url) + // Cloned because the redirect rewriting below resolves the upstream's + // `Location` against the URL that was actually requested, and reqwest + // takes ownership here. + .request(parts.method, url.clone()) .headers(headers) .timeout(proxy.timeout) .body(upstream_body) @@ -192,7 +209,12 @@ pub async fn forward( let duration = started.elapsed(); let status = response.status(); - let relayed = sanitize_headers(response.headers()); + let mut relayed = sanitize_headers(response.headers()); + if proxy.rewrite_redirects + && let Some(rewritten) = rewritten_location(&relayed, &url, &proxy.base_url) + { + relayed.insert("location", rewritten); + } let stream = response.bytes_stream(); let mut builder = Response::builder().status(status); @@ -267,6 +289,104 @@ fn apply_forwarded_for(target: &mut HeaderMap, original: &HeaderMap, peer: Optio } } +/// A `Location` the client can follow without leaving Doppel, or `None` when +/// the header should be relayed exactly as it arrived. +/// +/// The problem this solves is a consequence of not relaying `Host`: the upstream +/// answers a redirect with its *own* authority in `Location`, and a client that +/// follows it talks to the backend directly from then on -- past every injected +/// fault and every mock, with nothing logged and nothing failing. `nginx` has +/// `proxy_redirect` and Apache `ProxyPassReverse` for exactly this, both on by +/// default, because forgetting it is so easy. +/// +/// The value is resolved against the URL that was actually requested upstream, +/// which is what the upstream meant by it, and then judged: +/// +/// - Under `base` -- same origin, and a path below its path prefix -- it is +/// returned as a root-relative path plus query and fragment. Relative rather +/// than absolute so Doppel never has to guess its own public name: a client +/// resolves it against the URL it used, which is Doppel's. RFC 9110 has +/// allowed a relative `Location` since 7231 superseded 2616. +/// - Anywhere else it is returned in absolute form. That target is genuinely +/// elsewhere and rewriting it would be a lie, but stating it absolutely +/// removes a second, quieter bug: a root-relative `Location` meant against the +/// upstream's root (`/login` under a base of `/api/v1/`) would otherwise be +/// re-resolved against Doppel and forwarded back as `/api/v1/login`, which is +/// a different resource nobody asked for. +/// +/// Not covered: `Content-Location` names where a payload lives rather than where +/// to go next, `Refresh` is not a standard header, and `Set-Cookie`'s `Domain` +/// needs its own rewriting rule. Each would be its own decision. +fn rewritten_location( + relayed: &HeaderMap, + requested: &reqwest::Url, + base: &reqwest::Url, +) -> Option { + let location = relayed.get("location")?.to_str().ok()?; + let resolved = requested.join(location).ok()?; + + let base_path = base.path(); + let under_base = resolved.origin() == base.origin() + && resolved.path().starts_with(base_path) + // A base path is a directory prefix, so `/api/v1x` must not count as + // being under `/api/v1/`. `join_upstream` treats the base the same way. + && (base_path.ends_with('/') || resolved.path().len() == base_path.len()); + + let value = if under_base { + let mut tail = String::from("/"); + tail.push_str( + resolved + .path() + .trim_start_matches(base_path) + .trim_start_matches('/'), + ); + if let Some(query) = resolved.query() { + tail.push('?'); + tail.push_str(query); + } + if let Some(fragment) = resolved.fragment() { + tail.push('#'); + tail.push_str(fragment); + } + tail + } else { + resolved.to_string() + }; + + // Nothing to say if the header already reads that way -- returning `None` + // keeps the original bytes rather than a re-serialized equivalent. + if value == location { + return None; + } + HeaderValue::from_str(&value).ok() +} + +/// Records what the client asked for, since `Host` is not relayed: without +/// these the upstream cannot tell a request through Doppel from a direct one, +/// and cannot build a URL that points back here. +/// +/// An incoming value is left alone rather than overwritten. That is the same +/// treatment `X-Forwarded-For` gets above, and for the same reason: with Doppel +/// behind another proxy, the value that arrived names the authority the client +/// really used, and this hop's own `Host` is an internal detail. The cost is +/// that a client talking to Doppel directly can put whatever it likes in them -- +/// which is true of every proxy that preserves the chain, and is why these +/// headers are only ever as trustworthy as the hop that set them. +/// +/// `X-Forwarded-Proto` is `http` when Doppel sets it: its listeners are plain +/// TCP and it terminates no TLS, so `https` would be a claim about a hop that +/// does not exist here. +fn apply_forwarded_host_and_proto(target: &mut HeaderMap, original: &HeaderMap, authority: &str) { + if !original.contains_key("x-forwarded-host") + && let Ok(value) = HeaderValue::from_str(authority) + { + target.insert("x-forwarded-host", value); + } + if !original.contains_key("x-forwarded-proto") { + target.insert("x-forwarded-proto", HeaderValue::from_static("http")); + } +} + fn map_upstream_error(err: reqwest::Error) -> Error { if err.is_timeout() { Error::new(ErrorCode::UpstreamTimeout, "upstream timed out") @@ -576,6 +696,37 @@ mod tests { ) }), ) + // Three redirects that differ only in how the upstream spelled + // `Location`, which is what decides whether it can be rewritten: + // its own absolute URL, a path relative to its root, and a path + // relative to the current one. + .route( + "/redirect/self", + // The authority is read off the request rather than taken from + // an extractor: `axum::extract::Host` moved to `axum-extra` in + // axum 0.8, and this workspace does not depend on it. + any(|req: axum::extract::Request| async move { + let host = req + .headers() + .get("host") + .and_then(|v| v.to_str().ok()) + .unwrap_or("127.0.0.1") + .to_owned(); + ( + StatusCode::FOUND, + [("location", format!("http://{host}/moved?keep=1#frag"))], + "", + ) + }), + ) + .route( + "/redirect/rooted", + any(|| async { (StatusCode::FOUND, [("location", "/moved")], "") }), + ) + .route( + "/redirect/relative", + any(|| async { (StatusCode::FOUND, [("location", "sibling")], "") }), + ) // The 8 MiB streaming test below exceeds axum's default 2 MiB // whole-body extractor limit; this is a property of this mock // upstream's route, not of `forward`, so raise it here only. @@ -613,12 +764,32 @@ mod tests { loss: None, latency: None, replace: 1.0, + rewrite_redirects: true, resolve_header: None, mocks: Vec::new(), body_limit: 1024 * 1024, } } + /// The same proxy with redirect rewriting turned off, for the tests that + /// pin what `rewrite_redirects: false` relays. + fn proxy_relaying_redirects(base: &str) -> doppel_core::CompiledProxy { + doppel_core::CompiledProxy { + rewrite_redirects: false, + ..proxy(base) + } + } + + async fn location_of(response: axum::response::Response) -> String { + response + .headers() + .get("location") + .expect("a redirect must carry a Location") + .to_str() + .unwrap() + .to_owned() + } + fn request(method: Method, uri: &str) -> axum::extract::Request { axum::extract::Request::builder() .method(method) @@ -747,6 +918,73 @@ mod tests { ); } + /// `Host` is replaced by the upstream's own authority, so without these the + /// upstream has no way to learn what the client actually asked for and + /// cannot build a URL pointing back at Doppel. + #[tokio::test] + async fn sends_the_clients_authority_as_x_forwarded_host_and_http_as_the_proto() { + let base = upstream().await; + let client = reqwest::Client::new(); + let mut req = request(Method::GET, "/thing"); + req.headers_mut() + .insert("host", HeaderValue::from_static("public.example.com")); + + let (response, _) = fwd(&client, &proxy(&base), req, None).await.unwrap(); + let body: serde_json::Value = serde_json::from_str(&body_string(response).await).unwrap(); + let headers = body["headers"].as_array().unwrap(); + assert!( + headers + .iter() + .any(|h| h == "x-forwarded-host=public.example.com"), + "got {headers:?}" + ); + // `http`, not `https`: Doppel terminates no TLS, so claiming otherwise + // would describe a hop that does not exist. + assert!( + headers.iter().any(|h| h == "x-forwarded-proto=http"), + "got {headers:?}" + ); + } + + /// Behind another proxy, the value that arrived names the authority the + /// client really used; this hop's own `Host` is an internal detail. Same + /// treatment `X-Forwarded-For` gets, and the reason the two tests sit + /// together. + #[tokio::test] + async fn an_incoming_forwarded_host_and_proto_are_left_as_they_arrived() { + let base = upstream().await; + let client = reqwest::Client::new(); + let mut req = request(Method::GET, "/thing"); + req.headers_mut() + .insert("host", HeaderValue::from_static("inner.internal")); + req.headers_mut().insert( + "x-forwarded-host", + HeaderValue::from_static("edge.example.com"), + ); + req.headers_mut() + .insert("x-forwarded-proto", HeaderValue::from_static("https")); + + let (response, _) = fwd(&client, &proxy(&base), req, None).await.unwrap(); + let body: serde_json::Value = serde_json::from_str(&body_string(response).await).unwrap(); + let headers = body["headers"].as_array().unwrap(); + assert!( + headers + .iter() + .any(|h| h == "x-forwarded-host=edge.example.com"), + "the outermost authority must survive: {headers:?}" + ); + assert!( + headers.iter().any(|h| h == "x-forwarded-proto=https"), + "an upstream TLS terminator's proto must survive: {headers:?}" + ); + assert!( + !headers + .iter() + .any(|h| h == "x-forwarded-host=inner.internal"), + "this hop's own Host must not replace it: {headers:?}" + ); + } + #[tokio::test] async fn appends_to_x_forwarded_for_rather_than_replacing_it() { let base = upstream().await; @@ -856,6 +1094,9 @@ mod tests { .await .unwrap(); assert_eq!(response.status(), StatusCode::FOUND); + // Untouched even with rewriting on: `example.invalid` is not under this + // proxy's base, so the target really is elsewhere and claiming otherwise + // would be a lie. See `rewritten_location`. assert_eq!( response.headers().get("location").unwrap(), "http://example.invalid/target" @@ -863,6 +1104,110 @@ mod tests { assert_eq!(outcome.status, 302); } + /// The default, and the whole reason it is the default: `Host` is replaced + /// with the upstream's authority, so the upstream answers with its own name + /// in `Location`, and a client following that leaves Doppel -- past every + /// injected fault and every mock, with nothing logged. + /// + /// Rewritten to a root-relative path rather than an absolute URL so Doppel + /// never has to guess its own public name; the client resolves it against + /// the URL it used. Query and fragment survive. + #[tokio::test] + async fn a_redirect_into_the_proxied_space_is_rewritten_to_point_back_at_doppel() { + let base = upstream().await; + let client = reqwest::Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .build() + .unwrap(); + + let (response, _) = fwd( + &client, + &proxy(&base), + request(Method::GET, "/redirect/self"), + None, + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::FOUND); + assert_eq!(location_of(response).await, "/moved?keep=1#frag"); + } + + /// `rewrite_redirects: false` relays the header byte for byte, which is what + /// a client under test *for its redirect handling* needs to see. + #[tokio::test] + async fn rewrite_redirects_false_relays_the_upstreams_own_authority() { + let base = upstream().await; + let client = reqwest::Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .build() + .unwrap(); + + let (response, _) = fwd( + &client, + &proxy_relaying_redirects(&base), + request(Method::GET, "/redirect/self"), + None, + ) + .await + .unwrap(); + let location = location_of(response).await; + assert!( + location.starts_with("http://127.0.0.1:") && location.ends_with("/moved?keep=1#frag"), + "expected the upstream's own absolute URL, got {location}" + ); + } + + /// A base with a path prefix is where the quieter bug lives. The upstream + /// means `/moved` against its own root, which is *outside* the `/api/v1/` + /// prefix; relayed as-is, the client would resolve it against Doppel and + /// come back asking for `/api/v1/moved` -- a different resource nobody named. + /// Stated absolutely, the escape is at least honest. + #[tokio::test] + async fn a_root_relative_redirect_outside_the_base_path_is_made_absolute() { + let base = upstream().await; + let client = reqwest::Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .build() + .unwrap(); + + let mut with_prefix = proxy(&base); + with_prefix.base_url = url(&format!("{base}redirect/")); + + // `/rooted` under the base `…/redirect/` reaches the upstream's + // `/redirect/rooted` route, which answers `Location: /moved`. + let (response, _) = fwd(&client, &with_prefix, request(Method::GET, "/rooted"), None) + .await + .unwrap(); + let location = location_of(response).await; + assert_eq!(location, format!("{base}moved")); + assert!( + !location.starts_with('/'), + "a target outside the base cannot be expressed relative to Doppel: {location}" + ); + } + + /// A `Location` relative to the current path resolves inside the base, so it + /// comes back relative -- and correctly, which relaying would not have been: + /// the upstream meant `sibling` next to `/redirect/relative`. + #[tokio::test] + async fn a_path_relative_redirect_inside_the_base_stays_relative() { + let base = upstream().await; + let client = reqwest::Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .build() + .unwrap(); + + let (response, _) = fwd( + &client, + &proxy(&base), + request(Method::GET, "/redirect/relative"), + None, + ) + .await + .unwrap(); + assert_eq!(location_of(response).await, "/redirect/sibling"); + } + #[tokio::test] async fn error_response_renders_the_documented_envelope_for_upstream_timeout() { let err = Error::new(ErrorCode::UpstreamTimeout, "upstream timed out"); diff --git a/crates/doppel-store-postgres/migrations/0002_proxy_rewrite_redirects.sql b/crates/doppel-store-postgres/migrations/0002_proxy_rewrite_redirects.sql new file mode 100644 index 0000000..48d9118 --- /dev/null +++ b/crates/doppel-store-postgres/migrations/0002_proxy_rewrite_redirects.sql @@ -0,0 +1,15 @@ +-- `ProxyConfig::rewrite_redirects`: whether a redirect pointing back into the +-- space a proxy forwards is rewritten to point at Doppel. +-- +-- Nullable, and deliberately not `DEFAULT TRUE`. The field is `Option`, +-- where absent means "use the default" and is a different document from `true` +-- written out: the revision is derived from the document's content, so a store +-- that materialised the default would hand back a configuration with a +-- different revision from the one that was saved. Every other optional column +-- in this table is nullable for the same reason -- see `replace_ratio`. +-- +-- A separate migration rather than an edit to 0001: sqlx records a checksum per +-- migration, so editing one already applied is reported as tampering by +-- `config migrate --status`, which is the property that bookkeeping was chosen +-- for. +ALTER TABLE proxies ADD COLUMN rewrite_redirects BOOLEAN; diff --git a/crates/doppel-store-postgres/migrations/0003_admin_groups.sql b/crates/doppel-store-postgres/migrations/0003_admin_groups.sql new file mode 100644 index 0000000..9653581 --- /dev/null +++ b/crates/doppel-store-postgres/migrations/0003_admin_groups.sql @@ -0,0 +1,11 @@ +-- `AdminConfig::groups`: which names `access` may reference (rule V36). +-- +-- Nullable, and deliberately without `DEFAULT '["*"]'`. The field is an +-- `Option>` where absent means "any", and absent is a +-- different document from `["*"]` written out -- not in meaning, but in bytes. +-- The revision is derived from the document's canonical YAML, so a column that +-- materialised the default would hand back a configuration whose computed +-- revision no longer matched the stored one, and every configuration saved +-- before this migration would fail its own revision check on the first load +-- after it. `proxies.rewrite_redirects` is nullable for the same reason. +ALTER TABLE configurations ADD COLUMN admin_groups JSONB; diff --git a/crates/doppel-store-postgres/src/load.rs b/crates/doppel-store-postgres/src/load.rs index 56e3e80..7d425b1 100644 --- a/crates/doppel-store-postgres/src/load.rs +++ b/crates/doppel-store-postgres/src/load.rs @@ -114,6 +114,7 @@ impl PostgresStore { }, tokens, access: json_column::(row, "admin_access")?, + groups: optional_json::>(row, "admin_groups")?, upload: UploadConfig { limit: byte_size(row, "admin_upload_limit")?, }, @@ -138,7 +139,10 @@ impl PostgresStore { name, kind: match text(row, "kind")?.as_str() { "http" => ProxyKind::Http, - "tcp" => ProxyKind::Tcp, + // Including `tcp`, which earlier versions of this schema + // could store: the variant is gone, so a row holding it is + // a configuration this binary cannot serve, reported rather + // than silently coerced to `http`. other => return Err(corrupt("proxies.kind", &format!("is `{other}`"))), }, url: url(row, "url")?, @@ -158,6 +162,7 @@ impl PostgresStore { loss: loss_from(row)?, latency: latency_from(row)?, replace: optional_ratio(row, "replace_ratio")?, + rewrite_redirects: row.try_get("rewrite_redirects").map_err(query_failed)?, body_limit: byte_size(row, "body_limit")?, }); } @@ -289,7 +294,13 @@ fn text(row: &PgRow, column: &str) -> Result { /// by hand can hold one the configuration format would refuse. Parsing here /// means a `Config` this store produces is subject to the same rule as one /// read from YAML, rather than a second, laxer standard nobody wrote down. -fn name(row: &PgRow, column: &str) -> Result { +/// Generic over the cap so one function serves both a `Name` and the tighter +/// `ProxyName`, and the column is checked against the limit that type actually +/// carries rather than against whichever one this helper happened to name. +fn name( + row: &PgRow, + column: &str, +) -> Result, StoreError> { let raw: String = row.try_get(column).map_err(query_failed)?; doppel_core::config::Name::parse(raw).map_err(|err| corrupt(column, &err.to_string())) } diff --git a/crates/doppel-store-postgres/src/save.rs b/crates/doppel-store-postgres/src/save.rs index 205f5ab..75390ed 100644 --- a/crates/doppel-store-postgres/src/save.rs +++ b/crates/doppel-store-postgres/src/save.rs @@ -173,9 +173,10 @@ impl PostgresStore { ) -> Result<(), StoreError> { sqlx::query( "INSERT INTO proxies (config, name, ordinal, kind, url, timeout_seconds, body_limit, \ - replace_ratio, resolve_kind, resolve_header, loss_percentage, loss_status, \ - latency_percentage, latency_min, latency_max, headers, access) \ - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17)", + replace_ratio, rewrite_redirects, resolve_kind, resolve_header, loss_percentage, \ + loss_status, latency_percentage, latency_min, latency_max, headers, access) \ + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, \ + $18)", ) .bind(&self.config_name) .bind(proxy.name.as_str()) @@ -189,6 +190,7 @@ impl PostgresStore { ) .bind(i64::try_from(proxy.body_limit.get()).unwrap_or(i64::MAX)) .bind(proxy.replace.map(doppel_core::config::Ratio::get)) + .bind(proxy.rewrite_redirects) .bind(as_text(&proxy.resolve.kind)?) .bind( proxy @@ -266,19 +268,21 @@ const UPDATE_HEADER: &str = "UPDATE configurations SET revision = $1, \ admin_enable = $4, server_host = $5, server_port = $6, log_level = $7, \ log_format = $8, control_socket = $9, templates_dir = $10, sentry_dsn = $11, \ admin_host = $12, admin_port = $13, admin_auth_header = $14, \ - admin_upload_limit = $15, admin_access = $16, updated_at = now() \ + admin_upload_limit = $15, admin_access = $16, admin_groups = $17, \ + updated_at = now() \ WHERE name = $2 AND revision = $3"; const UPSERT_HEADER: &str = "INSERT INTO configurations \ (name, revision, admin_enable, server_host, server_port, log_level, log_format, \ control_socket, templates_dir, sentry_dsn, admin_host, admin_port, \ - admin_auth_header, admin_upload_limit, admin_access) \ - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15) \ + admin_auth_header, admin_upload_limit, admin_access, admin_groups) \ + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16) \ ON CONFLICT (name) DO UPDATE SET revision = $2, \ admin_enable = $3, server_host = $4, server_port = $5, log_level = $6, \ log_format = $7, control_socket = $8, templates_dir = $9, sentry_dsn = $10, \ admin_host = $11, admin_port = $12, admin_auth_header = $13, \ - admin_upload_limit = $14, admin_access = $15, updated_at = now()"; + admin_upload_limit = $14, admin_access = $15, admin_groups = $16, \ + updated_at = now()"; /// Bind the header values in `HEADER_COLUMNS` order. /// @@ -305,6 +309,16 @@ impl<'q> BindHeader<'q> for sqlx::query::Query<'q, Postgres, sqlx::postgres::PgA .bind(config.admin.auth.header.as_str()) .bind(i64::try_from(config.admin.upload.limit.get()).unwrap_or(i64::MAX)) .bind(serde_json::to_value(&config.admin.access).unwrap_or(serde_json::Value::Null)) + // `None` binds SQL NULL, which is what an absent `groups` has to + // round-trip as: see the 0003 migration for why materialising the + // default would break every pre-existing configuration's revision. + .bind( + config + .admin + .groups + .as_ref() + .map(|groups| serde_json::to_value(groups).unwrap_or(serde_json::Value::Null)), + ) } } @@ -375,6 +389,7 @@ mod tests { "admin_auth_header", "admin_upload_limit", "admin_access", + "admin_groups", ]; /// Every `column = $n` assignment in a statement. diff --git a/docs/overview/concepts.md b/docs/overview/concepts.md index aa995ca..9536e65 100644 --- a/docs/overview/concepts.md +++ b/docs/overview/concepts.md @@ -24,9 +24,11 @@ See [Several backends behind one port](../usage/multiple-backends.md). ## Fault A **fault** is a deliberate degradation: `loss` drops a share of requests with -a chosen status, `latency` delays a share of them by a random time in a range. -Both are properties of the proxy, so they apply before Doppel decides what will -answer the request. +a chosen status, `latency` makes a share of them take a random time in a range. + +Both are declared on the proxy, and a mock may declare its own. Which applies to +a given request depends on what answered it -- see +[How a request is handled](#how-a-request-is-handled) below. Shares are written as fractions. `0.1` is one request in ten; `50` is not fifty percent, it is refused. diff --git a/docs/usage/admin-api.md b/docs/usage/admin-api.md index 20b5e39..7b2d15c 100644 --- a/docs/usage/admin-api.md +++ b/docs/usage/admin-api.md @@ -243,6 +243,7 @@ and an oversized body all carry the envelope rather than an empty body. | `NOT_FOUND` | 404 | No such proxy, no such template file, or no such route | | `METHOD_NOT_ALLOWED` | 405 | The path exists and does not accept that verb; the response also carries `Allow` | | `PROXY_NOT_RESOLVED` | 404 | No proxy matched and there is no default | +| `NO_PROXIES_CONFIGURED` | 503 | The configuration names no proxies at all. See [No proxies configured](proxying.md#no-proxies-configured) | | `CONFLICT` | 409 | The name exists, or the store is under sustained contention | | `REVISION_MISMATCH` | 409 | The proxy changed since it was read | | `UPLOAD_TOO_LARGE` | 413 | A template body over `admin.upload.limit`, or a configuration document over 1 MiB | diff --git a/docs/usage/cli.md b/docs/usage/cli.md index 5f09699..8910975 100644 --- a/docs/usage/cli.md +++ b/docs/usage/cli.md @@ -132,6 +132,29 @@ so it answers the same on a developer's machine as in production. Checks that depend on the machine, such as whether the templates directory can be created, belong to `serve`. +## `config schema` + +Prints the configuration's JSON Schema on stdout. + +```bash +doppel config schema > doppel-config.schema.json +``` + +Takes no flags and reads nothing -- not even `--config`. The schema describes +what a configuration *may* contain, which is a property of the binary rather +than of any file, so accepting a path would only raise the question of whether +the answer depended on it. + +The same document is checked into the repository at +[`doppel-config.schema.json`](https://github.com/lorem-dev/doppel/blob/main/doppel-config.schema.json) +and attached to every release. See +[Editor support](configuration.md#editor-support) for pointing an editor at it. + +It is generated from the same types the admin API's OpenAPI document is built +from, so it cannot describe a field that does not exist. `scripts/config_schema.py` +regenerates the checked-in copy and `--check` fails when it is stale; CI runs +the check. + ## `config reload` Connects to the control socket and asks a running server to reload. diff --git a/docs/usage/configuration.md b/docs/usage/configuration.md index e2229ad..4330142 100644 --- a/docs/usage/configuration.md +++ b/docs/usage/configuration.md @@ -6,13 +6,40 @@ a mistyped field name fails at load rather than doing nothing at runtime. `main.example.yaml` in the repository is this reference made concrete, and is asserted against by the test suite. +## Editor support + +The configuration has a JSON Schema, so an editor can complete field names, show +what each field is for and mark a bad value as you type -- before Doppel is run +at all. Put this line at the top of your `main.yaml`: + +```yaml +# yaml-language-server: $schema=https://raw.githubusercontent.com/lorem-dev/doppel/main/doppel-config.schema.json +``` + +VS Code's YAML extension reads it, as does any other `yaml-language-server` +client. `main.example.yaml` already carries it. + +That URL follows `main`. Every release also attaches the schema as an asset, so +a deployment that pins a version can validate against the schema for exactly +that version rather than for whatever is current. + +The schema is generated from the same Rust types this page documents -- see +[`doppel config schema`](cli.md#config-schema) -- so it cannot describe a field +that does not exist, and CI fails if the checked-in copy falls behind. + +What it catches: an unknown key, `percentage: 45` where a fraction was meant, +`method: get` in lower case, a port of `0`, a missing `url`. What it cannot +catch: anything needing more than one field, such as `min <= max`. Those are the +[validation rules](#validation) below, and they run when the configuration is +loaded. + ## Top level | Key | Required | Purpose | |---|---|---| | `server` | yes | Where the proxy listens | | `admin` | yes | Admin API settings | -| `proxies` | yes | At least one proxy | +| `proxies` | no | Empty or absent is legal; requests then get `503` | | `logging` | no | Level and format; defaults to `info` and `json` | | `control` | no | Control socket path; defaults to `/tmp/doppel.sock` | | `templates` | no | Template directory; defaults to `./templates` | @@ -90,6 +117,7 @@ admin: enable: true host: "0.0.0.0" port: 8081 + groups: ["*"] auth: header: X-Proxy-Authorization tokens: @@ -131,6 +159,70 @@ action out entirely, which is far more often a typo than an intent. `access` maps each action to `public`, a single name, or a list of names. An empty list means public. Names are token names or group names. +### `groups`: which names `access` may reference + +```yaml +admin: + groups: ["*"] # the default: any name may be referenced +``` + +`groups` bounds the vocabulary `access` may draw on -- both here and in a +proxy's `access` overrides. It is checked by rule **V36**. + +| `groups` | What `access` may reference | +|---|---| +| absent | any name. This is the default | +| `["*"]` | any name. The same thing, written down | +| `["admin", "ci"]` | `admin` and `ci`, and nothing else. `user` is refused | +| `[]` | no name at all: every one of the six actions has to be spelled `public` | + +`public` is **never** governed by `groups`. It is the absence of a subject rather +than a name, so an allow-list has nothing to say about it -- and a deployment +locked down to `groups: []` still has to be able to say "anyone may read this". + +!!! warning "`groups: []` is not a small change" + Every action defaults to the `admin` group, and `groups: []` forbids naming + `admin`. So the defaults themselves become violations, and a configuration + with `groups: []` is only valid if **all six** actions are written out as + `public`: + + ``` + admin.access.list: `admin` is not an allowed group: `admin.groups` is empty, so only `public` may be used + admin.access.create: `admin` is not an allowed group: ... + admin.access.update: `admin` is not an allowed group: ... + ``` + + That is what an empty allow-list means, taken literally, and it is reported + per action rather than once, so nothing is missed on the way to fixing it. + If the intent was "no *custom* groups", name the ones you do want -- + `["admin"]` -- rather than emptying the list. + +The default is permissive, which is the opposite of how `access` itself +defaults, on purpose. `access` defaults to `admin` because the cost of getting it +wrong is unauthenticated writes. `groups` defaults to `*` because the cost of +getting it wrong is an operator unable to name their own groups, and an +allow-list nobody asked for only ever surprises. + +The violation names what is permitted, not just what was refused: + +``` +admin.access.read: `user` is not an allowed group: `admin.groups` allows only `admin`, `ci` +proxies[0].access.update: `admin` is not an allowed group: `admin.groups` is empty, so only `public` may be used +``` + +That matters because the reader has to choose between changing the reference and +widening the list, and cannot do either without seeing the list. + +Two things `groups` does not do. It does not create groups -- a name still has to +be a predefined group or one carried by a token, which is rule V27, and the two +rules are reported separately because widening `groups` and adding a token are +different fixes. And it is not authorisation: it constrains what a +*configuration* may say, so a caller's rights come from `access` as before. + +Since it applies to proxy overrides too, `POST` and `PUT /api/v1/proxies` refuse +a document whose `access` names something outside the list, with `400` and +`CONFIG_INVALID`. + Every action defaults to the `admin` group, reads included. The most common configuration is the one nobody wrote, so the default has to be the safe one. @@ -163,7 +255,8 @@ the choice is the operator's. ## `proxies` -At least one. Names must be unique. +Names must be unique. The list may be empty or left out -- see +[No proxies configured](proxying.md#no-proxies-configured). ```yaml proxies: @@ -192,7 +285,7 @@ proxies: | Key | Type | Default | Notes | |---|---|---|---| | `name` | string | required | Unique; also used as the template subdirectory | -| `type` | `http` | required | `tcp` is rejected with a message saying it is not implemented | +| `type` | `http` | required | The only value. `tcp` is refused while parsing, with a message saying it is not implemented | | `url` | absolute URL | required | `http` or `https`, no query or fragment | | `timeout` | seconds > 0 | 30 | Bounds the whole upstream exchange | | `body_limit` | byte size > 0 | 1 MiB | Only used when a matched mock extracts from the body | @@ -202,6 +295,7 @@ proxies: | `loss` | see below | none | | | `latency` | see below | none | | | `replace` | 0.0..1.0 | 1.0 | Probability a matching mock actually answers | +| `rewrite_redirects` | boolean | `true` | Point a redirect back at Doppel when its target is under this proxy's base. See [Redirects](proxying.md#redirects) | | `mocks` | list | none | See [Mocks and templating](mocks.md) | ### `resolve` @@ -238,6 +332,63 @@ A mock that extracts variables from the request body has to buffer it, which the proxy otherwise avoids -- bodies stream through. This bounds that buffer. Exceeding it is `413`. See [Mocks and templating](mocks.md#bodies-and-the-size-limit). +### `mocks[]` + +```yaml + mocks: + - name: pricing + request: + method: GET + url: "^/pricing/(?P[0-9]+)/$" + headers: + who: X-User + query: + page: .page + body: + items: .content.items + response: + status: 200 + json: '{"id": "{{ id }}", "page": "{{ page }}"}' + headers: + X-Served-By: "mock {{ id }}" + proxy: + replace: 0.5 +``` + +| Key | Type | Default | Notes | +|---|---|---|---| +| `name` | string | required | Unique within the proxy | +| `request` | see below | required | What the mock matches | +| `response` | see below | required | What it answers | +| `proxy` | see below | none | Per-mock overrides | + +`request`: + +| Key | Type | Default | Notes | +|---|---|---|---| +| `method` | upper-case method | required | Matched exactly; `get` is rejected at load | +| `url` | regex | required | Matched against the path, unanchored. Named groups become variables | +| `headers` | variable → header name | none | | +| `query` | variable → selector | none | | +| `body` | variable → selector | none | Buying the buffer bounded by `body_limit` | + +`response` -- exactly one of `body`, `json` or `template`: + +| Key | Type | Default | Notes | +|---|---|---|---| +| `status` | 100..599 | required | | +| `body` | template | none | Sent as `text/plain` | +| `json` | template | none | Sent as `application/json`; must render to valid JSON | +| `template` | file name | none | A file under this proxy's template directory | +| `headers` | header name → template | none | The value is a template, rendered per request | + +`proxy` accepts `replace`, `loss` and `latency`, with the same types and bounds +as on the proxy. What is inherited and what is not is in +[Injecting faults](faults.md#faults-on-one-endpoint-only). + +Every variable a template names has to be bound, or the render fails with +`500` -- see [Mocks and templating](mocks.md#rendering-is-strict). + ## Validation The rule set runs identically at startup, on reload, and under @@ -273,6 +424,8 @@ parsed. A message quoted in an old issue can be looked up here. | V2 | `server.host` is an IP | `IpAddr` | | V3 | `server.workers` is positive | the field is `--workers` | | V4 | log level and format are known | `LogLevel`, `LogFormat` | +| V5 | at least one proxy is configured | nothing -- an empty list is legal, see [No proxies configured](proxying.md#no-proxies-configured) | +| V7 | `type: tcp` is refused | `ProxyKind`, while the document is parsed | | V8, V32 | upstream url is absolute http(s), no query | [`UpstreamUrl`](#upstream-urls) | | V9 | timeout is positive | [`TimeoutSeconds`](#numbers-with-units) | | V12, V13 | probability in 0..=1, status in 100..=599 | [`Ratio`](#numbers-with-units), [`HttpStatus`](#methods-and-statuses) | @@ -289,15 +442,26 @@ parsed. A message quoted in an old issue can be looked up here. A retired number is never reused. -Sixteen rules remain: V1, V5, V6, V7, V10, V11, V14, V16, V19, V20, V21, V25, -V26, V27, V30 and V34. Each needs more than one field to decide, which is -exactly why none of them could become a type. +Fifteen rules remain: V1, V6, V10, V11, V14, V16, V19, V20, V21, V25, V26, V27, +V30, V34 and V36. Each needs more than one field to decide, which is exactly why +none of them could become a type -- V36, the newest, compares `access` against +`admin.groups`. ## Names A proxy name, a mock name, a token name and a group name follow one rule: -letters, digits, `.`, `-` and `_`, between 2 and 128 characters, not starting -with a dot and not containing `..`. +letters, digits, `-` and `_`, between 2 and 64 characters. A **proxy** name is +capped at 32 instead. + +`.` is not allowed. It was until 0.3.0, and the reference configuration taught +names like `Billing.API.v2`; write `Billing-API-v2`. Dropping it removed two +further rules with it -- a name becomes a directory component, so `.hidden` and +`..` each had to be refused separately, and neither can now be written at all. + +A proxy name is capped shorter because it travels further than any other: a +directory under `templates.dir`, a `proxy` label on every metric, a field in +every log line, and the value a client puts in a resolution header on every +request. The rule is enforced by the type, while the document is being parsed, rather than by a validation rule afterwards. A name becomes a directory component, a diff --git a/docs/usage/observability.md b/docs/usage/observability.md index eca3aed..ee0a22e 100644 --- a/docs/usage/observability.md +++ b/docs/usage/observability.md @@ -30,12 +30,21 @@ Every request logs once, on completion, with the same key set on every branch: | `duration_ms` | Time to the response headers | | `upstream_contacted` | Whether an upstream was reached at all | | `upstream_status`, `upstream_duration_ms` | Present only when it was | -| `loss_injected`, `latency_injected_ms` | Which faults fired | +| `loss_injected` | Whether the loss roll dropped this request | +| `latency_injected_ms` | How long the request was actually made to wait | `upstream_contacted` is a boolean rather than a null status because a null still invites a consumer to plot it, where a boolean says what happened. A dropped request, an unresolved one and a mocked one all report `false`. +`latency_injected_ms` is the wait taken, not the delay drawn. An injected +latency is a target for the whole response, so an upstream that already spent +longer than the target leaves nothing to wait for and this reads `0` even though +the roll fired. `doppel_latency_injected_total` counts the roll, so the two +disagree in exactly that case -- deliberately, since "how often latency was in +play" and "how much of it this request felt" are different questions. See +[Injecting faults](faults.md#the-delay-is-a-target-not-an-addition). + `duration_ms` and `upstream_duration_ms` both stop at the response headers, not at the end of the body. A large download is not reflected in either. diff --git a/docs/usage/proxying.md b/docs/usage/proxying.md index 0da7dc0..a3958df 100644 --- a/docs/usage/proxying.md +++ b/docs/usage/proxying.md @@ -1,5 +1,35 @@ # Proxy behaviour +## No proxies configured + +An empty `proxies` list, or no `proxies` key at all, is a valid configuration. +Doppel starts, binds both listeners, serves the admin API, and waits. + +A request arriving meanwhile is answered: + +```json +{ + "status": 503, + "message": "no proxies are configured; add one and reload", + "code": "NO_PROXIES_CONFIGURED" +} +``` + +`503`, not `404`. Nothing is wrong with the request -- the service is not in a +position to answer one yet. A `404` would tell the caller their path was wrong +and send whoever is debugging the client into the client. `503` also carries the +right invitation: add a proxy, reload, and the next attempt works, with no +restart. + +This is deliberately not a startup failure. Rule V5 used to refuse it, which +meant a fresh deployment could not come up until its proxies were written -- +so the two ways of adding one, `doppel config reload` and the admin API, were +both unreachable exactly when they were most useful. Provisioning an empty +Doppel and filling it over the API is now a supported order of operations. + +`NO_PROXIES_CONFIGURED` is distinct from `PROXY_NOT_RESOLVED` (`404`), which +means proxies exist and none of them wanted this particular request. + ## Choosing a proxy Several proxies can sit behind one port. A proxy either declares itself the @@ -62,8 +92,24 @@ Headers configured on the proxy are injected into the outbound request and override anything the client sent by the same name. The resolution headers are stripped, so the upstream does not learn Doppel's routing vocabulary. -`X-Forwarded-For` is appended to rather than replaced, preserving any chain -that arrived. +Because `Host` is replaced, what the client asked for is sent on instead: + +| Header | Value | +|---|---| +| `X-Forwarded-Host` | the authority the client used | +| `X-Forwarded-Proto` | `http` -- Doppel terminates no TLS, so `https` would name a hop that does not exist | +| `X-Forwarded-For` | the chain that arrived, with the peer appended | + +The first two are only set when the request did not already carry them, and +`X-Forwarded-For` is appended to rather than replaced. All three preserve what +arrived, so Doppel behind another proxy keeps the authority the client really +used rather than substituting an internal one. The flip side is that a client +talking to Doppel directly can put whatever it likes in them -- true of any proxy +that preserves a chain, and the reason these headers are only ever as +trustworthy as the hop that set them. + +`X-Forwarded-Port` and RFC 7239 `Forwarded` are not generated. One arriving from +a client is relayed untouched. `X-Request-ID` is reused if the client sent one and generated otherwise, sent upstream, and returned on the response, so one request can be followed across @@ -71,9 +117,50 @@ services. ## Redirects -A `3xx` from the upstream is relayed to the caller with its `Location` intact, -not followed. The redirect target is the client's decision, and a streamed -request body could not be replayed to it anyway. +A `3xx` from the upstream is relayed to the caller, not followed. The redirect +target is the client's decision, and a streamed request body could not be +replayed to it anyway. + +Its `Location` is rewritten to keep the client behind the proxy: + +```yaml +proxies: + - name: backend + url: "https://api.example.com/v2/" + rewrite_redirects: true # the default +``` + +With a base of `https://api.example.com/v2/`, an upstream answering +`Location: https://api.example.com/v2/orders/7` produces `Location: /orders/7` +to the client. Query and fragment survive. + +Relative rather than absolute, so Doppel never has to guess its own public +name: the client resolves it against the URL it used, which is Doppel's. + +!!! warning "Why this is on by default" + `Host` is not relayed, so the upstream answers with its *own* authority in + `Location`. Relayed untouched, a client following it talks to the backend + directly from then on -- past every injected fault and every mock, with + nothing logged and nothing failing. The test still passes; it has just + stopped testing anything. + + `nginx` has `proxy_redirect` for this and Apache `ProxyPassReverse`, both on + by default, for the same reason. + +A target that is **not** under the proxy's base is genuinely elsewhere and is +left pointing there, stated as an absolute URL. That covers a subtler case too: +an upstream answering `Location: /login` under a base of `/v2/` means its own +root, so relaying the header as-is would have the client come back asking for +`/v2/login` -- a different resource nobody named. + +Set `rewrite_redirects: false` to relay the header byte for byte. That is what a +client being tested *for its redirect handling* needs; it is not what a client +being tested against a degraded backend needs. + +Only `Location` is rewritten. `Content-Location` names where a payload lives +rather than where to go next, `Refresh` is not a standard header, and the +`Domain` attribute of a `Set-Cookie` needs its own rule -- none of the three is +touched. ## Faults diff --git a/doppel-config.schema.json b/doppel-config.schema.json new file mode 100644 index 0000000..d22af5e --- /dev/null +++ b/doppel-config.schema.json @@ -0,0 +1,841 @@ +{ + "$defs": { + "AccessConfig": { + "additionalProperties": false, + "properties": { + "create": { + "$ref": "#/$defs/Subjects", + "description": "Add a proxy. Refused for `public` by rule V34." + }, + "delete": { + "$ref": "#/$defs/Subjects", + "description": "Remove a proxy. Refused for `public` by rule V34." + }, + "list": { + "$ref": "#/$defs/Subjects", + "description": "List the proxies. A listing exposes upstream URLs and injected headers." + }, + "read": { + "$ref": "#/$defs/Subjects", + "description": "Read one proxy document, credentials in its `url` included." + }, + "update": { + "$ref": "#/$defs/Subjects", + "description": "Replace a proxy. Refused for `public` by rule V34." + }, + "upload": { + "$ref": "#/$defs/Subjects", + "description": "Upload or delete a template file. Refused for `public` by rule V34." + } + }, + "type": "object" + }, + "AdminConfig": { + "additionalProperties": false, + "properties": { + "access": { + "$ref": "#/$defs/AccessConfig", + "description": "Who may perform each admin action. Every action defaults to the\n`admin` group, reads included." + }, + "auth": { + "$ref": "#/$defs/AuthConfig", + "description": "Which header carries the bearer token." + }, + "enable": { + "description": "Whether to run the admin listener at all.\n\nDefaults to on. Off means the port is never bound and no admin task\nstarts; the proxy and the control socket are untouched, so\n`doppel config reload` still works and is then the only way in.\n\nThe validation rules do not consult this. A configuration that is only\nsafe because nothing serves it is a trap set for whoever turns the\nlistener on later, and they will not re-read the rules first.", + "type": "boolean" + }, + "groups": { + "description": "Which names `access` may reference, here and in a proxy's overrides.\n\n`[\"*\"]`, the default, allows any. A concrete list allows exactly those:\n`[\"admin\", \"ci\"]` permits `admin` and `ci` and refuses `user`.\n\n`[]` allows none, and that is stronger than it looks: every action\ndefaults to the `admin` group, so an empty list makes the defaults\nthemselves violations and every one of the six actions has to be written\nout as `public`.\n\n`public` is never governed by this. It is the absence of a subject rather\nthan a name, so an allow-list has nothing to say about it.\n\nChecked by rule V36, not by this type: it compares one field against\nanother, which is what is left for the rule set once the types have taken\neverything they can decide alone.\n\nAn `Option` rather than a `Vec` defaulting to `[\"*\"]`, and skipped when\nabsent, so that adding this field did not change the canonical YAML of\nevery configuration written before it existed. The revision is derived\nfrom that YAML, so a materialised default would have made every stored\nconfiguration fail its own revision check on the first load after the\nupgrade. Read it through [`AdminConfig::allowed_groups`].", + "items": { + "$ref": "#/$defs/AllowedGroup" + }, + "type": [ + "array", + "null" + ] + }, + "host": { + "description": "An IP address, not a hostname: a name would have to be resolved,\nand which address it resolves to is not the configuration's to\ndecide. `utoipa` has no schema for `IpAddr`, so it is described\nhere as the string it is written as.", + "examples": [ + "127.0.0.1" + ], + "type": "string" + }, + "port": { + "$ref": "#/$defs/Port", + "description": "The TCP port the admin API listens on. Must differ from\n`server.port`." + }, + "tokens": { + "description": "The tokens that may call the admin API. Names and token values are\neach unique. `DOPPEL_ADMIN_TOKENS` can supply these instead.", + "items": { + "$ref": "#/$defs/TokenConfig" + }, + "type": "array" + }, + "upload": { + "$ref": "#/$defs/UploadConfig", + "description": "Bounds an uploaded template file." + } + }, + "required": [ + "host", + "port", + "upload" + ], + "type": "object" + }, + "AllowedGroup": { + "description": "A token or group name `access` may reference, or `*` for any of them.", + "examples": [ + "*", + "admin" + ], + "pattern": "^(\\*|[A-Za-z0-9_-]{2,64})$", + "type": "string" + }, + "AuthConfig": { + "additionalProperties": false, + "properties": { + "header": { + "$ref": "#/$defs/HeaderName", + "description": "The header a caller presents its token in, as `Bearer `.\nDefaults to `X-Proxy-Authorization`." + } + }, + "type": "object" + }, + "ByteSize": { + "description": "A byte count from 1 to 1073741824, as a plain integer or with a binary (`Ki`, `Mi`, `Gi`) or decimal (`kB`, `MB`, `GB`) suffix. Always serialized back as an integer.", + "oneOf": [ + { + "examples": [ + 1048576 + ], + "maximum": 1073741824, + "minimum": 1, + "type": "integer" + }, + { + "examples": [ + "1Mi" + ], + "type": "string" + } + ] + }, + "ControlConfig": { + "additionalProperties": false, + "properties": { + "socket": { + "description": "Path to the control socket, created with mode 0600 and removed on\nshutdown. Its parent directory must already exist.\nA filesystem path. `utoipa` has no schema for `PathBuf`, so it is\ndescribed as the string it is written as.", + "type": "string" + } + }, + "type": "object" + }, + "HeaderName": { + "description": "An HTTP header name: a non-empty RFC 9110 token.", + "type": "string" + }, + "HeaderValue": { + "description": "An HTTP header value: visible ASCII, space and tab, with no line breaks.", + "type": "string" + }, + "HttpMethod": { + "description": "An HTTP method, upper case.", + "enum": [ + "GET", + "HEAD", + "POST", + "PUT", + "PATCH", + "DELETE", + "OPTIONS", + "TRACE", + "CONNECT", + "QUERY", + "PROPFIND", + "PROPPATCH", + "MKCOL", + "COPY", + "MOVE", + "LOCK", + "UNLOCK" + ], + "type": "string" + }, + "HttpStatus": { + "description": "An HTTP status code, 100 to 599.", + "maximum": 599, + "minimum": 100, + "type": "integer" + }, + "LatencyConfig": { + "additionalProperties": false, + "properties": { + "max": { + "$ref": "#/$defs/Seconds", + "description": "Upper bound of the delay, in seconds. Must be at least `min`." + }, + "min": { + "$ref": "#/$defs/Seconds", + "description": "Lower bound of the delay, in seconds. The delay is a target for the\nwhole response: time the upstream already spent is subtracted." + }, + "percentage": { + "$ref": "#/$defs/Ratio", + "description": "The share of requests to delay, as a fraction." + } + }, + "required": [ + "percentage", + "min", + "max" + ], + "type": "object" + }, + "LogFormat": { + "enum": [ + "json", + "text" + ], + "type": "string" + }, + "LogLevel": { + "enum": [ + "trace", + "debug", + "info", + "warn", + "error" + ], + "type": "string" + }, + "LoggingConfig": { + "additionalProperties": false, + "properties": { + "format": { + "$ref": "#/$defs/LogFormat", + "description": "`json` for machines, `text` for a terminal." + }, + "level": { + "$ref": "#/$defs/LogLevel", + "description": "The lowest level that is logged. `RUST_LOG` overrides it when set and\nnon-empty." + } + }, + "type": "object" + }, + "LossConfig": { + "additionalProperties": false, + "properties": { + "percentage": { + "$ref": "#/$defs/Ratio", + "description": "The share of requests to drop, as a fraction. `0.1` is one in ten." + }, + "status": { + "$ref": "#/$defs/HttpStatus", + "description": "The status a dropped request is answered with, rather than being left\nto hang." + } + }, + "required": [ + "percentage", + "status" + ], + "type": "object" + }, + "MockConfig": { + "additionalProperties": false, + "properties": { + "name": { + "$ref": "#/$defs/Name", + "description": "Names this mock in the `mock` log field and the hit counter. Unique\nwithin the proxy." + }, + "proxy": { + "description": "Per-mock overrides of the proxy's `replace`, `loss` and `latency`.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/MockProxyOverride" + } + ] + }, + "request": { + "$ref": "#/$defs/MockRequest", + "description": "What this mock matches, and what it takes out of the request." + }, + "response": { + "$ref": "#/$defs/MockResponse", + "description": "What it answers with." + } + }, + "required": [ + "name", + "request", + "response" + ], + "type": "object" + }, + "MockProxyOverride": { + "additionalProperties": false, + "properties": { + "latency": { + "description": "Replaces the proxy's `latency` for this mock's responses, rather than\nadding to it.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/LatencyConfig" + } + ] + }, + "loss": { + "description": "Drops a share of the requests this mock would have answered. Not\ninherited from the proxy: a mock without it is never dropped.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/LossConfig" + } + ] + }, + "replace": { + "description": "Overrides the proxy's `replace` for requests this mock matches.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/Ratio" + } + ] + } + }, + "type": "object" + }, + "MockRequest": { + "additionalProperties": false, + "properties": { + "body": { + "additionalProperties": { + "$ref": "#/$defs/Selector" + }, + "description": "Variable name -> selector such as `.content.items`.", + "propertyNames": { + "type": "string" + }, + "type": "object" + }, + "headers": { + "additionalProperties": { + "$ref": "#/$defs/HeaderName" + }, + "description": "Variable name -> request header name.", + "propertyNames": { + "type": "string" + }, + "type": "object" + }, + "method": { + "$ref": "#/$defs/HttpMethod", + "description": "The method this mock answers, matched exactly and upper case." + }, + "query": { + "additionalProperties": { + "$ref": "#/$defs/Selector" + }, + "description": "Variable name -> selector such as `.filter`.", + "propertyNames": { + "type": "string" + }, + "type": "object" + }, + "url": { + "$ref": "#/$defs/Pattern", + "description": "A regex matched against the request path, unanchored. Named capture\ngroups become template variables." + } + }, + "required": [ + "method", + "url" + ], + "type": "object" + }, + "MockResponse": { + "additionalProperties": false, + "properties": { + "body": { + "description": "A template rendered and sent as `text/plain`. Exclusive with `json` and\n`template`.", + "type": [ + "string", + "null" + ] + }, + "headers": { + "additionalProperties": { + "type": "string" + }, + "description": "Header name -> template producing the value.\n\nThe value is a template, not a header value: what it renders to is\nonly a header value once a request has been served, which is checked\nthere. The name is a name now.", + "propertyNames": { + "description": "An HTTP header name: a non-empty RFC 9110 token.", + "type": "string" + }, + "type": "object" + }, + "json": { + "description": "A template whose rendered output must be valid JSON, sent as\n`application/json`. Exclusive with `body` and `template`.", + "type": [ + "string", + "null" + ] + }, + "status": { + "$ref": "#/$defs/HttpStatus", + "description": "The status to answer with." + }, + "template": { + "description": "A template file under this proxy's template directory. Read per request,\nso it may be uploaded after the configuration was loaded. Exclusive with\n`body` and `json`.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/TemplateName" + } + ] + } + }, + "required": [ + "status" + ], + "type": "object" + }, + "Name": { + "description": "Letters, digits, `-` and `_`, between 2 and 64 characters.", + "maxLength": 64, + "minLength": 2, + "pattern": "^[A-Za-z0-9_-]{2,64}$", + "type": "string" + }, + "Pattern": { + "description": "A regular expression matched against the request path, unanchored. Named capture groups become template variables.", + "examples": [ + "/api/(?P\\d+)/" + ], + "type": "string" + }, + "Port": { + "description": "A TCP port, 1 to 65535.", + "maximum": 65535, + "minimum": 1, + "type": "integer" + }, + "ProxyAccessConfig": { + "additionalProperties": false, + "description": "Per-proxy override. Only these four actions may be overridden (rule V28 is\nexpressed in the type, so a config overriding `create` fails at parse time).", + "properties": { + "delete": { + "description": "Who may remove this proxy. Absent leaves the global rule.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/Subjects" + } + ] + }, + "read": { + "description": "Who may read this proxy's document. Absent leaves the global rule.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/Subjects" + } + ] + }, + "update": { + "description": "Who may replace this proxy. Absent leaves the global rule.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/Subjects" + } + ] + }, + "upload": { + "description": "Who may upload templates for this proxy. Absent leaves the global rule.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/Subjects" + } + ] + } + }, + "type": "object" + }, + "ProxyConfig": { + "additionalProperties": false, + "properties": { + "access": { + "description": "Overrides the admin `access` rules for this proxy alone.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/ProxyAccessConfig" + } + ] + }, + "body_limit": { + "$ref": "#/$defs/ByteSize", + "description": "Bounds the request body a matched mock is allowed to buffer in order\nto extract from it; phase 1 streams bodies deliberately, and reading\n`.content.items` needs the whole thing in hand. See rule V33." + }, + "headers": { + "additionalProperties": { + "$ref": "#/$defs/HeaderValue" + }, + "description": "Headers injected into every outbound request, overriding whatever the\nclient sent by the same name.", + "propertyNames": { + "description": "An HTTP header name: a non-empty RFC 9110 token.", + "type": "string" + }, + "type": "object" + }, + "latency": { + "description": "Makes a share of requests take a chosen time. Applies to mocked\nresponses too; a mock may override the figure.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/LatencyConfig" + } + ] + }, + "loss": { + "description": "Drops a share of requests rather than forwarding them. Not applied to\na request a mock answered.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/LossConfig" + } + ] + }, + "mocks": { + "description": "Mocks in the order they are tried. First match wins, and patterns are\nunanchored, so a general one placed first shadows the rest.", + "items": { + "$ref": "#/$defs/MockConfig" + }, + "type": "array" + }, + "name": { + "$ref": "#/$defs/ProxyName", + "description": "Names this proxy in `X-Proxy-Name`, in metrics labels, in log lines\nand as its template subdirectory. Unique within the document." + }, + "replace": { + "description": "What share of requests a matching mock actually answers; the rest go\nupstream. Defaults to 1.0, so a matching mock answers.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/Ratio" + } + ] + }, + "resolve": { + "$ref": "#/$defs/ResolveConfig", + "description": "How a request is matched to this proxy: by header, or as the default." + }, + "rewrite_redirects": { + "description": "Whether a redirect whose `Location` points back into the space this\nproxy forwards is rewritten to point at Doppel instead. Absent means\nenabled.\n\nOn by default because the alternative is a silent failure: `Host` is\nreplaced with the upstream's authority, so the upstream's `Location`\nnames the upstream, and a client following it leaves Doppel -- along with\nevery fault and every mock -- with nothing reported. Turn it off to have\nthe response relayed byte for byte, which is what a client being tested\n*against redirect handling itself* needs.", + "type": [ + "boolean", + "null" + ] + }, + "timeout": { + "description": "Bounds the whole upstream exchange, in seconds. Exceeding it is\n`504 UPSTREAM_TIMEOUT`. Defaults to 30.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/TimeoutSeconds" + } + ] + }, + "type": { + "$ref": "#/$defs/ProxyKind", + "description": "What it forwards. `http` is the only value." + }, + "url": { + "$ref": "#/$defs/UpstreamUrl", + "description": "The upstream base. A request path is grafted underneath it, and the\nresult can never escape it -- so a base with a path confines the proxy\nto that subtree." + } + }, + "required": [ + "name", + "type", + "url" + ], + "type": "object" + }, + "ProxyKind": { + "description": "What a proxy forwards. Only `http` exists.\n\nThere was a `Tcp` variant, admitted by the parser so that rule V7 could\nreject it afterwards with a message better than serde's. Both are gone: the\nvariant meant every layer downstream -- the runtime, the store, this schema --\nhad to carry a case that could never be reached, and the good message is\navailable without it. `Deserialize` below is written by hand for exactly that\nreason, so `type: tcp` still says *why* rather than only that the value is\nnot one of the accepted ones.", + "enum": [ + "http" + ], + "type": "string" + }, + "ProxyName": { + "description": "Letters, digits, `-` and `_`, between 2 and 32 characters.", + "maxLength": 32, + "minLength": 2, + "pattern": "^[A-Za-z0-9_-]{2,32}$", + "type": "string" + }, + "Ratio": { + "description": "A probability from 0.0 to 1.0. 50% is `0.5`.", + "maximum": 1, + "minimum": 0, + "type": "number" + }, + "ResolveConfig": { + "additionalProperties": false, + "properties": { + "header": { + "description": "The header carrying the proxy name. Required when `type: header`, and\nmeaningless otherwise.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/HeaderName" + } + ] + }, + "type": { + "$ref": "#/$defs/ResolveKind", + "description": "`default` takes anything unclaimed; `header` takes requests naming\nthis proxy in `header`." + } + }, + "type": "object" + }, + "ResolveKind": { + "enum": [ + "default", + "header" + ], + "type": "string" + }, + "Seconds": { + "description": "A latency in seconds, 0 to 300.", + "maximum": 300, + "minimum": 0, + "type": "number" + }, + "Selector": { + "description": "A selector addressing object keys: a leading dot, then dot-separated field names, as in `.content.items`.", + "examples": [ + ".content.items" + ], + "pattern": "^\\.[^.]+(\\.[^.]+)*$", + "type": "string" + }, + "SentryConfig": { + "additionalProperties": false, + "properties": { + "dsn": { + "description": "The Sentry DSN to report to. Empty disables reporting, so a deployment\ncan blank it without removing the section.", + "type": "string" + } + }, + "required": [ + "dsn" + ], + "type": "object" + }, + "ServerConfig": { + "additionalProperties": false, + "description": "No `workers` here. It sizes the tokio runtime, and a database-backed\nstore cannot be opened before that runtime exists -- so the value has to\nbe known before the configuration is read, which puts it on the same side\nof the boundary as the connection settings. It is `--workers` /\n`DOPPEL_WORKERS`.", + "properties": { + "host": { + "description": "An IP address, not a hostname: a name would have to be resolved,\nand which address it resolves to is not the configuration's to\ndecide. `utoipa` has no schema for `IpAddr`, so it is described\nhere as the string it is written as.", + "examples": [ + "127.0.0.1" + ], + "type": "string" + }, + "port": { + "$ref": "#/$defs/Port", + "description": "The TCP port proxied traffic arrives on. Must differ from\n`admin.port`." + } + }, + "required": [ + "host", + "port" + ], + "type": "object" + }, + "Subjects": { + "description": "Who may perform an action: `public`, one token or group name, or a list of them. An empty list means public.", + "oneOf": [ + { + "examples": [ + "public" + ], + "type": "string" + }, + { + "examples": [ + [ + "admin", + "user1" + ] + ], + "items": { + "type": "string" + }, + "type": "array" + } + ] + }, + "TemplateName": { + "description": "A template file name: one path component, no separators, no leading dot, no `..`.", + "examples": [ + "put.json.j2" + ], + "maxLength": 200, + "type": "string" + }, + "TemplatesConfig": { + "additionalProperties": false, + "properties": { + "dir": { + "description": "Directory holding mock templates, one subdirectory per proxy. Created\nat startup if absent.\nA filesystem path. `utoipa` has no schema for `PathBuf`, so it is\ndescribed as the string it is written as.", + "type": "string" + } + }, + "type": "object" + }, + "TimeoutSeconds": { + "description": "An upstream timeout in whole seconds, 1 to 3600.", + "maximum": 3600, + "minimum": 1, + "type": "integer" + }, + "Token": { + "description": "An admin token: printable ASCII with no spaces, 32 to 255 characters. A version 4 UUID is the recommended shape.", + "maxLength": 255, + "minLength": 32, + "type": "string" + }, + "TokenConfig": { + "additionalProperties": false, + "properties": { + "group": { + "$ref": "#/$defs/Name", + "description": "The group it belongs to. `admin` and `user` are predefined; any other\nname must be carried by at least one token." + }, + "name": { + "$ref": "#/$defs/Name", + "description": "What to call this token in `access` lists and in logs. Never the\nsecret itself." + }, + "token": { + "$ref": "#/$defs/Token", + "description": "The secret the caller sends. A version 4 UUID is the recommended\nshape." + } + }, + "required": [ + "name", + "group", + "token" + ], + "type": "object" + }, + "UploadConfig": { + "additionalProperties": false, + "properties": { + "limit": { + "$ref": "#/$defs/ByteSize", + "description": "Largest template file the admin API accepts. A larger upload is\nrefused with `413`." + } + }, + "required": [ + "limit" + ], + "type": "object" + }, + "UpstreamUrl": { + "description": "An absolute http or https base url, with no query string or fragment.", + "examples": [ + "https://example.com/api/" + ], + "type": "string" + } + }, + "$id": "https://raw.githubusercontent.com/lorem-dev/doppel/main/doppel-config.schema.json", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "additionalProperties": false, + "description": "The `main.yaml` Doppel reads. Generated from the Rust types by `doppel config schema`; do not edit by hand.", + "properties": { + "admin": { + "$ref": "#/$defs/AdminConfig", + "description": "The admin API's listener, its tokens, and who may do what." + }, + "control": { + "$ref": "#/$defs/ControlConfig", + "description": "The Unix socket `doppel config reload` talks to." + }, + "logging": { + "$ref": "#/$defs/LoggingConfig", + "description": "Log level and output format." + }, + "proxies": { + "description": "The proxies this instance serves, in the order they are tried.\n\nMay be empty or left out: Doppel then starts and serves the admin API,\nand a request is answered `503 NO_PROXIES_CONFIGURED` until a proxy is\nadded by reload or over that API.", + "items": { + "$ref": "#/$defs/ProxyConfig" + }, + "type": "array" + }, + "sentry": { + "description": "Optional error reporting. Absent, or an empty DSN, disables it.", + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/$defs/SentryConfig" + } + ] + }, + "server": { + "$ref": "#/$defs/ServerConfig", + "description": "Where the proxy listens for the traffic being forwarded or mocked." + }, + "templates": { + "$ref": "#/$defs/TemplatesConfig", + "description": "Where mock template files are read from and uploaded to." + } + }, + "required": [ + "server", + "admin" + ], + "title": "Doppel configuration", + "type": "object" +} diff --git a/main.example.yaml b/main.example.yaml index c63ecff..18da04d 100644 --- a/main.example.yaml +++ b/main.example.yaml @@ -1,5 +1,14 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/lorem-dev/doppel/main/doppel-config.schema.json +# # Doppel reference configuration. # Copy to main.yaml and adjust; main.yaml is git-ignored. +# +# The line above is what editors read: VS Code's YAML extension and any +# yaml-language-server client will complete field names, show each field's +# description and mark a bad value as you type. Keep it at the top of your own +# main.yaml. The schema is generated from the code -- see +# `doppel config schema` -- and every release also attaches it as an asset for +# anyone who would rather pin a version than follow `main`. server: host: "0.0.0.0" @@ -22,6 +31,12 @@ admin: enable: true host: "0.0.0.0" port: 8081 + # Which names `access` may reference, here and per proxy. "*" is any; + # ["admin", "ci"] allows exactly those. Never governs `public` itself. + # [] allows no name at all -- and since every action defaults to `admin`, + # that makes the defaults violations too, so all six would have to be + # written out as `public`. Rule V36. + groups: ["*"] auth: header: X-Proxy-Authorization # expects "Bearer {token}" tokens: @@ -66,6 +81,10 @@ proxies: min: 0.05 # seconds max: 0.2 replace: 1.0 # serve mocks for 100 percent of matching requests + # Rewrite a redirect whose target is under this proxy's base so it points + # back here instead of at the upstream; on by default. Set it to false to + # relay Location byte for byte. + rewrite_redirects: true body_limit: 1Mi # bounds the body buffered for mocks that extract from it (mock2, mock6 below); default 1Mi # Order matters. Patterns are matched as unanchored regexes, so a general # pattern placed first shadows every more specific one below it: with diff --git a/scripts/config_schema.py b/scripts/config_schema.py new file mode 100755 index 0000000..33ac1ab --- /dev/null +++ b/scripts/config_schema.py @@ -0,0 +1,86 @@ +#!/usr/bin/env -S uv run --script +# /// script +# requires-python = ">=3.11" +# dependencies = [] +# /// +"""Regenerate doppel-config.schema.json from the Rust configuration types. + + uv run scripts/config_schema.py # write the file + uv run scripts/config_schema.py --check # fail if it is out of date + +The schema is derived from the same `utoipa::ToSchema` implementations the admin +API's OpenAPI document uses, so there is no second description of the types to +keep in step. This script only moves the bytes: `doppel config schema` produces +them. + +`--check` exists because the file is what editors fetch and what a release +attaches, and a stale one is worse than none -- it reports mistakes that are not +mistakes and accepts fields that no longer exist. CI runs it. +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +OUTPUT = ROOT / "doppel-config.schema.json" + + +def fail(message: str) -> None: + print(f"config-schema: {message}", file=sys.stderr) + raise SystemExit(1) + + +def generate() -> str: + """The schema as `doppel config schema` prints it. + + Built in release-less debug mode on purpose: this runs in CI right after the + test job has already compiled the workspace, so the artifacts are warm, and + a `--release` build here would double the CI time to produce identical + bytes. + """ + build = subprocess.run( + ["cargo", "build", "--quiet", "-p", "doppel-cli"], + cwd=ROOT, + capture_output=True, + text=True, + ) + if build.returncode != 0: + fail(f"cannot build doppel-cli:\n{build.stderr}") + + result = subprocess.run( + ["cargo", "run", "--quiet", "-p", "doppel-cli", "--", "config", "schema"], + cwd=ROOT, + capture_output=True, + text=True, + ) + if result.returncode != 0: + fail(f"`doppel config schema` failed:\n{result.stderr}") + if not result.stdout.strip(): + fail("`doppel config schema` printed nothing") + return result.stdout + + +def main() -> None: + check = "--check" in sys.argv[1:] + generated = generate() + + if check: + if not OUTPUT.exists(): + fail(f"{OUTPUT.name} does not exist") + if OUTPUT.read_text(encoding="utf-8") != generated: + fail( + f"{OUTPUT.name} is out of date; " + "run `uv run scripts/config_schema.py`" + ) + print(f"config-schema: {OUTPUT.name} is up to date") + return + + OUTPUT.write_text(generated, encoding="utf-8") + print(f"config-schema: wrote {OUTPUT.relative_to(ROOT)}") + + +if __name__ == "__main__": + main() diff --git a/scripts/release_downloads.py b/scripts/release_downloads.py index 4cce14b..d37e6d4 100755 --- a/scripts/release_downloads.py +++ b/scripts/release_downloads.py @@ -70,6 +70,7 @@ def url(name: str) -> str: binaries: list[tuple[int, str, str]] = [] verification: list[tuple[int, str, str]] = [] + schema: list[tuple[int, str, str]] = [] other: list[tuple[int, str, str]] = [] for path in sorted(dist.iterdir()): @@ -78,6 +79,10 @@ def url(name: str) -> str: verification.append((0, name, name)) continue + if name == "doppel-config.schema.json": + schema.append((0, name, name)) + continue + stem = name.removesuffix(".tar.gz") if stem != name and stem.startswith("doppel-"): triple = stem.removeprefix("doppel-") @@ -87,7 +92,7 @@ def url(name: str) -> str: other.append((0, name, name)) - if not binaries and not other: + if not binaries and not schema and not other: fail(f"no assets found in {dist}") def bullets(entries: list[tuple[int, str, str]]) -> str: @@ -109,6 +114,22 @@ def bullets(entries: list[tuple[int, str, str]]) -> str: f"[Troubleshooting](https://lorem-dev.github.io/doppel/usage/troubleshooting/)." ) + if schema: + sections.append( + f"### Configuration schema\n\n{bullets(schema)}\n\n" + "The JSON Schema for `main.yaml` as of this release. Point an editor " + "at it to get completion, per-field descriptions and errors as you " + "type:\n\n" + "```yaml\n" + f"# yaml-language-server: $schema=https://github.com/{repo}/releases/download/" + f"{quote(raw_tag)}/doppel-config.schema.json\n" + "```\n\n" + "Or follow `main` instead of pinning a release:\n\n" + "```yaml\n" + f"# yaml-language-server: $schema=https://raw.githubusercontent.com/{repo}/main/doppel-config.schema.json\n" + "```" + ) + if other: sections.append(f"### Other\n\n{bullets(other)}")