Re: [Freeipa-devel] [PATCH 0018] Minor fix in ipa-replica-manage MAN page

2016-07-26 Thread Abhijeet Kasurde

Ping.


On 07/13/2016 09:24 AM, Abhijeet Kasurde wrote:


Hi All,

Please review patch.

Fixes: https://fedorahosted.org/freeipa/ticket/6058
--
Thanks,
Abhijeet Kasurde

IRC: akasurde
http://akasurde.github.io





--
Thanks,
Abhijeet Kasurde

IRC: akasurde
http://akasurde.github.io

-- 
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] [Test][patch-0053] Forced-client-reenrollment test fixed.

2016-07-26 Thread Martin Babinsky

On 07/26/2016 03:34 PM, Oleg Fayans wrote:

Hi Martin,

The patch was updated according to your suggestions. A separate patch
removing outdated tests is attached.

On 07/08/2016 02:10 PM, Martin Basti wrote:



On 07.07.2016 08:09, Oleg Fayans wrote:

Updated version of the patch is attached with the failing tests marked
as xfailed (let's make the jenkins green).

On 07/04/2016 10:50 PM, Oleg Fayans wrote:

2 out of 7 tests currently fail due to a known issue [1], others pass.

[1] https://fedorahosted.org/freeipa/ticket/6029









This is wrong:

1)
you are not getting SSHFP records, just SSH public key (with your
changes)

2)
you are using host-find without any arguments, so it will returns SSH
key for all hosts, the code before was getting SSHFP only for one host.
Would be better to use host-show?

3)
you actually found a bug, because host-find and host-show should print
only SSH fingerprints not SSH keys
https://fedorahosted.org/freeipa/ticket/6042
https://fedorahosted.org/freeipa/ticket/6043

4)
don't call it SSHFP records in code, because it is not DNS related,
probably you want to get SSH fingerprints instead of SSH keys

5)
It may contain multiple SSH keys, you always return only the first (the
original code returns all values)

 def get_sshfp_record(self):
-sshfp_record = ''
-client_host = self.clients[0].hostname.split('.')[0]
-
  result = self.master.run_command(
-['ipa', 'dnsrecord-show', self.master.domain.name,
client_host]
+['ipa', 'host-find']
  )
-
-lines = result.stdout_text.splitlines()
-for line in lines:
-if 'SSHFP record:' in line:
-sshfp_record = line.replace('SSHFP record:', '').strip()
-
-assert sshfp_record, 'SSHFP record not found'
-
-sshfp_record = set(sshfp_record.split(', '))
-self.log.debug("SSHFP record for host %s: %s", client_host,
str(sshfp_record))
-
-return sshfp_record
+records = result.stdout_text.split('\n\n')
+sshkey_re = re.compile('.+SSH public key: ssh-\w+ (\S+?),.+')
+for hostrecord in records:
+if self.clients[0].hostname in hostrecord:
+sshfps = sshkey_re.findall(hostrecord)
+assert sshfps, 'SSHFP record not found'
+sshfp = sshfps[0]
+return sshfp








Oleg,

the original forced client reenrollment suite passes for me:

"""
=== 8 passed in 1859.52 seconds 


"""

I am not quite sure what are you trying to accomplish with these 
patches, since you are deleting valid test cases and then removing 
'restore_client' method that simulates the use case of e.g. removing a 
client VM while keeping old keytab and host entry and then re-enrolling.


The commit messages did not help very much, either.

Please review the original design and test cases carefully and make sure 
you understand why is the test constructed the way it is:


http://www.freeipa.org/page/V3/Forced_client_re-enrollment

NACK until you clearly state the purpose of your patches.

--
Martin^3 Babinsky

--
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 0196] baseldap: Fix MidairCollision instantiation during entry modification

2016-07-26 Thread Alexander Bokovoy

On Tue, 26 Jul 2016, Alexander Bokovoy wrote:

On Tue, 26 Jul 2016, Martin Babinsky wrote:
> Fix for https://fedorahosted.org/freeipa/ticket/6097
> 
> Since this issue was found during investigation of other ticket[1], you 
> can test it by performing steps to reproduce #6041, but instead of 
> internal error you should see the MidairCollision raised as public error 
> with the right error message.
> 
> [1] https://fedorahosted.org/freeipa/ticket/6041

I have a preliminary patch for slapi-nis to fix 6041 (attached).

Tested the slapi-nis patch:

# kinit administra...@ad.test
Password for administra...@ad.test: 
# ipa idoverrideuser-find 'default trust view' administra...@ad.test --raw --all

--
1 User ID override matched
--
 dn: 
ipaanchoruuid=:SID:S-1-5-21-2275361654-3393353068-3720134936-500,cn=Default 
Trust View,cn=views,cn=accounts,dc=ipa,dc=ad,dc=test
 ipaanchoruuid: :SID:S-1-5-21-2275361654-3393353068-3720134936-500
 loginshell: /bin/bash
 ipaoriginaluid: administra...@ad.test
 objectclass: ipaOverrideAnchor
 objectclass: top
 objectclass: ipaUserOverride
 objectclass: ipasshuser
 objectclass: ipaSshGroupOfPubKeys

Number of entries returned 1

# ipa idoverrideuser-mod 'default trust view' administra...@ad.test 
--addattr='objectclass=nestedGroup'
ipa: ERROR: Insufficient access: Insufficient 'write' privilege to the 
'objectClass' attribute of entry
'ipaanchoruuid=:sid:s-1-5-21-2275361654-3393353068-3720134936-500,cn=default 
trust view,cn=views,cn=accounts,dc=ipa,dc=ad,dc=test'.
# klist -A
Ticket cache: KEYRING:persistent:0:0
Default principal: administra...@ad.test

Valid starting   Expires  Service principal
07/26/2016 18:45:46  07/27/2016 04:45:30
HTTP/f24-master.ipa.ad.t...@ipa.ad.test
renew until 07/27/2016 18:45:27
07/26/2016 18:45:46  07/27/2016 04:45:30  krbtgt/ipa.ad.t...@ad.test
renew until 07/27/2016 18:45:27
07/26/2016 18:45:30  07/27/2016 04:45:30  krbtgt/ad.t...@ad.test
renew until 07/27/2016 18:45:27
# ipa idoverrideuser-mod 'default trust view' administra...@ad.test 
--desc='Administrator of a trusted domain'

Modified an User ID override "administra...@ad.test"

 Anchor to override: administra...@ad.test
 Description: Administrator of a trusted domain
 Login shell: /bin/bash

So no MidairCollision anymore and editing ID override as the AD user
associated with the override works for those attributes that are
allowed.

--
/ Alexander Bokovoy

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0196] baseldap: Fix MidairCollision instantiation during entry modification

2016-07-26 Thread Alexander Bokovoy

On Tue, 26 Jul 2016, Martin Babinsky wrote:

Fix for https://fedorahosted.org/freeipa/ticket/6097

Since this issue was found during investigation of other ticket[1], 
you can test it by performing steps to reproduce #6041, but instead of 
internal error you should see the MidairCollision raised as public 
error with the right error message.


[1] https://fedorahosted.org/freeipa/ticket/6041

I have a preliminary patch for slapi-nis to fix 6041 (attached).

As for this fix -- ACK.



--
Martin^3 Babinsky



From 8f0d6dab08f61fe2fd1ad64a63f7ab91fc5227d4 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 25 Jul 2016 14:05:08 +0200
Subject: [PATCH] baseldap: Fix MidairCollision instantiation during entry
modification

https://fedorahosted.org/freeipa/ticket/6097
---
ipaserver/plugins/baseldap.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py
index 
6107e43a6ee17d9b9a63d9dc109664d8b232069f..f7844e3e7c59c259b9c8367d135b2dbefc3f0016
 100644
--- a/ipaserver/plugins/baseldap.py
+++ b/ipaserver/plugins/baseldap.py
@@ -1466,7 +1466,7 @@ class LDAPUpdate(LDAPQuery, crud.Update):
entry_attrs.dn, attrs_list)
except errors.NotFound:
raise errors.MidairCollision(
-format=_('the entry was deleted while being modified')
+message=_('the entry was deleted while being modified')
)

self.obj.get_indirect_members(entry_attrs, attrs_list)
@@ -2344,7 +2344,7 @@ class BaseLDAPModAttribute(LDAPQuery):
entry_attrs.dn, attrs_list)
except errors.NotFound:
raise errors.MidairCollision(
-format=_('the entry was deleted while being modified')
+message=_('the entry was deleted while being modified')
)

for callback in self.get_callbacks('post'):
--
2.7.4




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



--
/ Alexander Bokovoy
From 987472fbe8c3564c0bb70c0dd8eebbc430117e0a Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy 
Date: Tue, 26 Jul 2016 18:11:53 +0300
Subject: [PATCH] back-sch: do not clobber target of the pblock for idview

When extracting idview all we care is the DN of new target.
We don't really use the rewritten target as a string anymore,
so there is no need to rewrite the string in the pblock.

This fixes a bug when running with 389-ds 1.3.5.10+ which is more
strict about modification of the values in pblock.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1360245
---
 src/back-sch.c | 43 ++-
 src/back-sch.h |  2 +-
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/src/back-sch.c b/src/back-sch.c
index 0745329..e15988f 100644
--- a/src/back-sch.c
+++ b/src/back-sch.c
@@ -1652,6 +1652,8 @@ backend_search_cb(Slapi_PBlock *pb)
struct backend_search_cbdata cbdata;
struct backend_staged_search *staged, *next;
int i, isroot, ret;
+   char *original_target = NULL;
+   char *target = NULL;
 
if (wrap_get_call_level() > 0) {
return 0;
@@ -1676,7 +1678,7 @@ backend_search_cb(Slapi_PBlock *pb)
return 0;
}
 
-   slapi_pblock_get(pb, SLAPI_SEARCH_TARGET, );
+   slapi_pblock_get(pb, SLAPI_SEARCH_TARGET, _target);
slapi_pblock_get(pb, SLAPI_SEARCH_SCOPE, );
slapi_pblock_get(pb, SLAPI_SEARCH_SIZELIMIT, );
slapi_pblock_get(pb, SLAPI_SEARCH_TIMELIMIT, );
@@ -1697,15 +1699,15 @@ backend_search_cb(Slapi_PBlock *pb)
/* Okay, we can search. */
slapi_log_error(SLAPI_LOG_PLUGIN, cbdata.state->plugin_desc->spd_id,
"searching from \"%s\" for \"%s\" with scope %d%s\n",
-   cbdata.target, cbdata.strfilter, cbdata.scope,
+   original_target, cbdata.strfilter, cbdata.scope,
backend_sch_scope_as_string(cbdata.scope));
-   cbdata.target_dn = slapi_sdn_new_dn_byval(cbdata.target);
+   cbdata.target_dn = slapi_sdn_new_dn_byval(original_target);
/* Check if there's a backend handling this search. */
if (!slapi_be_exist(cbdata.target_dn)) {
slapi_log_error(SLAPI_LOG_PLUGIN,
cbdata.state->plugin_desc->spd_id,
"slapi_be_exists(\"%s\") = 0, "
-   "ignoring search\n", cbdata.target);
+   "ignoring search\n", original_target);
slapi_sdn_free(_dn);
return 0;
}
@@ -1716,22 +1718,23 @@ backend_search_cb(Slapi_PBlock *pb)
 * detect the ID view use. Unless the ID view is within the set we 
control, don't consider the override */

[Freeipa-devel] [PATCH] 0002 Add client install option to set ipa_backup_server

2016-07-26 Thread Ariel Barria
Hello everyone.

I send patch for review.

Regards,
From 3a27ef5bf3001f5f5ad2e71a4fc76a7a5c104e88 Mon Sep 17 00:00:00 2001
From: "Ariel O. Barria" 
Date: Tue, 26 Jul 2016 09:32:26 -0500
Subject: [PATCH] freeipa arielb 0002 Add client install option to set
 ipa_backup_server

Add ipa backup_server on sssd.conf using ipa-client-install

https://fedorahosted.org/freeipa/ticket/4016
---
 client/ipa-client-install | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index 05b6b6e0da07353750d0dca4e6df9d1f58d69c35..af5d72a5d9e0b89edbb1a5bb974621ae2f02b85a 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -115,6 +115,10 @@ def parse_options():
 basic_group = OptionGroup(parser, "basic options")
 basic_group.add_option("--domain", dest="domain", help="domain name")
 basic_group.add_option("--server", dest="server", help="IPA server", action="append")
+basic_group.add_option("--ipa-backup-server", dest="ipa_backup_server",
+  default=False,
+  help="Configure sssd to use backup server if no primary"
+   " servers can be reached.")
 basic_group.add_option("--realm", dest="realm_name", help="realm name")
 basic_group.add_option("--fixed-primary", dest="primary", action="store_true",
   default=False, help="Configure sssd to use fixed server as primary IPA server")
@@ -231,6 +235,13 @@ def parse_options():
 if (options.server and not options.domain):
 parser.error("--server cannot be used without providing --domain")
 
+if (options.server and not options.domain):
+parser.error("--server cannot be used without providing --domain")
+
+if (options.ipa_backup_server and not options.primary):
+parser.error("--ipa-backup-server cannot be used without providing "
+ "--fixed-primary")
+
 if options.domain:
 try:
 validate_domain_name(options.domain)
@@ -1273,6 +1284,9 @@ def configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options, clie
 if not options.on_master:
 if options.primary:
 domain.set_option('ipa_server', ', '.join(cli_server))
+if options.ipa_backup_server:
+domain.set_option('ipa_backup_server',
+   options.ipa_backup_server)
 else:
 domain.set_option('ipa_server', '_srv_, %s' % ', '.join(cli_server))
 else:
-- 
2.7.4

-- 
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 0196] baseldap: Fix MidairCollision instantiation during entry modification

2016-07-26 Thread Martin Babinsky

Fix for https://fedorahosted.org/freeipa/ticket/6097

Since this issue was found during investigation of other ticket[1], you 
can test it by performing steps to reproduce #6041, but instead of 
internal error you should see the MidairCollision raised as public error 
with the right error message.


[1] https://fedorahosted.org/freeipa/ticket/6041

--
Martin^3 Babinsky
From 8f0d6dab08f61fe2fd1ad64a63f7ab91fc5227d4 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 25 Jul 2016 14:05:08 +0200
Subject: [PATCH] baseldap: Fix MidairCollision instantiation during entry
 modification

https://fedorahosted.org/freeipa/ticket/6097
---
 ipaserver/plugins/baseldap.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py
index 6107e43a6ee17d9b9a63d9dc109664d8b232069f..f7844e3e7c59c259b9c8367d135b2dbefc3f0016 100644
--- a/ipaserver/plugins/baseldap.py
+++ b/ipaserver/plugins/baseldap.py
@@ -1466,7 +1466,7 @@ class LDAPUpdate(LDAPQuery, crud.Update):
 entry_attrs.dn, attrs_list)
 except errors.NotFound:
 raise errors.MidairCollision(
-format=_('the entry was deleted while being modified')
+message=_('the entry was deleted while being modified')
 )
 
 self.obj.get_indirect_members(entry_attrs, attrs_list)
@@ -2344,7 +2344,7 @@ class BaseLDAPModAttribute(LDAPQuery):
 entry_attrs.dn, attrs_list)
 except errors.NotFound:
 raise errors.MidairCollision(
-format=_('the entry was deleted while being modified')
+message=_('the entry was deleted while being modified')
 )
 
 for callback in self.get_callbacks('post'):
-- 
2.7.4

-- 
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] 0001 six.u function instead of the decode

2016-07-26 Thread Petr Spacek
On 26.7.2016 16:28, Jan Cholasta wrote:
> Hi,
> 
> On 26.7.2016 16:09, Martin Basti wrote:
>>
>>
>> On 22.07.2016 00:14, Ariel Barria wrote:
>>> Hello everyone.
>>>
>>> I send patch for review.
> 
> NACK, six.u() is supposed to be used on string literals *only* [1].
> 
> A proper fix would be something like:
> 
> value = self.to_text()
> if not isinstance(value, unicode):
> value = value.decode('ascii')
> return value

Most importantly, we should fix/provide this method in python-dns and inherit
this method from there.

Petr^2 Spacek

> 
>>>
>>> Regards,
>>>
>>>
>>
>> Thank you,
>>
>> I will look on this, for some reason we received your e-mail just today
>> (2016-07-26)
> 
> Honza
> 
> [1] 

-- 
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] 0001 six.u function instead of the decode

2016-07-26 Thread Jan Cholasta

Hi,

On 26.7.2016 16:09, Martin Basti wrote:



On 22.07.2016 00:14, Ariel Barria wrote:

Hello everyone.

I send patch for review.


NACK, six.u() is supposed to be used on string literals *only* [1].

A proper fix would be something like:

value = self.to_text()
if not isinstance(value, unicode):
value = value.decode('ascii')
return value



Regards,




Thank you,

I will look on this, for some reason we received your e-mail just today
(2016-07-26)


Honza

[1] 

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 0001 six.u function instead of the decode

2016-07-26 Thread Martin Basti



On 22.07.2016 00:14, Ariel Barria wrote:

Hello everyone.

I send patch for review.

Regards,




Thank you,

I will look on this, for some reason we received your e-mail just today 
(2016-07-26)


Martin
-- 
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] [Test][patch-0053] Forced-client-reenrollment test fixed.

2016-07-26 Thread Oleg Fayans

Here is the test output:

https://paste.fedoraproject.org/395706/69538081/

On 07/26/2016 03:34 PM, Oleg Fayans wrote:

Hi Martin,

The patch was updated according to your suggestions. A separate patch
removing outdated tests is attached.

On 07/08/2016 02:10 PM, Martin Basti wrote:



On 07.07.2016 08:09, Oleg Fayans wrote:

Updated version of the patch is attached with the failing tests marked
as xfailed (let's make the jenkins green).

On 07/04/2016 10:50 PM, Oleg Fayans wrote:

2 out of 7 tests currently fail due to a known issue [1], others pass.

[1] https://fedorahosted.org/freeipa/ticket/6029









This is wrong:

1)
you are not getting SSHFP records, just SSH public key (with your
changes)

2)
you are using host-find without any arguments, so it will returns SSH
key for all hosts, the code before was getting SSHFP only for one host.
Would be better to use host-show?

3)
you actually found a bug, because host-find and host-show should print
only SSH fingerprints not SSH keys
https://fedorahosted.org/freeipa/ticket/6042
https://fedorahosted.org/freeipa/ticket/6043

4)
don't call it SSHFP records in code, because it is not DNS related,
probably you want to get SSH fingerprints instead of SSH keys

5)
It may contain multiple SSH keys, you always return only the first (the
original code returns all values)

 def get_sshfp_record(self):
-sshfp_record = ''
-client_host = self.clients[0].hostname.split('.')[0]
-
  result = self.master.run_command(
-['ipa', 'dnsrecord-show', self.master.domain.name,
client_host]
+['ipa', 'host-find']
  )
-
-lines = result.stdout_text.splitlines()
-for line in lines:
-if 'SSHFP record:' in line:
-sshfp_record = line.replace('SSHFP record:', '').strip()
-
-assert sshfp_record, 'SSHFP record not found'
-
-sshfp_record = set(sshfp_record.split(', '))
-self.log.debug("SSHFP record for host %s: %s", client_host,
str(sshfp_record))
-
-return sshfp_record
+records = result.stdout_text.split('\n\n')
+sshkey_re = re.compile('.+SSH public key: ssh-\w+ (\S+?),.+')
+for hostrecord in records:
+if self.clients[0].hostname in hostrecord:
+sshfps = sshkey_re.findall(hostrecord)
+assert sshfps, 'SSHFP record not found'
+sshfp = sshfps[0]
+return sshfp








--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

--
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] [Test][patch-0053] Forced-client-reenrollment test fixed.

2016-07-26 Thread Oleg Fayans

Hi Martin,

The patch was updated according to your suggestions. A separate patch 
removing outdated tests is attached.


On 07/08/2016 02:10 PM, Martin Basti wrote:



On 07.07.2016 08:09, Oleg Fayans wrote:

Updated version of the patch is attached with the failing tests marked
as xfailed (let's make the jenkins green).

On 07/04/2016 10:50 PM, Oleg Fayans wrote:

2 out of 7 tests currently fail due to a known issue [1], others pass.

[1] https://fedorahosted.org/freeipa/ticket/6029









This is wrong:

1)
you are not getting SSHFP records, just SSH public key (with your changes)

2)
you are using host-find without any arguments, so it will returns SSH
key for all hosts, the code before was getting SSHFP only for one host.
Would be better to use host-show?

3)
you actually found a bug, because host-find and host-show should print
only SSH fingerprints not SSH keys
https://fedorahosted.org/freeipa/ticket/6042
https://fedorahosted.org/freeipa/ticket/6043

4)
don't call it SSHFP records in code, because it is not DNS related,
probably you want to get SSH fingerprints instead of SSH keys

5)
It may contain multiple SSH keys, you always return only the first (the
original code returns all values)

 def get_sshfp_record(self):
-sshfp_record = ''
-client_host = self.clients[0].hostname.split('.')[0]
-
  result = self.master.run_command(
-['ipa', 'dnsrecord-show', self.master.domain.name, client_host]
+['ipa', 'host-find']
  )
-
-lines = result.stdout_text.splitlines()
-for line in lines:
-if 'SSHFP record:' in line:
-sshfp_record = line.replace('SSHFP record:', '').strip()
-
-assert sshfp_record, 'SSHFP record not found'
-
-sshfp_record = set(sshfp_record.split(', '))
-self.log.debug("SSHFP record for host %s: %s", client_host,
str(sshfp_record))
-
-return sshfp_record
+records = result.stdout_text.split('\n\n')
+sshkey_re = re.compile('.+SSH public key: ssh-\w+ (\S+?),.+')
+for hostrecord in records:
+if self.clients[0].hostname in hostrecord:
+sshfps = sshkey_re.findall(hostrecord)
+assert sshfps, 'SSHFP record not found'
+sshfp = sshfps[0]
+return sshfp




--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From d5e6dd5ab115a10a8a504f4f0c5b3117cdbc0176 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Tue, 26 Jul 2016 15:06:41 +0200
Subject: [PATCH] Removed outdated reenrollment tests

https://fedorahosted.org/freeipa/ticket/6029
---
 .../test_forced_client_reenrollment.py | 38 +++---
 1 file changed, 5 insertions(+), 33 deletions(-)

diff --git a/ipatests/test_integration/test_forced_client_reenrollment.py b/ipatests/test_integration/test_forced_client_reenrollment.py
index d430a98e74450f44eac286ac0ad35a5aee7cc602..1ea57a871b0830f9afa18de8029739cae8115a49 100644
--- a/ipatests/test_integration/test_forced_client_reenrollment.py
+++ b/ipatests/test_integration/test_forced_client_reenrollment.py
@@ -58,42 +58,14 @@ class TestForcedClientReenrollment(IntegrationTest):
 
 def test_reenroll_with_keytab(self, client):
 """
-Client re-enrollment using keytab
+Client re-enrollment using keytab: the old keytab should be invalid,
+see https://fedorahosted.org/freeipa/ticket/6029 for reasoning
 """
 self.backup_keytab()
-sshfp_record_pre = self.get_sshfp_record()
-self.restore_client()
-self.check_client_host_entry()
+self.uninstall_client()
+self.check_client_host_entry(enabled=False)
 self.restore_keytab()
-self.reenroll_client(keytab=self.BACKUP_KEYTAB)
-sshfp_record_post = self.get_sshfp_record()
-assert sshfp_record_pre == sshfp_record_post
-
-def test_reenroll_with_both_force_join_and_keytab(self, client):
-"""
-Client re-enrollment using both --force-join and --keytab options
-"""
-self.backup_keytab()
-sshfp_record_pre = self.get_sshfp_record()
-self.restore_client()
-self.check_client_host_entry()
-self.restore_keytab()
-self.reenroll_client(force_join=True, keytab=self.BACKUP_KEYTAB)
-sshfp_record_post = self.get_sshfp_record()
-assert sshfp_record_pre == sshfp_record_post
-
-def test_reenroll_to_replica(self, client):
-"""
-Client re-enrollment using keytab, to a replica
-"""
-self.backup_keytab()
-sshfp_record_pre = self.get_sshfp_record()
-self.restore_client()
-self.check_client_host_entry()
-self.restore_keytab()
-self.reenroll_client(keytab=self.BACKUP_KEYTAB, to_replica=True)
-sshfp_record_post = self.get_sshfp_record()
-assert sshfp_record_pre == sshfp_record_post
+self.reenroll_client(keytab=self.BACKUP_KEYTAB, 

Re: [Freeipa-devel] [PATCH 0553] CI tests: improve log collecting in tests

2016-07-26 Thread Martin Babinsky

On 07/25/2016 06:24 PM, Martin Basti wrote:



On 25.07.2016 18:02, Martin Babinsky wrote:

On 07/22/2016 06:13 PM, Martin Basti wrote:



On 20.07.2016 17:41, Martin Basti wrote:




On 19.07.2016 17:05, Martin Basti wrote:




On 19.07.2016 16:18, Martin Basti wrote:

Patch attached.




self-NACK, my assumptions were wrong, this doesn't work if any of log
files do not exist



updated patches attached

Please note, that in case that any logfile does not exist tar returns
exit code 2, but provides correct output anyway.



Updated patches attached.



NACK:

* Module ipatests.test_integration.tasks
ipatests/test_integration/tasks.py:68: [E1101(no-member),
setup_server_logs_collecting] Instance of 'FedoraPathNamespace' has no
'IPASERVER_CA_INSTALL_LOG' member)
Makefile:137: recipe for target 'lint' failed
make: *** [lint] Error 1


Ooops, I forgot to merge fix

Updated patch attached.

ACK.

Pushed to master: ae623864ee6e5dc2f6d254111c9cdd369fc144a8

--
Martin^3 Babinsky

--
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] 0001 six.u function instead of the decode

2016-07-26 Thread Ariel Barria
Hello everyone.

I send patch for review.

Regards,
From 7aaf440617963facf7bd156f1e7295562391a3f2 Mon Sep 17 00:00:00 2001
From: "Ariel O. Barria" 
Date: Thu, 21 Jul 2016 17:06:05 -0500
Subject: [PATCH] freeipa arielb 0001 six.u function instead of the decode
 function

to avoid DNSName.ToASCII broken with python3
In versions 3.3 onwards is not necessary to use the function six.u
based on the following link:
https://pythonhosted.org/six/#six.u

https://fedorahosted.org/freeipa/ticket/5935
---
 ipapython/dnsutil.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ipapython/dnsutil.py b/ipapython/dnsutil.py
index 16549c8f6a4a0782191526856e918d7ac6b94dcc..d9c4a88f19a35048e24dc1c95dfaff0cae697ceb 100644
--- a/ipapython/dnsutil.py
+++ b/ipapython/dnsutil.py
@@ -71,7 +71,10 @@ class DNSName(dns.name.Name):
 
 def ToASCII(self):
 #method named by RFC 3490 and python standard library
-return self.to_text().decode('ascii')  # must be unicode string
+if six.PY2:
+return self.to_text().decode('ascii')  # must be unicode string
+else:
+return six.u(self.to_text())  # must be unicode string
 
 def canonicalize(self):
 return DNSName(super(DNSName, self).canonicalize())
-- 
2.7.4

-- 
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 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

2016-07-26 Thread Martin Basti



On 26.07.2016 12:17, Lenka Doudova wrote:



On 06/30/2016 01:14 PM, Martin Basti wrote:



On 30.06.2016 12:58, Lenka Doudova wrote:



On 06/30/2016 12:51 PM, Petr Spacek wrote:

On 30.6.2016 12:32, Lenka Doudova wrote:


On 06/29/2016 07:49 PM, Petr Spacek wrote:

On 29.6.2016 18:52, Lenka Doudova wrote:

On 06/29/2016 06:51 PM, Petr Spacek wrote:

On 29.6.2016 18:48, Lenka Doudova wrote:

On 06/27/2016 11:05 AM, Lenka Doudova wrote:

On 06/27/2016 10:33 AM, Martin Babinsky wrote:

On 06/27/2016 10:28 AM, Petr Spacek wrote:

On 27.6.2016 10:26, Petr Spacek wrote:

On 27.6.2016 10:18, Martin Babinsky wrote:

On 06/27/2016 10:04 AM, Petr Vobornik wrote:

On 06/27/2016 09:42 AM, Lenka Doudova wrote:

Hi!

With newly created AD machines in Brno lab, existing 
trust tests

fail on
'ipa dnsforwardzone-add' command claiming the zone is 
already

present,
as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.

To prevent these failures I prepared attached patch, 
that will still
attempt to add the forward zone, but in case of 
non-zero return code
will check the message if it says that the forward zone 
is already

configured, and lets the tests continue, if it is so.


Lenka

Current approach expects that every error of ipa 
dnsforward-add here
will mean that the zone exists. So it might hide other 
issues - not

very
good.

On the other hand it is not very robust to parse error 
message.


Question for general audience: What do you think if IPA 
client's exit
status would be the IPA error code instead of "1" for 
every error.

E.g.
in DuplicateEntry case it's 4002.

Btw, this is not a NACK.

Well AFAIK the exit status on POSIX systems is encoded 
into a single

byte so
you cannot have the return value greater that 255. We 
would have to

devise
some mapping between our XMLRPC status codes and 
subprocess return

codes.

Some of our exceptions have defined return values outside 
plain '1',

e.g.
NotFound has return value of 2. It would be possible to 
extend this

concept on
other common errors.
Even more importantly, the forward zone is completely 
unnecessary

because
DNS
when DNS is set up properly. I would simply remove the
dnsforwardzone-add.


Grr, I meant this:
Even more importantly, the forward zone is completely 
unnecessary when

DNS is
set up properly. I would simply remove the dnsforwardzone-add.

+1, our tests should not fiddle with the provisioned 
environment as

much as
they sometimes do.

Well, I have nothing against removing it completely, but left 
it there just
because with previous AD machines with "wild" domains it was 
necessary.
Looking at the code, your suggestion would probably mean to 
entirely remove
method configure_dns_for_trust from 
ipatests/test_integration/tasks.py,

right? I'll have to verify this won't break anything else.

Lenka


Hi,

to get back to this issue: do we really want to have the DNS 
configuration
method removed? I mean, we no longer need it for our CI tests, 
with new

AD VMs
it works without it, but should somebody else with different 
setup run these
tests, they could experience failures because their AD domain 
would not be
configured in DNS by default and the test would no longer 
provide that
configuration. It doesn't feel right to delete something we 
needed before

but
don't need anymore, in case somebody else is depending on the 
same

configuration. But of course, I'll abide by your counsel.
In case the call on DNS configuration method really is 
removed, should I
remove even it's definition? It's not used anywhere else, so 
it would be

quite
logical.
Feel free to keep empty method around as a "hook" for other 
people. The

important thing is that it should do nothing by default.

So leave the method call, but erase method contents and let it 
just pass?

Fine with me. (List re-added.)


Ah, sorry for doing the wrong reply.
Anyway, fixed patch attached. I decided to do as you suggested - 
the original
DNS configuring function is now empty, I modified the comment to 
explain
significance of this strange thing. I also changed patch title to 
better

reflect proposed changes.

ACK if it passes your tests.

Yes, I've had no problems running the tests since I started to use 
this.

Thanks.


Pushed to master: 1d9e1521c59a5b43c2322892ce5cbe8cceff2790


Hi,

I just realized the same problem occurs in 4.3 branch, but the 
original patch was pushed to master only, hence I ask for second review.
The originally pushed patch attached, should not need any 
modifications for ipa-4-3 branch.


Ticket: https://fedorahosted.org/freeipa/ticket/6133

Thanks,
Lenka



Pushed to ipa-4-3: 40b1459ad0299e95331699be9684682fca02a570

-- 
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] restrict setkeytab operation

2016-07-26 Thread Simo Sorce
On Mon, 2016-07-25 at 11:26 -0400, Simo Sorce wrote:
> On Mon, 2016-07-25 at 11:10 -0400, Rob Crittenden wrote:
> > Simo Sorce wrote:
> > > On Mon, 2016-07-25 at 10:55 -0400, Rob Crittenden wrote:
> > >> Simo Sorce wrote:
> > >>> As described in #232 start restricting the use of the setkeytab
> > >>> operation to just the computers objects.
> > >>>
> > >>> I haven't tested this with older RHEL/CentOS machines that actully use
> > >>> the setkeytab operation as I do not have such an old VM handy right now.
> > >>>
> > >>> Meanwhile I'd like to know if ppl agree with this approach.
> > >>
> > >> What about services?
> > >
> > > Do we automatically acquire keytab for services in the old clients ?
> > >
> > > Are you thinking about scripted ipa-getkytab callouts ?
> > 
> > You are limiting access to host keytabs, what about service keytabs? 
> > Should they be or are they now similarly restricted?
> > 
> > Installers for something like Foreman may try to generate a service 
> > keytab in its installer, probably using admin credentials. I am planning 
> > to do the same in Openstack.
> 
> Ok I'll amend the patch to allow service keytabs to still use the
> setkeytab control still, and restrict only users.
> However note that the idea of using this method is that admin can change
> this default on their own, so they can restrict more or less if they
> want, to that end I need to remember how to set a default that we do not
> override in the update file.
> 
> Simo.
> 
Amended patch to allow services too.
Only users are excluded.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
From e91403837f1ca679898030591f3fcc64f9378b98 Mon Sep 17 00:00:00 2001
From: Simo Sorce 
Date: Mon, 25 Jul 2016 06:46:24 -0400
Subject: [PATCH] Restrict the old setkeytab operation

Allow it only to set computers/services keys by default. This is to allow
older clients to join a newer IPA Server and manage their service. But at
the same time prevent users from bypassing password policies by using this
old broken interface.

Ticket: https://fedorahosted.org/freeipa/ticket/232

Signed-off-by: Simo Sorce 
---
 daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c | 13 -
 install/updates/20-aci.update   |  4 
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index 3c2c44f6198bf74615fff1ae231a48bed77526ee..48880cdb74d2d12f016905c151f8fa9ad36c6d8a 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -1171,6 +1171,8 @@ done:
 return rc;
 }
 
+#define SETKEYS_OP_CHECK "ipaProtectedOperation;set_keys"
+
 /* Password Modify Extended operation plugin function */
 static int ipapwd_setkeytab(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg)
 {
@@ -1238,15 +1240,24 @@ static int ipapwd_setkeytab(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg)
 goto free_and_return;
 }
 
-/* Accesseck strategy:
+/* Access check strategy:
  * If the user has WRITE access, a new keytab can be set on the entry.
  * If not, then we fail immediately with insufficient access. This
  * means that we don't leak any useful information to the client such
  * as current password wrong, etc.
+ *
+ * In addition to the historic check, we now also check if the setkeytab
+ * operation is allowed at all.
  */
 allowed_access = is_allowed_to_access_attr(pb, bindDN, targetEntry,
"krbPrincipalKey", NULL,
SLAPI_ACL_WRITE);
+if (allowed_access) {
+/* check if we are allowed to *set* keys */
+allowed_access = is_allowed_to_access_attr(pb, bindDN, targetEntry,
+   SETKEYS_OP_CHECK, NULL,
+   SLAPI_ACL_WRITE);
+}
 if (!allowed_access) {
 LOG_FATAL("Access not allowed to set keytab on [%s]!\n",
   serviceName);
diff --git a/install/updates/20-aci.update b/install/updates/20-aci.update
index e9c10f54aadf5062c5a03f0d4b36343079462919..b12018004d6dfc51aabd5d6a006ee62660914dd6 100644
--- a/install/updates/20-aci.update
+++ b/install/updates/20-aci.update
@@ -113,6 +113,10 @@ add:aci: (targetattr="ipaProtectedOperation;write_keys")(version 3.0; acl "Group
 add:aci: (targetattr="ipaProtectedOperation;write_keys")(version 3.0; acl "Entities are allowed to rekey themselves"; allow(write) userdn="ldap:///self;;)
 add:aci: (targetattr="ipaProtectedOperation;write_keys")(version 3.0; acl "Admins are allowed to rekey any entity"; allow(write) groupdn = "ldap:///cn=admins,cn=groups,cn=accounts,$SUFFIX;;)
 add:aci: (targetfilter="(|(objectclass=ipaHost)(objectclass=ipaService))")(targetattr="ipaProtectedOperation;write_keys")(version 

Re: [Freeipa-devel] [PATCH 0194] harden the check for trust namespace overlap in new principals

2016-07-26 Thread Martin Babinsky

On 07/21/2016 12:56 PM, Martin Babinsky wrote:

'*-add-principal' would crash with error if the trusted domains did not
have any UPN suffixes or NETBIOS name associated with them. This patch
fixes that.

Big thanks to Milan who found and reported the issue during writing
tests for the feature.

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




Bump for review.

--
Martin^3 Babinsky

--
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 42-47][tests] RFE: Allow users to authenticate with alternative names

2016-07-26 Thread Martin Babinsky

On 07/25/2016 02:05 PM, Milan Kubík wrote:

On 07/25/2016 01:53 PM, Milan Kubík wrote:

Hi,

I'm sending the tests for kerberos principal aliases rfe. The tests
are implemented according to test plan [1] sent earlier.
Some of the patches implement modifications and extensions to previous
code to allow implement the tests themselves.

The patches can be cloned also from github [2].

[1]: http://www.freeipa.org/page/V4/Kerberos_principal_aliases/Test_Plan
[2]: https://github.com/apophys/freeipa/tree/krb5-principal-aliases-test

Cheers,





Self nack for 0047, the ldapconn fixture is not needed. New patch attached.
Git repo updated (force-push).

--
Milan Kubik





Hi Milan,

the tests seem to work as expected except 
`test_enterprise_principal_UPN_overlap_without_additional_suffix` which 
crashes on #6099. I have a few comments, however:


Patch 42:

1.)
+class KerberosAliasMixin:

make sure the class inherits from 'object' so that it behaves like 
new-style class in Python 2.


Also, I think that 'check_for_tracker' method is slightly redundant. If 
somebody would use the mixin directly, then he will still get 
"NotImplementedError" exceptions and see that he probably did something 
wrong.


If you really really want to treat to prevent the instantiation of the 
class, then use ABC module to turn it into proper abstract class.


But in this case it should IMHO be enough that you explained the class 
usage in the module docstring.


2.)
+def _make_add_alias_cmd(self):
+raise RuntimeError("The _make_add_alias_cmd method "
+   "is not implemented.")
+
+def _make_remove_alias_cmd(self):
+raise RuntimeError("The _make_remove_alias_cmd method "
+   "is not implemented."

Abstract methods should raise "NotImplementedError" exception.

3.)
is the 'options=None' kwarg in {add,remove}_principals methods 
necessary? I didn't see it anywhere in the later commits so I guess you 
can safely remove it. Better yet, you can just replace it with 
'**options' since all it does is to pass options to the 
`*-add/remove-principal` commands.


4.)
a nitpick but IIRC the mixin class should be listed before the actual 
hierarchy base so that MRO works as expected.


So instead

+class UserTracker(Tracker, KerberosAliasMixin):

It should say
+class UserTracker(KerberosAliasMixin, Tracker):

PATCH 43: LGTM

PATCH 44:

Please do not create any more utility modules. Either put it to existing 
'ipatests/util.py' or better yet, rename the module to something like 
'mock_trust' since the scope of it is to provide mocked trust entries.


PATCH 45:

It would be nice if you could add '-E' option in the same way to 
indicate enterprise principal name resolution.


PATCH 46: LGTM
PATCH 47:

1.)
+from ipatests.test_xmlrpc.test_range_plugin import (
+get_trust_dn, get_trusted_dom_dict, encode_mockldap_value)

Since you already introduced a module grouping all trust-mocking 
machinery in patch 44, you could extract these functions and put it 
there to improve code reuse.


2.)
I am not a big fan of duplicate code in 'trusted_domain' and 
'trusted_domain_with_suffix' fixtures. Use some module-level mapping for 
common attributes and add more specific attributes per fixture.


3.)
I would like to expand the test cases for AD realm namespace overlap 
checks. We should reject enterprise principals with suffixes coinciding 
with realm names, NETBIOS names, and UPN suffixes and I would like to 
test all three cases to get a full coverage.


--
Martin^3 Babinsky

--
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 0022][Tests] Prevent trust test failures cause by adding duplicate DNS forward zone

2016-07-26 Thread Lenka Doudova



On 06/30/2016 01:14 PM, Martin Basti wrote:



On 30.06.2016 12:58, Lenka Doudova wrote:



On 06/30/2016 12:51 PM, Petr Spacek wrote:

On 30.6.2016 12:32, Lenka Doudova wrote:


On 06/29/2016 07:49 PM, Petr Spacek wrote:

On 29.6.2016 18:52, Lenka Doudova wrote:

On 06/29/2016 06:51 PM, Petr Spacek wrote:

On 29.6.2016 18:48, Lenka Doudova wrote:

On 06/27/2016 11:05 AM, Lenka Doudova wrote:

On 06/27/2016 10:33 AM, Martin Babinsky wrote:

On 06/27/2016 10:28 AM, Petr Spacek wrote:

On 27.6.2016 10:26, Petr Spacek wrote:

On 27.6.2016 10:18, Martin Babinsky wrote:

On 06/27/2016 10:04 AM, Petr Vobornik wrote:

On 06/27/2016 09:42 AM, Lenka Doudova wrote:

Hi!

With newly created AD machines in Brno lab, existing 
trust tests

fail on
'ipa dnsforwardzone-add' command claiming the zone is 
already

present,
as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.

To prevent these failures I prepared attached patch, 
that will still
attempt to add the forward zone, but in case of non-zero 
return code
will check the message if it says that the forward zone 
is already

configured, and lets the tests continue, if it is so.


Lenka

Current approach expects that every error of ipa 
dnsforward-add here
will mean that the zone exists. So it might hide other 
issues - not

very
good.

On the other hand it is not very robust to parse error 
message.


Question for general audience: What do you think if IPA 
client's exit
status would be the IPA error code instead of "1" for 
every error.

E.g.
in DuplicateEntry case it's 4002.

Btw, this is not a NACK.

Well AFAIK the exit status on POSIX systems is encoded 
into a single

byte so
you cannot have the return value greater that 255. We 
would have to

devise
some mapping between our XMLRPC status codes and 
subprocess return

codes.

Some of our exceptions have defined return values outside 
plain '1',

e.g.
NotFound has return value of 2. It would be possible to 
extend this

concept on
other common errors.
Even more importantly, the forward zone is completely 
unnecessary

because
DNS
when DNS is set up properly. I would simply remove the
dnsforwardzone-add.


Grr, I meant this:
Even more importantly, the forward zone is completely 
unnecessary when

DNS is
set up properly. I would simply remove the dnsforwardzone-add.

+1, our tests should not fiddle with the provisioned 
environment as

much as
they sometimes do.

Well, I have nothing against removing it completely, but left 
it there just
because with previous AD machines with "wild" domains it was 
necessary.
Looking at the code, your suggestion would probably mean to 
entirely remove
method configure_dns_for_trust from 
ipatests/test_integration/tasks.py,

right? I'll have to verify this won't break anything else.

Lenka


Hi,

to get back to this issue: do we really want to have the DNS 
configuration
method removed? I mean, we no longer need it for our CI tests, 
with new

AD VMs
it works without it, but should somebody else with different 
setup run these
tests, they could experience failures because their AD domain 
would not be
configured in DNS by default and the test would no longer 
provide that
configuration. It doesn't feel right to delete something we 
needed before

but
don't need anymore, in case somebody else is depending on the same
configuration. But of course, I'll abide by your counsel.
In case the call on DNS configuration method really is removed, 
should I
remove even it's definition? It's not used anywhere else, so it 
would be

quite
logical.
Feel free to keep empty method around as a "hook" for other 
people. The

important thing is that it should do nothing by default.

So leave the method call, but erase method contents and let it 
just pass?

Fine with me. (List re-added.)


Ah, sorry for doing the wrong reply.
Anyway, fixed patch attached. I decided to do as you suggested - 
the original
DNS configuring function is now empty, I modified the comment to 
explain
significance of this strange thing. I also changed patch title to 
better

reflect proposed changes.

ACK if it passes your tests.


Yes, I've had no problems running the tests since I started to use this.
Thanks.


Pushed to master: 1d9e1521c59a5b43c2322892ce5cbe8cceff2790


Hi,

I just realized the same problem occurs in 4.3 branch, but the original 
patch was pushed to master only, hence I ask for second review.
The originally pushed patch attached, should not need any modifications 
for ipa-4-3 branch.


Ticket: https://fedorahosted.org/freeipa/ticket/6133

Thanks,
Lenka
From a4772a1b8cc31f0020c86acabbeb944c6d5269b7 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Thu, 30 Jun 2016 12:26:28 +0200
Subject: [PATCH] Tests: Remove DNS configuration from trust tests

Since DNS configuration is no longer needed for running trust tests, this method's contents are removed. Method is left empty as reference for others, should they have issues with DNS configuration.
---
 

Re: [Freeipa-devel] [PATCH 0003] Fix several small typos

2016-07-26 Thread Martin Basti



On 26.07.2016 11:55, Petr Spacek wrote:

On 25.7.2016 17:00, Ben Lipton wrote:

On 07/18/2016 04:54 PM, Lukas Slebodnik wrote:

On (18/07/16 16:38), Petr Spacek wrote:

On 14.7.2016 16:11, Ben Lipton wrote:

On 07/14/2016 04:09 AM, Alexander Bokovoy wrote:

On Wed, 13 Jul 2016, Ben Lipton wrote:

Nothing too exciting, just fixes a few typos I've noticed in comments.

ACK. However, please file a ticket and mention it in the commit message.

Is it worth the bureaucracy? I do not think so. We will certainly not backport
typo fixes in comments to older branches so I would say that ticket is
useless.

Just my two cents.


+1

LS


Necessary or not, as the ticket is created now, any objection to pushing the
patch?

ACK


Pushed to master: 99a702568d1bcaad2c0d1f83fdc2b485958b6e3d

--
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 0003] Fix several small typos

2016-07-26 Thread Petr Spacek
On 25.7.2016 17:00, Ben Lipton wrote:
> On 07/18/2016 04:54 PM, Lukas Slebodnik wrote:
>> On (18/07/16 16:38), Petr Spacek wrote:
>>> On 14.7.2016 16:11, Ben Lipton wrote:
 On 07/14/2016 04:09 AM, Alexander Bokovoy wrote:
> On Wed, 13 Jul 2016, Ben Lipton wrote:
>> Nothing too exciting, just fixes a few typos I've noticed in comments.
> ACK. However, please file a ticket and mention it in the commit message.
>>> Is it worth the bureaucracy? I do not think so. We will certainly not 
>>> backport
>>> typo fixes in comments to older branches so I would say that ticket is
>>> useless.
>>>
>>> Just my two cents.
>>>
>> +1
>>
>> LS
>>
> Necessary or not, as the ticket is created now, any objection to pushing the
> patch?

ACK

-- 
Petr^2 Spacek

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