Return-Path: X-Original-To: apmail-falcon-commits-archive@minotaur.apache.org Delivered-To: apmail-falcon-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id DE80217989 for ; Mon, 9 Mar 2015 07:40:44 +0000 (UTC) Received: (qmail 85050 invoked by uid 500); 9 Mar 2015 07:40:44 -0000 Delivered-To: apmail-falcon-commits-archive@falcon.apache.org Received: (qmail 85017 invoked by uid 500); 9 Mar 2015 07:40:44 -0000 Mailing-List: contact commits-help@falcon.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@falcon.apache.org Delivered-To: mailing list commits@falcon.apache.org Received: (qmail 85008 invoked by uid 99); 9 Mar 2015 07:40:44 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 09 Mar 2015 07:40:44 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 44A1FE0613; Mon, 9 Mar 2015 07:40:44 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: suhasv@apache.org To: commits@falcon.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: falcon git commit: FALCON-1063 Falcon CLI list entities operation throws NullPointerException. Contributed by Pallavi Rao Date: Mon, 9 Mar 2015 07:40:44 +0000 (UTC) Repository: falcon Updated Branches: refs/heads/master e9e4ba4b6 -> c790761e0 FALCON-1063 Falcon CLI list entities operation throws NullPointerException. Contributed by Pallavi Rao Project: http://git-wip-us.apache.org/repos/asf/falcon/repo Commit: http://git-wip-us.apache.org/repos/asf/falcon/commit/c790761e Tree: http://git-wip-us.apache.org/repos/asf/falcon/tree/c790761e Diff: http://git-wip-us.apache.org/repos/asf/falcon/diff/c790761e Branch: refs/heads/master Commit: c790761e0492dbc2f0c59a91ab69f5ff35761a3f Parents: e9e4ba4 Author: Suhas Vasu Authored: Mon Mar 9 13:10:25 2015 +0530 Committer: Suhas Vasu Committed: Mon Mar 9 13:10:25 2015 +0530 ---------------------------------------------------------------------- FALCON-1063.patch | 204 +++++++++++++++++++ .../falcon/cleanup/AbstractCleanupHandler.java | 6 +- .../apache/falcon/security/SecurityUtil.java | 14 ++ .../falcon/cleanup/LogCleanupServiceTest.java | 10 + .../falcon/security/SecurityUtilTest.java | 28 +++ .../falcon/resource/AbstractEntityManager.java | 14 +- .../security/FalconAuthorizationFilter.java | 9 +- 7 files changed, 262 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/falcon/blob/c790761e/FALCON-1063.patch ---------------------------------------------------------------------- diff --git a/FALCON-1063.patch b/FALCON-1063.patch new file mode 100644 index 0000000..5aea017 --- /dev/null +++ b/FALCON-1063.patch @@ -0,0 +1,204 @@ +diff --git a/common/src/main/java/org/apache/falcon/cleanup/AbstractCleanupHandler.java b/common/src/main/java/org/apache/falcon/cleanup/AbstractCleanupHandler.java +index 5a4dce6..85d7263 100644 +--- a/common/src/main/java/org/apache/falcon/cleanup/AbstractCleanupHandler.java ++++ b/common/src/main/java/org/apache/falcon/cleanup/AbstractCleanupHandler.java +@@ -116,11 +116,11 @@ public abstract class AbstractCleanupHandler { + Entity entity) throws FalconException { + try { + final AccessControlList acl = entity.getACL(); +- if (acl == null) { +- throw new FalconException("ACL for entity " + entity.getName() + " is empty"); ++ // To support backward compatibility, will only use the ACL owner only if present ++ if (acl != null) { ++ CurrentUser.authenticate(acl.getOwner()); // proxy user + } + +- CurrentUser.authenticate(acl.getOwner()); // proxy user + return HadoopClientFactory.get().createProxiedFileSystem( + ClusterHelper.getConfiguration(cluster)); + } catch (Exception e) { +diff --git a/common/src/main/java/org/apache/falcon/security/SecurityUtil.java b/common/src/main/java/org/apache/falcon/security/SecurityUtil.java +index b9fd37e..861f80f 100644 +--- a/common/src/main/java/org/apache/falcon/security/SecurityUtil.java ++++ b/common/src/main/java/org/apache/falcon/security/SecurityUtil.java +@@ -19,11 +19,13 @@ + package org.apache.falcon.security; + + import org.apache.falcon.FalconException; ++import org.apache.falcon.entity.v0.Entity; + import org.apache.falcon.util.ReflectionUtils; + import org.apache.falcon.util.StartupProperties; + import org.apache.hadoop.security.authentication.server.KerberosAuthenticationHandler; + import org.apache.hadoop.security.authentication.server.PseudoAuthenticationHandler; + ++import java.io.IOException; + import java.net.InetAddress; + import java.net.UnknownHostException; + +@@ -104,4 +106,16 @@ public final class SecurityUtil { + "org.apache.falcon.security.DefaultAuthorizationProvider"); + return ReflectionUtils.getInstanceByClassName(providerClassName); + } ++ ++ public static void tryProxy(Entity entity) throws IOException, FalconException { ++ if (entity != null && entity.getACL() != null && SecurityUtil.isAuthorizationEnabled()) { ++ final String aclOwner = entity.getACL().getOwner(); ++ final String aclGroup = entity.getACL().getGroup(); ++ ++ if (SecurityUtil.getAuthorizationProvider().shouldProxy( ++ CurrentUser.getAuthenticatedUGI(), aclOwner, aclGroup)) { ++ CurrentUser.proxy(aclOwner, aclGroup); ++ } ++ } ++ } + } +diff --git a/common/src/test/java/org/apache/falcon/cleanup/LogCleanupServiceTest.java b/common/src/test/java/org/apache/falcon/cleanup/LogCleanupServiceTest.java +index 8e2e544..0df59b2 100644 +--- a/common/src/test/java/org/apache/falcon/cleanup/LogCleanupServiceTest.java ++++ b/common/src/test/java/org/apache/falcon/cleanup/LogCleanupServiceTest.java +@@ -55,6 +55,8 @@ public class LogCleanupServiceTest extends AbstractTestBase { + + "sample2" + "/logs/job-2010-01-01-01-00/000"); + private final Path instanceLogPath4 = new Path("/projects/falcon/staging/falcon/workflows/process/" + + "sample" + "/logs/latedata/2010-01-01-01-00"); ++ private final Path instanceLogPath5 = new Path("/projects/falcon/staging/falcon/workflows/process/" ++ + "sample3" + "/logs/job-2010-01-01-01-00/000"); + private final Path feedInstanceLogPath = new Path("/projects/falcon/staging/falcon/workflows/feed/" + + "impressionFeed" + "/logs/job-2010-01-01-01-00/testCluster/000"); + private final Path feedInstanceLogPath1 = new Path("/projects/falcon/staging/falcon/workflows/feed/" +@@ -90,15 +92,22 @@ public class LogCleanupServiceTest extends AbstractTestBase { + Process otherProcess = (Process) process.copy(); + otherProcess.setName("sample2"); + otherProcess.setFrequency(new Frequency("days(1)")); ++ Process noACLProcess = (Process) process.copy(); ++ noACLProcess.setName("sample3"); ++ noACLProcess.setACL(null); + ConfigurationStore.get().remove(EntityType.PROCESS, + otherProcess.getName()); + ConfigurationStore.get().publish(EntityType.PROCESS, otherProcess); ++ ConfigurationStore.get().remove(EntityType.PROCESS, ++ noACLProcess.getName()); ++ ConfigurationStore.get().publish(EntityType.PROCESS, noACLProcess); + + fs.mkdirs(instanceLogPath); + fs.mkdirs(instanceLogPath1); + fs.mkdirs(instanceLogPath2); + fs.mkdirs(instanceLogPath3); + fs.mkdirs(instanceLogPath4); ++ fs.mkdirs(instanceLogPath5); + + // fs.setTimes wont work on dirs + fs.createNewFile(new Path(instanceLogPath, "oozie.log")); +@@ -138,6 +147,7 @@ public class LogCleanupServiceTest extends AbstractTestBase { + Assert.assertFalse(fs.exists(instanceLogPath)); + Assert.assertFalse(fs.exists(instanceLogPath1)); + Assert.assertFalse(fs.exists(instanceLogPath2)); ++ Assert.assertFalse(fs.exists(instanceLogPath5)); + Assert.assertTrue(fs.exists(instanceLogPath3)); + } + +diff --git a/common/src/test/java/org/apache/falcon/security/SecurityUtilTest.java b/common/src/test/java/org/apache/falcon/security/SecurityUtilTest.java +index a36d916..7f9b405 100644 +--- a/common/src/test/java/org/apache/falcon/security/SecurityUtilTest.java ++++ b/common/src/test/java/org/apache/falcon/security/SecurityUtilTest.java +@@ -18,10 +18,17 @@ + + package org.apache.falcon.security; + ++ ++import org.apache.falcon.FalconException; ++import org.apache.falcon.entity.v0.process.*; ++import org.apache.falcon.entity.v0.process.Process; + import org.apache.falcon.util.StartupProperties; ++import org.mockito.Mockito; + import org.testng.Assert; + import org.testng.annotations.Test; + ++import java.io.IOException; ++ + /** + * Unit test for Security utils. + */ +@@ -81,4 +88,25 @@ public class SecurityUtilTest { + Assert.assertEquals(SecurityUtil.getAuthorizationProvider().getClass(), + DefaultAuthorizationProvider.class); + } ++ ++ @Test ++ public void testTryProxy() throws IOException, FalconException { ++ Process process = Mockito.mock(Process.class); ++ StartupProperties.get().setProperty("falcon.security.authorization.enabled", "true"); ++ final String currentUser = System.getProperty("user.name"); ++ ++ // When ACL not specified ++ CurrentUser.authenticate(currentUser); ++ SecurityUtil.tryProxy(process); ++ Assert.assertEquals(CurrentUser.getUser(), currentUser); ++ ++ ACL acl = new ACL(); ++ acl.setOwner("testuser"); ++ acl.setGroup("users"); ++ Mockito.when(process.getACL()).thenReturn(acl); ++ ++ // When ACL is specified ++ SecurityUtil.tryProxy(process); ++ Assert.assertEquals(CurrentUser.getUser(), "testuser"); ++ } + } +diff --git a/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java b/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java +index 9a044d9..67a66c6 100644 +--- a/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java ++++ b/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java +@@ -406,23 +406,13 @@ public abstract class AbstractEntityManager { + + "Can't be submitted again. Try removing before submitting."); + } + +- tryProxy(entity); // proxy before validating since FS/Oozie needs to be proxied ++ SecurityUtil.tryProxy(entity); // proxy before validating since FS/Oozie needs to be proxied + validate(entity); + configStore.publish(entityType, entity); + LOG.info("Submit successful: ({}): {}", type, entity.getName()); + return entity; + } + +- private void tryProxy(Entity entity) throws IOException, FalconException { +- final String aclOwner = entity.getACL().getOwner(); +- final String aclGroup = entity.getACL().getGroup(); +- if (SecurityUtil.isAuthorizationEnabled() +- && SecurityUtil.getAuthorizationProvider().shouldProxy( +- CurrentUser.getAuthenticatedUGI(), aclOwner, aclGroup)) { +- CurrentUser.proxy(aclOwner, aclGroup); +- } +- } +- + /** + * KLUDGE - Until ACL is mandated entity passed should be decorated for equals check to pass. + * existingEntity in config store will have teh decoration and equals check fails +@@ -646,7 +636,7 @@ public abstract class AbstractEntityManager { + // the user who requested list query has no permission to access this entity. Skip this entity + continue; + } +- tryProxy(entity); ++ SecurityUtil.tryProxy(entity); + + List tags = EntityUtil.getTags(entity); + List pipelines = EntityUtil.getPipelines(entity); +diff --git a/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java b/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java +index 6b022c9..15e94cd 100644 +--- a/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java ++++ b/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java +@@ -141,14 +141,7 @@ public class FalconAuthorizationFilter implements Filter { + try { + EntityType type = EntityType.getEnum(entityType); + Entity entity = EntityUtil.getEntity(type, entityName); +- if (entity != null && entity.getACL() != null) { +- final String aclOwner = entity.getACL().getOwner(); +- final String aclGroup = entity.getACL().getGroup(); +- if (authorizationProvider.shouldProxy( +- authenticatedUGI, aclOwner, aclGroup)) { +- CurrentUser.proxy(aclOwner, aclGroup); +- } +- } ++ SecurityUtil.tryProxy(entity); + } catch (FalconException ignore) { + // do nothing + } http://git-wip-us.apache.org/repos/asf/falcon/blob/c790761e/common/src/main/java/org/apache/falcon/cleanup/AbstractCleanupHandler.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/falcon/cleanup/AbstractCleanupHandler.java b/common/src/main/java/org/apache/falcon/cleanup/AbstractCleanupHandler.java index 5a4dce6..85d7263 100644 --- a/common/src/main/java/org/apache/falcon/cleanup/AbstractCleanupHandler.java +++ b/common/src/main/java/org/apache/falcon/cleanup/AbstractCleanupHandler.java @@ -116,11 +116,11 @@ public abstract class AbstractCleanupHandler { Entity entity) throws FalconException { try { final AccessControlList acl = entity.getACL(); - if (acl == null) { - throw new FalconException("ACL for entity " + entity.getName() + " is empty"); + // To support backward compatibility, will only use the ACL owner only if present + if (acl != null) { + CurrentUser.authenticate(acl.getOwner()); // proxy user } - CurrentUser.authenticate(acl.getOwner()); // proxy user return HadoopClientFactory.get().createProxiedFileSystem( ClusterHelper.getConfiguration(cluster)); } catch (Exception e) { http://git-wip-us.apache.org/repos/asf/falcon/blob/c790761e/common/src/main/java/org/apache/falcon/security/SecurityUtil.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/falcon/security/SecurityUtil.java b/common/src/main/java/org/apache/falcon/security/SecurityUtil.java index b9fd37e..861f80f 100644 --- a/common/src/main/java/org/apache/falcon/security/SecurityUtil.java +++ b/common/src/main/java/org/apache/falcon/security/SecurityUtil.java @@ -19,11 +19,13 @@ package org.apache.falcon.security; import org.apache.falcon.FalconException; +import org.apache.falcon.entity.v0.Entity; import org.apache.falcon.util.ReflectionUtils; import org.apache.falcon.util.StartupProperties; import org.apache.hadoop.security.authentication.server.KerberosAuthenticationHandler; import org.apache.hadoop.security.authentication.server.PseudoAuthenticationHandler; +import java.io.IOException; import java.net.InetAddress; import java.net.UnknownHostException; @@ -104,4 +106,16 @@ public final class SecurityUtil { "org.apache.falcon.security.DefaultAuthorizationProvider"); return ReflectionUtils.getInstanceByClassName(providerClassName); } + + public static void tryProxy(Entity entity) throws IOException, FalconException { + if (entity != null && entity.getACL() != null && SecurityUtil.isAuthorizationEnabled()) { + final String aclOwner = entity.getACL().getOwner(); + final String aclGroup = entity.getACL().getGroup(); + + if (SecurityUtil.getAuthorizationProvider().shouldProxy( + CurrentUser.getAuthenticatedUGI(), aclOwner, aclGroup)) { + CurrentUser.proxy(aclOwner, aclGroup); + } + } + } } http://git-wip-us.apache.org/repos/asf/falcon/blob/c790761e/common/src/test/java/org/apache/falcon/cleanup/LogCleanupServiceTest.java ---------------------------------------------------------------------- diff --git a/common/src/test/java/org/apache/falcon/cleanup/LogCleanupServiceTest.java b/common/src/test/java/org/apache/falcon/cleanup/LogCleanupServiceTest.java index 8e2e544..0df59b2 100644 --- a/common/src/test/java/org/apache/falcon/cleanup/LogCleanupServiceTest.java +++ b/common/src/test/java/org/apache/falcon/cleanup/LogCleanupServiceTest.java @@ -55,6 +55,8 @@ public class LogCleanupServiceTest extends AbstractTestBase { + "sample2" + "/logs/job-2010-01-01-01-00/000"); private final Path instanceLogPath4 = new Path("/projects/falcon/staging/falcon/workflows/process/" + "sample" + "/logs/latedata/2010-01-01-01-00"); + private final Path instanceLogPath5 = new Path("/projects/falcon/staging/falcon/workflows/process/" + + "sample3" + "/logs/job-2010-01-01-01-00/000"); private final Path feedInstanceLogPath = new Path("/projects/falcon/staging/falcon/workflows/feed/" + "impressionFeed" + "/logs/job-2010-01-01-01-00/testCluster/000"); private final Path feedInstanceLogPath1 = new Path("/projects/falcon/staging/falcon/workflows/feed/" @@ -90,15 +92,22 @@ public class LogCleanupServiceTest extends AbstractTestBase { Process otherProcess = (Process) process.copy(); otherProcess.setName("sample2"); otherProcess.setFrequency(new Frequency("days(1)")); + Process noACLProcess = (Process) process.copy(); + noACLProcess.setName("sample3"); + noACLProcess.setACL(null); ConfigurationStore.get().remove(EntityType.PROCESS, otherProcess.getName()); ConfigurationStore.get().publish(EntityType.PROCESS, otherProcess); + ConfigurationStore.get().remove(EntityType.PROCESS, + noACLProcess.getName()); + ConfigurationStore.get().publish(EntityType.PROCESS, noACLProcess); fs.mkdirs(instanceLogPath); fs.mkdirs(instanceLogPath1); fs.mkdirs(instanceLogPath2); fs.mkdirs(instanceLogPath3); fs.mkdirs(instanceLogPath4); + fs.mkdirs(instanceLogPath5); // fs.setTimes wont work on dirs fs.createNewFile(new Path(instanceLogPath, "oozie.log")); @@ -138,6 +147,7 @@ public class LogCleanupServiceTest extends AbstractTestBase { Assert.assertFalse(fs.exists(instanceLogPath)); Assert.assertFalse(fs.exists(instanceLogPath1)); Assert.assertFalse(fs.exists(instanceLogPath2)); + Assert.assertFalse(fs.exists(instanceLogPath5)); Assert.assertTrue(fs.exists(instanceLogPath3)); } http://git-wip-us.apache.org/repos/asf/falcon/blob/c790761e/common/src/test/java/org/apache/falcon/security/SecurityUtilTest.java ---------------------------------------------------------------------- diff --git a/common/src/test/java/org/apache/falcon/security/SecurityUtilTest.java b/common/src/test/java/org/apache/falcon/security/SecurityUtilTest.java index a36d916..7f9b405 100644 --- a/common/src/test/java/org/apache/falcon/security/SecurityUtilTest.java +++ b/common/src/test/java/org/apache/falcon/security/SecurityUtilTest.java @@ -18,10 +18,17 @@ package org.apache.falcon.security; + +import org.apache.falcon.FalconException; +import org.apache.falcon.entity.v0.process.*; +import org.apache.falcon.entity.v0.process.Process; import org.apache.falcon.util.StartupProperties; +import org.mockito.Mockito; import org.testng.Assert; import org.testng.annotations.Test; +import java.io.IOException; + /** * Unit test for Security utils. */ @@ -81,4 +88,25 @@ public class SecurityUtilTest { Assert.assertEquals(SecurityUtil.getAuthorizationProvider().getClass(), DefaultAuthorizationProvider.class); } + + @Test + public void testTryProxy() throws IOException, FalconException { + Process process = Mockito.mock(Process.class); + StartupProperties.get().setProperty("falcon.security.authorization.enabled", "true"); + final String currentUser = System.getProperty("user.name"); + + // When ACL not specified + CurrentUser.authenticate(currentUser); + SecurityUtil.tryProxy(process); + Assert.assertEquals(CurrentUser.getUser(), currentUser); + + ACL acl = new ACL(); + acl.setOwner("testuser"); + acl.setGroup("users"); + Mockito.when(process.getACL()).thenReturn(acl); + + // When ACL is specified + SecurityUtil.tryProxy(process); + Assert.assertEquals(CurrentUser.getUser(), "testuser"); + } } http://git-wip-us.apache.org/repos/asf/falcon/blob/c790761e/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java ---------------------------------------------------------------------- diff --git a/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java b/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java index 9a044d9..67a66c6 100644 --- a/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java +++ b/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java @@ -406,23 +406,13 @@ public abstract class AbstractEntityManager { + "Can't be submitted again. Try removing before submitting."); } - tryProxy(entity); // proxy before validating since FS/Oozie needs to be proxied + SecurityUtil.tryProxy(entity); // proxy before validating since FS/Oozie needs to be proxied validate(entity); configStore.publish(entityType, entity); LOG.info("Submit successful: ({}): {}", type, entity.getName()); return entity; } - private void tryProxy(Entity entity) throws IOException, FalconException { - final String aclOwner = entity.getACL().getOwner(); - final String aclGroup = entity.getACL().getGroup(); - if (SecurityUtil.isAuthorizationEnabled() - && SecurityUtil.getAuthorizationProvider().shouldProxy( - CurrentUser.getAuthenticatedUGI(), aclOwner, aclGroup)) { - CurrentUser.proxy(aclOwner, aclGroup); - } - } - /** * KLUDGE - Until ACL is mandated entity passed should be decorated for equals check to pass. * existingEntity in config store will have teh decoration and equals check fails @@ -646,7 +636,7 @@ public abstract class AbstractEntityManager { // the user who requested list query has no permission to access this entity. Skip this entity continue; } - tryProxy(entity); + SecurityUtil.tryProxy(entity); List tags = EntityUtil.getTags(entity); List pipelines = EntityUtil.getPipelines(entity); http://git-wip-us.apache.org/repos/asf/falcon/blob/c790761e/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java ---------------------------------------------------------------------- diff --git a/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java b/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java index 6b022c9..15e94cd 100644 --- a/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java +++ b/prism/src/main/java/org/apache/falcon/security/FalconAuthorizationFilter.java @@ -141,14 +141,7 @@ public class FalconAuthorizationFilter implements Filter { try { EntityType type = EntityType.getEnum(entityType); Entity entity = EntityUtil.getEntity(type, entityName); - if (entity != null && entity.getACL() != null) { - final String aclOwner = entity.getACL().getOwner(); - final String aclGroup = entity.getACL().getGroup(); - if (authorizationProvider.shouldProxy( - authenticatedUGI, aclOwner, aclGroup)) { - CurrentUser.proxy(aclOwner, aclGroup); - } - } + SecurityUtil.tryProxy(entity); } catch (FalconException ignore) { // do nothing }