-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Report selected configuration variables via REST API #1484
Changes from 10 commits
226e338
979d95c
3b7b186
eff1a0e
6625674
2e9117d
8164349
d983423
96e59b1
4337fb5
c1354af
fea560b
73038be
256a9a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/** | ||
* This file is part of Graylog. | ||
* | ||
* Graylog is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* Graylog is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with Graylog. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
package org.graylog2.rest.models.system.configuration; | ||
|
||
import com.fasterxml.jackson.annotation.JsonAutoDetect; | ||
import com.fasterxml.jackson.annotation.JsonCreator; | ||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
import com.google.auto.value.AutoValue; | ||
|
||
import java.util.List; | ||
|
||
@JsonAutoDetect | ||
@AutoValue | ||
public abstract class ConfigurationList { | ||
|
||
@JsonProperty | ||
public abstract List<ConfigurationVariable> variables(); | ||
|
||
@JsonCreator | ||
public static ConfigurationList create(@JsonProperty("variables") List<ConfigurationVariable> variables) { | ||
return new AutoValue_ConfigurationList(variables); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
/** | ||
* This file is part of Graylog. | ||
* | ||
* Graylog is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* Graylog is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with Graylog. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
package org.graylog2.rest.models.system.configuration; | ||
|
||
import com.fasterxml.jackson.annotation.JsonAutoDetect; | ||
import com.fasterxml.jackson.annotation.JsonCreator; | ||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
import com.google.auto.value.AutoValue; | ||
|
||
@JsonAutoDetect | ||
@AutoValue | ||
public abstract class ConfigurationVariable { | ||
|
||
@JsonProperty | ||
public abstract String name(); | ||
|
||
@JsonProperty | ||
public abstract Object value(); | ||
|
||
@JsonCreator | ||
public static ConfigurationVariable create(@JsonProperty("name") String name, @JsonProperty("value") Object value) { | ||
if (value instanceof String || value instanceof Number || value instanceof Boolean) { | ||
return new AutoValue_ConfigurationVariable(name, value); | ||
} else { | ||
throw new RuntimeException("Exposed configuration variable must be of type String, Number or Boolean."); | ||
} | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
/** | ||
* This file is part of Graylog. | ||
* | ||
* Graylog is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* Graylog is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with Graylog. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
package org.graylog2.rest.models.system.configuration; | ||
|
||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import org.junit.Test; | ||
|
||
import java.io.IOException; | ||
import java.util.HashMap; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
public class ConfigurationVariableTest { | ||
private final ObjectMapper objectMapper = new ObjectMapper(); | ||
|
||
@Test(expected = RuntimeException.class) | ||
public void doesNotAcceptInvalidTypes() { | ||
ConfigurationVariable.create("foo", new HashMap<String, String>()); | ||
} | ||
|
||
@Test | ||
public void doesAcceptStrings() { | ||
ConfigurationVariable.create("foo", "bar"); | ||
} | ||
|
||
@Test | ||
public void doesAcceptNumbers() { | ||
ConfigurationVariable.create("foo", 9001); | ||
ConfigurationVariable.create("foo", 9001L); | ||
ConfigurationVariable.create("foo", 9001.0); | ||
} | ||
|
||
@Test | ||
public void doesAcceptBooleans() { | ||
ConfigurationVariable.create("foo", true); | ||
ConfigurationVariable.create("foo", false); | ||
} | ||
|
||
@Test | ||
public void testDeserializeString() throws IOException { | ||
final String json = "{\"name\":\"string\",\"value\":\"foobar\"}"; | ||
final ConfigurationVariable configurationVariable = objectMapper.readValue(json, ConfigurationVariable.class); | ||
|
||
assertThat(configurationVariable.name()).isEqualTo("string"); | ||
assertThat(configurationVariable.value()).isEqualTo("foobar"); | ||
} | ||
|
||
@Test | ||
public void testDeserializeNumber() throws IOException { | ||
final String json = "{\"name\":\"number\",\"value\":42}"; | ||
final ConfigurationVariable configurationVariable = objectMapper.readValue(json, ConfigurationVariable.class); | ||
|
||
assertThat(configurationVariable.name()).isEqualTo("number"); | ||
assertThat(configurationVariable.value()).isEqualTo(42); | ||
} | ||
|
||
@Test | ||
public void testDeserializeBoolean() throws IOException { | ||
final String json = "{\"name\":\"boolean\",\"value\":true}"; | ||
final ConfigurationVariable configurationVariable = objectMapper.readValue(json, ConfigurationVariable.class); | ||
|
||
assertThat(configurationVariable.name()).isEqualTo("boolean"); | ||
assertThat(configurationVariable.value()).isEqualTo(true); | ||
} | ||
|
||
@Test | ||
public void testSerializeString() throws IOException { | ||
final ConfigurationVariable configurationVariable = ConfigurationVariable.create("string", "foobar"); | ||
final String json = objectMapper.writeValueAsString(configurationVariable); | ||
|
||
assertThat(json).isEqualToIgnoringWhitespace("{\"name\":\"string\",\"value\":\"foobar\"}"); | ||
} | ||
|
||
@Test | ||
public void testSerializeNumber() throws IOException { | ||
final ConfigurationVariable configurationVariable = ConfigurationVariable.create("number", 42); | ||
final String json = objectMapper.writeValueAsString(configurationVariable); | ||
|
||
assertThat(json).isEqualToIgnoringWhitespace("{\"name\":\"number\",\"value\":42}"); | ||
|
||
} | ||
|
||
@Test | ||
public void testSerializeBoolean() throws IOException { | ||
final ConfigurationVariable configurationVariable = ConfigurationVariable.create("boolean", true); | ||
final String json = objectMapper.writeValueAsString(configurationVariable); | ||
|
||
assertThat(json).isEqualToIgnoringWhitespace("{\"name\":\"boolean\",\"value\":true}"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
package org.graylog2.configuration; | ||
|
||
import com.google.common.collect.ImmutableList; | ||
import org.graylog2.Configuration; | ||
import org.graylog2.rest.models.system.configuration.ConfigurationVariable; | ||
|
||
/** | ||
* List of configuration values that are safe to return, i.e. do not include any sensitive | ||
* information. Building a list manually because we need to guarantee never to return any | ||
* sensitive variables like passwords etc. - See this as a whitelist approach. | ||
*/ | ||
public class ExposedConfiguration { | ||
|
||
private final int inputBufferProcessors; | ||
private final int processBufferProcessors; | ||
private final int outputBufferProcessors; | ||
|
||
private final String processorWaitStrategy; | ||
private final String inputBufferWaitStrategy; | ||
private final int inputBufferRingSize; | ||
private final int ringSize; | ||
private final String pluginDir; | ||
private final String nodeIdFile; | ||
|
||
private final boolean allowHighlighting; | ||
private final boolean allowLeadingWildcardSearches; | ||
|
||
private final String rotationStrategy; | ||
private final String retentionStrategy; | ||
|
||
private final int maxDocsPerIndex; | ||
private final long maxSizePerIndex; | ||
private final String maxTimePerIndex; | ||
private final int maxNumberOfIndices; | ||
private final int shards; | ||
private final int replicas; | ||
|
||
private final long streamProcessingTimeout; | ||
private final int streamProcessingMaxFaults; | ||
private final long outputModuleTimeout; | ||
private final int staleMasterTimeout; | ||
|
||
private final boolean disableIndexOptimization; | ||
private final int indexOptimizationMaxSegments; | ||
|
||
private final String gcWarningThreshold; | ||
|
||
public ExposedConfiguration(Configuration configuration, ElasticsearchConfiguration esConfiguration) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do we need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought that we might want to dump this into other places in the future, too so I kept it not bound to JSON. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather get rid of this (currently) unnecessary layer of indirection. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Given how close the code freeze is and how much this small change is improving our support process I'd ask you to merge it the way it is if you don't have strong opinions against it except a preference for not using this class and moving the logic. You can of course also perform the change yourself and then merge if you see an easier way that makes sense. Thank you! :) |
||
this.inputBufferProcessors = configuration.getInputbufferProcessors(); | ||
this.processBufferProcessors = configuration.getProcessBufferProcessors(); | ||
this.outputBufferProcessors = configuration.getOutputBufferProcessors(); | ||
|
||
this.processorWaitStrategy = configuration.getProcessorWaitStrategy().getClass().getName(); | ||
this.inputBufferWaitStrategy = configuration.getInputBufferWaitStrategy().getClass().getName(); | ||
this.inputBufferRingSize = configuration.getInputBufferRingSize(); | ||
|
||
this.ringSize = configuration.getRingSize(); | ||
this.pluginDir = configuration.getPluginDir(); | ||
this.nodeIdFile = configuration.getNodeIdFile(); | ||
|
||
this.allowHighlighting = configuration.isAllowHighlighting(); | ||
this.allowLeadingWildcardSearches = configuration.isAllowLeadingWildcardSearches(); | ||
|
||
this.rotationStrategy = esConfiguration.getRotationStrategy(); | ||
this.retentionStrategy = esConfiguration.getRetentionStrategy(); | ||
|
||
this.maxDocsPerIndex = esConfiguration.getMaxDocsPerIndex(); | ||
this.maxSizePerIndex = esConfiguration.getMaxSizePerIndex(); | ||
this.maxTimePerIndex = esConfiguration.getMaxTimePerIndex().toString(); | ||
|
||
this.maxNumberOfIndices = esConfiguration.getMaxNumberOfIndices(); | ||
this.shards = esConfiguration.getShards(); | ||
this.replicas = esConfiguration.getReplicas(); | ||
|
||
this.streamProcessingTimeout = configuration.getStreamProcessingTimeout(); | ||
this.streamProcessingMaxFaults = configuration.getStreamProcessingMaxFaults(); | ||
this.outputModuleTimeout = configuration.getOutputModuleTimeout(); | ||
this.staleMasterTimeout = configuration.getStaleMasterTimeout(); | ||
|
||
this.disableIndexOptimization = esConfiguration.isDisableIndexOptimization(); | ||
this.indexOptimizationMaxSegments = esConfiguration.getIndexOptimizationMaxNumSegments(); | ||
|
||
this.gcWarningThreshold = configuration.getGcWarningThreshold().toString(); | ||
} | ||
|
||
public ImmutableList<ConfigurationVariable> asList() { | ||
return ImmutableList.<ConfigurationVariable>builder() | ||
.add(ConfigurationVariable.create("inputbuffer_processors", inputBufferProcessors)) | ||
.add(ConfigurationVariable.create("processbuffer_processors", processBufferProcessors)) | ||
.add(ConfigurationVariable.create("outputbuffer_processors", outputBufferProcessors)) | ||
.add(ConfigurationVariable.create("processor_wait_strategy", processorWaitStrategy)) | ||
.add(ConfigurationVariable.create("inputbuffer_wait_strategy", inputBufferWaitStrategy)) | ||
.add(ConfigurationVariable.create("inputbuffer_ring_size", inputBufferRingSize)) | ||
.add(ConfigurationVariable.create("ring_size", ringSize)) | ||
.add(ConfigurationVariable.create("plugin_dir", pluginDir)) | ||
.add(ConfigurationVariable.create("node_id_file", nodeIdFile)) | ||
.add(ConfigurationVariable.create("allow_highlighting", allowHighlighting)) | ||
.add(ConfigurationVariable.create("allow_leading_wildcard_searches", allowLeadingWildcardSearches)) | ||
.add(ConfigurationVariable.create("rotation_strategy", rotationStrategy)) | ||
.add(ConfigurationVariable.create("retention_strategy", retentionStrategy)) | ||
.add(ConfigurationVariable.create("elasticsearch_max_docs_per_index", maxDocsPerIndex)) | ||
.add(ConfigurationVariable.create("elasticsearch_max_size_per_index", maxSizePerIndex)) | ||
.add(ConfigurationVariable.create("elasticsearch_max_time_per_index", maxTimePerIndex)) | ||
.add(ConfigurationVariable.create("elasticsearch_max_number_of_indices", maxNumberOfIndices)) | ||
.add(ConfigurationVariable.create("elasticsearch_shards", shards)) | ||
.add(ConfigurationVariable.create("elasticsearch_replicas", replicas)) | ||
.add(ConfigurationVariable.create("stream_processing_timeout", streamProcessingTimeout)) | ||
.add(ConfigurationVariable.create("stream_processing_max_faults", streamProcessingMaxFaults)) | ||
.add(ConfigurationVariable.create("output_module_timeout", outputModuleTimeout)) | ||
.add(ConfigurationVariable.create("stale_master_timeout", staleMasterTimeout)) | ||
.add(ConfigurationVariable.create("disable_index_optimization", disableIndexOptimization)) | ||
.add(ConfigurationVariable.create("index_optimization_max_num_segments", indexOptimizationMaxSegments)) | ||
.add(ConfigurationVariable.create("gc_warning_threshold", gcWarningThreshold)) | ||
.build(); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
/** | ||
* This file is part of Graylog. | ||
* | ||
* Graylog is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or | ||
* (at your option) any later version. | ||
* | ||
* Graylog is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU General Public License | ||
* along with Graylog. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
package org.graylog2.rest.resources.system; | ||
|
||
import com.codahale.metrics.annotation.Timed; | ||
import com.wordnik.swagger.annotations.Api; | ||
import com.wordnik.swagger.annotations.ApiOperation; | ||
import org.apache.shiro.authz.annotation.RequiresAuthentication; | ||
import org.graylog2.Configuration; | ||
import org.graylog2.configuration.ElasticsearchConfiguration; | ||
import org.graylog2.configuration.ExposedConfiguration; | ||
import org.graylog2.rest.models.system.configuration.ConfigurationList; | ||
import org.graylog2.shared.rest.resources.RestResource; | ||
|
||
import javax.inject.Inject; | ||
import javax.ws.rs.GET; | ||
import javax.ws.rs.Path; | ||
|
||
@RequiresAuthentication | ||
@Api(value = "System/Configuration", description = "Read-only access to configuration variables") | ||
@Path("/system/configuration") | ||
public class ConfigurationResource extends RestResource { | ||
|
||
private final Configuration configuration; | ||
private final ElasticsearchConfiguration esConfiguration; | ||
|
||
@Inject | ||
public ConfigurationResource(Configuration configuration, ElasticsearchConfiguration esConfiguration) { | ||
this.configuration = configuration; | ||
this.esConfiguration = esConfiguration; | ||
} | ||
|
||
/* | ||
* This call is returning a list of configuration values that are safe to return, i.e. do not include any sensitive | ||
* information. | ||
*/ | ||
@GET | ||
@Timed | ||
@ApiOperation(value = "Get relevant configuration variables and their values") | ||
public ConfigurationList getRelevant() { | ||
return ConfigurationList.create( | ||
new ExposedConfiguration(configuration, esConfiguration).asList() | ||
); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
@JsonCreator
annotation cannot be used on multiple factory methods or constructors. I've added a failing unit test to demonstrate this and to add some test coverage…There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 4337fb5