summaryrefslogtreecommitdiff
path: root/spec
diff options
context:
space:
mode:
authorToon Claes <toon@iotcl.com>2017-09-19 09:44:58 +0200
committerToon Claes <toon@iotcl.com>2017-10-06 22:37:40 +0200
commitd13669716ab0c31ce9039ae9f7f073e33a4dc40f (patch)
tree001bb2e6aa76ea7531c93e469c396f7fdcc408a7 /spec
parent2cf5dca8f80cdefeb8932bf80417f52f289668c8 (diff)
downloadgitlab-ce-d13669716ab0c31ce9039ae9f7f073e33a4dc40f.tar.gz
Create idea of read-only databasetc-geo-read-only-idea
In GitLab EE, a GitLab instance can be read-only (e.g. when it's a Geo secondary node). But in GitLab CE it also might be useful to have the "read-only" idea around. So port it back to GitLab CE. Also having the principle of read-only in GitLab CE would hopefully lead to less errors introduced, doing write operations when there aren't allowed for read-only calls. Closes gitlab-org/gitlab-ce#37534.
Diffstat (limited to 'spec')
-rw-r--r--spec/factories/projects.rb2
-rw-r--r--spec/lib/banzai/renderer_spec.rb9
-rw-r--r--spec/lib/gitlab/git_access_spec.rb23
-rw-r--r--spec/lib/gitlab/git_access_wiki_spec.rb29
-rw-r--r--spec/lib/gitlab/middleware/read_only_spec.rb142
-rw-r--r--spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb8
-rw-r--r--spec/models/concerns/cache_markdown_field_spec.rb72
-rw-r--r--spec/models/concerns/routable_spec.rb10
-rw-r--r--spec/models/project_spec.rb42
-rw-r--r--spec/requests/lfs_http_spec.rb28
-rw-r--r--spec/services/projects/hashed_storage_migration_service_spec.rb2
-rw-r--r--spec/services/users/activity_service_spec.rb12
12 files changed, 329 insertions, 50 deletions
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index 958d62181a2..4034e7905ad 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -149,7 +149,7 @@ FactoryGirl.define do
end
end
- trait :readonly do
+ trait :read_only do
repository_read_only true
end
diff --git a/spec/lib/banzai/renderer_spec.rb b/spec/lib/banzai/renderer_spec.rb
index da42272bbef..81a04a2d46d 100644
--- a/spec/lib/banzai/renderer_spec.rb
+++ b/spec/lib/banzai/renderer_spec.rb
@@ -31,7 +31,14 @@ describe Banzai::Renderer do
let(:object) { fake_object(fresh: false) }
it 'caches and returns the result' do
- expect(object).to receive(:refresh_markdown_cache!).with(do_update: true)
+ expect(object).to receive(:refresh_markdown_cache!)
+
+ is_expected.to eq('field_html')
+ end
+
+ it "skips database caching on a GitLab read-only instance" do
+ allow(Gitlab::Database).to receive(:read_only?).and_return(true)
+ expect(object).to receive(:refresh_markdown_cache!)
is_expected.to eq('field_html')
end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 458627ee4de..c54327bd2e4 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -598,6 +598,19 @@ describe Gitlab::GitAccess do
admin: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false }))
end
end
+
+ context "when in a read-only GitLab instance" do
+ before do
+ create(:protected_branch, name: 'feature', project: project)
+ allow(Gitlab::Database).to receive(:read_only?) { true }
+ end
+
+ # Only check admin; if an admin can't do it, other roles can't either
+ matrix = permissions_matrix[:admin].dup
+ matrix.each { |key, _| matrix[key] = false }
+
+ run_permission_checks(admin: matrix)
+ end
end
describe 'build authentication abilities' do
@@ -632,6 +645,16 @@ describe Gitlab::GitAccess do
end
end
+ context 'when the repository is read only' do
+ let(:project) { create(:project, :repository, :read_only) }
+
+ it 'denies push access' do
+ project.add_master(user)
+
+ expect { push_access_check }.to raise_unauthorized('The repository is temporarily read-only. Please try again later.')
+ end
+ end
+
describe 'deploy key permissions' do
let(:key) { create(:deploy_key, user: user, can_push: can_push) }
let(:actor) { key }
diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb
index 0376b4ee783..1056074264a 100644
--- a/spec/lib/gitlab/git_access_wiki_spec.rb
+++ b/spec/lib/gitlab/git_access_wiki_spec.rb
@@ -4,6 +4,7 @@ describe Gitlab::GitAccessWiki do
let(:access) { described_class.new(user, project, 'web', authentication_abilities: authentication_abilities, redirected_path: redirected_path) }
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
+ let(:changes) { ['6f6d7e7ed 570e7b2ab refs/heads/master'] }
let(:redirected_path) { nil }
let(:authentication_abilities) do
[
@@ -13,19 +14,27 @@ describe Gitlab::GitAccessWiki do
]
end
- describe 'push_allowed?' do
- before do
- create(:protected_branch, name: 'master', project: project)
- project.team << [user, :developer]
- end
+ describe '#push_access_check' do
+ context 'when user can :create_wiki' do
+ before do
+ create(:protected_branch, name: 'master', project: project)
+ project.team << [user, :developer]
+ end
- subject { access.check('git-receive-pack', changes) }
+ subject { access.check('git-receive-pack', changes) }
- it { expect { subject }.not_to raise_error }
- end
+ it { expect { subject }.not_to raise_error }
+
+ context 'when in a read-only GitLab instance' do
+ before do
+ allow(Gitlab::Database).to receive(:read_only?) { true }
+ end
- def changes
- ['6f6d7e7ed 570e7b2ab refs/heads/master']
+ it 'does not give access to upload wiki code' do
+ expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError, "You can't push code to a read-only GitLab instance.")
+ end
+ end
+ end
end
describe '#access_check_download!' do
diff --git a/spec/lib/gitlab/middleware/read_only_spec.rb b/spec/lib/gitlab/middleware/read_only_spec.rb
new file mode 100644
index 00000000000..742a792a1af
--- /dev/null
+++ b/spec/lib/gitlab/middleware/read_only_spec.rb
@@ -0,0 +1,142 @@
+require 'spec_helper'
+
+describe Gitlab::Middleware::ReadOnly do
+ include Rack::Test::Methods
+
+ RSpec::Matchers.define :be_a_redirect do
+ match do |response|
+ response.status == 301
+ end
+ end
+
+ RSpec::Matchers.define :disallow_request do
+ match do |middleware|
+ flash = middleware.send(:rack_flash)
+ flash['alert'] && flash['alert'].include?('You cannot do writing operations')
+ end
+ end
+
+ RSpec::Matchers.define :disallow_request_in_json do
+ match do |response|
+ json_response = JSON.parse(response.body)
+ response.body.include?('You cannot do writing operations') && json_response.key?('message')
+ end
+ end
+
+ let(:rack_stack) do
+ rack = Rack::Builder.new do
+ use ActionDispatch::Session::CacheStore
+ use ActionDispatch::Flash
+ use ActionDispatch::ParamsParser
+ end
+
+ rack.run(subject)
+ rack.to_app
+ end
+
+ subject { described_class.new(fake_app) }
+
+ let(:request) { Rack::MockRequest.new(rack_stack) }
+
+ context 'normal requests to a read-only Gitlab instance' do
+ let(:fake_app) { lambda { |env| [200, { 'Content-Type' => 'text/plain' }, ['OK']] } }
+
+ before do
+ allow(Gitlab::Database).to receive(:read_only?) { true }
+ end
+
+ it 'expects PATCH requests to be disallowed' do
+ response = request.patch('/test_request')
+
+ expect(response).to be_a_redirect
+ expect(subject).to disallow_request
+ end
+
+ it 'expects PUT requests to be disallowed' do
+ response = request.put('/test_request')
+
+ expect(response).to be_a_redirect
+ expect(subject).to disallow_request
+ end
+
+ it 'expects POST requests to be disallowed' do
+ response = request.post('/test_request')
+
+ expect(response).to be_a_redirect
+ expect(subject).to disallow_request
+ end
+
+ it 'expects a internal POST request to be allowed after a disallowed request' do
+ response = request.post('/test_request')
+
+ expect(response).to be_a_redirect
+
+ response = request.post("/api/#{API::API.version}/internal")
+
+ expect(response).not_to be_a_redirect
+ end
+
+ it 'expects DELETE requests to be disallowed' do
+ response = request.delete('/test_request')
+
+ expect(response).to be_a_redirect
+ expect(subject).to disallow_request
+ end
+
+ context 'whitelisted requests' do
+ it 'expects DELETE request to logout to be allowed' do
+ response = request.delete('/users/sign_out')
+
+ expect(response).not_to be_a_redirect
+ expect(subject).not_to disallow_request
+ end
+
+ it 'expects a POST internal request to be allowed' do
+ response = request.post("/api/#{API::API.version}/internal")
+
+ expect(response).not_to be_a_redirect
+ expect(subject).not_to disallow_request
+ end
+
+ it 'expects a POST LFS request to batch URL to be allowed' do
+ response = request.post('/root/rouge.git/info/lfs/objects/batch')
+
+ expect(response).not_to be_a_redirect
+ expect(subject).not_to disallow_request
+ end
+ end
+ end
+
+ context 'json requests to a read-only GitLab instance' do
+ let(:fake_app) { lambda { |env| [200, { 'Content-Type' => 'application/json' }, ['OK']] } }
+ let(:content_json) { { 'CONTENT_TYPE' => 'application/json' } }
+
+ before do
+ allow(Gitlab::Database).to receive(:read_only?) { true }
+ end
+
+ it 'expects PATCH requests to be disallowed' do
+ response = request.patch('/test_request', content_json)
+
+ expect(response).to disallow_request_in_json
+ end
+
+ it 'expects PUT requests to be disallowed' do
+ response = request.put('/test_request', content_json)
+
+ expect(response).to disallow_request_in_json
+ end
+
+ it 'expects POST requests to be disallowed' do
+ response = request.post('/test_request', content_json)
+
+ expect(response).to disallow_request_in_json
+ end
+
+ it 'expects DELETE requests to be disallowed' do
+ response = request.delete('/test_request', content_json)
+
+ expect(response).to disallow_request_in_json
+ end
+ end
+end
diff --git a/spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb b/spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb
index a0fb86345f3..b4b83b70d1c 100644
--- a/spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb
+++ b/spec/lib/system_check/app/git_user_default_ssh_config_check_spec.rb
@@ -39,6 +39,14 @@ describe SystemCheck::App::GitUserDefaultSSHConfigCheck do
it { is_expected.to eq(expected_result) }
end
+
+ it 'skips GitLab read-only instances' do
+ stub_user
+ stub_home_dir
+ allow(Gitlab::Database).to receive(:read_only?).and_return(true)
+
+ is_expected.to be_truthy
+ end
end
describe '#check?' do
diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index 40bbb10eaac..129dfa07f15 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -178,57 +178,59 @@ describe CacheMarkdownField do
end
end
- describe '#refresh_markdown_cache!' do
+ describe '#refresh_markdown_cache' do
before do
thing.foo = updated_markdown
end
- context 'do_update: false' do
- it 'fills all html fields' do
- thing.refresh_markdown_cache!
+ it 'fills all html fields' do
+ thing.refresh_markdown_cache
- expect(thing.foo_html).to eq(updated_html)
- expect(thing.foo_html_changed?).to be_truthy
- expect(thing.baz_html_changed?).to be_truthy
- end
+ expect(thing.foo_html).to eq(updated_html)
+ expect(thing.foo_html_changed?).to be_truthy
+ expect(thing.baz_html_changed?).to be_truthy
+ end
- it 'does not save the result' do
- expect(thing).not_to receive(:update_columns)
+ it 'does not save the result' do
+ expect(thing).not_to receive(:update_columns)
- thing.refresh_markdown_cache!
- end
+ thing.refresh_markdown_cache
+ end
- it 'updates the markdown cache version' do
- thing.cached_markdown_version = nil
- thing.refresh_markdown_cache!
+ it 'updates the markdown cache version' do
+ thing.cached_markdown_version = nil
+ thing.refresh_markdown_cache
- expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION)
- end
+ expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION)
end
+ end
- context 'do_update: true' do
- it 'fills all html fields' do
- thing.refresh_markdown_cache!(do_update: true)
+ describe '#refresh_markdown_cache!' do
+ before do
+ thing.foo = updated_markdown
+ end
- expect(thing.foo_html).to eq(updated_html)
- expect(thing.foo_html_changed?).to be_truthy
- expect(thing.baz_html_changed?).to be_truthy
- end
+ it 'fills all html fields' do
+ thing.refresh_markdown_cache!
- it 'skips saving if not persisted' do
- expect(thing).to receive(:persisted?).and_return(false)
- expect(thing).not_to receive(:update_columns)
+ expect(thing.foo_html).to eq(updated_html)
+ expect(thing.foo_html_changed?).to be_truthy
+ expect(thing.baz_html_changed?).to be_truthy
+ end
- thing.refresh_markdown_cache!(do_update: true)
- end
+ it 'skips saving if not persisted' do
+ expect(thing).to receive(:persisted?).and_return(false)
+ expect(thing).not_to receive(:update_columns)
- it 'saves the changes using #update_columns' do
- expect(thing).to receive(:persisted?).and_return(true)
- expect(thing).to receive(:update_columns)
- .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => CacheMarkdownField::CACHE_VERSION)
+ thing.refresh_markdown_cache!
+ end
- thing.refresh_markdown_cache!(do_update: true)
- end
+ it 'saves the changes using #update_columns' do
+ expect(thing).to receive(:persisted?).and_return(true)
+ expect(thing).to receive(:update_columns)
+ .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => CacheMarkdownField::CACHE_VERSION)
+
+ thing.refresh_markdown_cache!
end
end
diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb
index b463d12e448..ab8773b7ede 100644
--- a/spec/models/concerns/routable_spec.rb
+++ b/spec/models/concerns/routable_spec.rb
@@ -12,6 +12,16 @@ describe Group, 'Routable' do
it { is_expected.to have_many(:redirect_routes).dependent(:destroy) }
end
+ describe 'GitLab read-only instance' do
+ it 'does not save route if route is not present' do
+ group.route.path = ''
+ allow(Gitlab::Database).to receive(:read_only?).and_return(true)
+ expect(group).to receive(:update_route).and_call_original
+
+ expect { group.full_path }.to change { Route.count }.by(0)
+ end
+ end
+
describe 'Callbacks' do
it 'creates route record on create' do
expect(group.route.path).to eq(group.path)
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 9b470e79a76..ec700215521 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -692,6 +692,44 @@ describe Project do
project.cache_has_external_issue_tracker
end.to change { project.has_external_issue_tracker}.to(false)
end
+
+ it 'does not cache data when in a read-only GitLab instance' do
+ allow(Gitlab::Database).to receive(:read_only?) { true }
+
+ expect do
+ project.cache_has_external_issue_tracker
+ end.not_to change { project.has_external_issue_tracker }
+ end
+ end
+
+ describe '#cache_has_external_wiki' do
+ let(:project) { create(:project, has_external_wiki: nil) }
+
+ it 'stores true if there is any external_wikis' do
+ services = double(:service, external_wikis: [ExternalWikiService.new])
+ expect(project).to receive(:services).and_return(services)
+
+ expect do
+ project.cache_has_external_wiki
+ end.to change { project.has_external_wiki}.to(true)
+ end
+
+ it 'stores false if there is no external_wikis' do
+ services = double(:service, external_wikis: [])
+ expect(project).to receive(:services).and_return(services)
+
+ expect do
+ project.cache_has_external_wiki
+ end.to change { project.has_external_wiki}.to(false)
+ end
+
+ it 'does not cache data when in a read-only GitLab instance' do
+ allow(Gitlab::Database).to receive(:read_only?) { true }
+
+ expect do
+ project.cache_has_external_wiki
+ end.not_to change { project.has_external_wiki }
+ end
end
describe '#has_wiki?' do
@@ -2446,7 +2484,7 @@ describe Project do
expect(project.migrate_to_hashed_storage!).to be_truthy
end
- it 'flags as readonly' do
+ it 'flags as read-only' do
expect { project.migrate_to_hashed_storage! }.to change { project.repository_read_only }.to(true)
end
@@ -2573,7 +2611,7 @@ describe Project do
expect(project.migrate_to_hashed_storage!).to be_nil
end
- it 'does not flag as readonly' do
+ it 'does not flag as read-only' do
expect { project.migrate_to_hashed_storage! }.not_to change { project.repository_read_only }
end
end
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 27d09b8202e..eb4017e116b 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -824,6 +824,34 @@ describe 'Git LFS API and storage' do
end
end
+ describe 'when handling lfs batch request on a read-only GitLab instance' do
+ let(:authorization) { authorize_user }
+ let(:project) { create(:project) }
+ let(:path) { "#{project.http_url_to_repo}/info/lfs/objects/batch" }
+ let(:body) do
+ { 'objects' => [{ 'oid' => sample_oid, 'size' => sample_size }] }
+ end
+
+ before do
+ allow(Gitlab::Database).to receive(:read_only?) { true }
+ project.team << [user, :master]
+ enable_lfs
+ end
+
+ it 'responds with a 200 message on download' do
+ post_lfs_json path, body.merge('operation' => 'download'), headers
+
+ expect(response).to have_gitlab_http_status(200)
+ end
+
+ it 'responds with a 403 message on upload' do
+ post_lfs_json path, body.merge('operation' => 'upload'), headers
+
+ expect(response).to have_gitlab_http_status(403)
+ expect(json_response).to include('message' => 'You cannot write to this read-only GitLab instance.')
+ end
+ end
+
describe 'when pushing a lfs object' do
before do
enable_lfs
diff --git a/spec/services/projects/hashed_storage_migration_service_spec.rb b/spec/services/projects/hashed_storage_migration_service_spec.rb
index 1b61207b550..aa1988d29d6 100644
--- a/spec/services/projects/hashed_storage_migration_service_spec.rb
+++ b/spec/services/projects/hashed_storage_migration_service_spec.rb
@@ -20,7 +20,7 @@ describe Projects::HashedStorageMigrationService do
expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_truthy
end
- it 'updates project to be hashed and not readonly' do
+ it 'updates project to be hashed and not read-only' do
service.execute
expect(project.hashed_storage?).to be_truthy
diff --git a/spec/services/users/activity_service_spec.rb b/spec/services/users/activity_service_spec.rb
index fef4da0c76e..17eabad73be 100644
--- a/spec/services/users/activity_service_spec.rb
+++ b/spec/services/users/activity_service_spec.rb
@@ -38,6 +38,18 @@ describe Users::ActivityService do
end
end
end
+
+ context 'when in GitLab read-only instance' do
+ before do
+ allow(Gitlab::Database).to receive(:read_only?).and_return(true)
+ end
+
+ it 'does not update last_activity_at' do
+ service.execute
+
+ expect(last_hour_user_ids).to eq([])
+ end
+ end
end
def last_hour_user_ids