Fix 'https' scheme in ES hosts setting when set from Cloud ID. (#8492)

* fix 'https' scheme in ES hosts setting

* Fixes as per review

logstash-core/lib/logstash/modules/kibana_client.rb

@@ -24,9 +24,11 @@ module LogStash module Modules class KibanaClient
     end
   end
 
-  attr_reader :version
+  SCHEME_REGEX = /^https?$/
 
-  def initialize(settings)
+  attr_reader :version, :endpoint
+
+  def initialize(settings, client = nil) # allow for test mock injection
     @settings = settings
 
     client_options = {
@@ -38,8 +40,8 @@ module LogStash module Modules class KibanaClient
     }
 
     ssl_options = {}
-
-    if @settings["var.kibana.ssl.enabled"] == "true"
+    ssl_enabled = @settings["var.kibana.ssl.enabled"] == "true"
+    if ssl_enabled
       ssl_options[:verify] = @settings.fetch("var.kibana.ssl.verification_mode", "strict").to_sym
       ssl_options[:ca_file] = @settings.fetch("var.kibana.ssl.certificate_authority", nil)
       ssl_options[:client_cert] = @settings.fetch("var.kibana.ssl.certificate", nil)
@@ -48,9 +50,34 @@ module LogStash module Modules class KibanaClient
 
     client_options[:ssl] = ssl_options
 
-    @client = Manticore::Client.new(client_options)
     @host = @settings.fetch("var.kibana.host", "localhost:5601")
-    @scheme = @settings.fetch("var.kibana.scheme", "http")
+    implicit_scheme, colon_slash_slash, host = @host.partition("://")
+    explicit_scheme = @settings["var.kibana.scheme"]
+    @scheme = "http"
+    if !colon_slash_slash.empty?
+      if !explicit_scheme.nil? && implicit_scheme != explicit_scheme
+        # both are set and not the same - error
+        msg = sprintf("Detected differing Kibana host schemes as sourced from var.kibana.host: '%s' and var.kibana.scheme: '%s'", implicit_scheme, explicit_scheme)
+        raise ArgumentError.new(msg)
+      end
+      @scheme = implicit_scheme
+      @host = host
+    elsif !explicit_scheme.nil?
+      @scheme = explicit_scheme
+    end
+
+    if SCHEME_REGEX.match(@scheme).nil?
+      msg = sprintf("Kibana host scheme given is invalid, given value: '%s' - acceptable values: 'http', 'https'", @scheme)
+      raise ArgumentError.new(msg)
+    end
+
+    if ssl_enabled && @scheme != "https"
+      @scheme = "https"
+    end
+
+    @endpoint = "#{@scheme}://#{@host}"
+
+    @client = client || Manticore::Client.new(client_options)
     @http_options = {:headers => {'Content-Type' => 'application/json'}}
     username = @settings["var.kibana.username"]
     if username
@@ -77,7 +104,7 @@ module LogStash module Modules class KibanaClient
   end
 
   def version_parts
-    @version.split(/\.|\-/)
+    @version.split(/[.-]/)
   end
 
   def host_settings
@@ -119,6 +146,6 @@ module LogStash module Modules class KibanaClient
   end
 
   def full_url(relative)
-    "#{@scheme}://#{@host}/#{relative}"
+    "#{@endpoint}/#{relative}"
   end
 end end end

logstash-core/lib/logstash/modules/settings_merger.rb

@@ -40,9 +40,10 @@ module LogStash module Modules module SettingsMerger
       settings_copy = LogStash::Util.deep_clone(module_settings)
     end
 
-    module_settings["var.kibana.scheme"] = "https"
+    module_settings["var.kibana.scheme"] = cloud_id.kibana_scheme
     module_settings["var.kibana.host"] = cloud_id.kibana_host
-    module_settings["var.elasticsearch.hosts"] = cloud_id.elasticsearch_host
+    # elasticsearch client does not use scheme, it URI parses the host setting
+    module_settings["var.elasticsearch.hosts"] = "#{cloud_id.elasticsearch_scheme}://#{cloud_id.elasticsearch_host}"
     unless cloud_auth.nil?
       module_settings["var.elasticsearch.username"] = cloud_auth.username
       module_settings["var.elasticsearch.password"] = cloud_auth.password

logstash-core/lib/logstash/util/cloud_setting_id.rb

@@ -3,8 +3,26 @@ require "logstash/namespace"
 require "base64"
 
 module LogStash module Util class CloudSettingId
-  attr_reader :original, :decoded, :label, :elasticsearch_host, :kibana_host
 
+  def self.cloud_id_encode(*args)
+    Base64.urlsafe_encode64(args.join("$"))
+  end
+  DOT_SEPARATOR = "."
+  CLOUD_PORT = ":443"
+
+  attr_reader :original, :decoded, :label, :elasticsearch_host, :elasticsearch_scheme, :kibana_host, :kibana_scheme
+
+  # The constructor is expecting a 'cloud.id', a string in 2 variants.
+  # 1 part example: 'dXMtZWFzdC0xLmF3cy5mb3VuZC5pbyRub3RhcmVhbCRpZGVudGlmaWVy'
+  # 2 part example: 'foobar:dXMtZWFzdC0xLmF3cy5mb3VuZC5pbyRub3RhcmVhbCRpZGVudGlmaWVy'
+  # The two part variant has a 'label' prepended with a colon separator. The label is not encoded.
+  # The 1 part (or second section of the 2 part variant) is base64 encoded.
+  # The original string before encoding has three segments separated by a dollar sign.
+  # e.g. 'us-east-1.aws.found.io$notareal$identifier'
+  # The first segment is the cloud base url, e.g. 'us-east-1.aws.found.io'
+  # The second segment is the elasticsearch host identifier, e.g. 'notareal'
+  # The third segment is the kibana host identifier, e.g. 'identifier'
+  # The 'cloud.id' value decoded into the #attr_reader ivars.
   def initialize(value)
     return if value.nil?
 
@@ -12,27 +30,43 @@ module LogStash module Util class CloudSettingId
       raise ArgumentError.new("Cloud Id must be String. Received: #{value.class}")
     end
     @original = value
-    @label, sep, last = value.partition(":")
-    if last.empty?
+    @label, colon, encoded = @original.partition(":")
+    if encoded.empty?
       @decoded = Base64.urlsafe_decode64(@label) rescue ""
       @label = ""
     else
-      @decoded = Base64.urlsafe_decode64(last) rescue ""
+      @decoded = Base64.urlsafe_decode64(encoded) rescue ""
     end
+
+    @decoded = @decoded.encode(Encoding::UTF_8, :invalid => :replace, :undef => :replace)
+
     unless @decoded.count("$") == 2
-      raise ArgumentError.new("Cloud Id does not decode. Received: \"#{@original}\".")
+      raise ArgumentError.new("Cloud Id does not decode. You may need to enable Kibana in the Cloud UI. Received: \"#{@decoded}\".")
     end
-    parts = @decoded.split("$")
-    if parts.any?(&:empty?)
-      raise ArgumentError.new("Cloud Id, after decoding, is invalid. Format: '<part1>$<part2>$<part3>'. Received: \"#{@decoded}\".")
+
+    segments = @decoded.split("$")
+    if segments.any?(&:empty?)
+      raise ArgumentError.new("Cloud Id, after decoding, is invalid. Format: '<segment1>$<segment2>$<segment3>'. Received: \"#{@decoded}\".")
     end
-    cloud_host, es_server, kb_server = parts
-    @elasticsearch_host = sprintf("%s.%s:443", es_server, cloud_host)
-    @kibana_host  = sprintf("%s.%s:443", kb_server, cloud_host)
+    cloud_base = segments.shift
+    cloud_host = "#{DOT_SEPARATOR}#{cloud_base}#{CLOUD_PORT}"
+
+    @elasticsearch_host, @kibana_host = segments
+    if @elasticsearch_host == "undefined"
+      raise ArgumentError.new("Cloud Id, after decoding, elasticsearch segment is 'undefined', literally.")
+    end
+    @elasticsearch_scheme = "https"
+    @elasticsearch_host.concat(cloud_host)
+
+    if @kibana_host == "undefined"
+      raise ArgumentError.new("Cloud Id, after decoding, the kibana segment is 'undefined', literally. You may need to enable Kibana in the Cloud UI.")
+    end
+    @kibana_scheme = "https"
+    @kibana_host.concat(cloud_host)
   end
 
   def to_s
-    @original.to_s
+    @decoded.to_s
   end
 
   def inspect

logstash-core/spec/logstash/modules/kibana_client_spec.rb

@@ -0,0 +1,60 @@
+# encoding: utf-8
+require "spec_helper"
+require "logstash/modules/kibana_client"
+module LogStash module Modules
+  KibanaTestResponse = Struct.new(:code, :body, :headers)
+  class KibanaTestClient
+    def http(method, endpoint, options)
+      self
+    end
+    def call
+      KibanaTestResponse.new(200, '{"version":{"number":"1.2.3","build_snapshot":false}}', {})
+    end
+  end
+  describe KibanaClient do
+    let(:settings) { Hash.new }
+    let(:test_client) { KibanaTestClient.new }
+    let(:kibana_host) { "https://foo.bar:4321" }
+    subject(:kibana_client) { described_class.new(settings, test_client) }
+
+    context "when supplied with conflicting scheme data" do
+      let(:settings) { {"var.kibana.scheme" => "http", "var.kibana.host" => kibana_host} }
+      it "a new instance will throw an error" do
+        expect{described_class.new(settings, test_client)}.to raise_error(ArgumentError, /Detected differing Kibana host schemes as sourced from var\.kibana\.host: 'https' and var\.kibana\.scheme: 'http'/)
+      end
+    end
+
+    context "when supplied with invalid schemes" do
+      ["httpd", "ftp", "telnet"].each do |uri_scheme|
+        it "a new instance will throw an error" do
+          re = /Kibana host scheme given is invalid, given value: '#{uri_scheme}' - acceptable values: 'http', 'https'/
+          expect{described_class.new({"var.kibana.scheme" => uri_scheme}, test_client)}.to raise_error(ArgumentError, re)
+        end
+      end
+    end
+
+    context "when supplied with the scheme in the host only" do
+      let(:settings) { {"var.kibana.host" => kibana_host} }
+      it "has a version and an endpoint" do
+        expect(kibana_client.version).to eq("1.2.3")
+        expect(kibana_client.endpoint).to eq("https://foo.bar:4321")
+      end
+    end
+
+    context "when supplied with the scheme in the scheme setting" do
+      let(:settings) { {"var.kibana.scheme" => "https", "var.kibana.host" => "foo.bar:4321"} }
+      it "has a version and an endpoint" do
+        expect(kibana_client.version).to eq("1.2.3")
+        expect(kibana_client.endpoint).to eq(kibana_host)
+      end
+    end
+
+    context "when supplied with a no scheme host setting and ssl is enabled" do
+      let(:settings) { {"var.kibana.ssl.enabled" => "true", "var.kibana.host" => "foo.bar:4321"} }
+      it "has a version and an endpoint" do
+        expect(kibana_client.version).to eq("1.2.3")
+        expect(kibana_client.endpoint).to eq(kibana_host)
+      end
+    end
+  end
+end end

logstash-core/spec/logstash/modules/settings_merger_spec.rb

@@ -68,7 +68,7 @@ describe LogStash::Modules::SettingsMerger do
         {
           "var.kibana.scheme" => "https",
           "var.kibana.host" => "identifier.us-east-1.aws.found.io:443",
-          "var.elasticsearch.hosts" => "notareal.us-east-1.aws.found.io:443",
+          "var.elasticsearch.hosts" => "https://notareal.us-east-1.aws.found.io:443",
           "var.elasticsearch.username" => "elastix",
           "var.kibana.username" => "elastix"
         }
@@ -93,7 +93,7 @@ describe LogStash::Modules::SettingsMerger do
         {
           "var.kibana.scheme" => "https",
           "var.kibana.host" => "identifier.us-east-1.aws.found.io:443",
-          "var.elasticsearch.hosts" => "notareal.us-east-1.aws.found.io:443",
+          "var.elasticsearch.hosts" => "https://notareal.us-east-1.aws.found.io:443",
         }
       end
       let(:ls_settings) { SubstituteSettingsForRSpec.new({"cloud.id" => cloud_id}) }

logstash-core/spec/logstash/settings/modules_spec.rb

@@ -53,7 +53,7 @@ describe LogStash::Setting::Modules do
     context "when given a badly formatted encoded id" do
       it "should not raise an error" do
         encoded = Base64.urlsafe_encode64("foo$$bal")
-        expect { subject.set(encoded) }.to raise_error(ArgumentError, /Cloud Id, after decoding, is invalid. Format: '<part1>\$<part2>\$<part3>'/)
+        expect { subject.set(encoded) }.to raise_error(ArgumentError, "Cloud Id, after decoding, is invalid. Format: '<segment1>$<segment2>$<segment3>'. Received: \"foo$$bal\".")
       end
     end
 

logstash-core/spec/logstash/util/cloud_setting_id_spec.rb

@@ -0,0 +1,93 @@
+# encoding: utf-8
+require "spec_helper"
+require "logstash/util/cloud_setting_id"
+
+describe LogStash::Util::CloudSettingId do
+  let(:input) { "foobar:dXMtZWFzdC0xLmF3cy5mb3VuZC5pbyRub3RhcmVhbCRpZGVudGlmaWVy" }
+  subject { described_class.new(input) }
+
+  describe "when given unacceptable input" do
+    it "a nil input does not raise an exception" do
+      expect{described_class.new(nil)}.not_to raise_exception
+    end
+    it "when given a nil input, the accessors are all nil" do
+      cloud_id = described_class.new(nil)
+      expect(cloud_id.original).to be_nil
+      expect(cloud_id.decoded).to be_nil
+      expect(cloud_id.label).to be_nil
+      expect(cloud_id.elasticsearch_host).to be_nil
+      expect(cloud_id.kibana_host).to be_nil
+      expect(cloud_id.elasticsearch_scheme).to be_nil
+      expect(cloud_id.kibana_scheme).to be_nil
+    end
+
+    context "when a malformed value is given" do
+      let(:raw) {%w(first second)}
+      let(:input) { described_class.cloud_id_encode(*raw) }
+      it "raises an error" do
+        expect{subject}.to raise_exception(ArgumentError, "Cloud Id does not decode. You may need to enable Kibana in the Cloud UI. Received: \"#{raw[0]}$#{raw[1]}\".")
+      end
+    end
+
+    context "when at least one segment is empty" do
+      let(:raw) {["first", "", "third"]}
+      let(:input) { described_class.cloud_id_encode(*raw) }
+      it "raises an error" do
+        expect{subject}.to raise_exception(ArgumentError, "Cloud Id, after decoding, is invalid. Format: '<segment1>$<segment2>$<segment3>'. Received: \"#{raw[0]}$#{raw[1]}$#{raw[2]}\".")
+      end
+    end
+
+    context "when elasticsearch segment is undefined" do
+      let(:raw) {%w(us-east-1.aws.found.io undefined my-kibana)}
+      let(:input) { described_class.cloud_id_encode(*raw) }
+      it "raises an error" do
+        expect{subject}.to raise_exception(ArgumentError, "Cloud Id, after decoding, elasticsearch segment is 'undefined', literally.")
+      end
+    end
+
+    context "when kibana segment is undefined" do
+      let(:raw) {%w(us-east-1.aws.found.io my-elastic-cluster undefined)}
+      let(:input) { described_class.cloud_id_encode(*raw) }
+      it "raises an error" do
+        expect{subject}.to raise_exception(ArgumentError, "Cloud Id, after decoding, the kibana segment is 'undefined', literally. You may need to enable Kibana in the Cloud UI.")
+      end
+    end
+  end
+
+  describe "without a label" do
+    let(:input) { "dXMtZWFzdC0xLmF3cy5mb3VuZC5pbyRub3RhcmVhbCRpZGVudGlmaWVy" }
+    it "#label is empty" do
+      expect(subject.label).to be_empty
+    end
+    it "#decode is set" do
+      expect(subject.decoded).to eq("us-east-1.aws.found.io$notareal$identifier")
+    end
+  end
+
+  describe "when given acceptable input, the accessors:" do
+    it '#original has a value' do
+      expect(subject.original).to eq(input)
+    end
+    it '#decoded has a value' do
+      expect(subject.decoded).to eq("us-east-1.aws.found.io$notareal$identifier")
+    end
+    it '#label has a value' do
+      expect(subject.label).to eq("foobar")
+    end
+    it '#elasticsearch_host has a value' do
+      expect(subject.elasticsearch_host).to eq("notareal.us-east-1.aws.found.io:443")
+    end
+    it '#elasticsearch_scheme has a value' do
+      expect(subject.elasticsearch_scheme).to eq("https")
+    end
+    it '#kibana_host has a value' do
+      expect(subject.kibana_host).to eq("identifier.us-east-1.aws.found.io:443")
+    end
+    it '#kibana_scheme has a value' do
+      expect(subject.kibana_scheme).to eq("https")
+    end
+    it '#to_s has a value of #decoded' do
+      expect(subject.to_s).to eq(subject.decoded)
+    end
+  end
+end