Thanks,
attached a new version with comments and trying to use more meaningful
function names
On 06/11/2015 10:49 AM, thierry bordaz wrote:
On 06/11/2015 10:40 AM, Ludwig Krispenz wrote:
On 06/11/2015 10:27 AM, thierry bordaz wrote:
On 06/11/2015 08:12 AM, Ludwig Krispenz wrote:
Attached are two patches:
- reject direct modification of segment endpoints and connectivity
- better manage the rdn of a replication agreements represented by
a segment
Hello Ludwig,
The patches looks good. Two questions:
* Patch 0012
if (strcasecmp(agmt_rdn_str, topo_agmt->rdn)) {
slapi_ch_free_string(&topo_agmt->rdn);
topo_agmt->rdn = slapi_ch_strdup(agmt_rdn_str);
}
What is the benefit of replacing a string by the same one ?
if strcasecmp is not 0, they are not the same
Shame on me !
* Patch 0013
In ipa-topo-pre-mod.
if (ipa_topo_is_entry_managed(pb)){
if(ipa_topo_is_agmt_attr_restricted(pb)) {
errtxt = slapi_ch_smprintf("Entry and attributes are
managed by topology plugin."
"No direct modifications
allowed.\n");
}
} else if (ipa_topo_check_connect_restrict(pb)) {
errtxt = slapi_ch_smprintf("Modification of connectivity
and segment nodes "
" is not supported.\n");
}
If we have an external modify of replication agreement (managed
by topology plugin), then it will not call
'ipa_topo_check_connect_restrict'.
And the modify will not be reject if for example it updates
'ipaReplTopoSegmentDirection'.
this is not in an replication agreement. you cannot modify two
entries in the same mod. The first if catches mos to a repl
agreement, the second to a segment entry
Ok. Thanks for the explanations. To help reading you may add a comment
saying the the first test is related to RA and the second to segments.
Other than that the fix is good for me. ACK
* But I thought that this patch also wants to prevent external
update of some connectivity attribute of the managed entries.
Thanks
thierry
>From 8b131201fb47d4cc04acea8128042cd6cf2efbb0 Mon Sep 17 00:00:00 2001
From: Ludwig Krispenz <lkris...@redhat.com>
Date: Thu, 11 Jun 2015 11:22:19 +0200
Subject: [PATCH] v2-reject modifications of endpoints and connectivity of a
segment
---
daemons/ipa-slapi-plugins/topology/topology_pre.c | 69 ++++++++++++++++++++---
1 file changed, 60 insertions(+), 9 deletions(-)
diff --git a/daemons/ipa-slapi-plugins/topology/topology_pre.c b/daemons/ipa-slapi-plugins/topology/topology_pre.c
index 0a0cd65b592e2dc796a179e035598e5f641bb01e..cf935d8d2ad65b3c95f408d44511a8c0449c7288 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_pre.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_pre.c
@@ -60,7 +60,7 @@ int ipa_topo_is_entry_managed(Slapi_PBlock *pb)
}
int
-ipa_topo_is_modattr_restricted(Slapi_PBlock *pb)
+ipa_topo_is_agmt_attr_restricted(Slapi_PBlock *pb)
{
LDAPMod **mods;
int i;
@@ -75,6 +75,24 @@ ipa_topo_is_modattr_restricted(Slapi_PBlock *pb)
}
return rc;
}
+int
+ipa_topo_is_segm_attr_restricted(Slapi_PBlock *pb)
+{
+ LDAPMod **mods;
+ int i;
+ int rc = 0;
+
+ slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods);
+ for (i = 0; (mods != NULL) && (mods[i] != NULL); i++) {
+ if ((0 == strcasecmp(mods[i]->mod_type, "ipaReplTopoSegmentDirection")) ||
+ (0 == strcasecmp(mods[i]->mod_type, "ipaReplTopoSegmentLeftNode")) ||
+ (0 == strcasecmp(mods[i]->mod_type, "ipaReplTopoSegmentRightNode"))) {
+ rc = 1;
+ break;
+ }
+ }
+ return rc;
+}
/* connectivity check for topology
* checks if the nodes of a segment would still be connected after
@@ -266,7 +284,7 @@ ipa_topo_connection_exists(struct node_fanout *fanout, char* from, char *to)
}
int
-ipa_topo_check_connect_reject(Slapi_PBlock *pb)
+ipa_topo_check_segment_is_valid(Slapi_PBlock *pb)
{
int rc = 0;
Slapi_Entry *add_entry;
@@ -309,7 +327,29 @@ ipa_topo_check_connect_reject(Slapi_PBlock *pb)
}
int
-ipa_topo_check_disconnect_reject(Slapi_PBlock *pb)
+ipa_topo_check_segment_updates(Slapi_PBlock *pb)
+{
+ int rc = 0;
+ Slapi_Entry *mod_entry;
+ char *pi;
+
+ /* we have to check if the operation is triggered by the
+ * topology plugin itself - allow it
+ */
+ slapi_pblock_get(pb, SLAPI_PLUGIN_IDENTITY,&pi);
+ if (pi && 0 == strcasecmp(pi, ipa_topo_get_plugin_id())) {
+ return 0;
+ }
+ slapi_pblock_get(pb,SLAPI_MODIFY_EXISTING_ENTRY,&mod_entry);
+ if (TOPO_SEGMENT_ENTRY == ipa_topo_check_entry_type(mod_entry) &&
+ (ipa_topo_is_segm_attr_restricted(pb))) {
+ rc = 1;
+ }
+ return rc;
+}
+
+int
+ipa_topo_check_topology_disconnect(Slapi_PBlock *pb)
{
int rc = 1;
Slapi_Entry *del_entry;
@@ -385,7 +425,7 @@ int ipa_topo_pre_add(Slapi_PBlock *pb)
slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, errtxt);
slapi_pblock_set(pb, SLAPI_RESULT_CODE, &rc);
result = SLAPI_PLUGIN_FAILURE;
- } else if (ipa_topo_check_connect_reject(pb)) {
+ } else if (ipa_topo_check_segment_is_valid(pb)) {
int rc = LDAP_UNWILLING_TO_PERFORM;
char *errtxt;
errtxt = slapi_ch_smprintf("Segment already exists in topology or"
@@ -403,6 +443,7 @@ ipa_topo_pre_mod(Slapi_PBlock *pb)
{
int result = SLAPI_PLUGIN_SUCCESS;
+ char *errtxt = NULL;
slapi_log_error(SLAPI_LOG_PLUGIN, IPA_TOPO_PLUGIN_SUBSYSTEM,
"--> ipa_topo_pre_mod\n");
@@ -415,11 +456,21 @@ ipa_topo_pre_mod(Slapi_PBlock *pb)
if (ipa_topo_pre_ignore_op(pb)) return result;
- if (ipa_topo_is_entry_managed(pb) && ipa_topo_is_modattr_restricted(pb)) {
+ if (ipa_topo_is_entry_managed(pb)){
+ /* this means it is a replication agreement targeting a managed server
+ * next check is if it tries to modify restricted attributes
+ */
+ if(ipa_topo_is_agmt_attr_restricted(pb)) {
+ errtxt = slapi_ch_smprintf("Entry and attributes are managed by topology plugin."
+ "No direct modifications allowed.\n");
+ }
+ } else if (ipa_topo_check_segment_updates(pb)) {
+ /* some updates to segments are not supported */
+ errtxt = slapi_ch_smprintf("Modification of connectivity and segment nodes "
+ " is not supported.\n");
+ }
+ if (errtxt) {
int rc = LDAP_UNWILLING_TO_PERFORM;
- char *errtxt;
- errtxt = slapi_ch_smprintf("Entry and attributes are managed by topology plugin."
- "No direct modifications allowed.\n");
slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, errtxt);
slapi_pblock_set(pb, SLAPI_RESULT_CODE, &rc);
result = SLAPI_PLUGIN_FAILURE;
@@ -453,7 +504,7 @@ ipa_topo_pre_del(Slapi_PBlock *pb)
slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, errtxt);
slapi_pblock_set(pb, SLAPI_RESULT_CODE, &rc);
result = SLAPI_PLUGIN_FAILURE;
- } else if (ipa_topo_check_disconnect_reject(pb)) {
+ } else if (ipa_topo_check_topology_disconnect(pb)) {
int rc = LDAP_UNWILLING_TO_PERFORM;
char *errtxt;
errtxt = slapi_ch_smprintf("Removal of Segment disconnects topology."
--
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
