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. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc.
From 4f1cb2e12f76818a1cde1783301f3755915082c7 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic <akriv...@redhat.com> Date: Fri, 12 Apr 2013 15:20:07 +0200 Subject: [PATCH] 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. Add _kerberos TXT record to DNS zone when a new realmdomain is added. Delete _kerberos TXT record from DNS zone when realmdomain is deleted. Add unit tests to cover new functionality. https://fedorahosted.org/freeipa/ticket/3544 --- ipalib/plugins/dns.py | 21 +++ ipalib/plugins/realmdomains.py | 46 ++++++ tests/test_xmlrpc/test_dns_plugin.py | 8 +- .../test_dns_realmdomains_integration.py | 168 +++++++++++++++++++++ 4 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 tests/test_xmlrpc/test_dns_realmdomains_integration.py diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index becec14232e283e1f76cf4dd5d43de5cbba62d6e..f8ba158f56a5acb00d0ef6f6af00fda50bbb866a 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -1841,6 +1841,16 @@ class dnszone_add(LDAPCreate): dns_record, nameserver_ip_address) + # Add entry to realmdomains + # except for our own domain and for forwarded zones + zone = keys[0] + + if zone != api.env.domain and not options.get('idnsforwarders'): + try: + api.Command['realmdomains_mod'](add_domain=zone, force=True) + except errors.EmptyModlist: + pass + return dn api.register(dnszone_add) @@ -1857,6 +1867,17 @@ class dnszone_del(LDAPDelete): force=True) except errors.NotFound: pass + + # Delete entry from realmdomains + # except for our own domain + zone = keys[0] + + if zone != api.env.domain: + try: + api.Command['realmdomains_mod'](del_domain=zone, force=True) + except errors.AttrValueNotFound: + pass + return True api.register(dnszone_del) diff --git a/ipalib/plugins/realmdomains.py b/ipalib/plugins/realmdomains.py index f3dbf8dae21c94be12b432ef8d4de28cd83ab64e..6d75b15cacfd38cb3d26be97d6e32a91d4182b51 100644 --- a/ipalib/plugins/realmdomains.py +++ b/ipalib/plugins/realmdomains.py @@ -64,16 +64,19 @@ class realmdomains(LDAPObject): takes_params = ( Str('associateddomain+', _domain_name_validator, + normalizer=lambda d: d.lower().rstrip('.'), cli_name='domain', label=_('Domain'), ), Str('add_domain?', _domain_name_validator, + normalizer=lambda d: d.lower().rstrip('.'), cli_name='add_domain', label=_('Add domain'), ), Str('del_domain?', _domain_name_validator, + normalizer=lambda d: d.lower().rstrip('.'), cli_name='del_domain', label=_('Delete domain'), ), @@ -133,6 +136,49 @@ class realmdomains_mod(LDAPUpdate): entry_attrs['associateddomain'] = domains return dn + def execute(self, *keys, **options): + dn = self.obj.get_dn(*keys, **options) + ldap = self.obj.backend + + domains_old = set(ldap.get_entry(dn)[1]['associateddomain']) + result = super(realmdomains_mod, self).execute(*keys, **options) + domains_new = set(ldap.get_entry(dn)[1]['associateddomain']) + + domains_added = domains_new - domains_old + domains_deleted = domains_old - domains_new + + # Add a _kerberos TXT record for zones that correspond with + # domains which were added + for d in domains_added: + # Skip our own domain + if d == api.env.domain: + continue + try: + api.Command['dnsrecord_add']( + unicode(d), + u'_kerberos', + txtrecord=api.env.realm + ) + except (errors.EmptyModlist, errors.NotFound): + pass + + # Delete _kerberos TXT record from zones that correspond with + # domains which were deleted + for d in domains_deleted: + # Skip our own domain + if d == api.env.domain: + continue + try: + api.Command['dnsrecord_del']( + unicode(d), + u'_kerberos', + txtrecord=api.env.realm + ) + except (errors.AttrValueNotFound, errors.NotFound): + pass + + return result + api.register(realmdomains_mod) diff --git a/tests/test_xmlrpc/test_dns_plugin.py b/tests/test_xmlrpc/test_dns_plugin.py index 7b14fc6709dceb18bfd3a920f1829dcc1871061b..24fc7ecc4e50ecfc115da0ece3555b58d08ca326 100644 --- a/tests/test_xmlrpc/test_dns_plugin.py +++ b/tests/test_xmlrpc/test_dns_plugin.py @@ -34,6 +34,7 @@ dnszone1_rname = u'root.%s.' % dnszone1 dnszone1_permission = u'Manage DNS zone %s' % dnszone1 dnszone1_permission_dn = DN(('cn',dnszone1_permission), api.env.container_permission,api.env.basedn) +dnszone1_txtrec_dn = DN(('idnsname', '_kerberos'), dnszone1_dn) dnszone2 = u'dnszone2.test' dnszone2_dn = DN(('idnsname', dnszone2), api.env.container_dns, api.env.basedn) dnszone2_mname = u'ns1.%s.' % dnszone2 @@ -526,7 +527,7 @@ class test_dns(Declarative): command=('dnsrecord_find', [dnszone1], {}), expected={ 'summary': None, - 'count': 3, + 'count': 4, 'truncated': False, 'result': [ { @@ -535,6 +536,11 @@ class test_dns(Declarative): 'idnsname': [u'@'], }, { + 'dn': dnszone1_txtrec_dn, + 'txtrecord': [api.env.realm], + 'idnsname': [u'_kerberos'], + }, + { 'dn': dnszone1_mname_dn, 'idnsname': [u'ns1'], 'arecord': [u'1.2.3.4'], diff --git a/tests/test_xmlrpc/test_dns_realmdomains_integration.py b/tests/test_xmlrpc/test_dns_realmdomains_integration.py new file mode 100644 index 0000000000000000000000000000000000000000..c4660a577b0f3ae0eeab8aa9771dd00c49f43563 --- /dev/null +++ b/tests/test_xmlrpc/test_dns_realmdomains_integration.py @@ -0,0 +1,168 @@ +# Authors: +# Ana Krivokapic <akriv...@redhat.com> +# +# Copyright (C) 2013 Red Hat +# see file 'COPYING' for use and warranty information +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +""" +Test integration of DNS and realmdomains. +1. dnszone_{add,del} should create/delete appropriate entry in realmdomains. +2. realmdomains_mod should add a _kerberos TXT record in the DNS zone. +""" + +from ipalib import api, errors +from ipapython.dn import DN +from tests.test_xmlrpc import objectclasses +from xmlrpc_test import Declarative, fuzzy_digits + + +cn = u'Realm Domains' +dn = DN(('cn', cn), ('cn', 'ipa'), ('cn', 'etc'), api.env.basedn) +our_domain = api.env.domain +dnszone_1 = u'dnszone.test' +dnszone_1_dn = DN(('idnsname', dnszone_1), api.env.container_dns, + api.env.basedn) +idnssoamname = u'ns1.%s.' % dnszone_1 +idnssoarname = u'root.%s.' % dnszone_1 +dnszone_2 = u'dnszone2.test' +dnszone_2_dn = DN(('idnsname', dnszone_2), api.env.container_dns, + api.env.basedn) + + +def assert_realmdomain_and_txt_record_present(response): + zone = response['value'] + + r = api.Command['realmdomains_show']() + assert zone in r['result']['associateddomain'] + + r = api.Command['dnsrecord_show'](zone, u'_kerberos') + assert api.env.realm in r['result']['txtrecord'] + + return True + + +def assert_realmdomain_and_txt_record_not_present(response): + zone = response['value'] + + r = api.Command['realmdomains_show']() + assert zone not in r['result']['associateddomain'] + + try: + api.Command['dnsrecord_show'](zone, u'_kerberos') + except errors.NotFound: + return True + + +class test_dns_realmdomains_integration(Declarative): + cleanup_commands = [ + ('realmdomains_mod', [], {'associateddomain': [our_domain]}), + ('dnszone_del', [dnszone_1, dnszone_2], {'continue': True}), + ] + + tests = [ + dict( + desc='Check realmdomain and TXT record get created ' + 'during dnszone_add', + command=( + 'dnszone_add', [dnszone_1], { + 'idnssoamname': idnssoamname, + 'idnssoarname': idnssoarname, + 'ip_address': u'1.2.3.4', + } + ), + expected={ + 'value': dnszone_1, + 'summary': None, + 'result': { + 'dn': dnszone_1_dn, + 'idnsname': [dnszone_1], + 'idnszoneactive': [u'TRUE'], + 'idnssoamname': [idnssoamname], + 'nsrecord': [idnssoamname], + 'idnssoarname': [idnssoarname], + 'idnssoaserial': [fuzzy_digits], + 'idnssoarefresh': [fuzzy_digits], + 'idnssoaretry': [fuzzy_digits], + 'idnssoaexpire': [fuzzy_digits], + 'idnssoaminimum': [fuzzy_digits], + 'idnsallowdynupdate': [u'FALSE'], + 'idnsupdatepolicy': [u'grant %(realm)s krb5-self * A; ' + u'grant %(realm)s krb5-self * AAAA; ' + u'grant %(realm)s krb5-self * SSHFP;' + % dict(realm=api.env.realm)], + 'idnsallowtransfer': [u'none;'], + 'idnsallowquery': [u'any;'], + 'objectclass': objectclasses.dnszone, + + }, + }, + extra_check=assert_realmdomain_and_txt_record_present, + ), + + dict( + desc='Check realmdomain and TXT record do not get created ' + 'during dnszone_add for forwarded zone', + command=( + 'dnszone_add', [dnszone_2], { + 'idnssoamname': idnssoamname, + 'idnssoarname': idnssoarname, + 'idnsforwarders': u'1.2.3.4', + 'idnsforwardpolicy': u'only', + 'force': True, + } + ), + expected={ + 'value': dnszone_2, + 'summary': None, + 'result': { + 'dn': dnszone_2_dn, + 'idnsname': [dnszone_2], + 'idnszoneactive': [u'TRUE'], + 'idnssoamname': [idnssoamname], + 'idnsforwarders': [u'1.2.3.4'], + 'idnsforwardpolicy': [u'only'], + 'nsrecord': [idnssoamname], + 'idnssoarname': [idnssoarname], + 'idnssoaserial': [fuzzy_digits], + 'idnssoarefresh': [fuzzy_digits], + 'idnssoaretry': [fuzzy_digits], + 'idnssoaexpire': [fuzzy_digits], + 'idnssoaminimum': [fuzzy_digits], + 'idnsallowdynupdate': [u'FALSE'], + 'idnsupdatepolicy': [u'grant %(realm)s krb5-self * A; ' + u'grant %(realm)s krb5-self * AAAA; ' + u'grant %(realm)s krb5-self * SSHFP;' + % dict(realm=api.env.realm)], + 'idnsallowtransfer': [u'none;'], + 'idnsallowquery': [u'any;'], + 'objectclass': objectclasses.dnszone, + + }, + }, + extra_check=assert_realmdomain_and_txt_record_not_present, + ), + + dict( + desc='Check realmdomain and TXT record get deleted ' + 'during dnszone_del', + command=('dnszone_del', [dnszone_1], {}), + expected={ + 'value': dnszone_1, + 'summary': u'Deleted DNS zone "%s"' % dnszone_1, + 'result': {'failed': u''}, + }, + extra_check=assert_realmdomain_and_txt_record_not_present, + ), + ] -- 1.8.1.4
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel