Re: [Freeipa-devel] [PATCH] Add option to disable setkeytab extended operations

[Simo Sorce]

On Wed, 2015-12-02 at 08:40 +0100, Petr Spacek wrote:
> On 1.12.2015 12:00, Alexander Bokovoy wrote:
> > On Tue, 01 Dec 2015, Alexander Bokovoy wrote:
> >> On Tue, 01 Dec 2015, Petr Spacek wrote:
> >>> On 1.12.2015 09:47, Alexander Bokovoy wrote:
>  On Tue, 01 Dec 2015, Petr Spacek wrote:
> > On 1.12.2015 09:21, Alexander Bokovoy wrote:
> >> On Tue, 01 Dec 2015, Petr Spacek wrote:
> >>> On 24.11.2015 20:42, Simo Sorce wrote:
>  Since some time we use the getkeytab operation to fetch keytabs on 
>  newer
>  clients. According to bug #232 setkeytab can be used to circumvent
>  password quality controls so it needs to be slowly retired.
> 
>  The attached patches implement #5485 in 2 parts.
> 
>  The first introduces the option DisableSetKeytab which globally 
>  disables
>  the setkeytab extended operation. This is set to false by default for
>  backwards compatibility.
> 
>  The second introduces an option called DisableUserSetKeytab, which is
>  active by default in new installs (but not in upgraded ones), and 
>  only
>  disables the use of setkeytab for ipa suers, but not for 
>  hosts/services.
>  This is because user's are the ones that may abuse the interface to
>  escape password policies and users also normally do not acquire 
>  keytabs,
>  so it is a safe bet to disable just them by default in new installs.
> >>>
> >>> On a related note, how this works with plain kadmin & kpasswd 
> >>> protocols?
> >> It is unrelated. We don't support principal manipulation via kadmin
> >> protocol.
> >
> > Sure, I know that, but I'm trying to find out if we re-invented the 
> > wheel or
> > if our wheel has some additional features which cannot be incorporated 
> > to
> > the
> > original wheel :-)
>  There is no need to incorporate something specific into kadmin protocol,
>  the problem is with the fact that we don't have access controls within
>  our KDC driver. It always connects to LDAP server over ldapi as root and
>  thus maps to cn=Directory Manager which bypasses LDAP ACIs. As such,
>  this means we'll need to have some access control added to KDC DAL
>  driver before we can allow kadmin operations on principals.
> 
>  Adding those access controls is not an easy feat.
> >>>
> >>> Hmm, why is that? We could use LDAP Proxy control (RFC 4370) and let DS to
> >>> evaluate ACIs as usual. The change in kadmin would be adding LDAP Proxy
> >>> control with proper DN (i.e. mapping from principal name to DN) to the 
> >>> LDAP
> >>> requests.
> >> Not sure how does it help:
> >> $ ldapsearch -Dcn=Directory\ Manager -W -e
> >> \!authzid=dn:uid=abokovoy,cn=users,cn=accounts,dc=vda,dc=li -b
> >> uid=admin,cn=users,cn=accounts,dc=vda,dc=li userPassword
> >> Enter LDAP Password: # extended LDIF
> >> #
> >> # LDAPv3
> >> # base  with scope subtree
> >> # filter: (objectclass=*)
> >> # requesting: userPassword #
> >>
> >> # admin, users, accounts, vda.li
> >> dn: uid=admin,cn=users,cn=accounts,dc=vda,dc=li
> >> userPassword:: =
> >> =
> >>
> >> # search result
> >> search: 2
> >> result: 0 Success
> >>
> >> # numResponses: 2
> >> # numEntries: 1
> >> $ ldapsearch -Y GSSAPI -e
> >> \!authzid=dn:uid=admin,cn=users,cn=accounts,dc=vda,dc=li -b
> >> uid=admin,cn=users,cn=accounts,dc=vda,dc=li userPassword
> >> SASL/GSSAPI authentication started
> >> SASL username: aboko...@vda.li
> >> SASL SSF: 56
> >> SASL data security layer installed.
> >> # extended LDIF
> >> #
> >> # LDAPv3
> >> # base  with scope subtree
> >> # filter: (objectclass=*)
> >> # requesting: userPassword #
> >>
> >> # search result
> >> search: 4
> >> result: 0 Success
> >>
> >> # numResponses: 1
> >>
> >> As you can see, posing different authzid does not allow to get through
> >> ACLs unless you were already a directory manager. Which means the
> >> proxyauth control is not really useful here:
> >>
> >> [01/Dec/2015:10:51:22 +0100] conn=12466 op=0 BIND dn="cn=Directory Manager"
> >> method=128 version=3
> >> [01/Dec/2015:10:51:22 +0100] conn=12466 op=0 RESULT err=0 tag=97 nentries=0
> >> etime=0 dn="cn=directory manager"
> >> [01/Dec/2015:10:51:22 +0100] conn=12466 op=1 SRCH
> >> base="uid=admin,cn=users,cn=accounts,dc=vda,dc=li" scope=2
> >> filter="(objectClass=*)" attrs="userPassword"
> >> authzid="uid=abokovoy,cn=users,cn=accounts,dc=vda,dc=li"
> >> [01/Dec/2015:10:51:22 +0100] conn=12466 op=1 RESULT err=0 tag=101 
> >> nentries=1
> >> etime=0 notes=U
> >> [01/Dec/2015:10:51:22 +0100] conn=12466 op=2 UNBIND
> >>
> >> and
> >>
> >> [01/Dec/2015:10:48:44 +0100] conn=12465 op=0 BIND dn="" method=sasl
> >> version=3 mech=GSSAPI
> >> [01/Dec/2015:10:48:44 +0100] conn=12465 op=0 RESULT err=14 tag=97 
> >> nentries=0
> >> etime=0, SASL bind in progre

Re: [Freeipa-devel] [PATCH 556-557] Add option to disable setkeytab extended operations

[Simo Sorce]

On Tue, 2015-12-01 at 16:44 +0100, Petr Vobornik wrote:
> On 12/01/2015 04:20 PM, Alexander Bokovoy wrote:
> > On Tue, 01 Dec 2015, Martin Kosek wrote:
> >> On 12/01/2015 02:59 PM, Simo Sorce wrote:
> >>> On Tue, 2015-12-01 at 14:42 +0100, Martin Kosek wrote:
>  On 12/01/2015 02:38 PM, Simo Sorce wrote:
> > On Tue, 2015-12-01 at 10:11 +0200, Alexander Bokovoy wrote:
> >> On Mon, 30 Nov 2015, Simo Sorce wrote:
> >>> On Wed, 2015-11-25 at 09:47 -0500, Simo Sorce wrote:
>  On Wed, 2015-11-25 at 09:02 -0500, Rob Crittenden wrote:
> > Jan Cholasta wrote:
> >> On 24.11.2015 22:17, Simo Sorce wrote:
> >>> On Tue, 2015-11-24 at 14:57 -0500, Simo Sorce wrote:
>  On Tue, 2015-11-24 at 14:42 -0500, Simo Sorce wrote:
> > Since some time we use the getkeytab operation to fetch
> > keytabs on
> > newer
> > clients. According to bug #232 setkeytab can be used to
> > circumvent
> > password quality controls so it needs to be slowly retired.
> >
> > The attached patches implement #5485 in 2 parts.
> >
> > The first introduces the option DisableSetKeytab which
> > globally
> > disables
> > the setkeytab extended operation. This is set to false by
> > default for
> > backwards compatibility.
> >
> > The second introduces an option called
> > DisableUserSetKeytab, which is
> > active by default in new installs (but not in upgraded
> > ones), and only
> > disables the use of setkeytab for ipa suers, but not for
> > hosts/services.
> > This is because user's are the ones that may abuse the
> > interface to
> > escape password policies and users also normally do not
> > acquire
> > keytabs,
> > so it is a safe bet to disable just them by default in new
> > installs.
> >
> > (Testing in progress)
> 
>  Tested and working as expected.
> >>>
> >>> I realized that adding options to ipaConfig require to add
> >>> them in the
> >>> UI as well, attached patches add options in API.txt and
> >>> config.py
> >>> Make now complain I should change API Major or Minor, but it
> >>> is not
> >>> clear to me why given this are additional values and no real
> >>> change or
> >>> new function is introduced. What's the recommendation ?
> >>
> >> When does make complain? It is supposed to complain only when
> >> API.txt
> >> does not match code.
> >>
> >> Anyway, we usually bump minor version even for backward
> >> compatible
> >> changes, see e.g. commit 9549a59.
> >>
> >
> > The point of API.txt (and the heavy client) was to save a
> > round-trip.
> > Being able to pass in an invalid option would void that rule hence
> > having to update the API when new values are added.
> >
> > rob
> 
>  Ok added version change to the second patch (so we bump it only
>  once
>  given these are basically related changes.
> >>>
> >>> Bump, is this ok ?
> >> This patch is fine but please fix setkeytab use in ipa-sam before
> >> committing this patch.
> >
> > This patch does not disable setkeytab yet, so it can go in right away
> > (it blocks other patches already). We have a bug to change ipa-sam,
> > should we mark it blocker for 4.4 ?
> 
>  We also have to fix the permission to change keytab, so that it uses
>  the new
>  style (https://fedorahosted.org/freeipa/ticket/5487). I would
>  actually make
>  this ticket and the ipa-sam ticket a blocker for this patch set.
> 
>  Otherwise you are actually introducing a switch that breaks FreeIPA
>  as some of
>  it's core server functions still use the old method.
> >>>
> >>> The point of this patchset is to introduce the switch early so that
> >>> current server will support the off switch when newer servers down the
> >>> road are ready to flip it. The defaults are still to allow these
> >>> operations for services/hosts.
> >>
> >> I still do not get the logic about an early switch. Now, if switch is
> >> turned
> >> on, FreeIPA server breaks on several places. I would really rather
> >> expect the
> >> switch to be introduced when the server itself supports it. Then,
> >> admin would
> >> enable it when the clients are sufficiently updated and can use the
> >> new method.
> >>
> >> Why would admin want to enable the switch early if it breaks FreeIPA
> >> some of
> >> the FreeIPA servers? Permission can be upgraded in newer version and get
> >> replicated, that's 

[Freeipa-devel] patch acceptance criteria

[Rob Crittenden]

Is it still mandatory that tests pass the unit tests before acceptance?
I've seen a number of cases over the past couple of months where a
change goes through then shortly afterward a patch to fix the tests.
IMHO this should be caught in advance.

Things slip through and goodness knows I've acked more than a few
patches without running the full suite. I just have a feeling it has
become more frequent lately.

rob

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

[Freeipa-devel] [PATCH 0368] FIX DNS tests

[Martin Basti]

Test needs to be updated due deprecation of dns-resolve command.

Patch attached
From 0166e2455b273a5f8f2811edd92b8287b5d77222 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Mon, 30 Nov 2015 19:04:02 +0100
Subject: [PATCH] Fix DNS tests: dns-resolve returns warning

---
 ipatests/test_xmlrpc/test_dns_plugin.py | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index 5f692528eb9a5ae0dc488c663ab43cc47ffd29b3..45b0217c8027aff4b8de81011c21a4f03bec336c 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -2883,6 +2883,15 @@ class test_dns(Declarative):
 'result': True,
 'summary': "Found '%s'" % wildcard_rec1_test1,
 'value': wildcard_rec1_test1,
+'messages': ({
+'message': u"'dns-resolve' is deprecated. The "
+   u"command may return an unexpected result, "
+   u"the resolution of the DNS domain is done "
+   u"on a randomly chosen IPA server.",
+'code': 13015,
+'type': u'warning',
+'name': u'CommandDeprecatedWarning'
+},)
 },
 ),
 
@@ -2894,6 +2903,15 @@ class test_dns(Declarative):
 'result': True,
 'summary': "Found '%s'" % wildcard_rec1_test2,
 'value': wildcard_rec1_test2,
+'messages': ({
+'message': u"'dns-resolve' is deprecated. The "
+   u"command may return an unexpected result, "
+   u"the resolution of the DNS domain is done "
+   u"on a randomly chosen IPA server.",
+'code': 13015,
+'type': u'warning',
+'name': u'CommandDeprecatedWarning'
+},)
 },
 ),
 
-- 
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] Removed duplicate domain name validation function

[Martin Basti]

On 02.12.2015 17:28, Martin Basti wrote:



On 01.12.2015 16:03, Stanislav Laznicka wrote:

Sending the patch with renamed function.

Standa

On 12/01/2015 09:57 AM, Jan Cholasta wrote:

On 1.12.2015 09:37, Petr Spacek wrote:

On 30.11.2015 20:00, Martin Basti wrote:



On 27.11.2015 16:06, Stanislav Laznicka wrote:

Please, see the modified patch attached.

Standa

On 11/27/2015 03:48 PM, Martin Basti wrote:



On 27.11.2015 15:33, Petr Spacek wrote:

On 27.11.2015 15:32, Martin Basti wrote:


On 25.11.2015 17:18, Stanislav Laznicka wrote:

There were two functions for the same purpose. Removed one.



Hello,

I would like to have "log" param of is_host_resolvable as 
optional
Is there an immediate need for the optional param? If not, I 
would not

clutter
the code.

So at least I would like to move log param as the last param, in 
case of

need it can be modified to optional parameter.

Or log can be default as root_logger, IMO we us only root_logger
everywhere, but this need investigation.

Martin


It works, but I would like to have Honza's opinion if 
ipalib/util.py is the
right place for the new method and if we really need to use 
exceptions.


ipalib/util.py is fine for the function, since it is not used 
anywhere in ipapython.


I would rename the function to verify_host_resolvable() though, 
since the is_ prefix suggests the function returns a boolean value 
rather than raises an exception or not.




IMO is_record_resolvable() should return only True/False and then 
it can be

located in ipapython module


Hmm, I do not see a reason why it should not throw an exception. 
IMHO the
exception should contain the original error/result from resolver, 
so it is
possible to print meaningful error message like "DNS query timed 
out" instead

of "it does not work for some reason, trust us".







Pushed to master: 498471e4aed1367b72cd74d15811d0584a6ee268




Of course I forgot to add ACK
-- 
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] Removed duplicate domain name validation function

[Martin Basti]

On 01.12.2015 16:03, Stanislav Laznicka wrote:

Sending the patch with renamed function.

Standa

On 12/01/2015 09:57 AM, Jan Cholasta wrote:

On 1.12.2015 09:37, Petr Spacek wrote:

On 30.11.2015 20:00, Martin Basti wrote:



On 27.11.2015 16:06, Stanislav Laznicka wrote:

Please, see the modified patch attached.

Standa

On 11/27/2015 03:48 PM, Martin Basti wrote:



On 27.11.2015 15:33, Petr Spacek wrote:

On 27.11.2015 15:32, Martin Basti wrote:


On 25.11.2015 17:18, Stanislav Laznicka wrote:

There were two functions for the same purpose. Removed one.



Hello,

I would like to have "log" param of is_host_resolvable as optional
Is there an immediate need for the optional param? If not, I 
would not

clutter
the code.

So at least I would like to move log param as the last param, in 
case of

need it can be modified to optional parameter.

Or log can be default as root_logger, IMO we us only root_logger
everywhere, but this need investigation.

Martin


It works, but I would like to have Honza's opinion if 
ipalib/util.py is the
right place for the new method and if we really need to use 
exceptions.


ipalib/util.py is fine for the function, since it is not used 
anywhere in ipapython.


I would rename the function to verify_host_resolvable() though, since 
the is_ prefix suggests the function returns a boolean value rather 
than raises an exception or not.




IMO is_record_resolvable() should return only True/False and then 
it can be

located in ipapython module


Hmm, I do not see a reason why it should not throw an exception. 
IMHO the
exception should contain the original error/result from resolver, so 
it is
possible to print meaningful error message like "DNS query timed 
out" instead

of "it does not work for some reason, trust us".







Pushed to master: 498471e4aed1367b72cd74d15811d0584a6ee268

-- 
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 0364, 0367] ipa-kra-install: allow first KRA to be installed on replica

[Martin Basti]

On 02.12.2015 14:52, Martin Babinsky wrote:

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



On 30.11.2015 14:16, Martin Babinsky wrote:

On 11/27/2015 05:02 PM, Martin Basti wrote:

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

I tested just master, I will test ipa-4-2 later.
patch attached.




ACK for the master branch.


Thanks, additional patch improves error message when ipa-replica-install
--setup-ca --setup-kra is executed and KRA is not installed anywhere 
yet.


I'm working on patches for ipa-4-2 branch

Martin


ACK for patch 367.


Pushed to master: bbbe411f357b7fbad533b5211a90bb0558b1abbe

IPA 4.2 patches attached.
From 9166a7ad5243bff7681f1b8591536e45da2d6669 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Mon, 23 Nov 2015 13:43:53 +0100
Subject: [PATCH 1/2] ipa-kra-install: allow to install first KRA on replica

https://fedorahosted.org/freeipa/ticket/5460
---
 ipaserver/install/krainstance.py | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/ipaserver/install/krainstance.py b/ipaserver/install/krainstance.py
index a2514debae600bdc46afb92e426a5f616529fde2..625d84ab3129708cfdaf759cee6c2953b585a822 100644
--- a/ipaserver/install/krainstance.py
+++ b/ipaserver/install/krainstance.py
@@ -225,17 +225,13 @@ class KRAInstance(DogtagInstance):
 str(DN(('uid', 'pkidbuser'), ('ou', 'people'), ('o', 'ipaca'
 
 _p12_tmpfile_handle, p12_tmpfile_name = tempfile.mkstemp(dir=paths.TMP)
+
 if self.clone:
 krafile = self.pkcs12_info[0]
 shutil.copy(krafile, p12_tmpfile_name)
 pent = pwd.getpwnam(PKI_USER)
 os.chown(p12_tmpfile_name, pent.pw_uid, pent.pw_gid)
 
-# create admin cert file if it does not exist
-cert = DogtagInstance.get_admin_cert(self)
-with open(paths.ADMIN_CERT_PATH, "w") as admin_path:
-admin_path.write(cert)
-
 # Security domain registration
 config.set("KRA", "pki_security_domain_hostname", self.master_host)
 config.set("KRA", "pki_security_domain_https_port", "443")
@@ -252,6 +248,11 @@ class KRAInstance(DogtagInstance):
 "KRA", "pki_clone_uri",
 "https://%s"; % ipautil.format_netloc(self.master_host, 443))
 
+# the admin cert file is needed for the KRA
+cert = DogtagInstance.get_admin_cert(self)
+with open(paths.ADMIN_CERT_PATH, "w") as admin_path:
+admin_path.write(cert)
+
 # Generate configuration file
 with open(cfg_file, "wb") as f:
 config.write(f)
-- 
2.5.0

From e81fa00593541df6e8990b102c4b5a0c0829adbe Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Mon, 30 Nov 2015 18:18:38 +0100
Subject: [PATCH 2/2] Modify error message to install first instance of KRA

First instance of KRA should be installed by ipa-kra-install.

https://fedorahosted.org/freeipa/ticket/5460
---
 ipaserver/install/kra.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/kra.py b/ipaserver/install/kra.py
index f317e7aa0f5d9bec01ec743fb42f06cdff83d03c..e506f10a39234e1537a3233cd68abee262524f7b 100644
--- a/ipaserver/install/kra.py
+++ b/ipaserver/install/kra.py
@@ -34,7 +34,9 @@ def install_check(api, replica_config, options):
 
 if replica_config is not None:
 if not api.Command.kra_is_enabled()['result']:
-raise RuntimeError("KRA is not installed on the master system")
+raise RuntimeError(
+"KRA is not installed on the master system. Please use "
+"'ipa-kra-install' command to install the first instance.")
 
 with certdb.NSSDatabase() as tmpdb:
 pw = ipautil.write_tmp_file(ipautil.ipa_generate_password())
-- 
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 0388] tests: Add hostmask detection for sudo rules validating

[Lukas Slebodnik]

On (02/12/15 15:41), Tomas Babej wrote:
>
>
>On 12/02/2015 09:24 AM, Tomas Babej wrote:
>> 
>> 
>> On 12/01/2015 06:27 PM, Tomas Babej wrote:
>>>
>>>
>>> On 11/30/2015 05:32 PM, Lukas Slebodnik wrote:
 On (30/11/15 13:09), Tomas Babej wrote:
> Hi,
>
> 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

 >From e6f1846f0d7d17303e5b30b1643651ba739b2b6c Mon Sep 17 00:00:00 2001
> From: Tomas Babej 
> Date: Mon, 30 Nov 2015 12:53:39 +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.
>
>>>
>>> Thanks, updated patch attached.
>>>
>>> Tomas
>>>
>> 
>> Actually, a small improvement is necessary.
>> 
>> Updated patch attached.
>> 
>> Tomas
>> 
>> 
>> 
>
>Thanks to Lukas, we found another problem with the test.
>
>Updated patch attached.
>
Thank you for 4th revision of patch
but there is still one issue.

=== FAILURES ===
_ TestSudo.test_sudo_rule_restricted_to_one_hostmask_negative __

self = 

def test_sudo_rule_restricted_to_one_hostmask_negative(self):
result1 = self.list_sudo_commands("testuser1")
>   assert result1.returncode != 0
E   assert 0 != 0
E+  where 0 = .returncode

test_integration/test_sudo.py:323: AssertionError
 1 failed, 74 passed in 807.35 seconds =

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

Re: [Freeipa-devel] [patch 0025] Separated Tracker implementations into standalone package

[Martin Basti]

On 02.12.2015 16:53, Aleš Mareček wrote:


- Original Message -

From: "Milan Kubík" 
To: "Martin Basti" 
Cc: freeipa-devel@redhat.com, "Aleš Mareček" 
Sent: Tuesday, December 1, 2015 10:31:14 AM
Subject: Re: [Freeipa-devel] [patch 0025] Separated Tracker implementations 
into standalone package

On 11/30/2015 07:13 PM, Martin Basti wrote:

NACK

1)
With this patch I received this error in test_user_plugin.py

E   AssertionError: assert_deepequal: expected != got.
E 0106: user_status: Query status of "tuser1"
E expected = 1
E got = 2
E path = ('count',)

I have just admin user on my system

2)
__
ERROR collecting test_xmlrpc/test_stageuser_plugin.py
__
test_xmlrpc/test_stageuser_plugin.py:28: in 
 from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
E   ImportError: No module named tracker.user_plugin
__
ERROR collecting test_xmlrpc/test_stageuser_plugin.py
__
test_xmlrpc/test_stageuser_plugin.py:28: in 
 from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
E   ImportError: No module named tracker.user_plugin

3)

ERROR collecting test_xmlrpc/test_group_plugin.py

test_xmlrpc/test_group_plugin.py:34: in 
 from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
E   ImportError: No module named tracker.user_plugin

ERROR collecting test_xmlrpc/test_group_plugin.py

test_xmlrpc/test_group_plugin.py:34: in 
 from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
E   ImportError: No module named tracker.user_plugin

4)

ERROR collecting test_xmlrpc/test_host_plugin.py
_
test_xmlrpc/test_host_plugin.py:42: in 
 from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
E   ImportError: No module named tracker.host_plugin

ERROR collecting test_xmlrpc/test_host_plugin.py
_
test_xmlrpc/test_host_plugin.py:42: in 
 from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
E   ImportError: No module named tracker.host_plugin

5)

ERROR collecting test_xmlrpc/test_caacl_plugin.py

test_xmlrpc/test_caacl_plugin.py:15: in 
 from ipatests.test_xmlrpc.test_certprofile_plugin import
default_profile
../_pytest/assertion/rewrite.py:171: in load_module
 py.builtin.exec_(co, mod.__dict__)
test_xmlrpc/test_certprofile_plugin.py:17: in 
 from ipatests.test_xmlrpc.tracker.certprofile_plugin import
CertprofileTracker
E   ImportError: No module named tracker.certprofile_plugin

ERROR collecting test_xmlrpc/test_caacl_plugin.py

test_xmlrpc/test_caacl_plugin.py:15: in 
 from ipatests.test_xmlrpc.test_certprofile_plugin import
default_profile
../_pytest/assertion/rewrite.py:171: in load_module
 py.builtin.exec_(co, mod.__dict__)
test_xmlrpc/test_certprofile_plugin.py:17: in 
 from ipatests.test_xmlrpc.tracker.certprofile_plugin import
CertprofileTracker
E   ImportError: No module named tracker.certprofile_plugin




N)
rpm -ql freeipa-tests | grep tracker
returns nothing, IMO the tracker directory in not in RPM

All failing parts PASSED.



On 30.11.2015 17:50, Aleš Mareček wrote:

Tested with today's master, ACK.
   - alich -

- Original Message -

From: "Milan Kubík" 
To: freeipa-devel@redhat.com
Sent: Friday, November 27, 2015 3:40:29 PM
Subject: Re: [Freeipa-devel] [patch 0025] Separated Tracker
implementations into standalone package

On 11/27/2015 03:36 PM, Milan Kubík wrote:



On 11/27/2015 03:31 PM, Milan Kubík wrote:


On 11/23/2015 10:43 AM, Lenka Doudova wrote:


NACK - there's a "typo" in /tracker/user_plugin.py, line 17-18:

def get_user_dn(cn):

return DN(('cn', cn), api.env.container_user, api.env.basedn)


should be

def get_user_dn(uid):

return DN(('uid', uid), api.env.container_user, api.en

Re: [Freeipa-devel] [PATCH] First part of the replica promotion tests + testplan

[Martin Basti]

On 02.12.2015 16:18, Oleg Fayans wrote:

Hi Martin,

On 12/01/2015 04:08 PM, Martin Basti wrote:



On 27.11.2015 16:26, Oleg Fayans wrote:

And patch N 16 passes lint too:

On 11/27/2015 04:03 PM, Oleg Fayans wrote:

Hi,

On 11/27/2015 03:26 PM, Martin Basti wrote:



On 27.11.2015 15:04, Oleg Fayans wrote:

Hi Martin,

All your suggestions were taken into account. Both patches are
updated. Thank you for your help!

On 11/26/2015 10:50 AM, Martin Basti wrote:



On 26.11.2015 10:04, Oleg Fayans wrote:

Hi Martin,

I agree to all your points but one. please, see my comment below

On 11/25/2015 07:42 PM, Martin Basti wrote:

Hi,

0) Note
Please be aware of https://fedorahosted.org/freeipa/ticket/5469
during
KRA testing

1)
Please do not use MIN and MAX_DOMAIN_LEVEL constants, this may
change
over time, use DOMAIN_LEVEL_0 and DOMAIN_LEVEL_1 for domain 
level 0

and 1

2)
Why uninstall KRA then server, is not enough just uninstall 
server

which
covers KRA uninstall?

+def teardown_method(self, method):
+for host in self.replicas:
+host.run_command(self.kra_uninstall, 
raiseonerr=False)

+tasks.uninstall_master(host)


3)
Can be this function more generic? It should allow specify host
where
KRA should be installed not just master

+def test_kra_install_master(self):
+self.master.run_command(self.kra_install)


4)

TestLevel0(Dummy):
Can be the test name more specific, something like
TestReplicaPromotionLevel0


5)
please remove this, the patch is on review and it will be pushed
sooner
than tests
+@pytest.mark.xfail  # Ticket N 5455

and as I mentioned in ticket #5455, I cannot reproduce it with
ipa-kra-install, so please provide steps to reproduce if you 
insist

that
this still does not work as expected with KRA.

6) This is completely wrong, it removes everything that we 
tried to

achieve with previous patches with domain level in CI


Actually, being able to configure domain level per class is WAY 
more

convenient, than to always have to think which domain level is
appropriate for which particular test during jenkins job
configuration. In fact, I should have thought about it from the 
very

beginning. For example, in test_replica_promotion.py we have on
class,
which intiates with domain level = 1, while others - with domain
level
0. With config-based approach, we would have to implement a 
separate

step that raises domain level. Overall, I am against the approach,
when you have to remember to set certain domain level in config 
for

any particular test. The tests themselves should be aware of the
domain level they need.

I do not say that we should not have something that overrides
settings
in from config in a particular test case, I say your patch is
doing it
wrong.

I agree it is useful to have param domain_level in install_master,
and
intall_topo methods,  but is cannot be MAX_DOMAIN_LEVEL by default,
because with your current patch the domain_level in config is not
used
at all, it will be always MAX_DOMAIN_LEVEL

For example I want to achieve this goal:
test_vault.py, this test suite can run on domain level1 and on 
domain

level0, so with one test we can test 2 domain levels just with
putting
domain level into config file.

I agree that with extraordinary test like replica promotion test
is, we
need something that allows override the config file.

As I said bellow, domain_level default value should be None in
install_master and install_topo plugin. If domain level was 
specified

use the specified one, if not (value is None) use the domain level
from
config file.


Agreed :)



Martin




[PATCH] Enabled setting domain_level per class derived from
TestIntegration

When I configure domain level 0 in yaml config, how is this
supposed to
get into install methods when you removed that code?

-"--domain-level=%i" % host.config.domain_level
+"--domain-level=%i" % domain_level


You always use MAX_DOMAIN_LEVEL in this case or whatever is
specified in
domain_level option.
I suggest to use domain_level=None, and when it is None use
'host.config.domain_level', if it is not None, use 'domain_level'

With this we can specify domain level in config file for test 
that

can
be used for both domain levels and you can manually specify 
domain

level
for test that requires specific domain level.

Also this should go away

  @classmethod
  def install(cls, mh):
+if hasattr(cls, "domain_level") and cls.master:
+cls.master.config.domain_level = cls.domain_level
  if cls.topology is None:
  return

I do not see reason why test should override configuration in
config in
this case.

Martin

On 25.11.2015 16:44, Oleg Fayans wrote:

Hi,

Here is the updated version of the patch (more tests + fixed the
issues of the first one) + patch 0017, that implements the
necessary
changes in the background code, i. e. patch 16 does not work
without
patch 17

On 11/18/2015 05:20 PM, Martin Basti wrote:



On 09.11.2015

Re: [Freeipa-devel] [patch 0025] Separated Tracker implementations into standalone package

[Aleš Mareček]

- Original Message -
> From: "Milan Kubík" 
> To: "Martin Basti" 
> Cc: freeipa-devel@redhat.com, "Aleš Mareček" 
> Sent: Tuesday, December 1, 2015 10:31:14 AM
> Subject: Re: [Freeipa-devel] [patch 0025] Separated Tracker implementations 
> into standalone package
> 
> On 11/30/2015 07:13 PM, Martin Basti wrote:
> > NACK
> >
> > 1)
> > With this patch I received this error in test_user_plugin.py
> >
> > E   AssertionError: assert_deepequal: expected != got.
> > E 0106: user_status: Query status of "tuser1"
> > E expected = 1
> > E got = 2
> > E path = ('count',)
> >
> > I have just admin user on my system
> >
> > 2)
> > __
> > ERROR collecting test_xmlrpc/test_stageuser_plugin.py
> > __
> > test_xmlrpc/test_stageuser_plugin.py:28: in 
> > from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
> > E   ImportError: No module named tracker.user_plugin
> > __
> > ERROR collecting test_xmlrpc/test_stageuser_plugin.py
> > __
> > test_xmlrpc/test_stageuser_plugin.py:28: in 
> > from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
> > E   ImportError: No module named tracker.user_plugin
> >
> > 3)
> > 
> > ERROR collecting test_xmlrpc/test_group_plugin.py
> > 
> > test_xmlrpc/test_group_plugin.py:34: in 
> > from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
> > E   ImportError: No module named tracker.user_plugin
> > 
> > ERROR collecting test_xmlrpc/test_group_plugin.py
> > 
> > test_xmlrpc/test_group_plugin.py:34: in 
> > from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
> > E   ImportError: No module named tracker.user_plugin
> >
> > 4)
> > 
> > ERROR collecting test_xmlrpc/test_host_plugin.py
> > _
> > test_xmlrpc/test_host_plugin.py:42: in 
> > from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
> > E   ImportError: No module named tracker.host_plugin
> > 
> > ERROR collecting test_xmlrpc/test_host_plugin.py
> > _
> > test_xmlrpc/test_host_plugin.py:42: in 
> > from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
> > E   ImportError: No module named tracker.host_plugin
> >
> > 5)
> > 
> > ERROR collecting test_xmlrpc/test_caacl_plugin.py
> > 
> > test_xmlrpc/test_caacl_plugin.py:15: in 
> > from ipatests.test_xmlrpc.test_certprofile_plugin import
> > default_profile
> > ../_pytest/assertion/rewrite.py:171: in load_module
> > py.builtin.exec_(co, mod.__dict__)
> > test_xmlrpc/test_certprofile_plugin.py:17: in 
> > from ipatests.test_xmlrpc.tracker.certprofile_plugin import
> > CertprofileTracker
> > E   ImportError: No module named tracker.certprofile_plugin
> > 
> > ERROR collecting test_xmlrpc/test_caacl_plugin.py
> > 
> > test_xmlrpc/test_caacl_plugin.py:15: in 
> > from ipatests.test_xmlrpc.test_certprofile_plugin import
> > default_profile
> > ../_pytest/assertion/rewrite.py:171: in load_module
> > py.builtin.exec_(co, mod.__dict__)
> > test_xmlrpc/test_certprofile_plugin.py:17: in 
> > from ipatests.test_xmlrpc.tracker.certprofile_plugin import
> > CertprofileTracker
> > E   ImportError: No module named tracker.certprofile_plugin
> >
> >
> > 
> >
> > N)
> > rpm -ql freeipa-tests | grep tracker
> > returns nothing, IMO the tracker directory in not in RPM

All failing parts PASSED.

> >
> >
> > On 30.11.2015 17:50, Aleš Mareček wrote:
> >> Tested with today's master, ACK.
> >>   - alich -
> >>
> >> - Original Message -
> >>> From: "Milan Kubík" 
> >>> To: freeipa-devel@redhat.com
> >>> Sent: Friday, November 27, 2015 3:40:29 PM
> >>> Subject: Re: [Freeipa-devel] [patch 0025] Separated Tracker
> >>> implementations into standalone package
> >>>
> >>> On 11/27/2015 03:3

[Freeipa-devel] [PATCH] 939 topologysuffix: change iparepltopoconfroot API properties

[Petr Vobornik]

Change CLI option, label and type to reflect that it is a only a DN
of the suffix.
--
Petr Vobornik
From 746ac711ba96e9f5726e2aa37814e376a197219c Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Tue, 1 Dec 2015 13:02:18 +0100
Subject: [PATCH] topologysuffix: change iparepltopoconfroot API properties

Change CLI option, label and type to reflect that it is a only a DN
of the suffix.
---
 API.txt|  6 +++---
 VERSION|  4 ++--
 ipalib/plugins/topology.py | 10 --
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/API.txt b/API.txt
index 1e6942d067034cdc4b1d165fbe5d8ae580ecb688..60c98c31aa85d6c8879cd145f3d84188d4fea5b7 100644
--- a/API.txt
+++ b/API.txt
@@ -4921,7 +4921,7 @@ args: 1,6,3
 arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('iparepltopoconfroot', attribute=True, cli_name='suffix', maxlength=255, multivalue=False, required=True)
+option: DNParam('iparepltopoconfroot', attribute=True, cli_name='suffix_dn', multivalue=False, required=True)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('version?', exclude='webui')
@@ -4941,7 +4941,7 @@ args: 1,8,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('cn', attribute=True, autofill=False, cli_name='name', multivalue=False, primary_key=True, query=True, required=False)
-option: Str('iparepltopoconfroot', attribute=True, autofill=False, cli_name='suffix', maxlength=255, multivalue=False, query=True, required=False)
+option: DNParam('iparepltopoconfroot', attribute=True, autofill=False, cli_name='suffix_dn', multivalue=False, query=True, required=False)
 option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Int('sizelimit?', autofill=False, minvalue=0)
@@ -4957,7 +4957,7 @@ arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=Tr
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('delattr*', cli_name='delattr', exclude='webui')
-option: Str('iparepltopoconfroot', attribute=True, autofill=False, cli_name='suffix', maxlength=255, multivalue=False, required=False)
+option: DNParam('iparepltopoconfroot', attribute=True, autofill=False, cli_name='suffix_dn', multivalue=False, required=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Flag('rights', autofill=True, default=False)
 option: Str('setattr*', cli_name='setattr', exclude='webui')
diff --git a/VERSION b/VERSION
index 69a8a29708041d8e395c909060a14532c6855ca5..b7f261b5c5927053e2c2bfa4e3e7075c07caab5c 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=160
-# Last change: jcholast - server: use topologysuffix name in iparepltopomanagedsuffix
+IPA_API_VERSION_MINOR=161
+# Last change: pvoborni - topologysuffix: change iparepltopomanagedsuffix type
diff --git a/ipalib/plugins/topology.py b/ipalib/plugins/topology.py
index 2c54bbc4dacd04ab95e55c82852428f6b0e4b1aa..40f9fa8038d4bcb6c0eeb82a37ac796d732b82fd 100644
--- a/ipalib/plugins/topology.py
+++ b/ipalib/plugins/topology.py
@@ -5,7 +5,7 @@
 import six
 
 from ipalib import api, errors
-from ipalib import Int, Str, Bool, StrEnum, Flag
+from ipalib import Int, Str, Bool, StrEnum, Flag, DNParam
 from ipalib.plugable import Registry
 from ipalib.plugins.baseldap import (
 LDAPObject, LDAPSearch, LDAPCreate, LDAPDelete, LDAPUpdate, LDAPQuery,
@@ -342,12 +342,10 @@ class topologysuffix(LDAPObject):
 primary_key=True,
 label=_('Suffix name'),
 ),
-Str(
+DNParam(
 'iparepltopoconfroot',
-maxlength=255,
-cli_name='suffix',
-label=_('LDAP suffix to be managed'),
-normalizer=lambda value: value.lower(),
+cli_name='suffix_dn',
+label=_('Managed LDAP suffix DN'),
 ),
 )
 
-- 
2.4.3

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

[Freeipa-devel] [PATCH 522] replica promotion: allow OTP bulk client enrollment

[Jan Cholasta]

Hi,

the attached patch fixes .

Note that you still have to provide admin password in 
ipa-replica-install, either using --admin-password or interactively, 
because:


a) Admin password is required for replica promotion. This will be fixed 
with .


Patches are on the list: 
.


b) Admin password is required for connection check. This will be fixed 
with .


Honza

--
Jan Cholasta
From 251df9d82f59183cec876ed2dbc6efe05d21ffb1 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 2 Dec 2015 15:57:59 +0100
Subject: [PATCH] replica promotion: allow OTP bulk client enrollment

https://fedorahosted.org/freeipa/ticket/5498
---
 ipaserver/install/server/replicainstall.py | 42 +++---
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 74069f0..0f0a9c7 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -756,7 +756,9 @@ def ensure_enrolled(installer):
 config = installer._config
 
 # Perform only if we have the necessary options
-if not any([installer.admin_password, installer.keytab]):
+if not any([installer.password,
+installer.admin_password,
+installer.keytab]):
 sys.exit("IPA client is not configured on this system.\n"
  "You must join the system by running 'ipa-client-install' "
  "first. Alternatively, you may specify enrollment related "
@@ -766,6 +768,8 @@ def ensure_enrolled(installer):
 service.print_msg("Configuring client side components")
 try:
 args = [paths.IPA_CLIENT_INSTALL, "--unattended"]
+stdin = None
+
 if installer.domain_name:
 args.extend(["--domain", installer.domain_name])
 if installer.server:
@@ -775,12 +779,16 @@ def ensure_enrolled(installer):
 if installer.host_name:
 args.extend(["--hostname", installer.host_name])
 
-if installer.admin_password:
-# Always set principal if password was set explicitly,
-# the password itself gets passed directly via stdin
-args.extend(["--principal", installer.principal or "admin"])
-if installer.keytab:
-args.extend(["--keytab", installer.keytab])
+if installer.password:
+args.extend(["--password", installer.password])
+else:
+if installer.admin_password:
+# Always set principal if password was set explicitly,
+# the password itself gets passed directly via stdin
+args.extend(["--principal", installer.principal or "admin"])
+stdin = installer.admin_password
+if installer.keytab:
+args.extend(["--keytab", installer.keytab])
 
 if installer.no_dns_sshfp:
 args.append("--no-dns-sshfp")
@@ -793,7 +801,7 @@ def ensure_enrolled(installer):
 if installer.mkhomedir:
 args.append("--mkhomedir")
 
-ipautil.run(args, stdin=installer.admin_password or None)
+ipautil.run(args, stdin=stdin)
 
 except Exception as e:
 sys.exit("Configuration of client side components failed!\n"
@@ -1164,11 +1172,14 @@ class Replica(BaseServer):
  "multiple times"),
 )
 
-dm_password = Knob(
+dm_password = None
+
+password = Knob(
 BaseServer.dm_password,
-description="Directory Manager (existing master) password",
-cli_name='password',
-cli_metavar='PASSWORD',
+description=("Password to join the IPA realm. Assumes bulk password "
+ "unless principal is also set. (domain level 1+)\n"
+ "Directory Manager (existing master) password. "
+ "(domain level 0)"),
 )
 
 admin_password = Knob(
@@ -1246,6 +1257,10 @@ class Replica(BaseServer):
 
 if self.replica_file is None:
 self.promote = True
+
+if self.principal and not self.admin_password:
+self.admin_password = self.password
+self.password = None
 else:
 if not ipautil.file_exists(self.replica_file):
 raise RuntimeError("Replica file %s does not exist"
@@ -1258,7 +1273,6 @@ class Replica(BaseServer):
 CLIKnob(self.domain_name, '--domain'),
 CLIKnob(self.host_name, '--hostname'),
 CLIKnob(self.server, '--server'),
-CLIKnob(self.admin_password, '--admin-password'),
 CLIKnob(self.principal, '--principal'),
 )
 
@@ -1281,8 +1295,6 @@ class Replica(BaseServer):
 "You must specif

Re: [Freeipa-devel] [PATCH] First part of the replica promotion tests + testplan

[Oleg Fayans]

Hi Martin,

On 12/01/2015 04:08 PM, Martin Basti wrote:



On 27.11.2015 16:26, Oleg Fayans wrote:

And patch N 16 passes lint too:

On 11/27/2015 04:03 PM, Oleg Fayans wrote:

Hi,

On 11/27/2015 03:26 PM, Martin Basti wrote:



On 27.11.2015 15:04, Oleg Fayans wrote:

Hi Martin,

All your suggestions were taken into account. Both patches are
updated. Thank you for your help!

On 11/26/2015 10:50 AM, Martin Basti wrote:



On 26.11.2015 10:04, Oleg Fayans wrote:

Hi Martin,

I agree to all your points but one. please, see my comment below

On 11/25/2015 07:42 PM, Martin Basti wrote:

Hi,

0) Note
Please be aware of https://fedorahosted.org/freeipa/ticket/5469
during
KRA testing

1)
Please do not use MIN and MAX_DOMAIN_LEVEL constants, this may
change
over time, use DOMAIN_LEVEL_0 and DOMAIN_LEVEL_1 for domain level 0
and 1

2)
Why uninstall KRA then server, is not enough just uninstall server
which
covers KRA uninstall?

+def teardown_method(self, method):
+for host in self.replicas:
+host.run_command(self.kra_uninstall, raiseonerr=False)
+tasks.uninstall_master(host)


3)
Can be this function more generic? It should allow specify host
where
KRA should be installed not just master

+def test_kra_install_master(self):
+self.master.run_command(self.kra_install)


4)

TestLevel0(Dummy):
Can be the test name more specific, something like
TestReplicaPromotionLevel0


5)
please remove this, the patch is on review and it will be pushed
sooner
than tests
+@pytest.mark.xfail  # Ticket N 5455

and as I mentioned in ticket #5455, I cannot reproduce it with
ipa-kra-install, so please provide steps to reproduce if you insist
that
this still does not work as expected with KRA.

6) This is completely wrong, it removes everything that we tried to
achieve with previous patches with domain level in CI


Actually, being able to configure domain level per class is WAY more
convenient, than to always have to think which domain level is
appropriate for which particular test during jenkins job
configuration. In fact, I should have thought about it from the very
beginning. For example, in test_replica_promotion.py we have on
class,
which intiates with domain level = 1, while others - with domain
level
0. With config-based approach, we would have to implement a separate
step that raises domain level. Overall, I am against the approach,
when you have to remember to set certain domain level in config for
any particular test. The tests themselves should be aware of the
domain level they need.

I do not say that we should not have something that overrides
settings
in from config in a particular test case, I say your patch is
doing it
wrong.

I agree it is useful to have param domain_level in install_master,
and
intall_topo methods,  but is cannot be MAX_DOMAIN_LEVEL by default,
because with your current patch the domain_level in config is not
used
at all, it will be always MAX_DOMAIN_LEVEL

For example I want to achieve this goal:
test_vault.py, this test suite can run on domain level1 and on domain
level0, so with one test we can test 2 domain levels just with
putting
domain level into config file.

I agree that with extraordinary test like replica promotion test
is, we
need something that allows override the config file.

As I said bellow, domain_level default value should be None in
install_master and install_topo plugin. If domain level was specified
use the specified one, if not (value is None) use the domain level
from
config file.


Agreed :)



Martin




[PATCH] Enabled setting domain_level per class derived from
TestIntegration

When I configure domain level 0 in yaml config, how is this
supposed to
get into install methods when you removed that code?

-"--domain-level=%i" % host.config.domain_level
+"--domain-level=%i" % domain_level


You always use MAX_DOMAIN_LEVEL in this case or whatever is
specified in
domain_level option.
I suggest to use domain_level=None, and when it is None use
'host.config.domain_level', if it is not None, use 'domain_level'

With this we can specify domain level in config file for test that
can
be used for both domain levels and you can manually specify domain
level
for test that requires specific domain level.

Also this should go away

  @classmethod
  def install(cls, mh):
+if hasattr(cls, "domain_level") and cls.master:
+cls.master.config.domain_level = cls.domain_level
  if cls.topology is None:
  return

I do not see reason why test should override configuration in
config in
this case.

Martin

On 25.11.2015 16:44, Oleg Fayans wrote:

Hi,

Here is the updated version of the patch (more tests + fixed the
issues of the first one) + patch 0017, that implements the
necessary
changes in the background code, i. e. patch 16 does not work
without
patch 17

On 11/18/2015 05:20 PM, Martin Basti wrote:



On 09.11.2015 15:09, Oleg Fayans wrote:

Hi guys,

Here are first two automated te

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

[Tomas Babej]

On 12/02/2015 09:24 AM, Tomas Babej wrote:
> 
> 
> On 12/01/2015 06:27 PM, Tomas Babej wrote:
>>
>>
>> On 11/30/2015 05:32 PM, Lukas Slebodnik wrote:
>>> On (30/11/15 13:09), Tomas Babej wrote:
 Hi,

 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
>>>
>>> >From e6f1846f0d7d17303e5b30b1643651ba739b2b6c Mon Sep 17 00:00:00 2001
 From: Tomas Babej 
 Date: Mon, 30 Nov 2015 12:53:39 +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.

>>
>> Thanks, updated patch attached.
>>
>> Tomas
>>
> 
> Actually, a small improvement is necessary.
> 
> Updated patch attached.
> 
> Tomas
> 
> 
> 

Thanks to Lukas, we found another problem with the test.

Updated patch attached.

Tomas
From ca6ed3cdc696f04ad529a4498705f7a6c8b4058a 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 | 25 +++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_integration/test_sudo.py b/ipatests/test_integration/test_sudo.py
index 1dd4c5d73c9fa4288af4fc2708aa3abd51407217..444063e10c55e1e6f6bee9a63b1276e2766aee2d 100644
--- a/ipatests/test_integration/test_sudo.py
+++ b/ipatests/test_integration/test_sudo.py
@@ -17,6 +17,9 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see .
 
+import pytest
+import re
+
 from ipatests.test_integration.base import IntegrationTest
 from ipatests.test_integration.tasks import clear_sssd_cache
 
@@ -269,13 +272,31 @@ class TestSudo(IntegrationTest):
  '--hostgroups', 'testhostgroup'])
 
 def test_sudo_rule_restricted_to_one_hostmask_setup(self):
-# Add the client's /24 hostmask to the rule
+# Add the client's hostmask to the rule
 ip = self.client.ip
+
+# We need to detect the hostmask first
+result = self.client.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 a note for the next test, which needs to be skipped
+# if hostmask detection failed
+self.__class__.skip_hostmask_based = False
+
+if not match:
+self.__class__.skip_hostmask_based = True
+raise pytest.skip("Hostmask could not be detected")
+
+full_ip = match.group('full_ip')
 self.master.run_command(['ipa', '-n', 'sudorule-add-host',
  'testrule',
- '--hostmask', '%s/24' % ip])
+ '--hostmask', full_ip])
 
 def test_sudo_rule_restricted_to_one_hostmask(self):
+if self.__class__.skip_hostmask_based:
+raise pytest.skip("Hostmask could not be detected")
+
 result1 = self.list_sudo_commands("testuser1")
 assert "(ALL : ALL) NOPASSWD: ALL" in result1.stdout_text
 
-- 
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 0364, 0367] ipa-kra-install: allow first KRA to be installed on replica

[Martin Babinsky]

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



On 30.11.2015 14:16, Martin Babinsky wrote:

On 11/27/2015 05:02 PM, Martin Basti wrote:

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

I tested just master, I will test ipa-4-2 later.
patch attached.




ACK for the master branch.


Thanks, additional patch improves error message when ipa-replica-install
--setup-ca --setup-kra is executed and KRA is not installed anywhere yet.

I'm working on patches for ipa-4-2 branch

Martin


ACK for patch 367.

--
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] 0748 Handle encoding for ipautil.run

[Petr Viktorin]

On 12/02/2015 08:59 AM, Jan Cholasta wrote:
> On 1.12.2015 12:26, Petr Viktorin wrote:
>> On 11/30/2015 08:59 AM, Jan Cholasta wrote:
>>> On 25.11.2015 15:47, Petr Viktorin wrote:
 On 11/25/2015 11:04 AM, Jan Cholasta wrote:
> On 24.11.2015 17:21, Petr Viktorin wrote:
>> On 11/23/2015 10:50 AM, Jan Cholasta wrote:
>>> On 23.11.2015 07:43, Jan Cholasta wrote:
 On 19.11.2015 00:55, Petr Viktorin wrote:
> On 11/03/2015 02:39 PM, Petr Viktorin wrote:
>> Hello,
>> Python 3's strings are Unicode, so data coming to or leaving a
>> Python
>> program needs to be decoded/encoded if it's to be handled as a
>> string.
>> One of the boundaries where encoding is necessary is external
>> programs,
>> specifically, ipautil.run.
>> Unfortunately there's no one set of options that would work
>> for all
>> run() invocations, so I went through them all and specified the
>> stdin/stdout/stderr encoding where necessary. I've also
>> updated the
>> call
>> sites to make it clearer if the return values are used or not.
>> If an encoding is not set, run() will accept/return bytes. (This
>> is a
>> fail-safe setting, since it can't raise errors, and using bytes
>> where
>> strings are expected generally fails loudly in py3.)
>>
>> Note that the changes are not effective under Python 2.
>
> ping,
> Could someone look at this patch?

 Looking.
>>>
>>> 1) I think a better approach would be to use str for
>>> stdin/stdout/stderr
>>> by default in both Python 2 and 3, i.e. do nothing in Python 2 and
>>> encode/decode using locale.getpreferredencoding() in Python 3.
>>> This is
>>> consistent with sys.std{in,out,err} in both Python 2 and 3. Using
>>> bytes
>>> or different encoding should be opt-in.
>>>
>>> Note that different encoding should be used only if the LC_ALL or
>>> LANG
>>> variables are overriden in the command's environment.
>>
>> That would assume the output of *everything* run via ipautil.run
>> can be
>> decoded using the locale encoding. Any stray invalid byte would make
>> IPA
>> crash, even in cases where we don't care about the output. IPA calls
>> too
>> many weird tools to assume they all output text.
>
> Such a stray invalid byte may bubble through our code and cause havoc
> somewhere else, which is much harder to troubleshoot than a crash in
> ipautil.run(), where you can see that it was a misbehaving command
> that
> caused the crash and exactly what command it was.

 The invalid byte can't bubble through code, because if you don't
 specify
 an encoding you get bytes back. You'll get a crash when you try
 using it
 as a string.
>>>
>>> Invalid bytes *can* bubble through our code, into configuration files
>>> and other external places. The fact that it has not caused much grief so
>>> far suggests that it should be safe to use the locale encoding for most
>>> commands.
>>
>> Yes, "suggests" for "most". I'm still wary of using the locale encoding
>> for everything by default.
> 
> OK, let's keep bytes as the default, but I still think the locale
> encoding should be the default when decoding output to text.
> 
>>
>>> If we do subscribe to the statement that any command can output invalid
>>> bytes at any time, I don't see a point in handling encoding in
>>> ipautil.run() at all - just always return bytes and let the caller worry
>>> about encoding and handling related errors.
>>
>> Right, that's actually the basic idea.
>>
>> However, the encoding is two lines of boilerplate code ("if six.PY3" and
>> the actual encode). I've turned this into a convenience keyword argument
>> to minimize the repetition.
> 
> If you always return bytes in both Python 2 and 3, you can always
> .decode() the output, so there won't be any "if six.PY3". Bearing that
> in mind, I don't see much difference between adding ",
> stdout_encoding='ascii'" to the run() call or "stdout =
> stdout.decode('ascii')" on the line below, except the latter is more
> explicit and lets you handle decoding errors yourself.
>
 locale.getpreferredencoding() is not used consistently in all software,
 especially if it's not written in Python. For instance, wget won't
 magically re-encode the data it fetches for us. It's better to
 explicitly specify the encoding every time.
>>>
>>> I would say it is used consistently in most software used by IPA. The
>>> wget example is obviously not relevant to this discussion, since it's
>>> returning external data.
>>>
>>> IMO setting the encoding to 'ascii' without also setting LC_ALL=C in the
>>> environment, as seen frequently in your patch, has even higher
>>> probability of breaking something than using the locale encoding.
>>
>> 

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

[Martin Basti]

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

--
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 0098-0099] domain level 1 topology checks during IPA server uninstall

[Martin Basti]

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

--
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 0098-0099] domain level 1 topology checks during IPA server uninstall

[Martin Babinsky]

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.

--
Martin^3 Babinsky
From cd5eb8e2eaf7b632c3941c339b6ef9efb2940629 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Thu, 19 Nov 2015 17:58:44 +0100
Subject: [PATCH 2/2] implement domain level 1 specific topology checks into
 IPA server uninstaller

When uninstalling domain level 1 master its removal from topology is checked
on remote masters. The uninstaller also checks whether the uninstallation
disconnects the topology and if yes aborts the procedure. The
'--ignore-disconnected-topology' options skips this check.

https://fedorahosted.org/freeipa/ticket/5377
https://fedorahosted.org/freeipa/ticket/5409
---
 install/tools/man/ipa-server-install.1 |   3 +
 ipaserver/install/server/install

Re: [Freeipa-devel] Testing replication topologies

[Aleš Mareček]

Greetings!

- Original Message -
> From: "Martin Basti" 
> To: "freeipa-devel" , "Petr Vobornik" 
> , "Aleš Mareček"
> 
> Sent: Wednesday, December 2, 2015 12:20:56 PM
> Subject: Testing replication topologies
> 
> Hello all,
> 
> due to recent bug I found https://fedorahosted.org/freeipa/ticket/5506 I
> realized we do not have testing of complex topologies.
> 
> On the other hand, test framework is ready and allows to testing 5
> different topologies, can we create test which will test those 5
> topologies with for example 4-5 replicas?

We were talking about it in the morning with Petr. So the answer is: yes, we 
can. Adding Oleg...
> 
> (read test_topologies.py there are examples of topologies)
> 
> Martin
> 

Have a nice day!
 - alich -

-- 
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 25] Improve error logging for Dogtag subsystem installation

[Petr Spacek]

On 2.12.2015 13:23, Jan Cholasta wrote:
> On 2.12.2015 12:54, Petr Spacek wrote:
>> On 2.12.2015 12:51, Christian Heimes wrote:
>>> On 2015-12-02 08:37, Petr Spacek wrote:
 On 1.12.2015 18:42, Christian Heimes wrote:
>  From 33be1f56a64e53d261a1058c4606a7e48c0aac52 Mon Sep 17 00:00:00 2001
> From: Christian Heimes 
> Date: Tue, 1 Dec 2015 15:49:53 +0100
> Subject: [PATCH 25] Improve error logging for Dogtag subsystem 
> installation
>
> In the case of a failed installation or uninstallation of a Dogtag
> subsystem, the error output of pkispawn / pkidestroyed are now shown to
> the user. It makes it more obvious what went wrong and makes it easier
> to debug a problem.
>
> The error handler also attempts to get the full name of the installation
> / uninstallation log file from stdout. pkispawn and pkidestroy print the
> absolute name as 'Log file: /path/to/file.log'. The user no longer has
> to guess the right log file.
>
> Example:
>[1/8]: configuring KRA instance
> Failed to configure KRA instance: Command ''/usr/sbin/pkispawn' '-s'
> 'KRA' '-f' '/tmp/tmp1UpbwF'' returned non-zero exit status 1
> pkispawn: ERROR... PKI subsystem 'KRA' for instance
> 'pki-tomcat' already exists!
> See the installation logs and the following files/directories for more
> information:
>/var/log/pki/pki-tomcat
>/var/log/pki/pki-kra-spawn.20151201151735.log
>[error] RuntimeError: KRA configuration failed.
>
> The patch also changes a couple of modules that were using
> the CalledProcessError exception object from subprocess instead of
> ipautil.

 I'm wondering if ipautil.run() can log stdout and stderr on log level ERROR
 when return code is non-zero (and log on level DEBUG as usual when return
 code
 is zero).

 IMHO it would be nicer, universal, and does not require any changes in 
 places
 calling ipautil.run().
>>>
>>> I think it's a bit confusing to print out stdout and stderr, because
>>> both streams are captured separately. The output is missing its
>>> chronological order. subprocess can capture stdout and stderr in the
>>> same stream, but then we can't distinguish between output and error
>>> output...
>>
>> I do not think it is a problem if these two are clearly marked as such:
>> standard output: %s (if non-empty)
>> stanrard error output: %s (if non-empty)
> 
> We do not want to log with level ERROR by default when rc != 0, because some
> commands generate a *lot* of output.

I do not agree, but whatever. Somebody needs to review the original
Christian's patch.

Petr^2 Spacek

> IMO Christian's approach is OK, because it lets the caller decide how to log
> (or not) stderr on failure.
> 
>>
>>> In case of Dogtag stderr contains the relevant error message. In order
>>> to understand the events, that lead to the particular error, a user has
>>> to read the log file anyway -- unless you run pkispawn with '-vv' for
>>> extra verbosity. But then you get pages over pages of debug output on
>>> *stderr*. It's not helpful either.
>>
>> Sure, I was not talking about that :-)

-- 
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] 0749 Package ipapython, ipalib, ipaplatform, ipatests for Python 3

[Petr Viktorin]

On 12/01/2015 02:37 PM, Jan Cholasta wrote:
[...]
> /etc/ipa/default.conf is managed by freeipa-client and thus should be owned 
> by it.
>
> This is a common pattern in other packages (even other FreeIPA
> sub-packages) and I don't see any reason not to follow it here as well.

OK. After your patch is applied this won't be a problem, though.

[...]
 - python-ipap11helper has compiled code: with this pulled out,
 python3-ipalib can be noarch
>>>
>>> This is not the goal here, but if you insist on doing it, do it for
>>> Python 2 as well.
>>
>> It is definitely the goal of this patch to make the py3 packages as good
>> as possible. That includes making them noarch.
> 
> It is completely unnecessary for the initial py3 support.
> 
> I would rather maintain internal consistency than make the py3 packages
> "perfect".
> 
>> On the other hand improving the py2 packages is not a goal of this
>> particular patch.
> 
> Which is exactly the reason I have provided patches for py2 packages
> myself.

As far as I'm concerned, the patches look good, except for consistency
the package name should be "python2-ipalib".
Unless the process changed, you still need them reviewed by a core
FreeIPA developer.

 - python3-ipatests is needed if we want to start testing the py3
 packages in  CI
>>>
>>> Right.
>>>

 As for new provides, Fedora's Python packaging guidelines say:

 """
 Using a fictional module named "example", the subpackage containing
 the python2 version must provide python2-example. This is of course
 always the case if the subpackage is named python2-example [...]
 If the subpackage has some other name then then Provides:
 python2-example
 must be added explicitly (but see the %python_provide macro below).

 The python3 subpackage must provide python3-example. However, as the
 naming guidelines mandate that the python3 subpackage be named
 python3-example, this will happen automatically.
 """

 so I'm now adding Provides for the top-level modules.
>>>
>>> The goal of this work is to add support for Python 3, not to comply with
>>> Fedora packaging guidelines. FreeIPA on Fedora uses its own spec file
>>> anyway.
>>
>> The goal of this patch is to add new packages that support Python 3.
>> Yes, the Fedora spec is different, but it's heavily based on the
>> upstream one, and this is a good thing. I consider the Fedora guidelines
>> the standard in Python RPM packaging. If IPA uses different packaging
>> guidelines, can you point me to them?
> 
> FreeIPA never fully complied to Fedora packaging guidelines AFAIK and I
> don't see any reason to start now, since nobody seemed to care so far.
> Following them in just py3 sub-packages does not improve the state of
> FreeIPA as a whole and only brings inconsistency into it, so there's no
> benefit in doing it at all.
>
>>> Again, if you insist on doing this, do it for Python 2 as well.

OK, when your patches are ACKed I'll send patches to both improve py2
packaging and add the new packages.

-- 
Petr Viktorin

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

Re: [Freeipa-devel] [PATCH 25] Improve error logging for Dogtag subsystem installation

[Jan Cholasta]

On 2.12.2015 12:54, Petr Spacek wrote:

On 2.12.2015 12:51, Christian Heimes wrote:

On 2015-12-02 08:37, Petr Spacek wrote:

On 1.12.2015 18:42, Christian Heimes wrote:

 From 33be1f56a64e53d261a1058c4606a7e48c0aac52 Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Tue, 1 Dec 2015 15:49:53 +0100
Subject: [PATCH 25] Improve error logging for Dogtag subsystem installation

In the case of a failed installation or uninstallation of a Dogtag
subsystem, the error output of pkispawn / pkidestroyed are now shown to
the user. It makes it more obvious what went wrong and makes it easier
to debug a problem.

The error handler also attempts to get the full name of the installation
/ uninstallation log file from stdout. pkispawn and pkidestroy print the
absolute name as 'Log file: /path/to/file.log'. The user no longer has
to guess the right log file.

Example:
   [1/8]: configuring KRA instance
Failed to configure KRA instance: Command ''/usr/sbin/pkispawn' '-s'
'KRA' '-f' '/tmp/tmp1UpbwF'' returned non-zero exit status 1
pkispawn: ERROR... PKI subsystem 'KRA' for instance
'pki-tomcat' already exists!
See the installation logs and the following files/directories for more
information:
   /var/log/pki/pki-tomcat
   /var/log/pki/pki-kra-spawn.20151201151735.log
   [error] RuntimeError: KRA configuration failed.

The patch also changes a couple of modules that were using
the CalledProcessError exception object from subprocess instead of
ipautil.


I'm wondering if ipautil.run() can log stdout and stderr on log level ERROR
when return code is non-zero (and log on level DEBUG as usual when return code
is zero).

IMHO it would be nicer, universal, and does not require any changes in places
calling ipautil.run().


I think it's a bit confusing to print out stdout and stderr, because
both streams are captured separately. The output is missing its
chronological order. subprocess can capture stdout and stderr in the
same stream, but then we can't distinguish between output and error
output...


I do not think it is a problem if these two are clearly marked as such:
standard output: %s (if non-empty)
stanrard error output: %s (if non-empty)


We do not want to log with level ERROR by default when rc != 0, because 
some commands generate a *lot* of output.


IMO Christian's approach is OK, because it lets the caller decide how to 
log (or not) stderr on failure.





In case of Dogtag stderr contains the relevant error message. In order
to understand the events, that lead to the particular error, a user has
to read the log file anyway -- unless you run pkispawn with '-vv' for
extra verbosity. But then you get pages over pages of debug output on
*stderr*. It's not helpful either.


Sure, I was not talking about that :-)




--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 25] Improve error logging for Dogtag subsystem installation

[Petr Spacek]

On 2.12.2015 12:51, Christian Heimes wrote:
> On 2015-12-02 08:37, Petr Spacek wrote:
>> On 1.12.2015 18:42, Christian Heimes wrote:
>>> From 33be1f56a64e53d261a1058c4606a7e48c0aac52 Mon Sep 17 00:00:00 2001
>>> From: Christian Heimes 
>>> Date: Tue, 1 Dec 2015 15:49:53 +0100
>>> Subject: [PATCH 25] Improve error logging for Dogtag subsystem installation
>>>
>>> In the case of a failed installation or uninstallation of a Dogtag
>>> subsystem, the error output of pkispawn / pkidestroyed are now shown to
>>> the user. It makes it more obvious what went wrong and makes it easier
>>> to debug a problem.
>>>
>>> The error handler also attempts to get the full name of the installation
>>> / uninstallation log file from stdout. pkispawn and pkidestroy print the
>>> absolute name as 'Log file: /path/to/file.log'. The user no longer has
>>> to guess the right log file.
>>>
>>> Example:
>>>   [1/8]: configuring KRA instance
>>> Failed to configure KRA instance: Command ''/usr/sbin/pkispawn' '-s'
>>> 'KRA' '-f' '/tmp/tmp1UpbwF'' returned non-zero exit status 1
>>> pkispawn: ERROR... PKI subsystem 'KRA' for instance
>>> 'pki-tomcat' already exists!
>>> See the installation logs and the following files/directories for more
>>> information:
>>>   /var/log/pki/pki-tomcat
>>>   /var/log/pki/pki-kra-spawn.20151201151735.log
>>>   [error] RuntimeError: KRA configuration failed.
>>>
>>> The patch also changes a couple of modules that were using
>>> the CalledProcessError exception object from subprocess instead of
>>> ipautil.
>>
>> I'm wondering if ipautil.run() can log stdout and stderr on log level ERROR
>> when return code is non-zero (and log on level DEBUG as usual when return 
>> code
>> is zero).
>>
>> IMHO it would be nicer, universal, and does not require any changes in places
>> calling ipautil.run().
> 
> I think it's a bit confusing to print out stdout and stderr, because
> both streams are captured separately. The output is missing its
> chronological order. subprocess can capture stdout and stderr in the
> same stream, but then we can't distinguish between output and error
> output...

I do not think it is a problem if these two are clearly marked as such:
standard output: %s (if non-empty)
stanrard error output: %s (if non-empty)

> In case of Dogtag stderr contains the relevant error message. In order
> to understand the events, that lead to the particular error, a user has
> to read the log file anyway -- unless you run pkispawn with '-vv' for
> extra verbosity. But then you get pages over pages of debug output on
> *stderr*. It's not helpful either.

Sure, I was not talking about that :-)

-- 
Petr^2 Spacek

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

Re: [Freeipa-devel] [PATCH 25] Improve error logging for Dogtag subsystem installation

[Christian Heimes]

On 2015-12-02 08:37, Petr Spacek wrote:
> On 1.12.2015 18:42, Christian Heimes wrote:
>> From 33be1f56a64e53d261a1058c4606a7e48c0aac52 Mon Sep 17 00:00:00 2001
>> From: Christian Heimes 
>> Date: Tue, 1 Dec 2015 15:49:53 +0100
>> Subject: [PATCH 25] Improve error logging for Dogtag subsystem installation
>>
>> In the case of a failed installation or uninstallation of a Dogtag
>> subsystem, the error output of pkispawn / pkidestroyed are now shown to
>> the user. It makes it more obvious what went wrong and makes it easier
>> to debug a problem.
>>
>> The error handler also attempts to get the full name of the installation
>> / uninstallation log file from stdout. pkispawn and pkidestroy print the
>> absolute name as 'Log file: /path/to/file.log'. The user no longer has
>> to guess the right log file.
>>
>> Example:
>>   [1/8]: configuring KRA instance
>> Failed to configure KRA instance: Command ''/usr/sbin/pkispawn' '-s'
>> 'KRA' '-f' '/tmp/tmp1UpbwF'' returned non-zero exit status 1
>> pkispawn: ERROR... PKI subsystem 'KRA' for instance
>> 'pki-tomcat' already exists!
>> See the installation logs and the following files/directories for more
>> information:
>>   /var/log/pki/pki-tomcat
>>   /var/log/pki/pki-kra-spawn.20151201151735.log
>>   [error] RuntimeError: KRA configuration failed.
>>
>> The patch also changes a couple of modules that were using
>> the CalledProcessError exception object from subprocess instead of
>> ipautil.
> 
> I'm wondering if ipautil.run() can log stdout and stderr on log level ERROR
> when return code is non-zero (and log on level DEBUG as usual when return code
> is zero).
> 
> IMHO it would be nicer, universal, and does not require any changes in places
> calling ipautil.run().

I think it's a bit confusing to print out stdout and stderr, because
both streams are captured separately. The output is missing its
chronological order. subprocess can capture stdout and stderr in the
same stream, but then we can't distinguish between output and error
output...

In case of Dogtag stderr contains the relevant error message. In order
to understand the events, that lead to the particular error, a user has
to read the log file anyway -- unless you run pkispawn with '-vv' for
extra verbosity. But then you get pages over pages of debug output on
*stderr*. It's not helpful either.

Christian




signature.asc
Description: OpenPGP digital signature
-- 
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 0389] translations: Update ipa.pot file

[Petr Vobornik]

On 12/02/2015 11:30 AM, Tomas Babej wrote:

Hi,

the attached patch updates the translations in the master branch.

There aren't much string changes between 4.2 and master, most of the
bulk of the patch are changed references to the string locations.

Thanks,
Tomas




ACK

Pushed to master: f72f8c1ad04847e4d0f24b50c76a583bd6fe5a86
--
Petr Vobornik

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

Re: [Freeipa-devel] [PATCH] 494 Update Contributors.txt

[Martin Kosek]

On 12/02/2015 12:35 PM, Martin Basti wrote:
> 
> 
> On 02.12.2015 12:24, Martin Kosek wrote:
>> +Thierry Bordaz
>> 
>> +Thierry Bordaz
>> 
> 
> Do we want this there?

I do not want it there, but I had to add it since Author field in git was wrong
when the patch was pushed.

So I blame the original author and the reviewer for making me add this mail
alias :-)

-- 
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] 494 Update Contributors.txt

[Martin Basti]

On 02.12.2015 12:24, Martin Kosek wrote:

+Thierry Bordaz
+Thierry Bordaz


Do we want this there?

--
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] 494 Update Contributors.txt

[Petr Vobornik]

On 12/02/2015 12:24 PM, Martin Kosek wrote:

Update .mailmap with misconfigured patch authors since the last
feature release. Based on the git history, add new Development
contributors.





ACK

pushed to master 4a75a5f7ffd4f5060e84d04e5806b84e5605ddec
--
Petr Vobornik

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

[Freeipa-devel] [PATCH] 494 Update Contributors.txt

[Martin Kosek]

Update .mailmap with misconfigured patch authors since the last
feature release. Based on the git history, add new Development
contributors.

-- 
Martin Kosek 
Associate Manager, Software Engineering - Identity Management Team
Red Hat, Inc.
From 78ab138e09cebfd4a8df12ae89d43867a40e13c4 Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Wed, 2 Dec 2015 12:22:47 +0100
Subject: [PATCH] Update Contributors.txt

Update .mailmap with misconfigured patch authors since the last
feature release. Based on the git history, add new Developer
contributors.
---
 .mailmap |  3 +++
 Contributors.txt | 18 ++
 2 files changed, 21 insertions(+)

diff --git a/.mailmap b/.mailmap
index 176cf78ef4094ae7f53766fe121208228b3b7086..422a0089cc6f7de4aaed6a446a5d090bcee3fee8 100644
--- a/.mailmap
+++ b/.mailmap
@@ -23,6 +23,7 @@ Lubomír Rintel   Lubomir Rintel 
 Martin Bašti
 Martin Košek
+Milan Kubík 
 Martin Nagy  
 Nathaniel McCallum  
 Nalin Dahyabhai  
@@ -48,6 +49,8 @@ Simo Sorce  
 Sumit Bose   
 Sumit Bose   
 Thierry Bordaz  
+Thierry Bordaz 
+Thierry Bordaz 
 Tomáš Babej 
 Tomáš Babej 
 William Jon McCann 
diff --git a/Contributors.txt b/Contributors.txt
index 53ab05eae4cc292956582ef86ab100bee607497b..8858724fab4febf01e19d8660786c10ddcc7a3b6 100644
--- a/Contributors.txt
+++ b/Contributors.txt
@@ -8,12 +8,14 @@ Developers:
 	Gabe Alford
 	Jr Aquino
 	Tomáš Babej
+	Martin Babinsky
 	Kyle Baker
 	Martin Bašti
 	Sylvain Baubeau
 	Alexander Bokovoy
 	Thierry Bordaz
 	Sumit Bose
+	François Cami
 	Xiao-Long Chen
 	Jan Cholasta
 	Yuri Chornoivan
@@ -26,20 +28,32 @@ Developers:
 	Jason Gerard DeRose
 	Günther Deschner
 	Endi Sukma Dewata
+	Lenka Doudova
+	Benjamin Drung
+	Drew Erny
+	Oleg Fayans
 	Stephen Gallagher
 	Ondřej Hamada
 	Nick Hatch
+	Christian Heimes
 	Jakub Hrozek
+	Abhijeet Kasurde
 	Nathan Kinder
 	Krzysztof Klimonda
 	Nikolai Kondrashov
 	Martin Košek
 	Ludwig Krispenz
 	Ana Krivokapić
+	Milan Kubík
 	Ian Kumlien
 	David Kupka
+	Robert Kuska
+	Stanislav Laznicka
 	Ade Lee
 	Karl MacMillan
+	Niranjan Mallapadi
+	Ales 'alich' Marecek
+	Francesco Marella
 	Nathaniel McCallum
 	William Jon McCann
 	Kevin McCarthy
@@ -47,6 +61,7 @@ Developers:
 	Rich Megginson
 	Jim Meyering
 	Adam Misnyovszki
+	Niranjan MR
 	Marko Myllynen
 	Martin Nagy
 	David O'Brien
@@ -57,13 +72,16 @@ Developers:
 	Lubomír Rintel
 	Lynn Root
 	Pete Rowley
+	Lenka Ryznarova
 	Thorsten Scherf
+	Michael Simacek
 	Lars Sjostrom
 	Lukáš Slebodník
 	Simo Sorce
 	Petr Špaček
 	David Spångberg
 	Diane Trout
+	Fraser Tweedale
 	Petr Viktorin
 	Petr Voborník
 	Andrew Wnuk
-- 
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] Testing replication topologies

[Martin Basti]

Hello all,

due to recent bug I found https://fedorahosted.org/freeipa/ticket/5506 I 
realized we do not have testing of complex topologies.


On the other hand, test framework is ready and allows to testing 5 
different topologies, can we create test which will test those 5 
topologies with for example 4-5 replicas?


(read test_topologies.py there are examples of topologies)

Martin

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

Re: [Freeipa-devel] [IPAQE][REVIEW-REQUEST][TEST PLAN] Installation tests

[Martin Basti]

On 02.12.2015 09:52, Oleg Fayans wrote:

Hi all,

I've updated the test plan according to your feedback. The tests for 
'ca' suffix would be added to Topology Plugin testplan during this week.




On 11/27/2015 04:37 PM, Martin Basti wrote:



On 27.11.2015 15:05, Martin Basti wrote:



On 26.11.2015 14:39, Petr Vobornik wrote:

On 11/23/2015 06:51 PM, Oleg Fayans wrote:

Hi all,

Here is a draft of the Replica Promotion test plan
http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan




== Test case: Unprivileged users are not allowed to enroll and
promote clients ==
User credentials are passed there through -p $principal and -w
$password options. It is correct atm because it is required for
connection check. But end goal of replica promotion is the avoid it.
See https://fedorahosted.org/freeipa/ticket/5497 and
https://fedorahosted.org/freeipa/ticket/5498 for more information.

== Missing test cases ==
1. ipa-replica-install works on CA-less master with with both domain
levels
2. ipa-server-install works with --setup-dns option with both domain
levels
3. ipa-server-install works with externally signed CA cert with both
domain levels
4. ipa-replica-install with options(and their combination):
--setup-ca --setup-dns --setup-kra works with both domain levels

Note: Not sure if #2 and #3 belongs here, but should be tested. Maybe
tests for domain level 0 already exist.


Many of them are already part of some tests.

I wanted to crate install tests in one place with all combination of
options, but I did not have time for it yet.
I have a proof of concept test plan I can share it if you want.

Martin


Proof of concept test plan:

Master:
* ipa-server-install
* ipa-server-install --setup-dns
* caless ipa-server-install
* ipa-server-install && ipa-dns-install
* ipa-server-install && ipa-dns-install --dnssec-master
* caless ipa-server-install && ipa-dns-install
* caless ipa-server-install && ipa-ca-install
* caless ipa-server-install && ipa-ca-install && ipa-kra-install
* ipa-server-install && ipa-kra-install

Replica:
ipa-replica-install
ipa-replica-install --setup-dns
ipa-replica-install --setup-ca
ipa-replica-install --setup-dns --setup-ca
ipa-replica-install --setup-ca --setup-kra
ipa-replica-install --setup-dns --setup-ca --setup-kra
ipa-replica-install && ipa-dns-install
ipa-replica-install && ipa-dns-install --dnssec-master
ipa-replica-install && ipa-ca-install
ipa-replica-install && ipa-ca-install && ipa-kra-install
ipa-replica-install && ipa-dns-install && ipa-ca-install && 
ipa-kra-install

caless ipa-replica-install && ipa-ca-install
caless ipa-replica-install --setup-dns
caless ipa-replica-install && ipa-dns-install

These are basic tests, other specialized test like "install KRA first on
replica then on master" should be in separate test suite IMO

I'm not sure if caless install test should be covered in basic install
tests.
Same for DNSSEC master (this is fuly covered in dnssec tests)




Hello,
as I wrote above, the install tests should be in *separated* test plan, 
and should cover only ability to be installed, not any other functional 
tests, we have functional tests separated (vault, dnssec, ...)


Also is need to cover multiple variation of options, not just install it 
with all options (--setup-dns, --setup-ca, --setup-kra)


Your:
Test case: ipa-server-install works with all supported options under 
domain level 0
Test case: ipa-server-install works with all supported options under 
domain level 1


Should not be part of replica promotion testing.

Also I do not think that you should test DNSSEC in these tests, DNSSEC 
test are almost fully covered in DNSSEC integration test, it is just 
enough to run it on both domain levels.


If you disagree to have separate "Installation test suite" please write 
the reason why it should be a part of replica pro motion test plan.


Test case: ipa-ca install and ipa-kra-install work on master and replica 
under domain level 0
In this test, to be able install CA on master server, master has to be 
installed as CA-less. Shouldn't be there: expecting failure as result?


In case we will create basic install test we may exclude testing 
ipa-ca-install and ipa-kra-install from this testsuite.

Also I do not see reason why DNSSEC should be tested there.

Shouldn't be ipa-kra-install and uninstall a separate test case?

Martin


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

Re: [Freeipa-devel] [PATCH 0069] ipa-replica-install support caless install with promotion.

[David Kupka]

On 02/12/15 07:58, Jan Cholasta wrote:

On 1.12.2015 14:27, David Kupka wrote:

On 30/11/15 17:24, Jan Cholasta wrote:

Hi,

On 27.11.2015 07:57, David Kupka wrote:

On 26/11/15 15:22, David Kupka wrote:

On 26/11/15 15:13, David Kupka wrote:

On 26/11/15 15:01, David Kupka wrote:

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



Replaced accidentally inserted tabs.




Fixed indentation I screwed up when replacing tabs :-/


1) The deprecated --*_pkcs12 and --*_pin aliases should not be supported
in ipa-replica-install.


In ServerCA, inherit the knobs from BaseServerCA rather than
BaseServer.ca. The "#pylint: disable=no-member" will no longer be
necessary.

In ipa-server-install help, there are 2 "certificate system" option
groups. This is a shortcoming in the installer framework, which will be
addressed in the future. For now, please inherit *all* knobs of
BaseServerCA in ServerCA as a workaround.




2) This check from ipa-replica-prepare should be added to
Replica.__init__() as well:

 # If any of the PKCS#12 options are selected, all are required.
 cert_file_req = (options.dirsrv_cert_files,
options.http_cert_files)
 cert_file_opt = (options.pkinit_cert_files,)
 if any(cert_file_req + cert_file_opt) and not
all(cert_file_req):
 self.option_parser.error(
 "--dirsrv-cert-file and --http-cert-file are required
if any "
 "PKCS#12 options are used.")


The check is done when replica file is specified in the patch, but it
should be done only when replica file is *not* specified.


6) Please make the ca_is_enabled argument of install_replica_ds() and
install_http() mandatory and fill as appropriate when called, it will
make the code more readable.


This bit in install_http() is redundant now:

+if ca_is_configured is None:
+ca_is_configured = ipautil.file_exists(config.dir + "/cacert.p12")




7)

$ git diff -U0 | pep8 --diff
./ipaserver/install/server/replicainstall.py:99:80: E501 line too long
(82 > 79 characters)
./ipaserver/install/server/replicainstall.py:161:80: E501 line too long
(82 > 79 characters)
./ipaserver/install/server/replicainstall.py:1289:13: E265 block comment
should start with '# '
./ipaserver/install/server/replicainstall.py:1291:17: E125 continuation
line with same indent as next logical line
./ipaserver/install/server/replicainstall.py:1291:17: E128 continuation
line under-indented for visual indent


$ git diff -U0 | pep8 --diff
./ipaserver/install/server/install.py:1142:1: E302 expected 2 blank
lines, found 1
./ipaserver/install/server/install.py:1143:5: E265 block comment should
start with '# '
./ipaserver/install/server/install.py:1160:17: E222 multiple spaces
after operator
./ipaserver/install/server/install.py:1288:9: E265 block comment should
start with '# '
./ipaserver/install/server/replicainstall.py:100:80: E501 line too long
(82 > 79 characters)
./ipaserver/install/server/replicainstall.py:162:80: E501 line too long
(82 > 79 characters)
./ipaserver/install/server/replicainstall.py:697:41: E251 unexpected
spaces around keyword / parameter equals
./ipaserver/install/server/replicainstall.py:697:43: E251 unexpected
spaces around keyword / parameter equals
./ipaserver/install/server/replicainstall.py:922:9: E129 visually
indented line with same indent as next logical line
./ipaserver/install/server/replicainstall.py:925:14: E131 continuation
line unaligned for hanging indent
./ipaserver/install/server/replicainstall.py:1345:9: E265 block comment
should start with '# '
./ipaserver/install/server/replicainstall.py:1389:21: E128 continuation
line under-indented for visual indent



Thanks, updated patch attached.

--
David Kupka
From c1e2259bb352e160e41deb8853bd615f1c9f3db1 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Thu, 26 Nov 2015 09:01:27 +0100
Subject: [PATCH] ipa-replica-install support caless install with promotion.

https://fedorahosted.org/freeipa/ticket/5441
---
 ipaserver/install/custodiainstance.py  |   6 +-
 ipaserver/install/dsinstance.py|   3 +-
 ipaserver/install/server/common.py |   6 --
 ipaserver/install/server/install.py|  58 +-
 ipaserver/install/server/replicainstall.py | 168 -
 5 files changed, 199 insertions(+), 42 deletions(-)

diff --git a/ipaserver/install/custodiainstance.py b/ipaserver/install/custodiainstance.py
index df99962a7e6e8ecac044ff4e8341a4a9913e4d4d..dbe36af6d7af23fa859dcb78f3dc24224fd8fd07 100644
--- a/ipaserver/install/custodiainstance.py
+++ b/ipaserver/install/custodiainstance.py
@@ -17,7 +17,7 @@ import tempfile
 
 
 class CustodiaInstance(SimpleServiceInstance):
-def __init__(self, host_name=None, realm=None):
+def __init__(self, host_name=None, realm=None, ca_is_configured=True):
 super(CustodiaInstance, self).__init__("ipa-custodia")
 self.config_file = paths.IPA_CUSTODIA_CONF
 self.server_keys = os.path.join(paths.IPA_CUSTODIA_CONF_DIR,
@@ -2

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

[Martin Babinsky]

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.

--
Martin^3 Babinsky
From a6dfb5eb910c6bcc4c456a704ba499ba4037e7ca Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Thu, 19 Nov 2015 17:55:23 +0100
Subject: [PATCH 1/2] extract domain level 1 topology-checking code from
 ipa-replica-manage

This facilitates reusability of this code in other components, e.g. IPA server
uninstallers.

https://fedorahosted.org/freeipa/ticket/5409
---
 install/tools/ipa-replica-manage | 108 ---
 ipaserver/install/replication.py |  90 
 2 files changed, 101 insertions(+), 97 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 7bbef3593ef291b4af209d7f59c21e23d3d22944..6d303e6f008325648461287448ef3d2712e23911 100755
--- a/install/tools/ipa-replica-manag

Re: [Freeipa-devel] [PATCH 0385] replicainstall: Add possiblity to install client in one

[Tomas Babej]

On 12/02/2015 09:53 AM, Martin Babinsky wrote:
> On 12/01/2015 04:33 PM, Jan Cholasta wrote:
>> On 1.12.2015 16:19, Tomas Babej wrote:
>>>
>>>
>>> On 12/01/2015 08:19 AM, Jan Cholasta wrote:
 On 30.11.2015 19:17, Simo Sorce wrote:
> On Mon, 2015-11-30 at 12:25 +0100, Tomas Babej wrote:
>> +# Perform only if we have the necessary options
>> +if not any([installer.admin_password, installer.keytab]):
>> +sys.exit("IPA client is not configured on this system.\n"
>> + "You must use a replica file or join the system "
>> + "either by using by running 'ipa-client-install'. "
>> + "Alternatively, you may specify enrollment related
>> options "
>> + "directly, see man ipa-replica-install.")
>> +
>
> There is a typo "either by using by "
>
> Also this seem to be run in promote_check, so you should not mention
> replica files, as promotion can only be run at domain level 1 where
> replica files cannot be used.

 One more thing from me: admin password should be passed to
 ipa-client-install through stdin. Apply the following changes (tested
 and working) to make it so:

   args.extend(["--hostname", installer.host_name])

   if installer.admin_password:
 -args.extend(["--password", installer.admin_password])
   args.extend(["--principal", installer.principal or
 "admin"])
   if installer.keytab:
   args.extend(["--keytab", installer.keytab])
 @@ -792,7 +791,13 @@ def ensure_enrolled(installer):
   args.append("--no-sshd")
   if installer.mkhomedir:
   args.append("--mkhomedir")
 -ipautil.run(args)
 +
 +if installer.admin_password:
 +stdin = installer.admin_password
 +else:
 +stdin = None
 +
 +ipautil.run(args, stdin=stdin)
   except Exception as e:
   sys.exit("Configuration of client side components failed!\n"
"ipa-client-install returned: " + str(e))

>>>
>>> Both Simo's and Jan's suggestions make sense, thanks.
>>>
>>> Updated patch attached.
>>
>> Thank you, ACK.
>>
>> Pushed to master: 034e76062fd897dc67b5a395735a5471257bfc8b
>>
> 
> It would be nice if we could update ipa-replica-install manpage and
> document all three different ways to set up a replica
> (ipa-replica-install w/ replica file in domain level 0,
> ipa-client-install & ipa-replica-install via promotion in domain level
> 1, single ipa-replica-install w/ client-specific options in domain level
> 1).
> 
> Especially in the latter case it was not obvious to me which options
> should I use to set up client and replica in one go without peeking in
> the source code.
> 

Yes, a man page update is long due and in the forge already.

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] [PATCH 0385] replicainstall: Add possiblity to install client in one

[Martin Babinsky]

On 12/01/2015 04:33 PM, Jan Cholasta wrote:

On 1.12.2015 16:19, Tomas Babej wrote:



On 12/01/2015 08:19 AM, Jan Cholasta wrote:

On 30.11.2015 19:17, Simo Sorce wrote:

On Mon, 2015-11-30 at 12:25 +0100, Tomas Babej wrote:

+# Perform only if we have the necessary options
+if not any([installer.admin_password, installer.keytab]):
+sys.exit("IPA client is not configured on this system.\n"
+ "You must use a replica file or join the system "
+ "either by using by running 'ipa-client-install'. "
+ "Alternatively, you may specify enrollment related
options "
+ "directly, see man ipa-replica-install.")
+


There is a typo "either by using by "

Also this seem to be run in promote_check, so you should not mention
replica files, as promotion can only be run at domain level 1 where
replica files cannot be used.


One more thing from me: admin password should be passed to
ipa-client-install through stdin. Apply the following changes (tested
and working) to make it so:

  args.extend(["--hostname", installer.host_name])

  if installer.admin_password:
-args.extend(["--password", installer.admin_password])
  args.extend(["--principal", installer.principal or
"admin"])
  if installer.keytab:
  args.extend(["--keytab", installer.keytab])
@@ -792,7 +791,13 @@ def ensure_enrolled(installer):
  args.append("--no-sshd")
  if installer.mkhomedir:
  args.append("--mkhomedir")
-ipautil.run(args)
+
+if installer.admin_password:
+stdin = installer.admin_password
+else:
+stdin = None
+
+ipautil.run(args, stdin=stdin)
  except Exception as e:
  sys.exit("Configuration of client side components failed!\n"
   "ipa-client-install returned: " + str(e))



Both Simo's and Jan's suggestions make sense, thanks.

Updated patch attached.


Thank you, ACK.

Pushed to master: 034e76062fd897dc67b5a395735a5471257bfc8b



It would be nice if we could update ipa-replica-install manpage and 
document all three different ways to set up a replica 
(ipa-replica-install w/ replica file in domain level 0, 
ipa-client-install & ipa-replica-install via promotion in domain level 
1, single ipa-replica-install w/ client-specific options in domain level 1).


Especially in the latter case it was not obvious to me which options 
should I use to set up client and replica in one go without peeking in 
the source code.


--
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] [IPAQE][REVIEW-REQUEST][TEST PLAN] Installation tests

[Oleg Fayans]

Hi all,

I've updated the test plan according to your feedback. The tests for 
'ca' suffix would be added to Topology Plugin testplan during this week.




On 11/27/2015 04:37 PM, Martin Basti wrote:



On 27.11.2015 15:05, Martin Basti wrote:



On 26.11.2015 14:39, Petr Vobornik wrote:

On 11/23/2015 06:51 PM, Oleg Fayans wrote:

Hi all,

Here is a draft of the Replica Promotion test plan
http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan




== Test case: Unprivileged users are not allowed to enroll and
promote clients ==
User credentials are passed there through -p $principal and -w
$password options. It is correct atm because it is required for
connection check. But end goal of replica promotion is the avoid it.
See https://fedorahosted.org/freeipa/ticket/5497 and
https://fedorahosted.org/freeipa/ticket/5498 for more information.

== Missing test cases ==
1. ipa-replica-install works on CA-less master with with both domain
levels
2. ipa-server-install works with --setup-dns option with both domain
levels
3. ipa-server-install works with externally signed CA cert with both
domain levels
4. ipa-replica-install with options(and their combination):
--setup-ca --setup-dns --setup-kra works with both domain levels

Note: Not sure if #2 and #3 belongs here, but should be tested. Maybe
tests for domain level 0 already exist.


Many of them are already part of some tests.

I wanted to crate install tests in one place with all combination of
options, but I did not have time for it yet.
I have a proof of concept test plan I can share it if you want.

Martin


Proof of concept test plan:

Master:
* ipa-server-install
* ipa-server-install --setup-dns
* caless ipa-server-install
* ipa-server-install && ipa-dns-install
* ipa-server-install && ipa-dns-install --dnssec-master
* caless ipa-server-install && ipa-dns-install
* caless ipa-server-install && ipa-ca-install
* caless ipa-server-install && ipa-ca-install && ipa-kra-install
* ipa-server-install && ipa-kra-install

Replica:
ipa-replica-install
ipa-replica-install --setup-dns
ipa-replica-install --setup-ca
ipa-replica-install --setup-dns --setup-ca
ipa-replica-install --setup-ca --setup-kra
ipa-replica-install --setup-dns --setup-ca --setup-kra
ipa-replica-install && ipa-dns-install
ipa-replica-install && ipa-dns-install --dnssec-master
ipa-replica-install && ipa-ca-install
ipa-replica-install && ipa-ca-install && ipa-kra-install
ipa-replica-install && ipa-dns-install && ipa-ca-install && ipa-kra-install
caless ipa-replica-install && ipa-ca-install
caless ipa-replica-install --setup-dns
caless ipa-replica-install && ipa-dns-install

These are basic tests, other specialized test like "install KRA first on
replica then on master" should be in separate test suite IMO

I'm not sure if caless install test should be covered in basic install
tests.
Same for DNSSEC master (this is fuly covered in dnssec tests)



--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

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

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

[Tomas Babej]

On 12/01/2015 06:27 PM, Tomas Babej wrote:
> 
> 
> On 11/30/2015 05:32 PM, Lukas Slebodnik wrote:
>> On (30/11/15 13:09), Tomas Babej wrote:
>>> Hi,
>>>
>>> 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
>>
>> >From e6f1846f0d7d17303e5b30b1643651ba739b2b6c Mon Sep 17 00:00:00 2001
>>> From: Tomas Babej 
>>> Date: Mon, 30 Nov 2015 12:53:39 +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.
>>>
> 
> Thanks, updated patch attached.
> 
> Tomas
> 

Actually, a small improvement is necessary.

Updated patch attached.

Tomas
From 4c1fb168889a074338d90bcbbb155aa7be88eb89 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Mon, 30 Nov 2015 12:53:39 +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 | 25 +++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_integration/test_sudo.py b/ipatests/test_integration/test_sudo.py
index 1dd4c5d73c9fa4288af4fc2708aa3abd51407217..07aae337297587dd3d7e5097c4ef62b656846dac 100644
--- a/ipatests/test_integration/test_sudo.py
+++ b/ipatests/test_integration/test_sudo.py
@@ -17,6 +17,9 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see .
 
+import pytest
+import re
+
 from ipatests.test_integration.base import IntegrationTest
 from ipatests.test_integration.tasks import clear_sssd_cache
 
@@ -269,13 +272,31 @@ class TestSudo(IntegrationTest):
  '--hostgroups', 'testhostgroup'])
 
 def test_sudo_rule_restricted_to_one_hostmask_setup(self):
-# Add the client's /24 hostmask to the rule
+# Add the client's hostmask to the rule
 ip = self.client.ip
+
+# We need to detect the hostmask first
+result = self.client.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 a note for the next test, which needs to be skipped
+# if hostmask detection failed
+self.skip_hostmask_based = False
+
+if not match:
+self.skip_hostmask_based = True
+raise pytest.skip("Hostmask could not be detected")
+
+full_ip = match.group('full_ip')
 self.master.run_command(['ipa', '-n', 'sudorule-add-host',
  'testrule',
- '--hostmask', '%s/24' % ip])
+ '--hostmask', full_ip])
 
 def test_sudo_rule_restricted_to_one_hostmask(self):
+if self.skip_hostmask_based:
+raise pytest.skip("Hostmask could not be detected")
+
 result1 = self.list_sudo_commands("testuser1")
 assert "(ALL : ALL) NOPASSWD: ALL" in result1.stdout_text
 
-- 
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