diff --git a/app/models/concerns/cron_schedulable.rb b/app/models/concerns/cron_schedulable.rb index 48605ecc3d723160a9223d3dc3abb2aa530d8a96..d5b86db2640da6fd5257ebf75c00dc426d667703 100644 --- a/app/models/concerns/cron_schedulable.rb +++ b/app/models/concerns/cron_schedulable.rb @@ -14,12 +14,10 @@ def set_next_run_at # 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) diff --git a/app/validators/json_schemas/dast_profile_schedule_cadence.json b/app/validators/json_schemas/dast_profile_schedule_cadence.json new file mode 100644 index 0000000000000000000000000000000000000000..5583acfa5af5f7fe2c0b68c3f7bad20b0efec849 --- /dev/null +++ b/app/validators/json_schemas/dast_profile_schedule_cadence.json @@ -0,0 +1,31 @@ +{ + "$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] } + } + } + ] +} diff --git a/db/migrate/20210807101446_add_cadence_to_dast_profile_schedules.rb b/db/migrate/20210807101446_add_cadence_to_dast_profile_schedules.rb new file mode 100644 index 0000000000000000000000000000000000000000..c9b17e3d5c59d8da893dd06cd7943d6339745334 --- /dev/null +++ b/db/migrate/20210807101446_add_cadence_to_dast_profile_schedules.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddCadenceToDastProfileSchedules < ActiveRecord::Migration[6.1] + def change + add_column :dast_profile_schedules, :cadence, :jsonb, null: false, default: {} + end +end diff --git a/db/migrate/20210807101621_add_timezone_to_dast_profile_schedules.rb b/db/migrate/20210807101621_add_timezone_to_dast_profile_schedules.rb new file mode 100644 index 0000000000000000000000000000000000000000..3c3eb50743244d09e4c633c6770a41341e57d8ba --- /dev/null +++ b/db/migrate/20210807101621_add_timezone_to_dast_profile_schedules.rb @@ -0,0 +1,26 @@ +# 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 diff --git a/db/migrate/20210807102004_add_starts_at_to_dast_profile_schedules.rb b/db/migrate/20210807102004_add_starts_at_to_dast_profile_schedules.rb new file mode 100644 index 0000000000000000000000000000000000000000..4eea5fd7e8cc5a61c7a1d1527f8e2886afc62675 --- /dev/null +++ b/db/migrate/20210807102004_add_starts_at_to_dast_profile_schedules.rb @@ -0,0 +1,7 @@ +# 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 diff --git a/db/migrate/20210816095826_add_unique_index_on_dast_profile_to_dast_profile_schedules.rb b/db/migrate/20210816095826_add_unique_index_on_dast_profile_to_dast_profile_schedules.rb new file mode 100644 index 0000000000000000000000000000000000000000..b7ea8545df184702d9859266e60c2dc1992fc51a --- /dev/null +++ b/db/migrate/20210816095826_add_unique_index_on_dast_profile_to_dast_profile_schedules.rb @@ -0,0 +1,38 @@ +# 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 diff --git a/db/migrate/20210818061156_remove_project_profile_compound_index_from_dast_profile_schedules.rb b/db/migrate/20210818061156_remove_project_profile_compound_index_from_dast_profile_schedules.rb new file mode 100644 index 0000000000000000000000000000000000000000..b50947a0a999428ab10008f2f3f14dd54268535a --- /dev/null +++ b/db/migrate/20210818061156_remove_project_profile_compound_index_from_dast_profile_schedules.rb @@ -0,0 +1,30 @@ +# 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 diff --git a/db/migrate/20210818115613_add_index_project_id_on_dast_profile_schedule.rb b/db/migrate/20210818115613_add_index_project_id_on_dast_profile_schedule.rb new file mode 100644 index 0000000000000000000000000000000000000000..392b335ab451c5e1377e75ef15c8c625b25ee0c0 --- /dev/null +++ b/db/migrate/20210818115613_add_index_project_id_on_dast_profile_schedule.rb @@ -0,0 +1,13 @@ +# 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 diff --git a/db/schema_migrations/20210807101446 b/db/schema_migrations/20210807101446 new file mode 100644 index 0000000000000000000000000000000000000000..0b6d526429fd8bf663e1f7d498bced661db17785 --- /dev/null +++ b/db/schema_migrations/20210807101446 @@ -0,0 +1 @@ +30e1463616c60b92afb28bbb76e3c55830a385af6df0e60e16ed96d9e75943b9 \ No newline at end of file diff --git a/db/schema_migrations/20210807101621 b/db/schema_migrations/20210807101621 new file mode 100644 index 0000000000000000000000000000000000000000..ab053cf4cbc427556e80f17bc60ac48a602579ef --- /dev/null +++ b/db/schema_migrations/20210807101621 @@ -0,0 +1 @@ +7e9b39914ade766357751953a4981225dbae7e5d371d4824af61b01af70f46ae \ No newline at end of file diff --git a/db/schema_migrations/20210807102004 b/db/schema_migrations/20210807102004 new file mode 100644 index 0000000000000000000000000000000000000000..e63485435f822a846f7dd16fd8e61f1caa64d527 --- /dev/null +++ b/db/schema_migrations/20210807102004 @@ -0,0 +1 @@ +a2454f9fca3b1cedf7a0f2288b69abe799fe1f9ff4e2fe26d2cadfdddea73a83 \ No newline at end of file diff --git a/db/schema_migrations/20210816095826 b/db/schema_migrations/20210816095826 new file mode 100644 index 0000000000000000000000000000000000000000..079a83fae8f538ddfe6c2710a3feb328a931b5cb --- /dev/null +++ b/db/schema_migrations/20210816095826 @@ -0,0 +1 @@ +d1ad234656f49861d2ca7694d23116e930bba597fca32b1015db698cc23bdc1c \ No newline at end of file diff --git a/db/schema_migrations/20210818061156 b/db/schema_migrations/20210818061156 new file mode 100644 index 0000000000000000000000000000000000000000..fba5486b2a8ac9340d6b6a93d40aabdfb28f4564 --- /dev/null +++ b/db/schema_migrations/20210818061156 @@ -0,0 +1 @@ +23becdc9ad558882f4ce42e76391cdc2f760322a09c998082465fcb6d29dfeb5 \ No newline at end of file diff --git a/db/schema_migrations/20210818115613 b/db/schema_migrations/20210818115613 new file mode 100644 index 0000000000000000000000000000000000000000..efe76d3a46a2c776f1196c0426379a58ab5f4093 --- /dev/null +++ b/db/schema_migrations/20210818115613 @@ -0,0 +1 @@ +9c5114dac05e90c15567bb3274f20f03a82f9e4d73d5c72d89c26bc9d742cc35 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 870277556bb34f4c2e16783525a214e993a25b5a..0b1a0c1b33e9c86bbe3c8d5d22fc9a34c3dc8eaa 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -12064,7 +12064,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"}'; @@ -23612,9 +23616,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); diff --git a/ee/app/models/dast/profile.rb b/ee/app/models/dast/profile.rb index d79854c655424d3a04f39a6c4735f02858eed072..542f71bcc164b9981f918c7a6992f463b2357431 100644 --- a/ee/app/models/dast/profile.rb +++ b/ee/app/models/dast/profile.rb @@ -10,7 +10,7 @@ class Profile < ApplicationRecord 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 diff --git a/ee/app/models/dast/profile_schedule.rb b/ee/app/models/dast/profile_schedule.rb index debe43ed2bcdc500799915dc7d47312ed7abec97..25b23ae5ead7db82c940c791ef28ba0d02ab0bff 100644 --- a/ee/app/models/dast/profile_schedule.rb +++ b/ee/app/models/dast/profile_schedule.rb @@ -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 diff --git a/ee/spec/factories/dast/profile_schedules.rb b/ee/spec/factories/dast/profile_schedules.rb index 6098f733285192222c75e29d40ca61e120ce48d7..d00f03fc308263cc45893846bcf69418ce15613a 100644 --- a/ee/spec/factories/dast/profile_schedules.rb +++ b/ee/spec/factories/dast/profile_schedules.rb @@ -5,7 +5,8 @@ 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 diff --git a/ee/spec/models/dast/profile_schedule_spec.rb b/ee/spec/models/dast/profile_schedule_spec.rb index 0f7107136ca81ed0ec427017556c11e7499f08d3..a0562c3810c1107f2264b4e75be97e7341c9f83e 100644 --- a/ee/spec/models/dast/profile_schedule_spec.rb +++ b/ee/spec/models/dast/profile_schedule_spec.rb @@ -7,14 +7,52 @@ 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 @@ 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 @@ 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 diff --git a/ee/spec/models/dast/profile_spec.rb b/ee/spec/models/dast/profile_spec.rb index 12358f55d9caf2223be4d3a2b1759a38391552a3..a76216f431f5490190a55c92d7818a73d5d0c166 100644 --- a/ee/spec/models/dast/profile_spec.rb +++ b/ee/spec/models/dast/profile_spec.rb @@ -12,7 +12,7 @@ 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 diff --git a/ee/spec/workers/app_sec/dast/profile_schedule_worker_spec.rb b/ee/spec/workers/app_sec/dast/profile_schedule_worker_spec.rb index 4b0e999d558b2bf6e96448e18a478fa359d467d7..84fb16c477f6448663c8ec76991f5682ddac196e 100644 --- a/ee/spec/workers/app_sec/dast/profile_schedule_worker_spec.rb +++ b/ee/spec/workers/app_sec/dast/profile_schedule_worker_spec.rb @@ -101,5 +101,19 @@ def record_preloaded_queries 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 diff --git a/lib/gitlab/ci/cron_parser.rb b/lib/gitlab/ci/cron_parser.rb index bc03658aab8614c65b777fb587aa2e2aebd5915e..7334a112ccf2358de169fe8a15a505552d0fdc7d 100644 --- a/lib/gitlab/ci/cron_parser.rb +++ b/lib/gitlab/ci/cron_parser.rb @@ -6,8 +6,40 @@ class CronParser 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') diff --git a/spec/lib/gitlab/ci/cron_parser_spec.rb b/spec/lib/gitlab/ci/cron_parser_spec.rb index 15293429354c0e374e16fd7eb5ba6af0f4c49fa1..4017accb4620692686f8ae1706f56b0c4c8b528e 100644 --- a/spec/lib/gitlab/ci/cron_parser_spec.rb +++ b/spec/lib/gitlab/ci/cron_parser_spec.rb @@ -297,4 +297,65 @@ 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