You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "rtrivedi12 (via GitHub)" <gi...@apache.org> on 2025/03/11 13:48:28 UTC
[PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
rtrivedi12 opened a new pull request, #5689:
URL: https://github.com/apache/hive/pull/5689
…o explicit LOCATION
### What changes were proposed in this pull request?
This update ensures that DFS_URI authorization is triggered only when necessary during the CREATE TABLE command. Specifically:
For managed table creation, DFS_URI authorization is skipped if the location is under the default managed DB location.
For external table creation, DFS_URI authorization is skipped if the table location is under the default external DB location.
### Why are the changes needed?
This prevents unnecessary DFS_URI authorization for tables created within default DB locations, and adding extra policies in Ranger for query execution from external clients like Spark.
### Does this PR introduce _any_ user-facing change?
Yes, After this change, users will no longer need the DFS_URI policy for tables that are created in the default managed DB location (for managed tables) or the default external DB location (for external tables).
### Is the change a dependency upgrade?
No
### How was this patch tested?
Changes were verified by running the CREATE table command from Spark sql.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 merged PR #5689:
URL: https://github.com/apache/hive/pull/5689
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5689:
URL: https://github.com/apache/hive/pull/5689#issuecomment-2714858266
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5689) **Quality Gate passed**
Issues
 [5 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5689&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5689&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5689&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5689&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5689&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5689)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #5689:
URL: https://github.com/apache/hive/pull/5689#discussion_r2162532273
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -82,8 +100,19 @@ private List<HivePrivilegeObject> getOutputHObjs() {
ret.add(getHivePrivilegeObject(database));
ret.add(getHivePrivilegeObject(table));
- if (StringUtils.isNotEmpty(uri) && !TableType.EXTERNAL_TABLE.toString().equalsIgnoreCase(table.getTableType())) {
Review Comment:
You missed this condition in L#135
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #5689:
URL: https://github.com/apache/hive/pull/5689#discussion_r2021794312
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -76,20 +96,31 @@ private List<HivePrivilegeObject> getOutputHObjs() {
List<HivePrivilegeObject> ret = new ArrayList<>();
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
- Database database = event.getDatabase();
- String uri = getSdLocation(table.getSd());
+ Database database = event.getDatabase();
+ String uri = getSdLocation(table.getSd());
ret.add(getHivePrivilegeObject(database));
ret.add(getHivePrivilegeObject(table));
- if (StringUtils.isNotEmpty(uri) && !TableType.EXTERNAL_TABLE.toString().equalsIgnoreCase(table.getTableType())) {
- ret.add(new HivePrivilegeObject(HivePrivilegeObjectType.DFS_URI, null, uri));
+ if (StringUtils.isNotEmpty(uri)) {
+ boolean isExternalTable = table.getTableType().equalsIgnoreCase(TableType.EXTERNAL_TABLE.toString());
+ String expectedTablePath = null;
+ try {
+ expectedTablePath = wh.getDefaultTablePath(database, table.getTableName(), isExternalTable).toString();
+ } catch (MetaException e) {
+ LOG.warn("Got exception fetching Default table location for table " + table.getTableName(), e);
+ }
+
+ // Skip DFS_URI for external tables and if managed table location is under default db path
+ if (!isExternalTable) {
Review Comment:
curious, trying to understand: In getInputObj you didn't had this `!isExternalTable`, but here you have it, why?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "rtrivedi12 (via GitHub)" <gi...@apache.org>.
rtrivedi12 commented on code in PR #5689:
URL: https://github.com/apache/hive/pull/5689#discussion_r2162586118
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -82,8 +100,19 @@ private List<HivePrivilegeObject> getOutputHObjs() {
ret.add(getHivePrivilegeObject(database));
ret.add(getHivePrivilegeObject(table));
- if (StringUtils.isNotEmpty(uri) && !TableType.EXTERNAL_TABLE.toString().equalsIgnoreCase(table.getTableType())) {
Review Comment:
This condition is being checked at L#113
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "rtrivedi12 (via GitHub)" <gi...@apache.org>.
rtrivedi12 commented on PR #5689:
URL: https://github.com/apache/hive/pull/5689#issuecomment-3021645569
@saihemanth-cloudera / @nrg4878 Can you please help merge this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5689:
URL: https://github.com/apache/hive/pull/5689#issuecomment-3001587190
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5689) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5689&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5689&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5689&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5689&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5689&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5689)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #5689: HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n…
URL: https://github.com/apache/hive/pull/5689
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "rtrivedi12 (via GitHub)" <gi...@apache.org>.
rtrivedi12 commented on code in PR #5689:
URL: https://github.com/apache/hive/pull/5689#discussion_r2162582482
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -82,8 +100,19 @@ private List<HivePrivilegeObject> getOutputHObjs() {
ret.add(getHivePrivilegeObject(database));
ret.add(getHivePrivilegeObject(table));
- if (StringUtils.isNotEmpty(uri) && !TableType.EXTERNAL_TABLE.toString().equalsIgnoreCase(table.getTableType())) {
- ret.add(new HivePrivilegeObject(HivePrivilegeObjectType.DFS_URI, null, uri));
+ if (StringUtils.isNotEmpty(uri)) {
+ String expectedTablePath = null;
+ try {
+ expectedTablePath = preEventContext.getHandler().getWh().getDefaultTablePath(database, table).toString();
+ } catch (MetaException e) {
+ LOG.warn("Got exception fetching Default location for dbName: {} tableName: {} ", database.getName(),
+ table.getTableName(), e);
+ }
+
+ // Skip DFS_URI for external tables and if managed table location is under default db path
+ if (!MetaStoreUtils.isExternalTable(table) && this.needDFSUriAuth(uri, expectedTablePath)) {
+ ret.add(new HivePrivilegeObject(HivePrivilegeObjectType.DFS_URI, null, uri));
+ }
Review Comment:
Do you mean we should always send DFS_URI for authorization for output objects ? In that case, CREATE TABLE for managed table will still need DFS_URI policy for write access even when location is not specified.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "nrg4878 (via GitHub)" <gi...@apache.org>.
nrg4878 commented on PR #5689:
URL: https://github.com/apache/hive/pull/5689#issuecomment-2997901341
@saihemanth-cloudera Lets get this reviewed and merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5689:
URL: https://github.com/apache/hive/pull/5689#issuecomment-2998955197
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5689) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5689&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5689&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5689&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5689&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5689&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5689)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "rtrivedi12 (via GitHub)" <gi...@apache.org>.
rtrivedi12 commented on code in PR #5689:
URL: https://github.com/apache/hive/pull/5689#discussion_r2032277558
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -45,10 +47,13 @@ public class CreateTableEvent extends HiveMetaStoreAuthorizableEvent {
private static final Logger LOG = LoggerFactory.getLogger(CreateTableEvent.class);
private String COMMAND_STR = "create table";
+ private Warehouse wh;
public CreateTableEvent(PreEventContext preEventContext) {
- super(preEventContext);
+
+ super(preEventContext);
+ this.wh = preEventContext.getHandler().getWh();
Review Comment:
Initial thought was that it will be used multiple times but it definitely makes sense to not save it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on code in PR #5689:
URL: https://github.com/apache/hive/pull/5689#discussion_r2021774332
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -45,10 +47,13 @@ public class CreateTableEvent extends HiveMetaStoreAuthorizableEvent {
private static final Logger LOG = LoggerFactory.getLogger(CreateTableEvent.class);
private String COMMAND_STR = "create table";
+ private Warehouse wh;
public CreateTableEvent(PreEventContext preEventContext) {
- super(preEventContext);
+
+ super(preEventContext);
+ this.wh = preEventContext.getHandler().getWh();
Review Comment:
Why do we want to save it? It should be always available in `preEventContext`?
We can directly do below
```
preEventContext.getHandler().getWh().getDefaultTablePath(database, table.getTableName(), isExternalTable);
```
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -62,11 +67,26 @@ private List<HivePrivilegeObject> getInputHObjs() {
List<HivePrivilegeObject> ret = new ArrayList<>();
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
- String uri = getSdLocation(table.getSd());
+ Database database = event.getDatabase();
+ String uri = getSdLocation(table.getSd());
- if (StringUtils.isNotEmpty(uri)) {
+ if (StringUtils.isEmpty(uri)) {
+ return ret;
+ }
+
+ boolean isExternalTable = table.getTableType().equalsIgnoreCase(TableType.EXTERNAL_TABLE.toString());
Review Comment:
does this work?
```
MetaStoreUtils.isExternalTable(table);
```
maybe we can use this directly if it does rather than storing in the variable
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -62,11 +67,26 @@ private List<HivePrivilegeObject> getInputHObjs() {
List<HivePrivilegeObject> ret = new ArrayList<>();
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
- String uri = getSdLocation(table.getSd());
+ Database database = event.getDatabase();
+ String uri = getSdLocation(table.getSd());
- if (StringUtils.isNotEmpty(uri)) {
+ if (StringUtils.isEmpty(uri)) {
+ return ret;
+ }
+
+ boolean isExternalTable = table.getTableType().equalsIgnoreCase(TableType.EXTERNAL_TABLE.toString());
+ String expectedTablePath = null;
+ try {
+ expectedTablePath = wh.getDefaultTablePath(database, table.getTableName(), isExternalTable).toString();
Review Comment:
I think we don't need to getTableName & things like that
```
expectedTablePath = wh.getDefaultTablePath(database, table).toString();
```
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -62,11 +67,26 @@ private List<HivePrivilegeObject> getInputHObjs() {
List<HivePrivilegeObject> ret = new ArrayList<>();
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
- String uri = getSdLocation(table.getSd());
+ Database database = event.getDatabase();
+ String uri = getSdLocation(table.getSd());
Review Comment:
I think there was some unconventional spacing, but can we keep the same thing to maintain redability
```
List<HivePrivilegeObject> ret = new ArrayList<>();
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
Database database = event.getDatabase();
String uri = getSdLocation(table.getSd());
```
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -62,11 +67,26 @@ private List<HivePrivilegeObject> getInputHObjs() {
List<HivePrivilegeObject> ret = new ArrayList<>();
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
- String uri = getSdLocation(table.getSd());
+ Database database = event.getDatabase();
+ String uri = getSdLocation(table.getSd());
- if (StringUtils.isNotEmpty(uri)) {
+ if (StringUtils.isEmpty(uri)) {
+ return ret;
+ }
+
+ boolean isExternalTable = table.getTableType().equalsIgnoreCase(TableType.EXTERNAL_TABLE.toString());
+ String expectedTablePath = null;
+ try {
+ expectedTablePath = wh.getDefaultTablePath(database, table.getTableName(), isExternalTable).toString();
+ } catch (MetaException e) {
+ LOG.warn("Got exception fetching Default table location for table " + table.getTableName(), e);
Review Comment:
Can you use {} + add the database name as well
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -76,20 +96,31 @@ private List<HivePrivilegeObject> getOutputHObjs() {
List<HivePrivilegeObject> ret = new ArrayList<>();
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
- Database database = event.getDatabase();
- String uri = getSdLocation(table.getSd());
+ Database database = event.getDatabase();
+ String uri = getSdLocation(table.getSd());
Review Comment:
can you match the indentation with above lines
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -76,20 +96,31 @@ private List<HivePrivilegeObject> getOutputHObjs() {
List<HivePrivilegeObject> ret = new ArrayList<>();
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
- Database database = event.getDatabase();
- String uri = getSdLocation(table.getSd());
+ Database database = event.getDatabase();
+ String uri = getSdLocation(table.getSd());
ret.add(getHivePrivilegeObject(database));
ret.add(getHivePrivilegeObject(table));
- if (StringUtils.isNotEmpty(uri) && !TableType.EXTERNAL_TABLE.toString().equalsIgnoreCase(table.getTableType())) {
- ret.add(new HivePrivilegeObject(HivePrivilegeObjectType.DFS_URI, null, uri));
+ if (StringUtils.isNotEmpty(uri)) {
+ boolean isExternalTable = table.getTableType().equalsIgnoreCase(TableType.EXTERNAL_TABLE.toString());
+ String expectedTablePath = null;
+ try {
+ expectedTablePath = wh.getDefaultTablePath(database, table.getTableName(), isExternalTable).toString();
+ } catch (MetaException e) {
+ LOG.warn("Got exception fetching Default table location for table " + table.getTableName(), e);
+ }
+
+ // Skip DFS_URI for external tables and if managed table location is under default db path
+ if (!isExternalTable) {
Review Comment:
curious, trying to undersatand in getInputObj you didn't had this `!isExternalTable`, but here you have it, why?
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -76,20 +96,31 @@ private List<HivePrivilegeObject> getOutputHObjs() {
List<HivePrivilegeObject> ret = new ArrayList<>();
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
- Database database = event.getDatabase();
- String uri = getSdLocation(table.getSd());
+ Database database = event.getDatabase();
+ String uri = getSdLocation(table.getSd());
ret.add(getHivePrivilegeObject(database));
ret.add(getHivePrivilegeObject(table));
- if (StringUtils.isNotEmpty(uri) && !TableType.EXTERNAL_TABLE.toString().equalsIgnoreCase(table.getTableType())) {
- ret.add(new HivePrivilegeObject(HivePrivilegeObjectType.DFS_URI, null, uri));
+ if (StringUtils.isNotEmpty(uri)) {
+ boolean isExternalTable = table.getTableType().equalsIgnoreCase(TableType.EXTERNAL_TABLE.toString());
+ String expectedTablePath = null;
+ try {
+ expectedTablePath = wh.getDefaultTablePath(database, table.getTableName(), isExternalTable).toString();
+ } catch (MetaException e) {
+ LOG.warn("Got exception fetching Default table location for table " + table.getTableName(), e);
+ }
+
+ // Skip DFS_URI for external tables and if managed table location is under default db path
+ if (!isExternalTable) {
+ if (StringUtils.isEmpty(expectedTablePath) || !uri.equals(expectedTablePath)) {
Review Comment:
this part looks being duplicated, can you refactor into a method & return whether you need to add DFS_URI or not
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5689:
URL: https://github.com/apache/hive/pull/5689#issuecomment-2953298715
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "rtrivedi12 (via GitHub)" <gi...@apache.org>.
rtrivedi12 commented on PR #5689:
URL: https://github.com/apache/hive/pull/5689#issuecomment-2785067239
> Thanx @rtrivedi12, had a quick pass, dropped some questions/comments.
>
> One generic question, I couldn't find time to debug, does the HMS Translation layer kicks in before this or after. Just trying to see if things don't change post we made a decision here
Thanks @ayushtkn for review comments ! HMS translation kicks in before firing pre-event during get_table() call. So, we already have the table location set in the Storage descriptor at this point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "rtrivedi12 (via GitHub)" <gi...@apache.org>.
rtrivedi12 commented on code in PR #5689:
URL: https://github.com/apache/hive/pull/5689#discussion_r2032278719
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -62,11 +67,26 @@ private List<HivePrivilegeObject> getInputHObjs() {
List<HivePrivilegeObject> ret = new ArrayList<>();
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
- String uri = getSdLocation(table.getSd());
+ Database database = event.getDatabase();
+ String uri = getSdLocation(table.getSd());
- if (StringUtils.isNotEmpty(uri)) {
+ if (StringUtils.isEmpty(uri)) {
+ return ret;
+ }
+
+ boolean isExternalTable = table.getTableType().equalsIgnoreCase(TableType.EXTERNAL_TABLE.toString());
Review Comment:
Yes, it works and looks clean. Amended to use `MetaStoreUtils.isExternalTable(table);`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #5689:
URL: https://github.com/apache/hive/pull/5689#discussion_r2162674786
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -82,8 +100,19 @@ private List<HivePrivilegeObject> getOutputHObjs() {
ret.add(getHivePrivilegeObject(database));
ret.add(getHivePrivilegeObject(table));
- if (StringUtils.isNotEmpty(uri) && !TableType.EXTERNAL_TABLE.toString().equalsIgnoreCase(table.getTableType())) {
- ret.add(new HivePrivilegeObject(HivePrivilegeObjectType.DFS_URI, null, uri));
+ if (StringUtils.isNotEmpty(uri)) {
+ String expectedTablePath = null;
+ try {
+ expectedTablePath = preEventContext.getHandler().getWh().getDefaultTablePath(database, table).toString();
+ } catch (MetaException e) {
+ LOG.warn("Got exception fetching Default location for dbName: {} tableName: {} ", database.getName(),
+ table.getTableName(), e);
+ }
+
+ // Skip DFS_URI for external tables and if managed table location is under default db path
+ if (!MetaStoreUtils.isExternalTable(table) && this.needDFSUriAuth(uri, expectedTablePath)) {
Review Comment:
I would like to separate the 'fetching expectedTablePath' part into a separate method, as it is repetitive.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5689:
URL: https://github.com/apache/hive/pull/5689#issuecomment-2750228394
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5689) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5689&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5689&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5689&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5689&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5689&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5689)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "nrg4878 (via GitHub)" <gi...@apache.org>.
nrg4878 commented on PR #5689:
URL: https://github.com/apache/hive/pull/5689#issuecomment-2755469132
@saihemanth-cloudera @ayushtkn Could you please help review this PR? Thank you
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #5689:
URL: https://github.com/apache/hive/pull/5689#discussion_r2162673547
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -82,8 +100,19 @@ private List<HivePrivilegeObject> getOutputHObjs() {
ret.add(getHivePrivilegeObject(database));
ret.add(getHivePrivilegeObject(table));
- if (StringUtils.isNotEmpty(uri) && !TableType.EXTERNAL_TABLE.toString().equalsIgnoreCase(table.getTableType())) {
Review Comment:
No, I meant about StringUtils.isNotEmpty(uri) but it is already at L#103.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #5689:
URL: https://github.com/apache/hive/pull/5689#discussion_r2162528713
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -82,8 +100,19 @@ private List<HivePrivilegeObject> getOutputHObjs() {
ret.add(getHivePrivilegeObject(database));
ret.add(getHivePrivilegeObject(table));
- if (StringUtils.isNotEmpty(uri) && !TableType.EXTERNAL_TABLE.toString().equalsIgnoreCase(table.getTableType())) {
- ret.add(new HivePrivilegeObject(HivePrivilegeObjectType.DFS_URI, null, uri));
+ if (StringUtils.isNotEmpty(uri)) {
+ String expectedTablePath = null;
+ try {
+ expectedTablePath = preEventContext.getHandler().getWh().getDefaultTablePath(database, table).toString();
+ } catch (MetaException e) {
+ LOG.warn("Got exception fetching Default location for dbName: {} tableName: {} ", database.getName(),
+ table.getTableName(), e);
+ }
+
+ // Skip DFS_URI for external tables and if managed table location is under default db path
+ if (!MetaStoreUtils.isExternalTable(table) && this.needDFSUriAuth(uri, expectedTablePath)) {
+ ret.add(new HivePrivilegeObject(HivePrivilegeObjectType.DFS_URI, null, uri));
+ }
Review Comment:
Is this really required? I think having it in output objects is just fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5689:
URL: https://github.com/apache/hive/pull/5689#issuecomment-3006931406
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5689) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5689&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5689&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5689&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5689&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5689&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5689)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "rtrivedi12 (via GitHub)" <gi...@apache.org>.
rtrivedi12 commented on code in PR #5689:
URL: https://github.com/apache/hive/pull/5689#discussion_r2032287425
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -76,20 +96,31 @@ private List<HivePrivilegeObject> getOutputHObjs() {
List<HivePrivilegeObject> ret = new ArrayList<>();
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
- Database database = event.getDatabase();
- String uri = getSdLocation(table.getSd());
+ Database database = event.getDatabase();
+ String uri = getSdLocation(table.getSd());
ret.add(getHivePrivilegeObject(database));
ret.add(getHivePrivilegeObject(table));
- if (StringUtils.isNotEmpty(uri) && !TableType.EXTERNAL_TABLE.toString().equalsIgnoreCase(table.getTableType())) {
- ret.add(new HivePrivilegeObject(HivePrivilegeObjectType.DFS_URI, null, uri));
+ if (StringUtils.isNotEmpty(uri)) {
+ boolean isExternalTable = table.getTableType().equalsIgnoreCase(TableType.EXTERNAL_TABLE.toString());
+ String expectedTablePath = null;
+ try {
+ expectedTablePath = wh.getDefaultTablePath(database, table.getTableName(), isExternalTable).toString();
+ } catch (MetaException e) {
+ LOG.warn("Got exception fetching Default table location for table " + table.getTableName(), e);
+ }
+
+ // Skip DFS_URI for external tables and if managed table location is under default db path
+ if (!isExternalTable) {
Review Comment:
We decided to ease out WRITE permission for DFS_URI for external table creation through HIVE-27525. Thought was external clients like Spark can just create table to read data from a location.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "rtrivedi12 (via GitHub)" <gi...@apache.org>.
rtrivedi12 commented on code in PR #5689:
URL: https://github.com/apache/hive/pull/5689#discussion_r2032280359
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -76,20 +96,31 @@ private List<HivePrivilegeObject> getOutputHObjs() {
List<HivePrivilegeObject> ret = new ArrayList<>();
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
- Database database = event.getDatabase();
- String uri = getSdLocation(table.getSd());
+ Database database = event.getDatabase();
+ String uri = getSdLocation(table.getSd());
ret.add(getHivePrivilegeObject(database));
ret.add(getHivePrivilegeObject(table));
- if (StringUtils.isNotEmpty(uri) && !TableType.EXTERNAL_TABLE.toString().equalsIgnoreCase(table.getTableType())) {
- ret.add(new HivePrivilegeObject(HivePrivilegeObjectType.DFS_URI, null, uri));
+ if (StringUtils.isNotEmpty(uri)) {
+ boolean isExternalTable = table.getTableType().equalsIgnoreCase(TableType.EXTERNAL_TABLE.toString());
+ String expectedTablePath = null;
+ try {
+ expectedTablePath = wh.getDefaultTablePath(database, table.getTableName(), isExternalTable).toString();
+ } catch (MetaException e) {
+ LOG.warn("Got exception fetching Default table location for table " + table.getTableName(), e);
+ }
+
+ // Skip DFS_URI for external tables and if managed table location is under default db path
+ if (!isExternalTable) {
+ if (StringUtils.isEmpty(expectedTablePath) || !uri.equals(expectedTablePath)) {
Review Comment:
Did code changes to refactor into a method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
Re: [PR] HIVE-28736:Remove DFS_URI authorization for CREATE_TABLE event with n… [hive]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #5689:
URL: https://github.com/apache/hive/pull/5689#issuecomment-2785117599
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5689) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5689&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5689&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5689&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5689&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5689&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5689)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org