imbajin commented on code in PR #3119:
URL: https://github.com/apache/hugegraph/pull/3119#discussion_r3698441661
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -68,15 +86,27 @@ if [[ ! -f "${DOCKER_FOLDER}/${INIT_FLAG_FILE}" ]]; then
./bin/wait-storage.sh
fi
+ # init-store writes the marker itself, and only if it initialized. Deciding
+ # here would mean guessing from the environment variable, which says
+ # nothing about a config mounted with the property already set.
+ export HG_SERVER_INIT_COMPLETE_MARKER="${DOCKER_FOLDER}/${INIT_FLAG_FILE}"
Review Comment:
‼️ The new fail-closed validation is never reached when an existing
`docker/init_complete` marker is present. Previous releases wrote that marker
after initialization, and this guard skips `init-store.sh` entirely; after an
upgrade or configuration change to `init_store.enabled=false`, built-in auth
can therefore start without checking `usePD`, the HStore auth graph, or an
explicit `auth.admin_pa`. That can leave the admin unreachable or allow the
public default credential. Please run the disabled-path validation on every
startup regardless of the legacy marker (or version/revalidate the marker), and
add an upgrade regression with an existing marker and invalid built-in-auth
configurations.
##########
hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java:
##########
@@ -51,11 +65,31 @@ public static void main(String[] args) throws Exception {
String restConf = args[0];
- RegisterUtil.registerBackends();
- RegisterUtil.registerPlugins();
+ // Server options alone can answer the gate below. Backend and plugin
+ // registration waits for the enabled path, because registerPlugins()
+ // runs every plugin's register() and propagates its failures.
RegisterUtil.registerServer();
HugeConfig restServerConfig = new HugeConfig(restConf);
+
+ /*
+ * PD/HStore deployments let the storage side own the metadata, so
+ * init-store has nothing to do; on Kubernetes it re-ran on every
Server
+ * pod restart, since the entrypoint's flag file does not survive one.
+ * Skipping also skips creating the built-in admin, which only the PD
+ * startup path can replace, and only for a PD-backed HStore auth
graph.
+ */
+ if (!restServerConfig.get(ServerOptions.INIT_STORE_ENABLED)) {
+ LOG.warn("Skipping init-store: '{}' is false in '{}'. Local " +
+ "backend and admin initialization are not performed.",
+ ServerOptions.INIT_STORE_ENABLED.name(), restConf);
+ checkAdminBootstrapReachable(restServerConfig, restConf);
Review Comment:
‼️ This check only proves that the PD/HStore topology could create the
built-in admin; the actual startup path does not fail closed if creation fails.
`GraphManager.initAdminUserIfNeeded()` catches every exception from
`metaManager.createUser()` and `initDefaultGraphSpace()` and only logs it, so
PD write, permission, or validation failures can still let the server start
without a usable administrator. Please ignore only the explicit already-exists
case and propagate other bootstrap failures, with regressions for both an
existing admin and a non-duplicate creation failure.
--
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]
