From 33f562bba3729bc596e07dc2805d78b80de55784 Mon Sep 17 00:00:00 2001
From: Christian Heimes <cheimes@redhat.com>
Date: Tue, 10 Jul 2018 14:03:28 +0200
Subject: [PATCH] Handle races in replica config
When multiple replicas are installed in parallel, two replicas may try
to create the cn=replica entry at the same time. This leads to a
conflict on one of the replicas. replica_config() and
ensure_replication_managers() now handle conflicts.
ipaldap now maps TYPE_OR_VALUE_EXISTS to DuplicateEntry(). The type or
value exists exception is raised, when an attribute value or type is
already set.
Fixes: https://pagure.io/freeipa/issue/7566
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Thierry Bordaz <tbordaz@redhat.com>
Reviewed-By: Thierry Bordaz <tbordaz@redhat.com>
---
ipapython/ipaldap.py | 5 ++
ipaserver/install/replication.py | 125 ++++++++++++++++++-------------
ipaserver/plugins/ldap2.py | 3 +-
3 files changed, 81 insertions(+), 52 deletions(-)
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 1b0aaddd63d92776448d1a7ae6c1bd26a02d5dcc..e2ff0626986aa20f3a2bc42721fba9fad3156498 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -965,7 +965,12 @@ class LDAPClient(object):
except ldap.NO_SUCH_OBJECT:
raise errors.NotFound(reason=arg_desc or 'no such entry')
except ldap.ALREADY_EXISTS:
+ # entry already exists
raise errors.DuplicateEntry()
+ except ldap.TYPE_OR_VALUE_EXISTS:
+ # attribute type or attribute value already exists, usually only
+ # occurs, when two machines try to write at the same time.
+ raise errors.DuplicateEntry(message=unicode(desc))
except ldap.CONSTRAINT_VIOLATION:
# This error gets thrown by the uniqueness plugin
_msg = 'Another entry with the same attribute value already exists'
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index c017764468674830670a817b3d815c5e2d78728e..ffda9a182f840317d96f1b3b914b38233022fb5b 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -34,7 +34,7 @@ from ipalib import api, errors
from ipalib.cli import textui
from ipapython.ipa_log_manager import root_logger
from ipalib.text import _
-from ipapython import ipautil, ipaldap, kerberos
+from ipapython import ipautil, ipaldap
from ipapython.admintool import ScriptError
from ipapython.dn import DN
from ipaplatform.paths import paths
@@ -457,7 +457,7 @@ class ReplicationManager(object):
return DN(('cn', 'replica'), ('cn', self.db_suffix),
('cn', 'mapping tree'), ('cn', 'config'))
- def set_replica_binddngroup(self, r_conn, entry):
+ def _set_replica_binddngroup(self, r_conn, entry):
"""
Set nsds5replicabinddngroup attribute on remote master's replica entry.
Older masters (ipa < 3.3) may not support setting this attribute. In
@@ -472,11 +472,6 @@ class ReplicationManager(object):
mod.append((ldap.MOD_ADD, 'nsds5replicabinddngroup',
self.repl_man_group_dn))
- if 'nsds5replicabinddngroupcheckinterval' not in entry:
- mod.append(
- (ldap.MOD_ADD,
- 'nsds5replicabinddngroupcheckinterval',
- '60'))
if mod:
try:
r_conn.modify_s(entry.dn, mod)
@@ -484,49 +479,64 @@ class ReplicationManager(object):
root_logger.debug(
"nsds5replicabinddngroup attribute not supported on "
"remote master.")
+ except (ldap.ALREADY_EXISTS, ldap.CONSTRAINT_VIOLATION):
+ root_logger.debug("No update to %s necessary", entry.dn)
def replica_config(self, conn, replica_id, replica_binddn):
assert isinstance(replica_binddn, DN)
dn = self.replica_dn()
assert isinstance(dn, DN)
+ root_logger.debug("Add or update replica config %s", dn)
try:
entry = conn.get_entry(dn)
except errors.NotFound:
- pass
- else:
- binddns = entry.setdefault('nsDS5ReplicaBindDN', [])
- if replica_binddn not in {DN(m) for m in binddns}:
- # Add the new replication manager
- binddns.append(replica_binddn)
- for key, value in REPLICA_CREATION_SETTINGS.items():
- entry[key] = value
+ # no entry, create new one
+ entry = conn.make_entry(
+ dn,
+ objectclass=["top", "nsds5replica", "extensibleobject"],
+ cn=["replica"],
+ nsds5replicaroot=[str(self.db_suffix)],
+ nsds5replicaid=[str(replica_id)],
+ nsds5replicatype=[self.get_replica_type()],
+ nsds5flags=["1"],
+ nsds5replicabinddn=[replica_binddn],
+ nsds5replicabinddngroup=[self.repl_man_group_dn],
+ nsds5replicalegacyconsumer=["off"],
+ **REPLICA_CREATION_SETTINGS
+ )
try:
- conn.update_entry(entry)
- except errors.EmptyModlist:
- pass
+ conn.add_entry(entry)
+ except errors.DuplicateEntry:
+ root_logger.debug(
+ "Lost race against another replica, updating"
+ )
+ # fetch entry that have been added by another replica
+ entry = conn.get_entry(dn)
+ else:
+ root_logger.debug("Added replica config %s", dn)
+ # added entry successfully
+ return entry
- self.set_replica_binddngroup(conn, entry)
+ # either existing entry or lost race
+ binddns = entry.setdefault('nsDS5ReplicaBindDN', [])
+ if replica_binddn not in {DN(m) for m in binddns}:
+ # Add the new replication manager
+ binddns.append(replica_binddn)
- # replication is already configured
- return
+ for key, value in REPLICA_CREATION_SETTINGS.items():
+ entry[key] = value
- replica_type = self.get_replica_type()
+ try:
+ conn.update_entry(entry)
+ except errors.EmptyModlist:
+ root_logger.debug("No update to %s necessary", entry.dn)
+ else:
+ root_logger.debug("Update replica config %s", entry.dn)
- entry = conn.make_entry(
- dn,
- objectclass=["top", "nsds5replica", "extensibleobject"],
- cn=["replica"],
- nsds5replicaroot=[str(self.db_suffix)],
- nsds5replicaid=[str(replica_id)],
- nsds5replicatype=[replica_type],
- nsds5flags=["1"],
- nsds5replicabinddn=[replica_binddn],
- nsds5replicabinddngroup=[self.repl_man_group_dn],
- nsds5replicalegacyconsumer=["off"],
- **REPLICA_CREATION_SETTINGS
- )
- conn.add_entry(entry)
+ self._set_replica_binddngroup(conn, entry)
+
+ return entry
def setup_changelog(self, conn):
ent = conn.get_entry(
@@ -686,7 +696,10 @@ class ReplicationManager(object):
uid=["passsync"],
userPassword=[password],
)
- conn.add_entry(entry)
+ try:
+ conn.add_entry(entry)
+ except errors.DuplicateEntry:
+ pass
# Add the user to the list of users allowed to bypass password policy
extop_dn = DN(('cn', 'ipa_pwd_extop'), ('cn', 'plugins'), ('cn', 'config'))
@@ -1644,7 +1657,10 @@ class ReplicationManager(object):
objectclass=['top', 'groupofnames'],
cn=['replication managers']
)
- conn.add_entry(entry)
+ try:
+ conn.add_entry(entry)
+ except errors.DuplicateEntry:
+ pass
def ensure_replication_managers(self, conn, r_hostname):
"""
@@ -1654,23 +1670,24 @@ class ReplicationManager(object):
On FreeIPA 3.x masters lacking support for nsds5ReplicaBinddnGroup
attribute, add replica bind DN directly into the replica entry.
"""
- my_princ = kerberos.Principal((u'ldap', unicode(self.hostname)),
- realm=self.realm)
- remote_princ = kerberos.Principal((u'ldap', unicode(r_hostname)),
- realm=self.realm)
- services_dn = DN(api.env.container_service, api.env.basedn)
-
- mydn, remote_dn = tuple(
- DN(('krbprincipalname', unicode(p)), services_dn) for p in (
- my_princ, remote_princ))
+ my_dn = DN(
+ ('krbprincipalname', u'ldap/%s@%s' % (self.hostname, self.realm)),
+ api.env.container_service,
+ api.env.basedn
+ )
+ remote_dn = DN(
+ ('krbprincipalname', u'ldap/%s@%s' % (r_hostname, self.realm)),
+ api.env.container_service,
+ api.env.basedn
+ )
try:
conn.get_entry(self.repl_man_group_dn)
except errors.NotFound:
- self._add_replica_bind_dn(conn, mydn)
+ self._add_replica_bind_dn(conn, my_dn)
self._add_replication_managers(conn)
- self._add_dn_to_replication_managers(conn, mydn)
+ self._add_dn_to_replication_managers(conn, my_dn)
self._add_dn_to_replication_managers(conn, remote_dn)
def add_temp_sasl_mapping(self, conn, r_hostname):
@@ -1690,7 +1707,10 @@ class ReplicationManager(object):
entry = conn.get_entry(self.replica_dn())
entry['nsDS5ReplicaBindDN'].append(replica_binddn)
- conn.update_entry(entry)
+ try:
+ conn.update_entry(entry)
+ except errors.EmptyModlist:
+ pass
entry = conn.make_entry(
DN(('cn', 'Peer Master'), ('cn', 'mapping'), ('cn', 'sasl'),
@@ -1702,7 +1722,10 @@ class ReplicationManager(object):
nsSaslMapFilterTemplate=['(cn=&@%s)' % self.realm],
nsSaslMapPriority=['1'],
)
- conn.add_entry(entry)
+ try:
+ conn.add_entry(entry)
+ except errors.DuplicateEntry:
+ pass
def remove_temp_replication_user(self, conn, r_hostname):
"""
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 3b1e4da57a8e16e3d9b27eea24025de2caa53216..c0a0ed251f98cd06ccd13c873f38809d04d1b5d5 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -422,7 +422,8 @@ class ldap2(CrudBackend, LDAPClient):
modlist = [(a, b, self.encode(c))
for a, b, c in modlist]
self.conn.modify_s(str(group_dn), modlist)
- except errors.DatabaseError:
+ except errors.DuplicateEntry:
+ # TYPE_OR_VALUE_EXISTS
raise errors.AlreadyGroupMember()
def remove_entry_from_group(self, dn, group_dn, member_attr='member'):
--
2.17.1