Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 43 additions & 8 deletions crates/openshell-cli/src/commands/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1717,6 +1717,9 @@ pub async fn provider_profile_import(

let mut client = grpc_client(server, tls).await?;
if !items.is_empty() {
if profiles_require_stable_placeholders(&items) {
lint_profile_items(&mut client, items.clone(), workspace).await?;
}
let response = client
.import_provider_profiles(ImportProviderProfilesRequest {
request_id: String::new(),
Expand Down Expand Up @@ -1763,6 +1766,9 @@ pub async fn provider_profile_update(

let mut client = grpc_client(server, tls).await?;
if let Some(item) = items.pop() {
if profiles_require_stable_placeholders(std::slice::from_ref(&item)) {
lint_profile_items(&mut client, vec![item.clone()], workspace).await?;
}
let expected_resource_version = item
.profile
.as_ref()
Expand Down Expand Up @@ -1804,14 +1810,7 @@ pub async fn provider_profile_lint(

if !items.is_empty() {
let mut client = grpc_client(server, tls).await?;
let response = client
.lint_provider_profiles(LintProviderProfilesRequest {
profiles: items,
workspace_scope: provider_profile_workspace_scope(workspace),
})
.await
.into_diagnostic()?
.into_inner();
let response = lint_profile_items(&mut client, items, workspace).await?;
diagnostics.extend(response.diagnostics);
}

Expand Down Expand Up @@ -2120,6 +2119,42 @@ fn provider_refresh_strategy_name(strategy: ProviderCredentialRefreshStrategy) -
}
}

fn profiles_require_stable_placeholders(items: &[ProviderProfileImportItem]) -> bool {
items
.iter()
.filter_map(|item| item.profile.as_ref())
.any(|profile| {
profile
.credentials
.iter()
.any(|credential| credential.stable_placeholder)
})
}

// Check support before persisting an opt-in: older gateways discard unknown
// credential fields and can otherwise report a successful revision-scoped import.
async fn lint_profile_items(
client: &mut crate::tls::GrpcClient,
items: Vec<ProviderProfileImportItem>,
workspace: &str,
) -> Result<openshell_core::proto::LintProviderProfilesResponse> {
let requires_stable = profiles_require_stable_placeholders(&items);
let response = client
.lint_provider_profiles(LintProviderProfilesRequest {
profiles: items,
workspace_scope: provider_profile_workspace_scope(workspace),
})
.await
.into_diagnostic()?
.into_inner();
if requires_stable && !response.supports_stable_placeholder {
return Err(miette!(
"gateway does not support stable_placeholder; upgrade the gateway before enabling it"
));
}
Ok(response)
}

fn load_profile_import_items(
file: Option<&Path>,
from: Option<&Path>,
Expand Down
72 changes: 72 additions & 0 deletions crates/openshell-cli/tests/provider_commands_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ fn selected_workspace(scope: &Option<WorkspaceSelector>) -> Option<&str> {
struct ProviderState {
providers: Arc<Mutex<HashMap<String, Provider>>>,
profiles: Arc<Mutex<HashMap<String, ProviderProfile>>>,
supports_stable_placeholder: Arc<AtomicBool>,
scoped_profiles: Arc<Mutex<HashMap<(String, String), ProviderProfile>>>,
refresh_statuses: Arc<Mutex<HashMap<(String, String), ProviderCredentialRefreshStatus>>>,
refresh_requests: Arc<Mutex<Vec<ProviderRefreshRequestLog>>>,
Expand Down Expand Up @@ -1024,6 +1025,10 @@ impl OpenShell for TestOpenShell {
openshell_core::proto::LintProviderProfilesResponse {
diagnostics: Vec::new(),
valid: true,
supports_stable_placeholder: self
.state
.supports_stable_placeholder
.load(Ordering::SeqCst),
},
))
}
Expand Down Expand Up @@ -4341,6 +4346,73 @@ async fn sandbox_provider_attach_cli_surfaces_server_errors() {
);
}

#[tokio::test]
async fn provider_profile_stable_placeholder_checks_support_before_writing() {
let ts = run_server().await;
let dir = tempfile::tempdir().expect("profile directory");
let path = dir.path().join("external.yaml");
let stable = r"
id: external
display_name: External
category: other
resource_version: 1
credentials:
- name: token
env_vars: [EXTERNAL_TOKEN]
stable_placeholder: true
endpoints:
- host: api.example.com
port: 443
";
std::fs::write(&path, stable).expect("stable profile");
let error =
run::provider_profile_import(&ts.endpoint, Some(&path), None, None, "default", &ts.tls)
.await
.expect_err("legacy gateway cannot import the opt-in");
assert!(
error
.to_string()
.contains("does not support stable_placeholder")
);
assert!(ts.state.profiles.lock().await.is_empty());
assert!(
run::provider_profile_lint(&ts.endpoint, Some(&path), None, None, "default", &ts.tls)
.await
.is_err()
);

// A legacy gateway still accepts ordinary profiles through the old path.
std::fs::write(
&path,
stable.replace("stable_placeholder: true", "stable_placeholder: false"),
)
.expect("ordinary profile");
run::provider_profile_import(&ts.endpoint, Some(&path), None, None, "default", &ts.tls)
.await
.expect("ordinary import");
let original = ts.state.profiles.lock().await["external"].clone();
std::fs::write(&path, stable).expect("enable opt-in");
assert!(
run::provider_profile_update(&ts.endpoint, "external", &path, "default", &ts.tls)
.await
.is_err()
);
assert_eq!(ts.state.profiles.lock().await["external"], original);

ts.state
.supports_stable_placeholder
.store(true, Ordering::SeqCst);
run::provider_profile_update(&ts.endpoint, "external", &path, "default", &ts.tls)
.await
.expect("supporting gateway update");
assert!(ts.state.profiles.lock().await["external"].credentials[0].stable_placeholder);
std::fs::write(&path, stable.replace("id: external", "id: second")).expect("second profile");
run::provider_profile_import(&ts.endpoint, Some(&path), None, None, "default", &ts.tls)
.await
.expect("supporting gateway import");
assert!(ts.state.profiles.lock().await["second"].credentials[0].stable_placeholder);
}

#[tokio::test]
async fn provider_profile_cli_run_functions_support_custom_profiles() {
let ts = run_server().await;
Expand Down
Loading
Loading