[apparmor] [utils] proposed redesign for mergeprof
Kshitij Gupta
kgupta8592 at gmail.com
Thu Oct 16 12:48:41 UTC 2014
Hello,
On Thu, Oct 16, 2014 at 5:23 PM, Christian Boltz <apparmor at cboltz.de> wrote:
> Hello,
>
> Am Donnerstag, 16. Oktober 2014 schrieb Kshitij Gupta:
>> Sorry, I was away for a bit and missed the meeting. :( I've read up
>> the logs from the meeting. I hope we can and will try to get
>> aa-mergeprof ready for 2.9 release.
>
> I think so :-) (at least the 2-way-merge)
>
>> Thanks for skimming through the v1 of patch and the fixes. :-)
>>
>> Thus is bring the tool to the decided new decided syntax?
>
Hmm I need to better proof-read my mails. (that makes no sense) :(
> Yes, it's now just
> aa-mergeprof [-d /path/to/profiles] file [file ...]
>
>> I had made some changes post the old patch to allow decided format,
>> the entire --oldupstream --newupstream flags with some additional
>> checks. Unfortunately I couldn't get it to a good shape (or find time
>> to test enough) to send the new revision.
>>
>> I've attached a diff that applies those minor fixes on your changes.
>
> The traditional diff format is quite hard to read. Can you please resend
> the patch in unified (diff -u) format?.
>
> If I understand your patch right, it adds the --oldupstream and --
> newupstream parameters, but doesn't really implement the handling of
> them. Right?
>
They are handled actually. This patch just adds the params and gets
the files from the two directories and then passes then repeatedly to
the Merge method. It seems to work.
Here's the patch with the diff -u
--- aa-mergeprof 2014-10-16 15:03:44.005851363 +0530
+++ ./../../../mergeprof/original/apparmor/utils/aa-mergeprof
2014-10-16 15:45:03.000778117 +0530
@@ -15,6 +15,7 @@
import argparse
import re
import os
+import sys
import apparmor.aa
import apparmor.aamode
@@ -27,17 +28,51 @@
_ = init_translation()
parser = argparse.ArgumentParser(description=_('Merge the given
profiles into /etc/apparmor.d/ (or the directory specified with -d)'))
-parser.add_argument('files', nargs='+', type=str, help=_('base profile'))
-#parser.add_argument('other', nargs='?', type=str, help=_('other profile'))
+parser.add_argument('files', nargs='*', type=str, help=_('base profile'))
+
parser.add_argument('-d', '--dir', type=str, help=_('path to profiles'))
#parser.add_argument('-a', '--auto', action='store_true',
help=_('Automatically merge profiles, exits incase of *x conflicts'))
+# 3-way merge arguments
+parser.add_argument('--oldupstream', nargs='?', help=_('old upstream
profiles'))
+parser.add_argument('--newupstream', nargs='?', help=_('other profiles'))
+
args = parser.parse_args()
-args.other = None
+def get_dir_files(path):
+ files = list()
+ if not os.path.exists(path):
+ raise apparmor.AppArmorException(_("Path to profiles is not
valid: %(dir)") % { 'dir': path })
+
+ for item in os.listdir(path):
+ item_path = path + "/" + item
+ if os.path.isfile(item_path):
+ files.append(item_path)
+
+ return files
+
+newupstream_files = None
+oldupstream_files = None
+
+if args.newupstream or args.oldupstream:
+ if args.files or not (args.newupstream and args.oldupstream):
+ parser.print_help()
+ sys.exit()
+
+ newupstream_files = get_dir_files(args.newupstream)
+ oldupstream_files = get_dir_files(args.oldupstream)
+elif not args.files:
+ parser.print_help()
+ sys.exit()
+
# 2-way merge or 3-way merge based on number of params
-merge_mode = 2 #if args.other == None else 3
+merge_mode = 2 if args.newupstream == None else 3
-profiles = [args.files, [args.other]]
+profiles = None
+
+if merge_mode == 2:
+ profiles = [args.files, None]
+else:
+ profiles = [oldupstream_files, newupstream_files]
profiledir = args.dir
if profiledir:
@@ -86,7 +121,7 @@
if merge_mode == 3:
other_profile_to_file = find_profiles_from_files(other_files)
- profiles_to_merge.add(other_profile_to_file.keys())
+ profiles_to_merge =
profiles_to_merge.union(set(other_profile_to_file.keys()))
user_profile_to_file = find_files_from_profiles(profiles_to_merge)
@@ -117,9 +152,9 @@
reset_aa()
def act(files, merge_mode, merging_profile):
- mergeprofiles = Merge(files)
+ mergeprofiles = Merge(files, merge_mode)
#Get rid of common/superfluous stuff
-# mergeprofiles.clear_common()
+ mergeprofiles.clear_common()
# temporarily disabled because...
# Traceback (most recent call last):
@@ -150,8 +185,8 @@
mergeprofiles.ask_the_questions('base', merging_profile)
q = aaui.PromptQuestion()
- q.title = _('Changed Local Profiles')
- q.explanation = _('The following local profiles were changed.
Would you like to save them?')
+ q.title = _('Changes for Local Profile: %s') % (merging_profile)
+ q.explanation = _('Would you like to save them?')
q.functions = ['CMD_SAVE_CHANGES', 'CMD_VIEW_CHANGES',
'CMD_ABORT', 'CMD_IGNORE_ENTRY']
q.default = 'CMD_VIEW_CHANGES'
q.options = []
@@ -177,8 +212,9 @@
class Merge(object):
- def __init__(self, profiles):
+ def __init__(self, profiles, merge_mode):
user, base, other = profiles
+ self.merge_mode = merge_mode
#Read and parse base profile and save profile data, include
data from it and reset them
apparmor.aa.read_profile(base, True)
@@ -187,7 +223,7 @@
reset_aa()
#Read and parse other profile and save profile data, include
data from it and reset them
- if merge_mode == 3:
+ if self.merge_mode == 3:
apparmor.aa.read_profile(other, True)
self.other = cleanprofile.Prof(other)
reset_aa()
@@ -199,7 +235,7 @@
def clear_common(self):
deleted = 0
- if merge_mode == 3:
+ if self.merge_mode == 3:
#Remove off the parts in other profile which are
common/superfluous from user profile
user_other = cleanprofile.CleanProf(False, self.user, self.other)
deleted += user_other.compare_profiles()
@@ -208,7 +244,7 @@
user_base = cleanprofile.CleanProf(False, self.user, self.base)
deleted += user_base.compare_profiles()
- if merge_mode == 3:
+ if self.merge_mode == 3:
#Remove off the parts in other profile which are
common/superfluous from base profile
base_other = cleanprofile.CleanProf(False, self.base, self.other)
deleted += base_other.compare_profiles()
@@ -263,7 +299,7 @@
for inc in other.filelist[other.filename]['include'].keys():
if not inc in
self.user.filelist[self.user.filename]['include'].keys():
options.append('#include <%s>' %inc)
-
+
default_option = 1
q = aaui.PromptQuestion()
@@ -298,7 +334,7 @@
for inc in other.aa[profile][hat]['include'].keys():
if not inc in self.user.aa[profile][hat]['include'].keys():
options.append('#include <%s>' %inc)
-
+
default_option = 1
q = aaui.PromptQuestion()
Regards,
Kshitij Gupta
>
> For this part:
>
> 153,154c188,189
> < q.title = _('Changed Local Profiles')
> < q.explanation = _('The following local profiles were changed.
> Would you like to save them?')
> ---
>
>> q.title = _('Changes for Local Profile: %s') %
> (merging_profile)
>> q.explanation = _('Would you like to save them?')
>
> Displaying the profile name is a good idea, but I'd do it with q.options
> to keep it in sync with aa-logprof. (I hope we can use ask_the_question
> from aa.py also for aa-mergeprof one day ;-)
>
>> If they seem alright you might want to apply this diff and then make
>> any further changes.
>>
>> Note: Its a small diff with basically changes for the 3-way merge, you
>> may want to revise the description text for the tool
>
> We can add 3-way merge later (when it's ready) - nobody will complain
> about added commandline options or added features ;-)
>
>
> Regards,
>
> Christian Boltz
> --
> 116: Programm
> Sobald eine Datei von einem Virus infiziert werden kann, ist
> es ein Programm. (Markus Kuhn)
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
More information about the AppArmor
mailing list