Safe and readable ExecStart for systemd proposal

[roberth]

Describe the bug

ExecStart is hard to read and write.

• Cryptic prefixes
• Surprising escaping (ref unescaping: #154113, although perhaps a clean slate is preferable https://github.com/NixOS/nixpkgs/pull/154113#discussion_r780763630)

script is only suitable for a subset of use cases. (#135557, maybe other problems?)

• No prefixes
• No substitutions whatsoever
• Only ExecStart

Expected behavior

• Uphold the rule that a string is a string unless explicitly requested to be modified.
• Use module system enums instead of cryptic prefixes (or perhaps add a bool if a prefix can always be combined etc; haven't fully dissected those)
• Manage expectations: these aren't bash statements

It might look like this:

systemd.services.foo = {
  execStarts = [  # plural; it's a list
    { mode = "privileged";  # `+` prefix; TODO need to research exactly how prefixes combine
      exe = "echoAll";
      args = [ "$a" { substitute = "%i"; } ];  # escaped by default. not escaped when in { substitute = x; }
    }
  ]
}

Maybe even s/args/argv. More explicit, but also cryptic if you don't know C.

Additional context

man systemd.service (Exec* family of fields) man systemd.unit (substitutions)

Notify maintainers

@NixOS/systemd

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
output here

[pennae]

this sounds like a very good idea. not sure about calling the marker attr substitute though and what to do about string-coercible attrsets. maybe we should take inspiration from the module system and use a { type = "_rawArgument"; value = "%i"; } and a wrapper function rawArg? but that's going into bikeshedding :)

[roberth]

I'm open to many bike shed colors. { substitute = e; } has the benefit of not requiring any imports, but I don't care much.

[andir]

I think this might be a good idea. What I would like to avoid is a UI where we have serviceConfig.ExecStart and execStarts where both accept a different format. To stay true to our currently principles the serviceConfig arguments aren't really converting their values (other than changing booleans and allowing lists).

Perhaps we should call ist startScripts and support defining a script (as text) and the prefix for that only. What is the gain of the args/argv separation other than it being a syscall thing that nobody should have to care about.

[roberth]

What I would like to avoid is a UI where we have serviceConfig.ExecStart and execStarts where both accept a different format.

It wouldn't be the only module that provides both a high-level(-ish) interface and a low level one. I agree that this phenomenon can cause confusion and I think the best means we currently have to mitigate this, is to migrate code and documentation to the new way, so the redundant low-level variation can be forgotten. Alternatively, our linters could warn when they see the token ExecStart, ExecStartPre, etc.

To stay true to our currently principles

The principles must be different for low-level and high-level interfaces. We should have both and prefer to use high-level when possible.

Perhaps we should call it startScripts

I'm not opposed to a different name, but I don't see how we can capture the distinction between ExecStart, ExecStartPre, etc. with this name. It also doesn't convey that they're not quite scripts.

I suppose we could have a cascade of interfaces to the unit file:

• High-level conveniences like script.
• Custom / somewhat high-level representation of specific unit fields: wants, execStart
• Low-level; right down to the file: serviceConfig.*

What is the gain of the args/argv separation

Convey that these aren't bash statements, let alone shell scripts.

Script functionality could be added, to the submodule, as an alternative to exe with args, but this requires coming up with a way to get the % substitution values into the script. This is entirely feasible, but perhaps better scoped out at first.

[pennae]

how about this:

systemd.services gains new attributes {start,preStart,postStart,reload,stop,postStop}Commands. type for each is list of a new submodule type:

• argv0: nullOr string = null, equivalent to setting the @ prefix and inserting a first argument when not null
• mayFail: bool = false, equivalent to the - command prefix
• unrestricted: nullOr (oneOf [ "full" "credentials" ]) = null, equivalent to + and ! respectively.
• exe: string
• args: listOf (oneOf [ int float string expansion ]) = [ ], where the strings are escaped before being inserted into the file. expansion could be an attrset type that allows either a single env attr to expand an env var, or a single substitute attr to expand a complete string by systemd's rules. (the distinction is useful because systemd expand $FOO and ${FOO} differently, and when expanding env vars we usually want ${FOO} everywhere)

the !! command prefix is out of scope because all our kernels support ambient capabilities. clearing an Exec* directive from the commands is also out of scope for now, but could be done later when the first entry of a command list is { }. scripts could be supported by providing another attr script that is mutually exclusive with exe but retains the args list to get expanded %somethings into the script.

*Commands attrs are mutually exclusive with their corresponding script counterparts.

[roberth]

It didn't occur to me that script doesn't replace args.

scripts could be supported by providing another attr script that is mutually exclusive with exe

This should use ~mkAliasDefinitions~mkDerivedConfig, so that the priority behavior, including the mutual exclusion, works.

attributes {start,preStart,postStart,reload,stop,postStop}Commands.

I wish systemd had a more specific term for these lines, but "command" seems to it. "Exec" could be an alternative, but I feel that would just look weird, comparatively.

[ int float string expansion ]

[ string expansion int float ] order of relevance and frequency.

So, only nitpicking.

These are some good decisions you've made.

[pennae]

not sure how mkAliasDefinitions would work here, but exe = mkIf (cfg.script != null) (mkDerivedConfig opts.script toFile) might work? haven't used those features of the module system so far.

[roberth]

Oh, I confused the two. mkAliasDefinitions does not set the priority; mkDerivedConfig does.