fix(hugegraph-dist): gate init-store on a dedicated init_store.enabled option

[bitflicker64]

Fixes #3118

Kept in sync with hugegraph#171

Rescoped. This PR previously carried a second, unrelated change: a
ConfigTool CLI, a rewrite of the entrypoint's auth bootstrap, and a
1,022-line shell test suite for it. That made a ~2,900-line diff covering two
ideas at once. It has been split out to #3133 and follows as its own PR.

Problem

InitStore always performs local initialization: it scans conf/graphs,
initializes non-HStore backends, and bootstraps the built-in admin account.
That is correct for standalone and tarball deployments, but a PD/HStore
deployment already owns its metadata, and its Kubernetes pods cannot rely on
the container-local docker/init_complete flag surviving a restart.

Solution

A dedicated init_store.enabled option, defaulting to true, so existing
standalone and tarball configurations keep the current path unchanged.
Distributed deployments set it to false — or HG_SERVER_INIT_STORE_ENABLED=false
in the server container — to skip backend and plugin registration, the graph
initialization scan, and local admin initialization.

#3118 proposed reusing graph.load_from_local_config. A dedicated option was
used instead: that flag governs whether GraphManager loads graphs from
./conf/graphs, which is a different question from whether init-store should
run, and overloading it would have coupled two unrelated decisions.

---

Scope

In scope

Area	Change
ServerOptions	New init_store.enabled, Boolean, default true
InitStore	Gate before backend/plugin registration; fail-closed check on the disabled path; writes the completion marker when it initializes, and consults it only after the gate
GraphManager	initAdminUserIfNeeded propagates every failure except already-exists, judged by re-reading the user
bin/init-store.sh	Propagates InitStore's exit status
docker-entrypoint.sh	Maps HG_SERVER_INIT_STORE_ENABLED onto the property, validates it, hands the marker path to Java, and runs init-store on every startup
docker/README.md	Documents the variable, the auth requirements, and the marker behaviour
Tests	InitStoreConfigTest, 15 cases; GraphManagerAdminInitTest, 3 cases

Behaviour, by deployment shape

Shape	Before	After
Tarball / standalone, option unset	Full init	Unchanged
Tarball, option explicitly true	Full init	Full init
Docker, no auth, disabled	Full init	Skips; no marker written
Docker, built-in auth, disabled, usePD=true + HStore auth graph + explicit auth.admin_pa	Full init	Skips; server creates the admin from auth.admin_pa on the PD path
Docker, built-in auth, disabled, any of those missing	Full init	Fails closed
Remote auth or custom authenticator, disabled	Full init	Skips; exempt, they own their identities

Deliberately out of scope

• Entrypoint auth bootstrap and ConfigTool — split to [Bug] Docker entrypoint auth bootstrap is unsafe for mounted and upgraded configs #3133. Seven review
  threads on this PR target that code and are answered there.
• Reading the effective property back in the entrypoint. A shell reader
  cannot implement the Java-properties grammar HugeConfig uses. Solved here
  by having init-store own the marker instead; a general config-reading CLI
  belongs to [Bug] Docker entrypoint auth bootstrap is unsafe for mounted and upgraded configs #3133.
• Signalling "skipped" through a distinct exit code.
  run-native-runtime-smoke-test.sh, start-server.sh and
  test-start-hugegraph.sh all treat any non-zero status from
  bin/init-store.sh as fatal.
• Rotating an existing admin password. auth.admin_pa applies only when
  the account is first created.
• graph.load_from_local_config, GraphManager behaviour, Helm job structure,
  shipped Compose usePD defaults.

Downstream

#3132 (Helm chart) injects HG_SERVER_INIT_STORE_ENABLED=false and must land
after this PR. It also has to write auth.admin_pa, which it does not today —
commented there.

---

Fail-closed check

With the gate off and the built-in authenticator selected, the admin is created
on the PD startup path by GraphManager.initAdminUserIfNeeded(conf.get(ADMIN_PA)).
Creation failures there now abort startup; only the already-exists case is
ignored, judged by re-reading the user.
Only an HStore auth graph reads that account back, and ADMIN_PA defaults to
the public value pa. Docker PASSWORD never reaches this path — init-store
reads it from stdin and the disabled path returns first.

So the disabled path refuses unless usePD=true, the auth graph uses HStore,
and auth.admin_pa is explicitly set and non-empty. Remote auth and custom
authenticators are exempt. Refusing is what keeps a deployment from coming up
with a publicly known administrator credential.

bin/init-store.sh propagates the exit status for this to mean anything: its
trailing echo "Initialization finished." previously made the script exit 0
regardless, so a refusal was swallowed and the entrypoint started the server
anyway.

Completion marker

init-store writes docker/init_complete itself, and only after it has
initialized. The entrypoint supplies the path and no longer decides. Deciding
in shell meant reading the environment variable, which says nothing about a
config mounted with init_store.enabled=false already set — that was recorded
as initialized, and setting it back to true later skipped initialization
permanently. Nothing is written when no caller supplies a path, so the tarball
path is unaffected. init-store also reads the marker — after the disabled
gate — so an existing one skips re-initialization but never the fail-closed
check, and the entrypoint runs init-store on every startup to make that hold.

Validation

JDK 11:

• Full unit suite: Tests run: 610, Failures: 0, Errors: 0, Skipped: 1 (the
  skip is pre-existing). Checkstyle clean in all modules.
• bash -n on both changed scripts.
• Exit-status propagation checked through echo "$PASSWORD" | ./bin/init-store.sh
  under set -euo pipefail.
• Env-var validation exercised across false/FALSE/" false"/No/OFF/true/t/YES,
  unset, and the rejected 0/1/disabled. The accepted set is exactly what
  commons-lang 2.x BooleanUtils takes, reached through commons-configuration
  1.x PropertyConverter — note that is not commons-lang3, which would also
  accept 0 and 1.

Not covered: a successful enabled run writing the marker needs a live backend.
No end-to-end container build was run — entrypoint changes are verified by
extracted-script simulation and code reading. shellcheck was not available.

[imbajin]

Blocking: yes. Summary: Disabling local graph loading also bypasses the Docker password-based admin bootstrap, so the requested credential is never installed. Evidence: exact-head static trace through docker-entrypoint.sh, InitStore.java, and GraphManager.java; 3/3 focused tests passed; visible checks are green.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 76 lines in your changes missing coverage. Please review.
✅ Project coverage is 1.70%. Comparing base (f8e7abf) to head (edf07d0).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
.../main/java/org/apache/hugegraph/cmd/InitStore.java	0.00%	61 Missing ⚠️
...n/java/org/apache/hugegraph/core/GraphManager.java	0.00%	12 Missing ⚠️
...ava/org/apache/hugegraph/config/ServerOptions.java	0.00%	3 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (f8e7abf) and HEAD (edf07d0). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (f8e7abf)	HEAD (edf07d0)
	3	2

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #3119       +/-   ##
============================================
- Coverage     39.13%   1.70%   -37.43%     
+ Complexity      264      56      -208     
============================================
  Files           770     748       -22     
  Lines         65772   63335     -2437     
  Branches       8725    8291      -434     
============================================
- Hits          25741    1081    -24660     
- Misses        37272   62162    +24890     
+ Partials       2759      92     -2667     

☔ 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.

[imbajin]

Blocking: yes. Summary: The skip path still fails admin bootstrap for the shipped HStore configuration and has inconsistent option/password handling; backend and plugin registration also occurs before the gate. Evidence: six independent review lanes, exact-head static traces, an uppercase FALSE regression test failing four assertions, and all visible latest-head checks passing.

[bitflicker64]

Unrelated to this change, but noticed while checking the ServerOptions table, so I left a TODO on it rather than silently fixing it here.

The arthas options are declared camelCase (arthas.telnetPort, arthas.httpPort, arthas.disabledCommands) while conf/rest-server.properties ships them snake_case (arthas.telnet_port, arthas.http_port, arthas.disabled_commands). Nothing normalizes between the two, so those keys never match and config.get() returns the declared default. Only arthas.ip works.

It is invisible today because each shipped value equals its default. It bites on any change, e.g. arthas.disabled_commands=jad,exec is dropped and exec stays enabled.

[imbajin]

Blocking: yes. Summary: The mounted-properties override path can create duplicate scalar keys and make init-store or server startup fail. Evidence: six independent review lanes, exact-head static tracing through docker-entrypoint.sh and HugeConfig.java, and all visible latest-head checks passing.

[imbajin]

Blocking: yes. Summary: The disabled init path can still report success for an unusable auth configuration, and two Docker config rewrite paths can fail or corrupt mounted/protected configs. Evidence: six independent exact-head review lanes; entrypoint smoke tests passed 57 assertions with 2 macOS-only skips; git diff --check passed; codecov/patch and codecov/project are failing.

[imbajin]

Blocking: yes. Summary: The skip path can leave Gremlin authentication incomplete and rejects supported custom authenticators. Evidence: six independent exact-head review lanes, static traces through enable-auth.sh and HugeAuthenticator.loadAuthenticator(), 74/74 entrypoint assertions passing, git diff --check passing, and all visible latest-head workflows passing.

[bitflicker64]

Did a self-review pass over the whole diff and pushed 090763a with the follow-ups, no behaviour change beyond the two fixes below:

• get_prop now uses sed -E rather than -r, which only GNU sed documents.
• The gremlin-server.yaml block is no longer appended onto a last line that has no trailing newline.

The rest is text and hygiene: the option description, the InitStore comment and docker/README.md still said every auth.authenticator needs the built-in admin account, which stopped being true when the check was narrowed; InitStoreConfigTest is now imported in suite order.

Since the entrypoint reapplies what bin/enable-auth.sh does when it cannot run the script itself, there is now a test asserting the entrypoint still names every class and key that script does, so the two cannot drift silently. Happy to move that logic into enable-auth.sh instead if you would rather have one source of truth there, it just reaches outside the docker path.

Local run: 106 assertions across 30 entrypoint cases, unit suite 626 tests green on JDK 11, rat and editorconfig clean.

[imbajin]

Blocking: yes. Summary: The skip path still has inconsistent admin-bootstrap decisions across Docker and Java, and several auth configurations can start unusable or incompletely protected. Evidence: six independent exact-head review lanes, static traces through docker-entrypoint.sh, InitStore, GraphManager, and StandardHugeGraph; 106/106 entrypoint assertions and the focused Java test passed; git diff --check passed; visible Actions passed but Codecov statuses failed.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (1)

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/StandardAuthenticator.java:240

• StandardAuthenticator.initAdminUserIfNeeded() only checks whether auth.authenticator is empty, but it doesn't verify that the configured authenticator is actually StandardAuthenticator (or a subclass). If this helper is invoked with a config that sets a custom/local authenticator, it can still open the auth graph and potentially create HugeGraph's built-in admin account even though that authenticator doesn't use it. Add a defensive check so the method no-ops unless auth.authenticator resolves to StandardAuthenticator (or subclass).

        HugeConfig config = new HugeConfig(confFile);
        String authClass = config.get(ServerOptions.AUTHENTICATOR);
        if (authClass.isEmpty()) {
            return;
        }
        config.addProperty(INITING_STORE, true);
        auth.initAdminUser(config, password, fromConfig);

[imbajin]

Blocking: yes. Summary: The current head still has upgrade and authentication-consistency regressions, plus configuration durability and deployment-hardening gaps. Evidence: six independent exact-head review lanes; static traces through docker-entrypoint.sh, ConfigTool, and init-store.sh; 217/217 entrypoint assertions passed; Actions passed while codecov/patch and codecov/project failed.

[imbajin]

Blocking: yes. Summary: The latest head still treats aligned authentication configuration as proof that a legacy administrator exists, so a no-auth volume later configured for authentication can skip bootstrap without a usable admin. Evidence: six independent exact-head lanes; test-docker-entrypoint.sh:419-428 creates only init_complete plus enable-auth.sh configuration and asserts init-store is not run; existing discussion_r3687911099 covers the blocker.

[bitflicker64]

Rescoped this PR.

It had grown to ~2,900 lines doing two unrelated things: the init-store gate #3118 asked for, and a rewrite of the Docker auth bootstrap built on a new ConfigTool. Reviewing those as one diff was not reasonable, so the second half is now #3133 and follows as its own PR.

What is left here is 496 lines across 7 files — the option, the gate, the env mapping, and the init-flag guard.

Your 7 open threads are all on code that moved out. I have replied on each pointing at #3133 instead of resolving them, so they stay findable there.

Two things worth flagging from re-reading it:

• bin/init-store.sh always exited 0, because of the trailing echo "Initialization finished.". So the fail-closed check this PR adds was never observable — the entrypoint started the server regardless. Fixed here, which is what makes the check mean anything.
• PASSWORD does nothing when init-store is skipped. init-store reads it from stdin and the disabled path returns before that, so the admin is created from auth.admin_pa, whose default pa is public. Documented in docker/README.md and warned at runtime; the real fix needs [Bug] Docker entrypoint auth bootstrap is unsafe for mounted and upgraded configs #3133.

The admin-bootstrap check no longer requires auth.admin_pa — nothing in the container writes it, so that clause could never be satisfied and would have broken the distributed path once the exit status started propagating. usePD and the HStore auth-graph checks stay, which is what your original comment was about.

604 tests, 0 failures on JDK 11, checkstyle clean. Description updated to match. Not run: shellcheck (unavailable locally) and no end-to-end container build — the entrypoint changes are verified by extracted-script simulation, not a running container.

Mirrored to hugegraph#171.

[bitflicker64]

Follow-up push (5268091) after another review pass. No behaviour change — 604 tests, 0 failures, checkstyle clean.

• The entrypoint comment said the accepted spellings are "the ones HugeConfig accepts (BooleanUtils)". That is ambiguous, and it misled a bot reviewer into reporting the 0/1 test as a false green. Conversion actually runs through commons-configuration 1.x PropertyConverter → commons-lang 2.6 BooleanUtils, which rejects 0 and 1; commons-lang3 would accept them. The comment now names the path.
• docker/README.md said "With false, the entrypoint deliberately never writes docker/init_complete". True for the environment variable, not for the property set directly in a mounted config. Reworded, with the limitation stated.
• InitStoreConfigTest moved out of the /* core */ group in UnitTestSuite into its own /* cmd */ group, and its comments trimmed toward the density of the surrounding suite.

Two things I would rather raise myself than have you find them:

checkAdminBootstrapReachable may be in the wrong class. StandardAuthenticator.setup() already does the same resolution — scanGraphsDir(GRAPHS), look up auth.graph_store, fail naming both options — and StandardAuthenticator.initAdminUserIfNeeded(String) is the existing static config-path entry point that InitStore already calls. So the check arguably belongs beside that code rather than in InitStore, which as written reaches into auth internals and repeats the auth-graph lookup. I kept it in InitStore because that is where the skip decision is made and where the exit status has to be produced, but I will move it if you would rather.

One branch is effectively unreachable and untested. ConfigUtil.scanGraphsDir throws when the graphs directory is absent, so for usePD=true + built-in auth + no local conf/graphs, the failure surfaces as scanGraphsDir's generic message rather than "auth graph 'x' has no local configuration". The path == null branch only fires when the directory exists but lacks the named graph. I can add a test for that case, or fold the two messages together.

Also for ordering: #3132 (the Helm chart) injects HG_SERVER_INIT_STORE_ENABLED=false and depends on this PR's env mapping, so it should land after this one.

[imbajin]

Blocking: yes. Summary: The disabled-init path can expose a predictable administrator credential and can persist an incorrect initialization marker for mounted configurations. Evidence: exact-head static traces through InitStore, GraphManager, and docker-entrypoint.sh; 12/12 focused tests passed; all visible latest-head workflows passed.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (2)

docker/README.md:178

• The README still says the entrypoint writes docker/init_complete only when HG_SERVER_INIT_STORE_ENABLED=false is set, but the entrypoint no longer writes the marker at all; InitStore writes it only after a successful enabled run. As written, this incorrectly implies that disabling via a mounted rest-server.properties behaves differently from disabling via the env var.

> With the variable set to `false`, the entrypoint deliberately never writes
> `docker/init_complete`, so a later re-enable is still able to initialize.
> Setting the property directly in a mounted `rest-server.properties` does not
> get that guard.

hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:74

• HG_SERVER_INIT_STORE_ENABLED is mapped into rest-server.properties via set_prop, but set_prop only detects existing key= definitions. If a mounted rest-server.properties uses : as the separator (valid for Java properties), this will append a second logical definition, and HugeConfig will load it as a list and fail (as the unit test documents). Consider teaching set_prop to match both = and : separators (and ideally whitespace separators) when deciding whether to replace vs append.

INIT_STORE_ENABLED=$(printf '%s' "${HG_SERVER_INIT_STORE_ENABLED:-}" |
                     tr -d '[:space:]' | tr '[:upper:]' '[:lower:]')
case "${INIT_STORE_ENABLED}" in
    "" | y | t | yes | on | true | n | f | no | off | false) ;;
    *) log "ERROR: invalid HG_SERVER_INIT_STORE_ENABLED" \
           "'${HG_SERVER_INIT_STORE_ENABLED}'"
       exit 1 ;;
esac
[[ -n "${INIT_STORE_ENABLED}" ]] && \
    set_prop "init_store.enabled" "${INIT_STORE_ENABLED}" "${REST_SERVER_CONF}"

[bitflicker64]

Both Copilot points, one taken and one deferred.

README (taken, ff1325a). Correct catch, and it was my own stale text. After the marker moved into init-store, the paragraph still described the entrypoint owning it and still claimed the mounted-property case was unguarded — which is exactly what that change removed. It now says the marker is written by init-store only after it has initialized, so a skipped run records nothing whichever way it was disabled.

set_prop separators (deferred to #3133). The behaviour is real: set_prop detects only key=, so a mounted rest-server.properties using : gets a second logical definition appended and HugeConfig then rejects the file as a list — testDuplicateDefinitionFailsToLoad pins that symptom.

Leaving it here for three reasons. It is pre-existing and not specific to this option: the same set_prop already maps backend and pd.peers, so any fix should cover all three at once. It fails closed and loudly rather than silently, now that init-store propagates its exit status — the container stops instead of starting misconfigured. And teaching shell to match =, : and whitespace separators is the start of a properties parser in sed, which was rejected on this PR earlier for not matching the grammar HugeConfig actually uses.

That is the same reasoning behind moving ConfigTool out, and #3133 covers replacing grep/sed property access with the Commons Configuration path. I have added the separator case there explicitly.

[imbajin]

Blocking: yes. Summary: Existing initialization markers can bypass the new disabled-path safety checks, and PD administrator creation failures are still swallowed. Evidence: six independent exact-head review lanes, static traces through docker-entrypoint.sh, InitStore, and GraphManager; InitStoreConfigTest passed 13/13; git diff --check passed; visible exact-head checks are green.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (3)

hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java:175

• recordInitComplete() treats FileAlreadyExistsException as success unconditionally. If the marker path already exists but is a directory (or other non-regular file), this will silently “record” completion even though future runs (with the tightened check) won’t consider it a valid marker. It’s safer to accept the already-exists case only when the existing entry is a regular file.

        try {
            Files.createFile(path);
        } catch (FileAlreadyExistsException e) {
            // A concurrent container finishing its own successful init has
            // already recorded it, which is the same outcome
        }

hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:74

• The HG_SERVER_INIT_STORE_ENABLED mapping relies on set_prop, which only replaces existing definitions written as key=.... Java .properties also allow key:... and whitespace separators; with a mounted rest-server.properties that uses init_store.enabled: false, this will append a second logical definition instead of replacing, and HugeConfig will then load the key as a list and fail the boolean type check. Consider deleting any existing init_store.enabled line (regardless of separator) before appending the normalized value.

[[ -n "${INIT_STORE_ENABLED}" ]] && \
    set_prop "init_store.enabled" "${INIT_STORE_ENABLED}" "${REST_SERVER_CONF}"

hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java:151

• presentInitCompleteMarker() treats any existing filesystem entry as a completion marker (Files.exists). If the configured path exists but is a directory (or other non-regular file), init-store will incorrectly skip initialization. Consider requiring a regular file via Files.isRegularFile(...) so the marker can’t be accidentally satisfied by a directory.

This issue also appears on line 170 of the same file.

    private static String presentInitCompleteMarker() {
        String marker = configuredInitCompleteMarker();
        if (marker != null && Files.exists(Paths.get(marker))) {
            return marker;
        }
        return null;