You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@falcon.apache.org by sh...@apache.org on 2014/07/09 08:47:45 UTC

git commit: FALCON-497 Able to submit feed even though owner of storage specified (location type=data) is different from the ACL owner. Contributed by Shwetha GS

Repository: incubator-falcon
Updated Branches:
  refs/heads/master 5626b2dd1 -> 57953f77b


FALCON-497 Able to submit feed even though owner of storage specified (location type=data) is different from the ACL owner. Contributed by Shwetha GS


Project: http://git-wip-us.apache.org/repos/asf/incubator-falcon/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-falcon/commit/57953f77
Tree: http://git-wip-us.apache.org/repos/asf/incubator-falcon/tree/57953f77
Diff: http://git-wip-us.apache.org/repos/asf/incubator-falcon/diff/57953f77

Branch: refs/heads/master
Commit: 57953f77b03a68317b554619c2b17f8e9c623b2e
Parents: 5626b2d
Author: Shwetha GS <sh...@inmobi.com>
Authored: Wed Jul 9 12:17:33 2014 +0530
Committer: Shwetha GS <sh...@inmobi.com>
Committed: Wed Jul 9 12:17:33 2014 +0530

----------------------------------------------------------------------
 CHANGES.txt                                     |  3 ++
 .../java/org/apache/falcon/cli/FalconCLI.java   | 10 ++---
 .../apache/falcon/entity/FileSystemStorage.java | 17 +++++---
 .../falcon/entity/parser/FeedEntityParser.java  |  7 ++-
 .../falcon/entity/FileSystemStorageTest.java    | 46 ++++++++++++++------
 5 files changed, 55 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/57953f77/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 56c9ff6..4fd1cae 100755
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -20,6 +20,9 @@ Trunk (Unreleased)
   OPTIMIZATIONS
 
   BUG FIXES
+   FALCON-497 Able to submit feed even though owner of storage specified (location type=data) 
+   is different from the ACL owner. (Shwetha GS)
+
    FALCON-357 HCatalog Feed replication: Hive export job fails when table partition 
    contains multiple dated columns. (Satish Mittal via Shwetha GS)
 

http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/57953f77/client/src/main/java/org/apache/falcon/cli/FalconCLI.java
----------------------------------------------------------------------
diff --git a/client/src/main/java/org/apache/falcon/cli/FalconCLI.java b/client/src/main/java/org/apache/falcon/cli/FalconCLI.java
index 753c39f..0e60129 100644
--- a/client/src/main/java/org/apache/falcon/cli/FalconCLI.java
+++ b/client/src/main/java/org/apache/falcon/cli/FalconCLI.java
@@ -180,22 +180,22 @@ public class FalconCLI {
             }
 
             return exitValue;
-        } catch (FalconCLIException ex) {
-            ERR.get().println("Error: " + ex.getMessage());
-            return -1;
         } catch (ParseException ex) {
             ERR.get().println("Invalid sub-command: " + ex.getMessage());
             ERR.get().println();
             ERR.get().println(parser.shortHelp());
+            ERR.get().println("Stacktrace:");
+            ex.printStackTrace();
             return -1;
         } catch (ClientHandlerException ex) {
             ERR.get().print("Unable to connect to Falcon server, "
                     + "please check if the URL is correct and Falcon server is up and running\n");
-            ERR.get().println(ex.getMessage());
+            ERR.get().println("Stacktrace:");
+            ex.printStackTrace();
             return -1;
         } catch (Exception ex) {
+            ERR.get().println("Stacktrace:");
             ex.printStackTrace();
-            ERR.get().println(ex.getMessage());
             return -1;
         }
     }

http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/57953f77/common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java
----------------------------------------------------------------------
diff --git a/common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java b/common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java
index 9b76d3b..0d17f3d 100644
--- a/common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java
+++ b/common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java
@@ -19,6 +19,7 @@
 package org.apache.falcon.entity;
 
 import org.apache.falcon.FalconException;
+import org.apache.falcon.entity.common.FeedDataPath;
 import org.apache.falcon.entity.v0.feed.Feed;
 import org.apache.falcon.entity.v0.feed.Location;
 import org.apache.falcon.entity.v0.feed.LocationType;
@@ -32,10 +33,12 @@ import org.apache.hadoop.fs.Path;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.regex.Matcher;
 
 /**
  * A file system implementation of a feed storage.
@@ -251,9 +254,9 @@ public class FileSystemStorage implements Storage {
                     }
                 }
             }
-        } catch (Exception e) {
+        } catch (IOException e) {
             LOG.error("Can't validate ACL on storage {}", getStorageUrl(), e);
-            throw new FalconException("Can't validate storage ACL (URI " + getStorageUrl() + ")", e);
+            throw new RuntimeException("Can't validate storage ACL (URI " + getStorageUrl() + ")", e);
         }
     }
 
@@ -265,12 +268,12 @@ public class FileSystemStorage implements Storage {
 
     private String getRelativePath(Location location) {
         // if the path contains variables, locate on the "parent" path (just before first variable usage)
-        int index = location.getPath().indexOf(DOLLAR_EXPR_START_REGEX);
-        String pathString = location.getPath();
-        if (index != -1) {
-            pathString = pathString.substring(index);
+        Matcher matcher = FeedDataPath.PATTERN.matcher(location.getPath());
+        boolean timedPath = matcher.find();
+        if (timedPath) {
+            return location.getPath().substring(0, matcher.start());
         }
-        return pathString;
+        return location.getPath();
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/57953f77/common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java
----------------------------------------------------------------------
diff --git a/common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java b/common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java
index d5a8a76..e517e39 100644
--- a/common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java
+++ b/common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java
@@ -418,8 +418,11 @@ public class FeedEntityParser extends EntityParser<Feed> {
             }
 
             final Storage storage = FeedHelper.createStorage(cluster, feed);
-            storage.validateACL(feed.getACL().getOwner(), feed.getACL().getGroup(),
-                    feed.getACL().getPermission());
+            try {
+                storage.validateACL(feed.getACL().getOwner(), feed.getACL().getGroup(), feed.getACL().getPermission());
+            } catch(FalconException e) {
+                throw new ValidationException(e);
+            }
         }
     }
 }

http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/57953f77/common/src/test/java/org/apache/falcon/entity/FileSystemStorageTest.java
----------------------------------------------------------------------
diff --git a/common/src/test/java/org/apache/falcon/entity/FileSystemStorageTest.java b/common/src/test/java/org/apache/falcon/entity/FileSystemStorageTest.java
index 33e74fa..06f660d 100644
--- a/common/src/test/java/org/apache/falcon/entity/FileSystemStorageTest.java
+++ b/common/src/test/java/org/apache/falcon/entity/FileSystemStorageTest.java
@@ -18,9 +18,13 @@
 
 package org.apache.falcon.entity;
 
+import org.apache.falcon.FalconException;
+import org.apache.falcon.cluster.util.EmbeddedCluster;
 import org.apache.falcon.entity.v0.feed.Location;
 import org.apache.falcon.entity.v0.feed.LocationType;
 import org.apache.falcon.security.CurrentUser;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
 import org.testng.Assert;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.DataProvider;
@@ -69,7 +73,7 @@ public class FileSystemStorageTest {
 
         Assert.assertEquals("hdfs://localhost:8020", storage.getStorageUrl());
         Assert.assertEquals("hdfs://localhost:8020/data/YYYY/feed1/mmHH/dd/MM/${YEAR}-${MONTH}-${DAY}/more/${YEAR}",
-                storage.getUriTemplate(LocationType.DATA));
+            storage.getUriTemplate(LocationType.DATA));
         Assert.assertEquals("hdfs://localhost:8020/stats/YYYY/feed1/mmHH/dd/MM/${YEAR}-${MONTH}-${DAY}/more/${YEAR}",
                 storage.getUriTemplate(LocationType.STATS));
         Assert.assertEquals("hdfs://localhost:8020/meta/YYYY/feed1/mmHH/dd/MM/${YEAR}-${MONTH}-${DAY}/more/${YEAR}",
@@ -151,25 +155,39 @@ public class FileSystemStorageTest {
     @Test
     public void testValidateACL() throws Exception {
         final Location location = new Location();
-        location.setPath("/foo/bar");
+        Path path = new Path("/foo/bar");
+        location.setPath(path.toString());
         location.setType(LocationType.DATA);
         List<Location> locations = new ArrayList<Location>();
         locations.add(location);
 
-        FileSystemStorage storage = new FileSystemStorage("jail://global:00", locations);
-        storage.validateACL(USER, USER, "rrr");
-    }
+        String user = System.getProperty("user.name");
+        EmbeddedCluster cluster = EmbeddedCluster.newCluster(user);
+        FileSystem fs = cluster.getFileSystem();
+        fs.mkdirs(path);
 
-    @Test
-    public void testValidateACLWithTimeVariables() throws Exception {
-        final Location location = new Location();
-        location.setPath("/foo/bar/${YEAR}/${MONTH}/${DAY}");
-        location.setType(LocationType.DATA);
-        List<Location> locations = new ArrayList<Location>();
-        locations.add(location);
+        FileSystemStorage storage = new FileSystemStorage(cluster.getConf().get("fs.default.name"), locations);
+        storage.validateACL(user, user, "rrr");
 
-        FileSystemStorage storage = new FileSystemStorage("jail://global:00", locations);
-        storage.validateACL(USER, USER, "rrr");
+        //-ve case
+        try {
+            storage.validateACL("random", user, "rrr");
+            Assert.fail("Validation should have failed");
+        } catch(FalconException e) {
+            //expected exception
+        }
+
+        //Timed path
+        location.setPath("/foo/bar/${YEAR}/${MONTH}/${DAY}");
+        storage.validateACL(user, user, "rrr");
+
+        //-ve case
+        try {
+            storage.validateACL("random", user, "rrr");
+            Assert.fail("Validation should have failed");
+        } catch(FalconException e) {
+            //expected exception
+        }
     }
 
     @DataProvider(name = "locationTestWithRelativePathDataProvider")