Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-07-11

# Conflicts:
#	locale/gitlab.pot

[ci skip]

app/helpers/pipeline_schedules_helper.rb

@@ -3,7 +3,7 @@ module PipelineSchedulesHelper
     ActiveSupport::TimeZone.all.map do |timezone|
       {
         name: timezone.name,
-        offset: timezone.utc_offset,
+        offset: timezone.now.utc_offset,
         identifier: timezone.tzinfo.identifier
       }
     end

app/workers/git_garbage_collect_worker.rb

@@ -65,10 +65,10 @@ class GitGarbageCollectWorker
       client.repack_incremental
     end
   rescue GRPC::NotFound => e
-    Gitlab::GitLogger.error("#{method} failed:\nRepository not found")
+    Gitlab::GitLogger.error("#{__method__} failed:\nRepository not found")
     raise Gitlab::Git::Repository::NoRepository.new(e)
   rescue GRPC::BadStatus => e
-    Gitlab::GitLogger.error("#{method} failed:\n#{e}")
+    Gitlab::GitLogger.error("#{__method__} failed:\n#{e}")
     raise Gitlab::Git::CommandError.new(e)
   end
 

changelogs/unreleased/add-dst-support-to-pipeline-schedule.yml

+---
+title: Add support for daylight savings time to pipleline schedules
+merge_request: 20145
+author:
+type: fixed

changelogs/unreleased/runners-max-timeout-param.yml

+---
+title: Add missing maximum_timeout parameter
+merge_request: 20355 
+author: gfyoung
+type: fixed

changelogs/unreleased/sh-handle-colons-in-url-passwords.yml

+---
+title: Properly handle colons in URL passwords
+merge_request:
+author:
+type: fixed

doc/development/what_requires_downtime.md

@@ -198,14 +198,14 @@ And that's it, we're done!
 ## Changing The Schema For Large Tables
 
 While `change_column_type_concurrently` and `rename_column_concurrently` can be
-used for changing the schema of a table without downtime it doesn't work very
+used for changing the schema of a table without downtime, it doesn't work very
 well for large tables. Because all of the work happens in sequence the migration
 can take a very long time to complete, preventing a deployment from proceeding.
 They can also produce a lot of pressure on the database due to it rapidly
 updating many rows in sequence.
 
 To reduce database pressure you should instead use
-`change_column_type_using_background_migration` or `rename_column_concurrently`
+`change_column_type_using_background_migration` or `rename_column_using_background_migration`
 when migrating a column in a large table (e.g. `issues`). These methods work
 similarly to the concurrent counterparts but uses background migration to spread
 the work / load over a longer time period, without slowing down deployments.

lib/api/runners.rb

@@ -58,7 +58,7 @@ module API
         optional :access_level, type: String, values: Ci::Runner.access_levels.keys,
                                 desc: 'The access_level of the runner'
         optional :maximum_timeout, type: Integer, desc: 'Maximum timeout set when this Runner will handle the job'
-        at_least_one_of :description, :active, :tag_list, :run_untagged, :locked, :access_level
+        at_least_one_of :description, :active, :tag_list, :run_untagged, :locked, :access_level, :maximum_timeout
       end
       put ':id' do
         runner = get_runner(params.delete(:id))

lib/declarative_policy.rb

@@ -21,7 +21,17 @@ module DeclarativePolicy
       cache = opts[:cache] || {}
       key = Cache.policy_key(user, subject)
 
-      cache[key] ||= class_for(subject).new(user, subject, opts)
+      cache[key] ||=
+        if Gitlab.rails5?
+          # to avoid deadlocks in multi-threaded environment when
+          # autoloading is enabled, we allow concurrent loads,
+          # https://gitlab.com/gitlab-org/gitlab-ce/issues/48263
+          ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
+            class_for(subject).new(user, subject, opts)
+          end
+        else
+          class_for(subject).new(user, subject, opts)
+        end
     end
 
     def class_for(subject)

lib/gitlab/url_sanitizer.rb

@@ -58,7 +58,7 @@ module Gitlab
       if raw_credentials.present?
         url.sub!("#{raw_credentials}@", '')
 
-        user, password = raw_credentials.split(':')
+        user, _, password = raw_credentials.partition(':')
         @credentials ||= { user: user.presence, password: password.presence }
       end
 

lib/tasks/gettext.rake

@@ -20,16 +20,22 @@ namespace :gettext do
   end
 
   task :regenerate do
+    pot_file = 'locale/gitlab.pot'
     # Remove all translated files, this speeds up finding
     FileUtils.rm Dir['locale/**/gitlab.*']
     # remove the `pot` file to ensure it's completely regenerated
-    FileUtils.rm_f 'locale/gitlab.pot'
+    FileUtils.rm_f pot_file
 
     Rake::Task['gettext:find'].invoke
 
     # leave only the required changes.
     `git checkout -- locale/*/gitlab.po`
 
+    # Remove timestamps from the pot file
+    pot_content = File.read pot_file
+    pot_content.gsub!(/^"POT?\-(?:Creation|Revision)\-Date\:.*\n/, '')
+    File.write pot_file, pot_content
+
     puts <<~MSG
     All done. Please commit the changes to `locale/gitlab.pot`.
 

locale/gitlab.pot

@@ -8,8 +8,11 @@ msgid ""
 msgstr ""
 "Project-Id-Version: gitlab 1.0.0\n"
 "Report-Msgid-Bugs-To: \n"
+<<<<<<< HEAD
 "POT-Creation-Date: 2018-07-10 13:57-0500\n"
 "PO-Revision-Date: 2018-07-10 13:57-0500\n"
+=======
+>>>>>>> upstream/master
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
 "Language: \n"

spec/javascripts/helpers/wait_for_promises.js

+export default () => new Promise(resolve => requestAnimationFrame(resolve));

spec/javascripts/job_spec.js

@@ -5,6 +5,7 @@ import { numberToHumanSize } from '~/lib/utils/number_utils';
 import '~/lib/utils/datetime_utility';
 import Job from '~/job';
 import '~/breakpoints';
+import waitForPromises from 'spec/helpers/wait_for_promises';
 
 describe('Job', () => {
   const JOB_URL = `${gl.TEST_HOST}/frontend-fixtures/builds-project/-/jobs/1`;
@@ -12,10 +13,6 @@ describe('Job', () => {
   let response;
   let job;
 
-  function waitForPromise() {
-    return new Promise(resolve => requestAnimationFrame(resolve));
-  }
-
   preloadFixtures('builds/build-with-artifacts.html.raw');
 
   beforeEach(() => {
@@ -49,7 +46,7 @@ describe('Job', () => {
       beforeEach(function (done) {
         job = new Job();
 
-        waitForPromise()
+        waitForPromises()
           .then(done)
           .catch(done.fail);
       });
@@ -93,7 +90,7 @@ describe('Job', () => {
 
         job = new Job();
 
-        waitForPromise()
+        waitForPromises()
           .then(() => {
             expect($('#build-trace .js-build-output').text()).toMatch(/Update/);
             expect(job.state).toBe('newstate');
@@ -107,7 +104,7 @@ describe('Job', () => {
             };
           })
           .then(() => jasmine.clock().tick(4001))
-          .then(waitForPromise)
+          .then(waitForPromises)
           .then(() => {
             expect($('#build-trace .js-build-output').text()).toMatch(/UpdateMore/);
             expect(job.state).toBe('finalstate');
@@ -126,7 +123,7 @@ describe('Job', () => {
 
         job = new Job();
 
-        waitForPromise()
+        waitForPromises()
           .then(() => {
             expect($('#build-trace .js-build-output').text()).toMatch(/Update/);
 
@@ -137,7 +134,7 @@ describe('Job', () => {
             };
           })
           .then(() => jasmine.clock().tick(4001))
-          .then(waitForPromise)
+          .then(waitForPromises)
           .then(() => {
             expect($('#build-trace .js-build-output').text()).not.toMatch(/Update/);
             expect($('#build-trace .js-build-output').text()).toMatch(/Different/);
@@ -160,7 +157,7 @@ describe('Job', () => {
 
           job = new Job();
 
-          waitForPromise()
+          waitForPromises()
             .then(() => {
               expect(document.querySelector('.js-truncated-info').classList).not.toContain('hidden');
             })
@@ -181,7 +178,7 @@ describe('Job', () => {
 
           job = new Job();
 
-          waitForPromise()
+          waitForPromises()
             .then(() => {
               expect(
                 document.querySelector('.js-truncated-info-size').textContent.trim(),
@@ -203,7 +200,7 @@ describe('Job', () => {
 
           job = new Job();
 
-          waitForPromise()
+          waitForPromises()
             .then(() => {
               expect(
                 document.querySelector('.js-truncated-info-size').textContent.trim(),
@@ -219,7 +216,7 @@ describe('Job', () => {
               };
             })
             .then(() => jasmine.clock().tick(4001))
-            .then(waitForPromise)
+            .then(waitForPromises)
             .then(() => {
               expect(
                 document.querySelector('.js-truncated-info-size').textContent.trim(),
@@ -258,7 +255,7 @@ describe('Job', () => {
 
           job = new Job();
 
-          waitForPromise()
+          waitForPromises()
             .then(() => {
               expect(document.querySelector('.js-truncated-info').classList).toContain('hidden');
             })
@@ -280,7 +277,7 @@ describe('Job', () => {
 
         job = new Job();
 
-        waitForPromise()
+        waitForPromises()
           .then(done)
           .catch(done.fail);
       });

spec/javascripts/smart_interval_spec.js

 import $ from 'jquery';
 import _ from 'underscore';
 import SmartInterval from '~/smart_interval';
+import waitForPromises from 'spec/helpers/wait_for_promises';
 
 describe('SmartInterval', function () {
   const DEFAULT_MAX_INTERVAL = 100;
   const DEFAULT_STARTING_INTERVAL = 5;
   const DEFAULT_SHORT_TIMEOUT = 75;
-  const DEFAULT_LONG_TIMEOUT = 1000;
   const DEFAULT_INCREMENT_FACTOR = 2;
 
   function createDefaultSmartInterval(config) {
@@ -27,52 +27,65 @@ describe('SmartInterval', function () {
     return new SmartInterval(defaultParams);
   }
 
+  beforeEach(() => {
+    jasmine.clock().install();
+  });
+
+  afterEach(() => {
+    jasmine.clock().uninstall();
+  });
+
   describe('Increment Interval', function () {
-    beforeEach(function () {
-      this.smartInterval = createDefaultSmartInterval();
-    });
+    it('should increment the interval delay', (done) => {
+      const smartInterval = createDefaultSmartInterval();
 
-    it('should increment the interval delay', function (done) {
-      const interval = this.smartInterval;
-      setTimeout(() => {
-        const intervalConfig = this.smartInterval.cfg;
-        const iterationCount = 4;
-        const maxIntervalAfterIterations = intervalConfig.startingInterval *
-          (intervalConfig.incrementByFactorOf ** (iterationCount - 1)); // 40
-        const currentInterval = interval.getCurrentInterval();
-
-        // Provide some flexibility for performance of testing environment
-        expect(currentInterval).toBeGreaterThan(intervalConfig.startingInterval);
-        expect(currentInterval <= maxIntervalAfterIterations).toBeTruthy();
-
-        done();
-      }, DEFAULT_SHORT_TIMEOUT); // 4 iterations, increment by 2x = (5 + 10 + 20 + 40)
+      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
+
+      waitForPromises()
+        .then(() => {
+          const intervalConfig = smartInterval.cfg;
+          const iterationCount = 4;
+          const maxIntervalAfterIterations = intervalConfig.startingInterval *
+            (intervalConfig.incrementByFactorOf ** iterationCount);
+          const currentInterval = smartInterval.getCurrentInterval();
+
+          // Provide some flexibility for performance of testing environment
+          expect(currentInterval).toBeGreaterThan(intervalConfig.startingInterval);
+          expect(currentInterval).toBeLessThanOrEqual(maxIntervalAfterIterations);
+        })
+        .then(done)
+        .catch(done.fail);
     });
 
-    it('should not increment past maxInterval', function (done) {
-      const interval = this.smartInterval;
+    it('should not increment past maxInterval', (done) => {
+      const smartInterval = createDefaultSmartInterval({ maxInterval: DEFAULT_STARTING_INTERVAL });
 
-      setTimeout(() => {
-        const currentInterval = interval.getCurrentInterval();
-        expect(currentInterval).toBe(interval.cfg.maxInterval);
+      jasmine.clock().tick(DEFAULT_STARTING_INTERVAL);
+      jasmine.clock().tick(DEFAULT_STARTING_INTERVAL * DEFAULT_INCREMENT_FACTOR);
 
-        done();
-      }, DEFAULT_LONG_TIMEOUT);
+      waitForPromises()
+        .then(() => {
+          const currentInterval = smartInterval.getCurrentInterval();
+          expect(currentInterval).toBe(smartInterval.cfg.maxInterval);
+        })
+        .then(done)
+        .catch(done.fail);
     });
 
-    it('does not increment while waiting for callback', function () {
-      jasmine.clock().install();
-
+    it('does not increment while waiting for callback', done => {
       const smartInterval = createDefaultSmartInterval({
         callback: () => new Promise($.noop),
       });
 
       jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
 
-      const oneInterval = smartInterval.cfg.startingInterval * DEFAULT_INCREMENT_FACTOR;
-      expect(smartInterval.getCurrentInterval()).toEqual(oneInterval);
-
-      jasmine.clock().uninstall();
+      waitForPromises()
+        .then(() => {
+          const oneInterval = smartInterval.cfg.startingInterval * DEFAULT_INCREMENT_FACTOR;
+          expect(smartInterval.getCurrentInterval()).toEqual(oneInterval);
+        })
+        .then(done)
+        .catch(done.fail);
     });
   });
 
@@ -84,34 +97,39 @@ describe('SmartInterval', function () {
     it('should cancel an interval', function (done) {
       const interval = this.smartInterval;
 
-      setTimeout(() => {
-        interval.cancel();
+      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
 
-        const { intervalId } = interval.state;
-        const currentInterval = interval.getCurrentInterval();
-        const intervalLowerLimit = interval.cfg.startingInterval;
+      interval.cancel();
 
-        expect(intervalId).toBeUndefined();
-        expect(currentInterval).toBe(intervalLowerLimit);
+      waitForPromises()
+        .then(() => {
+          const { intervalId } = interval.state;
+          const currentInterval = interval.getCurrentInterval();
+          const intervalLowerLimit = interval.cfg.startingInterval;
 
-        done();
-      }, DEFAULT_SHORT_TIMEOUT);
+          expect(intervalId).toBeUndefined();
+          expect(currentInterval).toBe(intervalLowerLimit);
+        })
+        .then(done)
+        .catch(done.fail);
     });
 
     it('should resume an interval', function (done) {
       const interval = this.smartInterval;
 
-      setTimeout(() => {
-        interval.cancel();
-
-        interval.resume();
+      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
 
-        const { intervalId } = interval.state;
+      interval.cancel();
 
-        expect(intervalId).toBeTruthy();
+      interval.resume();
 
-        done();
-      }, DEFAULT_SHORT_TIMEOUT);
+      waitForPromises()
+        .then(() => {
+          const { intervalId } = interval.state;
+          expect(intervalId).toBeTruthy();
+        })
+        .then(done)
+        .catch(done.fail);
     });
   });
 
@@ -126,64 +144,79 @@ describe('SmartInterval', function () {
     it('should pause when page is not visible', function (done) {
       const interval = this.smartInterval;
 
-      setTimeout(() => {
-        expect(interval.state.intervalId).toBeTruthy();
+      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
+
+      waitForPromises()
+        .then(() => {
+          expect(interval.state.intervalId).toBeTruthy();
 
-        // simulates triggering of visibilitychange event
-        interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
+          // simulates triggering of visibilitychange event
+          interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
 
-        expect(interval.state.intervalId).toBeUndefined();
-        done();
-      }, DEFAULT_SHORT_TIMEOUT);
+          expect(interval.state.intervalId).toBeUndefined();
+        })
+        .then(done)
+        .catch(done.fail);
     });
 
-    it('should change to the hidden interval when page is not visible', function (done) {
+    it('should change to the hidden interval when page is not visible', done => {
       const HIDDEN_INTERVAL = 1500;
       const interval = createDefaultSmartInterval({ hiddenInterval: HIDDEN_INTERVAL });
 
-      setTimeout(() => {
-        expect(interval.state.intervalId).toBeTruthy();
-        expect(interval.getCurrentInterval() >= DEFAULT_STARTING_INTERVAL &&
-          interval.getCurrentInterval() <= DEFAULT_MAX_INTERVAL).toBeTruthy();
+      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
+
+      waitForPromises()
+        .then(() => {
+          expect(interval.state.intervalId).toBeTruthy();
+          expect(interval.getCurrentInterval() >= DEFAULT_STARTING_INTERVAL &&
+            interval.getCurrentInterval() <= DEFAULT_MAX_INTERVAL).toBeTruthy();
 
-        // simulates triggering of visibilitychange event
-        interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
+          // simulates triggering of visibilitychange event
+          interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
 
-        expect(interval.state.intervalId).toBeTruthy();
-        expect(interval.getCurrentInterval()).toBe(HIDDEN_INTERVAL);
-        done();
-      }, DEFAULT_SHORT_TIMEOUT);
+          expect(interval.state.intervalId).toBeTruthy();
+          expect(interval.getCurrentInterval()).toBe(HIDDEN_INTERVAL);
+        })
+        .then(done)
+        .catch(done.fail);
     });
 
     it('should resume when page is becomes visible at the previous interval', function (done) {
       const interval = this.smartInterval;
 
-      setTimeout(() => {
-        expect(interval.state.intervalId).toBeTruthy();
+      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
 
-        // simulates triggering of visibilitychange event
-        interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
+      waitForPromises()
+        .then(() => {
+          expect(interval.state.intervalId).toBeTruthy();
 
-        expect(interval.state.intervalId).toBeUndefined();
+          // simulates triggering of visibilitychange event
+          interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
 
-        // simulates triggering of visibilitychange event
-        interval.handleVisibilityChange({ target: { visibilityState: 'visible' } });
+          expect(interval.state.intervalId).toBeUndefined();
 
-        expect(interval.state.intervalId).toBeTruthy();
+          // simulates triggering of visibilitychange event
+          interval.handleVisibilityChange({ target: { visibilityState: 'visible' } });
 
-        done();
-      }, DEFAULT_SHORT_TIMEOUT);
+          expect(interval.state.intervalId).toBeTruthy();
+        })
+        .then(done)
+        .catch(done.fail);
     });
 
     it('should cancel on page unload', function (done) {
       const interval = this.smartInterval;
 
-      setTimeout(() => {
-        $(document).triggerHandler('beforeunload');
-        expect(interval.state.intervalId).toBeUndefined();
-        expect(interval.getCurrentInterval()).toBe(interval.cfg.startingInterval);
-        done();
-      }, DEFAULT_SHORT_TIMEOUT);
+      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
+
+      waitForPromises()
+        .then(() => {
+          $(document).triggerHandler('beforeunload');
+          expect(interval.state.intervalId).toBeUndefined();
+          expect(interval.getCurrentInterval()).toBe(interval.cfg.startingInterval);
+        })
+        .then(done)
+        .catch(done.fail);
     });
 
     it('should execute callback before first interval', function () {

spec/lib/gitlab/url_sanitizer_spec.rb

@@ -92,6 +92,7 @@ describe Gitlab::UrlSanitizer do
     context 'credentials in URL' do
       where(:url, :credentials) do
         'http://foo:bar@example.com' | { user: 'foo', password: 'bar' }
+        'http://foo:bar:baz@example.com' | { user: 'foo', password: 'bar:baz' }
         'http://:bar@example.com'    | { user: nil,   password: 'bar' }
         'http://foo:@example.com'    | { user: 'foo', password: nil }
         'http://foo@example.com'     | { user: 'foo', password: nil }

spec/requests/api/runners_spec.rb

@@ -211,6 +211,69 @@ describe API::Runners do
 
   describe 'PUT /runners/:id' do
     context 'admin user' do
+      # see https://gitlab.com/gitlab-org/gitlab-ce/issues/48625
+      context 'single parameter update' do
+        it 'runner description' do
+          description = shared_runner.description
+          update_runner(shared_runner.id, admin, description: "#{description}_updated")
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(shared_runner.reload.description).to eq("#{description}_updated")
+        end
+
+        it 'runner active state' do
+          active = shared_runner.active
+          update_runner(shared_runner.id, admin, active: !active)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(shared_runner.reload.active).to eq(!active)
+        end
+
+        it 'runner tag list' do
+          update_runner(shared_runner.id, admin, tag_list: ['ruby2.1', 'pgsql', 'mysql'])
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(shared_runner.reload.tag_list).to include('ruby2.1', 'pgsql', 'mysql')
+        end
+
+        it 'runner untagged flag' do
+          # Ensure tag list is non-empty before setting untagged to false.
+          update_runner(shared_runner.id, admin, tag_list: ['ruby2.1', 'pgsql', 'mysql'])
+          update_runner(shared_runner.id, admin, run_untagged: 'false')
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(shared_runner.reload.run_untagged?).to be(false)
+        end
+
+        it 'runner unlocked flag' do
+          update_runner(shared_runner.id, admin, locked: 'true')
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(shared_runner.reload.locked?).to be(true)
+        end
+
+        it 'runner access level' do
+          update_runner(shared_runner.id, admin, access_level: 'ref_protected')
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(shared_runner.reload.ref_protected?).to be_truthy
+        end
+
+        it 'runner maximum timeout' do
+          update_runner(shared_runner.id, admin, maximum_timeout: 1234)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(shared_runner.reload.maximum_timeout).to eq(1234)
+        end
+
+        it 'fails with no parameters' do
+          put api("/runners/#{shared_runner.id}", admin)
+
+          shared_runner.reload
+          expect(response).to have_gitlab_http_status(400)
+        end
+      end
+
       context 'when runner is shared' do
         it 'updates runner' do
           description = shared_runner.description

spec/workers/git_garbage_collect_worker_spec.rb

@@ -27,6 +27,12 @@ describe GitGarbageCollectWorker do
 
         subject.perform(project.id, :gc, lease_key, lease_uuid)
       end
+
+      it 'handles gRPC errors' do
+        expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect).and_raise(GRPC::NotFound)
+
+        expect { subject.perform(project.id, :gc, lease_key, lease_uuid) }.to raise_exception(Gitlab::Git::Repository::NoRepository)
+      end
     end
 
     context 'with different lease than the active one' do