Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NixOS modules: Add mkSuperOptionAlias and mkForEachSubModule. #152785

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nbp
Copy link
Member

@nbp nbp commented Dec 30, 2021

Previously, getting values from submodules to modify the super-module options,
required more complex filtering logic which is dependent on the option type
wrapping the submodule, as well as making sure that the types are coherent.

This change makes it possible to easily set super-module options from submodule
aliases. This change adds 2 major functions: mkSuperOptionAlias and
mkForEachSubModule.

mkSuperOptionAlias is made to create an option within a submodule, which
inherit the type of a parent module option, if it exists as well as preventing
any uses of its computed output. Previously, an easy mistake when implementing
such aliases was to use the computed value of the option. Using the computed
value is a problem as the type, which provide a merge and apply function is
not garanteed to be idempotent. The work-around is to expose the options with
the submoduleWith argument named exportOptionsUnderConfig. This flag, set to
false to avoid regressions, when enabled, it will add the options attribute
under the configuration produced by each submodule instance. This is mandatorry
to extract option definitions using mkAliasDefinitions.

mkForEachSubModule is made to abstract over the wrapping type of submodules,
and forward definitions with mkAliasDefinitions or similar functions, to skip
over complexity induced by the merge and apply function logic. This works by
adding a new attribute named extractSubValues to each option type. This
attribute is a function which is responsible from unpacking any wrapping types
which are wrapping submodules, such as list or attribute sets, and return an
unordered list of submodule instances. Each submodule instance is then iterated
over and given as argument to the first argument, which is in charge of
producing a valid module or option definition. This is best achieved with
mkAliasDefinitions followed with the access to the option exported by the
submodule, using exportOptionsUnderConfig flag of the submoduleWith type.

The following example illustrates how these functions can be used to aggregate
all definitions defined within multiple submodules, into the super-module which
collect all these definitions.

{ config, options, lib, ... }:

let superOptions = options; in
{
  # Suppose that the following option exists:
  #   options.aggregate = lib.mkOption { .. };

  # `namedAggregate` is a tiny submodule which exposes `super.aggregate`, and let
  # the use set parts of the aggregated content under a specific name, such as:
  #
  #   namedAggregate.setupEtc.super.aggregate = " .. ";
  #   namedAggregate.setupBin.super.aggregate = " .. ";
  #
  # All of which would be used as a definitions of the `aggregate` option.
  options.namedAggregates = mkOption {
    type = with lib.types; attrsOf submoduleWith {
      # Expose the eval.options attribute used as argument of `mkAliasDefinitions`
      exportOptionsUnderConfig = true;

      modules = [
        # Create a sub-module which aliases the super-module option, if it exists.
        #
        # To explicit the fact that this option is used only by the super-module,
        # it is added under `super.` attribute set, which might be a convention.
        #
        # Using (.. or null) as argument is useful for submodules which can be
        # embedded in various super-modules. As, this will also work if the
        # option does not exists.
        ({ lib, ... }: {
          options.super.aggregate =
            lib.mkSuperOptionAlias (superOptions.aggregate or null);
        })
      ];
    };
  };

  config = {
    # Collect all options definitions from submodules, if they are defined.
    # `mkForEachSubModule` iterate over all submodule instances. Each instance
    # `eval` has an attribute named `options` exposed by `exportOptionsUnderConfig`
    # flag. This `options` attribute is the same as the `options` argument of
    # submodules and contains the hierachy of options declarations and definitions.
    # `mkAliasDefinitions` is then used to extract options definitions if the
    # option is set in the submodule.
    aggregate = lib.mkForEachSubModule
      (eval: lib.mkAliasDefinitions eval.options.super.aggregate)
      options.namedAggregates;
  };
}
Things done

Tested locally using:

  cd <local-nixpkgs>/lib/tests
  NIX_PATH=<local-nixpkgs> ./modules.sh 

All changes made here are new functions, or should be backward compatible.

Previously, getting values from submodules to modify the super-module options,
required more complex filtering logic which is dependent on the option type
wrapping the submodule, as well as making sure that the types are coherent.

This change makes it possible to easily set super-module options from submodule
aliases. This change adds 2 major functions: `mkSuperOptionAlias` and
`mkForEachSubModule`.

`mkSuperOptionAlias` is made to create an option within a submodule, which
inherit the type of a parent module option, if it exists as well as preventing
any uses of its computed output. Previously, an easy mistake when implementing
such aliases was to use the computed value of the option. Using the computed
value is a problem as the type, which provide a `merge` and `apply` function is
not garanteed to be idempotent. The work-around is to expose the `options` with
the `submoduleWith` argument named `exportOptionsUnderConfig`. This flag, set to
`false` to avoid regressions, when enabled, it will add the `options` attribute
under the configuration produced by each submodule instance. This is mandatorry
to extract option `definitions` using `mkAliasDefinitions`.

`mkForEachSubModule` is made to abstract over the wrapping type of submodules,
and forward definitions with `mkAliasDefinitions` or similar functions, to skip
over complexity induced by the `merge` and `apply` function logic. This works by
adding a new attribute named `extractSubValues` to each option type. This
attribute is a function which is responsible from unpacking any wrapping types
which are wrapping submodules, such as list or attribute sets, and return an
unordered list of submodule instances. Each submodule instance is then iterated
over and given as argument to the first argument, which is in charge of
producing a valid module or option definition. This is best achieved with
`mkAliasDefinitions` followed with the access to the option exported by the
submodule, using `exportOptionsUnderConfig` flag of the `submoduleWith` type.

The following example illustrates how these functions can be used to aggregate
all definitions defined within multiple submodules, into the super-module which
collect all these definitions.

```
{ config, options, lib, ... }:

let superOptions = options; in
{
  # Suppose that the following option exists:
  #   options.aggregate = lib.mkOption { .. };

  # `namedAggregate` is a tiny submodule which exposes `super.aggregate`, and let
  # the use set parts of the aggregated content under a specific name, such as:
  #
  #   namedAggregate.setupEtc.super.aggregate = " .. ";
  #   namedAggregate.setupBin.super.aggregate = " .. ";
  #
  # All of which would be used as a definitions of the `aggregate` option.
  options.namedAggregates = mkOption {
    type = with lib.types; attrsOf submoduleWith {
      # Expose the eval.options attribute used as argument of `mkAliasDefinitions`
      exportOptionsUnderConfig = true;

      modules = [
        # Create a sub-module which aliases the super-module option, if it exists.
        #
        # To explicit the fact that this option is used only by the super-module,
        # it is added under `super.` attribute set, which might be a convention.
        #
        # Using (.. or null) as argument is useful for submodules which can be
        # embedded in various super-modules. As, this will also work if the
        # option does not exists.
        ({ lib, ... }: {
          options.super.aggregate =
            lib.mkSuperOptionAlias (superOptions.aggregate or null);
        })
      ];
    };
  };

  config = {
    # Collect all options definitions from submodules, if they are defined.
    # `mkForEachSubModule` iterate over all submodule instances. Each instance
    # `eval` has an attribute named `options` exposed by `exportOptionsUnderConfig`
    # flag. This `options` attribute is the same as the `options` argument of
    # submodules and contains the hierachy of options declarations and definitions.
    # `mkAliasDefinitions` is then used to extract options definitions if the
    # option is set in the submodule.
    aggregate = lib.mkForEachSubModule
      (eval: lib.mkAliasDefinitions eval.options.super.aggregate)
      options.namedAggregates;
  };
}
```
@roberth
Copy link
Member

roberth commented Jan 1, 2022

I'm not sure about exportOptionsUnderConfig. It doesn't seem right, to return options there.

What does a module return?

a. Since RFC42, we've cleaned up the return "config" so that it is (usually) ready to be used for config file generation. exportOptionsUnderConfig seems like a step in the other direction.

b. In existing submodules that have logic in them, the returned value is (conceptually) limited to one or two option values.

c. With this PR, we'll be "returning" definitions to merge elsewhere; not really a well-defined value, but more like a side effect, so to speak.

In one design, we could give submodules complete control over their return value. This allows a submodule to be more explicit in case of (b), although the resulting encapsulation makes troubleshooting in a repl a bit harder. The same facility could implement (c), but I don't like that the module author has to deal with a new "definition set" type that can only be used in one way anyway.

In another design, we could make "supermodules" an integrated feature that seems to fully capture the use case. From a module author perspective, it could look like:

acme.nix

{
  options.security.acme.certs = mkOption {
    type = attrsOf (submodule ({ config, name, ... }: {
      options.webroot = .....;
      super = { # supermodules section. Fixed field like `options`, attrset of configs
        system = { # config to apply at the system level
          services.systemd.services."acme-${name}" = {
            # ...
          };
        };
      };
    }));
  };
}

nixos.nix/nix-darwin.nix

{
  applySupermodules = "system"; # applies all `super.system` supermodules.
  options = .....;
  config = .....;
}

This is achievable with the following changes:

The latter is important for laziness. Without it, the module system will recurse infinitely, as it tries to determine which submodules exist. To determine the set of all system-level defs, you'd have to evaluate all submodules, but in order to know which submodules exist, you'd need to know all system-level defs. This recursion needs to be restricted. One way to achieve this is by requiring the module author to specify which options can be set via supermodules, as you've shown in your example (config.aggregate = /*...*/). This gets the job done, but it would be nicer if the module author didn't have to specify this. We could take this information from the imports graph, paired with some logic that can also be used for imports hermeticity checking, which was identified to be useful for NixOS/rfcs#78.

So this PR seems to provide an intermediate solution for part of the problem that will not be part of the final solution, unless I'm wrong about the feasibility of the design I've written about here.

@nbp
Copy link
Member Author

nbp commented Jan 2, 2022

I'm not sure about exportOptionsUnderConfig. It doesn't seem right, to return options there.

I agree, the problem is that I wanted to avoid have 2 evaluations of the submodules, one for getting the config, and one for getting the options set.

The exportOptionsUnderConfig flag is a hack in the sense that it does not fit nicely.
Maybe we should find a way to extract the options set of each submodule instance as part of the value computation, and make it bubble up the value computation of any wrapping type, such that we can expose the list of options set instances on the options of the super module. While removing it from the config value, exposed as the returned value of the submodule.

Thus, not polluting the configuration, not requiring the exportOptionsUnderConfig flag, while keeping the same usage.

I think this is doable. This will require some extra logic around the merge functions.
I will try to do it ...

[...] but I don't like that the module author has to deal with a new "definition set" type that can only be used in one way anyway.

What do you mean? The new mkForEachSubModule is made to explicitly handle values which are set in a submodule, as well as avoiding the lack of idempotent guarantee on the merge / apply functions.

In another design, we could make "supermodules" an integrated feature that seems to fully capture the use case. From a module author perspective, it could look like:

acme.nix

{
  options.security.acme.certs = mkOption {
    type = attrsOf (submodule ({ config, name, ... }: {
      options.webroot = .....;
      super = { # supermodules section. Fixed field like `options`, attrset of configs
        system = { # config to apply at the system level
          services.systemd.services."acme-${name}" = {
            # ...
          };
        };
      };
    }));
  };
}

nixos.nix/nix-darwin.nix

{
  applySupermodules = "system"; # applies all `super.system` supermodules.
  options = .....;
  config = .....;
}

(From what I understand) doing so implies that resolving the set of configurations for the system attribute set would lookup every submodule which exists, and seek for super.system inside these submodule. This sounds highly confusing to me:

  1. The system might itself have submodules, should they be allowed to participate in the merging part? If a submodule wants to setup a web service, should we state applySupermodules = "services";?
  2. A single module might allow any unrelated submodule to participate in the system configuration.
  3. There is no option definition which explicit the relation. Having implicit relations sounds more dangerous, from my point of view, than having explicit declarations and definitions.

[...] One way to achieve this is by requiring the module author to specify which options can be set via supermodules, as you've shown in your example (config.aggregate = /*...*/). This gets the job done, but it would be nicer if the module author didn't have to specify this. We could take this information from the imports graph, paired with some logic that can also be used for imports hermeticity checking, which was identified to be useful for NixOS/rfcs#78.

My understanding between what you suggest and the current patch is that the current patch is making everything explicit, at the cost of having 10's of options to export within submodules, and to alias in supermodules.

While your approach is to take an holistic approach to cover an attribute set of options, and alias these implicitly between submodules and supermodules.

I think the implicit / holistic approach to be dangerous and a huge performance hazard. Dangerous as a submodule which was not meant to participate can be taken into account, and a performance hazard as all options declarations would have to be scanned to filter all submodules which might provide extra definitions. This holistic approach will also skip mkIf properties, such as enable flags.

So this PR seems to provide an intermediate solution for part of the problem that will not be part of the final solution, unless I'm wrong about the feasibility of the design I've written about here.

Well, ... I do not think many options would have to be aliased, and if this is the case, maybe we can come-up with a short notation for it, similar to mkRenamedOptionModule. And if this short notation is too much of a burden as well, then we can think of aliasing set of options too.

I do not think that adding these short notation should block this change, nor that it would be too hard to add these notations in follow-up changes, if needed.

@roberth
Copy link
Member

roberth commented Jan 17, 2022

Superoptions or supermodules

I've found another way to write supermodules that avoids the early merge problem. It is very similar to what you're introducing in this PR. Here's what I did:

In the submodule:

      options.systemConfig = lib.mkOption {
        internal = true;
        type = types.unspecified; # A function from module arguments to config.
       };

      config.systemConfig = { config, ... }: {
           systemd.services."hercules-ci-agent${suffix}" = {
           # ...
       }

In the parent module ("system" module):

systemArgs@{ config, lib, ... }:
let
  mergeSub =
    f: lib.mkMerge (map (sub: f (sub.systemConfig systemArgs)) (lib.attrValues config.services.hercules-ci-agents));
    config = {
      nix = mergeSub (c: c.nix);
      systemd = mergeSub (c: c.systemd);
      users = mergeSub (c: c.users);
    }

This sidesteps the early/double merging problem by returning a function in a type that doesn't merge.
The other problems are still solved "manually", but can be generalized.

The main difference comes from trying to implement supermodules as opposed to superoptions -- keeping the system-level options together, as in a module.
Like a normal system-level module, the supermodule can also access the system-level config, pkgs, etc.
Some improvements are still necessary to turn this into a generalizable solution:

  1. Have a better type than unspecified.
  2. Generate the config value. This is still necessary for laziness and efficiency and seems unavoidable. Thank you for clarifying the performance hazard.
    1. Writing it manually is error prone (and slightly repetitive). Specifically, if the module author makes a change that sets an option that's not below nix, systemd, users (in this example), the new value is silently ignored. This problem can be detected by generating an assertions value that checks whether the set of options paths is complete. assertions can be evaluated last, so shouldn't create a strictness problem.
      This does depend on assertions, which is a NixOS module, not a module system built-in. We'll want to implement this first.
    2. The projection c: c.systemd is a little simplistic, because it doesn't consider mkIf.

Also perhaps some other improvements are still possible, such as putting the supermodules in _module, accessing all submodule values through options instead.

Full example: https://github.com/hercules-ci/hercules-ci-agent/pull/359/files#diff-508731c212bbfef59eff41255fdb91a1e89bb82e9eadd6e7ddb4630f9ac30d14

@edolstra
Copy link
Member

IMHO it shouldn't be possible for submodules to modify the super-configuration at all. I.e. it should not be possible for security.acme.certs to modify services.systemd.services.

The big problem with the NixOS module system as currently used is that it breaks POLA, i.e. any top-level module can modify anything. For instance, the postgresql module can change my window manager settings. So we should move to submodules that have a much more limited scope (e.g. a "system service" submodule type that can declare a systemd unit but can't mess with the global system configuration). But obviously that depends on submodules not being able to modify the parent configuration.

@roberth
Copy link
Member

roberth commented Jan 17, 2022

@edolstra, except for my unrealistic proposal where "supermodules" would be integrated into the system automatically, they don't actually allow the submodule itself to modify the system config.
The regular, non-submodule, "system-level" module is still responsible for setting system level configuration. What we're discussing is providing means to make this easier, as having access to both module and submodule scope simultaneously is convenient, especially when writing modules that allow multiple instances of a service.

Regarding POLA, this can be a useful guide, but not a necessary one, as we do not need to treat those modules as adversarial. Services, as they exist in NixOS and its siblings, are already fully allowed to set any system configuration. This is what makes NixOS convenient and powerful, with cross-service functionality like ACME as a prime example.

For instance, the postgresql module can change my window manager settings.

Yet, it doesn't. We can trust the postgresql module to do the right thing.

I'd be interested in a different configuration model where POLA applies within the configuration manager, but this would be a research project, not an incremental improvement to a tangential PR.

(e.g. a "system service" submodule type that can declare a systemd unit but can't mess with the global system configuration). But obviously that depends on submodules not being able to modify the parent configuration.

Submodules are all of the same type, as far as the module system is concerned. I think you mean a set of new options like restricted.services.<name>.x, I can see that work. We can easily have imports at the submodule level too (esp with submoduleWith). If this is what you're thinking of, please create an issue where this can be discussed separately.

Again, this is possible both before and after this PR. Applying "supermodule" configuration automatically is definitively off the table. This PR can continue with the goal of making the already common module+submodule logic a little easier to write.

@nbp
Copy link
Member Author

nbp commented Jan 20, 2022

IMHO it shouldn't be possible for submodules to modify the super-configuration at all. I.e. it should not be possible for security.acme.certs to modify services.systemd.services.

The big problem with the NixOS module system as currently used is that it breaks POLA, i.e. any top-level module can modify anything. For instance, the postgresql module can change my window manager settings. So we should move to submodules that have a much more limited scope (e.g. a "system service" submodule type that can declare a systemd unit but can't mess with the global system configuration). But obviously that depends on submodules not being able to modify the parent configuration.

Yes, NixOS breaks the Principle of Least Priviledges.
However, I completely disagree on the fact, that removing this feature is beneficial for end users.

The fact that a security flag can harden all services.systemd.services sounds to me like an appealing feature. If we were to use PoLP, we would limit our-self to only providing self-contained abstractions where users would have to manually forward the each configuration option.

Breaking PoLP gives us the ability to add abstractions and integrations. And today, these 2 are lacking:

  • Integration is completely left behind in NixOS. One will provide a module, but not plug it into other components, the minimal set you will find is that it will register a systemd service. Great⸮ This is what every over distribution provide out there! While for example, each module could provide an nginx/apache backend for all services which are requiring a website to work. No …, let's document that on a website a redo the same mistakes which motivated me to create the module system in the first place … what a shame!
  • Abstraction is also left out of NixOS, the only level of abstractions we have today is to provide installation devices. Great⸮ These already existed a decade ago! In the mean time the community is creating additional forks of NixOS, to provide curated version of it such as Musnix, Simple NixOS Mailserver or SelfPrivacy.

Unfortunately, this seems to be a general trend with Nix and NixOS these days, which is to make it good for DevOps but aweful for the rest of the world. Just because the few companies who are paying for making a mess in it only use it for DevOps should not imply that we degrade the experience of everybody else.

So, yes, we could add PoLP and make sure everybody learns about all the settings of everything, and that NixOS become THE super humans distribution who have absolute knowledge of everything, because we are too shy to make integration/abstraction choices.

I understand why DevOps prefer to have PoLP than trusting other developers. One reason is a flaw in how people operate today, which is that they want the latest versions of everything including bugs, but they do not want these bugs to mess-up other things.

  • Not having PoLP implies that we trust each others, we trust that developers are able to curate good settings, we trust that the integrations is well selected.
  • Not having PoLP means that we reduce the cognitive load of our users, with a risk induced by the trust we put in developers of NixOS.
  • Having PoLP implies that we validate every other changes that people are making, and selectively integrate each change at our discretion, even if this means adding more bugs in the process, but these would be our bugs and not others bugs.

At last … if you do not want to live in a world of trust, we can easily add a function to sandbox imported modules, and there would be no need for adding PoLP in NixOS, nor making another language from scratch … But then you would be in charge of validating every imported options, and risk having inconsistent configurations.

An alternative to having PoLP is to use features provided by the Linux kernel to sandbox applications, such as using hardening flags offered by systemd … if only we have a single flag to toggle that on all systemd services …

My values are towards reducing the cognitive load of NixOS users! Trust has to be part of the equation as I am personally unable to validate the hundred millions lines of code involved in building a running system.

@aanderse
Copy link
Member

Breaking PoLP gives us the ability to add abstractions and integrations. And today, these 2 are lacking:

* Integration is completely left behind in NixOS. One will provide a module, but not plug it into other components, the minimal set you will find is that it will register a systemd service. Great⸮ This is what every over distribution provide out there! While for example, each module could provide an nginx/apache backend for all services which are requiring a website to work. No …, let's document that on a website a redo the same mistakes which motivated me to create the module system in the first place … what a shame!

* Abstraction is also left out of NixOS, the only level of abstractions we have today is to provide installation devices. Great⸮ These already existed a decade ago! In the mean time the community is creating additional forks of NixOS, to provide curated version of it such as [Musnix](https://github.com/musnix/musnix), [Simple NixOS Mailserver](https://gitlab.com/simple-nixos-mailserver/nixos-mailserver) or [SelfPrivacy](https://git.selfprivacy.org/SelfPrivacy/selfprivacy-nixos-config).

Thanks for writing that @nbp! Especially +1 to the above quoted points. ❤️

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 31, 2022
@LiGoldragon

This comment was marked as off-topic.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 22, 2022
@roberth

This comment was marked as off-topic.

@LiGoldragon

This comment was marked as off-topic.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/is-it-possible-to-define-systemd-services-in-a-submodule/39538/5

@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants