vaijosh commented on code in PR #3253:
URL: https://github.com/apache/hugegraph/pull/3253#discussion_r4141524746
##########
hugegraph-pd/hg-pd-dist/src/assembly/static/bin/start-hugegraph-pd.sh:
##########
@@ -53,13 +58,13 @@ function abs_path() {
BIN=$(abs_path)
TOP="$(cd "$BIN"/../ && pwd)"
-CONF="$TOP/conf"
+CONF="${CONF_OVERRIDE:-$TOP/conf}"
LIB="$TOP/lib"
-PLUGINS="$TOP/plugins"
-LOGS="$TOP/logs"
+PLUGINS="${PLUGINS_OVERRIDE:-$TOP/plugins}"
Review Comment:
## 🚫 Blocking: Plugin classpath is silently broken by the override
Please check earlier comment for detailed description. Remove the
PLUGIN_OVERRIDE option for now.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/init-store.sh:
##########
@@ -25,11 +25,19 @@ function abs_path() {
cd -P "$(dirname "$SOURCE")" && pwd
}
+while getopts "o:" arg; do
+ case ${arg} in
+ o) PLUGINS_OVERRIDE="$OPTARG" ;;
+ ?) echo "USAGE: $0 [-o plugins_dir]" && exit 1 ;;
+ esac
+done
+shift $((OPTIND - 1))
+
BIN=$(abs_path)
TOP="$(cd "${BIN}"/../ && pwd)"
CONF="$TOP/conf"
LIB="$TOP/lib"
-PLUGINS="$TOP/plugins"
+PLUGINS="${PLUGINS_OVERRIDE:-$TOP/plugins}"
Review Comment:
🚫 Blocking: Plugin classpath is silently broken by the override
Please check earlier comment for detailed description. Remove the
PLUGIN_OVERRIDE option for now.
##########
hugegraph-pd/hg-pd-dist/src/assembly/static/bin/start-hugegraph-pd.sh:
##########
@@ -30,14 +30,19 @@ if [ -z "$DAEMON" ]; then
DAEMON="true"
fi
-while getopts "d:g:j:y:" arg; do
+while getopts "c:d:g:i:j:l:o:y:" arg; do
case ${arg} in
+ c) CONF_OVERRIDE="$OPTARG" ;;
g) GC_OPTION="$OPTARG" ;;
+ i) PID_FILE_OVERRIDE="$OPTARG" ;;
j) USER_OPTION="$OPTARG" ;;
+ l) LOGS_OVERRIDE="$OPTARG" ;;
+ o) PLUGINS_OVERRIDE="$OPTARG" ;;
Review Comment:
## 🚫 Blocking: Plugin classpath is silently broken by the override
`hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh` —
specifically the `PLUGINS=` assignment and the `find -L $PLUGINS` classpath
logic.
### Evidence
`$TOP/plugins` currently has two separate responsibilities:
1. It is the conventional location where packaged or admin-installed plugin
JARs are discovered **recursively** for both server startup and `init-store.sh`.
2. It is also used as the location for runtime artifacts, such as the
OpenTelemetry agent, which are downloaded at runtime.
This PR makes the entire directory writable/overridable. As a result, `-o
<dir>` effectively replaces `$TOP/plugins` for **both** responsibilities.
The script still calls `ensure_path_writable "$PLUGINS"`. Therefore, on a
read-only installation, operators are required to provide `-o`, at which point
the existing `$TOP/plugins` directory is no longer included in the classpath.
### Impact
This is particularly problematic for the **read-only installation scenario
that this PR targets**:
* Existing backend, serializer, analyzer, or other `HugeGraphPlugin` JARs
under `$TOP/plugins` are silently excluded from the classpath.
* `Server` or `init-store.sh` may fail with `ClassNotFoundException`, or the
service may start with missing functionality.
* Operators would have to copy the complete plugin set into the external
directory and keep it synchronized with every HugeGraph release. This behavior
is currently not documented.
* Recursively adding the writable directory to the application classpath
also means that downloaded runtime artifacts such as the OpenTelemetry agent
JAR can unintentionally become part of the application classpath, in addition
to being specified through `-javaagent`.
### Requested Fix
Please separate these two responsibilities instead of overriding a single
shared directory:
1. **Keep `$TOP/plugins` as the conventional plugin directory.**
* It should remain usable as a read-only directory.
* `ensure_path_writable "$PLUGINS"` should not be required for the normal
plugin directory.
2. **Introduce a separate writable runtime/agent directory**, for example:
* `-a <dir>` / `AGENTS_DIR_OVERRIDE`
* Default to `$TOP/plugins` for backward compatibility.
* Use this directory **only** for:
* OpenTelemetry agent download
* MD5/checksum validation
* `-javaagent`
##########
hugegraph-store/hg-store-dist/src/assembly/static/bin/start-hugegraph-store.sh:
##########
@@ -105,17 +100,28 @@ if [ -z "$DAEMON" ]; then
DAEMON="true"
fi
-while getopts "d:g:j:y:" arg; do
+while getopts "d:c:g:i:j:l:o:y:" arg; do
case ${arg} in
+ c) CONF_OVERRIDE="$OPTARG" ;;
g) GC_OPTION="$OPTARG" ;;
+ i) PID_FILE_OVERRIDE="$OPTARG" ;;
j) USER_OPTION="$OPTARG" ;;
+ l) LOGS_OVERRIDE="$OPTARG" ;;
+ o) PLUGINS_OVERRIDE="$OPTARG" ;;
Review Comment:
🚫 Blocking: Plugin classpath is silently broken by the override
Please check earlier comment for detailed description. Remove the
PLUGIN_OVERRIDE option for now.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/hugegraph-server.sh:
##########
@@ -34,11 +34,11 @@ fi
BIN=$(abs_path)
TOP="$(cd "$BIN"/../ && pwd)"
-CONF="$TOP/conf"
+CONF="${CONF_OVERRIDE:-$TOP/conf}"
LIB="$TOP/lib"
EXT="$TOP/ext"
-PLUGINS="$TOP/plugins"
-LOGS="$TOP/logs"
+PLUGINS="${PLUGINS_OVERRIDE:-$TOP/plugins}"
Review Comment:
🚫 Blocking: Plugin classpath is silently broken by the override
Please check earlier comment for detailed description. Remove the
PLUGIN_OVERRIDE option for now.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/start-hugegraph.sh:
##########
@@ -39,29 +39,37 @@ function abs_path() {
BIN=$(abs_path)
TOP="$(cd "$BIN"/../ && pwd)"
-CONF="$TOP/conf"
-LOGS="$TOP/logs"
SCRIPTS="$TOP/scripts"
-PID_FILE="$BIN/pid"
. "$BIN"/util.sh
# Note: keep ':' in the end of the string to indicate the option needs a value
-while getopts "d:g:m:p:s:j:t:y:" arg; do
- case ${arg} in
- d) DAEMON="$OPTARG" ;;
- g) GC_OPTION="$OPTARG" ;;
- m) OPEN_MONITOR="$OPTARG" ;;
- p) PRELOAD="$OPTARG" ;;
- s) OPEN_SECURITY_CHECK="$OPTARG" ;;
- j) USER_OPTION="$OPTARG" ;;
- t) SERVER_STARTUP_TIMEOUT_S="$OPTARG" ;;
- # Telemetry is used to collect metrics, traces and logs
- y) OPEN_TELEMETRY="$OPTARG" ;;
- # Note: update usage info when the params changed
- ?) exit_with_usage_help ;;
- esac
-done
+while getopts "c:d:g:i:j:l:m:o:p:s:t:y:" arg; do
+ case ${arg} in
+ c) CONF_OVERRIDE="$OPTARG" ;;
+ d) DAEMON="$OPTARG" ;;
+ g) GC_OPTION="$OPTARG" ;;
+ i) PID_FILE_OVERRIDE="$OPTARG" ;;
+ j) USER_OPTION="$OPTARG" ;;
+ l) LOGS_OVERRIDE="$OPTARG" ;;
+ m) OPEN_MONITOR="$OPTARG" ;;
+ o) PLUGINS_OVERRIDE="$OPTARG" ;;
Review Comment:
🚫 Blocking: Plugin classpath is silently broken by the override
Please check earlier comment for detailed description. Remove the
PLUGIN_OVERRIDE option for now.
##########
hugegraph-store/hg-store-dist/src/assembly/static/bin/start-hugegraph-store.sh:
##########
@@ -105,17 +100,28 @@ if [ -z "$DAEMON" ]; then
DAEMON="true"
fi
-while getopts "d:g:j:y:" arg; do
+while getopts "d:c:g:i:j:l:o:y:" arg; do
case ${arg} in
+ c) CONF_OVERRIDE="$OPTARG" ;;
g) GC_OPTION="$OPTARG" ;;
+ i) PID_FILE_OVERRIDE="$OPTARG" ;;
j) USER_OPTION="$OPTARG" ;;
+ l) LOGS_OVERRIDE="$OPTARG" ;;
+ o) PLUGINS_OVERRIDE="$OPTARG" ;;
# Telemetry is used to collect metrics, traces and logs
y) OPEN_TELEMETRY="$OPTARG" ;;
d) DAEMON="$OPTARG" ;;
- ?) echo "USAGE: $0 [-d true|false] [-g g1] [-j xxx] [-y true|false]"
&& exit 1 ;;
+ ?) echo "USAGE: $0 [-d true|false] [-c conf_dir] [-g g1] [-i pid_file]
[-j opts] [-l logs_dir] [-o
+ plugins_dir] [-y true|false]" && exit 1 ;;
esac
done
+CONF="${CONF_OVERRIDE:-$TOP/conf}"
+LOGS="${LOGS_OVERRIDE:-$TOP/logs}"
+PLUGINS="${PLUGINS_OVERRIDE:-$TOP/plugins}"
Review Comment:
🚫 Blocking: Plugin classpath is silently broken by the override
Please check earlier comment for detailed description. Remove the
PLUGIN_OVERRIDE option for now.
--
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]
