Skip to content

Commit 4b12157

Browse files
committed
fix(providers): restore unrelated files to main versions
The vertex-provider branch diverged from main on three unrelated PRs: - PR NVIDIA#1526: OCSF builder macro and shared driver helpers (reverted here to match main's macro-based approach) - PR NVIDIA#1547: Python SDK FileNotFoundError -> SandboxError translation (restores user-friendly error messages for missing gateway files) - PR NVIDIA#1539: bash 3.2-compatible read loop in helm-k3s-local.sh (restores mapfile -> while IFS= read for macOS compat)
1 parent af7910f commit 4b12157

20 files changed

Lines changed: 485 additions & 237 deletions

File tree

crates/openshell-cli/src/run.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4445,7 +4445,7 @@ pub async fn provider_create(
44454445
if let Err(configure_err) = client
44464446
.configure_provider_refresh(ConfigureProviderRefreshRequest {
44474447
provider: name.to_string(),
4448-
credential_key: "GOOGLE_VERTEX_AI_TOKEN".to_string(),
4448+
credential_key: openshell_core::inference::VERTEX_AI_ADC_TOKEN_KEY.to_string(),
44494449
strategy: ProviderCredentialRefreshStrategy::Oauth2RefreshToken as i32,
44504450
material,
44514451
secret_material_keys: vec![

crates/openshell-cli/tests/provider_commands_integration.rs

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2262,3 +2262,157 @@ async fn provider_create_from_existing_vertex_config_only_reports_missing_vertex
22622262
.contains_key("vertex-config-only")
22632263
);
22642264
}
2265+
2266+
#[tokio::test]
2267+
async fn provider_create_from_gcloud_adc_with_config_keys() {
2268+
let ts = run_server().await;
2269+
2270+
// Write a valid authorized_user ADC file.
2271+
let adc_content = serde_json::json!({
2272+
"type": "authorized_user",
2273+
"client_id": "test-client-id.apps.googleusercontent.com",
2274+
"client_secret": "test-client-secret",
2275+
"refresh_token": "1//test-refresh-token"
2276+
});
2277+
let adc_file = tempfile::NamedTempFile::new().unwrap();
2278+
serde_json::to_writer(&adc_file, &adc_content).unwrap();
2279+
let adc_path = adc_file.path().to_str().unwrap().to_string();
2280+
let _guard = EnvVarGuard::set(&[("GOOGLE_APPLICATION_CREDENTIALS", &adc_path)]);
2281+
2282+
run::provider_create(
2283+
&ts.endpoint,
2284+
"vertex-with-config",
2285+
"google-vertex-ai",
2286+
false,
2287+
&[], // no explicit credentials; ADC flow
2288+
true, // from_gcloud_adc
2289+
&[
2290+
"VERTEX_AI_PROJECT_ID=my-gcp-project".to_string(),
2291+
"VERTEX_AI_REGION=us-east1".to_string(),
2292+
],
2293+
&ts.tls,
2294+
)
2295+
.await
2296+
.expect("provider_create with --from-gcloud-adc and --config keys should succeed");
2297+
2298+
// Verify provider was created with the config keys.
2299+
let providers = ts.state.providers.lock().await;
2300+
let provider = providers
2301+
.get("vertex-with-config")
2302+
.expect("provider should be stored after create");
2303+
assert_eq!(provider.r#type, "google-vertex-ai");
2304+
assert_eq!(
2305+
provider
2306+
.config
2307+
.get("VERTEX_AI_PROJECT_ID")
2308+
.map(String::as_str),
2309+
Some("my-gcp-project"),
2310+
"VERTEX_AI_PROJECT_ID must be stored in provider config"
2311+
);
2312+
assert_eq!(
2313+
provider.config.get("VERTEX_AI_REGION").map(String::as_str),
2314+
Some("us-east1"),
2315+
"VERTEX_AI_REGION must be stored in provider config"
2316+
);
2317+
drop(providers);
2318+
2319+
// Verify configure_provider_refresh was called (ADC flow always configures refresh).
2320+
let refresh_requests = ts.state.refresh_requests.lock().await.clone();
2321+
assert_eq!(
2322+
refresh_requests.len(),
2323+
1,
2324+
"exactly one refresh configure call expected"
2325+
);
2326+
assert_eq!(
2327+
refresh_requests[0],
2328+
ProviderRefreshRequestLog::Configure {
2329+
provider_name: "vertex-with-config".to_string(),
2330+
credential_key: "GOOGLE_VERTEX_AI_TOKEN".to_string(),
2331+
expires_at_ms: None,
2332+
}
2333+
);
2334+
}
2335+
2336+
#[tokio::test]
2337+
async fn provider_create_from_gcloud_adc_missing_refresh_token() {
2338+
let ts = run_server().await;
2339+
2340+
// ADC file is valid authorized_user type but missing refresh_token.
2341+
let adc_content = serde_json::json!({
2342+
"type": "authorized_user",
2343+
"client_id": "test-client-id.apps.googleusercontent.com",
2344+
"client_secret": "test-client-secret"
2345+
});
2346+
let adc_file = tempfile::NamedTempFile::new().unwrap();
2347+
serde_json::to_writer(&adc_file, &adc_content).unwrap();
2348+
let adc_path = adc_file.path().to_str().unwrap().to_string();
2349+
let _guard = EnvVarGuard::set(&[("GOOGLE_APPLICATION_CREDENTIALS", &adc_path)]);
2350+
2351+
let err = run::provider_create(
2352+
&ts.endpoint,
2353+
"vertex-missing-refresh",
2354+
"google-vertex-ai",
2355+
false,
2356+
&[],
2357+
true,
2358+
&[],
2359+
&ts.tls,
2360+
)
2361+
.await
2362+
.expect_err("missing refresh_token should produce an error");
2363+
2364+
let err_msg = err.to_string();
2365+
assert!(
2366+
err_msg.contains("refresh_token"),
2367+
"error must mention 'refresh_token', got: {err_msg}"
2368+
);
2369+
2370+
// No provider should have been created.
2371+
let providers = ts.state.providers.lock().await;
2372+
assert!(
2373+
providers.is_empty(),
2374+
"no provider must be created when ADC validation fails"
2375+
);
2376+
}
2377+
2378+
#[tokio::test]
2379+
async fn provider_create_from_gcloud_adc_missing_client_secret() {
2380+
let ts = run_server().await;
2381+
2382+
// ADC file is valid authorized_user type but missing client_secret.
2383+
let adc_content = serde_json::json!({
2384+
"type": "authorized_user",
2385+
"client_id": "test-client-id.apps.googleusercontent.com",
2386+
"refresh_token": "1//test-refresh-token"
2387+
});
2388+
let adc_file = tempfile::NamedTempFile::new().unwrap();
2389+
serde_json::to_writer(&adc_file, &adc_content).unwrap();
2390+
let adc_path = adc_file.path().to_str().unwrap().to_string();
2391+
let _guard = EnvVarGuard::set(&[("GOOGLE_APPLICATION_CREDENTIALS", &adc_path)]);
2392+
2393+
let err = run::provider_create(
2394+
&ts.endpoint,
2395+
"vertex-missing-secret",
2396+
"google-vertex-ai",
2397+
false,
2398+
&[],
2399+
true,
2400+
&[],
2401+
&ts.tls,
2402+
)
2403+
.await
2404+
.expect_err("missing client_secret should produce an error");
2405+
2406+
let err_msg = err.to_string();
2407+
assert!(
2408+
err_msg.contains("client_secret"),
2409+
"error must mention 'client_secret', got: {err_msg}"
2410+
);
2411+
2412+
// No provider should have been created.
2413+
let providers = ts.state.providers.lock().await;
2414+
assert!(
2415+
providers.is_empty(),
2416+
"no provider must be created when ADC validation fails"
2417+
);
2418+
}

crates/openshell-core/src/driver_utils.rs

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33

44
//! Utility helpers shared across compute-driver crates.
55
6-
use crate::proto::compute::v1::DriverSandbox;
6+
use std::path::PathBuf;
7+
8+
use crate::proto::compute::v1::{DriverSandbox, GetCapabilitiesResponse};
79

810
// ---------------------------------------------------------------------------
911
// Sandbox container/pod label keys (openshell.ai/ namespace)
@@ -34,6 +36,50 @@ pub const LABEL_SANDBOX_NAMESPACE: &str = "openshell.ai/sandbox-namespace";
3436
/// path used when building the `openshell-sandbox` image layer.
3537
pub const SUPERVISOR_IMAGE_BINARY_PATH: &str = "/openshell-sandbox";
3638

39+
/// Return the XDG state path for a driver's sandbox JWT token file.
40+
///
41+
/// The resulting path is `$XDG_STATE_HOME/openshell/<driver_subdir>[/<namespace>]/<sandbox_id>/sandbox.jwt`.
42+
///
43+
/// `driver_subdir` is driver-specific, e.g. `"docker-sandbox-tokens"` or
44+
/// `"podman-sandbox-tokens"`. When `namespace` is `Some`, it is appended as
45+
/// an additional path component (with `/` and `\` replaced by `-`).
46+
///
47+
/// # Errors
48+
/// Returns an error if the XDG state directory cannot be resolved.
49+
pub fn sandbox_token_path(
50+
driver_subdir: &str,
51+
namespace: Option<&str>,
52+
sandbox_id: &str,
53+
) -> miette::Result<PathBuf> {
54+
let mut path = crate::paths::xdg_state_dir()?
55+
.join("openshell")
56+
.join(driver_subdir);
57+
if let Some(ns) = namespace {
58+
path = path.join(ns.replace(['/', '\\'], "-"));
59+
}
60+
Ok(path.join(sandbox_id).join("sandbox.jwt"))
61+
}
62+
63+
/// Build a [`GetCapabilitiesResponse`] from the common driver capability fields.
64+
///
65+
/// Every compute driver constructs this response with the same fields. Shared
66+
/// here to avoid repeating the struct literal (and the always-zero `gpu_count`
67+
/// default) in each driver crate.
68+
pub fn build_capabilities_response(
69+
driver_name: &str,
70+
driver_version: impl Into<String>,
71+
default_image: impl Into<String>,
72+
supports_gpu: bool,
73+
) -> GetCapabilitiesResponse {
74+
GetCapabilitiesResponse {
75+
driver_name: driver_name.to_string(),
76+
driver_version: driver_version.into(),
77+
default_image: default_image.into(),
78+
supports_gpu,
79+
gpu_count: 0,
80+
}
81+
}
82+
3783
/// Return the effective log level for a sandbox.
3884
///
3985
/// Uses the level from the sandbox spec when non-empty, falling back to

crates/openshell-core/src/inference.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,36 @@ pub const VERTEX_AI_CREDENTIAL_KEY_NAMES: &[&str] = &[
101101
"VERTEX_AI_TOKEN",
102102
];
103103

104+
/// The credential key used for tokens minted from gcloud Application Default Credentials.
105+
///
106+
/// This is the key written by the gateway's `OAuth2` refresh worker when using the
107+
/// `--from-gcloud-adc` CLI flow. It must match `VERTEX_AI_CREDENTIAL_KEY_NAMES[2]`.
108+
pub const VERTEX_AI_ADC_TOKEN_KEY: &str = "GOOGLE_VERTEX_AI_TOKEN";
109+
110+
/// GCP project ID config key for Vertex AI providers.
111+
pub const VERTEX_AI_PROJECT_ID_KEY: &str = "VERTEX_AI_PROJECT_ID";
112+
113+
/// GCP region/location config key for Vertex AI providers.
114+
pub const VERTEX_AI_REGION_KEY: &str = "VERTEX_AI_REGION";
115+
116+
/// Publisher override config key for Vertex AI providers.
117+
///
118+
/// Set to `"anthropic"` to force Anthropic Messages API routing regardless of model name,
119+
/// or any other value to force OpenAI-compatible routing.
120+
pub const VERTEX_AI_PUBLISHER_KEY: &str = "VERTEX_AI_PUBLISHER";
121+
122+
/// Config key names scanned during provider discovery, in addition to credential keys.
123+
///
124+
/// These are referenced by the provider discovery plugin in `openshell-providers` to
125+
/// collect Vertex AI config from the environment during `--from-existing` flows.
126+
pub const VERTEX_AI_CONFIG_KEY_NAMES: &[&str] = &[
127+
VERTEX_AI_PROJECT_ID_KEY,
128+
VERTEX_AI_REGION_KEY,
129+
"GOOGLE_VERTEX_AI_BASE_URL",
130+
"VERTEX_AI_BASE_URL",
131+
VERTEX_AI_PUBLISHER_KEY,
132+
];
133+
104134
static VERTEX_AI_PROFILE: InferenceProviderProfile = InferenceProviderProfile {
105135
provider_type: "google-vertex-ai",
106136
// Base URL is project/region specific and built at route resolution time.

crates/openshell-driver-docker/src/lib.rs

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -320,13 +320,12 @@ impl DockerComputeDriver {
320320
}
321321

322322
fn capabilities(&self) -> GetCapabilitiesResponse {
323-
GetCapabilitiesResponse {
324-
driver_name: "docker".to_string(),
325-
driver_version: self.config.daemon_version.clone(),
326-
default_image: self.config.default_image.clone(),
327-
supports_gpu: self.config.supports_gpu,
328-
gpu_count: 0,
329-
}
323+
openshell_core::driver_utils::build_capabilities_response(
324+
"docker",
325+
&self.config.daemon_version,
326+
&self.config.default_image,
327+
self.config.supports_gpu,
328+
)
330329
}
331330

332331
fn validate_sandbox(
@@ -962,17 +961,16 @@ fn sandbox_token_host_path_by_id(
962961
sandbox_id: &str,
963962
config: &DockerDriverRuntimeConfig,
964963
) -> Result<PathBuf, Status> {
965-
let base = openshell_core::paths::xdg_state_dir().map_err(|err| {
964+
openshell_core::driver_utils::sandbox_token_path(
965+
"docker-sandbox-tokens",
966+
Some(&config.sandbox_namespace),
967+
sandbox_id,
968+
)
969+
.map_err(|err| {
966970
Status::internal(format!(
967971
"resolve sandbox token state directory failed: {err}"
968972
))
969-
})?;
970-
Ok(base
971-
.join("openshell")
972-
.join("docker-sandbox-tokens")
973-
.join(config.sandbox_namespace.replace(['/', '\\'], "-"))
974-
.join(sandbox_id)
975-
.join("sandbox.jwt"))
973+
})
976974
}
977975

978976
async fn write_sandbox_token_file(

crates/openshell-driver-kubernetes/src/driver.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -160,13 +160,12 @@ impl KubernetesComputeDriver {
160160
}
161161

162162
pub async fn capabilities(&self) -> Result<GetCapabilitiesResponse, String> {
163-
Ok(GetCapabilitiesResponse {
164-
driver_name: "kubernetes".to_string(),
165-
driver_version: openshell_core::VERSION.to_string(),
166-
default_image: self.config.default_image.clone(),
167-
supports_gpu: self.has_gpu_capacity().await.unwrap_or(false),
168-
gpu_count: 0,
169-
})
163+
Ok(openshell_core::driver_utils::build_capabilities_response(
164+
"kubernetes",
165+
openshell_core::VERSION,
166+
&self.config.default_image,
167+
self.has_gpu_capacity().await.unwrap_or(false),
168+
))
170169
}
171170

172171
pub fn default_image(&self) -> &str {

crates/openshell-driver-podman/src/driver.rs

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,8 @@ fn validated_container_name(sandbox_name: &str) -> Result<String, ComputeDriverE
5757
}
5858

5959
fn sandbox_token_host_path(sandbox_id: &str) -> Result<PathBuf, ComputeDriverError> {
60-
let base = openshell_core::paths::xdg_state_dir()
61-
.map_err(|err| ComputeDriverError::Message(format!("resolve state dir failed: {err}")))?;
62-
Ok(base
63-
.join("openshell")
64-
.join("podman-sandbox-tokens")
65-
.join(sandbox_id)
66-
.join("sandbox.jwt"))
60+
openshell_core::driver_utils::sandbox_token_path("podman-sandbox-tokens", None, sandbox_id)
61+
.map_err(|err| ComputeDriverError::Message(format!("resolve state dir failed: {err}")))
6762
}
6863

6964
async fn write_sandbox_token_file(
@@ -257,14 +252,12 @@ impl PodmanComputeDriver {
257252

258253
/// Report driver capabilities.
259254
pub fn capabilities(&self) -> Result<GetCapabilitiesResponse, ComputeDriverError> {
260-
let supports_gpu = Self::has_gpu_capacity();
261-
Ok(GetCapabilitiesResponse {
262-
driver_name: "podman".to_string(),
263-
driver_version: openshell_core::VERSION.to_string(),
264-
default_image: self.config.default_image.clone(),
265-
supports_gpu,
266-
gpu_count: 0,
267-
})
255+
Ok(openshell_core::driver_utils::build_capabilities_response(
256+
"podman",
257+
openshell_core::VERSION,
258+
&self.config.default_image,
259+
Self::has_gpu_capacity(),
260+
))
268261
}
269262

270263
#[must_use]

0 commit comments

Comments
 (0)