chart(breaking change): enable TLS and default annotations for ingress

[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

Motivation and Context

Inline config TLS for the Ingress resource is also considered as enable secure connection to the Ingress proxy.
For example, below is the config with using external TLS Secret for the Ingress resource and enable sub-chart NGINX Ingress Controller:

ingress:
  hostname: selenium-grid.prod.domain.com
  tls:
    - secretName: my-external-tls-secret
      hosts:
        - selenium-grid.prod.domain.com

ingress-ngnix:
    enabled: true

In case the Ingress resource is configured without hostname and tls, the incoming traffic access via global.K8S_PUBLIC_IP. When sub-chart ingress-nginx is enabled (deploy Ingress NGINX Controller together), the default TLS secret can also be assigned via ingress-nginx.controller.extraArgs.default-ssl-certificate.
For example (replace $RELEASENAME and $NAMESPACE with your values):

helm upgrade -i $RELEASENAME -n $NAMESPACE docker-selenium/selenium-grid \
  --set global.K8S_PUBLIC_IP=$(hostname -i) \
  --set tls.ingress.enabled=true \
  --set tls.nameOverride=my-external-tls-secret \
  --set ingress-nginx.enabled=true \
  --set ingress-nginx.controller.extraArgs.default-ssl-certificate=$NAMESPACE/my-external-tls-secret

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, Documentation, Tests

---

Description

• Enhanced TLS and SSL configuration in Helm chart tests, including setting for ingress.nginx.sslSecret and refactoring Helm command settings.
• Added SSL certificate verification in smoke tests by importing ssl module and integrating certificate verification in test_grid_is_up method.
• Improved secure connection handling and annotations in Helm templates, including support for sslSecret in ingress annotations.
• Updated Makefile for inline TLS configuration in chart tests.
• Updated documentation for TLS and SSL configurations with examples and improved explanations.
• Updated ingress and Jaeger ingress templates for better TLS secret handling.
• Added SSL passthrough and secret settings in values file.
• Removed default SSL certificate setting in test values for ingress-nginx controller.

---

Changes walkthrough 📝

	Relevant files
Enhancement	5 files chart_test.sh Enhance TLS and SSL configuration in Helm chart tests        tests/charts/make/chart_test.sh Added setting for ingress.nginx.sslSecret when using external TLS certificates. Refactored Helm command settings for different TLS configurations. Enhanced conditional logic for setting default SSL certificates. +24/-8    _helpers.tpl Improve secure connection handling and annotations in Helm templates charts/selenium-grid/templates/_helpers.tpl Enhanced logic for determining secure connections. Added support for sslSecret in ingress annotations. Improved handling of ingress.tls configurations. +14/-3    ingress.yaml Update ingress template for TLS secret handling                    charts/selenium-grid/templates/ingress.yaml Updated logic for setting secretName in ingress TLS configurations. +2/-2      jaeger-ingress.yaml Update Jaeger ingress template for TLS secret handling      charts/selenium-grid/templates/jaeger-ingress.yaml Updated logic for setting secretName in Jaeger ingress TLS configurations. +4/-4      values.yaml Add SSL passthrough and secret settings in values file      charts/selenium-grid/values.yaml Added sslPassthrough and sslSecret settings in ingress configuration. +2/-0
Tests	1 files __init__.py Add SSL certificate verification in smoke tests                    tests/SmokeTests/init.py Imported ssl module. Added client_verify_cert method to verify certificates. Integrated certificate verification in test_grid_is_up method. +8/-1
Configuration changes	2 files Makefile Update Makefile for inline TLS configuration in chart tests Makefile Updated chart test command to use inline TLS configuration. +1/-1      simplex-minikube.yaml Remove default SSL certificate setting in test values        tests/charts/refValues/simplex-minikube.yaml Removed default SSL certificate setting for ingress-nginx controller. +0/-3
Documentation	1 files README.md Update documentation for TLS and SSL configurations            charts/selenium-grid/README.md Updated documentation for TLS and SSL configurations. Added examples for using external TLS secrets and inline configurations. Improved explanations for secure communication settings. +48/-29

---

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

[qodo-merge-pro]

PR Reviewer Guide 🔍

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Possible Bug The variable SELENIUM_NAMESPACE is used in the command line argument for setting ingress.nginx.sslSecret but it is not defined or exported in the script or its environment. This might cause the script to fail or behave unexpectedly. Code Improvement The method client_verify_cert uses environment variables directly which might make the code less flexible and harder to test. Consider passing these as parameters to the function or class. Logic Error The logic for determining if seleniumGrid.ingress.secureConnection is true seems overly complex and might be prone to errors if not all conditions are properly met or understood.

[qodo-merge-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Enclose variable expansions in double quotes to ensure correct parsing It's recommended to use double quotes around variable expansions to prevent globbing and word splitting. This change ensures that the entire concatenated string is treated as a single argument, even if the variable values contain spaces or special characters. tests/charts/make/chart_test.sh [216] +--set ingress.nginx.sslSecret="${SELENIUM_NAMESPACE}/${EXTERNAL_TLS_SECRET_NAME}" - Apply this suggestion Suggestion importance[1-10]: 10 Why: This suggestion addresses a best practice in shell scripting to prevent globbing and word splitting, which can lead to unexpected behavior. It ensures that the entire concatenated string is treated as a single argument, even if the variable values contain spaces or special characters.	10
	Ensure robust default value handling in Helm template for ingress configuration To ensure that the default value is correctly used when .secretName is not set, explicitly check for emptiness using the empty function. This change makes the template logic more explicit and robust. charts/selenium-grid/templates/ingress.yaml [47] -secretName: {{ tpl (default (include "seleniumGrid.tls.fullname" $) .secretName) $ | quote }} +secretName: {{ tpl (default (include "seleniumGrid.tls.fullname" $) (if (empty .secretName) "" .secretName)) $ | quote }} Apply this suggestion Suggestion importance[1-10]: 8 Why: Explicitly checking for emptiness using the empty function makes the template logic more explicit and robust, ensuring that the default value is correctly used when .secretName is not set. This enhances the reliability of the configuration.	8
Error handling	Add error handling for the network request to improve robustness To handle potential exceptions from the requests.get call, such as network issues or invalid URLs, wrap the request in a try-except block. This will improve the robustness of the testing script by allowing it to handle errors gracefully. tests/SmokeTests/init.py [56] -response = requests.get(grid_url_status, verify=cert_path) +try: + response = requests.get(grid_url_status, verify=cert_path) +except requests.exceptions.RequestException as e: + print(f"Failed to connect to {grid_url_status}: {e}") Apply this suggestion Suggestion importance[1-10]: 9 Why: Adding error handling for the requests.get call is crucial for robustness, as it allows the script to handle potential network issues or invalid URLs gracefully. This improves the reliability of the testing script.	9
Maintainability	Improve readability and correctness of the ternary operation in the Helm template To ensure that the ternary operator is correctly applied and readable, it's beneficial to use parentheses around the entire condition before applying the ternary operator. This makes the template more readable and less prone to errors if the logic becomes more complex. charts/selenium-grid/templates/_helpers.tpl [12] -{{- or $.Values.tls.enabled $.Values.tls.ingress.enabled $.Values.tls.ingress.generateTLS (not (empty $.Values.ingress.tls)) | ternary "true" "" -}} +{{- (or $.Values.tls.enabled $.Values.tls.ingress.enabled $.Values.tls.ingress.generateTLS (not (empty $.Values.ingress.tls))) | ternary "true" "" -}} Apply this suggestion Suggestion importance[1-10]: 8 Why: Using parentheses around the entire condition before applying the ternary operator enhances readability and reduces the risk of errors in complex logic. This is a good practice for maintainability.	8