Re: [Freeipa-devel] [PATCH 0059] Add Firefox options to ipa-client-install man page

[Martin Basti]

On 29.10.2015 13:28, Gabe Alford wrote:

Hello,

Fix for https://fedorahosted.org/freeipa/ticket/5375

Thanks,

Gabe



Thank you!

ACK

Pushed to master: cc5a659d4304873d6f89c47a11877026cd442199
-- 
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 0327] KRA: fix check if CA is installed on replica

[Martin Basti]

On 27.10.2015 19:00, Martin Babinsky wrote:

On 10/27/2015 10:43 AM, Martin Basti wrote:



On 26.10.2015 10:35, Martin Babinsky wrote:

On 10/23/2015 05:18 PM, Martin Basti wrote:



On 23.10.2015 15:15, Martin Babinsky wrote:

On 10/23/2015 03:12 PM, Martin Babinsky wrote:

On 10/16/2015 12:41 PM, Martin Basti wrote:

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

Patch attached.



I have tested it on 4-2 branch and it works as expected, ACK.

Obviously, master branch would require a different patch.



I actually missed your check in ipaserver/install/kra.py which can
break ipa-replica-install with --setup-kra, so NACK.


Updated patches attached.


NACK on the master-branch patch.

You forgot a 'return' in this code snippet:

+if self.installing_replica:
+domain_level = dsinstance.get_domain_level(api)
+if domain_level > DOMAIN_LEVEL_0:
+self.options.promote = True
+return

that would make installation abort when domain level is greter than 
zero.



Updated patch attached.

ACK.


Pushed to ipa-4-2: 0f77745c5033ee321447a86a0499865c3c4b29e4
Pushed to master: 4ec8df27392b4f47c03c2cded26d6695d8c38186

--
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 0092] ipa-replica-prepare: more robust and concise check for supported domain level

[Tomas Babej]

On 10/29/2015 01:10 PM, Petr Vobornik wrote:
> On 10/29/2015 11:19 AM, Martin Babinsky wrote:
>> Petr^2 and Tomas were not happy by the way
>> https://fedorahosted.org/freeipa/ticket/5175 was handled initially, so
>> here is a patch that tries to amend some of the issues.
>>
>>
>>
> 
> IMHO the original text was good.
> 
> Tomas, why is the huge blob thing in exception bad?
> 
> Issues with new code/text.
> 
> 1. it is supported but not for domain level != 0:
> self.log.error("Using replica files to set up IPA replicas is not "
> +   "supported."
> +)
> 
> 2. format() is not needed:
> +self.log.info(
> +"To create a replica, you must promote an existing "
> +"IPA client.".format(domain_level=domain_level)
> +)
> 
> 3. I don't like that the exception text says 'requires', which might
> imply something to be done - lower domain level - which is not possible.
> "allowed only" might be better.
> 
> 
> Just changing RuntimeError to InvalidDomainLevelError would be fine with
> me since the MIN_DOMAIN_LEVEL was already changed to DOMAIN_LEVEL_0.


I was fine with the old text too. In my opinion exceptions should not be
used to handle detailed instructions to the user, they should be used to
briefly summarize the gist of the error that occurred.

If not bad practice, it's at least quite unconventional. There are
better mechanisms to handle the detailed instructions spanning over
multiple lines (with newlines hardcoded) down to the user, such as using
the logger with sufficient level.

So I would approach this issue by just dumping the formatted
UNSUPPORTED_DOMAIN_LEVEL_TEMPLATE down the logger at the error level and
then raising the InvalidDomainLevelError exception with the brief reasoning.

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 0092] ipa-replica-prepare: more robust and concise check for supported domain level

[Martin Babinsky]

On 10/29/2015 01:28 PM, Tomas Babej wrote:



On 10/29/2015 01:10 PM, Petr Vobornik wrote:

On 10/29/2015 11:19 AM, Martin Babinsky wrote:

Petr^2 and Tomas were not happy by the way
https://fedorahosted.org/freeipa/ticket/5175 was handled initially, so
here is a patch that tries to amend some of the issues.





IMHO the original text was good.

Tomas, why is the huge blob thing in exception bad?

Issues with new code/text.

1. it is supported but not for domain level != 0:
self.log.error("Using replica files to set up IPA replicas is not "
+   "supported."
+)

2. format() is not needed:
+self.log.info(
+"To create a replica, you must promote an existing "
+"IPA client.".format(domain_level=domain_level)
+)

3. I don't like that the exception text says 'requires', which might
imply something to be done - lower domain level - which is not possible.
"allowed only" might be better.


Just changing RuntimeError to InvalidDomainLevelError would be fine with
me since the MIN_DOMAIN_LEVEL was already changed to DOMAIN_LEVEL_0.



I was fine with the old text too. In my opinion exceptions should not be
used to handle detailed instructions to the user, they should be used to
briefly summarize the gist of the error that occurred.

If not bad practice, it's at least quite unconventional. There are
better mechanisms to handle the detailed instructions spanning over
multiple lines (with newlines hardcoded) down to the user, such as using
the logger with sufficient level.

So I would approach this issue by just dumping the formatted
UNSUPPORTED_DOMAIN_LEVEL_TEMPLATE down the logger at the error level and
then raising the InvalidDomainLevelError exception with the brief reasoning.

Tomas


OK i seemed to misunderstand your concerns. Attaching updated patch.

--
Martin^3 Babinsky
From d6ac6712f78daeeb0a2b1a6eea518692d23ae9e9 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Thu, 29 Oct 2015 14:53:25 +0100
Subject: [PATCH] ipa-replica-prepare: domain level check improvements

ipa-replica-prepare command is disabled in non-zero domain-level. Instead of
raising and exception with the whole message instructing the user to promote
replicas from enrolled clients in level 1+ topologies, the exception itself
contains only a brief informative message and the rest is logged at error
level.

https://fedorahosted.org/freeipa/ticket/5175
---
 ipaserver/install/ipa_replica_prepare.py | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/ipaserver/install/ipa_replica_prepare.py b/ipaserver/install/ipa_replica_prepare.py
index 8998bc094e22ba9a95d528b09f73b2884d78462f..327deed772561f2b8403aa46cdd3398055703840 100644
--- a/ipaserver/install/ipa_replica_prepare.py
+++ b/ipaserver/install/ipa_replica_prepare.py
@@ -175,7 +175,7 @@ class ReplicaPrepare(admintool.AdminTool):
 api.bootstrap(in_server=True)
 api.finalize()
 
-self.check_domainlevel(api)
+self.check_for_supported_domain_level()
 
 if api.env.host == self.replica_fqdn:
 raise admintool.ScriptError("You can't create a replica on itself")
@@ -690,12 +690,25 @@ class ReplicaPrepare(admintool.AdminTool):
 '-o', ca_file
 ])
 
-def check_domainlevel(self, api):
+def check_for_supported_domain_level(self):
+"""
+check if we are in 0-level topology. If not, raise an error pointing
+the user to the replica promotion pathway
+"""
+
 domain_level = dsinstance.get_domain_level(api)
 if domain_level > DOMAIN_LEVEL_0:
-raise RuntimeError(
+self.log.error(
 UNSUPPORTED_DOMAIN_LEVEL_TEMPLATE.format(
 command_name=self.command_name,
 domain_level=DOMAIN_LEVEL_0,
-curr_domain_level=domain_level)
+curr_domain_level=domain_level
+)
+)
+raise errors.InvalidDomainLevelError(
+reason="'{command}' is allowed only in domain level "
+"{prep_domain_level}".format(
+command=self.command_name,
+prep_domain_level=DOMAIN_LEVEL_0
+)
 )
-- 
2.4.3

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

Re: [Freeipa-devel] [PATCH 0092] ipa-replica-prepare: more robust and concise check for supported domain level

[Petr Vobornik]

On 10/29/2015 11:19 AM, Martin Babinsky wrote:

Petr^2 and Tomas were not happy by the way
https://fedorahosted.org/freeipa/ticket/5175 was handled initially, so
here is a patch that tries to amend some of the issues.





IMHO the original text was good.

Tomas, why is the huge blob thing in exception bad?

Issues with new code/text.

1. it is supported but not for domain level != 0:
self.log.error("Using replica files to set up IPA replicas is not "
+   "supported."
+)

2. format() is not needed:
+self.log.info(
+"To create a replica, you must promote an existing "
+"IPA client.".format(domain_level=domain_level)
+)

3. I don't like that the exception text says 'requires', which might 
imply something to be done - lower domain level - which is not possible. 
"allowed only" might be better.



Just changing RuntimeError to InvalidDomainLevelError would be fine with 
me since the MIN_DOMAIN_LEVEL was already changed to DOMAIN_LEVEL_0.

--
Petr Vobornik

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

Re: [Freeipa-devel] [PATCH 0020-0021] some topology plugin fixes

[thierry bordaz]

On 10/23/2015 10:44 AM, Ludwig Krispenz wrote:

Hi,
the attached two patches address issues I found when testing ca 
management in the topology plugin


Thanks for review,
Ludwig



Hi Ludwig,

Patch 20 is good to me. I have one remark, you call 
ipa_topo_cfg_host_find with lock flag. So that the replica config is not 
updated during the test.
Now the lock protects each call separately. The risk is very low that 
the target host could become unmanaged by the time we test the source host.

ACK.

Patch 21 is also good. Just in ipa_topo_util_init_hosts, why not calling 
ipa_topo_cfg_host_add to not duplicate the source ?


thanks
thierry
-- 
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] interactive installer does not ignore leading/trailing whitespace

[Gabe Alford]

My bad Martin^2. Here is an updated patch.

Gabe

On Thu, Oct 29, 2015 at 7:14 AM, Martin Basti  wrote:

>
>
> On 28.10.2015 02:35, Gabe Alford wrote:
>
> Hello,
>
> Fix for https://fedorahosted.org/freeipa/ticket/5355
>
> Thanks,
>
> Gabe
>
>
> Thank you Gabe, but patch needs more work to be complete:
>
> Bool and integer choices also need to strip whitespaces, see bellow:
>
> Do you want to configure DNS forwarders? [yes]:   no
> Do you want to configure DNS forwarders? [yes]:   no
> Do you want to configure DNS forwarders? [yes]:   no
> Do you want to configure DNS forwarders? [yes]: no
> No DNS forwarders configured
>
> Martin^2
>
>
From f72f14b973d91689e5d139e6cc9e7ed5e5d5a2d6 Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Thu, 29 Oct 2015 07:37:36 -0600
Subject: [PATCH] interactive installer does not ignore leading/trailing
 whitespace

https://fedorahosted.org/freeipa/ticket/5355
---
 ipapython/ipautil.py | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index b6fd11338f5f55402d5e4502297866f3b0cc0534..4acdd1a98818bf311a8fef103e7219cc62a28ec1 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -763,7 +763,7 @@ def user_input(prompt, default = None, allow_empty = True):
 try:
 ret = input("%s: " % prompt)
 if allow_empty or ret.strip():
-return ret
+return ret.strip()
 except EOFError:
 if allow_empty:
 return ''
@@ -776,7 +776,7 @@ def user_input(prompt, default = None, allow_empty = True):
 if not ret and (allow_empty or default):
 return default
 elif ret.strip():
-return ret
+return ret.strip()
 except EOFError:
 return default
 
@@ -785,6 +785,7 @@ def user_input(prompt, default = None, allow_empty = True):
 while True:
 try:
 ret = input("%s [%s]: " % (prompt, choice))
+ret = ret.strip()
 if not ret:
 return default
 elif ret.lower()[0] == "y":
@@ -798,6 +799,7 @@ def user_input(prompt, default = None, allow_empty = True):
 while True:
 try:
 ret = input("%s [%s]: " % (prompt, default))
+ret = ret.strip()
 if not ret:
 return default
 ret = int(ret)
-- 
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 0059] Add Firefox options to ipa-client-install man page

[Gabe Alford]

Hello,

Fix for https://fedorahosted.org/freeipa/ticket/5375

Thanks,

Gabe
From 4e0dba6b17f78aa7dd631780cbfe7c4bfa9edea4 Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Wed, 28 Oct 2015 17:39:40 -0600
Subject: [PATCH] Add Firefox options to ipa-client-install man page

- Update --configure-firefox description in ipa-client-install

https://fedorahosted.org/freeipa/ticket/5375
---
 ipa-client/ipa-install/ipa-client-install | 2 +-
 ipa-client/man/ipa-client-install.1   | 6 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index e38a0f2f970087791b18fff3137bdb1bc9ac2470..14261e57f1fbc01ea57eb7e8160f9c8bf9d282f8 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -182,7 +182,7 @@ def parse_options():
help="Automount location")
 basic_group.add_option("--configure-firefox", dest="configure_firefox",
 action="store_true", default=False,
-help="configure Firefox")
+help="configure Firefox to use IPA domain credentials")
 basic_group.add_option("--firefox-dir", dest="firefox_dir", default=None,
 help="specify directory where Firefox is installed (for example: '/usr/lib/firefox')")
 basic_group.add_option("--ip-address", dest="ip_addresses", default=[],
diff --git a/ipa-client/man/ipa-client-install.1 b/ipa-client/man/ipa-client-install.1
index cdcc56fee6ce82e0fe00048d52b13d27e8fe3450..494fd4952e130bbe31a717522ec3279c49904a87 100644
--- a/ipa-client/man/ipa-client-install.1
+++ b/ipa-client/man/ipa-client-install.1
@@ -181,6 +181,12 @@ Request certificate for the machine. The certificate will be stored in /etc/ipa/
 Configure automount by running ipa\-client\-automount(1) with \fILOCATION\fR as
 automount location.
 .TP
+\fB\-\-configure\-firefox\fR
+Configure Firefox to use IPA domain credentials.
+.TP
+\fB\-\-firefox\-dir\fR=\fIDIR\fR
+Specify Firefox installation directory. For example: '/usr/lib/firefox'
+.TP
 \fB\-\-ip\-address\fR=\fIIP_ADDRESS\fR
 Use \fIIP_ADDRESS\fR in DNS A/ record for this host. May be specified multiple times to add multiple DNS records.
 .TP
-- 
2.4.3

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

Re: [Freeipa-devel] [PATCH] DNSZone command with user friendly message

[Martin Basti]

On 28.10.2015 08:06, Abhijeet Kasurde wrote:



On 10/27/2015 08:28 PM, Martin Basti wrote:



On 27.10.2015 14:46, Martin Basti wrote:



On 27.10.2015 13:00, Abhijeet Kasurde wrote:

Hi All,

This patch fixes bug - https://fedorahosted.org/freeipa/ticket/4811

Thanks,
Abhijeet Kasurde


[Tue Oct 27 14:44:51.328615 2015] [wsgi:error] [pid 5556] ipa: 
ERROR: non-public: AttributeError: 'dnszone' object has no attribute 
'handle_obj_found'
[Tue Oct 27 14:44:51.328654 2015] [wsgi:error] [pid 5556] Traceback 
(most recent call last):
[Tue Oct 27 14:44:51.328659 2015] [wsgi:error] [pid 5556] File 
"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 350, 
in wsgi_execute
[Tue Oct 27 14:44:51.328664 2015] [wsgi:error] [pid 5556] result = 
self.Command[name](*args, **options)
[Tue Oct 27 14:44:51.328669 2015] [wsgi:error] [pid 5556] File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 447, in 
__call__
[Tue Oct 27 14:44:51.328674 2015] [wsgi:error] [pid 5556] ret = 
self.run(*args, **options)
[Tue Oct 27 14:44:51.328678 2015] [wsgi:error] [pid 5556] File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 764, in run
[Tue Oct 27 14:44:51.328683 2015] [wsgi:error] [pid 5556] return 
self.execute(*args, **options)
[Tue Oct 27 14:44:51.328687 2015] [wsgi:error] [pid 5556] File 
"/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 2935, 
in execute
[Tue Oct 27 14:44:51.328692 2015] [wsgi:error] [pid 5556] result = 
super(dnszone_enable, self).execute(*keys, **options)
[Tue Oct 27 14:44:51.328696 2015] [wsgi:error] [pid 5556] File 
"/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 2262, 
in execute
[Tue Oct 27 14:44:51.328701 2015] [wsgi:error] [pid 5556] 
self.obj.handle_obj_found(*keys)
[Tue Oct 27 14:44:51.328705 2015] [wsgi:error] [pid 5556] 
AttributeError: 'dnszone' object has no attribute 'handle_obj_found'





Thank you, ACK patch works as expected

However now 2 tests are failing because error message was changed, 
please fix tests too.


test_xmlrpc/test_dns_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_forward_zones::test_command[0071: 
dnsforwardzone_disable: Try to disable non-existent forward zone] FAILED
test_xmlrpc/test_dns_plugin.py <- 
test_xmlrpc/xmlrpc_test.py::test_forward_zones::test_command[0075: 
dnsforwardzone_enable: Try to enable non-existent forward zone] FAILED


E   AssertionError: assert_deepequal: expected != got.
E
E expected = u'no such entry'
E got = u'non-existent.fwzone.test.: DNS forward zone not 
found'

E path = ()


Updated patch with testcase

Martin



Pushed to master: c60cec4fa7adf004d383b68b78f6fd51d5cecb21
-- 
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] interactive installer does not ignore leading/trailing whitespace

[Martin Basti]

On 28.10.2015 02:35, Gabe Alford wrote:

Hello,

Fix for https://fedorahosted.org/freeipa/ticket/5355

Thanks,

Gabe



Thank you Gabe, but patch needs more work to be complete:

Bool and integer choices also need to strip whitespaces, see bellow:

Do you want to configure DNS forwarders? [yes]:   no
Do you want to configure DNS forwarders? [yes]:   no
Do you want to configure DNS forwarders? [yes]:   no
Do you want to configure DNS forwarders? [yes]: no
No DNS forwarders configured

Martin^2

-- 
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 0082] remove Kerberos authenticators after service uninstall

[Martin Babinsky]

Once again for the whole list:

On 10/22/2015 05:32 PM, Petr Spacek wrote:

On 21.10.2015 17:55, Martin Babinsky wrote:

On 10/13/2015 09:17 AM, Petr Spacek wrote:

On 12.10.2015 13:38, Martin Babinsky wrote:


each service possessing Kerberos keytab wiil now remove it and destroy any
associated credentials cache during its uninstall

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


BTW some time ago Simo proposed that we should remove caches and old keytabs
during *install* so problems caused by failing uninstallation will be fixed on
repeated install. This is yet another step towards idempotent installer.

To me this makes more sense than doing so on uninstall. Does it make sense to
you, too?



Attaching updated patch that does cleanup also before each instance creation.
It is a bit ugly I admit, but I couldn't think of a better way to do it and
didn't want to poke into service/instance code more than neccesary.


NACK, but we are almost there!

* kdestroy -A is too aggressive and wipes root's keyring after each run of
ipa-*-install utils.


That was caused by the env variables of running process being leaked
into the subprocess running kdestroy. The attached patch should fix that.

* There are some scattered leftovers of ipautil.run['kdestroy'...] in the
tree. Please get rid of them.


I will make that in a separate patch if you are OK with it.


Thank you!




--
Martin^3 Babinsky



From a6bc5bc86bcf319f26e2103b6d258ec8eb6d2d0c Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 9 Oct 2015 18:08:38 +0200
Subject: [PATCH] remove Kerberos authenticators when installing/uninstalling
 service instance

each service possessing Kerberos keytab/ccache will now perform their removal
before service principal creation and during service uninstall

https://fedorahosted.org/freeipa/ticket/5243
---
 ipaserver/install/adtrustinstance.py |  4 ++--
 ipaserver/install/bindinstance.py|  3 +++
 ipaserver/install/dnskeysyncinstance.py  |  3 +++
 ipaserver/install/dsinstance.py  |  4 ++--
 ipaserver/install/httpinstance.py| 11 ++
 ipaserver/install/installutils.py| 37 
 ipaserver/install/odsexporterinstance.py |  3 +++
 7 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index f7a7899906ca47b4901881fb6f4099ace1780741..6c083358081de7f5a47cf25f5d13469b2217978a 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -528,6 +528,7 @@ class ADTRUSTInstance(service.Service):
 self.print_msg("Cannot add CIFS service: %s" % e)
 
 self.clean_samba_keytab()
+installutils.remove_ccache(paths.KRB5CC_SAMBA)
 
 try:
 ipautil.run(["ipa-getkeytab", "--server", self.fqdn,
@@ -918,8 +919,7 @@ class ADTRUSTInstance(service.Service):
 self.print_msg('WARNING: ' + str(e))
 
 # Remove samba's credentials cache
-krb5cc_samba = paths.KRB5CC_SAMBA
-installutils.remove_file(krb5cc_samba)
+installutils.remove_ccache(ccache_path=paths.KRB5CC_SAMBA)
 
 # Remove samba's configuration file
 installutils.remove_file(self.smb_conf)
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 0bc0557cc10a6d32f71cc0426ce350c394216022..51c5d2df4e16b85c68698da45a1c4ce7b10c771d 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -1202,3 +1202,6 @@ class BindInstance(service.Service):
 
 if named_regular_running:
 self.named_regular.start()
+
+installutils.remove_keytab(paths.NAMED_KEYTAB)
+installutils.remove_ccache(run_as='named')
diff --git a/ipaserver/install/dnskeysyncinstance.py b/ipaserver/install/dnskeysyncinstance.py
index 68130c92558a4feb8d08fa826dbf6333d4461d1f..b2ccc027469a352c815963abfd0c0a61dd37297f 100644
--- a/ipaserver/install/dnskeysyncinstance.py
+++ b/ipaserver/install/dnskeysyncinstance.py
@@ -417,6 +417,7 @@ class DNSKeySyncInstance(service.Service):
 
 def __setup_principal(self):
 assert self.ods_gid is not None
+installutils.remove_keytab(paths.IPA_DNSKEYSYNCD_KEYTAB)
 dnssynckey_principal = "ipa-dnskeysyncd/" + self.fqdn + "@" + self.realm
 installutils.kadmin_addprinc(dnssynckey_principal)
 
@@ -497,3 +498,5 @@ class DNSKeySyncInstance(service.Service):
 os.remove(paths.DNSSEC_SOFTHSM_PIN)
 except Exception:
 pass
+
+installutils.remove_keytab(paths.IPA_DNSKEYSYNCD_KEYTAB)
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 15b23a8704091fbcf54e5be52562f6da2eeb1d73..7bdcaea31dcdf593d1de3b98a2ddfb926c2ea990 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -937,8 +937,8 @@ class DsInstance(service.Service):
 root_logger.debug("Removing DS instance 

Re: [Freeipa-devel] [PATCH 0011] Replica promotion related changes in integration tests

[Martin Basti]

On 29.10.2015 18:31, Martin Basti wrote:

NACK

1)
DO NOT use tabs in code to indent

2)
Replica uninstallation does not work, uninstallation works different 
with domain level 0 and 1 (currently uninstallation with domain 1 
level will not work, it is known issue, but still the patch should 
solve the uninstallation)


3)
apply_common_fixes(host)
Method for domain_level 1 is called twice, first time in replica 
install, second time in client install


4)
during testing this patch I used test_simple_replication and I found 4 
bugs:

3 bugs -^^^

#5419, #5420, #5421
IMO it is related only to this one test case and to pass this test 
case #5419 or #5421 must be fixed.



On 27.10.2015 16:34, Oleg Fayans wrote:

Hi Martin,

The updated patch is attached

On 10/27/2015 01:58 PM, Martin Basti wrote:



On 27.10.2015 13:56, Oleg Fayans wrote:



On 10/27/2015 01:22 PM, Martin Basti wrote:



On 27.10.2015 12:06, Oleg Fayans wrote:

Hi Martin,

On 10/27/2015 10:50 AM, Martin Basti wrote:



On 27.10.2015 10:22, Martin Basti wrote:



On 27.10.2015 10:00, Oleg Fayans wrote:

Hi Martin,

The updated version of the patch is attached. Please, see my
comments
below
My comments inline, I may be completely wrong in how the test 
suite

work, so feel free to correct me.

Martin



On 10/26/2015 06:48 PM, Martin Basti wrote:



On 26.10.2015 08:59, Oleg Fayans wrote:



On 10/23/2015 03:10 PM, Martin Basti wrote:



On 23.10.2015 15:00, Oleg Fayans wrote:

Hi Martin,

Here comes the updated version.

On 10/22/2015 05:38 PM, Martin Basti wrote:



On 22.10.2015 15:23, Martin Basti wrote:


On 22.10.2015 14:13, Oleg Fayans wrote:






Hello,

thank you for the patch.

1)
please remove the added empty lines, they are unrelated to
this
ticket


done



2)
-def install_master(host, setup_dns=True, setup_kra=False):
+def install_master(host, setup_dns=True, setup_kra=False,
domainlevel=1):

I suggest to use default domainlevel=None, which will 
use the

default
domain level (specified in build)


done



3)
+domain_level = domainlevel(master)
I do not think that this meets expectations.

We have to test, both domain level 0 and 1 for IPA 4.3,
respectively
new IPA must support all older domain levels, domain 
level is

independent on IPA version, only admin can raise it up.

So you have to find out way how to pass the domain level 
for

which
test will be running, we were talking about using config
files,
but
feel free to find something new and better


Fixed. Now, we declare domain level in config.yaml with the
directive
domain_level



4)
Did you resolve the pytest fixtures which specifies which
tests
can be
run under which domain level?


In fact, we do not seem to have any tests yet that would
require it.
All the existing tests just use install_replica
 method, no matter how is it done.
How about topology CI test? This can be executed only with 
domain

level


That's right. The topology test was updated. Patch is attached
together with a proper version of 11-th patch (not a swap file,
sorry
about that).


1, right?




5)
+ '--ip-address', client.ip,

why this change to client install?


Right, it found to be unnecessary.



Martin^2



6)
* Module ipatests.test_integration.tasks
ipatests/test_integration/tasks.py:85:
[E1123(unexpected-keyword-arg),
allow_sync_ptr] Unexpected keyword argument 'raiseonerr' in
function
call)


Fixed












1)
+if not host.config.domain_level == None:
+args.append("--domain-level=%i" %
host.config.domain_level)

always use: variable *is not None*

2)
Why there is specified level 1 as default? IIRC we agreed 
that the

default level is the one which is default in tested package.
These should be None and "":
+"domain_level": "1"

+"DOMAINLVL": "1",

3)
However, as I read the patch 12, and I'm right, the 
pytest.fixture

needs
to know the value of domain level before we can do any dynamic
detection
on master.

So we should use the constants  MAX_DOMAIN_LEVEL as default, 
for 2)

Done


Also I'm not sure if the values are inherited from the
DEFAULT_OUTPUT_DICT to code, I think it is not, so for this part
you
need default value, or the fixture will not work as expected.
+self.domain_level = kwargs.get('domain_level',
MAX_DOMAIN_LEVEL)


This won't work in cases when domainlevel is explicitly set to 
0 in

config.yaml. This default value will always override the explicit
one.
wat? in case that kwargs is dict containing values from config 
file:


In [1]: kwargs = dict(domain_level=0)

In [2]: kwargs.get('domain_level', 123)
Out[2]: 0


Yep, right you are, it works as expected. Now the line looks like:
self.domain_level = kwargs.get('domain_level', MAX_DOMAIN_LEVEL)




Respectively you should use this:

self.domain_level = kwargs.get('domain_level')or MAX_DOMAIN_LEVEL

Now that would definitely not work in case of domain_level is 0:
0 or 1 is always 1

Oh right.

I had discussion with Tomas, and we should add there check if it 
is 

Re: [Freeipa-devel] [PATCH 0058] interactive installer does not ignore leading/trailing whitespace

[Martin Basti]

On 29.10.2015 14:42, Gabe Alford wrote:

My bad Martin^2. Here is an updated patch.

Gabe

Thanks, ACK

Pushed to master: 9ffb3882532436dfd475831ee74b06e1b785251f


On Thu, Oct 29, 2015 at 7:14 AM, Martin Basti > wrote:




On 28.10.2015 02:35, Gabe Alford wrote:

Hello,

Fix for https://fedorahosted.org/freeipa/ticket/5355

Thanks,

Gabe



Thank you Gabe, but patch needs more work to be complete:

Bool and integer choices also need to strip whitespaces, see bellow:

Do you want to configure DNS forwarders? [yes]:   no
Do you want to configure DNS forwarders? [yes]:   no
Do you want to configure DNS forwarders? [yes]:   no
Do you want to configure DNS forwarders? [yes]: no
No DNS forwarders configured

Martin^2




-- 
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] File contains no config_file_version

[Martin Babinsky]

On 10/28/2015 07:56 PM, Oleg Fayans wrote:

Hi guys,


The following error is thrown at the attempt to install a new replica
from a freshly built upstream packages:

   [24/24]: enabling CA instance
Done configuring certificate server (pki-tomcatd).
ipa.ipapython.install.cli.install_tool(Replica): ERRORFile contains
no config_file_version
ipa.ipapython.install.cli.install_tool(Replica): ERRORThe
ipa-replica-install command failed. See /var/log/ipareplica-install.log
for more information
Your system may be partly configured.
Run /usr/sbin/ipa-server-install --uninstall to clean up.

The log is attached

The basic replica's functionality is OK though.





Hi Oleg,

this is caused by a bug in sssd. The following build for F22 fixes it
https://bodhi.fedoraproject.org/updates/FEDORA-2015-5e1da56d16

--
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] [draft] Fate of ipa-replica-manage and ipa-csreplica-manage tools

[Petr Spacek]

On 27.10.2015 15:54, Petr Vobornik wrote:
> Both tools serve primarily for managing replication agreements and replicas.
> ipa-replica-manage also manages winsync agreements and DNA ranges.
> 
> FreeIPA 4.3 will introduce managed topology which affects these tools.
> 
> Let's go trough all sub-commands of both tools and decide what is the fate of
> them/how they should be replaced. Comments are welcome.
> 
> In text, term 'disable' means: print an error message with help what is the
> new alternative.
> 
> For domain level == 0 all sub-commands should behave the same way as before.
> Proposals are for domain level 1 if not stated otherwise.
> 
> == ipa-replica-manage ==
> === list ===
> Lists all IPA server or replication agreements of a specific IPA server
> including winsync agreements.
> 
> Server list is replaced by
>   ipa server-find
> Replication agreements by:
>   ipa topologysegment-find realm
> 
> I see following paths:
> 1. do not change (current state)
> 2. list only winsync agreements - IMO it will be easier to maintain
> 
> If winsync was not in play we could 'disable' it but winsync is not planned to
> be centrally managed. Mainly because the preferred alternative is trust.
> 
> === connect ===
> Allow for winsync, disable for REALM agmts. (current state)
> 
> === disconenct ===
> Allow for winsync, disable for REALM agmts. (current state)
> 
> === del ===
> (current state)
> With domain level 0:
> - removes replica and repl. agmts for REALM suffix and winsync
> With domain level 1:
> - removes replica entry and therefore repl. agmts for all suffices(REALM, CS)
> - ensure last services, e.g. sets renewal master
> - does additional cleanup
> 
> I'm not aware of any operation which needs directory manager. IMO it can be
> moved to API in future release(e.g. 4.4), especially if ipa-server-install
> --uninstall is modified to do most of the cleanup.
> 
> === re-initialize ===
> Not changed.
> 
> Can be disabled (long-term solution)
> 
> Same capability is in topologysegment_reinitialize API command. The only
> difference is that no API command shows state of the pending operation. Should
> we transform presence of 'start' and 'stop' in
> nsds5beginreplicarefresh;left|right attribute into an output of
> topologysegment_show, e.g.: 'initialization in progress', 'cancellation of
> re-initialization requested'.
> 
> === force-sync ===
> no change yet
> 
> Currently done by setting nsDS5ReplicaUpdateSchedule attribute of repl.
> agreement.
> 
> 1. Is it required?
> 2. Should the functionality be transferred to topologysegment/topology plugin?
> 3. Is current approach good?
> 
> IMO if we want to preserve the possibility then the long-term solution is to
> move it to topology plugin.
> 
> === list-ruv, clean-ruv, abort-clean-ruv, list-clean-ruv ===
> Commands manages clean-all-ruv operations on REALM suffix.
> ipa-csreplica-manage doesn't have these commands #4987. These operations are
> meant for removal of dangling ruvs but they can also remove "correct" RUV
> which is not desired.
> 
> The UX is not the best because if replica still exists it won't tell the admin
> what is the correct RUV and which are the dangling one(s) and therefore admin
> must get the info in cn=replica,cn=$SUFFIX,cn=mapping tree,cn=config
> 
> We have a ticket to automate it: https://fedorahosted.org/freeipa/ticket/5411
> 
> Is it possible to manage it in topology plugin in centralized manner?
> 
> I see $5411 as short-term solution for 4.3 or 4.4. +
> {list|clean|abort-clean-list-clean}-ruv sub-commands should be extended to
> work with all suffices.
> 
> Long term solution not in 4.3 is to move it to topology plugin.
> 
> === dna(next)range-{show|set} commands
> No change in 4.3.
> 
> Long term solution is to make it centrally manageable. Not sure if by topo
> plugin or something else.
> 
> 
> == ipa-csreplica-manage ==
> This tool manages only CS replication agreements.
> 
> === list ===
> Not needed. We have `ipa server-find` and `ipa topologysegment-find ipaca`
> commands.
> 
> Should be disabled, add to #5405
> 
> === connect and disconnect ===
> Replaced by `ipa topologysegment-{add,del}` commands.
> 
> disable #5405
> 
> === del ===
> The work is done in `ipa-replica-manage del` therefore disable #5405
> 
> === re-initialize ===
> Same as in ipa-replica-manage - can be disabled. No ticket yet.
> 
> === force-sync ===
> Same as in ipa-replica-manage - decide what to do. No ticket yet.
> 
> === set-renewal-master ===
> AFAIK it's only update in cn=masters so could be moved to API then this could
> be disabled.
> 
> The change is simple enough for changing in 4.3. No ticket yet.
> 
> == Conclusion ==
> ipa-csreplica-manage could be abandoned in 4.3 which plays well with topic
> "simplify management of CA replication agreements".
> 
> ipa-replica-manage is still needed for RUV handling and removal of replicas in
> 4.3. This can change in a future. Same situation with DNA ranges handling.
> 
> There is no future plan for 

[Freeipa-devel] [PATCH 0092] ipa-replica-prepare: more robust and concise check for supported domain level

[Martin Babinsky]

Petr^2 and Tomas were not happy by the way 
https://fedorahosted.org/freeipa/ticket/5175 was handled initially, so 
here is a patch that tries to amend some of the issues.


--
Martin^3 Babinsky
From 3c0cf0b0d6584ba0a1b329e409b67bcdffefbd1f Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 26 Oct 2015 14:55:21 +0100
Subject: [PATCH] ipa-replica-prepare: more robust and concise check for
 supported domain level

https://fedorahosted.org/freeipa/ticket/5175
---
 ipaserver/install/ipa_replica_prepare.py | 49 ++--
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/ipaserver/install/ipa_replica_prepare.py b/ipaserver/install/ipa_replica_prepare.py
index 8998bc094e22ba9a95d528b09f73b2884d78462f..fc495204dfc5911d8b8fb896a87a14706f38cfb1 100644
--- a/ipaserver/install/ipa_replica_prepare.py
+++ b/ipaserver/install/ipa_replica_prepare.py
@@ -44,20 +44,6 @@ from ipaplatform.paths import paths
 from ipalib.constants import CACERT, DOMAIN_LEVEL_0
 
 
-UNSUPPORTED_DOMAIN_LEVEL_TEMPLATE = """
-Replica creation using '{command_name}' to generate replica file
-is supported only in {domain_level}-level IPA domain.
-
-The current IPA domain level is {curr_domain_level} and thus the replica must
-be created by promoting an existing IPA client.
-
-To set up a replica use the following procedure:
-1.) set up a client on the host using 'ipa-client-install'
-2.) promote the client to replica running 'ipa-replica-install'
-*without* replica file specified
-"""
-
-
 class ReplicaPrepare(admintool.AdminTool):
 command_name = 'ipa-replica-prepare'
 
@@ -175,7 +161,7 @@ class ReplicaPrepare(admintool.AdminTool):
 api.bootstrap(in_server=True)
 api.finalize()
 
-self.check_domainlevel(api)
+self.check_for_supported_domain_level()
 
 if api.env.host == self.replica_fqdn:
 raise admintool.ScriptError("You can't create a replica on itself")
@@ -690,12 +676,31 @@ class ReplicaPrepare(admintool.AdminTool):
 '-o', ca_file
 ])
 
-def check_domainlevel(self, api):
+def check_for_supported_domain_level(self):
+"""
+check if we are in 0-level topology. If not, raise an error pointing
+the user to the replica promotion pathway
+"""
 domain_level = dsinstance.get_domain_level(api)
-if domain_level > DOMAIN_LEVEL_0:
-raise RuntimeError(
-UNSUPPORTED_DOMAIN_LEVEL_TEMPLATE.format(
-command_name=self.command_name,
-domain_level=DOMAIN_LEVEL_0,
-curr_domain_level=domain_level)
+
+if domain_level != DOMAIN_LEVEL_0:
+self.log.error(
+"Current IPA domain level is {domain_level}.".format(
+domain_level=domain_level
+)
+)
+self.log.error("Using replica files to set up IPA replicas is not "
+   "supported."
+)
+self.log.info(
+"To create a replica, you must promote an existing "
+"IPA client.".format(domain_level=domain_level)
+)
+raise errors.InvalidDomainLevelError(
+reason=(
+"'{command}' requires domain level {prep_domain_level}"
+).format(
+command=self.command_name,
+prep_domain_level=DOMAIN_LEVEL_0
+)
 )
-- 
2.4.3

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

[Freeipa-devel] [PATCH 0060] Incomplete ports for IPA AD Trust

[Gabe Alford]

Hello,

Fix for https://fedorahosted.org/freeipa/ticket/5414

Thanks,

Gabe
From 515582d66252521a3cbf6a6a48f33745bd788c86 Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Thu, 29 Oct 2015 20:28:27 -0600
Subject: [PATCH] Incomplete ports for IPA AD Trust

https://fedorahosted.org/freeipa/ticket/5414
---
 install/tools/ipa-adtrust-install | 1 +
 1 file changed, 1 insertion(+)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 1f41cc437e8a930c350eac0fb34e5bebc9f9b55b..84e28b57524b2c3308e52cc56b4b370276add0b7 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -472,6 +472,7 @@ Setup complete
 
 You must make sure these network ports are open:
 \tTCP Ports:
+\t  * 135: epmap
 \t  * 138: netbios-dgm
 \t  * 139: netbios-ssn
 \t  * 445: microsoft-ds
-- 
2.4.3

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

[Freeipa-devel] [PATCH 0061] Remove 50-lockout-policy.update file

[Gabe Alford]

Hello,

Fix for https://fedorahosted.org/freeipa/ticket/5418

Thanks,

Gabe
From 7a9086162717bc414a1d65ea71a2d65729f6fa7e Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Thu, 29 Oct 2015 20:30:35 -0600
Subject: [PATCH] Remove 50-lockout-policy.update file

https://fedorahosted.org/freeipa/ticket/5418
---
 install/updates/50-lockout-policy.update | 4 
 install/updates/Makefile.am  | 1 -
 2 files changed, 5 deletions(-)
 delete mode 100644 install/updates/50-lockout-policy.update

diff --git a/install/updates/50-lockout-policy.update b/install/updates/50-lockout-policy.update
deleted file mode 100644
index a5730709e2b649466118502ece1cc530c10e0b40..
--- a/install/updates/50-lockout-policy.update
+++ /dev/null
@@ -1,4 +0,0 @@
-dn: cn=global_policy,cn=$REALM,cn=kerberos,$SUFFIX
-replace:krbPwdLockoutDuration:10::600
-replace: krbPwdMaxFailure:3::6
-
diff --git a/install/updates/Makefile.am b/install/updates/Makefile.am
index 26e4c04ed66a4a2061a3bb3ca2f4a6cd84502598..04ddeb96de4e88d5909f13b13885d3207184e798 100644
--- a/install/updates/Makefile.am
+++ b/install/updates/Makefile.am
@@ -39,7 +39,6 @@ app_DATA =\
 	45-roles.update			\
 	50-7_bit_check.update	\
 	50-dogtag10-migration.update	\
-	50-lockout-policy.update	\
 	50-groupuuid.update		\
 	50-hbacservice.update		\
 	50-krbenctypes.update		\
-- 
2.4.3

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