Skip to content

feat(docker): run PD, Store and Server images as a non-root user - #3381

Open
byteayan wants to merge 4 commits into
apache:masterfrom
byteayan:claude/docker-non-root-images
Open

byteayan wants to merge 4 commits into
apache:masterfrom
byteayan:claude/docker-non-root-images

Conversation

@byteayan

Copy link
Copy Markdown
Contributor

Purpose of the PR

The hugegraph/pd, hugegraph/store, hugegraph/server and hugegraph/hugegraph images run Java as root. A namespace under the restricted Pod Security Standard rejects them, the Helm chart cannot default to non-root, and scanners flag every workload.

Upgrade required: existing Docker volumes written by the old root images need a one-time chown, and existing Helm releases upgraded with --reuse-values need fsGroup set explicitly. Both are documented below and should go into the release notes.

Main Changes

  • Runtime stage of all four Dockerfiles (build stages unchanged and still identical):
    • create hugegraph with fixed numeric UID/GID 10001 (ARG HG_UID/HG_GID), and a numeric USER, so Kubernetes can verify runAsNonRoot;
    • create the app directory, its data directory (pd_data, storage, rocksdb-data) and logs owned by that user, so new named volumes mounted there start out writable;
    • COPY --chown, and --chmod=755 for the entrypoint instead of a RUN chmod;
    • in the Server images USER comes before the build-time sed -i / mv of conf files, so the rewritten files stay user-owned.
  • .github/scripts/check-docker-images.sh: asserts each image runs as 10001:10001 and owns its app, data and log directories; the jar check copies files out with the runner's own UID, because the image user cannot write the runner's temp dir.
  • Helm: pd.podSecurityContext and store.podSecurityContext default to fsGroup: 10001 with fsGroupChangePolicy: OnRootMismatch, so PVCs, including ones written by older root images, stay writable. runAsNonRoot is not defaulted, so releases pinned to older root image tags keep working.
  • Docs:
    • docker/README.md: new "Container user" section covering bind mounts, mounted conf files, the one-time chown for old volumes, the Compose anonymous-volume caveat (down first or up -d --renew-anon-volumes), Kubernetes fsGroup, and docker exec -u 0; build-arg rows.
    • helm/hugegraph/README.md and docs/operations.md: --reuse-values keeps the old empty podSecurityContext, the default latest + pullPolicy: Always can pull the new image on any Pod restart, explicit --set commands, and OnRootMismatch / NFS / hostPath caveats.
    • PD and Store README docker run examples get a chown step for the bind-mounted data directory; the Helm local-PV example and the Server Docker README are updated.

Follow-ups, not in this PR: readOnlyRootFilesystem (narrowing VOLUME, moving runtime writes out of conf/), OpenShift arbitrary-UID support (chgrp 0 + g=u), the Hubble image (separate repository), docker/hbase/Dockerfile.

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)
  • Already covered by existing tests, such as (please modify tests here).
  • Need tests and can be verified as follows:
    • CI: the new user and ownership assertions in check-docker-images.sh, plus the existing deployment checks, which run against the PR-built images.
    • Locally:
      • built all four runtime stages: Config.User is 10001:10001, the app/data/log directories are owned by 10001, and a fresh named volume at each data path comes out owned by 10001;
      • a standalone Server image from the real 1.8.0 distribution started healthy as UID 10001 with PRELOAD=true on a new named volume;
      • a root-owned volume fails with Permission denied before the documented chown and works after it;
      • the updated check-docker-images.sh assertions pass on all four images, and shellcheck is clean.
    • Not run locally: helm lint / helm unittest (Helm download blocked in the test environment); values.yaml parses and the schema types podSecurityContext as an object.

Does this PR potentially affect the following parts?

  • Dependencies
  • Modify configurations: Helm pd.podSecurityContext / store.podSecurityContext defaults
  • The public API
  • Other affects (typed here): container user changes from root to UID 10001; existing root-owned volumes need a one-time chown / fsGroup
  • Nope

Documentation Status

  • Doc - TODO: required documentation is pending; complete it before merging.
  • Doc - Done: documentation is included here or linked below.
  • Doc - No Need: no user-visible documentation is affected.

Documentation files in this PR or paired hugegraph-doc PR: docker/README.md, helm/hugegraph/README.md, helm/hugegraph/docs/operations.md, helm/hugegraph/examples/README.md, hugegraph-pd/README.md, hugegraph-store/README.md, hugegraph-server/hugegraph-dist/docker/README.md. A paired apache/hugegraph-doc PR is still needed: the website quickstarts (hugegraph-pd.md, hugegraph-hstore.md, hugegraph-server.md) use bind mounts that now need a chown step and an upgrade note.

🤖 Generated with Claude Code

Run the images as hugegraph (UID/GID 10001) and create the data and log
directories owned by it, so new named volumes are writable. Default the
Helm chart's PD and Store fsGroup to 10001 so their volumes, including
ones written by older root images, stay writable. Document the one-time
chown for existing Docker volumes.

Fixes apache#3211

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y1iPNoucoek8X5zrHA8vU
…ages

Give the PD and Store docker run examples a chown step, document that an
existing Helm release keeps an empty podSecurityContext under
--reuse-values, and describe the fsGroupChangePolicy caveats. CI now also
checks the owner of the data and log directories.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y1iPNoucoek8X5zrHA8vU
@byteayan

Copy link
Copy Markdown
Contributor Author

Paired docs PR: apache/hugegraph-doc#544

@byteayan
byteayan marked this pull request as ready for review October 20, 2026 17:33
@codecov

codecov Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.17%. Comparing base (c2ec570) to head (caeaa37).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3381      +/-   ##
============================================
+ Coverage     55.93%   56.17%   +0.23%     
- Complexity     2277     2661     +384     
============================================
  Files           774      776       +2     
  Lines         68534    68865     +331     
  Branches       9333     9414      +81     
============================================
+ Hits          38334    38682     +348     
+ Misses        26634    26622      -12     
+ Partials       3566     3561       -5     
Flag Coverage Δ *Carryforward flag
commons 96.37% <ø> (ø) Carriedforward from b20d05d
hstore 35.97% <ø> (+0.17%) ⬆️
pd 49.44% <ø> (+0.27%) ⬆️
server_hbase 41.84% <ø> (-0.06%) ⬇️ Carriedforward from b20d05d
server_memory 54.58% <ø> (+0.24%) ⬆️
server_rocksdb 42.68% <ø> (+0.34%) ⬆️
store 31.93% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bitflicker64 bitflicker64 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: yes. Summary: The image changes look correct: all four images run as 10001:10001, own their app, data and log directories, and pass the CI ownership checks and the Compose deployment test. The Helm fsGroup default is set in values.yaml only, and the chart's own upgrade command (helm upgrade ... --reuse-values, README "Upgrade and verify") drops it, so PD and Store on PVCs written by the old root images get Permission denied after that upgrade (blocking). Evidence: read the full diff (13 files), the PD/Store/Server entrypoints and start scripts, the PD/Store/Server templates and both compose files; rendered the chart with testdata/values-pre-hardening.yaml as values.yaml (the CI step that stands in for --reuse-values) using Helm 4.2.4, and neither StatefulSet gets a securityContext, while the default render has fsGroup: 10001; a template-level default renders fsGroup: 10001 under the legacy values and keeps an explicit fsGroup=2000. CI at 305b4f5 is green: docker-build ran check-docker-images.sh on all four images and the deployment test (18 tests OK), and helm lint-and-render ran helm unittest. Not run: a Kind install that upgrades a release that ran the root images.

# including one created by an older image that ran as root. A restricted
# namespace also needs securityContext.runAsNonRoot: true, which is safe
# with images from this version only.
podSecurityContext:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important: The fsGroup default lives only in values.yaml, so the chart's own upgrade command drops it. The README's "Upgrade and verify" block runs helm upgrade hugegraph ./helm/hugegraph -n hugegraph --reuse-values --wait --timeout 15m, and --reuse-values replaces the new chart's defaults with the release's old values, where podSecurityContext is {}. The PR says so itself ("--reuse-values keeps the old empty value"), and the CI step "legacy --reuse-values compatibility" exists because of this. After such an upgrade the PD and Store pods start the new image from latest with pullPolicy: Always, without fsGroup, on a PVC the root images wrote. They then fail with Permission denied, and --wait times out. This is the case the new default is meant to cover ("PVCs, including ones written by older root images, stay writable"). Today only an operator who reads the new paragraph and adds four --set flags avoids it.

Evidence: I copied the chart and used testdata/values-pre-hardening.yaml as its values.yaml, which is what the CI step does to stand in for --reuse-values. helm template legacy <copy> --is-upgrade renders no securityContext on either StatefulSet, while the default render has fsGroup: 10001. The chart already puts defaults that must survive value reuse in the template, for example {{ .Values.store.waitTimeoutSeconds | default 900 }} in store-statefulset.yaml.

Requested change: apply the default in pd-statefulset.yaml and store-statefulset.yaml as well, so it also applies under --reuse-values and still respects an explicit value. In my local copy, this renders fsGroup: 10001 under the legacy values and keeps --set pd.podSecurityContext.fsGroup=2000:

      {{- $podSC := deepCopy (.Values.pd.podSecurityContext | default dict) }}
      {{- if not (hasKey $podSC "fsGroup") }}
      {{- $_ := set $podSC "fsGroup" 10001 }}
      {{- $_ := set $podSC "fsGroupChangePolicy" "OnRootMismatch" }}
      {{- end }}
      securityContext:
        {{- toYaml $podSC | nindent 8 }}

If you would rather keep it opt-in, at least have NOTES warn on .Release.IsUpgrade when pd/store.podSecurityContext has no fsGroup, as it already does for the NodePort entry types, and put the four --set flags in the main upgrade command.

The fsGroup default lived only in values.yaml, so an upgrade with
--reuse-values, the chart's own upgrade command, kept a pre-existing
release's empty podSecurityContext. The non-root PD and Store images then
failed with Permission denied on PVCs written by the old root images.
Apply fsGroup 10001 in the templates whenever podSecurityContext sets
none, keep an explicit value, and check it in the legacy --reuse-values
CI step.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y1iPNoucoek8X5zrHA8vU
byteayan pushed a commit to byteayan/hugegraph-doc that referenced this pull request Oct 20, 2026
apache/hugegraph#3381 applies the PD and Store fsGroup in the chart
templates, so an upgrade with --reuse-values no longer needs explicit
--set flags. Drop them from the English and Chinese upgrade notes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y1iPNoucoek8X5zrHA8vU

@bitflicker64 bitflicker64 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: yes. Summary: The images, the CI ownership checks and the template-level fsGroup default (which resolves the earlier --reuse-values comment) look correct. The Helm upgrade guidance tells operators to run the old root images under the new fsGroup/OnRootMismatch before the 10001 images arrive. Files those images create in that window stay unwritable, because the kubelet then skips the recursive ownership change (blocking). Evidence: read the full diff (18 files) at b20d05d. Also read the PD, Store and Server entrypoints and start scripts for writes outside the image-owned directories, the PD/Store/Server templates, the new hugegraph.dataPodSecurityContext helper and its unittest, the compose files' volume mounts, and the doc anchors. Checked requiresPermissionChange/skipPermissionChange in kubernetes pkg/volume/volume_linux.go: OnRootMismatch stats only the volume root. CI is green at this head, including docker-build (check-docker-images.sh on all four images), helm lint-and-render with the new legacy --reuse-values fsGroup assertion, and cluster-test. Not run: a Kind upgrade from root images to the 10001 images.

Comment thread helm/hugegraph/README.md Outdated
an older release's empty value, so that user can write PVCs created by older
images, which ran as root. Because the default tag is `latest` with
`pullPolicy: Always`, an existing release can pull the new image on any Pod
restart: upgrade the chart before that happens, or pin the old image tags until

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important: Both options this paragraph offers leave the PVCs half fixed. "Upgrade the chart before that happens" and "pin the old image tags" each mean the release runs the old root image under the new fsGroup: 10001 with OnRootMismatch for a while. On that first mount the kubelet gives the volume root group 10001 and the setgid bit. The root process then creates new directories and files as root:10001 with the default 022 umask, so 755 and 644. Examples are a new Store partition or graph, raft snapshots and RocksDB files. When the 10001 image arrives, OnRootMismatch checks only the volume root. requiresPermissionChange in kubelet pkg/volume/volume_linux.go stats rootDir alone, finds it matching and skips the recursive change. UID 10001 then cannot create or delete entries in those directories and gets Permission denied. docs/operations.md already describes this case ("files that image created can stay unwritable"), but this paragraph steers the operator into it and links there only after a Pod fails. A fresh install from this chart hits the same case while the published latest images still run as root.

Requested change: tell the operator to switch to the new images in the same rollout that first adds fsGroup, so the kubelet's recursive change runs right before the 10001 image starts. For a release that already ran root images with fsGroup set, roll out the first 10001 image with --set pd.podSecurityContext.fsGroupChangePolicy=Always --set store.podSecurityContext.fsGroupChangePolicy=Always, then go back to OnRootMismatch. The helper keeps an explicit policy and still adds fsGroup, so this also works under --reuse-values. Defaulting the policy to Always would close the gap without operator action, at the cost of a recursive walk on every Pod start.

Running the root images under fsGroup with OnRootMismatch leaves the files
they create unwritable for UID 10001, because the kubelet checks only the
volume root once the 10001 images arrive. Tell operators to switch images
in the same rollout that first applies fsGroup, and to use a one-time
fsGroupChangePolicy=Always rollout when root images already ran with it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y1iPNoucoek8X5zrHA8vU
byteayan pushed a commit to byteayan/hugegraph-doc that referenced this pull request Oct 21, 2026
Mirror apache/hugegraph#3381: switch PD and Store to the 10001 images in
the same rollout that first applies fsGroup, and use a one-time
fsGroupChangePolicy=Always rollout when root images already ran with it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y1iPNoucoek8X5zrHA8vU

@bitflicker64 bitflicker64 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: no. Summary: The latest commit resolves the earlier upgrade-guidance comment: the README and operations.md now say to switch to the 10001 images in the same rollout that first applies fsGroup, and give a one-time fsGroupChangePolicy=Always rollout for releases that already ran root images under fsGroup, backed by a new unittest that keeps an explicit policy. The images, the CI ownership checks and the template-level fsGroup default still look correct. One minor gap remains in the new one-time command. Evidence: read the full diff (18 files) at caeaa37 and the incremental diff from b20d05d. Also re-read the hugegraph.dataPodSecurityContext helper and its unittest, the PD and Store StatefulSets (the Server Deployment mounts no volume), the hugegraph.image helper, the README "Upgrade and verify" command (--reuse-values, no --set), the compose files' named volumes and the Dockerfile WORKDIR/install -d ordering. CI is green at this head, including docker-build (check-docker-images.sh on all four images), helm lint-and-render (helm unittest and the legacy --reuse-values fsGroup assertion) and cluster-test. Not run: a Kind upgrade from root images to the 10001 images.

Comment thread helm/hugegraph/README.md
every Pod start:

```bash
helm upgrade hugegraph ./helm/hugegraph -n hugegraph --reuse-values \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: The one-time recursive rollout can miss its target, and the README does not say how to go back afterwards. Blocking: no. Summary: The command sets only fsGroupChangePolicy=Always, under --reuse-values. That has two effects.

  • It keeps whatever image the release already has. A release that still pins the old root tags then spends the Always rollout on root images, and the later upgrade that brings the 10001 image runs under OnRootMismatch again. The paragraph above asks operators to "Pin pd.image and store.image", so pinned releases are the expected case.
  • "Return to OnRootMismatch on the next upgrade" does not happen by itself. A --set value is saved in the release values, and the main upgrade command in this section (helm upgrade ... --reuse-values --wait --timeout 15m, line 240) keeps it. So Always stays on, and its whole-volume walk runs on every Pod start, until someone sets the policy back.

Evidence: line 245 says --reuse-values preserves existing settings. The new unittest "keeps an explicit fsGroupChangePolicy" shows that the helper keeps an explicit Always, so nothing in the chart resets it.

Requested change: add the image pins to the one-time command, for example --set pd.image.tag=<this version> --set store.image.tag=<this version>. Also show the follow-up upgrade: --reuse-values --set pd.podSecurityContext.fsGroupChangePolicy=OnRootMismatch --set store.podSecurityContext.fsGroupChangePolicy=OnRootMismatch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Run the PD, Store and Server images as a non-root user

3 participants