imbajin commented on code in PR #3119:
URL: https://github.com/apache/hugegraph/pull/3119#discussion_r3653808356
##########
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:
⚠️ Shell and Java use different Boolean semantics here. The shell skips only
for exact lowercase `false`, while HugeConfig accepts case-insensitive false
values (and Java-properties syntax also permits separators other than `=`).
With `HG_SERVER_INIT_STORE_ENABLED=FALSE` and `PASSWORD`, the shell pipes the
password to `init-store.sh` and later writes `init_complete`, but Java parses
the option as false and returns before creating the admin. A focused exact-head
test reproduced this with four failed assertions. Please canonicalize or
validate the value consistently across both layers and cover uppercase and
mounted-property variants.
##########
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:
‼️ The claim that server startup creates the admin is not true for the
shipped HStore Docker configurations. `GraphManager` calls
`initAdminUserIfNeeded()` only inside `usePD=true`, but `ServerOptions.USE_PD`
defaults to `false` and the repository's HStore compose files set only the
HStore backend and PD peers. With this skip option plus `PASSWORD`, `InitStore`
no longer creates the admin and normal startup does not take the PD metadata
path, so authentication can start without the requested admin account. Please
either make bootstrap work for every advertised HStore skip configuration or
reject/avoid this mode unless `usePD=true`, and add an integration test that
authenticates with the supplied 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
+# 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:
⚠️ `set_prop()` escapes for `sed`, not for Java-properties serialization. A
valid password containing a backslash, such as `abc\def`, is written with a
single backslash and is read back by the properties parser as `abcdef`; the old
stdin path preserved it. Please serialize the secret using Java-properties
escaping (or avoid storing it in this file) and add a round-trip test
containing backslashes and other properties metacharacters.
##########
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:
⚠️ This gate runs only after `registerBackends()` and `registerPlugins()`.
Plugin registration invokes every discovered `plugin.register()` and propagates
failures, so disabled mode can still fail or trigger plugin side effects before
reaching the documented no-op/exit-0 path. Please register only the server
options first, evaluate this flag, and move backend/plugin registration to the
enabled path; add coverage proving disabled mode bypasses those calls.
--
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]
