Return-Path: X-Original-To: apmail-falcon-dev-archive@minotaur.apache.org Delivered-To: apmail-falcon-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 3ECD910FBD for ; Fri, 31 Jan 2014 09:47:21 +0000 (UTC) Received: (qmail 97318 invoked by uid 500); 31 Jan 2014 09:47:20 -0000 Delivered-To: apmail-falcon-dev-archive@falcon.apache.org Received: (qmail 97295 invoked by uid 500); 31 Jan 2014 09:47:20 -0000 Mailing-List: contact dev-help@falcon.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@falcon.incubator.apache.org Delivered-To: mailing list dev@falcon.incubator.apache.org Received: (qmail 97286 invoked by uid 99); 31 Jan 2014 09:47:18 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 31 Jan 2014 09:47:17 +0000 X-ASF-Spam-Status: No, hits=-1997.1 required=5.0 tests=ALL_TRUSTED,FORGED_HOTMAIL_RCVD2,HTML_MESSAGE,RP_MATCHES_RCVD X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO mail.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with SMTP; Fri, 31 Jan 2014 09:47:14 +0000 Received: (qmail 94711 invoked by uid 99); 31 Jan 2014 09:46:38 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 31 Jan 2014 09:46:38 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 5A1011D4718; Fri, 31 Jan 2014 09:46:36 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============6418896007328174605==" MIME-Version: 1.0 Subject: Re: Review Request 17580: FALCON-11 Add support for security in Falcon From: "Srikanth Sundarrajan" To: "Falcon" , "Srikanth Sundarrajan" Date: Fri, 31 Jan 2014 09:46:36 -0000 Message-ID: <20140131094636.21445.71304@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Srikanth Sundarrajan" X-ReviewGroup: Falcon X-ReviewRequest-URL: https://reviews.apache.org/r/17580/ X-Sender: "Srikanth Sundarrajan" References: <20140131052755.22330.79289@reviews.apache.org> In-Reply-To: <20140131052755.22330.79289@reviews.apache.org> Reply-To: "Srikanth Sundarrajan" X-ReviewRequest-Repository: falcon-git X-Virus-Checked: Checked by ClamAV on apache.org --===============6418896007328174605== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17580/#review33296 ----------------------------------------------------------- checkstyle/src/main/resources/falcon/checkstyle.xml Why is this downgraded to warning ? client/src/main/java/org/apache/falcon/client/FalconClient.java Remove ? common/src/main/java/org/apache/falcon/catalog/AbstractCatalogService.java Why is the metaStorePrincipal only in few methods ? Looks like they are the initial set of calls into the service and would end up caching the client object. But that is not a good assumption to make and can possibly introduce errors. Would prefer that either metaStorePrinicipal is in every call or allow something similar doAs functionality. common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java This seems to simply login as the current user, while the metaStorePrincipal is set as a var. Is this adequate to proxy for hcat client ? common/src/main/java/org/apache/falcon/entity/ClusterHelper.java Why is fs.default.name set twice through two different key constants? common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java Does it make sense to skip the validation for hftp then ? common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java This would set the umask for all files created henceforth through that file system object (which by the way is cached). There may be unwanted side effects with this. Can we simply set permission on the storePath dir instead ? common/src/main/java/org/apache/falcon/util/Preconditions.java Guava ? common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java Delete if not needed ? messaging/src/test/java/org/apache/falcon/messaging/FalconTopicProducerTest.java generic name: falcon ? messaging/src/test/java/org/apache/falcon/messaging/FalconTopicProducerTest.java generic name: falcon ? messaging/src/test/java/org/apache/falcon/messaging/FeedProducerTest.java generic name: falcon ? messaging/src/test/java/org/apache/falcon/messaging/FeedProducerTest.java generic name: falcon ? messaging/src/test/java/org/apache/falcon/messaging/ProcessProducerTest.java generic name: falcon ? messaging/src/test/java/org/apache/falcon/messaging/ProcessProducerTest.java generic name: falcon ? oozie/src/main/java/org/apache/falcon/converter/AbstractOozieEntityMapper.java Can you please elaborate on what the gap is ? oozie/src/main/java/org/apache/falcon/logging/LogProvider.java Is it required to proxy here ? Aren't logs written to a location that falcon owns with 777 permissions ? oozie/src/main/java/org/apache/falcon/workflow/FalconPostProcessing.java I am assuming this is FALCON-222 oozie/src/test/java/org/apache/falcon/oozie/workflow/FalconPostProcessingTest.java generic name: falcon ? oozie/src/test/java/org/apache/falcon/oozie/workflow/FalconPostProcessingTest.java generic name: falcon ? prism/src/main/java/org/apache/falcon/security/BasicAuthFilter.java Might be handy to have more java docs in here to explain a new reader on what is the role of the Options servlet and how are things wired up prism/src/main/java/org/apache/falcon/security/BasicAuthFilter.java Sane defaults have been removed. Is it intentional ? prism/src/test/java/org/apache/falcon/service/FalconTopicSubscriberTest.java generic name: falcon ? rerun/src/main/java/org/apache/falcon/latedata/LateDataHandler.java Except for detectChanges() all other functions are invoked in the MR job, which is already running as the workflow user. This might not work rerun/src/main/java/org/apache/falcon/latedata/LateDataHandler.java Except for detectChanges() all other functions are invoked in the MR job, which is already running as the workflow user. This might not work rerun/src/test/java/org/apache/falcon/rerun/queue/InMemoryQueueTest.java "falcon" instead of @author? src/conf/log4j.xml Why is this at trace level ? test-util/src/main/java/org/apache/falcon/cluster/util/EmbeddedCluster.java Perhaps this can be removed webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java This section is commented out. Intentional? - Srikanth Sundarrajan On Jan. 31, 2014, 5:27 a.m., Srikanth Sundarrajan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17580/ > ----------------------------------------------------------- > > (Updated Jan. 31, 2014, 5:27 a.m.) > > > Review request for Falcon. > > > Bugs: FALCON-11, FALCON-14, FALCON-15, FALCON-16, FALCON-17, FALCON-18, FALCON-219, and FALCON-220 > https://issues.apache.org/jira/browse/FALCON-11 > https://issues.apache.org/jira/browse/FALCON-14 > https://issues.apache.org/jira/browse/FALCON-15 > https://issues.apache.org/jira/browse/FALCON-16 > https://issues.apache.org/jira/browse/FALCON-17 > https://issues.apache.org/jira/browse/FALCON-18 > https://issues.apache.org/jira/browse/FALCON-219 > https://issues.apache.org/jira/browse/FALCON-220 > > > Repository: falcon-git > > > Description > ------- > > FALCON-11 Add support for security in Falcon > > > Diffs > ----- > > checkstyle/src/main/resources/falcon/checkstyle.xml f6df5cd > client/pom.xml a43f7f5 > client/src/main/java/org/apache/falcon/client/FalconClient.java 4a7207f > common/pom.xml 068a22c > common/src/main/java/org/apache/falcon/catalog/AbstractCatalogService.java 691d805 > common/src/main/java/org/apache/falcon/catalog/HiveCatalogService.java 51e4d6e > common/src/main/java/org/apache/falcon/cleanup/AbstractCleanupHandler.java 644afd2 > common/src/main/java/org/apache/falcon/cleanup/FeedCleanupHandler.java 7dbac58 > common/src/main/java/org/apache/falcon/entity/CatalogStorage.java 32f7605 > common/src/main/java/org/apache/falcon/entity/ClusterHelper.java 38b5c5b > common/src/main/java/org/apache/falcon/entity/FileSystemStorage.java 68370c7 > common/src/main/java/org/apache/falcon/entity/Storage.java 0634969 > common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java e633838 > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 5c1d9ad > common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 8647d43 > common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java 18ceb6e > common/src/main/java/org/apache/falcon/hadoop/HadoopClientFactory.java PRE-CREATION > common/src/main/java/org/apache/falcon/security/AuthenticationInitializationService.java PRE-CREATION > common/src/main/java/org/apache/falcon/security/CurrentUser.java 4d2299e > common/src/main/java/org/apache/falcon/security/FalconLoginModule.java d95e147 > common/src/main/java/org/apache/falcon/security/FalconSecurityConfiguration.java b80ab6d > common/src/main/java/org/apache/falcon/security/SecurityConstants.java 8f7ba4a > common/src/main/java/org/apache/falcon/security/SecurityUtil.java PRE-CREATION > common/src/main/java/org/apache/falcon/util/Preconditions.java PRE-CREATION > common/src/main/resources/startup.properties 3014418 > common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java 7668c7f > common/src/test/java/org/apache/falcon/entity/FileSystemStorageTest.java 7b48d2b > common/src/test/java/org/apache/falcon/hadoop/HadoopClientFactoryTest.java PRE-CREATION > common/src/test/java/org/apache/falcon/security/AuthenticationInitializationServiceTest.java PRE-CREATION > common/src/test/java/org/apache/falcon/security/SecurityUtilTest.java PRE-CREATION > common/src/test/java/org/apache/falcon/util/PreconditionsTest.java PRE-CREATION > feed/src/main/resources/config/workflow/replication-workflow.xml 91d0285 > feed/src/main/resources/config/workflow/retention-workflow.xml 8b444f5 > hadoop-webapp/pom.xml e576310 > messaging/pom.xml 9aa5347 > messaging/src/main/java/org/apache/falcon/messaging/EntityInstanceMessage.java eb49fd5 > messaging/src/main/java/org/apache/falcon/messaging/EntityInstanceMessageCreator.java ecda5eb > messaging/src/main/java/org/apache/falcon/messaging/MessageProducer.java b37931c > messaging/src/test/java/org/apache/falcon/messaging/FalconTopicProducerTest.java da126c7 > messaging/src/test/java/org/apache/falcon/messaging/FeedProducerTest.java e707567 > messaging/src/test/java/org/apache/falcon/messaging/ProcessProducerTest.java 3a40e76 > metrics/src/main/java/org/apache/falcon/aspect/GenericAlert.java 275a725 > oozie/src/main/java/org/apache/falcon/converter/AbstractOozieEntityMapper.java cecdeef > oozie/src/main/java/org/apache/falcon/logging/LogMover.java d544311 > oozie/src/main/java/org/apache/falcon/logging/LogProvider.java 48d4589 > oozie/src/main/java/org/apache/falcon/service/SharedLibraryHostingService.java 37f8cfa > oozie/src/main/java/org/apache/falcon/workflow/FalconPostProcessing.java 3f9256c > oozie/src/main/java/org/apache/falcon/workflow/engine/OozieClientFactory.java b757531 > oozie/src/main/java/org/apache/falcon/workflow/engine/OozieHouseKeepingService.java 068e980 > oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java 7c3a425 > oozie/src/main/java/org/apache/oozie/client/CustomOozieClient.java c55221e > oozie/src/main/java/org/apache/oozie/client/ProxyOozieClient.java PRE-CREATION > oozie/src/test/java/org/apache/falcon/oozie/workflow/FalconPostProcessingTest.java c6485cd > pom.xml 9252d80 > prism/src/main/java/org/apache/falcon/resource/channel/HTTPChannel.java 2ba76a7 > prism/src/main/java/org/apache/falcon/security/BasicAuthFilter.java f172e82 > prism/src/main/java/org/apache/falcon/security/RemoteUserInHeaderBasedAuthenticationHandler.java PRE-CREATION > prism/src/main/java/org/apache/falcon/service/FalconTopicSubscriber.java 6ac926d > prism/src/main/java/org/apache/falcon/service/ProcessSubscriberService.java 1cd7776 > prism/src/test/java/org/apache/falcon/aspect/GenericAlertTest.java 1db71fd > prism/src/test/java/org/apache/falcon/service/FalconTopicSubscriberTest.java f1536f4 > process/src/main/resources/config/workflow/process-parent-workflow.xml 494bf20 > rerun/src/main/java/org/apache/falcon/latedata/LateDataHandler.java 4b35760 > rerun/src/main/java/org/apache/falcon/rerun/event/LaterunEvent.java b5ac121 > rerun/src/main/java/org/apache/falcon/rerun/event/RerunEvent.java baf4601 > rerun/src/main/java/org/apache/falcon/rerun/event/RerunEventFactory.java 03230f9 > rerun/src/main/java/org/apache/falcon/rerun/event/RetryEvent.java 1396f19 > rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunConsumer.java b073117 > rerun/src/main/java/org/apache/falcon/rerun/handler/AbstractRerunHandler.java ab7f472 > rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunConsumer.java fffd5cd > rerun/src/main/java/org/apache/falcon/rerun/handler/LateRerunHandler.java 897e7ab > rerun/src/main/java/org/apache/falcon/rerun/handler/RetryConsumer.java 63dade8 > rerun/src/main/java/org/apache/falcon/rerun/handler/RetryHandler.java 2b41a7c > rerun/src/main/java/org/apache/falcon/rerun/queue/InMemoryQueue.java 8234d8a > rerun/src/test/java/org/apache/falcon/rerun/handler/TestLateRerunHandler.java e02b495 > rerun/src/test/java/org/apache/falcon/rerun/queue/ActiveMQTest.java 01d0415 > rerun/src/test/java/org/apache/falcon/rerun/queue/InMemoryQueueTest.java 6aafaa5 > src/bin/falcon d196a5d > src/conf/log4j.xml 58ebd80 > src/conf/startup.properties 79cd211 > test-util/src/main/java/org/apache/falcon/cluster/util/EmbeddedCluster.java 2b55407 > webapp/pom.xml 8c37409 > webapp/src/conf/oozie/conf/oozie-site.xml e5f404a > webapp/src/main/resources/log4j.xml d133b8e > webapp/src/test/java/org/apache/falcon/catalog/HiveCatalogServiceIT.java 9909140 > webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java 72369c0 > webapp/src/test/java/org/apache/falcon/cli/FalconCLISmokeIT.java 55f240f > webapp/src/test/java/org/apache/falcon/late/LateDataHandlerIT.java 6cfa4e6 > webapp/src/test/java/org/apache/falcon/lifecycle/FileSystemFeedReplicationIT.java 058b35c > webapp/src/test/java/org/apache/falcon/lifecycle/TableStorageFeedEvictorIT.java 37226e2 > webapp/src/test/java/org/apache/falcon/lifecycle/TableStorageFeedReplicationIT.java dbc6442 > webapp/src/test/java/org/apache/falcon/process/PigProcessIT.java 1f4e9e8 > webapp/src/test/java/org/apache/falcon/process/TableStorageProcessIT.java 2d539c2 > webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseyIT.java 1ceaabf > webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseySmokeIT.java b96aa48 > webapp/src/test/java/org/apache/falcon/resource/ProcessInstanceManagerIT.java c0785ba > webapp/src/test/java/org/apache/falcon/resource/TestContext.java 9e10956 > webapp/src/test/java/org/apache/falcon/security/BasicAuthFilterTest.java 0ff993b > webapp/src/test/java/org/apache/falcon/util/OozieTestUtils.java 3769dde > webapp/src/test/java/org/apache/falcon/util/ResourcesReflectionUtilTest.java e54f81c > webapp/src/test/java/org/apache/falcon/validation/ClusterEntityValidationIT.java 9299b5b > webapp/src/test/java/org/apache/falcon/validation/FeedEntityValidationIT.java db24b9a > > Diff: https://reviews.apache.org/r/17580/diff/ > > > Testing > ------- > > > Thanks, > > Srikanth Sundarrajan > > --===============6418896007328174605==--