This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new b0aa708cd0 [#11836] fix(server): Return visible service admins as list
(#11843)
b0aa708cd0 is described below
commit b0aa708cd0409f0cd126d64b910e2f1d0996bc55
Author: roryqi <[email protected]>
AuthorDate: Fri Jul 3 14:17:23 2026 +0800
[#11836] fix(server): Return visible service admins as list (#11843)
### What changes were proposed in this pull request?
This PR updates the config servlet to return
`gravitino.authorization.serviceAdmins` as a list when it is exposed
through `gravitino.server.visibleConfigs`.
It also adds regression tests for:
- a single configured service admin returned as a JSON array
- `visibleConfigs` including service admins while the service admins
value is not configured
### Why are the changes needed?
When
`gravitino.server.visibleConfigs=gravitino.authorization.serviceAdmins`
is configured, `/configs` currently returns
`gravitino.authorization.serviceAdmins` as a raw string for a single
value. This is inconsistent with the typed config value and may break
clients expecting an array.
Fix: #11836
### Does this PR introduce _any_ user-facing change?
Yes. `/configs` now returns `gravitino.authorization.serviceAdmins` as a
JSON array when it is exposed by `gravitino.server.visibleConfigs`.
### How was this patch tested?
- `./gradlew :server:test --tests
org.apache.gravitino.server.web.TestConfigServlet`
- `git diff --check`
---
.../apache/gravitino/server/web/ConfigServlet.java | 38 +++++++++++++++++++--
.../gravitino/server/web/TestConfigServlet.java | 39 ++++++++++++++++++++++
2 files changed, 75 insertions(+), 2 deletions(-)
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/ConfigServlet.java
b/server/src/main/java/org/apache/gravitino/server/web/ConfigServlet.java
index b8bdbfaa1b..9ea12fa474 100644
--- a/server/src/main/java/org/apache/gravitino/server/web/ConfigServlet.java
+++ b/server/src/main/java/org/apache/gravitino/server/web/ConfigServlet.java
@@ -18,10 +18,14 @@
*/
package org.apache.gravitino.server.web;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import java.io.IOException;
import java.io.PrintWriter;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
import java.util.List;
import java.util.Map;
import javax.servlet.http.HttpServlet;
@@ -52,6 +56,9 @@ public class ConfigServlet extends HttpServlet {
ImmutableSet.of(
Configs.AUTHENTICATORS, Configs.ENABLE_AUTHORIZATION,
Configs.SCHEMA_SEPARATOR);
+ private static final ImmutableMap<String, ConfigEntry<?>>
visibleConfigEntries =
+ buildVisibleConfigEntries();
+
private final Map<String, Object> configs = Maps.newHashMap();
public ConfigServlet(ServerConfig serverConfig) {
@@ -80,9 +87,12 @@ public class ConfigServlet extends HttpServlet {
for (String config : visibleConfigs) {
String configValue = serverConfig.getRawString(config);
- if (configValue != null) {
- configs.put(config, configValue);
+ if (configValue == null) {
+ continue;
}
+
+ ConfigEntry<?> configEntry = visibleConfigEntries.get(config);
+ configs.put(config, configEntry == null ? configValue :
serverConfig.get(configEntry));
}
}
@@ -107,6 +117,30 @@ public class ConfigServlet extends HttpServlet {
}
}
+ private static ImmutableMap<String, ConfigEntry<?>>
buildVisibleConfigEntries() {
+ ImmutableMap.Builder<String, ConfigEntry<?>> builder =
ImmutableMap.builder();
+ for (Class<?> configClass :
+ ImmutableList.of(
+ Configs.class, ServerConfig.class, OAuthConfig.class,
JettyServerConfig.class)) {
+ for (Field field : configClass.getFields()) {
+ if (!Modifier.isStatic(field.getModifiers())
+ || !ConfigEntry.class.isAssignableFrom(field.getType())) {
+ continue;
+ }
+
+ try {
+ ConfigEntry<?> configEntry = (ConfigEntry<?>) field.get(null);
+ builder.put(configEntry.getKey(), configEntry);
+ } catch (IllegalAccessException e) {
+ throw new IllegalStateException(
+ "Failed to access config entry " + configClass.getName() + "." +
field.getName(), e);
+ }
+ }
+ }
+
+ return builder.build();
+ }
+
private void sendErrorResponse(HttpServletResponse res, String message) {
try (PrintWriter writer = res.getWriter()) {
res.setContentType("application/json;charset=utf-8");
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/TestConfigServlet.java
b/server/src/test/java/org/apache/gravitino/server/web/TestConfigServlet.java
index a3ddee599f..10b163e706 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/TestConfigServlet.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/TestConfigServlet.java
@@ -30,6 +30,7 @@ import com.google.common.collect.Lists;
import java.io.IOException;
import java.io.PrintWriter;
import java.util.Map;
+import java.util.Properties;
import javax.servlet.http.HttpServletResponse;
import org.apache.gravitino.Configs;
import org.apache.gravitino.auth.AuthenticatorType;
@@ -101,6 +102,44 @@ public class TestConfigServlet {
Assertions.assertEquals("test", configs.get(customConfig.getKey()));
}
+ @Test
+ public void testConfigServletWithVisibleServiceAdminsFromProperties() throws
Exception {
+ ServerConfig serverConfig = new ServerConfig(false);
+ Properties properties = new Properties();
+ properties.setProperty(Configs.VISIBLE_CONFIGS.getKey(),
Configs.SERVICE_ADMINS.getKey());
+ properties.setProperty(Configs.SERVICE_ADMINS.getKey(), "admin1,admin2");
+ serverConfig.loadFromProperties(properties);
+
+ Map<String, Object> configs = fetchConfigs(serverConfig);
+ Assertions.assertEquals(
+ Lists.newArrayList("admin1", "admin2"),
configs.get(Configs.SERVICE_ADMINS.getKey()));
+ }
+
+ @Test
+ public void testConfigServletWithVisibleTypedConfigFromProperties() throws
Exception {
+ ServerConfig serverConfig = new ServerConfig(false);
+ Properties properties = new Properties();
+ properties.setProperty(
+ Configs.VISIBLE_CONFIGS.getKey(),
Configs.REST_API_EXTENSION_PACKAGES.getKey());
+ properties.setProperty(Configs.REST_API_EXTENSION_PACKAGES.getKey(),
"pkg.one");
+ serverConfig.loadFromProperties(properties);
+
+ Map<String, Object> configs = fetchConfigs(serverConfig);
+ Assertions.assertEquals(
+ Lists.newArrayList("pkg.one"),
configs.get(Configs.REST_API_EXTENSION_PACKAGES.getKey()));
+ }
+
+ @Test
+ public void testConfigServletWithVisibleServiceAdminsButNoValue() throws
Exception {
+ ServerConfig serverConfig = new ServerConfig(false);
+ Properties properties = new Properties();
+ properties.setProperty(Configs.VISIBLE_CONFIGS.getKey(),
Configs.SERVICE_ADMINS.getKey());
+ serverConfig.loadFromProperties(properties);
+
+ Map<String, Object> configs = fetchConfigs(serverConfig);
+
Assertions.assertFalse(configs.containsKey(Configs.SERVICE_ADMINS.getKey()));
+ }
+
@Test
public void testConfigServletWithOAuthJwksValidator() throws Exception {
ServerConfig serverConfig = new ServerConfig();
