Re: [Freeipa-devel] [PATCHES 523-525] replica install: add remote connection check over API

2015-12-10 Thread Jan Cholasta 

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.

--
Jan Cholasta
From efe9776a0484fb0bc2faa528ac2d104f6d28e1ca Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 9 Dec 2015 08:17:07 +0100
Subject: [PATCH 1/3] build: put oddjob scripts into separate directory

https://fedorahosted.org/freeipa/ticket/5497
---
 freeipa.spec.in| 4 
 install/oddjob/Makefile.am | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index a60d9b6..cd986d1 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -434,6 +434,9 @@ make client-install DESTDIR=%{buildroot}
 mkdir -p %{buildroot}%{_usr}/share/ipa
 
 %if ! %{ONLY_CLIENT}
+# FIXME: https://bugzilla.redhat.com/show_bug.cgi?id=1289930
+mv %{buildroot}%{_libexecdir}/ipa/oddjob/com.redhat.idm.trust-fetch-domains %{buildroot}%{_libexecdir}/ipa/com.redhat.idm.trust-fetch-domains
+
 # Remove .la files from libtool - we don't want to package
 # these files
 rm %{buildroot}/%{plugin_dir}/libipa_pwd_extop.la
@@ -740,6 +743,7 @@ fi
 %{_libexecdir}/ipa/ipa-dnskeysync-replica
 %{_libexecdir}/ipa/ipa-ods-exporter
 %{_libexecdir}/ipa/ipa-httpd-kdcproxy
+%dir %{_libexecdir}/ipa/oddjob
 %ghost %verify(not owner group) %dir %{_sharedstatedir}/kdcproxy
 %dir %attr(0755,root,root) %{_sysconfdir}/ipa/kdcproxy
 %config(noreplace) %{_sysconfdir}/sysconfig/ipa_memcached
diff --git a/install/oddjob/Makefile.am b/install/oddjob/Makefile.am
index 9dde10c..5cdaf2b 100644
--- a/install/oddjob/Makefile.am
+++ b/install/oddjob/Makefile.am
@@ -1,6 +1,6 @@
 NULL =
 
-oddjobdir = $(libexecdir)/ipa
+oddjobdir = $(libexecdir)/ipa/oddjob
 oddjobconfdir = $(sysconfdir)/oddjobd.conf.d
 dbusconfdir = $(sysconfdir)/dbus-1/system.d
 
-- 
2.4.3

From 60fdccb6618536d92384ccde5553c5ca2650ea7c Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 9 Dec 2015 08:18:21 +0100
Subject: [PATCH 2/3] replica install: add remote connection check over API

Add server_conncheck command which calls ipa-replica-conncheck --replica
over oddjob.

https://fedorahosted.org/freeipa/ticket/5497
---
 API.txt|   8 ++
 VERSION|   4 +-
 freeipa.spec.in|   9 +-
 install/oddjob/Makefile.am |   3 +
 .../etc/dbus-1/system.d/org.freeipa.server.conf|  21 
 install/oddjob/etc/oddjobd.conf.d/ipa-server.conf  |  20 
 install/oddjob/org.freeipa.server.conncheck|   2 +
 install/tools/ipa-ca-install   |   6 +
 install/tools/ipa-replica-conncheck| 131 ++---
 install/updates/90-post_upgrade_plugins.update |   1 -
 ipalib/messages.py |  10 ++
 ipalib/plugins/server.py   |  70 ++-
 ipaserver/install/adtrustinstance.py   |  19 ---
 ipaserver/install/ca.py|   2 +-
 ipaserver/install/httpinstance.py  |  26 
 ipaserver/install/installutils.py  |  12 --
 ipaserver/install/plugins/adtrust.py   |  21 
 ipaserver/install/replication.py

Re: [Freeipa-devel] [PATCH 0098-0099] domain level 1 topology checks during IPA server uninstall

2015-12-10 Thread Jan Cholasta 

On 9.12.2015 16:22, Martin Babinsky wrote:

On 12/09/2015 03:48 PM, Jan Cholasta wrote:

On 2.12.2015 14:19, Martin Basti wrote:



On 02.12.2015 14:10, Martin Basti wrote:



On 02.12.2015 14:08, Martin Babinsky wrote:

On 12/02/2015 10:45 AM, Martin Babinsky wrote:

On 12/01/2015 02:40 PM, Martin Babinsky wrote:

On 11/30/2015 08:34 PM, Martin Basti wrote:



On 30.11.2015 18:41, Martin Babinsky wrote:

On 11/30/2015 06:15 PM, Martin Basti wrote:



On 30.11.2015 16:43, Martin Babinsky wrote:

On 11/30/2015 12:31 PM, Jan Cholasta wrote:

Hi,

On 27.11.2015 14:58, Martin Babinsky wrote:

On 11/19/2015 06:19 PM, Martin Babinsky wrote:

These two patches fix the following tickets:

https://fedorahosted.org/freeipa/ticket/5377
https://fedorahosted.org/freeipa/ticket/5409

I have added a new option '--ignore-disconnected-topology'
which
forces
IPA master uninstall despite reported errors in topology.
I'm not
quite
sure if we want to flood ipa-server-install with
uninstall-specific
options, maybe it is better to skip the check in unattended
mode
and
just print a warning about disconnected topology and what
to do
about
it.

I would like to hear your opinions about this.




Attaching rebased and updated patches.


Patch 0098: LGTM


Patch 0099:

a) This check should be done in Server.__init__() rather than
install_check():

+if options.ignore_disconnected_topology:
+print("'--ignore-disconnected-topology' is used only
during "
+  "uninstallation")
+sys.exit(1)


b)
s/--ignore-disconnected-topology/--ignore-topology-disconnect/,
for
consistency with other options, e.g. --no-ui-redirect.

Maybe even shorten it to --ignore-topology? But we probably
don't
want
people to use this option much, so it might be better to
keep it
long?


I would rather leave it with the long option name, it is more
apparent
what this switch should be around.


c) I'm fine with uninstall options, you can remove the TODO:

+# TODO: ask jcholast about uninstallation options


Honza



Attaching updated patches.


NACK

ipa-server-install --uninstall

2015-11-30T17:14:30Z DEBUG Destroyed connection
context.ldap2_140081152041808
2015-11-30T17:14:30Z DEBUG Traceback (most recent call last):
   File
"/usr/lib/python2.7/site-packages/ipapython/install/common.py",
line 91, in _handle_exception
 super(Continuous, self)._handle_exception(exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 387, in _handle_exception
 six.reraise(*exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 439, in _handle_exception
 super(ComponentBase, self)._handle_exception(exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 387, in _handle_exception
 six.reraise(*exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 355, in __runner
 step()
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 352, in 
 step = lambda: next(self.__gen)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/util.py",
line 81, in run_generator_with_yield_from
 six.reraise(*exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/util.py",
line 59, in run_generator_with_yield_from
 value = gen.send(prev_value)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/common.py",
line 71, in _uninstall
 for nothing in self._uninstaller(self.parent):
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",




line 1409, in main
 uninstall_check(self)
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",




line 265, in decorated
 func(installer)
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",




line 1140, in uninstall_check
 api, masters, options.ignore_disconnected_topology)
AttributeError: 'uninstaller(Server)' object has no attribute
'ignore_disconnected_topology'

2015-11-30T17:14:30Z ERROR 'uninstaller(Server)' object has no
attribute
'ignore_disconnected_topology'
2015-11-30T17:14:30Z INFO The ipa-server-install command was
successful



Sorry I have failed horribly during option rename. Attaching patch
that should actually work.


functional ACK

Attaching rebased patches reflecting the recent changes in the
handling
of managed topology suffixes handling.






Jan had some more suggestions to the patches. Attaching updated
version.




Attaching updated patch 99 with fixed error message.


Pushed to master: b8c619a7139bd7b65caa03b68431e22791ff19bf


ACK :-)


I was doing some unrelated testing with domain level 0 and forgot to
remove --ignore-topology disconnect from my command line before
uninstalling server, which gave me an error that the option cannot be
used in domain level 0 and I had to re-run ipa-server-install
--uninstall with the option removed.

IMO it would be better for UX if the option was ignored in domain level
0, since topology disconn

Re: [Freeipa-devel] [PATCHES 523-525] replica install: add remote connection check over API

2015-12-10 Thread Jan Cholasta 

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.


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

[Freeipa-devel] [PATCH 0376] KRA: add RA cert during replica promotion

2015-12-10 Thread Martin Basti 

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

patch attached.
From 75b69aee3e3911cdf66c0d6dd40c49fd0da61492 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 10 Dec 2015 13:46:07 +0100
Subject: [PATCH] Install RA cert during replica promotion

This cert is needed with KRA to be able store and retrieve secrets.

https://fedorahosted.org/freeipa/ticket/5512
---
 ipaserver/install/cainstance.py| 4 
 ipaserver/install/server/replicainstall.py | 8 
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 56ec3fe74e8d4adfe17f46a62f705021f6a81f75..99582b5e0d33afc1c97a8e5067a3a10498651869 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1335,12 +1335,8 @@ class CAInstance(DogtagInstance):
 self.step("setting audit signing renewal to 2 years",
   self.set_audit_renewal)
 
-self.step("configure certmonger for renewals",
-  self.configure_certmonger_renewal)
 self.step("configure certificate renewals",
   self.configure_renewal)
-self.step("configure RA certificate renewal",
-  self.configure_agent_renewal)
 self.step("configure Server-Cert certificate renewal",
   self.track_servercert)
 self.step("Configure HTTP to proxy connections",
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4f239aacd50ab8692830a6f4505c66ba9b1518b2..bd2de16fc4a0ff5a050eebd942682e13de0b1583 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1282,6 +1282,14 @@ def promote(installer):
  installer._ca_enabled)
 custodia.create_replica(config.master_host_name)
 
+if installer._ca_enabled:
+CA = cainstance.CAInstance(config.realm_name, certs.NSS_DIR)
+
+CA.configure_certmonger_renewal()
+CA.configure_agent_renewal()
+cainstance.export_kra_agent_pem()
+CA.fix_ra_perms()
+
 krb = install_krb(config,
   setup_pkinit=not options.no_pkinit,
   promote=True)
-- 
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 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-12-10 Thread Petr Spacek 
On 10.12.2015 17:31, David Kupka wrote:
> On 09/12/15 18:55, Petr Spacek wrote:
>> On 9.12.2015 13:37, David Kupka wrote:
>>> On 08/12/15 15:24, Petr Spacek wrote:
 On 8.12.2015 12:19, David Kupka wrote:
> On 08/12/15 08:56, Petr Spacek wrote:
>> On 7.12.2015 14:41, David Kupka wrote:
>>> +def is_host_resolvable(fqdn):
>>> +if not isinstance(fqdn, DNSName):
>>> +fqdn = DNSName(fqdn)
>>> +for rdtype in (rdatatype.A, rdatatype.):
>>> +try:
>>> +resolver.query(fqdn.make_absolute(), rdtype)
>>> +except DNSException:
>>> +continue
>>> +else:
>>> +return True
>>> +
>>> +return False
>>>
>>
>> NACK, you are re-introducing duplicate function which was removed in
>> 498471e4aed1367b72cd74d15811d0584a6ee268.
>>
>> Please amend the patch ASAP to use new verify_host_resolvable() function
>> so I
>> can test it and get it into 4.3.
>>
> Updated patches attached.

 Hmm, my review script screams:

 Detected base branch:   origin/master
 Checks will be made against following base commit: 848912a add missing
 /ipaplatform/constants.py to .gitignore
 Writing API to API.txt
 ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual
 indentation
 ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for
 visual indent
 ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual
 indentation
 ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for
 visual indent
 ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1
 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for
 visual indent
 ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1
 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters)
 ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters)
 ./ipaserver/install/bindinstance.py:49:9: E128 continuation line
 under-indented for visual indent
 ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79
 characters)
 ./ipaserver/install/bindinstance.py:438:19: E128 continuation line
 under-indented for visual indent
 ./ipaserver/install/server/common.py:271:63: E261 at least two spaces 
 before
 inline comment
 ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79
 characters)
 ERROR: PEP8 --diff failed, continuing ...
 ERROR: API.txt was changed without a change in VERSION, continuing ...
 Summary of detected errors:
ERROR: PEP8 --diff failed
ERROR: API.txt was changed without a change in VERSION

 Please fix it :-)

 Make make this more pleasant for you, you can use (and review) my attached
 patch. It adds 'review' target to Makefile, it will do the same checks as
 I do.

>>>
>>> Unfortunately your tool does not work for me (output w/ -o xtrace attached).
>>> Anyway I have incremented API version and fixed most of the pep8 errors 
>>> except
>>> for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the
>>> file and E501 twice in ipapython/ipautil.py because IMO breaking string
>>> constant into multiple lines does not help readability.
>>>
>>> Updated patches also attached.
>>
>> We are almost there, but NACK for now.
>>
>> 1) Following attempt to install IPA explodes. Please note that
>> dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server
>> before installation is started, so it gives 'timeout' or possibly SERVFAIL.
>>
>> # ipa-server-install --ds-password=root4lab --admin-password=root4lab
>> --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap
>> --domain=dom-245.idm.lab.eng.brq.redhat.com
>> --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM
>> [...]
>>
>> Configuring DNS (named)
>>[1/11]: generating rndc key file
>>[2/11]: adding DNS container
>>[3/11]: setting up our zone
>>[error] InvocationError: DNS check for domain
>> dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after
>> 30.000453949 seconds. Please make sure that the domain is properly delegated
>> to this IPA server.
>> ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain
>> dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after
>> 30.000453949 seconds. Please make sure that the domain is properly delegated
>> to this IPA server.
>> ipa.ipapython.install.cli.install_tool(Server): ERRORThe
>> ipa-server-install command failed. See /var/log/ipaserver-install.log for 
>> more
>> information
>>
>> 2015-12-09T17:15:37Z DEBUG   File
>> "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in 
>> execute
>>  return_value = self.run()
>>File "/usr/lib/python2.7/site-pa

Re: [Freeipa-devel] [PATCH 0069] Add 'review' target for make

2015-12-10 Thread Petr Spacek 
On 9.12.2015 15:30, Petr Spacek wrote:
> Hello,
> 
> this patch automates some of sanity checks proposed by Petr Vobornik.
> 
> 'make review' should be used in root of clean Git tree which has patches under
> review applied.
> 
> Magic in review.sh attempts to detect nearest remote branch which can be used
> as diff base for review. Please see review.sh for further details.

And here is the patch! :-)

-- 
Petr^2 Spacek
From 52393bf2bb0ad74dbe37e496b1fd41a6ab22bd90 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Tue, 8 Dec 2015 12:06:33 +0100
Subject: [PATCH] Add 'review' target for make. It automates following tasks:

- check if ACI.txt and API.txt are up-to-date
- check if VERSION was changed if API was changed
- pep8 --diff does not produce new errors when ran on diff from origin/branch
- make lint does not produce errors
---
 Makefile  |  4 
 review.sh | 64 +++
 2 files changed, 68 insertions(+)
 create mode 100755 review.sh

diff --git a/Makefile b/Makefile
index d2c37f1597a011af2bd9ef1a4f7ce87ac59620a3..b85b3a5d6362150dcebf16745414c3d416c9547f 100644
--- a/Makefile
+++ b/Makefile
@@ -79,6 +79,10 @@ check: bootstrap-autogen server tests
 		(cd $$subdir && $(MAKE) check) || exit 1; \
 	done
 
+# works only in Git tree
+review: version-update
+	./review.sh
+
 bootstrap-autogen: version-update client-autogen
 	@echo "Building IPA $(IPA_VERSION)"
 	cd asn1; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi
diff --git a/review.sh b/review.sh
new file mode 100755
index ..3a5114fbe88d3aba8e926e049500f5ac5f41a83e
--- /dev/null
+++ b/review.sh
@@ -0,0 +1,64 @@
+#!/bin/bash
+is_tree_clean() {
+	test "$(git status --porcelain "$@")" == ""
+	return $?
+}
+
+log_error() {
+	echo "ERROR: ${1}, continuing ..."
+	errs=(${errs[@]} "ERROR: ${1}\n")
+}
+
+set -o errexit -o nounset #-o xtrace
+
+LOGFILE="$(mktemp --suff=.log)"
+exec > >(tee -i ${LOGFILE})
+exec 2>&1
+
+echo -n "make lint is running ... "
+make --silent lint || log_error "make lint failed"
+
+# Go backwards in history until you find a remote branch from which current
+# branch was created. This will be used as base for git diff.
+PATCHCNT=0
+BASEBRANCH=""
+CURRBRANCH="$(git branch --remote --contains)"
+while [ "${BASEBRANCH}" == "" ]
+do
+	BASEBRANCH="$(git branch --remote --contains "HEAD~${PATCHCNT}" | grep -v "^. ${CURRBRANCH}$" || :)"
+	PATCHCNT="$(expr "${PATCHCNT}" + 1)"
+done
+echo "Detected base branch: ${BASEBRANCH}"
+echo -n "Checks will be made against following base commit: "
+git log -1 --oneline ${BASEBRANCH}
+
+# gather all errors in one array and print error summary at the end
+declare -a errs
+errs=("Summary of detected errors:\n")
+
+is_tree_clean || log_error "Git tree must be clean before you start review"
+
+./makeapi
+is_tree_clean "API.txt" || log_error "./makeapi changed something"
+
+./makeaci
+is_tree_clean "ACI.txt" || log_error "./makeaci changed something"
+
+git diff ${BASEBRANCH} -U0 | pep8 --diff || log_error "PEP8 --diff failed"
+
+# if API.txt is changed require change in VERSION
+if ! git diff ${BASEBRANCH} --quiet -- API.txt;
+then
+	git diff ${BASEBRANCH} --quiet -- VERSION && log_error "API.txt was changed without a change in VERSION"
+fi
+
+# print error summary
+if [ "${#errs[*]}" != "1" ]
+then
+	echo -e "${errs[*]}"
+	echo "Please see ${LOGFILE}"
+	exit 1
+else
+	rm "${LOGFILE}"
+	echo "review.sh did not detect any problem"
+fi
-- 
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 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-12-10 Thread David Kupka 

On 09/12/15 18:55, Petr Spacek wrote:

On 9.12.2015 13:37, David Kupka wrote:

On 08/12/15 15:24, Petr Spacek wrote:

On 8.12.2015 12:19, David Kupka wrote:

On 08/12/15 08:56, Petr Spacek wrote:

On 7.12.2015 14:41, David Kupka wrote:

+def is_host_resolvable(fqdn):
+if not isinstance(fqdn, DNSName):
+fqdn = DNSName(fqdn)
+for rdtype in (rdatatype.A, rdatatype.):
+try:
+resolver.query(fqdn.make_absolute(), rdtype)
+except DNSException:
+continue
+else:
+return True
+
+return False



NACK, you are re-introducing duplicate function which was removed in
498471e4aed1367b72cd74d15811d0584a6ee268.

Please amend the patch ASAP to use new verify_host_resolvable() function so I
can test it and get it into 4.3.


Updated patches attached.


Hmm, my review script screams:

Detected base branch:   origin/master
Checks will be made against following base commit: 848912a add missing
/ipaplatform/constants.py to .gitignore
Writing API to API.txt
./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual
indentation
./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for
visual indent
./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual
indentation
./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for
visual indent
./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1
./ipapython/ipautil.py:948:17: E128 continuation line under-indented for
visual indent
./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1
./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters)
./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters)
./ipaserver/install/bindinstance.py:49:9: E128 continuation line
under-indented for visual indent
./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79
characters)
./ipaserver/install/bindinstance.py:438:19: E128 continuation line
under-indented for visual indent
./ipaserver/install/server/common.py:271:63: E261 at least two spaces before
inline comment
./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79
characters)
ERROR: PEP8 --diff failed, continuing ...
ERROR: API.txt was changed without a change in VERSION, continuing ...
Summary of detected errors:
   ERROR: PEP8 --diff failed
   ERROR: API.txt was changed without a change in VERSION

Please fix it :-)

Make make this more pleasant for you, you can use (and review) my attached
patch. It adds 'review' target to Makefile, it will do the same checks as I do.



Unfortunately your tool does not work for me (output w/ -o xtrace attached).
Anyway I have incremented API version and fixed most of the pep8 errors except
for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the
file and E501 twice in ipapython/ipautil.py because IMO breaking string
constant into multiple lines does not help readability.

Updated patches also attached.


We are almost there, but NACK for now.

1) Following attempt to install IPA explodes. Please note that
dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server
before installation is started, so it gives 'timeout' or possibly SERVFAIL.

# ipa-server-install --ds-password=root4lab --admin-password=root4lab
--setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap
--domain=dom-245.idm.lab.eng.brq.redhat.com
--realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM
[...]

Configuring DNS (named)
   [1/11]: generating rndc key file
   [2/11]: adding DNS container
   [3/11]: setting up our zone
   [error] InvocationError: DNS check for domain
dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after
30.000453949 seconds. Please make sure that the domain is properly delegated
to this IPA server.
ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain
dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after
30.000453949 seconds. Please make sure that the domain is properly delegated
to this IPA server.
ipa.ipapython.install.cli.install_tool(Server): ERRORThe
ipa-server-install command failed. See /var/log/ipaserver-install.log for more
information

2015-12-09T17:15:37Z DEBUG   File
"/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in execute
 return_value = self.run()
   File "/usr/lib/python2.7/site-packages/ipapython/install/cli.py", line 318,
in run
 cfgr.run()
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310,
in run
 self.execute()
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 332,
in execute
 for nothing in self._executor():
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 372,
in __runner
 self._handle_exception(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 394,
in _handle_exception
 six.reraise(*exc_info)
   File

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

2015-12-10 Thread Martin Basti 



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 
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.
From 0e5c42ac282f47138e106e4884e11bc5ff7ab12b 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 |  1 +
 ipaplatform/redhat/tasks.py | 53 +
 2 files changed, 54 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 9f82b3695fb10c4db65cc31278364b3b34e26098..7a5565c497b0dd20ed73af3112a37ac25d2abe6e 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -166,6 +166,7 @@ Requires: %{etc_systemd_dir}
 Requires: gzip
 Requires: python-gssapi >= 1.1.0
 Requires: custodia
+Requires: rpm-python
 
 Provides: %{alt_name}-server = %{version}
 Conflicts: %{alt_name}-server
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 94d2cb4e906965a20bcfdd55f38854005091c26f..c90e55f28cd492d43a98e4e0ee0476a23db8a099 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -30,11 +30,13 @@ import stat
 import socket
 import sys
 import base64
+from functools import total_ordering
 
 from subprocess import CalledProcessError
 from nss.error import NSPRError
 from pyasn1.error import PyAsn1Error
 from six.moves import urllib
+import rpm
 
 from ipapython.ipa_log_manager import root_logger, log_mgr
 from ipapython import ipautil
@@ -47,6 +49,35 @@ from ipaplatform.redhat.authconfig import RedHatAuthConfig
 from ipaplatform.base.tasks import BaseTaskNamespace
 
 
+# copied from rpmUtils/miscutils.py
+def stringToVersion(verstring):
+if verstring in [None, '']:
+return (None, None, None)
+i = verstring.find(':')
+if i != -1:
+try:
+epoch = str(long(verstring[:i]))
+except ValueError:
+# look, garbage in the epoch field, how fun, kill it
+epoch = '0' # this is our fallback, deal
+else:
+epoch = '0'
+j = verstring.find('-')
+if j != -1:
+if verstring[i + 1:j] == '':
+version = None
+else:
+version = verstring[i + 1:j]
+release = verstring[j + 1:]
+else:
+if verstring[i + 1:] == '':
+version = None
+else:
+version = verstring[i + 1:]
+release = None
+return (epoch, version, release)
+
+
 log = log_mgr.get_logger(__name__)
 
 
@@ -66,6 +97,21 @@ def selinux_enabled():
 return False
 
 
+@total_ordering
+class IPAVersion(object):
+
+def __init__(self, version):
+self.version_tuple = stringToVersion(version)
+
+def __eq__(self, other):
+assert isinstance(other, IPAVersion)
+return rpm.labelCompare(self.version_tuple, other.version_tuple) == 0
+
+def __lt__(self, other):
+assert isinstance(other, IPAVersion)
+return rpm.labelCompare(self.version_tuple, other.version_tuple) == -1
+
+
 class RedHatTaskNamespace(BaseTaskNamespace):
 
 def restore_context(self, filepath, restorecon=paths.SBIN_RESTORECON):
@@ -423,5 +469,12 @@ class RedHatTaskNamespace(BaseTaskNamespace):
 super(RedHatTaskNamespace, self).create_system_user(name, group,
 homedir, shell, uid, gid, comment, create_homedir)
 
+def parse_ipa_version(self, version):
+"""
+:param version: textu

Re: [Freeipa-devel] [PATCHES 523-525] replica install: add remote connection check over API

2015-12-10 Thread Martin Babinsky 

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.


That is a nitpick though, it does not affect the overall functionality 
of the patches so 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] [PATCH] 0001 Refactor test_user_plugin

2015-12-10 Thread Aleš Mareček 
Ahoj Filipe,
zase tam vidim to '\r\n'... Prisel jsi na to, co to zpusobuje?
Jinak patche jdu otestovat...

 - alich -

- Original Message -
> From: "Filip Škola" 
> To: "Aleš Mareček" 
> Cc: freeipa-devel@redhat.com, "Milan Kubík" 
> Sent: Thursday, December 10, 2015 11:29:52 AM
> Subject: Re: [Freeipa-devel] [PATCH] 0001 Refactor test_user_plugin
> 
> Hi,
> 
> this if fixed. Also issues with test_stageuser_plugin caused by
> UserTracker changes should be fixed here.
> 
> Filip
> 
> 
> On Mon, 7 Dec 2015 09:29:31 -0500 (EST)
> Aleš Mareček  wrote:
> 
> > NACK.
> > 
> > $ ./make-lint
> > * Module ipatests.test_xmlrpc.test_user_plugin
> > ipatests/test_xmlrpc/test_user_plugin.py:42:
> > [E0611(no-name-in-module), ] No name 'ldaptracker' in module
> > 'ipatests.test_xmlrpc')
> > 
> > $ grep ldaptracker ipatests/test_xmlrpc/test_user_plugin.py
> > from ipatests.test_xmlrpc.ldaptracker import Tracker
> > $ ls ipatests/test_xmlrpc/ldaptracker*
> > ls: cannot access ipatests/test_xmlrpc/ldaptracker*: No such file or
> > directory
> > 
> > 
> > - Original Message -
> > > From: "Filip Škola" 
> > > To: "Milan Kubík" 
> > > Cc: freeipa-devel@redhat.com
> > > Sent: Thursday, December 3, 2015 5:38:43 PM
> > > Subject: Re: [Freeipa-devel] [PATCH] 0001 Refactor test_user_plugin
> > > 
> > > Hi,
> > > 
> > > sending corrected version.
> > > 
> > > F.
> > > 
> > > On Thu, 12 Nov 2015 14:03:19 +0100
> > > Milan Kubík  wrote:
> > > 
> > > > On 11/10/2015 12:13 PM, Filip Škola wrote:
> > > > > Hi,
> > > > >
> > > > > fixed.
> > > > >
> > > > > F.
> > > > >
> > > > > On Tue, 10 Nov 2015 10:52:45 +0100
> > > > > Milan Kubík  wrote:
> > > > >
> > > > >> On 11/09/2015 04:35 PM, Filip Škola wrote:
> > > > >>> Another patch was applied in the meantime.
> > > > >>>
> > > > >>> Attaching an updated version.
> > > > >>>
> > > > >>> F.
> > > > >>>
> > > > >>> On Mon, 9 Nov 2015 13:35:02 +0100
> > > > >>> Milan Kubík  wrote:
> > > > >>>
> > > >  On 11/06/2015 11:32 AM, Filip Škola wrote:
> > > >  Hi,
> > > >  the patch doesn't apply.
> > > > 
> > > > >> Please fix this.
> > > > >>
> > > > >>   ipatests/test_xmlrpc/test_user_plugin.py:1419:
> > > > >> [E0602(undefined-variable),
> > > > >> TestDeniedBindWithExpiredPrincipal.teardown_class] Undefined
> > > > >> variable 'user1')
> > > > >>
> > > > >> Also, use the version numbers for your changed patches.
> > > > >>
> > > > >
> > > > >
> > > > Thanks for the patch. Several issues:
> > > > 
> > > > 1. Use dict.items instead of dict.iteritems, for python3
> > > > compatibility
> > > > 
> > > > 2. What is the purpose of TestPrepare class? The 'purge' methods
> > > > do not call any ipa commands.
> > > > Tracker.make_fixture should be used to make the Tracked resources
> > > > clean themselves up when they're out of scope.
> > > > 
> > > > 3. Why reference the resources by hardcoded name if they have a
> > > > fixture representation?
> > > > 
> > > > 4. Rewrite {create,delete}_test_group to a fixture. You may want
> > > > to use different scope (or not).
> > > > 
> > > > 5. In `def atest_rename_to_invalid_login(self, user):` - use
> > > > pytest.skipif decorator and provide a reason if you must,
> > > > do not obfuscate method name in order not to run it.
> > > > 
> > > > 
> > > 
> > > 
> > > --
> > > 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
> 
> 

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

2015-12-10 Thread 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] [PATCHES 509-514] replica promotion: use host credentials when setting up replication

2015-12-10 Thread Martin Babinsky 

On 12/10/2015 07:57 AM, Jan Cholasta wrote:

On 9.12.2015 16:39, Jan Cholasta wrote:

On 7.12.2015 08:14, Jan Cholasta wrote:

On 6.12.2015 21:32, Martin Basti wrote:



On 04.12.2015 16:58, Simo Sorce wrote:

On Fri, 2015-12-04 at 15:39 +0100, Jan Cholasta wrote:

On 4.12.2015 15:16, Jan Cholasta wrote:

On 4.12.2015 15:12, Jan Cholasta wrote:

On 4.12.2015 11:15, Petr Vobornik wrote:

On 12/03/2015 03:11 PM, Martin Basti wrote:


On 01.12.2015 12:19, Jan Cholasta wrote:

On 23.11.2015 15:47, Simo Sorce wrote:

On Mon, 2015-11-23 at 15:37 +0100, Jan Cholasta wrote:

Ad alternative is to add the host to ipaservers before the
checks
are
done and remove it again if any of them fail.

Too error prone, I am ok with the current way in your patches
until/unless I can think of a fail safe way. :-)

Updated patches attached. Note that 520 should be applied
between 509
and 510.




Functional ACK


Simo, do you want to review the ACIs or other things it the
patches? Or
can the patches be pushed?

There were no changes in the ACIs since last time.

Actually, memberPrincipal was removed from the "IPA server hosts can
manage own Custodia secrets" ACI, as per Simo's request.


Rebased patches attached.

Note that 520 should still be applied between 509 and 510.


LGTM


ACK


Thanks.

Pushed to master: 01ddf51df76f3298499973355c5461727e46ab5b


Martin Babinsky found out that ipaservers is not created early enough
when installing a replica of a 4.2 or older server which causes a crash.

The attached patch fixes that.


Actually I don't like how I fixed that, here's an updated patch.

Also, I noticed that replica promotion fails too late in domain level 0.
Fixed as well.





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

2015-12-10 Thread 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 
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

2015-12-10 Thread 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 
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] [PATCHES 516-517] spec file: put Python modules into standalone packages

2015-12-10 Thread Jan Cholasta 

On 10.12.2015 11:32, Jan Cholasta wrote:

On 9.12.2015 20:51, Petr Vobornik wrote:

On 12/07/2015 04:21 PM, Jan Cholasta wrote:

Hi,

the attached patches partially fix
. This is done to allow
the addition of Python 3 packages, see
.


See commit messages for more information.

In order to test:
1. make rpms
2.




3. Test with both dnf and yum-deprecated.

Beware that when you run "yum-deprecated clean all", it does not remove
cache for the on-disk repository created in step 2, you have to remove
the /var/cache/yum/$basearch/$releasever/$reponame directory manually.

Honza



Shouldn't freeipa-server-dns and freeipa-server-trust-add depend on
freeipa-server? They do not in this patch. IMO they should.


Fixed.



following updates work (all on f23, update from 4.2.3):
   dnf update
   dnf update freeipa-*
   yum-depracated update freeipa-*

for both client or server with all packages.

but when I tried to install only client "dnf install freeipa-client" and
then following failed:
   dnf update freeipa-client

The difference was:
 Installing:
  freeipa-client-common  noarch
  freeipa-common noarch
  python2-ipaclient  noarch
  python2-ipalib x86_64
 Upgrading:
  freeipa-client

Works:
  Installing:
  freeipa-client-common  noarch
  freeipa-common noarch
  freeipa-python-compat  noarch
  replacing  freeipa-python.x86_64 4.2.3-1.1.fc23
  python2-ipaclient  noarch
  python2-ipalib x86_64
 Upgrading:
  freeipa-client


not sure if it is a problem, otherwise the patch looks OK.


Hmm, Fedora packaging guidelines are silent about this.

When you run "dnf update freeipa-client freeipa-python" to force
freeipa-python update it works fine.

I removed Provides added Conflicts on freeipa-python which seem to have
fixed it.


Actually I think it would be better to keep the Provides and do 
Conflicts < 4.2.91 - fixed.


Also updated %alt_name dependencies to be in sync with %name 
dependencies and added arch-specific python-ipalib Provides to 
python2-ipalib.


Updated patches attached.

--
Jan Cholasta
From 4962c1c40073e278f1e2b379431b483ccd4351cb Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 26 Nov 2015 10:52:07 +0100
Subject: [PATCH 1/2] spec file: remove config files from freeipa-python

/etc/ipa/dnssec is now owned by freeipa-server. The remaining files are now
owned by freeipa-client.

https://fedorahosted.org/freeipa/ticket/3197
---
 freeipa.spec.in | 24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index a60d9b6..f776def 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -503,6 +503,9 @@ install daemons/dnssec/ipa-ods-exporter %{buildroot}%{_libexecdir}/ipa/ipa-ods-e
 # Web UI plugin dir
 mkdir -p %{buildroot}%{_usr}/share/ipa/ui/js/plugins
 
+# DNSSEC config
+mkdir -p %{buildroot}%{_sysconfdir}/ipa/dnssec
+
 # KDC proxy config (Apache config sets KDCPROXY_CONFIG to load this file)
 mkdir -p %{buildroot}%{_sysconfdir}/ipa/kdcproxy/
 install -m 644 install/share/kdcproxy.conf %{buildroot}%{_sysconfdir}/ipa/kdcproxy/kdcproxy.conf
@@ -536,7 +539,6 @@ mkdir -p %{buildroot}/%{_localstatedir}/lib/ipa/backup
 mkdir -p %{buildroot}%{_sysconfdir}/ipa/
 /bin/touch %{buildroot}%{_sysconfdir}/ipa/default.conf
 /bin/touch %{buildroot}%{_sysconfdir}/ipa/ca.crt
-mkdir -p %{buildroot}%{_sysconfdir}/ipa/dnssec
 mkdir -p %{buildroot}%{_sysconfdir}/ipa/nssdb
 mkdir -p %{buildroot}/%{_localstatedir}/lib/ipa-client/sysrestore
 mkdir -p %{buildroot}%{_sysconfdir}/bash_completion.d
@@ -840,6 +842,7 @@ fi
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/httpd/conf.d/ipa-kdc-proxy.conf
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/httpd/conf.d/ipa-pki-proxy.conf
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/kdcproxy/ipa-kdc-proxy.conf
+%dir %attr(0755,root,root) %{_sysconfdir}/ipa/dnssec
 %{_usr}/share/ipa/ipa.conf
 %{_usr}/share/ipa/ipa-rewrite.conf
 %{_usr}/share/ipa/ipa-pki-proxy.conf
@@ -928,6 +931,15 @@ fi
 %{_sbindir}/ipa-getkeytab
 %{_sbindir}/ipa-rmkeytab
 %{_sbindir}/ipa-join
+%dir %attr(0755,root,root) %{_sysconfdir}/ipa/
+%ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/default.conf
+%ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/ca.crt
+%dir %attr(0755,root,root) %{_sysconfdir}/ipa/nssdb
+%ghost %config(noreplace) %{_sysconfdir}/ipa/nssdb/cert8.db
+%ghost %conf

Re: [Freeipa-devel] [PATCH] 0001 Refactor test_user_plugin

2015-12-10 Thread Filip Škola 
Hi,

this if fixed. Also issues with test_stageuser_plugin caused by
UserTracker changes should be fixed here.

Filip


On Mon, 7 Dec 2015 09:29:31 -0500 (EST)
Aleš Mareček  wrote:

> NACK.
> 
> $ ./make-lint 
> * Module ipatests.test_xmlrpc.test_user_plugin
> ipatests/test_xmlrpc/test_user_plugin.py:42:
> [E0611(no-name-in-module), ] No name 'ldaptracker' in module
> 'ipatests.test_xmlrpc')
> 
> $ grep ldaptracker ipatests/test_xmlrpc/test_user_plugin.py 
> from ipatests.test_xmlrpc.ldaptracker import Tracker
> $ ls ipatests/test_xmlrpc/ldaptracker*
> ls: cannot access ipatests/test_xmlrpc/ldaptracker*: No such file or
> directory
> 
> 
> - Original Message -
> > From: "Filip Škola" 
> > To: "Milan Kubík" 
> > Cc: freeipa-devel@redhat.com
> > Sent: Thursday, December 3, 2015 5:38:43 PM
> > Subject: Re: [Freeipa-devel] [PATCH] 0001 Refactor test_user_plugin
> > 
> > Hi,
> > 
> > sending corrected version.
> > 
> > F.
> > 
> > On Thu, 12 Nov 2015 14:03:19 +0100
> > Milan Kubík  wrote:
> > 
> > > On 11/10/2015 12:13 PM, Filip Škola wrote:
> > > > Hi,
> > > >
> > > > fixed.
> > > >
> > > > F.
> > > >
> > > > On Tue, 10 Nov 2015 10:52:45 +0100
> > > > Milan Kubík  wrote:
> > > >
> > > >> On 11/09/2015 04:35 PM, Filip Škola wrote:
> > > >>> Another patch was applied in the meantime.
> > > >>>
> > > >>> Attaching an updated version.
> > > >>>
> > > >>> F.
> > > >>>
> > > >>> On Mon, 9 Nov 2015 13:35:02 +0100
> > > >>> Milan Kubík  wrote:
> > > >>>
> > >  On 11/06/2015 11:32 AM, Filip Škola wrote:
> > >  Hi,
> > >  the patch doesn't apply.
> > > 
> > > >> Please fix this.
> > > >>
> > > >>   ipatests/test_xmlrpc/test_user_plugin.py:1419:
> > > >> [E0602(undefined-variable),
> > > >> TestDeniedBindWithExpiredPrincipal.teardown_class] Undefined
> > > >> variable 'user1')
> > > >>
> > > >> Also, use the version numbers for your changed patches.
> > > >>
> > > >
> > > >
> > > Thanks for the patch. Several issues:
> > > 
> > > 1. Use dict.items instead of dict.iteritems, for python3
> > > compatibility
> > > 
> > > 2. What is the purpose of TestPrepare class? The 'purge' methods
> > > do not call any ipa commands.
> > > Tracker.make_fixture should be used to make the Tracked resources
> > > clean themselves up when they're out of scope.
> > > 
> > > 3. Why reference the resources by hardcoded name if they have a
> > > fixture representation?
> > > 
> > > 4. Rewrite {create,delete}_test_group to a fixture. You may want
> > > to use different scope (or not).
> > > 
> > > 5. In `def atest_rename_to_invalid_login(self, user):` - use
> > > pytest.skipif decorator and provide a reason if you must,
> > > do not obfuscate method name in order not to run it.
> > > 
> > > 
> > 
> > 
> > --
> > 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

>From 7867154636c4b77ea7f570eb5c78e0573ff9c430 Mon Sep 17 00:00:00 2001
From: Filip Skola 
Date: Fri, 6 Nov 2015 10:57:37 +0100
Subject: [PATCH] Refactor test_user_plugin, use UserTracker for tests

---
 ipatests/test_xmlrpc/test_user_plugin.py| 2387 ++-
 ipatests/test_xmlrpc/tracker/user_plugin.py |  172 +-
 2 files changed, 1039 insertions(+), 1520 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index 084fb83c42d362204ff4547357226c8f56d217fb..155e77446230786fbbd2c137674dbed29c4350c7 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -2,6 +2,7 @@
 #   Rob Crittenden 
 #   Pavel Zuna 
 #   Jason Gerard DeRose 
+#   Filip Skola 
 #
 # Copyright (C) 2008, 2009  Red Hat
 # see file 'COPYING' for use and warranty information
@@ -23,6 +24,7 @@
 Test the `ipalib/plugins/user.py` module.
 """
 
+import pytest
 import datetime
 import ldap
 import re
@@ -30,42 +32,42 @@ import re
 from ipalib import api, errors
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.util import (
-assert_equal, assert_not_equal, raises)
+assert_deepequal, assert_equal, assert_not_equal, raises)
 from xmlrpc_test import (
-XMLRPC_test, Declarative, fuzzy_digits, fuzzy_uuid, fuzzy_password,
-fuzzy_string, fuzzy_dergeneralizedtime, add_sid, add_oc)
+XMLRPC_test, fuzzy_digits, fuzzy_uuid, fuzzy_password,
+fuzzy_string, fuzzy_dergeneralizedtime, add_sid, add_oc, raises_exact)
 from ipapython.dn import DN
-import pytest
+from ipapython.version import API_VERSION
+
+from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.tracker.group_plugin import GroupTracker
+from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
 
-user1 = u'tuser1'
-user2 = u'tuser2'
 admin1 = u'admin'
-admin2 = u'admin2'
-renameduser1 = u'tuser'
-group1 = u'group1'
-admins_group = u'admins'
+admin_group = u'admins'
 
 invaliduser1 = u'+tuser1'
 invaliduser2 = u'tus

Re: [Freeipa-devel] [PATCHES 516-517] spec file: put Python modules into standalone packages

2015-12-10 Thread Jan Cholasta 

On 9.12.2015 20:51, Petr Vobornik wrote:

On 12/07/2015 04:21 PM, Jan Cholasta wrote:

Hi,

the attached patches partially fix
. This is done to allow
the addition of Python 3 packages, see
.

See commit messages for more information.

In order to test:
1. make rpms
2.



3. Test with both dnf and yum-deprecated.

Beware that when you run "yum-deprecated clean all", it does not remove
cache for the on-disk repository created in step 2, you have to remove
the /var/cache/yum/$basearch/$releasever/$reponame directory manually.

Honza



Shouldn't freeipa-server-dns and freeipa-server-trust-add depend on
freeipa-server? They do not in this patch. IMO they should.


Fixed.



following updates work (all on f23, update from 4.2.3):
   dnf update
   dnf update freeipa-*
   yum-depracated update freeipa-*

for both client or server with all packages.

but when I tried to install only client "dnf install freeipa-client" and
then following failed:
   dnf update freeipa-client

The difference was:
 Installing:
  freeipa-client-common  noarch
  freeipa-common noarch
  python2-ipaclient  noarch
  python2-ipalib x86_64
 Upgrading:
  freeipa-client

Works:
  Installing:
  freeipa-client-common  noarch
  freeipa-common noarch
  freeipa-python-compat  noarch
  replacing  freeipa-python.x86_64 4.2.3-1.1.fc23
  python2-ipaclient  noarch
  python2-ipalib x86_64
 Upgrading:
  freeipa-client


not sure if it is a problem, otherwise the patch looks OK.


Hmm, Fedora packaging guidelines are silent about this.

When you run "dnf update freeipa-client freeipa-python" to force 
freeipa-python update it works fine.


I removed Provides added Conflicts on freeipa-python which seem to have 
fixed it.


Updated patch attached.

--
Jan Cholasta
From 4962c1c40073e278f1e2b379431b483ccd4351cb Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 26 Nov 2015 10:52:07 +0100
Subject: [PATCH 1/2] spec file: remove config files from freeipa-python

/etc/ipa/dnssec is now owned by freeipa-server. The remaining files are now
owned by freeipa-client.

https://fedorahosted.org/freeipa/ticket/3197
---
 freeipa.spec.in | 24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index a60d9b6..f776def 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -503,6 +503,9 @@ install daemons/dnssec/ipa-ods-exporter %{buildroot}%{_libexecdir}/ipa/ipa-ods-e
 # Web UI plugin dir
 mkdir -p %{buildroot}%{_usr}/share/ipa/ui/js/plugins
 
+# DNSSEC config
+mkdir -p %{buildroot}%{_sysconfdir}/ipa/dnssec
+
 # KDC proxy config (Apache config sets KDCPROXY_CONFIG to load this file)
 mkdir -p %{buildroot}%{_sysconfdir}/ipa/kdcproxy/
 install -m 644 install/share/kdcproxy.conf %{buildroot}%{_sysconfdir}/ipa/kdcproxy/kdcproxy.conf
@@ -536,7 +539,6 @@ mkdir -p %{buildroot}/%{_localstatedir}/lib/ipa/backup
 mkdir -p %{buildroot}%{_sysconfdir}/ipa/
 /bin/touch %{buildroot}%{_sysconfdir}/ipa/default.conf
 /bin/touch %{buildroot}%{_sysconfdir}/ipa/ca.crt
-mkdir -p %{buildroot}%{_sysconfdir}/ipa/dnssec
 mkdir -p %{buildroot}%{_sysconfdir}/ipa/nssdb
 mkdir -p %{buildroot}/%{_localstatedir}/lib/ipa-client/sysrestore
 mkdir -p %{buildroot}%{_sysconfdir}/bash_completion.d
@@ -840,6 +842,7 @@ fi
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/httpd/conf.d/ipa-kdc-proxy.conf
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/httpd/conf.d/ipa-pki-proxy.conf
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/kdcproxy/ipa-kdc-proxy.conf
+%dir %attr(0755,root,root) %{_sysconfdir}/ipa/dnssec
 %{_usr}/share/ipa/ipa.conf
 %{_usr}/share/ipa/ipa-rewrite.conf
 %{_usr}/share/ipa/ipa-pki-proxy.conf
@@ -928,6 +931,15 @@ fi
 %{_sbindir}/ipa-getkeytab
 %{_sbindir}/ipa-rmkeytab
 %{_sbindir}/ipa-join
+%dir %attr(0755,root,root) %{_sysconfdir}/ipa/
+%ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/default.conf
+%ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/ca.crt
+%dir %attr(0755,root,root) %{_sysconfdir}/ipa/nssdb
+%ghost %config(noreplace) %{_sysconfdir}/ipa/nssdb/cert8.db
+%ghost %config(noreplace) %{_sysconfdir}/ipa/nssdb/key3.db
+%ghost %config(noreplace) %{_sysconfdir}/ipa/nssdb/secmod.db
+%ghost %config(noreplace) %{_sysconfdir}/ipa/nssdb/pwdfile.txt
+%ghost %config(noreplace) %{_sysconfdir}/pki/ca-trust/source/ipa.p11-kit
 %dir %{_usr}/share/ipa
 %dir %{_lo

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

2015-12-10 Thread Martin Basti 



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.
From 2cce3c5d0aa7de88e7a0a69af8a0e8e803bd8305 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 |  1 +
 ipaplatform/redhat/tasks.py | 47 +
 2 files changed, 48 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 9f82b3695fb10c4db65cc31278364b3b34e26098..7a5565c497b0dd20ed73af3112a37ac25d2abe6e 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -166,6 +166,7 @@ Requires: %{etc_systemd_dir}
 Requires: gzip
 Requires: python-gssapi >= 1.1.0
 Requires: custodia
+Requires: rpm-python
 
 Provides: %{alt_name}-server = %{version}
 Conflicts: %{alt_name}-server
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 94d2cb4e906965a20bcfdd55f38854005091c26f..9c975f9627ebd7207c1390aab264338972dbaff6 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -30,6 +30,7 @@ import stat
 import socket
 import sys
 import base64
+import rpm
 
 from subprocess import CalledProcessError
 from nss.error import NSPRError
@@ -47,6 +48,35 @@ from ipaplatform.redhat.authconfig import RedHatAuthConfig
 from ipaplatform.base.tasks import BaseTaskNamespace
 
 
+# copied from rpmUtils/miscutils.py
+def stringToVersion(verstring):
+if verstring in [None, '']:
+return (None, None, None)
+i = verstring.find(':')
+if i != -1:
+try:
+epoch = str(long(verstring[:i]))
+except ValueError:
+# look, garbage in the epoch field, how fun, kill it
+epoch = '0' # this is our fallback, deal
+else:
+epoch = '0'
+j = verstring.find('-')
+if j != -1:
+if verstring[i + 1:j] == '':
+version = None
+else:
+version = verstring[i + 1:j]
+release = verstring[j + 1:]
+else:
+if verstring[i + 1:] == '':
+version = None
+else:
+version = verstring[i + 1:]
+release = None
+return (epoch, version, release)
+
+
 log = log_mgr.get_logger(__name__)
 
 
@@ -66,6 +96,16 @@ def selinux_enabled():
 return False
 
 
+class IPAVersion(object):
+
+def __init__(self, version):
+self.version_tuple = stringToVersion(version)
+
+def __cmp__(self, other):
+assert isinstance(other, IPAVersion)
+return rpm.labelCompare(self.version_tuple, other.version_tuple)
+
+
 class RedHatTaskNamespace(BaseTaskNamespace):
 
 def restore_context(self, filepath, restorecon=paths.SBIN_RESTORECON):
@@ -423,5 +463,12 @@ class RedHatTaskNamespace(BaseTaskNamespace):
 super(RedHatTaskNamespace, self).create_system_user(name, group,
 homedir, shell, uid, gid, comment, create_homedir)
 
+def parse_ipa_version(self, version):
+"""
+:param version: textual version
+:return: object implementing proper __cmp__ method for version compare
+"""
+return IPAVersion(version)
+
 
 tasks = RedHatTaskNamespace()
-- 
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 0113] properly add ACIs to custodia container during IPA upgrade

2015-12-10 Thread Martin Babinsky 

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.

--
Martin^3 Babinsky
From 9c7accdc7facec47e9a75f91168dca28db9e343d Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Tue, 8 Dec 2015 09:51:09 +0100
Subject: [PATCH] add ACIs for custodia container to its parent during IPA
 upgrade

This fixes the situation when LDAPUpdater tries to add ACIs for storing
secrets in cn=custodia,cn=ipa,cn=etc,$SUFFIX before the container is actually
created leading to creation of container without any ACI and subsequent
erroneous behavior.

https://fedorahosted.org/freeipa/ticket/5524
---
 install/updates/20-aci.update | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/install/updates/20-aci.update b/install/updates/20-aci.update
index ca4c0df0576b07aa48e6bdd2e70e06f9819b6da9..5b9741d7e05537194038e860f82924018761391c 100644
--- a/install/updates/20-aci.update
+++ b/install/updates/20-aci.update
@@ -113,6 +113,6 @@ dn: cn=etc,$SUFFIX
 add:aci: (target = "ldap:///cn=replication,cn=etc,$SUFFIX";)(targetattr = "nsDS5ReplicaId")(version 3.0; acl "IPA server hosts can change replica ID"; allow(write) groupdn = "ldap:///cn=ipaservers,cn=hostgroups,cn=accounts,$SUFFIX";;)
 
 # IPA server hosts can create and manage own Custodia secrets
-dn: cn=custodia,cn=ipa,cn=etc,$SUFFIX
+dn: cn=ipa,cn=etc,$SUFFIX
 add:aci: (target = "ldap:///cn=*/($$dn),cn=custodia,cn=ipa,cn=etc,$SUFFIX")(version 3.0; acl "IPA server hosts can create own Custodia secrets"; allow(add) groupdn = "ldap:///cn=ipaservers,cn=hostgroups,cn=accounts,$SUFFIX"; and userdn = "ldap:///fqdn=($$dn),cn=computers,cn=accounts,$SUFFIX";)
 add:aci: (target = "ldap:///cn=*/($$dn),cn=custodia,cn=ipa,cn=etc,$SUFFIX")(targetattr = "ipaPublicKey")(version 3.0; acl "IPA server hosts can manage own Custodia secrets"; allow(write) groupdn = "ldap:///cn=ipaservers,cn=hostgroups,cn=accounts,$SUFFIX"; and userdn = "ldap:///fqdn=($$dn),cn=computers,cn=accounts,$SUFFIX";)
-- 
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 523-525] replica install: add remote connection check over API

2015-12-10 Thread Jan Cholasta 

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.

--
Jan Cholasta
From f807735945e95f13259faf5c8e952a324466c376 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 9 Dec 2015 08:17:07 +0100
Subject: [PATCH 1/3] build: put oddjob scripts into separate directory

https://fedorahosted.org/freeipa/ticket/5497
---
 freeipa.spec.in  | 3 ++-
 install/oddjob/Makefile.am   | 2 +-
 install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index a60d9b6..95948e7 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -740,6 +740,7 @@ fi
 %{_libexecdir}/ipa/ipa-dnskeysync-replica
 %{_libexecdir}/ipa/ipa-ods-exporter
 %{_libexecdir}/ipa/ipa-httpd-kdcproxy
+%dir %{_libexecdir}/ipa/oddjob
 %ghost %verify(not owner group) %dir %{_sharedstatedir}/kdcproxy
 %dir %attr(0755,root,root) %{_sysconfdir}/ipa/kdcproxy
 %config(noreplace) %{_sysconfdir}/sysconfig/ipa_memcached
@@ -914,7 +915,7 @@ fi
 %ghost %{_libdir}/krb5/plugins/libkrb5/winbind_krb5_locator.so
 %{_sysconfdir}/dbus-1/system.d/oddjob-ipa-trust.conf
 %{_sysconfdir}/oddjobd.conf.d/oddjobd-ipa-trust.conf
-%%attr(755,root,root) %{_libexecdir}/ipa/com.redhat.idm.trust-fetch-domains
+%attr(755,root,root) %{_libexecdir}/ipa/oddjob/com.redhat.idm.trust-fetch-domains
 
 %endif # ONLY_CLIENT
 
diff --git a/install/oddjob/Makefile.am b/install/oddjob/Makefile.am
index 9dde10c..5cdaf2b 100644
--- a/install/oddjob/Makefile.am
+++ b/install/oddjob/Makefile.am
@@ -1,6 +1,6 @@
 NULL =
 
-oddjobdir = $(libexecdir)/ipa
+oddjobdir = $(libexecdir)/ipa/oddjob
 oddjobconfdir = $(sysconfdir)/oddjobd.conf.d
 dbusconfdir = $(sysconfdir)/dbus-1/system.d
 
diff --git a/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf b/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf
index 17817de..bc2e8d1 100644
--- a/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf
+++ b/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf
@@ -10,7 +10,7 @@
   
   
 
-  
-- 
2.4.3

From b1b4bd0f6c2485e6b019455133a87db329b9f85c Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 9 Dec 2015 08:18:21 +0100
Subject: [PATCH 2/3] replica install: add remote connection check over API

Add server_conncheck command which calls ipa-replica-conncheck --replica
over oddjob.

https://fedorahosted.org/freeipa/ticket/5497
---
 API.txt|   8 ++
 VERSION|   4 +-
 freeipa.spec.in|   9 +-
 install/oddjob/Makefile.am |   3 +
 .../etc/dbus-1/system.d/org.freeipa.server.conf|  21 
 install/oddjob/etc/oddjobd.conf.d/ipa-server.conf  |  20 
 install/oddjob/org.freeipa.server.conncheck|   2 +
 install/tools/ipa-ca-install   |   6 +
 install/tools/ipa-replica-conncheck| 131 ++---
 install/updates/90-post_upgrade_plugins.update |   1 -
 ipalib/errors.py   |   1 +
 ipalib/messages.py |  10 ++
 ipalib/plugins/server.py   |  70 ++-
 ipaserver/install/adtrustinstance.py   |  19 ---
 ipaserver/install/ca.py|   2 +-
 ipaserver/install/httpinstance.py  |  26 
 ipaserver/install/installutils.py  |  12 --
 ipaserver/install/plugins/adtrust.py   |  21 
 ipaserver/install/replication.py   |   6 +-
 ipaserver/install/server/replicainstall.py |   6 +-
 ipaserver/install/server/upgrade.py|   1 +
 21 files changed, 301 insertions(+), 78 deletions(-)
 create mode 100644 install/oddjob/etc/dbus-1/system.d/org.freeipa.server.conf
 create mode 100644 install/oddjob/etc/oddjobd.conf.d/ipa-server.conf
 create mode 100755 install/oddjob/org.freeipa.server.conncheck

diff --git a/API.txt b/API.txt
index 60c98c3..15be32c 100644
--- a/API.txt
+++ b/API.txt
@@ -3812,6 +3812,14 @@ option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
+command: server_conncheck
+args: 2,1,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, quer

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

2015-12-10 Thread Lukas Slebodnik 
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 
>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

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