[Freeipa-devel] [PATCH 0113] properly add ACIs to custodia container during IPA upgrade

[Martin Babinsky]

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

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

During upgrade the ACIs required for IPA masters to store secrets in custodia
were added before the custodia container was properly created. This led to a
creation of entry with no objectclasses. This patch ensures that the
respective ACIs are added only after the container with required objectclasses
was created.

https://fedorahosted.org/freeipa/ticket/5524
---
 install/updates/20-aci.update  | 5 -
 install/updates/73-custodia.update | 5 +
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/install/updates/20-aci.update b/install/updates/20-aci.update
index ca4c0df0576b07aa48e6bdd2e70e06f9819b6da9..118563bad7465df7657a2947ce5e53dee04d634c 100644
--- a/install/updates/20-aci.update
+++ b/install/updates/20-aci.update
@@ -111,8 +111,3 @@ add:aci: (target = "ldap:///cn=replication managers,cn=sysaccounts,cn=etc,$SUFFI
 # IPA server hosts can change replica ID
 dn: cn=etc,$SUFFIX
 add:aci: (target = "ldap:///cn=replication,cn=etc,$SUFFIX;)(targetattr = "nsDS5ReplicaId")(version 3.0; acl "IPA server hosts can change replica ID"; allow(write) groupdn = "ldap:///cn=ipaservers,cn=hostgroups,cn=accounts,$SUFFIX;;)
-
-# IPA server hosts can create and manage own Custodia secrets
-dn: cn=custodia,cn=ipa,cn=etc,$SUFFIX
-add:aci: (target = "ldap:///cn=*/($$dn),cn=custodia,cn=ipa,cn=etc,$SUFFIX")(version 3.0; acl "IPA server hosts can create own Custodia secrets"; allow(add) groupdn = "ldap:///cn=ipaservers,cn=hostgroups,cn=accounts,$SUFFIX; and userdn = "ldap:///fqdn=($$dn),cn=computers,cn=accounts,$SUFFIX";)
-add:aci: (target = "ldap:///cn=*/($$dn),cn=custodia,cn=ipa,cn=etc,$SUFFIX")(targetattr = "ipaPublicKey")(version 3.0; acl "IPA server hosts can manage own Custodia secrets"; allow(write) groupdn = "ldap:///cn=ipaservers,cn=hostgroups,cn=accounts,$SUFFIX; and userdn = "ldap:///fqdn=($$dn),cn=computers,cn=accounts,$SUFFIX";)
diff --git a/install/updates/73-custodia.update b/install/updates/73-custodia.update
index f6520fb2e36dd1b234344a8cc4199ab72c664163..eb3d4d83957bd9fe83b0c5f370b4ed76738a1039 100644
--- a/install/updates/73-custodia.update
+++ b/install/updates/73-custodia.update
@@ -2,3 +2,8 @@ dn: cn=custodia,cn=ipa,cn=etc,$SUFFIX
 default: objectClass: top
 default: objectClass: nsContainer
 default: cn: custodia
+
+# IPA server hosts can create and manage own Custodia secrets
+dn: cn=custodia,cn=ipa,cn=etc,$SUFFIX
+add:aci: (target = "ldap:///cn=*/($$dn),cn=custodia,cn=ipa,cn=etc,$SUFFIX")(version 3.0; acl "IPA server hosts can create own Custodia secrets"; allow(add) groupdn = "ldap:///cn=ipaservers,cn=hostgroups,cn=accounts,$SUFFIX; and userdn = "ldap:///fqdn=($$dn),cn=computers,cn=accounts,$SUFFIX";)
+add:aci: (target = "ldap:///cn=*/($$dn),cn=custodia,cn=ipa,cn=etc,$SUFFIX")(targetattr = "ipaPublicKey")(version 3.0; acl "IPA server hosts can manage own Custodia secrets"; allow(write) groupdn = "ldap:///cn=ipaservers,cn=hostgroups,cn=accounts,$SUFFIX; and userdn = "ldap:///fqdn=($$dn),cn=computers,cn=accounts,$SUFFIX";)
-- 
2.5.0

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

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

[Martin Basti]

On 08.12.2015 08:52, Jan Cholasta wrote:

On 7.12.2015 21:11, Martin Basti wrote:



On 07.12.2015 08:21, Jan Cholasta wrote:

On 2.12.2015 16:23, Jan Cholasta wrote:

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





Pushed.




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


Martin Basti pointed out that admin password should not be asked
interactively during OTP replica promotion. Fixed.

Updated and rebased patch attached.





1)
[root@vm-058-138 ~]# ipa-replica-install --server
vm-058-137.abc.idm.lab.eng.brq.redhat.com --domain
abc.idm.lab.eng.brq.redhat.com --password=bubak  --setup-ca
Configuring client side components
Password for ad...@abc.idm.lab.eng.brq.redhat.com:

IMO password should be asked first, before any installation begins (IMO
this is for conncheck)


The same thing happens without my patch. Could you file a ticket?

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





2)
When host is not in ipaservers hostgroup. Also I would expect different
error message
ipa-replica-install --server vm-058-137.abc.idm.lab.eng.brq.redhat.com
--domain abc.idm.lab.eng.brq.redhat.com --password=bubak --setup-ca
--skip-conncheck


 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 63, in _install
 for nothing in self._installer(self.parent):
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/replicainstall.py", 


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


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


line 1002, in promote_check
 conn.connect(ccache=installer._ccache)
   File "/usr/lib/python2.7/site-packages/ipalib/backend.py", line 66,
in connect
 conn = self.create_connection(*args, **kw)
   File "/usr/lib/python2.7/site-packages/ipaserver/plugins/ldap2.py",
line 199, in create_connection
 principal = krb_utils.get_principal(ccache_name=ccache)
   File "/usr/lib/python2.7/site-packages/ipalib/krb_utils.py", line
184, in get_principal
 raise errors.CCacheError(message=unicode(e))

2015-12-07T16:23:40Z DEBUG The ipa-replica-install command failed,
exception: CCacheError: Major (851968): Unspecified GSS failure. Minor
code may provide more information, Minor (2529639053): No Kerberos
credentials available
2015-12-07T16:23:40Z ERROR Major (851968): Unspecified GSS failure.
Minor code may provide more information, Minor (2529639053): No Kerberos
credentials available


Fixed.




3)
This case is not handle very well:
a) install client with OTP password
b) install replica with the same OTP password (when host is no in
ipaservers group, if host is in ipaservers group it works)

ipa.ipapython.install.cli.install_tool(Replica): ERRORMajor
(851968): Unspecified GSS failure.  Minor code may provide more
information, Minor (2529639053): No Kerberos credentials available
ipa.ipapython.install.cli.install_tool(Replica): ERRORThe
ipa-replica-install command failed. See /var/log/ipareplica-install.log
for more information


This is the same as 2).



4)
This is not user friendly
I used wrong OTP password, can we somehow propagate the actual error
from client install to stderr?

ipa.ipapython.install.cli.install_tool(Replica): ERROR Configuration of
client side components failed!
ipa-client-install returned: Command ''/usr/sbin/ipa-client-install'
'--unattended' '--domain' 'abc.idm.lab.eng.brq.redhat.com' '--server'
'vm-058-137.abc.idm.lab.eng.brq.redhat.com' '--password' 'buba''
returned non-zero exit status 1
ipa.ipapython.install.cli.install_tool(Replica): ERRORThe
ipa-replica-install command failed. See /var/log/ipareplica-install.log
for more information


The same thing happens without my patch for any other error. Could you 
file a ticket?


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



Updated patch attached.


Working on review

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

Re: [Freeipa-devel] [PATCH 0372] CI: installation tests

[Oleg Fayans]

ACK

On 12/07/2015 10:44 PM, Martin Basti wrote:
> 
> 
> On 07.12.2015 15:51, Oleg Fayans wrote:
>>
>> On 12/07/2015 03:51 PM, Martin Basti wrote:
>>>
>>> On 07.12.2015 15:49, Oleg Fayans wrote:
 Hi,

 On 12/07/2015 02:37 PM, Martin Basti wrote:
> On 07.12.2015 14:32, Martin Basti wrote:
>> On 07.12.2015 13:24, Oleg Fayans wrote:
>>> Hi Martin,
>>>
>>> I would prefer both install_kra and install_ca methods to have
>>> raiseonerr parameter set to True by default. We need a way to test
>>> negatives and analyze results.
>>> Mine looks like this:
>>>
>>> def install_kra(host, domain_level=None,
>>>first_instance=False, raiseonerr=True):
>>>if not domain_level:
>>>   domain_level = host.config.domain_level
>>>command = ["ipa-kra-install", "-U", "-p",
>>> host.config.dirman_password]
>>>if domain_level == DOMAIN_LEVEL_0 and not first_instance:
>>>replica_file = get_replica_filename(host)
>>>command.append(replica_file)
>>>return host.run_command(command, raiseonerr=raiseonerr)
>>>
>>> The rest looks good to me, but I did not run the tests yet.
>> Sounds good, I will amend the patche later.
> I changed my mind, should not be the domain_level value get from
> function domainlevel(host)?
 We should have a way to test negatives, like providing replica file at
 domain level=1 and not providing at domain level=0. So these functions
 should either accept replica file as a parameter, or arbitrary domain
 level
>>> Agree, but I meant this
>>>
>>>   if not domain_level:
>>> -domain_level = host.config.domain_level
>>> +domain_level = domainlevel(host)
>> Oh, yes, you are right, that's better.
>>
>>>
> Martin^2
>
>>> On 12/06/2015 10:22 PM, Martin Basti wrote:
 My favorite today \o/ --> 67 <-- \o/ test cases, no more manual
 testing
 of installers \o/.

 Test suite contains: 6 combination how to install components on
 replica
 X 4 combinations of server installation + 3 extra server tests

 To save time tests install 1 master and 3 replicas per test class
 (except extra server tests):
 Class name  specifies what is installed on master.

 Remember, option "-k" is your friend
 $ ipa-run-tests -k '>>> regexp)>'
 otherwise you will need a lot of time until tests finish.

 To list all tests:
 $ ipa-run-tests test_integration/test_installation.py
 --collect-only


 Patch attached, it needs to have attached all patches I sent today
 and
 also attached workaround patch (Martin3 will provide proper fix)

 I haven't had time/mood/resources to test this patch with domain
 level
 0, so testing this patch with domain level 0 is appreciated.


 Oleg, I added some methods to tasks.py which you may want to reuse.

 Martin^2

> Updated patch attached.
> 
> Still some tests are failing, not sure if it is test issue or bugs. I
> will test later.

-- 
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] 0046 Create server certs with DNS altname

[Fraser Tweedale]

On Tue, Dec 08, 2015 at 09:00:20AM +0100, Martin Kosek wrote:
> On 12/08/2015 02:22 AM, Fraser Tweedale wrote:
> > On Tue, Dec 08, 2015 at 08:46:39AM +1000, Fraser Tweedale wrote:
> >> On Mon, Dec 07, 2015 at 01:53:15PM +0100, Martin Kosek wrote:
> >>> On 12/07/2015 06:26 AM, Fraser Tweedale wrote:
>  The attached patch fixes
>  https://fedorahosted.org/freeipa/ticket/4970.
> 
>  Note that the problem is addressed by adding the appropriate request
>  extension to the CSR; the fix does not involve changing the default
>  profile behaviour, which is complicated (see ticket for details).
> >>>
> >>> Thanks for the patch! This is something we should really fix, I already 
> >>> get
> >>> warnings in my Python scripts when I hit sites protected by such HTTPS 
> >>> cert:
> >>>
> >>> /usr/lib/python2.7/site-packages/requests/packages/urllib3/connection.py:264:
> >>> SubjectAltNameWarning: Certificate for projects.engineering.redhat.com 
> >>> has no
> >>> `subjectAltName`, falling back to check for a `commonName` for now. This
> >>> feature is being removed by major browsers and deprecated by RFC 2818. 
> >>> (See
> >>> https://github.com/shazow/urllib3/issues/497 for details.)
> >>>
> >>> Should we split ticket 4970, for the FreeIPA server part and then for cert
> >>> profile part? As it looks like the FreeIPA server will be fixed even in 
> >>> FreeIPA
> >>> 4.3.x and the other part later.
> >>>
> >>> How difficult do you see the general FreeIPA Certificate Profile part of 
> >>> this
> >>> request? Is it a too big task to handle in 4.4 time frame?
> >>>
> >> I will split the ticket and would suggest 4.4 Backlog - it might be
> >> doable but is a lower priority than e.g. Sub-CAs.
> >>
> > PKI ticket: https://fedorahosted.org/pki/ticket/1710
> > IPA tracker: https://fedorahosted.org/freeipa/ticket/5523
> 
> Thanks. I updated the ticket and added more information. I increased priority
> as I do not want us to overlook it, as it has potential to break FreeIPA
> certificates when the major browsers remove support for such certificates. 
> Right?
>
Yes.  With my (updated) patch the IPA HTTP/LDAP certs issued during
ipa-server-install or ipa-replica-prepare and IPA client host certs
issued during ipa-client-install will be OK.  But for service and
host certs issued due to user requests this is the case.

Cheers,
Fraser

-- 
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] 0046 Create server certs with DNS altname

[Martin Kosek]

On 12/08/2015 02:22 AM, Fraser Tweedale wrote:
> On Tue, Dec 08, 2015 at 08:46:39AM +1000, Fraser Tweedale wrote:
>> On Mon, Dec 07, 2015 at 01:53:15PM +0100, Martin Kosek wrote:
>>> On 12/07/2015 06:26 AM, Fraser Tweedale wrote:
 The attached patch fixes
 https://fedorahosted.org/freeipa/ticket/4970.

 Note that the problem is addressed by adding the appropriate request
 extension to the CSR; the fix does not involve changing the default
 profile behaviour, which is complicated (see ticket for details).
>>>
>>> Thanks for the patch! This is something we should really fix, I already get
>>> warnings in my Python scripts when I hit sites protected by such HTTPS cert:
>>>
>>> /usr/lib/python2.7/site-packages/requests/packages/urllib3/connection.py:264:
>>> SubjectAltNameWarning: Certificate for projects.engineering.redhat.com has 
>>> no
>>> `subjectAltName`, falling back to check for a `commonName` for now. This
>>> feature is being removed by major browsers and deprecated by RFC 2818. (See
>>> https://github.com/shazow/urllib3/issues/497 for details.)
>>>
>>> Should we split ticket 4970, for the FreeIPA server part and then for cert
>>> profile part? As it looks like the FreeIPA server will be fixed even in 
>>> FreeIPA
>>> 4.3.x and the other part later.
>>>
>>> How difficult do you see the general FreeIPA Certificate Profile part of 
>>> this
>>> request? Is it a too big task to handle in 4.4 time frame?
>>>
>> I will split the ticket and would suggest 4.4 Backlog - it might be
>> doable but is a lower priority than e.g. Sub-CAs.
>>
> PKI ticket: https://fedorahosted.org/pki/ticket/1710
> IPA tracker: https://fedorahosted.org/freeipa/ticket/5523

Thanks. I updated the ticket and added more information. I increased priority
as I do not want us to overlook it, as it has potential to break FreeIPA
certificates when the major browsers remove support for such certificates. 
Right?

-- 
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 0372] CI: installation tests

[Oleg Fayans]

On 12/08/2015 10:09 AM, Martin Basti wrote:
> 
> 
> On 08.12.2015 10:04, Oleg Fayans wrote:
>> ACK
> Pushed to master: a11cddd75b4e887998ad6fd52a05f87e0354ea30
> 
> How about patch mbasti-371, this will not work without it.

Looks safe. Could be pushed too :)

>>
>> On 12/07/2015 10:44 PM, Martin Basti wrote:
>>>
>>> On 07.12.2015 15:51, Oleg Fayans wrote:
 On 12/07/2015 03:51 PM, Martin Basti wrote:
> On 07.12.2015 15:49, Oleg Fayans wrote:
>> Hi,
>>
>> On 12/07/2015 02:37 PM, Martin Basti wrote:
>>> On 07.12.2015 14:32, Martin Basti wrote:
 On 07.12.2015 13:24, Oleg Fayans wrote:
> Hi Martin,
>
> I would prefer both install_kra and install_ca methods to have
> raiseonerr parameter set to True by default. We need a way to test
> negatives and analyze results.
> Mine looks like this:
>
> def install_kra(host, domain_level=None,
> first_instance=False, raiseonerr=True):
> if not domain_level:
>domain_level = host.config.domain_level
> command = ["ipa-kra-install", "-U", "-p",
> host.config.dirman_password]
> if domain_level == DOMAIN_LEVEL_0 and not first_instance:
> replica_file = get_replica_filename(host)
> command.append(replica_file)
> return host.run_command(command, raiseonerr=raiseonerr)
>
> The rest looks good to me, but I did not run the tests yet.
 Sounds good, I will amend the patche later.
>>> I changed my mind, should not be the domain_level value get from
>>> function domainlevel(host)?
>> We should have a way to test negatives, like providing replica
>> file at
>> domain level=1 and not providing at domain level=0. So these
>> functions
>> should either accept replica file as a parameter, or arbitrary domain
>> level
> Agree, but I meant this
>
>if not domain_level:
> -domain_level = host.config.domain_level
> +domain_level = domainlevel(host)
 Oh, yes, you are right, that's better.

>>> Martin^2
>>>
> On 12/06/2015 10:22 PM, Martin Basti wrote:
>> My favorite today \o/ --> 67 <-- \o/ test cases, no more manual
>> testing
>> of installers \o/.
>>
>> Test suite contains: 6 combination how to install components on
>> replica
>> X 4 combinations of server installation + 3 extra server tests
>>
>> To save time tests install 1 master and 3 replicas per test class
>> (except extra server tests):
>> Class name  specifies what is installed on master.
>>
>> Remember, option "-k" is your friend
>> $ ipa-run-tests -k '> regexp)>'
>> otherwise you will need a lot of time until tests finish.
>>
>> To list all tests:
>> $ ipa-run-tests test_integration/test_installation.py
>> --collect-only
>>
>>
>> Patch attached, it needs to have attached all patches I sent
>> today
>> and
>> also attached workaround patch (Martin3 will provide proper fix)
>>
>> I haven't had time/mood/resources to test this patch with domain
>> level
>> 0, so testing this patch with domain level 0 is appreciated.
>>
>>
>> Oleg, I added some methods to tasks.py which you may want to
>> reuse.
>>
>> Martin^2
>>
>>> Updated patch attached.
>>>
>>> Still some tests are failing, not sure if it is test issue or bugs. I
>>> will test later.
> 

-- 
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 0371] CI: fix function that prepares /etc/hosts

[Oleg Fayans]

ACK

On 12/06/2015 09:52 PM, Martin Basti wrote:
> Without this fix, function removes more entries from /etc/host than is
> required, and it causes installation failure in tests without DNS
> 
> Patch attached.

-- 
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] 0046 Create server certs with DNS altname

[Fraser Tweedale]

On Mon, Dec 07, 2015 at 05:50:05PM -0500, Rob Crittenden wrote:
> Fraser Tweedale wrote:
> > On Mon, Dec 07, 2015 at 01:53:15PM +0100, Martin Kosek wrote:
> >> On 12/07/2015 06:26 AM, Fraser Tweedale wrote:
> >>> The attached patch fixes
> >>> https://fedorahosted.org/freeipa/ticket/4970.
> >>>
> >>> Note that the problem is addressed by adding the appropriate request
> >>> extension to the CSR; the fix does not involve changing the default
> >>> profile behaviour, which is complicated (see ticket for details).
> >>
> >> Thanks for the patch! This is something we should really fix, I already get
> >> warnings in my Python scripts when I hit sites protected by such HTTPS 
> >> cert:
> >>
> >> /usr/lib/python2.7/site-packages/requests/packages/urllib3/connection.py:264:
> >> SubjectAltNameWarning: Certificate for projects.engineering.redhat.com has 
> >> no
> >> `subjectAltName`, falling back to check for a `commonName` for now. This
> >> feature is being removed by major browsers and deprecated by RFC 2818. (See
> >> https://github.com/shazow/urllib3/issues/497 for details.)
> >>
> >> Should we split ticket 4970, for the FreeIPA server part and then for cert
> >> profile part? As it looks like the FreeIPA server will be fixed even in 
> >> FreeIPA
> >> 4.3.x and the other part later.
> >>
> >> How difficult do you see the general FreeIPA Certificate Profile part of 
> >> this
> >> request? Is it a too big task to handle in 4.4 time frame?
> >>
> > I will split the ticket and would suggest 4.4 Backlog - it might be
> > doable but is a lower priority than e.g. Sub-CAs.
> 
> If you are going to defer the profile part then you should probably
> update the client to also include a SAN if --request-cert is provided.
> 
> rob
> 
Yes, good idea.  Updated patch attached.

Cheers,
Fraser
From 72e24bb90fbb331644f0509371872a17f86007cb Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Mon, 7 Dec 2015 16:14:28 +1100
Subject: [PATCH] Create server and host certs with DNS altname

Currently server (HTTP / LDAP) certs are created without a Subject
Alternative Name extension during server install, replica prepare
and host enrolment, a potentially problematic violation of RFC 2818.

Add the hostname as a SAN dNSName when these certs are created.

(Certmonger adds an appropriate request extension when renewing the
certificate, so nothing needs to be done for renewal).

Fixes: https://fedorahosted.org/freeipa/ticket/4970
---
 ipa-client/ipa-install/ipa-client-install | 2 +-
 ipapython/certmonger.py   | 9 -
 ipaserver/install/certs.py| 8 ++--
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install 
b/ipa-client/ipa-install/ipa-client-install
index 
974dd1da8bf3f5836170ca67d2f4c298e7ec6844..fd273597944b8d07a2c9bdb96f6a32566085747f
 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -1167,7 +1167,7 @@ def configure_certmonger(fstore, subject_base, cli_realm, 
hostname, options,
 try:
 certmonger.request_cert(nssdb=paths.IPA_NSSDB_DIR,
 nickname='Local IPA host',
-subject=subject,
+subject=subject, dns=[hostname],
 principal=principal,
 passwd_fname=passwd_fname)
 except Exception:
diff --git a/ipapython/certmonger.py b/ipapython/certmonger.py
index 
2a4e43d3c5d5746134fc5b11a2d01d05f67a2e26..8901d3bb068cc1e0c94ea6c5a093d054ce0557e6
 100644
--- a/ipapython/certmonger.py
+++ b/ipapython/certmonger.py
@@ -299,9 +299,14 @@ def add_subject(request_id, subject):
 add_request_value(request_id, 'template-subject', subject)
 
 
-def request_cert(nssdb, nickname, subject, principal, passwd_fname=None):
+def request_cert(
+nssdb, nickname, subject, principal, passwd_fname=None,
+dns=None):
 """
 Execute certmonger to request a server certificate.
+
+``dns``
+A sequence of DNS names to appear in SAN request extension.
 """
 cm = _certmonger()
 ca_path = cm.obj_if.find_ca_by_nickname('IPA')
@@ -312,6 +317,8 @@ def request_cert(nssdb, nickname, subject, principal, 
passwd_fname=None):
   KEY_LOCATION=nssdb, KEY_NICKNAME=nickname,
   SUBJECT=subject, PRINCIPAL=[principal],
   CA=ca_path)
+if dns is not None and len(dns) > 0:
+request_parameters['DNS'] = dns
 if passwd_fname:
 request_parameters['KEY_PIN_FILE'] = passwd_fname
 result = cm.obj_if.add_request(request_parameters)
diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py
index 
c918791f0be7a17e20123fe6f94c4ac0bbf09d7b..bd1792d32246bc3034c5403f1d868e0966ec0014
 100644
--- a/ipaserver/install/certs.py
+++ b/ipaserver/install/certs.py
@@ -335,7 +335,7 @@ class 

Re: [Freeipa-devel] [PATCH 0371] CI: fix function that prepares /etc/hosts

[Martin Basti]

On 08.12.2015 10:22, Oleg Fayans wrote:

ACK

On 12/06/2015 09:52 PM, Martin Basti wrote:

Without this fix, function removes more entries from /etc/host than is
required, and it causes installation failure in tests without DNS

Patch attached.

Pushed to master: e4259d5b49a6f501f0a6f1b020bf492cac06e1c7

--
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 0372] CI: installation tests

[Martin Basti]

On 08.12.2015 10:04, Oleg Fayans wrote:

ACK

Pushed to master: a11cddd75b4e887998ad6fd52a05f87e0354ea30

How about patch mbasti-371, this will not work without it.


On 12/07/2015 10:44 PM, Martin Basti wrote:


On 07.12.2015 15:51, Oleg Fayans wrote:

On 12/07/2015 03:51 PM, Martin Basti wrote:

On 07.12.2015 15:49, Oleg Fayans wrote:

Hi,

On 12/07/2015 02:37 PM, Martin Basti wrote:

On 07.12.2015 14:32, Martin Basti wrote:

On 07.12.2015 13:24, Oleg Fayans wrote:

Hi Martin,

I would prefer both install_kra and install_ca methods to have
raiseonerr parameter set to True by default. We need a way to test
negatives and analyze results.
Mine looks like this:

def install_kra(host, domain_level=None,
first_instance=False, raiseonerr=True):
if not domain_level:
   domain_level = host.config.domain_level
command = ["ipa-kra-install", "-U", "-p",
host.config.dirman_password]
if domain_level == DOMAIN_LEVEL_0 and not first_instance:
replica_file = get_replica_filename(host)
command.append(replica_file)
return host.run_command(command, raiseonerr=raiseonerr)

The rest looks good to me, but I did not run the tests yet.

Sounds good, I will amend the patche later.

I changed my mind, should not be the domain_level value get from
function domainlevel(host)?

We should have a way to test negatives, like providing replica file at
domain level=1 and not providing at domain level=0. So these functions
should either accept replica file as a parameter, or arbitrary domain
level

Agree, but I meant this

   if not domain_level:
-domain_level = host.config.domain_level
+domain_level = domainlevel(host)

Oh, yes, you are right, that's better.


Martin^2


On 12/06/2015 10:22 PM, Martin Basti wrote:

My favorite today \o/ --> 67 <-- \o/ test cases, no more manual
testing
of installers \o/.

Test suite contains: 6 combination how to install components on
replica
X 4 combinations of server installation + 3 extra server tests

To save time tests install 1 master and 3 replicas per test class
(except extra server tests):
Class name  specifies what is installed on master.

Remember, option "-k" is your friend
$ ipa-run-tests -k ''
otherwise you will need a lot of time until tests finish.

To list all tests:
$ ipa-run-tests test_integration/test_installation.py
--collect-only


Patch attached, it needs to have attached all patches I sent today
and
also attached workaround patch (Martin3 will provide proper fix)

I haven't had time/mood/resources to test this patch with domain
level
0, so testing this patch with domain level 0 is appreciated.


Oleg, I added some methods to tasks.py which you may want to reuse.

Martin^2


Updated patch attached.

Still some tests are failing, not sure if it is test issue or bugs. I
will test later.


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

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

[David Kupka]

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

On 7.12.2015 14:41, David Kupka wrote:

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



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

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


Updated patches attached.

--
David Kupka
From 6927ea57fe73ad9dfd64d432aa18fd7b3ecda084 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Wed, 2 Dec 2015 13:17:13 +
Subject: [PATCH] dns: do not add (forward)zone if it is already resolvable.

Check if the zone user wants to add is already resolvable and refuse to
create it if yes. --skip-overlap-check and --force options suppress this check.

https://fedorahosted.org/freeipa/ticket/5087
---
 API.txt   |  7 +++--
 ipalib/plugins/dns.py | 32 ---
 ipapython/ipautil.py  | 87 +--
 3 files changed, 117 insertions(+), 9 deletions(-)

diff --git a/API.txt b/API.txt
index 60c98c31aa85d6c8879cd145f3d84188d4fea5b7..3a9fb65a386a2a6529b8cd241642446c135471f2 100644
--- a/API.txt
+++ b/API.txt
@@ -959,7 +959,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
 command: dnsforwardzone_add
-args: 1,8,3
+args: 1,9,3
 arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, 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')
@@ -968,6 +968,7 @@ option: StrEnum('idnsforwardpolicy', attribute=True, cli_name='forward_policy',
 option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue=False, required=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_overlap_check', autofill=True, default=False)
 option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
@@ -1366,7 +1367,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
 command: dnszone_add
-args: 1,26,3
+args: 1,28,3
 arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, 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')
@@ -1393,6 +1394,8 @@ option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue
 option: Str('nsec3paramrecord', attribute=True, cli_name='nsec3param_rec', multivalue=False, pattern='^\\d+ \\d+ \\d+ (([0-9a-fA-F]{2})+|-)$', required=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_nameserver_check', autofill=True, default=False)
+option: Flag('skip_overlap_check', autofill=True, default=False)
 option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 67947360eb207de31ed114bb630705c409b2f9a9..9cad9cfb8b4175cc92778b2df057621ca055e58f 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -53,8 +53,7 @@ from ipalib.util import (normalize_zonemgr,
  validate_dnssec_zone_forwarder_step1,
  validate_dnssec_zone_forwarder_step2,
  verify_host_resolvable)
-
-from ipapython.ipautil import CheckedIPAddress
+from ipapython.ipautil import CheckedIPAddress, check_zone_overlap
 from ipapython.dnsutil import DNSName
 
 if six.PY3:
@@ -2121,6 +2120,13 @@ class DNSZoneBase(LDAPObject):
 
 class DNSZoneBase_add(LDAPCreate):
 
+takes_options = LDAPCreate.takes_options + (
+Flag('skip_overlap_check',
+ doc=_('Force DNS zone creation even if it will overlap with '
+   'an existing zone.')
+),
+)
+
 has_output_params = LDAPCreate.has_output_params + dnszone_output_params
 
 def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
@@ -2140,6 +2146,12 @@ class DNSZoneBase_add(LDAPCreate):
 
 

Re: [Freeipa-devel] [PATCH 0068] add missing /ipaplatform/constants.py to .gitignore

[Tomas Babej]

On 12/08/2015 01:26 PM, Tomas Babej wrote:
> 
> 
> On 12/08/2015 01:26 PM, Petr Spacek wrote:
>> Hello,
>>
>> add missing /ipaplatform/constants.py to .gitignore
>>
> 
> ACK.
> 

Pushed to master: 848912ae31d1549d5f6bed874cc6c4541bada6f4

-- 
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 0026] Workarounds for SELinux execmem violations in cryptography

[Christian Heimes]

On 2015-12-07 19:59, Petr Vobornik wrote:
> On 7.12.2015 16:26, Christian Heimes wrote:
>> On 2015-12-07 16:17, Alexander Bokovoy wrote:
>>> On Mon, 07 Dec 2015, Christian Heimes wrote:
 The patch fixes SELinux violations in Fedora 23.

 Background: Recent versions of cryptography cause SELinux violation
 which will lead to a segfault, see
 https://bugzilla.redhat.com/show_bug.cgi?id=1277224 . The segfault only
 occurs in the context of Apache HTTPD (FreeIPA web ui) when
 cryptography.hazmat.backends.default_backend() is initialized. I'm
 working on a fix for cryptography but it will take a while. First I
 have
 to wait for a new upstream release of python-cffi. Armin Ronacher plans
 to release cffi 1.4 in two weeks.


 ipaserver.dcerpc uses M2Crypto again on Python 2.7 and Dogtag's
 pki.client no longer tries to use PyOpenSSL instead of Python's ssl
 module.

 Some dependencies like Dogtag's pki.client library and custodia use
 python-requsts to make HTTPS connection. python-requests prefers
 PyOpenSSL over Python's stdlib ssl module. PyOpenSSL is build on top
 of python-cryptography which trigger a execmem SELinux violation
 in the context of Apache HTTPD (httpd_execmem).
 When requests is imported, it always tries to import pyopenssl glue
 code from urllib3's contrib directory. The import of PyOpenSSL is
 enough to trigger the SELinux denial.
 A hack in wsgi.py prevents the import by raising an ImportError.
>>> ACK. Thanks for these patches.
>>>
>>> Note to Debian/Ubuntu maintainers: AppArmor 'support' in python-cffi
>>> already detects apparmor by looking into /proc and disabling the use of
>>> writeable and executable memory. On those platforms I suspect recent
>>> enough python-cryptography would work without problem by downgrading own
>>> feature set. The code in this patches should be harmless, though.
>>
>> Cryptography's core depends on dynamic callbacks. There is no "downgrade
>> feature-set" feature.
>>
>> I guess the libffi uses the broken and potential dangerous workaround
>> with two shared mmap() with file backend.
>> (http://www.akkadia.org/drepper/selinux-mem.html). The approach requires
>> a writeable, executable temp file and breaks isolation between a parent
>> process and all its forked child processes.
>>
>> Christian
>>
> 
> The patch needs to be rebased to 4-2 branch to be usable on Fedora 23 -
> FreeIPA 4.2.3.

For FreeIPA 4.2 only the patch in wsgi.py is needed. The older version
doesn't use cryptography for RC4. I've attached a patch.

Christian

From ef68483bb3c9e328e3d65e0c02327cdb5ac9859a Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Tue, 8 Dec 2015 11:18:22 +0100
Subject: [PATCH 26/26] Workarounds for SELinux execmem violations in
 cryptography

Some dependencies like Dogtag's pki.client library and custodia use
python-requsts to make HTTPS connection. python-requests prefers
PyOpenSSL over Python's stdlib ssl module. PyOpenSSL is build on top
of python-cryptography which trigger a execmem SELinux violation
in the context of Apache HTTPD (httpd_execmem).
When requests is imported, it always tries to import pyopenssl glue
code from urllib3's contrib directory. The import of PyOpenSSL is
enough to trigger the SELinux denial.
A hack in wsgi.py prevents the import by raising an ImportError.
---
 install/share/wsgi.py | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/install/share/wsgi.py b/install/share/wsgi.py
index 9f7d3f487dbe07f60b748cfd48d533495de99f2c..ffeb3bb6caea62c82d19e4e772b47efa43cc715f 100644
--- a/install/share/wsgi.py
+++ b/install/share/wsgi.py
@@ -23,6 +23,20 @@
 """
 WSGI appliction for IPA server.
 """
+import sys
+
+# Some dependencies like Dogtag's pki.client library and custodia use
+# python-requsts to make HTTPS connection. python-requests prefers
+# PyOpenSSL over Python's stdlib ssl module. PyOpenSSL is build on top
+# of python-cryptography which trigger a execmem SELinux violation
+# in the context of Apache HTTPD (httpd_execmem).
+# When requests is imported, it always tries to import pyopenssl glue
+# code from urllib3's contrib directory. The import of PyOpenSSL is
+# enough to trigger the SELinux denial.
+# This hack prevents the import by raising an ImportError.
+
+sys.modules['request.packages.urllib3.contrib.pyopenssl'] = None
+
 from ipalib import api
 from ipalib.config import Env
 from ipalib.constants import DEFAULT_CONFIG
-- 
2.5.0



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 522] replica promotion: allow OTP bulk client enrollment

[Martin Basti]

On 08.12.2015 13:09, Jan Cholasta wrote:

On 8.12.2015 12:49, Martin Basti wrote:



On 08.12.2015 10:31, Martin Basti wrote:



On 08.12.2015 08:52, Jan Cholasta wrote:

On 7.12.2015 21:11, Martin Basti wrote:



On 07.12.2015 08:21, Jan Cholasta wrote:

On 2.12.2015 16:23, Jan Cholasta wrote:

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






Pushed.




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


Martin Basti pointed out that admin password should not be asked
interactively during OTP replica promotion. Fixed.

Updated and rebased patch attached.





1)
[root@vm-058-138 ~]# ipa-replica-install --server
vm-058-137.abc.idm.lab.eng.brq.redhat.com --domain
abc.idm.lab.eng.brq.redhat.com --password=bubak --setup-ca
Configuring client side components
Password for ad...@abc.idm.lab.eng.brq.redhat.com:

IMO password should be asked first, before any installation begins 
(IMO

this is for conncheck)


The same thing happens without my patch. Could you file a ticket?

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





2)
When host is not in ipaservers hostgroup. Also I would expect 
different

error message
ipa-replica-install --server 
vm-058-137.abc.idm.lab.eng.brq.redhat.com

--domain abc.idm.lab.eng.brq.redhat.com --password=bubak --setup-ca
--skip-conncheck


 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 63, in _install
 for nothing in self._installer(self.parent):
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/replicainstall.py", 



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



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



line 1002, in promote_check
 conn.connect(ccache=installer._ccache)
   File "/usr/lib/python2.7/site-packages/ipalib/backend.py", line 
66,

in connect
 conn = self.create_connection(*args, **kw)
   File 
"/usr/lib/python2.7/site-packages/ipaserver/plugins/ldap2.py",

line 199, in create_connection
 principal = krb_utils.get_principal(ccache_name=ccache)
   File "/usr/lib/python2.7/site-packages/ipalib/krb_utils.py", line
184, in get_principal
 raise errors.CCacheError(message=unicode(e))

2015-12-07T16:23:40Z DEBUG The ipa-replica-install command failed,
exception: CCacheError: Major (851968): Unspecified GSS failure. 
Minor

code may provide more information, Minor (2529639053): No Kerberos
credentials available
2015-12-07T16:23:40Z ERROR Major (851968): Unspecified GSS failure.
Minor code may provide more information, Minor (2529639053): No
Kerberos
credentials available


Fixed.




3)
This case is not handle very well:
a) install client with OTP password
b) install replica with the same OTP password (when host is no in
ipaservers group, if host is in ipaservers group it works)

ipa.ipapython.install.cli.install_tool(Replica): ERROR Major
(851968): Unspecified GSS failure.  Minor code may provide more
information, Minor (2529639053): No Kerberos credentials available
ipa.ipapython.install.cli.install_tool(Replica): ERROR The
ipa-replica-install command failed. See 
/var/log/ipareplica-install.log

for more information


This is the same as 2).



4)
This is not user friendly
I used wrong OTP password, can we somehow propagate the actual error
from client install to stderr?

ipa.ipapython.install.cli.install_tool(Replica): ERROR 
Configuration of

client side components failed!
ipa-client-install returned: Command ''/usr/sbin/ipa-client-install'
'--unattended' '--domain' 'abc.idm.lab.eng.brq.redhat.com' '--server'
'vm-058-137.abc.idm.lab.eng.brq.redhat.com' '--password' 'buba''
returned non-zero exit status 1
ipa.ipapython.install.cli.install_tool(Replica): ERROR The
ipa-replica-install command failed. See 
/var/log/ipareplica-install.log

for more information


The same thing happens without my patch for any other error. Could
you file a ticket?


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



Updated patch attached.


Working on review



Is 

Re: [Freeipa-devel] patch acceptance criteria

[Rob Crittenden]

Petr Spacek wrote:
> On 4.12.2015 14:42, Rob Crittenden wrote:
>> Lukas Slebodnik wrote:
 On (03/12/15 09:59), Rob Crittenden wrote:
>> Lukas Slebodnik wrote:
 On (02/12/15 13:14), Rob Crittenden wrote:
>> Is it still mandatory that tests pass the unit tests before 
>> acceptance?
 Unit test could be executed as part of "%check" phase in spec files.
 I recently added C-base unit tests there.

 I was not bale to run "make tests" there because many tests failed.
 If they require real IPA server for execution ( or lite server)
 then they are not unit test but integration tests.
 One solution would be to skip them or to usw cwrap[1] + lite server.
 So it can be run also in mock/koji which has many restrictions.
>>
>> It would represent quite a lot of work but it may be a good idea to
>> investigate. Ipsilon uses cwrap for its tests so some of the
>> configuration can be gleaned from that.
>>
>> I would definitely be opposed to this as part of the freeipa.spec in the
^^^
 What do you mean by this part?

 Did you mean "running make tests" in spec file?
 If yes, could you elaborate why it's not good idea?
 many projects run tests in "%check"
 sssd, certmonger, glibc ...

 Currently only C-based test are executed. And I added it only recently.
>> Because it would be overkill during development. The expectation is that
>> developers and reviewers run the tests before submission/acceptance. If
>> they fail to do that then it will be obvious.
>>
>> git tree. In Fedora it might help find problems when rawhide becomes
>> Fedora.next so it would provide some value there.
>>

 Also lint should be also part of "%check" phase and not part of
 ordinary build.
 BTW I could not see a lint[2] in fedora build at all. So I'm not sure
 if it is executed with upstream spec file.
>>
>> It isn't there because the expectation is that lint already passes as
>> part of the release process. I don't see the value on running lint on
>> release tarballs.
>>
 That's just an expectation. It needn't be true. Your initial mail was about
 stricter review process. And automating things is best way how to
 enforce it. So reviewer would just build rpms and if "%check" phase
 will not pass then he will not continue with review.
 If it will be part of "%check" then you can use mock and easily ensure
 that test passes on stable fedora and fedora rawhide (and maybe centOS)
>> By the time downstream gets a tarball it is too late to fix lint errors.
>> If upstream is doing a release with lint errors then there is something
>> deeply wrong with the release process. If someone wants to add it to the
>> downstream spec files I'm not going to complain, I just find it
>> extremely unlikely that it will provide any value, ever.
> 
> Sorry Rob, but I disagree. Lint already caught couple cases where Requires:
> were not properly updated so IPA code was referencing non-existing code in
> Dogtag/Custodia packages and so on.
> 
> So clearly there is some value in it.
> 

I'm referring to the downstream spec files (e.g. Fedora) which don't run
the lint target during the build.

rob

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

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

[Martin Basti]

On 08.12.2015 10:31, Martin Basti wrote:



On 08.12.2015 08:52, Jan Cholasta wrote:

On 7.12.2015 21:11, Martin Basti wrote:



On 07.12.2015 08:21, Jan Cholasta wrote:

On 2.12.2015 16:23, Jan Cholasta wrote:

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





Pushed.




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

with .


Martin Basti pointed out that admin password should not be asked
interactively during OTP replica promotion. Fixed.

Updated and rebased patch attached.





1)
[root@vm-058-138 ~]# ipa-replica-install --server
vm-058-137.abc.idm.lab.eng.brq.redhat.com --domain
abc.idm.lab.eng.brq.redhat.com --password=bubak  --setup-ca
Configuring client side components
Password for ad...@abc.idm.lab.eng.brq.redhat.com:

IMO password should be asked first, before any installation begins (IMO
this is for conncheck)


The same thing happens without my patch. Could you file a ticket?

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





2)
When host is not in ipaservers hostgroup. Also I would expect different
error message
ipa-replica-install --server vm-058-137.abc.idm.lab.eng.brq.redhat.com
--domain abc.idm.lab.eng.brq.redhat.com --password=bubak --setup-ca
--skip-conncheck


 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 63, in _install
 for nothing in self._installer(self.parent):
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/replicainstall.py", 


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


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


line 1002, in promote_check
 conn.connect(ccache=installer._ccache)
   File "/usr/lib/python2.7/site-packages/ipalib/backend.py", line 66,
in connect
 conn = self.create_connection(*args, **kw)
   File "/usr/lib/python2.7/site-packages/ipaserver/plugins/ldap2.py",
line 199, in create_connection
 principal = krb_utils.get_principal(ccache_name=ccache)
   File "/usr/lib/python2.7/site-packages/ipalib/krb_utils.py", line
184, in get_principal
 raise errors.CCacheError(message=unicode(e))

2015-12-07T16:23:40Z DEBUG The ipa-replica-install command failed,
exception: CCacheError: Major (851968): Unspecified GSS failure. Minor
code may provide more information, Minor (2529639053): No Kerberos
credentials available
2015-12-07T16:23:40Z ERROR Major (851968): Unspecified GSS failure.
Minor code may provide more information, Minor (2529639053): No 
Kerberos

credentials available


Fixed.




3)
This case is not handle very well:
a) install client with OTP password
b) install replica with the same OTP password (when host is no in
ipaservers group, if host is in ipaservers group it works)

ipa.ipapython.install.cli.install_tool(Replica): ERROR Major
(851968): Unspecified GSS failure.  Minor code may provide more
information, Minor (2529639053): No Kerberos credentials available
ipa.ipapython.install.cli.install_tool(Replica): ERRORThe
ipa-replica-install command failed. See /var/log/ipareplica-install.log
for more information


This is the same as 2).



4)
This is not user friendly
I used wrong OTP password, can we somehow propagate the actual error
from client install to stderr?

ipa.ipapython.install.cli.install_tool(Replica): ERROR Configuration of
client side components failed!
ipa-client-install returned: Command ''/usr/sbin/ipa-client-install'
'--unattended' '--domain' 'abc.idm.lab.eng.brq.redhat.com' '--server'
'vm-058-137.abc.idm.lab.eng.brq.redhat.com' '--password' 'buba''
returned non-zero exit status 1
ipa.ipapython.install.cli.install_tool(Replica): ERRORThe
ipa-replica-install command failed. See /var/log/ipareplica-install.log
for more information


The same thing happens without my patch for any other error. Could 
you file a ticket?


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



Updated patch attached.


Working on review



Is this expected that client will be installed even if there is not 
enough privileges to install 

[Freeipa-devel] [PATCH 0068] add missing /ipaplatform/constants.py to .gitignore

[Petr Spacek]

Hello,

add missing /ipaplatform/constants.py to .gitignore

-- 
Petr^2 Spacek
From f9116e6f999e0b9915bc6095fa0377eb13304bbb Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Tue, 8 Dec 2015 13:25:21 +0100
Subject: [PATCH] add missing /ipaplatform/constants.py to .gitignore

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index fa35df85abd5b18522d2be17070c3d8aceb9bdc5..13f9eb34a4528fc22af59a3b364057f642109153 100644
--- a/.gitignore
+++ b/.gitignore
@@ -78,3 +78,4 @@ freeipa2-dev-doc
 /ipaplatform/tasks.py
 /ipaplatform/services.py
 /ipaplatform/paths.py
+/ipaplatform/constants.py
-- 
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 0068] add missing /ipaplatform/constants.py to .gitignore

[Tomas Babej]

On 12/08/2015 01:26 PM, Petr Spacek wrote:
> Hello,
> 
> add missing /ipaplatform/constants.py to .gitignore
> 

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 0391] replicainstall: Add check for domain if server is specified

[Simo Sorce]

On Tue, 2015-12-08 at 13:34 +0100, Martin Kosek wrote:
> On 12/08/2015 08:28 AM, Jan Cholasta wrote:
> > On 8.12.2015 08:23, Martin Kosek wrote:
> >> On 12/08/2015 07:57 AM, Jan Cholasta wrote:
> >>> On 7.12.2015 16:43, Martin Kosek wrote:
>  On 12/07/2015 02:17 PM, Tomas Babej wrote:
> >
> >
> > On 12/04/2015 08:22 PM, Rob Crittenden wrote:
> >> Martin Kosek wrote:
> >>> On 12/04/2015 07:17 PM, Tomas Babej wrote:
>  Hi,
> 
>  Avoids failing in the later stages during the ipa-client-install
>  command.
> 
>  Tomas
> >>>
> >>> Is this change needed? Wouldn't it be better to update
> >>> ipa-client-install or ipa-replica-install to not require the --domain
> >>> option? I would hope that --domain can be figured out during
> >>> installation and not passed to ipa-replica-install manually by the 
> >>> admin.
> >>>
> >>> I just think that calling
> >>> # ipa-replica-install --server=master.example.com
> >>> is better than
> >>> # ipa-replica-install --server=master.example.com --domain example.com
> >>> if possible.
> >>
> >> IIRC this is for service discovery when using a specific server and not
> >> LDAP. This is the domain used to search for the kerberos realm, for
> >> example.
> >>
> >> That isn't to say this isn't discoverable but it would require another
> >> function in discovery to query what the IPA domain is from the given
> >> master but it gets tricky if anonymous search is disabled, for example.
> >>
> >> rob
> >>
> >
> > Needed or not, this is the behaviour that ipa-client-install has now.
> > Adding a domain detection method would be a RFE for ipa-client-install
> > (and imho not something we should be adding at this point).
> >
> > This patch only focuses on making the ipa-replica-install work more
> > smoothly.
> 
>  I am just thinking that client promotion (ipa-replica-install) and
>  ipa-client-install are a bit different use cases. While 
>  ipa-client-install
>  should be typically run in auto-discovery and you thus do not use 
>  --server
>  option much, while with ipa-replica-install, you want to make sure you 
>  have
>  the
>  expected topology and should use --server all the time without gambling 
>  on it.
> 
>  But I do not think it has to be there since 4.3 GA, can you please file a
>  ticket for this gap?
> >>>
> >>> I would rather do it now, because the change from optional to mandatory is
> >>> backward incompatible. (We don't want to break users' scripts, right?)
> >>
> >> I think it is the other way around - with the change I was suggesting
> >> (autodetecting --domain option instead of always requesting it, as in 
> >> Tomas'
> >> patch which we can merge if my proposal is not doable for 4.3 GA).
> >>
> > 
> > "with ipa-replica-install, you want to make sure you have the expected 
> > topology
> > and should use --server all the time" sounds like you want to make --server
> > mandatory for ipa-replica-install, which should be done either before 4.3 
> > GA or
> > never.
> 
> Ah, no, this is not what I meant. I was only discussing the --domain option in
> this mail the the typical use cases for --server option in ipa-client-install
> and ipa-replica-install.
> 
> If we can trust ipa-replica-install to do a good job in picking a server to
> replica from, the --server can stay optional. Although I am on fence here,
> being more explicit when creating topology may be helpful. CCing Simo, in case
> he has some opinions on this.

Leave it optional, our first order of business is making things simple,
then adding optional knobs to let the admin with knowledge to tweak
things.

Simo.

-- 
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 0394] topology: Make sure the old 'realm' topology suffix is not

[Tomas Babej]

Hi,

The old 'realm' topology suffix is no longer used, however, it was being
created on masters with version 4.2.3 and later. Make sure it's properly
removed.

Note that this is not the case for the 'ipaca' suffix, which was later
removed to 'ca'.

https://fedorahosted.org/freeipa/ticket/5526
From 4c60de6009140f389bc45a5649868f1fde938421 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Tue, 8 Dec 2015 13:34:15 +0100
Subject: [PATCH] topology: Make sure the old 'realm' topology suffix is not
 used

The old 'realm' topology suffix is no longer used, however, it was being
created on masters with version 4.2.3 and later. Make sure it's properly
removed.

Note that this is not the case for the 'ipaca' suffix, which was later
removed to 'ca'.

https://fedorahosted.org/freeipa/ticket/5526
---
 install/updates/20-replication.update | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/install/updates/20-replication.update b/install/updates/20-replication.update
index a471742532cf5954be1b76dbe4a6d908e4cefa2c..1543a04c917c386e93ed93dfd2767e0fde4685f5 100644
--- a/install/updates/20-replication.update
+++ b/install/updates/20-replication.update
@@ -31,6 +31,9 @@ add: nsDS5ReplicatedAttributeList: $EXCLUDES
 add: nsDS5ReplicatedAttributeListTotal: $TOTAL_EXCLUDES
 add: nsds5ReplicaStripAttrs: $STRIP_ATTRS
 
+# Remove old topology configuration area (unused)
+deleteentry: cn=realm,cn=topology,cn=ipa,cn=etc,$SUFFIX
+
 # add IPA realm managed suffix to master entry
 dn: cn=$FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX
 add: objectclass: ipaReplTopoManagedServer
-- 
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 acceptance criteria

[Petr Spacek]

On 4.12.2015 14:42, Rob Crittenden wrote:
> Lukas Slebodnik wrote:
>> > On (03/12/15 09:59), Rob Crittenden wrote:
>>> >> Lukas Slebodnik wrote:
 >>> On (02/12/15 13:14), Rob Crittenden wrote:
>  Is it still mandatory that tests pass the unit tests before 
>  acceptance?
 >>> Unit test could be executed as part of "%check" phase in spec files.
 >>> I recently added C-base unit tests there.
 >>>
 >>> I was not bale to run "make tests" there because many tests failed.
 >>> If they require real IPA server for execution ( or lite server)
 >>> then they are not unit test but integration tests.
 >>> One solution would be to skip them or to usw cwrap[1] + lite server.
 >>> So it can be run also in mock/koji which has many restrictions.
>>> >>
>>> >> It would represent quite a lot of work but it may be a good idea to
>>> >> investigate. Ipsilon uses cwrap for its tests so some of the
>>> >> configuration can be gleaned from that.
>>> >>
>>> >> I would definitely be opposed to this as part of the freeipa.spec in the
>> >^^^
>> > What do you mean by this part?
>> > 
>> > Did you mean "running make tests" in spec file?
>> > If yes, could you elaborate why it's not good idea?
>> > many projects run tests in "%check"
>> > sssd, certmonger, glibc ...
>> > 
>> > Currently only C-based test are executed. And I added it only recently.
> Because it would be overkill during development. The expectation is that
> developers and reviewers run the tests before submission/acceptance. If
> they fail to do that then it will be obvious.
> 
>>> >> git tree. In Fedora it might help find problems when rawhide becomes
>>> >> Fedora.next so it would provide some value there.
>>> >>
 >>>
 >>> Also lint should be also part of "%check" phase and not part of
 >>> ordinary build.
 >>> BTW I could not see a lint[2] in fedora build at all. So I'm not sure
 >>> if it is executed with upstream spec file.
>>> >>
>>> >> It isn't there because the expectation is that lint already passes as
>>> >> part of the release process. I don't see the value on running lint on
>>> >> release tarballs.
>>> >>
>> > That's just an expectation. It needn't be true. Your initial mail was about
>> > stricter review process. And automating things is best way how to
>> > enforce it. So reviewer would just build rpms and if "%check" phase
>> > will not pass then he will not continue with review.
>> > If it will be part of "%check" then you can use mock and easily ensure
>> > that test passes on stable fedora and fedora rawhide (and maybe centOS)
> By the time downstream gets a tarball it is too late to fix lint errors.
> If upstream is doing a release with lint errors then there is something
> deeply wrong with the release process. If someone wants to add it to the
> downstream spec files I'm not going to complain, I just find it
> extremely unlikely that it will provide any value, ever.

Sorry Rob, but I disagree. Lint already caught couple cases where Requires:
were not properly updated so IPA code was referencing non-existing code in
Dogtag/Custodia packages and so on.

So clearly there is some value in it.

-- 
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 522] replica promotion: allow OTP bulk client enrollment

[Jan Cholasta]

On 8.12.2015 12:49, Martin Basti wrote:



On 08.12.2015 10:31, Martin Basti wrote:



On 08.12.2015 08:52, Jan Cholasta wrote:

On 7.12.2015 21:11, Martin Basti wrote:



On 07.12.2015 08:21, Jan Cholasta wrote:

On 2.12.2015 16:23, Jan Cholasta wrote:

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




Pushed.




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


Martin Basti pointed out that admin password should not be asked
interactively during OTP replica promotion. Fixed.

Updated and rebased patch attached.





1)
[root@vm-058-138 ~]# ipa-replica-install --server
vm-058-137.abc.idm.lab.eng.brq.redhat.com --domain
abc.idm.lab.eng.brq.redhat.com --password=bubak  --setup-ca
Configuring client side components
Password for ad...@abc.idm.lab.eng.brq.redhat.com:

IMO password should be asked first, before any installation begins (IMO
this is for conncheck)


The same thing happens without my patch. Could you file a ticket?

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





2)
When host is not in ipaservers hostgroup. Also I would expect different
error message
ipa-replica-install --server vm-058-137.abc.idm.lab.eng.brq.redhat.com
--domain abc.idm.lab.eng.brq.redhat.com --password=bubak --setup-ca
--skip-conncheck


 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 63, in _install
 for nothing in self._installer(self.parent):
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/replicainstall.py",

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

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

line 1002, in promote_check
 conn.connect(ccache=installer._ccache)
   File "/usr/lib/python2.7/site-packages/ipalib/backend.py", line 66,
in connect
 conn = self.create_connection(*args, **kw)
   File "/usr/lib/python2.7/site-packages/ipaserver/plugins/ldap2.py",
line 199, in create_connection
 principal = krb_utils.get_principal(ccache_name=ccache)
   File "/usr/lib/python2.7/site-packages/ipalib/krb_utils.py", line
184, in get_principal
 raise errors.CCacheError(message=unicode(e))

2015-12-07T16:23:40Z DEBUG The ipa-replica-install command failed,
exception: CCacheError: Major (851968): Unspecified GSS failure. Minor
code may provide more information, Minor (2529639053): No Kerberos
credentials available
2015-12-07T16:23:40Z ERROR Major (851968): Unspecified GSS failure.
Minor code may provide more information, Minor (2529639053): No
Kerberos
credentials available


Fixed.




3)
This case is not handle very well:
a) install client with OTP password
b) install replica with the same OTP password (when host is no in
ipaservers group, if host is in ipaservers group it works)

ipa.ipapython.install.cli.install_tool(Replica): ERROR Major
(851968): Unspecified GSS failure.  Minor code may provide more
information, Minor (2529639053): No Kerberos credentials available
ipa.ipapython.install.cli.install_tool(Replica): ERRORThe
ipa-replica-install command failed. See /var/log/ipareplica-install.log
for more information


This is the same as 2).



4)
This is not user friendly
I used wrong OTP password, can we somehow propagate the actual error
from client install to stderr?

ipa.ipapython.install.cli.install_tool(Replica): ERROR Configuration of
client side components failed!
ipa-client-install returned: Command ''/usr/sbin/ipa-client-install'
'--unattended' '--domain' 'abc.idm.lab.eng.brq.redhat.com' '--server'
'vm-058-137.abc.idm.lab.eng.brq.redhat.com' '--password' 'buba''
returned non-zero exit status 1
ipa.ipapython.install.cli.install_tool(Replica): ERRORThe
ipa-replica-install command failed. See /var/log/ipareplica-install.log
for more information


The same thing happens without my patch for any other error. Could
you file a ticket?


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



Updated patch attached.


Working on review



Is this expected that client will be installed even if there is not
enough 

Re: [Freeipa-devel] [PATCH 0391] replicainstall: Add check for domain if server is specified

[Martin Kosek]

On 12/08/2015 08:28 AM, Jan Cholasta wrote:
> On 8.12.2015 08:23, Martin Kosek wrote:
>> On 12/08/2015 07:57 AM, Jan Cholasta wrote:
>>> On 7.12.2015 16:43, Martin Kosek wrote:
 On 12/07/2015 02:17 PM, Tomas Babej wrote:
>
>
> On 12/04/2015 08:22 PM, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> On 12/04/2015 07:17 PM, Tomas Babej wrote:
 Hi,

 Avoids failing in the later stages during the ipa-client-install
 command.

 Tomas
>>>
>>> Is this change needed? Wouldn't it be better to update
>>> ipa-client-install or ipa-replica-install to not require the --domain
>>> option? I would hope that --domain can be figured out during
>>> installation and not passed to ipa-replica-install manually by the 
>>> admin.
>>>
>>> I just think that calling
>>> # ipa-replica-install --server=master.example.com
>>> is better than
>>> # ipa-replica-install --server=master.example.com --domain example.com
>>> if possible.
>>
>> IIRC this is for service discovery when using a specific server and not
>> LDAP. This is the domain used to search for the kerberos realm, for
>> example.
>>
>> That isn't to say this isn't discoverable but it would require another
>> function in discovery to query what the IPA domain is from the given
>> master but it gets tricky if anonymous search is disabled, for example.
>>
>> rob
>>
>
> Needed or not, this is the behaviour that ipa-client-install has now.
> Adding a domain detection method would be a RFE for ipa-client-install
> (and imho not something we should be adding at this point).
>
> This patch only focuses on making the ipa-replica-install work more
> smoothly.

 I am just thinking that client promotion (ipa-replica-install) and
 ipa-client-install are a bit different use cases. While ipa-client-install
 should be typically run in auto-discovery and you thus do not use --server
 option much, while with ipa-replica-install, you want to make sure you have
 the
 expected topology and should use --server all the time without gambling on 
 it.

 But I do not think it has to be there since 4.3 GA, can you please file a
 ticket for this gap?
>>>
>>> I would rather do it now, because the change from optional to mandatory is
>>> backward incompatible. (We don't want to break users' scripts, right?)
>>
>> I think it is the other way around - with the change I was suggesting
>> (autodetecting --domain option instead of always requesting it, as in Tomas'
>> patch which we can merge if my proposal is not doable for 4.3 GA).
>>
> 
> "with ipa-replica-install, you want to make sure you have the expected 
> topology
> and should use --server all the time" sounds like you want to make --server
> mandatory for ipa-replica-install, which should be done either before 4.3 GA 
> or
> never.

Ah, no, this is not what I meant. I was only discussing the --domain option in
this mail the the typical use cases for --server option in ipa-client-install
and ipa-replica-install.

If we can trust ipa-replica-install to do a good job in picking a server to
replica from, the --server can stay optional. Although I am on fence here,
being more explicit when creating topology may be helpful. CCing Simo, in case
he has some opinions on this.

-- 
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 0065] ipa-replica-install prints incorrect error message when replica is already installed

[Martin Basti]

On 08.12.2015 16:14, Jan Cholasta wrote:

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford wrote:

Sorry guys, I forgot to add a meaningful subject to this message.
Ignore the previous thread start.

-- Forwarded message --
From: *Gabe Alford* >

Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel >


Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe




ACK


NACK, you can't install a server over an already installed client, 
thus the original check is correct.


Ahh domain level 0, right, but this check can be added before the client 
check.

With domain level 1, this check should stay there IMO.

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] First part of the replica promotion tests + testplan

[Oleg Fayans]

Hi all,


The patches are rebased against the current master.

On 12/02/2015 05:10 PM, Martin Basti wrote:
> 
> 
> 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
>

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

[Petr Spacek]

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

Hmm, my review script screams:

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

Please fix it :-)

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

-- 
Petr^2 Spacek
make lint is running ... make[1]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
Building IPA 4.2.90.201512081417GIT8d05032
~/pkg/ipa/git ~/pkg/ipa/git/install/po
install/po/tmp.pot.update: warning: Charset "CHARSET" is not a portable encoding name.
Message conversion to user's charset might not work.
~/pkg/ipa/git/install/po
tmp.pot updated
tmp.pot: 3112 entries, 3155 msgid, 0 msgstr, 0 warnings 0 errors
Detected base branch:   origin/master
Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore
Writing API to API.txt
./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation
./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent
./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation
./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent
./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1
./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent
./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1
./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters)
./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters)
./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent
./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters)
./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent
./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment
./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters)
ERROR: PEP8 --diff failed, continuing ...
ERROR: API.txt was changed without a change in VERSION, continuing ...
Summary of detected errors:
 ERROR: PEP8 --diff failed
 ERROR: API.txt was changed without a change in VERSION

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

- check if ACI.txt and API.txt are up-to-date
- check if VERSION was changed if API was changed
- pep8 --diff does not 

Re: [Freeipa-devel] [PATCH 0065] ipa-replica-install prints incorrect error message when replica is already installed

[Jan Cholasta]

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford wrote:

Sorry guys, I forgot to add a meaningful subject to this message.
Ignore the previous thread start.

-- Forwarded message --
From: *Gabe Alford* >
Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel >


Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe




ACK


NACK, you can't install a server over an already installed client, thus 
the original check is correct.


--
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 0388] tests: Add hostmask detection for sudo rules validating

[Tomas Babej]

On 12/03/2015 04:33 PM, Tomas Babej wrote:
> 
> 
> On 12/03/2015 04:26 PM, Aleš Mareček wrote:
>> Hello,
>>
>> ACK for code
>> NACK for the placing "get_client_ip_with_hostmask" function to test_sudo.py 
>> (this function should be in some more general file)
>>
> 
> What place would you propose? The task.py is not a good place, as this
> is not really a task.
> 
> Nevertheless, I'd rather have it moved when an use case outside
> test_sudo.py actually arises. Right now it would lead to unnecessary
> cluttering.
> 
> Tomas
> 

I re-discovered ipatests.test_integration.util (two years after I
created it :D) - which seemed ideal for this function.

Updated patch attached.

Tomas
From 33552d6078d75ee99f9ec19ae143df5a61ba84a4 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Wed, 2 Dec 2015 15:25:49 +0100
Subject: [PATCH] tests: Add hostmask detection for sudo rules validating on
 hostmask

IPA sudo tests worked under the assumption that the clients
that are executing the sudo commands have their IPs assigned
within 255.255.255.0 hostmask.

Removes this (invalid) assumption and adds a
dynamic detection of the hostmask of the IPA client.

https://fedorahosted.org/freeipa/ticket/5501
---
 ipatests/test_integration/test_sudo.py | 33 +++--
 ipatests/test_integration/util.py  | 16 
 2 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/ipatests/test_integration/test_sudo.py b/ipatests/test_integration/test_sudo.py
index 1dd4c5d73c9fa4288af4fc2708aa3abd51407217..fe0a7a3ea5f230053e8c00d21b36ee300acce4c7 100644
--- a/ipatests/test_integration/test_sudo.py
+++ b/ipatests/test_integration/test_sudo.py
@@ -17,8 +17,12 @@
 # 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
+from ipatests.test_integration import util
 
 
 class TestSudo(IntegrationTest):
@@ -269,13 +273,25 @@ class TestSudo(IntegrationTest):
  '--hostgroups', 'testhostgroup'])
 
 def test_sudo_rule_restricted_to_one_hostmask_setup(self):
-# Add the client's /24 hostmask to the rule
-ip = self.client.ip
+# We need to detect the hostmask first
+full_ip = util.get_host_ip_with_hostmask(self.client)
+
+# Make a note for the next test, which needs to be skipped
+# if hostmask detection failed
+self.__class__.skip_hostmask_based = False
+
+if not full_ip:
+self.__class__.skip_hostmask_based = True
+raise pytest.skip("Hostmask could not be detected")
+
 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
 
@@ -284,11 +300,16 @@ class TestSudo(IntegrationTest):
 assert result.returncode != 0
 
 def test_sudo_rule_restricted_to_one_hostmask_teardown(self):
-# Remove the client's /24 hostmask from the rule
-ip = self.client.ip
+if self.__class__.skip_hostmask_based:
+raise pytest.skip("Hostmask could not be detected")
+
+# Detect the hostmask first to delete the hostmask based rule
+full_ip = util.get_host_ip_with_hostmask(self.client)
+
+# Remove the client's hostmask from the rule
 self.master.run_command(['ipa', '-n', 'sudorule-remove-host',
  'testrule',
- '--hostmask', '%s/24' % ip])
+ '--hostmask', full_ip])
 
 def test_sudo_rule_restricted_to_one_hostmask_negative_setup(self):
 # Add the master's hostmask to the rule
diff --git a/ipatests/test_integration/util.py b/ipatests/test_integration/util.py
index 1a1bb3fcc923c9f2721f0a4c1cb7a1ba2ccc2dd8..187f39e80e84af0eb4938fb19ac3d3c7c2280ed9 100644
--- a/ipatests/test_integration/util.py
+++ b/ipatests/test_integration/util.py
@@ -58,3 +58,19 @@ def run_repeatedly(host, command, assert_zero_rc=True, test=None,
  .format(cmd=' '.join(command),
  times=timeout / time_step,
  timeout=timeout))
+
+
+def get_host_ip_with_hostmask(host):
+"""
+Detects the IP of the host including the hostmask.
+
+Returns None if the IP could not be detected.
+"""
+
+ip = host.ip
+result = host.run_command(['ip', 'addr'])
+

[Freeipa-devel] [PATCH 0071] replica: Fix ipa-replica-install with replica file (domain, level 0).

[David Kupka]

https://fedorahosted.org/freeipa/ticket/5531
--
David Kupka
From eee2c606aeba8aff61777cbf54fdb6c006e8c755 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Tue, 8 Dec 2015 14:22:01 +0100
Subject: [PATCH] replica: Fix ipa-replica-install with replica file (domain
 level 0).

https://fedorahosted.org/freeipa/ticket/5531
---
 ipaserver/install/server/replicainstall.py | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4554166752ce4e5db2a98a8f495aa061aec963e9..a962ef93442c201f9df80adfb0443ab37cf9dc59 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -654,6 +654,8 @@ def install(installer):
 if installer._update_hosts_file:
 installutils.update_hosts_file(config.ips, config.host_name, fstore)
 
+ca_enabled = ipautil.file_exists(config.dir + "/cacert.p12")
+
 # Create DS user/group if it doesn't exist yet
 dsinstance.create_ds_user()
 
@@ -675,7 +677,7 @@ def install(installer):
 ntp.create_instance()
 
 # Configure dirsrv
-ds = install_replica_ds(config, options, installer._ca_enabled)
+ds = install_replica_ds(config, options, ca_enabled)
 
 # Always try to install DNS records
 install_dns_records(config, options, remote_api)
@@ -690,20 +692,20 @@ def install(installer):
 options.domain_name = config.domain_name
 options.host_name = config.host_name
 
-if ipautil.file_exists(config.dir + "/cacert.p12"):
+if ca_enabled:
 options.ra_p12 = config.dir + "/ra.p12"
 
 ca.install(False, config, options)
 
 krb = install_krb(config, setup_pkinit=not options.no_pkinit)
 http = install_http(config, auto_redirect=not options.no_ui_redirect,
-ca_is_configured=installer._ca_enabled)
+ca_is_configured=ca_enabled)
 
 otpd = otpdinstance.OtpdInstance()
 otpd.create_instance('OTPD', config.host_name, config.dirman_password,
  ipautil.realm_to_suffix(config.realm_name))
 
-if ipautil.file_exists(config.dir + "/cacert.p12"):
+if ca_enabled:
 CA = cainstance.CAInstance(config.realm_name, certs.NSS_DIR)
 CA.dm_password = config.dirman_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 0071] replica: Fix ipa-replica-install with replica file (domain, level 0).

[Oleg Fayans]

ACK. The initial issue is fixed.

On 12/08/2015 03:03 PM, David Kupka wrote:
> https://fedorahosted.org/freeipa/ticket/5531
> 
> 

-- 
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 0065] ipa-replica-install prints incorrect error message when replica is already installed

[Gabe Alford]

Just to confirm:

if server is installed:
 Let's stop here and not do anything else

if domain level 0:
 check if client installed and stop here

Right?


On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta  wrote:

> On 8.12.2015 16:17, Martin Basti wrote:
>
>>
>>
>> On 08.12.2015 16:14, Jan Cholasta wrote:
>>
>>> On 8.12.2015 16:09, Martin Basti wrote:
>>>


 On 01.12.2015 14:57, Gabe Alford wrote:

> Sorry guys, I forgot to add a meaningful subject to this message.
> Ignore the previous thread start.
>
> -- Forwarded message --
> From: *Gabe Alford*  >
> Date: Mon, Nov 30, 2015 at 7:31 PM
> Subject: [PATCH 0065]
> To: freeipa-devel  >
>
>
> Hello,
>
> Patch fix for the following tickets:
>
> https://fedorahosted.org/freeipa/ticket/5022
> https://fedorahosted.org/freeipa/ticket/5320
>
> Thanks,
>
> Gabe
>
>
>
> ACK

>>>
>>> NACK, you can't install a server over an already installed client,
>>> thus the original check is correct.
>>>
>>> Ahh domain level 0, right, but this check can be added before the client
>> check.
>>
>
> Yes.
>
> With domain level 1, this check should stay there IMO.
>>
>
> Yes. It should say "IPA server is already configured" rather than "IPA
> replica is already configured", though.
>
> --
> Jan Cholasta
>
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0065] ipa-replica-install prints incorrect error message when replica is already installed

[Jan Cholasta]

On 8.12.2015 16:17, Martin Basti wrote:



On 08.12.2015 16:14, Jan Cholasta wrote:

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford wrote:

Sorry guys, I forgot to add a meaningful subject to this message.
Ignore the previous thread start.

-- Forwarded message --
From: *Gabe Alford* >
Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel >


Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe




ACK


NACK, you can't install a server over an already installed client,
thus the original check is correct.


Ahh domain level 0, right, but this check can be added before the client
check.


Yes.


With domain level 1, this check should stay there IMO.


Yes. It should say "IPA server is already configured" rather than "IPA 
replica is already configured", though.


--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 0065] ipa-replica-install prints incorrect error message when replica is already installed

[Martin Basti]

On 08.12.2015 16:26, Gabe Alford wrote:

Just to confirm:

if server is installed:
 Let's stop here and not do anything else

if domain level 0:
 check if client installed and stop here

Right?

yes




On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta > wrote:


On 8.12.2015 16:17, Martin Basti wrote:



On 08.12.2015 16:14, Jan Cholasta wrote:

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford wrote:

Sorry guys, I forgot to add a meaningful subject
to this message.
Ignore the previous thread start.

-- Forwarded message --
From: *Gabe Alford* 
>>
Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel 
>>


Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe



ACK


NACK, you can't install a server over an already installed
client,
thus the original check is correct.

Ahh domain level 0, right, but this check can be added before
the client
check.


Yes.

With domain level 1, this check should stay there IMO.


Yes. It should say "IPA server is already configured" rather than
"IPA replica is already configured", though.

-- 
Jan Cholasta





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

Re: [Freeipa-devel] [PATCH 0065] ipa-replica-install prints incorrect error message when replica is already installed

[Martin Basti]

On 01.12.2015 14:57, Gabe Alford wrote:
Sorry guys, I forgot to add a meaningful subject to this message. 
Ignore the previous thread start.


-- Forwarded message --
From: *Gabe Alford* >
Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel >



Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe




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 0394] topology: Make sure the old 'realm' topology suffix is not

[Tomas Babej]

On 12/08/2015 02:28 PM, Tomas Babej wrote:
> Hi,
> 
> The old 'realm' topology suffix is no longer used, however, it was being
> created on masters with version 4.2.3 and later. Make sure it's properly
> removed.
> 
> Note that this is not the case for the 'ipaca' suffix, which was later
> removed to 'ca'.
> 
> https://fedorahosted.org/freeipa/ticket/5526
> 

Actually, we found out with Martin that this patch deletes realm instead
of domain suffix, against all initial impressions.

Updated patch attached.

Tomas
From 669f741f8cc20772b84f5980b9b6b57f71e3b992 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Tue, 8 Dec 2015 13:34:15 +0100
Subject: [PATCH] topology: Make sure the old 'realm' topology suffix is not
 used

The old 'realm' topology suffix is no longer used, howver, it was being
created on masters with version 4.2.3 and later. Make sure it's properly
removed.

Note that this is not the case for the 'ipaca' suffix, whic was later
removed to 'ca'.

https://fedorahosted.org/freeipa/ticket/5526
---
 install/updates/20-replication.update | 4 
 1 file changed, 4 insertions(+)

diff --git a/install/updates/20-replication.update b/install/updates/20-replication.update
index a471742532cf5954be1b76dbe4a6d908e4cefa2c..c9d96066d5f9bec5b8b92a3f2c457636c095137a 100644
--- a/install/updates/20-replication.update
+++ b/install/updates/20-replication.update
@@ -31,6 +31,10 @@ add: nsDS5ReplicatedAttributeList: $EXCLUDES
 add: nsDS5ReplicatedAttributeListTotal: $TOTAL_EXCLUDES
 add: nsds5ReplicaStripAttrs: $STRIP_ATTRS
 
+# Remove old topology configuration area (unused)
+dn: cn=realm,cn=topology,cn=ipa,cn=etc,$SUFFIX
+deleteentry: cn=realm,cn=topology,cn=ipa,cn=etc,$SUFFIX
+
 # add IPA realm managed suffix to master entry
 dn: cn=$FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX
 add: objectclass: ipaReplTopoManagedServer
-- 
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 0065] ipa-replica-install prints incorrect error message when replica is already installed

[Gabe Alford]

Updated patch attached.

On Tue, Dec 8, 2015 at 8:27 AM, Martin Basti  wrote:

>
>
> On 08.12.2015 16:26, Gabe Alford wrote:
>
> Just to confirm:
>
> if server is installed:
>  Let's stop here and not do anything else
>
> if domain level 0:
>  check if client installed and stop here
>
> Right?
>
> yes
>
>
>
>
> On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta  wrote:
>
>> On 8.12.2015 16:17, Martin Basti wrote:
>>
>>>
>>>
>>> On 08.12.2015 16:14, Jan Cholasta wrote:
>>>
 On 8.12.2015 16:09, Martin Basti wrote:

>
>
> On 01.12.2015 14:57, Gabe Alford wrote:
>
>> Sorry guys, I forgot to add a meaningful subject to this message.
>> Ignore the previous thread start.
>>
>> -- Forwarded message --
>> From: *Gabe Alford* > >
>> Date: Mon, Nov 30, 2015 at 7:31 PM
>> Subject: [PATCH 0065]
>> To: freeipa-devel > >
>>
>>
>> Hello,
>>
>> Patch fix for the following tickets:
>>
>> https://fedorahosted.org/freeipa/ticket/5022
>> https://fedorahosted.org/freeipa/ticket/5320
>>
>> Thanks,
>>
>> Gabe
>>
>>
>>
>> ACK
>

 NACK, you can't install a server over an already installed client,
 thus the original check is correct.

 Ahh domain level 0, right, but this check can be added before the client
>>> check.
>>>
>>
>> Yes.
>>
>> With domain level 1, this check should stay there IMO.
>>>
>>
>> Yes. It should say "IPA server is already configured" rather than "IPA
>> replica is already configured", though.
>>
>> --
>> Jan Cholasta
>>
>
>
>
From 340a1316d8a71a4a3d7246fa87d2307f34484776 Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Tue, 8 Dec 2015 08:58:56 -0700
Subject: [PATCH] ipa-replica-install prints incorrect error message when
 replica is already installed

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320
---
 ipaserver/install/server/replicainstall.py | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4554166752ce4e5db2a98a8f495aa061aec963e9..e3f061a171e48f060464ef8e32630c8ca394c0b8 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -31,9 +31,8 @@ from ipaserver.install import (
 bindinstance, ca, cainstance, certs, dns, dsinstance, httpinstance,
 installutils, kra, krainstance, krbinstance, memcacheinstance,
 ntpinstance, otpdinstance, custodiainstance, service)
-from ipaserver.install.installutils import create_replica_config
-from ipaserver.install.installutils import ReplicaConfig
-from ipaserver.install.installutils import load_pkcs12
+from ipaserver.install.installutils import (
+create_replica_config, ReplicaConfig, load_pkcs12, is_ipa_configured)
 from ipaserver.install.replication import (
 ReplicationManager, replica_conn_check)
 import SSSDConfig
@@ -423,6 +422,11 @@ def install_check(installer):
 
 tasks.check_selinux_status()
 
+if is_ipa_configured():
+sys.exit("IPA server is already configured on this system.\n"
+ "If you want to reinstall the IPA server, please uninstall "
+ "it first using 'ipa-server-install --uninstall'.")
+
 client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
 if client_fstore.has_files():
 sys.exit("IPA client is already configured on this system.\n"
@@ -828,6 +832,11 @@ def promote_check(installer):
 
 tasks.check_selinux_status()
 
+if is_ipa_configured():
+sys.exit("IPA server is already configured on this system.\n"
+ "If you want to reinstall the IPA server, please uninstall "
+ "it first using 'ipa-server-install --uninstall'."
+
 client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
 if not client_fstore.has_files():
 ensure_enrolled(installer)
-- 
1.8.3.1

-- 
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 0114] harden domain level 1 topology connectivity checks

[Martin Babinsky]

A sort of auxilliary patch which makes topology checks more resistant to 
https://fedorahosted.org/freeipa/ticket/5526


If required I will open a separate ticket for it though.

--
Martin^3 Babinsky
From 6b722203ba9442559b1311be63b8b05b862af084 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Tue, 8 Dec 2015 17:24:36 +0100
Subject: [PATCH] harden domain level 1 topology connectivity checks

this patch makes the check_last_link_managed() function more resistant to both
orphaned topology suffixes and also to cases when there are IPA masters do not
seem to manage any suffix. The function will now only complain loudly about
these cases and not cause crashes.

https://fedorahosted.org/freeipa/ticket/5526
---
 install/tools/ipa-replica-manage |  6 ++
 ipaserver/install/replication.py | 41 
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 5124731255807287a7857b1a4cdc2e6799cd7278..81a133192b17bd2de45aabccec279c9b0c8b2fb8 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -785,11 +785,9 @@ def check_deleted_segments(hostname, masters, topo_errors, starting_host):
 hostname))
 return
 
-suffixes = api.Command.topologysuffix_find('', sizelimit=0)['result']
-suffix_to_masters = replication.map_masters_to_suffixes(masters, suffixes)
+suffix_to_masters = replication.map_masters_to_suffixes(masters)
 
-for suffix in suffixes:
-suffix_name = suffix['cn'][0]
+for suffix_name in suffix_to_masters:
 suffix_member_cns = [
 m['cn'][0] for m in suffix_to_masters[suffix_name]
 ]
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index aaa841ca6ac415f90d71a3126f18a787500d7e6a..522e93fa2c751c18064004d30059e67fba0e9294 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -1740,11 +1740,16 @@ class CAReplicationManager(ReplicationManager):
 raise RuntimeError("Failed to start replication")
 
 
-def map_masters_to_suffixes(masters, suffixes):
+def map_masters_to_suffixes(masters):
 masters_to_suffix = {}
 
 for master in masters:
-managed_suffixes = master['iparepltopomanagedsuffix_topologysuffix']
+try:
+managed_suffixes = master['iparepltopomanagedsuffix_topologysuffix']
+except KeyError:
+print("IPA master {0} does not manage any suffix")
+continue
+
 for suffix_name in managed_suffixes:
 try:
 masters_to_suffix[suffix_name].append(master)
@@ -1759,6 +1764,19 @@ def check_hostname_in_masters(hostname, masters):
 return hostname in master_cns
 
 
+def get_orphaned_suffixes(masters):
+"""
+:param masters: result of server_find command
+:return a set consisting of suffix names which are not managed by any
+master
+"""
+all_suffixes = api.Command.topologysuffix_find(sizelimit=0)['result']
+all_suffix_names = set(s['cn'][0] for s in all_suffixes)
+managed_suffixes = set(map_masters_to_suffixes(masters))
+
+return all_suffix_names ^ managed_suffixes
+
+
 def check_last_link_managed(api, hostname, masters):
 """
 Check if 'hostname' is safe to delete.
@@ -1767,16 +1785,23 @@ def check_last_link_managed(api, hostname, masters):
   {: (,
   )}
 """
-suffixes = api.Command.topologysuffix_find(sizelimit=0)['result']
-suffix_to_masters = map_masters_to_suffixes(masters, suffixes)
+suffix_to_masters = map_masters_to_suffixes(masters)
 topo_errors_by_suffix = {}
 
-for suffix in suffixes:
-suffix_name = suffix['cn'][0]
-suffix_members = suffix_to_masters[suffix_name]
+# sanity check for orphaned suffixes
+orphaned_suffixes = get_orphaned_suffixes(masters)
+if orphaned_suffixes:
+print("The following suffixes are not managed by any IPA master:")
+print("  {0}".format(
+', '.join(sorted(orphaned_suffixes))
+)
+)
+
+for suffix_name in suffix_to_masters:
 print("Checking connectivity in topology suffix '{0}'".format(
 suffix_name))
-if not check_hostname_in_masters(hostname, suffix_members):
+if not check_hostname_in_masters(hostname,
+ suffix_to_masters[suffix_name]):
 print(
 "'{0}' is not a part of topology suffix '{1}'".format(
 hostname, suffix_name
-- 
2.5.0

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

[Freeipa-devel] [PATCH 0115] fix error message assertion in negative forced client reenrollment tests

[Martin Babinsky]

This patch fixes the assertions in negative test cases of 
'test_forced_client_reenrollment' CI test suite.


On ipa-4-2 it fixes https://fedorahosted.org/freeipa/ticket/5511 and 
makes all 8 tests in this suite green, shiny and happy again.


It also fixes negative test cases on master branch, but the positive 
cases are broken there due to https://fedorahosted.org/freeipa/ticket/5528


--
Martin^3 Babinsky
From eb152f6996a8b653d8676ade826e806898fdf556 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Tue, 8 Dec 2015 17:00:11 +0100
Subject: [PATCH] fix error message assertion in negative forced client
 reenrollment tests

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

---
 ipatests/test_integration/test_forced_client_reenrollment.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipatests/test_integration/test_forced_client_reenrollment.py b/ipatests/test_integration/test_forced_client_reenrollment.py
index e1edff9b7f2d535a341d1e8ca55917012943818e..df0e90526c5c8dd78d3af9d7ddb7c9bdbf5a2268 100644
--- a/ipatests/test_integration/test_forced_client_reenrollment.py
+++ b/ipatests/test_integration/test_forced_client_reenrollment.py
@@ -198,7 +198,7 @@ class TestForcedClientReenrollment(IntegrationTest):
 assert 'IPA Server: %s' % server.hostname in result.stderr_text
 
 if expect_fail:
-err_msg = 'Kerberos authentication failed using keytab'
+err_msg = "Kerberos authentication failed: "
 assert result.returncode == 1
 assert err_msg in result.stderr_text
 elif force_join and keytab:
-- 
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 0071] replica: Fix ipa-replica-install with replica file (domain, level 0).

[Tomas Babej]

On 12/08/2015 04:20 PM, Oleg Fayans wrote:
> ACK. The initial issue is fixed.
> 
> On 12/08/2015 03:03 PM, David Kupka wrote:
>> https://fedorahosted.org/freeipa/ticket/5531
>>
>>
> 

Can we get some more love for the patch and provide at least a sentence
worth of commit message before pushing?

It's not obvious from the title what the patch does, other than it fixes
things.

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 0394] topology: Make sure the old 'realm' topology suffix is not

[Martin Babinsky]

On 12/08/2015 04:53 PM, Tomas Babej wrote:



On 12/08/2015 02:28 PM, Tomas Babej wrote:

Hi,

The old 'realm' topology suffix is no longer used, however, it was being
created on masters with version 4.2.3 and later. Make sure it's properly
removed.

Note that this is not the case for the 'ipaca' suffix, which was later
removed to 'ca'.

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



Actually, we found out with Martin that this patch deletes realm instead
of domain suffix, against all initial impressions.

Updated patch attached.

Tomas





Works for me, ACK.

I have also made some hardening in topology connectivity checks so that 
this kind of situations is handled and reported by them. I will send a 
patch in separate thread.


--
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] First part of the replica promotion tests + testplan

[Oleg Fayans]

Substituted a hardcoded suffix name with a constant DOMAIN_SUFFIX_NAME

On 12/08/2015 02:33 PM, Oleg Fayans wrote:
> Hi all,
> 
> 
> The patches are rebased against the current master.
> 
> On 12/02/2015 05:10 PM, Martin Basti wrote:
>>
>>
>> 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
> 

Re: [Freeipa-devel] [PATCH 0065] ipa-replica-install prints incorrect error message when replica is already installed

[Jan Cholasta]

LGTM

On 8.12.2015 17:04, Gabe Alford wrote:

Updated patch attached.

On Tue, Dec 8, 2015 at 8:27 AM, Martin Basti > wrote:



On 08.12.2015 16:26, Gabe Alford wrote:

Just to confirm:

if server is installed:
 Let's stop here and not do anything else

if domain level 0:
 check if client installed and stop here

Right?

yes





On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta > wrote:

On 8.12.2015 16:17, Martin Basti wrote:



On 08.12.2015 16:14, Jan Cholasta wrote:

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford wrote:

Sorry guys, I forgot to add a meaningful
subject to this message.
Ignore the previous thread start.

-- Forwarded message --
From: *Gabe Alford* 
>>
Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel 
>>


Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe



ACK


NACK, you can't install a server over an already
installed client,
thus the original check is correct.

Ahh domain level 0, right, but this check can be added
before the client
check.


Yes.

With domain level 1, this check should stay there IMO.


Yes. It should say "IPA server is already configured" rather
than "IPA replica is already configured", though.

--
Jan Cholasta








--
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 0071] replica: Fix ipa-replica-install with replica file (domain, level 0).

[David Kupka]

On 08/12/15 16:33, Tomas Babej wrote:



On 12/08/2015 04:20 PM, Oleg Fayans wrote:

ACK. The initial issue is fixed.

On 12/08/2015 03:03 PM, David Kupka wrote:

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






Can we get some more love for the patch and provide at least a sentence
worth of commit message before pushing?

It's not obvious from the title what the patch does, other than it fixes
things.

Tomas

I believe it's pretty obvious from linked ticket and attached patch 
changing just 5 lines.
But you're right verbosity in commit message could save time later. 
Patch with changed commit message attached.


--
David Kupka
From eee2c606aeba8aff61777cbf54fdb6c006e8c755 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Tue, 8 Dec 2015 14:22:01 +0100
Subject: [PATCH] replica: Fix ipa-replica-install with replica file (domain
 level 0).

Attribute _ca_enabled is set in promote_check() and is not available in
install(). When installing replica in domain level 0 we can determine existence
of CA service based on existence of cacert.p12 file in provided replica-file.

https://fedorahosted.org/freeipa/ticket/5531
---
 ipaserver/install/server/replicainstall.py | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4554166752ce4e5db2a98a8f495aa061aec963e9..a962ef93442c201f9df80adfb0443ab37cf9dc59 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -654,6 +654,8 @@ def install(installer):
 if installer._update_hosts_file:
 installutils.update_hosts_file(config.ips, config.host_name, fstore)
 
+ca_enabled = ipautil.file_exists(config.dir + "/cacert.p12")
+
 # Create DS user/group if it doesn't exist yet
 dsinstance.create_ds_user()
 
@@ -675,7 +677,7 @@ def install(installer):
 ntp.create_instance()
 
 # Configure dirsrv
-ds = install_replica_ds(config, options, installer._ca_enabled)
+ds = install_replica_ds(config, options, ca_enabled)
 
 # Always try to install DNS records
 install_dns_records(config, options, remote_api)
@@ -690,20 +692,20 @@ def install(installer):
 options.domain_name = config.domain_name
 options.host_name = config.host_name
 
-if ipautil.file_exists(config.dir + "/cacert.p12"):
+if ca_enabled:
 options.ra_p12 = config.dir + "/ra.p12"
 
 ca.install(False, config, options)
 
 krb = install_krb(config, setup_pkinit=not options.no_pkinit)
 http = install_http(config, auto_redirect=not options.no_ui_redirect,
-ca_is_configured=installer._ca_enabled)
+ca_is_configured=ca_enabled)
 
 otpd = otpdinstance.OtpdInstance()
 otpd.create_instance('OTPD', config.host_name, config.dirman_password,
  ipautil.realm_to_suffix(config.realm_name))
 
-if ipautil.file_exists(config.dir + "/cacert.p12"):
+if ca_enabled:
 CA = cainstance.CAInstance(config.realm_name, certs.NSS_DIR)
 CA.dm_password = config.dirman_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 0112] CI tests: ignore disconnected domain level 1 topology on IPA master teardown

[Martin Babinsky]

On 12/07/2015 01:53 PM, Martin Babinsky wrote:

On 12/07/2015 12:07 PM, Oleg Fayans wrote:

Hi Martin,

CONFIGURED_DOMAIN_LEVEL is declared, but not used. The rest looks fine
to me

On 12/07/2015 11:05 AM, Martin Babinsky wrote:

This patch should fix teardown methods in replication-related CI tests
ran at non-zero domain level.






Ah that was a leftover from previous implementation. Here's updated patch.




Patch needed a rebase. Attaching new revision.

--
Martin^3 Babinsky
From bf1659f9ff03983740a1e786c78568b7851e7a02 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 4 Dec 2015 18:24:31 +0100
Subject: [PATCH] CI tests: ignore disconnected domain level 1 topology on IPA
 master teardown

---
 ipatests/test_integration/tasks.py | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index ddbb5af29ce7b36fe178294f45a72cf941730918..c3681fca952807ac6ebcca56ce961df2d3f33f0c 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -343,7 +343,7 @@ def install_replica(master, replica, setup_ca=True, setup_dns=False,
 '--setup-dns',
 '--forwarder', replica.config.dns_forwarder
 ])
-if domainlevel(master) == 0:
+if domainlevel(master) == DOMAIN_LEVEL_0:
 apply_common_fixes(replica)
 # prepare the replica file on master and put it to replica, AKA "old way"
 replica_prepare(master, replica)
@@ -365,7 +365,7 @@ def install_replica(master, replica, setup_ca=True, setup_dns=False,
 "-p", replica.config.dirman_password,
 "-U",
 ]
-if domainlevel(master) == 0:
+if domainlevel(master) == DOMAIN_LEVEL_0:
 args.append(replica_filename)
 replica.run_command(args)
 
@@ -615,11 +615,16 @@ def kinit_admin(host):
  stdin_text=host.config.admin_password)
 
 
-def uninstall_master(host):
+def uninstall_master(host, ignore_topology_disconnect=True):
 host.collect_log(paths.IPASERVER_UNINSTALL_LOG)
+uninstall_cmd = ['ipa-server-install', '--uninstall', '-U']
 
-host.run_command(['ipa-server-install', '--uninstall', '-U'],
- raiseonerr=False)
+host_domain_level = domainlevel(host)
+
+if ignore_topology_disconnect and host_domain_level != DOMAIN_LEVEL_0:
+uninstall_cmd.append('--ignore-topology-disconnect')
+
+host.run_command(uninstall_cmd, raiseonerr=False)
 host.run_command(['pkidestroy', '-s', 'CA', '-i', 'pki-tomcat'],
  raiseonerr=False)
 host.run_command(['rm', '-rf',
-- 
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 0112] CI tests: ignore disconnected domain level 1 topology on IPA master teardown

[Oleg Fayans]

ACK

On 12/09/2015 07:37 AM, Martin Babinsky wrote:
> On 12/07/2015 01:53 PM, Martin Babinsky wrote:
>> On 12/07/2015 12:07 PM, Oleg Fayans wrote:
>>> Hi Martin,
>>>
>>> CONFIGURED_DOMAIN_LEVEL is declared, but not used. The rest looks fine
>>> to me
>>>
>>> On 12/07/2015 11:05 AM, Martin Babinsky wrote:
 This patch should fix teardown methods in replication-related CI tests
 ran at non-zero domain level.



>>>
>> Ah that was a leftover from previous implementation. Here's updated
>> patch.
>>
>>
>>
> Patch needed a rebase. Attaching new revision.
> 

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