diff options
48 files changed, 365 insertions, 242 deletions
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2dc69925710..001b3e376bb 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -544,15 +544,6 @@ Style/Lambda: Style/MissingRespondToMissing: Enabled: false -# Offense count: 5 -Style/MixinUsage: - Exclude: - - 'spec/factories/ci/builds.rb' - - 'spec/factories/ci/job_artifacts.rb' - - 'spec/factories/lfs_objects.rb' - - 'spec/factories/notes.rb' - - 'spec/lib/gitlab/import_export/version_checker_spec.rb' - # Offense count: 35 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, MinBodyLength. @@ -603,13 +594,6 @@ Style/SingleArgumentDig: Style/SoleNestedConditional: Enabled: false -# Offense count: 120 -# Cop supports --auto-correct. -# Configuration parameters: . -# SupportedStyles: use_perl_names, use_english_names -Style/SpecialGlobalVars: - EnforcedStyle: use_perl_names - # Offense count: 562 # Cop supports --auto-correct. Style/StringConcatenation: diff --git a/.rubocop_todo/style/special_global_vars.yml b/.rubocop_todo/style/special_global_vars.yml new file mode 100644 index 00000000000..559d8169f9f --- /dev/null +++ b/.rubocop_todo/style/special_global_vars.yml @@ -0,0 +1,60 @@ +--- +# Cop supports --auto-correct. +Style/SpecialGlobalVars: + Exclude: + - 'app/controllers/help_controller.rb' + - 'app/models/application_setting_implementation.rb' + - 'app/services/prometheus/proxy_variable_substitution_service.rb' + - 'ee/bin/geo_log_cursor' + - 'ee/lib/ee/banzai/filter/references/epic_reference_filter.rb' + - 'ee/lib/ee/banzai/filter/references/iteration_reference_filter.rb' + - 'ee/lib/ee/banzai/filter/references/vulnerability_reference_filter.rb' + - 'lib/backup/database.rb' + - 'lib/banzai/filter/blockquote_fence_filter.rb' + - 'lib/banzai/filter/commit_trailers_filter.rb' + - 'lib/banzai/filter/front_matter_filter.rb' + - 'lib/banzai/filter/inline_metrics_redactor_filter.rb' + - 'lib/banzai/filter/references/abstract_reference_filter.rb' + - 'lib/banzai/filter/references/commit_range_reference_filter.rb' + - 'lib/banzai/filter/references/commit_reference_filter.rb' + - 'lib/banzai/filter/references/external_issue_reference_filter.rb' + - 'lib/banzai/filter/references/label_reference_filter.rb' + - 'lib/banzai/filter/references/milestone_reference_filter.rb' + - 'lib/banzai/filter/references/project_reference_filter.rb' + - 'lib/banzai/filter/references/reference_cache.rb' + - 'lib/banzai/filter/references/user_reference_filter.rb' + - 'lib/banzai/filter/sanitization_filter.rb' + - 'lib/extracts_ref.rb' + - 'lib/gitlab/dependency_linker/godeps_json_linker.rb' + - 'lib/gitlab/gfm/uploads_rewriter.rb' + - 'lib/gitlab/git/gitmodules_parser.rb' + - 'lib/gitlab/graphql/queries.rb' + - 'lib/gitlab/hook_data/base_builder.rb' + - 'lib/gitlab/log_timestamp_formatter.rb' + - 'lib/gitlab/puma_logging/json_formatter.rb' + - 'lib/gitlab/quick_actions/extractor.rb' + - 'lib/gitlab/runtime.rb' + - 'lib/gitlab/string_placeholder_replacer.rb' + - 'lib/prometheus/pid_provider.rb' + - 'lib/tasks/lint.rake' + - 'qa/chemlab-library-gitlab.gemspec' + - 'qa/qa/git/repository.rb' + - 'qa/qa/service/cluster_provider/gcloud.rb' + - 'qa/qa/support/wait_for_requests.rb' + - 'scripts/api/cancel_pipeline.rb' + - 'scripts/api/download_job_artifact.rb' + - 'scripts/api/get_job_id.rb' + - 'scripts/changed-feature-flags' + - 'scripts/failed_tests.rb' + - 'scripts/perf/query_limiting_report.rb' + - 'scripts/pipeline_test_report_builder.rb' + - 'scripts/rubocop-max-files-in-cache-check' + - 'scripts/setup/find-jh-branch.rb' + - 'scripts/static-analysis' + - 'scripts/trigger-build.rb' + - 'spec/fast_spec_helper.rb' + - 'spec/rack_servers/puma_spec.rb' + - 'spec/spec_helper.rb' + - 'spec/support/generate-seed-repo-rb' + - 'spec/support/helpers/test_env.rb' + - 'spec/support/prepare-gitlab-git-test-for-commit' diff --git a/app/assets/javascripts/behaviors/markdown/render_gfm.js b/app/assets/javascripts/behaviors/markdown/render_gfm.js index c4e09efe263..4bfce12c7c5 100644 --- a/app/assets/javascripts/behaviors/markdown/render_gfm.js +++ b/app/assets/javascripts/behaviors/markdown/render_gfm.js @@ -1,6 +1,5 @@ import $ from 'jquery'; import syntaxHighlight from '~/syntax_highlight'; -import initUserPopovers from '../../user_popovers'; import highlightCurrentUser from './highlight_current_user'; import renderMath from './render_math'; import renderMermaid from './render_mermaid'; @@ -20,7 +19,6 @@ $.fn.renderGFM = function renderGFM() { renderMermaid(this.find('.js-render-mermaid')); } highlightCurrentUser(this.find('.gfm-project_member').get()); - initUserPopovers(this.find('.js-user-link').get()); const mrPopoverElements = this.find('.gfm-merge_request').get(); if (mrPopoverElements.length) { diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue index ba10f6deb29..8d0f6079f8f 100644 --- a/app/assets/javascripts/diffs/components/commit_item.vue +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -7,8 +7,6 @@ import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -import initUserPopovers from '../../user_popovers'; - /** * CommitItem * @@ -82,11 +80,6 @@ export default { return this.commit.description_html.replace(/^
/, ''); }, }, - created() { - this.$nextTick(() => { - initUserPopovers(this.$el.querySelectorAll('.js-user-link')); - }); - }, safeHtmlConfig: { ADD_TAGS: ['gl-emoji'], }, diff --git a/app/assets/javascripts/flash.js b/app/assets/javascripts/flash.js index fa605f8c056..24ec16bf20e 100644 --- a/app/assets/javascripts/flash.js +++ b/app/assets/javascripts/flash.js @@ -94,10 +94,10 @@ const addDismissFlashClickListener = (flashEl, fadeTransition) => { * * 1. Render a new alert * - * import { createAlert, ALERT_VARIANTS } from '~/flash'; + * import { createAlert, VARIANT_WARNING } from '~/flash'; * * createAlert({ message: 'My error message' }); - * createAlert({ message: 'My warning message', variant: ALERT_VARIANTS.WARNING }); + * createAlert({ message: 'My warning message', variant: VARIANT_WARNING }); * * 2. Dismiss this alert programmatically * diff --git a/app/assets/javascripts/members/components/table/members_table.vue b/app/assets/javascripts/members/components/table/members_table.vue index 0b97ce7e33e..660fbd79fe3 100644 --- a/app/assets/javascripts/members/components/table/members_table.vue +++ b/app/assets/javascripts/members/components/table/members_table.vue @@ -4,7 +4,6 @@ import { mapState } from 'vuex'; import MembersTableCell from 'ee_else_ce/members/components/table/members_table_cell.vue'; import { canOverride, canRemove, canResend, canUpdate } from 'ee_else_ce/members/utils'; import { mergeUrlParams } from '~/lib/utils/url_utility'; -import initUserPopovers from '~/user_popovers'; import UserDate from '~/vue_shared/components/user_date.vue'; import { FIELD_KEY_ACTIONS, @@ -85,9 +84,6 @@ export default { return this.tabQueryParamValue === TAB_QUERY_PARAM_VALUES.invite; }, }, - mounted() { - initUserPopovers(this.$el.querySelectorAll('.js-user-link')); - }, methods: { hasActionButtons(member) { return ( diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue index c4924cd41f5..c8af3c744e8 100644 --- a/app/assets/javascripts/notes/components/notes_app.vue +++ b/app/assets/javascripts/notes/components/notes_app.vue @@ -3,7 +3,6 @@ import { mapGetters, mapActions } from 'vuex'; import highlightCurrentUser from '~/behaviors/markdown/highlight_current_user'; import createFlash from '~/flash'; import { __ } from '~/locale'; -import initUserPopovers from '~/user_popovers'; import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue'; import OrderedLayout from '~/vue_shared/components/ordered_layout.vue'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; @@ -169,7 +168,6 @@ export default { updated() { this.$nextTick(() => { highlightCurrentUser(this.$el.querySelectorAll('.gfm-project_member')); - initUserPopovers(this.$el.querySelectorAll('.js-user-link')); }); }, beforeDestroy() { diff --git a/app/assets/javascripts/user_popovers.js b/app/assets/javascripts/user_popovers.js index 4544373d8aa..597b96ae18c 100644 --- a/app/assets/javascripts/user_popovers.js +++ b/app/assets/javascripts/user_popovers.js @@ -57,41 +57,35 @@ const populateUserInfo = (user) => { ); }; -const initializedPopovers = new Map(); -let domObservedForChanges = false; +function initPopover(el, user, mountPopover) { + const preloadedUserInfo = getPreloadedUserInfo(el.dataset); -const addPopoversToModifiedTree = new MutationObserver(() => { - const userLinks = document?.querySelectorAll('.js-user-link, .gfm-project_member'); + Object.assign(user, preloadedUserInfo); - if (userLinks) { - addPopovers(userLinks); /* eslint-disable-line no-use-before-define */ + if (preloadedUserInfo.userId) { + populateUserInfo(user); } -}); - -function observeBody() { - if (!domObservedForChanges) { - addPopoversToModifiedTree.observe(document.body, { - subtree: true, - childList: true, - }); - - domObservedForChanges = true; - } -} - -export default function addPopovers(elements = document.querySelectorAll('.js-user-link')) { - const userLinks = Array.from(elements); const UserPopoverComponent = Vue.extend(UserPopover); + const popoverInstance = new UserPopoverComponent({ + propsData: { + target: el, + user, + }, + }); + mountPopover(popoverInstance); + // wait for component to actually mount + setTimeout(() => { + // trigger an event to force tooltip to show + const event = new MouseEvent('mouseenter'); + event.isSelfTriggered = true; + el.dispatchEvent(event); + }); +} - observeBody(); - - return userLinks - .filter(({ dataset }) => dataset.user || dataset.userId) - .map((el) => { - if (initializedPopovers.has(el)) { - return initializedPopovers.get(el); - } - +function initPopovers(userLinks, mountPopover) { + userLinks + .filter(({ dataset, user }) => !user && (dataset.user || dataset.userId)) + .forEach((el) => { const user = { location: null, bio: null, @@ -99,31 +93,60 @@ export default function addPopovers(elements = document.querySelectorAll('.js-us status: null, loaded: false, }; - const renderedPopover = new UserPopoverComponent({ - propsData: { - target: el, - user, - }, + el.user = user; + const init = initPopover.bind(null, el, user, mountPopover); + el.addEventListener('mouseenter', init, { once: true }); + el.addEventListener('mouseenter', ({ target, isSelfTriggered }) => { + if (!isSelfTriggered) return; + removeTitle(target); }); + el.addEventListener('mouseleave', ({ target }) => { + target.removeAttribute('aria-describedby'); + }); + }); +} - initializedPopovers.set(el, renderedPopover); +const userLinkSelector = 'a.js-user-link, a.gfm-project_member'; - renderedPopover.$mount(); +const getUserLinkNodes = (node) => { + if (!('matches' in node)) return null; + if (node.matches(userLinkSelector)) return [node]; + return Array.from(node.querySelectorAll(userLinkSelector)); +}; - el.addEventListener('mouseenter', ({ target }) => { - removeTitle(target); - const preloadedUserInfo = getPreloadedUserInfo(target.dataset); +let observer; - Object.assign(user, preloadedUserInfo); +export default function addPopovers( + elements = document.querySelectorAll('.js-user-link'), + mountPopover = (popoverInstance) => popoverInstance.$mount(), +) { + const userLinks = Array.from(elements); - if (preloadedUserInfo.userId) { - populateUserInfo(user); - } - }); - el.addEventListener('mouseleave', ({ target }) => { - target.removeAttribute('aria-describedby'); - }); + initPopovers(userLinks, mountPopover); + + if (!observer) { + observer = new MutationObserver((mutationsList) => { + const newUserLinks = mutationsList + .filter((mutation) => mutation.type === 'childList' && mutation.addedNodes) + .reduce((acc, mutation) => { + const userLinkNodes = Array.from(mutation.addedNodes) + .flatMap(getUserLinkNodes) + .filter(Boolean); + acc.push(...userLinkNodes); + return acc; + }, []); + + if (newUserLinks.length !== 0) { + initPopovers(newUserLinks, mountPopover); + } + }); + observer.observe(document.body, { + subtree: true, + childList: true, + }); - return renderedPopover; + document.addEventListener('beforeunload', () => { + observer.disconnect(); }); + } } diff --git a/app/assets/javascripts/vue_shared/alert_details/components/alert_details.vue b/app/assets/javascripts/vue_shared/alert_details/components/alert_details.vue index d595c49f9aa..5f6490a9f3f 100644 --- a/app/assets/javascripts/vue_shared/alert_details/components/alert_details.vue +++ b/app/assets/javascripts/vue_shared/alert_details/components/alert_details.vue @@ -18,7 +18,6 @@ import { toggleContainerClasses } from '~/lib/utils/dom_utils'; import { visitUrl, joinPaths } from '~/lib/utils/url_utility'; import { s__ } from '~/locale'; import Tracking from '~/tracking'; -import initUserPopovers from '~/user_popovers'; import AlertDetailsTable from '~/vue_shared/components/alert_details_table.vue'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import { PAGE_CONFIG, SEVERITY_LEVELS } from '../constants'; @@ -175,7 +174,6 @@ export default { updated() { this.$nextTick(() => { highlightCurrentUser(this.$el.querySelectorAll('.gfm-project_member')); - initUserPopovers(this.$el.querySelectorAll('.js-user-link')); }); }, methods: { diff --git a/app/controllers/projects/google_cloud/base_controller.rb b/app/controllers/projects/google_cloud/base_controller.rb index f293ec752ab..0d65431d870 100644 --- a/app/controllers/projects/google_cloud/base_controller.rb +++ b/app/controllers/projects/google_cloud/base_controller.rb @@ -25,7 +25,11 @@ class Projects::GoogleCloud::BaseController < Projects::ApplicationController end def feature_flag_enabled! - unless Feature.enabled?(:incubation_5mp_google_cloud) + enabled_for_user = Feature.enabled?(:incubation_5mp_google_cloud, current_user) + enabled_for_group = Feature.enabled?(:incubation_5mp_google_cloud, project.group) + enabled_for_project = Feature.enabled?(:incubation_5mp_google_cloud, project) + feature_is_enabled = enabled_for_user || enabled_for_group || enabled_for_project + unless feature_is_enabled track_event('feature_flag_enabled!', 'access_denied', 'feature_flag_not_enabled') access_denied! end diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb index d3af6a00181..2942765a108 100644 --- a/app/helpers/submodule_helper.rb +++ b/app/helpers/submodule_helper.rb @@ -30,7 +30,7 @@ module SubmoduleHelper end end - namespace.sub!(%r{\A/}, '') + namespace.delete_prefix!('/') project.rstrip! project.delete_suffix!('.git') diff --git a/app/views/notify/_note_email.html.haml b/app/views/notify/_note_email.html.haml index 55984472047..f03e524a0f3 100644 --- a/app/views/notify/_note_email.html.haml +++ b/app/views/notify/_note_email.html.haml @@ -2,6 +2,7 @@ - diff_limit = local_assigns.fetch(:diff_limit, nil) - target_url = local_assigns.fetch(:target_url, @target_url) - note_style = local_assigns.fetch(:note_style, "") +- skip_stylesheet_link = local_assigns.fetch(:skip_stylesheet_link, false) - discussion = note.discussion if note.part_of_discussion? @@ -22,8 +23,9 @@ = link_to 'discussion', target_url - if discussion&.diff_discussion? && discussion.on_text? - = content_for :head do - = stylesheet_link_tag 'mailers/highlighted_diff_email' + - unless skip_stylesheet_link + = content_for :head do + = stylesheet_link_tag 'mailers/highlighted_diff_email' %table.code = render partial: "projects/diffs/email_line", diff --git a/app/views/notify/new_review_email.html.haml b/app/views/notify/new_review_email.html.haml index 11da7723d8d..d5399c872d5 100644 --- a/app/views/notify/new_review_email.html.haml +++ b/app/views/notify/new_review_email.html.haml @@ -1,3 +1,6 @@ += content_for :head do + = stylesheet_link_tag 'mailers/highlighted_diff_email' + %table{ border: "0", cellpadding: "0", cellspacing: "0", style: "width:100%;margin:0 auto;border-collapse:separate;border-spacing:0;" } %tbody %tr @@ -13,4 +16,4 @@ %td{ style: "overflow:hidden;font-size:14px;line-height:1.4;display:grid;" } - @notes.each do |note| - target_url = project_merge_request_url(@project, @merge_request, anchor: "note_#{note.id}") - = render 'note_email', note: note, diff_limit: 3, target_url: target_url, note_style: "border-bottom:1px solid #ededed;" + = render 'note_email', note: note, diff_limit: 3, target_url: target_url, note_style: "border-bottom:1px solid #ededed;", skip_stylesheet_link: true diff --git a/config/feature_flags/development/header_read_timeout_buffered_io.yml b/config/feature_flags/development/header_read_timeout_buffered_io.yml deleted file mode 100644 index ba7ef4cc000..00000000000 --- a/config/feature_flags/development/header_read_timeout_buffered_io.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: header_read_timeout_buffered_io -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78065 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350233 -milestone: '14.8' -type: development -group: group::integrations -default_enabled: true diff --git a/config/feature_flags/development/runner_list_group_view_vue_ui.yml b/config/feature_flags/development/runner_list_group_view_vue_ui.yml index 3bda540ba5b..542bad1b236 100644 --- a/config/feature_flags/development/runner_list_group_view_vue_ui.yml +++ b/config/feature_flags/development/runner_list_group_view_vue_ui.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/336405 milestone: '14.2' type: development group: group::runner -default_enabled: false +default_enabled: true diff --git a/config/metrics/counts_7d/20210916102312_templates_gitlab_slack_application_active.yml b/config/metrics/counts_7d/20210916102312_templates_gitlab_slack_application_active.yml index d465e1b3d03..27e0505c2cd 100644 --- a/config/metrics/counts_7d/20210916102312_templates_gitlab_slack_application_active.yml +++ b/config/metrics/counts_7d/20210916102312_templates_gitlab_slack_application_active.yml @@ -7,8 +7,9 @@ product_stage: ecosystem product_group: group::integrations product_category: integrations value_type: number -status: active +status: removed milestone: "14.3" +milestone_removed: "14.4" introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70496 time_frame: 7d data_source: database diff --git a/config/routes.rb b/config/routes.rb index c2114b3478d..972773467e6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -325,7 +325,7 @@ Rails.application.routes.draw do root to: "root#index" - get '*unmatched_route', to: 'application#route_not_found' + get '*unmatched_route', to: 'application#route_not_found', format: false end Gitlab::Routing.add_helpers(TimeboxesRoutingHelper) diff --git a/danger/z_metadata/Dangerfile b/danger/z_metadata/Dangerfile index 8aeb0c33a9c..9140bb7d988 100644 --- a/danger/z_metadata/Dangerfile +++ b/danger/z_metadata/Dangerfile @@ -2,7 +2,7 @@ # rubocop:disable Style/SignalException -DEFAULT_BRANCH = 'master' +default_branch = ENV['CI_DEFAULT_BRANCH'] || 'main' if gitlab.mr_body.size < 5 fail "Please provide a proper merge request description." @@ -14,12 +14,12 @@ end has_milestone = !gitlab.mr_json["milestone"].nil? -unless has_milestone || (helper.security_mr? && helper.mr_target_branch == DEFAULT_BRANCH) +unless has_milestone || (helper.security_mr? && helper.mr_target_branch == default_branch) warn "This merge request does not refer to an existing milestone.", sticky: false end has_pick_into_stable_label = helper.mr_labels.find { |label| label.start_with?('Pick into') } -if helper.mr_target_branch != DEFAULT_BRANCH && !has_pick_into_stable_label && !helper.security_mr? - warn "Most of the time, merge requests should target `#{DEFAULT_BRANCH}`. Otherwise, please set the relevant `Pick into X.Y` label." +if helper.mr_target_branch != default_branch && !has_pick_into_stable_label && !helper.security_mr? + warn "Most of the time, merge requests should target `#{default_branch}`. Otherwise, please set the relevant `Pick into X.Y` label." end diff --git a/lib/banzai/filter/repository_link_filter.rb b/lib/banzai/filter/repository_link_filter.rb index 408e6dc685d..f5cf1833304 100644 --- a/lib/banzai/filter/repository_link_filter.rb +++ b/lib/banzai/filter/repository_link_filter.rb @@ -180,7 +180,7 @@ module Banzai parts.pop if uri_type(request_path) != :tree - path.sub!(%r{\A\./}, '') + path.delete_prefix!('./') while path.start_with?('../') parts.pop diff --git a/lib/gitlab/auth/ldap/dn.rb b/lib/gitlab/auth/ldap/dn.rb index ea88dedadf5..a188aa168c1 100644 --- a/lib/gitlab/auth/ldap/dn.rb +++ b/lib/gitlab/auth/ldap/dn.rb @@ -30,7 +30,7 @@ module Gitlab def self.normalize_value(given_value) dummy_dn = "placeholder=#{given_value}" normalized_dn = new(*dummy_dn).to_normalized_s - normalized_dn.sub(/\Aplaceholder=/, '') + normalized_dn.delete_prefix('placeholder=') end ## diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb index 08321d5fda6..82ef7eed56a 100644 --- a/lib/gitlab/gfm/uploads_rewriter.rb +++ b/lib/gitlab/gfm/uploads_rewriter.rb @@ -37,7 +37,7 @@ module Gitlab if was_embedded?(markdown) moved_markdown else - moved_markdown.sub(/\A!/, "") + moved_markdown.delete_prefix('!') end end end diff --git a/lib/gitlab/git/ref.rb b/lib/gitlab/git/ref.rb index 47cfb483509..1d7966a11ed 100644 --- a/lib/gitlab/git/ref.rb +++ b/lib/gitlab/git/ref.rb @@ -24,7 +24,7 @@ module Gitlab # Ex. # Ref.extract_branch_name('refs/heads/master') #=> 'master' def self.extract_branch_name(str) - str.gsub(%r{\Arefs/heads/}, '') + str.delete_prefix('refs/heads/') end def initialize(repository, name, target, dereferenced_target) diff --git a/lib/gitlab/http_connection_adapter.rb b/lib/gitlab/http_connection_adapter.rb index 002708beb3c..7b1657d3854 100644 --- a/lib/gitlab/http_connection_adapter.rb +++ b/lib/gitlab/http_connection_adapter.rb @@ -29,17 +29,13 @@ module Gitlab http = super http.hostname_override = hostname if hostname - if Feature.enabled?(:header_read_timeout_buffered_io, default_enabled: :yaml) - gitlab_http = Gitlab::NetHttpAdapter.new(http.address, http.port) + gitlab_http = Gitlab::NetHttpAdapter.new(http.address, http.port) - http.instance_variables.each do |variable| - gitlab_http.instance_variable_set(variable, http.instance_variable_get(variable)) - end - - return gitlab_http + http.instance_variables.each do |variable| + gitlab_http.instance_variable_set(variable, http.instance_variable_get(variable)) end - http + gitlab_http end private diff --git a/lib/gitlab/project_template.rb b/lib/gitlab/project_template.rb index 629f49563c9..e7a12edf763 100644 --- a/lib/gitlab/project_template.rb +++ b/lib/gitlab/project_template.rb @@ -29,7 +29,7 @@ module Gitlab end def project_path - URI.parse(preview).path.sub(%r{\A/}, '') + URI.parse(preview).path.delete_prefix('/') end def uri_encoded_project_path diff --git a/lib/gitlab/setup_helper.rb b/lib/gitlab/setup_helper.rb index d19b8a2d4c6..a498e329c3f 100644 --- a/lib/gitlab/setup_helper.rb +++ b/lib/gitlab/setup_helper.rb @@ -98,7 +98,7 @@ module Gitlab storages << { name: key, path: storage_paths[key] } end - config = { socket_path: address.sub(/\Aunix:/, '') } + config = { socket_path: address.delete_prefix('unix:') } if Rails.env.test? socket_filename = options[:gitaly_socket] || "gitaly.socket" diff --git a/lib/gitlab/time_tracking_formatter.rb b/lib/gitlab/time_tracking_formatter.rb index 67ecf498cf7..87861b61119 100644 --- a/lib/gitlab/time_tracking_formatter.rb +++ b/lib/gitlab/time_tracking_formatter.rb @@ -8,7 +8,8 @@ module Gitlab CUSTOM_DAY_AND_MONTH_LENGTH = { hours_per_day: 8, days_per_month: 20 }.freeze def parse(string) - string = string.sub(/\A-/, '') + negative_time = string.start_with?('-') + string = string.delete_prefix('-') seconds = begin @@ -19,7 +20,7 @@ module Gitlab nil end - seconds *= -1 if seconds && Regexp.last_match + seconds *= -1 if seconds && negative_time seconds end diff --git a/lib/sidebars/projects/menus/infrastructure_menu.rb b/lib/sidebars/projects/menus/infrastructure_menu.rb index c012b3bb627..7bd9ac91efa 100644 --- a/lib/sidebars/projects/menus/infrastructure_menu.rb +++ b/lib/sidebars/projects/menus/infrastructure_menu.rb @@ -90,7 +90,10 @@ module Sidebars end def google_cloud_menu_item - feature_is_enabled = Feature.enabled?(:incubation_5mp_google_cloud, context.project) + enabled_for_user = Feature.enabled?(:incubation_5mp_google_cloud, context.current_user) + enabled_for_group = Feature.enabled?(:incubation_5mp_google_cloud, context.project.group) + enabled_for_project = Feature.enabled?(:incubation_5mp_google_cloud, context.project) + feature_is_enabled = enabled_for_user || enabled_for_group || enabled_for_project user_has_permissions = can?(context.current_user, :admin_project_google_cloud, context.project) unless feature_is_enabled && user_has_permissions diff --git a/qa/qa/specs/features/browser_ui/3_create/merge_request/revert/reverting_merge_request_spec.rb b/qa/qa/specs/features/browser_ui/3_create/merge_request/revert/reverting_merge_request_spec.rb index 90ca836f8b0..d66895de9c1 100644 --- a/qa/qa/specs/features/browser_ui/3_create/merge_request/revert/reverting_merge_request_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/merge_request/revert/reverting_merge_request_spec.rb @@ -19,7 +19,7 @@ module QA Flow::Login.sign_in end - it 'can be reverted', :can_use_large_setup, testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347709', quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/335987', type: :investigating } do + it 'can be reverted', :can_use_large_setup, testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347709' do revertable_merge_request.visit! Page::MergeRequest::Show.perform do |merge_request| diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index 1e453ba8816..d51880b282d 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -339,7 +339,7 @@ RSpec.describe Projects::ArtifactsController do def params @params ||= begin - base64_params = send_data.sub(/\Aartifacts\-entry:/, '') + base64_params = send_data.delete_prefix('artifacts-entry:') Gitlab::Json.parse(Base64.urlsafe_decode64(base64_params)) end end diff --git a/spec/fast_spec_helper.rb b/spec/fast_spec_helper.rb index ce3c9af22f1..6cbe97fb3f3 100644 --- a/spec/fast_spec_helper.rb +++ b/spec/fast_spec_helper.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# $" is $LOADED_FEATURES, but RuboCop didn't like it if $".include?(File.expand_path('spec_helper.rb', __dir__)) # There's no need to load anything here if spec_helper is already loaded # because spec_helper is more extensive than fast_spec_helper diff --git a/spec/frontend/diffs/components/commit_item_spec.js b/spec/frontend/diffs/components/commit_item_spec.js index d887029124f..b3d0781fde5 100644 --- a/spec/frontend/diffs/components/commit_item_spec.js +++ b/spec/frontend/diffs/components/commit_item_spec.js @@ -6,8 +6,6 @@ import Component from '~/diffs/components/commit_item.vue'; import { getTimeago } from '~/lib/utils/datetime_utility'; import CommitPipelineStatus from '~/projects/tree/components/commit_pipeline_status_component.vue'; -jest.mock('~/user_popovers'); - const TEST_AUTHOR_NAME = 'test'; const TEST_AUTHOR_EMAIL = 'test+test@gitlab.com'; const TEST_AUTHOR_GRAVATAR = `${TEST_HOST}/avatar/test?s=40`; diff --git a/spec/frontend/members/components/table/members_table_spec.js b/spec/frontend/members/components/table/members_table_spec.js index 298a01e4f4d..c02f483dac9 100644 --- a/spec/frontend/members/components/table/members_table_spec.js +++ b/spec/frontend/members/components/table/members_table_spec.js @@ -21,7 +21,6 @@ import { BADGE_LABELS_PENDING_OWNER_APPROVAL, TAB_QUERY_PARAM_VALUES, } from '~/members/constants'; -import * as initUserPopovers from '~/user_popovers'; import { member as memberMock, directMember, @@ -257,14 +256,6 @@ describe('MembersTable', () => { }); }); - it('initializes user popovers when mounted', () => { - const initUserPopoversMock = jest.spyOn(initUserPopovers, 'default'); - - createComponent(); - - expect(initUserPopoversMock).toHaveBeenCalled(); - }); - it('adds QA selector to table', () => { createComponent(); diff --git a/spec/frontend/notes/components/notes_app_spec.js b/spec/frontend/notes/components/notes_app_spec.js index bf36d6cb7a2..312af853d68 100644 --- a/spec/frontend/notes/components/notes_app_spec.js +++ b/spec/frontend/notes/components/notes_app_spec.js @@ -18,8 +18,6 @@ import '~/behaviors/markdown/render_gfm'; import OrderedLayout from '~/vue_shared/components/ordered_layout.vue'; import * as mockData from '../mock_data'; -jest.mock('~/user_popovers', () => jest.fn()); - setTestTimeout(1000); const TYPE_COMMENT_FORM = 'comment-form'; diff --git a/spec/frontend/user_popovers_spec.js b/spec/frontend/user_popovers_spec.js index 745b66fd700..f3e0a37e163 100644 --- a/spec/frontend/user_popovers_spec.js +++ b/spec/frontend/user_popovers_spec.js @@ -18,12 +18,13 @@ describe('User Popovers', () => { return link; }; + const findPopovers = () => { + return Array.from(document.querySelectorAll('[data-testid="user-popover"]')); + }; const dummyUser = { name: 'root' }; const dummyUserStatus = { message: 'active' }; - let popovers; - const triggerEvent = (eventName, el) => { const event = new MouseEvent(eventName, { bubbles: true, @@ -45,29 +46,49 @@ describe('User Popovers', () => { .spyOn(UsersCache, 'retrieveStatusById') .mockImplementation((userId) => userStatusCacheSpy(userId)); - popovers = initUserPopovers(document.querySelectorAll(selector)); + initUserPopovers(document.querySelectorAll(selector), (popoverInstance) => { + const mountingRoot = document.createElement('div'); + document.body.appendChild(mountingRoot); + popoverInstance.$mount(mountingRoot); + }); }); - it('initializes a popover for each user link with a user id', () => { - const linksWithUsers = findFixtureLinks(); + describe('shows a placeholder popover on hover', () => { + let linksWithUsers; + beforeEach(() => { + linksWithUsers = findFixtureLinks(); + linksWithUsers.forEach((el) => { + triggerEvent('mouseenter', el); + }); + }); - expect(linksWithUsers.length).toBe(popovers.length); - }); + it('for initial links', () => { + expect(findPopovers().length).toBe(linksWithUsers.length); + }); - it('adds popovers to user links added to the DOM tree after the initial call', async () => { - document.body.appendChild(createUserLink()); - document.body.appendChild(createUserLink()); + it('for elements added after initial load', async () => { + const addedLinks = [createUserLink(), createUserLink()]; + addedLinks.forEach((link) => { + document.body.appendChild(link); + }); - const linksWithUsers = findFixtureLinks(); + await Promise.resolve(); - expect(linksWithUsers.length).toBe(popovers.length + 2); + addedLinks.forEach((link) => { + triggerEvent('mouseenter', link); + }); + + expect(findPopovers().length).toBe(linksWithUsers.length + addedLinks.length); + }); }); it('does not initialize the user popovers twice for the same element', () => { - const newPopovers = initUserPopovers(document.querySelectorAll(selector)); - const samePopovers = popovers.every((popover, index) => newPopovers[index] === popover); + const [firstUserLink] = findFixtureLinks(); + triggerEvent('mouseenter', firstUserLink); + triggerEvent('mouseleave', firstUserLink); + triggerEvent('mouseenter', firstUserLink); - expect(samePopovers).toBe(true); + expect(findPopovers().length).toBe(1); }); describe('when user link emits mouseenter event', () => { @@ -86,11 +107,11 @@ describe('User Popovers', () => { expect(userLink.dataset.originalTitle).toBeFalsy(); }); - it('populates popovers with preloaded user data', () => { + it('populates popover with preloaded user data', () => { const { name, userId, username } = userLink.dataset; - const [firstPopover] = popovers; + const [firstPopover] = findFixtureLinks(); - expect(firstPopover.$props.user).toEqual( + expect(firstPopover.user).toEqual( expect.objectContaining({ name, userId, diff --git a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb index 5b78acc3b1d..f878f02f410 100644 --- a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb +++ b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb @@ -67,7 +67,7 @@ RSpec.describe Gitlab::Gfm::UploadsRewriter do it 'does not rewrite plain links as embedded' do embedded_link = image_uploader.markdown_link - plain_image_link = embedded_link.sub(/\A!/, "") + plain_image_link = embedded_link.delete_prefix('!') text = "#{plain_image_link} and #{embedded_link}" moved_text = described_class.new(text, old_project, user).rewrite(new_project) diff --git a/spec/lib/gitlab/http_connection_adapter_spec.rb b/spec/lib/gitlab/http_connection_adapter_spec.rb index e9e517f1fe6..cde8376febd 100644 --- a/spec/lib/gitlab/http_connection_adapter_spec.rb +++ b/spec/lib/gitlab/http_connection_adapter_spec.rb @@ -27,16 +27,6 @@ RSpec.describe Gitlab::HTTPConnectionAdapter do end end - context 'with header_read_timeout_buffered_io feature disabled' do - before do - stub_feature_flags(header_read_timeout_buffered_io: false) - end - - it 'uses the regular Net::HTTP class' do - expect(connection).to be_a(Net::HTTP) - end - end - context 'when local requests are allowed' do let(:options) { { allow_local_requests: true } } diff --git a/spec/lib/gitlab/import_export/version_checker_spec.rb b/spec/lib/gitlab/import_export/version_checker_spec.rb index 8b39330656f..9e69e04b17c 100644 --- a/spec/lib/gitlab/import_export/version_checker_spec.rb +++ b/spec/lib/gitlab/import_export/version_checker_spec.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true require 'spec_helper' -include ImportExport::CommonUtil RSpec.describe Gitlab::ImportExport::VersionChecker do + include ImportExport::CommonUtil + let!(:shared) { Gitlab::ImportExport::Shared.new(nil) } describe 'bundle a project Git repo' do diff --git a/spec/lib/sidebars/projects/menus/infrastructure_menu_spec.rb b/spec/lib/sidebars/projects/menus/infrastructure_menu_spec.rb index 8a6b0e4e95d..81114f5a0b3 100644 --- a/spec/lib/sidebars/projects/menus/infrastructure_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/infrastructure_menu_spec.rb @@ -112,6 +112,38 @@ RSpec.describe Sidebars::Projects::Menus::InfrastructureMenu do let(:item_id) { :google_cloud } it_behaves_like 'access rights checks' + + context 'when feature flag is turned off globally' do + before do + stub_feature_flags(incubation_5mp_google_cloud: false) + end + + it { is_expected.to be_nil } + + context 'when feature flag is enabled for specific project' do + before do + stub_feature_flags(incubation_5mp_google_cloud: project) + end + + it_behaves_like 'access rights checks' + end + + context 'when feature flag is enabled for specific group' do + before do + stub_feature_flags(incubation_5mp_google_cloud: project.group) + end + + it_behaves_like 'access rights checks' + end + + context 'when feature flag is enabled for specific project' do + before do + stub_feature_flags(incubation_5mp_google_cloud: user) + end + + it_behaves_like 'access rights checks' + end + end end end end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 4b7f0057559..e2ee63078bb 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -2189,6 +2189,10 @@ RSpec.describe Notify do ) end end + + it 'includes only one link to the highlighted_diff_email' do + expect(subject.html_part.body.raw_source).to include('assets/mailers/highlighted_diff_email').once + end end it 'contains review author name' do diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 65772895826..21012399edf 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -905,6 +905,13 @@ RSpec.describe 'project routing' do ) end + it 'routes to 404 without format for invalid page' do + expect(get: "/gitlab/gitlabhq/-/metrics/invalid_page.md").to route_to( + 'application#route_not_found', + unmatched_route: 'gitlab/gitlabhq/-/metrics/invalid_page.md' + ) + end + it 'routes to 404 with invalid dashboard_path' do expect(get: "/gitlab/gitlabhq/-/metrics/invalid_dashboard").to route_to( 'application#route_not_found', diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 86a7c079ea9..e9f2da93016 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# $" is $LOADED_FEATURES, but RuboCop didn't like it if $".include?(File.expand_path('fast_spec_helper.rb', __dir__)) warn 'Detected fast_spec_helper is loaded first than spec_helper.' warn 'If running test files using both spec_helper and fast_spec_helper,' diff --git a/workhorse/README.md b/workhorse/README.md index c7617645b34..ee62c230aa7 100644 --- a/workhorse/README.md +++ b/workhorse/README.md @@ -1,3 +1,9 @@ +--- +stage: Create +group: Source Code +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + # GitLab Workhorse GitLab Workhorse is a smart reverse proxy for GitLab. It handles @@ -10,26 +16,25 @@ GitLab](doc/architecture/gitlab_features.md) that would not work efficiently wit ## Canonical source The canonical source for Workhorse is -[gitlab-org/gitlab/workhorse](https://gitlab.com/gitlab-org/gitlab/tree/master/workhorse). -Prior to https://gitlab.com/groups/gitlab-org/-/epics/4826, it was -[gitlab-org/gitlab-workhorse](https://gitlab.com/gitlab-org/gitlab-workhorse/tree/master), +[`gitlab-org/gitlab/workhorse`](https://gitlab.com/gitlab-org/gitlab/tree/master/workhorse). +Prior to [epic #4826](https://gitlab.com/groups/gitlab-org/-/epics/4826), it was +[`gitlab-org/gitlab-workhorse`](https://gitlab.com/gitlab-org/gitlab-workhorse/tree/master), but that repository is no longer used for development. ## Documentation Workhorse documentation is available in the [`doc` folder of this repository](doc/). -* Architectural overview - * [GitLab features that rely on Workhorse](doc/architecture/gitlab_features.md) - * [Websocket channel support](doc/architecture/channel.md) -* Operating Workhorse - * [Source installation](doc/operations/install.md) - * [Workhorse configuration](doc/operations/configuration.md) -* [Contributing](CONTRIBUTING.md) - * [Adding new features](doc/development/new_features.md) - * [Testing your code](doc/development/tests.md) +- Architectural overview + - [GitLab features that rely on Workhorse](doc/architecture/gitlab_features.md) + - [Websocket channel support](doc/architecture/channel.md) +- Operating Workhorse + - [Source installation](doc/operations/install.md) + - [Workhorse configuration](doc/operations/configuration.md) +- [Contributing](CONTRIBUTING.md) + - [Adding new features](doc/development/new_features.md) + - [Testing your code](doc/development/tests.md) ## License This code is distributed under the MIT license, see the [LICENSE](LICENSE) file. - diff --git a/workhorse/doc/architecture/channel.md b/workhorse/doc/architecture/channel.md index f7a72d0fd45..9d7dd524c2f 100644 --- a/workhorse/doc/architecture/channel.md +++ b/workhorse/doc/architecture/channel.md @@ -1,3 +1,9 @@ +--- +stage: Create +group: Source Code +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + # Websocket channel support In some cases, GitLab can provide in-browser terminal access to an @@ -22,7 +28,7 @@ responses. WebSocket URLs have schemes like `ws://` (unencrypted) or When requesting an upgrade to WebSocket, the browser sends a HTTP/1.1 request that looks like this: -``` +```plaintext GET /path.ws HTTP/1.1 Connection: upgrade Upgrade: websocket @@ -116,10 +122,10 @@ This returns a JSON response containing details of where the terminal can be found, and how to connect it. In particular, the following details are returned in case of success: -* WebSocket URL to **connect** to, e.g.: `wss://example.com/terminals/1.ws?tty=1` -* WebSocket subprotocols to support, e.g.: `["channel.k8s.io"]` -* Headers to send, e.g.: `Authorization: Token xxyyz..` -* Certificate authority to verify `wss` connections with (optional) +- WebSocket URL to **connect** to, e.g.: `wss://example.com/terminals/1.ws?tty=1` +- WebSocket subprotocols to support, e.g.: `["channel.k8s.io"]` +- Headers to send, e.g.: `Authorization: Token xxyyz..` +- Certificate authority to verify `wss` connections with (optional) Workhorse periodically re-checks this endpoint, and if it gets an error response, or the details of the terminal change, it will @@ -186,9 +192,8 @@ written to that fd. Also used by Kubernetes, this subprotocol defines a similar multiplexed channel to `channel.k8s.io`. The main differences are: -* `TextMessage` frames are valid, rather than `BinaryMessage` frames. -* The first byte of each `TextMessage` frame represents the file +- `TextMessage` frames are valid, rather than `BinaryMessage` frames. +- The first byte of each `TextMessage` frame represents the file descriptor as a numeric UTF-8 character, so the character `U+0030`, or "0", is fd 0, STDIN). -* The remaining bytes represent base64-encoded arbitrary data. - +- The remaining bytes represent base64-encoded arbitrary data. diff --git a/workhorse/doc/architecture/gitlab_features.md b/workhorse/doc/architecture/gitlab_features.md index 2d3ca78f5e8..cdec2a8f0c1 100644 --- a/workhorse/doc/architecture/gitlab_features.md +++ b/workhorse/doc/architecture/gitlab_features.md @@ -1,10 +1,17 @@ +--- +stage: Create +group: Source Code +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + # Features that rely on Workhorse Workhorse itself is not a feature, but there are several features in GitLab that would not work efficiently without Workhorse. To put the efficiency benefit in context, consider that in 2020Q3 on -GitLab.com [we see][thanos] Rails application threads using on average +GitLab.com [we see][https://thanos-query.ops.gitlab.net/graph?g0.range_input=1h&g0.max_source_resolution=0s&g0.expr=sum(ruby_process_resident_memory_bytes%7Bapp%3D%22webservice%22%2Cenv%3D%22gprd%22%2Crelease%3D%22gitlab%22%7D)%20%2F%20sum(puma_max_threads%7Bapp%3D%22webservice%22%2Cenv%3D%22gprd%22%2Crelease%3D%22gitlab%22%7D)&g0.tab=1&g1.range_input=1h&g1.max_source_resolution=0s&g1.expr=sum(go_memstats_sys_bytes%7Bapp%3D%22webservice%22%2Cenv%3D%22gprd%22%2Crelease%3D%22gitlab%22%7D)%2Fsum(go_goroutines%7Bapp%3D%22webservice%22%2Cenv%3D%22gprd%22%2Crelease%3D%22gitlab%22%7D)&g1.tab=1] +Rails application threads using on average about 200MB of RSS vs about 200KB for Workhorse goroutines. Examples of features that rely on Workhorse: @@ -63,7 +70,4 @@ memory than it costs to have Workhorse look after it. - Workhorse does not clean up idle client connections. - We assume that all requests to Rails pass through Workhorse. -For more information see ['A brief history of GitLab Workhorse'][brief-history-blog]. - -[thanos]: https://thanos-query.ops.gitlab.net/graph?g0.range_input=1h&g0.max_source_resolution=0s&g0.expr=sum(ruby_process_resident_memory_bytes%7Bapp%3D%22webservice%22%2Cenv%3D%22gprd%22%2Crelease%3D%22gitlab%22%7D)%20%2F%20sum(puma_max_threads%7Bapp%3D%22webservice%22%2Cenv%3D%22gprd%22%2Crelease%3D%22gitlab%22%7D)&g0.tab=1&g1.range_input=1h&g1.max_source_resolution=0s&g1.expr=sum(go_memstats_sys_bytes%7Bapp%3D%22webservice%22%2Cenv%3D%22gprd%22%2Crelease%3D%22gitlab%22%7D)%2Fsum(go_goroutines%7Bapp%3D%22webservice%22%2Cenv%3D%22gprd%22%2Crelease%3D%22gitlab%22%7D)&g1.tab=1 -[brief-history-blog]: https://about.gitlab.com/2016/04/12/a-brief-history-of-gitlab-workhorse/ +For more information see ['A brief history of GitLab Workhorse'](https://about.gitlab.com/2016/04/12/a-brief-history-of-gitlab-workhorse/). diff --git a/workhorse/doc/development/new_features.md b/workhorse/doc/development/new_features.md index e8bbd345a40..97128d4e6f2 100644 --- a/workhorse/doc/development/new_features.md +++ b/workhorse/doc/development/new_features.md @@ -1,4 +1,10 @@ -## Adding new features to Workhorse +--- +stage: Create +group: Source Code +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Adding new features to Workhorse GitLab Workhorse is a smart reverse proxy for GitLab. It handles "long" HTTP requests such as file downloads, file uploads, Git @@ -16,14 +22,14 @@ We suggest to follow this route only if absolutely necessary and no other option Splitting a feature between the Rails code-base and Workhorse is deliberately choosing to introduce technical debt. It adds complexity to the system and coupling between the two components. -* Building features using Workhorse has a considerable complexity cost, so you should prefer designs based on Rails requests and Sidekiq jobs. -* Even when using Rails+Sidekiq is "more work" than using Rails+Workhorse, Rails+Sidekiq is easier to maintain in the long term because Workhorse is unique to GitLab while Rails+Sidekiq is an industry standard. -* For "global" behaviors around web requests consider using a Rack middleware instead of Workhorse. -* Generally speaking, we should only use Rails+Workhorse if the HTTP client expects behavior that is not reasonable to implement in Rails, like "long" requests. +- Building features using Workhorse has a considerable complexity cost, so you should prefer designs based on Rails requests and Sidekiq jobs. +- Even when using Rails+Sidekiq is "more work" than using Rails+Workhorse, Rails+Sidekiq is easier to maintain in the long term because Workhorse is unique to GitLab while Rails+Sidekiq is an industry standard. +- For "global" behaviors around web requests consider using a Rack middleware instead of Workhorse. +- Generally speaking, we should only use Rails+Workhorse if the HTTP client expects behavior that is not reasonable to implement in Rails, like "long" requests. ## What is a "long" request? -There is one order of magnitude between Workhorse and puma RAM usage. Having connection open for a period longer than milliseconds is a problem because of the amount of RAM it monopolizes once it reaches the Ruby on Rails controller. +There is one order of magnitude between Workhorse and Puma RAM usage. Having connection open for a period longer than milliseconds is a problem because of the amount of RAM it monopolizes once it reaches the Ruby on Rails controller. So far we identified two classes of "long" requests: data transfers and HTTP long polling. @@ -37,5 +43,4 @@ You can watch the recording for more details on the history of Workhorse and the [Uploads development documentation]( https://docs.gitlab.com/ee/development/uploads.html) contains the most common use-cases for adding a new type of upload and may answer all of your questions. -If you still think we should add a new feature to Workhorse, please open an issue explaining **what you want to implement** and **why it can't be implemented in our ruby code-base**. Workhorse maintainers will be happy to help you assessing the situation. - +If you still think we should add a new feature to Workhorse, please open an issue explaining **what you want to implement** and **why it can't be implemented in our Ruby code-base**. Workhorse maintainers will be happy to help you assessing the situation. diff --git a/workhorse/doc/development/tests.md b/workhorse/doc/development/tests.md index 82f74e8656b..032730fab45 100644 --- a/workhorse/doc/development/tests.md +++ b/workhorse/doc/development/tests.md @@ -1,8 +1,14 @@ +--- +stage: Create +group: Source Code +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + # Testing your code Run the tests with: -``` +```plaintext make clean test ``` diff --git a/workhorse/doc/operations/configuration.md b/workhorse/doc/operations/configuration.md index 8694cf1bd82..f715cf442eb 100644 --- a/workhorse/doc/operations/configuration.md +++ b/workhorse/doc/operations/configuration.md @@ -1,3 +1,9 @@ +--- +stage: Create +group: Source Code +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + # Workhorse configuration For historical reasons Workhorse uses both command line flags, a configuration file and environment variables. @@ -6,7 +12,7 @@ All new configuration options that get added to Workhorse should go into the con ## CLI options -``` +```plaintext gitlab-workhorse [OPTIONS] Options: @@ -60,10 +66,10 @@ HTTP. GitLab Workhorse can listen on either a TCP or a Unix domain socket. It can also open a second listening TCP listening socket with the Go -[net/http/pprof profiler server](http://golang.org/pkg/net/http/pprof/). +[`net/http/pprof` profiler server](http://golang.org/pkg/net/http/pprof/). -GitLab Workhorse can listen on redis events (currently only builds/register -for runners). This requires you to pass a valid TOML config file via +GitLab Workhorse can listen on Redis events (currently only builds/register +for runners). This requires you to pass a valid TOML configuration file via `-config` flag. For regular setups it only requires the following (replacing the string with the actual socket) @@ -73,19 +79,19 @@ with the actual socket) GitLab Workhorse integrates with Redis to do long polling for CI build requests. This is configured via two things: -- Redis settings in the TOML config file -- The `-apiCiLongPollingDuration` command line flag to control polling - behavior for CI build requests +- Redis settings in the TOML configuration file +- The `-apiCiLongPollingDuration` command line flag to control polling + behavior for CI build requests -It is OK to enable Redis in the config file but to leave CI polling +It is OK to enable Redis in the configuration file but to leave CI polling disabled; this just results in an idle Redis pubsub connection. The opposite is not possible: CI long polling requires a correct Redis configuration. -Below we discuss the options for the `[redis]` section in the config +Below we discuss the options for the `[redis]` section in the configuration file. -``` +```plaintext [redis] URL = "unix:///var/run/gitlab/redis.sock" Password = "my_awesome_password" @@ -95,12 +101,15 @@ SentinelMaster = "mymaster" - `URL` takes a string in the format `unix://path/to/redis.sock` or `tcp://host:port`. -- `Password` is only required if your redis instance is password-protected +- `Password` is only required if your Redis instance is password-protected - `Sentinel` is used if you are using Sentinel. - *NOTE* that if both `Sentinel` and `URL` are given, only `Sentinel` will be used + + NOTE: + If both `Sentinel` and `URL` are given, only `Sentinel` will be used. Optional fields are as follows: -``` + +```plaintext [redis] DB = 0 MaxIdle = 1 @@ -108,7 +117,7 @@ MaxActive = 1 ``` - `DB` is the Database to connect to. Defaults to `0` -- `MaxIdle` is how many idle connections can be in the redis-pool at once. Defaults to 1 +- `MaxIdle` is how many idle connections can be in the Redis pool at once. Defaults to 1 - `MaxActive` is how many connections the pool can keep. Defaults to 1 ## Relative URL support @@ -117,7 +126,7 @@ If you are mounting GitLab at a relative URL, e.g. `example.com/gitlab`, then you should also use this relative URL in the `authBackend` setting: -``` +```plaintext gitlab-workhorse -authBackend http://localhost:8080/gitlab ``` @@ -151,7 +160,7 @@ development. Omnibus (`/etc/gitlab/gitlab.rb`): -``` +```ruby gitlab_workhorse['env'] = { 'GITLAB_WORKHORSE_SENTRY_DSN' => 'https://foobar' 'GITLAB_WORKHORSE_SENTRY_ENVIRONMENT' => 'production' @@ -160,16 +169,16 @@ gitlab_workhorse['env'] = { Source installations (`/etc/default/gitlab`): -``` +```plaintext export GITLAB_WORKHORSE_SENTRY_DSN='https://foobar' export GITLAB_WORKHORSE_SENTRY_ENVIRONMENT='production' ``` ## Distributed Tracing -Workhorse supports distributed tracing through [LabKit][] using [OpenTracing APIs](https://opentracing.io). +Workhorse supports distributed tracing through [LabKit](https://gitlab.com/gitlab-org/labkit/) using [OpenTracing APIs](https://opentracing.io). -By default, no tracing implementation is linked into the binary, but different OpenTracing providers can be linked in using [build tags][build-tags]/[build constraints][build-tags]. This can be done by setting the `BUILD_TAGS` make variable. +By default, no tracing implementation is linked into the binary, but different OpenTracing providers can be linked in using [build tags](https://golang.org/pkg/go/build/#hdr-Build_Constraints) or build constraints. This can be done by setting the `BUILD_TAGS` make variable. For more details of the supported providers, see LabKit, but as an example, for Jaeger tracing support, include the tags: `BUILD_TAGS="tracer_static tracer_static_jaeger"`. @@ -187,9 +196,9 @@ GITLAB_TRACING=opentracing://jaeger ./gitlab-workhorse ## Continuous Profiling -Workhorse supports continuous profiling through [LabKit][] using [Stackdriver Profiler](https://cloud.google.com/profiler). +Workhorse supports continuous profiling through [LabKit](https://gitlab.com/gitlab-org/labkit/) using [Stackdriver Profiler](https://cloud.google.com/profiler). -By default, the Stackdriver Profiler implementation is linked in the binary using [build tags][build-tags], though it's not +By default, the Stackdriver Profiler implementation is linked in the binary using [build tags](https://golang.org/pkg/go/build/#hdr-Build_Constraints), though it's not required and can be skipped. For example: @@ -207,7 +216,4 @@ For example: GITLAB_CONTINUOUS_PROFILING="stackdriver?service=workhorse&service_version=1.0.1&project_id=test-123 ./gitlab-workhorse" ``` -More information about see the [LabKit monitoring docs](https://gitlab.com/gitlab-org/labkit/-/blob/master/monitoring/doc.go). - -[LabKit]: https://gitlab.com/gitlab-org/labkit/ -[build-tags]: https://golang.org/pkg/go/build/#hdr-Build_Constraints +More information about see the [LabKit monitoring documentation](https://gitlab.com/gitlab-org/labkit/-/blob/master/monitoring/doc.go). diff --git a/workhorse/doc/operations/install.md b/workhorse/doc/operations/install.md index 3bee13e2683..088bd26c877 100644 --- a/workhorse/doc/operations/install.md +++ b/workhorse/doc/operations/install.md @@ -6,13 +6,13 @@ Make](https://www.gnu.org/software/make/). To install into `/usr/local/bin` run `make install`. -``` +```plaintext make install ``` To install into `/foo/bin` set the PREFIX variable. -``` +```plaintext make install PREFIX=/foo ``` @@ -26,19 +26,19 @@ On some operating systems, such as FreeBSD, you may have to use ### Exiftool -Workhorse uses [exiftool](https://www.sno.phy.queensu.ca/~phil/exiftool/) for +Workhorse uses [Exiftool](https://www.sno.phy.queensu.ca/~phil/exiftool/) for removing EXIF data (which may contain sensitive information) from uploaded images. If you installed GitLab: -- Using the Omnibus package, you're all set. - *NOTE* that if you are using CentOS Minimal, you may need to install `perl` - package: `yum install perl` -- From source, make sure `exiftool` is installed: +- Using the Omnibus package, you're all set. + *NOTE* that if you are using CentOS Minimal, you may need to install `perl` + package: `yum install perl` +- From source, make sure `exiftool` is installed: - ```sh - # Debian/Ubuntu - sudo apt-get install libimage-exiftool-perl + ```shell + # Debian/Ubuntu + sudo apt-get install libimage-exiftool-perl - # RHEL/CentOS - sudo yum install perl-Image-ExifTool - ``` + # RHEL/CentOS + sudo yum install perl-Image-ExifTool + ``` |