From 4c436f1b09b3c02a63c11c46a6777f8188f49547 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sun, 10 Mar 2019 21:50:24 +0100 Subject: [PATCH 01/43] config-option: initial summary and motivation Write (not-so-)detailed design and drawbacks Adjust formatting Expand Additional config options Add section for configuration types Add format writers and documentation sections Adjust additional options example Add some more links and change name Move file --- rfcs/0042-config-option.md | 191 +++++++++++++++++++++++++++++++++++++ 1 file changed, 191 insertions(+) create mode 100644 rfcs/0042-config-option.md diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md new file mode 100644 index 000000000..b51e99e68 --- /dev/null +++ b/rfcs/0042-config-option.md @@ -0,0 +1,191 @@ +--- +feature: config-option +start-date: 2019-03-10 +author: Silvan Mosberger +co-authors: (find a buddy later to help our with the RFC) +related-issues: (will contain links to implementation PRs) +--- + +# Summary +[summary]: #summary + +A lot of NixOS options exist to specify single settings of configuration files. Along with such options come multiple disadvantages, such as having to synchronize with upstream changes, option bloat and more. An alternative is to provide options for passing the configuration as a string, such as the common `configFile` or `extraConfig` options, but this also comes with a set of disadvantages, including difficulty to override values and bad modularity. + +This RFC proposes aims to solve these problems by encouraging NixOS module authors to provide an option for specifying the programs configuration file as a Nix value, providing a set of utility functions for writing these options conveniently, and updating the documentation to recommend this way of doing program configuration. + +# Motivation +[motivation]: #motivation + +NixOS commonly has 2 models of specifying configuration for programs, each with their own set of problems. This RFC aims to solve all of them. + +## Single option for every setting + +Having a single option for every setting in the configuration file, this often gets combined with an `extraConfig` option to provide greater flexibility. Problems: + +- Coupling to upstream + - When upstream adds or removes settings, the NixOS module needs to be updated to reflect that. + - Upstream adds a setting: If the module has an `extraConfig` option people might set the new setting there. But if we ever add it as a NixOS option, we'll have trouble merging the values together with what the user already specified in `extraConfig` + - Upstream removes a setting (backwards incompatible): The NixOS module is straight up broken in nixpkgs until somebody fixes it, end users can't fix it themselves (unless the module provides a `configFile` option which can override the generated one) + - The documentation of the upstream settings needs to be copied over to the NixOS options, which means it might get out of sync with changes + - The upstream defaults are being copied to the NixOS modules, so we need to also update the defaults whenever upstream changes them. This can be solved by using `nullOr` to allow for a `null` value to indicate that the upstream default shall be used, but that's not very nice. +- Option bloat: NixOS evaluation time is still linear in the number of *available* options for all users, even though everybody only uses a fraction of them. This means that when a module adds 100 new options, this leads to a direct increase in evaluation time for every `nixos-rebuild switch` of everybody using NixOS. With high confidence debunked in [#57477](https://github.com/NixOS/nixpkgs/issues/57477). +- Timeconsuming to implement, tedious to review and hard to maintain + - It takes a non-zero amount of time to write all the wanted options out into a NixOS module + - Reviewing is tedious because people need to make sure types are correct, descriptions are fitting, defaults are acceptable, for every option added. Due to the size and repetitiveness, people are also less willing to thoroughly review the code. + - The bigger the NixOS module is the harder it is to maintain, and the less people want to actually maintain it. +- Responsibility for backwards compatibility: By adding such options, we obligate ourselves to handle backwards incompatibilites on our side. We will have to support these options for a long time and can't remove them without at least one person being annoyed about it. + +## `configFile` or `extraConfig` option + +An option for specifying the contents of the configuration file directly with `configFile` or `extraConfig`. Problems: + +- Not very modular at all + - In case of json or a `configFile` option, you can only assign the option once, merging is impossible + - In case of ini (or similar), assigning a single option multiple times to make use of list concatenation, ordering or priorities is impossible, so in general you can't e.g. override a default value set somewhere else. +- No syntax checking: Users will have to know the syntax of the configuration language and encode their values properly, any syntax error will only be realized when the program errors at runtime. + +## Occurences of problems + +- Because prometheus uses options to encode every possible setting, [#56017](https://github.com/NixOS/nixpkgs/pull/56017) is needed to allow users to set a part of the configuration that wasn't encoded yet. +- Because strongswan-ctl uses options to encode its full configuration, changes like [#49197](https://github.com/NixOS/nixpkgs/pull/49197) are needed to update our options with upstream changes. +- Pull requests like [#57036](https://github.com/NixOS/nixpkgs/pull/57036) or [#38324](https://github.com/NixOS/nixpkgs/pull/38324) are needed because users wish to have more configuration options than the ones provided. + +## Previous discussions + +- https://github.com/NixOS/nixpkgs/pull/44923#issuecomment-412393196 +- https://github.com/NixOS/nixpkgs/pull/55957#issuecomment-464561483 -> https://github.com/NixOS/nixpkgs/pull/57716 + +## Implementations + +This idea has been implemented already in some places: +- [#45470](https://github.com/NixOS/nixpkgs/pull/45470) +- [#52096](https://github.com/NixOS/nixpkgs/pull/52096) +- [My Murmur module](https://github.com/Infinisil/system/blob/45c3ea36651a2f4328c8a7474148f1c5ecb18e0a/config/new-modules/murmur.nix) + +# Detailed design +[design]: #detailed-design + +For specifying configuration files to programs in NixOS options, there should be a main single option called `config` which represents the configuration of the program as a Nix value, which can then be converted to the configuration file format the program expects. This may look as follows: + +```nix +{ config, lib, ... }: with lib; +let + + cfg = config.services.foo; + + # Use port specified in config or the upstream default otherwise + # Needed to open the correct port + port = cfg.config.port or 2546; + + configText = configGen.json cfg.config; + +in { + + options.services.foo = { + enable = mkEnableOption "foo service"; + + config = mkOption { + type = lib.types.config.json; + default = {}; + description = '' + Configuration for foo. Refer to + for documentation on the supported values. + ''; + }; + }; + + config = mkIf cfg.enable { + + # Set minimal config to get service working by default + services.foo.config = { + dataPath = "/var/lib/foo"; + logLevel = mkDefault "WARN"; + }; + + environment.etc."foo.json".text = configText; + + networking.firewall.allowedTCPPorts = [ port ]; + # ... + }; +} +``` + +This approach solves all* of the above mentioned problems. In addition we have the following properties that work with this approach out of the box: +- Ability to easily query arbitrary configuration values with `nix-instantiate --eval '' -A config.services.foo.config` +- The configuration file is well formatted with the right amount of indentation everywhere +- Usually hardcoded defaults can now be replaced by simple assignment of the `config` option, which also allows people to override those values with `mkForce` + +*: The only problem it doesn't solve is the coupling to upstream defaults for options that need to be known at evaluation time, such as `port` in the example above, but that's not really avoidable. + +### Configuration types + +A set of types for common configuration formats should be provided in `lib.types.config`. Such a type should encode what values can be set in files of this configuration format as a Nix value, with the module system being able to merge multiple values correctly. This is the part that checks whether the user set an encodeable value. This can be extended over time, but could include the following as a start: +- JSON +- YAML, which is probably the same as JSON +- INI +- A simple `key=value` format +- A recursive `key.subkey.subsubkey=value` format + +Sometimes programs have their own configuration formats, in which case the type should be implemented in the program's module directly. + +### Configuration format writers + +To convert the Nix value into the configuration string, a set of configuration format writers should be provided under `lib.config`. These should make sure that the resulting text is somewhat properly formatted with readable indentation. These writers will have to include ones for all of the above-mentioned configuration types. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. + +### Documentation + +The nixpkgs manual should be updated to recommend this way of doing program configuration in modules, along with examples. + +## Limitations + +- Limited to configuration file formats representable conveniently in Nix, such as JSON, YAML, INI, key-value files, or similar formats. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. For those it is recommended to not hardcode anything and provide a `config` option with type `types.str` or `types.lines` if it makes sense to merge multiple assignments of it. + +## Additional config options +Sometimes it makes sense to have an additional NixOS option for a specific configuration setting. In general this should be discussed on a case-by-case basis to judge whether it makes sense. However keep in mind that it's always possible to add more options later on, but you can't as easily remove existing options. Instances of where it can make sense are: +- Settings that are necessary for the module to work and are different for every user, so they can't have a default. Examples are `services.dd-agent.api_key`, `services.davmail.url` or `services.hydra.hydraURL`. Having a separate option for these settings can give a much better error message when you don't set them (instead of failing at runtime or having to encode the requirement in an assertion) and better discoverability. +- Password settings: Some programs configuration files require passwords in them directly. Since we try to avoid having passwords in the Nix store, it is advisable to provide a `passwordFile` option as well, which would replace a placeholder password in the configuration file at runtime. + +This `config` approach described here is very flexible for these kind of additions, here's an example where we add a `domain` setting to the above service as a separate NixOS option: +```nix +{ config, lib, ... }: with lib; { + + options.services.foo = { + # ... + + domain = mkOption { + type = types.str; + description = "Domain this service operates on."; + }; + }; + + config = mkIf cfg.enable { + services.foo.config = { + # ... + domain = mkDefault cfg.domain; + }; + }; +} +``` + +Even though we have two ways of specifying this configuration setting now, the user is free to choose either way. + +# Drawbacks +[drawbacks]: #drawbacks + +There are some disadvantages to this approach: +- Types of configuration settings can't be checked*, which can lead to packages failing at runtime instead of evaluation time. If the program supports a mode for checking its configuration, this problem can be solved elegantly by using it. +- Documentation for the configuration settings will not be available in the central NixOS manual, instead the upstream documentation has to be used, which can be unfamiliar. + +*: It would still be possible to check them by using `types.addCheck` and adjusting the types description, but that sounds more painful than it's worth. + +# Alternatives +[alternatives]: #alternatives + +See [Motivation](#motivation) + +# Unresolved questions +[unresolved]: #unresolved-questions + +# Future work +[future]: #future-work + From 68f1b4420c289da6a55fd315395aae8fc1a548a8 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sun, 24 Mar 2019 04:15:17 +0100 Subject: [PATCH 02/43] fixup! Address comments, add Defaults and Configuration Checking sections --- rfcs/0042-config-option.md | 96 +++++++++++++++++++++++++++++++------- 1 file changed, 78 insertions(+), 18 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index b51e99e68..da419f855 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -25,7 +25,7 @@ Having a single option for every setting in the configuration file, this often g - Coupling to upstream - When upstream adds or removes settings, the NixOS module needs to be updated to reflect that. - Upstream adds a setting: If the module has an `extraConfig` option people might set the new setting there. But if we ever add it as a NixOS option, we'll have trouble merging the values together with what the user already specified in `extraConfig` - - Upstream removes a setting (backwards incompatible): The NixOS module is straight up broken in nixpkgs until somebody fixes it, end users can't fix it themselves (unless the module provides a `configFile` option which can override the generated one) + - Upstream removes a setting (backwards incompatible): The NixOS module is straight up broken in nixpkgs until somebody fixes it, end users can't fix it themselves (unless the module provides a `configFile` option which can override the generated one). The same can also happen when the user uses overlays or `disabledModules` to cause a module/package version mismatch. - The documentation of the upstream settings needs to be copied over to the NixOS options, which means it might get out of sync with changes - The upstream defaults are being copied to the NixOS modules, so we need to also update the defaults whenever upstream changes them. This can be solved by using `nullOr` to allow for a `null` value to indicate that the upstream default shall be used, but that's not very nice. - Option bloat: NixOS evaluation time is still linear in the number of *available* options for all users, even though everybody only uses a fraction of them. This means that when a module adds 100 new options, this leads to a direct increase in evaluation time for every `nixos-rebuild switch` of everybody using NixOS. With high confidence debunked in [#57477](https://github.com/NixOS/nixpkgs/issues/57477). @@ -72,23 +72,19 @@ For specifying configuration files to programs in NixOS options, there should be let cfg = config.services.foo; - - # Use port specified in config or the upstream default otherwise - # Needed to open the correct port - port = cfg.config.port or 2546; - + configText = configGen.json cfg.config; - + in { options.services.foo = { enable = mkEnableOption "foo service"; - + config = mkOption { type = lib.types.config.json; default = {}; description = '' - Configuration for foo. Refer to + Configuration for foo. Refer to for documentation on the supported values. ''; }; @@ -98,13 +94,16 @@ in { # Set minimal config to get service working by default services.foo.config = { + # We don't use mkDefault, as this module requires this value in order to function dataPath = "/var/lib/foo"; logLevel = mkDefault "WARN"; + # Upstream default, needed for us to open the firewall + port = mkDefault 2546; }; environment.etc."foo.json".text = configText; - networking.firewall.allowedTCPPorts = [ port ]; + networking.firewall.allowedTCPPorts = [ cfg.config.port ]; # ... }; } @@ -113,9 +112,72 @@ in { This approach solves all* of the above mentioned problems. In addition we have the following properties that work with this approach out of the box: - Ability to easily query arbitrary configuration values with `nix-instantiate --eval '' -A config.services.foo.config` - The configuration file is well formatted with the right amount of indentation everywhere -- Usually hardcoded defaults can now be replaced by simple assignment of the `config` option, which also allows people to override those values with `mkForce` +- Usually hardcoded defaults can now be replaced by simple assignment of the `config` option, which also allows people to override those values. See the [Defaults](#defaults) section for more details. + +*: The only problem it doesn't solve is the coupling to upstream defaults for required defaults, see the [Defaults](#defaults) section. + +### Defaults -*: The only problem it doesn't solve is the coupling to upstream defaults for options that need to be known at evaluation time, such as `port` in the example above, but that's not really avoidable. +Depending on how default settings matter, we need to set them differently and for different reasons: +- If the module needs a specific value for a setting because of how the module or NixOS works (e.g. `logger = "systemd"`, because NixOS uses `systemd` for logging), then the value should *not* use `mkDefault`. This way a user can't easily override this setting (which would break the module in some way) and will have to use `mkForce` instead to change it. This also indicates that they are leaving supported territory, and will probably have to change something else to make it work again (e.g. if they set `logger = mkForce "/var/log/foo"` they'll have to change their workflow of where to look for logs). +- If the program needs a setting to be present in the configuration file because otherwise it would fail at runtime and demand a value, the module should set this value *with* a `mkDefault` to the default upstream value, which will then be the equivalent of a starter configuration file. This allows users to easily change the value, but also enables a smooth first use of the module without having to manually set such defaults to get it to a working state. Optimally modules should Just Work (tm) by setting their `enable` option to true. +- If the module itself needs to know the value of a configuration setting at evaluation time in order to influence other options (e.g. opening the firewall for a services port), we may set upstream's default with a `mkDefault`, even though the program might start just fine without it. This allows the module to use the configuration setting directly without having to worry whether it is set at all at evaluation time. + +If all of the above points don't apply to a configuration setting, that is the module doesn't care about the value, the program doesn't care about the setting being present and we don't need the value at evaluation time, there should be no need to specify any default value. + +### Configuration checking + +One general downside of this approach is that the module system is not able to check the types and values of the configuration file, which would be fast, simple and give good error messages by default. While it would be possible to use `types.addCheck` for the type of the `config` option, this sounds more painful than it's worth and would lead to bad error messages, so we'll ignore this here. Here are some alternatives. + +#### Configuration checking tools + +A number of programs have tools for checking their configuration that don't need to start the program itself. We can use this to verify the configuration at **build time** by running the tool for a derivation build. While this is not as fast as if we had the module system do these checks, which would be at evaluation time already, it is faster than the program failing at runtime due to misconfiguration. These tools however are also more powerful than the module system and can integrate tightly with the program itself, allowing for more thorough checks. If a configuration checking tool is available, optimally by the program itself, it should be used if possible, as it can greatly improve user experience. The following illustrates an example of how this might look like + +```nix +{ config, pkgs, lib, ... }: with lib; + +let + + configText = configGen.json cfg.config; + + configFile = pkgs.runCommand "foo-config.json" { + # Because this program will be run at build time, we need `nativeBuildInputs` instead of `buildInputs` + nativeBuildInputs = [ pkgs.foo ]; + + inherit configText; + passAsFile = [ "configText" ]; + } '' + foo check-config $configTextPath + cp $configTextPath $out + ''; + +in { /* ... */ } +``` + +#### Ad-hoc checks with assertions + +While not as optimal as a configuration checker tool, assertions can be used to add flexible ad-hoc checks for type or other properties at **evaluation time**. It should only be used to ensure important properties that break the service in ways that are otherwise hard or slow to detect (and easy to detect for the module system), not for things that make the service fail to start anyways (unless there's a good reason for it). The following example only demonstrates how assertions can be used for checks, but any reasonable program should bail out early in such cases, which would make these assertions redundant, and only add more coupling to upstream, which we're trying to avoid. + +```nix +{ config, lib, ... }: with lib; { + # ... + config = mkIf cfg.enable { + # Examples only for demonstration purposes, don't actually add assertions for such properties + assertions = [ + { + assertion = cfg.config.enableLogging or true -> cfg.config ? logLevel; + message = "You also need to set `services.foo.config.logLevel` if `services.foo.config.enableLogging` is turned on."; + } + { + assertion = cfg.config ? port -> types.port.check cfg.config.port; + message = "${toString cfg.config.port} is not a valid port number for `services.foo.config.port`." + } + ] + }; +} +``` + +TODO: Are there any good examples of using assertions for configuration checks at all? ### Configuration types @@ -130,7 +192,7 @@ Sometimes programs have their own configuration formats, in which case the type ### Configuration format writers -To convert the Nix value into the configuration string, a set of configuration format writers should be provided under `lib.config`. These should make sure that the resulting text is somewhat properly formatted with readable indentation. These writers will have to include ones for all of the above-mentioned configuration types. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. +To convert the Nix value into the configuration string, a set of configuration format writers should be provided under `lib.configGen`. These should make sure that the resulting text is somewhat properly formatted with readable indentation. Things like `builtins.toJSON` are therefore not optimal as it doesn't add any spacing for readability. These writers will have to include ones for all of the above-mentioned configuration types. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. ### Documentation @@ -143,7 +205,7 @@ The nixpkgs manual should be updated to recommend this way of doing program conf ## Additional config options Sometimes it makes sense to have an additional NixOS option for a specific configuration setting. In general this should be discussed on a case-by-case basis to judge whether it makes sense. However keep in mind that it's always possible to add more options later on, but you can't as easily remove existing options. Instances of where it can make sense are: - Settings that are necessary for the module to work and are different for every user, so they can't have a default. Examples are `services.dd-agent.api_key`, `services.davmail.url` or `services.hydra.hydraURL`. Having a separate option for these settings can give a much better error message when you don't set them (instead of failing at runtime or having to encode the requirement in an assertion) and better discoverability. -- Password settings: Some programs configuration files require passwords in them directly. Since we try to avoid having passwords in the Nix store, it is advisable to provide a `passwordFile` option as well, which would replace a placeholder password in the configuration file at runtime. +- Password settings: Some program's configuration files require passwords in them directly. Since we try to avoid having passwords in the Nix store, it is advisable to provide a `passwordFile` option as well, which would replace a placeholder password in the configuration file at runtime. This `config` approach described here is very flexible for these kind of additions, here's an example where we add a `domain` setting to the above service as a separate NixOS option: ```nix @@ -173,10 +235,8 @@ Even though we have two ways of specifying this configuration setting now, the u [drawbacks]: #drawbacks There are some disadvantages to this approach: -- Types of configuration settings can't be checked*, which can lead to packages failing at runtime instead of evaluation time. If the program supports a mode for checking its configuration, this problem can be solved elegantly by using it. -- Documentation for the configuration settings will not be available in the central NixOS manual, instead the upstream documentation has to be used, which can be unfamiliar. - -*: It would still be possible to check them by using `types.addCheck` and adjusting the types description, but that sounds more painful than it's worth. +- If there is no configuration checking tool as explained in [this section](#configuration-checking-tools), the types of configuration settings can't be checked as easily, which can lead to packages failing at runtime instead of evaluation time. Refer to [Configuration checking](#configuration-checking) for more info. +- Documentation for the configuration settings will not be available in the central NixOS manual, instead the upstream documentation has to be used, which can be unfamiliar and harder to read. # Alternatives [alternatives]: #alternatives From feb2e40854bb8ec548721a40d1551ae9f655cc27 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sun, 24 Mar 2019 04:25:26 +0100 Subject: [PATCH 03/43] fixup! Missing semicolon --- rfcs/0042-config-option.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index da419f855..480c4b7aa 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -170,7 +170,7 @@ While not as optimal as a configuration checker tool, assertions can be used to } { assertion = cfg.config ? port -> types.port.check cfg.config.port; - message = "${toString cfg.config.port} is not a valid port number for `services.foo.config.port`." + message = "${toString cfg.config.port} is not a valid port number for `services.foo.config.port`."; } ] }; @@ -179,7 +179,11 @@ While not as optimal as a configuration checker tool, assertions can be used to TODO: Are there any good examples of using assertions for configuration checks at all? -### Configuration types +### Implementation + +The implementation consists of three separate parts + +#### Configuration types A set of types for common configuration formats should be provided in `lib.types.config`. Such a type should encode what values can be set in files of this configuration format as a Nix value, with the module system being able to merge multiple values correctly. This is the part that checks whether the user set an encodeable value. This can be extended over time, but could include the following as a start: - JSON @@ -190,11 +194,11 @@ A set of types for common configuration formats should be provided in `lib.types Sometimes programs have their own configuration formats, in which case the type should be implemented in the program's module directly. -### Configuration format writers +#### Configuration format writers To convert the Nix value into the configuration string, a set of configuration format writers should be provided under `lib.configGen`. These should make sure that the resulting text is somewhat properly formatted with readable indentation. Things like `builtins.toJSON` are therefore not optimal as it doesn't add any spacing for readability. These writers will have to include ones for all of the above-mentioned configuration types. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. -### Documentation +#### Documentation The nixpkgs manual should be updated to recommend this way of doing program configuration in modules, along with examples. From 3d072beac60e912a96a038a49559c9f77ca89e95 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sun, 24 Mar 2019 04:26:24 +0100 Subject: [PATCH 04/43] fixup! Add TODO note --- rfcs/0042-config-option.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 480c4b7aa..14c357817 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -250,6 +250,8 @@ See [Motivation](#motivation) # Unresolved questions [unresolved]: #unresolved-questions +Ctrl-F for TODO + # Future work [future]: #future-work From 3e60aab3bc8fb2757eb38eeadf10a07d0a6ace6d Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sun, 24 Mar 2019 15:45:11 +0100 Subject: [PATCH 05/43] fixup! Address review, missing semicolon --- rfcs/0042-config-option.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 14c357817..a6646ad77 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -123,7 +123,7 @@ Depending on how default settings matter, we need to set them differently and fo - If the program needs a setting to be present in the configuration file because otherwise it would fail at runtime and demand a value, the module should set this value *with* a `mkDefault` to the default upstream value, which will then be the equivalent of a starter configuration file. This allows users to easily change the value, but also enables a smooth first use of the module without having to manually set such defaults to get it to a working state. Optimally modules should Just Work (tm) by setting their `enable` option to true. - If the module itself needs to know the value of a configuration setting at evaluation time in order to influence other options (e.g. opening the firewall for a services port), we may set upstream's default with a `mkDefault`, even though the program might start just fine without it. This allows the module to use the configuration setting directly without having to worry whether it is set at all at evaluation time. -If all of the above points don't apply to a configuration setting, that is the module doesn't care about the value, the program doesn't care about the setting being present and we don't need the value at evaluation time, there should be no need to specify any default value. +If the above points don't apply to a configuration setting, that is the module doesn't care about the value, the program doesn't care about the setting being present and we don't need the value at evaluation time, there should be no need to specify any default value. ### Configuration checking @@ -131,7 +131,7 @@ One general downside of this approach is that the module system is not able to c #### Configuration checking tools -A number of programs have tools for checking their configuration that don't need to start the program itself. We can use this to verify the configuration at **build time** by running the tool for a derivation build. While this is not as fast as if we had the module system do these checks, which would be at evaluation time already, it is faster than the program failing at runtime due to misconfiguration. These tools however are also more powerful than the module system and can integrate tightly with the program itself, allowing for more thorough checks. If a configuration checking tool is available, optimally by the program itself, it should be used if possible, as it can greatly improve user experience. The following illustrates an example of how this might look like +A number of programs have tools for checking their configuration that don't need to start the program itself. We can use this to verify the configuration at **build time** by running the tool for a derivation build. While this is not as fast as if we had the module system do these checks, which would be at evaluation time already, it is faster than the program failing at runtime due to misconfiguration. These tools however are also more powerful than the module system and can integrate tightly with the program itself, allowing for more thorough checks. In addition, it reduces the amount of RAM needed for evaluation. If a configuration checking tool is available, optimally by the program itself, it should be used if possible, as it can greatly improve user experience. The following illustrates an example of how this might look like ```nix { config, pkgs, lib, ... }: with lib; @@ -172,7 +172,7 @@ While not as optimal as a configuration checker tool, assertions can be used to assertion = cfg.config ? port -> types.port.check cfg.config.port; message = "${toString cfg.config.port} is not a valid port number for `services.foo.config.port`."; } - ] + ]; }; } ``` From cde38bcf8dffdc5324c805a96395f5554ad57ee1 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sun, 24 Mar 2019 23:43:58 +0100 Subject: [PATCH 06/43] fixup! Add better manual defaults note to future work --- rfcs/0042-config-option.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index a6646ad77..ee6992325 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -255,3 +255,5 @@ Ctrl-F for TODO # Future work [future]: #future-work +- When defaults for NixOS options are set *outside* the options definition such as `config.services.foo.config.logLevel = "DEBUG"` above, it's currently not possible to see these default values in the manual. This could be improved by having the manual not only look at the option definitions `default` attribute for determining the default, but also evaluate the options value with a minimal configuration to get the actual default value. This might be non-trivial. + From 9a263ef239e1113d6602f3a2b4acdcaaf0eab270 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 27 Mar 2019 00:47:00 +0100 Subject: [PATCH 07/43] fixup! Add another referenc to a relevant PR --- rfcs/0042-config-option.md | 1 + 1 file changed, 1 insertion(+) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index ee6992325..5ac5e9277 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -49,6 +49,7 @@ An option for specifying the contents of the configuration file directly with `c - Because prometheus uses options to encode every possible setting, [#56017](https://github.com/NixOS/nixpkgs/pull/56017) is needed to allow users to set a part of the configuration that wasn't encoded yet. - Because strongswan-ctl uses options to encode its full configuration, changes like [#49197](https://github.com/NixOS/nixpkgs/pull/49197) are needed to update our options with upstream changes. - Pull requests like [#57036](https://github.com/NixOS/nixpkgs/pull/57036) or [#38324](https://github.com/NixOS/nixpkgs/pull/38324) are needed because users wish to have more configuration options than the ones provided. +- [#58239](https://github.com/NixOS/nixpkgs/pull/58239) ## Previous discussions From 88b29979b57daf3bd9160aec2357fe07cd30e415 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 27 Mar 2019 19:28:00 +0100 Subject: [PATCH 08/43] fixup! Add some todos and fix a sentence in the summary --- rfcs/0042-config-option.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 5ac5e9277..bafb061c6 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -11,7 +11,7 @@ related-issues: (will contain links to implementation PRs) A lot of NixOS options exist to specify single settings of configuration files. Along with such options come multiple disadvantages, such as having to synchronize with upstream changes, option bloat and more. An alternative is to provide options for passing the configuration as a string, such as the common `configFile` or `extraConfig` options, but this also comes with a set of disadvantages, including difficulty to override values and bad modularity. -This RFC proposes aims to solve these problems by encouraging NixOS module authors to provide an option for specifying the programs configuration file as a Nix value, providing a set of utility functions for writing these options conveniently, and updating the documentation to recommend this way of doing program configuration. +This RFC aims to solve these problems by encouraging NixOS module authors to provide an option for specifying the programs configuration file as a Nix value, providing a set of utility functions for writing these options conveniently, and updating the documentation to recommend this way of doing program configuration. # Motivation [motivation]: #motivation @@ -258,3 +258,7 @@ Ctrl-F for TODO - When defaults for NixOS options are set *outside* the options definition such as `config.services.foo.config.logLevel = "DEBUG"` above, it's currently not possible to see these default values in the manual. This could be improved by having the manual not only look at the option definitions `default` attribute for determining the default, but also evaluate the options value with a minimal configuration to get the actual default value. This might be non-trivial. + +TODO: +- Make a note that this RFC only focuses on configuration formats that can reasonably be represented in Nix, so things like nginx's config aren't included. +- Make a note and example for backwards compatibility with existing modules that have options for settings. From 9986adff0775a7b5eefeffabe2c5450eaf28bca5 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 28 Mar 2019 00:06:39 +0100 Subject: [PATCH 09/43] fixup! Add another relevant PR --- rfcs/0042-config-option.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index bafb061c6..cceed63ea 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -49,7 +49,7 @@ An option for specifying the contents of the configuration file directly with `c - Because prometheus uses options to encode every possible setting, [#56017](https://github.com/NixOS/nixpkgs/pull/56017) is needed to allow users to set a part of the configuration that wasn't encoded yet. - Because strongswan-ctl uses options to encode its full configuration, changes like [#49197](https://github.com/NixOS/nixpkgs/pull/49197) are needed to update our options with upstream changes. - Pull requests like [#57036](https://github.com/NixOS/nixpkgs/pull/57036) or [#38324](https://github.com/NixOS/nixpkgs/pull/38324) are needed because users wish to have more configuration options than the ones provided. -- [#58239](https://github.com/NixOS/nixpkgs/pull/58239) +- [#58239](https://github.com/NixOS/nixpkgs/pull/58239), [#58181](https://github.com/NixOS/nixpkgs/pull/58181) ## Previous discussions From d59771b88b126e0fd4823fb14a3d26db8e7b5522 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 3 Apr 2019 20:52:36 +0200 Subject: [PATCH 10/43] Add section on backwards compatibility for config settings --- rfcs/0042-config-option.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index cceed63ea..19b45b7f4 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -126,6 +126,33 @@ Depending on how default settings matter, we need to set them differently and fo If the above points don't apply to a configuration setting, that is the module doesn't care about the value, the program doesn't care about the setting being present and we don't need the value at evaluation time, there should be no need to specify any default value. +### Backwards compatibility for configuration settings + +By having a single option instead of many, we by default keep responsibility for backwards compatibility at upstream. This however also means that if upstream breaks backwards compatibility, instead of the NixOS module fixing it up, the user would have to do it themselves by adjusting their NixOS configuration. However, because such `config` options allow deep introspection into their values, it is possible to provide backwards compatibility in the module itself. This is possible by not using `config` directly to write the configuration file, but instead transforming it first, adjusting everything that's needed. For a simple `key = value` type configuration format, this could look as follows: + +```nix +{ config, lib, ... }: with lib; +let + cfg = config.services.foo; + + fixedUpConfig = let + renamedKeys = { + # foo has been renamed to bar + foo = "bar"; + }; + in + # Remove all renamed keys + removeAttrs cfg.config (attrNames renamedKeys) // + # Readd all renamed keys with their new name + mapAttrs' (name: value: + nameValuePair value cfg.config.${name} + ) (intersectAttrs cfg.config renamedKeys); +in + # ... +``` + +If this is needed in the future, we may add a set of config deprecation fix-up functions for general use in modules. + ### Configuration checking One general downside of this approach is that the module system is not able to check the types and values of the configuration file, which would be fast, simple and give good error messages by default. While it would be possible to use `types.addCheck` for the type of the `config` option, this sounds more painful than it's worth and would lead to bad error messages, so we'll ignore this here. Here are some alternatives. @@ -257,6 +284,7 @@ Ctrl-F for TODO [future]: #future-work - When defaults for NixOS options are set *outside* the options definition such as `config.services.foo.config.logLevel = "DEBUG"` above, it's currently not possible to see these default values in the manual. This could be improved by having the manual not only look at the option definitions `default` attribute for determining the default, but also evaluate the options value with a minimal configuration to get the actual default value. This might be non-trivial. +- If needed, add config transformation functions for keeping backwards compatibility with upstream changes. See [Backwards compatibility for configuration settings](#backwards-compatibility-for-configuration-settings) TODO: From 1231eaa79c5b50a72e9c633cd67c4d924fb912bc Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 3 Apr 2019 21:04:37 +0200 Subject: [PATCH 11/43] Expand section on configuration file format limitations --- rfcs/0042-config-option.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 19b45b7f4..ebbe93252 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -232,7 +232,9 @@ The nixpkgs manual should be updated to recommend this way of doing program conf ## Limitations -- Limited to configuration file formats representable conveniently in Nix, such as JSON, YAML, INI, key-value files, or similar formats. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. For those it is recommended to not hardcode anything and provide a `config` option with type `types.str` or `types.lines` if it makes sense to merge multiple assignments of it. +### Nix-representable configuration formats + +Limited to configuration file formats representable conveniently in Nix, such as JSON, YAML, INI, key-value files, or similar formats. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", this the answer is probably No, and you should not use this approach. For those it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as Firefox' `autoconfig.js`, having the form `pref("key", "value");`), a `config` option as described in this RFC can be used. ## Additional config options Sometimes it makes sense to have an additional NixOS option for a specific configuration setting. In general this should be discussed on a case-by-case basis to judge whether it makes sense. However keep in mind that it's always possible to add more options later on, but you can't as easily remove existing options. Instances of where it can make sense are: From 9e37c0e3f0d214880634ecb65e5e1642a99cea97 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 3 Apr 2019 21:15:01 +0200 Subject: [PATCH 12/43] Note that popular/main settings also qualify for having their own NixOS options --- rfcs/0042-config-option.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index ebbe93252..232ff0707 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -238,6 +238,7 @@ Limited to configuration file formats representable conveniently in Nix, such as ## Additional config options Sometimes it makes sense to have an additional NixOS option for a specific configuration setting. In general this should be discussed on a case-by-case basis to judge whether it makes sense. However keep in mind that it's always possible to add more options later on, but you can't as easily remove existing options. Instances of where it can make sense are: +- Popular or main settings. Because such `config` options will have to refer to upstream documentation for all available settings, it's much harder for new module users to figure out how they can configure it. Having popular/main settings as NixOS options is a good compromise. It is up to the module author to decide which options qualify for this. - Settings that are necessary for the module to work and are different for every user, so they can't have a default. Examples are `services.dd-agent.api_key`, `services.davmail.url` or `services.hydra.hydraURL`. Having a separate option for these settings can give a much better error message when you don't set them (instead of failing at runtime or having to encode the requirement in an assertion) and better discoverability. - Password settings: Some program's configuration files require passwords in them directly. Since we try to avoid having passwords in the Nix store, it is advisable to provide a `passwordFile` option as well, which would replace a placeholder password in the configuration file at runtime. @@ -270,7 +271,7 @@ Even though we have two ways of specifying this configuration setting now, the u There are some disadvantages to this approach: - If there is no configuration checking tool as explained in [this section](#configuration-checking-tools), the types of configuration settings can't be checked as easily, which can lead to packages failing at runtime instead of evaluation time. Refer to [Configuration checking](#configuration-checking) for more info. -- Documentation for the configuration settings will not be available in the central NixOS manual, instead the upstream documentation has to be used, which can be unfamiliar and harder to read. +- Documentation for the configuration settings will not be available in the central NixOS manual, instead the upstream documentation has to be used, which can be unfamiliar and harder to read. As a compromise, [additional NixOS options](#additional-config-options) can be used to bring part of the settings back into the NixOS documentation. # Alternatives [alternatives]: #alternatives From 166d8d45b8bf0650537540eef588d84fe818e736 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 3 Apr 2019 21:22:55 +0200 Subject: [PATCH 13/43] Move sections around to make more sense --- rfcs/0042-config-option.md | 121 +++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 60 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 232ff0707..3f925e856 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -56,7 +56,7 @@ An option for specifying the contents of the configuration file directly with `c - https://github.com/NixOS/nixpkgs/pull/44923#issuecomment-412393196 - https://github.com/NixOS/nixpkgs/pull/55957#issuecomment-464561483 -> https://github.com/NixOS/nixpkgs/pull/57716 -## Implementations +## Previous implementations This idea has been implemented already in some places: - [#45470](https://github.com/NixOS/nixpkgs/pull/45470) @@ -117,7 +117,7 @@ This approach solves all* of the above mentioned problems. In addition we have t *: The only problem it doesn't solve is the coupling to upstream defaults for required defaults, see the [Defaults](#defaults) section. -### Defaults +## Defaults Depending on how default settings matter, we need to set them differently and for different reasons: - If the module needs a specific value for a setting because of how the module or NixOS works (e.g. `logger = "systemd"`, because NixOS uses `systemd` for logging), then the value should *not* use `mkDefault`. This way a user can't easily override this setting (which would break the module in some way) and will have to use `mkForce` instead to change it. This also indicates that they are leaving supported territory, and will probably have to change something else to make it work again (e.g. if they set `logger = mkForce "/var/log/foo"` they'll have to change their workflow of where to look for logs). @@ -126,38 +126,43 @@ Depending on how default settings matter, we need to set them differently and fo If the above points don't apply to a configuration setting, that is the module doesn't care about the value, the program doesn't care about the setting being present and we don't need the value at evaluation time, there should be no need to specify any default value. -### Backwards compatibility for configuration settings +## Additional config options -By having a single option instead of many, we by default keep responsibility for backwards compatibility at upstream. This however also means that if upstream breaks backwards compatibility, instead of the NixOS module fixing it up, the user would have to do it themselves by adjusting their NixOS configuration. However, because such `config` options allow deep introspection into their values, it is possible to provide backwards compatibility in the module itself. This is possible by not using `config` directly to write the configuration file, but instead transforming it first, adjusting everything that's needed. For a simple `key = value` type configuration format, this could look as follows: +Sometimes it makes sense to have an additional NixOS option for a specific configuration setting. In general this should be discussed on a case-by-case basis to judge whether it makes sense. However keep in mind that it's always possible to add more options later on, but you can't as easily remove existing options. Instances of where it can make sense are: +- Popular or main settings. Because such `config` options will have to refer to upstream documentation for all available settings, it's much harder for new module users to figure out how they can configure it. Having popular/main settings as NixOS options is a good compromise. It is up to the module author to decide which options qualify for this. +- Settings that are necessary for the module to work and are different for every user, so they can't have a default. Examples are `services.dd-agent.api_key`, `services.davmail.url` or `services.hydra.hydraURL`. Having a separate option for these settings can give a much better error message when you don't set them (instead of failing at runtime or having to encode the requirement in an assertion) and better discoverability. +- Password settings: Some program's configuration files require passwords in them directly. Since we try to avoid having passwords in the Nix store, it is advisable to provide a `passwordFile` option as well, which would replace a placeholder password in the configuration file at runtime. +This `config` approach described here is very flexible for these kind of additions, here's an example where we add a `domain` setting to the above service as a separate NixOS option: ```nix -{ config, lib, ... }: with lib; -let - cfg = config.services.foo; +{ config, lib, ... }: with lib; { - fixedUpConfig = let - renamedKeys = { - # foo has been renamed to bar - foo = "bar"; + options.services.foo = { + # ... + + domain = mkOption { + type = types.str; + description = "Domain this service operates on."; }; - in - # Remove all renamed keys - removeAttrs cfg.config (attrNames renamedKeys) // - # Readd all renamed keys with their new name - mapAttrs' (name: value: - nameValuePair value cfg.config.${name} - ) (intersectAttrs cfg.config renamedKeys); -in - # ... + }; + + config = mkIf cfg.enable { + services.foo.config = { + # ... + domain = mkDefault cfg.domain; + }; + }; +} ``` -If this is needed in the future, we may add a set of config deprecation fix-up functions for general use in modules. +Even though we have two ways of specifying this configuration setting now, the user is free to choose either way. + -### Configuration checking +## Configuration checking One general downside of this approach is that the module system is not able to check the types and values of the configuration file, which would be fast, simple and give good error messages by default. While it would be possible to use `types.addCheck` for the type of the `config` option, this sounds more painful than it's worth and would lead to bad error messages, so we'll ignore this here. Here are some alternatives. -#### Configuration checking tools +### Configuration checking tools A number of programs have tools for checking their configuration that don't need to start the program itself. We can use this to verify the configuration at **build time** by running the tool for a derivation build. While this is not as fast as if we had the module system do these checks, which would be at evaluation time already, it is faster than the program failing at runtime due to misconfiguration. These tools however are also more powerful than the module system and can integrate tightly with the program itself, allowing for more thorough checks. In addition, it reduces the amount of RAM needed for evaluation. If a configuration checking tool is available, optimally by the program itself, it should be used if possible, as it can greatly improve user experience. The following illustrates an example of how this might look like @@ -182,7 +187,7 @@ let in { /* ... */ } ``` -#### Ad-hoc checks with assertions +### Ad-hoc checks with assertions While not as optimal as a configuration checker tool, assertions can be used to add flexible ad-hoc checks for type or other properties at **evaluation time**. It should only be used to ensure important properties that break the service in ways that are otherwise hard or slow to detect (and easy to detect for the module system), not for things that make the service fail to start anyways (unless there's a good reason for it). The following example only demonstrates how assertions can be used for checks, but any reasonable program should bail out early in such cases, which would make these assertions redundant, and only add more coupling to upstream, which we're trying to avoid. @@ -207,11 +212,38 @@ While not as optimal as a configuration checker tool, assertions can be used to TODO: Are there any good examples of using assertions for configuration checks at all? -### Implementation +## Backwards compatibility for configuration settings + +By having a single option instead of many, we by default keep responsibility for backwards compatibility at upstream. This however also means that if upstream breaks backwards compatibility, instead of the NixOS module fixing it up, the user would have to do it themselves by adjusting their NixOS configuration. However, because such `config` options allow deep introspection into their values, it is possible to provide backwards compatibility in the module itself. This is possible by not using `config` directly to write the configuration file, but instead transforming it first, adjusting everything that's needed. For a simple `key = value` type configuration format, this could look as follows: + +```nix +{ config, lib, ... }: with lib; +let + cfg = config.services.foo; + + fixedUpConfig = let + renamedKeys = { + # foo has been renamed to bar + foo = "bar"; + }; + in + # Remove all renamed keys + removeAttrs cfg.config (attrNames renamedKeys) // + # Readd all renamed keys with their new name + mapAttrs' (name: value: + nameValuePair value cfg.config.${name} + ) (intersectAttrs cfg.config renamedKeys); +in + # ... +``` + +If this is needed in the future, we may add a set of config deprecation fix-up functions for general use in modules. + +## Implementation parts -The implementation consists of three separate parts +The implementation consists of three separate parts. -#### Configuration types +### Configuration types A set of types for common configuration formats should be provided in `lib.types.config`. Such a type should encode what values can be set in files of this configuration format as a Nix value, with the module system being able to merge multiple values correctly. This is the part that checks whether the user set an encodeable value. This can be extended over time, but could include the following as a start: - JSON @@ -222,11 +254,11 @@ A set of types for common configuration formats should be provided in `lib.types Sometimes programs have their own configuration formats, in which case the type should be implemented in the program's module directly. -#### Configuration format writers +### Configuration format writers To convert the Nix value into the configuration string, a set of configuration format writers should be provided under `lib.configGen`. These should make sure that the resulting text is somewhat properly formatted with readable indentation. Things like `builtins.toJSON` are therefore not optimal as it doesn't add any spacing for readability. These writers will have to include ones for all of the above-mentioned configuration types. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. -#### Documentation +### Documentation The nixpkgs manual should be updated to recommend this way of doing program configuration in modules, along with examples. @@ -236,36 +268,6 @@ The nixpkgs manual should be updated to recommend this way of doing program conf Limited to configuration file formats representable conveniently in Nix, such as JSON, YAML, INI, key-value files, or similar formats. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", this the answer is probably No, and you should not use this approach. For those it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as Firefox' `autoconfig.js`, having the form `pref("key", "value");`), a `config` option as described in this RFC can be used. -## Additional config options -Sometimes it makes sense to have an additional NixOS option for a specific configuration setting. In general this should be discussed on a case-by-case basis to judge whether it makes sense. However keep in mind that it's always possible to add more options later on, but you can't as easily remove existing options. Instances of where it can make sense are: -- Popular or main settings. Because such `config` options will have to refer to upstream documentation for all available settings, it's much harder for new module users to figure out how they can configure it. Having popular/main settings as NixOS options is a good compromise. It is up to the module author to decide which options qualify for this. -- Settings that are necessary for the module to work and are different for every user, so they can't have a default. Examples are `services.dd-agent.api_key`, `services.davmail.url` or `services.hydra.hydraURL`. Having a separate option for these settings can give a much better error message when you don't set them (instead of failing at runtime or having to encode the requirement in an assertion) and better discoverability. -- Password settings: Some program's configuration files require passwords in them directly. Since we try to avoid having passwords in the Nix store, it is advisable to provide a `passwordFile` option as well, which would replace a placeholder password in the configuration file at runtime. - -This `config` approach described here is very flexible for these kind of additions, here's an example where we add a `domain` setting to the above service as a separate NixOS option: -```nix -{ config, lib, ... }: with lib; { - - options.services.foo = { - # ... - - domain = mkOption { - type = types.str; - description = "Domain this service operates on."; - }; - }; - - config = mkIf cfg.enable { - services.foo.config = { - # ... - domain = mkDefault cfg.domain; - }; - }; -} -``` - -Even though we have two ways of specifying this configuration setting now, the user is free to choose either way. - # Drawbacks [drawbacks]: #drawbacks @@ -291,5 +293,4 @@ Ctrl-F for TODO TODO: -- Make a note that this RFC only focuses on configuration formats that can reasonably be represented in Nix, so things like nginx's config aren't included. - Make a note and example for backwards compatibility with existing modules that have options for settings. From 1e0bdd69a6fa1099cdee145e9bb050c406a9e785 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 3 Apr 2019 21:37:55 +0200 Subject: [PATCH 14/43] Add limitation for backwards compatibility with existing modules --- rfcs/0042-config-option.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 3f925e856..fda5e6b06 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -266,7 +266,15 @@ The nixpkgs manual should be updated to recommend this way of doing program conf ### Nix-representable configuration formats -Limited to configuration file formats representable conveniently in Nix, such as JSON, YAML, INI, key-value files, or similar formats. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", this the answer is probably No, and you should not use this approach. For those it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as Firefox' `autoconfig.js`, having the form `pref("key", "value");`), a `config` option as described in this RFC can be used. +Limited to configuration file formats representable conveniently in Nix, such as JSON, YAML, INI, key-value files, or similar formats. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", this the answer is probably No, and you should not use this approach. + +For unsuitable formats it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as Firefox' `autoconfig.js`, having the form `pref("key", "value");`), a `config` option as described in this RFC can be used. + +### Backwards compatibility with existing modules + +This RFC has to be thought of as a basis for *new* modules first and foremost. By using this approach we can provide a good basis for a new module, with great flexibility for future changes. + +A lot of already existing NixOS modules provide a mix of options for single settings and `extraConfig`-style options, which as explained in the [Motivation](#motivation) section leads to problems. In general it is not easy or even impossible to convert such a module to the style described in this RFC in a backwards-compatible way without any workarounds. One workaround is to add an option `useLegacyConfig` or `declarative` which determines the modules behavior in regards to old options. # Drawbacks [drawbacks]: #drawbacks @@ -291,6 +299,3 @@ Ctrl-F for TODO - When defaults for NixOS options are set *outside* the options definition such as `config.services.foo.config.logLevel = "DEBUG"` above, it's currently not possible to see these default values in the manual. This could be improved by having the manual not only look at the option definitions `default` attribute for determining the default, but also evaluate the options value with a minimal configuration to get the actual default value. This might be non-trivial. - If needed, add config transformation functions for keeping backwards compatibility with upstream changes. See [Backwards compatibility for configuration settings](#backwards-compatibility-for-configuration-settings) - -TODO: -- Make a note and example for backwards compatibility with existing modules that have options for settings. From 7dcc00b4e2027f8dede5231002091525c0171d91 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 3 Apr 2019 22:04:29 +0200 Subject: [PATCH 15/43] Use roundcube as representable unrepresentable config format And reword a bit --- rfcs/0042-config-option.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index fda5e6b06..131555149 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -268,7 +268,7 @@ The nixpkgs manual should be updated to recommend this way of doing program conf Limited to configuration file formats representable conveniently in Nix, such as JSON, YAML, INI, key-value files, or similar formats. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", this the answer is probably No, and you should not use this approach. -For unsuitable formats it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as Firefox' `autoconfig.js`, having the form `pref("key", "value");`), a `config` option as described in this RFC can be used. +For unsuitable formats it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as [Roundcube's PHP configuration](https://github.com/NixOS/nixpkgs/blob/e03966a60f517700f5fee5182a5a798f8d0709df/nixos/modules/services/mail/roundcube.nix#L86-L93) of the form `$config['key'] = 'value';`), a `config` option as described in this RFC could be used as well as a `configFile` option for more flexibility if needed. ### Backwards compatibility with existing modules From dd9e628d463fab2d892f5ce44d5ed85ed301fb5e Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 4 Apr 2019 15:41:53 +0200 Subject: [PATCH 16/43] English fix: this -> then --- rfcs/0042-config-option.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 131555149..ad2ed864c 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -266,7 +266,7 @@ The nixpkgs manual should be updated to recommend this way of doing program conf ### Nix-representable configuration formats -Limited to configuration file formats representable conveniently in Nix, such as JSON, YAML, INI, key-value files, or similar formats. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", this the answer is probably No, and you should not use this approach. +Limited to configuration file formats representable conveniently in Nix, such as JSON, YAML, INI, key-value files, or similar formats. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", then the answer is probably No, and you should not use this approach. For unsuitable formats it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as [Roundcube's PHP configuration](https://github.com/NixOS/nixpkgs/blob/e03966a60f517700f5fee5182a5a798f8d0709df/nixos/modules/services/mail/roundcube.nix#L86-L93) of the form `$config['key'] = 'value';`), a `config` option as described in this RFC could be used as well as a `configFile` option for more flexibility if needed. From e9082a92e0f97613cbdc9cb40ed0f6eb5a7ccd5e Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Tue, 23 Apr 2019 02:31:29 +0200 Subject: [PATCH 17/43] Reword RFC to be more inline with more additional options --- rfcs/0042-config-option.md | 52 +++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index ad2ed864c..b729c1e42 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -11,7 +11,9 @@ related-issues: (will contain links to implementation PRs) A lot of NixOS options exist to specify single settings of configuration files. Along with such options come multiple disadvantages, such as having to synchronize with upstream changes, option bloat and more. An alternative is to provide options for passing the configuration as a string, such as the common `configFile` or `extraConfig` options, but this also comes with a set of disadvantages, including difficulty to override values and bad modularity. -This RFC aims to solve these problems by encouraging NixOS module authors to provide an option for specifying the programs configuration file as a Nix value, providing a set of utility functions for writing these options conveniently, and updating the documentation to recommend this way of doing program configuration. +This RFC aims to solve these problems by encouraging NixOS module authors to provide a `config` option as a base for specifying program configurations with great flexibility. A set of utility functions for writing these options conveniently will be added and the documentation will be updated to recommend this way of writing modules that do program configuration. + +This RFC does *not* intend to get rid of all NixOS options related to program configuration. The previous version of this RFC had this intention, but due to feedback (and insight on my own) this was changed. Read the section on [additional config options](#additional-config-options) for details. # Motivation [motivation]: #motivation @@ -66,7 +68,9 @@ This idea has been implemented already in some places: # Detailed design [design]: #detailed-design -For specifying configuration files to programs in NixOS options, there should be a main single option called `config` which represents the configuration of the program as a Nix value, which can then be converted to the configuration file format the program expects. This may look as follows: +For specifying configuration files to programs in NixOS options, there should be a main option called `config` (TODO: or `settings`, name to be decided), which represents the configuration of the program as a Nix value, which can then be converted to the configuration file format the program expects. In order for the most prominent/popular/main options of the package to be easily discoverage, they should still be specified as separate NixOS options, see the [additional config option](#additional-config-options) section for more details. + +As a result, modules will look as follows: ```nix { config, lib, ... }: with lib; @@ -82,24 +86,37 @@ in { enable = mkEnableOption "foo service"; config = mkOption { - type = lib.types.config.json; + type = types.config.json; default = {}; description = '' Configuration for foo. Refer to for documentation on the supported values. ''; }; + + # Because this option is a main/popular one we provide a separate + # option for it, to improved discoverability and error checking + domain = mkOption { + type = types.str; + description = '' + Domain this service operates on. + ''; + }; }; config = mkIf cfg.enable { # Set minimal config to get service working by default services.foo.config = { - # We don't use mkDefault, as this module requires this value in order to function - dataPath = "/var/lib/foo"; - logLevel = mkDefault "WARN"; + # We don't use mkDefault here, as this module requires this value in order to function + data_path = "/var/lib/foo"; + + log_level = mkDefault "WARN"; + # Upstream default, needed for us to open the firewall port = mkDefault 2546; + + domain = cfg.domain; }; environment.etc."foo.json".text = configText; @@ -110,12 +127,10 @@ in { } ``` -This approach solves all* of the above mentioned problems. In addition we have the following properties that work with this approach out of the box: -- Ability to easily query arbitrary configuration values with `nix-instantiate --eval '' -A config.services.foo.config` -- The configuration file is well formatted with the right amount of indentation everywhere -- Usually hardcoded defaults can now be replaced by simple assignment of the `config` option, which also allows people to override those values. See the [Defaults](#defaults) section for more details. - -*: The only problem it doesn't solve is the coupling to upstream defaults for required defaults, see the [Defaults](#defaults) section. +This approach solves all of the above mentioned problems for the settings we don't refer to in the module, the defaults we specify however are unavoidably still tied to upstream. In addition we have the following properties that work with this approach out of the box: +- Ability to easily query arbitrary configuration values with `nix-instantiate --eval '' -A config.services.foo.config` (TODO: does `nixos-option services.foo.config` work too?) +- The configuration file can be well formatted with the right amount of indentation everywhere +- Usually hardcoded defaults can now be replaced by simple assignment of the `config` option, which in addition allows people to override those values. See the [Defaults](#defaults) section for more details. ## Defaults @@ -128,12 +143,14 @@ If the above points don't apply to a configuration setting, that is the module d ## Additional config options -Sometimes it makes sense to have an additional NixOS option for a specific configuration setting. In general this should be discussed on a case-by-case basis to judge whether it makes sense. However keep in mind that it's always possible to add more options later on, but you can't as easily remove existing options. Instances of where it can make sense are: +For multiple reasons, one may wish to still have additional options available for configuration settings: - Popular or main settings. Because such `config` options will have to refer to upstream documentation for all available settings, it's much harder for new module users to figure out how they can configure it. Having popular/main settings as NixOS options is a good compromise. It is up to the module author to decide which options qualify for this. - Settings that are necessary for the module to work and are different for every user, so they can't have a default. Examples are `services.dd-agent.api_key`, `services.davmail.url` or `services.hydra.hydraURL`. Having a separate option for these settings can give a much better error message when you don't set them (instead of failing at runtime or having to encode the requirement in an assertion) and better discoverability. - Password settings: Some program's configuration files require passwords in them directly. Since we try to avoid having passwords in the Nix store, it is advisable to provide a `passwordFile` option as well, which would replace a placeholder password in the configuration file at runtime. -This `config` approach described here is very flexible for these kind of additions, here's an example where we add a `domain` setting to the above service as a separate NixOS option: +Keep in mind that while it's trivial to add new options with the approach in this RFC, removing them again is hard (even without this RFC). So instead of introducing a lot of additional options when the module gets written, authors should try to keep the initial number low, to not introduce options almost nobody will end up using. Every additional option might be nice to use, but increases coupling to upstream, burden on nixpkgs maintainers and bug potential. Thus we should strive towards a balance between too little and too many options, between "I have no idea how to use this module because it provides too little options" and "This module has become too problematic due to its size". And because this balance can only be approached from below (we can only add options, not remove them), additional options should be used conservatively from the start. + +This `config` approach described here is very flexible for these kind of additions. As already showcased in the above example, an implementation of such an option looks like this: ```nix { config, lib, ... }: with lib; { @@ -157,14 +174,13 @@ This `config` approach described here is very flexible for these kind of additio Even though we have two ways of specifying this configuration setting now, the user is free to choose either way. - ## Configuration checking -One general downside of this approach is that the module system is not able to check the types and values of the configuration file, which would be fast, simple and give good error messages by default. While it would be possible to use `types.addCheck` for the type of the `config` option, this sounds more painful than it's worth and would lead to bad error messages, so we'll ignore this here. Here are some alternatives. +One general downside of this approach is that the module system is not able to check the types and values of the configuration file, which could be fast, simple and give good error messages by default. While it would be possible to use `types.addCheck` for the type of the `config` option, this sounds more painful than it's worth and would lead to bad error messages, so we'll ignore this here. Here are some alternatives. ### Configuration checking tools -A number of programs have tools for checking their configuration that don't need to start the program itself. We can use this to verify the configuration at **build time** by running the tool for a derivation build. While this is not as fast as if we had the module system do these checks, which would be at evaluation time already, it is faster than the program failing at runtime due to misconfiguration. These tools however are also more powerful than the module system and can integrate tightly with the program itself, allowing for more thorough checks. In addition, it reduces the amount of RAM needed for evaluation. If a configuration checking tool is available, optimally by the program itself, it should be used if possible, as it can greatly improve user experience. The following illustrates an example of how this might look like +Occasionally programs have tools for checking their configuration without the need to start the program itself. We can use this to verify the configuration at **build time** by running the tool for a derivation build. While this is not as fast as if we had the module system do these checks, which would be at evaluation time already, it is better than the program failing at runtime due to misconfiguration. These tools however are also more powerful than the module system and can integrate tightly with the program itself, allowing for more thorough checks. In addition, it reduces the amount of RAM needed for evaluation. If a configuration checking tool is available, optimally by the program itself, it should be used if possible, as it can greatly improve user experience. The following illustrates an example of how this might look like ```nix { config, pkgs, lib, ... }: with lib; @@ -214,7 +230,7 @@ TODO: Are there any good examples of using assertions for configuration checks a ## Backwards compatibility for configuration settings -By having a single option instead of many, we by default keep responsibility for backwards compatibility at upstream. This however also means that if upstream breaks backwards compatibility, instead of the NixOS module fixing it up, the user would have to do it themselves by adjusting their NixOS configuration. However, because such `config` options allow deep introspection into their values, it is possible to provide backwards compatibility in the module itself. This is possible by not using `config` directly to write the configuration file, but instead transforming it first, adjusting everything that's needed. For a simple `key = value` type configuration format, this could look as follows: +By having a single option instead of many, we by default keep responsibility for backwards compatibility in upstream. This however also means that if upstream breaks backwards compatibility, instead of the NixOS module fixing it up, the user would have to do it themselves by adjusting their NixOS configuration. However, because such `config` options allow deep introspection into their values, it is possible to provide backwards compatibility in the module itself. This is possible by not using `config` directly to write the configuration file, but instead transforming it first, adjusting everything that's needed. For a simple `key = value` type configuration format, this could look as follows (TODO: Verify that this code works): ```nix { config, lib, ... }: with lib; From 0e832f325aed8aa48f23132843674becd2a539bb Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 29 Apr 2019 03:37:19 +0200 Subject: [PATCH 18/43] Add another previous implementation --- rfcs/0042-config-option.md | 1 + 1 file changed, 1 insertion(+) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index b729c1e42..0f211b76e 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -64,6 +64,7 @@ This idea has been implemented already in some places: - [#45470](https://github.com/NixOS/nixpkgs/pull/45470) - [#52096](https://github.com/NixOS/nixpkgs/pull/52096) - [My Murmur module](https://github.com/Infinisil/system/blob/45c3ea36651a2f4328c8a7474148f1c5ecb18e0a/config/new-modules/murmur.nix) +- [#55413](https://github.com/NixOS/nixpkgs/pull/55413) # Detailed design [design]: #detailed-design From d1a8974c2ffee9453c0b3e2da0bebb75b1e0bcb2 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 1 Jul 2019 01:26:53 +0200 Subject: [PATCH 19/43] Add TODO for option.*.files for config checks --- rfcs/0042-config-option.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 0f211b76e..3cf7d7582 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -204,6 +204,8 @@ let in { /* ... */ } ``` +TODO: Explain how `options.services.foo.config.files` can be used to give a better indication of where a failure occurs. + ### Ad-hoc checks with assertions While not as optimal as a configuration checker tool, assertions can be used to add flexible ad-hoc checks for type or other properties at **evaluation time**. It should only be used to ensure important properties that break the service in ways that are otherwise hard or slow to detect (and easy to detect for the module system), not for things that make the service fail to start anyways (unless there's a good reason for it). The following example only demonstrates how assertions can be used for checks, but any reasonable program should bail out early in such cases, which would make these assertions redundant, and only add more coupling to upstream, which we're trying to avoid. From ada0658c2b093d60977175fa8c97991ebd31c777 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 18 Jul 2019 16:07:39 +0200 Subject: [PATCH 20/43] Add shepherd leader/team MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Domen Kožar --- rfcs/0042-config-option.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 3cf7d7582..539a25c26 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -3,6 +3,8 @@ feature: config-option start-date: 2019-03-10 author: Silvan Mosberger co-authors: (find a buddy later to help our with the RFC) +shepherd-leader: Robin Gloster +shepherd-team: Robin Gloster, Eelco Dolstra, Robert Helgesson, Franz Pletz related-issues: (will contain links to implementation PRs) --- From 10d8b15c0f32123a9f7ec9a7255855d1ca862b22 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 19 Jul 2019 18:33:02 +0200 Subject: [PATCH 21/43] Rewrite: Summary and Motivation --- rfcs/0042-config-option.md | 116 ++++++++++++++++++++++++++++--------- 1 file changed, 89 insertions(+), 27 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 3cf7d7582..ed3b8e358 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -6,49 +6,89 @@ co-authors: (find a buddy later to help our with the RFC) related-issues: (will contain links to implementation PRs) --- +Contents: +- Mention part 1 (not using stringly typed extraConfig), mention the problems with stringly typed options +- Mention part 2 (encouraging most important options, high-quality, more advanced users can use config, options can't be removed, etc.). Mention the problems with many options. Show examples of maintenance PR's that were needed because of this, e.g. i2pc, prometheus +- Implementation + - Part 1: Adding formatters, types (mention how types.attrs doesn't work correctly) and docs + - Defaults + - Part 2: Adding docs for encouraging most important options +- Limitations +- Future work + - showing defaults in the manual + - Command line options +- Addendums + - Configuration checking + - Backwards compatibility + - configFile + # Summary [summary]: #summary -A lot of NixOS options exist to specify single settings of configuration files. Along with such options come multiple disadvantages, such as having to synchronize with upstream changes, option bloat and more. An alternative is to provide options for passing the configuration as a string, such as the common `configFile` or `extraConfig` options, but this also comes with a set of disadvantages, including difficulty to override values and bad modularity. +## Part 1: Structural `settings` instead of stringly `extraConfig` +[part1]: #part-1-structural-settings-instead-of-stringly-extraconfig -This RFC aims to solve these problems by encouraging NixOS module authors to provide a `config` option as a base for specifying program configurations with great flexibility. A set of utility functions for writing these options conveniently will be added and the documentation will be updated to recommend this way of writing modules that do program configuration. +NixOS modules often use stringly-typed options like `extraConfig` to allow specifying extra settings in addition to the default ones. This has multiple disadvantages: The defaults can't be changed, multiple values might not get merged properly, inspection of it is almost impossible because it's an opaque string and more. The first part of this RFC aims to solve such problems by discouraging such options and instead to use a `settings` option which encodes the modules configuration file as a structural generic Nix value. Here is an example showcasing some advantages: -This RFC does *not* intend to get rid of all NixOS options related to program configuration. The previous version of this RFC had this intention, but due to feedback (and insight on my own) this was changed. Read the section on [additional config options](#additional-config-options) for details. +```nix +{ + # Old way + services.foo.extraConfig = '' + # Can't be set in multiple files because string concatenation doesn't merge such lists + listen-ports = 456, 457, 458 + + # Can't override this setting because the module hardcodes it + # bootstrap-ips = 172.22.68.74 -# Motivation -[motivation]: #motivation + enable-ipv6 = 0 + ${optionalString isServer "check-interval = 3600"} + }; + + # New way + services.foo.settings = { + listen-ports = [ 456 457 458 ]; + bootstrap-ips = [ "172.22.68.74" ]; + enable-ipv6 = false; + check-interval = mkIf isServer 3600; + }; +} +``` -NixOS commonly has 2 models of specifying configuration for programs, each with their own set of problems. This RFC aims to solve all of them. +## Part 2: Balancing module option count +[part2]: #part-2-balancing-module-option-count -## Single option for every setting +Since with this approach there will be no more hardcoded defaults and composability is not a problem anymore, there is not a big need to have NixOS options for every setting anymore. Traditionally this has lead to huge NixOS modules with dozens of options, each of them only for a single field in the configuration. Such modules are problematic because they're hard to review and maintain, require constant care as to keep all those options in line with upstream changes, are generally of lower quality and more. Such options aren't without advantages however: They will be presented in the NixOS manual and can have better type checking than the equivalent with `settings`. -Having a single option for every setting in the configuration file, this often gets combined with an `extraConfig` option to provide greater flexibility. Problems: +The second part of this RFC aims to encourage module authors to strike a balance for the number of such additional options such as to not make the module too big, but still provide the most commonly used settings as separate NixOS options. Quality is encouraged over quantity: Authors should spend more time on thinking about which options to add, documenting them properly, thinking about interactions with the rest of NixOS, or even writing useful high-level options instead. This is in contrast to the fiddly labor of copying dozens of options from upstream to NixOS. With a `settings` option, it's also always easy to add more options over time in a backwards-compatible way, whereas removing options has always been practically impossible. + +# Motivation +[motivation]: #motivation -- Coupling to upstream - - When upstream adds or removes settings, the NixOS module needs to be updated to reflect that. - - Upstream adds a setting: If the module has an `extraConfig` option people might set the new setting there. But if we ever add it as a NixOS option, we'll have trouble merging the values together with what the user already specified in `extraConfig` - - Upstream removes a setting (backwards incompatible): The NixOS module is straight up broken in nixpkgs until somebody fixes it, end users can't fix it themselves (unless the module provides a `configFile` option which can override the generated one). The same can also happen when the user uses overlays or `disabledModules` to cause a module/package version mismatch. - - The documentation of the upstream settings needs to be copied over to the NixOS options, which means it might get out of sync with changes - - The upstream defaults are being copied to the NixOS modules, so we need to also update the defaults whenever upstream changes them. This can be solved by using `nullOr` to allow for a `null` value to indicate that the upstream default shall be used, but that's not very nice. -- Option bloat: NixOS evaluation time is still linear in the number of *available* options for all users, even though everybody only uses a fraction of them. This means that when a module adds 100 new options, this leads to a direct increase in evaluation time for every `nixos-rebuild switch` of everybody using NixOS. With high confidence debunked in [#57477](https://github.com/NixOS/nixpkgs/issues/57477). -- Timeconsuming to implement, tedious to review and hard to maintain - - It takes a non-zero amount of time to write all the wanted options out into a NixOS module - - Reviewing is tedious because people need to make sure types are correct, descriptions are fitting, defaults are acceptable, for every option added. Due to the size and repetitiveness, people are also less willing to thoroughly review the code. - - The bigger the NixOS module is the harder it is to maintain, and the less people want to actually maintain it. -- Responsibility for backwards compatibility: By adding such options, we obligate ourselves to handle backwards incompatibilites on our side. We will have to support these options for a long time and can't remove them without at least one person being annoyed about it. +## [Part 1][part1] -## `configFile` or `extraConfig` option +Stringly-typed options such as `extraConfig` have multiple disadvantages: +- Impossible to even implement correctly with configuration formats like JSON (because concatenation doesn't make sense) +- Bad modularity + - No proper merging: Multiple assignments get merged together with string concatenation which can't merge assignments of the same setting + - No priorities: `mkDefault` and co. won't work on settings +- Values within it can't be inspected, since it's just an opaque string +- Syntax of assigned values can easily be wrong, especially with escaping sequences -An option for specifying the contents of the configuration file directly with `configFile` or `extraConfig`. Problems: +## [Part 2][part2] -- Not very modular at all - - In case of json or a `configFile` option, you can only assign the option once, merging is impossible - - In case of ini (or similar), assigning a single option multiple times to make use of list concatenation, ordering or priorities is impossible, so in general you can't e.g. override a default value set somewhere else. -- No syntax checking: Users will have to know the syntax of the configuration language and encode their values properly, any syntax error will only be realized when the program errors at runtime. +NixOS modules with dozens of options aren't optimal for these reasons: +- Writing takes a lot of time and is a repetitive task of copying the settings from upstream +- Reviewing is tedious because of the big size of it, which in turn demotivates reviewers to even do it +- Maintenance is hard to keep up with because upstream can add/remove/change settings over time + - If the module copied defaults from upstream, these might need to be updated. This is especially important for security. A workaround is using `types.nullOr` with `null` signifying that the upstream default should be used, but that's not very nice. + - Documentation will get out of date as the package updates + - If upstream removes a setting, the NixOS module is broken for every user until somebody fixes it with a PR. +- With overlays or `disabledModules`, the user can bring the NixOS module out of sync with the package in nixpkgs, which can lead to the same problems as in the previous point. +- Responsibility for backwards compatibility is now not only in upstream, but also on our side. ## Occurences of problems -- Because prometheus uses options to encode every possible setting, [#56017](https://github.com/NixOS/nixpkgs/pull/56017) is needed to allow users to set a part of the configuration that wasn't encoded yet. +- Because prometheus uses options to encode every possible setting, PR's like [#56017](https://github.com/NixOS/nixpkgs/pull/56017) are needed to allow users to set a part of the configuration that wasn't encoded yet. - Because strongswan-ctl uses options to encode its full configuration, changes like [#49197](https://github.com/NixOS/nixpkgs/pull/49197) are needed to update our options with upstream changes. - Pull requests like [#57036](https://github.com/NixOS/nixpkgs/pull/57036) or [#38324](https://github.com/NixOS/nixpkgs/pull/38324) are needed because users wish to have more configuration options than the ones provided. - [#58239](https://github.com/NixOS/nixpkgs/pull/58239), [#58181](https://github.com/NixOS/nixpkgs/pull/58181) @@ -318,3 +358,25 @@ Ctrl-F for TODO - When defaults for NixOS options are set *outside* the options definition such as `config.services.foo.config.logLevel = "DEBUG"` above, it's currently not possible to see these default values in the manual. This could be improved by having the manual not only look at the option definitions `default` attribute for determining the default, but also evaluate the options value with a minimal configuration to get the actual default value. This might be non-trivial. - If needed, add config transformation functions for keeping backwards compatibility with upstream changes. See [Backwards compatibility for configuration settings](#backwards-compatibility-for-configuration-settings) + +# Arguments for Part 2 + +- We currently have a lot of NixOS modules with a lot of options. We can't really get rid of them to simplify the module. Removing/deprecating options would annoy users and give the feeling off that functionality is reduced. +- In comparison if the module is small to start with, we can easily add more options as needed. This gives off the idea that new functionality is added. + + + + +- Typed config instead of stringly-typed + +# The option count scale + +- config option only: Very simple module, easy to maintain, but very little documentation in the NixOS manual, almost everything is a runtime error +- Every program setting as an option: Very big module, hard to maintain, but everything is neatly documented and eval errors for a lot of things + +Somewhere in-between is the sweet spot, where the module has the necessary options to be useful for most people, but where it's still reviewable and maintainable. + +This sweet-spot is only approachable from below, because once you add an option, you can't really remove it without users being annoyed that it broke their setup or people feeling they get robbed of functionality. + + + From 18b3b4eb425f3d4601f0c675adbcceab883e41bc Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 19 Jul 2019 21:57:39 +0200 Subject: [PATCH 22/43] Links and stuff --- rfcs/0042-config-option.md | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index ed3b8e358..241d84b8a 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -25,6 +25,8 @@ Contents: # Summary [summary]: #summary +The RFC consists of two parts which flow hand-in-hand + ## Part 1: Structural `settings` instead of stringly `extraConfig` [part1]: #part-1-structural-settings-instead-of-stringly-extraconfig @@ -66,7 +68,7 @@ The second part of this RFC aims to encourage module authors to strike a balance ## [Part 1][part1] -Stringly-typed options such as `extraConfig` have multiple disadvantages: +Stringly-typed options such as `extraConfig` have multiple disadvantages in comparison to a structural `settings` option. - Impossible to even implement correctly with configuration formats like JSON (because concatenation doesn't make sense) - Bad modularity - No proper merging: Multiple assignments get merged together with string concatenation which can't merge assignments of the same setting @@ -84,28 +86,21 @@ NixOS modules with dozens of options aren't optimal for these reasons: - Documentation will get out of date as the package updates - If upstream removes a setting, the NixOS module is broken for every user until somebody fixes it with a PR. - With overlays or `disabledModules`, the user can bring the NixOS module out of sync with the package in nixpkgs, which can lead to the same problems as in the previous point. +- The bigger the module, the more likely it contains bugs - Responsibility for backwards compatibility is now not only in upstream, but also on our side. -## Occurences of problems - +Problem instances: +- The [i2pd module](https://github.com/NixOS/nixpkgs/blob/2a669d3ee1308c7fd73f15beb35c0456ff9202bc/nixos/modules/services/networking/i2pd.nix) has a long [history](https://github.com/NixOS/nixpkgs/commits/2a669d3ee1308c7fd73f15beb35c0456ff9202bc/nixos/modules/services/networking/i2pd.nix) of option additions due to upstream updates, bug fixes and documentation changes - Because prometheus uses options to encode every possible setting, PR's like [#56017](https://github.com/NixOS/nixpkgs/pull/56017) are needed to allow users to set a part of the configuration that wasn't encoded yet. - Because strongswan-ctl uses options to encode its full configuration, changes like [#49197](https://github.com/NixOS/nixpkgs/pull/49197) are needed to update our options with upstream changes. -- Pull requests like [#57036](https://github.com/NixOS/nixpkgs/pull/57036) or [#38324](https://github.com/NixOS/nixpkgs/pull/38324) are needed because users wish to have more configuration options than the ones provided. -- [#58239](https://github.com/NixOS/nixpkgs/pull/58239), [#58181](https://github.com/NixOS/nixpkgs/pull/58181) + +These are only examples of where people *found* problems and fixed them. The number of modules that have outdated options and require maintenance is probably rather high. ## Previous discussions - https://github.com/NixOS/nixpkgs/pull/44923#issuecomment-412393196 - https://github.com/NixOS/nixpkgs/pull/55957#issuecomment-464561483 -> https://github.com/NixOS/nixpkgs/pull/57716 -## Previous implementations - -This idea has been implemented already in some places: -- [#45470](https://github.com/NixOS/nixpkgs/pull/45470) -- [#52096](https://github.com/NixOS/nixpkgs/pull/52096) -- [My Murmur module](https://github.com/Infinisil/system/blob/45c3ea36651a2f4328c8a7474148f1c5ecb18e0a/config/new-modules/murmur.nix) -- [#55413](https://github.com/NixOS/nixpkgs/pull/55413) - # Detailed design [design]: #detailed-design @@ -380,3 +375,11 @@ This sweet-spot is only approachable from below, because once you add an option, + +### Previous implementations + +This idea has been implemented already in some places: +- [#45470](https://github.com/NixOS/nixpkgs/pull/45470) +- [#52096](https://github.com/NixOS/nixpkgs/pull/52096) +- [My Murmur module](https://github.com/Infinisil/system/blob/45c3ea36651a2f4328c8a7474148f1c5ecb18e0a/config/new-modules/murmur.nix) +- [#55413](https://github.com/NixOS/nixpkgs/pull/55413) From 39a4f80b5f444013f00fbbe5ba516b0687afff68 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 19 Jul 2019 23:12:54 +0200 Subject: [PATCH 23/43] Rewrite: Implementation part 1 --- rfcs/0042-config-option.md | 164 +++++++++++++++---------------------- 1 file changed, 65 insertions(+), 99 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 241d84b8a..985828391 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -104,80 +104,90 @@ These are only examples of where people *found* problems and fixed them. The num # Detailed design [design]: #detailed-design -For specifying configuration files to programs in NixOS options, there should be a main option called `config` (TODO: or `settings`, name to be decided), which represents the configuration of the program as a Nix value, which can then be converted to the configuration file format the program expects. In order for the most prominent/popular/main options of the package to be easily discoverage, they should still be specified as separate NixOS options, see the [additional config option](#additional-config-options) section for more details. +## [Part 1][part1] -As a result, modules will look as follows: +Whether having a structural `settings` option for a module makes sense depends on whether the program's configuration format has a direct mapping from Nix. This includes formats like JSON, YAML, INI and similar. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", then the answer is probably No, and you should not use this approach. This RFC does not specify anything for unsuitable configuration formats, but there is [an addendum on that][unsuitable]. -```nix -{ config, lib, ... }: with lib; -let +### Configuration format types - cfg = config.services.foo; +In order for a structural `settings` to enforce a valid value and work correctly with merging and priorities, it needs to have a type that corresponds to its configuration format, `types.attrs` won't do. As an example, the INI type could be represented with `attrsOf (attrsOf (nullOr (either int str)))`, which means there's multiple named sections, each of which can contain a key-value pair where the value is either `null`, an integer or a string, where `null` signifies a key not being present (which is useful for unsetting existing values). - configText = configGen.json cfg.config; +Common format types will be provided under `lib.types.settings`. This could include JSON, YAML, INI, a simple `key=value` format and a recursive `key.subkey.subsubkey=value` format for a start. Sometimes programs have their own configuration formats which are specific to them, in which case the type should be specified in that programs module directly instead of going in `lib.types.settings`. + +### Configuration format writers + +In order for the final value of `settings` to be turned into a string, a set of configuration format writers should be provided under `lib.settings`. These should ideally make sure that the resulting text is somewhat properly formatted with readable indentation. Things like `builtins.toJSON` are therefore not optimal as it doesn't add any spacing for readability. These writers will have to include ones for all of the above-mentioned configuration types. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. + +### Default values + +Ideally modules should work by just setting `enable = true`. Often this requires the configuration to include some default settings. Defaults should get specified in the `config` section of the module by assigning the values to the `settings` option directly. Depending on how default settings matter, we need to set them differently and for different reasons: +- If the program needs a setting to be present in the configuration file because otherwise it would fail at runtime and demand a value, the module should set this value with a `mkDefault` to the default upstream value, which will then be the equivalent of a starter configuration file. This allows users to easily change the value, but also enables a smooth first use of the module without having to manually set such defaults to get it to a working state. Note that this doesn't necessarily require the module to be updated when upstream defaults change, because that's the expected behavior with starter configurations. +- If the module needs a specific value for a setting because of how the module or NixOS works (e.g. `logger = "systemd"`, because NixOS uses `systemd` for logging), then the value should *not* use `mkDefault`. This way a user can't easily override this setting (which would break the module in some way) and will have to use `mkForce` instead to change it. This also indicates that they are leaving supported territory, and will probably have to change something else to make it work again (e.g. if they set `logger = mkForce "/var/log/foo"` they'll have to change their workflow of where to look for logs). +- If the module itself needs to know the value of a configuration setting at evaluation time in order to influence other options (e.g. opening the firewall for a services port), we may set upstream's default with a `mkDefault`, even though the program might start just fine without it. This allows the module to use the configuration setting directly without having to worry whether it is set at all at evaluation time. + +If the above points don't apply to a configuration setting, that is the module doesn't care about the value, the program doesn't care about the setting being present and we don't need the value at evaluation time, there should be no need to specify any default value. + +TODO: Nice table for default kind / how to set it / whether it needs to track upstream / examples + +### Additional options for single settings + +One can easily add additional options that correspond to single configuration settings. This is done by defining an option as usual, then applying it to `settings` with a `mkDefault`. This approach allows users to set the value either through the specialized option, or `settings`, which also means that new options can be added without any worry for backwards incompatibility. + +### Documentation + +The nixpkgs manual should be updated to explain this way of doing program configuration. + +### An example +Putting it all together, here is an example of a NixOS module that uses such an approach: + +```nix +{ config, lib, ... }: +let cfg = config.services.foo; in { options.services.foo = { - enable = mkEnableOption "foo service"; + enable = lib.mkEnableOption "foo service"; - config = mkOption { - type = types.config.json; + settings = lib.mkOption { + type = lib.types.settings.json; default = {}; description = '' - Configuration for foo. Refer to - for documentation on the supported values. + Configuration for foo, see ''; }; - # Because this option is a main/popular one we provide a separate - # option for it, to improved discoverability and error checking - domain = mkOption { - type = types.str; - description = '' - Domain this service operates on. - ''; + # An additional option for a setting so we have an eval error if this is missing + domain = lib.mkOption { + type = lib.types.str; + description = "Domain this service operates on."; }; }; - config = mkIf cfg.enable { - - # Set minimal config to get service working by default - services.foo.config = { - # We don't use mkDefault here, as this module requires this value in order to function + config = lib.mkIf cfg.enable { + services.foo.settings = { + # Fails at runtime without any value set + log_level = lib.mkDefault "WARN"; + + # We use systemd's `StateDirectory`, so we require this (no mkDefault) data_path = "/var/lib/foo"; - - log_level = mkDefault "WARN"; - - # Upstream default, needed for us to open the firewall - port = mkDefault 2546; - - domain = cfg.domain; + + # We use this to open the firewall, so we need to know about the default at eval time + port = lib.mkDefault 2546; + + # Apply our specialized setting. + domain = lib.mkDefault cfg.domain; }; - environment.etc."foo.json".text = configText; + environment.etc."foo/config.json".text = lib.settings.genJSON cfg.settings; - networking.firewall.allowedTCPPorts = [ cfg.config.port ]; + networking.firewall.allowedTCPPorts = [ cfg.settings.port ]; # ... }; } ``` -This approach solves all of the above mentioned problems for the settings we don't refer to in the module, the defaults we specify however are unavoidably still tied to upstream. In addition we have the following properties that work with this approach out of the box: -- Ability to easily query arbitrary configuration values with `nix-instantiate --eval '' -A config.services.foo.config` (TODO: does `nixos-option services.foo.config` work too?) -- The configuration file can be well formatted with the right amount of indentation everywhere -- Usually hardcoded defaults can now be replaced by simple assignment of the `config` option, which in addition allows people to override those values. See the [Defaults](#defaults) section for more details. - -## Defaults - -Depending on how default settings matter, we need to set them differently and for different reasons: -- If the module needs a specific value for a setting because of how the module or NixOS works (e.g. `logger = "systemd"`, because NixOS uses `systemd` for logging), then the value should *not* use `mkDefault`. This way a user can't easily override this setting (which would break the module in some way) and will have to use `mkForce` instead to change it. This also indicates that they are leaving supported territory, and will probably have to change something else to make it work again (e.g. if they set `logger = mkForce "/var/log/foo"` they'll have to change their workflow of where to look for logs). -- If the program needs a setting to be present in the configuration file because otherwise it would fail at runtime and demand a value, the module should set this value *with* a `mkDefault` to the default upstream value, which will then be the equivalent of a starter configuration file. This allows users to easily change the value, but also enables a smooth first use of the module without having to manually set such defaults to get it to a working state. Optimally modules should Just Work (tm) by setting their `enable` option to true. -- If the module itself needs to know the value of a configuration setting at evaluation time in order to influence other options (e.g. opening the firewall for a services port), we may set upstream's default with a `mkDefault`, even though the program might start just fine without it. This allows the module to use the configuration setting directly without having to worry whether it is set at all at evaluation time. - -If the above points don't apply to a configuration setting, that is the module doesn't care about the value, the program doesn't care about the setting being present and we don't need the value at evaluation time, there should be no need to specify any default value. - -## Additional config options +## [Part 2][part2] For multiple reasons, one may wish to still have additional options available for configuration settings: - Popular or main settings. Because such `config` options will have to refer to upstream documentation for all available settings, it's much harder for new module users to figure out how they can configure it. Having popular/main settings as NixOS options is a good compromise. It is up to the module author to decide which options qualify for this. @@ -186,30 +196,6 @@ For multiple reasons, one may wish to still have additional options available fo Keep in mind that while it's trivial to add new options with the approach in this RFC, removing them again is hard (even without this RFC). So instead of introducing a lot of additional options when the module gets written, authors should try to keep the initial number low, to not introduce options almost nobody will end up using. Every additional option might be nice to use, but increases coupling to upstream, burden on nixpkgs maintainers and bug potential. Thus we should strive towards a balance between too little and too many options, between "I have no idea how to use this module because it provides too little options" and "This module has become too problematic due to its size". And because this balance can only be approached from below (we can only add options, not remove them), additional options should be used conservatively from the start. -This `config` approach described here is very flexible for these kind of additions. As already showcased in the above example, an implementation of such an option looks like this: -```nix -{ config, lib, ... }: with lib; { - - options.services.foo = { - # ... - - domain = mkOption { - type = types.str; - description = "Domain this service operates on."; - }; - }; - - config = mkIf cfg.enable { - services.foo.config = { - # ... - domain = mkDefault cfg.domain; - }; - }; -} -``` - -Even though we have two ways of specifying this configuration setting now, the user is free to choose either way. - ## Configuration checking One general downside of this approach is that the module system is not able to check the types and values of the configuration file, which could be fast, simple and give good error messages by default. While it would be possible to use `types.addCheck` for the type of the `config` option, this sounds more painful than it's worth and would lead to bad error messages, so we'll ignore this here. Here are some alternatives. @@ -293,36 +279,9 @@ in If this is needed in the future, we may add a set of config deprecation fix-up functions for general use in modules. -## Implementation parts - -The implementation consists of three separate parts. - -### Configuration types - -A set of types for common configuration formats should be provided in `lib.types.config`. Such a type should encode what values can be set in files of this configuration format as a Nix value, with the module system being able to merge multiple values correctly. This is the part that checks whether the user set an encodeable value. This can be extended over time, but could include the following as a start: -- JSON -- YAML, which is probably the same as JSON -- INI -- A simple `key=value` format -- A recursive `key.subkey.subsubkey=value` format - -Sometimes programs have their own configuration formats, in which case the type should be implemented in the program's module directly. - -### Configuration format writers - -To convert the Nix value into the configuration string, a set of configuration format writers should be provided under `lib.configGen`. These should make sure that the resulting text is somewhat properly formatted with readable indentation. Things like `builtins.toJSON` are therefore not optimal as it doesn't add any spacing for readability. These writers will have to include ones for all of the above-mentioned configuration types. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. - -### Documentation - -The nixpkgs manual should be updated to recommend this way of doing program configuration in modules, along with examples. ## Limitations -### Nix-representable configuration formats - -Limited to configuration file formats representable conveniently in Nix, such as JSON, YAML, INI, key-value files, or similar formats. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", then the answer is probably No, and you should not use this approach. - -For unsuitable formats it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as [Roundcube's PHP configuration](https://github.com/NixOS/nixpkgs/blob/e03966a60f517700f5fee5182a5a798f8d0709df/nixos/modules/services/mail/roundcube.nix#L86-L93) of the form `$config['key'] = 'value';`), a `config` option as described in this RFC could be used as well as a `configFile` option for more flexibility if needed. ### Backwards compatibility with existing modules @@ -330,6 +289,13 @@ This RFC has to be thought of as a basis for *new* modules first and foremost. B A lot of already existing NixOS modules provide a mix of options for single settings and `extraConfig`-style options, which as explained in the [Motivation](#motivation) section leads to problems. In general it is not easy or even impossible to convert such a module to the style described in this RFC in a backwards-compatible way without any workarounds. One workaround is to add an option `useLegacyConfig` or `declarative` which determines the modules behavior in regards to old options. +## Addendums + +### Unsuitable configuration formats +[unsuitable]: #unsuitable-configuration-formats + +For unsuitable formats it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as [Roundcube's PHP configuration](https://github.com/NixOS/nixpkgs/blob/e03966a60f517700f5fee5182a5a798f8d0709df/nixos/modules/services/mail/roundcube.nix#L86-L93) of the form `$config['key'] = 'value';`), a `config` option as described in this RFC could be used as well as a `configFile` option for more flexibility if needed. + # Drawbacks [drawbacks]: #drawbacks From 0fb77580097373c71b7d720fe9d910211e2911d5 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 24 Jul 2019 23:15:56 +0200 Subject: [PATCH 24/43] Rewrite second part --- rfcs/0042-config-option.md | 190 ++++++++++++++++++------------------- 1 file changed, 92 insertions(+), 98 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 985828391..d0b1f8f38 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -7,8 +7,6 @@ related-issues: (will contain links to implementation PRs) --- Contents: -- Mention part 1 (not using stringly typed extraConfig), mention the problems with stringly typed options -- Mention part 2 (encouraging most important options, high-quality, more advanced users can use config, options can't be removed, etc.). Mention the problems with many options. Show examples of maintenance PR's that were needed because of this, e.g. i2pc, prometheus - Implementation - Part 1: Adding formatters, types (mention how types.attrs doesn't work correctly) and docs - Defaults @@ -81,6 +79,7 @@ Stringly-typed options such as `extraConfig` have multiple disadvantages in comp NixOS modules with dozens of options aren't optimal for these reasons: - Writing takes a lot of time and is a repetitive task of copying the settings from upstream - Reviewing is tedious because of the big size of it, which in turn demotivates reviewers to even do it +- The option listing will be filled with a lot of options almost nobody ever needs, which in turn makes it hard for people to find the options they *do* need. - Maintenance is hard to keep up with because upstream can add/remove/change settings over time - If the module copied defaults from upstream, these might need to be updated. This is especially important for security. A workaround is using `types.nullOr` with `null` signifying that the upstream default should be used, but that's not very nice. - Documentation will get out of date as the package updates @@ -88,6 +87,12 @@ NixOS modules with dozens of options aren't optimal for these reasons: - With overlays or `disabledModules`, the user can bring the NixOS module out of sync with the package in nixpkgs, which can lead to the same problems as in the previous point. - The bigger the module, the more likely it contains bugs - Responsibility for backwards compatibility is now not only in upstream, but also on our side. +- Making a module with many options is a one-way ticket, because options can't really be removed again. Smaller modules however can always scale up to more needs with more options. + +By not doing the tedious work of writing out dozens of options, module authors also have more time to do more meaningful work such as +- Writing a NixOS test +- Writing documentation +- Implementing high-level options that tie different NixOS modules together in non-trivial ways (e.g. `enableACME`) Problem instances: - The [i2pd module](https://github.com/NixOS/nixpkgs/blob/2a669d3ee1308c7fd73f15beb35c0456ff9202bc/nixos/modules/services/networking/i2pd.nix) has a long [history](https://github.com/NixOS/nixpkgs/commits/2a669d3ee1308c7fd73f15beb35c0456ff9202bc/nixos/modules/services/networking/i2pd.nix) of option additions due to upstream updates, bug fixes and documentation changes @@ -106,19 +111,15 @@ These are only examples of where people *found* problems and fixed them. The num ## [Part 1][part1] -Whether having a structural `settings` option for a module makes sense depends on whether the program's configuration format has a direct mapping from Nix. This includes formats like JSON, YAML, INI and similar. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", then the answer is probably No, and you should not use this approach. This RFC does not specify anything for unsuitable configuration formats, but there is [an addendum on that][unsuitable]. +### Additions to the NixOS documentation -### Configuration format types +#### Writing options for program configuration -In order for a structural `settings` to enforce a valid value and work correctly with merging and priorities, it needs to have a type that corresponds to its configuration format, `types.attrs` won't do. As an example, the INI type could be represented with `attrsOf (attrsOf (nullOr (either int str)))`, which means there's multiple named sections, each of which can contain a key-value pair where the value is either `null`, an integer or a string, where `null` signifies a key not being present (which is useful for unsetting existing values). +TODO: Write this section, followed by another section on `configFile` -Common format types will be provided under `lib.types.settings`. This could include JSON, YAML, INI, a simple `key=value` format and a recursive `key.subkey.subsubkey=value` format for a start. Sometimes programs have their own configuration formats which are specific to them, in which case the type should be specified in that programs module directly instead of going in `lib.types.settings`. - -### Configuration format writers - -In order for the final value of `settings` to be turned into a string, a set of configuration format writers should be provided under `lib.settings`. These should ideally make sure that the resulting text is somewhat properly formatted with readable indentation. Things like `builtins.toJSON` are therefore not optimal as it doesn't add any spacing for readability. These writers will have to include ones for all of the above-mentioned configuration types. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. +Whether having a structural `settings` option for a module makes sense depends on whether the program's configuration format has a direct mapping from Nix. This includes formats like JSON, YAML, INI and similar. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", then the answer is probably No, and you should not use this approach. This RFC does not specify anything for unsuitable configuration formats, but there is [an addendum on that][unsuitable]. -### Default values +#### Default values Ideally modules should work by just setting `enable = true`. Often this requires the configuration to include some default settings. Defaults should get specified in the `config` section of the module by assigning the values to the `settings` option directly. Depending on how default settings matter, we need to set them differently and for different reasons: - If the program needs a setting to be present in the configuration file because otherwise it would fail at runtime and demand a value, the module should set this value with a `mkDefault` to the default upstream value, which will then be the equivalent of a starter configuration file. This allows users to easily change the value, but also enables a smooth first use of the module without having to manually set such defaults to get it to a working state. Note that this doesn't necessarily require the module to be updated when upstream defaults change, because that's the expected behavior with starter configurations. @@ -129,15 +130,11 @@ If the above points don't apply to a configuration setting, that is the module d TODO: Nice table for default kind / how to set it / whether it needs to track upstream / examples -### Additional options for single settings +#### Additional options for single settings One can easily add additional options that correspond to single configuration settings. This is done by defining an option as usual, then applying it to `settings` with a `mkDefault`. This approach allows users to set the value either through the specialized option, or `settings`, which also means that new options can be added without any worry for backwards incompatibility. -### Documentation - -The nixpkgs manual should be updated to explain this way of doing program configuration. - -### An example +#### An example Putting it all together, here is an example of a NixOS module that uses such an approach: @@ -187,63 +184,85 @@ in { } ``` -## [Part 2][part2] +### Configuration format types -For multiple reasons, one may wish to still have additional options available for configuration settings: -- Popular or main settings. Because such `config` options will have to refer to upstream documentation for all available settings, it's much harder for new module users to figure out how they can configure it. Having popular/main settings as NixOS options is a good compromise. It is up to the module author to decide which options qualify for this. -- Settings that are necessary for the module to work and are different for every user, so they can't have a default. Examples are `services.dd-agent.api_key`, `services.davmail.url` or `services.hydra.hydraURL`. Having a separate option for these settings can give a much better error message when you don't set them (instead of failing at runtime or having to encode the requirement in an assertion) and better discoverability. -- Password settings: Some program's configuration files require passwords in them directly. Since we try to avoid having passwords in the Nix store, it is advisable to provide a `passwordFile` option as well, which would replace a placeholder password in the configuration file at runtime. +In order for a structural `settings` to enforce a valid value and work correctly with merging and priorities, it needs to have a type that corresponds to its configuration format, `types.attrs` won't do. As an example, the INI type could be represented with `attrsOf (attrsOf (nullOr (either int str)))`, which means there's multiple named sections, each of which can contain a key-value pair where the value is either `null`, an integer or a string, where `null` signifies a key not being present (which is useful for unsetting existing values). + +Common format types will be provided under `lib.types.settings`. This could include JSON, YAML, INI, a simple `key=value` format and a recursive `key.subkey.subsubkey=value` format for a start. Sometimes programs have their own configuration formats which are specific to them, in which case the type should be specified in that programs module directly instead of going in `lib.types.settings`. -Keep in mind that while it's trivial to add new options with the approach in this RFC, removing them again is hard (even without this RFC). So instead of introducing a lot of additional options when the module gets written, authors should try to keep the initial number low, to not introduce options almost nobody will end up using. Every additional option might be nice to use, but increases coupling to upstream, burden on nixpkgs maintainers and bug potential. Thus we should strive towards a balance between too little and too many options, between "I have no idea how to use this module because it provides too little options" and "This module has become too problematic due to its size". And because this balance can only be approached from below (we can only add options, not remove them), additional options should be used conservatively from the start. +### Configuration format writers -## Configuration checking +In order for the final value of `settings` to be turned into a string, a set of configuration format writers should be provided under `lib.settings`. These should ideally make sure that the resulting text is somewhat properly formatted with readable indentation. Things like `builtins.toJSON` are therefore not optimal as it doesn't add any spacing for readability. These writers will have to include ones for all of the above-mentioned configuration types. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. -One general downside of this approach is that the module system is not able to check the types and values of the configuration file, which could be fast, simple and give good error messages by default. While it would be possible to use `types.addCheck` for the type of the `config` option, this sounds more painful than it's worth and would lead to bad error messages, so we'll ignore this here. Here are some alternatives. +## [Part 2][part2] -### Configuration checking tools +TODO: write what goes in the NixOS docs -Occasionally programs have tools for checking their configuration without the need to start the program itself. We can use this to verify the configuration at **build time** by running the tool for a derivation build. While this is not as fast as if we had the module system do these checks, which would be at evaluation time already, it is better than the program failing at runtime due to misconfiguration. These tools however are also more powerful than the module system and can integrate tightly with the program itself, allowing for more thorough checks. In addition, it reduces the amount of RAM needed for evaluation. If a configuration checking tool is available, optimally by the program itself, it should be used if possible, as it can greatly improve user experience. The following illustrates an example of how this might look like +The second part of this RFC aims to encourage people to write better NixOS modules in terms of quality, maintainability and discoverability by limiting NixOS options representing single settings to a set of most "valuable" options. Of course in general "valuable" doesn't have a clear definition, so it's up to the module author and other people familiar with the program to decide on it, but in the following section some conventions will be given. As more such options are deemed "valuable" they can be added to the module over time. -```nix -{ config, pkgs, lib, ... }: with lib; +### Valuable options -let +TODO: Add table + +- Popular or main settings. Because such `settings` options will have to refer to upstream documentation for all available settings, it's much harder for new module users to figure out how they can configure it. Having popular/main settings as NixOS options is therefore a good idea +- Settings that are necessary for the module to work and are different for every user, so they can't have a default. Examples are `services.dd-agent.api_key`, `services.davmail.url` or `services.hydra.hydraURL`. Having a separate option for these settings can give a much better error message when you don't set them (instead of failing at runtime or having to encode the requirement in an assertion) and better discoverability. +- Password settings: Some program's configuration files require passwords in them directly. Since we try to avoid having passwords in the Nix store, it is advisable to provide a `passwordFile` option as well, which would replace a placeholder password in the configuration file at runtime. - configText = configGen.json cfg.config; +### Configuration checking - configFile = pkgs.runCommand "foo-config.json" { - # Because this program will be run at build time, we need `nativeBuildInputs` instead of `buildInputs` - nativeBuildInputs = [ pkgs.foo ]; +One downside of using `settings` instead of having a dedicated NixOS option is that values can't be checked to have the correct key and type at evaluation time. Instead the default mode of operation will be to fail at runtime when the program reads the configuration. There are ways this can be improved however. - inherit configText; - passAsFile = [ "configText" ]; +#### Configuration checking tools + +Occasionally programs have tools for checking their configuration without the need to start the program itself. We can use this to verify the configuration at build time by running the tool during a derivation build. These tools are generally more thorough than the module system and can integrate tightly with the program itself, which can greatly improve user experience. A good side effect of this approach is that less RAM is needed for evaluation. The following illustrates an example of how this might look like: + +TODO: Rewrite in terms of `configFile` +```nix +{ config, lib, pkgs, ... }: +let + cfg = config.services.foo; + + checkedConfig = pkgs.runCommandNoCC "foo-config.json" { + # Because this program will be run at build time, we need `nativeBuildInputs` + nativeBuildInputs = [ pkgs.foo-checker ]; + + config = lib.settings.genJSON cfg.settings; + passAsFile = [ "config" ]; } '' - foo check-config $configTextPath - cp $configTextPath $out + foo-checker "$configPath" + cp "$configPath" "$out" ''; -in { /* ... */ } +in { + options = { /* ... */ }; + config = lib.mkIf cfg.enable { + + environment.etc."foo/config.json".source = checkedConfig; + + # ... + }; +} ``` TODO: Explain how `options.services.foo.config.files` can be used to give a better indication of where a failure occurs. ### Ad-hoc checks with assertions -While not as optimal as a configuration checker tool, assertions can be used to add flexible ad-hoc checks for type or other properties at **evaluation time**. It should only be used to ensure important properties that break the service in ways that are otherwise hard or slow to detect (and easy to detect for the module system), not for things that make the service fail to start anyways (unless there's a good reason for it). The following example only demonstrates how assertions can be used for checks, but any reasonable program should bail out early in such cases, which would make these assertions redundant, and only add more coupling to upstream, which we're trying to avoid. +While not as good as a configuration checker tool, assertions can be used to add flexible ad-hoc checks for type or other properties at evaluation time. It should only be used to ensure important properties that break the service in ways that are otherwise hard or slow to detect (and easy to detect for the module system), not for things that make the service fail to start anyways (unless there's a good reason for it). The following example only demonstrates how assertions can be used for checks, but any reasonable program should bail out early in such cases, which would make these assertions redundant, and only add more coupling to upstream, which we're trying to avoid in the first place. ```nix -{ config, lib, ... }: with lib; { +{ config, lib, ... }: { # ... - config = mkIf cfg.enable { + config = lib.mkIf cfg.enable { # Examples only for demonstration purposes, don't actually add assertions for such properties assertions = [ { - assertion = cfg.config.enableLogging or true -> cfg.config ? logLevel; - message = "You also need to set `services.foo.config.logLevel` if `services.foo.config.enableLogging` is turned on."; + assertion = cfg.settings.enableLogging or true -> cfg.settings ? logLevel; + message = "You also need to set `services.foo.settings.logLevel` if `services.foo.settings.enableLogging` is turned on."; } { - assertion = cfg.config ? port -> types.port.check cfg.config.port; - message = "${toString cfg.config.port} is not a valid port number for `services.foo.config.port`."; + assertion = cfg.settings ? port -> lib.types.port.check cfg.settings.port; + message = "${toString cfg.settings.port} is not a valid port number for `services.foo.settings.port`."; } ]; }; @@ -252,49 +271,40 @@ While not as optimal as a configuration checker tool, assertions can be used to TODO: Are there any good examples of using assertions for configuration checks at all? -## Backwards compatibility for configuration settings +### Backwards compatibility for configuration settings -By having a single option instead of many, we by default keep responsibility for backwards compatibility in upstream. This however also means that if upstream breaks backwards compatibility, instead of the NixOS module fixing it up, the user would have to do it themselves by adjusting their NixOS configuration. However, because such `config` options allow deep introspection into their values, it is possible to provide backwards compatibility in the module itself. This is possible by not using `config` directly to write the configuration file, but instead transforming it first, adjusting everything that's needed. For a simple `key = value` type configuration format, this could look as follows (TODO: Verify that this code works): +By shifting values from a specific NixOS option to the general `settings` one, guarding against upstream changes will have to be done differently. Due to the structural nature of `settings` options, it's possible to deeply inspect and rewrite them however needed before converting them to a string. If the need arises, convenience library functions can be written for such transformations. This might look as follows: ```nix -{ config, lib, ... }: with lib; +{ config, lib, ... }: let cfg = config.services.foo; - - fixedUpConfig = let - renamedKeys = { - # foo has been renamed to bar - foo = "bar"; - }; - in - # Remove all renamed keys - removeAttrs cfg.config (attrNames renamedKeys) // - # Readd all renamed keys with their new name - mapAttrs' (name: value: - nameValuePair value cfg.config.${name} - ) (intersectAttrs cfg.config renamedKeys); -in - # ... + + fixedUpSettings = + let + renamedKeys = builtins.intersectAttrs cfg.settings { + # foo has been renamed to bar + foo = "bar"; + }; + conflicts = lib.filter (from: cfg.settings ? ${renamedKeys.${from}}) (lib.attrNames renamedKeys); + in if conflicts == [] then lib.mapAttrs' (from: to: + lib.nameValuePair to cfg.settings.${from} + ) renamedKeys // builtins.removeAttrs cfg.settings (lib.attrNames renamedKeys) + else throw (lib.concatMapStringsSep "," (from: + "Can't mix the deprecated setting \"${from}\" with its replacement \"${renamedKeys.${from}}\"" + ) conflicts); + +in { + config.environment.etc."foo/config.json".text = lib.settings.genJSON fixedUpsettings; +} ``` -If this is needed in the future, we may add a set of config deprecation fix-up functions for general use in modules. - - -## Limitations - - -### Backwards compatibility with existing modules +## Backwards compatibility with existing modules This RFC has to be thought of as a basis for *new* modules first and foremost. By using this approach we can provide a good basis for a new module, with great flexibility for future changes. A lot of already existing NixOS modules provide a mix of options for single settings and `extraConfig`-style options, which as explained in the [Motivation](#motivation) section leads to problems. In general it is not easy or even impossible to convert such a module to the style described in this RFC in a backwards-compatible way without any workarounds. One workaround is to add an option `useLegacyConfig` or `declarative` which determines the modules behavior in regards to old options. -## Addendums - -### Unsuitable configuration formats -[unsuitable]: #unsuitable-configuration-formats - -For unsuitable formats it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as [Roundcube's PHP configuration](https://github.com/NixOS/nixpkgs/blob/e03966a60f517700f5fee5182a5a798f8d0709df/nixos/modules/services/mail/roundcube.nix#L86-L93) of the form `$config['key'] = 'value';`), a `config` option as described in this RFC could be used as well as a `configFile` option for more flexibility if needed. # Drawbacks [drawbacks]: #drawbacks @@ -319,30 +329,14 @@ Ctrl-F for TODO - When defaults for NixOS options are set *outside* the options definition such as `config.services.foo.config.logLevel = "DEBUG"` above, it's currently not possible to see these default values in the manual. This could be improved by having the manual not only look at the option definitions `default` attribute for determining the default, but also evaluate the options value with a minimal configuration to get the actual default value. This might be non-trivial. - If needed, add config transformation functions for keeping backwards compatibility with upstream changes. See [Backwards compatibility for configuration settings](#backwards-compatibility-for-configuration-settings) +# Addendums -# Arguments for Part 2 - -- We currently have a lot of NixOS modules with a lot of options. We can't really get rid of them to simplify the module. Removing/deprecating options would annoy users and give the feeling off that functionality is reduced. -- In comparison if the module is small to start with, we can easily add more options as needed. This gives off the idea that new functionality is added. - - - - -- Typed config instead of stringly-typed - -# The option count scale - -- config option only: Very simple module, easy to maintain, but very little documentation in the NixOS manual, almost everything is a runtime error -- Every program setting as an option: Very big module, hard to maintain, but everything is neatly documented and eval errors for a lot of things - -Somewhere in-between is the sweet spot, where the module has the necessary options to be useful for most people, but where it's still reviewable and maintainable. - -This sweet-spot is only approachable from below, because once you add an option, you can't really remove it without users being annoyed that it broke their setup or people feeling they get robbed of functionality. - - +## Unsuitable configuration formats +[unsuitable]: #unsuitable-configuration-formats +For unsuitable formats it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as [Roundcube's PHP configuration](https://github.com/NixOS/nixpkgs/blob/e03966a60f517700f5fee5182a5a798f8d0709df/nixos/modules/services/mail/roundcube.nix#L86-L93) of the form `$config['key'] = 'value';`), a `config` option as described in this RFC could be used as well as a `configFile` option for more flexibility if needed. -### Previous implementations +## Previous implementations This idea has been implemented already in some places: - [#45470](https://github.com/NixOS/nixpkgs/pull/45470) From b50d1a33e2e292af2b7a935a2ceea5b656f57c38 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 24 Jul 2019 23:26:57 +0200 Subject: [PATCH 25/43] Better part 1 summary --- rfcs/0042-config-option.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index d0b1f8f38..d513525f7 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -28,7 +28,7 @@ The RFC consists of two parts which flow hand-in-hand ## Part 1: Structural `settings` instead of stringly `extraConfig` [part1]: #part-1-structural-settings-instead-of-stringly-extraconfig -NixOS modules often use stringly-typed options like `extraConfig` to allow specifying extra settings in addition to the default ones. This has multiple disadvantages: The defaults can't be changed, multiple values might not get merged properly, inspection of it is almost impossible because it's an opaque string and more. The first part of this RFC aims to solve such problems by discouraging such options and instead to use a `settings` option which encodes the modules configuration file as a structural generic Nix value. Here is an example showcasing some advantages: +NixOS modules often use stringly-typed options like `extraConfig` to allow specifying extra settings in addition to the default ones. This has multiple disadvantages: The defaults can't be changed, multiple values might not get merged properly, inspection of it is almost impossible because it's an opaque string and more. The first part of this RFC aims to discourage such options and proposes generic `settings` options instead, which can encode the modules configuration file as a structural Nix value. Here is an example showcasing some advantages: ```nix { @@ -42,7 +42,7 @@ NixOS modules often use stringly-typed options like `extraConfig` to allow speci enable-ipv6 = 0 ${optionalString isServer "check-interval = 3600"} - }; + ''; # New way services.foo.settings = { From 3630c2c525a6fc3e3466a07eb4c3c49270172ca6 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Wed, 24 Jul 2019 23:45:36 +0200 Subject: [PATCH 26/43] Reformulate second part summary --- rfcs/0042-config-option.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index d513525f7..93d9602a6 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -57,9 +57,9 @@ NixOS modules often use stringly-typed options like `extraConfig` to allow speci ## Part 2: Balancing module option count [part2]: #part-2-balancing-module-option-count -Since with this approach there will be no more hardcoded defaults and composability is not a problem anymore, there is not a big need to have NixOS options for every setting anymore. Traditionally this has lead to huge NixOS modules with dozens of options, each of them only for a single field in the configuration. Such modules are problematic because they're hard to review and maintain, require constant care as to keep all those options in line with upstream changes, are generally of lower quality and more. Such options aren't without advantages however: They will be presented in the NixOS manual and can have better type checking than the equivalent with `settings`. +Since with this approach there will be no more hardcoded defaults and composability is not a problem anymore, there is not a big need to have NixOS options for every setting anymore. Traditionally this has lead to huge modules with dozens of options, each of them only for a single field in the configuration. Such modules are problematic because they're hard to write, review and maintain, are generally of lower quality, fill the option listing with noise and more. Additional options aren't without advantages however: They are presented in the NixOS manual and can have better type checking than the equivalent with `settings`. -The second part of this RFC aims to encourage module authors to strike a balance for the number of such additional options such as to not make the module too big, but still provide the most commonly used settings as separate NixOS options. Quality is encouraged over quantity: Authors should spend more time on thinking about which options to add, documenting them properly, thinking about interactions with the rest of NixOS, or even writing useful high-level options instead. This is in contrast to the fiddly labor of copying dozens of options from upstream to NixOS. With a `settings` option, it's also always easy to add more options over time in a backwards-compatible way, whereas removing options has always been practically impossible. +The second part of this RFC aims to encourage module authors to strike a balance for the number of additional options such as to not make the module too big, but still provide the most commonly used settings as separate options. Quality is encouraged over quantity: Authors should spend more time on writing documentation, NixOS tests or useful high-level abstractions. This is in contrast to the fiddly labor of copying dozens of options from upstream to NixOS. With a `settings` option, it's also very easy to add additional options over time if the need arises. In contrast, removing options has always been nigh impossible. # Motivation [motivation]: #motivation From abd729f7890f636f28232547381c2c7d7b241049 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 25 Jul 2019 00:10:02 +0200 Subject: [PATCH 27/43] Move around a bit and extend future work --- rfcs/0042-config-option.md | 110 ++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 49 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 93d9602a6..083cf03b1 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -15,10 +15,6 @@ Contents: - Future work - showing defaults in the manual - Command line options -- Addendums - - Configuration checking - - Backwards compatibility - - configFile # Summary [summary]: #summary @@ -99,12 +95,8 @@ Problem instances: - Because prometheus uses options to encode every possible setting, PR's like [#56017](https://github.com/NixOS/nixpkgs/pull/56017) are needed to allow users to set a part of the configuration that wasn't encoded yet. - Because strongswan-ctl uses options to encode its full configuration, changes like [#49197](https://github.com/NixOS/nixpkgs/pull/49197) are needed to update our options with upstream changes. -These are only examples of where people *found* problems and fixed them. The number of modules that have outdated options and require maintenance is probably rather high. +These are only examples of where people *found* problems and fixed them. The number of modules that have outdated options and require maintenance is probably much higher. -## Previous discussions - -- https://github.com/NixOS/nixpkgs/pull/44923#issuecomment-412393196 -- https://github.com/NixOS/nixpkgs/pull/55957#issuecomment-464561483 -> https://github.com/NixOS/nixpkgs/pull/57716 # Detailed design [design]: #detailed-design @@ -208,11 +200,63 @@ TODO: Add table - Settings that are necessary for the module to work and are different for every user, so they can't have a default. Examples are `services.dd-agent.api_key`, `services.davmail.url` or `services.hydra.hydraURL`. Having a separate option for these settings can give a much better error message when you don't set them (instead of failing at runtime or having to encode the requirement in an assertion) and better discoverability. - Password settings: Some program's configuration files require passwords in them directly. Since we try to avoid having passwords in the Nix store, it is advisable to provide a `passwordFile` option as well, which would replace a placeholder password in the configuration file at runtime. -### Configuration checking + +## Backwards compatibility with existing modules + +This RFC has to be thought of as a basis for *new* modules first and foremost. By using this approach we can provide a good basis for a new module, with great flexibility for future changes. + +A lot of already existing NixOS modules provide a mix of options for single settings and `extraConfig`-style options, which as explained in the [Motivation](#motivation) section leads to problems. In general it is not easy or even impossible to convert such a module to the style described in this RFC in a backwards-compatible way without any workarounds. One workaround is to add an option `useLegacyConfig` or `declarative` which determines the modules behavior in regards to old options. + + +# Drawbacks +[drawbacks]: #drawbacks + +There are some disadvantages to this approach: +- If there is no configuration checking tool as explained in [this section](#configuration-checking-tools), the types of configuration settings can't be checked as easily, which can lead to packages failing at runtime instead of evaluation time. Refer to [Configuration checking](#configuration-checking) for more info. +- Documentation for the configuration settings will not be available in the central NixOS manual, instead the upstream documentation has to be used, which can be unfamiliar and harder to read. As a compromise, [additional NixOS options](#additional-config-options) can be used to bring part of the settings back into the NixOS documentation. + +# Alternatives +[alternatives]: #alternatives + +See [Motivation](#motivation) + +# Unresolved questions +[unresolved]: #unresolved-questions + +Ctrl-F for TODO + +# Future work +[future]: #future-work + +## Documentation defaults +When defaults for NixOS options are set *outside* the options definition such as `config.services.foo.settings.log_level = lib.mkDefault "WARN"` above, it's currently not possible to see these default values in the manual. This could be improved by having the manual not only look at the option definitions `default` attribute for determining the default, but also evaluate the options values with a minimal configuration to get the actual default value. This might be pretty hard to achieve, because oftentimes those defaults are only even assigned if `cfg.enable = true` which won't be the case for a minimal configuration. The real solution might be to specify defaults even when the module is disabled, but this would need a rewrite of almost every module, which is impractical. + +## Command line interfaces +Sometimes programs use command arguments for configuration. While in general there's no trivial way to convert a NixOS value to those, most command line interfaces can be described as having arguments, options and flags, which could be mapped to from Nix values as follows (showing a `nix-build` invocation): + +```nix +{ + arguments = [ "nixos/release.nix" ]; # nixos/release.nix + options.attr = "tests.nginx.x86_64-linux"; # --attr tests.nginx.x86_64-linux + flags.pure-eval = true # --pure-eval + flags.v = 3; # -vvv +} +``` + +By using such an encoding, it would be possible to get all the benefits of a `settings` option. However this encoding isn't entirely obvious, so this should be thought about more. + +# Addendums + +## Unsuitable configuration formats +[unsuitable]: #unsuitable-configuration-formats + +For unsuitable formats it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as [Roundcube's PHP configuration](https://github.com/NixOS/nixpkgs/blob/e03966a60f517700f5fee5182a5a798f8d0709df/nixos/modules/services/mail/roundcube.nix#L86-L93) of the form `$config['key'] = 'value';`), a `config` option as described in this RFC could be used as well as a `configFile` option for more flexibility if needed. + +## Configuration checking One downside of using `settings` instead of having a dedicated NixOS option is that values can't be checked to have the correct key and type at evaluation time. Instead the default mode of operation will be to fail at runtime when the program reads the configuration. There are ways this can be improved however. -#### Configuration checking tools +### Configuration checking tools Occasionally programs have tools for checking their configuration without the need to start the program itself. We can use this to verify the configuration at build time by running the tool during a derivation build. These tools are generally more thorough than the module system and can integrate tightly with the program itself, which can greatly improve user experience. A good side effect of this approach is that less RAM is needed for evaluation. The following illustrates an example of how this might look like: @@ -271,7 +315,7 @@ While not as good as a configuration checker tool, assertions can be used to add TODO: Are there any good examples of using assertions for configuration checks at all? -### Backwards compatibility for configuration settings +## Backwards compatibility for configuration settings By shifting values from a specific NixOS option to the general `settings` one, guarding against upstream changes will have to be done differently. Due to the structural nature of `settings` options, it's possible to deeply inspect and rewrite them however needed before converting them to a string. If the need arises, convenience library functions can be written for such transformations. This might look as follows: @@ -299,43 +343,6 @@ in { } ``` -## Backwards compatibility with existing modules - -This RFC has to be thought of as a basis for *new* modules first and foremost. By using this approach we can provide a good basis for a new module, with great flexibility for future changes. - -A lot of already existing NixOS modules provide a mix of options for single settings and `extraConfig`-style options, which as explained in the [Motivation](#motivation) section leads to problems. In general it is not easy or even impossible to convert such a module to the style described in this RFC in a backwards-compatible way without any workarounds. One workaround is to add an option `useLegacyConfig` or `declarative` which determines the modules behavior in regards to old options. - - -# Drawbacks -[drawbacks]: #drawbacks - -There are some disadvantages to this approach: -- If there is no configuration checking tool as explained in [this section](#configuration-checking-tools), the types of configuration settings can't be checked as easily, which can lead to packages failing at runtime instead of evaluation time. Refer to [Configuration checking](#configuration-checking) for more info. -- Documentation for the configuration settings will not be available in the central NixOS manual, instead the upstream documentation has to be used, which can be unfamiliar and harder to read. As a compromise, [additional NixOS options](#additional-config-options) can be used to bring part of the settings back into the NixOS documentation. - -# Alternatives -[alternatives]: #alternatives - -See [Motivation](#motivation) - -# Unresolved questions -[unresolved]: #unresolved-questions - -Ctrl-F for TODO - -# Future work -[future]: #future-work - -- When defaults for NixOS options are set *outside* the options definition such as `config.services.foo.config.logLevel = "DEBUG"` above, it's currently not possible to see these default values in the manual. This could be improved by having the manual not only look at the option definitions `default` attribute for determining the default, but also evaluate the options value with a minimal configuration to get the actual default value. This might be non-trivial. -- If needed, add config transformation functions for keeping backwards compatibility with upstream changes. See [Backwards compatibility for configuration settings](#backwards-compatibility-for-configuration-settings) - -# Addendums - -## Unsuitable configuration formats -[unsuitable]: #unsuitable-configuration-formats - -For unsuitable formats it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as [Roundcube's PHP configuration](https://github.com/NixOS/nixpkgs/blob/e03966a60f517700f5fee5182a5a798f8d0709df/nixos/modules/services/mail/roundcube.nix#L86-L93) of the form `$config['key'] = 'value';`), a `config` option as described in this RFC could be used as well as a `configFile` option for more flexibility if needed. - ## Previous implementations This idea has been implemented already in some places: @@ -343,3 +350,8 @@ This idea has been implemented already in some places: - [#52096](https://github.com/NixOS/nixpkgs/pull/52096) - [My Murmur module](https://github.com/Infinisil/system/blob/45c3ea36651a2f4328c8a7474148f1c5ecb18e0a/config/new-modules/murmur.nix) - [#55413](https://github.com/NixOS/nixpkgs/pull/55413) + +## Previous discussions + +- https://github.com/NixOS/nixpkgs/pull/44923#issuecomment-412393196 +- https://github.com/NixOS/nixpkgs/pull/55957#issuecomment-464561483 -> https://github.com/NixOS/nixpkgs/pull/57716 From 49a2898039fbc4e9402c654fadb76b6f6f0d1879 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 25 Jul 2019 00:11:03 +0200 Subject: [PATCH 28/43] Remove some temporary stuff --- rfcs/0042-config-option.md | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 083cf03b1..177e9c82c 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -6,21 +6,9 @@ co-authors: (find a buddy later to help our with the RFC) related-issues: (will contain links to implementation PRs) --- -Contents: -- Implementation - - Part 1: Adding formatters, types (mention how types.attrs doesn't work correctly) and docs - - Defaults - - Part 2: Adding docs for encouraging most important options -- Limitations -- Future work - - showing defaults in the manual - - Command line options - # Summary [summary]: #summary -The RFC consists of two parts which flow hand-in-hand - ## Part 1: Structural `settings` instead of stringly `extraConfig` [part1]: #part-1-structural-settings-instead-of-stringly-extraconfig From e1d248cf3f18b73b38900e91393e35ecee5005c3 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 25 Jul 2019 00:18:03 +0200 Subject: [PATCH 29/43] Add table for defaults --- rfcs/0042-config-option.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 177e9c82c..d624b0e7b 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -101,14 +101,13 @@ Whether having a structural `settings` option for a module makes sense depends o #### Default values -Ideally modules should work by just setting `enable = true`. Often this requires the configuration to include some default settings. Defaults should get specified in the `config` section of the module by assigning the values to the `settings` option directly. Depending on how default settings matter, we need to set them differently and for different reasons: -- If the program needs a setting to be present in the configuration file because otherwise it would fail at runtime and demand a value, the module should set this value with a `mkDefault` to the default upstream value, which will then be the equivalent of a starter configuration file. This allows users to easily change the value, but also enables a smooth first use of the module without having to manually set such defaults to get it to a working state. Note that this doesn't necessarily require the module to be updated when upstream defaults change, because that's the expected behavior with starter configurations. -- If the module needs a specific value for a setting because of how the module or NixOS works (e.g. `logger = "systemd"`, because NixOS uses `systemd` for logging), then the value should *not* use `mkDefault`. This way a user can't easily override this setting (which would break the module in some way) and will have to use `mkForce` instead to change it. This also indicates that they are leaving supported territory, and will probably have to change something else to make it work again (e.g. if they set `logger = mkForce "/var/log/foo"` they'll have to change their workflow of where to look for logs). -- If the module itself needs to know the value of a configuration setting at evaluation time in order to influence other options (e.g. opening the firewall for a services port), we may set upstream's default with a `mkDefault`, even though the program might start just fine without it. This allows the module to use the configuration setting directly without having to worry whether it is set at all at evaluation time. +Ideally modules should work by just setting `enable = true`, which often means setting some defaults. They should get specified in the `config` section of the module by assigning the values to the `settings` option directly. Depending on how default settings matter, we need to set them differently and for different reasons: -If the above points don't apply to a configuration setting, that is the module doesn't care about the value, the program doesn't care about the setting being present and we don't need the value at evaluation time, there should be no need to specify any default value. - -TODO: Nice table for default kind / how to set it / whether it needs to track upstream / examples +| Reason | How to assign | Needs to track upstream | Examples | Note | +| --- | --- | --- | --- | --- | +| Program would fail otherwise | `mkDefault` | No | `bootstrap_ip = "172.22.68.74"` | Equivalent to a starter configuration | +| Needed for the module to work, NixOS specifics | **Without** `mkDefault` | No | `logger = "systemd"` `data_dir = "/var/lib/foo"` | Requires the user to use `mkForce` for overriding this, hinting that they leave supported territory | +| Module needs value to influence other options | `mkDefault` | Yes | `port = 456` (influences `allowedTCPPorts`) | | #### Additional options for single settings From 28a3111eeff098fac972ecf255c793c6a1231c58 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 25 Jul 2019 00:58:38 +0200 Subject: [PATCH 30/43] Use table for valuable options --- rfcs/0042-config-option.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index d624b0e7b..bbde19fcd 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -181,12 +181,11 @@ The second part of this RFC aims to encourage people to write better NixOS modul ### Valuable options -TODO: Add table - -- Popular or main settings. Because such `settings` options will have to refer to upstream documentation for all available settings, it's much harder for new module users to figure out how they can configure it. Having popular/main settings as NixOS options is therefore a good idea -- Settings that are necessary for the module to work and are different for every user, so they can't have a default. Examples are `services.dd-agent.api_key`, `services.davmail.url` or `services.hydra.hydraURL`. Having a separate option for these settings can give a much better error message when you don't set them (instead of failing at runtime or having to encode the requirement in an assertion) and better discoverability. -- Password settings: Some program's configuration files require passwords in them directly. Since we try to avoid having passwords in the Nix store, it is advisable to provide a `passwordFile` option as well, which would replace a placeholder password in the configuration file at runtime. - +| Kind | Why | Examples | Notes | +| --- | --- | --- | --- | +| Main/popular settings | These options are what you'll need for basic module usage, they provide a good overview and should be enough for most users | [`services.i2pd.address`](https://nixos.org/nixos/manual/options.html#opt-services.i2pd.address), [`services.taskserver.organisations`](https://nixos.org/nixos/manual/options.html#opt-services.taskserver.organisations) but **not** [~~`services.i2pd.logCLFTime`~~](https://nixos.org/nixos/manual/options.html#opt-services.i2pd.logCLFTime) and **not** [~~`services.taskserver.extensions`~~](https://nixos.org/nixos/manual/options.html#opt-services.taskserver.extensions) | Settings only needed by few can be set through the `settings` option instead | +| Mandatory user-specific values | Reminds the user that they have to set this in order for the program to work, an evaluation error will catch a missing value early | [`services.hydra.hydraURL`](https://nixos.org/nixos/manual/options.html#opt-services.hydra.hydraURL), [`services.davmail.url`](https://nixos.org/nixos/manual/options.html#opt-services.davmail.url) | | +| Sensitive data, passwords | To avoid those ending in the Nix store, ideally an option like `passwordFile` should replace a password placeholder in the configuration file at runtime | | This is specifically about configuration files that have a `password`-like setting | ## Backwards compatibility with existing modules From 4f98924a9abec1a67225813fdd29598e87bfe76a Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 25 Jul 2019 01:26:41 +0200 Subject: [PATCH 31/43] Some adjustments --- rfcs/0042-config-option.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index bbde19fcd..728bc9598 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -175,9 +175,7 @@ In order for the final value of `settings` to be turned into a string, a set of ## [Part 2][part2] -TODO: write what goes in the NixOS docs - -The second part of this RFC aims to encourage people to write better NixOS modules in terms of quality, maintainability and discoverability by limiting NixOS options representing single settings to a set of most "valuable" options. Of course in general "valuable" doesn't have a clear definition, so it's up to the module author and other people familiar with the program to decide on it, but in the following section some conventions will be given. As more such options are deemed "valuable" they can be added to the module over time. +The second part of this RFC aims to encourage people to write better NixOS modules in terms of quality, maintainability and discoverability by limiting NixOS options representing single settings to a set of most "valuable" options. The general idea of valuable options is that they provide more value (used by people, provide safety) than the trouble they're worth (bloated option listings, maintenance cost). Of course this isn't something we can measure, so it's up to the module author to make a reasonable decision, but some general suggestions are given in the next section. As more such options are deemed valuable they can be added to the module over time as well. ### Valuable options @@ -187,13 +185,14 @@ The second part of this RFC aims to encourage people to write better NixOS modul | Mandatory user-specific values | Reminds the user that they have to set this in order for the program to work, an evaluation error will catch a missing value early | [`services.hydra.hydraURL`](https://nixos.org/nixos/manual/options.html#opt-services.hydra.hydraURL), [`services.davmail.url`](https://nixos.org/nixos/manual/options.html#opt-services.davmail.url) | | | Sensitive data, passwords | To avoid those ending in the Nix store, ideally an option like `passwordFile` should replace a password placeholder in the configuration file at runtime | | This is specifically about configuration files that have a `password`-like setting | +This should be described in the NixOS manual. + ## Backwards compatibility with existing modules This RFC has to be thought of as a basis for *new* modules first and foremost. By using this approach we can provide a good basis for a new module, with great flexibility for future changes. A lot of already existing NixOS modules provide a mix of options for single settings and `extraConfig`-style options, which as explained in the [Motivation](#motivation) section leads to problems. In general it is not easy or even impossible to convert such a module to the style described in this RFC in a backwards-compatible way without any workarounds. One workaround is to add an option `useLegacyConfig` or `declarative` which determines the modules behavior in regards to old options. - # Drawbacks [drawbacks]: #drawbacks @@ -204,7 +203,7 @@ There are some disadvantages to this approach: # Alternatives [alternatives]: #alternatives -See [Motivation](#motivation) +The trivial alternative of not doing that, see [Motivation](#motivation) # Unresolved questions [unresolved]: #unresolved-questions From 15a6c5d6bda2b4cec0f2bccecc4f140b33be0113 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 25 Jul 2019 01:29:08 +0200 Subject: [PATCH 32/43] Add link to the example from the summary --- rfcs/0042-config-option.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 728bc9598..6fb5fcca8 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -38,6 +38,8 @@ NixOS modules often use stringly-typed options like `extraConfig` to allow speci } ``` +See [here](#an-example) for an example for how a NixOS module will look like. + ## Part 2: Balancing module option count [part2]: #part-2-balancing-module-option-count From 64aeeda54dfbb1dc54ac040b7ceeea31966ac729 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 25 Jul 2019 01:46:37 +0200 Subject: [PATCH 33/43] Move around and expand part 1 a bit --- rfcs/0042-config-option.md | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 6fb5fcca8..024402b74 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -93,17 +93,29 @@ These are only examples of where people *found* problems and fixed them. The num ## [Part 1][part1] +### Configuration format types + +In order for a structural `settings` to enforce a valid value and work correctly with merging and priorities, it needs to have a type that corresponds to its configuration format, `types.attrs` won't do. As an example, the INI type could be represented with `attrsOf (attrsOf (nullOr (either int str)))`, which means there's multiple named sections, each of which can contain a key-value pair where the value is either `null`, an integer or a string, where `null` signifies a key not being present (which is useful for unsetting existing values). + +Common format types will be provided under `lib.types.settings`. This could include JSON, YAML, INI, a simple `key=value` format and a recursive `key.subkey.subsubkey=value` format for a start. Sometimes programs have their own configuration formats which are specific to them, in which case the type should be specified in that programs module directly instead of going in `lib.types.settings`. + +### Configuration format writers + +In order for the final value of `settings` to be turned into a string, a set of configuration format writers should be provided under `lib.settings`. These should ideally make sure that the resulting text is somewhat properly formatted with readable indentation. Things like `builtins.toJSON` are therefore not optimal as it doesn't add any spacing for readability. These writers will have to include ones for all of the above-mentioned configuration types. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. + ### Additions to the NixOS documentation +The following sections should be added to the NixOS documentation (not verbatim however). + #### Writing options for program configuration -TODO: Write this section, followed by another section on `configFile` +Whether having a structural `settings` option for a module makes sense depends on whether the program's configuration format has an obvious mapping from Nix. This includes formats like JSON, YAML, INI and similar. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", then the answer is probably No, and you should not use this approach. This RFC does not specify anything for unsuitable configuration formats, but there is [an addendum on that][unsuitable]. -Whether having a structural `settings` option for a module makes sense depends on whether the program's configuration format has a direct mapping from Nix. This includes formats like JSON, YAML, INI and similar. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", then the answer is probably No, and you should not use this approach. This RFC does not specify anything for unsuitable configuration formats, but there is [an addendum on that][unsuitable]. +The two main ingredients for writing a `settings` option is to define its type as the one corresponding to the programs configuration format (e.g. `lib.types.settings.json`), and to convert that setting to a string with the corresponding function (e.g. `lib.settings.genJSON`). Very important for writing such options is to link to the upstream documentation. #### Default values -Ideally modules should work by just setting `enable = true`, which often means setting some defaults. They should get specified in the `config` section of the module by assigning the values to the `settings` option directly. Depending on how default settings matter, we need to set them differently and for different reasons: +Ideally modules should work by just setting `enable = true`, which often requires setting some default settings. These should get specified in the `config` section of the module by assigning the values to the `settings` option directly. Depending on how default settings matter, we need to set them differently and for different reasons: | Reason | How to assign | Needs to track upstream | Examples | Note | | --- | --- | --- | --- | --- | @@ -165,16 +177,6 @@ in { } ``` -### Configuration format types - -In order for a structural `settings` to enforce a valid value and work correctly with merging and priorities, it needs to have a type that corresponds to its configuration format, `types.attrs` won't do. As an example, the INI type could be represented with `attrsOf (attrsOf (nullOr (either int str)))`, which means there's multiple named sections, each of which can contain a key-value pair where the value is either `null`, an integer or a string, where `null` signifies a key not being present (which is useful for unsetting existing values). - -Common format types will be provided under `lib.types.settings`. This could include JSON, YAML, INI, a simple `key=value` format and a recursive `key.subkey.subsubkey=value` format for a start. Sometimes programs have their own configuration formats which are specific to them, in which case the type should be specified in that programs module directly instead of going in `lib.types.settings`. - -### Configuration format writers - -In order for the final value of `settings` to be turned into a string, a set of configuration format writers should be provided under `lib.settings`. These should ideally make sure that the resulting text is somewhat properly formatted with readable indentation. Things like `builtins.toJSON` are therefore not optimal as it doesn't add any spacing for readability. These writers will have to include ones for all of the above-mentioned configuration types. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. - ## [Part 2][part2] The second part of this RFC aims to encourage people to write better NixOS modules in terms of quality, maintainability and discoverability by limiting NixOS options representing single settings to a set of most "valuable" options. The general idea of valuable options is that they provide more value (used by people, provide safety) than the trouble they're worth (bloated option listings, maintenance cost). Of course this isn't something we can measure, so it's up to the module author to make a reasonable decision, but some general suggestions are given in the next section. As more such options are deemed valuable they can be added to the module over time as well. @@ -234,6 +236,8 @@ By using such an encoding, it would be possible to get all the benefits of a `se # Addendums +## `configFile` + ## Unsuitable configuration formats [unsuitable]: #unsuitable-configuration-formats From 65842f1868bf5996aef95cba276c34a07bfc9cc7 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 25 Jul 2019 02:00:42 +0200 Subject: [PATCH 34/43] Some more restructuring and tweaks --- rfcs/0042-config-option.md | 40 ++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 024402b74..e9d25822b 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -200,9 +200,9 @@ A lot of already existing NixOS modules provide a mix of options for single sett # Drawbacks [drawbacks]: #drawbacks -There are some disadvantages to this approach: -- If there is no configuration checking tool as explained in [this section](#configuration-checking-tools), the types of configuration settings can't be checked as easily, which can lead to packages failing at runtime instead of evaluation time. Refer to [Configuration checking](#configuration-checking) for more info. -- Documentation for the configuration settings will not be available in the central NixOS manual, instead the upstream documentation has to be used, which can be unfamiliar and harder to read. As a compromise, [additional NixOS options](#additional-config-options) can be used to bring part of the settings back into the NixOS documentation. +For [Part 2][part2]: +- The less encoded options there are, the less checks are happening at evaluation time, and by default this means more runtime failures for initial runs, which isn't as bad as it sounds. If available, [configuration checking tools](#configuration-checking-tools) can be used to have build-time failures instead, or alternatively [assertions][assertions] can be used to have additional evaluation-time checks. +- Only options that are specified will appear in the central NixOS option listings. This means with fewer options there are, the more often upstream documentation is needed. Since the NixOS documentation might be very outdated and incomplete however, this can often be a good thing. # Alternatives [alternatives]: #alternatives @@ -212,8 +212,6 @@ The trivial alternative of not doing that, see [Motivation](#motivation) # Unresolved questions [unresolved]: #unresolved-questions -Ctrl-F for TODO - # Future work [future]: #future-work @@ -236,8 +234,23 @@ By using such an encoding, it would be possible to get all the benefits of a `se # Addendums +## Previous implementations + +This idea has been implemented already in some places: +- [#45470](https://github.com/NixOS/nixpkgs/pull/45470) +- [#52096](https://github.com/NixOS/nixpkgs/pull/52096) +- [My Murmur module](https://github.com/Infinisil/system/blob/45c3ea36651a2f4328c8a7474148f1c5ecb18e0a/config/new-modules/murmur.nix) +- [#55413](https://github.com/NixOS/nixpkgs/pull/55413) + +## Previous discussions + +- https://github.com/NixOS/nixpkgs/pull/44923#issuecomment-412393196 +- https://github.com/NixOS/nixpkgs/pull/55957#issuecomment-464561483 -> https://github.com/NixOS/nixpkgs/pull/57716 + ## `configFile` +TODO + ## Unsuitable configuration formats [unsuitable]: #unsuitable-configuration-formats @@ -245,11 +258,11 @@ For unsuitable formats it is left up to the module author to decide the best set ## Configuration checking -One downside of using `settings` instead of having a dedicated NixOS option is that values can't be checked to have the correct key and type at evaluation time. Instead the default mode of operation will be to fail at runtime when the program reads the configuration. There are ways this can be improved however. +One downside of using `settings` instead of having a dedicated NixOS option is that values can't easily be checked to have the correct key and type at evaluation time. Instead the default mode of operation will be to fail at runtime when the program reads the configuration initially. There are ways this can be improved however. ### Configuration checking tools -Occasionally programs have tools for checking their configuration without the need to start the program itself. We can use this to verify the configuration at build time by running the tool during a derivation build. These tools are generally more thorough than the module system and can integrate tightly with the program itself, which can greatly improve user experience. A good side effect of this approach is that less RAM is needed for evaluation. The following illustrates an example of how this might look like: +Sometimes programs have tools for checking their configuration without the need to start the program itself. We can use this to verify the configuration at build time by running the tool during a derivation build. These tools are generally more thorough than the module system and can integrate tightly with the program itself, which can greatly improve error reporting. A good side effect of this approach is that less RAM is needed for evaluation. The following illustrates an example of how this might look like: TODO: Rewrite in terms of `configFile` ```nix @@ -282,6 +295,7 @@ in { TODO: Explain how `options.services.foo.config.files` can be used to give a better indication of where a failure occurs. ### Ad-hoc checks with assertions +[assertions]: #ad-hoc-checks-with-assertions While not as good as a configuration checker tool, assertions can be used to add flexible ad-hoc checks for type or other properties at evaluation time. It should only be used to ensure important properties that break the service in ways that are otherwise hard or slow to detect (and easy to detect for the module system), not for things that make the service fail to start anyways (unless there's a good reason for it). The following example only demonstrates how assertions can be used for checks, but any reasonable program should bail out early in such cases, which would make these assertions redundant, and only add more coupling to upstream, which we're trying to avoid in the first place. @@ -334,15 +348,3 @@ in { } ``` -## Previous implementations - -This idea has been implemented already in some places: -- [#45470](https://github.com/NixOS/nixpkgs/pull/45470) -- [#52096](https://github.com/NixOS/nixpkgs/pull/52096) -- [My Murmur module](https://github.com/Infinisil/system/blob/45c3ea36651a2f4328c8a7474148f1c5ecb18e0a/config/new-modules/murmur.nix) -- [#55413](https://github.com/NixOS/nixpkgs/pull/55413) - -## Previous discussions - -- https://github.com/NixOS/nixpkgs/pull/44923#issuecomment-412393196 -- https://github.com/NixOS/nixpkgs/pull/55957#issuecomment-464561483 -> https://github.com/NixOS/nixpkgs/pull/57716 From 13d4fe83081decad74afbe5eb8720a76bf3b108b Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 25 Jul 2019 02:03:29 +0200 Subject: [PATCH 35/43] Add cross links --- rfcs/0042-config-option.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index e9d25822b..d40a988a6 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -40,6 +40,8 @@ NixOS modules often use stringly-typed options like `extraConfig` to allow speci See [here](#an-example) for an example for how a NixOS module will look like. +Jump to the [detailed design][part1-design] of part 1 + ## Part 2: Balancing module option count [part2]: #part-2-balancing-module-option-count @@ -47,6 +49,8 @@ Since with this approach there will be no more hardcoded defaults and composabil The second part of this RFC aims to encourage module authors to strike a balance for the number of additional options such as to not make the module too big, but still provide the most commonly used settings as separate options. Quality is encouraged over quantity: Authors should spend more time on writing documentation, NixOS tests or useful high-level abstractions. This is in contrast to the fiddly labor of copying dozens of options from upstream to NixOS. With a `settings` option, it's also very easy to add additional options over time if the need arises. In contrast, removing options has always been nigh impossible. +Jump to the [detailed design][part2-design] of part 2 + # Motivation [motivation]: #motivation @@ -92,6 +96,7 @@ These are only examples of where people *found* problems and fixed them. The num [design]: #detailed-design ## [Part 1][part1] +[part1-design]: #part-1-1 ### Configuration format types @@ -178,6 +183,7 @@ in { ``` ## [Part 2][part2] +[part2-design]: #part-2-1 The second part of this RFC aims to encourage people to write better NixOS modules in terms of quality, maintainability and discoverability by limiting NixOS options representing single settings to a set of most "valuable" options. The general idea of valuable options is that they provide more value (used by people, provide safety) than the trouble they're worth (bloated option listings, maintenance cost). Of course this isn't something we can measure, so it's up to the module author to make a reasonable decision, but some general suggestions are given in the next section. As more such options are deemed valuable they can be added to the module over time as well. From a48b55bbf14252aca692a76360f8d4416bc7a863 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 1 Aug 2019 16:02:35 +0200 Subject: [PATCH 36/43] Add link to types.eithers PR --- rfcs/0042-config-option.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 1ec77f270..b52189e4d 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -5,7 +5,7 @@ author: Silvan Mosberger co-authors: (find a buddy later to help our with the RFC) shepherd-leader: Robin Gloster shepherd-team: Robin Gloster, Eelco Dolstra, Robert Helgesson, Franz Pletz -related-issues: (will contain links to implementation PRs) +related-issues: https://github.com/NixOS/nixpkgs/pull/65728, TBD --- # Summary From 7d6353d888bc0abb9520be38cd1b0988d350053c Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 6 Sep 2019 10:33:38 +0200 Subject: [PATCH 37/43] Mention how extraConfig can break services --- rfcs/0042-config-option.md | 1 + 1 file changed, 1 insertion(+) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index b52189e4d..408ad5573 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -65,6 +65,7 @@ Stringly-typed options such as `extraConfig` have multiple disadvantages in comp - No priorities: `mkDefault` and co. won't work on settings - Values within it can't be inspected, since it's just an opaque string - Syntax of assigned values can easily be wrong, especially with escaping sequences +- Can break services if users assign a value to `extraConfig` which later gets turned into a specialized option, [here](https://github.com/NixOS/nixpkgs/commit/23d1c7f4749#diff-d66632f8013e5976f782de43a0043604R750) is an example of this. ## [Part 2][part2] From 2c63f959d85a7edc7abd782ed06f34ff5a63a713 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Sat, 12 Oct 2019 21:25:47 +0200 Subject: [PATCH 38/43] Add link to lazyAttrsOf PR --- rfcs/0042-config-option.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 408ad5573..9abc0c2e1 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -5,7 +5,7 @@ author: Silvan Mosberger co-authors: (find a buddy later to help our with the RFC) shepherd-leader: Robin Gloster shepherd-team: Robin Gloster, Eelco Dolstra, Robert Helgesson, Franz Pletz -related-issues: https://github.com/NixOS/nixpkgs/pull/65728, TBD +related-issues: https://github.com/NixOS/nixpkgs/pull/65728, https://github.com/NixOS/nixpkgs/pull/70138, TBD --- # Summary From 5ab880faee3fc8f6f073d42b4f06e8f833995621 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 13 Dec 2019 00:39:16 +0100 Subject: [PATCH 39/43] Add format type implementation PR --- rfcs/0042-config-option.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 9abc0c2e1..2102b50dd 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -5,7 +5,7 @@ author: Silvan Mosberger co-authors: (find a buddy later to help our with the RFC) shepherd-leader: Robin Gloster shepherd-team: Robin Gloster, Eelco Dolstra, Robert Helgesson, Franz Pletz -related-issues: https://github.com/NixOS/nixpkgs/pull/65728, https://github.com/NixOS/nixpkgs/pull/70138, TBD +related-issues: https://github.com/NixOS/nixpkgs/pull/65728, https://github.com/NixOS/nixpkgs/pull/70138, https://github.com/NixOS/nixpkgs/pull/75584, TBD --- # Summary From e719102ace17c2b4a8b98efe0f08e344a7713dec Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 16 Jan 2020 23:53:46 +0100 Subject: [PATCH 40/43] Overhaul main parts --- rfcs/0042-config-option.md | 50 ++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 2102b50dd..dbb2931bd 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -103,13 +103,13 @@ These are only examples of where people *found* problems and fixed them. The num ### Configuration format types -In order for a structural `settings` to enforce a valid value and work correctly with merging and priorities, it needs to have a type that corresponds to its configuration format, `types.attrs` won't do. As an example, the INI type could be represented with `attrsOf (attrsOf (nullOr (either int str)))`, which means there's multiple named sections, each of which can contain a key-value pair where the value is either `null`, an integer or a string, where `null` signifies a key not being present (which is useful for unsetting existing values). +In order for a structural `settings` to enforce a valid value and work correctly with merging and priorities, it needs to have a type that corresponds to its configuration format, `types.attrs` won't do. In [PR 75584](https://github.com/NixOS/nixpkgs/pull/75584) fully specified types for JSON, INI, YAML and TOML are implemented in `lib.formats.{json,ini,yaml,toml}.type`. -Common format types will be provided under `lib.types.settings`. This could include JSON, YAML, INI, a simple `key=value` format and a recursive `key.subkey.subsubkey=value` format for a start. Sometimes programs have their own configuration formats which are specific to them, in which case the type should be specified in that programs module directly instead of going in `lib.types.settings`. +Sometimes programs have their own configuration formats which are specific to them, in which case the type should be specified in that programs module directly instead of going in `lib.types.settings`. -### Configuration format writers +### Configuration format generators -In order for the final value of `settings` to be turned into a string, a set of configuration format writers should be provided under `lib.settings`. These should ideally make sure that the resulting text is somewhat properly formatted with readable indentation. Things like `builtins.toJSON` are therefore not optimal as it doesn't add any spacing for readability. These writers will have to include ones for all of the above-mentioned configuration types. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. +In order for the final value of `settings` to be turned into a string, an accompanying set of config format generators is available under `lib.formats.{json,ini,yaml,toml}.generate`. These writers will have to have support all of their accompanying type's values. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. ### Additions to the NixOS documentation @@ -117,9 +117,9 @@ The following sections should be added to the NixOS documentation (not verbatim #### Writing options for program configuration -Whether having a structural `settings` option for a module makes sense depends on whether the program's configuration format has an obvious mapping from Nix. This includes formats like JSON, YAML, INI and similar. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", then the answer is probably No, and you should not use this approach. This RFC does not specify anything for unsuitable configuration formats, but there is [an addendum on that][unsuitable]. +Whether having a structural `settings` option for a module makes sense depends on whether the program's configuration format has an obvious mapping from Nix. This includes formats like JSON, YAML, INI and similar ones. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", then the answer is probably No, and you should not use this approach. This RFC does not specify anything for unsuitable configuration formats, but there is [an addendum on that][unsuitable] regarding this. -The two main ingredients for writing a `settings` option is to define its type as the one corresponding to the programs configuration format (e.g. `lib.types.settings.json`), and to convert that setting to a string with the corresponding function (e.g. `lib.settings.genJSON`). Very important for writing such options is to link to the upstream documentation. +The two main ingredients for writing a `settings` option is to define its type as the one corresponding to the programs configuration format (e.g. `lib.formats.json.type`), and to convert that options value to a string with the corresponding generator function (e.g. `lib.formats.json.generate`). Since these options won't include documentation for all supported values, upstream docs should be linked in the description. #### Default values @@ -133,25 +133,28 @@ Ideally modules should work by just setting `enable = true`, which often require #### Additional options for single settings -One can easily add additional options that correspond to single configuration settings. This is done by defining an option as usual, then applying it to `settings` with a `mkDefault`. This approach allows users to set the value either through the specialized option, or `settings`, which also means that new options can be added without any worry for backwards incompatibility. +One can easily add additional options that correspond to single configuration settings. This is done by defining an option as usual, then applying it to `settings` with a `mkDefault`. This approach allows users to set the value either through the specialized option or `settings`, which also means that new options can be added without having to worry about backwards-compatibility. #### An example Putting it all together, here is an example of a NixOS module that uses such an approach: ```nix -{ config, lib, ... }: -let cfg = config.services.foo; +{ config, lib, pkgs, ... }: +let + cfg = config.services.foo; + format = lib.formats.json; in { options.services.foo = { enable = lib.mkEnableOption "foo service"; settings = lib.mkOption { - type = lib.types.settings.json; + type = format.type; default = {}; description = '' Configuration for foo, see + for supported values. ''; }; @@ -161,24 +164,24 @@ in { description = "Domain this service operates on."; }; }; - + config = lib.mkIf cfg.enable { services.foo.settings = { # Fails at runtime without any value set log_level = lib.mkDefault "WARN"; - + # We use systemd's `StateDirectory`, so we require this (no mkDefault) data_path = "/var/lib/foo"; - + # We use this to open the firewall, so we need to know about the default at eval time port = lib.mkDefault 2546; - + # Apply our specialized setting. domain = lib.mkDefault cfg.domain; }; - - environment.etc."foo/config.json".text = lib.settings.genJSON cfg.settings; - + + environment.etc."foo-config".source = format.generate pkgs "foo-config.json" cfg.settings; + networking.firewall.allowedTCPPorts = [ cfg.settings.port ]; # ... }; @@ -228,18 +231,7 @@ The trivial alternative of not doing that, see [Motivation](#motivation) When defaults for NixOS options are set *outside* the options definition such as `config.services.foo.settings.log_level = lib.mkDefault "WARN"` above, it's currently not possible to see these default values in the manual. This could be improved by having the manual not only look at the option definitions `default` attribute for determining the default, but also evaluate the options values with a minimal configuration to get the actual default value. This might be pretty hard to achieve, because oftentimes those defaults are only even assigned if `cfg.enable = true` which won't be the case for a minimal configuration. The real solution might be to specify defaults even when the module is disabled, but this would need a rewrite of almost every module, which is impractical. ## Command line interfaces -Sometimes programs use command arguments for configuration. While in general there's no trivial way to convert a NixOS value to those, most command line interfaces can be described as having arguments, options and flags, which could be mapped to from Nix values as follows (showing a `nix-build` invocation): - -```nix -{ - arguments = [ "nixos/release.nix" ]; # nixos/release.nix - options.attr = "tests.nginx.x86_64-linux"; # --attr tests.nginx.x86_64-linux - flags.pure-eval = true # --pure-eval - flags.v = 3; # -vvv -} -``` - -By using such an encoding, it would be possible to get all the benefits of a `settings` option. However this encoding isn't entirely obvious, so this should be thought about more. +Sometimes programs use command arguments for configuration. Since [PR 75539](https://github.com/NixOS/nixpkgs/pull/75539) there is `lib.encodeGNUCommandLine` to convert a Nix value to an argument string. With compatible programs this brings all the above-mentioned benefits to those programs as well. # Addendums From 336130938df8608f12bd5287ef7ea5aeabf4064a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Thu, 30 Jul 2020 14:19:28 +0100 Subject: [PATCH 41/43] Update rfcs/0042-config-option.md --- rfcs/0042-config-option.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index dbb2931bd..6025bc769 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -3,8 +3,8 @@ feature: config-option start-date: 2019-03-10 author: Silvan Mosberger co-authors: (find a buddy later to help our with the RFC) -shepherd-leader: Robin Gloster -shepherd-team: Robin Gloster, Eelco Dolstra, Robert Helgesson, Franz Pletz +shepherd-leader: Jörg Thalheim +shepherd-team: Jörg Thalheim, Eelco Dolstra, Robert Helgesson, Franz Pletz related-issues: https://github.com/NixOS/nixpkgs/pull/65728, https://github.com/NixOS/nixpkgs/pull/70138, https://github.com/NixOS/nixpkgs/pull/75584, TBD --- @@ -348,4 +348,3 @@ in { config.environment.etc."foo/config.json".text = lib.settings.genJSON fixedUpsettings; } ``` - From 9ef0221a086f7dbe8662631407c583eb23daa626 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 18 Dec 2020 01:36:44 +0100 Subject: [PATCH 42/43] Clean up --- rfcs/0042-config-option.md | 236 ++++++++++--------------------------- 1 file changed, 64 insertions(+), 172 deletions(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 6025bc769..0461ba0cb 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -2,7 +2,7 @@ feature: config-option start-date: 2019-03-10 author: Silvan Mosberger -co-authors: (find a buddy later to help our with the RFC) +co-authors: shepherd-leader: Jörg Thalheim shepherd-team: Jörg Thalheim, Eelco Dolstra, Robert Helgesson, Franz Pletz related-issues: https://github.com/NixOS/nixpkgs/pull/65728, https://github.com/NixOS/nixpkgs/pull/70138, https://github.com/NixOS/nixpkgs/pull/75584, TBD @@ -14,7 +14,7 @@ related-issues: https://github.com/NixOS/nixpkgs/pull/65728, https://github.com/ ## Part 1: Structural `settings` instead of stringly `extraConfig` [part1]: #part-1-structural-settings-instead-of-stringly-extraconfig -NixOS modules often use stringly-typed options like `extraConfig` to allow specifying extra settings in addition to the default ones. This has multiple disadvantages: The defaults can't be changed, multiple values might not get merged properly, inspection of it is almost impossible because it's an opaque string and more. The first part of this RFC aims to discourage such options and proposes generic `settings` options instead, which can encode the modules configuration file as a structural Nix value. Here is an example showcasing some advantages: +NixOS modules often use stringly-typed options like `extraConfig` to allow specifying extra settings in addition to the default ones. This has multiple disadvantages: The defaults can't be changed, multiple values might not get merged properly, inspection of it is almost impossible because it's an opaque string and more. The first part of this RFC aims to discourage such options and encourage people to use the `settings` pattern instead, which can encode the modules configuration file as a structural Nix value. Here is an example showcasing some advantages: ```nix { @@ -40,9 +40,7 @@ NixOS modules often use stringly-typed options like `extraConfig` to allow speci } ``` -See [here](#an-example) for an example for how a NixOS module will look like. - -Jump to the [detailed design][part1-design] of part 1 +Jump to the [detailed design][part1-design] of part 1, which also shows how a module implementing the `settings` approach might look like. ## Part 2: Balancing module option count [part2]: #part-2-balancing-module-option-count @@ -101,93 +99,82 @@ These are only examples of where people *found* problems and fixed them. The num ## [Part 1][part1] [part1-design]: #part-1-1 -### Configuration format types - -In order for a structural `settings` to enforce a valid value and work correctly with merging and priorities, it needs to have a type that corresponds to its configuration format, `types.attrs` won't do. In [PR 75584](https://github.com/NixOS/nixpkgs/pull/75584) fully specified types for JSON, INI, YAML and TOML are implemented in `lib.formats.{json,ini,yaml,toml}.type`. - -Sometimes programs have their own configuration formats which are specific to them, in which case the type should be specified in that programs module directly instead of going in `lib.types.settings`. - -### Configuration format generators - -In order for the final value of `settings` to be turned into a string, an accompanying set of config format generators is available under `lib.formats.{json,ini,yaml,toml}.generate`. These writers will have to have support all of their accompanying type's values. As with the type, if the program has its own configuration format, the writer should be implemented in its module directly. - -### Additions to the NixOS documentation - -The following sections should be added to the NixOS documentation (not verbatim however). - -#### Writing options for program configuration - -Whether having a structural `settings` option for a module makes sense depends on whether the program's configuration format has an obvious mapping from Nix. This includes formats like JSON, YAML, INI and similar ones. Examples of unsuitable configuration formats are Haskell, Lisp, Lua or other generic programming languages. If you need to ask yourself "Does it make sense to use Nix for this configuration format", then the answer is probably No, and you should not use this approach. This RFC does not specify anything for unsuitable configuration formats, but there is [an addendum on that][unsuitable] regarding this. - -The two main ingredients for writing a `settings` option is to define its type as the one corresponding to the programs configuration format (e.g. `lib.formats.json.type`), and to convert that options value to a string with the corresponding generator function (e.g. `lib.formats.json.generate`). Since these options won't include documentation for all supported values, upstream docs should be linked in the description. +It's already possible to write generic `settings` options today, using PR's introducing [`pkgs.formats`](https://github.com/NixOS/nixpkgs/pull/75584) and [freeform modules](https://github.com/NixOS/nixpkgs/pull/82743) when needed. Documentation for these features and how to use them for declaring `settings` options already exists in the manual: [Options for Program Settings](https://nixos.org/manual/nixos/stable/index.html#sec-settings-options) and [Freeform Modules](https://nixos.org/manual/nixos/stable/index.html#sec-freeform-modules). -#### Default values - -Ideally modules should work by just setting `enable = true`, which often requires setting some default settings. These should get specified in the `config` section of the module by assigning the values to the `settings` option directly. Depending on how default settings matter, we need to set them differently and for different reasons: - -| Reason | How to assign | Needs to track upstream | Examples | Note | -| --- | --- | --- | --- | --- | -| Program would fail otherwise | `mkDefault` | No | `bootstrap_ip = "172.22.68.74"` | Equivalent to a starter configuration | -| Needed for the module to work, NixOS specifics | **Without** `mkDefault` | No | `logger = "systemd"` `data_dir = "/var/lib/foo"` | Requires the user to use `mkForce` for overriding this, hinting that they leave supported territory | -| Module needs value to influence other options | `mkDefault` | Yes | `port = 456` (influences `allowedTCPPorts`) | | - -#### Additional options for single settings - -One can easily add additional options that correspond to single configuration settings. This is done by defining an option as usual, then applying it to `settings` with a `mkDefault`. This approach allows users to set the value either through the specialized option or `settings`, which also means that new options can be added without having to worry about backwards-compatibility. - -#### An example - -Putting it all together, here is an example of a NixOS module that uses such an approach: +Using these features, a module supporting `settings` might look like ```nix -{ config, lib, pkgs, ... }: +{ options, config, lib, pkgs, ... }: let cfg = config.services.foo; - format = lib.formats.json; + # Define the settings format used for this program + settingsFormat = pkgs.formats.json {}; in { options.services.foo = { enable = lib.mkEnableOption "foo service"; settings = lib.mkOption { - type = format.type; + type = lib.types.submodule { + + # Declare that the settings option supports arbitrary format values, json here + freeformType = settingsFormat.type; + + # Declare an option for the port such that the type is checked and this option + # is shown in the manual. + options.port = lib.mkOption { + type = lib.types.port; + default = 8080; + description = '' + Which port this service should listen on. + ''; + }; + + }; default = {}; + # Add upstream documentation to the settings description description = '' - Configuration for foo, see + Configuration for Foo, see + for supported values. ''; }; - - # An additional option for a setting so we have an eval error if this is missing - domain = lib.mkOption { - type = lib.types.str; - description = "Domain this service operates on."; - }; }; config = lib.mkIf cfg.enable { + # We can assign some default settings here to make the service work by just + # enabling it. We use `mkDefault` for values that can be changed without + # problems services.foo.settings = { # Fails at runtime without any value set log_level = lib.mkDefault "WARN"; - # We use systemd's `StateDirectory`, so we require this (no mkDefault) + # We assume systemd's `StateDirectory` is used, so this value is required + # therefore no mkDefault, forcing the user to use mkForce to override it data_path = "/var/lib/foo"; - # We use this to open the firewall, so we need to know about the default at eval time - port = lib.mkDefault 2546; - - # Apply our specialized setting. - domain = lib.mkDefault cfg.domain; + # Since we use this to create a user we need to know the default value at + # eval time + user = lib.mkDefault "foo"; }; - environment.etc."foo-config".source = format.generate pkgs "foo-config.json" cfg.settings; + environment.etc."foo.json".source = + # The formats generator function takes a filename and the Nix value + # representing the format value and produces a filepath with that value + # rendered in the format + settingsFormat.generate "foo-config.json" cfg.settings; + + # We know that the `user` attribute exists because we set a default value + # for it above, allowing us to use it without worries here + users.users.${cfg.settings.user} = {}; - networking.firewall.allowedTCPPorts = [ cfg.settings.port ]; # ... }; } ``` +This RFC proposes to agree upon making this the standard way to specify configuration when this approach is feasible. Notably infeasible for this approach are configuration file formats that can't be directly mapped to Nix, such as bash, python, and others. + ## [Part 2][part2] [part2-design]: #part-2-1 @@ -201,19 +188,17 @@ The second part of this RFC aims to encourage people to write better NixOS modul | Mandatory user-specific values | Reminds the user that they have to set this in order for the program to work, an evaluation error will catch a missing value early | [`services.hydra.hydraURL`](https://nixos.org/nixos/manual/options.html#opt-services.hydra.hydraURL), [`services.davmail.url`](https://nixos.org/nixos/manual/options.html#opt-services.davmail.url) | | | Sensitive data, passwords | To avoid those ending in the Nix store, ideally an option like `passwordFile` should replace a password placeholder in the configuration file at runtime | | This is specifically about configuration files that have a `password`-like setting | -This should be described in the NixOS manual. - ## Backwards compatibility with existing modules -This RFC has to be thought of as a basis for *new* modules first and foremost. By using this approach we can provide a good basis for a new module, with great flexibility for future changes. +This RFC has to be thought of as a basis for *new* modules first and foremost. By using this approach we can provide a good basis for new modules, with great flexibility for future changes. -A lot of already existing NixOS modules provide a mix of options for single settings and `extraConfig`-style options, which as explained in the [Motivation](#motivation) section leads to problems. In general it is not easy or even impossible to convert such a module to the style described in this RFC in a backwards-compatible way without any workarounds. One workaround is to add an option `useLegacyConfig` or `declarative` which determines the modules behavior in regards to old options. +For existing modules, it is often not possible to use this `settings` style without breaking backwards compatibility. How this is handled is left up to the module authors. A workaround that could be employed is to define options `useLegacyConfig` or `declarative` which determin the modules behavior in regards to old options. # Drawbacks [drawbacks]: #drawbacks For [Part 2][part2]: -- The less encoded options there are, the less checks are happening at evaluation time, and by default this means more runtime failures for initial runs, which isn't as bad as it sounds. If available, [configuration checking tools](#configuration-checking-tools) can be used to have build-time failures instead, or alternatively [assertions][assertions] can be used to have additional evaluation-time checks. +- The less encoded options there are, the less checks are happening at evaluation time, and by default this means more runtime failures for initial runs, which isn't as bad as it sounds. If available, configuration checking tools can be used to have build-time failures instead, or alternatively assertions can be used to have additional evaluation-time checks. - Only options that are specified will appear in the central NixOS option listings. This means with fewer options there are, the more often upstream documentation is needed. Since the NixOS documentation might be very outdated and incomplete however, this can often be a good thing. # Alternatives @@ -228,123 +213,30 @@ The trivial alternative of not doing that, see [Motivation](#motivation) [future]: #future-work ## Documentation defaults -When defaults for NixOS options are set *outside* the options definition such as `config.services.foo.settings.log_level = lib.mkDefault "WARN"` above, it's currently not possible to see these default values in the manual. This could be improved by having the manual not only look at the option definitions `default` attribute for determining the default, but also evaluate the options values with a minimal configuration to get the actual default value. This might be pretty hard to achieve, because oftentimes those defaults are only even assigned if `cfg.enable = true` which won't be the case for a minimal configuration. The real solution might be to specify defaults even when the module is disabled, but this would need a rewrite of almost every module, which is impractical. +When defaults for NixOS options are set *outside* the options definition such as `config.services.foo.settings.log_level = lib.mkDefault "WARN"` above, these values don't show up in the documentation as the options defaults. Using the options `default` value would show up in the manual, but unfortunately that doesn't do merging correctly with `settings` options since the value is set with `mkOptionDefault`. To fix this, an option attribute like `recursiveDefault` could be implemented, which recursively sets `mkOptionDefault` on its value instead. + +# Addendums ## Command line interfaces Sometimes programs use command arguments for configuration. Since [PR 75539](https://github.com/NixOS/nixpkgs/pull/75539) there is `lib.encodeGNUCommandLine` to convert a Nix value to an argument string. With compatible programs this brings all the above-mentioned benefits to those programs as well. -# Addendums - -## Previous implementations +## Modules that use the `settings` style This idea has been implemented already in some places: -- [#45470](https://github.com/NixOS/nixpkgs/pull/45470) -- [#52096](https://github.com/NixOS/nixpkgs/pull/52096) +- [nixos/znc](https://github.com/NixOS/nixpkgs/tree/master/nixos/modules/services/networking/znc) +- [nixos/davmail](https://github.com/nixos/nixpkgs/blob/master/nixos/modules/services/mail/davmail.nix) - [My Murmur module](https://github.com/Infinisil/system/blob/45c3ea36651a2f4328c8a7474148f1c5ecb18e0a/config/new-modules/murmur.nix) -- [#55413](https://github.com/NixOS/nixpkgs/pull/55413) +- [nixos/bitwarden\_rs](https://github.com/nixos/nixpkgs/blob/master/nixos/modules/services/security/bitwarden_rs/default.nix) +- [nixos/hercules-ci-agent](https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/continuous-integration/hercules-ci-agent/common.nix) +- [nixos/blackfire](https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/development/blackfire.nix) +- [nixos/biboumi](https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/networking/biboumi.nix) +- [nixos/mackerel-agent](https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/monitoring/mackerel-agent.nix) +- [nixos/epgstation](https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/video/epgstation/default.nix) +- [nixos/klipper](https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/misc/klipper.nix) +- [nixos/redmine](https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/misc/redmine.nix) +- [nixos/nfs](https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/tasks/filesystems/nfs.nix) ## Previous discussions - https://github.com/NixOS/nixpkgs/pull/44923#issuecomment-412393196 - https://github.com/NixOS/nixpkgs/pull/55957#issuecomment-464561483 -> https://github.com/NixOS/nixpkgs/pull/57716 - -## `configFile` - -TODO - -## Unsuitable configuration formats -[unsuitable]: #unsuitable-configuration-formats - -For unsuitable formats it is left up to the module author to decide the best set of NixOS options. Sometimes it might make sense to have both a specialized set of options for single settings (e.g. `programs.bash.environment`) and a flexible option of type `types.lines` (such as `programs.bash.promptInit`). Alternatively it might be reasonable to only provide a `config`/`configFile` option of type `types.str`/`types.path`, such as for XMonad's Haskell configuration file. And for programs that use a general purpose language even though their configuration can be represented in key-value style (such as [Roundcube's PHP configuration](https://github.com/NixOS/nixpkgs/blob/e03966a60f517700f5fee5182a5a798f8d0709df/nixos/modules/services/mail/roundcube.nix#L86-L93) of the form `$config['key'] = 'value';`), a `config` option as described in this RFC could be used as well as a `configFile` option for more flexibility if needed. - -## Configuration checking - -One downside of using `settings` instead of having a dedicated NixOS option is that values can't easily be checked to have the correct key and type at evaluation time. Instead the default mode of operation will be to fail at runtime when the program reads the configuration initially. There are ways this can be improved however. - -### Configuration checking tools - -Sometimes programs have tools for checking their configuration without the need to start the program itself. We can use this to verify the configuration at build time by running the tool during a derivation build. These tools are generally more thorough than the module system and can integrate tightly with the program itself, which can greatly improve error reporting. A good side effect of this approach is that less RAM is needed for evaluation. The following illustrates an example of how this might look like: - -TODO: Rewrite in terms of `configFile` -```nix -{ config, lib, pkgs, ... }: -let - cfg = config.services.foo; - - checkedConfig = pkgs.runCommandNoCC "foo-config.json" { - # Because this program will be run at build time, we need `nativeBuildInputs` - nativeBuildInputs = [ pkgs.foo-checker ]; - - config = lib.settings.genJSON cfg.settings; - passAsFile = [ "config" ]; - } '' - foo-checker "$configPath" - cp "$configPath" "$out" - ''; - -in { - options = { /* ... */ }; - config = lib.mkIf cfg.enable { - - environment.etc."foo/config.json".source = checkedConfig; - - # ... - }; -} -``` - -TODO: Explain how `options.services.foo.config.files` can be used to give a better indication of where a failure occurs. - -### Ad-hoc checks with assertions -[assertions]: #ad-hoc-checks-with-assertions - -While not as good as a configuration checker tool, assertions can be used to add flexible ad-hoc checks for type or other properties at evaluation time. It should only be used to ensure important properties that break the service in ways that are otherwise hard or slow to detect (and easy to detect for the module system), not for things that make the service fail to start anyways (unless there's a good reason for it). The following example only demonstrates how assertions can be used for checks, but any reasonable program should bail out early in such cases, which would make these assertions redundant, and only add more coupling to upstream, which we're trying to avoid in the first place. - -```nix -{ config, lib, ... }: { - # ... - config = lib.mkIf cfg.enable { - # Examples only for demonstration purposes, don't actually add assertions for such properties - assertions = [ - { - assertion = cfg.settings.enableLogging or true -> cfg.settings ? logLevel; - message = "You also need to set `services.foo.settings.logLevel` if `services.foo.settings.enableLogging` is turned on."; - } - { - assertion = cfg.settings ? port -> lib.types.port.check cfg.settings.port; - message = "${toString cfg.settings.port} is not a valid port number for `services.foo.settings.port`."; - } - ]; - }; -} -``` - -TODO: Are there any good examples of using assertions for configuration checks at all? - -## Backwards compatibility for configuration settings - -By shifting values from a specific NixOS option to the general `settings` one, guarding against upstream changes will have to be done differently. Due to the structural nature of `settings` options, it's possible to deeply inspect and rewrite them however needed before converting them to a string. If the need arises, convenience library functions can be written for such transformations. This might look as follows: - -```nix -{ config, lib, ... }: -let - cfg = config.services.foo; - - fixedUpSettings = - let - renamedKeys = builtins.intersectAttrs cfg.settings { - # foo has been renamed to bar - foo = "bar"; - }; - conflicts = lib.filter (from: cfg.settings ? ${renamedKeys.${from}}) (lib.attrNames renamedKeys); - in if conflicts == [] then lib.mapAttrs' (from: to: - lib.nameValuePair to cfg.settings.${from} - ) renamedKeys // builtins.removeAttrs cfg.settings (lib.attrNames renamedKeys) - else throw (lib.concatMapStringsSep "," (from: - "Can't mix the deprecated setting \"${from}\" with its replacement \"${renamedKeys.${from}}\"" - ) conflicts); - -in { - config.environment.etc."foo/config.json".text = lib.settings.genJSON fixedUpsettings; -} -``` From d46bafd33cb6c9e4e22b224978b9ed386ab47bb1 Mon Sep 17 00:00:00 2001 From: Linus Heckemann Date: Thu, 7 Jan 2021 15:17:15 +0100 Subject: [PATCH 43/43] Remove fpletz from shepherd team fpletz has not been available for meetings, nor to approve FCP. Given that the other three shepherds are in agreement that this should move to FCP, the RFC steering committee has decided to remove fpletz from the shepherd team in order to make progress without implying that he has approved it. --- rfcs/0042-config-option.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfcs/0042-config-option.md b/rfcs/0042-config-option.md index 0461ba0cb..ecefdcd68 100644 --- a/rfcs/0042-config-option.md +++ b/rfcs/0042-config-option.md @@ -4,7 +4,7 @@ start-date: 2019-03-10 author: Silvan Mosberger co-authors: shepherd-leader: Jörg Thalheim -shepherd-team: Jörg Thalheim, Eelco Dolstra, Robert Helgesson, Franz Pletz +shepherd-team: Jörg Thalheim, Eelco Dolstra, Robert Helgesson related-issues: https://github.com/NixOS/nixpkgs/pull/65728, https://github.com/NixOS/nixpkgs/pull/70138, https://github.com/NixOS/nixpkgs/pull/75584, TBD ---