diff --git a/CHANGELOG.md b/CHANGELOG.md index 7591559da22..ec768ffe9b1 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index c20c645d7e4..ac14c3dfaa8 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -5.0.6 +5.1.1 diff --git a/app/assets/javascripts/behaviors/autosize.js b/app/assets/javascripts/behaviors/autosize.js index 3bea460dcc6..e00af4b2fa8 100644 --- a/app/assets/javascripts/behaviors/autosize.js +++ b/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); }); diff --git a/app/assets/javascripts/monitoring/components/monitoring_column.vue b/app/assets/javascripts/monitoring/components/monitoring_column.vue index 4f4792877ee..e933634643b 100644 --- a/app/assets/javascripts/monitoring/components/monitoring_column.vue +++ b/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; diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 042d792dada..ce432ddbfe6 100644 --- a/app/finders/labels_finder.rb +++ b/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) diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb index 96d6e120998..0b8d0ff881a 100644 --- a/app/models/ci/variable.rb +++ b/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 diff --git a/app/models/project.rb b/app/models/project.rb index a6708cf48ac..241e7e60dd2 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1074,21 +1074,21 @@ 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? - if gitlab_shell.add_repository(repository_storage_path, path_with_namespace) - repository.after_create - true - else - errors.add(:base, 'Failed to create repository via gitlab-shell') - false - end + return if forked? && !force + + if gitlab_shell.add_repository(repository_storage_path, path_with_namespace) + repository.after_create + true + else + errors.add(:base, 'Failed to create repository via gitlab-shell') + false end end def ensure_repository - create_repository unless repository_exists? + create_repository(force: true) unless repository_exists? end def repository_exists? diff --git a/app/views/shared/issuable/form/_merge_params.html.haml b/app/views/shared/issuable/form/_merge_params.html.haml index bfa91629e1e..8f6509a8ce8 100644 --- a/app/views/shared/issuable/form/_merge_params.html.haml +++ b/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. diff --git a/changelogs/unreleased/32885-unintentionally-removing-branch-when-merging-merge-request.yml b/changelogs/unreleased/32885-unintentionally-removing-branch-when-merging-merge-request.yml new file mode 100644 index 00000000000..313aeab91b5 --- /dev/null +++ b/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: diff --git a/changelogs/unreleased/add-ci_variables-environment_scope-mysql.yml b/changelogs/unreleased/add-ci_variables-environment_scope-mysql.yml new file mode 100644 index 00000000000..4948d415bed --- /dev/null +++ b/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: diff --git a/changelogs/unreleased/hb-hide-archived-labels-from-group-issue-tracker.yml b/changelogs/unreleased/hb-hide-archived-labels-from-group-issue-tracker.yml new file mode 100644 index 00000000000..3b465d84126 --- /dev/null +++ b/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 diff --git a/changelogs/unreleased/monitoring-dashboard-fix-y-label.yml b/changelogs/unreleased/monitoring-dashboard-fix-y-label.yml new file mode 100644 index 00000000000..8a0e9ca855c --- /dev/null +++ b/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: diff --git a/changelogs/unreleased/sh-allow-force-repo-create.yml b/changelogs/unreleased/sh-allow-force-repo-create.yml new file mode 100644 index 00000000000..2a65ba807bb --- /dev/null +++ b/changelogs/unreleased/sh-allow-force-repo-create.yml @@ -0,0 +1,4 @@ +--- +title: Make Project#ensure_repository force create a repo +merge_request: +author: diff --git a/changelogs/unreleased/sh-optimize-project-commit-api.yml b/changelogs/unreleased/sh-optimize-project-commit-api.yml new file mode 100644 index 00000000000..e6a8a80593c --- /dev/null +++ b/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: diff --git a/db/migrate/20170622135451_rename_duplicated_variable_key.rb b/db/migrate/20170622135451_rename_duplicated_variable_key.rb new file mode 100644 index 00000000000..368718ab0ce --- /dev/null +++ b/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 diff --git a/db/migrate/20170622135628_add_environment_scope_to_ci_variables.rb b/db/migrate/20170622135628_add_environment_scope_to_ci_variables.rb new file mode 100644 index 00000000000..17fe062d8d5 --- /dev/null +++ b/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 diff --git a/db/migrate/20170622135728_add_unique_constraint_to_ci_variables.rb b/db/migrate/20170622135728_add_unique_constraint_to_ci_variables.rb new file mode 100644 index 00000000000..8b2cc40ee59 --- /dev/null +++ b/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 diff --git a/db/migrate/20170623080805_remove_ci_variables_project_id_index.rb b/db/migrate/20170623080805_remove_ci_variables_project_id_index.rb new file mode 100644 index 00000000000..ddcc0292b9d --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index b022ade2e16..d52dab5417e 100644 --- a/db/schema.rb +++ b/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 diff --git a/lib/api/commits.rb b/lib/api/commits.rb index c6fc17cc391..bcb842b9211 100644 --- a/lib/api/commits.rb +++ b/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) diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index 22554236c38..0baea092e6a 100644 --- a/lib/gitlab/shell.rb +++ b/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,8 +69,8 @@ module Gitlab # add_repository("/path/to/storage", "gitlab/gitlab-ci") # def add_repository(storage, name) - Gitlab::Utils.system_silent([gitlab_shell_projects_path, - 'add-project', storage, "#{name}.git"]) + gitlab_shell_fast_execute([gitlab_shell_projects_path, + 'add-project', storage, "#{name}.git"]) end # Import repository @@ -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,8 +116,8 @@ 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', - storage, "#{path}.git", "#{new_path}.git"]) + gitlab_shell_fast_execute([gitlab_shell_projects_path, 'mv-project', + storage, "#{path}.git", "#{new_path}.git"]) end # Fork repository to new namespace @@ -131,9 +130,9 @@ 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', - forked_from_storage, "#{path}.git", forked_to_storage, - fork_namespace]) + gitlab_shell_fast_execute([gitlab_shell_projects_path, 'fork-project', + forked_from_storage, "#{path}.git", forked_to_storage, + fork_namespace]) end # Remove repository from file system @@ -145,8 +144,8 @@ module Gitlab # remove_repository("/path/to/storage", "gitlab/gitlab-ci") # def remove_repository(storage, name) - Gitlab::Utils.system_silent([gitlab_shell_projects_path, - 'rm-project', storage, "#{name}.git"]) + gitlab_shell_fast_execute([gitlab_shell_projects_path, + 'rm-project', storage, "#{name}.git"]) end # Add new key to gitlab-shell @@ -155,8 +154,8 @@ module Gitlab # add_key("key-42", "sha-rsa ...") # def add_key(key_id, key_content) - Gitlab::Utils.system_silent([gitlab_shell_keys_path, - 'add-key', key_id, self.class.strip_key(key_content)]) + gitlab_shell_fast_execute([gitlab_shell_keys_path, + 'add-key', key_id, self.class.strip_key(key_content)]) end # Batch-add keys to authorized_keys @@ -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 diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb index 1724cdba830..95d96354b77 100644 --- a/spec/finders/labels_finder_spec.rb +++ b/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 diff --git a/spec/javascripts/monitoring/mock_data.js b/spec/javascripts/monitoring/mock_data.js index 6f4cb989847..56d938e1fbe 100644 --- a/spec/javascripts/monitoring/mock_data.js +++ b/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', diff --git a/spec/javascripts/monitoring/monitoring_column_spec.js b/spec/javascripts/monitoring/monitoring_column_spec.js index c8787f9708c..b3bc97adc9f 100644 --- a/spec/javascripts/monitoring/monitoring_column_spec.js +++ b/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); + }); }); diff --git a/spec/lib/gitlab/popen_spec.rb b/spec/lib/gitlab/popen_spec.rb index 4ae216d55b0..af50ecdb2ab 100644 --- a/spec/lib/gitlab/popen_spec.rb +++ b/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)) @@ -45,7 +56,7 @@ describe 'Gitlab::Popen', lib: true, no_db: true do before do @output, @status = @klass.new.popen(%w[cat]) { |stdin| stdin.write 'hello' } end - + it { expect(@status).to be_zero } it { expect(@output).to eq('hello') } end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index a97a0f8452b..5b1b8f9516a 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/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 diff --git a/spec/migrations/rename_duplicated_variable_key_spec.rb b/spec/migrations/rename_duplicated_variable_key_spec.rb new file mode 100644 index 00000000000..11096564dfa --- /dev/null +++ b/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 diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index 329682a0771..50f7c029af8 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -3,8 +3,16 @@ require 'spec_helper' describe Ci::Variable, models: true do subject { build(:ci_variable) } - it { is_expected.to include_module(HasVariable) } - it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id) } + 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, :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 } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 6ff4ec3d417..ad98b4b669f 100644 --- a/spec/models/project_spec.rb +++ b/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