feat: add device passthru mode

[reddevillg]

Add --device-mode option to allow containers to share the host's /dev directory.

[gemini-code-assist]

Code Review

This pull request introduces support for device passthrough in the container runtime. It updates the API schema and CLI to support a new "passthru" device option, which triggers a bind-mount of the host's /dev and /run/udev directories into the container. The review feedback suggests security hardening for the /dev bind-mount by adding nosuid and nodev options to mitigate risks, as well as minor performance optimizations in loops that check for the passthrough configuration.

[gemini-code-assist]

For security hardening, it's recommended to add nosuid and nodev to the mount options when bind-mounting the host's /dev directory. This helps mitigate potential security risks associated with device passthrough by preventing SUID binaries from being executed and character/block devices from being created within the container's /dev mount.

                   .options = string_list{ "rbind", "nosuid", "nodev" },

[codecov]

Codecov Report

❌ Patch coverage is 21.05263% with 30 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...glong/oci-cfg-generators/container_cfg_builder.cpp	0.00%	11 Missing and 1 partial ⚠️
...inglong/src/linglong/runtime/container_builder.cpp	42.85%	3 Missing and 5 partials ⚠️
apps/ll-cli/src/main.cpp	0.00%	6 Missing ⚠️
libs/linglong/src/linglong/cli/cli.cpp	0.00%	2 Missing ⚠️
libs/utils/src/linglong/utils/runtime_config.cpp	50.00%	0 Missing and 2 partials ⚠️

Files with missing lines	Coverage Δ
libs/linglong/src/linglong/cli/cli.h	8.33% <ø> (ø)
...inglong/oci-cfg-generators/container_cfg_builder.h	34.14% <ø> (ø)
libs/linglong/src/linglong/cli/cli.cpp	1.70% <0.00%> (ø)
libs/utils/src/linglong/utils/runtime_config.cpp	26.66% <50.00%> (+1.66%)	⬆️
apps/ll-cli/src/main.cpp	0.00% <0.00%> (ø)
...inglong/src/linglong/runtime/container_builder.cpp	6.89% <42.85%> (+4.36%)	⬆️
...glong/oci-cfg-generators/container_cfg_builder.cpp	11.92% <0.00%> (-0.06%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[deepin-ci-robot]

deepin pr auto review

Git Diff 代码审查报告

我已仔细审查了这段代码，该代码主要实现了设备透传(device passthrough)功能，允许容器直接访问主机设备。以下是我的详细审查意见：

1. 语法和逻辑审查

1.1 语法正确性

• JSON Schema 和 YAML Schema 的语法正确，新增的 device_mode 字段和 DeviceOption 枚举定义符合规范
• C++ 代码语法正确，没有明显的编译错误
• 新增的 DeviceOption.hpp 文件格式正确，枚举定义规范

1.2 逻辑问题

1. 设备模式合并逻辑问题 (libs/utils/src/linglong/utils/runtime_config.cpp):

   if (config.deviceMode) {
       if (!result.deviceMode) {
           result.deviceMode = config.deviceMode;
       } else {
           result.deviceMode->insert(result.deviceMode->end(),
                                     config.deviceMode->begin(),
                                     config.deviceMode->end());
       }
   }

   • 问题: 当合并多个配置时，设备模式选项会被简单追加，可能导致重复值
   • 建议: 应该去重后再合并，例如使用 std::set 或在插入前检查是否已存在

2. 设备模式处理逻辑 (libs/linglong/src/linglong/runtime/container_builder.cpp):

   for (const auto &option : *runtimeConfig.deviceMode) {
       if (option == api::types::v1::DeviceOption::Passthru) {
           this->devicePassthru = true;
           break;
       }
   }

   • 问题: 只要检测到 Passthru 选项就设置 devicePassthru 为 true 并 break，但如果后续有其他选项需要处理，会被忽略
   • 建议: 如果将来可能添加其他设备选项，应该考虑如何处理多个选项的组合

2. 代码质量审查

2.1 命名规范

• device_mode 和 DeviceOption 的命名符合项目规范
• devicePassthru 布尔变量命名清晰

2.2 代码结构

• 新增的 DeviceOption 枚举类定义在独立的头文件中，符合单一职责原则
• 测试用例添加在 container_builder_test.cpp 中，覆盖了基本功能

2.3 可维护性

• DeviceOption 枚举目前只有一个值 Passthru，但设计为可扩展的枚举类型，便于未来添加更多选项
• 代码中有多处重复的设备模式检查逻辑，可以考虑提取为公共函数

3. 代码性能审查

1. 设备模式检查效率:

   for (const auto &option : *runtimeConfig.deviceMode) {
       if (option == api::types::v1::DeviceOption::Passthru) {
           this->devicePassthru = true;
           break;
       }
   }

   • 分析: 使用线性搜索检查设备模式，对于少量选项(目前只有一个)性能影响不大
   • 建议: 如果未来选项增多，可以考虑使用 std::find 或更高效的查找算法

2. 设备挂载性能:

   • 在 bindDev 方法中，根据 passthru 参数选择不同的挂载策略
   • 透传模式使用 rbind 直接挂载整个 /dev 目录，性能较好
   • 非透传模式需要挂载多个特定设备点，相对复杂但更安全

4. 代码安全审查

4.1 安全风险

1. 设备透传安全性:

   if (devPassthru) {
       devMount = {
           Mount{ .destination = "/dev",
                  .options = string_list{ "rbind" },
                  .source = "/dev",
                  .type = "bind" },
       };
   }

   • 风险: 直接将主机的 /dev 目录挂载到容器中，可能暴露敏感设备
   • 建议: 应该在文档中明确说明此功能的安全风险，并建议仅在必要时使用
   • 改进: 可以考虑添加白名单机制，只挂载特定的设备

2. 权限控制:

   • 代码中没有看到对设备透传功能的权限检查
   • 建议: 应该添加权限检查，确保只有授权用户可以使用此功能

3. 输入验证:

   • CLI 参数处理中使用了 CLI::CheckedTransformer 进行输入验证，这是好的做法
   • 但在处理 JSON 配置时，应该确保 device_mode 字段只包含预定义的选项

4.2 安全建议

1. 添加设备透传功能的权限检查
2. 考虑实现设备白名单机制
3. 在文档中明确说明设备透传的安全风险
4. 考虑添加日志记录，记录设备透传的使用情况

5. 其他建议

1. 测试覆盖:

   • 测试用例主要关注了 Passthru 选项的设置
   • 建议添加更多边界情况测试，例如空设备模式列表、无效设备模式等

2. 文档更新:

   • 建议更新 CLI 帮助文档，说明 --device-mode 选项的用途和风险
   • 更新 API 文档，说明 device_mode 字段的含义和可能的值

3. 错误处理:

   • 在处理设备模式时，可以添加更详细的错误信息
   • 例如，当检测到无效的设备选项时，提供清晰的错误提示

总结

这段代码实现了设备透传功能，整体设计合理，语法正确。主要改进点在于：

1. 设备模式合并逻辑需要去重
2. 应该添加设备透传的权限检查
3. 考虑实现更精细的设备控制机制
4. 增强测试覆盖和文档说明

这些改进将有助于提高代码的安全性、可靠性和可维护性。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengbo11, reddevillg

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment