bitflicker64 commented on code in PR #3192: URL: https://github.com/apache/hugegraph/pull/3192#discussion_r4234584336
########## hugegraph-server/hugegraph-dist/docker/yamlscan.awk: ########## @@ -0,0 +1,794 @@ +# 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. +# +# yamlscan.awk -- does the top-level `authentication` mapping of a Gremlin Review Comment: Important: should not exist. This file reimplements part of SnakeYAML so it can guess what `Settings.read()` returns. Nearly every rule in the header (6b, 8b, 9, 10, 12, 13) says "Settings.read() loads X, so we must too", and each one came from a review round finding another shape the awk read differently. The codebase already has the real reader: `HugeGremlinServer.prepare()` calls `Settings.read(conf)`, and `org.apache.hugegraph.cmd` already has small CLI mains (`ConfDumper`, `InitStore`) that run off `lib/`. Something like this in that package does the whole job: ```java public class AuthConfState { public static void main(String[] args) throws Exception { Settings s = Settings.read(args[0]); if (s.authentication == null) { System.out.println("none"); return; } String a = s.authentication.authenticator; System.out.println(a == null || a.trim().isEmpty() ? "nameless" : "named"); } } ``` Run it as `java -cp "lib/*" org.apache.hugegraph.cmd.AuthConfState conf/gremlin-server.yaml`. It ships in the existing hugegraph-dist jar, so there is no new build artifact. It works on the JRE base image and in the tarball, and a document SnakeYAML rejects makes it exit nonzero, which is the refusal you want. That lets you delete this file, its two Dockerfile COPYs, and the 37 `yaml_case` shape cases in test-docker-entrypoint.sh, which then only need one case per output. ########## hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh: ########## @@ -34,23 +36,295 @@ GREMLIN_SERVER_CONF="gremlin-server.yaml" REST_SERVER_CONF="rest-server.properties" GRAPH_CONF="hugegraph.properties" +fail() { + echo "enable-auth.sh: $*" >&2 + exit 1 +} + +# Reading and writing .properties files goes through props.awk, the same helper +# the docker entrypoint uses, because the keys below can be spelled in every way +# java.util.Properties accepts: `=`/`:`/bare-whitespace separators, a form feed +# as whitespace, `\.` or `\u002e` for the dots, and LF, CRLF or CR line +# terminators. grep and sed see a different file. A legal +# `gremlin\u002egraph=org.apache.hugegraph.HugeFactory` matched no pattern at +# all, so the factory was never wrapped for auth even though both servers were +# told authentication was on -- and the CR byte that the previous pattern had +# to be handed a carriage return for is now handled by the reader itself. +# +# props.awk is packaged in this same bin/ directory by the release assembly, so +# it is present in the tarball and in the image. PROPS_AWK lets a caller point +# at a different copy; the entrypoint reads that same variable for itself. +for candidate in "${PROPS_AWK:-}" "${BIN}/props.awk" "${TOP}/props.awk"; do + if [[ -n "${candidate}" && -f "${candidate}" ]]; then + PROPS_AWK="${candidate}" + break + fi +done +[[ -n "${PROPS_AWK:-}" ]] || fail "props.awk not found beside this script" + +# The Gremlin half of the decision below has to be made by the same reader the +# entrypoint uses. yamlscan.awk is not in bin/: the Dockerfile places it in the +# install home, one above this script, which is where the image layout is +# mirrored in the test tree; PROPS_AWK and YAMLSCAN_AWK cover a caller that +# keeps it elsewhere. The release tarball carries no copy at all, so the +# fallback below has to answer on its own. +YAMLSCAN="" +for candidate in "${YAMLSCAN_AWK:-}" "${TOP}/yamlscan.awk" "${BIN}/yamlscan.awk"; do + if [[ -n "${candidate}" && -f "${candidate}" ]]; then + YAMLSCAN="${candidate}" + break + fi +done + +# props_get is the only reader used here, and it treats any nonzero status from +# props.awk as an error: 2 means the file could not be read at all, which must +# not be mistaken for "the key is not there" and answered with a write. +props_get() { + local status=0 value + value=$(PROPS_MODE=get PROPS_DECODED=1 PROPS_KEY="$1" PROPS_FILE="$2" \ + awk -f "${PROPS_AWK}" /dev/null) || status=$? + if (( status > 0 )); then + fail "cannot read $2" + fi + printf '%s' "${value}" +} + +# The character check for every class this script writes. Java class names need +# no properties escaping; anything else would have to go through the entrypoint's +# encoder first. It is one function rather than a pattern at each call site so +# the early refusal below and the write cannot drift apart. +check_class_name() { + case "$1" in + *[!A-Za-z0-9_\.\$]*) fail "refusing to write an unescaped value: $1" ;; + esac +} + +props_set() { + check_class_name "$2" + PROPS_MODE=set PROPS_KEY="$1" PROPS_VALUE_ENCODED="$2" PROPS_FILE="$3" \ + awk -f "${PROPS_AWK}" /dev/null || fail "cannot update $3" +} + +# Give `$3` its default `$2` for key `$1`, in place, unless it already has a +# value. Guarding on the key being present and appending was not the same +# question: +# `auth.authenticator=` and a bare `auth.authenticator` line both parse to the +# empty string (measured against java.util.Properties, which also strips the +# trailing blanks of `auth.authenticator= `), so a presence check reported +# them as +# answered and the append was skipped -- while the entrypoint's +# check_auth_sides, which asks for the value rather than the key, counted the +# same file as unconfigured. `loadAuthenticator("")` returns null, so REST then +# served without authentication next to a Gremlin that required it. +# +# props_set covers both shapes the guard had to split: with a definition +# present it replaces the first one where it stands (no duplicate for +# first-definition-wins to bury), with none present it appends. +# +# `$4` is the key's current value, read before the first write. Reading it +# here instead -- `[[ -n "$(props_get "$1" "$3")" ]]` -- discarded the status of +# the command substitution, because `[[` only looks at the text, so props_get's +# refusal of a file props.awk cannot read (an `include` directive) exited the +# subshell, the guard saw an empty string, and props_set then tried to write +# that same unreadable file. +ensure_rest_prop() { + [[ -n "$4" ]] && return 0 + props_set "$1" "$2" "$3" +} + # make a backup BAK_CONF="$TOP/conf-bak" if [ ! -d "$BAK_CONF" ]; then - mkdir -p "$BAK_CONF" - 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" + mkdir -p "$BAK_CONF" || fail "cannot create ${BAK_CONF}" + cp "${CONF}/${GREMLIN_SERVER_CONF}" "${BAK_CONF}/${GREMLIN_SERVER_CONF}.bak" || + fail "cannot back up ${GREMLIN_SERVER_CONF}" + cp "${CONF}/${REST_SERVER_CONF}" "${BAK_CONF}/${REST_SERVER_CONF}.bak" || + fail "cannot back up ${REST_SERVER_CONF}" + cp "${CONF}/graphs/${GRAPH_CONF}" "${BAK_CONF}/${GRAPH_CONF}.bak" || + fail "cannot back up ${GRAPH_CONF}" +fi + +# Both writes below skip a side that already carries a real value, so they are +# no-ops on a mounted config or a re-run. 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. That is why the REST side goes +# through ensure_rest_prop rather than a presence guard plus an append: a +# presence guard also lets a defined-but-empty key count as answered, and the +# appended default would then be the definition the server never reads. +# +# Appended with `>>` rather than `sed -i '$a\...'`: GNU sed's `$` address never +# matches when the file has no lines, so on an empty mounted config every append +# silently did nothing. `sed -i '$a'` also closed the previous last line for us, +# which `>>` does not, so a file without a trailing newline gets one first. +# +# Every write here has to be seen to succeed. The docker entrypoint runs this +# script and trusts its exit status, and a partially updated tree -- REST +# configured, yaml append refused by a read-only mounted file -- is exactly the +# one-sided state the entrypoint refuses to start with. Without errexit and +# these checks the script exited 0 on that half-done job. +append_lines() { + local file="$1" + shift + if [[ ! -w "${file}" ]]; then + fail "cannot append to ${file}: not writable" + fi + if [[ -s "${file}" && -n "$(tail -c 1 "${file}")" ]]; then + printf '\n' >> "${file}" || fail "cannot append to ${file}" + fi + printf '%s\n' "$@" >> "${file}" || fail "cannot append to ${file}" +} + +AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}" + +# Refused here, before the first write, rather than at props_set's check when the +# value reaches rest-server.properties. That check ran one statement after the +# yaml block below carried the same class into gremlin-server.yaml, so a value +# with a space in it left the yaml naming a class no server can load beside an +# untouched REST config -- the one-sided tree the entrypoint's check_auth_sides +# then stops the next boot on. It did not self-repair either: once the yaml +# reads as `named` the append is skipped, so re-running with the variable fixed +# or unset wrote the default to REST only, exited 0, and left the two servers +# authenticating to different classes. +check_class_name "${AUTHENTICATOR_CLASS}" + +# Does the Gremlin config carry a top-level `authentication` mapping, and does +# that mapping name an authenticator? This is the same question +# check_auth_sides answers, so it has to go to the same reader: a mapping is +# the server's only at column 0, comment text is not content, and the key may +# be quoted. grep asks it differently -- it sees only the bare spelling, so an +# operator's `"authentication":` block read as absent and a second default +# block was appended beside it, after which the two servers can resolve the key +# in opposite directions while REST keeps its existing authenticator. +# +# The answer lands in GREMLIN_AUTH, and there are three of them because two +# were not enough: whether a mapping exists says nothing about whether it +# names a class, and only the second one decides what is safe to write. +# none no mapping, so the default block below is ours to append; +# named the operator's mapping names a class; +# nameless a mapping that names none, or a document the reader refuses +# rather than guess about -- yamlscan.awk reports both as +# nameless and check_auth_sides stops the boot on them. +# unverifiable only the tarball fallback can produce this: grep saw the key +# but has no reader to tell the three cases apart. +GREMLIN_AUTH="" +gremlin_auth_state() { + local file="$1" + GREMLIN_AUTH="none" + [[ -f "${file}" ]] || return 0 + if [[ -n "${YAMLSCAN}" ]]; then + GREMLIN_AUTH=$(awk -f "${YAMLSCAN}" "${file}") || fail "cannot read ${file}" + return 0 + fi + # No parser in this layout (the plain release tarball). Match what grep can + # honestly answer here: a column-0 key in either quote style or none. The + # nested-mapping and comment cases are the ones that need the real reader, + # and the image, where the entrypoint runs this script, always has it. + if grep -Eq "^[\"']?authentication[\"']?[[:blank:]]*:" "${file}"; then + GREMLIN_AUTH="unverifiable" + fi +} + +# Writing `auth.authenticator` is the one-way door: REST starts enforcing on +# the next boot, and TinkerPop 3.5.1 resolves a mapping that names no +# authenticator to AllowAllAuthenticator, so Gremlin keeps answering without +# credentials. That is the same one-sided state this script exists to avoid, +# arrived at by a route the entrypoint does not guard -- enable-auth.sh ships +# in the release tarball, where nothing calls check_auth_sides first, so the +# refusal has to live here rather than lean on the caller. +gremlin_auth_state "${CONF}/${GREMLIN_SERVER_CONF}" + +if [[ "${GREMLIN_AUTH}" == "nameless" ]]; then + fail "${GREMLIN_SERVER_CONF} carries an authentication mapping that names no authenticator, or a shape the reader refuses; writing ${REST_SERVER_CONF} beside it would enforce on REST and leave Gremlin on its default. Name authentication.authenticator in that mapping, or drop the mapping and let this script write both sides." +fi +# Every value the decisions below act on is read here, before the first write. +# A plain assignment is what makes the read fatal: errexit sees the status of a +# command substitution on an assignment statement, while `[[ -n "$(props_get +# ...)" ]]` looked only at the text and discarded it. Reading rest-server.properties +# from ensure_rest_prop instead discovered an unreadable file one statement after +# the yaml block had been appended, leaving the yaml naming StandardAuthenticator +# beside a REST config with no `auth.authenticator` -- the one-sided tree this +# script exists to prevent, which in the tarball layout then refuses to repair +# itself: the appended block reads as a mapping, so a rerun after the operator +# fixes the include line stops on the unverifiable branch below. +# graphs/hugegraph.properties was read last of all, so an unreadable graph config +# let the run write both auth sides and leave the factory unwrapped. +REST_AUTHENTICATOR=$(props_get "auth.authenticator" "${CONF}/${REST_SERVER_CONF}") +REST_GRAPH_STORE=$(props_get "auth.graph_store" "${CONF}/${REST_SERVER_CONF}") +GRAPH_FACTORY=$(props_get "gremlin.graph" "${CONF}/graphs/${GRAPH_CONF}") - sed -i -e '$a\authentication: {' \ - -e '$a\ authenticator: org.apache.hugegraph.auth.StandardAuthenticator,' \ - -e '$a\ authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler,' \ - -e '$a\ config: {tokens: conf/rest-server.properties}' \ - -e '$a\}' ${CONF}/${GREMLIN_SERVER_CONF} +# Wrap the graph factory only when it really is the plain HugeFactory, which is +# a question about the decoded value, so it goes through the same reader. +# +# The trailing blanks come off before the comparison because the server reads +# the trimmed line: commons-configuration right-trims a property line before it +# resolves the class, so a mounted `gremlin.graph=org.apache.hugegraph.HugeFactory ` +# opens the graph through the plain factory exactly as if it carried no blanks. +# java.util.Properties by itself keeps them (measured against JDK 17), which is +# why the reader hands the value back verbatim. Comparing the untrimmed bytes +# left such a config unwrapped: authentication on both servers, and no +# HugeFactoryAuthProxy in front of the graph, which GraphManager only warns +# about. A factory that is not HugeFactory stays untouched either way. +# +# The trim happens here rather than next to the write below because the +# writability guard under it asks the same question, and two copies of it would +# be free to disagree. +while [[ "${GRAPH_FACTORY}" =~ [[:space:]]$ ]]; do + GRAPH_FACTORY="${GRAPH_FACTORY%?}" +done + +# A config the script can read but not write has to be refused here, before the +# yaml block below, for the same reason the reads moved up. `append_lines` +# guards only the file it appends to, so with a read-only rest-server.properties +# the yaml gained the authentication mapping, props_set then failed on the REST +# side, and the run exited 1 leaving the one-sided tree the comment above +# `append_lines` says must not happen -- and in the release tarball it does not +# repair itself: the appended block reads as a mapping, so the rerun after the +# operator restores write access stops on the unverifiable branch and the yaml +# has to be edited by hand. Only a file this run actually owes a write to is +# guarded, so a read-only config that already answers every key still passes. +# `-w` asks the shell rather than reading the mode because root can write a 0444 +# file, and refusing that would break the image it is meant to protect. +if [[ -z "${REST_AUTHENTICATOR}" || -z "${REST_GRAPH_STORE}" ]] && + ! [[ -w "${CONF}/${REST_SERVER_CONF}" ]]; then + fail "${REST_SERVER_CONF} is not writable and does not answer auth.authenticator and auth.graph_store, so this run owes it a write it cannot make; nothing was written" +fi +if [[ "${GRAPH_FACTORY}" == "org.apache.hugegraph.HugeFactory" ]] && + ! [[ -w "${CONF}/graphs/${GRAPH_CONF}" ]]; then + fail "graphs/${GRAPH_CONF} is not writable and gremlin.graph is the plain HugeFactory, so this run owes it a write it cannot make; nothing was written" +fi + +if [[ "${GREMLIN_AUTH}" == "unverifiable" ]] && [[ -z "${REST_AUTHENTICATOR}" ]]; then + # The operator already naming a class on the REST side is the one answer + # this layout can act on without a reader: ensure_rest_prop then has + # nothing to write, so both sides stay as the operator left them. + fail "${GREMLIN_SERVER_CONF} has a top-level authentication mapping and this layout has no yaml reader to tell whether it names an authenticator, while ${REST_SERVER_CONF} names none. Set auth.authenticator there yourself, or run this from the server image, which ships the reader." +fi + +# Only a column-0 `authentication` mapping is the Gremlin server's, which is the +# rule yamlscan.awk applies to decide the same thing for check_auth_sides. With +# a guard that disagreed on nesting, the entrypoint read the file as `none`, so +# parity held and it called this script, but the guard saw the nested key and +# skipped the append, writing the REST side only -- StandardAuthenticator on +# REST, TinkerPop's AllowAllAuthenticator on Gremlin. +if [[ "${GREMLIN_AUTH}" == "none" ]]; then + append_lines "${CONF}/${GREMLIN_SERVER_CONF}" \ + 'authentication: {' \ + " authenticator: ${AUTHENTICATOR_CLASS}," \ + ' authenticationHandler: org.apache.hugegraph.auth.WsAndHttpBasicAuthHandler,' \ + ' config: {tokens: conf/rest-server.properties}' \ + '}' +fi - sed -i -e '$a\auth.authenticator=org.apache.hugegraph.auth.StandardAuthenticator' \ - -e '$a\auth.graph_store=hugegraph' ${CONF}/${REST_SERVER_CONF} +ensure_rest_prop "auth.authenticator" "${AUTHENTICATOR_CLASS}" \ + "${CONF}/${REST_SERVER_CONF}" "${REST_AUTHENTICATOR}" +ensure_rest_prop "auth.graph_store" "hugegraph" \ + "${CONF}/${REST_SERVER_CONF}" "${REST_GRAPH_STORE}" - sed -i 's/gremlin.graph=org.apache.hugegraph.HugeFactory/gremlin.graph=org.apache.hugegraph.auth.HugeFactoryAuthProxy/g' ${CONF}/graphs/${GRAPH_CONF} +# The value was trimmed, and the writability of this file was refused before the Review Comment: Minor: local tidy. This comment only says where the code used to be. The script is now 175 comment lines against 130 lines of code, and a lot of that prose tells review history ("used to", "the previous pattern", "an earlier grep-shaped version", "that has cost this repo twice already"). That history belongs in the commit messages, which already record it. Keep one line per guard saying what it protects, delete this comment, and trim the rest the same way in enable-auth.sh, docker-entrypoint.sh and the props.awk/yamlscan.awk headers. ########## hugegraph-server/hugegraph-dist/src/assembly/static/bin/enable-auth.sh: ########## @@ -34,23 +36,295 @@ GREMLIN_SERVER_CONF="gremlin-server.yaml" REST_SERVER_CONF="rest-server.properties" GRAPH_CONF="hugegraph.properties" +fail() { + echo "enable-auth.sh: $*" >&2 + exit 1 +} + +# Reading and writing .properties files goes through props.awk, the same helper +# the docker entrypoint uses, because the keys below can be spelled in every way +# java.util.Properties accepts: `=`/`:`/bare-whitespace separators, a form feed +# as whitespace, `\.` or `\u002e` for the dots, and LF, CRLF or CR line +# terminators. grep and sed see a different file. A legal +# `gremlin\u002egraph=org.apache.hugegraph.HugeFactory` matched no pattern at +# all, so the factory was never wrapped for auth even though both servers were +# told authentication was on -- and the CR byte that the previous pattern had +# to be handed a carriage return for is now handled by the reader itself. +# +# props.awk is packaged in this same bin/ directory by the release assembly, so +# it is present in the tarball and in the image. PROPS_AWK lets a caller point +# at a different copy; the entrypoint reads that same variable for itself. +for candidate in "${PROPS_AWK:-}" "${BIN}/props.awk" "${TOP}/props.awk"; do + if [[ -n "${candidate}" && -f "${candidate}" ]]; then + PROPS_AWK="${candidate}" + break + fi +done +[[ -n "${PROPS_AWK:-}" ]] || fail "props.awk not found beside this script" + +# The Gremlin half of the decision below has to be made by the same reader the +# entrypoint uses. yamlscan.awk is not in bin/: the Dockerfile places it in the +# install home, one above this script, which is where the image layout is +# mirrored in the test tree; PROPS_AWK and YAMLSCAN_AWK cover a caller that +# keeps it elsewhere. The release tarball carries no copy at all, so the +# fallback below has to answer on its own. +YAMLSCAN="" +for candidate in "${YAMLSCAN_AWK:-}" "${TOP}/yamlscan.awk" "${BIN}/yamlscan.awk"; do + if [[ -n "${candidate}" && -f "${candidate}" ]]; then + YAMLSCAN="${candidate}" + break + fi +done + +# props_get is the only reader used here, and it treats any nonzero status from +# props.awk as an error: 2 means the file could not be read at all, which must +# not be mistaken for "the key is not there" and answered with a write. +props_get() { + local status=0 value + value=$(PROPS_MODE=get PROPS_DECODED=1 PROPS_KEY="$1" PROPS_FILE="$2" \ + awk -f "${PROPS_AWK}" /dev/null) || status=$? + if (( status > 0 )); then + fail "cannot read $2" + fi + printf '%s' "${value}" +} + +# The character check for every class this script writes. Java class names need +# no properties escaping; anything else would have to go through the entrypoint's +# encoder first. It is one function rather than a pattern at each call site so +# the early refusal below and the write cannot drift apart. +check_class_name() { + case "$1" in + *[!A-Za-z0-9_\.\$]*) fail "refusing to write an unescaped value: $1" ;; + esac +} + +props_set() { + check_class_name "$2" + PROPS_MODE=set PROPS_KEY="$1" PROPS_VALUE_ENCODED="$2" PROPS_FILE="$3" \ + awk -f "${PROPS_AWK}" /dev/null || fail "cannot update $3" +} + +# Give `$3` its default `$2` for key `$1`, in place, unless it already has a +# value. Guarding on the key being present and appending was not the same +# question: +# `auth.authenticator=` and a bare `auth.authenticator` line both parse to the +# empty string (measured against java.util.Properties, which also strips the +# trailing blanks of `auth.authenticator= `), so a presence check reported +# them as +# answered and the append was skipped -- while the entrypoint's +# check_auth_sides, which asks for the value rather than the key, counted the +# same file as unconfigured. `loadAuthenticator("")` returns null, so REST then +# served without authentication next to a Gremlin that required it. +# +# props_set covers both shapes the guard had to split: with a definition +# present it replaces the first one where it stands (no duplicate for +# first-definition-wins to bury), with none present it appends. +# +# `$4` is the key's current value, read before the first write. Reading it +# here instead -- `[[ -n "$(props_get "$1" "$3")" ]]` -- discarded the status of +# the command substitution, because `[[` only looks at the text, so props_get's +# refusal of a file props.awk cannot read (an `include` directive) exited the +# subshell, the guard saw an empty string, and props_set then tried to write +# that same unreadable file. +ensure_rest_prop() { + [[ -n "$4" ]] && return 0 + props_set "$1" "$2" "$3" +} + # make a backup BAK_CONF="$TOP/conf-bak" if [ ! -d "$BAK_CONF" ]; then - mkdir -p "$BAK_CONF" - 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" + mkdir -p "$BAK_CONF" || fail "cannot create ${BAK_CONF}" + cp "${CONF}/${GREMLIN_SERVER_CONF}" "${BAK_CONF}/${GREMLIN_SERVER_CONF}.bak" || + fail "cannot back up ${GREMLIN_SERVER_CONF}" + cp "${CONF}/${REST_SERVER_CONF}" "${BAK_CONF}/${REST_SERVER_CONF}.bak" || + fail "cannot back up ${REST_SERVER_CONF}" + cp "${CONF}/graphs/${GRAPH_CONF}" "${BAK_CONF}/${GRAPH_CONF}.bak" || + fail "cannot back up ${GRAPH_CONF}" +fi + +# Both writes below skip a side that already carries a real value, so they are +# no-ops on a mounted config or a re-run. 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. That is why the REST side goes +# through ensure_rest_prop rather than a presence guard plus an append: a +# presence guard also lets a defined-but-empty key count as answered, and the +# appended default would then be the definition the server never reads. +# +# Appended with `>>` rather than `sed -i '$a\...'`: GNU sed's `$` address never +# matches when the file has no lines, so on an empty mounted config every append +# silently did nothing. `sed -i '$a'` also closed the previous last line for us, +# which `>>` does not, so a file without a trailing newline gets one first. +# +# Every write here has to be seen to succeed. The docker entrypoint runs this +# script and trusts its exit status, and a partially updated tree -- REST +# configured, yaml append refused by a read-only mounted file -- is exactly the +# one-sided state the entrypoint refuses to start with. Without errexit and +# these checks the script exited 0 on that half-done job. +append_lines() { + local file="$1" + shift + if [[ ! -w "${file}" ]]; then + fail "cannot append to ${file}: not writable" + fi + if [[ -s "${file}" && -n "$(tail -c 1 "${file}")" ]]; then + printf '\n' >> "${file}" || fail "cannot append to ${file}" + fi + printf '%s\n' "$@" >> "${file}" || fail "cannot append to ${file}" +} + +AUTHENTICATOR_CLASS="${AUTHENTICATOR_CLASS:-org.apache.hugegraph.auth.StandardAuthenticator}" + +# Refused here, before the first write, rather than at props_set's check when the +# value reaches rest-server.properties. That check ran one statement after the +# yaml block below carried the same class into gremlin-server.yaml, so a value +# with a space in it left the yaml naming a class no server can load beside an +# untouched REST config -- the one-sided tree the entrypoint's check_auth_sides +# then stops the next boot on. It did not self-repair either: once the yaml +# reads as `named` the append is skipped, so re-running with the variable fixed +# or unset wrote the default to REST only, exited 0, and left the two servers +# authenticating to different classes. +check_class_name "${AUTHENTICATOR_CLASS}" + +# Does the Gremlin config carry a top-level `authentication` mapping, and does +# that mapping name an authenticator? This is the same question +# check_auth_sides answers, so it has to go to the same reader: a mapping is +# the server's only at column 0, comment text is not content, and the key may +# be quoted. grep asks it differently -- it sees only the bare spelling, so an +# operator's `"authentication":` block read as absent and a second default +# block was appended beside it, after which the two servers can resolve the key +# in opposite directions while REST keeps its existing authenticator. +# +# The answer lands in GREMLIN_AUTH, and there are three of them because two +# were not enough: whether a mapping exists says nothing about whether it +# names a class, and only the second one decides what is safe to write. +# none no mapping, so the default block below is ours to append; +# named the operator's mapping names a class; +# nameless a mapping that names none, or a document the reader refuses +# rather than guess about -- yamlscan.awk reports both as +# nameless and check_auth_sides stops the boot on them. +# unverifiable only the tarball fallback can produce this: grep saw the key +# but has no reader to tell the three cases apart. +GREMLIN_AUTH="" +gremlin_auth_state() { + local file="$1" + GREMLIN_AUTH="none" + [[ -f "${file}" ]] || return 0 + if [[ -n "${YAMLSCAN}" ]]; then + GREMLIN_AUTH=$(awk -f "${YAMLSCAN}" "${file}") || fail "cannot read ${file}" + return 0 + fi + # No parser in this layout (the plain release tarball). Match what grep can + # honestly answer here: a column-0 key in either quote style or none. The + # nested-mapping and comment cases are the ones that need the real reader, + # and the image, where the entrypoint runs this script, always has it. + if grep -Eq "^[\"']?authentication[\"']?[[:blank:]]*:" "${file}"; then + GREMLIN_AUTH="unverifiable" Review Comment: Important: replace or collapse. The fourth state, `unverifiable`, exists only because yamlscan.awk is copied into the image home by the Dockerfiles while props.awk ships in `bin/` through the assembly. So the tarball has no yaml reader, and this script carries a grep fallback, an extra state, a separate refusal branch (lines 298-304) and a three-path search loop for the scanner. If the yaml reader ships wherever props.awk does, this goes away: either a jar main in `lib/` (see the yamlscan.awk comment) or, at minimum, yamlscan.awk moved to `src/assembly/static/bin/` next to props.awk. Then `gremlin_auth_state` becomes one line, `GREMLIN_AUTH=$(awk -f "${BIN}/yamlscan.awk" "${file}") || fail "cannot read ${file}"`. The `unverifiable` branch, the grep fallback, the `YAMLSCAN` candidate loop here and in the entrypoint, and both Dockerfile `COPY ... yamlscan.awk` lines can all be deleted. -- 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]
