Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add nix option in terraform and fix run-nixos-anywhere.sh #310

Merged
merged 10 commits into from
Jul 1, 2024

Conversation

misumisumi
Copy link
Contributor

@misumisumi misumisumi commented May 7, 2024

The current terraform module does not accept nix options.
Therefore, it may not work properly when needing --options pure-eval false and so on.
I add nix_options to the terraform module to address this issue.

Also, I fixed an issue where $tmpdir/keys could not be created again when multiple disk_encryption_key_scripts were specified in run-nixos-anywhere.sh, resulting in an error.

@misumisumi
Copy link
Contributor Author

Also, I fixed an issue where $tmpdir/keys could not be created again when multiple disk_encryption_key_scripts were specified in run-nixos-anywhere.sh, resulting in an error.

The example shown in All-In-One seems wrong.
decrypt-zfs-key.sh is shown to redirect, but even within the script I am trying to create a file with the redirect.
As shown in ./decrypt.sh > $tmpdir/key/0 (./decrypt.sh like echo "test" > key.txt), there is a double redirect, but is it as intended?

if [[ -n ${file-} ]] && [[ -e ${file-} ]]; then
out=$(nix build --no-link --json -f "$file" "$attribute")
out=$(nix build --no-link --json $(echo "$nix_options") -f "$file" "$attribute")
Copy link
Member

@Mic92 Mic92 May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shellcheck doesn't like this line.
Does jq handles the quoting here correctly?
If so, we can make shellcheck ignore this.

@Mic92
Copy link
Member

Mic92 commented May 31, 2024

Also, I fixed an issue where $tmpdir/keys could not be created again when multiple disk_encryption_key_scripts were specified in run-nixos-anywhere.sh, resulting in an error.

The example shown in All-In-One seems wrong. decrypt-zfs-key.sh is shown to redirect, but even within the script I am trying to create a file with the redirect. As shown in ./decrypt.sh > $tmpdir/key/0 (./decrypt.sh like echo "test" > key.txt), there is a double redirect, but is it as intended?

Fixed. I checked again with the orignal machine where I took this from.

@@ -1,8 +1,12 @@
locals {
nix_options = var.nix_options == null ? "" : join(" ", [for k, v in var.nix_options : "--option ${k} ${v}"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we already give up on quoting here already.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we not dump this as a json object here?
And than on the shell script side, we use jq to get the data back?
Than we can build up some array in bash instead with all quoting handled correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to json format and handled it on the shell side.

@misumisumi
Copy link
Contributor Author

@Mic92
Sorry for the late reply and work.
I have corrected the points you pointed out, so please check.,

terraform/nix-build/main.tf Outdated Show resolved Hide resolved
@misumisumi
Copy link
Contributor Author

misumisumi commented Jul 1, 2024

Thank you for the correction.
It worked fine in my environment.

@misumisumi
Copy link
Contributor Author

On a separate matter, there is no ignore_systemd_errors option in all-in-one, is this intentional?
If not, is it okay to add it? or should I separate PR?

@Mic92
Copy link
Member

Mic92 commented Jul 1, 2024

@mergify queue

Copy link
Contributor

mergify bot commented Jul 1, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at aebe3d1

@Mic92
Copy link
Member

Mic92 commented Jul 1, 2024

On a separate matter, there is no ignore_systemd_errors option in all-in-one, is this intentional? If not, is it okay to add it? or should I separate PR?

Probably just an oversight, we can add it there as well.

@mergify mergify bot merged commit aebe3d1 into nix-community:main Jul 1, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants