On 04/16/2013 03:54 PM, Martin Kosek wrote: > On 04/16/2013 03:03 PM, Ana Krivokapic wrote: >> 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: >>>>>> ad...@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. >> > ACK. Pushed to master. > > Martin
I updated the design page and linked it in http://www.freeipa.org/page/V3_Designs -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
