Added validations in include and exclude keys (#3181)

Signed-off-by: Asif Sohail Mohammed <[email protected]>

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<String> routes,
         private SinkInternalJsonModel(final List<String> routes, final String tagsTargetKey, final List<String> includeKeys, final List<String> excludeKeys, final Map<String, Object> 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<String> preprocessingKeys(final List<String> keys) {
-            if (keys.contains("/")) {
-                return new ArrayList<>();
-            }
-            List<String> 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 /");
+            });
         }
     }
 

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<String, Object> 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<String>()));
-        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<String, Object> 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<String, Object> 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<String>()));
-        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<String, Object> 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<String, Object> pluginSettings = new LinkedHashMap<>();
         assertThrows(InvalidPluginConfigurationException.class, () -> new SinkModel("customSinkPlugin", Arrays.asList("routeA", "routeB"), null, List.of("abc"), List.of("bcd"), pluginSettings));
     }

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"