From a7764d0e845db524f2913b6c11c88dfd121ec522 Mon Sep 17 00:00:00 2001 From: Vladimir Shushlin Date: Mon, 24 Jun 2019 20:35:12 +0000 Subject: [PATCH] Renew Let's Encrypt certificates Add index for pages domain ssl auto renewal Add PagesDomain.needs_ssl_renewal scope Add cron worker for ssl renewal Add worker for ssl renewal Add pages ssl renewal worker queues settings --- app/models/pages_domain.rb | 10 ++++ app/workers/all_queues.yml | 2 + .../pages_domain_ssl_renewal_cron_worker.rb | 14 +++++ .../pages_domain_ssl_renewal_worker.rb | 15 +++++ config/initializers/1_settings.rb | 4 ++ config/sidekiq_queues.yml | 1 + ...45325_add_pages_domains_ssl_renew_index.rb | 25 ++++++++ db/schema.rb | 1 + spec/factories/pages_domains.rb | 1 + spec/models/pages_domain_spec.rb | 26 ++++++++ ...ges_domain_ssl_renewal_cron_worker_spec.rb | 59 +++++++++++++++++++ .../pages_domain_ssl_renewal_worker_spec.rb | 35 +++++++++++ 12 files changed, 193 insertions(+) create mode 100644 app/workers/pages_domain_ssl_renewal_cron_worker.rb create mode 100644 app/workers/pages_domain_ssl_renewal_worker.rb create mode 100644 db/migrate/20190607145325_add_pages_domains_ssl_renew_index.rb create mode 100644 spec/workers/pages_domain_ssl_renewal_cron_worker_spec.rb create mode 100644 spec/workers/pages_domain_ssl_renewal_worker_spec.rb diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb index 07195c0bfd3..d6d879c6d89 100644 --- a/app/models/pages_domain.rb +++ b/app/models/pages_domain.rb @@ -3,6 +3,7 @@ class PagesDomain < ApplicationRecord VERIFICATION_KEY = 'gitlab-pages-verification-code'.freeze VERIFICATION_THRESHOLD = 3.days.freeze + SSL_RENEWAL_THRESHOLD = 30.days.freeze enum certificate_source: { user_provided: 0, gitlab_provided: 1 }, _prefix: :certificate @@ -41,6 +42,15 @@ class PagesDomain < ApplicationRecord where(verified_at.eq(nil).or(enabled_until.eq(nil).or(enabled_until.lt(threshold)))) end + scope :need_auto_ssl_renewal, -> do + expiring = where(certificate_valid_not_after: nil).or( + where(arel_table[:certificate_valid_not_after].lt(SSL_RENEWAL_THRESHOLD.from_now))) + + user_provided_or_expiring = certificate_user_provided.or(expiring) + + where(auto_ssl_enabled: true).merge(user_provided_or_expiring) + end + scope :for_removal, -> { where("remove_at < ?", Time.now) } def verified? diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index fd0cc5fb24e..e55962b629e 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -9,6 +9,7 @@ - cronjob:import_export_project_cleanup - cronjob:pages_domain_verification_cron - cronjob:pages_domain_removal_cron +- cronjob:pages_domain_ssl_renewal_cron - cronjob:pipeline_schedule - cronjob:prune_old_events - cronjob:remove_expired_group_links @@ -133,6 +134,7 @@ - new_note - pages - pages_domain_verification +- pages_domain_ssl_renewal - plugin - post_receive - process_commit diff --git a/app/workers/pages_domain_ssl_renewal_cron_worker.rb b/app/workers/pages_domain_ssl_renewal_cron_worker.rb new file mode 100644 index 00000000000..4ca9db922b4 --- /dev/null +++ b/app/workers/pages_domain_ssl_renewal_cron_worker.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class PagesDomainSslRenewalCronWorker + include ApplicationWorker + include CronjobQueue + + def perform + return unless ::Gitlab::LetsEncrypt::Client.new.enabled? + + PagesDomain.need_auto_ssl_renewal.find_each do |domain| + PagesDomainSslRenewalWorker.perform_async(domain.id) + end + end +end diff --git a/app/workers/pages_domain_ssl_renewal_worker.rb b/app/workers/pages_domain_ssl_renewal_worker.rb new file mode 100644 index 00000000000..00c9c4782d8 --- /dev/null +++ b/app/workers/pages_domain_ssl_renewal_worker.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class PagesDomainSslRenewalWorker + include ApplicationWorker + + def perform(domain_id) + return unless ::Gitlab::LetsEncrypt::Client.new.enabled? + + domain = PagesDomain.find_by_id(domain_id) + + return unless domain + + ::PagesDomains::ObtainLetsEncryptCertificateService.new(domain).execute + end +end diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 6cca7a3b75f..4b0bb86e42a 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -367,6 +367,10 @@ Settings.cron_jobs['pages_domain_removal_cron_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['pages_domain_removal_cron_worker']['cron'] ||= '47 0 * * *' Settings.cron_jobs['pages_domain_removal_cron_worker']['job_class'] = 'PagesDomainRemovalCronWorker' +Settings.cron_jobs['pages_domain_ssl_renewal_cron_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['pages_domain_ssl_renewal_cron_worker']['cron'] ||= '*/5 * * * *' +Settings.cron_jobs['pages_domain_ssl_renewal_cron_worker']['job_class'] = 'PagesDomainSslRenewalCronWorker' + Settings.cron_jobs['issue_due_scheduler_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['issue_due_scheduler_worker']['cron'] ||= '50 00 * * *' Settings.cron_jobs['issue_due_scheduler_worker']['job_class'] = 'IssueDueSchedulerWorker' diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 4fda9d69077..25fd65d8644 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -72,6 +72,7 @@ - [project_rollback_hashed_storage, 1] - [hashed_storage, 1] - [pages_domain_verification, 1] + - [pages_domain_ssl_renewal, 1] - [object_storage_upload, 1] - [object_storage, 1] - [plugin, 1] diff --git a/db/migrate/20190607145325_add_pages_domains_ssl_renew_index.rb b/db/migrate/20190607145325_add_pages_domains_ssl_renew_index.rb new file mode 100644 index 00000000000..7167accbf1e --- /dev/null +++ b/db/migrate/20190607145325_add_pages_domains_ssl_renew_index.rb @@ -0,0 +1,25 @@ +# 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 AddPagesDomainsSslRenewIndex < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + INDEX_NAME = 'index_pages_domains_need_auto_ssl_renewal' + + disable_ddl_transaction! + + def up + add_concurrent_index(:pages_domains, [:certificate_source, :certificate_valid_not_after], + where: "auto_ssl_enabled = #{::Gitlab::Database.true_value}", name: INDEX_NAME) + end + + def down + remove_concurrent_index(:pages_domains, [:certificate_source, :certificate_valid_not_after], + where: "auto_ssl_enabled = #{::Gitlab::Database.true_value}", name: INDEX_NAME) + end +end diff --git a/db/schema.rb b/db/schema.rb index f6cf2ee07e3..b81558178b9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2334,6 +2334,7 @@ ActiveRecord::Schema.define(version: 20190620112608) do t.datetime_with_timezone "certificate_valid_not_before" t.datetime_with_timezone "certificate_valid_not_after" t.integer "certificate_source", limit: 2, default: 0, null: false + t.index ["certificate_source", "certificate_valid_not_after"], name: "index_pages_domains_need_auto_ssl_renewal", where: "(auto_ssl_enabled = true)", using: :btree 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 diff --git a/spec/factories/pages_domains.rb b/spec/factories/pages_domains.rb index 8da19a37a6a..3e0baab04ce 100644 --- a/spec/factories/pages_domains.rb +++ b/spec/factories/pages_domains.rb @@ -182,6 +182,7 @@ ZDXgrA== end trait :letsencrypt do + auto_ssl_enabled { true } certificate_source { :gitlab_provided } end end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 4fb7b71a3c7..661957cf08b 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -479,4 +479,30 @@ describe PagesDomain do end end end + + describe '.need_auto_ssl_renewal' do + subject { described_class.need_auto_ssl_renewal } + + let!(:domain_with_user_provided_certificate) { create(:pages_domain) } + let!(:domain_with_expired_user_provided_certificate) do + create(:pages_domain, :with_expired_certificate) + end + let!(:domain_with_user_provided_certificate_and_auto_ssl) do + create(:pages_domain, auto_ssl_enabled: true) + end + + let!(:domain_with_gitlab_provided_certificate) { create(:pages_domain, :letsencrypt) } + let!(:domain_with_expired_gitlab_provided_certificate) do + create(:pages_domain, :letsencrypt, :with_expired_certificate) + end + + it 'contains only domains needing verification' do + is_expected.to( + contain_exactly( + domain_with_user_provided_certificate_and_auto_ssl, + domain_with_expired_gitlab_provided_certificate + ) + ) + end + end end diff --git a/spec/workers/pages_domain_ssl_renewal_cron_worker_spec.rb b/spec/workers/pages_domain_ssl_renewal_cron_worker_spec.rb new file mode 100644 index 00000000000..2ae4872f51d --- /dev/null +++ b/spec/workers/pages_domain_ssl_renewal_cron_worker_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe PagesDomainSslRenewalCronWorker do + include LetsEncryptHelpers + + subject(:worker) { described_class.new } + + before do + stub_lets_encrypt_settings + end + + describe '#perform' do + let!(:domain) { create(:pages_domain) } + let!(:domain_with_enabled_auto_ssl) { create(:pages_domain, auto_ssl_enabled: true) } + let!(:domain_with_obtained_letsencrypt) { create(:pages_domain, :letsencrypt, auto_ssl_enabled: true) } + let!(:domain_without_auto_certificate) do + create(:pages_domain, :without_certificate, :without_key, auto_ssl_enabled: true) + end + + let!(:domain_with_expired_auto_ssl) do + create(:pages_domain, :letsencrypt, :with_expired_certificate) + end + + it 'enqueues a PagesDomainSslRenewalWorker for domains needing renewal' do + [domain_without_auto_certificate, + domain_with_enabled_auto_ssl, + domain_with_expired_auto_ssl].each do |domain| + expect(PagesDomainSslRenewalWorker).to receive(:perform_async).with(domain.id) + end + + [domain, + domain_with_obtained_letsencrypt].each do |domain| + expect(PagesDomainVerificationWorker).not_to receive(:perform_async).with(domain.id) + end + + worker.perform + end + + shared_examples 'does nothing' do + it 'does nothing' do + expect(PagesDomainSslRenewalWorker).not_to receive(:perform_async) + + worker.perform + end + end + + context 'when letsencrypt integration is disabled' do + before do + stub_application_setting( + lets_encrypt_terms_of_service_accepted: false + ) + end + + include_examples 'does nothing' + end + end +end diff --git a/spec/workers/pages_domain_ssl_renewal_worker_spec.rb b/spec/workers/pages_domain_ssl_renewal_worker_spec.rb new file mode 100644 index 00000000000..a3d33de1b40 --- /dev/null +++ b/spec/workers/pages_domain_ssl_renewal_worker_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe PagesDomainSslRenewalWorker do + include LetsEncryptHelpers + + subject(:worker) { described_class.new } + + let(:domain) { create(:pages_domain) } + + before do + stub_lets_encrypt_settings + end + + describe '#perform' do + it 'delegates to ObtainLetsEncryptCertificateService' do + service = double(:service) + expect(::PagesDomains::ObtainLetsEncryptCertificateService).to receive(:new).with(domain).and_return(service) + expect(service).to receive(:execute) + + worker.perform(domain.id) + end + + context 'when domain was deleted' do + before do + domain.destroy! + end + + it 'does nothing' do + expect(::PagesDomains::ObtainLetsEncryptCertificateService).not_to receive(:new) + end + end + end +end