Add latest changes from gitlab-org/gitlab@master

.gitlab/merge_request_templates/Documentation.md

@@ -29,7 +29,7 @@ When applicable:
 - [ ] Link docs to and from the higher-level index page, plus other related docs where helpful.
 - [ ] Add [GitLab's version history note(s)](https://docs.gitlab.com/ee/development/documentation/styleguide.html#text-for-documentation-requiring-version-text).
 - [ ] Add the [product tier badge](https://docs.gitlab.com/ee/development/documentation/styleguide.html#product-badges).
-- [ ] Add/update the [feature flag section](https://docs.gitlab.com/ee/development/documentation/styleguide.html#feature-flags).
+- [ ] Add/update the [feature flag section](https://docs.gitlab.com/ee/development/documentation/feature_flags.html).
 - [ ] If you're changing document headings, search `doc/*`, `app/views/*`, and `ee/app/views/*` for old headings replacing with the new ones to [avoid broken anchors](https://docs.gitlab.com/ee/development/documentation/styleguide.html#anchor-links).
 
 ## Review checklist

app/assets/javascripts/blob/components/blob_header.vue

@@ -66,7 +66,7 @@ export default {
       </template>
     </blob-filepath>
 
-    <div class="file-actions d-none d-sm-block">
+    <div class="file-actions d-none d-sm-flex">
       <viewer-switcher v-if="showViewerSwitcher" v-model="viewer" />
 
       <slot name="actions"></slot>

app/assets/javascripts/blob/components/blob_header_viewer_switcher.vue

@@ -45,7 +45,7 @@ export default {
 };
 </script>
 <template>
-  <gl-button-group class="js-blob-viewer-switcher ml-2">
+  <gl-button-group class="js-blob-viewer-switcher mx-2">
     <gl-deprecated-button
       v-gl-tooltip.hover
       :aria-label="$options.SIMPLE_BLOB_VIEWER_TITLE"

app/models/ci/pipeline_schedule.rb

@@ -6,6 +6,10 @@ module Ci
     include Importable
     include StripAttribute
     include Schedulable
+    include Limitable
+
+    self.limit_name = 'ci_pipeline_schedules'
+    self.limit_scope = :project
 
     belongs_to :project
     belongs_to :owner, class_name: 'User'

app/models/concerns/limitable.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module Limitable
+  extend ActiveSupport::Concern
+
+  included do
+    class_attribute :limit_scope
+    class_attribute :limit_name
+    self.limit_name = self.name.demodulize.tableize
+
+    validate :validate_plan_limit_not_exceeded, on: :create
+  end
+
+  private
+
+  def validate_plan_limit_not_exceeded
+    scope_relation = self.public_send(limit_scope) # rubocop:disable GitlabSecurity/PublicSend
+    return unless scope_relation
+
+    relation = self.class.where(limit_scope => scope_relation)
+
+    if scope_relation.actual_limits.exceeded?(limit_name, relation)
+      errors.add(:base, _("Maximum number of %{name} (%{count}) exceeded") %
+        { name: limit_name.humanize(capitalize: false), count: scope_relation.actual_limits.public_send(limit_name) }) # rubocop:disable GitlabSecurity/PublicSend
+    end
+  end
+end

app/models/hooks/project_hook.rb

@@ -3,6 +3,9 @@
 class ProjectHook < WebHook
   include TriggerableHooks
   include Presentable
+  include Limitable
+
+  self.limit_scope = :project
 
   triggerable_hooks [
     :push_hooks,

app/models/namespace.rb

@@ -346,6 +346,21 @@ class Namespace < ApplicationRecord
       .try(name)
   end
 
+  def actual_plan
+    Plan.default
+  end
+
+  def actual_limits
+    # We default to PlanLimits.new otherwise a lot of specs would fail
+    # On production each plan should already have associated limits record
+    # https://gitlab.com/gitlab-org/gitlab/issues/36037
+    actual_plan.limits || PlanLimits.new
+  end
+
+  def actual_plan_name
+    actual_plan.name
+  end
+
   private
 
   def all_projects_with_pages

app/models/plan.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+class Plan < ApplicationRecord
+  DEFAULT = 'default'.freeze
+
+  has_one :limits, class_name: 'PlanLimits'
+
+  ALL_PLANS = [DEFAULT].freeze
+  DEFAULT_PLANS = [DEFAULT].freeze
+  private_constant :ALL_PLANS, :DEFAULT_PLANS
+
+  # This always returns an object
+  def self.default
+    Gitlab::SafeRequestStore.fetch(:plan_default) do
+      # find_by allows us to find object (cheaply) against replica DB
+      # safe_find_or_create_by does stick to primary DB
+      find_by(name: DEFAULT) || safe_find_or_create_by(name: DEFAULT)
+    end
+  end
+
+  def self.all_plans
+    ALL_PLANS
+  end
+
+  def self.default_plans
+    DEFAULT_PLANS
+  end
+
+  def default?
+    self.class.default_plans.include?(name)
+  end
+
+  def paid?
+    false
+  end
+end
+
+Plan.prepend_if_ee('EE::Plan')

app/models/plan_limits.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+class PlanLimits < ApplicationRecord
+  belongs_to :plan
+
+  def exceeded?(limit_name, object)
+    return false unless enabled?(limit_name)
+
+    if object.is_a?(Integer)
+      object >= read_attribute(limit_name)
+    else
+      # object.count >= limit value is slower than checking
+      # if a record exists at the limit value - 1 position.
+      object.limit(1).offset(read_attribute(limit_name) - 1).exists?
+    end
+  end
+
+  private
+
+  def enabled?(limit_name)
+    read_attribute(limit_name) > 0
+  end
+end

app/models/project.rb

@@ -357,6 +357,7 @@ class Project < ApplicationRecord
   delegate :external_dashboard_url, to: :metrics_setting, allow_nil: true, prefix: true
   delegate :default_git_depth, :default_git_depth=, to: :ci_cd_settings, prefix: :ci
   delegate :forward_deployment_enabled, :forward_deployment_enabled=, :forward_deployment_enabled?, to: :ci_cd_settings
+  delegate :actual_limits, :actual_plan_name, to: :namespace, allow_nil: true
 
   # Validations
   validates :creator, presence: true, on: :create

app/services/projects/propagate_service_template.rb

@@ -4,8 +4,10 @@ module Projects
   class PropagateServiceTemplate
     BATCH_SIZE = 100
 
-    def self.propagate(*args)
-      new(*args).propagate
+    delegate :data_fields_present?, to: :template
+
+    def self.propagate(template)
+      new(template).propagate
     end
 
     def initialize(template)
@@ -13,15 +15,15 @@ module Projects
     end
 
     def propagate
-      return unless @template.active?
-
-      Rails.logger.info("Propagating services for template #{@template.id}") # rubocop:disable Gitlab/RailsLogger
+      return unless template.active?
 
       propagate_projects_with_template
     end
 
     private
 
+    attr_reader :template
+
     def propagate_projects_with_template
       loop do
         batch = Project.uncached { project_ids_batch }
@@ -38,7 +40,14 @@ module Projects
       end
 
       Project.transaction do
-        bulk_insert_services(service_hash.keys << 'project_id', service_list)
+        results = bulk_insert(Service, service_hash.keys << 'project_id', service_list)
+
+        if data_fields_present?
+          data_list = results.map { |row| data_hash.values << row['id'] }
+
+          bulk_insert(template.data_fields.class, data_hash.keys << 'service_id', data_list)
+        end
+
         run_callbacks(batch)
       end
     end
@@ -52,36 +61,27 @@ module Projects
             SELECT true
             FROM services
             WHERE services.project_id = projects.id
-            AND services.type = '#{@template.type}'
+            AND services.type = #{ActiveRecord::Base.connection.quote(template.type)}
           )
           AND projects.pending_delete = false
           AND projects.archived = false
           LIMIT #{BATCH_SIZE}
-      SQL
+        SQL
       )
     end
 
-    def bulk_insert_services(columns, values_array)
-      ActiveRecord::Base.connection.execute(
-        <<-SQL.strip_heredoc
-          INSERT INTO services (#{columns.join(', ')})
-          VALUES #{values_array.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')}
-      SQL
-      )
+    def bulk_insert(klass, columns, values_array)
+      items_to_insert = values_array.map { |array| Hash[columns.zip(array)] }
+
+      klass.insert_all(items_to_insert, returning: [:id])
     end
 
     def service_hash
-      @service_hash ||=
-        begin
-          template_hash = @template.as_json(methods: :type).except('id', 'template', 'project_id')
+      @service_hash ||= template.as_json(methods: :type, except: %w[id template project_id])
+    end
 
-          template_hash.each_with_object({}) do |(key, value), service_hash|
-            value = value.is_a?(Hash) ? value.to_json : value
-
-            service_hash[ActiveRecord::Base.connection.quote_column_name(key)] =
-              ActiveRecord::Base.connection.quote(value)
-          end
-        end
+    def data_hash
+      @data_hash ||= template.data_fields.as_json(only: template.data_fields.class.column_names).except('id', 'service_id')
     end
 
     # rubocop: disable CodeReuse/ActiveRecord
@@ -97,11 +97,11 @@ module Projects
     # rubocop: enable CodeReuse/ActiveRecord
 
     def active_external_issue_tracker?
-      @template.issue_tracker? && !@template.default
+      template.issue_tracker? && !template.default
     end
 
     def active_external_wiki?
-      @template.type == 'ExternalWikiService'
+      template.type == 'ExternalWikiService'
     end
   end
 end

app/services/projects/update_repository_storage_service.rb

@@ -18,7 +18,7 @@ module Projects
 
       mark_old_paths_for_archive
 
-      project.update(repository_storage: new_repository_storage_key, repository_read_only: false)
+      project.update!(repository_storage: new_repository_storage_key, repository_read_only: false)
       project.leave_pool_repository
       project.track_project_repository
 
@@ -26,8 +26,8 @@ module Projects
 
       success
 
-    rescue Error, ArgumentError, Gitlab::Git::BaseError => e
-      project.update(repository_read_only: false)
+    rescue StandardError => e
+      project.update!(repository_read_only: false)
 
       Gitlab::ErrorTracking.track_exception(e, project_path: project.full_path)
 

changelogs/unreleased/212549-padding-in-snippet-blob-header.yml

@@ -0,0 +1,5 @@
+---
+title: Fix minor spacing issue at Snippet blob viewer
+merge_request: 29625
+author: Karthick Venkatesan
+type: fixed

changelogs/unreleased/214834-propagate-service-tempate.yml

@@ -0,0 +1,5 @@
+---
+title: Propagation of service templates also covers services with separate data tables.
+merge_request: 29805
+author:
+type: fixed

changelogs/unreleased/38096-add-index-on-resource-weight-events-created-at-pd.yml

@@ -0,0 +1,5 @@
+---
+title: Add index to issue_id and created_at of resource_weight_events
+merge_request: 28930
+author:
+type: other

changelogs/unreleased/shard_move_capture_all_errors.yml

@@ -0,0 +1,5 @@
+---
+title: Capture all errors when updating repository storage
+merge_request: 30119
+author:
+type: fixed

danger/roulette/Dangerfile

@@ -13,8 +13,9 @@ MARKDOWN
 CATEGORY_TABLE_HEADER = <<MARKDOWN
 
 To spread load more evenly across eligible reviewers, Danger has randomly picked
-a candidate for each review slot. Feel free to override this selection if you
-think someone else would be better-suited, or the chosen person is unavailable.
+a candidate for each review slot. Feel free to
+[override these selections](https://about.gitlab.com/handbook/engineering/projects/#gitlab)
+if you think someone else would be better-suited, or the chosen person is unavailable.
 
 To read more on how to use the reviewer roulette, please take a look at the
 [Engineering workflow](https://about.gitlab.com/handbook/engineering/workflow/#basics)

db/migrate/20200406141452_add_index_to_issue_id_and_created_at_on_resource_weight_events.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class AddIndexToIssueIdAndCreatedAtOnResourceWeightEvents < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  INDEX_NAME = 'index_resource_weight_events_on_issue_id_and_created_at'
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :resource_weight_events, [:issue_id, :created_at], name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :resource_weight_events, INDEX_NAME
+  end
+end

db/migrate/20200415153154_add_unique_index_on_plan_name.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class AddUniqueIndexOnPlanName < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    remove_concurrent_index :plans, :name
+    add_concurrent_index :plans, :name, unique: true
+  end
+
+  def down
+    remove_concurrent_index :plans, :name, unique: true
+    add_concurrent_index :plans, :name
+  end
+end

db/structure.sql

@@ -9872,7 +9872,7 @@ CREATE INDEX index_personal_access_tokens_on_user_id ON public.personal_access_t
 
 CREATE UNIQUE INDEX index_plan_limits_on_plan_id ON public.plan_limits USING btree (plan_id);
 
-CREATE INDEX index_plans_on_name ON public.plans USING btree (name);
+CREATE UNIQUE INDEX index_plans_on_name ON public.plans USING btree (name);
 
 CREATE UNIQUE INDEX index_pool_repositories_on_disk_path ON public.pool_repositories USING btree (disk_path);
 
@@ -10160,6 +10160,8 @@ CREATE INDEX index_resource_milestone_events_on_milestone_id ON public.resource_
 
 CREATE INDEX index_resource_milestone_events_on_user_id ON public.resource_milestone_events USING btree (user_id);
 
+CREATE INDEX index_resource_weight_events_on_issue_id_and_created_at ON public.resource_weight_events USING btree (issue_id, created_at);
+
 CREATE INDEX index_resource_weight_events_on_issue_id_and_weight ON public.resource_weight_events USING btree (issue_id, weight);
 
 CREATE INDEX index_resource_weight_events_on_user_id ON public.resource_weight_events USING btree (user_id);
@@ -13232,6 +13234,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200406102111
 20200406102120
 20200406135648
+20200406141452
 20200406192059
 20200406193427
 20200407094005
@@ -13262,6 +13265,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200413072059
 20200413230056
 20200414144547
+20200415153154
 20200415160722
 20200415161021
 20200415161206

doc/api/graphql/reference/gitlab_schema.graphql

@@ -340,6 +340,11 @@ type BoardList {
   """
   label: Label
 
+  """
+  The current limit metric for the list
+  """
+  limitMetric: ListLimitMetric
+
   """
   Type of the list
   """
@@ -4619,6 +4624,15 @@ type LabelEdge {
   node: Label
 }
 
+"""
+List limit metric setting
+"""
+enum ListLimitMetric {
+  all_metrics
+  issue_count
+  issue_weights
+}
+
 """
 Autogenerated input type of MarkAsSpamSnippet
 """

doc/api/graphql/reference/gitlab_schema.json

@@ -1036,6 +1036,20 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "limitMetric",
+              "description": "The current limit metric for the list",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "ENUM",
+                "name": "ListLimitMetric",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "listType",
               "description": "Type of the list",
@@ -13171,6 +13185,35 @@
           "enumValues": null,
           "possibleTypes": null
         },
+        {
+          "kind": "ENUM",
+          "name": "ListLimitMetric",
+          "description": "List limit metric setting",
+          "fields": null,
+          "inputFields": null,
+          "interfaces": null,
+          "enumValues": [
+            {
+              "name": "all_metrics",
+              "description": null,
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "issue_count",
+              "description": null,
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "issue_weights",
+              "description": null,
+              "isDeprecated": false,
+              "deprecationReason": null
+            }
+          ],
+          "possibleTypes": null
+        },
         {
           "kind": "INPUT_OBJECT",
           "name": "MarkAsSpamSnippetInput",

doc/api/graphql/reference/index.md

@@ -89,6 +89,7 @@ Represents a list for an issue board
 | `collapsed` | Boolean | Indicates if list is collapsed for this user |
 | `id` | ID! | ID (global ID) of the list |
 | `label` | Label | Label of the list |
+| `limitMetric` | ListLimitMetric | The current limit metric for the list |
 | `listType` | String! | Type of the list |
 | `maxIssueCount` | Int | Maximum number of issues in the list |
 | `maxIssueWeight` | Int | Maximum weight of issues in the list |

doc/development/application_limits.md

@@ -90,7 +90,7 @@ project.actual_limits.exceeded?(:project_hooks, 10)
 
 #### `Limitable` concern
 
-The [`Limitable` concern](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/models/concerns/limitable.rb)
+The [`Limitable` concern](https://gitlab.com/gitlab-org/gitlab/blob/master/app/models/concerns/limitable.rb)
 can be used to validate that a model does not exceed the limits. It ensures
 that the count of the records for the current model does not exceed the defined
 limit.

doc/install/aws/index.md

@@ -533,6 +533,22 @@ Since we're adding our SSL certificate at the load balancer, we do not need GitL
    sudo gitlab-ctl reconfigure
    ```
 
+#### Fast lookup of authorized SSH keys
+
+The public SSH keys for users allowed to access GitLab are stored in `/var/opt/gitlab/.ssh/authorized_keys`. Typically we'd use shared storage so that all the instances are able to access this file when a user performs a Git action over SSH. Since we do not have shared storage in our setup, we'll update our configuration to authorize SSH users via indexed lookup in the GitLab database.
+
+Follow the instructions at [Setting up fast lookup via GitLab Shell](../../administration/operations/fast_ssh_key_lookup.md#setting-up-fast-lookup-via-gitlab-shell) to switch from using the `authorized_keys` file to the database.
+
+If you do not configure fast lookup, Git actions over SSH will result in the following error:
+
+```shell
+Permission denied (publickey).
+fatal: Could not read from remote repository.
+
+Please make sure you have the correct access rights
+and the repository exists.
+```
+
 #### Configure host keys
 
 Ordinarily we would manually copy the contents (primary and public keys) of `/etc/ssh/` on the primary application server to `/etc/ssh` on all secondary servers. This prevents false man-in-the-middle-attack alerts when accessing servers in your High Availability cluster behind a load balancer.

qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb

@@ -17,7 +17,7 @@ module QA
         end
       end
 
-      context 'when using attachments in comments', :object_storage, quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/issues/205408', type: :bug } do
+      context 'when using attachments in comments', :object_storage do
         let(:gif_file_name) { 'banana_sample.gif' }
         let(:file_to_attach) do
           File.absolute_path(File.join('spec', 'fixtures', gif_file_name))

spec/factories/plan_limits.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :plan_limits do
+    plan
+
+    trait :default_plan do
+      plan factory: :default_plan
+    end
+  end
+end

spec/factories/plans.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :plan do
+    Plan.all_plans.each do |plan|
+      factory :"#{plan}_plan" do
+        name { plan }
+        title { name.titleize }
+        initialize_with { Plan.find_or_create_by(name: plan) }
+      end
+    end
+  end
+end

spec/frontend/blob/components/__snapshots__/blob_header_spec.js.snap

@@ -9,7 +9,7 @@ exports[`Blob Header Default Actions rendering matches the snapshot 1`] = `
   />
    
   <div
-    class="file-actions d-none d-sm-block"
+    class="file-actions d-none d-sm-flex"
   >
     <viewer-switcher-stub
       value="simple"

spec/frontend/ci_variable_list/ci_variable_list/ajax_variable_list_spec.js

@@ -35,8 +35,8 @@ describe('AjaxFormVariableList', () => {
       maskableRegex: container.dataset.maskableRegex,
     });
 
-    spyOn(ajaxVariableList, 'updateRowsWithPersistedVariables').and.callThrough();
-    spyOn(ajaxVariableList.variableList, 'toggleEnableRow').and.callThrough();
+    jest.spyOn(ajaxVariableList, 'updateRowsWithPersistedVariables');
+    jest.spyOn(ajaxVariableList.variableList, 'toggleEnableRow');
   });
 
   afterEach(() => {
@@ -44,7 +44,7 @@ describe('AjaxFormVariableList', () => {
   });
 
   describe('onSaveClicked', () => {
-    it('shows loading spinner while waiting for the request', done => {
+    it('shows loading spinner while waiting for the request', () => {
       const loadingIcon = saveButton.querySelector('.js-ci-variables-save-loading-icon');
 
       mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(() => {
@@ -55,63 +55,47 @@ describe('AjaxFormVariableList', () => {
 
       expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(true);
 
-      ajaxVariableList
-        .onSaveClicked()
-        .then(() => {
-          expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(true);
-        })
-        .then(done)
-        .catch(done.fail);
+      return ajaxVariableList.onSaveClicked().then(() => {
+        expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(true);
+      });
     });
 
-    it('calls `updateRowsWithPersistedVariables` with the persisted variables', done => {
+    it('calls `updateRowsWithPersistedVariables` with the persisted variables', () => {
       const variablesResponse = [{ id: 1, key: 'foo', value: 'bar' }];
       mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200, {
         variables: variablesResponse,
       });
 
-      ajaxVariableList
-        .onSaveClicked()
-        .then(() => {
-          expect(ajaxVariableList.updateRowsWithPersistedVariables).toHaveBeenCalledWith(
-            variablesResponse,
-          );
-        })
-        .then(done)
-        .catch(done.fail);
+      return ajaxVariableList.onSaveClicked().then(() => {
+        expect(ajaxVariableList.updateRowsWithPersistedVariables).toHaveBeenCalledWith(
+          variablesResponse,
+        );
+      });
     });
 
-    it('hides any previous error box', done => {
+    it('hides any previous error box', () => {
       mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200);
 
       expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
 
-      ajaxVariableList
-        .onSaveClicked()
-        .then(() => {
-          expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
-        })
-        .then(done)
-        .catch(done.fail);
+      return ajaxVariableList.onSaveClicked().then(() => {
+        expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
+      });
     });
 
-    it('disables remove buttons while waiting for the request', done => {
+    it('disables remove buttons while waiting for the request', () => {
       mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(() => {
         expect(ajaxVariableList.variableList.toggleEnableRow).toHaveBeenCalledWith(false);
 
         return [200, {}];
       });
 
-      ajaxVariableList
-        .onSaveClicked()
-        .then(() => {
-          expect(ajaxVariableList.variableList.toggleEnableRow).toHaveBeenCalledWith(true);
-        })
-        .then(done)
-        .catch(done.fail);
+      return ajaxVariableList.onSaveClicked().then(() => {
+        expect(ajaxVariableList.variableList.toggleEnableRow).toHaveBeenCalledWith(true);
+      });
     });
 
-    it('hides secret values', done => {
+    it('hides secret values', () => {
       mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200, {});
 
       const row = container.querySelector('.js-row');
@@ -124,46 +108,34 @@ describe('AjaxFormVariableList', () => {
       expect(valuePlaceholder.classList.contains(HIDE_CLASS)).toBe(true);
       expect(valueInput.classList.contains(HIDE_CLASS)).toBe(false);
 
-      ajaxVariableList
-        .onSaveClicked()
-        .then(() => {
-          expect(valuePlaceholder.classList.contains(HIDE_CLASS)).toBe(false);
-          expect(valueInput.classList.contains(HIDE_CLASS)).toBe(true);
-        })
-        .then(done)
-        .catch(done.fail);
+      return ajaxVariableList.onSaveClicked().then(() => {
+        expect(valuePlaceholder.classList.contains(HIDE_CLASS)).toBe(false);
+        expect(valueInput.classList.contains(HIDE_CLASS)).toBe(true);
+      });
     });
 
-    it('shows error box with validation errors', done => {
+    it('shows error box with validation errors', () => {
       const validationError = 'some validation error';
       mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(400, [validationError]);
 
       expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
 
-      ajaxVariableList
-        .onSaveClicked()
-        .then(() => {
-          expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(false);
-          expect(errorBox.textContent.trim().replace(/\n+\s+/m, ' ')).toEqual(
-            `Validation failed ${validationError}`,
-          );
-        })
-        .then(done)
-        .catch(done.fail);
+      return ajaxVariableList.onSaveClicked().then(() => {
+        expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(false);
+        expect(errorBox.textContent.trim().replace(/\n+\s+/m, ' ')).toEqual(
+          `Validation failed ${validationError}`,
+        );
+      });
     });
 
-    it('shows flash message when request fails', done => {
+    it('shows flash message when request fails', () => {
       mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(500);
 
       expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
 
-      ajaxVariableList
-        .onSaveClicked()
-        .then(() => {
-          expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
-        })
-        .then(done)
-        .catch(done.fail);
+      return ajaxVariableList.onSaveClicked().then(() => {
+        expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
+      });
     });
   });
 

spec/frontend/ci_variable_list/ci_variable_list/ci_variable_list_spec.js

@@ -1,5 +1,5 @@
 import $ from 'jquery';
-import getSetTimeoutPromise from 'spec/helpers/set_timeout_promise_helper';
+import waitForPromises from 'helpers/wait_for_promises';
 import VariableList from '~/ci_variable_list/ci_variable_list';
 
 const HIDE_CLASS = 'hide';
@@ -127,86 +127,74 @@ describe('VariableList', () => {
       variableList.init();
     });
 
-    it('should not add another row when editing the last rows protected checkbox', done => {
+    it('should not add another row when editing the last rows protected checkbox', () => {
       const $row = $wrapper.find('.js-row:last-child');
       $row.find('.ci-variable-protected-item .js-project-feature-toggle').click();
 
-      getSetTimeoutPromise()
-        .then(() => {
-          expect($wrapper.find('.js-row').length).toBe(1);
-        })
-        .then(done)
-        .catch(done.fail);
+      return waitForPromises().then(() => {
+        expect($wrapper.find('.js-row').length).toBe(1);
+      });
     });
 
-    it('should not add another row when editing the last rows masked checkbox', done => {
+    it('should not add another row when editing the last rows masked checkbox', () => {
+      jest.spyOn(variableList, 'checkIfRowTouched');
       const $row = $wrapper.find('.js-row:last-child');
       $row.find('.ci-variable-masked-item .js-project-feature-toggle').click();
 
-      getSetTimeoutPromise()
-        .then(() => {
-          expect($wrapper.find('.js-row').length).toBe(1);
-        })
-        .then(done)
-        .catch(done.fail);
+      return waitForPromises().then(() => {
+        // This validates that we are checking after the event listener has run
+        expect(variableList.checkIfRowTouched).toHaveBeenCalled();
+        expect($wrapper.find('.js-row').length).toBe(1);
+      });
     });
 
     describe('validateMaskability', () => {
       let $row;
 
       const maskingErrorElement = '.js-row:last-child .masking-validation-error';
+      const clickToggle = () =>
+        $row.find('.ci-variable-masked-item .js-project-feature-toggle').click();
 
       beforeEach(() => {
         $row = $wrapper.find('.js-row:last-child');
-        $row.find('.ci-variable-masked-item .js-project-feature-toggle').click();
       });
 
       it('has a regex provided via a data attribute', () => {
+        clickToggle();
+
         expect($wrapper.attr('data-maskable-regex')).toBe('^[a-zA-Z0-9_+=/@:.-]{8,}$');
       });
 
-      it('allows values that are 8 characters long', done => {
+      it('allows values that are 8 characters long', () => {
         $row.find('.js-ci-variable-input-value').val('looooong');
 
-        getSetTimeoutPromise()
-          .then(() => {
-            expect($wrapper.find(maskingErrorElement)).toHaveClass('hide');
-          })
-          .then(done)
-          .catch(done.fail);
+        clickToggle();
+
+        expect($wrapper.find(maskingErrorElement)).toHaveClass('hide');
       });
 
-      it('rejects values that are shorter than 8 characters', done => {
+      it('rejects values that are shorter than 8 characters', () => {
         $row.find('.js-ci-variable-input-value').val('short');
 
-        getSetTimeoutPromise()
-          .then(() => {
-            expect($wrapper.find(maskingErrorElement)).toBeVisible();
-          })
-          .then(done)
-          .catch(done.fail);
+        clickToggle();
+
+        expect($wrapper.find(maskingErrorElement)).toBeVisible();
       });
 
-      it('allows values with base 64 characters', done => {
+      it('allows values with base 64 characters', () => {
         $row.find('.js-ci-variable-input-value').val('abcABC123_+=/-');
 
-        getSetTimeoutPromise()
-          .then(() => {
-            expect($wrapper.find(maskingErrorElement)).toHaveClass('hide');
-          })
-          .then(done)
-          .catch(done.fail);
+        clickToggle();
+
+        expect($wrapper.find(maskingErrorElement)).toHaveClass('hide');
       });
 
-      it('rejects values with other special characters', done => {
+      it('rejects values with other special characters', () => {
         $row.find('.js-ci-variable-input-value').val('1234567$');
 
-        getSetTimeoutPromise()
-          .then(() => {
-            expect($wrapper.find(maskingErrorElement)).toBeVisible();
-          })
-          .then(done)
-          .catch(done.fail);
+        clickToggle();
+
+        expect($wrapper.find(maskingErrorElement)).toBeVisible();
       });
     });
   });

spec/models/ci/pipeline_schedule_spec.rb

@@ -17,6 +17,10 @@ describe Ci::PipelineSchedule do
   it { is_expected.to respond_to(:description) }
   it { is_expected.to respond_to(:next_run_at) }
 
+  it_behaves_like 'includes Limitable concern' do
+    subject { build(:ci_pipeline_schedule) }
+  end
+
   describe 'validations' do
     it 'does not allow invalid cron patters' do
       pipeline_schedule = build(:ci_pipeline_schedule, cron: '0 0 0 * *')

spec/models/hooks/project_hook_spec.rb

@@ -11,6 +11,10 @@ describe ProjectHook do
     it { is_expected.to validate_presence_of(:project) }
   end
 
+  it_behaves_like 'includes Limitable concern' do
+    subject { build(:project_hook, project: create(:project)) }
+  end
+
   describe '.push_hooks' do
     it 'returns hooks for push events only' do
       hook = create(:project_hook, push_events: true)

spec/models/plan_spec.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Plan do
+  describe '#default?' do
+    subject { plan.default? }
+
+    Plan.default_plans.each do |plan|
+      context "when '#{plan}'" do
+        let(:plan) { build("#{plan}_plan".to_sym) }
+
+        it { is_expected.to be_truthy }
+      end
+    end
+  end
+end

spec/requests/api/graphql/boards/board_lists_query_spec.rb

@@ -27,23 +27,23 @@ describe 'get board lists' do
       board_parent_type,
       { 'fullPath' => board_parent.full_path },
       <<~BOARDS
-      boards(first: 1) {
-        edges {
-          node {
-            #{field_with_params('lists', list_params)} {
-              pageInfo {
-                startCursor
-                endCursor
-              }
-              edges {
-                node {
-                  #{all_graphql_fields_for('board_lists'.classify)}
+        boards(first: 1) {
+          edges {
+            node {
+              #{field_with_params('lists', list_params)} {
+                pageInfo {
+                  startCursor
+                  endCursor
+                }
+                edges {
+                  node {
+                    #{all_graphql_fields_for('board_lists'.classify)}
+                  }
                 }
               }
             }
           }
         }
-      }
     BOARDS
     )
   end

spec/services/projects/propagate_service_template_spec.rb

@@ -8,16 +8,19 @@ describe Projects::PropagateServiceTemplate do
       PushoverService.create(
         template: true,
         active: true,
+        push_events: false,
         properties: {
           device: 'MyDevice',
           sound: 'mic',
           priority: 4,
           user_key: 'asdf',
           api_key: '123456789'
-        })
+        }
+      )
     end
 
     let!(:project) { create(:project) }
+    let(:excluded_attributes) { %w[id project_id template created_at updated_at title description] }
 
     it 'creates services for projects' do
       expect(project.pushover_service).to be_nil
@@ -35,7 +38,7 @@ describe Projects::PropagateServiceTemplate do
         properties: {
           bamboo_url: 'http://gitlab.com',
           username: 'mic',
-          password: "password",
+          password: 'password',
           build_key: 'build'
         }
       )
@@ -54,7 +57,7 @@ describe Projects::PropagateServiceTemplate do
         properties: {
           bamboo_url: 'http://gitlab.com',
           username: 'mic',
-          password: "password",
+          password: 'password',
           build_key: 'build'
         }
       )
@@ -70,6 +73,33 @@ describe Projects::PropagateServiceTemplate do
       described_class.propagate(service_template)
 
       expect(project.pushover_service.properties).to eq(service_template.properties)
+
+      expect(project.pushover_service.attributes.except(*excluded_attributes))
+        .to eq(service_template.attributes.except(*excluded_attributes))
+    end
+
+    context 'service with data fields' do
+      let(:service_template) do
+        JiraService.create!(
+          template: true,
+          active: true,
+          push_events: false,
+          url: 'http://jira.instance.com',
+          username: 'user',
+          password: 'secret'
+        )
+      end
+
+      it 'creates the service containing the template attributes' do
+        described_class.propagate(service_template)
+
+        expect(project.jira_service.attributes.except(*excluded_attributes))
+          .to eq(service_template.attributes.except(*excluded_attributes))
+
+        excluded_attributes = %w[id service_id created_at updated_at]
+        expect(project.jira_service.data_fields.attributes.except(*excluded_attributes))
+          .to eq(service_template.data_fields.attributes.except(*excluded_attributes))
+      end
     end
 
     describe 'bulk update', :use_sql_query_cache do

spec/services/projects/update_repository_storage_service_spec.rb

@@ -12,6 +12,7 @@ describe Projects::UpdateRepositoryStorageService do
 
     before do
       allow(Time).to receive(:now).and_return(time)
+      allow(Gitlab.config.repositories.storages).to receive(:keys).and_return(%w[default test_second_storage])
     end
 
     context 'without wiki and design repository' do

spec/support/shared_examples/models/concerns/limitable_shared_examples.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'includes Limitable concern' do
+  describe 'validations' do
+    let(:plan_limits) { create(:plan_limits, :default_plan) }
+
+    it { is_expected.to be_a(Limitable) }
+
+    context 'without plan limits configured' do
+      it 'can create new models' do
+        expect { subject.save }.to change { described_class.count }
+      end
+    end
+
+    context 'with plan limits configured' do
+      before do
+        plan_limits.update(subject.class.limit_name => 1)
+      end
+
+      it 'can create new models' do
+        expect { subject.save }.to change { described_class.count }
+      end
+
+      context 'with an existing model' do
+        before do
+          subject.dup.save
+        end
+
+        it 'cannot create new models exceding the plan limits' do
+          expect { subject.save }.not_to change { described_class.count }
+          expect(subject.errors[:base]).to contain_exactly("Maximum number of #{subject.class.limit_name.humanize(capitalize: false)} (1) exceeded")
+        end
+      end
+    end
+  end
+end

spec/support/shared_examples/workers/pages_domain_cron_worker_shared_examples.rb

@@ -3,12 +3,14 @@
 RSpec.shared_examples 'a pages cronjob scheduling jobs with context' do |scheduled_worker_class|
   let(:worker) { described_class.new }
 
-  it 'does not cause extra queries for multiple domains' do
-    control = ActiveRecord::QueryRecorder.new { worker.perform }
+  context 'with RequestStore enabled', :request_store do
+    it 'does not cause extra queries for multiple domains' do
+      control = ActiveRecord::QueryRecorder.new { worker.perform }
 
-    extra_domain
+      extra_domain
 
-    expect { worker.perform }.not_to exceed_query_limit(control)
+      expect { worker.perform }.not_to exceed_query_limit(control)
+    end
   end
 
   it 'schedules the renewal with a context' do