Re: [Freeipa-devel] [PATCH 0405] idviews: Add user certificate attribute to user ID overrides

[Tomas Babej]

On 04/19/2016 08:20 AM, Jan Cholasta wrote:
> On 13.4.2016 14:13, Tomas Babej wrote:
>> On 04/13/2016 09:55 AM, Tomas Babej wrote:
>>> On 04/07/2016 01:53 PM, Sumit Bose wrote:
>>>> On Mon, Apr 04, 2016 at 04:27:02PM +0200, Jan Cholasta wrote:
>>>>> Hi,
>>>>>
>>>>> On 1.4.2016 16:53, Tomas Babej wrote:
>>>>>> Hi,
>>>>>>
>>>>>> this extends the user ID overrides with capability to store the user
>>>>>> certificate.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/4955
>>>>>
>>>>> The preferred way of managing certificates nowadays is using
>>>>> $OBJ-add-cert
>>>>> and $OBJ-remove-cert commands, you should add them here as well.
>>>>>
>>>>> I would even go as far as not allowing to modify certificates using
>>>>> idoverrideuser-mod - in user-mod and host-mod, it's there just for
>>>>> backward
>>>>> compatibility, which is not the case here. But I don't have a
>>>>> strong opinion
>>>>> on that.
>>>>>
>>>>> For consistency with user-find and host-find, the full certificate
>>>>> blob
>>>>> should not be shown in idoverrideuser-find. You can do that by setting
>>>>> search_display_attributes attribute on the idoverrideuser class
>>>>> appropriately.
>>>>
>>>> I tested the current patch with my related patches for SSSD and all is
>>>> working as expected.
>>>>
>>>> bye,
>>>> Sumit
>>>>
>>>>>
>>>>> 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
>>>>
>>>
>>> Thanks for the reviews,
>>>
>>> attaching a updated patch that addresses Honza's comments.
>>>
>>> Tomas
>>>
>>
>> Sending an improved version addressing a couple of additional issues.
> 
> 1) This bit in idoverrideuser_add.pre_callback() is redundant, as the
> certificate will always be DER here already:
> 
> # Normalize the certificate to DER format
> certs = options.get('usercertificate', [])
> certs_der = [x509.normalize_certificate(c) for c in certs]
> entry_attrs['usercertificate'] = certs_der
> 
> 
> 2) You need to call convert_usercertificate_pre() in
> idoverrideuser_mod.pre_callback() and convert_usercertificate_post() in
> idoverrideuser_{mod,find,show}.post_callback() as well.
> 
> Honza
> 

Updated patch attached, mentioned issues should be fixed, I also removed
one redundant import which escaped my careful eye.

Tomas
From ecfb6dbfb39120fa1c2caf83fd0d6c22471c212d Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 3 Mar 2016 15:14:10 +0100
Subject: [PATCH] idviews: Add user certificate attribute to user ID overrides

---
 ACI.txt  |  2 +-
 API.txt  | 30 +++--
 VERSION  |  4 +--
 install/share/71idviews.ldif |  2 +-
 ipalib/plugins/idviews.py| 79 ++--
 5 files changed, 109 insertions(+), 8 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index 24cb332ce6e10c82a5bfab76d084fb6c0277800d..ae00cf7a1b8e2ea0e33798993bb24dc5f06127e3 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -149,7 +149,7 @@ aci: (targetfilter = "(objectclass=ipahostgroup)")(version 3.0;acl "permission:S
 dn: cn=views,cn=accounts,dc=ipa,dc=example
 aci: (targetattr = "cn || createtimestamp || description || entryusn || gidnumber || ipaanchoruuid || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipaGroupOverride)")(version 3.0;acl "permission:System: Read Group ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=views,cn=accounts,dc=ipa,dc=example
-aci: (targetattr = "createtimestamp || description || entryusn || gecos || gidnumber || homedirectory || ipaanchoruuid || ipaoriginaluid || ipasshpubkey || loginshell || modifytimestamp || objectclass || uid || uidnumber")(targetfilter = "(objectclass=ipaUserOverride)")(version 3.0;acl "permission:System: Read User ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
+aci: (targetattr = "createtimestamp || description || entr

Re: [Freeipa-devel] [PATCH 0405] idviews: Add user certificate attribute to user ID overrides

[Tomas Babej]

On 04/13/2016 09:55 AM, Tomas Babej wrote:
> On 04/07/2016 01:53 PM, Sumit Bose wrote:
>> On Mon, Apr 04, 2016 at 04:27:02PM +0200, Jan Cholasta wrote:
>>> Hi,
>>>
>>> On 1.4.2016 16:53, Tomas Babej wrote:
>>>> Hi,
>>>>
>>>> this extends the user ID overrides with capability to store the user
>>>> certificate.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/4955
>>>
>>> The preferred way of managing certificates nowadays is using $OBJ-add-cert
>>> and $OBJ-remove-cert commands, you should add them here as well.
>>>
>>> I would even go as far as not allowing to modify certificates using
>>> idoverrideuser-mod - in user-mod and host-mod, it's there just for backward
>>> compatibility, which is not the case here. But I don't have a strong opinion
>>> on that.
>>>
>>> For consistency with user-find and host-find, the full certificate blob
>>> should not be shown in idoverrideuser-find. You can do that by setting
>>> search_display_attributes attribute on the idoverrideuser class
>>> appropriately.
>>
>> I tested the current patch with my related patches for SSSD and all is
>> working as expected.
>>
>> bye,
>> Sumit
>>
>>>
>>> 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
>>
> 
> Thanks for the reviews,
> 
> attaching a updated patch that addresses Honza's comments.
> 
> Tomas
> 

Sending an improved version addressing a couple of additional issues.

Tomas
From f56129024fecfe1522cd6bd85f7daddfd3bf5129 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 3 Mar 2016 15:14:10 +0100
Subject: [PATCH] idviews: Add user certificate attribute to user ID overrides

---
 ACI.txt  |  2 +-
 API.txt  | 30 ++--
 install/share/71idviews.ldif |  2 +-
 ipalib/plugins/idviews.py| 82 ++--
 4 files changed, 110 insertions(+), 6 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index 24cb332ce6e10c82a5bfab76d084fb6c0277800d..ae00cf7a1b8e2ea0e33798993bb24dc5f06127e3 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -149,7 +149,7 @@ aci: (targetfilter = "(objectclass=ipahostgroup)")(version 3.0;acl "permission:S
 dn: cn=views,cn=accounts,dc=ipa,dc=example
 aci: (targetattr = "cn || createtimestamp || description || entryusn || gidnumber || ipaanchoruuid || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipaGroupOverride)")(version 3.0;acl "permission:System: Read Group ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=views,cn=accounts,dc=ipa,dc=example
-aci: (targetattr = "createtimestamp || description || entryusn || gecos || gidnumber || homedirectory || ipaanchoruuid || ipaoriginaluid || ipasshpubkey || loginshell || modifytimestamp || objectclass || uid || uidnumber")(targetfilter = "(objectclass=ipaUserOverride)")(version 3.0;acl "permission:System: Read User ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
+aci: (targetattr = "createtimestamp || description || entryusn || gecos || gidnumber || homedirectory || ipaanchoruuid || ipaoriginaluid || ipasshpubkey || loginshell || modifytimestamp || objectclass || uid || uidnumber || usercertificate")(targetfilter = "(objectclass=ipaUserOverride)")(version 3.0;acl "permission:System: Read User ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=ranges,cn=etc,dc=ipa,dc=example
 aci: (targetattr = "cn || createtimestamp || entryusn || ipabaseid || ipabaserid || ipaidrangesize || ipanttrusteddomainsid || iparangetype || ipasecondarybaserid || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipaidrange)")(version 3.0;acl "permission:System: Read ID Ranges";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=views,cn=accounts,dc=ipa,dc=example
diff --git a/API.txt b/API.txt
index 5b75413f930d0e9caaffc68023bed8106d786653..76b260da72533ee88027f72d56a591c7566c72b7 100644
--- a/API.txt
+++ b/API.txt
@@ -2429,7 +2429,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
 command: idoverrideuser_add
-args: 2,15,3
+args: 2,16,3
 arg: Str('idviewcn', cli_name='idview', multivalue=False, primary_key=True, query=True, required=True)
 arg: Str('ipaanchoruuid', attr

Re: [Freeipa-devel] [PATCH 0405] idviews: Add user certificate attribute to user ID overrides

[Tomas Babej]

On 04/07/2016 01:53 PM, Sumit Bose wrote:
> On Mon, Apr 04, 2016 at 04:27:02PM +0200, Jan Cholasta wrote:
>> Hi,
>>
>> On 1.4.2016 16:53, Tomas Babej wrote:
>>> Hi,
>>>
>>> this extends the user ID overrides with capability to store the user
>>> certificate.
>>>
>>> https://fedorahosted.org/freeipa/ticket/4955
>>
>> The preferred way of managing certificates nowadays is using $OBJ-add-cert
>> and $OBJ-remove-cert commands, you should add them here as well.
>>
>> I would even go as far as not allowing to modify certificates using
>> idoverrideuser-mod - in user-mod and host-mod, it's there just for backward
>> compatibility, which is not the case here. But I don't have a strong opinion
>> on that.
>>
>> For consistency with user-find and host-find, the full certificate blob
>> should not be shown in idoverrideuser-find. You can do that by setting
>> search_display_attributes attribute on the idoverrideuser class
>> appropriately.
> 
> I tested the current patch with my related patches for SSSD and all is
> working as expected.
> 
> bye,
> Sumit
> 
>>
>> 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
> 

Thanks for the reviews,

attaching a updated patch that addresses Honza's comments.

Tomas
From bc7a20b942931e43b4d7e4e79b88cae8a113385d Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 3 Mar 2016 15:14:10 +0100
Subject: [PATCH] idviews: Add user certificate attribute to user ID overrides

---
 ACI.txt  |  2 +-
 API.txt  | 30 +++--
 install/share/71idviews.ldif |  2 +-
 ipalib/plugins/idviews.py| 80 ++--
 4 files changed, 108 insertions(+), 6 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index 24cb332ce6e10c82a5bfab76d084fb6c0277800d..ae00cf7a1b8e2ea0e33798993bb24dc5f06127e3 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -149,7 +149,7 @@ aci: (targetfilter = "(objectclass=ipahostgroup)")(version 3.0;acl "permission:S
 dn: cn=views,cn=accounts,dc=ipa,dc=example
 aci: (targetattr = "cn || createtimestamp || description || entryusn || gidnumber || ipaanchoruuid || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipaGroupOverride)")(version 3.0;acl "permission:System: Read Group ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=views,cn=accounts,dc=ipa,dc=example
-aci: (targetattr = "createtimestamp || description || entryusn || gecos || gidnumber || homedirectory || ipaanchoruuid || ipaoriginaluid || ipasshpubkey || loginshell || modifytimestamp || objectclass || uid || uidnumber")(targetfilter = "(objectclass=ipaUserOverride)")(version 3.0;acl "permission:System: Read User ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
+aci: (targetattr = "createtimestamp || description || entryusn || gecos || gidnumber || homedirectory || ipaanchoruuid || ipaoriginaluid || ipasshpubkey || loginshell || modifytimestamp || objectclass || uid || uidnumber || usercertificate")(targetfilter = "(objectclass=ipaUserOverride)")(version 3.0;acl "permission:System: Read User ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=ranges,cn=etc,dc=ipa,dc=example
 aci: (targetattr = "cn || createtimestamp || entryusn || ipabaseid || ipabaserid || ipaidrangesize || ipanttrusteddomainsid || iparangetype || ipasecondarybaserid || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipaidrange)")(version 3.0;acl "permission:System: Read ID Ranges";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=views,cn=accounts,dc=ipa,dc=example
diff --git a/API.txt b/API.txt
index 5b75413f930d0e9caaffc68023bed8106d786653..76b260da72533ee88027f72d56a591c7566c72b7 100644
--- a/API.txt
+++ b/API.txt
@@ -2429,7 +2429,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
 command: idoverrideuser_add
-args: 2,15,3
+args: 2,16,3
 arg: Str('idviewcn', cli_name='idview', multivalue=False, primary_key=True, query=True, required=True)
 arg: Str('ipaanchoruuid', attribute=True, cli_name='anchor', multivalue=False, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
@@ -2446,6 +2446,19 @@ option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('uid',

[Freeipa-devel] [PATCH 0406] admintool: Remove the option to override the log file

[Tomas Babej]

Hi,

This option has been rarely used, and can be replaced by proper shell
output redirection.

https://fedorahosted.org/freeipa/ticket/5385

Tomas
From ee3b3d295e696488bef9abd16eb3108255afd0b0 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Tue, 10 Nov 2015 14:20:45 +0100
Subject: [PATCH] admintool: Remove the option to override the log file

This has been rarely used, and can be replaced by proper shell output
redirection.

https://fedorahosted.org/freeipa/ticket/5385
---
 install/tools/man/ipa-kra-install.1|  3 ---
 install/tools/man/ipa-server-upgrade.1 |  3 ---
 ipapython/admintool.py | 17 ++---
 ipapython/install/cli.py   |  7 +--
 4 files changed, 7 insertions(+), 23 deletions(-)

diff --git a/install/tools/man/ipa-kra-install.1 b/install/tools/man/ipa-kra-install.1
index e3133eee188af0b613fca76b51d2f5b4f8d1ba7d..5bda4fe4b80947588ad7afde2c9f8fd81f320614 100644
--- a/install/tools/man/ipa-kra-install.1
+++ b/install/tools/man/ipa-kra-install.1
@@ -47,9 +47,6 @@ Enable debug output when more verbose output is needed
 .TP
 \fB\-q\fR, \fB\-\-quiet\fR
 Output only errors
-.TP
-\fB\-v\fR, \fB\-\-log-file\fR=\fFILE\fR
-Log to the given file
 .SH "EXIT STATUS"
 0 if the command was successful
 
diff --git a/install/tools/man/ipa-server-upgrade.1 b/install/tools/man/ipa-server-upgrade.1
index cbbdc590171bff0a88b67bcf1de961fd783ac35c..7f06e09fc4d181fa9a89772e7285d4a6567bc361 100644
--- a/install/tools/man/ipa-server-upgrade.1
+++ b/install/tools/man/ipa-server-upgrade.1
@@ -36,9 +36,6 @@ Print debugging information
 \fB\-q\fR, \fB\-\-quiet\fR
 Output only errors
 .TP
-\fB-\-log-file=FILE\fR
-Log to given file
-.TP
 
 .SH "EXIT STATUS"
 0 if the command was successful
diff --git a/ipapython/admintool.py b/ipapython/admintool.py
index e40f300b1318b7325eb2b39ec83970753d39c406..0c8a5d54676fac0704202cf183990115978cebed 100644
--- a/ipapython/admintool.py
+++ b/ipapython/admintool.py
@@ -113,8 +113,6 @@ class AdminTool(object):
 action="store_true", help="alias for --verbose (deprecated)")
 group.add_option("-q", "--quiet", dest="quiet", default=False,
 action="store_true", help="output only errors")
-group.add_option("--log-file", dest="log_file", default=None,
-metavar="FILE", help="log to the given file")
 parser.add_option_group(group)
 
 @classmethod
@@ -208,9 +206,8 @@ class AdminTool(object):
 :param _to_file: Setting this to false will disable logging to file.
 For internal use.
 
-If the --log-file option was given or if a filename is in
-self.log_file_name, the tool will log to that file. In this case,
-all messages are logged.
+If self.log_file_name is set, the tool will log to that file.
+In this case, all messages are logged.
 
 What is logged to the console depends on command-line options:
 the default is INFO; --quiet sets ERROR; --verbose sets DEBUG.
@@ -232,12 +229,8 @@ class AdminTool(object):
 self._setup_logging(log_file_mode=log_file_mode)
 
 def _setup_logging(self, log_file_mode='w', no_file=False):
-if no_file:
-log_file_name = None
-elif self.options.log_file:
-log_file_name = self.options.log_file
-else:
-log_file_name = self.log_file_name
+log_file_name = None if no_file else self.log_file_name
+
 if self.options.verbose:
 console_format = '%(name)s: %(levelname)s: %(message)s'
 verbose = True
@@ -249,10 +242,12 @@ class AdminTool(object):
 verbose = False
 else:
 verbose = True
+
 ipa_log_manager.standard_logging_setup(
 log_file_name, console_format=console_format,
 filemode=log_file_mode, debug=debug, verbose=verbose)
 self.log = ipa_log_manager.log_mgr.get_logger(self)
+
 if log_file_name:
 self.log.debug('Logging to %s' % log_file_name)
 elif not no_file:
diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py
index aed0bc9fe12e0c56987a4e2f78d73f476dcfc2c8..37ede148b0cbde2f4c4ef46bddf39d13cbda6ed9 100644
--- a/ipapython/install/cli.py
+++ b/ipapython/install/cli.py
@@ -265,12 +265,7 @@ class ConfigureTool(admintool.AdminTool):
 index += 1
 
 def _setup_logging(self, log_file_mode='w', no_file=False):
-if no_file:
-log_file_name = None
-elif self.options.log_file:
-log_file_name = self.options.log_file
-else:
-log_file_name = self.log_file_name
+log_file_name = None if no_file else self.log_file_name
 ipa_log_manager.standard_logging_setup(log_file_name,
debug=self.opt

[Freeipa-devel] [PATCH 0405] idviews: Add user certificate attribute to user ID overrides

[Tomas Babej]

Hi,

this extends the user ID overrides with capability to store the user
certificate.

https://fedorahosted.org/freeipa/ticket/4955

Tomas
From 4ab4ac5871f14d164544298fc5763321b8ef7558 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 3 Mar 2016 15:14:10 +0100
Subject: [PATCH] idviews: Add user certificate attribute to user ID overrides

---
 ACI.txt  |  2 +-
 API.txt  |  6 --
 install/share/71idviews.ldif |  2 +-
 ipalib/plugins/idviews.py| 34 +++---
 4 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/ACI.txt b/ACI.txt
index 24cb332ce6e10c82a5bfab76d084fb6c0277800d..ae00cf7a1b8e2ea0e33798993bb24dc5f06127e3 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -149,7 +149,7 @@ aci: (targetfilter = "(objectclass=ipahostgroup)")(version 3.0;acl "permission:S
 dn: cn=views,cn=accounts,dc=ipa,dc=example
 aci: (targetattr = "cn || createtimestamp || description || entryusn || gidnumber || ipaanchoruuid || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipaGroupOverride)")(version 3.0;acl "permission:System: Read Group ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=views,cn=accounts,dc=ipa,dc=example
-aci: (targetattr = "createtimestamp || description || entryusn || gecos || gidnumber || homedirectory || ipaanchoruuid || ipaoriginaluid || ipasshpubkey || loginshell || modifytimestamp || objectclass || uid || uidnumber")(targetfilter = "(objectclass=ipaUserOverride)")(version 3.0;acl "permission:System: Read User ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
+aci: (targetattr = "createtimestamp || description || entryusn || gecos || gidnumber || homedirectory || ipaanchoruuid || ipaoriginaluid || ipasshpubkey || loginshell || modifytimestamp || objectclass || uid || uidnumber || usercertificate")(targetfilter = "(objectclass=ipaUserOverride)")(version 3.0;acl "permission:System: Read User ID Overrides";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=ranges,cn=etc,dc=ipa,dc=example
 aci: (targetattr = "cn || createtimestamp || entryusn || ipabaseid || ipabaserid || ipaidrangesize || ipanttrusteddomainsid || iparangetype || ipasecondarybaserid || modifytimestamp || objectclass")(targetfilter = "(objectclass=ipaidrange)")(version 3.0;acl "permission:System: Read ID Ranges";allow (compare,read,search) userdn = "ldap:///all;;)
 dn: cn=views,cn=accounts,dc=ipa,dc=example
diff --git a/API.txt b/API.txt
index 5b75413f930d0e9caaffc68023bed8106d786653..34053640ccc0928ae76d9ae658a55e171478ceab 100644
--- a/API.txt
+++ b/API.txt
@@ -2429,7 +2429,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
 command: idoverrideuser_add
-args: 2,15,3
+args: 2,16,3
 arg: Str('idviewcn', cli_name='idview', multivalue=False, primary_key=True, query=True, required=True)
 arg: Str('ipaanchoruuid', attribute=True, cli_name='anchor', multivalue=False, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
@@ -2446,6 +2446,7 @@ option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', required=False)
 option: Int('uidnumber', attribute=True, cli_name='uid', minvalue=1, multivalue=False, required=False)
+option: Bytes('usercertificate', attribute=True, cli_name='certificate', multivalue=True, required=False)
 option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
@@ -2485,7 +2486,7 @@ output: ListOfEntries('result', (, ), Gettext('A list
 output: Output('summary', (, ), None)
 output: Output('truncated', , None)
 command: idoverrideuser_mod
-args: 2,18,3
+args: 2,19,3
 arg: Str('idviewcn', cli_name='idview', multivalue=False, primary_key=True, query=True, required=True)
 arg: Str('ipaanchoruuid', attribute=True, cli_name='anchor', multivalue=False, primary_key=True, query=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
@@ -2505,6 +2506,7 @@ option: Flag('rights', autofill=True, default=False)
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('uid', attribute=True, autofill=False, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', required=False)
 option: Int('uidnumber', attribute=True, autofill=False, cli_name='uid', minvalue=1, multivalue=False, required=False)
+option: Bytes('usercertif

Re: [Freeipa-devel] [MAN] [PATCH] 0004 Fix phrasing in man page for stageuser.py

[Tomas Babej]

On 07/07/2015 08:44 AM, Tomas Babej wrote:
> 
> 
> On 07/04/2015 02:03 PM, Jérôme Fenal wrote:
>> Hi all,
>>
>> A quick patch to the man page part of stageuser to avoid ambiguity in
>> the phrasing, spotted while translating the page.
>>
>> Regards,
>>
>> J.
>>
>>
>>
> 
> Thanks, ACK.
> 
> I will not push this patch to master until we branch off 4.2 development
> branch as it would disrupt already translated strings in the other
> languages.
> 
> Tomas
> 

One could say the condition has been fulfilled, so let's follow up on
that promise:

Pushed to master: 0e4e5cbddd349300340fd4a2fb19e8505f57b47d

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

Re: [Freeipa-devel] [PATCH 0087] Pylint: enable parallelism

[Tomas Babej]

On 03/07/2016 10:58 AM, Petr Spacek wrote:
> On 4.3.2016 14:13, Tomas Babej wrote:
>> On 03/01/2016 03:46 PM, Petr Spacek wrote:
>>> Hello,
>>>
>>> Pylint: enable parallelism
>>>
>>> The config file specifies 8 cores but Pylint very quickly
>>> ends up with 3 cores so do not worry about overwhelming your system.
>>
>> I like the idea of the patch, however, on my single-CPU VM this causes
>> additional overhead and lint ends up taking more time (+15%).
>>
>> >From the pylint docs [1]:
>>
>>  If the provided number is 0 then the number of CPUs will be used.
>>
>> I'd suggest we use this value instead, to get the best of both worlds :)
> 
> Here you go.
> 
> Petr^2 Spacek
> 
> 
>> Tomas
>>
>> [1] https://docs.pylint.org/run.html
>>
>>

Thanks, ACK.

Pushed to master: 42c01eb3270d8c47c41f7f9b0da7064edb4b0e47

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

Re: [Freeipa-devel] [PATCH 0404] ipalib: Fix user certificate docstrings

[Tomas Babej]

On 03/07/2016 05:50 AM, Fraser Tweedale wrote:
> On Fri, Mar 04, 2016 at 12:49:46PM +0100, Tomas Babej wrote:
>> Hi,
>>
>> this fixes incorrect usercertificate attribute docstrings in several IPA
>> objects.
>>
>> Tomas
>>
> ACK.
> 

Pushed to master: 8bf6aa2c1c957025c7d466f7a33202a191764f0b

-- 
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 0087] Pylint: enable parallelism

[Tomas Babej]

On 03/01/2016 03:46 PM, Petr Spacek wrote:
> Hello,
> 
> Pylint: enable parallelism
> 
> The config file specifies 8 cores but Pylint very quickly
> ends up with 3 cores so do not worry about overwhelming your system.

I like the idea of the patch, however, on my single-CPU VM this causes
additional overhead and lint ends up taking more time (+15%).

>From the pylint docs [1]:

 If the provided number is 0 then the number of CPUs will be used.

I'd suggest we use this value instead, to get the best of both worlds :)

Tomas

[1] https://docs.pylint.org/run.html

-- 
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 0429] fix suspicious except statement

[Tomas Babej]

On 03/02/2016 06:23 PM, Martin Basti wrote:
> 
> 
> On 02.03.2016 17:17, Martin Basti wrote:
>> Patch attached, read commit message for more info.
>>
>>
> https://fedorahosted.org/freeipa/ticket/5718
> 
> Updated patch attached.
> 
> 

Good catch, ACK. Issue is present both in master and ipa-4-3.

Pushed to:
master: 2c8e100c73ee1f0c9b57a0aabfc8ab8820e80687
ipa-4-3: 367a1cbd1e01cf758414f97606028571768fb459

-- 
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 0404] ipalib: Fix user certificate docstrings

[Tomas Babej]

Hi,

this fixes incorrect usercertificate attribute docstrings in several IPA
objects.

Tomas

From a056b839c48363721d0c27a196e9b47bdd28f12a Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Fri, 4 Mar 2016 12:45:39 +0100
Subject: [PATCH] ipalib: Fix user certificate docstrings

---
 ipalib/plugins/baseuser.py | 2 +-
 ipalib/plugins/host.py | 2 +-
 ipalib/plugins/service.py  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipalib/plugins/baseuser.py b/ipalib/plugins/baseuser.py
index a19229ab7db4eb70d2b60c6356568931a8bfc052..9c78a521dcb9a7a7db0be695468c85735d80620c 100644
--- a/ipalib/plugins/baseuser.py
+++ b/ipalib/plugins/baseuser.py
@@ -386,7 +386,7 @@ class baseuser(LDAPObject):
 Bytes('usercertificate*', validate_certificate,
 cli_name='certificate',
 label=_('Certificate'),
-doc=_('Base-64 encoded server certificate'),
+doc=_('Base-64 encoded user certificate'),
 ),
 )
 
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index d9be712d6383c636b18afd4829e84f129365500d..6ff751ca88187bb37ac64ca291234eed56e26e6f 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -508,7 +508,7 @@ class host(LDAPObject):
 Bytes('usercertificate*', validate_certificate,
 cli_name='certificate',
 label=_('Certificate'),
-doc=_('Base-64 encoded server certificate'),
+doc=_('Base-64 encoded host certificate'),
 ),
 Str('krbprincipalname?',
 label=_('Principal name'),
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index 1b43f58578971af639751681451ae14487cd9e97..644b07f463a008eff12d3ff36de9404c03cf8a69 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -494,7 +494,7 @@ class service(LDAPObject):
 Bytes('usercertificate*', validate_certificate,
 cli_name='certificate',
 label=_('Certificate'),
-doc=_('Base-64 encoded server certificate'),
+doc=_('Base-64 encoded service certificate'),
 flags=['no_search',],
 ),
 StrEnum('ipakrbauthzdata*',
-- 
2.5.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

Re: [Freeipa-devel] [PATCH] 951 webui: fail nicely if cookies are disabled

[Tomas Babej]

On 01/28/2016 04:25 PM, Petr Vobornik wrote:
> On 01/28/2016 04:23 PM, Tomas Babej wrote:
>>
>>
>> On 01/28/2016 04:15 PM, Petr Vobornik wrote:
>>> Reworks also sessionStorage test because disablement of cookies might be
>>> connected with sessionStorage and localStorage. E.g. Chrome raises
>>> exception when *Storage is accessed with "Block sites from setting any
>>> data" settings set in "Content Settings/Cookies" section.
>>>
>>> https://fedorahosted.org/freeipa/ticket/4338
>>>
>>>
>>
>> Seems that two spaces inserted themselves to the error message for
>> localStorage :)
>>
> 
> updated patch attached.
> 
> 

ACK, works fine.

Pushed to master: 3c519951c5a719421d5abfa864dfeb6fbce6869d

-- 
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 0425] pylint: suppress false positive no-member errors

[Tomas Babej]

On 03/02/2016 01:35 PM, David Kupka wrote:
> Tested with pylint-1.5.4-2, works for me, ACK.
> 
> - Original Message -
> From: "Martin Basti" 
> To: "freeipa-devel" 
> Sent: Tuesday, March 1, 2016 5:55:54 PM
> Subject: Re: [Freeipa-devel] [PATCH 0425] pylint: suppress false positive 
> no-member errors
> 
> 
> 
> On 25.02.2016 17:50, Martin Basti wrote: 
> 
> 
> 
> 
> On 25.02.2016 15:48, Martin Basti wrote: 
> 
> 
> The last pylint 1.5 patch, \o/ 
> 
> https://fedorahosted.org/freeipa/ticket/5615 
> 
> 
> self-NACK too broad disables 
> 
> 
> Updated patches attached. 
> 

Pushed to:
ipa-4-2: aaad91d32ee855813bac5f57f8af128cfee327a5
ipa-4-3: 76545e2a54ebbfc5422d2a4bcd51fad1bbd8a79a
master: 72d5499c5a902c860c5496ee6e604526672e5777

-- 
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] 954 fix incorrect name of ipa-winsync-migrate command in help

[Tomas Babej]

On 03/02/2016 12:42 PM, Petr Vobornik wrote:
> Help and status text used incorrect name "ipa-migrate-winsync"
> 
> https://fedorahosted.org/freeipa/ticket/5713
> 
> 

ACK, Pushed to:
ipa-4-2: 7151ea394aac00ca596a8d7460a2fcefd258b36e
ipa-4-3: 57e02c7140fe2ad6fe6a6bc9823f84500bb78732
master: c68e9510d03abb75d353e209ea32ac9d1ed362bc

-- 
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 0086] Fix URL for reporting bugs in string

[Tomas Babej]

On 03/01/2016 03:23 PM, Petr Spacek wrote:
> Hello,
> 
> Fix URL for reporting bugs in strings.
> 

ACK, good catch.

-- 
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 0086] Fix URL for reporting bugs in string

[Tomas Babej]

On 03/01/2016 03:25 PM, Tomas Babej wrote:
> 
> 
> On 03/01/2016 03:23 PM, Petr Spacek wrote:
>> Hello,
>>
>> Fix URL for reporting bugs in strings.
>>
> 
> ACK, good catch.
> 

Pushed to master: e9922c36b15476f99426d0e85fde857887fb5c7d

-- 
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] 0007 Refactor test_sudocmd_plugin

[Tomas Babej]

On 03/01/2016 01:27 PM, Aleš Mareček wrote:
> ACK.
> Thank you!
>  - alich -
> 
> - Original Message -
>> From: "Filip Skola" 
>> To: "Aleš Mareček" 
>> Cc: freeipa-devel@redhat.com, "Milan Kubík" 
>> Sent: Wednesday, February 24, 2016 8:07:55 PM
>> Subject: Re: [Freeipa-devel] [PATCH] 0007 Refactor test_sudocmd_plugin
>>
>> Hi,
>>
>> these problems have been fixed.
>>
>> F.
>>
>> - Original Message -
>>> NACK.
>>> Some little changes still required:
>>>  * fixing the pep8 errors
>>>  * fixing the wrong comment
>>>
>>> [root@master2 freeipa]# pep8 ipatests/test_xmlrpc/test_sudocmd_plugin.py
>>> ipatests/test_xmlrpc/test_sudocmd_plugin.py:94:80: E501 line too long (87 >
>>> 79 characters)
>>> ipatests/test_xmlrpc/test_sudocmd_plugin.py:97:80: E501 line too long (87 >
>>> 79 characters)
>>> ipatests/test_xmlrpc/test_sudocmd_plugin.py:134:80: E501 line too long (80

>>> 79 characters)
>>>   
>>> [root@master2 freeipa]# pep8 ipatests/test_xmlrpc/tracker/sudocmd_plugin.py
>>> ipatests/test_xmlrpc/tracker/sudocmd_plugin.py:14:80: E501 line too long
>>> (81
 79 characters)
>>>
>>> [root@master2 freeipa]# grep 'Class for'
>>> ipatests/test_xmlrpc/tracker/sudocmd_plugin.py
>>> """ Class for host plugin like tests """
>>>
>>>
>>> - Original Message -
 From: "Filip Skola" 
 To: "Aleš Mareček" 
 Cc: freeipa-devel@redhat.com, "Milan Kubík" 
 Sent: Monday, February 22, 2016 1:59:43 PM
 Subject: Re: [Freeipa-devel] [PATCH] 0007 Refactor test_sudocmd_plugin

 Hi,

 sudocmd tracker has been created.

 Filip

 - Original Message -
> NACK.
>
> "create_sudocmd" and "delete_sudocmd" should be placed in Tracker. So
> this
> patch should create the tracker as well.
>
> - Original Message -
>> From: "Filip Skola" 
>> To: freeipa-devel@redhat.com
>> Sent: Monday, January 25, 2016 3:57:25 PM
>> Subject: [Freeipa-devel] [PATCH] 0007 Refactor test_sudocmd_plugin
>>
>> Hello,
>>
>> attaching refactored sudocmd_plugin.
>>
>> Filip
>> --
>> 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
>

>>>
>>
> 

Pushed to master: 007c360f85151caab7d608cc0a4eb1916b18eba9

Note: Please keep to down-posting on freeipa-devel.

-- 
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] 0008 Refactor test_sudocmdgroup_plugin, create SudoCmdGroupTracker

[Tomas Babej]

On 03/01/2016 01:29 PM, Aleš Mareček wrote:
> ACK.
> Thank you!
> 
> Master push: Make sure it will go *after or together with* the previous patch 
> from Filip, #0007, thanks!
> 
>  - alich -
> 
> - Original Message -
>> From: "Filip Skola" 
>> To: "Aleš Mareček" 
>> Cc: freeipa-devel@redhat.com, "Milan Kubík" 
>> Sent: Wednesday, February 24, 2016 8:13:03 PM
>> Subject: Re: [Freeipa-devel] [PATCH] 0008 Refactor test_sudocmdgroup_plugin, 
>> create SudoCmdGroupTracker
>>
>> Hi,
>>
>> fixed. To be honest, I left that +1char longer lines there on purpose. IMHO
>> it brings better readability and pep8 *.py | wc -l in test_xmlrpc dir
>> returns an overwhelming number anyway. But yeah, some of these weren't
>> really necessary...so I changed them all :)
>>
>> This patch is dependent on 0007-3 patch.
>>
>> Filip
>>
>> - Original Message -
>>> NACK.
>>>
>>>
>>> [root@master2 test_xmlrpc]# pep8 test_sudocmdgroup_plugin.py
>>> test_sudocmdgroup_plugin.py:26:80: E501 line too long (80 > 79 characters)
>>> test_sudocmdgroup_plugin.py:70:80: E501 line too long (80 > 79 characters)
>>> test_sudocmdgroup_plugin.py:76:80: E501 line too long (80 > 79 characters)
>>> test_sudocmdgroup_plugin.py:84:80: E501 line too long (80 > 79 characters)
>>> test_sudocmdgroup_plugin.py:90:80: E501 line too long (80 > 79 characters)
>>> test_sudocmdgroup_plugin.py:98:80: E501 line too long (80 > 79 characters)
>>> test_sudocmdgroup_plugin.py:104:80: E501 line too long (80 > 79 characters)
>>> test_sudocmdgroup_plugin.py:166:80: E501 line too long (80 > 79 characters)
>>> test_sudocmdgroup_plugin.py:180:80: E501 line too long (80 > 79 characters)
>>> test_sudocmdgroup_plugin.py:186:80: E501 line too long (84 > 79 characters)
>>> [root@master2 test_xmlrpc]# pep8 tracker/sudocmdgroup_plugin.py
>>> tracker/sudocmdgroup_plugin.py:36:80: E501 line too long (82 > 79
>>> characters)
>>> tracker/sudocmdgroup_plugin.py:42:80: E501 line too long (82 > 79
>>> characters)
>>> tracker/sudocmdgroup_plugin.py:46:80: E501 line too long (85 > 79
>>> characters)
>>> tracker/sudocmdgroup_plugin.py:55:80: E501 line too long (82 > 79
>>> characters)
>>> tracker/sudocmdgroup_plugin.py:64:80: E501 line too long (82 > 79
>>> characters)
>>>
>>>
>>>
>>> - Original Message -
 From: "Filip Skola" 
 To: "Aleš Mareček" 
 Cc: freeipa-devel@redhat.com, "Milan Kubík" 
 Sent: Monday, February 22, 2016 3:41:36 PM
 Subject: Re: [Freeipa-devel] [PATCH] 0008 Refactor
 test_sudocmdgroup_plugin, create SudoCmdGroupTracker

 Hi,

 the test has been updated so it now uses the SudoCmdTracker (from the
 previous patch).

 Filip

 - Original Message -
> NACK.
>
> "create_sudocmd" and "delete_sudocmd" should be imported from Tracker,
> not
> from the previous test (sudocmd_plugin).
>
>   - alich -
>
> - Original Message -
>> From: "Filip Skola" 
>> To: freeipa-devel@redhat.com
>> Sent: Thursday, January 28, 2016 12:49:17 PM
>> Subject: [Freeipa-devel] [PATCH] 0008 Refactor
>> test_sudocmdgroup_plugin,
>> create SudoCmdGroupTracker
>>
>> Hi,
>>
>> sending the next sudo patch. This one depends on the previous one
>> (sudocmd_plugin).
>>
>> Filip
>>
>> --
>> 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
>

>>>
>>
> 

Pushed to master: dd38602fa5ea3f0a51db5458e846f3756ab74e47

-- 
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 00136] use LDAPS during standalone CA/KRA subsystem deployment

[Tomas Babej]

On 02/26/2016 06:03 PM, Martin Babinsky wrote:
> This patch fixes https://fedorahosted.org/freeipa/ticket/5570 and also
> enables CA installation on CA-less master with hardened dirsrv
> configuration.
> 
> When testing I ran into the issue with Dogtag restart during KRA
> installation [1] which I will try to troubleshoot with Dogtag guys. You
> are welcome to troubleshoot it also during the review, maybe I did some
> misconfiguration on my part.
> 
> [1] https://www.redhat.com/archives/pki-devel/2016-February/msg00100.html
> 
> 

Works fine, ACK!

Pushed to:
master: 276d16775a4ce8af5d39ca8a7bf5bcd638df343f
ipa-4-3: 8de860cc081dd0e5e8b0ae3a97fbb89d6d1386c4
ipa-4-2: c7c126fb51c5b2c92622f493d1c7efbadb899e49

-- 
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] 953 advise: configure TLS in redhat_nss_pam_ldapd and redhat_nss_ldap plugins

[Tomas Babej]

On 03/01/2016 10:36 AM, Petr Vobornik wrote:
> On 02/26/2016 03:29 PM, Petr Spacek wrote:
>> On 25.2.2016 18:01, Petr Vobornik wrote:
>>> I did not add --enableldapstarttls to config_redhat_nss_ldap because
>>> I'm not
>>> sure if it is present on el5 (IMO it is not).
>>>
>>> authconfig in:
>>> * config_redhat_nss_ldap got
>>>* --enableldaptls
>>>
>>> * config_redhat_nss_pam_ldapd got
>>>* --enableldaptls
>>>* --enableldapstarttls
>>> options
>>
>> Shouldn't it get only one of them?
>>
>> It seems weird to enable both at the same time.
>>
>> Petr^2 Spacek
>>
>>> https://fedorahosted.org/freeipa/ticket/5654
>>
> 
> Updated patch attached. It uses only --enableldaptls in both commands.
> 
> --enableldapstarttls is an alias for enableldaptls.
> 
> After testing and checking /etc/openldap/ldap.conf, I don't think that
> these options have any effect on el6. There is no 'ssl no' or 'ssl
> start_tls' in any combination or lack of the options. Maybe they have
> effect somewhere else. Anyway it shouldn't do any harm.
> 
> 

ACK.

Pushed to:
master: 02d3ea106214c7e170cb9bf051e4085ade440134
ipa-4-3: b2c5c32d78f099ecc0fb1f10fbf2acd9e36da3ae
ipa-4-2: 6111a30962db4f4bf095201854f3aaa3493adf7c

-- 
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 200] slapi-nis: update configuration to allow external members

[Tomas Babej]

On 02/29/2016 01:07 PM, Tomas Babej wrote:
> 
> 
> On 02/29/2016 07:19 AM, Jan Cholasta wrote:
>> On 26.2.2016 21:38, Lukas Slebodnik wrote:
>>> On (26/02/16 12:37), Tomas Babej wrote:
>>>>
>>>>
>>>> On 02/26/2016 07:30 AM, Jan Cholasta wrote:
>>>>> On 22.2.2016 19:56, Tomas Babej wrote:
>>>>>>
>>>>>>
>>>>>> On 02/22/2016 06:14 PM, Alexander Bokovoy wrote:
>>>>>>> On Mon, 22 Feb 2016, Tomas Babej wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 02/22/2016 11:48 AM, Alexander Bokovoy wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> attached patch should update compat tree configuration if it
>>>>>>>>> exist to
>>>>>>>>> follow slapi-nis 0.55 which has support for external members of IPA
>>>>>>>>> groups.
>>>>>>>>>
>>>>>>>>> However, the real work is done in SSSD. These patches are not
>>>>>>>>> upstreamed
>>>>>>>>> yet. We'll need to bump SSSD dependency in future once they come to
>>>>>>>>> distros.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> This looks good.
>>>>>>>>
>>>>>>>> However, the new update file needs to be added to Makefile.am.
>>>>>>>> Additionally, patch adds a whitespace error.
>>>>>>> Updated patch is attached.
>>>>>>>
>>>>>>
>>>>>> ACK.
>>>>>>
>>>>>> This should not be pushed until the dependency for SSSD can be bumped.
>>>>>
>>>>> https://bodhi.fedoraproject.org/updates/FEDORA-2016-d872920f74
>>>>>
>>>>
>>>> Attaching the required spec change.
>>>>
>>>> Tomas
>>>
>>>> From dae8b8fd0b23bf25ccf75b275deaa5c599faa27b Mon Sep 17 00:00:00 2001
>>>> From: Tomas Babej <tba...@redhat.com>
>>>> Date: Fri, 26 Feb 2016 12:35:09 +0100
>>>> Subject: [PATCH] spec: Bump required sssd version to 1.13.3-5
>>>>
>>>> Required as part of: https://fedorahosted.org/freeipa/ticket/4403
>>>   ^
>>> There isn't mentioned sssd related ticket in slapi-nis bug
>>> It would be good to add also sssd related ticket to commit message
>>> https://fedorahosted.org/sssd/ticket/2522
>>
>> +1, that's <https://fedorahosted.org/freeipa/ticket/4436> in IPA.
>>
> 
> Attaching patch with updated commit message.
> 
> Tomas
> 

Rebased and pushed to:

ipa-4-2: dbea05e1578e2d6d80940f1d4289ecd98a0593ab
ipa-4-3: 5e2c6b0f630300e20c11595e67c61e7eb3982aae
master: 271086ebdd10b2229534220d830d1cbd5af6a352

-- 
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 0401] ipa-adtrust-install: Allow dash in the NETBIOS name

[Tomas Babej]

On 02/29/2016 03:20 PM, Martin Babinsky wrote:
> On 02/29/2016 02:59 PM, Tomas Babej wrote:
>>
>>
>> On 02/29/2016 02:04 PM, Martin Babinsky wrote:
>>> On 02/25/2016 02:13 PM, Tomas Babej wrote:
>>>> Hi,
>>>>
>>>> Dash should be one of the allowed characters in the netbios names,
>>>> so relax the too strict validation.
>>>>
>>>> Note: the set of allowed characters might expand in the future
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/5286
>>>>
>>>> Tomas
>>>>
>>>>
>>>>
>>>
>>> NACK, since this patch breaks the interactive installation of adtrust,
>>> see the following log: http://fpaste.org/331088/56750906/
>>>
>>> Keep in mind that the argument of any is first instantiated and then
>>> each element is tested. Since during interactive installation there is a
>>> possibility in the current code that check_netbios_name receives None as
>>> argument. You will have to correct this somehow.
>>>
>>
>> Good catch. My original patch indeed breaks the interactive installation
>> on a clean machine where no netbios name has been specified explicitly.
>>
>> Fixed, attaching patches for both branches.
>>
>> Tomas
>>
> ACK
> 

Pushed to ipa-4-2: 657838462c4b0ce5be2cee584b3be112aca6c660
Pushed to ipa-4-3: 1496fb779d72fb590376df23e39206938fe8dad2
Pushed to master: b41fbceeafea126f8e014da5d3596138c6cf0feb

-- 
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 0401] ipa-adtrust-install: Allow dash in the NETBIOS name

[Tomas Babej]

On 02/29/2016 02:04 PM, Martin Babinsky wrote:
> On 02/25/2016 02:13 PM, Tomas Babej wrote:
>> Hi,
>>
>> Dash should be one of the allowed characters in the netbios names,
>> so relax the too strict validation.
>>
>> Note: the set of allowed characters might expand in the future
>>
>> https://fedorahosted.org/freeipa/ticket/5286
>>
>> Tomas
>>
>>
>>
> 
> NACK, since this patch breaks the interactive installation of adtrust,
> see the following log: http://fpaste.org/331088/56750906/
> 
> Keep in mind that the argument of any is first instantiated and then
> each element is tested. Since during interactive installation there is a
> possibility in the current code that check_netbios_name receives None as
> argument. You will have to correct this somehow.
> 

Good catch. My original patch indeed breaks the interactive installation
on a clean machine where no netbios name has been specified explicitly.

Fixed, attaching patches for both branches.

Tomas
From 5620f30ec94bf58f5cf1c36d5e480c65acadbf97 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 25 Feb 2016 14:09:48 +0100
Subject: [PATCH] ipa-adtrust-install: Allow dash in the NETBIOS name

---
 install/tools/ipa-adtrust-install|  6 --
 ipaserver/install/adtrustinstance.py | 18 --
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 8b3fce2db8535ac2124a347c6ca2f63f7c6e3e42..f972835ec89c18d0453e659142d4434aaa571dd5 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -88,13 +88,15 @@ def parse_options():
 
 def netbios_name_error(name):
 print("\nIllegal NetBIOS name [%s].\n" % name)
-print("Up to 15 characters and only uppercase ASCII letter and digits are allowed.")
+print("Up to 15 characters and only uppercase ASCII letters, digits "
+  "and dashes are allowed.")
 
 def read_netbios_name(netbios_default):
 netbios_name = ""
 
 print("Enter the NetBIOS name for the IPA domain.")
-print("Only up to 15 uppercase ASCII letters and digits are allowed.")
+print("Only up to 15 uppercase ASCII letters, digits "
+  "and dashes are allowed.")
 print("Example: EXAMPLE.")
 print("")
 print("")
diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index 9e7e001f7c505d09d5a61164399e9ed256ae9865..8a6e31e5cdecb27d67c0a047151aaa9b503b1405 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -51,7 +51,7 @@ from ipaplatform.tasks import tasks
 if six.PY3:
 unicode = str
 
-ALLOWED_NETBIOS_CHARS = string.ascii_uppercase + string.digits
+ALLOWED_NETBIOS_CHARS = string.ascii_uppercase + string.digits + '-'
 
 UPGRADE_ERROR = """
 Entry %(dn)s does not exist.
@@ -90,13 +90,19 @@ def ipa_smb_conf_exists():
 return False
 
 
-def check_netbios_name(s):
-# NetBIOS names may not be longer than 15 allowed characters
-if not s or len(s) > 15 or \
-   ''.join([c for c in s if c not in ALLOWED_NETBIOS_CHARS]):
+def check_netbios_name(name):
+# Empty NetBIOS name is not allowed
+if name is None:
 return False
 
-return True
+# NetBIOS names may not be longer than 15 allowed characters
+invalid_netbios_name = any([
+len(name) > 15,
+''.join([c for c in name if c not in ALLOWED_NETBIOS_CHARS])
+])
+
+return not invalid_netbios_name
+
 
 def make_netbios_name(s):
 return ''.join([c for c in s.split('.')[0].upper() \
-- 
2.5.0

From a377b98d222bf82fcad8e7b8f2993e01be4f9f58 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 25 Feb 2016 14:02:08 +0100
Subject: [PATCH] ipa-adtrust-install: Allow dash in the NETBIOS name

Dash should be one of the allowed characters in the netbios names,
so relax the too strict validation.

Note: the set of allowed characters might expand in the future

https://fedorahosted.org/freeipa/ticket/5286
---
 install/tools/ipa-adtrust-install| 19 ---
 ipaserver/install/adtrustinstance.py | 18 --
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index d2cec17891976e911d42869d5c871bf4f2805db7..ada11e076f7dec6f47afc02dc178a5306fb53daf 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -74,17 +74,22 @@ def parse_options():
 return safe_options, options
 
 def netbios_name_error(name):
-print "\nIllegal NetBIOS name [%s].\n" % name
-print "Up to 15 characters and only uppercase ASCII letter and digits are allowed."
+

Re: [Freeipa-devel] [PATCH 0421] Make PTR records check optional for IPA installation

[Tomas Babej]

On 02/29/2016 01:20 PM, Tomas Babej wrote:
> 
> 
> On 02/26/2016 10:01 AM, Oleg Fayans wrote:
>>
>>
>> On 02/25/2016 12:06 PM, Petr Spacek wrote:
>>> On 24.2.2016 15:13, Martin Basti wrote:
>>>> https://fedorahosted.org/freeipa/ticket/5686
>>>>
>>>> Patch attached.
>>>
>>> LGTM, ACK if it passes QE testing.
>>>
>> That did it. Works with both replica-prepare under domain level 0 and
>> with replica-install on domlevel1.
>>
>> ACK
>>
>>
> 
> Pushed to:
> master: 8f01b47ed96ee3118479d366a71a018e8f938156
> ipa-4-3: bd725f4ba4449f285a8b1f646dc7a54978211ff9
> ipa-4-2: e66ce1a63b5ea11fbecb9e268d1ac40f3994b339
> 

Note: 4.2 patch required a manual rebase.

-- 
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 0421] Make PTR records check optional for IPA installation

[Tomas Babej]

On 02/26/2016 10:01 AM, Oleg Fayans wrote:
> 
> 
> On 02/25/2016 12:06 PM, Petr Spacek wrote:
>> On 24.2.2016 15:13, Martin Basti wrote:
>>> https://fedorahosted.org/freeipa/ticket/5686
>>>
>>> Patch attached.
>>
>> LGTM, ACK if it passes QE testing.
>>
> That did it. Works with both replica-prepare under domain level 0 and
> with replica-install on domlevel1.
> 
> ACK
> 
> 

Pushed to:
master: 8f01b47ed96ee3118479d366a71a018e8f938156
ipa-4-3: bd725f4ba4449f285a8b1f646dc7a54978211ff9
ipa-4-2: e66ce1a63b5ea11fbecb9e268d1ac40f3994b339

-- 
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 200] slapi-nis: update configuration to allow external members

[Tomas Babej]

On 02/29/2016 07:19 AM, Jan Cholasta wrote:
> On 26.2.2016 21:38, Lukas Slebodnik wrote:
>> On (26/02/16 12:37), Tomas Babej wrote:
>>>
>>>
>>> On 02/26/2016 07:30 AM, Jan Cholasta wrote:
>>>> On 22.2.2016 19:56, Tomas Babej wrote:
>>>>>
>>>>>
>>>>> On 02/22/2016 06:14 PM, Alexander Bokovoy wrote:
>>>>>> On Mon, 22 Feb 2016, Tomas Babej wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 02/22/2016 11:48 AM, Alexander Bokovoy wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> attached patch should update compat tree configuration if it
>>>>>>>> exist to
>>>>>>>> follow slapi-nis 0.55 which has support for external members of IPA
>>>>>>>> groups.
>>>>>>>>
>>>>>>>> However, the real work is done in SSSD. These patches are not
>>>>>>>> upstreamed
>>>>>>>> yet. We'll need to bump SSSD dependency in future once they come to
>>>>>>>> distros.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> This looks good.
>>>>>>>
>>>>>>> However, the new update file needs to be added to Makefile.am.
>>>>>>> Additionally, patch adds a whitespace error.
>>>>>> Updated patch is attached.
>>>>>>
>>>>>
>>>>> ACK.
>>>>>
>>>>> This should not be pushed until the dependency for SSSD can be bumped.
>>>>
>>>> https://bodhi.fedoraproject.org/updates/FEDORA-2016-d872920f74
>>>>
>>>
>>> Attaching the required spec change.
>>>
>>> Tomas
>>
>>> From dae8b8fd0b23bf25ccf75b275deaa5c599faa27b Mon Sep 17 00:00:00 2001
>>> From: Tomas Babej <tba...@redhat.com>
>>> Date: Fri, 26 Feb 2016 12:35:09 +0100
>>> Subject: [PATCH] spec: Bump required sssd version to 1.13.3-5
>>>
>>> Required as part of: https://fedorahosted.org/freeipa/ticket/4403
>>   ^
>> There isn't mentioned sssd related ticket in slapi-nis bug
>> It would be good to add also sssd related ticket to commit message
>> https://fedorahosted.org/sssd/ticket/2522
> 
> +1, that's <https://fedorahosted.org/freeipa/ticket/4436> in IPA.
> 

Attaching patch with updated commit message.

Tomas
From 80e381cb5139057258cf710006ccae6b0c6d75b7 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Fri, 26 Feb 2016 12:35:09 +0100
Subject: [PATCH] spec: Bump required sssd version to 1.13.3-5

https://fedorahosted.org/freeipa/ticket/4403
https://fedorahosted.org/freeipa/ticket/4436
---
 freeipa.spec.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index db354590236aaf6037cedd33de819a14ba4264d4..2c7bac3815c38173b5002ea93b666c125485d55a 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -323,7 +323,7 @@ Requires: pam_krb5
 Requires: curl
 Requires: libcurl >= 7.21.7-2
 Requires: xmlrpc-c >= 1.27.4
-Requires: sssd >= 1.13.1
+Requires: sssd >= 1.13.3-5
 Requires: python-sssdconfig
 Requires: certmonger >= 0.78
 Requires: nss-tools
-- 
2.5.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 0403] adtrustinstance: Make sure smb.conf exists

[Tomas Babej]

Hi,

The 'net' command fails unless smb.conf exists. Touch
the file prior to any 'net' call to make sure we do not crash
for this very reason.

I couldn't find the aforementioned Samba bug either in the RH/Samba
bugzilla, despite spending non-trivial amount of time searching for it.

Can somebody point me to it? I referenced the IPA ticket for now.

https://fedorahosted.org/freeipa/ticket/5687
From 4fd714beec5c95e079028836168e0188ed27ff0c Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Fri, 26 Feb 2016 14:28:26 +0100
Subject: [PATCH] adtrustinstance: Make sure smb.conf exists

The 'net' command fails unless smb.conf exists. Touch
the file prior to any 'net' call to make sure we do not crash
for this very reason.

https://fedorahosted.org/freeipa/ticket/5687
---
 ipaserver/install/adtrustinstance.py | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index 54a23bc15d818864f0987b02294244414ed9883f..cd346e0698e7c7d2e0f9f76893104cdf7262fb98 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -517,6 +517,11 @@ class ADTRUSTInstance(service.Service):
 os.write(tmp_fd, conf)
 os.close(tmp_fd)
 
+# Workaround for: https://fedorahosted.org/freeipa/ticket/5687
+# We make sure that paths.SMB_CONF file exists, hence touch it
+with open(paths.SMB_CONF, 'a'):
+os.utime(paths.SMB_CONF, None)
+
 args = [paths.NET, "conf", "import", tmp_name]
 
 try:
-- 
2.5.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

Re: [Freeipa-devel] [PATCH 200] slapi-nis: update configuration to allow external members

[Tomas Babej]

On 02/26/2016 07:30 AM, Jan Cholasta wrote:
> On 22.2.2016 19:56, Tomas Babej wrote:
>>
>>
>> On 02/22/2016 06:14 PM, Alexander Bokovoy wrote:
>>> On Mon, 22 Feb 2016, Tomas Babej wrote:
>>>>
>>>>
>>>> On 02/22/2016 11:48 AM, Alexander Bokovoy wrote:
>>>>> Hi,
>>>>>
>>>>> attached patch should update compat tree configuration if it exist to
>>>>> follow slapi-nis 0.55 which has support for external members of IPA
>>>>> groups.
>>>>>
>>>>> However, the real work is done in SSSD. These patches are not
>>>>> upstreamed
>>>>> yet. We'll need to bump SSSD dependency in future once they come to
>>>>> distros.
>>>>>
>>>>>
>>>>>
>>>>
>>>> This looks good.
>>>>
>>>> However, the new update file needs to be added to Makefile.am.
>>>> Additionally, patch adds a whitespace error.
>>> Updated patch is attached.
>>>
>>
>> ACK.
>>
>> This should not be pushed until the dependency for SSSD can be bumped.
> 
> https://bodhi.fedoraproject.org/updates/FEDORA-2016-d872920f74
> 

Attaching the required spec change.

Tomas
From dae8b8fd0b23bf25ccf75b275deaa5c599faa27b Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Fri, 26 Feb 2016 12:35:09 +0100
Subject: [PATCH] spec: Bump required sssd version to 1.13.3-5

Required as part of: https://fedorahosted.org/freeipa/ticket/4403
---
 freeipa.spec.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 7dff2605d2d681a07841f401967971cbfffe2243..3ae12fff7745e24edffa0c49d314aefb4feda404 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -323,7 +323,7 @@ Requires: pam_krb5
 Requires: curl
 Requires: libcurl >= 7.21.7-2
 Requires: xmlrpc-c >= 1.27.4
-Requires: sssd >= 1.13.1
+Requires: sssd >= 1.13.3-5
 Requires: python-sssdconfig
 Requires: certmonger >= 0.78
 Requires: nss-tools
-- 
2.5.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 0401] ipa-adtrust-install: Allow dash in the NETBIOS name

[Tomas Babej]

Hi,

Dash should be one of the allowed characters in the netbios names,
so relax the too strict validation.

Note: the set of allowed characters might expand in the future

https://fedorahosted.org/freeipa/ticket/5286

Tomas
From eab57f7d15758bd998d944b33f338a35a57de218 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 25 Feb 2016 14:02:08 +0100
Subject: [PATCH] ipa-adtrust-install: Allow dash in the NETBIOS name

Dash should be one of the allowed characters in the netbios names,
so relax the too strict validation.

Note: the set of allowed characters might expand in the future

https://fedorahosted.org/freeipa/ticket/5286
---
 install/tools/ipa-adtrust-install| 19 ---
 ipaserver/install/adtrustinstance.py | 15 +--
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index d2cec17891976e911d42869d5c871bf4f2805db7..ada11e076f7dec6f47afc02dc178a5306fb53daf 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -74,17 +74,22 @@ def parse_options():
 return safe_options, options
 
 def netbios_name_error(name):
-print "\nIllegal NetBIOS name [%s].\n" % name
-print "Up to 15 characters and only uppercase ASCII letter and digits are allowed."
+print(
+"\nIllegal NetBIOS name [{}].\n\n"
+"Up to 15 characters and only uppercase ASCII letters, "
+"digits and dashes are allowed.".format(name)
+)
 
 def read_netbios_name(netbios_default):
 netbios_name = ""
 
-print "Enter the NetBIOS name for the IPA domain."
-print "Only up to 15 uppercase ASCII letters and digits are allowed."
-print "Example: EXAMPLE."
-print ""
-print ""
+print(
+"Enter the NetBIOS name for the IPA domain.\n"
+"Only up to 15 uppercase ASCII letters, digits and "
+"dashes are allowed.\n"
+"Example: EXAMPLE\n\n"
+)
+
 if not netbios_default:
 netbios_default = "EXAMPLE"
 while True:
diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index a890f141b5ea5d79511cbd7eb3d24c73cf04f3b5..9fa1c37b6e5a23cf2f6da3dc084e1c3f050ac721 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -44,7 +44,7 @@ from ipaplatform.paths import paths
 from ipaplatform.tasks import tasks
 
 
-ALLOWED_NETBIOS_CHARS = string.ascii_uppercase + string.digits
+ALLOWED_NETBIOS_CHARS = string.ascii_uppercase + string.digits + '-'
 
 UPGRADE_ERROR = """
 Entry %(dn)s does not exist.
@@ -83,13 +83,16 @@ def ipa_smb_conf_exists():
 return False
 
 
-def check_netbios_name(s):
+def check_netbios_name(name):
 # NetBIOS names may not be longer than 15 allowed characters
-if not s or len(s) > 15 or \
-   ''.join([c for c in s if c not in ALLOWED_NETBIOS_CHARS]):
-return False
+invalid_netbios_name = any([
+not name,
+len(name) > 15,
+''.join([c for c in name if c not in ALLOWED_NETBIOS_CHARS])
+])
+
+return not invalid_netbios_name
 
-return True
 
 def make_netbios_name(s):
 return ''.join([c for c in s.split('.')[0].upper() \
-- 
2.5.0

From 853e3e5759a12214a72c27c7d76ab3fe9ed8df9a Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 25 Feb 2016 14:09:48 +0100
Subject: [PATCH] ipa-adtrust-install: Allow dash in the NETBIOS name

Dash should be one of the allowed characters in the netbios names,
so relax the too strict validation.

Note: the set of allowed characters might expand in the future

https://fedorahosted.org/freeipa/ticket/5286
---
 install/tools/ipa-adtrust-install|  6 --
 ipaserver/install/adtrustinstance.py | 15 +--
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 8b3fce2db8535ac2124a347c6ca2f63f7c6e3e42..f972835ec89c18d0453e659142d4434aaa571dd5 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -88,13 +88,15 @@ def parse_options():
 
 def netbios_name_error(name):
 print("\nIllegal NetBIOS name [%s].\n" % name)
-print("Up to 15 characters and only uppercase ASCII letter and digits are allowed.")
+print("Up to 15 characters and only uppercase ASCII letters, digits "
+  "and dashes are allowed.")
 
 def read_netbios_name(netbios_default):
 netbios_name = ""
 
 print("Enter the NetBIOS name for the IPA domain.")
-print("Only up to 15 uppercase ASCII letters and digits are allowed.")
+print("Only up to 15 uppercase ASCII letters, digits "
+  "and dashes

Re: [Freeipa-devel] [PATCH 0134] CI tests: use old schema when testing hostmask-based sudo rules

[Tomas Babej]

On 02/18/2016 10:32 AM, Martin Babinsky wrote:
> https://fedorahosted.org/freeipa/ticket/5625

ACK, works fine for me.

Thanks for the patch.

Pushed to master: 94a836dd46e5e041443b7da03e4ce8a7a7aaa7e3
Pushed to ipa-4-2: 61475631f64206d771e3fd243220be242f4bdd38

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] [PATCH 0400] l10n: Remove Transifex configuration

[Tomas Babej]

Hi,

We're not using Transifex to manage our translations anymore.

Tomas
From 89b2da7d936b6c8aad115e05375c4dcdf8af11c5 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 20 Jan 2016 19:44:25 +0100
Subject: [PATCH] l10n: Remove Transifex configuration

We're not using Transifex to manage our translations anymore.
---
 .tx/config | 8 
 1 file changed, 8 deletions(-)
 delete mode 100644 .tx/config

diff --git a/.tx/config b/.tx/config
deleted file mode 100644
index 75461a9d46209b55616008a24d2c4e22b958f678..
--- a/.tx/config
+++ /dev/null
@@ -1,8 +0,0 @@
-[main]
-host = https://www.transifex.com
-
-[freeipa.ipa]
-file_filter = install/po/.po
-source_file = install/po/ipa.pot
-source_lang = en
-
-- 
2.5.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

Re: [Freeipa-devel] [PATCH 0416-0419] fix broken configuration of sidgen and extdom plugins

[Tomas Babej]

On 02/23/2016 01:25 PM, Martin Basti wrote:
> 
> 
> On 23.02.2016 13:02, Alexander Bokovoy wrote:
>> On Tue, 23 Feb 2016, Martin Basti wrote:
>>> From f2ae1bd129a1741500d2f3dcb86a0da553604d15 Mon Sep 17 00:00:00 2001
>>> From: Martin Basti 
>>> Date: Tue, 23 Feb 2016 10:37:47 +0100
>>> Subject: [PATCH 4/4] fix upgrade: wait for proper DS socket after DS
>>> restart
>>>
>>> Restarting DS executed by upgrade plugin causes that upgrade frameworg
>>> was waiting for not proper socket to be ready. This commit fix issue.
>> Please fix the commit message typos.
>>
> Fixed. Updated patches attached.

ACK.

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

Re: [Freeipa-devel] [PATCH 0416-0417] fix broken configuration of sidgen and extdom plugins

[Tomas Babej]

On 02/22/2016 07:15 PM, Martin Basti wrote:
> 
> 
> On 22.02.2016 17:05, Martin Basti wrote:
>>
>>
>> On 19.02.2016 15:02, Alexander Bokovoy wrote:
>>> On Fri, 19 Feb 2016, Petr Vobornik wrote:
 On 02/19/2016 11:12 AM, Alexander Bokovoy wrote:
> On Fri, 19 Feb 2016, Martin Basti wrote:
>> WIP patch attached
>>
>> https://fedorahosted.org/freeipa/ticket/5665
>>
> Comments inline.
>
>> +# we need to run sidgen task
>> +sidgen_task_dn = DN("cn=sidgen,cn=ipa-sidgen-task,cn=tasks,"
>> +"cn=config")
>> +sidgen_tasks_attr = {
>> +"objectclass": ["top", "extensibleObject"],
>> +"cn": ["sidgen"],
>> +"delay": [0],
>> +"nsslapd-basedn": [self.api.env.basedn],
>> +}
> May be you are better to name this task more uniquely?
> Something like 'cn=generate domain sid,cn=...'?
>
>> +
>> +task_entry = ldap.make_entry(sidgen_task_dn,
>> + **sidgen_tasks_attr)
>> +try:
>> +ldap.add_entry(task_entry)
>> +except errors.DuplicateEntry:
>> +self.log.debug("sidgen task already created")
>> +else:
>> +self.log.debug("sidgen task has been created")
> There could be multiple tasks running in parallel, that's why it could
> be good to use a different and unique name.
>
>> +# we have to check all trusts domains which have been added
>> after the
>> +# upgrade that caused bug was done.
>> +
>> +base_dn = DN(self.api.env.container_adtrusts,
>> self.api.env.basedn)
>> +trust_domain_entries, truncated = ldap.find_entries(
>> +base_dn=base_dn,
>> +scope=ldap.SCOPE_ONELEVEL,
>> +attrs_list=[attr_name, "cn"],
>> +)
>> +
>> +if truncated:
>> +self.log.warning("update_sids: Search results were
>> truncated")
>> +
>> +for entry in trust_domain_entries:
>> +if entry.single_value[attr_name] is None:
>> +domain = entry.single_value["cn"]
>> +self.log.error(
>> +"Your trust to %s is broken. Please re-create it
>> by "
>> +"running 'ipa trust-add' again", domain)
>> +
>> +sysupgrade.set_upgrade_state('sidgen', 'update_sids', False)
>> +return False, ()
> This part looks fine. Basically, a similar check needs to be added to
> trust_find, trust_show, and may be trust_add.
>

 Why trust-add?

 I'm not a big fan of cluttering existing commands(find, show, mod)
 with logic to fix one upgrade bug. But I understand a need to
 communicate it somehow.

 Would it make sense to move such logic to a separate command, e.g.
 trust-check/trust-verify?  The command can do additional check in a
 future.
>>> No. What is the value of trust-add if it says you 'Trust established and
>>> verified' while in reality it didn't? This specific issue is very easy
>>> to catch.
>>>
>> Patch attached, patch with warning will land soon.
>>
>>
> Updated patches attached

During the RPM upgrade, the ipa-server-upgrade times out and leaves
directory server stopped.

Issues seem to be fixed after manual DS start, but we should get
the upgrade during RPM cleanup sorted out to have a seamless experience.

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

Re: [Freeipa-devel] [PATCH 200] slapi-nis: update configuration to allow external members

[Tomas Babej]

On 02/22/2016 06:14 PM, Alexander Bokovoy wrote:
> On Mon, 22 Feb 2016, Tomas Babej wrote:
>>
>>
>> On 02/22/2016 11:48 AM, Alexander Bokovoy wrote:
>>> Hi,
>>>
>>> attached patch should update compat tree configuration if it exist to
>>> follow slapi-nis 0.55 which has support for external members of IPA
>>> groups.
>>>
>>> However, the real work is done in SSSD. These patches are not upstreamed
>>> yet. We'll need to bump SSSD dependency in future once they come to
>>> distros.
>>>
>>>
>>>
>>
>> This looks good.
>>
>> However, the new update file needs to be added to Makefile.am.
>> Additionally, patch adds a whitespace error.
> Updated patch is attached.
> 

ACK.

This should not be pushed until the dependency for SSSD can be bumped.

-- 
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 200] slapi-nis: update configuration to allow external members

[Tomas Babej]

On 02/22/2016 11:48 AM, Alexander Bokovoy wrote:
> Hi,
> 
> attached patch should update compat tree configuration if it exist to
> follow slapi-nis 0.55 which has support for external members of IPA
> groups.
> 
> However, the real work is done in SSSD. These patches are not upstreamed
> yet. We'll need to bump SSSD dependency in future once they come to
> distros.
> 
> 
> 

This looks good.

However, the new update file needs to be added to Makefile.am.
Additionally, patch adds a whitespace error.

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

Re: [Freeipa-devel] [PATCH 0128] ipalib/cli.py: pythonify Collector class

[Tomas Babej]

On 01/28/2016 04:44 PM, Martin Babinsky wrote:
> On 01/28/2016 03:20 PM, Tomas Babej wrote:
>>
>>
>> On 01/27/2016 03:58 PM, Martin Babinsky wrote:
>>> On 01/18/2016 06:43 PM, Martin Babinsky wrote:
>>>> A little patch that should make some future pylint errors disappear.
>>>>
>>>>
>>>>
>>> Attaching updated patch that does not promote direct molestation of
>>> instance dictionaries.
>>>
>>>
>>>
>>
>> Patch looks good, one thing I am concerened about though is that
>> __todict__ now returns a direct reference to the internal, mutable dict,
>> and no longer a (shallow) copy.
>>
>> Maybe we should use dict.copy() there?
>>
>> Tomas
>>
> 
> Ah I didn't realize that. Fixed in updated patch.
> 

Nitpick: Sorry for being misleading - I did not mean to suggest invoking
the method using the dict type directly. While being equivalent, the

dict.copy(self.__options)

it's less idiomatic than:

self.__options.copy()

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

Re: [Freeipa-devel] [PATCH] 951 webui: fail nicely if cookies are disabled

[Tomas Babej]

On 01/28/2016 04:15 PM, Petr Vobornik wrote:
> Reworks also sessionStorage test because disablement of cookies might be
> connected with sessionStorage and localStorage. E.g. Chrome raises
> exception when *Storage is accessed with "Block sites from setting any
> data" settings set in "Content Settings/Cookies" section.
> 
> https://fedorahosted.org/freeipa/ticket/4338
> 
> 

Seems that two spaces inserted themselves to the error message for
localStorage :)

-- 
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 0128] ipalib/cli.py: pythonify Collector class

[Tomas Babej]

On 01/27/2016 03:58 PM, Martin Babinsky wrote:
> On 01/18/2016 06:43 PM, Martin Babinsky wrote:
>> A little patch that should make some future pylint errors disappear.
>>
>>
>>
> Attaching updated patch that does not promote direct molestation of
> instance dictionaries.
> 
> 
> 

Patch looks good, one thing I am concerened about though is that
__todict__ now returns a direct reference to the internal, mutable dict,
and no longer a (shallow) copy.

Maybe we should use dict.copy() there?

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

Re: [Freeipa-devel] [PATCH 0399] ipa-getkeytab: Handle the possibility of not obtaining a result

[Tomas Babej]

On 01/27/2016 05:03 PM, Martin Babinsky wrote:
> On 01/26/2016 05:48 PM, Tomas Babej wrote:
>> Hi,
>>
>> The ldap_result operation can time out, returning a NULL result,
>> which in turn causes the parsing operation to crash.
>>
>> https://fedorahosted.org/freeipa/ticket/5642
>>
>> Tomas
>>
>>
>>
> ACK
> 

Pushed to master: d53c2f6b806335507ffd5e78be42471b85a39bbf

-- 
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 540] cert renewal: import all external CA certs on IPA CA cert renewal

[Tomas Babej]

On 01/27/2016 08:06 AM, Martin Babinsky wrote:
> On 01/25/2016 08:19 AM, Jan Cholasta wrote:
>> On 22.1.2016 12:28, Jan Cholasta wrote:
>>> On 22.1.2016 10:34, Martin Babinsky wrote:
 On 01/21/2016 10:27 AM, Jan Cholasta wrote:
> Hi,
>
> the attached patch fixes
> .
>
> Honza
>
>
>
 ACK
>>>
>>> Self-NACK. Doesn't work with external CA install.
>>>
>>
>> Updated patches attached.
>>
> ACK
> 

Pushed to master: eaafeddf769c25bd44b490ae18ffb58e97df4963
Pushed to ipa-4-2: 2314fa66fd7fe543209292660d4f7f9611cdedb2

-- 
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 543] CA install: explicitly set dogtag_version to 10

[Tomas Babej]

On 01/27/2016 12:10 PM, Martin Babinsky wrote:
> On 01/27/2016 09:27 AM, Jan Cholasta wrote:
>> On 26.1.2016 10:23, Martin Babinsky wrote:
>>> On 01/26/2016 10:14 AM, Martin Babinsky wrote:
 On 01/25/2016 08:56 AM, Alexander Bokovoy wrote:
> On Mon, 25 Jan 2016, Jan Cholasta wrote:
>> Hi,
>>
>> the attached patch fixes
>> .
>>
>> Note that this is a 4.2-specific fix.
>>
>> Honza
>>
>> -- 
>> Jan Cholasta
>
>> From c2a0684c64538166809883a235bd131518b6e78f Mon Sep 17 00:00:00
>> 2001
>> From: Jan Cholasta 
>> Date: Mon, 25 Jan 2016 08:48:42 +0100
>> Subject: [PATCH] CA install: explicitly set dogtag_version to 10
>>
>> When installing new CA master, explicitly set the dogtag_version
>> option to
>> 10 in api.bootstrap() to prevent failures in code which expects the
>> value
>> to be 10 rather than the default value of 9.
>>
>> https://fedorahosted.org/freeipa/ticket/5611
>> ---
>> install/tools/ipa-ca-install | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/install/tools/ipa-ca-install
>> b/install/tools/ipa-ca-install
>> index 6564e4d..e8ccaef 100755
>> --- a/install/tools/ipa-ca-install
>> +++ b/install/tools/ipa-ca-install
>> @@ -162,7 +162,7 @@ def install_master(safe_options, options):
>>
>> # override ra_plugin setting read from default.conf so that we
>> have
>> # functional dogtag backend plugins during CA install
>> -api.bootstrap(in_server=True, ra_plugin='dogtag')
>> +api.bootstrap(in_server=True, ra_plugin='dogtag',
>> dogtag_version=10)
>> api.finalize()
>>
>> dm_password = options.password
>> -- 
> ACK.
>

 Not so fast, I have this patch applied on top of ipa-4-2 and it does
 not
 fix the crash described in the ticket.

>>>
>>> See the end of CA install log (http://fpaste.org/314777/14537999/), it
>>> seems that despite setting dogtag version to 10 in API initialization,
>>> CA instance still thinks it needs to work with version 9.
>>>
>>> It seems that dogtag.configured_constants() function is to blame:
>>>
>>> """
>>> In [4]: from ipalib import api
>>>
>>> In [5]: api.bootstrap(dogtag_version=10)
>>>
>>> In [6]: api.finalize()
>>>
>>> In [7]: dogtag.configured_constants()
>>> Out[7]: ipapython.dogtag.Dogtag9Constants
>>>
>>> In [8]: dogtag.configured_constants(api)
>>> Out[8]: ipapython.dogtag.Dogtag10Constants
>>> """
>>
>> Updated patch attached.
>>
> 
> ACK
> 

Pushed to ipa-4-2: 7c78a1f1a4637d0b736b976822646f51926aa214

-- 
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 540] cert renewal: import all external CA certs on IPA CA cert renewal

[Tomas Babej]

On 01/27/2016 02:53 PM, Jan Cholasta wrote:
> On 27.1.2016 14:41, Tomas Babej wrote:
>>
>>
>> On 01/27/2016 08:06 AM, Martin Babinsky wrote:
>>> On 01/25/2016 08:19 AM, Jan Cholasta wrote:
>>>> On 22.1.2016 12:28, Jan Cholasta wrote:
>>>>> On 22.1.2016 10:34, Martin Babinsky wrote:
>>>>>> On 01/21/2016 10:27 AM, Jan Cholasta wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> the attached patch fixes
>>>>>>> <https://fedorahosted.org/freeipa/ticket/5595>.
>>>>>>>
>>>>>>> Honza
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> ACK
>>>>>
>>>>> Self-NACK. Doesn't work with external CA install.
>>>>>
>>>>
>>>> Updated patches attached.
>>>>
>>> ACK
>>>
>>
>> Pushed to master: eaafeddf769c25bd44b490ae18ffb58e97df4963
>> Pushed to ipa-4-2: 2314fa66fd7fe543209292660d4f7f9611cdedb2
> 
> It seems you forgot ipa-4-3.
> 

Yep, thanks. Pushed to ipa-4-3: 659c5ae7e649c1f03ac9f93c1b5369f037811d7d

-- 
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] webui: 949 crash nicely if sessionStorage is not available

[Tomas Babej]

On 01/26/2016 06:59 PM, Tomas Babej wrote:
> ACK
> 
> On 01/26/2016 06:57 PM, Petr Vobornik wrote:
>> https://fedorahosted.org/freeipa/ticket/5643
>>
>>
> 

Pushed to master: 6e1eb5bc8f83faa38203bd308896d0b15f359b24

-- 
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 155] ipa-kdb: get_authz_data_types() make sure entry can be NULL

[Tomas Babej]

On 01/06/2016 12:15 PM, Sumit Bose wrote:
> Hi,
> 
> this patch fixes and issue found by Simo when he called
> get_authz_data_types() with the second argument being NULL.
> This function determines which type of authorization data should be
> added to the Kerberos ticket. There are global default and it is
> possible to configure this per service as well. The second argument is
> the data base entry of a service. If no service is given it makes sens
> to return the global defaults and most parts of get_authz_data_types()
> handle this case well and this patch fixes the remain issue and adds a
> test for this as well.
> 
> Please note that currently get_authz_data_types() is used in a code path
> where the service entry is expected to be not NULL and it turned out
> that in Simo's case it will be non-NULL as well. Nevertheless the patch
> makes the code more robust and makes the future use of
> get_authz_data_types() more safe.
> 
> bye,
> Sumit
> 
> 
> 

ACK, thanks.

-- 
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 155] ipa-kdb: get_authz_data_types() make sure entry can be NULL

[Tomas Babej]

On 01/27/2016 04:04 PM, Tomas Babej wrote:
> 
> 
> On 01/06/2016 12:15 PM, Sumit Bose wrote:
>> Hi,
>>
>> this patch fixes and issue found by Simo when he called
>> get_authz_data_types() with the second argument being NULL.
>> This function determines which type of authorization data should be
>> added to the Kerberos ticket. There are global default and it is
>> possible to configure this per service as well. The second argument is
>> the data base entry of a service. If no service is given it makes sens
>> to return the global defaults and most parts of get_authz_data_types()
>> handle this case well and this patch fixes the remain issue and adds a
>> test for this as well.
>>
>> Please note that currently get_authz_data_types() is used in a code path
>> where the service entry is expected to be not NULL and it turned out
>> that in Simo's case it will be non-NULL as well. Nevertheless the patch
>> makes the code more robust and makes the future use of
>> get_authz_data_types() more safe.
>>
>> bye,
>> Sumit
>>
>>
>>
> 
> ACK, thanks.
> 

It is worth noting I added a commit message for the patch, based on the
Sumit's summary here.

Pushed to master: 45b0148fcce3fded5cea52b6fadd50114358ba25

-- 
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 0399] ipa-getkeytab: Handle the possibility of not obtaining a result

[Tomas Babej]

Hi,

The ldap_result operation can time out, returning a NULL result,
which in turn causes the parsing operation to crash.

https://fedorahosted.org/freeipa/ticket/5642

Tomas
From 3b4482cdd5494890a220bfd935874fa84677524c Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Tue, 26 Jan 2016 17:32:29 +0100
Subject: [PATCH] ipa-getkeytab: Handle the possibility of not obtaining a
 result

The ldap_result operation can time out, returning a NULL result,
which in turn causes the parsing operation to crash.

https://fedorahosted.org/freeipa/ticket/5642
---
 ipa-client/ipa-getkeytab.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/ipa-client/ipa-getkeytab.c b/ipa-client/ipa-getkeytab.c
index 3592d9970412e91fa62b6ca8310a9511c4235656..d4925865393ea19705d5d9ae09847966402fd8f0 100644
--- a/ipa-client/ipa-getkeytab.c
+++ b/ipa-client/ipa-getkeytab.c
@@ -275,6 +275,10 @@ static int ipa_ldap_extended_op(LDAP *ld, const char *reqoid,
 fprintf(stderr, _("Failed to get result: %s\n"), ldap_err2string(ret));
 goto done;
 }
+else if (res == NULL) {
+fprintf(stderr, _("Timeout exceeded."));
+goto done;
+}
 
 ret = ldap_parse_extended_result(ld, res, , , 0);
 if (ret != LDAP_SUCCESS) {
-- 
2.5.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

Re: [Freeipa-devel] [PATCH] webui: 949 crash nicely if sessionStorage is not available

[Tomas Babej]

ACK

On 01/26/2016 06:57 PM, Petr Vobornik wrote:
> https://fedorahosted.org/freeipa/ticket/5643
> 
> 

-- 
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 0397] ipapython: Use custom datetime to LDAP generalized time

[Tomas Babej]

Hi,

For the dates older than 1900, Python is unable to convert the datetime
representation to string using strftime:

https://bugs.python.org/issue1777412

Work around the issue adding a custom method to convert the datetime
objects to LDAP generalized time strings.

https://fedorahosted.org/freeipa/ticket/5579

Tomas
From d746dd233c07b0dc81f539f502844a16e5cc97e2 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Fri, 15 Jan 2016 12:20:12 +0100
Subject: [PATCH] ipapython: Use custom datetime to LDAP generalized time
 converter

For the dates older than 1900, Python is unable to convert the datetime
representation to string using strftime:

https://bugs.python.org/issue1777412

Work around the issue adding a custom method to convert the datetime
objects to LDAP generalized time strings.

https://fedorahosted.org/freeipa/ticket/5579
---
 daemons/dnssec/ipa-ods-exporter  |  5 +
 ipalib/cli.py|  5 +++--
 ipalib/rpc.py|  6 +++---
 ipapython/ipaldap.py |  4 ++--
 ipapython/ipautil.py | 17 +
 ipaserver/install/ipa_otptoken_import.py |  4 ++--
 6 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/daemons/dnssec/ipa-ods-exporter b/daemons/dnssec/ipa-ods-exporter
index 2aa936040c373e366e7e15539ed6e3413aac7d55..b2df53dee0ecb8cc08fcde9c20e17f72588b18de 100755
--- a/daemons/dnssec/ipa-ods-exporter
+++ b/daemons/dnssec/ipa-ods-exporter
@@ -83,9 +83,6 @@ def dnskey_flags_to_text_set(flags):
 mask <<= 1
 return flags_set
 
-def datetime2ldap(dt):
-return dt.strftime(ipalib.constants.LDAP_GENERALIZED_TIME_FORMAT)
-
 def sql2datetime(sql_time):
 """Convert SQL date format from local time zone into UTC."""
 localtz = dateutil.tz.tzlocal()
@@ -276,7 +273,7 @@ def get_ods_keys(zone_name):
 
 key_data.update(sql2ldap_algorithm(row['algorithm']))
 key_id = "%s-%s-%s" % (key_type,
-   datetime2ldap(key_data['idnsSecKeyCreated']),
+   ipautil.datetime_to_ldap_gentime(key_data['idnsSecKeyCreated']),
row['HSMkey_id'])
 
 key_data.update(sql2ldap_keyid(row['HSMkey_id']))
diff --git a/ipalib/cli.py b/ipalib/cli.py
index 3b1b5a39371845d59bab07ac2fc32de598a469be..58fbf048fdda4278bec0846486837fd35a581526 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -56,11 +56,12 @@ from ipalib import plugable
 from ipalib.errors import (PublicError, CommandError, HelpError, InternalError,
NoSuchNamespaceError, ValidationError, NotFound,
NotConfiguredError, PromptFailed)
-from ipalib.constants import CLI_TAB, LDAP_GENERALIZED_TIME_FORMAT
+from ipalib.constants import CLI_TAB
 from ipalib.parameters import File, Str, Enum, Any, Flag
 from ipalib.text import _
 from ipalib import api  # pylint: disable=unused-import
 from ipapython.dnsutil import DNSName
+from ipapython import ipautil
 
 import datetime
 
@@ -169,7 +170,7 @@ class textui(backend.Backend):
 if type(value) is bytes:
 return base64.b64encode(value)
 elif type(value) is datetime.datetime:
-return value.strftime(LDAP_GENERALIZED_TIME_FORMAT)
+return ipautil.datetime_to_ldap_gentime(value)
 elif isinstance(value, DNSName):
 return unicode(value)
 else:
diff --git a/ipalib/rpc.py b/ipalib/rpc.py
index a165491adea5366a14a86d7c8bd6337e36fd1b44..a2ca7cb3374e28074332c8827ab51088cc83a5e7 100644
--- a/ipalib/rpc.py
+++ b/ipalib/rpc.py
@@ -185,7 +185,7 @@ def xml_wrap(value, version):
 if capabilities.client_has_capability(version, 'datetime_values'):
 return DateTime(value)
 else:
-return value.strftime(LDAP_GENERALIZED_TIME_FORMAT)
+return ipautil.datetime_to_ldap_gentime(value)
 
 if isinstance(value, DNSName):
 if capabilities.client_has_capability(version, 'dns_name_values'):
@@ -304,9 +304,9 @@ def json_encode_binary(val, version):
 return str(val)
 elif isinstance(val, datetime.datetime):
 if capabilities.client_has_capability(version, 'datetime_values'):
-return {'__datetime__': val.strftime(LDAP_GENERALIZED_TIME_FORMAT)}
+return {'__datetime__': ipautil.datetime_to_ldap_gentime(val)}
 else:
-return val.strftime(LDAP_GENERALIZED_TIME_FORMAT)
+return ipautil.datetime_to_ldap_gentime(val)
 elif isinstance(val, DNSName):
 if capabilities.client_has_capability(version, 'dns_name_values'):
 return {'__dns_name__': unicode(val)}
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 28bfcb5c2ee2140d38f17248fc9c90861cd251e4..d916fd62698a0ff6fe023357238ec33b5ae099b9 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -38,7 +38,7 @@ import six
 

Re: [Freeipa-devel] [PATCH 536] ipapython: remove default_encoding_utf8

[Tomas Babej]

On 01/12/2016 10:24 AM, Jan Cholasta wrote:
> On 6.1.2016 12:33, Christian Heimes wrote:
>> On 2016-01-05 11:30, Tomas Babej wrote:
>>>
>>>
>>> On 01/05/2016 08:54 AM, Jan Cholasta wrote:
>>>> Hi,
>>>>
>>>> the attached patch replaces the default_encoding_utf8 binary module
>>>> with
>>>> 2 lines of equivalent Python code.
>>>>
>>>> Honza
>>>>
>>>>
>>>>
>>>
>>> This looks fine to me, however, I wonder, why this approach was ever
>>> taken? The sys.setdefaultencoding is available in all versions of Python
>>> ever supported by FreeIPA.
>>>
>>> Is it possible we're missing something here? Or was this option simply
>>> overlooked?
>>
>> sys.setdefaultencoding() is not available unless you use a hack and
>> reload the sys module. The function is hidden for a very good reason. It
>> can and will break internal assumption as well as libraries in bad, hard
>> to detect ways. For example it wreaks havoc on hashing for dicts and
>> sets.
>>
>> The blog posting
>> https://anonbadger.wordpress.com/2015/06/16/why-sys-setdefaultencoding-will-break-code/
>>
>> explains the problem in much greater detail.
> 
> Tomáši, does this answer your question?
> 

Not really, I was more curious as to why the current, more complex
solution using the C extension was ever preferred over pure python version.

> Updated patch attached.

Patch works fine, ACK.

Pushed to master: 7e56b4bbd79d9d42af23babc7496dd15d85d28ea

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] [PATCH 0398] logger: Use warning instead of warn

[Tomas Babej]

Hi,

this should build up to another pylint-related patch Martin^2 has in works.

Tomas

From b5e445c1dfdd469a7b85ba418b910f2a85fa470f Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Fri, 15 Jan 2016 16:25:33 +0100
Subject: [PATCH] logger: Use warning instead of warn

---
 install/tools/ipa-httpd-kdcproxy | 10 +-
 ipa-client/ipaclient/ipadiscovery.py |  6 +++---
 ipalib/plugins/dns.py|  2 +-
 ipalib/plugins/migration.py  | 16 
 ipalib/plugins/passwd.py |  2 +-
 ipalib/plugins/permission.py |  4 ++--
 ipaserver/dcerpc.py  |  2 +-
 ipaserver/install/ipa_otptoken_import.py |  2 +-
 ipaserver/install/ipa_replica_prepare.py |  2 +-
 ipaserver/install/ipa_restore.py |  2 +-
 ipatests/pytest_plugins/integration.py   |  2 +-
 11 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/install/tools/ipa-httpd-kdcproxy b/install/tools/ipa-httpd-kdcproxy
index 5e9863f8bd82e1628030b0b767a6697ab2a1d7bd..5e67f61a6e2b3fe26532323d773bd502ac52f454 100755
--- a/install/tools/ipa-httpd-kdcproxy
+++ b/install/tools/ipa-httpd-kdcproxy
@@ -141,7 +141,7 @@ class KDCProxyConfig(object):
 try:
 valid = self.validate_symlink()
 except ConfigFileError as e:
-self.log.warn("Cannot enable KDC proxy: %s " % e)
+self.log.warning("Cannot enable KDC proxy: %s " % e)
 return False
 
 if valid:
@@ -149,7 +149,7 @@ class KDCProxyConfig(object):
 return True
 
 if not os.path.isfile(self.conf):
-self.log.warn("'%s' does not exist", self.conf)
+self.log.warning("'%s' does not exist", self.conf)
 return False
 
 # create the symbolic link
@@ -163,7 +163,7 @@ class KDCProxyConfig(object):
 try:
 valid = self.validate_symlink()
 except CheckError as e:
-self.log.warn("Cannot disable KDC proxy: %s " % e)
+self.log.warning("Cannot disable KDC proxy: %s " % e)
 return False
 
 if valid:
@@ -203,8 +203,8 @@ def main(debug=DEBUG, time_limit=TIME_LIMIT):
 api.log.info('KDC proxy disabled')
 return 0
 except CheckError as e:
-api.log.warn(str(e))
-api.log.warn('Disabling KDC proxy')
+api.log.warning(str(e))
+api.log.warning('Disabling KDC proxy')
 cfg.remove_symlink()
 return 0
 except Exception as e:
diff --git a/ipa-client/ipaclient/ipadiscovery.py b/ipa-client/ipaclient/ipadiscovery.py
index 2e51293c2db35d655b1d9936103a729bd61a60ae..45a71e190e56d33d51d37f16ae61a7b4c28df521 100644
--- a/ipa-client/ipaclient/ipadiscovery.py
+++ b/ipa-client/ipaclient/ipadiscovery.py
@@ -266,14 +266,14 @@ class IPADiscovery(object):
 # via DNS
 break
 elif ldapret[0] == NOT_IPA_SERVER:
-root_logger.warn(
+root_logger.warning(
'Skip %s: not an IPA server', server)
 elif ldapret[0] == NO_LDAP_SERVER:
-root_logger.warn(
+root_logger.warning(
'Skip %s: LDAP server is not responding, unable to verify if '
'this is an IPA server', server)
 else:
-root_logger.warn(
+root_logger.warning(
'Skip %s: cannot verify if this is an IPA server', server)
 
 # If one of LDAP servers checked rejects access (maybe anonymous
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 59cb0ea3982256e9d98b8216207514e28e229d03..55f2609d7081112df2adc909a9c928b50fccdfb1 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -3514,7 +3514,7 @@ class dnsrecord(LDAPObject):
 except dns.resolver.NoNameservers as e:
 # Do not raise exception if we have got SERVFAILs.
 # Maybe the user has created an invalid zone intentionally.
-self.log.warn('waiting for DNS answer {%s}: got {%s}; '
+self.log.warning('waiting for DNS answer {%s}: got {%s}; '
   'ignoring', ldap_rrset, type(e))
 continue
 
diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index 59c49fae5441531015a45532df07439daac35290..f19324f0e4e0511f9445aae9e9ac14347da4a03b 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -168,7 +168,7 @@ def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs
 # See if the gidNumber at least points to a valid group on the remote
 # server.
 if entry_attrs['gidnumber'][0] in invalid_gids:
-api.log.warn('GID number %s of migrated user %s does not point to a known group.' \
+api.log.warning(

Re: [Freeipa-devel] [PATCH 0398] logger: Use warning instead of warn

[Tomas Babej]

On 01/15/2016 05:12 PM, Martin Basti wrote:
> 
> 
> On 15.01.2016 16:27, Tomas Babej wrote:
>> Hi,
>>
>> this should build up to another pylint-related patch Martin^2 has in works.
>>
>> Tomas
>>
>>
>>
> NACK :)
> 
> * Module ipalib.plugins.dns
> ipalib/plugins/dns.py:3441: [E1101(no-member),
> dnsrecord.wait_for_modified_attr] Class 'log' has no 'warn' member)
> 

My regexp was too strict, it seems :)

Updated patch attached.
From 66bcfd88cf6020ecc7aca769a41cdf1dd747ae19 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Fri, 15 Jan 2016 16:25:33 +0100
Subject: [PATCH] logger: Use warning instead of warn

---
 install/tools/ipa-httpd-kdcproxy | 10 +-
 ipa-client/ipaclient/ipadiscovery.py |  6 +++---
 ipalib/plugins/dns.py|  4 ++--
 ipalib/plugins/migration.py  | 16 
 ipalib/plugins/passwd.py |  2 +-
 ipalib/plugins/permission.py |  4 ++--
 ipaserver/dcerpc.py  |  2 +-
 ipaserver/install/ipa_otptoken_import.py |  2 +-
 ipaserver/install/ipa_replica_prepare.py |  2 +-
 ipaserver/install/ipa_restore.py |  2 +-
 ipatests/pytest_plugins/integration.py   |  2 +-
 11 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/install/tools/ipa-httpd-kdcproxy b/install/tools/ipa-httpd-kdcproxy
index 5e9863f8bd82e1628030b0b767a6697ab2a1d7bd..5e67f61a6e2b3fe26532323d773bd502ac52f454 100755
--- a/install/tools/ipa-httpd-kdcproxy
+++ b/install/tools/ipa-httpd-kdcproxy
@@ -141,7 +141,7 @@ class KDCProxyConfig(object):
 try:
 valid = self.validate_symlink()
 except ConfigFileError as e:
-self.log.warn("Cannot enable KDC proxy: %s " % e)
+self.log.warning("Cannot enable KDC proxy: %s " % e)
 return False
 
 if valid:
@@ -149,7 +149,7 @@ class KDCProxyConfig(object):
 return True
 
 if not os.path.isfile(self.conf):
-self.log.warn("'%s' does not exist", self.conf)
+self.log.warning("'%s' does not exist", self.conf)
 return False
 
 # create the symbolic link
@@ -163,7 +163,7 @@ class KDCProxyConfig(object):
 try:
 valid = self.validate_symlink()
 except CheckError as e:
-self.log.warn("Cannot disable KDC proxy: %s " % e)
+self.log.warning("Cannot disable KDC proxy: %s " % e)
 return False
 
 if valid:
@@ -203,8 +203,8 @@ def main(debug=DEBUG, time_limit=TIME_LIMIT):
 api.log.info('KDC proxy disabled')
 return 0
 except CheckError as e:
-api.log.warn(str(e))
-api.log.warn('Disabling KDC proxy')
+api.log.warning(str(e))
+api.log.warning('Disabling KDC proxy')
 cfg.remove_symlink()
 return 0
 except Exception as e:
diff --git a/ipa-client/ipaclient/ipadiscovery.py b/ipa-client/ipaclient/ipadiscovery.py
index 2e51293c2db35d655b1d9936103a729bd61a60ae..45a71e190e56d33d51d37f16ae61a7b4c28df521 100644
--- a/ipa-client/ipaclient/ipadiscovery.py
+++ b/ipa-client/ipaclient/ipadiscovery.py
@@ -266,14 +266,14 @@ class IPADiscovery(object):
 # via DNS
 break
 elif ldapret[0] == NOT_IPA_SERVER:
-root_logger.warn(
+root_logger.warning(
'Skip %s: not an IPA server', server)
 elif ldapret[0] == NO_LDAP_SERVER:
-root_logger.warn(
+root_logger.warning(
'Skip %s: LDAP server is not responding, unable to verify if '
'this is an IPA server', server)
 else:
-root_logger.warn(
+root_logger.warning(
'Skip %s: cannot verify if this is an IPA server', server)
 
 # If one of LDAP servers checked rejects access (maybe anonymous
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 59cb0ea3982256e9d98b8216207514e28e229d03..3da44ef3c34f8e0540b6dcdc3465b913c11833c5 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -3438,7 +3438,7 @@ class dnsrecord(LDAPObject):
 
 while attempt < max_attempts:
 if attempt >= warn_attempts:
-log_fn = self.log.warn
+log_fn = self.log.warning
 attempt += 1
 try:
 dns_answer = resolver.query(dns_name, rdtype,
@@ -3514,7 +3514,7 @@ class dnsrecord(LDAPObject):
 except dns.resolver.NoNameservers as e:
 # Do not raise exception if we have got SERVFAILs.
 # Maybe the user has created an invalid zone intentionally.
-self.log.warn('waiting for DNS answer {%s}: got {%s}; '
+self.log

Re: [Freeipa-devel] import rpm causes failure during IPA caless install

[Tomas Babej]

On 01/08/2016 01:45 PM, Martin Basti wrote:
> Hello all,
> 
> fix for ticket https://fedorahosted.org/freeipa/ticket/5535
> requires to import rpm module
> 
> This import somehow breaks nsslib in IPA
> https://fedorahosted.org/freeipa/ticket/5572
> 
> 
> We have 2 ways how to fix it:
> 
> 1) move import rpm to body of methods (attached patch)
> We are not sure how stable is this solution.
> 
> 2) use solution with rpmdevtools proposed here:
> https://www.redhat.com/archives/freeipa-devel/2016-January/msg00092.html
> This should be rock stable but it needs many dependencies (rpm-python
> too, perl)
> 
> The second way looks safer, so I would like to reimplement it, do you
> all agree or do you have better idea?
> Feedback welcome, please ASAP.
> 
> Martin^2
> 
> 

I guess this needs to be fixed on the RPM side, so we should file a
ticket there, referencing ours. As we discussed, this is most likely
insufficient cleanup performed on their side.

As far as the fix goes, I'd leverage the external tool in this situation.

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

Re: [Freeipa-devel] import rpm causes failure during IPA caless install

[Tomas Babej]

On 01/08/2016 03:31 PM, Lukas Slebodnik wrote:
> On (08/01/16 14:14), Jan Cholasta wrote:
>> On 8.1.2016 14:09, Martin Basti wrote:
>>>
>>>
>>> On 08.01.2016 14:00, Martin Kosek wrote:
 On 01/08/2016 01:45 PM, Martin Basti wrote:
> Hello all,
>
> fix for ticket https://fedorahosted.org/freeipa/ticket/5535
> requires to import rpm module
>
> This import somehow breaks nsslib in IPA
> https://fedorahosted.org/freeipa/ticket/5572
>
>
> We have 2 ways how to fix it:
>
> 1) move import rpm to body of methods (attached patch)
> We are not sure how stable is this solution.
>
> 2) use solution with rpmdevtools proposed here:
> https://www.redhat.com/archives/freeipa-devel/2016-January/msg00092.html
> This should be rock stable but it needs many dependencies (rpm-python
> too, perl)
>
> The second way looks safer, so I would like to reimplement it, do you
> all agree
> or do you have better idea?
> Feedback welcome, please ASAP.
>
> Martin^2
 Since it's Friday, I invested 15 minutes to practice my C skills and
 use the
 python-cffi library to call rpm rpmvercmp library call directly
 (attached):

 $ python rpm.py 4.2.0-15.el7 4.2.0-15.el7_2.3
 4.2.0-15.el7 < 4.2.0-15.el7_2.3

 This would not introduce any additional dependency besides rpm-devel,
 right? :-)
>>
>> Not rpm-devel, but rpm-libs (you should dlopen "librpm.so.3").
>>
> CentOS 7 has librpm.so.3
> but fedora 23+ has librpm.so.7
> 
> So if it is possible it will be good to avoid using specific vertsion.
> 
> LS
> 

Yes.

I think it should be quite possible to not use specific version, the
interface signature itself is not likely to change.

Even if it did, the problem would be detected immediately with the most
basic of tests ()installation).

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

Re: [Freeipa-devel] [PATCH 0118] fix Py3 incompatible exception instantiation in replica install code

[Tomas Babej]

On 01/07/2016 05:56 PM, Martin Babinsky wrote:
> On 01/04/2016 09:02 AM, Martin Babinsky wrote:
>>
>>
>>
> I have created ticket to patch and added it to commit message:
> 
> https://fedorahosted.org/freeipa/ticket/5585
> 
> 
> 

ACK for these changes, however, there are additional occurrences in the
code base, attaching a patch.

Tomas
From 7475c1650e5cc5478a65166d853822b93419cd5e Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Fri, 8 Jan 2016 18:23:35 +0100
Subject: [PATCH] py3: Remove py3 incompatible exception handling

---
 doc/guide/guide.org| 4 ++--
 doc/guide/wsgi.py.txt  | 2 +-
 ipaserver/install/server/common.py | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/doc/guide/guide.org b/doc/guide/guide.org
index 55c172535007b71e95f804f1a171fa547cfdf032..6d181559f0af90e7be7089aa94ab4900fa4e90b5 100644
--- a/doc/guide/guide.org
+++ b/doc/guide/guide.org
@@ -752,9 +752,9 @@ def run(api):
 except KeyboardInterrupt:
 print ''
 api.log.info('operation aborted')
-except PublicError, e:
+except PublicError as e:
 error = e
-except Exception, e:
+except Exception as e:
 api.log.exception('%s: %s', e.__class__.__name__, str(e))
 error = InternalError()
 if error is not None:
diff --git a/doc/guide/wsgi.py.txt b/doc/guide/wsgi.py.txt
index eb64f3a8285495ac0131872c99ab05485587556b..8566a25a16baa8c43288eee8bc480ffbd6eadf0b 100644
--- a/doc/guide/wsgi.py.txt
+++ b/doc/guide/wsgi.py.txt
@@ -13,7 +13,7 @@ env._finalize_core(**dict(DEFAULT_CONFIG))
 api.bootstrap(context='server', debug=env.debug, log=None) (ref:wsgi-app-bootstrap)
 try:
 api.finalize() (ref:wsgi-app-finalize)
-except Exception, e:
+except Exception as e:
 api.log.error('Failed to start IPA: %s' % e)
 else:
 api.log.info('*** PROCESS START ***')
diff --git a/ipaserver/install/server/common.py b/ipaserver/install/server/common.py
index 637e5664348bf3b7f2e4f2a867b8ecb224ccf388..08980c60f59ce0599a9bbe1cf53dcb2eedf7808d 100644
--- a/ipaserver/install/server/common.py
+++ b/ipaserver/install/server/common.py
@@ -138,7 +138,7 @@ class BaseServerCA(common.Installable, core.Group, core.Composite):
 for rdn in dn:
 if rdn.attr.lower() not in VALID_SUBJECT_ATTRS:
 raise ValueError("invalid attribute: \"%s\"" % rdn.attr)
-except ValueError, e:
+except ValueError as e:
 raise ValueError("invalid subject base format: %s" % e)
 
 ca_signing_algorithm = Knob(
@@ -243,7 +243,7 @@ class BaseServerDNS(common.Installable, core.Group, core.Composite):
 encoding = 'utf-8'
 value = value.decode(encoding)
 bindinstance.validate_zonemgr_str(value)
-except ValueError, e:
+except ValueError as e:
 # FIXME we can do this in better way
 # https://fedorahosted.org/freeipa/ticket/4804
 # decode to proper stderr encoding
-- 
2.5.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

Re: [Freeipa-devel] [PATCH 536] ipapython: remove default_encoding_utf8

[Tomas Babej]

On 01/05/2016 08:54 AM, Jan Cholasta wrote:
> Hi,
> 
> the attached patch replaces the default_encoding_utf8 binary module with
> 2 lines of equivalent Python code.
> 
> Honza
> 
> 
> 

This looks fine to me, however, I wonder, why this approach was ever
taken? The sys.setdefaultencoding is available in all versions of Python
ever supported by FreeIPA.

Is it possible we're missing something here? Or was this option simply
overlooked?

Ccing Rob.

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

Re: [Freeipa-devel] [PATCHES 531-532] server install: redirect ipa-client-install output to standard output

[Tomas Babej]

On 12/14/2015 12:52 PM, Jan Cholasta wrote:
> Hi,
> 
> the attached patches fix .
> 
> Honza
> 
> 
> 

Shouldn't skip_output be also marked as incompatible with redirect_output?

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

Re: [Freeipa-devel] [PATCH] 0047 dogtaginstance: remove unused function 'check_inst'

[Tomas Babej]

On 12/14/2015 06:56 AM, Fraser Tweedale wrote:
> Just some drive-by cleanup of an unused function.
> 
> Cheers,
> Fraser
> 

ACK, thanks for the cleanup!

Pushed to master: 38861428e76c19107a03f07530e3724aee60a270

-- 
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 534] replica promotion: let ipa-client-install validate enrollment options

[Tomas Babej]

On 12/14/2015 03:37 PM, Jan Cholasta wrote:
> Hi,
> 
> the attached patch fixes .
> 
> Honza
> 
> 
> 

ACK, Pushed to master: 110e3dfc5401899ae0a54cc979ca0820e53cfa02

-- 
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 531-532] server install: redirect ipa-client-install output to standard output

[Tomas Babej]

On 12/14/2015 12:57 PM, Jan Cholasta wrote:
> On 14.12.2015 12:54, Tomas Babej wrote:
>>
>>
>> On 12/14/2015 12:52 PM, Jan Cholasta wrote:
>>> Hi,
>>>
>>> the attached patches fix <https://fedorahosted.org/freeipa/ticket/5527>.
>>>
>>> Honza
>>>
>>>
>>>
>>
>> Shouldn't skip_output be also marked as incompatible with
>> redirect_output?
> 
> Yes, it should.
> 

Otherwise the patch works fine, so conditional ACK here given that the
additional sanity check is implemented.

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

Re: [Freeipa-devel] [PATCH 533] replica promotion: notify user about ignoring client enrollment options

[Tomas Babej]

On 12/14/2015 02:02 PM, Jan Cholasta wrote:
> On 14.12.2015 13:41, Jan Cholasta wrote:
>> Hi,
>>
>> the attached patch fixes .
> 
> Self-NACK, updated patch attached.
> 
> 
> 

ACK, works fine.

-- 
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 0373] Upgrade: Fix IPA version comparison

[Tomas Babej]

On 12/14/2015 10:21 AM, Martin Basti wrote:
> 
> 
> On 14.12.2015 09:24, Martin Kosek wrote:
>> On 12/14/2015 07:21 AM, Jan Cholasta wrote:
>>> On 11.12.2015 19:01, Tomas Babej wrote:
>>>>
>>>> On 12/11/2015 09:36 AM, Martin Kosek wrote:
>>>>> On 12/10/2015 05:09 PM, Martin Basti wrote:
>>>>>>
>>>>>> On 10.12.2015 15:49, Tomas Babej wrote:
>>>>>>> On 12/10/2015 11:23 AM, Martin Basti wrote:
>>>>>>>> On 10.12.2015 09:13, Lukas Slebodnik wrote:
>>>>>>>>> On (09/12/15 19:22), Martin Basti wrote:
>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/5535
>>>>>>>>>>
>>>>>>>>>> Patch attached.
>>>>>>>>> >From 8ef93485d61e8732166fb0c5b6c4559209740f3e Mon Sep 17 00:00:00
>>>>>>>>> 2001
>>>>>>>>>> From: Martin Basti <mba...@redhat.com>
>>>>>>>>>> Date: Wed, 9 Dec 2015 18:53:35 +0100
>>>>>>>>>> Subject: [PATCH] Fix version comparison
>>>>>>>>>>
>>>>>>>>>> Use RPM library to compare vendor versions of IPA for redhat
>>>>>>>>>> platform
>>>>>>>>>>
>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/5535
>>>>>>>>>> ---
>>>>>>>>>> freeipa.spec.in |  2 ++
>>>>>>>>>> ipaplatform/redhat/tasks.py | 19 +++
>>>>>>>>>> 2 files changed, 21 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/freeipa.spec.in b/freeipa.spec.in
>>>>>>>>>> index
>>>>>>>>>> 9f82b3695fb10c4db65cc31278364b3b34e26098..09feba7b8324f5e645da3e8010de86b6c3ee5ab9
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 100644
>>>>>>>>>> --- a/freeipa.spec.in
>>>>>>>>>> +++ b/freeipa.spec.in
>>>>>>>>>> @@ -166,6 +166,8 @@ Requires: %{etc_systemd_dir}
>>>>>>>>>> Requires: gzip
>>>>>>>>>> Requires: python-gssapi >= 1.1.0
>>>>>>>>>> Requires: custodia
>>>>>>>>>> +Requires: rpm-python
>>>>>>>>>> +Requires: rpmdevtools
>>>>>>>>> Could you explain why do you need the 2nd package?
>>>>>>>>> It does not contains any python modules
>>>>>>>>> and I cannot see usage of any binary in this patch
>>>>>>>>>
>>>>>>>>> LS
>>>>>>>> Thanks for this catch, it is actually located in yum package, I
>>>>>>>> rather
>>>>>>>> copy stringToVersion function from there to IPA, to avoid
>>>>>>>> dependency
>>>>>>>> hell
>>>>>>>>
>>>>>>>> Updated patch attached.
>>>>>>>>
>>>>>>>>
>>>>>>> Looking good. The __cmp__ function, however, is not available in
>>>>>>> Python
>>>>>>> 3. As we will eventually support python3 in RHEL as well, maybe we
>>>>>>> should make sure even platform-dependent parts are python3
>>>>>>> compatible?
>>>>>>> For the future's sake.
>>>>>>>
>>>>>>> Tomas
>>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> python 3 compatible patch attached.
>>>>> I wonder why we have to add so much code here and reimplement RPM
>>>>> version checking if it is already provided by rpmdevtools:
>>>>>
>>>>> ~~~
>>>>> $ /usr/bin/rpmdev-vercmp ipa-4.2.0-15.el7 4.2.0-15.el7_2.3; echo $?
>>>>> WARNING: hyphen in release1: 4.2.0-15.el7
>>>>>
>>>>> rpmdev-vercmp  
>>>>> rpmdev-vercmp  
>>>>> rpmdev-vercmp # with no arguments, prompt
>>>>>
>>>>> Exit status is 0 if the EVR's are equal, 11 if EVR1 is newer, and
>>>>> 12 if
>>>>> EVR2
>>>>> is newer.  Other exit statuses indicate problems.
>>>>>
>>>>> ipa-4.2.0-15.el7 < 4.2.0-15.el7_2.3
>>>>> 12
>>>>> ~~~
>>>>>
>>>>> which could be directly called from __eq__ or __lt__, since we are in
>>>>> platform specific code anyway already.
>>>>>
>>>>> Martin
>>>> Imho we should generally prefer reaching out to a Python library rather
>>>> launching a subprocess to compare the versions, it's unnecessary
>>>> overhead.
>> I would not be too worried about miliseconds longer execution on a
>> function
>> called during RPM upgrade.
>>
>>>> That said, the main argument for the usage of rpm-python over
>>>> rpmdevtools I see is that rpm-python is very likely to be present on
>>>> the
>>>> system (i.e. it is yum's own dependency), while rpmdevtools will not be
>>>> present by default.
>>>>
>>>>   From the standpoint of trying to minimize the size of freeipa
>>>> installation (i.e recent wget -> curl migration), we should keep the
>>>> spirit here and do not introduce a dependency for a collection of
>>>> developer tools.
>>>>
>>>> /2 cents
>>> +1, also a single function is hardly "much code".
>> Ok then. If you all want to add yet another N-V-R parsing function in the
>> FreeIPA code, I can live with it (with raised eyebrows though).
> 
> Rebased patch attached.

I tested the patch, and it works fine - so conditional ACK from me for
the current iteration of the patch, given developer consensus which was
not reached yet.

There's a split of opinions (external binary camp vs. copy camp),
so we need to decide if we both camps are OK with proceeding.

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

Re: [Freeipa-devel] [PATCH 0378] Tests: fix always true assertion

[Tomas Babej]

On 12/14/2015 12:24 PM, Martin Basti wrote:
> Fixes:
> /usr/lib/python2.7/site-packages/ipatests/test_cmdline/test_ipagetkeytab.py:116:
> SyntaxWarning: assertion is always true, perhaps remove parentheses?
> 
> Patch attached.
> 
> 

Nice catch. ACK.

Pushed to master: e1cb802d15d07f80b4812c51e25eb4f7ea48d7c5

-- 
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 531-532] server install: redirect ipa-client-install output to standard output

[Tomas Babej]

On 12/14/2015 02:41 PM, Jan Cholasta wrote:
> On 14.12.2015 14:20, Tomas Babej wrote:
>>
>>
>> On 12/14/2015 12:57 PM, Jan Cholasta wrote:
>>> On 14.12.2015 12:54, Tomas Babej wrote:
>>>>
>>>>
>>>> On 12/14/2015 12:52 PM, Jan Cholasta wrote:
>>>>> Hi,
>>>>>
>>>>> the attached patches fix
>>>>> <https://fedorahosted.org/freeipa/ticket/5527>.
>>>>>
>>>>> Honza
>>>>>
>>>>>
>>>>>
>>>>
>>>> Shouldn't skip_output be also marked as incompatible with
>>>> redirect_output?
>>>
>>> Yes, it should.
>>>
>>
>> Otherwise the patch works fine, so conditional ACK here given that the
>> additional sanity check is implemented.
> 
> Updated patches attched.
> 

Thanks, I just removed the empty space in the 'print ()' call before
pushing.

ACK, Pushed to master: c856401478ce2f4fdd9cd7192afd18704f78e2e6

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

Re: [Freeipa-devel] [PATCH 0374-0375] Fix permissions on newly created directories

[Tomas Babej]

On 12/11/2015 07:19 PM, Martin Basti wrote:
> 
> 
> On 10.12.2015 15:18, Martin Basti wrote:
>> Hello,
>>
>> patch 0374 fixes the ticket, but I found more issues with directory
>> permission, I fixed them in 0375
>>
>> https://fedorahosted.org/freeipa/ticket/5520
>>
>> Patches attached.
> 
> Patches attached.


ACK, works as expected.

Pushed to master: 4272ba40ea909b1f783a6fada5b1eebb6efbdf93

-- 
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 533] replica promotion: notify user about ignoring client enrollment options

[Tomas Babej]

On 12/14/2015 02:19 PM, Tomas Babej wrote:
> 
> 
> On 12/14/2015 02:02 PM, Jan Cholasta wrote:
>> On 14.12.2015 13:41, Jan Cholasta wrote:
>>> Hi,
>>>
>>> the attached patch fixes <https://fedorahosted.org/freeipa/ticket/5530>.
>>
>> Self-NACK, updated patch attached.
>>
>>
>>
> 
> ACK, works fine.
> 

Pushed to master: d68613194b4a26239ebb689d7270cf6c2035afbd

-- 
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 0070] Makefile: disable parallel build

[Tomas Babej]

On 12/11/2015 09:35 AM, Petr Spacek wrote:
> Hello,
> 
> Makefile: disable parallel build
> 
> IPA build system cannot cope with parallel build anyway, so this patch
> disables parallel build explicitly so it does not blow up when user
> has -j specified in default MAKEOPTS.
> 
> 
> 

ACK.

Pushed to:
master: e650e5eda161a912d22ee2e6ebaf94df3a07d7ff
ipa-4-2: 58453bc25092d93f79aca8fa2bd3cc3a10bc6810

-- 
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 0117] ipa-client-install: create a temporary directory for ccache files

[Tomas Babej]

On 12/14/2015 05:31 PM, Martin Babinsky wrote:
> fixes https://fedorahosted.org/freeipa/ticket/5528

Works as expected, code-wise looks good.

Thanks for looking into this, ACK!

Pushed to master: 5886f87f974fa508047a21350c2e6e75a3001da6

-- 
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 0388] tests: Add hostmask detection for sudo rules validating

[Tomas Babej]

On 12/10/2015 09:05 AM, Lukas Slebodnik wrote:
> On (08/12/15 14:47), Tomas Babej wrote:
>>
>>
>> On 12/03/2015 04:33 PM, Tomas Babej wrote:
>>>
>>>
>>> On 12/03/2015 04:26 PM, Aleš Mareček wrote:
>>>> Hello,
>>>>
>>>> ACK for code
>>>> NACK for the placing "get_client_ip_with_hostmask" function to 
>>>> test_sudo.py (this function should be in some more general file)
>>>>
>>>
>>> What place would you propose? The task.py is not a good place, as this
>>> is not really a task.
>>>
>>> Nevertheless, I'd rather have it moved when an use case outside
>>> test_sudo.py actually arises. Right now it would lead to unnecessary
>>> cluttering.
>>>
>>> Tomas
>>>
>>
>> I re-discovered ipatests.test_integration.util (two years after I
>> created it :D) - which seemed ideal for this function.
>>
>> Updated patch attached.
>>
>> Tomas
> 
>>From 33552d6078d75ee99f9ec19ae143df5a61ba84a4 Mon Sep 17 00:00:00 2001
>> From: Tomas Babej <tba...@redhat.com>
>> Date: Wed, 2 Dec 2015 15:25:49 +0100
>> Subject: [PATCH] tests: Add hostmask detection for sudo rules validating on
>> hostmask
>>
>> IPA sudo tests worked under the assumption that the clients
>> that are executing the sudo commands have their IPs assigned
>> within 255.255.255.0 hostmask.
>>
>> Removes this (invalid) assumption and adds a
>> dynamic detection of the hostmask of the IPA client.
>>
>> https://fedorahosted.org/freeipa/ticket/5501
>> ---
>> ipatests/test_integration/test_sudo.py | 33 +++--
>> ipatests/test_integration/util.py  | 16 
>> 2 files changed, 43 insertions(+), 6 deletions(-)
>>
>> diff --git a/ipatests/test_integration/util.py 
>> b/ipatests/test_integration/util.py
>> index 
>> 1a1bb3fcc923c9f2721f0a4c1cb7a1ba2ccc2dd8..187f39e80e84af0eb4938fb19ac3d3c7c2280ed9
>>  100644
>> --- a/ipatests/test_integration/util.py
>> +++ b/ipatests/test_integration/util.py
>> @@ -58,3 +58,19 @@ def run_repeatedly(host, command, assert_zero_rc=True, 
>> test=None,
>>  .format(cmd=' '.join(command),
>>  times=timeout / time_step,
>>  timeout=timeout))
>> +
>> +
>> +def get_host_ip_with_hostmask(host):
>> +"""
>> +Detects the IP of the host including the hostmask.
>> +
>> +Returns None if the IP could not be detected.
>> +"""
>> +
>> +ip = host.ip
>> +result = host.run_command(['ip', 'addr'])
>> +full_ip_regex = r'(?P%s/\d{1,2}) ' % re.escape(ip)
>> +match = re.search(full_ip_regex, result.stdout_text)
> ./make-lint 
> * Module ipatests.test_integration.util
> ipatests/test_integration/util.py:72: [E0602(undefined-variable), 
> get_host_ip_with_hostmask] Undefined variable 're')
> ipatests/test_integration/util.py:73: [E0602(undefined-variable), 
> get_host_ip_with_hostmask] Undefined variable 're')
> ===
> Errors were found during the static code check.
> If you are certain that any of the reported errors are false positives, please
> mark them in the source code according to the pylint documentation.
> ===
> Makefile:124: recipe for target 'lint' failed
> 
> LS
> 

Nothing can break when moving a function, right? Missing import fixed.

Tomas
From c176ff1ab9ea1c56dc0c5d44bc490d925fad1497 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 2 Dec 2015 15:25:49 +0100
Subject: [PATCH] tests: Add hostmask detection for sudo rules validating on
 hostmask

IPA sudo tests worked under the assumption that the clients
that are executing the sudo commands have their IPs assigned
within 255.255.255.0 hostmask.

Removes this (invalid) assumption and adds a
dynamic detection of the hostmask of the IPA client.

https://fedorahosted.org/freeipa/ticket/5501
---
 ipatests/test_integration/test_sudo.py | 32 ++--
 ipatests/test_integration/util.py  | 17 +
 2 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/ipatests/test_integration/test_sudo.py b/ipatests/test_integration/test_sudo.py
index 1dd4c5d73c9fa4288af4fc2708aa3abd51407217..b1f31556a96180c3b30b2fcc03dd35b5cd994ff5 100644
--- a/ipatests/test_integration/test_sudo.py
+++ b/ipatests/test_integration/test_sudo.

Re: [Freeipa-devel] [PATCH 0071] replica: Fix ipa-replica-install with replica file (domain, level 0).

[Tomas Babej]

On 12/09/2015 08:31 AM, David Kupka wrote:
> On 08/12/15 16:33, Tomas Babej wrote:
>>
>>
>> On 12/08/2015 04:20 PM, Oleg Fayans wrote:
>>> ACK. The initial issue is fixed.
>>>
>>> On 12/08/2015 03:03 PM, David Kupka wrote:
>>>> https://fedorahosted.org/freeipa/ticket/5531
>>>>
>>>>
>>>
>>
>> Can we get some more love for the patch and provide at least a sentence
>> worth of commit message before pushing?
>>
>> It's not obvious from the title what the patch does, other than it fixes
>> things.
>>
>> Tomas
>>
> I believe it's pretty obvious from linked ticket and attached patch
> changing just 5 lines.
> But you're right verbosity in commit message could save time later.
> Patch with changed commit message attached.
> 

Yes, I'd rather avoid the need to go to the ticket when perusing the git
log.

ACK, thanks.

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

Re: [Freeipa-devel] [PATCH 0395] replicainstall: Make sure the enrollment state is preserved

[Tomas Babej]

On 12/11/2015 06:05 PM, Martin Basti wrote:
> 
> 
> On 11.12.2015 14:03, Tomas Babej wrote:
>>
>> On 12/10/2015 02:22 PM, Tomas Babej wrote:
>>>
>>> On 12/10/2015 02:18 PM, Tomas Babej wrote:
>>>> Hi,
>>>>
>>>> During the promote_check phase, the subsequent checks after the machine
>>>> is enrolled may cause the installation to abort, hence leaving it
>>>> enrolled even though it might not have been prior to the execution of
>>>> the ipa-replica-install command.
>>>>
>>>> Make sure that ipa-client-install --uninstall is called on the machine
>>>> that has not been enrolled before in case of failure during the
>>>> promote_check phase.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/5529
>>>>
>>> Self-NACK, installer is redundant for uninstall_client.
>>>
>>> Updated patch attached.
>>>
>>> Tomas
>>>
>>>
>>>
>> Attaching rebased version.
>>
>>
> ACK

Pushed to master: 90f7fa074ac10f907c7a300305e17e6de17bd29a

-- 
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 523-525] replica install: add remote connection check over API

[Tomas Babej]

On 12/11/2015 05:37 PM, Martin Basti wrote:
> 
> 
> On 11.12.2015 15:40, Jan Cholasta wrote:
>> On 11.12.2015 08:03, Jan Cholasta wrote:
>>> On 11.12.2015 07:08, Jan Cholasta wrote:
 On 10.12.2015 15:56, Martin Babinsky wrote:
> On 12/10/2015 09:48 AM, Jan Cholasta wrote:
>> On 9.12.2015 16:38, Jan Cholasta wrote:
>>> On 9.12.2015 14:52, Jan Cholasta wrote:
 On 9.12.2015 10:02, Jan Cholasta wrote:
> Hi,
>
> the attached patches fix
> .

 Note that this needs selinux-policy fix to work, so put SELinux
 into
 permissive mode for testing:
 .
>>>
>>> Updated patches attached.
>>
>> I screwed up a change in patch 524 and accidentally included a
>> chunk of
>> code in patch 525 that doesn't belong in it.
>>
>> Updated patches attached.
>>
>>
>>
>
> Patches work as expected and I was not able to find any functional
> problem.
>
> I have a question about the naming of the oddjob helper script: the
> one
> related to trusts is named 'com.redhat.idm.trust-fetch-domains',
> and the
> conncheck runner is named 'org.freeipa.server.conncheck'. I don't want
> to start another bikeshedding conversation but shouldn't we named them
> in a consistent fashion (either rename the first one in separate patch
> or rename the new helper to com.redhat.idm.server.conncheck)?
>
> I understand that as an upstream, we should go with the
> 'org.freeipa.*'
> convention, but having two helpers with different prefixes makes me
> sad.

 If you look at the larger picture, org.freeipa is the consistent name.
 It makes me sad as well, but mistakes should be corrected. This is
 similar to how we use PEP8 in new code, but do not fix it in old code
 just for the sake of fixing it.

>
> That is a nitpick though, it does not affect the overall functionality
> of the patches so ACK.

 Thanks for the review. The current patch 523 breaks the trusts oddjob
 with SELinux in enforcing mode, I will send an update which corrects
 that, until bug 1289930 is fixed.
>>>
>>> Updated patches attached.
>>
>> Rebased on top of current master.
>>
>>
>>
> Just question, should be any kinited user allowed to run conncheck via rpc?
> 
> Martin^2

I guess there's is little harm, any kinited user that was allowed to
access the machine could perform the conncheck even without these patches:

# ipa-replica-conncheck --master master.ipa.test -p ran...@ipa.test -w
ratarata -a -r IPA.TEST
Check connection from replica to remote master 'master.ipa.test':
   Directory Service: Unsecure port (389): OK
   Directory Service: Secure port (636): OK
   Kerberos KDC: TCP (88): OK
   Kerberos Kpasswd: TCP (464): OK
   HTTP Server: Unsecure port (80): OK
   HTTP Server: Secure port (443): OK

The following list of ports use UDP protocol and would need to be
checked manually:
   Kerberos KDC: UDP (88): SKIPPED
   Kerberos Kpasswd: UDP (464): SKIPPED

Connection from replica to master is OK.
Start listening on required ports for remote master check
Get credentials to log in to remote master
Check SSH connection to remote master
Execute check on remote master
Check connection from master to remote replica 'replica.ipa.test':
   Directory Service: Unsecure port (389): OK
   Directory Service: Secure port (636): OK
   Kerberos KDC: TCP (88): OK
   Kerberos KDC: UDP (88): OK
   Kerberos Kpasswd: TCP (464): OK
   Kerberos Kpasswd: UDP (464): OK
   HTTP Server: Unsecure port (80): OK
   HTTP Server: Secure port (443): OK

Connection from master to replica is OK.

-- 
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 0113] properly add ACIs to custodia container during IPA upgrade

[Tomas Babej]

On 12/11/2015 10:37 AM, David Kupka wrote:
> On 10/12/15 10:14, Martin Babinsky wrote:
>> On 12/08/2015 10:45 AM, Martin Babinsky wrote:
>>> fixes https://fedorahosted.org/freeipa/ticket/5524
>>>
>>>
>>>
>>
>> Attaching updated patch with simpler fix suggested by Jan.
>>
>>
>>
> Thanks for the patch. Works for me, ACK.
> 

I was also finally able to reproduce it on a clear machine.

Pushed to master: a66a2c5160dbc23cdeec55d17422812028939e16

-- 
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 0113] properly add ACIs to custodia container during IPA upgrade

[Tomas Babej]

On 12/11/2015 12:50 PM, Tomas Babej wrote:
> 
> 
> On 12/11/2015 10:37 AM, David Kupka wrote:
>> On 10/12/15 10:14, Martin Babinsky wrote:
>>> On 12/08/2015 10:45 AM, Martin Babinsky wrote:
>>>> fixes https://fedorahosted.org/freeipa/ticket/5524
>>>>
>>>>
>>>>
>>>
>>> Attaching updated patch with simpler fix suggested by Jan.
>>>
>>>
>>>
>> Thanks for the patch. Works for me, ACK.
>>
> 
> I was also finally able to reproduce it on a clear machine.
> 
> Pushed to master: a66a2c5160dbc23cdeec55d17422812028939e16
> 

Martin actually pushed this 30 minutes ago, actual commit hash is
e130d35687a05cb3d2dd8708b76e7745e337c0c0.

-- 
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 0395] replicainstall: Make sure the enrollment state is preserved

[Tomas Babej]

On 12/10/2015 02:22 PM, Tomas Babej wrote:
> 
> 
> On 12/10/2015 02:18 PM, Tomas Babej wrote:
>> Hi,
>>
>> During the promote_check phase, the subsequent checks after the machine
>> is enrolled may cause the installation to abort, hence leaving it
>> enrolled even though it might not have been prior to the execution of
>> the ipa-replica-install command.
>>
>> Make sure that ipa-client-install --uninstall is called on the machine
>> that has not been enrolled before in case of failure during the
>> promote_check phase.
>>
>> https://fedorahosted.org/freeipa/ticket/5529
>>
> 
> Self-NACK, installer is redundant for uninstall_client.
> 
> Updated patch attached.
> 
> Tomas
> 
> 
> 

Attaching rebased version.
From faaf1da99a8b04ff546d8df38e1c7e65177b35d7 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 10 Dec 2015 14:10:18 +0100
Subject: [PATCH] replicainstall: Make sure the enrollment state is preserved

During the promote_check phase, the subsequent checks after the machine
is enrolled may cause the installation to abort, hence leaving it
enrolled even though it might not have been prior to the execution of
the ipa-replica-install command.

Make sure that ipa-client-install --uninstall is called on the machine
that has not been enrolled before in case of failure during the
promote_check phase.

https://fedorahosted.org/freeipa/ticket/5529
---
 ipaserver/install/server/replicainstall.py | 32 ++
 1 file changed, 32 insertions(+)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 97f19c7fd4b156a3a3a9fd4b87d6ce297ad2983c..6733caad8b3b159cd3c354f6933dcce797fb94cf 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -385,6 +385,34 @@ def common_cleanup(func):
 return decorated
 
 
+def preserve_enrollment_state(func):
+"""
+Makes sure the machine is unenrollled if the decorated function
+failed.
+"""
+def decorated(installer):
+try:
+func(installer)
+except BaseException:
+if installer._enrollment_performed:
+uninstall_client()
+raise
+
+return decorated
+
+
+def uninstall_client():
+"""
+Attempts to unenroll the IPA client using the ipa-client-install utility.
+
+An unsuccessful attempt to uninstall is ignored (no exception raised).
+"""
+
+print("Removing client side components")
+ipautil.run([paths.IPA_CLIENT_INSTALL, "--unattended", "--uninstall"],
+raiseonerr=False)
+
+
 def promote_sssd(host_name):
 sssdconfig = SSSDConfig.SSSDConfig()
 sssdconfig.import_config()
@@ -790,6 +818,8 @@ def ensure_enrolled(installer):
 # Call client install script
 service.print_msg("Configuring client side components")
 try:
+installer._enrollment_performed = True
+
 args = [paths.IPA_CLIENT_INSTALL, "--unattended"]
 stdin = None
 
@@ -831,9 +861,11 @@ def ensure_enrolled(installer):
  "ipa-client-install returned: " + str(e))
 
 @common_cleanup
+@preserve_enrollment_state
 def promote_check(installer):
 options = installer
 
+installer._enrollment_performed = False
 installer._top_dir = tempfile.mkdtemp("ipa")
 
 tasks.check_selinux_status()
-- 
2.5.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

Re: [Freeipa-devel] [PATCH 0388] tests: Add hostmask detection for sudo rules validating

[Tomas Babej]

On 12/11/2015 02:01 PM, Lukas Slebodnik wrote:
> On (10/12/15 11:40), Tomas Babej wrote:
>> On 12/10/2015 09:05 AM, Lukas Slebodnik wrote:
>>> On (08/12/15 14:47), Tomas Babej wrote:
>>>>
>>>>
>>>> On 12/03/2015 04:33 PM, Tomas Babej wrote:
>>>>>
>>>>>
>>>>> On 12/03/2015 04:26 PM, Aleš Mareček wrote:
>>>>>> Hello,
>>>>>>
>>>>>> ACK for code
>>>>>> NACK for the placing "get_client_ip_with_hostmask" function to 
>>>>>> test_sudo.py (this function should be in some more general file)
>>>>>>
>>>>>
>>>>> What place would you propose? The task.py is not a good place, as this
>>>>> is not really a task.
>>>>>
>>>>> Nevertheless, I'd rather have it moved when an use case outside
>>>>> test_sudo.py actually arises. Right now it would lead to unnecessary
>>>>> cluttering.
>>>>>
>>>>> Tomas
>>>>>
>>>>
>>>> I re-discovered ipatests.test_integration.util (two years after I
>>>> created it :D) - which seemed ideal for this function.
>>>>
>>>> Updated patch attached.
>>>>
>>>> Tomas
>>>
>>> >From 33552d6078d75ee99f9ec19ae143df5a61ba84a4 Mon Sep 17 00:00:00 2001
>>>> From: Tomas Babej <tba...@redhat.com>
>>>> Date: Wed, 2 Dec 2015 15:25:49 +0100
>>>> Subject: [PATCH] tests: Add hostmask detection for sudo rules validating on
>>>> hostmask
>>>>
>>>> IPA sudo tests worked under the assumption that the clients
>>>> that are executing the sudo commands have their IPs assigned
>>>> within 255.255.255.0 hostmask.
>>>>
>>>> Removes this (invalid) assumption and adds a
>>>> dynamic detection of the hostmask of the IPA client.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/5501
>>>> ---
>>>> ipatests/test_integration/test_sudo.py | 33 
>>>> +++--
>>>> ipatests/test_integration/util.py  | 16 
>>>> 2 files changed, 43 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/ipatests/test_integration/util.py 
>>>> b/ipatests/test_integration/util.py
>>>> index 
>>>> 1a1bb3fcc923c9f2721f0a4c1cb7a1ba2ccc2dd8..187f39e80e84af0eb4938fb19ac3d3c7c2280ed9
>>>>  100644
>>>> --- a/ipatests/test_integration/util.py
>>>> +++ b/ipatests/test_integration/util.py
>>>> @@ -58,3 +58,19 @@ def run_repeatedly(host, command, assert_zero_rc=True, 
>>>> test=None,
>>>>  .format(cmd=' '.join(command),
>>>>  times=timeout / time_step,
>>>>  timeout=timeout))
>>>> +
>>>> +
>>>> +def get_host_ip_with_hostmask(host):
>>>> +"""
>>>> +Detects the IP of the host including the hostmask.
>>>> +
>>>> +Returns None if the IP could not be detected.
>>>> +"""
>>>> +
>>>> +ip = host.ip
>>>> +result = host.run_command(['ip', 'addr'])
>>>> +full_ip_regex = r'(?P%s/\d{1,2}) ' % re.escape(ip)
>>>> +match = re.search(full_ip_regex, result.stdout_text)
>>> ./make-lint 
>>> * Module ipatests.test_integration.util
>>> ipatests/test_integration/util.py:72: [E0602(undefined-variable), 
>>> get_host_ip_with_hostmask] Undefined variable 're')
>>> ipatests/test_integration/util.py:73: [E0602(undefined-variable), 
>>> get_host_ip_with_hostmask] Undefined variable 're')
>>> ===
>>> Errors were found during the static code check.
>>> If you are certain that any of the reported errors are false positives, 
>>> please
>>> mark them in the source code according to the pylint documentation.
>>> ===
>>> Makefile:124: recipe for target 'lint' failed
>>>
>>> LS
>>>
>>
>> Nothing can break when moving a function, right? Missing import fixed.
>>
>> Tomas
> 
>>From c176ff1ab9ea1c56dc0c5d44bc490d925fad1497 Mon Sep 17 00:00:00 2001
>> From: Tomas Babej <tba...@redhat.com>
>> Date: Wed, 2 Dec 2015 15:25:49 +0100
>> Subject: [PATCH] tests: Add hostmask detection for sudo rules validating on
>> hostmask
>>
>> IPA sudo tests worked under the assumption that the clients
>> that are executing the sudo commands have their IPs assigned
>> within 255.255.255.0 hostmask.
>>
>> Removes this (invalid) assumption and adds a
>> dynamic detection of the hostmask of the IPA client.
>>
>> https://fedorahosted.org/freeipa/ticket/5501
>> ---
>> ipatests/test_integration/test_sudo.py | 32 ++--
>> ipatests/test_integration/util.py  | 17 +
>> 2 files changed, 43 insertions(+), 6 deletions(-)
>>
> Thank you very much.
> 
> ACK
> 
> LS
> 

Pushed to:
master: a02f83ff9c6a1920cedebee69dc6857c3521f161
ipa-4-2: f676b122848f7d843e200484ac06568d71f2f4ef

-- 
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 523-525] replica install: add remote connection check over API

[Tomas Babej]

On 12/11/2015 03:40 PM, Jan Cholasta wrote:
> On 11.12.2015 08:03, Jan Cholasta wrote:
>> On 11.12.2015 07:08, Jan Cholasta wrote:
>>> On 10.12.2015 15:56, Martin Babinsky wrote:
 On 12/10/2015 09:48 AM, Jan Cholasta wrote:
> On 9.12.2015 16:38, Jan Cholasta wrote:
>> On 9.12.2015 14:52, Jan Cholasta wrote:
>>> On 9.12.2015 10:02, Jan Cholasta wrote:
 Hi,

 the attached patches fix
 .
>>>
>>> Note that this needs selinux-policy fix to work, so put SELinux into
>>> permissive mode for testing:
>>> .
>>
>> Updated patches attached.
>
> I screwed up a change in patch 524 and accidentally included a
> chunk of
> code in patch 525 that doesn't belong in it.
>
> Updated patches attached.
>
>
>

 Patches work as expected and I was not able to find any functional
 problem.

 I have a question about the naming of the oddjob helper script: the one
 related to trusts is named 'com.redhat.idm.trust-fetch-domains', and
 the
 conncheck runner is named 'org.freeipa.server.conncheck'. I don't want
 to start another bikeshedding conversation but shouldn't we named them
 in a consistent fashion (either rename the first one in separate patch
 or rename the new helper to com.redhat.idm.server.conncheck)?

 I understand that as an upstream, we should go with the 'org.freeipa.*'
 convention, but having two helpers with different prefixes makes me
 sad.
>>>
>>> If you look at the larger picture, org.freeipa is the consistent name.
>>> It makes me sad as well, but mistakes should be corrected. This is
>>> similar to how we use PEP8 in new code, but do not fix it in old code
>>> just for the sake of fixing it.
>>>

 That is a nitpick though, it does not affect the overall functionality
 of the patches so ACK.
>>>
>>> Thanks for the review. The current patch 523 breaks the trusts oddjob
>>> with SELinux in enforcing mode, I will send an update which corrects
>>> that, until bug 1289930 is fixed.
>>
>> Updated patches attached.
> 
> Rebased on top of current master.
> 
> 
> 

ACK from me too,
Pushed to master: 14a44ea47bf9a617019ebc91fbe272215c428d82

-- 
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 0373] Upgrade: Fix IPA version comparison

[Tomas Babej]

On 12/11/2015 09:36 AM, Martin Kosek wrote:
> On 12/10/2015 05:09 PM, Martin Basti wrote:
>>
>>
>> On 10.12.2015 15:49, Tomas Babej wrote:
>>>
>>> On 12/10/2015 11:23 AM, Martin Basti wrote:
>>>>
>>>> On 10.12.2015 09:13, Lukas Slebodnik wrote:
>>>>> On (09/12/15 19:22), Martin Basti wrote:
>>>>>> https://fedorahosted.org/freeipa/ticket/5535
>>>>>>
>>>>>> Patch attached.
>>>>> >From 8ef93485d61e8732166fb0c5b6c4559209740f3e Mon Sep 17 00:00:00
>>>>> 2001
>>>>>> From: Martin Basti <mba...@redhat.com>
>>>>>> Date: Wed, 9 Dec 2015 18:53:35 +0100
>>>>>> Subject: [PATCH] Fix version comparison
>>>>>>
>>>>>> Use RPM library to compare vendor versions of IPA for redhat platform
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/5535
>>>>>> ---
>>>>>> freeipa.spec.in |  2 ++
>>>>>> ipaplatform/redhat/tasks.py | 19 +++
>>>>>> 2 files changed, 21 insertions(+)
>>>>>>
>>>>>> diff --git a/freeipa.spec.in b/freeipa.spec.in
>>>>>> index
>>>>>> 9f82b3695fb10c4db65cc31278364b3b34e26098..09feba7b8324f5e645da3e8010de86b6c3ee5ab9
>>>>>>
>>>>>>
>>>>>> 100644
>>>>>> --- a/freeipa.spec.in
>>>>>> +++ b/freeipa.spec.in
>>>>>> @@ -166,6 +166,8 @@ Requires: %{etc_systemd_dir}
>>>>>> Requires: gzip
>>>>>> Requires: python-gssapi >= 1.1.0
>>>>>> Requires: custodia
>>>>>> +Requires: rpm-python
>>>>>> +Requires: rpmdevtools
>>>>> Could you explain why do you need the 2nd package?
>>>>> It does not contains any python modules
>>>>> and I cannot see usage of any binary in this patch
>>>>>
>>>>> LS
>>>> Thanks for this catch, it is actually located in yum package, I rather
>>>> copy stringToVersion function from there to IPA, to avoid dependency
>>>> hell
>>>>
>>>> Updated patch attached.
>>>>
>>>>
>>> Looking good. The __cmp__ function, however, is not available in Python
>>> 3. As we will eventually support python3 in RHEL as well, maybe we
>>> should make sure even platform-dependent parts are python3 compatible?
>>> For the future's sake.
>>>
>>> Tomas
>>>
>> Thanks,
>>
>> python 3 compatible patch attached.
> 
> I wonder why we have to add so much code here and reimplement RPM
> version checking if it is already provided by rpmdevtools:
> 
> ~~~
> $ /usr/bin/rpmdev-vercmp ipa-4.2.0-15.el7 4.2.0-15.el7_2.3; echo $?
> WARNING: hyphen in release1: 4.2.0-15.el7
> 
> rpmdev-vercmp  
> rpmdev-vercmp  
> rpmdev-vercmp # with no arguments, prompt
> 
> Exit status is 0 if the EVR's are equal, 11 if EVR1 is newer, and 12 if
> EVR2
> is newer.  Other exit statuses indicate problems.
> 
> ipa-4.2.0-15.el7 < 4.2.0-15.el7_2.3
> 12
> ~~~
> 
> which could be directly called from __eq__ or __lt__, since we are in
> platform specific code anyway already.
> 
> Martin

Imho we should generally prefer reaching out to a Python library rather
launching a subprocess to compare the versions, it's unnecessary overhead.

That said, the main argument for the usage of rpm-python over
rpmdevtools I see is that rpm-python is very likely to be present on the
system (i.e. it is yum's own dependency), while rpmdevtools will not be
present by default.

>From the standpoint of trying to minimize the size of freeipa
installation (i.e recent wget -> curl migration), we should keep the
spirit here and do not introduce a dependency for a collection of
developer tools.

/2 cents

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

Re: [Freeipa-devel] [PATCH 0395] replicainstall: Make sure the enrollment state is preserved

[Tomas Babej]

On 12/10/2015 02:18 PM, Tomas Babej wrote:
> Hi,
> 
> During the promote_check phase, the subsequent checks after the machine
> is enrolled may cause the installation to abort, hence leaving it
> enrolled even though it might not have been prior to the execution of
> the ipa-replica-install command.
> 
> Make sure that ipa-client-install --uninstall is called on the machine
> that has not been enrolled before in case of failure during the
> promote_check phase.
> 
> https://fedorahosted.org/freeipa/ticket/5529
> 

Self-NACK, installer is redundant for uninstall_client.

Updated patch attached.

Tomas
From b6e88996206aa6695be24449653f91894c329f3b Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 10 Dec 2015 14:10:18 +0100
Subject: [PATCH] replicainstall: Make sure the enrollment state is preserved

During the promote_check phase, the subsequent checks after the machine
is enrolled may cause the installation to abort, hence leaving it
enrolled even though it might not have been prior to the execution of
the ipa-replica-install command.

Make sure that ipa-client-install --uninstall is called on the machine
that has not been enrolled before in case of failure during the
promote_check phase.

https://fedorahosted.org/freeipa/ticket/5529
---
 ipaserver/install/server/replicainstall.py | 32 ++
 1 file changed, 32 insertions(+)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4443bfd437f5b291182f65dd2a1ad2afe0ff89bc..e92d50a56e8416eb14e0526c684d9558c9ff91d5 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -385,6 +385,34 @@ def common_cleanup(func):
 return decorated
 
 
+def preserve_enrollment_state(func):
+"""
+Makes sure the machine is unenrollled if the decorated function
+failed.
+"""
+def decorated(installer):
+try:
+func(installer)
+except BaseException:
+if installer._enrollment_performed:
+uninstall_client()
+raise
+
+return decorated
+
+
+def uninstall_client():
+"""
+Attempts to unenroll the IPA client using the ipa-client-install utility.
+
+An unsuccessful attempt to uninstall is ignored (no exception raised).
+"""
+
+print("Removing client side components")
+ipautil.run([paths.IPA_CLIENT_INSTALL, "--unattended", "--uninstall"],
+raiseonerr=False)
+
+
 def promote_sssd(host_name):
 sssdconfig = SSSDConfig.SSSDConfig()
 sssdconfig.import_config()
@@ -786,6 +814,8 @@ def ensure_enrolled(installer):
 # Call client install script
 service.print_msg("Configuring client side components")
 try:
+installer._enrollment_performed = True
+
 args = [paths.IPA_CLIENT_INSTALL, "--unattended"]
 if installer.domain_name:
 args.extend(["--domain", installer.domain_name])
@@ -821,9 +851,11 @@ def ensure_enrolled(installer):
  "ipa-client-install returned: " + str(e))
 
 @common_cleanup
+@preserve_enrollment_state
 def promote_check(installer):
 options = installer
 
+installer._enrollment_performed = False
 installer._top_dir = tempfile.mkdtemp("ipa")
 
 tasks.check_selinux_status()
-- 
2.5.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 0395] replicainstall: Make sure the enrollment state is preserved

[Tomas Babej]

Hi,

During the promote_check phase, the subsequent checks after the machine
is enrolled may cause the installation to abort, hence leaving it
enrolled even though it might not have been prior to the execution of
the ipa-replica-install command.

Make sure that ipa-client-install --uninstall is called on the machine
that has not been enrolled before in case of failure during the
promote_check phase.

https://fedorahosted.org/freeipa/ticket/5529
From 183cea1e3a7efd8574d6b74b9181485e6cf7d19b Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 10 Dec 2015 14:10:18 +0100
Subject: [PATCH] replicainstall: Make sure the enrollment state is preserved

During the promote_check phase, the subsequent checks after the machine
is enrolled may cause the installation to abort, hence leaving it
enrolled even though it might not have been prior to the execution of
the ipa-replica-install command.

Make sure that ipa-client-install --uninstall is called on the machine
that has not been enrolled before in case of failure during the
promote_check phase.

https://fedorahosted.org/freeipa/ticket/5529
---
 ipaserver/install/server/replicainstall.py | 32 ++
 1 file changed, 32 insertions(+)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4443bfd437f5b291182f65dd2a1ad2afe0ff89bc..70f3351618207e8bc8351690a0cf2571072d30bf 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -385,6 +385,34 @@ def common_cleanup(func):
 return decorated
 
 
+def preserve_enrollment_state(func):
+"""
+Makes sure the machine is unenrollled if the decorated function
+failed.
+"""
+def decorated(installer):
+try:
+func(installer)
+except BaseException:
+if installer._enrollment_performed:
+uninstall_client(installer)
+raise
+
+return decorated
+
+
+def uninstall_client(installer):
+"""
+Attempts to unenroll the IPA client using the ipa-client-install utility.
+
+An unsuccessful attempt to uninstall is ignored (no exception raised).
+"""
+
+print("Removing client side components")
+ipautil.run([paths.IPA_CLIENT_INSTALL, "--unattended", "--uninstall"],
+raiseonerr=False)
+
+
 def promote_sssd(host_name):
 sssdconfig = SSSDConfig.SSSDConfig()
 sssdconfig.import_config()
@@ -786,6 +814,8 @@ def ensure_enrolled(installer):
 # Call client install script
 service.print_msg("Configuring client side components")
 try:
+installer._enrollment_performed = True
+
 args = [paths.IPA_CLIENT_INSTALL, "--unattended"]
 if installer.domain_name:
 args.extend(["--domain", installer.domain_name])
@@ -821,9 +851,11 @@ def ensure_enrolled(installer):
  "ipa-client-install returned: " + str(e))
 
 @common_cleanup
+@preserve_enrollment_state
 def promote_check(installer):
 options = installer
 
+installer._enrollment_performed = False
 installer._top_dir = tempfile.mkdtemp("ipa")
 
 tasks.check_selinux_status()
-- 
2.5.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

Re: [Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison

[Tomas Babej]

On 12/10/2015 11:23 AM, Martin Basti wrote:
> 
> 
> On 10.12.2015 09:13, Lukas Slebodnik wrote:
>> On (09/12/15 19:22), Martin Basti wrote:
>>> https://fedorahosted.org/freeipa/ticket/5535
>>>
>>> Patch attached.
>> >From 8ef93485d61e8732166fb0c5b6c4559209740f3e Mon Sep 17 00:00:00 2001
>>> From: Martin Basti 
>>> Date: Wed, 9 Dec 2015 18:53:35 +0100
>>> Subject: [PATCH] Fix version comparison
>>>
>>> Use RPM library to compare vendor versions of IPA for redhat platform
>>>
>>> https://fedorahosted.org/freeipa/ticket/5535
>>> ---
>>> freeipa.spec.in |  2 ++
>>> ipaplatform/redhat/tasks.py | 19 +++
>>> 2 files changed, 21 insertions(+)
>>>
>>> diff --git a/freeipa.spec.in b/freeipa.spec.in
>>> index
>>> 9f82b3695fb10c4db65cc31278364b3b34e26098..09feba7b8324f5e645da3e8010de86b6c3ee5ab9
>>> 100644
>>> --- a/freeipa.spec.in
>>> +++ b/freeipa.spec.in
>>> @@ -166,6 +166,8 @@ Requires: %{etc_systemd_dir}
>>> Requires: gzip
>>> Requires: python-gssapi >= 1.1.0
>>> Requires: custodia
>>> +Requires: rpm-python
>>> +Requires: rpmdevtools
>> Could you explain why do you need the 2nd package?
>> It does not contains any python modules
>> and I cannot see usage of any binary in this patch
>>
>> LS
> Thanks for this catch, it is actually located in yum package, I rather
> copy stringToVersion function from there to IPA, to avoid dependency hell
> 
> Updated patch attached.
> 
> 

Looking good. The __cmp__ function, however, is not available in Python
3. As we will eventually support python3 in RHEL as well, maybe we
should make sure even platform-dependent parts are python3 compatible?
For the future's sake.

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

Re: [Freeipa-devel] [PATCH 0068] add missing /ipaplatform/constants.py to .gitignore

[Tomas Babej]

On 12/08/2015 01:26 PM, Tomas Babej wrote:
> 
> 
> On 12/08/2015 01:26 PM, Petr Spacek wrote:
>> Hello,
>>
>> add missing /ipaplatform/constants.py to .gitignore
>>
> 
> ACK.
> 

Pushed to master: 848912ae31d1549d5f6bed874cc6c4541bada6f4

-- 
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 0068] add missing /ipaplatform/constants.py to .gitignore

[Tomas Babej]

On 12/08/2015 01:26 PM, Petr Spacek wrote:
> Hello,
> 
> add missing /ipaplatform/constants.py to .gitignore
> 

ACK.

-- 
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 0394] topology: Make sure the old 'realm' topology suffix is not

[Tomas Babej]

Hi,

The old 'realm' topology suffix is no longer used, however, it was being
created on masters with version 4.2.3 and later. Make sure it's properly
removed.

Note that this is not the case for the 'ipaca' suffix, which was later
removed to 'ca'.

https://fedorahosted.org/freeipa/ticket/5526
From 4c60de6009140f389bc45a5649868f1fde938421 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Tue, 8 Dec 2015 13:34:15 +0100
Subject: [PATCH] topology: Make sure the old 'realm' topology suffix is not
 used

The old 'realm' topology suffix is no longer used, however, it was being
created on masters with version 4.2.3 and later. Make sure it's properly
removed.

Note that this is not the case for the 'ipaca' suffix, which was later
removed to 'ca'.

https://fedorahosted.org/freeipa/ticket/5526
---
 install/updates/20-replication.update | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/install/updates/20-replication.update b/install/updates/20-replication.update
index a471742532cf5954be1b76dbe4a6d908e4cefa2c..1543a04c917c386e93ed93dfd2767e0fde4685f5 100644
--- a/install/updates/20-replication.update
+++ b/install/updates/20-replication.update
@@ -31,6 +31,9 @@ add: nsDS5ReplicatedAttributeList: $EXCLUDES
 add: nsDS5ReplicatedAttributeListTotal: $TOTAL_EXCLUDES
 add: nsds5ReplicaStripAttrs: $STRIP_ATTRS
 
+# Remove old topology configuration area (unused)
+deleteentry: cn=realm,cn=topology,cn=ipa,cn=etc,$SUFFIX
+
 # add IPA realm managed suffix to master entry
 dn: cn=$FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX
 add: objectclass: ipaReplTopoManagedServer
-- 
2.5.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

Re: [Freeipa-devel] [PATCH 0388] tests: Add hostmask detection for sudo rules validating

[Tomas Babej]

On 12/03/2015 04:33 PM, Tomas Babej wrote:
> 
> 
> On 12/03/2015 04:26 PM, Aleš Mareček wrote:
>> Hello,
>>
>> ACK for code
>> NACK for the placing "get_client_ip_with_hostmask" function to test_sudo.py 
>> (this function should be in some more general file)
>>
> 
> What place would you propose? The task.py is not a good place, as this
> is not really a task.
> 
> Nevertheless, I'd rather have it moved when an use case outside
> test_sudo.py actually arises. Right now it would lead to unnecessary
> cluttering.
> 
> Tomas
> 

I re-discovered ipatests.test_integration.util (two years after I
created it :D) - which seemed ideal for this function.

Updated patch attached.

Tomas
From 33552d6078d75ee99f9ec19ae143df5a61ba84a4 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 2 Dec 2015 15:25:49 +0100
Subject: [PATCH] tests: Add hostmask detection for sudo rules validating on
 hostmask

IPA sudo tests worked under the assumption that the clients
that are executing the sudo commands have their IPs assigned
within 255.255.255.0 hostmask.

Removes this (invalid) assumption and adds a
dynamic detection of the hostmask of the IPA client.

https://fedorahosted.org/freeipa/ticket/5501
---
 ipatests/test_integration/test_sudo.py | 33 +++--
 ipatests/test_integration/util.py  | 16 
 2 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/ipatests/test_integration/test_sudo.py b/ipatests/test_integration/test_sudo.py
index 1dd4c5d73c9fa4288af4fc2708aa3abd51407217..fe0a7a3ea5f230053e8c00d21b36ee300acce4c7 100644
--- a/ipatests/test_integration/test_sudo.py
+++ b/ipatests/test_integration/test_sudo.py
@@ -17,8 +17,12 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import pytest
+import re
+
 from ipatests.test_integration.base import IntegrationTest
 from ipatests.test_integration.tasks import clear_sssd_cache
+from ipatests.test_integration import util
 
 
 class TestSudo(IntegrationTest):
@@ -269,13 +273,25 @@ class TestSudo(IntegrationTest):
  '--hostgroups', 'testhostgroup'])
 
 def test_sudo_rule_restricted_to_one_hostmask_setup(self):
-# Add the client's /24 hostmask to the rule
-ip = self.client.ip
+# We need to detect the hostmask first
+full_ip = util.get_host_ip_with_hostmask(self.client)
+
+# Make a note for the next test, which needs to be skipped
+# if hostmask detection failed
+self.__class__.skip_hostmask_based = False
+
+if not full_ip:
+self.__class__.skip_hostmask_based = True
+raise pytest.skip("Hostmask could not be detected")
+
 self.master.run_command(['ipa', '-n', 'sudorule-add-host',
  'testrule',
- '--hostmask', '%s/24' % ip])
+ '--hostmask', full_ip])
 
 def test_sudo_rule_restricted_to_one_hostmask(self):
+if self.__class__.skip_hostmask_based:
+raise pytest.skip("Hostmask could not be detected")
+
 result1 = self.list_sudo_commands("testuser1")
 assert "(ALL : ALL) NOPASSWD: ALL" in result1.stdout_text
 
@@ -284,11 +300,16 @@ class TestSudo(IntegrationTest):
 assert result.returncode != 0
 
 def test_sudo_rule_restricted_to_one_hostmask_teardown(self):
-# Remove the client's /24 hostmask from the rule
-ip = self.client.ip
+if self.__class__.skip_hostmask_based:
+raise pytest.skip("Hostmask could not be detected")
+
+# Detect the hostmask first to delete the hostmask based rule
+full_ip = util.get_host_ip_with_hostmask(self.client)
+
+# Remove the client's hostmask from the rule
 self.master.run_command(['ipa', '-n', 'sudorule-remove-host',
  'testrule',
- '--hostmask', '%s/24' % ip])
+ '--hostmask', full_ip])
 
 def test_sudo_rule_restricted_to_one_hostmask_negative_setup(self):
 # Add the master's hostmask to the rule
diff --git a/ipatests/test_integration/util.py b/ipatests/test_integration/util.py
index 1a1bb3fcc923c9f2721f0a4c1cb7a1ba2ccc2dd8..187f39e80e84af0eb4938fb19ac3d3c7c2280ed9 100644
--- a/ipatests/test_integration/util.py
+++ b/ipatests/test_integration/util.py
@@ -58,3 +58,19 @@ def run_repeatedly(host, command, assert_zero_rc=True, test=None,
  .format(cmd=' '.join(command),
  times=timeout / time_step,
  timeout=timeout))
+
+
+def get_host_ip_with_hostmask(host):
+"""
+Detects the IP

Re: [Freeipa-devel] [PATCH 0394] topology: Make sure the old 'realm' topology suffix is not

[Tomas Babej]

On 12/08/2015 02:28 PM, Tomas Babej wrote:
> Hi,
> 
> The old 'realm' topology suffix is no longer used, however, it was being
> created on masters with version 4.2.3 and later. Make sure it's properly
> removed.
> 
> Note that this is not the case for the 'ipaca' suffix, which was later
> removed to 'ca'.
> 
> https://fedorahosted.org/freeipa/ticket/5526
> 

Actually, we found out with Martin that this patch deletes realm instead
of domain suffix, against all initial impressions.

Updated patch attached.

Tomas
From 669f741f8cc20772b84f5980b9b6b57f71e3b992 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Tue, 8 Dec 2015 13:34:15 +0100
Subject: [PATCH] topology: Make sure the old 'realm' topology suffix is not
 used

The old 'realm' topology suffix is no longer used, howver, it was being
created on masters with version 4.2.3 and later. Make sure it's properly
removed.

Note that this is not the case for the 'ipaca' suffix, whic was later
removed to 'ca'.

https://fedorahosted.org/freeipa/ticket/5526
---
 install/updates/20-replication.update | 4 
 1 file changed, 4 insertions(+)

diff --git a/install/updates/20-replication.update b/install/updates/20-replication.update
index a471742532cf5954be1b76dbe4a6d908e4cefa2c..c9d96066d5f9bec5b8b92a3f2c457636c095137a 100644
--- a/install/updates/20-replication.update
+++ b/install/updates/20-replication.update
@@ -31,6 +31,10 @@ add: nsDS5ReplicatedAttributeList: $EXCLUDES
 add: nsDS5ReplicatedAttributeListTotal: $TOTAL_EXCLUDES
 add: nsds5ReplicaStripAttrs: $STRIP_ATTRS
 
+# Remove old topology configuration area (unused)
+dn: cn=realm,cn=topology,cn=ipa,cn=etc,$SUFFIX
+deleteentry: cn=realm,cn=topology,cn=ipa,cn=etc,$SUFFIX
+
 # add IPA realm managed suffix to master entry
 dn: cn=$FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX
 add: objectclass: ipaReplTopoManagedServer
-- 
2.5.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

Re: [Freeipa-devel] [PATCH 0071] replica: Fix ipa-replica-install with replica file (domain, level 0).

[Tomas Babej]

On 12/08/2015 04:20 PM, Oleg Fayans wrote:
> ACK. The initial issue is fixed.
> 
> On 12/08/2015 03:03 PM, David Kupka wrote:
>> https://fedorahosted.org/freeipa/ticket/5531
>>
>>
> 

Can we get some more love for the patch and provide at least a sentence
worth of commit message before pushing?

It's not obvious from the title what the patch does, other than it fixes
things.

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

Re: [Freeipa-devel] You cannot specify '--admin-password' option(s) with replica file

[Tomas Babej]

On 12/07/2015 10:36 AM, Oleg Fayans wrote:
> This is an error message that I received at the attempt to install
> replica with the following command:
> 
> ipa-replica-install --setup-ca -p  -w 
> /var/lib/ipa/replica-info-replica2.justfor.test.gpg
> 
> However, if I remove the '-w ', then I get the password
> prompt for admin password interactively. The domain level is 0. The
> packages are built last Friday from upstream code.
> 

This is a legitimate issue, can you file a ticket?

Thanks,
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

Re: [Freeipa-devel] [PATCH 0393] replicainstall: Admin password should not conflict with

[Tomas Babej]

On 12/07/2015 02:33 PM, Tomas Babej wrote:
> Hi,
> 
> The --admin-password (-w) has its use both in domain level 0 and 1.
> 
> https://fedorahosted.org/freeipa/ticket/5517
> 
> 
> 

ACK.
Pushed to master: dcb6626e870bcededb62d801720721d5d6c9795f

-- 
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 0393] replicainstall: Admin password should not conflict with

[Tomas Babej]

Hi,

The --admin-password (-w) has its use both in domain level 0 and 1.

https://fedorahosted.org/freeipa/ticket/5517
From 9f5a6c6b257955ccad03840090d1b8fd2463bf6d Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Mon, 7 Dec 2015 14:32:11 +0100
Subject: [PATCH] replicainstall: Admin password should not conflict with
 replica file

The --admin-password (-w) has its use both in domain level 0 and 1.

https://fedorahosted.org/freeipa/ticket/5517
---
 ipaserver/install/server/replicainstall.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4ab40256d15bbd534e910c0ca008bb79a15b268b..683d157c78e876dc690d20139f1b5e27ebe20c36 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1377,7 +1377,6 @@ class Replica(BaseServer):
 CLIKnob(self.domain_name, '--domain'),
 CLIKnob(self.host_name, '--hostname'),
 CLIKnob(self.server, '--server'),
-CLIKnob(self.admin_password, '--admin-password'),
 CLIKnob(self.principal, '--principal'),
 )
 
-- 
2.5.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

Re: [Freeipa-devel] [PATCH 0392] tests: Fix incorrect uninstall method invocation

[Tomas Babej]

On 12/07/2015 10:58 AM, Tomas Babej wrote:
> Hi,
> 
> this fixes: https://fedorahosted.org/freeipa/ticket/5516
> 
> Tomas
> 

Pushed under oneliner rule:
master: 5cb003f0b4b85dce47499f594c410b34b5c961e2
ipa-4-2: e5189ef6e23e4691f6c74541da5bc1a0b0f2e73f

-- 
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 0392] tests: Fix incorrect uninstall method invocation

[Tomas Babej]

Hi,

this fixes: https://fedorahosted.org/freeipa/ticket/5516

Tomas
From efd1304be61c792c23c8e8560db6508c63fdd5e6 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Sat, 5 Dec 2015 16:54:04 +0100
Subject: [PATCH] tests: Fix incorrect uninstall method invocation

https://fedorahosted.org/freeipa/ticket/5516
---
 ipatests/test_integration/test_caless.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index 9cfba3ee29114badf5a703ccc1d47a1d3e0c41b7..4dda79bf9baac65aeab97ed1a4ac2786bf3102a2 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -118,7 +118,7 @@ class CALessBase(IntegrationTest):
 '-n', 'External CA cert'],
raiseonerr=False)
 
-super(CALessBase, cls).uninstall()
+super(CALessBase, cls).uninstall(mh)
 
 @classmethod
 def install_server(cls, host=None,
-- 
2.5.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

Re: [Freeipa-devel] [PATCH 0391] replicainstall: Add check for domain if server is specified

[Tomas Babej]

On 12/04/2015 08:22 PM, Rob Crittenden wrote:
> Martin Kosek wrote:
>> On 12/04/2015 07:17 PM, Tomas Babej wrote:
>>> Hi,
>>>
>>> Avoids failing in the later stages during the ipa-client-install
>>> command.
>>>
>>> Tomas
>>
>> Is this change needed? Wouldn't it be better to update
>> ipa-client-install or ipa-replica-install to not require the --domain
>> option? I would hope that --domain can be figured out during
>> installation and not passed to ipa-replica-install manually by the admin.
>>
>> I just think that calling
>> # ipa-replica-install --server=master.example.com
>> is better than
>> # ipa-replica-install --server=master.example.com --domain example.com
>> if possible.
> 
> IIRC this is for service discovery when using a specific server and not
> LDAP. This is the domain used to search for the kerberos realm, for
> example.
> 
> That isn't to say this isn't discoverable but it would require another
> function in discovery to query what the IPA domain is from the given
> master but it gets tricky if anonymous search is disabled, for example.
> 
> rob
> 

Needed or not, this is the behaviour that ipa-client-install has now.
Adding a domain detection method would be a RFE for ipa-client-install
(and imho not something we should be adding at this point).

This patch only focuses on making the ipa-replica-install work more
smoothly.

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

Re: [Freeipa-devel] [PATCH] 940 Update ipa-(cs)replica-manage man pages

[Tomas Babej]

On 12/04/2015 12:09 PM, Petr Vobornik wrote:
> On 12/03/2015 04:58 PM, Petr Vobornik wrote:
>> SSIA
>>
> 
> Updated patch attached.
> 
> 

ACK, Pushed to master: 95d659b634b2ea13d18d26cacbd73e19972145f2

-- 
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 0390] man: Update the ipa-replica-install manpage with promotion

[Tomas Babej]

On 12/04/2015 12:08 PM, Petr Vobornik wrote:
> On 12/03/2015 12:54 PM, Petr Vobornik wrote:
>> On 12/03/2015 12:06 PM, Tomas Babej wrote:
>>> Hi,
>>>
>>> this patch updates the man page for the ipa-replica-install given the
>>> latest changes (including the Jan's OTP patch).
>>>
>>> Tomas
>>>
>>
>>
>> Questions/suggestions:
>>
>> 1. "you cannot provide an replica file"
>>
>> It's true, but wouldn't it be better to say that "you don't need to". Or
>> communicate that it is a good thing. Cannot sounds too negative to me.
>>
>> 2. Why options: --password, --admin-password are in 'BASIC OPTIONS'
>> section and not also in new 'ENROLLMENT OPTIONS' section together with
>> --principal, --keytab, etc..? Maybe 'ENROLLMENT OPTIONS' should be
>> before "BASIC OPTIONS" and "BASIC OPTIONS" renamed.
>>
>>
>> General ideas, not related to this patch:
>> """
>> If the installation fails you may need to run ipa-server-install
>> --uninstall and ipa-client-install before running ipa-replica-install
>> again.
>>
>> The installation will fail if the host you are installing the
>> replica on exists as a host in IPA or an existing replication agreement
>> exists (for example, from a previously failed installation)
>> """
>>
>> I believe validators check this situation, right? So do we need to write
>> it in the man page.
> 
> 
> Attaching updated patch which reflects off-line cooperation with Tomas.
> 
> New option sections were added:
> DOMAIN LEVEL 1 OPTIONS
> DOMAIN LEVEL 1 CLIENT ENROLLMENT OPTIONS
> DOMAIN LEVEL 0 OPTIONS

ACK, Pushed to master: bb7934e3bc91155f21b45b5d5186bfda76a21f15.

-- 
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 0391] replicainstall: Add check for domain if server is specified

[Tomas Babej]

Hi,

Avoids failing in the later stages during the ipa-client-install
command.

Tomas
From 477a9a197524ff39373f5e58cf7c7ee173657c91 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Fri, 4 Dec 2015 19:13:07 +0100
Subject: [PATCH] replicainstall: Add check for domain if server is specified

Avoids failing in the later stages during the ipa-client-install
command.
---
 ipaserver/install/server/replicainstall.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index ec77ab21b1e4969bdcd8d9e588eed7b97e3a9079..4ab40256d15bbd534e910c0ca008bb79a15b268b 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1353,6 +1353,12 @@ class Replica(BaseServer):
 raise RuntimeError("--dirsrv-cert-file and --http-cert-file "
"are required if any PKCS#12 options are "
"used")
+
+if self.server and not self.domain_name:
+raise RuntimeError("The --server option cannot be used "
+   "without providing domain via the --domain "
+   "option")
+
 else:
 if not ipautil.file_exists(self.replica_file):
 raise RuntimeError("Replica file %s does not exist"
-- 
2.5.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

Re: [Freeipa-devel] [PATCH] 941 Extend topology help

[Tomas Babej]

On 12/04/2015 06:58 PM, Tomas Babej wrote:
> 
> 
> On 12/03/2015 04:58 PM, Petr Vobornik wrote:
>> `ipa help topology` is improved.
>>
>>
> 
> Looks good. I changed one part of the documentation for more clarity,
> see the attached patch.
> 
> Otherwise ACK from me.
> 
> Tomas
> 
> 
> 

Fixed a typo.

s/the/that

Tomas
From 80b12c6198fc3c845ddeb8d361a65dbb59fb09aa Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Thu, 3 Dec 2015 16:29:27 +0100
Subject: [PATCH] Extend topology help

`ipa help topology` is improved.
---
 ipalib/plugins/topology.py | 53 --
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugins/topology.py b/ipalib/plugins/topology.py
index 40f9fa8038d4bcb6c0eeb82a37ac796d732b82fd..128a347484939be3e1f1ce59a2fcd8d1fcedb3e4 100644
--- a/ipalib/plugins/topology.py
+++ b/ipalib/plugins/topology.py
@@ -22,9 +22,58 @@ if six.PY3:
 __doc__ = _("""
 Topology
 
-Management of a replication topology.
+Management of a replication topology at domain level 1.
+""") + _("""
+IPA server's data is stored in LDAP server in two suffixes:
+* domain suffix, e.g., 'dc=example,dc=com', contains all domain related data
+* ca suffix, 'o=ipaca', is present only on server with CA installed. It
+  contains data for Certificate Server component
+""") + _("""
+Data stored on IPA servers is replicated to other IPA servers. The way it is
+replicated is defined by replication agreements. Replication agreements needs
+to be set for both suffixes separately. On domain level 0 they are managed
+using ipa-replica-manage and ipa-csreplica-manage tools. With domain level 1
+they are managed centrally using `ipa topology*` commands.
+""") + _("""
+Agreements are represented by topology segments. By default topology segment
+represents 2 replication agreements - one for each direction, e.g., A to B and
+B to A. Creation of unidirectional segments is not allowed.
+""") + _("""
+To verify that no server is disconnected in the topology of the given suffix,
+use:
+  ipa topologysuffix-verify $suffix
+""") + _("""
 
-Requires minimum domain level 1.
+Examples:
+  Find all IPA servers:
+ipa server-find
+""") + _("""
+  Find all suffixes:
+ipa topologysuffix-find
+""") + _("""
+  Add topology segment to 'domain' suffix:
+ipa topologysegment-add domain --left IPA_SERVER_A --right IPA_SERVER_B
+""") + _("""
+  Add topology segment to 'ca' suffix:
+ipa topologysegment-add ca --left IPA_SERVER_A --right IPA_SERVER_B
+""") + _("""
+  List all topology segments in 'domain' suffix:
+ipa topologysegment-find domain
+""") + _("""
+  List all topology segments in 'ca' suffix:
+ipa topologysegment-find ca
+""") + _("""
+  Delete topology segment in 'domain' suffix:
+ipa topologysegment-del domain segment_name
+""") + _("""
+  Delete topology segment in 'ca' suffix:
+ipa topologysegment-del ca segment_name
+""") + _("""
+  Verify topology of 'domain' suffix:
+ipa topologysuffix-verify domain
+""") + _("""
+  Verify topology of 'ca' suffix:
+ipa topologysuffix-verify ca
 """)
 
 register = Registry()
-- 
2.5.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

Re: [Freeipa-devel] [PATCH] 941 Extend topology help

[Tomas Babej]

On 12/03/2015 04:58 PM, Petr Vobornik wrote:
> `ipa help topology` is improved.
> 
> 

Looks good. I changed one part of the documentation for more clarity,
see the attached patch.

Otherwise ACK from me.

Tomas
From b460994c2a0f454ffcfdc8345d58c0c963155c7d Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 3 Dec 2015 16:29:27 +0100
Subject: [PATCH] Extend topology help

`ipa help topology` is improved.
---
 ipalib/plugins/topology.py | 53 --
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugins/topology.py b/ipalib/plugins/topology.py
index 40f9fa8038d4bcb6c0eeb82a37ac796d732b82fd..662a8e2f354992336a21316ee06cb19af7c31bf2 100644
--- a/ipalib/plugins/topology.py
+++ b/ipalib/plugins/topology.py
@@ -22,9 +22,58 @@ if six.PY3:
 __doc__ = _("""
 Topology
 
-Management of a replication topology.
+Management of a replication topology at domain level 1.
+""") + _("""
+IPA server's data is stored in LDAP server in two suffixes:
+* domain suffix, e.g., 'dc=example,dc=com', contains all domain related data
+* ca suffix, 'o=ipaca', is present only on server with CA installed. It
+  contains data for Certificate Server component
+""") + _("""
+Data stored on IPA servers is replicated to other IPA servers. The way it is
+replicated is defined by replication agreements. Replication agreements needs
+to be set for both suffixes separately. On domain level 0 they are managed
+using ipa-replica-manage and ipa-csreplica-manage tools. With domain level 1
+they are managed centrally using `ipa topology*` commands.
+""") + _("""
+Agreements are represented by topology segments. By default topology segment
+represents 2 replication agreements - one for each direction, e.g., A to B and
+B to A. Creation of unidirectional segments is not allowed.
+""") + _("""
+To verify the no server is disconnected in the topology of the given suffix,
+use:
+  ipa topologysuffix-verify $suffix
+""") + _("""
 
-Requires minimum domain level 1.
+Examples:
+  Find all IPA servers:
+ipa server-find
+""") + _("""
+  Find all suffixes:
+ipa topologysuffix-find
+""") + _("""
+  Add topology segment to 'domain' suffix:
+ipa topologysegment-add domain --left IPA_SERVER_A --right IPA_SERVER_B
+""") + _("""
+  Add topology segment to 'ca' suffix:
+ipa topologysegment-add ca --left IPA_SERVER_A --right IPA_SERVER_B
+""") + _("""
+  List all topology segments in 'domain' suffix:
+ipa topologysegment-find domain
+""") + _("""
+  List all topology segments in 'ca' suffix:
+ipa topologysegment-find ca
+""") + _("""
+  Delete topology segment in 'domain' suffix:
+ipa topologysegment-del domain segment_name
+""") + _("""
+  Delete topology segment in 'ca' suffix:
+ipa topologysegment-del ca segment_name
+""") + _("""
+  Verify topology of 'domain' suffix:
+ipa topologysuffix-verify domain
+""") + _("""
+  Verify topology of 'ca' suffix:
+ipa topologysuffix-verify ca
 """)
 
 register = Registry()
-- 
2.5.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 0390] man: Update the ipa-replica-install manpage with promotion

[Tomas Babej]

Hi,

this patch updates the man page for the ipa-replica-install given the
latest changes (including the Jan's OTP patch).

Tomas
From 454e091bc29a536452094ecbf6fe72c100d46f30 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 3 Dec 2015 11:42:03 +0100
Subject: [PATCH] man: Update the ipa-replica-install manpage with promotion
 related info

---
 install/tools/man/ipa-replica-install.1 | 45 -
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/install/tools/man/ipa-replica-install.1 b/install/tools/man/ipa-replica-install.1
index df01c616c89abeb5c1f421c6e29b1034b2ec9ea7..05753fac51af29024450033c025a733f8fe6a59f 100644
--- a/install/tools/man/ipa-replica-install.1
+++ b/install/tools/man/ipa-replica-install.1
@@ -20,13 +20,24 @@
 .SH "NAME"
 ipa\-replica\-install \- Create an IPA replica
 .SH "SYNOPSIS"
+.SS "DOMAIN LEVEL 0"
+.TP
 ipa\-replica\-install [\fIOPTION\fR]... replica_file
+.SS "DOMAIN LEVEL 1"
+.TP
+ipa\-replica\-install [\fIOPTION\fR]...
 .SH "DESCRIPTION"
-Configures a new IPA server that is a replica of the server that generated it. Once it has been created it is an exact copy of the original IPA server and is an equal master. Changes made to any master are automatically replicated to other masters.
+Configures a new IPA server that is a replica of the server. Once it has been created it is an exact copy of the original IPA server and is an equal master. Changes made to any master are automatically replicated to other masters.
 
-The replica_file is created using the ipa\-replica\-prepare utility.
+To create a replica in a domain at domain level 0, you need to provide an replica file. The replica_file is created using the ipa\-replica\-prepare utility.
 
-If the installation fails you may need to run ipa\-server\-install \-\-uninstall before running ipa\-replica\-install again.
+To create a replica in a domain at domain level 1, you cannot provide an replica file, the machine only needs to be enrolled in the FreeIPA domain first. This process of turning the IPA client into a replica is also referred to as replica promotion.
+
+If you're starting with an existing IPA client, simply run ipa\-replica\-install to have it promoted into a replica.
+
+To promote a blank machine into a replica, you have two options, you can either run ipa\-client\-install in a separate step, or pass the enrollment related options to the ipa\-replica\-install (see ENROLLMENT OPTIONS). In the latter case, ipa\-replica\-install will join the machine to the IPA realm automatically and will proceed with the promotion step.
+
+If the installation fails you may need to run ipa\-server\-install \-\-uninstall and ipa\-client\-install before running ipa\-replica\-install again.
 
 The installation will fail if the host you are installing the replica on exists as a host in IPA or an existing replication agreement exists (for example, from a previously failed installation).
 
@@ -42,11 +53,11 @@ certificate operations will be forwarded to a master with a CA installed.
 The IP address of this server. If this address does not match the address the host resolves to and \-\-setup\-dns is not selected the installation will fail. If the server hostname is not resolvable, a record for the hostname and IP_ADDRESS is added to /etc/hosts.
 This this option can be used multiple times to specify more IP addresses of the server (e.g. multihomed and/or dualstacked server).
 .TP
-\fB\-p\fR \fIDM_PASSWORD\fR, \fB\-\-password\fR=\fIDM_PASSWORD\fR
-Directory Manager (existing master) password
+\fB\-p\fR \fIPASSWORD\fR, \fB\-\-password\fR=\fIPASSWORD\fR
+For domain level 0, this enter the existing Directory Manager password. Under domain level 1, use this option to specify the password for joining a machine to the IPA realm. Assumes bulk password unless principal is also set.
 .TP
-\fB\-w\fR \fIADMIN_PASSWORD\fR, \fB\-\-admin\-password\fR=\fIADMIN_PASSWORD\fR
-Admin user Kerberos password used for connection check
+\fB\-w\fR, \fB\-\-admin\-password\fR
+The Kerberos password for the given principal.
 .TP
 \fB\-\-mkhomedir\fR
 Create home directories for users on their first login
@@ -78,6 +89,26 @@ An unattended installation that will never prompt for user input
 \fB\-\-dirsrv\-config\-file\fR
 The path to LDIF file that will be used to modify configuration of dse.ldif during installation of the directory server instance
 
+.SS "ENROLLMENT OPTIONS"
+.TP
+\fB\-\-server\fR
+The fully qualified domain name of the IPA server to enroll to.
+.TP
+\fB\-d\fR, \fB\-\-realm\fR=\fIREALM_NAME\fR
+Set the IPA realm name to REALM_NAME. Under normal circumstances, this option is not needed as the realm name is retrieved from the IPA server.
+.TP
+\fB\-n\fR, \fB\-\-domain\fR=\fIDOMAIN\fR
+Set the domain name to DOMAIN. When no \-\-server option is specified, the installer will try to discover all available servers via DNS SRV record 

Re: [Freeipa-devel] [PATCH 0388] tests: Add hostmask detection for sudo rules validating

[Tomas Babej]

On 12/02/2015 05:25 PM, Lukas Slebodnik wrote:
> On (02/12/15 15:41), Tomas Babej wrote:
>>
>>
>> On 12/02/2015 09:24 AM, Tomas Babej wrote:
>>>
>>>
>>> On 12/01/2015 06:27 PM, Tomas Babej wrote:
>>>>
>>>>
>>>> On 11/30/2015 05:32 PM, Lukas Slebodnik wrote:
>>>>> On (30/11/15 13:09), Tomas Babej wrote:
>>>>>> Hi,
>>>>>>
>>>>>> IPA sudo tests worked under the assumption that the clients that
>>>>>> are executing the sudo commands have their IPs assigned within
>>>>>> 255.255.255.0 hostmask.
>>>>>>
>>>>>> Removes this (invalid) assumption and adds a dynamic detection of
>>>>>> the hostmask of the IPA client.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/5501
>>>>>
>>>>> >From e6f1846f0d7d17303e5b30b1643651ba739b2b6c Mon Sep 17 00:00:00 2001
>>>>>> From: Tomas Babej <tba...@redhat.com>
>>>>>> Date: Mon, 30 Nov 2015 12:53:39 +0100
>>>>>> Subject: [PATCH] tests: Add hostmask detection for sudo rules validating 
>>>>>> on
>>>>>> hostmask
>>>>>>
>>>>>> IPA sudo tests worked under the assumption that the clients that
>>>>>> are executing the sudo commands have their IPs assigned within
>>>>>> 255.255.255.0 hostmask.
>>>>>>
>>>>>> Removes this (invalid) assumption and adds a dynamic detection of
>>>>>> the hostmask of the IPA client.
>>>>>>
>>>>
>>>> Thanks, updated patch attached.
>>>>
>>>> Tomas
>>>>
>>>
>>> Actually, a small improvement is necessary.
>>>
>>> Updated patch attached.
>>>
>>> Tomas
>>>
>>>
>>>
>>
>> Thanks to Lukas, we found another problem with the test.
>>
>> Updated patch attached.
>>
> Thank you for 4th revision of patch
> but there is still one issue.
> 
> === FAILURES 
> ===
> _ TestSudo.test_sudo_rule_restricted_to_one_hostmask_negative 
> __
> 
> self = 
> 
> def test_sudo_rule_restricted_to_one_hostmask_negative(self):
> result1 = self.list_sudo_commands("testuser1")
>>   assert result1.returncode != 0
> E   assert 0 != 0
> E+  where 0 =  0x7ff695f56bd0>.returncode
> 
> test_integration/test_sudo.py:323: AssertionError
>  1 failed, 74 passed in 807.35 seconds 
> =
> 
> LS
> 

Here the test affected the negative test setup, which is fixed in the
latest revision.

Thanks,
Tomas
From 6242dfda205f6b2749627023384878448fd9e60c Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 2 Dec 2015 15:25:49 +0100
Subject: [PATCH] tests: Add hostmask detection for sudo rules validating on
 hostmask

IPA sudo tests worked under the assumption that the clients
that are executing the sudo commands have their IPs assigned
within 255.255.255.0 hostmask.

Removes this (invalid) assumption and adds a
dynamic detection of the hostmask of the IPA client.

https://fedorahosted.org/freeipa/ticket/5501
---
 ipatests/test_integration/test_sudo.py | 46 +-
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/ipatests/test_integration/test_sudo.py b/ipatests/test_integration/test_sudo.py
index 1dd4c5d73c9fa4288af4fc2708aa3abd51407217..a1b7aa6cfc257e0eeb7def3c0e5b3c7fa98ee250 100644
--- a/ipatests/test_integration/test_sudo.py
+++ b/ipatests/test_integration/test_sudo.py
@@ -17,6 +17,9 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import pytest
+import re
+
 from ipatests.test_integration.base import IntegrationTest
 from ipatests.test_integration.tasks import clear_sssd_cache
 
@@ -104,6 +107,20 @@ class TestSudo(IntegrationTest):
 
 return result
 
+def get_client_ip_with_hostmask(self):
+"""
+Detects the IP of the client including the hostmask.
+"""
+
+ip = self.client.ip
+result = self.client.run_command(['ip', 'addr'])
+full_ip_regex = r'(?P%s/\d{1,2}) ' % re.escape(ip)
+match = re.search(full_ip_regex, result.stdout_text)
+
+if match:
+return match.group('full_ip')
+
+
 def test_nisdomainname(self):
 result = self.client.run_command('nisdomainname')
 a

Re: [Freeipa-devel] [PATCH 0388] tests: Add hostmask detection for sudo rules validating

[Tomas Babej]

On 12/03/2015 04:26 PM, Aleš Mareček wrote:
> Hello,
> 
> ACK for code
> NACK for the placing "get_client_ip_with_hostmask" function to test_sudo.py 
> (this function should be in some more general file)
> 

What place would you propose? The task.py is not a good place, as this
is not really a task.

Nevertheless, I'd rather have it moved when an use case outside
test_sudo.py actually arises. Right now it would lead to unnecessary
cluttering.

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