Skip to content

Commit

Permalink
Incompatible IPA versions, pausing replication
Browse files Browse the repository at this point in the history
Bug Description:
	The bug is not systematic. Symptoms are replication broken and this message
	ERR - repl_version_plugin_recv_acquire_cb - [file ipa_repl_version.c, line 119]: Incompatible IPA versions, pausing replication
	IPA Version Replication is a plugin that checks replication version (payload).
	It does the checking in the early phase of replication protocol (acquire_replica).
	The plugin must be enabled on both side, or disabled on both side.
	If we have a mixed setting (i.e. disabled on Master and enabled on Replica), there is
	a risk that the Master sends an incomplete payload. The incomplete payload will be rejected
	by the Replica and replication in both direction will fail.
	The risk is that on the master, the plugin got initialized but not started.
	Initialization of the plugin registers a broker API that allows a plugin (replication) to call
	a callback even if the plugin is not started.
	The problem is that to build valid payload the 'IPA Version Replication' needs to be started
	(data_version to be initialized).

Fix Description:
	The fix consist in:
		- define data_version during plugin initialization
		- Make plugin callback as noop if the plugin is not started. This is just for perf.
		- Do not enable 'IPA Version Replication' plugin

https://pagure.io/freeipa/issue/7690
  • Loading branch information
tbordaz committed Sep 11, 2018
1 parent c049992 commit 40edc91
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 8 deletions.
16 changes: 15 additions & 1 deletion daemons/ipa-slapi-plugins/ipa-version/ipa_repl_version.c
Expand Up @@ -45,6 +45,7 @@

#define IPA_PLUGIN_NAME "ipa_replication_version"
static char *data_version = NULL;
static PRBool plugin_started = PR_FALSE;

/*
* Plugin identifiers
Expand Down Expand Up @@ -74,6 +75,9 @@ static int
repl_version_plugin_pre_acquire_cb(void *cookie, const Slapi_DN *repl_subtree,
int is_total, char **data_guid, struct berval **data)
{
if (!plugin_started) {
return 0;
}
LOG("repl_version_plugin_pre_acquire_cb() called for suffix \"%s\", "
"is_total: \"%s\".\n", slapi_sdn_get_ndn(repl_subtree),
is_total ? "TRUE" : "FALSE");
Expand Down Expand Up @@ -106,6 +110,9 @@ static int
repl_version_plugin_recv_acquire_cb(const char *repl_subtree, int is_total,
const char *data_guid, const struct berval *data)
{
if (!plugin_started) {
return 0;
}
LOG("test_repl_session_plugin_recv_acquire_cb() called for suffix \"%s\", is_total: \"%s\".\n",
repl_subtree, is_total ? "TRUE" : "FALSE");

Expand Down Expand Up @@ -145,7 +152,9 @@ repl_version_plugin_start(Slapi_PBlock *pb)
{
LOG("--> repl_version_plugin_start -- begin\n");

data_version = slapi_ch_smprintf("%llu", (unsigned long long) DATA_VERSION);
if (data_version == NULL) {
data_version = slapi_ch_smprintf("%llu", (unsigned long long) DATA_VERSION);
}

LOG("<-- repl_version_plugin_start -- end\n");
return 0;
Expand Down Expand Up @@ -181,6 +190,11 @@ int repl_version_plugin_init(Slapi_PBlock *pb)
return -1;
}

/* broker API allows plugin to call repl_version callback EVEN if the plugin is not
* started. It is then important to initialize data_version (used in callbacks) at the
* same time as the callbacks are registered
*/
data_version = slapi_ch_smprintf("%llu", (unsigned long long) DATA_VERSION);
if( slapi_apib_register(REPL_SESSION_v1_0_GUID, repl_version_api) ) {
LOG_FATAL("<-- repl_version_plugin_start -- failed to register repl_version api -- end\n");
return -1;
Expand Down
5 changes: 2 additions & 3 deletions ipaserver/install/dsinstance.py
Expand Up @@ -444,9 +444,8 @@ def __setup_replica(self):
is used (we do not have access to masters' DM password in this
stage)
"""
replication.enable_replication_version_checking(
self.realm,
self.dm_password)
serverid = "-".join(self.realm.split("."))
installutils.restart_dirsrv(serverid)

repl, bind_dn, bind_pw = self._get_replication_manager()
repl.setup_promote_replication(
Expand Down
4 changes: 0 additions & 4 deletions ipaserver/install/ipa_replica_prepare.py
Expand Up @@ -34,7 +34,6 @@
import six

from ipaserver.install import certs, installutils, bindinstance, dsinstance, ca
from ipaserver.install.replication import enable_replication_version_checking
from ipaserver.install.server.replicainstall import install_ca_cert
from ipaserver.install.bindinstance import (
add_zone, add_fwd_rr, add_ptr_rr, dns_container_exists)
Expand Down Expand Up @@ -321,9 +320,6 @@ def run(self):

logger.info("Preparing replica for %s from %s",
self.replica_fqdn, api.env.host)
enable_replication_version_checking(
api.env.realm,
self.dirman_password)

self.top_dir = tempfile.mkdtemp("ipa")
self.dir = os.path.join(self.top_dir, "realm_info")
Expand Down

0 comments on commit 40edc91

Please sign in to comment.