Re: [Freeipa-devel] [PATCH 0126-0127] reset openldap client config to point to freshly promote replica

2016-01-14 Thread Martin Babinsky

On 01/13/2016 05:59 PM, Rob Crittenden wrote:

Martin Babinsky wrote:

fixes https://fedorahosted.org/freeipa/ticket/5584

In order to ensure consistent behavior with ipa-client-install, I opted
to reuse the configure_openldap_conf() function and restoring the config
from client sysrestore before modifying it.

If you think this approach is not optimal please propose an alternative
solution.


You could also just do an action set on URI to change the value, right?
It would need a new function but it would be very small.

If you do end up keeping this I'd want a new commit message for moving
the code to include why you're moving it (to avoid the need to deference
the ticket).

rob



Here's the patch that implements the change in URI directive. Please 
keep in mind that we not only have to change the URI to point to 
ourselves, we also have to do it in a way consistent with 
ipa-client-install, i.e. leave a comment with new URI if it was already 
set by third party.


Plain 'addifnotset' directive will not do, however, because then we end 
up with two comments, one original, and one pointing to ourselves. Plain 
'set' may rewrite the URI set by user and thus we would have to test its 
value anyway.


The correct handling of these cases coupled with a way IPAChangeConf is 
written results in a solution presented here.


The fact that it is not much shorter than configure_openldap_conf and is 
additionally pretty ugly (a fact at least partially caused by me not 
being very fluent in IPAChangeConf usage) led me to the conclusion that 
restoring original ldap.conf and reusing already wirrten code for 
reediting it anew with replica as URI is actually not that bad idea.


--
Martin^3 Babinsky
From 41d0441d19756a6809fa0c522f7c61980df127d6 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Thu, 14 Jan 2016 17:15:31 +0100
Subject: [PATCH] reset ldap.conf to point to newly installer replica after
 promotion

When promoting a client to replica reset openldap client config so that it no
longer uses remote master as default LDAP hosts but uses local connection to
replica. Also make sure that the behavior regarding editing of user-customized
config is consistent with the client installer.

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

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 7edee88e101ff59b516c97934e201bed69671cdb..f0f973160467b2c2b603302949e5c30a46d96953 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -443,6 +443,49 @@ def promote_sssd(host_name):
 root_logger.warning("SSSD service restart was unsuccessful.")
 
 
+def promote_openldap_conf(hostname, master):
+"""
+Reset the URI directive in openldap-client configuration file to point to
+newly promoted replica. If this directive was set by third party, then
+replace the added comment with the one pointing to replica
+
+:param hostname: replica FQDN
+:param master: FQDN of remote master
+"""
+
+ldap_conf = paths.OPENLDAP_LDAP_CONF
+
+ldap_change_conf = ipaclient.ipachangeconf.IPAChangeConf(
+"IPA replica installer")
+ldap_change_conf.setOptionAssignment((" ", "\t"))
+
+new_opts = []
+
+with open(ldap_conf, 'r') as f:
+old_opts = ldap_change_conf.parse(f)
+
+for opt in old_opts:
+if opt['type'] == 'comment' and master in opt['value']:
+continue
+elif (opt['type'] == 'option' and opt['name'] == 'URI' and
+master in opt['value']):
+continue
+new_opts.append(opt)
+
+change_opts = [
+{'action': 'addifnotset',
+ 'name': 'URI',
+ 'type': 'option',
+ 'value': 'ldaps://' + hostname}
+]
+
+try:
+ldap_change_conf.newConf(ldap_conf, new_opts)
+ldap_change_conf.changeConf(ldap_conf, change_opts)
+except Exception as e:
+root_logger.info("Failed to update {}: {}".format(ldap_conf, e))
+
+
 @common_cleanup
 def install_check(installer):
 options = installer
@@ -1373,6 +1416,7 @@ def promote(installer):
 custodia.import_dm_password(config.master_host_name)
 
 promote_sssd(config.host_name)
+promote_openldap_conf(config.host_name, config.master_host_name)
 
 # Switch API so that it uses the new servr configuration
 server_api = create_api(mode=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] [PATCHES] 0752-0757, 0759 Enable pylint --py3k

2016-01-14 Thread Petr Viktorin
On 01/12/2016 03:44 PM, Petr Viktorin wrote:
> 
>>> Hello I tried --py3k option and it doesn't print any error, can we
>>> enable that check by default to prevent python3 regressions?
>>>
>>> # ./make-lint --py3k
>>> No config file found, using default configuration
>>
>> Squash in the other attached patch to enable it by default.
> 
> Sorry, please ignore the squash patch -- if --py3k is to be on by
> default, I'll also need to add all needed packages to BuildRequires.

Actually, after all, that's not true -- the py3 libraries aren't needed
for pylint --py3k.
So, patches are ready for review as they are (with your choice of what
the default should be).


-- 
Petr Viktorin

-- 
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 0011-0012][RFE] ipa-replica-manage: automatically clean dangling RUVs

2016-01-14 Thread Stanislav Laznicka

Please see the rebased patches attached.

On 01/13/2016 02:01 PM, Martin Basti wrote:



On 18.12.2015 12:46, Stanislav Laznicka wrote:

Hi,

Attached are the patches for auto-find and clean of dangling 
(cs)ruvs. Currently, the cleaning of an RUV waits for all replicas to 
be online, even on --force. If that were an issue, I can make the 
command fail before trying to clean any of RUVs. However, the user is 
shown a replica is offline and is prompted to confirm the cleaning so 
the possible wait should not be a problem I believe.


Standa L.



Hello,

patches needs rebase, I cannot apply them.


From 46ccb4fc174d1cab8b6631e16689a1047b8dbb28 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka 
Date: Fri, 18 Dec 2015 10:30:44 +0100
Subject: [PATCH 1/2] Listing and cleaning RUV extended for CA suffix

https://fedorahosted.org/freeipa/ticket/5411
---
 install/tools/ipa-replica-manage | 36 +++-
 ipaserver/install/replication.py |  2 +-
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index e4af7b2fd9a40482dfa75d275d528221a1bc22ad..188e2c73a41aa1fd476475f74128b85b7383b09e 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -345,7 +345,7 @@ def del_link(realm, replica1, replica2, dirman_passwd, force=False):
 
 return True
 
-def get_ruv(realm, host, dirman_passwd, nolookup=False):
+def get_ruv(realm, host, dirman_passwd, nolookup=False, ca=False):
 """
 Return the RUV entries as a list of tuples: (hostname, rid)
 """
@@ -354,7 +354,10 @@ def get_ruv(realm, host, dirman_passwd, nolookup=False):
 enforce_host_existence(host)
 
 try:
-thisrepl = replication.ReplicationManager(realm, host, dirman_passwd)
+if ca:
+thisrepl = replication.get_cs_replication_manager(realm, host, dirman_passwd)
+else:
+thisrepl = replication.ReplicationManager(realm, host, dirman_passwd)
 except Exception as e:
 print("Failed to connect to server %s: %s" % (host, e))
 sys.exit(1)
@@ -362,7 +365,7 @@ def get_ruv(realm, host, dirman_passwd, nolookup=False):
 search_filter = '(&(nsuniqueid=---)(objectclass=nstombstone))'
 try:
 entries = thisrepl.conn.get_entries(
-api.env.basedn, thisrepl.conn.SCOPE_SUBTREE, search_filter,
+thisrepl.db_suffix, thisrepl.conn.SCOPE_SUBTREE, search_filter,
 ['nsds50ruv'])
 except errors.NotFound:
 print("No RUV records found.")
@@ -402,7 +405,7 @@ def get_rid_by_host(realm, sourcehost, host, dirman_passwd, nolookup=False):
 if '%s:389' % host == netloc:
 return int(rid)
 
-def clean_ruv(realm, ruv, options):
+def clean_ruv(realm, ruv, options, ca=False):
 """
 Given an RID create a CLEANALLRUV task to clean it up.
 """
@@ -412,7 +415,7 @@ def clean_ruv(realm, ruv, options):
 sys.exit("Replica ID must be an integer: %s" % ruv)
 
 servers = get_ruv(realm, options.host, options.dirman_passwd,
-  options.nolookup)
+  options.nolookup, ca=ca)
 found = False
 for (netloc, rid) in servers:
 if ruv == int(rid):
@@ -424,14 +427,21 @@ def clean_ruv(realm, ruv, options):
 sys.exit("Replica ID %s not found" % ruv)
 
 print("Clean the Replication Update Vector for %s" % hostname)
-print()
-print("Cleaning the wrong replica ID will cause that server to no")
-print("longer replicate so it may miss updates while the process")
-print("is running. It would need to be re-initialized to maintain")
-print("consistency. Be very careful.")
-if not options.force and not ipautil.user_input("Continue to clean?", False):
-sys.exit("Aborted")
-thisrepl = replication.ReplicationManager(realm, options.host,
+
+if not options.force:
+print()
+print("Cleaning the wrong replica ID will cause that server to no")
+print("longer replicate so it may miss updates while the process")
+print("is running. It would need to be re-initialized to maintain")
+print("consistency. Be very careful.")
+if not ipautil.user_input("Continue to clean?", False):
+sys.exit("Aborted")
+
+if ca:
+thisrepl = replication.get_cs_replication_manager(realm, options.host,
+options.dirman_passwd)
+else:
+thisrepl = replication.ReplicationManager(realm, options.host,
   options.dirman_passwd)
 thisrepl.cleanallruv(ruv)
 print("Cleanup task created")
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 19592e21f32b2013225036b3ce692f6cdee15a73..3221a1bd00bf9375d4348e5ba44d1645f0911b3e 100644
--- a/ipaserver/install/replication.py
+++ 

Re: [Freeipa-devel] [PATCH 0401] Pylint enable unbalanced tuple unpacking check

2016-01-14 Thread Martin Babinsky

On 01/12/2016 01:09 PM, Martin Basti wrote:

Patch attached.



ACK.

--
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] Should we split up ipa-client?

2016-01-14 Thread Jan Cholasta

On 14.1.2016 10:48, Petr Viktorin wrote:

On 01/14/2016 07:55 AM, Jan Cholasta wrote:

Hi,

On 13.1.2016 13:03, Martin Babinsky wrote:

On 01/13/2016 11:34 AM, Petr Viktorin wrote:

Hello,
I'm planning to port the ipa-client to Python 3, and I'm likely to end
up shaking out some dusty corners of the codebase, rather than doing the
minimal amount of work :)
So I'd like to get your opinions before I commit significant time to
this.


[...]

client-tools/
- man/*
- *.c
- *.h
- all the automake stuff
- current contents of ipa-install (Python scripts that go in /usr/sbin)


I would rather s/client-tools/client/, as this stuff goes into the
freeipa-*client* subpackage.


OK. It's just that there's no admintools/ or server/ either.

Putting the scripts into install/tools/ (or install/client/) is another
possibility.


Right. I guess we have to decide whether we want a directory layout 
based on the component/subpackage or not. install/tools/ works for me 
equally well.





I'm not sure if this is what you are suggesting or not, but I would like
the man page files to be in the same directory as the corresponding
source code files.


Do you mean not having the man/ subdirectory?


Yes. (I don't insist though.)

--
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 0404] Pylint: enable unpacking-non-sequence check

2016-01-14 Thread Martin Babinsky

On 01/14/2016 11:02 AM, Martin Basti wrote:

Patch attached.



ACK.

--
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] Should we split up ipa-client?

2016-01-14 Thread Petr Viktorin
On 01/14/2016 07:55 AM, Jan Cholasta wrote:
> Hi,
> 
> On 13.1.2016 13:03, Martin Babinsky wrote:
>> On 01/13/2016 11:34 AM, Petr Viktorin wrote:
>>> Hello,
>>> I'm planning to port the ipa-client to Python 3, and I'm likely to end
>>> up shaking out some dusty corners of the codebase, rather than doing the
>>> minimal amount of work :)
>>> So I'd like to get your opinions before I commit significant time to
>>> this.

[...]
>>> client-tools/
>>> - man/*
>>> - *.c
>>> - *.h
>>> - all the automake stuff
>>> - current contents of ipa-install (Python scripts that go in /usr/sbin)
> 
> I would rather s/client-tools/client/, as this stuff goes into the
> freeipa-*client* subpackage.

OK. It's just that there's no admintools/ or server/ either.

Putting the scripts into install/tools/ (or install/client/) is another
possibility.

> I'm not sure if this is what you are suggesting or not, but I would like
> the man page files to be in the same directory as the corresponding
> source code files.

Do you mean not having the man/ subdirectory?


-- 
Petr Viktorin

-- 
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 0125] IPA upgrade: move replication ACIs to the mapping tree entry

2016-01-14 Thread Martin Babinsky

On 01/13/2016 09:30 AM, Martin Babinsky wrote:

On 01/13/2016 07:18 AM, Jan Cholasta wrote:

On 12.1.2016 19:13, Martin Babinsky wrote:

commit 6ea868e172738bdd6a8fae34e65126cdd134bbbe broke replica install
and management on IPA servers upgraded from pre-4.3 version. The
attached patch fixes this.

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


Any reason to repeat the DN 3 times?

Besides that LGTM.



No other reason than not using brain during copy-pasting ACIs.

Attaching updated patch.




Attaching updated patch.

--
Martin^3 Babinsky
From 9ed4f643e63c371f0377848b0cf61b753301cbe8 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Tue, 12 Jan 2016 18:59:11 +0100
Subject: [PATCH] IPA upgrade: move replication ACIs to the mapping tree entry

During IPA server upgrade from pre-4.3 versions, the ACIs permitting
manipulation of replication agreements are removed from the
'cn="$SUFFIX",cn=mapping tree,cn=config' and 'cn=o\3Dipaca,cn=mapping
tree,cn=config'. However they are never re-added breaking management and
installation of replicas.

This patch modifies the update process so that the ACIs are first added to the
'cn=mapping tree,cn=config' and then removed from the child entries.

https://fedorahosted.org/freeipa/ticket/5575
---
 install/updates/20-aci.update | 8 
 1 file changed, 8 insertions(+)

diff --git a/install/updates/20-aci.update b/install/updates/20-aci.update
index cef842bbdf291762ef91d6be63c435b2f2161897..5ede14bbdc99c1ae5bcc589f5f83aa5ce094f2fd 100644
--- a/install/updates/20-aci.update
+++ b/install/updates/20-aci.update
@@ -66,6 +66,13 @@ add:aci:(targetattr="*")(version 3.0; acl "Admin can read all tasks"; allow (rea
 dn: cn=mapping tree,cn=config
 add:aci: (target = "ldap:///cn=meTo($$dn),cn=*,cn=mapping tree,cn=config")(targetattr = "objectclass || cn")(version 3.0; acl "Allow hosts to read their replication agreements"; allow(read, search, compare) userdn = "ldap:///fqdn=($$dn),cn=computers,cn=accounts,$SUFFIX";)
 
+
+# replication ACIs should reside in cn=mapping tree,cn=config and be common for both suffixes
+dn: cn=mapping tree,cn=config
+add: aci: (targetattr=*)(version 3.0;acl "permission:Add Replication Agreements";allow (add) groupdn = "ldap:///cn=Add Replication Agreements,cn=permissions,cn=pbac,$SUFFIX";)
+add: aci: (targetattr=*)(targetfilter="(|(objectclass=nsds5Replica)(objectclass=nsds5replicationagreement)(objectclass=nsDSWindowsReplicationAgreement)(objectClass=nsMappingTree))")(version 3.0; acl "permission:Modify Replication Agreements"; allow (read, write, search) groupdn = "ldap:///cn=Modify Replication Agreements,cn=permissions,cn=pbac,$SUFFIX";)
+add: aci: (targetattr=*)(targetfilter="(|(objectclass=nsds5replicationagreement)(objectclass=nsDSWindowsReplicationAgreement))")(version 3.0;acl "permission:Remove Replication Agreements";allow (delete) groupdn = "ldap:///cn=Remove Replication Agreements,cn=permissions,cn=pbac,$SUFFIX";)
+
 dn: cn="$SUFFIX",cn=mapping tree,cn=config
 remove:aci: (targetattr=*)(version 3.0;acl "permission:Add Replication Agreements";allow (add) groupdn = "ldap:///cn=Add Replication Agreements,cn=permissions,cn=pbac,$SUFFIX";)
 remove:aci: (targetattr=*)(targetfilter="(|(objectclass=nsds5Replica)(objectclass=nsds5replicationagreement)(objectclass=nsDSWindowsReplicationAgreement)(objectClass=nsMappingTree))")(version 3.0; acl "permission:Modify Replication Agreements"; allow (read, write, search) groupdn = "ldap:///cn=Modify Replication Agreements,cn=permissions,cn=pbac,$SUFFIX";)
@@ -76,6 +83,7 @@ remove:aci: (targetattr=*)(version 3.0;acl "permission:Add Replication Agreement
 remove:aci: (targetattr=*)(targetfilter="(|(objectclass=nsds5Replica)(objectclass=nsds5replicationagreement)(objectclass=nsDSWindowsReplicationAgreement)(objectClass=nsMappingTree))")(version 3.0; acl "permission:Modify Replication Agreements"; allow (read, write, search) groupdn = "ldap:///cn=Modify Replication Agreements,cn=permissions,cn=pbac,$SUFFIX";)
 remove:aci: (targetattr=*)(targetfilter="(|(objectclass=nsds5replicationagreement)(objectclass=nsDSWindowsReplicationAgreement))")(version 3.0;acl "permission:Remove Replication Agreements";allow (delete) groupdn = "ldap:///cn=Remove Replication Agreements,cn=permissions,cn=pbac,$SUFFIX";)
 
+
 # Removal of obsolete ACIs
 dn: cn=config
 # Replaced by 'System: Read Replication Agreements'
-- 
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 562-563] Fix ipa-sam to use the getkeytab control instead of the setkeytab control

2016-01-14 Thread Martin Basti



On 14.01.2016 08:24, Alexander Bokovoy wrote:

On Thu, 03 Dec 2015, Simo Sorce wrote:

The first patch is preparatory and is needed in general now that we want
top allow alias and use krbCanonicalName as the canonical name when
multiple values are avilable in krbPrincipalName.

The second patch changes slightly how the interdomain trust account is
created so that the getkeytab control can generate the proper key (with
the right salt) for interop reasons with AD. The change should be
upgrade safe because keys are generate at account creation so older
accounts lacking the alias won't be a problem.

Fixes ##5495

This patchset seems to fall through cracks -- it was ACKed but not
committed.
IIRC all simo's ACKed patches which haven't been pushed depend on simo's 
patch 560, which has no ACK


If not then, patches need rebase, they have missing blobs

--
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 562-563] Fix ipa-sam to use the getkeytab control instead of the setkeytab control

2016-01-14 Thread Alexander Bokovoy

On Thu, 14 Jan 2016, Martin Basti wrote:



On 14.01.2016 08:24, Alexander Bokovoy wrote:

On Thu, 03 Dec 2015, Simo Sorce wrote:

The first patch is preparatory and is needed in general now that we want
top allow alias and use krbCanonicalName as the canonical name when
multiple values are avilable in krbPrincipalName.

The second patch changes slightly how the interdomain trust account is
created so that the getkeytab control can generate the proper key (with
the right salt) for interop reasons with AD. The change should be
upgrade safe because keys are generate at account creation so older
accounts lacking the alias won't be a problem.

Fixes ##5495

This patchset seems to fall through cracks -- it was ACKed but not
committed.
IIRC all simo's ACKed patches which haven't been pushed depend on 
simo's patch 560, which has no ACK


If not then, patches need rebase, they have missing blobs

no, 560 is unrelated.

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


[Freeipa-devel] [PATCH 0404] Pylint: enable unpacking-non-sequence check

2016-01-14 Thread Martin Basti

Patch attached.
From b13ef989eabc361d17fdc5d1d6137a460e650d6b Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 17 Dec 2015 15:51:32 +0100
Subject: [PATCH] Enable pylint unpacking-non-sequence check

Enables check and marks occurences of runtime error in dcerpc.py as
false positive.
---
 ipaserver/dcerpc.py | 12 ++--
 make-lint   |  1 -
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py
index 13cb7767cc2ba303613a2d4065964d51599d728e..eb646b8af1fd7e9bfae3896424bd4ab66fbfb956 100644
--- a/ipaserver/dcerpc.py
+++ b/ipaserver/dcerpc.py
@@ -813,7 +813,7 @@ class TrustDomainInstance(object):
result = lsa.lsarpc(binding, self.parm, self.creds)
return result
except RuntimeError as e:
-   num, message = e.args
+   num, message = e.args  # pylint: disable=unpacking-non-sequence
raise assess_dcerpc_exception(num=num, message=message)
 
 def init_lsa_pipe(self, remote_host):
@@ -930,7 +930,7 @@ class TrustDomainInstance(object):
 self._policy_handle = self._pipe.OpenPolicy2(u"", objectAttribute, security.SEC_FLAG_MAXIMUM_ALLOWED)
 result = self._pipe.QueryInfoPolicy2(self._policy_handle, lsa.LSA_POLICY_INFO_DNS)
 except RuntimeError as e:
-num, message = e.args
+num, message = e.args  # pylint: disable=unpacking-non-sequence
 raise assess_dcerpc_exception(num=num, message=message)
 
 self.info['name'] = unicode(result.name.string)
@@ -943,7 +943,7 @@ class TrustDomainInstance(object):
 try:
 result = self._pipe.QueryInfoPolicy2(self._policy_handle, lsa.LSA_POLICY_INFO_ROLE)
 except RuntimeError as e:
-num, message = e.args
+num, message = e.args  # pylint: disable=unpacking-non-sequence
 raise assess_dcerpc_exception(num=num, message=message)
 
 self.info['is_pdc'] = (result.role == lsa.LSA_ROLE_PRIMARY)
@@ -1069,7 +1069,7 @@ class TrustDomainInstance(object):
 res = self._pipe.QueryTrustedDomainInfoByName(self._policy_handle, dname, lsa.LSA_TRUSTED_DOMAIN_INFO_FULL_INFO)
 self._pipe.DeleteTrustedDomain(self._policy_handle, res.info_ex.sid)
 except RuntimeError as e:
-num, message = e.args
+num, message = e.args  # pylint: disable=unpacking-non-sequence
 # Ignore anything but access denied (NT_STATUS_ACCESS_DENIED)
 if num == -1073741790:
 raise access_denied_error
@@ -1077,7 +1077,7 @@ class TrustDomainInstance(object):
 try:
 trustdom_handle = self._pipe.CreateTrustedDomainEx2(self._policy_handle, info, self.auth_info, security.SEC_STD_DELETE)
 except RuntimeError as e:
-num, message = e.args
+num, message = e.args  # pylint: disable=unpacking-non-sequence
 raise assess_dcerpc_exception(num=num, message=message)
 
 # We should use proper trustdom handle in order to modify the
@@ -1119,7 +1119,7 @@ class TrustDomainInstance(object):
)
 return result
 except RuntimeError as e:
-num, message = e.args
+num, message = e.args  # pylint: disable=unpacking-non-sequence
 raise assess_dcerpc_exception(num=num, message=message)
 
 result = retrieve_netlogon_info_2(None, self,
diff --git a/make-lint b/make-lint
index 4d21c6e827ed610e57a089b0203abca6dc9619c4..b1780f94adcb56e1b2c5ec014ad73d203b96c351 100755
--- a/make-lint
+++ b/make-lint
@@ -280,7 +280,6 @@ def main():
 linter.set_option('disable', 'unused-variable')
 linter.set_option('disable', 'useless-else-on-loop')
 linter.set_option('disable', 'unbalanced-tuple-unpacking')
-linter.set_option('disable', 'unpacking-non-sequence')
 
 linter.set_option('enable', 'C')
 linter.set_option('disable', 'bad-classmethod-argument')
-- 
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 0402] Warn user about possibility to loss CA, KRA, DNSSEC master during uninstall

2016-01-14 Thread David Kupka

On 13/01/16 18:18, Martin Basti wrote:

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

Patch attached.




Thanks for the patch, works for me, ACK.

--
David Kupka

--
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] Pylint enable unbalanced tuple unpacking check

2016-01-14 Thread Martin Basti



On 14.01.2016 11:23, Martin Babinsky wrote:

On 01/12/2016 01:09 PM, Martin Basti wrote:

Patch attached.



ACK.


Pushed to master: 2320be18a34628fa6d05ffc42e695da5dd6dab6e

--
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][test] CI: fix regression in task.install_kra

2016-01-14 Thread Martin Basti

Regression caused by commit c4b9b295d8184694c50c0d56051e0273445c98ec

Patch attached.

One-liner rule applied.

Pushed to:
master: 26899c91afd36b39908a3a73363aff1787c9dc07
ipa-4-3: f3c1856d566cdcce0d411bc59efc63ddd42e5d07

From c47dde756f44c2810646bdf345433b1544d5adda Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 14 Jan 2016 10:23:11 +0100
Subject: [PATCH] CI test: fix regression in task.install_kra

ipa-kra-install needs directory manager password

Regression caused by c4b9b295d8184694c50c0d56051e0273445c98ec
---
 ipatests/test_integration/tasks.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 346c96ea31185251209f471d1a626b9958925698..6fdcf9a9019244d0ee5ae9f4d456d72df7c0566e 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -957,7 +957,7 @@ def ipa_restore(master, backup_path):
 def install_kra(host, domain_level=None, first_instance=False, raiseonerr=True):
 if domain_level is None:
 domain_level = domainlevel(host)
-command = ["ipa-kra-install", "-U"]
+command = ["ipa-kra-install", "-U", "-p", host.config.dirman_password]
 if domain_level == DOMAIN_LEVEL_0 and not first_instance:
 replica_file = get_replica_filename(host)
 command.append(replica_file)
-- 
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 0404] Pylint: enable unpacking-non-sequence check

2016-01-14 Thread Martin Basti



On 14.01.2016 11:22, Martin Babinsky wrote:

On 01/14/2016 11:02 AM, Martin Basti wrote:

Patch attached.



ACK.


Pushed to master: 267bad10a81f101db8b645abf01b2cdd62c91775

--
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 0126-0127] reset openldap client config to point to freshly promote replica

2016-01-14 Thread Martin Babinsky

On 01/13/2016 05:59 PM, Rob Crittenden wrote:

Martin Babinsky wrote:

fixes https://fedorahosted.org/freeipa/ticket/5584

In order to ensure consistent behavior with ipa-client-install, I opted
to reuse the configure_openldap_conf() function and restoring the config
from client sysrestore before modifying it.

If you think this approach is not optimal please propose an alternative
solution.


You could also just do an action set on URI to change the value, right?
It would need a new function but it would be very small.

If you do end up keeping this I'd want a new commit message for moving
the code to include why you're moving it (to avoid the need to deference
the ticket).

rob

In the hindsight my approach is probably overkill for this case. I will 
rework the patches to only set the URI directive after promotion.


--
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] [TEST][PATCH 0019] A proper fix for reverse-zone creation in integration tests

2016-01-14 Thread Oleg Fayans

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 249db32dc5a7ce1e6df826c68f2919c07d953c45 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Thu, 14 Jan 2016 14:59:37 +0100
Subject: [PATCH] fixed an issue with master installation not creating reverse
 zone

When resolv.conf is set to point to the master's ip before installation, the
ipa-server-install does not create a reverse zone for it's ip even despite
--auto-reverse option provided. The fix is not to mess around with resolv.conf
before master installation.
---
 ipatests/test_integration/tasks.py | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 6fdcf9a9019244d0ee5ae9f4d456d72df7c0566e..a1d6cb88a1bfe2ac8c2fae9d758da726a5536ce2 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -88,11 +88,12 @@ def allow_sync_ptr(host):
  raiseonerr=False)
 
 
-def apply_common_fixes(host):
+def apply_common_fixes(host, fix_resolv=True):
 fix_etc_hosts(host)
 fix_hostname(host)
 modify_nm_resolv_conf_settings(host)
-fix_resolv_conf(host)
+if fix_resolv:
+fix_resolv_conf(host)
 
 
 def backup_file(host, filename):
@@ -264,7 +265,7 @@ def install_master(host, setup_dns=True, setup_kra=False):
 host.collect_log(paths.SLAPD_INSTANCE_ERROR_LOG_TEMPLATE % inst)
 host.collect_log(paths.SLAPD_INSTANCE_ACCESS_LOG_TEMPLATE % inst)
 
-apply_common_fixes(host)
+apply_common_fixes(host, fix_resolv=False)
 fix_apache_semaphores(host)
 
 args = [
-- 
2.4.3

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

Re: [Freeipa-devel] [PATCH 0402] Warn user about possibility to loss CA, KRA, DNSSEC master during uninstall

2016-01-14 Thread Martin Basti



On 14.01.2016 14:18, David Kupka wrote:

On 13/01/16 18:18, Martin Basti wrote:

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

Patch attached.




Thanks for the patch, works for me, ACK.


Pushed to:
master: 58c42ddac0964a8cce7c1e1faa7516da53f028ad
ipa-4-3: 56d921f359a667fc3c5b33ccf6b3767da06f64ab

--
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 0011-0012][RFE] ipa-replica-manage: automatically clean dangling RUVs

2016-01-14 Thread Rob Crittenden
Stanislav Laznicka wrote:
> Please see the rebased patches attached.
> 
> On 01/13/2016 02:01 PM, Martin Basti wrote:
>>
>>
>> On 18.12.2015 12:46, Stanislav Laznicka wrote:
>>> Hi,
>>>
>>> Attached are the patches for auto-find and clean of dangling
>>> (cs)ruvs. Currently, the cleaning of an RUV waits for all replicas to
>>> be online, even on --force. If that were an issue, I can make the
>>> command fail before trying to clean any of RUVs. However, the user is
>>> shown a replica is offline and is prompted to confirm the cleaning so
>>> the possible wait should not be a problem I believe.
>>>
>>> Standa L.
>>>
>>>
>> Hello,
>>
>> patches needs rebase, I cannot apply them.

Will this confuse people? Currently, for good or bad, there are two
commands for managing the two different topologies. This mixes some CA
work into ipa-replica-manage.

rob

-- 
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 0011-0012][RFE] ipa-replica-manage: automatically clean dangling RUVs

2016-01-14 Thread Petr Vobornik

On 01/14/2016 04:16 PM, Ludwig Krispenz wrote:


On 01/14/2016 03:59 PM, Stanislav Laznicka wrote:

On 01/14/2016 03:21 PM, Rob Crittenden wrote:

Stanislav Laznicka wrote:

Please see the rebased patches attached.

On 01/13/2016 02:01 PM, Martin Basti wrote:


On 18.12.2015 12:46, Stanislav Laznicka wrote:

Hi,

Attached are the patches for auto-find and clean of dangling
(cs)ruvs. Currently, the cleaning of an RUV waits for all replicas to
be online, even on --force. If that were an issue, I can make the
command fail before trying to clean any of RUVs. However, the user is
shown a replica is offline and is prompted to confirm the cleaning so
the possible wait should not be a problem I believe.

Standa L.



Hello,

patches needs rebase, I cannot apply them.

Will this confuse people? Currently, for good or bad, there are two
commands for managing the two different topologies. This mixes some CA
work into ipa-replica-manage.

rob


Well, in the patch, I was just following the discussion at
https://fedorahosted.org/freeipa/ticket/5411. Ludwig mentions that
ipa-csreplica-manage should go deprecated and does not want to enhance
it. Also, the only thing the code does is removing trash from the ds
so it makes sense to me to do it in just one command, as well as the
users might expect that, too.

I guess it would be possible to add an option that would select which
of the subtrees should be cleaned of RUVs. It should stay as one
command nonetheless. Adding such an option for this command would then
probably mean all the commands should have it as it would make more
sense, though.

Let me add Petr and Ludwig to CC: as they both had inputs on keeping
the command in just ipa-replica-manage.

yes, that was the idea to keep ipa-csreplica-manage (which does not have
clean-ruv,..) for domain-level 0, but not add new features. Also
"ipa-replica-manage del" now triggers the ruv cleaning of ipaca



Yes, ipa-csreplica-manage should be deprecated.

I think that one of the reasons why dangling CA RUVs are not uncommon is 
that users forget about `ipa-csreplica-manage del` command when removing 
a replica.


New `ipa-replica-manage del` also removes replication agreements and 
therefore cleans RUVs of CA suffix (on domain level 1). In this context 
it is not inconsistent.


Btw, one of the good example why this commands will be helpful is 
following bz, especially a sentence in: 
https://bugzilla.redhat.com/show_bug.cgi?id=1295971#c5

"""
I had some mistakes to clean some valid RUV, for example, 52 for eupre1
"""

We should think about list-clean-ruv and abort-clean-ruv commands. There 
is no counterpart for CA suffix now. Could be in different patch.


With clean-dangling-ruvs command it would be good to deprecate clean-ruv 
command of ipa-replica-manage - should be different patch.


I'm not sure if it should abort if some replica is down. Maybe yes until 
https://fedorahosted.org/freeipa/ticket/5396 is fixed.


The path set misses update of man page.
--
Petr Vobornik

--
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 0121] consider IPA master removed from topology when request for host TGT fails

2016-01-14 Thread Simo Sorce
On Wed, 2016-01-13 at 17:31 +0100, Martin Babinsky wrote:
> On 01/13/2016 10:31 AM, Martin Babinsky wrote:
> > On 01/07/2016 05:38 PM, Martin Babinsky wrote:
> >> On 01/07/2016 05:37 PM, Martin Babinsky wrote:
> >>> https://fedorahosted.org/freeipa/ticket/5584
> >>>
> >> And the patch is here.
> >>
> >>
> >>
> > self-NACK, there may be a better way to handle this. I will do some
> > investigation and send updated patch.
> >
> Attaching updated patch.

A failure to obtain a tgt may be due to other reasons (for example the
KDC crashed), why are you trying to use this test ?
Isn't it sufficient to see there is no host entry in the directory ?

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

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


Re: [Freeipa-devel] [PATCH 0011-0012][RFE] ipa-replica-manage: automatically clean dangling RUVs

2016-01-14 Thread Stanislav Laznicka

On 01/14/2016 03:21 PM, Rob Crittenden wrote:

Stanislav Laznicka wrote:

Please see the rebased patches attached.

On 01/13/2016 02:01 PM, Martin Basti wrote:


On 18.12.2015 12:46, Stanislav Laznicka wrote:

Hi,

Attached are the patches for auto-find and clean of dangling
(cs)ruvs. Currently, the cleaning of an RUV waits for all replicas to
be online, even on --force. If that were an issue, I can make the
command fail before trying to clean any of RUVs. However, the user is
shown a replica is offline and is prompted to confirm the cleaning so
the possible wait should not be a problem I believe.

Standa L.



Hello,

patches needs rebase, I cannot apply them.

Will this confuse people? Currently, for good or bad, there are two
commands for managing the two different topologies. This mixes some CA
work into ipa-replica-manage.

rob

Well, in the patch, I was just following the discussion at 
https://fedorahosted.org/freeipa/ticket/5411. Ludwig mentions that 
ipa-csreplica-manage should go deprecated and does not want to enhance 
it. Also, the only thing the code does is removing trash from the ds so 
it makes sense to me to do it in just one command, as well as the users 
might expect that, too.


I guess it would be possible to add an option that would select which of 
the subtrees should be cleaned of RUVs. It should stay as one command 
nonetheless. Adding such an option for this command would then probably 
mean all the commands should have it as it would make more sense, though.


Let me add Petr and Ludwig to CC: as they both had inputs on keeping the 
command in just ipa-replica-manage.


--
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 0011-0012][RFE] ipa-replica-manage: automatically clean dangling RUVs

2016-01-14 Thread Ludwig Krispenz


On 01/14/2016 03:59 PM, Stanislav Laznicka wrote:

On 01/14/2016 03:21 PM, Rob Crittenden wrote:

Stanislav Laznicka wrote:

Please see the rebased patches attached.

On 01/13/2016 02:01 PM, Martin Basti wrote:


On 18.12.2015 12:46, Stanislav Laznicka wrote:

Hi,

Attached are the patches for auto-find and clean of dangling
(cs)ruvs. Currently, the cleaning of an RUV waits for all replicas to
be online, even on --force. If that were an issue, I can make the
command fail before trying to clean any of RUVs. However, the user is
shown a replica is offline and is prompted to confirm the cleaning so
the possible wait should not be a problem I believe.

Standa L.



Hello,

patches needs rebase, I cannot apply them.

Will this confuse people? Currently, for good or bad, there are two
commands for managing the two different topologies. This mixes some CA
work into ipa-replica-manage.

rob

Well, in the patch, I was just following the discussion at 
https://fedorahosted.org/freeipa/ticket/5411. Ludwig mentions that 
ipa-csreplica-manage should go deprecated and does not want to enhance 
it. Also, the only thing the code does is removing trash from the ds 
so it makes sense to me to do it in just one command, as well as the 
users might expect that, too.


I guess it would be possible to add an option that would select which 
of the subtrees should be cleaned of RUVs. It should stay as one 
command nonetheless. Adding such an option for this command would then 
probably mean all the commands should have it as it would make more 
sense, though.


Let me add Petr and Ludwig to CC: as they both had inputs on keeping 
the command in just ipa-replica-manage.
yes, that was the idea to keep ipa-csreplica-manage (which does not have 
clean-ruv,..) for domain-level 0, but not add new features. Also 
"ipa-replica-manage del" now triggers the ruv cleaning of ipaca


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