[SSSD] [PATCH] Active Directory dynamic DNS updates

Jakub Hrozek jhrozek at redhat.com
Fri May 3 18:34:08 UTC 2013


On Fri, May 03, 2013 at 08:11:37PM +0200, Lukas Slebodnik wrote:
> On (02/05/13 20:04), Jakub Hrozek wrote:
> >On Tue, Apr 30, 2013 at 08:16:05PM +0200, Jakub Hrozek wrote:
> >> On Mon, Apr 22, 2013 at 02:49:11PM +0200, Jakub Hrozek wrote:
> >> > On Mon, Apr 22, 2013 at 02:20:59PM +0200, Pavel Březina wrote:
> >> > > On 04/17/2013 09:04 PM, Jakub Hrozek wrote:
> >> > > >https://fedorahosted.org/sssd/ticket/1504
> >> > > >
> >> > > >Implements dynamic DNS updates for the AD provider. By default, the
> >> > > >updates also update the reverse zone and run periodically every 24
> >> > > >hours.
> >> > > 
> >> > > Hi,
> >> > > I will do more thorough review in next days, but there are two thing
> >> > > that struck in my eyes.
> >> > > 
> >> > > >0001-Active-Directory-dynamic-DNS-updates.patch
> >> > > >
> >> > > >
> >> > > >From 5306abceba7e6eed97763a0cd48bb886ff161835 Mon Sep 17 00:00:00
> >> > > >2001 From: Jakub Hrozek<jhrozek at redhat.com> Date: Tue, 16 Apr 2013
> >> > > >17:04:43 +0200 Subject: [PATCH] Active Directory dynamic DNS updates
> >> > > >
> >> > > >https://fedorahosted.org/sssd/ticket/1504
> >> > > >
> >> > > >Implements dynamic DNS updates for the AD provider. By default, the
> >> > > >updates also update the reverse zone and run periodically every 24
> >> > > >hours.
> >> > > 
> >> > > >--- a/src/man/sssd-ad.5.xml +++ b/src/man/sssd-ad.5.xml
> >> > > 
> >> > > Please, move dyndns options into a separate xml and include it into
> >> > > sssd-ad and sssd-ipa.
> >> > > 
> >> > 
> >> > yes, I wanted to do so, but there are two differences I didn't know how
> >> > to solve without spending too much time on a man page:
> >> > 1) defaults. We could solve this by saying "In IPA the default is foo
> >> > but in AD default is bar". It's ugly.
> >> > 2) the IPA manpage includes the old options.
> >> > 
> >> > Maybe it could be solved by using a stringparam of xsltproc, I'll take a
> >> > look. Ideally I wanted to get the feature accepted first so it's
> >> > testable, then worry about duplication in man pages :)
> >> > 
> >> > > ad_dyndns.c and ipa_dyndns.c are *very* similar. It must be possible to
> >> > > write this code only ones. Do you have any specific reasons why haven't
> >> > > you done so?
> >> > 
> >> > They access private data and call static functions directly. As above, I
> >> > will take a look, but reducing code duplication is something we can do
> >> > post-beta.
> >> 
> >> I split the common code into the sdap_dyndns.c module and now both IPA
> >> and AD timers are just thin wrappers around this request.
> >> 
> >> I didn't squash the new request into the periodic update task patch,
> >> sorry. I don't think it's necessary to spend time resolving conflicts
> >> and actually I think the patches are easier to review this way.
> >> 
> >> New patches attached.
> >
> >Attached patches are rebased on top of Pavel's AD site patches.
> 
> I tested patches and AD dynamic updates works.
> 
> My test cases:
>    change IP address and restart network (simulation of network failure)
>    only change IP address (simulation of changing IP by dhcp)
> 
> I tried some combination of non default options (dyndns_refresh_interval,
> dyndns_update_ptr, dyndns_force_tcp) and dns updates were send.
> 
> ACK
> 
> LS

Thank you for the thorough testing.

Pushed to master.



More information about the sssd-devel mailing list