Skip to content

feat(core): add CDI policy resolver - #2775

Open
elezar wants to merge 2 commits into
mainfrom
1606-cdi-policy-core/elezar
Open

feat(core): add CDI policy resolver#2775
elezar wants to merge 2 commits into
mainfrom
1606-cdi-policy-core/elezar

Conversation

@elezar

@elezar elezar commented Aug 17, 2026

Copy link
Copy Markdown
Member

Summary

Add a portable, fail-closed CDI context resolver that turns validated CDI specs into policy-relevant device, mount, and group requirements. This is the foundation of the stacked #1606 implementation.

Related Issue

Part of #1606.

Changes

  • Add the shared CDI context schema and Linux-only resolver dependency boundary.
  • Validate device-node types, paths, writable mounts, and supplemental GIDs.
  • Add resolver unit coverage using temporary CDI specifications.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (not applicable to this layer)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

@elezar
elezar requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners August 17, 2026 13:02
@elezar
elezar force-pushed the 1606-cdi-policy-core/elezar branch 8 times, most recently from 722e6d9 to a81daf0 Compare August 18, 2026 12:53

@drew drew left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two fail-closed validation issues need correction before this layer merges. I reproduced the cache-refresh case with a focused openshell-core test: two conflicting specs defining the selected device currently return Ok.

context: &CdiContext,
selected_devices: &[String],
) -> Result<CdiContainerEdits, CdiError> {
let (mut cache, refresh_error) = build_cache(&context.spec_dirs);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P1] Fail on cache refresh errors before resolving selected devices. The pinned CDI cache can return an error while retaining a resolvable device (for example, two equal-priority specs defining nvidia.com/gpu=0). Because refresh_error is only consulted when lookup fails, this resolver accepts one ambiguous spec instead of failing closed; I reproduced this with two conflicting YAML specs and resolve_cdi_context returned Ok. Reject the refresh error here, or prove specifically that the selected device was unaffected, and add a duplicate-selected-device regression test.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The issue here is that the building the cache loads all CDI specs from the specified spec directories. These could include specs from different vendors or devices. Furthermore, if a spec fails to load (e.g due to malfomed YAML) there is no way to know which vendor or devices it was intended for.

This is the reason that we retain this information and include it if a device is unresolvable. This behaviour aligns with the behaviour for other CDI consumers including Docker, Podman, Containerd, Crio, and Kata.

I can add a test to prove that duplicate devices of the same priority causes a failure though.

}

fn add_gid(&mut self, gid: u32) {
if gid == 0 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Reject root supplemental GID instead of silently dropping it. The accepted #1606 implementation plan requires GID 0 to fail validation. Skipping it makes an unsafe CDI request look valid and prevents the supervisor from emitting its validation finding. Return a dedicated CdiError and change the existing root-GID test to expect rejection.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's a fair point. When generating a CDI spec, the NVIDIA Contianer Toolkit skips gid==0 but there is no guarantee that other vendors do.

Note that the the Go implementation of the CDI injection code also skips gid==0.

elezar added 2 commits August 19, 2026 09:34
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants