You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "codelipenghui (via GitHub)" <gi...@apache.org> on 2025/08/07 20:48:07 UTC
[PR] [fix][ws] Fix WebSocket proxy originalPrincipal for HTTP admin API calls [pulsar]
codelipenghui opened a new pull request, #24613:
URL: https://github.com/apache/pulsar/pull/24613
## Motivation
This is a follow-up fix to PR #24533 which addressed WebSocket authorization issues by introducing a dummy original principal for binary protocol connections, but missed the case where WebSocket proxy uses HTTP admin API calls (web service URLs).
## Modifications
**Issue:**
When WebSocket proxy is configured to use web service URLs instead of broker service URLs, it makes HTTP admin API calls for operations like topic lookup. These HTTP requests were missing the `X-Original-Principal` header, causing authorization failures with the error:
```
Illegal combination of role [websocket_proxy] and originalPrincipal [null]:
originalPrincipal must be provided when connecting with a proxy role.
```
**Root Cause:**
The `originalPrincipal` was correctly set for binary protocol connections via `ClientBuilderImpl.originalPrincipal()`, but `HttpClient` did not include this information as an `X-Original-Principal` header in HTTP requests to admin APIs.
**Fix:**
- Modified `HttpClient` to store `ClientConfigurationData` and automatically include `X-Original-Principal` header when `originalPrincipal` is configured
- Added `ProxyRoleAuthWebServiceURLTest` to verify the fix for web service URL scenarios
## Verifying this change
- [x] Added new test `ProxyRoleAuthWebServiceURLTest` that reproduces the issue and validates the fix
- [x] Verified existing `ProxyRoleAuthTest` continues to pass (binary protocol scenarios)
- [x] The fix ensures WebSocket proxy authentication works correctly for both:
1. Binary protocol (broker service URLs) - already fixed by #24533
2. HTTP admin API (web service URLs) - now fixed by this change
## Does this pull request potentially affect one of the following parts:
*If the box was checked, please highlight the changes*
- [x] Dependencies (add or upgrade a dependency)
- No new dependencies added, only modified existing HttpClient behavior
- [x] The public API
- No public API changes, internal HTTP client behavior only
- [x] The schema
- No schema changes
- [x] The default behavior
- Improved: WebSocket proxy now properly includes originalPrincipal in HTTP requests when configured
- [x] The wire protocol
- No wire protocol changes
- [x] The REST endpoints
- No REST endpoint changes, but improves HTTP client behavior when calling admin APIs
- [x] The admin CLI or the admin API
- No admin CLI/API changes, but fixes HTTP client calls to admin APIs
- [x] Anything that affects deployment
- No deployment changes
## Documentation
- [x] `doc-required`
- No documentation changes required, this is a bug fix that makes existing functionality work correctly
- [x] `doc-not-needed`
- This is an internal fix for a missing feature from #24533
- [x] `doc-complete`
- No documentation needed
🤖 Generated with [Claude Code](https://claude.ai/code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][ws] Fix WebSocket proxy originalPrincipal for HTTP admin API calls [pulsar]
Posted by "codelipenghui (via GitHub)" <gi...@apache.org>.
codelipenghui commented on PR #24613:
URL: https://github.com/apache/pulsar/pull/24613#issuecomment-3165698606
/pulsarbot run-failure-checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][ws] Fix WebSocket proxy originalPrincipal for HTTP admin API calls [pulsar]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #24613:
URL: https://github.com/apache/pulsar/pull/24613#issuecomment-3166483655
## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/24613?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 74.28%. Comparing base ([`66b512c`](https://app.codecov.io/gh/apache/pulsar/commit/66b512c4a5bec88212856ec2e25f096c953714b4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`84a6536`](https://app.codecov.io/gh/apache/pulsar/commit/84a65367457f241327087c52091d6603fca4ee5c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/pulsar/pull/24613?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #24613 +/- ##
=============================================
+ Coverage 38.78% 74.28% +35.49%
- Complexity 13011 32764 +19753
=============================================
Files 1824 1881 +57
Lines 142703 146781 +4078
Branches 16384 16858 +474
=============================================
+ Hits 55349 109034 +53685
+ Misses 79946 29076 -50870
- Partials 7408 8671 +1263
```
| [Flag](https://app.codecov.io/gh/apache/pulsar/pull/24613/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [inttests](https://app.codecov.io/gh/apache/pulsar/pull/24613/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.69% <0.00%> (-0.07%)` | :arrow_down: |
| [systests](https://app.codecov.io/gh/apache/pulsar/pull/24613/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `23.20% <0.00%> (-0.14%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pulsar/pull/24613/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.77% <100.00%> (+39.05%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files with missing lines](https://app.codecov.io/gh/apache/pulsar/pull/24613?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...java/org/apache/pulsar/client/impl/HttpClient.java](https://app.codecov.io/gh/apache/pulsar/pull/24613?src=pr&el=tree&filepath=pulsar-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fclient%2Fimpl%2FHttpClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL0h0dHBDbGllbnQuamF2YQ==) | `81.56% <100.00%> (+41.70%)` | :arrow_up: |
... and [1385 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/24613/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][ws] Fix WebSocket proxy originalPrincipal for HTTP admin API calls [pulsar]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #24613:
URL: https://github.com/apache/pulsar/pull/24613#issuecomment-3165663846
@codelipenghui Please select only one documentation label in your PR description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [fix][ws] Fix WebSocket proxy originalPrincipal for HTTP admin API calls [pulsar]
Posted by "codelipenghui (via GitHub)" <gi...@apache.org>.
codelipenghui merged PR #24613:
URL: https://github.com/apache/pulsar/pull/24613
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org