bitflicker64 commented on code in PR #526:
URL: https://github.com/apache/hugegraph-doc/pull/526#discussion_r4257616049
##########
content/en/docs/config/config-option.md:
##########
@@ -69,6 +69,7 @@ Corresponding configuration file `rest-server.properties`
| auth.authenticator |
| The class path of authenticator implementation. e.g.,
org.apache.hugegraph.auth.StandardAuthenticator, or a custom implementation.
|
| auth.graph_store | hugegraph
| The name of graph used to store authentication information, like
users, only for org.apache.hugegraph.auth.StandardAuthenticator.
|
| auth.admin_pa | pa | Used when the Server startup path initializes admin.
Explicitly set a strong password before production deployment; the public
default is `pa`. On local persistent backends, `init-store.sh` prompts for the
initial password. |
+| auth.admin_pa_file | | Path of a file holding the initial password for the
built-in admin account. The file is read as raw UTF-8 with one trailing line
break removed, so spaces, backslashes and non-ASCII characters need no
properties escaping. When set, it takes precedence over `auth.admin_pa` and the
`init-store.sh` password prompt. A missing, empty or non-UTF-8 file stops
startup. Takes effect on first startup. |
Review Comment:
Minor: two behaviours of the current Server head are missing from this row
and its CN counterpart. `readAdminPasswordFile` (StandardAuthenticator.java in
apache/hugegraph#3334 at 128c0b3) also strips a leading UTF-8 BOM, and it
rejects a file larger than 4096 bytes (`MAX_ADMIN_PA_FILE_BYTES`) with "is
larger than 4096 bytes". The failure list here names only missing, empty and
non-UTF-8 files. Both were added in the third commit of that PR, after this doc
commit was written.
Requested change: mention the BOM alongside the trailing line break, and add
the 4096-byte limit to the conditions that stop startup, in both the EN and CN
rows (and the BOM in the two `rest-server.properties` comments, to match the
shipped `rest-server.properties`).
##########
content/cn/docs/config/config-option.md:
##########
@@ -69,6 +69,7 @@ search_boost: 1.5
| auth.authenticator |
| The class path of authenticator implementation. e.g.,
org.apache.hugegraph.auth.StandardAuthenticator, or a custom implementation.
|
| auth.graph_store | hugegraph
| The name of graph used to store authentication information, like
users, only for org.apache.hugegraph.auth.StandardAuthenticator.
|
| auth.admin_pa | pa
| Server 启动路径初始化内置 admin 账户时使用，默认值为公开的
`pa`，生产部署前必须显式设置强密码。本地持久化后端首次初始化由 `init-store.sh` 交互式读取密码。
|
+| auth.admin_pa_file |
| 存放内置 admin 账户初始密码的文件路径。文件按原始 UTF-8 读取，并去掉末尾的一个换行符，因此空格、反斜杠和非
ASCII 字符无需按 properties 语法转义。设置后优先于 `auth.admin_pa` 和 `init-store.sh`
的交互式密码输入。文件不存在、为空或不是合法 UTF-8 时启动失败。仅在首次启动时生效。 |
Review Comment:
Important: "仅在首次启动时生效" (takes effect only on first startup) is stronger than
the English row and contradicts the Server code for PD/hstore. The file is read
on every start there, so deleting it after the first boot stops the Server from
starting.
Evidence: apache/hugegraph#3334 head 128c0b3:
`GraphManager.loadMetaFromPD()` (GraphManager.java:375-378) evaluates
`StandardAuthenticator.initialAdminPassword(conf)` as the argument of
`initAdminUserIfNeeded(...)`, so the file is read before the admin-exists
check, on every PD start. `readAdminPasswordFile` throws `ConfigException` when
the file cannot be read. The option description in
`ServerOptions.ADMIN_PA_FILE` says the same: "PD and non-persistent backends
read it on every start, so keep it readable while this option is set."
Requested change: state that the password is only applied when the admin
account is first created, and that with PD (hstore) and non-persistent backends
the file is read on every start and must stay readable while the option is set.
Apply the same to `content/cn/docs/config/config-backend-hstore.md` line 151
and the properties comment in `content/cn/docs/config/config-authentication.md`.
##########
content/en/docs/config/config-option.md:
##########
@@ -69,6 +69,7 @@ Corresponding configuration file `rest-server.properties`
| auth.authenticator |
| The class path of authenticator implementation. e.g.,
org.apache.hugegraph.auth.StandardAuthenticator, or a custom implementation.
|
| auth.graph_store | hugegraph
| The name of graph used to store authentication information, like
users, only for org.apache.hugegraph.auth.StandardAuthenticator.
|
| auth.admin_pa | pa | Used when the Server startup path initializes admin.
Explicitly set a strong password before production deployment; the public
default is `pa`. On local persistent backends, `init-store.sh` prompts for the
initial password. |
+| auth.admin_pa_file | | Path of a file holding the initial password for the
built-in admin account. The file is read as raw UTF-8 with one trailing line
break removed, so spaces, backslashes and non-ASCII characters need no
properties escaping. When set, it takes precedence over `auth.admin_pa` and the
`init-store.sh` password prompt. A missing, empty or non-UTF-8 file stops
startup. Takes effect on first startup. |
Review Comment:
Important: "Takes effect on first startup" is only half of the contract, and
the missing half breaks restarts on PD/hstore. The password is applied once,
but the file is read on every Server start when `usePD=true` (and on
non-persistent backends). An operator who reads this row and removes the secret
mount after the first boot gets a Server that refuses to start.
Evidence: apache/hugegraph#3334 head 128c0b3:
`GraphManager.loadMetaFromPD()` (GraphManager.java:375-378) evaluates
`StandardAuthenticator.initialAdminPassword(conf)` as the argument of
`initAdminUserIfNeeded(...)`, so the file is read before the admin-exists
check, on every PD start. `readAdminPasswordFile` throws `ConfigException` when
the file cannot be read. The option description in
`ServerOptions.ADMIN_PA_FILE` says the same: "PD and non-persistent backends
read it on every start, so keep it readable while this option is set."
Requested change: say that the password is only applied when the admin
account is first created, but that with PD (hstore) and non-persistent backends
the file is read on every start and must stay readable for as long as the
option is set. The same sentence is needed in `config-backend-hstore.md` (line
179, "which is read at this point"), since that is the page for exactly the
deployment that re-reads the file, and in the `rest-server.properties` comment
in `config-authentication.md`.
--
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]
