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.special_args, closes #413, supersedes #414. #418

Merged
merged 3 commits into from
Nov 16, 2024

Conversation

KiaraGrouwstra
Copy link
Contributor

@KiaraGrouwstra KiaraGrouwstra commented Oct 31, 2024

This PR adds a Terraform input variable named special_args.

This allows passing in a map from Terraform to expose to NixOS's specialArgs at build-time.
Example usage, in this case presuming deployment to a Hetzner Cloud server (resource.hcloud_server):

let
  servers = ...;
  variable = ...;
  data = ...;
  resource = ...;
in
{
  inherit variable data resource;
  module =
    lib.mapAttrs (server_name: _server_config: let
    in {
      # pin module version by nix flake inputs
      source =
"github.com/numtide/nixos-anywhere?ref=${inputs.nixos-anywhere.sourceInfo.rev}/terraform/all-in-one";
      ...
      special_args = {
        tf = {
          inherit server_name;
          # all variables
          # var = lib.mapAttrs (k: _: lib.tfRef "var.${k}") variable;
          # non-sensitive variables
          var = lib.mapAttrs (k: _: lib.tfRef "var.${k}")
  (lib.filterAttrs (_k: v: !(v ? sensitive && v.sensitive)) variable);
          data = lib.mapAttrs (type: instances: lib.mapAttrs (k: _:
  tfRef "data.${type}.${k}") instances) data;
          resource = lib.mapAttrs (type: instances: lib.mapAttrs (k:
  _: tfRef "resource.${type}.${k}") instances) resource;
          server = lib.tfRef "resource.hcloud_server.${server_name}";
        };
      };
    })
    servers;
}

You can then use these in your nixosConfigurations, in this example thru the tf argument.

As a note on security, information passed this way will hit /nix/store/. As such, the above usage example has defaulted to omitting TF variables marked as sensitive.

This PR incorporates ideas from:

An alternative design suggested by @Mic92 involved passing the information not directly, but rather thru a file. The idea would be that this might help reduce the risk of stack overflows, tho I have imagined (perhaps naively) that TF info has tended not to get too big, whereas I also had a bit more trouble getting that approach to work properly so far (involving both NARs that would suddenly mismatch again, while I'd also yet to test if one could put such files in .gitignore).

@KiaraGrouwstra KiaraGrouwstra changed the title terraform: add var.special_args, closes #413. terraform: add var.special_args, closes #413, supersedes #414. Oct 31, 2024
@Mic92
Copy link
Member

Mic92 commented Oct 31, 2024

Looks much better. Specialargs are also a good idea, because it will allow it uses values also for imports.

@Mic92
Copy link
Member

Mic92 commented Oct 31, 2024

Oh wait, this is computing the nar from the flake that defines the NixOS config and than passes in the special args as a command line argument again. The idea was to do the same nar calculation for the JSON file that is passed in as an input and than just use getFlake on the original configuration and pass in the JSON file path via readFile. This than avoids large string I can follow up later with more specific, currently on my phone. But it's going in the right direction.

@KiaraGrouwstra
Copy link
Contributor Author

@Mic92 yes, that sounds like this earlier iteration where i tried specialArgs thru a file:

An alternative design suggested by @Mic92 involved passing the information not directly, but rather thru a file. The idea would be that this might help reduce the risk of stack overflows, tho I have imagined (perhaps naively) that TF info has tended not to get too big, whereas I also had a bit more trouble getting that approach to work properly so far (involving both NARs that would suddenly mismatch again, while I'd also yet to test if one could put such files in .gitignore).

@KiaraGrouwstra
Copy link
Contributor Author

i did consider a hybrid, where an optional filename argument could be set such as to go thru that... but felt like the feature as-is seemed functional enough to submit already

@Mic92
Copy link
Member

Mic92 commented Nov 1, 2024

Okay. I will try out later.

@KiaraGrouwstra
Copy link
Contributor Author

KiaraGrouwstra commented Nov 3, 2024

extra notes:

  • this is only supported using flakes currently (edit: pushed a commit to error clarifying this to the user)
  • it would also be possible to split this up for toplevel vs diskoScriptNoDeps - so far i had not done that

@Mic92
Copy link
Member

Mic92 commented Nov 6, 2024

I still have it on my radar, but haven't got around to test it.

@threddast
Copy link

threddast commented Nov 7, 2024

I'm trying to test this, but I'm always getting this error

nix build --expr "builtins.getFlake ''git+file://${flake_dir}?narHash=sha256-0000000000000000000000000000000000000000000=''"     
error:
       … while calling the 'getFlake' builtin
         at «string»:1:1:
            1| builtins.getFlake ''git+file:///home/threddast/devel/cloud-infra?narHash=sha256-0000000000000000000000000000000000000000000=''
             | ^

       error: cannot call 'getFlake' on unlocked flake reference 'git+file:///home/threddast/devel/cloud-infra?narHash=sha256-0000000000000000000000000000000000000000000=', at «none»:0 (use --impure to override)

Even though I have a flake.lock committed in git.
I get the same also when trying with the flake from https://codeberg.org/kiara/e2ed-hetzner
My Nix version is 2.24.10

Moreover, I'm not sure it works if the flake.nix is in a parent directory relative to where terraform plan is run

@KiaraGrouwstra
Copy link
Contributor Author

KiaraGrouwstra commented Nov 7, 2024

@threddast thank you for your feedback.
i did run into issues like that as well using Nix, tho using Lix seemed to address this.
i guess it might be possible to bisect Nix versions on that to see if/where it gets fixed.
a search in the Nix repo shows similar symptoms had been reported previously, tho it seems the present bug (regression?) seems to pop up even with the NAR hash specified and all.
i imagine nix would fix it eventually, whereas lix seems just more stable already.

@KiaraGrouwstra
Copy link
Contributor Author

@threddast courtesy of @getchoo's advice on lix matrix, i pushed an update to calculate the NAR hash differently.
it's looking like that should address the issues experienced when running this with Nix.

Copy link

@threddast threddast left a comment

Choose a reason for hiding this comment

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

Thank you, I tested the module and it works really well now.
Just a suggestion, would it make sense to move the jsonencode inside the module, similar to the nix_options variable?

# Skip to `Pass data persistently to the NixOS` for an alternative approach
#special_args = {
# terraform = {
# ip = "192.0.2.0"
Copy link
Member

@Mic92 Mic92 Nov 15, 2024

Choose a reason for hiding this comment

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

@KiaraGrouwstra is this example correct? I derived it from your pull request message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nit: the user can pass arguments under any name (or multiple even), rather than just terraform - otherwise this sounds right!

@Mic92
Copy link
Member

Mic92 commented Nov 15, 2024

If you could also solve the merge conflict, than we could finally merge this.
Thank you!

This allows passing in a map from Terraform to expose to NixOS's `specialArgs` at build-time.
Example usage, in this case presuming deployment to a Hetzner Cloud server (`resource.hcloud_server`):

```nix
let
  servers = ...;
  variable = ...;
  data = ...;
  resource = ...;
in
{
  inherit variable data resource;
  module =
    lib.mapAttrs (server_name: _server_config: let
    in {
      # pin module version by nix flake inputs
      source =
"github.com/numtide/nixos-anywhere?ref=${inputs.nixos-anywhere.sourceInfo.rev}/terraform/all-in-one";
      ...
      special_args = {
        tf = {
          inherit server_name;
          # all variables
          # var = lib.mapAttrs (k: _: lib.tfRef "var.${k}") variable;
          # non-sensitive variables
          var = lib.mapAttrs (k: _: lib.tfRef "var.${k}")
  (lib.filterAttrs (_k: v: !(v ? sensitive && v.sensitive)) variable);
          data = lib.mapAttrs (type: instances: lib.mapAttrs (k: _:
  tfRef "data.${type}.${k}") instances) data;
          resource = lib.mapAttrs (type: instances: lib.mapAttrs (k:
  _: tfRef "resource.${type}.${k}") instances) resource;
          server = lib.tfRef "resource.hcloud_server.${server_name}";
        };
      };
    })
    servers;
}
```

You can then use these in your `nixosConfigurations`, in this example thru the `tf` argument.

As a note on security, information passed this way _will_ hit `/nix/store/`. As such, the above usage example has defaulted to omitting TF variables marked as sensitive.

This PR incorporates ideas from:

- @aanderse, who implemented a similar feature in [teraflops](https://github.com/aanderse/teraflops) that inspired this PR.
- @Mic92, who suggested (see nix-community#414) to extend the original `lib.nixosSystem` call to pass in info without `--impure` or staging to Git.
- @getchoo, who suggested getting the NAR hash by `nix flake prefetch` over `getFlake`, fixing an 'unlocked flake reference' error on (non-Lix) Nix.
- @threddast, who suggested to use TF's `any` type to automate serializing.

An [alternative design](nix-community/nixos-anywhere@main...KiaraGrouwstra:nixos-anywhere:tf-info-to-wrapper#diff-2e2429dde4812f0b50c784e8d4c8b93cc9faeb52cce4747733200f65ea5c2bbb) suggested by @Mic92 involved passing the information not directly, but rather thru a file. The idea would be that this might help reduce the risk of stack overflows, tho I have imagined (perhaps naively) that TF info has tended not to get too big, whereas I also had a bit more trouble getting that approach to work properly so far (involving both NARs that would suddenly mismatch again, while I'd also yet to test if one could put such files in `.gitignore`).
@Mic92
Copy link
Member

Mic92 commented Nov 16, 2024

@mergify queue

Copy link
Contributor

mergify bot commented Nov 16, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 80a2e7d

@mergify mergify bot merged commit 80a2e7d into nix-community:main Nov 16, 2024
5 checks passed
@KiaraGrouwstra KiaraGrouwstra deleted the tf-special-args branch November 17, 2024 08:45
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.

3 participants