Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is changing
Closes #3133 (the parts still open on master
3681148, since #3119 landed the rest).Properties rewriting now implements the Java grammar.
docker-entrypoint.shpreviously rewroterest-server.properties/hugegraph.propertieswithgrep/sed, which disagrees with HugeConfig on mounted or upgraded configs: backslash-escaped keys,:/whitespace separators, line continuations, and duplicate definitions are all parsed differently. The property logic moves to a newprops.awkloaded by the entrypoint, which implements thejava.util.Propertiesline grammar (comments, both separators, continuations, backslash escapes, first-definition-wins duplicates) and rewrites the first definition in place while keeping every untouched line byte-for-byte.PASSWORD no longer appears in
psoutput. The oldsedrewrite interpolated the encoded value into sed's command line, so when a key already existed (mounted or persisted config) the password was visible inps. Values now travel through an environment variable into awk, never through argv.enable-auth.sh appends are per-file guarded. The old script appended authentication definitions whenever
conf-bak/was absent. On a config it did not write, that created duplicate definitions that the properties parser (first definition wins) and snakeyaml (last definition wins) resolved in opposite directions — Gremlin and REST could land on different authenticators with no error from either. Now each append runs only when its file lacks the definition (or still has it commented out), re-runs are idempotent, customgremlin.graphfactories are preserved, and the authenticator class can be overridden viaAUTHENTICATOR_CLASS.The entrypoint aligns both sides before enabling auth. If the yaml declares an authenticator but the properties file does not (or vice versa), the entrypoint propagates it to the other side instead of letting the default
StandardAuthenticatorsplit the pair. When both sides name genuinely different authenticators, it logs a WARN and leaves both untouched instead of silently splitting them.Implementation notes
ConfigToolCLI: it delivers the same parser-agreement contract with a much smaller footprint and no new build artifact. Happy to rework toward the ConfigTool if reviewers prefer that direction.props.awkships in both server images (Dockerfile COPY) and in the test sandbox; CI already runs the unit suite viadocker-build-ci.yml.How was this tested
docker/test/test-docker-entrypoint.shextended with cases for: escaped-key definitions rewritten in place, continuation lines consumed with the key they belong to, get-mode separator/continuation/duplicate semantics, and appends when the key only exists commented out. All pass.docker-entrypoint-test.shfull harness passes end-to-end (secret round-trips incl. backslash/space/trailing-space secrets, enable-auth call counting unchanged).gremlin.graphfactory (preserved).Code Review Handbook
props.awkis the core: block model (comment lines and logical entries), first-definition-wins, raw-value round-trip (get returns the on-disk escaped form so feeding it back into set is byte-exact).docker-entrypoint-test.sh).auth\.admin_pascenario: the old grep could not match it, so the append created a duplicate and HugeConfig silently keptpa.Visual summary