Commit 7c60a340 authored by Robert Speicher's avatar Robert Speicher

Merge branch '13985_rename_security_approval_rule_enum' into 'master'

Rename approval rule `security` enum as `vulnerability`

See merge request gitlab-org/gitlab!45537
parents 92decc1c 59117bea
......@@ -55,12 +55,12 @@ class ApprovalMergeRequestRule < ApplicationRecord
alias_method :code_owner, :code_owner?
enum report_type: {
security: 1,
vulnerability: 1,
license_scanning: 2
}
scope :security_report, -> { report_approver.where(report_type: :security) }
scope :license_compliance, -> { report_approver.where(report_type: :license_scanning) }
scope :vulnerability_report, -> { report_approver.vulnerability }
scope :license_compliance, -> { report_approver.license_scanning }
scope :with_head_pipeline, -> { includes(merge_request: [:head_pipeline]) }
scope :open_merge_requests, -> { merge(MergeRequest.opened) }
scope :for_checks_that_can_be_refreshed, -> { license_compliance.open_merge_requests.with_head_pipeline }
......
......@@ -5,10 +5,10 @@ module ApprovalRuleLike
DEFAULT_NAME = 'Default'
DEFAULT_NAME_FOR_LICENSE_REPORT = 'License-Check'
DEFAULT_NAME_FOR_SECURITY_REPORT = 'Vulnerability-Check'
DEFAULT_NAME_FOR_VULNERABILITY_REPORT = 'Vulnerability-Check'
REPORT_TYPES_BY_DEFAULT_NAME = {
DEFAULT_NAME_FOR_LICENSE_REPORT => :license_scanning,
DEFAULT_NAME_FOR_SECURITY_REPORT => :security
DEFAULT_NAME_FOR_VULNERABILITY_REPORT => :vulnerability
}.freeze
APPROVALS_REQUIRED_MAX = 100
ALL_MEMBERS = 'All Members'
......
......@@ -42,7 +42,7 @@ module Security
# If we don't have reports, then we should wait until pipeline stops.
return if reports.empty? && !pipeline.complete?
remove_required_approvals_for(ApprovalMergeRequestRule.security_report, sync_required_merge_requests)
remove_required_approvals_for(ApprovalMergeRequestRule.vulnerability_report, sync_required_merge_requests)
end
def reports
......
......@@ -21,7 +21,7 @@ FactoryBot.define do
factory :report_approver_rule, parent: :approval_merge_request_rule do
merge_request
rule_type { :report_approver }
report_type { :security }
report_type { :vulnerability }
sequence(:name) { |n| "*-#{n}.js" }
trait :requires_approval do
......@@ -48,13 +48,13 @@ FactoryBot.define do
approvals_required { rand(1..ApprovalProjectRule::APPROVALS_REQUIRED_MAX) }
end
trait :security_report do
trait :vulnerability_report do
rule_type { :report_approver }
name { ApprovalRuleLike::DEFAULT_NAME_FOR_SECURITY_REPORT }
name { ApprovalRuleLike::DEFAULT_NAME_FOR_VULNERABILITY_REPORT }
end
trait :security do
security_report
trait :vulnerability do
vulnerability_report
end
trait :license_scanning do
......
......@@ -136,9 +136,9 @@ RSpec.describe ApprovalMergeRequestRule do
end
end
describe '.security_report' do
describe '.vulnerability_report' do
it 'returns the correct rules' do
expect(described_class.security_report)
expect(described_class.vulnerability_report)
.to contain_exactly(report_approver_rule)
end
end
......
......@@ -21,7 +21,7 @@ RSpec.describe ApprovalProjectRule do
describe '.regular' do
it 'returns non-report_approver records' do
rules = create_list(:approval_project_rule, 2)
create(:approval_project_rule, :security_report)
create(:approval_project_rule, :vulnerability_report)
expect(described_class.regular).to contain_exactly(*rules)
end
......@@ -31,7 +31,7 @@ RSpec.describe ApprovalProjectRule do
it 'returns regular or any-approver rules' do
any_approver_rule = create(:approval_project_rule, rule_type: :any_approver)
regular_rule = create(:approval_project_rule)
create(:approval_project_rule, :security_report)
create(:approval_project_rule, :vulnerability_report)
expect(described_class.regular_or_any_approver).to(
contain_exactly(any_approver_rule, regular_rule)
......@@ -48,14 +48,14 @@ RSpec.describe ApprovalProjectRule do
end
describe '#regular?' do
let(:security_approver_rule) { build(:approval_project_rule, :security_report) }
let(:vulnerability_approver_rule) { build(:approval_project_rule, :vulnerability_report) }
it 'returns true for regular rules' do
expect(subject.regular?).to eq(true)
end
it 'returns false for report_approver rules' do
expect(security_approver_rule.regular?). to eq(false)
expect(vulnerability_approver_rule.regular?). to eq(false)
end
end
......@@ -66,14 +66,14 @@ RSpec.describe ApprovalProjectRule do
end
describe '#report_approver?' do
let(:security_approver_rule) { build(:approval_project_rule, :security_report) }
let(:vulnerability_approver_rule) { build(:approval_project_rule, :vulnerability_report) }
it 'returns false for regular rules' do
expect(subject.report_approver?).to eq(false)
end
it 'returns true for report_approver rules' do
expect(security_approver_rule.report_approver?). to eq(true)
expect(vulnerability_approver_rule.report_approver?). to eq(true)
end
end
......@@ -82,8 +82,8 @@ RSpec.describe ApprovalProjectRule do
expect(build(:approval_project_rule).rule_type).to eq('regular')
end
it 'returns the report_approver type for security report approvers rules' do
expect(build(:approval_project_rule, :security_report).rule_type).to eq('report_approver')
it 'returns the report_approver type for vulnerability report approvers rules' do
expect(build(:approval_project_rule, :vulnerability_report).rule_type).to eq('report_approver')
end
end
......@@ -114,7 +114,7 @@ RSpec.describe ApprovalProjectRule do
describe "validation" do
let(:project_approval_rule) { create(:approval_project_rule) }
let(:license_compliance_rule) { create(:approval_project_rule, :license_scanning) }
let(:vulnerability_check_rule) { create(:approval_project_rule, :security) }
let(:vulnerability_check_rule) { create(:approval_project_rule, :vulnerability) }
context "when creating a new rule" do
specify { expect(project_approval_rule).to be_valid }
......@@ -166,7 +166,7 @@ RSpec.describe ApprovalProjectRule do
let_it_be(:new_user) { create(:user, name: 'Spiderman') }
let_it_be(:new_group) { create(:group, name: 'Avengers') }
let_it_be(:rule, reload: true) { create(:approval_project_rule, name: 'Security', users: [user], groups: [group]) }
let_it_be(:rule, reload: true) { create(:approval_project_rule, name: 'Vulnerability', users: [user], groups: [group]) }
shared_examples 'auditable' do
context 'when audit event queue is active' do
......@@ -196,28 +196,28 @@ RSpec.describe ApprovalProjectRule do
describe '#audit_add users after :add' do
let(:action!) { rule.update(users: [user, new_user]) }
let(:message) { 'Added User Spiderman to approval group on Security rule' }
let(:message) { 'Added User Spiderman to approval group on Vulnerability rule' }
it_behaves_like 'auditable'
end
describe '#audit_remove users after :remove' do
let(:action!) { rule.update(users: []) }
let(:message) { 'Removed User Batman from approval group on Security rule' }
let(:message) { 'Removed User Batman from approval group on Vulnerability rule' }
it_behaves_like 'auditable'
end
describe '#audit_add groups after :add' do
let(:action!) { rule.update(groups: [group, new_group]) }
let(:message) { 'Added Group Avengers to approval group on Security rule' }
let(:message) { 'Added Group Avengers to approval group on Vulnerability rule' }
it_behaves_like 'auditable'
end
describe '#audit_remove groups after :remove' do
let(:action!) { rule.update(groups: []) }
let(:message) { 'Removed Group Justice League from approval group on Security rule' }
let(:message) { 'Removed Group Justice League from approval group on Vulnerability rule' }
it_behaves_like 'auditable'
end
......
......@@ -16,7 +16,7 @@ RSpec.describe API::ProjectApprovalRules do
context 'when the request is correct' do
let!(:rule) do
rule = create(:approval_project_rule, name: 'security', project: project, approvals_required: 7)
rule = create(:approval_project_rule, name: 'vulnerability', project: project, approvals_required: 7)
rule.users << approver
rule
end
......@@ -40,7 +40,7 @@ RSpec.describe API::ProjectApprovalRules do
rule = json.first
expect(rule['approvals_required']).to eq(7)
expect(rule['name']).to eq('security')
expect(rule['name']).to eq('vulnerability')
end
context 'private group filtering' do
......@@ -73,7 +73,7 @@ RSpec.describe API::ProjectApprovalRules do
context 'report_approver rules' do
let!(:report_approver_rule) do
create(:approval_project_rule, :security_report, project: project)
create(:approval_project_rule, :vulnerability_report, project: project)
end
it 'includes report_approver rules' do
......
......@@ -16,7 +16,7 @@ RSpec.describe API::ProjectApprovalSettings do
context 'when the request is correct' do
let!(:rule) do
rule = create(:approval_project_rule, name: 'security', project: project, approvals_required: 7)
rule = create(:approval_project_rule, name: 'vulnerability', project: project, approvals_required: 7)
rule.users << approver
rule
end
......@@ -40,7 +40,7 @@ RSpec.describe API::ProjectApprovalSettings do
rule = json['rules'].first
expect(rule['approvals_required']).to eq(7)
expect(rule['name']).to eq('security')
expect(rule['name']).to eq('vulnerability')
end
context 'when target_branch is specified' do
......@@ -103,7 +103,7 @@ RSpec.describe API::ProjectApprovalSettings do
context 'report_approver rules' do
let!(:report_approver_rule) do
create(:approval_project_rule, :security_report, project: project)
create(:approval_project_rule, :vulnerability_report, project: project)
end
it 'includes report_approver rules' do
......
......@@ -12,35 +12,35 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do
stub_licensed_features(report_approver_rules: true)
end
context "when a project has a single `#{ApprovalProjectRule::DEFAULT_NAME_FOR_SECURITY_REPORT}` approval rule" do
let!(:security_approval_project_rule) { create(:approval_project_rule, :security_report, project: merge_request.target_project, approvals_required: 2) }
context "when a project has a single `#{ApprovalProjectRule::DEFAULT_NAME_FOR_VULNERABILITY_REPORT}` approval rule" do
let!(:vulnerability_approval_project_rule) { create(:approval_project_rule, :vulnerability_report, project: merge_request.target_project, approvals_required: 2) }
context 'when report_approver_rules are enabled' do
let!(:regular_approval_project_rule) { create(:approval_project_rule, project: merge_request.target_project) }
it 'creates rule for report approvers' do
expect { service.execute }
.to change { merge_request.approval_rules.security_report.count }.from(0).to(1)
.to change { merge_request.approval_rules.vulnerability_report.count }.from(0).to(1)
rule = merge_request.approval_rules.security_report.first
rule = merge_request.approval_rules.vulnerability_report.first
expect(rule).to be_report_approver
expect(rule.report_type).to eq 'security'
expect(rule.name).to eq(security_approval_project_rule.name)
expect(rule.approval_project_rule).to eq(security_approval_project_rule)
expect(rule.report_type).to eq 'vulnerability'
expect(rule.name).to eq(vulnerability_approval_project_rule.name)
expect(rule.approval_project_rule).to eq(vulnerability_approval_project_rule)
end
it 'updates previous rules if defined' do
mr_rule = create(:report_approver_rule, merge_request: merge_request, approvals_required: 0)
expect { service.execute }
.not_to change { merge_request.approval_rules.security_report.count }
.not_to change { merge_request.approval_rules.vulnerability_report.count }
expect(mr_rule.reload).to be_report_approver
expect(mr_rule.report_type).to eq 'security'
expect(mr_rule.name).to eq(security_approval_project_rule.name)
expect(mr_rule.approvals_required).to eq security_approval_project_rule.approvals_required
expect(mr_rule.approval_project_rule).to eq(security_approval_project_rule)
expect(mr_rule.report_type).to eq 'vulnerability'
expect(mr_rule.name).to eq(vulnerability_approval_project_rule.name)
expect(mr_rule.approvals_required).to eq vulnerability_approval_project_rule.approvals_required
expect(mr_rule.approval_project_rule).to eq(vulnerability_approval_project_rule)
end
end
end
......@@ -76,11 +76,11 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do
end
context "when a project has multiple report approval rules" do
let!(:vulnerability_project_rule) { create(:approval_project_rule, :security_report, project: merge_request.target_project) }
let!(:vulnerability_project_rule) { create(:approval_project_rule, :vulnerability_report, project: merge_request.target_project) }
let!(:license_compliance_project_rule) { create(:approval_project_rule, :license_scanning, project: merge_request.target_project) }
context "when none of the rules have been synchronized to the merge request yet" do
let(:vulnerability_check_rule) { merge_request.reload.approval_rules.security.last }
let(:vulnerability_check_rule) { merge_request.reload.approval_rules.vulnerability_report.last }
let(:license_check_rule) { merge_request.reload.approval_rules.find_by(name: ApprovalProjectRule::DEFAULT_NAME_FOR_LICENSE_REPORT) }
before do
......@@ -95,7 +95,7 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do
specify { expect(merge_request.reload.approval_rules.count).to be(2) }
specify { expect(vulnerability_check_rule).to be_report_approver }
specify { expect(vulnerability_check_rule.approvals_required).to eql(vulnerability_project_rule.approvals_required) }
specify { expect(vulnerability_check_rule).to be_security }
specify { expect(vulnerability_check_rule).to be_vulnerability }
specify { expect(vulnerability_check_rule.name).to eq(vulnerability_project_rule.name) }
specify { expect(vulnerability_check_rule.approval_project_rule).to eq(vulnerability_project_rule) }
specify { expect(license_check_rule).to be_report_approver }
......@@ -113,7 +113,7 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do
end
specify { expect(merge_request.reload.approval_rules.count).to be(2) }
specify { expect(merge_request.reload.approval_rules.security_report.count).to be(1) }
specify { expect(merge_request.reload.approval_rules.vulnerability_report.count).to be(1) }
specify { expect(merge_request.reload.approval_rules.where(report_type: :license_scanning)).to match_array([previous_rule]) }
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment