Re: [Freeipa-devel] [PATCH 424] install: Introduce installer framework ipapython.install

2015-05-06 Thread Martin Kosek
On 04/29/2015 06:25 PM, Jan Cholasta wrote:
 Dne 20.4.2015 v 16:56 Jan Cholasta napsal(a):
 Dne 20.4.2015 v 15:14 Martin Basti napsal(a):
 On 17/04/15 16:15, Jan Cholasta wrote:
 Dne 16.4.2015 v 16:46 Jan Cholasta napsal(a):
 Hi,

 the attached patch adds the basics of the new installer framework.

 As a next step, I plan to convert the install scripts to use the
 framework with their old code (the old code will be gradually ported to
 the framework later).

 (Note I didn't manage to write docstrings today, expect update
 tomorrow.)

 Added some docstrings.

 Also updated the patch to reflect little brainstorming David and I had
 this morning.


 Honza



 Hello, see comments bellow:

 1) We started using new shorter License header in files:
 #
 # Copyright (C) 2015  FreeIPA Contributors see COPYING for license
 #

 OK.


 2) IMO this will not work, NoneType has no 'obj' attribute
 +else:
 +if isinstance(value, from_):
 +value = None
 +stack.append(value.obj)
 +continue

 Right.


 3) Multiple inheritance. I do not like it much.
 +class CompositeInstaller(Installer, CompositeConfigurator):

 I guess you are antagonistic to multiple inheritance because of how
 other languages (like C++) do it. In Python it can be pretty elegant and
 is basis for e.g. the mixin design pattern.


 Installer and CompositeConfigurator inherites from Configurator class,
 and all of them implements _generator method.

 Both of them call super()._generator(), so it's no problem (same for
 other methods).


 If I understand correctly
 (https://www.python.org/download/releases/2.3/mro/) the
 Installer._generator method will be used in this case.
 However in case when CompositeConfigurator has more levels (respectively
 it is more specialized) of inheritance, it could take precedence and its
 _generator method may be used instead.

 The order of precedence is defined by the order of base classes in the
 class definition.


 I'm afraid this may suddenly stop working.
 Maybe I'm wrong, please fix me.

 As long as you call the super class, it will work fine.


 And Multiple inheritance is not easily readable, this is even a diamond
 inheritance model.

 Cooperative inheritance is used by design and IMHO is easily readable if
 you know how to read it. Every class defines a single bit of behavior.
 Without cooperative inheritance, it would have to be hardcoded and/or
 hacked around, which I wanted to avoid.

 This blog post explains it nicely:
 https://rhettinger.wordpress.com/2011/05/26/super-considered-super/.

 
 Updated patch attached.
 
 Also attached is patch 425 which migrates ipa-server-install to the install
 framework.

Good job there. I am just curious, will this framework and new option
processing be friendly to other types of option passing than just via options?
I mean tickets

https://fedorahosted.org/freeipa/ticket/4517
https://fedorahosted.org/freeipa/ticket/4468

Especially 4517 is important, we need to be able to run

# cat install.conf
ds_password=Secret123
admin_password=Secret456
ip_address=123456
setup_dns=False

# ipa-server-install --unattended --conf install.conf

I assume yes, but I am just making sure.

-- 
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] Wiki: automatic bookkeeping of Design documents

2015-05-06 Thread Petr Vobornik

On 05/06/2015 08:47 AM, Martin Kosek wrote:

Hello all,

Knowing the sorrow and unmaintained state of the pages collecting links to our
designs [1][2], I think we need to execute the second half of my evil plan for
Design Document management.

We have the Feature design box (see top right corner, e.g. in [3]), so we can
easily automatically generate mediawiki categories. The first I implemented in
the template are FreeIPA $VERSION Design when target version is filled (and
design is thus accepted for a release) and FreeIPA Design Proposal for
others. We can be creative with other categories in future, if needed.

But even these 2 and a DynamicPageList plugin allowed me to create
automatically generated design lists, in [4]. I had to update the box in many
designs, however.

Makes sense? If yes, I would update these pages. Of course, this requires
developers to maintain the Feature box properly, but I think it's worth it.


I like it.

What do you think about keeping design proposals on the same page with 
approved designs as is now in [4]? When I'm looking for some design, it 
often happens that I'm not sure on which page it is. With both lists on 
the same page, one could do simple search to find the design quickly.





[1] http://www.freeipa.org/page/V4_Proposals
[2] http://www.freeipa.org/page/V4_Designs
[3] http://www.freeipa.org/page/V4/User_Certificates
[4] http://www.freeipa.org/page/Talk:V4_Designs


--
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] Wiki: automatic bookkeeping of Design documents

2015-05-06 Thread Petr Spacek
On 6.5.2015 08:47, Martin Kosek wrote:
 Hello all,
 
 Knowing the sorrow and unmaintained state of the pages collecting links to our
 designs [1][2], I think we need to execute the second half of my evil plan for
 Design Document management.
 
 We have the Feature design box (see top right corner, e.g. in [3]), so we can
 easily automatically generate mediawiki categories. The first I implemented in
 the template are FreeIPA $VERSION Design when target version is filled (and
 design is thus accepted for a release) and FreeIPA Design Proposal for
 others. We can be creative with other categories in future, if needed.
 
 But even these 2 and a DynamicPageList plugin allowed me to create
 automatically generated design lists, in [4]. I had to update the box in many
 designs, however.
 
 Makes sense? If yes, I would update these pages. Of course, this requires
 developers to maintain the Feature box properly, but I think it's worth it.

It makes sense do me if we document the rules. E.g. 'design is accepted if and
only if version + reviewer fields are filled' or something like that.

 [1] http://www.freeipa.org/page/V4_Proposals
 [2] http://www.freeipa.org/page/V4_Designs
 [3] http://www.freeipa.org/page/V4/User_Certificates
 [4] http://www.freeipa.org/page/Talk:V4_Designs

-- 
Petr^2 Spacek

-- 
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 424] install: Introduce installer framework ipapython.install

2015-05-06 Thread Jan Cholasta

Dne 6.5.2015 v 08:11 Martin Kosek napsal(a):

On 04/29/2015 06:25 PM, Jan Cholasta wrote:

Dne 20.4.2015 v 16:56 Jan Cholasta napsal(a):

Dne 20.4.2015 v 15:14 Martin Basti napsal(a):

On 17/04/15 16:15, Jan Cholasta wrote:

Dne 16.4.2015 v 16:46 Jan Cholasta napsal(a):

Hi,

the attached patch adds the basics of the new installer framework.

As a next step, I plan to convert the install scripts to use the
framework with their old code (the old code will be gradually ported to
the framework later).

(Note I didn't manage to write docstrings today, expect update
tomorrow.)


Added some docstrings.

Also updated the patch to reflect little brainstorming David and I had
this morning.



Honza





Hello, see comments bellow:

1) We started using new shorter License header in files:
#
# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
#


OK.



2) IMO this will not work, NoneType has no 'obj' attribute
+else:
+if isinstance(value, from_):
+value = None
+stack.append(value.obj)
+continue


Right.



3) Multiple inheritance. I do not like it much.
+class CompositeInstaller(Installer, CompositeConfigurator):


I guess you are antagonistic to multiple inheritance because of how
other languages (like C++) do it. In Python it can be pretty elegant and
is basis for e.g. the mixin design pattern.



Installer and CompositeConfigurator inherites from Configurator class,
and all of them implements _generator method.


Both of them call super()._generator(), so it's no problem (same for
other methods).



If I understand correctly
(https://www.python.org/download/releases/2.3/mro/) the
Installer._generator method will be used in this case.
However in case when CompositeConfigurator has more levels (respectively
it is more specialized) of inheritance, it could take precedence and its
_generator method may be used instead.


The order of precedence is defined by the order of base classes in the
class definition.



I'm afraid this may suddenly stop working.
Maybe I'm wrong, please fix me.


As long as you call the super class, it will work fine.



And Multiple inheritance is not easily readable, this is even a diamond
inheritance model.


Cooperative inheritance is used by design and IMHO is easily readable if
you know how to read it. Every class defines a single bit of behavior.
Without cooperative inheritance, it would have to be hardcoded and/or
hacked around, which I wanted to avoid.

This blog post explains it nicely:
https://rhettinger.wordpress.com/2011/05/26/super-considered-super/.



Updated patch attached.

Also attached is patch 425 which migrates ipa-server-install to the install
framework.


Good job there. I am just curious, will this framework and new option
processing be friendly to other types of option passing than just via options?
I mean tickets

https://fedorahosted.org/freeipa/ticket/4517
https://fedorahosted.org/freeipa/ticket/4468

Especially 4517 is important, we need to be able to run

# cat install.conf
ds_password=Secret123
admin_password=Secret456
ip_address=123456
setup_dns=False

# ipa-server-install --unattended --conf install.conf

I assume yes, but I am just making sure.


Yes, definitely.

--
Jan Cholasta

--
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] [PATCHES 306-316] Automated migration tool from Winsync

2015-05-06 Thread Tomas Babej



On 05/05/2015 02:02 PM, Tomas Babej wrote:



On 04/29/2015 12:28 PM, Tomas Babej wrote:



On 03/11/2015 04:20 PM, Jan Cholasta wrote:

Hi,

Dne 10.3.2015 v 16:35 Tomas Babej napsal(a):


On 03/09/2015 12:26 PM, Tomas Babej wrote:

Hi,

this couple of patches provides a initial implementation of the
winsync migration tool:

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

Some parts could use some polishing, but this is a sound foundation.

Tomas





Attaching one more patch to the bundle. This one should make the 
winsync

tool readily available after install.

Tomas




Nitpicks:

The winsync_migrate module should be in ipaserver.install. Also I 
don't see why it has to be a package when there is just one short 
file in it.


By convention, the AdminTool subclass should be named 
WinsyncMigrate, or the tool should be named ipa-migrate-winsync.


Honza



Updated patches attached.

Tomas


Rebased patches with cleaned membership bits.

Tomas


I did some self-review, updated patches attached.
From d98b18d7e1fe98eb81c9030b566ee28860a0d43e Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Wed, 29 Apr 2015 08:15:50 +0200
Subject: [PATCH] winsync-migrate: Add initial plumbing

https://fedorahosted.org/freeipa/ticket/4524
---
 install/tools/ipa-winsync-migrate | 23 
 ipaserver/winsync_migrate/__init__.py | 22 
 ipaserver/winsync_migrate/base.py | 67 +++
 3 files changed, 112 insertions(+)
 create mode 100755 install/tools/ipa-winsync-migrate
 create mode 100644 ipaserver/winsync_migrate/__init__.py
 create mode 100644 ipaserver/winsync_migrate/base.py

diff --git a/install/tools/ipa-winsync-migrate b/install/tools/ipa-winsync-migrate
new file mode 100755
index ..9eb9a03eb92396c855706e0f85850baece45b27e
--- /dev/null
+++ b/install/tools/ipa-winsync-migrate
@@ -0,0 +1,23 @@
+#! /usr/bin/python2 -E
+# Authors: Tomas Babej tba...@redhat.com
+#
+# Copyright (C) 2015  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/.
+#
+
+from ipaserver.winsync_migrate.base import MigrateWinsync
+
+MigrateWinsync.run_cli()
diff --git a/ipaserver/winsync_migrate/__init__.py b/ipaserver/winsync_migrate/__init__.py
new file mode 100644
index ..e0da63db3a369adc4a34e8675471929b5839a812
--- /dev/null
+++ b/ipaserver/winsync_migrate/__init__.py
@@ -0,0 +1,22 @@
+# Authors: Tomas Babej tba...@redhat.com
+#
+# Copyright (C) 2015  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/.
+#
+
+
+Base subpackage for winsync-migrate related code.
+
diff --git a/ipaserver/winsync_migrate/base.py b/ipaserver/winsync_migrate/base.py
new file mode 100644
index ..c21a861c2e49b4ab6d9783c117d1e4de126cb0b6
--- /dev/null
+++ b/ipaserver/winsync_migrate/base.py
@@ -0,0 +1,67 @@
+# Authors: Tomas Babej tba...@redhat.com
+#
+# Copyright (C) 2015  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# 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 krbV
+import 

Re: [Freeipa-devel] Wiki: automatic bookkeeping of Design documents

2015-05-06 Thread Martin Kosek
On 05/06/2015 12:20 PM, Petr Vobornik wrote:
 On 05/06/2015 08:47 AM, Martin Kosek wrote:
 Hello all,

 Knowing the sorrow and unmaintained state of the pages collecting links to 
 our
 designs [1][2], I think we need to execute the second half of my evil plan 
 for
 Design Document management.

 We have the Feature design box (see top right corner, e.g. in [3]), so we can
 easily automatically generate mediawiki categories. The first I implemented 
 in
 the template are FreeIPA $VERSION Design when target version is filled (and
 design is thus accepted for a release) and FreeIPA Design Proposal for
 others. We can be creative with other categories in future, if needed.

 But even these 2 and a DynamicPageList plugin allowed me to create
 automatically generated design lists, in [4]. I had to update the box in many
 designs, however.

 Makes sense? If yes, I would update these pages. Of course, this requires
 developers to maintain the Feature box properly, but I think it's worth it.
 
 I like it.
 
 What do you think about keeping design proposals on the same page with 
 approved
 designs as is now in [4]? When I'm looking for some design, it often happens
 that I'm not sure on which page it is. With both lists on the same page, one
 could do simple search to find the design quickly.

I guess I could live with V4_Proposals page merged to V4_Designs. The number of
proposed is not that big.

But I do not think the confusion should happen - designs for next versions
should be versioned, in Trac tickets and thus seen on the V4_Designs page or in
the ticket.

 [1] http://www.freeipa.org/page/V4_Proposals
 [2] http://www.freeipa.org/page/V4_Designs
 [3] http://www.freeipa.org/page/V4/User_Certificates
 [4] http://www.freeipa.org/page/Talk:V4_Designs


-- 
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 0238] Server Upgrade: fix broken memberUid index

2015-05-06 Thread Martin Basti

On 06/05/15 13:22, Martin Basti wrote:

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

Patch attached.




Requires patch mbasti-231-4

--
Martin Basti

-- 
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] bind-dyndb-ldap: [PATCH] Add note about installation from Git to README.

2015-05-06 Thread Petr Spacek
Hello David,

don't be shy and share your patches with the ipa-devel list :-)

Anyway, I did few changes to the patch and pushed it as commit
8e42fe2add7f97e9c9412b08306e73d6ec53a20e to master branch.

Thank you!

-- 
Petr^2 Spacek
From 604c5c19f7e93424dcad3a449ac966e2f4e73df6 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 6 May 2015 12:08:05 +0200
Subject: [PATCH] Add note about installation from Git to README.

---
 README | 5 +
 1 file changed, 5 insertions(+)

diff --git a/README b/README
index 0c0352b70f6b5884585afb8e5acd57cd7cdc7b6c..f28608a395514254ac9dd18b050ee558ffd03554 100644
--- a/README
+++ b/README
@@ -45,6 +45,11 @@ Then, to install, run this as root:
 
 This will install the file ldap.so into the libdir/bind/ directory.
 
+Alternatively, the latest version of LDAP back-end can be installed from Git
+repository. Instead of downloading and extracting the tarbal you need to:
+
+$ git clone https://github.com/pspacek/bind-dyndb-ldap
+$ autoreconf -fvi
 
 4. LDAP schema
 ==
-- 
2.4.0

From 8e42fe2add7f97e9c9412b08306e73d6ec53a20e Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 6 May 2015 12:08:05 +0200
Subject: [PATCH] Add note about installation from Git to README.

---
 README | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/README b/README
index 0c0352b70f6b5884585afb8e5acd57cd7cdc7b6c..0b1a47402a70df16239d33134c62f4905be1651f 100644
--- a/README
+++ b/README
@@ -45,6 +45,12 @@ Then, to install, run this as root:
 
 This will install the file ldap.so into the libdir/bind/ directory.
 
+Alternatively, the latest version can be obtained from Git repository.
+You can use following commands to prepare latest source tree for compilation:
+
+$ git clone https://git.fedorahosted.org/git/bind-dyndb-ldap.git
+$ cd bind-dyndb-ldap
+$ autoreconf -fvi
 
 4. LDAP schema
 ==
-- 
2.1.0

-- 
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] 807 webui-ci: do not open 2 browser windows

2015-05-06 Thread Milan Kubik

On 02/19/2015 03:51 PM, Petr Vobornik wrote:

Only for master branch.


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

ACK

Milan
-- 
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] Replication Topology design feedback

2015-05-06 Thread Oleg Fayans
Hi Ludwig,

I have a question. What is the proper way of removing ipa replica from
the server that is replication-topology-aware? Standard way
`ipa-replica-manage del` does not work anymore, since the topology is
controlled by the plugin, but I was unable to remove it using
topology-aware tools as well. Here is the transcript of my session:
http://pastebin.test.redhat.com/281213

Thanks in advance!


On 05/04/2015 04:46 PM, Martin Kosek wrote:
 Thanks for answers.

 BTW, for future, I think Oleg that it would be useful to ask this questions on
 freeipa-devel directly as it may be helpful to other developers and we would
 have it archived for other uses.

 On 05/04/2015 04:20 PM, Ludwig Krispenz wrote:
 On 04/30/2015 03:22 PM, Oleg Fayans wrote:
 Hi Ludwig,

 While getting myself familiar with Replication Topology Plugin design
 page I've found a number of places that need some clarifications.

 1.
 Check at startup.
 When the directory server starts, the plugin init and start functions
 are executed, they will read the domain level
 attribute and act accordingly
 Could you describe how exactly should the plugin work depending on the
 domain level revealed.
 there are only two scenarios:
 1] domain_level  plugin_level:
 the plugin does almost nothing, reading its config and waiting for a dom 
 level
 increase
 2] doamin_level = plugin_level:
 the plugin controls topology for managed servers, rejecting direct mods of
 replication agreements, transforming adding/deleting/modification of segments
 into corresponding actions on replication agreements
 2.
 Check for modify operation
 If an admin or tool changes the domain level the plugin detects this
 change and performs initialization tasks if the
 domain level is greater than the plugin version
 The same here: what exactly happens after the domain level has changed.
 if the domain level raises, so that teh plugin becomes active, this will 
 happen
 on all servers since domain level is replicated.
 the plugin will read all the exisußing info inthe shared tree, read all
 replication configuration and match them creating missing data in both areas:
 cn=config and shared tree
 3. Regarding the startup delay. How can I make sure the plugin has started?
 As far as I understood the Topology plugin needs to be started only
 after all other plugins are started to prevent it from making a tree
 changes before it is able to be replicated. The question is: how do I
 check whether all other plugins are already started?
 the plugin should be started when the server starts, it will expose this in 
 the
 root dse and you can search for it, I just updated teh design page with an 
 example
 4. Shared configuration layout
 It is written there, that the replication topology information can be
 configured to be stored in the custom place of the tree. How do we do that?
 The configuration is in the plugin conf in cn=config. At the moment it is
 populated when a DS instance is created.

 Most probably I''ll have some more questions once I have the branch
 installed.

 Thank you!


-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

-- 
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] 814-818 migrate-ds: optimize adding users to default group

2015-05-06 Thread Martin Basti

On 05/05/15 10:59, Petr Vobornik wrote:

On 04/30/2015 03:53 PM, Martin Basti wrote:

On 10/04/15 12:55, Petr Vobornik wrote:

The essential patch is 814.

815 a proposal for new option.

816 and 818 are cleanup patches.

817 little optimization.

== [PATCH] 814 migrate-ds: optimize adding users to default group ==
Migrate-ds searches for user without a group and adds them to default
group. There is no point in checking if the user's selected by
previous query are not member of default group because they are not
member of any group.

The operation is also speeded up by not fetching the default group.
Users are added right away.

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


NACK

Users haven't been added into ipa default group after migration.


Fixed in patch 815.



Just nitpick

1) too many parentheses
 api.log.error(('Adding new members to default group failed:
%s \n'
'members: %s') % (e, 
(','.join(member_dns

You can use this instead:
 api.log.error('Adding new members to default group failed:
%s \n'
   'members: %s', e, ','.join(member_dns))


Fixed.



== [PATCH] 815 migrate-ds: skip default group options ==

New option --use-default-group=False could be used to disable adding of
migrated users into default group.

By default, the default group is no longer POSIX therefore it doesn't
fulfill the original idea of providing GID and therefore it could be
skipped during migration.

LGTM


the option got so the default would be applied.
+autofill=True,




== [PATCH] 816 migrate-ds: remove unused def_group_gid context
property ==
it's no longer used anywhere


1)
You can remove the unused variable 'g_attrs'


removed


== [PATCH] 817 migrate-ds: optimize gid checks by utilizing dictionary
nature of set ==


LGTM
== [PATCH] 818 migrate-ds: log migrated group members only on debug 
level ==

It pollutes error_log.


1)
you do not need % formatting in logger
api.log.debug('migrating %s group %s' , member_attr, m)


fixed and also changed migrating %s user %s' line to debug, which 
was the actual reason for this patch.







Martin^2





Thanks.

1)
Please create new API file, probably missing autofill in API.txt:

Option 'use_def_group?' in command 'migrate_ds' in API file not found
Options count in migrate_ds of 18 doesn't match expected: 19
Option use_def_group? of command migrate_ds in ipalib, not in API file:
Bool('use_def_group?', autofill=True, cli_name='use_default_group', 
default=True)


There are one or more changes to the API.
Either undo the API changes or update API.txt and increment the major 
version in VERSION.


2)
ipa: error: --use-default-group option does not take a value

and a nitpick:

patch 814
1)
Why this change?

-api.log.debug('Adding %d users to group%s duration %s',
-len(new_members), mode, d)
+api.log.info('Adding %d users to group%s duration %s',
+  len(member_dns), mode, d)

--
Martin Basti

--
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 0322-0337] Fix mysterious failures in PTR record synchronization

2015-05-06 Thread David Kupka

On 05/05/2015 05:24 PM, Petr Spacek wrote:

Hello,

Attached patch set is the best fix for
https://fedorahosted.org/bind-dyndb-ldap/ticket/155
I was able to write.

This patch set should fix vast majority of race conditions. Unfortunately it
cannot be 100 % reliable without support for LDAP transactions.

For convenience you can download the whole tree from
https://github.com/pspacek/bind-dyndb-ldap/commits/t155.syncptr
HEAD = da2552632f6ce67f1bb9d9b3cdd3e0a8e06ce9ea

Enjoy.


Hi,
thanks for the patch set.
I tested only a functionality because I really don't know the code base 
of bind nor bind-dyndb-ldap.


To test the patch set I used ipa-client-install with patch 
freeipa-dkupka-0035-6. DNS server does not return an error unless 
creation/update of record that I requested has failed. This is exactly 
the behavior I expect, thanks.


--
David Kupka

--
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 0032] prevent duplicate IDs when setting up multiple replicas against single master

2015-05-06 Thread thierry bordaz

On 05/06/2015 03:19 PM, Martin Babinsky wrote:

Hello Thierry,

replies are inline.

On 05/06/2015 02:22 PM, thierry bordaz wrote:

On 05/06/2015 01:54 PM, Martin Babinsky wrote:

The attached patch tries to fix
https://fedorahosted.org/freeipa/ticket/4378

After discussion with Thierry we concluded that while this issue is
more complex than it seems, the transition from REPLACE to DEL/ADD
operations when updating nsDS5ReplicaId should suffice for this ticket.


Hello Martin,

Few comments, you are using MOD_DEL 'replicaID' with None value. So this
is going to delete all previous values and it should be equivalent to a
MOD_REPL.
I was thinking you wanted to retrieve the id_value and call MOD_DEL
'replicaID' current_value. So that if by the time you fetched the
replicaId, an other replica updated the replicaId, the MOD_DEL/MOD_ADD
would fail and you need a new iteration.

Sorry I didn't know you can MOD_DEL a particular value (I'm LDAP noob 
I know). Will fix this.

If replicaId was multi-valued and you want to make it single valued, you
may want to do create a more complex MOD (e.g. (ldap.MOD_DELETE,
'nsDS5ReplicaId', str(value1), (ldap.MOD_DELETE, 'nsDS5ReplicaId',
str(value2)...)

AFAIK ReplicaId is single-valued (looking at the schema right now) so 
this shouldn't be problem.

If it is updating successfully do you want to return 'retval' or
'retval+1' ?

If several replicas try to update the replicaId of the master and the
current replicaId is 1000.
Replica1 successfully updates the replicaId and gets 1001 as the new 
value.

Replica2 successfully updates the replicaId and gets 1002.
The final value on master will be 1002, but replica1 will assum it is
1001. Is it a problem ?

I studied the code in the master branch and IIUC (and please correct 
me if I got this wrong) nsDS5ReplicaId attribute in 
'cn=replication,cn=etc,$SUFFIX' represents replicaID of the _next_ 
replica that will be installed.


So if a replica is installed, it sets the current value of 
nsDS5ReplicaId as its replica ID (the function returns 'retval') and 
then increments it in 'cn=replication,cn=etc,$SUFFIX' entry ('retval + 
1' is written to master) so that the next installed replica fetches 
this updated value.


So the case you described should be the expected behavior. To change 
it would require different patch IMHO.


Thank for your precious explanations, in fact the value in 
'cn=replication,cn=etc,SUFFIX' is the next available replicaId value and 
is the centralized mechanism that assign unique replicaID.


The risk was that several replica pick the same value. So yes it is 
important that the MOD_DEL specifies the previously read value so that 
the test/set will be atomic. If several replicas read the same value, 
only the faster one will use it to install the replica.


thanks
thierry

thanks
thierry





--
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] 800 rpc-client: add forms based auth support

2015-05-06 Thread Milan Kubik

On 02/19/2015 03:51 PM, Petr Vobornik wrote:
This patch is a prerequisite for patch 801 which will follow. It was 
developed to enable to use ipalib RPC client in Web UI tests. Plus it 
will enable to significantly speed up Web UI tests suite (if 
preparation of data is transformed to use this method).


Partly related https://fedorahosted.org/freeipa/ticket/4772 and 
https://fedorahosted.org/freeipa/ticket/4307



Leverage session support to enable forms-based authenticate in rpc 
client.


In order to do that session support in KerbTransport was moved to new
SessionTransport. RPCClient.create_connection is then modified to
force forms-based auth if new optional options - user and password are
specified. For this case SessionTransport is used and user is
authenticated by calling
'https://ipa.server/ipa/session/login_password'. Session cookie is
stored and used in subsequent calls.

This feature is usable for use cases where one wants to call the API
without being on ipa client. Non-being on ipa client also means that
IPA's NSS database and configuration is not available. Therefore one
has to define ~/.ipa/default.conf in a similar way as ipa client
does and prepare a NSS database with IPA CA cert.

Usage:

api.Backend.rpcclient.connect(
nss_dir=my_nss_dir_path,
user=user,
password=password
)

It's possible to switch users with:

api.Backend.rpcclient.disconnect()

api.Backend.rpcclient.connect(
nss_dir=my_nss_dir_path,
user=other_user,
password=other_password
)

Or check connection with:

api.Backend.rpcclient.isconnected()

Example: download a CA cert and add it to a new temporary NSS database:
from urllib2 import urlparse
from ipaplatform.paths import paths
from ipapython import certdb, ipautil
from ipapython.ipautil import run
from ipalib import x509

# create new NSSDatabase
tmp_db = certdb.NSSDatabase()
pwd_file = ipautil.write_tmp_file(ipautil.ipa_generate_password())
tmp_db.create_db(pwd_file.name)

# download and add cert
url = urlparse.urlunparse(('http', ipautil.format_netloc(ipa_server),
   '/ipa/config/ca.crt', '', '', ''))
stdout, stderr, rc = run([paths.BIN_WGET, -O, -, url])
certs = x509.load_certificate_list(stdout, tmp_db.secdir)
ca_certs = [cert.der_data for cert in certs]
for i, cert in enumerate(ca_certs):
tmp_db.add_cert(cert, 'CA certificate %d' % (i + 1), 'C,,')

my_nss_dir_path = tmp_db.secdir


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Hi,

thanks for the patch. Please, fix the pep8 complaints.

Can someone else look at the code as well, please?

Thanks,
Milan
-- 
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

[Freeipa-devel] [PATCH 0238] Server Upgrade: fix broken memberUid index

2015-05-06 Thread Martin Basti

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

Patch attached.

--
Martin Basti

From 12d460bffa36791b4123f0bbba4f822aa82ead45 Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Tue, 5 May 2015 19:47:07 +0200
Subject: [PATCH] Server Upgrade: fix memberUid index

https://fedorahosted.org/freeipa/ticket/5007
---
 install/updates/20-indices.update | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/install/updates/20-indices.update b/install/updates/20-indices.update
index 88d62013043cbaddde2a9fd734628cd21796188b..e6e4888e2eda1848b636183528cbf90e22384ea9 100644
--- a/install/updates/20-indices.update
+++ b/install/updates/20-indices.update
@@ -10,8 +10,8 @@ default:cn: memberuid
 default:ObjectClass: top
 default:ObjectClass: nsIndex
 default:nsSystemIndex: false
-default:nsIndexType: eq
-default:nsIndexType: pres
+only:nsIndexType: eq
+only:nsIndexType: pres
 
 dn: cn=memberHost,cn=index,cn=userRoot,cn=ldbm database,cn=plugins,cn=config
 default:cn: memberHost
-- 
2.1.0

-- 
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] [PATCHES 0233-0234] DNSSEC: forwarders validation

2015-05-06 Thread Martin Basti

On 05/05/15 15:00, Martin Basti wrote:

On 30/04/15 15:37, David Kupka wrote:

On 04/24/2015 02:56 PM, Martin Basti wrote:

Patches attached.





Hi,
thanks for patches.

1. You changed message in DNSServerNotRespondingWarning class but not 
the test in ipatest/test_xmlrpc/test_dns_plugin.py


nitpick. Please spell 'edns' correctly. I've seen several instances 
of 'ends'.



Thank you,

updated patches attached:
* new error messages
* logging to debug log server output if exception was raised
* fixed test
* fixed spelling




Fixed tests (again)

Updated patches attached

--
Martin Basti

From acde762eb941e61991f9ec3d8069d723d1021bd8 Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Wed, 22 Apr 2015 15:29:21 +0200
Subject: [PATCH 1/2] DNSSEC: Improve global forwarders validation

Validation now provides more detailed information and less false
positives failures.

https://fedorahosted.org/freeipa/ticket/4657
---
 ipalib/messages.py  |  23 +-
 ipalib/plugins/dns.py   |  64 +---
 ipalib/util.py  | 130 ++--
 ipaserver/install/bindinstance.py   |  30 +---
 ipatests/test_xmlrpc/test_dns_plugin.py |   5 +-
 5 files changed, 186 insertions(+), 66 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index b44beca729f5483a7241e4c98a9f724ed663e70f..236b683b30692d88e5257d9189c559dd9f848885 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -179,14 +179,14 @@ class OptionSemanticChangedWarning(PublicMessage):
u%(hint)s)
 
 
-class DNSServerNotRespondingWarning(PublicMessage):
+class DNSServerValidationWarning(PublicMessage):
 
-**13006**  Used when a DNS server is not responding to queries
+**13006**  Used when a DNS server is not to able to resolve query
 
 
 errno = 13006
 type = warning
-format = _(uDNS server %(server)s not responding.)
+format = _(uDNS server %(server)s: %(error)s.)
 
 
 class DNSServerDoesNotSupportDNSSECWarning(PublicMessage):
@@ -196,10 +196,11 @@ class DNSServerDoesNotSupportDNSSECWarning(PublicMessage):
 
 errno = 13007
 type = warning
-format = _(uDNS server %(server)s does not support DNSSEC. 
+format = _(uDNS server %(server)s does not support DNSSEC: %(error)s.\n
uIf DNSSEC validation is enabled on IPA server(s), 
uplease disable it.)
 
+
 class ForwardzoneIsNotEffectiveWarning(PublicMessage):
 
 **13008** Forwardzone is not effective, forwarding will not work because
@@ -214,6 +215,20 @@ class ForwardzoneIsNotEffectiveWarning(PublicMessage):
u\%(ns_rec)s\ to parent zone \%(authzone)s\.)
 
 
+class DNSServerDoesNotSupportEDNS0Warning(PublicMessage):
+
+**13009** Used when a DNS server does not support EDNS0, required for
+DNSSEC support
+
+
+errno = 13009
+type = warning
+format = _(uDNS server %(server)s does not support EDNS0 (RFC 6891): 
+   u%(error)s.\n
+   uIf DNSSEC validation is enabled on IPA server(s), 
+   uplease disable it.)
+
+
 def iter_messages(variables, base):
 Return a tuple with all subclasses
 
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index f589ab5b77a918b75fe6c48b465ecd9f02cb6d42..d2dcff9084ddf0a2f91b32812e670eb747392b05 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -43,7 +43,10 @@ from ipalib.util import (normalize_zonemgr,
  get_dns_forward_zone_update_policy,
  get_dns_reverse_zone_update_policy,
  get_reverse_zone_default, REVERSE_DNS_ZONES,
- normalize_zone, validate_dnssec_forwarder)
+ normalize_zone, validate_dnssec_global_forwarder,
+ DNSSECSignatureMissingError, UnresolvableRecordError,
+ EDNS0UnsupportedError)
+
 from ipapython.ipautil import CheckedIPAddress, is_host_resolvable
 from ipapython.dnsutil import DNSName
 
@@ -4262,40 +4265,43 @@ class dnsconfig_mod(LDAPUpdate):
 
 def interactive_prompt_callback(self, kw):
 if kw.get('idnsforwarders', False):
-self.Backend.textui.print_plain(Server will check forwarder(s).)
-self.Backend.textui.print_plain(This may take some time, please wait ...)
+self.Backend.textui.print_plain(
+_(Server will check DNS forwarder(s).))
+self.Backend.textui.print_plain(
+_(This may take some time, please wait ...))
 
 def execute(self, *keys, **options):
 # test dnssec forwarders
-non_dnssec_forwarders = []
-not_responding_forwarders = []
 forwarders = options.get('idnsforwarders')
+
+result = super(dnsconfig_mod, self).execute(*keys, **options)
+self.obj.postprocess_result(result)
+
 if forwarders:
 for forwarder in 

Re: [Freeipa-devel] [PATCH 0032] prevent duplicate IDs when setting up multiple replicas against single master

2015-05-06 Thread thierry bordaz

On 05/06/2015 05:56 PM, Martin Babinsky wrote:

On 05/06/2015 04:25 PM, thierry bordaz wrote:

On 05/06/2015 03:19 PM, Martin Babinsky wrote:

Hello Thierry,

replies are inline.

On 05/06/2015 02:22 PM, thierry bordaz wrote:

On 05/06/2015 01:54 PM, Martin Babinsky wrote:

The attached patch tries to fix
https://fedorahosted.org/freeipa/ticket/4378

After discussion with Thierry we concluded that while this issue is
more complex than it seems, the transition from REPLACE to DEL/ADD
operations when updating nsDS5ReplicaId should suffice for this 
ticket.



Hello Martin,

Few comments, you are using MOD_DEL 'replicaID' with None value. So 
this
is going to delete all previous values and it should be equivalent 
to a

MOD_REPL.
I was thinking you wanted to retrieve the id_value and call MOD_DEL
'replicaID' current_value. So that if by the time you fetched the
replicaId, an other replica updated the replicaId, the MOD_DEL/MOD_ADD
would fail and you need a new iteration.


Sorry I didn't know you can MOD_DEL a particular value (I'm LDAP noob
I know). Will fix this.
If replicaId was multi-valued and you want to make it single 
valued, you

may want to do create a more complex MOD (e.g. (ldap.MOD_DELETE,
'nsDS5ReplicaId', str(value1), (ldap.MOD_DELETE, 'nsDS5ReplicaId',
str(value2)...)


AFAIK ReplicaId is single-valued (looking at the schema right now) so
this shouldn't be problem.

If it is updating successfully do you want to return 'retval' or
'retval+1' ?

If several replicas try to update the replicaId of the master and the
current replicaId is 1000.
Replica1 successfully updates the replicaId and gets 1001 as the new
value.
Replica2 successfully updates the replicaId and gets 1002.
The final value on master will be 1002, but replica1 will assum it is
1001. Is it a problem ?


I studied the code in the master branch and IIUC (and please correct
me if I got this wrong) nsDS5ReplicaId attribute in
'cn=replication,cn=etc,$SUFFIX' represents replicaID of the _next_
replica that will be installed.

So if a replica is installed, it sets the current value of
nsDS5ReplicaId as its replica ID (the function returns 'retval') and
then increments it in 'cn=replication,cn=etc,$SUFFIX' entry ('retval +
1' is written to master) so that the next installed replica fetches
this updated value.

So the case you described should be the expected behavior. To change
it would require different patch IMHO.


Thank for your precious explanations, in fact the value in
'cn=replication,cn=etc,SUFFIX' is the next available replicaId value and
is the centralized mechanism that assign unique replicaID.

The risk was that several replica pick the same value. So yes it is
important that the MOD_DEL specifies the previously read value so that
the test/set will be atomic. If several replicas read the same value,
only the faster one will use it to install the replica.

thanks
thierry

thanks
thierry







Attaching updated patch with fixed MOD_DELETE operation.



Hi Martin,

The fix looks good to me except I think you need to do (ldap.MOD_DELETE, 
'nsDS5ReplicaId', *str(*retval*)*)


thanks
thierry
-- 
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] Replication Topology design feedback

2015-05-06 Thread Ludwig Krispenz

hi,

you have to remove the server from the manged servers, 
ipa-replica-manage del would do it, but it fails in line:13 not allowed 
on leaf entry
This is a bug I reported today. In my tests I did use a script to delete 
a master and its services (before).

Tomas has proposed a fix, which should allow ipa-replica-manage del succeed.

What is interesting in your log is line:7, it contains a corrpted ruv, 
and we have been trying to reproduce this, but did fail so far. Could 
you provide me a full log of actvities you have don in this test scenario ?


Regards,
Ludwig

On 05/06/2015 04:47 PM, Oleg Fayans wrote:

Hi Ludwig,

I have a question. What is the proper way of removing ipa replica from
the server that is replication-topology-aware? Standard way
`ipa-replica-manage del` does not work anymore, since the topology is
controlled by the plugin, but I was unable to remove it using
topology-aware tools as well. Here is the transcript of my session:
http://pastebin.test.redhat.com/281213

Thanks in advance!


On 05/04/2015 04:46 PM, Martin Kosek wrote:

Thanks for answers.

BTW, for future, I think Oleg that it would be useful to ask this questions on
freeipa-devel directly as it may be helpful to other developers and we would
have it archived for other uses.

On 05/04/2015 04:20 PM, Ludwig Krispenz wrote:

On 04/30/2015 03:22 PM, Oleg Fayans wrote:

Hi Ludwig,

While getting myself familiar with Replication Topology Plugin design
page I've found a number of places that need some clarifications.

1.

Check at startup.
When the directory server starts, the plugin init and start functions

are executed, they will read the domain level

attribute and act accordingly

Could you describe how exactly should the plugin work depending on the
domain level revealed.

there are only two scenarios:
1] domain_level  plugin_level:
the plugin does almost nothing, reading its config and waiting for a dom level
increase
2] doamin_level = plugin_level:
the plugin controls topology for managed servers, rejecting direct mods of
replication agreements, transforming adding/deleting/modification of segments
into corresponding actions on replication agreements

2.

Check for modify operation
If an admin or tool changes the domain level the plugin detects this

change and performs initialization tasks if the

domain level is greater than the plugin version

The same here: what exactly happens after the domain level has changed.

if the domain level raises, so that teh plugin becomes active, this will happen
on all servers since domain level is replicated.
the plugin will read all the exisußing info inthe shared tree, read all
replication configuration and match them creating missing data in both areas:
cn=config and shared tree

3. Regarding the startup delay. How can I make sure the plugin has started?
As far as I understood the Topology plugin needs to be started only
after all other plugins are started to prevent it from making a tree
changes before it is able to be replicated. The question is: how do I
check whether all other plugins are already started?

the plugin should be started when the server starts, it will expose this in the
root dse and you can search for it, I just updated teh design page with an 
example

4. Shared configuration layout
It is written there, that the replication topology information can be
configured to be stored in the custom place of the tree. How do we do that?

The configuration is in the plugin conf in cn=config. At the moment it is
populated when a DS instance is created.


Most probably I''ll have some more questions once I have the branch
installed.

Thank you!



--
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 0032] prevent duplicate IDs when setting up multiple replicas against single master

2015-05-06 Thread Martin Babinsky

On 05/06/2015 04:25 PM, thierry bordaz wrote:

On 05/06/2015 03:19 PM, Martin Babinsky wrote:

Hello Thierry,

replies are inline.

On 05/06/2015 02:22 PM, thierry bordaz wrote:

On 05/06/2015 01:54 PM, Martin Babinsky wrote:

The attached patch tries to fix
https://fedorahosted.org/freeipa/ticket/4378

After discussion with Thierry we concluded that while this issue is
more complex than it seems, the transition from REPLACE to DEL/ADD
operations when updating nsDS5ReplicaId should suffice for this ticket.


Hello Martin,

Few comments, you are using MOD_DEL 'replicaID' with None value. So this
is going to delete all previous values and it should be equivalent to a
MOD_REPL.
I was thinking you wanted to retrieve the id_value and call MOD_DEL
'replicaID' current_value. So that if by the time you fetched the
replicaId, an other replica updated the replicaId, the MOD_DEL/MOD_ADD
would fail and you need a new iteration.


Sorry I didn't know you can MOD_DEL a particular value (I'm LDAP noob
I know). Will fix this.

If replicaId was multi-valued and you want to make it single valued, you
may want to do create a more complex MOD (e.g. (ldap.MOD_DELETE,
'nsDS5ReplicaId', str(value1), (ldap.MOD_DELETE, 'nsDS5ReplicaId',
str(value2)...)


AFAIK ReplicaId is single-valued (looking at the schema right now) so
this shouldn't be problem.

If it is updating successfully do you want to return 'retval' or
'retval+1' ?

If several replicas try to update the replicaId of the master and the
current replicaId is 1000.
Replica1 successfully updates the replicaId and gets 1001 as the new
value.
Replica2 successfully updates the replicaId and gets 1002.
The final value on master will be 1002, but replica1 will assum it is
1001. Is it a problem ?


I studied the code in the master branch and IIUC (and please correct
me if I got this wrong) nsDS5ReplicaId attribute in
'cn=replication,cn=etc,$SUFFIX' represents replicaID of the _next_
replica that will be installed.

So if a replica is installed, it sets the current value of
nsDS5ReplicaId as its replica ID (the function returns 'retval') and
then increments it in 'cn=replication,cn=etc,$SUFFIX' entry ('retval +
1' is written to master) so that the next installed replica fetches
this updated value.

So the case you described should be the expected behavior. To change
it would require different patch IMHO.


Thank for your precious explanations, in fact the value in
'cn=replication,cn=etc,SUFFIX' is the next available replicaId value and
is the centralized mechanism that assign unique replicaID.

The risk was that several replica pick the same value. So yes it is
important that the MOD_DEL specifies the previously read value so that
the test/set will be atomic. If several replicas read the same value,
only the faster one will use it to install the replica.

thanks
thierry

thanks
thierry







Attaching updated patch with fixed MOD_DELETE operation.

--
Martin^3 Babinsky
From bdb686454ae7a7066ea5b568b91fcf88ba78d4b8 Mon Sep 17 00:00:00 2001
From: Martin Babinsky mbabi...@redhat.com
Date: Mon, 4 May 2015 18:33:44 +0200
Subject: [PATCH] prevent duplicate IDs when setting up multiple replicas
 against single master

This patch forces replicas to use DELETE+ADD operations to increment
'nsDS5ReplicaId' in 'cn=replication,cn=etc,$SUFFIX' on master, and retry
multiple times in the case of conflict with another update. Thus when multiple
replicas are set-up against single master none of them will have duplicate ID.

https://fedorahosted.org/freeipa/ticket/4378
---
 ipaserver/install/replication.py | 78 ++--
 1 file changed, 52 insertions(+), 26 deletions(-)

diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 66764c22f69328942fe2e4581cfafb3806438d7c..57d4b9917781e5d459856480050c8ce1457c25dd 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -21,6 +21,7 @@ import time
 import datetime
 import sys
 import os
+from random import randint
 
 import ldap
 
@@ -230,34 +231,59 @@ class ReplicationManager(object):
 
 # Ok, either the entry doesn't exist or the attribute isn't set
 # so get it from the other master
-retval = -1
+return self._get_and_update_id_from_master(master_conn)
+
+def _get_and_update_id_from_master(self, master_conn, attempts=5):
+
+Fetch replica ID from remote master and update nsDS5ReplicaId attribute
+on 'cn=replication,cn=etc,$SUFFIX' entry. Do it as MOD_DELETE+MOD_ADD
+operations and retry when conflict occurs, e.g. due to simultaneous
+update from another replica.
+:param master_conn: LDAP connection to master
+:param attempts: number of attempts to update nsDS5ReplicaId
+:return: value of nsDS5ReplicaId before incrementation
+
 dn = DN(('cn','replication'),('cn','etc'), self.suffix)
-try:
-replica = master_conn.get_entry(dn)
-