On 06/11/2015 01:11 PM, Ludwig Krispenz wrote:


On 06/11/2015 12:53 PM, Petr Vobornik wrote:
On 06/11/2015 12:35 PM, Ludwig Krispenz wrote:

On 06/11/2015 12:19 PM, Petr Vobornik wrote:
On 06/11/2015 10:22 AM, Martin Babinsky wrote:
On 06/10/2015 03:13 PM, Petr Vobornik wrote:
topology plugin doesn't properly handle:
- creation of segment with direction 'none' and then upgrade to other
   direction
- downgrade of direction

These situations are now forbidden in API.

part of: https://fedorahosted.org/freeipa/ticket/4302


ACK


Looking at Ludwig's path 12, the patch completely forbids mod of
ipaReplTopoSegmentDirection?
that's what I thought we agreed on,

I thought, that we will only complain loudly on downgrade of connection.

so you would have to add a segment
in the opposite direction an they would be merged to both,
but maybe this is a bit strict.

This could work as well, but:

I just tried (without patch 12) to create:
1. A to B, left-right: success
2. B to A, right-left: "Server is unwilling to perform: Segment
already exists in topology or is self referential. Add rejected."
yes, B to  A, right-left is the same as A-B, left right

Sorry, you are right, I wrote it badly. I'm not sure if the servers are broken from testing and previous bugs. Maybe I should reinstalled, but I'm experiencing following weird behavior:

A-B segment, doesn't exist.

1. A to B, left-right: success
2. A to B, right-left: "Server is unwilling to perform: Segment already exists in topology or is self referential. Add rejected."

If I try different direction (started with 4 segments):
1. A to B, right-left: success, 5 segments exist
2. A to B, left-right: success, 4 segments exist - the new ones are gone

Martin, can you reproduce it?


I.e., the upgrade didn't happen.

I could allow for
ipaReplTopoSegmentDirection replace: both
So that upgrade from right-left and left-right to both is not
allowed?  If so then this patch needs to be updated.
depends a bit on what you prefer and what we can get in for alpha.

Depends what's better, I already have adjusted patch for ^^ so it's
not about the work.
so lets take the changes to your patch and we could still extend
functionality a bit for beta or later


OK, attaching rebased patch.
--
Petr Vobornik
From 58284c506035906e3609bbf6f153936e49d2a21a Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Wed, 10 Jun 2015 14:44:09 +0200
Subject: [PATCH] topology: restrict direction changes

topology plugin doesn't properly handle:
- creation of segment with direction 'none' and then upgrade to other
  direction
- downgrade of direction

These situations are now forbidden in API.

part of: https://fedorahosted.org/freeipa/ticket/4302
---
 API.txt                            |  7 +++----
 VERSION                            |  4 ++--
 install/ui/src/freeipa/topology.js | 11 ++---------
 ipalib/plugins/topology.py         |  3 ++-
 4 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/API.txt b/API.txt
index 15356f7ee6330de0822e7582580d2d4f48ad261d..853d26a59bb5bb1ebff698924a36a30b7757c398 100644
--- a/API.txt
+++ b/API.txt
@@ -4757,7 +4757,7 @@ arg: Str('topologysuffixcn', cli_name='topologysuffix', multivalue=False, primar
 arg: Str('cn', attribute=True, cli_name='name', maxlength=255, multivalue=False, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: StrEnum('iparepltoposegmentdirection', attribute=True, cli_name='direction', default=u'both', multivalue=False, required=True, values=(u'both', u'left-right', u'right-left', u'none'))
+option: StrEnum('iparepltoposegmentdirection', attribute=True, cli_name='direction', default=u'both', multivalue=False, required=True, values=(u'both', u'left-right', u'right-left'))
 option: Str('iparepltoposegmentleftnode', attribute=True, cli_name='leftnode', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9.][a-zA-Z0-9.-]{0,252}[a-zA-Z0-9.$-]?$', required=True)
 option: Str('iparepltoposegmentrightnode', attribute=True, cli_name='rightnode', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9.][a-zA-Z0-9.-]{0,252}[a-zA-Z0-9.$-]?$', required=True)
 option: StrEnum('nsds5replicaenabled', attribute=True, cli_name='enabled', multivalue=False, required=False, values=(u'on', u'off'))
@@ -4786,7 +4786,7 @@ arg: Str('topologysuffixcn', cli_name='topologysuffix', multivalue=False, primar
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('cn', attribute=True, autofill=False, cli_name='name', maxlength=255, multivalue=False, primary_key=True, query=True, required=False)
-option: StrEnum('iparepltoposegmentdirection', attribute=True, autofill=False, cli_name='direction', default=u'both', multivalue=False, query=True, required=False, values=(u'both', u'left-right', u'right-left', u'none'))
+option: StrEnum('iparepltoposegmentdirection', attribute=True, autofill=False, cli_name='direction', default=u'both', multivalue=False, query=True, required=False, values=(u'both', u'left-right', u'right-left'))
 option: Str('iparepltoposegmentleftnode', attribute=True, autofill=False, cli_name='leftnode', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9.][a-zA-Z0-9.-]{0,252}[a-zA-Z0-9.$-]?$', query=True, required=False)
 option: Str('iparepltoposegmentrightnode', attribute=True, autofill=False, cli_name='rightnode', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9.][a-zA-Z0-9.-]{0,252}[a-zA-Z0-9.$-]?$', query=True, required=False)
 option: StrEnum('nsds5replicaenabled', attribute=True, autofill=False, cli_name='enabled', multivalue=False, query=True, required=False, values=(u'on', u'off'))
@@ -4804,13 +4804,12 @@ output: ListOfEntries('result', (<type 'list'>, <type 'tuple'>), Gettext('A list
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('truncated', <type 'bool'>, None)
 command: topologysegment_mod
-args: 2,13,3
+args: 2,12,3
 arg: Str('topologysuffixcn', cli_name='topologysuffix', multivalue=False, primary_key=True, query=True, required=True)
 arg: Str('cn', attribute=True, cli_name='name', maxlength=255, multivalue=False, primary_key=True, query=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('delattr*', cli_name='delattr', exclude='webui')
-option: StrEnum('iparepltoposegmentdirection', attribute=True, autofill=False, cli_name='direction', default=u'both', multivalue=False, required=False, values=(u'both', u'left-right', u'right-left', u'none'))
 option: StrEnum('nsds5replicaenabled', attribute=True, autofill=False, cli_name='enabled', multivalue=False, required=False, values=(u'on', u'off'))
 option: Str('nsds5replicastripattrs', attribute=True, autofill=False, cli_name='stripattrs', multivalue=False, required=False)
 option: Str('nsds5replicatedattributelist', attribute=True, autofill=False, cli_name='replattrs', multivalue=False, required=False)
diff --git a/VERSION b/VERSION
index 897c34e72a88867812d5335bc1b1b00260ac5361..a5f40747aa7aa5a19073e077cb3a0607ae42e146 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=130
-# Last change: pvoborni - disallow mod of topologysegment nodes
+IPA_API_VERSION_MINOR=131
+# Last change: pvoborni - toposegment direction restrictions
diff --git a/install/ui/src/freeipa/topology.js b/install/ui/src/freeipa/topology.js
index 20c78cb1653bf9c86c285830ef4c683387decf8b..c8a8acbe9b51dfc3a71e84ed36b812c5de74a39b 100644
--- a/install/ui/src/freeipa/topology.js
+++ b/install/ui/src/freeipa/topology.js
@@ -127,14 +127,7 @@ return {
                             other_field: 'cn',
                             z_index: 1
                         },
-                        {
-                            $type: 'radio',
-                            name: 'iparepltoposegmentdirection',
-                            options: IPA.create_options([
-                                'both', 'left-right', 'right-left', 'none'
-                            ]),
-                            default_value: 'both'
-                        }
+                        'iparepltoposegmentdirection'
                     ]
                 },
                 {
@@ -180,7 +173,7 @@ return {
                 $type: 'radio',
                 name: 'iparepltoposegmentdirection',
                 options: IPA.create_options([
-                    'both', 'left-right', 'right-left', 'none'
+                    'both', 'left-right', 'right-left'
                 ]),
                 default_value: 'both'
             }
diff --git a/ipalib/plugins/topology.py b/ipalib/plugins/topology.py
index 9574b78f658d5cb86b430074ae8a9004114b36ba..be2ac0bd23dd074536cbd098cb90d12b208de499 100644
--- a/ipalib/plugins/topology.py
+++ b/ipalib/plugins/topology.py
@@ -98,10 +98,11 @@ class topologysegment(LDAPObject):
             'iparepltoposegmentdirection',
             cli_name='direction',
             label=_('Connectivity'),
-            values=(u'both', u'left-right', u'right-left', u'none'),
+            values=(u'both', u'left-right', u'right-left'),
             default=u'both',
             doc=_('Direction of replication between left and right replication '
                   'node'),
+            flags={'no_update'},
         ),
         Str(
             'nsds5replicastripattrs?',
-- 
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

Reply via email to