From cc67edab12e4ba0780185cf6c4d599bafb8fae76 Mon Sep 17 00:00:00 2001 From: git-echo Date: Wed, 27 May 2026 15:04:25 -0500 Subject: [PATCH] fix: resolve CI failures (fmt, clippy, tests) - Fix rustfmt formatting in cache.rs, patches.rs, system.rs, routes.rs, main.rs - Add Default impl for PackageCacheState (clippy new_without_default) - Change apply_with_cache_retry generic bound from Fn to FnMut - Add mut to refresh_fn parameter for FnMut compatibility - Replace bool comparison with ! operator (clippy bool_comparison) - Update todo.md with completed status --- src/api/handlers/patches.rs | 54 ++++++++++++++++++++++++++------- src/api/handlers/system.rs | 24 +++++++++++---- src/api/routes.rs | 33 ++++++++++---------- src/main.rs | 7 ++++- src/packages/cache.rs | 31 +++++++++---------- tasks/todo.md | 60 +++++++++++++++++++++++-------------- 6 files changed, 137 insertions(+), 72 deletions(-) diff --git a/src/api/handlers/patches.rs b/src/api/handlers/patches.rs index 85589d6..41b2c75 100644 --- a/src/api/handlers/patches.rs +++ b/src/api/handlers/patches.rs @@ -126,7 +126,12 @@ pub async fn apply_patches( // MANDATORY: Refresh package cache before applying patches let _ = job_manager_clone - .update_job(&job_id_clone, JobStatus::Running, Some(0), Some("Refreshing package index...".to_string())) + .update_job( + &job_id_clone, + JobStatus::Running, + Some(0), + Some("Refreshing package index...".to_string()), + ) .await; let _ = job_manager_clone .add_job_log(&job_id_clone, "Refreshing package cache...".to_string()) @@ -135,10 +140,18 @@ pub async fn apply_patches( match backend_clone.refresh_package_cache(&cache_state_clone) { Ok(_) => { let _ = job_manager_clone - .add_job_log(&job_id_clone, "Package cache refreshed successfully".to_string()) + .add_job_log( + &job_id_clone, + "Package cache refreshed successfully".to_string(), + ) .await; let _ = job_manager_clone - .update_job(&job_id_clone, JobStatus::Running, Some(10), Some("Cache refreshed, applying patches...".to_string())) + .update_job( + &job_id_clone, + JobStatus::Running, + Some(10), + Some("Cache refreshed, applying patches...".to_string()), + ) .await; } Err(e) => { @@ -194,17 +207,25 @@ pub async fn apply_patches( // 404/fetch error: refresh cache and retry once info!(job_id = %job_id_clone, "Patch apply failed with fetch error, refreshing cache and retrying"); let _ = job_manager_clone - .add_job_log(&job_id_clone, "Fetch error detected, refreshing cache and retrying...".to_string()) + .add_job_log( + &job_id_clone, + "Fetch error detected, refreshing cache and retrying..." + .to_string(), + ) .await; match backend_clone.refresh_package_cache(&cache_state_clone) { Ok(_) => { let _ = job_manager_clone - .add_job_log(&job_id_clone, "Cache refreshed, retrying patch apply...".to_string()) + .add_job_log( + &job_id_clone, + "Cache refreshed, retrying patch apply...".to_string(), + ) .await; } Err(refresh_err) => { - let err_msg = format!("Cache refresh on retry failed: {}", refresh_err); + let err_msg = + format!("Cache refresh on retry failed: {}", refresh_err); let _ = job_manager_clone.fail_job(&job_id_clone, err_msg).await; error!(job_id = %job_id_clone, error = %refresh_err, "Cache refresh on retry failed"); return; @@ -228,29 +249,40 @@ pub async fn apply_patches( ), ) .await; - match backend_clone.reboot_system(request.reboot_delay_seconds) { + match backend_clone.reboot_system(request.reboot_delay_seconds) + { Ok(_) => { let _ = job_manager_clone - .add_job_log(&job_id_clone, "Reboot command executed".to_string()) + .add_job_log( + &job_id_clone, + "Reboot command executed".to_string(), + ) .await; } Err(e) => { let _ = job_manager_clone - .add_job_log(&job_id_clone, format!("Reboot failed: {}", e)) + .add_job_log( + &job_id_clone, + format!("Reboot failed: {}", e), + ) .await; } } } } Err(retry_err) => { - let _ = job_manager_clone.fail_job(&job_id_clone, retry_err.to_string()).await; + let _ = job_manager_clone + .fail_job(&job_id_clone, retry_err.to_string()) + .await; error!(job_id = %job_id_clone, error = %retry_err, "Patch application failed after retry"); } } } Err(e) => { // Non-fetch error: fail immediately - let _ = job_manager_clone.fail_job(&job_id_clone, e.to_string()).await; + let _ = job_manager_clone + .fail_job(&job_id_clone, e.to_string()) + .await; error!(job_id = %job_id_clone, error = %e, "Patch application failed"); } } diff --git a/src/api/handlers/system.rs b/src/api/handlers/system.rs index 0cb15fe..fd6dd81 100644 --- a/src/api/handlers/system.rs +++ b/src/api/handlers/system.rs @@ -42,11 +42,11 @@ pub struct SystemInfoData { /// Health check response data #[derive(Debug, Serialize)] pub struct HealthData { - pub status: String, // "healthy" or "degraded" + pub status: String, // "healthy" or "degraded" pub uptime_seconds: u64, pub version: String, - pub last_cache_update: Option, // RFC3339 timestamp - pub cache_status: String, // "fresh", "stale", "unknown", "failed" + pub last_cache_update: Option, // RFC3339 timestamp + pub cache_status: String, // "fresh", "stale", "unknown", "failed" } /// Service status response data @@ -138,15 +138,27 @@ pub async fn health_check( match backend.refresh_package_cache(&cache_state) { Ok(_) => { let updated = cache_state.status(); - ("healthy".to_string(), "fresh".to_string(), updated.last_update.map(|dt| dt.to_rfc3339())) + ( + "healthy".to_string(), + "fresh".to_string(), + updated.last_update.map(|dt| dt.to_rfc3339()), + ) } Err(e) => { error!("Health check cache refresh failed: {}", e); - ("degraded".to_string(), "failed".to_string(), cache_status_val.last_update.map(|dt| dt.to_rfc3339())) + ( + "degraded".to_string(), + "failed".to_string(), + cache_status_val.last_update.map(|dt| dt.to_rfc3339()), + ) } } } else { - ("healthy".to_string(), "fresh".to_string(), cache_status_val.last_update.map(|dt| dt.to_rfc3339())) + ( + "healthy".to_string(), + "fresh".to_string(), + cache_status_val.last_update.map(|dt| dt.to_rfc3339()), + ) }; let response = ApiResponse::success(HealthData { diff --git a/src/api/routes.rs b/src/api/routes.rs index 6076027..c17f6af 100644 --- a/src/api/routes.rs +++ b/src/api/routes.rs @@ -26,21 +26,24 @@ pub fn configure_api_routes( ) { info!("Configuring API v1 routes"); - cfg.app_data(job_manager).app_data(backend).app_data(cache_state).service( - web::scope("/api/v1") - // VULN-005: Default handler for unsupported methods returns 405 instead of 404 - .default_service(web::route().to(method_not_allowed)) - // Package Management Endpoints - .configure(packages::configure_routes) - // Patch Management Endpoints - .configure(patches::configure_routes) - // System Management Endpoints - .configure(system::configure_routes) - // Job Management Endpoints - .configure(jobs::configure_routes) - // WebSocket Endpoint - .configure(websocket::configure_routes), - ); + cfg.app_data(job_manager) + .app_data(backend) + .app_data(cache_state) + .service( + web::scope("/api/v1") + // VULN-005: Default handler for unsupported methods returns 405 instead of 404 + .default_service(web::route().to(method_not_allowed)) + // Package Management Endpoints + .configure(packages::configure_routes) + // Patch Management Endpoints + .configure(patches::configure_routes) + // System Management Endpoints + .configure(system::configure_routes) + // Job Management Endpoints + .configure(jobs::configure_routes) + // WebSocket Endpoint + .configure(websocket::configure_routes), + ); } /// Health check route (outside API scope for load balancer checks) diff --git a/src/main.rs b/src/main.rs index 52e1896..6904d88 100644 --- a/src/main.rs +++ b/src/main.rs @@ -166,7 +166,12 @@ async fn main() -> Result<()> { // Configure API routes app = app.configure(|cfg| { - configure_api_routes(cfg, job_manager_data.clone(), backend_data.clone(), cache_state.clone()); + configure_api_routes( + cfg, + job_manager_data.clone(), + backend_data.clone(), + cache_state.clone(), + ); }); // Configure health route (outside API scope) diff --git a/src/packages/cache.rs b/src/packages/cache.rs index 352b078..7a83c22 100644 --- a/src/packages/cache.rs +++ b/src/packages/cache.rs @@ -21,7 +21,7 @@ const CACHE_REFRESH_TIMEOUT_SECS: u64 = 120; /// Persistent cache state (written to cache.json) #[derive(Debug, Clone, Serialize, Deserialize)] pub struct CacheStateFile { - pub last_cache_update: Option, // RFC3339 + pub last_cache_update: Option, // RFC3339 pub last_update_success: bool, } @@ -44,6 +44,12 @@ struct CacheStateInner { last_update_error: Option, } +impl Default for PackageCacheState { + fn default() -> Self { + Self::new() + } +} + impl PackageCacheState { pub fn new() -> Self { // Try to load from state file on startup @@ -83,8 +89,7 @@ impl PackageCacheState { Some(t) => { let threshold = Duration::from_secs(STALE_THRESHOLD_SECS); Utc::now() - t - > chrono::Duration::from_std(threshold) - .unwrap_or(chrono::TimeDelta::MAX) + > chrono::Duration::from_std(threshold).unwrap_or(chrono::TimeDelta::MAX) } } } @@ -151,12 +156,9 @@ pub fn is_fetch_error(error: &anyhow::Error) -> bool { /// Execute a patch apply with automatic cache refresh retry on 404/fetch errors. /// Hardcoded 1 retry after cache refresh. -pub fn apply_with_cache_retry( - refresh_fn: F, - apply_fn: impl Fn() -> Result<()>, -) -> Result<()> +pub fn apply_with_cache_retry(mut refresh_fn: F, apply_fn: impl Fn() -> Result<()>) -> Result<()> where - F: Fn() -> Result<()>, + F: FnMut() -> Result<()>, { match apply_fn() { Ok(()) => Ok(()), @@ -226,7 +228,7 @@ mod tests { let status = state.status(); // Fresh state should have no last_update (unless state file exists) // Just verify it doesn't panic - assert!(status.last_update_success == false || status.last_update.is_some()); + assert!(!status.last_update_success || status.last_update.is_some()); } #[test] @@ -259,19 +261,14 @@ mod tests { #[test] fn test_apply_with_cache_retry_success() { - let result = apply_with_cache_retry( - || Ok(()), - || Ok(()), - ); + let result = apply_with_cache_retry(|| Ok(()), || Ok(())); assert!(result.is_ok()); } #[test] fn test_apply_with_cache_retry_non_fetch_error() { - let result: Result<()> = apply_with_cache_retry( - || Ok(()), - || Err(anyhow::anyhow!("Permission denied")), - ); + let result: Result<()> = + apply_with_cache_retry(|| Ok(()), || Err(anyhow::anyhow!("Permission denied"))); assert!(result.is_err()); let err = result.unwrap_err(); assert!(!is_fetch_error(&err)); diff --git a/tasks/todo.md b/tasks/todo.md index 977fc0c..7bcc2af 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -2,33 +2,49 @@ **Spec:** tasks/issue-2-package-cache-refresh.md **Version:** 1.1.17 -**Status:** In Progress +**Status:** Complete - PR #3 Open --- ## Implementation Checklist -- [ ] 1. Create `src/packages/cache.rs` - Core cache types, stale detection, state persistence, 404 retry logic -- [ ] 2. Add `mod cache;` to `src/packages/mod.rs` -- [ ] 3. Implement `refresh_package_cache()` on AptBackend -- [ ] 4. Implement `refresh_package_cache()` on DnfBackend -- [ ] 5. Implement `refresh_package_cache()` on YumBackend -- [ ] 6. Implement `refresh_package_cache()` on ApkBackend -- [ ] 7. Implement `refresh_package_cache()` on PacmanBackend -- [ ] 8. Implement `last_cache_update()` on all backends (shared state) -- [ ] 9. Add `refresh_package_cache` and `last_cache_update` to PackageManagerBackend trait -- [ ] 10. Enhance health check in `src/api/handlers/system.rs` - add cache status, trigger refresh -- [ ] 11. Update HealthData struct with `last_cache_update` and `cache_status` fields -- [ ] 12. Add pre-apply cache refresh in `src/api/handlers/patches.rs` -- [ ] 13. Bump version in `Cargo.toml` to 1.1.17 -- [ ] 14. Update `ARCHITECTURE.md` with cache refresh flow -- [ ] 15. Update `REQUIREMENTS.md` with FR-007 -- [ ] 16. Implement state file persistence (cache.json read/write) -- [ ] 17. Write unit tests for cache module -- [ ] 18. Build and verify compilation -- [ ] 19. Commit and push to fix/package-cache-refresh branch -- [ ] 20. Create PR and reference Issue #2 +- [x] 1. Create `src/packages/cache.rs` - Core cache types, stale detection, state persistence, 404 retry logic +- [x] 2. Add `mod cache;` to `src/packages/mod.rs` +- [x] 3. Implement `refresh_package_cache()` on AptBackend +- [x] 4. Implement `refresh_package_cache()` on DnfBackend +- [x] 5. Implement `refresh_package_cache()` on YumBackend +- [x] 6. Implement `refresh_package_cache()` on ApkBackend +- [x] 7. Implement `refresh_package_cache()` on PacmanBackend +- [x] 8. Implement `last_cache_update()` on all backends (shared state) +- [x] 9. Add `refresh_package_cache` and `last_cache_update` to PackageManagerBackend trait +- [x] 10. Enhance health check in `src/api/handlers/system.rs` - add cache status, trigger refresh +- [x] 11. Update HealthData struct with `last_cache_update` and `cache_status` fields +- [x] 12. Add pre-apply cache refresh in `src/api/handlers/patches.rs` +- [x] 13. Bump version in `Cargo.toml` to 1.1.17 +- [x] 14. Update `ARCHITECTURE.md` with cache refresh flow +- [x] 15. Update `REQUIREMENTS.md` with FR-007 +- [x] 16. Implement state file persistence (cache.json read/write) +- [x] 17. Write unit tests for cache module +- [x] 18. Build and verify compilation +- [x] 19. Commit and push to fix/package-cache-refresh branch +- [x] 20. Create PR and reference Issue #2 ## Review -_To be filled after implementation_ +**PR:** https://gitea-lxc.moon-dragon.us/git-echo/linux_patch_api/pulls/3 +**Branch:** fix/package-cache-refresh +**Commit:** cf3d597 +**Files Changed:** 12 files, 944 insertions, 15 deletions + +### Issue Resolution + +All 4 requirements from Issue #2 addressed: +1. ✅ Pre-Upgrade Cache Refresh (MUST) - Mandatory cache refresh before every patch_apply +2. ✅ Regular Interval Cache Refresh (MUST) - Cache refresh triggered on health check when stale (>4h) +3. ✅ 404/Fetch Error Handling (SHOULD) - Auto-retry with cache refresh on fetch errors (1 retry) +4. ✅ Stale Cache Detection (SHOULD) - Tracks last_cache_update, reports in health response + +### Known Issue +- SSH key `git_echo_id_ed25519` was rejected by Gitea on port 2222 - pushed via HTTPS + API token instead +- Root cause: Key fingerprint SHA256:W1BK9fCA53/or7iJkONbFSf3KJ6+oiAggPgisZNPhsc not registered in git-echo Gitea account +- Needs investigation: SSH key may need re-registration in Gitea