> On Sept. 27, 2013, 10:45 p.m., Ben Mahler wrote: > > src/master/flags.hpp, line 110 > > <https://reviews.apache.org/r/14292/diff/4/?file=357755#file357755line110> > > > > Please also allow non "file://" prefixed paths, akin to what we do for > > other local file/directory flags: > > > > --webui_dir > > --log_dir > > --work_dir > > --launcher_dir > > --frameworks_home > > > > I realize --zk and --whitelist do not, but we should fix that too! > > > > Just seems a bit more consistent and nicer from an operators > > perspective :)
well..those are all directories. --zk, --whitelist and --credentials are files :) But yea, I will allow non file:// for whitelist and credentials for now. > On Sept. 27, 2013, 10:45 p.m., Ben Mahler wrote: > > src/master/master.cpp, lines 314-315 > > <https://reviews.apache.org/r/14292/diff/4/?file=357757#file357757line314> > > > > Should we EXIT(1) here? Not sure. Maybe someone wants to disallow frameworks to authenticate with masters for whatever reason? > On Sept. 27, 2013, 10:45 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 806 > > <https://reviews.apache.org/r/14292/diff/4/?file=357757#file357757line806> > > > > HostPort(from) > > > > We could also store pair<string, int> and have HostPort(string) return > > pair<string, int>. That way, we can avoid these accidental uses of the > > implicit conversion from UPID to string. That makes printing it harder. Will stick with string for now and will do a HostPort struct class in a subsequent review. > On Sept. 27, 2013, 10:45 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 320 > > <https://reviews.apache.org/r/14292/diff/4/?file=357757#file357757line320> > > > > quotes around line? Might be good to include the path? > > > > e.g. > > > > Failed to parse credentials file '/foo/bar': Invalid format on line > > containing 'garbled'. Actually printing the credential was a bad idea. Fixed it to only print the line. > On Sept. 27, 2013, 10:45 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 1584 > > <https://reviews.apache.org/r/14292/diff/4/?file=357757#file357757line1584> > > > > There's a race here between discarding the Future on this Authenticator > > and the deferred _authenticate already executing. This would result in the > > CHECK failing in _authenticate. Thanks for catching this. Now made this very similar to what we do in authenticatee. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14292/#review26440 ----------------------------------------------------------- On Sept. 27, 2013, 6 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14292/ > ----------------------------------------------------------- > > (Updated Sept. 27, 2013, 6 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-704 > https://issues.apache.org/jira/browse/MESOS-704 > > > Repository: mesos-git > > > Description > ------- > > Added authentication support for scheduler driver and master. > > > Diffs > ----- > > include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664 > include/mesos/scheduler.hpp cf3ecdaaf40fd878a80fe0b6f7e61a0997329cbd > src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda > src/authentication/authenticatee.hpp PRE-CREATION > src/authentication/authenticator.hpp PRE-CREATION > src/common/type_utils.hpp 674a8820c339c6446dfa7d444457477ab4512e79 > src/java/jni/construct.cpp b01bd7ae2eda2dc5e0dcd68848c65bd9f9ea81f0 > src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp > 6d2a03b6a88e71ac4e2e2d1ee8e15925e393ef3d > src/java/src/org/apache/mesos/MesosSchedulerDriver.java > 7ef1fe7755286bf92b94d7ece4f72d54e5b57a84 > src/master/flags.hpp d59e67d5b2799d6d7a37e9cfe7246ae7372091ac > src/master/master.hpp bd5cb1ff05967518d7dc7f3a9dc0d32c22eb9643 > src/master/master.cpp a49b17ef43fca5b385a89731ca8776a26b61399a > src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5 > src/python/native/mesos_scheduler_driver_impl.cpp > f25d41d38caf2701813dbec0d342a3b327e9dedf > src/sched/sched.cpp c399f2481259683a8e178abb3478307042292f23 > src/tests/allocator_tests.cpp c57da6eb3c431b47468b6a6941c3de06af9209e5 > src/tests/allocator_zookeeper_tests.cpp > 6e3214c15c14dc8ba82082738c172c8833cd0887 > src/tests/authentication_tests.cpp PRE-CREATION > src/tests/exception_tests.cpp 3fc1ac32d553644080a88f04f22077691ae1820b > src/tests/fault_tolerance_tests.cpp > 10e52c401476eb8416361de49b8e4061bb7ac4f3 > src/tests/gc_tests.cpp e404de3bfacfcac5515995f1b45c3d39181e138f > src/tests/isolator_tests.cpp cd3b3000060b379ef10e38a2a98a2eebe69d90fc > src/tests/master_detector_tests.cpp > 2d140ba1a364a7af4d643951d6016ac17dd10526 > src/tests/master_tests.cpp 52f09d4f1ddeabcc1a797a13fae9641b72425dd5 > src/tests/mesos.hpp 8fbd56c8dd438a08673b54630bfe25d60ad5ee0e > src/tests/mesos.cpp 776cb0f13d10b4ae437fe9a3c97dc8b3481290af > src/tests/resource_offers_tests.cpp > 3888e461de5c8fa807cff2fd2bd7ca12c704823a > src/tests/slave_recovery_tests.cpp 48b2e6380a9ae688291992f3bf25c3cc473bc808 > src/tests/status_update_manager_tests.cpp > cf420e4764356402f05b27c3b8e8802c21a58f8e > > Diff: https://reviews.apache.org/r/14292/diff/ > > > Testing > ------- > > make check > > All our tests do auth now. yay! > > Also added couple specific tests. Will likely add more after integrating with > SASL authenticator. > > > Thanks, > > Vinod Kone > >
