undo allow_env flag (#5265)

allow-env flag was introduced because of a backwards incompatible
change, this flag is no longer needed for 5.0 and onwards.

Closes #5263.

docs/static/command-line-flags.asciidoc

@@ -65,9 +65,6 @@ Logstash has the following flags. You can use the `--help` flag to display this
 -r, --[no-]auto-reload
   Monitor configuration changes and reload the configuration whenever it is changed.
 
---allow-env
-  EXPERIMENTAL: Enable environment variable templating within configuration parameters.
-  
 --reload-interval RELOAD_INTERVAL
   Specifies how often Logstash checks the config files for changes. The default is every 3 seconds.
 

docs/static/configuration.asciidoc

@@ -609,8 +609,6 @@ output {
 [[environment-variables]]
 === Using Environment Variables in Configuration
 
-This feature is _experimental_, to enable it you will need to run logstash with the `--allow-env` flag.
-
 ==== Overview
 
 * You can set environment variable references into Logstash plugins configuration using `${var}`.

logstash-core/lib/logstash/config/mixin.rb

@@ -39,7 +39,6 @@ module LogStash::Config::Mixin
   PLUGIN_VERSION_1_0_0 = LogStash::Util::PluginVersion.new(1, 0, 0)
   PLUGIN_VERSION_0_9_0 = LogStash::Util::PluginVersion.new(0, 9, 0)
 
-  ALLOW_ENV_FLAG = "__ALLOW_ENV__"
   ENV_PLACEHOLDER_REGEX = /\$\{(?<name>\w+)(\:(?<default>[^}]*))?\}/
 
   # This method is called when someone does 'include LogStash::Config'
@@ -49,14 +48,6 @@ module LogStash::Config::Mixin
   end
 
   def config_init(params)
-    # HACK(talevy): https://github.com/elastic/logstash/issues/4958
-    # Currently, the regular plugins params argument is hijacked
-    # to pass along the `allow_env` configuration variable. This was done as to 
-    # not change the method signature of Plugin. This also makes it difficul to 
-    # reason about at the same time. ALLOW_ENV_FLAG is a special param that users 
-    # are now forbidden to set in their configuration definitions.
-    allow_env = params.delete(LogStash::Config::Mixin::ALLOW_ENV_FLAG) { false }
-
     # Validation will modify the values inside params if necessary.
     # For example: converting a string to a number, etc.
     
@@ -112,20 +103,18 @@ module LogStash::Config::Mixin
     end
 
     # Resolve environment variables references
-    if allow_env
-        params.each do |name, value|
-        if (value.is_a?(Hash))
-          value.each do |valueHashKey, valueHashValue|
-            value[valueHashKey.to_s] = replace_env_placeholders(valueHashValue)
+    params.each do |name, value|
+      if (value.is_a?(Hash))
+        value.each do |valueHashKey, valueHashValue|
+          value[valueHashKey.to_s] = replace_env_placeholders(valueHashValue)
+        end
+      else
+        if (value.is_a?(Array))
+          value.each_index do |valueArrayIndex|
+            value[valueArrayIndex] = replace_env_placeholders(value[valueArrayIndex])
           end
         else
-          if (value.is_a?(Array))
-            value.each_index do |valueArrayIndex|
-              value[valueArrayIndex] = replace_env_placeholders(value[valueArrayIndex])
-            end
-          else
-            params[name.to_s] = replace_env_placeholders(value)
-          end
+          params[name.to_s] = replace_env_placeholders(value)
         end
       end
     end

logstash-core/lib/logstash/pipeline.rb

@@ -45,8 +45,7 @@ module LogStash; class Pipeline
     :pipeline_batch_delay => 5, # in milliseconds
     :flush_interval => 5, # in seconds
     :flush_timeout_interval => 60, # in seconds
-    :debug_config => false,
-    :allow_env => false
+    :debug_config => false
   }
   MAX_INFLIGHT_WARN_THRESHOLD = 10_000
 
@@ -62,7 +61,6 @@ module LogStash; class Pipeline
     @settings = DEFAULT_SETTINGS.clone
     settings.each {|setting, value| configure(setting, value) }
     @reporter = LogStash::PipelineReporter.new(@logger, self)
-    @allow_env = settings[:allow_env]
 
     @inputs = nil
     @filters = nil
@@ -453,7 +451,6 @@ module LogStash; class Pipeline
 
   def plugin(plugin_type, name, *args)
     args << {} if args.empty?
-    args.first.merge!(LogStash::Config::Mixin::ALLOW_ENV_FLAG => @allow_env)
 
     pipeline_scoped_metric = metric.namespace([:stats, :pipelines, pipeline_id.to_s.to_sym, :plugins])
 

logstash-core/lib/logstash/runner.rb

@@ -98,10 +98,6 @@ class LogStash::Runner < Clamp::Command
     I18n.t("logstash.web_api.flag.http_port"),
     :attribute_name => :web_api_http_port, :default => 9600
 
-  option ["--allow-env"], :flag,
-    I18n.t("logstash.runner.flag.allow-env"),
-    :attribute_name => :allow_env, :default => false
-
   option ["--[no-]log-in-json"], :flag,
     I18n.t("logstash.runner.flag.log-in-json"),
     :default => false
@@ -207,8 +203,7 @@ class LogStash::Runner < Clamp::Command
     @agent.register_pipeline("main", @pipeline_settings.merge({
                           :config_string => config_string,
                           :config_path => config_path,
-                          :debug_config => debug_config?,
-                          :allow_env => allow_env?
+                          :debug_config => debug_config?
                           }))
 
     # enable sigint/sigterm before starting the agent

logstash-core/locales/en.yml

@@ -181,10 +181,6 @@ en:
           the empty string for the '-e' flag.
         configtest: |+
           Check configuration for valid syntax and then exit.
-        allow-env: |+
-          EXPERIMENTAL. Enables templating of environment variable
-          values. Instances of "${VAR}" in strings will be replaced
-          with the respective environment variable value named "VAR".
         pipeline-workers: |+
           Sets the number of pipeline workers to run.
         pipeline-batch-size: |+

logstash-core/spec/logstash/agent_spec.rb

@@ -205,19 +205,10 @@ describe LogStash::Agent do
       :config_string => pipeline_config,
     } }
 
-    context "when allow_env is false" do
-      it "does not interpolate environment variables" do
-        expect(subject).to receive(:fetch_config).and_return(pipeline_config)
-        subject.register_pipeline(pipeline_id, pipeline_settings)
-        expect(subject.pipelines[pipeline_id].inputs.first.message).to eq("${FOO}-bar")
-      end
-    end
-
-    context "when allow_env is true" do
+    context "environment variable templating" do
       before :each do
         @foo_content = ENV["FOO"]
         ENV["FOO"] = "foo"
-        pipeline_settings.merge!(:allow_env => true)
       end
 
       after :each do

logstash-core/spec/logstash/config/mixin_spec.rb

@@ -160,14 +160,14 @@ describe LogStash::Config::Mixin do
     let(:plugin_class) do
       Class.new(LogStash::Filters::Base)  do
         config_name "one_plugin"
-        config :oneString, :validate => :string
-        config :oneBoolean, :validate => :boolean
-        config :oneNumber, :validate => :number
-        config :oneArray, :validate => :array
-        config :oneHash, :validate => :hash
+        config :oneString, :validate => :string, :required => false
+        config :oneBoolean, :validate => :boolean, :required => false
+        config :oneNumber, :validate => :number, :required => false
+        config :oneArray, :validate => :array, :required => false
+        config :oneHash, :validate => :hash, :required => false
 
         def initialize(params)
-          super(params.merge(LogStash::Config::Mixin::ALLOW_ENV_FLAG => true))
+          super(params)
         end
       end
     end
@@ -231,8 +231,34 @@ describe LogStash::Config::Mixin do
         expect(subject.oneArray).to(be == [ "first array value", "fancy" ])
         expect(subject.oneHash).to(be == { "key1" => "fancy", "key2" => "fancy is true", "key3" => "true or false" })
       end
+    end
 
+    context "should support $ in values" do
+      before do
+        ENV["bar"] = "foo"
+        ENV["f$$"] = "bar"
+      end
+
+      after do
+        ENV.delete("bar")
+        ENV.delete("f$$")
+      end
+
+      subject do
+        plugin_class.new(
+          "oneString" => "${f$$:val}",
+          "oneArray" => ["foo$bar", "${bar:my$val}"]
+          # "dollar_in_env" => "${f$$:final}"
+        )
+      end
+
+      it "should support $ in values" do
+        expect(subject.oneArray).to(be == ["foo$bar", "foo"])
+      end
+
+      it "should not support $ in environment variable name" do
+        expect(subject.oneString).to(be == "${f$$:val}")
+      end
     end
   end
-
 end

logstash-core/spec/logstash/runner_spec.rb

@@ -190,19 +190,5 @@ describe LogStash::Runner do
         subject.run("bin/logstash", args)
       end
     end
-
-    context "when configuring environment variable support" do
-      it "should set 'allow_env' to false by default" do
-        args = ["-e", pipeline_string]
-        expect(LogStash::Pipeline).to receive(:new).with(pipeline_string, hash_including(:allow_env => false)).and_return(pipeline)
-        subject.run("bin/logstash", args)
-      end
-
-      it "should support templating environment variables" do
-        args = ["-e", pipeline_string, "--allow-env"]
-        expect(LogStash::Pipeline).to receive(:new).with(pipeline_string, hash_including(:allow_env => true)).and_return(pipeline)
-        subject.run("bin/logstash", args)
-      end
-    end
   end
 end