[Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS
Alexander Bokovoy
abokovoy at redhat.com
Tue Apr 16 11:32:43 UTC 2013
On Tue, 16 Apr 2013, 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.
Looking into DNS schema I see most records there use caseIgnoreIA5Match
or similar caseless search. It means our comparison in the python code
should follow caseless searches too.
--
/ Alexander Bokovoy
More information about the Freeipa-devel
mailing list