Add latest changes from gitlab-org/gitlab@12-10-stable-ee

app/controllers/projects/pages_domains_controller.rb

@@ -7,6 +7,8 @@ class Projects::PagesDomainsController < Projects::ApplicationController
   before_action :authorize_update_pages!
   before_action :domain, except: [:new, :create]
 
+  helper_method :domain_presenter
+
   def show
   end
 
@@ -27,7 +29,7 @@ class Projects::PagesDomainsController < Projects::ApplicationController
   end
 
   def retry_auto_ssl
-    PagesDomains::RetryAcmeOrderService.new(@domain.pages_domain).execute
+    PagesDomains::RetryAcmeOrderService.new(@domain).execute
 
     redirect_to project_pages_domain_path(@project, @domain)
   end
@@ -88,6 +90,10 @@ class Projects::PagesDomainsController < Projects::ApplicationController
   end
 
   def domain
-    @domain ||= @project.pages_domains.find_by_domain!(params[:id].to_s).present(current_user: current_user)
+    @domain ||= @project.pages_domains.find_by_domain!(params[:id].to_s)
+  end
+
+  def domain_presenter
+    @domain_presenter ||= domain.present(current_user: current_user)
   end
 end

app/uploaders/file_uploader.rb

@@ -15,7 +15,7 @@ class FileUploader < GitlabUploader
   prepend ObjectStorage::Extension::RecordsUploads
 
   MARKDOWN_PATTERN = %r{\!?\[.*?\]\(/uploads/(?<secret>[0-9a-f]{32})/(?<file>.*?)\)}.freeze
-  DYNAMIC_PATH_PATTERN = %r{.*/(?<secret>\h{10,32})/(?<identifier>.*)}.freeze
+  DYNAMIC_PATH_PATTERN = %r{.*(?<secret>\b(\h{10}|\h{32}))\/(?<identifier>.*)}.freeze
   VALID_SECRET_PATTERN = %r{\A\h{10,32}\z}.freeze
 
   InvalidSecret = Class.new(StandardError)

app/views/projects/pages_domains/_certificate.html.haml

 - auto_ssl_available = ::Gitlab::LetsEncrypt.enabled?
-- auto_ssl_enabled = @domain.auto_ssl_enabled?
+- auto_ssl_enabled = domain_presenter.auto_ssl_enabled?
 - auto_ssl_available_and_enabled = auto_ssl_available && auto_ssl_enabled
-- has_user_defined_certificate = @domain.certificate && @domain.certificate_user_provided?
+- has_user_defined_certificate = domain_presenter.certificate && domain_presenter.certificate_user_provided?
 
 - if auto_ssl_available
   .form-group.border-section
@@ -36,9 +36,9 @@
             = _('Certificate')
           .d-flex.justify-content-between.align-items-center.p-3
             %span
-              = @domain.pages_domain.subject || _('missing')
+              = domain_presenter.pages_domain.subject || _('missing')
             = link_to _('Remove'),
-              clean_certificate_project_pages_domain_path(@project, @domain),
+              clean_certificate_project_pages_domain_path(@project, domain_presenter),
               data: { confirm: _('Are you sure?') },
               class: 'btn btn-remove btn-sm',
               method: :delete

app/views/projects/pages_domains/_dns.html.haml

 - verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled?
-- dns_record = "#{@domain.domain} CNAME #{@domain.project.pages_subdomain}.#{Settings.pages.host}."
+- dns_record = "#{domain_presenter.domain} CNAME #{domain_presenter.project.pages_subdomain}.#{Settings.pages.host}."
 
 .form-group.border-section
   .row
@@ -13,17 +13,17 @@
       %p.form-text.text-muted
         = _("To access this domain create a new DNS record")
 - if verification_enabled
-  - verification_record = "#{@domain.verification_domain} TXT #{@domain.keyed_verification_code}"
+  - verification_record = "#{domain_presenter.verification_domain} TXT #{domain_presenter.keyed_verification_code}"
   .form-group.border-section
     .row
       .col-sm-2
         = _("Verification status")
       .col-sm-10
         .status-badge
-          - text, status = @domain.unverified? ? [_('Unverified'), 'badge-danger'] : [_('Verified'), 'badge-success']
+          - text, status = domain_presenter.unverified? ? [_('Unverified'), 'badge-danger'] : [_('Verified'), 'badge-success']
           .badge{ class: status }
             = text
-          = link_to sprite_icon("redo"), verify_project_pages_domain_path(@project, @domain), method: :post, class: "btn has-tooltip", title: _("Retry verification")
+          = link_to sprite_icon("redo"), verify_project_pages_domain_path(@project, domain_presenter), method: :post, class: "btn has-tooltip", title: _("Retry verification")
         .input-group
           = text_field_tag :domain_verification, verification_record, class: "monospace js-select-on-focus form-control", readonly: true
           .input-group-append

app/views/projects/pages_domains/_form.html.haml

-- if @domain.errors.any?
+- if domain_presenter.errors.any?
   .alert.alert-danger
-    - @domain.errors.full_messages.each do |msg|
+    - domain_presenter.errors.full_messages.each do |msg|
       = msg
 
 .form-group.border-section
   .row
-    - if @domain.persisted?
+    - if domain_presenter.persisted?
       .col-sm-2
         = _("Domain")
       .col-sm-10
-        = external_link(@domain.url, @domain.url)
+        = external_link(domain_presenter.url, domain_presenter.url)
     - else
       .col-sm-2
         = f.label :domain, _("Domain")
@@ -17,7 +17,7 @@
         .input-group
           = f.text_field :domain, required: true, autocomplete: "off", class: "form-control"
 
-- if @domain.persisted?
+- if domain_presenter.persisted?
   = render 'dns'
 
 - if Gitlab.config.pages.external_https

app/views/projects/pages_domains/_lets_encrypt_callout.html.haml

-- if @domain.enabled?
-  - if @domain.auto_ssl_enabled
-    - if @domain.show_auto_ssl_failed_warning?
+- if domain_presenter.enabled?
+  - if domain_presenter.auto_ssl_enabled
+    - if domain_presenter.show_auto_ssl_failed_warning?
       .form-group.border-section.js-shown-if-auto-ssl{ class: ("d-none" unless auto_ssl_available_and_enabled) }
         .row
           .col-sm-10.offset-sm-2
@@ -9,8 +9,8 @@
                 = icon('warning', class: 'mr-2')
                 = _("Something went wrong while obtaining the Let's Encrypt certificate.")
               .row.mx-0.mt-3
-                = link_to s_('GitLabPagesDomains|Retry'), retry_auto_ssl_project_pages_domain_path(@project, @domain), class: "btn btn-sm btn-grouped btn-warning", method: :post
-    - elsif !@domain.certificate_gitlab_provided?
+                = link_to s_('GitLabPagesDomains|Retry'), retry_auto_ssl_project_pages_domain_path(@project, domain_presenter), class: "btn btn-sm btn-grouped btn-warning", method: :post
+    - elsif !domain_presenter.certificate_gitlab_provided?
       .form-group.border-section.js-shown-if-auto-ssl{ class: ("d-none" unless auto_ssl_available_and_enabled) }
         .row
           .col-sm-10.offset-sm-2

app/views/projects/pages_domains/new.html.haml

@@ -4,7 +4,7 @@
   = _("New Pages Domain")
 = render 'projects/pages_domains/helper_text'
 %div
-  = form_for [@project.namespace.becomes(Namespace), @project, @domain], html: { class: 'fieldset-form' } do |f|
+  = form_for [@project.namespace.becomes(Namespace), @project, domain_presenter], html: { class: 'fieldset-form' } do |f|
     = render 'form', { f: f }
     .form-actions
       = f.submit _('Create New Domain'), class: "btn btn-success"

app/views/projects/pages_domains/show.html.haml

 - add_to_breadcrumbs _("Pages"), project_pages_path(@project)
-- breadcrumb_title @domain.domain
-- page_title @domain.domain
+- breadcrumb_title domain_presenter.domain
+- page_title domain_presenter.domain
 
 - verification_enabled = Gitlab::CurrentSettings.pages_domain_verification_enabled?
 
-- if verification_enabled && @domain.unverified?
+- if verification_enabled && domain_presenter.unverified?
   = content_for :flash_message do
     .alert.alert-warning
       .container-fluid.container-limited
@@ -14,7 +14,7 @@
   = _('Pages Domain')
 = render 'projects/pages_domains/helper_text'
 %div
-  = form_for [@project.namespace.becomes(Namespace), @project, @domain], html: { class: 'fieldset-form' } do |f|
+  = form_for [@project.namespace.becomes(Namespace), @project, domain_presenter], html: { class: 'fieldset-form' } do |f|
     = render 'form', { f: f }
     .form-actions.d-flex.justify-content-between
       = f.submit _('Save Changes'), class: "btn btn-success"

changelogs/unreleased/216728-actionview-template-error-undefined-method-pages_domain-for-pagesd.yml

+---
+title: Fix 500 on creating an invalid domains and verification
+merge_request: 31190
+author:
+type: fixed

changelogs/unreleased/216851-graphql-externallypaginatedarrayconnection-can-return-incorrect-nu.yml

+---
+title: Fix incorrect number of errors returned when querying sentry errors
+merge_request: 31252
+author:
+type: fixed

changelogs/unreleased/216970-add-instance-to-service-if-missing.yml

+---
+title: Add instance column to services table if it's missing
+merge_request: 31631
+author:
+type: fixed

changelogs/unreleased/217602-file-uploads-on-local-storage-with-nil-secret-in-the-db-are-broken.yml

+---
+title: Fix incorrect regex used in FileUploader#extract_dynamic_path
+merge_request: 32271
+author:
+type: fixed

changelogs/unreleased/cat-duplicate-ci-pipelines-index-215790.yml

+---
+title: Fix duplicate index removal on ci_pipelines.project_id
+merge_request: 31043
+author:
+type: fixed

db/migrate/20191229140154_drop_index_ci_pipelines_on_project_id.rb

@@ -8,10 +8,13 @@ class DropIndexCiPipelinesOnProjectId < ActiveRecord::Migration[5.2]
   disable_ddl_transaction!
 
   def up
-    remove_concurrent_index :ci_pipelines, :project_id
+    remove_concurrent_index_by_name :ci_pipelines, 'index_ci_pipelines_on_project_id'
+
+    # extra (duplicate) index that already existed on some installs
+    remove_concurrent_index_by_name :ci_pipelines, 'ci_pipelines_project_id_idx'
   end
 
   def down
-    add_concurrent_index :ci_pipelines, :project_id
+    add_concurrent_index :ci_pipelines, :project_id, name: 'index_ci_pipelines_on_project_id'
   end
 end

db/post_migrate/20200511162057_add_missing_instance_to_servicess.rb

+# frozen_string_literal: true
+
+class AddMissingInstanceToServicess < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  # This is a corrective migration to keep the instance column.
+  # Upgrade from 12.7 to 12.9 removes the instance column as it was first added
+  # in the normal migration and then removed in the post migration.
+  #
+  # 12.8 removed the instance column in a post deployment migration https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24885
+  # 12.9 added the instance column in a normal migration https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25714
+  #
+  # rubocop:disable Migration/AddColumnWithDefault
+  # rubocop:disable Migration/UpdateLargeTable
+  def up
+    unless column_exists?(:services, :instance)
+      add_column_with_default(:services, :instance, :boolean, default: false)
+    end
+  end
+  # rubocop:enable Migration/AddColumnWithDefault
+  # rubocop:enable Migration/UpdateLargeTable
+
+  def down
+    # Does not apply
+  end
+end

db/post_migrate/20200511162115_add_missing_index_to_service_unique_instance_per_type.rb

+# frozen_string_literal: true
+
+class AddMissingIndexToServiceUniqueInstancePerType < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  # This is a corrective migration to keep the index on instance column.
+  # Upgrade from 12.7 to 12.9 removes the instance column as it was first added
+  # in the normal migration and then removed in the post migration.
+  #
+  # 12.8 removed the instance column in a post deployment migration https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24885
+  # 12.9 added the instance column in a normal migration https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25714
+  def up
+    unless index_exists_by_name?(:services, 'index_services_on_type_and_instance')
+      add_concurrent_index(:services, [:type, :instance], unique: true, where: 'instance IS TRUE')
+    end
+  end
+
+  def down
+    # Does not apply
+  end
+end

db/structure.sql

@@ -13214,5 +13214,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200416111111
 20200416120128
 20200416120354
+20200511162057
+20200511162115
 \.
 

doc/user/project/issues/index.md

@@ -82,7 +82,7 @@ must be set.
 
 While you can view and manage the full details of an issue on the [issue page](#issue-page),
 you can also work with multiple issues at a time using the [Issues List](#issues-list),
-[Issue Boards](#issue-boards), Issue references, and [Epics](#epics-ultimate)**(ULTIMATE)**.
+[Issue Boards](#issue-boards), Issue references, and [Epics](#epics-premium)**(PREMIUM)**.
 
 Key actions for Issues include:
 
@@ -132,7 +132,7 @@ With [Design Management](design_management.md), you can upload design
 assets to issues and view them all together to easily share and
 collaborate with your team.
 
-### Epics **(ULTIMATE)**
+### Epics **(PREMIUM)**
 
 [Epics](../../group/epics/index.md) let you manage your portfolio of projects more
 efficiently and with less effort by tracking groups of issues that share a theme, across

doc/user/project/issues/issue_data_and_actions.md

@@ -16,7 +16,7 @@ You can find all the information for that issue on one screen.
 - **2.** [To Do](#to-do)
 - **3.** [Assignee](#assignee)
   - **3.1.** [Multiple Assignees **(STARTER)**](#multiple-assignees-starter)
-- **4.** [Epic **(ULTIMATE)**](#epic-ultimate)
+- **4.** [Epic **(PREMIUM)**](#epic-premium)
 - **5.** [Milestone](#milestone)
 - **6.** [Time tracking](#time-tracking)
 - **7.** [Due date](#due-date)
@@ -100,7 +100,7 @@ to track in large teams where there is shared ownership of an issue.
 In [GitLab Starter](https://about.gitlab.com/pricing/), you can
 [assign multiple people](multiple_assignees_for_issues.md) to an issue.
 
-### Epic **(ULTIMATE)**
+### Epic **(PREMIUM)**
 
 You can assign issues to an [Epic](../../group/epics/index.md), which allows better
 management of groups of related issues.

lib/gitlab/graphql/pagination/externally_paginated_array_connection.rb

@@ -23,6 +23,20 @@ module Gitlab
 
         alias_method :has_next_page, :next_page?
         alias_method :has_previous_page, :previous_page?
+
+        private
+
+        def load_nodes
+          @nodes ||= begin
+            # As the pagination happens externally we just grab all the nodes
+            limited_nodes = items
+
+            limited_nodes = limited_nodes.first(first) if first
+            limited_nodes = limited_nodes.last(last) if last
+
+            limited_nodes
+          end
+        end
       end
     end
   end

spec/controllers/projects/pages_domains_controller_spec.rb

@@ -148,16 +148,10 @@ describe Projects::PagesDomainsController do
   describe 'POST verify' do
     let(:params) { request_params.merge(id: pages_domain.domain) }
 
-    def stub_service
-      service = double(:service)
-
-      expect(VerifyPagesDomainService).to receive(:new) { service }
-
-      service
-    end
-
     it 'handles verification success' do
-      expect(stub_service).to receive(:execute).and_return(status: :success)
+      expect_next_instance_of(VerifyPagesDomainService, pages_domain) do |service|
+        expect(service).to receive(:execute).and_return(status: :success)
+      end
 
       post :verify, params: params
 
@@ -166,7 +160,9 @@ describe Projects::PagesDomainsController do
     end
 
     it 'handles verification failure' do
-      expect(stub_service).to receive(:execute).and_return(status: :failed)
+      expect_next_instance_of(VerifyPagesDomainService, pages_domain) do |service|
+        expect(service).to receive(:execute).and_return(status: :failed)
+      end
 
       post :verify, params: params
 

spec/features/projects/pages_spec.rb

@@ -158,6 +158,17 @@ shared_examples 'pages settings editing' do
           expect(page).to have_content('my.test.domain.com')
         end
 
+        it 'shows validation error if domain is duplicated' do
+          project.pages_domains.create!(domain: 'my.test.domain.com')
+
+          visit new_project_pages_domain_path(project)
+
+          fill_in 'Domain', with: 'my.test.domain.com'
+          click_button 'Create New Domain'
+
+          expect(page).to have_content('Domain has already been taken')
+        end
+
         describe 'with dns verification enabled' do
           before do
             stub_application_setting(pages_domain_verification_enabled: true)

spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb

@@ -19,6 +19,20 @@ describe Gitlab::Graphql::Pagination::ExternallyPaginatedArrayConnection do
     it_behaves_like 'connection with paged nodes' do
       let(:paged_nodes_size) { values.size }
     end
+
+    context 'when after or before is specified, they are ignored' do
+      # after and before are not used to filter the array, as they
+      # were already used to directly fetch the external array
+      it_behaves_like 'connection with paged nodes' do
+        let(:arguments) { { after: next_cursor } }
+        let(:paged_nodes_size) { values.size }
+      end
+
+      it_behaves_like 'connection with paged nodes' do
+        let(:arguments) { { before: prev_cursor } }
+        let(:paged_nodes_size) { values.size }
+      end
+    end
   end
 
   describe '#start_cursor' do

spec/uploaders/file_uploader_spec.rb

@@ -145,39 +145,57 @@ describe FileUploader do
   end
 
   describe '.extract_dynamic_path' do
-    context 'with a 32-byte hexadecimal secret in the path' do
-      let(:secret) { SecureRandom.hex }
-      let(:path) { "export/4b227777d4dd1fc61c6f884f48641d02b4d121d3fd328cb08b5531fcacdabf8a/test/uploads/#{secret}/dummy.txt" }
+    shared_examples 'a valid secret' do |root_path|
+      context 'with a 32-byte hexadecimal secret' do
+        let(:secret) { SecureRandom.hex }
+        let(:path) { File.join(*[root_path, secret, 'dummy.txt'].compact) }
 
-      it 'extracts the secret' do
-        expect(described_class.extract_dynamic_path(path)[:secret]).to eq(secret)
-      end
+        it 'extracts the secret' do
+          expect(described_class.extract_dynamic_path(path)[:secret]).to eq(secret)
+        end
 
-      it 'extracts the identifier' do
-        expect(described_class.extract_dynamic_path(path)[:identifier]).to eq('dummy.txt')
+        it 'extracts the identifier' do
+          expect(described_class.extract_dynamic_path(path)[:identifier]).to eq('dummy.txt')
+        end
       end
-    end
 
-    context 'with a 10-byte hexadecimal secret in the path' do
-      let(:secret) { SecureRandom.hex(10) }
-      let(:path) { "export/4b227777d4dd1fc61c6f884f48641d02b4d121d3fd328cb08b5531fcacdabf8a/test/uploads/#{secret}/dummy.txt" }
+      context 'with a 10-byte hexadecimal secret' do
+        let(:secret) { SecureRandom.hex[0, 10] }
+        let(:path) { File.join(*[root_path, secret, 'dummy.txt'].compact) }
+
+        it 'extracts the secret' do
+          expect(described_class.extract_dynamic_path(path)[:secret]).to eq(secret)
+        end
 
-      it 'extracts the secret' do
-        expect(described_class.extract_dynamic_path(path)[:secret]).to eq(secret)
+        it 'extracts the identifier' do
+          expect(described_class.extract_dynamic_path(path)[:identifier]).to eq('dummy.txt')
+        end
       end
 
-      it 'extracts the identifier' do
-        expect(described_class.extract_dynamic_path(path)[:identifier]).to eq('dummy.txt')
+      context 'with an invalid secret' do
+        let(:secret) { 'foo' }
+        let(:path) { File.join(*[root_path, secret, 'dummy.txt'].compact) }
+
+        it 'returns nil' do
+          expect(described_class.extract_dynamic_path(path)).to be_nil
+        end
       end
     end
 
-    context 'with an invalid secret in the path' do
-      let(:secret) { 'foo' }
-      let(:path) { "export/4b227777d4dd1fc61c6f884f48641d02b4d121d3fd328cb08b5531fcacdabf8a/test/uploads/#{secret}/dummy.txt" }
+    context 'with an absolute path without a slash in the beginning' do
+      it_behaves_like 'a valid secret', 'export/4b227777d4dd1fc61c6f884f48641d02b4d121d3fd328cb08b5531fcacdabf8a/test/uploads'
+    end
 
-      it 'returns nil' do
-        expect(described_class.extract_dynamic_path(path)).to be_nil
-      end
+    context 'with an absolute path with a slash in the beginning' do
+      it_behaves_like 'a valid secret', '/export/4b227777d4dd1fc61c6f884f48641d02b4d121d3fd328cb08b5531fcacdabf8a/test/uploads'
+    end
+
+    context 'with an relative path without a slash in the beginning' do
+      it_behaves_like 'a valid secret', nil
+    end
+
+    context 'with an relative path with a slash in the beginning' do
+      it_behaves_like 'a valid secret', '/'
     end
   end
 
@@ -202,7 +220,7 @@ describe FileUploader do
       end
 
       context "10-byte hexadecimal" do
-        let(:secret) { SecureRandom.hex(10) }
+        let(:secret) { SecureRandom.hex[0, 10] }
 
         it "returns the secret" do
           expect(uploader.secret).to eq(secret)

spec/views/projects/pages_domains/show.html.haml_spec.rb

@@ -7,7 +7,7 @@ describe 'projects/pages_domains/show' do
 
   before do
     assign(:project, project)
-    assign(:domain, domain.present)
+    allow(view).to receive(:domain_presenter).and_return(domain.present)
     stub_pages_setting(external_https: true)
   end