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.

Sign up for GitHub

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

Jump to bottom

Instance: Allow nosymfollow mount flag for container apparmor profile #13681

Merged
merged 2 commits into from
Jul 16, 2024
Merged

Instance: Allow nosymfollow mount flag for container apparmor profile #13681

merged 2 commits into from
Jul 16, 2024

Conversation

mihalicyn
Copy link
Member

@mihalicyn mihalicyn commented Jun 28, 2024 •
edited by tomponline
Loading

Closes #12698

@mihalicyn mihalicyn requested a review from tomponline as a code owner June 28, 2024 12:36
@mihalicyn
Copy link
Member Author

cc @enr0n

@mihalicyn mihalicyn requested a review from simondeziel June 28, 2024 12:37
@mihalicyn mihalicyn mentioned this pull request Jun 28, 2024
Closed
@mihalicyn mihalicyn force-pushed the apparmor_nosymfollow branch 2 times, most recently from b940de9 to 24dcedb Compare June 28, 2024 13:09
@tomponline
Copy link
Member

@mihalicyn please can you add what this fix resolves in terms of which ubuntu version we can now run as containers?

@mihalicyn mihalicyn force-pushed the apparmor_nosymfollow branch from 24dcedb to b7e13e8 Compare June 28, 2024 13:44
@mihalicyn
Copy link
Member Author

@mihalicyn please can you add what this fix resolves in terms of which ubuntu version we can now run as containers?

I'm not sure, because if I understand correcly this issue is only reproducible with some experimental systemd versions and with Oracular. I wasn't able to reproduce any issues locally. I hope @enr0n will help us with testing/validation of this.

@mihalicyn mihalicyn force-pushed the apparmor_nosymfollow branch from b7e13e8 to 42a75bc Compare June 28, 2024 14:28
simondeziel
simondeziel previously approved these changes Jun 28, 2024
View reviewed changes
lxd/apparmor/feature_check.go Outdated Show resolved Hide resolved
@tomponline tomponline changed the title apparmor/lxc: allow nosymfollow mount flag Instance: Allow nosymfollow mount flag for container apparmor profile Jun 28, 2024
mihalicyn added 2 commits July 1, 2024 10:38
@mihalicyn
lxd/apparmor/feature_check: add infastructure to check AppArmor features
47180a3 
Add some infastructure to check AppArmor features availability, as
version checks can not be reliable because of backports of different features
between branches.

Signed-off-by: Alexander Mikhalitsyn <[email protected]>
@mihalicyn
lxd/apparmor/instance_lxc: allow nosymfollow mount flag
b5a4a65 
See also:
canonical#12698

Thanks-to: Nick Rosbrook <[email protected]>
Signed-off-by: Alexander Mikhalitsyn <[email protected]>
@mihalicyn mihalicyn force-pushed the apparmor_nosymfollow branch from 42a75bc to b5a4a65 Compare July 1, 2024 08:38
@mihalicyn mihalicyn requested a review from simondeziel July 1, 2024 08:39
tomponline
tomponline reviewed Jul 8, 2024
View reviewed changes
lxd/sys/os.go
@@ -69,6 +75,7 @@ type OS struct {
AppArmorConfined bool
AppArmorStacked bool
AppArmorStacking bool
AppArmorFeatures AppArmorFeaturesInfo
Copy link
Member

Choose a reason for hiding this comment

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

Can we make AppArmor a field with its own struct and internal mutex and then Features a map type, so we access it using OS.AppArmor.Features, rather than OS.AppArmorFeatures.Map which seems rather unusual (to have a field named after a data type) or OS.AppArmorFeatures.Features (which stutters)?

Copy link
Member

Choose a reason for hiding this comment

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

We'd need to move the other AppArmor* fields into this new sub-struct.

@tomponline
Copy link
Member

tomponline commented Jul 8, 2024 •
edited
Loading

@mihalicyn @enr0n has advised:

The quickest thing to check is systemctl status systemd-resolved (or systemd-networkd, systemd-journald, or many others).
Without the fix, they will fail with 243/CREDENTIALS
On oracular with systemd from oracular-proposed
So systemd v256 essentially
proposed is not enabled by default. So you need to enable that source and then apt install systemd -t oracular-proposed

Please can you test this PR with that to check it resolves it. Ta

@mihalicyn
Copy link
Member Author

Tested and it does not work. Looks like we need something else in addition to this change.

@tomponline
Copy link
Member

Tested and it does not work. Looks like we need something else in addition to this change.

Lets move this back to draft.

@tomponline tomponline marked this pull request as draft July 11, 2024 10:01
@tomponline tomponline mentioned this pull request Jul 12, 2024
Open
@tomponline tomponline marked this pull request as ready for review July 16, 2024 12:53
@tomponline tomponline merged commit d5501d7 into canonical:main Jul 16, 2024
29 checks passed
@mihalicyn mihalicyn mentioned this pull request Jul 26, 2024
Closed
4 tasks
tomponline added a commit to tomponline/lxd-pkg-snap that referenced this pull request Aug 1, 2024
@tomponline
lxd: Cherry-pick Oracular container apparmor fixes
552bb80 
canonical/lxd#13681
canonical/lxd#13820

Signed-off-by: Thomas Parrott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simondeziel simondeziel simondeziel approved these changes

@tomponline tomponline tomponline approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mihalicyn @tomponline @simondeziel