feature(containerz): Add CNTR-1.11 comprehensive capabilities matrix and deviation enforcement - #5969
Conversation
…ons tests to CNTR-1 Updates feature/container/containerz/tests/container_lifecycle/: - README.md: Document CNTR-1.9 (Capabilities) and CNTR-1.10 (Volume Mount Options Matrix). - containerz_test.go: Implement TestCapabilities (elevated, dropped CAP_NET_RAW/CAP_SYS_CHROOT, invalid rejection) and TestVolumeMountOptions (rbind, rslave, rprivate, ro, bind matrix reflection and deletion verification). Hardware Verification: - Target: Arista DCS-7280CR3K-32D4-F (EOS 4.34.2FX-wbb.2-DPE-CTNR) - Subtests passed: TestCapabilities (Elevated, Dropped, Invalid), TestVolumeMountOptions (RbindRslave, RbindRprivate, BindRslaveRo, BindRo, Bind)
…error in TestCapabilities
…lid capability in TestCapabilities
…n for capability rejection
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive validation suite for container capabilities within the gnoi.Containerz framework. It ensures that the system correctly handles the full spectrum of Linux capabilities, including both granting elevated privileges and dropping default bounding set capabilities, while also enforcing platform-specific deviations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Pull Request Functional Test Report for #5969 / f2770c7Virtual Devices
Hardware Devices
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds test plans and Go implementations for three new container lifecycle tests: CNTR-1.9 (capabilities validation), CNTR-1.10 (volume mount options), and CNTR-1.11 (comprehensive capabilities matrix). Feedback highlights that the platform deviation check for unsupported capabilities is missing from the CNTR-1.11 implementation, and that CNTR-1.9 and CNTR-1.10 are not registered in testregistry.textproto. Additionally, the exec path for CNTR-1.11 is left blank, and boolean flags should be used to prevent redundant container cleanup operations in the subtests.
| // TestComprehensiveCapabilities implements CNTR-1.11 validating that containers can be started with | ||
| // the complete spectrum of standard Linux capabilities, dropped capabilities in the default bounding set, | ||
| // and deviation/invalid capability rejection handling. | ||
| func TestComprehensiveCapabilities(t *testing.T) { |
There was a problem hiding this comment.
The test plan in README.md specifies checking platform deviations using deviations.ContainerzUnsupportedCapabilities(dut) to validate unsupported capabilities on targets where vendor daemon policies restrict specific capability flags. However, this check is completely missing from the test implementation in TestComprehensiveCapabilities.\n\nPlease implement this deviation check as documented in the test plan. According to the Deviation Guidelines in the repository style guide:\n1. Define the deviation in proto/metadata.proto.\n2. Generate Go code using make proto/metadata_go_proto/metadata.pb.go.\n3. Add an accessor function in internal/deviations/deviations.go that accepts *ondatra.DUTDevice.\n4. Enable the deviation in the test's metadata.textproto file.\n5. Access the deviation in the test via deviations.ContainerzUnsupportedCapabilities(dut).
References
- Access deviations via deviations.DeviationName(dut). (link)
| if err := cli.StopContainer(ctx, instName, true); err != nil { | ||
| t.Errorf("StopContainer(%q) failed: %v", instName, err) | ||
| } | ||
| if err := cli.RemoveContainer(ctx, instName, true); err != nil { | ||
| t.Errorf("RemoveContainer(%q) failed: %v", instName, err) | ||
| } |
There was a problem hiding this comment.
The explicit calls to cli.StopContainer and cli.RemoveContainer at the end of the CapAdd subtest are performed during normal execution, but they are also registered in the deferred cleanup block. To prevent redundant cleanup operations (and unnecessary RPCs) when the deferred block runs, use a boolean flag to track whether the cleanup has already occurred.
References
- When deferring cleanup actions (such as stopping traffic) that may also be called explicitly during normal execution, use a boolean flag to track whether the cleanup has already occurred to prevent redundant cleanup operations in the deferred block.
| if err := cli.StopContainer(ctx, instName, true); err != nil { | ||
| t.Errorf("StopContainer(%q) failed: %v", instName, err) | ||
| } | ||
| if err := cli.RemoveContainer(ctx, instName, true); err != nil { | ||
| t.Errorf("RemoveContainer(%q) failed: %v", instName, err) | ||
| } |
There was a problem hiding this comment.
Similarly, the explicit calls to cli.StopContainer and cli.RemoveContainer at the end of the CapDrop subtest are also registered in the deferred cleanup block. Please use a boolean flag to track whether the cleanup has already occurred to prevent redundant cleanup operations in the deferred block.
References
- When deferring cleanup actions (such as stopping traffic) that may also be called explicitly during normal execution, use a boolean flag to track whether the cleanup has already occurred to prevent redundant cleanup operations in the deferred block.
| id: "CNTR-1.11" | ||
| description: "Comprehensive Container Capabilities Matrix and Deviation Enforcement" | ||
| readme: "https://github.com/openconfig/featureprofiles/blob/main/feature/container/containerz/tests/container_lifecycle/README.md" | ||
| exec: " " |
There was a problem hiding this comment.
The exec field for CNTR-1.11 is currently set to a blank space " ". It should point to the actual test execution file containerz_test.go where TestComprehensiveCapabilities is implemented.
| exec: " " | |
| exec: "https://github.com/openconfig/featureprofiles/blob/main/feature/container/containerz/tests/container_lifecycle/containerz_test.go" |
There was a problem hiding this comment.
Updated the exec field for CNTR-1.11 to point directly to containerz_test.go.
| test: { | ||
| id: "CNTR-1.11" | ||
| description: "Comprehensive Container Capabilities Matrix and Deviation Enforcement" | ||
| readme: "https://github.com/openconfig/featureprofiles/blob/main/feature/container/containerz/tests/container_lifecycle/README.md" | ||
| exec: " " | ||
| } |
There was a problem hiding this comment.
This PR introduces two new test cases: CNTR-1.9 (Container Capabilities Provisioning and Validation) and CNTR-1.10 (Volume Mount Driver Option Matrix and Reflection). However, these test cases are not registered in testregistry.textproto.\n\nPlease register both CNTR-1.9 and CNTR-1.10 in the test registry so they can be properly tracked and executed by the CI pipeline.
There was a problem hiding this comment.
Registered both CNTR-1.9 and CNTR-1.10 in testregistry.textproto alongside CNTR-1.11.
312613d to
5e304e3
Compare
…nations to CNTR-1.10
…and deviation enforcement - Added CNTR-1.11 specification to README.md defining the full 38 standard Linux capabilities catalog and 14 default dropped capability candidates. - Implemented TestComprehensiveCapabilities in containerz_test.go with table-driven subtests for CapAdd and CapDrop. - Handled daemon rejection error codes (InvalidArgument, FailedPrecondition, Internal) and verified zero orphaned containers. - Registered CNTR-1.11 in testregistry.textproto.
5e304e3 to
f2770c7
Compare
2 participants
Description
This PR introduces CNTR-1.11: Comprehensive Container Capabilities Matrix and Deviation Enforcement to validate
gnoi.Containerz.StartContaineragainst the complete spectrum of standard Linux capabilities defined bycapabilities(7)and OpenConfigcontainerz.proto.Scope of Changes
Specification (
README.md):## CNTR-1.11: Comprehensive Container Capabilities Matrix and Deviation Enforcement.CAP_SYS_ADMIN,CAP_SYS_MODULE,CAP_SYS_RAWIO,CAP_SYS_PTRACE,CAP_SYS_PACCT,CAP_SYS_BOOT,CAP_SYS_NICE,CAP_SYS_RESOURCE,CAP_SYS_TIME,CAP_SYS_TTY_CONFIG,CAP_SYSLOG,CAP_WAKE_ALARM,CAP_BLOCK_SUSPEND,CAP_AUDIT_CONTROL,CAP_AUDIT_READ,CAP_AUDIT_WRITECAP_NET_ADMIN,CAP_NET_RAW,CAP_NET_BIND_SERVICE,CAP_NET_BROADCASTCAP_CHOWN,CAP_DAC_OVERRIDE,CAP_DAC_READ_SEARCH,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_SETGID,CAP_SETUID,CAP_SETPCAP,CAP_LINUX_IMMUTABLE,CAP_IPC_LOCK,CAP_IPC_OWNER,CAP_SYS_CHROOT,CAP_MKNOD,CAP_LEASE,CAP_SETFCAP,CAP_MAC_ADMIN,CAP_MAC_OVERRIDECapAdd): Procedures for elevated capability requests, verifying start success and handling daemon rejections (INVALID_ARGUMENT,FAILED_PRECONDITION,INTERNAL) while asserting zero orphaned container state.CapDrop): Procedures for dropping default bounding set capabilities (CAP_NET_RAW,CAP_SYS_CHROOT,CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_MKNOD,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_AUDIT_WRITE).Test Automation (
containerz_test.go):TestComprehensiveCapabilities(t *testing.T)executing table-driven subtests forCapAddandCapDrop.t.Cleanupfor containers and deployed test images.Test Registry (
testregistry.textproto):CNTR-1.11entry with description and README link.