Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#6499] improvement(server): Support custom configuration options in the REST API /configs #6501

Merged
merged 2 commits into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions core/src/main/java/org/apache/gravitino/Configs.java
Original file line number Diff line number Diff line change
Expand Up @@ -310,4 +310,12 @@ private Configs() {}
.version(ConfigConstants.VERSION_0_7_0)
.stringConf()
.createWithDefault(SimpleFormatterV2.class.getName());

public static final ConfigEntry<List<String>> VISIBLE_CONFIGS =
new ConfigBuilder("gravitino.server.visibleConfigs")
.doc("List of configs that are visible in the config servlet")
.version(ConfigConstants.VERSION_0_9_0)
.stringConf()
.toSequence()
.createWithDefault(Collections.emptyList());
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,7 @@ private ConfigConstants() {}

/** The version number for the 0.8.0 release. */
public static final String VERSION_0_8_0 = "0.8.0";

/** The version number for the 0.9.0 release. */
public static final String VERSION_0_9_0 = "0.9.0";
}
1 change: 1 addition & 0 deletions docs/gravitino-server-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ The `gravitino.conf` file lists the configuration items in the following table.
| `gravitino.server.shutdown.timeout` | Time in milliseconds to gracefully shut down of the Gravitino webserver. | `3000` | No | 0.2.0 |
| `gravitino.server.webserver.customFilters` | Comma-separated list of filter class names to apply to the API. | (none) | No | 0.4.0 |
| `gravitino.server.rest.extensionPackages` | Comma-separated list of REST API packages to expand | (none) | No | 0.6.0-incubating |
| `gravitino.server.visibleConfigs` | List of configs that are visible in the config servlet | (none) | No | 0.9.0-incubating |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:) This is a pretty honest name. It can be parsed as "... I have many configs, but I can only show you some ...".


The filter in the customFilters should be a standard javax servlet filter.
You can also specify filter parameters by setting configuration entries of the form `gravitino.server.webserver.<class name of filter>.param.<param name>=<value>`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.common.collect.Maps;
import java.io.IOException;
import java.io.PrintWriter;
import java.util.List;
import java.util.Map;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
Expand Down Expand Up @@ -58,6 +59,15 @@ public ConfigServlet(ServerConfig serverConfig) {
configs.put(key.getKey(), serverConfig.get(key));
}
}

List<String> visibleConfigs = serverConfig.get(Configs.VISIBLE_CONFIGS);

for (String config : visibleConfigs) {
String configValue = serverConfig.getRawString(config);
if (configValue != null) {
configs.put(config, configValue);
}
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.google.common.collect.Lists;
import java.io.PrintWriter;
import javax.servlet.http.HttpServletResponse;
import org.apache.gravitino.Configs;
import org.apache.gravitino.server.ServerConfig;
import org.junit.jupiter.api.Test;

Expand All @@ -43,4 +45,23 @@ public void testConfigServlet() throws Exception {
"{\"gravitino.authorization.enable\":false,\"gravitino.authenticators\":[\"simple\"]}");
configServlet.destroy();
}

@Test
public void testConfigServletWithVisibleConfigs() throws Exception {
ServerConfig serverConfig = new ServerConfig();
serverConfig.set(
Configs.VISIBLE_CONFIGS,
Lists.newArrayList(Configs.AUDIT_LOG_FORMATTER_CLASS_NAME.getKey()));
serverConfig.set(Configs.AUDIT_LOG_FORMATTER_CLASS_NAME, "test");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Em ... this is an interesting test case. Can we use something more realistic? I'm not sure who wants to know the audit log formatter class? Maybe there are something I missed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I can use a more realistic case.

ConfigServlet configServlet = new ConfigServlet(serverConfig);
configServlet.init();
HttpServletResponse res = mock(HttpServletResponse.class);
PrintWriter writer = mock(PrintWriter.class);
when(res.getWriter()).thenReturn(writer);
configServlet.doGet(null, res);
verify(writer)
.write(
"{\"gravitino.audit.formatter.className\":\"test\",\"gravitino.authorization.enable\":false,\"gravitino.authenticators\":[\"simple\"]}");
configServlet.destroy();
}
}