ayushtkn commented on code in PR #5561:
URL: https://github.com/apache/hadoop/pull/5561#discussion_r1211225704
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestHDFSConcat.java:
##########
@@ -564,4 +566,113 @@ public void testConcatOnSameFile() throws Exception {
assertEquals(1, dfs.getContentSummary(new Path(dir)).getFileCount());
}
+
+ /**
+ * Verifies concat with wrong user when dfs.permissions.enabled is false.
+ *
+ * @throws IOException
+ */
+ @Test
+ public void testConcatPermissionEnabled() throws IOException {
+ Configuration conf2 = new Configuration();
+ conf2.setLong(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, blockSize);
+ conf2.setBoolean(DFSConfigKeys.DFS_PERMISSIONS_ENABLED_KEY, true);
+ MiniDFSCluster cluster2 = new
MiniDFSCluster.Builder(conf2).numDataNodes(REPL_FACTOR).build();
+ cluster2.waitClusterUp();
+ DistributedFileSystem dfs2 = cluster2.getFileSystem();
+
+ String testPathDir = "/dir2";
+ Path dir = new Path(testPathDir);
+ dfs2.mkdirs(dir);
+ Path trg = new Path(testPathDir, "trg");
+ Path src = new Path(testPathDir, "src");
+ DFSTestUtil.createFile(dfs2, trg, blockSize, REPL_FACTOR, 1);
+ DFSTestUtil.createFile(dfs2, src, blockSize, REPL_FACTOR, 1);
+
+ // Check permissions with the wrong user when dfs.permissions.enabled is
true.
+ final UserGroupInformation user =
+ UserGroupInformation.createUserForTesting("theDoctor", new String[]
{"tardis"});
+ DistributedFileSystem hdfs1 = (DistributedFileSystem)
DFSTestUtil.getFileSystemAs(user, conf2);
+ try {
+ hdfs1.concat(trg, new Path[] {src});
+ fail("Permission exception expected");
+ } catch (IOException ie) {
+ LOG.info("Got expected exception for permissions:" +
ie.getLocalizedMessage());
+ }
+
+ dfs2.close();
+ cluster2.shutdownDataNodes();
+ cluster2.shutdown();
+
+ conf2.setBoolean(DFSConfigKeys.DFS_PERMISSIONS_ENABLED_KEY, false);
+ cluster2 = new
MiniDFSCluster.Builder(conf2).numDataNodes(REPL_FACTOR).build();
+ cluster2.waitClusterUp();
+ dfs2 = cluster2.getFileSystem();
+ dfs2.mkdirs(dir);
+ DFSTestUtil.createFile(dfs2, trg, blockSize, REPL_FACTOR, 1);
+ DFSTestUtil.createFile(dfs2, src, blockSize, REPL_FACTOR, 1);
+
+ // Check permissions with the wrong user when dfs.permissions.enabled is
false.
+ DistributedFileSystem hdfs2 = (DistributedFileSystem)
DFSTestUtil.getFileSystemAs(user, conf2);
+ try {
+ hdfs2.concat(trg, new Path[] {src});
+ } catch (IOException ie) {
+ fail("Got unexpected exception for permissions:" +
ie.getLocalizedMessage());
+ }
+ dfs2.close();
+ cluster2.shutdownDataNodes();
+ cluster2.shutdown();
+ }
+
+ /**
+ * Test permissions of Concat operation.
+ */
+ @Test
+ public void testConcatPermissions() throws IOException {
+ String testPathDir = "/dir";
+ Path dir = new Path(testPathDir);
+ dfs.mkdirs(dir);
+ dfs.setPermission(dir, new FsPermission((short) 0777));
+
+ Path dst = new Path(testPathDir, "dst");
+ Path src = new Path(testPathDir, "src");
+ DFSTestUtil.createFile(dfs, dst, blockSize, REPL_FACTOR, 1);
+
+ // Create a user who is not the owner of the file and try concat operation.
+ final UserGroupInformation user =
+ UserGroupInformation.createUserForTesting("theDoctor", new String[]
{"group"});
+ DistributedFileSystem dfs2 = (DistributedFileSystem)
DFSTestUtil.getFileSystemAs(user, conf);
+
+ // Test 1: User is not the owner of the file and has src & dst permission.
+ DFSTestUtil.createFile(dfs, src, blockSize, REPL_FACTOR, 1);
+ dfs.setPermission(dst, new FsPermission((short) 0777));
+ dfs.setPermission(src, new FsPermission((short) 0777));
+ try {
+ dfs2.concat(dst, new Path[] {src});
+ } catch (AccessControlException ace) {
+ fail("Got unexpected exception:" + ace.getLocalizedMessage());
+ }
Review Comment:
Use LambdaTestUtils.intercept, and for the below ones as well
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestHDFSConcat.java:
##########
@@ -564,4 +566,113 @@ public void testConcatOnSameFile() throws Exception {
assertEquals(1, dfs.getContentSummary(new Path(dir)).getFileCount());
}
+
+ /**
+ * Verifies concat with wrong user when dfs.permissions.enabled is false.
+ *
+ * @throws IOException
+ */
+ @Test
+ public void testConcatPermissionEnabled() throws IOException {
+ Configuration conf2 = new Configuration();
+ conf2.setLong(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, blockSize);
+ conf2.setBoolean(DFSConfigKeys.DFS_PERMISSIONS_ENABLED_KEY, true);
+ MiniDFSCluster cluster2 = new
MiniDFSCluster.Builder(conf2).numDataNodes(REPL_FACTOR).build();
+ cluster2.waitClusterUp();
+ DistributedFileSystem dfs2 = cluster2.getFileSystem();
+
+ String testPathDir = "/dir2";
+ Path dir = new Path(testPathDir);
+ dfs2.mkdirs(dir);
+ Path trg = new Path(testPathDir, "trg");
+ Path src = new Path(testPathDir, "src");
+ DFSTestUtil.createFile(dfs2, trg, blockSize, REPL_FACTOR, 1);
+ DFSTestUtil.createFile(dfs2, src, blockSize, REPL_FACTOR, 1);
+
+ // Check permissions with the wrong user when dfs.permissions.enabled is
true.
+ final UserGroupInformation user =
+ UserGroupInformation.createUserForTesting("theDoctor", new String[]
{"tardis"});
+ DistributedFileSystem hdfs1 = (DistributedFileSystem)
DFSTestUtil.getFileSystemAs(user, conf2);
+ try {
+ hdfs1.concat(trg, new Path[] {src});
+ fail("Permission exception expected");
+ } catch (IOException ie) {
+ LOG.info("Got expected exception for permissions:" +
ie.getLocalizedMessage());
+ }
+
+ dfs2.close();
+ cluster2.shutdownDataNodes();
+ cluster2.shutdown();
+
+ conf2.setBoolean(DFSConfigKeys.DFS_PERMISSIONS_ENABLED_KEY, false);
+ cluster2 = new
MiniDFSCluster.Builder(conf2).numDataNodes(REPL_FACTOR).build();
+ cluster2.waitClusterUp();
+ dfs2 = cluster2.getFileSystem();
+ dfs2.mkdirs(dir);
+ DFSTestUtil.createFile(dfs2, trg, blockSize, REPL_FACTOR, 1);
+ DFSTestUtil.createFile(dfs2, src, blockSize, REPL_FACTOR, 1);
+
+ // Check permissions with the wrong user when dfs.permissions.enabled is
false.
+ DistributedFileSystem hdfs2 = (DistributedFileSystem)
DFSTestUtil.getFileSystemAs(user, conf2);
+ try {
+ hdfs2.concat(trg, new Path[] {src});
+ } catch (IOException ie) {
+ fail("Got unexpected exception for permissions:" +
ie.getLocalizedMessage());
+ }
+ dfs2.close();
+ cluster2.shutdownDataNodes();
+ cluster2.shutdown();
Review Comment:
only ``cluster2.shutdown();`` is enough it would shutdown the datanode and
do the cleanup.
Moreover the shutdown should be in the ``finally`` block
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestHDFSConcat.java:
##########
@@ -564,4 +566,113 @@ public void testConcatOnSameFile() throws Exception {
assertEquals(1, dfs.getContentSummary(new Path(dir)).getFileCount());
}
+
+ /**
+ * Verifies concat with wrong user when dfs.permissions.enabled is false.
+ *
+ * @throws IOException
+ */
+ @Test
+ public void testConcatPermissionEnabled() throws IOException {
+ Configuration conf2 = new Configuration();
+ conf2.setLong(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, blockSize);
+ conf2.setBoolean(DFSConfigKeys.DFS_PERMISSIONS_ENABLED_KEY, true);
+ MiniDFSCluster cluster2 = new
MiniDFSCluster.Builder(conf2).numDataNodes(REPL_FACTOR).build();
+ cluster2.waitClusterUp();
+ DistributedFileSystem dfs2 = cluster2.getFileSystem();
+
+ String testPathDir = "/dir2";
+ Path dir = new Path(testPathDir);
+ dfs2.mkdirs(dir);
+ Path trg = new Path(testPathDir, "trg");
+ Path src = new Path(testPathDir, "src");
+ DFSTestUtil.createFile(dfs2, trg, blockSize, REPL_FACTOR, 1);
+ DFSTestUtil.createFile(dfs2, src, blockSize, REPL_FACTOR, 1);
+
+ // Check permissions with the wrong user when dfs.permissions.enabled is
true.
+ final UserGroupInformation user =
+ UserGroupInformation.createUserForTesting("theDoctor", new String[]
{"tardis"});
+ DistributedFileSystem hdfs1 = (DistributedFileSystem)
DFSTestUtil.getFileSystemAs(user, conf2);
+ try {
+ hdfs1.concat(trg, new Path[] {src});
+ fail("Permission exception expected");
+ } catch (IOException ie) {
+ LOG.info("Got expected exception for permissions:" +
ie.getLocalizedMessage());
+ }
Review Comment:
use LambdaTestUtils.intercept() for these try-catch-fail scenarios. do it
for the ones below as well
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
