Merge branch 'remove-auto-ssl-ff' into 'master'

Remove auto ssl feature flags

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

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

app/views/admin/application_settings/_pages.html.haml

@@ -15,7 +15,6 @@
       .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

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
 

app/views/projects/pages_domains/_helper_text.html.haml

@@ -2,8 +2,4 @@
 - docs_link_start = "<a href=\"%{docs_link_url}\" target=\"_blank\" rel=\"noopener noreferrer\" class=\"text-nowrap\">".html_safe % { docs_link_url: docs_link_url }
 - docs_link_end = "</a>".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 }

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

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

changelogs/unreleased/remove-auto-ssl-ff.yml

+---
+title: Add support for generating SSL certificates for custon pages domains through
+  Let's Encrypt
+merge_request:
+author:
+type: added

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

spec/features/admin/admin_settings_spec.rb

@@ -400,13 +400,8 @@ 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
+      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"
@@ -418,20 +413,6 @@ describe 'Admin updates settings' do
     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
-      end
-
-      it "Doesn't show Let's Encrypt options" do
-        page.within('.as-pages') do
-          expect(page).not_to have_content('Email')
-        end
-      end
-    end
-  end
-
   def check_all_events
     page.check('Active')
     page.check('Push')

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

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|