deniskuzZ commented on code in PR #5975:
URL: https://github.com/apache/hive/pull/5975#discussion_r2222015429
##########
ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java:
##########
@@ -120,6 +121,8 @@ public void setUp() throws Exception {
hmsHandler = new HMSHandler("test", conf);
hmsHandler.init();
rawStore = new ObjectStore();
+ wh = new Warehouse(conf);
+ handler.wh = wh;
Review Comment:
@zxl-333 that doesn't even compile! use this
````
Subject: [PATCH] test
---
Index:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
(revision 67cb35e75297a9e6887a905ea92fb8c6a858b014)
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
(date 1753175396594)
@@ -311,6 +311,11 @@
@Override
public void init() throws MetaException {
+ init(new Warehouse(conf));
+ }
+
+ @VisibleForTesting
+ public void init(Warehouse wh) throws MetaException {
Metrics.initialize(conf);
initListeners = MetaStoreServerUtils.getMetaStoreListeners(
MetaStoreInitListener.class, conf, MetastoreConf.getVar(conf,
ConfVars.INIT_HOOKS));
@@ -322,7 +327,7 @@
String alterHandlerName = MetastoreConf.getVar(conf,
ConfVars.ALTER_HANDLER);
alterHandler = ReflectionUtils.newInstance(JavaUtils.getClass(
alterHandlerName, AlterHandler.class), conf);
- wh = new Warehouse(conf);
+ this.wh = wh;
synchronized (HMSHandler.class) {
if (currentUrl == null ||
!currentUrl.equals(MetaStoreInit.getConnectionURL(conf))) {
@@ -3047,11 +3052,13 @@
tableDataShouldBeDeleted = checkTableDataShouldBeDeleted(tbl,
deleteData);
if (tableDataShouldBeDeleted && tbl.getSd().getLocation() != null) {
tblPath = new Path(tbl.getSd().getLocation());
- if (!wh.isWritable(tblPath.getParent())) {
- String target = indexName == null ? "Table" : "Index table";
- throw new MetaException(target + " metadata not deleted since " +
- tblPath.getParent() + " is not writable by " +
- SecurityUtils.getUser());
+ String target = indexName == null ? "Table" : "Index table";
+ if (!wh.isWritable(tblPath.getParent())) {
+ throw new MetaException("%s metadata not deleted since %s is not
writable by %s"
+ .formatted(target, tblPath, SecurityUtils.getUser()));
+ } else if (!wh.isWritable(tblPath)) {
+ throw new MetaException("%s metadata not deleted since %s is not
writable by %s"
+ .formatted(target, tblPath, SecurityUtils.getUser()));
}
}
Index:
ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git
a/ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java
b/ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java
---
a/ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java
(revision 67cb35e75297a9e6887a905ea92fb8c6a858b014)
+++
b/ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java
(date 1753178967863)
@@ -23,6 +23,7 @@
import org.apache.commons.lang3.tuple.ImmutablePair;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hive.common.FileUtils;
import org.apache.hadoop.hive.conf.HiveConf;
import org.apache.hadoop.hive.metastore.ColumnType;
@@ -70,6 +71,7 @@
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
/*
Test whether HiveAuthorizer for MetaStore operation is trigger and
HiveMetaStoreAuthzInfo is created by HiveMetaStoreAuthorizer
@@ -94,6 +96,7 @@
private RawStore rawStore;
private Configuration conf;
private HMSHandler hmsHandler;
+ private Warehouse wh;
static HiveAuthorizer mockHiveAuthorizer;
static final List<String> allowedUsers = Arrays.asList("sam", "rob");
@@ -116,9 +119,10 @@
conf.set("hadoop.proxyuser.hive.users", "*");
MetaStoreTestUtils.setConfForStandloneMode(conf);
+ wh = Mockito.spy(new Warehouse(conf));
hmsHandler = new HMSHandler("test", conf);
- hmsHandler.init();
+ hmsHandler.init(wh);
rawStore = new ObjectStore();
rawStore.setConf(hmsHandler.getConf());
// Create the 'hive' catalog with new warehouse directory
@@ -386,7 +390,7 @@
UserGroupInformation.setLoginUser(UserGroupInformation.createRemoteUser(authorizedUser));
try {
Table table = new TableBuilder()
- .setTableName(tblName)
+ .setTableName("test_drop")
.addCol("name", ColumnType.STRING_TYPE_NAME)
.setOwner(authorizedUser)
.build(conf);
@@ -395,7 +399,7 @@
Table alteredTable = new TableBuilder()
.addCol("dep", ColumnType.STRING_TYPE_NAME)
.build(conf);
- hmsHandler.alter_table("default", tblName, alteredTable);
+ hmsHandler.alter_table("default", "test_drop", alteredTable);
} catch (Exception e) {
// No Exception for create table for authorized user
}
@@ -829,4 +833,29 @@
}
}
}
+
+ @Test
+ public void testDropTableNoTablePathWritePermissionShouldFail() throws
Exception {
+ UserGroupInformation.setLoginUser(
+ UserGroupInformation.createRemoteUser(authorizedUser));
+
+ Table table = new TableBuilder()
+ .setTableName(tblName)
+ .addCol("name", ColumnType.STRING_TYPE_NAME)
+ .setOwner(authorizedUser)
+ .build(conf);
+ hmsHandler.create_table(table);
+
+ Path tablePath = new Path(table.getSd().getLocation());
+ when(wh.isWritable(Mockito.eq(tablePath.getParent()))).thenReturn(true);
+ when(wh.isWritable(Mockito.eq(tablePath))).thenReturn(false);
+
+ try {
+ hmsHandler.drop_table("default", tblName, true);
+ } catch (MetaException e) {
+ String expected = "%s metadata not deleted since %s is not writable
by %s"
+ .formatted("Table", tablePath.toString(), authorizedUser);
+ assertEquals(expected, e.getMessage());
+ }
+ }
}
````
--
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-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
