[virtual-machine] Fix cloudInit and sshKeys - #1175
Conversation
WalkthroughThe cloud-init and SSH key handling logic in Helm templates for virtual-machine and vm-instance was refactored. Now, if either Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HelmChart
participant Kubernetes
User->>HelmChart: Deploy VM with .Values.cloudInit and/or .Values.sshKeys
HelmChart->>HelmChart: Check if .Values.cloudInit or .Values.sshKeys is present
alt .Values.cloudInit present
HelmChart->>Kubernetes: Create Secret with user-provided cloudInit
else .Values.sshKeys present
HelmChart->>Kubernetes: Create Secret with generated cloud-config (SSH keys)
end
HelmChart->>Kubernetes: Create VM referencing cloud-init Secret as volume
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
868a4b6 to
ed37fc7
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/apps/virtual-machine/templates/secret.yaml (1)
8-9: Trailing whitespace violates YAML-lintThe extra space after
quote $v }}triggers the reported lint error.
Trim it to keep the chart CI-friendly.- key{{ $k }}: {{ quote $v }} + key{{ $k }}: {{ quote $v }}packages/apps/vm-instance/templates/secret.yaml (1)
8-9: Trim trailing spaces to satisfy YAML-lint- key{{ $k }}: {{ quote $v }} + key{{ $k }}: {{ quote $v }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/apps/virtual-machine/Chart.yaml(1 hunks)packages/apps/virtual-machine/templates/secret.yaml(1 hunks)packages/apps/virtual-machine/templates/vm.yaml(2 hunks)packages/apps/vm-instance/templates/secret.yaml(1 hunks)packages/apps/vm-instance/templates/vm.yaml(2 hunks)packages/system/dashboard/images/dashboard/Dockerfile(1 hunks)packages/system/dashboard/images/kubeapps-apis/Dockerfile(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/apps/virtual-machine/templates/secret.yaml
[error] 26-26: trailing spaces
(trailing-spaces)
packages/apps/vm-instance/templates/secret.yaml
[error] 26-26: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (7)
packages/system/dashboard/images/dashboard/Dockerfile (1)
4-4: Commit hash bumped – please confirm the source still buildsChanging
COMMIT_REFdirectly pins the build to a new commit. Before merging, double-check that
- the tarball for
e1382f51c6db1bca0a8ecd454407c8e282fe0243is still available/public, and- no files referenced later in the Dockerfile (dashboard path, package.json, etc.) were moved or removed in that commit.
A quick
docker buildlocally or in CI will catch surprises early.packages/system/dashboard/images/kubeapps-apis/Dockerfile (1)
7-7: Sync’d hash is good, but validate build + generated binariesThe
COMMIT_REFnow matches the dashboard image, keeping both stages consistent—nice.
Please run or ensure CI runs:docker build -f packages/system/dashboard/images/kubeapps-apis/Dockerfile .to verify proto linting and Go compilation still succeed with this commit.
packages/apps/virtual-machine/Chart.yaml (1)
20-26: Version bump looks finePatch-level increment is appropriate for these template-only changes.
No further action required.packages/apps/virtual-machine/templates/vm.yaml (2)
95-100: Condition refactor LGTMUnified
or .Values.cloudInit .Values.sshKeyskeeps disk & volume definitions in sync with the new secret logic.
125-130: Volume name matches disk, secret reference correctThe volume correctly points to
<fullname>-cloud-init; aligns with the updated secret template.packages/apps/vm-instance/templates/vm.yaml (2)
71-75: Disk condition update consistent with secret changesNo issues spotted; mirrors virtual-machine chart behaviour.
98-103: Volume reference looks goodVolume → secret linkage matches generated secret name; everything lines up.
ed37fc7 to
d28dee3
Compare
d28dee3 to
43da812
Compare
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
43da812 to
c831f53
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/apps/vm-instance/templates/secret.yaml (1)
6-6: Chart name mismatch – will render to the wrong Secret name
include "virtual-machine.fullname"is invoked inside the vm-instance chart.
Unless your helpers definevirtual-machine.fullnameglobally, Helm will error out (template not found).
Even if it exists, you’ll end up with two different charts emitting identically named Secrets, causing collisions in the same namespace.- name: {{ include "virtual-machine.fullname" $ }}-ssh-keys + name: {{ include "vm-instance.fullname" $ }}-ssh-keysReplicate the same fix for the cloud-init Secret further below.
🧹 Nitpick comments (3)
packages/apps/virtual-machine/templates/secret.yaml (2)
24-27: Remove trailing whitespace – YAML lint is currently failingThe bullet lines in the SSH-key fallback block keep a single trailing space that trips YAML lint (
[error] 26-26: trailing spaces).
This is invisible in Git but breaks CI in lint–strict pipelines.- - {{ quote . }} + - {{ quote . }}
20-28: Indentation of fallback cloud-config is brittleWhen
.Values.cloudInitis absent, the inline#cloud-configsection relies on hard-coded spaces in the template file.
Helm users frequently indent templates differently; if anyone re-formats this file, the four leading spaces can shift and the rendered YAML will break.A safer pattern is to render the whole block via
nindent 4, just like the customcloudInitpath:- #cloud-config - ssh_authorized_keys: - {{- range .Values.sshKeys }} - - {{ quote . }} - {{- end }} +{{- printf "#cloud-config\nssh_authorized_keys:\n%s" (join .Values.sshKeys "\n") | nindent 4 }}Any equivalent approach that keeps the four-space indent programmatically will do.
packages/apps/vm-instance/templates/secret.yaml (1)
24-27: Trailing whitespace – identical YAML lint failure hereSame as in the virtual-machine template: strip the space after
quote .to silence YAML lint.- - {{ quote . }} + - {{ quote . }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/apps/virtual-machine/templates/secret.yaml(1 hunks)packages/apps/virtual-machine/templates/vm.yaml(2 hunks)packages/apps/vm-instance/templates/secret.yaml(1 hunks)packages/apps/vm-instance/templates/vm.yaml(2 hunks)packages/system/dashboard/images/dashboard/Dockerfile(1 hunks)packages/system/dashboard/images/kubeapps-apis/Dockerfile(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/system/dashboard/images/kubeapps-apis/Dockerfile
- packages/apps/vm-instance/templates/vm.yaml
- packages/system/dashboard/images/dashboard/Dockerfile
- packages/apps/virtual-machine/templates/vm.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/apps/virtual-machine/templates/secret.yaml
[error] 26-26: trailing spaces
(trailing-spaces)
packages/apps/vm-instance/templates/secret.yaml
[error] 26-26: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
What this PR does
fixes #1148
This PR does two things:
Dashboard comments were removed unintentionally, which also stripped out the cloud-init shebang. This fix puts it back.
The update refines how various cloudInit options are processed, whether or not sshKeys are provided.
Release note
Summary by CodeRabbit
New Features
Refactor
Chores