Add latest changes from gitlab-org/gitlab@master

app/assets/stylesheets/pages/issuable.scss

@@ -883,7 +883,7 @@
   .time-tracking-help-state {
     background: $white-light;
-    margin: 16px -20px 0;
+    margin: 16px -20px -20px;
     padding: 16px 20px;
     border-top: 1px solid $border-gray-light;
     border-bottom: 1px solid $border-gray-light;

app/helpers/emails_helper.rb

@@ -112,20 +112,20 @@ module EmailsHelper
     end
   end
-  # "You are receiving this email because #{reason}"
+  # "You are receiving this email because #{reason} on #{gitlab_host}."
   def notification_reason_text(reason)
-    string = case reason
-             when NotificationReason::OWN_ACTIVITY
-               'of your activity'
-             when NotificationReason::ASSIGNED
-               'you have been assigned an item'
-             when NotificationReason::MENTIONED
-               'you have been mentioned'
-             else
-               'of your account'
-             end
-
-    "#{string} on #{Gitlab.config.gitlab.host}"
+    gitlab_host = Gitlab.config.gitlab.host
+
+    case reason
+    when NotificationReason::OWN_ACTIVITY
+      _("You're receiving this email because of your activity on %{host}.") % { host: gitlab_host }
+    when NotificationReason::ASSIGNED
+      _("You're receiving this email because you have been assigned an item on %{host}.") % { host: gitlab_host }
+    when NotificationReason::MENTIONED
+      _("You're receiving this email because you have been mentioned on %{host}.") % { host: gitlab_host }
+    else
+      _("You're receiving this email because of your account on %{host}.") % { host: gitlab_host }
+    end
   end
   def create_list_id_string(project, list_id_max_length = 255)

app/models/active_session.rb

@@ -146,8 +146,9 @@ class ActiveSession
     # remove sessions if there are more than ALLOWED_NUMBER_OF_ACTIVE_SESSIONS.
     sessions = active_session_entries(session_ids, user.id, redis)
     sessions.sort_by! {|session| session.updated_at }.reverse!
-    sessions = sessions[ALLOWED_NUMBER_OF_ACTIVE_SESSIONS..-1].map { |session| session.session_id }
-    destroy_sessions(redis, user, sessions)
+    sessions = sessions.drop(ALLOWED_NUMBER_OF_ACTIVE_SESSIONS)
+    sessions = sessions.map { |session| session.session_id }
+    destroy_sessions(redis, user, sessions) if sessions.any?
   end
   def self.cleaned_up_lookup_entries(redis, user)

app/models/clusters/cluster.rb

@@ -271,6 +271,21 @@ module Clusters
       kubernetes_namespaces.delete_all(:delete_all)
     end
+    def clusterable
+      return unless cluster_type
+
+      case cluster_type
+      when 'project_type'
+        project
+      when 'group_type'
+        group
+      when 'instance_type'
+        instance
+      else
+        raise NotImplementedError
+      end
+    end
+
     private
     def unique_management_project_environment_scope

app/services/clusters/applications/ingress_modsecurity_usage_service.rb

+# frozen_string_literal: true
+
+# rubocop: disable CodeReuse/ActiveRecord
+module Clusters
+  module Applications
+    ##
+    # This service measures usage of the Modsecurity Web Application Firewall across the entire
+    # instance's deployed environments.
+    #
+    # The default configuration is`AUTO_DEVOPS_MODSECURITY_SEC_RULE_ENGINE=DetectionOnly` so we
+    # measure non-default values via definition of either ci_variables or ci_pipeline_variables.
+    # Since both these values are encrypted, we must decrypt and count them in memory.
+    #
+    # NOTE: this service is an approximation as it does not yet take into account `environment_scope` or `ci_group_variables`.
+    ##
+    class IngressModsecurityUsageService
+      ADO_MODSEC_KEY = "AUTO_DEVOPS_MODSECURITY_SEC_RULE_ENGINE"
+
+      def initialize(blocking_count: 0, disabled_count: 0)
+        @blocking_count = blocking_count
+        @disabled_count = disabled_count
+      end
+
+      def execute
+        conditions = -> { merge(::Environment.available).merge(::Deployment.success).where(key: ADO_MODSEC_KEY) }
+
+        ci_pipeline_var_enabled =
+          ::Ci::PipelineVariable
+            .joins(pipeline: { environments: :last_visible_deployment })
+            .merge(conditions)
+            .order('deployments.environment_id, deployments.id DESC')
+
+        ci_var_enabled =
+          ::Ci::Variable
+            .joins(project: { environments: :last_visible_deployment })
+            .merge(conditions)
+            .merge(
+              # Give priority to pipeline variables by excluding from dataset
+              ::Ci::Variable.joins(project: :environments).where.not(
+                environments: { id: ci_pipeline_var_enabled.select('DISTINCT ON (deployments.environment_id) deployments.environment_id') }
+              )
+            ).select('DISTINCT ON (deployments.environment_id) ci_variables.*')
+
+        sum_modsec_config_counts(
+          ci_pipeline_var_enabled.select('DISTINCT ON (deployments.environment_id) ci_pipeline_variables.*')
+        )
+        sum_modsec_config_counts(ci_var_enabled)
+
+        {
+          ingress_modsecurity_blocking: @blocking_count,
+          ingress_modsecurity_disabled: @disabled_count
+        }
+      end
+
+      private
+
+      # These are encrypted so we must decrypt and count in memory
+      def sum_modsec_config_counts(dataset)
+        dataset.each do |var|
+          case var.value
+          when "On" then @blocking_count += 1
+          when "Off" then @disabled_count += 1
+            # `else` could be default or any unsupported user input
+          end
+        end
+      end
+    end
+  end
+end

app/views/layouts/notify.html.haml

@@ -20,7 +20,7 @@
             #{link_to _("View it on GitLab"), @target_url}.
           %br
           -# Don't link the host in the line below, one link in the email is easier to quickly click than two.
-          = _("You're receiving this email because %{reason}.") % { reason: notification_reason_text(@reason) }
+          = notification_reason_text(@reason)
           If you'd like to receive fewer emails, you can
           - if @labels_url
             adjust your #{link_to 'label subscriptions', @labels_url}.

app/views/layouts/notify.text.erb

@@ -11,7 +11,7 @@
 <%    end -%>
 <%  end -%>
-<%= "You're receiving this email because #{notification_reason_text(@reason)}." %>
+<%= notification_reason_text(@reason) %>
 <%= render_if_exists 'layouts/mailer/additional_text' %>
 <%= text_footer_message -%>

changelogs/unreleased/32358-add-modsec-blocking-usage-stats-per-project.yml

+---
+title: Add modsecurity deployment counts to usage ping
+merge_request: 20196
+author:
+type: added

changelogs/unreleased/37480-validate-instance-level-cluster-environment-scope.yml

+---
+title: Validate unique environment scope for instance clusters
+merge_request: 20886
+author:
+type: fixed

changelogs/unreleased/remove_extra_spacing_below_sidebar_time_tracking_info.yml

+---
+title: Remove extra spacing below sidebar time tracking info
+merge_request: 20657
+author: Lee Tickett
+type: other

config/routes/group.rb

 # frozen_string_literal: true
+# rubocop: disable Cop/PutGroupRoutesUnderScope
 resources :groups, only: [:index, :new, :create] do
   post :preview_markdown
 end
+# rubocop: enable Cop/PutGroupRoutesUnderScope
 constraints(::Constraints::GroupUrlConstrainer.new) do
   scope(path: 'groups/*id',

db/migrate/20191115001123_add_index_to_mod_sec_ci_variables.rb

+# frozen_string_literal: true
+
+class AddIndexToModSecCiVariables < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :ci_variables, :project_id, where: "key = 'AUTO_DEVOPS_MODSECURITY_SEC_RULE_ENGINE'"
+  end
+
+  def down
+    remove_concurrent_index :ci_variables, :project_id
+  end
+end

db/migrate/20191115001843_add_index_to_mod_sec_ci_pipeline_variables.rb

+# frozen_string_literal: true
+
+class AddIndexToModSecCiPipelineVariables < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :ci_pipeline_variables, :pipeline_id, where: "key = 'AUTO_DEVOPS_MODSECURITY_SEC_RULE_ENGINE'"
+  end
+
+  def down
+    remove_concurrent_index :ci_pipeline_variables, :pipeline_id
+  end
+end

db/post_migrate/20191202031812_drop_operations_feature_flags_clients_token.rb

+# frozen_string_literal: true
+
+class DropOperationsFeatureFlagsClientsToken < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    # Ignored in 12.5 - https://gitlab.com/gitlab-org/gitlab/merge_requests/18923
+    remove_column :operations_feature_flags_clients, :token
+  end
+
+  def down
+    unless column_exists?(:operations_feature_flags_clients, :token)
+      add_column :operations_feature_flags_clients, :token, :string # rubocop:disable Migration/AddLimitToStringColumns
+    end
+
+    add_concurrent_index :operations_feature_flags_clients, [:project_id, :token], unique: true,
+      name: 'index_operations_feature_flags_clients_on_project_id_and_token'
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 2019_11_25_140458) do
+ActiveRecord::Schema.define(version: 2019_12_02_031812) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -821,6 +821,7 @@ ActiveRecord::Schema.define(version: 2019_11_25_140458) do
     t.integer "pipeline_id", null: false
     t.integer "variable_type", limit: 2, default: 1, null: false
     t.index ["pipeline_id", "key"], name: "index_ci_pipeline_variables_on_pipeline_id_and_key", unique: true
+    t.index ["pipeline_id"], name: "index_ci_pipeline_variables_on_pipeline_id", where: "((key)::text = 'AUTO_DEVOPS_MODSECURITY_SEC_RULE_ENGINE'::text)"
   end
   create_table "ci_pipelines", id: :serial, force: :cascade do |t|
@@ -979,6 +980,7 @@ ActiveRecord::Schema.define(version: 2019_11_25_140458) do
     t.boolean "masked", default: false, null: false
     t.integer "variable_type", limit: 2, default: 1, null: false
     t.index ["project_id", "key", "environment_scope"], name: "index_ci_variables_on_project_id_and_key_and_environment_scope", unique: true
+    t.index ["project_id"], name: "index_ci_variables_on_project_id", where: "((key)::text = 'AUTO_DEVOPS_MODSECURITY_SEC_RULE_ENGINE'::text)"
   end
   create_table "cluster_groups", id: :serial, force: :cascade do |t|
@@ -2774,9 +2776,7 @@ ActiveRecord::Schema.define(version: 2019_11_25_140458) do
   create_table "operations_feature_flags_clients", force: :cascade do |t|
     t.integer "project_id", null: false
-    t.string "token"
     t.string "token_encrypted"
-    t.index ["project_id", "token"], name: "index_operations_feature_flags_clients_on_project_id_and_token", unique: true
     t.index ["project_id", "token_encrypted"], name: "index_feature_flags_clients_on_project_id_and_token_encrypted", unique: true
   end

lib/gitlab/usage_data.rb

@@ -108,7 +108,8 @@ module Gitlab
             services_usage,
             approximate_counts,
             usage_counters,
-            user_preferences_usage
+            user_preferences_usage,
+            ingress_modsecurity_usage
           )
         }
       end
@@ -170,6 +171,10 @@ module Gitlab
         }
       end
+      def ingress_modsecurity_usage
+        ::Clusters::Applications::IngressModsecurityUsageService.new.execute
+      end
+
       # rubocop: disable CodeReuse/ActiveRecord
       def services_usage
         types = {

locale/gitlab.pot

@@ -20306,15 +20306,21 @@ msgstr ""
 msgid "You're only seeing %{startTag}other activity%{endTag} in the feed. To add a comment, switch to one of the following options."
 msgstr ""
-msgid "You're receiving this email because %{reason}."
-msgstr ""
-
 msgid "You're receiving this email because of your account on %{host}."
 msgstr ""
 msgid "You're receiving this email because of your account on %{host}. %{manage_notifications_link} · %{help_link}"
 msgstr ""
+msgid "You're receiving this email because of your activity on %{host}."
+msgstr ""
+
+msgid "You're receiving this email because you have been assigned an item on %{host}."
+msgstr ""
+
+msgid "You're receiving this email because you have been mentioned on %{host}."
+msgstr ""
+
 msgid "You've already enabled two-factor authentication using one time password authenticators. In order to register a different device, you must first disable two-factor authentication."
 msgstr ""

rubocop/cop/put_group_routes_under_scope.rb

+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    # Checks for a group routes outside '/-/' scope.
+    # For more information see: https://gitlab.com/gitlab-org/gitlab/issues/29572
+    class PutGroupRoutesUnderScope < RuboCop::Cop::Cop
+      MSG = 'Put new group routes under /-/ scope'
+
+      def_node_matcher :dash_scope?, <<~PATTERN
+        (:send nil? :scope (hash <(pair (sym :path)(str "groups/*group_id/-")) ...>))
+      PATTERN
+
+      def on_send(node)
+        return unless in_group_routes?(node)
+        return unless resource?(node)
+        return unless outside_scope?(node)
+
+        add_offense(node)
+      end
+
+      def outside_scope?(node)
+        node.each_ancestor(:block).none? do |parent|
+          dash_scope?(parent.to_a.first)
+        end
+      end
+
+      def in_group_routes?(node)
+        path = node.location.expression.source_buffer.name
+        dirname = File.dirname(path)
+        filename = File.basename(path)
+
+        dirname.end_with?('config/routes') &&
+          filename.end_with?('group.rb')
+      end
+
+      def resource?(node)
+        node.method_name == :resource ||
+          node.method_name == :resources
+      end
+    end
+  end
+end

rubocop/rubocop.rb

@@ -15,6 +15,7 @@ require_relative 'cop/avoid_route_redirect_leading_slash'
 require_relative 'cop/line_break_around_conditional_block'
 require_relative 'cop/prefer_class_methods_over_module'
 require_relative 'cop/put_project_routes_under_scope'
+require_relative 'cop/put_group_routes_under_scope'
 require_relative 'cop/migration/add_column'
 require_relative 'cop/migration/add_concurrent_foreign_key'
 require_relative 'cop/migration/add_concurrent_index'

spec/helpers/emails_helper_spec.rb

@@ -74,6 +74,28 @@ describe EmailsHelper do
     end
   end
+  describe 'notification_reason_text' do
+    subject { helper.notification_reason_text(reason_code) }
+
+    using RSpec::Parameterized::TableSyntax
+
+    where(:reason_code, :reason_text) do
+      NotificationReason::OWN_ACTIVITY | ' of your activity '
+      NotificationReason::ASSIGNED     | ' you have been assigned an item '
+      NotificationReason::MENTIONED    | ' you have been mentioned '
+      ""                               | ' of your account '
+      nil                              | ' of your account '
+    end
+
+    with_them do
+      it { is_expected.to start_with "You're receiving this email because" }
+
+      it { is_expected.to include reason_text }
+
+      it { is_expected.to end_with "on #{Gitlab.config.gitlab.host}." }
+    end
+  end
+
   describe 'sanitize_name' do
     context 'when name contains a valid URL string' do
       it 'returns name with `.` replaced with `_` to prevent mail clients from auto-linking URLs' do