Skip to content

Support fetching intermediates #103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ctz wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Support fetching intermediates #103

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2542b09
Extract parameters to `fetch()` for reuse
ctz Aug 5, 2026
40c9391
Customise revocation-specific behaviour in `fetch()`
ctz Aug 5, 2026
6d08a64
Generalise and reuse `Manifest` and associated
ctz Aug 5, 2026
2cb4e6c
Support configuration of intermediate fetching
ctz Mar 26, 2026
83bd503
Hook up intermediates fetching
ctz Aug 5, 2026
baf1994
Align integration tests with how mirrors now work
ctz Aug 5, 2026
b7e9619
Allow test configs to be customised per-test
ctz Aug 5, 2026
620f190
Test intermediate fetching
ctz Aug 5, 2026
816722c
Avoid using `ExitCode::SUCCESS` as unit type
ctz Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 109 additions & 87 deletions upki/src/revocation/fetch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,84 +36,112 @@ pub async fn fetch(dry_run: bool, config: &Config) -> Result<ExitCode, Error> {
"fetching {} into {:?}...",
&config.revocation.fetch_url, &cache_dir,
);
let old_manifest = Manifest::from_config(config).ok();

let manifest_url = format!("{}{MANIFEST_JSON}", config.revocation.fetch_url);
#[cfg(feature = "fetch")]
let builder = reqwest::Client::builder().use_rustls_tls();
#[cfg(all(feature = "fetch-native-tls", not(feature = "fetch")))]
let builder = reqwest::Client::builder().use_native_tls();

let client = builder
.timeout(Duration::from_secs(REQUEST_TIMEOUT))
.user_agent(format!(
"{}/{} ({})",
env!("CARGO_PKG_NAME"),
env!("CARGO_PKG_VERSION"),
env!("CARGO_PKG_REPOSITORY")
))
.build()
.map_err(|error| Error::HttpFetch {
error: Box::new(error),
url: manifest_url.clone(),
})?;

let response = client
.get(&manifest_url)
.send()
.await
.map_err(|error| Error::HttpFetch {
error: Box::new(error),
url: manifest_url.clone(),
})?
.error_for_status()
.map_err(|error| Error::HttpFetch {
error: Box::new(error),
url: manifest_url.clone(),
})?;

let manifest = response
.json::<Manifest>()
.await
.map_err(|error| Error::FileDecode {
error: Box::new(error),
path: None,
})?;

manifest.introduce()?;
FetchContext {
cache_dir,
fetch_url: &config.revocation.fetch_url,
old_manifest,
typ: FetchType::Revocation,
}
.fetch(dry_run)
.await
}

let old_manifest = Manifest::from_config(config).ok();
pub(crate) struct FetchContext<'a> {
pub(crate) cache_dir: PathBuf,
pub(crate) fetch_url: &'a str,
pub(crate) old_manifest: Option<Manifest>,
pub(crate) typ: FetchType,
}

let plan = Plan::construct(
&manifest,
&old_manifest,
&config.revocation.fetch_url,
&cache_dir,
)?;
impl FetchContext<'_> {
pub(crate) async fn fetch(&self, dry_run: bool) -> Result<ExitCode, Error> {
let manifest_url = format!("{}{MANIFEST_JSON}", self.fetch_url);
#[cfg(feature = "fetch")]
let builder = reqwest::Client::builder().use_rustls_tls();
#[cfg(all(feature = "fetch-native-tls", not(feature = "fetch")))]
let builder = reqwest::Client::builder().use_native_tls();

let client = builder
.timeout(Duration::from_secs(REQUEST_TIMEOUT))
.user_agent(format!(
"{}/{} ({})",
env!("CARGO_PKG_NAME"),
env!("CARGO_PKG_VERSION"),
env!("CARGO_PKG_REPOSITORY")
))
.build()
.map_err(|error| Error::HttpFetch {
error: Box::new(error),
url: manifest_url.clone(),
})?;

if dry_run {
println!(
"{} steps required ({} bytes to download)",
let response = client
.get(&manifest_url)
.send()
.await
.map_err(|error| Error::HttpFetch {
error: Box::new(error),
url: manifest_url.clone(),
})?
.error_for_status()
.map_err(|error| Error::HttpFetch {
error: Box::new(error),
url: manifest_url.clone(),
})?;

let manifest = response
.json::<Manifest>()
.await
.map_err(|error| Error::FileDecode {
error: Box::new(error),
path: None,
})?;

manifest.introduce()?;

let plan = Plan::construct(&manifest, self)?;

if dry_run {
println!(
"{} steps required ({} bytes to download)",
plan.steps.len(),
plan.download_bytes()
);
for step in plan.steps {
println!("- {step}");
}
return Ok(ExitCode::SUCCESS);
}

info!(
"{} steps required ({} bytes to download).",
plan.steps.len(),
plan.download_bytes()
);

for step in plan.steps {
println!("- {step}");
step.execute(&client).await?;
}
return Ok(ExitCode::SUCCESS);

info!("success");
Ok(ExitCode::SUCCESS)
}

info!(
"{} steps required ({} bytes to download).",
plan.steps.len(),
plan.download_bytes()
);
fn should_clean_up_file_name(&self, name: &str) -> bool {
match self.typ {
FetchType::Revocation => name.ends_with(".filter") || name.ends_with(".delta"),
}
}

for step in plan.steps {
step.execute(&client).await?;
fn requires_revocation_index(&self) -> bool {
Comment on lines +132 to +138

@djc djc Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like these could/should be methods on FetchType?

Also I'm inclined to say that this could/should be a trait instead, so that we can decentralize this setup (that is, collect revocation parameters for fetching in revocation and other stuff elsewhere), rather than centralizing it in fetch.

matches!(self.typ, FetchType::Revocation)
}
}

info!("success");
Ok(ExitCode::SUCCESS)
pub(crate) enum FetchType {
Revocation,
}

pub(crate) struct Plan {
Expand All @@ -124,24 +152,16 @@ impl Plan {
/// Form a plan of how to synchronize with the remote server.
///
/// - `manifest` describes the contents of the remote server.
/// - `old_manifest` is an alleged current manifest, whose files are left alone.
/// - `remote_url` is the base URL.
/// - `local` is the path into which files are downloaded. The caller ensures this exists.
pub(crate) fn construct(
manifest: &Manifest,
old_manifest: &Option<Manifest>,
remote_url: &str,
local: &Path,
) -> Result<Self, Error> {
pub(crate) fn construct(manifest: &Manifest, ctx: &FetchContext<'_>) -> Result<Self, Error> {

@djc djc Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think the idiomatic abbrevation for "context" in Rust is cx.

let mut steps = Vec::new();

// Collect unwanted files for deletion
let mut unwanted_files = HashSet::new();

if local.exists() {
let iter = fs::read_dir(local).map_err(|error| Error::CreateDirectory {
if ctx.cache_dir.exists() {
let iter = fs::read_dir(&ctx.cache_dir).map_err(|error| Error::CreateDirectory {
error,
path: local.to_owned(),
path: ctx.cache_dir.to_owned(),
})?;

for entry in iter {
Expand All @@ -152,44 +172,46 @@ impl Plan {

let path = Path::new(&entry.file_name()).to_owned();
let name = path.to_string_lossy();
if name.ends_with(".filter") || name.ends_with(".delta") {
if ctx.should_clean_up_file_name(&name) {
unwanted_files.insert(path);
}
}
} else {
steps.push(PlanStep::CreateDir(local.to_owned()));
steps.push(PlanStep::CreateDir(ctx.cache_dir.to_owned()));
}

for file in &manifest.files {
unwanted_files.remove(Path::new(&file.filename));

let path = local.join(&file.filename);
let path = ctx.cache_dir.join(&file.filename);
match hash_file(&path) {
Ok(digest) if digest.as_ref() == file.hash => continue,
_ => {}
}

steps.push(PlanStep::download(file, remote_url, local));
steps.push(PlanStep::download(file, ctx.fetch_url, &ctx.cache_dir));
}

if let Some(old_manifest) = &old_manifest {
if let Some(old_manifest) = &ctx.old_manifest {
for file in &old_manifest.files {
unwanted_files.remove(Path::new(&file.filename));
}
}

steps.push(PlanStep::SaveIndex {
manifest: manifest.clone(),
local_dir: local.to_owned(),
});
if ctx.requires_revocation_index() {
steps.push(PlanStep::SaveIndex {
manifest: manifest.clone(),
local_dir: ctx.cache_dir.to_owned(),
});
}

steps.push(PlanStep::SaveManifest {
manifest: manifest.clone(),
local_dir: local.to_owned(),
local_dir: ctx.cache_dir.to_owned(),
});

for filename in unwanted_files {
steps.push(PlanStep::Delete(local.join(filename)));
steps.push(PlanStep::Delete(ctx.cache_dir.join(filename)));
}

Ok(Self { steps })
Expand Down
14 changes: 11 additions & 3 deletions upki/src/revocation/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ use crate::sha256;
#[cfg(feature = "__fetch")]
mod fetch;
#[cfg(feature = "__fetch")]
use fetch::Plan;
#[cfg(feature = "__fetch")]
pub use fetch::fetch;
#[cfg(feature = "__fetch")]
use fetch::{FetchContext, FetchType, Plan};

mod index;
pub use index::Index;
Expand Down Expand Up @@ -77,7 +77,15 @@ impl Manifest {
#[cfg(feature = "__fetch")]
pub fn verify(&self, config: &Config) -> Result<ExitCode, Error> {
self.introduce()?;
let plan = Plan::construct(self, &None, "https://.../", &config.revocation_cache_dir())?;
let plan = Plan::construct(
self,
&FetchContext {
cache_dir: config.revocation_cache_dir(),
fetch_url: "https://.../",
old_manifest: None,
typ: FetchType::Revocation,
},
)?;
match plan.download_bytes() {
0 => Ok(ExitCode::SUCCESS),
bytes => Err(Error::Outdated(bytes)),
Expand Down