Merge branch 'fix-only-https-pages-domains' into 'master'

Fix saving domain without certificate for auto_ssl

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

2 files changed, 27 insertions, 12 deletions

diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb
index d6d879c6d89..e6e491634ab 100644
--- a/app/models/pages_domain.rb
+++ b/app/models/pages_domain.rb
@@ -12,9 +12,11 @@ class PagesDomain < ApplicationRecord
 
   validates :domain, hostname: { allow_numeric_hostname: true }
   validates :domain, uniqueness: { case_sensitive: false }
-  validates :certificate, presence: { message: 'must be present if HTTPS-only is enabled' }, if: ->(domain) { domain.project&.pages_https_only? }
+  validates :certificate, presence: { message: 'must be present if HTTPS-only is enabled' },
+            if: :certificate_should_be_present?
   validates :certificate, certificate: true, if: ->(domain) { domain.certificate.present? }
-  validates :key, presence: { message: 'must be present if HTTPS-only is enabled' }, if: ->(domain) { domain.project&.pages_https_only? }
+  validates :key, presence: { message: 'must be present if HTTPS-only is enabled' },
+            if: :certificate_should_be_present?
   validates :key, certificate_key: true, if: ->(domain) { domain.key.present? }
   validates :verification_code, presence: true, allow_blank: false
 
@@ -249,4 +251,8 @@ class PagesDomain < ApplicationRecord
   rescue OpenSSL::PKey::PKeyError, OpenSSL::Cipher::CipherError
     nil
   end
+
+  def certificate_should_be_present?
+    !auto_ssl_enabled? && project&.pages_https_only?
+  end
 end
diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb
index 661957cf08b..973c67937b7 100644
--- a/spec/models/pages_domain_spec.rb
+++ b/spec/models/pages_domain_spec.rb
@@ -53,24 +53,33 @@ describe PagesDomain do
       end
 
       let(:pages_domain) do
-        build(:pages_domain, certificate: certificate, key: key).tap do |pd|
+        build(:pages_domain, certificate: certificate, key: key,
+              auto_ssl_enabled: auto_ssl_enabled).tap do |pd|
           allow(pd).to receive(:project).and_return(project)
           pd.valid?
         end
       end
 
-      where(:pages_https_only, :certificate, :key, :errors_on) do
+      where(:pages_https_only, :certificate, :key, :auto_ssl_enabled, :errors_on) do
         attributes = attributes_for(:pages_domain)
         cert, key = attributes.fetch_values(:certificate, :key)
 
-        true  | nil  | nil | %i(certificate key)
-        true  | cert | nil | %i(key)
-        true  | nil  | key | %i(certificate key)
-        true  | cert | key | []
-        false | nil  | nil | []
-        false | cert | nil | %i(key)
-        false | nil  | key | %i(key)
-        false | cert | key | []
+        true  | nil  | nil | false | %i(certificate key)
+        true  | nil  | nil | true  | []
+        true  | cert | nil | false | %i(key)
+        true  | cert | nil | true  | %i(key)
+        true  | nil  | key | false | %i(certificate key)
+        true  | nil  | key | true  | %i(key)
+        true  | cert | key | false | []
+        true  | cert | key | true  | []
+        false | nil  | nil | false | []
+        false | nil  | nil | true  | []
+        false | cert | nil | false | %i(key)
+        false | cert | nil | true  | %i(key)
+        false | nil  | key | false | %i(key)
+        false | nil  | key | true  | %i(key)
+        false | cert | key | false | []
+        false | cert | key | true  | []
       end
 
       with_them do