Repository navigation
feat(cli): add --annotation to sandbox create - #4325
ericcurtin wants to merge 2 commits into
Conversation
Signed-off-by: Eric Curtin <eric.curtin@docker.com>
| /// Annotation keys under this prefix are managed by the system, not callers. | ||
| const RESERVED_ANNOTATION_PREFIX: &str = "openshell.nvidia.com/"; | ||
|
|
||
| pub fn parse_annotation_pairs(items: &[String]) -> Result<HashMap<String, String>> { | ||
| let map = parse_key_value_pairs(items, "--annotation")?; | ||
| if let Some(key) = map | ||
| .keys() | ||
| .find(|key| key.starts_with(RESERVED_ANNOTATION_PREFIX)) | ||
| { | ||
| return Err(miette::miette!( | ||
| "--annotation keys starting with {RESERVED_ANNOTATION_PREFIX} are reserved; got '{key}'" | ||
| )); | ||
| } | ||
| Ok(map) | ||
| } |
There was a problem hiding this comment.
Question: Does this mean that when using the gRPC API users can override reserved keys?
There was a problem hiding this comment.
Yes. The server only checks the key shape, and the CLI itself sends openshell.nvidia.com/retention through this field. This check is a CLI guardrail, not a security boundary. Server side enforcement would need an allowlist, happy to do that as a follow-up.
|
@elezar @johntmyers PTAL when you get a chance. Thank you! |
elezar
left a comment
There was a problem hiding this comment.
Please remove the CLI-only reserved annotation key check, its associated test, and the documentation stating that openshell.nvidia.com/ is reserved.
Annotation ownership should be defined consistently and enforced by the gateway across all clients. Existing annotations use both openshell.nvidia.com/ and internal.openshell.ai/, and some keys under the former already support caller/interceptor-supplied provenance.
Follow-up issue #4333 covers namespace consistency and gateway enforcement where reservation applies, including compatibility with retention and provenance workflows. This PR can stay focused on exposing creation-time annotations.
Good call removing the company name from openshell.nvidia.com/ and allowing internal.openshell.ai/ FWIW. When you start blurring the name of the company with the name of the upstream project things can get messy fast... |
Signed-off-by: Eric Curtin <eric.curtin@docker.com>
|
Removed the reserved key check, its test, and the docs note. @elezar PTAL, and |
Summary
Add repeatable
--annotation KEY=VALUEtosandbox create.Related Issue
Closes #4303
Changes
--annotationflag, merged with the ephemeral retention annotation.openshell.nvidia.com/are rejected.--labelnow uses the sharedparse_key_value_pairs(also trims keys and rejects empty ones).Testing
cargo fmt,cargo clippy -p openshell-cli --all-targets -- -D warnings,cargo test -p openshell-cli(lib, bin,sandbox_create_lifecycle_integration), and markdownlint on changed docs.Checklist