Hi Martin,

The patch was updated according to your suggestions. A separate patch removing outdated tests is attached.


On 07/08/2016 02:10 PM, Martin Basti wrote:


On 07.07.2016 08:09, Oleg Fayans wrote:
Updated version of the patch is attached with the failing tests marked
as xfailed (let's make the jenkins green).

On 07/04/2016 10:50 PM, Oleg Fayans wrote:
2 out of 7 tests currently fail due to a known issue [1], others pass.

[1] https://fedorahosted.org/freeipa/ticket/6029







This is wrong:

1)
you are not getting SSHFP records, just SSH public key (with your changes)

2)
you are using host-find without any arguments, so it will returns SSH
key for all hosts, the code before was getting SSHFP only for one host.
Would be better to use host-show?

3)
you actually found a bug, because host-find and host-show should print
only SSH fingerprints not SSH keys
https://fedorahosted.org/freeipa/ticket/6042
https://fedorahosted.org/freeipa/ticket/6043

4)
don't call it SSHFP records in code, because it is not DNS related,
probably you want to get SSH fingerprints instead of SSH keys

5)
It may contain multiple SSH keys, you always return only the first (the
original code returns all values)

     def get_sshfp_record(self):
-        sshfp_record = ''
-        client_host = self.clients[0].hostname.split('.')[0]
-
          result = self.master.run_command(
-            ['ipa', 'dnsrecord-show', self.master.domain.name, client_host]
+            ['ipa', 'host-find']
          )
-
-        lines = result.stdout_text.splitlines()
-        for line in lines:
-            if 'SSHFP record:' in line:
-                sshfp_record = line.replace('SSHFP record:', '').strip()
-
-        assert sshfp_record, 'SSHFP record not found'
-
-        sshfp_record = set(sshfp_record.split(', '))
-        self.log.debug("SSHFP record for host %s: %s", client_host,
str(sshfp_record))
-
-        return sshfp_record
+        records = result.stdout_text.split('\n\n')
+        sshkey_re = re.compile('.+SSH public key: ssh-\w+ (\S+?),.+')
+        for hostrecord in records:
+            if self.clients[0].hostname in hostrecord:
+                sshfps = sshkey_re.findall(hostrecord)
+                assert sshfps, 'SSHFP record not found'
+                sshfp = sshfps[0]
+        return sshfp



--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From d5e6dd5ab115a10a8a504f4f0c5b3117cdbc0176 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Tue, 26 Jul 2016 15:06:41 +0200
Subject: [PATCH] Removed outdated reenrollment tests

https://fedorahosted.org/freeipa/ticket/6029
---
 .../test_forced_client_reenrollment.py             | 38 +++-------------------
 1 file changed, 5 insertions(+), 33 deletions(-)

diff --git a/ipatests/test_integration/test_forced_client_reenrollment.py b/ipatests/test_integration/test_forced_client_reenrollment.py
index d430a98e74450f44eac286ac0ad35a5aee7cc602..1ea57a871b0830f9afa18de8029739cae8115a49 100644
--- a/ipatests/test_integration/test_forced_client_reenrollment.py
+++ b/ipatests/test_integration/test_forced_client_reenrollment.py
@@ -58,42 +58,14 @@ class TestForcedClientReenrollment(IntegrationTest):
 
     def test_reenroll_with_keytab(self, client):
         """
-        Client re-enrollment using keytab
+        Client re-enrollment using keytab: the old keytab should be invalid,
+        see https://fedorahosted.org/freeipa/ticket/6029 for reasoning
         """
         self.backup_keytab()
-        sshfp_record_pre = self.get_sshfp_record()
-        self.restore_client()
-        self.check_client_host_entry()
+        self.uninstall_client()
+        self.check_client_host_entry(enabled=False)
         self.restore_keytab()
-        self.reenroll_client(keytab=self.BACKUP_KEYTAB)
-        sshfp_record_post = self.get_sshfp_record()
-        assert sshfp_record_pre == sshfp_record_post
-
-    def test_reenroll_with_both_force_join_and_keytab(self, client):
-        """
-        Client re-enrollment using both --force-join and --keytab options
-        """
-        self.backup_keytab()
-        sshfp_record_pre = self.get_sshfp_record()
-        self.restore_client()
-        self.check_client_host_entry()
-        self.restore_keytab()
-        self.reenroll_client(force_join=True, keytab=self.BACKUP_KEYTAB)
-        sshfp_record_post = self.get_sshfp_record()
-        assert sshfp_record_pre == sshfp_record_post
-
-    def test_reenroll_to_replica(self, client):
-        """
-        Client re-enrollment using keytab, to a replica
-        """
-        self.backup_keytab()
-        sshfp_record_pre = self.get_sshfp_record()
-        self.restore_client()
-        self.check_client_host_entry()
-        self.restore_keytab()
-        self.reenroll_client(keytab=self.BACKUP_KEYTAB, to_replica=True)
-        sshfp_record_post = self.get_sshfp_record()
-        assert sshfp_record_pre == sshfp_record_post
+        self.reenroll_client(keytab=self.BACKUP_KEYTAB, expect_fail=True)
 
     def test_try_to_reenroll_with_disabled_host(self, client):
         """
-- 
1.8.3.1

From 0e03377a9148a20371416cc4e890a32c1cd00856 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Tue, 26 Jul 2016 15:13:30 +0200
Subject: [PATCH] Updated forced_client_reenrollment test

With current implementation the client host record stays in master's
ldap after client uninstallation in a common way so there is no need to dance
around with turning iptables on and off.
Related ticket:

https://fedorahosted.org/freeipa/ticket/6042
---
 .../test_forced_client_reenrollment.py             | 89 ++++++----------------
 1 file changed, 23 insertions(+), 66 deletions(-)

diff --git a/ipatests/test_integration/test_forced_client_reenrollment.py b/ipatests/test_integration/test_forced_client_reenrollment.py
index 1ea57a871b0830f9afa18de8029739cae8115a49..4277488000a8d37fdbfae626d8a138f214735eea 100644
--- a/ipatests/test_integration/test_forced_client_reenrollment.py
+++ b/ipatests/test_integration/test_forced_client_reenrollment.py
@@ -17,6 +17,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 import os
+import re
 import subprocess
 from ipaplatform.paths import paths
 import pytest
@@ -49,12 +50,12 @@ class TestForcedClientReenrollment(IntegrationTest):
         """
         Client re-enrollment using admin credentials (--force-join)
         """
-        sshfp_record_pre = self.get_sshfp_record()
-        self.restore_client()
-        self.check_client_host_entry()
+        sshfp_records_pre = self.get_sshfps(self.clients[0])
+        self.uninstall_client()
+        self.check_client_host_entry(enabled=False)
         self.reenroll_client(force_join=True)
-        sshfp_record_post = self.get_sshfp_record()
-        assert sshfp_record_pre == sshfp_record_post
+        sshfp_records_post = self.get_sshfps(self.clients[0])
+        assert sshfp_records_pre == sshfp_records_post
 
     def test_reenroll_with_keytab(self, client):
         """
@@ -73,7 +74,7 @@ class TestForcedClientReenrollment(IntegrationTest):
         """
         self.backup_keytab()
         self.disable_client_host_entry()
-        self.restore_client()
+        self.uninstall_client()
         self.check_client_host_entry(enabled=False)
         self.restore_keytab()
         self.reenroll_client(keytab=self.BACKUP_KEYTAB, expect_fail=True)
@@ -84,7 +85,6 @@ class TestForcedClientReenrollment(IntegrationTest):
         """
         self.backup_keytab()
         self.uninstall_client()
-        self.restore_client()
         self.check_client_host_entry(enabled=False)
         self.restore_keytab()
         self.reenroll_client(keytab=self.BACKUP_KEYTAB, expect_fail=True)
@@ -95,7 +95,7 @@ class TestForcedClientReenrollment(IntegrationTest):
         """
         self.backup_keytab()
         self.delete_client_host_entry()
-        self.restore_client()
+        self.uninstall_client()
         self.check_client_host_entry(not_found=True)
         self.restore_keytab()
         self.reenroll_client(keytab=self.BACKUP_KEYTAB, expect_fail=True)
@@ -108,45 +108,16 @@ class TestForcedClientReenrollment(IntegrationTest):
             self.clients[0].config.test_dir,
             'empty.keytab'
         )
-        self.restore_client()
-        self.check_client_host_entry()
+        self.uninstall_client()
+        self.check_client_host_entry(enabled=False)
         self.clients[0].run_command(['touch', EMPTY_KEYTAB])
         self.reenroll_client(keytab=EMPTY_KEYTAB, expect_fail=True)
 
-    def uninstall_client(self):
-        self.clients[0].run_command(
-            ['ipa-client-install', '--uninstall', '-U'],
-            set_env=False,
-            raiseonerr=False
-        )
-
-    def restore_client(self):
-        client = self.clients[0]
-
-        client.run_command([
-            'iptables',
-            '-A', 'INPUT',
-            '-j', 'ACCEPT',
-            '-p', 'tcp',
-            '--dport', '22'
-        ])
-        client.run_command([
-            'iptables',
-            '-A', 'INPUT',
-            '-j', 'REJECT',
-            '-p', 'all',
-            '--source', self.master.ip
-        ])
-        self.uninstall_client()
-        client.run_command(['iptables', '-F'])
-
     def reenroll_client(self, keytab=None, to_replica=False, force_join=False,
                         expect_fail=False):
         server = self.replicas[0] if to_replica else self.master
         client = self.clients[0]
 
-        self.fix_resolv_conf(client, server)
-
         args = [
             'ipa-client-install', '-U',
             '--server', server.hostname,
@@ -209,25 +180,17 @@ class TestForcedClientReenrollment(IntegrationTest):
             if e.returncode != 2:
                 raise
 
-    def get_sshfp_record(self):
-        sshfp_record = ''
-        client_host = self.clients[0].hostname.split('.')[0]
-
+    def get_sshfps(self, host):
         result = self.master.run_command(
-            ['ipa', 'dnsrecord-show', self.master.domain.name, client_host]
+            ['ipa', 'host-show', host.hostname]
         )
-
-        lines = result.stdout_text.splitlines()
-        for line in lines:
-            if 'SSHFP record:' in line:
-                sshfp_record = line.replace('SSHFP record:', '').strip()
-
-        assert sshfp_record, 'SSHFP record not found'
-
-        sshfp_record = set(sshfp_record.split(', '))
-        self.log.debug("SSHFP record for host %s: %s", client_host, str(sshfp_record))
-
-        return sshfp_record
+        records = result.stdout_text.split('\n\n')
+        sshfp_re = re.compile('(?:\w{2}\:){15}\w{2}')
+        for hostrecord in records:
+            if self.clients[0].hostname in hostrecord:
+                sshfps = sshfp_re.findall(hostrecord)
+                assert sshfps, 'SSHFP record not found'
+        return set(sshfps)
 
     def backup_keytab(self):
         contents = self.clients[0].get_file_contents(CLIENT_KEYTAB)
@@ -237,16 +200,10 @@ class TestForcedClientReenrollment(IntegrationTest):
         contents = self.master.get_file_contents(self.BACKUP_KEYTAB)
         self.clients[0].put_file_contents(self.BACKUP_KEYTAB, contents)
 
-    def fix_resolv_conf(self, client, server):
-        """
-        Put server's ip address at the top of resolv.conf
-        """
-        contents = client.get_file_contents(paths.RESOLV_CONF)
-        nameserver = 'nameserver %s\n' % server.ip
-
-        if not contents.startswith(nameserver):
-            contents = nameserver + contents.replace(nameserver, '')
-            client.put_file_contents(paths.RESOLV_CONF, contents)
+    def uninstall_client(self):
+        self.clients[0].run_command(['ipa-client-install',
+                                     '--uninstall', '-U'],
+                                    raiseonerr=False)
 
 
 @pytest.fixture()
-- 
1.8.3.1

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to