From 7bd1a28c1549258cd95bee8578324e74178dfe5f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 28 Mar 2019 10:31:56 +0000 Subject: [PATCH 1/7] Merge branch 'remove-cng-job' into 'master' Remove cloud-native-job from CI See merge request gitlab-org/gitlab-ce!25605 (cherry picked from commit 04496085c1b83ccf5d9bfa546daf2a92e9c644e7) a1912444 Make cloud release job manual, as a fallback --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4e8453726a3..1f0e798e8a0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -315,7 +315,7 @@ cloud-native-image: variables: GIT_DEPTH: "1" cache: {} - when: always + when: manual script: - gem install gitlab --no-document - CNG_PROJECT_PATH="gitlab-org/build/CNG" BUILD_TRIGGER_TOKEN=$CI_JOB_TOKEN ./scripts/trigger-build cng -- GitLab From 6ece4274828a4a3b514319ed26d750d4229b715f Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Wed, 6 Mar 2019 07:55:15 +0000 Subject: [PATCH 2/7] Merge branch 'ce-9445-kerberos-clone-url-is-no-longer-visible' into 'master' Backport Kerberos clone URL to CE See merge request gitlab-org/gitlab-ce!25750 (cherry picked from commit c9e5ce8dbd25203484b43c48f0a55a5d7bf396e8) c0a97cf5 Backport Kerberos clone URL to CE --- app/views/projects/buttons/_clone.html.haml | 5 +++-- app/views/shared/_mobile_clone_panel.html.haml | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/views/projects/buttons/_clone.html.haml b/app/views/projects/buttons/_clone.html.haml index 159d9e44e17..09f05b30433 100644 --- a/app/views/projects/buttons/_clone.html.haml +++ b/app/views/projects/buttons/_clone.html.haml @@ -7,7 +7,7 @@ = sprite_icon("arrow-down", css_class: "icon") %ul.p-3.dropdown-menu.dropdown-menu-right.dropdown-menu-large.dropdown-menu-selectable.clone-options-dropdown.qa-clone-options - if ssh_enabled? - %li.pb-2 + %li %label.label-bold = _('Clone with SSH') .input-group @@ -16,7 +16,7 @@ = clipboard_button(target: '#ssh_project_clone', title: _("Copy URL to clipboard"), class: "input-group-text btn-default btn-clipboard") = render_if_exists 'projects/buttons/geo' - if http_enabled? - %li + %li.pt-2 %label.label-bold = _('Clone with %{http_label}') % { http_label: gitlab_config.protocol.upcase } .input-group @@ -24,5 +24,6 @@ .input-group-append = clipboard_button(target: '#http_project_clone', title: _("Copy URL to clipboard"), class: "input-group-text btn-default btn-clipboard") = render_if_exists 'projects/buttons/geo' + = render_if_exists 'projects/buttons/kerberos_clone_field' = render_if_exists 'shared/geo_info_modal', project: project diff --git a/app/views/shared/_mobile_clone_panel.html.haml b/app/views/shared/_mobile_clone_panel.html.haml index 6e2527bd1a1..1e6b6f7c79b 100644 --- a/app/views/shared/_mobile_clone_panel.html.haml +++ b/app/views/shared/_mobile_clone_panel.html.haml @@ -13,3 +13,4 @@ - if http_enabled? %li = dropdown_item_with_description(http_copy_label, project.http_url_to_repo, href: project.http_url_to_repo, data: { clone_type: 'http' }) + = render_if_exists 'shared/mobile_kerberos_clone' -- GitLab From ed61ca9b4e256bbe17d71fb49531b0556f03f007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Tue, 12 Mar 2019 11:08:27 +0000 Subject: [PATCH 3/7] Merge branch 'ce-9826-fix-broken-downstreams' into 'master' CE port for expanded pipelines See merge request gitlab-org/gitlab-ce!25859 (cherry picked from commit 71dbd613982c6f247b3897e2d012a261904178a4) 2dbf3da2 CE port for expanded pipelines --- .../pipelines/pipeline_details_mediator.js | 16 +++++++++++++++- .../pipelines/services/pipeline_service.js | 4 ++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/pipelines/pipeline_details_mediator.js b/app/assets/javascripts/pipelines/pipeline_details_mediator.js index bd1e1895660..d67d88c4dba 100644 --- a/app/assets/javascripts/pipelines/pipeline_details_mediator.js +++ b/app/assets/javascripts/pipelines/pipeline_details_mediator.js @@ -19,6 +19,7 @@ export default class pipelinesMediator { this.poll = new Poll({ resource: this.service, method: 'getPipeline', + data: this.store.state.expandedPipelines ? this.getExpandedParameters() : undefined, successCallback: this.successCallback.bind(this), errorCallback: this.errorCallback.bind(this), }); @@ -56,6 +57,19 @@ export default class pipelinesMediator { .getPipeline() .then(response => this.successCallback(response)) .catch(() => this.errorCallback()) - .finally(() => this.poll.restart()); + .finally(() => + this.poll.restart( + this.store.state.expandedPipelines ? this.getExpandedParameters() : undefined, + ), + ); + } + + /** + * Backend expects paramets in the following format: `expanded[]=id&expanded[]=id` + */ + getExpandedParameters() { + return { + expanded: this.store.state.expandedPipelines, + }; } } diff --git a/app/assets/javascripts/pipelines/services/pipeline_service.js b/app/assets/javascripts/pipelines/services/pipeline_service.js index a53a9cc8365..e44eb9cdfd1 100644 --- a/app/assets/javascripts/pipelines/services/pipeline_service.js +++ b/app/assets/javascripts/pipelines/services/pipeline_service.js @@ -5,8 +5,8 @@ export default class PipelineService { this.pipeline = endpoint; } - getPipeline() { - return axios.get(this.pipeline); + getPipeline(params) { + return axios.get(this.pipeline, { params }); } // eslint-disable-next-line class-methods-use-this -- GitLab From c929dd570c2b32104d5f85686106fe199bbd0b07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Tue, 12 Mar 2019 17:26:07 +0000 Subject: [PATCH 4/7] Merge branch 'ce-9826-fix-broken-downstreams-backport-factory' into 'master' Backports EE change for expanded pipelines See merge request gitlab-org/gitlab-ce!26043 (cherry picked from commit 30e52b239ce9ac7ba83778e00f4b45d65e61a4a0) f8bac850 Backports EE change for expanded pipelines --- spec/factories/ci/pipelines.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index 8a44ce52849..ee5d27355f1 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -82,6 +82,12 @@ FactoryBot.define do end end + trait :with_job do + after(:build) do |pipeline, evaluator| + pipeline.builds << build(:ci_build, pipeline: pipeline, project: pipeline.project) + end + end + trait :auto_devops_source do config_source { Ci::Pipeline.config_sources[:auto_devops_source] } end -- GitLab From be180a44801d8e6755fa07d259b8900228f5c0c1 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 14 Mar 2019 10:05:18 +0000 Subject: [PATCH 5/7] Merge branch 'fj-58804-fix-bitbucket-import' into 'master' Fix Bitbucket import Closes #58804 See merge request gitlab-org/gitlab-ce!26050 (cherry picked from commit 516987e4ba40bffa68a1060efd901af2f1e6a3c3) e4663942 Fix Bitbucket import a89851e7 Added changelog to MR b5b9925e Removing SHA length validation 4268fc87 Added SHA length validation 2e2e3a8c Small replacement in spec to use a constant --- app/validators/sha_validator.rb | 2 +- .../fj-58804-fix-bitbucket-import.yml | 5 +++++ .../gitlab/bitbucket_import/importer_spec.rb | 20 +++++++++++++++++-- spec/validators/sha_validator_spec.rb | 9 +++++++-- 4 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/fj-58804-fix-bitbucket-import.yml diff --git a/app/validators/sha_validator.rb b/app/validators/sha_validator.rb index 085fca4d65d..77e7cfa4f6b 100644 --- a/app/validators/sha_validator.rb +++ b/app/validators/sha_validator.rb @@ -2,7 +2,7 @@ class ShaValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - return if value.blank? || value.match(/\A\h{40}\z/) + return if value.blank? || Commit.valid_hash?(value) record.errors.add(attribute, 'is not a valid SHA') end diff --git a/changelogs/unreleased/fj-58804-fix-bitbucket-import.yml b/changelogs/unreleased/fj-58804-fix-bitbucket-import.yml new file mode 100644 index 00000000000..dc44c64a055 --- /dev/null +++ b/changelogs/unreleased/fj-58804-fix-bitbucket-import.yml @@ -0,0 +1,5 @@ +--- +title: Fix bug in BitBucket imports with SHA shorter than 40 chars +merge_request: 26050 +author: +type: fixed diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index c432cc223b9..e1a2bae5fe8 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -95,6 +95,9 @@ describe Gitlab::BitbucketImport::Importer do subject { described_class.new(project) } describe '#import_pull_requests' do + let(:source_branch_sha) { sample.commits.last } + let(:target_branch_sha) { sample.commits.first } + before do allow(subject).to receive(:import_wiki) allow(subject).to receive(:import_issues) @@ -102,9 +105,9 @@ describe Gitlab::BitbucketImport::Importer do pull_request = instance_double( Bitbucket::Representation::PullRequest, iid: 10, - source_branch_sha: sample.commits.last, + source_branch_sha: source_branch_sha, source_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.source_branch, - target_branch_sha: sample.commits.first, + target_branch_sha: target_branch_sha, target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch, title: 'This is a title', description: 'This is a test pull request', @@ -162,6 +165,19 @@ describe Gitlab::BitbucketImport::Importer do expect(reply_note).to be_a(DiffNote) expect(reply_note.note).to eq(@reply.note) end + + context "when branches' sha is not found in the repository" do + let(:source_branch_sha) { 'a' * Commit::MIN_SHA_LENGTH } + let(:target_branch_sha) { 'b' * Commit::MIN_SHA_LENGTH } + + it 'uses the pull request sha references' do + expect { subject.execute }.to change { MergeRequest.count }.by(1) + + merge_request_diff = MergeRequest.first.merge_request_diff + expect(merge_request_diff.head_commit_sha).to eq source_branch_sha + expect(merge_request_diff.start_commit_sha).to eq target_branch_sha + end + end end context 'issues statuses' do diff --git a/spec/validators/sha_validator_spec.rb b/spec/validators/sha_validator_spec.rb index b9242ef931e..fa3dd68df2d 100644 --- a/spec/validators/sha_validator_spec.rb +++ b/spec/validators/sha_validator_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe ShaValidator do let(:validator) { described_class.new(attributes: [:base_commit_sha]) } - let(:merge_diff) { build(:merge_request_diff) } + let!(:merge_diff) { build(:merge_request_diff) } subject { validator.validate_each(merge_diff, :base_commit_sha, value) } @@ -10,6 +10,8 @@ describe ShaValidator do let(:value) { nil } it 'does not add any error if value is empty' do + expect(Commit).not_to receive(:valid_hash?) + subject expect(merge_diff.errors).to be_empty @@ -19,7 +21,9 @@ describe ShaValidator do context 'with valid sha' do let(:value) { Digest::SHA1.hexdigest(SecureRandom.hex) } - it 'does not add any error if value is empty' do + it 'does not add any error' do + expect(Commit).to receive(:valid_hash?).and_call_original + subject expect(merge_diff.errors).to be_empty @@ -30,6 +34,7 @@ describe ShaValidator do let(:value) { 'foo' } it 'adds error to the record' do + expect(Commit).to receive(:valid_hash?).and_call_original expect(merge_diff.errors).to be_empty subject -- GitLab From eccfd3ea350a6e4cc0e8ddba3211735e2ae159ed Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 12 Mar 2019 20:59:17 +0000 Subject: [PATCH 6/7] Merge branch 'sh-revert-rack-request-health-checks' into 'master' Fix health checks not working behind load balancers Closes #58573 See merge request gitlab-org/gitlab-ce!26055 (cherry picked from commit ef19ded4b0b5cc3aabb50b3432c8711f23a2742b) 01203e71 Fix health checks not working behind load balancers --- .../sh-revert-rack-request-health-checks.yml | 5 ++++ lib/gitlab/middleware/basic_health_check.rb | 8 ++++- lib/gitlab/request_context.rb | 8 ++++- .../middleware/basic_health_check_spec.rb | 29 +++++++++++++++++++ spec/lib/gitlab/request_context_spec.rb | 27 ++++++++++++++++- 5 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/sh-revert-rack-request-health-checks.yml diff --git a/changelogs/unreleased/sh-revert-rack-request-health-checks.yml b/changelogs/unreleased/sh-revert-rack-request-health-checks.yml new file mode 100644 index 00000000000..5dd5e5b731c --- /dev/null +++ b/changelogs/unreleased/sh-revert-rack-request-health-checks.yml @@ -0,0 +1,5 @@ +--- +title: Fix health checks not working behind load balancers +merge_request: 26055 +author: +type: fixed diff --git a/lib/gitlab/middleware/basic_health_check.rb b/lib/gitlab/middleware/basic_health_check.rb index acf8c301b8f..84e49805428 100644 --- a/lib/gitlab/middleware/basic_health_check.rb +++ b/lib/gitlab/middleware/basic_health_check.rb @@ -24,7 +24,13 @@ module Gitlab def call(env) return @app.call(env) unless env['PATH_INFO'] == HEALTH_PATH - request = ActionDispatch::Request.new(env) + # We should be using ActionDispatch::Request instead of + # Rack::Request to be consistent with Rails, but due to a Rails + # bug described in + # https://gitlab.com/gitlab-org/gitlab-ce/issues/58573#note_149799010 + # hosts behind a load balancer will only see 127.0.0.1 for the + # load balancer's IP. + request = Rack::Request.new(env) return OK_RESPONSE if client_ip_whitelisted?(request) diff --git a/lib/gitlab/request_context.rb b/lib/gitlab/request_context.rb index d9811e036d3..f6d289476c5 100644 --- a/lib/gitlab/request_context.rb +++ b/lib/gitlab/request_context.rb @@ -13,7 +13,13 @@ module Gitlab end def call(env) - req = ActionDispatch::Request.new(env) + # We should be using ActionDispatch::Request instead of + # Rack::Request to be consistent with Rails, but due to a Rails + # bug described in + # https://gitlab.com/gitlab-org/gitlab-ce/issues/58573#note_149799010 + # hosts behind a load balancer will only see 127.0.0.1 for the + # load balancer's IP. + req = Rack::Request.new(env) Gitlab::SafeRequestStore[:client_ip] = req.ip diff --git a/spec/lib/gitlab/middleware/basic_health_check_spec.rb b/spec/lib/gitlab/middleware/basic_health_check_spec.rb index 187d903a5e1..86bdc479b66 100644 --- a/spec/lib/gitlab/middleware/basic_health_check_spec.rb +++ b/spec/lib/gitlab/middleware/basic_health_check_spec.rb @@ -28,6 +28,35 @@ describe Gitlab::Middleware::BasicHealthCheck do end end + context 'with X-Forwarded-For headers' do + let(:load_balancer_ip) { '1.2.3.4' } + + before do + env['HTTP_X_FORWARDED_FOR'] = "#{load_balancer_ip}, 127.0.0.1" + env['REMOTE_ADDR'] = '127.0.0.1' + env['PATH_INFO'] = described_class::HEALTH_PATH + end + + it 'returns 200 response when endpoint is allowed' do + allow(Settings.monitoring).to receive(:ip_whitelist).and_return([load_balancer_ip]) + expect(app).not_to receive(:call) + + response = middleware.call(env) + + expect(response[0]).to eq(200) + expect(response[1]).to eq({ 'Content-Type' => 'text/plain' }) + expect(response[2]).to eq(['GitLab OK']) + end + + it 'returns 404 when whitelist is not configured' do + allow(Settings.monitoring).to receive(:ip_whitelist).and_return([]) + + response = middleware.call(env) + + expect(response[0]).to eq(404) + end + end + context 'whitelisted IP' do before do env['REMOTE_ADDR'] = '127.0.0.1' diff --git a/spec/lib/gitlab/request_context_spec.rb b/spec/lib/gitlab/request_context_spec.rb index fd443cc1f71..3ed57c2c916 100644 --- a/spec/lib/gitlab/request_context_spec.rb +++ b/spec/lib/gitlab/request_context_spec.rb @@ -6,6 +6,31 @@ describe Gitlab::RequestContext do let(:app) { -> (env) {} } let(:env) { Hash.new } + context 'with X-Forwarded-For headers', :request_store do + let(:load_balancer_ip) { '1.2.3.4' } + let(:headers) do + { + 'HTTP_X_FORWARDED_FOR' => "#{load_balancer_ip}, 127.0.0.1", + 'REMOTE_ADDR' => '127.0.0.1' + } + end + + let(:env) { Rack::MockRequest.env_for("/").merge(headers) } + + it 'returns the load balancer IP' do + client_ip = nil + + endpoint = proc do + client_ip = Gitlab::SafeRequestStore[:client_ip] + [200, {}, ["Hello"]] + end + + Rails.application.middleware.build(endpoint).call(env) + + expect(client_ip).to eq(load_balancer_ip) + end + end + context 'when RequestStore::Middleware is used' do around do |example| RequestStore::Middleware.new(-> (env) { example.run }).call({}) @@ -15,7 +40,7 @@ describe Gitlab::RequestContext do let(:ip) { '192.168.1.11' } before do - allow_any_instance_of(ActionDispatch::Request).to receive(:ip).and_return(ip) + allow_any_instance_of(Rack::Request).to receive(:ip).and_return(ip) described_class.new(app).call(env) end -- GitLab From b93b23cb403c8dd4b0a9e9ac89110d99e46a3e68 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Fri, 15 Mar 2019 11:34:26 +0000 Subject: [PATCH 7/7] Merge branch 'sh-handle-null-bytes-in-merge-request-diffs' into 'master' Fix error creating a merge request when diff includes a null byte Closes #57710 See merge request gitlab-org/gitlab-ce!26190 (cherry picked from commit a885e2d0a63b25411995fce2057067348f1bad9b) 6552197e Fix error creating a merge request when diff includes a null byte --- app/models/merge_request_diff.rb | 7 ++++++- ...ndle-null-bytes-in-merge-request-diffs.yml | 5 +++++ spec/models/merge_request_diff_spec.rb | 21 +++++++++++++++++++ spec/support/helpers/repo_helpers.rb | 14 +++++++++++++ 4 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/sh-handle-null-bytes-in-merge-request-diffs.yml diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 481be2da8ac..be2f1319fd2 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -301,6 +301,11 @@ class MergeRequestDiff < ActiveRecord::Base private + def encode_in_base64?(diff_text) + (diff_text.encoding == Encoding::BINARY && !diff_text.ascii_only?) || + diff_text.include?("\0") + end + def create_merge_request_diff_files(diffs) rows = if has_attribute?(:external_diff) && Gitlab.config.external_diffs.enabled @@ -353,7 +358,7 @@ class MergeRequestDiff < ActiveRecord::Base diff_hash.tap do |hash| diff_text = hash[:diff] - if diff_text.encoding == Encoding::BINARY && !diff_text.ascii_only? + if encode_in_base64?(diff_text) hash[:binary] = true hash[:diff] = [diff_text].pack('m0') end diff --git a/changelogs/unreleased/sh-handle-null-bytes-in-merge-request-diffs.yml b/changelogs/unreleased/sh-handle-null-bytes-in-merge-request-diffs.yml new file mode 100644 index 00000000000..01b6b08b61b --- /dev/null +++ b/changelogs/unreleased/sh-handle-null-bytes-in-merge-request-diffs.yml @@ -0,0 +1,5 @@ +--- +title: Fix error creating a merge request when diff includes a null byte +merge_request: 26190 +author: +type: fixed diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index e530e0302f5..53f5307ea0b 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe MergeRequestDiff do + include RepoHelpers + let(:diff_with_commits) { create(:merge_request).merge_request_diff } describe 'validations' do @@ -194,6 +196,25 @@ describe MergeRequestDiff do expect(diff_file).to be_binary expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [path]).to_a.first.diff) end + + context 'with diffs that contain a null byte' do + let(:filename) { 'test-null.txt' } + let(:content) { "a" * 10000 + "\x00" } + let(:project) { create(:project, :repository) } + let(:branch) { 'null-data' } + let(:target_branch) { 'master' } + + it 'saves diffs correctly' do + create_file_in_repo(project, target_branch, branch, filename, content) + + mr_diff = create(:merge_request, target_project: project, source_project: project, source_branch: branch, target_branch: target_branch).merge_request_diff + diff_file = mr_diff.merge_request_diff_files.find_by(new_path: filename) + + expect(diff_file).to be_binary + expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [filename]).to_a.first.diff) + expect(diff_file.diff).to include(content) + end + end end end diff --git a/spec/support/helpers/repo_helpers.rb b/spec/support/helpers/repo_helpers.rb index 3c6956cf5e0..4af90f4af79 100644 --- a/spec/support/helpers/repo_helpers.rb +++ b/spec/support/helpers/repo_helpers.rb @@ -115,4 +115,18 @@ eos commits: commits ) end + + def create_file_in_repo( + project, start_branch, branch_name, filename, content, + commit_message: 'Add new content') + Files::CreateService.new( + project, + project.owner, + commit_message: commit_message, + start_branch: start_branch, + branch_name: branch_name, + file_path: filename, + file_content: content + ).execute + end end -- GitLab