Re: [Freeipa-devel] [PATH 0053] Inconsistency between ipasearchrecordslimit and --sizelimit
Hello,
Updated patch attached.
- Time limit is -1 for unlimited. I found this
https://www.redhat.com/archives/freeipa-devel/2011-January/msg00330.html in
reference to keeping the time limit as -1 for unlimited.
Sure enough, testing time limit at 0 did not work for unlimited as well as
appeared to have negative effects on IPA. Also, I believe that
http://www.python-ldap.org/doc/html/ldap.html#ldap.LDAPObject.search_ext_s
specifies unlimited for time limit as -1. (Please correct me if I am wrong.)
- Size limit is 0 for unlimited per Jan's comment including a conversion
from -1 to 0 if -1 is entered for unlimited size limit.
Actually, 0 means unlimited for size limit, see
http://www.python-ldap.org/doc/html/ldap.html#ldap.LDAPObject.search_ext_s
Thanks,
Gabe
On Tue, Aug 4, 2015 at 3:28 AM, Jan Cholasta jchol...@redhat.com wrote:
Dne 31.7.2015 v 17:08 Gabe Alford napsal(a):
Updated patch attached.
Thanks,
Gabe
On Thu, Jul 30, 2015 at 7:15 AM, Gabe Alford redhatri...@gmail.com
mailto:redhatri...@gmail.com wrote:
On Thu, Jul 30, 2015 at 1:32 AM, Jan Cholasta jchol...@redhat.com
mailto:jchol...@redhat.com wrote:
Dne 30.7.2015 v 09:23 Jan Cholasta napsal(a):
Hi,
Dne 29.7.2015 v 17:23 Gabe Alford napsal(a):
Hello,
Fix for https://fedorahosted.org/freeipa/ticket/4023
Actually, 0 means unlimited for size limit, see
http://www.python-ldap.org/doc/html/ldap.html#ldap.LDAPObject.search_ext_s
.
After reading the ticket I think this should be fixed the other
way around: make 0 mean unlimited for both time and size limit
and fix the config plugin and LDAPClient to respect that.
Thanks for the review. Updated patch attached.
We still need to accept -1 in config-mod for backward compatibility - when
received, it should be converted to 0.
--
Jan Cholasta
From 73a7fd9f2f3fbfa703da68f1a55bb16e4627ffba Mon Sep 17 00:00:00 2001
From: Gabe redhatri...@gmail.com
Date: Thu, 6 Aug 2015 13:18:06 -0600
Subject: [PATCH] Standardize minvalue for ipasearchrecordlimit and
ipasesarchsizelimit for unlimited minvalue
https://fedorahosted.org/freeipa/ticket/4023
---
API.txt | 82 ++---
VERSION | 2 +-
install/ui/test/data/ipa_init_commands.json | 4 +-
install/ui/test/data/ipa_init_objects.json | 4 +-
install/ui/test/data/json_metadata.json | 2 +-
ipalib/plugins/baseldap.py | 6 +--
ipalib/plugins/config.py| 7 ++-
7 files changed, 56 insertions(+), 51 deletions(-)
diff --git a/API.txt b/API.txt
index 2e19d6b2f1e16cc1c89d71ed7d443145426a28e3..19c7857bee7cd7fb63c96a130b53946612f0c74e 100644
--- a/API.txt
+++ b/API.txt
@@ -273,7 +273,7 @@ option: IA5Str('automountinformation', attribute=True, autofill=False, cli_name=
option: IA5Str('automountkey', attribute=True, autofill=False, cli_name='key', multivalue=False, query=True, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Int('sizelimit?', autofill=False, minvalue=0)
-option: Int('timelimit?', autofill=False, minvalue=0)
+option: Int('timelimit?', autofill=False, minvalue=-1)
option: Str('version?', exclude='webui')
output: Output('count', type 'int', None)
output: ListOfEntries('result', (type 'list', type 'tuple'), Gettext('A list of LDAP entries', domain='ipa', localedir=None))
@@ -337,7 +337,7 @@ option: Str('cn', attribute=True, autofill=False, cli_name='location', multivalu
option: Flag('pkey_only?', autofill=True, default=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Int('sizelimit?', autofill=False, minvalue=0)
-option: Int('timelimit?', autofill=False, minvalue=0)
+option: Int('timelimit?', autofill=False, minvalue=-1)
option: Str('version?', exclude='webui')
output: Output('count', type 'int', None)
output: ListOfEntries('result', (type 'list', type 'tuple'), Gettext('A list of LDAP entries', domain='ipa', localedir=None))
@@ -412,7 +412,7 @@ option: Str('description', attribute=True, autofill=False, cli_name='desc', mult
option: Flag('pkey_only?', autofill=True, default=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Int('sizelimit?', autofill=False, minvalue=0)
-option: Int('timelimit?', autofill=False, minvalue=0)
+option: Int('timelimit?', autofill=False, minvalue=-1)
option: Str('version?', exclude='webui')
output: Output('count', type 'int', None)
output: ListOfEntries('result', (type 'list', type 'tuple'), Gettext('A list of LDAP entries', domain='ipa', localedir=None))
@@ -556,7 +556,7 @@ option: Flag('pkey_only?', autofill=True, default=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option:
Re: [Freeipa-devel] Time-Based Account Policies
On 08/03/2015 04:30 PM, Alexander Bokovoy wrote: On Mon, 03 Aug 2015, Stanislav Laznicka wrote: dragons may appear, although with a tiny tiny possibility of a golden treasure in the end. Yes, I think intervals are required. Alright. I gave it a little thought considering the current state of the language for time rules and considering where I should go with these intervals. The thing about the 'interval' in iCalendar is that to use it it is necessary to also add at least the 'frequency' and 'startdate' elements to the language. With that, adding 'enddate' and 'count' optional elements should not be that bad. What I am hoping for is that the same functionality as in iCalendar can be achieved here and so far I do not see why not. Yep. So I have been struggling with this for some time but I hopefully have a solution now. As for the language, I suggest using a new keyword with possible values as in following: repeat=date1(-date2)?\+[1-9][0-9]*(d|w|m|y)(:[1-9][0-9]*)? Here, the date1 and optional date2 are dates in format MMDD and stand for the start and end dates. The number after + denotes the repetition interval and d/w/m/y stand for day/week/month/year periods respectively. I am omitting the SECONDLY, MINUTELY and HOURLY frequencies from iCalendar as I don't consider them appropriate for our purpose. Besides, if repeat is not set, the behavior is similar as with freq=SECONDLY. The number in the optional part with ':' stands for the iCalendar COUNT feature. **However**, I do not think the COUNT feature should be included in this implementation. It probably makes sense as far as the generating of time events (single occurences in time) goes, but in the context of time policies it seems quite confusing to me. Here are some reasons why I think it should not be included: 1. Suppose you want to set, say, something like freq=DAILY;count=100; with timeofday=0730-1600. Such range ends at 9:10 the first day and setting time with any freq+count combination would not therefore make much sense. Yes, omitting the time from COUNT would be possible solution, yet it is a different behavior as in iCalendar. 2. You want to set some really complex time policy that should repeat for certain amount of times. To me, it seems safer to set a certain fixed final date rather than have it automagically decided for you. 3. The code would get messier with COUNT included. Currently, the time policies are evaluated as they are parsed without the need of storing them. With COUNT, they will need to be stored and also, before the actual evaluation, the events would need to be generated which can be hard with more complex policies (is it possible to set an event that would never happen, yet each part describing it would be correct?). 4. Not including COUNT in implementation is not a blocker from importing from the iCalendar format. If anything, COUNT just allows generating a finite set of events. During the import, the last event of such set can be either counted and filled in as end date, or the events can just be imported one by one. Do you consider such a solution passable? -- 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
[Freeipa-devel] [PATCHES 466-468] install: Add common base class for server and replica install
Hi,
the attached patch fixes part of
https://fedorahosted.org/freeipa/ticket/4517.
See also Martin Babinsky's patch 51:
https://www.redhat.com/archives/freeipa-devel/2015-August/msg00012.html.
Honza
--
Jan Cholasta
From f8c2de11794777f406fec377ed404108cdce1655 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 6 Aug 2015 08:14:17 +0200
Subject: [PATCH 1/3] install: Support overriding knobs in subclasses
https://fedorahosted.org/freeipa/ticket/4517
---
ipapython/install/cli.py | 2 -
ipapython/install/core.py | 203 ++
2 files changed, 117 insertions(+), 88 deletions(-)
diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py
index 1ba9a81..5ee445d 100644
--- a/ipapython/install/cli.py
+++ b/ipapython/install/cli.py
@@ -86,8 +86,6 @@ class ConfigureTool(admintool.AdminTool):
transformed_cls = cls._transform(cls.configurable_class)
for owner_cls, name in transformed_cls.knobs():
knob_cls = getattr(owner_cls, name)
-if not knob_cls.initializable:
-continue
if cls.positional_arguments and name in cls.positional_arguments:
continue
diff --git a/ipapython/install/core.py b/ipapython/install/core.py
index c313c27..3af9371 100644
--- a/ipapython/install/core.py
+++ b/ipapython/install/core.py
@@ -6,9 +6,11 @@
The framework core.
-import sys
import abc
+import collections
+import functools
import itertools
+import sys
from ipapython.ipa_log_manager import root_logger
@@ -31,7 +33,8 @@ _missing = object()
_counter = itertools.count()
-def _class_cmp(a, b):
+@functools.cmp_to_key
+def _class_key(a, b):
if a is b:
return 0
elif issubclass(a, b):
@@ -52,27 +55,48 @@ class KnobValueError(ValueError):
self.name = name
-class InnerClass(object):
+class AttributeBase(object):
__metaclass__ = util.InnerClassMeta
+
+# shut up pylint
__outer_class__ = None
__outer_name__ = None
-
-class PropertyBase(InnerClass):
@property
def default(self):
raise AttributeError('default')
def __init__(self, outer):
-self.outer = outer
+pass
def __get__(self, obj, obj_type):
+while obj is not None:
+try:
+return obj.__dict__[self.__outer_name__]
+except KeyError:
+pass
+obj = obj._get_fallback()
+
try:
-return obj._get_property(self.__outer_name__)
-except AttributeError:
-if not hasattr(self, 'default'):
-raise
return self.default
+except AttributeError:
+raise AttributeError(self.__outer_name__)
+
+def __set__(self, obj, value):
+try:
+obj.__dict__[self.__outer_name__] = value
+except KeyError:
+raise AttributeError(self.__outer_name__)
+
+def __delete__(self, obj):
+try:
+del obj.__dict__[self.__outer_name__]
+except KeyError:
+raise AttributeError(self.__outer_name__)
+
+
+class PropertyBase(AttributeBase):
+pass
def Property(default=_missing):
@@ -83,9 +107,8 @@ def Property(default=_missing):
return util.InnerClassMeta('Property', (PropertyBase,), class_dict)
-class KnobBase(PropertyBase):
+class KnobBase(AttributeBase):
type = None
-initializable = True
sensitive = False
deprecated = False
description = None
@@ -96,21 +119,8 @@ class KnobBase(PropertyBase):
_order = None
-def __set__(self, obj, value):
-try:
-self.validate(value)
-except KnobValueError:
-raise
-except ValueError as e:
-raise KnobValueError(self.__outer_name__, str(e))
-
-obj.__dict__[self.__outer_name__] = value
-
-def __delete__(self, obj):
-try:
-del obj.__dict__[self.__outer_name__]
-except KeyError:
-raise AttributeError(self.__outer_name__)
+def __init__(self, outer):
+self.outer = outer
def validate(self, value):
pass
@@ -134,12 +144,17 @@ class KnobBase(PropertyBase):
return cls
-def Knob(type, default=_missing, initializable=_missing, sensitive=_missing,
+def Knob(type_or_base, default=_missing, sensitive=_missing,
deprecated=_missing, description=_missing, cli_name=_missing,
cli_short_name=_missing, cli_aliases=_missing, cli_metavar=_missing):
class_dict = {}
class_dict['_order'] = next(_counter)
-class_dict['type'] = type
+
+if (not isinstance(type_or_base, type) or
+not issubclass(type_or_base, KnobBase)):
+class_dict['type'] = type_or_base
+type_or_base = KnobBase
+
if default is not _missing:
class_dict['default'] = default
if sensitive is not _missing:
@@ -157,7 +172,7 @@ def Knob(type, default=_missing,
Re: [Freeipa-devel] [PATCH 0051] IPA server and replica installers can accept options from config file
On 08/03/2015 01:56 PM, Martin Babinsky wrote: On 07/30/2015 08:55 AM, Jan Cholasta wrote: Dne 29.7.2015 v 17:43 Petr Vobornik napsal(a): On 07/29/2015 05:13 PM, Martin Babinsky wrote: On 07/29/2015 01:25 PM, Jan Cholasta wrote: Dne 29.7.2015 v 12:20 Martin Babinsky napsal(a): Initial attempt to implement https://fedorahosted.org/freeipa/ticket/4517 Some points to discuss: 1.) name of the config entries: currently the option names are derived from CLI options but have underscores in them instead of dashes. Maybe keeping the CLI option names also for config entries will make it easier for the user to transfer their CLI options from scripts to config files. NACK. There is no point in generating config names from CLI names, which are generated from knob names - use knob names directly. The problem is that in some cases the cli_name does not map directly to knob name, leading in different naming of CLI options and config entries, confusion and mayhem. What works for CLI may not work for config files and vice versa. For example, this works for CLI: --no-ntp --no-forwarders --forwarder 1.2.3.4 --forwarder 5.6.7.8 but this works better in config file: ntp = False forwarders = forwarders = 1.2.3.4, 5.6.7.8 These are some offenders from `ipaserver/install/server.py`: http://fpaste.org/249424/18226114/ On the other hand, this can be an incentive to finally put an end to inconsistent option/knob naming across server/replica/etc. installers. Yes please. If the names are different than cli names, then they should be made discoverable somehow or be documented. IMHO documenting them is easy. 2.) Config sections: there is currently only one valid section named '[global]' in accordance with the format of 'default.conf'. Should we have separate sections equivalent to option groups in CLI (e.g. [basic], [certificate system], [dns])? No, because they would have to be maintained forever. For example, some options are in wrong sections and we wouldn't be able to move them. I'm also more inclined to a single section, at least for now since we are pressed for time with this RFE. That's not to say that we should ditch Alexander's idea about separate sections with overrides for different hosts. We should consider it as a future enhancement to this feature once the basic plumbing is in place. Right. 3.) Handling of unattended mode when specifying a config file: Currently there is no connection between --config-file and unattended mode. So when you run ipa-server-install using config file, you still get asked for missing stuff. Should '--config-file' automatically imply '--unattended'? The behavior should be the same as if you specified the options on the command line. So no, --config-file should not imply --unattended. That sound reasonable. the code behaves this way already so no changes here. There are probably other issues to discuss. Feel free to write email/ping me on IRC. (I haven't looked at the patch yet.) Please take a look at it ASAP. I am on PTO tomorrow and on Friday, but I will find time to work at it in the evening if you send me you comments. 1) IMO the option should be in the top-level option section, not in a separate group (use parser.add_option()). Also maybe rename it to --config, AFAIK that's what is usually used. A short name (-c?) would be nice too. Nitpick: if the option is named --config-file, dest should be config_file, to make it easier to look it up in the code. 2) Please don't duplicate the knob retrieval code, store knobs in a list and pass that as an argument to parse_config_file. 3) I'm not sure about using newline as a list separator. I don't know about other IPA components, but SSSD in particular uses commas, maybe we should be consistent with that? 4) Booleans should be assignable either True or False, i.e. do not use _parse_knob to parse them. Honza Attaching updated patch. Attaching updated patch. I have also sneaked in a small change that improves the testability of installers (making configurator a member of ConfigureTool instance). Should I send a separate patch for that? -- Martin^3 Babinsky From c73cae8dd0a189a2384f010b81273d88fc450f8d Mon Sep 17 00:00:00 2001 From: Martin Babinsky mbabi...@redhat.com Date: Wed, 22 Jul 2015 13:55:26 +0200 Subject: [PATCH 1/2] IPA server and replica installers can accept options from config file New option '-c'/'--config' enables ipa-server-install and ipa-replica-install to obtain parameters from supplied configuration file in INI format. The file syntax is as follows: * all options are listed in a single [global] section * the option name can be derived from long CLI option name by replacing dashes with underscores * to specify a multivalued parameter, assign a list of comma separated values to a single option. Escape commas in values like DNs by a backslash. Parameters specified explicitly through CLI options
[Freeipa-devel] [PATCH 0053] fix crash when installer with no positional arguments handles invalid options
This bug was discovered when writing tests for functionality introduced
in my PATCH 0051.
This patch should apply on top of PATCH 0051.
--
Martin^3 Babinsky
From 7b281ba47e4fec7da7eab4a861a7cbaceb2bd859 Mon Sep 17 00:00:00 2001
From: Martin Babinsky mbabi...@redhat.com
Date: Thu, 6 Aug 2015 14:19:52 +0200
Subject: [PATCH] fix crash when installer with no positional arguments handles
invalid options
when ipa-server-install received an option value which did not pass knob
validator an Attribute error was raised due to incorrect error handling with
no positional arguments.
---
ipapython/install/cli.py | 51 +---
1 file changed, 31 insertions(+), 20 deletions(-)
diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py
index 98297aa3d3361dbaa08529340e69e034fb04a4f8..242eb40550e92d41cc6ad31bffbd84bb3d332000 100644
--- a/ipapython/install/cli.py
+++ b/ipapython/install/cli.py
@@ -31,48 +31,59 @@ def install_tool(configurable_class, command_name, log_file_name,
configurable_class=configurable_class,
command_name=command_name,
log_file_name=uninstall_log_file_name,
-positional_arguments=uninstall_positional_arguments,
usage=uninstall_usage,
debug_option=debug_option,
)
+if uninstall_positional_arguments is not None:
+uninstall_kwargs.update(
+dict(positional_arguments=uninstall_positional_arguments)
+)
else:
uninstall_kwargs = None
+kwargs = dict(
+configurable_class=configurable_class,
+command_name=command_name,
+log_file_name=log_file_name,
+usage=usage,
+debug_option=debug_option,
+uninstall_kwargs=uninstall_kwargs,
+)
+
+if positional_arguments is not None:
+kwargs.update(dict(positional_arguments=positional_arguments))
+
return type(
'install_tool({0})'.format(configurable_class.__name__),
(InstallTool,),
-dict(
-configurable_class=configurable_class,
-command_name=command_name,
-log_file_name=log_file_name,
-positional_arguments=positional_arguments,
-usage=usage,
-debug_option=debug_option,
-uninstall_kwargs=uninstall_kwargs,
-)
+kwargs
)
def uninstall_tool(configurable_class, command_name, log_file_name,
positional_arguments=None, usage=None, debug_option=False):
+kwargs = dict(
+configurable_class=configurable_class,
+command_name=command_name,
+log_file_name=log_file_name,
+usage=usage,
+debug_option=debug_option,
+)
+
+if positional_arguments is not None:
+kwargs.update(dict(positional_arguments=positional_arguments))
+
return type(
'uninstall_tool({0})'.format(configurable_class.__name__),
(UninstallTool,),
-dict(
-configurable_class=configurable_class,
-command_name=command_name,
-log_file_name=log_file_name,
-positional_arguments=positional_arguments,
-usage=usage,
-debug_option=debug_option,
-)
+kwargs
)
class ConfigureTool(admintool.AdminTool):
configurable_class = None
debug_option = False
-positional_arguments = None
+positional_arguments = ()
cfgr = None
@staticmethod
@@ -311,7 +322,7 @@ class ConfigureTool(admintool.AdminTool):
knob_cls = getattr(transformed_cls, e.name)
try:
index = self.positional_arguments.index(e.name)
-except IndexError:
+except ValueError:
cli_name = knob_cls.cli_name or e.name.replace('_', '-')
desc = option --{0}.format(cli_name)
else:
--
2.4.3
--
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
Re: [Freeipa-devel] [PATCH 0053] fix crash when installer with no positional arguments handles invalid options
Hi,
Dne 6.8.2015 v 14:29 Martin Babinsky napsal(a):
This bug was discovered when writing tests for functionality introduced
in my PATCH 0051.
This patch should apply on top of PATCH 0051.
This whole patch can be reduced to:
except core.KnobValueError as e:
knob_cls = getattr(transformed_cls, e.name)
try:
-index = self.positional_arguments.index(e.name)
-except IndexError:
+if self.positional_arguments:
+index = self.positional_arguments.index(e.name)
+else:
+raise ValueError
+except ValueError:
cli_name = knob_cls.cli_name or e.name.replace('_', '-')
desc = option --{0}.format(cli_name)
else:
Honza
--
Jan Cholasta
--
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
Re: [Freeipa-devel] [PATCHES] changes in preparation of replica promotion work
On Thu, 2015-08-06 at 07:21 +0200, Jan Cholasta wrote: Dne 5.8.2015 v 17:24 Simo Sorce napsal(a): On Wed, 2015-08-05 at 08:20 +0200, Jan Cholasta wrote: Hi, Dne 31.7.2015 v 12:46 Simo Sorce napsal(a): I've been carrying these patches in my tree for a while, I think it is time to put them in master as they stand on their own. Simo. Patch 530: ACK Patch 531: ACK Patch 532: The methods should be static methods: @staticmethod def setOption(name, value): ... Care to explain why ? @staticmethod is not used anywhere else in that file. Because the methods do not use any instance or class state. They will of course work fine even if they are normal methods, but making them static methods is cleaner. Ok, I embedded the change in my tree. I am working on some fixes to the replica promotion patchset with Ludwig, so I will respin all of the patches at once later on. Simo. -- Simo Sorce * Red Hat, Inc * New York -- 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
Re: [Freeipa-devel] [PATCH] 0194 Fix selector of protocol for LSA RPC binding string
- Original Message - On 08/05/2015 08:40 PM, Alexander Bokovoy wrote: Hi, attached patch fixes a bug https://bugzilla.redhat.com/show_bug.cgi?id=1249455 details are in the commit message. Looks good to me, generates bindings strings as described in the BZ. Just a readability nitpick, can we get rid of the binding_template lambda abstraction and use something like this? binding_template=u'%s:%s[%s]' return [binding_template % (t, remote_host, o) for t in transports for o in options] or just plain: return [u'%s:%s[%s]' % (t, remote_host, o) for t in transports for o in options] instead of: binding_template=lambda x,y,z: u'%s:%s[%s]' % (x, y, z) return [binding_template(t, remote_host, o) for t in transports for o in options] Up to you. :) -- / Alexander Bokovoy -- 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
Re: [Freeipa-devel] [PATCH 0053] fix crash when installer with no positional arguments handles invalid options
On 08/06/2015 02:35 PM, Jan Cholasta wrote:
Hi,
Dne 6.8.2015 v 14:29 Martin Babinsky napsal(a):
This bug was discovered when writing tests for functionality introduced
in my PATCH 0051.
This patch should apply on top of PATCH 0051.
This whole patch can be reduced to:
except core.KnobValueError as e:
knob_cls = getattr(transformed_cls, e.name)
try:
-index = self.positional_arguments.index(e.name)
-except IndexError:
+if self.positional_arguments:
+index = self.positional_arguments.index(e.name)
+else:
+raise ValueError
+except ValueError:
cli_name = knob_cls.cli_name or e.name.replace('_', '-')
desc = option --{0}.format(cli_name)
else:
Honza
Right. I have rewritten the patch in your way.
--
Martin^3 Babinsky
From f854921ca3bc4ef73a8a4c5565036911041ce594 Mon Sep 17 00:00:00 2001
From: Martin Babinsky mbabi...@redhat.com
Date: Thu, 6 Aug 2015 14:43:32 +0200
Subject: [PATCH] fix crash when installer with no positional arguments handles
invalid options
when ipa-server-install received an option value which did not pass knob
validator an AttributeError was raised due to incorrect error handling with
no positional arguments.
---
ipapython/install/cli.py | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py
index 98297aa3d3361dbaa08529340e69e034fb04a4f8..4a0fbdbd82fdbef78dbe96c008e2c90a9d839833 100644
--- a/ipapython/install/cli.py
+++ b/ipapython/install/cli.py
@@ -310,8 +310,11 @@ class ConfigureTool(admintool.AdminTool):
except core.KnobValueError as e:
knob_cls = getattr(transformed_cls, e.name)
try:
-index = self.positional_arguments.index(e.name)
-except IndexError:
+if self.positional_arguments:
+index = self.positional_arguments.index(e.name)
+else:
+raise ValueError
+except ValueError:
cli_name = knob_cls.cli_name or e.name.replace('_', '-')
desc = option --{0}.format(cli_name)
else:
--
2.4.3
--
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
[Freeipa-devel] [PATCH 0054] test suite for functionality implemented #4517
I have slapped together a small test suite for the functionality
implemented in PATCH 0051. Obviously it requires this patch to work.
It also requires PATCH 0052 to pass all tests.
https://fedorahosted.org/freeipa/ticket/4517
--
Martin^3 Babinsky
From d09a4f1134a0e0535b1cab6989b41929f793d33c Mon Sep 17 00:00:00 2001
From: Martin Babinsky mbabi...@redhat.com
Date: Wed, 22 Jul 2015 13:54:44 +0200
Subject: [PATCH] test suite for configuration of installers based on
ConfigureTool
Tests for new functionality introduced in
https://fedorahosted.org/freeipa/ticket/4517
---
ipatests/test_install/test_installer_config.py | 440 +
1 file changed, 440 insertions(+)
create mode 100644 ipatests/test_install/test_installer_config.py
diff --git a/ipatests/test_install/test_installer_config.py b/ipatests/test_install/test_installer_config.py
new file mode 100644
index ..48c43d06161e9b1612c718be6833f9a24030e2b5
--- /dev/null
+++ b/ipatests/test_install/test_installer_config.py
@@ -0,0 +1,440 @@
+#
+# Copyright (C) 2015 FreeIPA Contributors see COPYING for license
+#
+
+
+tests for passing options to new-style installer from CLI and config file
+
+
+from ConfigParser import RawConfigParser
+from csv import writer, QUOTE_NONE
+from cStringIO import StringIO
+import os
+
+import pytest
+
+from ipapython.install import cli, common, core
+
+class MockInstaller(common.Installable, common.Interactive, core.Composite):
+
+bool_opt = core.Knob(
+bool,
+False,
+cli_short_name='b',
+description='boolean option'
+)
+
+str_opt = core.Knob(
+str,
+'string',
+cli_short_name='s',
+description='string option'
+)
+
+int_opt = core.Knob(
+int,
+1,
+cli_short_name='i',
+description='integer option'
+)
+
+list_ints_opt = core.Knob(
+(list, int),
+None,
+cli_short_name='l',
+description='option storing list of integers'
+)
+list_strings_opt = core.Knob(
+(list, str),
+None,
+cli_short_name='t',
+description='option storing list of strings'
+)
+
+validated_int_opt = core.Knob(
+int,
+0,
+cli_short_name='o',
+description='option with custom validator'
+)
+
+@validated_int_opt.validator
+def validated_int_opt(self, value):
+if value 0 or value 10:
+raise ValueError('invalid value {0}'.format(value))
+
+no_default_opt = core.Knob(
+int,
+cli_short_name='n',
+description='option with no default'
+)
+
+@pytest.mark.usefixtures('configs')
+@pytest.mark.skipif(os.geteuid() != 0,
+reason=Must be root to run installer tests)
+class TestInstallerConfiguration(object):
+valid_section = 'global'
+invalid_section = 'glebl'
+
+configs = {
+'simple.txt': {
+valid_section: dict(
+bool_opt=False,
+int_opt=3,
+str_opt='test_string',
+no_default_opt=1
+)
+},
+'multivalue_int.txt': {
+valid_section: dict(
+bool_opt=False,
+int_opt=5,
+list_ints_opt=[1, 2, 3],
+no_default_opt=1
+)
+},
+'multivalue_string.txt': {
+valid_section: dict(
+bool_opt=True,
+int_opt=4,
+list_strings_opt=['value1', 'value2', 'value3'],
+no_default_opt=1
+)
+},
+'multivalue_string_with_comma.txt': {
+valid_section: dict(
+bool_opt=False,
+list_strings_opt=['value1', 'value,2', 'value3'],
+no_default_opt=1
+)
+},
+'multiline_string.txt': {
+valid_section: dict(
+bool_opt=True,
+str_opt='value1\nvalue2',
+no_default_opt=1
+)
+},
+'validated_int.txt': {
+valid_section: dict(
+bool_opt=True,
+validated_int_opt=4,
+no_default_opt=1
+)
+},
+'invalid_bool.txt': {
+valid_section: dict(
+bool_opt='trve',
+no_default_opt=1
+)
+},
+'invalid_int.txt': {
+valid_section: dict(
+bool_opt=True,
+int_opt='fwefghhe',
+no_default_opt=1
+)
+},
+'invalid_list_ints.txt': {
+valid_section: dict(
+int_opt=4,
+list_ints_opt=[1, 'two', 3],
+no_default_opt=1
+)
+},
+'invalid_validated.txt': {
+valid_section: dict(
+bool_opt=True,
+
[Freeipa-devel] [PATCH 0357] trusts: Detect domain clash with IPA domain when adding a AD
Hi,
When IPA is deployed in the same domain as AD, trust-add fails since
the names of the local domain and trusted domain ranges is the same
- it's always DOMAIN.NAME_id_range.
When adding a trusted domain, we look for previous ranges for
this domain (which may have been left behind by previous trust
attempts). Since AD and IPA are in the same domain, we find
a local domain range, which does not have a SID.
Detect such domain collisions early and bail out with an appropriate
error message.
https://fedorahosted.org/freeipa/ticket/4549
From ea8b725d5bc4c31a03dc998ef85d91d463542b8c Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Thu, 6 Aug 2015 10:54:47 +0200
Subject: [PATCH] trusts: Detect domain clash with IPA domain when adding a AD
trust
When IPA is deployed in the same domain as AD, trust-add fails since
the names of the local domain and trusted domain ranges is the same
- it's always DOMAIN.NAME_id_range.
When adding a trusted domain, we look for previous ranges for
this domain (which may have been left behind by previous trust
attempts). Since AD and IPA are in the same domain, we find
a local domain range, which does not have a SID.
Detect such domain collisions early and bail out with an appropriate
error message.
https://fedorahosted.org/freeipa/ticket/4549
---
ipalib/plugins/trust.py | 8
1 file changed, 8 insertions(+)
diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index ba80eefe4735a8800cc530e60b4435c3d8cdcf4d..b64a550216ea534ce58d6c825484ebe837671462 100644
--- a/ipalib/plugins/trust.py
+++ b/ipalib/plugins/trust.py
@@ -730,6 +730,14 @@ sides.
error=_('only ad is supported')
)
+# Detect IPA-AD domain clash
+if self.api.env.domain.lower() == trusted_realm_domain.lower():
+raise errors.ValidationError(
+name=_('domain'),
+error=_('Cannot establish a trust to AD deployed in the same '
+'domain as IPA. Such setup is not supported.')
+)
+
# If domain name and realm does not match, IPA server is not be able
# to establish trust with Active Directory.
--
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
[Freeipa-devel] [PATCH 0356] trusts: Detect missing Samba instance
Hi,
In the event of invocation of trust related commands, IPA server needs
to contact local Samba instance. This is not possible on servers that
merely act as AD trust agents, since they do not have Samba instance
running.
Properly detect the absence of the Samba instance and output
user-friendly
message which includes list of servers that are capable of running
the command, if such exist.
List of commands affected:
* ipa trust-add
* ipa trust-fetch-domains
* all of the trustdomain commands available via CLI
https://fedorahosted.org/freeipa/ticket/5165
From 128ee05bbebe17f77272b8f2a6bd5039cfbc26b0 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Thu, 6 Aug 2015 10:10:04 +0200
Subject: [PATCH] trusts: Detect missing Samba instance
In the event of invocation of trust related commands, IPA server needs to
contact local Samba instance. This is not possible on servers that
merely act as AD trust agents, since they do not have Samba instance
running.
Properly detect the absence of the Samba instance and output
user-friendly
message which includes list of servers that are capable of running
the command, if such exist.
List of commands affected:
* ipa trust-add
* ipa trust-fetch-domains
* all of the trustdomain commands available via CLI
https://fedorahosted.org/freeipa/ticket/5165
---
ipalib/plugins/trust.py | 99 +++--
1 file changed, 79 insertions(+), 20 deletions(-)
diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index 940e06a5ffa387c6cc18983d7b56f089f58a236e..ba80eefe4735a8800cc530e60b4435c3d8cdcf4d 100644
--- a/ipalib/plugins/trust.py
+++ b/ipalib/plugins/trust.py
@@ -199,6 +199,73 @@ def make_trust_dn(env, trust_type, dn):
return DN(dn, container_dn)
return dn
+def find_adtrust_masters(ldap, api):
+
+Returns a list of names of IPA servers with ADTRUST component configured.
+
+
+try:
+entries, truncated = ldap.find_entries(
+cn=ADTRUST,
+base_dn=api.env.container_masters + api.env.basedn
+)
+except errors.NotFound:
+entries = []
+
+return [entry.dn[1].value for entry in entries]
+
+def verify_samba_component_presence(ldap, api):
+
+Verifies that Samba is installed and configured on this particular master.
+If Samba is not available, provide a heplful hint with the list of masters
+capable of running the commands.
+
+
+adtrust_present = api.Command['adtrust_is_enabled']()['result']
+
+hint = _(
+' Alternatively, following servers are capable of running this '
+'command: %(masters)s'
+)
+
+def raise_missing_component_error(message):
+masters_with_adtrust = find_adtrust_masters(ldap, api)
+
+# If there are any masters capable of running Samba requiring commands
+# let's advertise them directly
+if masters_with_adtrust:
+message += hint % dict(masters=', '.join(masters_with_adtrust))
+
+raise errors.NotFound(
+name=_('AD Trust setup'),
+reason=message,
+)
+
+# We're ok in this case, bail out
+if adtrust_present and _bindings_installed:
+return
+
+# First check for packages missing
+elif not _bindings_installed:
+error_message=_(
+'Cannot perform the selected command without Samba 4 support '
+'installed. Make sure you have installed server-trust-ad '
+'sub-package of IPA.'
+)
+
+raise_missing_component_error(error_message)
+
+# Packages present, but ADTRUST instance is not configured
+elif not adtrust_present:
+error_message=_(
+'Cannot perform the selected command without Samba 4 instance '
+'configured on this machine. Make sure you have run '
+'ipa-adtrust-install on this server.'
+)
+
+raise_missing_component_error(error_message)
+
+
def generate_creds(trustinstance, style, **options):
Generate string representing credentials using trust instance
@@ -554,6 +621,10 @@ sides.
has_output_params = LDAPCreate.has_output_params + trust_output_params
def execute(self, *keys, **options):
+ldap = self.obj.backend
+
+verify_samba_component_presence(ldap, self.api)
+
full_join = self.validate_options(*keys, **options)
old_range, range_name, dom_sid = self.validate_range(*keys, **options)
result = self.execute_ad(full_join, *keys, **options)
@@ -569,7 +640,6 @@ sides.
created_range_type = old_range['result']['iparangetype'][0]
trust_filter = cn=%s % result['value']
-ldap = self.obj.backend
(trusts, truncated) = ldap.find_entries(
base_dn=DN(self.api.env.container_trusts, self.api.env.basedn),
filter=trust_filter)
@@ -642,16 +712,6 @@ sides.
def validate_options(self, *keys, **options):
Re: [Freeipa-devel] [PATCH] 0194 Fix selector of protocol for LSA RPC binding string
On 08/05/2015 08:40 PM, Alexander Bokovoy wrote: Hi, attached patch fixes a bug https://bugzilla.redhat.com/show_bug.cgi?id=1249455 details are in the commit message. Looks good to me, generates bindings strings as described in the BZ. Just a readability nitpick, can we get rid of the binding_template lambda abstraction and use something like this? binding_template=u'%s:%s[%s]' return [binding_template % (t, remote_host, o) for t in transports for o in options] or just plain: return [u'%s:%s[%s]' % (t, remote_host, o) for t in transports for o in options] instead of: binding_template=lambda x,y,z: u'%s:%s[%s]' % (x, y, z) return [binding_template(t, remote_host, o) for t in transports for o in options] Tomas -- 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
[Freeipa-devel] Unable to install bits from ipa-4-2 branch
Hi list, I just noticed that the bits built from ipa-4-2 branch cannot be installed. The freeipa packages built have version such as freeipa-server-dns-4.2.0-0.20150806083844Zjenkins9git2812242.fc22.x86_64 The version check in the spec file makes the server-dns package obsolete the server package from tha same build. The cause is the commit [1]. This issue blocks us from running tests on ipa-4-2 branch. Should we bump the minor version on this branch to 4.2.1? [1]: https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=f555fe95dba9ec453fa10f160089dcc5404f724a Cheers, Milan -- 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
