Skip to content

remove old Kerberos ticket after relogin to make Kudu authentication work correctly#14373

Merged
findepi merged 1 commit intotrinodb:masterfrom
xiacongling:kudu-krb-bugfix
Feb 8, 2023
Merged

remove old Kerberos ticket after relogin to make Kudu authentication work correctly#14373
findepi merged 1 commit intotrinodb:masterfrom
xiacongling:kudu-krb-bugfix

Conversation

@xiacongling
Copy link
Copy Markdown
Contributor

@xiacongling xiacongling commented Sep 29, 2022

Description

A bugfix to make authentication work correctly on a Kerberized Kudu cluster. Old Kerberos tickets may be used to make Kudu connections, it may lead to Kudu UnrecoverableException being raised when query Kudu's data even though relogin recently. Besides, next ticket refresh time is calculated based on the ticket's startTime and endTime, a wrong result will lead to frequent access to the KDC. Remove old tickets after relogin can fix the issue.

This PR fixes #14372
fixes #14441

Non-technical explanation

Remove old Kerberos ticket to avoid authentication check failures when access Kerberized Kudu clusters.

Release notes

( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# Section
* Fix a bug in Kudu Kerberos ticket management. ({issue}`14372`)

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Nov 8, 2022

Does this PR fix #14441?

@xiacongling
Copy link
Copy Markdown
Contributor Author

Does this PR fix #14441?

Yes, the error message in #14441 and #14372 are the same.

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Nov 8, 2022

Could you add a temporary commit for a stress test like #14393?

@grantatspothero
Copy link
Copy Markdown
Contributor

grantatspothero commented Nov 8, 2022

@ebyhr for the release notes, do any other connectors utilize KerberosAuthentication?

The bug is not kudu specific.

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Nov 10, 2022

@grantatspothero KerberosAuthentication.attemptLogin method is used only from Kudu as far as I confirmed.

try {
LoginContext loginContext = new LoginContext("", subject, null, configuration);
loginContext.login();
synchronized (subject.getPrivateCredentials()) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why on subject.getPrivateCredentials() and not on subject?
Why do we need synchronize at all?

please capture reasoning as a code comment

@grantatspothero
Copy link
Copy Markdown
Contributor

+1 on @findepi 's comment, but otherwise LGTM.

Thanks for adding a detailed description of the bug here

@findepi
Copy link
Copy Markdown
Member

findepi commented Feb 8, 2023

I am not convinced this is the right fix, but it seems to help with test flakiness (#15990). Therefore I am merging this.

@grantatspothero is following up with a better fix here #15997

cc @anusudarsan

@findepi findepi merged commit 9625430 into trinodb:master Feb 8, 2023
@github-actions github-actions bot added this to the 407 milestone Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

5 participants