kevinrr888 commented on code in PR #5073:
URL: https://github.com/apache/accumulo/pull/5073#discussion_r1852858133
##########
server/base/src/main/java/org/apache/accumulo/server/conf/CheckServerConfig.java:
##########
@@ -18,17 +18,45 @@
*/
package org.apache.accumulo.server.conf;
+import java.io.IOException;
+
import org.apache.accumulo.core.conf.SiteConfiguration;
import org.apache.accumulo.server.ServerContext;
+import org.apache.accumulo.server.ServerDirs;
+import org.apache.accumulo.server.fs.VolumeManager;
+import org.apache.accumulo.server.fs.VolumeManagerImpl;
import org.apache.accumulo.start.spi.KeywordExecutable;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
import com.google.auto.service.AutoService;
@AutoService(KeywordExecutable.class)
public class CheckServerConfig implements KeywordExecutable {
public static void main(String[] args) {
- try (var context = new ServerContext(SiteConfiguration.auto())) {
+ var siteConfig = SiteConfiguration.auto();
+ var hadoopConfig = new Configuration();
+ VolumeManager volumeManager;
+ try {
+ volumeManager = VolumeManagerImpl.get(siteConfig, hadoopConfig);
+ } catch (IOException e) {
+ throw new IllegalStateException(e);
+ }
+ var serverDirs = new ServerDirs(siteConfig, hadoopConfig);
+ Path instanceIdPath =
serverDirs.getInstanceIdLocation(volumeManager.getFirst());
+ // Check if an instance exists. If it doesn't, we have performed all the
checks we can.
+ // If it does, we can perform further checks.
+ try {
+ VolumeManager.getInstanceIDFromHdfs(instanceIdPath, hadoopConfig);
+ } catch (Exception e) {
+ System.out.println("WARNING : Performed only a subset of checks (which
passed). There is a "
Review Comment:
To avoid messing with the original functionality of the `CheckServerConfig`
check (as it seems like it is working as intended), created new check to
validate the Accumulo configuration file in
1255f900b169a7f174e940d7847b44206bd022dc. Allows users to optionally provide
the config file, as you mentioned. Also see
[comment](https://github.com/apache/accumulo/pull/5073#discussion_r1852853928).
Edit: removed the ability to accept a file path as an arg in
973aefacf6672f937b8a4803045c2d28b80477ff. Appears unacceptable in our build
constraints:
```
[ERROR] High: This API (java/io/File.<init>(Ljava/lang/String;)V) reads a
file whose location might be specified by user input
[org.apache.accumulo.server.conf.CheckAccumuloConfig] At
CheckAccumuloConfig.java:[line 44] PATH_TRAVERSAL_IN
```
--
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]
