bitflicker64 commented on code in PR #3189:
URL: https://github.com/apache/hugegraph/pull/3189#discussion_r3957231248
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh:
##########
@@ -93,19 +116,34 @@ if env | grep '^hugegraph\.' > /dev/null; then
export PD_REST_LIST
log "PD REST peers = $PD_REST_LIST"
+ # Only worth saying where PD is actually polled: topologies without
+ # pd.peers never send this credential anywhere.
+ if [ -z "${PD_AUTH_PASSWORD}" ]; then
+ log "WARN: PD_AUTH_PASSWORD is empty; PD will answer 401 unless
it runs without auth"
+ fi
log "Timeout = ${WAIT_STORAGE_TIMEOUT_S}s"
timeout "${WAIT_STORAGE_TIMEOUT_S}s" bash -c "
log() { echo '[wait-storage] '\"\$1\"; }
+ # curl stays out of the grep pipeline so its status code is
+ # readable: a 401 is a wrong secret, not a storage problem, and
+ # retrying it for 300s only hides that.
check_any_pd_stores() {
for peer in \$(echo \"\$PD_REST_LIST\" | tr ',' ' '); do
- if curl ${PD_AUTH_ARGS} -f -s \
- --connect-timeout ${WAIT_STORAGE_PD_CONNECT_TIMEOUT_S} \
- --max-time ${WAIT_STORAGE_PD_MAX_TIMEOUT_S} \
- http://\${peer}/v1/stores 2>/dev/null | \
- grep -qi '\"state\"[[:space:]]*:[[:space:]]*\"Up\"'; then
+ body=\$(printf 'user = \"%s:%s\"\n' \
+ \"\$PD_AUTH_CURL_USER\" \"\$PD_AUTH_CURL_PASSWORD\"
| \
+ curl -K - -s -w '\n%{http_code}' \
+ --connect-timeout
${WAIT_STORAGE_PD_CONNECT_TIMEOUT_S} \
+ --max-time ${WAIT_STORAGE_PD_MAX_TIMEOUT_S} \
+ \"http://\${peer}/v1/stores\"; 2>/dev/null)
+ code=\${body##*\$'\n'}
+ if [ \"\$code\" = 401 ]; then
+ log \"ERROR: PD at \${peer} refused the credential (401):
PD_AUTH_PASSWORD must match PD's auth.secret-key\" >&2
+ return 2
Review Comment:
Taken in ae2a39a5f, as the alternative you described. A 401 sets `refused=1`
and the loop continues to the next peer; `return 2` fires only when the pass
ends with no Up store. A fleet-wide wrong secret still aborts on the first pass
rather than retrying for 300s, and one stale peer no longer costs a Server.
test-wait-storage.sh follows: the auth-401 case expects one call per peer
instead of one call in total, and a new `one-401` scenario, where pd0 refuses
and pd1 has an Up store, asserts rc=0, both peers called, the 401 still named
in the output, and `PASSED via pd1:8620`. 9 passed, 0 failed.
##########
hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh:
##########
@@ -26,10 +26,31 @@ require_env() {
fi
}
+# Escape a value for use inside a JSON string: backslash and quote, then every
+# remaining C0 control character as \uXXXX. Dropping only LF, as an earlier
+# version did, left CR and TAB to produce invalid JSON and a container that
+# failed before startup.
json_escape() {
- local s="$1"
- s=${s//\\/\\\\}; s=${s//\"/\\\"}; s=${s//$'\n'/}
- printf "%s" "$s"
+ local s="$1" out="" i c
Review Comment:
Taken in ae2a39a5f: `local LC_ALL=C` is the first line of json_escape, with
the reasoning in a comment above it.
Being accurate about what it changed, though: I could not get the unpinned
version to corrupt anything. test-pd-docker-entrypoint.sh now runs the
non-ASCII and CR/TAB cases a second time under a UTF-8 locale, picked from
`locale -a` so an ungenerated one does not make the case vacuous, and the
secret round-trips both with and without the pin on this host. So this removes
the locale dependency rather than fixing an observed corruption, which is how
your comment framed it too. 15 passed, 0 failed.
--
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]
