Remove validation that made keys starting or ending with . - or _ inv…

…alid, catch all exceptions in the parse json processor (opensearch-project#2945) Remove validation that made keys starting or ending with . - or _ invalid, catch all exceptions in the parse json processor Signed-off-by: Taylor Gray <[email protected]>
ajeeshakd · Jun 27, 2023 · 05d229a · 05d229a
1 parent 8bb96dd
commit 05d229a
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 41 deletions.
diff --git a/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/event/JacksonEvent.java b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/event/JacksonEvent.java
@@ -373,19 +373,9 @@ private String trimKey(final String key) {
     }
 
     private boolean isValidKey(final String key) {
-        char previous = ' ';
-        char next = ' ';
         for (int i = 0; i < key.length(); i++) {
             char c = key.charAt(i);
 
-            if (i < key.length() - 1) {
-                next = key.charAt(i + 1);
-            }
-
-            if ((i == 0 || i == key.length() - 1 || previous == '/' || next == '/') && (c == '_' || c == '.' || c == '-')) {
-                return false;
-            }
-
             if (!(c >= 48 && c <= 57
                     || c >= 65 && c <= 90
                     || c >= 97 && c <= 122
@@ -397,7 +387,6 @@ private boolean isValidKey(final String key) {
 
                 return false;
             }
-            previous = c;
         }
         return true;
     }

diff --git a/data-prepper-api/src/test/java/org/opensearch/dataprepper/model/event/JacksonEventTest.java b/data-prepper-api/src/test/java/org/opensearch/dataprepper/model/event/JacksonEventTest.java
@@ -323,9 +323,8 @@ public void testIsValueAList_withNull() {
     }
 
     @ParameterizedTest
-    @ValueSource(strings = {"", "withSpecialChars*$%", "-withPrefixDash", "\\-withEscapeChars", "\\\\/withMultipleEscapeChars",
-            "withDashSuffix-", "withDashSuffix-/nestedKey", "withDashPrefix/-nestedKey", "_withUnderscorePrefix", "withUnderscoreSuffix_",
-            ".withDotPrefix", "withDotSuffix.", "with,Comma", "with:Colon", "with[Bracket", "with|Brace"})
+    @ValueSource(strings = {"", "withSpecialChars*$%", "\\-withEscapeChars", "\\\\/withMultipleEscapeChars",
+            "with,Comma", "with:Colon", "with[Bracket", "with|Brace"})
     void testKey_withInvalidKey_throwsIllegalArgumentException(final String invalidKey) {
         assertThrowsForKeyCheck(IllegalArgumentException.class, invalidKey);
     }

diff --git a/data-prepper-plugins/opensearch-source/README.md b/data-prepper-plugins/opensearch-source/README.md
@@ -197,4 +197,3 @@ The default behavior is to process all indices.
 #### <a name="index_configuration">Index Configuration</a>
 
 * `index_name_regex`: A regex pattern to represent the index names for filtering
-
diff --git a/.../main/java/org/opensearch/dataprepper/plugins/processor/parsejson/ParseJsonProcessor.java b/.../main/java/org/opensearch/dataprepper/plugins/processor/parsejson/ParseJsonProcessor.java
@@ -64,34 +64,37 @@ public Collection<Record<Event>> doExecute(final Collection<Record<Event>> recor
         final boolean doUsePointer = Objects.nonNull(pointer);
 
         for (final Record<Event> record : records) {
-            final Event event = record.getData();
 
-            if (Objects.nonNull(parseWhen) && !expressionEvaluator.evaluateConditional(parseWhen, event)) {
-                continue;
-            }
-
-            final String message = event.get(source, String.class);
-            if (Objects.isNull(message)) {
-                continue;
-            }
-
-            try {
-                final TypeReference<HashMap<String, Object>> hashMapTypeReference = new TypeReference<HashMap<String, Object>>() {};
-                Map<String, Object> parsedJson = objectMapper.readValue(message, hashMapTypeReference);
-
-                if (doUsePointer) {
-                    parsedJson = parseUsingPointer(event, parsedJson, pointer, doWriteToRoot);
-                }
-
-                if (doWriteToRoot) {
-                    writeToRoot(event, parsedJson);
-                } else {
-                    event.put(destination, parsedJson);
+                final Event event = record.getData();
+                try {
+                    if (Objects.nonNull(parseWhen) && !expressionEvaluator.evaluateConditional(parseWhen, event)) {
+                        continue;
+                    }
+
+                    final String message = event.get(source, String.class);
+                    if (Objects.isNull(message)) {
+                        continue;
+                    }
+                    final TypeReference<HashMap<String, Object>> hashMapTypeReference = new TypeReference<HashMap<String, Object>>() {
+                    };
+                    Map<String, Object> parsedJson = objectMapper.readValue(message, hashMapTypeReference);
+
+                    if (doUsePointer) {
+                        parsedJson = parseUsingPointer(event, parsedJson, pointer, doWriteToRoot);
+                    }
+
+                    if (doWriteToRoot) {
+                        writeToRoot(event, parsedJson);
+                    } else {
+                        event.put(destination, parsedJson);
+                    }
+                } catch (final JsonProcessingException jsonException) {
+                    event.getMetadata().addTags(tagsOnFailure);
+                    LOG.error(EVENT, "An exception occurred due to invalid JSON while reading event [{}]", event, jsonException);
+                } catch (final Exception e) {
+                    event.getMetadata().addTags(tagsOnFailure);
+                    LOG.error(EVENT, "An exception occurred while using the parse_json processor on Event [{}]", event, e);
                 }
-            } catch (final JsonProcessingException jsonException) {
-                event.getMetadata().addTags(tagsOnFailure);
-                LOG.error(EVENT, "An exception occurred due to invalid JSON while reading event [{}]", event, jsonException);
-            }
         }
         return records;
     }

diff --git a/...t/java/org/opensearch/dataprepper/plugins/processor/parsejson/ParseJsonProcessorTest.java b/...t/java/org/opensearch/dataprepper/plugins/processor/parsejson/ParseJsonProcessorTest.java
@@ -306,6 +306,25 @@ void test_tags_when_json_parse_fails() {
         assertTrue(parsedEvent.getMetadata().hasTags(testTags));
     }
 
+    @Test
+    void when_evaluate_conditional_throws_RuntimeException_events_are_not_dropped() {
+        final String source = "different_source";
+        final String destination = "destination_key";
+        when(processorConfig.getSource()).thenReturn(source);
+        when(processorConfig.getDestination()).thenReturn(destination);
+        final String whenCondition = UUID.randomUUID().toString();
+        when(processorConfig.getParseWhen()).thenReturn(whenCondition);
+        final Map<String, Object> data = Collections.singletonMap("key", "value");
+        final String serializedMessage = convertMapToJSONString(data);
+        final Record<Event> testEvent = createMessageEvent(serializedMessage);
+        when(expressionEvaluator.evaluateConditional(whenCondition, testEvent.getData())).thenThrow(RuntimeException.class);
+        parseJsonProcessor = createObjectUnderTest();
+
+        final Event parsedEvent = createAndParseMessageEvent(testEvent);
+
+        assertThat(parsedEvent.toMap(), equalTo(testEvent.getData().toMap()));
+    }
+
     private String constructDeeplyNestedJsonPointer(final int numberOfLayers) {
         String pointer = "/" + DEEPLY_NESTED_KEY_NAME;
         for (int layer = 0; layer < numberOfLayers; layer++) {