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

CLI arguments to pass data to and retrieve data from Nickel configurations #1408

Closed
yannham opened this issue Jun 27, 2023 · 9 comments · Fixed by #1475 or #1712
Closed

CLI arguments to pass data to and retrieve data from Nickel configurations #1408

yannham opened this issue Jun 27, 2023 · 9 comments · Fixed by #1475 or #1712

Comments

@yannham
Copy link
Member

yannham commented Jun 27, 2023

Is your feature request related to a problem? Please describe.
It often happens that some configuration values are not known when writing Nickel code but only when we finally run the nickel CLI invocation. Or that those values vary. See for example #1202 and #413. Another related example is secrets (an SSH key or other credentials) that you don't want to hardcode into the configuration but to pass either via the CLI or environment variables.

We might also want to override quickly the field of an existing configuration on-the-fly, without having to write a separate file which just imports the old one and do the override.

Similarly, it may happen that the configuration we want to extract is just part of a larger one: we need <config>.ubuntu.system exported to a YAML file.

Currently, the only existing solution to do those kind of things is to inject Nickel code in the nickel invocation: for example:

$ nickel export <<< 'let as_function = import "base.ncl" in as_function "somearg" "coming from CLI"'
...
$ nickel export <<< 'let as_record = import "record-base.ncl" in as_record & {foo.bar.baz = "set/overridden"}'
...
$ nickel export <<< '(import "big-config.ncl").ubuntu.system'

Describe the solution you'd like
While possible today, those operations aren't very elegant. They require to generate Nickel code with boilerplate (importing the original file, and applying whatever operations needed, such as merging), and can also have security implications: if a field is set to a value coming from an external source, the Nickel part is subject to Nickel injections (see #1202).

The proposal to add new CLI arguments to ease such operations: the overall goal is to make Nickel configuration composable with respect to shell operations, not only between Nickel configurations themselves.

Typically, one would have at least one argument to set a value of a configuration and one to select one output of the configuration:

$ nickel export -f "base.ncl" --bikeshed-arg-for-function '"somearg"' --bikeshed-arg-for-function '"coming from CLI"'
...
$ nickel export -f "record-base.ncl" --bikeshed-arg-for-record foo.bar.baz '"set/overridden"'
...
$ nickel export -f "big-config.ncl" --bikeshed-output ubuntu.system
...

Bikeshedding

There are several things to consider:

  • should we pass the arguments as arguments to functions, as setting record values, or both (with different CLI arguments)?
  • should we provide a different CLI argument to set a field, and to override a field (distinguishing the "normal" usage of filling a missing value versus intentionally overriding existing behavior)
  • what should be the name of the new CLI argument? Proposals
    • --arg, --input, --param, --set, --set-field for arguments
    • --access, --select, --field, --path, --field-path for output

My very personal view is that we should encourage the recursive records (aka partial configurations) approach, and thus support in priority passing data to records rather than to functions (also, this has the advantage of forcing users to name such inputs, while functions can be purely positional). And that we should differentiate between "passing an argument from the CLI" and "overriding an existing value", so we should have an additional --override CLI argument (if needed).

@aspiwack
Copy link
Member

If you want to encourage the partial record style, make the record generate new argument flags

$ nickel export -f base.ncl --my-record-field=foo

Whereas function arguments and overrides can be gated behind a special (possibly slightly verbose) flag

$ nickel export -f base.ncl --argument "my-function-arg=foo"

Something of the sort.

@yannham
Copy link
Member Author

yannham commented Jun 28, 2023

If you want to encourage the partial record style, make the record generate new argument flags

Shouldn't they be guarded by a prefix, at least? Otherwise they could clash with existing CLI argument (for example, currently you can precise --format)

@aspiwack
Copy link
Member

Maybe, but it's not obvious how to make such a prefix “feel native”.

Think about it: how reluctant are you to use --arg in Nix? Probably somewhere between “rather” and “very”. It's just not inviting (despite the fact that it's really hard to articulate why). So if we think that they are a good feature, we should probably make these new arguments as lightweight to use as possible.

Another requirement is that completion works on these arguments (at least with Bash). Also that they can be typed unescaped on major shells (Bash, Zsh, Fish, at least)

Several options that I can think of (there may be more):

  • Having a prefix to prevent clashes
  • Having a prefix but we can omit the prefix when it doesn't clash with an existing command-line argument
  • Passing the extra flags after --: nickel export -f base.ncl -- --my-record-field foo --my-other-field bar

@MMesch
Copy link
Contributor

MMesch commented Jun 28, 2023

An idea that I haven't fully thought through but that seems interesting:

It would be cool if everything after a certain flag (--cli or just --) would make nickel fully invisible to the user. E.g.

nickel export myconfig.ncl --cli

would be a fully independent cli that shows help (on empty or with --help) with all available partial fields, their type, description etc, and that generates the config as specified by nickels export command and other flags before. Maybe this could be included in a shebang on top of the nickel file so that the user can just do ./myconfig and find their way without knowing anything about nickel (not sure how to deal with the .ncl extension then).

@yannham
Copy link
Member Author

yannham commented Jun 29, 2023

Passing the extra flags after --: nickel export -f base.ncl -- --my-record-field foo --my-other-field bar

I think I really like the idea, in conjunction with @MMesch 's proposal. You would type nickel export myconfig.ncl -- help and that would print a help message synthesized from the metadata, with available fields etc. Interestingly this might in fact make Nickel attractive as a simple shell scripting language for processing small data (I'm not saying it's a good idea, I'm simply thinking out loud). You can do basic data manipulation as in any functional language, but contracts would give you a simple way to specify a typed interface and generate the validation code and the CLI boilerplate automatically (well, it wouldn't be generated, but taken care of by the nickel binary)

@bew
Copy link

bew commented Sep 1, 2023

Hello,
I like where this is going 👍

I'm sorry if this was mentioned before, I figure it'll simpler to ask directly than to read all issue/PR comments:
Have you considered using plain key=value for the fields?

Where it would look like:

$ nickel export -f config-gcc.ncl -- --help
...

$ nickel export -f config-gcc.ncl -- path_libc=/new/path --override optimization_level=1

Apart from the fact that mixing - & _ for --path_libc looks weird, using k=v means cli-given fields are always in the same format, not sometimes using flags like --path_libc or positional args for overrides. And there is a clear distinction between fields and overrides (since only --override is a flag)

WDYT?


We could go a little further, as now kv are not flags we can remove the -- (not ambiguous anymore) and have:

$ nickel export -f config-gcc.ncl    path_libc='"/new/path"' --override optimization_level=1
And a little further again

By separating all overrides after --overrides:

$ nickel export -f config-gcc.ncl    path_libc='"/new/path"' other_config='"foo"'  --overrides override1=1  override2=2

For passing values, there is also the terraform way with their TF_VAR_your_toplevel_variable=here, were nickel could read variables from the environment, prefixed with NICKEL_EXPORT_path_lib=/new/path and NICKEL_EXPORT_OVERRIDE_optimization_level=1

Another thing nickel or the language could provide is 'parsers' from CLI, where giving foo=bar would parse bar as a string (if foo is typed as a string) and transform it to "bar", so we don't need to do potentially complex escaping to pass strings through CLI.

WDYT?

@bew
Copy link

bew commented Sep 1, 2023

Also, as mentioned in #1475 Partially fixes #1408 (it doesn't extract specific fields).

This issue should remain open until there is a way to extract specific fields?

@yannham
Copy link
Member Author

yannham commented Sep 15, 2023

Hi @bew , sorry, I missed this notification (I wonder if it actually notifies when commenting on a closed issue?).

This issue should remain open until there is a way to extract specific fields?

Absolutely, this was just GitHub being zealous.

I'm sorry if this was mentioned before, I figure it'll simpler to ask directly than to read all issue/PR comments:
Have you considered using play key=value for the fields?

We were probably biased toward a classical command line format because clap would provide us with several things for free (help message, error message when the parsing is wrong, error message when a required field isn't there, etc.), making the initial implementation easier. That being said, this isn't set in stone - the customize mode is explicitly unstable - and your proposal does solve the name conflict issue, as well as being somehow consistent with Nickel syntax. Those are great suggestions, thanks for that! We'll discuss it in the next weekly meeting. As well as your proposed extensions. We'll just have to make sure it's compatible with the potential evolution of the CLI (for example, we would like to move from the default behavior being to do evaluation to having a dedicated eval subcommand, and maybe move the input file from an argument to a positional argument, etc.).

About environment variables, that might be useful in some settings - I wouldn't replace the current customize mode with it, but it's possible to have in addition to the argument way.

Another thing nickel or the language could provide is 'parsers' from CLI, where giving foo=bar would parse bar as a string (if foo is typed as a string) and transform it to "bar", so we don't need to do potentially complex escaping to pass strings through CLI.

I'm less of a fan of this proposal. I do agree that escaping strings on the CLI is annoying, but we have to consider consistency and principle of least surprise. Now, if you write nickel export -f file.ncl foo=1, it might mean 1 : Number or "1" : String depending on the content of file.ncl. I'm not saying this is a no-go, but it's non trivial magic.

@yannham
Copy link
Member Author

yannham commented Oct 20, 2023

As an update, we agreed that (at the least the first part of) @bew's proposal is a better way forward. We just need to find the bandwith to implement it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants