Conversation
92cd3b0 to
d4133fe
Compare
|
d4133fe to
60204a5
Compare
philiptaron
left a comment
There was a problem hiding this comment.
Thanks for putting this together! Checkmate looks like a useful monitoring tool, and the overall structure of the package + module + test is solid.
I dug into the upstream source to understand the environment variables, and found a couple of issues that need addressing before this can merge:
REFRESH_TOKEN_SECRET doesn't exist upstream. The module sets REFRESH_TOKEN_SECRET = cfg.settings.tokenTTL in the systemd environment, but the Checkmate server never reads this variable. It only appears once in the entire upstream repo — as a commented-out line in a Helm chart values file. The server's env validation (server/src/validation/envValidation.ts) doesn't include it, and SettingsService doesn't use it. This line should just be removed.
settings.port is misleading. The upstream server hardcodes port 52345 in server/src/index.ts:53 — it's not configurable via an environment variable. The module's port option only affects the nginx proxyPass target URL, so if someone sets it to anything other than 52345, nginx will proxy to the wrong port and things will silently break. I'd suggest either removing the option and hardcoding 52345 in the nginx config, or at minimum documenting that it must match the upstream default.
The test should exercise nginx. Right now the test curls the backend directly on port 52345, but the module sets up nginx as a reverse proxy. It'd be good to also test a request through nginx to validate that the proxy config actually works.
A few smaller things I noticed while reading through:
mkEnableOption "Enable Checkmate."produces "Whether to enable Enable Checkmate.." — should be something likemkEnableOption "the Checkmate monitoring server"inherit (builtins) toStringis unnecessary sincetoStringis already in scopebuild-systeminpackage.nixis a Python/buildPythonPackage concept —makeWrappershould go innativeBuildInputsinstead- The
package-lock.jsonoverride and the.toFixed()source patch could use comments explaining why they're needed
d527c68 to
bbe097b
Compare
removed. it was my mistake.
the issue is fixed. the support for a PORT env were removed by mistake. The upstream released new version (3.5.1) with appropriate fixes.
done.
fixed.
fixed.
|
bbe097b to
c3f282a
Compare
An open-source, self-hosted monitoring tool for tracking server hardware, uptime, response times, and incidents in real-time with beautiful visualisations. Checkmate regularly checks whether a server/website is accessible and performs optimally, providing real-time alerts and reports on the monitored services' availability, downtime, and response time.
Checkmate also has an agent, called Capture, to retrieve data from remote servers. While Capture is not required to run Checkmate, it provides additional insights about your servers' CPU, RAM, disk, and temperature status.
homepage: https://checkmate.so
documentation: https://docs.checkmate.so
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.