Add instant clusters monitoring#143
Open
brennentsmith-runpod wants to merge 1 commit into
Open
Conversation
| for k, v in resp.getheaders(): | ||
| if k.lower() in DROP: | ||
| continue | ||
| self.send_header(k, v) |
| for k, v in resp.getheaders(): | ||
| if k.lower() in DROP: | ||
| continue | ||
| self.send_header(k, v) |
| # Fresh valid token in the URL -> (re)establish the session cookie. | ||
| self.send_header( | ||
| "Set-Cookie", | ||
| f"{COOKIE}={token}; Path=/; HttpOnly; Secure; SameSite=Lax", |
| # Fresh valid token in the URL -> (re)establish the session cookie. | ||
| self.send_header( | ||
| "Set-Cookie", | ||
| f"{COOKIE}={token}; Path=/; HttpOnly; Secure; SameSite=Lax", |
daveseddon-runpod
left a comment
There was a problem hiding this comment.
Nice work. I reviewed this by standing up a pinned Nix static-analysis harness (ruff, bandit, shellcheck, hadolint, yamllint, actionlint, JSON validation) and running it against the new files, plus a manual pass on the security-sensitive pieces. The auth model is well thought through and correctly implemented — no blocking issues.
Security model — verified sound ✅
The shared-token proxy in front of Grafana holds up end to end:
- Grafana binds loopback only (
http_addr = 127.0.0.1:3300) andauth.proxyuseswhitelist = 127.0.0.1, so the injectedX-WEBAUTH-Useridentity is unspoofable from outside. auth_proxy.pystrips any client-suppliedX-WEBAUTH-*and injects it only on a successfulhmac.compare_digesttoken match (constant-time).- Login form and HTTP basic auth are disabled, and
post_start.shrandomizes the built-in admin password — so the proxy is genuinely the only way in. - Only
EXPOSE 8889(the proxy) is public;3300is never mapped. No bypass path.
Nits (non-blocking)
post_start.sh:71— shellcheck SC2155.export GF_SECURITY_ADMIN_PASSWORD="$(openssl rand … || …)"masks the subshell's return value. Harmless here (the||fallback covers a missingopenssland the export status isn't checked), but you could split declaration/assignment to keep shellcheck clean.Dockerfile:33— hadolint DL3008 (unpinned apt versions). Consistent with the ignore rules the repo already applies to its other Dockerfiles, so fine to leave — flagging only for parity.cluster.yml— actionlint "unknown runner label" forblacksmith-16vcpu-ubuntu-2404/blacksmith-4vcpu-ubuntu-2204. False positive — these are the same Blacksmith self-hosted runners already used inbase.yml,nvidia.yml, androcm.yml. If we want CI to stop warning, we could add an.github/actionlint.yamldeclaring the custom labels.
Optional robustness notes in auth_proxy.py (low risk for this use case, just noting)
connis only closed on the success path; a non-OSErrorexception fromgetresponse()/read()would leak the upstream connection.- Chunked request bodies aren't forwarded (
transfer-encodingis stripped and onlyContent-Lengthbodies are read). Grafana traffic is effectively alwaysContent-Length, so this shouldn't bite in practice.
Clean: ruff (auth_proxy.py), all 6 Grafana dashboard JSONs parse valid, yamllint (only a line-length warning on the workflow).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.