Add latest changes from gitlab-org/gitlab@master

9 files changed, 277 insertions, 106 deletions

diff --git a/spec/lib/gitlab/database/connection_spec.rb b/spec/lib/gitlab/database/connection_spec.rb
index 517d40deb1c..52e43fb0f61 100644
--- a/spec/lib/gitlab/database/connection_spec.rb
+++ b/spec/lib/gitlab/database/connection_spec.rb
@@ -378,42 +378,6 @@ RSpec.describe Gitlab::Database::Connection do
     end
   end
 
-  describe '#create_connection_pool' do
-    it 'creates a new connection pool with specific pool size' do
-      pool = connection.create_connection_pool(5)
-
-      begin
-        expect(pool)
-          .to be_kind_of(ActiveRecord::ConnectionAdapters::ConnectionPool)
-
-        expect(pool.db_config.pool).to eq(5)
-      ensure
-        pool.disconnect!
-      end
-    end
-
-    it 'allows setting of a custom hostname' do
-      pool = connection.create_connection_pool(5, '127.0.0.1')
-
-      begin
-        expect(pool.db_config.host).to eq('127.0.0.1')
-      ensure
-        pool.disconnect!
-      end
-    end
-
-    it 'allows setting of a custom hostname and port' do
-      pool = connection.create_connection_pool(5, '127.0.0.1', 5432)
-
-      begin
-        expect(pool.db_config.host).to eq('127.0.0.1')
-        expect(pool.db_config.configuration_hash[:port]).to eq(5432)
-      ensure
-        pool.disconnect!
-      end
-    end
-  end
-
   describe '#cached_column_exists?' do
     it 'only retrieves data once' do
       expect(connection.scope.connection)
diff --git a/spec/lib/gitlab/database/load_balancing/host_list_spec.rb b/spec/lib/gitlab/database/load_balancing/host_list_spec.rb
index 6b88505de1a..6a358b5d430 100644
--- a/spec/lib/gitlab/database/load_balancing/host_list_spec.rb
+++ b/spec/lib/gitlab/database/load_balancing/host_list_spec.rb
@@ -3,25 +3,17 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Database::LoadBalancing::HostList do
-  def expect_metrics(hosts)
-    expect(Gitlab::Metrics.registry.get(:db_load_balancing_hosts).get({})).to eq(hosts)
-  end
-
-  before do
-    allow(Gitlab::Database.main)
-      .to receive(:create_connection_pool)
-      .and_return(ActiveRecord::Base.connection_pool)
-  end
-
+  let(:db_host) { ActiveRecord::Base.connection_pool.db_config.host }
   let(:load_balancer) { double(:load_balancer) }
   let(:host_count) { 2 }
+  let(:hosts) { Array.new(host_count) { Gitlab::Database::LoadBalancing::Host.new(db_host, load_balancer, port: 5432) } }
+  let(:host_list) { described_class.new(hosts) }
 
-  let(:host_list) do
-    hosts = Array.new(host_count) do
-      Gitlab::Database::LoadBalancing::Host.new('localhost', load_balancer, port: 5432)
+  before do
+    # each call generate a new replica pool
+    allow(load_balancer).to receive(:create_replica_connection_pool) do
+      double(:replica_connection_pool)
     end
-
-    described_class.new(hosts)
   end
 
   describe '#initialize' do
@@ -42,8 +34,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::HostList do
     context 'with ports' do
       it 'returns the host names of all hosts' do
         hosts = [
-          ['localhost', 5432],
-          ['localhost', 5432]
+          [db_host, 5432],
+          [db_host, 5432]
         ]
 
         expect(host_list.host_names_and_ports).to eq(hosts)
@@ -51,18 +43,12 @@ RSpec.describe Gitlab::Database::LoadBalancing::HostList do
     end
 
     context 'without ports' do
-      let(:host_list) do
-        hosts = Array.new(2) do
-          Gitlab::Database::LoadBalancing::Host.new('localhost', load_balancer)
-        end
-
-        described_class.new(hosts)
-      end
+      let(:hosts) { Array.new(2) { Gitlab::Database::LoadBalancing::Host.new(db_host, load_balancer) } }
 
       it 'returns the host names of all hosts' do
         hosts = [
-          ['localhost', nil],
-          ['localhost', nil]
+          [db_host, nil],
+          [db_host, nil]
         ]
 
         expect(host_list.host_names_and_ports).to eq(hosts)
@@ -71,10 +57,6 @@ RSpec.describe Gitlab::Database::LoadBalancing::HostList do
   end
 
   describe '#manage_pool?' do
-    before do
-      allow(Gitlab::Database.main).to receive(:create_connection_pool) { double(:connection) }
-    end
-
     context 'when the testing pool belongs to one host of the host list' do
       it 'returns true' do
         pool = host_list.hosts.first.pool
@@ -185,4 +167,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::HostList do
       end
     end
   end
+
+  def expect_metrics(hosts)
+    expect(Gitlab::Metrics.registry.get(:db_load_balancing_hosts).get({})).to eq(hosts)
+  end
 end
diff --git a/spec/lib/gitlab/database/load_balancing/host_spec.rb b/spec/lib/gitlab/database/load_balancing/host_spec.rb
index 23467e0ae34..f42ac8be1bb 100644
--- a/spec/lib/gitlab/database/load_balancing/host_spec.rb
+++ b/spec/lib/gitlab/database/load_balancing/host_spec.rb
@@ -3,15 +3,16 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Database::LoadBalancing::Host do
-  let(:load_balancer) do
-    Gitlab::Database::LoadBalancing::LoadBalancer.new(%w[localhost])
-  end
+  let(:load_balancer) { Gitlab::Database::LoadBalancing::LoadBalancer.new }
 
-  let(:host) { load_balancer.host_list.hosts.first }
+  let(:host) do
+    Gitlab::Database::LoadBalancing::Host.new('localhost', load_balancer)
+  end
 
   before do
-    allow(Gitlab::Database.main).to receive(:create_connection_pool)
-      .and_return(ActiveRecord::Base.connection_pool)
+    allow(load_balancer).to receive(:create_replica_connection_pool) do
+      ActiveRecord::Base.connection_pool
+    end
   end
 
   def raise_and_wrap(wrapper, original)
@@ -63,7 +64,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::Host do
       expect(host.pool)
         .to receive(:disconnect!)
 
-      host.disconnect!(1)
+      host.disconnect!(timeout: 1)
     end
   end
 
diff --git a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb
index 01a17cb2805..358f382bc39 100644
--- a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb
+++ b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb
@@ -3,21 +3,22 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
-  let(:pool) { Gitlab::Database.main.create_connection_pool(2) }
   let(:conflict_error) { Class.new(RuntimeError) }
-
-  let(:lb) { described_class.new(%w(localhost localhost)) }
+  let(:db_host) { ActiveRecord::Base.connection_pool.db_config.host }
+  let(:lb) { described_class.new([db_host, db_host]) }
   let(:request_cache) { lb.send(:request_cache) }
 
   before do
-    allow(Gitlab::Database.main).to receive(:create_connection_pool)
-      .and_return(pool)
     stub_const(
       'Gitlab::Database::LoadBalancing::LoadBalancer::PG::TRSerializationFailure',
       conflict_error
     )
   end
 
+  after do |example|
+    lb.disconnect!(timeout: 0) unless example.metadata[:skip_disconnect]
+  end
+
   def raise_and_wrap(wrapper, original)
     raise original
   rescue original.class
@@ -239,7 +240,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
     context 'when the connection comes from the primary pool' do
       it 'returns :primary' do
         connection = double(:connection)
-        allow(connection).to receive(:pool).and_return(ActiveRecord::Base.connection_pool)
+        allow(connection).to receive(:pool).and_return(lb.send(:pool))
 
         expect(lb.db_role_for_connection(connection)).to be(:primary)
       end
@@ -271,8 +272,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
     end
 
     it 'does not create conflicts with other load balancers when caching hosts' do
-      lb1 = described_class.new(%w(localhost localhost), ActiveRecord::Base)
-      lb2 = described_class.new(%w(localhost localhost), Ci::CiDatabaseRecord)
+      lb1 = described_class.new([db_host, db_host], ActiveRecord::Base)
+      lb2 = described_class.new([db_host, db_host], Ci::CiDatabaseRecord)
 
       host1 = lb1.host
       host2 = lb2.host
@@ -456,4 +457,45 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do
       end
     end
   end
+
+  describe '#create_replica_connection_pool' do
+    it 'creates a new connection pool with specific pool size and name' do
+      with_replica_pool(5, 'other_host') do |replica_pool|
+        expect(replica_pool)
+          .to be_kind_of(ActiveRecord::ConnectionAdapters::ConnectionPool)
+
+        expect(replica_pool.db_config.host).to eq('other_host')
+        expect(replica_pool.db_config.pool).to eq(5)
+        expect(replica_pool.db_config.name).to end_with("_replica")
+      end
+    end
+
+    it 'allows setting of a custom hostname and port' do
+      with_replica_pool(5, 'other_host', 5432) do |replica_pool|
+        expect(replica_pool.db_config.host).to eq('other_host')
+        expect(replica_pool.db_config.configuration_hash[:port]).to eq(5432)
+      end
+    end
+
+    it 'does not modify connection class pool' do
+      expect { with_replica_pool(5) { } }.not_to change { ActiveRecord::Base.connection_pool }
+    end
+
+    def with_replica_pool(*args)
+      pool = lb.create_replica_connection_pool(*args)
+      yield pool
+    ensure
+      pool&.disconnect!
+    end
+  end
+
+  describe '#disconnect!' do
+    it 'calls disconnect on all hosts with a timeout', :skip_disconnect do
+      expect_next_instances_of(Gitlab::Database::LoadBalancing::Host, 2) do |host|
+        expect(host).to receive(:disconnect!).with(timeout: 30)
+      end
+
+      lb.disconnect!(timeout: 30)
+    end
+  end
 end
diff --git a/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb b/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb
index b65b68c463e..c853e827144 100644
--- a/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb
+++ b/spec/lib/gitlab/database/load_balancing/service_discovery_spec.rb
@@ -169,7 +169,7 @@ RSpec.describe Gitlab::Database::LoadBalancing::ServiceDiscovery do
 
       expect(host)
         .to receive(:disconnect!)
-        .with(2)
+        .with(timeout: 2)
 
       service.replace_hosts([address_bar])
     end
diff --git a/spec/lib/gitlab/database/load_balancing_spec.rb b/spec/lib/gitlab/database/load_balancing_spec.rb
index ba5aae110ca..fb482061d7c 100644
--- a/spec/lib/gitlab/database/load_balancing_spec.rb
+++ b/spec/lib/gitlab/database/load_balancing_spec.rb
@@ -3,10 +3,6 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Database::LoadBalancing do
-  before do
-    stub_env('ENABLE_LOAD_BALANCING_FOR_FOSS', 'true')
-  end
-
   describe '.proxy' do
     before do
       @previous_proxy = ActiveRecord::Base.load_balancing_proxy
diff --git a/spec/lib/gitlab/usage/metric_definition_spec.rb b/spec/lib/gitlab/usage/metric_definition_spec.rb
index f3c3e5fc550..a7cff80e43a 100644
--- a/spec/lib/gitlab/usage/metric_definition_spec.rb
+++ b/spec/lib/gitlab/usage/metric_definition_spec.rb
@@ -87,14 +87,14 @@ RSpec.describe Gitlab::Usage::MetricDefinition do
       end
 
       it 'raise exception' do
-        expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once).with(instance_of(Gitlab::Usage::Metric::InvalidMetricError))
+        expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once).with(instance_of(Gitlab::Usage::MetricDefinition::InvalidError))
 
         described_class.new(path, attributes).validate!
       end
 
       context 'with skip_validation' do
         it 'raise exception if skip_validation: false' do
-          expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once).with(instance_of(Gitlab::Usage::Metric::InvalidMetricError))
+          expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once).with(instance_of(Gitlab::Usage::MetricDefinition::InvalidError))
 
           described_class.new(path, attributes.merge( { skip_validation: false } )).validate!
         end
@@ -113,7 +113,7 @@ RSpec.describe Gitlab::Usage::MetricDefinition do
           attributes[:status] = 'broken'
           attributes.delete(:repair_issue_url)
 
-          expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once).with(instance_of(Gitlab::Usage::Metric::InvalidMetricError))
+          expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once).with(instance_of(Gitlab::Usage::MetricDefinition::InvalidError))
 
           described_class.new(path, attributes).validate!
         end
@@ -173,7 +173,7 @@ RSpec.describe Gitlab::Usage::MetricDefinition do
       write_metric(metric1, path, yaml_content)
       write_metric(metric2, path, yaml_content)
 
-      expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).with(instance_of(Gitlab::Usage::Metric::InvalidMetricError))
+      expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).with(instance_of(Gitlab::Usage::MetricDefinition::InvalidError))
 
       subject
     end
diff --git a/spec/lib/gitlab/usage/metric_spec.rb b/spec/lib/gitlab/usage/metric_spec.rb
index d4a789419a4..d83f59e4a7d 100644
--- a/spec/lib/gitlab/usage/metric_spec.rb
+++ b/spec/lib/gitlab/usage/metric_spec.rb
@@ -3,27 +3,46 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Usage::Metric do
-  describe '#definition' do
-    it 'returns key_path metric definiton' do
-      expect(described_class.new(key_path: 'uuid').definition).to be_an(Gitlab::Usage::MetricDefinition)
-    end
+  let!(:issue) { create(:issue) }
+
+  let(:attributes) do
+    {
+      data_category: "Operational",
+      key_path: "counts.issues",
+      description: "Count of Issues created",
+      product_section: "dev",
+      product_stage: "plan",
+      product_group: "group::plan",
+      product_category: "issue_tracking",
+      value_type: "number",
+      status: "data_available",
+      time_frame: "all",
+      data_source: "database",
+      instrumentation_class: "CountIssuesMetric",
+      distribution: %w(ce ee),
+      tier: %w(free premium ultimate)
+    }
   end
 
-  describe '#unflatten_default_path' do
-    using RSpec::Parameterized::TableSyntax
+  let(:issue_count_metric_definiton) do
+    double(:issue_count_metric_definiton,
+      attributes.merge({ attributes: attributes })
+    )
+  end
 
-    where(:key_path, :value, :expected_hash) do
-      'uuid'                                     | nil    | { uuid: nil }
-      'uuid'                                     | '1111' | { uuid: '1111' }
-      'counts.issues'                            | nil    | { counts: { issues: nil } }
-      'counts.issues'                            | 100    | { counts: { issues: 100 } }
-      'usage_activity_by_stage.verify.ci_builds' | 100    | { usage_activity_by_stage: { verify: { ci_builds: 100 } } }
-    end
+  before do
+    allow(ApplicationRecord.connection).to receive(:transaction_open?).and_return(false)
+  end
 
-    with_them do
-      subject { described_class.new(key_path: key_path, value: value).unflatten_key_path }
+  describe '#with_value' do
+    it 'returns key_path metric with the corresponding value' do
+      expect(described_class.new(issue_count_metric_definiton).with_value).to eq({ counts: { issues: 1 } })
+    end
+  end
 
-      it { is_expected.to eq(expected_hash) }
+  describe '#with_instrumentation' do
+    it 'returns key_path metric with the corresponding generated query' do
+      expect(described_class.new(issue_count_metric_definiton).with_instrumentation).to eq({ counts: { issues: "SELECT COUNT(\"issues\".\"id\") FROM \"issues\"" } })
     end
   end
 end
diff --git a/spec/lib/sidebars/groups/menus/packages_registries_menu_spec.rb b/spec/lib/sidebars/groups/menus/packages_registries_menu_spec.rb
new file mode 100644
index 00000000000..5ebd67462f8
--- /dev/null
+++ b/spec/lib/sidebars/groups/menus/packages_registries_menu_spec.rb
@@ -0,0 +1,163 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Groups::Menus::PackagesRegistriesMenu do
+  let_it_be(:owner) { create(:user) }
+  let_it_be(:group) do
+    build(:group, :private).tap do |g|
+      g.add_owner(owner)
+    end
+  end
+
+  let(:user) { owner }
+  let(:context) { Sidebars::Groups::Context.new(current_user: user, container: group) }
+  let(:menu) { described_class.new(context) }
+
+  describe '#render?' do
+    context 'when menu has menu items to show' do
+      it 'returns true' do
+        expect(menu.render?).to eq true
+      end
+    end
+
+    context 'when menu does not have any menu item to show' do
+      it 'returns false' do
+        stub_container_registry_config(enabled: false)
+        stub_config(packages: { enabled: false })
+        stub_config(dependency_proxy: { enabled: false })
+
+        expect(menu.render?).to eq false
+      end
+    end
+  end
+
+  describe '#link' do
+    let(:registry_enabled) { true }
+    let(:packages_enabled) { true }
+
+    before do
+      stub_container_registry_config(enabled: registry_enabled)
+      stub_config(packages: { enabled: packages_enabled })
+      stub_config(dependency_proxy: { enabled: true })
+    end
+
+    subject { menu.link }
+
+    context 'when Packages Registry is visible' do
+      it 'menu link points to Packages Registry page' do
+        expect(subject).to eq find_menu(menu, :packages_registry).link
+      end
+    end
+
+    context 'when Packages Registry is not visible' do
+      let(:packages_enabled) { false }
+
+      it 'menu link points to Container Registry page' do
+        expect(subject).to eq find_menu(menu, :container_registry).link
+      end
+
+      context 'when Container Registry is not visible' do
+        let(:registry_enabled) { false }
+
+        it 'menu link points to Dependency Proxy page' do
+          expect(subject).to eq find_menu(menu, :dependency_proxy).link
+        end
+      end
+    end
+  end
+
+  describe 'Menu items' do
+    subject { find_menu(menu, item_id) }
+
+    describe 'Packages Registry' do
+      let(:item_id) { :packages_registry }
+
+      context 'when user can read packages' do
+        before do
+          stub_config(packages: { enabled: packages_enabled })
+        end
+
+        context 'when config package setting is disabled' do
+          let(:packages_enabled) { false }
+
+          it 'the menu item is not added to list of menu items' do
+            is_expected.to be_nil
+          end
+        end
+
+        context 'when config package setting is enabled' do
+          let(:packages_enabled) { true }
+
+          it 'the menu item is added to list of menu items' do
+            is_expected.not_to be_nil
+          end
+        end
+      end
+    end
+
+    describe 'Container Registry' do
+      let(:item_id) { :container_registry }
+
+      context 'when user can read container images' do
+        before do
+          stub_container_registry_config(enabled: container_enabled)
+        end
+
+        context 'when config registry setting is disabled' do
+          let(:container_enabled) { false }
+
+          it 'the menu item is not added to list of menu items' do
+            is_expected.to be_nil
+          end
+        end
+
+        context 'when config registry setting is enabled' do
+          let(:container_enabled) { true }
+
+          it 'the menu item is added to list of menu items' do
+            is_expected.not_to be_nil
+          end
+
+          context 'when user cannot read container images' do
+            let(:user) { nil }
+
+            it 'the menu item is not added to list of menu items' do
+              is_expected.to be_nil
+            end
+          end
+        end
+      end
+    end
+
+    describe 'Dependency Proxy' do
+      let(:item_id) { :dependency_proxy }
+
+      before do
+        stub_config(dependency_proxy: { enabled: dependency_enabled })
+      end
+
+      context 'when config dependency_proxy is enabled' do
+        let(:dependency_enabled) { true }
+
+        it 'the menu item is added to list of menu items' do
+          is_expected.not_to be_nil
+        end
+      end
+
+      context 'when config dependency_proxy is not enabled' do
+        let(:dependency_enabled) { false }
+
+        it 'the menu item is not added to list of menu items' do
+          is_expected.to be_nil
+        end
+      end
+    end
+  end
+
+  private
+
+  def find_menu(menu, item)
+    menu.renderable_items.find { |i| i.item_id == item }
+  end
+end