From cf852ecf870cabdb98527fee1c4d16767eee07ba Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Thu, 25 Aug 2016 13:58:59 +0100 Subject: [PATCH 1/7] adds migration and respective validation and update of limit of memory per project --- app/controllers/admin/application_settings_controller.rb | 1 + app/models/application_setting.rb | 4 ++++ app/views/admin/application_settings/_form.html.haml | 4 ++++ db/schema.rb | 1 + 4 files changed, 10 insertions(+) diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 6ef7cf0bae66..be483459f70a 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -118,6 +118,7 @@ def application_setting_params :container_registry_token_expire_delay, :repository_storage, :enabled_git_access_protocol, + :project_size_limit, restricted_visibility_levels: [], import_sources: [], disabled_oauth_sign_in_sources: [] diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 246477ffe88e..36d8bfa36ef3 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -63,6 +63,10 @@ class ApplicationSetting < ActiveRecord::Base presence: true, numericality: { only_integer: true, greater_than: 0 } + validates :project_size_limit, + presence: true, + numericality: { only_integer: true, greater_than: 0 } + validates :container_registry_token_expire_delay, presence: true, numericality: { only_integer: true, greater_than: 0 } diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index d929364fc965..7410e6d883e2 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -88,6 +88,10 @@ = f.label :max_attachment_size, 'Maximum attachment size (MB)', class: 'control-label col-sm-2' .col-sm-10 = f.number_field :max_attachment_size, class: 'form-control' + .form-group + = f.label :project_size_limit, 'Project size limit (MB)', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :project_size_limit, class: 'form-control' .form-group = f.label :session_expire_delay, 'Session duration (minutes)', class: 'control-label col-sm-2' .col-sm-10 diff --git a/db/schema.rb b/db/schema.rb index 963d528d170b..101be92b4f0d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -92,6 +92,7 @@ t.text "domain_blacklist" t.boolean "koding_enabled" t.string "koding_url" + t.integer "project_size_limit", default: 0 end create_table "audit_events", force: :cascade do |t| -- GitLab From f12e9cb0d6384ef742ed4c712909df3e31c2c892 Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Sun, 28 Aug 2016 10:41:31 +0100 Subject: [PATCH 2/7] adds migration and controller updating for repository_size_limit --- .../admin/application_settings_controller.rb | 2 +- app/models/application_setting.rb | 4 ++-- app/models/project.rb | 9 +++++++++ app/views/admin/application_settings/_form.html.haml | 5 +++-- ..._repository_size_limit_to_application_settings.rb | 12 ++++++++++++ db/schema.rb | 2 +- 6 files changed, 28 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20160829104026_add_repository_size_limit_to_application_settings.rb diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index be483459f70a..42e5a2887102 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -118,7 +118,7 @@ def application_setting_params :container_registry_token_expire_delay, :repository_storage, :enabled_git_access_protocol, - :project_size_limit, + :repository_size_limit, restricted_visibility_levels: [], import_sources: [], disabled_oauth_sign_in_sources: [] diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 36d8bfa36ef3..73c4d6393356 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -63,9 +63,9 @@ class ApplicationSetting < ActiveRecord::Base presence: true, numericality: { only_integer: true, greater_than: 0 } - validates :project_size_limit, + validates :repository_size_limit, presence: true, - numericality: { only_integer: true, greater_than: 0 } + numericality: { only_integer: true, greater_than_or_equal_to: 0 } validates :container_registry_token_expire_delay, presence: true, diff --git a/app/models/project.rb b/app/models/project.rb index e5027af4a0e3..41122caf4ae0 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1286,6 +1286,15 @@ def append_or_update_attribute(name, value) end end + def above_size_limit? + return false if current_application_settings.repository_size_limit == 0 + repository_size >= current_application_settings.repository_size_limit + end + + def size_to_remove + return repository_size - current_application_settings.repository_size_limit + end + private def default_branch_protected? diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 7410e6d883e2..e8dde039c1be 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -89,9 +89,10 @@ .col-sm-10 = f.number_field :max_attachment_size, class: 'form-control' .form-group - = f.label :project_size_limit, 'Project size limit (MB)', class: 'control-label col-sm-2' + = f.label :repository_size_limit, 'Repository size limit (MB)', class: 'control-label col-sm-2' .col-sm-10 - = f.number_field :project_size_limit, class: 'form-control' + = f.number_field :repository_size_limit, class: 'form-control' + %span.help-block#repository_size_limit_help_block This limit is per repository. This includes LFS storage. 0 for unlimited .form-group = f.label :session_expire_delay, 'Session duration (minutes)', class: 'control-label col-sm-2' .col-sm-10 diff --git a/db/migrate/20160829104026_add_repository_size_limit_to_application_settings.rb b/db/migrate/20160829104026_add_repository_size_limit_to_application_settings.rb new file mode 100644 index 000000000000..f934281cc9bf --- /dev/null +++ b/db/migrate/20160829104026_add_repository_size_limit_to_application_settings.rb @@ -0,0 +1,12 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddRepositorySizeLimitToApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :application_settings, :repository_size_limit, :integer, default: 0 + end +end diff --git a/db/schema.rb b/db/schema.rb index 101be92b4f0d..e407ee6c2fea 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -92,7 +92,7 @@ t.text "domain_blacklist" t.boolean "koding_enabled" t.string "koding_url" - t.integer "project_size_limit", default: 0 + t.integer "repository_size_limit", default: 0 end create_table "audit_events", force: :cascade do |t| -- GitLab From 3a314b9d6f4356a3e0711d4683dbdefd05ec5647 Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Mon, 29 Aug 2016 14:45:36 +0100 Subject: [PATCH 3/7] add message when user has reached size limit over http and ssh push --- app/controllers/projects/git_http_controller.rb | 7 ++++++- lib/gitlab/git_access.rb | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb index b4373ef89efa..df212475ab02 100644 --- a/app/controllers/projects/git_http_controller.rb +++ b/app/controllers/projects/git_http_controller.rb @@ -65,7 +65,12 @@ def render_http_not_allowed def render_denied if user && user.can?(:read_project, project) - render plain: 'Access denied', status: :forbidden + message = if project.above_size_limit? + access_check.message + else + 'Access denied' + end + render plain: message, status: :forbidden else # Do not leak information about project existence render_not_found diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 1882eb8d0508..435b93056d9a 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -29,6 +29,10 @@ def check(cmd, changes) return build_status_object(false, 'The project you were looking for could not be found.') end + if project.above_size_limit? + return build_status_object(false, render_above_size_limit_message) + end + case cmd when *DOWNLOAD_COMMANDS download_access_check @@ -100,6 +104,18 @@ def protocol_allowed? private + def render_above_size_limit_message + repository_size_limit = Gitlab::CurrentSettings.current_application_settings.repository_size_limit + + [ + "GitLab: ---", + "GitLab: This repository's size (#{project.repository_size}MB) exceeds the limit of #{repository_size_limit}MB by", + "GitLab: #{project.size_to_remove}MB and as a result you are unable to push to it.", + "GitLab: Please contact your Gitlab administrator for more information.", + "GitLab: ---" + ].join("\n") + "\n" + end + def matching_merge_request?(newrev, branch_name) Checks::MatchingMergeRequest.new(newrev, branch_name, project).match? end -- GitLab From 4091c47f7278ccdfb2242ec9480d9e5d31294b1e Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Tue, 30 Aug 2016 11:12:43 +0100 Subject: [PATCH 4/7] user cannot create/accept merge requests when size limit has been exceeded --- .../projects/merge_requests_controller.rb | 6 ++++++ app/services/merge_requests/base_service.rb | 6 ++++++ app/services/merge_requests/build_service.rb | 20 ++++++++++++++++--- .../projects/merge_requests/merge.js.haml | 3 +++ .../widget/open/_size_limit_reached.html.haml | 6 ++++++ 5 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 app/views/projects/merge_requests/widget/open/_size_limit_reached.html.haml diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 4f5f3b6aa09d..d267661cff4d 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -295,6 +295,12 @@ def cancel_merge_when_build_succeeds def merge return access_denied! unless @merge_request.can_be_merged_by?(current_user) + # user is not able to merge if project is above size limit + if @merge_request.target_project.above_size_limit? + @status = :size_limit_reached + return + end + # Disable the CI check if merge_when_build_succeeds is enabled since we have # to wait until CI completes to know unless @merge_request.mergeable?(skip_ci_check: merge_when_build_succeeds_active?) diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index ba424b09463a..55bee838ea73 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -35,6 +35,12 @@ def execute_hooks(merge_request, action = 'open', oldrev = nil) end end + def render_size_limit_message(project_size, project_size_remaining) + repository_size_limit = Gitlab::CurrentSettings.current_application_settings.repository_size_limit + + "The target's repository size (#{project_size}MB) exceeds the limit of #{repository_size_limit}MB by #{project_size_remaining}MB" + end + private def filter_params diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 290742f1506d..43e7cd505eb0 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -13,15 +13,24 @@ def execute merge_request.target_project ||= (project.forked_from_project || project) merge_request.target_branch ||= merge_request.target_project.default_branch + if merge_request.target_project.above_size_limit? + message = render_size_limit_message(merge_request.target_project.repository_size, + merge_request.target_project.size_to_remove) + + merge_request.errors.add(:base, message) + end + if merge_request.target_branch.blank? || merge_request.source_branch.blank? message = if params[:source_branch] || params[:target_branch] "You must select source and target branch" end - return build_failed(merge_request, message) + merge_request.errors.add(:base, message) unless message.nil? end + return build_failed(merge_request) if invalid?(merge_request) + compare = CompareService.new.execute( merge_request.source_project, merge_request.source_branch, @@ -92,8 +101,13 @@ def set_title_and_description(merge_request) merge_request end - def build_failed(merge_request, message) - merge_request.errors.add(:base, message) unless message.nil? + def invalid?(merge_request) + merge_request.target_project.above_size_limit? || + merge_request.target_project.blank? || + merge_request.source_branch.blank? + end + + def build_failed(merge_request) merge_request.compare_commits = [] merge_request.can_be_created = false merge_request diff --git a/app/views/projects/merge_requests/merge.js.haml b/app/views/projects/merge_requests/merge.js.haml index 84b6c9ebc5cf..92a2a9c548d7 100644 --- a/app/views/projects/merge_requests/merge.js.haml +++ b/app/views/projects/merge_requests/merge.js.haml @@ -8,6 +8,9 @@ - when :sha_mismatch :plain $('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/sha_mismatch'))}"); +- when :size_limit_reached + :plain + $('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/size_limit_reached'))}"); - else :plain $('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/reload'))}"); diff --git a/app/views/projects/merge_requests/widget/open/_size_limit_reached.html.haml b/app/views/projects/merge_requests/widget/open/_size_limit_reached.html.haml new file mode 100644 index 000000000000..6aa2a9393dec --- /dev/null +++ b/app/views/projects/merge_requests/widget/open/_size_limit_reached.html.haml @@ -0,0 +1,6 @@ +%h4.size-limit-reached + = icon("exclamation-triangle") + This repository has reached it's size limit + +%p + Please contact your GitLab administrator for more information -- GitLab From 446a7ce0b55f28547ea9243d3d072ecc776b6895 Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Tue, 30 Aug 2016 14:40:07 +0100 Subject: [PATCH 5/7] blocks file creation/updating while repository's size is above the limit and adds some refactors --- app/models/merge_request.rb | 4 ++++ app/services/files/base_service.rb | 8 ++++++++ app/services/files/create_dir_service.rb | 4 ++++ app/services/files/create_service.rb | 4 ++++ app/services/files/update_service.rb | 4 ++++ app/services/merge_requests/build_service.rb | 8 +------- .../widget/open/_size_limit_reached.html.haml | 2 +- lib/gitlab/git_access.rb | 6 ++---- 8 files changed, 28 insertions(+), 12 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 62163e740007..5ff85ad7a1dc 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -284,6 +284,10 @@ def diff_sha_refs end end + def valid? + !(target_project.above_size_limit? || target_project.blank? || source_branch.blank?) + end + def branch_merge_base_sha branch_merge_base_commit.try(:sha) end diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index ea94818713be..ed3360fee44c 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -34,6 +34,14 @@ def execute error(ex.message) end + def repository_size_limit + Gitlab::CurrentSettings.current_application_settings.repository_size_limit + end + + def size_limit_error_message + "Your changes could not be committed, because the repository's size (#{project.repository_size}MB) has exceeded its size limit of #{repository_size_limit}MB by #{project.size_to_remove}MB" + end + private def different_branch? diff --git a/app/services/files/create_dir_service.rb b/app/services/files/create_dir_service.rb index 6107254a34ee..e22aa2338246 100644 --- a/app/services/files/create_dir_service.rb +++ b/app/services/files/create_dir_service.rb @@ -9,6 +9,10 @@ def commit def validate super + if project.above_size_limit? + raise_error(size_limit_error_message) + end + unless @file_path =~ Gitlab::Regex.file_path_regex raise_error( 'Your changes could not be committed, because the file path ' + diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 8eaf6db8012e..16ac511763ef 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -9,6 +9,10 @@ def commit def validate super + if project.above_size_limit? + raise_error(size_limit_error_message) + end + if @file_path =~ Gitlab::Regex.directory_traversal_regex raise_error( 'Your changes could not be committed, because the file name ' + diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index 4fc3b6407992..347fc6e09133 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -16,6 +16,10 @@ def commit def validate super + if project.above_size_limit? + raise_error(size_limit_error_message) + end + if file_has_changed? raise FileChangedError.new("You are attempting to update a file that has changed since you started editing it.") end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 43e7cd505eb0..3666e23f9fa2 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -29,7 +29,7 @@ def execute merge_request.errors.add(:base, message) unless message.nil? end - return build_failed(merge_request) if invalid?(merge_request) + return build_failed(merge_request) unless merge_request.valid? compare = CompareService.new.execute( merge_request.source_project, @@ -101,12 +101,6 @@ def set_title_and_description(merge_request) merge_request end - def invalid?(merge_request) - merge_request.target_project.above_size_limit? || - merge_request.target_project.blank? || - merge_request.source_branch.blank? - end - def build_failed(merge_request) merge_request.compare_commits = [] merge_request.can_be_created = false diff --git a/app/views/projects/merge_requests/widget/open/_size_limit_reached.html.haml b/app/views/projects/merge_requests/widget/open/_size_limit_reached.html.haml index 6aa2a9393dec..9b4bffc6ebf8 100644 --- a/app/views/projects/merge_requests/widget/open/_size_limit_reached.html.haml +++ b/app/views/projects/merge_requests/widget/open/_size_limit_reached.html.haml @@ -1,6 +1,6 @@ %h4.size-limit-reached = icon("exclamation-triangle") - This repository has reached it's size limit + This repository has reached its size limit %p Please contact your GitLab administrator for more information diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 435b93056d9a..040d350b5b5a 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -29,10 +29,6 @@ def check(cmd, changes) return build_status_object(false, 'The project you were looking for could not be found.') end - if project.above_size_limit? - return build_status_object(false, render_above_size_limit_message) - end - case cmd when *DOWNLOAD_COMMANDS download_access_check @@ -80,6 +76,8 @@ def user_push_access_check(changes) return build_status_object(false, "A repository for this project does not exist yet.") end + return build_status_object(false, render_above_size_limit_message) if project.above_size_limit? + changes_list = Gitlab::ChangesList.new(changes) # Iterate over all changes to find if user allowed all of them to be applied -- GitLab From babd0d8d5bbf6549daf45c15ce731084180b044b Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Wed, 31 Aug 2016 16:53:20 +0100 Subject: [PATCH 6/7] fix broken specs --- app/models/merge_request.rb | 2 +- app/models/project.rb | 2 +- app/services/merge_requests/build_service.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5ff85ad7a1dc..faa0968fc741 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -284,7 +284,7 @@ def diff_sha_refs end end - def valid? + def is_valid? !(target_project.above_size_limit? || target_project.blank? || source_branch.blank?) end diff --git a/app/models/project.rb b/app/models/project.rb index 41122caf4ae0..05d4132b1008 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1292,7 +1292,7 @@ def above_size_limit? end def size_to_remove - return repository_size - current_application_settings.repository_size_limit + repository_size - current_application_settings.repository_size_limit end private diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 3666e23f9fa2..67327e297a63 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -29,7 +29,7 @@ def execute merge_request.errors.add(:base, message) unless message.nil? end - return build_failed(merge_request) unless merge_request.valid? + return build_failed(merge_request) unless merge_request.is_valid? compare = CompareService.new.execute( merge_request.source_project, -- GitLab From 2ec77f3c817a6678f5f63f061021570725ca1fe7 Mon Sep 17 00:00:00 2001 From: tiagonbotelho Date: Wed, 31 Aug 2016 20:39:04 +0100 Subject: [PATCH 7/7] adds specs for related additions --- lib/gitlab/git_access.rb | 4 ++-- .../projects/merge_requests_controller_spec.rb | 11 +++++++++++ spec/requests/git_http_spec.rb | 16 ++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 040d350b5b5a..58ab8290aac7 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -51,6 +51,8 @@ def download_access_check def push_access_check(changes) if user + return build_status_object(false, render_above_size_limit_message) if project.above_size_limit? + user_push_access_check(changes) elsif deploy_key build_status_object(false, "Deploy keys are not allowed to push code.") @@ -76,8 +78,6 @@ def user_push_access_check(changes) return build_status_object(false, "A repository for this project does not exist yet.") end - return build_status_object(false, render_above_size_limit_message) if project.above_size_limit? - changes_list = Gitlab::ChangesList.new(changes) # Iterate over all changes to find if user allowed all of them to be applied diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index a219400d75fc..971dc0d90c5a 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -224,6 +224,17 @@ def get_merge_requests end end + context 'when the repository is above size limit' do + before do + allow_any_instance_of(Project).to receive(:above_size_limit?).and_return(true) + post :merge, base_params.merge(sha: merge_request.diff_head_sha) + end + + it 'returns :size_limit_reached' do + expect(assigns(:status)).to eq(:size_limit_reached) + end + end + context 'when the merge request is not mergeable' do before do merge_request.update_attributes(title: "WIP: #{merge_request.title}") diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index afaf4b7cefbe..08ac852d4e91 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -122,6 +122,22 @@ end end + context "when repository is above size limit" do + let(:env) { { user: user.username, password: user.password } } + + before do + project.team << [user, :master] + end + + it 'responds with status 403' do + allow_any_instance_of(Project).to receive(:above_size_limit?).and_return(true) + + upload(path, env) do |response| + expect(response).to have_http_status(403) + end + end + end + context "when username and password are provided" do let(:env) { { user: user.username, password: 'nope' } } -- GitLab