[Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS
Martin Kosek
mkosek at redhat.com
Mon Apr 15 17:06:34 UTC 2013
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
More information about the Freeipa-devel
mailing list