Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
On 08/09/14 17:56, Martin Basti wrote: On 02/09/14 16:55, David Kupka wrote: The patch now depends on freeipa-dkupka-0012 as both modifies the same part of code. On 09/02/2014 10:29 AM, David Kupka wrote: Forget to add str() conversion to some places when removing map(). Now it should be working again. On 08/27/2014 02:24 PM, David Kupka wrote: Patch modified according to jcholast's personally-delivered feedback: 1) use action='append' instead of that ugly parsing 2) do not use map(), FreeIPA doesn't like it On 08/25/2014 05:04 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/3575 Also should fix https://bugzilla.redhat.com/show_bug.cgi?id=1128380 as installation is no longer interrupted when multiple IPs are resolved. But it does not add the option to change the IP address during second run. I haven't tested it yet, I only take a look because there may be conflict with 'dns root zone support' refactoring 1) +for ns_ip_address in nameserver_ip_address: +add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup, +ns_hostname=api.env.host, ns_ip_address=ns_ip_address, +force=True) Are you sure this will work? Domain name is the same, so no new zone will be created (DuplicateEntry exception is handled inside add_zone function). IMO you should call add_zone only once. BTW: I will change the add_zone function in refactoring , ns_hostname wil be remove, and ns_ip_address will take an p+ipv6 address 2) +resolv_txt = '' +for ip_address in self.ip_address: +resolv_txt += search +self.domain+\nnameserver +str(ip_address)+\n There is multiple search statements. search example.com nameserver 192.168.1.1 search example.com nameserver 2001:db8::1 ... and also there si a limit of namesevers which can be in resolv.conf, but I dont know if we care, statements over limit should be just ignored. http://linux.die.net/man/5/resolv.conf 3) self.ip_address is confusing for me, I'm expecting only one address. Could it be ip_addresses or ip_address_list? Ask the framework gurus :-) We might add support for more ip addres for replica install too ipa-replica-prepare replica.example.com --ip-address=192.168.1.2 --ip-address=2001:db8::2 -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0083 Remove internaldb pasword from password.conf
On 09/02/2014 11:37 AM, Jan Cholasta wrote: Patch attached. Dne 2.9.2014 v 09:03 Jan Cholasta napsal(a): Also, Dogtag certificate renewal does not work with internaldb removed, I'm working on a patch to fix that. Dne 1.9.2014 v 18:19 Petr Viktorin napsal(a): On 11/06/2013 01:41 PM, Ana Krivokapic wrote: On 11/06/2013 01:34 PM, Ana Krivokapic wrote: Hello, This patch addresses tickethttps://fedorahosted.org/freeipa/ticket/4005. ACK to both patches, pushed to: master: 3acec1267ea3e1f4faa8757d384386d8035dd6cf ipa-4-1: be4d5bf86336825aed2a9038ebc5caff713d6b0a -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0032] Hardcoded lib dir in freeipa.spec
On 09/08/2014 04:10 PM, Gabe Alford wrote: Hello, This patch should fix https://fedorahosted.org/freeipa/ticket/4528 Thanks, Gabe Thank you! ACK, pushed to: master: 8cb27bfa4fe73fa4c236f5e7d9591a28ee064f2b ipa-4-1: ce86e5d874b86a5118a84459e0624f61c49210d6 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Fwd: [freeipa] update to Java/8
On 5.9.2014 12:35, Martin Kosek wrote: Petr, why do we require java-1.7.0-openjdk in BuildRequires anyway? Shouldn't rhino be enough? I don't think that rhino pulls Java. The correct patch would be the one which we already used upstream: http://www.redhat.com/archives/freeipa-devel/2014-August/msg00111.html Basically replace java-1.7.0-openjdk with java-headless Original Message Subject: [freeipa] update to Java/8 Date: Tue, 2 Sep 2014 17:41:13 + (UTC) From: Pádraig Brady pbr...@fedoraproject.org To: freeipa-ow...@fedoraproject.org, scm-comm...@lists.fedoraproject.org commit c1d3c76c37530d0608f710f986be1614d2ed848b Author: Pádraig Brady p...@draigbrady.com Date: Tue Sep 2 18:40:05 2014 +0100 update to Java/8 Java/7 is no longer available in rawhide, so update to allow rebuilds to proceed. freeipa.spec |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) --- diff --git a/freeipa.spec b/freeipa.spec index f871260..2f8eac0 100644 --- a/freeipa.spec +++ b/freeipa.spec @@ -67,7 +67,7 @@ BuildRequires: m2crypto BuildRequires: check BuildRequires: libsss_idmap-devel BuildRequires: libsss_nss_idmap-devel -BuildRequires: java-1.7.0-openjdk +BuildRequires: java-1.8.0-openjdk BuildRequires: rhino BuildRequires: libverto-devel BuildRequires: systemd -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Fwd: [freeipa] update to Java/8
On 09/09/2014 01:52 PM, Petr Vobornik wrote: On 5.9.2014 12:35, Martin Kosek wrote: Petr, why do we require java-1.7.0-openjdk in BuildRequires anyway? Shouldn't rhino be enough? I don't think that rhino pulls Java. The correct patch would be the one which we already used upstream: http://www.redhat.com/archives/freeipa-devel/2014-August/msg00111.html Basically replace java-1.7.0-openjdk with java-headless This is already in 4.0.2, released to f21 updates-testing a week after Pádraig's update. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0008 Use certmonger D-Bus API instead of messing with its files.
On 05/09/14 10:59, Martin Kosek wrote:
On 09/04/2014 03:09 PM, Jan Cholasta wrote:
Dne 4.9.2014 v 13:40 Martin Kosek napsal(a):
On 09/04/2014 01:19 PM, Jan Cholasta wrote:
Dne 4.9.2014 v 12:31 David Kupka napsal(a):
On 09/03/2014 04:45 PM, Jan Cholasta wrote:
Dne 3.9.2014 v 16:25 David Kupka napsal(a):
On 09/03/2014 04:05 PM, Jan Cholasta wrote:
Dne 3.9.2014 v 12:37 David Kupka napsal(a):
On 09/02/2014 01:56 PM, Jan Cholasta wrote:
Dne 29.8.2014 v 14:34 David Kupka napsal(a):
Hope, I've addressed all the issues (except 9 and 11, inline).
Let's go
for another round :-)
On 08/27/2014 11:05 AM, Jan Cholasta wrote:
Hi,
Dne 25.8.2014 v 15:39 David Kupka napsal(a):
On 08/19/2014 05:44 PM, Rob Crittenden wrote:
David Kupka wrote:
On 08/19/2014 09:58 AM, Martin Kosek wrote:
On 08/19/2014 09:05 AM, David Kupka wrote:
FreeIPA will use certmonger D-Bus API as discussed in this
thread
https://www.redhat.com/archives/freeipa-devel/2014-July/msg00304.html
This change should prevent hard-to-reproduce bugs like
https://fedorahosted.org/freeipa/ticket/4280
Thanks for this effort, the updated certmonger module looks
much
better! This
will help us get rid of the non-standard communication with
certmonger.
Just couple initial comments from me by reading the code:
1) Testing needs fixed version of certmonger, right? This needs
to be
spelled
out right with the patch.
Yes, certmonger 0.75.13 and above should be fine according
ticket
https://fedorahosted.org/certmonger/ticket/36. Added to patch
description.
You should update the spec to set the minimum version as well.
Sure, thanks.
2) Description text in patches is cheap, do not be afraid to
use it
and
describe what you did and why. Link to the ticket is missing in
the
description
as well:
Ok, increased verbosity a bit :-)
Subject: [PATCH] Use certmonger D-Bus API instead of messing
with
its
files.
---
3) get_request_id API:
criteria = (
-('cert_storage_location',
dogtag_constants.ALIAS_DIR,
- certmonger.NPATH),
-('cert_nickname', nickname, None),
+('cert_storage_location',
dogtag_constants.ALIAS_DIR),
+('cert_nickname', nickname),
)
request_id = certmonger.get_request_id(criteria)
Do we want to continue using the criteria object or should we
rather
switch
to normal function options? I.e. rather using
request_id = certmonger.get_request_id(cert_nickname=nickname,
cert_storage_location=dogtag_constants.ALIAS_DIR)
? It would look more consistent with other calls. I am just
asking,
not insisting.
I've no preference here. It seems to be a very small change. Has
anyone
a reason to do it one way and not the other?
I think I used this criteria thing to avoid having a bazillion
optional
parameters and for future-proofing. I think at this point the
list is
probably pretty stable, so I'd base it on whether you care about
having
a whole ton of optional parameters or not (it has the
advantage of
self-documenting itself).
The list is probably stable but also really excessive. I don't
think it
would help to have more than dozen optional parameters. So I
prefer to
leave as-is and change it in future if it is wanted.
3) Starting function:
+try:
+ipautil.run([paths.SYSTEMCTL, 'start', 'certmonger'],
skip_output=True)
+except Exception, e:
+root_logger.error('Failed to start certmonger: %s'
% e)
+raise e
I see 2 issues related to this code:
a) Do not call SYSTEMCTL directly. To be platform independent,
rather use
services.knownservices.messagebus.start() that is
overridable by
someone else
porting to non-systemd platforms.
Is there anything that can't be done using
ipalib/ipapython/ipaplatform?
It can't make coffee (yet).
b) In this case, do not use raise e, but just raise to keep
the
exception
stack trace intact for better debugging.
Every day there's something new to learn about python or
FreeIPA.
Both a) and b) should be fixed in other occasions and places.
I found only one occurence of a) issue. Is there some hidden or
are
you
talking about the whole FreeIPA project?
4) Feel free to add yourself to Authors section of this module.
You
refactored
it greatly to earn it :-)
Done.
You already import dbus, why also separately import
DBusException?
Removed, thanks for noticing.
rob
1) The patch needs to be rebased.
I didn't notice the patch is targeted for 4.0. Can you please provide
patches for both ipa-4-0 and ipa-4-1/master?
Attached, 0008-5 works on master/ipa-4-1 and 0008-5-ipa40 works on
ipa-4-0.
There is a little bug in ipa-upgradeconfig in the 4.0 version of the
patch. This is wrong:
for request in requests:
nss_dir, nickname, ca_name, pre_command, post_command, profile
= request
criteria = {
'cert-database': nss_dir,
'cert-nickname': nickname,
'ca-name': ca_name,
'template-profile': profile,
Re: [Freeipa-devel] [PATCH] 720-729 OTP usability improvements
On 3.9.2014 21:44, Endi Sukma Dewata wrote:
On 8/22/2014 3:31 AM, Petr Vobornik wrote:
On 12.8.2014 17:59, Endi Sukma Dewata wrote:
On 8/5/2014 6:31 AM, Petr Vobornik wrote:
ticket: https://fedorahosted.org/freeipa/ticket/4402
snip
[PATCH] 727 webui: hide empty fields and sections
Hide widgets without a value. Must be explicitly turned on. In
widget by
`hidden_if_empty` flag. Or globally by `hide_empty_widgets` flag.
Global
hiding can be individually turned off by `ignore_empty_hiding` flag.
In item #2 of ticket #4402 I was originally thinking the widget
visibility would be determined by the token type.
Originally I wrote it that way but with this patch it became redundant.
Any plan to add the
token type field in the future?
maybe, I don't have any strong feelings about it. Will users benefit
from adding it? If yes, it should be also added to CLI.
Can the users tell which type of token they have based on the existing
fields? There is a model field which is populated with the type (e.g.
totp or hotp). But since the value can be changed to anything it's not a
reliable way to determine the type. I don't think it's very
user-friendly to ask the user to see whether the type-specific fields
are shown in order to determine the type.
I can't say there's a big benefit by adding the field, but maybe some
admins might find it useful, and it can be used to sort/filter out
search results.
Atm. token type is derived from object classes. It exists only in add
operation as a virtual attribute. We can check a presence of
ipatokenhotp or ipatokentotp object classes to simulate the field.
Yes, if people can add an attribute usually they will expect to be able
to see what they added.
The issue is that it's not an attribute per se.
Nathaniel: do you think we should add a virtual output param(computed
from object class) for the token type since 'model' could be easily
overwritten? I would prefer to do it both in Web UI and CLI.
Will the counter field strictly have a
value with HOTP only and clock offset interval fields strictly have
value with TOTP only? Do these fields contain the configured values or
the effective values? I was just thinking maybe in the future some of
these fields might be configured empty and they will use the default
values instead. If that's not a problem then ACK.
Respective fields are present only in corresponding object classes -
there won't be an HTOP token with 'clock offset'. If they are present,
they have a default value. - No false hiding - Shouldn't be a problem.
Btw: Other of my other original intents was to hide it based on ACI
rights. The issues is that the rights are not present without
corresponding OC. Hiding in such case is dangerous - explicitly disabled
in patch 728.
What do you think about setting `hide_empty_widgets` global setting to
True?
On a read-only page I think it's OK to hide empty fields. But on edit
page I don't think we should do that by default.
Maybe we should first clarify what is a read-only page. All details
pages are writable if user has the rights or read-only if he doesn't.
But mostly it depends on individual fields/attributes.
Does empty mean unset, or blank like empty string/array? Does
unset always mean non-editable and invisible and blank always mean
editable and visible but it's just empty? If this definition isn't
strictly followed there could be an editable field that accidentally
gets hidden because it's empty.
My intent was to show everything what user can edit. And hide fields:
1. for which he doesn't have read rights
2. have no (undefined) or empty('') value
3. are explicitly hidden by other logic - not related to this patch
When thinking about #1. Maybe we should base it on a presence or rather
not presence of a specific objectclass, rather than on a value. That way
it would be less confusing for newcomers. The logic is: if fields is
bound to an object class and the class is not present (that also means
we don't have attribute level rights) - hide it.
#2 is questionable. IMHO it would require user tests. Also hiding on ''
value might not be always desirable. Nevertheless I like the behavior
but it may be caused by the fact that I already know what to expect.
Generally if a field is supposed to be hidden in an edit page because of
other condition (e.g. token type), not because of the value, the code
should also use that condition instead of relying on the value being
unset. In this particular case, instead of:
if (field !== undefined) {
// display field
}
we probably should do:
var type = ...
if (type == 'hotp') {
// display hotp fields
} else { // totp
// display totp fields
}
I agree - use case #3.
For now the type can be determined by the availability of certain
type-specific fields, but in the future we may add the type itself.
btw somewhere is a rebase issue which leads too:
if (that.readable !== undefined) {
Re: [Freeipa-devel] [PATCH] 720-729 OTP usability improvements
On Tue, 2014-09-09 at 17:48 +0200, Petr Vobornik wrote: Nathaniel: do you think we should add a virtual output param(computed from object class) for the token type since 'model' could be easily overwritten? I would prefer to do it both in Web UI and CLI. That seems reasonable to me. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
