From ec69cf3f7b865bd6ed95451855518fb1d070d2dc Mon Sep 17 00:00:00 2001 From: K900 Date: Sat, 25 Jul 2026 10:19:30 +0300 Subject: [PATCH] Revert "nixos/modular-services: add portable `process.environment`" --- doc/build-helpers/testers.chapter.md | 2 +- lib/services/service.nix | 20 ------ lib/services/test.nix | 11 ---- nixos/doc/manual/default.nix | 11 +--- .../system/service/systemd/service.nix | 31 +-------- .../modules/system/service/systemd/system.nix | 2 +- nixos/modules/system/service/systemd/test.nix | 50 --------------- .../testers/modular-service-compliance.nix | 63 ------------------- 8 files changed, 6 insertions(+), 184 deletions(-) diff --git a/doc/build-helpers/testers.chapter.md b/doc/build-helpers/testers.chapter.md index 4c1e7cb47e01..241acb439ea6 100644 --- a/doc/build-helpers/testers.chapter.md +++ b/doc/build-helpers/testers.chapter.md @@ -741,7 +741,7 @@ Notable attributes: Compliance suite for [modular service](https://nixos.org/manual/nixos/unstable/#modular-services) integrations. -Tests that a service manager integration correctly handles the portable modular services contract: `process.argv`, `process.environment` (including `null` values that unset a variable), sub-services, assertions, and warnings. +Tests that a service manager integration correctly handles the portable modular services contract: `process.argv`, sub-services, assertions, and warnings. ### Return value {#tester-modularServiceCompliance-return} diff --git a/lib/services/service.nix b/lib/services/service.nix index 1af36a1dae42..5911202547fe 100644 --- a/lib/services/service.nix +++ b/lib/services/service.nix @@ -70,26 +70,6 @@ in Command used for reloading in the underlying service manager to reload. ''; }; - - environment = lib.mkOption { - type = types.lazyAttrsOf ( - types.nullOr (types.coercedTo (types.either types.path types.package) (x: "${x}") types.str) - ); - default = { }; - example = lib.literalExpression ''{ FOO = "bar"; PATH = null; }''; - description = '' - Environment variables passed verbatim to the service process by the - service manager. Entries set to `null` actively unset the variable - before the process starts -- backends without native unset support use - a wrapper (e.g. `execline`'s `unexport`) so the variable is absent - even when the service manager or a backend-specific override would - otherwise supply it. - - Values appear in the rendered unit and may be world-readable. For - secrets, use a backend-specific mechanism such as - `systemd.service.serviceConfig.EnvironmentFile`. - ''; - }; }; notificationProtocol = mkOption { diff --git a/lib/services/test.nix b/lib/services/test.nix index 2ba3b6e07009..65d05ceb39e9 100644 --- a/lib/services/test.nix +++ b/lib/services/test.nix @@ -48,10 +48,6 @@ let (dummyPkg "cowsay.sh") "world" ]; - environment = { - FOO = "bar"; - DROPPED = null; - }; }; }; service3 = { @@ -114,7 +110,6 @@ let "/usr/bin/echo" "hello" ]; - environment = { }; }; services = { }; assertions = [ @@ -133,10 +128,6 @@ let "${dummyPkg "cowsay.sh"}" "world" ]; - environment = { - FOO = "bar"; - DROPPED = null; - }; }; services = { }; assertions = [ ]; @@ -145,7 +136,6 @@ let service3 = { process = { argv = [ "/bin/false" ]; - environment = { }; }; services.exclacow = { process = { @@ -153,7 +143,6 @@ let "${dummyPkg "cowsay-ng"}/bin/cowsay" "!" ]; - environment = { }; }; services = { }; assertions = [ diff --git a/nixos/doc/manual/default.nix b/nixos/doc/manual/default.nix index a172e1eb36a0..78f44ad9ad74 100644 --- a/nixos/doc/manual/default.nix +++ b/nixos/doc/manual/default.nix @@ -152,16 +152,7 @@ let }; systemdServiceOptions = buildPackages.nixosOptionsDoc { - inherit - (evalModules { - modules = [ - (modules.importApply ../../modules/system/service/systemd/service.nix { - pkgs = throw "nixos docs / systemdServiceOptions: Do not reference pkgs in docs"; - }) - ]; - }) - options - ; + inherit (evalModules { modules = [ ../../modules/system/service/systemd/service.nix ]; }) options; # TODO: filter out options that are not systemd-specific, maybe also change option prefix to just `service-opt-`? inherit revision warningsAreErrors; transformOptions = diff --git a/nixos/modules/system/service/systemd/service.nix b/nixos/modules/system/service/systemd/service.nix index ae68945b2a44..e7fa338ff97b 100644 --- a/nixos/modules/system/service/systemd/service.nix +++ b/nixos/modules/system/service/systemd/service.nix @@ -1,9 +1,3 @@ -# Non-module arguments -# These are separate from the module arguments to avoid implicit dependencies. -# This makes service modules self-contained, allowing mixing of Nixpkgs versions. -{ pkgs }: - -# The module { lib, config, @@ -98,11 +92,6 @@ in to prevent systemd substitution. Set this option explicitly to enable systemd's substitution features. - When {option}`process.environment` contains keys set to `null`, the default - is automatically prefixed with `unexport KEY` invocations (from - `pkgs.execline`) so those variables are unset before the process starts, - regardless of what `Environment=` or inherited environment supplies. - To extend {option}`process.argv` with systemd specifiers, you can append to the escaped arguments: @@ -120,19 +109,8 @@ in for available specifiers like `%n`, `%i`, `%t`. ''; type = types.str; - default = - let - nullEnvKeys = lib.attrNames (lib.filterAttrs (_: v: v == null) config.process.environment); - in - if nullEnvKeys == [ ] then - config.systemd.lib.escapeSystemdExecArgs config.process.argv - else - lib.concatMapStringsSep " " ( - k: "${escapeSystemdExecArg "${pkgs.execline}/bin/unexport"} ${escapeSystemdExecArg k}" - ) nullEnvKeys - + " " - + config.systemd.lib.escapeSystemdExecArgs config.process.argv; - defaultText = lib.literalMD "The escaped `process.argv`, prefixed with `\"unexport\" \"KEY\"` (from `pkgs.execline`) for each key in `process.environment` set to `null`."; + default = config.systemd.lib.escapeSystemdExecArgs config.process.argv; + defaultText = lib.literalExpression "config.systemd.lib.escapeSystemdExecArgs config.process.argv"; }; systemd.mainExecReload = mkOption { @@ -209,7 +187,7 @@ in types.submoduleWith { class = "service"; modules = [ - (lib.modules.importApply ./service.nix { inherit pkgs; }) + ./service.nix ]; specialArgs = { inherit systemdPackage; @@ -229,9 +207,6 @@ in systemd.services."" = { # TODO description; wantedBy = lib.mkDefault [ "multi-user.target" ]; - environment = lib.mapAttrs (_: lib.mkDefault) ( - lib.filterAttrs (_: v: v != null) config.process.environment - ); serviceConfig = { ExecReload = config.systemd.mainExecReload; Type = lib.mkDefault (if config.notificationProtocol.systemd then "notify" else "simple"); diff --git a/nixos/modules/system/service/systemd/system.nix b/nixos/modules/system/service/systemd/system.nix index 3cf6ee95e969..b1ae25b85118 100644 --- a/nixos/modules/system/service/systemd/system.nix +++ b/nixos/modules/system/service/systemd/system.nix @@ -63,7 +63,7 @@ let modularServiceConfiguration = portable-lib.configure { serviceManagerPkgs = pkgs; extraRootModules = [ - (lib.modules.importApply ./service.nix { inherit pkgs; }) + ./service.nix ./config-data-path.nix ]; extraRootSpecialArgs = { diff --git a/nixos/modules/system/service/systemd/test.nix b/nixos/modules/system/service/systemd/test.nix index edead26e5f02..abd192716b09 100644 --- a/nixos/modules/system/service/systemd/test.nix +++ b/nixos/modules/system/service/systemd/test.nix @@ -76,40 +76,6 @@ let }; }; - # Test that `process.environment` becomes `Environment=` entries on the unit, - # that null values are dropped from `Environment=` and wrapped with unexport - # in `ExecStart`. - system.services.envvars = { - process = { - argv = [ hello' ]; - environment = { - FOO = "bar"; - BAZ = "qux"; - DROPPED = null; - }; - }; - }; - - # Test that an explicit `systemd.service.environment` override wins over - # the portable default produced by `process.environment`. - system.services.envvars-override = { - process = { - argv = [ hello' ]; - environment.FOO = "from-process"; - }; - systemd.service.environment.FOO = "from-systemd"; - }; - - # Test that `process.environment` `null` unsets via wrapper even when the - # systemd layer sets the same key (true unset, not just "skip setting"). - system.services.envvars-unset = { - process = { - argv = [ hello' ]; - environment.FOO = null; - }; - systemd.service.environment.FOO = "leaked"; - }; - # Test extending process.argv with systemd specifiers system.services.argv-extended = { config, ... }: @@ -171,22 +137,6 @@ runCommand "test-modular-service-systemd-units" # The base command should be escaped ($1 -> $$1, m%n -> m%%n), but the appended --systemd-unit %n should not be grep -F 'ExecStart="${hello}/bin/hello" "--greeting" "Fun $$1 fact, remainder is often expressed as m%%n" --systemd-unit %n' ${toplevel}/etc/systemd/system/argv-extended.service >/dev/null - # process.environment becomes Environment= entries; null values are dropped - # from Environment= and wrapped with unexport in ExecStart. - grep -F 'Environment="FOO=bar"' ${toplevel}/etc/systemd/system/envvars.service >/dev/null - grep -F 'Environment="BAZ=qux"' ${toplevel}/etc/systemd/system/envvars.service >/dev/null - ! grep -F 'Environment=.*DROPPED' ${toplevel}/etc/systemd/system/envvars.service - grep 'ExecStart=.*unexport.*DROPPED' ${toplevel}/etc/systemd/system/envvars.service >/dev/null - - # systemd.service.environment override wins over process.environment. - grep -F 'Environment="FOO=from-systemd"' ${toplevel}/etc/systemd/system/envvars-override.service >/dev/null - ! grep -F 'FOO=from-process' ${toplevel}/etc/systemd/system/envvars-override.service - - # process.environment null uses unexport wrapper for true unset, even when - # the systemd layer has an Environment= entry for the same key. - grep -F 'Environment="FOO=leaked"' ${toplevel}/etc/systemd/system/envvars-unset.service >/dev/null - grep 'ExecStart=.*unexport.*FOO' ${toplevel}/etc/systemd/system/envvars-unset.service >/dev/null - [[ ! -e ${toplevel}/etc/systemd/system/foo.socket ]] [[ ! -e ${toplevel}/etc/systemd/system/bar.socket ]] [[ ! -e ${toplevel}/etc/systemd/system/bar-db.socket ]] diff --git a/pkgs/build-support/testers/modular-service-compliance.nix b/pkgs/build-support/testers/modular-service-compliance.nix index 7b5c51bebf6e..2349e9e96a8f 100644 --- a/pkgs/build-support/testers/modular-service-compliance.nix +++ b/pkgs/build-support/testers/modular-service-compliance.nix @@ -29,10 +29,6 @@ let services = { svc = { process.argv = [ "${coreutils}/bin/true" ]; - process.environment = { - FOO = "bar"; - DROPPED = null; - }; assertions = [ { assertion = true; @@ -74,19 +70,6 @@ let expected = [ "${coreutils}/bin/true" ]; }; - # A set environment variable round-trips through process.environment. - testProcessEnvironment = { - expr = c.process.environment.FOO; - expected = "bar"; - }; - - # A null environment variable is preserved as null (unset request), - # rather than coerced to a string or dropped from the attrset. - testProcessEnvironmentNull = { - expr = c.process.environment.DROPPED; - expected = null; - }; - testAssertions = { expr = lib.elem { assertion = true; @@ -203,9 +186,6 @@ let mkdir -p "$dir" echo "$$" > "$dir/pid" printf '%s\n' "$@" > "$dir/args" - # Record the process's own environment as received from the service - # manager (NUL-delimited, as the kernel stores it). - "${coreutils}/bin/cat" "/proc/$$/environ" > "$dir/environ" exec "${coreutils}/bin/sleep" infinity ''; @@ -258,31 +238,6 @@ let || { echo "${id}: expected arg ${lib.escapeShellArg arg} not found"; cat "${sharedDir}/${id}/args"; exit 1; } '') expectedArgs; - /** - Shell snippet: assert that the service's recorded environment contains - each `present` entry (an exact `KEY=value` string) and contains no - variable named in `absent`. - */ - checkEnv = - id: - { - present ? [ ], - absent ? [ ], - }: - '' - # The recorded environ is NUL-delimited; render one entry per line. - tr '\0' '\n' < "${sharedDir}/${id}/environ" > "${sharedDir}/${id}/environ.lines" - '' - + lib.concatMapStrings (entry: '' - grep -qxF -- ${lib.escapeShellArg entry} "${sharedDir}/${id}/environ.lines" \ - || { echo "${id}: expected env ${lib.escapeShellArg entry} not found"; cat "${sharedDir}/${id}/environ.lines"; exit 1; } - '') present - + lib.concatMapStrings (key: '' - if grep -qE ${lib.escapeShellArg "^${key}="} "${sharedDir}/${id}/environ.lines"; then - echo "${id}: env variable ${lib.escapeShellArg key} should be unset"; exit 1 - fi - '') absent; - mkTestScript = name: text: lib.getExe (writeShellApplication { @@ -375,24 +330,6 @@ in ); }; - environment = mkTest { - name = "${namePrefix}-environment"; - services.test = { - process.argv = mkArgv "env" [ ]; - process.environment = { - FOO = "bar"; - DROPPED = null; - }; - }; - testExe = mkTestScript "environment" ( - waitAndCheck "env" [ ] - + checkEnv "env" { - present = [ "FOO=bar" ]; - absent = [ "DROPPED" ]; - } - ); - }; - sub-services = mkTest { name = "${namePrefix}-sub-services"; services.a = {