bitflicker64 commented on code in PR #3449:
URL: https://github.com/apache/hugegraph/pull/3449#discussion_r4459423422
##########
helm/hugegraph/templates/hubble-deployment.yaml:
##########
@@ -150,6 +150,18 @@ spec:
exit 1
fi
{{- end }}
+ # fsGroup can widen a persisted key's permissions on volume
mount.
+ # An environment key takes precedence; otherwise restore
owner-only
+ # access without creating a key or following a symbolic link.
+ if [[ "${HUBBLE_TEXT2GQL_MASTER_KEY:-}" =~ ^[[:space:]]*$ &&
+ ! -L "${HG_HUBBLE_TEXT2GQL_MASTER_KEY_FILE}" &&
+ -f "${HG_HUBBLE_TEXT2GQL_MASTER_KEY_FILE}" ]]; then
+ if ! chmod 600 "${HG_HUBBLE_TEXT2GQL_MASTER_KEY_FILE}"; then
+ echo "cannot secure the Text2GQL master key; ensure the file
is owned" \
+ "by the Hubble runAsUser and the data volume is
writable" >&2
+ exit 1
Review Comment:
**A key file the Hubble user cannot `chmod` now stops all of Hubble from
starting**
The new block exits when `chmod 600` fails. Only the file owner (or root)
can `chmod`, and `fsGroup` does not change ownership. So any existing key that
the current `runAsUser` doesn't own makes the container exit before Hubble
starts, and the Pod crash-loops.
How the owner can change: a release that saved an API key while running as
the image's default root user, and was later hardened to a non-root `runAsUser`
with `fsGroup`. `fsGroup` lets H2 write the volume, but the key stays
root-owned.
At `6524cc14`, the same state only made Text2GQL fail (the new UID could not
use the key); graph browsing, schema and loading kept working. At this head, it
takes down the whole Hubble UI, including for users who never use Text2GQL. The
new docs do ask operators to keep `runAsUser` stable, so this needs an
operator-initiated UID change. Reviewers rated the severity differently,
Important vs Minor, but agree on the behaviour.
Suggestion: on `chmod` failure, log a clear warning and continue. Hubble
already refuses a key that isn't owner-only (it returns 503
`secret-unavailable`), so the failure stays limited to Text2GQL. Alternatively,
keep the hard failure but document the `chown` recovery step in
`docs/operations.md`. The current test (`helm/test_helm_hubble.py`,
chmod-failure case) would need updating to match whichever behaviour you choose.
_Edited: an earlier version said that enabling Vermeer changes the UID. That
was wrong: the Vermeer UID follows the configured `runAsUser`, which defaults
to 0._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
