Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dirs,interfaces/apparmor: remove unused apparmor cache entries #770

Merged
merged 6 commits into from Apr 1, 2016

Conversation

zyga
Copy link
Collaborator

@zyga zyga commented Mar 31, 2016

This branch ensures we remove entries from /var/cache/apparmor corresponding to removed apparmor profiles. We have to do this because apparmor_parser writes the cache but never removes it.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
This patch just makes access to apparmor cache directory follow standard
directory variables so that upcoming support to apparmor cache removal
can be easily tested.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
This patch ensures that we remove the apparmor cache file when removing
the profile text. The cache is written by apparmor_parser but is never
removed.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
@@ -70,6 +73,22 @@ apparmor_parser output:
"--replace --write-cache -O no-expr-simplify --cache-loc=/var/cache/apparmor /path/to/snap.samba.smbd"})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This context for the diff shows a hard-coded --cache-loc. Please adjust it and any other uses of --cache-loc. With those changes, LGTM.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is okay (in this case) because we don't divert the root directory. I can add the diversion to ensure we are indeed using the variable name.

@niemeyer
Copy link
Contributor

LGTM, but it feels like the logic might be simplified a bit in some cases, per inline comments.

zyga added 2 commits April 1, 2016 13:02
This patch merges the two function so that LoadProfile is completely
undone by UnloadProfile. In addition, it is no longer an error for
UnloadProfile to not remove a missing cache entry (for whatever reason,
the cache might be missing and this should not be a problem).

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
@zyga zyga merged commit baf26ec into snapcore:master Apr 1, 2016
err = os.MkdirAll(dirs.AppArmorCacheDir, 0700)
c.Assert(err, IsNil)
// Mock away any real apparmor interaction
s.mockCmd = testutil.MockCommand(c, "apparmor_parser", fakeAppArmorParser)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which mock command? This should probably be s.parserCmd.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants