Merge branch '38175-add-domain-field-to-auto-devops-application-setting' into 'master'

Resolve "Add domain field to Auto DevOps application setting"

Closes #38175

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

app/helpers/application_settings_helper.rb

@@ -146,6 +146,7 @@ module ApplicationSettingsHelper
       :akismet_enabled,
       :authorized_keys_enabled,
       :auto_devops_enabled,
+      :auto_devops_domain,
       :circuitbreaker_access_retries,
       :circuitbreaker_check_interval,
       :circuitbreaker_failure_count_threshold,

app/helpers/auto_devops_helper.rb

@@ -9,21 +9,28 @@ module AutoDevopsHelper
   end
 
   def auto_devops_warning_message(project)
-    missing_domain = !project.auto_devops&.has_domain?
-    missing_service = !project.deployment_platform&.active?
-
-    if missing_service
+    if missing_auto_devops_service?(project)
       params = {
         kubernetes: link_to('Kubernetes cluster', project_clusters_path(project))
       }
 
-      if missing_domain
+      if missing_auto_devops_domain?(project)
         _('Auto Review Apps and Auto Deploy need a domain name and a %{kubernetes} to work correctly.') % params
       else
         _('Auto Review Apps and Auto Deploy need a %{kubernetes} to work correctly.') % params
       end
-    elsif missing_domain
+    elsif missing_auto_devops_domain?(project)
       _('Auto Review Apps and Auto Deploy need a domain name to work correctly.')
     end
   end
+
+  private
+
+  def missing_auto_devops_domain?(project)
+    !(project.auto_devops || project.build_auto_devops)&.has_domain?
+  end
+
+  def missing_auto_devops_service?(project)
+    !project.deployment_platform&.active?
+  end
 end

app/models/application_setting.rb

@@ -117,6 +117,11 @@ class ApplicationSetting < ActiveRecord::Base
   validates :repository_storages, presence: true
   validate :check_repository_storages
 
+  validates :auto_devops_domain,
+            allow_blank: true,
+            hostname: { allow_numeric_hostname: true, require_valid_tld: true },
+            if: :auto_devops_enabled?
+
   validates :enabled_git_access_protocol,
             inclusion: { in: %w(ssh http), allow_blank: true, allow_nil: true }
 

app/models/project.rb

@@ -1601,7 +1601,7 @@ class Project < ActiveRecord::Base
   def auto_devops_variables
     return [] unless auto_devops_enabled?
 
-    auto_devops&.variables || []
+    (auto_devops || build_auto_devops)&.variables
   end
 
   def append_or_update_attribute(name, value)

app/models/project_auto_devops.rb

@@ -6,13 +6,17 @@ class ProjectAutoDevops < ActiveRecord::Base
 
   validates :domain, allow_blank: true, hostname: { allow_numeric_hostname: true }
 
+  def instance_domain
+    Gitlab::CurrentSettings.auto_devops_domain
+  end
+
   def has_domain?
-    domain.present?
+    domain.present? || instance_domain.present?
   end
 
   def variables
     variables = []
-    variables << { key: 'AUTO_DEVOPS_DOMAIN', value: domain, public: true } if domain.present?
+    variables << { key: 'AUTO_DEVOPS_DOMAIN', value: domain.presence || instance_domain, public: true } if has_domain?
     variables
   end
 end

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

@@ -249,7 +249,12 @@
         .help-block
           It will automatically build, test, and deploy applications based on a predefined CI/CD configuration
           = link_to icon('question-circle'), help_page_path('topics/autodevops/index.md')
-
+    .form-group
+      = f.label :auto_devops_domain, class: 'control-label col-sm-2'
+      .col-sm-10
+        = f.text_field :auto_devops_domain, class: 'form-control', placeholder: 'domain.com'
+        .help-block
+          = s_("AdminSettings|Specify a domain to use by default for every project's Auto Review Apps and Auto Deploy stages.")
     .form-group
       .col-sm-offset-2.col-sm-10
         .checkbox

changelogs/unreleased/38175-add-domain-field-to-auto-devops-application-setting.yml

+---
+title: Add Auto DevOps Domain application setting
+merge_request: 16604
+author:
+type: changed

db/migrate/20180122162010_add_auto_devops_domain_to_application_settings.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddAutoDevopsDomainToApplicationSettings < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    add_column :application_settings, :auto_devops_domain, :string
+  end
+end

db/schema.rb

@@ -155,6 +155,7 @@ ActiveRecord::Schema.define(version: 20180206200543) do
     t.integer "gitaly_timeout_medium", default: 30, null: false
     t.integer "gitaly_timeout_fast", default: 10, null: false
     t.boolean "authorized_keys_enabled", default: true, null: false
+    t.string "auto_devops_domain"
   end
 
   create_table "audit_events", force: :cascade do |t|

spec/features/admin/admin_settings_spec.rb

@@ -47,6 +47,16 @@ feature 'Admin updates settings' do
     expect(page).to have_content "Application settings saved successfully"
   end
 
+  scenario 'Change AutoDevOps settings' do
+    check 'Enabled Auto DevOps (Beta) for projects by default'
+    fill_in 'Auto devops domain', with: 'domain.com'
+    click_button 'Save'
+
+    expect(Gitlab::CurrentSettings.auto_devops_enabled?).to be true
+    expect(Gitlab::CurrentSettings.auto_devops_domain).to eq('domain.com')
+    expect(page).to have_content "Application settings saved successfully"
+  end
+
   scenario 'Change Slack Notifications Service template settings' do
     first(:link, 'Service Templates').click
     click_link 'Slack notifications'

spec/helpers/auto_devops_helper_spec.rb

@@ -82,4 +82,39 @@ describe AutoDevopsHelper do
       it { is_expected.to eq(false) }
     end
   end
+
+  describe '.auto_devops_warning_message' do
+    subject { helper.auto_devops_warning_message(project) }
+
+    context 'when the service is missing' do
+      before do
+        allow(helper).to receive(:missing_auto_devops_service?).and_return(true)
+      end
+
+      context 'when the domain is missing' do
+        before do
+          allow(helper).to receive(:missing_auto_devops_domain?).and_return(true)
+        end
+
+        it { is_expected.to match(/Auto Review Apps and Auto Deploy need a domain name and a .* to work correctly./) }
+      end
+
+      context 'when the domain is not missing' do
+        before do
+          allow(helper).to receive(:missing_auto_devops_domain?).and_return(false)
+        end
+
+        it { is_expected.to match(/Auto Review Apps and Auto Deploy need a .* to work correctly./) }
+      end
+    end
+
+    context 'when the domain is missing' do
+      before do
+        allow(helper).to receive(:missing_auto_devops_service?).and_return(false)
+        allow(helper).to receive(:missing_auto_devops_domain?).and_return(true)
+      end
+
+      it { is_expected.to eq('Auto Review Apps and Auto Deploy need a domain name to work correctly.') }
+    end
+  end
 end

spec/models/application_setting_spec.rb

@@ -114,6 +114,40 @@ describe ApplicationSetting do
       it { expect(setting.repository_storages).to eq(['default']) }
     end
 
+    context 'auto_devops_domain setting' do
+      context 'when auto_devops_enabled? is true' do
+        before do
+          setting.update(auto_devops_enabled: true)
+        end
+
+        it 'can be blank' do
+          setting.update(auto_devops_domain: '')
+
+          expect(setting).to be_valid
+        end
+
+        context 'with a valid value' do
+          before do
+            setting.update(auto_devops_domain: 'domain.com')
+          end
+
+          it 'is valid' do
+            expect(setting).to be_valid
+          end
+        end
+
+        context 'with an invalid value' do
+          before do
+            setting.update(auto_devops_domain: 'definitelynotahostname')
+          end
+
+          it 'is invalid' do
+            expect(setting).to be_invalid
+          end
+        end
+      end
+    end
+
     context 'circuitbreaker settings' do
       [:circuitbreaker_failure_count_threshold,
        :circuitbreaker_check_interval,

spec/models/project_auto_devops_spec.rb

@@ -18,9 +18,23 @@ describe ProjectAutoDevops do
     context 'when domain is empty' do
       let(:auto_devops) { build_stubbed(:project_auto_devops, project: project, domain: '') }
 
+      context 'when there is an instance domain specified' do
+        before do
+          allow(Gitlab::CurrentSettings).to receive(:auto_devops_domain).and_return('example.com')
+        end
+
+        it { expect(auto_devops).to have_domain }
+      end
+
+      context 'when there is no instance domain specified' do
+        before do
+          allow(Gitlab::CurrentSettings).to receive(:auto_devops_domain).and_return(nil)
+        end
+
         it { expect(auto_devops).not_to have_domain }
       end
     end
+  end
 
   describe '#variables' do
     let(:auto_devops) { build_stubbed(:project_auto_devops, project: project, domain: domain) }
@@ -29,9 +43,32 @@ describe ProjectAutoDevops do
       let(:domain) { 'example.com' }
 
       it 'returns AUTO_DEVOPS_DOMAIN' do
-        expect(auto_devops.variables).to include(
-          { key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true })
+        expect(auto_devops.variables).to include(domain_variable)
+      end
+    end
+
+    context 'when domain is not defined' do
+      let(:domain) { nil }
+
+      context 'when there is an instance domain specified' do
+        before do
+          allow(Gitlab::CurrentSettings).to receive(:auto_devops_domain).and_return('example.com')
+        end
+
+        it { expect(auto_devops.variables).to include(domain_variable) }
+      end
+
+      context 'when there is no instance domain specified' do
+        before do
+          allow(Gitlab::CurrentSettings).to receive(:auto_devops_domain).and_return(nil)
+        end
+
+        it { expect(auto_devops.variables).not_to include(domain_variable) }
       end
     end
+
+    def domain_variable
+      { key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true }
+    end
   end
 end

spec/models/project_spec.rb

@@ -3009,18 +3009,40 @@ describe Project do
 
     subject { project.auto_devops_variables }
 
-    context 'when enabled in settings' do
+    context 'when enabled in instance settings' do
       before do
         stub_application_setting(auto_devops_enabled: true)
       end
 
+      context 'when domain is empty' do
+        before do
+          stub_application_setting(auto_devops_domain: nil)
+        end
+
+        it 'variables does not include AUTO_DEVOPS_DOMAIN' do
+          is_expected.not_to include(domain_variable)
+        end
+      end
+
+      context 'when domain is configured' do
+        before do
+          stub_application_setting(auto_devops_domain: 'example.com')
+        end
+
+        it 'variables includes AUTO_DEVOPS_DOMAIN' do
+          is_expected.to include(domain_variable)
+        end
+      end
+    end
+
+    context 'when explicitely enabled' do
       context 'when domain is empty' do
         before do
           create(:project_auto_devops, project: project, domain: nil)
         end
 
-        it 'variables are empty' do
-          is_expected.to be_empty
+        it 'variables does not include AUTO_DEVOPS_DOMAIN' do
+          is_expected.not_to include(domain_variable)
         end
       end
 
@@ -3029,11 +3051,15 @@ describe Project do
           create(:project_auto_devops, project: project, domain: 'example.com')
         end
 
-        it "variables are not empty" do
-          is_expected.not_to be_empty
+        it 'variables includes AUTO_DEVOPS_DOMAIN' do
+          is_expected.to include(domain_variable)
         end
       end
     end
+
+    def domain_variable
+      { key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true }
+    end
   end
 
   describe '#latest_successful_builds_for' do