🐛 Prevent catalog temp directory prefix overlap

[tmshort]

Summary

• Change catalogd and operator-controller staging directory prefixes from .{catalog}- to .{catalog}_ in both creation and orphan cleanup.
• Kubernetes catalog names cannot contain underscores, so cleanup for redhat-operators cannot match a staging directory for redhat-operators-mirrored.
• Add regression coverage for overlapping catalog names in both components.
• Correct the operator-controller comment to describe a container crash, which can leave data in the pod's emptyDir.

The two affected catalog-derived prefixes are in internal/catalogd/storage/localdir.go and internal/operator-controller/catalogmetadata/cache/cache.go. This addresses Joe's review comment on the 4.21 backport.

Validation

go test -p 2 -tags containers_image_openpgp ./internal/catalogd/storage ./internal/operator-controller/catalogmetadata/cache

Summary by CodeRabbit

• Bug Fixes
  • Improved cleanup of interrupted catalog writes. Temporary directories are now matched more precisely, helping preserve directories belonging to other catalogs.

[netlify]

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	59ce783
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/6abfef861e41fe0008068769
😎 Deploy Preview	https://deploy-preview-2975--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: d304db42-7e63-46a2-a618-41f34405ac6e

📥 Commits

Reviewing files that changed from the base of the PR and between b980fea and 59ce783.

📒 Files selected for processing (4)

• internal/catalogd/storage/localdir.go
• internal/catalogd/storage/localdir_test.go
• internal/operator-controller/catalogmetadata/cache/cache.go
• internal/operator-controller/catalogmetadata/cache/cache_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

---

📝 Walkthrough

Walkthrough

Temporary catalog directories now use underscore-separated prefixes in both storage implementations. Orphan cleanup matches those prefixes, and tests check that cleanup removes target-catalog directories while preserving directories for other catalogs.

Changes

Catalog temporary-directory cleanup

Layer / File(s)	Summary
Update staging and orphan cleanup prefixes internal/catalogd/storage/localdir.go, internal/catalogd/storage/localdir_test.go, internal/operator-controller/catalogmetadata/cache/cache.go, internal/operator-controller/catalogmetadata/cache/cache_test.go	Both storage implementations create staging directories and identify orphan directories with an underscore-separated catalog prefix. Tests verify removal of target-catalog directories and preservation of directories for other catalogs, including one whose name extends the target name.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: grokspawn

Merge Risk: ⚪ Minimal · up to 59ce7

The reported prefix change and cleanup-boundary tests establish no concrete merge-blocking failure. The catalog-name constraint remains unconfirmed.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 59ce7

The change narrows cleanup to the intended catalog and preserves existing write synchronization and publication behavior. Startup cleanup also prevents old-format staging directories from accumulating across normal upgrades or rollbacks. No material security risk was found to be introduced or worsened.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed destructive operation acts on matching staging entries beneath each implementation's configured root. Its catalog-selection scope is narrowed rather than expanded; the production diff introduces no new caller, credential access, or authority transition.

Trust Boundaries and Controls

• observed — The production catalog identity continues to come from ClusterCatalog object names. Cleanup remains serialized with writes by the existing storage-instance locks; the new delimiter does not grant additional filesystem authority.

Resilience and Maintainability Implications

• observed — Regression coverage checks the cleanup ownership boundary in both components: target leftovers are removed, while unrelated and similarly named catalogs retain their staging directories.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and clearly describes the main bug fix: preventing overlap between catalog temporary-directory prefixes.
Description check	✅ Passed	The description explains the change, motivation, affected components, regression tests, validation command, and related review discussion. It does not reproduce the Reviewer Checklist, but the missing…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[joelanford]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: joelanford

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [joelanford]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment