[Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS
Ana Krivokapic
akrivoka at redhat.com
Mon Apr 15 17:28:01 UTC 2013
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.
--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akrivoka-0017-04-Integrate-realmdomains-with-IPA-DNS.patch
Type: text/x-patch
Size: 12669 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130415/c3328d8b/attachment.bin>
More information about the Freeipa-devel
mailing list