[Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS

Mon Apr 15 21:21:10 UTC 2013

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