bitflicker64 commented on code in PR #3119:
URL: https://github.com/apache/hugegraph/pull/3119#discussion_r3684818190
##########
hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java:
##########
@@ -41,21 +41,64 @@
public class InitStore {
private static final Logger LOG = Log.logger(InitStore.class);
+ private static final String USE_CONFIGURED_ADMIN_PASSWORD =
+ "--use-configured-admin-password";
public static void main(String[] args) throws Exception {
- E.checkArgument(args.length == 1,
+ E.checkArgument(args.length == 1 ||
+ args.length == 2 &&
+ USE_CONFIGURED_ADMIN_PASSWORD.equals(args[1]),
"HugeGraph init-store need to pass the config file " +
- "of RestServer, like: conf/rest-server.properties");
+ "of RestServer, like: conf/rest-server.properties, " +
+ "with an optional %s flag",
+ USE_CONFIGURED_ADMIN_PASSWORD);
E.checkArgument(args[0].endsWith(".properties"),
"Expect the parameter is properties config file.");
String restConf = args[0];
+ boolean useConfiguredAdminPassword = args.length == 2;
- RegisterUtil.registerBackends();
- RegisterUtil.registerPlugins();
+ /*
+ * Only the server options are needed to read the gate below. Backend
+ * and plugin registration is deferred to the enabled path:
+ * registerPlugins() invokes every discovered plugin's register() and
+ * propagates their failures, which must not happen on a path that is
+ * documented to be a no-op.
+ */
RegisterUtil.registerServer();
HugeConfig restServerConfig = new HugeConfig(restConf);
+
+ /*
+ * Distributed deployments (PD/HStore) let the storage side own the
+ * metadata, 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.
Review Comment:
Valid, fixed in . The comment now distinguishes the full initialization scan
from the disabled-path validation: validation may scan graph configuration
files to prove the PD/HStore auth topology, but it does not open those graphs.
The focused Java suite passes 30/30.
##########
.github/workflows/docker-build-ci.yml:
##########
@@ -26,8 +26,31 @@ on:
paths:
- '**/Dockerfile*'
- '.dockerignore'
+ - 'hugegraph-server/hugegraph-dist/docker/**'
+ -
'hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh'
+ - 'hugegraph-server/hugegraph-dist/src/assembly/static/bin/init-store.sh'
+ - '**/docker/docker-entrypoint.sh'
Review Comment:
Valid, fixed in . The Docker workflow filter now includes , , , , and ,
covering every Java-side dependency of the entrypoint contract. The local
entrypoint suite passes 213/213.
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -19,24 +19,234 @@ set -euo pipefail
DOCKER_FOLDER="./docker"
INIT_FLAG_FILE="init_complete"
+AUTH_INIT_STATE_FILE="auth_init_state"
GRAPH_CONF="./conf/graphs/hugegraph.properties"
+REST_SERVER_CONF="./conf/rest-server.properties"
+GREMLIN_SERVER_CONF="./conf/gremlin-server.yaml"
+
+# The only in-tree HugeAuthenticator that bootstraps HugeGraph's built-in admin
+# account. auth.authenticator accepts any implementation class, and a custom
one
+# (LDAP, OIDC, a plugin) manages its identities elsewhere, so the admin-account
+# requirement below must not be applied to it.
+BUILTIN_AUTHENTICATOR="org.apache.hugegraph.auth.StandardAuthenticator"
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
+ local key="$1" val="$2" file="$3" tmp
+
+ # The scratch file holds auth.admin_pa, so keep it off the process umask
+ # and use an unpredictable same-directory name rather than following a
+ # pre-created predictable symlink.
+ if ! tmp=$(umask 077; mktemp "${file}.tmp.XXXXXX"); then
+ return 1
+ fi
+
+ if ! 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}" > "${tmp}"; then
+ rm -f "${tmp}"
+ return 1
+ fi
+
+ # Truncate and rewrite in place rather than rename: a single-file bind
+ # mount cannot be replaced by rename, and a rename would also discard the
+ # original ownership and mode, which matters where auth.admin_pa is written
+ if ! cat "${tmp}" > "${file}"; then
+ rm -f "${tmp}"
+ return 1
+ fi
+ rm -f "${tmp}"
+}
+
+count_prop() {
+ local key="$1" file="$2"
+
+ [[ -f "${file}" ]] || { echo 0; return; }
+ SET_PROP_KEY="$key" awk '
+ BEGIN { key = ENVIRON["SET_PROP_KEY"] }
+ {
+ probe = $0
+ sub(/^[[:space:]]+/, "", probe)
+ if (index(probe, key) == 1) {
+ rest = substr(probe, length(key) + 1)
+ if (rest ~ /^[[:space:]]*[=:]/ || rest ~ /^[[:space:]]+/) {
+ count++
+ }
+ }
+ }
+ END { print count + 0 }
+ ' "${file}"
+}
+
+# Drops duplicate definitions while leaving a single valid definition
untouched.
+# Avoiding a needless rewrite lets complete read-only mounted configs start.
+canonicalize_prop() {
+ local key="$1" file="$2" count cur
+ count=$(count_prop "${key}" "${file}")
+ if (( count > 1 )); then
+ cur=$(get_prop "${key}" "${file}")
+ set_prop "${key}" "${cur}" "${file}"
+ fi
+}
+# Escapes a UTF-8 value for Java-properties serialization. Encoding every
+# UTF-16 code unit as a Unicode escape keeps separators, leading whitespace,
+# backslashes and embedded control characters out of the physical property
+# line while the Java parser reconstructs the exact original string.
+props_escape() {
+ printf '%s' "$1" | iconv -f UTF-8 -t UTF-16BE | \
+ od -An -v -t x1 | awk '
+ {
+ for (i = 1; i <= NF; i++) {
+ if (high == "") {
+ high = $i
+ } else {
+ printf "\\u%s%s", high, $i
+ high = ""
+ }
+ }
+ }
+ END { if (high != "") exit 1 }
+ '
+}
+
+# Echoes the value of a property, or nothing when the key or the file is
+# absent, so callers apply their own default. Accepts the `=`, `:` and
+# whitespace separators that properties files allow. On duplicate keys the last
+# one wins, matching how the properties parser reads the same file. Only
+# surrounding whitespace is trimmed, as the parser does; whitespace inside a
+# value is part of the value and deleting it would corrupt one.
+get_prop() {
+ local key="$1" file="$2"
+ local esc_key
+
+ [[ -f "${file}" ]] || return 0
esc_key=$(printf '%s' "$key" | sed -e 's/[][(){}.^$*+?|\\/]/\\&/g')
- esc_val=$(printf '%s' "$val" | sed -e 's/[&|\\]/\\&/g')
+ # '#' delimits the s command because the pattern itself contains '|'.
+ # '-E' rather than '-r': both GNU and BSD sed accept it
+ sed -En
"s#^[[:space:]]*${esc_key}([[:space:]]*[=:]|[[:space:]]+)[[:space:]]*(.*)\$#\\2#p"
\
Review Comment:
Valid, fixed in . The physical-line shell parser and rewriter were replaced
by , which uses Commons Configuration, the same grammar as . Regressions cover
escaped keys, continued booleans, duplicate logical definitions,
comments/layout, and exact complex password round trips. Java passes 30/30 and
the entrypoint suite passes 213/213.
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -54,29 +264,134 @@ 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
+
+# A mounted configuration can enable REST authentication without carrying the
+# matching Gremlin handler or auth graph proxy. Complete all three configs for
+# every configured authenticator, whether or not Docker supplied a PASSWORD.
+AUTHENTICATOR=$(get_prop "auth.authenticator" "${REST_SERVER_CONF}")
+if [[ -n "${PASSWORD:-}" || -n "${AUTHENTICATOR}" ]]; then
+ ensure_auth_enabled
+ AUTHENTICATOR=$(get_prop "auth.authenticator" "${REST_SERVER_CONF}")
+fi
+
+AUTH_STATE=""
+AUTH_INIT_REQUIRED=false
+if [[ -n "${AUTHENTICATOR}" ]]; then
+ AUTH_STATE=$(printf '%s\n%s\n%s' \
+ "${AUTHENTICATOR}" \
+ "$(get_prop "auth.remote_url" "${REST_SERVER_CONF}")" \
+ "$(get_prop "auth.graph_store" "${REST_SERVER_CONF}")")
+ STORED_AUTH_STATE=$(cat \
+ "${DOCKER_FOLDER}/${AUTH_INIT_STATE_FILE}" 2>/dev/null || true)
+ if [[ -f "${DOCKER_FOLDER}/${INIT_FLAG_FILE}" &&
+ "${STORED_AUTH_STATE}" != "${AUTH_STATE}" ]]; then
+ AUTH_INIT_REQUIRED=true
+ fi
+fi
+
+if [[ "${INIT_STORE_ENABLED:-true}" == "false" ]]; then
+ log "init-store disabled; validating the no-op configuration"
+
+ # Let InitStore make the type-aware decision about whether this effective
+ # authenticator needs the built-in admin and whether the configured auth
+ # graph can read the PD-created account. The gate returns before backend or
+ # plugin registration, so this invocation performs validation only.
+ ./bin/init-store.sh
+
+ # 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"
+ # TODO: auth.admin_pa only applies when the admin account is first
+ # created, so changing PASSWORD on a later restart keeps the old one.
+ if ! ESCAPED_PASSWORD=$(props_escape "${PASSWORD}"); then
+ log "ERROR: PASSWORD must be valid UTF-8"
+ exit 1
+ fi
+ if ! chmod 600 "${REST_SERVER_CONF}"; then
+ log "ERROR: cannot protect ${REST_SERVER_CONF} before writing
auth.admin_pa"
+ exit 1
+ fi
+ if ! set_prop "auth.admin_pa" "${ESCAPED_PASSWORD}" \
Review Comment:
Valid, fixed in . The entrypoint now asks Java whether the effective
authenticator is a local (including subclasses and excluding ). Only that mode
persists or pipes ; custom and remote auth explicitly log that it is ignored.
Both custom and remote cases assert that is absent.
--
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]
