Re: [Freeipa-devel] [PATCHES] 165-166 Clean up ipa-server-certinstall CLI options

2013-08-28 Thread Petr Viktorin

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

2013-08-28 Thread Sumit Bose
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

2013-08-28 Thread Sumit Bose
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

2013-08-28 Thread Tomas Babej

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

2013-08-28 Thread Petr Viktorin

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

2013-08-28 Thread Tomas Babej

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

2013-08-28 Thread Petr Viktorin

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

2013-08-28 Thread Petr Viktorin

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

2013-08-28 Thread Rob Crittenden

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

2013-08-28 Thread Ana Krivokapic
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

2013-08-28 Thread Tomas Babej

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

2013-08-28 Thread Petr Viktorin

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

2013-08-28 Thread Petr Viktorin

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

2013-08-28 Thread Rob Crittenden

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

2013-08-28 Thread Petr Viktorin

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

2013-08-28 Thread Tomas Babej

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

2013-08-28 Thread Ana Krivokapic
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

2013-08-28 Thread Jan Cholasta

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

2013-08-28 Thread Rob Crittenden
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

2013-08-28 Thread Petr Viktorin

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

2013-08-28 Thread Jan Cholasta

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

2013-08-28 Thread Jan Cholasta

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

2013-08-28 Thread Petr Viktorin

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