URL: https://github.com/freeipa/freeipa/pull/1628
Author: tiran
 Title: #1628: [Backport][ipa-4-6] ipa host-add: do not raise exception when 
reverse record not added
Action: opened

PR body:
"""
This PR was opened automatically because PR #1521 was pushed to master and 
backport to ipa-4-6 is required.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/1628/head:pr1628
git checkout pr1628
From 6f8f60801e05b227e05ab557f10301d6a25fbe03 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 | 42 ++++++++++++++++++++++++++------
 3 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index 2e44da3a27..9e2c990d6d 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 291a90a2ab..306105d67a 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 0acb6cf100..50b1cf6659 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
@@ -646,11 +646,13 @@ def test_create_host_with_ip(self, dns_setup_nonameserver, host4):
         """
         try:
             command = host4.make_create_command()
-            with raises_exact(errors.NonFatalError(
-                reason=u'The host was added but the DNS update failed with'
-                    ': All nameservers failed to answer the query for DNS '
-                    'reverse zone %s' % missingrevzone)):
-                command(ip_address=ipv4_in_missingrevzone_ip)
+            result = command(ip_address=ipv4_in_missingrevzone_ip)
+            msg = result['messages'][0]
+            assert msg['code'] == messages.FailedToAddHostDNSRecords.errno
+            expected_msg = ("The host was added but the DNS update failed "
+                            "with: All nameservers failed to answer the query "
+                            "for DNS reverse zone {}").format(missingrevzone)
+            assert msg['message'] == expected_msg
             # Make sure the host is added
             host4.run_command('host_show', host4.fqdn)
         finally:
@@ -658,7 +660,33 @@ def test_create_host_with_ip(self, dns_setup_nonameserver, host4):
             command = host4.make_delete_command()
             try:
                 command(updatedns=True)
-            except Exception:
+            except errors.NotFound:
+                pass
+
+    def test_create_host_with_otp(self, dns_setup_nonameserver, host4):
+        """
+        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
+            assert msg['message'].startswith(
+                u'The host was added but the DNS update failed with:')
+        finally:
+            # Cleanup
+            try:
+                command = host4.make_delete_command()
+                command(updatedns=True)
+            except errors.NotFound:
                 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