Hive Meta Store impersonation access

[BlueStalker]

This commits squash the original commits from PR
#13699
which includes the follow commits:

HMS impersonation access
refactoring to use HMS Authentication Module
add Config for multiple hms instances
Update HMS memory settings
address review comments

== RELEASE NOTES ==

General Changes
* N/A

Hive Changes
* add hive metastore impersonation access
* add multiple hive metastore instances support and metrics
* Code refactoring

If release note is NOT required, use:

== NO RELEASE NOTE ==

[BlueStalker]

@arhimondr This comes from #13699 and let's make another round of code reviews. Thanks. Also, FYI, @zhenxiao

[arhimondr]

@BlueStalker Thank you, will start reviewing it soon.

[arhimondr]

Just skimmed through. Some high level comments.

I don't see any changes to the CachingHiveMetastore. How is this supposed to work?

Let's assume the first call to getTable is done by user Alice that is authorized to get the table. The response is cached by the CachingHiveMetastore. Then the user Bob calls getTable, and the table information is returned from the CachingHiveMetastore without any security checks. This is a potential security risk.

[arhimondr]

We don't use ApacheCommons in Presto. Please use Guava instead.

[arhimondr]

In what cases we fallback to the default user? It feels like if impersonation is enabled we should never contact the metastore with "default".

[BlueStalker]

MetastoreHiveStatisticsProvider#getPartitionsStatistics, it gets the paritition stats.

[arhimondr]

These changes should go directly to the docker containers. Here is the repository: https://github.com/prestodb/docker-images

[BlueStalker]

Hi, Andrii, Sorry for late reply.
Actually I am wondering how the CachingHiveMetastore works here. I saw in HiveMetadataFactory, it delegates SemiTransactionalHiveMetastore into "CachingHiveMetastore.memoizeMetastore(this.metastore, perTransactionCacheMaximumSize)", the cache is per transaction scope if I understand correctly? Anywhere else we are using this cache globally?

Just skimmed through. Some high level comments.

I don't see any changes to the CachingHiveMetastore. How is this supposed to work?

Let's assume the first call to getTable is done by user Alice that is authorized to get the table. The response is cached by the CachingHiveMetastore. Then the user Bob calls getTable, and the table information is returned from the CachingHiveMetastore without any security checks. This is a potential security risk.

[arhimondr]

@BlueStalker The global metastore cache can be enabled with https://github.com/prestodb/presto/blob/master/presto-hive-metastore/src/main/java/com/facebook/presto/hive/MetastoreClientConfig.java#L35.

Also It doesn't feel right to wrap with doAs at the SemiTransactionalHiveMetastore level. The doAs will only work for ThriftHiveMetastoreClient.java. Wrapping it with doAs actually breaks encapsulation. Other metastore implementation may have their own means of providing user. It feels like the information about identity has to be passed all the way down to the ThriftHiveMetastoreClient, and then the ThriftHiveMetastoreClient can implement authentication by applying doAs, as this is very ThriftHiveMetastoreClient specific.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. If you'd still like this PR merged, please comment on the task, make sure you've addressed reviewer comments, and rebase on the latest master. Thank you for your contributions!