diff --git a/docker/README.md b/docker/README.md index 9bc21b1ba7..e55879874c 100644 --- a/docker/README.md +++ b/docker/README.md @@ -160,7 +160,32 @@ Configuration is injected via environment variables. The old `docker/configs/app | `HG_SERVER_BACKEND` | Yes | — | `backend` in `hugegraph.properties` | Storage backend (e.g. `hstore`) | | `HG_SERVER_PD_PEERS` | Yes | — | `pd.peers` | PD cluster addresses (e.g. `pd0:8686,pd1:8686,pd2:8686`) | | `STORE_REST` | No | — | Used by `wait-partition.sh` | Store REST endpoint for partition verification (e.g. `store0:8520`) | -| `PASSWORD` | No | — | Enables auth mode | Optional authentication password | +| `PASSWORD` | No | — | Enables auth mode | Optional authentication password; ignored when `HG_SERVER_INIT_STORE_ENABLED` is `false` (see below) | +| `HG_SERVER_INIT_STORE_ENABLED` | No | `true` | `init_store.enabled` in `rest-server.properties` | Set `false` in PD/HStore deployments so init-store skips local backend and admin initialization | + +> **The built-in authenticator with `HG_SERVER_INIT_STORE_ENABLED=false` +> requires `usePD=true` and an HStore-backed `auth.graph_store`, unless +> `auth.remote_url` delegates auth elsewhere.** With init-store skipped, the +> server creates the built-in admin in PD metadata, and only an HStore auth +> graph uses the PD-backed auth manager that can read that account. init-store +> exits non-zero when the combination is unusable, rather than leaving a server +> nobody can log in to. A custom `auth.authenticator` is exempt because it +> manages its own identities. +> +> `docker/init_complete` is written by init-store itself, and only after it has +> initialized. A skipped run therefore records nothing, whether it was disabled +> by the variable or by the property in a mounted `rest-server.properties`, so a +> later re-enable is still able to initialize. The marker only short-circuits +> re-initialization: init-store runs on every container start, and a disabled +> one performs the fail-closed check above first, so a marker left by an +> earlier release or an earlier enabled run cannot bypass it. +> +> **`PASSWORD` does not reach that path.** init-store reads it from standard +> input, and a disabled one returns before doing so. The admin is instead +> created from `auth.admin_pa`, whose `pa` default is public, so init-store +> refuses to skip unless it is explicitly set to a non-empty value in a mounted +> `rest-server.properties`. It applies only when the account is first created, +> so changing it later does not rotate an existing password. **Deprecated aliases** (still work but log a warning): diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java index 7d706de8f5..4f59a79b51 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java @@ -368,6 +368,17 @@ public class ServerOptions extends OptionHolder { "./conf/graphs" ); + public static final ConfigOption INIT_STORE_ENABLED = + new ConfigOption<>( + "init_store.enabled", + "Whether init-store initializes the local backend stores " + + "and the built-in admin account. Set false in distributed " + + "deployments (PD/HStore) where the storage side already " + + "owns the metadata.", + disallowEmpty(), + true + ); + public static final ConfigOption SERVER_START_IGNORE_SINGLE_GRAPH_ERROR = new ConfigOption<>( "server.start_ignore_single_graph_error", diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index f716285c67..4e02d1f955 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -368,6 +368,13 @@ private void loadMetaFromPD() { this.listenMetaChanges(); } + /** + * Creates the built-in admin account in PD metadata. With init-store + * disabled this is the only bootstrap that admin gets, and init-store's + * fail-closed check assumes it works, so only the already-exists case is + * benign; any other failure aborts startup instead of leaving the server + * without a usable administrator. + */ public void initAdminUserIfNeeded(String password) { HugeUser user = new HugeUser("admin"); user.nickname("超级管理员"); @@ -380,10 +387,29 @@ public void initAdminUserIfNeeded(String password) { user.create(new Date()); user.avatar("/image.png"); try { - this.metaManager.createUser(user); + try { + this.metaManager.createUser(user); + } catch (Exception e) { + // Judged by re-reading rather than by matching the message: + // benign only if the admin actually exists, from an earlier + // startup or from a concurrent server that won the race + HugeUser existing; + try { + existing = this.metaManager.findUser(user.name()); + } catch (Exception probe) { + e.addSuppressed(probe); + throw e; + } + if (existing == null) { + throw e; + } + LOG.info("The built-in admin user already exists, " + + "skip creating it"); + } this.metaManager.initDefaultGraphSpace(); } catch (Exception e) { - LOG.info(e.getMessage()); + throw new HugeException("Failed to init the built-in admin " + + "user or the default graph space", e); } } diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh index 2c76a54435..0862a22f49 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh @@ -20,6 +20,7 @@ set -euo pipefail DOCKER_FOLDER="./docker" INIT_FLAG_FILE="init_complete" GRAPH_CONF="./conf/graphs/hugegraph.properties" +REST_SERVER_CONF="./conf/rest-server.properties" mkdir -p "${DOCKER_FOLDER}" @@ -55,13 +56,39 @@ migrate_env "PD_PEERS" "HG_SERVER_PD_PEERS" [[ -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}" +# Normalized once here and reused by the init-flag guard below. The accepted +# spellings are the ones HugeConfig accepts, case-insensitive: commons-lang 2.x +# BooleanUtils, reached through commons-configuration 1.x PropertyConverter. +# That set excludes 0 and 1, which commons-lang3 would have taken. Anything +# outside it is rejected now rather than touching the init flag for a value the +# server is going to refuse anyway. +INIT_STORE_ENABLED=$(printf '%s' "${HG_SERVER_INIT_STORE_ENABLED:-}" | + tr -d '[:space:]' | tr '[:upper:]' '[:lower:]') +case "${INIT_STORE_ENABLED}" in + "" | y | t | yes | on | true | n | f | no | off | false) ;; + *) log "ERROR: invalid HG_SERVER_INIT_STORE_ENABLED" \ + "'${HG_SERVER_INIT_STORE_ENABLED}'" + exit 1 ;; +esac +[[ -n "${INIT_STORE_ENABLED}" ]] && \ + set_prop "init_store.enabled" "${INIT_STORE_ENABLED}" "${REST_SERVER_CONF}" + # ── 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 +# ── Init store ──────────────────────────────────────────────────────── +# init-store owns the marker: it skips re-initialization when the marker is +# present and writes it only after it has actually initialized. Deciding here +# would mean guessing from the environment variable, which says nothing about +# a config mounted with the property already set. Absolute, so the in-Java +# existence check agrees with the guard below no matter where init-store.sh +# leaves its working directory. +INIT_MARKER_PATH="$(cd "${DOCKER_FOLDER}" && pwd)/${INIT_FLAG_FILE}" +export HG_SERVER_INIT_COMPLETE_MARKER="${INIT_MARKER_PATH}" + +if [[ ! -f "${INIT_MARKER_PATH}" ]]; then if (( ${#WAIT_ENV[@]} > 0 )); then env "${WAIT_ENV[@]}" ./bin/wait-storage.sh else @@ -74,11 +101,25 @@ 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}" else - log "HugeGraph initialization already done. Skipping re-init..." + log "HugeGraph initialization already done. Revalidating the config..." + # The marker skips re-initialization inside init-store, not init-store + # itself: a disabled one must pass its fail-closed check on every startup, + # because the marker may predate this configuration or this release and + # says nothing about whether the admin the current config relies on is + # reachable. An enabled one returns at the marker, before it touches the + # backend or reads stdin, so neither wait-storage nor PASSWORD is needed. + ./bin/init-store.sh fi ./bin/start-hugegraph.sh -j "${JAVA_OPTS:-}" -t 120 diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/init-store.sh b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/init-store.sh index b9da84166b..74ec0bb731 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/bin/init-store.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/bin/init-store.sh @@ -56,5 +56,9 @@ CP="$CP":$(find -L "${LIB}" -name '*.jar' \! -name 'hugegraph*' | sort | tr '\n' CP="$CP":$(find -L "${PLUGINS}" -name '*.jar' | sort | tr '\n' ':') $JAVA -cp $CP ${DEFAULT_JAVA_OPTIONS} \ org.apache.hugegraph.cmd.InitStore "${CONF}"/rest-server.properties +INIT_STORE_STATUS=$? +if [[ ${INIT_STORE_STATUS} -ne 0 ]]; then + exit "${INIT_STORE_STATUS}" +fi echo "Initialization finished." diff --git a/hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java b/hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java index 4391edb438..f878a0c073 100644 --- a/hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java +++ b/hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/cmd/InitStore.java @@ -17,6 +17,11 @@ package org.apache.hugegraph.cmd; +import java.io.IOException; +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -42,6 +47,19 @@ public class InitStore { private static final Logger LOG = Log.logger(InitStore.class); + /** + * Where to record that initialization actually happened. The caller that + * wants the record supplies the path; nothing is read or written when it + * is unset, so tarball callers are unaffected. A present marker skips + * re-initialization only: the disabled path's fail-closed check runs + * before it is consulted, since a marker left by an earlier release or an + * earlier enabled run says nothing about the current configuration. + */ + public static final String INIT_COMPLETE_MARKER = + "hugegraph.init_complete_marker"; + private static final String INIT_COMPLETE_MARKER_ENV = + "HG_SERVER_INIT_COMPLETE_MARKER"; + public static void main(String[] args) throws Exception { E.checkArgument(args.length == 1, "HugeGraph init-store need to pass the config file " + @@ -51,11 +69,39 @@ 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); + return; + } + + String initedMarker = presentInitCompleteMarker(); + if (initedMarker != null) { + LOG.info("Skipping init-store: completion marker '{}' is " + + "present, so this deployment is already initialized", + initedMarker); + return; + } + + RegisterUtil.registerBackends(); + RegisterUtil.registerPlugins(); + PDAuthConfig.setAuthority( ServiceConstant.SERVICE_NAME, ServiceConstant.AUTHORITY); @@ -81,6 +127,134 @@ public static void main(String[] args) throws Exception { } HugeFactory.shutdown(30L, true); } + + recordInitComplete(); + } + + private static String configuredInitCompleteMarker() { + String marker = System.getProperty(INIT_COMPLETE_MARKER, + System.getenv(INIT_COMPLETE_MARKER_ENV)); + return marker == null || marker.isEmpty() ? null : marker; + } + + /** + * The configured marker path, or null when none is configured or the + * file does not exist yet. Consulted only after the disabled-path check, + * so an existing marker can never bypass the fail-closed validation. + */ + private static String presentInitCompleteMarker() { + String marker = configuredInitCompleteMarker(); + if (marker != null && Files.exists(Paths.get(marker))) { + return marker; + } + return null; + } + + /** + * Only this process knows whether it initialized anything. The Docker + * entrypoint used to decide from its environment variable alone, so a + * mounted config that disabled init-store was still recorded as done and a + * later re-enable skipped for good. Reached only on the enabled path, and + * only after initialization succeeded. + */ + private static void recordInitComplete() throws IOException { + String marker = configuredInitCompleteMarker(); + if (marker == null) { + return; + } + Path path = Paths.get(marker); + Path dir = path.toAbsolutePath().getParent(); + if (dir != null) { + Files.createDirectories(dir); + } + try { + Files.createFile(path); + } catch (FileAlreadyExistsException e) { + // A concurrent container finishing its own successful init has + // already recorded it, which is the same outcome + } + LOG.info("Recorded init-store completion at '{}'", path); + } + + /** + * 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)) { + throw unreachableAdmin(restConf, ServerOptions.USE_PD.name() + + " is false"); + } + + String name = conf.get(ServerOptions.AUTH_GRAPH_STORE); + String path = ConfigUtil.scanGraphsDir( + conf.get(ServerOptions.GRAPHS)).get(name); + if (path == null) { + throw unreachableAdmin(restConf, "auth graph '" + name + + "' has no local configuration"); + } + String backend = new HugeConfig(path).get(CoreOptions.BACKEND); + if (!"hstore".equals(backend)) { + throw unreachableAdmin(restConf, "auth graph '" + name + + "' uses backend '" + backend + + "', not 'hstore'"); + } + + // The server creates the admin from this value and cannot prompt for + // it, and Docker PASSWORD never reaches this path. An absent or empty + // one would hand out the public 'pa' default, so fail instead. Checked + // with containsKey because the default is not a configured secret. + if (!conf.containsKey(ServerOptions.ADMIN_PA.name()) || + conf.get(ServerOptions.ADMIN_PA).isEmpty()) { + throw unreachableAdmin(restConf, "no explicit non-empty '" + + ServerOptions.ADMIN_PA.name() + + "' is configured, so the admin " + + "would be created with the " + + "public default"); + } + } + + private static IllegalStateException unreachableAdmin(String restConf, + String reason) { + return new IllegalStateException(String.format( + "Refusing to skip init-store: '%s' configures the built-in " + + "authenticator but %s, so the admin created on the PD startup " + + "path would be unreachable. See docker/README.md.", + restConf, reason)); + } + + /** + * HugeAuthenticator.loadAuthenticator() accepts any implementation class, + * and only StandardAuthenticator bootstraps HugeGraph's built-in admin + * account. A custom one (LDAP, OIDC, a plugin) manages its identities + * elsewhere, so it must not be held to the requirement above. The class is + * resolved without initializing it, and one that is not on the init-store + * classpath is by definition not the built-in authenticator. + */ + private static boolean requiresLocalBuiltinAdmin(HugeConfig conf) { + if (!conf.get(ServerOptions.AUTH_REMOTE_URL).isEmpty()) { + return false; + } + String authClass = conf.get(ServerOptions.AUTHENTICATOR); + if (authClass.isEmpty()) { + return false; + } + try { + Class clazz = Class.forName(authClass, false, + InitStore.class.getClassLoader()); + return StandardAuthenticator.class.isAssignableFrom(clazz); + } catch (ClassNotFoundException | LinkageError e) { + LOG.info("Authenticator '{}' is not on the init-store classpath, " + + "so it is not the built-in one", authClass); + return false; + } } private static HugeGraph initGraph(String configPath) throws Exception { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java index 1d3dd58a83..1733680e3f 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java @@ -37,6 +37,7 @@ import org.apache.hugegraph.unit.cache.CachedGraphTransactionTest; import org.apache.hugegraph.unit.cache.CachedSchemaTransactionTest; import org.apache.hugegraph.unit.cache.RamTableTest; +import org.apache.hugegraph.unit.cmd.InitStoreConfigTest; import org.apache.hugegraph.unit.core.AnalyzerTest; import org.apache.hugegraph.unit.core.BackendMutationTest; import org.apache.hugegraph.unit.core.BackendStoreInfoTest; @@ -45,6 +46,7 @@ import org.apache.hugegraph.unit.core.DataTypeTest; import org.apache.hugegraph.unit.core.DirectionsTest; import org.apache.hugegraph.unit.core.ExceptionTest; +import org.apache.hugegraph.unit.core.GraphManagerAdminInitTest; import org.apache.hugegraph.unit.core.GraphManagerConfigTest; import org.apache.hugegraph.unit.core.LocksTableTest; import org.apache.hugegraph.unit.core.PageStateTest; @@ -143,6 +145,7 @@ SecurityManagerTest.class, RolePermissionTest.class, ExceptionTest.class, + GraphManagerAdminInitTest.class, GraphManagerConfigTest.class, BackendStoreInfoTest.class, TraversalUtilTest.class, @@ -155,6 +158,9 @@ HugeGraphAuthProxyTest.class, SchemaElementTest.class, + /* cmd */ + InitStoreConfigTest.class, + /* serializer */ BytesBufferTest.class, SerializerFactoryTest.class, diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cmd/InitStoreConfigTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cmd/InitStoreConfigTest.java new file mode 100644 index 0000000000..959f8ef140 --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/cmd/InitStoreConfigTest.java @@ -0,0 +1,427 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.unit.cmd; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.List; +import java.util.stream.Stream; + +import org.apache.commons.configuration2.PropertiesConfiguration; +import org.apache.hugegraph.auth.StandardAuthenticator; +import org.apache.hugegraph.cmd.InitStore; +import org.apache.hugegraph.config.ConfigException; +import org.apache.hugegraph.config.CoreOptions; +import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.config.OptionSpace; +import org.apache.hugegraph.config.ServerOptions; +import org.apache.hugegraph.dist.RegisterUtil; +import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.util.ConfigUtil; +import org.junit.After; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * {@code init_store.enabled} controls whether init-store performs local + * backend and admin initialization. It defaults to true, so standalone and + * tarball installs keep the full init path; distributed (PD/HStore) + * deployments set it to false. + */ +public class InitStoreConfigTest { + + private static final String MISSING_GRAPHS_DIR = "no-such-graphs-dir"; + // Registered by RegisterUtil.registerBackends(), not by registerServer() + private static final String ROCKSDB_OPTION = "rocksdb.data_path"; + + private Path workDir; + private int confSeq; + + @BeforeClass + public static void registerOptions() { + RegisterUtil.registerServer(); + } + + @Before + public void setup() throws IOException { + this.workDir = Files.createTempDirectory("init-store-config-test"); + } + + @After + public void teardown() throws IOException { + try (Stream paths = Files.walk(this.workDir)) { + for (Path path : paths.sorted(Comparator.reverseOrder()) + .toArray(Path[]::new)) { + Files.deleteIfExists(path); + } + } + } + + @Test + public void testInitStoreEnabledByDefault() { + HugeConfig config = new HugeConfig(new PropertiesConfiguration()); + Assert.assertTrue(config.get(ServerOptions.INIT_STORE_ENABLED)); + } + + /** + * Pins the spellings docker-entrypoint.sh may pass through. The shell side + * is not executed here, so keep the two lists in step by hand. + */ + @Test + public void testBooleanSpellingsAcceptedByHugeConfig() { + for (String value : new String[]{"true", "TRUE", "t", "on", "y", "yes"}) { + Assert.assertTrue(value, initStoreEnabled(value)); + } + for (String value : new String[]{"false", "FALSE", "f", "off", "n", "no"}) { + Assert.assertFalse(value, initStoreEnabled(value)); + } + } + + /** + * Conversion runs at load time via commons-configuration 1.x + * PropertyConverter, which delegates to commons-lang 2.x BooleanUtils — + * so "0" and "1" are refused here even though commons-lang3 accepts them. + */ + @Test + public void testUnparseableBooleanFailsToLoad() { + for (String value : new String[]{"0", "1", "disabled"}) { + Assert.assertThrows(ConfigException.class, () -> { + initStoreEnabled(value); + }); + } + } + + private static boolean initStoreEnabled(String value) { + PropertiesConfiguration properties = new PropertiesConfiguration(); + properties.setProperty(ServerOptions.INIT_STORE_ENABLED.name(), value); + return new HugeConfig(properties).get(ServerOptions.INIT_STORE_ENABLED); + } + + /** + * A key defined twice loads as a list and fails the scalar type check, so + * a caller mapping an env var onto it must replace, not append. + */ + @Test + public void testDuplicateDefinitionFailsToLoad() throws IOException { + Path conf = this.workDir.resolve("duplicate.properties"); + String key = ServerOptions.INIT_STORE_ENABLED.name(); + Files.write(conf, Arrays.asList(key + "=false", key + "=true"), + StandardCharsets.UTF_8); + + Assert.assertThrows(IllegalArgumentException.class, () -> { + new HugeConfig(conf.toString()); + }, e -> Assert.assertContains("[false, true]", e.getMessage())); + } + + /** + * Establishes that graph scanning fails for these configs, which is what + * keeps {@link #testDisabledInitStoreExitsBeforeGraphInit()} honest. + */ + @Test + public void testMissingGraphsDirFailsScan() { + String graphsDir = this.workDir.resolve(MISSING_GRAPHS_DIR).toString(); + Assert.assertThrows(IllegalArgumentException.class, () -> { + ConfigUtil.scanGraphsDir(graphsDir); + }); + } + + @Test + public void testDisabledInitStoreExitsBeforeGraphInit() throws Exception { + String restConf = this.writeDisabledRestServerConf(); + // Completes instead of failing on the missing graphs directory, which + // proves the gate is applied before any graph or admin initialization + InitStore.main(new String[]{restConf}); + } + + /** + * Disabled mode must not reach registerBackends() or registerPlugins(), + * since plugin registration propagates every plugin's failures. Backend + * options reach OptionSpace only through registerBackends(), so that state + * shows whether it ran; OptionSpace is process-wide, hence unchanged rather + * than empty. The enabled run shares this method so it provably follows the + * disabled assertions, which registering backends first would make vacuous. + * It leaves backend providers registered for the rest of the suite's JVM, + * and BackendProviderFactory.register() rejects duplicates, so this has to + * stay the only enabled run in the class. + *

+ * The completion marker is asserted here for the same reason. A disabled + * run must not write it: the entrypoint treats it as "already initialized", + * so recording one for a config mounted with the property set to false + * would skip the real initialization for good on a later re-enable. Only + * a successful enabled run may write it, which needs a live backend and so + * is not covered here. + */ + @Test + public void testGateDecidesWhetherRegistrationRuns() throws Exception { + boolean backendsRegistered = OptionSpace.containKey(ROCKSDB_OPTION); + Path marker = this.workDir.resolve("docker/init_complete"); + System.setProperty(InitStore.INIT_COMPLETE_MARKER, marker.toString()); + + try { + InitStore.main(new String[]{this.writeDisabledRestServerConf()}); + + Assert.assertEquals(backendsRegistered, + OptionSpace.containKey(ROCKSDB_OPTION)); + // Server options stay registered, the gate is read from them + Assert.assertTrue(OptionSpace.containKey( + ServerOptions.INIT_STORE_ENABLED.name())); + Assert.assertFalse("a disabled run initialized nothing", + Files.exists(marker)); + + // Absent gate means enabled, so the same config now has to reach + // the graph scan and fail on the directory the disabled run never + // looked at — a re-enable is not short-circuited by the marker + Assert.assertThrows(IllegalArgumentException.class, () -> { + InitStore.main(new String[]{this.writeEnabledRestServerConf()}); + }); + Assert.assertTrue(OptionSpace.containKey(ROCKSDB_OPTION)); + Assert.assertFalse("a failed run initialized nothing", + Files.exists(marker)); + } finally { + System.clearProperty(InitStore.INIT_COMPLETE_MARKER); + } + } + + /** + * The upgrade scenario: earlier releases wrote the completion marker + * from the entrypoint, so one can exist for a configuration that was + * never validated — including a later switch to the disabled gate. The + * entrypoint half of the regression (skipping init-store entirely on a + * present marker) is shell and out of a unit test's reach; what is + * pinned here is the ordering invariant the fix relies on instead: the + * fail-closed check runs before the marker is consulted, so it fires + * with the marker present exactly as {@code + * testDisabledInitStoreFailsWhenAdminCannotBeCreated} shows without it. + */ + @Test + public void testExistingMarkerDoesNotBypassDisabledPathCheck() + throws IOException { + System.setProperty(InitStore.INIT_COMPLETE_MARKER, + this.writeExistingMarker().toString()); + try { + String restConf = this.writeDisabledRestServerConf( + ServerOptions.AUTHENTICATOR.name() + + "=" + StandardAuthenticator.class.getName()); + + Assert.assertThrows(IllegalStateException.class, () -> { + InitStore.main(new String[]{restConf}); + }, e -> Assert.assertContains("Refusing to skip init-store", + e.getMessage())); + } finally { + System.clearProperty(InitStore.INIT_COMPLETE_MARKER); + } + } + + /** + * The enabled path with a present marker is the plain Docker restart: it + * must return before the graph scan — completing on a config whose graphs + * directory is missing proves that — and before backend registration, + * which {@link #testGateDecidesWhetherRegistrationRuns} relies on being + * run at most once per JVM. + */ + @Test + public void testExistingMarkerSkipsReinitializationWhenEnabled() + throws Exception { + System.setProperty(InitStore.INIT_COMPLETE_MARKER, + this.writeExistingMarker().toString()); + try { + InitStore.main(new String[]{this.writeEnabledRestServerConf()}); + } finally { + System.clearProperty(InitStore.INIT_COMPLETE_MARKER); + } + } + + private Path writeExistingMarker() throws IOException { + Path marker = this.workDir.resolve("docker/init_complete"); + Files.createDirectories(marker.getParent()); + Files.createFile(marker); + return marker; + } + + /** + * The CLI must not report success for a configuration that would start an + * auth-enabled server with no admin account, since tarball and init-job + * callers only see the exit status. + */ + @Test + public void testDisabledInitStoreFailsWhenAdminCannotBeCreated() + throws IOException { + String restConf = this.writeDisabledRestServerConf( + ServerOptions.AUTHENTICATOR.name() + + "=" + StandardAuthenticator.class.getName()); + + Assert.assertThrows(IllegalStateException.class, () -> { + InitStore.main(new String[]{restConf}); + }, e -> Assert.assertContains("Refusing to skip init-store", + e.getMessage())); + } + + @Test + public void testDisabledInitStoreFailsWithNonHstoreAuthGraph() + throws IOException { + Path graphsDir = this.writeAuthGraphConfig("memory"); + String restConf = this.writeDisabledRestServerConfForGraphs( + graphsDir, + ServerOptions.AUTHENTICATOR.name() + + "=" + StandardAuthenticator.class.getName(), + ServerOptions.AUTH_GRAPH_STORE.name() + "=hugegraph", + ServerOptions.USE_PD.name() + "=true"); + + Assert.assertThrows(IllegalStateException.class, () -> { + InitStore.main(new String[]{restConf}); + }, e -> Assert.assertContains("uses backend 'memory', not 'hstore'", + e.getMessage())); + } + + @Test + public void testDisabledInitStoreAllowsPdBackedHstoreAuthGraph() + throws Exception { + Path graphsDir = this.writeAuthGraphConfig("hstore"); + String restConf = this.writeDisabledRestServerConfForGraphs( + graphsDir, + ServerOptions.AUTHENTICATOR.name() + + "=" + StandardAuthenticator.class.getName(), + ServerOptions.AUTH_GRAPH_STORE.name() + "=hugegraph", + ServerOptions.USE_PD.name() + "=true", + ServerOptions.ADMIN_PA.name() + "=secret"); + + InitStore.main(new String[]{restConf}); + } + + /** + * The server creates the admin from auth.admin_pa without prompting, and + * Docker PASSWORD never reaches this path, so an absent or empty value + * would publish the well-known 'pa' default as a working credential. + */ + @Test + public void testDisabledInitStoreRejectsDefaultAdminPassword() + throws IOException { + Path graphsDir = this.writeAuthGraphConfig("hstore"); + for (String adminPa : new String[]{null, ""}) { + List extra = new ArrayList<>(Arrays.asList( + ServerOptions.AUTHENTICATOR.name() + + "=" + StandardAuthenticator.class.getName(), + ServerOptions.AUTH_GRAPH_STORE.name() + "=hugegraph", + ServerOptions.USE_PD.name() + "=true")); + if (adminPa != null) { + extra.add(ServerOptions.ADMIN_PA.name() + "=" + adminPa); + } + String restConf = this.writeDisabledRestServerConfForGraphs( + graphsDir, extra.toArray(new String[0])); + + Assert.assertThrows(IllegalStateException.class, () -> { + InitStore.main(new String[]{restConf}); + }, e -> Assert.assertContains("public default", e.getMessage())); + } + } + + /** + * Remote auth delegates to another service, so there is no local admin to + * create and the check above must not fire. + */ + @Test + public void testDisabledInitStoreAllowsRemoteAuth() throws Exception { + String restConf = this.writeDisabledRestServerConf( + ServerOptions.AUTHENTICATOR.name() + + "=" + StandardAuthenticator.class.getName(), + ServerOptions.AUTH_REMOTE_URL.name() + "=127.0.0.1:8899"); + + InitStore.main(new String[]{restConf}); + } + + /** + * auth.authenticator takes any implementation class, and only the built-in + * one bootstraps HugeGraph's admin account. A custom authenticator keeps + * its identities elsewhere, so the check must not reject it; a subclass of + * the built-in one still relies on the same bootstrap. + */ + @Test + public void testDisabledInitStoreAllowsCustomAuthenticator() + throws Exception { + String restConf = this.writeDisabledRestServerConf( + ServerOptions.AUTHENTICATOR.name() + + "=org.example.auth.LdapAuthenticator"); + + InitStore.main(new String[]{restConf}); + + String subclassConf = this.writeDisabledRestServerConf( + ServerOptions.AUTHENTICATOR.name() + "=" + + DerivedAuthenticator.class.getName()); + + Assert.assertThrows(IllegalStateException.class, () -> { + InitStore.main(new String[]{subclassConf}); + }, e -> Assert.assertContains("Refusing to skip init-store", + e.getMessage())); + } + + private String writeDisabledRestServerConf(String... extraLines) + throws IOException { + return this.writeDisabledRestServerConfForGraphs( + this.workDir.resolve(MISSING_GRAPHS_DIR), extraLines); + } + + /** + * The same configuration with the gate left out entirely, so it takes the + * option's default rather than an explicit value. + */ + private String writeEnabledRestServerConf() throws IOException { + Path restConf = this.workDir.resolve( + "rest-server-" + this.confSeq++ + ".properties"); + Files.write(restConf, Arrays.asList(ServerOptions.GRAPHS.name() + "=" + + this.workDir.resolve(MISSING_GRAPHS_DIR)), + StandardCharsets.UTF_8); + return restConf.toString(); + } + + private String writeDisabledRestServerConfForGraphs(Path graphsDir, + String... extraLines) + throws IOException { + // A distinct file per call, so two configs written by one test do not + // collide inside its temporary directory + Path restConf = this.workDir.resolve( + "rest-server-" + this.confSeq++ + ".properties"); + List lines = new ArrayList<>(); + lines.add(ServerOptions.GRAPHS.name() + "=" + graphsDir); + lines.add(ServerOptions.INIT_STORE_ENABLED.name() + "=false"); + lines.addAll(Arrays.asList(extraLines)); + Files.write(restConf, lines, StandardCharsets.UTF_8); + return restConf.toString(); + } + + private Path writeAuthGraphConfig(String backend) throws IOException { + Path graphsDir = this.workDir.resolve("graphs-" + this.confSeq++); + Files.createDirectories(graphsDir); + Files.write(graphsDir.resolve("hugegraph.properties"), + Arrays.asList(CoreOptions.BACKEND.name() + "=" + backend), + StandardCharsets.UTF_8); + return graphsDir; + } + + /** + * Stands in for a deployment that subclasses the built-in authenticator, + * which still depends on the admin account it creates. + */ + public static class DerivedAuthenticator extends StandardAuthenticator { + } +} diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java new file mode 100644 index 0000000000..cb4a0b8abb --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerAdminInitTest.java @@ -0,0 +1,145 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.unit.core; + +import java.lang.reflect.Field; +import java.util.HashMap; +import java.util.Map; + +import org.apache.commons.configuration2.PropertiesConfiguration; +import org.apache.hugegraph.HugeException; +import org.apache.hugegraph.auth.HugeUser; +import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.core.GraphManager; +import org.apache.hugegraph.event.EventHub; +import org.apache.hugegraph.meta.MetaDriver; +import org.apache.hugegraph.meta.MetaManager; +import org.apache.hugegraph.meta.managers.AuthMetaManager; +import org.apache.hugegraph.meta.managers.SpaceMetaManager; +import org.apache.hugegraph.testutil.Assert; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +/** + * {@link GraphManager#initAdminUserIfNeeded} is the only bootstrap the + * built-in admin gets when init-store is disabled, and init-store's + * fail-closed check assumes it works. Only the already-exists case is benign; + * every other failure must abort startup rather than leave the server up + * without a usable administrator. + */ +public class GraphManagerAdminInitTest { + + private static final String CLUSTER = "admin-init-test"; + + private Map store; + private MetaDriver driver; + private Object originalAuthManager; + private Object originalSpaceManager; + private GraphManager manager; + + @Before + public void setup() throws Exception { + this.store = new HashMap<>(); + this.driver = Mockito.mock(MetaDriver.class); + Mockito.when(this.driver.get(Mockito.anyString())).thenAnswer( + i -> this.store.get(i.getArgument(0))); + Mockito.doAnswer(i -> this.store.put(i.getArgument(0), + i.getArgument(1))) + .when(this.driver) + .put(Mockito.anyString(), Mockito.anyString()); + + this.originalAuthManager = swapMetaManagerField( + "authMetaManager", new AuthMetaManager(this.driver, CLUSTER)); + this.originalSpaceManager = swapMetaManagerField( + "spaceMetaManager", new SpaceMetaManager(this.driver, CLUSTER)); + this.manager = new GraphManager( + new HugeConfig(new PropertiesConfiguration()), + new EventHub("admin-init-test")); + } + + @After + public void teardown() throws Exception { + try { + if (this.manager != null) { + this.manager.close(); + } + } finally { + swapMetaManagerField("authMetaManager", this.originalAuthManager); + swapMetaManagerField("spaceMetaManager", + this.originalSpaceManager); + } + } + + @Test + public void testCreatesAdminOnFreshMetadata() throws Exception { + this.manager.initAdminUserIfNeeded("s3cret"); + + HugeUser admin = MetaManager.instance().findUser("admin"); + Assert.assertNotNull("the admin must be created", admin); + } + + /** + * Every restart after the first sees the admin already recorded and + * surfaces the already-exists signal, which must neither fail startup + * nor rotate the existing password. (True concurrency is weaker than + * this: createUser is get-then-put without compare-and-set, so a tight + * race can overwrite rather than throw — benign only because every + * server writes the admin derived from the same configured password.) + */ + @Test + public void testExistingAdminIsKeptWithoutFailing() throws Exception { + this.manager.initAdminUserIfNeeded("first"); + HugeUser created = MetaManager.instance().findUser("admin"); + + this.manager.initAdminUserIfNeeded("second"); + + HugeUser kept = MetaManager.instance().findUser("admin"); + Assert.assertNotNull(kept); + Assert.assertEquals("an existing admin's password must not rotate", + created.password(), kept.password()); + } + + /** + * A PD write, permission or validation failure used to be logged and + * swallowed, so the server started with no usable administrator. It has + * to propagate instead: the failure is not the already-exists case, + * proven by the admin still being absent. + */ + @Test + public void testNonDuplicateCreationFailurePropagates() { + RuntimeException refused = new RuntimeException("pd write refused"); + Mockito.doThrow(refused).when(this.driver) + .put(Mockito.anyString(), Mockito.anyString()); + + Assert.assertThrows(HugeException.class, () -> { + this.manager.initAdminUserIfNeeded("s3cret"); + }, e -> Assert.assertEquals(refused, e.getCause())); + } + + private static Object swapMetaManagerField(String field, + Object replacement) + throws Exception { + Field f = MetaManager.class.getDeclaredField(field); + f.setAccessible(true); + Object previous = f.get(MetaManager.instance()); + f.set(MetaManager.instance(), replacement); + return previous; + } +}