Remove event_api.tags.illegal for v9 (#16461)

This commit removed `--event_api.tags.illegal` option
Fix: #16356

docker/data/logstash/env2yaml/env2yaml.go

@@ -16,12 +16,12 @@ package main
 
 import (
 	"errors"
+	"fmt"
 	"gopkg.in/yaml.v2"
 	"io/ioutil"
 	"log"
 	"os"
 	"strings"
-	"fmt"
 )
 
 var validSettings = []string{
@@ -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",

docs/static/settings-file.asciidoc

@@ -361,12 +361,6 @@ separating each log lines per pipeline could be helpful in case you need to trou
 | Setting to `true` to allow or `false` to block running Logstash as a superuser.
 | `true`
 
-| `event_api.tags.illegal`
-| When set to `warn`, allow illegal value assignment to the reserved `tags` field.
-When set to `rename`, Logstash events can't be created with an illegal value in `tags`. This value will be moved to `_tags` and a `_tagsparsefailure` tag is added to indicate the illegal operation. Doing `set` operation with illegal value will throw exception.
-Setting this flag to `warn` is deprecated and will be removed in a future release.
-| `rename`
-
 | `pipeline.buffer.type`
 | Determine where to allocate memory buffers, for plugins that leverage them.
 Currently defaults to `direct` but can be switched to `heap` to select Java heap space, which will become the default in the future.

logstash-core/lib/logstash/environment.rb

@@ -51,7 +51,6 @@ module LogStash
          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),

logstash-core/lib/logstash/runner.rb

@@ -263,12 +263,6 @@ class LogStash::Runner < Clamp::StrictCommand
     I18n.t("logstash.runner.flag.http_port"),
     :new_flag => "api.http.port", :passthrough => true # use settings to disambiguate
 
-  deprecated_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", :passthrough => true,
-         :obsoleted_version => "9"
-
   # We configure the registry and load any plugin that can register hooks
   # with logstash, this needs to be done before any operation.
   SYSTEM_SETTINGS = LogStash::SETTINGS.clone
@@ -356,12 +350,6 @@ class LogStash::Runner < Clamp::StrictCommand
     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"))

logstash-core/locales/en.yml

@@ -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 is deprecated and will be removed in version 9.
       # YAML named reference to the logstash.runner.configuration
       # so we can later alias it from logstash.agent.configuration
       configuration: &runner_configuration
@@ -247,23 +244,6 @@ en:
              HTML-style ampersand-hash encoding notation
              representing decimal unicode codepoints
              (`[` is `[`; `]` is `]`).
-        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

logstash-core/spec/logstash/runner_spec.rb

@@ -380,32 +380,6 @@ describe LogStash::Runner do
       end
     end
 
-    context "event_api.tags.illegal" do
-      let(:runner_deprecation_logger_stub) { double("DeprecationLogger(Runner)").as_null_object }
-      let(:args) { ["--event_api.tags.illegal", "warn", "-e", pipeline_string] }
-      before(:each) { allow(runner).to receive(:deprecation_logger).and_return(runner_deprecation_logger_stub) }
-      DEPRECATED_MSG="The flag [\"--event_api.tags.illegal\"] has been deprecated and will be removed in version 9"
-
-      it "gives deprecation message when setting to `warn`" do
-        expect(runner_deprecation_logger_stub).to receive(:deprecated)
-          .with(a_string_including "This flag is deprecated and will be removed in version 9")
-          .with(a_string_including DEPRECATED_MSG)
-        subject.run("bin/logstash", args)
-      end
-
-      it "gives deprecation message when setting to `rename`" do
-        expect(runner_deprecation_logger_stub).to receive(:deprecated)
-          .with(a_string_including DEPRECATED_MSG)
-        subject.run("bin/logstash", args)
-      end
-
-      it "does not give deprecation message when unset" do
-        expect(runner_deprecation_logger_stub).not_to receive(:deprecated)
-          .with(a_string_including DEPRECATED_MSG)
-        subject.run("bin/logstash", ["-e", pipeline_string])
-      end
-    end
-
     context "when :pipeline_workers is not defined by the user" do
       it "should not pass the value to the pipeline" do
         expect(LogStash::Agent).to receive(:new) do |settings|

logstash-core/src/main/java/org/logstash/Event.java

@@ -67,9 +67,6 @@ public final class Event implements Cloneable, Queueable, co.elastic.logstash.ap
     public static final String TAGS_FAILURE_TAG = "_tagsparsefailure";
     public static final String TAGS_FAILURE = "_tags";
 
-    enum IllegalTagsAction { RENAME, WARN }
-    private static IllegalTagsAction ILLEGAL_TAGS_ACTION = IllegalTagsAction.RENAME;
-
     private static final FieldReference TAGS_FIELD = FieldReference.from(TAGS);
     private static final FieldReference TAGS_FAILURE_FIELD = FieldReference.from(TAGS_FAILURE);
 
@@ -115,13 +112,11 @@ public final class Event implements Cloneable, Queueable, co.elastic.logstash.ap
         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);
         }
-        }
 
         Object providedTimestamp = data.get(TIMESTAMP);
         // keep reference to the parsedTimestamp for tagging below
@@ -217,12 +212,9 @@ public final class Event implements Cloneable, Queueable, co.elastic.logstash.ap
 
     @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)) {
+        if ((field.equals(TAGS_FIELD) && !isLegalTagValue(value)) || isTagsWithMap(field)) {
             throw new InvalidTagsTypeException(field, value);
         }
-        }
 
         switch (field.type()) {
             case FieldReference.META_PARENT:
@@ -544,14 +536,6 @@ public final class Event implements Cloneable, Queueable, co.elastic.logstash.ap
         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;
-    }
-
     @Override
     public byte[] serialize() throws JsonProcessingException {
         final Map<String, Map<String, Object>> map = new HashMap<>(2, 1.0F);

logstash-core/src/test/java/org/logstash/EventTest.java

@@ -20,6 +20,12 @@
 
 package org.logstash;
 
+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;
@@ -30,11 +36,6 @@ 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 static net.javacrumbs.jsonunit.JsonAssert.assertJsonEquals;
 import static org.hamcrest.CoreMatchers.is;
@@ -43,7 +44,6 @@ 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;
 
 public final class EventTest extends RubyTestBase {
 
@@ -565,39 +565,4 @@ public final class EventTest extends RubyTestBase {
         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());
-        }
-    }
 }

qa/integration/specs/reserved_tags_field_spec.rb

@@ -21,7 +21,7 @@ require_relative '../services/logstash_service'
 require_relative '../framework/helpers'
 require "logstash/devutils/rspec/spec_helper"
 
-# reserved tags should accept string and array of string only in rename mode
+# reserved tags should accept string and array of string only
 describe "Guard reserved tags field against incorrect use" do
   before(:all) {
     @fixture = Fixture.new(__FILE__)
@@ -48,11 +48,10 @@ describe "Guard reserved tags field against incorrect use" do
   }
   let(:settings_dir) { Stud::Temporary.directory }
 
-  shared_examples_for 'assign illegal value to tags' do |mode, pipeline_fixture, tags_match, fail_tags_match|
-    it "[#{mode}] update tags and _tags successfully" do
+  shared_examples_for 'assign illegal value to tags' do |pipeline_fixture, tags_match, fail_tags_match|
+    it "update tags and _tags successfully" do
       @logstash.env_variables = test_env
       @logstash.spawn_logstash("-f", config_to_temp_file(@fixture.config(pipeline_fixture)),
-                               "--event_api.tags.illegal", "#{mode}",
                                "--path.settings", settings_dir)
 
       Stud.try(num_retries.times, [StandardError, RSpec::Expectations::ExpectationNotMetError]) do
@@ -65,18 +64,15 @@ describe "Guard reserved tags field against incorrect use" do
   end
 
   describe 'create event' do
-    it_behaves_like 'assign illegal value to tags', 'rename', 'create_tags_map', /"tags":\["_tagsparsefailure"\]/, /"_tags":\[{"poison":true}\]/
-    it_behaves_like 'assign illegal value to tags', 'warn',  'create_tags_map', /"tags":{"poison":true}/, /(?!_tags)/
-    it_behaves_like 'assign illegal value to tags', 'rename', 'create_tags_number', /"tags":\["_tagsparsefailure"\]/, /"_tags":\[\[1,2,3\]\]/
-    it_behaves_like 'assign illegal value to tags', 'warn', 'create_tags_number', /"tags":\[1,2,3\]/, /(?!_tags)/
+    it_behaves_like 'assign illegal value to tags', 'create_tags_map', /"tags":\["_tagsparsefailure"\]/, /"_tags":\[{"poison":true}\]/
+    it_behaves_like 'assign illegal value to tags', 'create_tags_number', /"tags":\["_tagsparsefailure"\]/, /"_tags":\[\[1,2,3\]\]/
   end
 
   it "should throw exception when assigning two illegal values" do
-    ['rename', 'warn'].each do |mode|
     logstash = @logstash.run_cmd(["bin/logstash", "-e", @fixture.config('set_illegal_tags').gsub("\n", ""),
-                                    "--path.settings", settings_dir, "--event_api.tags.illegal", mode],
+                                  "--path.settings", settings_dir],
                                  true, test_env)
     expect(logstash.stderr_and_stdout).to match(/Ruby exception occurred/)
   end
-  end
+
 end