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 <mko...@redhat.com> 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 (default %s): ' % (label, default_prompt) + else: + prompt = u'%s Yes/No: ' % label + + while True: + 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 +1073,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.__class__.EXC_CALLBACKS = [] + if not hasattr(self.__class__, 'INTERACTIVE_PROMPT_CALLBACKS'): + self.__class__.INTERACTIVE_PROMPT_CALLBACKS = [] if hasattr(self, 'pre_callback'): self.register_pre_callback(self.pre_callback, True) if hasattr(self, 'post_callback'): self.register_post_callback(self.post_callback, True) if hasattr(self, 'exc_callback'): self.register_exc_callback(self.exc_callback, True) + if hasattr(self, 'interactive_prompt_callback'): + self.register_interactive_prompt_callback( + self.interactive_prompt_callback, True) #pylint: disable=E1101 super(Method, self).__init__() @classmethod @@ -488,6 +493,16 @@ class CallbackInterface(Method): else: klass.EXC_CALLBACKS.append(callback) + @classmethod + def register_interactive_prompt_callback(klass, callback, first=False): + assert callable(callback) + if not hasattr(klass, 'INTERACTIVE_PROMPT_CALLBACKS'): + klass.INTERACTIVE_PROMPT_CALLBACKS = [] + if first: + klass.INTERACTIVE_PROMPT_CALLBACKS.insert(0, callback) + else: + klass.INTERACTIVE_PROMPT_CALLBACKS.append(callback) + def _call_exc_callbacks(self, args, options, exc, call_func, *call_args, **call_kwargs): rv = None for i in xrange(len(getattr(self, 'EXC_CALLBACKS', []))): @@ -635,6 +650,9 @@ class LDAPCreate(CallbackInterface, crud.Create): def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs): raise exc + def interactive_prompt_callback(self, kw): + return + # list of attributes we want exported to JSON json_friendly_attributes = ( 'takes_args', 'takes_options', @@ -760,6 +778,9 @@ class LDAPRetrieve(LDAPQuery): def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs): raise exc + def interactive_prompt_callback(self, kw): + return + class LDAPUpdate(LDAPQuery, crud.Update): """ @@ -921,6 +942,9 @@ class LDAPUpdate(LDAPQuery, crud.Update): def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs): raise exc + def interactive_prompt_callback(self, kw): + return + class LDAPDelete(LDAPMultiQuery): """ @@ -1008,6 +1032,9 @@ class LDAPDelete(LDAPMultiQuery): def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs): raise exc + def interactive_prompt_callback(self, kw): + return + class LDAPModMember(LDAPQuery): """ @@ -1153,6 +1180,9 @@ class LDAPAddMember(LDAPModMember): def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs): raise exc + def interactive_prompt_callback(self, kw): + return + class LDAPRemoveMember(LDAPModMember): """ @@ -1259,6 +1289,9 @@ class LDAPRemoveMember(LDAPModMember): def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs): raise exc + def interactive_prompt_callback(self, kw): + return + class LDAPSearch(CallbackInterface, crud.Search): """ @@ -1463,6 +1496,9 @@ class LDAPSearch(CallbackInterface, crud.Search): def exc_callback(self, args, options, exc, call_func, *call_args, **call_kwargs): raise exc + def interactive_prompt_callback(self, kw): + return + # list of attributes we want exported to JSON json_friendly_attributes = ( 'takes_options', @@ -1606,6 +1642,9 @@ class LDAPAddReverseMember(LDAPModReverseMember): def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs): raise exc + def interactive_prompt_callback(self, kw): + return + class LDAPRemoveReverseMember(LDAPModReverseMember): """ Remove other LDAP entries from members in reverse. @@ -1715,3 +1754,6 @@ class LDAPRemoveReverseMember(LDAPModReverseMember): def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs): raise exc + + def interactive_prompt_callback(self, kw): + return diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index 3f8753d11a4b65f6e529a9a8a0c85ac39a811fa2..42ca498c9920b6b66b78083e6eca9f58921ae228 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -1,5 +1,6 @@ # Authors: # Pavel Zuna <pz...@redhat.com> +# Martin Kosek <mko...@redhat.com> # # Copyright (C) 2010 Red Hat # see file 'COPYING' for use and warranty information @@ -49,6 +50,28 @@ EXAMPLES: ipa dnsrecord-add example.com _ldap._tcp --srv-rec="0 1 389 slow.example.com" ipa dnsrecord-add example.com _ldap._tcp --srv-rec="1 1 389 backup.example.com" + When dnsrecord-add command is executed with no option to add a specific record + an interactive mode is started. The mode interactively prompts for the most + typical record types for the respective zone: + ipa dnsrecord-add example.com www + [A record]: 1.2.3.4,11.22.33.44 (2 interactively entered random IPs) + [AAAA record]: (no AAAA address entered) + Record name: www + A record: 1.2.3.4, 11.22.33.44 + + The interactive mode can also be used for deleting the DNS records: + ipa dnsrecord-del example.com www + No option to delete specific record provided. + Delete all? Yes/No (default No): (do not delete all records) + Current DNS record contents: + + A record: 1.2.3.4, 11.22.33.44 + + Delete A record '1.2.3.4'? Yes/No (default No): + Delete A record '11.22.33.44'? Yes/No (default No): y + Record name: www + A record: 1.2.3.4 (A record 11.22.33.44 has been deleted) + Show zone example.com: ipa dnszone-show example.com @@ -71,7 +94,6 @@ EXAMPLES: if one is not included): ipa dns-resolve www.example.com ipa dns-resolve www - """ import netaddr @@ -93,6 +115,14 @@ _record_types = ( u'TSIG', u'TXT', ) +# DNS zone record identificator +_dns_zone_record = u'@' + +# most used record types, always ask for those in interactive prompt +_top_record_types = ('A', 'AAAA', ) +_rev_top_record_types = ('PTR', ) +_zone_top_record_types = ('NS', 'MX', 'LOC', ) + # attributes derived from record types _record_attributes = [str('%srecord' % t.lower()) for t in _record_types] @@ -195,6 +225,14 @@ _valid_reverse_zones = { '.ip6.arpa.' : 32, } +def zone_is_reverse(zone_name): + for rev_zone_name in _valid_reverse_zones.keys(): + if zone_name.endswith(rev_zone_name): + return True + + return False + + def has_cli_options(entry, no_option_msg): entry = dict((t, entry.get(t, [])) for t in _record_attributes) numattr = reduce(lambda x,y: x+y, @@ -505,7 +543,7 @@ class dnsrecord(LDAPObject): def is_pkey_zone_record(self, *keys): idnsname = keys[-1] - if idnsname == '@' or idnsname == ('%s.' % keys[-2]): + if idnsname == str(_dns_zone_record) or idnsname == ('%s.' % keys[-2]): return True return False @@ -533,25 +571,40 @@ class dnsrecord_cmd_w_record_options(Command): def get_record_options(self): for t in _record_types: t = t.encode('utf-8') - doc = self.record_param_doc % t - validator = _record_validators.get(t) - if validator: - yield List( - '%srecord?' % t.lower(), validator, - cli_name='%s_rec' % t.lower(), doc=doc, - label='%s record' % t, attribute=True - ) - else: - yield List( - '%srecord?' % t.lower(), cli_name='%s_rec' % t.lower(), - doc=doc, label='%s record' % t, attribute=True - ) + yield self.get_record_option(t) def record_options_2_entry(self, **options): entries = dict((t, options.get(t, [])) for t in _record_attributes) entries.update(dict((k, []) for (k,v) in entries.iteritems() if v == None )) return entries + def get_record_option(self, rec_type): + doc = self.record_param_doc % rec_type + validator = _record_validators.get(rec_type) + if validator: + return List( + '%srecord?' % rec_type.lower(), validator, + cli_name='%s_rec' % rec_type.lower(), doc=doc, + label='%s record' % rec_type, attribute=True + ) + else: + return List( + '%srecord?' % rec_type.lower(), cli_name='%s_rec' % rec_type.lower(), + doc=doc, label='%s record' % rec_type, attribute=True + ) + + def prompt_record_options(self, rec_type_list): + user_options = {} + # ask for all usual record types + for rec_type in rec_type_list: + rec_option = self.get_record_option(rec_type) + raw = self.Backend.textui.prompt(rec_option.label,optional=True) + rec_value = rec_option(raw) + if rec_value is not None: + user_options[rec_option.name] = rec_value + + return user_options + class dnsrecord_mod_record(LDAPQuery, dnsrecord_cmd_w_record_options): """ @@ -599,7 +652,7 @@ class dnsrecord_mod_record(LDAPQuery, dnsrecord_cmd_w_record_options): self.obj.handle_not_found(*keys) if self.obj.is_pkey_zone_record(*keys): - entry_attrs[self.obj.primary_key.name] = [u'@'] + entry_attrs[self.obj.primary_key.name] = [_dns_zone_record] retval = self.post_callback(keys, entry_attrs) if retval: @@ -637,7 +690,8 @@ class dnsrecord_add(LDAPCreate, dnsrecord_cmd_w_record_options): """ Add new DNS resource record. """ - no_option_msg = 'No options to add a specific record provided.' + no_option_msg = 'No options to add a specific record provided.\n' \ + "Command help may be consulted for all supported record types." takes_options = LDAPCreate.takes_options + ( Flag('force', label=_('Force'), @@ -693,6 +747,24 @@ class dnsrecord_add(LDAPCreate, dnsrecord_cmd_w_record_options): return dn + def interactive_prompt_callback(self, kw): + for param in kw.keys(): + if param in _record_attributes: + # some record type entered, skip this helper + return + + # check zone type + if kw['idnsname'] == _dns_zone_record: + top_record_types = _zone_top_record_types + elif zone_is_reverse(kw['dnszoneidnsname']): + top_record_types = _rev_top_record_types + else: + top_record_types = _top_record_types + + # ask for all usual record types + user_options = self.prompt_record_options(top_record_types) + kw.update(user_options) + def pre_callback(self, ldap, dn, entry_attrs, *keys, **options): for rtype in options: rtype_cb = '_%s_pre_callback' % rtype @@ -727,7 +799,8 @@ class dnsrecord_del(dnsrecord_mod_record): """ Delete DNS resource record. """ - no_option_msg = _('Neither --del-all nor options to delete a specific record provided.') + no_option_msg = _('Neither --del-all nor options to delete a specific record provided.\n'\ + "Command help may be consulted for all supported record types.") takes_options = ( Flag('del_all', default=False, @@ -745,6 +818,52 @@ class dnsrecord_del(dnsrecord_mod_record): entry = super(dnsrecord_del, self).record_options_2_entry(**options) return has_cli_options(entry, self.no_option_msg) + def interactive_prompt_callback(self, kw): + if kw.get('del_all', False): + return + for param in kw.keys(): + if param in _record_attributes: + # we have something to delete, skip this helper + return + + # get DNS record first so that the NotFound exception is raised + # before the helper would start + dns_record = api.Command['dnsrecord_show'](kw['dnszoneidnsname'], kw['idnsname'])['result'] + rec_types = [rec_type for rec_type in dns_record if rec_type in _record_attributes] + + self.Backend.textui.print_plain(_("No option to delete specific record provided.")) + user_del_all = self.Backend.textui.prompt_yesno(_("Delete all?"), default=False) + + if user_del_all is True: + kw['del_all'] = True + return + + # ask user for records to be removed + dns_record = api.Command['dnsrecord_show'](kw['dnszoneidnsname'], kw['idnsname'])['result'] + rec_types = [rec_type for rec_type in dns_record if rec_type in _record_attributes] + + self.Backend.textui.print_plain(_(u'Current DNS record contents:\n')) + present_params = [] + for param in self.params(): + if param.name in _record_attributes and param.name in dns_record: + present_params.append(param) + rec_type_content = u', '.join(dns_record[param.name]) + self.Backend.textui.print_plain(u'%s: %s' % (param.label, rec_type_content)) + self.Backend.textui.print_plain(u'') + + # ask what records to remove + for param in present_params: + deleted_values = [] + for rec_value in dns_record[param.name]: + user_del_value = self.Backend.textui.prompt_yesno( + _(u"Delete %s '%s'?" + % (param.label, rec_value)), default=False) + if user_del_value is True: + deleted_values.append(rec_value) + if deleted_values: + deleted_list = u','.join(deleted_values) + kw[param.name] = param(deleted_list) + def update_old_entry_callback(self, entry_attrs, old_entry_attrs): for (a, v) in entry_attrs.iteritems(): if not isinstance(v, (list, tuple)): @@ -776,7 +895,7 @@ class dnsrecord_show(LDAPRetrieve, dnsrecord_cmd_w_record_options): def post_callback(self, ldap, dn, entry_attrs, *keys, **options): if self.obj.is_pkey_zone_record(*keys): - entry_attrs[self.obj.primary_key.name] = [u'@'] + entry_attrs[self.obj.primary_key.name] = [_dns_zone_record] return dn api.register(dnsrecord_show) @@ -805,7 +924,7 @@ class dnsrecord_find(LDAPSearch, dnsrecord_cmd_w_record_options): zone_obj = self.api.Object[self.obj.parent_object] zone_dn = zone_obj.get_dn(args[0]) if entries[0][0] == zone_dn: - entries[0][1][zone_obj.primary_key.name] = [u'@'] + entries[0][1][zone_obj.primary_key.name] = [_dns_zone_record] api.register(dnsrecord_find) -- 1.7.5.1
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
