Fix node.lifecycle not being rendered from values file

[brunobritorj]

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

Fixes #2419, an issue when rendering <chromeNode|edgeNode|firefoxNode>.lifecycle from Helm values. The logic seems to be broken at that point

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

Bug fix

---

Description

• Fixed an issue where the lifecycle configuration for nodes was not being rendered correctly from the Helm values file.
• Updated the condition to properly check for the presence of .node.lifecycle and merge configurations accordingly.

---

Changes walkthrough 📝

	Relevant files
Bug fix	_helpers.tpl Fix lifecycle rendering logic for node configuration          charts/selenium-grid/templates/_helpers.tpl Fixed the condition to check if .node.lifecycle is present. Ensured lifecycle configuration is merged correctly when .node.lifecycle is defined. +1/-1

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Logic Change The condition for merging the lifecycle configuration has been inverted. This change needs to be carefully reviewed to ensure it correctly handles all possible scenarios.

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Best practice	Use a more explicit check for the existence of a key in the configuration Consider using the hasKey function to check if .node.lifecycle exists instead of relying on the truthiness of the value. This can prevent potential issues if .node.lifecycle is defined but empty. charts/selenium-grid/templates/_helpers.tpl [785-787] -{{- if .node.lifecycle -}} +{{- if hasKey .node "lifecycle" -}} {{- $lifecycle = mergeOverwrite ($lifecycle | fromYaml) (tpl (toYaml .node.lifecycle) $ | fromYaml) | toYaml -}} {{- end -}} Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion to use hasKey for checking the existence of .node.lifecycle is a good practice as it explicitly checks for the presence of the key, preventing potential issues if the key is defined but empty. This enhances the robustness of the code.	8
Possible issue	Add a null check for the configuration value before processing it Consider adding a check to ensure that .node.lifecycle is not null before merging it. This can prevent potential errors if .node.lifecycle is explicitly set to null in the values file. charts/selenium-grid/templates/_helpers.tpl [785-787] -{{- if .node.lifecycle -}} +{{- if and .node.lifecycle (ne .node.lifecycle null) -}} {{- $lifecycle = mergeOverwrite ($lifecycle | fromYaml) (tpl (toYaml .node.lifecycle) $ | fromYaml) | toYaml -}} {{- end -}} Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding a null check for .node.lifecycle can prevent errors if the value is explicitly set to null, which is a reasonable safeguard. However, the likelihood of this scenario may be low, so the impact is moderate.	7

💡 Need additional feedback ? start a PR chat

[VietND96]

Thank you, @brunobritorj!