From c6979035c114b40e3b49f5ff3572cdf5fe19bb0b Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Thu, 25 Oct 2018 16:35:04 +0200 Subject: Support tls communication in gitaly --- config/initializers/1_settings.rb | 1 + lib/gitlab/gitaly_client.rb | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index bd02b85c7ce..a5d20ef12f6 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -429,6 +429,7 @@ Settings.rack_attack.git_basic_auth['bantime'] ||= 1.hour # Gitaly # Settings['gitaly'] ||= Settingslogic.new({}) +Settings.gitaly['tls'] ||= Settingslogic.new({}) # # Webpack settings diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index d99a9f15371..c39b75c7fba 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -53,6 +53,10 @@ module Gitlab base_labels Gitlab::Metrics::Transaction::BASE_LABELS.merge(gitaly_service: nil, rpc: nil) end + def self.creds + Gitlab.config.gitaly.tls.credentials + end + def self.stub(name, storage) MUTEX.synchronize do @stubs ||= {} @@ -60,11 +64,20 @@ module Gitlab @stubs[storage][name] ||= begin klass = stub_class(name) addr = stub_address(storage) - klass.new(addr, :this_channel_is_insecure) + creds = stub_creds(storage) + klass.new(addr, creds) end end end + def self.stub_creds(storage) + if URI(address(storage)).scheme == 'tls' + GRPC::Code::ChannelCredentials.new + else + :this_channel_is_insecure + end + end + def self.stub_class(name) if name == :health_check Grpc::Health::V1::Health::Stub @@ -75,7 +88,7 @@ module Gitlab def self.stub_address(storage) addr = address(storage) - addr = addr.sub(%r{^tcp://}, '') if URI(addr).scheme == 'tcp' + addr = addr.sub(%r{^tcp://|^tls://}, '') if %w(tcp tls).include? URI(addr).scheme addr end @@ -98,8 +111,8 @@ module Gitlab raise "storage #{storage.inspect} is missing a gitaly_address" end - unless URI(address).scheme.in?(%w(tcp unix)) - raise "Unsupported Gitaly address: #{address.inspect} does not use URL scheme 'tcp' or 'unix'" + unless URI(address).scheme.in?(%w(tcp unix tls)) + raise "Unsupported Gitaly address: #{address.inspect} does not use URL scheme 'tcp' or 'unix' or 'tls'" end address -- cgit v1.2.1 From b8ab35d6fedea6e93c755d5a805f13de8b8793a7 Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Wed, 31 Oct 2018 14:24:31 +0200 Subject: Add gitaly client specs --- spec/lib/gitlab/gitaly_client_spec.rb | 15 ++ spec/models/blob_spec.rb | 382 ---------------------------------- 2 files changed, 15 insertions(+), 382 deletions(-) delete mode 100644 spec/models/blob_spec.rb diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index 81bcd8c28ed..65177c627f3 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -47,6 +47,21 @@ describe Gitlab::GitalyClient, skip_gitaly_mock: true do end end + context 'when passed a TLS address' do + it 'strips tls:// prefix before passing it to GRPC::Core::Channel initializer' do + address = 'localhost:9876' + prefixed_address = "tls://#{address}" + + allow(Gitlab.config.repositories).to receive(:storages).and_return({ + 'default' => { 'gitaly_address' => prefixed_address } + }) + + expect(Gitaly::CommitService::Stub).to receive(:new).with(address, any_args) + + described_class.stub(:commit_service, 'default') + end + end + context 'when passed a TCP address' do it 'strips tcp:// prefix before passing it to GRPC::Core::Channel initializer' do address = 'localhost:9876' diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb deleted file mode 100644 index 81e35e6c931..00000000000 --- a/spec/models/blob_spec.rb +++ /dev/null @@ -1,382 +0,0 @@ -# encoding: utf-8 -require 'rails_helper' - -describe Blob do - include FakeBlobHelpers - - let(:project) { build(:project, lfs_enabled: true) } - - before do - allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) - end - - describe '.decorate' do - it 'returns NilClass when given nil' do - expect(described_class.decorate(nil)).to be_nil - end - end - - describe '.lazy' do - let(:project) { create(:project, :repository) } - let(:commit) { project.commit_by(oid: 'e63f41fe459e62e1228fcef60d7189127aeba95a') } - - it 'fetches all blobs when the first is accessed' do - changelog = described_class.lazy(project, commit.id, 'CHANGELOG') - contributing = described_class.lazy(project, commit.id, 'CONTRIBUTING.md') - - expect(Gitlab::Git::Blob).to receive(:batch).once.and_call_original - expect(Gitlab::Git::Blob).not_to receive(:find) - - # Access property so the values are loaded - changelog.id - contributing.id - end - end - - describe '#data' do - context 'using a binary blob' do - it 'returns the data as-is' do - data = "\n\xFF\xB9\xC3" - blob = fake_blob(binary: true, data: data) - - expect(blob.data).to eq(data) - end - end - - context 'using a text blob' do - it 'converts the data to UTF-8' do - blob = fake_blob(binary: false, data: "\n\xFF\xB9\xC3") - - expect(blob.data).to eq("\n���") - end - end - end - - describe '#external_storage_error?' do - context 'if the blob is stored in LFS' do - let(:blob) { fake_blob(path: 'file.pdf', lfs: true) } - - context 'when the project has LFS enabled' do - it 'returns false' do - expect(blob.external_storage_error?).to be_falsey - end - end - - context 'when the project does not have LFS enabled' do - before do - project.lfs_enabled = false - end - - it 'returns true' do - expect(blob.external_storage_error?).to be_truthy - end - end - end - - context 'if the blob is not stored in LFS' do - let(:blob) { fake_blob(path: 'file.md') } - - it 'returns false' do - expect(blob.external_storage_error?).to be_falsey - end - end - end - - describe '#stored_externally?' do - context 'if the blob is stored in LFS' do - let(:blob) { fake_blob(path: 'file.pdf', lfs: true) } - - context 'when the project has LFS enabled' do - it 'returns true' do - expect(blob.stored_externally?).to be_truthy - end - end - - context 'when the project does not have LFS enabled' do - before do - project.lfs_enabled = false - end - - it 'returns false' do - expect(blob.stored_externally?).to be_falsey - end - end - end - - context 'if the blob is not stored in LFS' do - let(:blob) { fake_blob(path: 'file.md') } - - it 'returns false' do - expect(blob.stored_externally?).to be_falsey - end - end - end - - describe '#raw_binary?' do - context 'if the blob is stored externally' do - context 'if the extension has a rich viewer' do - context 'if the viewer is binary' do - it 'returns true' do - blob = fake_blob(path: 'file.pdf', lfs: true) - - expect(blob.raw_binary?).to be_truthy - end - end - - context 'if the viewer is text-based' do - it 'return false' do - blob = fake_blob(path: 'file.md', lfs: true) - - expect(blob.raw_binary?).to be_falsey - end - end - end - - context "if the extension doesn't have a rich viewer" do - context 'if the extension has a text mime type' do - context 'if the extension is for a programming language' do - it 'returns false' do - blob = fake_blob(path: 'file.txt', lfs: true) - - expect(blob.raw_binary?).to be_falsey - end - end - - context 'if the extension is not for a programming language' do - it 'returns false' do - blob = fake_blob(path: 'file.ics', lfs: true) - - expect(blob.raw_binary?).to be_falsey - end - end - end - - context 'if the extension has a binary mime type' do - context 'if the extension is for a programming language' do - it 'returns false' do - blob = fake_blob(path: 'file.rb', lfs: true) - - expect(blob.raw_binary?).to be_falsey - end - end - - context 'if the extension is not for a programming language' do - it 'returns true' do - blob = fake_blob(path: 'file.exe', lfs: true) - - expect(blob.raw_binary?).to be_truthy - end - end - end - - context 'if the extension has an unknown mime type' do - context 'if the extension is for a programming language' do - it 'returns false' do - blob = fake_blob(path: 'file.ini', lfs: true) - - expect(blob.raw_binary?).to be_falsey - end - end - - context 'if the extension is not for a programming language' do - it 'returns true' do - blob = fake_blob(path: 'file.wtf', lfs: true) - - expect(blob.raw_binary?).to be_truthy - end - end - end - end - end - - context 'if the blob is not stored externally' do - context 'if the blob is binary' do - it 'returns true' do - blob = fake_blob(path: 'file.pdf', binary: true) - - expect(blob.raw_binary?).to be_truthy - end - end - - context 'if the blob is text-based' do - it 'return false' do - blob = fake_blob(path: 'file.md') - - expect(blob.raw_binary?).to be_falsey - end - end - end - end - - describe '#extension' do - it 'returns the extension' do - blob = fake_blob(path: 'file.md') - - expect(blob.extension).to eq('md') - end - end - - describe '#file_type' do - it 'returns the file type' do - blob = fake_blob(path: 'README.md') - - expect(blob.file_type).to eq(:readme) - end - end - - describe '#simple_viewer' do - context 'when the blob is empty' do - it 'returns an empty viewer' do - blob = fake_blob(data: '', size: 0) - - expect(blob.simple_viewer).to be_a(BlobViewer::Empty) - end - end - - context 'when the file represented by the blob is binary' do - it 'returns a download viewer' do - blob = fake_blob(binary: true) - - expect(blob.simple_viewer).to be_a(BlobViewer::Download) - end - end - - context 'when the file represented by the blob is text-based' do - it 'returns a text viewer' do - blob = fake_blob - - expect(blob.simple_viewer).to be_a(BlobViewer::Text) - end - end - end - - describe '#rich_viewer' do - context 'when the blob has an external storage error' do - before do - project.lfs_enabled = false - end - - it 'returns nil' do - blob = fake_blob(path: 'file.pdf', lfs: true) - - expect(blob.rich_viewer).to be_nil - end - end - - context 'when the blob is empty' do - it 'returns nil' do - blob = fake_blob(data: '') - - expect(blob.rich_viewer).to be_nil - end - end - - context 'when the blob is stored externally' do - it 'returns a matching viewer' do - blob = fake_blob(path: 'file.pdf', lfs: true) - - expect(blob.rich_viewer).to be_a(BlobViewer::PDF) - end - end - - context 'when the blob is binary' do - it 'returns a matching binary viewer' do - blob = fake_blob(path: 'file.pdf', binary: true) - - expect(blob.rich_viewer).to be_a(BlobViewer::PDF) - end - end - - context 'when the blob is text-based' do - it 'returns a matching text-based viewer' do - blob = fake_blob(path: 'file.md') - - expect(blob.rich_viewer).to be_a(BlobViewer::Markup) - end - end - end - - describe '#auxiliary_viewer' do - context 'when the blob has an external storage error' do - before do - project.lfs_enabled = false - end - - it 'returns nil' do - blob = fake_blob(path: 'LICENSE', lfs: true) - - expect(blob.auxiliary_viewer).to be_nil - end - end - - context 'when the blob is empty' do - it 'returns nil' do - blob = fake_blob(data: '') - - expect(blob.auxiliary_viewer).to be_nil - end - end - - context 'when the blob is stored externally' do - it 'returns a matching viewer' do - blob = fake_blob(path: 'LICENSE', lfs: true) - - expect(blob.auxiliary_viewer).to be_a(BlobViewer::License) - end - end - - context 'when the blob is binary' do - it 'returns nil' do - blob = fake_blob(path: 'LICENSE', binary: true) - - expect(blob.auxiliary_viewer).to be_nil - end - end - - context 'when the blob is text-based' do - it 'returns a matching text-based viewer' do - blob = fake_blob(path: 'LICENSE') - - expect(blob.auxiliary_viewer).to be_a(BlobViewer::License) - end - end - end - - describe '#rendered_as_text?' do - context 'when ignoring errors' do - context 'when the simple viewer is text-based' do - it 'returns true' do - blob = fake_blob(path: 'file.md', size: 100.megabytes) - - expect(blob.rendered_as_text?).to be_truthy - end - end - - context 'when the simple viewer is binary' do - it 'returns false' do - blob = fake_blob(path: 'file.pdf', binary: true, size: 100.megabytes) - - expect(blob.rendered_as_text?).to be_falsey - end - end - end - - context 'when not ignoring errors' do - context 'when the viewer has render errors' do - it 'returns false' do - blob = fake_blob(path: 'file.md', size: 100.megabytes) - - expect(blob.rendered_as_text?(ignore_errors: false)).to be_falsey - end - end - - context "when the viewer doesn't have render errors" do - it 'returns true' do - blob = fake_blob(path: 'file.md') - - expect(blob.rendered_as_text?(ignore_errors: false)).to be_truthy - end - end - end - end -end -- cgit v1.2.1 From e2cb9245ea54a1c49ad24e0d8001e1a14902348f Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Wed, 31 Oct 2018 14:24:39 +0200 Subject: Add changelog entry --- changelogs/unreleased/support-gitaly-tls.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/support-gitaly-tls.yml diff --git a/changelogs/unreleased/support-gitaly-tls.yml b/changelogs/unreleased/support-gitaly-tls.yml new file mode 100644 index 00000000000..c564f5b7ca0 --- /dev/null +++ b/changelogs/unreleased/support-gitaly-tls.yml @@ -0,0 +1,5 @@ +--- +title: Support tls communication in gitaly +merge_request: 22602 +author: +type: changed -- cgit v1.2.1 From 08a57fe8280ddef66f9c78860a97bf332ceea8d1 Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Wed, 31 Oct 2018 14:47:21 +0200 Subject: Add more specs --- config/initializers/1_settings.rb | 1 - lib/gitlab/gitaly_client.rb | 6 +- spec/lib/gitlab/gitaly_client_spec.rb | 18 ++ spec/models/blob_spec.rb | 382 ++++++++++++++++++++++++++++++++++ 4 files changed, 401 insertions(+), 6 deletions(-) create mode 100644 spec/models/blob_spec.rb diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index a5d20ef12f6..bd02b85c7ce 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -429,7 +429,6 @@ Settings.rack_attack.git_basic_auth['bantime'] ||= 1.hour # Gitaly # Settings['gitaly'] ||= Settingslogic.new({}) -Settings.gitaly['tls'] ||= Settingslogic.new({}) # # Webpack settings diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index c39b75c7fba..70c89109c61 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -53,10 +53,6 @@ module Gitlab base_labels Gitlab::Metrics::Transaction::BASE_LABELS.merge(gitaly_service: nil, rpc: nil) end - def self.creds - Gitlab.config.gitaly.tls.credentials - end - def self.stub(name, storage) MUTEX.synchronize do @stubs ||= {} @@ -72,7 +68,7 @@ module Gitlab def self.stub_creds(storage) if URI(address(storage)).scheme == 'tls' - GRPC::Code::ChannelCredentials.new + GRPC::Core::ChannelCredentials.new else :this_channel_is_insecure end diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index 65177c627f3..74831197bfb 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -28,6 +28,24 @@ describe Gitlab::GitalyClient, skip_gitaly_mock: true do end end + describe '.stub_creds' do + it 'returns :this_channel_is_insecure if tcp' do + address = 'tcp://localhost:9876' + allow(Gitlab.config.repositories).to receive(:storages).and_return({ + 'default' => { 'gitaly_address' => address } + }) + expect(described_class.stub_creds('default')).to eq(:this_channel_is_insecure) + end + + it 'returns Credentials object if tls' do + address = 'tls://localhost:9876' + allow(Gitlab.config.repositories).to receive(:storages).and_return({ + 'default' => { 'gitaly_address' => address } + }) + expect(described_class.stub_creds('default')).to be_a(GRPC::Core::ChannelCredentials) + end + end + describe '.stub' do # Notice that this is referring to gRPC "stubs", not rspec stubs before do diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb new file mode 100644 index 00000000000..81e35e6c931 --- /dev/null +++ b/spec/models/blob_spec.rb @@ -0,0 +1,382 @@ +# encoding: utf-8 +require 'rails_helper' + +describe Blob do + include FakeBlobHelpers + + let(:project) { build(:project, lfs_enabled: true) } + + before do + allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) + end + + describe '.decorate' do + it 'returns NilClass when given nil' do + expect(described_class.decorate(nil)).to be_nil + end + end + + describe '.lazy' do + let(:project) { create(:project, :repository) } + let(:commit) { project.commit_by(oid: 'e63f41fe459e62e1228fcef60d7189127aeba95a') } + + it 'fetches all blobs when the first is accessed' do + changelog = described_class.lazy(project, commit.id, 'CHANGELOG') + contributing = described_class.lazy(project, commit.id, 'CONTRIBUTING.md') + + expect(Gitlab::Git::Blob).to receive(:batch).once.and_call_original + expect(Gitlab::Git::Blob).not_to receive(:find) + + # Access property so the values are loaded + changelog.id + contributing.id + end + end + + describe '#data' do + context 'using a binary blob' do + it 'returns the data as-is' do + data = "\n\xFF\xB9\xC3" + blob = fake_blob(binary: true, data: data) + + expect(blob.data).to eq(data) + end + end + + context 'using a text blob' do + it 'converts the data to UTF-8' do + blob = fake_blob(binary: false, data: "\n\xFF\xB9\xC3") + + expect(blob.data).to eq("\n���") + end + end + end + + describe '#external_storage_error?' do + context 'if the blob is stored in LFS' do + let(:blob) { fake_blob(path: 'file.pdf', lfs: true) } + + context 'when the project has LFS enabled' do + it 'returns false' do + expect(blob.external_storage_error?).to be_falsey + end + end + + context 'when the project does not have LFS enabled' do + before do + project.lfs_enabled = false + end + + it 'returns true' do + expect(blob.external_storage_error?).to be_truthy + end + end + end + + context 'if the blob is not stored in LFS' do + let(:blob) { fake_blob(path: 'file.md') } + + it 'returns false' do + expect(blob.external_storage_error?).to be_falsey + end + end + end + + describe '#stored_externally?' do + context 'if the blob is stored in LFS' do + let(:blob) { fake_blob(path: 'file.pdf', lfs: true) } + + context 'when the project has LFS enabled' do + it 'returns true' do + expect(blob.stored_externally?).to be_truthy + end + end + + context 'when the project does not have LFS enabled' do + before do + project.lfs_enabled = false + end + + it 'returns false' do + expect(blob.stored_externally?).to be_falsey + end + end + end + + context 'if the blob is not stored in LFS' do + let(:blob) { fake_blob(path: 'file.md') } + + it 'returns false' do + expect(blob.stored_externally?).to be_falsey + end + end + end + + describe '#raw_binary?' do + context 'if the blob is stored externally' do + context 'if the extension has a rich viewer' do + context 'if the viewer is binary' do + it 'returns true' do + blob = fake_blob(path: 'file.pdf', lfs: true) + + expect(blob.raw_binary?).to be_truthy + end + end + + context 'if the viewer is text-based' do + it 'return false' do + blob = fake_blob(path: 'file.md', lfs: true) + + expect(blob.raw_binary?).to be_falsey + end + end + end + + context "if the extension doesn't have a rich viewer" do + context 'if the extension has a text mime type' do + context 'if the extension is for a programming language' do + it 'returns false' do + blob = fake_blob(path: 'file.txt', lfs: true) + + expect(blob.raw_binary?).to be_falsey + end + end + + context 'if the extension is not for a programming language' do + it 'returns false' do + blob = fake_blob(path: 'file.ics', lfs: true) + + expect(blob.raw_binary?).to be_falsey + end + end + end + + context 'if the extension has a binary mime type' do + context 'if the extension is for a programming language' do + it 'returns false' do + blob = fake_blob(path: 'file.rb', lfs: true) + + expect(blob.raw_binary?).to be_falsey + end + end + + context 'if the extension is not for a programming language' do + it 'returns true' do + blob = fake_blob(path: 'file.exe', lfs: true) + + expect(blob.raw_binary?).to be_truthy + end + end + end + + context 'if the extension has an unknown mime type' do + context 'if the extension is for a programming language' do + it 'returns false' do + blob = fake_blob(path: 'file.ini', lfs: true) + + expect(blob.raw_binary?).to be_falsey + end + end + + context 'if the extension is not for a programming language' do + it 'returns true' do + blob = fake_blob(path: 'file.wtf', lfs: true) + + expect(blob.raw_binary?).to be_truthy + end + end + end + end + end + + context 'if the blob is not stored externally' do + context 'if the blob is binary' do + it 'returns true' do + blob = fake_blob(path: 'file.pdf', binary: true) + + expect(blob.raw_binary?).to be_truthy + end + end + + context 'if the blob is text-based' do + it 'return false' do + blob = fake_blob(path: 'file.md') + + expect(blob.raw_binary?).to be_falsey + end + end + end + end + + describe '#extension' do + it 'returns the extension' do + blob = fake_blob(path: 'file.md') + + expect(blob.extension).to eq('md') + end + end + + describe '#file_type' do + it 'returns the file type' do + blob = fake_blob(path: 'README.md') + + expect(blob.file_type).to eq(:readme) + end + end + + describe '#simple_viewer' do + context 'when the blob is empty' do + it 'returns an empty viewer' do + blob = fake_blob(data: '', size: 0) + + expect(blob.simple_viewer).to be_a(BlobViewer::Empty) + end + end + + context 'when the file represented by the blob is binary' do + it 'returns a download viewer' do + blob = fake_blob(binary: true) + + expect(blob.simple_viewer).to be_a(BlobViewer::Download) + end + end + + context 'when the file represented by the blob is text-based' do + it 'returns a text viewer' do + blob = fake_blob + + expect(blob.simple_viewer).to be_a(BlobViewer::Text) + end + end + end + + describe '#rich_viewer' do + context 'when the blob has an external storage error' do + before do + project.lfs_enabled = false + end + + it 'returns nil' do + blob = fake_blob(path: 'file.pdf', lfs: true) + + expect(blob.rich_viewer).to be_nil + end + end + + context 'when the blob is empty' do + it 'returns nil' do + blob = fake_blob(data: '') + + expect(blob.rich_viewer).to be_nil + end + end + + context 'when the blob is stored externally' do + it 'returns a matching viewer' do + blob = fake_blob(path: 'file.pdf', lfs: true) + + expect(blob.rich_viewer).to be_a(BlobViewer::PDF) + end + end + + context 'when the blob is binary' do + it 'returns a matching binary viewer' do + blob = fake_blob(path: 'file.pdf', binary: true) + + expect(blob.rich_viewer).to be_a(BlobViewer::PDF) + end + end + + context 'when the blob is text-based' do + it 'returns a matching text-based viewer' do + blob = fake_blob(path: 'file.md') + + expect(blob.rich_viewer).to be_a(BlobViewer::Markup) + end + end + end + + describe '#auxiliary_viewer' do + context 'when the blob has an external storage error' do + before do + project.lfs_enabled = false + end + + it 'returns nil' do + blob = fake_blob(path: 'LICENSE', lfs: true) + + expect(blob.auxiliary_viewer).to be_nil + end + end + + context 'when the blob is empty' do + it 'returns nil' do + blob = fake_blob(data: '') + + expect(blob.auxiliary_viewer).to be_nil + end + end + + context 'when the blob is stored externally' do + it 'returns a matching viewer' do + blob = fake_blob(path: 'LICENSE', lfs: true) + + expect(blob.auxiliary_viewer).to be_a(BlobViewer::License) + end + end + + context 'when the blob is binary' do + it 'returns nil' do + blob = fake_blob(path: 'LICENSE', binary: true) + + expect(blob.auxiliary_viewer).to be_nil + end + end + + context 'when the blob is text-based' do + it 'returns a matching text-based viewer' do + blob = fake_blob(path: 'LICENSE') + + expect(blob.auxiliary_viewer).to be_a(BlobViewer::License) + end + end + end + + describe '#rendered_as_text?' do + context 'when ignoring errors' do + context 'when the simple viewer is text-based' do + it 'returns true' do + blob = fake_blob(path: 'file.md', size: 100.megabytes) + + expect(blob.rendered_as_text?).to be_truthy + end + end + + context 'when the simple viewer is binary' do + it 'returns false' do + blob = fake_blob(path: 'file.pdf', binary: true, size: 100.megabytes) + + expect(blob.rendered_as_text?).to be_falsey + end + end + end + + context 'when not ignoring errors' do + context 'when the viewer has render errors' do + it 'returns false' do + blob = fake_blob(path: 'file.md', size: 100.megabytes) + + expect(blob.rendered_as_text?(ignore_errors: false)).to be_falsey + end + end + + context "when the viewer doesn't have render errors" do + it 'returns true' do + blob = fake_blob(path: 'file.md') + + expect(blob.rendered_as_text?(ignore_errors: false)).to be_truthy + end + end + end + end +end -- cgit v1.2.1 From ebc174e968ece65110aa722a18cdf437cfa75eeb Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Fri, 2 Nov 2018 10:37:46 +0200 Subject: Add documentation for tls gitaly --- config/gitlab.yml.example | 2 +- doc/administration/gitaly/index.md | 46 ++++++++++++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index a4db125f831..5390ea78e62 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -605,7 +605,7 @@ production: &base storages: # You must have at least a `default` storage path. default: path: /home/git/repositories/ - gitaly_address: unix:/home/git/gitlab/tmp/sockets/private/gitaly.socket # TCP connections are supported too (e.g. tcp://host:port) + gitaly_address: unix:/home/git/gitlab/tmp/sockets/private/gitaly.socket # TCP connections are supported too (e.g. tcp://host:port). TLS connections are also supported using the system certificate pool (eg: tls://host:port). # gitaly_token: 'special token' # Optional: override global gitaly.token for this storage. ## Backup settings diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index e1b2a0a24eb..2eec0e30e62 100644 --- a/doc/administration/gitaly/index.md +++ b/doc/administration/gitaly/index.md @@ -25,7 +25,7 @@ gitaly['prometheus_listen_addr'] = 'localhost:9236' ``` To change a Gitaly setting in installations from source you can edit -`/home/git/gitaly/config.toml`. Changes will be applied when you run +`/home/git/gitaly/config.toml`. Changes will be applied when you run `service gitlab restart`. ```toml @@ -99,13 +99,13 @@ documentation on configuring Gitaly authentication](https://gitlab.com/gitlab-org/gitaly/blob/master/doc/configuration/README.md#authentication) . -Gitaly must trigger some callbacks to GitLab via GitLab Shell. As a result, +Gitaly must trigger some callbacks to GitLab via GitLab Shell. As a result, the GitLab Shell secret must be the same between the other GitLab servers and the Gitaly server. The easiest way to accomplish this is to copy `/etc/gitlab/gitlab-secrets.json` from an existing GitLab server to the Gitaly server. Without this shared secret, -Git operations in GitLab will result in an API error. +Git operations in GitLab will result in an API error. -> **NOTE:** In most or all cases the storage paths below end in `/repositories` which is +> **NOTE:** In most or all cases the storage paths below end in `/repositories` which is different than `path` in `git_data_dirs` of Omnibus installations. Check the directory layout on your Gitaly server to be sure. @@ -213,6 +213,44 @@ Gitaly logs on your Gitaly server (`sudo gitlab-ctl tail gitaly` or coming in. One sure way to trigger a Gitaly request is to clone a repository from your GitLab server over HTTP. +## TLS support + +Gitaly supports TLS credentials for GRPC authentication. To be able to communicate +with a gitaly instance that listens for secure connections you will need to use `tls://` url +scheme in the `gitaly_address` of the corresponding storage entry in the gitlab configuration. + +### Example TLS configuration + +Omnibus installations: + +```ruby +# /etc/gitlab/gitlab.rb +git_data_dirs({ + 'default' => { 'path' => '/mnt/gitlab/default', 'gitaly_address' => 'tls://gitaly.internal:8075' }, + 'storage1' => { 'path' => '/mnt/gitlab/storage1', 'gitaly_address' => 'tls://gitaly.internal:8075' }, +}) + +gitlab_rails['gitaly_token'] = 'abc123secret' +``` + +Source installations: + +```yaml +# /home/git/gitlab/config/gitlab.yml +gitlab: + repositories: + storages: + default: + path: /mnt/gitlab/default/repositories + gitaly_address: tls://gitaly.internal:8075 + storage1: + path: /mnt/gitlab/storage1/repositories + gitaly_address: tls://gitaly.internal:8075 + + gitaly: + token: 'abc123secret' +``` + ## Disabling or enabling the Gitaly service in a cluster environment If you are running Gitaly [as a remote -- cgit v1.2.1 From 76a37456a899173c4cdcabec08764d95b66ad5be Mon Sep 17 00:00:00 2001 From: David Planella Date: Sun, 11 Nov 2018 05:34:16 +0000 Subject: Fix anchor link to customizing AutoDevops .yaml file --- doc/topics/autodevops/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md index 96e788666a1..088518644e0 100644 --- a/doc/topics/autodevops/index.md +++ b/doc/topics/autodevops/index.md @@ -551,7 +551,7 @@ To view the metrics, open the While Auto DevOps provides great defaults to get you started, you can customize almost everything to fit your needs; from custom [buildpacks](#custom-buildpacks), to [`Dockerfile`s](#custom-dockerfile), [Helm charts](#custom-helm-chart), or -even copying the complete [CI/CD configuration](#customizing-gitlab-ci-yml) +even copying the complete [CI/CD configuration](#customizing-gitlab-ciyml) into your project to enable staging and canary deployments, and more. ### Custom buildpacks -- cgit v1.2.1 From b8e457827e57212abca61f5d295f5e1bde178cb8 Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Mon, 19 Nov 2018 13:57:48 +0200 Subject: Manually load the certificates --- lib/gitlab/gitaly_client.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 70c89109c61..cb6601786dc 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -66,9 +66,15 @@ module Gitlab end end + def self.load_certs + @certs ||= Dir["#{OpenSSL::X509::DEFAULT_CERT_DIR}/*"].map do |cert| + File.read(cert) + end.join("\n") + end + def self.stub_creds(storage) if URI(address(storage)).scheme == 'tls' - GRPC::Core::ChannelCredentials.new + GRPC::Core::ChannelCredentials.new load_certs else :this_channel_is_insecure end -- cgit v1.2.1 From f208b7ee05ef7841bba7d26230b1d74d1082b66e Mon Sep 17 00:00:00 2001 From: Philipp Hasper Date: Thu, 29 Nov 2018 08:55:51 +0000 Subject: Fixed link in doc/ci/yaml/README.md --- doc/ci/yaml/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index e46b2bbc79c..d1bc475b5ab 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -595,7 +595,7 @@ failure. fails. 1. `always` - execute job regardless of the status of jobs from prior stages. 1. `manual` - execute job manually (added in GitLab 8.10). Read about - [manual actions](#when-manual) below. + [manual actions](#whenmanual) below. For example: -- cgit v1.2.1 From 943827b39ae1e3203736ec87724ec255505ae980 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Wed, 31 Oct 2018 20:12:22 +0100 Subject: option to make variables protected by default --- .../ci_variable_list/ci_variable_list.js | 4 ++- app/helpers/application_settings_helper.rb | 3 +- app/models/application_setting.rb | 3 +- .../admin/application_settings/_ci_cd.html.haml | 7 ++++ app/views/ci/variables/_variable_row.html.haml | 6 ++-- ...otected_ci_variables_to_application_settings.rb | 17 ++++++++++ db/schema.rb | 1 + locale/gitlab.pot | 6 ++++ .../ci_variable_list/ci_variable_list_spec.js | 2 ++ .../features/variable_list_shared_examples.rb | 38 ++++++++++++++++++++++ 10 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20181031145139_add_protected_ci_variables_to_application_settings.rb diff --git a/app/assets/javascripts/ci_variable_list/ci_variable_list.js b/app/assets/javascripts/ci_variable_list/ci_variable_list.js index ee0f7cda189..5b20fa141cd 100644 --- a/app/assets/javascripts/ci_variable_list/ci_variable_list.js +++ b/app/assets/javascripts/ci_variable_list/ci_variable_list.js @@ -36,7 +36,9 @@ export default class VariableList { }, protected: { selector: '.js-ci-variable-input-protected', - default: 'false', + // use `attr` instead of `data` as we don't want the value to be + // converted. we need the value as a string. + default: $('.js-ci-variable-input-protected').attr('data-default'), }, environment_scope: { // We can't use a `.js-` class here because diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 086bb38ce9a..72731d969a2 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -218,7 +218,8 @@ module ApplicationSettingsHelper :version_check_enabled, :web_ide_clientside_preview_enabled, :diff_max_patch_bytes, - :commit_email_hostname + :commit_email_hostname, + :protected_ci_variables ] end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 207ffae873a..da2e095e336 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -302,7 +302,8 @@ class ApplicationSetting < ActiveRecord::Base user_show_add_ssh_key_message: true, usage_stats_set_by_user_id: nil, diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES, - commit_email_hostname: default_commit_email_hostname + commit_email_hostname: default_commit_email_hostname, + protected_ci_variables: false } end diff --git a/app/views/admin/application_settings/_ci_cd.html.haml b/app/views/admin/application_settings/_ci_cd.html.haml index 0d42094fc89..fdaad1cf181 100644 --- a/app/views/admin/application_settings/_ci_cd.html.haml +++ b/app/views/admin/application_settings/_ci_cd.html.haml @@ -49,5 +49,12 @@ Once that time passes, the jobs will be archived and no longer able to be retried. Make it empty to never expire jobs. It has to be no less than 1 day, for example: 15 days, 1 month, 2 years. + .form-group + .form-check + = f.check_box :protected_ci_variables, class: 'form-check-input' + = f.label :protected_ci_variables, class: 'form-check-label' do + = s_('AdminSettings|Environment variables are protected by default') + .form-text.text-muted + = s_('AdminSettings|When creating a new environment variable it will be protected by default.') = f.submit 'Save changes', class: "btn btn-success" diff --git a/app/views/ci/variables/_variable_row.html.haml b/app/views/ci/variables/_variable_row.html.haml index 6ee55836dd2..151a329228e 100644 --- a/app/views/ci/variables/_variable_row.html.haml +++ b/app/views/ci/variables/_variable_row.html.haml @@ -5,7 +5,8 @@ - id = variable&.id - key = variable&.key - value = variable&.value -- is_protected = variable && !only_key_value ? variable.protected : false +- is_protected_default = Gitlab::CurrentSettings.current_application_settings.protected_ci_variables +- is_protected = variable && !only_key_value ? variable.protected : is_protected_default - id_input_name = "#{form_field}[variables_attributes][][id]" - destroy_input_name = "#{form_field}[variables_attributes][][_destroy]" @@ -39,7 +40,8 @@ %input{ type: "hidden", class: 'js-ci-variable-input-protected js-project-feature-toggle-input', name: protected_input_name, - value: is_protected } + value: is_protected, + data: { default: is_protected_default.to_s } } %span.toggle-icon = sprite_icon('status_success_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-checked') = sprite_icon('status_failed_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-unchecked') diff --git a/db/migrate/20181031145139_add_protected_ci_variables_to_application_settings.rb b/db/migrate/20181031145139_add_protected_ci_variables_to_application_settings.rb new file mode 100644 index 00000000000..85ee34afe1e --- /dev/null +++ b/db/migrate/20181031145139_add_protected_ci_variables_to_application_settings.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddProtectedCiVariablesToApplicationSettings < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:application_settings, :protected_ci_variables, :boolean, default: false, allow_null: false) + end + + def down + remove_column(:application_settings, :protected_ci_variables) + end +end diff --git a/db/schema.rb b/db/schema.rb index 9c9c19aa897..a6ce749f45f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -166,6 +166,7 @@ ActiveRecord::Schema.define(version: 20181126153547) do t.integer "diff_max_patch_bytes", default: 102400, null: false t.integer "archive_builds_in_seconds" t.string "commit_email_hostname" + t.boolean "protected_ci_variables", default: false, null: false t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 172a0dc5e91..f06c1a24759 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -414,9 +414,15 @@ msgstr "" msgid "AdminProjects|Delete project" msgstr "" +msgid "AdminSettings|Environment variables are protected by default" +msgstr "" + msgid "AdminSettings|Specify a domain to use by default for every project's Auto Review Apps and Auto Deploy stages." msgstr "" +msgid "AdminSettings|When creating a new environment variable it will be protected by default." +msgstr "" + msgid "AdminUsers|Block user" msgstr "" diff --git a/spec/javascripts/ci_variable_list/ci_variable_list_spec.js b/spec/javascripts/ci_variable_list/ci_variable_list_spec.js index 30b15011def..bef59b86d0c 100644 --- a/spec/javascripts/ci_variable_list/ci_variable_list_spec.js +++ b/spec/javascripts/ci_variable_list/ci_variable_list_spec.js @@ -118,6 +118,8 @@ describe('VariableList', () => { loadFixtures('projects/ci_cd_settings.html.raw'); $wrapper = $('.js-ci-variable-list-section'); + $wrapper.find('.js-ci-variable-input-protected').attr('data-default', 'false'); + variableList = new VariableList({ container: $wrapper, formField: 'variables', diff --git a/spec/support/features/variable_list_shared_examples.rb b/spec/support/features/variable_list_shared_examples.rb index bce1fb01355..95f26a01f79 100644 --- a/spec/support/features/variable_list_shared_examples.rb +++ b/spec/support/features/variable_list_shared_examples.rb @@ -63,6 +63,44 @@ shared_examples 'variable list' do end end + context 'defaults to the application setting' do + context 'application setting is true' do + before do + stub_application_setting(protected_ci_variables: true) + end + + it 'defaults to protected' do + visit page_path + + page.within('.js-ci-variable-list-section .js-row:last-child') do + find('.js-ci-variable-input-key').set('key') + end + + values = all('.js-ci-variable-input-protected', visible: false).map(&:value) + + expect(values).to eq %w(false true true) + end + end + + context 'application setting is false' do + before do + stub_application_setting(protected_ci_variables: false) + end + + it 'defaults to unprotected' do + visit page_path + + page.within('.js-ci-variable-list-section .js-row:last-child') do + find('.js-ci-variable-input-key').set('key') + end + + values = all('.js-ci-variable-input-protected', visible: false).map(&:value) + + expect(values).to eq %w(false false false) + end + end + end + it 'reveals and hides variables' do page.within('.js-ci-variable-list-section') do expect(first('.js-ci-variable-input-key').value).to eq(variable.key) -- cgit v1.2.1 From a28335546b7e67d8db0120c85b29d46efb2658c0 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Thu, 1 Nov 2018 12:55:08 +0100 Subject: add changelog --- changelogs/unreleased/feature-option-to-make-variables-protected.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/feature-option-to-make-variables-protected.yml diff --git a/changelogs/unreleased/feature-option-to-make-variables-protected.yml b/changelogs/unreleased/feature-option-to-make-variables-protected.yml new file mode 100644 index 00000000000..c99c0481c35 --- /dev/null +++ b/changelogs/unreleased/feature-option-to-make-variables-protected.yml @@ -0,0 +1,5 @@ +--- +title: Add option to make ci variables protected by default +merge_request: 22744 +author: Alexis Reigel +type: added -- cgit v1.2.1 From 2f8297265855ccb811171a359841f4269189f312 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Tue, 27 Nov 2018 15:43:20 +0100 Subject: change copy to "Environment variables" --- app/views/ci/variables/_content.html.haml | 2 +- app/views/projects/settings/ci_cd/show.html.haml | 2 +- locale/gitlab.pot | 9 ++++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/views/ci/variables/_content.html.haml b/app/views/ci/variables/_content.html.haml index d355e7799df..fa82611d9c1 100644 --- a/app/views/ci/variables/_content.html.haml +++ b/app/views/ci/variables/_content.html.haml @@ -1 +1 @@ -= _('Variables are applied to environments via the runner. They can be protected by only exposing them to protected branches or tags. You can use variables for passwords, secret keys, or whatever you want.') += _('Environment variables are applied to environments via the runner. They can be protected by only exposing them to protected branches or tags. You can use environment variables for passwords, secret keys, or whatever you want.') diff --git a/app/views/projects/settings/ci_cd/show.html.haml b/app/views/projects/settings/ci_cd/show.html.haml index 98e2829ba43..cceb8abcd52 100644 --- a/app/views/projects/settings/ci_cd/show.html.haml +++ b/app/views/projects/settings/ci_cd/show.html.haml @@ -44,7 +44,7 @@ %section.qa-variables-settings.settings.no-animate{ class: ('expanded' if expanded) } .settings-header %h4 - = _('Variables') + = _('Environment variables') = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'variables'), target: '_blank', rel: 'noopener noreferrer' %button.btn.js-settings-toggle{ type: 'button' } = expanded ? _('Collapse') : _('Expand') diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f06c1a24759..b05e42a3704 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2669,6 +2669,12 @@ msgstr "" msgid "Enter the merge request title" msgstr "" +msgid "Environment variables" +msgstr "" + +msgid "Environment variables are applied to environments via the runner. They can be protected by only exposing them to protected branches or tags. You can use environment variables for passwords, secret keys, or whatever you want." +msgstr "" + msgid "Environments" msgstr "" @@ -7083,9 +7089,6 @@ msgstr "" msgid "Variables" msgstr "" -msgid "Variables are applied to environments via the runner. They can be protected by only exposing them to protected branches or tags. You can use variables for passwords, secret keys, or whatever you want." -msgstr "" - msgid "Various container registry settings." msgstr "" -- cgit v1.2.1 From ccecf436abdca602e703e09f1017f4376263c1d9 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Tue, 4 Dec 2018 13:50:20 +0100 Subject: extract common env. variable template to partial --- app/views/ci/variables/_header.html.haml | 11 +++++++++++ app/views/groups/settings/ci_cd/show.html.haml | 8 +------- app/views/projects/settings/ci_cd/show.html.haml | 8 +------- locale/gitlab.pot | 3 --- 4 files changed, 13 insertions(+), 17 deletions(-) create mode 100644 app/views/ci/variables/_header.html.haml diff --git a/app/views/ci/variables/_header.html.haml b/app/views/ci/variables/_header.html.haml new file mode 100644 index 00000000000..cb7779e2175 --- /dev/null +++ b/app/views/ci/variables/_header.html.haml @@ -0,0 +1,11 @@ +- expanded = local_assigns.fetch(:expanded) + +%h4 + = _('Environment variables') + = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'variables'), target: '_blank', rel: 'noopener noreferrer' + +%button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') + +%p.append-bottom-0 + = render "ci/variables/content" diff --git a/app/views/groups/settings/ci_cd/show.html.haml b/app/views/groups/settings/ci_cd/show.html.haml index a5e6abdba52..d9332e36ef5 100644 --- a/app/views/groups/settings/ci_cd/show.html.haml +++ b/app/views/groups/settings/ci_cd/show.html.haml @@ -5,13 +5,7 @@ %section.settings#ci-variables.no-animate{ class: ('expanded' if expanded) } .settings-header - %h4 - = _('Variables') - = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'variables'), target: '_blank', rel: 'noopener noreferrer' - %button.btn.btn-default.js-settings-toggle{ type: "button" } - = expanded ? _('Collapse') : _('Expand') - %p.append-bottom-0 - = render "ci/variables/content" + = render 'ci/variables/header', expanded: expanded .settings-content = render 'ci/variables/index', save_endpoint: group_variables_path diff --git a/app/views/projects/settings/ci_cd/show.html.haml b/app/views/projects/settings/ci_cd/show.html.haml index cceb8abcd52..6966bf96724 100644 --- a/app/views/projects/settings/ci_cd/show.html.haml +++ b/app/views/projects/settings/ci_cd/show.html.haml @@ -43,13 +43,7 @@ %section.qa-variables-settings.settings.no-animate{ class: ('expanded' if expanded) } .settings-header - %h4 - = _('Environment variables') - = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'variables'), target: '_blank', rel: 'noopener noreferrer' - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? _('Collapse') : _('Expand') - %p.append-bottom-0 - = render "ci/variables/content" + = render 'ci/variables/header', expanded: expanded .settings-content = render 'ci/variables/index', save_endpoint: project_variables_path(@project) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b05e42a3704..29855c97c7e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7086,9 +7086,6 @@ msgstr "" msgid "Users requesting access to" msgstr "" -msgid "Variables" -msgstr "" - msgid "Various container registry settings." msgstr "" -- cgit v1.2.1 From a648bcad7bb7fcf24c0be81abf273da8d0bbf410 Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Tue, 4 Dec 2018 14:40:47 +0100 Subject: extract protected variable logic to helper --- app/helpers/ci_variables_helper.rb | 15 +++++++++++++++ app/views/ci/variables/_variable_row.html.haml | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 app/helpers/ci_variables_helper.rb diff --git a/app/helpers/ci_variables_helper.rb b/app/helpers/ci_variables_helper.rb new file mode 100644 index 00000000000..e3728804c2a --- /dev/null +++ b/app/helpers/ci_variables_helper.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module CiVariablesHelper + def ci_variable_protected_by_default? + Gitlab::CurrentSettings.current_application_settings.protected_ci_variables + end + + def ci_variable_protected?(variable, only_key_value) + if variable && !only_key_value + variable.protected + else + ci_variable_protected_by_default? + end + end +end diff --git a/app/views/ci/variables/_variable_row.html.haml b/app/views/ci/variables/_variable_row.html.haml index 151a329228e..16a7527c8ce 100644 --- a/app/views/ci/variables/_variable_row.html.haml +++ b/app/views/ci/variables/_variable_row.html.haml @@ -5,8 +5,8 @@ - id = variable&.id - key = variable&.key - value = variable&.value -- is_protected_default = Gitlab::CurrentSettings.current_application_settings.protected_ci_variables -- is_protected = variable && !only_key_value ? variable.protected : is_protected_default +- is_protected_default = ci_variable_protected_by_default? +- is_protected = ci_variable_protected?(variable, only_key_value) - id_input_name = "#{form_field}[variables_attributes][][id]" - destroy_input_name = "#{form_field}[variables_attributes][][_destroy]" -- cgit v1.2.1 From cad0661aadff50b4d2c2b4cc7b012809b945213c Mon Sep 17 00:00:00 2001 From: Alexis Reigel Date: Tue, 4 Dec 2018 15:04:33 +0100 Subject: callout when ci variables are protected by default --- app/views/ci/variables/_index.html.haml | 5 +++++ locale/gitlab.pot | 3 +++ spec/support/features/variable_list_shared_examples.rb | 16 ++++++++++++---- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/app/views/ci/variables/_index.html.haml b/app/views/ci/variables/_index.html.haml index f34305e94fa..dc9ccb6cc39 100644 --- a/app/views/ci/variables/_index.html.haml +++ b/app/views/ci/variables/_index.html.haml @@ -1,5 +1,10 @@ - save_endpoint = local_assigns.fetch(:save_endpoint, nil) +- if ci_variable_protected_by_default? + %p.settings-message.text-center + - link_start = ''.html_safe % { url: help_page_path('ci/variables/README', anchor: 'protected-variables') } + = s_('Environment variables are configured by your administrator to be %{link_start}protected%{link_end} by default').html_safe % { link_start: link_start, link_end: ''.html_safe } + .row .col-lg-12.js-ci-variable-list-section{ data: { save_endpoint: save_endpoint } } .hide.alert.alert-danger.js-ci-variable-error-box diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 29855c97c7e..24b34eedf6a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2675,6 +2675,9 @@ msgstr "" msgid "Environment variables are applied to environments via the runner. They can be protected by only exposing them to protected branches or tags. You can use environment variables for passwords, secret keys, or whatever you want." msgstr "" +msgid "Environment variables are configured by your administrator to be %{link_start}protected%{link_end} by default" +msgstr "" + msgid "Environments" msgstr "" diff --git a/spec/support/features/variable_list_shared_examples.rb b/spec/support/features/variable_list_shared_examples.rb index 95f26a01f79..0a464d77cb7 100644 --- a/spec/support/features/variable_list_shared_examples.rb +++ b/spec/support/features/variable_list_shared_examples.rb @@ -67,11 +67,11 @@ shared_examples 'variable list' do context 'application setting is true' do before do stub_application_setting(protected_ci_variables: true) - end - it 'defaults to protected' do visit page_path + end + it 'defaults to protected' do page.within('.js-ci-variable-list-section .js-row:last-child') do find('.js-ci-variable-input-key').set('key') end @@ -80,16 +80,20 @@ shared_examples 'variable list' do expect(values).to eq %w(false true true) end + + it 'shows a message regarding the changed default' do + expect(page).to have_content 'Environment variables are configured by your administrator to be protected by default' + end end context 'application setting is false' do before do stub_application_setting(protected_ci_variables: false) - end - it 'defaults to unprotected' do visit page_path + end + it 'defaults to unprotected' do page.within('.js-ci-variable-list-section .js-row:last-child') do find('.js-ci-variable-input-key').set('key') end @@ -98,6 +102,10 @@ shared_examples 'variable list' do expect(values).to eq %w(false false false) end + + it 'does not show a message regarding the default' do + expect(page).not_to have_content 'Environment variables are configured by your administrator to be protected by default' + end end end -- cgit v1.2.1 From 212eb1bbe46455a6d7c7e023a894699d3c39dfe5 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Wed, 21 Nov 2018 15:28:34 +0800 Subject: Disable duplicate label merging in search bar dropdown --- .../javascripts/filtered_search/dropdown_utils.js | 61 ---------- .../filtered_search_visual_tokens.js | 19 +--- .../filtered_search/dropdown_utils_spec.js | 126 --------------------- .../filtered_search_visual_tokens_spec.js | 43 ------- 4 files changed, 1 insertion(+), 248 deletions(-) diff --git a/app/assets/javascripts/filtered_search/dropdown_utils.js b/app/assets/javascripts/filtered_search/dropdown_utils.js index 1b79a3320c6..8d92af2cf7e 100644 --- a/app/assets/javascripts/filtered_search/dropdown_utils.js +++ b/app/assets/javascripts/filtered_search/dropdown_utils.js @@ -54,67 +54,6 @@ export default class DropdownUtils { return updatedItem; } - static mergeDuplicateLabels(dataMap, newLabel) { - const updatedMap = dataMap; - const key = newLabel.title; - - const hasKeyProperty = Object.prototype.hasOwnProperty.call(updatedMap, key); - - if (!hasKeyProperty) { - updatedMap[key] = newLabel; - } else { - const existing = updatedMap[key]; - - if (!existing.multipleColors) { - existing.multipleColors = [existing.color]; - } - - existing.multipleColors.push(newLabel.color); - } - - return updatedMap; - } - - static duplicateLabelColor(labelColors) { - const colors = labelColors; - const spacing = 100 / colors.length; - - // Reduce the colors to 4 - colors.length = Math.min(colors.length, 4); - - const color = colors - .map((c, i) => { - const percentFirst = Math.floor(spacing * i); - const percentSecond = Math.floor(spacing * (i + 1)); - return `${c} ${percentFirst}%, ${c} ${percentSecond}%`; - }) - .join(', '); - - return `linear-gradient(${color})`; - } - - static duplicateLabelPreprocessing(data) { - const results = []; - const dataMap = {}; - - data.forEach(DropdownUtils.mergeDuplicateLabels.bind(null, dataMap)); - - Object.keys(dataMap).forEach(key => { - const label = dataMap[key]; - - if (label.multipleColors) { - label.color = DropdownUtils.duplicateLabelColor(label.multipleColors); - label.text_color = '#000000'; - } - - results.push(label); - }); - - results.preprocessed = true; - - return results; - } - static filterHint(config, item) { const { input, allowedKeys } = config; const updatedItem = item; diff --git a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js index 89dcff74d0e..fba31f16d65 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js +++ b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js @@ -79,11 +79,7 @@ export default class FilteredSearchVisualTokens { static setTokenStyle(tokenContainer, backgroundColor, textColor) { const token = tokenContainer; - // Labels with linear gradient should not override default background color - if (backgroundColor.indexOf('linear-gradient') === -1) { - token.style.backgroundColor = backgroundColor; - } - + token.style.backgroundColor = backgroundColor; token.style.color = textColor; if (textColor === '#FFFFFF') { @@ -94,18 +90,6 @@ export default class FilteredSearchVisualTokens { return token; } - static preprocessLabel(labelsEndpoint, labels) { - let processed = labels; - - if (!labels.preprocessed) { - processed = DropdownUtils.duplicateLabelPreprocessing(labels); - AjaxCache.override(labelsEndpoint, processed); - processed.preprocessed = true; - } - - return processed; - } - static updateLabelTokenColor(tokenValueContainer, tokenValue) { const filteredSearchInput = FilteredSearchContainer.container.querySelector('.filtered-search'); const { baseEndpoint } = filteredSearchInput.dataset; @@ -115,7 +99,6 @@ export default class FilteredSearchVisualTokens { ); return AjaxCache.retrieve(labelsEndpoint) - .then(FilteredSearchVisualTokens.preprocessLabel.bind(null, labelsEndpoint)) .then(labels => { const matchingLabel = (labels || []).find( label => `~${DropdownUtils.getEscapedText(label.title)}` === tokenValue, diff --git a/spec/javascripts/filtered_search/dropdown_utils_spec.js b/spec/javascripts/filtered_search/dropdown_utils_spec.js index 6605b0a30d7..cfd0b96ec43 100644 --- a/spec/javascripts/filtered_search/dropdown_utils_spec.js +++ b/spec/javascripts/filtered_search/dropdown_utils_spec.js @@ -211,132 +211,6 @@ describe('Dropdown Utils', () => { }); }); - describe('mergeDuplicateLabels', () => { - const dataMap = { - label: { - title: 'label', - color: '#FFFFFF', - }, - }; - - it('should add label to dataMap if it is not a duplicate', () => { - const newLabel = { - title: 'new-label', - color: '#000000', - }; - - const updated = DropdownUtils.mergeDuplicateLabels(dataMap, newLabel); - - expect(updated[newLabel.title]).toEqual(newLabel); - }); - - it('should merge colors if label is a duplicate', () => { - const duplicate = { - title: 'label', - color: '#000000', - }; - - const updated = DropdownUtils.mergeDuplicateLabels(dataMap, duplicate); - - expect(updated.label.multipleColors).toEqual([dataMap.label.color, duplicate.color]); - }); - }); - - describe('duplicateLabelColor', () => { - it('should linear-gradient 2 colors', () => { - const gradient = DropdownUtils.duplicateLabelColor(['#FFFFFF', '#000000']); - - expect(gradient).toEqual( - 'linear-gradient(#FFFFFF 0%, #FFFFFF 50%, #000000 50%, #000000 100%)', - ); - }); - - it('should linear-gradient 3 colors', () => { - const gradient = DropdownUtils.duplicateLabelColor(['#FFFFFF', '#000000', '#333333']); - - expect(gradient).toEqual( - 'linear-gradient(#FFFFFF 0%, #FFFFFF 33%, #000000 33%, #000000 66%, #333333 66%, #333333 100%)', - ); - }); - - it('should linear-gradient 4 colors', () => { - const gradient = DropdownUtils.duplicateLabelColor([ - '#FFFFFF', - '#000000', - '#333333', - '#DDDDDD', - ]); - - expect(gradient).toEqual( - 'linear-gradient(#FFFFFF 0%, #FFFFFF 25%, #000000 25%, #000000 50%, #333333 50%, #333333 75%, #DDDDDD 75%, #DDDDDD 100%)', - ); - }); - - it('should not linear-gradient more than 4 colors', () => { - const gradient = DropdownUtils.duplicateLabelColor([ - '#FFFFFF', - '#000000', - '#333333', - '#DDDDDD', - '#EEEEEE', - ]); - - expect(gradient.indexOf('#EEEEEE')).toBe(-1); - }); - }); - - describe('duplicateLabelPreprocessing', () => { - it('should set preprocessed to true', () => { - const results = DropdownUtils.duplicateLabelPreprocessing([]); - - expect(results.preprocessed).toEqual(true); - }); - - it('should not mutate existing data if there are no duplicates', () => { - const data = [ - { - title: 'label1', - color: '#FFFFFF', - }, - { - title: 'label2', - color: '#000000', - }, - ]; - const results = DropdownUtils.duplicateLabelPreprocessing(data); - - expect(results.length).toEqual(2); - expect(results[0]).toEqual(data[0]); - expect(results[1]).toEqual(data[1]); - }); - - describe('duplicate labels', () => { - const data = [ - { - title: 'label', - color: '#FFFFFF', - }, - { - title: 'label', - color: '#000000', - }, - ]; - const results = DropdownUtils.duplicateLabelPreprocessing(data); - - it('should merge duplicate labels', () => { - expect(results.length).toEqual(1); - }); - - it('should convert multiple colored labels into linear-gradient', () => { - expect(results[0].color).toEqual(DropdownUtils.duplicateLabelColor(['#FFFFFF', '#000000'])); - }); - - it('should set multiple colored label text color to black', () => { - expect(results[0].text_color).toEqual('#000000'); - }); - }); - }); - describe('setDataValueIfSelected', () => { beforeEach(() => { spyOn(FilteredSearchDropdownManager, 'addWordToInput').and.callFake(() => {}); diff --git a/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js b/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js index 4f561df7943..9aa3cbaa231 100644 --- a/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js @@ -909,16 +909,6 @@ describe('Filtered Search Visual Tokens', () => { expect(token.style.backgroundColor).not.toEqual(originalBackgroundColor); }); - it('should not set backgroundColor when it is a linear-gradient', () => { - const token = subject.setTokenStyle( - bugLabelToken, - 'linear-gradient(135deg, red, blue)', - 'white', - ); - - expect(token.style.backgroundColor).toEqual(bugLabelToken.style.backgroundColor); - }); - it('should set textColor', () => { const token = subject.setTokenStyle(bugLabelToken, 'white', 'black'); @@ -935,39 +925,6 @@ describe('Filtered Search Visual Tokens', () => { }); }); - describe('preprocessLabel', () => { - const endpoint = 'endpoint'; - - it('does not preprocess more than once', () => { - let labels = []; - - spyOn(DropdownUtils, 'duplicateLabelPreprocessing').and.callFake(() => []); - - labels = FilteredSearchVisualTokens.preprocessLabel(endpoint, labels); - FilteredSearchVisualTokens.preprocessLabel(endpoint, labels); - - expect(DropdownUtils.duplicateLabelPreprocessing.calls.count()).toEqual(1); - }); - - describe('not preprocessed before', () => { - it('returns preprocessed labels', () => { - let labels = []; - - expect(labels.preprocessed).not.toEqual(true); - labels = FilteredSearchVisualTokens.preprocessLabel(endpoint, labels); - - expect(labels.preprocessed).toEqual(true); - }); - - it('overrides AjaxCache with preprocessed results', () => { - spyOn(AjaxCache, 'override').and.callFake(() => {}); - FilteredSearchVisualTokens.preprocessLabel(endpoint, []); - - expect(AjaxCache.override.calls.count()).toEqual(1); - }); - }); - }); - describe('updateLabelTokenColor', () => { const jsonFixtureName = 'labels/project_labels.json'; const dummyEndpoint = '/dummy/endpoint'; -- cgit v1.2.1 From 16afcb5565d261061c28f929a21f69836b1d8f79 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Wed, 21 Nov 2018 16:12:33 +0800 Subject: Disable duplicate label merging in other dropdowns --- app/assets/javascripts/labels_select.js | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index c0a76814102..bb0af7de5da 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -7,7 +7,6 @@ import _ from 'underscore'; import { sprintf, __ } from './locale'; import axios from './lib/utils/axios_utils'; import IssuableBulkUpdateActions from './issuable_bulk_update_actions'; -import DropdownUtils from './filtered_search/dropdown_utils'; import CreateLabelDropdown from './create_label'; import flash from './flash'; import ModalStore from './boards/stores/modal_store'; @@ -171,23 +170,7 @@ export default class LabelsSelect { axios .get(labelUrl) .then(res => { - let data = _.chain(res.data) - .groupBy(function(label) { - return label.title; - }) - .map(function(label) { - var color; - color = _.map(label, function(dup) { - return dup.color; - }); - return { - id: label[0].id, - title: label[0].title, - color: color, - duplicate: color.length > 1, - }; - }) - .value(); + let data = res.data; if ($dropdown.hasClass('js-extra-options')) { var extraData = []; if (showNo) { @@ -272,15 +255,8 @@ export default class LabelsSelect { selectedClass.push('dropdown-clear-active'); } } - if (label.duplicate) { - color = DropdownUtils.duplicateLabelColor(label.color); - } else { - if (label.color != null) { - [color] = label.color; - } - } - if (color) { - colorEl = ""; + if (label.color) { + colorEl = ""; } else { colorEl = ''; } @@ -435,7 +411,7 @@ export default class LabelsSelect { new ListLabel({ id: label.id, title: label.title, - color: label.color[0], + color: label.color, textColor: '#fff', }), ); -- cgit v1.2.1 From 0ec5edd5bed6918e27330da3140cf87a008c06f3 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Wed, 21 Nov 2018 16:15:47 +0800 Subject: Allow adding of duplicate label names in boards --- app/assets/javascripts/boards/models/issue.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/boards/models/issue.js b/app/assets/javascripts/boards/models/issue.js index 5e0f0b07247..dd92d3c8552 100644 --- a/app/assets/javascripts/boards/models/issue.js +++ b/app/assets/javascripts/boards/models/issue.js @@ -55,12 +55,12 @@ class ListIssue { } findLabel(findLabel) { - return this.labels.filter(label => label.title === findLabel.title)[0]; + return this.labels.find(label => label.id === findLabel.id); } removeLabel(removeLabel) { if (removeLabel) { - this.labels = this.labels.filter(label => removeLabel.title !== label.title); + this.labels = this.labels.filter(label => removeLabel.id !== label.id); } } @@ -75,7 +75,7 @@ class ListIssue { } findAssignee(findAssignee) { - return this.assignees.filter(assignee => assignee.id === findAssignee.id)[0]; + return this.assignees.find(assignee => assignee.id === findAssignee.id); } removeAssignee(removeAssignee) { -- cgit v1.2.1 From cb5214f4807676c1a4d1e207814b0b658b0b6979 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Wed, 21 Nov 2018 17:23:26 +0800 Subject: Allow creating label lists with the same label name --- app/assets/javascripts/boards/components/new_list_dropdown.js | 4 ++-- app/assets/javascripts/boards/stores/boards_store.js | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/boards/components/new_list_dropdown.js b/app/assets/javascripts/boards/components/new_list_dropdown.js index f7016561f93..10577da9305 100644 --- a/app/assets/javascripts/boards/components/new_list_dropdown.js +++ b/app/assets/javascripts/boards/components/new_list_dropdown.js @@ -37,7 +37,7 @@ export default function initNewListDropdown() { }); }, renderRow(label) { - const active = boardsStore.findList('title', label.title); + const active = boardsStore.findListByLabelId(label.id); const $li = $('
  • '); const $a = $('', { class: active ? `is-active js-board-list-${active.id}` : '', @@ -63,7 +63,7 @@ export default function initNewListDropdown() { const label = options.selectedObj; e.preventDefault(); - if (!boardsStore.findList('title', label.title)) { + if (!boardsStore.findListByLabelId(label.id)) { boardsStore.new({ title: label.title, position: boardsStore.state.lists.length - 2, diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js index cf88a973d33..01fc1cb5af3 100644 --- a/app/assets/javascripts/boards/stores/boards_store.js +++ b/app/assets/javascripts/boards/stores/boards_store.js @@ -166,6 +166,11 @@ const boardsStore = { }); return filteredList[0]; }, + findListByLabelId(id) { + return this.state.lists.find(list => { + return list.type === 'label' && list.label.id === id; + }); + }, updateFiltersUrl() { window.history.pushState(null, null, `?${this.filter.path}`); }, -- cgit v1.2.1 From 2dd84abf41d7294cf52f18ce059356f33a12dcdf Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Wed, 21 Nov 2018 17:32:00 +0800 Subject: Add changelog entry --- changelogs/unreleased/51994-disable-merging-labels-in-dropdowns.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/51994-disable-merging-labels-in-dropdowns.yml diff --git a/changelogs/unreleased/51994-disable-merging-labels-in-dropdowns.yml b/changelogs/unreleased/51994-disable-merging-labels-in-dropdowns.yml new file mode 100644 index 00000000000..2d54cf814b7 --- /dev/null +++ b/changelogs/unreleased/51994-disable-merging-labels-in-dropdowns.yml @@ -0,0 +1,5 @@ +--- +title: Disable merging of labels with same names +merge_request: 23265 +author: +type: changed -- cgit v1.2.1 From f57b1323ffd72607eb72ec670a27cbf572732d96 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Thu, 22 Nov 2018 00:04:28 +0800 Subject: Fix failed tests and add extra test --- app/assets/javascripts/boards/stores/boards_store.js | 4 +--- app/assets/javascripts/labels_select.js | 5 +++-- spec/javascripts/boards/boards_store_spec.js | 7 +++++++ spec/javascripts/boards/issue_spec.js | 18 +++++++++++++++--- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js index 01fc1cb5af3..802796208c2 100644 --- a/app/assets/javascripts/boards/stores/boards_store.js +++ b/app/assets/javascripts/boards/stores/boards_store.js @@ -167,9 +167,7 @@ const boardsStore = { return filteredList[0]; }, findListByLabelId(id) { - return this.state.lists.find(list => { - return list.type === 'label' && list.label.id === id; - }); + return this.state.lists.find(list => list.type === 'label' && list.label.id === id); }, updateFiltersUrl() { window.history.pushState(null, null, `?${this.filter.path}`); diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index bb0af7de5da..f7a611fbca0 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -170,7 +170,7 @@ export default class LabelsSelect { axios .get(labelUrl) .then(res => { - let data = res.data; + let { data } = res; if ($dropdown.hasClass('js-extra-options')) { var extraData = []; if (showNo) { @@ -256,7 +256,8 @@ export default class LabelsSelect { } } if (label.color) { - colorEl = ""; + colorEl = + ""; } else { colorEl = ''; } diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js index 54f1edfb1f9..22f192bc7f3 100644 --- a/spec/javascripts/boards/boards_store_spec.js +++ b/spec/javascripts/boards/boards_store_spec.js @@ -65,6 +65,13 @@ describe('Store', () => { expect(list).toBeDefined(); }); + it('finds list by label ID', () => { + boardsStore.addList(listObj); + const list = boardsStore.findListByLabelId(listObj.label.id); + + expect(list.id).toBe(listObj.id); + }); + it('gets issue when new list added', done => { boardsStore.addList(listObj); const list = boardsStore.findList('id', listObj.id); diff --git a/spec/javascripts/boards/issue_spec.js b/spec/javascripts/boards/issue_spec.js index 437ab4bb3df..54fb0e8228b 100644 --- a/spec/javascripts/boards/issue_spec.js +++ b/spec/javascripts/boards/issue_spec.js @@ -55,15 +55,27 @@ describe('Issue model', () => { expect(issue.labels.length).toBe(2); }); - it('does not add existing label', () => { + it('does not add label if label id exists', () => { + issue.addLabel({ + id: 1, + title: 'test 2', + color: 'blue', + description: 'testing', + }); + + expect(issue.labels.length).toBe(1); + expect(issue.labels[0].color).toBe('red'); + }); + + it('adds other label with same title', () => { issue.addLabel({ id: 2, title: 'test', color: 'blue', - description: 'bugs!', + description: 'other test', }); - expect(issue.labels.length).toBe(1); + expect(issue.labels.length).toBe(2); }); it('finds label', () => { -- cgit v1.2.1 From d3b14e9d87cb81262a02d9367a344fcc34deeb96 Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Fri, 9 Nov 2018 06:34:48 +0000 Subject: Document the "Abuse reports" feature Add documentation for both admins (/user/admin_area/abuse_reports.md) and non-admins (/user/abuse_reports.md) to help them use the abuse reports feature --- doc/administration/index.md | 1 + doc/user/abuse_reports.md | 53 +++++++++++++++++++++ doc/user/admin_area/abuse_reports.md | 31 ++++++++++++ .../admin_area/img/abuse_report_blocked_user.png | Bin 0 -> 13821 bytes doc/user/admin_area/img/abuse_reports_page.png | Bin 0 -> 215813 bytes doc/user/index.md | 1 + doc/user/profile/account/delete_account.md | 3 +- 7 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 doc/user/abuse_reports.md create mode 100644 doc/user/admin_area/abuse_reports.md create mode 100644 doc/user/admin_area/img/abuse_report_blocked_user.png create mode 100644 doc/user/admin_area/img/abuse_reports_page.png diff --git a/doc/administration/index.md b/doc/administration/index.md index 6083806af6c..89132cd95f0 100644 --- a/doc/administration/index.md +++ b/doc/administration/index.md @@ -95,6 +95,7 @@ created in snippets, wikis, and repos. - [Postfix for incoming email](reply_by_email_postfix_setup.md): Set up a basic Postfix mail server with IMAP authentication on Ubuntu for incoming emails. +- [Abuse reports](../user/admin_area/abuse_reports.md): View and resolve abuse reports from your users. [reply by email]: reply_by_email.md [issues by email]: ../user/project/issues/create_new_issue.md#new-issue-via-email diff --git a/doc/user/abuse_reports.md b/doc/user/abuse_reports.md new file mode 100644 index 00000000000..1f4f598b121 --- /dev/null +++ b/doc/user/abuse_reports.md @@ -0,0 +1,53 @@ +# Abuse reports + +Report abuse from users to GitLab administrators. + +You can report a user through their: + +- [Profile](#reporting-abuse-through-a-users-profile) +- [Comments](#reporting-abuse-through-a-users-comment) +- [Issues and Merge requests](#reporting-abuse-through-a-users-issue-or-merge-request) + +## Reporting abuse through a user's profile + +To report abuse from a user's profile page: + +1. Click on the exclamation point report abuse button at the top right of the user's profile. +1. Complete an abuse report. +1. Click the **Send report** button. + +## Reporting abuse through a user's comment + +To report abuse from a user's comment: + +1. Click on the vertical ellipsis (⋮) more actions button to open the dropdown. +1. Select **Report as abuse**. +1. Complete an abuse report. +1. Click the **Send report** button. + + +NOTE: **Note:** +A URL to the reported user's comment will be +pre-filled in the abuse report's **Message** field. + +## Reporting abuse through a user's issue or merge request + +The **Report abuse** button is displayed at the top right of the issue or merge request: + +- When **Report abuse** is selected from the menu that appears when the **Close issue** or **Close merge request** button is clicked, for users that have permission to close the issue or merge request. +- When viewing the issue or merge request, for users that don't have permission to close the issue or merge request. + +With the **Report abuse** button displayed, to submit an abuse report: + +1. Click the **Report abuse** button. +1. Submit an abuse report. +1. Click the **Send report** button. + +NOTE: **Note:** +A URL to the reported user's issue or merge request will be pre-filled +in the abuse report's **Message** field. + +## Managing abuse reports + +Admins are able to view and resolve abuse reports. +For more information, see [abuse reports administration documentation](admin_area/abuse_reports.md). diff --git a/doc/user/admin_area/abuse_reports.md b/doc/user/admin_area/abuse_reports.md new file mode 100644 index 00000000000..01c2d9607f5 --- /dev/null +++ b/doc/user/admin_area/abuse_reports.md @@ -0,0 +1,31 @@ +# Abuse reports + +View and resolve abuse reports from GitLab users. + +Admins can view abuse reports in the admin area and are able to +resolve the reports by removing the reported user, blocking the reported user, or removing the report. + +## Reporting abuse + +To find out more about reporting abuse, see [abuse reports user documentation](../abuse_reports.md). + +## Resolving abuse reports + +To access abuse reports, go to **Admin area > Abuse Reports**. + +There are 3 ways to resolve an abuse report, with a button for each method: + +- Remove user & report: [Deletes the reported user](../profile/account/delete_account.md) from the instance and removes the abuse report from the list. +- Block user: Blocks the reported user from the instance and does not remove the abuse report from the list. +- Remove report: Removes the abuse report from the list and does not restrict the access for the reported user. + +![abuse-reports-page-image](img/abuse_reports_page.png) + +## Blocked users + +Blocking a user will not remove the abuse report from the list. + +Instead, the block button will be disabled and explain that the user is "Already blocked". +You are still able to remove the user and/or report if necessary. + +![abuse-report-blocked-user-image](img/abuse_report_blocked_user.png) diff --git a/doc/user/admin_area/img/abuse_report_blocked_user.png b/doc/user/admin_area/img/abuse_report_blocked_user.png new file mode 100644 index 00000000000..0cb4c7bb8ac Binary files /dev/null and b/doc/user/admin_area/img/abuse_report_blocked_user.png differ diff --git a/doc/user/admin_area/img/abuse_reports_page.png b/doc/user/admin_area/img/abuse_reports_page.png new file mode 100644 index 00000000000..81dbe976cda Binary files /dev/null and b/doc/user/admin_area/img/abuse_reports_page.png differ diff --git a/doc/user/index.md b/doc/user/index.md index 08995032cb1..fc68404d0c2 100644 --- a/doc/user/index.md +++ b/doc/user/index.md @@ -113,6 +113,7 @@ methods available in GitLab. user type (guest, reporter, developer, maintainer, owner). - [Feature highlight](feature_highlight.md): Learn more about the little blue dots around the app that explain certain features +- [Abuse reports](abuse_reports.md): Report abuse from users to GitLab administrators ## Groups diff --git a/doc/user/profile/account/delete_account.md b/doc/user/profile/account/delete_account.md index 49f0ce2cd79..b497cc414af 100644 --- a/doc/user/profile/account/delete_account.md +++ b/doc/user/profile/account/delete_account.md @@ -25,7 +25,7 @@ Here's a list of things that will not be deleted: Instead of being deleted, these records will be moved to a system-wide "Ghost User", whose sole purpose is to act as a container for such records. -When a user is deleted from an abuse report or spam log, these associated +When a user is deleted from an [abuse report](../../admin_area/abuse_reports.md) or spam log, these associated records are not ghosted and will be removed, along with any groups the user is a sole owner of. Administrators can also request this behaviour when deleting users from the [API](../../../api/users.md#user-deletion) or the @@ -35,4 +35,3 @@ admin area. [ce-10273]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10273 [ce-10467]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10467 [ce-11853]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11853 - -- cgit v1.2.1 From 7827eae5b2e1c8a019a8ce859f45ff4936e57227 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Tue, 23 Oct 2018 12:21:58 +0000 Subject: ci/yaml: remove "recursion" mention if merging is not recursive --- doc/ci/yaml/README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 1277d1fdf8b..47da92f5d5e 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1716,13 +1716,17 @@ include: --- -Since GitLab 10.8 we are now recursively merging the files defined in `include` +Since GitLab 10.8 we are now deep merging the files defined in `include` with those in `.gitlab-ci.yml`. Files defined by `include` are always -evaluated first and recursively merged with the content of `.gitlab-ci.yml`, no +evaluated first and merged with the content of `.gitlab-ci.yml`, no matter the position of the `include` keyword. You can take advantage of -recursive merging to customize and override details in included CI +merging to customize and override details in included CI configurations with local definitions. +NOTE: **Note:** +The recursive includes are not supported, meaning your external files +should not use the `include` keyword, as it will be ignored. + The following example shows specific YAML-defined variables and details of the `production` job from an include file being customized in `.gitlab-ci.yml`. @@ -1772,11 +1776,7 @@ with the environment url of the `production` job defined in `autodevops-template.yml` have been overridden by new values defined in `.gitlab-ci.yml`. -NOTE: **Note:** -Recursive includes are not supported meaning your external files -should not use the `include` keyword, as it will be ignored. - -Recursive merging lets you extend and override dictionary mappings, but +The merging lets you extend and override dictionary mappings, but you cannot add or modify items to an included array. For example, to add an additional item to the production job script, you must repeat the existing script items. -- cgit v1.2.1 From 9f16b2497a42c54e4c479e6c076a1dc0159432c4 Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Tue, 4 Dec 2018 14:25:26 +0100 Subject: Adjust padding of .dropdown-title to comply with design specs --- app/assets/stylesheets/framework/dropdowns.scss | 5 +++-- changelogs/unreleased/winh-dropdown-title-padding.yml | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/winh-dropdown-title-padding.yml diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index f273eb9533d..ffd02630d37 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -530,8 +530,9 @@ .dropdown-title { position: relative; - padding: 2px 25px 10px; - margin: 0 10px 10px; + padding: $dropdown-item-padding-y $dropdown-item-padding-x; + padding-bottom: #{2 * $dropdown-item-padding-y}; + margin-bottom: $dropdown-item-padding-y; font-weight: $gl-font-weight-bold; line-height: 1; text-align: center; diff --git a/changelogs/unreleased/winh-dropdown-title-padding.yml b/changelogs/unreleased/winh-dropdown-title-padding.yml new file mode 100644 index 00000000000..9d65175b536 --- /dev/null +++ b/changelogs/unreleased/winh-dropdown-title-padding.yml @@ -0,0 +1,5 @@ +--- +title: Adjust padding of .dropdown-title to comply with design specs +merge_request: 23546 +author: +type: changed -- cgit v1.2.1 From db267fa31bcbf991a2efb932b85a8935a13b0941 Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Tue, 11 Dec 2018 11:29:58 +0500 Subject: Fix RSpec/HookArgument rubocop offense --- .rubocop_todo.yml | 11 ----------- spec/spec_helper.rb | 6 +++--- spec/support/carrierwave.rb | 2 +- spec/support/db_cleaner.rb | 6 +++--- spec/support/setup_builds_storage.rb | 2 +- 5 files changed, 8 insertions(+), 19 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 571df7534cb..6c16442845d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -155,17 +155,6 @@ RSpec/ExpectChange: RSpec/ExpectInHook: Enabled: false -# Offense count: 7 -# Configuration parameters: EnforcedStyle. -# SupportedStyles: implicit, each, example -RSpec/HookArgument: - Exclude: - - 'spec/spec_helper.rb' - - 'spec/support/carrierwave.rb' - - 'spec/support/db_cleaner.rb' - - 'spec/support/gitaly.rb' - - 'spec/support/setup_builds_storage.rb' - # Offense count: 19 # Configuration parameters: EnforcedStyle. # SupportedStyles: it_behaves_like, it_should_behave_like diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3fedb9ed48c..8e424afffa5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -115,7 +115,7 @@ RSpec.configure do |config| TestEnv.clean_test_path end - config.before(:example) do + config.before do # Enable all features by default for testing allow(Feature).to receive(:enabled?) { true } @@ -136,11 +136,11 @@ RSpec.configure do |config| RequestStore.clear! end - config.after(:example) do + config.after do Fog.unmock! if Fog.mock? end - config.after(:example) do + config.after do Gitlab::CurrentSettings.clear_in_memory_application_settings! end diff --git a/spec/support/carrierwave.rb b/spec/support/carrierwave.rb index b4b016e408f..b376822d530 100644 --- a/spec/support/carrierwave.rb +++ b/spec/support/carrierwave.rb @@ -1,7 +1,7 @@ CarrierWave.root = File.expand_path('tmp/tests/public', Rails.root) RSpec.configure do |config| - config.after(:each) do + config.after do FileUtils.rm_rf(CarrierWave.root) end end diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index 5edc5de2a09..34b9efaaecd 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -23,7 +23,7 @@ RSpec.configure do |config| DatabaseCleaner.clean_with(:deletion, cache_tables: false) end - config.before(:each) do + config.before do DatabaseCleaner.strategy = :transaction end @@ -39,11 +39,11 @@ RSpec.configure do |config| DatabaseCleaner.strategy = :deletion, { cache_tables: false } end - config.before(:each) do + config.before do DatabaseCleaner.start end - config.append_after(:each) do + config.append_after do DatabaseCleaner.clean end end diff --git a/spec/support/setup_builds_storage.rb b/spec/support/setup_builds_storage.rb index 2e7c88bfc09..1d2a4856724 100644 --- a/spec/support/setup_builds_storage.rb +++ b/spec/support/setup_builds_storage.rb @@ -11,7 +11,7 @@ RSpec.configure do |config| FileUtils.mkdir_p(builds_path) end - config.before(:each) do + config.before do FileUtils.rm_rf(builds_path) FileUtils.mkdir_p(builds_path) end -- cgit v1.2.1 From d0daa1591b7e4dc8cf5ba787420d09cb7e76d8d7 Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Tue, 11 Dec 2018 13:41:03 +0200 Subject: Rename load_certs and include default cert file --- lib/gitlab/gitaly_client.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index cb6601786dc..586d8650db1 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -66,15 +66,20 @@ module Gitlab end end - def self.load_certs - @certs ||= Dir["#{OpenSSL::X509::DEFAULT_CERT_DIR}/*"].map do |cert| + def self.certs + return @certs if @certs + + cert_paths = Dir["#{OpenSSL::X509::DEFAULT_CERT_DIR}/*"] + cert_paths << OpenSSL::X509::DEFAULT_CERT_FILE if File.exist? OpenSSL::X509::DEFAULT_CERT_FILE + + @certs = cert_paths.map do |cert| File.read(cert) end.join("\n") end def self.stub_creds(storage) if URI(address(storage)).scheme == 'tls' - GRPC::Core::ChannelCredentials.new load_certs + GRPC::Core::ChannelCredentials.new certs else :this_channel_is_insecure end -- cgit v1.2.1 From fb11bc8ab05102dec19fcd375ee618d87ff79201 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Tue, 11 Dec 2018 18:08:50 +0100 Subject: Revert "Document gitaly network architecture" This reverts commit cc7353523bc1d19054769d7a0a61b0cb7f6ce4e3. I pushed this commit to master accidentally. Oops! --- doc/administration/gitaly/index.md | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index d7c45e7d91d..dc6a71e2ebd 100644 --- a/doc/administration/gitaly/index.md +++ b/doc/administration/gitaly/index.md @@ -49,25 +49,6 @@ Starting with GitLab 11.4, Gitaly is a replacement for NFS except when the [Elastic Search indexer](https://gitlab.com/gitlab-org/gitlab-elasticsearch-indexer) is used. -### Network architecture - -- gitlab-rails shards repositories into "repository storages" -- gitlab-rails/config/gitlab.yml contains a map from storage names to - (Gitaly address, Gitaly token) pairs -- the `storage name` -\> `(Gitaly address, Gitaly token)` map in - gitlab.yml is the single source of truth for the Gitaly network - topology -- a (Gitaly address, Gitaly token) corresponds to a Gitaly server -- a Gitaly server hosts one or more storages -- Gitaly addresses must be specified in such a way that they resolve - correctly for ALL Gitaly clients -- Gitaly clients are: unicorn, sidekiq, gitlab-workhorse, - gitlab-shell, and Gitaly itself -- special case: a Gitaly server must be able to make RPC calls **to - itself** via its own (Gitaly address, Gitaly token) pair as - specified in gitlab-rails/config/gitlab.yml -- Gitaly servers must not be exposed to the public internet - Gitaly network traffic is unencrypted so you should use a firewall to restrict access to your Gitaly server. -- cgit v1.2.1 From 3ee0710d1d47bec895568563aeca2d3b53bfa8ce Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Tue, 11 Dec 2018 16:52:22 +0000 Subject: Validate LFS hrefs before downloading them --- app/services/projects/lfs_pointers/lfs_download_service.rb | 3 +++ changelogs/unreleased/security-2754-fix-lfs-import.yml | 5 +++++ .../projects/lfs_pointers/lfs_download_service_spec.rb | 12 ++++++++++++ 3 files changed, 20 insertions(+) create mode 100644 changelogs/unreleased/security-2754-fix-lfs-import.yml diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb index 1c4a8d05be6..f9b9781ad5f 100644 --- a/app/services/projects/lfs_pointers/lfs_download_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_service.rb @@ -4,6 +4,8 @@ module Projects module LfsPointers class LfsDownloadService < BaseService + VALID_PROTOCOLS = %w[http https].freeze + # rubocop: disable CodeReuse/ActiveRecord def execute(oid, url) return unless project&.lfs_enabled? && oid.present? && url.present? @@ -11,6 +13,7 @@ module Projects return if LfsObject.exists?(oid: oid) sanitized_uri = Gitlab::UrlSanitizer.new(url) + Gitlab::UrlBlocker.validate!(sanitized_uri.sanitized_url, protocols: VALID_PROTOCOLS) with_tmp_file(oid) do |file| size = download_and_save_file(file, sanitized_uri) diff --git a/changelogs/unreleased/security-2754-fix-lfs-import.yml b/changelogs/unreleased/security-2754-fix-lfs-import.yml new file mode 100644 index 00000000000..e8e74c9c3f6 --- /dev/null +++ b/changelogs/unreleased/security-2754-fix-lfs-import.yml @@ -0,0 +1,5 @@ +--- +title: Validate LFS hrefs before downloading them +merge_request: +author: +type: security diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb index 6af5bfc7689..d7d7f1874eb 100644 --- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb @@ -54,6 +54,18 @@ describe Projects::LfsPointers::LfsDownloadService do end end + context 'when a bad URL is used' do + where(download_link: ['/etc/passwd', 'ftp://example.com', 'http://127.0.0.2']) + + with_them do + it 'does not download the file' do + expect(subject).not_to receive(:download_and_save_file) + + expect { subject.execute(oid, download_link) }.not_to change { LfsObject.count } + end + end + end + context 'when an lfs object with the same oid already exists' do before do create(:lfs_object, oid: 'oid') -- cgit v1.2.1 From 1d241206b59ee9c7160642e1cb3672c4e4375945 Mon Sep 17 00:00:00 2001 From: Chris Baumbauer Date: Tue, 11 Dec 2018 15:03:31 -0800 Subject: Upgrade Knative from 0.1.3 to 0.2.2 --- app/models/clusters/applications/knative.rb | 2 +- changelogs/unreleased/triggermesh-knative-version.yml | 5 +++++ spec/models/clusters/applications/knative_spec.rb | 6 +++--- 3 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/triggermesh-knative-version.yml diff --git a/app/models/clusters/applications/knative.rb b/app/models/clusters/applications/knative.rb index 168a24da738..0c72d7d8340 100644 --- a/app/models/clusters/applications/knative.rb +++ b/app/models/clusters/applications/knative.rb @@ -3,7 +3,7 @@ module Clusters module Applications class Knative < ActiveRecord::Base - VERSION = '0.1.3'.freeze + VERSION = '0.2.2'.freeze REPOSITORY = 'https://storage.googleapis.com/triggermesh-charts'.freeze FETCH_IP_ADDRESS_DELAY = 30.seconds diff --git a/changelogs/unreleased/triggermesh-knative-version.yml b/changelogs/unreleased/triggermesh-knative-version.yml new file mode 100644 index 00000000000..27f400962da --- /dev/null +++ b/changelogs/unreleased/triggermesh-knative-version.yml @@ -0,0 +1,5 @@ +--- +title: Knative version bump 0.1.3 -> 0.2.2 +merge_request: +author: Chris Baumbauer +type: changed diff --git a/spec/models/clusters/applications/knative_spec.rb b/spec/models/clusters/applications/knative_spec.rb index a1579b90436..809880f5969 100644 --- a/spec/models/clusters/applications/knative_spec.rb +++ b/spec/models/clusters/applications/knative_spec.rb @@ -33,10 +33,10 @@ describe Clusters::Applications::Knative do end context 'application install previously errored with older version' do - let(:application) { create(:clusters_applications_knative, :scheduled, version: '0.1.3') } + let(:application) { create(:clusters_applications_knative, :scheduled, version: '0.2.2') } it 'updates the application version' do - expect(application.reload.version).to eq('0.1.3') + expect(application.reload.version).to eq('0.2.2') end end end @@ -105,7 +105,7 @@ describe Clusters::Applications::Knative do it 'should be initialized with knative arguments' do expect(subject.name).to eq('knative') expect(subject.chart).to eq('knative/knative') - expect(subject.version).to eq('0.1.3') + expect(subject.version).to eq('0.2.2') expect(subject.files).to eq(knative.files) end end -- cgit v1.2.1 From 05dcb2dd76f2a9890398543f6de1516f5ec0b379 Mon Sep 17 00:00:00 2001 From: Sean Nichols Date: Sat, 8 Dec 2018 00:50:56 -0500 Subject: Display empty files properly on MR diffs --- app/assets/javascripts/diffs/components/diff_file.vue | 3 +++ app/serializers/diff_file_entity.rb | 1 + changelogs/unreleased/54786-mr-empty-file-display.yml | 5 +++++ lib/gitlab/diff/file.rb | 4 ++++ 4 files changed, 13 insertions(+) create mode 100644 changelogs/unreleased/54786-mr-empty-file-display.yml diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index bed29efb253..d397ffbefe3 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -176,6 +176,9 @@ export default { {{ __('This source diff could not be displayed because it is too large.') }} +
    + {{ __('Empty file') }} +
    diff --git a/app/serializers/diff_file_entity.rb b/app/serializers/diff_file_entity.rb index f0881829efd..b0aaec3326d 100644 --- a/app/serializers/diff_file_entity.rb +++ b/app/serializers/diff_file_entity.rb @@ -5,6 +5,7 @@ class DiffFileEntity < DiffFileBaseEntity include IconsHelper expose :too_large?, as: :too_large + expose :empty?, as: :empty expose :added_lines expose :removed_lines diff --git a/changelogs/unreleased/54786-mr-empty-file-display.yml b/changelogs/unreleased/54786-mr-empty-file-display.yml new file mode 100644 index 00000000000..b2ee9069234 --- /dev/null +++ b/changelogs/unreleased/54786-mr-empty-file-display.yml @@ -0,0 +1,5 @@ +--- +title: Display empty files properly on MR diffs +merge_request: +author: Sean Nichols +type: fixed diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index 8ba44dff06f..0be463f16a2 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -245,6 +245,10 @@ module Gitlab end # rubocop: enable CodeReuse/ActiveRecord + def empty? + valid_blobs.map(&:empty?).all? + end + def raw_binary? try_blobs(:raw_binary?) end -- cgit v1.2.1 From e6a3898e907d9e211415e298e521e87da584ca66 Mon Sep 17 00:00:00 2001 From: Sean Nichols Date: Sat, 8 Dec 2018 01:31:47 -0500 Subject: Fix Prettier formatting and add MR to changelog --- app/assets/javascripts/diffs/components/diff_file.vue | 4 +--- changelogs/unreleased/54786-mr-empty-file-display.yml | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index d397ffbefe3..836f1af881c 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -176,9 +176,7 @@ export default { {{ __('This source diff could not be displayed because it is too large.') }} -
    - {{ __('Empty file') }} -
    +
    {{ __('Empty file') }}
    diff --git a/changelogs/unreleased/54786-mr-empty-file-display.yml b/changelogs/unreleased/54786-mr-empty-file-display.yml index b2ee9069234..5adf5744755 100644 --- a/changelogs/unreleased/54786-mr-empty-file-display.yml +++ b/changelogs/unreleased/54786-mr-empty-file-display.yml @@ -1,5 +1,5 @@ --- title: Display empty files properly on MR diffs -merge_request: +merge_request: 23671 author: Sean Nichols type: fixed -- cgit v1.2.1 From 48d3fff062ca923bfecb9e86b0dc67d2109c36db Mon Sep 17 00:00:00 2001 From: Sean Nichols Date: Wed, 12 Dec 2018 00:00:50 -0500 Subject: Update localization files --- locale/gitlab.pot | 3 +++ 1 file changed, 3 insertions(+) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 032498babec..339d422735c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2656,6 +2656,9 @@ msgstr "" msgid "Embed" msgstr "" +msgid "Empty file" +msgstr "" + msgid "Enable" msgstr "" -- cgit v1.2.1 From dc1871b433ed898fac94c83d88bfd84dedb05488 Mon Sep 17 00:00:00 2001 From: Enrique Alvarez Date: Wed, 12 Dec 2018 06:08:57 +0000 Subject: Fix missing empty space that makes rendered gitlab documentation page missing list. Different output between gitlab internal .md reader and docs.gitlab.com rendered documentation. --- doc/user/project/integrations/prometheus.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/user/project/integrations/prometheus.md b/doc/user/project/integrations/prometheus.md index 7d0e567cae7..d9a2eeb32ae 100644 --- a/doc/user/project/integrations/prometheus.md +++ b/doc/user/project/integrations/prometheus.md @@ -41,6 +41,7 @@ Once you have a connected Kubernetes cluster with Helm installed, deploying a ma Prometheus is deployed into the `gitlab-managed-apps` namespace, using the [official Helm chart](https://github.com/kubernetes/charts/tree/master/stable/prometheus). Prometheus is only accessible within the cluster, with GitLab communicating through the [Kubernetes API](https://kubernetes.io/docs/concepts/overview/kubernetes-api/). The Prometheus server will [automatically detect and monitor](https://prometheus.io/docs/prometheus/latest/configuration/configuration/#%3Ckubernetes_sd_config%3E) nodes, pods, and endpoints. To configure a resource to be monitored by Prometheus, simply set the following [Kubernetes annotations](https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/): + * `prometheus.io/scrape` to `true` to enable monitoring of the resource. * `prometheus.io/port` to define the port of the metrics endpoint. * `prometheus.io/path` to define the path of the metrics endpoint. Defaults to `/metrics`. -- cgit v1.2.1 From 2463da920a5188b76f657952d3c866c12b528c37 Mon Sep 17 00:00:00 2001 From: Paul Gear Date: Tue, 11 Dec 2018 22:19:17 +0700 Subject: 54376 Fix border radius for omniauth signin block --- app/assets/stylesheets/pages/login.scss | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/pages/login.scss b/app/assets/stylesheets/pages/login.scss index fa0ab1a3bae..67d7a8175ac 100644 --- a/app/assets/stylesheets/pages/login.scss +++ b/app/assets/stylesheets/pages/login.scss @@ -49,8 +49,8 @@ .login-box, .omniauth-container { box-shadow: 0 0 0 1px $border-color; - border-bottom-right-radius: 2px; - border-bottom-left-radius: 2px; + border-bottom-right-radius: $border-radius-small; + border-bottom-left-radius: $border-radius-small; padding: 15px; .login-heading h3 { @@ -95,6 +95,7 @@ } .omniauth-container { + border-radius: $border-radius-small; font-size: 13px; p { -- cgit v1.2.1 From 2b488c466393b3c2b08801329ccf124a3e45da10 Mon Sep 17 00:00:00 2001 From: Paul Gear Date: Tue, 11 Dec 2018 22:19:40 +0700 Subject: 54376 Add margin to .row on signin page --- app/views/layouts/devise.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/layouts/devise.html.haml b/app/views/layouts/devise.html.haml index 43bd07679ba..4f8db74382f 100644 --- a/app/views/layouts/devise.html.haml +++ b/app/views/layouts/devise.html.haml @@ -9,7 +9,7 @@ .container.navless-container .content = render "layouts/flash" - .row + .row.append-bottom-15 .col-sm-7.brand-holder %h1 = brand_title -- cgit v1.2.1 From 0fc2a55773fcc76054241e2ca11b7a4751ada3c1 Mon Sep 17 00:00:00 2001 From: Paul Gear Date: Tue, 11 Dec 2018 22:49:28 +0700 Subject: 54376 Add changelog --- changelogs/unreleased/54736-sign-in-bottom-margin.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/54736-sign-in-bottom-margin.yml diff --git a/changelogs/unreleased/54736-sign-in-bottom-margin.yml b/changelogs/unreleased/54736-sign-in-bottom-margin.yml new file mode 100644 index 00000000000..32b5b44fe35 --- /dev/null +++ b/changelogs/unreleased/54736-sign-in-bottom-margin.yml @@ -0,0 +1,5 @@ +--- +title: Fix login box bottom margins on signin page +merge_request: 23739 +author: '@gear54' +type: fixed -- cgit v1.2.1 From 4ae1591b9768f8f727048ff83f675fe99a10eb90 Mon Sep 17 00:00:00 2001 From: Sean Nichols Date: Wed, 12 Dec 2018 23:07:49 -0500 Subject: Move empty file message to diff_content component --- app/assets/javascripts/diffs/components/diff_content.vue | 3 ++- app/assets/javascripts/diffs/components/diff_file.vue | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index 11cc4c09fed..e9e9c8a5ec9 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -69,7 +69,8 @@ export default {