fix(state): handle deleted artifacts in satellite state replication - #622
fix(state): handle deleted artifacts in satellite state replication#622harshal0704 wants to merge 2 commits into
Conversation
- Add Deleted field to Entity struct for tracking deletion state - Propagate artifact.IsDeleted() in FetchEntitiesFromState - Filter out deleted artifacts on initial sync (oldEntities == nil) - Route newEntity.Deleted with active oldEntity to entityToDelete in GetChanges - Guard deletion scheduling with !oldEntity.Deleted for reconciliation idempotency - Allow re-replication of previously deleted artifacts if re-added upstream - Add comprehensive unit tests covering all deletion edge cases Signed-off-by: Harshal <harshal@iitbhilai.ac.in>
|
Warning Review limit reached
Next review available in: 13 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe state replication model now preserves artifact deletion status. ChangesDeleted artifact reconciliation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant StateJSON as State JSON
participant FetchEntitiesFromState
participant GetChanges
participant SatelliteStorage as Satellite storage
StateJSON->>FetchEntitiesFromState: Read artifact deletion status
FetchEntitiesFromState->>GetChanges: Provide Entity.Deleted
GetChanges->>SatelliteStorage: Schedule active deleted entities for deletion
GetChanges->>SatelliteStorage: Replicate restored entities
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 15 |
| Duplication | 44 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/satellite/state/state_process.go`:
- Around line 209-215: Update the reconciliation key used by the
state-processing logic around the deleted-entity branch to include Repository,
Name, and Tag consistently in both map construction and lookup, preventing
cross-repository collisions. Add a regression test covering identical names and
tags in two repositories and verify deletion only affects the matching
repository entity.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f2959c0e-7e0c-4e12-8d54-8cfb5451097b
📒 Files selected for processing (3)
internal/satellite/state/replicator.gointernal/satellite/state/state_process.gointernal/satellite/state/state_process_test.go
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Add Deleted field to Entity struct for tracking deletion state - Propagate artifact.IsDeleted() in FetchEntitiesFromState - Filter out deleted artifacts on initial sync (oldEntities == nil) - Route newEntity.Deleted with active oldEntity to entityToDelete in GetChanges - Guard deletion scheduling with !oldEntity.Deleted for reconciliation idempotency - Allow re-replication of previously deleted artifacts if re-added upstream - Key entity lookup by Repository|Name|Tag to prevent cross-repository collisions - Update initial-sync log message to reflect deleted-artifact filtering - Add comprehensive unit tests covering all deletion and collision edge cases Signed-off-by: Harshal <harshal@iitbhilai.ac.in>
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Description
This PR fixes an issue in Harbor Satellite state processing where artifacts marked as deleted (
deleted: true) in Ground Control state manifests were being replicated/pulled to local satellite storage instead of being deleted.Summary of Changes
internal/satellite/state/replicator.go:Deleted boolfield toEntitystruct withjson:"deleted,omitempty"for backward compatibility with persisted state.internal/satellite/state/state_process.go:FetchEntitiesFromState: Propagatedartifact.IsDeleted()intoEntity.Deleted.GetChanges:entity.Deletedon initial sync (oldEntities == nil) so deleted images are not pulled on first satellite sync.newEntity.Deletedwith activeoldEntity(!oldEntity.Deleted) toentityToDeleteand skipped replication.!oldEntity.Deletedto ensure deletion is idempotent and not repeatedly scheduled on subsequent reconciliation loops.oldEntity.Deleted == trueandnewEntity.Deleted == false) by allowing re-replication.internal/satellite/state/state_process_test.go:TestGetChangesandTestFetchEntitiesFromStatecovering initial sync filtering, active entity deletion, loop idempotency, re-added artifact replication, and deleted flag propagation.Verification & Test Evidence
All unit tests pass cleanly across the workspace:
Additional context
Signed-off-by).state.json(json:"deleted,omitempty").