ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tom Beerbower" <tbeerbo...@hortonworks.com>
Subject Re: Review Request 25814: Admin : LDAP Sync API to support sync events
Date Fri, 19 Sep 2014 15:13:59 GMT


> On Sept. 19, 2014, 3:05 p.m., Jonathan Hurley wrote:
> >

Thanks for reviewing!  I'll make the suggested changes and upload a new diff.


> On Sept. 19, 2014, 3:05 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/LdapSyncEventResourceProvider.java,
line 66
> > <https://reviews.apache.org/r/25814/diff/1/?file=694333#file694333line66>
> >
> >     This is actually a question that I had when doing some work on alerts. I would
say that we probably should be exposing these.

Yeah, I agree.  This patch is just a first step.  I'll leave the TODO and open a new JIRA
to expose the config.


> On Sept. 19, 2014, 3:05 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/LdapSyncEventResourceProvider.java,
line 140
> > <https://reviews.apache.org/r/25814/diff/1/?file=694333#file694333line140>
> >
> >     It looks like you're always synchronizing on this queue before reading/writing
to it. It probably doesn't need to be concurrent in that case. Unless the synchronizing isn't
necessary b/c you are using a concurrent queue.

I think your right.  The synchronization here is intended to control more than just access
to the queue but with it the queue doesn't need to be concurrent.  I'll change it.


> On Sept. 19, 2014, 3:05 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/LdapSyncEventResourceProvider.java,
line 298
> > <https://reviews.apache.org/r/25814/diff/1/?file=694333#file694333line298>
> >
> >     Why not use the epoch value here to be consistent with other dates returned
by the REST API?

Good point.  I'll make the change.


> On Sept. 19, 2014, 3:05 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/LdapSyncEventResourceProvider.java,
line 351
> > <https://reviews.apache.org/r/25814/diff/1/?file=694333#file694333line351>
> >
> >     Just a nit, but I like using AtomicLong instead of a synchronized method.

Agreed.  I'll change it.  Thanks!


- Tom


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


On Sept. 19, 2014, 2:21 a.m., Tom Beerbower wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25814/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2014, 2:21 a.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley and Nate Cole.
> 
> 
> Bugs: AMBARI-7349
>     https://issues.apache.org/jira/browse/AMBARI-7349
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This is the first step to introduce a new API for LDAP sync.  It does not hook up the
API to the exising LDAP sync back end.
> 
> 
> LDAP_SYNC API Proposal...
> 
> The ldap sync event resource contains the information needed to perform an ldap sync.
 This includes the principal type (user, group), the sync type (all, existing, specific),
and an optional set of principal names.  Wildcards will be supported for principal names.
 You can mix and match specs, so it is possible to have an event that syncs all users but
specific groups, for example.
> 
> --  Create ldap sync events ...
> {code}
> POST http://c6401.ambari.apache.org:8080/api/v1/ldap-sync-events
> {
>   "specs" : [
>     {
>       "principal-type":"users",
>       "sync-type":"all"
>     },
>     {
>       "principal-type":"groups",
>       "sync-type":"all"
>     }
>   ]
> }
> 
> 
> POST http://c6401.ambari.apache.org:8080/api/v1/ldap-sync-events
> {
>   "specs" : [
>     {
>       "principal-type":"users",
>       "sync-type":"existing"
>     },
>     {
>       "principal-type":"groups",
>       "sync-type":"existing"
>     }
>   ]
> }
> 
> POST http://c6401.ambari.apache.org:8080/api/v1/ldap-sync-events
> {
>   "specs" : [
>     {
>       "principal-type":"users",
>       "names":"fred,joe,admin*",
>       "sync-type":"specific"
>     },
>     {
>       "principal-type":"groups",
>       "names":"group1,group2,admin*",
>       "sync-type":"specific"
>     }
>   ]
> }
> {code}
> 
> When you create an event it is assigned a unique id.
> The response of each post will contain the href to the new ldap sync event resource.
 For example, 
> {code}"href" : "http://c6401.ambari.apache.org:8080/api/v1/ldap-sync-events/1"{code}
> The events are not persisted in the DB.  We will keep them in memory only for now, possibly
with an expiry value.  If there is a need to persist them beyond a stop then we can add the
DB persistence later.
> 
> When an event is added, it will be placed on a queue.  Events will be serviced sequentially
(for now we will not handle concurrent syncing).  
> 
> Events will be run as they are taken off the queue.  In the future we may add the ability
to schedule events. 
> 
> 
> --  Get all of the sync events ...
> {code}
> GET http://c6401.ambari.apache.org:8080/api/v1/ldap-sync-events
> {
>   "href" : "http://c6401.ambari.apache.org:8080/api/v1/ldap-sync-events/",
>   "items" : [
>     {
>       "href" : "http://c6401.ambari.apache.org:8080/api/v1/ldap-sync-events/1",
>       "Event" : {
>         "id":1
>       },
>       "href" : "http://c6401.ambari.apache.org:8080/api/v1/ldap-sync-events/2",
>       "Event" : {
>         "id":2
>       },
>       "href" : "http://c6401.ambari.apache.org:8080/api/v1/ldap-sync-events/3",
>       "Event" : {
>         "id":3
>       },
>     }
>   ]
> }
> {code}
> 
> You can view a specific sync event.
> 
> --  Get a specific sync event ...
> {code}
> GET http://c6401.ambari.apache.org:8080/api/v1/ldap-sync-events/1
> {
>   "href" : "http://c6401.ambari.apache.org:8080/api/v1/ldap-sync-events/1/",
>   "Event":{
>     "id":1,
>     "state":"COMPLETE",
>     "sync-start-time":"10/12/2004 12:14:26.002"
>     "sync-end-time":"10/12/2004 12:15:36.006"
>     "users-fetched":200,
>     "users-created":12,
>     "users-updated":4,
>     "users-removed":5,
>     "groups-fetched":10,
>     "groups-created":3,
>     "groups-updated":2,
>     "groups-removed":1,
>     "membership-fetched":20,
>     "membership-created":1,
>     "membership-updated":5,
>     "specs" : [
>       {
>         "principal-type":"users",
>         "sync-type":"all"
>       },
>       {
>         "principal-type":"groups",
>         "sync-type":"all"
>       }
>     ]
>   }
> }
> {code}
> 
> The values for the state property will be PENDING, RUNNING, COMPLETE.  The result values
will be null until the state is COMPLETE.
> 
> When an event is COMPLETE, the ldap user / group / member resources of Ambari should
have been updated accordingly.
> 
> Note that the results do not show the specific users and groups that were synced, only
the totals.  The specific users and groups touched by the sync can be added if required. 
> 
> Also note that this revised proposal removes the LDAP_SYNC resource as a root level service.
 It doesn't seem to be needed to meet the current requirements.  We can add it later, if needed.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/resources/LdapSyncEventResourceDefinition.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java
f85c0ea 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/LdapSyncEventService.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java
1b5075a 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/LdapSyncEventResourceProvider.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java a14f1f0

>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/LdapSyncEventEntity.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/LdapSyncSpecEntity.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/api/resources/LdapSyncEventResourceDefinitionTest.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/api/services/LdapSyncEventServiceTest.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/LdapSyncEventResourceProviderTest.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/entities/LdapSyncEventEntityTest.java
PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/orm/entities/LdapSyncSpecEntityTest.java
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25814/diff/
> 
> 
> Testing
> -------
> 
> Manual tests.
> 
> New unit tests added.  All existing tests pass ...
> 
> Results :
> 
> Tests run: 2021, Failures: 0, Errors: 0, Skipped: 16
> 
> ...
> 
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 31:18.827s
> [INFO] Finished at: Thu Sep 18 22:03:51 EDT 2014
> [INFO] Final Memory: 45M/196M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Tom Beerbower
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message