diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-17 21:09:16 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-17 21:09:16 +0000 |
commit | 154b9bae142ba15fec753f44327654595094b879 (patch) | |
tree | 027f8ae024961778d5b00c77a72fe302f985d4f3 /app | |
parent | 2c156e3c7bbade01c36eee18327f1ced6eebea79 (diff) | |
download | gitlab-ce-154b9bae142ba15fec753f44327654595094b879.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'app')
19 files changed, 191 insertions, 102 deletions
diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue index 637f0237b63..0ccc58ec2da 100644 --- a/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue +++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue @@ -3,6 +3,7 @@ import { __ } from '~/locale'; import { mapActions, mapState } from 'vuex'; import { ADD_CI_VARIABLE_MODAL_ID } from '../constants'; import { + GlButton, GlModal, GlFormSelect, GlFormGroup, @@ -16,6 +17,7 @@ import { export default { modalId: ADD_CI_VARIABLE_MODAL_ID, components: { + GlButton, GlModal, GlFormSelect, GlFormGroup, @@ -66,20 +68,6 @@ export default { attributes: { variant: 'success', disabled: !this.canSubmit }, }; }, - deleteAction() { - if (this.variableBeingEdited) { - return { - text: __('Delete variable'), - attributes: { variant: 'danger', category: 'secondary' }, - }; - } - return null; - }, - cancelAction() { - return { - text: __('Cancel'), - }; - }, maskedFeedback() { return __('This variable can not be masked'); }, @@ -99,6 +87,7 @@ export default { } else { this.addVariable(); } + this.hideModal(); }, resetModalHandler() { if (this.variableBeingEdited) { @@ -107,20 +96,23 @@ export default { this.clearModal(); } }, + hideModal() { + this.$refs.modal.hide(); + }, + deleteVarAndClose() { + this.deleteVariable(this.variableBeingEdited); + this.hideModal(); + }, }, }; </script> <template> <gl-modal + ref="modal" :modal-id="$options.modalId" :title="modalActionText" - :action-primary="primaryAction" - :action-secondary="deleteAction" - :action-cancel="cancelAction" - @ok="updateOrAddVariable" @hidden="resetModalHandler" - @secondary="deleteVariable(variableBeingEdited)" > <form> <gl-form-group :label="__('Key')" label-for="ci-variable-key"> @@ -210,5 +202,23 @@ export default { </gl-form-checkbox> </gl-form-group> </form> + <template #modal-footer> + <gl-button @click="hideModal">{{ __('Cancel') }}</gl-button> + <gl-button + v-if="variableBeingEdited" + ref="deleteCiVariable" + category="secondary" + variant="danger" + @click="deleteVarAndClose" + >{{ __('Delete variable') }}</gl-button + > + <gl-button + ref="updateOrAddVariable" + :disabled="!canSubmit" + variant="success" + @click="updateOrAddVariable" + >{{ modalActionText }} + </gl-button> + </template> </gl-modal> </template> diff --git a/app/assets/stylesheets/bootstrap_migration.scss b/app/assets/stylesheets/bootstrap_migration.scss index 885e9ac6667..f3293510e9a 100644 --- a/app/assets/stylesheets/bootstrap_migration.scss +++ b/app/assets/stylesheets/bootstrap_migration.scss @@ -120,8 +120,8 @@ kbd { code { padding: 2px 4px; - color: $red-600; - background-color: $red-100; + color: $code-color; + background-color: $gray-100; border-radius: $border-radius-default; .code > & { diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss index 0de5aae4b0e..d61a32d2d95 100644 --- a/app/assets/stylesheets/framework/typography.scss +++ b/app/assets/stylesheets/framework/typography.scss @@ -592,7 +592,7 @@ pre { word-wrap: break-word; color: $gl-text-color; background-color: $gray-light; - border: 1px solid $border-color; + border: 1px solid $gray-200; border-radius: $border-radius-small; } diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index d31d9245e9c..4d858f88921 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -356,6 +356,7 @@ $list-text-height: 42px; */ $code-font-size: 90%; $code-line-height: 1.6; +$code-color: $gray-950; /* * Tooltips diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb index ffa3f2c3364..3d347429398 100644 --- a/app/controllers/groups/settings/ci_cd_controller.rb +++ b/app/controllers/groups/settings/ci_cd_controller.rb @@ -7,7 +7,7 @@ module Groups before_action :authorize_admin_group! before_action :authorize_update_max_artifacts_size!, only: [:update] before_action do - push_frontend_feature_flag(:new_variables_ui, @group) + push_frontend_feature_flag(:new_variables_ui, @group, default_enabled: true) end before_action :define_variables, only: [:show, :create_deploy_token] diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 11b6e5eb66f..ee102248cb7 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -60,10 +60,10 @@ class Projects::PipelinesController < Projects::ApplicationController .new(project, current_user, create_params) .execute(:web, ignore_skip_ci: true, save_on_errors: false) - if @pipeline.persisted? + if @pipeline.created_successfully? redirect_to project_pipeline_path(project, @pipeline) else - render 'new' + render 'new', status: :bad_request end end diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index aac6ecb07e4..43c798bfc6e 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -6,7 +6,7 @@ module Projects before_action :authorize_admin_pipeline! before_action :define_variables before_action do - push_frontend_feature_flag(:new_variables_ui, @project) + push_frontend_feature_flag(:new_variables_ui, @project, default_enabled: true) end def show diff --git a/app/models/clusters/applications/prometheus.rb b/app/models/clusters/applications/prometheus.rb index adce55cb61b..8297f653ea7 100644 --- a/app/models/clusters/applications/prometheus.rb +++ b/app/models/clusters/applications/prometheus.rb @@ -17,6 +17,11 @@ module Clusters default_value_for :version, VERSION + attr_encrypted :alert_manager_token, + mode: :per_attribute_iv, + key: Settings.attr_encrypted_db_key_base_truncated, + algorithm: 'aes-256-gcm' + after_destroy do run_after_commit do disable_prometheus_integration @@ -103,8 +108,18 @@ module Clusters false end + def generate_alert_manager_token! + unless alert_manager_token.present? + update!(alert_manager_token: generate_token) + end + end + private + def generate_token + SecureRandom.hex + end + def disable_prometheus_integration ::Clusters::Applications::DeactivateServiceWorker .perform_async(cluster_id, ::PrometheusService.to_param) # rubocop:disable CodeReuse/ServiceClass diff --git a/app/models/concerns/versioned_description.rb b/app/models/concerns/versioned_description.rb index 63a24aadc8a..ee8d2d45357 100644 --- a/app/models/concerns/versioned_description.rb +++ b/app/models/concerns/versioned_description.rb @@ -16,7 +16,7 @@ module VersionedDescription def save_description_version self.saved_description_version = nil - return unless Feature.enabled?(:save_description_versions, issuing_parent) + return unless Feature.enabled?(:save_description_versions, issuing_parent, default_enabled: true) return unless saved_change_to_description? unless description_versions.exists? diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb index 32be89439ba..82ebf6ba6a5 100644 --- a/app/policies/note_policy.rb +++ b/app/policies/note_policy.rb @@ -12,6 +12,7 @@ class NotePolicy < BasePolicy condition(:editable, scope: :subject) { @subject.editable? } condition(:can_read_noteable) { can?(:"read_#{@subject.noteable_ability_name}") } + condition(:commit_is_deleted) { @subject.for_commit? && @subject.noteable.blank? } condition(:is_visible) { @subject.system_note_with_references_visible_for?(@user) } @@ -25,12 +26,16 @@ class NotePolicy < BasePolicy # If user can't read the issue/MR/etc then they should not be allowed to do anything to their own notes rule { ~can_read_noteable }.policy do - prevent :read_note prevent :admin_note prevent :resolve_note prevent :award_emoji end + # Special rule for deleted commits + rule { ~(can_read_noteable | commit_is_deleted) }.policy do + prevent :read_note + end + rule { is_author }.policy do enable :read_note enable :admin_note diff --git a/app/policies/snippet_policy.rb b/app/policies/snippet_policy.rb new file mode 100644 index 00000000000..64c56e8091d --- /dev/null +++ b/app/policies/snippet_policy.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class SnippetPolicy < PersonalSnippetPolicy +end diff --git a/app/services/metrics/dashboard/update_dashboard_service.rb b/app/services/metrics/dashboard/update_dashboard_service.rb index dccdedf61db..65e6e195f79 100644 --- a/app/services/metrics/dashboard/update_dashboard_service.rb +++ b/app/services/metrics/dashboard/update_dashboard_service.rb @@ -4,84 +4,89 @@ module Metrics module Dashboard class UpdateDashboardService < ::BaseService + include Stepable + ALLOWED_FILE_TYPE = '.yml' USER_DASHBOARDS_DIR = ::Metrics::Dashboard::ProjectDashboardService::DASHBOARD_ROOT - # rubocop:disable Cop/BanCatchThrow - def execute - catch(:error) do - throw(:error, error(_(%q(You are not allowed to push into this branch. Create another branch or open a merge request.)), :forbidden)) unless push_authorized? - - result = ::Files::UpdateService.new(project, current_user, dashboard_attrs).execute - throw(:error, result.merge(http_status: :bad_request)) unless result[:status] == :success + steps :check_push_authorized, + :check_branch_name, + :check_file_type, + :update_file - success(result.merge(http_status: :created, dashboard: dashboard_details)) - end + def execute + execute_steps end - # rubocop:enable Cop/BanCatchThrow private - def dashboard_attrs - { - commit_message: params[:commit_message], - file_path: update_dashboard_path, - file_content: update_dashboard_content, - encoding: 'text', - branch_name: branch, - start_branch: repository.branch_exists?(branch) ? branch : project.default_branch - } + def check_push_authorized(result) + return error(_('You are not allowed to push into this branch. Create another branch or open a merge request.'), :forbidden) unless push_authorized? + + success(result) end - def dashboard_details - { - path: update_dashboard_path, - display_name: ::Metrics::Dashboard::ProjectDashboardService.name_for_path(update_dashboard_path), - default: false, - system_dashboard: false - } + def check_branch_name(result) + return error(_('There was an error updating the dashboard, branch name is invalid.'), :bad_request) unless valid_branch_name? + return error(_('There was an error updating the dashboard, branch named: %{branch} already exists.') % { branch: params[:branch] }, :bad_request) unless new_or_default_branch? + + success(result) end - def push_authorized? - Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch) + def check_file_type(result) + return error(_('The file name should have a .yml extension'), :bad_request) unless target_file_type_valid? + + success(result) end - # rubocop:disable Cop/BanCatchThrow - def branch - @branch ||= begin - throw(:error, error(_('There was an error updating the dashboard, branch name is invalid.'), :bad_request)) unless valid_branch_name? - throw(:error, error(_('There was an error updating the dashboard, branch named: %{branch} already exists.') % { branch: params[:branch] }, :bad_request)) unless new_or_default_branch? + def update_file(result) + file_update_response = ::Files::UpdateService.new(project, current_user, dashboard_attrs).execute - params[:branch] + if file_update_response[:status] == :success + success(result.merge(file_update_response, http_status: :created, dashboard: dashboard_details)) + else + error(file_update_response[:message], :bad_request) end end - # rubocop:enable Cop/BanCatchThrow - def new_or_default_branch? - !repository.branch_exists?(params[:branch]) || project.default_branch == params[:branch] + def push_authorized? + Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch) end def valid_branch_name? - Gitlab::GitRefValidator.validate(params[:branch]) + Gitlab::GitRefValidator.validate(branch) end - def update_dashboard_path - File.join(USER_DASHBOARDS_DIR, file_name) + def new_or_default_branch? + !repository.branch_exists?(branch) || project.default_branch == branch end def target_file_type_valid? File.extname(params[:file_name]) == ALLOWED_FILE_TYPE end - # rubocop:disable Cop/BanCatchThrow + def dashboard_attrs + { + commit_message: params[:commit_message], + file_path: update_dashboard_path, + file_content: update_dashboard_content, + encoding: 'text', + branch_name: branch, + start_branch: repository.branch_exists?(branch) ? branch : project.default_branch + } + end + + def update_dashboard_path + File.join(USER_DASHBOARDS_DIR, file_name) + end + def file_name - @file_name ||= begin - throw(:error, error(_('The file name should have a .yml extension'), :bad_request)) unless target_file_type_valid? + @file_name ||= File.basename(CGI.unescape(params[:file_name])) + end - File.basename(CGI.unescape(params[:file_name])) - end + def branch + @branch ||= params[:branch] end - # rubocop:enable Cop/BanCatchThrow def update_dashboard_content ::PerformanceMonitoring::PrometheusDashboard.from_json(params[:file_content]).to_yaml @@ -90,6 +95,15 @@ module Metrics def repository @repository ||= project.repository end + + def dashboard_details + { + path: update_dashboard_path, + display_name: ::Metrics::Dashboard::ProjectDashboardService.name_for_path(update_dashboard_path), + default: false, + system_dashboard: false + } + end end end end diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index a4771e864d4..4b294a97516 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -2,8 +2,6 @@ module Projects class ImportService < BaseService - include Gitlab::ShellAdapter - Error = Class.new(StandardError) # Returns true if this importer is supposed to perform its work in the @@ -72,9 +70,9 @@ module Projects project.ensure_repository project.repository.fetch_as_mirror(project.import_url, refmap: refmap) else - gitlab_shell.import_project_repository(project) + project.repository.import_repository(project.import_url) end - rescue Gitlab::Shell::Error => e + rescue ::Gitlab::Git::CommandError => e # Expire cache to prevent scenarios such as: # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true # 2. Retried import, repo is broken or not imported but +exists?+ still returns true diff --git a/app/services/search_service.rb b/app/services/search_service.rb index fe5e823b56c..75cd6c78a52 100644 --- a/app/services/search_service.rb +++ b/app/services/search_service.rb @@ -3,6 +3,11 @@ class SearchService include Gitlab::Allowable + REDACTABLE_RESULTS = [ + ActiveRecord::Relation, + Gitlab::Search::FoundBlob + ].freeze + SEARCH_TERM_LIMIT = 64 SEARCH_CHAR_LIMIT = 4096 @@ -60,11 +65,52 @@ class SearchService end def search_objects - @search_objects ||= search_results.objects(scope, params[:page]) + @search_objects ||= redact_unauthorized_results(search_results.objects(scope, params[:page])) + end + + def redactable_results + REDACTABLE_RESULTS end private + def visible_result?(object) + return true unless object.respond_to?(:to_ability_name) && DeclarativePolicy.has_policy?(object) + + Ability.allowed?(current_user, :"read_#{object.to_ability_name}", object) + end + + def redact_unauthorized_results(results) + return results unless redactable_results.any? { |redactable| results.is_a?(redactable) } + + permitted_results = results.select do |object| + visible_result?(object) + end + + filtered_results = (results - permitted_results).each_with_object({}) do |object, memo| + memo[object.id] = { ability: :"read_#{object.to_ability_name}", id: object.id, class_name: object.class.name } + end + + log_redacted_search_results(filtered_results.values) if filtered_results.any? + + return results.id_not_in(filtered_results.keys) if results.is_a?(ActiveRecord::Relation) + + Kaminari.paginate_array( + permitted_results, + total_count: results.total_count, + limit: results.limit_value, + offset: results.offset_value + ) + end + + def log_redacted_search_results(filtered_results) + logger.error(message: "redacted_search_results", filtered: filtered_results, current_user_id: current_user&.id, query: params[:search]) + end + + def logger + @logger ||= ::Gitlab::RedactedSearchResultsLogger.build + end + def search_service @search_service ||= if project diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index 450ebb00b49..0e83932ff26 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -394,7 +394,7 @@ module ObjectStorage def storage_for(store) case store when Store::REMOTE - raise 'Object Storage is not enabled' unless self.class.object_store_enabled? + raise "Object Storage is not enabled for #{self.class}" unless self.class.object_store_enabled? CarrierWave::Storage::Fog.new(self) when Store::LOCAL diff --git a/app/views/ci/variables/_index.html.haml b/app/views/ci/variables/_index.html.haml index f11c730eba6..aadb2c62d83 100644 --- a/app/views/ci/variables/_index.html.haml +++ b/app/views/ci/variables/_index.html.haml @@ -5,7 +5,7 @@ - link_start = '<a href="%{url}">'.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: '</a>'.html_safe } -- if Feature.enabled?(:new_variables_ui, @project || @group) +- if Feature.enabled?(:new_variables_ui, @project || @group, default_enabled: true) - is_group = !@group.nil? #js-ci-project-variables{ data: { endpoint: save_endpoint, project_id: @project&.id || '', group: is_group.to_s, maskable_regex: ci_variable_maskable_regex} } diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 9b9b1417e22..28fab10d931 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -866,8 +866,8 @@ :idempotent: - :name: chat_notification :feature_category: :chatops - :has_external_dependencies: - :urgency: :high + :has_external_dependencies: true + :urgency: :low :resource_boundary: :unknown :weight: 2 :idempotent: diff --git a/app/workers/chat_notification_worker.rb b/app/workers/chat_notification_worker.rb index 058ac024f8a..5fab437f49f 100644 --- a/app/workers/chat_notification_worker.rb +++ b/app/workers/chat_notification_worker.rb @@ -7,13 +7,9 @@ class ChatNotificationWorker # rubocop:disable Scalability/IdempotentWorker sidekiq_options retry: false feature_category :chatops - urgency :high + urgency :low # Can't be high as it has external dependencies weight 2 - - # TODO: break this into multiple jobs - # as the `responder` uses external dependencies - # See https://gitlab.com/gitlab-com/gl-infra/scalability/issues/34 - # worker_has_external_dependencies! + worker_has_external_dependencies! RESCHEDULE_INTERVAL = 2.seconds RESCHEDULE_TIMEOUT = 5.minutes diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index b202c568d93..395fce0696c 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -2,7 +2,6 @@ class RepositoryForkWorker # rubocop:disable Scalability/IdempotentWorker include ApplicationWorker - include Gitlab::ShellAdapter include ProjectStartImport include ProjectImportOptions @@ -27,18 +26,8 @@ class RepositoryForkWorker # rubocop:disable Scalability/IdempotentWorker Gitlab::Metrics.add_event(:fork_repository) - result = gitlab_shell.fork_repository(source_project, target_project) - - if result - link_lfs_objects(source_project, target_project) - else - raise_fork_failure( - source_project, - target_project, - 'Failed to create fork repository' - ) - end - + gitaly_fork!(source_project, target_project) + link_lfs_objects(source_project, target_project) target_project.after_import end @@ -49,6 +38,17 @@ class RepositoryForkWorker # rubocop:disable Scalability/IdempotentWorker false end + def gitaly_fork!(source_project, target_project) + source_repo = source_project.repository.raw + target_repo = target_project.repository.raw + + ::Gitlab::GitalyClient::RepositoryService.new(target_repo).fork_repository(source_repo) + rescue GRPC::BadStatus => e + Gitlab::ErrorTracking.track_exception(e, source_project_id: source_project.id, target_project_id: target_project.id) + + raise_fork_failure(source_project, target_project, 'Failed to create fork repository') + end + def link_lfs_objects(source_project, target_project) Projects::LfsPointers::LfsLinkService .new(target_project) |