Merge branch 'master' into 'fix/gb/improve-stage-id-foreign-key-migration'

# Conflicts:
#   db/schema.rb

CHANGELOG.md

@@ -2,6 +2,10 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 9.3.4 (2017-07-03)
+
+- No changes.
+
 ## 9.3.3 (2017-06-30)
 
 - Fix head pipeline stored in merge request for external pipelines. !12478

GITLAB_SHELL_VERSION

@@ -1 +1 @@
-5.0.6
+5.1.1

app/assets/javascripts/behaviors/autosize.js

@@ -1,23 +1,8 @@
 import autosize from 'vendor/autosize';
 
-$(() => {
-  const $fields = $('.js-autosize');
+document.addEventListener('DOMContentLoaded', () => {
+  const autosizeEls = document.querySelectorAll('.js-autosize');
 
-  $fields.on('autosize:resized', function resized() {
-    const $field = $(this);
-    $field.data('height', $field.outerHeight());
-  });
-
-  $fields.on('resize.autosize', function resize() {
-    const $field = $(this);
-    if ($field.data('height') !== $field.outerHeight()) {
-      $field.data('height', $field.outerHeight());
-      autosize.destroy($field);
-      $field.css('max-height', window.outerHeight);
-    }
-  });
-
-  autosize($fields);
-  autosize.update($fields);
-  $fields.css('resize', 'vertical');
+  autosize(autosizeEls);
+  autosize.update(autosizeEls);
 });

app/assets/javascripts/monitoring/components/monitoring_column.vue

@@ -104,7 +104,7 @@
         }
         this.data = query.result[0].values;
         this.unitOfDisplay = query.unit || 'N/A';
-        this.yAxisLabel = this.columnData.y_axis || 'Values';
+        this.yAxisLabel = this.columnData.y_label || 'Values';
         this.legendTitle = query.legend || 'Average';
         this.graphWidth = this.$refs.baseSvg.clientWidth -
                      this.margin.left - this.margin.right;

app/finders/labels_finder.rb

@@ -83,7 +83,12 @@ class LabelsFinder < UnionFinder
   def projects
     return @projects if defined?(@projects)
 
-    @projects = skip_authorization ? Project.all : ProjectsFinder.new(current_user: current_user).execute
+    @projects = if skip_authorization
+                  Project.all
+                else
+                  ProjectsFinder.new(params: { non_archived: true }, current_user: current_user).execute
+                end
+
     @projects = @projects.in_namespace(params[:group_id]) if group?
     @projects = @projects.where(id: params[:project_ids]) if projects?
     @projects = @projects.reorder(nil)

app/models/ci/variable.rb

@@ -5,7 +5,7 @@ module Ci
 
     belongs_to :project
 
-    validates :key, uniqueness: { scope: :project_id }
+    validates :key, uniqueness: { scope: [:project_id, :environment_scope] }
 
     scope :unprotected, -> { where(protected: false) }
   end

app/models/project.rb

@@ -1074,9 +1074,10 @@ class Project < ActiveRecord::Base
     merge_requests.where(source_project_id: self.id)
   end
 
-  def create_repository
+  def create_repository(force: false)
     # Forked import is handled asynchronously
-    unless forked?
+    return if forked? && !force
+
     if gitlab_shell.add_repository(repository_storage_path, path_with_namespace)
       repository.after_create
       true
@@ -1085,10 +1086,9 @@ class Project < ActiveRecord::Base
       false
     end
   end
-  end
 
   def ensure_repository
-    create_repository unless repository_exists?
+    create_repository(force: true) unless repository_exists?
   end
 
   def repository_exists?

app/views/shared/issuable/form/_merge_params.html.haml

@@ -11,8 +11,7 @@
   .col-sm-10.col-sm-offset-2
     - if issuable.can_remove_source_branch?(current_user)
       .checkbox
-        - initial_checkbox_value = issuable.merge_params.key?('force_remove_source_branch') ? issuable.force_remove_source_branch? : true
         = label_tag 'merge_request[force_remove_source_branch]' do
           = hidden_field_tag 'merge_request[force_remove_source_branch]', '0', id: nil
-          = check_box_tag 'merge_request[force_remove_source_branch]', '1', initial_checkbox_value
+          = check_box_tag 'merge_request[force_remove_source_branch]', '1', issuable.force_remove_source_branch?
           Remove source branch when merge request is accepted.

changelogs/unreleased/32885-unintentionally-removing-branch-when-merging-merge-request.yml

@@ -0,0 +1,4 @@
+---
+title: Set default for Remove source branch to false.
+merge_request: !12576
+author:

changelogs/unreleased/add-ci_variables-environment_scope-mysql.yml

@@ -0,0 +1,6 @@
+---
+title: Rename duplicated variables with the same key for projects. Add environment_scope
+  column to variables and add unique constraint to make sure that no variables could
+  be created with the same key within a project
+merge_request: 12363
+author:

changelogs/unreleased/hb-hide-archived-labels-from-group-issue-tracker.yml

@@ -0,0 +1,4 @@
+---
+title: Hide archived project labels from group issue tracker
+merge_request: 12547
+author: Horacio Bertorello

changelogs/unreleased/monitoring-dashboard-fix-y-label.yml

@@ -0,0 +1,4 @@
+---
+title: Fixed the y_label not setting correctly for each graph on the monitoring dashboard
+merge_request:
+author:

changelogs/unreleased/sh-allow-force-repo-create.yml

@@ -0,0 +1,4 @@
+---
+title: Make Project#ensure_repository force create a repo
+merge_request:
+author:

changelogs/unreleased/sh-optimize-project-commit-api.yml

@@ -0,0 +1,4 @@
+---
+title: Optimize creation of commit API by using Repository#commit instead of Repository#commits
+merge_request:
+author:

db/migrate/20170622135451_rename_duplicated_variable_key.rb

@@ -0,0 +1,38 @@
+class RenameDuplicatedVariableKey < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    execute(<<~SQL)
+      UPDATE ci_variables
+      SET #{key} = CONCAT(#{key}, #{underscore}, id)
+      WHERE id IN (
+        SELECT *
+        FROM ( -- MySQL requires an extra layer
+          SELECT dup.id
+          FROM ci_variables dup
+          INNER JOIN (SELECT max(id) AS id, #{key}, project_id
+                      FROM ci_variables tmp
+                      GROUP BY #{key}, project_id) var
+          USING (#{key}, project_id) where dup.id <> var.id
+        ) dummy
+      )
+    SQL
+  end
+
+  def down
+    # noop
+  end
+
+  def key
+    # key needs to be quoted in MySQL
+    quote_column_name('key')
+  end
+
+  def underscore
+    quote('_')
+  end
+end

db/migrate/20170622135628_add_environment_scope_to_ci_variables.rb

@@ -0,0 +1,15 @@
+class AddEnvironmentScopeToCiVariables < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:ci_variables, :environment_scope, :string, default: '*')
+  end
+
+  def down
+    remove_column(:ci_variables, :environment_scope)
+  end
+end

db/migrate/20170622135728_add_unique_constraint_to_ci_variables.rb

@@ -0,0 +1,38 @@
+class AddUniqueConstraintToCiVariables < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    unless this_index_exists?
+      add_concurrent_index(:ci_variables, columns, name: index_name, unique: true)
+    end
+  end
+
+  def down
+    if this_index_exists?
+      if Gitlab::Database.mysql? && !index_exists?(:ci_variables, :project_id)
+        # Need to add this index for MySQL project_id foreign key constraint
+        add_concurrent_index(:ci_variables, :project_id)
+      end
+
+      remove_concurrent_index(:ci_variables, columns, name: index_name)
+    end
+  end
+
+  private
+
+  def this_index_exists?
+    index_exists?(:ci_variables, columns, name: index_name)
+  end
+
+  def columns
+    @columns ||= [:project_id, :key, :environment_scope]
+  end
+
+  def index_name
+    'index_ci_variables_on_project_id_and_key_and_environment_scope'
+  end
+end

db/migrate/20170623080805_remove_ci_variables_project_id_index.rb

@@ -0,0 +1,19 @@
+class RemoveCiVariablesProjectIdIndex < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    if index_exists?(:ci_variables, :project_id)
+      remove_concurrent_index(:ci_variables, :project_id)
+    end
+  end
+
+  def down
+    unless index_exists?(:ci_variables, :project_id)
+      add_concurrent_index(:ci_variables, :project_id)
+    end
+  end
+end

db/schema.rb

@@ -374,9 +374,10 @@ ActiveRecord::Schema.define(version: 20170703102400) do
     t.string "encrypted_value_iv"
     t.integer "project_id", null: false
     t.boolean "protected", default: false, null: false
+    t.string "environment_scope", default: "*", null: false
   end
 
-  add_index "ci_variables", ["project_id"], name: "index_ci_variables_on_project_id", using: :btree
+  add_index "ci_variables", ["project_id", "key", "environment_scope"], name: "index_ci_variables_on_project_id_and_key_and_environment_scope", unique: true, using: :btree
 
   create_table "container_repositories", force: :cascade do |t|
     t.integer "project_id", null: false

lib/api/commits.rb

@@ -67,7 +67,7 @@ module API
         result = ::Files::MultiService.new(user_project, current_user, attrs).execute
 
         if result[:status] == :success
-          commit_detail = user_project.repository.commits(result[:result], limit: 1).first
+          commit_detail = user_project.repository.commit(result[:result])
           present commit_detail, with: Entities::RepoCommitDetail
         else
           render_api_error!(result[:message], 400)

lib/gitlab/shell.rb

@@ -2,6 +2,8 @@ require 'securerandom'
 
 module Gitlab
   class Shell
+    GITLAB_SHELL_ENV_VARS = %w(GIT_TERMINAL_PROMPT).freeze
+
     Error = Class.new(StandardError)
 
     KeyAdder = Struct.new(:io) do
@@ -67,7 +69,7 @@ module Gitlab
     #   add_repository("/path/to/storage", "gitlab/gitlab-ci")
     #
     def add_repository(storage, name)
-      Gitlab::Utils.system_silent([gitlab_shell_projects_path,
+      gitlab_shell_fast_execute([gitlab_shell_projects_path,
                                  'add-project', storage, "#{name}.git"])
     end
 
@@ -82,10 +84,9 @@ module Gitlab
     def import_repository(storage, name, url)
       # Timeout should be less than 900 ideally, to prevent the memory killer
       # to silently kill the process without knowing we are timing out here.
-      output, status = Popen.popen([gitlab_shell_projects_path, 'import-project',
-                                    storage, "#{name}.git", url, "#{Gitlab.config.gitlab_shell.git_timeout}"])
-      raise Error, output unless status.zero?
-      true
+      cmd = [gitlab_shell_projects_path, 'import-project',
+             storage, "#{name}.git", url, "#{Gitlab.config.gitlab_shell.git_timeout}"]
+      gitlab_shell_fast_execute_raise_error(cmd)
     end
 
     # Fetch remote for repository
@@ -103,9 +104,7 @@ module Gitlab
       args << '--force' if forced
       args << '--no-tags' if no_tags
 
-      output, status = Popen.popen(args)
-      raise Error, output unless status.zero?
-      true
+      gitlab_shell_fast_execute_raise_error(args)
     end
 
     # Move repository
@@ -117,7 +116,7 @@ module Gitlab
     #   mv_repository("/path/to/storage", "gitlab/gitlab-ci", "randx/gitlab-ci-new")
     #
     def mv_repository(storage, path, new_path)
-      Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'mv-project',
+      gitlab_shell_fast_execute([gitlab_shell_projects_path, 'mv-project',
                                  storage, "#{path}.git", "#{new_path}.git"])
     end
 
@@ -131,7 +130,7 @@ module Gitlab
     #  fork_repository("/path/to/forked_from/storage", "gitlab/gitlab-ci", "/path/to/forked_to/storage", "randx")
     #
     def fork_repository(forked_from_storage, path, forked_to_storage, fork_namespace)
-      Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'fork-project',
+      gitlab_shell_fast_execute([gitlab_shell_projects_path, 'fork-project',
                                  forked_from_storage, "#{path}.git", forked_to_storage,
                                  fork_namespace])
     end
@@ -145,7 +144,7 @@ module Gitlab
     #   remove_repository("/path/to/storage", "gitlab/gitlab-ci")
     #
     def remove_repository(storage, name)
-      Gitlab::Utils.system_silent([gitlab_shell_projects_path,
+      gitlab_shell_fast_execute([gitlab_shell_projects_path,
                                  'rm-project', storage, "#{name}.git"])
     end
 
@@ -155,7 +154,7 @@ module Gitlab
     #   add_key("key-42", "sha-rsa ...")
     #
     def add_key(key_id, key_content)
-      Gitlab::Utils.system_silent([gitlab_shell_keys_path,
+      gitlab_shell_fast_execute([gitlab_shell_keys_path,
                                  'add-key', key_id, self.class.strip_key(key_content)])
     end
 
@@ -175,8 +174,10 @@ module Gitlab
     #   remove_key("key-342", "sha-rsa ...")
     #
     def remove_key(key_id, key_content)
-      Gitlab::Utils.system_silent([gitlab_shell_keys_path,
-                                   'rm-key', key_id, key_content])
+      args = [gitlab_shell_keys_path, 'rm-key', key_id]
+      args << key_content if key_content
+
+      gitlab_shell_fast_execute(args)
     end
 
     # Remove all ssh keys from gitlab shell
@@ -185,7 +186,7 @@ module Gitlab
     #   remove_all_keys
     #
     def remove_all_keys
-      Gitlab::Utils.system_silent([gitlab_shell_keys_path, 'clear'])
+      gitlab_shell_fast_execute([gitlab_shell_keys_path, 'clear'])
     end
 
     # Add empty directory for storing repositories
@@ -267,5 +268,31 @@ module Gitlab
     def gitlab_shell_keys_path
       File.join(gitlab_shell_path, 'bin', 'gitlab-keys')
     end
+
+    private
+
+    def gitlab_shell_fast_execute(cmd)
+      output, status = gitlab_shell_fast_execute_helper(cmd)
+
+      return true if status.zero?
+
+      Rails.logger.error("gitlab-shell failed with error #{status}: #{output}")
+      false
+    end
+
+    def gitlab_shell_fast_execute_raise_error(cmd)
+      output, status = gitlab_shell_fast_execute_helper(cmd)
+
+      raise Error, output unless status.zero?
+      true
+    end
+
+    def gitlab_shell_fast_execute_helper(cmd)
+      vars = ENV.to_h.slice(*GITLAB_SHELL_ENV_VARS)
+
+      # Don't pass along the entire parent environment to prevent gitlab-shell
+      # from wasting I/O by searching through GEM_PATH
+      Bundler.with_original_env { Popen.popen(cmd, nil, vars) }
+    end
   end
 end

spec/finders/labels_finder_spec.rb

@@ -49,12 +49,12 @@ describe LabelsFinder do
     end
 
     context 'filtering by group_id' do
-      it 'returns labels available for any project within the group' do
+      it 'returns labels available for any non-archived project within the group' do
         group_1.add_developer(user)
-
+        project_1.archive!
         finder = described_class.new(user, group_id: group_1.id)
 
-        expect(finder.execute).to eq [group_label_2, project_label_1, group_label_1, project_label_5]
+        expect(finder.execute).to eq [group_label_2, group_label_1, project_label_5]
       end
     end
 

spec/javascripts/monitoring/mock_data.js

@@ -13,7 +13,7 @@ const metricsGroupsAPIResponse = {
             'queries': [
                 {
                   'query_range': 'avg(container_memory_usage_bytes{%{environment_filter}}) / 2^20',
-                  'label': 'Container memory',
+                  'y_label': 'Memory',
                   'unit': 'MiB',
                   'result': [
                     {
@@ -2477,7 +2477,7 @@ export const singleRowMetrics = [
   {
     'title': 'CPU usage',
     'weight': 1,
-    'y_label': 'Values',
+    'y_label': 'Memory',
     'queries': [
       {
         'query_range': 'avg(rate(container_cpu_usage_seconds_total{%{environment_filter}}[2m])) * 100',

spec/javascripts/monitoring/monitoring_column_spec.js

@@ -94,4 +94,15 @@ describe('MonitoringColumn', () => {
       done();
     });
   });
+
+  it('has a title for the y-axis that comes from the backend', () => {
+    const component = createComponent({
+      columnData: singleRowMetrics[0],
+      classType: 'col-md-6',
+      updateAspectRatio: false,
+      deploymentData,
+    });
+
+    expect(component.yAxisLabel).toEqual(component.columnData.y_label);
+  });
 });

spec/lib/gitlab/popen_spec.rb

@@ -32,6 +32,17 @@ describe 'Gitlab::Popen', lib: true, no_db: true do
     end
   end
 
+  context 'with custom options' do
+    let(:vars) { { 'foobar' => 123, 'PWD' => path } }
+    let(:options) { { chdir: path } }
+
+    it 'calls popen3 with the provided environment variables' do
+      expect(Open3).to receive(:popen3).with(vars, 'ls', options)
+
+      @output, @status = @klass.new.popen(%w(ls), path, { 'foobar' => 123 })
+    end
+  end
+
   context 'without a directory argument' do
     before do
       @output, @status = @klass.new.popen(%w(ls))

spec/lib/gitlab/shell_spec.rb

@@ -4,6 +4,7 @@ require 'stringio'
 describe Gitlab::Shell, lib: true do
   let(:project) { double('Project', id: 7, path: 'diaspora') }
   let(:gitlab_shell) { Gitlab::Shell.new }
+  let(:popen_vars) { { 'GIT_TERMINAL_PROMPT' => ENV['GIT_TERMINAL_PROMPT'] } }
 
   before do
     allow(Project).to receive(:find).and_return(project)
@@ -50,7 +51,7 @@ describe Gitlab::Shell, lib: true do
   describe '#add_key' do
     it 'removes trailing garbage' do
       allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path)
-      expect(Gitlab::Utils).to receive(:system_silent).with(
+      expect(gitlab_shell).to receive(:gitlab_shell_fast_execute).with(
         [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar']
       )
 
@@ -100,17 +101,91 @@ describe Gitlab::Shell, lib: true do
       allow(Gitlab.config.gitlab_shell).to receive(:git_timeout).and_return(800)
     end
 
+    describe '#add_repository' do
+      it 'returns true when the command succeeds' do
+        expect(Gitlab::Popen).to receive(:popen)
+          .with([projects_path, 'add-project', 'current/storage', 'project/path.git'],
+                nil, popen_vars).and_return([nil, 0])
+
+        expect(gitlab_shell.add_repository('current/storage', 'project/path')).to be true
+      end
+
+      it 'returns false when the command fails' do
+        expect(Gitlab::Popen).to receive(:popen)
+          .with([projects_path, 'add-project', 'current/storage', 'project/path.git'],
+                nil, popen_vars).and_return(["error", 1])
+
+        expect(gitlab_shell.add_repository('current/storage', 'project/path')).to be false
+      end
+    end
+
+    describe '#remove_repository' do
+      it 'returns true when the command succeeds' do
+        expect(Gitlab::Popen).to receive(:popen)
+          .with([projects_path, 'rm-project', 'current/storage', 'project/path.git'],
+                nil, popen_vars).and_return([nil, 0])
+
+        expect(gitlab_shell.remove_repository('current/storage', 'project/path')).to be true
+      end
+
+      it 'returns false when the command fails' do
+        expect(Gitlab::Popen).to receive(:popen)
+          .with([projects_path, 'rm-project', 'current/storage', 'project/path.git'],
+                nil, popen_vars).and_return(["error", 1])
+
+        expect(gitlab_shell.remove_repository('current/storage', 'project/path')).to be false
+      end
+    end
+
+    describe '#mv_repository' do
+      it 'returns true when the command succeeds' do
+        expect(Gitlab::Popen).to receive(:popen)
+          .with([projects_path, 'mv-project', 'current/storage', 'project/path.git', 'project/newpath.git'],
+                nil, popen_vars).and_return([nil, 0])
+
+        expect(gitlab_shell.mv_repository('current/storage', 'project/path', 'project/newpath')).to be true
+      end
+
+      it 'returns false when the command fails' do
+        expect(Gitlab::Popen).to receive(:popen)
+          .with([projects_path, 'mv-project', 'current/storage', 'project/path.git', 'project/newpath.git'],
+                nil, popen_vars).and_return(["error", 1])
+
+        expect(gitlab_shell.mv_repository('current/storage', 'project/path', 'project/newpath')).to be false
+      end
+    end
+
+    describe '#fork_repository' do
+      it 'returns true when the command succeeds' do
+        expect(Gitlab::Popen).to receive(:popen)
+          .with([projects_path, 'fork-project', 'current/storage', 'project/path.git', 'new/storage', 'new-namespace'],
+                nil, popen_vars).and_return([nil, 0])
+
+        expect(gitlab_shell.fork_repository('current/storage', 'project/path', 'new/storage', 'new-namespace')).to be true
+      end
+
+      it 'return false when the command fails' do
+        expect(Gitlab::Popen).to receive(:popen)
+          .with([projects_path, 'fork-project', 'current/storage', 'project/path.git', 'new/storage', 'new-namespace'],
+                nil, popen_vars).and_return(["error", 1])
+
+        expect(gitlab_shell.fork_repository('current/storage', 'project/path', 'new/storage', 'new-namespace')).to be false
+      end
+    end
+
     describe '#fetch_remote' do
       it 'returns true when the command succeeds' do
         expect(Gitlab::Popen).to receive(:popen)
-          .with([projects_path, 'fetch-remote', 'current/storage', 'project/path.git', 'new/storage', '800']).and_return([nil, 0])
+          .with([projects_path, 'fetch-remote', 'current/storage', 'project/path.git', 'new/storage', '800'],
+                nil, popen_vars).and_return([nil, 0])
 
         expect(gitlab_shell.fetch_remote('current/storage', 'project/path', 'new/storage')).to be true
       end
 
       it 'raises an exception when the command fails' do
         expect(Gitlab::Popen).to receive(:popen)
-        .with([projects_path, 'fetch-remote', 'current/storage', 'project/path.git', 'new/storage', '800']).and_return(["error", 1])
+        .with([projects_path, 'fetch-remote', 'current/storage', 'project/path.git', 'new/storage', '800'],
+              nil, popen_vars).and_return(["error", 1])
 
         expect { gitlab_shell.fetch_remote('current/storage', 'project/path', 'new/storage') }.to raise_error(Gitlab::Shell::Error, "error")
       end
@@ -119,14 +194,16 @@ describe Gitlab::Shell, lib: true do
     describe '#import_repository' do
       it 'returns true when the command succeeds' do
         expect(Gitlab::Popen).to receive(:popen)
-          .with([projects_path, 'import-project', 'current/storage', 'project/path.git', 'https://gitlab.com/gitlab-org/gitlab-ce.git', "800"]).and_return([nil, 0])
+          .with([projects_path, 'import-project', 'current/storage', 'project/path.git', 'https://gitlab.com/gitlab-org/gitlab-ce.git', "800"],
+                nil, popen_vars).and_return([nil, 0])
 
         expect(gitlab_shell.import_repository('current/storage', 'project/path', 'https://gitlab.com/gitlab-org/gitlab-ce.git')).to be true
       end
 
       it 'raises an exception when the command fails' do
         expect(Gitlab::Popen).to receive(:popen)
-        .with([projects_path, 'import-project', 'current/storage', 'project/path.git', 'https://gitlab.com/gitlab-org/gitlab-ce.git', "800"]).and_return(["error", 1])
+        .with([projects_path, 'import-project', 'current/storage', 'project/path.git', 'https://gitlab.com/gitlab-org/gitlab-ce.git', "800"],
+              nil, popen_vars).and_return(["error", 1])
 
         expect { gitlab_shell.import_repository('current/storage', 'project/path', 'https://gitlab.com/gitlab-org/gitlab-ce.git') }.to raise_error(Gitlab::Shell::Error, "error")
       end

spec/migrations/rename_duplicated_variable_key_spec.rb

@@ -0,0 +1,34 @@
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20170622135451_rename_duplicated_variable_key.rb')
+
+describe RenameDuplicatedVariableKey, :migration do
+  let(:variables) { table(:ci_variables) }
+  let(:projects) { table(:projects) }
+
+  before do
+    projects.create!(id: 1)
+    variables.create!(id: 1, key: 'key1', project_id: 1)
+    variables.create!(id: 2, key: 'key2', project_id: 1)
+    variables.create!(id: 3, key: 'keyX', project_id: 1)
+    variables.create!(id: 4, key: 'keyX', project_id: 1)
+    variables.create!(id: 5, key: 'keyY', project_id: 1)
+    variables.create!(id: 6, key: 'keyX', project_id: 1)
+    variables.create!(id: 7, key: 'key7', project_id: 1)
+    variables.create!(id: 8, key: 'keyY', project_id: 1)
+  end
+
+  it 'correctly remove duplicated records with smaller id' do
+    migrate!
+
+    expect(variables.pluck(:id, :key)).to contain_exactly(
+      [1, 'key1'],
+      [2, 'key2'],
+      [3, 'keyX_3'],
+      [4, 'keyX_4'],
+      [5, 'keyY_5'],
+      [6, 'keyX'],
+      [7, 'key7'],
+      [8, 'keyY']
+    )
+  end
+end

spec/models/ci/variable_spec.rb

@@ -3,8 +3,16 @@ require 'spec_helper'
 describe Ci::Variable, models: true do
   subject { build(:ci_variable) }
 
+  let(:secret_value) { 'secret' }
+
+  describe 'validations' do
     it { is_expected.to include_module(HasVariable) }
-  it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id) }
+    it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope) }
+    it { is_expected.to validate_length_of(:key).is_at_most(255) }
+    it { is_expected.to allow_value('foo').for(:key) }
+    it { is_expected.not_to allow_value('foo bar').for(:key) }
+    it { is_expected.not_to allow_value('foo/bar').for(:key) }
+  end
 
   describe '.unprotected' do
     subject { described_class.unprotected }

spec/models/project_spec.rb

@@ -1346,7 +1346,7 @@ describe Project, models: true do
         .with(project.repository_storage_path, project.path_with_namespace)
         .and_return(true)
 
-      expect(project).to receive(:create_repository)
+      expect(project).to receive(:create_repository).with(force: true)
 
       project.ensure_repository
     end
@@ -1359,6 +1359,19 @@ describe Project, models: true do
 
       project.ensure_repository
     end
+
+    it 'creates the repository if it is a fork' do
+      expect(project).to receive(:forked?).and_return(true)
+
+      allow(project).to receive(:repository_exists?)
+        .and_return(false)
+
+      expect(shell).to receive(:add_repository)
+        .with(project.repository_storage_path, project.path_with_namespace)
+        .and_return(true)
+
+      project.ensure_repository
+    end
   end
 
   describe '#user_can_push_to_empty_repo?' do