[Freeipa-devel] [PATCH] 072 Fix support for nss-pam-ldapd

2011-06-01 Thread Martin Kosek
Test hints:
1) Test with nss-ldap package
- install nss-ldap on the client machine
- install IPA client with --no-sssd option
- `id admin', logging to the machine should work (even after the
restart, i.e. correct services are run after the restart)
2) Test with nss-pam-ldapd
- uninstall nss-ldap, install nss-pam-ldapd
- install IPA client with --no-sssd option
- `id admin', logging to the machine should work 
3) Test with SSSD
- install IPA client
- `id admin', logging to the machine should work

---

Client installation with --no-sssd option was broken if the client
was based on a nss-pam-ldap instead of nss_ldap. The main issue is
with authconfig rewriting the nslcd.conf after it has been
configured by ipa-client-install.

This has been fixed by changing an order of installation steps.
Additionally, nslcd daemon needed for nss-pam-ldap function is
correctly started.

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

From bf81eec9abd7686124a5184a3c5da35d5bf8f5c2 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Wed, 18 May 2011 17:06:15 +0200
Subject: [PATCH] Fix support for nss-pam-ldapd

Client installation with --no-sssd option was broken if the client
was based on a nss-pam-ldap instead of nss_ldap. The main issue is
with authconfig rewriting the nslcd.conf after it has been
configured by ipa-client-install.

This has been fixed by changing an order of installation steps.
Additionally, nslcd daemon needed for nss-pam-ldap function is
correctly started.

https://fedorahosted.org/freeipa/ticket/1235
---
 ipa-client/ipa-install/ipa-client-install |   51 +++--
 1 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 67196022acb3057ebb74121550193dbb3010dbc9..467638940d86891b4e100106f5daf34363bb55ae 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -249,6 +249,20 @@ def uninstall(options, env):
 # this is optional service, just log
 logging.info(NSCD daemon is not installed, skip configuration)
 
+if ipautil.service_is_installed('nslcd'):
+try:
+ipautil.service_stop('nslcd')
+except:
+print Failed to stop the NSLCD daemon
+
+try:
+ipautil.chkconfig_off('nslcd')
+except:
+print Failed to disable automatic startup of the NSLCD daemon
+else:
+# this is optional service, just log
+logging.info(NSLCD daemon is not installed, skip configuration)
+
 if not options.unattended:
 print The original nsswitch.conf configuration has been restored.
 print You may need to restart services or reboot the machine.
@@ -365,6 +379,20 @@ def configure_nslcd_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server,
 print Creation of %s: %s % ('/etc/nslcd.conf', str(e))
 return 1
 
+if ipautil.service_is_installed('nslcd'):
+try:
+ipautil.service_restart('nslcd')
+except Exception, e:
+logging.error(nslcd failed to restart: %s % str(e))
+
+try:
+ipautil.chkconfig_on('nslcd')
+except Exception, e:
+print Failed to configure automatic startup of the NSLCD daemon
+logging.error(Failed to enable automatic startup of the NSLCD daemon: %s % str(e))
+else:
+logging.debug(NSLCD daemon is not installed, skip configuration)
+
 return 0
 
 def hardcode_ldap_server(cli_server):
@@ -856,12 +884,6 @@ def main():
 if configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options):
 return 1
 print Configured /etc/sssd/sssd.conf
-else:
-if configure_ldap_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options):
-return 1
-if configure_nslcd_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options):
-return 1
-print Configured LDAP
 
 # Add the CA to the default NSS database and trust it
 run([/usr/bin/certutil, -A, -d, /etc/pki/nssdb, -n, IPA CA, -t, CT,C,C, -a, -i, /etc/ipa/ca.crt])
@@ -922,6 +944,19 @@ def main():
 run(cmd)
 print message
 
+#Modify pam to add pam_krb5
+run([/usr/sbin/authconfig, --enablekrb5, --update, --nostart])
+print Kerberos 5 enabled
+
+# Update non-SSSD LDAP configuration after authconfig calls as it would
+# change its configuration otherways
+if not options.sssd:
+if configure_ldap_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options):
+return 1
+if configure_nslcd_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options):
+return 1
+print LDAP configured
+
 #Check that nss is working properly
 if not options.on_master:
 n = 0
@@ -946,10 +981,6 @@ def main():
 except 

[Freeipa-devel] [PATCH] 073 IPA installation with --no-host-dns fails

2011-06-01 Thread Martin Kosek
Patch for both master and ipa-2-0 branch attached.
---
--no-host-dns option should allow installing IPA server on a host
without a DNS resolvable name.

Update parse_ip_address and verify_ip_address functions has been
changed not to return None and print error messages in case of
an error, but rather let the Exception be handled by the calling
routine.

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

From cfcd83e387b750459e6d3df420aad975a94991e0 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Tue, 31 May 2011 12:51:38 +0200
Subject: [PATCH] IPA installation with --no-host-dns fails

--no-host-dns option should allow installing IPA server on a host
without a DNS resolvable name.

Update parse_ip_address and verify_ip_address functions has been
changed not to return None and print error messages in case of
an error, but rather let the Exception be handled by the calling
routine.

https://fedorahosted.org/freeipa/ticket/1246
---
 install/tools/ipa-dns-install |9 -
 install/tools/ipa-replica-prepare |5 -
 install/tools/ipa-server-install  |   21 +++--
 ipaserver/install/installutils.py |   37 +++--
 4 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index 91edcca8a438871cb73a6792105e24040b77953e..39998ac47b02244aaec1d09a14980db25ec636f4 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -108,7 +108,14 @@ def main():
 else:
 hostaddr = resolve_host(api.env.host)
 ip_address = hostaddr and ipautil.CheckedIPAddress(hostaddr)
-if not ip_address or not verify_ip_address(ip_address):
+
+try:
+verify_ip_address(ip_address)
+except Exception, e:
+print Error: Invalid IP Address %s: %s % (ip_address, e)
+ip_address = None
+
+if not ip_address:
 if options.unattended:
 sys.exit(Unable to resolve IP address for host name)
 else:
diff --git a/install/tools/ipa-replica-prepare b/install/tools/ipa-replica-prepare
index 2765e4a0e5635d5400241d83070f58c46a13f840..ac70f7c9b09a0e0655ee2f83413cc74a99e3c8ae 100755
--- a/install/tools/ipa-replica-prepare
+++ b/install/tools/ipa-replica-prepare
@@ -78,11 +78,6 @@ def parse_options():
 if cnt  0 and cnt  num:
 parser.error(All PKCS#12 options are required if any are used.)
 
-if options.ip_address:
-if not installutils.verify_ip_address(options.ip_address, match_local=False):
-parser.error(Bad IP address)
-sys.exit(1)
-
 if len(args) != 1:
 parser.error(must provide the fully-qualified name of the replica)
 
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 23e495e6348f588970e444a95841c73e66f26ee2..70d2d8963fb9cead9474134b1d3dab1c866364b5 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -598,14 +598,20 @@ def main():
 if hostaddr is not None:
 ip = CheckedIPAddress(hostaddr)
 else:
+if not options.ip_address:
+print Unable to resolve IP address for host name
 ip = options.ip_address
 if ip is None and options.unattended:
 sys.exit(Unable to resolve IP address for host name)
 
-if not verify_ip_address(ip):
-ip = None
-if options.unattended:
-sys.exit(1)
+if ip:
+try:
+verify_ip_address(ip)
+except Exception, e:
+print Error: Invalid IP Address %s: %s % (ip, e)
+if options.unattended:
+sys.exit(1)
+ip = None
 
 if options.ip_address:
 if options.ip_address != ip and not options.setup_dns:
@@ -615,8 +621,11 @@ def main():
 return 1
 
 ip = options.ip_address
-if not verify_ip_address(ip):
-return 1
+try:
+verify_ip_address(ip)
+except Exception, e:
+print Error: Invalid IP Address %s: %s % (ip, e)
+sys.exit(1)
 
 if ip is None:
 ip = read_ip_address(host_name, fstore)
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index d99af3742a288c60dbf11530587f3f0eb9540161..d203f4f9392a0738cc9fe2486a9f0ef6b9a64f3c 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -108,6 +108,10 @@ def verify_fqdn(host_name,no_host_dns=False):
 if host_name != host_name.lower():
 raise RuntimeError(Invalid hostname '%s', must be lower-case. % host_name)
 
+if no_host_dns:
+print Warning: skipping DNS resolution of host, host_name
+return
+
 try:
 hostaddr = socket.getaddrinfo(host_name, None)
 except:
@@ -127,10 +131,6 @@ def verify_fqdn(host_name,no_host_dns=False):
 if revname != host_name:
 raise RuntimeError(The host name %s does not match the reverse lookup %s % (host_name, 

[Freeipa-devel] [PATCH] 0231-redirect-on-error

2011-06-01 Thread Adam Young

https://fedorahosted.org/freeipa/ticket/1227
From bb079f53efcab3b8216db0e15f9115726afc2b12 Mon Sep 17 00:00:00 2001
From: Adam Young ayo...@redhat.com
Date: Wed, 1 Jun 2011 09:34:44 -0400
Subject: [PATCH] redirect on error Code for redirecting on error has been
 moved to IPA.facet so it can be called from both details
 and assocaiton facets.

---
 install/ui/associate.js |8 +---
 install/ui/details.js   |   17 +
 install/ui/entity.js|   19 +++
 3 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/install/ui/associate.js b/install/ui/associate.js
index ad2da521687d5e684882aa85eab9b94226d4f7ce..5e27d3bf43c5af0d00ca450a17d0d2414022aba8 100644
--- a/install/ui/associate.js
+++ b/install/ui/associate.js
@@ -1085,12 +1085,6 @@ IPA.association_facet = function (spec) {
 that.refresh_table();
 }
 
-function on_error(xhr, text_status, error_thrown) {
-var summary = $('span[name=summary]', that.table.tfoot).empty();
-summary.append('pError: '+error_thrown.name+'/p');
-summary.append('p'+error_thrown.message+'/p');
-}
-
 var pkey = IPA.get_entity(that.entity_name).get_primary_key();
 
 IPA.command({
@@ -1098,7 +1092,7 @@ IPA.association_facet = function (spec) {
 method: 'show',
 args: pkey,
 on_success: on_success,
-on_error: on_error
+on_error: that.on_error
 }).execute();
 };
 
diff --git a/install/ui/details.js b/install/ui/details.js
index d9c948e978e86b78b890f34c65cac793c58ef264..80e7bd9e6cbd998d8b4bbd89086a308cae5ba0e1 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -640,22 +640,7 @@ IPA.details_facet = function(spec) {
 that.load(data.result.result);
 };
 
-command.on_error = function(xhr, text_status, error_thrown) {
-if (that.entity.redirect_facet) {
-var current_entity = that.entity;
-while (current_entity.containing_entity){
-current_entity = current_entity.containing_entity;
-}
-IPA.nav.show_page(
-current_entity.name,
-that.entity.redirect_facet);
-return;
-}else{
-var details = $('.details', that.container).empty();
-details.append('pError: '+error_thrown.name+'/p');
-details.append('p'+error_thrown.message+'/p');
-}
-};
+command.on_error = that.on_error;
 
 command.execute();
 };
diff --git a/install/ui/entity.js b/install/ui/entity.js
index 4b6ce1d0ebfefacb501210a13942166b62315793..030bcb01697bdf0fcbb1e626695be33d7318dbf1 100644
--- a/install/ui/entity.js
+++ b/install/ui/entity.js
@@ -130,6 +130,25 @@ IPA.facet = function (spec) {
 return $('.content', that.container);
 };
 
+
+that.on_error = function(xhr, text_status, error_thrown) {
+if (that.entity.redirect_facet) {
+var current_entity = that.entity;
+while (current_entity.containing_entity){
+current_entity = current_entity.containing_entity;
+}
+IPA.nav.show_page(
+current_entity.name,
+that.entity.redirect_facet);
+return;
+}else{
+var details = $('.details', that.container).empty();
+details.append('pError: '+error_thrown.name+'/p');
+details.append('p'+error_thrown.message+'/p');
+}
+};
+
+
 // methods that should be invoked by subclasses
 that.facet_init = that.init;
 that.facet_create_header = that.create_header;
-- 
1.7.5.1

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

Re: [Freeipa-devel] [PATCH] 0231-redirect-on-error

2011-06-01 Thread Endi Sukma Dewata

On 6/1/2011 8:36 AM, Adam Young wrote:

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


ACK and pushed to master.

--
Endi S. Dewata

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


Re: [Freeipa-devel] [PATCH] 0229-automount-delete-key

2011-06-01 Thread Adam Young


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


Comments for what was fixed are in the patch.  Here's what I didn't change:


4. Clicking 'Back to List' when viewing a map brings you back to list
   of locations. Is this still intentional? Perhaps the label should be
   changed to 'Back to Locations' or simply hidden.


Left it as is.  if UXD objects, we can change in a follow on patch.



5. The conditional fields in IPA.dialog are a little bit limited
   because there is only one set of conditional fields which has to be
   enabled/disabled together. It might be better to replace the
   'conditional' boolean paramter into 'field_group' then replace the
   enable/disable methods to accept a field group. This could be done
   later.

Agreed.  I'd like to merge this with the sections code used for aci


8. In dialog.js line 626 and search.js line 253, the hasOwnProperty()
   invocations are probably redundant because the key is obtained from
   the object itself, so that method will always return true.
This falls under the rules from Javascript the good parts and is 
probably a good idea to leave in, even though in our code it is strictly 
unnecessary.




10. The 3rd level tab for automount key was removed. At this point does
it makes sense to remove the 3rd level tabs completely?
The 3rd tab will come back if/when we do autmountkey details.  Leaving 
for now.




11. The option values for automount map adder dialog could be
simplified to direct and indirect.



The values used are what is appended to the command's method. Had to 
leave them as is to keep that working.


From f3a313b393a51b2a7e83e35882565ff5601f5ae0 Mon Sep 17 00:00:00 2001
From: Adam Young ayo...@redhat.com
Date: Fri, 27 May 2011 11:32:17 -0400
Subject: [PATCH] automount delete key indirect automount maps

code review changes for automount:

Removed: fields for mount and parentmap in maps details since they are not present in show or mod

Hid undo link for adder dialog

set up click handler for checkboxes when row does not have primary key

removed add override in automountmap_adder_dialog

moved 'var input...' in automount.js  line 158 to start of method.

changed logic in if statmenet ,dialog.js line 628 it if (!first) as suggested
---
 install/ui/add.js   |4 +-
 install/ui/automount.js |   81 +-
 install/ui/dialog.js|   51 -
 install/ui/search.js|   23 +++--
 install/ui/webui.js |3 +-
 install/ui/widget.js|   15 +
 6 files changed, 159 insertions(+), 18 deletions(-)

diff --git a/install/ui/add.js b/install/ui/add.js
index 73a423f00744394241638acceeb0dfa315af40cf..33df62abcaef6e5ec30e397b10d73ea5d8b478ff 100644
--- a/install/ui/add.js
+++ b/install/ui/add.js
@@ -32,7 +32,7 @@ IPA.add_dialog = function (spec) {
 that.name = spec.name;
 that.title = spec.title;
 that._entity_name = spec.entity_name;
-
+that.method = spec.method || 'add';
 that.init = function() {
 
 that.add_button(IPA.messages.buttons.add, function() {
@@ -102,7 +102,7 @@ IPA.add_dialog = function (spec) {
 
 var command = IPA.command({
 entity: that.entity_name,
-method: 'add',
+method: that.method,
 on_success: on_success,
 on_error: on_error
 });
diff --git a/install/ui/automount.js b/install/ui/automount.js
index f865fe73f315c224b53bbe2d74ff2f0109e4d6f2..f98b0b06cf498ac84ded0e3d48673438f1539bf2 100644
--- a/install/ui/automount.js
+++ b/install/ui/automount.js
@@ -63,7 +63,8 @@ IPA.entity_factories.automountmap = function() {
 nested_entity : 'automountkey',
 label : IPA.metadata.objects.automountkey.label,
 name: 'keys',
-columns:['description']
+get_values: IPA.get_option_values,
+columns:['automountkey','automountinformation']
 }).
 details_facet({
 sections:[
@@ -75,7 +76,27 @@ IPA.entity_factories.automountmap = function() {
 ]
 }).
 adder_dialog({
-fields:['automountmapname','description']
+factory: IPA.automountmap_adder_dialog,
+fields:[{factory:IPA.method_radio_widget,
+ name: 'method',
+ undo: false,
+ label:'Map Type',
+ options:[{value:'add',label:'Direct'},
+  {value:'add_indirect',label:'Indirect'}]
+},
+'automountmapname','description',
+{
+name:'key',
+label:'Mount Point',
+conditional:true,
+undo: false
+},
+{
+name:'parentmap',
+label:'Parent Map',
+conditional:true,
+

[Freeipa-devel] [PATCH] 074 Handle LDAP search references

2011-06-01 Thread Martin Kosek
LDAP search operation may return a search reference pointing to
an LDAP resource. As the framework does not handle search
references, skip these results to prevent result processing
failures.

Migrate operation crashed when the migrated DS contained search
references. Now, it correctly skips these records and prints the
failed references to user.

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

From 3310419e3d20c570a0601386038c1bc02e1c230e Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Wed, 1 Jun 2011 18:04:24 +0200
Subject: [PATCH] Handle LDAP search references

LDAP search operation may return a search reference pointing to
an LDAP resource. As the framework does not handle search
references, skip these results to prevent result processing
failures.

Migrate operation crashed when the migrated DS contained search
references. Now, it correctly skips these records and prints the
failed references to user.

https://fedorahosted.org/freeipa/ticket/1209
---
 ipalib/plugins/migration.py |   12 +---
 ipaserver/plugins/ldap2.py  |7 +--
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index ea591d31ee5af2e167866dcc31aa59d0e95caa94..67eaf0e8905f49983651a4738d5090c4e19c4747 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -77,6 +77,7 @@ from ipalib.text import Gettext # FIXME: remove once the other Gettext FIXME is
 
 _krb_err_msg = _('Kerberos principal %s already exists. Use \'ipa user-mod\' to set it manually.')
 _grp_err_msg = _('Failed to add user to the default group. Use \'ipa group-add-member\' to add manually.')
+_ref_err_msg = _('Migration of LDAP search reference is not supported.')
 
 _supported_schemas = (u'RFC2307bis', u'RFC2307')
 
@@ -118,7 +119,7 @@ def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs
 except errors.NotFound:
 entry_attrs['krbprincipalname'] = principal
 else:
-failed[pkey] = _krb_err_msg % principal
+failed[pkey] = unicode(_krb_err_msg % principal)
 
 return dn
 
@@ -128,7 +129,7 @@ def _post_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx):
 try:
 ldap.add_entry_to_group(dn, ctx['def_group_dn'])
 except errors.ExecutionError, e:
-failed[pkey] = _grp_err_msg
+failed[pkey] = unicode(_grp_err_msg)
 
 
 # GROUP MIGRATION CALLBACKS AND VARS
@@ -417,7 +418,8 @@ can use their Kerberos accounts.''')
 (entries, truncated) = ds_ldap.find_entries(
 search_filter, ['*'], search_bases[ldap_obj_name],
 ds_ldap.SCOPE_ONELEVEL,
-time_limit=0, size_limit=-1
+time_limit=0, size_limit=-1,
+search_refs=True# migrated DS may contain search references
 )
 except errors.NotFound:
 if not options.get('continue',False):
@@ -435,6 +437,10 @@ can use their Kerberos accounts.''')
 )
 
 for (dn, entry_attrs) in entries:
+if dn is None:  # LDAP search reference
+failed[ldap_obj_name][entry_attrs[0]] = unicode(_ref_err_msg)
+continue
+
 pkey = entry_attrs[ldap_obj.primary_key.name][0].lower()
 if pkey in exclude:
 continue
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 5556773c95d58b5a891610dc22529e9a981017ea..b0a5c2c2c2eae9aa54a0a65946c442c0b594092b 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -516,7 +516,7 @@ class ldap2(CrudBackend, Encoder):
 @decode_retval()
 def find_entries(self, filter=None, attrs_list=None, base_dn='',
 scope=_ldap.SCOPE_SUBTREE, time_limit=None, size_limit=None,
-normalize=True):
+normalize=True, search_refs=False):
 
 Return a list of entries and indication of whteher the results where
 truncated ([(dn, entry_attrs)], truncated) matching specified search
@@ -530,6 +530,7 @@ class ldap2(CrudBackend, Encoder):
 time_limit -- time limit in seconds (default use IPA config values)
 size_limit -- size (number of entries returned) limit (default use IPA config values)
 normalize -- normalize the DN (default True)
+search_refs -- allow search references to be returned (default skips these entries)
 
 if normalize:
 base_dn = self.normalize_dn(base_dn)
@@ -564,7 +565,9 @@ class ldap2(CrudBackend, Encoder):
 (objtype, res_list) = self.conn.result(id, 0)
 if not res_list:
 break
-res.append(res_list[0])
+if objtype == _ldap.RES_SEARCH_ENTRY or \
+   (search_refs and objtype == _ldap.RES_SEARCH_REFERENCE):
+res.append(res_list[0])
 except 

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

2011-06-01 Thread Rob Crittenden

Martin Kosek wrote:

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



Wow, this is very nice indeed. Ack.

rob

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


Re: [Freeipa-devel] [PATCH] 0229-automount-delete-key

2011-06-01 Thread Adam Young

On 06/01/2011 07:05 PM, Endi Sukma Dewata wrote:

On 6/1/2011 10:09 AM, Adam Young wrote:

11. The option values for automount map adder dialog could be
simplified to direct and indirect.



The values used are what is appended to the command's method. Had to
leave them as is to keep that working.


The option values (direct  indirect) could be translated internally
into method names (add and add_indirect). It wouldn't matter for users,
but it could improve clarity of Selenium tests. This is no big deal,
it can be fixed another time.

One more issue:

12. The map type radio buttons in the automount map adder dialog retain
the previous selection. Try creating an indirect map, then click Add
again, the indirect radio is selected but the mount point and parent
map fields are not shown.

It's ACKed. Issue #12 could be fixed before push.

#12 was fixed and pushed.  I also added sample data for 
automountmap_add_indirect.


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