contrib/apparmor: add tests for generated template, add "header comment", and cleanup whitespace#13270
Draft
thaJeztah wants to merge 5 commits into
Draft
contrib/apparmor: add tests for generated template, add "header comment", and cleanup whitespace#13270thaJeztah wants to merge 5 commits into
thaJeztah wants to merge 5 commits into
Conversation
thaJeztah
force-pushed
the
apparmor_testable_templates
branch
3 times, most recently
from
April 23, 2026 12:52
558494d to
4392a3e
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds golden-file based tests for the AppArmor profile generator in contrib/apparmor, updates the template to include a generated-by header comment, and normalizes template whitespace/output formatting.
Changes:
- Introduce golden files covering multiple template configurations (ABI, tunables, abstractions, custom imports, daemon profile, rootlesskit).
- Add a table-driven golden test with an
-updateflag to refresh expected outputs. - Update the template header/upstream reference and adjust whitespace trimming; allow tests to override macro-detection.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| contrib/apparmor/template.go | Adds generated-by header, whitespace trimming tweaks, and loadData macroExists override hook for tests. |
| contrib/apparmor/template_test.go | Adds table-driven golden tests and golden update helper. |
| contrib/apparmor/testdata/default.golden | Golden output for default template. |
| contrib/apparmor/testdata/with-api3.golden | Golden output when ABI 3.0 macro exists. |
| contrib/apparmor/testdata/with-tunables.golden | Golden output when tunables/global is included. |
| contrib/apparmor/testdata/with-abstractions-base.golden | Golden output when abstractions/base exists as an inner import. |
| contrib/apparmor/testdata/with-daemon-profile.golden | Golden output with explicit daemon profile peer. |
| contrib/apparmor/testdata/with-custom-imports.golden | Golden output with custom top-level imports. |
| contrib/apparmor/testdata/with-custom-inner-imports.golden | Golden output with custom inner imports. |
| contrib/apparmor/testdata/with-rootlesskit.golden | Golden output including RootlessKit peer signal rule. |
| contrib/apparmor/apparmor.go | Updates loadData call sites for new signature; doc comment capitalization. |
| contrib/apparmor/apparmor_unsupported.go | Doc comment capitalization consistency (AppArmor). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
thaJeztah
force-pushed
the
apparmor_testable_templates
branch
from
April 23, 2026 12:56
4392a3e to
0cfca3d
Compare
Member
Author
|
Ah, fun; looks like the profile we detect contains a newline; Diff:
--- Expected
+++ Actual
@@ -16,3 +16,4 @@
# Manager may send signals to container processes.
- signal (receive) peer=unconfined,
+ signal (receive) peer=unconfined
+,
# Container processes may send signals amongst themselves. |
thaJeztah
force-pushed
the
apparmor_testable_templates
branch
from
April 23, 2026 17:56
06a22ac to
bb251fe
Compare
thaJeztah
marked this pull request as draft
April 23, 2026 18:00
thaJeztah
force-pushed
the
apparmor_testable_templates
branch
from
April 23, 2026 18:02
bb251fe to
cbb52c4
Compare
thaJeztah
marked this pull request as ready for review
April 23, 2026 18:02
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
thaJeztah
force-pushed
the
apparmor_testable_templates
branch
from
April 23, 2026 18:46
cbb52c4 to
18ccfd6
Compare
thaJeztah
commented
Apr 23, 2026
| const dir = "/etc/apparmor.d" | ||
|
|
||
| const defaultTemplate = ` | ||
| const defaultTemplate = `# profile generated by github.com/containerd/containerd/v2/contrib/apparmor. |
Member
Author
There was a problem hiding this comment.
Wasn't sure if we wanted to add the DO NOT EDIT. here as well, so left that out for now.
thaJeztah
commented
Apr 23, 2026
| // Normally profiles are suffixed by " (enforce)". AppArmor profiles cannot | ||
| // contain spaces so this doesn't restrict daemon profile names. | ||
| profile, _, _ = strings.Cut(profile, " ") | ||
| profile = strings.TrimSpace(profile) |
Member
Author
There was a problem hiding this comment.
Looks like this might actually be fixing a bug; a profile with a newline here looks to be invalid; I can open this commit as a separate PR first (in case we need / want to backport it)
Diff:
--- Expected
+++ Actual
@@ -16,3 +16,4 @@
# Manager may send signals to container processes.
- signal (receive) peer=unconfined,
+ signal (receive) peer=unconfined
+,
# Container processes may send signals amongst themselves.
thaJeztah
force-pushed
the
apparmor_testable_templates
branch
from
April 23, 2026 18:52
18ccfd6 to
24b1d09
Compare
thaJeztah
marked this pull request as draft
April 23, 2026 18:53
thaJeztah
force-pushed
the
apparmor_testable_templates
branch
from
April 24, 2026 10:37
24b1d09 to
0b211d7
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
With testify, expected values go to the left and use sub-tests. Signed-off-by: Sebastiaan van Stijn <[email protected]>
The profile we read from /proc/self/attr/current will contain a newline,
resulting in a stray newline to be added to the profile;
Diff:
--- Expected
+++ Actual
@@ -16,3 +16,4 @@
# Manager may send signals to container processes.
- signal (receive) peer=unconfined,
+ signal (receive) peer=unconfined
+,
# Container processes may send signals amongst themselves.
Trim whitespace to account for newlines and fix handling of whitespace as
AppArmor profile names are allowed to contain spaces when quoted; from the
[apparmor.d(5)] man-page:
PROFILE NAME ( UNQUOTED PROFILE NAME | QUOTED PROFILE NAME )
QUOTED PROFILE NAME = '"' UNQUOTED PROFILE NAME '"'
UNQUOTED PROFILE NAME = (must start with alphanumeric character (after
variable expansion), or '/' AARE have special meanings; see below. May
include VARIABLE. Rules with embedded spaces or tabs must be quoted.)
While we don't use those names in our code, let's make the code correct.
Also update the template to use quotes.
[apparmor.d(5)]: https://manpages.ubuntu.com/manpages/xenial/man5/apparmor.d.5.html
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
- The generated profiles had a leading newline; replace it with something useful. - Trim whitespace / stray newlines for conditional parts. Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
apparmor_testable_templates
branch
from
June 29, 2026 17:15
0b211d7 to
26922ba
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.