> On Jan. 31, 2014, 9:46 a.m., Srikanth Sundarrajan wrote: > > common/src/main/java/org/apache/falcon/catalog/AbstractCatalogService.java, > > line 41 > > <https://reviews.apache.org/r/17580/diff/1/?file=457491#file457491line41> > > > > 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. > > Seetharam Venkatesh wrote: > Only these 2 methods are invoked from with in falcon server but > everything else is with in oozie namespace which oozie handles. May be I can > add a comment.
Will prefer that a new method say "login" added to AbstractCatalogService that isAlive or tableExists or any other method can probably use. > On Jan. 31, 2014, 9:46 a.m., Srikanth Sundarrajan wrote: > > common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java, > > line 101 > > <https://reviews.apache.org/r/17580/diff/1/?file=457502#file457502line101> > > > > 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 ? > > Seetharam Venkatesh wrote: > Thats a good point, I tried with FsPermissions instead but did not work. > Its funny that I have run into issues a few time. But will change. should be FsPermission permission = new FsPermission(FsAction.ALL, FsAction.READ_EXECUTE, FsAction.NONE); instead of FsPermission permission = new FsPermission(FsAction.ALL, FsAction.EXECUTE, FsAction.NONE); > On Jan. 31, 2014, 9:46 a.m., Srikanth Sundarrajan wrote: > > common/src/main/java/org/apache/falcon/util/Preconditions.java, line 1 > > <https://reviews.apache.org/r/17580/diff/1/?file=457510#file457510line1> > > > > Guava ? > > Seetharam Venkatesh wrote: > Thought about it but just for one class? There are numerous places where Guava can help. We should track this separately. Meanwhile does it help to use commons-lang:Validate instead of adding another class ? > On Jan. 31, 2014, 9:46 a.m., Srikanth Sundarrajan wrote: > > prism/src/main/java/org/apache/falcon/security/BasicAuthFilter.java, line 46 > > <https://reviews.apache.org/r/17580/diff/1/?file=457542#file457542line46> > > > > 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 > > Seetharam Venkatesh wrote: > Its all documented in Hadoop-auth. If you look at security docs, they > contain a link. Links here are broken: http://hadoop.apache.org/docs/current/hadoop-auth/Configuration.html, We ought to file jira with the hadoop team I guess. > On Jan. 31, 2014, 9:46 a.m., Srikanth Sundarrajan wrote: > > prism/src/main/java/org/apache/falcon/security/BasicAuthFilter.java, line 63 > > <https://reviews.apache.org/r/17580/diff/1/?file=457542#file457542line63> > > > > Sane defaults have been removed. Is it intentional ? > > Seetharam Venkatesh wrote: > Yes, this was too restrictive and hard to change. Instead, I have it > wired up from the startup props file which is better. Please remember to add this as an incompatible change in the CHANGES file if we remove the defaults. > On Jan. 31, 2014, 9:46 a.m., Srikanth Sundarrajan wrote: > > rerun/src/main/java/org/apache/falcon/latedata/LateDataHandler.java, line > > 139 > > <https://reviews.apache.org/r/17580/diff/1/?file=457549#file457549line139> > > > > Except for detectChanges() all other functions are invoked in the MR > > job, which is already running as the workflow user. This might not work > > Seetharam Venkatesh wrote: > Good catch, its unnecessary. > > Seetharam Venkatesh wrote: > Well, after looking deeper, I think this is a necessary evil. > org.apache.falcon.rerun.handler.LateRerunConsumer#detectLate invokes > org.apache.falcon.latedata.LateDataHandler#computeStorageMetric in falcon > server to recompute and see if late needs to be run in > org.apache.falcon.latedata.LateDataHandler#detectChanges. > > A proxy ugi for the login user does not hurt. > > An alternate would be to make 755 for latedata dir under logs dir owned > by falcon and falcon should be able to read the recorded size. makes sense? Can fs be passed to detectChanges to avoid creating proxies FS in incorrect places. User may not be able to proxy as themselves unless they are superuser. > On Jan. 31, 2014, 9:46 a.m., Srikanth Sundarrajan wrote: > > webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java, line 514 > > <https://reviews.apache.org/r/17580/diff/1/?file=457572#file457572line514> > > > > This section is commented out. Intentional? > > Seetharam Venkatesh wrote: > Not sure which one here. Line 508 does not have a comment. It it: > Assert.assertEquals(0, > new FalconCLI().run(("admin -stack -url " + > TestContext.BASE_URL).split("\\s"))); > testGetVersion and some tests in testClientProperties have been disabled. Any specific reason ? - Srikanth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17580/#review33296 ----------------------------------------------------------- On Feb. 14, 2014, 6:20 a.m., Srikanth Sundarrajan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17580/ > ----------------------------------------------------------- > > (Updated Feb. 14, 2014, 6:20 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 > ----- > > client/pom.xml a43f7f5 > client/src/main/java/org/apache/falcon/client/FalconClient.java 367fcc5 > 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/EntityUtil.java a3ad83d > 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/update/UpdateHelper.java 7ed4394 > 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 > docs/src/site/twiki/Security.twiki PRE-CREATION > docs/src/site/twiki/index.twiki 81c4c3e > docs/src/site/twiki/restapi/AdminConfig.twiki 2841b25 > docs/src/site/twiki/restapi/AdminStack.twiki a241999 > docs/src/site/twiki/restapi/AdminVersion.twiki fbf1405 > docs/src/site/twiki/restapi/EntityDefinition.twiki 955be71 > docs/src/site/twiki/restapi/EntityDelete.twiki 7a7e22a > docs/src/site/twiki/restapi/EntityDependencies.twiki 6daab68 > docs/src/site/twiki/restapi/EntityList.twiki bca84b0 > docs/src/site/twiki/restapi/EntityResume.twiki 223a83f > docs/src/site/twiki/restapi/EntitySchedule.twiki e481613 > docs/src/site/twiki/restapi/EntityStatus.twiki f0e772b > docs/src/site/twiki/restapi/EntitySubmit.twiki e4b608e > docs/src/site/twiki/restapi/EntitySubmitAndSchedule.twiki fb3649d > docs/src/site/twiki/restapi/EntitySuspend.twiki 9d6e9ab > docs/src/site/twiki/restapi/EntityUpdate.twiki 16ec439 > docs/src/site/twiki/restapi/EntityValidate.twiki bc0f508 > docs/src/site/twiki/restapi/InstanceKill.twiki 5c429f6 > docs/src/site/twiki/restapi/InstanceLogs.twiki f84b828 > docs/src/site/twiki/restapi/InstanceRerun.twiki cf35475 > docs/src/site/twiki/restapi/InstanceResume.twiki 2bdd6e1 > docs/src/site/twiki/restapi/InstanceRunning.twiki 6b5ee66 > docs/src/site/twiki/restapi/InstanceStatus.twiki eddc2c8 > docs/src/site/twiki/restapi/InstanceSuspend.twiki 62cf72b > docs/src/site/twiki/restapi/ResourceList.twiki b9ec4b6 > feed/src/main/java/org/apache/falcon/converter/OozieFeedMapper.java 6ca2134 > feed/src/main/resources/config/workflow/replication-workflow.xml 91d0285 > feed/src/main/resources/config/workflow/retention-workflow.xml 8b444f5 > feed/src/test/java/org/apache/falcon/converter/OozieFeedMapperTest.java > a37755b > hadoop-webapp/pom.xml e576310 > html5-ui/js/falcon.js ff3a929 > 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 > 71ff430 > 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 fca01b5 > 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/java/org/apache/falcon/converter/OozieProcessMapper.java > 87be709 > process/src/main/resources/config/workflow/process-parent-workflow.xml > 494bf20 > > process/src/test/java/org/apache/falcon/converter/OozieProcessMapperTest.java > 61ddbdc > 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 > 6dc1e3f > > 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 > >
