[Freeipa-devel] [PATCH 0008-0009] use 1 as domain level to activate plugin, fix a crash when removing a replica
Fraser Tweedale
ftweedal at redhat.com
Tue Jun 2 10:59:05 UTC 2015
On Tue, Jun 02, 2015 at 10:04:55AM +0200, Ludwig Krispenz wrote:
> Hi,
>
> with the first patch the topo plugin no longer uses plugin version to
> compare to set domainlevel, always gets activated if dom level >= 1
> the second patch fixes a crash at replica removal
>
> Ludwig
These patches fix the issue for me.
I don't know what is (supposed to be) happening in the code. Is my
testing enough for the ACK?
Cheers,
Fraser
> >From 7e08b6181973cc51e50eae69974682878b8ca66b Mon Sep 17 00:00:00 2001
> From: Ludwig Krispenz <lkrispen at redhat.com>
> Date: Tue, 2 Jun 2015 09:17:54 +0200
> Subject: [PATCH] plugin uses 1 as minimum domain level to become active no
> calculation based on plugin version
>
> ---
> daemons/ipa-slapi-plugins/topology/topology.h | 9 ++------
> daemons/ipa-slapi-plugins/topology/topology_cfg.c | 25 +++++++---------------
> daemons/ipa-slapi-plugins/topology/topology_init.c | 2 +-
> daemons/ipa-slapi-plugins/topology/topology_util.c | 4 +---
> 4 files changed, 12 insertions(+), 28 deletions(-)
>
> diff --git a/daemons/ipa-slapi-plugins/topology/topology.h b/daemons/ipa-slapi-plugins/topology/topology.h
> index 38c2823f50153c6b02a234608869617c92d1bdf2..4135a8ff71b9160919a089fde63a95a989830de8 100644
> --- a/daemons/ipa-slapi-plugins/topology/topology.h
> +++ b/daemons/ipa-slapi-plugins/topology/topology.h
> @@ -125,11 +125,6 @@ typedef struct topo_plugin_config {
> int activated;
> } TopoPluginConf;
>
> -typedef struct ipa_domain_level {
> - int major;
> - int minor;
> -} IpaDomainLevel;
> -
> #define CONFIG_ATTR_SHARED_BASE "nsslapd-topo-plugin-shared-config-base"
> #define CONFIG_ATTR_REPLICA_ROOT "nsslapd-topo-plugin-shared-replica-root"
> #define CONFIG_ATTR_SHARED_BINDDNGROUP "nsslapd-topo-plugin-shared-binddngroup"
> @@ -158,8 +153,8 @@ int ipa_topo_get_plugin_version_major(void);
> int ipa_topo_get_plugin_version_minor(void);
> char *ipa_topo_get_domain_level_entry(void);
> Slapi_DN *ipa_topo_get_domain_level_entry_dn(void);
> -int ipa_topo_get_domain_level_major(void);
> -int ipa_topo_get_domain_level_minor(void);
> +int ipa_topo_get_domain_level(void);
> +int ipa_topo_get_min_domain_level(void);
> int ipa_topo_get_plugin_startup_delay(void);
> void ipa_topo_set_plugin_id(void *plg_id);
> void ipa_topo_set_plugin_active(int state);
> diff --git a/daemons/ipa-slapi-plugins/topology/topology_cfg.c b/daemons/ipa-slapi-plugins/topology/topology_cfg.c
> index 17493495af83d1095fbafead104d6f56bd7af10e..982ad647db9737c1aa0fc7f68c7d9b20de895fb6 100644
> --- a/daemons/ipa-slapi-plugins/topology/topology_cfg.c
> +++ b/daemons/ipa-slapi-plugins/topology/topology_cfg.c
> @@ -10,7 +10,8 @@
> */
> static TopoPluginConf topo_plugin_conf = {0};
> static TopoReplicaConf topo_shared_conf = {0};
> -static IpaDomainLevel ipa_domain_level = {0,0};
> +static int ipa_domain_level = 0;
> +static int topo_min_domain_level = 1;
>
> char *ipa_topo_plugin_managed_attrs[] = {
> "nsds5ReplicaStripAttrs",
> @@ -95,15 +96,15 @@ ipa_topo_get_domain_level_entry_dn(void)
> }
>
> int
> -ipa_topo_get_domain_level_major(void)
> +ipa_topo_get_min_domain_level(void)
> {
> - return ipa_domain_level.major;
> + return topo_min_domain_level;
> }
>
> int
> -ipa_topo_get_domain_level_minor(void)
> +ipa_topo_get_domain_level(void)
> {
> - return ipa_domain_level.minor;
> + return ipa_domain_level;
> }
>
> char *
> @@ -199,22 +200,12 @@ ipa_topo_set_plugin_shared_bindgroup(char *bindgroup)
> void
> ipa_topo_set_domain_level(char *level)
> {
> - char *minor;
> -
> if (level == NULL) {
> - ipa_domain_level.major = 0;
> - ipa_domain_level.minor = 0;
> + ipa_domain_level = 0;
> return;
> }
>
> - minor = strchr(level,'.');
> - if (minor) {
> - *minor = '\0';
> - ipa_domain_level.minor = atoi(++minor);
> - } else {
> - ipa_domain_level.minor = 0;
> - }
> - ipa_domain_level.major = atoi(level);
> + ipa_domain_level = atoi(level);
> }
>
> void
> diff --git a/daemons/ipa-slapi-plugins/topology/topology_init.c b/daemons/ipa-slapi-plugins/topology/topology_init.c
> index 77e740ea182c2331c88d2716d1c4f7be8ef8c257..af5b8021f4ba6833dff11d9c89543e9bb74bdeb9 100644
> --- a/daemons/ipa-slapi-plugins/topology/topology_init.c
> +++ b/daemons/ipa-slapi-plugins/topology/topology_init.c
> @@ -264,7 +264,7 @@ ipa_topo_rootdse_search(Slapi_PBlock *pb, Slapi_Entry* e, Slapi_Entry* entryAfte
> /* we expose temporarily the domain level in this function, should
> * finally be handled in a plugin managing the domain level
> */
> - char *level = slapi_ch_smprintf("%d", ipa_topo_get_domain_level_major());
> + char *level = slapi_ch_smprintf("%d", ipa_topo_get_domain_level());
> slapi_entry_attr_set_charptr(e, "ipaDomainLevel", level);
> slapi_ch_free_string(&version);
> slapi_ch_free_string(&level);
> diff --git a/daemons/ipa-slapi-plugins/topology/topology_util.c b/daemons/ipa-slapi-plugins/topology/topology_util.c
> index f206464a5b47b9dc7e0edd5dd764228b076b6dd9..d487cfb638ac9bd0fb94cdd2638d5fd5ae4e6908 100644
> --- a/daemons/ipa-slapi-plugins/topology/topology_util.c
> +++ b/daemons/ipa-slapi-plugins/topology/topology_util.c
> @@ -110,9 +110,7 @@ ipa_topo_util_get_pluginhost(void)
> void
> ipa_topo_util_check_plugin_active(void)
> {
> - if (ipa_topo_get_plugin_version_major() < ipa_topo_get_domain_level_major() ||
> - (ipa_topo_get_plugin_version_major() == ipa_topo_get_domain_level_major() &&
> - ipa_topo_get_plugin_version_minor() <= ipa_topo_get_domain_level_minor())) {
> + if (ipa_topo_get_min_domain_level() <= ipa_topo_get_domain_level()) {
> ipa_topo_set_plugin_active(1);
> } else {
> ipa_topo_set_plugin_active(0);
> --
> 2.1.0
>
> >From 2f27a90394da56925694d771592c9fe3ae40eeeb Mon Sep 17 00:00:00 2001
> From: Ludwig Krispenz <lkrispen at redhat.com>
> Date: Tue, 2 Jun 2015 09:29:23 +0200
> Subject: [PATCH] crash when removing a replica
>
> when a server is removed from the topology the plugin tries to remove the
> credentials from the replica and the bind dn group.
> It performs an internal search for the ldap principal, but can fail if it was already removed
> Due to an unitialized variable in this case it can eitehr crash or erroneously remove all
> principals.
> ---
> daemons/ipa-slapi-plugins/topology/topology_util.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/daemons/ipa-slapi-plugins/topology/topology_util.c b/daemons/ipa-slapi-plugins/topology/topology_util.c
> index d487cfb638ac9bd0fb94cdd2638d5fd5ae4e6908..67014a05d4f89260d4307e5212a5594335617482 100644
> --- a/daemons/ipa-slapi-plugins/topology/topology_util.c
> +++ b/daemons/ipa-slapi-plugins/topology/topology_util.c
> @@ -1201,7 +1201,15 @@ void
> ipa_topo_util_disable_repl_from_host(char *repl_root, char *delhost)
> {
> char *principal = ipa_topo_util_get_ldap_principal(repl_root, delhost);
> - ipa_topo_util_disable_repl_for_principal(repl_root, principal);
> + if (principal) {
> + ipa_topo_util_disable_repl_for_principal(repl_root, principal);
> + slapi_ch_free_string(&principal);
> + } else {
> + slapi_log_error(SLAPI_LOG_PLUGIN, IPA_TOPO_PLUGIN_SUBSYSTEM,
> + "ipa_topo_util_disable_repl_from_host: "
> + "failed to get ldap principal for host: %s \n",
> + delhost);
> + }
> }
>
> void
> @@ -1322,10 +1330,10 @@ char *
> ipa_topo_util_get_ldap_principal(char *repl_root, char *hostname)
> {
> int rc = 0;
> - Slapi_Entry **entries;
> + Slapi_Entry **entries = NULL;
> Slapi_PBlock *pb = NULL;
> char *filter;
> - char *dn;
> + char *dn = NULL;
>
> filter = slapi_ch_smprintf("krbprincipalname=ldap/%s*",hostname);
> pb = slapi_pblock_new();
> --
> 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
More information about the Freeipa-devel
mailing list