nixos/utils: introduce genJqStructuredSecretsReplacementSnippet, deprecate genJqSecretsReplacementSnippet

[jian-lin]

Description of changes

genJqReplacementSnippet quotes the content of the secret file in the
output json file, which prevents structured secret, such as a list or
an object, from being used.

This patch is a breaking change for genJqSecretsReplacementSnippet'.
Since the function is somewhat internal, it should be fine.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[jian-lin]

@ofborg test gitlab sing-box lemmy engelsystem artalk discourse

filebeat does not have a NixOS test.

netbird's NixOS test does not use secrets.

[Aleksanaa]

We usually add a new function in this case and guide users to use this function in the documentation. Changing the current behavior seems less ideal.

[jian-lin]

EDIT: the old implementation can be viewed here:

• linj-fork@091a790
• linj-fork@a705ef9

---

We usually add a new function in this case and guide users to use this function in the documentation

Could you provide some precedents?

Changing the current behavior seems less ideal.

Providing two slightly different functions is also not good.

To be clear, there is no implicit behavior change. Out-of-tree usage of genJqSecretsReplacementSnippet will fail to eval without damaging running systems. With a post in https://discourse.nixos.org/t/breaking-changes-announcement-for-unstable/17574 , I consider the situation good enough.

The current somewhat mysterious error message "error: cannot coerce a function to a string" can be improved at the cost of worse interface of genJqSecretsReplacementSnippet: we decrease the number of parameters by move some parameters into the newly added attribute set like this

genJqSecretsReplacementSnippet' = attr: { set, output, quote ? false }:

Then the error message becomes more clear

error: function 'genJqSecretsReplacementSnippet'' called without required argument 'set'

       at /path/to/nixpkgs/nixos/lib/utils.nix:215:43:

          214|   # attr which identifies the secret to be changed.
          215|   genJqSecretsReplacementSnippet' = attr: { set, output, quote ? false }:
             |                                           ^
          216|     let

[Guanran928]

Could you provide some precedents?

Maybe #287364

[jian-lin]

Switched to a non-breaking approach. Please re-review.

[linyinfeng]

I think this is not an ideal solution since end users can not choose between string or JSON secrets.

A better user interface should allow users to choose whether the secret is JSON or string for every single secret. For example,

genJqSecretsReplacementSnippet {
  example = [
    {
      irrelevant = "not interesting";
    }
    {
      ignored = "ignored attr";
      relevant = {
        secretString1 = {
          _secret = "/path/to/secret";
        };
        secretString2 = {
          _secret = "/path/to/secret";
          quote = true; # or some other attribute name
        };
        secretJson = {
          _secret = "/path/to/secret";
          quote = false; # or some other attribute name
        };
      };
    }
  ];
} "/path/to/output.json"

And, another advantage of this user interface is that it keeps backward compatibility.

[Aleksanaa]

Perhaps we could use a shorter name for your new function, and keep the initially compatible behavior?

[linyinfeng]

A better user interface should allow users to choose whether the secret is JSON or string for every single secret.

A demo: master...linyinfeng:nixpkgs:json-secrets.

# demo.nix
let
  demoNixpkgs = fetchTarball "https://github.com/linyinfeng/nixpkgs/archive/refs/heads/json-secrets.zip";
  pkgs = import demoNixpkgs {};
  inherit (pkgs) lib;
  utils = import "${demoNixpkgs}/nixos/lib/utils.nix" {
    inherit lib pkgs;
    config = null;
  };
in
utils.genJqSecretsReplacementSnippet {
  example = [
    {
      irrelevant = "not interesting";
    }
    {
      ignored = "ignored attr";
      relevant = {
        secretString1 = {
          _secret = "/path/to/secret-quote-1";
        };
        secretString2 = {
          _secret = "/path/to/secret-quote-2";
          quote = true; # or some other attribute name
        };
        secretJson = {
          _secret = "/path/to/secret-json";
          quote = false; # or some other attribute name
        };
      };
    }
  ];
} "/path/to/output.json"

$ nix eval --file ./demo.nix --impure --raw
if [[ -h '/path/to/output.json' ]]; then
  rm '/path/to/output.json'
fi

inherit_errexit_enabled=0
shopt -pq inherit_errexit && inherit_errexit_enabled=1
shopt -s inherit_errexit
secret1=$(<'/path/to/secret-json')
export secret1

secret2=$(<'/path/to/secret-quote-1')
export secret2

secret3=$(<'/path/to/secret-quote-2')
export secret3

/nix/store/axpqa3f5l63mc38h11x1gc5cykw8bn5i-jq-1.7.1-bin/bin/jq >'/path/to/output.json' '."example"[1]."relevant"."secretJson" = ($ENV.secret1 | fromjson) | ."example"[1]."relevant"."secretString1" = ($ENV.secret2) | ."example"[1]."relevant"."secretString2" = ($ENV.secret3)' <<'EOF'
{"example":[{"irrelevant":"not interesting"},{"ignored":"ignored attr","relevant":{"secretJson":{"_secret":"/path/to/secret-json","quote":false},"secretString1":{"_secret":"/path/to/secret-quote-1"},"secretString2":{"_secret":"/path/to/secret-quote-2","quote":true}}}]}
EOF
(( ! $inherit_errexit_enabled )) && shopt -u inherit_errexit

[jian-lin]

I think this is not an ideal solution since end users can not choose between string or JSON secrets.

Even though I do not like the idea that users are able to choose between string and JSON secrets, especially with string as the default, I think it is the only backward compatible way.

I also realize that my current implementation is actually a breaking change due to lack of a notification mechanism to tell users genJqSecretsReplacementSnippet is replaced with genJqStructuredSecretsReplacementSnippet and they should quote their secrets.

---

@linyinfeng Would you like to create a PR?

[linyinfeng]

Even though I do not like the idea that users are able to choose between string and JSON secrets, especially with string as the default, I think it is the only backward compatible way.

I believe that string should be a reasonable default.

1. Most of the time, a secret is a string. The old genJqSecretsReplacementSnippet already meets the needs of most users.

2. Converting some secrets to JSON string literals is hard since some strong secrets can contain special characters like " or \.

3. Since typical secrets are string, using plain text files makes the secret files format-orthogonal.

@linyinfeng Would you like to create a PR?

I created #325217.

[jian-lin]

Closing in favor of #325217