[grid] Improve browser container labels and naming in Dynamic Grid

[VietND96]

User description

🔗 Related Issues

💥 What does this PR do?

Dynamic Grid Container Naming Implementation

• Pattern: browser-<browserName>-<timestamp>-<uuid> and recorder-<browserName>-<timestamp>-<uuid>
• Example: browser-chrome-1731625200000-a3f2b8c9, recorder-chrome-1731625200000-a3f2b8c9
• Benefits:
  • Easy identification of container purpose and browser type
  • Timestamp allows chronological sorting
  • 8-character UUID prevents naming conflicts in concurrent session creation
  • Browser and recorder containers share the same identifier for easy correlation

Docker Compose Label Inheritance

• Extracted Labels: com.docker.compose.* labels from parent node container
• Applied To: Both browser and video recorder containers
• Benefits:
  • Containers recognized as part of the same Compose project
  • Proper lifecycle management by Docker Compose
  • Network inheritance and isolation
  • Cleanup handled by docker-compose down*

Before

After

🔧 Implementation Notes

Backward Compatibility
All changes are backward compatible
Container name is optional; existing code without names continues to work
Labels are optional; nodes not running in Docker Compose will have empty label maps

Related Issues
Improves debugging and monitoring of Docker-based Grid sessions
Addresses container lifecycle management in Docker Compose environments
Prevents naming conflicts in high-concurrency scenarios

💡 Additional Considerations

🔄 Types of changes

• Cleanup (formatting, renaming)
• Bug fix (backwards compatible)
• New feature (non-breaking change which adds functionality and tests!)
• Breaking change (fix or feature that would cause existing functionality to change)

---

PR Type

Enhancement

---

Description

• Implement container naming pattern for browser and recorder containers

• Extract and apply Docker Compose labels to containers for lifecycle management

• Add labels and name fields to ContainerConfig and ContainerInfo classes

• Generate unique container identifiers using timestamp and UUID

---

Diagram Walkthrough

flowchart LR
  A["DockerSessionFactory"] -->|generates sessionIdentifier| B["timestamp + UUID"]
  B -->|creates browserContainer| C["browser-browserName-timestamp-uuid"]
  B -->|creates videoContainer| D["recorder-browserName-timestamp-uuid"]
  E["Parent Node Container"] -->|extracts com.docker.compose labels| F["composeLabels"]
  F -->|applies to| C
  F -->|applies to| D
  C -->|stored in| G["ContainerConfig"]
  D -->|stored in| G
  G -->|passed to| H["CreateContainer API"]

Loading

File Walkthrough

Relevant files

Enhancement

ContainerConfig.java Add labels and name support to ContainerConfig                      java/src/org/openqa/selenium/docker/ContainerConfig.java Added labels and name fields to store container metadata Added multiple constructor overloads to support labels and name parameters Added getName(), labels(), and name() methods for fluent API Updated all builder methods (map(), env(), bind(), etc.) to preserve labels and name Modified toJson() to include labels in container creation request when non-empty +163/-11 ContainerInfo.java Add labels field to ContainerInfo                                                java/src/org/openqa/selenium/docker/ContainerInfo.java Added labels field to store container labels from Docker inspect response Added constructor overload accepting labels parameter Added getLabels() getter method to retrieve container labels +16/-0    CreateContainer.java Support container naming in Docker API request                      java/src/org/openqa/selenium/docker/client/CreateContainer.java Modified container creation URL to include optional name query parameter Dynamically builds URL with container name when provided in ContainerConfig +7/-1      InspectContainer.java Extract labels from Docker inspect response                            java/src/org/openqa/selenium/docker/client/InspectContainer.java Extract labels from container Config section in Docker inspect response Pass extracted labels to ContainerInfo constructor +5/-1      DockerOptions.java Extract and pass Docker Compose labels                                      java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java Added getComposeLabels() method to filter Docker Compose labels from parent node container Filters labels starting with com.docker.compose. prefix Pass compose labels to DockerSessionFactory constructor +17/-1    DockerSessionFactory.java Implement container naming and label application                  java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java Added composeLabels field to store Docker Compose labels from parent node Generate unique session identifier using timestamp and 6-character UUID Create browser container with name pattern browser--- Create video recorder container with name pattern recorder--- Apply compose labels to both browser and video containers Updated createBrowserContainer() and startVideoContainer() methods to accept and use sessionIdentifier +40/-8

---

[qodo-code-review]

PR Compliance Guide 🔍

(Compliance updated until commit f95d8f3)

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	URL parameter injection Description: Container name from user-derived capabilities is appended to the Docker create URL and only URL-encoded, which could allow injection of unexpected query parameters or path traversal if upstream inputs are not strictly validated; sanitize against characters like '&', '=' and '/' beyond encoding or pass name via a safe parameterization method. CreateContainer.java [69-77] Referred Code String url = String.format("/v%s/containers/create", apiVersion); if (info.getName() != null && !info.getName().trim().isEmpty()) { String containerName = info.getName().trim(); try { String encodedName = URLEncoder.encode(containerName, StandardCharsets.UTF_8.toString()); url += "?name=" + encodedName; } catch (UnsupportedEncodingException e) { throw new DockerException("Failed to encode container name: " + containerName, e); }
Ticket Compliance
🟡	🎫 #1234 🔴 Investigate and fix regression where clicking a link with JavaScript in href no longer triggers in Selenium 2.48.x on Firefox 42 (worked in 2.47.1). Provide reproducible behavior and ensure alert is triggered as in prior version.
🟡	🎫 #5678 🔴 Diagnose and resolve intermittent "Error: ConnectFailure (Connection refused)" when instantiating multiple ChromeDriver instances on Ubuntu with specified versions. Ensure subsequent ChromeDriver instances connect reliably without console errors.
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 Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	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 Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit logs: New critical actions like creating containers with names and labels are added without additional audit logging capturing identifiers (e.g., container names, labels, sessionIdentifier) to reconstruct events. Referred Code public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sessionRequest) { LOG.info("Starting session for " + sessionRequest.getDesiredCapabilities()); int port = runningInDocker ? 4444 : PortProber.findFreePort(); // Generate unique identifier for consistent naming between browser and recorder containers // Using browserName-timestamp-UUID to avoid conflicts in concurrent session creation String browserName = sessionRequest.getDesiredCapabilities().getBrowserName(); if (browserName != null && !browserName.isEmpty()) { browserName = browserName.toLowerCase(); } else { browserName = "unknown"; } long timestamp = System.currentTimeMillis(); String uniqueId = UUID.randomUUID().toString().substring(0, 8); String sessionIdentifier = String.format("%s-%d-%s", browserName, timestamp, uniqueId); try (Span span = tracer.getCurrentContext().createSpan("docker_session_factory.apply")) { AttributeMap attributeMap = tracer.createAttributeMap(); attributeMap.put(AttributeKey.LOGGER_CLASS.getKey(), this.getClass().getName()); String logMessage = runningInDocker ? "Creating container..." ... (clipped 5 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Error detail risk: The thrown DockerException includes the raw container name which may later encode internal naming patterns; confirm this message is not user-facing. Referred Code public Container apply(ContainerConfig info) { this.protocol.getImage(info.getImage().getName()); // Convert ContainerConfig to JSON and adapt for API version Map<String, Object> requestJson = JSON.toType(JSON.toJson(info), MAP_TYPE); Map<String, Object> adaptedRequest = adapter.adaptContainerCreateRequest(requestJson); // Build the URL with optional name parameter String url = String.format("/v%s/containers/create", apiVersion); if (info.getName() != null && !info.getName().trim().isEmpty()) { String containerName = info.getName().trim(); try { String encodedName = URLEncoder.encode(containerName, StandardCharsets.UTF_8.toString()); url += "?name=" + encodedName; } catch (UnsupportedEncodingException e) { throw new DockerException("Failed to encode container name: " + containerName, e); } } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

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

---

Previous compliance checks

Compliance check up to commit c701bc1

Security Compliance
⚪	Unencoded query param Description: Container name is appended to the Docker create URL without URL-encoding, so names containing spaces or reserved characters could break the request or enable request smuggling in edge cases; encode the 'name' query parameter. CreateContainer.java [66-69] Referred Code String url = String.format("/v%s/containers/create", apiVersion); if (info.getName() != null && !info.getName().isEmpty()) { url += "?name=" + info.getName(); }
Ticket Compliance
🟡	🎫 #5678 🔴 Investigate and resolve repeated "Error: ConnectFailure (Connection refused)" after instantiating multiple ChromeDriver instances on Ubuntu 16.04 with Chrome 65 / ChromeDriver 2.35 and Selenium 3.9.0. Ensure that subsequent ChromeDriver instantiations do not log connection refused errors, while the first instance works normally. ⚪ Provide guidance or fixes specific to environment (Docker or non-Docker) if relevant.
🟡	🎫 #1234 🔴 Fix regression where Selenium 2.48.0/2.48.2 with Firefox 42 does not trigger JavaScript in link href on click(), which worked in 2.47.1. ⚪ Verify behavior via test case reproducing alert trigger.
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 Learn more about managing compliance generic rules or creating your own custom rules
	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 Learn more about managing compliance generic rules or creating your own custom rules
	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 Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Action Logging: New critical actions (container creation with names/labels) were added without explicit audit logging of key context (e.g., container name, labels) to ensure reconstructability. Referred Code // Generate container name: browser-<browserName>-<timestamp>-<uuid> String browserName = stereotype.getBrowserName().toLowerCase(); String containerName = String.format("browser-%s-%s", browserName, sessionIdentifier); ContainerConfig containerConfig = image(browserImage) .env(browserContainerEnvVars) .shmMemorySize(browserContainerShmMemorySize) .network(networkName) .devices(devices) .applyHostConfig(hostConfig, hostConfigKeys) .labels(composeLabels) .name(containerName); Optional<DockerAssetsPath> path = ofNullable(this.assetsPath); if (path.isPresent() && videoImage == null && recordVideoForSession(sessionCapabilities)) { containerConfig = containerConfig.bind(Collections.singletonMap(this.assetsPath.getHostPath(), "/videos")); } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: URL Encoding: The container name is appended to the create URL without URL-encoding, which may fail for edge-case characters or empty values not validated. Referred Code String url = String.format("/v%s/containers/create", apiVersion); if (info.getName() != null && !info.getName().isEmpty()) { url += "?name=" + info.getName(); } HttpResponse res = DockerMessages.throwIfNecessary( client.execute( new HttpRequest(POST, url) .addHeader("Content-Type", JSON_UTF_8) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Input Validation: Externalized inputs (container name and labels) are forwarded to Docker without explicit validation/sanitization, which may cause API errors or label/name policy violations. Referred Code String url = String.format("/v%s/containers/create", apiVersion); if (info.getName() != null && !info.getName().isEmpty()) { url += "?name=" + info.getName(); } HttpResponse res = DockerMessages.throwIfNecessary( client.execute( new HttpRequest(POST, url) .addHeader("Content-Type", JSON_UTF_8) Learn more about managing compliance generic rules or creating your own custom rules

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ Sanitize browser name for container Suggestion Impact: The commit changed the container naming to use a precomputed sessionIdentifier and removed direct use of the raw browser name in container names, effectively avoiding invalid characters in container names. While it did not implement the exact sanitization regex, it addressed the core concern by not embedding the browser name in the container name at all. code diff: // Generate container name: browser-<browserName>-<timestamp>-<uuid> - String browserName = stereotype.getBrowserName().toLowerCase(); - String containerName = String.format("browser-%s-%s", browserName, sessionIdentifier); + String containerName = String.format("browser-%s", sessionIdentifier); ContainerConfig containerConfig = image(browserImage) @@ -380,8 +386,7 @@ Map<String, String> volumeBinds = Collections.singletonMap(hostPath, "/videos"); // Generate container name: recorder-<browserName>-<timestamp>-<uuid> - String browserName = stereotype.getBrowserName().toLowerCase(); - String containerName = String.format("recorder-%s-%s", browserName, sessionIdentifier); + String containerName = String.format("recorder-%s", sessionIdentifier); Sanitize the browser name by replacing characters invalid for Docker container names to prevent potential creation failures. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [314-316] -// Generate container name: browser-<browserName>-<timestamp>-<uuid> -String browserName = stereotype.getBrowserName().toLowerCase(); +// Generate container name: browser-<browserName>-<sessionIdentifier> +String browserName = stereotype.getBrowserName().toLowerCase().replaceAll("[^a-z0-9_.-]", "-"); String containerName = String.format("browser-%s-%s", browserName, sessionIdentifier); [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the browser name is not sanitized before being used in a Docker container name, which could lead to container creation failures if the name contains invalid characters.	Medium
	Use full UUID for uniqueness To prevent potential name collisions during concurrent session creation, replace the current session identifier generation logic (timestamp + short UUID) with a full UUID. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [162-165] -// Using timestamp + short UUID to avoid conflicts in concurrent session creation -long timestamp = System.currentTimeMillis(); -String uniqueId = UUID.randomUUID().toString().substring(0, 6); -String sessionIdentifier = timestamp + "-" + uniqueId; +// Using a UUID to avoid conflicts in concurrent session creation +String sessionIdentifier = UUID.randomUUID().toString(); Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential race condition in sessionIdentifier generation that could lead to container name collisions and failures under concurrent load, which is a valid concern for the new container naming feature.	Medium
Learned best practice	✅ Validate and encode URL parameter Suggestion Impact: The commit trims the name, checks for non-empty after trimming, URL-encodes it using UTF-8, and updates the URL accordingly. It also adds error handling for encoding. code diff: // Build the URL with optional name parameter String url = String.format("/v%s/containers/create", apiVersion); - if (info.getName() != null && !info.getName().isEmpty()) { - url += "?name=" + info.getName(); + if (info.getName() != null && !info.getName().trim().isEmpty()) { + String containerName = info.getName().trim(); + try { + String encodedName = URLEncoder.encode(containerName, StandardCharsets.UTF_8.toString()); + url += "?name=" + encodedName; + } catch (UnsupportedEncodingException e) { + throw new DockerException("Failed to encode container name: " + containerName, e); + } } Validate and URL-encode the optional container name to avoid invalid requests and injection issues. Reject empty/whitespace-only names and percent-encode the value. java/src/org/openqa/selenium/docker/client/CreateContainer.java [66-69] String url = String.format("/v%s/containers/create", apiVersion); -if (info.getName() != null && !info.getName().isEmpty()) { - url += "?name=" + info.getName(); +String name = info.getName(); +if (name != null) { + name = name.trim(); + if (!name.isEmpty()) { + String encoded = java.net.URLEncoder.encode(name, java.nio.charset.StandardCharsets.UTF_8); + url += "?name=" + encoded; + } } [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Guard external API and I/O operations with targeted validation and proper encoding when building request URLs.	Low
Update