Commit dda67bb3 authored by Alex Kalderimis's avatar Alex Kalderimis

Merge branch 'sk/337537-get-branches-from-rule-index' into 'master'

Get branches from rules using rule_index for security policy

See merge request gitlab-org/gitlab!67607
parents 65e906db f8ea455f
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
module Security module Security
class OrchestrationPolicyRuleSchedule < ApplicationRecord class OrchestrationPolicyRuleSchedule < ApplicationRecord
include CronSchedulable include CronSchedulable
include Gitlab::Utils::StrongMemoize
self.table_name = 'security_orchestration_policy_rule_schedules' self.table_name = 'security_orchestration_policy_rule_schedules'
...@@ -26,7 +27,20 @@ module Security ...@@ -26,7 +27,20 @@ module Security
end end
def policy def policy
security_orchestration_policy_configuration.active_policies.at(policy_index) strong_memoize(:policy) do
security_orchestration_policy_configuration.active_policies.at(policy_index)
end
end
def applicable_branches
configured_branches = policy&.dig(:rules, rule_index, :branches)
return [] if configured_branches.blank?
branch_names = security_orchestration_policy_configuration.project.repository.branches
configured_branches
.flat_map { |pattern| RefMatcher.new(pattern).matching(branch_names).map(&:name) }
.uniq
end end
private private
......
...@@ -22,13 +22,14 @@ module Security ...@@ -22,13 +22,14 @@ module Security
def create_new_schedule_rules def create_new_schedule_rules
return unless policy_configuration.enabled? return unless policy_configuration.enabled?
policy[:rules] policy[:rules].each_with_index do |rule, rule_index|
.select { |rule| rule[:type] == Security::OrchestrationPolicyConfiguration::RULE_TYPES[:schedule] } next if rule[:type] != Security::OrchestrationPolicyConfiguration::RULE_TYPES[:schedule]
.each do |rule|
Security::OrchestrationPolicyRuleSchedule Security::OrchestrationPolicyRuleSchedule
.create!( .create!(
security_orchestration_policy_configuration: policy_configuration, security_orchestration_policy_configuration: policy_configuration,
policy_index: policy_index, policy_index: policy_index,
rule_index: rule_index,
cron: rule[:cadence], cron: rule[:cadence],
owner: policy_configuration.policy_last_updated_by owner: policy_configuration.policy_last_updated_by
) )
......
...@@ -5,8 +5,10 @@ module Security ...@@ -5,8 +5,10 @@ module Security
class RuleScheduleService < BaseContainerService class RuleScheduleService < BaseContainerService
def execute(schedule) def execute(schedule)
schedule.schedule_next_run! schedule.schedule_next_run!
branches = schedule.applicable_branches
actions_for(schedule) actions_for(schedule)
.each { |action| process_action(action) } .each { |action| process_action(action, branches) }
end end
private private
...@@ -18,24 +20,27 @@ module Security ...@@ -18,24 +20,27 @@ module Security
policy[:actions] policy[:actions]
end end
def process_action(action) def process_action(action, branches)
case action[:scan] case action[:scan]
when 'dast' then schedule_dast_on_demand_scan(action) when 'dast' then schedule_dast_on_demand_scan(action, branches)
end end
end end
def schedule_dast_on_demand_scan(action) def schedule_dast_on_demand_scan(action, branches)
dast_site_profile = find_dast_site_profile(container, action[:site_profile]) dast_site_profile = find_dast_site_profile(container, action[:site_profile])
dast_scanner_profile = find_dast_scanner_profile(container, action[:scanner_profile]) dast_scanner_profile = find_dast_scanner_profile(container, action[:scanner_profile])
::DastOnDemandScans::CreateService.new( branches.each do |branch|
container: container, ::DastOnDemandScans::CreateService.new(
current_user: current_user, container: container,
params: { current_user: current_user,
dast_site_profile: dast_site_profile, params: {
dast_scanner_profile: dast_scanner_profile branch: branch,
} dast_site_profile: dast_site_profile,
).execute dast_scanner_profile: dast_scanner_profile
}
).execute
end
end end
def find_dast_site_profile(project, dast_site_profile) def find_dast_site_profile(project, dast_site_profile)
......
...@@ -105,6 +105,83 @@ RSpec.describe Security::OrchestrationPolicyRuleSchedule do ...@@ -105,6 +105,83 @@ RSpec.describe Security::OrchestrationPolicyRuleSchedule do
end end
end end
describe '#applicable_branches' do
let_it_be_with_reload(:project) { create(:project, :repository) }
let_it_be_with_reload(:policy_configuration) { create(:security_orchestration_policy_configuration, project: project) }
let_it_be_with_reload(:rule_schedule) do
create(:security_orchestration_policy_rule_schedule, security_orchestration_policy_configuration: policy_configuration)
end
let(:branches) { ['production'] }
let(:policy) do
{
name: 'Scheduled DAST 1',
description: 'This policy runs DAST every 20 mins',
enabled: true,
rules: [{ type: 'schedule', branches: branches, cadence: '*/20 * * * *' }],
actions: [
{ scan: 'dast', site_profile: 'Site Profile', scanner_profile: 'Scanner Profile' }
]
}
end
subject { rule_schedule.applicable_branches }
before do
allow(rule_schedule).to receive(:policy).and_return(policy)
end
context 'when branches does not exist' do
let(:branches) { ['production'] }
it { is_expected.to be_empty }
end
context 'when branches is empty' do
let(:branches) { [] }
it { is_expected.to be_empty }
end
context 'when some of the branches exists' do
let(:branches) { %w[feature-a feature-b] }
before do
project.repository.create_branch('feature-a', project.default_branch)
project.repository.create_branch('x-feature', project.default_branch)
end
it { is_expected.to eq(%w[feature-a]) }
end
context 'when branches with wildcards matches' do
let(:branches) { ['feature-*'] }
before do
project.repository.create_branch('feature-a', project.default_branch)
project.repository.create_branch('feature-b', project.default_branch)
project.repository.create_branch('x-feature', project.default_branch)
end
it { is_expected.to eq(%w[feature-a feature-b]) }
end
context 'when policy is not present' do
let(:policy) { nil }
it { is_expected.to be_empty }
end
context 'when policy rules are not present' do
before do
policy[:rules] = []
end
it { is_expected.to be_empty }
end
end
describe '#set_next_run_at' do describe '#set_next_run_at' do
it_behaves_like 'handles set_next_run_at' do it_behaves_like 'handles set_next_run_at' do
let(:schedule) { create(:security_orchestration_policy_rule_schedule, cron: '*/1 * * * *') } let(:schedule) { create(:security_orchestration_policy_rule_schedule, cron: '*/1 * * * *') }
......
...@@ -14,10 +14,13 @@ RSpec.describe Security::SecurityOrchestrationPolicies::ProcessRuleService do ...@@ -14,10 +14,13 @@ RSpec.describe Security::SecurityOrchestrationPolicies::ProcessRuleService do
let(:policy) do let(:policy) do
{ {
name: 'Scheduled DAST', name: 'DAST Scan',
description: 'This policy runs DAST for every 15 mins', description: 'This policy runs DAST on pipeline and for every 15 mins',
enabled: true, enabled: true,
rules: [{ type: 'schedule', branches: %w[production], cadence: '*/15 * * * *' }], rules: [
{ type: 'pipeline', branches: %w[production] },
{ type: 'schedule', branches: %w[production], cadence: '*/15 * * * *' }
],
actions: [ actions: [
{ scan: 'dast', site_profile: 'Site Profile', scanner_profile: 'Scanner Profile' } { scan: 'dast', site_profile: 'Site Profile', scanner_profile: 'Scanner Profile' }
] ]
...@@ -38,6 +41,7 @@ RSpec.describe Security::SecurityOrchestrationPolicies::ProcessRuleService do ...@@ -38,6 +41,7 @@ RSpec.describe Security::SecurityOrchestrationPolicies::ProcessRuleService do
expect(policy_configuration.configured_at).not_to be_nil expect(policy_configuration.configured_at).not_to be_nil
expect(Security::OrchestrationPolicyRuleSchedule.count).to eq(1) expect(Security::OrchestrationPolicyRuleSchedule.count).to eq(1)
expect(new_schedule.id).not_to eq(schedule.id) expect(new_schedule.id).not_to eq(schedule.id)
expect(new_schedule.rule_index).to eq(1)
expect(new_schedule.next_run_at).to be > schedule.next_run_at expect(new_schedule.next_run_at).to be > schedule.next_run_at
end end
end end
......
...@@ -15,7 +15,7 @@ RSpec.describe Security::SecurityOrchestrationPolicies::RuleScheduleService do ...@@ -15,7 +15,7 @@ RSpec.describe Security::SecurityOrchestrationPolicies::RuleScheduleService do
name: 'Run DAST in every pipeline', name: 'Run DAST in every pipeline',
description: 'This policy enforces to run DAST for every pipeline within the project', description: 'This policy enforces to run DAST for every pipeline within the project',
enabled: true, enabled: true,
rules: [{ type: 'schedule', branches: %w[production], cadence: '*/20 * * * *' }], rules: [{ type: 'schedule', branches: %w[master production], cadence: '*/20 * * * *' }],
actions: [ actions: [
{ scan: 'dast', site_profile: 'Site Profile', scanner_profile: 'Scanner Profile' } { scan: 'dast', site_profile: 'Site Profile', scanner_profile: 'Scanner Profile' }
] ]
...@@ -35,19 +35,35 @@ RSpec.describe Security::SecurityOrchestrationPolicies::RuleScheduleService do ...@@ -35,19 +35,35 @@ RSpec.describe Security::SecurityOrchestrationPolicies::RuleScheduleService do
before do before do
stub_licensed_features(security_on_demand_scans: true) stub_licensed_features(security_on_demand_scans: true)
project.repository.create_branch('production', project.default_branch)
allow_next_instance_of(Security::OrchestrationPolicyConfiguration) do |instance| allow_next_instance_of(Security::OrchestrationPolicyConfiguration) do |instance|
allow(instance).to receive(:active_policies).and_return([policy]) allow(instance).to receive(:active_policies).and_return([policy])
end end
end end
context 'when policy actions exists' do context 'when policy actions exists and there are multiple matching branches' do
it 'creates a DAST on demand-scan pipeline and updates next_run_at' do it 'creates multiple DAST on demand-scan pipelines and updates next_run_at' do
expect { service.execute(schedule) }.to change(Ci::Pipeline, :count).by(1) expect { service.execute(schedule) }.to change(Ci::Pipeline, :count).by(2)
expect(schedule.next_run_at).to be > Time.zone.now expect(schedule.next_run_at).to be > Time.zone.now
end end
end end
context 'when the branch in rules does not exist' do
let(:policy) do
{
name: 'Run DAST in every pipeline',
description: 'This policy enforces to run DAST for every pipeline within the project',
enabled: true,
rules: [{ type: 'schedule', branches: %w[invalid_branch], cadence: '*/20 * * * *' }],
actions: []
}
end
it_behaves_like 'does not execute DAST on demand-scan'
end
context 'when policy actions does not exist' do context 'when policy actions does not exist' do
let(:policy) do let(:policy) do
{ {
......
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