Skip to content

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Oct 19, 2025

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Disable the custom exec probe to restart the Distributor as a workaround if there is no Node able to register with the Distributor.
Now, [grid] Add event bus heartbeat to prevent steal connection - SeleniumHQ/selenium#16444
So, issue Distributor Requires Restart to Register New Nodes After Hours of Inactivity could be resolved.

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Enhancement


Description

  • Add event bus heartbeat period configuration support across all Selenium Grid components

  • Enable SE_EVENT_BUS_HEARTBEAT_PERIOD environment variable in startup scripts

  • Change default component liveness probe from exec to httpGet in Helm charts

  • Prevent connection stealing by implementing heartbeat mechanism in event bus


Diagram Walkthrough

flowchart LR
  ENV["SE_EVENT_BUS_HEARTBEAT_PERIOD<br/>Environment Variable"]
  SCRIPTS["Startup Scripts<br/>EventBus, Hub, Standalone,<br/>StandaloneDocker"]
  OPTS["--eventbus-heartbeat-period<br/>Command Option"]
  HELM["Helm Chart<br/>defaultComponentLivenessProbe"]
  
  ENV -- "passed to" --> SCRIPTS
  SCRIPTS -- "appends" --> OPTS
  HELM -- "updated default" --> HELM
Loading

File Walkthrough

Relevant files
Enhancement
start-selenium-grid-eventbus.sh
Add event bus heartbeat period configuration                         

EventBus/start-selenium-grid-eventbus.sh

  • Add conditional check for SE_EVENT_BUS_HEARTBEAT_PERIOD environment
    variable
  • Append --eventbus-heartbeat-period option to Selenium Grid startup
    command
+4/-0     
start-selenium-grid-hub.sh
Add event bus heartbeat period configuration                         

Hub/start-selenium-grid-hub.sh

  • Add conditional check for SE_EVENT_BUS_HEARTBEAT_PERIOD environment
    variable
  • Append --eventbus-heartbeat-period option to Selenium Grid startup
    command
+4/-0     
start-selenium-standalone.sh
Add event bus heartbeat period configuration                         

Standalone/start-selenium-standalone.sh

  • Add conditional check for SE_EVENT_BUS_HEARTBEAT_PERIOD environment
    variable
  • Append --eventbus-heartbeat-period option to Selenium Grid startup
    command
+4/-0     
start-selenium-grid-docker.sh
Add event bus heartbeat period configuration                         

StandaloneDocker/start-selenium-grid-docker.sh

  • Add conditional check for SE_EVENT_BUS_HEARTBEAT_PERIOD environment
    variable
  • Append --eventbus-heartbeat-period option to Selenium Grid startup
    command
+4/-0     
Documentation
CONFIGURATION.md
Update default component liveness probe documentation       

charts/selenium-grid/CONFIGURATION.md

  • Update default value for
    global.seleniumGrid.defaultComponentLivenessProbe from exec to httpGet
  • Align documentation with Helm chart configuration changes
+1/-1     
Configuration changes
values.yaml
Change default component liveness probe method                     

charts/selenium-grid/values.yaml

  • Change defaultComponentLivenessProbe default value from exec to
    httpGet
  • Update Helm chart configuration for component health checks
+1/-1     

…eat to prevent steal connection

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Unvalidated input

Description: Passing the heartbeat period from an environment variable directly to a command-line flag
without validation could allow unintended values (e.g., extremely small or huge numbers)
that may cause denial of service or instability.
start-selenium-grid-eventbus.sh [57-59]

Referred Code
if [ ! -z "${SE_EVENT_BUS_HEARTBEAT_PERIOD}" ]; then
  append_se_opts "--eventbus-heartbeat-period" "${SE_EVENT_BUS_HEARTBEAT_PERIOD}"
fi
Unvalidated input

Description: Unvalidated SE_EVENT_BUS_HEARTBEAT_PERIOD is forwarded to the process; lack of bounds
checking may lead to resource exhaustion or misbehavior.
start-selenium-grid-hub.sh [136-138]

Referred Code
if [ ! -z "${SE_EVENT_BUS_HEARTBEAT_PERIOD}" ]; then
  append_se_opts "--eventbus-heartbeat-period" "${SE_EVENT_BUS_HEARTBEAT_PERIOD}"
fi
Unvalidated input

Description: The heartbeat period environment variable is appended without validation; malicious or
improper values could impact stability.
start-selenium-standalone.sh [102-104]

Referred Code
if [ ! -z "${SE_EVENT_BUS_HEARTBEAT_PERIOD}" ]; then
  append_se_opts "--eventbus-heartbeat-period" "${SE_EVENT_BUS_HEARTBEAT_PERIOD}"
fi
Unvalidated input

Description: Forwarding SE_EVENT_BUS_HEARTBEAT_PERIOD directly to command args without validation may
allow abusive values affecting availability.
start-selenium-grid-docker.sh [72-74]

Referred Code
if [ ! -z "${SE_EVENT_BUS_HEARTBEAT_PERIOD}" ]; then
  append_se_opts "--eventbus-heartbeat-period" "${SE_EVENT_BUS_HEARTBEAT_PERIOD}"
fi
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Enable the new heartbeat feature by default

Set a default value for the new SE_EVENT_BUS_HEARTBEAT_PERIOD environment
variable in the Helm chart's values.yaml file. This will enable the new
connection stability feature by default.

Examples:

Hub/start-selenium-grid-hub.sh [136-138]
if [ ! -z "${SE_EVENT_BUS_HEARTBEAT_PERIOD}" ]; then
  append_se_opts "--eventbus-heartbeat-period" "${SE_EVENT_BUS_HEARTBEAT_PERIOD}"
fi
EventBus/start-selenium-grid-eventbus.sh [57-59]
if [ ! -z "${SE_EVENT_BUS_HEARTBEAT_PERIOD}" ]; then
  append_se_opts "--eventbus-heartbeat-period" "${SE_EVENT_BUS_HEARTBEAT_PERIOD}"
fi

Solution Walkthrough:

Before:

# In charts/selenium-grid/values.yaml (conceptual)

hub:
  # ... other configs
  env:
    - name: SOME_VAR
      value: "some_value"
    # SE_EVENT_BUS_HEARTBEAT_PERIOD is not configured,
    # so the feature is disabled by default for Helm users.

eventBus:
  # ... other configs
  env:
    - name: SOME_OTHER_VAR
      value: "another_value"
    # SE_EVENT_BUS_HEARTBEAT_PERIOD is not configured.

After:

# In charts/selenium-grid/values.yaml (conceptual)

hub:
  # ... other configs
  env:
    - name: SOME_VAR
      value: "some_value"
    - name: SE_EVENT_BUS_HEARTBEAT_PERIOD
      value: "60" # A sensible default value

eventBus:
  # ... other configs
  env:
    - name: SOME_OTHER_VAR
      value: "another_value"
    - name: SE_EVENT_BUS_HEARTBEAT_PERIOD
      value: "60" # A sensible default value
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that the new heartbeat feature, a key part of the PR, is not enabled by default in the Helm chart, which limits its immediate benefit for users.

Medium
Possible issue
Validate environment variable is positive

Validate that the SE_EVENT_BUS_HEARTBEAT_PERIOD environment variable is a
positive integer, not just a non-empty string, to prevent invalid values from
being passed to the Selenium Grid.

EventBus/start-selenium-grid-eventbus.sh [57-59]

-if [ ! -z "${SE_EVENT_BUS_HEARTBEAT_PERIOD}" ]; then
+if [[ "${SE_EVENT_BUS_HEARTBEAT_PERIOD}" =~ ^[1-9][0-9]*$ ]]; then
   append_se_opts "--eventbus-heartbeat-period" "${SE_EVENT_BUS_HEARTBEAT_PERIOD}"
 fi
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a lack of input validation for the SE_EVENT_BUS_HEARTBEAT_PERIOD variable and proposes a more robust check to ensure it is a positive integer, preventing potential runtime issues.

Low
Learned
best practice
Quote and use -n in tests

Use -n for non-empty checks and quote variables; prefer parameter expansion with
defaults to avoid empty args.

EventBus/start-selenium-grid-eventbus.sh [57-59]

-if [ ! -z "${SE_EVENT_BUS_HEARTBEAT_PERIOD}" ]; then
+if [ -n "${SE_EVENT_BUS_HEARTBEAT_PERIOD:-}" ]; then
   append_se_opts "--eventbus-heartbeat-period" "${SE_EVENT_BUS_HEARTBEAT_PERIOD}"
 fi
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Harden shell scripts by properly quoting variable expansions in test expressions and arguments to handle spaces and special characters.

Low
  • More

@VietND96 VietND96 merged commit a22c199 into trunk Oct 19, 2025
56 of 57 checks passed
@VietND96 VietND96 deleted the event-bus-heartbeat branch October 19, 2025 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant