From 7b5936ebdad8df646cbce7a9c035da62cac7cfce Mon Sep 17 00:00:00 2001 From: Vladimir Shushlin Date: Fri, 12 Jul 2019 16:53:44 +0000 Subject: Remove auto ssl feature flags * remove feature flag for admin settings * remove feature flag for domain settings --- .../admin/application_settings_controller.rb | 12 ++------ .../admin/application_settings/_pages.html.haml | 33 ++++++++++---------- app/views/projects/pages_domains/_form.html.haml | 2 +- .../projects/pages_domains/_helper_text.html.haml | 6 +--- .../pages_domain_ssl_renewal_cron_worker.rb | 4 +-- app/workers/pages_domain_ssl_renewal_worker.rb | 2 +- changelogs/unreleased/remove-auto-ssl-ff.yml | 6 ++++ lib/gitlab/lets_encrypt.rb | 11 ++----- spec/features/admin/admin_settings_spec.rb | 35 +++++----------------- spec/lib/gitlab/lets_encrypt_spec.rb | 31 +------------------ ...obtain_lets_encrypt_certificate_service_spec.rb | 6 ++++ 11 files changed, 46 insertions(+), 102 deletions(-) create mode 100644 changelogs/unreleased/remove-auto-ssl-ff.yml diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index a570da61d54..e9ec8876688 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -103,7 +103,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController [ *::ApplicationSettingsHelper.visible_attributes, *::ApplicationSettingsHelper.external_authorization_service_attributes, - *lets_encrypt_visible_attributes, + :lets_encrypt_notification_email, + :lets_encrypt_terms_of_service_accepted, :domain_blacklist_file, disabled_oauth_sign_in_sources: [], import_sources: [], @@ -143,13 +144,4 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController render action end - - def lets_encrypt_visible_attributes - return [] unless Feature.enabled?(:pages_auto_ssl) - - [ - :lets_encrypt_notification_email, - :lets_encrypt_terms_of_service_accepted - ] - end end diff --git a/app/views/admin/application_settings/_pages.html.haml b/app/views/admin/application_settings/_pages.html.haml index d7d709ffd62..5b8c1e4d54f 100644 --- a/app/views/admin/application_settings/_pages.html.haml +++ b/app/views/admin/application_settings/_pages.html.haml @@ -15,22 +15,21 @@ .form-text.text-muted = _("Domain verification is an essential security measure for public GitLab sites. Users are required to demonstrate they control a domain before it is enabled") = link_to icon('question-circle'), help_page_path('user/project/pages/getting_started_part_three.md', anchor: 'dns-txt-record') - - if Feature.enabled?(:pages_auto_ssl) - %h5 - = _("Configure Let's Encrypt") - %p - - lets_encrypt_link_start = ''.html_safe % { url: "https://letsencrypt.org/" } - = _("%{lets_encrypt_link_start}Let's Encrypt%{lets_encrypt_link_end} is a free, automated, and open certificate authority (CA), that give digital certificates in order to enable HTTPS (SSL/TLS) for websites.").html_safe % { lets_encrypt_link_start: lets_encrypt_link_start, lets_encrypt_link_end: ''.html_safe } - .form-group - = f.label :lets_encrypt_notification_email, _("Email"), class: 'label-bold' - = f.text_field :lets_encrypt_notification_email, class: 'form-control' - .form-text.text-muted - = _("A Let's Encrypt account will be configured for this GitLab installation using your email address. You will receive emails to warn of expiring certificates.") - .form-group - .form-check - = f.check_box :lets_encrypt_terms_of_service_accepted, class: 'form-check-input' - = f.label :lets_encrypt_terms_of_service_accepted, class: 'form-check-label' do - - terms_of_service_link_start = ''.html_safe % { url: lets_encrypt_terms_of_service_admin_application_settings_path } - = _("I have read and agree to the Let's Encrypt %{link_start}Terms of Service%{link_end}").html_safe % { link_start: terms_of_service_link_start, link_end: ''.html_safe } + %h5 + = _("Configure Let's Encrypt") + %p + - lets_encrypt_link_start = ''.html_safe % { url: "https://letsencrypt.org/" } + = _("%{lets_encrypt_link_start}Let's Encrypt%{lets_encrypt_link_end} is a free, automated, and open certificate authority (CA), that give digital certificates in order to enable HTTPS (SSL/TLS) for websites.").html_safe % { lets_encrypt_link_start: lets_encrypt_link_start, lets_encrypt_link_end: ''.html_safe } + .form-group + = f.label :lets_encrypt_notification_email, _("Email"), class: 'label-bold' + = f.text_field :lets_encrypt_notification_email, class: 'form-control' + .form-text.text-muted + = _("A Let's Encrypt account will be configured for this GitLab installation using your email address. You will receive emails to warn of expiring certificates.") + .form-group + .form-check + = f.check_box :lets_encrypt_terms_of_service_accepted, class: 'form-check-input' + = f.label :lets_encrypt_terms_of_service_accepted, class: 'form-check-label' do + - terms_of_service_link_start = ''.html_safe % { url: lets_encrypt_terms_of_service_admin_application_settings_path } + = _("I have read and agree to the Let's Encrypt %{link_start}Terms of Service%{link_end}").html_safe % { link_start: terms_of_service_link_start, link_end: ''.html_safe } = f.submit _('Save changes'), class: "btn btn-success" diff --git a/app/views/projects/pages_domains/_form.html.haml b/app/views/projects/pages_domains/_form.html.haml index 5b657966909..0e5c65a2f72 100644 --- a/app/views/projects/pages_domains/_form.html.haml +++ b/app/views/projects/pages_domains/_form.html.haml @@ -11,7 +11,7 @@ - if Gitlab.config.pages.external_https - - auto_ssl_available = ::Gitlab::LetsEncrypt.enabled?(@domain) + - auto_ssl_available = ::Gitlab::LetsEncrypt.enabled? - auto_ssl_enabled = @domain.auto_ssl_enabled? - auto_ssl_available_and_enabled = auto_ssl_available && auto_ssl_enabled diff --git a/app/views/projects/pages_domains/_helper_text.html.haml b/app/views/projects/pages_domains/_helper_text.html.haml index 5a79fefabfc..4e3daa597e0 100644 --- a/app/views/projects/pages_domains/_helper_text.html.haml +++ b/app/views/projects/pages_domains/_helper_text.html.haml @@ -2,8 +2,4 @@ - docs_link_start = "".html_safe % { docs_link_url: docs_link_url } - docs_link_end = "".html_safe --# Hiding behind a feature flag to avoid any changes to this feature's implemention --# when the :pages_auto_ssl feature flag is disabled. This check should be removed --# once the :pages_auto_ssl feature flag is removed. -- if Feature.enabled?(:pages_auto_ssl) - %p= _("Learn more about adding certificates to your project by following the %{docs_link_start}documentation on GitLab Pages%{docs_link_end}.").html_safe % { docs_link_url: docs_link_url, docs_link_start: docs_link_start, docs_link_end: docs_link_end } +%p= _("Learn more about adding certificates to your project by following the %{docs_link_start}documentation on GitLab Pages%{docs_link_end}.").html_safe % { docs_link_url: docs_link_url, docs_link_start: docs_link_start, docs_link_end: docs_link_end } diff --git a/app/workers/pages_domain_ssl_renewal_cron_worker.rb b/app/workers/pages_domain_ssl_renewal_cron_worker.rb index 40c34d29970..e5dde07a648 100644 --- a/app/workers/pages_domain_ssl_renewal_cron_worker.rb +++ b/app/workers/pages_domain_ssl_renewal_cron_worker.rb @@ -5,9 +5,9 @@ class PagesDomainSslRenewalCronWorker include CronjobQueue def perform - PagesDomain.need_auto_ssl_renewal.find_each do |domain| - next unless ::Gitlab::LetsEncrypt.enabled?(domain) + return unless ::Gitlab::LetsEncrypt.enabled? + PagesDomain.need_auto_ssl_renewal.find_each do |domain| PagesDomainSslRenewalWorker.perform_async(domain.id) end end diff --git a/app/workers/pages_domain_ssl_renewal_worker.rb b/app/workers/pages_domain_ssl_renewal_worker.rb index b32458ca777..87fd8059946 100644 --- a/app/workers/pages_domain_ssl_renewal_worker.rb +++ b/app/workers/pages_domain_ssl_renewal_worker.rb @@ -6,7 +6,7 @@ class PagesDomainSslRenewalWorker def perform(domain_id) domain = PagesDomain.find_by_id(domain_id) return unless domain&.enabled? - return unless ::Gitlab::LetsEncrypt.enabled?(domain) + return unless ::Gitlab::LetsEncrypt.enabled? ::PagesDomains::ObtainLetsEncryptCertificateService.new(domain).execute end diff --git a/changelogs/unreleased/remove-auto-ssl-ff.yml b/changelogs/unreleased/remove-auto-ssl-ff.yml new file mode 100644 index 00000000000..1b4d0dcde08 --- /dev/null +++ b/changelogs/unreleased/remove-auto-ssl-ff.yml @@ -0,0 +1,6 @@ +--- +title: Add support for generating SSL certificates for custon pages domains through + Let's Encrypt +merge_request: +author: +type: added diff --git a/lib/gitlab/lets_encrypt.rb b/lib/gitlab/lets_encrypt.rb index cdf24f24647..08ad2ab91b0 100644 --- a/lib/gitlab/lets_encrypt.rb +++ b/lib/gitlab/lets_encrypt.rb @@ -2,15 +2,8 @@ module Gitlab module LetsEncrypt - def self.enabled?(pages_domain = nil) - return false unless Gitlab::CurrentSettings.lets_encrypt_terms_of_service_accepted - - return false unless Feature.enabled?(:pages_auto_ssl) - - # If no domain is passed, just check whether we're enabled globally - return true unless pages_domain - - !!pages_domain.project && Feature.enabled?(:pages_auto_ssl_for_project, pages_domain.project) + def self.enabled? + Gitlab::CurrentSettings.lets_encrypt_terms_of_service_accepted end end end diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index 4a9037afb43..518b3625348 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -400,35 +400,16 @@ describe 'Admin updates settings' do .to have_content "The form contains the following error: Polling interval multiplier must be greater than or equal to 0" end - context 'When pages_auto_ssl is enabled' do - before do - stub_feature_flags(pages_auto_ssl: true) - visit preferences_admin_application_settings_path - end - - it "Change Pages Let's Encrypt settings" do - page.within('.as-pages') do - fill_in 'Email', with: 'my@test.example.com' - check "I have read and agree to the Let's Encrypt Terms of Service" - click_button 'Save changes' - end - - expect(current_settings.lets_encrypt_notification_email).to eq 'my@test.example.com' - expect(current_settings.lets_encrypt_terms_of_service_accepted).to eq true - end - end - - context 'When pages_auto_ssl is disabled' do - before do - stub_feature_flags(pages_auto_ssl: false) - visit preferences_admin_application_settings_path + it "Change Pages Let's Encrypt settings" do + visit preferences_admin_application_settings_path + page.within('.as-pages') do + fill_in 'Email', with: 'my@test.example.com' + check "I have read and agree to the Let's Encrypt Terms of Service" + click_button 'Save changes' end - it "Doesn't show Let's Encrypt options" do - page.within('.as-pages') do - expect(page).not_to have_content('Email') - end - end + expect(current_settings.lets_encrypt_notification_email).to eq 'my@test.example.com' + expect(current_settings.lets_encrypt_terms_of_service_accepted).to eq true end end diff --git a/spec/lib/gitlab/lets_encrypt_spec.rb b/spec/lib/gitlab/lets_encrypt_spec.rb index 674b114e9d3..65aea0937f1 100644 --- a/spec/lib/gitlab/lets_encrypt_spec.rb +++ b/spec/lib/gitlab/lets_encrypt_spec.rb @@ -10,21 +10,10 @@ describe ::Gitlab::LetsEncrypt do end describe '.enabled?' do - let(:project) { create(:project) } - let(:pages_domain) { create(:pages_domain, project: project) } - - subject { described_class.enabled?(pages_domain) } + subject { described_class.enabled? } context 'when terms of service are accepted' do it { is_expected.to eq(true) } - - context 'when feature flag is disabled' do - before do - stub_feature_flags(pages_auto_ssl: false) - end - - it { is_expected.to eq(false) } - end end context 'when terms of service are not accepted' do @@ -34,23 +23,5 @@ describe ::Gitlab::LetsEncrypt do it { is_expected.to eq(false) } end - - context 'when feature flag for project is disabled' do - before do - stub_feature_flags(pages_auto_ssl_for_project: false) - end - - it 'returns false' do - is_expected.to eq(false) - end - end - - context 'when domain has not project' do - let(:pages_domain) { create(:pages_domain) } - - it 'returns false' do - is_expected.to eq(false) - end - end end end diff --git a/spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb b/spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb index 8d43ce4f662..af79a42b611 100644 --- a/spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb +++ b/spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb @@ -12,6 +12,12 @@ describe PagesDomains::ObtainLetsEncryptCertificateService do stub_lets_encrypt_settings end + around do |example| + Sidekiq::Testing.fake! do + example.run + end + end + def expect_to_create_acme_challenge expect(::PagesDomains::CreateAcmeOrderService).to receive(:new).with(pages_domain) .and_wrap_original do |m, *args| -- cgit v1.2.1