Proxy the referrer's API to upstream registry by stonezdj · Pull Request #22746 · goharbor/harbor

stonezdj · 2026-01-20T07:46:53Z

Thank you for contributing to Harbor!

Comprehensive Summary of your change

Issue being fixed

Fixes #20808

Please indicate you've done the following:

Well Written Title and Summary of the PR
Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
Accepted the DCO. Commits without the DCO will delay acceptance.
Made sure tests are passing and test coverage is added if needed.
Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

codecov · 2026-01-20T07:50:21Z

Codecov Report

❌ Patch coverage is 28.97727% with 125 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.90%. Comparing base (c8c11b4) to head (335e879).
⚠️ Report is 690 commits behind head on main.

Files with missing lines	Patch %	Lines
src/server/middleware/repoproxy/proxy.go	8.57%	95 Missing and 1 partial ⚠️
src/pkg/registry/client.go	55.81%	14 Missing and 5 partials ⚠️
src/controller/proxy/controller.go	40.00%	3 Missing ⚠️
src/server/middleware/subject/subject.go	57.14%	2 Missing and 1 partial ⚠️
src/controller/proxy/remote.go	0.00%	2 Missing ⚠️
src/server/registry/route.go	0.00%	1 Missing ⚠️
src/server/v2.0/handler/project_metadata.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main   #22746       +/-   ##
===========================================
+ Coverage   45.36%   65.90%   +20.53%     
===========================================
  Files         244     1074      +830     
  Lines       13333   116555   +103222     
  Branches     2719     2935      +216     
===========================================
+ Hits         6049    76815    +70766     
- Misses       6983    35480    +28497     
- Partials      301     4260     +3959

Flag	Coverage Δ
unittests	`65.90% <28.97%> (+20.53%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/pkg/accessory/dao/model.go	`100.00% <ø> (ø)`
src/pkg/accessory/manager.go	`84.21% <100.00%> (ø)`
src/pkg/accessory/model/accessory.go	`88.00% <ø> (ø)`
src/pkg/project/models/project.go	`43.39% <100.00%> (ø)`
src/pkg/reg/adapter/adapter.go	`92.85% <ø> (ø)`
src/pkg/registry/auth/bearer/scope.go	`92.72% <100.00%> (ø)`
src/server/registry/route.go	`0.00% <0.00%> (ø)`
src/server/v2.0/handler/project_metadata.go	`14.28% <0.00%> (ø)`
src/controller/proxy/remote.go	`0.00% <0.00%> (ø)`
src/controller/proxy/controller.go	`17.82% <40.00%> (ø)`
... and 3 more

... and 976 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

wy65701436

lgtm

chlins

lgtm

Signed-off-by: stonezdj <stonezdj@gmail.com>

Copilot

Pull request overview

Adds support for proxy-cache projects to proxy the OCI Distribution Spec 1.1 Referrers API (/v2/<name>/referrers/<digest>) to the upstream registry, addressing Harbor issue #20808.

Changes:

Add ListReferrers support across the registry client/adapter/proxy remote layers.
Introduce a per-project metadata flag (proxy_referrer_api) and wire a new middleware to proxy referrers requests to upstream (with caching for unhealthy upstream scenarios).
Persist a new source field on accessories (DB + model) to distinguish locally-created vs proxycache-created accessories.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/testing/pkg/registry/fake_registry_client.go	Extends mock registry client with `ListReferrers`.
src/testing/controller/proxy/remote_interface.go	Extends proxy remote interface mock with `ListReferrers`.
src/server/v2.0/handler/project_metadata.go	Allows `proxy_referrer_api` in project metadata validation.
src/server/registry/route.go	Adds `ProxyReferrerMiddleware` to the referrers route.
src/server/middleware/subject/subject.go	Adds accessory `Source` assignment (local vs proxycache).
src/server/middleware/repoproxy/proxy.go	Implements referrers proxying + caching + header forwarding.
src/server/middleware/repoproxy/proxy_test.go	Adds tests for referrers proxy helpers/middleware (currently placeholders).
src/pkg/registry/client.go	Adds registry client `ListReferrers` + URL builder.
src/pkg/registry/client_test.go	Adds tests for `ListReferrers` and `buildReferrersURL`.
src/pkg/registry/auth/bearer/scope.go	Adds referrers URL parsing for token scope generation.
src/pkg/registry/auth/bearer/scope_test.go	Adds test coverage for referrers scope parsing.
src/pkg/reg/adapter/adapter.go	Extends `ArtifactRegistry` interface with `ListReferrers`.
src/pkg/project/models/project.go	Adds `Project.ProxyReferrerAPI()` helper.
src/pkg/project/models/pro_meta.go	Adds `ProMetaProxyReferrerAPI` metadata key constant.
src/pkg/accessory/model/accessory.go	Adds `Source` to accessory API/data model.
src/pkg/accessory/manager.go	Maps `Source` through accessory manager read/write paths.
src/pkg/accessory/dao/model.go	Adds `source` ORM column to accessory DAO model.
src/controller/replication/transfer/image/transfer_test.go	Updates fake registry to satisfy new interface method.
src/controller/replication/flow/mock_adapter_test.go	Updates mock adapter with `ListReferrers`.
src/controller/proxy/remote.go	Adds `ListReferrers` to proxy remote helper interface/impl.
src/controller/proxy/controller.go	Exports `GetRemoteRepo` for reuse.
src/controller/proxy/controller_test.go	Updates tests to use exported `GetRemoteRepo`.
make/migrations/postgresql/0180_2.15.0_schema.up.sql	Adds `artifact_accessory.source` column.
api/v2.0/swagger.yaml	Updates description for `proxy_referrer_api` metadata.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-26T13:37:42Z

 			if err := orm.WithTransaction(func(ctx context.Context) error {
+				source := sourceLocal
+				securityCtx, exist := security.FromContext(ctx)
+				if !exist && securityCtx != nil && securityCtx.GetUsername() == proxycachesecret.ProxyCacheService {


The proxy cache source detection is inverted: security.FromContext returns (nil, false) when no security context is present, so the condition if !exist && securityCtx != nil ... can never be true. This prevents accData.Source from ever being set to proxycache even when the request is made by proxycachesecret.ProxyCacheService. Flip the condition to check exist (and then compare username) before setting sourceProxyCache.

Suggested change

if !exist && securityCtx != nil && securityCtx.GetUsername() == proxycachesecret.ProxyCacheService {

if exist && securityCtx != nil && securityCtx.GetUsername() == proxycachesecret.ProxyCacheService {

Copilot · 2026-01-26T13:37:42Z

+	WriteProxyHeaders(w, headerMap)
+	w.WriteHeader(http.StatusOK)
+	b, err := json.Marshal(index)
+	if err != nil {
+		return err
+	}


proxyReferrerGet writes headers/status (WriteProxyHeaders + WriteHeader(200)) before json.Marshal(index) can fail. If marshaling fails, the middleware falls back to next.ServeHTTP, but the response has already started, risking a double-write/corrupted response. Marshal first (or otherwise ensure no further handler runs after headers are written), then write headers/status/body.

Suggested change

WriteProxyHeaders(w, headerMap)

w.WriteHeader(http.StatusOK)

b, err := json.Marshal(index)

if err != nil {

return err

}

b, err := json.Marshal(index)

if err != nil {

return err

}

WriteProxyHeaders(w, headerMap)

w.WriteHeader(http.StatusOK)

Copilot · 2026-01-26T13:37:43Z

+// TestProxyReferrerMiddlewareNonProxyProject tests that non-proxy projects bypass the middleware
+func TestProxyReferrerMiddlewareNonProxyProject(t *testing.T) {
+	// Create a non-proxy project
+	proj := &proModels.Project{
+		ProjectID:  1,
+		Name:       "library",
+		RegistryID: 0, // 0 indicates non-proxy project
+	}
+
+	// Create context with artifact info
+	ctx := context.Background()
+	artInfo := lib.ArtifactInfo{
+		ProjectName: "library",
+		Repository:  "library/hello-world",
+		Reference:   "latest",
+		Digest:      "sha256:abc123",
+	}
+	ctx = lib.WithArtifactInfo(ctx, artInfo)
+
+	// Create request
+	req := httptest.NewRequest("GET", "/v2/library/hello-world/referrers", nil)
+	req = req.WithContext(ctx)
+
+	// Create response writer
+	_ = httptest.NewRecorder()
+
+	// Track if next handler was called
+	_ = false
+	nextHandler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+		rw.WriteHeader(http.StatusOK)
+	})
+
+	// Create the middleware - we need to temporarily replace the project controller
+	middleware := ProxyReferrerMiddleware()
+
+	// Apply middleware
+	handler := middleware(nextHandler)
+
+	// This will fail without mocking, but demonstrates the test structure
+	// In a real scenario, we'd need to mock the project controller
+	t.Run("non-proxy project should call next handler", func(t *testing.T) {
+		// The test here demonstrates the expected behavior
+		// In practice, you would mock the project.Ctl.GetByName to return the non-proxy project
+		require.NotNil(t, handler)
+		require.False(t, proj.IsProxy())
+		_ = req
+	})


These new middleware tests don't exercise behavior: they never call handler.ServeHTTP and only assert on local variables (with comments like "This will fail without mocking"). As written they will pass even if ProxyReferrerMiddleware is broken. Either implement proper mocking (e.g., patch project.Ctl/registry.Ctl and validate response/next-handler invocation) or remove these placeholder tests.

Copilot · 2026-01-26T13:37:43Z

+	// Create request
+	req := httptest.NewRequest("GET", "/v2/library/hello-world/referrers", nil)
+	req = req.WithContext(ctx)
+
+	// Create response writer
+	_ = httptest.NewRecorder()
+
+	// Track if next handler was called
+	_ = false
+	nextHandler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+		rw.WriteHeader(http.StatusOK)
+	})
+
+	// Create the middleware - we need to temporarily replace the project controller
+	middleware := ProxyReferrerMiddleware()
+
+	// Apply middleware
+	handler := middleware(nextHandler)
+
+	// This will fail without mocking, but demonstrates the test structure
+	// In a real scenario, we'd need to mock the project controller
+	t.Run("non-proxy project should call next handler", func(t *testing.T) {
+		// The test here demonstrates the expected behavior
+		// In practice, you would mock the project.Ctl.GetByName to return the non-proxy project
+		require.NotNil(t, handler)
+		require.False(t, proj.IsProxy())
+		_ = req
+	})
+}
+
+// TestProxyReferrerMiddlewareProjectNotFound tests error handling when project is not found
+func TestProxyReferrerMiddlewareProjectNotFound(t *testing.T) {
+	// Create context with artifact info
+	ctx := context.Background()
+	artInfo := lib.ArtifactInfo{
+		ProjectName: "unknown",
+		Repository:  "unknown/image",
+		Reference:   "latest",
+		Digest:      "sha256:abc123",
+	}
+	ctx = lib.WithArtifactInfo(ctx, artInfo)
+
+	// Create request
+	req := httptest.NewRequest("GET", "/v2/unknown/image/referrers", nil)
+	req = req.WithContext(ctx)


The request paths used in these tests (e.g. .../referrers without /:reference) don't match the actual routed referrers endpoint (/v2/*/referrers/:reference). Even after adding mocks, these tests won't reflect real requests unless they include a digest/reference segment.

Copilot · 2026-01-26T13:37:43Z

+func (c *client) ListReferrers(repository, ref string, rawQuery string) (*v1.Index, map[string][]string, error) {
+	remoteURL := buildReferrersURL(c.url, repository, ref, rawQuery)
+	req, err := http.NewRequest(http.MethodGet, remoteURL, nil)
+	if err != nil {
+		return nil, nil, err
+	}
+	log.Debugf("upstream url %v", remoteURL)
+
+	if c.authorizer == nil {
+		log.Debug("registry client authorizer is nil")
+	}
+
+	resp, err := c.do(req)
+	if err != nil {


ListReferrers doesn’t set an Accept header. Other registry client calls (e.g. manifest fetch) explicitly set Accept for content negotiation; referrers should similarly request the OCI index media type so upstream registries that enforce Accept don’t return 406/unsupported responses.

Copilot · 2026-01-26T13:37:44Z

+	// HTTP Status check
+	if resp.StatusCode != http.StatusOK {
+		if resp.StatusCode == http.StatusNotFound {
+			return nil, nil, errors.New(nil).WithCode(errors.NotFoundCode).
+				WithMessagef("referrers for %s:%s not found", repository, ref)
+		}
+		body, _ := io.ReadAll(resp.Body)
+		return nil, nil, fmt.Errorf("upstream returned status %d: %s", resp.StatusCode, string(body))
+	}
+


ListReferrers calls c.do(req), which already converts any non-2xx response into an error and returns resp=nil. The subsequent resp.StatusCode checks and error-body reads are therefore unreachable and can be removed/simplified (e.g., rely on errors.IsNotFoundErr(err) from c.do instead of a second status check).

Suggested change

// HTTP Status check

if resp.StatusCode != http.StatusOK {

if resp.StatusCode == http.StatusNotFound {

return nil, nil, errors.New(nil).WithCode(errors.NotFoundCode).

WithMessagef("referrers for %s:%s not found", repository, ref)

}

body, _ := io.ReadAll(resp.Body)

return nil, nil, fmt.Errorf("upstream returned status %d: %s", resp.StatusCode, string(body))

}

This reverts commit 30ba1b3.

This reverts commit 30ba1b3. Signed-off-by: stonezdj <stone.zhang@broadcom.com>

Revert "Proxy the referrer's API to upstream registry (#22746)" This reverts commit 30ba1b3. Signed-off-by: stonezdj <stone.zhang@broadcom.com>

Signed-off-by: stonezdj <stonezdj@gmail.com> Signed-off-by: sias32 <sias.32@yandex.ru>

Revert "Proxy the referrer's API to upstream registry (goharbor#22746)" This reverts commit 30ba1b3. Signed-off-by: stonezdj <stone.zhang@broadcom.com> Signed-off-by: sias32 <sias.32@yandex.ru>

Signed-off-by: stonezdj <stonezdj@gmail.com> Signed-off-by: sias32 <sias.32@yandex.ru>

Revert "Proxy the referrer's API to upstream registry (goharbor#22746)" This reverts commit 30ba1b3. Signed-off-by: stonezdj <stone.zhang@broadcom.com> Signed-off-by: sias32 <sias.32@yandex.ru>

Signed-off-by: stonezdj <stonezdj@gmail.com> Signed-off-by: sias32 <sias320@gmail.com>

Revert "Proxy the referrer's API to upstream registry (goharbor#22746)" This reverts commit 30ba1b3. Signed-off-by: stonezdj <stone.zhang@broadcom.com> Signed-off-by: sias32 <sias320@gmail.com>

stonezdj requested a review from a team as a code owner January 20, 2026 07:46

github-actions Bot assigned chlins, MinerYang and OrlinVasilev Jan 20, 2026

stonezdj added the release-note/new-feature New Harbor Feature label Jan 20, 2026

stonezdj requested a review from wy65701436 January 20, 2026 07:47

stonezdj force-pushed the 25nov17_try_proxy_referrer_api branch 5 times, most recently from 0abb982 to 5399759 Compare January 21, 2026 06:56

wy65701436 added the target/2.15.0 label Jan 23, 2026