Add latest changes from gitlab-org/gitlab@master

11 files changed, 155 insertions, 23 deletions

diff --git a/app/assets/javascripts/import_entities/import_projects/components/import_projects_table.vue b/app/assets/javascripts/import_entities/import_projects/components/import_projects_table.vue
index bd0f4cd5dd7..e0703a77424 100644
--- a/app/assets/javascripts/import_entities/import_projects/components/import_projects_table.vue
+++ b/app/assets/javascripts/import_entities/import_projects/components/import_projects_table.vue
@@ -109,7 +109,7 @@ export default {
     </template>
     <div class="gl-display-flex gl-justify-content-space-between gl-flex-wrap gl-mb-5">
       <gl-button
-        variant="success"
+        variant="confirm"
         :loading="isImportingAnyRepo"
         :disabled="!hasImportableRepos"
         type="button"
diff --git a/app/models/ci/secure_file.rb b/app/models/ci/secure_file.rb
index 1509344415d..6a26a5341aa 100644
--- a/app/models/ci/secure_file.rb
+++ b/app/models/ci/secure_file.rb
@@ -15,6 +15,7 @@ module Ci
 
     validates :file, presence: true, file_size: { maximum: FILE_SIZE_LIMIT }
     validates :checksum, :file_store, :name, :permissions, :project_id, presence: true
+    validates :name, uniqueness: { scope: :project }
 
     after_initialize :generate_key_data
     before_validation :assign_checksum
diff --git a/app/models/container_repository.rb b/app/models/container_repository.rb
index 92e53beebae..78bd520d5d5 100644
--- a/app/models/container_repository.rb
+++ b/app/models/container_repository.rb
@@ -34,7 +34,17 @@ class ContainerRepository < ApplicationRecord
 
   enum status: { delete_scheduled: 0, delete_failed: 1 }
   enum expiration_policy_cleanup_status: { cleanup_unscheduled: 0, cleanup_scheduled: 1, cleanup_unfinished: 2, cleanup_ongoing: 3 }
-  enum migration_skipped_reason: { not_in_plan: 0, too_many_retries: 1, too_many_tags: 2, root_namespace_in_deny_list: 3, migration_canceled: 4, not_found: 5, native_import: 6 }
+
+  enum migration_skipped_reason: {
+    not_in_plan: 0,
+    too_many_retries: 1,
+    too_many_tags: 2,
+    root_namespace_in_deny_list: 3,
+    migration_canceled: 4,
+    not_found: 5,
+    native_import: 6,
+    migration_forced_canceled: 7
+  }
 
   delegate :client, :gitlab_api_client, to: :registry
 
@@ -504,6 +514,19 @@ class ContainerRepository < ApplicationRecord
     gitlab_api_client.cancel_repository_import(self.path)
   end
 
+  # This method is not meant for consumption by the code
+  # It is meant for manual use in the case that a migration needs to be
+  # cancelled by an admin or SRE
+  def force_migration_cancel
+    return :error unless gitlab_api_client.supports_gitlab_api?
+
+    response = gitlab_api_client.cancel_repository_import(self.path, force: true)
+
+    skip_import(reason: :migration_forced_canceled) if response[:status] == :ok
+
+    response
+  end
+
   def self.build_from_path(path)
     self.new(project: path.repository_project,
              name: path.repository_name)
diff --git a/doc/api/secure_files.md b/doc/api/secure_files.md
index 8775e9a5f46..7d30cc0c4c7 100644
--- a/doc/api/secure_files.md
+++ b/doc/api/secure_files.md
@@ -103,7 +103,7 @@ Supported attributes:
 | Attribute       | Type           | Required               | Description |
 |-----------------|----------------|------------------------|-------------|
 | `project_id`    | integer/string | **{check-circle}** Yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding) owned by the authenticated user. |
-| `name`          | string         | **{check-circle}** Yes | The `name` of the file being uploaded. |
+| `name`          | string         | **{check-circle}** Yes | The `name` of the file being uploaded. The file name must be unique within the project. |
 | `file`          | file           | **{check-circle}** Yes | The `file` being uploaded (5 MB limit). |
 | `permissions`   | string         | **{dotted-circle}** No | The file is created with the specified permissions when created in the CI/CD job. Available types are: `read_only` (default), `read_write`, and `execute`. |
 
diff --git a/lib/container_registry/gitlab_api_client.rb b/lib/container_registry/gitlab_api_client.rb
index 565ce3a8c2e..0cd8f8509f6 100644
--- a/lib/container_registry/gitlab_api_client.rb
+++ b/lib/container_registry/gitlab_api_client.rb
@@ -58,10 +58,12 @@ module ContainerRegistry
       IMPORT_RESPONSES.fetch(response.status, :error)
     end
 
-    # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#import-repository
-    def cancel_repository_import(path)
+    # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#cancel-repository-import
+    def cancel_repository_import(path, force: false)
       response = with_import_token_faraday do |faraday_client|
-        faraday_client.delete(import_url_for(path))
+        faraday_client.delete(import_url_for(path)) do |req|
+          req.params['force'] = true if force
+        end
       end
 
       status = IMPORT_RESPONSES.fetch(response.status, :error)
diff --git a/qa/qa/page/admin/settings/component/snowplow.rb b/qa/qa/page/admin/settings/component/snowplow.rb
index e05679feac3..c7f103e29a8 100644
--- a/qa/qa/page/admin/settings/component/snowplow.rb
+++ b/qa/qa/page/admin/settings/component/snowplow.rb
@@ -31,11 +31,11 @@ module QA
             private
 
             def check_snowplow_enabled_checkbox
-              check_element(:snowplow_enabled_checkbox)
+              check_element(:snowplow_enabled_checkbox, true)
             end
 
             def uncheck_snowplow_enabled_checkbox
-              uncheck_element(:snowplow_enabled_checkbox)
+              uncheck_element(:snowplow_enabled_checkbox, true)
             end
 
             def click_save_changes_button
diff --git a/spec/frontend/import_entities/import_projects/components/import_projects_table_spec.js b/spec/frontend/import_entities/import_projects/components/import_projects_table_spec.js
index 16adf88700f..88fcedd31b2 100644
--- a/spec/frontend/import_entities/import_projects/components/import_projects_table_spec.js
+++ b/spec/frontend/import_entities/import_projects/components/import_projects_table_spec.js
@@ -31,7 +31,7 @@ describe('ImportProjectsTable', () => {
   const findImportAllButton = () =>
     wrapper
       .findAll(GlButton)
-      .filter((w) => w.props().variant === 'success')
+      .filter((w) => w.props().variant === 'confirm')
       .at(0);
   const findImportAllModal = () => wrapper.find({ ref: 'importAllModal' });
 
diff --git a/spec/lib/container_registry/gitlab_api_client_spec.rb b/spec/lib/container_registry/gitlab_api_client_spec.rb
index 9fe74534292..16d2c42f332 100644
--- a/spec/lib/container_registry/gitlab_api_client_spec.rb
+++ b/spec/lib/container_registry/gitlab_api_client_spec.rb
@@ -107,7 +107,9 @@ RSpec.describe ContainerRegistry::GitlabApiClient do
   end
 
   describe '#cancel_repository_import' do
-    subject { client.cancel_repository_import(path) }
+    let(:force) { false }
+
+    subject { client.cancel_repository_import(path, force: force) }
 
     where(:status_code, :expected_result) do
       200 | :already_imported
@@ -124,7 +126,7 @@ RSpec.describe ContainerRegistry::GitlabApiClient do
 
     with_them do
       before do
-        stub_import_cancel(path, status_code)
+        stub_import_cancel(path, status_code, force: force)
       end
 
       it { is_expected.to eq({ status: expected_result, migration_state: nil }) }
@@ -134,11 +136,21 @@ RSpec.describe ContainerRegistry::GitlabApiClient do
       let(:status) { 'this_is_a_test' }
 
       before do
-        stub_import_cancel(path, 400, status: status)
+        stub_import_cancel(path, 400, status: status, force: force)
       end
 
       it { is_expected.to eq({ status: :bad_request, migration_state: status }) }
     end
+
+    context 'force cancel' do
+      let(:force) { true }
+
+      before do
+        stub_import_cancel(path, 202, force: force)
+      end
+
+      it { is_expected.to eq({ status: :ok, migration_state: nil }) }
+    end
   end
 
   describe '#import_status' do
@@ -330,15 +342,24 @@ RSpec.describe ContainerRegistry::GitlabApiClient do
       )
   end
 
-  def stub_import_cancel(path, http_status, status: nil)
+  def stub_import_cancel(path, http_status, status: nil, force: false)
     body = {}
 
     if http_status == 400
       body = { status: status }
     end
 
-    stub_request(:delete, "#{registry_api_url}/gitlab/v1/import/#{path}/")
-      .with(headers: { 'Accept' => described_class::JSON_TYPE, 'Authorization' => "bearer #{import_token}" })
+    headers = {
+      'Accept' => described_class::JSON_TYPE,
+      'Authorization' => "bearer #{import_token}",
+      'User-Agent' => "GitLab/#{Gitlab::VERSION}",
+      'Accept-Encoding' => 'gzip;q=1.0,deflate;q=0.6,identity;q=0.3'
+    }
+
+    params = force ? '?force=true' : ''
+
+    stub_request(:delete, "#{registry_api_url}/gitlab/v1/import/#{path}/#{params}")
+      .with(headers: headers)
       .to_return(
         status: http_status,
         body: body.to_json,
diff --git a/spec/models/ci/secure_file_spec.rb b/spec/models/ci/secure_file_spec.rb
index 4382385aaf5..f92db3fe8db 100644
--- a/spec/models/ci/secure_file_spec.rb
+++ b/spec/models/ci/secure_file_spec.rb
@@ -3,14 +3,14 @@
 require 'spec_helper'
 
 RSpec.describe Ci::SecureFile do
-  let(:sample_file) { fixture_file('ci_secure_files/upload-keystore.jks') }
-
-  subject { create(:ci_secure_file) }
-
   before do
     stub_ci_secure_file_object_storage
   end
 
+  let(:sample_file) { fixture_file('ci_secure_files/upload-keystore.jks') }
+
+  subject { create(:ci_secure_file, file: CarrierWaveStringFile.new(sample_file)) }
+
   it { is_expected.to be_a FileStoreMounter }
 
   it { is_expected.to belong_to(:project).required }
@@ -27,6 +27,26 @@ RSpec.describe Ci::SecureFile do
     it { is_expected.to validate_presence_of(:name) }
     it { is_expected.to validate_presence_of(:permissions) }
     it { is_expected.to validate_presence_of(:project_id) }
+    context 'unique filename' do
+      let_it_be(:project1) { create(:project) }
+
+      it 'ensures the file name is unique within a given project' do
+        file1 = create(:ci_secure_file, project: project1, name: 'file1')
+        expect do
+          create(:ci_secure_file, project: project1, name: 'file1')
+        end.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Name has already been taken')
+
+        expect(project1.secure_files.where(name: 'file1').count).to be 1
+        expect(project1.secure_files.find_by(name: 'file1').id).to eq(file1.id)
+      end
+
+      it 'allows duplicate file names in different projects' do
+        create(:ci_secure_file, project: project1)
+        expect do
+          create(:ci_secure_file, project: create(:project))
+        end.not_to raise_error
+      end
+    end
   end
 
   describe '#permissions' do
@@ -37,8 +57,6 @@ RSpec.describe Ci::SecureFile do
 
   describe '#checksum' do
     it 'computes SHA256 checksum on the file before encrypted' do
-      subject.file = CarrierWaveStringFile.new(sample_file)
-      subject.save!
       expect(subject.checksum).to eq(Digest::SHA256.hexdigest(sample_file))
     end
   end
@@ -51,8 +69,6 @@ RSpec.describe Ci::SecureFile do
 
   describe '#file' do
     it 'returns the saved file' do
-      subject.file = CarrierWaveStringFile.new(sample_file)
-      subject.save!
       expect(Base64.encode64(subject.file.read)).to eq(Base64.encode64(sample_file))
     end
   end
diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb
index 6a4f2fb3e30..2ea042fb767 100644
--- a/spec/models/container_repository_spec.rb
+++ b/spec/models/container_repository_spec.rb
@@ -762,6 +762,61 @@ RSpec.describe ContainerRepository, :aggregate_failures do
 
       it_behaves_like 'gitlab migration client request', :cancel_repository_import
     end
+
+    describe '#force_migration_cancel' do
+      subject { repository.force_migration_cancel }
+
+      shared_examples 'returning the same response as the client' do
+        it 'returns the same response' do
+          expect(repository.gitlab_api_client)
+            .to receive(:cancel_repository_import).with(repository.path, force: true).and_return(client_response)
+
+          expect(subject).to eq(client_response)
+        end
+      end
+
+      context 'successful cancellation' do
+        let(:client_response) { { status: :ok } }
+
+        it_behaves_like 'returning the same response as the client'
+
+        it 'skips the migration' do
+          expect(repository.gitlab_api_client)
+            .to receive(:cancel_repository_import).with(repository.path, force: true).and_return(client_response)
+
+          expect { subject }.to change { repository.reload.migration_state }.to('import_skipped')
+            .and change { repository.migration_skipped_reason }.to('migration_forced_canceled')
+            .and change { repository.migration_skipped_at }
+        end
+      end
+
+      context 'failed cancellation' do
+        let(:client_response) { { status: :error } }
+
+        it_behaves_like 'returning the same response as the client'
+
+        it 'does not skip the migration' do
+          expect(repository.gitlab_api_client)
+            .to receive(:cancel_repository_import).with(repository.path, force: true).and_return(client_response)
+
+          expect { subject }.to not_change { repository.reload.migration_state }
+            .and not_change { repository.migration_skipped_reason }
+            .and not_change { repository.migration_skipped_at }
+        end
+      end
+
+      context 'when the gitlab_api feature is not supported' do
+        before do
+          allow(repository.gitlab_api_client).to receive(:supports_gitlab_api?).and_return(false)
+        end
+
+        it 'returns :error' do
+          expect(repository.gitlab_api_client).not_to receive(:cancel_repository_import)
+
+          expect(subject).to eq(:error)
+        end
+      end
+    end
   end
 
   describe '.build_from_path' do
diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb
index 56838c4f6fd..d699fe02ba4 100644
--- a/spec/requests/api/ci/secure_files_spec.rb
+++ b/spec/requests/api/ci/secure_files_spec.rb
@@ -293,6 +293,20 @@ RSpec.describe API::Ci::SecureFiles do
         expect(json_response['error']).to eq('name is missing')
       end
 
+      it 'returns an error when the file name has already been used' do
+        post_params = {
+          name: secure_file.name,
+          file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks')
+        }
+
+        expect do
+          post api("/projects/#{project.id}/secure_files", maintainer), params: post_params
+        end.not_to change { project.secure_files.count }
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(json_response['message']['name']).to include('has already been taken')
+      end
+
       it 'returns an error when an unexpected permission is supplied' do
         post_params = {
           file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),