Skip to content

fix(docker): make auth bootstrap safe for mounted and upgraded configs - #3192

Open
Adarsh-Me wants to merge 23 commits into
apache:masterfrom
Adarsh-Me:fix-entrypoint-auth-bootstrap
Open

Adarsh-Me wants to merge 23 commits into
apache:masterfrom
Adarsh-Me:fix-entrypoint-auth-bootstrap

Conversation

@Adarsh-Me

@Adarsh-Me Adarsh-Me commented Sep 3, 2026 •

Copy link
Copy Markdown

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.

Visual summary

Docker auth bootstrap

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Docker entrypoint auth bootstrap is unsafe for mounted and upgraded configs

5 participants