Add latest changes from gitlab-org/gitlab@13-4-stable-ee

9 files changed, 396 insertions, 19 deletions

diff --git a/scripts/docs_screenshots.rb b/scripts/docs_screenshots.rb
new file mode 100755
index 00000000000..e02d67de748
--- /dev/null
+++ b/scripts/docs_screenshots.rb
@@ -0,0 +1,60 @@
+#!/usr/bin/env ruby
+
+# frozen_string_literal: true
+
+require 'png_quantizator'
+require 'open3'
+require 'parallel'
+require_relative '../tooling/lib/tooling/images.rb'
+
+generator = ARGV[0]
+milestone = ARGV[1]
+
+unless generator
+  warn('Error: missing generator, please supply one')
+  abort
+end
+
+unless milestone
+  warn('Error: missing milestone, please supply one')
+  abort
+end
+
+def rename_image(file, milestone)
+  path = File.dirname(file)
+  basename = File.basename(file)
+  final_name = File.join(path, "#{basename}_v#{milestone}.png")
+  FileUtils.mv(file, final_name)
+end
+
+system('spring', 'rspec', generator)
+
+files = []
+
+Open3.popen3("git diff --name-only -- '*.png'") do |stdin, stdout, stderr, thread|
+  files.concat stdout.read.chomp.split("\n")
+end
+
+Open3.popen3("git status --porcelain -- '*.png'") do |stdin, stdout, stderr, thread|
+  files.concat stdout.read.chomp.split("?? ")
+end
+
+files.reject!(&:empty?)
+
+if files.empty?
+  puts "No file generated, did you select the right screenshot generator?"
+else
+  puts "Compressing newly generated screenshots"
+
+  Parallel.each(files) do |file|
+    file_path = File.join(Dir.pwd, file.to_s.strip)
+    was_uncompressed, savings = Tooling::Image.compress_image(file_path)
+    rename_image(file_path, milestone)
+
+    if was_uncompressed
+      puts "#{file} was reduced by #{savings} bytes."
+    else
+      puts "Skipping already compressed file: #{file}."
+    end
+  end
+end
diff --git a/scripts/frontend/check_page_bundle_mixins_css_for_sideeffects.js b/scripts/frontend/check_page_bundle_mixins_css_for_sideeffects.js
new file mode 100755
index 00000000000..a2bb9f56d84
--- /dev/null
+++ b/scripts/frontend/check_page_bundle_mixins_css_for_sideeffects.js
@@ -0,0 +1,36 @@
+#!/usr/bin/env node
+
+if (process.env.RAILS_ENV !== 'production') {
+  console.log(
+    `RAILS_ENV is not set to 'production': ${process.env.RAILS_ENV} - Not executing check`,
+  );
+  process.exit(0);
+}
+
+const path = require('path');
+const fs = require('fs');
+const glob = require('glob');
+const pjs = require('postcss');
+
+const paths = glob.sync('public/assets/page_bundles/_mixins_and_variables_and_functions*.css', {
+  cwd: path.join(__dirname, '..', '..'),
+});
+
+if (!paths[0]) {
+  console.log('Could not find mixins test file');
+  process.exit(1);
+}
+
+console.log(`Checking ${paths[0]} for side effects`);
+
+const file = fs.readFileSync(paths[0], 'utf-8');
+
+const parsed = pjs.parse(file);
+
+if (parsed.nodes.every(node => ['comment', 'atrule'].includes(node.type))) {
+  console.log('The file does not introduce any side effects, we are all good.');
+  process.exit(0);
+}
+
+console.log(`At least one unwanted style was introduced.`);
+process.exit(1);
diff --git a/scripts/lint-doc.sh b/scripts/lint-doc.sh
index 72e6334d0fc..4886323c836 100755
--- a/scripts/lint-doc.sh
+++ b/scripts/lint-doc.sh
@@ -55,7 +55,18 @@ then
   ((ERRORCODE++))
 fi
 
-MD_DOC_PATH=${MD_DOC_PATH:-doc}
+# Run Vale and Markdownlint only on changed files. Only works on merged results
+# pipelines, so first checks if a merged results CI variable is present. If not present,
+# runs test on all files.
+if [ -z "${CI_MERGE_REQUEST_TARGET_BRANCH_SHA}" ]
+then
+  MD_DOC_PATH=${MD_DOC_PATH:-doc}
+  echo "Merge request pipeline (detached) detected. Testing all files."
+else
+  MERGE_BASE=$(git merge-base ${CI_MERGE_REQUEST_TARGET_BRANCH_SHA} ${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA})
+  MD_DOC_PATH=$(git diff --name-only "${MERGE_BASE}..${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA}" 'doc/*.md')
+  echo -e "Merged results pipeline detected. Testing only the following files:\n${MD_DOC_PATH}"
+ fi
 
 function run_locally_or_in_docker() {
   local cmd=$1
diff --git a/scripts/review_apps/review-apps.sh b/scripts/review_apps/review-apps.sh
index 59189c94cde..862c3b4bb62 100755
--- a/scripts/review_apps/review-apps.sh
+++ b/scripts/review_apps/review-apps.sh
@@ -147,13 +147,12 @@ function disable_sign_ups() {
   run_task "${ruby_cmd}"
 
   # Disable sign-ups
-  retry 'curl --silent --show-error --request PUT --header "PRIVATE-TOKEN: ${REVIEW_APPS_ROOT_TOKEN}" "${CI_ENVIRONMENT_URL}/api/v4/application/settings?signup_enabled=false"'
+  local signup_enabled=$(retry 'curl --silent --show-error --request PUT --header "PRIVATE-TOKEN: ${REVIEW_APPS_ROOT_TOKEN}" "${CI_ENVIRONMENT_URL}/api/v4/application/settings?signup_enabled=false" | jq ".signup_enabled"')
 
-  local signup_enabled=$(retry 'curl --silent --show-error --request GET --header "PRIVATE-TOKEN: ${REVIEW_APPS_ROOT_TOKEN}" "${CI_ENVIRONMENT_URL}/api/v4/application/settings" | jq ".signup_enabled"')
   if [[ "${signup_enabled}" == "false" ]]; then
     echoinfo "Sign-ups have been disabled successfully."
   else
-    echoerr "Sign-ups should be disabled but are still enabled!"
+    echoerr "Sign-ups are still enabled!"
     false
   fi
 }
diff --git a/scripts/review_apps/seed-dast-test-data.sh b/scripts/review_apps/seed-dast-test-data.sh
new file mode 100644
index 00000000000..cba5859a1db
--- /dev/null
+++ b/scripts/review_apps/seed-dast-test-data.sh
@@ -0,0 +1,69 @@
+[[ "$TRACE" ]] && set -x
+
+function create_user() {
+    local user="${1}"
+
+    # API details at https://docs.gitlab.com/ee/api/users.html#user-creation
+    #
+    # We set "can_create_group=false" because we don't want the DAST user to create groups.
+    # Otherwise, the DAST user likely creates a group and enables 2FA for all group members,
+    # which leads to the DAST scan getting "stuck" on the 2FA set up page.
+    # Once https://gitlab.com/gitlab-org/gitlab/-/issues/231447 is resolved, we can use 
+    # DAST_AUTH_EXCLUDE_URLS instead to prevent DAST from enabling 2FA.
+    curl --silent --show-error --header "PRIVATE-TOKEN: ${REVIEW_APPS_ROOT_TOKEN}" \
+        --data "email=${user}@example.com" \
+        --data "name=${user}" \
+        --data "username=${user}" \
+        --data "password=${REVIEW_APPS_ROOT_PASSWORD}" \
+        --data "skip_confirmation=true" \
+        --data "can_create_group=false" \
+        "${CI_ENVIRONMENT_URL}/api/v4/users" > /tmp/user.json
+
+    [[ "$TRACE" ]] && cat /tmp/user.json >&2
+
+    jq .id /tmp/user.json
+}
+
+function create_project_for_user() {
+    local userid="${1}"
+
+    # API details at https://docs.gitlab.com/ee/api/projects.html#create-project-for-user
+    curl --silent --show-error --header "PRIVATE-TOKEN: ${REVIEW_APPS_ROOT_TOKEN}" \
+        --data "user_id=${userid}" \
+        --data "name=awesome-test-project-${userid}" \
+        --data "visibility=private" \
+        "${CI_ENVIRONMENT_URL}/api/v4/projects/user/${userid}" > /tmp/project.json
+
+    [[ "$TRACE" ]] && cat /tmp/project.json >&2
+}
+
+function trigger_proj_user_creation(){
+    local u1=$(create_user "user1")
+    create_project_for_user $u1
+    local u2=$(create_user "user2")
+    create_project_for_user $u2
+    local u3=$(create_user "user3")
+    create_project_for_user $u3
+    local u4=$(create_user "user4")
+    create_project_for_user $u4
+    local u5=$(create_user "user5")
+    create_project_for_user $u5
+    local u6=$(create_user "user6")
+    create_project_for_user $u6
+    local u7=$(create_user "user7")
+    create_project_for_user $u7
+    local u8=$(create_user "user8")
+    create_project_for_user $u8
+    local u9=$(create_user "user9")
+    create_project_for_user $u9
+    local u10=$(create_user "user10")
+    create_project_for_user $u10
+    local u11=$(create_user "user11")
+    create_project_for_user $u11
+    local u12=$(create_user "user12")
+    create_project_for_user $u12
+    local u13=$(create_user "user13")
+    create_project_for_user $u13
+    local u14=$(create_user "user14")
+    create_project_for_user $u14
+}
diff --git a/scripts/rspec_helpers.sh b/scripts/rspec_helpers.sh
index 1ecf9a566d7..3812a8b8ef7 100644
--- a/scripts/rspec_helpers.sh
+++ b/scripts/rspec_helpers.sh
@@ -111,7 +111,28 @@ function rspec_paralellized_job() {
   date
 }
 
-function rspec_matched_tests() {
+function rspec_fail_fast() {
+  local test_file_count_threshold=${RSPEC_FAIL_FAST_TEST_FILE_COUNT_THRESHOLD:-10}
+  local matching_tests_file=${1}
+  local rspec_opts=${2}
+  local test_files="$(cat "${matching_tests_file}")"
+  local test_file_count=$(wc -w "${matching_tests_file}" | awk {'print $1'})
+
+  if [[ "${test_file_count}" -gt "${test_file_count_threshold}" ]]; then
+    echo "This job is intentionally skipped because there are more than ${test_file_count_threshold} test files matched,"
+    echo "which would take too long to run in this job."
+    echo "All the tests would be run in other rspec jobs."
+    exit 0
+  fi
+
+  if [[ -n $test_files ]]; then
+    rspec_simple_job "${rspec_opts} ${test_files}"
+  else
+    echo "No rspec fail-fast tests to run"
+  fi
+}
+
+function rspec_matched_foss_tests() {
   local test_file_count_threshold=20
   local matching_tests_file=${1}
   local rspec_opts=${2}
@@ -131,6 +152,6 @@ function rspec_matched_tests() {
   if [[ -n $test_files ]]; then
     rspec_simple_job "${rspec_opts} ${test_files}"
   else
-    echo "No test files to run"
+    echo "No impacted FOSS rspec tests to run"
   fi
 }
diff --git a/scripts/trigger-build b/scripts/trigger-build
index 7fc550d86ee..ab6dcc63e11 100755
--- a/scripts/trigger-build
+++ b/scripts/trigger-build
@@ -138,8 +138,12 @@ module Trigger
     def extra_variables
       # Use CI_MERGE_REQUEST_SOURCE_BRANCH_SHA for omnibus checkouts due to pipeline for merged results
       # and fallback to CI_COMMIT_SHA for the `detached` pipelines.
+      # We also set IMAGE_TAG so the GitLab and QA docker images are tagged with
+      # that SHA.
+      source_sha = Trigger.non_empty_variable_value('CI_MERGE_REQUEST_SOURCE_BRANCH_SHA') || ENV['CI_COMMIT_SHA']
       {
-        'GITLAB_VERSION' => Trigger.non_empty_variable_value('CI_MERGE_REQUEST_SOURCE_BRANCH_SHA') || ENV['CI_COMMIT_SHA'],
+        'GITLAB_VERSION' => source_sha,
+        'IMAGE_TAG' => source_sha,
         'ALTERNATIVE_SOURCES' => 'true',
         'SECURITY_SOURCES' => Trigger.security? ? 'true' : 'false',
         'ee' => Trigger.ee? ? 'true' : 'false',
@@ -317,8 +321,6 @@ module Trigger
     INTERVAL = 60 # seconds
     MAX_DURATION = 3600 * 3 # 3 hours
 
-    attr_reader :project, :id
-
     def self.unscoped_class_name
       name.split('::').last
     end
@@ -330,34 +332,30 @@ module Trigger
     def initialize(project, id)
       @project = project
       @id = id
-      @start = Time.now.to_i
+      @start_time = Time.now.to_i
     end
 
     def wait!
-      loop do
-        raise "#{self.class.unscoped_class_name} timed out after waiting for #{duration} minutes!" if timeout?
-
+      (MAX_DURATION / INTERVAL).times do
         case status
         when :created, :pending, :running
           print "."
           sleep INTERVAL
         when :success
           puts "#{self.class.unscoped_class_name} succeeded in #{duration} minutes!"
-          break
+          return
         else
           raise "#{self.class.unscoped_class_name} did not succeed!"
         end
 
         STDOUT.flush
       end
-    end
 
-    def timeout?
-      Time.now.to_i > (@start + MAX_DURATION)
+      raise "#{self.class.unscoped_class_name} timed out after waiting for #{duration} minutes!"
     end
 
     def duration
-      (Time.now.to_i - @start) / 60
+      (Time.now.to_i - start_time) / 60
     end
 
     def status
@@ -368,6 +366,10 @@ module Trigger
       # timeout anyway.
       :running
     end
+
+    private
+
+    attr_reader :project, :id, :start_time
   end
 
   Job = Class.new(Pipeline)
diff --git a/scripts/utils.sh b/scripts/utils.sh
index f81e5c8982a..9d188fc7b77 100644
--- a/scripts/utils.sh
+++ b/scripts/utils.sh
@@ -35,6 +35,10 @@ function install_gitlab_gem() {
   gem install gitlab --no-document --version 4.13.0
 }
 
+function install_tff_gem() {
+  gem install test_file_finder --version 0.1.0
+}
+
 function run_timed_command() {
   local cmd="${1}"
   local start=$(date +%s)
@@ -106,7 +110,7 @@ function get_job_id() {
     let "page++"
   done
 
-  if [[ "${job_id}" == "" ]]; then
+  if [[ "${job_id}" == "null" ]]; then # jq prints "null" for non-existent attribute
     echoerr "The '${job_name}' job ID couldn't be retrieved!"
   else
     echoinfo "The '${job_name}' job ID is ${job_id}"
@@ -133,3 +137,15 @@ function play_job() {
   job_url=$(curl --silent --show-error --request POST --header "PRIVATE-TOKEN: ${api_token}" "${url}" | jq ".web_url")
   echoinfo "Manual job '${job_name}' started at: ${job_url}"
 }
+
+function fail_pipeline_early() {
+  local dont_interrupt_me_job_id
+  dont_interrupt_me_job_id=$(get_job_id 'dont-interrupt-me' 'scope=success')
+
+  if [[ -n "${dont_interrupt_me_job_id}" ]]; then
+    echoinfo "This pipeline cannot be interrupted due to \`dont-interrupt-me\` job ${dont_interrupt_me_job_id}"
+  else
+    echoinfo "Failing pipeline early for fast feedback due to test failures in rspec fail-fast."
+    curl --request POST --header "PRIVATE-TOKEN: ${GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN}" "https://${CI_SERVER_HOST}/api/v4/projects/${CI_PROJECT_ID}/pipelines/${CI_PIPELINE_ID}/cancel"
+  fi
+}
diff --git a/scripts/verify-tff-mapping b/scripts/verify-tff-mapping
new file mode 100755
index 00000000000..9541d2468b1
--- /dev/null
+++ b/scripts/verify-tff-mapping
@@ -0,0 +1,163 @@
+#!/usr/bin/env ruby
+
+require 'set'
+
+# These tests run a sanity check on the mapping file `tests.yml`
+# used with the `test_file_finder` gem (`tff`) to identify matching test files.
+# The verification depend on the presence of actual test files,
+# so they would fail if one of the test files mentioned here is deleted.
+# To minimize the chance of this test failing due to unrelated changes,
+# the test files are chosen to be critical files that are unlikely to be deleted in a typical Merge Request
+tests = [
+  {
+    explanation: 'EE code should map to respective spec',
+    source: 'ee/app/controllers/admin/licenses_controller.rb',
+    expected: ['ee/spec/controllers/admin/licenses_controller_spec.rb']
+  },
+
+  {
+    explanation: 'FOSS code should map to respective spec',
+    source: 'app/finders/admin/projects_finder.rb',
+    expected: ['spec/finders/admin/projects_finder_spec.rb']
+  },
+
+  {
+    explanation: 'EE extension should map to its EE extension spec and its FOSS class spec',
+    source: 'ee/app/finders/ee/projects_finder.rb',
+    expected: ['ee/spec/finders/ee/projects_finder_spec.rb', 'spec/finders/projects_finder_spec.rb']
+  },
+
+  {
+    explanation: 'Some EE extensions also map to its EE class spec, but this is not recommended: https://docs.gitlab.com/ee/development/ee_features.html#testing-ee-features-based-on-ce-features',
+    source: 'ee/app/models/ee/user.rb',
+    expected: ['ee/spec/models/user_spec.rb', 'spec/models/user_spec.rb']
+  },
+
+  {
+    explanation: 'FOSS lib should map to respective spec',
+    source: 'lib/gitaly/server.rb',
+    expected: ['spec/lib/gitaly/server_spec.rb']
+  },
+
+  {
+    explanation: 'EE lib should map to respective spec',
+    source: 'ee/lib/flipper_session.rb',
+    expected: ['ee/spec/lib/flipper_session_spec.rb']
+  },
+
+  {
+    explanation: 'Tooling should map to respective spec',
+    source: 'tooling/lib/tooling/test_file_finder.rb',
+    expected: ['spec/tooling/lib/tooling/test_file_finder_spec.rb']
+  },
+
+  {
+    explanation: 'FOSS spec code should map to itself',
+    source: 'spec/models/issue_spec.rb',
+    expected: ['spec/models/issue_spec.rb']
+  },
+
+  {
+    explanation: 'EE spec code should map to itself',
+    source: 'ee/spec/models/user_spec.rb',
+    expected: ['ee/spec/models/user_spec.rb']
+  },
+
+  {
+    explanation: 'EE extension spec should map to itself and the FOSS class spec',
+    source: 'ee/spec/services/ee/notification_service_spec.rb',
+    expected: ['ee/spec/services/ee/notification_service_spec.rb', 'spec/services/notification_service_spec.rb']
+  },
+
+  {
+    explanation: 'FOSS factory should map to factories spec',
+    source: 'spec/factories/users.rb',
+    expected: ['spec/factories_spec.rb']
+  },
+
+  {
+    explanation: 'EE factory should map to factories spec',
+    source: 'ee/spec/factories/users.rb',
+    expected: ['spec/factories_spec.rb']
+  },
+
+  {
+    explanation: 'Initializers should map to respective spec',
+    source: 'config/initializers/action_mailer_hooks.rb',
+    expected: ['spec/initializers/action_mailer_hooks_spec.rb']
+  },
+
+  {
+    explanation: 'FOSS views should map to respective spec',
+    source: 'app/views/admin/users/_user.html.haml',
+    expected: ['spec/views/admin/users/_user.html.haml_spec.rb']
+  },
+
+  {
+    explanation: 'EE views should map to respective spec',
+    source: 'ee/app/views/admin/licenses/show.html.haml',
+    expected: ['ee/spec/views/admin/licenses/show.html.haml_spec.rb']
+  },
+
+  {
+    explanation: 'DB structure should map to schema spec',
+    source: 'db/structure.sql',
+    expected: ['spec/db/schema_spec.rb']
+  },
+
+  {
+    explanation: 'Migration should map to its non-timestamped spec',
+    source: 'db/migrate/20191023152913_add_default_and_free_plans.rb',
+    expected: ['spec/migrations/add_default_and_free_plans_spec.rb']
+  },
+
+  {
+    explanation: 'Migration should map to its timestamped spec',
+    source: 'db/post_migrate/20190924152703_migrate_issue_trackers_data.rb',
+    expected: ['spec/migrations/20190924152703_migrate_issue_trackers_data_spec.rb']
+  }
+]
+
+class MappingTest
+  def initialize(explanation:, source:, expected:, mapping: 'tests.yml')
+    @explanation = explanation
+    @source = source
+    @mapping = mapping
+    @expected_set = Set.new(expected)
+    @actual_set = Set.new(actual)
+  end
+
+  def passed?
+    expected_set.eql?(actual_set)
+  end
+
+  def failed?
+    !passed?
+  end
+
+  def failure_message
+    "#{explanation}: #{source}: Expected #{expected_set.to_a}, got #{actual_set.to_a}."
+  end
+
+  private
+
+  attr_reader :explanation, :source, :expected_set, :actual_set, :mapping
+
+  def actual
+    `tff -f #{mapping} #{source}`.split(' ')
+  end
+end
+
+results = tests.map { |test| MappingTest.new(test) }
+
+failed_tests = results.select(&:failed?)
+if failed_tests.any?
+  puts <<~MESSAGE
+    tff mapping verification failed:
+    #{failed_tests.map(&:failure_message).join("\n")}
+  MESSAGE
+
+  exit 1
+end
+
+puts 'tff mapping verification passed.'