Skip to content

fix: allow duplicate additionalGids #3189

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

Merged
merged 1 commit into from
Jun 23, 2025
Merged

Conversation

saku3
Copy link
Contributor

@saku3 saku3 commented Jun 22, 2025

Description

In runc v1.3.0, the behavior has changed when specifying duplicate additionalGids.
To align with this change, we are updating the behavior of additionalGids handling in youki accordingly.

For more details, please refer to the following issue.
opencontainers/runc#4769 (comment)

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):
    compatibility with runc

Testing

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

Related Issues

Fixes #

Additional Context

@utam0k utam0k requested a review from Copilot June 23, 2025 11:36
Copy link
Contributor

@Copilot 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 updates Youki’s handling of duplicate supplementary group IDs (additionalGids) to match runc v1.3.0 by preserving duplicates instead of deduplicating them. It also adapts existing tests to conditionally skip the duplicate-GID check when running under runc.

  • Remove HashSet-based deduplication in set_supplementary_gids
  • Update unit tests to expect preserved duplicates
  • Introduce ConditionalTest to skip duplicate-GID tests on runc

Reviewed Changes

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

File Description
tests/contest/contest/src/tests/process_user/process_user_test.rs Switch to ConditionalTest and adjust duplicate-gid test logic
crates/libcontainer/src/process/init/process.rs Remove duplicate filtering and add unit test for duplicate GIDs
Comments suppressed due to low confidence (1)

tests/contest/contest/src/tests/process_user/process_user_test.rs:69

  • [nitpick] The comment mentions skipping only runc v1.1, but the condition !is_runtime_runc() skips on any runc runtime. Consider clarifying the comment (or the condition) to accurately reflect that all runc runtimes are excluded.
    // The current CI for Youki uses runc 1.1, so this will be skipped.

@utam0k
Copy link
Member

utam0k commented Jun 23, 2025

This leads to a breaking change.

@utam0k utam0k merged commit 356db35 into youki-dev:main Jun 23, 2025
31 of 32 checks passed
@github-actions github-actions bot mentioned this pull request Jun 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants