Add uid_mappings test by moz-sec · Pull Request #3161 · youki-dev/youki

moz-sec · 2025-05-04T01:36:51Z

Description

This implements the uid_mappings validation in #361 .
I wrote it based on linux_uid_mappings.go from opencontainers/runtime-tools.

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

#361

Additional Context

utam0k · 2025-05-04T02:20:42Z

Please check the failed CIs

Copilot

Pull Request Overview

This PR adds tests to validate uid and gid mappings as part of issue #361, ensuring that the runtime specification correctly reflects the system's uid/gid mapping state. Key changes include:

Adding a helper function to validate id mappings (uid/gid) in tests.
Updating the test runner to include the "uid_mappings" test.
Adding a new test suite for uid_mappings.

Reviewed Changes

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

Show a summary per file

File	Description
tests/contest/runtimetest/src/tests.rs	Added validate_id_mappings and validate_uid_mappings functions to check uid/gid mapping files.
tests/contest/runtimetest/src/main.rs	Included the uid_mappings test in the test selection logic.
tests/contest/contest/src/tests/uid_mappings/uid_mappings_test.rs	Added a new test suite for uid_mappings with spec creation and test execution.
tests/contest/contest/src/tests/uid_mappings/mod.rs	Exposes the uid_mappings test for integration with the overall test framework.
tests/contest/contest/src/tests/mod.rs	Registered the uid_mappings module as part of the tests.
tests/contest/contest/src/main.rs	Updated the test manager to include the uid_mappings test group.

moz-sec · 2025-05-04T04:03:02Z

To correct the second embarrassing mistake, I re-committed and did a force push.
At the same time, I fixed the first one.

Copilot

Pull Request Overview

This PR adds tests for uid_mappings validation as described in issue #361. It introduces a new helper function (validate_id_mappings) in the runtime tests, updates the test executor to handle "uid_mappings", and creates an integration test for uid_mappings under the contest tests.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/contest/runtimetest/src/tests.rs	Adds validate_id_mappings and validate_uid_mappings to verify id mapping files
tests/contest/runtimetest/src/main.rs	Updates the test runner to include "uid_mappings"
tests/contest/contest/src/tests/uid_mappings/uid_mappings_test.rs	Provides a new test that creates a spec with uid and gid mappings
tests/contest/contest/src/tests/uid_mappings/mod.rs	Exposes the uid_mappings test group
tests/contest/contest/src/tests/mod.rs	Registers the uid_mappings test module
tests/contest/contest/src/main.rs	Includes uid_mappings in the execution of test groups

Signed-off-by: moz-sec <m0253c@gmail.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: moz-sec <m0253c@gmail.com>

Signed-off-by: moz-sec <m0253c@gmail.com>

Copilot

Pull Request Overview

This PR adds tests for UID/GID mappings validation. The changes introduce a new function, validate_id_mappings, and integrate UID/GID mapping tests into both the runtime tests and contest test suites.

Adds functions to validate UID/GID mappings in tests/contest/runtimetest/src/tests.rs.
Updates main test dispatchers in tests/contest/runtimetest/src/main.rs and tests/contest/contest/src/main.rs.
Adds a new uid_mappings test in tests/contest/contest/src/tests/uid_mappings/uid_mappings_test.rs and corresponding mod updates.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/contest/runtimetest/src/tests.rs	Introduces validate_id_mappings and validate_uid_mappings for UID/GID testing; potential issues with error messaging and index assumptions noted.
tests/contest/runtimetest/src/main.rs	Registers the new "uid_mappings" test.
tests/contest/contest/src/tests/uid_mappings/uid_mappings_test.rs	Adds a new test case for uid and gid mappings.
tests/contest/contest/src/tests/uid_mappings/mod.rs	Exposes the uid_mappings test group.
tests/contest/contest/src/main.rs	Updates test groups to include uid_mappings.

Comments suppressed due to low confidence (1)

tests/contest/runtimetest/src/tests.rs:1014

The loop iterating over the lines assumes that the number of lines exactly matches the count of expected_id_mappings, which may lead to an index out-of-bounds error. Consider checking that the lengths match before iterating or using a zip-based iteration to safely compare corresponding entries.

if !(actual_host_id == expected_id_mappings[i].host_id() && actual_container_id == expected_id_mappings[i].container_id() && actual_map_size == expected_id_mappings[i].size())

Signed-off-by: moz-sec <m0253c@gmail.com>

Copilot

Pull Request Overview

This PR adds tests for uid_mappings validation as described in #361 by implementing new validation functions and integrating them into both the runtime test suite and the contest test framework.

Introduces validate_id_mappings and validate_uid_mappings functions in tests/contest/runtimetest/src/tests.rs.
Updates main and contest test runners to register and execute the uid_mappings tests.
Adds a new uid_mappings test module in tests/contest/contest/src/tests/uid_mappings.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/contest/runtimetest/src/tests.rs	Adds uid_mappings validation functions and necessary import updates.
tests/contest/runtimetest/src/main.rs	Registers uid_mappings test in the main test runner.
tests/contest/contest/src/tests/uid_mappings/uid_mappings_test.rs	Introduces a new uid_mappings test using LinuxIdMappingBuilder.
tests/contest/contest/src/tests/uid_mappings/mod.rs	Declares uid_mappings_test module and re-exports the test getter.
tests/contest/contest/src/tests/mod.rs	Registers uid_mappings test group in the contest tests module.
tests/contest/contest/src/main.rs	Integrates uid_mappings test group into the contest main runner.

Comments suppressed due to low confidence (1)

tests/contest/contest/src/tests/uid_mappings/uid_mappings_test.rs:48

[nitpick] Consider renaming the variable 'gid_mapping' to 'gid_mappings' to better reflect that it is a collection of mappings and to maintain consistency with 'uid_mappings'.

let gid_mapping = vec![LinuxIdMappingBuilder::default()

moz-sec · 2025-05-10T03:24:15Z

@utam0k
I have corrected review of Copilot.
May I ask you to review it again?

utam0k · 2025-05-12T12:07:03Z

+        .typ(LinuxNamespaceType::User)
+        .build()
+        .unwrap();
+    default_namespaces.push(userns);


This is obviously no longer default_namespaces.

Are you saying that the variable name default_namespaces is not appropriate?
For example, does it mean that I should name the variable namespaces and push user namespaces?

default_namespaces clearly has a different variable name and role at the time it is pushed.

Signed-off-by: moz-sec <m0253c@gmail.com>

utam0k · 2025-05-28T11:49:58Z

Sorry, but this PR is in conflict. Could you check it?

Signed-off-by: moz-sec <m0253c@gmail.com>

YJDoc2

couple of minor comments, but overall ok. Also when you're responding to these, can you rebase / merge main?

Signed-off-by: moz-sec <m0253c@gmail.com>

YJDoc2

LGTM! I think comments by @utam0k are also addressed, so going ahead and merging.

Thanks @moz-sec :)

* add_uid_mappings test Signed-off-by: moz-sec <m0253c@gmail.com> * feat: handle multiple mappings Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: moz-sec <m0253c@gmail.com> * resolve borrowing issue with variable Signed-off-by: moz-sec <m0253c@gmail.com> * change the error message to match the process Signed-off-by: moz-sec <m0253c@gmail.com> * use zip to pair and compare expected_id_mappings and lines Signed-off-by: moz-sec <m0253c@gmail.com> * make all variable names common in uid/gid_mappings Signed-off-by: moz-sec <m0253c@gmail.com> * rename the variable Signed-off-by: moz-sec <m0253c@gmail.com> * delete unwanted comments Signed-off-by: moz-sec <m0253c@gmail.com> * fix: unwrap directly without using a match Signed-off-by: moz-sec <m0253c@gmail.com> * feat: randomize the numbers to be mapped Signed-off-by: moz-sec <m0253c@gmail.com> --------- Signed-off-by: moz-sec <m0253c@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

utam0k added the kind/test label May 4, 2025

utam0k requested a review from Copilot May 4, 2025 02:20

Copilot AI reviewed May 4, 2025

View reviewed changes

Comment thread tests/contest/runtimetest/src/tests.rs

Comment thread tests/contest/contest/src/tests/uid_mappings/uid_mappings_test.rs Outdated

moz-sec force-pushed the test-uid-mappings branch from 983a51b to 9be2e23 Compare May 4, 2025 03:59

utam0k requested a review from Copilot May 9, 2025 12:02

Copilot AI reviewed May 9, 2025

View reviewed changes

Comment thread tests/contest/runtimetest/src/tests.rs Outdated

moz-sec force-pushed the test-uid-mappings branch from 1cc7c98 to 8fc4387 Compare May 10, 2025 01:52

moz-sec and others added 3 commits May 10, 2025 10:54

add_uid_mappings test

438b3f7

Signed-off-by: moz-sec <m0253c@gmail.com>

feat: handle multiple mappings

558056b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: moz-sec <m0253c@gmail.com>

resolve borrowing issue with variable

049a251

Signed-off-by: moz-sec <m0253c@gmail.com>

moz-sec force-pushed the test-uid-mappings branch from 8fc4387 to 049a251 Compare May 10, 2025 02:21

moz-sec requested a review from Copilot May 10, 2025 02:29

Copilot AI reviewed May 10, 2025

View reviewed changes

Comment thread tests/contest/runtimetest/src/tests.rs Outdated

moz-sec added 2 commits May 10, 2025 11:41

change the error message to match the process

576977c

Signed-off-by: moz-sec <m0253c@gmail.com>

use zip to pair and compare expected_id_mappings and lines

9ccf03e

Signed-off-by: moz-sec <m0253c@gmail.com>

moz-sec requested a review from Copilot May 10, 2025 03:08

Copilot AI reviewed May 10, 2025

View reviewed changes

Comment thread tests/contest/runtimetest/src/tests.rs

utam0k reviewed May 12, 2025

View reviewed changes

moz-sec added 2 commits May 18, 2025 11:24

make all variable names common in uid/gid_mappings

026517a

Signed-off-by: moz-sec <m0253c@gmail.com>

rename the variable

f3e34f9

Signed-off-by: moz-sec <m0253c@gmail.com>

moz-sec requested a review from utam0k May 24, 2025 02:02

utam0k reviewed May 28, 2025

View reviewed changes

Comment thread tests/contest/contest/src/tests/uid_mappings/uid_mappings_test.rs Outdated

moz-sec and others added 2 commits May 29, 2025 23:24

Merge branch 'main' into test-uid-mappings

b17815d

delete unwanted comments

8b1ff9e

Signed-off-by: moz-sec <m0253c@gmail.com>

moz-sec requested a review from utam0k May 30, 2025 00:16

YJDoc2 self-requested a review June 4, 2025 11:11

YJDoc2 reviewed Jun 4, 2025

View reviewed changes

Comment thread tests/contest/runtimetest/src/tests.rs Outdated

Comment thread tests/contest/contest/src/tests/uid_mappings/uid_mappings_test.rs Outdated

moz-sec added 2 commits June 5, 2025 10:46

fix: unwrap directly without using a match

3db8ce2

Signed-off-by: moz-sec <m0253c@gmail.com>

feat: randomize the numbers to be mapped

79b395c

Signed-off-by: moz-sec <m0253c@gmail.com>

moz-sec requested a review from YJDoc2 June 5, 2025 13:23

YJDoc2 approved these changes Jun 6, 2025

View reviewed changes

YJDoc2 enabled auto-merge (squash) June 6, 2025 04:39

YJDoc2 merged commit c2ab4de into youki-dev:main Jun 6, 2025

github-actions Bot mentioned this pull request Jun 6, 2025

Release for v0.5.4 #3124

Merged

moz-sec deleted the test-uid-mappings branch June 6, 2025 05:42

This was referenced Jun 9, 2025

Remove oci tests that are duplicates of contest #3184

Merged

[Tracking Issue] Implement Integraion Tests #361

Open

Uh oh!

Conversation

moz-sec commented May 4, 2025

Description

Type of Change

Testing

Related Issues

Additional Context

Uh oh!

utam0k commented May 4, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

moz-sec commented May 4, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

moz-sec commented May 10, 2025

Uh oh!

utam0k May 12, 2025

Choose a reason for hiding this comment

Uh oh!

moz-sec May 15, 2025

Choose a reason for hiding this comment

Uh oh!

utam0k May 17, 2025

Choose a reason for hiding this comment

Uh oh!

utam0k commented May 28, 2025

Uh oh!

Uh oh!

YJDoc2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

YJDoc2 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants