bitflicker64 commented on code in PR #3192:
URL: https://github.com/apache/hugegraph/pull/3192#discussion_r4030331496
##########
hugegraph-server/hugegraph-dist/docker/test/test-docker-entrypoint.sh:
##########
@@ -66,3 +76,218 @@ assert_line_count 1 \
"${duplicate_file}"
assert_line_count 1 '^init_store\.enabled=true$' "${duplicate_file}"
grep -q '^unrelated=true$' "${duplicate_file}"
+
+# An escaped key is one logical definition of that key, not a key with
+# backslashes in its name: setting the plain key must rewrite it in place
+# rather than appending a second definition whose only resolution is
+# parser-dependent (and which HugeConfig then reports as a list).
+escaped_file="${test_dir}/config-escaped-key"
+printf '%s\n' \
+ 'auth\.admin_pa=old' \
+ 'unrelated=true' > "${escaped_file}"
+set_prop "auth.admin_pa" "new" "${escaped_file}"
+assert_line_count 1 '^auth\.admin_pa=new$' "${escaped_file}"
+assert_line_count 1 '^unrelated=true$' "${escaped_file}"
+
+# A value continued onto the next line is part of the same definition:
+# setting the key must remove the continuation, not leave it behind as a
+# stray property of its own.
+continued_file="${test_dir}/config-continuation"
+printf '%s\n' \
+ 'pd.peers 127.0.0.1:8686,\' \
+ ' 127.0.0.2:8686' \
+ 'unrelated=true' > "${continued_file}"
+set_prop "pd.peers" "10.0.0.1:8686" "${continued_file}"
+assert_line_count 1 '^pd\.peers=10\.0\.0\.1:8686$' "${continued_file}"
+assert_line_count 1 '^unrelated=true$' "${continued_file}"
+[[ "$(grep -c '127\.0\.0\.2' "${continued_file}")" -eq 0 ]]
+
+# get_prop_encoded reads through the same grammar: separators, escapes,
+# continuations, and first-definition-wins duplicates.
+get_file="${test_dir}/config-get"
+printf '%s\n' \
+ '#comment' \
+ 'a\=b : colon value' \
+ 'multiline first \' \
+ ' second' \
+ 'dup : one' \
+ 'dup=two' > "${get_file}"
+[[ "$(get_prop_encoded 'a=b' "${get_file}")" == "colon value" ]]
+[[ "$(get_prop_encoded 'multiline' "${get_file}")" == "first second" ]]
+[[ "$(get_prop_encoded 'dup' "${get_file}")" == "one" ]]
+
+# Appends must still happen when the file has no definition of the key,
+# including when the only occurrences are inside comments.
+append_file="${test_dir}/config-append"
+printf '%s\n' \
+ '#init_store.enabled=false' \
+ 'unrelated=true' > "${append_file}"
+set_prop "init_store.enabled" "true" "${append_file}"
+assert_line_count 1 '^init_store\.enabled=true$' "${append_file}"
+assert_line_count 1 '^#init_store\.enabled=false$' "${append_file}"
+
+# A key indented with leading whitespace is still one definition of the
+# key: java.util.Properties ignores whitespace before a key, so an
+# indented key must be read and rewritten in place rather than duplicated.
+indented_file="${test_dir}/config-indented-key"
+printf '%s\n' \
+ ' auth.token_secret: old-secret' \
+ 'unrelated=true' > "${indented_file}"
+[[ "$(get_prop_encoded 'auth.token_secret' "${indented_file}")" ==
"old-secret" ]]
+set_prop_encoded 'auth.token_secret' 'new-secret' "${indented_file}"
+assert_line_count 1 'auth\.token_secret' "${indented_file}"
+assert_line_count 1 '^unrelated=true$' "${indented_file}"
+
+# get_yaml_authenticator must agree with snakeyaml on what a mounted
+# gremlin-server.yaml says: the authenticator inside the authentication
+# block — quoted scalars and inline comments cleaned the way snakeyaml
+# strips them — and a flow mapping on the authentication line itself.
+# align_auth_config refuses an authentication block without a readable
+# authenticator instead of treating it as "no yaml side": exporting the
+# default there would override an explicit choice, and continuing would let
+# enable-auth.sh write the REST side alone.
+yaml_dir="${test_dir}/yaml"
+mkdir -p "${yaml_dir}/conf"
+(
+ cd "${yaml_dir}" || exit 1
+ REST_SERVER_CONF="./conf/rest-server.properties"
+ : > "${REST_SERVER_CONF}"
+
+ printf '%s\n' \
+ 'authentication:' \
+ ' authenticator: "com.example.MyAuth" # custom' \
+ ' authenticationHandler:
org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \
+ > conf/gremlin-server.yaml
+ [[ "$(get_yaml_authenticator)" == "com.example.MyAuth" ]]
+
+ printf '%s\n' \
+ 'authentication: {authenticator: com.example.FlowAuth,
authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler,
config: {tokens: conf/rest-server.properties}}' \
+ > conf/gremlin-server.yaml
+ [[ "$(get_yaml_authenticator)" == "com.example.FlowAuth" ]]
+
+# align_auth_config must refuse an authentication block without a readable
+# authenticator: continuing would let enable-auth.sh write the REST side
+# alone (REST on StandardAuthenticator, Gremlin on TinkerPop's
+# AllowAllAuthenticator default), so the entrypoint stops here instead.
+ printf '%s\n' \
+ 'authentication:' \
+ ' authenticationHandler:
org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler' \
+ > conf/gremlin-server.yaml
+ unset AUTHENTICATOR_CLASS
+ if align_auth_config; then
+ echo "align_auth_config must refuse an authentication block" \
+ "without a readable authenticator" >&2
+ exit 1
+ fi
+ [[ -z "${AUTHENTICATOR_CLASS:-}" ]]
+ [[ ! -s "${REST_SERVER_CONF}" ]]
+
+ printf '%s\n' \
+ 'authentication:' \
+ ' authenticator: com.example.YamlAuth' \
+ > conf/gremlin-server.yaml
+ align_auth_config
+ grep -q '^auth\.authenticator=com\.example\.YamlAuth$'
"${REST_SERVER_CONF}"
+)
+
+# The refusal above is what keeps enable-auth.sh from writing one side:
+# against the same ambiguous layout, enable-auth.sh on its own writes only
+# the REST file (its yaml guard already sees an `authentication:` line),
+# leaving REST on StandardAuthenticator and Gremlin on TinkerPop's
+# AllowAllAuthenticator default. The entrypoint never lets it run there
+# because align_auth_config fails first under set -e.
+onesided_dir="${test_dir}/yaml-onesided"
+mkdir -p "${onesided_dir}/bin" "${onesided_dir}/conf/graphs"
+cp "$(cd "$(dirname "${BASH_SOURCE[0]}")/../../src/assembly/static/bin" &&
pwd)/enable-auth.sh" \
+ "${onesided_dir}/bin/enable-auth.sh"
+chmod +x "${onesided_dir}/bin/enable-auth.sh"
+(
+ cd "${onesided_dir}" || exit 1
+ REST_SERVER_CONF="./conf/rest-server.properties"
+ : > conf/rest-server.properties
Review Comment:
⚠️ This empty fixture makes the new one-sided test fail, which is why
`docker-build (hugegraph-server/Dockerfile)` is red at bf2718f.
`enable-auth.sh` appends with `sed -i -e '$a\...'`. On an empty file GNU sed
has no last line, so `$a` never runs and nothing is written. The `grep -q
'^auth\.authenticator=org\.apache\.hugegraph\.auth\.StandardAuthenticator$'` on
line 221 then exits 1 and `set -e` stops the suite. Checked with GNU sed 4.10:
```
$ : > empty.properties
$ sed -i -e
'$a\auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator'
empty.properties
$ wc -c < empty.properties
0
```
In CI (run 35101931936), the log stops right after the second refusal ERROR,
which comes from this block.
Requested change: seed the fixture with one unrelated line so it matches a
real `rest-server.properties`, for example `printf '%s\n'
'restserver.url=http://0.0.0.0:8080' > conf/rest-server.properties`. If an
empty mounted file should also work, switch the two REST appends in
`enable-auth.sh` to `printf '%s\n' ... >> "${CONF}/${REST_SERVER_CONF}"`, which
also writes to an empty file.
--
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]
