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

Reply via email to