Allow configuring credential-cache for kerberized hive connector by Praveen2112 · Pull Request #13482 · trinodb/trino

Praveen2112 · 2022-08-03T14:55:03Z

Description

This allows us deploy hive connector in a keytab less environment where the credentials are fetched from credential cache file.

Is this change a fix, improvement, new feature, refactoring, or other?

New feature for Hive and iceberg connector.

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

This is specific to hive connector and iceberg connector.

How would you describe this change to a non-technical end user or system administrator?

This allows us deploy hive connector in a keytab less environment where the credentials are fetched from credential cache file.

Related issues, pull requests, and links

Documentation

( ) No documentation is needed.
( ) Sufficient documentation is included in this PR.
(x) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

# Section
* Allow using credential-cache for kerberized hive connector

wendigo

LGTM % comments

...plugin-toolkit/src/main/java/io/trino/plugin/base/authentication/KerberosAuthentication.java

plugin/trino-hive/src/main/java/io/trino/plugin/hive/authentication/HdfsKerberosConfig.java

kokosing

looks good

...plugin-toolkit/src/main/java/io/trino/plugin/base/authentication/KerberosAuthentication.java

kokosing · 2022-08-10T11:24:26Z

plugin/trino-hive/src/main/java/io/trino/plugin/hive/authentication/HdfsKerberosConfig.java

Can we use keytab together with cache? Do we need validation for this?

Using a keytab along with cache is also a valid configuration. So I don't think we need a verification for it. Maybe we can add an additional config to check if either of them are present.

Can we have a test for this then?

Additional PT environment for both keytab with cache ?

then we need to multiply the tests. Instead of copying tests I think adding more environment would give us better coverage.

You can make tests parametric

It is also kind of tricky - if we are testing for group configured_features then we need to make all the tests parametric but some of them would run in a non-kerberos environment. So we need to duplicate them for kerberos and non-kerberos environment - or introduce some dummy catalogs to non-keberos environment.

OK. So let's add new environment then.

...plugin-toolkit/src/main/java/io/trino/plugin/base/authentication/KerberosAuthentication.java

Praveen2112 · 2022-08-16T09:45:18Z

@wendigo / @kokosing / @s2lomon Have rebased it due to logical conflict. Addressed additional comments.

s2lomon

lgtm

s2lomon · 2022-08-16T13:20:28Z

lib/trino-hdfs/src/main/java/io/trino/hdfs/authentication/HdfsKerberosConfig.java

Wow, I've not known that we can do that. So it's either null, or path to existing file correct?

Yeah. And it applies check if the file exists.

...-plugin-toolkit/src/main/java/io/trino/plugin/base/authentication/KerberosConfiguration.java

...nment/singlenode-kerberos-kms-hdfs-no-impersonation-with-credential-cache/iceberg.properties

Praveen2112 · 2022-08-19T10:58:39Z

@wendigo / @kokosing Added tests and applied comments.

kokosing · 2022-08-22T10:36:08Z

CI is red

...src/main/java/io/trino/plugin/hive/metastore/thrift/ThriftMetastoreAuthenticationModule.java

kokosing · 2022-08-22T10:39:48Z

...gin-toolkit/src/test/java/io/trino/plugin/base/authentication/TestKerberosConfiguration.java

Use HOST_NAME in these values to make it clear what you tests

Praveen2112 · 2022-08-24T09:57:00Z

@wendigo / @kokosing / @s2lomon We made one change in KerberosConfiguration - We enable storeKey - if keyTab is configured and credential-cache is not configured. (As TGT is fetched from Credential Cache, then the KeyTab will not be loaded, and we would get a LoginException stating 'No key to store'.). Is this the correct approach or should we restrict keyTab or credentialCache cannot be configured at the same time.

kokosing · 2022-08-24T11:20:02Z

should we restrict keyTab or credentialCache cannot be configured at the same time.

I would start with this approach. It sounds that we could relax that in future when we learn about the use case. Configuring these two things is kind of misleading and I am not sure what user really wants.

wendigo · 2022-08-24T11:25:30Z

I agree with @kokosing on this

Hostname substitution happens even if the realm part is missing. Realm is optional, if there is no realm component in the principal, then it will be assumed that the principal is in the default realm.

Praveen2112 · 2022-08-25T09:31:45Z

@wendigo / @kokosing / @s2lomon AC

s2lomon

lgtm

...-plugin-toolkit/src/main/java/io/trino/plugin/base/authentication/KerberosConfiguration.java

colebow · 2022-08-29T18:02:32Z

@Praveen2112 the template suggests there's another PR with docs for this change, but I can't find it. Could you link?

Also, what section should the release note go into?

Praveen2112 · 2022-08-31T13:57:22Z

I'm working on the PR for docs.

The release notes should be part of Hive connector.

Praveen2112 requested review from kokosing, s2lomon and wendigo August 3, 2022 14:55

cla-bot bot added the cla-signed label Aug 3, 2022

github-actions bot added the tests:hive label Aug 3, 2022

Praveen2112 force-pushed the kerberos_credential_cache_hive branch from 2f86907 to 2cdea75 Compare August 4, 2022 05:58

wendigo approved these changes Aug 10, 2022

View reviewed changes

...plugin-toolkit/src/main/java/io/trino/plugin/base/authentication/KerberosAuthentication.java Outdated Show resolved Hide resolved

plugin/trino-hive/src/main/java/io/trino/plugin/hive/authentication/HdfsKerberosConfig.java Outdated Show resolved Hide resolved

kokosing reviewed Aug 10, 2022

View reviewed changes

...plugin-toolkit/src/main/java/io/trino/plugin/base/authentication/KerberosAuthentication.java Outdated Show resolved Hide resolved

kokosing reviewed Aug 10, 2022

View reviewed changes

Praveen2112 force-pushed the kerberos_credential_cache_hive branch 3 times, most recently from bde6ddd to 1ec7ef9 Compare August 16, 2022 09:43

Praveen2112 force-pushed the kerberos_credential_cache_hive branch from 1ec7ef9 to 3fb0190 Compare August 16, 2022 11:26

s2lomon approved these changes Aug 16, 2022

View reviewed changes

Praveen2112 mentioned this pull request Aug 17, 2022

Minor cleanup for Kerberized Hadoop KMS Environment. #13700

Merged

Praveen2112 force-pushed the kerberos_credential_cache_hive branch from 3fb0190 to 12ff76c Compare August 17, 2022 06:47

wendigo reviewed Aug 17, 2022

View reviewed changes

Praveen2112 force-pushed the kerberos_credential_cache_hive branch from 12ff76c to a794072 Compare August 18, 2022 12:32

Extract KerberosConfiguration from KerberosAuthentication

c497254

Praveen2112 force-pushed the kerberos_credential_cache_hive branch 2 times, most recently from 14c6238 to 252dbef Compare August 19, 2022 10:53

kokosing approved these changes Aug 22, 2022

View reviewed changes

Praveen2112 force-pushed the kerberos_credential_cache_hive branch from 252dbef to 58df151 Compare August 24, 2022 09:38

Praveen2112 force-pushed the kerberos_credential_cache_hive branch from 58df151 to 4671f70 Compare August 24, 2022 12:53

Make hostname substitution resilient

325039c

Hostname substitution happens even if the realm part is missing. Realm is optional, if there is no realm component in the principal, then it will be assumed that the principal is in the default realm.

Praveen2112 force-pushed the kerberos_credential_cache_hive branch from 4671f70 to 16c9650 Compare August 25, 2022 05:30

s2lomon approved these changes Aug 25, 2022

View reviewed changes

kokosing approved these changes Aug 25, 2022

View reviewed changes

...-plugin-toolkit/src/main/java/io/trino/plugin/base/authentication/KerberosConfiguration.java Outdated Show resolved Hide resolved

Allow configuring credential-cache for kerberized hive connector

adf88dc

Praveen2112 force-pushed the kerberos_credential_cache_hive branch from 16c9650 to adf88dc Compare August 25, 2022 14:48

Praveen2112 merged commit 5120f3f into trinodb:master Aug 29, 2022

github-actions bot added this to the 394 milestone Aug 29, 2022

colebow mentioned this pull request Aug 29, 2022

Add Trino 394 release notes #13813

Merged

Praveen2112 mentioned this pull request Apr 18, 2023

Support Kerberos Ticket Cache for trino authentication #17080

Closed

Praveen2112 mentioned this pull request Mar 4, 2024

Fix docs for hive-security.md #20923

Merged

Conversation

Praveen2112 commented Aug 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues, pull requests, and links

Documentation

Release notes

Uh oh!

wendigo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kokosing left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Praveen2112 commented Aug 16, 2022

Uh oh!

s2lomon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Praveen2112 commented Aug 19, 2022

Uh oh!

kokosing commented Aug 22, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Praveen2112 commented Aug 24, 2022

Uh oh!

kokosing commented Aug 24, 2022

Uh oh!

wendigo commented Aug 24, 2022

Uh oh!

Praveen2112 commented Aug 25, 2022

Uh oh!

s2lomon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

colebow commented Aug 29, 2022

Uh oh!

Praveen2112 commented Aug 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

Praveen2112 commented Aug 3, 2022 •

edited

Loading