URL: https://github.com/freeipa/freeipa/pull/278 Author: martbab Title: #278: Restore the original functionality of `env` and `plugins` commands Action: opened
PR body: """ This reverts commit 1166fbc4946596fcc2ed51a1ec6990fc7dae8964 "Add 'ipa localenv' subcommand" and instead fixes the command to be executed locally unless `--server` option is given. This also should make the command usable locally without Kerberos TGT. Note than `plugins` command fails due to https://fedorahosted.org/freeipa/ticket/6513 but now at least it fails either locally or on server-side :). https://fedorahosted.org/freeipa/ticket/6482 https://fedorahosted.org/freeipa/ticket/6490 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/278/head:pr278 git checkout pr278
From f0318899c75e67fc57bcb0b73d3e6daa3d9a2167 Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Mon, 28 Nov 2016 15:10:22 +0100 Subject: [PATCH 1/3] Revert "Add 'ipa localenv' subcommand" This reverts commit 1166fbc4946596fcc2ed51a1ec6990fc7dae8964. The proper fix is to restore pre-thin client behavior of commands inheriting from LocalOrRemote class. https://fedorahosted.org/freeipa/ticket/6490 --- client/man/ipa.1 | 6 +----- ipalib/cli.py | 24 ------------------------ 2 files changed, 1 insertion(+), 29 deletions(-) diff --git a/client/man/ipa.1 b/client/man/ipa.1 index cc5641b..9194ca0 100644 --- a/client/man/ipa.1 +++ b/client/man/ipa.1 @@ -65,17 +65,13 @@ The principal function of the CLI is to execute administrative commands specifie From the implementation perspective, the CLI distinguishes two types of commands \- built\-ins and plugin provided. -Built\-in commands are static and are all available in all installations of IPA. There are three of them: +Built\-in commands are static and are all available in all installations of IPA. There are two of them: .TP \fBconsole\fR Start the IPA interactive Python console. .TP \fBhelp\fR [\fITOPIC\fR | \fICOMMAND\fR | \fBtopics\fR | \fBcommands\fR] Display help for a command or topic. -\fBlocal-env\fR -Show local IPA api.env values. -.TP - The \fBhelp\fR command invokes the built\-in documentation system. Without parameters a list of built\-in commands and help topics is displayed. Help topics are generated from loaded IPA plugin modules. Executing \fBhelp\fR with the name of an available topic displays a help message provided by the corresponding plugin module and list of commands it contains. .LP diff --git a/ipalib/cli.py b/ipalib/cli.py index 696be62..94b4971 100644 --- a/ipalib/cli.py +++ b/ipalib/cli.py @@ -54,7 +54,6 @@ from ipalib.parameters import File, Str, Enum, Any, Flag from ipalib.text import _ from ipalib import api # pylint: disable=unused-import -from ipapython.dn import DN from ipapython.dnsutil import DNSName import datetime @@ -860,28 +859,6 @@ def print_commands(self, topic, outfile): writer(_(' ipa <command> --help')) writer() - -class local_env(frontend.Local): - """Dump local api.env vars - """ - - takes_args = () - takes_options = () - has_output = () - topic = None - - def run(self, *args, **options): - for key in sorted(self.api.env): - value = getattr(api.env, key) - if isinstance(value, DN): - value = unicode(value) - if isinstance(value, (str, unicode)): - # shell escape - value = value.replace(r'"', r'\"') - value = value.replace('$', r'$$') - print('{}="{}"'.format(key, value)) - - class show_mappings(frontend.Command): """ Show mapping of LDAP attributes to command-line option. @@ -1359,7 +1336,6 @@ def format_description(self, description): console, help, show_mappings, - local_env, ) From 0042dd7bc64d975cf297ebbe5b8a590398cea61c Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Mon, 28 Nov 2016 14:50:57 +0100 Subject: [PATCH 2/3] Make `env` and `plugins` commands local again During thin client refactoring, LocalOrRemote class implementation of `run` method was overriden by default Command implementation during instantiation of client plugins from schema. This caused these commands to always forward this request to IPA master. This patch restores the original behavior: unless `--server` option was specified, the commands will always print out local config. https://fedorahosted.org/freeipa/ticket/6490 --- ipaclient/plugins/misc.py | 7 +-- ipalib/misc.py | 124 ++++++++++++++++++++++++++++++++++++++++++++++ ipaserver/plugins/misc.py | 123 ++------------------------------------------- 3 files changed, 133 insertions(+), 121 deletions(-) create mode 100644 ipalib/misc.py diff --git a/ipaclient/plugins/misc.py b/ipaclient/plugins/misc.py index 2c195f8..f4f2f07 100644 --- a/ipaclient/plugins/misc.py +++ b/ipaclient/plugins/misc.py @@ -2,14 +2,15 @@ # Copyright (C) 2016 FreeIPA Contributors see COPYING for license # -from ipaclient.frontend import CommandOverride +from ipalib.misc import env as _env +from ipalib.misc import plugins as _plugins from ipalib.plugable import Registry register = Registry() @register(override=True, no_fail=True) -class env(CommandOverride): +class env(_env): def output_for_cli(self, textui, output, *args, **options): output = dict(output) output.pop('count', None) @@ -20,7 +21,7 @@ def output_for_cli(self, textui, output, *args, **options): @register(override=True, no_fail=True) -class plugins(CommandOverride): +class plugins(_plugins): def output_for_cli(self, textui, output, *args, **options): options['all'] = True return super(plugins, self).output_for_cli(textui, output, diff --git a/ipalib/misc.py b/ipalib/misc.py new file mode 100644 index 0000000..7206d29 --- /dev/null +++ b/ipalib/misc.py @@ -0,0 +1,124 @@ +# +# Copyright (C) 2016 FreeIPA Contributors see COPYING for license +# + +import re +from ipalib import LocalOrRemote, _, ngettext +from ipalib.output import Output, summary +from ipalib import Flag +from ipalib.plugable import Registry + +register = Registry() + +# FIXME: We should not let env return anything in_server +# when mode == 'production'. This would allow an attacker to see the +# configuration of the server, potentially revealing compromising +# information. However, it's damn handy for testing/debugging. + + +class env(LocalOrRemote): + __doc__ = _('Show environment variables.') + + msg_summary = _('%(count)d variables') + + takes_args = ( + 'variables*', + ) + + takes_options = LocalOrRemote.takes_options + ( + Flag('all', + cli_name='all', + doc=_('retrieve and print all attributes from the server. Affects command output.'), + exclude='webui', + flags=['no_option', 'no_output'], + default=True, + ), + ) + + has_output = ( + Output('result', + type=dict, + doc=_('Dictionary mapping variable name to value'), + ), + Output('total', + type=int, + doc=_('Total number of variables env (>= count)'), + flags=['no_display'], + ), + Output('count', + type=int, + doc=_('Number of variables returned (<= total)'), + flags=['no_display'], + ), + summary, + ) + + def __find_keys(self, variables): + keys = set() + for query in variables: + if '*' in query: + pat = re.compile(query.replace('*', '.*') + '$') + for key in self.env: + if pat.match(key): + keys.add(key) + elif query in self.env: + keys.add(query) + return keys + + def execute(self, variables=None, **options): + if variables is None: + keys = self.env + else: + keys = self.__find_keys(variables) + ret = dict( + result=dict( + (key, self.env[key]) for key in keys + ), + count=len(keys), + total=len(self.env), + ) + if len(keys) > 1: + ret['summary'] = self.msg_summary % ret + else: + ret['summary'] = None + return ret + + + +class plugins(LocalOrRemote): + __doc__ = _('Show all loaded plugins.') + + msg_summary = ngettext( + '%(count)d plugin loaded', '%(count)d plugins loaded', 0 + ) + + takes_options = LocalOrRemote.takes_options + ( + Flag('all', + cli_name='all', + doc=_('retrieve and print all attributes from the server. Affects command output.'), + exclude='webui', + flags=['no_option', 'no_output'], + default=True, + ), + ) + + has_output = ( + Output('result', dict, 'Dictionary mapping plugin names to bases'), + Output('count', + type=int, + doc=_('Number of plugins loaded'), + ), + summary, + ) + + def execute(self, **options): + result = {} + for namespace in self.api: + for plugin in self.api[namespace](): + cls = type(plugin) + key = '{}.{}'.format(cls.__module__, cls.__name__) + result.setdefault(key, []).append(namespace) + + return dict( + result=result, + ) diff --git a/ipaserver/plugins/misc.py b/ipaserver/plugins/misc.py index 7618e23..834d166 100644 --- a/ipaserver/plugins/misc.py +++ b/ipaserver/plugins/misc.py @@ -17,129 +17,16 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. -import re -from ipalib import LocalOrRemote, _, ngettext -from ipalib.output import Output, summary -from ipalib import Flag +from ipalib import _ +from ipalib.misc import env, plugins from ipalib.plugable import Registry __doc__ = _(""" Misc plug-ins """) -register = Registry() - -# FIXME: We should not let env return anything in_server -# when mode == 'production'. This would allow an attacker to see the -# configuration of the server, potentially revealing compromising -# information. However, it's damn handy for testing/debugging. - - -@register() -class env(LocalOrRemote): - __doc__ = _('Show environment variables.') - - msg_summary = _('%(count)d variables') - - takes_args = ( - 'variables*', - ) - - takes_options = LocalOrRemote.takes_options + ( - Flag('all', - cli_name='all', - doc=_('retrieve and print all attributes from the server. Affects command output.'), - exclude='webui', - flags=['no_option', 'no_output'], - default=True, - ), - ) - - has_output = ( - Output('result', - type=dict, - doc=_('Dictionary mapping variable name to value'), - ), - Output('total', - type=int, - doc=_('Total number of variables env (>= count)'), - flags=['no_display'], - ), - Output('count', - type=int, - doc=_('Number of variables returned (<= total)'), - flags=['no_display'], - ), - summary, - ) - - def __find_keys(self, variables): - keys = set() - for query in variables: - if '*' in query: - pat = re.compile(query.replace('*', '.*') + '$') - for key in self.env: - if pat.match(key): - keys.add(key) - elif query in self.env: - keys.add(query) - return keys - def execute(self, variables=None, **options): - if variables is None: - keys = self.env - else: - keys = self.__find_keys(variables) - ret = dict( - result=dict( - (key, self.env[key]) for key in keys - ), - count=len(keys), - total=len(self.env), - ) - if len(keys) > 1: - ret['summary'] = self.msg_summary % ret - else: - ret['summary'] = None - return ret - - - -@register() -class plugins(LocalOrRemote): - __doc__ = _('Show all loaded plugins.') - - msg_summary = ngettext( - '%(count)d plugin loaded', '%(count)d plugins loaded', 0 - ) - - takes_options = LocalOrRemote.takes_options + ( - Flag('all', - cli_name='all', - doc=_('retrieve and print all attributes from the server. Affects command output.'), - exclude='webui', - flags=['no_option', 'no_output'], - default=True, - ), - ) - - has_output = ( - Output('result', dict, 'Dictionary mapping plugin names to bases'), - Output('count', - type=int, - doc=_('Number of plugins loaded'), - ), - summary, - ) - - def execute(self, **options): - result = {} - for namespace in self.api: - for plugin in self.api[namespace](): - cls = type(plugin) - key = '{}.{}'.format(cls.__module__, cls.__name__) - result.setdefault(key, []).append(namespace) +register = Registry() - return dict( - result=result, - ) +env = register()(env) +plugins = register()(plugins) From 4eeb3fb3430c4e1131bdec02d0246d9162d6ea1a Mon Sep 17 00:00:00 2001 From: Martin Babinsky <mbabi...@redhat.com> Date: Mon, 28 Nov 2016 15:22:41 +0100 Subject: [PATCH 3/3] Fix pep-8 transgressions in ipalib/misc.py Make the code moved from `ipaserver/plugins` pep-8 conformant. https://fedorahosted.org/freeipa/ticket/6490 --- ipalib/misc.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/ipalib/misc.py b/ipalib/misc.py index 7206d29..bfc06ae 100644 --- a/ipalib/misc.py +++ b/ipalib/misc.py @@ -26,9 +26,11 @@ class env(LocalOrRemote): ) takes_options = LocalOrRemote.takes_options + ( - Flag('all', + Flag( + 'all', cli_name='all', - doc=_('retrieve and print all attributes from the server. Affects command output.'), + doc=_('retrieve and print all attributes from the server. ' + 'Affects command output.'), exclude='webui', flags=['no_option', 'no_output'], default=True, @@ -36,16 +38,19 @@ class env(LocalOrRemote): ) has_output = ( - Output('result', + Output( + 'result', type=dict, doc=_('Dictionary mapping variable name to value'), ), - Output('total', + Output( + 'total', type=int, doc=_('Total number of variables env (>= count)'), flags=['no_display'], ), - Output('count', + Output( + 'count', type=int, doc=_('Number of variables returned (<= total)'), flags=['no_display'], @@ -93,9 +98,11 @@ class plugins(LocalOrRemote): ) takes_options = LocalOrRemote.takes_options + ( - Flag('all', + Flag( + 'all', cli_name='all', - doc=_('retrieve and print all attributes from the server. Affects command output.'), + doc=_('retrieve and print all attributes from the server. ' + 'Affects command output.'), exclude='webui', flags=['no_option', 'no_output'], default=True, @@ -104,7 +111,8 @@ class plugins(LocalOrRemote): has_output = ( Output('result', dict, 'Dictionary mapping plugin names to bases'), - Output('count', + Output( + 'count', type=int, doc=_('Number of plugins loaded'), ),
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
