Merge branch 'ce-to-ee-2018-04-27' into 'master'

CE upstream - 2018-04-27 18:24 UTC

Closes gitlab-development-kit#342, gitlab-ce#45045, and gitaly#314

See merge request gitlab-org/gitlab-ee!5504

.gitignore

@@ -73,3 +73,4 @@ eslint-report.html
 /locale/**/*.time_stamp
 /.rspec
 /plugins/*
+/.gitlab_pages_secret

GITLAB_PAGES_VERSION

-0.8.0
+0.9.0

app/views/projects/settings/ci_cd/_autodevops_form.html.haml

@@ -36,5 +36,6 @@
             = form.text_field :domain, class: 'form-control', placeholder: 'domain.com'
             .help-block
               = s_('CICD|You need to specify a domain if you want to use Auto Review Apps and Auto Deploy stages.')
+              = link_to icon('question-circle'), help_page_path('topics/autodevops/index.md', anchor: 'auto-devops-base-domain'), target: '_blank'
 
           = f.submit 'Save changes', class: "btn btn-success prepend-top-15"

changelogs/unreleased/zj-repository-exist-mandatory.yml

+---
+title: Repository#exists? is always executed through Gitaly
+merge_request:
+author:
+type: performance

config/gitlab.yml.example

@@ -184,7 +184,7 @@ production: &base
     # base_dir: uploads/-/system
     object_store:
       enabled: false
-      # remote_directory: uploads # Bucket name
+      remote_directory: uploads # Bucket name
       # direct_upload: false # Use Object Storage directly for uploads instead of background uploads if enabled (Default: false)
       # background_upload: false # Temporary option to limit automatic upload (Default: true)
       # proxy_download: false # Passthrough all downloads via GitLab instead of using Redirects to Object Storage
@@ -212,6 +212,8 @@ production: &base
     artifacts_server: true
     # external_http: ["1.1.1.1:80", "[2001::1]:80"] # If defined, enables custom domain support in GitLab Pages
     # external_https: ["1.1.1.1:443", "[2001::1]:443"] # If defined, enables custom domain and certificate support in GitLab Pages
+    admin:
+      address: unix:/home/git/gitlab/tmp/sockets/private/pages-admin.socket # TCP connections are supported too (e.g. tcp://host:port)
 
   ## Mattermost
   ## For enabling Add to Mattermost button

config/initializers/1_settings.rb

@@ -232,6 +232,9 @@ Settings.pages['external_http']     ||= false unless Settings.pages['external_ht
 Settings.pages['external_https']    ||= false unless Settings.pages['external_https'].present?
 Settings.pages['artifacts_server']  ||= Settings.pages['enabled'] if Settings.pages['artifacts_server'].nil?
 
+Settings.pages['admin'] ||= Settingslogic.new({})
+Settings.pages.admin['certificate'] ||= ''
+
 #
 # Geo
 #

config/initializers/pages.rb

+Gitlab::PagesClient.read_or_create_token
+Gitlab::PagesClient.load_certificate

doc/topics/autodevops/index.md

@@ -135,6 +135,11 @@ and `1.2.3.4` is the IP address of your load balancer; generally NGINX
 ([see prerequisites](#prerequisites)). How to set up the DNS record is beyond
 the scope of this document; you should check with your DNS provider.
 
+Alternatively you can use free public services like [xip.io](http://xip.io) or
+[nip.io](http://nip.io) which provide automatic wildcard DNS without any
+configuration. Just set the Auto DevOps base domain to `1.2.3.4.xip.io` or
+`1.2.3.4.nip.io`.
+
 Once set up, all requests will hit the load balancer, which in turn will route
 them to the Kubernetes pods that run your application(s).
 

ee/spec/services/projects/update_repository_storage_service_spec.rb

@@ -19,6 +19,9 @@ describe Projects::UpdateRepositoryStorageService do
       stub_storage_settings(storages)
 
       allow(Time).to receive(:now).and_return(time)
+
+      # This will force the creation of the repository and bypass Gitaly
+      allow_any_instance_of(Gitlab::Git::Repository).to receive(:exists?).and_return(false)
     end
 
     after do
@@ -64,7 +67,9 @@ describe Projects::UpdateRepositoryStorageService do
       let(:wiki_repository_double) { double(:repository) }
 
       before do
+        allow_any_instance_of(Gitlab::Git::Repository).to receive(:exists?).and_return(false)
         project.create_wiki
+        allow_any_instance_of(Gitlab::Git::Repository).to receive(:exists?).and_return(true)
 
         # Default stub for non-specified params
         allow(Gitlab::Git::Repository).to receive(:new).and_call_original

lib/gitlab/git/repository.rb

@@ -142,15 +142,7 @@ module Gitlab
       end
 
       def exists?
-        Gitlab::GitalyClient.migrate(:repository_exists, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |enabled|
-          if enabled
         gitaly_repository_client.exists?
-          else
-            circuit_breaker.perform do
-              File.exist?(File.join(path, 'refs'))
-            end
-          end
-        end
       end
 
       # Returns an Array of branch names

lib/gitlab/pages_client.rb

+module Gitlab
+  class PagesClient
+    class << self
+      attr_reader :certificate, :token
+
+      def call(service, rpc, request, timeout: nil)
+        kwargs = request_kwargs(timeout)
+        stub(service).__send__(rpc, request, kwargs) # rubocop:disable GitlabSecurity/PublicSend
+      end
+
+      # This function is not thread-safe. Call it from an initializer only.
+      def read_or_create_token
+        @token = read_token
+      rescue Errno::ENOENT
+        # TODO: uncomment this when omnibus knows how to write the token file for us
+        # https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/2466
+        #
+        # write_token(SecureRandom.random_bytes(64))
+        #
+        # # Read from disk in case someone else won the race and wrote the file
+        # # before us. If this fails again let the exception bubble up.
+        # @token = read_token
+      end
+
+      # This function is not thread-safe. Call it from an initializer only.
+      def load_certificate
+        cert_path = config.certificate
+        return unless cert_path.present?
+
+        @certificate = File.read(cert_path)
+      end
+
+      def ping
+        request = Grpc::Health::V1::HealthCheckRequest.new
+        call(:health_check, :check, request, timeout: 5.seconds)
+      end
+
+      private
+
+      def request_kwargs(timeout)
+        encoded_token = Base64.strict_encode64(token.to_s)
+        metadata = {
+          'authorization' => "Bearer #{encoded_token}"
+        }
+
+        result = { metadata: metadata }
+
+        return result unless timeout
+
+        # Do not use `Time.now` for deadline calculation, since it
+        # will be affected by Timecop in some tests, but grpc's c-core
+        # uses system time instead of timecop's time, so tests will fail
+        # `Time.at(Process.clock_gettime(Process::CLOCK_REALTIME))` will
+        # circumvent timecop
+        deadline = Time.at(Process.clock_gettime(Process::CLOCK_REALTIME)) + timeout
+        result[:deadline] = deadline
+
+        result
+      end
+
+      def stub(name)
+        stub_class(name).new(address, grpc_creds)
+      end
+
+      def stub_class(name)
+        if name == :health_check
+          Grpc::Health::V1::Health::Stub
+        else
+          # TODO use pages namespace
+          Gitaly.const_get(name.to_s.camelcase.to_sym).const_get(:Stub)
+        end
+      end
+
+      def address
+        addr = config.address
+        addr = addr.sub(%r{^tcp://}, '') if URI(addr).scheme == 'tcp'
+        addr
+      end
+
+      def grpc_creds
+        if address.start_with?('unix:')
+          :this_channel_is_insecure
+        elsif @certificate
+          GRPC::Core::ChannelCredentials.new(@certificate)
+        else
+          # Use system certificate pool
+          GRPC::Core::ChannelCredentials.new
+        end
+      end
+
+      def config
+        Gitlab.config.pages.admin
+      end
+
+      def read_token
+        File.read(token_path)
+      end
+
+      def token_path
+        Rails.root.join('.gitlab_pages_secret').to_s
+      end
+
+      def write_token(new_token)
+        Tempfile.open(File.basename(token_path), File.dirname(token_path),  encoding: 'ascii-8bit') do |f|
+          f.write(new_token)
+          f.close
+          File.link(f.path, token_path)
+        end
+      rescue Errno::EACCES => ex
+        # TODO stop rescuing this exception in GitLab 11.0 https://gitlab.com/gitlab-org/gitlab-ce/issues/45672
+        Rails.logger.error("Could not write pages admin token file: #{ex}")
+      rescue Errno::EEXIST
+        # Another process wrote the token file concurrently with us. Use their token, not ours.
+      end
+    end
+  end
+end

lib/tasks/gitlab/pages.rake

+namespace :gitlab do
+  namespace :pages do
+    desc 'Ping the pages admin API'
+    task admin_ping: :gitlab_environment do
+      Gitlab::PagesClient.ping
+      puts "OK: gitlab-pages admin API is reachable"
+    end
+  end
+end

spec/lib/gitlab/pages_client_spec.rb

+require 'spec_helper'
+
+describe Gitlab::PagesClient do
+  subject { described_class }
+
+  describe '.token' do
+    it 'returns the token as it is on disk' do
+      pending 'add omnibus support for generating the secret file https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/2466'
+      expect(subject.token).to eq(File.read('.gitlab_pages_secret'))
+    end
+  end
+
+  describe '.read_or_create_token' do
+    subject { described_class.read_or_create_token }
+    let(:token_path) { 'tmp/tests/gitlab-pages-secret' }
+    before do
+      allow(described_class).to receive(:token_path).and_return(token_path)
+      FileUtils.rm_f(token_path)
+    end
+
+    it 'uses the existing token file if it exists' do
+      secret = 'existing secret'
+      File.write(token_path, secret)
+
+      subject
+      expect(described_class.token).to eq(secret)
+    end
+
+    it 'creates one if none exists' do
+      pending 'add omnibus support for generating the secret file https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests/2466'
+
+      old_token = described_class.token
+      # sanity check
+      expect(File.exist?(token_path)).to eq(false)
+
+      subject
+      expect(described_class.token.bytesize).to eq(64)
+      expect(described_class.token).not_to eq(old_token)
+    end
+  end
+
+  describe '.write_token' do
+    let(:token_path) { 'tmp/tests/gitlab-pages-secret' }
+    before do
+      allow(described_class).to receive(:token_path).and_return(token_path)
+      FileUtils.rm_f(token_path)
+    end
+
+    it 'writes the secret' do
+      new_secret = 'hello new secret'
+      expect(File.exist?(token_path)).to eq(false)
+
+      described_class.send(:write_token, new_secret)
+
+      expect(File.read(token_path)).to eq(new_secret)
+    end
+
+    it 'does nothing if the file already exists' do
+      existing_secret = 'hello secret'
+      File.write(token_path, existing_secret)
+
+      described_class.send(:write_token, 'new secret')
+
+      expect(File.read(token_path)).to eq(existing_secret)
+    end
+  end
+
+  describe '.load_certificate' do
+    subject { described_class.load_certificate }
+    before do
+      allow(described_class).to receive(:config).and_return(config)
+    end
+
+    context 'with no certificate in the config' do
+      let(:config) { double(:config, certificate: '') }
+
+      it 'does not set @certificate' do
+        subject
+
+        expect(described_class.certificate).to be_nil
+      end
+    end
+
+    context 'with a certificate path in the config' do
+      let(:certificate_path) { 'tmp/tests/fake-certificate' }
+      let(:config) { double(:config, certificate: certificate_path) }
+
+      it 'sets @certificate' do
+        certificate_data = "--- BEGIN CERTIFICATE ---\nbla\n--- END CERTIFICATE ---\n"
+        File.write(certificate_path, certificate_data)
+        subject
+
+        expect(described_class.certificate).to eq(certificate_data)
+      end
+    end
+  end
+
+  describe '.request_kwargs' do
+    let(:token) { 'secret token' }
+    let(:auth_header) { 'Bearer c2VjcmV0IHRva2Vu' }
+    before do
+      allow(described_class).to receive(:token).and_return(token)
+    end
+
+    context 'without timeout' do
+      it { expect(subject.send(:request_kwargs, nil)[:metadata]['authorization']).to eq(auth_header) }
+    end
+
+    context 'with timeout' do
+      let(:timeout) { 1.second }
+
+      it 'still sets the authorization header' do
+        expect(subject.send(:request_kwargs, timeout)[:metadata]['authorization']).to eq(auth_header)
+      end
+
+      it 'sets a deadline value' do
+        now = Time.now
+        deadline = subject.send(:request_kwargs, timeout)[:deadline]
+
+        expect(deadline).to be_between(now, now + 2 * timeout)
+      end
+    end
+  end
+
+  describe '.stub' do
+    before do
+      allow(described_class).to receive(:address).and_return('unix:/foo/bar')
+    end
+
+    it { expect(subject.send(:stub, :health_check)).to be_a(Grpc::Health::V1::Health::Stub) }
+  end
+
+  describe '.address' do
+    subject { described_class.send(:address) }
+
+    before do
+      allow(described_class).to receive(:config).and_return(config)
+    end
+
+    context 'with a unix: address' do
+      let(:config) { double(:config, address: 'unix:/foo/bar') }
+
+      it { expect(subject).to eq('unix:/foo/bar') }
+    end
+
+    context 'with a tcp:// address' do
+      let(:config) { double(:config, address: 'tcp://localhost:1234') }
+
+      it { expect(subject).to eq('localhost:1234') }
+    end
+  end
+
+  describe '.grpc_creds' do
+    subject { described_class.send(:grpc_creds) }
+
+    before do
+      allow(described_class).to receive(:config).and_return(config)
+    end
+
+    context 'with a unix: address' do
+      let(:config) { double(:config, address: 'unix:/foo/bar') }
+
+      it { expect(subject).to eq(:this_channel_is_insecure) }
+    end
+
+    context 'with a tcp:// address' do
+      let(:config) { double(:config, address: 'tcp://localhost:1234') }
+
+      it { expect(subject).to be_a(GRPC::Core::ChannelCredentials) }
+    end
+  end
+end

spec/models/repository_spec.rb

@@ -1256,15 +1256,15 @@ describe Repository do
     end
   end
 
-  shared_examples 'repo exists check' do
+  describe '#exists?' do
     it 'returns true when a repository exists' do
-      expect(repository.exists?).to eq(true)
+      expect(repository.exists?).to be(true)
     end
 
     it 'returns false if no full path can be constructed' do
       allow(repository).to receive(:full_path).and_return(nil)
 
-      expect(repository.exists?).to eq(false)
+      expect(repository.exists?).to be(false)
     end
 
     context 'with broken storage', :broken_storage do
@@ -1274,16 +1274,6 @@ describe Repository do
     end
   end
 
-  describe '#exists?' do
-    context 'when repository_exists is disabled' do
-      it_behaves_like 'repo exists check'
-    end
-
-    context 'when repository_exists is enabled', :skip_gitaly_mock do
-      it_behaves_like 'repo exists check'
-    end
-  end
-
   describe '#has_visible_content?' do
     before do
       # If raw_repository.has_visible_content? gets called more than once then