Skip to content

fix(server,k8s): propagate template container securityContext to pods - #1564

Merged
Pangjiping merged 3 commits into
opensandbox-group:mainfrom
gegemeimingzi:fix/batchsandbox-container-security-context
Aug 19, 2026
Merged

fix(server,k8s): propagate template container securityContext to pods#1564
Pangjiping merged 3 commits into
opensandbox-group:mainfrom
gegemeimingzi:fix/batchsandbox-container-security-context

Conversation

@gegemeimingzi

Copy link
Copy Markdown
Contributor

Closes #1556

Problem

When creating a BatchSandbox workload from a template, the pod-level securityContext from the template is applied, but the container-level securityContext is silently dropped from the generated Pod.

Root cause: merge_with_runtime_values_deep_merge replaces lists wholesale, so the runtime manifest's own containers list replaces the template's containers entirely. _merge_pod_spec_extras only carried forward the template container's volumeMounts, so any other container-level fields (including securityContext) were lost.

Fix

Extract the template main container's securityContext alongside the existing volumes/mounts and merge it back into the generated pod's main container.

  • Template securityContext is applied when the runtime container has none (the reported case).
  • When the runtime container already has a securityContext (e.g. the capabilities.drop: [NET_ADMIN] set by network-policy wiring), template keys supplement it key-by-key and runtime keys keep precedence — same "supplement, don't override" philosophy the code already uses for volumeMounts.

Tests

  • test_create_workload_applies_template_container_security_context — template container securityContext now reaches the generated Pod's main container (template image still does not override the runtime image).
  • test_create_workload_merges_template_security_context_with_runtime_network_policy — template keys supplement, network-policy capabilities keep precedence.

Verified: full server/tests/k8s/ suite passes (584 tests), ruff check clean.

The template container's securityContext was dropped when creating
BatchSandbox workloads: merge_with_runtime_values replaces the runtime
containers list wholesale, and only volumeMounts were carried forward.
Extract the template main container's securityContext and merge it into
the generated pod's main container, supplementing the runtime
securityContext key-by-key so network-policy/isolation wiring keeps
precedence.

Closes opensandbox-group#1556

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added component/server size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 18, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b50a029365

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/opensandbox_server/services/k8s/batchsandbox_provider.py Outdated
The previous merge only setdefault'd at the top level, so when the
runtime container already had a securityContext (e.g. capabilities.drop
from network-policy wiring), a template's nested members such as
capabilities.add never reached the generated Pod. Merge nested dicts
recursively, keeping runtime precedence on actual conflicting leaves.

Co-Authored-By: Claude <noreply@anthropic.com>

@Pangjiping Pangjiping left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Pangjiping
Pangjiping merged commit 749a87f into opensandbox-group:main Aug 19, 2026
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/server size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kubernetes template does not apply container-level securityContext

2 participants