hooks: add script to remove unneeded apparmor profiles#181
hooks: add script to remove unneeded apparmor profiles#181zyga wants to merge 1 commit intocanonical:masterfrom
Conversation
This speeds up boot time as we prevent many calls to apparmor_parser. This patch is amended compared to the core26 counterpart, to provide the correct set of expected profiles (in this case, none). Those differ from base to base.
There was a problem hiding this comment.
Pull request overview
This PR adds a hook script to remove unneeded AppArmor profiles during the core20 image build process, improving boot time by preventing unnecessary calls to apparmor_parser. The implementation is adapted from the core26 base, with the key difference being that core20 expects zero AppArmor profiles to remain after cleanup.
Changes:
- Added hook script that iterates through AppArmor profiles and removes those without matching binaries
- Added test to verify that only expected profiles (none for core20) remain after hook execution
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| hooks/800-remove-unneeded-profiles.chroot | Hook script that removes AppArmor profiles when their corresponding binaries don't exist in specified directories |
| hook-tests/800-remove-unneeded-profiles.test | Test script that verifies no profiles remain after cleanup (core20 expects zero profiles) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #!/bin/bash -e | ||
| set -x | ||
| APPARMOR_PROF_D="etc/apparmor.d" | ||
|
|
There was a problem hiding this comment.
The allowed_profs associative array is intentionally empty to indicate that no AppArmor profiles are expected in core20. Consider adding a comment here to make this explicit, such as: # Core20 expects no AppArmor profiles to remain after cleanup. This would improve code clarity for future maintainers.
| # Core20 expects no AppArmor profiles to remain after cleanup, so this is intentionally empty |
| if [ "$filename" = unix-chkpwd ] | ||
| then filename=unix_chkpwd |
There was a problem hiding this comment.
The if-then statement should follow consistent formatting. The comparison value should be quoted for safety, and either put the then on the same line as if with a semicolon, or put then on a new line. Consider: if [ "$filename" = "unix-chkpwd" ]; then on one line, or split across lines with proper indentation matching the codebase style.
| if [ "$filename" = unix-chkpwd ] | |
| then filename=unix_chkpwd | |
| if [ "$filename" = "unix-chkpwd" ]; then | |
| filename=unix_chkpwd |
| echo "Starting AppArmor profile cleanup..." | ||
|
|
||
| # Iterate through files in /etc/apparmor.d/ | ||
| for profile in "$APPARMOR_PROF_D"/*; do |
There was a problem hiding this comment.
Consider adding a check for directory existence before the loop or setting the nullglob option to handle the case where /etc/apparmor.d doesn't exist or is empty. While the current checks at lines 22-27 will prevent errors, adding 'shopt -s nullglob' before the loop would make the intent clearer and follow bash best practices for glob iteration.
2 participants
This speeds up boot time as we prevent many calls to apparmor_parser.
This patch is amended compared to the core26 counterpart, to provide the correct set of expected profiles (in this case, none). Those differ from base to base.
Thanks for helping us make a better ubuntu core!
Before continuing with the PR, you might want to consider also creating a PR for the new core-base repository
at https://github.com/snapcore/core-base, which contains newer core versions (22+), if the PR is also relevant
for newer core versions.