From 429d1abad29d379d8bc8f5219eb72384ad485deb Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 14 Oct 2019 15:06:07 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .gitlab/ci/review.gitlab-ci.yml | 8 +++ Gemfile.lock | 22 ++++---- .../components/error_tracking_list.vue | 33 ++++++------ app/assets/javascripts/labels_select.js | 14 +++++ .../javascripts/snippet/snippet_embed.js | 41 +++++++------- app/assets/stylesheets/framework/common.scss | 2 + app/controllers/concerns/uploads_actions.rb | 20 +++---- .../groups/settings/ci_cd_controller.rb | 23 ++++++++ .../projects/settings/ci_cd_controller.rb | 10 +++- app/controllers/uploads_controller.rb | 9 +++- app/policies/group_policy.rb | 5 +- app/policies/project_policy.rb | 2 + .../application_settings/_ci_cd.html.haml | 2 +- .../application_settings/_pages.html.haml | 2 +- .../groups/settings/ci_cd/_form.html.haml | 13 +++++ .../groups/settings/ci_cd/show.html.haml | 15 ++++++ .../projects/settings/ci_cd/_form.html.haml | 9 ++++ app/workers/migrate_external_diffs_worker.rb | 2 +- .../schedule_migrate_external_diffs_worker.rb | 2 +- .../update_project_statistics_worker.rb | 1 - ...roject-group-max-artifacts-size-fields.yml | 5 ++ .../psi-responsive-error-tracking.yml | 5 ++ config/routes/group.rb | 2 +- doc/administration/high_availability/redis.md | 2 +- doc/development/README.md | 3 +- doc/development/deleting_migrations.md | 33 ++++++++++++ .../settings/continuous_integration.md | 16 +++++- locale/gitlab.pot | 17 ++++-- .../groups/settings/ci_cd_controller_spec.rb | 54 +++++++++++++++++++ .../settings/ci_cd_controller_spec.rb | 24 +++++++++ spec/controllers/uploads_controller_spec.rb | 27 +++++----- spec/features/boards/sidebar_spec.rb | 31 +++++++++++ .../snippets/private_snippets_spec.rb | 22 ++++++++ .../features/snippets/public_snippets_spec.rb | 2 + .../snippets/user_creates_snippet_spec.rb | 8 +-- spec/policies/group_policy_spec.rb | 24 +++++++++ spec/policies/project_policy_spec.rb | 24 +++++++++ spec/requests/openid_connect_spec.rb | 25 +++------ 38 files changed, 451 insertions(+), 108 deletions(-) create mode 100644 app/views/groups/settings/ci_cd/_form.html.haml create mode 100644 changelogs/unreleased/eb-project-group-max-artifacts-size-fields.yml create mode 100644 changelogs/unreleased/psi-responsive-error-tracking.yml create mode 100644 doc/development/deleting_migrations.md create mode 100644 spec/features/snippets/private_snippets_spec.rb diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index ef7827fc917..7c4ba3878f1 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -113,13 +113,21 @@ schedule:review-build-cng: - install_api_client_dependencies_with_apk - source scripts/review_apps/review-apps.sh script: + - date - check_kube_domain + - date - ensure_namespace + - date - install_tiller + - date - install_external_dns + - date - download_chart + - date - deploy || (display_deployment_debug && exit 1) + - date - add_license + - date artifacts: paths: [review_app_url.txt] expire_in: 2 days diff --git a/Gemfile.lock b/Gemfile.lock index 4285cfac45e..e879fdc65fc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -95,7 +95,7 @@ GEM babosa (1.0.2) base32 (0.3.2) batch-loader (1.4.0) - bcrypt (3.1.12) + bcrypt (3.1.13) bcrypt_pbkdf (1.0.0) benchmark-ips (2.3.0) benchmark-memory (0.1.2) @@ -209,10 +209,10 @@ GEM descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) device_detector (1.0.0) - devise (4.6.2) + devise (4.7.1) bcrypt (~> 3.0) orm_adapter (~> 0.1) - railties (>= 4.1.0, < 6.0) + railties (>= 4.1.0) responders warden (~> 1.2.3) devise-two-factor (3.0.0) @@ -488,7 +488,7 @@ GEM mime-types (~> 3.0) multi_xml (>= 0.5.2) httpclient (2.8.3) - i18n (1.6.0) + i18n (1.7.0) concurrent-ruby (~> 1.0) i18n_data (0.8.0) icalendar (2.4.1) @@ -770,8 +770,8 @@ GEM rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) - rails-html-sanitizer (1.2.0) - loofah (~> 2.2, >= 2.2.2) + rails-html-sanitizer (1.3.0) + loofah (~> 2.3) rails-i18n (5.1.1) i18n (>= 0.7, < 2) railties (>= 5.0, < 6) @@ -824,9 +824,9 @@ GEM declarative-option (< 0.2.0) uber (< 0.2.0) request_store (1.3.1) - responders (2.4.0) - actionpack (>= 4.2.0, < 5.3) - railties (>= 4.2.0, < 5.3) + responders (2.4.1) + actionpack (>= 4.2.0, < 6.0) + railties (>= 4.2.0, < 6.0) rest-client (2.0.2) http-cookie (>= 1.0.2, < 2.0) mime-types (>= 1.16, < 4.0) @@ -1058,8 +1058,8 @@ GEM descendants_tracker (~> 0.0, >= 0.0.3) equalizer (~> 0.0, >= 0.0.9) vmstat (2.3.0) - warden (1.2.7) - rack (>= 1.0) + warden (1.2.8) + rack (>= 2.0.6) webfinger (1.1.0) activesupport httpclient (>= 2.4) diff --git a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue index 1d54c4a4264..3528f0a9335 100644 --- a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue +++ b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue @@ -7,10 +7,10 @@ import { __ } from '~/locale'; export default { fields: [ - { key: 'error', label: __('Open errors') }, + { key: 'error', label: __('Open errors'), thClass: 'w-70p' }, { key: 'events', label: __('Events') }, { key: 'users', label: __('Users') }, - { key: 'lastSeen', label: __('Last seen') }, + { key: 'lastSeen', label: __('Last seen'), thClass: 'w-15p' }, ], components: { GlEmptyState, @@ -67,40 +67,39 @@ export default {
{{ __('View in Sentry') }} - +
- + + diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 22b062563b5..72de3b5d726 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -32,6 +32,7 @@ export default class LabelsSelect { $selectbox, $sidebarCollapsedValue, $value, + $dropdownMenu, abilityName, defaultLabel, issueUpdateURL, @@ -67,6 +68,7 @@ export default class LabelsSelect { $sidebarCollapsedValue = $block.find('.sidebar-collapsed-icon span'); $sidebarLabelTooltip = $block.find('.js-sidebar-labels-tooltip'); $value = $block.find('.value'); + $dropdownMenu = $dropdown.parent().find('.dropdown-menu'); $loading = $block.find('.block-loading').fadeOut(); fieldName = $dropdown.data('fieldName'); initialSelected = $selectbox @@ -454,9 +456,21 @@ export default class LabelsSelect { } $loading.fadeIn(); + const oldLabels = boardsStore.detail.issue.labels; boardsStore.detail.issue .update($dropdown.attr('data-issue-update')) + .then(() => { + if (isScopedLabel(label)) { + const prevIds = oldLabels.map(label => label.id); + const newIds = boardsStore.detail.issue.labels.map(label => label.id); + const differentIds = _.difference(prevIds, newIds); + $dropdown.data('marked', newIds); + $dropdownMenu + .find(differentIds.map(id => `[data-label-id="${id}"]`).join(',')) + .removeClass('is-active'); + } + }) .then(fadeOutLoader) .catch(fadeOutLoader); } else if (handleClick) { diff --git a/app/assets/javascripts/snippet/snippet_embed.js b/app/assets/javascripts/snippet/snippet_embed.js index fe08d2c7ebb..6606271c4fa 100644 --- a/app/assets/javascripts/snippet/snippet_embed.js +++ b/app/assets/javascripts/snippet/snippet_embed.js @@ -1,25 +1,30 @@ import { __ } from '~/locale'; export default () => { - const { protocol, host, pathname } = window.location; const shareBtn = document.querySelector('.js-share-btn'); - const embedBtn = document.querySelector('.js-embed-btn'); - const snippetUrlArea = document.querySelector('.js-snippet-url-area'); - const embedAction = document.querySelector('.js-embed-action'); - const url = `${protocol}//${host + pathname}`; - shareBtn.addEventListener('click', () => { - shareBtn.classList.add('is-active'); - embedBtn.classList.remove('is-active'); - snippetUrlArea.value = url; - embedAction.innerText = __('Share'); - }); + if (shareBtn) { + const { protocol, host, pathname } = window.location; - embedBtn.addEventListener('click', () => { - embedBtn.classList.add('is-active'); - shareBtn.classList.remove('is-active'); - const scriptTag = ``; - snippetUrlArea.value = scriptTag; - embedAction.innerText = __('Embed'); - }); + const embedBtn = document.querySelector('.js-embed-btn'); + + const snippetUrlArea = document.querySelector('.js-snippet-url-area'); + const embedAction = document.querySelector('.js-embed-action'); + const url = `${protocol}//${host + pathname}`; + + shareBtn.addEventListener('click', () => { + shareBtn.classList.add('is-active'); + embedBtn.classList.remove('is-active'); + snippetUrlArea.value = url; + embedAction.innerText = __('Share'); + }); + + embedBtn.addEventListener('click', () => { + embedBtn.classList.add('is-active'); + shareBtn.classList.remove('is-active'); + const scriptTag = ``; + snippetUrlArea.value = scriptTag; + embedAction.innerText = __('Embed'); + }); + } }; diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 922051ab0e9..16cb63fc0df 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -452,6 +452,8 @@ img.emoji { .w-0 { width: 0; } .w-8em { width: 8em; } .w-3rem { width: 3rem; } +.w-15p { width: 15%; } +.w-70p { width: 70%; } .h-12em { height: 12em; } .h-32-px { height: 32px;} diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb index 77afffc4b27..6d9ee39f841 100644 --- a/app/controllers/concerns/uploads_actions.rb +++ b/app/controllers/concerns/uploads_actions.rb @@ -29,13 +29,15 @@ module UploadsActions def show return render_404 unless uploader&.exists? - if cache_publicly? - # We need to reset caching from the applications controller to get rid of the no-store value - headers['Cache-Control'] = '' - expires_in 5.minutes, public: true, must_revalidate: false - else - expires_in 0.seconds, must_revalidate: true, private: true - end + # We need to reset caching from the applications controller to get rid of the no-store value + headers['Cache-Control'] = '' + headers['Pragma'] = '' + + ttl, directives = *cache_settings + ttl ||= 6.months + directives ||= { private: true, must_revalidate: true } + + expires_in ttl, directives disposition = uploader.embeddable? ? 'inline' : 'attachment' @@ -120,8 +122,8 @@ module UploadsActions nil end - def cache_publicly? - false + def cache_settings + [] end def model diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb index c465e622de0..0e83d057484 100644 --- a/app/controllers/groups/settings/ci_cd_controller.rb +++ b/app/controllers/groups/settings/ci_cd_controller.rb @@ -5,11 +5,22 @@ module Groups class CiCdController < Groups::ApplicationController skip_cross_project_access_check :show before_action :authorize_admin_group! + before_action :authorize_update_max_artifacts_size!, only: [:update] def show define_ci_variables end + def update + if update_group_service.execute + flash[:notice] = s_('GroupSettings|Pipeline settings was updated for the group') + else + flash[:alert] = s_("GroupSettings|There was a problem updating the pipeline settings: %{error_messages}." % { error_messages: group.errors.full_messages }) + end + + redirect_to group_settings_ci_cd_path + end + def reset_registration_token @group.reset_runners_token! @@ -40,6 +51,10 @@ module Groups return render_404 unless can?(current_user, :admin_group, group) end + def authorize_update_max_artifacts_size! + return render_404 unless can?(current_user, :update_max_artifacts_size, group) + end + def auto_devops_params params.require(:group).permit(:auto_devops_enabled) end @@ -47,6 +62,14 @@ module Groups def auto_devops_service Groups::AutoDevopsService.new(group, current_user, auto_devops_params) end + + def update_group_service + Groups::UpdateService.new(group, current_user, update_group_params) + end + + def update_group_params + params.require(:group).permit(:max_artifacts_size) + end end end end diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index 0d61c3cc031..cfed8727450 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -46,13 +46,19 @@ module Projects private def update_params - params.require(:project).permit( + params.require(:project).permit(*permitted_project_params) + end + + def permitted_project_params + [ :runners_token, :builds_enabled, :build_allow_git_fetch, :build_timeout_human_readable, :build_coverage_regex, :public_builds, :auto_cancel_pending_pipelines, :ci_config_path, auto_devops_attributes: [:id, :domain, :enabled, :deploy_strategy], ci_cd_settings_attributes: [:default_git_depth] - ) + ].tap do |list| + list << :max_artifacts_size if can?(current_user, :update_max_artifacts_size, project) + end end def run_autodevops_pipeline(service) diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 2adfeab182e..635db386792 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -81,8 +81,13 @@ class UploadsController < ApplicationController end end - def cache_publicly? - User === model || Appearance === model + def cache_settings + case model + when User, Appearance + [5.minutes, { public: true, must_revalidate: false }] + when Project, Group + [5.minutes, { private: true, must_revalidate: true }] + end end def secret? diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 951a104122d..9e8ee3acf00 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -53,7 +53,10 @@ class GroupPolicy < BasePolicy enable :upload_file end - rule { admin }.enable :read_group + rule { admin }.policy do + enable :read_group + enable :update_max_artifacts_size + end rule { has_projects }.policy do enable :read_group diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index e6f8d1052ed..a3540f31077 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -137,6 +137,8 @@ class ProjectPolicy < BasePolicy # not. rule { guest | admin }.enable :read_project_for_iids + rule { admin }.enable :update_max_artifacts_size + rule { guest }.enable :guest_access rule { reporter }.enable :reporter_access rule { developer }.enable :developer_access diff --git a/app/views/admin/application_settings/_ci_cd.html.haml b/app/views/admin/application_settings/_ci_cd.html.haml index d1de4286ee7..1f5bce19bc6 100644 --- a/app/views/admin/application_settings/_ci_cd.html.haml +++ b/app/views/admin/application_settings/_ci_cd.html.haml @@ -34,7 +34,7 @@ = f.number_field :max_artifacts_size, class: 'form-control' .form-text.text-muted = _("Set the maximum file size for each job's artifacts") - = link_to icon('question-circle'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'maximum-artifacts-size') + = link_to icon('question-circle'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'maximum-artifacts-size-core-only') .form-group = f.label :default_artifacts_expire_in, _('Default artifacts expiration'), class: 'label-bold' = f.text_field :default_artifacts_expire_in, class: 'form-control' diff --git a/app/views/admin/application_settings/_pages.html.haml b/app/views/admin/application_settings/_pages.html.haml index 1282a032f52..b15afb3b806 100644 --- a/app/views/admin/application_settings/_pages.html.haml +++ b/app/views/admin/application_settings/_pages.html.haml @@ -30,6 +30,6 @@ = f.check_box :lets_encrypt_terms_of_service_accepted, class: 'form-check-input' = f.label :lets_encrypt_terms_of_service_accepted, class: 'form-check-label' do - terms_of_service_link_start = ''.html_safe % { url: lets_encrypt_terms_of_service_admin_application_settings_path } - = _("I have read and agree to the Let's Encrypt %{link_start}Terms of Service%{link_end}").html_safe % { link_start: terms_of_service_link_start, link_end: ''.html_safe } + = _("I have read and agree to the Let's Encrypt %{link_start}Terms of Service%{link_end} (PDF)").html_safe % { link_start: terms_of_service_link_start, link_end: ''.html_safe } = f.submit _('Save changes'), class: "btn btn-success" diff --git a/app/views/groups/settings/ci_cd/_form.html.haml b/app/views/groups/settings/ci_cd/_form.html.haml new file mode 100644 index 00000000000..54e88d11827 --- /dev/null +++ b/app/views/groups/settings/ci_cd/_form.html.haml @@ -0,0 +1,13 @@ +.row.prepend-top-default + .col-lg-12 + = form_for group, url: group_settings_ci_cd_path(group, anchor: 'js-general-pipeline-settings') do |f| + = form_errors(group) + %fieldset.builds-feature + .form-group + = f.label :max_artifacts_size, _('Maximum artifacts size (MB)'), class: 'label-bold' + = f.number_field :max_artifacts_size, class: 'form-control' + %p.form-text.text-muted + = _("Set the maximum file size for each job's artifacts") + = link_to icon('question-circle'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'maximum-artifacts-size-core-only'), target: '_blank' + + = f.submit _('Save changes'), class: "btn btn-success" diff --git a/app/views/groups/settings/ci_cd/show.html.haml b/app/views/groups/settings/ci_cd/show.html.haml index d21496ee0aa..a3f35b72cc6 100644 --- a/app/views/groups/settings/ci_cd/show.html.haml +++ b/app/views/groups/settings/ci_cd/show.html.haml @@ -2,6 +2,21 @@ - page_title "CI / CD" - expanded = expanded_by_default? +- general_expanded = @group.errors.empty? ? expanded : true + +-# Given we only have one field in this form which is also admin-only, +-# we don't want to show an empty section to non-admin users, +- if can?(current_user, :update_max_artifacts_size, @group) + %section.settings#js-general-pipeline-settings.no-animate{ class: ('expanded' if general_expanded) } + .settings-header + %h4 + = _("General pipelines") + %button.btn.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') + %p + = _("Customize your pipeline configuration.") + .settings-content + = render 'groups/settings/ci_cd/form', group: @group %section.settings#ci-variables.no-animate{ class: ('expanded' if expanded) } .settings-header diff --git a/app/views/projects/settings/ci_cd/_form.html.haml b/app/views/projects/settings/ci_cd/_form.html.haml index 430d6071468..a674136e791 100644 --- a/app/views/projects/settings/ci_cd/_form.html.haml +++ b/app/views/projects/settings/ci_cd/_form.html.haml @@ -40,6 +40,15 @@ = _('If any job surpasses this timeout threshold, it will be marked as failed. Human readable time input language is accepted like "1 hour". Values without specification represent seconds.') = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'timeout'), target: '_blank' + - if can?(current_user, :update_max_artifacts_size, @project) + %hr + .form-group + = f.label :max_artifacts_size, _('Maximum artifacts size (MB)'), class: 'label-bold' + = f.number_field :max_artifacts_size, class: 'form-control' + %p.form-text.text-muted + = _("Set the maximum file size for each job's artifacts") + = link_to icon('question-circle'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'maximum-artifacts-size-core-only'), target: '_blank' + %hr .form-group = f.label :ci_config_path, _('Custom CI config path'), class: 'label-bold' diff --git a/app/workers/migrate_external_diffs_worker.rb b/app/workers/migrate_external_diffs_worker.rb index debac97af2c..fe757968d49 100644 --- a/app/workers/migrate_external_diffs_worker.rb +++ b/app/workers/migrate_external_diffs_worker.rb @@ -1,4 +1,4 @@ -# frozen_string_literal: false +# frozen_string_literal: true class MigrateExternalDiffsWorker include ApplicationWorker diff --git a/app/workers/schedule_migrate_external_diffs_worker.rb b/app/workers/schedule_migrate_external_diffs_worker.rb index 70910f7ca04..04a370f01af 100644 --- a/app/workers/schedule_migrate_external_diffs_worker.rb +++ b/app/workers/schedule_migrate_external_diffs_worker.rb @@ -1,4 +1,4 @@ -# frozen_string_literal: false +# frozen_string_literal: true class ScheduleMigrateExternalDiffsWorker include ApplicationWorker diff --git a/app/workers/update_project_statistics_worker.rb b/app/workers/update_project_statistics_worker.rb index 9a29cc12707..3abb7e34a9d 100644 --- a/app/workers/update_project_statistics_worker.rb +++ b/app/workers/update_project_statistics_worker.rb @@ -1,4 +1,3 @@ - # frozen_string_literal: true # Worker for updating project statistics. diff --git a/changelogs/unreleased/eb-project-group-max-artifacts-size-fields.yml b/changelogs/unreleased/eb-project-group-max-artifacts-size-fields.yml new file mode 100644 index 00000000000..8e911ffead5 --- /dev/null +++ b/changelogs/unreleased/eb-project-group-max-artifacts-size-fields.yml @@ -0,0 +1,5 @@ +--- +title: Add max_artifacts_size fields under project and group settings. +merge_request: 18286 +author: +type: added diff --git a/changelogs/unreleased/psi-responsive-error-tracking.yml b/changelogs/unreleased/psi-responsive-error-tracking.yml new file mode 100644 index 00000000000..36be17d7bf0 --- /dev/null +++ b/changelogs/unreleased/psi-responsive-error-tracking.yml @@ -0,0 +1,5 @@ +--- +title: Fix error tracking table layout on small screens +merge_request: 18325 +author: +type: fixed diff --git a/config/routes/group.rb b/config/routes/group.rb index 37bc6085931..1baac9874a2 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -30,7 +30,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do as: :group, constraints: { group_id: Gitlab::PathRegex.full_namespace_route_regex }) do namespace :settings do - resource :ci_cd, only: [:show], controller: 'ci_cd' do + resource :ci_cd, only: [:show, :update], controller: 'ci_cd' do put :reset_registration_token patch :update_auto_devops end diff --git a/doc/administration/high_availability/redis.md b/doc/administration/high_availability/redis.md index b82e8947e39..ba4599e5bcd 100644 --- a/doc/administration/high_availability/redis.md +++ b/doc/administration/high_availability/redis.md @@ -68,7 +68,7 @@ Omnibus: gitaly['enable'] = false redis['bind'] = '0.0.0.0' - redis['port'] = '6379' + redis['port'] = 6379 redis['password'] = 'SECRET_PASSWORD_HERE' gitlab_rails['auto_migrate'] = false diff --git a/doc/development/README.md b/doc/development/README.md index 6f712fcf0f4..7e1a563ea02 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -99,6 +99,7 @@ description: 'Learn how to contribute to GitLab.' - [Post deployment migrations](post_deployment_migrations.md) - [Background migrations](background_migrations.md) - [Swapping tables](swapping_tables.md) +- [Deleting exiting migrations](deleting_migrations.md) ### Best practices @@ -118,7 +119,7 @@ description: 'Learn how to contribute to GitLab.' - [Database helper modules](database_helpers.md) - [Code comments](code_comments.md) -## Case studies +### Case studies - [Database case study: Filtering by label](filtering_by_label.md) - [Database case study: Namespaces storage statistics](namespaces_storage_statistics.md) diff --git a/doc/development/deleting_migrations.md b/doc/development/deleting_migrations.md new file mode 100644 index 00000000000..438e8c9f5e9 --- /dev/null +++ b/doc/development/deleting_migrations.md @@ -0,0 +1,33 @@ +# Delete existing migrations + +When removing existing migrations from the GitLab project, you have to take into account +the possibility of the migration already been included in past releases or in the current release, and thus already executed on GitLab.com and/or in self-hosted instances. + +Because of it, it's not possible to delete existing migrations, as that could lead to: + +- Schema inconsistency, as changes introduced into the database were not rollbacked properly. +- Leaving a record on the `schema_versions` table, that points out to migration that no longer exists on the codebase. + +Instead of deleting we can opt for disabling the migration. + +## Pre-requisites to disable a migration + +Migrations can be disabled if: + +- They caused a timeout or general issue on GitLab.com. +- They are obsoleted, e.g. changes are not necessary due to a feature change. +- Migration is a data migration only, i.e. the migration does not change the database schema. + +## How to disable a data migration? + +In order to disable a migration, the following steps apply to all types of migrations: + +1. Turn the migration into a noop by removing the code inside `#up`, `#down` + or `#perform` methods, and adding `#no-op` comment instead. +1. Add a comment explaining why the code is gone. + +Disabling migrations requires explicit approval of Database Maintainer. + +## Examples + +- [Disable scheduling of productivity analytics](https://gitlab.com/gitlab-org/gitlab/merge_requests/17253) diff --git a/doc/user/admin_area/settings/continuous_integration.md b/doc/user/admin_area/settings/continuous_integration.md index 6ba027dc24a..ca2a0127ede 100644 --- a/doc/user/admin_area/settings/continuous_integration.md +++ b/doc/user/admin_area/settings/continuous_integration.md @@ -29,15 +29,27 @@ If you want to disable it for a specific project, you can do so in ## Maximum artifacts size **(CORE ONLY)** The maximum size of the [job artifacts](../../../administration/job_artifacts.md) -can be set in the Admin area of your GitLab instance. The value is in *MB* and +can be set at the project level, group level, and at the instance level. The value is in *MB* and the default is 100MB per job; on GitLab.com it's [set to 1G](../../gitlab_com/index.md#gitlab-cicd). -To change it: +To change it at the instance level: 1. Go to **Admin area > Settings > Continuous Integration and Deployment**. 1. Change the value of maximum artifacts size (in MB). 1. Hit **Save changes** for the changes to take effect. +at the group level (this will override the instance setting): + +1. Go to **Group > Settings > CI / CD > General Pipelines**. +1. Change the value of maximum artifacts size (in MB). +1. Hit **Save changes** for the changes to take effect. + +at the project level (this will override the instance and group settings): + +1. Go to **Project > Settings > CI / CD > General Pipelines**. +1. Change the value of maximum artifacts size (in MB). +1. Hit **Save changes** for the changes to take effect. + ## Default artifacts expiration **(CORE ONLY)** The default expiration time of the [job artifacts](../../../administration/job_artifacts.md) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ae3c240419b..0efaa04625c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4026,6 +4026,9 @@ msgstr "" msgid "Code owners" msgstr "" +msgid "CodeAnalytics|Max files" +msgstr "" + msgid "CodeOwner|Pattern" msgstr "" @@ -4840,6 +4843,9 @@ msgstr "" msgid "Customize your pipeline configuration, view your pipeline status and coverage report." msgstr "" +msgid "Customize your pipeline configuration." +msgstr "" + msgid "Cycle Analytics" msgstr "" @@ -8222,6 +8228,9 @@ msgstr "" msgid "GroupSettings|New runners registration token has been generated!" msgstr "" +msgid "GroupSettings|Pipeline settings was updated for the group" +msgstr "" + msgid "GroupSettings|Prevent sharing a project within %{group} with other groups" msgstr "" @@ -8234,6 +8243,9 @@ msgstr "" msgid "GroupSettings|There was a problem updating Auto DevOps pipeline: %{error_messages}." msgstr "" +msgid "GroupSettings|There was a problem updating the pipeline settings: %{error_messages}." +msgstr "" + msgid "GroupSettings|This setting is applied on %{ancestor_group} and has been overridden on this subgroup." msgstr "" @@ -8479,7 +8491,7 @@ msgstr "" msgid "I forgot my password" msgstr "" -msgid "I have read and agree to the Let's Encrypt %{link_start}Terms of Service%{link_end}" +msgid "I have read and agree to the Let's Encrypt %{link_start}Terms of Service%{link_end} (PDF)" msgstr "" msgid "I'd like to receive updates via email about GitLab" @@ -10758,9 +10770,6 @@ msgstr "" msgid "No deployments found" msgstr "" -msgid "No details available" -msgstr "" - msgid "No due date" msgstr "" diff --git a/spec/controllers/groups/settings/ci_cd_controller_spec.rb b/spec/controllers/groups/settings/ci_cd_controller_spec.rb index 70b3a5fb496..897ba491036 100644 --- a/spec/controllers/groups/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/groups/settings/ci_cd_controller_spec.rb @@ -156,4 +156,58 @@ describe Groups::Settings::CiCdController do end end end + + describe 'PATCH #update' do + subject do + patch :update, params: { + group_id: group, + group: { max_artifacts_size: 10 } + } + end + + context 'when user is not an admin' do + before do + group.add_owner(user) + end + + it { is_expected.to have_gitlab_http_status(404) } + end + + context 'when user is an admin' do + let(:user) { create(:admin) } + + before do + group.add_owner(user) + end + + it { is_expected.to redirect_to(group_settings_ci_cd_path) } + + context 'when service execution went wrong' do + let(:update_service) { double } + + before do + allow(Groups::UpdateService).to receive(:new).and_return(update_service) + allow(update_service).to receive(:execute).and_return(false) + allow_any_instance_of(Group).to receive_message_chain(:errors, :full_messages) + .and_return(['Error 1']) + + subject + end + + it 'returns a flash alert' do + expect(response).to set_flash[:alert] + .to eq("There was a problem updating the pipeline settings: [\"Error 1\"].") + end + end + + context 'when service execution was successful' do + it 'returns a flash notice' do + subject + + expect(response).to set_flash[:notice] + .to eq('Pipeline settings was updated for the group') + end + end + end + end end diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb index f4dbfbe15e8..93507b58910 100644 --- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb @@ -215,6 +215,30 @@ describe Projects::Settings::CiCdController do expect(project.ci_default_git_depth).to eq(10) end end + + context 'when max_artifacts_size is specified' do + let(:params) { { max_artifacts_size: 10 } } + + context 'and user is not an admin' do + it 'does not set max_artifacts_size' do + subject + + project.reload + expect(project.max_artifacts_size).to be_nil + end + end + + context 'and user is an admin' do + let(:user) { create(:admin) } + + it 'sets max_artifacts_size' do + subject + + project.reload + expect(project.max_artifacts_size).to eq(10) + end + end + end end end end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index 5f4a6bf8ee7..dd7ab4f9d47 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -1,16 +1,15 @@ # frozen_string_literal: true require 'spec_helper' -shared_examples 'content not cached without revalidation' do +shared_examples 'content 5 min private cached with revalidation' do it 'ensures content will not be cached without revalidation' do - expect(subject['Cache-Control']).to eq('max-age=0, private, must-revalidate') + expect(subject['Cache-Control']).to eq('max-age=300, private, must-revalidate') end end -shared_examples 'content not cached without revalidation and no-store' do +shared_examples 'content long term private cached with revalidation' do it 'ensures content will not be cached without revalidation' do - # Fixed in newer versions of ActivePack, it will only output a single `private`. - expect(subject['Cache-Control']).to eq('max-age=0, private, must-revalidate, no-store') + expect(subject['Cache-Control']).to eq('max-age=15778476, private, must-revalidate') end end @@ -285,7 +284,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation' do + it_behaves_like 'content 5 min private cached with revalidation' do subject do get :show, params: { model: 'project', mounted_as: 'avatar', id: project.id, filename: 'dk.png' } @@ -305,7 +304,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation and no-store' do + it_behaves_like 'content 5 min private cached with revalidation' do subject do get :show, params: { model: 'project', mounted_as: 'avatar', id: project.id, filename: 'dk.png' } @@ -358,7 +357,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation and no-store' do + it_behaves_like 'content 5 min private cached with revalidation' do subject do get :show, params: { model: 'project', mounted_as: 'avatar', id: project.id, filename: 'dk.png' } @@ -390,7 +389,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation' do + it_behaves_like 'content 5 min private cached with revalidation' do subject do get :show, params: { model: 'group', mounted_as: 'avatar', id: group.id, filename: 'dk.png' } @@ -410,7 +409,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation and no-store' do + it_behaves_like 'content 5 min private cached with revalidation' do subject do get :show, params: { model: 'group', mounted_as: 'avatar', id: group.id, filename: 'dk.png' } @@ -454,7 +453,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation and no-store' do + it_behaves_like 'content 5 min private cached with revalidation' do subject do get :show, params: { model: 'group', mounted_as: 'avatar', id: group.id, filename: 'dk.png' } @@ -491,7 +490,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation' do + it_behaves_like 'content long term private cached with revalidation' do subject do get :show, params: { model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png' } @@ -511,7 +510,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation and no-store' do + it_behaves_like 'content long term private cached with revalidation' do subject do get :show, params: { model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png' } @@ -564,7 +563,7 @@ describe UploadsController do expect(response).to have_gitlab_http_status(200) end - it_behaves_like 'content not cached without revalidation and no-store' do + it_behaves_like 'content long term private cached with revalidation' do subject do get :show, params: { model: 'note', mounted_as: 'attachment', id: note.id, filename: 'dk.png' } diff --git a/spec/features/boards/sidebar_spec.rb b/spec/features/boards/sidebar_spec.rb index 2b923df40c5..2fc79272c21 100644 --- a/spec/features/boards/sidebar_spec.rb +++ b/spec/features/boards/sidebar_spec.rb @@ -14,6 +14,8 @@ describe 'Issue Boards', :js do let!(:bug) { create(:label, project: project, name: 'Bug') } let!(:regression) { create(:label, project: project, name: 'Regression') } let!(:stretch) { create(:label, project: project, name: 'Stretch') } + let!(:scoped_label_1) { create(:label, project: project, name: 'Scoped::Label1') } + let!(:scoped_label_2) { create(:label, project: project, name: 'Scoped::Label2') } let!(:issue1) { create(:labeled_issue, project: project, assignees: [user], milestone: milestone, labels: [development], relative_position: 2) } let!(:issue2) { create(:labeled_issue, project: project, labels: [development, stretch], relative_position: 1) } let(:board) { create(:board, project: project) } @@ -27,6 +29,8 @@ describe 'Issue Boards', :js do end before do + stub_licensed_features(scoped_labels: true) + project.add_maintainer(user) sign_in(user) @@ -309,6 +313,33 @@ describe 'Issue Boards', :js do expect(card).to have_content(bug.title) end + it 'removes existing scoped label' do + click_card(card) + + page.within('.labels') do + click_link 'Edit' + + wait_for_requests + + click_link scoped_label_1.title + click_link scoped_label_2.title + + wait_for_requests + + find('.dropdown-menu-close-icon').click + + page.within('.value') do + expect(page).to have_selector('.badge', count: 3) + expect(page).not_to have_content(scoped_label_1.title) + expect(page).to have_content(scoped_label_2.title) + end + end + + expect(card).to have_selector('.badge', count: 3) + expect(card).not_to have_content(scoped_label_1.title) + expect(card).to have_content(scoped_label_2.title) + end + it 'adds a multiple labels' do click_card(card) diff --git a/spec/features/snippets/private_snippets_spec.rb b/spec/features/snippets/private_snippets_spec.rb new file mode 100644 index 00000000000..9df4cd01103 --- /dev/null +++ b/spec/features/snippets/private_snippets_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Private Snippets', :js do + let(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'Private Snippet renders for creator' do + private_snippet = create(:personal_snippet, :private, author: user) + + visit snippet_path(private_snippet) + wait_for_requests + + expect(page).to have_content(private_snippet.content) + expect(page).not_to have_css('.js-embed-btn') + expect(page).not_to have_css('.js-share-btn') + end +end diff --git a/spec/features/snippets/public_snippets_spec.rb b/spec/features/snippets/public_snippets_spec.rb index a0db00cfc67..82edda509c2 100644 --- a/spec/features/snippets/public_snippets_spec.rb +++ b/spec/features/snippets/public_snippets_spec.rb @@ -10,6 +10,8 @@ describe 'Public Snippets', :js do wait_for_requests expect(page).to have_content(public_snippet.content) + expect(page).to have_css('.js-embed-btn', visible: false) + expect(page).to have_css('.js-share-btn', visible: false) end it 'Unauthenticated user should see raw public snippets' do diff --git a/spec/features/snippets/user_creates_snippet_spec.rb b/spec/features/snippets/user_creates_snippet_spec.rb index 52ec5eddd5c..9a141dd463a 100644 --- a/spec/features/snippets/user_creates_snippet_spec.rb +++ b/spec/features/snippets/user_creates_snippet_spec.rb @@ -45,7 +45,9 @@ describe 'User creates snippet', :js do link = find('a.no-attachment-icon img[alt="banana_sample"]')['src'] expect(link).to match(%r{/uploads/-/system/user/#{user.id}/\h{32}/banana_sample\.gif\z}) - reqs = inspect_requests { visit(link) } + # Adds a cache buster for checking if the image exists as Selenium is now handling the cached regquests + # not anymore as requests when they come straight from memory cache. + reqs = inspect_requests { visit("#{link}?ran=#{SecureRandom.base64(20)}") } expect(reqs.first.status_code).to eq(200) end end @@ -63,7 +65,7 @@ describe 'User creates snippet', :js do link = find('a.no-attachment-icon img[alt="banana_sample"]')['src'] expect(link).to match(%r{/uploads/-/system/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z}) - reqs = inspect_requests { visit(link) } + reqs = inspect_requests { visit("#{link}?ran=#{SecureRandom.base64(20)}") } expect(reqs.first.status_code).to eq(200) end @@ -88,7 +90,7 @@ describe 'User creates snippet', :js do link = find('a.no-attachment-icon img[alt="banana_sample"]')['src'] expect(link).to match(%r{/uploads/-/system/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z}) - reqs = inspect_requests { visit(link) } + reqs = inspect_requests { visit("#{link}?ran=#{SecureRandom.base64(20)}") } expect(reqs.first.status_code).to eq(200) end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 02bcc716bee..603e7e874c9 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -547,4 +547,28 @@ describe GroupPolicy do groups: [clusterable]) end end + + describe 'update_max_artifacts_size' do + let(:group) { create(:group, :public) } + + context 'when no user' do + let(:current_user) { nil } + + it { expect_disallowed(:update_max_artifacts_size) } + end + + context 'admin' do + let(:current_user) { admin } + + it { expect_allowed(:update_max_artifacts_size) } + end + + %w(guest reporter developer maintainer owner).each do |role| + context role do + let(:current_user) { send(role) } + + it { expect_disallowed(:update_max_artifacts_size) } + end + end + end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 71ba73d5661..6093464c949 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -478,4 +478,28 @@ describe ProjectPolicy do end end end + + describe 'update_max_artifacts_size' do + subject { described_class.new(current_user, project) } + + context 'when no user' do + let(:current_user) { nil } + + it { expect_disallowed(:update_max_artifacts_size) } + end + + context 'admin' do + let(:current_user) { admin } + + it { expect_allowed(:update_max_artifacts_size) } + end + + %w(guest reporter developer maintainer owner).each do |role| + context role do + let(:current_user) { send(role) } + + it { expect_disallowed(:update_max_artifacts_size) } + end + end + end end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index da2e7b71dbe..dfa17c5ff27 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -148,34 +148,25 @@ describe 'OpenID Connect requests' do end end - # These 2 calls shouldn't actually throw, they should be handled as an - # unauthorized request, so we should be able to check the response. - # - # This was not possible due to an issue with Warden: - # https://github.com/hassox/warden/pull/162 - # - # When the patch gets merged and we update Warden, these specs will need to - # updated to check the response instead of a raised exception. - # https://gitlab.com/gitlab-org/gitlab-foss/issues/40218 context 'when user is blocked' do - it 'returns authentication error' do + it 'redirects to login page' do access_grant user.block! - expect do - request_access_token! - end.to raise_error UncaughtThrowError + request_access_token! + + expect(response).to redirect_to('/users/sign_in') end end context 'when user is ldap_blocked' do - it 'returns authentication error' do + it 'redirects to login page' do access_grant user.ldap_block! - expect do - request_access_token! - end.to raise_error UncaughtThrowError + request_access_token! + + expect(response).to redirect_to('/users/sign_in') end end end