diff --git a/nixos/modules/services/monitoring/prometheus/exporters.nix b/nixos/modules/services/monitoring/prometheus/exporters.nix index 2dc12a221bf0..b54bdcded180 100644 --- a/nixos/modules/services/monitoring/prometheus/exporters.nix +++ b/nixos/modules/services/monitoring/prometheus/exporters.nix @@ -3,7 +3,7 @@ let inherit (lib) concatStrings foldl foldl' genAttrs literalExpression maintainers mapAttrs mapAttrsToList mkDefault mkEnableOption mkIf mkMerge mkOption - optional types mkOptionDefault flip attrNames; + optional types mkOptionDefault flip attrNames xor; cfg = config.services.prometheus.exporters; @@ -230,6 +230,7 @@ let in mkIf conf.enable { warnings = conf.warnings or []; + assertions = conf.assertions or []; users.users."${name}-exporter" = (mkIf (conf.user == "${name}-exporter" && !enableDynamicUser) { description = "Prometheus ${name} exporter service user"; isSystemUser = true; @@ -363,13 +364,6 @@ in Please specify either 'services.prometheus.exporters.nextcloud.passwordFile' or 'services.prometheus.exporters.nextcloud.tokenFile' ''; - } { - assertion = cfg.pgbouncer.enable -> ( - (cfg.pgbouncer.connectionStringFile != null || cfg.pgbouncer.connectionString != "") - ); - message = '' - PgBouncer exporter needs either connectionStringFile or connectionString configured" - ''; } { assertion = cfg.pgbouncer.enable -> ( config.services.pgbouncer.ignoreStartupParameters != null && builtins.match ".*extra_float_digits.*" config.services.pgbouncer.ignoreStartupParameters != null @@ -413,7 +407,15 @@ in Please ensure you have either `services.prometheus.exporters.idrac.configuration' or `services.prometheus.exporters.idrac.configurationPath' set! ''; - } ] ++ (flip map (attrNames exporterOpts) (exporter: { + } { + assertion = cfg.pgbouncer.enable -> ( + xor (cfg.pgbouncer.connectionEnvFile == null) (cfg.pgbouncer.connectionString == null) + ); + message = '' + Options `services.prometheus.exporters.pgbouncer.connectionEnvFile` and + `services.prometheus.exporters.pgbouncer.connectionString` are mutually exclusive! + ''; + }] ++ (flip map (attrNames exporterOpts) (exporter: { assertion = cfg.${exporter}.firewallFilter != null -> cfg.${exporter}.openFirewall; message = '' The `firewallFilter'-option of exporter ${exporter} doesn't have any effect unless @@ -427,11 +429,6 @@ in Consider using `services.prometheus.exporters.idrac.configuration` instead. '' ) - (mkIf - (cfg.pgbouncer.enable && cfg.pgbouncer.connectionString != "") '' - config.services.prometheus.exporters.pgbouncer.connectionString is insecure. Use connectionStringFile instead. - '' - ) (mkIf (cfg.pgbouncer.enable && config.services.pgbouncer.authType != "any") '' Admin user (with password or passwordless) MUST exist in the services.pgbouncer.authFile if authType other than any is used. diff --git a/nixos/modules/services/monitoring/prometheus/exporters/pgbouncer.nix b/nixos/modules/services/monitoring/prometheus/exporters/pgbouncer.nix index 71b602638632..471849cad770 100644 --- a/nixos/modules/services/monitoring/prometheus/exporters/pgbouncer.nix +++ b/nixos/modules/services/monitoring/prometheus/exporters/pgbouncer.nix @@ -6,6 +6,7 @@ let mkOption types optionals + getExe escapeShellArg concatStringsSep ; @@ -23,8 +24,8 @@ in }; connectionString = mkOption { - type = types.str; - default = ""; + type = types.nullOr types.str; + default = null; example = "postgres://admin:@localhost:6432/pgbouncer?sslmode=require"; description = '' Connection string for accessing pgBouncer. @@ -35,24 +36,28 @@ in in the services.pgbouncer.authFile if authType other than any is used. WARNING: this secret is stored in the world-readable Nix store! - Use {option}`connectionStringFile` instead. + Use [](#opt-services.prometheus.exporters.pgbouncer.connectionEnvFile) if the + URL contains a secret. ''; }; - connectionStringFile = mkOption { - type = types.nullOr types.path; + connectionEnvFile = mkOption { + type = types.nullOr types.str; default = null; - example = "/run/keys/pgBouncer-connection-string"; description = '' - File that contains pgBouncer connection string in format: - postgres://admin:@localhost:6432/pgbouncer?sslmode=require + File that must contain the environment variable + `PGBOUNCER_EXPORTER_CONNECTION_STRING` which is set to the connection + string used by pgbouncer. I.e. the format is supposed to look like this: - NOTE: You MUST keep pgbouncer as database name (special internal db)!!! + ``` + PGBOUNCER_EXPORTER_CONNECTION_STRING="postgres://admin@localhost:6432/pgbouncer?sslmode=require" + ``` - NOTE: Admin user (with password or passwordless) MUST exist - in the services.pgbouncer.authFile if authType other than any is used. + NOTE: You MUST keep pgbouncer as database name (special internal db)! + NOTE: `services.pgbouncer.settings.pgbouncer.ignore_startup_parameters` + MUST contain "extra_float_digits". - {option}`connectionStringFile` takes precedence over {option}`connectionString` + Mutually exclusive with [](#opt-services.prometheus.exporters.pgbouncer.connectionString). ''; }; @@ -120,10 +125,9 @@ in startScript = pkgs.writeShellScriptBin "pgbouncer-start" "${concatStringsSep " " ([ "${pkgs.prometheus-pgbouncer-exporter}/bin/pgbouncer_exporter" "--web.listen-address ${cfg.listenAddress}:${toString cfg.port}" - "--pgBouncer.connectionString ${if cfg.connectionStringFile != null then - "$(head -n1 ${cfg.connectionStringFile})" else "${escapeShellArg cfg.connectionString}"}" - ] - ++ optionals (cfg.telemetryPath != null) [ + ] ++ optionals (cfg.connectionString != null) [ + "--pgBouncer.connectionString ${escapeShellArg cfg.connectionString}" + ] ++ optionals (cfg.telemetryPath != null) [ "--web.telemetry-path ${escapeShellArg cfg.telemetryPath}" ] ++ optionals (cfg.pidFile != null) [ @@ -145,6 +149,22 @@ in in { ExecStart = "${startScript}/bin/pgbouncer-start"; + EnvironmentFile = lib.mkIf (cfg.connectionEnvFile != null) [ + cfg.connectionEnvFile + ]; }; }; + + imports = [ + (lib.mkRemovedOptionModule [ "connectionStringFile" ] '' + As replacement, the option `services.prometheus.exporters.pgbouncer.connectionEnvFile` + has been added. In contrast to `connectionStringFile` it must be an environment file + with the connection string being set to `PGBOUNCER_EXPORTER_CONNECTION_STRING`. + + The change was necessary since the former option wrote the contents of the file + into the cmdline of the exporter making the connection string effectively + world-readable. + '') + ({ options.warnings = options.warnings; options.assertions = options.assertions; }) + ]; } diff --git a/nixos/tests/prometheus-exporters.nix b/nixos/tests/prometheus-exporters.nix index 56569c4de2c8..af6978bd8809 100644 --- a/nixos/tests/prometheus-exporters.nix +++ b/nixos/tests/prometheus-exporters.nix @@ -435,7 +435,6 @@ let json = { exporterConfig = { enable = true; - url = "http://localhost"; configFile = pkgs.writeText "json-exporter-conf.json" (builtins.toJSON { modules = { default = { @@ -948,7 +947,9 @@ let pgbouncer = { exporterConfig = { enable = true; - connectionStringFile = pkgs.writeText "connection.conf" "postgres://admin:@localhost:6432/pgbouncer?sslmode=disable"; + connectionEnvFile = "${pkgs.writeText "connstr-env" '' + PGBOUNCER_EXPORTER_CONNECTION_STRING=postgres://admin@localhost:6432/pgbouncer?sslmode=disable + ''}"; }; metricProvider = { diff --git a/pkgs/servers/monitoring/prometheus/pgbouncer-exporter.nix b/pkgs/servers/monitoring/prometheus/pgbouncer-exporter.nix index 970d63ec0f2e..90ffdb48f0d1 100644 --- a/pkgs/servers/monitoring/prometheus/pgbouncer-exporter.nix +++ b/pkgs/servers/monitoring/prometheus/pgbouncer-exporter.nix @@ -2,16 +2,16 @@ buildGoModule rec { pname = "pgbouncer-exporter"; - version = "0.8.0"; + version = "0.9.0"; src = fetchFromGitHub { owner = "prometheus-community"; repo = "pgbouncer_exporter"; rev = "v${version}"; - hash = "sha256-QnA9H4qedCPZKqJQ1I2OJO42mCWcWqYxLmeF3+JXzTw="; + hash = "sha256-fKoyRHYLwVefsZ014eazVCD5B9eV8/CUkuHE4mbUqVo="; }; - vendorHash = "sha256-NYiVW+CNrxFrEUl1nsTeNNgy7SmTYgqs1d50rCvyBcw="; + vendorHash = "sha256-IxmxfF9WsF0Hbym4G0UecyW8hAvucoaCFUE1kXUljJs="; meta = with lib; { description = "Prometheus exporter for PgBouncer";