Skip to content

[grid] inherit the Node Pod container securityContext for Dynamic Grid on K8s - #17860

Merged
VietND96 merged 1 commit into
SeleniumHQ:trunkfrom
NDViet:k8s-inherit-container-security-context
Aug 2, 2026
Merged

[grid] inherit the Node Pod container securityContext for Dynamic Grid on K8s#17860
VietND96 merged 1 commit into
SeleniumHQ:trunkfrom
NDViet:k8s-inherit-container-security-context

Conversation

@VietND96

@VietND96 VietND96 commented Aug 2, 2026

Copy link
Copy Markdown
Member

🔗 Related Issues

Fixes #17847

💥 What does this PR do?

When Dynamic Grid scales on Kubernetes, the Node inspects its own Pod and inherits selected spec fields onto the browser session Pods it spawns. Until now only the pod-level securityContext (PodSecurityContext) was inherited; the container-level securityContext was left empty.

Some settings — notably allowPrivilegeEscalation and capabilities — can only be configured on the container securityContext, so spawned session Pods could not satisfy a restricted Pod Security Standard.

This PR reads the Node Pod's first container securityContext and applies it to the spawned browser container and the video sidecar, so session Pods can run under the restricted profile.

🔧 Implementation Notes

  • KubernetesOptions.inspectNodePod now reads firstContainer.getSecurityContext() (container-level io.fabric8.kubernetes.api.model.SecurityContext) alongside the container fields it already extracts (image pull policy, resource requests/limits).
  • InheritedPodSpec carries a new @Nullable SecurityContext containerSecurityContext field (getter + inclusion in hasInheritedFields()). Rather than editing every positional call site, a new constructor overload was added and the existing 15-arg / 17-arg constructors delegate to it with containerSecurityContext = null — this keeps their signatures unchanged, matching the incremental-constructor pattern already used in this class for nodePodName/nodePodUid.
  • KubernetesSessionFactory applies the inherited container securityContext to both the browser container and the video sidecar. All containers in a Pod must comply for the restricted profile, so the sidecar is included too.
  • Scope matches the existing pod-level securityContext behavior: inheritance applies to the auto-generated image mode path only. The template mode path is left fully user-controlled (a user-supplied Job template's container securityContext is not overridden).

Alternatives considered: exposing a dedicated CLI/config flag to set the container securityContext explicitly. Inheritance was chosen to mirror the existing pod-level securityContext behavior and to make "run the whole Grid under restricted" work with zero extra configuration. A config override could be layered on later if needed.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code (Opus 4.8)
    • What was generated: implementation and unit tests for inheriting the container securityContext
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

  • High-risk area: touches Grid Node K8s Pod-spawning logic. The change is additive — a container securityContext is set only when the Node Pod actually defines one, so default behavior is unchanged (a new test asserts the browser container has no securityContext when nothing is inherited).
  • Cross-binding: Java-only Grid server feature with no wire/protocol impact; no parity work needed in other bindings.
  • Possible follow-up: an explicit config override for the container securityContext, and documentation for the restricted Pod Security Standard setup.

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

…d on K8s

Dynamic Grid on Kubernetes inherited only the Node Pod's pod-level
securityContext when spawning browser session Pods. Container-level
settings such as allowPrivilegeEscalation and capabilities can only be
configured on the container securityContext, so spawned Pods could not
satisfy the restricted Pod Security Standard.

Read the Node Pod's first container securityContext and apply it to the
browser and video containers (image mode only, matching how the pod-level
securityContext is inherited).

Fixes SeleniumHQ#17847

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@selenium-ci selenium-ci added B-grid Everything grid and server related C-java Java Bindings labels Aug 2, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

[grid] Inherit Node Pod container SecurityContext for Dynamic Grid sessions on Kubernetes

🐞 Bug fix 🧪 Tests ✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Inherit Node Pod container-level SecurityContext for spawned session Pods (image mode only).
• Apply inherited container SecurityContext to both browser and video containers for PSS restricted.
• Add unit tests covering SecurityContext propagation and default null behavior.
Diagram

graph TD
  A[("Node Pod")] --> B["KubernetesOptions.inspectNodePod"] --> C["InheritedPodSpec"] --> D["KubernetesSessionFactory"] --> E["Session Job/Pod spec"]
  E --> F["browser container"]
  E --> G["video container"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add explicit config/CLI for session container SecurityContext
  • ➕ More explicit and predictable than inheriting from the Node Pod
  • ➕ Allows overriding only sessions without changing Node deployment
  • ➖ Adds new user-facing surface area and documentation burden
  • ➖ Requires users to duplicate Node Pod settings instead of reusing them
  • ➖ May diverge from existing pod-level securityContext inheritance behavior
2. Inherit SecurityContext per container name (browser vs node vs sidecars)
  • ➕ Avoids relying on "first container" semantics when Node Pod has multiple containers
  • ➕ Enables different policies for browser vs video sidecar
  • ➖ More complex mapping logic and additional configuration/heuristics
  • ➖ Harder to keep behavior intuitive; may surprise users

Recommendation: The chosen inheritance approach is appropriate because it mirrors existing pod-level inheritance and enables "restricted" compliance with zero additional configuration. If multi-container Node Pods become common, consider evolving from "first container" inheritance to an explicit container-name mapping or a dedicated override flag.

Files changed (5) +201 / -8

Enhancement (1) +50 / -0
InheritedPodSpec.javaAdd inherited container SecurityContext to InheritedPodSpec +50/-0

Add inherited container SecurityContext to InheritedPodSpec

• Introduces a new nullable container-level SecurityContext field, getter, and inclusion in hasInheritedFields(). Adds a constructor overload so existing call sites remain compatible while enabling the new field to be passed through.

java/src/org/openqa/selenium/grid/node/kubernetes/InheritedPodSpec.java

Bug fix (2) +29 / -8
KubernetesOptions.javaInspect Node Pod first container SecurityContext for inheritance +5/-1

Inspect Node Pod first container SecurityContext for inheritance

• Extends Node Pod inspection to read the first container's SecurityContext alongside existing extracted fields. Passes the inherited container SecurityContext into InheritedPodSpec so downstream session creation can apply it.

java/src/org/openqa/selenium/grid/node/kubernetes/KubernetesOptions.java

KubernetesSessionFactory.javaApply inherited container SecurityContext to browser and video containers +24/-7

Apply inherited container SecurityContext to browser and video containers

• When building the browser container (and the optional video sidecar), applies the inherited container SecurityContext if present. This ensures all containers in the session Pod can satisfy restricted Pod Security Standard requirements.

java/src/org/openqa/selenium/grid/node/kubernetes/KubernetesSessionFactory.java

Tests (2) +122 / -0
InheritedPodSpecTest.javaTest container SecurityContext storage and hasInheritedFields behavior +39/-0

Test container SecurityContext storage and hasInheritedFields behavior

• Adds assertions that an empty spec has no container SecurityContext and a new test verifying the container SecurityContext is stored and makes the spec count as having inherited fields.

java/test/org/openqa/selenium/grid/node/kubernetes/InheritedPodSpecTest.java

KubernetesSessionFactoryTest.javaTest image-mode inheritance into browser and video container securityContext +83/-0

Test image-mode inheritance into browser and video container securityContext

• Adds helpers and tests to verify that, in image mode, the browser and video containers inherit the SecurityContext when provided, and that no SecurityContext is set when not inherited.

java/test/org/openqa/selenium/grid/node/kubernetes/KubernetesSessionFactoryTest.java

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. SecurityContext copied wholesale 🐞 Bug ⛨ Security
Description
KubernetesSessionFactory applies the Node Pod’s first container SecurityContext object directly onto
the generated browser/video containers via withSecurityContext(). This propagates all
container-level security settings from the Node container to session containers (not just the
intended fields like allowPrivilegeEscalation/capabilities), making session behavior tightly coupled
to Node container configuration changes.
Code

java/src/org/openqa/selenium/grid/node/kubernetes/KubernetesSessionFactory.java[R840-843]

+    SecurityContext containerSecurityContext = inheritedPodSpec.getContainerSecurityContext();
+    if (containerSecurityContext != null) {
+      containerBuilder.withSecurityContext(containerSecurityContext);
+    }
Evidence
The Node Pod’s first container SecurityContext is captured and stored in InheritedPodSpec, and then
applied directly (without filtering) to the generated browser and video containers via
withSecurityContext(), meaning all fields present on the Node container securityContext will be
propagated to session containers.

java/src/org/openqa/selenium/grid/node/kubernetes/KubernetesOptions.java[432-447]
java/src/org/openqa/selenium/grid/node/kubernetes/KubernetesOptions.java[477-496]
java/src/org/openqa/selenium/grid/node/kubernetes/InheritedPodSpec.java[129-168]
java/src/org/openqa/selenium/grid/node/kubernetes/KubernetesSessionFactory.java[838-846]
java/src/org/openqa/selenium/grid/node/kubernetes/KubernetesSessionFactory.java[873-889]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Session Pods currently receive the Node Pod container `SecurityContext` **as-is**. This creates an overly broad inheritance contract where any current/future Node container `securityContext` fields are automatically applied to browser/video session containers.

### Issue Context
- The Node Pod inspection stores `firstContainer.getSecurityContext()` into `InheritedPodSpec`.
- The session factory then calls `containerBuilder.withSecurityContext(containerSecurityContext)` for both `browser` and `video`.

### Fix Focus Areas
- Implement an **allowlist copy** (construct a new `SecurityContext`) containing only the explicitly supported/inherited fields, rather than passing the Node object reference through untouched.
- Keep the allowlist aligned with what you intend to inherit for restricted PSS (and document it in code).

- java/src/org/openqa/selenium/grid/node/kubernetes/KubernetesOptions.java[432-447]
- java/src/org/openqa/selenium/grid/node/kubernetes/KubernetesOptions.java[477-496]
- java/src/org/openqa/selenium/grid/node/kubernetes/InheritedPodSpec.java[129-168]
- java/src/org/openqa/selenium/grid/node/kubernetes/KubernetesSessionFactory.java[838-846]
- java/src/org/openqa/selenium/grid/node/kubernetes/KubernetesSessionFactory.java[873-889]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@VietND96
VietND96 merged commit e107560 into SeleniumHQ:trunk Aug 2, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-grid Everything grid and server related C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🚀 Feature]: allow to configure container security context for dynamic scaling in k8s

2 participants