Skip to content

Commit 0b1f6f8

Browse files
authored
feat(providers): add CLI label creation and filtering (#4265)
* docs: document workspace and provider labels (fixes #4250) Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * feat(providers): support CLI labels and selector filtering Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(providers): update schema inventory and scoped test environment Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --------- Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
1 parent fce4e5a commit 0b1f6f8

13 files changed

Lines changed: 493 additions & 21 deletions

File tree

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,7 @@ pub async fn ensure_required_providers(
382382
loop {
383383
let response = client
384384
.list_providers(ListProvidersRequest {
385+
label_selector: String::new(),
385386
page_size: 100,
386387
page_token,
387388
workspace_scope: Some(openshell_core::proto::workspace_selector(workspace)),
@@ -1046,6 +1047,7 @@ pub async fn provider_create(
10461047
server,
10471048
name,
10481049
provider_type,
1050+
labels: &[],
10491051
credentials,
10501052
credential_source,
10511053
config,
@@ -1060,6 +1062,7 @@ pub struct ProviderCreateOptions<'a> {
10601062
pub server: &'a str,
10611063
pub name: &'a str,
10621064
pub provider_type: &'a str,
1065+
pub labels: &'a [String],
10631066
pub credentials: &'a [String],
10641067
pub credential_source: ProviderCreateCredentialSource,
10651068
pub config: &'a [String],
@@ -1082,6 +1085,7 @@ pub async fn provider_create_with_options(options: ProviderCreateOptions<'_>) ->
10821085
server,
10831086
name,
10841087
provider_type,
1088+
labels,
10851089
credentials,
10861090
credential_source,
10871091
config,
@@ -1090,6 +1094,8 @@ pub async fn provider_create_with_options(options: ProviderCreateOptions<'_>) ->
10901094
tls,
10911095
} = options;
10921096

1097+
let labels = parse_key_value_pairs(labels, "--label")?;
1098+
10931099
let from_existing = credential_source == ProviderCreateCredentialSource::Existing;
10941100
let from_gcloud_adc = credential_source == ProviderCreateCredentialSource::GcloudAdc;
10951101
let from_oidc_token = credential_source == ProviderCreateCredentialSource::OidcToken;
@@ -1209,7 +1215,7 @@ pub async fn provider_create_with_options(options: ProviderCreateOptions<'_>) ->
12091215
id: String::new(),
12101216
name: name.to_string(),
12111217
created_time: None,
1212-
labels: HashMap::new(),
1218+
labels,
12131219
resource_version: 0,
12141220
annotations: HashMap::new(),
12151221
workspace: workspace.to_string(),
@@ -1346,6 +1352,21 @@ pub async fn provider_get(
13461352
println!(" {} {}", "Id:".dimmed(), provider.object_id());
13471353
println!(" {} {}", "Name:".dimmed(), provider.object_name());
13481354
println!(" {} {}", "Type:".dimmed(), provider.r#type);
1355+
if let Some(meta) = &provider.metadata
1356+
&& !meta.labels.is_empty()
1357+
{
1358+
let mut labels: Vec<_> = meta.labels.iter().collect();
1359+
labels.sort_unstable_by_key(|(key, _)| *key);
1360+
println!(
1361+
" {} {}",
1362+
"Labels:".dimmed(),
1363+
labels
1364+
.into_iter()
1365+
.map(|(key, value)| format!("{key}={value}"))
1366+
.collect::<Vec<_>>()
1367+
.join(", ")
1368+
);
1369+
}
13491370
println!(
13501371
" {} {}",
13511372
"Resource version:".dimmed(),
@@ -1455,6 +1476,7 @@ pub async fn provider_list(
14551476
server: &str,
14561477
page_size: i32,
14571478
page_token: &str,
1479+
label_selector: &str,
14581480
names_only: bool,
14591481
output: &str,
14601482
workspace: &str,
@@ -1466,6 +1488,7 @@ pub async fn provider_list(
14661488
.list_providers(ListProvidersRequest {
14671489
page_size,
14681490
page_token: page_token.to_string(),
1491+
label_selector: label_selector.to_string(),
14691492
workspace_scope: Some(if all_workspaces {
14701493
openshell_core::proto::all_workspaces_selector()
14711494
} else {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ pub fn complete_provider_names(_prefix: &OsStr) -> Vec<CompletionCandidate> {
6363
let mut client = completion_grpc_client(&endpoint, &gateway_name).await?;
6464
let response = client
6565
.list_providers(ListProvidersRequest {
66+
label_selector: String::new(),
6667
page_size: 200,
6768
page_token: String::new(),
6869
workspace_scope: Some(openshell_core::proto::workspace_selector(

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -860,6 +860,10 @@ enum ProviderCommands {
860860
#[arg(long = "type")]
861861
provider_type: String,
862862

863+
/// Labels to attach to the provider (KEY=VALUE, repeatable).
864+
#[arg(long = "label", value_name = "KEY=VALUE")]
865+
labels: Vec<String>,
866+
863867
/// Load provider credentials/config from existing local state.
864868
#[arg(long, conflicts_with_all = ["credentials", "from_gcloud_adc", "runtime_credentials", "from_oidc_token"])]
865869
from_existing: bool,
@@ -919,6 +923,10 @@ enum ProviderCommands {
919923
#[arg(long, default_value = "")]
920924
page_token: String,
921925

926+
/// Filter providers by labels (e.g. env=dev,team=ml).
927+
#[arg(long)]
928+
label_selector: Option<String>,
929+
922930
/// Print only provider names, one per line.
923931
#[arg(long, conflicts_with = "output")]
924932
names: bool,
@@ -3876,6 +3884,7 @@ async fn run_async() -> Result<()> {
38763884
ProviderCommands::Create {
38773885
name,
38783886
provider_type,
3887+
labels,
38793888
from_existing,
38803889
credentials,
38813890
from_gcloud_adc,
@@ -3900,6 +3909,7 @@ async fn run_async() -> Result<()> {
39003909
server: endpoint,
39013910
name: &name,
39023911
provider_type: provider_type.as_str(),
3912+
labels: &labels,
39033913
credentials: &credentials,
39043914
credential_source,
39053915
config: &config,
@@ -3995,6 +4005,7 @@ async fn run_async() -> Result<()> {
39954005
ProviderCommands::List {
39964006
page_size,
39974007
page_token,
4008+
label_selector,
39984009
names,
39994010
output,
40004011
all_workspaces,
@@ -4003,6 +4014,7 @@ async fn run_async() -> Result<()> {
40034014
endpoint,
40044015
page_size,
40054016
&page_token,
4017+
label_selector.as_deref().unwrap_or_default(),
40064018
names,
40074019
output.as_str(),
40084020
&cli.workspace,
@@ -5653,6 +5665,75 @@ mod tests {
56535665
}
56545666
}
56555667

5668+
#[test]
5669+
fn provider_create_accepts_labels_with_each_credential_source() {
5670+
for source in [
5671+
vec![],
5672+
vec!["--from-existing"],
5673+
vec!["--credential", "GITHUB_TOKEN=token"],
5674+
vec!["--from-gcloud-adc"],
5675+
vec!["--from-oidc-token"],
5676+
vec!["--runtime-credentials"],
5677+
] {
5678+
let mut args = vec![
5679+
"openshell",
5680+
"provider",
5681+
"create",
5682+
"--name",
5683+
"labeled",
5684+
"--type",
5685+
"github",
5686+
"--label",
5687+
"env=dev",
5688+
"--label",
5689+
"team=ml",
5690+
];
5691+
args.extend(source);
5692+
let cli = Cli::try_parse_from(args).unwrap();
5693+
match cli.command {
5694+
Some(Commands::Provider {
5695+
command: Some(ProviderCommands::Create { labels, .. }),
5696+
}) => assert_eq!(labels, ["env=dev", "team=ml"]),
5697+
other => panic!("expected provider create, got {other:?}"),
5698+
}
5699+
}
5700+
}
5701+
5702+
#[test]
5703+
fn provider_list_accepts_label_selector_in_all_output_modes() {
5704+
for output in [
5705+
vec![],
5706+
vec!["--names"],
5707+
vec!["-o", "json"],
5708+
vec!["-o", "yaml"],
5709+
] {
5710+
let mut args = vec![
5711+
"openshell",
5712+
"provider",
5713+
"list",
5714+
"--all-workspaces",
5715+
"--label-selector",
5716+
"env=dev,team=ml",
5717+
];
5718+
args.extend(output);
5719+
let cli = Cli::try_parse_from(args).unwrap();
5720+
match cli.command {
5721+
Some(Commands::Provider {
5722+
command:
5723+
Some(ProviderCommands::List {
5724+
label_selector,
5725+
all_workspaces,
5726+
..
5727+
}),
5728+
}) => {
5729+
assert_eq!(label_selector.as_deref(), Some("env=dev,team=ml"));
5730+
assert!(all_workspaces);
5731+
}
5732+
other => panic!("expected provider list, got {other:?}"),
5733+
}
5734+
}
5735+
}
5736+
56565737
#[test]
56575738
fn provider_create_accepts_no_credential_source() {
56585739
let cli = Cli::try_parse_from([

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

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ use tonic::{Response, Status};
4242
#[derive(Clone, Default)]
4343
struct ProviderState {
4444
providers: Arc<Mutex<HashMap<String, Provider>>>,
45+
list_requests: Arc<Mutex<Vec<ListProvidersRequest>>>,
4546
}
4647

4748
#[derive(Clone, Default)]
@@ -364,8 +365,13 @@ impl OpenShell for TestOpenShell {
364365

365366
async fn list_providers(
366367
&self,
367-
_request: tonic::Request<ListProvidersRequest>,
368+
request: tonic::Request<ListProvidersRequest>,
368369
) -> Result<Response<ListProvidersResponse>, Status> {
370+
self.state
371+
.list_requests
372+
.lock()
373+
.await
374+
.push(request.into_inner());
369375
let providers = self
370376
.state
371377
.providers
@@ -867,6 +873,108 @@ async fn run_server() -> TestServer {
867873

868874
// ── tests ────────────────────────────────────────────────────────────
869875

876+
#[tokio::test]
877+
async fn provider_create_forwards_labels_for_explicit_and_discovered_credentials() {
878+
let ts = run_server().await;
879+
Environment::from_pairs(&[("NVIDIA_API_KEY", "nvapi-test-key")])
880+
.run_async(async {
881+
let labels = vec!["env=dev".to_string(), "team=ml".to_string()];
882+
let credentials = vec!["NVIDIA_API_KEY=test-explicit".to_string()];
883+
for (name, source, credentials) in [
884+
(
885+
"explicit",
886+
run::ProviderCreateCredentialSource::ExplicitCredentials,
887+
credentials.as_slice(),
888+
),
889+
(
890+
"discovered",
891+
run::ProviderCreateCredentialSource::Existing,
892+
&[],
893+
),
894+
] {
895+
run::provider_create_with_options(run::ProviderCreateOptions {
896+
server: &ts.endpoint,
897+
name,
898+
provider_type: "nvidia",
899+
labels: &labels,
900+
credentials,
901+
credential_source: source,
902+
config: &[],
903+
workspace: "default",
904+
profile_workspace: "",
905+
tls: &ts.tls,
906+
})
907+
.await
908+
.unwrap();
909+
let providers = ts.openshell.state.providers.lock().await;
910+
let provider = &providers[name];
911+
assert_eq!(
912+
provider.metadata.as_ref().unwrap().labels,
913+
HashMap::from([("env".into(), "dev".into()), ("team".into(), "ml".into())])
914+
);
915+
assert!(provider.credentials.contains_key("NVIDIA_API_KEY"));
916+
}
917+
918+
for label in ["no-equals-sign", "=no-key"] {
919+
let err = run::provider_create_with_options(run::ProviderCreateOptions {
920+
server: &ts.endpoint,
921+
name: "invalid",
922+
provider_type: "nvidia",
923+
labels: &[label.into()],
924+
credentials: &credentials,
925+
credential_source: run::ProviderCreateCredentialSource::ExplicitCredentials,
926+
config: &[],
927+
workspace: "default",
928+
profile_workspace: "",
929+
tls: &ts.tls,
930+
})
931+
.await
932+
.unwrap_err();
933+
assert!(err.to_string().contains("--label"));
934+
}
935+
assert!(
936+
!ts.openshell
937+
.state
938+
.providers
939+
.lock()
940+
.await
941+
.contains_key("invalid")
942+
);
943+
})
944+
.await;
945+
}
946+
947+
#[tokio::test]
948+
async fn provider_list_forwards_selector_pagination_and_workspace_scope() {
949+
let ts = run_server().await;
950+
for all_workspaces in [false, true] {
951+
run::provider_list(
952+
&ts.endpoint,
953+
3,
954+
"cursor",
955+
"env=dev,team=ml",
956+
false,
957+
"json",
958+
"team-ml",
959+
all_workspaces,
960+
&ts.tls,
961+
)
962+
.await
963+
.unwrap();
964+
let requests = ts.openshell.state.list_requests.lock().await;
965+
let request = requests.last().unwrap();
966+
assert_eq!(request.page_size, 3);
967+
assert_eq!(request.page_token, "cursor");
968+
assert_eq!(request.label_selector, "env=dev,team=ml");
969+
let expected = if all_workspaces {
970+
openshell_core::proto::all_workspaces_selector()
971+
} else {
972+
openshell_core::proto::workspace_selector("team-ml")
973+
};
974+
assert_eq!(request.workspace_scope.as_ref(), Some(&expected));
975+
}
976+
}
977+
870978
/// When `--provider nvidia` is passed and a provider named "nvidia" already
871979
/// exists, `ensure_required_providers` should return it directly without
872980
/// creating anything new.

0 commit comments

Comments
 (0)