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


##########
hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java:
##########
@@ -74,6 +74,20 @@ public static void main(String[] args) throws Exception {
         RegisterUtil.registerServer();
 
         HugeConfig restServerConfig = new HugeConfig(restConf);
+
+        // Skip local init only when the flag is *explicitly* false (Helm /
+        // HStore). Unset keeps master behavior: full standalone init-store.
+        // ServerOptions default is false for GraphManager; we do not treat
+        // "missing key" as skip so existing tarball users are not broken.
+        if (shouldSkipLocalInit(restServerConfig)) {
+            LOG.warn("Skipping init-store: '{}' is false in {}. "
+                     + "Unset the property (or set true) to run local "
+                     + "backend/admin init; distributed/Helm sets false.",
+                     ServerOptions.GRAPH_LOAD_FROM_LOCAL_CONFIG.name(),
+                     restConf);
+            return;

Review Comment:
   Final state in `95bfe451`: the disabled path still maps `PASSWORD` to 
`auth.admin_pa`, but only after Java proves that the PD-created account is 
reachable through the selected HStore auth graph. Unusable local combinations 
exit non-zero. The enabled path separately supports persisted no-auth to auth 
transitions, with or without `PASSWORD`.



##########
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:
   Final update in `95bfe451`: `usePD=true` alone is no longer accepted. The 
Java gate resolves `auth.graph_store`, loads its graph configuration, and 
requires `backend=hstore` for local built-in auth. Direct tests cover the 
rejected non-HStore and accepted HStore combinations.



##########
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:
   Final update in `95bfe451`: serialization now covers the complete 
Java-properties surface, not only backslash and leading space. Each UTF-16 code 
unit is emitted as `\\uXXXX`, and round-trip tests use the Java parser for 
controls, embedded newline, metacharacters, and non-ASCII values.



##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -20,23 +20,74 @@ set -euo pipefail
 DOCKER_FOLDER="./docker"
 INIT_FLAG_FILE="init_complete"
 GRAPH_CONF="./conf/graphs/hugegraph.properties"
+REST_SERVER_CONF="./conf/rest-server.properties"
 
 mkdir -p "${DOCKER_FOLDER}"
 
 log() { echo "[hugegraph-server-entrypoint] $*"; }
 
+# Sets a property to exactly one canonical `key=value` line. Existing
+# definitions are matched on any separator a properties file allows (`=`, `:`
+# or whitespace) and collapsed into that single line, because leaving a second
+# definition behind would make the parser expose the key as a list and a scalar
+# read of it would then fail. Comment lines are left alone. Matching is 
literal,
+# so no regex escaping of the key or value is needed.
 set_prop() {
     local key="$1" val="$2" file="$3"
-    local esc_key esc_val
 
+    SET_PROP_KEY="$key" SET_PROP_VAL="$val" awk '
+        BEGIN { key = ENVIRON["SET_PROP_KEY"]; val = ENVIRON["SET_PROP_VAL"] }
+        {
+            line = $0
+            probe = line
+            sub(/^[[:space:]]+/, "", probe)
+            if (index(probe, key) == 1) {
+                rest = substr(probe, length(key) + 1)
+                if (rest ~ /^[[:space:]]*[=:]/ || rest ~ /^[[:space:]]+/) {
+                    if (!done) { print key "=" val; done = 1 }
+                    next
+                }
+            }
+            print line
+        }
+        END { if (!done) print key "=" val }
+    ' "${file}" > "${file}.tmp" && mv "${file}.tmp" "${file}"

Review Comment:
   Final update in `95bfe451`: the secret path now protects the shipped `0644` 
file with `chmod 600` before writing, while the in-place rewrite preserves its 
inode. Complete single-definition read-only mounts are not rewritten at all.



##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -54,19 +105,89 @@ 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}"
+if [[ -n "${HG_SERVER_INIT_STORE_ENABLED:-}" ]]; then
+    # Canonicalize before writing, so the property file only ever holds `true`
+    # or `false` and cannot be read differently by the shell and the server
+    if ! HG_SERVER_INIT_STORE_ENABLED=$(to_bool 
"${HG_SERVER_INIT_STORE_ENABLED}"); then
+        log "ERROR: HG_SERVER_INIT_STORE_ENABLED must be a boolean, got 
'${HG_SERVER_INIT_STORE_ENABLED}'"
+        exit 1
+    fi
+    set_prop "init_store.enabled" "${HG_SERVER_INIT_STORE_ENABLED}" 
"${REST_SERVER_CONF}"
+fi
 
 # ── 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 [[ -n "${INIT_STORE_ENABLED}" ]]; then
+    if ! INIT_STORE_ENABLED=$(to_bool "${INIT_STORE_ENABLED}"); then
+        log "ERROR: init_store.enabled in ${REST_SERVER_CONF} must be a 
boolean," \
+            "got '${INIT_STORE_ENABLED}'"
+        exit 1
+    fi
+fi
+if [[ "${INIT_STORE_ENABLED:-true}" == "false" ]]; then
+    log "init-store disabled, skipping local backend/admin init"
+
+    # With init-store skipped, nothing creates the built-in admin account
+    # unless the server takes the PD metadata path, which it only does when
+    # `usePD=true`. Enabling auth without that combination starts a server
+    # that enforces authentication while no account exists, so refuse it here
+    # rather than fail every request later.
+    AUTH_REQUESTED=""
+    [[ -n "${PASSWORD:-}" ]] && AUTH_REQUESTED=1
+    [[ -n "$(get_prop "auth.authenticator" "${REST_SERVER_CONF}")" ]] && 
AUTH_REQUESTED=1
+    if [[ -n "${AUTH_REQUESTED}" ]]; then
+        USE_PD=$(to_bool "$(get_prop "usePD" "${REST_SERVER_CONF}")" 
2>/dev/null || echo "false")
+        if [[ "${USE_PD}" != "true" ]]; then
+            log "ERROR: auth is enabled and init_store.enabled=false, but 
usePD is not true."
+            log "ERROR: With init-store skipped the admin account is only 
created on the PD"
+            log "ERROR: metadata path, so this combination would start a 
server that nobody"
+            log "ERROR: can authenticate against."
+            log "ERROR: Set usePD=true in ${REST_SERVER_CONF}, or leave 
init-store enabled"
+            log "ERROR: so that it can create the admin account locally."
+            exit 1
+        fi
+    fi
+
+    # 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

Review Comment:
   Final update in `95bfe451`: auth completion is driven by the effective 
authenticator, not only by `PASSWORD`. Mounted configurations are completed 
idempotently across REST, Gremlin, and the graph proxy with or without a 
password, while duplicate scalar definitions are collapsed only when necessary.



##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -54,19 +125,101 @@ 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}"
+if [[ -n "${HG_SERVER_INIT_STORE_ENABLED:-}" ]]; then
+    # Canonicalize before writing, so the property file only ever holds `true`
+    # or `false` and cannot be read differently by the shell and the server
+    if ! HG_SERVER_INIT_STORE_ENABLED=$(to_bool 
"${HG_SERVER_INIT_STORE_ENABLED}"); then
+        log "ERROR: HG_SERVER_INIT_STORE_ENABLED must be a boolean, got 
'${HG_SERVER_INIT_STORE_ENABLED}'"
+        exit 1
+    fi
+    set_prop "init_store.enabled" "${HG_SERVER_INIT_STORE_ENABLED}" 
"${REST_SERVER_CONF}"
+fi
 
 # ── 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 [[ -n "${INIT_STORE_ENABLED}" ]]; then
+    if ! INIT_STORE_ENABLED=$(to_bool "${INIT_STORE_ENABLED}"); then
+        log "ERROR: init_store.enabled in ${REST_SERVER_CONF} must be a 
boolean," \
+            "got '${INIT_STORE_ENABLED}'"
+        exit 1
+    fi
+fi
+if [[ "${INIT_STORE_ENABLED:-true}" == "false" ]]; then
+    log "init-store disabled, skipping local backend/admin init"
+
+    # With init-store skipped, nothing creates the built-in admin account
+    # unless the server takes the PD metadata path, which it only does when
+    # `usePD=true`. Enabling auth without that combination starts a server
+    # that enforces authentication while no account exists, so refuse it here
+    # rather than fail every request later.
+    AUTH_REQUESTED=""
+    [[ -n "${PASSWORD:-}" ]] && AUTH_REQUESTED=1
+    [[ -n "$(get_prop "auth.authenticator" "${REST_SERVER_CONF}")" ]] && 
AUTH_REQUESTED=1
+    # Remote auth delegates to another service and has no local admin to
+    # create, so it is exempt from the requirement below
+    [[ -n "$(get_prop "auth.remote_url" "${REST_SERVER_CONF}")" ]] && 
AUTH_REQUESTED=""
+    if [[ -n "${AUTH_REQUESTED}" ]]; then
+        USE_PD=$(to_bool "$(get_prop "usePD" "${REST_SERVER_CONF}")" 
2>/dev/null || echo "false")
+        if [[ "${USE_PD}" != "true" ]]; then
+            log "ERROR: auth is enabled and init_store.enabled=false, but 
usePD is not true."
+            log "ERROR: With init-store skipped the admin account is only 
created on the PD"
+            log "ERROR: metadata path, so this combination would start a 
server that nobody"
+            log "ERROR: can authenticate against."
+            log "ERROR: Set usePD=true in ${REST_SERVER_CONF}, or leave 
init-store enabled"
+            log "ERROR: so that it can create the admin account locally."
+            exit 1
+        fi
+    fi
+
+    # 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"
+        # enable-auth.sh appends its keys unconditionally on its first run, so
+        # running it against a mounted config that already enables auth would
+        # leave those scalar keys defined twice, which the config parser
+        # rejects. Only run it when auth is not configured yet, then collapse
+        # whatever it appended into single definitions.
+        if [[ -z "$(get_prop "auth.authenticator" "${REST_SERVER_CONF}")" ]]; 
then

Review Comment:
   Final update in `95bfe451`: the same idempotent REST, Gremlin, and proxy 
postconditions are enforced for every configured authenticator, with or without 
`PASSWORD`. A persisted no-auth init also triggers one initialization pass when 
auth is mounted later.



##########
hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java:
##########
@@ -101,6 +138,36 @@ public static void main(String[] args) throws Exception {
         }
     }
 
+    /**
+     * Skipping means init-store does not create the built-in admin account,
+     * and the only other component that creates it is
+     * GraphManager.initAdminUserIfNeeded(), reached from loadMetaFromPD() and
+     * so gated on 'usePD'. Failing here rather than returning zero keeps
+     * tarball and init-job callers, which see only the exit status, from
+     * continuing into a server that enforces authentication with no account
+     * to authenticate against.
+     * <p>
+     * Remote auth is exempt: the auth manager is then an RPC client, and
+     * StandardAuthenticator only bootstraps an admin for a local one.
+     */
+    private static void checkAdminBootstrapReachable(HugeConfig conf,
+                                                     String restConf) {
+        if (conf.get(ServerOptions.AUTHENTICATOR).isEmpty() ||

Review Comment:
   Final update in `95bfe451`: the type-aware check is centralized in Java and 
uses assignability from `StandardAuthenticator`. The entrypoint no longer has a 
competing class decision, so custom auth with `PASSWORD` stays exempt and 
standard subclasses remain protected.



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