Re: [Freeipa-devel] [PATCHES] 165-166 Clean up ipa-server-certinstall CLI options
On 08/27/2013 06:28 PM, Jan Cholasta wrote: Hi, the attached patches fix https://fedorahosted.org/freeipa/ticket/3869. Note that I made a slight change to the design page to reflect what ipa-server-certinstall actually does: The -d and -w options are allowed to be used simultaneously to replace both DS and HTTP certificates in one step. Honza Thanks, ACK, pushed to master: 3c9261699a79bffcb6362aeb03dec36ed588f81e ipa-3-3: 220f6f69b6f6c109a4e46018553529b74453a02c -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 119 CLDAP: make sure an empty reply is returned on any error
Hi, this patch fixes an issue in the CLDAP plugin found by Coverity. bye, Sumit From c993567c2c23857df361c527a4abd185b1f01a1e Mon Sep 17 00:00:00 2001 From: Sumit Bose sb...@redhat.com Date: Wed, 28 Aug 2013 10:10:52 +0200 Subject: [PATCH] CLDAP: make sure an empty reply is returned on any error If ipa_cldap_decode() reply is not initialized. Fixes https://fedorahosted.org/freeipa/ticket/3885 --- daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c index df7cc11a97c843692f461b21e1a99e75773dd886..db4a3d0611a713feb30843bb01b7645afcb2b695 100644 --- a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c +++ b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c @@ -276,13 +276,14 @@ static void ipa_cldap_process(struct ipa_cldap_ctx *ctx, LOG_TRACE(CLDAP Request received); ret = ipa_cldap_netlogon(ctx, req, reply); + +done: if (ret != 0) { /* bad request, or internal error, return empty reply */ /* as Windows does per MS-ADTS 6.3.3.3 */ memset(reply, 0, sizeof(struct berval)); } -done: ipa_cldap_respond(ctx, req, reply); ipa_cldap_free_kvps(req-kvps); -- 1.8.1.4 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] Fixes for some coverity errors
On Tue, Aug 27, 2013 at 02:27:08PM -0400, Simo Sorce wrote: Tickets 3882, 3883, 3884 Minor coverity issues, but should all be pushed to master and current release tree where appropriate. The memory leak is particularly important to fix for the OTP case. ACK to all three. bye, Sumit Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] FreeIPA server package group
On 08/26/2013 10:14 AM, Tomas Babej wrote: On Mon 26 Aug 2013 10:12:09 AM CEST, Petr Vobornik wrote: On 08/26/2013 09:54 AM, Tomas Babej wrote: Hi, I cooked up a patch for comps that adds a FreeIPA package group. Please chime in if you're OK with package selection / description. For illustration, see the attached image. FreeIPA will be added as an add-on in an installer under the Infrastructure server environment, that means, in the included images it will be at the same level as DNS or FTP server. It will also appear in the Software Selection tool (PackageKit). It should also be available under as yum groupinstall FreeIPA server, and in PackageKit, as I understand comps is also source for that too. https://fedoraproject.org/wiki/How_to_use_and_edit_comps.xml_for_package_groups https://fedorahosted.org/freeipa/ticket/3630 IMO the Audit part in the description is false advertisement. Same issue is in package descriptions. I know, it's taken directly from there. I'd rather have it consistent, if we're going to change it here, we should do there too, so that we do not end up with multiple (seemingly incomplete) descriptions at various places. Anybody else does have any other concerns? We need to move with this effort since string freeze for F20 is coming. I'm particulary dubious about including the freeipa-tests package. We discussed the A (as Audit) part in the description with Rob. The fact is that this is taken from the freeipa-server package description and nobody complained in 7 years. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] FreeIPA server package group
On 08/28/2013 11:46 AM, Tomas Babej wrote: On 08/26/2013 10:14 AM, Tomas Babej wrote: On Mon 26 Aug 2013 10:12:09 AM CEST, Petr Vobornik wrote: On 08/26/2013 09:54 AM, Tomas Babej wrote: Hi, I cooked up a patch for comps that adds a FreeIPA package group. Please chime in if you're OK with package selection / description. For illustration, see the attached image. FreeIPA will be added as an add-on in an installer under the Infrastructure server environment, that means, in the included images it will be at the same level as DNS or FTP server. It will also appear in the Software Selection tool (PackageKit). It should also be available under as yum groupinstall FreeIPA server, and in PackageKit, as I understand comps is also source for that too. https://fedoraproject.org/wiki/How_to_use_and_edit_comps.xml_for_package_groups https://fedorahosted.org/freeipa/ticket/3630 IMO the Audit part in the description is false advertisement. Same issue is in package descriptions. I know, it's taken directly from there. I'd rather have it consistent, if we're going to change it here, we should do there too, so that we do not end up with multiple (seemingly incomplete) descriptions at various places. Anybody else does have any other concerns? We need to move with this effort since string freeze for F20 is coming. I'm particulary dubious about including the freeipa-tests package. I don't think that should be included, developer tests are unnecessary for a server. We discussed the A (as Audit) part in the description with Rob. The fact is that this is taken from the freeipa-server package description and nobody complained in 7 years. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] FreeIPA server package group
On 08/28/2013 12:03 PM, Petr Viktorin wrote: On 08/28/2013 11:46 AM, Tomas Babej wrote: On 08/26/2013 10:14 AM, Tomas Babej wrote: On Mon 26 Aug 2013 10:12:09 AM CEST, Petr Vobornik wrote: On 08/26/2013 09:54 AM, Tomas Babej wrote: Hi, I cooked up a patch for comps that adds a FreeIPA package group. Please chime in if you're OK with package selection / description. For illustration, see the attached image. FreeIPA will be added as an add-on in an installer under the Infrastructure server environment, that means, in the included images it will be at the same level as DNS or FTP server. It will also appear in the Software Selection tool (PackageKit). It should also be available under as yum groupinstall FreeIPA server, and in PackageKit, as I understand comps is also source for that too. https://fedoraproject.org/wiki/How_to_use_and_edit_comps.xml_for_package_groups https://fedorahosted.org/freeipa/ticket/3630 IMO the Audit part in the description is false advertisement. Same issue is in package descriptions. I know, it's taken directly from there. I'd rather have it consistent, if we're going to change it here, we should do there too, so that we do not end up with multiple (seemingly incomplete) descriptions at various places. Anybody else does have any other concerns? We need to move with this effort since string freeze for F20 is coming. I'm particulary dubious about including the freeipa-tests package. I don't think that should be included, developer tests are unnecessary for a server. It was marked as optional in the initial proposal, but I agree it's unnecessary for it to be there at all. We discussed the A (as Audit) part in the description with Rob. The fact is that this is taken from the freeipa-server package description and nobody complained in 7 years. Updated tests attached. -- Tomas Babej Associate Software Engeneer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From 6a6e84935213c38ce28b8767417c5037d25d0e95 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Mon, 26 Aug 2013 09:26:23 +0200 Subject: [PATCH] Add FreeIPA server package group --- comps-f20.xml.in | 15 +++ comps-f21.xml.in | 15 +++ 2 files changed, 30 insertions(+) diff --git a/comps-f20.xml.in b/comps-f20.xml.in index b0820461c95ff5f20827adb432c04faefffad738..a84300d4f297ee67b19efa12956652e417eacd1e 100644 --- a/comps-f20.xml.in +++ b/comps-f20.xml.in @@ -2358,6 +2358,19 @@ /packagelist /group group +idfreeipa-server/id +_nameFreeIPA Server/_name +_descriptionIntegrated solution to provide centrally managed Identity, Policy and Audit./_description +defaultfalse/default +uservisibletrue/uservisible +packagelist + packagereq type=mandatoryfreeipa-server/packagereq + packagereq type=defaultfreeipa-server-strict/packagereq + packagereq type=defaultfreeipa-server-trust-ad/packagereq + packagereq type=optionalfreeipa-tests/packagereq +/packagelist + /group + group idfinnish-support/id _nameFinnish Support/_name _description/ @@ -6374,6 +6387,7 @@ optionlist groupiddogtag/groupid groupiddns-server/groupid + groupidfreeipa-server/groupid groupidftp-server/groupid groupidmail-server/groupid groupidnetwork-server/groupid @@ -6590,6 +6604,7 @@ groupiddirectory-server/groupid groupiddns-server/groupid groupiddogtag/groupid + groupidfreeipa-server/groupid groupidftp-server/groupid groupidha/groupid groupidhaproxy/groupid diff --git a/comps-f21.xml.in b/comps-f21.xml.in index bd990709a347ff37ee354743317d11b55a983f5c..30aabe18d8c9f0dff91028734628d32025c85cdd 100644 --- a/comps-f21.xml.in +++ b/comps-f21.xml.in @@ -2358,6 +2358,19 @@ /packagelist /group group +idfreeipa-server/id +_nameFreeIPA Server/_name +_descriptionIntegrated solution to provide centrally managed Identity, Policy and Audit./_description +defaultfalse/default +uservisibletrue/uservisible +packagelist + packagereq type=mandatoryfreeipa-server/packagereq + packagereq type=defaultfreeipa-server-strict/packagereq + packagereq type=defaultfreeipa-server-trust-ad/packagereq + packagereq type=optionalfreeipa-tests/packagereq +/packagelist + /group + group idfinnish-support/id _nameFinnish Support/_name _description/ @@ -6387,6 +6400,7 @@ optionlist groupiddogtag/groupid groupiddns-server/groupid + groupidfreeipa-server/groupid groupidftp-server/groupid groupidmail-server/groupid groupidnetwork-server/groupid @@ -6603,6 +6617,7 @@ groupiddirectory-server/groupid groupiddns-server/groupid groupiddogtag/groupid + groupidfreeipa-server/groupid groupidftp-server/groupid groupidha/groupid groupidhaproxy/groupid -- 1.8.3.1
Re: [Freeipa-devel] [PATCH] 0269 Add man pages for testing tools
On 08/28/2013 12:02 AM, Rob Crittenden wrote: Petr Viktorin wrote: Hello, This patch adds man pages for testing tools. As far as I can see, we use autotools for installing man pages. I added the autotools machinery to ipatests/man only. I'd appreciate if an autotools expert could check if this approach is OK. Or would it be better to not use autotools at all here? https://fedorahosted.org/freeipa/ticket/3855 (part 5) Thanks for the review! You don't have any man pages in section 8 so that can be removed from Makefile.am. Removed You need to add a line break for the various ways to run the commands. ipa-test-config [options] ipa-test-config [options] --global ipa-test-config [options] hostname renders as ipa-test-config [options]ipa-test-config[options]--global ipa-test-config [options] hostname Added ipa-test-config lacks a header. Which header do you mean? I see the same header as on the other pages. ipa-test-config doesn't say where the configuration is stored. It is not stored anywhere; it's read from environment variables and printed to stdout. I've clarified the description a bit. ipa-test-task, in the install-topo description drop the word Please. Removed Almost none of the 72 options to ipa-run-test are documented in the man page. These are taken from the nosetests command and documented in nosetests(1). Also, the list can change depending on what plugins are installed. I think pointing the reader to nosetests(1) is enough. rob It's a shame the test commands don't run in the tree. Well, they will work in-tree if you set PYTHONPATH to the tree. For example these work without the packages installed: PYTHONPATH=. ./ipatests/ipa-run-tests test_ipalib/test_config.py PYTHONPATH=. ./ipatests/ipa-test-task uninstall-all You can also point the system-installed ipa-run-tests to in-tree tests. You just need to use an absolute path because it changes the current directory: ipa-run-tests `pwd`/ipatests/test_ipalib/test_config.py -- Petr³ From 272d740a7c8514eee5e5cc50036b1b3de77e4775 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Tue, 13 Aug 2013 18:32:36 +0200 Subject: [PATCH] Add man pages for testing tools Add man pages for ipa-run-tests, ipa-test-task, and ipa-test-config. https://fedorahosted.org/freeipa/ticket/3855 (part 5) --- .gitignore | 5 ++ Makefile | 7 +- freeipa.spec.in| 6 ++ ipatests/man/Makefile.am | 18 + ipatests/man/configure.ac | 24 +++ ipatests/man/ipa-run-tests.1 | 63 + ipatests/man/ipa-test-config.1 | 157 + ipatests/man/ipa-test-task.1 | 126 + 8 files changed, 405 insertions(+), 1 deletion(-) create mode 100644 ipatests/man/Makefile.am create mode 100644 ipatests/man/configure.ac create mode 100644 ipatests/man/ipa-run-tests.1 create mode 100644 ipatests/man/ipa-test-config.1 create mode 100644 ipatests/man/ipa-test-task.1 diff --git a/.gitignore b/.gitignore index 738b00b13b7014ea915da9bcd06ab1ac4eee3764..5252dad1befa9279b8e5cd033e395309f6a9ae8a 100644 --- a/.gitignore +++ b/.gitignore @@ -81,6 +81,11 @@ ipa-client/py-compile ipa-client/stamp-h1 ipa-client/version.m4 ipatests/test_xmlrpc/service.crt +ipatests/man/aclocal.m4 +ipatests/man/autom4te.cache/ +ipatests/man/config.status +ipatests/man/install-sh +ipatests/man/missing freeipa.spec ipapython/setup.py ipapython/version.py diff --git a/Makefile b/Makefile index 674143b8df14e93833d87fcea5740efbb28a9374..a21cf7e33275fd1a783e89baf237c8dcd8db6508 100644 --- a/Makefile +++ b/Makefile @@ -71,6 +71,9 @@ client-autogen: version-update cd ipa-client; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi cd install; if [ ! -e Makefile ]; then ../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi +tests-man-autogen: version-update + cd ipatests/man; if [ ! -e Makefile ]; then ../../autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=$(LIBDIR); fi + install: all server-install tests-install @for subdir in $(SUBDIRS); do \ (cd $$subdir $(MAKE) $@) || exit 1; \ @@ -153,15 +156,17 @@ server-install: server $(PYTHON) setup.py install --root $(DESTDIR); \ fi -tests: version-update +tests: version-update tests-man-autogen cd ipatests; $(PYTHON) setup.py build + cd ipatests/man $(MAKE) all tests-install: tests if [ $(DESTDIR) = ]; then \ cd ipatests; $(PYTHON) setup.py install; \ else \ cd ipatests; $(PYTHON) setup.py install --root $(DESTDIR); \ fi + cd ipatests/man $(MAKE) install archive: -mkdir -p dist diff --git a/freeipa.spec.in b/freeipa.spec.in index 46a2ae0210c6b75fea971f5d02a10db22815787c..67bd3667db42ebb3639f50a4dd0c3a9fda000781 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@
Re: [Freeipa-devel] [PATCHES] Fixes for some coverity errors
On 08/28/2013 10:57 AM, Sumit Bose wrote: On Tue, Aug 27, 2013 at 02:27:08PM -0400, Simo Sorce wrote: Tickets 3882, 3883, 3884 Minor coverity issues, but should all be pushed to master and current release tree where appropriate. The memory leak is particularly important to fix for the OTP case. ACK to all three. Pushed to master: bea533c69a2b11e4a774144b8ee335e458333f7a ipa-3-3: 634823b9278a150e9fcc2920329da15a5a005168 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0087] Log proper error message when defaultNamingContext not found
Tomas Babej wrote: Hi, When adding a trust using trust-add with misconfigured DNS, an improper LDAP entry might be returned. Log a proper error message. https://fedorahosted.org/freeipa/ticket/3690 I think this should this include which KeyError was raised to help diagnose the problem. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 446 Update idrange search facet after trust creation
On 08/22/2013 04:11 PM, Petr Vobornik wrote: Adding a trust creates a range - range search facet should be marked as expired. https://fedorahosted.org/freeipa/ticket/3874 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0087] Log proper error message when defaultNamingContext not found
On 08/28/2013 01:22 PM, Rob Crittenden wrote: Tomas Babej wrote: Hi, When adding a trust using trust-add with misconfigured DNS, an improper LDAP entry might be returned. Log a proper error message. https://fedorahosted.org/freeipa/ticket/3690 I think this should this include which KeyError was raised to help diagnose the problem. rob Fixed and rebased on top of current head. -- Tomas Babej Associate Software Engeneer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From 1aafcd1c800806d92a2df996d6115aef63ddbb1b Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Tue, 6 Aug 2013 12:15:22 +0200 Subject: [PATCH] Log proper error message when defaultNamingContext not found --- ipaserver/dcerpc.py | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ipaserver/dcerpc.py b/ipaserver/dcerpc.py index 4b87a9e8c8e4a7e6433ab9f9f0dffb5dd82cdfd4..a27a64d2f5ba7932c1f44405cc3cf4fb7c8256d2 100644 --- a/ipaserver/dcerpc.py +++ b/ipaserver/dcerpc.py @@ -790,9 +790,15 @@ class TrustDomainInstance(object): root_logger.error( LDAP error when connecting to %(host)s: %(error)s % dict(host=unicode(result.pdc_name), error=str(e))) +except KeyError, e: +root_logger.error(KeyError: {err}, LDAP entry from {host} + returned malformed. Your DNS might be + misconfigured. + .format(host=unicode(result.pdc_name), + err=unicode(e))) if search_result: - self.info['sid'] = self.parse_naming_context(search_result) +self.info['sid'] = self.parse_naming_context(search_result) return True def parse_naming_context(self, context): -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 446 Update idrange search facet after trust creation
On 08/28/2013 02:07 PM, Ana Krivokapic wrote: On 08/22/2013 04:11 PM, Petr Vobornik wrote: Adding a trust creates a range - range search facet should be marked as expired. https://fedorahosted.org/freeipa/ticket/3874 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK Pushed to master: 92569b712c04c6e4a1eb8c1e2c118a649f727997 ipa-3-3: 844404af720d3b289fc673198166b7e201886c39 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0272 Add missing license header to ipa-test-config
I'm sorry for the omission. -- Petr³ From 9d1b244987666b1d66fecd782448798aac34ffe9 Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Wed, 28 Aug 2013 14:31:46 +0200 Subject: [PATCH] Add missing license header to ipa-test-config --- ipatests/ipa-test-config | 19 +++ 1 file changed, 19 insertions(+) diff --git a/ipatests/ipa-test-config b/ipatests/ipa-test-config index eb2da10f29fab89f6a8ec7a85322b4d5f0ebdedd..ce1f55b5f6f81c337559eaccca42fdb942a1321a 100755 --- a/ipatests/ipa-test-config +++ b/ipatests/ipa-test-config @@ -1,5 +1,24 @@ #! /usr/bin/python +# Authors: +# Petr Viktorin pvikt...@redhat.com +# +# Copyright (C) 2013 Red Hat +# see file 'COPYING' for use and warranty information +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. + import sys import os import argparse -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0272 Add missing license header to ipa-test-config
Petr Viktorin wrote: I'm sorry for the omission. ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0272 Add missing license header to ipa-test-config
On 08/28/2013 02:38 PM, Rob Crittenden wrote: Petr Viktorin wrote: I'm sorry for the omission. ACK Pushed to master: fed7e7b23182ef0355cdaf6a712d8cce84382872 ipa-3-3: 023385510a1b9ce6b40e40b788044ba853463696 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0055 Fix tests which fail after ipa-adtrust-install
On 08/26/2013 09:38 AM, Ana Krivokapic wrote: On 08/22/2013 06:13 PM, Tomas Babej wrote: On 08/20/2013 04:14 PM, Ana Krivokapic wrote: On 08/09/2013 05:35 PM, Tomas Babej wrote: On 08/09/2013 04:03 PM, Ana Krivokapic wrote: On 08/09/2013 09:39 AM, Tomas Babej wrote: On 08/08/2013 04:09 PM, Ana Krivokapic wrote: Hello, This patch should fix the failing unit tests. https://fedorahosted.org/freeipa/ticket/3852 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel There are two tests failing on my machine when running the tests after ipa-adtrust-install with your patch applied: You say there are two tests failing but I only see one below. That was just debris from trying to break your patch too much, one of my comments rendered invalid in the end :) == FAIL: test_group[24]: group_find: Search for POSIX groups -- Traceback (most recent call last): [...] AssertionError: assert_deepequal: dict keys mismatch. test_group[24]: group_find: Search for POSIX groups missing keys = [] extra keys = ['ipantsecurityidentifier'] expected = {'dn': ipapython.dn.DN('cn=editors,cn=groups,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com'), 'cn': [u'editors'], 'objectclass': Fuzzy(None, None, function lambda at 0x3768c08), 'gidnumber': [Fuzzy('^\\d+$', type 'basestring', None)], 'ipauniqueid': [Fuzzy('^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$', type 'unicode', None)], 'description': [u'Limited admins who can edit other users']} got = {'dn': u'cn=editors,cn=groups,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com', 'cn': (u'editors',), 'objectclass': (u'top', u'groupofnames', u'posixgroup', u'ipausergroup', u'ipaobject', u'nestedGroup', u'ipantgroupattrs'), 'ipantsecurityidentifier': (u'S-1-5-21-1457515837-642396627-3509099663-1002',), 'gidnumber': (u'180462',), 'ipauniqueid': (u'7c6e1672-0039-11e3-9567-001a4a2221fb',), 'description': (u'Limited admins who can edit other users',)} path = ('result', 1) I think you need the wrap the dictionary discribing the editor's group entry with the add_sid wrapper, and its objectclasses using the add_oc wrapper. [tbabej@vm-139 freeipa]$ git diff diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py index d380fe5..14c70cd 100644 --- a/ipatests/test_xmlrpc/test_group_plugin.py +++ b/ipatests/test_xmlrpc/test_group_plugin.py @@ -447,14 +447,15 @@ class test_group(Declarative): objectclasses.posixgroup, u'ipantgroupattrs')), 'ipauniqueid': [fuzzy_uuid], }), -{ +add_sid({ 'dn': get_group_dn('editors'), 'gidnumber': [fuzzy_digits], 'cn': [u'editors'], 'description': [u'Limited admins who can edit other users'], -'objectclass': fuzzy_set_ci(objectclasses.posixgroup), +'objectclass': fuzzy_set_ci(add_oc( +objectclasses.posixgroup, u'ipantgroupattrs')), 'ipauniqueid': [fuzzy_uuid], -}, +}), dict( dn=get_group_dn(group1), cn=[group1], These changes were sufficient for me to have the unit test suite run without errors. -- Tomas Babej Associate Software Engeneer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org I retested the patch and the tests are passing in my setup. The editors group definitely does not have the ipantsecurityidentifier attribute nor the ipantgroupattrs objectclass: [akrivoka@vm-181 freeipa]$ ipa group-show editors --all dn: cn=editors,cn=groups,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com Group name: editors Description: Limited admins who can edit other users GID: 197702 ipauniqueid: 91b3597e-00f3-11e3-92ae-001a4a22217b objectclass: top, groupofnames, posixgroup, ipausergroup, ipaobject, nestedGroup What I noticed though, is that if I delete and re-create the editors group (after ipa-adtrust-install has been run), it then gets the above mentioned attribute and objectclass. Maybe you did some similar manipulation in your setup, resulting in the test failing? I think it does depend on whether you have ran the ipa-sidgen task when running the ipa-adtrust-install. Do you think we can cover both cases here? -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. -- Tomas Babej Associate Software Engeneer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org
[Freeipa-devel] [PATCH] 0058 Add integration tests for forced client re-enrollment
Hello, This patch adds integration tests for the forced client re-enrollment feature, according to the test plan at: http://www.freeipa.org/page/V3/Forced_client_re-enrollment#Test_Plan https://fedorahosted.org/freeipa/ticket/3832 -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. From 646ec8995ced69f6538f549edcd08f9a98f84c34 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic akriv...@redhat.com Date: Wed, 28 Aug 2013 14:58:44 +0200 Subject: [PATCH] Add integration tests for forced client re-enrollment Add integration tests for the forced client re-enrollment feature: http://www.freeipa.org/page/V3/Forced_client_re-enrollment#Test_Plan https://fedorahosted.org/freeipa/ticket/3832 --- .../test_forced_client_reenrollment.py | 241 + 1 file changed, 241 insertions(+) create mode 100644 ipatests/test_integration/test_forced_client_reenrollment.py diff --git a/ipatests/test_integration/test_forced_client_reenrollment.py b/ipatests/test_integration/test_forced_client_reenrollment.py new file mode 100644 index ..1f8f0c221240d8bd9001d5ceaec833925b552e95 --- /dev/null +++ b/ipatests/test_integration/test_forced_client_reenrollment.py @@ -0,0 +1,241 @@ +# Authors: +# Ana Krivokapic akriv...@redhat.com +# +# Copyright (C) 2013 Red Hat +# see file 'COPYING' for use and warranty information +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +import subprocess + +from ipatests.test_integration.base import IntegrationTest +from ipatests.test_integration import tasks + +CLIENT_KEYTAB = '/etc/krb5.keytab' +BACKUP_KEYTAB = '/root/krb5.keytab' +EMPTY_KEYTAB = '/tmp/empty.keytab' +TEMP_KEYTAB = '/tmp/tmp.keytab' + + +class TestForcedClientReenrollment(IntegrationTest): + +Forced client re-enrollment +http://www.freeipa.org/page/V3/Forced_client_re-enrollment#Test_Plan + +num_replicas = 1 +num_clients = 1 + +@classmethod +def install(cls): +super(TestForcedClientReenrollment, cls).install() +tasks.install_master(cls.master) +tasks.install_replica(cls.master, cls.replicas[0], setup_ca=False) + +def setUp(self): +tasks.prepare_host(self.clients[0]) +tasks.install_client(self.master, self.clients[0]) + +def tearDown(self): +tasks.uninstall_client(self.clients[0]) +self.delete_client_host_entry() + +def test_reenroll_with_force_join(self): +sshfp_record_pre = self.get_sshfp_record() +self.restore_client() +self.check_client_host_entry() +self.reenroll_client(force_join=True) +sshfp_record_post = self.get_sshfp_record() +assert sshfp_record_pre == sshfp_record_post + +def test_reenroll_with_keytab(self): +self.backup_keytab() +sshfp_record_pre = self.get_sshfp_record() +self.restore_client() +self.check_client_host_entry() +self.restore_keytab() +self.reenroll_client(keytab=BACKUP_KEYTAB) +sshfp_record_post = self.get_sshfp_record() +assert sshfp_record_pre == sshfp_record_post + +def test_reenroll_with_both_force_join_and_keytab(self): +self.backup_keytab() +sshfp_record_pre = self.get_sshfp_record() +self.restore_client() +self.check_client_host_entry() +self.restore_keytab() +self.reenroll_client(force_join=True, keytab=BACKUP_KEYTAB) +sshfp_record_post = self.get_sshfp_record() +assert sshfp_record_pre == sshfp_record_post + +def test_reenroll_to_replica(self): +self.backup_keytab() +sshfp_record_pre = self.get_sshfp_record() +self.restore_client() +self.check_client_host_entry() +self.restore_keytab() +self.reenroll_client(keytab=BACKUP_KEYTAB, to_replica=True) +sshfp_record_post = self.get_sshfp_record() +assert sshfp_record_pre == sshfp_record_post + +def test_try_to_reenroll_with_disabled_host(self): +self.backup_keytab() +self.disable_client_host_entry() +self.restore_client() +self.check_client_host_entry(enabled=False) +self.restore_keytab() +self.reenroll_client(keytab=BACKUP_KEYTAB, expect_fail=True) + +def test_try_to_reenroll_with_uninstalled_host(self): +self.backup_keytab() +self.uninstall_client() +
Re: [Freeipa-devel] [PATCH] 0257 Add initial CA-less installation tests
On 27.8.2013 10:16, Petr Viktorin wrote: On 08/26/2013 09:23 AM, Jan Cholasta wrote: On 22.8.2013 09:46, Petr Viktorin wrote: On 08/16/2013 07:13 PM, Petr Viktorin wrote: On 07/30/2013 05:47 PM, Petr Viktorin wrote: Hello, This patch implements the first batch of integration tests for CA-less intallation. Tests from http://www.freeipa.org/page/V3/CA-less_install up to IPA server install with missing DS PKCS#12 password are included. Running this already takes an hour in the lab I use, so I decided to split the patch up and post the first part for review now. The two tests for revoked certificates fail. This is expected as we don't handle revoked certs yet. Continuing, this patch includes all tests except the ones for UI (pvoborni's patch 443) and certinstall (I'll review jcholast's fixes first). See commit message for details. Here is the completed patch, with all test except the Web UI ones. - The following case is omitted as it is invalid: - Verify that IPA client install does not configure certmonger Instead of making a note in the commit, I would prefer if you deleted the test case. There's no need to keep it if it's invalid, right? Honza You're right. I've deleted the case from the test plan. Thanks. The service-disable and host-disable tests fail with AlreadyInactive, because the certificate is removed with service-mod and host-mod in earlier tests. I think the service and host command tests should look like this: 1. Verify that {service,host}-del does not attempt to revoke {host,service}'s certificate {service,host}-add {service,host}-del 2. Verify that {service,host}-mod does not attempt to revoke {host,service}'s certificate {service,host}-add {service,host}-mod {service,host}-del 3. Verify that {service,host}-disable does not attempt to revoke {host,service}'s certificate {service,host}-add {service,host}-disable {service,host}-del There are a few wrong docstrings: +def test_service_mod_doesnt_revoke(self): +Verify that service-mod does not attempt to revoke host's certificate +def test_service_del_doesnt_revoke(self): +Verify that service-del does not attempt to revoke host's certificate +def test_ds_san(self): +Install new HTTP certificate with SAN -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 1104 move NULL check
Move a NULL check in the lockout plugin to address something Coverity found. There is no risk of a crash here but it there is also no point calling something if we know the values are NULL. rob From 6d490529b6d96838e0eaa3b29ac5c5f651b59875 Mon Sep 17 00:00:00 2001 From: Rob Crittenden rcrit...@redhat.com Date: Tue, 27 Aug 2013 17:34:46 -0400 Subject: [PATCH] Re-order NULL check in ipa_lockout. There is no risk of crash here as slapi_valueset_first_value() can handle the case where the valueset is NULL, but there is no point in calling that if we know there are no values. https://fedorahosted.org/freeipa/ticket/3880 --- daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c b/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c index 9e903aaf179e9a7a8e95f76ec25f47f7ba9a6be6..fd6602fdee9b2fd95c154fd512fcba4f37e56bad 100644 --- a/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c +++ b/daemons/ipa-slapi-plugins/ipa-lockout/ipa_lockout.c @@ -244,9 +244,8 @@ int ipalockout_getpolicy(Slapi_Entry *target_entry, Slapi_Entry **policy_entry, if (ldrc == 0) { Slapi_Value *sv = NULL; -slapi_valueset_first_value(*values, sv); - if (values != NULL) { +slapi_valueset_first_value(*values, sv); *policy_dn = slapi_value_get_string(sv); } } -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0055 Fix tests which fail after ipa-adtrust-install
On 08/28/2013 03:03 PM, Tomas Babej wrote: On 08/26/2013 09:38 AM, Ana Krivokapic wrote: On 08/22/2013 06:13 PM, Tomas Babej wrote: On 08/20/2013 04:14 PM, Ana Krivokapic wrote: On 08/09/2013 05:35 PM, Tomas Babej wrote: On 08/09/2013 04:03 PM, Ana Krivokapic wrote: On 08/09/2013 09:39 AM, Tomas Babej wrote: On 08/08/2013 04:09 PM, Ana Krivokapic wrote: Hello, This patch should fix the failing unit tests. https://fedorahosted.org/freeipa/ticket/3852 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel There are two tests failing on my machine when running the tests after ipa-adtrust-install with your patch applied: You say there are two tests failing but I only see one below. That was just debris from trying to break your patch too much, one of my comments rendered invalid in the end :) == FAIL: test_group[24]: group_find: Search for POSIX groups -- Traceback (most recent call last): [...] AssertionError: assert_deepequal: dict keys mismatch. test_group[24]: group_find: Search for POSIX groups missing keys = [] extra keys = ['ipantsecurityidentifier'] expected = {'dn': ipapython.dn.DN('cn=editors,cn=groups,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com'), 'cn': [u'editors'], 'objectclass': Fuzzy(None, None, function lambda at 0x3768c08), 'gidnumber': [Fuzzy('^\\d+$', type 'basestring', None)], 'ipauniqueid': [Fuzzy('^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$', type 'unicode', None)], 'description': [u'Limited admins who can edit other users']} got = {'dn': u'cn=editors,cn=groups,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com', 'cn': (u'editors',), 'objectclass': (u'top', u'groupofnames', u'posixgroup', u'ipausergroup', u'ipaobject', u'nestedGroup', u'ipantgroupattrs'), 'ipantsecurityidentifier': (u'S-1-5-21-1457515837-642396627-3509099663-1002',), 'gidnumber': (u'180462',), 'ipauniqueid': (u'7c6e1672-0039-11e3-9567-001a4a2221fb',), 'description': (u'Limited admins who can edit other users',)} path = ('result', 1) I think you need the wrap the dictionary discribing the editor's group entry with the add_sid wrapper, and its objectclasses using the add_oc wrapper. [tbabej@vm-139 freeipa]$ git diff diff --git a/ipatests/test_xmlrpc/test_group_plugin.py b/ipatests/test_xmlrpc/test_group_plugin.py index d380fe5..14c70cd 100644 --- a/ipatests/test_xmlrpc/test_group_plugin.py +++ b/ipatests/test_xmlrpc/test_group_plugin.py @@ -447,14 +447,15 @@ class test_group(Declarative): objectclasses.posixgroup, u'ipantgroupattrs')), 'ipauniqueid': [fuzzy_uuid], }), -{ +add_sid({ 'dn': get_group_dn('editors'), 'gidnumber': [fuzzy_digits], 'cn': [u'editors'], 'description': [u'Limited admins who can edit other users'], -'objectclass': fuzzy_set_ci(objectclasses.posixgroup), +'objectclass': fuzzy_set_ci(add_oc( +objectclasses.posixgroup, u'ipantgroupattrs')), 'ipauniqueid': [fuzzy_uuid], -}, +}), dict( dn=get_group_dn(group1), cn=[group1], These changes were sufficient for me to have the unit test suite run without errors. -- Tomas Babej Associate Software Engeneer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org I retested the patch and the tests are passing in my setup. The editors group definitely does not have the ipantsecurityidentifier attribute nor the ipantgroupattrs objectclass: [akrivoka@vm-181 freeipa]$ ipa group-show editors --all dn: cn=editors,cn=groups,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com Group name: editors Description: Limited admins who can edit other users GID: 197702 ipauniqueid: 91b3597e-00f3-11e3-92ae-001a4a22217b objectclass: top, groupofnames, posixgroup, ipausergroup, ipaobject, nestedGroup What I noticed though, is that if I delete and re-create the editors group (after ipa-adtrust-install has been run), it then gets the above mentioned attribute and objectclass. Maybe you did some similar manipulation in your setup, resulting in the test failing? I think it does depend on whether you have ran the ipa-sidgen task when running the ipa-adtrust-install. Do you think we can cover both cases here? -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. -- Tomas Babej Associate Software Engeneer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org
Re: [Freeipa-devel] [PATCH] 0257 Add initial CA-less installation tests
On 28.8.2013 17:15, Petr Viktorin wrote: On 08/28/2013 03:23 PM, Jan Cholasta wrote: Thanks. The service-disable and host-disable tests fail with AlreadyInactive, because the certificate is removed with service-mod and host-mod in earlier tests. I think the service and host command tests should look like this: 1. Verify that {service,host}-del does not attempt to revoke {host,service}'s certificate {service,host}-add {service,host}-del 2. Verify that {service,host}-mod does not attempt to revoke {host,service}'s certificate {service,host}-add {service,host}-mod {service,host}-del 3. Verify that {service,host}-disable does not attempt to revoke {host,service}'s certificate {service,host}-add {service,host}-disable {service,host}-del Fixed. I've used context managers so the -del isn't skipped when there's an error. There are a few wrong docstrings: +def test_service_mod_doesnt_revoke(self): +Verify that service-mod does not attempt to revoke host's certificate +def test_service_del_doesnt_revoke(self): +Verify that service-del does not attempt to revoke host's certificate +def test_ds_san(self): +Install new HTTP certificate with SAN Fixed. The TestIPACommands.service context manager does not call service-del. TestCertInstall.test_ds_san docstring still says HTTP instead of DS. Thanks for the review, fixed patch attached. The service-disable test still fails, due to https://fedorahosted.org/freeipa/ticket/3886. OK. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0257 Add initial CA-less installation tests
On 28.8.2013 19:39, Petr Viktorin wrote: On 08/28/2013 06:24 PM, Jan Cholasta wrote: On 28.8.2013 17:15, Petr Viktorin wrote: On 08/28/2013 03:23 PM, Jan Cholasta wrote: Thanks. The service-disable and host-disable tests fail with AlreadyInactive, because the certificate is removed with service-mod and host-mod in earlier tests. I think the service and host command tests should look like this: 1. Verify that {service,host}-del does not attempt to revoke {host,service}'s certificate {service,host}-add {service,host}-del 2. Verify that {service,host}-mod does not attempt to revoke {host,service}'s certificate {service,host}-add {service,host}-mod {service,host}-del 3. Verify that {service,host}-disable does not attempt to revoke {host,service}'s certificate {service,host}-add {service,host}-disable {service,host}-del Fixed. I've used context managers so the -del isn't skipped when there's an error. There are a few wrong docstrings: +def test_service_mod_doesnt_revoke(self): +Verify that service-mod does not attempt to revoke host's certificate +def test_service_del_doesnt_revoke(self): +Verify that service-del does not attempt to revoke host's certificate +def test_ds_san(self): +Install new HTTP certificate with SAN Fixed. The TestIPACommands.service context manager does not call service-del. That's not necessary, deleting the host deletes all its services. Ah, I missed the with self.host() part. How silly of me. TestCertInstall.test_ds_san docstring still says HTTP instead of DS. Fixed ACK! -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0257 Add initial CA-less installation tests
On 08/28/2013 08:10 PM, Jan Cholasta wrote: On 28.8.2013 19:39, Petr Viktorin wrote: On 08/28/2013 06:24 PM, Jan Cholasta wrote: On 28.8.2013 17:15, Petr Viktorin wrote: On 08/28/2013 03:23 PM, Jan Cholasta wrote: Thanks. The service-disable and host-disable tests fail with AlreadyInactive, because the certificate is removed with service-mod and host-mod in earlier tests. I think the service and host command tests should look like this: 1. Verify that {service,host}-del does not attempt to revoke {host,service}'s certificate {service,host}-add {service,host}-del 2. Verify that {service,host}-mod does not attempt to revoke {host,service}'s certificate {service,host}-add {service,host}-mod {service,host}-del 3. Verify that {service,host}-disable does not attempt to revoke {host,service}'s certificate {service,host}-add {service,host}-disable {service,host}-del Fixed. I've used context managers so the -del isn't skipped when there's an error. There are a few wrong docstrings: +def test_service_mod_doesnt_revoke(self): +Verify that service-mod does not attempt to revoke host's certificate +def test_service_del_doesnt_revoke(self): +Verify that service-del does not attempt to revoke host's certificate +def test_ds_san(self): +Install new HTTP certificate with SAN Fixed. The TestIPACommands.service context manager does not call service-del. That's not necessary, deleting the host deletes all its services. Ah, I missed the with self.host() part. How silly of me. TestCertInstall.test_ds_san docstring still says HTTP instead of DS. Fixed ACK! Thanks again! Pushed to master: 9b200c7c728604018bc56638a3d5e86c29d69099 ipa-3-3: c706859df2ae6a000d33874e4bb6bf79e9e9da52 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel