Skip to content

Commit 42d8f46

Browse files
committed
refactor: deduplicate shared driver and provider constants
Move default_sandbox_image() and SUPERVISOR_IMAGE_BINARY_PATH into openshell-core so all compute drivers share a single authoritative definition. Eliminates four identical copies of default_sandbox_image() and two identical copies of SUPERVISOR_IMAGE_BINARY_PATH. Implement ProviderPlugin for ProviderDiscoverySpec to remove the repeated three-method delegation boilerplate from eight simple provider modules. Each now declares only a SPEC constant; the blanket impl handles id(), discover_existing(), and credential_env_vars().
1 parent cade0bb commit 42d8f46

16 files changed

Lines changed: 57 additions & 203 deletions

File tree

crates/openshell-core/src/driver_utils.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@
55
66
use crate::proto::compute::v1::DriverSandbox;
77

8+
/// Path to the sandbox supervisor binary inside the container image.
9+
///
10+
/// All compute drivers must launch this binary as the container entrypoint to
11+
/// start the sandboxed environment. The value must be kept in sync with the
12+
/// path used when building the `openshell-sandbox` image layer.
13+
pub const SUPERVISOR_IMAGE_BINARY_PATH: &str = "/openshell-sandbox";
14+
815
/// Return the effective log level for a sandbox.
916
///
1017
/// Uses the level from the sandbox spec when non-empty, falling back to

crates/openshell-core/src/image.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,15 @@
1313
/// Override at runtime with the `OPENSHELL_COMMUNITY_REGISTRY` env var.
1414
pub const DEFAULT_COMMUNITY_REGISTRY: &str = "ghcr.io/nvidia/openshell-community/sandboxes";
1515

16+
/// Return the default sandbox image reference (`{registry}/base:latest`).
17+
///
18+
/// Used by all compute drivers as the fallback image when none is specified in
19+
/// the sandbox spec.
20+
#[must_use]
21+
pub fn default_sandbox_image() -> String {
22+
format!("{DEFAULT_COMMUNITY_REGISTRY}/base:latest")
23+
}
24+
1625
/// Resolve a user-supplied image string into a fully-qualified reference.
1726
///
1827
/// Resolution rules (applied in order):

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

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use bollard::query_parameters::{
1919
use bytes::Bytes;
2020
use futures::{Stream, StreamExt};
2121
use openshell_core::config::{DEFAULT_DOCKER_NETWORK_NAME, DEFAULT_STOP_TIMEOUT_SECS};
22+
use openshell_core::driver_utils::SUPERVISOR_IMAGE_BINARY_PATH;
2223
use openshell_core::gpu::cdi_gpu_device_ids;
2324
use openshell_core::proto::compute::v1::{
2425
CreateSandboxRequest, CreateSandboxResponse, DeleteSandboxRequest, DeleteSandboxResponse,
@@ -68,10 +69,6 @@ const DOCKER_NETWORK_DRIVER: &str = "bridge";
6869
/// explicit `supervisor_bin` override or local build is available.
6970
const DEFAULT_DOCKER_SUPERVISOR_IMAGE_REPO: &str = "ghcr.io/nvidia/openshell/supervisor";
7071

71-
/// Path to the supervisor binary inside the `openshell/supervisor` image
72-
/// (a `FROM scratch` image containing only the binary).
73-
const SUPERVISOR_IMAGE_BINARY_PATH: &str = "/openshell-sandbox";
74-
7572
/// Return the default `ghcr.io/nvidia/openshell/supervisor:<tag>` reference
7673
/// used when no supervisor binary override is provided.
7774
pub fn default_docker_supervisor_image() -> String {
@@ -173,7 +170,7 @@ pub struct DockerComputeConfig {
173170
impl Default for DockerComputeConfig {
174171
fn default() -> Self {
175172
Self {
176-
default_image: default_sandbox_image(),
173+
default_image: openshell_core::image::default_sandbox_image(),
177174
image_pull_policy: String::new(),
178175
sandbox_namespace: "default".to_string(),
179176
grpc_endpoint: String::new(),
@@ -189,13 +186,6 @@ impl Default for DockerComputeConfig {
189186
}
190187
}
191188

192-
fn default_sandbox_image() -> String {
193-
format!(
194-
"{}/base:latest",
195-
openshell_core::image::DEFAULT_COMMUNITY_REGISTRY
196-
)
197-
}
198-
199189
#[derive(Debug, Clone, PartialEq, Eq)]
200190
pub(crate) struct DockerGuestTlsPaths {
201191
pub(crate) ca: PathBuf,

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ impl Default for KubernetesComputeConfig {
7474
fn default() -> Self {
7575
Self {
7676
namespace: DEFAULT_K8S_NAMESPACE.to_string(),
77-
default_image: default_sandbox_image(),
77+
default_image: openshell_core::image::default_sandbox_image(),
7878
// Default empty so the gateway omits `imagePullPolicy` from pod
7979
// specs and Kubernetes applies its own default (Always for `latest`,
8080
// IfNotPresent otherwise). `DEFAULT_IMAGE_PULL_POLICY` ("missing")
@@ -93,13 +93,6 @@ impl Default for KubernetesComputeConfig {
9393
}
9494
}
9595

96-
fn default_sandbox_image() -> String {
97-
format!(
98-
"{}/base:latest",
99-
openshell_core::image::DEFAULT_COMMUNITY_REGISTRY
100-
)
101-
}
102-
10396
#[cfg(test)]
10497
mod tests {
10598
use super::*;

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use kube::core::gvk::GroupVersionKind;
1313
use kube::core::{DynamicObject, ObjectMeta};
1414
use kube::runtime::watcher::{self, Event};
1515
use kube::{Client, Error as KubeError};
16+
use openshell_core::driver_utils::SUPERVISOR_IMAGE_BINARY_PATH;
1617
use openshell_core::progress::{
1718
PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, PROGRESS_STEP_STARTING_SANDBOX,
1819
mark_progress_active, mark_progress_complete, mark_progress_detail,
@@ -767,13 +768,6 @@ fn supervisor_volume_mount() -> serde_json::Value {
767768
})
768769
}
769770

770-
/// Path of the supervisor binary inside the supervisor image.
771-
///
772-
/// The supervisor image places the binary at the filesystem root. We invoke
773-
/// it directly so the init path does not depend on shell utilities or PATH
774-
/// resolution inside the image.
775-
const SUPERVISOR_IMAGE_BINARY_PATH: &str = "/openshell-sandbox";
776-
777771
/// Build an image volume that mounts the supervisor OCI image directly.
778772
///
779773
/// Requires Kubernetes >= v1.33 (`ImageVolume` beta) or >= v1.36 (GA).

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ impl Default for PodmanComputeConfig {
180180
fn default() -> Self {
181181
Self {
182182
socket_path: Self::default_socket_path(),
183-
default_image: default_sandbox_image(),
183+
default_image: openshell_core::image::default_sandbox_image(),
184184
image_pull_policy: ImagePullPolicy::default(),
185185
grpc_endpoint: String::new(),
186186
gateway_port: openshell_core::config::DEFAULT_SERVER_PORT,
@@ -195,13 +195,6 @@ impl Default for PodmanComputeConfig {
195195
}
196196
}
197197

198-
fn default_sandbox_image() -> String {
199-
format!(
200-
"{}/base:latest",
201-
openshell_core::image::DEFAULT_COMMUNITY_REGISTRY
202-
)
203-
}
204-
205198
impl std::fmt::Debug for PodmanComputeConfig {
206199
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
207200
f.debug_struct("PodmanComputeConfig")

crates/openshell-providers/src/lib.rs

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,25 @@ pub trait ProviderPlugin: Send + Sync {
7373
}
7474
}
7575

76+
/// Blanket implementation of [`ProviderPlugin`] for [`ProviderDiscoverySpec`].
77+
///
78+
/// Providers that only need standard env-var discovery can register their
79+
/// `SPEC` constant directly, instead of defining a dedicated struct and
80+
/// repeating the same three-method delegation.
81+
impl ProviderPlugin for ProviderDiscoverySpec {
82+
fn id(&self) -> &'static str {
83+
self.id
84+
}
85+
86+
fn discover_existing(&self) -> Result<Option<DiscoveredProvider>, ProviderError> {
87+
discover_with_spec(self, &RealDiscoveryContext)
88+
}
89+
90+
fn credential_env_vars(&self) -> &'static [&'static str] {
91+
self.credential_env_vars
92+
}
93+
}
94+
7695
#[derive(Default)]
7796
pub struct ProviderRegistry {
7897
plugins: HashMap<&'static str, Box<dyn ProviderPlugin>>,
@@ -82,16 +101,16 @@ impl ProviderRegistry {
82101
#[must_use]
83102
pub fn new() -> Self {
84103
let mut registry = Self::default();
85-
registry.register(providers::claude::ClaudeProvider);
86-
registry.register(providers::codex::CodexProvider);
87-
registry.register(providers::copilot::CopilotProvider);
104+
registry.register(providers::claude::SPEC);
105+
registry.register(providers::codex::SPEC);
106+
registry.register(providers::copilot::SPEC);
88107
registry.register(providers::opencode::OpencodeProvider);
89108
registry.register(providers::generic::GenericProvider);
90-
registry.register(providers::openai::OpenaiProvider);
91-
registry.register(providers::anthropic::AnthropicProvider);
92-
registry.register(providers::nvidia::NvidiaProvider);
93-
registry.register(providers::gitlab::GitlabProvider);
94-
registry.register(providers::github::GithubProvider);
109+
registry.register(providers::openai::SPEC);
110+
registry.register(providers::anthropic::SPEC);
111+
registry.register(providers::nvidia::SPEC);
112+
registry.register(providers::gitlab::SPEC);
113+
registry.register(providers::github::SPEC);
95114
registry.register(providers::outlook::OutlookProvider);
96115
registry
97116
}

crates/openshell-providers/src/providers/anthropic.rs

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,13 @@
11
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
use crate::{
5-
ProviderDiscoverySpec, ProviderError, ProviderPlugin, RealDiscoveryContext, discover_with_spec,
6-
};
7-
8-
pub struct AnthropicProvider;
4+
use crate::ProviderDiscoverySpec;
95

106
pub const SPEC: ProviderDiscoverySpec = ProviderDiscoverySpec {
117
id: "anthropic",
128
credential_env_vars: &["ANTHROPIC_API_KEY"],
139
};
1410

15-
impl ProviderPlugin for AnthropicProvider {
16-
fn id(&self) -> &'static str {
17-
SPEC.id
18-
}
19-
20-
fn discover_existing(&self) -> Result<Option<crate::DiscoveredProvider>, ProviderError> {
21-
discover_with_spec(&SPEC, &RealDiscoveryContext)
22-
}
23-
24-
fn credential_env_vars(&self) -> &'static [&'static str] {
25-
SPEC.credential_env_vars
26-
}
27-
}
28-
2911
#[cfg(test)]
3012
mod tests {
3113
use super::SPEC;

crates/openshell-providers/src/providers/claude.rs

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,13 @@
11
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
use crate::{
5-
ProviderDiscoverySpec, ProviderError, ProviderPlugin, RealDiscoveryContext, discover_with_spec,
6-
};
7-
8-
pub struct ClaudeProvider;
4+
use crate::ProviderDiscoverySpec;
95

106
pub const SPEC: ProviderDiscoverySpec = ProviderDiscoverySpec {
117
id: "claude-code",
128
credential_env_vars: &["ANTHROPIC_API_KEY", "CLAUDE_API_KEY"],
139
};
1410

15-
impl ProviderPlugin for ClaudeProvider {
16-
fn id(&self) -> &'static str {
17-
SPEC.id
18-
}
19-
20-
fn discover_existing(&self) -> Result<Option<crate::DiscoveredProvider>, ProviderError> {
21-
discover_with_spec(&SPEC, &RealDiscoveryContext)
22-
}
23-
24-
fn credential_env_vars(&self) -> &'static [&'static str] {
25-
SPEC.credential_env_vars
26-
}
27-
}
28-
2911
#[cfg(test)]
3012
mod tests {
3113
use super::SPEC;

crates/openshell-providers/src/providers/codex.rs

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,13 @@
11
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
use crate::{
5-
ProviderDiscoverySpec, ProviderError, ProviderPlugin, RealDiscoveryContext, discover_with_spec,
6-
};
7-
8-
pub struct CodexProvider;
4+
use crate::ProviderDiscoverySpec;
95

106
pub const SPEC: ProviderDiscoverySpec = ProviderDiscoverySpec {
117
id: "codex",
128
credential_env_vars: &["OPENAI_API_KEY"],
139
};
1410

15-
impl ProviderPlugin for CodexProvider {
16-
fn id(&self) -> &'static str {
17-
SPEC.id
18-
}
19-
20-
fn discover_existing(&self) -> Result<Option<crate::DiscoveredProvider>, ProviderError> {
21-
discover_with_spec(&SPEC, &RealDiscoveryContext)
22-
}
23-
24-
fn credential_env_vars(&self) -> &'static [&'static str] {
25-
SPEC.credential_env_vars
26-
}
27-
}
28-
2911
#[cfg(test)]
3012
mod tests {
3113
use super::SPEC;

0 commit comments

Comments
 (0)