yuqi1129 commented on code in PR #11558:
URL: https://github.com/apache/gravitino/pull/11558#discussion_r3450451670


##########
lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java:
##########
@@ -66,7 +70,13 @@ public void serviceInit(Map<String, String> properties, 
boolean auxMode) {
     LanceConfig lanceConfig = new LanceConfig(properties);
     JettyServerConfig serverConfig = JettyServerConfig.fromConfig(lanceConfig);
 
-    server = new JettyServer();
+    server =
+        new JettyServer() {
+          @Override
+          protected javax.servlet.Filter createAuthenticationFilter() {
+            return new LanceAuthenticationFilter();
+          }
+        };

Review Comment:
   Should Lance also register root-level health aliases, matching Gravitino and 
Iceberg REST? Iceberg adds `new HealthAliasServlet("/iceberg")` for `/health/*` 
and `/health.html`, so common probes can use `/health`, `/health/live`, 
`/health/ready`, and `/health.html`. This PR only exposes `/lance/health*`; 
adding `HealthAliasServlet("/lance")` after registering the Lance servlet would 
make Lance consistent with the existing health endpoint behavior.



##########
lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/LanceRESTService.java:
##########
@@ -66,7 +70,13 @@ public void serviceInit(Map<String, String> properties, 
boolean auxMode) {
     LanceConfig lanceConfig = new LanceConfig(properties);
     JettyServerConfig serverConfig = JettyServerConfig.fromConfig(lanceConfig);
 
-    server = new JettyServer();
+    server =
+        new JettyServer() {
+          @Override
+          protected javax.servlet.Filter createAuthenticationFilter() {
+            return new LanceAuthenticationFilter();

Review Comment:
   When adding a Lance-specific auth filter for `/lance/health*`, please also 
pass the same Lance health matcher to `HttpAuditFilter` below. Iceberg REST 
uses `IcebergHealthCheckPathMatcher` for both auth and audit filtering. Without 
the audit-side matcher, `/lance/health/ready` returning 503 will be treated as 
a normal request failure and emit audit/failure events, because the default 
matcher only recognizes `/health*` and `/api/health*`.



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

Reply via email to