Skip to content

Commit 8fcba28

Browse files
feat(cli): add --annotation to sandbox create (#4325)
* feat(cli): add --annotation to sandbox create Signed-off-by: Eric Curtin <eric.curtin@docker.com> * fix(cli): drop reserved annotation check Signed-off-by: Eric Curtin <eric.curtin@docker.com> * test(cli): adapt annotation tests to env helper Signed-off-by: Eric Curtin <eric.curtin@docker.com> --------- Signed-off-by: Eric Curtin <eric.curtin@docker.com> Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> Co-authored-by: John Myers <9696606+johntmyers@users.noreply.github.com>
1 parent b8526eb commit 8fcba28

6 files changed

Lines changed: 165 additions & 21 deletions

File tree

‎crates/openshell-cli/src/commands/common.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,6 +1113,27 @@ pub fn scrub_git_env(command: &mut Command) -> &mut Command {
11131113
mod tests {
11141114
use super::*;
11151115

1116+
#[test]
1117+
fn parse_key_value_pairs_keeps_slashes_spaces_and_equals() {
1118+
let items = vec!["source=/home/you/my policy.yaml?a=b".to_string()];
1119+
let map = parse_key_value_pairs(&items, "--annotation").expect("annotation should parse");
1120+
assert_eq!(
1121+
map.get("source").map(String::as_str),
1122+
Some("/home/you/my policy.yaml?a=b")
1123+
);
1124+
}
1125+
1126+
#[test]
1127+
fn parse_key_value_pairs_rejects_malformed_and_empty_keys() {
1128+
let err =
1129+
parse_key_value_pairs(&["nokey".to_string()], "--annotation").expect_err("missing '='");
1130+
assert!(err.to_string().contains("--annotation expects KEY=VALUE"));
1131+
1132+
let err =
1133+
parse_key_value_pairs(&["=value".to_string()], "--annotation").expect_err("empty key");
1134+
assert!(err.to_string().contains("--annotation key cannot be empty"));
1135+
}
1136+
11161137
#[test]
11171138
fn parse_duration_to_ms_parses_supported_units() {
11181139
assert_eq!(parse_duration_to_ms("30s").expect("parse"), 30_000);

‎crates/openshell-cli/src/main.rs‎

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use clap_complete::engine::ArgValueCompleter;
1010
use clap_complete::env::CompleteEnv;
1111
use miette::Result;
1212
use openshell_cli::color::{self, ColorChoice, Colorize};
13-
use std::collections::HashMap;
1413
use std::io::Write;
1514
use std::path::PathBuf;
1615

@@ -1590,6 +1589,11 @@ enum SandboxCommands {
15901589
#[arg(long = "label")]
15911590
labels: Vec<String>,
15921591

1592+
/// Attach non-secret annotations to the sandbox (key=value format,
1593+
/// repeatable). Values may contain `/`, spaces, and `=`.
1594+
#[arg(long = "annotation", value_name = "KEY=VALUE")]
1595+
annotations: Vec<String>,
1596+
15931597
/// Set a non-secret environment variable in the sandbox.
15941598
/// Do not use this option for API keys, tokens, or other secrets; create
15951599
/// a provider and attach it with `--provider` instead. Repeatable.
@@ -3378,6 +3382,7 @@ async fn run_async() -> Result<()> {
33783382
auto_providers,
33793383
no_auto_providers,
33803384
labels,
3385+
annotations,
33813386
envs,
33823387
env_from,
33833388
no_credential_warnings,
@@ -3403,18 +3408,9 @@ async fn run_async() -> Result<()> {
34033408
None // prompt or auto-detect
34043409
};
34053410

3406-
// Parse --label flags into a HashMap<String, String>.
3407-
let mut labels_map = HashMap::new();
3408-
for label_str in &labels {
3409-
let parts: Vec<&str> = label_str.splitn(2, '=').collect();
3410-
if parts.len() != 2 {
3411-
return Err(miette::miette!(
3412-
"invalid label format '{}', expected key=value",
3413-
label_str
3414-
));
3415-
}
3416-
labels_map.insert(parts[0].to_string(), parts[1].to_string());
3417-
}
3411+
// Parse --label and --annotation flags into maps.
3412+
let labels_map = run::parse_key_value_pairs(&labels, "--label")?;
3413+
let annotations_map = run::parse_key_value_pairs(&annotations, "--annotation")?;
34183414

34193415
// Parse --env flags into a HashMap<String, String>.
34203416
let mut env_map = run::parse_env_pairs(&envs)?;
@@ -3471,6 +3467,7 @@ async fn run_async() -> Result<()> {
34713467
tty_override,
34723468
auto_providers_override,
34733469
labels: labels_map,
3470+
annotations: annotations_map,
34743471
environment: env_map,
34753472
approval_mode: &approval_mode,
34763473
output: output.as_str(),
@@ -6208,6 +6205,33 @@ mod tests {
62086205
}
62096206
}
62106207

6208+
#[test]
6209+
fn sandbox_create_annotation_is_repeatable() {
6210+
let cli = Cli::try_parse_from([
6211+
"openshell",
6212+
"sandbox",
6213+
"create",
6214+
"--annotation",
6215+
"policy-source-file=/home/you/my policy.yaml",
6216+
"--annotation",
6217+
"owner=platform",
6218+
])
6219+
.unwrap();
6220+
match cli.command {
6221+
Some(Commands::Sandbox {
6222+
command: Some(SandboxCommands::Create { annotations, .. }),
6223+
..
6224+
}) => assert_eq!(
6225+
annotations,
6226+
vec![
6227+
"policy-source-file=/home/you/my policy.yaml".to_string(),
6228+
"owner=platform".to_string(),
6229+
]
6230+
),
6231+
other => panic!("expected SandboxCommands::Create, got: {other:?}"),
6232+
}
6233+
}
6234+
62116235
#[test]
62126236
fn sandbox_create_restart_policy_defaults_to_never() {
62136237
let cli = Cli::try_parse_from(["openshell", "sandbox", "create"]).unwrap();

‎crates/openshell-cli/src/run.rs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,7 @@ pub struct SandboxCreateConfig<'a> {
450450
pub tty_override: Option<bool>,
451451
pub auto_providers_override: Option<bool>,
452452
pub labels: HashMap<String, String>,
453+
pub annotations: HashMap<String, String>,
453454
pub environment: HashMap<String, String>,
454455
pub approval_mode: &'a str,
455456
pub output: &'a str,
@@ -480,6 +481,7 @@ impl Default for SandboxCreateConfig<'_> {
480481
tty_override: None,
481482
auto_providers_override: None,
482483
labels: HashMap::new(),
484+
annotations: HashMap::new(),
483485
environment: HashMap::new(),
484486
approval_mode: "manual",
485487
output: "table",
@@ -518,6 +520,7 @@ pub async fn sandbox_create(
518520
tty_override,
519521
auto_providers_override,
520522
labels,
523+
mut annotations,
521524
environment,
522525
approval_mode,
523526
output,
@@ -662,14 +665,12 @@ pub async fn sandbox_create(
662665
&& command.is_empty()
663666
&& (!std::io::stdin().is_terminal() || !std::io::stdout().is_terminal()));
664667
let await_main_process_attachment = output == "table" && editor.is_none() && !create_detaches;
665-
let annotations = if persist {
666-
HashMap::new()
667-
} else {
668-
HashMap::from([(
668+
if !persist {
669+
annotations.insert(
669670
"openshell.nvidia.com/retention".to_string(),
670671
"ephemeral".to_string(),
671-
)])
672-
};
672+
);
673+
}
673674
let request = CreateSandboxRequest {
674675
request_id: String::new(),
675676
spec: Some(SandboxSpec {

‎crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs‎

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2035,6 +2035,97 @@ async fn detached_ephemeral_command_delegates_cleanup_to_gateway() {
20352035
.await;
20362036
}
20372037

2038+
#[tokio::test]
2039+
async fn sandbox_create_sends_annotations() {
2040+
let server = run_server().await;
2041+
let fake_ssh_dir = helpers::tempdir().unwrap();
2042+
let xdg_dir = helpers::tempdir().unwrap();
2043+
2044+
test_env(&fake_ssh_dir, &xdg_dir)
2045+
.run_async(async {
2046+
let tls = test_tls(&server);
2047+
install_fake_ssh(&fake_ssh_dir);
2048+
2049+
let source = "/home/you/my policy.yaml=1".to_string();
2050+
run::sandbox_create(
2051+
&server.endpoint,
2052+
"openshell",
2053+
run::SandboxCreateConfig {
2054+
name: Some("annotated"),
2055+
keep: true,
2056+
command: &["echo".into(), "OK".into()],
2057+
annotations: HashMap::from([(
2058+
"policy-source-file".to_string(),
2059+
source.clone(),
2060+
)]),
2061+
..test_config()
2062+
},
2063+
"default",
2064+
&tls,
2065+
)
2066+
.await
2067+
.expect("sandbox create should succeed");
2068+
2069+
let requests = create_requests(&server).await;
2070+
assert_eq!(
2071+
requests[0]
2072+
.annotations
2073+
.get("policy-source-file")
2074+
.map(String::as_str),
2075+
Some(source.as_str())
2076+
);
2077+
assert!(
2078+
!requests[0]
2079+
.annotations
2080+
.contains_key("openshell.nvidia.com/retention"),
2081+
"a kept sandbox must not be marked ephemeral"
2082+
);
2083+
})
2084+
.await;
2085+
}
2086+
2087+
#[tokio::test]
2088+
async fn ephemeral_sandbox_create_keeps_annotations_and_retention() {
2089+
let server = run_server().await;
2090+
let fake_ssh_dir = helpers::tempdir().unwrap();
2091+
let xdg_dir = helpers::tempdir().unwrap();
2092+
2093+
test_env(&fake_ssh_dir, &xdg_dir)
2094+
.run_async(async {
2095+
let tls = test_tls(&server);
2096+
install_fake_ssh(&fake_ssh_dir);
2097+
2098+
run::sandbox_create(
2099+
&server.endpoint,
2100+
"openshell",
2101+
run::SandboxCreateConfig {
2102+
name: Some("annotated-ephemeral"),
2103+
keep: false,
2104+
command: &["echo".into(), "OK".into()],
2105+
annotations: HashMap::from([("owner".to_string(), "platform".to_string())]),
2106+
..test_config()
2107+
},
2108+
"default",
2109+
&tls,
2110+
)
2111+
.await
2112+
.expect("sandbox create should succeed");
2113+
2114+
let annotations = &create_requests(&server).await[0].annotations;
2115+
assert_eq!(
2116+
annotations.get("owner").map(String::as_str),
2117+
Some("platform")
2118+
);
2119+
assert_eq!(
2120+
annotations
2121+
.get("openshell.nvidia.com/retention")
2122+
.map(String::as_str),
2123+
Some("ephemeral")
2124+
);
2125+
})
2126+
.await;
2127+
}
2128+
20382129
#[tokio::test]
20392130
async fn sandbox_create_sends_driver_config_json() {
20402131
let server = run_server().await;
@@ -2992,7 +3083,7 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() {
29923083
let tls = test_tls(&server);
29933084
install_fake_forwarding_ssh(&fake_ssh_dir);
29943085
let (endpoint, tls) = (server.endpoint.as_str(), &tls);
2995-
let (forward_port, result) = with_free_forward_port(|spec| async move {
3086+
let (forward_port, result) = Box::pin(with_free_forward_port(|spec| async move {
29963087
run::sandbox_create(
29973088
endpoint,
29983089
"openshell",
@@ -3007,7 +3098,7 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() {
30073098
tls,
30083099
)
30093100
.await
3010-
})
3101+
}))
30113102
.await;
30123103
result.expect("sandbox create with forward should succeed");
30133104

‎docs/how-it-works/sandboxes/overview.mdx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,12 @@ openshell sandbox list --selector env=dev
437437
openshell sandbox list --selector env=dev,team=platform
438438
```
439439
440+
Use `--annotation KEY=VALUE` for non-secret metadata that does not fit label limits, such as a file path. Annotation values can contain `/`, spaces, and `=`, and appear in the `annotations` field of `sandbox get -o json` and `sandbox list -o json`.
441+
442+
```shell
443+
openshell sandbox create --annotation "policy-source-file=/home/you/policies/research policy.yaml" -- claude
444+
```
445+
440446
The Python SDK accepts the same gateway labels and selectors. Labels passed to
441447
`create` are stored on the gateway sandbox object and are returned on
442448
`SandboxRef.labels`, so selector-based listing finds Python-created sandboxes:

‎skills/openshell-cli/SKILL.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ Key flags:
305305
- `--driver-config-json`: Pass experimental driver-specific sandbox configuration
306306
- `--template NAME`: Create from a named sandbox workload template. Conflicts with inline workload flags such as `--from`, `--gpu`, `--cpu`, `--memory`, `--env`, `--env-from`, and `--driver-config-json`.
307307
- `--label KEY=VALUE`: Add labels for later selection (repeatable)
308+
- `--annotation KEY=VALUE`: Add non-secret metadata such as file paths (repeatable)
308309
- `--env KEY=VALUE`: Set non-secret sandbox environment variables (repeatable); use `--provider` for credentials
309310
- `--env-from KEY[=ENVVAR]`: Read a sandbox environment value from the CLI process environment without putting its value in arguments; use `--provider` to keep credentials hidden from the workload
310311
- `--tty`: Allocate a retained PTY for the canonical main process

0 commit comments

Comments
 (0)