----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54698/#review159121 -----------------------------------------------------------
ambari-agent/pom.xml <https://reviews.apache.org/r/54698/#comment230098> Removing this causes `ambari-agent-2.0.0.0-SNAPSHOT.jar` to be built. Is that necessary in addition to building `zkmigrator.jar`? ambari-agent/pom.xml (line 76) <https://reviews.apache.org/r/54698/#comment230087> Specifying 4.8.1 here is unnecessary. Version is inherited from parent POM, and the tests pass with that version, too. ambari-agent/src/main/java/org/apache/ambari/tools/zk/ZkAcl.java (line 1) <https://reviews.apache.org/r/54698/#comment230085> RAT check fails for this file. ambari-agent/src/main/java/org/apache/ambari/tools/zk/ZkAcl.java (line 21) <https://reviews.apache.org/r/54698/#comment230094> "of me" seems more appropriate in this sentence than "from me" ambari-agent/src/main/java/org/apache/ambari/tools/zk/ZkAcl.java (line 23) <https://reviews.apache.org/r/54698/#comment230089> Empty `@return` is unnecessary. ambari-agent/src/main/java/org/apache/ambari/tools/zk/ZkAcl.java (lines 42 - 43) <https://reviews.apache.org/r/54698/#comment230099> Please do not add `@throws` without description if `throws` already lists the same exceptions. Same in `ZkConnection#open`. ambari-agent/src/main/java/org/apache/ambari/tools/zk/ZkConnection.java (lines 38 - 40) <https://reviews.apache.org/r/54698/#comment230090> Please do not add `@param` without description. ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java (line 3018) <https://reviews.apache.org/r/54698/#comment230093> Why `public`? (same for `addStopZookeeperStage`) ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java (line 3040) <https://reviews.apache.org/r/54698/#comment230092> Only 1 item passed to `asList`, could be `singletonList`. Same in `addStopZookeeperStage`. ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java (lines 3041 - 3042) <https://reviews.apache.org/r/54698/#comment230097> Can you please extract the empty map to a local variable and pass it to both `ActionExecutionContext` and `addExecutionCommandsToStage`? (Having to do so looks like a code smell, but I think it's OK not to fix it in this patch.) Same in `addStopZookeeperStage`. ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java (lines 3066 - 3067) <https://reviews.apache.org/r/54698/#comment230095> Wouldn't `cluster.getServices().get("ZOOKEEPER")` work here? - Attila Doroszlai On Dec. 13, 2016, 6:30 p.m., Attila Magyar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54698/ > ----------------------------------------------------------- > > (Updated Dec. 13, 2016, 6:30 p.m.) > > > Review request for Ambari, Attila Doroszlai, Jaimin Jetly, Laszlo Puskas, > Oliver Szabo, Robert Levas, and Sebastian Toader. > > > Bugs: AMBARI-19187 > https://issues.apache.org/jira/browse/AMBARI-19187 > > > Repository: ambari > > > Description > ------- > > Hadoop components need to establish a secure connection with ZooKeeper when > Kerberos is enabled. This involves the setup of the correct authentication > (JAAS config file) and authorization (per-component Kerberos-backed ACLs on > the znodes) between the service and ZooKeeper. Most services are able to set > these ACLs based on their config when the user enable kerberos. > When we disable kerberos again, the sasl ACL should be removed otherwise the > services won't be able to access their znodes. > > This issue is about introducing a new command (DISABLE_SECURITY) that will be > sent by the ambari server to the services upon the dekerberiztion process. > When a service receives this command it will be able to do the zookeeper > secure to unsecure migration process (e.g. removing sasl ACLs). > > Notable changes: > - Added a java command line tool to the agent project that can setAcls > recursively on a znode > - Modified the dekerberization workflow: > - 1. UI stops all services but zookeeper > - 2. 2 new stages was introduced in the backend (send DISABLE_SECURITY > command to the services, start zookeeper) > > > Diffs > ----- > > ambari-agent/pom.xml a8ed7f1 > ambari-agent/src/main/java/org/apache/ambari/tools/zk/ZkAcl.java > PRE-CREATION > ambari-agent/src/main/java/org/apache/ambari/tools/zk/ZkConnection.java > PRE-CREATION > ambari-agent/src/main/java/org/apache/ambari/tools/zk/ZkMigrator.java > PRE-CREATION > ambari-agent/src/test/java/org/apache/ambari/tools/zk/ZkMigratorTest.java > PRE-CREATION > > ambari-common/src/main/python/resource_management/core/resources/zkmigrator.py > PRE-CREATION > > ambari-common/src/main/python/resource_management/libraries/script/script.py > 584775e > ambari-server/pom.xml 48ddb52 > > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java > 3261a56 > > ambari-server/src/main/java/org/apache/ambari/server/metadata/ActionMetadata.java > 0064662 > > ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java > 90f8098 > ambari-web/app/controllers/main/admin/kerberos/disable_controller.js > cec4503 > > Diff: https://reviews.apache.org/r/54698/diff/ > > > Testing > ------- > > Added unittests for ZkMigrator, KerberosHelperImpl > > Manual testings: > - created cluster with ambari > - enabled kerberos > - disabled kerberos > - checked if the DISABLE_SECURITY command was sent to the services > > > Ambari agent: > ---------------------------------------------------------------------- > Ran 450 tests in 10.634s > > Ambari server: > ---------------------------------------------------------------------- > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 34:44.448s > [INFO] Finished at: Tue Dec 13 14:29:00 CET 2016 > [INFO] Final Memory: 160M/798M > > > Thanks, > > Attila Magyar > >
