From 0ef88ec54f5e938d7f8de158c64956ee8222364a Mon Sep 17 00:00:00 2001 From: Asif Sohail Mohammed Date: Wed, 16 Aug 2023 15:51:05 -0500 Subject: [PATCH] Added validations in include and exclude keys Signed-off-by: Asif Sohail Mohammed --- .../model/configuration/SinkModel.java | 33 ++++++++----------- .../model/configuration/SinkModelTest.java | 28 ++++++++++++---- data-prepper-plugins/opensearch/README.md | 8 ++--- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/configuration/SinkModel.java b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/configuration/SinkModel.java index 328a84e586..936a5445e8 100644 --- a/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/configuration/SinkModel.java +++ b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/configuration/SinkModel.java @@ -12,13 +12,11 @@ import com.fasterxml.jackson.databind.annotation.JsonSerialize; import org.opensearch.dataprepper.model.plugin.InvalidPluginConfigurationException; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; /** * Represents an extension of the {@link PluginModel} which is specific to Sink @@ -120,10 +118,11 @@ private SinkInternalJsonModel(@JsonProperty("routes") final List routes, private SinkInternalJsonModel(final List routes, final String tagsTargetKey, final List includeKeys, final List excludeKeys, final Map pluginSettings) { super(pluginSettings); this.routes = routes != null ? routes : Collections.emptyList(); - this.includeKeys = includeKeys != null ? preprocessingKeys(includeKeys) : Collections.emptyList(); - this.excludeKeys = excludeKeys != null ? preprocessingKeys(excludeKeys) : Collections.emptyList(); + this.includeKeys = includeKeys != null ? includeKeys : Collections.emptyList(); + this.excludeKeys = excludeKeys != null ? excludeKeys : Collections.emptyList(); this.tagsTargetKey = tagsTargetKey; validateConfiguration(); + validateKeys(); } void validateConfiguration() { @@ -132,24 +131,18 @@ void validateConfiguration() { } } - /** - * Pre-processes a list of Keys and returns a sorted list - * The keys must start with `/` and not end with `/` - * - * @param keys a list of raw keys - * @return a sorted processed keys + * Validates both include and exclude keys if they contain / */ - private List preprocessingKeys(final List keys) { - if (keys.contains("/")) { - return new ArrayList<>(); - } - List result = keys.stream() - .map(k -> k.startsWith("/") ? k : "/" + k) - .map(k -> k.endsWith("/") ? k.substring(0, k.length() - 1) : k) - .collect(Collectors.toList()); - Collections.sort(result); - return result; + private void validateKeys() { + includeKeys.forEach(key -> { + if(key.contains("/")) + throw new InvalidPluginConfigurationException("include_keys cannot contain /"); + }); + excludeKeys.forEach(key -> { + if(key.contains("/")) + throw new InvalidPluginConfigurationException("exclude_keys cannot contain /"); + }); } } diff --git a/data-prepper-api/src/test/java/org/opensearch/dataprepper/model/configuration/SinkModelTest.java b/data-prepper-api/src/test/java/org/opensearch/dataprepper/model/configuration/SinkModelTest.java index a5fc6363cb..9d895ecd32 100644 --- a/data-prepper-api/src/test/java/org/opensearch/dataprepper/model/configuration/SinkModelTest.java +++ b/data-prepper-api/src/test/java/org/opensearch/dataprepper/model/configuration/SinkModelTest.java @@ -144,27 +144,41 @@ void serialize_with_just_pluginModel() throws IOException { } @Test - void sinkModel_with_include_keys() throws IOException { + void sinkModel_with_include_keys() { final Map pluginSettings = new LinkedHashMap<>(); - final SinkModel sinkModel = new SinkModel("customSinkPlugin", Arrays.asList("routeA", "routeB"), null, Arrays.asList("bcd", "/abc", "efg/"), null, pluginSettings); + final SinkModel sinkModel = new SinkModel("customSinkPlugin", Arrays.asList("routeA", "routeB"), null, Arrays.asList("bcd", "abc", "efg"), null, pluginSettings); assertThat(sinkModel.getExcludeKeys(), equalTo(new ArrayList())); - assertThat(sinkModel.getIncludeKeys(), equalTo(Arrays.asList("/abc", "/bcd", "/efg"))); + assertThat(sinkModel.getIncludeKeys(), equalTo(Arrays.asList("bcd", "abc", "efg"))); } @Test - void sinkModel_with_exclude_keys() throws IOException { + void sinkModel_with_invalid_include_keys() { final Map pluginSettings = new LinkedHashMap<>(); - final SinkModel sinkModel = new SinkModel("customSinkPlugin", Arrays.asList("routeA", "routeB"), null, List.of("/"), Arrays.asList("bcd", "/abc", "efg/"), pluginSettings); + assertThrows(InvalidPluginConfigurationException.class, () -> new SinkModel("customSinkPlugin", Arrays.asList("routeA", "routeB"), null, List.of("/bcd"), List.of(), pluginSettings)); + } + + @Test + void sinkModel_with_exclude_keys() { + final Map pluginSettings = new LinkedHashMap<>(); + final SinkModel sinkModel = new SinkModel("customSinkPlugin", Arrays.asList("routeA", "routeB"), null, List.of(), Arrays.asList("abc", "bcd", "efg"), pluginSettings); assertThat(sinkModel.getIncludeKeys(), equalTo(new ArrayList())); - assertThat(sinkModel.getExcludeKeys(), equalTo(Arrays.asList("/abc", "/bcd", "/efg"))); + assertThat(sinkModel.getExcludeKeys(), equalTo(Arrays.asList("abc", "bcd", "efg"))); + + } + @Test + void sinkModel_with_invalid_exclude_keys() { + final Map pluginSettings = new LinkedHashMap<>(); + assertThrows(InvalidPluginConfigurationException.class, () -> new SinkModel("customSinkPlugin", Arrays.asList("routeA", "routeB"), null, List.of(), List.of("/bcd"), pluginSettings)); } + + @Test - void sinkModel_with_both_include_and_exclude_keys() throws IOException { + void sinkModel_with_both_include_and_exclude_keys() { final Map pluginSettings = new LinkedHashMap<>(); assertThrows(InvalidPluginConfigurationException.class, () -> new SinkModel("customSinkPlugin", Arrays.asList("routeA", "routeB"), null, List.of("abc"), List.of("bcd"), pluginSettings)); } diff --git a/data-prepper-plugins/opensearch/README.md b/data-prepper-plugins/opensearch/README.md index 2f08f8d3c5..54cb09588d 100644 --- a/data-prepper-plugins/opensearch/README.md +++ b/data-prepper-plugins/opensearch/README.md @@ -209,7 +209,7 @@ With the `document_root_key` set to `status`. The document structure would be `{ duration: "15 ms" } ``` -- `include_keys`: A list of keys to be included (retained). The key in the list can be a valid JSON path, such as 'request/status'. This option can work together with `document_root_key`. +- `include_keys`: A list of keys to be included (retained). The key in the list cannot contain '/'. This option can work together with `document_root_key`. For example, If we have the following sample event: ``` @@ -224,7 +224,7 @@ For example, If we have the following sample event: } } ``` -if `include_keys` is set to ["status", "metadata/sourceIp"], the document written to OpenSearch would be: +if `include_keys` is set to ["status", "metadata"], the document written to OpenSearch would be: ``` { status: 200, @@ -256,11 +256,11 @@ For example, If we have the following sample event: } } ``` -if `exclude_keys` is set to ["status", "metadata/sourceIp"], the document written to OpenSearch would be: +if `exclude_keys` is set to ["message", "status"], the document written to OpenSearch would be: ``` { - message: null, metadata: { + sourceIp: "123.212.49.58", destinationIp: "79.54.67.231", bytes: 3545, duration: "15 ms"