Add latest changes from gitlab-org/gitlab@master

config/feature_flags/development/vulnerability_finding_replace_metadata.yml

@@ -0,0 +1,7 @@
+---
+name: vulnerability_finding_replace_metadata
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66868
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/337253
+group: group::threat insights
+type: development
+default_enabled: false

config/metrics/counts_all/20210216182006_source_code_pushes.yml

@@ -10,6 +10,10 @@ value_type: number
 status: data_available
 time_frame: all
 data_source: redis
+instrumentation_class: RedisMetric
+options:
+  counter_class: SourceCodeCounter
+  event: pushes
 distribution:
 - ce
 - ee

doc/development/service_ping/metrics_instrumentation.md

@@ -11,7 +11,7 @@ This guide describes how to develop Service Ping metrics using metrics instrumen
 ## Nomenclature
 
 - **Instrumentation class**:
-  - Inherits one of the metric classes: `DatabaseMetric`, `RedisHLLMetric` or `GenericMetric`.
+  - Inherits one of the metric classes: `DatabaseMetric`, `RedisMetric`, `RedisHLLMetric` or `GenericMetric`.
   - Implements the logic that calculates the value for a Service Ping metric.
 
 - **Metric definition**
@@ -24,7 +24,7 @@ This guide describes how to develop Service Ping metrics using metrics instrumen
 
 A metric definition has the [`instrumentation_class`](metrics_dictionary.md) field, which can be set to a class.
 
-The defined instrumentation class should have one of the existing metric classes: `DatabaseMetric`, `RedisHLLMetric`, or `GenericMetric`.
+The defined instrumentation class should have one of the existing metric classes: `DatabaseMetric`, `RedisMetric`, `RedisHLLMetric`, or `GenericMetric`.
 
 Using the instrumentation classes ensures that metrics can fail safe individually, without breaking the entire
  process of Service Ping generation.
@@ -51,6 +51,26 @@ module Gitlab
 end
 ```
 
+## Redis metrics
+
+[Example of a merge request that adds a `Redis` metric](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66582).
+
+Count unique values for `source_code_pushes` event.
+
+Required options:
+
+- `event`: the event name.
+- `counter_class`: one of the counter classes from the `Gitlab::UsageDataCounters` namespace; it should implement `read` method or inherit it from `BaseCounter`.
+
+```yaml
+time_frame: all
+data_source: redis
+instrumentation_class: 'RedisMetric'
+options:
+  event: pushes
+  counter_class: SourceCodeCounter
+```
+
 ## Redis HyperLogLog metrics
 
 [Example of a merge request that adds a `RedisHLL` metric](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61685).
@@ -60,7 +80,7 @@ Count unique values for `i_quickactions_approve` event.
 ```yaml
 time_frame: 28d
 data_source: redis_hll
-instrumentation_class: 'Gitlab::Usage::Metrics::Instrumentations::RedisHLLMetric'
+instrumentation_class: 'RedisHLLMetric'
 options:
   events:
     - i_quickactions_approve
@@ -91,13 +111,13 @@ end
 There is support for:
 
 - `count`, `distinct_count` for [database metrics](#database-metrics).
+- [Redis metrics](#redis-metrics).
 - [Redis HLL metrics](#redis-hyperloglog-metrics).
 - [Generic metrics](#generic-metrics), which are metrics based on settings or configurations.
 
 Currently, there is no support for:
 
 - `add`, `sum`, `histogram`, `estimate_batch_distinct_count` for database metrics.
-- Regular Redis counters.
 
 You can [track the progress to support these](https://gitlab.com/groups/gitlab-org/-/epics/6118).
 
@@ -107,7 +127,7 @@ To create a stub instrumentation for a Service Ping metric, you can use a dedica
 
 The generator takes the class name as an argument and the following options:
 
-- `--type=TYPE` Required. Indicates the metric type. It must be one of: `database`, `generic`.
+- `--type=TYPE` Required. Indicates the metric type. It must be one of: `database`, `generic`, `redis`.
 - `--operation` Required for `database` type. It must be one of: `count`, `distinct_count`.
 - `--ee` Indicates if the metric is for EE.
 

lib/backup/manager.rb

@@ -232,7 +232,7 @@ module Backup
     end
 
     def folders_to_backup
-      FOLDERS_TO_BACKUP.reject { |name| skipped?(name) }
+      FOLDERS_TO_BACKUP.select { |name| !skipped?(name) && Dir.exist?(File.join(backup_path, name)) }
     end
 
     def disabled_features

lib/generators/gitlab/usage_metric/usage_metric_generator.rb

@@ -11,7 +11,8 @@ module Gitlab
 
     ALLOWED_SUPERCLASSES = {
       generic: 'Generic',
-      database: 'Database'
+      database: 'Database',
+      redis: 'Redis'
     }.freeze
 
     ALLOWED_OPERATIONS = %w(count distinct_count).freeze

lib/gitlab/usage/metrics/instrumentations/redis_metric.rb

@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Usage
+    module Metrics
+      module Instrumentations
+        # Usage example
+        #
+        # In metric YAML definition:
+        #
+        # instrumentation_class: RedisMetric
+        # options:
+        #   event: pushes
+        #   counter_class: SourceCodeCounter
+        #
+        class RedisMetric < BaseMetric
+          def initialize(time_frame:, options: {})
+            super
+
+            raise ArgumentError, "'event' option is required" unless metric_event.present?
+            raise ArgumentError, "'counter class' option is required" unless counter_class.present?
+          end
+
+          def metric_event
+            options[:event]
+          end
+
+          def counter_class_name
+            options[:counter_class]
+          end
+
+          def counter_class
+            "Gitlab::UsageDataCounters::#{counter_class_name}".constantize
+          end
+
+          def value
+            redis_usage_data do
+              counter_class.read(metric_event)
+            end
+          end
+
+          def suggested_name
+            Gitlab::Usage::Metrics::NameSuggestion.for(:redis)
+          end
+        end
+      end
+    end
+  end
+end

lib/sidebars/projects/menus/packages_registries_menu.rb

@@ -31,7 +31,7 @@ module Sidebars
         private
 
         def packages_registry_menu_item
-          if !::Gitlab.config.packages.enabled || !can?(context.current_user, :read_package, context.project)
+          if packages_registry_disabled?
             return ::Sidebars::NilMenuItem.new(item_id: :packages_registry)
           end
 
@@ -58,7 +58,7 @@ module Sidebars
         end
 
         def infrastructure_registry_menu_item
-          if Feature.disabled?(:infrastructure_registry_page, context.current_user, default_enabled: :yaml)
+          if Feature.disabled?(:infrastructure_registry_page, context.current_user, default_enabled: :yaml) || packages_registry_disabled?
             return ::Sidebars::NilMenuItem.new(item_id: :infrastructure_registry)
           end
 
@@ -69,6 +69,10 @@ module Sidebars
             item_id: :infrastructure_registry
           )
         end
+
+        def packages_registry_disabled?
+          !::Gitlab.config.packages.enabled || !can?(context.current_user, :read_package, context.project)
+        end
       end
     end
   end

qa/qa/specs/features/api/1_manage/import_large_github_repo_spec.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require 'octokit'
-require 'parallel'
 
 # rubocop:disable Rails/Pluck
 module QA
@@ -21,11 +20,10 @@ module QA
       let(:user) do
         Resource::User.fabricate_via_api! do |resource|
           resource.api_client = api_client
-          resource.hard_delete_on_api_removal = true
         end
       end
 
-      let(:github_repo) { 'rspec/rspec-core' }
+      let(:github_repo) { ENV['QA_LARGE_GH_IMPORT_REPO'] || 'rspec/rspec-core' }
       let(:github_client) do
         Octokit.middleware = Faraday::RackBuilder.new do |builder|
           builder.response(:logger, logger, headers: false, bodies: false)
@@ -98,13 +96,19 @@ module QA
       end
 
       after do |example|
+        # skip saving data if example is skipped or failed before import finished
         next if example.pending?
 
+        user.remove_via_api!
+        next unless defined?(@import_time)
+
         # save data for comparison after run finished
         save_json(
           "data",
           {
+            import_time: @import_time,
             github: {
+              project_name: github_repo,
               branches: gh_branches,
               commits: gh_commits,
               labels: gh_labels,
@@ -113,6 +117,7 @@ module QA
               issues: gh_issues
             },
             gitlab: {
+              project_name: imported_project.path_with_namespace,
               branches: gl_branches,
               commits: gl_commits,
               labels: gl_labels,
@@ -125,6 +130,8 @@ module QA
       end
 
       it 'imports large Github repo via api' do
+        start = Time.now
+
         imported_project # import the project
         fetch_github_objects # fetch all objects right after import has started
 
@@ -132,6 +139,7 @@ module QA
           duration: 3600,
           interval: 30
         )
+        @import_time = Time.now - start
 
         aggregate_failures do
           verify_repository_import
@@ -146,7 +154,7 @@ module QA
       #
       # @return [void]
       def fetch_github_objects
-        logger.debug("Fetching objects for github repo: '#{github_repo}'")
+        logger.debug("== Fetching objects for github repo: '#{github_repo}' ==")
 
         gh_repo
         gh_branches
@@ -161,7 +169,7 @@ module QA
       #
       # @return [void]
       def verify_repository_import
-        logger.debug("Verifying repository import")
+        logger.debug("== Verifying repository import ==")
         expect(imported_project.description).to eq(gh_repo.description)
         # check via include, importer creates more branches
         # https://gitlab.com/gitlab-org/gitlab/-/issues/332711
@@ -173,7 +181,7 @@ module QA
       #
       # @return [void]
       def verify_merge_requests_import
-        logger.debug("Verifying merge request import")
+        logger.debug("== Verifying merge request import ==")
         verify_mrs_or_issues('mr')
       end
 
@@ -181,7 +189,7 @@ module QA
       #
       # @return [void]
       def verify_issues_import
-        logger.debug("Verifying issue import")
+        logger.debug("== Verifying issue import ==")
         verify_mrs_or_issues('issue')
       end
 
@@ -189,15 +197,16 @@ module QA
       #
       # @return [void]
       def verify_labels_import
-        logger.debug("Verifying label import")
-        expect(gl_labels).to match_array(gh_labels)
+        logger.debug("== Verifying label import ==")
+        # check via include, additional labels can be inherited from parent group
+        expect(gl_labels).to include(*gh_labels)
       end
 
       # Verify milestones import
       #
       # @return [void]
       def verify_milestones_import
-        logger.debug("Verifying milestones import")
+        logger.debug("== Verifying milestones import ==")
         expect(gl_milestones).to match_array(gh_milestones)
       end
 
@@ -217,8 +226,9 @@ module QA
           eq(actual.length),
           "Expected to contain same amount of #{type}s. Expected: #{expected.length}, actual: #{actual.length}"
         )
+        logger.debug("= Comparing #{type}s =")
         actual.each do |title, actual_item|
-          logger.debug("Comparing #{type} with title '#{title}'")
+          print "." # indicate that it is still going but don't spam the output with newlines
 
           expected_item = expected[title]
 
@@ -235,34 +245,47 @@ module QA
           )
           expect(expected_item[:comments]).to match_array(actual_item[:comments])
         end
+        puts # print newline after last print to make output pretty
       end
 
       # Imported project branches
       #
       # @return [Array]
       def gl_branches
-        @gl_branches ||= imported_project.repository_branches(auto_paginate: true).map { |b| b[:name] }
+        @gl_branches ||= begin
+          logger.debug("= Fetching branches =")
+          imported_project.repository_branches(auto_paginate: true).map { |b| b[:name] }
+        end
       end
 
       # Imported project commits
       #
       # @return [Array]
       def gl_commits
-        @gl_commits ||= imported_project.commits(auto_paginate: true).map { |c| c[:id] }
+        @gl_commits ||= begin
+          logger.debug("= Fetching commits =")
+          imported_project.commits(auto_paginate: true).map { |c| c[:id] }
+        end
       end
 
       # Imported project labels
       #
       # @return [Array]
       def gl_labels
-        @gl_labels ||= imported_project.labels(auto_paginate: true).map { |label| label.slice(:name, :color) }
+        @gl_labels ||= begin
+          logger.debug("= Fetching labels =")
+          imported_project.labels(auto_paginate: true).map { |label| label.slice(:name, :color) }
+        end
       end
 
       # Imported project milestones
       #
       # @return [<Type>] <description>
       def gl_milestones
-        @gl_milestones ||= imported_project.milestones(auto_paginate: true).map { |ms| ms.slice(:title, :description) }
+        @gl_milestones ||= begin
+          logger.debug("= Fetching milestones =")
+          imported_project.milestones(auto_paginate: true).map { |ms| ms.slice(:title, :description) }
+        end
       end
 
       # Imported project merge requests
@@ -270,19 +293,17 @@ module QA
       # @return [Hash]
       def mrs
         @mrs ||= begin
-          logger.debug("Fetching merge requests")
+          logger.debug("= Fetching merge requests =")
           imported_mrs = imported_project.merge_requests(auto_paginate: true)
-          # fetch comments in parallel since we need to do it for each mr separately
-          logger.debug("Transforming merge request objects for comparison")
-          mrs_hashes = Parallel.map(imported_mrs) do |mr|
+          logger.debug("= Transforming merge request objects for comparison =")
+          imported_mrs.each_with_object({}) do |mr, hash|
             resource = Resource::MergeRequest.init do |resource|
               resource.project = imported_project
               resource.iid = mr[:iid]
               resource.api_client = api_client
             end
 
-            {
-              title: mr[:title],
+            hash[mr[:title]] = {
               body: mr[:description],
               comments: resource.comments(auto_paginate: true)
                 # remove system notes
@@ -290,13 +311,6 @@ module QA
                 .map { |c| sanitize(c[:body]) }
             }
           end
-
-          mrs_hashes.each_with_object({}) do |mr, hash|
-            hash[mr[:title]] = {
-              body: mr[:body],
-              comments: mr[:comments]
-            }
-          end
         end
       end
 
@@ -305,30 +319,21 @@ module QA
       # @return [Hash]
       def gl_issues
         @gl_issues ||= begin
-          logger.debug("Fetching issues")
+          logger.debug("= Fetching issues =")
           imported_issues = imported_project.issues(auto_paginate: true)
-          # fetch comments in parallel since we need to do it for each mr separately
-          logger.debug("Transforming issue objects for comparison")
-          issue_hashes = Parallel.map(imported_issues) do |issue|
+          logger.debug("= Transforming issue objects for comparison =")
+          imported_issues.each_with_object({}) do |issue, hash|
             resource = Resource::Issue.init do |issue_resource|
               issue_resource.project = imported_project
               issue_resource.iid = issue[:iid]
               issue_resource.api_client = api_client
             end
 
-            {
-              title: issue[:title],
+            hash[issue[:title]] = {
               body: issue[:description],
               comments: resource.comments(auto_paginate: true).map { |c| sanitize(c[:body]) }
             }
           end
-
-          issue_hashes.each_with_object({}) do |issue, hash|
-            hash[issue[:title]] = {
-              body: issue[:body],
-              comments: issue[:comments]
-            }
-          end
         end
       end
 

qa/spec/support/matchers/eventually_matcher.rb

@@ -55,12 +55,13 @@ module Matchers
       def wait_and_check(actual, expectation_name)
         attempt = 0
 
+        QA::Runtime::Logger.debug("Running eventually matcher with '#{operator_msg}' operator")
         QA::Support::Retrier.retry_until(
           max_attempts: @attempts,
           max_duration: @duration,
           sleep_interval: @interval || 0.5
         ) do
-          QA::Runtime::Logger.debug("Evaluating expectation '#{operator_msg}', attempt: #{attempt += 1}")
+          QA::Runtime::Logger.debug("evaluating expectation, attempt: #{attempt += 1}")
 
           public_send(expectation_name, actual)
         rescue RSpec::Expectations::ExpectationNotMetError, QA::Resource::ApiFabricator::ResourceNotFoundError

rubocop/rubocop-usage-data.yml

@@ -122,3 +122,4 @@ UsageData/InstrumentationSuperclass:
     - :DatabaseMetric
     - :GenericMetric
     - :RedisHLLMetric
+    - :RedisMetric

spec/lib/backup/manager_spec.rb

@@ -12,20 +12,13 @@ RSpec.describe Backup::Manager do
   before do
     allow(progress).to receive(:puts)
     allow(progress).to receive(:print)
-
-    @old_progress = $progress # rubocop:disable Style/GlobalVars
-    $progress = progress # rubocop:disable Style/GlobalVars
-  end
-
-  after do
-    $progress = @old_progress # rubocop:disable Style/GlobalVars
   end
 
   describe '#pack' do
-    let(:backup_contents) { ['backup_contents'] }
+    let(:expected_backup_contents) { %w(repositories db uploads.tar.gz builds.tar.gz artifacts.tar.gz pages.tar.gz lfs.tar.gz backup_information.yml) }
+    let(:tar_file) { '1546300800_2019_01_01_12.3_gitlab_backup.tar' }
     let(:tar_system_options) { { out: [tar_file, 'w', Gitlab.config.backup.archive_permissions] } }
-    let(:tar_cmdline) { ['tar', '-cf', '-', *backup_contents, tar_system_options] }
-
+    let(:tar_cmdline) { ['tar', '-cf', '-', *expected_backup_contents, tar_system_options] }
     let(:backup_information) do
       {
         backup_created_at: Time.zone.parse('2019-01-01'),
@@ -36,21 +29,21 @@ RSpec.describe Backup::Manager do
     before do
       allow(ActiveRecord::Base.connection).to receive(:reconnect!)
       allow(Kernel).to receive(:system).and_return(true)
+      allow(YAML).to receive(:load_file).and_return(backup_information)
+
+      ::Backup::Manager::FOLDERS_TO_BACKUP.each do |folder|
+        allow(Dir).to receive(:exist?).with(File.join(Gitlab.config.backup.path, folder)).and_return(true)
+      end
 
-      allow(subject).to receive(:backup_contents).and_return(backup_contents)
       allow(subject).to receive(:backup_information).and_return(backup_information)
       allow(subject).to receive(:upload)
     end
 
-    context 'when BACKUP is not set' do
-      let(:tar_file) { '1546300800_2019_01_01_12.3_gitlab_backup.tar' }
-
-      it 'uses the default tar file name' do
+    it 'executes tar' do
       subject.pack
 
       expect(Kernel).to have_received(:system).with(*tar_cmdline)
     end
-    end
 
     context 'when BACKUP is set' do
       let(:tar_file) { 'custom_gitlab_backup.tar' }
@@ -62,6 +55,37 @@ RSpec.describe Backup::Manager do
         expect(Kernel).to have_received(:system).with(*tar_cmdline)
       end
     end
+
+    context 'when skipped is set in backup_information.yml' do
+      let(:expected_backup_contents) { %w{db uploads.tar.gz builds.tar.gz artifacts.tar.gz pages.tar.gz lfs.tar.gz backup_information.yml} }
+      let(:backup_information) do
+        {
+          backup_created_at: Time.zone.parse('2019-01-01'),
+          gitlab_version: '12.3',
+          skipped: ['repositories']
+        }
+      end
+
+      it 'executes tar' do
+        subject.pack
+
+        expect(Kernel).to have_received(:system).with(*tar_cmdline)
+      end
+    end
+
+    context 'when a directory does not exist' do
+      let(:expected_backup_contents) { %w{db uploads.tar.gz builds.tar.gz artifacts.tar.gz pages.tar.gz lfs.tar.gz backup_information.yml} }
+
+      before do
+        expect(Dir).to receive(:exist?).with(File.join(Gitlab.config.backup.path, 'repositories')).and_return(false)
+      end
+
+      it 'executes tar' do
+        subject.pack
+
+        expect(Kernel).to have_received(:system).with(*tar_cmdline)
+      end
+    end
   end
 
   describe '#remove_old' do

spec/lib/gitlab/usage/metrics/instrumentations/redis_metric_spec.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Usage::Metrics::Instrumentations::RedisMetric, :clean_gitlab_redis_shared_state do
+  before do
+    4.times do
+      Gitlab::UsageDataCounters::SourceCodeCounter.count(:pushes)
+    end
+  end
+
+  let(:expected_value) { 4 }
+
+  it_behaves_like 'a correct instrumented metric value', { options: { event: 'pushes', counter_class: 'SourceCodeCounter' } }
+
+  it 'raises an exception if event option is not present' do
+    expect { described_class.new(counter_class: 'SourceCodeCounter') }.to raise_error(ArgumentError)
+  end
+
+  it 'raises an exception if counter_class option is not present' do
+    expect { described_class.new(event: 'pushes') }.to raise_error(ArgumentError)
+  end
+end

spec/lib/sidebars/projects/menus/packages_registries_menu_spec.rb

@@ -51,8 +51,8 @@ RSpec.describe Sidebars::Projects::Menus::PackagesRegistriesMenu do
       context 'when Container Registry is not visible' do
         let(:registry_enabled) { false }
 
-        it 'menu link points to Infrastructure Registry page' do
-          expect(subject.link).to eq described_class.new(context).renderable_items.find { |i| i.item_id == :infrastructure_registry }.link
+        it 'does not display menu link' do
+          expect(subject.render?).to eq false
         end
       end
     end
@@ -130,10 +130,26 @@ RSpec.describe Sidebars::Projects::Menus::PackagesRegistriesMenu do
 
           is_expected.not_to be_nil
         end
+
+        context 'when config package setting is disabled' do
+          it 'does not add the menu item to the list' do
+            stub_config(packages: { enabled: false })
+
+            is_expected.to be_nil
+          end
+        end
+
+        context 'when user cannot read packages' do
+          let(:user) { nil }
+
+          it 'does not add the menu item to the list' do
+            is_expected.to be_nil
+          end
+        end
       end
 
       context 'when feature flag :infrastructure_registry_page is disabled' do
-        it 'the menu item is not added to list of menu items' do
+        it 'does not add the menu item to the list' do
           stub_feature_flags(infrastructure_registry_page: false)
 
           is_expected.to be_nil