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

[Martin Kosek]

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