Repository: falcon Updated Branches: refs/heads/master 728eb36c0 -> bbd01241c
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/bbd01241 Tree: http://git-wip-us.apache.org/repos/asf/falcon/tree/bbd01241 Diff: http://git-wip-us.apache.org/repos/asf/falcon/diff/bbd01241 Branch: refs/heads/master Commit: bbd01241c7999b4c0362160c32659ea18a97df7b Parents: 728eb36 Author: Sowmya Ramesh <[email protected]> Authored: Wed Apr 29 14:34:55 2015 -0700 Committer: Sowmya Ramesh <[email protected]> Committed: Wed Apr 29 14:34:55 2015 -0700 ---------------------------------------------------------------------- .../entity/parser/ClusterEntityParser.java | 30 ++++---------------- .../entity/parser/ClusterEntityParserTest.java | 26 ++++++++++++++++- docs/src/site/twiki/EntitySpecification.twiki | 2 +- 3 files changed, 32 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/falcon/blob/bbd01241/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/bbd01241/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/bbd01241/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
