Hello, Am Donnerstag, 13. April 2017, 05:55:54 CEST schrieb Goldwyn Rodrigues: > From: Goldwyn Rodrigues <[email protected]> > > This adds JSON support for tools in order to be able to talk to > other utilities such as Yast. > > The json is one per line, in order to differentiate between multiple > records. This is based on work presented by Christian Boltz some time > back. > > Signed-off-by: Goldwyn Rodrigues <[email protected]> > --- > Changes since v1: > - implementation of set_json_mode(), write_json() > - Changed the way output is provided to keep input the same. This > would write in either of two formats: text or json, but will keep > input the same. This helps in keeping localizations in place. I so > wish UI was a class.. - Removed all yast calls. > > Changes since v2: > - use if UI_mode == json else, to make sure some output is returned > in case of faulty UI_mode. > - spelling correction yesnocancal > - Added default_key > - added dialog entry in all communication, to identify the kind of > json output.
We just discussed this a bit on IRC. 'dialog' is a very good idea and
can be helpful to ensure we don't break anything.
I'd even say we should never change the parameters of existing 'dialog'
types. If changes are needed, we should create a new 'dialog' type
instead. (I'm speaking about released versions here - changes for
dialogs only in bzr are not that problematic.)
Also, it would be helpful to print out the json scheme version at the
beginning. This can help clients to break early (at startup, if the json
version is too new for them) instead of breaking later when receiving a
new ("unknown") 'dialog' type.
Just as an idea, set_json_mode() could do
jsonout = {'dialog': 'apparmor-json-version', 'data': '2.12'}
write_json(jsonout)
'2.12' obviously matches the next AppArmor release. It will only be
changed if the json interface changes, so if it turns out your patch is
good enough for the next 50 years, it will stay at '2.12' forever ;-)
If you think a simple number is easier to handle - I'm also fine with
version '1' ;-)
IMHO the client (YaST) should error out if the version is newer than
what it supports - that's better than breaking somewhere in the middle
of an aa-logprof run.
BTW: Adding a summary of the above as comment to ui.py would be a good
idea.
You can do this in a follow-up patch - it wouldn't be the first time we
see a 3/2 patch ;-)
(If you prefer to update this patch, please tell me.)
> diff --git a/utils/apparmor/ui.py b/utils/apparmor/ui.py
> index f25fff3..c4462b0 100644
> --- a/utils/apparmor/ui.py
> +++ b/utils/apparmor/ui.py
> @@ -44,12 +54,18 @@ def getkey():
>
> def UI_Info(text):
> debug_logger.info(text)
> - if UI_mode == 'text':
> + if UI_mode == 'json':
> + jsonout = {'dialog': 'info', 'data': text}
> + write_json(jsonout)
> + else: # text mode
Minor nitpicking: python coding styles expect _two_ spaces between code
and comments, so this line should be
+ else: # text mode
(Applies to several comments in your patch.)
> @@ -324,6 +348,17 @@ class PromptQuestion(object):
> function_regexp += ')$'
>
> ans = 'XXXINVALIDXXX'
> + hdict = dict()
> + jsonprompt = {
> + 'dialog': 'promptuser',
> + 'title': title,
> + 'headers': hdict,
> + 'explanation': explanation,
> + 'options': options,
> + 'menu_items': menu_items,
> + 'default_key': default_key,
The last line has a whitespace issue - tab instead of spaces?
BTW: Don't forget to let YaST send the selected option in this dialog,
and also make sure it works if more than 9 options are available.
Hint: It might be a good idea to provide the response as json {'option':
<selected option>, 'menu_item': <pressed button>} in this case.
(Again, this could be a follow-up patch.)
The attached file contains your patch with the whitespace issues fixed
(no other changes).
If you promise to send a follow-up patch that adds the json version info
(and the comments about interface stability), I'm ready to give this
patch (with the whitespace fixes as listed above) my
Acked-by: Christian Boltz <[email protected]>
If you prefer to send a v4 of this patch, tell me ;-)
Note: The patch is based on some code I wrote, so an additional review
(Steve? Seth?) would be nice ;-)
Regards,
Christian Boltz
--
We need to make sure that the direction we're going and the decisions
we're taking are made by people who will suffer the consequences of
them. [Henne Vogelsang in opensuse-project]
[apparmor] [PATCH 2/2] json support for tools (logprof and genprof) From: Goldwyn Rodrigues <[email protected]> This adds JSON support for tools in order to be able to talk to other utilities such as Yast. The json is one per line, in order to differentiate between multiple records. This is based on work presented by Christian Boltz some time back. Signed-off-by: Goldwyn Rodrigues <[email protected]> --- Changes since v1: - implementation of set_json_mode(), write_json() - Changed the way output is provided to keep input the same. This would write in either of two formats: text or json, but will keep input the same. This helps in keeping localizations in place. I so wish UI was a class.. - Removed all yast calls. Changes since v2: - use if UI_mode == json else, to make sure some output is returned in case of faulty UI_mode. - spelling correction yesnocancal - Added default_key - added dialog entry in all communication, to identify the kind of json output. Changes by cboltz: - fix spaces in comments - fix spaces in 'default_key' line utils/aa-genprof | 4 ++ utils/aa-logprof | 5 ++ utils/apparmor/ui.py | 193 +++++++++++++++++++++++++++++++-------------------- 3 files changed, 125 insertions(+), 77 deletions(-) diff --git a/utils/aa-genprof b/utils/aa-genprof index e2e6544..9a8783d 100755 --- a/utils/aa-genprof +++ b/utils/aa-genprof @@ -61,8 +61,12 @@ parser = argparse.ArgumentParser(description=_('Generate profile for the given p parser.add_argument('-d', '--dir', type=str, help=_('path to profiles')) parser.add_argument('-f', '--file', type=str, help=_('path to logfile')) parser.add_argument('program', type=str, help=_('name of program to profile')) +parser.add_argument('-j', '--json', action="store_true", help=_('provide output in json format')) args = parser.parse_args() +if args.json: + aaui.set_json_mode() + profiling = args.program profiledir = args.dir diff --git a/utils/aa-logprof b/utils/aa-logprof index c05cbef..a00cfea 100755 --- a/utils/aa-logprof +++ b/utils/aa-logprof @@ -16,6 +16,7 @@ import argparse import os import apparmor.aa as apparmor +import apparmor.ui as aaui # setup exception handling from apparmor.fail import enable_aa_exception_handler @@ -29,8 +30,12 @@ parser = argparse.ArgumentParser(description=_('Process log entries to generate parser.add_argument('-d', '--dir', type=str, help=_('path to profiles')) parser.add_argument('-f', '--file', type=str, help=_('path to logfile')) parser.add_argument('-m', '--mark', type=str, help=_('mark in the log to start processing after')) +parser.add_argument('-j', '--json', action='store_true', help=_('provide the output in json format')) args = parser.parse_args() +if args.json: + aaui.set_json_mode() + profiledir = args.dir logmark = args.mark or '' diff --git a/utils/apparmor/ui.py b/utils/apparmor/ui.py index f25fff3..c4462b0 100644 --- a/utils/apparmor/ui.py +++ b/utils/apparmor/ui.py @@ -1,5 +1,7 @@ # ---------------------------------------------------------------------- # Copyright (C) 2013 Kshitij Gupta <[email protected]> +# Copyright (C) 2017 Christian Boltz <[email protected]> +# Copyright (C) 2017 SUSE Linux # # This program is free software; you can redistribute it and/or # modify it under the terms of version 2 of the GNU General Public @@ -11,6 +13,8 @@ # GNU General Public License for more details. # # ---------------------------------------------------------------------- + +import json import sys import re import readline @@ -24,14 +28,20 @@ _ = init_translation() # Set up UI logger for separate messages from UI module debug_logger = DebugLogger('UI') -# The operating mode: yast or text, text by default -UI_mode = 'text' - # If Python3, wrap input in raw_input so make check passes if not 'raw_input' in dir(__builtins__): raw_input = input ARROWS = {'A': 'UP', 'B': 'DOWN', 'C': 'RIGHT', 'D': 'LEFT'} +UI_mode = 'text' + +def set_json_mode(): + global UI_mode + UI_mode = 'json' + +def write_json(jsonout): + sys.stdout.write(json.dumps(jsonout, sort_keys=False, separators=(',', ': ')) + '\n') + def getkey(): key = readkey() if key == '\x1B': @@ -44,12 +54,18 @@ def getkey(): def UI_Info(text): debug_logger.info(text) - if UI_mode == 'text': + if UI_mode == 'json': + jsonout = {'dialog': 'info', 'data': text} + write_json(jsonout) + else: # text mode sys.stdout.write(text + '\n') def UI_Important(text): debug_logger.debug(text) - if UI_mode == 'text': + if UI_mode == 'json': + jsonout = {'dialog': 'important', 'data': text} + write_json(jsonout) + else: # text mode: sys.stdout.write('\n' + text + '\n') def get_translated_hotkey(translated, cmsg=''): @@ -67,53 +83,57 @@ def get_translated_hotkey(translated, cmsg=''): def UI_YesNo(text, default): debug_logger.debug('UI_YesNo: %s: %s %s' % (UI_mode, text, default)) default = default.lower() - ans = None - if UI_mode == 'text': - yes = CMDS['CMD_YES'] - no = CMDS['CMD_NO'] - yeskey = get_translated_hotkey(yes).lower() - nokey = get_translated_hotkey(no).lower() - ans = 'XXXINVALIDXXX' - while ans not in ['y', 'n']: + yes = CMDS['CMD_YES'] + no = CMDS['CMD_NO'] + yeskey = get_translated_hotkey(yes).lower() + nokey = get_translated_hotkey(no).lower() + ans = 'XXXINVALIDXXX' + while ans not in ['y', 'n']: + if UI_mode == 'json': + jsonout = {'dialog': 'yesno', 'text': text, 'default': default} + write_json(jsonout) + else: # text mode: sys.stdout.write('\n' + text + '\n') if default == 'y': sys.stdout.write('\n[%s] / %s\n' % (yes, no)) else: sys.stdout.write('\n%s / [%s]\n' % (yes, no)) - ans = getkey() - if ans: - # Get back to english from localised answer - ans = ans.lower() - if ans == yeskey: - ans = 'y' - elif ans == nokey: - ans = 'n' - elif ans == 'left': - default = 'y' - elif ans == 'right': - default = 'n' - else: - ans = 'XXXINVALIDXXX' - continue # If user presses any other button ask again + ans = getkey() + if ans: + # Get back to english from localised answer + ans = ans.lower() + if ans == yeskey: + ans = 'y' + elif ans == nokey: + ans = 'n' + elif ans == 'left': + default = 'y' + elif ans == 'right': + default = 'n' else: - ans = default + ans = 'XXXINVALIDXXX' + continue # If user presses any other button ask again + else: + ans = default return ans def UI_YesNoCancel(text, default): debug_logger.debug('UI_YesNoCancel: %s: %s %s' % (UI_mode, text, default)) default = default.lower() - ans = None - if UI_mode == 'text': - yes = CMDS['CMD_YES'] - no = CMDS['CMD_NO'] - cancel = CMDS['CMD_CANCEL'] - - yeskey = get_translated_hotkey(yes).lower() - nokey = get_translated_hotkey(no).lower() - cancelkey = get_translated_hotkey(cancel).lower() - - ans = 'XXXINVALIDXXX' - while ans not in ['c', 'n', 'y']: + yes = CMDS['CMD_YES'] + no = CMDS['CMD_NO'] + cancel = CMDS['CMD_CANCEL'] + + yeskey = get_translated_hotkey(yes).lower() + nokey = get_translated_hotkey(no).lower() + cancelkey = get_translated_hotkey(cancel).lower() + + ans = 'XXXINVALIDXXX' + while ans not in ['c', 'n', 'y']: + if UI_mode == 'json': + jsonout = {'dialog': 'yesnocancel', 'text': text, 'default': default} + write_json(jsonout) + else: # text mode: sys.stdout.write('\n' + text + '\n') if default == 'y': sys.stdout.write('\n[%s] / %s / %s\n' % (yes, no, cancel)) @@ -121,55 +141,60 @@ def UI_YesNoCancel(text, default): sys.stdout.write('\n%s / [%s] / %s\n' % (yes, no, cancel)) else: sys.stdout.write('\n%s / %s / [%s]\n' % (yes, no, cancel)) - ans = getkey() - if ans: - # Get back to english from localised answer - ans = ans.lower() - if ans == yeskey: - ans = 'y' - elif ans == nokey: - ans = 'n' - elif ans == cancelkey: - ans = 'c' - elif ans == 'left': - if default == 'n': - default = 'y' - elif default == 'c': - default = 'n' - elif ans == 'right': - if default == 'y': - default = 'n' - elif default == 'n': - default = 'c' - else: - ans = default + ans = getkey() + if ans: + # Get back to english from localised answer + ans = ans.lower() + if ans == yeskey: + ans = 'y' + elif ans == nokey: + ans = 'n' + elif ans == cancelkey: + ans = 'c' + elif ans == 'left': + if default == 'n': + default = 'y' + elif default == 'c': + default = 'n' + elif ans == 'right': + if default == 'y': + default = 'n' + elif default == 'n': + default = 'c' + else: + ans = default return ans def UI_GetString(text, default): debug_logger.debug('UI_GetString: %s: %s %s' % (UI_mode, text, default)) string = default - if UI_mode == 'text': + if UI_mode == 'json': + jsonout = {'dialog': 'getstring', 'text': text, 'default': default} + write_json(jsonout) + else: # text mode: readline.set_startup_hook(lambda: readline.insert_text(default)) - try: - string = raw_input('\n' + text) - except EOFError: - string = '' - finally: - readline.set_startup_hook() + try: + string = raw_input('\n' + text) + except EOFError: + string = '' + finally: + readline.set_startup_hook() return string.strip() def UI_GetFile(file): debug_logger.debug('UI_GetFile: %s' % UI_mode) filename = None - if UI_mode == 'text': + if UI_mode == 'json': + jsonout = {'dialog': 'getfile', 'text': file['description']} + write_json(jsonout) + else: # text mode: sys.stdout.write(file['description'] + '\n') - filename = sys.stdin.read() + filename = sys.stdin.read() return filename def UI_BusyStart(message): debug_logger.debug('UI_BusyStart: %s' % UI_mode) - if UI_mode == 'text': - UI_Info(message) + UI_Info(message) def UI_BusyStop(): debug_logger.debug('UI_BusyStop: %s' % UI_mode) @@ -254,8 +279,7 @@ class PromptQuestion(object): def promptUser(self, params=''): cmd = None arg = None - if UI_mode == 'text': - cmd, arg = self.Text_PromptUser() + cmd, arg = self.Text_PromptUser() if cmd == 'CMD_ABORT': confirm_and_abort() cmd = 'XXXINVALIDXXX' @@ -324,6 +348,17 @@ class PromptQuestion(object): function_regexp += ')$' ans = 'XXXINVALIDXXX' + hdict = dict() + jsonprompt = { + 'dialog': 'promptuser', + 'title': title, + 'headers': hdict, + 'explanation': explanation, + 'options': options, + 'menu_items': menu_items, + 'default_key': default_key, + } + while not re.search(function_regexp, ans, flags=re.IGNORECASE): prompt = '\n' @@ -335,6 +370,7 @@ class PromptQuestion(object): while header_copy: header = header_copy.pop(0) value = header_copy.pop(0) + hdict[header] = value prompt += formatstr % (header + ':', value) prompt += '\n' @@ -352,7 +388,10 @@ class PromptQuestion(object): prompt += ' / '.join(menu_items) - sys.stdout.write(prompt + '\n') + if UI_mode == 'json': + write_json(jsonprompt) + else: # text mode: + sys.stdout.write(prompt + '\n') ans = getkey().lower() -- 2.10.2
signature.asc
Description: This is a digitally signed message part.
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
