Re: [Freeipa-devel] [PATCH 0063] Remove dead code in ipaserver/install/installutils: read_ip_address(

[Tomas Babej]

On 11/10/2015 02:10 PM, Tomas Babej wrote:
> 
> 
> On 11/10/2015 12:58 PM, Petr Spacek wrote:
>> Hello,
>>
>> Remove dead code in ipaserver/install/installutils: read_ip_address().
>>
>> git grep claims that nothing is calling this function.
>>
>>
>>
> 
> ACK. This should have been removed with the
> 26c1851e98f31f11cf52b36bcb7e399ccbb2af17.
> 
> Tomas
> 

Pushed to master: bca9371d8e8aa138a0e638b52acc8929801d586f

-- 
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 0064-0065] ipa-dns-install offers IP addresses from resolv.conf as default forwarder

[Petr Spacek]

On 10.11.2015 17:07, Gabe Alford wrote:
> Does this also fix https://fedorahosted.org/freeipa/ticket/3926?

Yes. Good catch, I did not know about this ticket :-)

Petr Spacek @ Red Hat

> On Tue, Nov 10, 2015 at 8:58 AM, Petr Spacek  wrote:
> 
>> Hello,
>>
>> Patch 64:
>> ipa-dns-install offer IP addresses from resolv.conf as default forwarders
>>
>> In non-interactive more option --auto-forwarders can be used to do the
>> same. --forward option can be used to supply additional IP addresses.
>>
>> https://fedorahosted.org/freeipa/ticket/5438
>>
>>
>> Patch 65:
>> Remove global variable dns_forwarders from ipaserver.install.dns
>> It seems to me that the global thingy is not necessary, so I've ripped it
>> out.
>>
>> --
>> 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

[Freeipa-devel] [PATCH 0064-0065] ipa-dns-install offers IP addresses from resolv.conf as default forwarder

[Petr Spacek]

Hello,

Patch 64:
ipa-dns-install offer IP addresses from resolv.conf as default forwarders

In non-interactive more option --auto-forwarders can be used to do the
same. --forward option can be used to supply additional IP addresses.

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


Patch 65:
Remove global variable dns_forwarders from ipaserver.install.dns
It seems to me that the global thingy is not necessary, so I've ripped it out.

-- 
Petr^2 Spacek
From aeb8a1a3edafdf50d46968bee3a7f28c6039d4e1 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Tue, 10 Nov 2015 11:22:43 +0100
Subject: [PATCH] ipa-dns-install offer IP addresses from resolv.conf as
 default forwarders

In non-interactive more option --auto-forwarders can be used to do the
same. --forward option can be used to supply additional IP addresses.

https://fedorahosted.org/freeipa/ticket/5438
---
 ipaserver/install/dns.py   | 12 ++--
 ipaserver/install/installutils.py  |  7 +++
 ipaserver/install/server/common.py | 14 ++
 ipaserver/install/server/install.py|  7 ---
 ipaserver/install/server/replicainstall.py |  7 ---
 5 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py
index da24a6f2f4872581f4c0dc6194614b27a4006a0d..a26f4e3e7d44393ea948457e5e8db7a4b9bdc7f3 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -2,8 +2,11 @@
 # Copyright (C) 2015  FreeIPA Contributors see COPYING for license
 #
 
+from __future__ import absolute_import
 from __future__ import print_function
 
+# absolute import is necessary because IPA module dns clashes with python-dns
+from dns import resolver
 import sys
 
 from subprocess import CalledProcessError
@@ -232,8 +235,13 @@ def install_check(standalone, replica, options, hostname):
 
 if options.no_forwarders:
 dns_forwarders = ()
-elif options.forwarders:
-dns_forwarders = options.forwarders
+elif options.forwarders or options.auto_forwarders:
+if options.forwarders:
+dns_forwarders = options.forwarders
+else:
+dns_forwarders = []
+if options.auto_forwarders:
+dns_forwarders += resolver.get_default_resolver().nameservers
 elif standalone or not replica:
 dns_forwarders = read_dns_forwarders()
 
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 1d3551f8bb9cfcac1f6fa24043aea4b5d0a07719..39b5ba6eb2f3ddbe5fd6d68537330a482e966aec 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -295,6 +295,13 @@ def read_ip_addresses():
 def read_dns_forwarders():
 addrs = []
 if ipautil.user_input("Do you want to configure DNS forwarders?", True):
+print("Following DNS servers are configured in /etc/resolv.conf: %s" %
+", ".join(resolver.get_default_resolver().nameservers))
+if ipautil.user_input("Do you want to configure these servers as DNS "
+"forwarders?", True):
+addrs = resolver.default_resolver.nameservers[:]
+print("All DNS servers from /etc/resolv.conf were added. You can "
+  "enter additional addresses now:")
 while True:
 ip = ipautil.user_input("Enter an IP address for a DNS forwarder, "
 "or press Enter to skip", allow_empty=True)
diff --git a/ipaserver/install/server/common.py b/ipaserver/install/server/common.py
index 93c95dd8e8d2b24af193ee19368959188bcd6cb9..82c2c9eac253f82baeffbebfa388718dcc30d14a 100644
--- a/ipaserver/install/server/common.py
+++ b/ipaserver/install/server/common.py
@@ -167,6 +167,11 @@ class BaseServerDNS(common.Installable, core.Group, core.Composite):
 cli_name='forwarder',
 )
 
+auto_forwarders = Knob(
+bool, False,
+description="Use DNS forwarders configured in /etc/resolv.conf",
+)
+
 no_forwarders = Knob(
 bool, False,
 description="Do not add any DNS forwarders, use root servers instead",
@@ -395,6 +400,10 @@ class BaseServer(common.Installable, common.Interactive, core.Composite):
 raise RuntimeError(
 "You cannot specify a --forwarder option without the "
 "--setup-dns option")
+if self.dns.auto_forwarders:
+raise RuntimeError(
+"You cannot specify a --auto-forwarders option without "
+"the --setup-dns option")
 if self.dns.no_forwarders:
 raise RuntimeError(
 "You cannot specify a --no-forwarders option without the "
@@ -415,6 +424,10 @@ class BaseServer(common.Installable, common.Interactive, core.Composite):
 raise RuntimeError(
 "You cannot specify a --forwarder option together with "
 "--no-forwarders")
+elif 

Re: [Freeipa-devel] [PATCH 0064-0065] ipa-dns-install offers IP addresses from resolv.conf as default forwarder

[Gabe Alford]

Does this also fix https://fedorahosted.org/freeipa/ticket/3926?

On Tue, Nov 10, 2015 at 8:58 AM, Petr Spacek  wrote:

> Hello,
>
> Patch 64:
> ipa-dns-install offer IP addresses from resolv.conf as default forwarders
>
> In non-interactive more option --auto-forwarders can be used to do the
> same. --forward option can be used to supply additional IP addresses.
>
> https://fedorahosted.org/freeipa/ticket/5438
>
>
> Patch 65:
> Remove global variable dns_forwarders from ipaserver.install.dns
> It seems to me that the global thingy is not necessary, so I've ripped it
> out.
>
> --
> 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
>
-- 
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] [PATCHES 377-379] Hardening of ipa-adtrust-install

[Tomas Babej]

On 11/10/2015 03:35 PM, Martin Babinsky wrote:
> On 10/27/2015 04:24 PM, Tomas Babej wrote:
>> Hi,
>>
>> this couple of patches harden the adtrust installer.
>>
>> Details in the commit messages.
>>
>> Fixes: https://fedorahosted.org/freeipa/ticket/5134
>>
>> Tomas
>>
>>
>>
> NACK,
> 
> in the first patch you forgot to instantiate the caught exception in the
> following snippet:
> 
> +except Exception:
> +root_logger.debug("Exception occured during SID generation:
> {0}"
> +  .format(str(e)))
> 
> You should use 'except Exception as e:'.
> 
> I'm also not quite sure that it is enough to log the error at debug level.
> 
> If the sidgen task somehow fails, isn't it something which should
> interest the user and deserve at least warning-level message?
> 

Thanks for catching this. Inappropriate message level indeed, I probably
wasn't using my brain much when writing that snippet :)

Updated patchset attached.

Tomas
From 38bdd4be27eff57effeb5ff47585378aa92db409 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Tue, 27 Oct 2015 16:05:03 +0100
Subject: [PATCH] adtrustinstance: Wait for sidgen task completion

As part of hardening of adtrust installer, we should wait until
the sidgen task is completed before continuing, as it can take
considerable amount of time for a larger deployment.

https://fedorahosted.org/freeipa/ticket/5134
---
 ipaserver/install/adtrustinstance.py | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index f7a7899906ca47b4901881fb6f4099ace1780741..721cc23e7128751ca71eb68951a200d362ab7a74 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -34,6 +34,7 @@ from ipaserver.install import service
 from ipaserver.install import installutils
 from ipaserver.install.bindinstance import get_rr, add_rr, del_rr, \
dns_zone_exists
+from ipaserver.install.replication import wait_for_task
 from ipalib import errors, api
 from ipalib.util import normalize_zone
 from ipapython.dn import DN
@@ -469,13 +470,24 @@ class ADTRUSTInstance(service.Service):
 
 def __add_sids(self):
 """
-Add SIDs for existing users and groups
+Add SIDs for existing users and groups. Make sure the task is finished
+before continuing.
 """
 
 try:
+# Start the sidgen task
 self._ldap_mod("ipa-sidgen-task-run.ldif", self.sub_dict)
-except:
-pass
+
+# Notify the user about the possible delay
+self.print_msg("This step may take considerable amount of time, please wait..")
+
+# Wait for the task to complete
+task_dn = DN('cn=sidgen,cn=ipa-sidgen-task,cn=tasks,cn=config')
+wait_for_task(self.admin_conn, task_dn)
+
+except Exception as e:
+root_logger.warning("Exception occured during SID generation: {0}"
+.format(str(e)))
 
 def __add_s4u2proxy_target(self):
 """
-- 
2.4.3

From efd99d99020e56dabde25f7b6af19e470fbb01e1 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Tue, 27 Oct 2015 16:05:35 +0100
Subject: [PATCH] adtrustinstance: Restart samba service at the end of
 adtrust-install

Errors related to establishing trust can occur if samba service is not
restarted after ipa-adtrust-install has been run. Restart the service at
the end of the installer to avoid such issues.

https://fedorahosted.org/freeipa/ticket/5134
---
 ipaserver/install/adtrustinstance.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index 721cc23e7128751ca71eb68951a200d362ab7a74..ab8783b4939d580c6d6b14d385ed4e847ed960e3 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -749,6 +749,12 @@ class ADTRUSTInstance(service.Service):
 except:
 pass
 
+def __restart_smb(self):
+try:
+services.knownservices.smb.restart()
+except Exception:
+pass
+
 def __enable(self):
 self.backup_state("enabled", self.is_enabled())
 # We do not let the system start IPA components on its own,
@@ -880,6 +886,7 @@ class ADTRUSTInstance(service.Service):
 if self.add_sids:
 self.step("adding SIDs to existing users and groups",
   self.__add_sids)
+self.step("restarting smbd", self.__restart_smb)
 
 self.start_creation(show_service_name=False)
 
-- 
2.4.3

From 66ed28240c2b7a808bd752e810770a8d68dfe842 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Tue, 27 Oct 2015 16:08:10 +0100
Subject: [PATCH] adtrustinstance: Do not use bare except clauses

https://fedorahosted.org/freeipa/ticket/5134
---
 

[Freeipa-devel] [PATCH 0066] Remove unused constant NEW_MASTER_MARK from ipaserver.install.dn

[Petr Spacek]

Hello,

Remove unused constant NEW_MASTER_MARK from ipaserver.install.dns.

-- 
Petr^2 Spacek
From 0d387a4afe0a166c04584ccaacf6fa75bb171354 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Tue, 10 Nov 2015 17:04:15 +0100
Subject: [PATCH] Remove unused constant NEW_MASTER_MARK from
 ipaserver.install.dns

---
 ipaserver/install/dns.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py
index 421afbc0c9f6b44c3c96748d58930ec82e36f14e..258bf5dbe46e2167e07a62127c7fd8fd4be23ee6 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -34,8 +34,6 @@ from ipaserver.install import opendnssecinstance
 ip_addresses = []
 reverse_zones = []
 
-NEW_MASTER_MARK = 'NEW_DNSSEC_MASTER'
-
 
 def _find_dnssec_enabled_zones(conn):
 search_kw = {'idnssecinlinesigning': True}
-- 
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] [PATCHES 377-379] Hardening of ipa-adtrust-install

[Martin Babinsky]

On 11/10/2015 04:35 PM, Tomas Babej wrote:



On 11/10/2015 03:35 PM, Martin Babinsky wrote:

On 10/27/2015 04:24 PM, Tomas Babej wrote:

Hi,

this couple of patches harden the adtrust installer.

Details in the commit messages.

Fixes: https://fedorahosted.org/freeipa/ticket/5134

Tomas




NACK,

in the first patch you forgot to instantiate the caught exception in the
following snippet:

+except Exception:
+root_logger.debug("Exception occured during SID generation:
{0}"
+  .format(str(e)))

You should use 'except Exception as e:'.

I'm also not quite sure that it is enough to log the error at debug level.

If the sidgen task somehow fails, isn't it something which should
interest the user and deserve at least warning-level message?



Thanks for catching this. Inappropriate message level indeed, I probably
wasn't using my brain much when writing that snippet :)

Updated patchset attached.

Tomas


Thanks, ACK.

--
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 0093] perform connectivity checks for all topology suffixes during node deletion

[Martin Babinsky]

On 11/04/2015 06:50 PM, Petr Vobornik wrote:

On 11/04/2015 01:30 PM, Martin Babinsky wrote:

On 10/30/2015 05:06 PM, Martin Babinsky wrote:

On 10/30/2015 03:38 PM, Petr Vobornik wrote:

On 10/30/2015 03:26 PM, Martin Babinsky wrote:

patch for https://fedorahosted.org/freeipa/ticket/5309

The ticket itself is about connectivity checks in topology suffixes,
but
there is a code (install/tools/ipa-replica-manage starting at line 788
after applying my patch) which monitors whether the segments pointing
to/from the deleted host are already deleted.

These checks are currently hardcoded for 'realm' prefix, should we
generalize them as well or is it a part of other effort?



Could be separate patch but yes.

Ok I have included it in the attached patch so that both of these
operations are performed for all suffixes.




Hmm, I'm thinking whether the 'check_last_link_managed' and
'check_deleted_segments' should not be called per-suffix, but should
themselves check all suffixes available. This could make the fix for
https://fedorahosted.org/freeipa/ticket/5409 also easier.



Depends if the output is reusable. If so then why not.
check_last_link_managed basically adds text to several
get_topology_connection_errors calls.


Attaching updated patch.

--
Martin^3 Babinsky
From bb0506c3f79d34bf70441eddb36e0c150229e2f0 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Fri, 30 Oct 2015 13:59:03 +0100
Subject: [PATCH] check for disconnected topology and deleted agreements for
 all suffices

The code in ipa-replica-manage which checks for disconnected topology and
deleted agreements during node removal was generalized so that it now performs
these checks for all suffixes to which the node belongs.

https://fedorahosted.org/freeipa/ticket/5309
---
 install/tools/ipa-replica-manage | 247 ++-
 1 file changed, 166 insertions(+), 81 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index b9998da44dcc1f01c5eb342ee713634de0ee84ee..8a44895c17e7fdc4c5160a77f55119fd3cc7f58d 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -570,46 +570,97 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
 else:
 return None
 
-def check_last_link_managed(api, masters, hostname, force):
+
+def map_masters_to_suffices(masters, suffices):
+masters_to_suffix = {}
+suffix_name_to_root = {
+s['iparepltopoconfroot'][0]: s['cn'][0] for s in suffices
+}
+
+for master in masters:
+managed_suffices = master['iparepltopomanagedsuffix']
+for suffix in managed_suffices:
+suffix_name = suffix_name_to_root[suffix]
+try:
+masters_to_suffix[suffix_name].append(master)
+except KeyError:
+masters_to_suffix[suffix_name] = [master]
+
+return masters_to_suffix
+
+
+def check_hostname_in_masters(hostname, masters):
+master_cns = {m['cn'][0] for m in masters}
+return hostname in master_cns
+
+
+def check_last_link_managed(api, hostname, masters, force):
 """
 Check if 'hostname' is safe to delete.
 
 :returns: tuple with lists of current and future errors in topology
   (current_errors, new_errors)
 """
+suffices = api.Command.topologysuffix_find(u'')['result']
+suffix_to_masters = map_masters_to_suffices(masters, suffices)
+topo_errors = ([], [])
+
+for suffix in suffices:
+suffix_name = suffix['cn'][0]
+suffix_members = suffix_to_masters[suffix_name]
+print("Checking connectivity in topology suffix '{}'".format(
+suffix_name))
+if not check_hostname_in_masters(hostname, suffix_members):
+print(
+"'{}' is not a part of topology suffix '{}'".format(
+hostname, suffix_name
+)
+)
+print("Not checking connectivity")
+continue
+
+segments = api.Command.topologysegment_find(
+suffix_name, sizelimit=0).get('result')
+graph = create_topology_graph(suffix_to_masters[suffix_name], segments)
+
+# check topology before removal
+orig_errors = get_topology_connection_errors(graph)
+if orig_errors:
+print("Current topology in suffix '{}' is disconnected:".format(
+suffix_name))
+print("Changes are not replicated to all servers and data are "
+  "probably inconsistent.")
+print("You need to add segments to reconnect the topology.")
+print_connect_errors(orig_errors)
+
+# after removal
+try:
+graph.remove_vertex(hostname)
+except ValueError:
+pass  # ignore already deleted master, continue to clean
+
+new_errors = get_topology_connection_errors(graph)
+if new_errors:
+print("WARNING: Removal of '{}' will lead to 

Re: [Freeipa-devel] [PATCHES 377-379] Hardening of ipa-adtrust-install

[Tomas Babej]

On 11/10/2015 04:43 PM, Martin Babinsky wrote:
> On 11/10/2015 04:35 PM, Tomas Babej wrote:
>>
>>
>> On 11/10/2015 03:35 PM, Martin Babinsky wrote:
>>> On 10/27/2015 04:24 PM, Tomas Babej wrote:
 Hi,

 this couple of patches harden the adtrust installer.

 Details in the commit messages.

 Fixes: https://fedorahosted.org/freeipa/ticket/5134

 Tomas



>>> NACK,
>>>
>>> in the first patch you forgot to instantiate the caught exception in the
>>> following snippet:
>>>
>>> +except Exception:
>>> +root_logger.debug("Exception occured during SID generation:
>>> {0}"
>>> +  .format(str(e)))
>>>
>>> You should use 'except Exception as e:'.
>>>
>>> I'm also not quite sure that it is enough to log the error at debug
>>> level.
>>>
>>> If the sidgen task somehow fails, isn't it something which should
>>> interest the user and deserve at least warning-level message?
>>>
>>
>> Thanks for catching this. Inappropriate message level indeed, I probably
>> wasn't using my brain much when writing that snippet :)
>>
>> Updated patchset attached.
>>
>> Tomas
>>
> Thanks, ACK.
> 

Pushed to master: 767b8de01298e9de20f3b85bad5ac98472b14e99

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

[Petr Spacek]

On 4.11.2015 11:56, Martin Babinsky wrote:
> 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.
>>
>> * There are some scattered leftovers of ipautil.run['kdestroy'...] in the
>> tree. Please get rid of them.
>>
>> Thank you!
>>
> Attaching updated patch. It got lost somewhere in the list.

ACK, thank you for patience.

-- 
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 506] cert renewal: make renewal of ipaCert atomic

[Rob Crittenden]

Jan Cholasta wrote:
> On 9.11.2015 16:51, Rob Crittenden wrote:
>> Jan Cholasta wrote:
>>> Hi,
>>>
>>> the attached patch fixes .
>>>
>>> Honza
>>>
>>>
>>>
>>
>> There be a note in renew_ra_cert that the lock is obtained in advance by
>> renew_ra_cert_pre.
> 
> Added comment.
> 
>>
>> It looks like it will silently fail if the lock cannot be acquired. Is
>> that desired?
> 
> All unhandled exceptions are logged to syslog in both renew_ra_cert_pre
> and renew_ra_cert:
> 
> try:
> main()
> except Exception:
> syslog.syslog(syslog.LOG_ERR, traceback.format_exc())
> 
> Updated patch attached.
> 

My confusion was with the auto-expiration. I guess this is ok. When
debugging this sort of thing via logs the more the merrier, so I guess
I'd have added a syslog to say "obtaining lock" or "locked" and then
something when the renewal actually starts, so one can try to piece
together what happened after the fact if something goes wrong.

I guess certmonger already logs when a pre/post command is executed so
that may already be available.

rob

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

[Freeipa-devel] [PATCH] Fix download message

[Francois Cami]

Hi,

The "Do you want download the CA cert from" message in ipa-client-install 
should be changed to "Do you want to download the CA cert from".
As this is a single-line trivial fix I haven't opened a trac ticket. I will do 
so if anyone feels this is necessary even in this case.

Thank you,
François
From 359ca1a9a27b7807d1e5954d42654630a0c7b9ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Cami?= 
Date: Tue, 10 Nov 2015 22:50:25 +0100
Subject: [PATCH] ipa-client-install: Fix the "download the CA cert" query

---
 ipa-client/ipa-install/ipa-client-install | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 14261e57f1fbc01ea57eb7e8160f9c8bf9d282f8..3011d25f3b8141e6a8464c71eeed38cf5302544e 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -2062,7 +2062,7 @@ def get_ca_certs(fstore, options, server, basedn, realm):
 url = http_url()
 override = not interactive
 if interactive and not user_input(
-"Do you want download the CA cert from " + url + " ?\n"
+"Do you want to download the CA cert from " + url + " ?\n"
 "(this is INSECURE)", False):
 raise errors.NoCertificateError(message=u"HTTP certificate"
 " download declined by user")
-- 
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 0094] Fix bogus error message in choice-type installer options

[Martin Basti]

On 09.11.2015 09:55, Martin Babinsky wrote:

On 11/09/2015 07:15 AM, Jan Cholasta wrote:

On 6.11.2015 17:02, Martin Babinsky wrote:

On 11/06/2015 10:30 AM, Martin Babinsky wrote:

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





Attaching updated patch.


NACK, the first patch was better, there should be quotes around the 
values.




Attaching updated patch.


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 0062] DNS record-add warns when a suspicious DNS name is detected

[Petr Spacek]

On 10.11.2015 11:17, Martin Basti wrote:
> 
> 
> On 09.11.2015 08:42, Petr Spacek wrote:
>> Hello,
>>
>> DNS record-add warns when a suspicious DNS name is detected
>>
>> Relative name "record.zone" is being added into zone "zone.",
>> which is probably a mistake. User probably wanted to either specify
>> relative name "record" or use FQDN "record.zone.".
>>
> 
> works as expected, I just have one nitpick:
> 
> record name and zone are instances of DNSName class, thus would be better to 
> use
> zone.relativize(DNSName.root)
> instead of
> zone.relativize(dns.name.root)

Thank you, I will keep it in mind for the next time.

Please fix it before push ;-)

-- 
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] 0001 Refactor test_user_plugin

[Milan Kubík]

On 11/09/2015 04:35 PM, Filip Škola wrote:

Another patch was applied in the meantime.

Attaching an updated version.

F.

On Mon, 9 Nov 2015 13:35:02 +0100
Milan Kubík  wrote:


On 11/06/2015 11:32 AM, Filip Škola wrote:



Hi,
the patch doesn't apply.


Please fix this.

ipatests/test_xmlrpc/test_user_plugin.py:1419: 
[E0602(undefined-variable), 
TestDeniedBindWithExpiredPrincipal.teardown_class] Undefined variable 
'user1')


Also, use the version numbers for your changed patches.

--
Milan Kubik

--
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 506] cert renewal: make renewal of ipaCert atomic

[Jan Cholasta]

On 9.11.2015 16:51, Rob Crittenden wrote:

Jan Cholasta wrote:

Hi,

the attached patch fixes .

Honza





There be a note in renew_ra_cert that the lock is obtained in advance by
renew_ra_cert_pre.


Added comment.



It looks like it will silently fail if the lock cannot be acquired. Is
that desired?


All unhandled exceptions are logged to syslog in both renew_ra_cert_pre 
and renew_ra_cert:


try:
main()
except Exception:
syslog.syslog(syslog.LOG_ERR, traceback.format_exc())

Updated patch attached.

--
Jan Cholasta
From a1beeccedf53678e1522002b366d2d7e7a4f447f Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Mon, 9 Nov 2015 10:53:02 +0100
Subject: [PATCH] cert renewal: make renewal of ipaCert atomic

This prevents errors when renewing other certificates during the renewal of
ipaCert.

https://fedorahosted.org/freeipa/ticket/5436
---
 install/restart_scripts/Makefile.am   |  1 +
 install/restart_scripts/renew_ra_cert |  5 -
 install/restart_scripts/renew_ra_cert_pre | 18 ++
 ipaserver/install/cainstance.py   |  2 +-
 ipaserver/install/server/upgrade.py   |  4 ++--
 5 files changed, 26 insertions(+), 4 deletions(-)
 create mode 100755 install/restart_scripts/renew_ra_cert_pre

diff --git a/install/restart_scripts/Makefile.am b/install/restart_scripts/Makefile.am
index 58057aa..c4bf819 100644
--- a/install/restart_scripts/Makefile.am
+++ b/install/restart_scripts/Makefile.am
@@ -7,6 +7,7 @@ app_DATA =  \
 	renew_ca_cert			\
 	renew_ra_cert			\
 	stop_pkicad			\
+	renew_ra_cert_pre		\
 	$(NULL)
 
 EXTRA_DIST =\
diff --git a/install/restart_scripts/renew_ra_cert b/install/restart_scripts/renew_ra_cert
index cf770a9..9b5e231 100644
--- a/install/restart_scripts/renew_ra_cert
+++ b/install/restart_scripts/renew_ra_cert
@@ -77,8 +77,11 @@ def _main():
 
 
 def main():
-with certs.renewal_lock:
+try:
 _main()
+finally:
+# lock acquired in renew_ra_cert_pre
+certs.renewal_lock.release('renew_ra_cert')
 
 
 try:
diff --git a/install/restart_scripts/renew_ra_cert_pre b/install/restart_scripts/renew_ra_cert_pre
new file mode 100755
index 000..d0f743c
--- /dev/null
+++ b/install/restart_scripts/renew_ra_cert_pre
@@ -0,0 +1,18 @@
+#!/usr/bin/python2 -E
+#
+# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
+#
+
+import syslog
+import traceback
+
+from ipaserver.install import certs
+
+
+def main():
+certs.renewal_lock.acquire('renew_ra_cert')
+
+try:
+main()
+except Exception:
+syslog.syslog(syslog.LOG_ERR, traceback.format_exc())
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 23fdf30..1cbc0d0 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1339,7 +1339,7 @@ class CAInstance(DogtagInstance):
 pin=None,
 pinfile=paths.ALIAS_PWDFILE_TXT,
 secdir=paths.HTTPD_ALIAS_DIR,
-pre_command=None,
+pre_command='renew_ra_cert_pre',
 post_command='renew_ra_cert')
 except RuntimeError as e:
 self.log.error(
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index 4337995..b9621a3 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -806,7 +806,7 @@ def certificate_renewal_update(ca):
 dogtag_constants = dogtag.configured_constants()
 
 # bump version when requests is changed
-version = 3
+version = 4
 requests = (
 (
 dogtag_constants.ALIAS_DIR,
@@ -844,7 +844,7 @@ def certificate_renewal_update(ca):
 paths.HTTPD_ALIAS_DIR,
 'ipaCert',
 'dogtag-ipa-ca-renew-agent',
-None,
+'renew_ra_cert_pre',
 'renew_ra_cert',
 None,
 ),
-- 
2.4.3

From d19a34271a779eee62f30c479b20adf502c751e5 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Mon, 9 Nov 2015 10:53:02 +0100
Subject: [PATCH] cert renewal: make renewal of ipaCert atomic

This prevents errors when renewing other certificates during the renewal of
ipaCert.

https://fedorahosted.org/freeipa/ticket/5436
---
 install/restart_scripts/Makefile.am   |  1 +
 install/restart_scripts/renew_ra_cert |  5 -
 install/restart_scripts/renew_ra_cert_pre | 18 ++
 ipaserver/install/cainstance.py   |  2 +-
 ipaserver/install/server/upgrade.py   |  4 ++--
 5 files changed, 26 insertions(+), 4 deletions(-)
 create mode 100755 install/restart_scripts/renew_ra_cert_pre

diff --git a/install/restart_scripts/Makefile.am b/install/restart_scripts/Makefile.am
index 58057aa..c4bf819 100644
--- a/install/restart_scripts/Makefile.am
+++ b/install/restart_scripts/Makefile.am
@@ -7,6 +7,7 @@ app_DATA =  

Re: [Freeipa-devel] [PATCH 0343] Upgrade: enable custodia service during upgrade

[Martin Basti]

On 09.11.2015 14:39, Gabe Alford wrote:

Ack.

Thanks,

Gabe

Thank you!
Pushed to master: a8c3d6fbb7ac9c5e9f665473bfb7414bb073ae09



On Tue, Nov 3, 2015 at 11:18 AM, Martin Basti > wrote:


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

Patch attached.

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




-- 
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 0062] DNS record-add warns when a suspicious DNS name is detected

[Martin Basti]

On 09.11.2015 08:42, Petr Spacek wrote:

Hello,

DNS record-add warns when a suspicious DNS name is detected

Relative name "record.zone" is being added into zone "zone.",
which is probably a mistake. User probably wanted to either specify
relative name "record" or use FQDN "record.zone.".



works as expected, I just have one nitpick:

record name and zone are instances of DNSName class, thus would be 
better to use

zone.relativize(DNSName.root)
instead of
zone.relativize(dns.name.root)

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 0342] Use domain level constants in topology plugin

[Martin Basti]

On 10.11.2015 12:59, David Kupka wrote:

On 03/11/15 10:45, Martin Basti wrote:

Patch attached.



Works for me, ACK.


Pushed to master: c339abbad1e82a264afc5cf9efa3aff80814aeaf

--
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 0342] Use domain level constants in topology plugin

[David Kupka]

On 03/11/15 10:45, Martin Basti wrote:

Patch attached.



Works for me, ACK.

--
David Kupka

--
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] 0001 Refactor test_user_plugin

[Filip Škola]

Hi,

fixed.

F.

On Tue, 10 Nov 2015 10:52:45 +0100
Milan Kubík  wrote:

> On 11/09/2015 04:35 PM, Filip Škola wrote:
> > Another patch was applied in the meantime.
> >
> > Attaching an updated version.
> >
> > F.
> >
> > On Mon, 9 Nov 2015 13:35:02 +0100
> > Milan Kubík  wrote:
> >
> >> On 11/06/2015 11:32 AM, Filip Škola wrote:
> >>>
> >> Hi,
> >> the patch doesn't apply.
> >>
> Please fix this.
> 
>  ipatests/test_xmlrpc/test_user_plugin.py:1419: 
> [E0602(undefined-variable), 
> TestDeniedBindWithExpiredPrincipal.teardown_class] Undefined variable 
> 'user1')
> 
> Also, use the version numbers for your changed patches.
> 

>From cdaa729b2bf3694f157e7f3fe62dd782e1a03334 Mon Sep 17 00:00:00 2001
From: Filip Skola 
Date: Fri, 6 Nov 2015 10:57:37 +0100
Subject: [PATCH] Refactor test_user_plugin, use UserTracker for tests

---
 ipatests/test_xmlrpc/test_user_plugin.py | 2809 +-
 1 file changed, 1195 insertions(+), 1614 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index 81185e449acaa127aa9429fff9587d39a2be81e6..2d63e1636abd6bb55d541df354d039404c6475e8 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -2,6 +2,7 @@
 #   Rob Crittenden 
 #   Pavel Zuna 
 #   Jason Gerard DeRose 
+#   Filip Skola 
 #
 # Copyright (C) 2008, 2009  Red Hat
 # see file 'COPYING' for use and warranty information
@@ -23,6 +24,7 @@
 Test the `ipalib/plugins/user.py` module.
 """
 
+import pytest
 import functools
 import datetime
 import ldap
@@ -33,41 +35,38 @@ from ipatests.test_xmlrpc import objectclasses
 from ipatests.util import (
 assert_equal, assert_not_equal, raises, assert_deepequal)
 from xmlrpc_test import (
-XMLRPC_test, Declarative, fuzzy_digits, fuzzy_uuid, fuzzy_password,
+XMLRPC_test, fuzzy_digits, fuzzy_uuid, fuzzy_password,
 fuzzy_string, fuzzy_dergeneralizedtime, add_sid, add_oc, raises_exact)
 from ipapython.dn import DN
 from ipatests.test_xmlrpc.ldaptracker import Tracker
 import pytest
 
-user1 = u'tuser1'
-user2 = u'tuser2'
 admin1 = u'admin'
-admin2 = u'admin2'
-renameduser1 = u'tuser'
 group1 = u'group1'
-admins_group = u'admins'
+admin_group = u'admins'
 
 invaliduser1 = u'+tuser1'
 invaliduser2 = u'tuser1234567890123456789012345678901234567890'
 
 sshpubkey = (u'ssh-rsa B3NzaC1yc2EDAQABAAABAQDGAX3xAeLeaJggwTqMjxNwa6X'
-  'HBUAikXPGMzEpVrlLDCZtv00djsFTBi38PkgxBJVkgRWMrcBsr/35lq7P6w8KGI'
-  'wA8GI48Z0qBS2NBMJ2u9WQ2hjLN6GdMlo77O0uJY3251p12pCVIS/bHRSq8kHO2'
-  'No8g7KA9fGGcagPfQH+ee3t7HUkpbQkFTmbPPN++r3V8oVUk5LxbryB3UIIVzNm'
-  'cSIn3JrXynlvui4MixvrtX6zx+O/bBo68o8/eZD26QrahVbA09fivrn/4h3TM01'
-  '9Eu/c2jOdckfU3cHUV/3Tno5d6JicibyaoDDK7S/yjdn5jhaz8MSEayQvFkZkiF'
-  '0L public key test')
+ 'HBUAikXPGMzEpVrlLDCZtv00djsFTBi38PkgxBJVkgRWMrcBsr/35lq7P6w8KGI'
+ 'wA8GI48Z0qBS2NBMJ2u9WQ2hjLN6GdMlo77O0uJY3251p12pCVIS/bHRSq8kHO2'
+ 'No8g7KA9fGGcagPfQH+ee3t7HUkpbQkFTmbPPN++r3V8oVUk5LxbryB3UIIVzNm'
+ 'cSIn3JrXynlvui4MixvrtX6zx+O/bBo68o8/eZD26QrahVbA09fivrn/4h3TM01'
+ '9Eu/c2jOdckfU3cHUV/3Tno5d6JicibyaoDDK7S/yjdn5jhaz8MSEayQvFkZkiF'
+ '0L public key test')
 sshpubkeyfp = (u'13:67:6B:BF:4E:A2:05:8E:AE:25:8B:A1:31:DE:6F:1B '
-'public key test (ssh-rsa)')
+   'public key test (ssh-rsa)')
 
-validlanguage1 = u'en-US;q=0.987 , en, abcdfgh-abcdefgh;q=1, a;q=1.000'
-validlanguage2 = u'*'
+validlanguages = {
+u'en-US;q=0.987 , en, abcdfgh-abcdefgh;q=1, a;q=1.000',
+u'*'
+}
 
-invalidlanguage1 = u'abcdfghji-abcdfghji'
-invalidlanguage2 = u'en-us;q=0,123'
-invalidlanguage3 = u'en-us;q=0.1234'
-invalidlanguage4 = u'en-us;q=1.1'
-invalidlanguage5 = u'en-us;q=1.'
+invalidlanguages = {
+u'abcdfghji-abcdfghji', u'en-us;q=0,123',
+u'en-us;q=0.1234', u'en-us;q=1.1', u'en-us;q=1.'
+}
 
 principal_expiration_string = "2020-12-07T19:54:13Z"
 principal_expiration_date = datetime.datetime(2020, 12, 7, 19, 54, 13)
@@ -79,1583 +78,6 @@ expired_expiration_string = "1991-12-07T19:54:13Z"
 isodate_re = re.compile('^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$')
 
 
-def get_user_result(uid, givenname, sn, operation='show', omit=[],
-**overrides):
-"""Get a user result for a user-{add,mod,find,show} command
-
-This gives the result as from a user_add(uid, givenname=givenname, sn=sn);
-modifications to that can be specified in ``omit`` and ``overrides``.
-
-The ``operation`` can be one of:
-- add
-- show
-- show-all ((show with the --all flag)
-- find
-- mod
-
-Attributes named in ``omit`` are removed from the result; any additional
-or non-default values can be specified in 

[Freeipa-devel] [PATCH 0063] Remove dead code in ipaserver/install/installutils: read_ip_address(

[Petr Spacek]

Hello,

Remove dead code in ipaserver/install/installutils: read_ip_address().

git grep claims that nothing is calling this function.

-- 
Petr^2 Spacek
From dcb4b959933a9085ddcb98e8a8c61ad52de846e7 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Tue, 10 Nov 2015 12:57:56 +0100
Subject: [PATCH] Remove dead code in ipaserver/install/installutils:
 read_ip_address()

---
 ipaserver/install/installutils.py | 14 --
 1 file changed, 14 deletions(-)

diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 9afcb1f3527a8c88451522b65141fd9910146c52..bc1950f1dd4417a324582e2b0e3b7087bef92884 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -261,20 +261,6 @@ def add_record_to_hosts(ip, host_name, conf_file=paths.HOSTS):
 hosts_fd.write(ip+'\t'+host_name+' '+host_name.split('.')[0]+'\n')
 hosts_fd.close()
 
-# TODO: Remove when removing usage from ipa-adtrust-install
-def read_ip_address(host_name, fstore):
-while True:
-ip = ipautil.user_input("Please provide the IP address to be used for this host name", allow_empty = False)
-try:
-ip_parsed = ipautil.CheckedIPAddress(ip, match_local=True)
-except Exception as e:
-print("Error: Invalid IP Address %s: %s" % (ip, e))
-continue
-else:
-break
-
-return ip_parsed
-
 
 def read_ip_addresses():
 ips = []
-- 
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 0064-0065] ipa-dns-install offers IP addresses from resolv.conf as default forwarder

[Jan Cholasta]

On 10.11.2015 16:58, Petr Spacek wrote:

Hello,

Patch 64:
ipa-dns-install offer IP addresses from resolv.conf as default forwarders

In non-interactive more option --auto-forwarders can be used to do the
same. --forward option can be used to supply additional IP addresses.

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


IMO it's perverse to add option which effectively means "use default 
value" instead of actually using the value as default. This is 
inconsistent with every other option and I don't see what makes 
forwarders so special to require this.


NACK unless you have a strong justification for this.




Patch 65:
Remove global variable dns_forwarders from ipaserver.install.dns
It seems to me that the global thingy is not necessary, so I've ripped it out.


ACK.

--
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] enable pem=True in export_pem_cert function

[Niranjan]

Niranjan wrote:
> Tomas Babej wrote:
> > On 10/26/2015 08:59 PM, Niranjan wrote:
> > > Greetings,
> > > 
> > > export_pem_cert function from ipapython/certdb  should export the 
> > > certificate
> > > in pem format but instead exports the cert in der format as it doesn't 
> > > enable pem=True.
> > > 
> > > This patch specifies pem=True for export_pem_cert function
> > > 
> > > Regards
> > > Niranjan
> > 
> > Hi,
> > 
> > the patch looks good, however, I'm curious as to how did you find this
> > bug? Does it affect anything?
> I am part of the CS(dogtag) QE team, and as part of CS Automation, i am 
> relying 
> on some generic functions provided by ipapython. While using those functions
> for our automation, I found it. 
> > 
> > It seems to me that this part of the code is a dead branch which should
> > be removed.
> I did not look further ipapython, so i am not aware where else export_pem_cert
> is being used, but i would like the functions in certdb.py be available.
> > 
> > $ git grep export_pem_cert
> > ipapython/certdb.py:def export_pem_cert(self, nickname, location):
> > ipaserver/install/certs.py:def export_pem_cert(self, nickname,
> > ipaserver/install/certs.py:return self.nssdb.export_pem_ce..

Any update on this.

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



pgpHPEL8LgJqo.pgp
Description: PGP 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] [PATCHES 377-379] Hardening of ipa-adtrust-install

[Martin Babinsky]

On 10/27/2015 04:24 PM, Tomas Babej wrote:

Hi,

this couple of patches harden the adtrust installer.

Details in the commit messages.

Fixes: https://fedorahosted.org/freeipa/ticket/5134

Tomas




NACK,

in the first patch you forgot to instantiate the caught exception in the 
following snippet:


+except Exception:
+root_logger.debug("Exception occured during SID generation: 
{0}"

+  .format(str(e)))

You should use 'except Exception as e:'.

I'm also not quite sure that it is enough to log the error at debug level.

If the sidgen task somehow fails, isn't it something which should 
interest the user and deserve at least warning-level message?


--
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 0062] DNS record-add warns when a suspicious DNS name is detected

[Martin Basti]

On 10.11.2015 11:19, Petr Spacek wrote:

On 10.11.2015 11:17, Martin Basti wrote:


On 09.11.2015 08:42, Petr Spacek wrote:

Hello,

DNS record-add warns when a suspicious DNS name is detected

Relative name "record.zone" is being added into zone "zone.",
which is probably a mistake. User probably wanted to either specify
relative name "record" or use FQDN "record.zone.".


works as expected, I just have one nitpick:

record name and zone are instances of DNSName class, thus would be better to use
zone.relativize(DNSName.root)
instead of
zone.relativize(dns.name.root)

Thank you, I will keep it in mind for the next time.

Please fix it before push ;-)


ACK

the nitpick has been fixed, Pushed to master: 
50b0471f01985d2d43998df1a9c4a73cf5cf47c1


--
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 0063] Remove dead code in ipaserver/install/installutils: read_ip_address(

[Tomas Babej]

On 11/10/2015 12:58 PM, Petr Spacek wrote:
> Hello,
> 
> Remove dead code in ipaserver/install/installutils: read_ip_address().
> 
> git grep claims that nothing is calling this function.
> 
> 
> 

ACK. This should have been removed with the
26c1851e98f31f11cf52b36bcb7e399ccbb2af17.

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

[Freeipa-devel] [PATCHES] PoC: improve CLI and config option processing for installer

[Martin Babinsky]

these patches are a rough PoC of a new machinery that abstracts option 
handling/processing for installers ported on Jan's new installer 
framework and also implement https://fedorahosted.org/freeipa/ticket/4517


The code is admittedly ugly but functional and demonstrated the base 
architecture of the module. The user-facing entrypoints are the 
FrontendManager function call and the individual Frontend 
implementations which handle processing of options.


Here is a short example of the usage:

"""
# we want the Server installer to accept options from both CLI and
# config file

ServerCLIConfig = FrontendManager(Server, ConfigFrontend, CLIFrontend)

# add frontend-specific options to external option parser
# in case of CLIFrontend, all installer knobs are fed to the parser as # 
options

# in case of ConfigFrontend, a single option is added which holds
# the location of config file
option_parser = IPAOptionParser()
ServerCLIConfig.add_options(option_parser)

# let the parser parse the command line
options, args = option_parser.parse_args(sys.argv[1:])

# get and validate all CLI/config options, instantiate the Server
# configurator and run installation

ServerCLIConfig.run(options, args)

"""

The second patch contains some basic tests for the new functionality

The third patch in the series plugs this functionality into the existing 
CLI interface in a particularly nasty but (sorta-) functional way.


I would very much appreciate any of your comments concerning the whole 
design and implementation. Suggestions to clean up the code are most 
welcome.


--
Martin^3 Babinsky
From 881e3d58d492271a4d1c16e2833f992d7258fd28 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 9 Nov 2015 18:17:52 +0100
Subject: [PATCH 3/3] POC: plug the new option handling machinery to
 ipa-{server,replica}-install

This patch demonstrate the usage of the novel machinery for option processing
in the ipa-server-install and ipa-replica-install commands.

https://fedorahosted.org/freeipa/ticket/4517
---
 install/tools/ipa-replica-install |   7 +-
 install/tools/ipa-server-install  |   4 +-
 ipapython/install/cli.py  | 212 --
 3 files changed, 25 insertions(+), 198 deletions(-)

diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 60a853b419e503b5fb7fb1048034feb5c17afa02..c2f446aae011efbd12aaa593d2c9dd9a41d14d2a 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -18,15 +18,16 @@
 # along with this program.  If not, see .
 #
 
-from ipapython.install import cli
+from ipapython.install import cli, config
 from ipaplatform.paths import paths
 from ipaserver.install.server import Replica
 
 
 ReplicaInstall = cli.install_tool(
-Replica,
+config.FrontendManager(Replica, config.ConfigFileFrontend,
+   config.CLIFrontend,
+   positional_arguments=['replica_file']),
 command_name='ipa-replica-install',
-positional_arguments='replica_file',
 usage='%prog [options] REPLICA_FILE',
 log_file_name=paths.IPAREPLICA_INSTALL_LOG,
 debug_option=True,
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 9fc78b538889fdfaca91d633e85398872c0fed0f..2cbd6b3c1fb3b807fb21f89ba2de670ca24e9973 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -21,12 +21,14 @@
 #
 
 from ipapython.install import cli
+from ipapython.install import config
 from ipaplatform.paths import paths
 from ipaserver.install.server import Server
 
 
 ServerInstall = cli.install_tool(
-Server,
+config.FrontendManager(Server, config.ConfigFileFrontend,
+   config.CLIFrontend),
 command_name='ipa-server-install',
 log_file_name=paths.IPASERVER_INSTALL_LOG,
 debug_option=True,
diff --git a/ipapython/install/cli.py b/ipapython/install/cli.py
index d2250e51650b9de1c85473399e3462f42bf8770b..d778219ae0482509851727b44f2f0f049e7cafe1 100644
--- a/ipapython/install/cli.py
+++ b/ipapython/install/cli.py
@@ -15,7 +15,7 @@ import six
 from ipapython import admintool, ipa_log_manager
 from ipapython.ipautil import CheckedIPAddress, private_ccache
 
-from . import core, common
+from . import core, common, config
 
 __all__ = ['install_tool', 'uninstall_tool']
 
@@ -23,8 +23,8 @@ if six.PY3:
 long = int
 
 
-def install_tool(configurable_class, command_name, log_file_name,
- positional_arguments=None, usage=None, debug_option=False,
+def install_tool(frontend, command_name, log_file_name,
+ usage=None, debug_option=False,
  use_private_ccache=True,
  uninstall_log_file_name=None,
  uninstall_positional_arguments=None, uninstall_usage=None):
@@ -32,10 +32,9 @@ def install_tool(configurable_class, command_name, log_file_name,
 

[Freeipa-devel] [PATCH 0381] admintool: Remove the option to override the log file

[Tomas Babej]

Hi,

This has been rarely used, and can be replaced by proper shell output
redirection.

https://fedorahosted.org/freeipa/ticket/5408
From 98ecd2447b1552b0605e19dc8c245bfa6b64 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Tue, 10 Nov 2015 14:20:45 +0100
Subject: [PATCH] admintool: Remove the option to override the log file

This has been rarely used, and can be replaced by proper shell output
redirection.

https://fedorahosted.org/freeipa/ticket/5408
---
 install/tools/man/ipa-kra-install.1|  3 ---
 install/tools/man/ipa-server-upgrade.1 |  3 ---
 ipapython/admintool.py | 17 ++---
 3 files changed, 6 insertions(+), 17 deletions(-)

diff --git a/install/tools/man/ipa-kra-install.1 b/install/tools/man/ipa-kra-install.1
index e3133eee188af0b613fca76b51d2f5b4f8d1ba7d..5bda4fe4b80947588ad7afde2c9f8fd81f320614 100644
--- a/install/tools/man/ipa-kra-install.1
+++ b/install/tools/man/ipa-kra-install.1
@@ -47,9 +47,6 @@ Enable debug output when more verbose output is needed
 .TP
 \fB\-q\fR, \fB\-\-quiet\fR
 Output only errors
-.TP
-\fB\-v\fR, \fB\-\-log-file\fR=\fFILE\fR
-Log to the given file
 .SH "EXIT STATUS"
 0 if the command was successful
 
diff --git a/install/tools/man/ipa-server-upgrade.1 b/install/tools/man/ipa-server-upgrade.1
index cbbdc590171bff0a88b67bcf1de961fd783ac35c..7f06e09fc4d181fa9a89772e7285d4a6567bc361 100644
--- a/install/tools/man/ipa-server-upgrade.1
+++ b/install/tools/man/ipa-server-upgrade.1
@@ -36,9 +36,6 @@ Print debugging information
 \fB\-q\fR, \fB\-\-quiet\fR
 Output only errors
 .TP
-\fB-\-log-file=FILE\fR
-Log to given file
-.TP
 
 .SH "EXIT STATUS"
 0 if the command was successful
diff --git a/ipapython/admintool.py b/ipapython/admintool.py
index e40f300b1318b7325eb2b39ec83970753d39c406..0c8a5d54676fac0704202cf183990115978cebed 100644
--- a/ipapython/admintool.py
+++ b/ipapython/admintool.py
@@ -113,8 +113,6 @@ class AdminTool(object):
 action="store_true", help="alias for --verbose (deprecated)")
 group.add_option("-q", "--quiet", dest="quiet", default=False,
 action="store_true", help="output only errors")
-group.add_option("--log-file", dest="log_file", default=None,
-metavar="FILE", help="log to the given file")
 parser.add_option_group(group)
 
 @classmethod
@@ -208,9 +206,8 @@ class AdminTool(object):
 :param _to_file: Setting this to false will disable logging to file.
 For internal use.
 
-If the --log-file option was given or if a filename is in
-self.log_file_name, the tool will log to that file. In this case,
-all messages are logged.
+If self.log_file_name is set, the tool will log to that file.
+In this case, all messages are logged.
 
 What is logged to the console depends on command-line options:
 the default is INFO; --quiet sets ERROR; --verbose sets DEBUG.
@@ -232,12 +229,8 @@ class AdminTool(object):
 self._setup_logging(log_file_mode=log_file_mode)
 
 def _setup_logging(self, log_file_mode='w', no_file=False):
-if no_file:
-log_file_name = None
-elif self.options.log_file:
-log_file_name = self.options.log_file
-else:
-log_file_name = self.log_file_name
+log_file_name = None if no_file else self.log_file_name
+
 if self.options.verbose:
 console_format = '%(name)s: %(levelname)s: %(message)s'
 verbose = True
@@ -249,10 +242,12 @@ class AdminTool(object):
 verbose = False
 else:
 verbose = True
+
 ipa_log_manager.standard_logging_setup(
 log_file_name, console_format=console_format,
 filemode=log_file_mode, debug=debug, verbose=verbose)
 self.log = ipa_log_manager.log_mgr.get_logger(self)
+
 if log_file_name:
 self.log.debug('Logging to %s' % log_file_name)
 elif not no_file:
-- 
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