Skip to content

fix(proxy): preserve URL path prefix during registry auth discovery#22989

Merged
reasonerjt merged 2 commits intogoharbor:mainfrom
mco69:fix/proxy-auth-path-prefix
Apr 9, 2026
Merged

fix(proxy): preserve URL path prefix during registry auth discovery#22989
reasonerjt merged 2 commits intogoharbor:mainfrom
mco69:fix/proxy-auth-path-prefix

Conversation

@mco69
Copy link
Copy Markdown
Contributor

@mco69 mco69 commented Mar 12, 2026

While Harbor itself does not support being deployed with a path prefix (e.g., /path/v2), it does support proxying registries that use them. This is significant; not only do registries like Artifactory utilizing such deployment structures, but in enterprise environments, where it is often impractical to create a distinct FQDN for every registry endpoint (and proxy may be used).

However when a registry endpoint includes a path prefix (e.g. https:///path), the authorizer's initialize method was hardcoded to probe scheme://host/v2/ for auth challenge discovery, discarding the path entirely. On the flip side, when injecting a Bearer token everything seems to work fine!

This caused two failures for path-prefixed registries:

  1. Auth discovery hit the wrong endpoint (missing path prefix)
  2. isTarget() refused to attach credentials to actual API requests because the stored /v2/ path didn't match the request's /path/v2/

Extract the path prefix before /v2/ from the first outgoing request and include it when constructing the auth probe URL. The isTarget comparison already handles this correctly once the stored URL contains the full prefix.

As seen in the NGINX logs below, Harbor correctly targets the custom path prefix but fails the subsequent authentication challenge:

10.42.0.39 - - [12/Mar/2026:00:21:32 +0000] "HEAD /hardcoded-path/v2/2.1.0/xwz-tbr-bom/manifests/2.1.0-20094027 HTTP/1.1" 401 0 "-" "harbor-registry-client" "-"

As seen in the NGINX logs below, Harbor correctly targets the custom path an image pull succeeds if NGING injects bearer token:

10.42.0.39 - - [12/Mar/2026:22:41:15 +0000] "HEAD /hardcoded-path/v2/2.1.0/xwz-tbr-bom/manifests/2.1.0-20094027 HTTP/1.1" 200 0 "-" "harbor-registry-client" "-"
<truncated>

Thank you for contributing to Harbor!

Comprehensive Summary of your change

Issue being fixed

Fixes #(issue)

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.

@mco69
Copy link
Copy Markdown
Contributor Author

mco69 commented Mar 13, 2026

looks like an alternative to this is #22891

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.04%. Comparing base (6d822c2) to head (fe24105).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #22989      +/-   ##
==========================================
+ Coverage   66.02%   66.04%   +0.01%     
==========================================
  Files        1073     1073              
  Lines      116503   116507       +4     
  Branches     2939     2939              
==========================================
+ Hits        76922    76947      +25     
+ Misses      35328    35311      -17     
+ Partials     4253     4249       -4     
Flag Coverage Δ
unittests 66.04% <100.00%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
src/pkg/registry/auth/authorizer.go 66.27% <100.00%> (+21.15%) ⬆️

... and 10 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.

@MinerYang
Copy link
Copy Markdown
Contributor

MinerYang commented Mar 16, 2026

Hi @mco69 ,

Thanks for connecting with us.

I believe for this feature we need a design approved first, and you could add a proposal on here https://github.com/goharbor/community/tree/main/proposals and join our bi-weekly meeting for further discussion if needs.
https://goharbor.io/community/

@mco69
Copy link
Copy Markdown
Contributor Author

mco69 commented Mar 16, 2026

Hi @mco69 ,

Thanks for connecting with us.

I believe for this feature we need a design approved first, and you could add a proposal on here https://github.com/goharbor/community/tree/main/proposals and join our bi-weekly meeting for further discussion if needs. https://goharbor.io/community/

@MinerYang thanks, here is the proposal: goharbor/community#278

@reasonerjt
Copy link
Copy Markdown
Contributor

reasonerjt commented Mar 17, 2026

Thanks @mco69, but as we discussed offline, this approach is preferred:
#22891

Let's keep this open unless we agree #22891 doesn't solve your problem.

@reasonerjt
Copy link
Copy Markdown
Contributor

Thanks @mco69, but as we discussed offline, this approach is preferred: #22891

Let's keep this open unless we agree #22891 doesn't solve your problem.

Ignore this one, after more discussion, we will try to merge this one.

Copy link
Copy Markdown
Contributor

@wy65701436 wy65701436 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

When a registry endpoint includes a path prefix (e.g.
https://hostname/path), the authorizer's initialize method was
hardcoded to probe scheme://host/v2/ for auth challenge discovery,
discarding the path entirely. This caused two failures for
path-prefixed registries:

1. Auth discovery hit the wrong endpoint (missing path prefix)
2. isTarget() refused to attach credentials to actual API requests
   because the stored /v2/ path didn't match the request's /path/v2/

Extract the path prefix before /v2/ from the first outgoing request
and include it when constructing the auth probe URL. The isTarget
comparison already handles this correctly once the stored URL contains
the full prefix.

Signed-off-by: mcolombet <mathieu.colombet@broadcom.com>
@mco69 mco69 force-pushed the fix/proxy-auth-path-prefix branch from 048806c to 8b2b98e Compare April 9, 2026 04:05
@reasonerjt reasonerjt added the release-note/enhancement Label to mark PR to be added under release notes as enhancement label Apr 9, 2026
@reasonerjt reasonerjt merged commit 02bdc9e into goharbor:main Apr 9, 2026
12 of 13 checks passed
bupd added a commit to container-registry/harbor-next that referenced this pull request Apr 10, 2026
…discovery

When a registry endpoint includes a path prefix, the authorizer was hardcoded to probe scheme://host/v2/ discarding the path entirely. This caused auth discovery and credential attachment to fail for path-prefixed registries.

Upstream: goharbor/harbor#22989

Signed-off-by: mcolombet <mathieu.colombet@broadcom.com>
Signed-off-by: Prasanth Baskar <bupdprasanth@gmail.com>
bupd added a commit to container-registry/harbor-next that referenced this pull request Apr 10, 2026
…discovery

When a registry endpoint includes a path prefix, the authorizer was hardcoded to probe scheme://host/v2/ discarding the path entirely. This caused auth discovery and credential attachment to fail for path-prefixed registries.

Upstream: goharbor/harbor#22989

Signed-off-by: mcolombet <mathieu.colombet@broadcom.com>
Signed-off-by: Prasanth Baskar <bupdprasanth@gmail.com>
sm-moshi pushed a commit to sm-moshi/harbor that referenced this pull request Apr 12, 2026
…oharbor#22989)

When a registry endpoint includes a path prefix (e.g.
https://hostname/path), the authorizer's initialize method was
hardcoded to probe scheme://host/v2/ for auth challenge discovery,
discarding the path entirely. This caused two failures for
path-prefixed registries:

1. Auth discovery hit the wrong endpoint (missing path prefix)
2. isTarget() refused to attach credentials to actual API requests
   because the stored /v2/ path didn't match the request's /path/v2/

Extract the path prefix before /v2/ from the first outgoing request
and include it when constructing the auth probe URL. The isTarget
comparison already handles this correctly once the stored URL contains
the full prefix.

Signed-off-by: mcolombet <mathieu.colombet@broadcom.com>
(cherry picked from commit 02bdc9e)
ipsitapp8 pushed a commit to ipsitapp8/harbor that referenced this pull request Apr 13, 2026
…oharbor#22989)

When a registry endpoint includes a path prefix (e.g.
https://hostname/path), the authorizer's initialize method was
hardcoded to probe scheme://host/v2/ for auth challenge discovery,
discarding the path entirely. This caused two failures for
path-prefixed registries:

1. Auth discovery hit the wrong endpoint (missing path prefix)
2. isTarget() refused to attach credentials to actual API requests
   because the stored /v2/ path didn't match the request's /path/v2/

Extract the path prefix before /v2/ from the first outgoing request
and include it when constructing the auth probe URL. The isTarget
comparison already handles this correctly once the stored URL contains
the full prefix.

Signed-off-by: mcolombet <mathieu.colombet@broadcom.com>
Signed-off-by: ipsitapp8 <ipsitapp8@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs/design release-note/enhancement Label to mark PR to be added under release notes as enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants