-
Notifications
You must be signed in to change notification settings - Fork 314
Add support for parent event field access during iterate_on processing in add_entries processor #6713
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
base: main
Are you sure you want to change the base?
Add support for parent event field access during iterate_on processing in add_entries processor #6713
Changes from 1 commit
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 |
|---|---|---|
|
|
@@ -166,6 +166,28 @@ public static class Entry { | |
| }) | ||
| private boolean flattenKey = true; | ||
|
|
||
| @JsonProperty("disable_root_keys") | ||
| @JsonPropertyDescription( | ||
| "When set to <code>false</code> and used with <code>iterate_on</code>, resolves <code>value_expression</code> and <code>format</code> " + | ||
| "against the root event instead of the individual array element. This allows referencing root-level fields during array iteration. " + | ||
| "Has no effect when not used with <code>iterate_on</code>. The default value is <code>true</code>.") | ||
| @AlsoRequired(values = { | ||
| @AlsoRequired.Required(name="iterate_on") | ||
| }) | ||
| private boolean disableRootKeys = true; | ||
|
|
||
| @JsonProperty("evaluate_when_on_element") | ||
| @JsonPropertyDescription( | ||
| "When set to <code>true</code> and used with <code>iterate_on</code> and <code>add_to_element_when</code>, " + | ||
| "evaluates the <code>add_to_element_when</code> condition against each individual array element instead of the root event. " + | ||
| "This enables per-element conditional logic during array iteration. " + | ||
| "The default value is <code>false</code>.") | ||
| @AlsoRequired(values = { | ||
| @AlsoRequired.Required(name="iterate_on"), | ||
| @AlsoRequired.Required(name="add_to_element_when") | ||
| }) | ||
| private boolean evaluateWhenOnElement = false; | ||
|
|
||
| @JsonProperty("add_when") | ||
| @JsonPropertyDescription("A <a href=\"https://opensearch.org/docs/latest/data-prepper/pipelines/expression-syntax/\">conditional expression</a>, " + | ||
| "such as <code>/some-key == \"test\"</code>, that will be evaluated to determine whether the processor will be run on the event.") | ||
|
|
@@ -212,6 +234,14 @@ public boolean getFlattenKey(){ | |
| return flattenKey; | ||
| } | ||
|
|
||
| public boolean getDisableRootKeys() { | ||
| return disableRootKeys; | ||
| } | ||
|
|
||
| public boolean getEvaluateWhenOnElement() { | ||
| return evaluateWhenOnElement; | ||
| } | ||
|
|
||
| public String getAddWhen() { return addWhen; } | ||
|
|
||
| @AssertTrue(message = "Exactly one of value, format, or value_expression must be specified") | ||
|
|
@@ -229,6 +259,64 @@ boolean flattenKeyFalseIsUsedWithIterateOn() { | |
| return (!flattenKey && iterateOn!=null) || flattenKey; | ||
| } | ||
|
|
||
| @AssertTrue(message = "disable_root_keys=false only applies when iterate_on is configured.") | ||
| boolean disableRootKeysFalseIsUsedWithIterateOn() { | ||
| return disableRootKeys || iterateOn != null; | ||
| } | ||
|
|
||
| @AssertTrue(message = "evaluate_when_on_element only applies when both iterate_on and add_to_element_when are configured.") | ||
| boolean evaluateWhenOnElementIsUsedWithIterateOnAndAddToElementWhen() { | ||
| return !evaluateWhenOnElement || (iterateOn != null && addToElementWhen != null); | ||
| } | ||
|
|
||
| public Entry(final String key, | ||
|
Contributor
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. issue (non-blocking): Massive constructor duplication
public Entry(String key, String metadataKey, Object value, String format,
String valueExpression, boolean overwriteIfKeyExists, boolean appendIfKeyExists,
String addWhen, String iterateOn, boolean flattenKey, String addToElementWhen) {
this(key, metadataKey, value, format, valueExpression, overwriteIfKeyExists,
appendIfKeyExists, addWhen, iterateOn, flattenKey, true, false, addToElementWhen);
}This eliminates all duplicated validation.
Author
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. Removed the duplication in the constructor as suggested. |
||
| final String metadataKey, | ||
| final Object value, | ||
| final String format, | ||
| final String valueExpression, | ||
| final boolean overwriteIfKeyExists, | ||
| final boolean appendIfKeyExists, | ||
| final String addWhen, | ||
| final String iterateOn, | ||
| final boolean flattenKey, | ||
| final boolean disableRootKeys, | ||
| final boolean evaluateWhenOnElement, | ||
| final String addToElementWhen) | ||
| { | ||
| if (key != null && metadataKey != null) { | ||
| throw new IllegalArgumentException("Only one of the two - key and metadatakey - should be specified"); | ||
| } | ||
| if (key == null && metadataKey == null) { | ||
| throw new IllegalArgumentException("At least one of the two - key and metadatakey - must be specified"); | ||
| } | ||
| if (metadataKey != null && iterateOn != null) { | ||
| throw new IllegalArgumentException("iterate_on cannot be applied to metadata"); | ||
| } | ||
| if (iterateOn == null && addToElementWhen != null) { | ||
| throw new InvalidPluginConfigurationException("add_to_element_when only applies when iterate_on is configured."); | ||
| } | ||
| if (!disableRootKeys && iterateOn == null) { | ||
| throw new InvalidPluginConfigurationException("disable_root_keys=false only applies when iterate_on is configured."); | ||
| } | ||
| if (evaluateWhenOnElement && (iterateOn == null || addToElementWhen == null)) { | ||
| throw new InvalidPluginConfigurationException("evaluate_when_on_element only applies when both iterate_on and add_to_element_when are configured."); | ||
| } | ||
|
|
||
| this.key = key; | ||
| this.metadataKey = metadataKey; | ||
| this.value = value; | ||
| this.format = format; | ||
| this.valueExpression = valueExpression; | ||
| this.overwriteIfKeyExists = overwriteIfKeyExists; | ||
| this.appendIfKeyExists = appendIfKeyExists; | ||
| this.addWhen = addWhen; | ||
| this.iterateOn = iterateOn; | ||
| this.flattenKey = flattenKey; | ||
| this.disableRootKeys = disableRootKeys; | ||
| this.evaluateWhenOnElement = evaluateWhenOnElement; | ||
| this.addToElementWhen = addToElementWhen; | ||
| } | ||
|
|
||
| public Entry(final String key, | ||
| final String metadataKey, | ||
| final Object value, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * The OpenSearch Contributors require contributions made to | ||
| * this file be licensed under the Apache-2.0 license or a | ||
| * compatible open source license. | ||
| */ | ||
|
|
||
| package org.opensearch.dataprepper.plugins.processor.mutateevent; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.opensearch.dataprepper.model.event.Event; | ||
| import org.opensearch.dataprepper.model.event.JacksonEvent; | ||
| import org.opensearch.dataprepper.model.processor.Processor; | ||
| import org.opensearch.dataprepper.model.record.Record; | ||
| import org.opensearch.dataprepper.test.plugins.DataPrepperPluginTest; | ||
| import org.opensearch.dataprepper.test.plugins.PluginConfigurationFile; | ||
| import org.opensearch.dataprepper.test.plugins.junit.BaseDataPrepperPluginStandardTestSuite; | ||
|
|
||
| import java.util.*; | ||
|
Collaborator
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. Star import may result in checkstyle failures. I triggered the checks, please check that gradle builds are OK. |
||
|
|
||
| import static org.hamcrest.CoreMatchers.equalTo; | ||
| import static org.hamcrest.CoreMatchers.is; | ||
| import static org.hamcrest.MatcherAssert.assertThat; | ||
|
|
||
| @DataPrepperPluginTest(pluginName = "add_entries", pluginType = Processor.class) | ||
| class AddEntryProcessorIT extends BaseDataPrepperPluginStandardTestSuite { | ||
|
|
||
| @Test | ||
| void disable_root_keys_resolves_value_expression_from_root( | ||
| @PluginConfigurationFile("iterate_on_with_disable_root_keys.yaml") final Processor<Record<Event>, Record<Event>> objectUnderTest) { | ||
| final Map<String, Object> data = new HashMap<>(); | ||
| data.put("alert_title", "SQL Injection Detected"); | ||
| data.put("vulns", Arrays.asList( | ||
| new HashMap<>(Map.of("cve", "CVE-2024-001")), | ||
| new HashMap<>(Map.of("cve", "CVE-2024-002")))); | ||
|
|
||
| final Record<Event> record = buildRecordWithEvent(data); | ||
| final List<Record<Event>> result = (List<Record<Event>>) objectUnderTest.execute(Collections.singletonList(record)); | ||
|
|
||
| List<Map<String, Object>> vulns = result.get(0).getData().get("vulns", List.class); | ||
| assertThat(vulns.size(), equalTo(2)); | ||
| assertThat(vulns.get(0).get("title"), equalTo("SQL Injection Detected")); | ||
| assertThat(vulns.get(1).get("title"), equalTo("SQL Injection Detected")); | ||
| } | ||
|
|
||
| @Test | ||
| void evaluate_when_on_element_filters_per_element( | ||
| @PluginConfigurationFile("iterate_on_with_evaluate_when_on_element.yaml") final Processor<Record<Event>, Record<Event>> objectUnderTest) { | ||
| final Map<String, Object> data = new HashMap<>(); | ||
| data.put("vulns", Arrays.asList( | ||
| new HashMap<>(Map.of("cve", "CVE-2024-001", "severity", "critical")), | ||
| new HashMap<>(Map.of("cve", "CVE-2024-002", "severity", "low")), | ||
| new HashMap<>(Map.of("cve", "CVE-2024-003", "severity", "critical")))); | ||
|
|
||
| final Record<Event> record = buildRecordWithEvent(data); | ||
| final List<Record<Event>> result = (List<Record<Event>>) objectUnderTest.execute(Collections.singletonList(record)); | ||
|
|
||
| List<Map<String, Object>> vulns = result.get(0).getData().get("vulns", List.class); | ||
| assertThat(vulns.size(), equalTo(3)); | ||
| assertThat(vulns.get(0).get("flagged"), equalTo(true)); | ||
| assertThat(vulns.get(1).containsKey("flagged"), is(false)); | ||
| assertThat(vulns.get(2).get("flagged"), equalTo(true)); | ||
| } | ||
|
|
||
| @Test | ||
| void both_flags_combined_resolves_root_value_with_per_element_condition( | ||
| @PluginConfigurationFile("iterate_on_with_both_flags.yaml") final Processor<Record<Event>, Record<Event>> objectUnderTest) { | ||
| final Map<String, Object> data = new HashMap<>(); | ||
| data.put("alert_title", "SQL Injection Detected"); | ||
| data.put("vulns", Arrays.asList( | ||
| new HashMap<>(Map.of("cve", "CVE-2024-001", "severity", "critical")), | ||
| new HashMap<>(Map.of("cve", "CVE-2024-002", "severity", "low")), | ||
| new HashMap<>(Map.of("cve", "CVE-2024-003", "severity", "critical")))); | ||
|
|
||
| final Record<Event> record = buildRecordWithEvent(data); | ||
| final List<Record<Event>> result = (List<Record<Event>>) objectUnderTest.execute(Collections.singletonList(record)); | ||
|
|
||
| List<Map<String, Object>> vulns = result.get(0).getData().get("vulns", List.class); | ||
| assertThat(vulns.size(), equalTo(3)); | ||
| assertThat(vulns.get(0).get("title"), equalTo("SQL Injection Detected")); | ||
| assertThat(vulns.get(1).containsKey("title"), is(false)); | ||
| assertThat(vulns.get(2).get("title"), equalTo("SQL Injection Detected")); | ||
| } | ||
|
|
||
| private static Record<Event> buildRecordWithEvent(final Map<String, Object> data) { | ||
| return new Record<>(JacksonEvent.builder() | ||
| .withData(data) | ||
| .withEventType("event") | ||
| .build()); | ||
| } | ||
| } | ||
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.
suggestion: Confusing double-negative naming for
disable_root_keysnaturally.
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.
Noted!!
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.
I would suggest using a positive field name as well, like
resolve_from_root,use_root_context, etc. so that: 1) the default value would be false; 2) it's easier for users to comprehend and use