Merge branch '24295-freeze-period-db-changes' into 'master'

Add Freeze Period Model

See merge request gitlab-org/gitlab!29162

app/models/ci/freeze_period.rb

+# frozen_string_literal: true
+
+module Ci
+  class FreezePeriod < ApplicationRecord
+    include StripAttribute
+    self.table_name = 'ci_freeze_periods'
+
+    belongs_to :project, inverse_of: :freeze_periods
+
+    strip_attributes :freeze_start, :freeze_end
+
+    validates :freeze_start, cron: true, presence: true
+    validates :freeze_end, cron: true, presence: true
+    validates :cron_timezone, cron_freeze_period_timezone: true, presence: true
+  end
+end

app/models/project.rb

@@ -302,6 +302,7 @@ class Project < ApplicationRecord
   has_many :project_deploy_tokens
   has_many :deploy_tokens, through: :project_deploy_tokens
   has_many :resource_groups, class_name: 'Ci::ResourceGroup', inverse_of: :project
+  has_many :freeze_periods, class_name: 'Ci::FreezePeriod', inverse_of: :project
 
   has_one :auto_devops, class_name: 'ProjectAutoDevops', inverse_of: :project, autosave: true
   has_many :custom_attributes, class_name: 'ProjectCustomAttribute'

app/validators/cron_freeze_period_timezone_validator.rb

+# frozen_string_literal: true
+
+# CronTimezoneValidator
+#
+# Custom validator for CronTimezone.
+class CronFreezePeriodTimezoneValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    freeze_start_parser = Gitlab::Ci::CronParser.new(record.freeze_start, record.cron_timezone)
+    freeze_end_parser = Gitlab::Ci::CronParser.new(record.freeze_end, record.cron_timezone)
+
+    record.errors.add(attribute, " is invalid syntax") unless freeze_start_parser.cron_timezone_valid? && freeze_end_parser.cron_timezone_valid?
+  end
+end

app/validators/cron_validator.rb

 # frozen_string_literal: true
 
-# CronValidator
-#
-# Custom validator for Cron.
 class CronValidator < ActiveModel::EachValidator
+  ATTRIBUTE_WHITELIST = %i[cron freeze_start freeze_end].freeze
+
+  NonWhitelistedAttributeError = Class.new(StandardError)
+
   def validate_each(record, attribute, value)
-    cron_parser = Gitlab::Ci::CronParser.new(record.cron, record.cron_timezone)
-    record.errors.add(attribute, " is invalid syntax") unless cron_parser.cron_valid?
+    if ATTRIBUTE_WHITELIST.include?(attribute)
+      cron_parser = Gitlab::Ci::CronParser.new(record.public_send(attribute), record.cron_timezone) # rubocop:disable GitlabSecurity/PublicSend
+      record.errors.add(attribute, " is invalid syntax") unless cron_parser.cron_valid?
+    else
+      raise NonWhitelistedAttributeError.new "Non-whitelisted attribute"
+    end
   end
 end

changelogs/unreleased/24295-freeze-period-db-changes.yml

+---
+title: Add freeze period model
+merge_request: 29162
+author:
+type: added

db/migrate/20200408125046_create_ci_freeze_periods.rb

+# frozen_string_literal: true
+
+class CreateCiFreezePeriods < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    unless table_exists?(:ci_freeze_periods)
+      create_table :ci_freeze_periods do |t|
+        t.references :project, foreign_key: true, null: false
+        t.text :freeze_start, null: false
+        t.text :freeze_end, null: false
+        t.text :cron_timezone, null: false
+
+        t.timestamps_with_timezone null: false
+      end
+    end
+
+    add_text_limit :ci_freeze_periods, :freeze_start, 998
+    add_text_limit :ci_freeze_periods, :freeze_end, 998
+    add_text_limit :ci_freeze_periods, :cron_timezone, 255
+  end
+
+  def down
+    drop_table :ci_freeze_periods
+  end
+end

db/structure.sql

@@ -1069,6 +1069,25 @@ CREATE SEQUENCE public.ci_daily_report_results_id_seq
 
 ALTER SEQUENCE public.ci_daily_report_results_id_seq OWNED BY public.ci_daily_report_results.id;
 
+CREATE TABLE public.ci_freeze_periods (
+    id bigint NOT NULL,
+    project_id bigint NOT NULL,
+    freeze_start character varying(998) NOT NULL,
+    freeze_end character varying(998) NOT NULL,
+    cron_timezone character varying(255) NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL
+);
+
+CREATE SEQUENCE public.ci_freeze_periods_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE public.ci_freeze_periods_id_seq OWNED BY public.ci_freeze_periods.id;
+
 CREATE TABLE public.ci_group_variables (
     id integer NOT NULL,
     key character varying NOT NULL,
@@ -7319,6 +7338,8 @@ ALTER TABLE ONLY public.ci_daily_build_group_report_results ALTER COLUMN id SET
 
 ALTER TABLE ONLY public.ci_daily_report_results ALTER COLUMN id SET DEFAULT nextval('public.ci_daily_report_results_id_seq'::regclass);
 
+ALTER TABLE ONLY public.ci_freeze_periods ALTER COLUMN id SET DEFAULT nextval('public.ci_freeze_periods_id_seq'::regclass);
+
 ALTER TABLE ONLY public.ci_group_variables ALTER COLUMN id SET DEFAULT nextval('public.ci_group_variables_id_seq'::regclass);
 
 ALTER TABLE ONLY public.ci_instance_variables ALTER COLUMN id SET DEFAULT nextval('public.ci_instance_variables_id_seq'::regclass);
@@ -7992,6 +8013,9 @@ ALTER TABLE ONLY public.ci_daily_build_group_report_results
 ALTER TABLE ONLY public.ci_daily_report_results
     ADD CONSTRAINT ci_daily_report_results_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY public.ci_freeze_periods
+    ADD CONSTRAINT ci_freeze_periods_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY public.ci_group_variables
     ADD CONSTRAINT ci_group_variables_pkey PRIMARY KEY (id);
 
@@ -9190,6 +9214,8 @@ CREATE INDEX index_ci_daily_build_group_report_results_on_last_pipeline_id ON pu
 
 CREATE INDEX index_ci_daily_report_results_on_last_pipeline_id ON public.ci_daily_report_results USING btree (last_pipeline_id);
 
+CREATE INDEX index_ci_freeze_periods_on_project_id ON public.ci_freeze_periods USING btree (project_id);
+
 CREATE UNIQUE INDEX index_ci_group_variables_on_group_id_and_key ON public.ci_group_variables USING btree (group_id, key);
 
 CREATE UNIQUE INDEX index_ci_instance_variables_on_key ON public.ci_instance_variables USING btree (key);
@@ -11670,6 +11696,9 @@ ALTER TABLE ONLY public.geo_repository_updated_events
 ALTER TABLE ONLY public.protected_branch_unprotect_access_levels
     ADD CONSTRAINT fk_rails_2d2aba21ef FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.ci_freeze_periods
+    ADD CONSTRAINT fk_rails_2e02bbd1a6 FOREIGN KEY (project_id) REFERENCES public.projects(id);
+
 ALTER TABLE ONLY public.saml_providers
     ADD CONSTRAINT fk_rails_306d459be7 FOREIGN KEY (group_id) REFERENCES public.namespaces(id) ON DELETE CASCADE;
 
@@ -13659,6 +13688,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200407182205
 20200407222647
 20200408110856
+20200408125046
 20200408133211
 20200408153842
 20200408154331

spec/factories/ci/freeze_periods.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :ci_freeze_period, class: 'Ci::FreezePeriod' do
+    project
+    freeze_start { '0 23 * * 5' }
+    freeze_end { '0 7 * * 1' }
+    cron_timezone { 'UTC' }
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -490,6 +490,7 @@ project:
 - metrics_users_starred_dashboards
 - alert_management_alerts
 - repository_storage_moves
+- freeze_periods
 award_emoji:
 - awardable
 - user

spec/models/ci/freeze_period_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::FreezePeriod, type: :model do
+  subject { build(:ci_freeze_period) }
+
+  it { is_expected.to belong_to(:project) }
+
+  it { is_expected.to respond_to(:freeze_start) }
+  it { is_expected.to respond_to(:freeze_end) }
+  it { is_expected.to respond_to(:cron_timezone) }
+
+  describe 'cron validations' do
+    it 'allows valid cron patterns' do
+      freeze_period = build(:ci_freeze_period, freeze_start: '0 23 * * 5')
+
+      expect(freeze_period).to be_valid
+    end
+
+    it 'does not allow invalid cron patterns' do
+      freeze_period = build(:ci_freeze_period, freeze_start: '0 0 0 * *')
+
+      expect(freeze_period).not_to be_valid
+    end
+
+    it 'does not allow non-cron strings' do
+      freeze_period = build(:ci_freeze_period, cron_timezone: 'invalid')
+
+      expect(freeze_period).not_to be_valid
+    end
+
+    context 'when cron contains trailing whitespaces' do
+      it 'strips the attribute' do
+        freeze_period = build(:ci_freeze_period, freeze_start: ' 0 0 * * *   ')
+
+        expect(freeze_period).to be_valid
+        expect(freeze_period.freeze_start).to eq('0 0 * * *')
+      end
+    end
+  end
+end

spec/models/ci/pipeline_schedule_spec.rb

@@ -22,13 +22,13 @@ describe Ci::PipelineSchedule do
   end
 
   describe 'validations' do
-    it 'does not allow invalid cron patters' do
+    it 'does not allow invalid cron patterns' do
       pipeline_schedule = build(:ci_pipeline_schedule, cron: '0 0 0 * *')
 
       expect(pipeline_schedule).not_to be_valid
     end
 
-    it 'does not allow invalid cron patters' do
+    it 'does not allow invalid cron patterns' do
       pipeline_schedule = build(:ci_pipeline_schedule, cron_timezone: 'invalid')
 
       expect(pipeline_schedule).not_to be_valid

spec/validators/cron_freeze_period_timezone_validator_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe CronFreezePeriodTimezoneValidator do
+  using RSpec::Parameterized::TableSyntax
+
+  subject { create :ci_freeze_period }
+
+  where(:freeze_start, :freeze_end, :is_valid) do
+    '0 23 * * 5'  | '0 7 * * 1'  | true
+    '0 23 * * 5'  | 'invalid'    | false
+    'invalid'     | '0 7 * * 1'  | false
+  end
+
+  with_them do
+    it 'crontab validation' do
+      subject.freeze_start = freeze_start
+      subject.freeze_end = freeze_end
+
+      expect(subject.valid?).to eq(is_valid)
+    end
+  end
+end

spec/validators/cron_validator_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe CronValidator do
+  subject do
+    Class.new do
+      include ActiveModel::Model
+      include ActiveModel::Validations
+      attr_accessor :cron
+      validates :cron, cron: true
+
+      def cron_timezone
+        'UTC'
+      end
+    end.new
+  end
+
+  it 'validates valid crontab' do
+    subject.cron = '0 23 * * 5'
+
+    expect(subject.valid?).to be_truthy
+  end
+
+  it 'validates invalid crontab' do
+    subject.cron = 'not a cron'
+
+    expect(subject.valid?).to be_falsy
+  end
+
+  context 'cron field is not whitelisted' do
+    subject do
+      Class.new do
+        include ActiveModel::Model
+        include ActiveModel::Validations
+        attr_accessor :cron_partytime
+        validates :cron_partytime, cron: true
+      end.new
+    end
+
+    it 'raises an error' do
+      subject.cron_partytime = '0 23 * * 5'
+
+      expect { subject.valid? }.to raise_error(StandardError, "Non-whitelisted attribute")
+    end
+  end
+end