bitflicker64 commented on code in PR #3119:
URL: https://github.com/apache/hugegraph/pull/3119#discussion_r3663922549
##########
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:
Right, that was a real hole. Fixed in 0dd5a7a1.
The entrypoint no longer decides between running `enable-auth.sh` and doing
nothing. It runs the shipped script for an untouched config, and otherwise
applies only the parts that are missing: the REST keys, the `authentication:`
block in `gremlin-server.yaml`, and the `HugeFactoryAuthProxy` switch in
`hugegraph.properties`. The Gremlin block is pointed at whichever authenticator
the REST config names rather than always at the built-in one. Used in both the
skip and the normal init path, so a mounted already-auth config no longer
duplicates keys there either.
Tests assert all three configs, with exactly one YAML block, for a mount
that carries only `auth.authenticator` and for one that is already fully
enabled.
##########
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:
Agreed, the guard was too broad. Fixed in 0dd5a7a1.
It now resolves the configured class without initializing it and only fires
when it is `StandardAuthenticator` or a subclass. A class that is not on the
init-store classpath is treated as not needing the built-in admin, which covers
the plugin case. Same narrowing in the entrypoint, matched on the class name.
Tests cover an unresolvable custom class passing and a subclass of the
built-in one still being refused.
--
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]
