On 07/13/2016 04:48 PM, Milan Kubík wrote:
On 07/11/2016 01:34 PM, Lenka Doudova wrote:



On 07/08/2016 02:24 PM, Milan Kubík wrote:
On 07/01/2016 05:13 PM, Lenka Doudova wrote:



On 07/01/2016 02:42 PM, Milan Kubík wrote:
On 06/16/2016 03:23 PM, Lenka Doudova wrote:
Hi,

attached are tests for authentication indicators. Please note:

1. newly created service tracker is not exactly complete, list of unimplemented methods is in doc. These methods can be filled in when existing declarative tests are refactored.

2. patch 0015 depends on 0014, so it should not be pushed without it.


Lenka




patch 0014:

In the update method, what happens when the updated attributes contain addattr? It is not clear to me. Is it necessary?

Example:
    ipa service-mod SRV --addattr="authind=radius"

Result:
The way the tracker works, this adds /u'addattr="authind=radius"'/ to the list of expected results (result of /self.attrs.update(updates)/. Of course nothing like that appears anywhere, so in case there's the /--addattr/ option, it's necessary to ensure it won't get to the /self.attrs/ atribute.

patch 0015:

host1 and service2 do not tell anything about the purpose of the fixture. Please assign more descriptive names to them. Why do the fixtures have 'function' scope? Does the service entry exist during the second and third test case?

Renamed.

patch 0016:

Per offline discussion, admin user has no special privileges here, LGTM.

--
Milan Kubik

Thanks for review, fixed patches (14.2 and 15.2) attached.
Lenka

NACK,

the update method of ServiceTracker creates the entry if it doesn't exist. Why? I know the base class has this problem also [1], though. Given this will be addressed, the fixtures in the xmlrpc test will fail since the fixture scope is wrong - function instead of class.

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

--
Milan Kubik
Hi,

fixed patch attached. I chose to leave the scope as it was (in case one test breaks and entry is not even created, the other tests can still be successful), but I removed the creation command from ServiceTracker update method and called it directly in the tests, so there shouldn't be hidden actions.

Lenka

Thanks for the updated patches. There are still some issues I haven't noticed before:

service tracker: track_create method doesn't set self.exists flag which is needed

In the xmlrpc test method `test_adding_second_indicator` uses 'addattr' to set the indicator, why?

--
Milan Kubik

Hi,
fixes for comments in attached patches.
After offline discussion, I removed the usage of "addattr" and replaced it with test to add two indicators in one command.

Lenka
From 1e69ce92786668d80507a02b03a3fae40d7bbbd2 Mon Sep 17 00:00:00 2001
From: Lenka Doudova <ldoud...@redhat.com>
Date: Wed, 15 Jun 2016 13:40:00 +0200
Subject: [PATCH 1/2] Tests: Tracker class for services

Provides basic service tracker, so far for purposes of [1].
Tracker is not complete, some methods will need to be added in case of service test refactoring.

[1] https://fedorahosted.org/freeipa/ticket/433
---
 ipatests/test_xmlrpc/tracker/service_plugin.py | 173 +++++++++++++++++++++++++
 1 file changed, 173 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py

diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py
new file mode 100644
index 0000000000000000000000000000000000000000..a7eb4303241e136ec88c51642f51fe703b16de7e
--- /dev/null
+++ b/ipatests/test_xmlrpc/tracker/service_plugin.py
@@ -0,0 +1,173 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+import six
+
+from ipalib import api
+from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
+from ipatests.test_xmlrpc import objectclasses
+from ipatests.util import assert_deepequal
+from ipapython.dn import DN
+
+if six.PY3:
+    unicode = str
+
+
+class ServiceTracker(Tracker):
+    """
+    Tracker class for service plugin
+
+    So far does not include methods for these commands:
+        service-add-host
+        service-remove-host
+        service-allow-retrieve-keytab
+        service-disallow-retrieve-keytab
+        service-allow-create-keytab
+        service-disallow-create-keytab
+        service-disable
+        service-add-cert
+        service-remove-cert
+    """
+
+    retrieve_keys = {
+        u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab',
+        u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject',
+        u'managedby', u'serial_number', u'serial_number_hex', u'issuer',
+        u'valid_not_before', u'valid_not_after', u'md5_fingerprint',
+        u'sha1_fingerprint', u'krbprincipalauthind', u'managedby_host',
+        u'krbcanonicalname'}
+    retrieve_all_keys = retrieve_keys | {
+        u'ipaKrbPrincipalAlias', u'ipaUniqueID', u'krbExtraData',
+        u'krbLastPwdChange', u'krbLoginFailedCount', u'memberof',
+        u'objectClass', u'ipakrbrequirespreauth',
+        u'ipakrbokasdelegate'}
+
+    create_keys = (retrieve_keys | {u'objectclass', u'ipauniqueid'}) - {
+        u'usercertificate', u'has_keytab'}
+    update_keys = retrieve_keys - {u'dn'}
+
+    def __init__(self, name, host_fqdn, options):
+        super(ServiceTracker, self).__init__(default_version=None)
+        self._name = "{0}/{1}@{2}".format(name, host_fqdn, api.env.realm)
+        self.dn = DN(
+            ('krbprincipalname', self.name), api.env.container_service,
+            api.env.basedn)
+        self.host_fqdn = host_fqdn
+        self.options = options
+
+    @property
+    def name(self):
+        return self._name
+
+    def make_create_command(self, force=True):
+        """ Make function that creates a service """
+        return self.make_command('service_add', self.name,
+                                 force=force, **self.options)
+
+    def make_delete_command(self):
+        """ Make function that deletes a service """
+        return self.make_command('service_del', self.name)
+
+    def make_retrieve_command(self, all=False, raw=False):
+        """ Make function that retrieves a service """
+        return self.make_command('service_show', self.name, all=all)
+
+    def make_find_command(self, *args, **kwargs):
+        """ Make function that searches for a service"""
+        return self.make_command('service_find', *args, **kwargs)
+
+    def make_update_command(self, updates):
+        """ Make function that updates a service """
+
+        return self.make_command('service_mod', self.name, **updates)
+
+    def track_create(self, **options):
+        """ Update expected state for service creation """
+        self.attrs = {
+            u'dn': self.dn,
+            u'krbprincipalname': [u'{0}'.format(self.name)],
+            u'objectclass': objectclasses.service,
+            u'ipauniqueid': [fuzzy_uuid],
+            u'managedby_host': [self.host_fqdn],
+            u'krbcanonicalname': [u'{0}'.format(self.name)]
+        }
+
+        for key in self.options:
+            self.attrs[key] = [self.options[key]]
+
+        self.exists = True
+
+    def check_create(self, result):
+        """ Check service-add command result """
+        assert_deepequal({
+            u'value': u'{0}'.format(self.name),
+            u'summary': u'Added service "{0}"'.format(self.name),
+            u'result': self.filter_attrs(self.create_keys)
+            }, result)
+
+    def check_delete(self, result):
+        """ Check service-del command result """
+        assert_deepequal({
+            u'value': [u'{0}'.format(self.name)],
+            u'summary': u'Deleted service "{0}"'.format(self.name),
+            u'result': {u'failed': []}
+            }, result)
+
+    def check_retrieve(self, result, all=False, raw=False):
+        """ Check service-show command result """
+        if all:
+            expected = self.filter_attrs(self.retrieve_all_keys)
+        else:
+            expected = self.filter_attrs(self.retrieve_keys)
+
+        assert_deepequal({
+            u'value': u'{0}'.format(self.name),
+            u'summary': None,
+            u'result': expected,
+        }, result)
+
+    def check_find(self, result, all=False, raw=False):
+        """ Check service-find command result """
+        if all:
+            expected = self.filter_attrs(self.retrieve_all_keys)
+        else:
+            expected = self.filter_attrs(self.retrieve_keys)
+
+        assert_deepequal({
+            u'count': 1,
+            u'truncated': False,
+            u'summary': u'1 service matched',
+            u'result': [expected]
+            }, result)
+
+    def check_update(self, result, extra_keys=()):
+        """ Check service-mod command result """
+        assert_deepequal({
+            u'value': u'{0}'.format(self.name),
+            u'summary': u'Modified service "{0}"'.format(self.name),
+            u'result': self.filter_attrs(self.update_keys | set(extra_keys))
+            }, result)
+
+    def update(self, updates, expected_updates=None):
+        """Helper function to update service and check the result
+
+        The ``updates`` are used as options to the *_mod command,
+        and the self.attrs is updated with this dict.
+        Additionally, self.attrs is updated with ``expected_updates``.
+        """
+        if expected_updates is None:
+            expected_updates = {}
+
+        command = self.make_update_command(updates)
+        result = command()
+        self.attrs.update(updates)
+        self.attrs.update(expected_updates)
+        for key, value in self.attrs.items():
+            if value is None:
+                del self.attrs[key]
+
+        self.check_update(result, extra_keys=set(updates.keys()) |
+                          set(expected_updates.keys()))
-- 
2.7.4

From c50301079954c9dc1f98d4b2ee9af924cbb9429a Mon Sep 17 00:00:00 2001
From: Lenka Doudova <ldoud...@redhat.com>
Date: Wed, 15 Jun 2016 13:44:16 +0200
Subject: [PATCH 2/2] Tests: Authentication indicators xmlrpc tests

https://fedorahosted.org/freeipa/ticket/433
---
 ipatests/test_xmlrpc/test_service_plugin.py | 45 ++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/test_service_plugin.py b/ipatests/test_xmlrpc/test_service_plugin.py
index 54ae55963c5ba45fb36dd6bb37416e33e9e49aa1..56e2c7a7a0570cb15dc5d27dcdba488da089350f 100644
--- a/ipatests/test_xmlrpc/test_service_plugin.py
+++ b/ipatests/test_xmlrpc/test_service_plugin.py
@@ -24,10 +24,14 @@ Test the `ipaserver/plugins/service.py` module.
 from ipalib import api, errors, x509
 from ipatests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid, fuzzy_hash
 from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_digits, fuzzy_date, fuzzy_issuer
-from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_hex
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_hex, XMLRPC_test
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.test_xmlrpc.testcert import get_testcert
 from ipatests.test_xmlrpc.test_user_plugin import get_user_result, get_group_dn
+
+from ipatests.test_xmlrpc.tracker.service_plugin import ServiceTracker
+from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
+
 import base64
 from ipapython.dn import DN
 import pytest
@@ -1262,3 +1266,42 @@ class test_service_allowed_to(Declarative):
             ),
         ),
     ]
+
+
+@pytest.fixture(scope='function')
+def indicators_host(request):
+    tracker = HostTracker(name=u'testhost1', fqdn=fqdn1)
+    return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='function')
+def indicators_service(request):
+    tracker = ServiceTracker(
+        name=u'SRV1', host_fqdn=fqdn1, options={
+            u'krbprincipalauthind': u'otp'})
+    return tracker.make_fixture(request)
+
+
+@pytest.mark.tier1
+class TestAuthenticationIndicators(XMLRPC_test):
+    def test_create_service_with_otp_indicator(
+            self, indicators_host, indicators_service):
+        """ Since range of authentication indicator values is not limited,
+        only 'otp' option is tested """
+        indicators_host.create()
+        indicators_service.create()
+
+    def test_adding_second_indicator(
+            self, indicators_host, indicators_service):
+        indicators_host.create()
+        indicators_service.create()
+        indicators_service.update(
+            updates={u'krbprincipalauthind': [u'otp', u'radius']})
+
+    def test_update_indicator(self, indicators_host, indicators_service):
+        indicators_host.create()
+        indicators_service.create()
+        indicators_service.update(
+            updates={u'krbprincipalauthind': u'radius'},
+            expected_updates={u'krbprincipalauthind': [u'radius']}
+        )
-- 
2.7.4

-- 
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