> On Nov. 28, 2017, 8:59 p.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java
> > Lines 614-616 (original), 616-618 (patched)
> > <https://reviews.apache.org/r/64118/diff/1/?file=1902406#file1902406line620>
> >
> >     It seems like `service`, `component`, and `principal` are not needed 
> > here anymore.   Maybe we should remove them from the Heartbeat request and 
> > response structures. This will require some changes on the agent side as 
> > well.

As for now I removed service and component, because it is really not needed by 
agent, but principal field will become List in future, as it will represent 
list of principals for each keytab. But this will be changed


> On Nov. 28, 2017, 8:59 p.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java
> > Lines 1902-1903 (original), 1921-1953 (patched)
> > <https://reviews.apache.org/r/64118/diff/1/?file=1902410#file1902410line1923>
> >
> >     It seems like this could be simplified using more _JPA-like_ logic.  
> > Someone with more experience with JPA (than me) should probably take a look.

I guess it can, I will try. But service-component mapping will require creating 
of additional entity. But maybe it will be simpler than current variant.


> On Nov. 28, 2017, 8:59 p.m., Robert Levas wrote:
> > ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql
> > Lines 938 (patched)
> > <https://reviews.apache.org/r/64118/diff/1/?file=1902432#file1902432line938>
> >
> >     Technially, this _unnatural_ key is not necessary since you have a 
> > valid _natural_ key - principal, keytab, component, service.

This is done to avoid one-to-many between KerberosKeytabPrincipalEntity and 
HostEntity, and indicate that principals with kkp_id=-1 belongs to 
ambari-server only, and still have mapping to host. So for cases when 
ambari-server host does not have ambari agent, host_id will be null and kkp_id 
-1.


> On Nov. 28, 2017, 8:59 p.m., Robert Levas wrote:
> > ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql
> > Lines 945 (patched)
> > <https://reviews.apache.org/r/64118/diff/1/?file=1902432#file1902432line945>
> >
> >     Should host_id be part of this?

see comment above, host_id=kkp_id or host_id=null kkp_id=-1.


- Eugene


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64118/#review192040
-----------------------------------------------------------


On Nov. 28, 2017, 1:17 p.m., Eugene Chekanskiy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64118/
> -----------------------------------------------------------
> 
> (Updated Nov. 28, 2017, 1:17 p.m.)
> 
> 
> Review request for Ambari, Attila Magyar, Balázs Bence Sári, and Robert Levas.
> 
> 
> Bugs: AMBARI-22530
>     https://issues.apache.org/jira/browse/AMBARI-22530
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Moved out of data files for principal generation.
> Now before every kerberos action all info written to database.
> 
> Server actions determine what principals to process based on filters passed 
> to action, this allow:
> 
> * selective generation
> * split generation process per-host/per-service/per-component basis and make 
> generation concurrent for seperate parts
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java
>  53cceb0ded 
>   
> ambari-server/src/main/java/org/apache/ambari/server/agent/HeartbeatProcessor.java
>  83d2c9808d 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/DeleteIdentityHandler.java
>  a7b9d80df0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java
>  749943dc00 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java
>  ab85aa1d7c 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/KerberosKeytabDAO.java
>  a8723b7bfa 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/KerberosKeytabPrincipalDAO.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostGroupComponentEntityPK.java
>  0898133bb5 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/KerberosKeytabEntity.java
>  a25931b946 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/KerberosKeytabPrincipalEntity.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/KerberosKeytabPrincipalEntityPK.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/KerberosPrincipalHostEntity.java
>  d4e80c65d2 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractPrepareKerberosServerAction.java
>  b8affb4e19 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CleanupServerAction.java
>  002076d85c 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/ConfigureAmbariIdentitiesServerAction.java
>  338415280f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreateKeytabFilesServerAction.java
>  5ec4c1011e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java
>  0c906592af 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/DestroyPrincipalsServerAction.java
>  4c80bd425e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/FinalizeKerberosServerAction.java
>  bfd5e4036d 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java
>  ff5f5cef1c 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/PrepareEnableKerberosServerAction.java
>  671ad95c8f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/PrepareKerberosIdentitiesServerAction.java
>  83a2106afd 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/stageutils/KerberosKeytabController.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/stageutils/ResolvedKerberosKeytab.java
>  17e484ad10 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/stageutils/ResolvedKerberosPrincipal.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/upgrades/PreconfigureKerberosAction.java
>  ca78dbb8a2 
>   ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql 7045240b30 
>   ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql c950c7ef83 
>   ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 537ae196c5 
>   ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql b4952c2e86 
>   ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql 
> 4fb0d0981a 
>   ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 8a88aba905 
>   ambari-server/src/main/resources/META-INF/persistence.xml 686c8312cd 
>   
> ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java
>  b4ff5c10ab 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java
>  ee87d24d8a 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/db/DDLTests.java 
> 96cf64e53c 
>   
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/FinalizeKerberosServerActionTest.java
>  c9301f3b7e 
>   
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerActionTest.java
>  e6f0868360 
>   
> ambari-server/src/test/java/org/apache/ambari/server/serveraction/upgrades/PreconfigureKerberosActionTest.java
>  a08f7a0a25 
> 
> 
> Diff: https://reviews.apache.org/r/64118/diff/1/
> 
> 
> Testing
> -------
> 
> manual testing, all kind of regenerate keytabs
> unit tests updating in progress
> 
> 
> Thanks,
> 
> Eugene Chekanskiy
> 
>

Reply via email to