Skip to content

Conversation

@VietND96
Copy link
Member

@VietND96 VietND96 commented Oct 28, 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

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 session request timeout configuration support for autoscaling relay node tests

  • Introduce SET_SESSION_REQUEST_TIMEOUT environment variable in Makefile target

  • Enable dynamic Helm chart configuration with session timeout parameter

  • Set timeout value to 600 seconds for relay node autoscaling job


Diagram Walkthrough

flowchart LR
  A["Makefile<br/>chart_test_autoscaling_job_relay"] -->|"SET_SESSION_REQUEST_TIMEOUT=600"| B["chart_test.sh"]
  B -->|"--set global.seleniumGrid.sessionRequestTimeout"| C["Helm Chart<br/>Configuration"]
Loading

File Walkthrough

Relevant files
Configuration changes
Makefile
Add session timeout configuration to relay autoscaling target

Makefile

  • Added SET_MAX_REPLICAS=3 and SET_SESSION_REQUEST_TIMEOUT=600
    environment variables to chart_test_autoscaling_job_relay target
  • Configures autoscaling relay node test with specific session request
    timeout value
+1/-0     
Enhancement
chart_test.sh
Add dynamic session timeout Helm parameter support             

tests/charts/make/chart_test.sh

  • Added conditional block to check for SET_SESSION_REQUEST_TIMEOUT
    environment variable
  • Dynamically appends --set global.seleniumGrid.sessionRequestTimeout
    parameter to Helm command when variable is set
  • Enables flexible session timeout configuration for Selenium Grid
    deployment
+6/-0     

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 28, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logs: The newly added configuration logic does not include any logging of actions or outcomes,
making it unclear when and how session timeout settings are applied.

Referred Code
if [ -n "${SET_SESSION_REQUEST_TIMEOUT}" ]; then
  HELM_COMMAND_SET_IMAGES="${HELM_COMMAND_SET_IMAGES} \
  --set global.seleniumGrid.sessionRequestTimeout=${SET_SESSION_REQUEST_TIMEOUT} \
  "
fi
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing validation: The script forwards SET_SESSION_REQUEST_TIMEOUT directly to Helm without validating that
it is a positive integer or within acceptable bounds, risking misconfiguration.

Referred Code
if [ -n "${SET_SESSION_REQUEST_TIMEOUT}" ]; then
  HELM_COMMAND_SET_IMAGES="${HELM_COMMAND_SET_IMAGES} \
  --set global.seleniumGrid.sessionRequestTimeout=${SET_SESSION_REQUEST_TIMEOUT} \
  "
fi
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unvalidated input: The environment variable SET_SESSION_REQUEST_TIMEOUT is consumed without sanitization or
validation before being injected into Helm parameters, which could allow unintended values
to affect deployment configuration.

Referred Code
if [ -n "${SET_SESSION_REQUEST_TIMEOUT}" ]; then
  HELM_COMMAND_SET_IMAGES="${HELM_COMMAND_SET_IMAGES} \
  --set global.seleniumGrid.sessionRequestTimeout=${SET_SESSION_REQUEST_TIMEOUT} \
  "
fi
  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 28, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Validate environment variable before use

Add validation to ensure the SET_SESSION_REQUEST_TIMEOUT environment variable is
an integer before it is used in the helm command. This will prevent potential
script failures or unexpected behavior.

tests/charts/make/chart_test.sh [214-218]

 if [ -n "${SET_SESSION_REQUEST_TIMEOUT}" ]; then
+  if ! [[ "${SET_SESSION_REQUEST_TIMEOUT}" =~ ^[0-9]+$ ]]; then
+    echo "Error: SET_SESSION_REQUEST_TIMEOUT must be an integer." >&2
+    exit 1
+  fi
   HELM_COMMAND_SET_IMAGES="${HELM_COMMAND_SET_IMAGES} \
   --set global.seleniumGrid.sessionRequestTimeout=${SET_SESSION_REQUEST_TIMEOUT} \
   "
 fi
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the SET_SESSION_REQUEST_TIMEOUT environment variable is used without validation, and proposes adding a check to ensure it's an integer, which improves the script's robustness.

Medium
  • Update

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@VietND96 VietND96 enabled auto-merge (squash) October 29, 2025 04:41
@VietND96 VietND96 disabled auto-merge October 29, 2025 04:41
@VietND96 VietND96 merged commit 3a102c8 into trunk Oct 29, 2025
29 checks passed
@VietND96 VietND96 deleted the test-relay branch October 29, 2025 04:42
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.

2 participants