Skip to content

Conversation

@VietND96
Copy link
Member

@VietND96 VietND96 commented Nov 20, 2025

User description

🔗 Related Issues

💥 What does this PR do?

Continue of #16599, #16613 - manage and group dynamic containers in Dynamic Grid under a compose stack (compatible when running with Docker Compose, Podman, or other platforms based on grouping labels defined by the user).

Enhanced filtering to support multiple orchestration systems:

  • Docker Compose: com.docker.compose.project
  • Podman Compose: io.podman.compose.project
  • User-defined grouping labels via CLI --docker-grouping-labels or TOML config key grouping-labels

✅ Platform agnostic - Works with any container orchestration system
✅ User configurable - No code changes needed for new platforms
✅ Backward compatible - Existing setups continue to work
✅ Flexible - Supports multiple custom labels simultaneously
✅ Safe - Only project labels are copied, service labels are excluded

Makes the Dynamic Grid Docker integration truly platform-independent and ready for any container orchestration system.

🔧 Implementation Notes

💡 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

  • Add configurable grouping labels for Docker containers

  • Support multiple orchestration systems (Docker Compose, Podman Compose)

  • Allow user-defined custom labels via CLI and TOML config

  • Filter and apply only project-level labels to containers


Diagram Walkthrough

flowchart LR
  A["DockerFlags"] -->|"adds grouping-labels parameter"| B["DockerOptions"]
  B -->|"reads custom labels from config"| C["getGroupingLabels"]
  C -->|"filters Docker/Podman/custom labels"| D["DockerSessionFactory"]
  D -->|"applies labels to containers"| E["Browser & Video Containers"]
Loading

File Walkthrough

Relevant files
Configuration changes
DockerFlags.java
Add CLI parameter for custom grouping labels                         

java/src/org/openqa/selenium/grid/node/docker/DockerFlags.java

  • Add new --docker-grouping-labels CLI parameter
  • Support TOML config key grouping-labels
  • Allow users to specify custom labels for container grouping
  • Example labels: azure.container.group, aws.ecs.cluster
+11/-0   
Enhancement
DockerOptions.java
Enhance label filtering for multiple orchestration systems

java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java

  • Rename getComposeLabels() to getGroupingLabels() for clarity
  • Enhance filtering to support Docker Compose, Podman Compose, and
    custom labels
  • Read custom label keys from configuration
  • Filter labels to include only project identifiers, excluding
    service-specific labels
  • Update method calls to use new naming convention
+21/-5   
DockerSessionFactory.java
Refactor to use generic grouping labels                                   

java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java

  • Rename composeLabels field to groupingLabels throughout class
  • Remove hardcoded com.docker.compose.oneoff=False label logic
  • Simplify label handling to use filtered grouping labels directly
  • Update constructor and container creation methods
+5/-9     

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@selenium-ci selenium-ci added B-grid Everything grid and server related C-java Java Bindings labels Nov 20, 2025
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 20, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Label injection risk

Description: User-supplied label keys from configuration are copied directly to container labels, which
could enable label injection to unintentionally join containers to existing compose/podman
projects or leak sensitive grouping metadata; consider validating/whitelisting allowed
keys or scoping with a Selenium-specific prefix.
DockerOptions.java [270-289]

Referred Code
List<String> customLabelKeys =
    config.getAll(DOCKER_SECTION, "grouping-labels").orElseGet(Collections::emptyList);

Map<String, String> allLabels = info.get().getLabels();
// Filter for project/grouping labels that work across orchestration systems
// Keep only project identifiers, exclude service-specific labels to prevent
// exit monitoring in Docker Compose, Podman Compose, etc.
return allLabels.entrySet().stream()
    .filter(
        entry -> {
          String key = entry.getKey();
          // Docker Compose project label
          if (key.equals("com.docker.compose.project")) return true;
          // Podman Compose project label
          if (key.equals("io.podman.compose.project")) return true;
          // Custom user-defined grouping labels
          if (customLabelKeys.contains(key)) return true;
          return false;
        })
    .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
Ticket Compliance
🟡
🎫 #1234
🔴 Investigate and resolve why click() does not trigger JavaScript in an anchor href in
Selenium 2.48.x (works in 2.47.1) on Firefox 42.
Provide a fix or regression handling specific to Firefox driver behavior so that alert is
triggered as in 2.47.1.
Verify behavior with the provided test case/videos.
🟡
🎫 #5678
🔴 Diagnose "Error: ConnectFailure (Connection refused)" when instantiating multiple
ChromeDriver instances on Ubuntu 16.04, Chrome 65, ChromeDriver 2.35, Selenium 3.9.0.
Implement a fix or provide configuration/workaround to prevent connection failures on
subsequent ChromeDriver instantiations.
Validate that subsequent driver instances start 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: 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:
Missing auditing: New logic that reads and applies container grouping labels is not accompanied by any
logging to audit which labels were used or applied, which could hinder reconstructing
actions.

Referred Code
// Get custom grouping labels from configuration
List<String> customLabelKeys =
    config.getAll(DOCKER_SECTION, "grouping-labels").orElseGet(Collections::emptyList);

Map<String, String> allLabels = info.get().getLabels();
// Filter for project/grouping labels that work across orchestration systems
// Keep only project identifiers, exclude service-specific labels to prevent
// exit monitoring in Docker Compose, Podman Compose, etc.
return allLabels.entrySet().stream()
    .filter(
        entry -> {
          String key = entry.getKey();
          // Docker Compose project label
          if (key.equals("com.docker.compose.project")) return true;
          // Podman Compose project label
          if (key.equals("io.podman.compose.project")) return true;
          // Custom user-defined grouping labels
          if (customLabelKeys.contains(key)) return true;
          return false;
        })
    .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

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:
Input validation: The code consumes user-provided grouping label keys from configuration without validating
for emptiness, duplicates, or malformed values, and proceeds silently with an empty set if
absent.

Referred Code
List<String> customLabelKeys =
    config.getAll(DOCKER_SECTION, "grouping-labels").orElseGet(Collections::emptyList);

Map<String, String> allLabels = info.get().getLabels();
// Filter for project/grouping labels that work across orchestration systems
// Keep only project identifiers, exclude service-specific labels to prevent
// exit monitoring in Docker Compose, Podman Compose, etc.
return allLabels.entrySet().stream()
    .filter(
        entry -> {
          String key = entry.getKey();
          // Docker Compose project label
          if (key.equals("com.docker.compose.project")) return true;
          // Podman Compose project label
          if (key.equals("io.podman.compose.project")) return true;
          // Custom user-defined grouping labels
          if (customLabelKeys.contains(key)) return true;
          return false;

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:
Unvalidated labels: User-configurable grouping labels from configuration are passed directly to container
label sets without normalization or validation, which could allow unintended or
conflicting labels.

Referred Code
ContainerConfig containerConfig =
    image(browserImage)
        .env(browserContainerEnvVars)
        .shmMemorySize(browserContainerShmMemorySize)
        .network(networkName)
        .devices(devices)
        .applyHostConfig(hostConfig, hostConfigKeys)
        .labels(groupingLabels)
        .name(containerName);

Learn more about managing compliance generic rules or creating your own custom rules

  • 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 Nov 20, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Improve filtering logic for performance
Suggestion Impact:The commit introduced a HashSet of grouping keys and replaced the multi-branch filter logic with a contains check against the Set, matching the suggested optimization.

code diff:

@@ -270,22 +272,14 @@
     List<String> customLabelKeys =
         config.getAll(DOCKER_SECTION, "grouping-labels").orElseGet(Collections::emptyList);
 
+    Set<String> groupingKeys = new HashSet<>(customLabelKeys);
+    groupingKeys.add("com.docker.compose.project");
+    groupingKeys.add("io.podman.compose.project");
+
     Map<String, String> allLabels = info.get().getLabels();
-    // Filter for project/grouping labels that work across orchestration systems
-    // Keep only project identifiers, exclude service-specific labels to prevent
-    // exit monitoring in Docker Compose, Podman Compose, etc.
+    // Filter for grouping labels that work across orchestration systems
     return allLabels.entrySet().stream()
-        .filter(
-            entry -> {
-              String key = entry.getKey();
-              // Docker Compose project label
-              if (key.equals("com.docker.compose.project")) return true;
-              // Podman Compose project label
-              if (key.equals("io.podman.compose.project")) return true;
-              // Custom user-defined grouping labels
-              if (customLabelKeys.contains(key)) return true;
-              return false;
-            })
+        .filter(entry -> groupingKeys.contains(entry.getKey()))
         .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

Refactor the label filtering logic to use a Set for grouping keys instead of a
List and multiple if conditions, improving lookup performance and code clarity.

java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java [269-289]

 // Get custom grouping labels from configuration
 List<String> customLabelKeys =
     config.getAll(DOCKER_SECTION, "grouping-labels").orElseGet(Collections::emptyList);
+
+Set<String> groupingKeys = new HashSet<>(customLabelKeys);
+groupingKeys.add("com.docker.compose.project");
+groupingKeys.add("io.podman.compose.project");
 
 Map<String, String> allLabels = info.get().getLabels();
 // Filter for project/grouping labels that work across orchestration systems
 // Keep only project identifiers, exclude service-specific labels to prevent
 // exit monitoring in Docker Compose, Podman Compose, etc.
 return allLabels.entrySet().stream()
-    .filter(
-        entry -> {
-          String key = entry.getKey();
-          // Docker Compose project label
-          if (key.equals("com.docker.compose.project")) return true;
-          // Podman Compose project label
-          if (key.equals("io.podman.compose.project")) return true;
-          // Custom user-defined grouping labels
-          if (customLabelKeys.contains(key)) return true;
-          return false;
-        })
+    .filter(entry -> groupingKeys.contains(entry.getKey()))
     .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

[Suggestion processed]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly proposes using a Set for more efficient lookups, which improves performance and code readability, aligning with best practices.

Low
Learned
best practice
Validate custom grouping labels

Validate that configured grouping-labels are non-empty strings and reject
invalid entries with a clear message to avoid silent misconfiguration.

java/src/org/openqa/selenium/grid/node/docker/DockerOptions.java [270-271]

 List<String> customLabelKeys =
-    config.getAll(DOCKER_SECTION, "grouping-labels").orElseGet(Collections::emptyList);
+    config.getAll(DOCKER_SECTION, "grouping-labels").orElseGet(Collections::emptyList)
+        .stream()
+        .map(String::trim)
+        .filter(s -> !s.isEmpty())
+        .collect(Collectors.toList());
+if (!config.getAll(DOCKER_SECTION, "grouping-labels").orElseGet(Collections::emptyList).isEmpty()
+    && customLabelKeys.isEmpty()) {
+  throw new IllegalArgumentException("Invalid docker grouping-labels: only blank values provided");
+}
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Guard external configuration values with validation and clear errors before use.

Low
Clarify groupingLabels purpose

Add a concise field-level comment clarifying that groupingLabels are
non-service-specific labels used to group dynamic containers across
orchestrators.

java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [108]

+// Labels that identify a project/group across orchestrators (e.g., Docker/Podman Compose, custom),
+// excluding service-specific labels. Used to group dynamic containers for lifecycle handling.
 private final Map<String, String> groupingLabels;
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Enforce accurate and consistent naming/documentation to match behavior and aid maintainability.

Low
  • Update

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@VietND96 VietND96 merged commit 32dc667 into trunk Nov 20, 2025
46 checks passed
@VietND96 VietND96 deleted the dynamic-grid-compose-stack branch November 20, 2025 17:26
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 Review effort 3/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants