fix: [AH-2826]: Fixed write flows to go through existing replication in gitness

[abhinavcode]

Summary by CodeRabbit

• Refactor
  • Avoid unnecessary blob store reads by skipping retrieval when blob identifiers are not present, improving efficiency and reducing redundant operations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

📝 Walkthrough

Walkthrough

Added a check in GetBlobsContext to skip retrieving the blob store unless blobLocator.BlobID != 0 or blobLocator.GenericBlobID != uuid.Nil, preventing the blob store lookup when both identifiers are absent. No public API changes.

Changes

Cohort / File(s)	Summary
Docker registry control flow registry/app/pkg/docker/app.go	Imported google/uuid and updated GetBlobsContext to conditionally call GetBlobStore only when blobLocator.BlobID != 0 or blobLocator.GenericBlobID != uuid.Nil; skips the blob store lookup when both are absent. No exported signatures changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I nibbled at code with a careful hop,
If IDs are missing, the store we drop.
A tiny gate opens only when seen,
Lightweight hops keep the path clean. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title mentions fixing write flows for replication in gitness, but the actual change only adds a condition gate to skip blob store retrieval when blob identifiers are absent—no replication-related logic is introduced.	Update the title to accurately reflect the change, such as 'refactor: Skip blob store retrieval when blob identifiers are absent' or 'fix: Gate blob store access behind identifier validation'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch AH-2826-fix-merge-issue-gitness

---

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 and usage tips.}

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@registry/app/pkg/docker/app.go`:
- Around line 143-149: The guard is allowing GenericBlobID to drive the branch
but GetBlobStore only uses BlobID; update the condition to only check
blobLocator.BlobID != 0 (remove reference to blobLocator.GenericBlobID) so we
don't call app.bucketService.GetBlobStore with a zero blob id; locate the block
around GetOciBlobsContext and the call to app.bucketService.GetBlobStore and
change the if to rely solely on blobLocator.BlobID to match the actual semantic
use.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect BlobLocator type definition
echo "=== BlobLocator definition ==="
rg -n "type BlobLocator" --type go -A 10

echo ""
echo "=== GetBlobStore interface/function signatures ==="
rg -n "GetBlobStore" --type go -B 2 -A 5

echo ""
echo "=== Context: Show lines 143-149 in app.go ==="
sed -n '140,160p' registry/app/pkg/docker/app.go

Repository: abhinavcode/harness

Length of output: 4105

---

🏁 Script executed:

#!/bin/bash
# Check how GetBlobStore is implemented and handles blobID=0
echo "=== Searching for GetBlobStore implementation ==="
rg -n "GetBlobStore" --type go -A 15 | head -80

echo ""
echo "=== Check context around the problematic code to understand flow ==="
sed -n '130,160p' registry/app/pkg/docker/app.go

echo ""
echo "=== Check if there are other callers of GetBlobStore ==="
rg -n "GetBlobStore\(" --type go -B 2 -A 2

Repository: abhinavcode/harness

Length of output: 6678

---

GenericBlobID is checked in the guard but never forwarded to GetBlobStore.

The OR condition at line 143 enters the if-block when blobLocator.GenericBlobID != uuid.Nil (even when BlobID == 0), but GetBlobStore only accepts a blobID parameter of type any—there is no parameter for GenericBlobID. When BlobID is 0 and GenericBlobID is non-nil, the call passes a zero BlobID:

app.bucketService.GetBlobStore(c, info.RegIdentifier, info.RootIdentifier, blobLocator.BlobID, ...)

This results in either a wasted lookup (if GetBlobStore returns nil for ID 0) or unpredictable behavior depending on the implementation.

For OCI operations (in GetOciBlobsContext), GenericBlobID is irrelevant—only BlobID (the int64 field) is semantically meaningful. The condition should be simplified to blobLocator.BlobID != 0 to match what is actually consumed.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@registry/app/pkg/docker/app.go` around lines 143 - 149, The guard is allowing
GenericBlobID to drive the branch but GetBlobStore only uses BlobID; update the
condition to only check blobLocator.BlobID != 0 (remove reference to
blobLocator.GenericBlobID) so we don't call app.bucketService.GetBlobStore with
a zero blob id; locate the block around GetOciBlobsContext and the call to
app.bucketService.GetBlobStore and change the if to rely solely on
blobLocator.BlobID to match the actual semantic use.