Helm chart: Manage TLS Certificate Externally

[declan-fitzpatrick]

User description

Description

The tls-cert-secret.yaml has limited functionality.

It only allows self signed cert generation if ingress is enabled, and tls is disabled. Alternatively, you have to pass the values of the certificate in as non-base64 literals, which causes an issue with the selenium.jks binary.

Motivation and Context

solves #2293

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• [x] I have read the contributing document.
• [x] My change requires a change to the documentation.
• [x] I have updated the documentation accordingly.
• [ ] I have added tests to cover my changes.
• [ ] All new and existing tests passed.

---

PR Type

Enhancement, Documentation

---

Description

• Added support for referencing an external TLS secret in the Helm chart.
• Updated the _nameHelpers.tpl to conditionally use the external TLS secret name if provided.
• Modified tls-cert-secret.yaml to skip secret creation when an external TLS secret name is specified.
• Updated values.yaml to include the externalSecretName field in the TLS configuration.
• Enhanced README documentation with instructions on how to use an external TLS secret.

---

Changes walkthrough 📝

	Relevant files
Enhancement	_nameHelpers.tpl Add support for external TLS secret reference                        charts/selenium-grid/templates/_nameHelpers.tpl Added conditional logic to use an external TLS secret name if provided. +4/-0      tls-cert-secret.yaml Conditional secret creation based on external TLS secret  charts/selenium-grid/templates/tls-cert-secret.yaml Added conditional logic to skip secret creation if an external TLS secret name is provided. +2/-0      values.yaml Add externalSecretName field to TLS configuration                charts/selenium-grid/values.yaml Added externalSecretName field to TLS configuration. +1/-0
Documentation	README.md Update README with external TLS secret reference                  charts/selenium-grid/README.md Updated documentation to include instructions for using an external TLS secret. +22/-2

---

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

[CLAassistant]

All committers have signed the CLA.

[qodo-code-review[bot]]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	2
🧪 Relevant tests	No
🔒 Security concerns	No
⚡ Key issues to review	Possible Bug: The PR introduces the ability to use an external TLS secret, but there is no validation to ensure that the external secret exists or is correctly formatted before it is used. This could lead to runtime errors if the secret is missing or misconfigured.
	Documentation Clarity: The documentation update in the README.md could be more explicit about the steps required to use an external TLS secret, including necessary commands and expected file formats.

[qodo-code-review[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Security	Add a security warning about using self-signed certificates in production Update the documentation to include a warning about the security risks of using self-signed certificates in production environments. charts/selenium-grid/README.md [609] -In the chart, there is directory [certs](./certs) contains the default certificate, private key (as PKCS8 format), and Java Keystore (JKS) to teach Java about secure connection (since we are using a non-standard CA) for your trial, local testing purpose. +In the chart, there is directory [certs](./certs) contains the default certificate, private key (as PKCS8 format), and Java Keystore (JKS) to teach Java about secure connection (since we are using a non-standard CA) for your trial, local testing purpose. **Note: Using self-signed certificates in production environments can expose you to security risks.** [ ] Apply this suggestion Suggestion importance[1-10]: 9 Why: Adding a security warning is crucial for informing users about the risks of using self-signed certificates in production environments, thus improving the overall security awareness.	9
Maintainability	Ensure secrets are only created when TLS is enabled and not managed externally Add a condition to check for the existence of the externalSecretName before creating the secret to avoid conflicts and ensure that secrets are not unnecessarily created when managed externally. charts/selenium-grid/templates/tls-cert-secret.yaml [1-4] -{{- if not .Values.tls.externalSecretName }} +{{- if and (not .Values.tls.externalSecretName) (.Values.tls.enabled) }} apiVersion: v1 kind: Secret metadata: [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: This suggestion enhances maintainability by adding a condition to check if TLS is enabled and not managed externally before creating the secret, preventing potential conflicts and unnecessary secret creation.	8
	Refactor the template to centralize and simplify the truncation and trimming logic Refactor the template to avoid redundancy by defining a helper template for the common truncation and suffix trimming logic. charts/selenium-grid/templates/_nameHelpers.tpl [148-150] -{{- tpl ( .Values.tls.externalSecretName ) $ | trunc 63 | trimSuffix "-" -}} -{{- tpl (default (include "seleniumGrid.component.name" (list "selenium-tls-secret" $)) .Values.tls.nameOverride) $ | trunc 63 | trimSuffix "-" -}} +{{- define "seleniumGrid.tls.trimmedName" -}} +{{- tpl . $ | trunc 63 | trimSuffix "-" -}} +{{- end -}} +{{- if .Values.tls.externalSecretName }} +{{- include "seleniumGrid.tls.trimmedName" .Values.tls.externalSecretName -}} +{{- else }} +{{- include "seleniumGrid.tls.trimmedName" (default (include "seleniumGrid.component.name" (list "selenium-tls-secret" $)) .Values.tls.nameOverride) -}} +{{- end }} + [ ] Apply this suggestion Suggestion importance[1-10]: 7 Why: The refactoring suggestion improves maintainability by reducing redundancy and centralizing common logic, making the template easier to manage and understand. However, it is not a critical change.	7
Best practice	Improve the clarity of variable names related to TLS configuration Consider using a more descriptive variable name for the Helm template that generates the TLS secret name. The current name externalSecretName is generic and might not clearly convey its purpose related to TLS configuration. charts/selenium-grid/templates/_nameHelpers.tpl [147-151] -{{- if .Values.tls.externalSecretName }} -{{- tpl ( .Values.tls.externalSecretName ) $ | trunc 63 | trimSuffix "-" -}} +{{- if .Values.tls.tlsExternalSecretName }} +{{- tpl ( .Values.tls.tlsExternalSecretName ) $ | trunc 63 | trimSuffix "-" -}} {{- else }} {{- tpl (default (include "seleniumGrid.component.name" (list "selenium-tls-secret" $)) .Values.tls.nameOverride) $ | trunc 63 | trimSuffix "-" -}} {{- end }} [ ] Apply this suggestion Suggestion importance[1-10]: 6 Why: The suggestion improves the clarity of the variable name, making it more descriptive and specific to TLS configuration. However, it is a minor improvement and not crucial for functionality.	6

[VietND96]

Thanks for your feedback and initial contribution. I believe PR #2306 is able to do needful for the requirement. If have time you can walk through the details. I am closing this PR.