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

[Goldwyn Rodrigues]

On 02/27/2017 10:24 PM, Seth Arnold wrote:
> 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?

aaui sets it, but I think it is best to reinforce it.

> 
>>  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 い?)

Yes, I will change it to perform as the text mode does.

> 
>> +
>>      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.


There are two ways to solve what I am trying to do:

1. Get the list of all changes in json and ask the user to select which
ones are relevant. Eventually perform a mergeprof. However, this would
not allow features such as globbing etc. Just a Allow/Deny.

2. Keep it interactive as the current text tool is. This would allow all
options currently supported.

As you would have guessed, this one solves the second one, albeit with
an interpreter such as yast.

If you meant the localization, I understand the error and will deal with
it.

-- 
Goldwyn