Add guard against empty cloudInit in vm-instance app #646
Conversation
dosubot
bot
added
the
size:M
WalkthroughThe changes update the handling of the Changes
Sequence Diagram(s)sequenceDiagram
participant V as Values Input (.Values.cloudInit)
participant T as Template Engine
participant D as Deployment Process
V->>T: Provide cloudInit value
alt cloudInit value provided
T->>D: Inject secret reference for cloud-init data
else cloudInit not provided
T->>D: Insert default blank cloud-init user data
end
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/apps/vm-instance/README.md (1)
39-43: Improve Description Wording forexternalMethodThe table row for
externalMethoduses the phrase “passthrough the traffic” which can be improved for grammatical clarity. Consider rewording it to better reflect the intended verb usage (e.g. “pass through” rather than “passthrough”).-| `externalMethod` | specify method to passthrough the traffic to the virtual machine. Allowed values: `WholeIP` and `PortList` | `WholeIP` | +| `externalMethod` | specifies the method to pass through traffic to the virtual machine. Allowed values: `WholeIP` and `PortList` | `WholeIP` |🧰 Tools
🪛 LanguageTool
[grammar] ~42-~42: The word “passthrough” is a noun. The verb is spelled with a white space.
Context: ...externalMethod| specify method to passthrough the traffic to the virtual machine. All...(NOUN_VERB_CONFUSION)
packages/apps/vm-instance/values.yaml (1)
15-16: Refine Instance Profile Parameter DescriptionThe description for the
instanceProfileparameter can be made slightly clearer. Consider rephrasing it to include a definite subject for better readability.-## @param instanceProfile Virtual Machine preferences profile +## @param instanceProfile The virtual machine's preferences profile
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/apps/vm-instance/README.md(1 hunks)packages/apps/vm-instance/templates/vm.yaml(3 hunks)packages/apps/vm-instance/values.schema.json(1 hunks)packages/apps/vm-instance/values.yaml(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/apps/vm-instance/values.schema.json
🧰 Additional context used
🪛 LanguageTool
packages/apps/vm-instance/README.md
[grammar] ~42-~42: The word “passthrough” is a noun. The verb is spelled with a white space.
Context: ... externalMethod | specify method to passthrough the traffic to the virtual machine. All...
(NOUN_VERB_CONFUSION)
[style] ~50-~50: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ... of SSH public keys for authentication. Can be a single key or a list of keys. ...
(MISSING_IT_THERE)
🔇 Additional comments (4)
packages/apps/vm-instance/README.md (1)
49-51: Confirm Updated Default forcloudInitThe default for the
cloudInitparameter is now set to an empty string (""), replacing the older multi-line#cloud-configstring. This change is consistent with the new logic that provides a safeguard against referencing a non-existent secret when cloud-init is empty.🧰 Tools
🪛 LanguageTool
[style] ~50-~50: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ... of SSH public keys for authentication. Can be a single key or a list of keys. ...(MISSING_IT_THERE)
packages/apps/vm-instance/templates/vm.yaml (2)
1-6: Clarify Error Messages for Instance Type and Profile ChecksThe error messages have been updated to read “Specified instanceType does not exist in the cluster” and “Specified instanceProfile does not exist in the cluster”. These now offer improved clarity compared to the previous phrasing. The use of the
failfunction in the conditional checks is appropriate to halt deployment when an invalid value is provided.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
91-98: Guard Against Empty Cloud-Init User DataThe conditional branch in the
cloudInitNoCloudsection correctly distinguishes between cases when.Values.cloudInitis set and when it is empty. When empty, a default user data block is provided with a message indicating that no cloud-init configuration was intended. This effectively prevents the system from referencing a non-existent secret.packages/apps/vm-instance/values.yaml (1)
50-51: Update Default forcloudInitParameterThe
cloudInitparameter is now set to an empty string ("") rather than a multi-line cloud-config string. This update is consistent with the overall design change to provide a safeguard against accessing non-existent secrets when cloud-init data is not provided.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/apps/vm-instance/README.md (2)
42-42: Refine Terminology inexternalMethodDescriptionThe description currently uses "passthrough" as a verb. To improve clarity and adhere to proper grammatical usage, consider changing it to "pass through". For example, update the text to:
specify method to pass through the traffic to the virtual machine. Allowed values: 'WholeIP' and 'PortList'🧰 Tools
🪛 LanguageTool
[grammar] ~42-~42: The word “passthrough” is a noun. The verb is spelled with a white space.
Context: ...externalMethod| specify method to passthrough the traffic to the virtual machine. All...(NOUN_VERB_CONFUSION)
50-50: Improve Clarity in SSH Keys DescriptionThe description for
sshKeysis slightly fragmented. Consider revising it to a complete sentence for clarity. For example:
List of SSH public keys for authentication. It can be a single key or a list of keys.
This will enhance readability for users.🧰 Tools
🪛 LanguageTool
[style] ~50-~50: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ... of SSH public keys for authentication. Can be a single key or a list of keys. ...(MISSING_IT_THERE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/apps/vm-instance/README.md(1 hunks)packages/apps/vm-instance/values.schema.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/apps/vm-instance/values.schema.json
🧰 Additional context used
🪛 LanguageTool
packages/apps/vm-instance/README.md
[grammar] ~42-~42: The word “passthrough” is a noun. The verb is spelled with a white space.
Context: ... externalMethod | specify method to passthrough the traffic to the virtual machine. All...
(NOUN_VERB_CONFUSION)
[style] ~50-~50: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ... of SSH public keys for authentication. Can be a single key or a list of keys. ...
(MISSING_IT_THERE)
🔇 Additional comments (1)
packages/apps/vm-instance/README.md (1)
39-51: Confirm Consistent Default Values in Parameter TableThe default value for
cloudInithas been correctly updated to an empty string ("") to align with the new safeguard against referencing a non-existent secret. Ensure that this change is consistently reflected in the related files (values.schema.json,values.yaml, and VM template files) to prevent any configuration mismatches.🧰 Tools
🪛 LanguageTool
[grammar] ~42-~42: The word “passthrough” is a noun. The verb is spelled with a white space.
Context: ...externalMethod| specify method to passthrough the traffic to the virtual machine. All...(NOUN_VERB_CONFUSION)
[style] ~50-~50: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ... of SSH public keys for authentication. Can be a single key or a list of keys. ...(MISSING_IT_THERE)
| {{- else }} | ||
| userData: | | ||
| #cloud-config | ||
| final_message: Cloud-init user-data was left blank intentionally. |
There was a problem hiding this comment.
Do we really need this?
Cloud-init user-data might have diferent formats, it might be blocking for very specific distros.
Prevent the VM resource from referencing a non-existent secret when
sshKeysare set andcloudInitis set to empty.Summary by CodeRabbit