Skip to content

Comments

fix(3293) Ambient capabilities are not applied as expected#3294

Merged
utam0k merged 5 commits intoyouki-dev:mainfrom
tommady:close-issue-3293
Dec 12, 2025
Merged

fix(3293) Ambient capabilities are not applied as expected#3294
utam0k merged 5 commits intoyouki-dev:mainfrom
tommady:close-issue-3293

Conversation

@tommady
Copy link
Collaborator

@tommady tommady commented Nov 17, 2025

Description

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test updates
  • CI/CD related changes
  • Other (please describe):

Testing

  • Added new unit tests
  • Added new integration tests
  • Ran existing test suite
  • Tested manually (please provide steps)

Related Issues

Fixes #3293

Additional Context

follow the Steps to Reproduce and Expectation from the issue

Signed-off-by: tommady <tommady@users.noreply.github.com>
@tommady tommady marked this pull request as ready for review November 17, 2025 13:26
@tommady
Copy link
Collaborator Author

tommady commented Nov 17, 2025

hi @saku3
please help review while you have time.
thanks.

…e move that into deeper layer

Signed-off-by: tommady <tommady@users.noreply.github.com>
@saku3 saku3 added the kind/bug label Nov 17, 2025
@saku3 saku3 requested a review from Copilot November 17, 2025 21:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue where ambient capabilities were not being applied correctly to containers. The fix changes the error handling approach from silently logging warnings to properly raising ambient capabilities and propagating errors.

Key Changes:

  • Added explicit handling for CapSet::Ambient in the syscall layer to raise each capability individually
  • Changed ambient capability setting to propagate errors instead of silently failing with warnings

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
crates/libcontainer/src/syscall/linux.rs Added new match arm for CapSet::Ambient to raise capabilities individually with error logging
crates/libcontainer/src/capabilities.rs Removed silent error handling for ambient capabilities, now propagates errors via ? operator

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: tommady <tommady@users.noreply.github.com>
Signed-off-by: tommady <tommady@users.noreply.github.com>
@tommady tommady requested a review from saku3 November 18, 2025 07:02
Signed-off-by: tommady <tommady@users.noreply.github.com>
Copy link
Member

@saku3 saku3 left a comment

Choose a reason for hiding this comment

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

Thanks!

It looks good for me.

for i in {1..100}; do youki run -b tutorial/ a; done

I ran the above command locally and confirmed that the result is consistent.
(However, I’m not sure how we should write a test for this...)

@saku3
Copy link
Member

saku3 commented Nov 18, 2025

@utam0k @YJDoc2
PTAL

@saku3 saku3 requested a review from utam0k November 29, 2025 06:19
Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

May I ask you to add the e2e test(contest)?

@tommady
Copy link
Collaborator Author

tommady commented Dec 2, 2025

May I ask you to add the e2e test(contest)?

Hi @utam0k,
there will be an e2e test in this PR
#3210
which is the reason why @saku3 found this issue

is that ok with you?

@saku3
Copy link
Member

saku3 commented Dec 7, 2025

As tommady commented, once this PR(#3210) is merged, there shouldn’t be any regressions related to this issue.

Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

LGTM

@utam0k utam0k merged commit ed3d6f6 into youki-dev:main Dec 12, 2025
28 checks passed
@github-actions github-actions bot mentioned this pull request Dec 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Ambient capabilities are not applied as expected

3 participants