bitflicker64 commented on code in PR #3334:
URL: https://github.com/apache/hugegraph/pull/3334#discussion_r4266300035


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -374,7 +374,8 @@ private void loadMetaFromPD() {
         this.initK8sManagerIfNeeded(conf);
         if (shouldBootstrapAdmin(this.authenticator,
                                  conf.get(ServerOptions.AUTH_REMOTE_URL))) {
-            this.initAdminUserIfNeeded(conf.get(ServerOptions.ADMIN_PA));
+            this.initAdminUserIfNeeded(
+                    StandardAuthenticator.initialAdminPassword(conf));

Review Comment:
   Minor: no test runs this call with `auth.admin_pa_file` set, and it is the 
only link between the option and the PD bootstrap, the path it mostly serves 
(hstore with a mounted Secret). The review of 128c0b3 noted the gap; it still 
holds at 7ba00bd. `loadMetaFromPD()` runs only from the constructor when 
`usePD=true`, no unit test builds a `GraphManager` that way, and the hstore API 
job starts the Server with the option unset.
   
   Evidence (Linux x86_64, OpenJDK 17.0.20.1): with these two lines put back to 
`this.initAdminUserIfNeeded(conf.get(ServerOptions.ADMIN_PA));`, 
`AdminPasswordFileTest`, `GraphManagerAdminInitTest` and `InitStoreConfigTest` 
still pass all 42 of their tests.
   
   Requested change: move the `shouldBootstrapAdmin` guard and this call into a 
small method that takes the config, and cover it in 
`GraphManagerAdminInitTest`, which already swaps in an `AuthMetaManager` over a 
mock driver: one case with a password file whose content 
`StringEncoding.checkPassword` accepts against the stored admin hash, and one 
with `auth.remote_url` set and a missing file, which must not be read.



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