Re: [Freeipa-devel] [PATCH] 16 Netgroup nisdomain and hosts validation
On Wed, 2012-03-14 at 16:54 +0100, Ondrej Hamada wrote:
On 03/09/2012 04:34 PM, Martin Kosek wrote:
On Thu, 2012-03-08 at 14:52 +0100, Ondrej Hamada wrote:
Netgroup nisdomain and hosts validation
nisdomain validation:
Added pattern to the 'nisdomain' parameter to validate the specified
nisdomain name. According to most common use cases the same patter as
for netgroup should fit. Unit-tests added.
https://fedorahosted.org/freeipa/ticket/2447
hosts validation:
Added precallback to netgroup_add_member. It validates the specified
hostnames and raises ValidationError exception for invalid hostnames.
Unit-test added.
https://fedorahosted.org/freeipa/ticket/2448
I checked the host validation part and it could be improved. Issue
described in #2447 (you have switched the ticket IDs) affects all
objects that allow external hosts, users, ..., i.e. those who call
add_external_post_callback in their post_callback.
Should we fix all of these when we deal with this issue? Otherwise user
could do something like this:
# ipa sudorule-add-user foo --users=a+b
Rule name: foo
Enabled: TRUE
External User: a+b
We could create a similar function called add_external_pre_callback()
and pass it attribute name and validating function (which would be
common with the linked object). It would then do the validation for all
these affected objects consistently and without redundant code.
I didn't liked much the implemented pre_callback anyway
+def pre_callback(self, ldap, dn, found, not_found, *keys,
**options):
+# validate entered hostnames
+if 'host' in options:
+invalid_hostnames=[]
+for hostname in options['host']:
+try:
+validate_hostname(hostname, False)
+except ValueError:
+invalid_hostnames.append(hostname)
+if invalid_hostnames:
+raise errors.ValidationError(name='host',
error='hostnames:\%s\ contain invalid characters' %
','.join(invalid_hostnames))
+return dn
I would rather raise the ValidationError with the first invalid hostname
and tell what's wrong (function validate_hostname tells it to you). If
you go with the proposed approach, you wouldn't have to deal with
formatting error messages, you would just raise the one returned by the
validator shared with the linked LDAP object (hostname, user, ...).
Martin
external_pre_callback function seems as a good idea, but there is a
problem how to get the validators for various LDAP objects. For the
hostname we already have one in ipalib.utils, but for the uid or group
name we use only patterns specified in the parameter objects.
Below I propose solution how to use the already defined parameter
objects for validation (the only problem is that I have to assume, that
it is always the first parameter in takes_params). Do you think this is
a good approach?
I think the approach is OK, it can just be much improved in order to get
rid of the hardcoded parts. See comments below.
def add_external_pre_callback(memberattr, membertype, externalattr,
ldap, dn, found, not_found, *keys, **options):
Pre callback to validate external members.
if membertype in options:
validator = api.Object[membertype].takes_params[0]
You can use api.Object[membertype].params[memberattr]
for value in options[membertype]:
try:
validator(value)
except errors.ValidationError as e:
error_msg = e[(e.find(':')+1):]
You don't have to parse error message, you can just use e.name or
e.error right from the caught ValidationError.
raise errors.ValidationError(name=membertype,
error=e[e.find(':')+1:])
return dn
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 983 add subject key identifier
On Wed, 2012-03-14 at 17:31 -0400, Rob Crittenden wrote:
Martin Kosek wrote:
On Wed, 2012-03-07 at 17:49 -0500, Rob Crittenden wrote:
Add subject key identifier to the dogtag server cert profile.
This will add it on upgrades too and any new certs issued will have a
subject key identifier set.
If the user has customized the profile themselves then this won't be
applied.
rob
NACK
I found few issues with the patch:
1) There is an extraneous pdb statement:
+import pdb; pdb.set_trace()
2) A name of config file should be put to some variable once and not
created every time again in enable_subject_key_identifier. It would be
much more readable and less error prone:
+installutils.set_directive('/var/lib/%
s/profiles/ca/caIPAserviceCert.cfg' % PKI_INSTANCE_NAME,
'policyset.serverCertSet.list', '1,2,3,4,5,6,7,8,10', quotes=False,
separator='=')
+installutils.set_directive('/var/lib/%
s/profiles/ca/caIPAserviceCert.cfg' % PKI_INSTANCE_NAME,
'policyset.serverCertSet.10.constraint.class_id', 'noConstraintImpl',
quotes=False, separator='=')
...
3) We do not handle gracefully missing config file. This is what happens
when replica without CA is upgraded:
# rpm -Uvh --force /home/mkosek/dist-review/rpms/freeipa-*
Preparing...###
[100%]
1:freeipa-python ### [
17%]
2:freeipa-client ### [
33%]
3:freeipa-admintools ### [
50%]
4:freeipa-server ### [
67%]
Upgraded /etc/httpd/conf.d/ipa-pki-proxy.conf to version 1
Traceback (most recent call last):
File /usr/sbin/ipa-upgradeconfig, line 301, inmodule
sys.exit(main())
File /usr/sbin/ipa-upgradeconfig, line 297, in main
upgrade_ipa_profile(krbctx.default_realm)
File /usr/sbin/ipa-upgradeconfig, line 243, in upgrade_ipa_profile
if ca.enable_subject_key_identifier():
File /usr/lib/python2.7/site-packages/ipaserver/install/cainstance.py,
line 1079, in enable_subject_key_identifier
setlist =
installutils.get_directive('/var/lib/%s/profiles/ca/caIPAserviceCert.cfg' %
PKI_INSTANCE_NAME, 'policyset.serverCertSet.list', separator='=')
File
/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py, line
429, in get_directive
fd = open(filename, r)
IOError: [Errno 2] No such file or directory:
'/var/lib/pki-ca/profiles/ca/caIPAserviceCert.cfg'
5:freeipa-server-selinux ### [
83%]
6:freeipa-debuginfo ###
[100%]
1. Martin
I think this should do it.
rob
Yup, its much better. ACK. Pushed to master, ipa-2-2.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 232 Treat UPGs correctly in winsync replication
On Wed, 2012-03-14 at 22:25 -0400, Rob Crittenden wrote: Martin Kosek wrote: There are some test hints attached to the ticket. --- IPA winsync plugin failed to replicate users when default user group was non-posix even though User Private Groups (UPG) were enabled on the server. Both their uidNumber and gidNumber were empty and they missed essential object classes. When the default user group was made posix and UPG was disabled it did not set gidNumber to the default group gidNumber. This patch improves this behavior to set gidNumber correctly according to UPG configuration and the default group status (posix/non-posix). 4 situations can occur, the following list specifies what value is assigned to user gidNumber: 1) Default group posix, UPG enabled: gidNumber = UPG gidNumber 2) Default group posix, UPG disabled: gidNumber = default group gidNumber 3) Default group non-posix, UPG enabled: gidNumber = UPG gidNumber 4) Default group non-posix, UPG disabled: an error is printed to the dirsrv log as the gidNumber cannot be retrieved. User is replicated in the same way as before this patch, i.e. without essential object classes. https://fedorahosted.org/freeipa/ticket/2436 ACK, works as advertised. rob Pushed to master, ipa-2-2. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 236 Amend permissions for new DNS attributes
New features in bind-dyndb-ldap and IPA DNS plugin pulled new attributes and objectclasses. ACIs and permissions need to be updated to allow users with appropriate permissions update these attributes in LDAP. This patch updates the ACI for DNS record updates and adds one new permission to update global DNS configuration. https://fedorahosted.org/freeipa/ticket/2510 From 905f923a817b0ee05a33d8802e9e28b072d18785 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Wed, 14 Mar 2012 10:38:33 +0100 Subject: [PATCH] Amend permissions for new DNS attributes New features in bind-dyndb-ldap and IPA DNS plugin pulled new attributes and objectclasses. ACIs and permissions need to be updated to allow users with appropriate permissions update these attributes in LDAP. This patch updates the ACI for DNS record updates and adds one new permission to update global DNS configuration. https://fedorahosted.org/freeipa/ticket/2510 --- install/share/dns.ldif | 12 +++- install/updates/40-dns.update|4 ipaserver/install/plugins/dns.py | 35 +++ 3 files changed, 50 insertions(+), 1 deletions(-) diff --git a/install/share/dns.ldif b/install/share/dns.ldif index 5a60bc11bf6f16271ad6d9e19b943335b9bfd9b5..3fd8cfb87f12c815eafd749b8a310f1f37baa1b4 100644 --- a/install/share/dns.ldif +++ b/install/share/dns.ldif @@ -10,7 +10,8 @@ changetype: modify add: aci aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)(version 3.0;acl permission:add dns entries;allow (add) groupdn = ldap:///cn=add dns entries,cn=permissions,cn=pbac,$SUFFIX;) aci: (target = ldap:///idnsname=*,cn=dns,$SUFFIX;)(version 3.0;acl permission:remove dns entries;allow (delete) groupdn = ldap:///cn=remove dns entries,cn=permissions,cn=pbac,$SUFFIX;) -aci: (targetattr = idnsname || cn || idnsallowdynupdate || dnsttl || dnsclass || arecord || record || a6record || nsrecord || cnamerecord || ptrrecord || srvrecord || txtrecord || mxrecord || mdrecord || hinforecord || minforecord || afsdbrecord || sigrecord || keyrecord || locrecord || nxtrecord || naptrrecord || kxrecord || certrecord || dnamerecord || dsrecord || sshfprecord || rrsigrecord || nsecrecord || idnsname || idnszoneactive || idnssoamname || idnssoarname || idnssoaserial || idnssoarefresh || idnssoaretry || idnssoaexpire || idnssoaminimum || idnsupdatepolicy)(target = ldap:///idnsname=*,cn=dns,$SUFFIX;)(version 3.0;acl permission:update dns entries;allow (write) groupdn = ldap:///cn=update dns entries,cn=permissions,cn=pbac,$SUFFIX;) +aci: (targetattr = idnsname || cn || idnsallowdynupdate || dnsttl || dnsclass || arecord || record || a6record || nsrecord || cnamerecord || ptrrecord || srvrecord || txtrecord || mxrecord || mdrecord || hinforecord || minforecord || afsdbrecord || sigrecord || keyrecord || locrecord || nxtrecord || naptrrecord || kxrecord || certrecord || dnamerecord || dsrecord || sshfprecord || rrsigrecord || nsecrecord || idnsname || idnszoneactive || idnssoamname || idnssoarname || idnssoaserial || idnssoarefresh || idnssoaretry || idnssoaexpire || idnssoaminimum || idnsupdatepolicy || idnsallowquery || idnsallowtransfer || idnsallowsyncptr || idnsforwardpolicy || idnsforwarders)(target = ldap:///idnsname=*,cn=dns,$SUFFIX;)(version 3.0;acl permission:update dns entries;allow (write) groupdn = ldap:///cn=update dns entries,cn=permissions,cn=pbac,$SUFFIX;) +aci: (targetattr = idnsforwardpolicy || idnsforwarders || idnsallowsyncptr || idnszonerefresh || idnspersistentsearch)(target = ldap:///cn=dns,$SUFFIX;)(version 3.0;acl permission:Write DNS Configuration;allow (write) groupdn = ldap:///cn=Write DNS Configuration,cn=permissions,cn=pbac,$SUFFIX;) dn: cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX changetype: add @@ -54,3 +55,12 @@ cn: update dns entries description: Update DNS entries member: cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX member: cn=DNS Servers,cn=privileges,cn=pbac,$SUFFIX + +dn: cn=Write DNS Configuration,cn=permissions,cn=pbac,$SUFFIX +changetype: add +objectClass: groupofnames +objectClass: top +cn: Write DNS Configuration +description: Write DNS Configuration +member: cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX +member: cn=DNS Servers,cn=privileges,cn=pbac,$SUFFIX diff --git a/install/updates/40-dns.update b/install/updates/40-dns.update index ef2627bd7f04bc92acd540b743ee6286948d791d..02af8e467c99f905232b785b3677f44020a69c40 100644 --- a/install/updates/40-dns.update +++ b/install/updates/40-dns.update @@ -23,3 +23,7 @@ add: ttl: 10 # add idnsConfigObject if it is not there already dn: cn=dns, $SUFFIX addifexist: objectClass: idnsConfigObject + +# update DNS acis with new idnsRecord attributes +dn: $SUFFIX +replace:aci:'(targetattr = idnsname || cn || idnsallowdynupdate || dnsttl || dnsclass || arecord || record || a6record || nsrecord || cnamerecord || ptrrecord || srvrecord || txtrecord || mxrecord || mdrecord || hinforecord || minforecord ||
Re: [Freeipa-devel] [PATCH] 924 display both hex and decimal serial numbers
On 03/13/2012 10:57 PM, Rob Crittenden wrote: Jan Cholasta wrote: ..snip.. This patch works for me, but let me repeat myself: BTW, wouldn't it make sense to format serial numbers in the cert plugin in the same way? Honza Updated. rob diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py index b31058c1418f7f73854f7ac06701fb6f821a2e40..b56e04f4d8675c34cc5e7db42a1b45402ef79084 100644 --- a/ipaserver/plugins/dogtag.py +++ b/ipaserver/plugins/dogtag.py @@ -609,6 +609,7 @@ def parse_profile_submit_result_xml(doc): if len(serial_number) == 1: serial_number = int(serial_number[0].text, 16) # parse as hex response_request['serial_number'] = serial_number +response['serial_number_hex'] = u'0x%X' % serial_number Why are you adding serial_number_hex to the entire response, when serial_number is on the individual response_request? (same in parse_revoke_cert_xml) I think you should also update the docstrings with the new item. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0014 Add final debug message in installers
On 03/01/2012 11:45 AM, Petr Viktorin wrote:
On 02/29/2012 07:46 PM, Rob Crittenden wrote:
Martin Kosek wrote:
On Mon, 2012-02-27 at 17:51 +0100, Petr Viktorin wrote:
On 02/22/2012 10:41 AM, Petr Viktorin wrote:
This fixes https://fedorahosted.org/freeipa/ticket/2071 (Add final
debug
message in installers). The try/except blocks at the end of
installers/management scripts are replaced by a call to a common
function, which includes the final message.
Obviously the installers still need some more love. This is as far
as I
got before Martin stopped me, saying I shouldn't change too much
before
a release :)
If it's still too many changes to test, I could just wrap the
blocks in
some `with add_final_message` block for now, and resubmit this patch
after the release.
Yeah, this is exactly the kind of changes that can have yet-unseen
consequences and I don't like pushing this close to the release.
The original ticket just asks for a debug message when the install
script ends. If possible, I would really prefer to have some low-risk
patch adding just those and leave install script refactoring for next
big release, like 3.x. Rob, what's your opinion on this?
Martin
Yes, I agree. Simpler is better at this point.
rob
This patch simply wraps the try blocks in a context that logs the final
result.
Most of the changes are indentation; diff with -w to see the additions.
Not sure if this would count as an update or a new patch...
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Rebased against current master.
--
Petr³
From b4d10086b34b63215af1437c138d8857b2355962 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 1 Mar 2012 05:29:52 -0500
Subject: [PATCH] Add final debug message in installers
Invocation of install tools is wrapped in a context manager that logs
the final success or failure.
https://fedorahosted.org/freeipa/ticket/2071
---
install/tools/ipa-adtrust-install| 47 +++---
install/tools/ipa-ca-install | 71 +-
install/tools/ipa-compat-manage | 43 ++--
install/tools/ipa-compliance | 17
install/tools/ipa-csreplica-manage | 33
install/tools/ipa-dns-install| 47 +++---
install/tools/ipa-ldap-updater | 27 +++--
install/tools/ipa-managed-entries| 35 +
install/tools/ipa-nis-manage | 43 ++--
install/tools/ipa-replica-conncheck | 33
install/tools/ipa-replica-install| 71 +-
install/tools/ipa-replica-manage | 45 +++--
install/tools/ipa-replica-prepare| 33
install/tools/ipa-server-certinstall | 17
install/tools/ipa-server-install | 66 ---
install/tools/ipa-upgradeconfig | 16 ---
ipaserver/install/installutils.py| 16
17 files changed, 347 insertions(+), 313 deletions(-)
diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 248ea35eaa86dd59ebbc871b86df780cfd71ccf6..c3558fce0cceae879f83b61e0057d14fe42811de 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -227,26 +227,27 @@ def main():
return 0
-try:
-sys.exit(main())
-except SystemExit, e:
-sys.exit(e)
-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
-message = str(e)
-for str in traceback.format_tb(sys.exc_info()[2]):
-message = message + \n + str
-root_logger.debug(message)
-sys.exit(1)
+with installutils.script_context('ipa-adtrust-install'):
+try:
+sys.exit(main())
+except SystemExit, e:
+sys.exit(e)
+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
[Freeipa-devel] [PATCHES] Improve framework parameter validation
(this is a continuation of
http://www.redhat.com/archives/freeipa-devel/2011-September/msg00327.html)
Hi,
the attached patches fix https://fedorahosted.org/freeipa/ticket/1847
and https://fedorahosted.org/freeipa/ticket/2245:
[PATCH] Fix the procedure for getting default values of command parameters.
The parameters used in default_from of other parameters are now properly
validated before the default_from is called.
[PATCH] Change parameters to use only default_from for dynamic default
values.
Replace all occurences of create_default with equivalent default_from
and remove create_default from the framework. This is needed for proper
parameter validation, as there is no way to tell which parameters to
validate prior to calling create_default, because create_default does
not provide information about which parameters are used for generating
the default value.
Honza
--
Jan Cholasta
From c5e1f63ea3bcb15fce5c90aafe700a23565b2213 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 16 Jan 2012 09:21:50 -0500
Subject: [PATCH 2/2] Fix the procedure for getting default values of command
parameters.
The parameters used in default_from of other parameters are now
properly validated before the default_from is called.
ticket 1847
---
ipalib/cli.py |8 ++--
ipalib/frontend.py| 79 -
ipalib/plugins/dns.py | 15 -
3 files changed, 75 insertions(+), 27 deletions(-)
diff --git a/ipalib/cli.py b/ipalib/cli.py
index 7af6376..b955082 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -48,7 +48,7 @@ import plugable
import util
from errors import (PublicError, CommandError, HelpError, InternalError,
NoSuchNamespaceError, ValidationError, NotFound, NotConfiguredError,
-PromptFailed)
+PromptFailed, ConversionError)
from constants import CLI_TAB
from parameters import Password, Bytes, File, Str, StrEnum
from text import _
@@ -1151,7 +1151,7 @@ class cli(backend.Executioner):
if (param.required and param.name not in kw) or \
(param.alwaysask and honor_alwaysask) or self.env.prompt_all:
if param.autofill:
-kw[param.name] = param.get_default(**kw)
+kw[param.name] = cmd.get_default_of(param.name, **kw)
if param.name in kw and kw[param.name] is not None:
continue
if param.password:
@@ -1159,7 +1159,7 @@ class cli(backend.Executioner):
param.label, param.confirm
)
else:
-default = param.get_default(**kw)
+default = cmd.get_default_of(param.name, **kw)
error = None
while True:
if error is not None:
@@ -1171,7 +1171,7 @@ class cli(backend.Executioner):
if value is not None:
kw[param.name] = value
break
-except ValidationError, e:
+except (ValidationError, ConversionError), e:
error = e.error
elif param.password and kw.get(param.name, False) is True:
kw[param.name] = self.Backend.textui.prompt_password(
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index da25b4c..6d7a696 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -396,6 +396,7 @@ class Command(HasParam):
args = Plugin.finalize_attr('args')
options = Plugin.finalize_attr('options')
params = Plugin.finalize_attr('params')
+params_by_default = Plugin.finalize_attr('params_by_default')
obj = None
use_output_validation = True
@@ -419,11 +420,7 @@ class Command(HasParam):
self.debug(
'raw: %s(%s)', self.name, ', '.join(self._repr_iter(**params))
)
-while True:
-default = self.get_default(**params)
-if len(default) == 0:
-break
-params.update(default)
+params.update(self.get_default(**params))
params = self.normalize(**params)
params = self.convert(**params)
self.debug(
@@ -628,17 +625,53 @@ class Command(HasParam):
c.get_default(color=u'Yellow')
{}
-return dict(self.__get_default_iter(kw))
+params = [p.name for p in self.params() if p.name not in kw and (p.required or p.autofill)]
+return dict(self.__get_default_iter(params, kw))
-def __get_default_iter(self, kw):
+def get_default_of(self, name, **kw):
-Generator method used by `Command.get_default`.
+Return default value for parameter `name`.
-for param in self.params():
-if param.name in kw:
-continue
-if param.required or param.autofill:
-default =
Re: [Freeipa-devel] [PATCHES] Improve framework parameter validation
On 15.3.2012 11:36, Jan Cholasta wrote:
(this is a continuation of
http://www.redhat.com/archives/freeipa-devel/2011-September/msg00327.html)
Hi,
the attached patches fix https://fedorahosted.org/freeipa/ticket/1847
and https://fedorahosted.org/freeipa/ticket/2245:
[PATCH] Fix the procedure for getting default values of command parameters.
The parameters used in default_from of other parameters are now properly
validated before the default_from is called.
[PATCH] Change parameters to use only default_from for dynamic default
values.
Replace all occurences of create_default with equivalent default_from
and remove create_default from the framework. This is needed for proper
parameter validation, as there is no way to tell which parameters to
validate prior to calling create_default, because create_default does
not provide information about which parameters are used for generating
the default value.
Honza
Forgot to remove one FIXME bit in dns.py. Update patch attached.
Honza
--
Jan Cholasta
From 0191725580268c280f02b3428f7de2effd7a4b02 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Mon, 16 Jan 2012 09:21:50 -0500
Subject: [PATCH 2/2] Fix the procedure for getting default values of command
parameters.
The parameters used in default_from of other parameters are now
properly validated before the default_from is called.
ticket 1847
---
ipalib/cli.py |8 ++--
ipalib/frontend.py| 79 -
ipalib/plugins/dns.py | 22 --
3 files changed, 75 insertions(+), 34 deletions(-)
diff --git a/ipalib/cli.py b/ipalib/cli.py
index 7af6376..b955082 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -48,7 +48,7 @@ import plugable
import util
from errors import (PublicError, CommandError, HelpError, InternalError,
NoSuchNamespaceError, ValidationError, NotFound, NotConfiguredError,
-PromptFailed)
+PromptFailed, ConversionError)
from constants import CLI_TAB
from parameters import Password, Bytes, File, Str, StrEnum
from text import _
@@ -1151,7 +1151,7 @@ class cli(backend.Executioner):
if (param.required and param.name not in kw) or \
(param.alwaysask and honor_alwaysask) or self.env.prompt_all:
if param.autofill:
-kw[param.name] = param.get_default(**kw)
+kw[param.name] = cmd.get_default_of(param.name, **kw)
if param.name in kw and kw[param.name] is not None:
continue
if param.password:
@@ -1159,7 +1159,7 @@ class cli(backend.Executioner):
param.label, param.confirm
)
else:
-default = param.get_default(**kw)
+default = cmd.get_default_of(param.name, **kw)
error = None
while True:
if error is not None:
@@ -1171,7 +1171,7 @@ class cli(backend.Executioner):
if value is not None:
kw[param.name] = value
break
-except ValidationError, e:
+except (ValidationError, ConversionError), e:
error = e.error
elif param.password and kw.get(param.name, False) is True:
kw[param.name] = self.Backend.textui.prompt_password(
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index da25b4c..6d7a696 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -396,6 +396,7 @@ class Command(HasParam):
args = Plugin.finalize_attr('args')
options = Plugin.finalize_attr('options')
params = Plugin.finalize_attr('params')
+params_by_default = Plugin.finalize_attr('params_by_default')
obj = None
use_output_validation = True
@@ -419,11 +420,7 @@ class Command(HasParam):
self.debug(
'raw: %s(%s)', self.name, ', '.join(self._repr_iter(**params))
)
-while True:
-default = self.get_default(**params)
-if len(default) == 0:
-break
-params.update(default)
+params.update(self.get_default(**params))
params = self.normalize(**params)
params = self.convert(**params)
self.debug(
@@ -628,17 +625,53 @@ class Command(HasParam):
c.get_default(color=u'Yellow')
{}
-return dict(self.__get_default_iter(kw))
+params = [p.name for p in self.params() if p.name not in kw and (p.required or p.autofill)]
+return dict(self.__get_default_iter(params, kw))
-def __get_default_iter(self, kw):
+def get_default_of(self, name, **kw):
-Generator method used by `Command.get_default`.
+Return default value for parameter `name`.
-for param in self.params():
-if param.name in kw:
[Freeipa-devel] [PATCH] 237 Improve user awareness about dnsconfig
Global DNS configuration is a nice tool to maintain a common DNS
settings stored in LDAP which are then used for all enrolled IPA
servers. However, the settings stored in LDAP override local
settings in named.conf on DNS servers.
This patch adds more information about global DNS configuration
options in install scripts and DNS module help.
https://fedorahosted.org/freeipa/ticket/2525
From 1f08ae0f10117ea279843615745e88c81201dbba Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Thu, 15 Mar 2012 13:51:59 +0100
Subject: [PATCH] Improve user awareness about dnsconfig
Global DNS configuration is a nice tool to maintain a common DNS
settings stored in LDAP which are then used for all enrolled IPA
servers. However, the settings stored in LDAP override local
settings in named.conf on DNS servers.
This patch adds more information about global DNS configuration
options in install scripts and DNS module help.
https://fedorahosted.org/freeipa/ticket/2525
---
install/tools/ipa-dns-install |3 +++
install/tools/ipa-replica-install |4
install/tools/ipa-server-install |3 +++
ipalib/plugins/dns.py | 22 ++
ipaserver/install/bindinstance.py | 20
5 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index 096020c5e2619c3719eed15098ec2b1239b720ce..b540630f4f2782603c31ce1905870c38c9af98ab 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -232,6 +232,9 @@ def main():
print ==
print Setup complete
print
+bind.check_global_configuration()
+print
+print
print \tYou must make sure these network ports are open:
print \t\tTCP Ports:
print \t\t * 53: bind
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 65f5229df222a54a6a159d6f2f31067015369d8d..07b1781ee7f99cacf1a3abbd6207b95f38da1807 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -229,6 +229,10 @@ def install_bind(config, options):
config.domain_name, forwarders, options.conf_ntp, reverse_zone)
bind.create_instance()
+print
+bind.check_global_configuration()
+print
+
def install_dns_records(config, options):
if not bindinstance.dns_container_exists(config.master_host_name,
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 9c7388b40dc00385e816bd939a1a843070eea662..1dd02ba870a02e902c4c345d9f5802ef09f78a7a 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -1019,6 +1019,9 @@ def main():
api.Backend.ldap2.connect(bind_dn=cn=Directory Manager, bind_pw=dm_password)
bind.create_instance()
+print
+bind.check_global_configuration()
+print
else:
bind.create_sample_bind_zone()
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index a10960a2c20b8915b199ed82462a844ce8f5915c..a70d889dc28ef029bf7cd12d7b2bfb8d3e741679 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -189,6 +189,14 @@ EXAMPLES:
ipa dns-resolve www.example.com
ipa dns-resolve www
+
+GLOBAL DNS CONFIGURATION
+
+DNS configuration passed to command line install script is stored in a local
+configuration file on each IPA server where DNS service is configured. These
+local settings can be overridden with a common configuration stored in LDAP
+server:
+
Show global DNS configuration:
ipa dnsconfig-show
@@ -2645,16 +2653,30 @@ class dnsconfig(LDAPObject):
return entry
+def postprocess_result(self, result):
+if not any(param in result['result'] for param in self.params):
+result['summary'] = unicode(_('Global DNS configuration is empty'))
+
api.register(dnsconfig)
class dnsconfig_mod(LDAPUpdate):
__doc__ = _('Modify global DNS configuration.')
+def execute(self, *keys, **options):
+result = super(dnsconfig_mod, self).execute(*keys, **options)
+self.obj.postprocess_result(result)
+return result
+
api.register(dnsconfig_mod)
class dnsconfig_show(LDAPRetrieve):
__doc__ = _('Show the current global DNS configuration.')
+def execute(self, *keys, **options):
+result = super(dnsconfig_show, self).execute(*keys, **options)
+self.obj.postprocess_result(result)
+return result
+
api.register(dnsconfig_show)
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index a37a29303909b89c3f2c42e5561e5c6279344cb5..ba8b7b5cc3c7f327fb86b98a3cc6d11720ed1d47 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -637,6 +637,26 @@ class BindInstance(service.Service):
# remove also master NS record from the reverse zone
[Freeipa-devel] [PATCH] 0028 Remove ipausers' gidnumber from tests
Here's an one-liner: don't test for the gidnumber attribute on ipausers,
since it's not there by default now.
--
Petr³
From 600c7722db6accd17e97425914cc69b08b082eb7 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 15 Mar 2012 08:49:41 -0400
Subject: [PATCH] Remove ipausers' gidnumber from tests
The ipausers group is no longer a POSIX group by default.
Reflect that in the tests.
---
tests/test_xmlrpc/test_group_plugin.py |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/tests/test_xmlrpc/test_group_plugin.py b/tests/test_xmlrpc/test_group_plugin.py
index 3ef60f693815dd9a6a00e647394fa3ed4352ebb2..44682b69ce09450904e7825b89b9ac12c8bb5a19 100644
--- a/tests/test_xmlrpc/test_group_plugin.py
+++ b/tests/test_xmlrpc/test_group_plugin.py
@@ -368,7 +368,6 @@ class test_group(Declarative):
'dn': lambda x: DN(x) == \
DN(('cn','ipausers'),('cn','groups'),('cn','accounts'),
api.env.basedn),
-'gidnumber': [fuzzy_digits],
'cn': [u'ipausers'],
'description': [u'Default group for all users'],
},
--
1.7.7.6
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] Improve framework parameter validation
On 03/15/2012 12:05 PM, Jan Cholasta wrote:
On 15.3.2012 11:36, Jan Cholasta wrote:
(this is a continuation of
http://www.redhat.com/archives/freeipa-devel/2011-September/msg00327.html)
Hi,
the attached patches fix https://fedorahosted.org/freeipa/ticket/1847
and https://fedorahosted.org/freeipa/ticket/2245:
[PATCH] Fix the procedure for getting default values of command
parameters.
The parameters used in default_from of other parameters are now properly
validated before the default_from is called.
[PATCH] Change parameters to use only default_from for dynamic default
values.
Replace all occurences of create_default with equivalent default_from
and remove create_default from the framework. This is needed for proper
parameter validation, as there is no way to tell which parameters to
validate prior to calling create_default, because create_default does
not provide information about which parameters are used for generating
the default value.
Honza
Forgot to remove one FIXME bit in dns.py. Update patch attached.
Honza
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index a10960a..61c645d 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -1528,7 +1528,7 @@ class dnszone(LDAPObject):
label=_('SOA serial'),
doc=_('SOA record serial number'),
minvalue=1,
-create_default=_create_zone_serial,
+default_from=_create_zone_serial,
autofill=True,
),
Int('idnssoarefresh',
diff --git a/ipalib/plugins/passwd.py b/ipalib/plugins/passwd.py
index b26f7e9..9bee314 100644
--- a/ipalib/plugins/passwd.py
+++ b/ipalib/plugins/passwd.py
@@ -69,7 +69,7 @@ class passwd(Command):
label=_('User name'),
primary_key=True,
autofill=True,
-create_default=lambda **kw: util.get_current_principal(),
+default_from=lambda: util.get_current_principal(),
normalizer=lambda value: normalize_principal(value),
),
Password('password',
This is just a minor nitpick, but I'd like to know if there's a reason
behind it: why are you sometimes using lambda and sometimes not?
The patch works well here, but I think I'm not the one to ack it.
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] Improve framework parameter validation
On 15.3.2012 14:20, Petr Viktorin wrote:
On 03/15/2012 12:05 PM, Jan Cholasta wrote:
On 15.3.2012 11:36, Jan Cholasta wrote:
(this is a continuation of
http://www.redhat.com/archives/freeipa-devel/2011-September/msg00327.html)
Hi,
the attached patches fix https://fedorahosted.org/freeipa/ticket/1847
and https://fedorahosted.org/freeipa/ticket/2245:
[PATCH] Fix the procedure for getting default values of command
parameters.
The parameters used in default_from of other parameters are now properly
validated before the default_from is called.
[PATCH] Change parameters to use only default_from for dynamic default
values.
Replace all occurences of create_default with equivalent default_from
and remove create_default from the framework. This is needed for proper
parameter validation, as there is no way to tell which parameters to
validate prior to calling create_default, because create_default does
not provide information about which parameters are used for generating
the default value.
Honza
Forgot to remove one FIXME bit in dns.py. Update patch attached.
Honza
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index a10960a..61c645d 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -1528,7 +1528,7 @@ class dnszone(LDAPObject):
label=_('SOA serial'),
doc=_('SOA record serial number'),
minvalue=1,
- create_default=_create_zone_serial,
+ default_from=_create_zone_serial,
autofill=True,
),
Int('idnssoarefresh',
diff --git a/ipalib/plugins/passwd.py b/ipalib/plugins/passwd.py
index b26f7e9..9bee314 100644
--- a/ipalib/plugins/passwd.py
+++ b/ipalib/plugins/passwd.py
@@ -69,7 +69,7 @@ class passwd(Command):
label=_('User name'),
primary_key=True,
autofill=True,
- create_default=lambda **kw: util.get_current_principal(),
+ default_from=lambda: util.get_current_principal(),
normalizer=lambda value: normalize_principal(value),
),
Password('password',
This is just a minor nitpick, but I'd like to know if there's a reason
behind it: why are you sometimes using lambda and sometimes not?
I use lambda as a protective measure against accidents caused by adding
optional arguments to the functions used. _create_zone_serial is an
exception to that rule, because it is private to the dns plugin.
The patch works well here, but I think I'm not the one to ack it.
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 108 Better hbactest validation message
On 03/14/2012 03:13 PM, Endi Sukma Dewata wrote: On 3/12/2012 8:22 AM, Petr Vobornik wrote: HBAC Test validation message now contains all missing values in form of list of links instead of general 'missing values' message and redirection to first missing value's facet. When a link is clicked user is redirected to value's facet. https://fedorahosted.org/freeipa/ticket/2182 Note: In a list I chose to display an option label instead of facet label. IMHO it seems better with message 'Missing values'. It looks good. ACK. Pushed to master, ipa-2-2. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 107 Fixed evaluating checkbox dirty status
On 03/14/2012 03:11 PM, Endi Sukma Dewata wrote: ACK. I have some comments below. Pushed to master, ipa-2-2. On 3/9/2012 11:20 AM, Petr Vobornik wrote: Problem: When value in checkbox is modified twice in a row (so it is at its original value) an 'undo' button is still visible even when it shouldn't be. Cause: IPA server sends boolean values as 'TRUE' or 'FALSE' (strings). Checkbox_widget converts them to JavaScript? boolean (true, false). Save method in checkbox_widget is returning array with a boolean. So test_dirty method always evaluates to dirty because 'FALSE' != false. This patch is fixing the problem. Note (future enhancements): As we were talking before about making fields less dependent on widget types. The dependency comes from the fact that dirty evaluation is in field. I plan to move the core to widget. I'm thinking about something like: Widget would have: * input value parser - ie for parsing strings to booleans - configurable * original value (parsed) * inner state (inner_save()) * is_dirty() - compare inner state with original value * output formatter - can make boolean back to strings (just example) - configurable * save() would return array of formatted values Field: * load(record) would pick values from record as now * is_dirty - needed for facets. Would be: dirty = 'one of associated widgets is dirty' * save() - merge associated widgets values - usually only on array from one widget Maybe input and output formatters should be in field. We might need it in both field and widget. There are 3 different values that we need to consider: * stored value (LDAP), e.g. TRUE/FALSE * internal value (JavaScript), e.g. true/false * external value (human readable), e.g. True/False The field will be responsible for converting between stored and internal value. The widget will be responsible for converting between internal external value if needed. Suppose we want to display a boolean attribute using a checkbox. We will need a boolean field which will convert TRUE/FALSE into true/false. Then we also need a standard checkbox that displays true as checked and false as unchecked. Suppose we want to display the boolean attribute in a table column. We should be able to use the same boolean field, but then we use a boolean column that converts true/false into True/False. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 238 Improve automount indirect map error message
When user does not pass a name of parent map in automountmap-add-indirect command, auto.master is used as a default. However, when auto.master does not exist in a given location, we raise NotFound error with a name of a location instead of a name of the missing automount map. https://fedorahosted.org/freeipa/ticket/2387 From e39b385e63490abc97bf19c04b616da01d09f5df Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Thu, 15 Mar 2012 16:10:05 +0100 Subject: [PATCH] Improve automount indirect map error message When user does not pass a name of parent map in automountmap-add-indirect command, auto.master is used as a default. However, when auto.master does not exist in a given location, we raise NotFound error with a name of a location instead of a name of the missing automount map. https://fedorahosted.org/freeipa/ticket/2387 --- ipalib/plugins/automount.py |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/ipalib/plugins/automount.py b/ipalib/plugins/automount.py index 31c143d8418a9101a14e39ab752bb229e4219094..9df400d2ec87088b9f9f41240bec7226cb5f7a22 100644 --- a/ipalib/plugins/automount.py +++ b/ipalib/plugins/automount.py @@ -845,6 +845,10 @@ class automountmap_add_indirect(LDAPCreate): automountkey=options['key'], **kw ) else: # adding to auto.master +# Ensure auto.master exists +self.api.Command['automountmap_show']( +keys[0], options['parentmap'] +) options['automountinformation'] = keys[1] self.api.Command['automountkey_add']( keys[0], u'auto.master', -- 1.7.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 924 display both hex and decimal serial numbers
Petr Viktorin wrote: On 03/13/2012 10:57 PM, Rob Crittenden wrote: Jan Cholasta wrote: ..snip.. This patch works for me, but let me repeat myself: BTW, wouldn't it make sense to format serial numbers in the cert plugin in the same way? Honza Updated. rob diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py index b31058c1418f7f73854f7ac06701fb6f821a2e40..b56e04f4d8675c34cc5e7db42a1b45402ef79084 100644 --- a/ipaserver/plugins/dogtag.py +++ b/ipaserver/plugins/dogtag.py @@ -609,6 +609,7 @@ def parse_profile_submit_result_xml(doc): if len(serial_number) == 1: serial_number = int(serial_number[0].text, 16) # parse as hex response_request['serial_number'] = serial_number + response['serial_number_hex'] = u'0x%X' % serial_number Why are you adding serial_number_hex to the entire response, when serial_number is on the individual response_request? (same in parse_revoke_cert_xml) I think you should also update the docstrings with the new item. What docstrings are those? These calls aren't needed at all. I submitted a new patch, 986, to remove them. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 986 remove unnecessary serial conversions
This is related to my patch 924. Petr Viktorin noticed a couple of
serial to hex conversions were wrong and it turns out they aren't needed
at all. This patch removes them.
rob
From 8d4f4c597fe6daba8c6f4a6b2b747d206bc274dc Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Thu, 15 Mar 2012 13:29:02 -0400
Subject: [PATCH] Remove unnecessary conversions of serial number to hex in
dogtag backend.
These are not needed because they are internal calls. The response to the
user is already handled.
https://fedorahosted.org/freeipa/ticket/1991
---
ipaserver/plugins/dogtag.py |3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index b56e04f4d8675c34cc5e7db42a1b45402ef79084..c2b43596e47e0bbf5a9321985e9ca51e2caaf754 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -835,7 +835,6 @@ def parse_display_cert_xml(doc):
if len(serial_number) == 1:
serial_number = int(serial_number[0].text, 16) # parse as hex
response['serial_number'] = serial_number
-response['serial_number_hex'] = u'0x%X' % serial_number
pkcs7_chain = doc.xpath('//xml/header/pkcs7ChainBase64[1]')
if len(pkcs7_chain) == 1:
@@ -1028,7 +1027,6 @@ def parse_revoke_cert_xml(doc):
if len(serial_number) == 1:
serial_number = int(serial_number[0].text, 16) # parse as hex
response_record['serial_number'] = serial_number
-response['serial_number_hex'] = u'0x%X' % serial_number
error_string = record.xpath('error[1]')
if len(error_string) == 1:
@@ -1190,7 +1188,6 @@ def parse_unrevoke_cert_xml(doc):
if len(serial_number) == 1:
serial_number = int(serial_number[0].text, 16) # parse as hex
response['serial_number'] = serial_number
-response['serial_number_hex'] = u'0x%X' % serial_number
return response
--
1.7.6
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 937 configure /etc/openldap/ldap.conf
Simo Sorce wrote: On Tue, 2012-01-31 at 23:25 -0500, Rob Crittenden wrote: Configure the openldap configuration file with the basics for IPA. This is mostly to make querying with StartTLS easier but it does make ldapsearch a lot nicer in general. I got a little carried away with the man page. I wanted to include that we were updating yet another configuration file and found no FILES section at all so I added one. I think I caught every file we update, it is the bulk in any case. Ack Simo. Rebased and pushed to master and ipa-2-2 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0015 Only split CSV strings once
Petr Viktorin wrote:
https://fedorahosted.org/freeipa/ticket/2227 (Unable to add certain sudo
commands to groups). What an interesting bug to get :)
One problem with our CSV splitting is that it's not idempotent
(baskslashes are eaten when there are escaped commas), but when we
forward a call it gets done on both the client and the server. The
attached patch fixes that in a somewhat hacky way. It's not a complete
fix for the issue though, for that I need to ask what to do.
Some Params use the CSV format when we just need a list of
comma-separated values. Our flavor of CSV does escaping in two different
ways. This is pretty bad for predictability, least-surprise,
documentation, ...
To recap, the two ways are escaping (escaped commas are ignored,
backslashes also need to be escaped) and quoting (values are optionally
enclosed in double quotes, to include a '' in a quoted string, use '').
Escaping is good because users are used to it, but currently literal
backslashes need to be doubled, making multivalue syntax different from
single-value syntax, even if there are no commas in the value.
Quoting is weird, but complete by itself so it doesn't also need escaping.
Do we need to use both? Is this documented somewhere? Some of our tests
check only for one, some assume the other. Users are probably even more
confused.
If we only keep one of those, the fix for #2227 should be quite easy.
If not (backwards compatibility), we need to document this properly,
test all the corner cases, and fix the UI to handle CSV escaping.
Since it's subtly broken in lots of places, maybe it's best to break
backwards compatibility and go for a simple solution now.
Anyway, if I want to do a proper fix, CSV handling should be only done
on the client. Unfortunately turning off CSV handling in the server will
break the UI, which at places uses `join(',')` (no escaping commas, no
single place to change it), even though it can just send a list.
I'm with Honza, not too keen on the _forwarded_call part. I'd rather you
do a mix of comparing self.env.in_server and whether the value is a
basestring to determine if we need to split it.
I'm not sure why this is broken out of normalize. Isn't this normalizing
the incoming values into something more sane?
rob
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0016 Fixes for{add, set, del}attr with managed attributes
Petr Viktorin wrote:
On 02/29/2012 04:34 PM, Petr Viktorin wrote:
On 02/29/2012 03:50 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 02/27/2012 11:03 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
Patch 16 defers validation conversion until after
{add,del,set}attr is
processed, so that we don't search for an integer in a list of
strings
(this caused ticket #2405), and so that the end result of these
operations is validated (#2407).
Patch 17 makes these options honor params marked no_create and
no_update.
https://fedorahosted.org/freeipa/ticket/2405
https://fedorahosted.org/freeipa/ticket/2407
https://fedorahosted.org/freeipa/ticket/2408
NACK on Patch 17 which breaks patch 16.
How is patch 16 broken? It works for me.
My point is they rely on one another, IMHO, so without 17 the reported
problem still exists.
*attr is specifically made to be powerful. We don't want to
arbitrarily
block updating certain values.
Noted
Not having patch 17 means that the problem reported in 2408 still
occurs. It should probably check both the schema and the param to
determine if something can have multiple values and reject that way.
I see the problem now: the certificate subject base is defined as a
multi-value attribute in the LDAP schema. If it's changed to
single-value the existing validation should catch it.
Also, most of the config attributes should probably be required in the
schema. Am I right?
I'm a newbie here; what do I need to do when changing the schema? Is
there a patch that does something similar I could use as an example?
The framework should be able to impose its own single-value will as
well. If a Param is designated as single-value the *attr should honor
it.
Here is the updated patch.
Since *attr is powerful enough to modify 'no_update' Params, which
CRUDUpdate forgets about, I need to check the params of the LDAPObject
itself.
Updating schema is a bit of a nasty business right now. See
10-selinuxusermap.update for an example.
I'll leave schema changes for after the release, then.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Attached patch includes tests. Note that the test depends on my patches
12-13, which make ipasearchrecordslimit required.
I gather that this eliminates the need for patch 17? It seems to work as-is.
The patch doesn't apply because of an encoding change Martin made recently.
It does seem to do the right thing though.
rob
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 988 fix host test
A test to make sure hosts can be renamed using setattr is using an
invalid hostname so the host_mod will never get hit.
rob
From 43266a275dd7ceb452545c36de796c26e886a708 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Thu, 15 Mar 2012 17:01:54 -0400
Subject: [PATCH] Fix test failure testing rename with an invalid
hostname.
Validation is going to catch the invalid hostname before the mod is tried.
---
tests/test_xmlrpc/test_host_plugin.py |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tests/test_xmlrpc/test_host_plugin.py b/tests/test_xmlrpc/test_host_plugin.py
index 8f5bd7cf347dd2f712ff441c90dbf61315be8c6b..5e49e2bade1a585d5c724d9f7f4fb351d34f3091 100644
--- a/tests/test_xmlrpc/test_host_plugin.py
+++ b/tests/test_xmlrpc/test_host_plugin.py
@@ -462,7 +462,7 @@ class test_host(Declarative):
dict(
desc='Try to rename %r' % fqdn1,
-command=('host_mod', [fqdn1], dict(setattr=u'fqdn=changed')),
+command=('host_mod', [fqdn1], dict(setattr=u'fqdn=changed.example.com')),
expected=errors.NotAllowedOnRDN()
),
--
1.7.6
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0028 Remove ipausers' gidnumber from tests
Petr Viktorin wrote: Here's an one-liner: don't test for the gidnumber attribute on ipausers, since it's not there by default now. ACK, pushed to master and ipa-2-2 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0024 Fix expected exception checking in tests
Petr Viktorin wrote: Can you spot the bug in this test code? try: do_invalid_operation() except ExpectedError: pass Our test suite had several of those. Nose provides nice tools, `raises` (a decorator) and `assert_raises` (a context manager) that make checking expected exceptions a lot easier and less error-prone. This patch makes our tests use them. If you didn't catch it, the error is that the test will pass when no exception is raised. Some of our tests handled that by adding an `else: assert False`, or an `assert False` at the end of the try block. For consistency, the patch switches these correct ones to raises/assert_raises as well. I've also uncovered and fixed a few test bugs that were hidden by this. test_1a_automountmap_add_indirect() was failing, checking for the wrong exception. I also suggest using @raises for clarity in another spot. Here are my suggested changes: diff --git a/tests/test_xmlrpc/test_automount_plugin.py b/tests/test_xmlrpc/test _automount_plugin.py index 6abc44f..dedd234 100644 --- a/tests/test_xmlrpc/test_automount_plugin.py +++ b/tests/test_xmlrpc/test_automount_plugin.py @@ -79,12 +79,12 @@ class test_automount(XMLRPC_test): assert res assert_attr_equal(res, 'automountkey', self.keyname) +@raises(errors.DuplicateEntry) def test_4_automountkey_add(self): Test adding a duplicate key using `xmlrpc.automountkey_add` method. -with assert_raises(errors.DuplicateEntry): -api.Command['automountkey_add'](self.locname, self.mapname, **self. key_kw) +res = api.Command['automountkey_add'](self.locname, self.mapname, **sel f.key_kw) def test_5_automountmap_show(self): @@ -247,12 +247,12 @@ class test_automount_indirect(XMLRPC_test): assert res assert_attr_equal(res, 'automountmapname', self.mapname) +@raises(errors.DuplicateEntry) def test_1a_automountmap_add_indirect(self): Test adding a duplicate indirect map. -with assert_raises(errors.NotFound): -api.Command['automountmap_add_indirect'](self.locname, self.mapname , **self.map_kw)['result'] +api.Command['automountmap_add_indirect'](self.locname, self.mapname, ** self.map_kw)['result'] def test_2_automountmap_show(self): ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
