Automatic merge of gitlab-org/gitlab master

.gitlab/ci/test-metadata.gitlab-ci.yml

@@ -26,6 +26,7 @@ update-tests-metadata:
     - .test-metadata:rules:update-tests-metadata
   stage: post-test
   dependencies:
+    - retrieve-tests-metadata
     - setup-test-env
     - rspec migration pg12
     - rspec frontend_fixture

db/migrate/20210721145029_add_state_to_members.rb

@@ -4,14 +4,10 @@ class AddStateToMembers < ActiveRecord::Migration[6.1]
   include Gitlab::Database::MigrationHelpers
 
   def up
-    with_lock_retries do
-      add_column :members, :state, :integer, limit: 2, default: 0
-    end
+    # no-op
   end
 
   def down
-    with_lock_retries do
-      remove_column :members, :state
-    end
+    # no-op
   end
 end

db/structure.sql

@@ -14766,8 +14766,7 @@ CREATE TABLE members (
     expires_at date,
     ldap boolean DEFAULT false NOT NULL,
     override boolean DEFAULT false NOT NULL,
-    invite_email_success boolean DEFAULT true NOT NULL,
-    state smallint DEFAULT 0
+    invite_email_success boolean DEFAULT true NOT NULL
 );
 
 CREATE SEQUENCE members_id_seq

ee/app/controllers/ee/projects/security/configuration_controller.rb

@@ -47,11 +47,13 @@ module EE
         # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
         def auto_fix
-          service = ::Security::Configuration::SaveAutoFixService.new(project, auto_fix_params[:feature])
+          service = ::Security::Configuration::SaveAutoFixService
+                      .new(project, auto_fix_params[:feature])
+                      .execute(enabled: auto_fix_params[:enabled])
 
-          return respond_422 unless service.execute(enabled: auto_fix_params[:enabled])
+          return respond_422 unless service.success?
 
-          render status: :ok, json: auto_fix_settings
+          render status: :ok, json: service.payload
         end
 
         private
@@ -77,15 +79,6 @@ module EE
           render_404 if ::Feature.disabled?(:security_auto_fix, project)
         end
 
-        def auto_fix_settings
-          setting = project.security_setting
-
-          {
-            dependency_scanning: setting.auto_fix_dependency_scanning,
-            container_scanning: setting.auto_fix_container_scanning
-          }
-        end
-
         def security_dashboard_feature_enabled?
           vulnerable.feature_available?(:security_dashboard)
         end

ee/app/controllers/subscriptions_controller.rb

@@ -111,10 +111,6 @@ class SubscriptionsController < ApplicationController
     Gitlab::SubscriptionPortal::Client
   end
 
-  def customer_portal_new_subscription_url
-    "#{EE::SUBSCRIPTIONS_URL}/subscriptions/new?plan_id=#{params[:plan_id]}&transaction=create_subscription"
-  end
-
   def redirect_unauthenticated_user(from = action_name)
     return if current_user
 

ee/app/services/security/configuration/save_auto_fix_service.rb

@@ -13,15 +13,32 @@ module Security
       end
 
       def execute(enabled:)
-        return unless valid?
+        return error("Auto fix is not available for #{feature} feature") unless valid?
+        return error("Project has no security setting") unless setting
 
-        project&.security_setting&.update(toggle_params(enabled))
+        if setting&.update(toggle_params(enabled))
+          success(updated_setting)
+        else
+          error('Error during updating the auto fix param')
+        end
       end
 
       private
 
       attr_reader :enabled, :feature, :project
 
+      def error(message)
+        ServiceResponse.error(message: message)
+      end
+
+      def setting
+        @setting ||= project&.security_setting
+      end
+
+      def success(payload)
+        ServiceResponse.success(payload: payload)
+      end
+
       def toggle_params(enabled)
         if feature == 'all'
           {
@@ -37,6 +54,13 @@ module Security
         end
       end
 
+      def updated_setting
+        {
+          container_scanning: setting.auto_fix_container_scanning,
+          dependency_scanning: setting.auto_fix_dependency_scanning
+        }
+      end
+
       def valid?
         SUPPORTED_SCANNERS.include?(feature)
       end

ee/app/views/shared/issuable/_approver_suggestion.haml

@@ -15,5 +15,5 @@
   'mr_settings_path': presenter.api_approval_settings_path,
   'eligible_approvers_docs_path': help_page_path('user/project/merge_requests/approvals/rules', anchor: 'eligible-approvers'),
   'project_settings_path': presenter.api_project_approval_settings_path } }
-  = sprite_icon('spinner', size: 24, css_class: 'gl-spinner')
+  = sprite_icon('spinner', size: 24, css_class: 'gl-spinner gl-mt-5')
 = render 'projects/merge_requests/code_owner_approval_rules', merge_request: @mr_presenter

ee/spec/controllers/projects/security/configuration_controller_spec.rb

@@ -140,20 +140,13 @@ RSpec.describe Projects::Security::ConfigurationController do
       context 'with sufficient permissions' do
         let(:user) { maintainer }
 
-        it 'shows auto fix disable for dependency scanning for json format' do
-          get :show, params: { namespace_id: project.namespace, project_id: project, format: :json }
-
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response['auto_fix_enabled']).to include({ 'dependency_scanning' => false })
-        end
-
         context 'with setup feature param' do
           let(:feature) { :dependency_scanning }
 
           it 'processes request and updates setting' do
             expect(response).to have_gitlab_http_status(:ok)
             expect(project.security_setting.reload.auto_fix_dependency_scanning).to be_falsey
-            expect(response[:dependency_scanning]).to be_falsey
+            expect(json_response['dependency_scanning']).to be(false)
           end
         end
 
@@ -166,7 +159,8 @@ RSpec.describe Projects::Security::ConfigurationController do
             expect(response).to have_gitlab_http_status(:ok)
             expect(setting.auto_fix_dependency_scanning).to be_falsey
             expect(setting.auto_fix_dast).to be_falsey
-            expect(response[:container_scanning]).to be_falsey
+            expect(json_response['dependency_scanning']).to be(false)
+            expect(json_response['container_scanning']).to be(false)
           end
         end
 

ee/spec/services/security/configuration/save_auto_fix_service_spec.rb

@@ -6,16 +6,21 @@ RSpec.describe Security::Configuration::SaveAutoFixService do
   describe '#execute' do
     let_it_be_with_reload(:project) { create(:project) }
 
-    subject(:service) { described_class.new(project, feature) }
+    let(:service) { described_class.new(project, feature) }
 
-    before do
-      service.execute(enabled: false)
-    end
+    subject(:response) { service.execute(enabled: false) }
 
     context 'with supported scanner type' do
       let(:feature) { 'dependency_scanning' }
 
+      it 'returns success status' do
+        expect(response).to be_success
+        expect(response.payload).to eq({ container_scanning: true, dependency_scanning: false })
+      end
+
       it 'changes setting' do
+        response
+
         expect(project.security_setting.auto_fix_dependency_scanning).to be_falsey
       end
     end
@@ -23,7 +28,13 @@ RSpec.describe Security::Configuration::SaveAutoFixService do
     context 'with all scanners' do
       let(:feature) { 'all' }
 
+      it 'returns success status' do
+        expect(response).to be_success
+      end
+
       it 'changes setting' do
+        response
+
         expect(project.security_setting.auto_fix_dependency_scanning).to be_falsey
         expect(project.security_setting.auto_fix_container_scanning).to be_falsey
       end
@@ -33,7 +44,8 @@ RSpec.describe Security::Configuration::SaveAutoFixService do
       let(:feature) { :dep_scan }
 
       it 'does not change setting' do
-        expect(project.security_setting.auto_fix_dependency_scanning).to be_truthy
+        expect(response).to be_error
+        expect(response.message).to eq('Auto fix is not available for dep_scan feature')
       end
     end
   end

lib/gitlab/import_export/project/import_export.yml

@@ -328,9 +328,6 @@ excluded_attributes:
     - :release_id
   project_members:
     - :source_id
-    - :state
-  group_members:
-    - :state
   metrics:
     - :merge_request_id
     - :pipeline_id

scripts/rspec_helpers.sh

 #!/usr/bin/env bash
 
 function retrieve_tests_metadata() {
-  mkdir -p knapsack/ rspec_flaky/ rspec_profiling/
+  mkdir -p $(dirname "$KNAPSACK_RSPEC_SUITE_REPORT_PATH") $(dirname "$FLAKY_RSPEC_SUITE_REPORT_PATH") rspec_profiling/
 
-  # ${CI_DEFAULT_BRANCH} might not be master in other forks but we want to
-  # always target the canonical project here, so the branch must be hardcoded
-  local project_path="gitlab-org/gitlab"
-  local artifact_branch="master"
-  local test_metadata_job_id
+  if [[ -z "${RETRIEVE_TESTS_METADATA_FROM_ARTIFACTS}" ]]; then
+    if [[ ! -f "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" ]]; then
+      curl --location -o "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" "https://gitlab-org.gitlab.io/gitlab/${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}"
+    fi
 
-  # Ruby
-  test_metadata_job_id=$(scripts/api/get_job_id.rb --project "${project_path}" -q "status=success" -q "ref=${artifact_branch}" -q "username=gitlab-bot" -Q "scope=success" --job-name "update-tests-metadata")
+    if [[ ! -f "${FLAKY_RSPEC_SUITE_REPORT_PATH}" ]]; then
+      curl --location -o "${FLAKY_RSPEC_SUITE_REPORT_PATH}" "https://gitlab-org.gitlab.io/gitlab/${FLAKY_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${FLAKY_RSPEC_SUITE_REPORT_PATH}"
+    fi
+  else
+    # ${CI_DEFAULT_BRANCH} might not be master in other forks but we want to
+    # always target the canonical project here, so the branch must be hardcoded
+    local project_path="gitlab-org/gitlab"
+    local artifact_branch="master"
+    local test_metadata_job_id
 
-  if [[ ! -f "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" ]]; then
-    scripts/api/download_job_artifact.rb --project "${project_path}" --job-id "${test_metadata_job_id}" --artifact-path "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}"
-  fi
+    # Ruby
+    test_metadata_job_id=$(scripts/api/get_job_id.rb --project "${project_path}" -q "status=success" -q "ref=${artifact_branch}" -q "username=gitlab-bot" -Q "scope=success" --job-name "update-tests-metadata")
 
-  if [[ ! -f "${FLAKY_RSPEC_SUITE_REPORT_PATH}" ]]; then
-    scripts/api/download_job_artifact.rb --project "${project_path}" --job-id "${test_metadata_job_id}" --artifact-path "${FLAKY_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${FLAKY_RSPEC_SUITE_REPORT_PATH}"
+    if [[ ! -f "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" ]]; then
+      scripts/api/download_job_artifact.rb --project "${project_path}" --job-id "${test_metadata_job_id}" --artifact-path "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${KNAPSACK_RSPEC_SUITE_REPORT_PATH}"
+    fi
+
+    if [[ ! -f "${FLAKY_RSPEC_SUITE_REPORT_PATH}" ]]; then
+      scripts/api/download_job_artifact.rb --project "${project_path}" --job-id "${test_metadata_job_id}" --artifact-path "${FLAKY_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${FLAKY_RSPEC_SUITE_REPORT_PATH}"
+    fi
   fi
 }
 
@@ -40,18 +50,24 @@ function update_tests_metadata() {
 }
 
 function retrieve_tests_mapping() {
-  mkdir -p crystalball/
+  mkdir -p $(dirname "$RSPEC_PACKED_TESTS_MAPPING_PATH")
 
-  # ${CI_DEFAULT_BRANCH} might not be master in other forks but we want to
-  # always target the canonical project here, so the branch must be hardcoded
-  local project_path="gitlab-org/gitlab"
-  local artifact_branch="master"
-  local test_metadata_with_mapping_job_id
+  if [[ -z "${RETRIEVE_TESTS_METADATA_FROM_ARTIFACTS}" ]]; then
+    if [[ ! -f "${RSPEC_PACKED_TESTS_MAPPING_PATH}" ]]; then
+      (curl --location  -o "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz" "https://gitlab-org.gitlab.io/gitlab/${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz" && gzip -d "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz") || echo "{}" > "${RSPEC_PACKED_TESTS_MAPPING_PATH}"
+    fi
+  else
+    # ${CI_DEFAULT_BRANCH} might not be master in other forks but we want to
+    # always target the canonical project here, so the branch must be hardcoded
+    local project_path="gitlab-org/gitlab"
+    local artifact_branch="master"
+    local test_metadata_with_mapping_job_id
 
-  test_metadata_with_mapping_job_id=$(scripts/api/get_job_id.rb --project "${project_path}" -q "status=success" -q "ref=${artifact_branch}" -q "username=gitlab-bot" -Q "scope=success" --job-name "update-tests-metadata" --artifact-path "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz")
+    test_metadata_with_mapping_job_id=$(scripts/api/get_job_id.rb --project "${project_path}" -q "status=success" -q "ref=${artifact_branch}" -q "username=gitlab-bot" -Q "scope=success" --job-name "update-tests-metadata" --artifact-path "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz")
 
-  if [[ ! -f "${RSPEC_PACKED_TESTS_MAPPING_PATH}" ]]; then
-   (scripts/api/download_job_artifact.rb --project "${project_path}" --job-id "${test_metadata_with_mapping_job_id}" --artifact-path "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz" && gzip -d "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz") || echo "{}" > "${RSPEC_PACKED_TESTS_MAPPING_PATH}"
+    if [[ ! -f "${RSPEC_PACKED_TESTS_MAPPING_PATH}" ]]; then
+     (scripts/api/download_job_artifact.rb --project "${project_path}" --job-id "${test_metadata_with_mapping_job_id}" --artifact-path "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz" && gzip -d "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz") || echo "{}" > "${RSPEC_PACKED_TESTS_MAPPING_PATH}"
+    fi
   fi
 
   scripts/unpack-test-mapping "${RSPEC_PACKED_TESTS_MAPPING_PATH}" "${RSPEC_TESTS_MAPPING_PATH}"