Repository: falcon Updated Branches: refs/heads/0.6.1 813746b38 -> 051a58ca0
FALCON-1162 Cluster submit succeeds when staging HDFS dir does not have 777 (ALL) permission. Contributed by Venkat Ramachandran Project: http://git-wip-us.apache.org/repos/asf/falcon/repo Commit: http://git-wip-us.apache.org/repos/asf/falcon/commit/051a58ca Tree: http://git-wip-us.apache.org/repos/asf/falcon/tree/051a58ca Diff: http://git-wip-us.apache.org/repos/asf/falcon/diff/051a58ca Branch: refs/heads/0.6.1 Commit: 051a58ca0e4cc0caf7fe7a8dcf4016253208cd0e Parents: 813746b Author: Suhas Vasu <[email protected]> Authored: Thu Apr 30 17:39:12 2015 +0530 Committer: Suhas Vasu <[email protected]> Committed: Thu Apr 30 17:39:12 2015 +0530 ---------------------------------------------------------------------- CHANGES.txt | 3 ++ .../entity/parser/ClusterEntityParser.java | 30 ++++---------------- .../entity/parser/ClusterEntityParserTest.java | 26 ++++++++++++++++- docs/src/site/twiki/EntitySpecification.twiki | 2 +- 4 files changed, 35 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/falcon/blob/051a58ca/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 1d15300..dfeef51 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -140,6 +140,9 @@ Branch: 0.6.1 (Proposed Release Version: 0.6.1) (Suhas vasu) BUG FIXES + FALCON-1162 Cluster submit succeeds when staging HDFS dir does not + have 777 (ALL) permission (Venkat Ramachandran via Suhas Vasu) + FALCON-1161 Test case feedFeedBasePathExists fails intermittently (Suhas Vasu) http://git-wip-us.apache.org/repos/asf/falcon/blob/051a58ca/common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java b/common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java index 4eb3ea8..4555cb0 100644 --- a/common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java +++ b/common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java @@ -256,7 +256,7 @@ public class ClusterEntityParser extends EntityParser<Cluster> { } else { checkPathOwnerAndPermission(cluster.getName(), stagingLocation.getPath(), fs, - HadoopClientFactory.READ_EXECUTE_PERMISSION, false); + HadoopClientFactory.ALL_PERMISSION); if (!ClusterHelper.checkWorkingLocationExists(cluster)) { //Creating location type of working in the sub dir of staging dir with perms 755. FALCON-910 @@ -299,7 +299,7 @@ public class ClusterEntityParser extends EntityParser<Cluster> { } else { checkPathOwnerAndPermission(cluster.getName(), workingLocation.getPath(), fs, - HadoopClientFactory.READ_EXECUTE_PERMISSION, true); + HadoopClientFactory.READ_EXECUTE_PERMISSION); } } @@ -309,7 +309,7 @@ public class ClusterEntityParser extends EntityParser<Cluster> { } private void checkPathOwnerAndPermission(String clusterName, String location, FileSystem fs, - FsPermission expectedPermission, Boolean exactPerms) throws ValidationException { + FsPermission expectedPermission) throws ValidationException { Path locationPath = new Path(location); try { @@ -330,27 +330,9 @@ public class ClusterEntityParser extends EntityParser<Cluster> { } String errorMessage = "Path " + locationPath + " has permissions: " + fileStatus.getPermission().toString() + ", should be " + expectedPermission; - if (exactPerms) { - if (fileStatus.getPermission().toShort() != expectedPermission.toShort()) { - LOG.error(errorMessage); - throw new ValidationException(errorMessage); - } - } else { - if (expectedPermission.getUserAction().ordinal() > fileStatus.getPermission().getUserAction() - .ordinal()) { - LOG.error(errorMessage + " at least"); - throw new ValidationException(errorMessage + " at least"); - } - if (expectedPermission.getGroupAction().ordinal() > fileStatus.getPermission().getGroupAction() - .ordinal()) { - LOG.error(errorMessage + " at least"); - throw new ValidationException(errorMessage + " at least"); - } - if (expectedPermission.getOtherAction().ordinal() > fileStatus.getPermission().getOtherAction() - .ordinal()) { - LOG.error(errorMessage + " at least"); - throw new ValidationException(errorMessage + " at least"); - } + if (fileStatus.getPermission().toShort() != expectedPermission.toShort()) { + LOG.error(errorMessage); + throw new ValidationException(errorMessage); } // try to list to see if the user is able to write to this folder fs.listStatus(locationPath); http://git-wip-us.apache.org/repos/asf/falcon/blob/051a58ca/common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java ---------------------------------------------------------------------- diff --git a/common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java b/common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java index acc14a4..4920d03 100644 --- a/common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java +++ b/common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java @@ -286,7 +286,7 @@ public class ClusterEntityParserTest extends AbstractTestBase { Mockito.doNothing().when(clusterEntityParser).validateMessagingInterface(cluster); Mockito.doNothing().when(clusterEntityParser).validateRegistryInterface(cluster); this.dfsCluster.getFileSystem().mkdirs(new Path(ClusterHelper.getLocation(cluster, - ClusterLocationType.STAGING).getPath()), HadoopClientFactory.READ_EXECUTE_PERMISSION); + ClusterLocationType.STAGING).getPath()), HadoopClientFactory.ALL_PERMISSION); clusterEntityParser.validate(cluster); String workingDirPath = cluster.getLocations().getLocations().get(0).getPath() + "/working"; Assert.assertEquals(ClusterHelper.getLocation(cluster, ClusterLocationType.WORKING).getPath(), workingDirPath); @@ -320,6 +320,30 @@ public class ClusterEntityParserTest extends AbstractTestBase { Assert.fail("Should have thrown a validation exception"); } + /** + * A lightweight unit test for a cluster where staging location + * does not have ALL_PERMISSION (777). + * Staging has permission less than ALL_PERMISSION + * ValidationException should be thrown + * + * @throws ValidationException + */ + @Test(expectedExceptions = ValidationException.class) + public void testClusterWithStagingPermission() throws Exception { + ClusterEntityParser clusterEntityParser = Mockito + .spy((ClusterEntityParser) EntityParserFactory.getParser(EntityType.CLUSTER)); + Cluster cluster = (Cluster) this.dfsCluster.getCluster().copy(); + cluster.getLocations().getLocations().get(0).setPath("/projects/falcon/staging2"); + cluster.getLocations().getLocations().remove(1); + Mockito.doNothing().when(clusterEntityParser).validateWorkflowInterface(cluster); + Mockito.doNothing().when(clusterEntityParser).validateMessagingInterface(cluster); + Mockito.doNothing().when(clusterEntityParser).validateRegistryInterface(cluster); + this.dfsCluster.getFileSystem().mkdirs(new Path(ClusterHelper.getLocation(cluster, + ClusterLocationType.STAGING).getPath()), HadoopClientFactory.READ_EXECUTE_PERMISSION); + clusterEntityParser.validate(cluster); + Assert.fail("Should have thrown a validation exception"); + } + @BeforeClass public void init() throws Exception { this.dfsCluster = EmbeddedCluster.newCluster("testCluster"); http://git-wip-us.apache.org/repos/asf/falcon/blob/051a58ca/docs/src/site/twiki/EntitySpecification.twiki ---------------------------------------------------------------------- diff --git a/docs/src/site/twiki/EntitySpecification.twiki b/docs/src/site/twiki/EntitySpecification.twiki index 758d591..7656e48 100644 --- a/docs/src/site/twiki/EntitySpecification.twiki +++ b/docs/src/site/twiki/EntitySpecification.twiki @@ -70,7 +70,7 @@ Path is the hdfs path for each location. Falcon would use the location to do intermediate processing of entities in hdfs and hence Falcon should have read/write/execute permission on these locations. These locations MUST be created prior to submitting a cluster entity to Falcon. -*staging* should have atleast 755 permissions and is a mandatory location .The parent dirs must have execute permissions so multiple +*staging* should have 777 permissions and is a mandatory location .The parent dirs must have execute permissions so multiple users can write to this location. *working* must have 755 permissions and is a optional location. If *working* is not specified, falcon creates a sub directory in the *staging* location with 755 perms. The parent dir for *working* must have execute permissions so multiple
