Skip to content

Remove old expired kerberos tickets before relogging in#15997

Closed
grantatspothero wants to merge 1 commit intotrinodb:masterfrom
grantatspothero:gn/fixKerberosAuthKudu
Closed

Remove old expired kerberos tickets before relogging in#15997
grantatspothero wants to merge 1 commit intotrinodb:masterfrom
grantatspothero:gn/fixKerberosAuthKudu

Conversation

@grantatspothero
Copy link
Copy Markdown
Contributor

@grantatspothero grantatspothero commented Feb 6, 2023

Description

This ensures old Kerberos tickets are removed before logging in again.

Approach is similar to this PR: #14373

but instead uses the LoginContext#logout method, which internally does similar logic to subject.getPrivateCredentials().clear()

Additional context and related issues

Stress test PR is here: https://github.com/trinodb/trino/pull/16018/files

Fixes: #14372
Fixes: #14441

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`)

@cla-bot cla-bot bot added the cla-signed label Feb 6, 2023
@grantatspothero grantatspothero changed the title fix(kudu): perform logout before login when reauthenticating to kerberos fix(kudu): remove old expired kerberos tickets before relogging in Feb 6, 2023
@kokosing kokosing requested a review from Praveen2112 February 7, 2023 10:14
@findepi
Copy link
Copy Markdown
Member

findepi commented Feb 8, 2023

#14373 got merged, please rebase
you may want to revert that change if you think the changes here made that one obsolete.

@grantatspothero
Copy link
Copy Markdown
Contributor Author

@Praveen2112 @findepi @anusudarsan mind taking another look? Stress tests are green.

@Praveen2112
Copy link
Copy Markdown
Member

@grantatspothero Can you please reword the commit Remove old expired kerberos tickets before relogging in and squash them.

@grantatspothero
Copy link
Copy Markdown
Contributor Author

Remove old expired kerberos tickets before relogging in

Was away on vacation 🌴, just fixed @Praveen2112

@grantatspothero
Copy link
Copy Markdown
Contributor Author

@Praveen2112 can you take another look?

Copy link
Copy Markdown
Member

@Praveen2112 Praveen2112 left a comment

Choose a reason for hiding this comment

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

LGTM. Can we reword the commit message to Remove old expired kerberos tickets before relogging in - as the fix is not only for Kudu and even for other connectors.

@grantatspothero grantatspothero force-pushed the gn/fixKerberosAuthKudu branch from 9510fba to 68db1e2 Compare March 3, 2023 20:00
@grantatspothero
Copy link
Copy Markdown
Contributor Author

@Praveen2112 fixed.

@grantatspothero grantatspothero changed the title fix(kudu): remove old expired kerberos tickets before relogging in fix: remove old expired kerberos tickets before relogging in Mar 3, 2023
@Praveen2112 Praveen2112 changed the title fix: remove old expired kerberos tickets before relogging in Remove old expired kerberos tickets before relogging in Mar 6, 2023
@grantatspothero
Copy link
Copy Markdown
Contributor Author

@Praveen2112 some unrelated tests flaked, it looks like I do not have permission to rerun just the failed modules. Can you do that?

@kokosing
Copy link
Copy Markdown
Member

kokosing commented Mar 8, 2023

CI hit: #16013

@github-actions
Copy link
Copy Markdown

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

@github-actions github-actions bot added the stale label Jan 17, 2024
@mosabua
Copy link
Copy Markdown
Member

mosabua commented Jan 17, 2024

👋 @grantatspothero - this PR has become inactive. We hope you are still interested in working on it. Please let us know, and we can try to get reviewers to help with that.

We're working on closing out old and inactive PRs, so if you're too busy or this has too many merge conflicts to be worth picking back up, we'll be making another pass to close it out in a few weeks.

@github-actions github-actions bot removed the stale label Jan 18, 2024
@github-actions
Copy link
Copy Markdown

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

@github-actions github-actions bot added the stale label Feb 12, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 5, 2024

Closing this pull request, as it has been stale for six weeks. Feel free to re-open at any time.

@github-actions github-actions bot closed this Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

6 participants