Re: [Freeipa-devel] [PATCH 0416-0419] fix broken configuration of sidgen and extdom plugins

2016-03-01 Thread Petr Vobornik 

On 03/01/2016 05:02 PM, Martin Basti wrote:



On 01.03.2016 16:39, Petr Vobornik wrote:

On 02/23/2016 06:15 PM, Martin Basti wrote:



On 23.02.2016 17:31, Tomas Babej wrote:


On 02/23/2016 01:25 PM, Martin Basti wrote:


On 23.02.2016 13:02, Alexander Bokovoy wrote:

On Tue, 23 Feb 2016, Martin Basti wrote:

 From f2ae1bd129a1741500d2f3dcb86a0da553604d15 Mon Sep 17 00:00:00
2001
From: Martin Basti 
Date: Tue, 23 Feb 2016 10:37:47 +0100
Subject: [PATCH 4/4] fix upgrade: wait for proper DS socket after DS
restart

Restarting DS executed by upgrade plugin causes that upgrade
frameworg
was waiting for not proper socket to be ready. This commit fix
issue.

Please fix the commit message typos.


Fixed. Updated patches attached.

ACK.

Tomas

Pushed to master: 0accf8ccb64963954dbe7c137d23f52e5901ac4f
Pushed to ipa-4-3: 4734012c8063460f93f3b819a5bbcca797f6059e
Pushed to ipa-4-2: 63d8caf0d105f02decc0b5d865fedf6ad063bc1a



Testing freeipa-4.2.4 build and it fails at
install/dsinstance.py:add_sidgen_plugin:936

adding self.ldap_connect() on line 937 fixed the issue.


Well I may rework PATCH 0416, and fix it in different way, or I can add
self.ldap_connect() to sidgen and extdom steps.

Which is better?



I would avoid reworking it in all 3 branches if in 4.3 and master it 
works and is actually correct. Doesn't make sense to change new code 
because of missing features in old branches. Adding connect to ipa-4-2 
seems enough to me.

--
Petr Vobornik

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

Re: [Freeipa-devel] [PATCH 0416-0419] fix broken configuration of sidgen and extdom plugins

2016-03-01 Thread Martin Basti 



On 01.03.2016 16:39, Petr Vobornik wrote:

On 02/23/2016 06:15 PM, Martin Basti wrote:



On 23.02.2016 17:31, Tomas Babej wrote:


On 02/23/2016 01:25 PM, Martin Basti wrote:


On 23.02.2016 13:02, Alexander Bokovoy wrote:

On Tue, 23 Feb 2016, Martin Basti wrote:

 From f2ae1bd129a1741500d2f3dcb86a0da553604d15 Mon Sep 17 00:00:00
2001
From: Martin Basti 
Date: Tue, 23 Feb 2016 10:37:47 +0100
Subject: [PATCH 4/4] fix upgrade: wait for proper DS socket after DS
restart

Restarting DS executed by upgrade plugin causes that upgrade 
frameworg
was waiting for not proper socket to be ready. This commit fix 
issue.

Please fix the commit message typos.


Fixed. Updated patches attached.

ACK.

Tomas

Pushed to master: 0accf8ccb64963954dbe7c137d23f52e5901ac4f
Pushed to ipa-4-3: 4734012c8063460f93f3b819a5bbcca797f6059e
Pushed to ipa-4-2: 63d8caf0d105f02decc0b5d865fedf6ad063bc1a



Testing freeipa-4.2.4 build and it fails at 
install/dsinstance.py:add_sidgen_plugin:936


adding self.ldap_connect() on line 937 fixed the issue.


Well I may rework PATCH 0416, and fix it in different way, or I can add 
self.ldap_connect() to sidgen and extdom steps.


Which is better?


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

Re: [Freeipa-devel] [PATCH 0416-0419] fix broken configuration of sidgen and extdom plugins

2016-03-01 Thread Petr Vobornik 

On 02/23/2016 06:15 PM, Martin Basti wrote:



On 23.02.2016 17:31, Tomas Babej wrote:


On 02/23/2016 01:25 PM, Martin Basti wrote:


On 23.02.2016 13:02, Alexander Bokovoy wrote:

On Tue, 23 Feb 2016, Martin Basti wrote:

 From f2ae1bd129a1741500d2f3dcb86a0da553604d15 Mon Sep 17 00:00:00
2001
From: Martin Basti 
Date: Tue, 23 Feb 2016 10:37:47 +0100
Subject: [PATCH 4/4] fix upgrade: wait for proper DS socket after DS
restart

Restarting DS executed by upgrade plugin causes that upgrade frameworg
was waiting for not proper socket to be ready. This commit fix issue.

Please fix the commit message typos.


Fixed. Updated patches attached.

ACK.

Tomas

Pushed to master: 0accf8ccb64963954dbe7c137d23f52e5901ac4f
Pushed to ipa-4-3: 4734012c8063460f93f3b819a5bbcca797f6059e
Pushed to ipa-4-2: 63d8caf0d105f02decc0b5d865fedf6ad063bc1a



Testing freeipa-4.2.4 build and it fails at 
install/dsinstance.py:add_sidgen_plugin:936


adding self.ldap_connect() on line 937 fixed the issue.
--
Petr Vobornik

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

Re: [Freeipa-devel] [PATCH 0416-0419] fix broken configuration of sidgen and extdom plugins

2016-02-23 Thread Martin Basti 



On 23.02.2016 17:31, Tomas Babej wrote:


On 02/23/2016 01:25 PM, Martin Basti wrote:


On 23.02.2016 13:02, Alexander Bokovoy wrote:

On Tue, 23 Feb 2016, Martin Basti wrote:

 From f2ae1bd129a1741500d2f3dcb86a0da553604d15 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 23 Feb 2016 10:37:47 +0100
Subject: [PATCH 4/4] fix upgrade: wait for proper DS socket after DS
restart

Restarting DS executed by upgrade plugin causes that upgrade frameworg
was waiting for not proper socket to be ready. This commit fix issue.

Please fix the commit message typos.


Fixed. Updated patches attached.

ACK.

Tomas

Pushed to master: 0accf8ccb64963954dbe7c137d23f52e5901ac4f
Pushed to ipa-4-3: 4734012c8063460f93f3b819a5bbcca797f6059e
Pushed to ipa-4-2: 63d8caf0d105f02decc0b5d865fedf6ad063bc1a

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

Re: [Freeipa-devel] [PATCH 0416-0419] fix broken configuration of sidgen and extdom plugins

2016-02-23 Thread Tomas Babej 


On 02/23/2016 01:25 PM, Martin Basti wrote:
> 
> 
> On 23.02.2016 13:02, Alexander Bokovoy wrote:
>> On Tue, 23 Feb 2016, Martin Basti wrote:
>>> From f2ae1bd129a1741500d2f3dcb86a0da553604d15 Mon Sep 17 00:00:00 2001
>>> From: Martin Basti 
>>> Date: Tue, 23 Feb 2016 10:37:47 +0100
>>> Subject: [PATCH 4/4] fix upgrade: wait for proper DS socket after DS
>>> restart
>>>
>>> Restarting DS executed by upgrade plugin causes that upgrade frameworg
>>> was waiting for not proper socket to be ready. This commit fix issue.
>> Please fix the commit message typos.
>>
> Fixed. Updated patches attached.

ACK.

Tomas

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

Re: [Freeipa-devel] [PATCH 0416-0419] fix broken configuration of sidgen and extdom plugins

2016-02-23 Thread Martin Basti 



On 23.02.2016 13:02, Alexander Bokovoy wrote:

On Tue, 23 Feb 2016, Martin Basti wrote:

From f2ae1bd129a1741500d2f3dcb86a0da553604d15 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 23 Feb 2016 10:37:47 +0100
Subject: [PATCH 4/4] fix upgrade: wait for proper DS socket after DS 
restart


Restarting DS executed by upgrade plugin causes that upgrade frameworg
was waiting for not proper socket to be ready. This commit fix issue.

Please fix the commit message typos.


Fixed. Updated patches attached.
From 779ca0e230252d40bb275539b4cc512d0460a056 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 18 Feb 2016 19:59:50 +0100
Subject: [PATCH 1/4] upgrade: fix config of sidgen and extdom plugins

During upgrade to IPA 4.2, literally "$SUFFIX" value was added to
configuration of sidgen and extdom plugins. This cause that SID are not properly configured.

Upgrade must fix "$SUFFIX" to reals suffix DN, and run sidgen task
against IPA domain (if exists).

All trusts added when plugins configuration was broken must be re-added.

https://fedorahosted.org/freeipa/ticket/5665
---
 install/updates/90-post_upgrade_plugins.update |   2 +
 ipaserver/install/dsinstance.py|  12 +-
 ipaserver/install/plugins/adtrust.py   | 153 +
 ipaserver/install/server/upgrade.py|   4 +-
 4 files changed, 163 insertions(+), 8 deletions(-)

diff --git a/install/updates/90-post_upgrade_plugins.update b/install/updates/90-post_upgrade_plugins.update
index 5642021ad93cf336db2d872bf3ef6db99b5ffa46..9c9ee160fffedbc8e8d59705169e6cf08ddc9779 100644
--- a/install/updates/90-post_upgrade_plugins.update
+++ b/install/updates/90-post_upgrade_plugins.update
@@ -5,6 +5,8 @@
 plugin: update_ca_topology
 plugin: update_dnszones
 plugin: update_dns_limits
+plugin: update_sigden_extdom_broken_config
+plugin: update_sids
 plugin: update_default_range
 plugin: update_default_trust_view
 plugin: update_ca_renewal_master
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 2905c31c64c4fa8bd034d2cdf6ce5d90ecfea091..f474e189a47f945b7c91cba4ccc17266b9c7e430 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -1054,9 +1054,9 @@ class DsInstance(service.Service):
 """
 Add sidgen directory server plugin configuration if it does not already exist.
 """
-self._ldap_mod('ipa-sidgen-conf.ldif', self.sub_dict)
+self.add_sidgen_plugin(self.sub_dict['SUFFIX'])
 
-def add_sidgen_plugin(self):
+def add_sidgen_plugin(self, suffix):
 """
 Add sidgen plugin configuration only if it does not already exist.
 """
@@ -1064,7 +1064,7 @@ class DsInstance(service.Service):
 try:
 self.admin_conn.get_entry(dn)
 except errors.NotFound:
-self._add_sidgen_plugin()
+self._ldap_mod('ipa-sidgen-conf.ldif', dict(SUFFIX=suffix))
 else:
 root_logger.debug("sidgen plugin is already configured")
 
@@ -1072,9 +1072,9 @@ class DsInstance(service.Service):
 """
 Add directory server configuration for the extdom extended operation.
 """
-self._ldap_mod('ipa-extdom-extop-conf.ldif', self.sub_dict)
+self.add_extdom_plugin(self.sub_dict['SUFFIX'])
 
-def add_extdom_plugin(self):
+def add_extdom_plugin(self, suffix):
 """
 Add extdom configuration if it does not already exist.
 """
@@ -1082,7 +1082,7 @@ class DsInstance(service.Service):
 try:
 self.admin_conn.get_entry(dn)
 except errors.NotFound:
-self._add_extdom_plugin()
+self._ldap_mod('ipa-extdom-extop-conf.ldif', dict(SUFFIX=suffix))
 else:
 root_logger.debug("extdom plugin is already configured")
 
diff --git a/ipaserver/install/plugins/adtrust.py b/ipaserver/install/plugins/adtrust.py
index df9412adba833251cc1c70d7d72ebebbdc77c2a4..5b81b2efd5be63f45b02b8d357f89ec5ba142170 100644
--- a/ipaserver/install/plugins/adtrust.py
+++ b/ipaserver/install/plugins/adtrust.py
@@ -21,6 +21,8 @@ from ipalib import api, errors
 from ipalib import Updater
 from ipapython.dn import DN
 from ipapython.ipa_log_manager import root_logger
+from ipaserver.install import sysupgrade
+
 
 DEFAULT_ID_RANGE_SIZE = 20
 
@@ -161,5 +163,156 @@ class update_default_trust_view(Updater):
 
 return False, [update]
 
+
+class update_sigden_extdom_broken_config(Updater):
+"""Fix configuration of sidgen and extdom plugins
+
+Upgrade to IPA 4.2+ cause that sidgen and extdom plugins have improperly
+configured basedn.
+
+All trusts which have been added when config was broken must to be
+re-added manually.
+
+https://fedorahosted.org/freeipa/ticket/5665
+"""
+
+sidgen_config_dn = DN("cn=IPA SIDGEN,cn=plugins,cn=config")
+extdom_config_dn =

Re: [Freeipa-devel] [PATCH 0416-0419] fix broken configuration of sidgen and extdom plugins

2016-02-23 Thread Alexander Bokovoy 

On Tue, 23 Feb 2016, Martin Basti wrote:

From f2ae1bd129a1741500d2f3dcb86a0da553604d15 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 23 Feb 2016 10:37:47 +0100
Subject: [PATCH 4/4] fix upgrade: wait for proper DS socket after DS restart

Restarting DS executed by upgrade plugin causes that upgrade frameworg
was waiting for not proper socket to be ready. This commit fix issue.

Please fix the commit message typos.

--
/ Alexander Bokovoy

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

Re: [Freeipa-devel] [PATCH 0416-0419] fix broken configuration of sidgen and extdom plugins

2016-02-23 Thread Martin Basti 



On 22.02.2016 20:11, Martin Basti wrote:



On 22.02.2016 19:15, Martin Basti wrote:



On 22.02.2016 17:05, Martin Basti wrote:



On 19.02.2016 15:02, Alexander Bokovoy wrote:

On Fri, 19 Feb 2016, Petr Vobornik wrote:

On 02/19/2016 11:12 AM, Alexander Bokovoy wrote:

On Fri, 19 Feb 2016, Martin Basti wrote:

WIP patch attached

https://fedorahosted.org/freeipa/ticket/5665


Comments inline.


+# we need to run sidgen task
+sidgen_task_dn = 
DN("cn=sidgen,cn=ipa-sidgen-task,cn=tasks,"

+"cn=config")
+sidgen_tasks_attr = {
+"objectclass": ["top", "extensibleObject"],
+"cn": ["sidgen"],
+"delay": [0],
+"nsslapd-basedn": [self.api.env.basedn],
+}

May be you are better to name this task more uniquely?
Something like 'cn=generate domain sid,cn=...'?


+
+task_entry = ldap.make_entry(sidgen_task_dn,
+ **sidgen_tasks_attr)
+try:
+ldap.add_entry(task_entry)
+except errors.DuplicateEntry:
+self.log.debug("sidgen task already created")
+else:
+self.log.debug("sidgen task has been created")
There could be multiple tasks running in parallel, that's why it 
could

be good to use a different and unique name.

+# we have to check all trusts domains which have been 
added

after the
+# upgrade that caused bug was done.
+
+base_dn = DN(self.api.env.container_adtrusts,
self.api.env.basedn)
+trust_domain_entries, truncated = ldap.find_entries(
+base_dn=base_dn,
+scope=ldap.SCOPE_ONELEVEL,
+attrs_list=[attr_name, "cn"],
+)
+
+if truncated:
+self.log.warning("update_sids: Search results were
truncated")
+
+for entry in trust_domain_entries:
+if entry.single_value[attr_name] is None:
+domain = entry.single_value["cn"]
+self.log.error(
+"Your trust to %s is broken. Please 
re-create it

by "
+"running 'ipa trust-add' again", domain)
+
+sysupgrade.set_upgrade_state('sidgen', 'update_sids', 
False)

+return False, ()
This part looks fine. Basically, a similar check needs to be 
added to

trust_find, trust_show, and may be trust_add.



Why trust-add?

I'm not a big fan of cluttering existing commands(find, show, mod) 
with logic to fix one upgrade bug. But I understand a need to 
communicate it somehow.


Would it make sense to move such logic to a separate command, e.g. 
trust-check/trust-verify?  The command can do additional check in 
a future.
No. What is the value of trust-add if it says you 'Trust 
established and

verified' while in reality it didn't? This specific issue is very easy
to catch.


Patch attached, patch with warning will land soon.



Updated patches attached


I fixed except clause in 416, added patch with user warnings, IMO to 
have separate search is the cleanest solution here, command is not 
used often, I would like to avoid any hacks in find and show command


Patches attached.

I will look on ldapsearch timeouts after restarting DS tomorrow.



Updated patches attached + new patch fixing timeout error

From 779ca0e230252d40bb275539b4cc512d0460a056 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 18 Feb 2016 19:59:50 +0100
Subject: [PATCH 1/4] upgrade: fix config of sidgen and extdom plugins

During upgrade to IPA 4.2, literally "$SUFFIX" value was added to
configuration of sidgen and extdom plugins. This cause that SID are not properly configured.

Upgrade must fix "$SUFFIX" to reals suffix DN, and run sidgen task
against IPA domain (if exists).

All trusts added when plugins configuration was broken must be re-added.

https://fedorahosted.org/freeipa/ticket/5665
---
 install/updates/90-post_upgrade_plugins.update |   2 +
 ipaserver/install/dsinstance.py|  12 +-
 ipaserver/install/plugins/adtrust.py   | 153 +
 ipaserver/install/server/upgrade.py|   4 +-
 4 files changed, 163 insertions(+), 8 deletions(-)

diff --git a/install/updates/90-post_upgrade_plugins.update b/install/updates/90-post_upgrade_plugins.update
index 5642021ad93cf336db2d872bf3ef6db99b5ffa46..9c9ee160fffedbc8e8d59705169e6cf08ddc9779 100644
--- a/install/updates/90-post_upgrade_plugins.update
+++ b/install/updates/90-post_upgrade_plugins.update
@@ -5,6 +5,8 @@
 plugin: update_ca_topology
 plugin: update_dnszones
 plugin: update_dns_limits
+plugin: update_sigden_extdom_broken_config
+plugin: update_sids
 plugin: update_default_range
 plugin: update_default_trust_view
 plugin: update_ca_renewal_master
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 2905c31c64c4fa8bd034d2cdf6ce5d90ecfea091..f474e189a47f945b7c91cba4ccc17266b9c7e430 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -1054,9 +1054,9 @@ class