Merge branch 'pages-domains-validity-time-mysql' into 'master'

Add certificate valid time to pages domain table

See merge request gitlab-org/gitlab-ce!29287

app/models/pages_domain.rb

@@ -135,6 +135,14 @@ class PagesDomain < ApplicationRecord
     "#{VERIFICATION_KEY}=#{verification_code}"
   end
 
+  def certificate=(certificate)
+    super(certificate)
+
+    # set nil, if certificate is nil
+    self.certificate_valid_not_before = x509&.not_before
+    self.certificate_valid_not_after = x509&.not_after
+  end
+
   private
 
   def set_verification_code
@@ -187,7 +195,7 @@ class PagesDomain < ApplicationRecord
   end
 
   def x509
-    return unless certificate
+    return unless certificate.present?
 
     @x509 ||= OpenSSL::X509::Certificate.new(certificate)
   rescue OpenSSL::X509::CertificateError

db/migrate/20190524071727_add_ssl_valid_period_to_pages_domain.rb

+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddSslValidPeriodToPagesDomain < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    add_column :pages_domains, :certificate_valid_not_before, :datetime_with_timezone
+    add_column :pages_domains, :certificate_valid_not_after, :datetime_with_timezone
+  end
+end

db/post_migrate/20190524073827_schedule_fill_valid_time_for_pages_domain_certificates.rb

+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class ScheduleFillValidTimeForPagesDomainCertificates < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  MIGRATION = 'FillValidTimeForPagesDomainCertificate'
+  BATCH_SIZE = 500
+  BATCH_TIME = 5.minutes
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class PagesDomain < ActiveRecord::Base
+    include ::EachBatch
+
+    self.table_name = 'pages_domains'
+  end
+
+  def up
+    queue_background_migration_jobs_by_range_at_intervals(
+      PagesDomain.where.not(certificate: [nil, '']),
+      MIGRATION,
+      BATCH_TIME,
+      batch_size: BATCH_SIZE)
+  end
+
+  def down
+  end
+end

db/schema.rb

@@ -1597,6 +1597,8 @@ ActiveRecord::Schema.define(version: 20190530154715) do
     t.datetime_with_timezone "enabled_until"
     t.datetime_with_timezone "remove_at"
     t.boolean "auto_ssl_enabled", default: false, null: false
+    t.datetime_with_timezone "certificate_valid_not_before"
+    t.datetime_with_timezone "certificate_valid_not_after"
     t.index ["domain"], name: "index_pages_domains_on_domain", unique: true, using: :btree
     t.index ["project_id", "enabled_until"], name: "index_pages_domains_on_project_id_and_enabled_until", using: :btree
     t.index ["project_id"], name: "index_pages_domains_on_project_id", using: :btree

lib/gitlab/background_migration/fill_valid_time_for_pages_domain_certificate.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # save validity time pages domain
+    class FillValidTimeForPagesDomainCertificate
+      # define PagesDomain with only needed code
+      class PagesDomain < ActiveRecord::Base
+        self.table_name = 'pages_domains'
+
+        def x509
+          return unless certificate.present?
+
+          @x509 ||= OpenSSL::X509::Certificate.new(certificate)
+        rescue OpenSSL::X509::CertificateError
+          nil
+        end
+      end
+
+      def perform(start_id, stop_id)
+        PagesDomain.where(id: start_id..stop_id).find_each do |domain|
+          if Gitlab::Database.mysql?
+            domain.update_columns(
+              certificate_valid_not_before: domain.x509&.not_before,
+              certificate_valid_not_after: domain.x509&.not_after
+            )
+          else
+            # for some reason activerecord doesn't append timezone, iso8601 forces this
+            domain.update_columns(
+              certificate_valid_not_before: domain.x509&.not_before&.iso8601,
+              certificate_valid_not_after: domain.x509&.not_after&.iso8601
+            )
+          end
+        rescue => e
+          Rails.logger.error "Failed to update pages domain certificate valid time. id: #{domain.id}, message: #{e.message}"
+        end
+      end
+    end
+  end
+end

spec/migrations/enqueue_verify_pages_domain_workers_spec.rb

@@ -8,9 +8,13 @@ describe EnqueueVerifyPagesDomainWorkers, :sidekiq, :migration do
     end
   end
 
+  let(:domains_table) { table(:pages_domains) }
+
   describe '#up' do
     it 'enqueues a verification worker for every domain' do
-      domains = 1.upto(3).map { |i| PagesDomain.create!(domain: "my#{i}.domain.com") }
+      domains = Array.new(3) do |i|
+        domains_table.create!(domain: "my#{i}.domain.com", verification_code: "123#{i}")
+      end
 
       expect { migrate! }.to change(PagesDomainVerificationWorker.jobs, :size).by(3)
 

spec/migrations/schedule_fill_valid_time_for_pages_domain_certificates_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20190524073827_schedule_fill_valid_time_for_pages_domain_certificates.rb')
+
+describe ScheduleFillValidTimeForPagesDomainCertificates, :migration, :sidekiq do
+  let(:migration_class) { described_class::MIGRATION }
+  let(:migration_name)  { migration_class.to_s.demodulize }
+
+  let(:domains_table) { table(:pages_domains) }
+
+  let(:certificate) do
+    File.read('spec/fixtures/passphrase_x509_certificate.crt')
+  end
+
+  before do
+    domains_table.create!(domain: "domain1.example.com", verification_code: "123")
+    domains_table.create!(domain: "domain2.example.com", verification_code: "123", certificate: '')
+    domains_table.create!(domain: "domain3.example.com", verification_code: "123", certificate: certificate)
+    domains_table.create!(domain: "domain4.example.com", verification_code: "123", certificate: certificate)
+  end
+
+  it 'correctly schedules background migrations' do
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        first_id = domains_table.find_by_domain("domain3.example.com").id
+        last_id = domains_table.find_by_domain("domain4.example.com").id
+
+        expect(migration_name).to be_scheduled_delayed_migration(5.minutes, first_id, last_id)
+        expect(BackgroundMigrationWorker.jobs.size).to eq(1)
+      end
+    end
+  end
+
+  it 'sets certificate valid_not_before/not_after' do
+    perform_enqueued_jobs do
+      migrate!
+
+      domain = domains_table.find_by_domain("domain3.example.com")
+      expect(domain.certificate_valid_not_before)
+        .to eq(Time.parse("2018-03-23 14:02:08 UTC"))
+      expect(domain.certificate_valid_not_after)
+        .to eq(Time.parse("2019-03-23 14:02:08 UTC"))
+    end
+  end
+end

spec/models/pages_domain_spec.rb

@@ -81,6 +81,17 @@ describe PagesDomain do
     end
   end
 
+  describe 'when certificate is specified' do
+    let(:domain) { build(:pages_domain) }
+
+    it 'saves validity time' do
+      domain.save
+
+      expect(domain.certificate_valid_not_before).to be_like_time(Time.parse("2016-02-12 14:32:00 UTC"))
+      expect(domain.certificate_valid_not_after).to be_like_time(Time.parse("2020-04-12 14:32:00 UTC"))
+    end
+  end
+
   describe 'validate certificate' do
     subject { domain }