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

terraform: add var.extra_build_env_vars, closes #413 #414

Closed
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions terraform/all-in-one.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ No resources.
| <a name="input_debug_logging"></a> [debug\_logging](#input_debug_logging) | Enable debug logging | `bool` | `false` | no |
| <a name="input_deployment_ssh_key"></a> [deployment\_ssh\_key](#input_deployment_ssh_key) | Content of private key used to deploy to the target\_host after initial installation. To ensure maximum security, it is advisable to connect to your host using ssh-agent instead of relying on this variable | `string` | `null` | no |
| <a name="input_disk_encryption_key_scripts"></a> [disk\_encryption\_key\_scripts](#input_disk_encryption_key_scripts) | Each script will be executed locally. Output of each will be created at the given path to disko during installation. The keys will be not copied to the final system | <pre>list(object({<br> path = string<br> script = string<br> }))</pre> | `[]` | no |
| <a name="input_extra_build_env_vars"></a> [extra\_build\_env\_vars](#input_extra_build_env_vars) | Extra environment variables to be passed to the build. | `map(string)` | `{}` | no |
| <a name="input_extra_environment"></a> [extra\_environment](#input_extra_environment) | Extra environment variables to be set during installation. This can be useful to set extra variables for the extra\_files\_script or disk\_encryption\_key\_scripts | `map(string)` | `{}` | no |
| <a name="input_extra_files_script"></a> [extra\_files\_script](#input_extra_files_script) | A script that should place files in the current directory that will be copied to the targets / directory | `string` | `null` | no |
| <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 |
Expand Down
2 changes: 2 additions & 0 deletions terraform/all-in-one/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ module "system-build" {
attribute = var.nixos_system_attr
file = var.file
nix_options = var.nix_options
extra_build_env_vars = var.extra_build_env_vars
}

module "partitioner-build" {
source = "../nix-build"
attribute = var.nixos_partitioner_attr
file = var.file
nix_options = var.nix_options
extra_build_env_vars = var.extra_build_env_vars
}

locals {
Expand Down
6 changes: 6 additions & 0 deletions terraform/all-in-one/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,9 @@ variable "nixos_facter_path" {
description = "Path to which to write a `facter.json` generated by `nixos-facter`."
default = ""
}

variable "extra_build_env_vars" {
type = map(string)
description = "Extra environment variables to be passed to the build."
default = {}
}
11 changes: 6 additions & 5 deletions terraform/nix-build.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ No modules.

## Inputs

| Name | Description | Type | Default | Required |
| ------------------------------------------------------------------- | -------------------------------------------------- | ------------- | ------- | :------: |
| <a name="input_attribute"></a> [attribute](#input_attribute) | the attribute to build, can also be a flake | `string` | n/a | yes |
| <a name="input_file"></a> [file](#input_file) | the nix file to evaluate, if not run in flake mode | `string` | `null` | no |
| <a name="input_nix_options"></a> [nix\_options](#input_nix_options) | the options of nix | `map(string)` | `{}` | no |
| Name | Description | Type | Default | Required |
| ------------------------------------------------------------------------------------------------ | ------------------------------------------------------ | ------------- | ------- | :------: |
| <a name="input_attribute"></a> [attribute](#input_attribute) | the attribute to build, can also be a flake | `string` | n/a | yes |
| <a name="input_extra_build_env_vars"></a> [extra\_build\_env\_vars](#input_extra_build_env_vars) | Extra environment variables to be passed to the build. | `map(string)` | `{}` | no |
| <a name="input_file"></a> [file](#input_file) | the nix file to evaluate, if not run in flake mode | `string` | `null` | no |
| <a name="input_nix_options"></a> [nix\_options](#input_nix_options) | the options of nix | `map(string)` | `{}` | no |

## Outputs

Expand Down
1 change: 1 addition & 0 deletions terraform/nix-build/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ data "external" "nix-build" {
attribute = var.attribute
file = var.file
nix_options = local.nix_options
environment = jsonencode(var.extra_build_env_vars)
}
}
output "result" {
Expand Down
9 changes: 5 additions & 4 deletions terraform/nix-build/nix-build.sh
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
#!/usr/bin/env bash
set -efu

declare file attribute nix_options
eval "$(jq -r '@sh "attribute=\(.attribute) file=\(.file) nix_options=\(.nix_options)"')"
declare file attribute nix_options environment
eval "$(jq -r '@sh "attribute=\(.attribute) file=\(.file) nix_options=\(.nix_options) environment=\(.environment)"')"
options=$(echo "${nix_options}" | jq -r '.options | to_entries | map("--option \(.key) \(.value)") | join(" ")')
vars=$(echo "${environment}" | jq -r "to_entries | map(\"\(.key)='\(.value)'\") | join(\" \")")
if [[ -n ${file-} ]] && [[ -e ${file-} ]]; then
# shellcheck disable=SC2086
out=$(nix build --no-link --json $options -f "$file" "$attribute")
out=$(eval "env ${vars} nix build --no-link --json --impure $options -f '$file' '$attribute'")
Copy link
Member

@Mic92 Mic92 Oct 24, 2024

Choose a reason for hiding this comment

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

Mhm --impure by default is not so nice.

Have you considered the following approach?

https://github.com/NixOS/nixos-wiki-infra/blob/main/terraform/nixos-wiki/nixos_vars.tf

Basically one can use terraform to generate a json file and using git add to make sure it's recognized by the flake. Maybe we can encode this pattern into the terraform module instead of using builtins.getEnv?
The big upside on this is that, you are no longer forced to always have to use terraform to build or deploy your nixos configuration.

Copy link
Member

Choose a reason for hiding this comment

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

Just imagine you want your machines to be build by CI. Before you could just do nixos-rebuild build --flake .mymachine, now you have to do some sort of terraform plan.

Copy link
Member

Choose a reason for hiding this comment

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

Interface wise I would propose a nix-file module that would use git add --intent-to-add --force -- ./file >/dev/null 2>&1 || true to make sure that it is added to your git repository (if one exists). It would take content as a parameter:

module "nixos_vars" { 
   source  = "../nix-file"
   content = jsonencode({})
   filename = "${path.module}/nixos-vars.json"
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm. if one could make a file without terraform, why couldn't one similarly make env vars, if the nixos configuration is indeed made to require them?

locally such a nix-file module might give some noise in version control, tho one could opt to just never use git add . anymore. whereas deploying from CI might circumvent that, it would seem a bit annoying in a development setting. going by file may have use-cases as well, tho it'd be nice to see a route more amenable to the common use-case of git + flakes + development supported as well.

on --impure, maybe a conditional could be added.

Copy link
Member

Choose a reason for hiding this comment

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

As we discussed it's possible to achieve your goal without impure evaluation. Check how to import json into a nixos configuration: https://github.com/nix-community/disko/blob/09a776702b004fdf9c41a024e1299d575ee18a7d/install-cli.nix#L56

disko-install still adds --impure. However it's possible to use getFlake also in a pure evaluation: Check out this code in clan: https://git.clan.lol/clan/clan-core/src/commit/bcf2cd1814bbef57c74b0b4eec8f6a7cc0479475/pkgs/clan-cli/clan_cli/machines/machines.py#L229

Copy link
Member

Choose a reason for hiding this comment

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

KiaraGrouwstra@75e09d1 also looks good.

Copy link
Contributor Author

@KiaraGrouwstra KiaraGrouwstra Oct 26, 2024

Choose a reason for hiding this comment

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

@Mic92 thanks for elaborating - i originally hadn't quite figured out the tempfile thing. i'll try and see if I can use your mentioned approach to resolve the flake+git friction.

Copy link
Contributor Author

@KiaraGrouwstra KiaraGrouwstra Oct 26, 2024

Choose a reason for hiding this comment

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

notes to self to reconcile the PRs, before i forget: info may be passed purely without staging to git thru nix build --expr, reading a file by builtins.fetchTree to pass the content as file to avoid stack overflows based on content size. the original lib.nixosSystem can then be wrapped so as to e.g. write the extra info to some file.

edit: potential alternative: nix build path:.

printf '%s' "$out" | jq -c '.[].outputs'
else
# shellcheck disable=SC2086
out=$(nix build --no-link --json $options "$attribute")
out=$(eval "env ${vars} nix build --no-link --json --impure $options '$attribute'")
printf '%s' "$out" | jq -c '.[].outputs'
fi
6 changes: 6 additions & 0 deletions terraform/nix-build/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,9 @@ variable "nix_options" {
description = "the options of nix"
default = {}
}

variable "extra_build_env_vars" {
Copy link
Member

Choose a reason for hiding this comment

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

I think in general it would be fine to have environment variables or flags passed to nixos-rebuild. This could be used for injecting binary cache configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, i'll check it out. i still got a few question marks around say how that would handle conditionals based on environment variables, but i guess i can just try that.

type = map(string)
description = "Extra environment variables to be passed to the build."
default = {}
}
2 changes: 1 addition & 1 deletion terraform/update-docs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ set -euo pipefail
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
cd "$SCRIPT_DIR"
files=()
find "${SCRIPT_DIR}"/* -type d | while read -r i; do
find "${SCRIPT_DIR}"/* -maxdepth 1 -type d | while read -r i; do
module_name=$(basename "$i")
markdown_file="${SCRIPT_DIR}/${module_name}.md"
terraform-docs --config "${SCRIPT_DIR}/.terraform-docs.yml" markdown table --output-file "${markdown_file}" --output-mode inject "${module_name}"
Expand Down
Loading