Skip to content

Commit

Permalink
remove event_api.tags.illegal option
Browse files Browse the repository at this point in the history
  • Loading branch information
kaisecheng committed Sep 17, 2024
1 parent 12b26ff commit ae0a3aa
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 99 deletions.
3 changes: 1 addition & 2 deletions docker/data/logstash/env2yaml/env2yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ package main

import (
"errors"
"fmt"
"gopkg.in/yaml.v2"
"io/ioutil"
"log"
"os"
"strings"
"fmt"
)

var validSettings = []string{
Expand Down Expand Up @@ -49,7 +49,6 @@ var validSettings = []string{
"config.debug",
"config.support_escapes",
"config.field_reference.escape_style",
"event_api.tags.illegal",
"queue.type",
"path.queue",
"queue.page_capacity",
Expand Down
1 change: 0 additions & 1 deletion logstash-core/lib/logstash/environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ module Environment
Setting::TimeValue.new("config.reload.interval", "3s"), # in seconds
Setting::Boolean.new("config.support_escapes", false),
Setting::String.new("config.field_reference.escape_style", "none", true, %w(none percent ampersand)),
Setting::String.new("event_api.tags.illegal", "rename", true, %w(rename warn)),
Setting::Boolean.new("metric.collect", true),
Setting::String.new("pipeline.id", "main"),
Setting::Boolean.new("pipeline.system", false),
Expand Down
11 changes: 0 additions & 11 deletions logstash-core/lib/logstash/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,6 @@ class LogStash::Runner < Clamp::StrictCommand
:default => LogStash::SETTINGS.get_default("config.field_reference.escape_style"),
:attribute_name => "config.field_reference.escape_style"

option ["--event_api.tags.illegal"], "STRING",
I18n.t("logstash.runner.flag.event_api.tags.illegal"),
:default => LogStash::SETTINGS.get_default("event_api.tags.illegal"),
:attribute_name => "event_api.tags.illegal"

# Module settings
option ["--modules"], "MODULES",
I18n.t("logstash.runner.flag.modules"),
Expand Down Expand Up @@ -354,12 +349,6 @@ def execute
logger.debug("Setting global FieldReference escape style: #{field_reference_escape_style}")
org.logstash.FieldReference::set_escape_style(field_reference_escape_style)

tags_illegal_setting = settings.get_setting('event_api.tags.illegal').value
if tags_illegal_setting == 'warn'
deprecation_logger.deprecated(I18n.t("logstash.runner.tags-illegal-warning"))
org.logstash.Event::set_illegal_tags_action(tags_illegal_setting)
end

return start_shell(setting("interactive"), binding) if setting("interactive")

module_parser = LogStash::Modules::CLIParser.new(setting("modules_list"), setting("modules_variable_list"))
Expand Down
20 changes: 0 additions & 20 deletions logstash-core/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,6 @@ en:
configtest-flag-information: |-
You may be interested in the '--configtest' flag which you can use to validate
logstash's configuration before you choose to restart a running system.
tags-illegal-warning: >-
Setting `event_api.tags.illegal` to `warn` allows illegal values in the reserved `tags` field, which may crash pipeline unexpectedly.
This flag value is deprecated and may be removed in a future release.
# YAML named reference to the logstash.runner.configuration
# so we can later alias it from logstash.agent.configuration
configuration: &runner_configuration
Expand Down Expand Up @@ -247,23 +244,6 @@ en:
HTML-style ampersand-hash encoding notation
representing decimal unicode codepoints
(`[` is `&#91;`; `]` is `&#93;`).
event_api:
tags:
illegal: |+
The top-level `tags` field is reserved, and may only contain a
single `string` or an array of `string`s -- other values will cause
subsequent access of the `tags` field to crash the pipeline.
This flag controls how the Event API handles a `tags` field that is
an illegal shape, such as a key-value map.
Available options are:
- `rename`: illegal value in `tags` will be moved to `_tags`.
A tag `_tagsparsefailure` is added to `tags` field to
indicate the illegal assignment. Doing `set` operation with
illegal value will throw exception. This is the default option.
- `warn`: allow illegal value assignment and print warning
at startup. This option is deprecated and slated
for removal.
modules: |+
Load Logstash modules.
Modules can be defined using multiple instances
Expand Down
21 changes: 6 additions & 15 deletions logstash-core/src/main/java/org/logstash/Event.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,10 @@ public Event(ConvertedMap data) {
this.cancelled = false;

// guard tags field from key/value map, only string or list is allowed
if (ILLEGAL_TAGS_ACTION == IllegalTagsAction.RENAME) {
final Object tags = Accessors.get(data, TAGS_FIELD);
if (!isLegalTagValue(tags)) {
initFailTag(tags);
initTag(TAGS_FAILURE_TAG);
}
final Object tags = Accessors.get(data, TAGS_FIELD);
if (!isLegalTagValue(tags)) {
initFailTag(tags);
initTag(TAGS_FAILURE_TAG);
}

Object providedTimestamp = data.get(TIMESTAMP);
Expand Down Expand Up @@ -217,11 +215,8 @@ public void setField(final String reference, final Object value) {

@SuppressWarnings("unchecked")
public void setField(final FieldReference field, final Object value) {
if (ILLEGAL_TAGS_ACTION == IllegalTagsAction.RENAME) {
if ((field.equals(TAGS_FIELD) && !isLegalTagValue(value)) ||
isTagsWithMap(field)) {
throw new InvalidTagsTypeException(field, value);
}
if ((field.equals(TAGS_FIELD) && !isLegalTagValue(value)) || isTagsWithMap(field)) {
throw new InvalidTagsTypeException(field, value);
}

switch (field.type()) {
Expand Down Expand Up @@ -544,10 +539,6 @@ private void scalarTagFallback(final String existing, final String tag) {
appendTag(tags, tag);
}

public static void setIllegalTagsAction(final String action) {
ILLEGAL_TAGS_ACTION = IllegalTagsAction.valueOf(action.toUpperCase());
}

public static IllegalTagsAction getIllegalTagsAction() {
return ILLEGAL_TAGS_ACTION;
}
Expand Down
56 changes: 6 additions & 50 deletions logstash-core/src/test/java/org/logstash/EventTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,21 @@

package org.logstash;

import java.io.IOException;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.jruby.RubyString;
import org.jruby.RubySymbol;
import org.jruby.RubyTime;
import org.jruby.java.proxies.ConcreteJavaProxy;
import org.junit.Test;

import java.io.IOException;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.*;

import static net.javacrumbs.jsonunit.JsonAssert.assertJsonEquals;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.logstash.Event.getIllegalTagsAction;
import static org.junit.Assert.*;

public final class EventTest extends RubyTestBase {

Expand Down Expand Up @@ -565,39 +556,4 @@ public void allowTopLevelTagsListOfStrings() {
assertNull(event.getField(Event.TAGS_FAILURE));
assertEquals(event.getField("[tags]"), List.of("foo", "bar"));
}

@Test
public void allowTopLevelTagsWithMap() {
withIllegalTagsAction(Event.IllegalTagsAction.WARN, () -> {
final Event event = new Event();
event.setField("[tags][foo]", "bar");

assertNull(event.getField(Event.TAGS_FAILURE));
assertEquals(event.getField("[tags][foo]"), "bar");
});
}

@Test
public void allowCreatingEventWithTopLevelTagsWithMap() {
withIllegalTagsAction(Event.IllegalTagsAction.WARN, () -> {
Map<String, Object> inner = new HashMap<>();
inner.put("poison", "true");
Map<String, Object> data = new HashMap<>();
data.put("tags", inner);
final Event event = new Event(data);

assertNull(event.getField(Event.TAGS_FAILURE));
assertEquals(event.getField("[tags][poison]"), "true");
});
}

private void withIllegalTagsAction(final Event.IllegalTagsAction temporaryIllegalTagsAction, final Runnable runnable) {
final Event.IllegalTagsAction previous = getIllegalTagsAction();
try {
Event.setIllegalTagsAction(temporaryIllegalTagsAction.toString());
runnable.run();
} finally {
Event.setIllegalTagsAction(previous.toString());
}
}
}

0 comments on commit ae0a3aa

Please sign in to comment.