Skip to content
Merged
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
7 changes: 4 additions & 3 deletions .agents/skills/helm-dev-environment/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,10 @@ mise run helm:skaffold:run
Resource admission defaults to enabled and caller driver config to disabled.
Driver-config scenarios need an explicit `allowDriverConfig` opt-in; external
attachments also need administrator-controlled approval labels in the target
namespace. GPU attachments are exempt from labels. Managed workspace image-pull
Secrets are copied from an approved source in the gateway namespace; do not grant
approval to the gateway database PVC or disable admission to make tests pass.
namespace. GPU attachments and operator-selected image-pull Secrets are exempt
from labels. Managed workspace image-pull Secrets are copied from the configured
source in the gateway namespace; do not grant approval to the gateway database
PVC or disable admission to make tests pass.

The Skaffold flow builds distinct `gateway`, `sandbox`, and `supervisor` images
and deploys the OpenShell Helm chart. The Kubernetes driver creates a
Expand Down
6 changes: 3 additions & 3 deletions architecture/compute-runtimes.md
Original file line number Diff line number Diff line change
Expand Up @@ -465,9 +465,9 @@ management. RBAC uses a namespace-scoped Role.
Each new namespace receives a ServiceAccount and the configured gateway-only
SSH ingress NetworkPolicy. Configured image-pull Secrets are copied from the
driver's source namespace on every sandbox create so registry credential
rotations propagate. Resource admission first validates the source as shared
operator infrastructure, and copies carry gateway and workspace ownership
labels; an unrelated existing target is never adopted. The namespace also copies
rotations propagate. Their names are operator-selected gateway configuration,
not caller attachments. Copies carry gateway and workspace ownership labels; an
unrelated existing target is never adopted. The namespace also copies
OpenShift SCC UID-range and supplemental-group annotations from the gateway
namespace when present. The driver deletes the namespace during workspace
deletion. The workspace remains durably `Terminating` until the Kubernetes API
Expand Down
4 changes: 2 additions & 2 deletions crates/openshell-driver-kubernetes/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ Kubernetes-backed compute driver for OpenShell cluster deployments.
Caller driver config is disabled by default. External resource references need
administrator-controlled approval labels in every workspace mode, including
before restart and scheduling-gate release. GPU devices are temporarily exempt.
Managed workspace image-pull Secrets are copied only after the configured source
Secret passes shared-resource admission; copies carry gateway ownership metadata.
Image-pull Secrets are operator-selected gateway configuration rather than caller
attachments. Managed workspace copies carry gateway ownership metadata.
See [resource admission configuration](../../docs/reference/gateway-config.mdx#external-resource-admission).

The driver uses the Kubernetes API to create, delete, fetch, and watch sandbox
Expand Down
74 changes: 3 additions & 71 deletions crates/openshell-driver-kubernetes/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1306,28 +1306,6 @@ impl KubernetesComputeDriver {
}
};

self.config
.resource_admission
.admit_shared(
source
.metadata
.labels
.as_ref()
.into_iter()
.flat_map(|labels| labels.iter()),
)
.map_err(|error| {
admission_error(tonic::Status::new(
error.code(),
format!(
"Secret '{}/{}': {}",
self.config.namespace,
secret_name,
error.message()
),
))
})?;

let existing = tokio::time::timeout(KUBE_API_TIMEOUT, target_api.get_opt(secret_name))
.await
.map_err(|_| {
Expand Down Expand Up @@ -11070,7 +11048,7 @@ mod tests {
}

#[tokio::test]
async fn managed_image_pull_secret_admits_source_before_copying() {
async fn managed_image_pull_secret_copies_operator_selected_source() {
let source_path = "/api/v1/namespaces/openshell/secrets/regcred";
let target_path = "/api/v1/namespaces/managed-team-a/secrets/regcred";
let copied = serde_json::json!({
Expand Down Expand Up @@ -11101,7 +11079,7 @@ mod tests {
"metadata": {
"name": "regcred",
"namespace": "openshell",
"labels": {"openshell.ai/sandbox-attachable": "true"}
"labels": {}
},
"type": "kubernetes.io/dockerconfigjson",
"data": { ".dockerconfigjson": "e30=" }
Expand Down Expand Up @@ -11150,56 +11128,10 @@ mod tests {
driver
.ensure_image_pull_secrets("managed-team-a", "team-a")
.await
.expect("approved source should be copied");
.expect("operator-selected source should be copied");
assert!(steps.lock().unwrap().is_empty());
}

#[tokio::test]
async fn managed_image_pull_secret_rejects_unapproved_source_without_copying() {
let service = tower::service_fn(
move |request: http::Request<kube::client::Body>| async move {
assert_eq!(request.method(), http::Method::GET);
assert_eq!(
request.uri().path(),
"/api/v1/namespaces/openshell/secrets/regcred"
);
Ok::<_, std::convert::Infallible>(kube_test_response(
http::StatusCode::OK,
serde_json::json!({
"apiVersion": "v1",
"kind": "Secret",
"metadata": {"name": "regcred", "namespace": "openshell"},
"type": "kubernetes.io/dockerconfigjson",
"data": { ".dockerconfigjson": "e30=" }
}),
))
},
);
let client = Client::new(service, "openshell");
let driver = KubernetesComputeDriver {
client: client.clone(),
watch_client: client,
sandbox_api_version: Arc::new(OnceCell::new()),
config: KubernetesComputeConfig {
namespace: "openshell".into(),
gateway_id: "gateway-a".into(),
image_pull_secrets: vec!["regcred".into()],
..Default::default()
},
operator_allowlist: None,
};

let error = driver
.ensure_image_pull_secrets("managed-team-a", "team-a")
.await
.expect_err("unapproved source must not be copied");
assert!(matches!(error, KubernetesDriverError::Precondition(_)));
assert!(
error.to_string().contains("Secret 'openshell/regcred'"),
"unexpected error: {error}"
);
}

#[test]
fn namespace_owned_with_correct_labels() {
let labels = BTreeMap::from([
Expand Down
72 changes: 25 additions & 47 deletions crates/openshell-driver-kubernetes/src/resource_admission.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,9 @@ fn inventory(spec: &Value, private_secret: &str) -> Result<BTreeSet<Reference>,
{
return Err(deny());
}
for secret in spec["imagePullSecrets"].as_array().into_iter().flatten() {
reference(&mut refs, "Secret", secret["name"].as_str(), Scope::Shared);
}
// Image-pull Secrets are selected by gateway configuration rather than by
// the sandbox caller. The kubelet resolves them; they are not workload data
// attachments and do not participate in caller resource admission.
for volume in spec["volumes"].as_array().into_iter().flatten() {
let object = volume.as_object().ok_or_else(deny)?;
let sources: Vec<_> = object.keys().filter(|key| key.as_str() != "name").collect();
Expand All @@ -127,50 +127,27 @@ fn inventory(spec: &Value, private_secret: &str) -> Result<BTreeSet<Reference>,
volume["persistentVolumeClaim"]["claimName"].as_str(),
Scope::Workspace,
),
"secret" => {
let name = volume["secret"]["secretName"].as_str();
if name != Some(private_secret) {
reference(&mut refs, "Secret", name, Scope::Workspace);
}
}
"configMap" => reference(
&mut refs,
"ConfigMap",
volume["configMap"]["name"].as_str(),
Scope::Workspace,
),
"secret" if volume["secret"]["secretName"].as_str() == Some(private_secret) => {}
// The typed Kubernetes driver config does not expose Secret or
// ConfigMap volumes. The closed-set fallback rejects them instead
// of expanding gateway RBAC for attachments callers cannot request.
_ => return Err(deny()),
}
}
for field in ["containers", "initContainers", "ephemeralContainers"] {
for container in spec[field].as_array().into_iter().flatten() {
for env in container["envFrom"].as_array().into_iter().flatten() {
reference(
&mut refs,
"Secret",
env["secretRef"]["name"].as_str(),
Scope::Workspace,
);
reference(
&mut refs,
"ConfigMap",
env["configMapRef"]["name"].as_str(),
Scope::Workspace,
);
if container["envFrom"]
.as_array()
.is_some_and(|env| !env.is_empty())
{
return Err(deny());
}
for env in container["env"].as_array().into_iter().flatten() {
reference(
&mut refs,
"Secret",
env["valueFrom"]["secretKeyRef"]["name"].as_str(),
Scope::Workspace,
);
reference(
&mut refs,
"ConfigMap",
env["valueFrom"]["configMapKeyRef"]["name"].as_str(),
Scope::Workspace,
);
if !env["valueFrom"]["secretKeyRef"].is_null()
|| !env["valueFrom"]["configMapKeyRef"].is_null()
{
return Err(deny());
}
}
for field in ["requests", "limits"] {
for (resource, _) in container["resources"][field]
Expand Down Expand Up @@ -207,8 +184,6 @@ pub async fn admit(
for reference in inventory(spec, private_secret)? {
let (group, version, plural, cluster) = match reference.kind {
"PersistentVolumeClaim" => ("", "v1", "persistentvolumeclaims", false),
"Secret" => ("", "v1", "secrets", false),
"ConfigMap" => ("", "v1", "configmaps", false),
"RuntimeClass" => ("node.k8s.io", "v1", "runtimeclasses", true),
"PriorityClass" => ("scheduling.k8s.io", "v1", "priorityclasses", true),
_ => unreachable!("closed resource inventory"),
Expand Down Expand Up @@ -479,13 +454,12 @@ mod tests {
}

#[test]
fn inventories_all_containers_and_reference_aliases() {
fn inventories_supported_external_resources() {
let pod = serde_json::json!({"automountServiceAccountToken":false,"runtimeClassName":"r","priorityClassName":"p",
"volumes":[{"name":"data","persistentVolumeClaim":{"claimName":"gateway-db","readOnly":true}}],
"initContainers":[{"envFrom":[{"secretRef":{"name":"secret"}}]}],
"containers":[{"env":[{"valueFrom":{"configMapKeyRef":{"name":"config"}}}]}]});
"imagePullSecrets":[{"name":"regcred"}]});
let refs = inventory(&pod, "private").unwrap();
assert_eq!(refs.len(), 5);
assert_eq!(refs.len(), 3);
assert!(
refs.iter()
.any(|r| r.name == "gateway-db" && r.scope == Scope::Workspace)
Expand All @@ -497,9 +471,13 @@ mod tests {
}
#[test]
fn rejects_unsupported_volume_sources_but_allows_gpu() {
for kind in ["hostPath", "csi", "projected", "image"] {
for kind in ["hostPath", "csi", "projected", "image", "configMap"] {
assert!(inventory(&serde_json::json!({"automountServiceAccountToken":false,"volumes":[{"name":"x",kind:{}}]}), "private").is_err());
}
assert!(inventory(&serde_json::json!({"automountServiceAccountToken":false,"volumes":[{"name":"x","secret":{"secretName":"external"}}]}), "private").is_err());
assert!(inventory(&serde_json::json!({"automountServiceAccountToken":false,"volumes":[{"name":"x","secret":{"secretName":"private"}}]}), "private").is_ok());
assert!(inventory(&serde_json::json!({"automountServiceAccountToken":false,"containers":[{"envFrom":[{"secretRef":{"name":"external"}}]}]}), "private").is_err());
assert!(inventory(&serde_json::json!({"automountServiceAccountToken":false,"containers":[{"env":[{"valueFrom":{"configMapKeyRef":{"name":"external"}}}]}]}), "private").is_err());
assert!(inventory(&serde_json::json!({"automountServiceAccountToken":false,"containers":[{"resources":{"limits":{"nvidia.com/gpu":"1"}}}]}), "private").is_ok());
}
#[test]
Expand Down
6 changes: 5 additions & 1 deletion deploy/helm/openshell-workspace/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,17 @@ helm install openshell-workspace ./deploy/helm/openshell-workspace \

Keep `sandboxServiceAccount.name` aligned with the gateway chart's
`sandboxServiceAccount.name`. The defaults for both charts are
`openshell-sandbox`.
`openshell-sandbox`. If the gateway enables
`server.drivers.kubernetes.allowDriverConfig`, also set
`gateway.allowDriverConfig=true` in every workspace release so caller-selected
PVCs can be admitted.

## Values

| Key | Type | Default | Description |
|-----|------|---------|-------------|
| fullnameOverride | string | `""` | Override the full generated resource name. |
| gateway.allowDriverConfig | bool | `false` | Grant PVC metadata reads required when the gateway enables caller driver config. Keep this aligned with server.drivers.kubernetes.allowDriverConfig in the gateway chart. |
| gateway.networkPolicy.podSelector | object | `{"app.kubernetes.io/instance":"openshell","app.kubernetes.io/name":"openshell"}` | Labels selecting gateway pods allowed to reach sandbox SSH. |
| gateway.serviceAccount.name | string | `"openshell"` | Name of the shared gateway ServiceAccount. |
| gateway.serviceAccount.namespace | string | `"openshell"` | Namespace containing the shared gateway ServiceAccount. |
Expand Down
5 changes: 4 additions & 1 deletion deploy/helm/openshell-workspace/README.md.gotmpl
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ helm install openshell-workspace ./deploy/helm/openshell-workspace \

Keep `sandboxServiceAccount.name` aligned with the gateway chart's
`sandboxServiceAccount.name`. The defaults for both charts are
`openshell-sandbox`.
`openshell-sandbox`. If the gateway enables
`server.drivers.kubernetes.allowDriverConfig`, also set
`gateway.allowDriverConfig=true` in every workspace release so caller-selected
PVCs can be admitted.

{{ template "chart.valuesSection" . }}
{{ template "helm-docs.versionFooter" . }}
6 changes: 4 additions & 2 deletions deploy/helm/openshell-workspace/templates/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ metadata:
labels:
{{- include "openshell-workspace.labels" . | nindent 4 }}
rules:
# Metadata-only application admission still requires Kubernetes get permission.
{{- if .Values.gateway.allowDriverConfig }}
# Metadata-only admission of caller-selected PVCs requires get permission.
- apiGroups: [""]
resources: ["persistentvolumeclaims", "configmaps", "secrets"]
resources: ["persistentvolumeclaims"]
verbs: ["get"]
{{- end }}
- apiGroups:
- agents.x-k8s.io
resources:
Expand Down
30 changes: 30 additions & 0 deletions deploy/helm/openshell-workspace/tests/workspace_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,36 @@ tests:
- equal:
path: metadata.namespace
value: app-a
- notContains:
path: rules
content:
apiGroups: [""]
resources: ["persistentvolumeclaims"]
verbs: ["get"]
- notContains:
path: rules
content:
apiGroups: [""]
resources: ["secrets"]
verbs: ["get"]
- notContains:
path: rules
content:
apiGroups: [""]
resources: ["configmaps"]
verbs: ["get"]

- it: grants PVC admission reads only when caller driver config is enabled
template: templates/role.yaml
set:
gateway.allowDriverConfig: true
asserts:
- contains:
path: rules
content:
apiGroups: [""]
resources: ["persistentvolumeclaims"]
verbs: ["get"]

- it: binds the shared gateway service account
template: templates/rolebinding.yaml
Expand Down
3 changes: 3 additions & 0 deletions deploy/helm/openshell-workspace/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ nameOverride: ""
fullnameOverride: ""

gateway:
# -- Grant PVC metadata reads required when the gateway enables caller driver config.
# Keep this aligned with server.drivers.kubernetes.allowDriverConfig in the gateway chart.
allowDriverConfig: false
serviceAccount:
# -- Name of the shared gateway ServiceAccount.
name: openshell
Expand Down
4 changes: 2 additions & 2 deletions deploy/helm/openshell/templates/clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ rules:
- apiGroups: ["scheduling.k8s.io"]
resources: ["priorityclasses"]
verbs: ["get"]
{{- if ne $workspaceMode "shared" }}
{{- if and (ne $workspaceMode "shared") .Values.server.drivers.kubernetes.allowDriverConfig }}
- apiGroups: [""]
resources: ["persistentvolumeclaims", "configmaps", "secrets"]
resources: ["persistentvolumeclaims"]
verbs: ["get"]
{{- end }}
# Validate projected ServiceAccount tokens during sandbox bootstrap and
Expand Down
6 changes: 4 additions & 2 deletions deploy/helm/openshell/templates/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ metadata:
labels:
{{- include "openshell.labels" . | nindent 4 }}
rules:
# Metadata-only application admission still requires Kubernetes get permission.
{{- if .Values.server.drivers.kubernetes.allowDriverConfig }}
# Metadata-only admission of caller-selected PVCs requires get permission.
- apiGroups: [""]
resources: ["persistentvolumeclaims", "configmaps", "secrets"]
resources: ["persistentvolumeclaims"]
verbs: ["get"]
{{- end }}
- apiGroups:
- agents.x-k8s.io
resources:
Expand Down
Loading
Loading