Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix][client] Fix compatibility between kerberos and tls #23798

Merged
merged 6 commits into from
Jan 2, 2025

Conversation

nodece
Copy link
Member

@nodece nodece commented Dec 31, 2024

Motivation

When the pulsar-admin CLI uses the kerberos authentication with tls transport, we encounter the following issue:

$ bin/pulsar-admin tenants list
2024-12-31T15:35:56,308+0800 [main] INFO  org.apache.pulsar.client.impl.auth.AuthenticationSasl - JAAS loginContext is: PulsarClient.
2024-12-31T15:35:56,455+0800 [main] INFO  org.apache.pulsar.common.sasl.JAASCredentialsContainer - successfully logged in.
2024-12-31T15:35:56,459+0800 [pulsar-tgt-refresh-thread] INFO  org.apache.pulsar.common.sasl.TGTRefreshThread - TGT refresh thread started.
2024-12-31T15:35:56,463+0800 [pulsar-tgt-refresh-thread] INFO  org.apache.pulsar.common.sasl.TGTRefreshThread - Client principal is "client/localhost@PULSAR.COM".
2024-12-31T15:35:56,463+0800 [pulsar-tgt-refresh-thread] INFO  org.apache.pulsar.common.sasl.TGTRefreshThread - Server principal is "krbtgt/PULSAR.COM@PULSAR.COM".
2024-12-31T15:35:56,483+0800 [pulsar-tgt-refresh-thread] INFO  org.apache.pulsar.common.sasl.TGTRefreshThread - TGT valid starting at:        Tue Dec 31 15:35:56 CST 2024
2024-12-31T15:35:56,484+0800 [pulsar-tgt-refresh-thread] INFO  org.apache.pulsar.common.sasl.TGTRefreshThread - TGT expires:                  Wed Jan 01 15:35:56 CST 2025
2024-12-31T15:35:56,484+0800 [pulsar-tgt-refresh-thread] INFO  org.apache.pulsar.common.sasl.TGTRefreshThread - TGT refresh sleeping until: Wed Jan 01 11:38:40 CST 2025
class org.apache.pulsar.client.api.PulsarClientException$UnsupportedAuthenticationException: Method not implemented!

Authentication providers two methods to get the authentication data:

  • org.apache.pulsar.client.api.Authentication#getAuthData(java.lang.String)
    • For compatibility, which will call org.apache.pulsar.client.api.Authentication#getAuthData()
  • org.apache.pulsar.client.api.Authentication#getAuthData()

However, the client/admin/broker/proxy still calls the org.apache.pulsar.client.api.Authentication#getAuthData(), this breaks the kerberos authentication.

Modifications

  • Use org.apache.pulsar.client.impl.auth.AuthenticationSasl#getAuthData(java.lang.String) instead of org.apache.pulsar.client.api.Authentication#getAuthData().
  • Make org.apache.pulsar.client.api.Authentication#getAuthData() as deprecated.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 31, 2024
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece nodece self-assigned this Dec 31, 2024
@nodece nodece added type/bug The PR fixed a bug or issue reported a bug area/authn labels Dec 31, 2024
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@liudezhi2098 liudezhi2098 self-requested a review January 1, 2025 11:07
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece
Copy link
Member Author

nodece commented Jan 2, 2025

/pulsarbot rerun-failure-checks

@codecov-commenter
Copy link

codecov-commenter commented Jan 2, 2025

Codecov Report

Attention: Patch coverage is 73.61111% with 19 lines in your changes missing coverage. Please review.

Project coverage is 74.14%. Comparing base (bbc6224) to head (642217a).
Report is 825 commits behind head on master.

Files with missing lines Patch % Lines
...apache/pulsar/proxy/server/DirectProxyHandler.java 68.00% 8 Missing ⚠️
...e/pulsar/client/impl/PulsarChannelInitializer.java 75.00% 6 Missing and 1 partial ⚠️
.../java/org/apache/pulsar/client/cli/CmdConsume.java 0.00% 1 Missing ⚠️
.../java/org/apache/pulsar/client/cli/CmdProduce.java 0.00% 1 Missing ⚠️
...ain/java/org/apache/pulsar/client/cli/CmdRead.java 0.00% 1 Missing ⚠️
.../apache/pulsar/proxy/server/AdminProxyHandler.java 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23798      +/-   ##
============================================
+ Coverage     73.57%   74.14%   +0.57%     
+ Complexity    32624    31699     -925     
============================================
  Files          1877     1853      -24     
  Lines        139502   143405    +3903     
  Branches      15299    16280     +981     
============================================
+ Hits         102638   106329    +3691     
+ Misses        28908    28710     -198     
- Partials       7956     8366     +410     
Flag Coverage Δ
inttests 26.74% <51.38%> (+2.16%) ⬆️
systests 23.15% <12.50%> (-1.17%) ⬇️
unittests 73.67% <70.83%> (+0.82%) ⬆️

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

Files with missing lines Coverage Δ
...client/admin/internal/http/AsyncHttpConnector.java 85.60% <100.00%> (-0.82%) ⬇️
...a/org/apache/pulsar/client/api/Authentication.java 80.00% <ø> (ø)
...java/org/apache/pulsar/client/impl/HttpClient.java 80.91% <100.00%> (+2.72%) ⬆️
.../java/org/apache/pulsar/client/cli/CmdConsume.java 42.33% <0.00%> (+1.03%) ⬆️
.../java/org/apache/pulsar/client/cli/CmdProduce.java 52.71% <0.00%> (+0.24%) ⬆️
...ain/java/org/apache/pulsar/client/cli/CmdRead.java 42.14% <0.00%> (+1.00%) ⬆️
.../apache/pulsar/proxy/server/AdminProxyHandler.java 75.26% <90.00%> (+1.17%) ⬆️
...e/pulsar/client/impl/PulsarChannelInitializer.java 84.40% <75.00%> (-6.78%) ⬇️
...apache/pulsar/proxy/server/DirectProxyHandler.java 72.88% <68.00%> (-0.23%) ⬇️

... and 1013 files with indirect coverage changes

@nodece nodece merged commit fd45029 into apache:master Jan 2, 2025
52 checks passed
nodece added a commit that referenced this pull request Jan 2, 2025
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit fd45029)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/authn cherry-picked/branch-4.0 doc-not-needed Your PR changes do not impact docs release/4.0.2 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants