Skip to content

Commit 48a7d09

Browse files
johntmyersjohntmyers
authored
feat(providers): support profile updates (#1914)
* feat(providers): support profile updates Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(providers): require CAS for profile updates Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(providers): serialize profile update validation Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(providers): serialize profile import invariants Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(providers): require profile update target id Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --------- Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
1 parent d64542f commit 48a7d09

20 files changed

Lines changed: 1151 additions & 37 deletions

‎architecture/gateway.md‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,8 @@ modes:
216216
mutation. If they diverge it returns `Conflict` without attempting the
217217
write. Client-facing operations that carry an `expected_resource_version`
218218
field use this mode: `AttachSandboxProvider`, `DetachSandboxProvider`,
219-
`UpdateProvider`, and `UpdateConfig` (policy backfill path).
219+
`UpdateProvider`, `UpdateProviderProfiles`, and `UpdateConfig` (policy
220+
backfill path).
220221

221222
**Lists.** The `list_messages` and `list_messages_with_selector` helpers decode
222223
protobuf payloads from list results and hydrate `resource_version` from the
@@ -235,7 +236,7 @@ coverage:
235236
|---|---|---|---|
236237
| Sandbox | `MustCreate` | `update_message_cas` | `list_messages` |
237238
| Provider | `MustCreate` | `update_message_cas` | `list_messages` |
238-
| ProviderProfile | `MustCreate` | (immutable) | `list_messages` |
239+
| ProviderProfile | `MustCreate` | `MatchResourceVersion` | `list_messages` |
239240
| InferenceRoute | `MustCreate` | `update_message_cas` | `list_messages` |
240241
| SandboxPolicy | scoped versioning | scoped versioning | scoped query |
241242
| Settings | `Mutex`-guarded | `Mutex`-guarded | single-row |
@@ -247,7 +248,20 @@ gateways, the Mutex alone would be insufficient. Sandbox-scoped settings
247248
rely entirely on CAS without a Mutex.
248249

249250
The `resource_version` is surfaced to clients through `ObjectMeta` in proto
250-
responses. Database migrations backfill existing rows with version 1.
251+
responses. Provider profiles are the exception: custom profile get/list/export
252+
responses copy the stored version onto the profile payload so exported YAML can
253+
carry the expected version for safe single-profile updates. Profile update
254+
requests also carry an explicit target profile ID; the payload ID must match the
255+
target so an edited export cannot overwrite a different profile. Database
256+
migrations backfill existing rows with version 1.
257+
258+
Provider profile imports, updates, and deletes hold the sandbox synchronization
259+
guard while checking attached-sandbox dynamic token grant ambiguity or in-use
260+
state and writing the profile record. Sandbox creation with initial providers and
261+
sandbox provider attach/detach use the same guard, so one gateway process cannot
262+
interleave a profile mutation with a sandbox provider-set mutation that would
263+
leave an ambiguous final dynamic-token state or a deleted custom profile that is
264+
still referenced by a sandbox.
251265

252266
Policy and runtime settings are delivered together through the effective sandbox
253267
config path. A gateway-global policy can override sandbox-scoped policy. The

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -930,6 +930,17 @@ enum ProviderProfileCommands {
930930
from: Option<PathBuf>,
931931
},
932932

933+
/// Update an existing custom provider profile from a file.
934+
#[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")]
935+
Update {
936+
/// Existing provider profile id to update.
937+
id: String,
938+
939+
/// Profile file to update.
940+
#[arg(short = 'f', long = "file", value_hint = ValueHint::FilePath)]
941+
file: PathBuf,
942+
},
943+
933944
/// Validate provider profile files without registering them.
934945
#[command(group = clap::ArgGroup::new("source").required(true).args(["file", "from"]), help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")]
935946
Lint {
@@ -2927,6 +2938,9 @@ async fn main() -> Result<()> {
29272938
)
29282939
.await?;
29292940
}
2941+
ProviderProfileCommands::Update { id, file } => {
2942+
run::provider_profile_update(endpoint, &id, &file, &tls).await?;
2943+
}
29302944
ProviderProfileCommands::Lint { file, from } => {
29312945
run::provider_profile_lint(
29322946
endpoint,
@@ -3765,6 +3779,26 @@ mod tests {
37653779
})
37663780
));
37673781

3782+
let update = Cli::try_parse_from([
3783+
"openshell",
3784+
"provider",
3785+
"profile",
3786+
"update",
3787+
"custom-api",
3788+
"-f",
3789+
"./profiles/custom-api.yaml",
3790+
])
3791+
.expect("provider profile update should parse");
3792+
assert!(matches!(
3793+
update.command,
3794+
Some(Commands::Provider {
3795+
command: Some(ProviderCommands::Profile(ProviderProfileCommands::Update {
3796+
id,
3797+
file: _
3798+
}))
3799+
}) if id == "custom-api"
3800+
));
3801+
37683802
let delete =
37693803
Cli::try_parse_from(["openshell", "provider", "profile", "delete", "custom-api"])
37703804
.expect("provider profile delete should parse");

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

Lines changed: 64 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ use openshell_core::proto::{
5151
RevokeSshSessionRequest, RotateProviderCredentialRequest, Sandbox, SandboxPhase, SandboxPolicy,
5252
SandboxSpec, SandboxTemplate, ServiceEndpointResponse, SetClusterInferenceRequest,
5353
SettingScope, SettingValue, TcpForwardFrame, TcpForwardInit, TcpRelayTarget,
54-
UpdateConfigRequest, UpdateProviderRequest, WatchSandboxRequest, exec_sandbox_event,
55-
setting_value, tcp_forward_init,
54+
UpdateConfigRequest, UpdateProviderProfilesRequest, UpdateProviderRequest, WatchSandboxRequest,
55+
exec_sandbox_event, setting_value, tcp_forward_init,
5656
};
5757
use openshell_core::settings::{self, SettingValueKind};
5858
use openshell_core::{ObjectId, ObjectName};
@@ -4955,6 +4955,21 @@ pub async fn provider_profile_export(
49554955
output: &str,
49564956
tls: &TlsOptions,
49574957
) -> Result<()> {
4958+
let rendered = provider_profile_export_text(server, id, output, tls).await?;
4959+
if output == "json" {
4960+
println!("{rendered}");
4961+
} else {
4962+
print!("{rendered}");
4963+
}
4964+
Ok(())
4965+
}
4966+
4967+
pub async fn provider_profile_export_text(
4968+
server: &str,
4969+
id: &str,
4970+
output: &str,
4971+
tls: &TlsOptions,
4972+
) -> Result<String> {
49584973
let mut client = grpc_client(server, tls).await?;
49594974
let response = client
49604975
.get_provider_profile(GetProviderProfileRequest { id: id.to_string() })
@@ -4966,16 +4981,14 @@ pub async fn provider_profile_export(
49664981
.ok_or_else(|| miette!("provider profile '{id}' not found"))?;
49674982
let profile = ProviderTypeProfile::from_proto(&profile);
49684983

4969-
if !crate::output::print_output_direct(
4970-
output,
4971-
|| profile_to_json(&profile).into_diagnostic(),
4972-
|| profile_to_yaml(&profile).into_diagnostic(),
4973-
)? {
4974-
return Err(miette!(
4984+
match output {
4985+
"json" => profile_to_json(&profile).into_diagnostic(),
4986+
"yaml" => profile_to_yaml(&profile).into_diagnostic(),
4987+
"table" => Err(miette!(
49754988
"profile export supports '-o yaml' and '-o json'; table output is not supported"
4976-
));
4989+
)),
4990+
_ => Err(miette!("unsupported output format: {output}")),
49774991
}
4978-
Ok(())
49794992
}
49804993

49814994
pub async fn provider_profile_import(
@@ -5019,6 +5032,47 @@ pub async fn provider_profile_import(
50195032
Err(miette!("provider profile import failed"))
50205033
}
50215034

5035+
pub async fn provider_profile_update(
5036+
server: &str,
5037+
id: &str,
5038+
file: &Path,
5039+
tls: &TlsOptions,
5040+
) -> Result<()> {
5041+
let (mut items, mut diagnostics) = load_profile_import_items(Some(file), None)?;
5042+
if items.is_empty() && diagnostics.is_empty() {
5043+
return Err(miette!("no provider profile files found"));
5044+
}
5045+
if profile_diagnostics_have_errors(&diagnostics) {
5046+
print_profile_diagnostics(&diagnostics);
5047+
return Err(miette!("provider profile update failed"));
5048+
}
5049+
5050+
let mut client = grpc_client(server, tls).await?;
5051+
if let Some(item) = items.pop() {
5052+
let expected_resource_version = item
5053+
.profile
5054+
.as_ref()
5055+
.map_or(0, |profile| profile.resource_version);
5056+
let response = client
5057+
.update_provider_profiles(UpdateProviderProfilesRequest {
5058+
profile: Some(item),
5059+
expected_resource_version,
5060+
id: id.to_string(),
5061+
})
5062+
.await
5063+
.into_diagnostic()?
5064+
.into_inner();
5065+
diagnostics.extend(response.diagnostics);
5066+
if response.updated {
5067+
println!("Updated provider profile.");
5068+
return Ok(());
5069+
}
5070+
}
5071+
5072+
print_profile_diagnostics(&diagnostics);
5073+
Err(miette!("provider profile update failed"))
5074+
}
5075+
50225076
pub async fn provider_profile_lint(
50235077
server: &str,
50245078
file: Option<&Path>,

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,13 @@ impl OpenShell for TestOpenShell {
276276
Err(Status::unimplemented("not implemented in test"))
277277
}
278278

279+
async fn update_provider_profiles(
280+
&self,
281+
_request: tonic::Request<openshell_core::proto::UpdateProviderProfilesRequest>,
282+
) -> Result<Response<openshell_core::proto::UpdateProviderProfilesResponse>, Status> {
283+
Err(Status::unimplemented("not implemented in test"))
284+
}
285+
279286
async fn lint_provider_profiles(
280287
&self,
281288
_request: tonic::Request<openshell_core::proto::LintProviderProfilesRequest>,

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,13 @@ impl OpenShell for TestOpenShell {
226226
Err(Status::unimplemented("not implemented in test"))
227227
}
228228

229+
async fn update_provider_profiles(
230+
&self,
231+
_request: tonic::Request<openshell_core::proto::UpdateProviderProfilesRequest>,
232+
) -> Result<Response<openshell_core::proto::UpdateProviderProfilesResponse>, Status> {
233+
Err(Status::unimplemented("not implemented in test"))
234+
}
235+
229236
async fn lint_provider_profiles(
230237
&self,
231238
_request: tonic::Request<openshell_core::proto::LintProviderProfilesRequest>,

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,10 @@ impl OpenShell for TestOpenShell {
455455
.profiles
456456
.into_iter()
457457
.filter_map(|item| item.profile)
458+
.map(|mut profile| {
459+
profile.resource_version = 1;
460+
profile
461+
})
458462
.inspect(|profile| {
459463
profiles.insert(profile.id.clone(), profile.clone());
460464
})
@@ -468,6 +472,72 @@ impl OpenShell for TestOpenShell {
468472
))
469473
}
470474

475+
async fn update_provider_profiles(
476+
&self,
477+
request: tonic::Request<openshell_core::proto::UpdateProviderProfilesRequest>,
478+
) -> Result<Response<openshell_core::proto::UpdateProviderProfilesResponse>, Status> {
479+
let mut profiles = self.state.profiles.lock().await;
480+
let request = request.into_inner();
481+
let mut profile = request
482+
.profile
483+
.and_then(|item| item.profile)
484+
.ok_or_else(|| Status::invalid_argument("provider profile is required"))?;
485+
let target_id = request.id;
486+
if target_id != profile.id {
487+
return Ok(Response::new(
488+
openshell_core::proto::UpdateProviderProfilesResponse {
489+
diagnostics: vec![openshell_core::proto::ProviderProfileDiagnostic {
490+
source: target_id.clone(),
491+
profile_id: profile.id.clone(),
492+
field: "id".to_string(),
493+
message: format!(
494+
"provider profile update target '{}' does not match payload id '{}'",
495+
target_id, profile.id
496+
),
497+
severity: "error".to_string(),
498+
}],
499+
profile: None,
500+
updated: false,
501+
},
502+
));
503+
}
504+
let Some(current) = profiles.get(&target_id) else {
505+
return Ok(Response::new(
506+
openshell_core::proto::UpdateProviderProfilesResponse {
507+
diagnostics: vec![openshell_core::proto::ProviderProfileDiagnostic {
508+
source: target_id.clone(),
509+
profile_id: target_id.clone(),
510+
field: "id".to_string(),
511+
message: format!("custom provider profile '{target_id}' does not exist"),
512+
severity: "error".to_string(),
513+
}],
514+
profile: None,
515+
updated: false,
516+
},
517+
));
518+
};
519+
let expected_resource_version = if request.expected_resource_version != 0 {
520+
request.expected_resource_version
521+
} else {
522+
profile.resource_version
523+
};
524+
if expected_resource_version == 0 || expected_resource_version != current.resource_version {
525+
return Err(Status::aborted(format!(
526+
"provider profile was modified concurrently (current resource_version: {})",
527+
current.resource_version
528+
)));
529+
}
530+
profile.resource_version = current.resource_version + 1;
531+
profiles.insert(profile.id.clone(), profile.clone());
532+
Ok(Response::new(
533+
openshell_core::proto::UpdateProviderProfilesResponse {
534+
diagnostics: Vec::new(),
535+
profile: Some(profile),
536+
updated: true,
537+
},
538+
))
539+
}
540+
471541
async fn lint_provider_profiles(
472542
&self,
473543
_request: tonic::Request<openshell_core::proto::LintProviderProfilesRequest>,
@@ -1366,6 +1436,31 @@ binaries: [/usr/bin/custom]
13661436
run::provider_profile_import(&ts.endpoint, Some(&profile_path), None, &ts.tls)
13671437
.await
13681438
.expect("profile import");
1439+
let exported_yaml =
1440+
run::provider_profile_export_text(&ts.endpoint, "custom-api", "yaml", &ts.tls)
1441+
.await
1442+
.expect("profile export text");
1443+
assert!(exported_yaml.contains("resource_version: 1"));
1444+
let updated_yaml = exported_yaml
1445+
.replace(
1446+
"display_name: Custom API",
1447+
"display_name: Custom API Updated",
1448+
)
1449+
.replace("host: api.custom.example", "host: api.updated.example");
1450+
std::fs::write(&profile_path, updated_yaml).unwrap();
1451+
run::provider_profile_update(&ts.endpoint, "custom-api", &profile_path, &ts.tls)
1452+
.await
1453+
.expect("profile update");
1454+
assert_eq!(
1455+
ts.state
1456+
.profiles
1457+
.lock()
1458+
.await
1459+
.get("custom-api")
1460+
.and_then(|profile| profile.endpoints.first())
1461+
.map(|endpoint| endpoint.host.as_str()),
1462+
Some("api.updated.example")
1463+
);
13691464
run::provider_profile_export(&ts.endpoint, "custom-api", "yaml", &ts.tls)
13701465
.await
13711466
.expect("profile export");

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,13 @@ impl OpenShell for TestOpenShell {
277277
Err(Status::unimplemented("not implemented in test"))
278278
}
279279

280+
async fn update_provider_profiles(
281+
&self,
282+
_request: tonic::Request<openshell_core::proto::UpdateProviderProfilesRequest>,
283+
) -> Result<Response<openshell_core::proto::UpdateProviderProfilesResponse>, Status> {
284+
Err(Status::unimplemented("not implemented in test"))
285+
}
286+
280287
async fn lint_provider_profiles(
281288
&self,
282289
_request: tonic::Request<openshell_core::proto::LintProviderProfilesRequest>,

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,13 @@ impl OpenShell for TestOpenShell {
262262
Err(Status::unimplemented("not implemented in test"))
263263
}
264264

265+
async fn update_provider_profiles(
266+
&self,
267+
_request: tonic::Request<openshell_core::proto::UpdateProviderProfilesRequest>,
268+
) -> Result<Response<openshell_core::proto::UpdateProviderProfilesResponse>, Status> {
269+
Err(Status::unimplemented("not implemented in test"))
270+
}
271+
265272
async fn lint_provider_profiles(
266273
&self,
267274
_request: tonic::Request<openshell_core::proto::LintProviderProfilesRequest>,

‎crates/openshell-providers/src/discovery.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ mod tests {
8383
fn profile() -> ProviderTypeProfile {
8484
ProviderTypeProfile {
8585
id: "custom".to_string(),
86+
resource_version: 0,
8687
display_name: "Custom".to_string(),
8788
description: String::new(),
8889
category: openshell_core::proto::ProviderProfileCategory::Other,

0 commit comments

Comments
 (0)