Adarsh-Me commented on code in PR #3192:
URL: https://github.com/apache/hugegraph/pull/3192#discussion_r3991589459
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -70,12 +74,58 @@ set_prop() {
get_prop_encoded() {
local key="$1" file="$2"
- local esc_key
- esc_key=$(printf '%s' "$key" | sed -e 's/[][(){}.^$*+?|\\/]/\\&/g')
- sed -nE \
-
"s~^[[:space:]]*${esc_key}([[:space:]]*[:=][[:space:]]*|[[:space:]]+)(.*)$~\\2~p"
\
- "${file}" | head -n 1
+ PROPS_MODE=get PROPS_KEY="${key}" PROPS_FILE="${file}" \
+ awk -f "${PROPS_AWK}" /dev/null
+}
+
+# First uncommented `authenticator:` inside the gremlin-server.yaml
+# authentication block. snakeyaml resolves duplicate top-level keys to the
+# last one, but a mounted file carrying two authentication blocks is
+# pathological; report the first and let the mismatch WARN handle it.
+get_yaml_authenticator() {
+ local yaml="./conf/gremlin-server.yaml"
+
+ [[ -f "${yaml}" ]] || return 0
+ awk '
+ /^[ \t]*#/ { next }
+ /^[ \t]*authentication[ \t]*:/ { inblk = 1; next }
+ inblk && /^[ \t]+authenticator[ \t]*:/ {
+ line = $0
+ sub(/^[ \t]*authenticator[ \t]*:[ \t]*/, "", line)
+ sub(/[,:].*$/, "", line)
Review Comment:
Fixed in f5e368c: the extracted scalar now goes through a snakeyaml-shaped
cleanup before landing in rest-server.properties — an inline `#` comment (only
when the `#` is preceded by whitespace, so `#` inside a quoted scalar
survives), surrounding single/double quotes, and trailing padding are stripped.
Covered by a regression test with `authenticator: "com.example.MyAuth" #
custom`.
##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -70,12 +74,58 @@ set_prop() {
get_prop_encoded() {
local key="$1" file="$2"
- local esc_key
- esc_key=$(printf '%s' "$key" | sed -e 's/[][(){}.^$*+?|\\/]/\\&/g')
- sed -nE \
-
"s~^[[:space:]]*${esc_key}([[:space:]]*[:=][[:space:]]*|[[:space:]]+)(.*)$~\\2~p"
\
- "${file}" | head -n 1
+ PROPS_MODE=get PROPS_KEY="${key}" PROPS_FILE="${file}" \
+ awk -f "${PROPS_AWK}" /dev/null
+}
+
+# First uncommented `authenticator:` inside the gremlin-server.yaml
+# authentication block. snakeyaml resolves duplicate top-level keys to the
+# last one, but a mounted file carrying two authentication blocks is
+# pathological; report the first and let the mismatch WARN handle it.
+get_yaml_authenticator() {
+ local yaml="./conf/gremlin-server.yaml"
+
+ [[ -f "${yaml}" ]] || return 0
+ awk '
+ /^[ \t]*#/ { next }
+ /^[ \t]*authentication[ \t]*:/ { inblk = 1; next }
Review Comment:
Fixed in f5e368c: `authenticator:` is now also matched on the
`authentication:` line itself, so a single-line flow mapping is read instead of
silently falling back to the default. And when an `authentication` block is
present but no authenticator can be read, `align_auth_config` now takes the
WARN branch and leaves both sides untouched rather than the both-empty branch
that exported `StandardAuthenticator`. Both cases are covered in the suite.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh:
##########
@@ -41,16 +41,34 @@ if [ ! -d "$BAK_CONF" ]; then
cp "${CONF}/${GREMLIN_SERVER_CONF}"
"${BAK_CONF}/${GREMLIN_SERVER_CONF}.bak"
cp "${CONF}/${REST_SERVER_CONF}" "${BAK_CONF}/${REST_SERVER_CONF}.bak"
cp "${CONF}/graphs/${GRAPH_CONF}" "${BAK_CONF}/${GRAPH_CONF}.bak"
+fi
+
+# The appends below are guarded per file and match only an absent or still
+# commented-out definition, so they are no-ops on any config that already
+# carries authentication (e.g. a mounted one, or a re-run of this script).
+# Appending unconditionally used to create duplicate definitions that the
+# properties parser (first definition wins) and the yaml parser (last wins)
+# resolved in opposite directions, leaving Gremlin and REST on different
+# authenticators.
+AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}"
+if ! grep -Eq '^[ \t]*authentication[ \t]*:' "${CONF}/${GREMLIN_SERVER_CONF}";
then
sed -i -e '$a\authentication: {' \
- -e '$a\ authenticator:
org.apache.hugegraph.auth.StandardAuthenticator,' \
+ -e "\$a\\ authenticator: ${AUTHENTICATOR_CLASS}," \
-e '$a\ authenticationHandler:
org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler,' \
-e '$a\ config: {tokens: conf/rest-server.properties}' \
-e '$a\}' ${CONF}/${GREMLIN_SERVER_CONF}
+fi
- sed -i -e
'$a\auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \
- -e '$a\auth.graph_store=hugegraph' ${CONF}/${REST_SERVER_CONF}
+if ! grep -Eq '^[ \t]*auth\.authenticator[ \t]*='
"${CONF}/${REST_SERVER_CONF}"; then
Review Comment:
Fixed in f5e368c: the guards use `[[:blank:]]`, accept `[:=]` and the
bare-whitespace separator, and cover the backslash-escaped key spelling, so
every form `java.util.Properties` reads as a definition now satisfies the
guard. I kept the guards local rather than shipping props.awk under bin/ — that
felt like a structural change worth the maintainers' call, happy to do it if
you prefer one grammar in one place.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh:
##########
@@ -41,16 +41,34 @@ if [ ! -d "$BAK_CONF" ]; then
cp "${CONF}/${GREMLIN_SERVER_CONF}"
"${BAK_CONF}/${GREMLIN_SERVER_CONF}.bak"
cp "${CONF}/${REST_SERVER_CONF}" "${BAK_CONF}/${REST_SERVER_CONF}.bak"
cp "${CONF}/graphs/${GRAPH_CONF}" "${BAK_CONF}/${GRAPH_CONF}.bak"
+fi
+
+# The appends below are guarded per file and match only an absent or still
+# commented-out definition, so they are no-ops on any config that already
+# carries authentication (e.g. a mounted one, or a re-run of this script).
+# Appending unconditionally used to create duplicate definitions that the
+# properties parser (first definition wins) and the yaml parser (last wins)
+# resolved in opposite directions, leaving Gremlin and REST on different
+# authenticators.
+AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}"
+if ! grep -Eq '^[ \t]*authentication[ \t]*:' "${CONF}/${GREMLIN_SERVER_CONF}";
then
sed -i -e '$a\authentication: {' \
- -e '$a\ authenticator:
org.apache.hugegraph.auth.StandardAuthenticator,' \
+ -e "\$a\\ authenticator: ${AUTHENTICATOR_CLASS}," \
-e '$a\ authenticationHandler:
org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler,' \
-e '$a\ config: {tokens: conf/rest-server.properties}' \
-e '$a\}' ${CONF}/${GREMLIN_SERVER_CONF}
+fi
- sed -i -e
'$a\auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \
- -e '$a\auth.graph_store=hugegraph' ${CONF}/${REST_SERVER_CONF}
+if ! grep -Eq '^[ \t]*auth\.authenticator[ \t]*='
"${CONF}/${REST_SERVER_CONF}"; then
+ sed -i -e "\$a\\auth.authenticator=${AUTHENTICATOR_CLASS}"
${CONF}/${REST_SERVER_CONF}
+fi
+
+if ! grep -Eq '^[ \t]*auth\.graph_store[ \t]*=' "${CONF}/${REST_SERVER_CONF}";
then
+ sed -i -e '$a\auth.graph_store=hugegraph' ${CONF}/${REST_SERVER_CONF}
+fi
- sed -i
's/gremlin.graph=org.apache.hugegraph.HugeFactory/gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy/g'
${CONF}/graphs/${GRAPH_CONF}
+if grep -Eq '^gremlin\.graph[ \t]*=org\.apache\.hugegraph\.HugeFactory[ \t]*$'
"${CONF}/graphs/${GRAPH_CONF}"; then
Review Comment:
Fixed in f5e368c, with one deviation from the suggested pattern: GNU grep
reads `\r` in a pattern as the letter `r`, not a carriage return (verified
against GNU grep 3.11 — the suggested `\r?` pattern still fails on a CRLF
file). The carriage return is therefore embedded as a byte via `CR=$'\r'` in
the guard, and the `sed` replaces the class in place (dropping the `$` anchor)
so the trailing CR is preserved. Verified end to end: plain LF and CRLF configs
both flip, an already-proxied line and a longer class name are both left alone.
##########
hugegraph-server/hugegraph-dist/docker/props.awk:
##########
@@ -0,0 +1,227 @@
+# 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.
+#
+# props.awk — read and rewrite Java ".properties" files with the grammar
+# HugeConfig (commons-configuration over JDK Properties) applies, so the
+# entrypoint and the server agree on what a mounted file means. grep/sed
+# rewrites do not: they see `\`-escaped keys, `:` separators, continuation
+# lines and duplicate definitions differently, which is how a mounted
+# config ends up with two definitions of one key.
+#
+# One invocation, selected with the `mode` environment variable:
+#
+# mode=get key=K file=F
+# print the value of K's first logical definition
+# mode=set key=K file=F
+# replace K's first definition in place, drop every other
+# definition of K, append one when the file has none. The new
+# value arrives pre-encoded in PROP_VALUE_ENCODED (an environment
Review Comment:
Fixed in f5e368c: the header now documents the actual environment variables
— `PROPS_MODE`, `PROPS_KEY`, `PROPS_FILE` and `PROPS_VALUE_ENCODED`.
##########
hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh:
##########
@@ -23,10 +23,16 @@ entrypoint="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." &&
pwd)/docker-entrypoint.s
test_dir="$(mktemp -d)"
trap 'rm -rf "${test_dir}"' EXIT
+# Eval the property helpers plus the PROPS_AWK location block they depend
+# on. The entrypoint's top-level code hard-exits when props.awk is
+# missing, so it cannot be sourced directly; anchor to the marker comment
Review Comment:
Fixed in f5e368c: the marker-comment sentence is gone and the helpers are no
longer captured by count at all — each is extracted by function name, so the
eval is independent of helper order and a helper inserted between
`encode_prop_value` and `get_prop_encoded` cannot cut it short.
--
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]
