Commit 36c681d1 authored by Aditya Tiwari's avatar Aditya Tiwari Committed by Bob Van Landuyt

Implements Scheduling logic for on demand DAST

parent 9405fc7a
......@@ -14,12 +14,10 @@ module CronSchedulable
# The `next_run_at` column is set to the actual execution date of worker that
# triggers the schedule. This way, a schedule like `*/1 * * * *` won't be triggered
# in a short interval when the worker runs irregularly by Sidekiq Memory Killer.
def calculate_next_run_at
now = Time.zone.now
def calculate_next_run_at(start_time = Time.zone.now)
ideal_next_run = ideal_next_run_from(start_time)
ideal_next_run = ideal_next_run_from(now)
if ideal_next_run == cron_worker_next_run_from(now)
if ideal_next_run == cron_worker_next_run_from(start_time)
ideal_next_run
else
cron_worker_next_run_from(ideal_next_run)
......
{
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Dast profile schedule cadence schema",
"type": "object",
"anyOf": [
{
"properties": {
"unit": { "enum": ["day"] },
"duration": { "enum": [1] }
}
},
{
"properties": {
"unit": { "enum": ["week"] },
"duration": { "enum": [1] }
}
},
{
"properties": {
"unit": { "enum": ["month"] },
"duration": { "enum": [1, 3 ,6] }
}
},
{
"properties": {
"unit": { "enum": ["year"] },
"duration": { "enum": [1] }
}
}
]
}
# frozen_string_literal: true
class AddCadenceToDastProfileSchedules < ActiveRecord::Migration[6.1]
def change
add_column :dast_profile_schedules, :cadence, :jsonb, null: false, default: {}
end
end
# frozen_string_literal: true
class AddTimezoneToDastProfileSchedules < ActiveRecord::Migration[6.1]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
# We disable these cops here because adding the column is safe. The table does not
# have any data in it as it's behind a feature flag.
# rubocop: disable Rails/NotNullColumn
def up
execute('DELETE FROM dast_profile_schedules')
unless column_exists?(:dast_profile_schedules, :timezone)
add_column :dast_profile_schedules, :timezone, :text, null: false
end
add_text_limit :dast_profile_schedules, :timezone, 255
end
def down
return unless column_exists?(:dast_profile_schedules, :timezone)
remove_column :dast_profile_schedules, :timezone
end
end
# frozen_string_literal: true
class AddStartsAtToDastProfileSchedules < ActiveRecord::Migration[6.1]
def change
add_column :dast_profile_schedules, :starts_at, :datetime_with_timezone, null: false, default: -> { 'NOW()' }
end
end
# 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 AddUniqueIndexOnDastProfileToDastProfileSchedules < ActiveRecord::Migration[6.1]
include Gitlab::Database::MigrationHelpers
INDEX_NAME = 'index_dast_profile_schedules_on_dast_profile_id'
TABLE = :dast_profile_schedules
# We disable these cops here because changing this index is safe. The table does not
# have any data in it as it's behind a feature flag.
# rubocop: disable Migration/AddIndex
# rubocop: disable Migration/RemoveIndex
def up
execute('DELETE FROM dast_profile_schedules')
if index_exists_by_name?(TABLE, INDEX_NAME)
remove_index TABLE, :dast_profile_id, name: INDEX_NAME
end
unless index_exists_by_name?(TABLE, INDEX_NAME)
add_index TABLE, :dast_profile_id, unique: true, name: INDEX_NAME
end
end
def down
execute('DELETE FROM dast_profile_schedules')
if index_exists_by_name?(TABLE, INDEX_NAME)
remove_index TABLE, :dast_profile_id, name: INDEX_NAME
end
unless index_exists_by_name?(TABLE, INDEX_NAME)
add_index TABLE, :dast_profile_id
end
end
end
# 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 RemoveProjectProfileCompoundIndexFromDastProfileSchedules < ActiveRecord::Migration[6.1]
include Gitlab::Database::MigrationHelpers
TABLE = :dast_profile_schedules
INDEX_NAME = 'index_dast_profile_schedules_on_project_id_and_dast_profile_id'
# We disable these cops here because changing this index is safe. The table does not
# have any data in it as it's behind a feature flag.
# rubocop: disable Migration/AddIndex
# rubocop: disable Migration/RemoveIndex
def up
execute('DELETE FROM dast_profile_schedules')
if index_exists_by_name?(TABLE, INDEX_NAME)
remove_index TABLE, %i[project_id dast_profile_id], name: INDEX_NAME
end
end
def down
execute('DELETE FROM dast_profile_schedules')
unless index_exists_by_name?(TABLE, INDEX_NAME)
add_index TABLE, %i[project_id dast_profile_id], unique: true, name: INDEX_NAME
end
end
end
# 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 AddIndexProjectIdOnDastProfileSchedule < ActiveRecord::Migration[6.1]
# We disable these cops here because changing this index is safe. The table does not
# have any data in it as it's behind a feature flag.
# rubocop: disable Migration/AddIndex
def change
add_index :dast_profile_schedules, :project_id
end
end
30e1463616c60b92afb28bbb76e3c55830a385af6df0e60e16ed96d9e75943b9
\ No newline at end of file
7e9b39914ade766357751953a4981225dbae7e5d371d4824af61b01af70f46ae
\ No newline at end of file
a2454f9fca3b1cedf7a0f2288b69abe799fe1f9ff4e2fe26d2cadfdddea73a83
\ No newline at end of file
d1ad234656f49861d2ca7694d23116e930bba597fca32b1015db698cc23bdc1c
\ No newline at end of file
23becdc9ad558882f4ce42e76391cdc2f760322a09c998082465fcb6d29dfeb5
\ No newline at end of file
9c5114dac05e90c15567bb3274f20f03a82f9e4d73d5c72d89c26bc9d742cc35
\ No newline at end of file
......@@ -12089,7 +12089,11 @@ CREATE TABLE dast_profile_schedules (
updated_at timestamp with time zone NOT NULL,
active boolean DEFAULT true NOT NULL,
cron text NOT NULL,
CONSTRAINT check_86531ea73f CHECK ((char_length(cron) <= 255))
cadence jsonb DEFAULT '{}'::jsonb NOT NULL,
timezone text NOT NULL,
starts_at timestamp with time zone DEFAULT now() NOT NULL,
CONSTRAINT check_86531ea73f CHECK ((char_length(cron) <= 255)),
CONSTRAINT check_be4d1c3af1 CHECK ((char_length(timezone) <= 255))
);
COMMENT ON TABLE dast_profile_schedules IS '{"owner":"group::dynamic analysis","description":"Scheduling for scans using DAST Profiles"}';
......@@ -23812,9 +23816,9 @@ CREATE UNIQUE INDEX index_daily_build_group_report_results_unique_columns ON ci_
CREATE INDEX index_dast_profile_schedules_active_next_run_at ON dast_profile_schedules USING btree (active, next_run_at);
CREATE INDEX index_dast_profile_schedules_on_dast_profile_id ON dast_profile_schedules USING btree (dast_profile_id);
CREATE UNIQUE INDEX index_dast_profile_schedules_on_dast_profile_id ON dast_profile_schedules USING btree (dast_profile_id);
CREATE UNIQUE INDEX index_dast_profile_schedules_on_project_id_and_dast_profile_id ON dast_profile_schedules USING btree (project_id, dast_profile_id);
CREATE INDEX index_dast_profile_schedules_on_project_id ON dast_profile_schedules USING btree (project_id);
CREATE INDEX index_dast_profile_schedules_on_user_id ON dast_profile_schedules USING btree (user_id);
......@@ -10,7 +10,7 @@ module Dast
has_many :secret_variables, through: :dast_site_profile, class_name: 'Dast::SiteProfileSecretVariable'
has_many :dast_profile_schedules, class_name: 'Dast::ProfileSchedule', foreign_key: :dast_profile_id, inverse_of: :dast_profile
has_one :dast_profile_schedule, class_name: 'Dast::ProfileSchedule', foreign_key: :dast_profile_id, inverse_of: :dast_profile
validates :description, length: { maximum: 255 }
validates :name, length: { maximum: 255 }, uniqueness: { scope: :project_id }, presence: true
......
......@@ -3,27 +3,72 @@
class Dast::ProfileSchedule < ApplicationRecord
include CronSchedulable
CRON_DEFAULT = '* * * * *'
self.table_name = 'dast_profile_schedules'
belongs_to :project
belongs_to :dast_profile, class_name: 'Dast::Profile', optional: false, inverse_of: :dast_profile_schedules
belongs_to :dast_profile, class_name: 'Dast::Profile', optional: false, inverse_of: :dast_profile_schedule
belongs_to :owner, class_name: 'User', optional: true, foreign_key: :user_id
validates :cron, presence: true
validates :next_run_at, presence: true
validates :timezone, presence: true, inclusion: { in: :timezones }
validates :starts_at, presence: true
validates :cadence, json_schema: { filename: 'dast_profile_schedule_cadence', draft: 7 }
validates :dast_profile_id, uniqueness: true
serialize :cadence, Serializers::Json # rubocop:disable Cop/ActiveRecordSerialize
scope :with_project, -> { includes(:project) }
scope :with_profile, -> { includes(dast_profile: [:dast_site_profile, :dast_scanner_profile]) }
scope :with_owner, -> { includes(:owner) }
scope :active, -> { where(active: true) }
before_save :set_cron, :set_next_run_at
def repeat?
cadence.present?
end
def schedule_next_run!
return deactivate! unless repeat?
super
end
def audit_details
owner&.name
end
private
def deactivate!
update!(active: false)
end
def cron_timezone
next_run_at.zone
Time.zone.name
end
def set_cron
self.cron =
if repeat?
Gitlab::Ci::CronParser.parse_natural_with_timestamp(starts_at, cadence)
else
CRON_DEFAULT
end
end
def set_next_run_at
return super unless will_save_change_to_starts_at?
self.next_run_at = cron_worker_next_run_from(starts_at)
end
def worker_cron_expression
Settings.cron_jobs['app_sec_dast_profile_schedule_worker']['cron']
end
def timezones
@timezones ||= ActiveSupport::TimeZone.all.map { |tz| tz.tzinfo.identifier }
end
end
......@@ -5,7 +5,8 @@ FactoryBot.define do
project
dast_profile
owner { association(:user) }
cron { '*/10 * * * *' }
next_run_at { Time.now }
timezone { FFaker::Address.time_zone }
starts_at { Time.now }
cadence { { unit: %w(day month year week).sample, duration: 1 } }
end
end
......@@ -7,14 +7,52 @@ RSpec.describe Dast::ProfileSchedule, type: :model do
describe 'associations' do
it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:dast_profile).class_name('Dast::Profile').required.inverse_of(:dast_profile_schedules) }
it { is_expected.to belong_to(:dast_profile).class_name('Dast::Profile').required.inverse_of(:dast_profile_schedule) }
it { is_expected.to belong_to(:owner).class_name('User').with_foreign_key(:user_id) }
end
describe 'validations' do
let(:timezones) { ActiveSupport::TimeZone.all.map { |tz| tz.tzinfo.identifier } }
it { is_expected.to be_valid }
it { is_expected.to validate_presence_of(:cron) }
it { is_expected.to validate_presence_of(:next_run_at) }
it { is_expected.to validate_presence_of(:timezone) }
it { is_expected.to validate_inclusion_of(:timezone).in_array(timezones) }
it { is_expected.to validate_presence_of(:starts_at) }
it { is_expected.to validate_uniqueness_of(:dast_profile_id) }
describe 'cadence' do
context 'when valid values' do
[
{ unit: 'day', duration: 1 },
{ unit: 'week', duration: 1 },
{ unit: 'month', duration: 1 },
{ unit: 'month', duration: 3 },
{ unit: 'month', duration: 6 },
{ unit: 'year', duration: 1 },
{}
].each do |cadence|
it "allows #{cadence[:unit]} values" do
schedule = build(:dast_profile_schedule, cadence: cadence)
expect(schedule).to be_valid
expect(schedule.cadence).to eq(cadence.stringify_keys)
end
end
end
context 'when invalid values' do
[
{ unit: 'day', duration: 3 },
{ unit: 'month_foo', duration: 100 }
].each do |cadence|
it "disallow #{cadence[:unit]} values" do
expect { build(:dast_profile_schedule, cadence: cadence).validate! }.to raise_error(ActiveRecord::RecordInvalid) do |err|
expect(err.record.errors.full_messages).to include('Cadence must be a valid json schema')
end
end
end
end
end
end
describe 'scopes' do
......@@ -31,13 +69,13 @@ RSpec.describe Dast::ProfileSchedule, type: :model do
end
end
describe 'runnable_schedules' do
describe '.runnable_schedules' do
subject { described_class.runnable_schedules }
context 'when there are runnable schedules' do
let!(:profile_schedule) do
travel_to(1.day.ago) do
create(:dast_profile_schedule)
travel_to(2.days.ago) do
create(:dast_profile_schedule, cadence: { unit: 'day', duration: 1 })
end
end
......@@ -80,15 +118,73 @@ RSpec.describe Dast::ProfileSchedule, type: :model do
end
end
describe 'before_save' do
describe '#set_cron' do
context 'when repeat? is true' do
it 'sets the cron value' do
freeze_time do
cron_statement = Gitlab::Ci::CronParser.parse_natural_with_timestamp(subject.starts_at, subject.cadence)
expect(subject.cron).to eq cron_statement
end
end
end
context 'when repeat? is false' do
subject { create(:dast_profile_schedule, cadence: {}) }
it 'sets the cron value to default when non repeating' do
expect(subject.cron).to eq Dast::ProfileSchedule::CRON_DEFAULT
end
end
end
end
describe '#set_next_run_at' do
it_behaves_like 'handles set_next_run_at' do
let(:schedule) { create(:dast_profile_schedule, cron: '*/1 * * * *') }
let(:schedule_1) { create(:dast_profile_schedule) }
let(:schedule_2) { create(:dast_profile_schedule) }
let(:new_cron) { '0 0 1 1 *' }
let(:ideal_next_run_at) { schedule.send(:ideal_next_run_from, Time.zone.now) }
let(:cron_worker_next_run_at) { schedule.send(:cron_worker_next_run_from, Time.zone.now) }
let(:schedule) { create(:dast_profile_schedule, cadence: { unit: 'day', duration: 1 }, starts_at: Time.zone.now) }
let(:schedule_1) { create(:dast_profile_schedule, cadence: { unit: 'day', duration: 1 }) }
let(:schedule_2) { create(:dast_profile_schedule, cadence: { unit: 'day', duration: 1 }) }
let(:cron_worker_next_run_at) { schedule.send(:cron_worker_next_run_from, Time.zone.now) }
context 'when schedule runs every minute' do
it "updates next_run_at to the worker's execution time" do
travel_to(1.day.ago) do
expect(schedule.next_run_at.to_i).to eq(cron_worker_next_run_at.to_i)
end
end
end
context 'when there are two different schedules in the same time zones' do
it 'sets the sames next_run_at' do
expect(schedule_1.next_run_at.to_i).to eq(schedule_2.next_run_at.to_i)
end
end
context 'when starts_at is updated for existing schedules' do
it 'updates next_run_at automatically' do
expect { schedule.update!(starts_at: Time.zone.now + 2.days) }.to change { schedule.next_run_at }
end
end
end
describe '#schedule_next_run!' do
context 'when repeat? is true' do
it 'sets active to true' do
subject.schedule_next_run!
expect(subject.active).to be true
end
end
context 'when repeat? is false' do
it 'sets active to false' do
subject.update_column(:cadence, {})
subject.schedule_next_run!
expect(subject.active).to be false
end
end
end
end
......@@ -12,7 +12,7 @@ RSpec.describe Dast::Profile, type: :model do
it { is_expected.to belong_to(:dast_site_profile) }
it { is_expected.to belong_to(:dast_scanner_profile) }
it { is_expected.to have_many(:secret_variables).through(:dast_site_profile).class_name('Dast::SiteProfileSecretVariable') }
it { is_expected.to have_many(:dast_profile_schedules).class_name('Dast::ProfileSchedule').with_foreign_key(:dast_profile_id).inverse_of(:dast_profile) }
it { is_expected.to have_one(:dast_profile_schedule).class_name('Dast::ProfileSchedule').with_foreign_key(:dast_profile_id).inverse_of(:dast_profile) }
end
describe 'validations' do
......
......@@ -101,5 +101,19 @@ RSpec.describe AppSec::Dast::ProfileScheduleWorker do
subject
end
end
context 'when single run schedule exists' do
before do
schedule.update_columns(next_run_at: 1.minute.ago, cadence: {})
end
it 'executes the rule schedule service and deactivate the schedule', :aggregate_failures do
expect(schedule.repeat?).to be(false)
subject
expect(schedule.reload.active).to be(false)
end
end
end
end
......@@ -6,8 +6,40 @@ module Gitlab
VALID_SYNTAX_SAMPLE_TIME_ZONE = 'UTC'
VALID_SYNTAX_SAMPLE_CRON = '* * * * *'
def self.parse_natural(expression, cron_timezone = 'UTC')
new(Fugit::Nat.parse(expression)&.original, cron_timezone)
class << self
def parse_natural(expression, cron_timezone = 'UTC')
new(Fugit::Nat.parse(expression)&.original, cron_timezone)
end
# This method generates compatible expressions that can be
# parsed by Fugit::Nat.parse to generate a cron line.
# It takes start date of the cron and cadence in the following format:
# cadence = {
# unit: 'day/week/month/year'
# duration: 1
# }
def parse_natural_with_timestamp(starts_at, cadence)
case cadence[:unit]
when 'day' # Currently supports only 'every 1 day'.
"#{starts_at.min} #{starts_at.hour} * * *"
when 'week' # Currently supports only 'every 1 week'.
"#{starts_at.min} #{starts_at.hour} * * #{starts_at.wday}"
when 'month'
unless [1, 3, 6, 12].include?(cadence[:duration])
raise NotImplementedError, "The cadence #{cadence} is not supported"
end
"#{starts_at.min} #{starts_at.hour} #{starts_at.mday} #{fall_in_months(cadence[:duration], starts_at)} *"
when 'year' # Currently supports only 'every 1 year'.
"#{starts_at.min} #{starts_at.hour} #{starts_at.mday} #{starts_at.month} *"
else
raise NotImplementedError, "The cadence unit #{cadence[:unit]} is not implemented"
end
end
def fall_in_months(offset, start_date)
(1..(12 / offset)).map { |i| start_date.next_month(offset * i).month }.join(',')
end
end
def initialize(cron, cron_timezone = 'UTC')
......
......@@ -297,4 +297,65 @@ RSpec.describe Gitlab::Ci::CronParser do
it { is_expected.to eq(true) }
end
end
describe '.parse_natural', :aggregate_failures do
let(:cron_line) { described_class.parse_natural_with_timestamp(time, { unit: 'day', duration: 1 }) }
let(:time) { Time.parse('Mon, 30 Aug 2021 06:29:44.067132000 UTC +00:00') }
let(:hours) { Fugit::Cron.parse(cron_line).hours }
let(:minutes) { Fugit::Cron.parse(cron_line).minutes }
let(:weekdays) { Fugit::Cron.parse(cron_line).weekdays.first }
let(:months) { Fugit::Cron.parse(cron_line).months }
context 'when repeat cycle is day' do
it 'generates daily cron expression', :aggregate_failures do
expect(hours).to include time.hour
expect(minutes).to include time.min
end
end
context 'when repeat cycle is week' do
let(:cron_line) { described_class.parse_natural_with_timestamp(time, { unit: 'week', duration: 1 }) }
it 'generates weekly cron expression', :aggregate_failures do
expect(hours).to include time.hour
expect(minutes).to include time.min
expect(weekdays).to include time.wday
end
end
context 'when repeat cycle is month' do
let(:cron_line) { described_class.parse_natural_with_timestamp(time, { unit: 'month', duration: 3 }) }
it 'generates monthly cron expression', :aggregate_failures do
expect(minutes).to include time.min
expect(months).to include time.month
end
context 'when an unsupported duration is specified' do
subject { described_class.parse_natural_with_timestamp(time, { unit: 'month', duration: 7 }) }
it 'raises an exception' do
expect { subject }.to raise_error(NotImplementedError, 'The cadence {:unit=>"month", :duration=>7} is not supported')
end
end
end
context 'when repeat cycle is year' do
let(:cron_line) { described_class.parse_natural_with_timestamp(time, { unit: 'year', duration: 1 }) }
it 'generates yearly cron expression', :aggregate_failures do
expect(hours).to include time.hour
expect(minutes).to include time.min
expect(months).to include time.month
end
end
context 'when the repeat cycle is not implemented' do
subject { described_class.parse_natural_with_timestamp(time, { unit: 'quarterly', duration: 1 }) }
it 'raises an exception' do
expect { subject }.to raise_error(NotImplementedError, 'The cadence unit quarterly is not implemented')
end
end
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