chart(add): Grid scaler use trigger auth to secure GraphQL endpoint

[VietND96]

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

• Use resource TriggerAuthentication in KEDA to hide GraphQL endpoint URL and basic auth.
• Use new scaler parameter to provide username, password instead of putting in URL - https://github.com/SeleniumHQ/docker-selenium/blob/trunk/.keda/scalers/selenium-grid-scaler.md
• Fixes [🚀 Feature]: Selenium grid Authentication password is stored as plain text in Helm chat values.yaml file #2391

Set config key basicAuth.embeddedUrl is true will be used to put basic auth credentials in URL again

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, tests

---

Description

• Added basic authentication to various scripts and components to secure GraphQL endpoints.
• Removed basic authentication credentials from URLs and implemented them as headers.
• Enhanced error handling and logging for authentication-related issues.
• Updated Helm templates to support new authentication and autoscaling configurations.
• Added tests to verify the correct handling of authentication and URL construction.
• Updated documentation to reflect new features and security recommendations.

---

Changes walkthrough 📝

	Relevant files
Enhancement	12 files check-grid.sh Add basic authentication to grid status check                        Base/check-grid.sh Added basic authentication header to curl command. Encoded username and password in base64 for authentication. +2/-1      video.sh Enhance video recording with GraphQL authentication            Video/video.sh Added handling for GraphQL endpoint authentication and errors. Updated video recording logic with authentication checks. +8/-1      video_graphQLQuery.sh Secure GraphQL queries with basic authentication                  Video/video_graphQLQuery.sh Added basic authentication to GraphQL queries. Returned additional endpoint status and URL information. +6/-1      video_gridUrl.sh Update grid URL handling with authentication                          Video/video_gridUrl.sh Removed basic authentication from grid URL construction. Added basic authentication header to curl command. +2/-5      distributorProbe.sh Secure distributor probe with authentication                          charts/selenium-grid/configs/distributor/distributorProbe.sh Added basic authentication to GraphQL endpoint checks. +6/-5      nodeGridUrl.sh Refactor node grid URL handling                                                    charts/selenium-grid/configs/node/nodeGridUrl.sh Removed basic authentication from grid URL. Added basic authentication header to curl command. +9/-13    nodePreStop.sh Enhance node pre-stop with authentication                                charts/selenium-grid/configs/node/nodePreStop.sh Added basic authentication to node pre-stop signal. Improved logging for authentication errors. +13/-5    nodeProbe.sh Secure node probe with authentication                                        charts/selenium-grid/configs/node/nodeProbe.sh Added basic authentication to node probe checks. Improved error handling for authentication issues. +15/-2    routerGraphQLUrl.sh Simplify router GraphQL URL handling                                          charts/selenium-grid/configs/router/routerGraphQLUrl.sh Removed basic authentication from router GraphQL URL. +1/-4      routerProbe.sh Secure router probe with authentication                                    charts/selenium-grid/configs/router/routerProbe.sh Added basic authentication to router probe checks. +2/-1      _helpers.tpl Enhance templates with authentication support                        charts/selenium-grid/templates/_helpers.tpl Added template for basic authentication secrets. Updated autoscaling authentication reference handling. +14/-4    basic-auth-secret.yaml Add basic authentication secret template                                  charts/selenium-grid/templates/basic-auth-secret.yaml Added new template for basic authentication secret. +21/-0
Tests	1 files test.py Add tests for basic authentication and URL handling            tests/charts/templates/test.py Added tests for basic authentication handling. Verified GraphQL URL construction without basic auth in URL. +44/-16
Documentation	2 files CONFIGURATION.md Update configuration documentation for new features            charts/selenium-grid/CONFIGURATION.md Documented new basic authentication and autoscaling options. +3/-3      README.md Update README with new scaler and security info                    charts/selenium-grid/README.md Added section for new Selenium Grid Scaler implementation. Updated security recommendations for Grid URL. +8/-2

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[qodo-code-review]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 Security concerns Sensitive information exposure: The changes include modifications to how authentication credentials are handled and stored. While the implementation appears to use secrets and base64 encoding, a thorough review should be conducted to ensure that sensitive information like usernames and passwords are not inadvertently exposed in logs, error messages, or insecure configurations. Special attention should be paid to the new basic-auth-secret.yaml and trigger-auth.yaml files, as well as any changes to environment variables that might contain sensitive data.

⚡ Key issues to review Error Handling New error handling for GraphQL endpoint authentication and 404 errors has been added. This should be carefully reviewed to ensure it covers all possible error scenarios and provides appropriate feedback. Security Configuration A new template for basic authentication secret has been added. This should be thoroughly reviewed to ensure it's implemented securely and doesn't expose sensitive information. Authentication Configuration A new TriggerAuthentication resource for KEDA has been added. This should be carefully reviewed to ensure it's correctly configured and securely references the necessary secrets.

[qodo-code-review]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Add a check for the existence of username and password values before creating the basic auth secret Consider adding a check for the existence of username and password values before creating the secret. This will prevent creating an empty secret if the values are not provided. charts/selenium-grid/templates/basic-auth-secret.yaml [17-20] -{{- if eq .Values.basicAuth.enabled true }} +{{- if and (eq .Values.basicAuth.enabled true) .Values.basicAuth.username .Values.basicAuth.password }} SE_ROUTER_USERNAME: {{ .Values.basicAuth.username | b64enc }} SE_ROUTER_PASSWORD: {{ .Values.basicAuth.password | b64enc }} {{- end }} Apply this suggestion Suggestion importance[1-10]: 9 Why: This suggestion is important as it prevents the creation of an empty secret, which could lead to configuration issues or security concerns if the username and password are not provided.	9
Security	Use a more secure method for handling credentials Consider using a more secure method to store and retrieve credentials, such as using environment variables directly or a secrets management system, instead of encoding them in base64. charts/selenium-grid/configs/distributor/distributorProbe.sh [10] -BASIC_AUTH="$(echo -n "${SE_ROUTER_USERNAME}:${SE_ROUTER_PASSWORD}" | base64)" +BASIC_AUTH="${SE_ROUTER_USERNAME}:${SE_ROUTER_PASSWORD}" Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion addresses a security concern by recommending a more secure method for handling credentials, which is crucial for maintaining the integrity and confidentiality of sensitive information.	8
Enhancement	Add a condition to check if basic authentication is enabled before creating the TriggerAuthentication resource Consider adding a condition to check if basic authentication is enabled before creating the TriggerAuthentication resource. This will prevent potential errors if basic auth is disabled but KEDA is enabled. charts/selenium-grid/templates/trigger-auth.yaml [1] -{{- if and (eq (include "seleniumGrid.useKEDA" $) "true") (not $.Values.autoscaling.authenticationRef.name) }} +{{- if and (eq (include "seleniumGrid.useKEDA" $) "true") (not $.Values.autoscaling.authenticationRef.name) (eq $.Values.basicAuth.enabled true) }} Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion is valid as it prevents potential errors by ensuring that the TriggerAuthentication resource is only created when basic authentication is enabled, which is a logical enhancement to the existing condition.	8
	Add conditions to check if the URLs are defined before adding them to the secret Consider adding a condition to check if the URLs are defined before adding them to the secret. This will prevent adding empty values to the secret if the URLs are not set. charts/selenium-grid/templates/secrets.yaml [24-25] +{{- if (include "seleniumGrid.url" $) }} SE_NODE_GRID_URL: {{ include "seleniumGrid.url" $ | b64enc }} +{{- end }} +{{- if (include "seleniumGrid.graphqlURL" $) }} SE_NODE_GRID_GRAPHQL_URL: {{ include "seleniumGrid.graphqlURL" $ | b64enc }} +{{- end }} Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion is beneficial as it ensures that only defined URLs are added to the secret, preventing the inclusion of empty values which could lead to misconfigurations.	8
	Improve error handling and logging Consider using a more robust method for error handling and logging, such as creating separate functions for different error scenarios and using a logging framework. charts/selenium-grid/configs/node/nodePreStop.sh [38-42] +handle_error() { + local error_code=$1 + local error_message=$2 + echo "$(date +%FT%T%Z) [${probe_name}] - Error ${error_code}: ${error_message}" +} + if [ "${grid_check}" = "401" ]; then - echo "$(date +%FT%T%Z) [${probe_name}] - Hub/Router requires authentication. Please check SE_ROUTER_USERNAME and SE_ROUTER_PASSWORD." + handle_error 401 "Hub/Router requires authentication. Please check SE_ROUTER_USERNAME and SE_ROUTER_PASSWORD." elif [ "${grid_check}" = "404" ]; then - echo "$(date +%FT%T%Z) [${probe_name}] - Hub/Router endpoint could not be found. Please check the endpoint ${grid_url}" + handle_error 404 "Hub/Router endpoint could not be found. Please check the endpoint ${grid_url}" fi Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion enhances code maintainability and readability by proposing a structured approach to error handling and logging, which is beneficial but not critical.	7
	Improve GraphQL response handling and parsing Consider using a more robust method for handling and parsing the GraphQL response, such as using a JSON parsing tool like jq for better error handling and data extraction. Video/video_graphQLQuery.sh [30-34] -endpoint_checks=$(curl --noproxy "*" -m ${max_time} -k -X POST \ +response=$(curl --noproxy "*" -m ${max_time} -k -X POST \ -H "Content-Type: application/json" \ -H "Authorization: Basic ${BASIC_AUTH}" \ --data '{"query":"{ session (id: \"'${SESSION_ID}'\") { id, capabilities, startTime, uri, nodeId, nodeUri, sessionDurationMillis, slot { id, stereotype, lastStarted } } } "}' \ - -s "${GRAPHQL_ENDPOINT}" -o "/tmp/graphQL_${SESSION_ID}.json" -w "%{http_code}") + -s "${GRAPHQL_ENDPOINT}") +endpoint_checks=$(echo "${response}" | jq -r '.errors | length') +if [ "${endpoint_checks}" -eq 0 ]; then + echo "${response}" > "/tmp/graphQL_${SESSION_ID}.json" + endpoint_checks=200 +else + endpoint_checks=500 +fi + Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion improves the robustness of the code by using a JSON parsing tool for better error handling and data extraction, enhancing reliability but not addressing a critical issue.	7
Best practice	Improve handling of environment variables Consider using a more robust method for handling environment variables, such as using default values or checking if they are set before using them. Video/video.sh [205-209] return_list=($(bash ${VIDEO_CONFIG_DIRECTORY}/video_graphQLQuery.sh "$session_id")) -caps_se_video_record="${return_list[0]}" -video_file_name="${return_list[1]}.mp4" -endpoint_status="${return_list[2]}" -endpoint_url="${return_list[3]}" +caps_se_video_record="${return_list[0]:-false}" +video_file_name="${return_list[1]:-unknown}.mp4" +endpoint_status="${return_list[2]:-unknown}" +endpoint_url="${return_list[3]:-unknown}" Apply this suggestion Suggestion importance[1-10]: 6 Why: The suggestion improves code robustness by ensuring default values are used for environment variables, which is a good practice but not essential for functionality.	6

💡 Need additional feedback ? start a PR chat

[VietND96]

@Doofus100500, if removing basic auth putting in URL when constructing env var SE_NODE_GRID_URL. Do you think it might cause any issue for Playwright tests rely on CDP Websocket URL in node capabilities?

[VietND96]

To balance any requirement, config key basicAuth.embeddedUrl will be used to embed basic auth credentials to SE_NODE_GRID_URL if needed

[Doofus100500]

@VietND96 Removed basic auth from the URL in the SE_NODE_GRID_URL variable in the cluster with basic authentication, the Playwright test passed successfully

[VietND96]

Thank you for your confirmation. I think we are good to go with this change

[farioas]

Looks like it's breaking change in helm 0.36.0.

After upgrade, nodes are not scaled, while in KEDA we have this exception:

2024-09-23T10:46:26Z	ERROR	scale_handler	Error getting scaler metrics and activity, but continue	{"scaledJob.Name": "selenium-grid-selenium-firefox-node", "Scaler": "*scalers.seleniumGridScaler:", "error": "error requesting selenium grid endpoint: selenium grid returned 401"}
github.com/kedacore/keda/v2/pkg/scaling.(*scaleHandler).getScaledJobMetrics
	/workspace/pkg/scaling/scale_handler.go:853
github.com/kedacore/keda/v2/pkg/scaling.(*scaleHandler).isScaledJobActive
	/workspace/pkg/scaling/scale_handler.go:897
github.com/kedacore/keda/v2/pkg/scaling.(*scaleHandler).checkScalers
	/workspace/pkg/scaling/scale_handler.go:262
github.com/kedacore/keda/v2/pkg/scaling.(*scaleHandler).startScaleLoop
	/workspace/pkg/scaling/scale_handler.go:182

This is happened because existing implementation of selenium grid scaler in KEDA 2.15.1 expects to receive:

graphql-url: base64 encoded value of GraphQL URL

Fyi, this change wasn't merged kedacore/keda#6169

[VietND96]

@farioas, if there is no dependency, can you replace registry and tag to use KEDA components in our build https://github.com/SeleniumHQ/docker-selenium/blob/trunk/.keda/README.md

[VietND96]

@farioas, can you also try to enable basicAuth.embeddedUrl is true, basic auth will be embedded in GraphQL endpoint as the old format.

[VietND96]

Ok, looks like basicAuth.embeddedUrl not worked for GraphQL endpoint. I will give a fix in 0.36.1 soon

[farioas]

It was 6hrs ago, so I tried to play with embeddedUrl, but it didn't work. So I rolled back to 0.35.2 and pinned image to 4.25.0-20240922.