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.
Two ways the owner can change:
- Enabling `vermeer.enabled` on an existing release. The chart then forces
`runAsUser` to the Vermeer UID (`hubble-deployment.yaml` around line 104), but
the key was created by the image's default root user.
- Switching from the default root user to non-root with `fsGroup`, as
`values.yaml` suggests.
At `6524cc14`, the same state only made Text2GQL return 503; 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 docs do say to keep the UID
stable. However, Vermeer can change the UID, so the problem can be hit through
the chart's own settings.
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, and mention the
Vermeer UID change in `docs/operations.md`. The current test
(`helm/test_helm_hubble.py`, chmod-failure case) would need updating to match
whichever behaviour you choose.
##########
helm/hugegraph/templates/hubble-deployment.yaml:
##########
@@ -286,6 +296,8 @@ spec:
value: {{ if $vermeerEnabled }}{{ printf "http://%s:%v"; (include
"hugegraph.vermeer.masterName" .) .Values.vermeer.master.httpPort | quote }}{{
else }}""{{ end }}
- name: HG_HUBBLE_VERMEER_TOKEN_FILE
value: {{ if $vermeerEnabled
}}"/var/run/hugegraph/vermeer/token"{{ else }}""{{ end }}
+ - name: HG_HUBBLE_TEXT2GQL_MASTER_KEY_FILE
+ value: "/hubble-data/text2gql-master.key"
Review Comment:
Addressed in `857dd12c`. When the Hubble user owns the key file, the startup
wrapper now restores it to `0600` before Hubble starts. It skips symlinks and
an environment-supplied key, and the new wrapper tests cover repeated
permission widening. Verified by reading the code and the tests. It was not
exercised on a live cluster or CSI driver. A related follow-up, for the case
where the file is owned by a different UID, is in a new R2 thread. Resolving
this one.
##########
helm/hugegraph/templates/hubble-deployment.yaml:
##########
@@ -267,6 +272,11 @@ spec:
if [[ "${FOUND_SERVER_HOST}" == false ]]; then
printf 'server.host=0.0.0.0\n' >>"${TMP}"
fi
+ # The image default (data/text2gql-master.key, relative to
/hubble)
+ # is outside the data volume; keep the key next to the H2
database.
Review Comment:
Addressed in `857dd12c`. The new section
`docs/operations.md#hubble-text2gql-credentials`, linked from the README and
`values.yaml`, documents the one-time re-entry, why H2 still shows a saved key,
lazy key creation, backup alongside H2, the `emptyDir` limit and the
`HUBBLE_TEXT2GQL_MASTER_KEY` alternative. Resolving.
--
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]
