[Freeipa-devel] [PATCH] 0017 Integrate realmdomains with IPA DNS
Ana Krivokapic
akrivoka at redhat.com
Tue Apr 16 13:03:59 UTC 2013
On 04/16/2013 01:55 PM, Martin Kosek wrote:
> On 04/16/2013 12:31 PM, Ana Krivokapic wrote:
>> On 04/16/2013 09:14 AM, 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.
>>>
>>> Otherwise, we would have duplicate zones in realmdomains object:
>>>
>>> # ipa dnszone-add example2.com --name-server=`hostname`.
>>>
>>> # ipa realmdomains-show
>>> Domain: idm.lab.bos.redhat.com, example2.com
>>>
>>> # ipa realmdomains-mod --add-domain=example2.com.
>>> Domain: idm.lab.bos.redhat.com, example2.com, example2.com.
>>>
>>> example2.com. should be normalized to example.com
>>>
>>>
>>> # ipa dnszone-add example3.com. --name-server=`hostname`.
>>>
>>> # ipa realmdomains-show
>>> Domain: idm.lab.bos.redhat.com, example2.com, example2.com., example3.com.
>>>
>>> I think example3.com. should be normalized to example3.com
>>>
>>> Martin
>> I added normalizers for realmdomains-mod options, which should fix
>> issues encountered by Rob and Martin.
>>
>> Updated patch is attached.
>>
> Good! I think this addresses all concerns and issues we found. I have one last
> minor issue and then we can push it.
>
> Please just add the normalizers to an own function as it is done in other
> plugins. It is then much easier to change such normalizer in one central location.
>
> Thanks,
> Martin
As we agreed on IRC, I implemented the following two changes:
1) Create a separate normalizer function
2) Properly handle reverse zones (i.e. do not create realmdomains
entries or TXT records for them)
Updated patch is attached.
--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akrivoka-0017-06-Integrate-realmdomains-with-IPA-DNS.patch
Type: text/x-patch
Size: 13607 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130416/2ef7e4e6/attachment.bin>
More information about the Freeipa-devel
mailing list