Change manage_template default value for data streams (#1111)

This commit changed the `manage_template` default value to false when the data stream is enabled, the option wasn't set by the user, or the `auto` mechanism resolves to true. It also added the `manage_template => false` as a valid data stream configuration.

Closes #1093

Author: edmocosta

Diff for: CHANGELOG.md

@@ -1,3 +1,7 @@
+## Unreleased
+ - Changed the `manage_template` default value to `false` when data streams is enabled [#1111](https://github.com/logstash-plugins/logstash-output-elasticsearch/pull/1111)
+   - Added the `manage_template => false` as a valid data stream option
+
 ## 11.12.3
  - Changed the log messages for data stream checks [#1109](https://github.com/logstash-plugins/logstash-output-elasticsearch/pull/1109)
    - Added more details about incompatible data streams supplied configurations

Diff for: docs/index.asciidoc

@@ -790,7 +790,7 @@ Set the keystore password
 ===== `manage_template`
 
   * Value type is <<boolean,boolean>>
-  * Default value is `true`
+  * Default value is `true` for non-time series data, and `false` for data streams.
 
 From Logstash 1.3 onwards, a template is applied to Elasticsearch during
 Logstash's startup if one with the name <<plugins-{type}s-{plugin}-template_name>>

Diff for: lib/logstash/outputs/elasticsearch.rb

@@ -158,6 +158,8 @@ class LogStash::Outputs::ElasticSearch < LogStash::Outputs::Base
   # control over template creation, (e.g. creating indices dynamically based on
   # field names) you should set `manage_template` to false and use the REST
   # API to apply your templates manually.
+  #
+  # Default value is `true` unless data streams is enabled
   config :manage_template, :validate => :boolean, :default => true
 
   # This configuration option defines how the template is named inside Elasticsearch.
@@ -304,6 +306,8 @@ def register
     # It's being concurrently invoked by this register method and by the finish_register on the @after_successful_connection_thread
     data_stream_enabled = data_stream_config?
 
+    setup_template_manager_defaults(data_stream_enabled)
+
     @after_successful_connection_thread = after_successful_connection do
       begin
         finish_register
@@ -608,6 +612,13 @@ def setup_ecs_compatibility_related_defaults
     @template_name ||= default_template_name
   end
 
+  def setup_template_manager_defaults(data_stream_enabled)
+    if original_params["manage_template"].nil? && data_stream_enabled
+      logger.debug("Disabling template management since data streams are enabled")
+      @manage_template = false
+    end
+  end
+
   # To be overidden by the -java version
   VALID_HTTP_ACTIONS = ["index", "delete", "create", "update"]
   def valid_actions

Diff for: lib/logstash/outputs/elasticsearch/data_stream_support.rb

@@ -118,14 +118,15 @@ def invalid_data_stream_params(params)
       params.reject do |name, value|
         # NOTE: intentionally do not support explicit DS configuration like:
         # - `index => ...` identifier provided by data_stream_xxx settings
-        # - `manage_template => false` implied by not setting the parameter
         case name
         when 'action'
           value == 'create'
         when 'routing', 'pipeline'
           true
         when 'data_stream'
           value.to_s == 'true'
+        when 'manage_template'
+          value.to_s == 'false'
         when 'ecs_compatibility' then true # required for LS <= 6.x
         else
           name.start_with?('data_stream_') ||

Diff for: spec/unit/outputs/elasticsearch/data_stream_support_spec.rb

@@ -152,6 +152,39 @@
 
   end
 
+  context 'ds value-dependent configuration' do
+    context 'with valid values' do
+      let(:options) { super().merge(
+        'action' => 'create',
+        'routing' => 'any',
+        'pipeline' => 'any',
+        'manage_template' => 'false',
+        'data_stream' => 'true',
+        'data_stream_type' => 'logs',
+        'data_stream_dataset' => 'any',
+        'data_stream_namespace' => 'any',
+        'data_stream_sync_fields' => true,
+        'data_stream_auto_routing' => true)
+      }
+
+      it 'should enable data-streams by default' do
+        expect( subject.data_stream_config? ).to be_truthy
+      end
+    end
+
+    context 'with invalid values' do
+      let(:options) { super().merge(
+        'data_stream' => 'true',
+        'action' => 'index',
+        'manage_template' => 'true')
+      }
+
+      it 'should raise a configuration error' do
+        expect { subject.data_stream_config? }.to raise_error(LogStash::ConfigurationError, 'Invalid data stream configuration: ["action", "manage_template"]')
+      end
+    end
+  end
+
   context "default (non data-stream) configuration (on 7.x)" do
 
     let(:options) do

Diff for: spec/unit/outputs/elasticsearch_spec.rb

@@ -606,6 +606,26 @@
     end
   end
 
+  describe "the manage_template option" do
+    context "with data stream enabled" do
+      let(:options) { {"data_stream" => "true", "data_stream_type" => "logs" } }
+      let(:do_register) { true }
+
+      it "should default to false" do
+        expect(subject).to have_attributes(manage_template: false)
+      end
+    end
+
+    context "with data stream disabled" do
+      let(:options) { {"data_stream" => "false", "index" => "logs" } }
+      let(:do_register) { true }
+
+      it "should default to true" do
+        expect(subject).to have_attributes(manage_template: true)
+      end
+    end
+  end
+
   describe "SSL end to end" do
     let(:do_register) { false } # skip the register in the global before block, as is called here.
 
@@ -870,7 +890,7 @@
   end if LOGSTASH_VERSION > '6.0'
 
   context 'handling elasticsearch document-level status meant for the DLQ' do
-    let(:options) { { "manage_template" => false } }
+    let(:options) { { "manage_template" => false, "data_stream" => 'false' } }
     let(:action) { LogStash::Outputs::ElasticSearch::EventActionTuple.new(:action, :params, LogStash::Event.new("foo" => "bar")) }
 
     context 'when @dlq_writer is nil' do
@@ -1140,7 +1160,7 @@
   describe "post-register ES setup" do
     let(:do_register) { false }
     let(:es_version) { '7.10.0' } # DS default on LS 8.x
-    let(:options) { { 'hosts' => '127.0.0.1:9999' } }
+    let(:options) { { 'hosts' => '127.0.0.1:9999', 'data_stream' => 'false' } }
     let(:logger) { subject.logger }
 
     before do