Skip to content

fix(harbor-gc): make the garbage collector fail safe#149

Open
lfrancke wants to merge 1 commit into
mainfrom
fix/harbor-gc-safety
Open

fix(harbor-gc): make the garbage collector fail safe#149
lfrancke wants to merge 1 commit into
mainfrom
fix/harbor-gc-safety

Conversation

@lfrancke

Copy link
Copy Markdown
Member

Harbor garbage-collector safety

A garbage collector that reports success while deleting the wrong image (or nothing) is a quiet operational hazard. All in tools/harbor-garbage-collector/src/main.rs.

  • Silent delete failuresDELETE responses were never status-checked, and the robot password used .ok(), so a missing/renamed HARBOR_ROBOT_PASSWORD silently degraded to unauthenticated deletes: the tool prints "Removing…", deletes nothing (401/403), and exits 0. Now the password is required and every delete uses error_for_status().
  • TOCTOU race could delete the current latestlatest's references were snapshotted, then the artifact list fetched separately; a rust-builder:latest pushed in that window would be deleted as "dangling". Added a re-check: if latest's digest moved during the run, skip the cleanup for that image.
  • Unpaginated listing — the rust-builder artifact list used a single page_size=100 request; >100 artifacts silently ignored. Now paginated like the rest of the file.
  • Panic on external data.references.unwrap() panicked if latest had no references (e.g. single-arch). Now unwrap_or_default().
  • No HTTP timeout / client-per-delete — replaced ad-hoc reqwest::get / Client::new() with one shared client with a 30s timeout.

Verification

cargo check and cargo clippy both clean.

Scoped to this one tool; the other Rust tools' timeouts/hardening are a separate PR.

- Check DELETE responses with error_for_status() instead of ignoring them,
  and require HARBOR_ROBOT_PASSWORD (was `.ok()`, so a missing/renamed var
  silently produced unauthenticated no-op deletes while exiting 0).
- Guard the rust-builder cleanup against a TOCTOU race: if a new image is
  tagged "latest" between snapshotting its references and listing artifacts,
  re-check and skip the cleanup instead of deleting the current latest.
- Paginate the rust-builder artifact listing (a single page_size=100 request
  silently ignored anything past the first 100).
- Don't panic when "latest" has no references (unwrap -> unwrap_or_default).
- Use a single shared reqwest client with a 30s timeout so a hung registry
  can't block forever (also avoids building a client per delete).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant