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

HIVE-20001: With doas set to true, running select query as hrt_qa use… #389

Closed
wants to merge 1 commit into from

Conversation

beltran
Copy link
Contributor

@beltran beltran commented Jul 2, 2018

…r on external table fails due to permission denied to read /warehouse/tablespace/managed directory

…r on external table fails due to permission denied to read /warehouse/tablespace/managed directory
@beltran beltran closed this Jul 5, 2018
@beltran beltran deleted the HIVE-20001-3 branch July 5, 2018 18:54
@@ -138,7 +138,11 @@ public void authorize(Privilege[] readRequiredPriv, Privilege[] writeRequiredPri
@Override
public void authorize(Database db, Privilege[] readRequiredPriv, Privilege[] writeRequiredPriv)
throws HiveException, AuthorizationException {
Path path = getDbLocation(db);
Path path;
path = wh.determineDatabaseExternalPath(db);
Copy link

Choose a reason for hiding this comment

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

Thought the logic here was supposed to be getDefaultExternalDatabasePath(), and falling back to getDatabasePath() if the external DB path did not actually exist on the filesystem. This seems to be doing something different.

Copy link
Contributor Author

@beltran beltran Jul 5, 2018

Choose a reason for hiding this comment

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

getDefaultExternalDatabasePath is never null and may return the directory inside the managed space which may not be the location of the database. My understanding was that if an external folder hasn't been created for the database we should default to the location of the database.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually at this point we should check not only if !=null but also if it exists or default to the db location

@@ -1,5 +1,5 @@
--! qt:dataset:src
set hive.metastore.warehouse.dir=invalid_scheme://${system:test.tmp.dir};
-- set hive.metastore.warehouse.dir=invalid_scheme://${system:test.tmp.dir};
Copy link

Choose a reason for hiding this comment

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

Why is this commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In a previous patch it had to be commented because we were defaulting to hive.metastore.warehouse.dir if the external warehouse directory so this doesn't have to be commented any more. I'll remove the comment.

@@ -201,6 +201,48 @@ public Path determineDatabasePath(Catalog cat, Database db) throws MetaException
}
}

/**
Copy link

Choose a reason for hiding this comment

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

Given the changes from StorageBasedAuthorizationProvider.java, the changes for Warehouse.java may not be necessary.

if (wh.hasExternalWarehouseRoot() &&
wh.isDatabaseLocationDefault(db)) {
try {
madeExternalDir = UserGroupInformation.getCurrentUser().doAs(
Copy link

Choose a reason for hiding this comment

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

@thejasmn, in the case of remote HiveMetastore, is UserGroupInformation.getCurrentUser() set to the user that issued the request to the Metastore, or is it still the "hive" user?

@beltran
Copy link
Contributor Author

beltran commented Jul 5, 2018

@jdere I've closed this PR and opened #391 but I think everything you said here applies to the new one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants