Docker: Add Redis external datastore support for Distributor configuration#3137
Merged
Conversation
…ation Signed-off-by: Viet Nguyen Duc <[email protected]>
Contributor
Review Summary by QodoAdd Redis external datastore support for Distributor configuration
WalkthroughsDescription• Add Redis external datastore support for Distributor component • Introduce SE_DISTRIBUTOR_IMPLEMENTATION and SE_DISTRIBUTOR_BACKEND_URL environment variables • Update Helm chart values and configuration for Redis-backed Distributor • Create new docker-compose file for Redis-backed external datastore setup • Update documentation and environment variable references Diagramflowchart LR
A["Distributor Configuration"] --> B["SE_DISTRIBUTOR_IMPLEMENTATION"]
A --> C["SE_DISTRIBUTOR_BACKEND_URL"]
B --> D["RedisBackedDistributor"]
C --> E["Redis URL Connection"]
D --> F["Shared State Management"]
E --> F
F --> G["Zero-downtime Rolling Restarts"]
File Changes1. Distributor/start-selenium-grid-distributor.sh
|
Contributor
Code Review by Qodo
1. redis:latest image tag used
|
Comment on lines
+28
to
+30
| redis: | ||
| image: redis:latest | ||
| restart: always |
Contributor
There was a problem hiding this comment.
1. redis:latest image tag used 📘 Rule violation ⛨ Security
The new docker-compose file pins Redis to latest, which is an implicit mutable artifact and harms reproducibility/supply-chain safety. This violates the requirement to avoid implicit "latest" artifacts by pinning versions (preferably with digests).
Agent Prompt
## Issue description
The docker-compose definition uses `redis:latest`, which is a mutable tag and violates the requirement to avoid implicit "latest" artifacts.
## Issue Context
Compliance requires explicitly pinned versions (ideally with digests) to improve reproducibility and reduce supply-chain risk.
## Fix Focus Areas
- docker-compose-v3-full-grid-external-redis-backed.yml[28-30]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+160
to
+162
| if [ ! -z "$SE_DISTRIBUTOR_BACKEND_URL" ]; then | ||
| append_se_opts "--distributor-backend-url" "${SE_DISTRIBUTOR_BACKEND_URL}" | ||
| fi |
Contributor
There was a problem hiding this comment.
2. Redis url logged unmasked 🐞 Bug ⛨ Security
Distributor/start-selenium-grid-distributor.sh appends --distributor-backend-url using default logging, which prints the full URL; Redis URLs can embed credentials and will be exposed in container logs. This is inconsistent with other sensitive options (e.g., --registration-secret) which are masked.
Agent Prompt
## Issue description
`SE_DISTRIBUTOR_BACKEND_URL` is appended with `append_se_opts` using default logging, which prints the full backend URL to logs. Redis URLs may include credentials (e.g. `redis://:password@host:6379`), so this leaks secrets.
## Issue Context
`append_se_opts` supports masking by passing `log_message=false` (used for `--registration-secret`). The new `--distributor-backend-url` path should use the same masking, and the "already set" log branch should also respect masking to avoid leaking when `SE_OPTS` already contains the option.
## Fix Focus Areas
- Distributor/start-selenium-grid-distributor.sh[8-24]
- Distributor/start-selenium-grid-distributor.sh[144-162]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
783
to
+786
| # -- Enable external datastore for Session Map | ||
| enabled: false | ||
| # -- Backend for external datastore (supported: postgresql, redis). Details for each backend are described below config key | ||
| backend: postgresql | ||
| backend: redis |
Contributor
There was a problem hiding this comment.
3. Sessionmap default backend flip 🐞 Bug ☼ Reliability
Helm components.sessionMap.externalDatastore.backend default is changed to redis, which changes behavior for users who enable SessionMap external datastore without explicitly setting backend. Because the Redis dependency is disabled by default, this can lead to a broken/misconfigured deployment unless users also enable/configure Redis (or override back to postgresql).
Agent Prompt
## Issue description
The chart’s default for `components.sessionMap.externalDatastore.backend` was changed to `redis`. This is a behavior change that can break upgrades/configurations where users set only `components.sessionMap.externalDatastore.enabled=true` and rely on the previous default backend.
## Issue Context
The SessionMap template selects which env vars to render based on the `backend` key. With Redis as default and `redis.enabled=false` by default, enabling external datastore without additional settings can produce a non-working configuration.
## Fix Focus Areas
- charts/selenium-grid/values.yaml[782-799]
- charts/selenium-grid/values.yaml[2374-2384]
- charts/selenium-grid/templates/session-map-configmap.yaml[13-18]
- charts/selenium-grid/CONFIGURATION.md[315-318]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Checklist