mirror of
https://github.com/getnora-io/nora.git
synced 2026-04-12 11:30:32 +00:00
quality: MSRV, tarpaulin config, proptest for parsers (#84)
* fix: proxy dedup, multi-registry GC, TOCTOU and credential hygiene - Deduplicate proxy_fetch/proxy_fetch_text into generic proxy_fetch_core with response extractor closure (removes ~50 lines of copy-paste) - GC now scans all registry prefixes, not just docker/ - Add tracing::warn to fire-and-forget cache writes in docker proxy - Mark S3 credentials as skip_serializing to prevent accidental leaks - Remove TOCTOU race in LocalStorage get/delete (redundant exists check) * chore: clean up root directory structure - Move Dockerfile.astra and Dockerfile.redos to deploy/ (niche builds should not clutter the project root) - Harden .gitignore to exclude session files, working notes, and internal review scripts * refactor(metrics): replace 13 atomic fields with CounterMap Per-registry download/upload counters were 13 individual AtomicU64 fields, each duplicated across new(), with_persistence(), save(), record_download(), record_upload(), and get_registry_* (6 touch points per counter). Adding a new registry required changes in 6+ places. Now uses CounterMap (HashMap<String, AtomicU64>) for per-registry counters. Adding a new registry = one entry in REGISTRIES const. Added Go registry to REGISTRIES, gaining go metrics for free. * quality: add MSRV, tarpaulin config, proptest for parsers - Set rust-version = 1.75 in workspace Cargo.toml (MSRV policy) - Add tarpaulin.toml: llvm engine, fail-under=25, json+html output - Add coverage/ to .gitignore - Update CI to use tarpaulin.toml instead of inline flags - Add proptest dev-dependency and property tests: - validation.rs: 16 tests (never-panics + invariants for all 4 validators) - pypi.rs: 5 tests (extract_filename never-panics + format assertions) * test: add unit tests for 14 modules, coverage 21% → 30% Add 149 new tests across auth, backup, gc, metrics, mirror parsers, docker (manifest detection, session cleanup, metadata serde), docker_auth (token cache), maven, npm, pypi (normalize, rewrite, extract), raw (content-type guessing), request_id, and s3 (URI encoding). Update tarpaulin.toml: raise fail-under to 30, exclude UI/main from coverage reporting as they require integration tests. * bench: add criterion benchmarks for validation and manifest parsing Add parsing benchmark suite with 14 benchmarks covering: - Storage key, Docker name, digest, and reference validation - Docker manifest media type detection (v2, OCI index, minimal, invalid) Run with: cargo bench --package nora-registry --bench parsing * test: add 48 integration tests via tower oneshot Add integration tests for all HTTP handlers: - health (3), raw (7), cargo (4), maven (4), request_id (2) - pypi (5), npm (5), docker (12), auth (6) Create test_helpers.rs with TestContext pattern. Add tower and http-body-util dev-dependencies. Update tarpaulin fail-under 30 to 40. Coverage: 29.5% to 43.3% (2089/4825 lines) * fix: clean clippy warnings in tests, fix flaky audit test Add #[allow(clippy::unwrap_used)] to 18 test modules. Fix 3 additional clippy lints: writeln_empty_string, needless_update, unnecessary_get_then_check. Fix flaky audit test: replace single sleep(50ms) with retry loop (max 1s). Prefix unused token variable with underscore. cargo clippy --all-targets = 0 warnings (was 245 errors)
This commit is contained in:
@@ -2,52 +2,83 @@
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::collections::HashMap;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
use std::time::Instant;
|
||||
use tracing::{info, warn};
|
||||
|
||||
/// Serializable snapshot of metrics for persistence
|
||||
/// Known registry names for per-registry metrics
|
||||
const REGISTRIES: &[&str] = &["docker", "maven", "npm", "cargo", "pypi", "raw", "go"];
|
||||
|
||||
/// Serializable snapshot of metrics for persistence.
|
||||
/// Uses HashMap for per-registry counters — adding a new registry only
|
||||
/// requires adding its name to REGISTRIES (one line).
|
||||
#[derive(Serialize, Deserialize, Default)]
|
||||
struct MetricsSnapshot {
|
||||
downloads: u64,
|
||||
uploads: u64,
|
||||
cache_hits: u64,
|
||||
cache_misses: u64,
|
||||
docker_downloads: u64,
|
||||
docker_uploads: u64,
|
||||
npm_downloads: u64,
|
||||
maven_downloads: u64,
|
||||
maven_uploads: u64,
|
||||
cargo_downloads: u64,
|
||||
pypi_downloads: u64,
|
||||
raw_downloads: u64,
|
||||
raw_uploads: u64,
|
||||
#[serde(default)]
|
||||
registry_downloads: HashMap<String, u64>,
|
||||
#[serde(default)]
|
||||
registry_uploads: HashMap<String, u64>,
|
||||
}
|
||||
|
||||
/// Dashboard metrics for tracking registry activity
|
||||
/// Uses atomic counters for thread-safe access without locks
|
||||
/// Thread-safe atomic counter map for per-registry metrics.
|
||||
struct CounterMap(HashMap<String, AtomicU64>);
|
||||
|
||||
impl CounterMap {
|
||||
fn new(keys: &[&str]) -> Self {
|
||||
let mut map = HashMap::with_capacity(keys.len());
|
||||
for &k in keys {
|
||||
map.insert(k.to_string(), AtomicU64::new(0));
|
||||
}
|
||||
Self(map)
|
||||
}
|
||||
|
||||
fn inc(&self, key: &str) {
|
||||
if let Some(counter) = self.0.get(key) {
|
||||
counter.fetch_add(1, Ordering::Relaxed);
|
||||
}
|
||||
}
|
||||
|
||||
fn get(&self, key: &str) -> u64 {
|
||||
self.0
|
||||
.get(key)
|
||||
.map(|c| c.load(Ordering::Relaxed))
|
||||
.unwrap_or(0)
|
||||
}
|
||||
|
||||
fn snapshot(&self) -> HashMap<String, u64> {
|
||||
self.0
|
||||
.iter()
|
||||
.map(|(k, v)| (k.clone(), v.load(Ordering::Relaxed)))
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn load_from(&self, data: &HashMap<String, u64>) {
|
||||
for (k, v) in data {
|
||||
if let Some(counter) = self.0.get(k.as_str()) {
|
||||
counter.store(*v, Ordering::Relaxed);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Dashboard metrics for tracking registry activity.
|
||||
/// Global counters are separate fields; per-registry counters use CounterMap.
|
||||
pub struct DashboardMetrics {
|
||||
// Global counters
|
||||
pub downloads: AtomicU64,
|
||||
pub uploads: AtomicU64,
|
||||
pub cache_hits: AtomicU64,
|
||||
pub cache_misses: AtomicU64,
|
||||
|
||||
// Per-registry download counters
|
||||
pub docker_downloads: AtomicU64,
|
||||
pub docker_uploads: AtomicU64,
|
||||
pub npm_downloads: AtomicU64,
|
||||
pub maven_downloads: AtomicU64,
|
||||
pub maven_uploads: AtomicU64,
|
||||
pub cargo_downloads: AtomicU64,
|
||||
pub pypi_downloads: AtomicU64,
|
||||
pub raw_downloads: AtomicU64,
|
||||
pub raw_uploads: AtomicU64,
|
||||
registry_downloads: CounterMap,
|
||||
registry_uploads: CounterMap,
|
||||
|
||||
pub start_time: Instant,
|
||||
|
||||
/// Path to metrics.json for persistence
|
||||
persist_path: Option<PathBuf>,
|
||||
}
|
||||
|
||||
@@ -58,15 +89,8 @@ impl DashboardMetrics {
|
||||
uploads: AtomicU64::new(0),
|
||||
cache_hits: AtomicU64::new(0),
|
||||
cache_misses: AtomicU64::new(0),
|
||||
docker_downloads: AtomicU64::new(0),
|
||||
docker_uploads: AtomicU64::new(0),
|
||||
npm_downloads: AtomicU64::new(0),
|
||||
maven_downloads: AtomicU64::new(0),
|
||||
maven_uploads: AtomicU64::new(0),
|
||||
cargo_downloads: AtomicU64::new(0),
|
||||
pypi_downloads: AtomicU64::new(0),
|
||||
raw_downloads: AtomicU64::new(0),
|
||||
raw_uploads: AtomicU64::new(0),
|
||||
registry_downloads: CounterMap::new(REGISTRIES),
|
||||
registry_uploads: CounterMap::new(REGISTRIES),
|
||||
start_time: Instant::now(),
|
||||
persist_path: None,
|
||||
}
|
||||
@@ -77,7 +101,6 @@ impl DashboardMetrics {
|
||||
let path = Path::new(storage_path).join("metrics.json");
|
||||
let mut metrics = Self::new();
|
||||
|
||||
// Load existing metrics if file exists
|
||||
if path.exists() {
|
||||
match std::fs::read_to_string(&path) {
|
||||
Ok(data) => match serde_json::from_str::<MetricsSnapshot>(&data) {
|
||||
@@ -86,15 +109,10 @@ impl DashboardMetrics {
|
||||
metrics.uploads = AtomicU64::new(snap.uploads);
|
||||
metrics.cache_hits = AtomicU64::new(snap.cache_hits);
|
||||
metrics.cache_misses = AtomicU64::new(snap.cache_misses);
|
||||
metrics.docker_downloads = AtomicU64::new(snap.docker_downloads);
|
||||
metrics.docker_uploads = AtomicU64::new(snap.docker_uploads);
|
||||
metrics.npm_downloads = AtomicU64::new(snap.npm_downloads);
|
||||
metrics.maven_downloads = AtomicU64::new(snap.maven_downloads);
|
||||
metrics.maven_uploads = AtomicU64::new(snap.maven_uploads);
|
||||
metrics.cargo_downloads = AtomicU64::new(snap.cargo_downloads);
|
||||
metrics.pypi_downloads = AtomicU64::new(snap.pypi_downloads);
|
||||
metrics.raw_downloads = AtomicU64::new(snap.raw_downloads);
|
||||
metrics.raw_uploads = AtomicU64::new(snap.raw_uploads);
|
||||
metrics
|
||||
.registry_downloads
|
||||
.load_from(&snap.registry_downloads);
|
||||
metrics.registry_uploads.load_from(&snap.registry_uploads);
|
||||
info!(
|
||||
downloads = snap.downloads,
|
||||
uploads = snap.uploads,
|
||||
@@ -121,17 +139,9 @@ impl DashboardMetrics {
|
||||
uploads: self.uploads.load(Ordering::Relaxed),
|
||||
cache_hits: self.cache_hits.load(Ordering::Relaxed),
|
||||
cache_misses: self.cache_misses.load(Ordering::Relaxed),
|
||||
docker_downloads: self.docker_downloads.load(Ordering::Relaxed),
|
||||
docker_uploads: self.docker_uploads.load(Ordering::Relaxed),
|
||||
npm_downloads: self.npm_downloads.load(Ordering::Relaxed),
|
||||
maven_downloads: self.maven_downloads.load(Ordering::Relaxed),
|
||||
maven_uploads: self.maven_uploads.load(Ordering::Relaxed),
|
||||
cargo_downloads: self.cargo_downloads.load(Ordering::Relaxed),
|
||||
pypi_downloads: self.pypi_downloads.load(Ordering::Relaxed),
|
||||
raw_downloads: self.raw_downloads.load(Ordering::Relaxed),
|
||||
raw_uploads: self.raw_uploads.load(Ordering::Relaxed),
|
||||
registry_downloads: self.registry_downloads.snapshot(),
|
||||
registry_uploads: self.registry_uploads.snapshot(),
|
||||
};
|
||||
// Atomic write: write to tmp then rename
|
||||
let tmp = path.with_extension("json.tmp");
|
||||
if let Ok(data) = serde_json::to_string_pretty(&snap) {
|
||||
if tokio::fs::write(&tmp, &data).await.is_ok() {
|
||||
@@ -140,42 +150,24 @@ impl DashboardMetrics {
|
||||
}
|
||||
}
|
||||
|
||||
/// Record a download event for the specified registry
|
||||
pub fn record_download(&self, registry: &str) {
|
||||
self.downloads.fetch_add(1, Ordering::Relaxed);
|
||||
match registry {
|
||||
"docker" => self.docker_downloads.fetch_add(1, Ordering::Relaxed),
|
||||
"npm" => self.npm_downloads.fetch_add(1, Ordering::Relaxed),
|
||||
"maven" => self.maven_downloads.fetch_add(1, Ordering::Relaxed),
|
||||
"cargo" => self.cargo_downloads.fetch_add(1, Ordering::Relaxed),
|
||||
"pypi" => self.pypi_downloads.fetch_add(1, Ordering::Relaxed),
|
||||
"raw" => self.raw_downloads.fetch_add(1, Ordering::Relaxed),
|
||||
_ => 0,
|
||||
};
|
||||
self.registry_downloads.inc(registry);
|
||||
}
|
||||
|
||||
/// Record an upload event for the specified registry
|
||||
pub fn record_upload(&self, registry: &str) {
|
||||
self.uploads.fetch_add(1, Ordering::Relaxed);
|
||||
match registry {
|
||||
"docker" => self.docker_uploads.fetch_add(1, Ordering::Relaxed),
|
||||
"maven" => self.maven_uploads.fetch_add(1, Ordering::Relaxed),
|
||||
"raw" => self.raw_uploads.fetch_add(1, Ordering::Relaxed),
|
||||
_ => 0,
|
||||
};
|
||||
self.registry_uploads.inc(registry);
|
||||
}
|
||||
|
||||
/// Record a cache hit
|
||||
pub fn record_cache_hit(&self) {
|
||||
self.cache_hits.fetch_add(1, Ordering::Relaxed);
|
||||
}
|
||||
|
||||
/// Record a cache miss
|
||||
pub fn record_cache_miss(&self) {
|
||||
self.cache_misses.fetch_add(1, Ordering::Relaxed);
|
||||
}
|
||||
|
||||
/// Calculate the cache hit rate as a percentage
|
||||
pub fn cache_hit_rate(&self) -> f64 {
|
||||
let hits = self.cache_hits.load(Ordering::Relaxed);
|
||||
let misses = self.cache_misses.load(Ordering::Relaxed);
|
||||
@@ -187,27 +179,12 @@ impl DashboardMetrics {
|
||||
}
|
||||
}
|
||||
|
||||
/// Get download count for a specific registry
|
||||
pub fn get_registry_downloads(&self, registry: &str) -> u64 {
|
||||
match registry {
|
||||
"docker" => self.docker_downloads.load(Ordering::Relaxed),
|
||||
"npm" => self.npm_downloads.load(Ordering::Relaxed),
|
||||
"maven" => self.maven_downloads.load(Ordering::Relaxed),
|
||||
"cargo" => self.cargo_downloads.load(Ordering::Relaxed),
|
||||
"pypi" => self.pypi_downloads.load(Ordering::Relaxed),
|
||||
"raw" => self.raw_downloads.load(Ordering::Relaxed),
|
||||
_ => 0,
|
||||
}
|
||||
self.registry_downloads.get(registry)
|
||||
}
|
||||
|
||||
/// Get upload count for a specific registry
|
||||
pub fn get_registry_uploads(&self, registry: &str) -> u64 {
|
||||
match registry {
|
||||
"docker" => self.docker_uploads.load(Ordering::Relaxed),
|
||||
"maven" => self.maven_uploads.load(Ordering::Relaxed),
|
||||
"raw" => self.raw_uploads.load(Ordering::Relaxed),
|
||||
_ => 0,
|
||||
}
|
||||
self.registry_uploads.get(registry)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -218,6 +195,7 @@ impl Default for DashboardMetrics {
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[allow(clippy::unwrap_used)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use tempfile::TempDir;
|
||||
@@ -238,12 +216,12 @@ mod tests {
|
||||
m.record_download(reg);
|
||||
}
|
||||
assert_eq!(m.downloads.load(Ordering::Relaxed), 6);
|
||||
assert_eq!(m.docker_downloads.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(m.npm_downloads.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(m.maven_downloads.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(m.cargo_downloads.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(m.pypi_downloads.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(m.raw_downloads.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(m.get_registry_downloads("docker"), 1);
|
||||
assert_eq!(m.get_registry_downloads("npm"), 1);
|
||||
assert_eq!(m.get_registry_downloads("maven"), 1);
|
||||
assert_eq!(m.get_registry_downloads("cargo"), 1);
|
||||
assert_eq!(m.get_registry_downloads("pypi"), 1);
|
||||
assert_eq!(m.get_registry_downloads("raw"), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -251,8 +229,7 @@ mod tests {
|
||||
let m = DashboardMetrics::new();
|
||||
m.record_download("unknown");
|
||||
assert_eq!(m.downloads.load(Ordering::Relaxed), 1);
|
||||
// no per-registry counter should increment
|
||||
assert_eq!(m.docker_downloads.load(Ordering::Relaxed), 0);
|
||||
assert_eq!(m.get_registry_downloads("docker"), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -262,15 +239,15 @@ mod tests {
|
||||
m.record_upload("maven");
|
||||
m.record_upload("raw");
|
||||
assert_eq!(m.uploads.load(Ordering::Relaxed), 3);
|
||||
assert_eq!(m.docker_uploads.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(m.maven_uploads.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(m.raw_uploads.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(m.get_registry_uploads("docker"), 1);
|
||||
assert_eq!(m.get_registry_uploads("maven"), 1);
|
||||
assert_eq!(m.get_registry_uploads("raw"), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_record_upload_unknown_registry() {
|
||||
let m = DashboardMetrics::new();
|
||||
m.record_upload("npm"); // npm has no upload counter
|
||||
m.record_upload("npm");
|
||||
assert_eq!(m.uploads.load(Ordering::Relaxed), 1);
|
||||
}
|
||||
|
||||
@@ -322,7 +299,6 @@ mod tests {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let path = tmp.path().to_str().unwrap();
|
||||
|
||||
// Create metrics, record some data, save
|
||||
{
|
||||
let m = DashboardMetrics::with_persistence(path);
|
||||
m.record_download("docker");
|
||||
@@ -332,13 +308,12 @@ mod tests {
|
||||
m.save().await;
|
||||
}
|
||||
|
||||
// Load in new instance
|
||||
{
|
||||
let m = DashboardMetrics::with_persistence(path);
|
||||
assert_eq!(m.downloads.load(Ordering::Relaxed), 2);
|
||||
assert_eq!(m.uploads.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(m.docker_downloads.load(Ordering::Relaxed), 2);
|
||||
assert_eq!(m.maven_uploads.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(m.get_registry_downloads("docker"), 2);
|
||||
assert_eq!(m.get_registry_uploads("maven"), 1);
|
||||
assert_eq!(m.cache_hits.load(Ordering::Relaxed), 1);
|
||||
}
|
||||
}
|
||||
@@ -347,8 +322,6 @@ mod tests {
|
||||
fn test_persistence_missing_file() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let path = tmp.path().to_str().unwrap();
|
||||
|
||||
// Should work even without existing metrics.json
|
||||
let m = DashboardMetrics::with_persistence(path);
|
||||
assert_eq!(m.downloads.load(Ordering::Relaxed), 0);
|
||||
}
|
||||
@@ -358,4 +331,11 @@ mod tests {
|
||||
let m = DashboardMetrics::default();
|
||||
assert_eq!(m.downloads.load(Ordering::Relaxed), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_go_registry_supported() {
|
||||
let m = DashboardMetrics::new();
|
||||
m.record_download("go");
|
||||
assert_eq!(m.get_registry_downloads("go"), 1);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user