Merge branch 'remove_cluster_image_scanning_from_approval_project_rules' into 'master'

Remove cluster_image_scanning from approval_project_rules table

See merge request gitlab-org/gitlab!67272

db/migrate/20210804202057_add_tmp_index_approval_project_rules_scanners.rb

+# frozen_string_literal: true
+
+# See https://docs.gitlab.com/ee/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddTmpIndexApprovalProjectRulesScanners < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  INDEX_NAME = 'tmp_index_approval_project_rules_scanners'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :approval_project_rules, :scanners, name: INDEX_NAME, using: :gin, where: "scanners @> '{cluster_image_scanning}'"
+  end
+
+  def down
+    remove_concurrent_index_by_name :approval_project_rules, INDEX_NAME
+  end
+end

db/post_migrate/20210804150624_remove_cluster_image_scanning_from_approval_project_rules.rb

+# frozen_string_literal: true
+
+class RemoveClusterImageScanningFromApprovalProjectRules < ActiveRecord::Migration[6.1]
+  def up
+    execute("update approval_project_rules set scanners = array_remove(scanners, 'cluster_image_scanning') where scanners @> '{cluster_image_scanning}'")
+  end
+
+  def down
+    # nothing to do here
+  end
+end

db/schema_migrations/20210804150624

+52e71aa3ec92473006b37e9319797133356f7747b91c32b09a746e183501655f
\ No newline at end of file

db/schema_migrations/20210804202057

+48f140728fede7cf38469c8dfcb5480b4f2b8e29af4b1edd5d38024548493c2d
\ No newline at end of file

db/structure.sql

@@ -25596,6 +25596,8 @@ CREATE INDEX tmp_idx_deduplicate_vulnerability_occurrences ON vulnerability_occu
 
 CREATE INDEX tmp_idx_on_namespaces_delayed_project_removal ON namespaces USING btree (id) WHERE (delayed_project_removal = true);
 
+CREATE INDEX tmp_index_approval_project_rules_scanners ON approval_project_rules USING gin (scanners) WHERE (scanners @> '{cluster_image_scanning}'::text[]);
+
 CREATE INDEX tmp_index_merge_requests_draft_and_status ON merge_requests USING btree (id) WHERE ((draft = false) AND (state_id = 1) AND ((title)::text ~* '^\[draft\]|\(draft\)|draft:|draft|\[WIP\]|WIP:|WIP'::text));
 
 CREATE INDEX tmp_index_namespaces_empty_traversal_ids_with_child_namespaces ON namespaces USING btree (id) WHERE ((parent_id IS NOT NULL) AND (traversal_ids = '{}'::integer[]));

ee/app/assets/javascripts/approvals/components/rule_form.vue

@@ -6,7 +6,7 @@ import {
   GlFormCheckbox,
   GlFormCheckboxGroup,
 } from '@gitlab/ui';
-import { groupBy, isEqual, isNumber } from 'lodash';
+import { groupBy, isEqual, isNumber, omit } from 'lodash';
 import { mapState, mapActions } from 'vuex';
 import { REPORT_TYPES } from 'ee/security_dashboard/store/constants';
 import ProtectedBranchesSelector from 'ee/vue_shared/components/branches_selector/protected_branches_selector.vue';
@@ -26,6 +26,8 @@ import ApproversSelect from './approvers_select.vue';
 
 const DEFAULT_NAME = 'Default';
 
+const EXCLUDED_REPORT_TYPE = 'cluster_image_scanning';
+
 export const READONLY_NAMES = [LICENSE_CHECK_NAME, VULNERABILITY_CHECK_NAME, COVERAGE_CHECK_NAME];
 
 function mapServerResponseToValidationErrors(messages) {
@@ -362,7 +364,7 @@ export default {
     },
   },
   APPROVAL_DIALOG_I18N,
-  REPORT_TYPES,
+  REPORT_TYPES: omit(REPORT_TYPES, EXCLUDED_REPORT_TYPE),
 };
 </script>
 
@@ -417,7 +419,7 @@ export default {
         </gl-form-checkbox>
         <gl-form-checkbox-group
           v-model="scanners"
-          :options="this.$options.REPORT_TYPES"
+          :options="$options.REPORT_TYPES"
           class="gl-ml-2"
         />
       </gl-dropdown>

ee/app/models/approval_project_rule.rb

@@ -4,6 +4,9 @@ class ApprovalProjectRule < ApplicationRecord
   include ApprovalRuleLike
   include Auditable
 
+  UNSUPPORTED_SCANNER = 'cluster_image_scanning'
+  SUPPORTED_SCANNERS = (::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES - [UNSUPPORTED_SCANNER]).freeze
+
   belongs_to :project
   has_and_belongs_to_many :protected_branches
   has_many :approval_merge_request_rule_sources
@@ -22,8 +25,8 @@ class ApprovalProjectRule < ApplicationRecord
   validates :name, uniqueness: { scope: [:project_id, :rule_type] }
   validates :rule_type, uniqueness: { scope: :project_id, message: proc { _('any-approver for the project already exists') } }, if: :any_approver?
 
-  validates :scanners, inclusion: { in: ::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES }
-  default_value_for :scanners, allows_nil: false, value: ::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES
+  validates :scanners, if: :scanners_changed?, inclusion: { in: SUPPORTED_SCANNERS }
+  default_value_for :scanners, allows_nil: false, value: SUPPORTED_SCANNERS
 
   validates :vulnerabilities_allowed, numericality: { only_integer: true }
   default_value_for :vulnerabilities_allowed, allows_nil: false, value: 0

ee/spec/frontend/approvals/components/rule_form_spec.js

-import { GlFormGroup, GlFormInput } from '@gitlab/ui';
+import { GlFormGroup, GlFormInput, GlFormCheckboxGroup } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import Vue, { nextTick } from 'vue';
 import Vuex from 'vuex';
@@ -620,6 +620,11 @@ describe('EE Approvals RuleForm', () => {
             expect.objectContaining({ scanners }),
           );
         });
+
+        it('does not contain unsupported report type', () => {
+          const group = wrapper.find(GlFormCheckboxGroup);
+          expect(Object.keys(group.props('options'))).not.toContain('cluster_image_scanning');
+        });
       });
     });
   });

ee/spec/models/approval_project_rule_spec.rb

@@ -162,14 +162,14 @@ RSpec.describe ApprovalProjectRule do
         using RSpec::Parameterized::TableSyntax
 
         where(:is_valid, :scanners, :vulnerabilities_allowed) do
-          true  | []                       | 0
-          true  | %w(dast)                 | 1
-          true  | %w(dast sast)            | 10
-          true  | %w(dast dast)            | 100
-          false | %w(dast unknown_scanner) | 100
-          false | %w(unknown_scanner)      | 100
-          false | %w(dast sast)            | 1.1
-          false | %w(dast sast)            | 'one'
+          true  | []                                     | 0
+          true  | %w(dast)                               | 1
+          true  | %w(dast sast)                          | 10
+          true  | %w(dast dast)                          | 100
+          false | %w(dast unknown_scanner)               | 100
+          false | [described_class::UNSUPPORTED_SCANNER] | 100
+          false | %w(dast sast)                          | 1.1
+          false | %w(dast sast)                          | 'one'
         end
 
         with_them do

ee/spec/support/shared_examples/requests/api/project_approval_rules_api_shared_examples.rb

@@ -128,7 +128,7 @@ RSpec.shared_examples 'an API endpoint for updating project approval rule' do
       it 'returns 200 status' do
         expect do
           put api(url, current_user), params: { scanners: scanners }
-        end.to change { approval_rule.reload.scanners.count }.from(::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES.count).to(scanners.count)
+        end.to change { approval_rule.reload.scanners.count }.from(::ApprovalProjectRule::SUPPORTED_SCANNERS.count).to(scanners.count)
         expect(response).to have_gitlab_http_status(:ok)
       end
     end