You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@falcon.apache.org by so...@apache.org on 2015/04/29 23:35:01 UTC

falcon git commit: FALCON-1162 Cluster submit succeeds when staging HDFS dir does not have 777 (ALL) permission. Contributed by Venkat Ramachandran

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 <sr...@hortonworks.com>
Authored: Wed Apr 29 14:34:55 2015 -0700
Committer: Sowmya Ramesh <sr...@hortonworks.com>
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