terraform: expose ssh options for installation and deployment #564
Conversation
Mic92
force-pushed
the
tf-expose-ssh-options
branch
from
a24b6fe to
e8a0ceb
Compare
WalkthroughIntroduces SSH options as configurable inputs across install and deploy paths. Terraform variables and module interfaces are extended to accept lists of SSH options, which are passed via environment variables to shell scripts. The scripts parse JSON-encoded options and append them to underlying ssh/nix commands. Documentation reflects the new inputs and minor wording updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant TF as Terraform (install module)
participant LE as local-exec
participant SH as run-nixos-anywhere.sh
participant NX as nix run (nixos-anywhere)
U->>TF: Apply with var.ssh_options = [..]
TF->>LE: Execute with env SSH_OPTIONS=json(list)
LE->>SH: Invoke script
SH->>SH: Parse SSH_OPTIONS JSON
SH->>NX: nix run ... --ssh-option <each option>
NX-->>U: Installation proceeds
sequenceDiagram
autonumber
actor U as User
participant TF as Terraform (nixos-rebuild module)
participant LE as local-exec
participant SH as deploy.sh
participant SSH as ssh/nix copy
U->>TF: Apply with var.ssh_options = [..]
TF->>LE: Execute with env SSH_OPTIONS=json(list)
LE->>SH: Invoke script
SH->>SH: Parse SSH_OPTIONS JSON
SH->>SSH: Use -o <each option> in SSH operations
SSH-->>U: Deploy/copy proceeds
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
terraform/install/run-nixos-anywhere.sh (1)
92-94: Simplify jq expression for array handling.The current code uses
to_entries[] | (.value, "\u0000")which converts the array to key-value pairs with numeric indices before extracting values. Sincevar.ssh_optionsis alist(string), this adds unnecessary complexity. Both approaches produce identical output, but the simplified version is more direct and readable.Consider simplifying:
-while IFS= read -r -d '' value; do - args+=("--ssh-option" "$value") -done < <(jq -j 'to_entries[] | (.value, "\u0000")' <<<"${SSH_OPTIONS}") +while IFS= read -r -d '' value; do + args+=("--ssh-option" "$value") +done < <(jq -j '.[] | (., "\u0000")' <<<"${SSH_OPTIONS}")Verified that both expressions handle empty arrays, single options, and multiple options identically.
terraform/all-in-one.md (1)
207-207: Minor wording inconsistency in description.The description says "during deploy" while similar parameters use full nouns (e.g., line 206: "after initial installation"). Consider using "during deployment" for consistency.
Apply this diff for consistency:
-| <a name="input_deployment_ssh_options"></a> [deployment\_ssh\_options](#input_deployment_ssh_options) | Additional SSH options to use during deploy | `list(string)` | `[]` | no | +| <a name="input_deployment_ssh_options"></a> [deployment\_ssh\_options](#input_deployment_ssh_options) | Additional SSH options to use during deployment | `list(string)` | `[]` | no |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
terraform/all-in-one.md(2 hunks)terraform/all-in-one/main.tf(2 hunks)terraform/all-in-one/variables.tf(1 hunks)terraform/install.md(1 hunks)terraform/install/main.tf(1 hunks)terraform/install/run-nixos-anywhere.sh(1 hunks)terraform/install/variables.tf(1 hunks)terraform/nixos-rebuild.md(1 hunks)terraform/nixos-rebuild/deploy.sh(1 hunks)terraform/nixos-rebuild/main.tf(1 hunks)terraform/nixos-rebuild/variables.tf(1 hunks)
🔇 Additional comments (11)
terraform/install.md (1)
80-80: LGTM! Documentation clearly describes the new SSH options input.The new
ssh_optionsinput is properly documented with correct type, default value, and clear description.terraform/all-in-one/main.tf (2)
42-42: LGTM! SSH options correctly passed to install module.The
install_ssh_optionsvariable is properly forwarded to the install module.
63-63: LGTM! SSH options correctly passed to nixos-rebuild module.The
deployment_ssh_optionsvariable is properly forwarded to the nixos-rebuild module, allowing different SSH options for deployment vs installation.terraform/install/main.tf (1)
30-31: LGTM! SSH options correctly encoded and passed to provisioner.The
SSH_OPTIONSenvironment variable is properly JSON-encoded fromvar.ssh_optionsand merged into the provisioner environment.terraform/install/variables.tf (1)
125-129: LGTM! Variable declaration is correct and properly integrated.The
ssh_optionsvariable is properly declared with appropriate type, description, and default value. Verification confirms thatterraform/install/run-nixos-anywhere.shcorrectly handles theSSH_OPTIONSenvironment variable by parsing it as JSON and passing each value to nixos-anywhere with the--ssh-optionflag.terraform/nixos-rebuild/variables.tf (1)
41-45: LGTM!The variable definition is consistent with the pattern used in other modules and follows Terraform best practices.
terraform/nixos-rebuild.md (1)
53-62: LGTM!The documentation accurately reflects the new
ssh_optionsinput with correct type, default value, and description.terraform/all-in-one/variables.tf (2)
153-157: LGTM!The
install_ssh_optionsvariable is well-defined and clearly scoped to the installation phase.
159-163: LGTM!The
deployment_ssh_optionsvariable properly separates deployment-time SSH options from installation options, providing clear control over SSH behavior in each phase.terraform/nixos-rebuild/main.tf (1)
7-8: deploy.sh correctly handles SSH_OPTIONS as JSON array.Verification confirms that
deploy.shproperly parses theSSH_OPTIONSenvironment variable. The script usesjq 'to_entries[] | .value'at line 43, which works correctly with the JSON-encoded list from Terraform. When applied to an array,to_entriesconverts each element to{key: index, value: element}, and.valueextracts the SSH options, appending each as-oarguments to thesshOptsarray. These options are then applied to bothnix copy(line 46) andsshcommands (line 68).terraform/all-in-one.md (1)
215-215: LGTM!The documentation for
install_ssh_optionsis clear, accurate, and consistent with the module's purpose.
| | <a name="input_file"></a> [file](#input_file) | Nix file containing the nixos\_system\_attr and nixos\_partitioner\_attr. Use this if you are not using flake | `string` | `null` | no | | ||
| | <a name="input_install_bootloader"></a> [install\_bootloader](#input_install_bootloader) | Install/re-install the bootloader | `bool` | `false` | no | | ||
| | <a name="input_install_port"></a> [install\_port](#input_install_port) | SSH port used to connect to the target\_host, before installing NixOS. If null then the value of `target_port` is used | `string` | `null` | no | | ||
| | <a name="input_install_port"></a> [install\_port](#input_install_port) | SSH port used to connect to the target\_host, before installing NixOS. If null than the value of `target_port` is used | `string` | `null` | no | |
There was a problem hiding this comment.
Fix grammatical error in description.
The description contains "than" which should be "then" in this context ("If null then the value...").
Apply this diff:
-| <a name="input_install_port"></a> [install\_port](#input_install_port) | SSH port used to connect to the target\_host, before installing NixOS. If null than the value of `target_port` is used | `string` | `null` | no |
+| <a name="input_install_port"></a> [install\_port](#input_install_port) | SSH port used to connect to the target\_host, before installing NixOS. If null then the value of `target_port` is used | `string` | `null` | no |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | <a name="input_install_port"></a> [install\_port](#input_install_port) | SSH port used to connect to the target\_host, before installing NixOS. If null than the value of `target_port` is used | `string` | `null` | no | | |
| | <a name="input_install_port"></a> [install\_port](#input_install_port) | SSH port used to connect to the target\_host, before installing NixOS. If null then the value of `target_port` is used | `string` | `null` | no | |
🤖 Prompt for AI Agents
In terraform/all-in-one.md around line 213, the input description uses "than"
incorrectly; change "If null than the value of `target_port` is used" to "If
null then the value of `target_port` is used" to correct the grammar, updating
the markdown table cell accordingly.
| | <a name="input_install_ssh_key"></a> [install\_ssh\_key](#input_install_ssh_key) | Content of private key used to connect to the target\_host during initial installation | `string` | `null` | no | | ||
| | <a name="input_install_user"></a> [install\_user](#input_install_user) | SSH user used to connect to the target\_host, before installing NixOS. If null then the value of `target_host` is used | `string` | `null` | no | | ||
| | <a name="input_install_ssh_options"></a> [install\_ssh\_options](#input_install_ssh_options) | Additional SSH options to use during installation | `list(string)` | `[]` | no | | ||
| | <a name="input_install_user"></a> [install\_user](#input_install_user) | SSH user used to connect to the target\_host, before installing NixOS. If null than the value of `target_host` is used | `string` | `null` | no | |
There was a problem hiding this comment.
Fix grammatical and logical errors in description.
The description has two issues:
- "than" should be "then"
- "the value of
target_host" should be "the value oftarget_user" (the description is about SSH user, not host)
Apply this diff:
-| <a name="input_install_user"></a> [install\_user](#input_install_user) | SSH user used to connect to the target\_host, before installing NixOS. If null than the value of `target_host` is used | `string` | `null` | no |
+| <a name="input_install_user"></a> [install\_user](#input_install_user) | SSH user used to connect to the target\_host, before installing NixOS. If null then the value of `target_user` is used | `string` | `null` | no |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | <a name="input_install_user"></a> [install\_user](#input_install_user) | SSH user used to connect to the target\_host, before installing NixOS. If null than the value of `target_host` is used | `string` | `null` | no | | |
| | <a name="input_install_user"></a> [install\_user](#input_install_user) | SSH user used to connect to the target\_host, before installing NixOS. If null then the value of `target_user` is used | `string` | `null` | no | |
🤖 Prompt for AI Agents
In terraform/all-in-one.md around line 216, the input description for
install_user has grammatical and logical errors: change "than" to "then" and
replace "the value of `target_host`" with "the value of `target_user`" so the
sentence correctly reads that if null then the SSH user falls back to
target_user (and ensure spacing/punctuation remains consistent with the table
format).
| | <a name="input_target_host"></a> [target\_host](#input_target_host) | DNS host to deploy to | `string` | n/a | yes | | ||
| | <a name="input_target_port"></a> [target\_port](#input_target_port) | SSH port used to connect to the target\_host after installing NixOS. If install\_port is not set then this port is also used before installing. | `number` | `22` | no | | ||
| | <a name="input_target_user"></a> [target\_user](#input_target_user) | SSH user used to connect to the target\_host after installing NixOS. If install\_user is not set then this user is also used before installing. | `string` | `"root"` | no | | ||
| | <a name="input_target_port"></a> [target\_port](#input_target_port) | SSH port used to connect to the target\_host after installing NixOS. If install\_port is not set than this port is also used before installing. | `number` | `22` | no | |
There was a problem hiding this comment.
Fix grammatical error in description.
The description contains "than" which should be "then" ("If install_port is not set then this port...").
Apply this diff:
-| <a name="input_target_port"></a> [target\_port](#input_target_port) | SSH port used to connect to the target\_host after installing NixOS. If install\_port is not set than this port is also used before installing. | `number` | `22` | no |
+| <a name="input_target_port"></a> [target\_port](#input_target_port) | SSH port used to connect to the target\_host after installing NixOS. If install\_port is not set then this port is also used before installing. | `number` | `22` | no |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | <a name="input_target_port"></a> [target\_port](#input_target_port) | SSH port used to connect to the target\_host after installing NixOS. If install\_port is not set than this port is also used before installing. | `number` | `22` | no | | |
| | <a name="input_target_port"></a> [target\_port](#input_target_port) | SSH port used to connect to the target\_host after installing NixOS. If install\_port is not set then this port is also used before installing. | `number` | `22` | no | |
🤖 Prompt for AI Agents
In terraform/all-in-one.md around line 229, the description for input
target_port uses "than" incorrectly; change "If install_port is not set than
this port is also used before installing." to use "then" ("If install_port is
not set then this port is also used before installing.") so the grammar is
correct.
| | <a name="input_target_port"></a> [target\_port](#input_target_port) | SSH port used to connect to the target\_host after installing NixOS. If install\_port is not set then this port is also used before installing. | `number` | `22` | no | | ||
| | <a name="input_target_user"></a> [target\_user](#input_target_user) | SSH user used to connect to the target\_host after installing NixOS. If install\_user is not set then this user is also used before installing. | `string` | `"root"` | no | | ||
| | <a name="input_target_port"></a> [target\_port](#input_target_port) | SSH port used to connect to the target\_host after installing NixOS. If install\_port is not set than this port is also used before installing. | `number` | `22` | no | | ||
| | <a name="input_target_user"></a> [target\_user](#input_target_user) | SSH user used to connect to the target\_host after installing NixOS. If install\_user is not set than this user is also used before installing. | `string` | `"root"` | no | |
There was a problem hiding this comment.
Fix grammatical error in description.
The description contains "than" which should be "then" ("If install_user is not set then this user...").
Apply this diff:
-| <a name="input_target_user"></a> [target\_user](#input_target_user) | SSH user used to connect to the target\_host after installing NixOS. If install\_user is not set than this user is also used before installing. | `string` | `"root"` | no |
+| <a name="input_target_user"></a> [target\_user](#input_target_user) | SSH user used to connect to the target\_host after installing NixOS. If install\_user is not set then this user is also used before installing. | `string` | `"root"` | no |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | <a name="input_target_user"></a> [target\_user](#input_target_user) | SSH user used to connect to the target\_host after installing NixOS. If install\_user is not set than this user is also used before installing. | `string` | `"root"` | no | | |
| | <a name="input_target_user"></a> [target\_user](#input_target_user) | SSH user used to connect to the target\_host after installing NixOS. If install\_user is not set then this user is also used before installing. | `string` | `"root"` | no | |
🤖 Prompt for AI Agents
In terraform/all-in-one.md around line 230, the markdown table description uses
"than" incorrectly; change "If install_user is not set than this user is also
used before installing." to "If install_user is not set then this user is also
used before installing." so the grammar is correct.
| while IFS= read -r -d '' value; do | ||
| sshOpts+=(-o "$value") | ||
| done < <(jq -j 'to_entries[] | (.value, "\u0000")' <<<"${SSH_OPTIONS}") |
There was a problem hiding this comment.
Add error handling for SSH_OPTIONS environment variable.
The script assumes SSH_OPTIONS is always set and contains valid JSON. If the variable is unset or contains invalid JSON, jq will fail and cause the script to exit (due to set -e).
Apply this diff to add safe handling:
+# Parse SSH options if provided
+if [[ -n ${SSH_OPTIONS:-} ]]; then
while IFS= read -r -d '' value; do
sshOpts+=(-o "$value")
done < <(jq -j 'to_entries[] | (.value, "\u0000")' <<<"${SSH_OPTIONS}")
+fiAlternatively, provide a default empty array:
-while IFS= read -r -d '' value; do
+while IFS= read -r -d '' value; do
sshOpts+=(-o "$value")
-done < <(jq -j 'to_entries[] | (.value, "\u0000")' <<<"${SSH_OPTIONS}")
+done < <(jq -j 'to_entries[] | (.value, "\u0000")' <<<"${SSH_OPTIONS:-[]}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| while IFS= read -r -d '' value; do | |
| sshOpts+=(-o "$value") | |
| done < <(jq -j 'to_entries[] | (.value, "\u0000")' <<<"${SSH_OPTIONS}") | |
| # Parse SSH options if provided | |
| if [[ -n ${SSH_OPTIONS:-} ]]; then | |
| while IFS= read -r -d '' value; do | |
| sshOpts+=(-o "$value") | |
| done < <(jq -j 'to_entries[] | (.value, "\u0000")' <<<"${SSH_OPTIONS}") | |
| fi |
🤖 Prompt for AI Agents
In terraform/nixos-rebuild/deploy.sh around lines 41 to 43, the loop assumes
SSH_OPTIONS is set and valid JSON so jq can fail under set -e; guard against
unset/invalid JSON by first setting a safe default (e.g. use an empty JSON array
when SSH_OPTIONS is empty or unset) and/or validate the variable before piping
to jq (run jq -e to test validity and fall back to the default on error), then
only run the read loop when you have a valid JSON array; ensure failures from jq
are handled gracefully so the script does not exit unexpectedly.
1 participant
Summary by CodeRabbit
New Features
Documentation