falcon-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From suh...@apache.org
Subject falcon git commit: FALCON-1162 Cluster submit succeeds when staging HDFS dir does not have 777 (ALL) permission. Contributed by Venkat Ramachandran
Date Thu, 30 Apr 2015 12:09:48 GMT
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 <suhas.v@inmobi.com>
Authored: Thu Apr 30 17:39:12 2015 +0530
Committer: Suhas Vasu <suhas.v@inmobi.com>
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


Mime
View raw message