bitflicker64 commented on code in PR #3119:
URL: https://github.com/apache/hugegraph/pull/3119#discussion_r3697914185
##########
hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java:
##########
@@ -83,6 +103,74 @@ public static void main(String[] args) throws Exception {
}
}
+ /**
+ * Skipping leaves the built-in admin to
GraphManager.initAdminUserIfNeeded()
+ * on the PD startup path, which writes it to PD metadata. Only an HStore
+ * auth graph reads that metadata back, so every other local built-in-auth
+ * configuration would start a server nobody can log in to. Remote auth and
+ * custom authenticators keep their identities elsewhere and are exempt.
+ */
+ private static void checkAdminBootstrapReachable(HugeConfig conf,
+ String restConf) {
+ if (!requiresLocalBuiltinAdmin(conf)) {
+ return;
+ }
+ if (!conf.get(ServerOptions.USE_PD)) {
Review Comment:
You're right, and I had it backwards. Fixed in 9707feb.
I dropped this clause because nothing in the container writes
`auth.admin_pa`, so it could never pass. Wrong conclusion from the right fact —
unsatisfiable is correct here, since the alternative is handing out
`admin`/`pa`. It now requires an explicit non-empty value, raised through the
same `unreachableAdmin` path so all four refusals share one message.
You were also right that the accepted-HStore test pinned the unsafe case; I
had deleted its `auth.admin_pa` line. It is back, with coverage for absent and
empty.
This does make apache/hugegraph#3132 fail at container start — its wrapper
writes `usePD` and `pd.peers` but not `auth.admin_pa`. Commented there.
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -74,9 +92,22 @@ if [[ ! -f "${DOCKER_FOLDER}/${INIT_FLAG_FILE}" ]]; then
else
log "init hugegraph with auth mode"
./bin/enable-auth.sh
+ # init-store reads the password from stdin, and a disabled one returns
+ # before it gets there, so say plainly that PASSWORD is being dropped
+ case "${INIT_STORE_ENABLED}" in
+ n | f | no | off | false)
+ log "WARN: PASSWORD is ignored while init-store is disabled;" \
+ "the admin is created on the PD startup path from" \
+ "'auth.admin_pa', which defaults to the public value 'pa'"
;;
+ esac
echo "${PASSWORD}" | ./bin/init-store.sh
fi
- touch "${DOCKER_FOLDER}/${INIT_FLAG_FILE}"
+ # A disabled init-store initialized nothing, so recording it as done would
+ # stop a later re-enable from initializing.
+ case "${INIT_STORE_ENABLED}" in
+ n | f | no | off | false) ;;
+ *) touch "${DOCKER_FOLDER}/${INIT_FLAG_FILE}" ;;
Review Comment:
Fixed in 9707feb, taking the "result" reading rather than re-parsing the
property in shell.
`init-store` now writes the marker itself, only after it has initialized.
The path is handed to it, so nothing is written when no caller asks for one and
the tarball path is unchanged; the entrypoint just exports it and the `case` on
the variable is gone. That closes the mounted-config case — no marker, so a
later flip to `true` still initializes.
Regression is in `testGateDecidesWhetherRegistrationRuns`. It lives in that
method because registering backends twice in one JVM trips
`BackendProviderFactory`'s duplicate check. Not covered: a successful enabled
run writing the marker, which needs a live backend.
I went this way because a shell properties reader cannot match the grammar
`HugeConfig` uses. Happy to switch to a Java-side read if you would rather the
entrypoint stayed the owner.
--
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]
