-
Notifications
You must be signed in to change notification settings - Fork 636
fix(docker): make auth bootstrap safe for mounted and upgraded configs #3192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
698b0c3
f5e368c
5f50511
bedc21e
bf2718f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,18 @@ mkdir -p "${DOCKER_FOLDER}" | |
|
|
||
| log() { echo "[hugegraph-server-entrypoint] $*"; } | ||
|
|
||
| # Property reading/writing goes through props.awk, which implements the | ||
| # java.util.Properties grammar HugeConfig applies (escapes, `:`/whitespace | ||
| # separators, continuations, first-definition-wins duplicates). grep/sed | ||
| # rewrites disagree with it on mounted or upgraded configs, silently | ||
| # producing two definitions of one key. Values move through environment | ||
| # variables rather than argv so a PASSWORD never shows up in `ps` output. | ||
| PROPS_AWK="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/props.awk" | ||
| if [[ ! -f "${PROPS_AWK}" ]]; then | ||
| log "ERROR: props.awk not found next to the entrypoint" | ||
| exit 1 | ||
| fi | ||
|
|
||
| encode_prop_value() { | ||
| local value="$1" encoded="" char | ||
| local i | ||
|
|
@@ -48,18 +60,10 @@ encode_prop_value() { | |
|
|
||
| set_prop_encoded() { | ||
| local key="$1" encoded_val="$2" file="$3" | ||
| local esc_key esc_val key_re | ||
|
|
||
| esc_key=$(printf '%s' "$key" | sed -e 's/[][(){}.^$*+?|\\/]/\\&/g') | ||
| esc_val=$(printf '%s' "$encoded_val" | sed -e 's/[&|\\~]/\\&/g') | ||
| key_re="^[[:space:]]*${esc_key}([[:space:]]*[:=]|[[:space:]]+|[[:space:]]*$)" | ||
|
|
||
| if grep -qE "${key_re}" "${file}"; then | ||
| sed -ri "0,/${key_re}/!{/${key_re}/d;}" "${file}" | ||
| sed -ri "0,/${key_re}/s~${key_re}.*~${key}=${esc_val}~" "${file}" | ||
| else | ||
| printf '%s=%s\n' "$key" "$encoded_val" >> "${file}" | ||
| fi | ||
| PROPS_MODE=set PROPS_KEY="${key}" \ | ||
| PROPS_VALUE_ENCODED="${encoded_val}" PROPS_FILE="${file}" \ | ||
| awk -f "${PROPS_AWK}" /dev/null | ||
| } | ||
|
|
||
| set_prop() { | ||
|
|
@@ -70,12 +74,127 @@ 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 | ||
| } | ||
|
|
||
| # Decoded read: unescapes the on-disk value the way java.util.Properties | ||
| # does, so it compares equal with the snakeyaml-decoded scalar from | ||
| # get_yaml_authenticator. The raw get_prop_encoded mode stays for the | ||
| # secret round trip, which must replay backslashes byte-for-byte. | ||
| get_prop() { | ||
| local key="$1" file="$2" | ||
|
|
||
| PROPS_MODE=get-decoded PROPS_KEY="${key}" PROPS_FILE="${file}" \ | ||
| awk -f "${PROPS_AWK}" /dev/null | ||
| } | ||
|
|
||
| # First uncommented `authenticator:` inside the gremlin-server.yaml | ||
| # authentication block, or on the `authentication:` line itself (a flow | ||
| # mapping). 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. The scalar is | ||
| # cleaned the way snakeyaml reads it — an inline comment (a '#' preceded | ||
| # by whitespace), surrounding quotes and padding are stripped — because | ||
| # java.util.Properties keeps all of those in the class name. | ||
| get_yaml_authenticator() { | ||
| local yaml="./conf/gremlin-server.yaml" | ||
|
|
||
| [[ -f "${yaml}" ]] || return 0 | ||
| awk ' | ||
| function scalar(s, out, i, n, c, q) { | ||
| out = "" | ||
| q = "" | ||
| n = length(s) | ||
| for (i = 1; i <= n; i++) { | ||
| c = substr(s, i, 1) | ||
| if (q != "") { | ||
| if (c == q) q = "" | ||
| else out = out c | ||
| continue | ||
| } | ||
| if (c == "\"" || c == "\047") { q = c; continue } | ||
| if (c == "#" && | ||
| (out == "" || substr(out, length(out), 1) ~ /[ \t]/)) | ||
| break | ||
| if (c == "," || c == "}" || c == "]") break | ||
| out = out c | ||
| } | ||
| sub(/^[ \t\r]+/, "", out) | ||
| sub(/[ \t\r]+$/, "", out) | ||
| return out | ||
| } | ||
| /^[ \t]*#/ { next } | ||
| /^[ \t]*authentication[ \t]*:/ { | ||
| inblk = 1 | ||
| line = $0 | ||
| sub(/^[ \t]*authentication[ \t]*:[ \t]*/, "", line) | ||
| if (match(line, /authenticator[ \t]*:/)) { | ||
| print scalar(substr(line, RSTART + RLENGTH)) | ||
| exit | ||
| } | ||
| next | ||
| } | ||
| inblk && /^[ \t]+authenticator[ \t]*:/ { | ||
| line = $0 | ||
| sub(/^[ \t]*authenticator[ \t]*:[ \t]*/, "", line) | ||
| print scalar(line) | ||
| exit | ||
| } | ||
| ' "${yaml}" | ||
| } | ||
|
|
||
| # A mounted yaml can carry an authentication block whose authenticator | ||
| # cannot be read (an empty or unparseable one). That is not the | ||
| # both-empty case: exporting the default would override an explicit | ||
| # choice that snakeyaml does resolve, so callers treat it as a mismatch. | ||
| has_yaml_authentication_block() { | ||
| local yaml="./conf/gremlin-server.yaml" | ||
|
|
||
| [[ -f "${yaml}" ]] || return 1 | ||
| grep -Eq '^[[:blank:]]*authentication[[:blank:]]*:' "${yaml}" | ||
| } | ||
|
|
||
| # enable-auth.sh appends definitions to files it did not write. On a | ||
| # mounted config those appended definitions are duplicates the two parsers | ||
| # resolve in opposite directions — HugeConfig (commons-configuration) takes | ||
| # the first, snakeyaml takes the last — so Gremlin and REST can land on | ||
| # different authenticators with no error from either. Normalize both sides | ||
| # to one definition of the same authenticator here; enable-auth.sh's | ||
| # per-file guards then make its appends no-ops on anything already set. | ||
| align_auth_config() { | ||
| local rest_auth yaml_auth | ||
|
|
||
| rest_auth=$(get_prop "auth.authenticator" "${REST_SERVER_CONF}") | ||
| yaml_auth=$(get_yaml_authenticator) | ||
| if [[ -z "${yaml_auth}" ]] && has_yaml_authentication_block; then | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
At bedc21e, with a yaml block that has no authenticator and a rest-server.properties without In TinkerPop 3.5.1 Requested change: make this branch keep the bootstrap from writing just one side. Skip
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in bf2718f: the branch now fails the entrypoint (ERROR + nonzero exit under set -e) instead of warning while enable-auth.sh writes REST alone. Chose fail-fast over skip/add-to-yaml: skipping would start a PASSWORD deployment with auth silently half-applied, and appending an authenticator line into YAML by shell surgery risks corrupting the user's block. The unit test now asserts the refusal and runs enable-auth.sh against the same layout to prove it would write only REST (yaml untouched) — i.e. what the refusal prevents. Both shell suites pass locally. |
||
| # Refuse instead of bootstrapping one side: enable-auth.sh runs right | ||
| # after align and only touches the REST side, so continuing would put | ||
| # REST on StandardAuthenticator while Gremlin stays on TinkerPop's | ||
| # AllowAllAuthenticator default. Failing fast (rather than skipping | ||
| # enable-auth.sh) keeps a PASSWORD deployment from starting with | ||
| # authentication silently half-applied. | ||
| log "ERROR: gremlin-server.yaml carries an authentication block" \ | ||
| "without a readable authenticator; refusing to bootstrap" \ | ||
| "authentication one-sided. Add an 'authenticator:' entry to" \ | ||
| "the block or remove the block, then restart." | ||
| return 1 | ||
| fi | ||
| if [[ -n "${rest_auth}" && -n "${yaml_auth}" && "${rest_auth}" != "${yaml_auth}" ]]; then | ||
|
Adarsh-Me marked this conversation as resolved.
|
||
| log "WARN: REST and Gremlin name different authenticators" \ | ||
| "('${rest_auth}' vs '${yaml_auth}'); leaving both untouched" | ||
| return | ||
| fi | ||
| if [[ -z "${rest_auth}" && -z "${yaml_auth}" ]]; then | ||
| export AUTHENTICATOR_CLASS="org.apache.hugegraph.auth.StandardAuthenticator" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| elif [[ -n "${yaml_auth}" ]]; then | ||
| set_prop "auth.authenticator" "${yaml_auth}" "${REST_SERVER_CONF}" | ||
| else | ||
| export AUTHENTICATOR_CLASS="${rest_auth}" | ||
| fi | ||
| # auth.graph_store and the gremlin.graph flip are left to enable-auth.sh, | ||
| # which appends/rewrites only what is absent or still the plain default. | ||
| } | ||
|
|
||
| migrate_env() { | ||
|
|
@@ -147,6 +266,9 @@ elif [[ -n "${AUTH_TOKEN_SECRET_ENCODED}" ]]; then | |
| fi | ||
| if [[ -n "${PASSWORD:-}" ]]; then | ||
| set_prop "auth.admin_pa" "${PASSWORD}" "${REST_SERVER_CONF}" | ||
| # A refusal inside align_auth_config exits the entrypoint under set -e, | ||
| # so enable-auth.sh can never run one-sided after it. | ||
| align_auth_config | ||
| # This script is idempotent and must run outside the initialization guard: | ||
| # an upgrade can preserve the marker from an unauthenticated deployment. | ||
| ./bin/enable-auth.sh | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inblkis never cleared when the YAML mapping leavesauthentication:. For example, afterauthentication:has noauthenticator, a laterother: authenticator: com.example.Otheris accepted here and then written into REST byalign_auth_config, although it is notauthentication.authenticatorand Gremlin still uses its default. Track the block indentation or use the production YAML parser, and cover a following top-level mapping.