Refactoring: LDAP Connection Management #145

tkrizek · 2016-10-07T14:42:32Z

Design doc: http://www.freeipa.org/page/V4/LDAP_Connection_Management_Refactoring

Many changes in current PR are only temporary for easier refactoring, but feedback is welcome.

tbordaz · 2016-10-10T13:30:36Z

ipaserver/install/dsinstance.py

@@ -265,12 +261,14 @@ def __common_setup(self, enable_ssl=False):
        self.step("creating directory server instance", self.__create_instance)
        self.step("updating configuration in dse.ldif", self.__update_dse_ldif)
        self.step("restarting directory server", self.__restart_instance)
+        self.step("enabling ldapi", self.__enable_ldapi)


Hello,

Isn't it possible to enable ldapi+autobind before the 'line 263' restart ?
I would save a DS restart

Agree, I'd like to avoid additional DS restart as much as possible

I've tried to do that, but I haven't been able to connect to LDAP before the first restart happens.

ok. Sorry I thought that all steps were done through ldap connection but they were done by editing dse.ldif. So there was no already established connection.
I wonder if after __create_instance it wouldn't be possible to open a connection (LDAPClient(ldap://fqdn:389)) and bind using 'directory manager' password. So that enable ldapi, etc.. could be done via ldap:389 so that only one restart would be possible.

You're right, it is possible to connect to ldap right after the instance is created. I didn't notice the service is stopped before updating the dse. Thanks, fixed in next synchronization.

MartinBasti · 2016-10-10T15:51:44Z

Please fix PEP8

./ipapython/ipaldap.py:766:49: E231 missing whitespace after ','
./ipapython/ipaldap.py:822:9: E265 block comment should start with '# '
./ipaserver/install/bindinstance.py:231:1: E302 expected 2 blank lines, found 1
./ipaserver/install/bindinstance.py:231:80: E501 line too long (82 > 79 characters)
./ipaserver/install/krbinstance.py:170:80: E501 line too long (89 > 79 characters)
./ipaserver/install/ldapupdate.py:59:80: E501 line too long (85 > 79 characters)
./ipaserver/install/replication.py:218:80: E501 line too long (82 > 79 characters)

MartinBasti · 2016-10-10T16:12:59Z

ipapython/ipaldap.py

@@ -1639,48 +1636,25 @@ def __init__(self, host='', port=389, cacert=None, debug=None, ldapi=False,
    def __str__(self):
        return self.host + ":" + str(self.port)

-    def __wait_for_connection(self, timeout):


LGTM
https://fedorahosted.org/freeipa/ticket/2175

Upgrade is using ldapi now.

However I'm curious if there is a time delay between DS is listening on ldapi socket and when DS is listening on 389 port. But in our case we plan to use just ldapi, so it should be ok.

MartinBasti · 2016-10-10T16:28:18Z

ipapython/ipaldap.py

@@ -704,7 +704,7 @@ class LDAPClient(object):
    time_limit = -1.0   # unlimited
    size_limit = 0      # unlimited

-    def __init__(self, ldap_uri, start_tls=False, force_schema_updates=False,
+    def __init__(self, ldap_uri, start_tls=False, force_schema_updates=True,


Please add reason for this change into commit message

Fixed in next sync.

MartinBasti · 2016-10-10T16:30:54Z

ipapython/ipaldap.py

+            return 'ldap'
+
+    def __init__(self, host='', start_tls=False,
+                 force_schema_updates=True, no_schema=False,


I think this comment should be in previous patch

Fixed in next sync.

MartinBasti · 2016-10-10T16:38:39Z

ipaserver/install/dsinstance.py

@@ -265,12 +261,14 @@ def __common_setup(self, enable_ssl=False):
        self.step("creating directory server instance", self.__create_instance)
        self.step("updating configuration in dse.ldif", self.__update_dse_ldif)
        self.step("restarting directory server", self.__restart_instance)
+        self.step("enabling ldapi", self.__enable_ldapi)


Agree, I'd like to avoid additional DS restart as much as possible

MartinBasti · 2016-10-10T16:42:54Z

ipaserver/install/service.py

@@ -157,7 +158,8 @@ def ldap_disconnect(self):
        LDAPConnectionManager.close_connection()
        self.admin_conn = None

-    def _ldap_mod(self, ldif, sub_dict=None, raise_on_err=True):
+    def _ldap_mod(self, ldif, sub_dict=None, raise_on_err=True,


I don't like to ahve so much options, can we split this into separate method _simple_ldap_mod(), nonldapi, or something

I'm not convinced this is a worthwhile change. But perhaps we could eliminate the ldap_uri, because it seems the method connects to the localhost anyway. But I'm not sure this is always the case, since the method uses ldap_uri from admin_conn, which could, theoretically, have different ldap_uri.

MartinBasti · 2016-10-10T16:47:14Z

ipapython/ipaldap.py

@@ -1656,3 +1656,27 @@ def entry_exists(self, dn):
            return False
        else:
            return True
+
+
+class LDAPConnectionManager(object):


Because this is related only to ldapi, I would rename it to LDAPI connection manager

MartinBasti · 2016-10-10T16:48:50Z

I did some inline comments, I'm not fully satisfied with implementation of the connection manager, I'll think about it tomorrow.

tkrizek · 2016-10-14T11:47:22Z

I made some changes and removed the connection manager, as discussed with sub-team. The refactoring is not finished, but these changes can be pushed to master if it doesn't break any working tests. Jobs are currently pending in jenkins.

tkrizek · 2016-10-20T10:25:56Z

Please review the code. I fixed a problem that was identified with the previous set of integration tests, but I'm still waiting for results of this build. Nevertheless, no more changes should be introduced in this PR, only potential fixes of issues.

rcritten · 2016-10-20T13:05:09Z

Removing the connection timeout makes me a bit edgy. It is quite unclear why that's there, perhaps python-ldap doesn't provide a timeout and things hung forever, I'm not sure. I'd double-check that there is a timeout if the remote server either isn't there (probably raises a connection error), or is there but doesn't respond.

MartinBasti · 2016-10-25T16:20:29Z

ipaserver/install/server/replicainstall.py

+    env._bootstrap(context='installer', log=None)
+    env._finalize_core(**dict(constants.DEFAULT_CONFIG))
+
+    # pylint: disable=no-member


NACK: you disabled pylint globally until end of file, enable it later or put it to the right line

MartinBasti · 2016-10-25T16:23:25Z

ipaserver/install/dsinstance.py

@@ -670,7 +670,7 @@ def apply_updates(self):
            # very fatal errors only will raise exception
            raise RuntimeError("Update failed: %s" % e)
        installutils.store_version()
-
+        self.ldap_disconnect()


Why it is located here, I don't see DS restart in this function, IMO this disconnect should be as close to restart as possible (i.e. in the same function as restart)

MartinBasti · 2016-10-25T16:33:48Z

@rcritten Tomas removed timelimit that was used for repeated connections, it is not used for preventing hangs. (If we talk about the same commit 'ldap refactoring: remove wait/timeout during binds') We added this functionality to DS restart, restart will block code until DS is not ready on LDAPI port.

@tomaskrizek I put two inline comments there, otherwise changes make sense. I'll wait for tests results

HonzaCholasta · 2016-10-26T08:22:03Z

install/restart_scripts/restart_dirsrv

@@ -38,6 +38,8 @@ def _main():

    syslog.syslog(syslog.LOG_NOTICE, "certmonger restarted dirsrv instance '%s'" % instance)

+    api.Backend.ldap2.close()


On ldap2, .disconnect() should be called rather than .close().

HonzaCholasta · 2016-10-26T08:25:00Z

ipapython/ipaldap.py

+                self.conn.set_option(ldap.OPT_X_SASL_NOCANON, ldap.OPT_ON)
+
+            if start_tls:
+                self.conn.start_tls_s()


This block should either be moved to _connect(), or the rest of _connect() should be moved here.

tkrizek · 2016-11-01T17:49:14Z

In an offline discussion we decided not to push temporary changes to master. Here's the final code for review.

HonzaCholasta · 2016-11-02T06:24:31Z

ipaserver/plugins/ldap2.py

-        self.__time_limit = None
-        self.__size_limit = None
+        self.__time_limit = float(LDAPClient.time_limit)
+        self.__size_limit = float(LDAPClient.size_limit)


Size limit is an int, not float.

HonzaCholasta · 2016-11-02T06:30:02Z

ipapython/ipaldap.py

+                self.conn.start_tls_s()
+
+    @staticmethod
+    def IPAdmin_init(host='', port=389, cacert=None, debug=None,


This should be a function called IPAdmin rather than a static method of LDAPClient, that way you don't have to modify any of the existing code which calls IPAdmin.

Better solution in next push.

HonzaCholasta · 2016-11-02T06:34:00Z

pylintrc

@@ -27,6 +27,7 @@ disable=
    too-many-boolean-expressions,
    too-many-branches,
    too-many-instance-attributes,
+    too-many-function-args,


Why is this necessary?

IPAdmin_init has too many arguments. This check didn't trigger previously when it was in IPAdmin.__init__

No longer necessary.

HonzaCholasta · 2016-11-02T06:43:36Z

ipapython/ipaldap.py

@@ -1085,7 +1085,7 @@ def simple_bind(self, bind_dn=DN(('cn', 'directory manager')),
            self.conn.simple_bind_s(
                bind_dn, bind_password, server_controls, client_controls)

-    def external_bind(self, user_name, server_controls=None,
+    def external_bind(self, user_name=None, server_controls=None,


Remove the user_name argument altogether, as it always has to be set to the effective username.

HonzaCholasta · 2016-11-02T06:45:43Z

ipapython/ipaldap.py

@@ -1068,7 +1069,8 @@ def _connect(self):

        return conn

-    def simple_bind(self, bind_dn, bind_password, server_controls=None,
+    def simple_bind(self, bind_dn=DN(('cn', 'directory manager')),
+                    bind_password='', server_controls=None,


I would really prefer if both bind_dn and bind_password had to be always specified explicitly.

Removed default values and specified both explicitly in function calls.

HonzaCholasta · 2016-11-02T06:48:22Z

ipapython/ipaldap.py

+        if debug and debug.lower() == "on":
+            ldap.set_option(ldap.OPT_DEBUG_LEVEL, 255)
+        if cacert is not None:
+            ldap.set_option(ldap.OPT_X_TLS_CACERTFILE, cacert)


These are global options, you should not really touch them from connection-specific code.

When i tried to set OPT_X_TLS_CACERTFILE and OPT_X_TLS_REQUIRE_CERT directly to the ldap connection, ipadiscovery stopped working with the following error message

TLS error -8172:Peer's certificate issuer has been marked as not trusted by the user.

thus preventing ipa-client-install from successfully finishing.

Fixing this issue is out of the scope of this PR. This code was already present, I'm just moving it to some other place.

Merging this PR is a priority. Once that's done, feel free to investigate and submit a fix.

HonzaCholasta · 2016-11-02T06:55:06Z

ipapython/ipaldap.py

+        ldap_client.cacert = cacert
+        ldap_client.ldapi = ldapi
+        ldap_client.realm = realm
+        ldap_client.suffixes = {}


Are all of these really necessary? git grep shows me that .cacert, .realm and .suffixes are not used anywhere and .ldapi is used only in ipa-httpd-kdcproxy. That leaves us with .host and .port, which should preferrably be transformed into properties of LDAPClient rather than set here.

Moved host and port to LDAPClient. Removed the rest.

HonzaCholasta · 2016-11-02T07:00:01Z

ipaserver/install/dsinstance.py

@@ -257,14 +257,15 @@ def __common_setup(self, enable_ssl=False):

        self.step("creating directory server user", create_ds_user)
        self.step("creating directory server instance", self.__create_instance)
+        self.step("enabling ldapi", self.__enable_ldapi)
+        self.step("configure autobind for root", self.__root_autobind)


I would rather merge these into __update_dse_ldif, if possible.

That would be a nice change, as well as removing _ldap_mod() completely, but it's out of scope for this refactoring.

HonzaCholasta · 2016-11-02T07:06:01Z

ipaserver/install/dsinstance.py

+    def start(self, *args, **kwargs):
+        super(DsInstance, self).start(*args, **kwargs)
+        api.Backend.ldap2.connect(size_limit=LDAPClient.size_limit,
+                                  time_limit=LDAPClient.time_limit)


These are the default limits in ldap2. Why do you set them again here?

That's a temporary change that is refactored in a later commit, the final code calls api.Backend.ldap2.connect().

HonzaCholasta · 2016-11-02T07:07:37Z

ipaserver/plugins/ldap2.py

-        self.__size_limit = int(val)
+        if val is not None:
+            val = int(val)
+        self.__size_limit = val

    @size_limit.deleter


You should also update the deleters.

Nice catch.

HonzaCholasta · 2016-11-02T07:14:10Z

ipaserver/plugins/ldap2.py

@@ -150,7 +150,7 @@ def create_connection(
        Extends backend.Connectible.create_connection.
        """
        if bind_dn is None:
-            bind_dn = DN()
+            bind_dn = DN(('cn', 'directory manager'))


Actually, if bind_dn is None, simple bind should not be attempted.

Let's keep the default here and remove it in simple_bind() instead.

HonzaCholasta · 2016-11-02T07:16:27Z

ipaserver/install/replication.py

@@ -505,7 +505,7 @@ def setup_changelog(self, conn):
    def setup_chaining_backend(self, conn):
        chaindn = DN(('cn', 'chaining database'), ('cn', 'plugins'), ('cn', 'config'))
        benamebase = "chaindb"
-        urls = [self.to_ldap_url(conn)]
+        urls = [self.to_ldap_url()]


I would rather throw away self.to_ldap_url() and use conn.ldap_uri here.

HonzaCholasta · 2016-11-02T07:22:26Z

In addition to my inline comments:

use component name ("ipaldap", "ldap2", "install", ...) rather than "lda refactoring" as a prefix in commit subjects,
please move "ldap refactoring: change default time/size limit in ldap2" before "ldap refactoring: conn management in dsinstance" and squash it with "ldap refactoring: restore previous time/size limit in backend",
squash "ldap refactoring: add restart_dirsrv to installutils" and "ldap refactoring: use restart_dirsrv in installers",
maybe squash all of the "ldap refactoring: conn management in ipa-...", as it is a single change accross multiple scripts.

Testing whether it is possible to connect to directory server is already done in RedHatDirectoryService.restart(). https://fedorahosted.org/freeipa/ticket/6461

* Use LDAPClient.simple_bind instead of extra call to IPAdmin.do_simple_bind * Rename binddn to bind_dn * Rename bindpw to bind_password * Explicitly specify bind_dn in all calls https://fedorahosted.org/freeipa/ticket/6461

* Rename do_external_bind to external_bind * Remove user_name argument in external_bind() and always set it to effective user name https://fedorahosted.org/freeipa/ticket/6461

* Rename do_sasl_gssapi_bind to gssapi_bind https://fedorahosted.org/freeipa/ticket/6461

* move IPAdmin methods to LDAPClient * add extra arguments (cacert, sasl_nocanon) to LDAPClient.__init__() * add host, port, _protocol to LDAPClient (parsed from ldap_uri) * create get_ldap_uri() method to create ldap_uri from former IPAdmin.__init__() arguments * replace IPAdmin with LDAPClient + get_ldap_uri() * remove ununsed function argument hostname from enable_replication_version_checking() https://fedorahosted.org/freeipa/ticket/6461

Remove directory manager's password from service's constructors https://fedorahosted.org/freeipa/ticket/6461

* enable ldapi and root autobind early during the ds installation * perform these changes using simple_bind with dm_password https://fedorahosted.org/freeipa/ticket/6461

* read realm from config file * configure api.env to use ldapi genrated from realm https://fedorahosted.org/freeipa/ticket/6461

Installation of Certificate Server replica requires directory manager password. Specify it explicitly in function call and pass it in through an argument. https://fedorahosted.org/freeipa/ticket/6461

* Set default time_limit and size_limit in ldap2 to unlimited. * Set time_limit and size_limit to None in backend. This will respect ipaconfig values. https://fedorahosted.org/freeipa/ticket/6461

Connect and/or disconnect api.Backend.ldap2 connection when directory server is stopped/restarted. Checking is ldap2 connection is connected is neccesary for edge cases during ds installation (initial start). https://fedorahosted.org/freeipa/ticket/6461

connect/disconnect api.Backend.ldap2 connection when directory server is started/stopped https://fedorahosted.org/freeipa/ticket/6461

* Create a utility function to restart a directory server and reconnect the api.Backend.ldap2 connection. * Use restart_dirsrv instead of knownservices.dirsrv.restart to ensure api.Backend.ldap2 is reconnected. https://fedorahosted.org/freeipa/ticket/6461

Remove adhoc connects and disconnects of api.Backend.ldap2. Connection should be established only at the start of the script, destroyed at the end of the script and re-established when directory server is restarted. https://fedorahosted.org/freeipa/ticket/6461

* Move connect to the beggining of the uninstall_check and properly close the connection at the end of the script. * Connect to ldap in external CA installation (step2). https://fedorahosted.org/freeipa/ticket/6461

Diconnect the established connection oncee is it no longer needed. https://fedorahosted.org/freeipa/ticket/6461

Configure ldap connection in LDAPUpdate to use ldapi. https://fedorahosted.org/freeipa/ticket/6461

Remove ldap_connect and ldap_disconnect from services. admin_conn is just an alias to api.Backend.ldap2 and therefore the connection should be managed elsewhere. https://fedorahosted.org/freeipa/ticket/6461

Properly close ldap connection. https://fedorahosted.org/freeipa/ticket/6461

Set default bind_dn to cn=directory manager. https://fedorahosted.org/freeipa/ticket/6461

* ipca-ca-install: Use a single ldap connection for the entire script. Connecting with ccache in promote is not needed. * ipa-cacert-manage: Always connect to ldap, since renew and install are the only options and renew seems to need ldap connection even for self signed certificates. * ipa-compat-manage: Use one ldap connection for the entire script. Replaced try-finally with proper disconnect, code block reindented. * ipa-csreplica-manage: Properly establish and close the ldap connection. * ipa-dns-install: Proper connect, disconnect to ldap. * ipa-kra-install: Proper connect/disconnect for install and uninstall. * ipa-ldap-update: Proper connect and disconnect to ldap. * ipa-nis-manage: Proper connect/disconnect for ldap. Try-finally removed and code block reindented. * ipa-replica-manage: Proper connect/disconnect to ldap. * ipa-replica-prepare: Connect added to validate_options(), where api is initialized and disconnected added at the end of run. Reconnect in ask_for_options() to validate directory manager password. * ipa-server-certinstall: Use api.Backend.ldap2 for ldap connections. * ipa-server-upgrade: Connect to and disconnect from api.Backend.ldap2. https://fedorahosted.org/freeipa/ticket/6461

Use self.hostname instead of self.conn.host. https://fedorahosted.org/freeipa/ticket/6461

Use conn.ldap_uri everywhere. https://fedorahosted.org/freeipa/ticket/6461

* Remove unused and obsolete function arguments: * tls_certfile * tls_keyfile * debug_level * Rename tls_cacertfile to cacert (same as name in LDAPClient) * Set cacert to constants.CACERT by default. https://fedorahosted.org/freeipa/ticket/6461

tkrizek · 2016-11-07T09:42:50Z

Updated commit messages with link to a ticket.

MartinBasti · 2016-11-07T10:34:51Z

tbordaz reviewed Oct 10, 2016

View reviewed changes

MartinBasti suggested changes Oct 10, 2016

View reviewed changes

MartinBasti self-assigned this Oct 10, 2016

tkrizek force-pushed the refactoring-ldap branch from 0b5195b to ad4dfdf Compare October 14, 2016 11:41

tkrizek changed the title ~~[WIP] Refactoring: LDAP Connection Management~~ Refactoring: LDAP Connection Management Oct 14, 2016

tkrizek force-pushed the refactoring-ldap branch from ad4dfdf to 05a3e0f Compare October 20, 2016 10:22

MartinBasti suggested changes Oct 25, 2016

View reviewed changes

HonzaCholasta reviewed Oct 26, 2016

View reviewed changes

tkrizek force-pushed the refactoring-ldap branch from 05a3e0f to b7c9209 Compare November 1, 2016 17:46

HonzaCholasta reviewed Nov 2, 2016

View reviewed changes

MartinBasti added the ack Pull Request approved, can be merged label Nov 7, 2016

Tomas Krizek added 24 commits November 7, 2016 10:37

ipaldap: remove wait/timeout during binds

aee0657

Testing whether it is possible to connect to directory server is already done in RedHatDirectoryService.restart(). https://fedorahosted.org/freeipa/ticket/6461

ipaldap: merge simple_bind into LDAPClient

9dbac4d

* Use LDAPClient.simple_bind instead of extra call to IPAdmin.do_simple_bind * Rename binddn to bind_dn * Rename bindpw to bind_password * Explicitly specify bind_dn in all calls https://fedorahosted.org/freeipa/ticket/6461

ipaldap: merge external_bind into LDAPClient

80994b2

* Rename do_external_bind to external_bind * Remove user_name argument in external_bind() and always set it to effective user name https://fedorahosted.org/freeipa/ticket/6461

ipaldap: merge gssapi_bind to LDAPClient

7b712c2

* Rename do_sasl_gssapi_bind to gssapi_bind https://fedorahosted.org/freeipa/ticket/6461

install: remove dirman_pw from services

38eb281

Remove directory manager's password from service's constructors https://fedorahosted.org/freeipa/ticket/6461

dsinstance: enable ldapi and autobind in ds

0c7c039

* enable ldapi and root autobind early during the ds installation * perform these changes using simple_bind with dm_password https://fedorahosted.org/freeipa/ticket/6461

replicainstall: set ldapi uri in replica promotion

c46a1a3

* read realm from config file * configure api.env to use ldapi genrated from realm https://fedorahosted.org/freeipa/ticket/6461

cainstall: add dm_password to CA installation

15c255b

Installation of Certificate Server replica requires directory manager password. Specify it explicitly in function call and pass it in through an argument. https://fedorahosted.org/freeipa/ticket/6461

ldap2: change default time/size limit

18325a2

* Set default time_limit and size_limit in ldap2 to unlimited. * Set time_limit and size_limit to None in backend. This will respect ipaconfig values. https://fedorahosted.org/freeipa/ticket/6461

dsinstance: conn management

08bbb41

Connect and/or disconnect api.Backend.ldap2 connection when directory server is stopped/restarted. Checking is ldap2 connection is connected is neccesary for edge cases during ds installation (initial start). https://fedorahosted.org/freeipa/ticket/6461

upgradeinstance: ldap conn management

5470de1

connect/disconnect api.Backend.ldap2 connection when directory server is started/stopped https://fedorahosted.org/freeipa/ticket/6461

install: ldap conn management

cf9c5f5

* Move connect to the beggining of the uninstall_check and properly close the connection at the end of the script. * Connect to ldap in external CA installation (step2). https://fedorahosted.org/freeipa/ticket/6461

replicainstall: properly close adhoc connection in promote

ad224a2

Diconnect the established connection oncee is it no longer needed. https://fedorahosted.org/freeipa/ticket/6461

ldapupdate: use ldapi in LDAPUpdate

85de2fd

Configure ldap connection in LDAPUpdate to use ldapi. https://fedorahosted.org/freeipa/ticket/6461

install: remove adhoc dis/connect from services

b917b04

Remove ldap_connect and ldap_disconnect from services. admin_conn is just an alias to api.Backend.ldap2 and therefore the connection should be managed elsewhere. https://fedorahosted.org/freeipa/ticket/6461

ipa-adtrust-install: ldap conn management

7c18256

Properly close ldap connection. https://fedorahosted.org/freeipa/ticket/6461

ldap2: change default bind_dn

56fbc01

Set default bind_dn to cn=directory manager. https://fedorahosted.org/freeipa/ticket/6461

replicainstall: correct hostname in ReplicationManager

6c95420

Use self.hostname instead of self.conn.host. https://fedorahosted.org/freeipa/ticket/6461

replicainstall: use ldap_uri in ReplicationManager

ec7618b

Use conn.ldap_uri everywhere. https://fedorahosted.org/freeipa/ticket/6461

tkrizek force-pushed the refactoring-ldap branch from ffad1bb to 37fdf25 Compare November 7, 2016 09:42

MartinBasti added the pushed Pull Request has already been pushed label Nov 7, 2016

MartinBasti closed this Nov 7, 2016

		@@ -38,6 +38,8 @@ def _main():

		syslog.syslog(syslog.LOG_NOTICE, "certmonger restarted dirsrv instance '%s'" % instance)

		api.Backend.ldap2.close()

Refactoring: LDAP Connection Management #145

Refactoring: LDAP Connection Management #145

Conversation

tkrizek commented Oct 7, 2016 • edited by MartinBasti

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinBasti commented Oct 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinBasti commented Oct 10, 2016

tkrizek commented Oct 14, 2016

tkrizek commented Oct 20, 2016

rcritten commented Oct 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinBasti commented Oct 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkrizek commented Nov 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkrizek Nov 2, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkrizek Nov 2, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkrizek Nov 2, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkrizek Nov 2, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HonzaCholasta commented Nov 2, 2016

tkrizek commented Nov 7, 2016

MartinBasti commented Nov 7, 2016

tkrizek commented Oct 7, 2016 •

edited by MartinBasti

tkrizek Nov 2, 2016 •

edited

tkrizek Nov 2, 2016 •

edited

tkrizek Nov 2, 2016 •

edited

tkrizek Nov 2, 2016 •

edited