Re: [Freeipa-devel] [PATCH] 068 Connection check program for replica installation

2011-05-27 Thread Rob Crittenden 

Martin Kosek wrote:

On Mon, 2011-05-23 at 16:41 -0400, Rob Crittenden wrote:

Martin Kosek wrote:

This is a first version of connection checking program for replica
installation. See patch for program purpose description. Currently,
there is no man pages for the program.

Note to Simo and Rob: I use password for logging as admin. Btw would it
be safe to have an admin keytab in the replica file? Replica file
contents are lying freely in /tmp after the replica installation.

Martin


nack, you aren't including the new binary in the spec.


Oh, thanks for this one.



You should also:

- set KRB5CCNAME to a temporary ccache and remove that when the install
exists (successful or not)


Done.


- remove the temporary krb5.conf you create


Done.


- be a bit more explicit what we are doing, at least more than "Run
connection check to master".


Actually, I am if you run the new script separately. I removed "--quiet"
parameter passed to the script in ipa-replica-install so that it is more
verbose. Plus, I improved texts sent to the user.


- yes, we should remove the replica file contents


I enhanced ipa-replica-install to do that.

Martin



Works great until the very end:
...
...

Execute check on remote master
Check connection from master to remote replica 'slinky.greyoak.com':
   Directory Service: unsecure port (389): FAILED
   Directory Service: secure port (636): FAILED
   Kerberos (88): OK

Remote master check failed with following error message(s):
Could not chdir to home directory /home/admin: No such file or directory
Port check failed! Unaccessible port(s): 389, 636

Connection check failed with following error: None

rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 168 Fixed problem deleting value in text field.

2011-05-27 Thread Adam Young 

On 05/27/2011 01:44 PM, Endi Sukma Dewata wrote:

Previously deleting a value in a text field did not work because
the field is not included in the modify operation when the value
is empty. The details facet's update() method has been modified
to update only dirty fields.

The section lists in details facet and dialog have been converted
into ordered maps.

Ticket #1256


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

ACK.  Pushed to master
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin

2011-05-27 Thread Martin Kosek 
On Fri, 2011-05-27 at 16:25 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > On Thu, 2011-05-26 at 22:39 -0400, Rob Crittenden wrote:
> >> Martin Kosek wrote:
> >>> Interactive mode for commands manipulating with DNS records
> >>> (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances
> >>> the server framework with new callback for interactive mode, which
> >>> can be used by commands to inject their own interactive handling.
> >>>
> >>> The callback is then used to improve aforementioned commands'
> >>> interactive mode.
> >>>
> >>> https://fedorahosted.org/freeipa/ticket/1018
> >>
> >> This works pretty nicely but it seems like with just a bit more it can
> >> be great.
> >>
> >> Can you add some doc examples for how this works?
> >
> > Done. At least user will know that we have a feature like that to offer.
> >
> >>
> >> And you display the records now and then prompt for each to delete. Can
> >> you combine the two?
> >>
> >> For example:
> >>
> >> ipa dnsrecord-del greyoak.com lion
> >> No option to delete specific record provided.
> >> Delete all? Yes/No (default No):
> >> Current DNS record contents:
> >>
> >> A record: 192.168.166.32
> >>
> >> Enter value(s) to remove:
> >> [A record]:
> >>
> >> If we know there is an record why not just prompt for each value yes/no
> >> to delete?
> >
> > Actually, this is a very good idea, I like it. I updated the patch so
> > that the user can only do yes/no decision in ipa dnsrecord-del
> > interactive mode. This makes dnsrecord-del interactive mode very usable.
> >
> >>
> >> The yes/no function needs more documentation on what default does too.
> >> It appears that the possible values are None/True/False and that None
> >> means that '' can be returned (which could still be evaluated as False
> >> if this isn't used right).
> >
> > Done. '' shouldn't be returned as I return the value of "default" if it
> > is not None. But yes, it needed more documenting.
> >
> > Updated patch is attached. It may need some language corrections, I am
> > no native speaker.
> >
> > Martin
> 
> Not to be too pedantic but...
> 
> The result variable isn't really used, a while True: would suffice.
> 
> I'm not really sure what the purpose of default = None is. I think a 
> True/False is more appropriate, this 3rd answer of a binary question is 
> confusing.

I fixed the result variable. This was a left-over from function
evolution.

I am not sure why is the yes/no function still confusing. Maybe I miss
something. I improved function help a bit. But let me explain:

If default is None, that means that there is no default answer to yes/no
question and user has to answer either "y" or "n". He cannot skip the
answer and is prompted until the answer is given.

When default is True, user can just enter empty answer, which is treated
as "yes" and True is returned.

When default is False and user enters empty answer, it is treated as
"no" and False is returned.

None shouldn't be returned at all... (Maybe only in a case of an error)

Martin

>From 13621b72cc9d45128e1438d7decf472f14eeb3e1 Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Thu, 26 May 2011 09:55:53 +0200
Subject: [PATCH] Improve interactive mode for DNS plugin

Interactive mode for commands manipulating with DNS records
(dnsrecord-add, dnsrecord-del) is not usable. This patch enhances
the server framework with new callback for interactive mode, which
can be used by commands to inject their own interactive handling.

The callback is then used to improve aforementioned commands'
interactive mode.

https://fedorahosted.org/freeipa/ticket/1018
---
 ipalib/cli.py  |   44 
 ipalib/plugins/baseldap.py |   42 
 ipalib/plugins/dns.py  |  159 ++--
 3 files changed, 225 insertions(+), 20 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 99f236bb4103c8524320b03aa4a311689ecef8e8..5e1365dc37c290eeeb38aa6266a9798854a132ee 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -527,6 +527,47 @@ class textui(backend.Backend):
 return None
 return self.decode(data)
 
+def prompt_yesno(self, label, default=None):
+"""
+Prompt user for yes/no input. This method returns True/False according
+to user response.
+
+Parameter "default" should be True, False or None
+
+If Default parameter is not None, user can enter an empty input instead
+of Yes/No answer. Value passed to Default is returned in that case.
+
+If Default parameter is None, user is asked for Yes/No answer until
+a correct answer is provided. Answer is then returned.
+
+In case of an error, a None value may returned
+"""
+
+default_prompt = None
+if default is not None:
+if default:
+default_prompt = "Yes"
+else:
+default_prompt = "No"
+
+if default_prompt:
+prompt = u'%s Yes/No (d

Re: [Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin

2011-05-27 Thread Rob Crittenden 

Martin Kosek wrote:

On Thu, 2011-05-26 at 22:39 -0400, Rob Crittenden wrote:

Martin Kosek wrote:

Interactive mode for commands manipulating with DNS records
(dnsrecord-add, dnsrecord-del) is not usable. This patch enhances
the server framework with new callback for interactive mode, which
can be used by commands to inject their own interactive handling.

The callback is then used to improve aforementioned commands'
interactive mode.

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


This works pretty nicely but it seems like with just a bit more it can
be great.

Can you add some doc examples for how this works?


Done. At least user will know that we have a feature like that to offer.



And you display the records now and then prompt for each to delete. Can
you combine the two?

For example:

ipa dnsrecord-del greyoak.com lion
No option to delete specific record provided.
Delete all? Yes/No (default No):
Current DNS record contents:

A record: 192.168.166.32

Enter value(s) to remove:
[A record]:

If we know there is an record why not just prompt for each value yes/no
to delete?


Actually, this is a very good idea, I like it. I updated the patch so
that the user can only do yes/no decision in ipa dnsrecord-del
interactive mode. This makes dnsrecord-del interactive mode very usable.



The yes/no function needs more documentation on what default does too.
It appears that the possible values are None/True/False and that None
means that '' can be returned (which could still be evaluated as False
if this isn't used right).


Done. '' shouldn't be returned as I return the value of "default" if it
is not None. But yes, it needed more documenting.

Updated patch is attached. It may need some language corrections, I am
no native speaker.

Martin


Not to be too pedantic but...

The result variable isn't really used, a while True: would suffice.

I'm not really sure what the purpose of default = None is. I think a 
True/False is more appropriate, this 3rd answer of a binary question is 
confusing.


rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 3 Add ability to specify netmask with IP addresses during installation

2011-05-27 Thread Jan Cholasta 

On 27.5.2011 16:49, Jan Cholasta wrote:

On 20.5.2011 20:29, Jan Cholasta wrote:

On 12.5.2011 14:47, Jan Cholasta wrote:


Rewrote host.py so that it doesn't use get_reverse_zone from
ipaserver.bindinstance (which fixes the pylint errors).

Honza



Patch updated. Requires patch 18.1.



Another update, requires patch 18.3.

Honza



Updated, requires 18.4.

--
Jan Cholasta
>From 43acc55fc0acc45746cb2605a730974c74ae0c94 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Fri, 27 May 2011 20:29:33 +0200
Subject: [PATCH] Honor netmask in DNS reverse zone setup.

ticket 910
---
 install/tools/ipa-dns-install |3 +-
 install/tools/ipa-replica-install |6 +++-
 install/tools/ipa-replica-prepare |   34 
 install/tools/ipa-server-install  |3 +-
 ipalib/plugins/host.py|   45 
 ipaserver/install/bindinstance.py |   52 -
 6 files changed, 98 insertions(+), 45 deletions(-)

diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index e837919..91edcca 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -113,6 +113,7 @@ def main():
 sys.exit("Unable to resolve IP address for host name")
 else:
 ip_address = read_ip_address(api.env.host, fstore)
+ip_prefixlen = ip_address.prefixlen
 ip_address = str(ip_address)
 logging.debug("will use ip_address: %s\n", ip_address)
 
@@ -158,7 +159,7 @@ def main():
 create_reverse = not options.no_reverse
 elif not options.no_reverse:
 create_reverse = bindinstance.create_reverse()
-bind.setup(api.env.host, ip_address, api.env.realm, api.env.domain, dns_forwarders, conf_ntp, create_reverse, zonemgr=options.zonemgr)
+bind.setup(api.env.host, ip_address, ip_prefixlen, api.env.realm, api.env.domain, dns_forwarders, conf_ntp, create_reverse, zonemgr=options.zonemgr)
 
 if bind.dm_password:
 api.Backend.ldap2.connect(bind_dn="cn=Directory Manager", bind_pw=bind.dm_password)
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 6df5123..2848366 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -272,6 +272,7 @@ def install_bind(config, options):
 sys.exit("Unable to resolve IP address for host name")
 ip = installutils.parse_ip_address(ip_address)
 ip_address = str(ip)
+ip_prefixlen = ip.prefixlen
 
 create_reverse = True
 if options.unattended:
@@ -285,7 +286,7 @@ def install_bind(config, options):
 # specified, ask the user
 create_reverse = bindinstance.create_reverse()
 
-bind.setup(config.host_name, ip_address, config.realm_name,
+bind.setup(config.host_name, ip_address, ip_prefixlen, config.realm_name,
config.domain_name, forwarders, options.conf_ntp, create_reverse)
 bind.create_instance()
 
@@ -309,8 +310,9 @@ def install_dns_records(config, options):
 sys.exit("Unable to resolve IP address for host name")
 ip = installutils.parse_ip_address(ip_address)
 ip_address = str(ip)
+ip_prefixlen = ip.prefixlen
 
-bind.add_master_dns_records(config.host_name, ip_address,
+bind.add_master_dns_records(config.host_name, ip_address, ip_prefixlen,
 config.realm_name, config.domain_name,
 options.conf_ntp)
 
diff --git a/install/tools/ipa-replica-prepare b/install/tools/ipa-replica-prepare
index 21f30f0..2765e4a 100755
--- a/install/tools/ipa-replica-prepare
+++ b/install/tools/ipa-replica-prepare
@@ -27,7 +27,7 @@ import krbV
 
 from ipapython import ipautil
 from ipaserver.install import bindinstance, dsinstance, installutils, certs
-from ipaserver.install.bindinstance import add_zone, add_reverse_zone, add_rr, add_ptr_rr
+from ipaserver.install.bindinstance import add_zone, add_reverse_zone, add_fwd_rr, add_ptr_rr, dns_zone_exists
 from ipaserver.install.replication import check_replication_plugin, enable_replication_version_checking
 from ipaserver.install.installutils import resolve_host
 from ipaserver.plugins.ldap2 import ldap2
@@ -426,12 +426,34 @@ def main():
 name = domain.pop(0)
 domain = ".".join(domain)
 
-ip_address = str(options.ip_address)
+ip = options.ip_address
+ip_address = str(ip)
+ip_prefixlen = ip.prefixlen
+
+if ip.defaultnet:
+revzone = ip.reverse_dns
+if ip.version == 4:
+prefix = 32
+dec = 8
+elif ip.version == 6:
+prefix = 128
+dec = 4
+
+while prefix > 0:
+dummy, dot, revzone = revzone.partition('.')
+prefix = prefix - dec
+if dns_zone_exists(revzone):
+break
+
+if prefix > 0:
+ip_prefixlen = prefix
+else:
+

Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-05-27 Thread Jan Cholasta 

On 27.5.2011 18:59, Martin Kosek wrote:

On Fri, 2011-05-27 at 16:47 +0200, Jan Cholasta wrote:

On 24.5.2011 15:38, Jan Cholasta wrote:

On 20.5.2011 20:27, Jan Cholasta wrote:

On 10.5.2011 20:06, Jan Cholasta wrote:

Parse netmasks in IP addresses passed to server install.

ticket 1212


Patch updated.

TODO: Write unit test for ipapython.ipautil.CheckedIPAddress
TODO: Clean unreachable code paths off of ipa-server-install (?)
TODO: Workarounds for netaddr bugs (?)



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Fixed ipa-replica-prepare and added a unit test.



Another update.

Honza


Can you please rebase your patches? My patch 070 fixing
add_reverse_zone() function was pushed today. Unfortunately, it made
your patches 18 and 3 not applicable.


Done.



You may want to look closer at the patch 070 as it is relevant to your
patch set and also to make sure the fix is still functional after your
set of patches.


It seems it's ok.



Thanks,
Martin



Honza

--
Jan Cholasta
>From 49bc2ab0b1050f930161bc525f06516c9afbdacc Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Fri, 27 May 2011 20:17:22 +0200
Subject: [PATCH] Parse netmasks in IP addresses passed to server install.

ticket 1212
---
 freeipa.spec.in  |1 +
 install/tools/ipa-dns-install|9 +++--
 install/tools/ipa-replica-install|6 +++-
 install/tools/ipa-replica-prepare|   17 +
 install/tools/ipa-server-install |   36 +--
 ipapython/config.py  |   13 ++-
 ipapython/ipautil.py |   67 ++
 ipaserver/install/installutils.py|   39 +---
 tests/test_ipapython/__init__.py |   22 +++
 tests/test_ipapython/test_ipautil.py |   56 
 10 files changed, 213 insertions(+), 53 deletions(-)
 create mode 100644 tests/test_ipapython/__init__.py
 create mode 100644 tests/test_ipapython/test_ipautil.py

diff --git a/freeipa.spec.in b/freeipa.spec.in
index b936616..fba2f31 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -188,6 +188,7 @@ Requires: python-kerberos >= 1.1-3
 %endif
 Requires: authconfig
 Requires: gnupg
+Requires: iproute
 Requires: pyOpenSSL
 Requires: python-nss >= 0.11
 Requires: python-lxml
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index a763297..e837919 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -37,9 +37,10 @@ def parse_options():
   sensitive=True, help="admin password")
 parser.add_option("-d", "--debug", dest="debug", action="store_true",
   default=False, help="print debugging information")
-parser.add_option("--ip-address", dest="ip_address", help="Master Server IP Address")
+parser.add_option("--ip-address", dest="ip_address",
+  type="ipnet", help="Master Server IP Address")
 parser.add_option("--forwarder", dest="forwarders", action="append",
-  help="Add a DNS forwarder")
+  type="ipaddr", help="Add a DNS forwarder")
 parser.add_option("--no-forwarders", dest="no_forwarders", action="store_true",
   default=False, help="Do not add any DNS forwarders, use root servers instead")
 parser.add_option("--no-reverse", dest="no_reverse",
@@ -105,12 +106,14 @@ def main():
 if options.ip_address:
 ip_address = options.ip_address
 else:
-ip_address = resolve_host(api.env.host)
+hostaddr = resolve_host(api.env.host)
+ip_address = hostaddr and ipautil.CheckedIPAddress(hostaddr)
 if not ip_address or not verify_ip_address(ip_address):
 if options.unattended:
 sys.exit("Unable to resolve IP address for host name")
 else:
 ip_address = read_ip_address(api.env.host, fstore)
+ip_address = str(ip_address)
 logging.debug("will use ip_address: %s\n", ip_address)
 
 if options.no_forwarders:
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 293a0a0..6df5123 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -61,7 +61,7 @@ def parse_options():
 parser.add_option("--setup-dns", dest="setup_dns", action="store_true",
   default=False, help="configure bind with our zone")
 parser.add_option("--forwarder", dest="forwarders", action="append",
-  help="Add a DNS forwarder")
+  type="ipaddr", help="Add a DNS forwarder")
 parser.add_option("--no-forwarders", dest="no_forwarders", action="store_true",
   default=False, help="Do not add any DNS forwarders, use root servers instead")
 parser.add_option("--no-reverse", dest="no_reverse", action="store_true",
@@ -270,6 +270,8 @@ de

Re: [Freeipa-devel] [PATCH] 762 Let the framework be able to override the hostname

2011-05-27 Thread Rob Crittenden 

Martin Kosek wrote:

On Wed, 2011-05-25 at 11:29 -0400, Rob Crittenden wrote:

Martin Kosek wrote:

On Fri, 2011-04-01 at 11:47 -0400, Rob Crittenden wrote:

The hostname is passed in during the server installation. We should use
this hostname for the resulting server as well. It was being discarded
and we always used the system hostname value.

ticket 1052

rob


I have to NACK this again. I have a problem communicating with IPA on a
master machine. I reproduced in on 2 different machines. Please, correct
my steps if I am wrong, I do the following procedure

1) I prepare a fresh minimal F-15
2) Install freeipa-server (current master with your patches)
3) Add custom hostname to /etc/hosts
4) Install IPA server:
ipa-server-install -p secret123 -a secret123 --hostname 
ipa.idm.lab.bos.redhat.com --setup-dns --forwarder=10.16.255.2
5) # kinit admin
Password for ad...@idm.lab.bos.redhat.com:
6) # ipa user-show admin
ipa: ERROR: cannot connect to 'any of the configured servers':
https://ipa.idm.lab.bos.redhat.com/ipa/xml,
https://ipa.idm.lab.bos.redhat.com/ipa/xml

# ping -c 1 ipa.idm.lab.bos.redhat.com
PING ipa.idm.lab.bos.redhat.com (10.16.78.140) 56(84) bytes of data.
64 bytes from ipa.idm.lab.bos.redhat.com (10.16.78.140): icmp_req=1
ttl=64 time=0.049 ms

Apache error_log shows relevant errors:

[Wed May 25 06:42:38 2011] [error] ipa: ERROR: Failed to start IPA: Unable to 
retrieve LDAP schema: Invalid credentials: SASL(-1): generic failure: GSSAPI 
Error: Unspecified GSS failure.  Minor code may provide more information 
(Permission denied)
[Wed May 25 06:42:38 2011] [error] ipa: ERROR: Failed to start IPA: Unable to 
retrieve LDAP schema: Invalid credentials: SASL(-1): generic failure: GSSAPI 
Error: Unspecified GSS failure.  Minor code may provide more information 
(Permission denied)
[Wed May 25 06:43:55 2011] [error] Exception KeyError: KeyError(140250828974112,) 
in   ignored
[Wed May 25 06:43:55 2011] [error] Exception KeyError: KeyError(140250828974112,) 
in   ignored
[Wed May 25 06:43:55 2011] [error] Exception KeyError: KeyError(140250828974112,) 
in   ignored
[Wed May 25 06:43:55 2011] [error] Exception KeyError: KeyError(140250828974112,) 
in   ignored
[Wed May 25 06:43:55 2011] [error] Exception KeyError: KeyError(140250828974112,) 
in   ignored
[Wed May 25 06:43:55 2011] [error] Exception KeyError: KeyError(140250828974112,) 
in   ignored
[Wed May 25 06:43:55 2011] [error] Exception KeyError: KeyError(140250828974112,) 
in   ignored
[Wed May 25 06:43:55 2011] [error] Exception KeyError: KeyError(140250828974112,) 
in   ignored
[Wed May 25 06:43:55 2011] [error] Exception KeyError: KeyError(140250828974112,) 
in   ignored
[Wed May 25 06:43:55 2011] [error] Exception KeyError: KeyError(140250828974112,) 
in   ignored
[Wed May 25 06:43:56 2011] [notice] caught SIGTERM, shutting down
[Wed May 25 06:43:56 2011] [notice] SELinux policy enabled; httpd running as 
context system_u:system_r:kernel_t:s0
[Wed May 25 06:43:57 2011] [notice] Digest: generating secret for digest 
authentication ...
[Wed May 25 06:43:57 2011] [notice] Digest: done
[Wed May 25 06:43:57 2011] [notice] Apache/2.2.17 (Unix) DAV/2 
mod_auth_kerb/5.4 mod_nss/2.2.17 NSS/3.12.9.0 mod_wsgi/3.2 Python/2.7.1 
configured -- resuming normal operations
[Wed May 25 06:44:04 2011] [error] ipa: INFO: *** PROCESS START ***
[Wed May 25 06:44:04 2011] [error] ipa: INFO: *** PROCESS START ***
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140] mod_wsgi (pid=5192): 
Exception occurred processing WSGI script '/usr/share/ipa/wsgi.py'.
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140] Traceback (most recent 
call last):
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140]   File 
"/usr/share/ipa/wsgi.py", line 48, in application
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140] return 
api.Backend.session(environ, start_response)
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140]   File 
"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 141, in __call__
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140] 
self.create_context(ccache=environ.get('KRB5CCNAME'))
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140]   File 
"/usr/lib/python2.7/site-packages/ipalib/backend.py", line 110, in 
create_context
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140] 
self.Backend.ldap2.connect(ccache=ccache)
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140]   File 
"/usr/lib/python2.7/site-packages/ipalib/backend.py", line 62, in connect
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140] conn = 
self.create_connection(*args, **kw)
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140]   File 
"/usr/lib/python2.7/site-packages/ipalib/encoder.py", line 188, in new_f
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140] return 
f(*new_args, **kwargs)
[Wed May 25 06:45:25 2011] [error] [client 10.16.78.140]   File 
"/usr/lib/python2.7/site-packages/ipaserver/plugins/ldap2.py

Re: [Freeipa-devel] [PATCH] 789 improve label when prompting for members

2011-05-27 Thread Rob Crittenden 

Jan Cholasta wrote:

On 24.5.2011 04:33, Rob Crittenden wrote:

Include the word 'member' with autogenerated optional member labels.

There were reports of confusion over what was being prompted for,
hopefully adding member will make things clearer.

This has a big API.txt change but it is all labels so minor in nature,
just affecting the CLI.

ticket 1062

rob



ACK, works as expected.

Honza



pushed to master and ipa-2-0

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 784 limit what attributes may be modified

2011-05-27 Thread Rob Crittenden 

Martin Kosek wrote:

On Fri, 2011-05-27 at 11:10 -0400, Rob Crittenden wrote:

Martin Kosek wrote:

On Mon, 2011-05-16 at 17:46 -0400, Rob Crittenden wrote:

Add option to limit the attributes allowed in an entry.

Kerberos ticket policy can update policy in a user entry. This allowed
set/addattr to be used to modify attributes outside of the ticket policy
perview, also bypassing all validation/normalization. Likewise the
ticket policy was updatable by the user plugin bypassing all validation.

Add two new LDAPObject values to control this behavior:

limit_object_classes: only attributes in these are allowed
disallow_object_classes: attributes in these are disallowed

By default both of these lists are empty so are skipped.

ticket 744

rob


NACK. I have some concerns with this patch. In function
_check_limit_object_class:

1) You change input attribute 'attrs' by removing the items from it. If
user passes the same list of attrs to be checked and the function is run
twice, the 'attrs' parameter in second run is corrupt.

You can try it by running e.g. `ipa krbtpolicy-mod --maxrenew=24044' and
checking the value of this parameter in the function.


Good catch, updated patch attached.



2) The purpose of this statement is not clear to me:
+if len(attrs)>   0 and allow_only:
+raise errors.ObjectclassViolation(info='attribute "%(attribute)s" not 
allowed' % dict(attribute=attrs[0]))
Maybe just the exception text is misleading.


This function has 2 modes: "allow only the attributes in these
objectclasses" or "specifically deny the attributes in these
objectclasses". This enforces the first type. If when we've gone through
all the attributes there are any left over they must not be allowed so
raise an error. This is documented in the function header.


Thanks for explanation, now I get it. It all looks OK, ACK.

Martin




pushed to master and ipa-2-0

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 168 Fixed problem deleting value in text field.

2011-05-27 Thread Endi Sukma Dewata 

Previously deleting a value in a text field did not work because
the field is not included in the modify operation when the value
is empty. The details facet's update() method has been modified
to update only dirty fields.

The section lists in details facet and dialog have been converted
into ordered maps.

Ticket #1256

--
Endi S. Dewata
From a099a945d47eea4e93d911f8603042912fcd2dee Mon Sep 17 00:00:00 2001
From: Endi S. Dewata 
Date: Fri, 27 May 2011 12:04:20 -0500
Subject: [PATCH] Fixed problem deleting value in text field.

Previously deleting a value in a text field did not work because
the field is not included in the modify operation when the value
is empty. The details facet's update() method has been modified
to update only dirty fields.

The section lists in details facet and dialog have been converted
into ordered maps.

Ticket #1256
---
 install/ui/add.js|5 +-
 install/ui/details.js|  119 --
 install/ui/dialog.js |   36 +++-
 install/ui/hbac.js   |   42 ++---
 install/ui/ipa.js|9 +++
 install/ui/sudo.js   |   35 ++-
 install/ui/test/details_tests.js |   10 ++-
 7 files changed, 143 insertions(+), 113 deletions(-)

diff --git a/install/ui/add.js b/install/ui/add.js
index 0df0db61279016752c44ae04300cf9be9162ce83..73a423f00744394241638acceeb0dfa315af40cf 100644
--- a/install/ui/add.js
+++ b/install/ui/add.js
@@ -128,8 +128,9 @@ IPA.add_dialog = function (spec) {
 }
 }
 
-for (var j=0; j', {
 name: section.name,
@@ -450,8 +455,9 @@ IPA.details_facet = function(spec) {
 
 var details = $('div[name=details]', that.container);
 
-for (var i = 0; i < that.sections.length; ++i) {
-var section = that.sections[i];
+var sections = that.sections.values;
+for (var i=0; i 1){
-if (field.join) {
-modlist[field.name] = values.join(',');
-} else {
-modlist[field.name] = values;
-}
-} else if (param_info['multivalue']){
-modlist[field.name] = [];
+command.set_option(field.name, values[0]);
+} else if (field.join) {
+command.set_option(field.name, values.join(','));
+} else {
+command.set_option(field.name, values);
 }
+
 } else {
-if (values.length) attrs_wo_option[field.name] = values;
+if (values.length) {
+command.add_option('setattr', field.name+'='+values[0]);
+} else {
+command.add_option('setattr', field.name+'=');
+}
+for (var k=1; k', {
 name: section.name,
@@ -182,8 +184,9 @@ IPA.dialog = function(spec) {
 field.setup(span);
 }
 
-for (var j=0; j___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 070 Fix reverse zone creation in ipa-replica-prepare

2011-05-27 Thread Martin Kosek 
On Fri, 2011-05-27 at 11:58 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > This patch replaces Rob's patch 791.
> > ---
> > When a new reverse zone was created in ipa-replica-prepare (this
> > may happen when a new replica is from different subnet), the master
> > DNS address was corrupted by invalid A/ record. This caused
> > problems for example in installing replica.
> >
> > https://fedorahosted.org/freeipa/ticket/1223
> 
> ack

Pushed to master, ipa-2-0.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 784 limit what attributes may be modified

2011-05-27 Thread Martin Kosek 
On Fri, 2011-05-27 at 11:10 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > On Mon, 2011-05-16 at 17:46 -0400, Rob Crittenden wrote:
> >> Add option to limit the attributes allowed in an entry.
> >>
> >> Kerberos ticket policy can update policy in a user entry. This allowed
> >> set/addattr to be used to modify attributes outside of the ticket policy
> >> perview, also bypassing all validation/normalization. Likewise the
> >> ticket policy was updatable by the user plugin bypassing all validation.
> >>
> >> Add two new LDAPObject values to control this behavior:
> >>
> >> limit_object_classes: only attributes in these are allowed
> >> disallow_object_classes: attributes in these are disallowed
> >>
> >> By default both of these lists are empty so are skipped.
> >>
> >> ticket 744
> >>
> >> rob
> >
> > NACK. I have some concerns with this patch. In function
> > _check_limit_object_class:
> >
> > 1) You change input attribute 'attrs' by removing the items from it. If
> > user passes the same list of attrs to be checked and the function is run
> > twice, the 'attrs' parameter in second run is corrupt.
> >
> > You can try it by running e.g. `ipa krbtpolicy-mod --maxrenew=24044' and
> > checking the value of this parameter in the function.
> 
> Good catch, updated patch attached.
> 
> >
> > 2) The purpose of this statement is not clear to me:
> > +if len(attrs)>  0 and allow_only:
> > +raise errors.ObjectclassViolation(info='attribute "%(attribute)s" 
> > not allowed' % dict(attribute=attrs[0]))
> > Maybe just the exception text is misleading.
> 
> This function has 2 modes: "allow only the attributes in these 
> objectclasses" or "specifically deny the attributes in these 
> objectclasses". This enforces the first type. If when we've gone through 
> all the attributes there are any left over they must not be allowed so 
> raise an error. This is documented in the function header.

Thanks for explanation, now I get it. It all looks OK, ACK.

Martin


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 167 Added Update and Reset buttons into Dirty dialog.

2011-05-27 Thread Adam Young 

On 05/27/2011 12:43 PM, Endi Sukma Dewata wrote:

The Dirty dialogs have been combined into IPA.dirty_dialog. It
provides the Update and Reset buttons with customizable callback.

Previously the widget's dirty status is computed by comparing the
old values with the new values. This method is sometimes inaccurate,
so the is_dirty() method has been modified to simply return a flag
which is set to true if the widget is changed.

Ticket #896.


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

ACK.  Pushed to master
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-05-27 Thread Martin Kosek 
On Fri, 2011-05-27 at 16:47 +0200, Jan Cholasta wrote:
> On 24.5.2011 15:38, Jan Cholasta wrote:
> > On 20.5.2011 20:27, Jan Cholasta wrote:
> >> On 10.5.2011 20:06, Jan Cholasta wrote:
> >>> Parse netmasks in IP addresses passed to server install.
> >>>
> >>> ticket 1212
> >>
> >> Patch updated.
> >>
> >> TODO: Write unit test for ipapython.ipautil.CheckedIPAddress
> >> TODO: Clean unreachable code paths off of ipa-server-install (?)
> >> TODO: Workarounds for netaddr bugs (?)
> >>
> >>
> >>
> >> ___
> >> Freeipa-devel mailing list
> >> Freeipa-devel@redhat.com
> >> https://www.redhat.com/mailman/listinfo/freeipa-devel
> >
> > Fixed ipa-replica-prepare and added a unit test.
> >
> 
> Another update.
> 
> Honza

Can you please rebase your patches? My patch 070 fixing
add_reverse_zone() function was pushed today. Unfortunately, it made
your patches 18 and 3 not applicable.

You may want to look closer at the patch 070 as it is relevant to your
patch set and also to make sure the fix is still functional after your
set of patches.

Thanks,
Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 167 Added Update and Reset buttons into Dirty dialog.

2011-05-27 Thread Endi Sukma Dewata 

The Dirty dialogs have been combined into IPA.dirty_dialog. It
provides the Update and Reset buttons with customizable callback.

Previously the widget's dirty status is computed by comparing the
old values with the new values. This method is sometimes inaccurate,
so the is_dirty() method has been modified to simply return a flag
which is set to true if the widget is changed.

Ticket #896.

--
Endi S. Dewata
From 826122a193737641e24b6bcd1ff2ba3e21353aba Mon Sep 17 00:00:00 2001
From: Endi S. Dewata 
Date: Thu, 26 May 2011 17:57:39 -0500
Subject: [PATCH] Added Update and Reset buttons into Dirty dialog.

The Dirty dialogs have been combined into IPA.dirty_dialog. It
provides the Update and Reset buttons with customizable callback.

Previously the widget's dirty status is computed by comparing the
old values with the new values. This method is sometimes inaccurate,
so the is_dirty() method has been modified to simply return a flag
which is set to true if the widget is changed.

Ticket #896.
---
 install/ui/aci.js   |7 +-
 install/ui/associate.js |   41 ++--
 install/ui/details.js   |8 +--
 install/ui/dns.js   |6 --
 install/ui/entity.js|5 +-
 install/ui/hbac.js  |5 +-
 install/ui/ipa.js   |   68 +++-
 install/ui/navigation.js|   22 ++-
 install/ui/sudo.js  |3 +-
 install/ui/test/widget_tests.js |9 ++-
 install/ui/widget.js|  134 +++
 11 files changed, 142 insertions(+), 166 deletions(-)

diff --git a/install/ui/aci.js b/install/ui/aci.js
index 336a965fc5d236c91802e9abae019d0b5eea6c1b..d507e2d07b85809d1e45bddbbe6cc3f59faffd02 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -233,7 +233,7 @@ IPA.attributes_widget = function(spec) {
 click: function(){
 $('.aci-attribute', that.table).
 attr('checked', $(this).attr('checked'));
-that.show_undo();
+that.set_dirty(true);
 }
 })
 })).append($('', {
@@ -245,7 +245,6 @@ IPA.attributes_widget = function(spec) {
 that.create_undo(container);
 that.get_undo().click(function(){
 that.reset();
-that.hide_undo();
 });
 }
 
@@ -298,7 +297,7 @@ IPA.attributes_widget = function(spec) {
 value: value,
 'class': 'aci-attribute',
 click: function() {
-that.show_undo();
+that.set_dirty(true);
 }
 }));
 td =  $('').appendTo(aci_tr);
@@ -335,7 +334,7 @@ IPA.attributes_widget = function(spec) {
 value: value,
 'class': 'aci-attribute',
 change: function() {
-that.show_undo();
+that.set_dirty(true);
 }
 }));
 
diff --git a/install/ui/associate.js b/install/ui/associate.js
index 3ba510f10caf502cfa3323137b6a4eba27afbc72..5eb84260eee57ef556db13cf4e04eeb9c430f52a 100644
--- a/install/ui/associate.js
+++ b/install/ui/associate.js
@@ -348,7 +348,6 @@ IPA.association_table_widget = function (spec) {
 
 that.create = function(container) {
 
-var entity = IPA.get_entity(that.entity_name);
 var column;
 
 // create a column if none defined
@@ -395,21 +394,6 @@ IPA.association_table_widget = function (spec) {
 
 that.table_setup(container);
 
-var dialog = IPA.dialog({
-title: IPA.messages.dialogs.dirty_title,
-width: '20em'
-});
-
-dialog.create = function() {
-dialog.container.append(IPA.messages.dialogs.dirty_message);
-};
-
-dialog.add_button(IPA.messages.buttons.ok, function() {
-dialog.close();
-});
-
-dialog.init();
-
 var entity = IPA.get_entity(that.entity_name);
 var facet_name = IPA.current_facet(entity);
 var facet = entity.get_facet(facet_name);
@@ -424,7 +408,17 @@ IPA.association_table_widget = function (spec) {
 }
 
 if (facet.is_dirty()) {
+var dialog = IPA.dirty_dialog({
+facet: facet
+});
+
+dialog.callback = function() {
+that.show_remove_dialog();
+};
+
+dialog.init();
 dialog.open(that.container);
+
 } else {
 that.show_remove_dialog();
 }
@@ -443,7 +437,17 @@ IPA.association_table_widget = function (spec) {
 }
 
 if (facet.is_dirty()) {
+var dialog = IPA.dirty_dialog({
+facet: facet
+});

Re: [Freeipa-devel] [PATCH] 070 Fix reverse zone creation in ipa-replica-prepare

2011-05-27 Thread Rob Crittenden 

Martin Kosek wrote:

This patch replaces Rob's patch 791.
---
When a new reverse zone was created in ipa-replica-prepare (this
may happen when a new replica is from different subnet), the master
DNS address was corrupted by invalid A/ record. This caused
problems for example in installing replica.

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


ack

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 789 improve label when prompting for members

2011-05-27 Thread Jan Cholasta 

On 24.5.2011 04:33, Rob Crittenden wrote:

Include the word 'member' with autogenerated optional member labels.

There were reports of confusion over what was being prompted for,
hopefully adding member will make things clearer.

This has a big API.txt change but it is all labels so minor in nature,
just affecting the CLI.

ticket 1062

rob



ACK, works as expected.

Honza

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 070 Fix reverse zone creation in ipa-replica-prepare

2011-05-27 Thread Martin Kosek 
This patch replaces Rob's patch 791.
---
When a new reverse zone was created in ipa-replica-prepare (this
may happen when a new replica is from different subnet), the master
DNS address was corrupted by invalid A/ record. This caused
problems for example in installing replica.

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

>From 0434292b18c7bc5acf20715e49a13625289c6e76 Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Fri, 27 May 2011 17:05:45 +0200
Subject: [PATCH] Fix reverse zone creation in ipa-replica-prepare

When a new reverse zone was created in ipa-replica-prepare (this
may happen when a new replica is from different subnet), the master
DNS address was corrupted by invalid A/ record. This caused
problems for example in installing replica.

https://fedorahosted.org/freeipa/ticket/1223
---
 install/tools/ipa-dns-install |   32 +++-
 install/tools/ipa-replica-install |   17 +
 install/tools/ipa-replica-prepare |4 +++-
 install/tools/ipa-server-install  |   29 +++--
 ipaserver/install/bindinstance.py |7 ---
 ipaserver/install/installutils.py |   15 +++
 6 files changed, 37 insertions(+), 67 deletions(-)

diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index aac85bf230d006455c5f4289ec9f5fd997261d52..a763297678907effd0497517d6d1607ac1e5a2f3 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -62,31 +62,6 @@ def parse_options():
 
 return safe_options, options
 
-def resolve_host(host_name):
-ip = None
-try:
-addrinfos = socket.getaddrinfo(host_name, None,
-   socket.AF_UNSPEC, socket.SOCK_DGRAM)
-except:
-print "Unable to lookup the IP address of the provided host"
-return None
-
-for ai in addrinfos:
-ip = ai[4][0]
-if ip == "127.0.0.1" or ip == "::1":
-print "The hostname resolves to the localhost address (127.0.0.1/::1)"
-print "Please change your /etc/hosts file so that the hostname."
-print "resolves to the ip address of your network interface."
-print ""
-print "Please fix your /etc/hosts file and restart the setup program."
-print ""
-sys.exit("Aborting installation.")
-
-if addrinfos:
-ip = addrinfos[0][4][0]
-
-return ip
-
 def main():
 safe_options, options = parse_options()
 
@@ -211,6 +186,13 @@ except KeyboardInterrupt:
 print "Installation cancelled."
 except RuntimeError, e:
 print str(e)
+except HostnameLocalhost:
+print "The hostname resolves to the localhost address (127.0.0.1/::1)"
+print "Please change your /etc/hosts file so that the hostname"
+print "resolves to the ip address of your network interface."
+print "The KDC service does not listen on localhost"
+print ""
+print "Please fix your /etc/hosts file and restart the setup program"
 except Exception, e:
 message = "Unexpected error - see ipaserver-install.log for details:\n %s" % str(e)
 print message
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 49df7fef9aceb3dbf8dd1dfdd91dd03132798484..293a0a06c8e4ff608d8327135ea1b4e008ab4d33 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -30,6 +30,7 @@ from ipapython import ipautil
 from ipaserver.install import dsinstance, installutils, krbinstance, service
 from ipaserver.install import bindinstance, httpinstance, ntpinstance, certs
 from ipaserver.install.replication import check_replication_plugin
+from ipaserver.install.installutils import HostnameLocalhost, resolve_host
 from ipaserver.plugins.ldap2 import ldap2
 from ipapython import version
 from ipalib import api, errors, util
@@ -38,9 +39,6 @@ from ipapython import sysrestore
 
 CACERT="/etc/ipa/ca.crt"
 
-class HostnameLocalhost(Exception):
-pass
-
 class ReplicaConfig:
 def __init__(self):
 self.realm_name = ""
@@ -131,19 +129,6 @@ def get_host_name(no_host_dns):
 
 return hostname
 
-def resolve_host(host_name):
-try:
-addrinfos = socket.getaddrinfo(host_name, None,
-   socket.AF_UNSPEC, socket.SOCK_STREAM)
-for ai in addrinfos:
-ip = ai[4][0]
-if ip == "127.0.0.1" or ip == "::1":
-raise HostnameLocalhost
-
-return addrinfos[0][4][0]
-except:
-return None
-
 def set_owner(config, dir):
 pw = pwd.getpwnam(dsinstance.DS_USER)
 os.chown(dir, pw.pw_uid, pw.pw_gid)
diff --git a/install/tools/ipa-replica-prepare b/install/tools/ipa-replica-prepare
index e9122351f5236bef4e82a15d1ab47c896ff03554..a41ca5121cd451093af3ee7c9d7282e300df53ca 100755
--- a/install/tools/ipa-replica-prepare
+++ b/install/tools/ipa-replica-prepare
@@ -30,6 +30,7 @@ from ipapython import ipautil
 from ipaserver.install import bindinstance, ds

Re: [Freeipa-devel] [PATCH] 784 limit what attributes may be modified

2011-05-27 Thread Rob Crittenden 

Martin Kosek wrote:

On Mon, 2011-05-16 at 17:46 -0400, Rob Crittenden wrote:

Add option to limit the attributes allowed in an entry.

Kerberos ticket policy can update policy in a user entry. This allowed
set/addattr to be used to modify attributes outside of the ticket policy
perview, also bypassing all validation/normalization. Likewise the
ticket policy was updatable by the user plugin bypassing all validation.

Add two new LDAPObject values to control this behavior:

limit_object_classes: only attributes in these are allowed
disallow_object_classes: attributes in these are disallowed

By default both of these lists are empty so are skipped.

ticket 744

rob


NACK. I have some concerns with this patch. In function
_check_limit_object_class:

1) You change input attribute 'attrs' by removing the items from it. If
user passes the same list of attrs to be checked and the function is run
twice, the 'attrs' parameter in second run is corrupt.

You can try it by running e.g. `ipa krbtpolicy-mod --maxrenew=24044' and
checking the value of this parameter in the function.


Good catch, updated patch attached.



2) The purpose of this statement is not clear to me:
+if len(attrs)>  0 and allow_only:
+raise errors.ObjectclassViolation(info='attribute "%(attribute)s" not 
allowed' % dict(attribute=attrs[0]))
Maybe just the exception text is misleading.


This function has 2 modes: "allow only the attributes in these 
objectclasses" or "specifically deny the attributes in these 
objectclasses". This enforces the first type. If when we've gone through 
all the attributes there are any left over they must not be allowed so 
raise an error. This is documented in the function header.




Otherways it's good, it correctly raised an exception when I tried to
misuse --setattr option.

Martin



>From 3be1c53c3eed62660ff102330675620931d195c4 Mon Sep 17 00:00:00 2001
From: Rob Crittenden 
Date: Mon, 16 May 2011 17:39:23 -0400
Subject: [PATCH] Add option to limit the attributes allowed in an entry.

Kerberos ticket policy can update policy in a user entry. This allowed
set/addattr to be used to modify attributes outside of the ticket policy
perview, also bypassing all validation/normalization. Likewise the
ticket policy was updatable by the user plugin bypassing all validation.

Add two new LDAPObject values to control this behavior:

limit_object_classes: only attributes in these are allowed
disallow_object_classes: attributes in these are disallowed

By default both of these lists are empty so are skipped.

ticket 744
---
 ipalib/plugins/baseldap.py|   36 +
 ipalib/plugins/krbtpolicy.py  |1 +
 ipalib/plugins/user.py|2 +
 tests/test_xmlrpc/test_krbtpolicy.py  |  138 +
 tests/test_xmlrpc/test_user_plugin.py |   20 +
 5 files changed, 197 insertions(+), 0 deletions(-)
 create mode 100644 tests/test_xmlrpc/test_krbtpolicy.py

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 43533c8..f07fb27 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -253,6 +253,8 @@ class LDAPObject(Object):
 # If an objectclass is possible but not default in an entry. Needed for
 # collecting attributes for ACI UI.
 possible_objectclasses = []
+limit_object_classes = [] # Only attributes in these are allowed
+disallow_object_classes = [] # Disallow attributes in these
 search_attributes = []
 search_attributes_config = None
 default_attributes = []
@@ -438,6 +440,36 @@ def _check_empty_attrs(params, entry_attrs):
 raise errors.RequirementError(name=a)
 
 
+def _check_limit_object_class(attributes, attrs, allow_only):
+"""
+If the set of objectclasses is limited enforce that only those
+are updated in entry_attrs (plus dn)
+
+allow_only tells us what mode to check in:
+
+If True then we enforce that the attributes must be in the list of
+allowed.
+
+If False then those attributes are not allowed.
+"""
+if len(attributes[0]) == 0 and len(attributes[1]) == 0:
+return
+limitattrs = deepcopy(attrs)
+# Go through the MUST first
+for (oid, attr) in attributes[0].iteritems():
+if attr.names[0].lower() in limitattrs:
+if not allow_only:
+raise errors.ObjectclassViolation(info='attribute "%(attribute)s" not allowed' % dict(attribute=attr.names[0].lower()))
+limitattrs.remove(attr.names[0].lower())
+# And now the MAY
+for (oid, attr) in attributes[1].iteritems():
+if attr.names[0].lower() in limitattrs:
+if not allow_only:
+raise errors.ObjectclassViolation(info='attribute "%(attribute)s" not allowed' % dict(attribute=attr.names[0].lower()))
+limitattrs.remove(attr.names[0].lower())
+if len(limitattrs) > 0 and allow_only:
+raise errors.ObjectclassViolation(info='attribute "%(attribute)s" not all

Re: [Freeipa-devel] [PATCH] 19 Do stricter checking of IP addressed passed to server install

2011-05-27 Thread Jan Cholasta 

On 25.5.2011 09:46, Martin Kosek wrote:

On Tue, 2011-05-24 at 15:42 +0200, Jan Cholasta wrote:

On 24.5.2011 14:44, Jan Cholasta wrote:

On 24.5.2011 14:43, Martin Kosek wrote:

On Fri, 2011-05-20 at 20:34 +0200, Jan Cholasta wrote:

On 18.5.2011 10:51, Martin Kosek wrote:

On Mon, 2011-05-16 at 19:15 +0200, Jan Cholasta wrote:

On 16.5.2011 17:26, Martin Kosek wrote:

On Tue, 2011-05-10 at 20:11 +0200, Jan Cholasta wrote:

Split from patch 3, requires patch 18.

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

Honza



I tested all patches (3.6, 18, 19), but I think some work still
needs to
be done:

1) What about adding /sbin/ip package to Requires in spec? I thought
there was an agreement to do it.


Will do.


Ok.





2) When I run `ipa-server-install --ip-address=$ADDR`, and $ADDR is
invalid address (e.g. $ADDR==foo), loopback address (e.g.
$ADDR==127.0.0.1) or just another that the local address (e.g.
$ADDR==123.123.123.123) the installer always fails with "the hostname
resolves to an IP address that is different from the one provided
on the
command line".

I think we may want a different error message in those 3 cases - it
should be easy to do it now, with the improved IP handling.


It looks like the print statements from verify_ip_address doesn't
actually print anything to the user. Will look onto that.


Ok.





3) When I pass netmask to ipa-server-install --ip-address=$ADDR, the
installation always fails with the above message. Even though I
took the
addr+netmask from "/sbin/ip address" output.


Works for me. Please make sure you've added your hostname to
/etc/hosts.


I think I had. But I will recheck when you send a fix.





4) I miss IP address checks in --ip-address and --forwarder
parameters
of ipa-dns-install script. I can pass invalid or local addresses to
these parameters. This breaks Bind configuration.


--ip-address is checked, but --forwarder is not. Will fix that.


Ok, I will recheck both of them when you do.





5) I think we may want to check also for local address in
#ipa host-add $HOST --ip-address=127.0.0.1

6) I couldn't add IP address with netmask in host module:
# ipa host-add $HOST --ip-address=10.16.78.102/22
ipa: ERROR: invalid 'ip_address': invalid IP address


The patches are for the installer, as are the tickets they fix, so
these
issues are out of scope. A new ticket should be opened for them.



You touched this parameter in your patches, that's why I tested it. I
created a new ticket for it:

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

Ticket 1234, yey :-)



7) Why is the _ParsedIPAddress named with a leading underscore?
It's not
really an internal use since it is returned by new IP handling
functions
and used in other modules.


_ParsedIPAddress is not for public use. The fact that object of this
class is returned by parse_ip_address doesn't really matter - this is
Python, not C++ or Java.


Hm, snappy... And I was wondering why my /usr/bin/java doesn't want to
run FreeIPA, now I know - it's because its Python.

Martin



Patch updated. Requires patch 18.1

Honza



All reported issues were fixed, good idea with a new type for our
IPAOptionParser.

Still, NACK from me:

ipa-replica-install doesn't use IPAOptionParser, but the good old
OptionParser which doesn't know the new type. This makes
ipa-replica-prepare crash all the time. I know, I am nitpicker :-)

Martin



Thanks, I missed that.

Honza



Fixed and added a unit test.



NACK. Please test your patches before you send them for a review. It
saves reviewer's time.


Sorry, I'll do better next time.



1) Unwanted warning about unmatching network interface when replica is
installed:

# ipa-replica-prepare vm-059.idm.lab.bos.redhat.com
--ip-address=10.16.78.59
Warning: No network interface matches IP address 10.16.78.59
Directory Manager (existing master) password:
...


Fixed.



2) ipa-replica-install crashes
# ipa-replica-install 
/home/mkosek/replica-info-vm-059.idm.lab.bos.redhat.com.gpg
Directory Manager (existing master) password:

Configuring ntpd
   [1/4]: stopping ntpd
   [2/4]: writing configuration
   [3/4]: configuring ntpd to start on boot
   [4/4]: starting ntpd
done configuring ntpd.
creation of replica failed: unsupported operand type(s) for /: 'NoneType' and 
'int'

Your system may be partly configured.
Run /usr/sbin/ipa-server-install --uninstall to clean up.


ipa-replica-install log:
2011-05-25 03:36:18,503 DEBUG unsupported operand type(s) for /: 'NoneType' and 
'int'
   File "/usr/sbin/ipa-replica-install", line 550, in
 main()

   File "/usr/sbin/ipa-replica-install", line 496, in main
 install_dns_records(config, options)

   File "/usr/sbin/ipa-replica-install", line 329, in install_dns_records
 options.conf_ntp)

   File "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py", 
line 469, in add_master_dns_records
 self.__add_self()

   File "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py", 
line 399, in __add_self
 if dns_z

Re: [Freeipa-devel] [PATCH] 3 Add ability to specify netmask with IP addresses during installation

2011-05-27 Thread Jan Cholasta 

On 20.5.2011 20:29, Jan Cholasta wrote:

On 12.5.2011 14:47, Jan Cholasta wrote:


Rewrote host.py so that it doesn't use get_reverse_zone from
ipaserver.bindinstance (which fixes the pylint errors).

Honza



Patch updated. Requires patch 18.1.



Another update, requires patch 18.3.

Honza

--
Jan Cholasta
>From d131dd9ea659bb1cc21aa2146b14a0832c45e42a Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Fri, 27 May 2011 13:52:07 +0200
Subject: [PATCH] Honor netmask in DNS reverse zone setup.

ticket 910
---
 install/tools/ipa-dns-install |3 +-
 install/tools/ipa-replica-install |6 +++-
 install/tools/ipa-replica-prepare |   32 +++---
 install/tools/ipa-server-install  |3 +-
 ipalib/plugins/host.py|   45 
 ipaserver/install/bindinstance.py |   52 -
 6 files changed, 97 insertions(+), 44 deletions(-)

diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index 491585b..d41a476 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -138,6 +138,7 @@ def main():
 sys.exit("Unable to resolve IP address for host name")
 else:
 ip_address = read_ip_address(api.env.host, fstore)
+ip_prefixlen = ip_address.prefixlen
 ip_address = str(ip_address)
 logging.debug("will use ip_address: %s\n", ip_address)
 
@@ -183,7 +184,7 @@ def main():
 create_reverse = not options.no_reverse
 elif not options.no_reverse:
 create_reverse = bindinstance.create_reverse()
-bind.setup(api.env.host, ip_address, api.env.realm, api.env.domain, dns_forwarders, conf_ntp, create_reverse, zonemgr=options.zonemgr)
+bind.setup(api.env.host, ip_address, ip_prefixlen, api.env.realm, api.env.domain, dns_forwarders, conf_ntp, create_reverse, zonemgr=options.zonemgr)
 
 if bind.dm_password:
 api.Backend.ldap2.connect(bind_dn="cn=Directory Manager", bind_pw=bind.dm_password)
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 2727fa6..65b5536 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -287,6 +287,7 @@ def install_bind(config, options):
 sys.exit("Unable to resolve IP address for host name")
 ip = installutils.parse_ip_address(ip_address)
 ip_address = str(ip)
+ip_prefixlen = ip.prefixlen
 
 create_reverse = True
 if options.unattended:
@@ -300,7 +301,7 @@ def install_bind(config, options):
 # specified, ask the user
 create_reverse = bindinstance.create_reverse()
 
-bind.setup(config.host_name, ip_address, config.realm_name,
+bind.setup(config.host_name, ip_address, ip_prefixlen, config.realm_name,
config.domain_name, forwarders, options.conf_ntp, create_reverse)
 bind.create_instance()
 
@@ -324,8 +325,9 @@ def install_dns_records(config, options):
 sys.exit("Unable to resolve IP address for host name")
 ip = installutils.parse_ip_address(ip_address)
 ip_address = str(ip)
+ip_prefixlen = ip.prefixlen
 
-bind.add_master_dns_records(config.host_name, ip_address,
+bind.add_master_dns_records(config.host_name, ip_address, ip_prefixlen,
 config.realm_name, config.domain_name,
 options.conf_ntp)
 
diff --git a/install/tools/ipa-replica-prepare b/install/tools/ipa-replica-prepare
index f54436d..69ebd4a 100755
--- a/install/tools/ipa-replica-prepare
+++ b/install/tools/ipa-replica-prepare
@@ -27,7 +27,7 @@ import krbV
 
 from ipapython import ipautil
 from ipaserver.install import bindinstance, dsinstance, installutils, certs
-from ipaserver.install.bindinstance import add_zone, add_reverse_zone, add_rr, add_ptr_rr
+from ipaserver.install.bindinstance import add_zone, add_reverse_zone, add_fwd_rr, add_ptr_rr, dns_zone_exists
 from ipaserver.install.replication import check_replication_plugin, enable_replication_version_checking
 from ipaserver.plugins.ldap2 import ldap2
 from ipapython import version
@@ -425,11 +425,33 @@ def main():
 name = domain.pop(0)
 domain = ".".join(domain)
 
-ip_address = str(options.ip_address)
+ip = options.ip_address
+ip_address = str(ip)
+ip_prefixlen = ip.prefixlen
+
+if ip.defaultnet:
+revzone = ip.reverse_dns
+if ip.version == 4:
+prefix = 32
+dec = 8
+elif ip.version == 6:
+prefix = 128
+dec = 4
+
+while prefix > 0:
+dummy, dot, revzone = revzone.partition('.')
+prefix = prefix - dec
+if dns_zone_exists(revzone):
+break
+
+if prefix > 0:
+ip_prefixlen = prefix
+else:
+add_reverse_zone(ip_address, ip_prefixlen)
+
 zone = add_zone(domain, nsa

Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-05-27 Thread Jan Cholasta 

On 24.5.2011 15:38, Jan Cholasta wrote:

On 20.5.2011 20:27, Jan Cholasta wrote:

On 10.5.2011 20:06, Jan Cholasta wrote:

Parse netmasks in IP addresses passed to server install.

ticket 1212


Patch updated.

TODO: Write unit test for ipapython.ipautil.CheckedIPAddress
TODO: Clean unreachable code paths off of ipa-server-install (?)
TODO: Workarounds for netaddr bugs (?)



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Fixed ipa-replica-prepare and added a unit test.



Another update.

Honza

--
Jan Cholasta
>From 9bf06f27c816f98281e6e33bed6725e0322727f0 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Fri, 27 May 2011 13:01:41 +0200
Subject: [PATCH] Parse netmasks in IP addresses passed to server install.

ticket 1212
---
 freeipa.spec.in  |1 +
 install/tools/ipa-dns-install|9 +++--
 install/tools/ipa-replica-install|6 +++-
 install/tools/ipa-replica-prepare|   17 +
 install/tools/ipa-server-install |   36 +--
 ipapython/config.py  |   13 ++-
 ipapython/ipautil.py |   67 ++
 ipaserver/install/installutils.py|   39 +---
 tests/test_ipapython/__init__.py |   22 +++
 tests/test_ipapython/test_ipautil.py |   56 
 10 files changed, 213 insertions(+), 53 deletions(-)
 create mode 100644 tests/test_ipapython/__init__.py
 create mode 100644 tests/test_ipapython/test_ipautil.py

diff --git a/freeipa.spec.in b/freeipa.spec.in
index b936616..fba2f31 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -188,6 +188,7 @@ Requires: python-kerberos >= 1.1-3
 %endif
 Requires: authconfig
 Requires: gnupg
+Requires: iproute
 Requires: pyOpenSSL
 Requires: python-nss >= 0.11
 Requires: python-lxml
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index aac85bf..491585b 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -37,9 +37,10 @@ def parse_options():
   sensitive=True, help="admin password")
 parser.add_option("-d", "--debug", dest="debug", action="store_true",
   default=False, help="print debugging information")
-parser.add_option("--ip-address", dest="ip_address", help="Master Server IP Address")
+parser.add_option("--ip-address", dest="ip_address",
+  type="ipnet", help="Master Server IP Address")
 parser.add_option("--forwarder", dest="forwarders", action="append",
-  help="Add a DNS forwarder")
+  type="ipaddr", help="Add a DNS forwarder")
 parser.add_option("--no-forwarders", dest="no_forwarders", action="store_true",
   default=False, help="Do not add any DNS forwarders, use root servers instead")
 parser.add_option("--no-reverse", dest="no_reverse",
@@ -130,12 +131,14 @@ def main():
 if options.ip_address:
 ip_address = options.ip_address
 else:
-ip_address = resolve_host(api.env.host)
+hostaddr = resolve_host(api.env.host)
+ip_address = hostaddr and ipautil.CheckedIPAddress(hostaddr)
 if not ip_address or not verify_ip_address(ip_address):
 if options.unattended:
 sys.exit("Unable to resolve IP address for host name")
 else:
 ip_address = read_ip_address(api.env.host, fstore)
+ip_address = str(ip_address)
 logging.debug("will use ip_address: %s\n", ip_address)
 
 if options.no_forwarders:
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 49df7fe..2727fa6 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -63,7 +63,7 @@ def parse_options():
 parser.add_option("--setup-dns", dest="setup_dns", action="store_true",
   default=False, help="configure bind with our zone")
 parser.add_option("--forwarder", dest="forwarders", action="append",
-  help="Add a DNS forwarder")
+  type="ipaddr", help="Add a DNS forwarder")
 parser.add_option("--no-forwarders", dest="no_forwarders", action="store_true",
   default=False, help="Do not add any DNS forwarders, use root servers instead")
 parser.add_option("--no-reverse", dest="no_reverse", action="store_true",
@@ -285,6 +285,8 @@ def install_bind(config, options):
 ip_address = resolve_host(config.host_name)
 if not ip_address:
 sys.exit("Unable to resolve IP address for host name")
+ip = installutils.parse_ip_address(ip_address)
+ip_address = str(ip)
 
 create_reverse = True
 if options.unattended:
@@ -320,6 +322,8 @@ def install_dns_records(config, options):
 ip_address = resolve_host(config.host_name)
 if not ip_address:
 sys.exit("Unable to resolve I

Re: [Freeipa-devel] [PATCH] 787 Don't load LDAP schema at startup

2011-05-27 Thread Martin Kosek 
On Mon, 2011-05-23 at 15:09 -0400, Rob Crittenden wrote:
> Rob Crittenden wrote:
> > Do a lazy retrieval of the LDAP schema rather than at module load.
> >
> > Attempt to retrieve the schema the first time it is needed rather than
> > when Apache is started. A global copy is cached for future requests for
> > performance reasons.
> >
> > The schema will be retrieved once per Apache child process.
> >
> > ticket 583
> >
> > This replaces Jan's patch titled "Don't load the LDAP schema during
> > startup"
> >
> > rob
> 
> Updated patch. This removes a debugging statement I left in and forces a 
> schema load in a couple of other places in baseldap.
> 
> This relies on patch 784 to apply.
> 
> rob

ACK. Looks good to me. No suspicious test failures too.

Martin


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 784 limit what attributes may be modified

2011-05-27 Thread Martin Kosek 
On Mon, 2011-05-16 at 17:46 -0400, Rob Crittenden wrote:
> Add option to limit the attributes allowed in an entry.
> 
> Kerberos ticket policy can update policy in a user entry. This allowed 
> set/addattr to be used to modify attributes outside of the ticket policy 
> perview, also bypassing all validation/normalization. Likewise the 
> ticket policy was updatable by the user plugin bypassing all validation.
> 
> Add two new LDAPObject values to control this behavior:
> 
> limit_object_classes: only attributes in these are allowed
> disallow_object_classes: attributes in these are disallowed
> 
> By default both of these lists are empty so are skipped.
> 
> ticket 744
> 
> rob

NACK. I have some concerns with this patch. In function
_check_limit_object_class:

1) You change input attribute 'attrs' by removing the items from it. If
user passes the same list of attrs to be checked and the function is run
twice, the 'attrs' parameter in second run is corrupt.

You can try it by running e.g. `ipa krbtpolicy-mod --maxrenew=24044' and
checking the value of this parameter in the function.

2) The purpose of this statement is not clear to me:
+if len(attrs) > 0 and allow_only:
+raise errors.ObjectclassViolation(info='attribute "%(attribute)s" not 
allowed' % dict(attribute=attrs[0]))
Maybe just the exception text is misleading.

Otherways it's good, it correctly raised an exception when I tried to
misuse --setattr option.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 791 don't add IP address when creating zone

2011-05-27 Thread Martin Kosek 
On Thu, 2011-05-26 at 15:11 -0400, Rob Crittenden wrote:
> When creating a DNS zone if an IP address was passed in that address was 
> added to the record of the IPA server.
> 
> This was causing problems when creating new reverse zones for different 
> subnets with ipa-replica-prepare. If you padded in --ip_address then a 
> new reverse DNS zone would be created and the new IP would be added to 
> the IPA master. Installing the replica file would fail with odd errors.
> 
> ticket 1223
> 
> rob

NACK. This breaks current --ip-address option functionality for
dnszone-add added in ticket #838. It is a shortcut to add a new zone
with a non-resolvable name server and the A/ record of the new name
server at the same time.

This is behavior with your patch (ns.example.com is not resolvable):
# ipa dnszone-add example.com --name-server=ns.example.com 
--admin-email=ad...@example.com --ip-address=1.2.3.4
  Zone name: example.com
  Authoritative nameserver: ns.example.com.
  Administrator e-mail address: admin.example.com.
  SOA serial: 2011270501
  SOA refresh: 3600
  SOA retry: 900
  SOA expire: 1209600
  SOA minimum: 3600
  Active zone: TRUE
  Dynamic update: FALSE
# ipa dnsrecord-show example.com ns
ipa: ERROR: ns: DNS resource record not found

And without it:
# ipa dnszone-add example2.com --name-server=ns.example2.com 
--admin-email=ad...@example2.com --ip-address=1.2.3.4
  Zone name: example2.com
  Authoritative nameserver: ns.example2.com.
  Administrator e-mail address: admin.example2.com.
  SOA serial: 2011270501
  SOA refresh: 3600
  SOA retry: 900
  SOA expire: 1209600
  SOA minimum: 3600
  Active zone: TRUE
  Dynamic update: FALSE
# ipa dnsrecord-show example2.com ns  Record name: ns
  A record: 1.2.3.4

I think all we have to do is to fix ipa-replica-prepare:
...
if options.ip_address:
print "Adding DNS records for %s" % replica_fqdn
api.Backend.ldap2.connect(bind_dn="cn=Directory Manager", 
bind_pw=dirman_password)

domain = replica_fqdn.split(".")
name = domain.pop(0)
domain = ".".join(domain)

zone = add_zone(domain, nsaddr=options.ip_address)
add_rr(zone, name, "A", options.ip_address)
add_reverse_zone(options.ip_address)   <== BUG
add_ptr_rr(options.ip_address, replica_fqdn)

Currently, we are adding a reverse zone with a name server IP address
pointing to the new replica instead of the current master. And this is
just wrong.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin

2011-05-27 Thread Martin Kosek 
On Thu, 2011-05-26 at 21:00 +0200, Jan Cholasta wrote:
> On 26.5.2011 14:32, Martin Kosek wrote:
> > Interactive mode for commands manipulating with DNS records
> > (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances
> > the server framework with new callback for interactive mode, which
> > can be used by commands to inject their own interactive handling.
> >
> > The callback is then used to improve aforementioned commands'
> > interactive mode.
> >
> > https://fedorahosted.org/freeipa/ticket/1018
> >
> 
> ACK, works fine.
> 
> Just a minor thing:
> 
> $ git apply 
> freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch
> freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch:33: 
> trailing whitespace.
> 
> freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch:41: 
> trailing whitespace.
> 
> freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch:289: 
> trailing whitespace.
> 
> freeipa-mkosek-069-improve-interactive-mode-for-dns-plugin.patch:193: 
> new blank line at EOF.
> +
> warning: 4 lines add whitespace errors.
> 
> Honza
> 

Yeah, I fixed these. Nothing in my workflow reports me such errors, I
must enhance my .vimrc to do it for me.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 069 Improve interactive mode for DNS plugin

2011-05-27 Thread Martin Kosek 
On Thu, 2011-05-26 at 22:39 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > Interactive mode for commands manipulating with DNS records
> > (dnsrecord-add, dnsrecord-del) is not usable. This patch enhances
> > the server framework with new callback for interactive mode, which
> > can be used by commands to inject their own interactive handling.
> >
> > The callback is then used to improve aforementioned commands'
> > interactive mode.
> >
> > https://fedorahosted.org/freeipa/ticket/1018
> 
> This works pretty nicely but it seems like with just a bit more it can 
> be great.
> 
> Can you add some doc examples for how this works?

Done. At least user will know that we have a feature like that to offer.

> 
> And you display the records now and then prompt for each to delete. Can 
> you combine the two?
> 
> For example:
> 
> ipa dnsrecord-del greyoak.com lion
> No option to delete specific record provided.
> Delete all? Yes/No (default No):
> Current DNS record contents:
> 
> A record: 192.168.166.32
> 
> Enter value(s) to remove:
> [A record]:
> 
> If we know there is an record why not just prompt for each value yes/no 
> to delete?

Actually, this is a very good idea, I like it. I updated the patch so
that the user can only do yes/no decision in ipa dnsrecord-del
interactive mode. This makes dnsrecord-del interactive mode very usable.

> 
> The yes/no function needs more documentation on what default does too. 
> It appears that the possible values are None/True/False and that None 
> means that '' can be returned (which could still be evaluated as False 
> if this isn't used right).

Done. '' shouldn't be returned as I return the value of "default" if it
is not None. But yes, it needed more documenting.

Updated patch is attached. It may need some language corrections, I am
no native speaker.

Martin
>From d12e9389f280ac8848f3d61d3e382d2d220319e3 Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Thu, 26 May 2011 09:55:53 +0200
Subject: [PATCH] Improve interactive mode for DNS plugin

Interactive mode for commands manipulating with DNS records
(dnsrecord-add, dnsrecord-del) is not usable. This patch enhances
the server framework with new callback for interactive mode, which
can be used by commands to inject their own interactive handling.

The callback is then used to improve aforementioned commands'
interactive mode.

https://fedorahosted.org/freeipa/ticket/1018
---
 ipalib/cli.py  |   41 +++
 ipalib/plugins/baseldap.py |   42 
 ipalib/plugins/dns.py  |  159 ++--
 3 files changed, 222 insertions(+), 20 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 99f236bb4103c8524320b03aa4a311689ecef8e8..dc3a730b460ce0a033a9418bf1cc6535b5d6ddff 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -527,6 +527,44 @@ class textui(backend.Backend):
 return None
 return self.decode(data)
 
+def prompt_yesno(self, label, default=None):
+"""
+Prompt user for yes/no input. This method returns True/False according
+to user response.
+
+If Default parameter is not None, its value is returned if user pass
+no input to yes/no prompt.
+
+If Default parameter is None, user is asked until a correct answer is
+provided (yes/no) or the program is not terminated.
+"""
+
+default_prompt = None
+if default is not None:
+if default:
+default_prompt = "Yes"
+else:
+default_prompt = "No"
+
+if default_prompt:
+prompt = u'%s Yes/No (default %s): ' % (label, default_prompt)
+else:
+prompt = u'%s Yes/No: ' % label
+
+result=None
+while result is None:
+try:
+data = raw_input(self.encode(prompt)).lower()
+except EOFError:
+return None
+
+if data in (u'yes', u'y'):
+return True
+elif data in ( u'n', u'no'):
+return False
+elif default is not None and data == u'':
+return default
+
 def prompt_password(self, label):
 """
 Prompt user for a password or read it in via stdin depending
@@ -1032,6 +1070,9 @@ class cli(backend.Executioner):
 param.label
 )
 
+for callback in getattr(cmd, 'INTERACTIVE_PROMPT_CALLBACKS', []):
+callback(kw)
+
 def load_files(self, cmd, kw):
 """
 Load files from File parameters.
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 43533c8c969e368f450cb049235816db8a954b46..1823e08b03bc942cef679ed6d5dbb0d1f7ce05e6 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -450,12 +450,17 @@ class CallbackInterface(Method):
 self.__class__.POST_CALLBACKS = []
 if not hasattr(self.__class__, 'EXC_CALLBACKS'):
 self.__cl