Endi Sukma Dewata wrote:
On 11/28/2011 12:09 PM, Rob Crittenden wrote:
Some attributes in the framework were not marked as required even though
they are in the schema. These are typically computed values and I think
the intention was to not prompt for them. This has the side-effect of
them showing as not required in the UI even though they are.

Since they all have default values set they won't be prompted for on the
CLI so there won't be any practical changes.

This patch fixes the problem with required attributes in DNS Zones and
cn, uidNumber, and gidNumber in Users. The UI now shows the required
indicators for these attributes. So this patch is ACKed.

Some problems mentioned in ticket #2015 are still present:

1. Removing the homeDirectory from a user fails because it's required by
posixAccount.

2. Removing the gidNumber from a group fails because it's required by
posixGroup.

3. Removing config attributes listed in the ticket generates internal
error. I think at least the server should return a proper error message.
The required indicator can be hard-coded in the UI if necessary.


I know you acked this already but I went ahead and addressed #1 and #2 and updated the patch.

For #3 I filed a new ticket, https://fedorahosted.org/freeipa/ticket/2159

rob
>From 27feb376d5df5812abe35b7a903bd8c16cb25be3 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Mon, 28 Nov 2011 12:31:45 -0500
Subject: [PATCH] Mark some attributes required to match the schema.

This makes no changes to the functionality in the command-line or
GUI because these all have defaults anyway. This is mostly to show
them properly in the UI and prevent someone from trying to erase the
value (and getting a nasty schema error in response).

https://fedorahosted.org/freeipa/ticket/2015
---
 API.txt                 |   16 ++++++++--------
 ipalib/plugins/dns.py   |   10 +++++-----
 ipalib/plugins/group.py |   11 +++++++++++
 ipalib/plugins/user.py  |   12 +++++++-----
 4 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/API.txt b/API.txt
index d2a58a3..1863dc2 100644
--- a/API.txt
+++ b/API.txt
@@ -865,11 +865,11 @@ arg: Str('idnsname', attribute=True, cli_name='name', multivalue=False, primary_
 option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue=False, required=False)
 option: Str('idnssoamname', attribute=True, cli_name='name_server', multivalue=False, required=True)
 option: Str('idnssoarname', attribute=True, cli_name='admin_email', multivalue=False, required=True)
-option: Int('idnssoaserial', attribute=True, autofill=True, cli_name='serial', minvalue=1, multivalue=False, required=False)
-option: Int('idnssoarefresh', attribute=True, autofill=True, cli_name='refresh', default=3600, minvalue=0, multivalue=False, required=False)
-option: Int('idnssoaretry', attribute=True, autofill=True, cli_name='retry', default=900, minvalue=0, multivalue=False, required=False)
-option: Int('idnssoaexpire', attribute=True, autofill=True, cli_name='expire', default=1209600, minvalue=0, multivalue=False, required=False)
-option: Int('idnssoaminimum', attribute=True, autofill=True, cli_name='minimum', default=3600, maxvalue=10800, minvalue=0, multivalue=False, required=False)
+option: Int('idnssoaserial', attribute=True, autofill=True, cli_name='serial', minvalue=1, multivalue=False, required=True)
+option: Int('idnssoarefresh', attribute=True, autofill=True, cli_name='refresh', default=3600, minvalue=0, multivalue=False, required=True)
+option: Int('idnssoaretry', attribute=True, autofill=True, cli_name='retry', default=900, minvalue=0, multivalue=False, required=True)
+option: Int('idnssoaexpire', attribute=True, autofill=True, cli_name='expire', default=1209600, minvalue=0, multivalue=False, required=True)
+option: Int('idnssoaminimum', attribute=True, autofill=True, cli_name='minimum', default=3600, maxvalue=10800, minvalue=0, multivalue=False, required=True)
 option: Int('dnsttl', attribute=True, cli_name='ttl', multivalue=False, required=False)
 option: StrEnum('dnsclass', attribute=True, cli_name='class', multivalue=False, required=False, values=(u'IN', u'CS', u'CH', u'HS'))
 option: Str('idnsupdatepolicy', attribute=True, cli_name='update_policy', multivalue=False, required=False)
@@ -2894,7 +2894,7 @@ args: 1,31,3
 arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', pattern_errmsg='may only include letters, numbers, _, -, . and $', primary_key=True, required=True)
 option: Str('givenname', attribute=True, cli_name='first', multivalue=False, required=True)
 option: Str('sn', attribute=True, cli_name='last', multivalue=False, required=True)
-option: Str('cn', attribute=True, autofill=True, cli_name='cn', multivalue=False, required=False)
+option: Str('cn', attribute=True, autofill=True, cli_name='cn', multivalue=False, required=True)
 option: Str('displayname', attribute=True, autofill=True, cli_name='displayname', multivalue=False, required=False)
 option: Str('initials', attribute=True, autofill=True, cli_name='initials', multivalue=False, required=False)
 option: Str('homedirectory', attribute=True, cli_name='homedir', multivalue=False, required=False)
@@ -2903,8 +2903,8 @@ option: Str('loginshell', attribute=True, cli_name='shell', default=u'/bin/sh',
 option: Str('krbprincipalname', attribute=True, autofill=True, cli_name='principal', multivalue=False, required=False)
 option: Str('mail', attribute=True, cli_name='email', multivalue=True, required=False)
 option: Password('userpassword', attribute=True, cli_name='password', exclude='webui', multivalue=False, required=False)
-option: Int('uidnumber', attribute=True, autofill=True, cli_name='uid', default=999, minvalue=1, multivalue=False, required=False)
-option: Int('gidnumber', attribute=True, cli_name='gidnumber', multivalue=False, required=False)
+option: Int('uidnumber', attribute=True, autofill=True, cli_name='uid', default=999, minvalue=1, multivalue=False, required=True)
+option: Int('gidnumber', attribute=True, autofill=True, cli_name='gidnumber', multivalue=False, required=True)
 option: Str('street', attribute=True, cli_name='street', multivalue=False, required=False)
 option: Str('l', attribute=True, cli_name='city', multivalue=False, required=False)
 option: Str('st', attribute=True, cli_name='state', multivalue=False, required=False)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 7acb3df..d4f91af 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -673,7 +673,7 @@ class dnszone(LDAPObject):
             default_from=lambda idnsname: 'hostmaster.%s' % idnsname,
             normalizer=normalize_zonemgr,
         ),
-        Int('idnssoaserial?',
+        Int('idnssoaserial',
             cli_name='serial',
             label=_('SOA serial'),
             doc=_('SOA record serial number'),
@@ -681,7 +681,7 @@ class dnszone(LDAPObject):
             create_default=_create_zone_serial,
             autofill=True,
         ),
-        Int('idnssoarefresh?',
+        Int('idnssoarefresh',
             cli_name='refresh',
             label=_('SOA refresh'),
             doc=_('SOA record refresh time'),
@@ -689,7 +689,7 @@ class dnszone(LDAPObject):
             default=3600,
             autofill=True,
         ),
-        Int('idnssoaretry?',
+        Int('idnssoaretry',
             cli_name='retry',
             label=_('SOA retry'),
             doc=_('SOA record retry time'),
@@ -697,7 +697,7 @@ class dnszone(LDAPObject):
             default=900,
             autofill=True,
         ),
-        Int('idnssoaexpire?',
+        Int('idnssoaexpire',
             cli_name='expire',
             label=_('SOA expire'),
             doc=_('SOA record expire time'),
@@ -705,7 +705,7 @@ class dnszone(LDAPObject):
             minvalue=0,
             autofill=True,
         ),
-        Int('idnssoaminimum?',
+        Int('idnssoaminimum',
             cli_name='minimum',
             label=_('SOA minimum'),
             doc=_('How long should negative responses be cached'),
diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py
index 4872872..49bc823 100644
--- a/ipalib/plugins/group.py
+++ b/ipalib/plugins/group.py
@@ -202,8 +202,19 @@ class group_mod(LDAPUpdate):
                 entry_attrs['objectclass'] = old_entry_attrs['objectclass']
                 if not 'gidnumber' in options:
                     entry_attrs['gidnumber'] = 999
+        # Can't check for this in a validator because we lack context
+        if 'gidnumber' in options and options['gidnumber'] is None:
+            raise errors.RequirementError(name='gid')
         return dn
 
+    def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs):
+        # Check again for GID requirement in case someone tried to clear it
+        # using --setattr.
+        if isinstance(exc, errors.ObjectclassViolation):
+            if 'gidNumber' in exc.message and 'posixGroup' in exc.message:
+                raise errors.RequirementError(name='gid')
+        raise exc
+
 api.register(group_mod)
 
 
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index d3e63ef..a3c17dc 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -187,7 +187,7 @@ class user(LDAPObject):
             cli_name='last',
             label=_('Last name'),
         ),
-        Str('cn?',
+        Str('cn',
             label=_('Full name'),
             default_from=lambda givenname, sn: '%s %s' % (givenname, sn),
             autofill=True,
@@ -202,10 +202,11 @@ class user(LDAPObject):
             default_from=lambda givenname, sn: '%c%c' % (givenname[0], sn[0]),
             autofill=True,
         ),
-        Str('homedirectory?',
+        Str('homedirectory',
             cli_name='homedir',
             label=_('Home directory'),
             default_from=lambda uid: '/home/%s' % uid,
+            autofill=True,
         ),
         Str('gecos?',
             label=_('GECOS field'),
@@ -237,7 +238,7 @@ class user(LDAPObject):
             # bomb out via the webUI.
             exclude='webui',
         ),
-        Int('uidnumber?',
+        Int('uidnumber',
             cli_name='uid',
             label=_('UID'),
             doc=_('User ID Number (system will assign one if not provided)'),
@@ -245,10 +246,11 @@ class user(LDAPObject):
             default=999,
             minvalue=1,
         ),
-        Int('gidnumber?',
+        Int('gidnumber',
             label=_('GID'),
             doc=_('Group ID Number'),
-            default_from=lambda uid: uid,
+            default_from=lambda uidnumber: uidnumber,
+            autofill=True,
         ),
         Str('street?',
             cli_name='street',
-- 
1.7.6

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

Reply via email to