Adarsh-Me opened a new pull request, #3192:
URL: https://github.com/apache/hugegraph/pull/3192

   ## 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.sh` previously rewrote `rest-server.properties` / 
`hugegraph.properties` with `grep`/`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 new `props.awk` loaded by the entrypoint, which 
implements the `java.util.Properties` line 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 `ps` output.** The old `sed` rewrite 
interpolated the encoded value into sed's command line, so when a key already 
existed (mounted or persisted config) the password was visible in `ps`. 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, custom `gremlin.graph` 
factories are preserved, and the authenticator class can be overridden via 
`AUTHENTICATOR_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 
`StandardAuthenticator` split the pair. When both sides name genuinely 
different authenticators, it logs a WARN and leaves both untouched instead of 
silently splitting them.
   
   ## Implementation notes
   
   - I chose a shell/awk implementation of the properties grammar over the 
issue's proposed Java `ConfigTool` CLI: 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.awk` ships in both server images (Dockerfile COPY) and in the test 
sandbox; CI already runs the unit suite via `docker-build-ci.yml`.
   
   ## How was this tested
   
   - `docker/test/test-docker-entrypoint.sh` extended 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.sh` full harness passes end-to-end (secret 
round-trips incl. backslash/space/trailing-space secrets, enable-auth call 
counting unchanged).
   - enable-auth.sh manually exercised against: fresh default config, 
idempotent re-run, mounted config with custom authenticator (no duplicates), 
and custom `gremlin.graph` factory (preserved).
   
   ## Code Review Handbook
   
   - `props.awk` is 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).
   - The value of get is intentionally *not* unescaped: the entrypoint 
re-writes secrets it just read, and unescape-then-re-encode would double-escape 
backslashes (caught by the complex-secret round-trip in 
`docker-entrypoint-test.sh`).
   - The escaped-key unit case is the reported `auth\.admin_pa` scenario: the 
old grep could not match it, so the append created a duplicate and HugeConfig 
silently kept `pa`.
   


-- 
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]

Reply via email to