bitflicker64 commented on code in PR #3119:
URL: https://github.com/apache/hugegraph/pull/3119#discussion_r3655764208


##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -54,19 +68,52 @@ migrate_env "PD_PEERS" "HG_SERVER_PD_PEERS"
 # ── Map env → properties file ─────────────────────────────────────────
 [[ -n "${HG_SERVER_BACKEND:-}"  ]] && set_prop "backend"  
"${HG_SERVER_BACKEND}"  "${GRAPH_CONF}"
 [[ -n "${HG_SERVER_PD_PEERS:-}" ]] && set_prop "pd.peers" 
"${HG_SERVER_PD_PEERS}" "${GRAPH_CONF}"
+[[ -n "${HG_SERVER_INIT_STORE_ENABLED:-}" ]] && set_prop "init_store.enabled" 
"${HG_SERVER_INIT_STORE_ENABLED}" "${REST_SERVER_CONF}"
 
 # ── Build wait-storage env ─────────────────────────────────────────────
 WAIT_ENV=()
 [[ -n "${HG_SERVER_BACKEND:-}"  ]] && 
WAIT_ENV+=("hugegraph.backend=${HG_SERVER_BACKEND}")
 [[ -n "${HG_SERVER_PD_PEERS:-}" ]] && 
WAIT_ENV+=("hugegraph.pd.peers=${HG_SERVER_PD_PEERS}")
 
-# ── Init store (once) ─────────────────────────────────────────────────
-if [[ ! -f "${DOCKER_FOLDER}/${INIT_FLAG_FILE}" ]]; then
+wait_storage() {
     if (( ${#WAIT_ENV[@]} > 0 )); then
         env "${WAIT_ENV[@]}" ./bin/wait-storage.sh
     else
         ./bin/wait-storage.sh
     fi
+}
+
+# ── Init store (once) ─────────────────────────────────────────────────
+# With `init_store.enabled=false` (distributed PD/HStore) init-store is a 
no-op:
+# storage owns the metadata and the admin account is created on server startup
+# from `auth.admin_pa`. A requested PASSWORD is therefore written to that

Review Comment:
   Confirmed, and worse than a mismatch: with `usePD` false nothing creates the 
admin at all. `StandardAuthenticator.initAdminUserIfNeeded` is called only from 
`InitStore`, and `GraphManager.initAdminUserIfNeeded` only from 
`loadMetaFromPD()`, which is gated on `usePD`.
   
   Took the reject option in a62f6ffe. The entrypoint refuses skip + auth 
unless `usePD=true` and exits with the reason; `InitStore` logs the same for 
non-Docker installs. Skip without auth is unchanged. `usePD=true` is already 
the pairing in the cluster-test `rest-server.properties.template`.
   
   The shipped compose files not setting `usePD` is pre-existing and left 
alone, since enabling it also moves graph loading to PD metadata.
   



##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -54,19 +68,52 @@ migrate_env "PD_PEERS" "HG_SERVER_PD_PEERS"
 # ── Map env → properties file ─────────────────────────────────────────
 [[ -n "${HG_SERVER_BACKEND:-}"  ]] && set_prop "backend"  
"${HG_SERVER_BACKEND}"  "${GRAPH_CONF}"
 [[ -n "${HG_SERVER_PD_PEERS:-}" ]] && set_prop "pd.peers" 
"${HG_SERVER_PD_PEERS}" "${GRAPH_CONF}"
+[[ -n "${HG_SERVER_INIT_STORE_ENABLED:-}" ]] && set_prop "init_store.enabled" 
"${HG_SERVER_INIT_STORE_ENABLED}" "${REST_SERVER_CONF}"
 
 # ── Build wait-storage env ─────────────────────────────────────────────
 WAIT_ENV=()
 [[ -n "${HG_SERVER_BACKEND:-}"  ]] && 
WAIT_ENV+=("hugegraph.backend=${HG_SERVER_BACKEND}")
 [[ -n "${HG_SERVER_PD_PEERS:-}" ]] && 
WAIT_ENV+=("hugegraph.pd.peers=${HG_SERVER_PD_PEERS}")
 
-# ── Init store (once) ─────────────────────────────────────────────────
-if [[ ! -f "${DOCKER_FOLDER}/${INIT_FLAG_FILE}" ]]; then
+wait_storage() {
     if (( ${#WAIT_ENV[@]} > 0 )); then
         env "${WAIT_ENV[@]}" ./bin/wait-storage.sh
     else
         ./bin/wait-storage.sh
     fi
+}
+
+# ── Init store (once) ─────────────────────────────────────────────────
+# With `init_store.enabled=false` (distributed PD/HStore) init-store is a 
no-op:
+# storage owns the metadata and the admin account is created on server startup
+# from `auth.admin_pa`. A requested PASSWORD is therefore written to that
+# property rather than piped into init-store.sh, where it would be read and
+# discarded without creating the account.
+#
+# The value is read back from the config file rather than from the env var, so
+# that a rest-server.properties mounted with the property already set behaves
+# the same as `HG_SERVER_INIT_STORE_ENABLED` (the env mapping above has already
+# been applied, so env still wins).
+INIT_STORE_ENABLED=$(get_prop "init_store.enabled" "${REST_SERVER_CONF}")
+if [[ "${INIT_STORE_ENABLED:-true}" == "false" ]]; then

Review Comment:
   Confirmed and fixed in a62f6ffe. `to_bool` now mirrors 
`PropertyConverter.toBoolean` (BooleanUtils: `y/t/on/yes/true`, 
`n/f/no/off/false`, case-insensitive), canonicalizes before writing so the file 
only ever holds `true` or `false`, and exits non-zero on anything else instead 
of diverging silently.
   
   `get_prop` also accepts the `:` and whitespace separators a properties file 
allows, not just `=`. Covered for `FALSE`, `off`, `no`, a non-boolean, and a 
mounted `:` separator.
   



##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -54,19 +68,52 @@ migrate_env "PD_PEERS" "HG_SERVER_PD_PEERS"
 # ── Map env → properties file ─────────────────────────────────────────
 [[ -n "${HG_SERVER_BACKEND:-}"  ]] && set_prop "backend"  
"${HG_SERVER_BACKEND}"  "${GRAPH_CONF}"
 [[ -n "${HG_SERVER_PD_PEERS:-}" ]] && set_prop "pd.peers" 
"${HG_SERVER_PD_PEERS}" "${GRAPH_CONF}"
+[[ -n "${HG_SERVER_INIT_STORE_ENABLED:-}" ]] && set_prop "init_store.enabled" 
"${HG_SERVER_INIT_STORE_ENABLED}" "${REST_SERVER_CONF}"
 
 # ── Build wait-storage env ─────────────────────────────────────────────
 WAIT_ENV=()
 [[ -n "${HG_SERVER_BACKEND:-}"  ]] && 
WAIT_ENV+=("hugegraph.backend=${HG_SERVER_BACKEND}")
 [[ -n "${HG_SERVER_PD_PEERS:-}" ]] && 
WAIT_ENV+=("hugegraph.pd.peers=${HG_SERVER_PD_PEERS}")
 
-# ── Init store (once) ─────────────────────────────────────────────────
-if [[ ! -f "${DOCKER_FOLDER}/${INIT_FLAG_FILE}" ]]; then
+wait_storage() {
     if (( ${#WAIT_ENV[@]} > 0 )); then
         env "${WAIT_ENV[@]}" ./bin/wait-storage.sh
     else
         ./bin/wait-storage.sh
     fi
+}
+
+# ── Init store (once) ─────────────────────────────────────────────────
+# With `init_store.enabled=false` (distributed PD/HStore) init-store is a 
no-op:
+# storage owns the metadata and the admin account is created on server startup
+# from `auth.admin_pa`. A requested PASSWORD is therefore written to that
+# property rather than piped into init-store.sh, where it would be read and
+# discarded without creating the account.
+#
+# The value is read back from the config file rather than from the env var, so
+# that a rest-server.properties mounted with the property already set behaves
+# the same as `HG_SERVER_INIT_STORE_ENABLED` (the env mapping above has already
+# been applied, so env still wins).
+INIT_STORE_ENABLED=$(get_prop "init_store.enabled" "${REST_SERVER_CONF}")
+if [[ "${INIT_STORE_ENABLED:-true}" == "false" ]]; then
+    log "init-store disabled, skipping local backend/admin init"
+    # Still wait: the server needs the storage side reachable at startup even
+    # though nothing is initialized here
+    wait_storage
+
+    if [[ -n "${PASSWORD:-}" ]]; then
+        log "enabling auth mode, admin password applied via auth.admin_pa"
+        ./bin/enable-auth.sh
+        # TODO: auth.admin_pa only applies when the admin account is first
+        # created, so changing PASSWORD on a later restart silently keeps the
+        # old one. It also leaves the password at rest in 
rest-server.properties,
+        # unlike the enabled path where it only travels over stdin.
+        set_prop "auth.admin_pa" "${PASSWORD}" "${REST_SERVER_CONF}"

Review Comment:
   Confirmed and fixed in a62f6ffe. `props_escape` doubles backslashes and 
escapes a leading space before the value reaches `set_prop`, so `abc\def` 
round-trips instead of arriving as `abcdef`. Test asserts the escaped form is 
what lands in the file.
   



##########
hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java:
##########
@@ -74,6 +74,31 @@ public static void main(String[] args) throws Exception {
         RegisterUtil.registerServer();
 
         HugeConfig restServerConfig = new HugeConfig(restConf);
+
+        /*
+         * Distributed deployments (PD/HStore) let the storage side own the
+         * metadata, and create the admin account on server startup from
+         * 'auth.admin_pa', so there is nothing for init-store to do. The
+         * option defaults to true, keeping standalone/tarball installs on the
+         * full init path.
+         *
+         * The loop below already skips hstore backends, so what this gate
+         * additionally avoids is scanning the graphs directory (which must
+         * otherwise exist), and, when auth is configured, opening the auth
+         * graph store in initAdminUserIfNeeded(). On Kubernetes that ran on
+         * every Server pod restart, since the entrypoint's init flag file does
+         * not survive one.
+         */
+        if (!restServerConfig.get(ServerOptions.INIT_STORE_ENABLED)) {

Review Comment:
   Confirmed and fixed in a62f6ffe. Order is now `registerServer()`, build the 
config, evaluate the gate, then `registerBackends()` and `registerPlugins()` on 
the enabled path only.
   
   Verified no default-path change: every key in the shipped 
`rest-server.properties` is declared in `ServerOptions`, which 
`registerServer()` covers. Added a test asserting `rocksdb.data_path` never 
enters `OptionSpace` in disabled mode.
   



-- 
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]

Reply via email to