[apparmor] [PATCH] json support for tools (logprof and genprof)

[Seth Arnold]

On Mon, Feb 27, 2017 at 08:39:39PM -0600, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn at suse.com>
> 
> 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 <rgoldwyn at suse.com>

Hi Goldwyn, thanks for the contribution, I have some comments inline:

> --- 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.UI_mode = 'json'
> +

This block is missing the 'else .. text' that is repeated elsewhere, is
this an oversight?

>  profiling = args.program
>  profiledir = args.dir
>  

> +    elif UI_mode == 'json':
> +        jsonout = {'info': text}
> +        sys.stdout.write(json.dumps(jsonout, sort_keys=False, separators=(',', ': ')) + '\n')

I strongly recommend making this a function. Not only is this repeated
often, but I'm pretty sure that Python won't use line-buffered output
in the common case, so each of these should probably have an explicit
sys.stdout.flush() after each one to ensure that Python sends the block
to YaST.

> +        #yastLog(text)

.. and commenting out code is one of my pet peeves :) please just delete
it instead.

> @@ -160,14 +166,13 @@ def UI_YesNoCancel(text, default):
>                          default = 'c'
>              else:
>                  ans = default
> -    else:
> -        SendDataToYast({'type': 'dialog-yesnocancel',
> -                        'question': text
> -                        })
> -        ypath, yarg = GetDataFromYast()
> -        ans = yarg['answer']
> -        if not ans:
> -            ans = default
> +    elif UI_mode == 'json':
> +        jsonout = {'dialog': 'yesnocancal', 'text': text, 'default': default}

Could you double-check the spelling on this 'yesnocancal' token? (/me
inserts usual grumblings about hash-based programming preventing static
analysis.)

> +        sys.stdout.write(json.dumps(jsonout, sort_keys=False,  separators=(',', ': ')) + '\n')
> +        ans = 'XXXINVALIDXXX'
> +        while ans not in ['y', 'n', 'c']:
> +            ans = getkey()

Is getkey() the right thing to call after sending json down the pipe? Will
de_DE.UTF-8 return 'j' or 'y'? How about ja_JA? (は vs い?)

> +
>      return ans
>  
>  def UI_GetString(text, default):
> @@ -181,44 +186,34 @@ def UI_GetString(text, default):
>              string = ''
>          finally:
>              readline.set_startup_hook()
> -    else:
> -        SendDataToYast({'type': 'dialog-getstring',
> -                        'label': text,
> -                        'default': default
> -                        })
> -        ypath, yarg = GetDataFromYast()
> -        string = yarg['string']
> +    elif UI_mode == 'json':
> +        jsonout = {'dialog': 'getstring', 'text': text, 'default': default}
> +        sys.stdout.write(json.dumps(jsonout, sort_keys=False,  separators=(',', ': ')) + '\n')
> +
> +        string = raw_input('\n' + text)
> +
>      return string.strip()
>  
>  def UI_GetFile(file):
> -    debug_logger.debug('UI_GetFile: %s' % UI_mode)
> +    debug_logger.debug('UI_Mode: %s' % UI_mode)
>      filename = None
>      if UI_mode == 'text':
>          sys.stdout.write(file['description'] + '\n')
>          filename = sys.stdin.read()
> -    else:
> -        file['type'] = 'dialog-getfile'
> -        SendDataToYast(file)
> -        ypath, yarg = GetDataFromYast()
> -        if yarg['answer'] == 'okay':
> -            filename = yarg['filename']
> +    elif UI_mode == 'json':
> +        jsonout = {'dialog': 'getfile', 'text': file['description']}
> +        sys.stdout.write(json.dumps(jsonout, sort_keys=False,  separators=(',', ': ')) + '\n')
> +        filename = raw_input('\n')

Is raw_input() the right function to run via the json ui?

> +    #if UI_mode == 'text':
> +    UI_Info(message)

Another instance of commented-out-code, please remove.

>  
>  def UI_BusyStop():
>      debug_logger.debug('UI_BusyStop: %s' % UI_mode)
> -    if UI_mode != 'text':
> -        SendDataToYast({'type': 'dialog-busy-stop'})
> -        ypath, yarg = GetDataFromYast()
>  
>  CMDS = {'CMD_ALLOW': _('(A)llow'),
>          'CMD_OTHER': _('(M)ore'),
> @@ -300,7 +295,7 @@ class PromptQuestion(object):
>      def promptUser(self, params=''):
>          cmd = None
>          arg = None
> -        if UI_mode == 'text':
> +        if UI_mode == 'text' or UI_mode == 'json':
>              cmd, arg = self.Text_PromptUser()
>          else:
>              self.type = 'wizard'
> @@ -377,6 +372,15 @@ class PromptQuestion(object):
>          function_regexp += ')$'
>  
>          ans = 'XXXINVALIDXXX'
> +        hdict = dict()
> +        jsonprompt = {
> +            'title': title,
> +            'headers': hdict,
> +            'explanation': explanation,
> +            'options': options,
> +            'menu_items': menu_items,
> +        }
> +
>          while not re.search(function_regexp, ans, flags=re.IGNORECASE):
>  
>              prompt = '\n'
> @@ -388,6 +392,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'
>  
> @@ -405,7 +410,10 @@ class PromptQuestion(object):
>  
>              prompt += ' / '.join(menu_items)
>  
> -            sys.stdout.write(prompt + '\n')
> +            if UI_mode == 'json':
> +                sys.stdout.write(json.dumps(jsonprompt, sort_keys=False,  separators=(',', ': ')) + '\n')
> +            elif UI_mode == 'text':
> +                sys.stdout.write(prompt + '\n')
>  
>              ans = getkey().lower()

And, again, a question if getkey() is the right UI mechanism for the json
interface.

Thanks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170227/28672ac9/attachment-0001.pgp>