imbajin commented on code in PR #3119:
URL: https://github.com/apache/hugegraph/pull/3119#discussion_r3663365715
##########
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:
⚠️ This guard treats every non-empty `auth.authenticator` as requiring
HugeGraph's built-in admin, but `HugeAuthenticator.loadAuthenticator()`
supports arbitrary implementation classes and only the standard local
authenticator relies on this bootstrap. A custom LDAP/OIDC/plugin authenticator
can manage identities externally without `usePD` or `auth.remote_url`, so
`init_store.enabled=false` now rejects an otherwise supported deployment.
Please limit the guard to authentication modes known to require the built-in
admin, or introduce an explicit bootstrap-capability contract, and cover a
custom authenticator.
##########
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:
‼️ Skipping `enable-auth.sh` when `auth.authenticator` already exists
suppresses more than duplicate REST properties. That script also adds the
authentication handler to `gremlin-server.yaml` and switches
`hugegraph.properties` to `HugeFactoryAuthProxy`; the shipped defaults contain
neither. A mounted REST config with `usePD=true`, `PASSWORD`, and only
`auth.authenticator` can therefore log that auth was enabled while Gremlin
remains unauthenticated or outside the auth proxy. Please make the remaining
auth-enablement changes idempotent and always apply them for `PASSWORD`, then
add a test that verifies all three generated configs (or performs an
authenticated startup).
--
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]
