Merge branch '11-0-stable-prepare-rc1' into '11-0-stable'

Prepare 11.0 RC1 release

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

20 files changed, 198 insertions, 58 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index d3daab78940..1679ae378c9 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -1,4 +1,4 @@
-image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.7-golang-1.9-git-2.17-chrome-65.0-node-8.x-yarn-1.2-postgresql-9.6"
+image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.4.4-golang-1.9-git-2.17-chrome-65.0-node-8.x-yarn-1.2-postgresql-9.6"
 
 .dedicated-runner: &dedicated-runner
   retry: 1
@@ -6,7 +6,7 @@ image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.7-golang-1.9-git
     - gitlab-org
 
 .default-cache: &default-cache
-  key: "ruby-2.3.7-debian-stretch-with-yarn"
+  key: "ruby-2.4.4-debian-stretch-with-yarn"
   paths:
     - vendor/ruby
     - .yarn-cache/
@@ -550,7 +550,7 @@ static-analysis:
   script:
     - scripts/static-analysis
   cache:
-    key: "ruby-2.3.7-debian-stretch-with-yarn-and-rubocop"
+    key: "ruby-2.4.4-debian-stretch-with-yarn-and-rubocop"
     paths:
       - vendor/ruby
       - .yarn-cache/
diff --git a/.ruby-version b/.ruby-version
index 00355e29d11..79a614418f7 100644
--- a/.ruby-version
+++ b/.ruby-version
@@ -1 +1 @@
-2.3.7
+2.4.4
diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb
index ba6552f238f..25eac5160f1 100644
--- a/app/models/clusters/platforms/kubernetes.rb
+++ b/app/models/clusters/platforms/kubernetes.rb
@@ -11,12 +11,12 @@ module Clusters
 
       attr_encrypted :password,
         mode: :per_attribute_iv,
-        key: Gitlab::Application.secrets.db_key_base,
+        key: Settings.attr_encrypted_db_key_base,
         algorithm: 'aes-256-cbc'
 
       attr_encrypted :token,
         mode: :per_attribute_iv,
-        key: Gitlab::Application.secrets.db_key_base,
+        key: Settings.attr_encrypted_db_key_base,
         algorithm: 'aes-256-cbc'
 
       before_validation :enforce_namespace_to_lower_case
diff --git a/app/models/clusters/providers/gcp.rb b/app/models/clusters/providers/gcp.rb
index 7fac32466ab..eb2e42fd3fe 100644
--- a/app/models/clusters/providers/gcp.rb
+++ b/app/models/clusters/providers/gcp.rb
@@ -11,7 +11,7 @@ module Clusters
 
       attr_encrypted :access_token,
         mode: :per_attribute_iv,
-        key: Gitlab::Application.secrets.db_key_base,
+        key: Settings.attr_encrypted_db_key_base,
         algorithm: 'aes-256-cbc'
 
       validates :gcp_project_id,
diff --git a/app/models/concerns/cacheable_attributes.rb b/app/models/concerns/cacheable_attributes.rb
index b32459fdabf..d58d7165969 100644
--- a/app/models/concerns/cacheable_attributes.rb
+++ b/app/models/concerns/cacheable_attributes.rb
@@ -6,15 +6,16 @@ module CacheableAttributes
   end
 
   class_methods do
+    def cache_key
+      "#{name}:#{Gitlab::VERSION}:#{Gitlab.migrations_hash}:#{Rails.version}".freeze
+    end
+
     # Can be overriden
     def current_without_cache
       last
     end
 
-    def cache_key
-      "#{name}:#{Gitlab::VERSION}:#{Gitlab.migrations_hash}:json".freeze
-    end
-
+    # Can be overriden
     def defaults
       {}
     end
@@ -24,10 +25,18 @@ module CacheableAttributes
     end
 
     def cached
-      json_attributes = Rails.cache.read(cache_key)
-      return nil unless json_attributes.present?
+      if RequestStore.active?
+        RequestStore[:"#{name}_cached_attributes"] ||= retrieve_from_cache
+      else
+        retrieve_from_cache
+      end
+    end
+
+    def retrieve_from_cache
+      record = Rails.cache.read(cache_key)
+      ensure_cache_setup if record.present?
 
-      build_from_defaults(JSON.parse(json_attributes))
+      record
     end
 
     def current
@@ -35,7 +44,12 @@ module CacheableAttributes
       return cached_record if cached_record.present?
 
       current_without_cache.tap { |current_record| current_record&.cache! }
-    rescue
+    rescue => e
+      if Rails.env.production?
+        Rails.logger.warn("Cached record for #{name} couldn't be loaded, falling back to uncached record: #{e}")
+      else
+        raise e
+      end
       # Fall back to an uncached value if there are any problems (e.g. Redis down)
       current_without_cache
     end
@@ -46,9 +60,15 @@ module CacheableAttributes
       # Gracefully handle when Redis is not available. For example,
       # omnibus may fail here during gitlab:assets:compile.
     end
+
+    def ensure_cache_setup
+      # This is a workaround for a Rails bug that causes attribute methods not
+      # to be loaded when read from cache: https://github.com/rails/rails/issues/27348
+      define_attribute_methods
+    end
   end
 
   def cache!
-    Rails.cache.write(self.class.cache_key, attributes.to_json)
+    Rails.cache.write(self.class.cache_key, self)
   end
 end
diff --git a/app/models/concerns/has_variable.rb b/app/models/concerns/has_variable.rb
index 8a241e4374a..c8e20c0ab81 100644
--- a/app/models/concerns/has_variable.rb
+++ b/app/models/concerns/has_variable.rb
@@ -13,7 +13,7 @@ module HasVariable
     attr_encrypted :value,
        mode: :per_attribute_iv_and_salt,
        insecure_mode: true,
-       key: Gitlab::Application.secrets.db_key_base,
+       key: Settings.attr_encrypted_db_key_base,
        algorithm: 'aes-256-cbc'
 
     def key=(new_key)
diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb
index 2e478a24778..bfea64c3759 100644
--- a/app/models/pages_domain.rb
+++ b/app/models/pages_domain.rb
@@ -19,7 +19,7 @@ class PagesDomain < ActiveRecord::Base
   attr_encrypted :key,
     mode: :per_attribute_iv_and_salt,
     insecure_mode: true,
-    key: Gitlab::Application.secrets.db_key_base,
+    key: Settings.attr_encrypted_db_key_base,
     algorithm: 'aes-256-cbc'
 
   after_initialize :set_verification_code
diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb
index 6da6632f4f2..1d7089ccfc7 100644
--- a/app/models/project_import_data.rb
+++ b/app/models/project_import_data.rb
@@ -3,7 +3,7 @@ require 'carrierwave/orm/activerecord'
 class ProjectImportData < ActiveRecord::Base
   belongs_to :project, inverse_of: :import_data
   attr_encrypted :credentials,
-                 key: Gitlab::Application.secrets.db_key_base,
+                 key: Settings.attr_encrypted_db_key_base,
                  marshal: true,
                  encode: true,
                  mode: :per_attribute_iv_and_salt,
diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb
index bbf8fd9c6a7..aba1f2f384f 100644
--- a/app/models/remote_mirror.rb
+++ b/app/models/remote_mirror.rb
@@ -5,7 +5,7 @@ class RemoteMirror < ActiveRecord::Base
   UNPROTECTED_BACKOFF_DELAY = 5.minutes
 
   attr_encrypted :credentials,
-                 key: Gitlab::Application.secrets.db_key_base,
+                 key: Settings.attr_encrypted_db_key_base,
                  marshal: true,
                  encode: true,
                  mode: :per_attribute_iv_and_salt,
diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb
index 4291631913a..925775aea0b 100644
--- a/app/services/ci/register_job_service.rb
+++ b/app/services/ci/register_job_service.rb
@@ -89,7 +89,10 @@ module Ci
     end
 
     def builds_for_group_runner
-      hierarchy_groups = Gitlab::GroupHierarchy.new(runner.groups).base_and_descendants
+      # Workaround for weird Rails bug, that makes `runner.groups.to_sql` to return `runner_id = NULL`
+      groups = ::Group.joins(:runner_namespaces).merge(runner.runner_namespaces)
+
+      hierarchy_groups = Gitlab::GroupHierarchy.new(groups).base_and_descendants
       projects = Project.where(namespace_id: hierarchy_groups)
         .with_group_runners_enabled
         .with_builds_enabled
diff --git a/config/initializers/secret_token.rb b/config/initializers/01_secret_token.rb
index 750a5b34f3b..02bded43083 100644
--- a/config/initializers/secret_token.rb
+++ b/config/initializers/01_secret_token.rb
@@ -1,3 +1,6 @@
+# This file needs to be loaded BEFORE any initializers that attempt to
+# prepend modules that require access to secrets (e.g. EE's 0_as_concern.rb).
+#
 # Be sure to restart your server when you modify this file.
 
 require 'securerandom'
diff --git a/config/settings.rb b/config/settings.rb
index 69d637761ea..4aa903109ea 100644
--- a/config/settings.rb
+++ b/config/settings.rb
@@ -85,6 +85,10 @@ class Settings < Settingslogic
       File.expand_path(path, Rails.root)
     end
 
+    def attr_encrypted_db_key_base
+      Gitlab::Application.secrets.db_key_base[0..31]
+    end
+
     private
 
     def base_url(config)
diff --git a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb
index 611767ac7fe..95105118764 100644
--- a/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb
+++ b/db/migrate/20160302152808_remove_wrong_import_url_from_projects.rb
@@ -8,7 +8,7 @@ class RemoveWrongImportUrlFromProjects < ActiveRecord::Migration
     extend AttrEncrypted
     attr_accessor :credentials
     attr_encrypted :credentials,
-                   key: Gitlab::Application.secrets.db_key_base,
+                   key: Settings.attr_encrypted_db_key_base,
                    marshal: true,
                    encode: true,
                    :mode => :per_attribute_iv_and_salt,
diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb
index 11b581e4b57..1586a7eb92f 100644
--- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb
+++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb
@@ -48,7 +48,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati
 
     attr_encrypted :token,
       mode: :per_attribute_iv,
-      key: Gitlab::Application.secrets.db_key_base,
+      key: Settings.attr_encrypted_db_key_base,
       algorithm: 'aes-256-cbc'
   end
 
diff --git a/doc/install/installation.md b/doc/install/installation.md
index a0ae9017f71..34268c67140 100644
--- a/doc/install/installation.md
+++ b/doc/install/installation.md
@@ -133,9 +133,9 @@ Remove the old Ruby 1.8 if present:
 Download Ruby and compile it:
 
     mkdir /tmp/ruby && cd /tmp/ruby
-    curl --remote-name --progress https://cache.ruby-lang.org/pub/ruby/2.3/ruby-2.3.7.tar.gz
-    echo '540996fec64984ab6099e34d2f5820b14904f15a  ruby-2.3.7.tar.gz' | shasum -c - && tar xzf ruby-2.3.7.tar.gz
-    cd ruby-2.3.7
+    curl --remote-name --progress https://cache.ruby-lang.org/pub/ruby/2.4/ruby-2.4.4.tar.gz
+    echo 'ec82b0d53bd0adad9b19e6b45e44d54e9ec3f10c  ruby-2.4.4.tar.gz' | shasum -c - && tar xzf ruby-2.4.4.tar.gz
+    cd ruby-2.4.4
 
     ./configure --disable-install-rdoc
     make
diff --git a/lib/feature.rb b/lib/feature.rb
index 6474de6e56d..314ae224d90 100644
--- a/lib/feature.rb
+++ b/lib/feature.rb
@@ -63,8 +63,15 @@ class Feature
     end
 
     def flipper
-      Thread.current[:flipper] ||=
-        Flipper.new(flipper_adapter).tap { |flip| flip.memoize = true }
+      if RequestStore.active?
+        RequestStore[:flipper] ||= build_flipper_instance
+      else
+        @flipper ||= build_flipper_instance
+      end
+    end
+
+    def build_flipper_instance
+      Flipper.new(flipper_adapter).tap { |flip| flip.memoize = true }
     end
 
     # This method is called from config/initializers/flipper.rb and can be used
diff --git a/spec/initializers/secret_token_spec.rb b/spec/initializers/secret_token_spec.rb
index d56e14e0e0b..c3dfd7bedbe 100644
--- a/spec/initializers/secret_token_spec.rb
+++ b/spec/initializers/secret_token_spec.rb
@@ -1,5 +1,5 @@
 require 'spec_helper'
-require_relative '../../config/initializers/secret_token'
+require_relative '../../config/initializers/01_secret_token'
 
 describe 'create_tokens' do
   include StubENV
diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb
index 10020511bf8..6eb10497428 100644
--- a/spec/lib/feature_spec.rb
+++ b/spec/lib/feature_spec.rb
@@ -64,4 +64,28 @@ describe Feature do
       expect(described_class.all).to eq(features.to_a)
     end
   end
+
+  describe '.flipper' do
+    shared_examples 'a memoized Flipper instance' do
+      it 'memoizes the Flipper instance' do
+        expect(Flipper).to receive(:new).once.and_call_original
+
+        2.times do
+          described_class.flipper
+        end
+      end
+    end
+
+    context 'when request store is inactive' do
+      before do
+        described_class.instance_variable_set(:@flipper, nil)
+      end
+
+      it_behaves_like 'a memoized Flipper instance'
+    end
+
+    context 'when request store is inactive', :request_store do
+      it_behaves_like 'a memoized Flipper instance'
+    end
+  end
 end
diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb
index 49e4b23ebc7..c6331c5ec15 100644
--- a/spec/models/concerns/cacheable_attributes_spec.rb
+++ b/spec/models/concerns/cacheable_attributes_spec.rb
@@ -22,7 +22,7 @@ describe CacheableAttributes do
 
       attr_accessor :attributes
 
-      def initialize(attrs = {})
+      def initialize(attrs = {}, *)
         @attributes = attrs
       end
     end
@@ -52,7 +52,7 @@ describe CacheableAttributes do
 
   describe '.cache_key' do
     it 'excludes cache attributes' do
-      expect(minimal_test_class.cache_key).to eq("TestClass:#{Gitlab::VERSION}:#{Gitlab.migrations_hash}:json")
+      expect(minimal_test_class.cache_key).to eq("TestClass:#{Gitlab::VERSION}:#{Gitlab.migrations_hash}:#{Rails.version}")
     end
   end
 
@@ -75,49 +75,117 @@ describe CacheableAttributes do
 
     context 'without any attributes given' do
       it 'intializes a new object with the defaults' do
-        expect(minimal_test_class.build_from_defaults).not_to be_persisted
+        expect(minimal_test_class.build_from_defaults.attributes).to eq(minimal_test_class.defaults)
       end
     end
 
-    context 'without attributes given' do
+    context 'with attributes given' do
       it 'intializes a new object with the given attributes merged into the defaults' do
         expect(minimal_test_class.build_from_defaults(foo: 'd').attributes[:foo]).to eq('d')
       end
     end
+
+    describe 'edge cases on concrete implementations' do
+      describe '.build_from_defaults' do
+        context 'without any attributes given' do
+          it 'intializes all attributes even if they are nil' do
+            record = ApplicationSetting.build_from_defaults
+
+            expect(record).not_to be_persisted
+            expect(record.sign_in_text).to be_nil
+          end
+        end
+      end
+    end
   end
 
   describe '.current', :use_clean_rails_memory_store_caching do
     context 'redis unavailable' do
-      it 'returns an uncached record' do
+      before do
         allow(minimal_test_class).to receive(:last).and_return(:last)
-        expect(Rails.cache).to receive(:read).and_raise(Redis::BaseError)
+        expect(Rails.cache).to receive(:read).with(minimal_test_class.cache_key).and_raise(Redis::BaseError)
+      end
+
+      context 'in production environment' do
+        before do
+          expect(Rails.env).to receive(:production?).and_return(true)
+        end
+
+        it 'returns an uncached record and logs a warning' do
+          expect(Rails.logger).to receive(:warn).with("Cached record for TestClass couldn't be loaded, falling back to uncached record: Redis::BaseError")
 
-        expect(minimal_test_class.current).to eq(:last)
+          expect(minimal_test_class.current).to eq(:last)
+        end
+      end
+
+      context 'in other environments' do
+        before do
+          expect(Rails.env).to receive(:production?).and_return(false)
+        end
+
+        it 'returns an uncached record and logs a warning' do
+          expect(Rails.logger).not_to receive(:warn)
+
+          expect { minimal_test_class.current }.to raise_error(Redis::BaseError)
+        end
       end
     end
 
     context 'when a record is not yet present' do
       it 'does not cache nil object' do
         # when missing settings a nil object is returned, but not cached
-        allow(minimal_test_class).to receive(:last).twice.and_return(nil)
+        allow(ApplicationSetting).to receive(:current_without_cache).twice.and_return(nil)
 
-        expect(minimal_test_class.current).to be_nil
-        expect(Rails.cache.exist?(minimal_test_class.cache_key)).to be(false)
+        expect(ApplicationSetting.current).to be_nil
+        expect(Rails.cache.exist?(ApplicationSetting.cache_key)).to be(false)
       end
 
-      it 'cache non-nil object' do
-        # when the settings are set the method returns a valid object
-        allow(minimal_test_class).to receive(:last).and_call_original
+      it 'caches non-nil object' do
+        create(:application_setting)
 
-        expect(minimal_test_class.current).to eq(minimal_test_class.last)
-        expect(Rails.cache.exist?(minimal_test_class.cache_key)).to be(true)
+        expect(ApplicationSetting.current).to eq(ApplicationSetting.last)
+        expect(Rails.cache.exist?(ApplicationSetting.cache_key)).to be(true)
 
         # subsequent calls retrieve the record from the cache
-        last_record = minimal_test_class.last
-        expect(minimal_test_class).not_to receive(:last)
-        expect(minimal_test_class.current.attributes).to eq(last_record.attributes)
+        last_record = ApplicationSetting.last
+        expect(ApplicationSetting).not_to receive(:current_without_cache)
+        expect(ApplicationSetting.current.attributes).to eq(last_record.attributes)
       end
     end
+
+    describe 'edge cases' do
+      describe 'caching behavior', :use_clean_rails_memory_store_caching do
+        it 'retrieves upload fields properly' do
+          ar_record = create(:appearance, :with_logo)
+          ar_record.cache!
+
+          cache_record = Appearance.current
+
+          expect(cache_record).to be_persisted
+          expect(cache_record.logo).to be_an(AttachmentUploader)
+          expect(cache_record.logo.url).to end_with('/dk.png')
+        end
+
+        it 'retrieves markdown fields properly' do
+          ar_record = create(:appearance, description: '**Hello**')
+          ar_record.cache!
+
+          cache_record = Appearance.current
+
+          expect(cache_record.description).to eq('**Hello**')
+          expect(cache_record.description_html).to eq('<p dir="auto"><strong>Hello</strong></p>')
+        end
+      end
+    end
+
+    it 'uses RequestStore in addition to Rails.cache', :request_store do
+      # Warm up the cache
+      create(:application_setting).cache!
+
+      expect(Rails.cache).to receive(:read).with(ApplicationSetting.cache_key).once.and_call_original
+
+      2.times { ApplicationSetting.current }
+    end
   end
 
   describe '.cached', :use_clean_rails_memory_store_caching do
@@ -127,27 +195,36 @@ describe CacheableAttributes do
       end
     end
 
-    context 'when cached settings do not include the latest defaults' do
+    context 'when cached is warm' do
       before do
-        Rails.cache.write(minimal_test_class.cache_key, { bar: 'b', baz: 'c' }.to_json)
-        minimal_test_class.define_singleton_method(:defaults) do
-          { foo: 'a', bar: 'b', baz: 'c' }
-        end
+        # Warm up the cache
+        create(:appearance).cache!
       end
 
-      it 'includes attributes from defaults' do
-        expect(minimal_test_class.cached.attributes[:foo]).to eq(minimal_test_class.defaults[:foo])
+      it 'retrieves the record from cache' do
+        expect(ActiveRecord::QueryRecorder.new { Appearance.cached }.count).to eq(0)
+        expect(Appearance.cached).to eq(Appearance.current_without_cache)
       end
     end
   end
 
   describe '#cache!', :use_clean_rails_memory_store_caching do
-    let(:appearance_record) { create(:appearance) }
+    let(:record) { create(:appearance) }
 
     it 'caches the attributes' do
-      appearance_record.cache!
+      record.cache!
 
-      expect(Rails.cache.read(Appearance.cache_key)).to eq(appearance_record.attributes.to_json)
+      expect(Rails.cache.read(Appearance.cache_key)).to eq(record)
+    end
+
+    describe 'edge cases' do
+      let(:record) { create(:appearance) }
+
+      it 'caches the attributes' do
+        record.cache!
+
+        expect(Rails.cache.read(Appearance.cache_key)).to eq(record)
+      end
     end
   end
 end
diff --git a/spec/models/concerns/has_variable_spec.rb b/spec/models/concerns/has_variable_spec.rb
index f87869a2fdc..3fbe86c5b56 100644
--- a/spec/models/concerns/has_variable_spec.rb
+++ b/spec/models/concerns/has_variable_spec.rb
@@ -45,8 +45,10 @@ describe HasVariable do
     end
 
     it 'fails to decrypt if iv is incorrect' do
-      subject.encrypted_value_iv = SecureRandom.hex
+      # attr_encrypted expects the IV to be 16 bytes and base64-encoded
+      subject.encrypted_value_iv = [SecureRandom.hex(8)].pack('m')
       subject.instance_variable_set(:@value, nil)
+
       expect { subject.value }
         .to raise_error(OpenSSL::Cipher::CipherError, 'bad decrypt')
     end