bitflicker64 commented on code in PR #3253:
URL: https://github.com/apache/hugegraph/pull/3253#discussion_r4368421176
##########
hugegraph-store/hg-store-node/src/main/java/org/apache/hugegraph/store/node/listener/PdConfigureListener.java:
##########
@@ -57,10 +57,31 @@ public class PdConfigureListener implements
private static final String PD_CONFIG_FILE_NAME = "application-pd.yml";
private final String workDir = System.getProperty("user.dir");
private final String fileSeparator = System.getProperty("file.separator");
- private final String configFilePath =
- workDir + fileSeparator + "conf" + fileSeparator +
PD_CONFIG_FILE_NAME;
+ // start-hugegraph-store.sh always passes
+ // -Dspring.config.location=${CONF}/application.yml, where CONF already
+ // resolves the -c override (or $TOP/conf by default). Deriving the conf
+ // dir from that property (instead of user.dir, which is always $TOP
+ // because the script does `cd "${TOP}"` before exec'ing java) keeps this
+ // listener's read/write target in sync with the conf dir the rest of the
+ // process actually uses, including after a -c-overridden restart.
+ private final String configFilePath = resolveConfigFilePath();
private final String restartShellPath = workDir + fileSeparator + "bin" +
fileSeparator
+ "restart-hugegraph-store.sh";
+
+ private static String resolveConfigFilePath() {
+ String springConfigLocation =
System.getProperty("spring.config.location");
+ if (!StringUtils.isEmpty(springConfigLocation)) {
+ File appYml = new File(springConfigLocation);
Review Comment:
Minor: This treats `spring.config.location` as a plain file path, but Spring
also accepts `file:` and `optional:` prefixes, a directory ending in `/`, and a
comma-separated list. The shipped launchers pass a plain absolute file, so they
are fine, but a Store started by hand with a Spring-style value gets a wrong
path. A JDK 17 probe of this logic turns `file:/etc/store/application.yml` into
`file:/etc/store/application-pd.yml`, a relative path under the working
directory (so `updatePdConfig` would create a `file:` directory there), and
`/etc/store/` into `/etc/application-pd.yml`. Before this change those launches
used `user.dir/conf/application-pd.yml`. Please strip `optional:` and `file:`,
use the value only when it is a single file path, and fall back to
`user.dir/conf` otherwise, with a test for the prefixed and directory forms.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/start-hugegraph.sh:
##########
@@ -71,12 +103,21 @@ if [[ "$OPEN_SECURITY_CHECK" != "true" &&
"$OPEN_SECURITY_CHECK" != "false" ]];
exit_with_usage_help
fi
+# Verify the config files exist before reading them
+ensure_file_readable "$CONF/rest-server.properties" "REST server config" ||
exit 1
+ensure_file_readable "$CONF/gremlin-server.yaml" "Gremlin server config" ||
exit 1
+
GREMLIN_SERVER_URL=$(read_property "$CONF/rest-server.properties"
"gremlinserver.url")
if [ -z "$GREMLIN_SERVER_URL" ]; then
GREMLIN_SERVER_URL="http://127.0.0.1:8182";
fi
REST_SERVER_URL=$(read_property "$CONF/rest-server.properties"
"restserver.url")
+# 'graphs' is read by HugeGraphServer/GraphManager relative to the working
+# directory (set to $TOP below), not relative to $CONF.
+GRAPHS_DIR=$(read_property "$CONF/rest-server.properties" "graphs")
+reject_relative_conf_path "$GRAPHS_DIR" "graphs"
"$CONF/rest-server.properties" || exit 1
Review Comment:
Important: `graphs` is now checked under `-c`, but the HTTPS keystore has
the same problem and is not checked. `ssl.keystore_file` defaults to the
relative `conf/hugegraph-server.keystore` (ServerOptions.SSL_KEYSTORE_FILE),
the stock rest-server.properties does not set it, and
RestServer.configHttpServer hands it unchanged to Grizzly's
SSLContextConfigurator, which opens it against the working directory, `$TOP`.
Lines 125-126 below look for the keystore at
`${CONF}/hugegraph-server.keystore` and download it there, which under `-c` is
the external conf dir. So with `restserver.url=https://...` and `-c
/etc/hugegraph`, the script finds or downloads the keystore in `/etc/hugegraph`
while the JVM opens `$TOP/conf/hugegraph-server.keystore`: the operator's
keystore is ignored, and on a fresh or read-only install, where the tarball
ships no keystore, the Server fails to set up TLS. Please run the same check
for the keystore when the REST URL is https: read `ssl.keystore_file`, treat an
empty value as the default `conf/hugegraph-server.keystore`, and reject a
relative value under `-c` with the same message. The download should then
target the path the JVM will open rather than `${CONF}`.
--
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]
