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