[Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS
Ana Krivokapic
akrivoka at redhat.com
Tue Apr 16 14:20:34 UTC 2013
On 04/16/2013 03:54 PM, Martin Kosek wrote:
> On 04/16/2013 03:03 PM, Ana Krivokapic wrote:
>> On 04/16/2013 01:55 PM, Martin Kosek wrote:
>>> On 04/16/2013 12:31 PM, Ana Krivokapic wrote:
>>>> On 04/16/2013 09:14 AM, Martin Kosek wrote:
>>>>> On 04/15/2013 11:21 PM, Rob Crittenden wrote:
>>>>>> Ana Krivokapic wrote:
>>>>>>> On 04/15/2013 07:06 PM, Martin Kosek wrote:
>>>>>>>> On 04/15/2013 06:53 PM, Ana Krivokapic wrote:
>>>>>>>>> On 04/15/2013 06:30 PM, Martin Kosek wrote:
>>>>>>>>>> On 04/12/2013 08:45 PM, Ana Krivokapic wrote:
>>>>>>>>>>> On 04/12/2013 01:26 PM, Ana Krivokapic wrote:
>>>>>>>>>>>> On 04/12/2013 12:44 PM, Martin Kosek wrote:
>>>>>>>>>>>>> On 04/12/2013 12:20 PM, Ana Krivokapic wrote:
>>>>>>>>>>>>>> On 04/11/2013 03:03 PM, Alexander Bokovoy wrote:
>>>>>>>>>>>>>>> On Thu, 11 Apr 2013, Ana Krivokapic wrote:
>>>>>>>>>>>>>>>> On 04/11/2013 01:43 PM, Alexander Bokovoy wrote:
>>>>>>>>>>>>>>>>> On Thu, 11 Apr 2013, Petr Spacek wrote:
>>>>>>>>>>>>>>>>>> On 11.4.2013 13:24, Alexander Bokovoy wrote:
>>>>>>>>>>>>>>>>>>> On Thu, 11 Apr 2013, Petr Spacek wrote:
>>>>>>>>>>>>>>>>>>>> On 11.4.2013 13:09, Ana Krivokapic wrote:
>>>>>>>>>>>>>>>>>>>>> Integrate realmdomains with IPA DNS
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Add an entry to realmdomains when a DNS zone is added to IPA.
>>>>>>>>>>>>>>>>>>>>> Delete the
>>>>>>>>>>>>>>>>>>>>> related entry from realmdomains when the DNS zone is deleted from
>>>>>>>>>>>>>>>>>>>>> IPA.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/3544
>>>>>>>>>>>>>>>>>>>> I would add a TXT record as I described in
>>>>>>>>>>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/3544#comment:8
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> This integration probably should go to both commands,
>>>>>>>>>>>>>>>>>>>> realmdomains-*
>>>>>>>>>>>>>>>>>>>> dnszone-*.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Any objections? AB?
>>>>>>>>>>>>>>>>>>> Adding TXT record is probably harmless.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I would actually add the TXT record creation only to
>>>>>>>>>>>>>>>>>>> realmdomains-* and
>>>>>>>>>>>>>>>>>>> trigger it only in case we manage our DNS and DNS zone is there.
>>>>>>>>>>>>>>>>>>> This way a hook from dnszone-add will trigger adding TXT record back
>>>>>>>>>>>>>>>>>>> (via call to
>>>>>>>>>>>>>>>>>>> realmdomains-mod --add and then TXT record addition from there).
>>>>>>>>>>>>>>>>>>> Also
>>>>>>>>>>>>>>>>>>> the fact that admin added manually some domain to realmdomains
>>>>>>>>>>>>>>>>>>> mapping
>>>>>>>>>>>>>>>>>>> means that it is implied to be used in obtaining TGTs, so TXT
>>>>>>>>>>>>>>>>>>> record is
>>>>>>>>>>>>>>>>>>> helpful there as well.
>>>>>>>>>>>>>>>>>> Okay, it makes sense. We will see how it will work in reality.
>>>>>>>>>>>>>>>>> One more thing to check is that we don't do this for our own domain.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Our own domain is already in realmdomains by default, and it cannot be
>>>>>>>>>>>>>>>> removed from there. So I don't think any check related to our domain is
>>>>>>>>>>>>>>>> necessary.
>>>>>>>>>>>>>>> We shouldn't start creating TXT records for our own domain, that's what
>>>>>>>>>>>>>>> I'm asking for here.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Think about server install stage -- we start creating our own domain and
>>>>>>>>>>>>>>> the hook then causes to create realmdomains entry for the domain,
>>>>>>>>>>>>>>> causing realmdomains-mod code to raise ValidationError which is not
>>>>>>>>>>>>>>> handled in dnszone-add code with this patch.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Same for TXT record creation starting from realmdomains-mod side -- it
>>>>>>>>>>>>>>> simply should avoid calling dnsrecord-add for the case we know wouldn't
>>>>>>>>>>>>>>> work.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I just realized that this ticket was not marked as RFE although it
>>>>>>>>>>>>>> obviously is
>>>>>>>>>>>>>> one. I fixed the ticket summary and wrote the design page for this
>>>>>>>>>>>>>> enhancement:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> http://www.freeipa.org/page/V3/DNS_realmdomains_integration
>>>>>>>>>>>>>>
>>>>>>>>>>>>> Right, that was a good thing to do. I just have comment for the UPN
>>>>>>>>>>>>> enumeration
>>>>>>>>>>>>> image which you linked in the RFE - can you please process it, upload
>>>>>>>>>>>>> to the
>>>>>>>>>>>>> wiki and include in the overview? This will make the RFE page more
>>>>>>>>>>>>> appealing
>>>>>>>>>>>>> and it will also prevent us from having a broken link when Alexander
>>>>>>>>>>>>> removes
>>>>>>>>>>>>> the file from his temporary directory.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Martin
>>>>>>>>>>>> Sure, done.
>>>>>>>>>>>>
>>>>>>>>>>> I added the functionality to create TXT record to realmdomains-mod, and also
>>>>>>>>>>> made sure that the case of our own domain is handled properly. Unit tests
>>>>>>>>>>> have
>>>>>>>>>>> been added to cover the new functionality. One unit test of the dns plugin
>>>>>>>>>>> needed adjusting, but it still fails due to the bug in the testing
>>>>>>>>>>> framework[1]. It should pass after the bug is fixed.
>>>>>>>>>>>
>>>>>>>>>>> Updated patch is attached.
>>>>>>>>>>>
>>>>>>>>>>> [1] https://fedorahosted.org/freeipa/ticket/3562
>>>>>>>>>>>
>>>>>>>>>> This looks nice, thanks for the new test cases.
>>>>>>>>>>
>>>>>>>>>> I experienced an issue with dnsrecord-find test in
>>>>>>>>>> tests/test_xmlrpc/test_dns_plugin.py, but I see you already have an open
>>>>>>>>>> ticket
>>>>>>>>>> to fix that (https://fedorahosted.org/freeipa/ticket/3562) so it is not a
>>>>>>>>>> show-stopper.
>>>>>>>>>>
>>>>>>>>>> This is a nitpick, but could you update
>>>>>>>>>> tests/test_xmlrpc/test_dns_realmdomains_integration.py to use the same
>>>>>>>>>> domains
>>>>>>>>>> for testing as tests/test_xmlrpc/test_dns_plugin.py does?
>>>>>>>>>>
>>>>>>>>>> I often use example*.com zones in my testing and we also advertise test
>>>>>>>>>> commands with these zones in "ipa help dns" too, so I (and maybe others)
>>>>>>>>>> could
>>>>>>>>>> get surprised that these zones are deleted after running the test suite.
>>>>>>>>>> I.e. I
>>>>>>>>>> would prefer to have dnszone*.test used for test.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Martin
>>>>>>>>> Sure. Updated patch attached.
>>>>>>>>>
>>>>>>>> One more nitpick (sorry for not spotting it earlier). In realmdomains-mod, you
>>>>>>>> do checks for zone/record before you do the dnsrecord-add/dnsrecord-del
>>>>>>>> command.
>>>>>>>>
>>>>>>>> I think this will unnecessarily make the command slower. You can just try
>>>>>>>> add/delete a record and catch also a NotFound error - these commands already
>>>>>>>> check for zone/record existence, so we do not need to do the checks twice.
>>>>>>>>
>>>>>>>> Relevant sections:
>>>>>>>>
>>>>>>>> + try:
>>>>>>>> + api.Command['dnszone_show'](unicode(d))
>>>>>>>> + except errors.NotFound:
>>>>>>>> + pass
>>>>>>>> + else:
>>>>>>>> + try:
>>>>>>>> + api.Command['dnsrecord_add'](
>>>>>>>> + unicode(d),
>>>>>>>> + u'_kerberos',
>>>>>>>> + txtrecord=api.env.realm
>>>>>>>> + )
>>>>>>>> + except errors.EmptyModlist:
>>>>>>>> + pass
>>>>>>>>
>>>>>>>> ...
>>>>>>>>
>>>>>>>> + try:
>>>>>>>> + api.Command['dnszone_show'](unicode(d))
>>>>>>>> + except errors.NotFound:
>>>>>>>> + pass
>>>>>>>> + else:
>>>>>>>> + try:
>>>>>>>> + api.Command['dnsrecord_del'](
>>>>>>>> + unicode(d),
>>>>>>>> + u'_kerberos',
>>>>>>>> + txtrecord=api.env.realm
>>>>>>>> + )
>>>>>>>> + except errors.AttrValueNotFound:
>>>>>>>> + pass
>>>>>>>>
>>>>>>>> Martin
>>>>>>> Nice catch, those checks were totally unnecessary. Thanks.
>>>>>> I tried adding an upper-case version of my primary domain and got an odd error:
>>>>>>
>>>>>> $ ipa realmdomains-mod --add-domain EXAMPLE.COM
>>>>>> ipa: ERROR: Type or value exists:
>>>>>>
>>>>>> Ok, I can sort of accept that, but there is no real detail.
>>>>>>
>>>>>> httpd logged this though:
>>>>>>
>>>>>> [Mon Apr 15 17:15:48.048169 2013] [:error] [pid 3515] ipa: INFO:
>>>>>> admin at EXAMPLE.COM: realmdomains_mod(add_domain=u'EXAMPLE.COM', rights=False,
>>>>>> force=False, all=False, raw=False, version=u'2.57'): DatabaseError
>>>>>>
>>>>>> I'm really a little confused about this discrepancy. The client side looks
>>>>>> right, I don't understand why we logged a DB error on the server.
>>>>>>
>>>>>> LDAP has the right error:
>>>>>>
>>>>>> [15/Apr/2013:17:15:47 -0400] conn=20 op=7 MOD dn="cn=Realm
>>>>>> Domains,cn=ipa,cn=etc,dc=example,dc=com"
>>>>>> [15/Apr/2013:17:15:47 -0400] conn=20 op=7 RESULT err=20 tag=103 nentries=0 etime=0
>>>>>>
>>>>>> What led me to try this was this code:
>>>>>>
>>>>>> + for d in domains_added:
>>>>>> + # Skip our own domain
>>>>>> + if d == api.env.domain:
>>>>>> + continue
>>>>>>
>>>>>> This probably needs to be case insensitive.
>>>>>>
>>>>>> rob
>>>>> We may also want to normalize zones to the non-fqdn DNS version before adding
>>>>> them to realmdomains - i.e. store "example.com" instead of "example.com.".
>>>>> Adding Alexander to CC list to verify this is a sound requirement.
>>>>>
>>>>> Otherwise, we would have duplicate zones in realmdomains object:
>>>>>
>>>>> # ipa dnszone-add example2.com --name-server=`hostname`.
>>>>>
>>>>> # ipa realmdomains-show
>>>>> Domain: idm.lab.bos.redhat.com, example2.com
>>>>>
>>>>> # ipa realmdomains-mod --add-domain=example2.com.
>>>>> Domain: idm.lab.bos.redhat.com, example2.com, example2.com.
>>>>>
>>>>> example2.com. should be normalized to example.com
>>>>>
>>>>>
>>>>> # ipa dnszone-add example3.com. --name-server=`hostname`.
>>>>>
>>>>> # ipa realmdomains-show
>>>>> Domain: idm.lab.bos.redhat.com, example2.com, example2.com., example3.com.
>>>>>
>>>>> I think example3.com. should be normalized to example3.com
>>>>>
>>>>> Martin
>>>> I added normalizers for realmdomains-mod options, which should fix
>>>> issues encountered by Rob and Martin.
>>>>
>>>> Updated patch is attached.
>>>>
>>> Good! I think this addresses all concerns and issues we found. I have one last
>>> minor issue and then we can push it.
>>>
>>> Please just add the normalizers to an own function as it is done in other
>>> plugins. It is then much easier to change such normalizer in one central location.
>>>
>>> Thanks,
>>> Martin
>> As we agreed on IRC, I implemented the following two changes:
>>
>> 1) Create a separate normalizer function
>> 2) Properly handle reverse zones (i.e. do not create realmdomains
>> entries or TXT records for them)
>>
>> Updated patch is attached.
>>
> ACK. Pushed to master.
>
> Martin
I updated the design page and linked it in
http://www.freeipa.org/page/V3_Designs
--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.
More information about the Freeipa-devel
mailing list