URL: https://github.com/freeipa/freeipa/pull/1521 Author: flo-renaud Title: #1521: ipa host-add: do not raise exception when reverse record not added Action: opened
PR body: """ When ipa host-add --random is unable to add a reverse record (for instance because the server does not manage any reverse zone), the command adds the host but exits (return code=1) with an error without actually outputing the random password generated. With this fix, the behavior is modified. The commands succeeds (return code=0) but prints a warning. This commit also adds a unit test. https://pagure.io/freeipa/issue/7374 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/1521/head:pr1521 git checkout pr1521
From af90a5a4cc4eb233de529a2f016e6dfb6022c3b4 Mon Sep 17 00:00:00 2001 From: Florence Blanc-Renaud <f...@redhat.com> Date: Tue, 30 Jan 2018 12:49:35 +0100 Subject: [PATCH] ipa host-add: do not raise exception when reverse record not added When ipa host-add --random is unable to add a reverse record (for instance because the server does not manage any reverse zone), the command adds the host but exits (return code=1) with an error without actually outputing the random password generated. With this fix, the behavior is modified. The commands succeeds (return code=0) but prints a warning. This commit also adds a unit test. https://pagure.io/freeipa/issue/7374 --- ipalib/messages.py | 11 ++++++++++ ipaserver/plugins/host.py | 10 +++------ ipatests/test_xmlrpc/test_host_plugin.py | 37 +++++++++++++++++++++++++++++++- 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/ipalib/messages.py b/ipalib/messages.py index fd458a1757..7c699ce1d1 100644 --- a/ipalib/messages.py +++ b/ipalib/messages.py @@ -476,6 +476,17 @@ class CertificateInvalid(PublicMessage): "%(reason)s") +class FailedToAddHostDNSRecords(PublicMessage): + """ + **13030** Failed to add host DNS records + """ + + errno = 13030 + type = "warning" + format = _("The host was added but the DNS update failed with: " + "(%(reason)s)") + + def iter_messages(variables, base): """Return a tuple with all subclasses """ diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py index 6487cd612d..f7e23753c4 100644 --- a/ipaserver/plugins/host.py +++ b/ipaserver/plugins/host.py @@ -700,7 +700,6 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): def post_callback(self, ldap, dn, entry_attrs, *keys, **options): assert isinstance(dn, DN) - exc = None if dns_container_exists(ldap): try: parts = keys[-1].split('.') @@ -719,18 +718,15 @@ def post_callback(self, ldap, dn, entry_attrs, *keys, **options): update_sshfp_record(domain, unicode(parts[0]), entry_attrs) except Exception as e: - exc = e + self.add_message(messages.FailedToAddHostDNSRecords(reason=e)) if options.get('random', False): try: - entry_attrs['randompassword'] = unicode(getattr(context, 'randompassword')) + entry_attrs['randompassword'] = unicode( + getattr(context, 'randompassword')) except AttributeError: # On the off-chance some other extension deletes this from the # context, don't crash. pass - if exc: - raise errors.NonFatalError( - reason=_('The host was added but the DNS update failed with: %(exc)s') % dict(exc=exc) - ) set_certificate_attrs(entry_attrs) set_kerberos_attrs(entry_attrs, options) rename_ipaallowedtoperform_from_ldap(entry_attrs, options) diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py index e0df6bc04c..c99af6f221 100644 --- a/ipatests/test_xmlrpc/test_host_plugin.py +++ b/ipatests/test_xmlrpc/test_host_plugin.py @@ -31,7 +31,7 @@ import pytest from ipapython import ipautil -from ipalib import api, errors +from ipalib import api, errors, messages from ipapython.dn import DN from ipapython.dnsutil import DNSName from ipatests.test_util import yield_fixture @@ -100,6 +100,9 @@ host_cert = get_testcert(DN(('CN', api.env.host), subject_base()), 'host/%s@%s' % (api.env.host, api.env.realm)) +missingrevzone = u'30.16.172.in-addr.arpa.' +ipv4_in_missingrevzone_ip = u'172.16.30.22' + @pytest.fixture(scope='class') def host(request): @@ -119,6 +122,12 @@ def host3(request): return tracker.make_fixture(request) +@pytest.fixture(scope='class') +def host4(request): + tracker = HostTracker(name=u'testhost4') + return tracker.make_fixture(request) + + @pytest.fixture(scope='class') def lab_host(request): name = u'testhost1' @@ -590,10 +599,36 @@ def test_join_host(self, host, keytabname): command() +@pytest.mark.tier1 +class TestHostMissingRevZone(XMLRPC_test): + + def test_create_host_with_otp(self, host4, keytabname): + """ + Create a test host specifying an IP address for which + IPA does not handle the reverse zone, and requesting + the creation of a random password. + Non-reg test for ticket 7374. + """ + + command = host4.make_create_command() + try: + result = command(random=True, ip_address=ipv4_in_missingrevzone_ip) + # Make sure a random password is returned + assert result['result']['randompassword'] + # Make sure the warning about missing DNS record is added + msg = result['messages'][0] + assert msg['code'] == messages.FailedToAddHostDNSRecords.errno + finally: + # Cleanup + command = host4.make_delete_command() + command(updatedns=True) + + @yield_fixture(scope='class') def dns_setup(host): try: host.run_command('dnszone_del', dnszone, revzone, revipv6zone, + missingrevzone, **{'continue': True}) except (errors.NotFound, errors.EmptyModlist): pass
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org