Merge branch...

Merge branch '13739-elasticsearch-integration-throws-a-401-with-special-character-in-password' into 'master'

Pass URI to elasticsearch client instead of string

See merge request gitlab-org/gitlab!79374

ee/app/models/ee/application_setting.rb

@@ -299,7 +299,9 @@ module EE
     end
 
     def elasticsearch_url
-      read_attribute(:elasticsearch_url).split(',').map(&:strip)
+      read_attribute(:elasticsearch_url).split(',').map do |s|
+        URI.parse(s.strip)
+      end
     end
 
     def elasticsearch_url=(values)
@@ -317,12 +319,10 @@ module EE
     def elasticsearch_url_with_credentials
       return elasticsearch_url if elasticsearch_username.blank?
 
-      elasticsearch_url.map do |url|
-        uri = URI.parse(url)
-
+      elasticsearch_url.map do |uri|
         uri.user = URI.encode_www_form_component(elasticsearch_username)
         uri.password = URI.encode_www_form_component(elasticsearch_password)
-        uri.to_s
+        uri
       end
     end
 

ee/spec/controllers/admin/application_settings_controller_spec.rb

@@ -351,7 +351,7 @@ RSpec.describe Admin::ApplicationSettingsController do
     context 'advanced search settings' do
       it 'updates the advanced search settings' do
         settings = {
-            elasticsearch_url: 'http://my-elastic.search:9200',
+            elasticsearch_url: URI.parse('http://my-elastic.search:9200'),
             elasticsearch_indexing: false,
             elasticsearch_aws: true,
             elasticsearch_aws_access_key: 'elasticsearch_aws_access_key',

ee/spec/models/application_setting_spec.rb

@@ -311,25 +311,25 @@ RSpec.describe ApplicationSetting do
     it 'presents a single URL as a one-element array' do
       setting.elasticsearch_url = 'http://example.com'
 
-      expect(setting.elasticsearch_url).to eq(%w[http://example.com])
+      expect(setting.elasticsearch_url).to match_array([URI.parse('http://example.com')])
     end
 
     it 'presents multiple URLs as a many-element array' do
       setting.elasticsearch_url = 'http://example.com,https://invalid.invalid:9200'
 
-      expect(setting.elasticsearch_url).to eq(%w[http://example.com https://invalid.invalid:9200])
+      expect(setting.elasticsearch_url).to match_array([URI.parse('http://example.com'), URI.parse('https://invalid.invalid:9200')])
     end
 
     it 'strips whitespace from around URLs' do
       setting.elasticsearch_url = ' http://example.com, https://invalid.invalid:9200 '
 
-      expect(setting.elasticsearch_url).to eq(%w[http://example.com https://invalid.invalid:9200])
+      expect(setting.elasticsearch_url).to match_array([URI.parse('http://example.com'), URI.parse('https://invalid.invalid:9200')])
     end
 
     it 'strips trailing slashes from URLs' do
       setting.elasticsearch_url = 'http://example.com/, https://example.com:9200/, https://example.com:9200/prefix//'
 
-      expect(setting.elasticsearch_url).to eq(%w[http://example.com https://example.com:9200 https://example.com:9200/prefix])
+      expect(setting.elasticsearch_url).to match_array([URI.parse('http://example.com'), URI.parse('https://example.com:9200'), URI.parse('https://example.com:9200/prefix')])
     end
   end
 
@@ -339,7 +339,7 @@ RSpec.describe ApplicationSetting do
       setting.elasticsearch_username = 'foo'
       setting.elasticsearch_password = 'bar'
 
-      expect(setting.elasticsearch_url_with_credentials).to eq(%w[http://foo:bar@example.com https://foo:bar@example.org:9200])
+      expect(setting.elasticsearch_url_with_credentials).to match_array([URI.parse('http://foo:bar@example.com'), URI.parse('https://foo:bar@example.org:9200')])
     end
 
     it 'embeds username only' do
@@ -347,7 +347,7 @@ RSpec.describe ApplicationSetting do
       setting.elasticsearch_username = 'foo'
       setting.elasticsearch_password = ''
 
-      expect(setting.elasticsearch_url_with_credentials).to eq(%w[http://foo:@example.com https://foo:@example.org:9200])
+      expect(setting.elasticsearch_url_with_credentials).to match_array([URI.parse('http://foo:@example.com'), URI.parse('https://foo:@example.org:9200')])
     end
 
     it 'overrides existing embedded credentials' do
@@ -355,7 +355,7 @@ RSpec.describe ApplicationSetting do
       setting.elasticsearch_username = 'foo'
       setting.elasticsearch_password = 'bar'
 
-      expect(setting.elasticsearch_url_with_credentials).to eq(%w[http://foo:bar@example.com https://foo:bar@example.org:9200])
+      expect(setting.elasticsearch_url_with_credentials).to match_array([URI.parse('http://foo:bar@example.com'), URI.parse('https://foo:bar@example.org:9200')])
     end
 
     it 'returns original url if credentials blank' do
@@ -363,7 +363,7 @@ RSpec.describe ApplicationSetting do
       setting.elasticsearch_username = ''
       setting.elasticsearch_password = ''
 
-      expect(setting.elasticsearch_url_with_credentials).to eq(%w[http://username:password@example.com https://test:test@example.org:9200])
+      expect(setting.elasticsearch_url_with_credentials).to match_array([URI.parse('http://username:password@example.com'), URI.parse('https://test:test@example.org:9200')])
     end
 
     it 'encodes the credentials' do
@@ -371,9 +371,9 @@ RSpec.describe ApplicationSetting do
       setting.elasticsearch_username = 'foo/admin'
       setting.elasticsearch_password = 'b@r'
 
-      expect(setting.elasticsearch_url_with_credentials).to eq(%w[
-        http://foo%2Fadmin:b%40r@example.com
-        https://foo%2Fadmin:b%40r@example.org:9200
+      expect(setting.elasticsearch_url_with_credentials).to match_array([
+        URI.parse('http://foo%2Fadmin:b%40r@example.com'),
+        URI.parse('https://foo%2Fadmin:b%40r@example.org:9200')
       ])
     end
   end
@@ -404,7 +404,7 @@ RSpec.describe ApplicationSetting do
       )
 
       expect(setting.elasticsearch_config).to eq(
-        url: ['http://foo:bar@example.com:9200'],
+        url: [URI.parse('http://foo:bar@example.com:9200')],
         aws: false,
         aws_region:     'test-region',
         aws_access_key: 'test-access-key',