diff options
73 files changed, 528 insertions, 233 deletions
diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index c6a89ddfa9c..b3bba2fdc36 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -27,6 +27,7 @@ /doc/api/graphql/ @msedlakjakubowski @kpaizee /doc/api/graphql/reference/ @kpaizee /doc/api/group_activity_analytics.md @fneill +/doc/api/vulnerabilities.md @fneill /doc/ci/ @marcel.amirault @sselhorn /doc/ci/environments/ @rdickenson /doc/ci/services/ @sselhorn @@ -49,6 +50,11 @@ /doc/user/application_security/ @rdickenson /doc/user/application_security/container_scanning/ @ngaskill /doc/user/application_security/cluster_image_scanning/ @ngaskill +/doc/user/application_security/cve_id_request.md @fneill +/doc/user/application_security/security_dashboard @fneill +/doc/user/application_security/vulnerabilities @fneill +/doc/user/application_security/vulnerability_management @fneill +/doc/user/application_security/vulnerability_report @fneill /doc/user/clusters/ @marcia /doc/user/compliance/ @rdickenson @eread /doc/user/group/ @msedlakjakubowski diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 7b961e7ee6b..6c280736d49 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -2599,7 +2599,6 @@ Style/OpenStructUse: - 'spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb' - 'spec/lib/gitlab/legacy_github_import/project_creator_spec.rb' - 'spec/lib/gitlab/quick_actions/command_definition_spec.rb' - - 'spec/lib/gitlab/quick_actions/dsl_spec.rb' - 'spec/lib/gitlab/relative_positioning/range_spec.rb' - 'spec/models/design_management/design_action_spec.rb' - 'spec/models/design_management/design_at_version_spec.rb' diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 397a7ea2e1d..ea65aa7b7ce 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -d69b465fdff3c04f8e3f395aed34aaa59f23fe76 +b6dda5d1f7a7e05c34ed0f72f161a46aee536d75 diff --git a/app/assets/javascripts/experimentation/utils.js b/app/assets/javascripts/experimentation/utils.js index dcb6a8e20a3..1dbdf3bf17d 100644 --- a/app/assets/javascripts/experimentation/utils.js +++ b/app/assets/javascripts/experimentation/utils.js @@ -1,5 +1,5 @@ // This file only applies to use of experiments through https://gitlab.com/gitlab-org/gitlab-experiment -import { get } from 'lodash'; +import { get, mapValues, pick } from 'lodash'; import { DEFAULT_VARIANT, CANDIDATE_VARIANT, TRACKING_CONTEXT_SCHEMA } from './constants'; function getExperimentsData() { @@ -8,19 +8,18 @@ function getExperimentsData() { // Pull from preferred window.gl.experiments const experimentsFromGl = get(window, ['gl', 'experiments'], {}); - return { ...experimentsFromGon, ...experimentsFromGl }; -} - -function convertExperimentDataToExperimentContext(experimentData) { - // Bandaid to allow-list only the properties which the current gitlab_experiment context schema suppports. + // Bandaid to allow-list only the properties which the current gitlab_experiment + // context schema suppports, since we most often use this data to create that + // Snowplow context. // See TRACKING_CONTEXT_SCHEMA for current version (1-0-0) // https://gitlab.com/gitlab-org/iglu/-/blob/master/public/schemas/com.gitlab/gitlab_experiment/jsonschema/1-0-0 - const { experiment: experimentName, key, variant, migration_keys } = experimentData; + return mapValues({ ...experimentsFromGon, ...experimentsFromGl }, (xp) => { + return pick(xp, ['experiment', 'key', 'variant', 'migration_keys']); + }); +} - return { - schema: TRACKING_CONTEXT_SCHEMA, - data: { experiment: experimentName, key, variant, migration_keys }, - }; +function createGitlabExperimentContext(experimentData) { + return { schema: TRACKING_CONTEXT_SCHEMA, data: experimentData }; } export function getExperimentData(experimentName) { @@ -28,7 +27,7 @@ export function getExperimentData(experimentName) { } export function getAllExperimentContexts() { - return Object.values(getExperimentsData()).map(convertExperimentDataToExperimentContext); + return Object.values(getExperimentsData()).map(createGitlabExperimentContext); } export function isExperimentVariant(experimentName, variantName) { diff --git a/app/assets/javascripts/notes/components/discussion_filter.vue b/app/assets/javascripts/notes/components/discussion_filter.vue index 88f053aed67..102afaf308f 100644 --- a/app/assets/javascripts/notes/components/discussion_filter.vue +++ b/app/assets/javascripts/notes/components/discussion_filter.vue @@ -39,7 +39,7 @@ export default { }; }, computed: { - ...mapGetters(['getNotesDataByProp', 'timelineEnabled']), + ...mapGetters(['getNotesDataByProp', 'timelineEnabled', 'isLoading']), currentFilter() { if (!this.currentValue) return this.filters[0]; return this.filters.find((filter) => filter.value === this.currentValue); @@ -119,6 +119,7 @@ export default { class="gl-mr-3 full-width-mobile discussion-filter-container js-discussion-filter-container" data-qa-selector="discussion_filter_dropdown" :text="currentFilter.title" + :disabled="isLoading" > <div v-for="filter in filters" :key="filter.value" class="dropdown-item-wrapper"> <gl-dropdown-item diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index c862a29ad9c..50b05ea9d69 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -601,7 +601,8 @@ export const setLoadingState = ({ commit }, data) => { commit(types.SET_NOTES_LOADING_STATE, data); }; -export const filterDiscussion = ({ dispatch }, { path, filter, persistFilter }) => { +export const filterDiscussion = ({ commit, dispatch }, { path, filter, persistFilter }) => { + commit(types.CLEAR_DISCUSSIONS); dispatch('setLoadingState', true); dispatch('fetchDiscussions', { path, filter, persistFilter }) .then(() => { diff --git a/app/assets/javascripts/notes/stores/mutation_types.js b/app/assets/javascripts/notes/stores/mutation_types.js index fcd2846ff0d..ebda08a3d62 100644 --- a/app/assets/javascripts/notes/stores/mutation_types.js +++ b/app/assets/javascripts/notes/stores/mutation_types.js @@ -1,6 +1,7 @@ export const ADD_NEW_NOTE = 'ADD_NEW_NOTE'; export const ADD_NEW_REPLY_TO_DISCUSSION = 'ADD_NEW_REPLY_TO_DISCUSSION'; export const ADD_OR_UPDATE_DISCUSSIONS = 'ADD_OR_UPDATE_DISCUSSIONS'; +export const CLEAR_DISCUSSIONS = 'CLEAR_DISCUSSIONS'; export const DELETE_NOTE = 'DELETE_NOTE'; export const REMOVE_PLACEHOLDER_NOTES = 'REMOVE_PLACEHOLDER_NOTES'; export const SET_NOTES_DATA = 'SET_NOTES_DATA'; diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index 1a99750ddb3..ba19ecd0c04 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -129,6 +129,10 @@ export default { Object.assign(state, { userData: data }); }, + [types.CLEAR_DISCUSSIONS](state) { + state.discussions = []; + }, + [types.ADD_OR_UPDATE_DISCUSSIONS](state, discussionsData) { discussionsData.forEach((d) => { const discussion = { ...d }; diff --git a/app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue b/app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue index 6f19a9f4379..0d975ffbb26 100644 --- a/app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue +++ b/app/assets/javascripts/pages/shared/wikis/components/wiki_form.vue @@ -15,6 +15,7 @@ import { setUrlFragment } from '~/lib/utils/url_utility'; import { __, s__, sprintf } from '~/locale'; import Tracking from '~/tracking'; import MarkdownField from '~/vue_shared/components/markdown/field.vue'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { CONTENT_EDITOR_LOADED_ACTION, SAVED_USING_CONTENT_EDITOR_ACTION, @@ -104,6 +105,8 @@ export default { newPage: s__('WikiPage|Create page'), }, cancel: s__('WikiPage|Cancel'), + editSourceButtonText: s__('WikiPage|Edit source'), + editRichTextButtonText: s__('WikiPage|Edit rich text'), }, contentEditorFeedbackIssue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332629', components: { @@ -123,7 +126,7 @@ export default { directives: { GlModalDirective, }, - mixins: [trackingMixin], + mixins: [trackingMixin, glFeatureFlagMixin()], inject: ['formatOptions', 'pageInfo'], data() { return { @@ -131,7 +134,6 @@ export default { format: this.pageInfo.format || 'markdown', content: this.pageInfo.content || '', isContentEditorAlertDismissed: false, - isContentEditorLoading: true, useContentEditor: false, commitMessage: '', isDirty: false, @@ -164,6 +166,11 @@ export default { linkExample() { return MARKDOWN_LINK_TEXT[this.format]; }, + toggleEditingModeButtonText() { + return this.isContentEditorActive + ? this.$options.i18n.editSourceButtonText + : this.$options.i18n.editRichTextButtonText; + }, submitButtonText() { return this.pageInfo.persisted ? this.$options.i18n.submitButton.existingPage @@ -188,7 +195,23 @@ export default { return this.format === 'markdown'; }, showContentEditorAlert() { - return this.isMarkdownFormat && !this.useContentEditor && !this.isContentEditorAlertDismissed; + return ( + !this.glFeatures.wikiSwitchBetweenContentEditorRawMarkdown && + this.isMarkdownFormat && + !this.useContentEditor && + !this.isContentEditorAlertDismissed + ); + }, + showSwitchEditingModeButton() { + return this.glFeatures.wikiSwitchBetweenContentEditorRawMarkdown && this.isMarkdownFormat; + }, + displayWikiSpecificMarkdownHelp() { + return !this.isContentEditorActive; + }, + displaySwitchBackToClassicEditorMessage() { + return ( + !this.glFeatures.wikiSwitchBetweenContentEditorRawMarkdown && this.isContentEditorActive + ); }, disableSubmitButton() { return this.noContent || !this.title || this.contentEditorRenderFailed; @@ -212,6 +235,14 @@ export default { .then(({ data }) => data.body); }, + toggleEditingMode() { + if (this.useContentEditor) { + this.content = this.contentEditor.getSerializedContent(); + } + + this.useContentEditor = !this.useContentEditor; + }, + async handleFormSubmit(e) { e.preventDefault(); @@ -405,6 +436,17 @@ export default { }}</label> </div> <div class="col-sm-10"> + <div + v-if="showSwitchEditingModeButton" + class="gl-display-flex gl-justify-content-end gl-mb-3" + > + <gl-button + data-testid="toggle-editing-mode-button" + variant="link" + @click="toggleEditingMode" + >{{ toggleEditingModeButtonText }}</gl-button + > + </div> <gl-alert v-if="showContentEditorAlert" class="gl-mb-6" @@ -498,7 +540,7 @@ export default { <div class="error-alert"></div> <div class="form-text gl-text-gray-600"> - <gl-sprintf v-if="!isContentEditorActive" :message="$options.i18n.linksHelpText"> + <gl-sprintf v-if="displayWikiSpecificMarkdownHelp" :message="$options.i18n.linksHelpText"> <template #linkExample ><code>{{ linkExample }}</code></template > @@ -513,7 +555,7 @@ export default { ></template > </gl-sprintf> - <span v-else> + <span v-if="displaySwitchBackToClassicEditorMessage"> {{ $options.i18n.contentEditor.switchToOldEditor.helpText }} <gl-button variant="link" @click="confirmSwitchToOldEditor">{{ $options.i18n.contentEditor.switchToOldEditor.label diff --git a/app/controllers/concerns/wiki_actions.rb b/app/controllers/concerns/wiki_actions.rb index 848b7ee44c5..ba9eab47803 100644 --- a/app/controllers/concerns/wiki_actions.rb +++ b/app/controllers/concerns/wiki_actions.rb @@ -21,6 +21,10 @@ module WikiActions before_action :load_sidebar, except: [:pages] before_action :set_content_class + before_action do + push_frontend_feature_flag(:wiki_switch_between_content_editor_raw_markdown, @group, default_enabled: :yaml) + end + before_action only: [:show, :edit, :update] do @valid_encoding = valid_encoding? end diff --git a/app/controllers/projects/prometheus/alerts_controller.rb b/app/controllers/projects/prometheus/alerts_controller.rb index 312919831d4..7aebff13278 100644 --- a/app/controllers/projects/prometheus/alerts_controller.rb +++ b/app/controllers/projects/prometheus/alerts_controller.rb @@ -82,17 +82,17 @@ module Projects def create_service Projects::Prometheus::Alerts::CreateService - .new(project, current_user, alerts_params) + .new(project: project, current_user: current_user, params: alerts_params) end def update_service Projects::Prometheus::Alerts::UpdateService - .new(project, current_user, alerts_params) + .new(project: project, current_user: current_user, params: alerts_params) end def destroy_service Projects::Prometheus::Alerts::DestroyService - .new(project, current_user, nil) + .new(project: project, current_user: current_user, params: nil) end def schedule_prometheus_update! diff --git a/app/services/projects/prometheus/alerts/create_service.rb b/app/services/projects/prometheus/alerts/create_service.rb index dc0cacf49f3..0d7d8ab1a62 100644 --- a/app/services/projects/prometheus/alerts/create_service.rb +++ b/app/services/projects/prometheus/alerts/create_service.rb @@ -3,7 +3,7 @@ module Projects module Prometheus module Alerts - class CreateService < BaseService + class CreateService < BaseProjectService include AlertParams def execute diff --git a/app/services/projects/prometheus/alerts/destroy_service.rb b/app/services/projects/prometheus/alerts/destroy_service.rb index 14e88a2e356..243b12eb654 100644 --- a/app/services/projects/prometheus/alerts/destroy_service.rb +++ b/app/services/projects/prometheus/alerts/destroy_service.rb @@ -3,7 +3,7 @@ module Projects module Prometheus module Alerts - class DestroyService < BaseService + class DestroyService < BaseProjectService def execute(alert) alert.destroy end diff --git a/app/services/projects/prometheus/alerts/update_service.rb b/app/services/projects/prometheus/alerts/update_service.rb index a0c8a5ccc2d..1802f35dae9 100644 --- a/app/services/projects/prometheus/alerts/update_service.rb +++ b/app/services/projects/prometheus/alerts/update_service.rb @@ -3,7 +3,7 @@ module Projects module Prometheus module Alerts - class UpdateService < BaseService + class UpdateService < BaseProjectService include AlertParams def execute(alert) diff --git a/app/validators/json_schemas/error_tracking_event_payload.json b/app/validators/json_schemas/error_tracking_event_payload.json index 73ff71043ce..1497a05a68f 100644 --- a/app/validators/json_schemas/error_tracking_event_payload.json +++ b/app/validators/json_schemas/error_tracking_event_payload.json @@ -57,7 +57,7 @@ "type": "array" }, "context_line": { - "type": "string" + "type": ["string", "null"] }, "post_context": { "type": "array" diff --git a/app/workers/issuable_export_csv_worker.rb b/app/workers/issuable_export_csv_worker.rb index 9d543a21dc3..ffa0ed68fc7 100644 --- a/app/workers/issuable_export_csv_worker.rb +++ b/app/workers/issuable_export_csv_worker.rb @@ -41,7 +41,7 @@ class IssuableExportCsvWorker # rubocop:disable Scalability/IdempotentWorker def parse_params(params, project_id) params - .symbolize_keys + .with_indifferent_access .except(:sort) .merge(project_id: project_id) end diff --git a/config/feature_flags/development/wiki_switch_between_content_editor_raw_markdown.yml b/config/feature_flags/development/wiki_switch_between_content_editor_raw_markdown.yml new file mode 100644 index 00000000000..f499263acec --- /dev/null +++ b/config/feature_flags/development/wiki_switch_between_content_editor_raw_markdown.yml @@ -0,0 +1,8 @@ +--- +name: wiki_switch_between_content_editor_raw_markdown +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74457 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/345398 +milestone: '14.6' +type: development +group: group::editor +default_enabled: false diff --git a/config/initializers/7_prometheus_metrics.rb b/config/initializers/7_prometheus_metrics.rb index 28f3da9b3df..e44a170d768 100644 --- a/config/initializers/7_prometheus_metrics.rb +++ b/config/initializers/7_prometheus_metrics.rb @@ -28,11 +28,15 @@ Gitlab::Application.configure do |config| config.middleware.insert_after(Labkit::Middleware::Rack, Gitlab::Metrics::RequestsRackMiddleware) end -Sidekiq.configure_server do |config| - config.on(:startup) do - # Do not clean the metrics directory here - the supervisor script should - # have already taken care of that - Gitlab::Metrics::Exporter::SidekiqExporter.instance.start +if Gitlab::Runtime.sidekiq? && (!ENV['SIDEKIQ_WORKER_ID'] || ENV['SIDEKIQ_WORKER_ID'] == '0') + # The single worker outside of a sidekiq-cluster, or the first worker (sidekiq_0) + # in a cluster of processes, is responsible for serving health checks. + Sidekiq.configure_server do |config| + config.on(:startup) do + # Do not clean the metrics directory here - the supervisor script should + # have already taken care of that + Gitlab::Metrics::Exporter::SidekiqExporter.instance.start + end end end diff --git a/db/migrate/20211115145107_add_created_at_to_namespace_monthly_usages.rb b/db/migrate/20211115145107_add_created_at_to_namespace_monthly_usages.rb new file mode 100644 index 00000000000..e0e2bec72ad --- /dev/null +++ b/db/migrate/20211115145107_add_created_at_to_namespace_monthly_usages.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddCreatedAtToNamespaceMonthlyUsages < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + with_lock_retries do + add_column :ci_namespace_monthly_usages, :created_at, :datetime_with_timezone + end + end + + def down + with_lock_retries do + remove_column :ci_namespace_monthly_usages, :created_at + end + end +end diff --git a/db/migrate/20211115154103_add_created_at_to_project_monthly_usage.rb b/db/migrate/20211115154103_add_created_at_to_project_monthly_usage.rb new file mode 100644 index 00000000000..eb0cd448da3 --- /dev/null +++ b/db/migrate/20211115154103_add_created_at_to_project_monthly_usage.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddCreatedAtToProjectMonthlyUsage < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + with_lock_retries do + add_column :ci_project_monthly_usages, :created_at, :datetime_with_timezone + end + end + + def down + with_lock_retries do + remove_column :ci_project_monthly_usages, :created_at + end + end +end diff --git a/db/schema_migrations/20211115145107 b/db/schema_migrations/20211115145107 new file mode 100644 index 00000000000..25270a5c587 --- /dev/null +++ b/db/schema_migrations/20211115145107 @@ -0,0 +1 @@ +2b6bc8067402744b79eee06022cf3c91ba7ffd519df83aae4067600a6bbf43ad
\ No newline at end of file diff --git a/db/schema_migrations/20211115154103 b/db/schema_migrations/20211115154103 new file mode 100644 index 00000000000..2b721bc7b39 --- /dev/null +++ b/db/schema_migrations/20211115154103 @@ -0,0 +1 @@ +ad65e6deb885397dc91f33dc117a50e9a1b6d60f4caed8c5b77d474ec0340995
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 5373aa7a31e..b708e48a111 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11748,6 +11748,7 @@ CREATE TABLE ci_namespace_monthly_usages ( amount_used numeric(18,2) DEFAULT 0.0 NOT NULL, notification_level smallint DEFAULT 100 NOT NULL, shared_runners_duration integer DEFAULT 0 NOT NULL, + created_at timestamp with time zone, CONSTRAINT ci_namespace_monthly_usages_year_month_constraint CHECK ((date = date_trunc('month'::text, (date)::timestamp with time zone))) ); @@ -11993,6 +11994,7 @@ CREATE TABLE ci_project_monthly_usages ( date date NOT NULL, amount_used numeric(18,2) DEFAULT 0.0 NOT NULL, shared_runners_duration integer DEFAULT 0 NOT NULL, + created_at timestamp with time zone, CONSTRAINT ci_project_monthly_usages_year_month_constraint CHECK ((date = date_trunc('month'::text, (date)::timestamp with time zone))) ); diff --git a/doc/administration/geo/secondary_proxy/index.md b/doc/administration/geo/secondary_proxy/index.md index 2b8c0d1e6fa..b0d0861066a 100644 --- a/doc/administration/geo/secondary_proxy/index.md +++ b/doc/administration/geo/secondary_proxy/index.md @@ -7,11 +7,14 @@ type: howto # Geo proxying for secondary sites **(PREMIUM SELF)** -> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5914) in GitLab 14.4 [with a flag](../../feature_flags.md) named `geo_secondary_proxy`. Disabled by default. +> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5914) in GitLab 14.4 [with a flag](../../feature_flags.md) named `geo_secondary_proxy`. Disabled by default. +> - [Enabled by default for unified URLs](https://gitlab.com/gitlab-org/gitlab/-/issues/325732) in GitLab 14.5. +> - [Disabled by default for different URLs](https://gitlab.com/gitlab-org/gitlab/-/issues/325732) in GitLab 14.5 [with a flag](../../feature_flags.md) named `geo_secondary_proxy_separate_urls`. FLAG: -On self-managed GitLab, by default this feature is not available. See below to [Set up a unified URL for Geo sites](#set-up-a-unified-url-for-geo-sites). -The feature is not ready for production use. +On self-managed GitLab, this feature is only available by default for Geo sites using a unified URL. See below to +[set up a unified URL for Geo sites](#set-up-a-unified-url-for-geo-sites). +The feature is not ready for production use with separate URLs. Use Geo proxying to: @@ -65,8 +68,10 @@ a single URL used by all Geo sites, including the primary. In the Geo administration page of the **primary** site, edit each Geo secondary that is using the secondary proxying and set the `URL` field to the single URL. Make sure the primary site is also using this URL. + +## Disable Geo proxying -### Enable secondary proxying +You can disable the secondary proxying on each Geo site, separately, by following these steps: 1. SSH into each application node (serving user traffic directly) on your secondary Geo site and add the following environment variable: @@ -77,7 +82,7 @@ a single URL used by all Geo sites, including the primary. ```ruby gitlab_workhorse['env'] = { - "GEO_SECONDARY_PROXY" => "1" + "GEO_SECONDARY_PROXY" => "0" } ``` @@ -87,18 +92,34 @@ a single URL used by all Geo sites, including the primary. gitlab-ctl reconfigure ``` -1. SSH into one node running Rails on your primary Geo site and enable the Geo secondary proxy feature flag: - - ```shell - sudo gitlab-rails runner "Feature.enable(:geo_secondary_proxy)" - ``` - ## Enable Geo proxying with Separate URLs The ability to use proxying with separate URLs is still in development. You can follow the ["Geo secondary proxying with separate URLs" epic](https://gitlab.com/groups/gitlab-org/-/epics/6865) for progress. +To try out this feature, enable the `geo_secondary_proxy_separate_urls` feature flag. +SSH into one node running Rails on your primary Geo site and run: + +```shell +sudo gitlab-rails runner "Feature.enable(:geo_secondary_proxy_separate_urls)" +``` + +## Limitations + +The asynchronous Geo replication can cause unexpected issues when secondary proxying is used, for accelerated +data types that may be replicated to the Geo secondaries with a delay. + +For example, we found a potential issue where +[Replication lag introduces read-your-own-write inconsistencies](https://gitlab.com/gitlab-org/gitlab/-/issues/345267). +If the replication lag is high enough, this can result in Git reads receiving stale data when hitting a secondary. + +Non-Rails requests are not proxied, so other services may need to use a separate, non-unified URL to ensure requests +are always sent to the primary. These services include: + +- GitLab Container Registry - [can be configured to use a separate domain](../../packages/container_registry.md#configure-container-registry-under-its-own-domain). +- GitLab Pages - should always use a separate domain, as part of [the prerequisites for running GitLab Pages](../../pages/index.md#prerequisites). + ## Features accelerated by secondary Geo sites Most HTTP traffic sent to a secondary Geo site can be proxied to the primary Geo site. With this architecture, diff --git a/doc/administration/geo/setup/index.md b/doc/administration/geo/setup/index.md index 84dff69ebe7..7d365f73101 100644 --- a/doc/administration/geo/setup/index.md +++ b/doc/administration/geo/setup/index.md @@ -26,6 +26,7 @@ If you installed GitLab using the Omnibus packages (highly recommended): 1. [Configure GitLab](../replication/configuration.md) to set the **primary** and **secondary** site(s). 1. Optional: [Configure a secondary LDAP server](../../auth/ldap/index.md) for the **secondary** site(s). See [notes on LDAP](../index.md#ldap). 1. Follow the [Using a Geo Site](../replication/usage.md) guide. +1. [Configure Geo secondary proxying](../secondary_proxy/index.md) to use a single, unified URL for all Geo sites. This step is recommended to accelerate most read requests while transparently proxying writes to the primary Geo site. ## Post-installation documentation diff --git a/doc/api/packages/nuget.md b/doc/api/packages/nuget.md index aee3a4fe542..f25a3a25754 100644 --- a/doc/api/packages/nuget.md +++ b/doc/api/packages/nuget.md @@ -287,12 +287,13 @@ Example response: Returns metadata for a specific package version: ```plaintext -GET <route-prefix>/metadata/:package_name/index +GET <route-prefix>/metadata/:package_name/:package_version ``` -| Attribute | Type | Required | Description | -| -------------- | ------ | -------- | ----------- | -| `package_name` | string | yes | The name of the package. | +| Attribute | Type | Required | Description | +| ----------------- | ------ | -------- | ----------- | +| `package_name` | string | yes | The name of the package. | +| `package_version` | string | yes | The version of the package. | ```shell curl --user <username>:<personal_access_token> "https://gitlab.example.com/api/v4/projects/1/packages/nuget/metadata/MyNuGetPkg/1.3.0.17" diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md index 9340f89c502..e56c5830105 100644 --- a/doc/topics/autodevops/index.md +++ b/doc/topics/autodevops/index.md @@ -6,7 +6,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Auto DevOps **(FREE)** -> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/38366) in GitLab 11.0. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/38366) in GitLab 11.0. +> - Support for the GitLab Kubernetes Agent was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/299350) in GitLab 14.5. GitLab Auto DevOps is a collection of pre-configured features and integrations that work together to support your software delivery process. diff --git a/doc/user/application_security/vulnerability_report/index.md b/doc/user/application_security/vulnerability_report/index.md index d13647937a2..2d4fce084a0 100644 --- a/doc/user/application_security/vulnerability_report/index.md +++ b/doc/user/application_security/vulnerability_report/index.md @@ -7,8 +7,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Vulnerability Report **(ULTIMATE)** -The Vulnerability Report provides information about vulnerabilities from scans of the branch most -recently merged into the default branch. It is available for groups, projects, and the Security Center. +The Vulnerability Report provides information about vulnerabilities from scans of the default branch. It is available for groups, projects, and the Security Center. At all levels, the Vulnerability Report contains: diff --git a/doc/user/infrastructure/clusters/index.md b/doc/user/infrastructure/clusters/index.md index 06a77912876..fefdbfb0ea7 100644 --- a/doc/user/infrastructure/clusters/index.md +++ b/doc/user/infrastructure/clusters/index.md @@ -58,7 +58,6 @@ the Kubernetes Agent model on the [Agent's blueprint documentation](../../../arc - [Pod logs](../../project/clusters/kubernetes_pod_logs.md) - [Clusters health](manage/clusters_health.md) - [Crossplane integration](../../clusters/crossplane.md) -- [Auto Deploy](../../../topics/autodevops/stages.md#auto-deploy) - [Web terminals](../../../administration/integration/terminal.md) ### Cluster levels diff --git a/lib/gitlab/ci/templates/Jobs/SAST-IaC.latest.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/SAST-IaC.latest.gitlab-ci.yml index b763705857e..fa7f6ffa2b7 100644 --- a/lib/gitlab/ci/templates/Jobs/SAST-IaC.latest.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/SAST-IaC.latest.gitlab-ci.yml @@ -24,7 +24,7 @@ kics-iac-sast: image: name: "$SAST_ANALYZER_IMAGE" variables: - SAST_ANALYZER_IMAGE_TAG: 0 + SAST_ANALYZER_IMAGE_TAG: 1 SAST_ANALYZER_IMAGE: "$SECURE_ANALYZERS_PREFIX/kics:$SAST_ANALYZER_IMAGE_TAG" rules: - if: $SAST_DISABLED diff --git a/lib/gitlab/ci/templates/Scala.gitlab-ci.yml b/lib/gitlab/ci/templates/Scala.gitlab-ci.yml index ff8f9601189..de54d64dc42 100644 --- a/lib/gitlab/ci/templates/Scala.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Scala.gitlab-ci.yml @@ -13,8 +13,10 @@ before_script: - apt-get update -yqq - apt-get install apt-transport-https -yqq # Add keyserver for SBT - - echo "deb http://dl.bintray.com/sbt/debian /" | tee -a /etc/apt/sources.list.d/sbt.list - - apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv 2EE0EA64E40A89B84B2DF73499E82A75642AC823 + - echo "deb https://repo.scala-sbt.org/scalasbt/debian /" | tee -a /etc/apt/sources.list.d/sbt.list + - mkdir -p /root/.gnupg + - gpg --recv-keys --no-default-keyring --keyring gnupg-ring:/etc/apt/trusted.gpg.d/scalasbt-release.gpg --keyserver hkp://keyserver.ubuntu.com:80 2EE0EA64E40A89B84B2DF73499E82A75642AC823 + - chmod 644 /etc/apt/trusted.gpg.d/scalasbt-release.gpg # Install SBT - apt-get update -yqq - apt-get install sbt -yqq diff --git a/lib/gitlab/metrics/exporter/sidekiq_exporter.rb b/lib/gitlab/metrics/exporter/sidekiq_exporter.rb index 4d38d9e67bf..8d0f3192436 100644 --- a/lib/gitlab/metrics/exporter/sidekiq_exporter.rb +++ b/lib/gitlab/metrics/exporter/sidekiq_exporter.rb @@ -15,29 +15,6 @@ module Gitlab File::NULL end end - - private - - # Sidekiq Exporter does not work properly in sidekiq-cluster - # mode. It tries to start the service on the same port for - # each of the cluster workers, this results in failure - # due to duplicate binding. - # - # For now we ignore this error, as metrics are still "kind of" - # valid as they are rendered from shared directory. - # - # Issue: https://gitlab.com/gitlab-org/gitlab/issues/5714 - def start_working - super - rescue Errno::EADDRINUSE => e - Sidekiq.logger.error( - class: self.class.to_s, - message: 'Cannot start sidekiq_exporter', - 'exception.message' => e.message - ) - - false - end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6bf385b4a5c..b3d1ca1edaa 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -39140,6 +39140,12 @@ msgstr "" msgid "WikiPage|Create page" msgstr "" +msgid "WikiPage|Edit rich text" +msgstr "" + +msgid "WikiPage|Edit source" +msgstr "" + msgid "WikiPage|Format" msgstr "" diff --git a/spec/features/projects/issues/design_management/user_uploads_designs_spec.rb b/spec/features/projects/issues/design_management/user_uploads_designs_spec.rb index bfa7be5bb5c..e885c0c4413 100644 --- a/spec/features/projects/issues/design_management/user_uploads_designs_spec.rb +++ b/spec/features/projects/issues/design_management/user_uploads_designs_spec.rb @@ -27,10 +27,10 @@ RSpec.describe 'User uploads new design', :js do expect(page).to have_content('dk.png') end - upload_design(gif_fixture, count: 2) + upload_design([gif_fixture, logo_svg_fixture, big_image_fixture], count: 4) - expect(page).to have_selector('.js-design-list-item', count: 2) - expect(page.all('.js-design-list-item').map(&:text)).to eq(['dk.png', 'banana_sample.gif']) + expect(page).to have_selector('.js-design-list-item', count: 4) + expect(page.all('.js-design-list-item').map(&:text)).to eq(['dk.png', 'banana_sample.gif', 'logo_sample.svg', 'big-image.png']) end end @@ -50,8 +50,16 @@ RSpec.describe 'User uploads new design', :js do Rails.root.join('spec', 'fixtures', 'banana_sample.gif') end - def upload_design(fixture, count:) - attach_file(:upload_file, fixture, match: :first, make_visible: true) + def logo_svg_fixture + Rails.root.join('spec', 'fixtures', 'logo_sample.svg') + end + + def big_image_fixture + Rails.root.join('spec', 'fixtures', 'big-image.png') + end + + def upload_design(fixtures, count:) + attach_file(:upload_file, fixtures, multiple: true, match: :first, make_visible: true) wait_for('designs uploaded') do issue.reload.designs.count == count diff --git a/spec/fixtures/error_tracking/python_event_repl.json b/spec/fixtures/error_tracking/python_event_repl.json new file mode 100644 index 00000000000..bb2891c744a --- /dev/null +++ b/spec/fixtures/error_tracking/python_event_repl.json @@ -0,0 +1 @@ +{"breadcrumbs":{"values":[]},"contexts":{"runtime":{"build":"3.9.5 (default, May 12 2021, 15:36:59) \n[GCC 8.3.0]","name":"CPython","version":"3.9.5"}},"environment":"production","event_id":"","exception":{"values":[{"mechanism":null,"module":null,"stacktrace":{"frames":[{"abs_path":"/srv/autodevops/<stdin>","context_line":null,"filename":"<stdin>","function":"<module>","in_app":true,"lineno":2,"module":"__main__","post_context":[],"pre_context":[],"vars":{"__annotations__":{},"__builtins__":"<module 'builtins' (built-in)>","__doc__":"None","__loader__":"<class '_frozen_importlib.BuiltinImporter'>","__name__":"'__main__'","__package__":"None","__spec__":"None","capture_exception":"<function capture_exception at 0x7f5dbb3eb940>","e":"ZeroDivisionError('division by zero')","init":"<function _init at 0x7f5dbb3ea1f0>"}}]},"type":"ZeroDivisionError","value":"division by zero"}]},"extra":{"sys.argv":[""]},"level":"error","modules":{"appdirs":"1.4.4","apscheduler":"3.7.0","asgiref":"3.3.4","beautifulsoup4":"4.9.3","certifi":"2020.12.5","chardet":"4.0.0","django":"3.2.3","django-anymail":"1.3","django-environ":"0.4.5","django-livereload-server":"0.3.2","django-widget-tweaks":"1.4.8","fcache":"0.4.7","idna":"2.10","mmh3":"3.0.0","pip":"21.1.2","psycopg2-binary":"2.8.6","pytz":"2021.1","requests":"2.25.1","sentry-sdk":"1.5.0","setuptools":"57.0.0","six":"1.16.0","soupsieve":"2.2.1","sqlparse":"0.4.1","tornado":"6.1","tzlocal":"2.1","unleashclient":"4.2.0","urllib3":"1.26.4","uwsgi":"2.0.19.1","wheel":"0.36.2"},"platform":"python","sdk":{"integrations":["argv","atexit","dedupe","django","excepthook","logging","modules","stdlib","threading","tornado"],"name":"sentry.python","packages":[{"name":"pypi:sentry-sdk","version":"1.5.0"}],"version":"1.5.0"},"server_name":"","timestamp":"2021-11-17T14:46:20.898210Z"} diff --git a/spec/frontend/experimentation/utils_spec.js b/spec/frontend/experimentation/utils_spec.js index 923795ca3f3..6ed7aa6665c 100644 --- a/spec/frontend/experimentation/utils_spec.js +++ b/spec/frontend/experimentation/utils_spec.js @@ -51,6 +51,29 @@ describe('experiment Utilities', () => { expect(experimentUtils.getExperimentData(...input)).toEqual(output); }); }); + + it('only collects the data properties which are supported by the schema', () => { + origGl = window.gl; + window.gl.experiments = { + my_experiment: { + experiment: 'my_experiment', + variant: 'control', + key: 'randomization-unit-key', + migration_keys: 'migration_keys object', + excluded: false, + other: 'foobar', + }, + }; + + expect(experimentUtils.getExperimentData('my_experiment')).toEqual({ + experiment: 'my_experiment', + variant: 'control', + key: 'randomization-unit-key', + migration_keys: 'migration_keys object', + }); + + window.gl = origGl; + }); }); describe('getAllExperimentContexts', () => { @@ -72,19 +95,6 @@ describe('experiment Utilities', () => { it('returns an empty array if there are no experiments', () => { expect(experimentUtils.getAllExperimentContexts()).toEqual([]); }); - - it('only collects the data properties which are supported by the schema', () => { - origGl = window.gl; - window.gl.experiments = { - my_experiment: { experiment: 'my_experiment', variant: 'control', excluded: false }, - }; - - expect(experimentUtils.getAllExperimentContexts()).toEqual([ - { schema, data: { experiment: 'my_experiment', variant: 'control' } }, - ]); - - window.gl = origGl; - }); }); describe('isExperimentVariant', () => { diff --git a/spec/frontend/notes/components/discussion_filter_spec.js b/spec/frontend/notes/components/discussion_filter_spec.js index 6f62b8ba528..17998dfc9d5 100644 --- a/spec/frontend/notes/components/discussion_filter_spec.js +++ b/spec/frontend/notes/components/discussion_filter_spec.js @@ -1,3 +1,4 @@ +import { GlDropdown } from '@gitlab/ui'; import { createLocalVue, mount } from '@vue/test-utils'; import AxiosMockAdapter from 'axios-mock-adapter'; import Vuex from 'vuex'; @@ -88,6 +89,12 @@ describe('DiscussionFilter component', () => { ); }); + it('disables the dropdown when discussions are loading', () => { + store.state.isLoading = true; + + expect(wrapper.findComponent(GlDropdown).props('disabled')).toBe(true); + }); + it('updates to the selected item', () => { const filterItem = findFilter(DISCUSSION_FILTER_TYPES.ALL); diff --git a/spec/frontend/notes/stores/actions_spec.js b/spec/frontend/notes/stores/actions_spec.js index bbe074f0105..7424a87bc0f 100644 --- a/spec/frontend/notes/stores/actions_spec.js +++ b/spec/frontend/notes/stores/actions_spec.js @@ -1183,8 +1183,14 @@ describe('Actions Notes Store', () => { dispatch.mockReturnValue(new Promise(() => {})); }); + it('clears existing discussions', () => { + actions.filterDiscussion({ commit, dispatch }, { path, filter, persistFilter: false }); + + expect(commit.mock.calls).toEqual([[mutationTypes.CLEAR_DISCUSSIONS]]); + }); + it('fetches discussions with filter and persistFilter false', () => { - actions.filterDiscussion({ dispatch }, { path, filter, persistFilter: false }); + actions.filterDiscussion({ commit, dispatch }, { path, filter, persistFilter: false }); expect(dispatch.mock.calls).toEqual([ ['setLoadingState', true], @@ -1193,7 +1199,7 @@ describe('Actions Notes Store', () => { }); it('fetches discussions with filter and persistFilter true', () => { - actions.filterDiscussion({ dispatch }, { path, filter, persistFilter: true }); + actions.filterDiscussion({ commit, dispatch }, { path, filter, persistFilter: true }); expect(dispatch.mock.calls).toEqual([ ['setLoadingState', true], diff --git a/spec/frontend/notes/stores/mutation_spec.js b/spec/frontend/notes/stores/mutation_spec.js index c9e24039b64..da1547ab6e7 100644 --- a/spec/frontend/notes/stores/mutation_spec.js +++ b/spec/frontend/notes/stores/mutation_spec.js @@ -159,6 +159,18 @@ describe('Notes Store mutations', () => { }); }); + describe('CLEAR_DISCUSSIONS', () => { + it('should set discussions to an empty array', () => { + const state = { + discussions: [discussionMock], + }; + + mutations.CLEAR_DISCUSSIONS(state); + + expect(state.discussions).toEqual([]); + }); + }); + describe('ADD_OR_UPDATE_DISCUSSIONS', () => { it('should set the initial notes received', () => { const state = { diff --git a/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js b/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js index 9d510b3d231..b06e8d31643 100644 --- a/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js +++ b/spec/frontend/pages/shared/wikis/components/wiki_form_spec.js @@ -1,5 +1,6 @@ +import { nextTick } from 'vue'; import { GlLoadingIcon, GlModal } from '@gitlab/ui'; -import { mount } from '@vue/test-utils'; +import { mount, shallowMount } from '@vue/test-utils'; import axios from 'axios'; import MockAdapter from 'axios-mock-adapter'; import { mockTracking } from 'helpers/tracking_helper'; @@ -32,12 +33,15 @@ describe('WikiForm', () => { const findSubmitButton = () => wrapper.findByTestId('wiki-submit-button'); const findCancelButton = () => wrapper.findByRole('link', { name: 'Cancel' }); const findUseNewEditorButton = () => wrapper.findByRole('button', { name: 'Use the new editor' }); + const findToggleEditingModeButton = () => wrapper.findByTestId('toggle-editing-mode-button'); const findDismissContentEditorAlertButton = () => wrapper.findByRole('button', { name: 'Try this later' }); const findSwitchToOldEditorButton = () => wrapper.findByRole('button', { name: 'Switch me back to the classic editor.' }); const findTitleHelpLink = () => wrapper.findByRole('link', { name: 'More Information.' }); const findMarkdownHelpLink = () => wrapper.findByTestId('wiki-markdown-help-link'); + const findContentEditor = () => wrapper.findComponent(ContentEditor); + const findClassicEditor = () => wrapper.findComponent(MarkdownField); const setFormat = (value) => { const format = findFormat(); @@ -73,18 +77,24 @@ describe('WikiForm', () => { path: '/project/path/-/wikis/home', }; - function createWrapper(persisted = false, { pageInfo } = {}) { + const formatOptions = { + Markdown: 'markdown', + RDoc: 'rdoc', + AsciiDoc: 'asciidoc', + Org: 'org', + }; + + function createWrapper( + persisted = false, + { pageInfo, glFeatures = { wikiSwitchBetweenContentEditorRawMarkdown: false } } = {}, + ) { wrapper = extendedWrapper( mount( WikiForm, { provide: { - formatOptions: { - Markdown: 'markdown', - RDoc: 'rdoc', - AsciiDoc: 'asciidoc', - Org: 'org', - }, + formatOptions, + glFeatures, pageInfo: { ...(persisted ? pageInfoPersisted : pageInfoNew), ...pageInfo, @@ -96,6 +106,27 @@ describe('WikiForm', () => { ); } + const createShallowWrapper = ( + persisted = false, + { pageInfo, glFeatures = { wikiSwitchBetweenContentEditorRawMarkdown: false } } = {}, + ) => { + wrapper = extendedWrapper( + shallowMount(WikiForm, { + provide: { + formatOptions, + glFeatures, + pageInfo: { + ...(persisted ? pageInfoPersisted : pageInfoNew), + ...pageInfo, + }, + }, + stubs: { + MarkdownField, + }, + }), + ); + }; + beforeEach(() => { trackingSpy = mockTracking(undefined, null, jest.spyOn); mock = new MockAdapter(axios); @@ -193,14 +224,13 @@ describe('WikiForm', () => { }); describe('when wiki content is updated', () => { - beforeEach(() => { + beforeEach(async () => { createWrapper(true); const input = findContent(); input.setValue(' Lorem ipsum dolar sit! '); - input.element.dispatchEvent(new Event('input')); - return wrapper.vm.$nextTick(); + await input.trigger('input'); }); it('sets before unload warning', () => { @@ -279,6 +309,100 @@ describe('WikiForm', () => { ); }); + describe('when wikiSwitchBetweenContentEditorRawMarkdown feature flag is not enabled', () => { + beforeEach(() => { + createShallowWrapper(true, { + glFeatures: { wikiSwitchBetweenContentEditorRawMarkdown: false }, + }); + }); + + it('hides toggle editing mode button', () => { + expect(findToggleEditingModeButton().exists()).toBe(false); + }); + }); + + describe('when wikiSwitchBetweenContentEditorRawMarkdown feature flag is enabled', () => { + beforeEach(() => { + createShallowWrapper(true, { + glFeatures: { wikiSwitchBetweenContentEditorRawMarkdown: true }, + }); + }); + + it('hides gl-alert containing "use new editor" button', () => { + expect(findUseNewEditorButton().exists()).toBe(false); + }); + + it('displays toggle editing mode button', () => { + expect(findToggleEditingModeButton().exists()).toBe(true); + }); + + describe('when content editor is not active', () => { + it('displays "Edit rich text" label in the toggle editing mode button', () => { + expect(findToggleEditingModeButton().text()).toBe('Edit rich text'); + }); + + describe('when clicking the toggle editing mode button', () => { + beforeEach(() => { + findToggleEditingModeButton().vm.$emit('click'); + }); + + it('hides the classic editor', () => { + expect(findClassicEditor().exists()).toBe(false); + }); + + it('hides the content editor', () => { + expect(findContentEditor().exists()).toBe(true); + }); + }); + }); + + describe('when content editor is active', () => { + let mockContentEditor; + + beforeEach(() => { + mockContentEditor = { + getSerializedContent: jest.fn(), + setSerializedContent: jest.fn(), + }; + + findToggleEditingModeButton().vm.$emit('click'); + }); + + it('hides switch to old editor button', () => { + expect(findSwitchToOldEditorButton().exists()).toBe(false); + }); + + it('displays "Edit source" label in the toggle editing mode button', () => { + expect(findToggleEditingModeButton().text()).toBe('Edit source'); + }); + + describe('when clicking the toggle editing mode button', () => { + const contentEditorFakeSerializedContent = 'fake content'; + + beforeEach(() => { + mockContentEditor.getSerializedContent.mockReturnValueOnce( + contentEditorFakeSerializedContent, + ); + + findContentEditor().vm.$emit('initialized', mockContentEditor); + findToggleEditingModeButton().vm.$emit('click'); + }); + + it('hides the content editor', () => { + expect(findContentEditor().exists()).toBe(false); + }); + + it('displays the classic editor', () => { + expect(findClassicEditor().exists()).toBe(true); + }); + + it('updates the classic editor content field', () => { + expect(findContent().element.value).toBe(contentEditorFakeSerializedContent); + }); + }); + }); + }); + describe('wiki content editor', () => { beforeEach(() => { createWrapper(true); @@ -306,8 +430,8 @@ describe('WikiForm', () => { }); const assertOldEditorIsVisible = () => { - expect(wrapper.findComponent(ContentEditor).exists()).toBe(false); - expect(wrapper.findComponent(MarkdownField).exists()).toBe(true); + expect(findContentEditor().exists()).toBe(false); + expect(findClassicEditor().exists()).toBe(true); expect(findSubmitButton().props('disabled')).toBe(false); expect(wrapper.text()).not.toContain( @@ -376,10 +500,6 @@ describe('WikiForm', () => { findUseNewEditorButton().trigger('click'); }); - it('shows a loading indicator for the rich text editor', () => { - expect(wrapper.findComponent(GlLoadingIcon).exists()).toBe(true); - }); - it('shows a tip to send feedback', () => { expect(wrapper.text()).toContain('Tell us your experiences with the new Markdown editor'); }); @@ -412,16 +532,8 @@ describe('WikiForm', () => { }); describe('when wiki content is updated', () => { - beforeEach(async () => { - // wait for content editor to load - await waitForPromises(); - - wrapper.vm.contentEditor.tiptapEditor.commands.setContent( - '<p>hello __world__ from content editor</p>', - true, - ); - - return wrapper.vm.$nextTick(); + beforeEach(() => { + findContentEditor().vm.$emit('change', { empty: false }); }); it('sets before unload warning', () => { @@ -432,7 +544,7 @@ describe('WikiForm', () => { it('unsets before unload warning on form submit', async () => { triggerFormSubmit(); - await wrapper.vm.$nextTick(); + await nextTick(); const e = dispatchBeforeUnload(); expect(e.preventDefault).not.toHaveBeenCalled(); diff --git a/spec/lib/banzai/reference_parser/base_parser_spec.rb b/spec/lib/banzai/reference_parser/base_parser_spec.rb index 4701caa0667..d31ccccd6c3 100644 --- a/spec/lib/banzai/reference_parser/base_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/base_parser_spec.rb @@ -29,10 +29,10 @@ RSpec.describe Banzai::ReferenceParser::BaseParser do describe '#project_for_node' do it 'returns the Project for a node' do - document = instance_double('document', fragment?: false) - project = instance_double('project') - object = instance_double('object', project: project) - node = instance_double('node', document: document) + document = double('document', fragment?: false) + project = instance_double('Project') + object = double('object', project: project) + node = double('node', document: document) context.associate_document(document, object) diff --git a/spec/lib/banzai/render_context_spec.rb b/spec/lib/banzai/render_context_spec.rb index c4b609b936e..4b5c2c5a7df 100644 --- a/spec/lib/banzai/render_context_spec.rb +++ b/spec/lib/banzai/render_context_spec.rb @@ -7,15 +7,15 @@ RSpec.describe Banzai::RenderContext do describe '#project_for_node' do it 'returns the default project if no associated project was found' do - project = instance_double('project') + project = instance_double('Project') context = described_class.new(project) expect(context.project_for_node(document)).to eq(project) end it 'returns the associated project if one was associated explicitly' do - project = instance_double('project') - obj = instance_double('object', project: project) + project = instance_double('Project') + obj = double('object', project: project) context = described_class.new context.associate_document(document, obj) @@ -24,8 +24,8 @@ RSpec.describe Banzai::RenderContext do end it 'returns the project associated with a DocumentFragment when using a node' do - project = instance_double('project') - obj = instance_double('object', project: project) + project = instance_double('Project') + obj = double('object', project: project) context = described_class.new node = document.children.first diff --git a/spec/lib/error_tracking/collector/payload_validator_spec.rb b/spec/lib/error_tracking/collector/payload_validator_spec.rb index 852cf9eac6c..ab5ec448dff 100644 --- a/spec/lib/error_tracking/collector/payload_validator_spec.rb +++ b/spec/lib/error_tracking/collector/payload_validator_spec.rb @@ -3,16 +3,18 @@ require 'spec_helper' RSpec.describe ErrorTracking::Collector::PayloadValidator do + let(:validator) { described_class.new } + describe '#valid?' do RSpec.shared_examples 'valid payload' do - it 'returns true' do - expect(described_class.new.valid?(payload)).to be_truthy + specify do + expect(validator).to be_valid(payload) end end RSpec.shared_examples 'invalid payload' do - it 'returns false' do - expect(described_class.new.valid?(payload)).to be_falsey + specify do + expect(validator).not_to be_valid(payload) end end @@ -28,6 +30,12 @@ RSpec.describe ErrorTracking::Collector::PayloadValidator do it_behaves_like 'valid payload' end + context 'python payload in repl' do + let(:payload) { Gitlab::Json.parse(fixture_file('error_tracking/python_event_repl.json')) } + + it_behaves_like 'valid payload' + end + context 'browser payload' do let(:payload) { Gitlab::Json.parse(fixture_file('error_tracking/browser_event.json')) } diff --git a/spec/lib/gitlab/contributions_calendar_spec.rb b/spec/lib/gitlab/contributions_calendar_spec.rb index 384609c6664..0e9629b9d27 100644 --- a/spec/lib/gitlab/contributions_calendar_spec.rb +++ b/spec/lib/gitlab/contributions_calendar_spec.rb @@ -67,12 +67,12 @@ RSpec.describe Gitlab::ContributionsCalendar do context "when the user has opted-in for private contributions" do it "shows private and public events to all users" do - user.update_column(:include_private_contributions, true) + contributor.update_column(:include_private_contributions, true) create_event(private_project, today) create_event(public_project, today) - expect(calendar.activity_dates[today]).to eq(1) - expect(calendar(user).activity_dates[today]).to eq(1) + expect(calendar.activity_dates[today]).to eq(2) + expect(calendar(user).activity_dates[today]).to eq(2) expect(calendar(contributor).activity_dates[today]).to eq(2) end end diff --git a/spec/lib/gitlab/experimentation/controller_concern_spec.rb b/spec/lib/gitlab/experimentation/controller_concern_spec.rb index 1f7b7b90467..8a96771eeb8 100644 --- a/spec/lib/gitlab/experimentation/controller_concern_spec.rb +++ b/spec/lib/gitlab/experimentation/controller_concern_spec.rb @@ -97,7 +97,7 @@ RSpec.describe Gitlab::Experimentation::ControllerConcern, type: :controller do describe '#push_frontend_experiment' do it 'pushes an experiment to the frontend' do - gon = instance_double('gon') + gon = class_double('Gon') stub_experiment_for_subject(my_experiment: true) allow(controller).to receive(:gon).and_return(gon) diff --git a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb index 16cea1dc1a3..b2603e099e6 100644 --- a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb +++ b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb @@ -110,7 +110,7 @@ RSpec.describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do describe '#running_puma_with_multiple_threads?' do context 'when using Puma' do before do - stub_const('::Puma', class_double('Puma')) + stub_const('::Puma', double('puma constant')) allow(Gitlab::Runtime).to receive(:puma?).and_return(true) end diff --git a/spec/lib/gitlab/gon_helper_spec.rb b/spec/lib/gitlab/gon_helper_spec.rb index 3d3f381b6d2..b8ed4cf608d 100644 --- a/spec/lib/gitlab/gon_helper_spec.rb +++ b/spec/lib/gitlab/gon_helper_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Gitlab::GonHelper do end it 'pushes a feature flag to the frontend' do - gon = instance_double('gon') + gon = class_double('Gon') thing = stub_feature_flag_gate('thing') stub_feature_flags(my_feature_flag: thing) diff --git a/spec/lib/gitlab/lets_encrypt/client_spec.rb b/spec/lib/gitlab/lets_encrypt/client_spec.rb index 54b9bd3bfba..f1284318687 100644 --- a/spec/lib/gitlab/lets_encrypt/client_spec.rb +++ b/spec/lib/gitlab/lets_encrypt/client_spec.rb @@ -73,7 +73,7 @@ RSpec.describe ::Gitlab::LetsEncrypt::Client do subject(:new_order) { client.new_order('example.com') } before do - order_double = instance_double('Acme::Order') + order_double = double('Acme::Order') allow(stub_client).to receive(:new_order).and_return(order_double) end @@ -107,7 +107,7 @@ RSpec.describe ::Gitlab::LetsEncrypt::Client do subject { client.load_challenge(url) } before do - acme_challenge = instance_double('Acme::Client::Resources::Challenge') + acme_challenge = double('Acme::Client::Resources::Challenge') allow(stub_client).to receive(:challenge).with(url: url).and_return(acme_challenge) end diff --git a/spec/lib/gitlab/metrics/exporter/sidekiq_exporter_spec.rb b/spec/lib/gitlab/metrics/exporter/sidekiq_exporter_spec.rb index 01cf47a7c58..88c2c4a1918 100644 --- a/spec/lib/gitlab/metrics/exporter/sidekiq_exporter_spec.rb +++ b/spec/lib/gitlab/metrics/exporter/sidekiq_exporter_spec.rb @@ -50,40 +50,4 @@ RSpec.describe Gitlab::Metrics::Exporter::SidekiqExporter do expect(exporter.log_filename).to end_with('sidekiq_exporter.log') end end - - context 'when port is already taken' do - let(:first_exporter) { described_class.new } - - before do - stub_config( - monitoring: { - sidekiq_exporter: { - enabled: true, - port: 9992, - address: '127.0.0.1' - } - } - ) - - first_exporter.start - end - - after do - first_exporter.stop - end - - it 'does print error message' do - expect(Sidekiq.logger).to receive(:error) - .with( - class: described_class.to_s, - message: 'Cannot start sidekiq_exporter', - 'exception.message' => anything) - - exporter.start - end - - it 'does not start thread' do - expect(exporter.start).to be_nil - end - end end diff --git a/spec/lib/gitlab/quick_actions/dsl_spec.rb b/spec/lib/gitlab/quick_actions/dsl_spec.rb index f990abfb253..942d347424f 100644 --- a/spec/lib/gitlab/quick_actions/dsl_spec.rb +++ b/spec/lib/gitlab/quick_actions/dsl_spec.rb @@ -96,8 +96,8 @@ RSpec.describe Gitlab::QuickActions::Dsl do expect(dynamic_description_def.name).to eq(:dynamic_description) expect(dynamic_description_def.aliases).to eq([]) - expect(dynamic_description_def.to_h(OpenStruct.new(noteable: 'issue'))[:description]).to eq('A dynamic description for ISSUE') - expect(dynamic_description_def.execute_message(OpenStruct.new(noteable: 'issue'), 'arg')).to eq('A dynamic execution message for ISSUE passing arg') + expect(dynamic_description_def.to_h(double('desc', noteable: 'issue'))[:description]).to eq('A dynamic description for ISSUE') + expect(dynamic_description_def.execute_message(double('desc', noteable: 'issue'), 'arg')).to eq('A dynamic execution message for ISSUE passing arg') expect(dynamic_description_def.params).to eq(['The first argument', 'The second argument']) expect(dynamic_description_def.condition_block).to be_nil expect(dynamic_description_def.types).to eq([]) diff --git a/spec/lib/gitlab/rack_attack_spec.rb b/spec/lib/gitlab/rack_attack_spec.rb index 8f03905e08d..f0d6e3a527a 100644 --- a/spec/lib/gitlab/rack_attack_spec.rb +++ b/spec/lib/gitlab/rack_attack_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' RSpec.describe Gitlab::RackAttack, :aggregate_failures do describe '.configure' do let(:fake_rack_attack) { class_double("Rack::Attack") } - let(:fake_rack_attack_request) { class_double("Rack::Attack::Request") } - let(:fake_cache) { instance_double("Rack::Attack::Cache") } + let(:fake_rack_attack_request) { class_double(Rack::Attack::Request) } + let(:fake_cache) { instance_double(Rack::Attack::Cache) } let(:throttles) do { @@ -27,9 +27,6 @@ RSpec.describe Gitlab::RackAttack, :aggregate_failures do end before do - stub_const("Rack::Attack", fake_rack_attack) - stub_const("Rack::Attack::Request", fake_rack_attack_request) - allow(fake_rack_attack).to receive(:throttled_response=) allow(fake_rack_attack).to receive(:throttle) allow(fake_rack_attack).to receive(:track) @@ -37,6 +34,9 @@ RSpec.describe Gitlab::RackAttack, :aggregate_failures do allow(fake_rack_attack).to receive(:blocklist) allow(fake_rack_attack).to receive(:cache).and_return(fake_cache) allow(fake_cache).to receive(:store=) + + fake_rack_attack.const_set('Request', fake_rack_attack_request) + stub_const("Rack::Attack", fake_rack_attack) end it 'extends the request class' do diff --git a/spec/models/deployment_metrics_spec.rb b/spec/models/deployment_metrics_spec.rb index c804e20d66d..fe9218a9ae2 100644 --- a/spec/models/deployment_metrics_spec.rb +++ b/spec/models/deployment_metrics_spec.rb @@ -111,7 +111,7 @@ RSpec.describe DeploymentMetrics do } end - let(:prometheus_adapter) { instance_double('prometheus_adapter', can_query?: true, configured?: true) } + let(:prometheus_adapter) { instance_double(::Integrations::Prometheus, can_query?: true, configured?: true) } before do allow(deployment_metrics).to receive(:prometheus_adapter).and_return(prometheus_adapter) diff --git a/spec/requests/api/graphql/mutations/design_management/delete_spec.rb b/spec/requests/api/graphql/mutations/design_management/delete_spec.rb index 1dffb86b344..1f43f113e65 100644 --- a/spec/requests/api/graphql/mutations/design_management/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/design_management/delete_spec.rb @@ -53,7 +53,7 @@ RSpec.describe "deleting designs" do context 'the designs list contains filenames we cannot find' do it_behaves_like 'a failed request' do - let(:designs) { %w/foo bar baz/.map { |fn| instance_double('file', filename: fn) } } + let(:designs) { %w/foo bar baz/.map { |fn| double('file', filename: fn) } } let(:the_error) { a_string_matching %r/filenames were not found/ } end end diff --git a/spec/services/clusters/integrations/prometheus_health_check_service_spec.rb b/spec/services/clusters/integrations/prometheus_health_check_service_spec.rb index 9db3b9d2417..7147f1b9b28 100644 --- a/spec/services/clusters/integrations/prometheus_health_check_service_spec.rb +++ b/spec/services/clusters/integrations/prometheus_health_check_service_spec.rb @@ -58,7 +58,7 @@ RSpec.describe Clusters::Integrations::PrometheusHealthCheckService, '#execute' let(:prometheus_enabled) { true } before do - client = instance_double('PrometheusClient', healthy?: client_healthy) + client = instance_double('Gitlab::PrometheusClient', healthy?: client_healthy) expect(prometheus).to receive(:prometheus_client).and_return(client) end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index e3f33304aab..127c94763d9 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -151,7 +151,7 @@ RSpec.describe MergeRequests::MergeService do it 'closes GitLab issue tracker issues' do issue = create :issue, project: project - commit = instance_double('commit', safe_message: "Fixes #{issue.to_reference}", date: Time.current, authored_date: Time.current) + commit = double('commit', safe_message: "Fixes #{issue.to_reference}", date: Time.current, authored_date: Time.current) allow(merge_request).to receive(:commits).and_return([commit]) merge_request.cache_merge_request_closes_issues! diff --git a/spec/services/projects/prometheus/alerts/create_service_spec.rb b/spec/services/projects/prometheus/alerts/create_service_spec.rb index c0bc9336558..6b9d43e4e81 100644 --- a/spec/services/projects/prometheus/alerts/create_service_spec.rb +++ b/spec/services/projects/prometheus/alerts/create_service_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Projects::Prometheus::Alerts::CreateService do let_it_be(:project) { create(:project) } let_it_be(:user) { create(:user) } - let(:service) { described_class.new(project, user, params) } + let(:service) { described_class.new(project: project, current_user: user, params: params) } subject { service.execute } diff --git a/spec/services/projects/prometheus/alerts/destroy_service_spec.rb b/spec/services/projects/prometheus/alerts/destroy_service_spec.rb index 573711051b7..a3e9c3516c2 100644 --- a/spec/services/projects/prometheus/alerts/destroy_service_spec.rb +++ b/spec/services/projects/prometheus/alerts/destroy_service_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Projects::Prometheus::Alerts::DestroyService do let_it_be(:user) { create(:user) } let_it_be(:alert) { create(:prometheus_alert, project: project) } - let(:service) { described_class.new(project, user, nil) } + let(:service) { described_class.new(project: project, current_user: user, params: nil) } describe '#execute' do subject { service.execute(alert) } diff --git a/spec/services/projects/prometheus/alerts/update_service_spec.rb b/spec/services/projects/prometheus/alerts/update_service_spec.rb index e831d001838..ec6766221f6 100644 --- a/spec/services/projects/prometheus/alerts/update_service_spec.rb +++ b/spec/services/projects/prometheus/alerts/update_service_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Projects::Prometheus::Alerts::UpdateService do create(:prometheus_alert, project: project, environment: environment) end - let(:service) { described_class.new(project, user, params) } + let(:service) { described_class.new(project: project, current_user: user, params: params) } let(:params) do { diff --git a/spec/sidekiq_cluster/sidekiq_cluster_spec.rb b/spec/sidekiq_cluster/sidekiq_cluster_spec.rb index 3df2aa70553..86b3fc6a48c 100644 --- a/spec/sidekiq_cluster/sidekiq_cluster_spec.rb +++ b/spec/sidekiq_cluster/sidekiq_cluster_spec.rb @@ -7,19 +7,26 @@ require_relative '../../sidekiq_cluster/sidekiq_cluster' RSpec.describe Gitlab::SidekiqCluster do # rubocop:disable RSpec/FilePath describe '.start' do it 'starts Sidekiq with the given queues, environment and options' do - expected_options = { - env: :production, - directory: 'foo/bar', - max_concurrency: 20, - min_concurrency: 10, - timeout: 25, - dryrun: true + process_options = { + pgroup: true, + err: $stderr, + out: $stdout } - expect(described_class).to receive(:start_sidekiq).ordered.with(%w(foo), expected_options.merge(worker_id: 0)) - expect(described_class).to receive(:start_sidekiq).ordered.with(%w(bar baz), expected_options.merge(worker_id: 1)) - - described_class.start([%w(foo), %w(bar baz)], env: :production, directory: 'foo/bar', max_concurrency: 20, min_concurrency: 10, dryrun: true) + expect(Process).to receive(:spawn).ordered.with({ + "ENABLE_SIDEKIQ_CLUSTER" => "1", + "SIDEKIQ_WORKER_ID" => "0" + }, + "bundle", "exec", "sidekiq", "-c10", "-eproduction", "-t25", "-gqueues:foo", "-rfoo/bar", "-qfoo,1", process_options + ) + expect(Process).to receive(:spawn).ordered.with({ + "ENABLE_SIDEKIQ_CLUSTER" => "1", + "SIDEKIQ_WORKER_ID" => "1" + }, + "bundle", "exec", "sidekiq", "-c10", "-eproduction", "-t25", "-gqueues:bar,baz", "-rfoo/bar", "-qbar,1", "-qbaz,1", process_options + ) + + described_class.start([%w(foo), %w(bar baz)], env: :production, directory: 'foo/bar', max_concurrency: 20, min_concurrency: 10) end it 'starts Sidekiq with the given queues and sensible default options' do diff --git a/spec/simplecov_env.rb b/spec/simplecov_env.rb index e6ce4d256f0..c880f2d0829 100644 --- a/spec/simplecov_env.rb +++ b/spec/simplecov_env.rb @@ -52,8 +52,6 @@ module SimpleCovEnv add_filter '/app/controllers/sherlock/' # Profiling tool used only in development add_filter '/bin/' add_filter 'db/fixtures/' # Matches EE files as well - add_filter '/lib/gitlab/sidekiq_middleware/' - add_filter '/lib/system_check/' add_group 'Channels', 'app/channels' # Matches EE files as well add_group 'Controllers', 'app/controllers' # Matches EE files as well diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 25759ca50b8..0128a6334b3 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -475,3 +475,7 @@ Rugged::Settings['search_path_global'] = Rails.root.join('tmp/tests').to_s # Initialize FactoryDefault to use create_default helper TestProf::FactoryDefault.init + +# Exclude the Geo proxy API request from getting on_next_request Warden handlers, +# necessary to prevent race conditions with feature tests not getting authenticated. +::Warden.asset_paths << %r{^/api/v4/geo/proxy$} diff --git a/spec/support/rspec.rb b/spec/support/rspec.rb index 00b9aac7bf4..b4a25fd121d 100644 --- a/spec/support/rspec.rb +++ b/spec/support/rspec.rb @@ -15,7 +15,10 @@ require 'rubocop' require 'rubocop/rspec/support' RSpec.configure do |config| - config.mock_with :rspec + config.mock_with :rspec do |mocks| + mocks.verify_doubled_constant_names = true + end + config.raise_errors_for_deprecations! config.include StubConfiguration diff --git a/spec/support/shared_examples/features/wiki/user_updates_wiki_page_shared_examples.rb b/spec/support/shared_examples/features/wiki/user_updates_wiki_page_shared_examples.rb index 7ced8508a31..a456b76b324 100644 --- a/spec/support/shared_examples/features/wiki/user_updates_wiki_page_shared_examples.rb +++ b/spec/support/shared_examples/features/wiki/user_updates_wiki_page_shared_examples.rb @@ -138,11 +138,26 @@ RSpec.shared_examples 'User updates wiki page' do end context 'when using the content editor' do - before do - click_button 'Use the new editor' + context 'with feature flag on' do + before do + click_button 'Edit rich text' + end + + it_behaves_like 'edits content using the content editor' end - it_behaves_like 'edits content using the content editor' + context 'with feature flag off' do + before do + stub_feature_flags(wiki_switch_between_content_editor_raw_markdown: false) + visit(wiki_path(wiki)) + + click_link('Edit') + + click_button 'Use the new editor' + end + + it_behaves_like 'edits content using the content editor' + end end end diff --git a/spec/workers/issuable_export_csv_worker_spec.rb b/spec/workers/issuable_export_csv_worker_spec.rb index bcc2420996d..a18d10ad3df 100644 --- a/spec/workers/issuable_export_csv_worker_spec.rb +++ b/spec/workers/issuable_export_csv_worker_spec.rb @@ -35,10 +35,15 @@ RSpec.describe IssuableExportCsvWorker do end context 'with params' do - let(:params) { { 'test_key' => true } } + let(:params) { { 'test_key' => true, 'not' => { 'label_name' => ['SomeLabel'] } } } - it 'converts controller string keys to symbol keys for IssuesFinder' do - expect(IssuesFinder).to receive(:new).with(user, hash_including(test_key: true)).and_call_original + it 'allows symbol access for IssuesFinder' do + expect(IssuesFinder).to receive(:new).and_wrap_original do |method, user, params| + expect(params[:test_key]).to eq(true) + expect(params[:not][:label_name]).to eq(['SomeLabel']) + + method.call(user, params) + end subject end diff --git a/workhorse/.tool-versions b/workhorse/.tool-versions index 9d3b3b30ff2..febe02eb0bc 100644 --- a/workhorse/.tool-versions +++ b/workhorse/.tool-versions @@ -1 +1 @@ -golang 1.16.9 +golang 1.16.10 diff --git a/workhorse/internal/artifacts/artifacts_upload_test.go b/workhorse/internal/artifacts/artifacts_upload_test.go index df1c30dcff0..3e8a52be1a1 100644 --- a/workhorse/internal/artifacts/artifacts_upload_test.go +++ b/workhorse/internal/artifacts/artifacts_upload_test.go @@ -270,7 +270,7 @@ func TestUploadHandlerForMultipleFiles(t *testing.T) { require.NoError(t, s.writer.Close()) response := testUploadArtifacts(t, s.writer.FormDataContentType(), s.url, s.buffer) - require.Equal(t, http.StatusBadRequest, response.Code) + require.Equal(t, http.StatusInternalServerError, response.Code) } func TestUploadFormProcessing(t *testing.T) { diff --git a/workhorse/internal/upload/rewrite.go b/workhorse/internal/upload/rewrite.go index 3dfab120188..b9324ac8b7b 100644 --- a/workhorse/internal/upload/rewrite.go +++ b/workhorse/internal/upload/rewrite.go @@ -24,10 +24,12 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload/exif" ) +const maxFilesAllowed = 10 + // ErrInjectedClientParam means that the client sent a parameter that overrides one of our own fields var ( - ErrInjectedClientParam = errors.New("injected client parameter") - ErrMultipleFilesUploaded = errors.New("upload request contains more than one file") + ErrInjectedClientParam = errors.New("injected client parameter") + ErrTooManyFilesUploaded = fmt.Errorf("upload request contains more than %v files", maxFilesAllowed) ) var ( @@ -117,8 +119,8 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, pr } func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipart.Part, opts *filestore.SaveFileOpts) error { - if rew.filter.Count() > 0 { - return ErrMultipleFilesUploaded + if rew.filter.Count() >= maxFilesAllowed { + return ErrTooManyFilesUploaded } multipartFiles.WithLabelValues(rew.filter.Name()).Inc() diff --git a/workhorse/internal/upload/saved_file_tracker.go b/workhorse/internal/upload/saved_file_tracker.go index cd41e661b7d..e6f9a8c9a88 100644 --- a/workhorse/internal/upload/saved_file_tracker.go +++ b/workhorse/internal/upload/saved_file_tracker.go @@ -27,6 +27,10 @@ func (s *SavedFileTracker) Count() int { } func (s *SavedFileTracker) ProcessFile(_ context.Context, fieldName string, file *filestore.FileHandler, _ *multipart.Writer) error { + if _, ok := s.rewrittenFields[fieldName]; ok { + return fmt.Errorf("the %v field has already been processed", fieldName) + } + s.Track(fieldName, file.LocalPath) return nil } diff --git a/workhorse/internal/upload/saved_file_tracker_test.go b/workhorse/internal/upload/saved_file_tracker_test.go index b34dd9aed4f..ba927db253e 100644 --- a/workhorse/internal/upload/saved_file_tracker_test.go +++ b/workhorse/internal/upload/saved_file_tracker_test.go @@ -37,3 +37,14 @@ func TestSavedFileTracking(t *testing.T) { require.Contains(t, rewrittenFields, "test") } + +func TestDuplicatedFileProcessing(t *testing.T) { + tracker := SavedFileTracker{} + file := &filestore.FileHandler{} + + require.NoError(t, tracker.ProcessFile(context.Background(), "file", file, nil)) + + err := tracker.ProcessFile(context.Background(), "file", file, nil) + require.Error(t, err) + require.Equal(t, "the file field has already been processed", err.Error()) +} diff --git a/workhorse/internal/upload/uploads.go b/workhorse/internal/upload/uploads.go index 8f4c8bb95f0..b408d260379 100644 --- a/workhorse/internal/upload/uploads.go +++ b/workhorse/internal/upload/uploads.go @@ -35,8 +35,8 @@ func HandleFileUploads(w http.ResponseWriter, r *http.Request, h http.Handler, p switch err { case ErrInjectedClientParam: helper.CaptureAndFail(w, r, err, "Bad Request", http.StatusBadRequest) - case ErrMultipleFilesUploaded: - helper.CaptureAndFail(w, r, err, "Uploading multiple files is not allowed", http.StatusBadRequest) + case ErrTooManyFilesUploaded: + helper.CaptureAndFail(w, r, err, err.Error(), http.StatusBadRequest) case http.ErrNotMultipart: h.ServeHTTP(w, r) case filestore.ErrEntityTooLarge: diff --git a/workhorse/internal/upload/uploads_test.go b/workhorse/internal/upload/uploads_test.go index b5db8003d41..7c22b3b4e28 100644 --- a/workhorse/internal/upload/uploads_test.go +++ b/workhorse/internal/upload/uploads_test.go @@ -260,10 +260,10 @@ func TestUploadingMultipleFiles(t *testing.T) { var buffer bytes.Buffer writer := multipart.NewWriter(&buffer) - _, err = writer.CreateFormFile("file", "my.file") - require.NoError(t, err) - _, err = writer.CreateFormFile("file", "my.file") - require.NoError(t, err) + for i := 0; i < 11; i++ { + _, err = writer.CreateFormFile(fmt.Sprintf("file %v", i), "my.file") + require.NoError(t, err) + } require.NoError(t, writer.Close()) httpRequest, err := http.NewRequest("PUT", "/url/path", &buffer) @@ -279,7 +279,7 @@ func TestUploadingMultipleFiles(t *testing.T) { HandleFileUploads(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts) require.Equal(t, 400, response.Code) - require.Equal(t, "Uploading multiple files is not allowed\n", response.Body.String()) + require.Equal(t, "upload request contains more than 10 files\n", response.Body.String()) } func TestUploadProcessingFile(t *testing.T) { diff --git a/workhorse/internal/upstream/upstream.go b/workhorse/internal/upstream/upstream.go index e57f58d59dd..6835569dfa8 100644 --- a/workhorse/internal/upstream/upstream.go +++ b/workhorse/internal/upstream/upstream.go @@ -65,7 +65,7 @@ func newUpstream(cfg config.Config, accessLogger *logrus.Logger, routesCallback Config: cfg, accessLogger: accessLogger, // Kind of a feature flag. See https://gitlab.com/groups/gitlab-org/-/epics/5914#note_564974130 - enableGeoProxyFeature: os.Getenv("GEO_SECONDARY_PROXY") == "1", + enableGeoProxyFeature: os.Getenv("GEO_SECONDARY_PROXY") != "0", geoProxyBackend: &url.URL{}, } if up.geoProxyPollSleep == nil { @@ -207,8 +207,8 @@ func (u *upstream) findGeoProxyRoute(cleanedPath string, r *http.Request) *route func (u *upstream) pollGeoProxyAPI() { for { - u.callGeoProxyAPI() u.geoProxyPollSleep(geoProxyApiPollingInterval) + u.callGeoProxyAPI() } } diff --git a/workhorse/internal/upstream/upstream_test.go b/workhorse/internal/upstream/upstream_test.go index 53c15bb7e91..4d8efed2b73 100644 --- a/workhorse/internal/upstream/upstream_test.go +++ b/workhorse/internal/upstream/upstream_test.go @@ -310,5 +310,9 @@ func startWorkhorseServer(railsServerURL string, enableGeoProxyFeature bool) (*h } } + // Since the first sleep happens before any API call, this ensures + // we call the API at least once. + waitForNextApiPoll() + return ws, ws.Close, waitForNextApiPoll } |