diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-04-12 15:08:32 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-04-12 15:08:32 +0000 |
commit | 6e881971b0cbe42e3d8ca10a5e91cb9c611ae95b (patch) | |
tree | bf273e11608fdff70bed74f7dae759d06583717e | |
parent | 47bb4281f0b12f158923f8a1e5691c7a4be2f2f6 (diff) | |
download | gitlab-ce-6e881971b0cbe42e3d8ca10a5e91cb9c611ae95b.tar.gz |
Add latest changes from gitlab-org/gitlab@master
34 files changed, 697 insertions, 88 deletions
diff --git a/.editorconfig b/.editorconfig index 932acffe9fc..0edad7878b5 100644 --- a/.editorconfig +++ b/.editorconfig @@ -16,3 +16,6 @@ charset = utf-8 [*.{md,markdown,js.snap}] trim_trailing_whitespace = false + +[*.rb] +max_line_length = 120 diff --git a/.rubocop_todo/style/open_struct_use.yml b/.rubocop_todo/style/open_struct_use.yml index 4ccf0ccd033..47d272def01 100644 --- a/.rubocop_todo/style/open_struct_use.yml +++ b/.rubocop_todo/style/open_struct_use.yml @@ -1,7 +1,6 @@ --- Style/OpenStructUse: Exclude: - - app/helpers/application_settings_helper.rb - ee/spec/features/projects/new_project_spec.rb - ee/spec/finders/template_finder_spec.rb - ee/spec/helpers/ee/blob_helper_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index f0a3aca58a1..e02c646674e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 14.9.3 (2022-04-12) + +### Fixed (4 changes) + +- [Revert Protected Environment group access inheritence](gitlab-org/gitlab@488fd8f3f6770eebae10c815398534ff41d57546) ([merge request](gitlab-org/gitlab!84664)) +- [Fix URL blocker when object storage enabled but type is disabled](gitlab-org/gitlab@d0da89768774de9cf635af530ed7386e65f92d40) ([merge request](gitlab-org/gitlab!84664)) +- [Remove pending builds from the queue on conflict](gitlab-org/gitlab@8c88898dfd1619cc635ce5b98e30eebd91da497f) ([merge request](gitlab-org/gitlab!84664)) +- [Fix null argument handling in background migration Rake task](gitlab-org/gitlab@23e1eb3272828b3546e18efdfaea5a8077cb20f4) ([merge request](gitlab-org/gitlab!84664)) + ## 14.9.2 (2022-03-31) ### Security (20 changes) diff --git a/app/assets/javascripts/security_configuration/components/constants.js b/app/assets/javascripts/security_configuration/components/constants.js index 82cd96aa3b5..6db28ef0fad 100644 --- a/app/assets/javascripts/security_configuration/components/constants.js +++ b/app/assets/javascripts/security_configuration/components/constants.js @@ -50,11 +50,17 @@ export const SAST_IAC_CONFIG_HELP_PATH = helpPagePath( export const DAST_NAME = __('Dynamic Application Security Testing (DAST)'); export const DAST_SHORT_NAME = s__('ciReport|DAST'); -export const DAST_DESCRIPTION = __('Analyze a review version of your web application.'); +export const DAST_DESCRIPTION = s__( + 'ciReport|Analyze a deployed version of your web application for known vulnerabilities by examining it from the outside in. DAST works by simulating external attacks on your application while it is running.', +); export const DAST_HELP_PATH = helpPagePath('user/application_security/dast/index'); export const DAST_CONFIG_HELP_PATH = helpPagePath('user/application_security/dast/index', { anchor: 'enable-dast', }); +export const DAST_BADGE_TEXT = __('Available on-demand'); +export const DAST_BADGE_TOOLTIP = __( + 'On-demand scans run outside of the DevOps cycle and find vulnerabilities in your projects', +); export const DAST_PROFILES_NAME = __('DAST profiles'); export const DAST_PROFILES_DESCRIPTION = s__( @@ -171,18 +177,23 @@ export const securityFeatures = [ type: REPORT_TYPE_SAST_IAC, }, { - name: DAST_NAME, - shortName: DAST_SHORT_NAME, - description: DAST_DESCRIPTION, - helpPath: DAST_HELP_PATH, - configurationHelpPath: DAST_CONFIG_HELP_PATH, - type: REPORT_TYPE_DAST, + badge: { + text: DAST_BADGE_TEXT, + tooltipText: DAST_BADGE_TOOLTIP, + variant: 'info', + }, secondary: { type: REPORT_TYPE_DAST_PROFILES, name: DAST_PROFILES_NAME, description: DAST_PROFILES_DESCRIPTION, configurationText: DAST_PROFILES_CONFIG_TEXT, }, + name: DAST_NAME, + shortName: DAST_SHORT_NAME, + description: DAST_DESCRIPTION, + helpPath: DAST_HELP_PATH, + configurationHelpPath: DAST_CONFIG_HELP_PATH, + type: REPORT_TYPE_DAST, }, { name: DEPENDENCY_SCANNING_NAME, diff --git a/app/assets/javascripts/security_configuration/components/feature_card.vue b/app/assets/javascripts/security_configuration/components/feature_card.vue index 4ce3c7cbc9a..309e5f21445 100644 --- a/app/assets/javascripts/security_configuration/components/feature_card.vue +++ b/app/assets/javascripts/security_configuration/components/feature_card.vue @@ -1,8 +1,9 @@ <script> import { GlButton, GlCard, GlIcon, GlLink } from '@gitlab/ui'; import { __, s__, sprintf } from '~/locale'; -import ManageViaMr from '~/vue_shared/security_configuration/components/manage_via_mr.vue'; import { REPORT_TYPE_SAST_IAC } from '~/vue_shared/security_reports/constants'; +import ManageViaMr from '~/vue_shared/security_configuration/components/manage_via_mr.vue'; +import FeatureCardBadge from './feature_card_badge.vue'; export default { components: { @@ -10,6 +11,7 @@ export default { GlCard, GlIcon, GlLink, + FeatureCardBadge, ManageViaMr, }, props: { @@ -37,11 +39,14 @@ export default { text: this.$options.i18n.enableFeature, }; - button.category = 'secondary'; + button.category = this.feature.category || 'secondary'; button.text = sprintf(button.text, { feature: this.shortName }); return button; }, + manageViaMrButtonCategory() { + return this.feature.category || 'secondary'; + }, showManageViaMr() { return ManageViaMr.canRender(this.feature); }, @@ -49,13 +54,17 @@ export default { return { 'gl-bg-gray-10': !this.available }; }, statusClasses() { - const { enabled } = this; + const { enabled, hasBadge } = this; return { 'gl-ml-auto': true, 'gl-flex-shrink-0': true, 'gl-text-gray-500': !enabled, 'gl-text-green-500': enabled, + 'gl-w-full': hasBadge, + 'gl-justify-content-space-between': hasBadge, + 'gl-display-flex': hasBadge, + 'gl-mb-4': hasBadge, }; }, hasSecondary() { @@ -68,6 +77,9 @@ export default { isNotSastIACTemporaryHack() { return this.feature.type !== REPORT_TYPE_SAST_IAC; }, + hasBadge() { + return Boolean(this.available && this.feature.badge?.text); + }, }, methods: { onError(message) { @@ -88,7 +100,10 @@ export default { <template> <gl-card :class="cardClasses"> - <div class="gl-display-flex gl-align-items-baseline"> + <div + class="gl-display-flex gl-align-items-baseline" + :class="{ 'gl-flex-direction-column-reverse': hasBadge }" + > <h3 class="gl-font-lg gl-m-0 gl-mr-3">{{ feature.name }}</h3> <div @@ -97,13 +112,19 @@ export default { data-testid="feature-status" :data-qa-selector="`${feature.type}_status`" > + <feature-card-badge + v-if="hasBadge" + :badge="feature.badge" + :badge-href="feature.badge.badgeHref" + /> + <template v-if="enabled"> <gl-icon name="check-circle-filled" /> <span class="gl-text-green-700">{{ $options.i18n.enabled }}</span> </template> <template v-else-if="available"> - {{ $options.i18n.notEnabled }} + <span>{{ $options.i18n.notEnabled }}</span> </template> <template v-else> @@ -133,7 +154,7 @@ export default { v-else-if="showManageViaMr" :feature="feature" variant="confirm" - category="secondary" + :category="manageViaMrButtonCategory" class="gl-mt-5" :data-qa-selector="`${feature.type}_mr_button`" @error="onError" diff --git a/app/assets/javascripts/security_configuration/components/feature_card_badge.vue b/app/assets/javascripts/security_configuration/components/feature_card_badge.vue new file mode 100644 index 00000000000..0907e33c8e2 --- /dev/null +++ b/app/assets/javascripts/security_configuration/components/feature_card_badge.vue @@ -0,0 +1,40 @@ +<script> +import { GlBadge, GlTooltip } from '@gitlab/ui'; + +export default { + components: { + GlBadge, + GlTooltip, + }, + props: { + badge: { + type: Object, + required: true, + }, + badgeHref: { + type: String, + required: false, + default: '', + }, + }, +}; +</script> + +<template> + <span> + <gl-tooltip + v-if="badge.tooltipText" + placement="top" + boundary="window" + title="Tooltip title" + :target="() => $refs.badge" + > + {{ badge.tooltipText }} + </gl-tooltip> + <span ref="badge"> + <gl-badge size="sm" :href="badgeHref" :variant="badge.variant"> + {{ badge.text }} + </gl-badge> + </span> + </span> +</template> diff --git a/app/assets/javascripts/security_configuration/utils.js b/app/assets/javascripts/security_configuration/utils.js index 173560f8370..df23698ba7e 100644 --- a/app/assets/javascripts/security_configuration/utils.js +++ b/app/assets/javascripts/security_configuration/utils.js @@ -30,6 +30,10 @@ export const augmentFeatures = (securityFeatures, complianceFeatures, features = augmented.secondary = { ...augmented.secondary, ...featuresByType[feature.secondary.type] }; } + if (augmented.badge && augmented.metaInfoPath) { + augmented.badge.badgeHref = augmented.metaInfoPath; + } + return augmented; }; diff --git a/app/controllers/concerns/wiki_actions.rb b/app/controllers/concerns/wiki_actions.rb index e673ba501ff..91de1d8aeae 100644 --- a/app/controllers/concerns/wiki_actions.rb +++ b/app/controllers/concerns/wiki_actions.rb @@ -308,8 +308,7 @@ module WikiActions end def load_content? - return false if %w[history destroy diff].include?(params[:action]) - return false if params[:action] == 'show' && Feature.enabled?(:wiki_async_load, container, default_enabled: :yaml) + return false if %w[history destroy diff show].include?(params[:action]) true end diff --git a/app/graphql/types/dependency_proxy/manifest_type_enum.rb b/app/graphql/types/dependency_proxy/manifest_type_enum.rb index dd16f542e68..ddd1652eeea 100644 --- a/app/graphql/types/dependency_proxy/manifest_type_enum.rb +++ b/app/graphql/types/dependency_proxy/manifest_type_enum.rb @@ -5,7 +5,7 @@ module Types graphql_name 'DependencyProxyManifestStatus' ::DependencyProxy::Manifest.statuses.keys.each do |status| - value status.upcase, { description: "Dependency proxy manifest has a status of #{status}.", value: status } + value status.upcase, description: "Dependency proxy manifest has a status of #{status}.", value: status end end end diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index a9721766dc8..77cf3cc92eb 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -37,9 +37,15 @@ module ApplicationSettingsHelper end def storage_weights - Gitlab.config.repositories.storages.keys.each_with_object(OpenStruct.new) do |storage, weights| - weights[storage.to_sym] = @application_setting.repository_storages_weighted[storage] || 0 + # Instead of using a `Struct` we could wrap this into an object. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/358419 + weights = Struct.new(*Gitlab.config.repositories.storages.keys.map(&:to_sym)) + + values = Gitlab.config.repositories.storages.keys.map do |storage| + @application_setting.repository_storages_weighted[storage] || 0 end + + weights.new(*values) end def all_protocols_enabled? @@ -223,6 +229,7 @@ module ApplicationSettingsHelper :default_project_visibility, :default_projects_limit, :default_snippet_visibility, + :delete_inactive_projects, :disable_feed_token, :disabled_oauth_sign_in_sources, :domain_denylist, @@ -273,6 +280,9 @@ module ApplicationSettingsHelper :html_emails_enabled, :import_sources, :in_product_marketing_emails_enabled, + :inactive_projects_delete_after_months, + :inactive_projects_min_size_mb, + :inactive_projects_send_warning_email_after_months, :invisible_captcha_enabled, :max_artifacts_size, :max_attachment_size, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index f0888009f7f..7e8e3cdb3d8 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -578,6 +578,15 @@ class ApplicationSetting < ApplicationRecord validates :public_runner_releases_url, addressable_url: true, presence: true + validates :inactive_projects_min_size_mb, + numericality: { only_integer: true, greater_than_or_equal_to: 0 } + + validates :inactive_projects_delete_after_months, + numericality: { only_integer: true, greater_than: 0 } + + validates :inactive_projects_send_warning_email_after_months, + numericality: { only_integer: true, greater_than: 0, less_than: :inactive_projects_delete_after_months } + attr_encrypted :asset_proxy_secret_key, mode: :per_attribute_iv, key: Settings.attr_encrypted_db_key_base_truncated, diff --git a/app/presenters/projects/security/configuration_presenter.rb b/app/presenters/projects/security/configuration_presenter.rb index 1798d4b780f..77f4d57ae09 100644 --- a/app/presenters/projects/security/configuration_presenter.rb +++ b/app/presenters/projects/security/configuration_presenter.rb @@ -81,7 +81,8 @@ module Projects configured: scan.configured?, configuration_path: scan.configuration_path, available: scan.available?, - can_enable_by_merge_request: scan.can_enable_by_merge_request? + can_enable_by_merge_request: scan.can_enable_by_merge_request?, + meta_info_path: scan.meta_info_path } end diff --git a/app/views/shared/wikis/show.html.haml b/app/views/shared/wikis/show.html.haml index 08e78f083fd..6591e8fae7b 100644 --- a/app/views/shared/wikis/show.html.haml +++ b/app/views/shared/wikis/show.html.haml @@ -27,9 +27,6 @@ - if can?(current_user, :create_wiki, @wiki.container) && @page.latest? && @valid_encoding = link_to sprite_icon('pencil', css_class: 'gl-icon'), wiki_page_path(@wiki, @page, action: :edit), title: 'Edit', role: "button", class: 'btn gl-button btn-icon btn-default js-wiki-edit', data: { qa_selector: 'edit_page_button', testid: 'wiki_edit_button' } - - if Feature.enabled?(:wiki_async_load, @wiki.container, default_enabled: :yaml) - .js-async-wiki-page-content.md.gl-pt-2{ data: { qa_selector: 'wiki_page_content', testid: 'wiki_page_content', tracking_context: wiki_page_tracking_context(@page).to_json, get_wiki_content_url: wiki_page_render_api_endpoint(@page) } } - - else - = render 'shared/wikis/wiki_content' + .js-async-wiki-page-content.md.gl-pt-2{ data: { qa_selector: 'wiki_page_content', testid: 'wiki_page_content', tracking_context: wiki_page_tracking_context(@page).to_json, get_wiki_content_url: wiki_page_render_api_endpoint(@page) } } = render 'shared/wikis/sidebar' diff --git a/config/feature_flags/development/wiki_async_load.yml b/config/feature_flags/development/wiki_async_load.yml deleted file mode 100644 index f5b2c00518e..00000000000 --- a/config/feature_flags/development/wiki_async_load.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: wiki_async_load -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82394 -rollout_issue_url: -milestone: '14.9' -type: development -group: group::editor -default_enabled: false diff --git a/config/feature_flags/experiment/bypass_registration.yml b/config/feature_flags/experiment/bypass_registration.yml deleted file mode 100644 index b3a0d634248..00000000000 --- a/config/feature_flags/experiment/bypass_registration.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: bypass_registration -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72827 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/340560 -milestone: '14.5' -type: experiment -group: group::adoption -default_enabled: false diff --git a/db/migrate/20220406113217_add_inactive_project_deletion_to_application_settings.rb b/db/migrate/20220406113217_add_inactive_project_deletion_to_application_settings.rb new file mode 100644 index 00000000000..78970d308d9 --- /dev/null +++ b/db/migrate/20220406113217_add_inactive_project_deletion_to_application_settings.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddInactiveProjectDeletionToApplicationSettings < Gitlab::Database::Migration[1.0] + def change + add_column :application_settings, :delete_inactive_projects, :boolean, default: false, null: false + add_column :application_settings, :inactive_projects_delete_after_months, :integer, default: 2, null: false + add_column :application_settings, :inactive_projects_min_size_mb, :integer, default: 0, null: false + add_column :application_settings, :inactive_projects_send_warning_email_after_months, :integer, default: 1, + null: false + end +end diff --git a/db/schema_migrations/20220406113217 b/db/schema_migrations/20220406113217 new file mode 100644 index 00000000000..1fb61b081a4 --- /dev/null +++ b/db/schema_migrations/20220406113217 @@ -0,0 +1 @@ +161ba8db7400c12dc0550246af8db86487e811803eaecedcb2761f4a8349920b
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 2ce4da1cebc..3c00cade4d4 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -11257,6 +11257,10 @@ CREATE TABLE application_settings ( database_grafana_api_url text, database_grafana_tag text, public_runner_releases_url text DEFAULT 'https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-runner/releases'::text NOT NULL, + delete_inactive_projects boolean DEFAULT false NOT NULL, + inactive_projects_delete_after_months integer DEFAULT 2 NOT NULL, + inactive_projects_min_size_mb integer DEFAULT 0 NOT NULL, + inactive_projects_send_warning_email_after_months integer DEFAULT 1 NOT NULL, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), diff --git a/doc/.vale/gitlab/spelling-exceptions.txt b/doc/.vale/gitlab/spelling-exceptions.txt index e998215a592..048b196105a 100644 --- a/doc/.vale/gitlab/spelling-exceptions.txt +++ b/doc/.vale/gitlab/spelling-exceptions.txt @@ -524,6 +524,7 @@ repurposing requeue requeued requeues +requeuing Restlet resync resynced diff --git a/doc/api/settings.md b/doc/api/settings.md index 02c3f8b0d30..cc9df7917e2 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -274,6 +274,7 @@ listed in the descriptions of the relevant settings. | `default_projects_limit` | integer | no | Project limit per user. Default is `100000`. | | `default_snippet_visibility` | string | no | What visibility level new snippets receive. Can take `private`, `internal` and `public` as a parameter. Default is `private`. | | `delayed_project_deletion` **(PREMIUM SELF)** | boolean | no | Enable delayed project deletion by default in new groups. Default is `false`. | +| `delete_inactive_projects` | boolean | no | Enable inactive project deletion feature. Default is `false`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84519) in GitLab 14.10. | | `deletion_adjourned_period` **(PREMIUM SELF)** | integer | no | The number of days to wait before deleting a project or group that is marked for deletion. Value must be between 0 and 90. | `diff_max_patch_bytes` | integer | no | Maximum [diff patch size](../user/admin_area/diff_limits.md), in bytes. | | `diff_max_files` | integer | no | Maximum [files in a diff](../user/admin_area/diff_limits.md). | @@ -350,6 +351,9 @@ listed in the descriptions of the relevant settings. | `html_emails_enabled` | boolean | no | Enable HTML emails. | | `import_sources` | array of strings | no | Sources to allow project import from, possible values: `github`, `bitbucket`, `bitbucket_server`, `gitlab`, `fogbugz`, `git`, `gitlab_project`, `gitea`, `manifest`, and `phabricator`. | | `in_product_marketing_emails_enabled` | boolean | no | Enable [in-product marketing emails](../user/profile/notifications.md#global-notification-settings). Enabled by default. | +| `inactive_projects_delete_after_months` | integer | no | If `delete_inactive_projects` is `true`, the time (in months) to wait before deleting inactive projects. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84519) in GitLab 14.10. | +| `inactive_projects_min_size_mb` | integer | no | If `delete_inactive_projects` is `true`, the minimum repository size for projects to be checked for inactivity. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84519) in GitLab 14.10. | +| `inactive_projects_send_warning_email_after_months` | integer | no | If `delete_inactive_projects` is `true`, sets the time (in months) to wait before emailing maintainers that the project will be deleted because it is inactive. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84519) in GitLab 14.10. | | `invisible_captcha_enabled` | boolean | no | Enable Invisible CAPTCHA spam detection during sign-up. Disabled by default. | | `issues_create_limit` | integer | no | Max number of issue creation requests per minute per user. Disabled by default.| | `keep_latest_artifact` | boolean | no | Prevent the deletion of the artifacts from the most recent successful jobs, regardless of the expiry time. Enabled by default. | diff --git a/doc/development/batched_background_migrations.md b/doc/development/batched_background_migrations.md new file mode 100644 index 00000000000..e7703b5dd2b --- /dev/null +++ b/doc/development/batched_background_migrations.md @@ -0,0 +1,319 @@ +--- +type: reference, dev +stage: Enablement +group: Database +info: "See the Technical Writers assigned to Development Guidelines: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments-to-development-guidelines" +--- + +# Batched background migrations + +Batched Background Migrations should be used to perform data migrations whenever a +migration exceeds [the time limits](migration_style_guide.md#how-long-a-migration-should-take) +in our guidelines. For example, you can use batched background +migrations to migrate data that's stored in a single JSON column +to a separate table instead. + +## When to use batched background migrations + +Use a batched background migration when you migrate _data_ in tables containing +so many rows that the process would exceed +[the time limits in our guidelines](migration_style_guide.md#how-long-a-migration-should-take) +if performed using a regular Rails migration. + +- Batched background migrations should be used when migrating data in + [high-traffic tables](migration_style_guide.md#high-traffic-tables). +- Batched background migrations may also be used when executing numerous single-row queries + for every item on a large dataset. Typically, for single-record patterns, runtime is + largely dependent on the size of the dataset. Split the dataset accordingly, + and put it into background migrations. +- Don't use batched background migrations to perform schema migrations. + +Background migrations can help when: + +- Migrating events from one table to multiple separate tables. +- Populating one column based on JSON stored in another column. +- Migrating data that depends on the output of external services. (For example, an API.) + +NOTE: +If the batched background migration is part of an important upgrade, it must be announced +in the release post. Discuss with your Project Manager if you're unsure if the migration falls +into this category. + +## Isolation + +Batched background migrations must be isolated and can not use application code. (For example, +models defined in `app/models`.). Because these migrations can take a long time to +run, it's possible for new versions to deploy while the migrations are still running. + +## Idempotence + +Batched background migrations are executed in a context of a Sidekiq process. +The usual Sidekiq rules apply, especially the rule that jobs should be small +and idempotent. Make sure that in case that your migration job is retried, data +integrity is guaranteed. + +See [Sidekiq best practices guidelines](https://github.com/mperham/sidekiq/wiki/Best-Practices) +for more details. + +## Batched background migrations for EE-only features + +All the background migration classes for EE-only features should be present in GitLab CE. +For this purpose, create an empty class for GitLab CE, and extend it for GitLab EE +as explained in the guidelines for +[implementing Enterprise Edition features](ee_features.md#code-in-libgitlabbackground_migration). + +Batched Background migrations are simple classes that define a `perform` method. A +Sidekiq worker then executes such a class, passing any arguments to it. All +migration classes must be defined in the namespace +`Gitlab::BackgroundMigration`. Place the files in the directory +`lib/gitlab/background_migration/`. + +## Queueing + +Queueing a batched background migration should be done in a post-deployment +migration. Use this `queue_batched_background_migration` example, queueing the +migration to be executed in batches. Replace the class name and arguments with the values +from your migration: + +```ruby +queue_batched_background_migration( + JOB_CLASS_NAME, + TABLE_NAME, + JOB_ARGUMENTS, + JOB_INTERVAL + ) +``` + +Make sure the newly-created data is either migrated, or +saved in both the old and new version upon creation. Removals in +turn can be handled by defining foreign keys with cascading deletes. + +### Requeuing batched background migrations + +If one of the batched background migrations contains a bug that is fixed in a patch +release, you must requeue the batched background migration so the migration +repeats on systems that already performed the initial migration. + +When you requeue the batched background migration, turn the original +queuing into a no-op by clearing up the `#up` and `#down` methods of the +migration performing the requeuing. Otherwise, the batched background migration is +queued multiple times on systems that are upgrading multiple patch releases at +once. + +When you start the second post-deployment migration, delete the +previously batched migration with the provided code: + +```ruby +Gitlab::Database::BackgroundMigration::BatchedMigration + .for_configuration(MIGRATION_NAME, TABLE_NAME, COLUMN, JOB_ARGUMENTS) + .delete_all +``` + +## Cleaning up + +NOTE: +Cleaning up any remaining background migrations must be done in either a major +or minor release. You must not do this in a patch release. + +Because background migrations can take a long time, you can't immediately clean +things up after queueing them. For example, you can't drop a column used in the +migration process, as jobs would fail. You must add a separate _post-deployment_ +migration in a future release that finishes any remaining +jobs before cleaning things up. (For example, removing a column.) + +To migrate the data from column `foo` (containing a big JSON blob) to column `bar` +(containing a string), you would: + +1. Release A: + 1. Create a migration class that performs the migration for a row with a given ID. + 1. Update new rows using one of these techniques: + - Create a new trigger for simple copy operations that don't need application logic. + - Handle this operation in the model/service as the records are created or updated. + - Create a new custom background job that updates the records. + 1. Queue the batched background migration for all existing rows in a post-deployment migration. +1. Release B: + 1. Add a post-deployment migration that checks if the batched background migration is completed. + 1. Deploy code so that the application starts using the new column and stops to update new records. + 1. Remove the old column. + +Bump to the [import/export version](../user/project/settings/import_export.md) may +be required, if importing a project from a prior version of GitLab requires the +data to be in the new format. + +## Example + +The table `integrations` has a field called `properties`, stored in JSON. For all rows, +extract the `url` key from this JSON object and store it in the `integrations.url` +column. Millions of integrations exist, and parsing JSON is slow, so you can't +do this work in a regular migration. + +1. Start by defining our migration class: + + ```ruby + class Gitlab::BackgroundMigration::ExtractIntegrationsUrl + class Integration < ActiveRecord::Base + self.table_name = 'integrations' + end + + def perform(start_id, end_id) + Integration.where(id: start_id..end_id).each do |integration| + json = JSON.load(integration.properties) + + integration.update(url: json['url']) if json['url'] + rescue JSON::ParserError + # If the JSON is invalid we don't want to keep the job around forever, + # instead we'll just leave the "url" field to whatever the default value + # is. + next + end + end + end + ``` + + NOTE: + To get a `connection` in the batched background migration,use an inheritance + relation using the following base class `Gitlab::BackgroundMigration::BaseJob`. + For example: `class Gitlab::BackgroundMigration::ExtractIntegrationsUrl < Gitlab::BackgroundMigration::BaseJob` + +1. Add a new trigger to the database to update newly created and updated integrations, + similar to this example: + + ```ruby + execute(<<~SQL) + CREATE OR REPLACE FUNCTION example() RETURNS trigger + LANGUAGE plpgsql + AS $$ + BEGIN + NEW."url" := NEW.properties -> "url" + RETURN NEW; + END; + $$; + SQL + ``` + +1. Create a post-deployment migration that queues the migration for existing data: + + ```ruby + class QueueExtractIntegrationsUrl < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + MIGRATION = 'ExtractIntegrationsUrl' + DELAY_INTERVAL = 2.minutes + + def up + queue_batched_background_migration( + MIGRATION, + :migrations, + :id, + job_interval: DELAY_INTERVAL + ) + end + + def down + Gitlab::Database::BackgroundMigration::BatchedMigration + .for_configuration(MIGRATION, :migrations, :id, []).delete_all + end + end + ``` + + After deployment, our application: + - Continues using the data as before. + - Ensures that both existing and new data are migrated. + +1. In the next release, remove the trigger. We must also add a new post-deployment migration + that checks that the batched background migration is completed. For example: + + ```ruby + class FinalizeExtractIntegrationsUrlJobs < Gitlab::Database::Migration[1.0] + MIGRATION = 'ExtractIntegrationsUrl' + disable_ddl_transaction! + + def up + ensure_batched_background_migration_is_finished( + job_class_name: MIGRATION, + table_name: :integrations, + column_name: :id, + job_arguments: [] + ) + end + + def down + # no-op + end + end + ``` + + If the application does not depend on the data being 100% migrated (for + instance, the data is advisory, and not mission-critical), then you can skip this + final step. This step confirms that the migration is completed, and all of the rows were migrated. + +After the batched migration is completed, you can safely remove the `integrations.properties` column. + +## Testing + +Writing tests is required for: + +- The batched background migrations' queueing migration. +- The batched background migration itself. +- A cleanup migration. + +The `:migration` and `schema: :latest` RSpec tags are automatically set for +background migration specs. Refer to the +[Testing Rails migrations](testing_guide/testing_migrations_guide.md#testing-a-non-activerecordmigration-class) +style guide. + +Remember that `before` and `after` RSpec hooks +migrate your database down and up. These hooks can result in other batched background +migrations being called. Using `spy` test doubles with +`have_received` is encouraged, instead of using regular test doubles, because +your expectations defined in a `it` block can conflict with what is +called in RSpec hooks. Refer to [issue #35351](https://gitlab.com/gitlab-org/gitlab/-/issues/18839) +for more details. + +## Best practices + +1. Know how much data you're dealing with. +1. Make sure the batched background migration jobs are idempotent. +1. Confirm the tests you write are not false positives. +1. If the data being migrated is critical and cannot be lost, the + clean-up migration must also check the final state of the data before completing. +1. Discuss the numbers with a database specialist. The migration may add + more pressure on DB than you expect. Measure on staging, + or ask someone to measure on production. +1. Know how much time is required to run the batched background migration. + +## Additional tips and strategies + +### Viewing failure error logs + +You can view failures in two ways: + +- Via GitLab logs: + 1. After running a batched background migration, if any jobs fail, + view the logs in [Kibana](https://log.gprd.gitlab.net/goto/5f06a57f768c6025e1c65aefb4075694). + View the production Sidekiq log and filter for: + + - `json.new_state: failed` + - `json.job_class_name: <Batched Background Migration job class name>` + - `json.job_arguments: <Batched Background Migration job class arguments>` + + 1. Review the `json.exception_class` and `json.exception_message` values to help + understand why the jobs failed. + + 1. Remember the retry mechanism. Having a failure does not mean the job failed. + Always check the last status of the job. + +- Via database: + + 1. Get the batched background migration `CLASS_NAME`. + 1. Execute the following query in the PostgreSQL console: + + ```sql + SELECT migration.id, migration.job_class_name, transition_logs.exception_class, transition_logs.exception_message + FROM batched_background_migrations as migration + INNER JOIN batched_background_migration_jobs as jobs + ON jobs.batched_background_migration_id = migration.id + INNER JOIN batched_background_migration_job_transition_logs as transition_logs + ON transition_logs.batched_background_migration_job_id = jobs.id + WHERE transition_logs.next_status = '2' AND migration.job_class_name = "CLASS_NAME"; + ``` diff --git a/doc/integration/elasticsearch.md b/doc/integration/elasticsearch.md index 5265a24d299..ca557400e42 100644 --- a/doc/integration/elasticsearch.md +++ b/doc/integration/elasticsearch.md @@ -982,7 +982,7 @@ There is also an easy way to check it automatically with `sudo gitlab-rake gitla This exception is seen when your Elasticsearch cluster is configured to reject requests above a certain size (10MiB in this case). This corresponds to the `http.max_content_length` setting in `elasticsearch.yml`. Increase it to a larger size and restart your Elasticsearch cluster. -AWS has [fixed limits](https://docs.aws.amazon.com/opensearch-service/latest/developerguide/aes-limits.html) for this setting ("Maximum Size of HTTP Request Payloads"), based on the size of the underlying instance. +AWS has [fixed limits](https://docs.aws.amazon.com/opensearch-service/latest/developerguide/limits.html#network-limits) for this setting ("Maximum size of HTTP request payloads"), based on the size of the underlying instance. ### My single node Elasticsearch cluster status never goes from `yellow` to `green` even though everything seems to be running properly diff --git a/doc/user/project/members/index.md b/doc/user/project/members/index.md index cff5d8f82a7..ff4677eddde 100644 --- a/doc/user/project/members/index.md +++ b/doc/user/project/members/index.md @@ -49,7 +49,7 @@ flowchart RL [Feature flag `invite_members_group_modal`](https://gitlab.com/gitlab-org/gitlab/-/issues/352526) removed. Add users to a project so they become members and have permission -to perform actions. +to perform actions. The Owner [role](../../permissions.md#project-members-permissions) can only be added at the group level. Prerequisite: diff --git a/lib/gitlab/database/background_migration/batched_job.rb b/lib/gitlab/database/background_migration/batched_job.rb index feffa2b33aa..ebc3ee240bd 100644 --- a/lib/gitlab/database/background_migration/batched_job.rb +++ b/lib/gitlab/database/background_migration/batched_job.rb @@ -63,7 +63,13 @@ module Gitlab job.split_and_retry! if job.can_split?(exception) rescue SplitAndRetryError => error - Gitlab::AppLogger.error(message: error.message, batched_job_id: job.id) + Gitlab::AppLogger.error( + message: error.message, + batched_job_id: job.id, + batched_migration_id: job.batched_migration.id, + job_class_name: job.migration_job_class_name, + job_arguments: job.migration_job_arguments + ) end after_transition do |job, transition| @@ -73,13 +79,23 @@ module Gitlab job.batched_job_transition_logs.create(previous_status: transition.from, next_status: transition.to, exception_class: exception&.class, exception_message: exception&.message) - Gitlab::ErrorTracking.track_exception(exception, batched_job_id: job.id) if exception - - Gitlab::AppLogger.info(message: 'BatchedJob transition', batched_job_id: job.id, previous_state: transition.from_name, new_state: transition.to_name) + Gitlab::ErrorTracking.track_exception(exception, batched_job_id: job.id, job_class_name: job.migration_job_class_name, job_arguments: job.migration_job_arguments) if exception + + Gitlab::AppLogger.info( + message: 'BatchedJob transition', + batched_job_id: job.id, + previous_state: transition.from_name, + new_state: transition.to_name, + batched_migration_id: job.batched_migration.id, + job_class_name: job.migration_job_class_name, + job_arguments: job.migration_job_arguments, + exception_class: exception&.class, + exception_message: exception&.message + ) end end - delegate :job_class, :table_name, :column_name, :job_arguments, + delegate :job_class, :table_name, :column_name, :job_arguments, :job_class_name, to: :batched_migration, prefix: :migration attribute :pause_ms, :integer, default: 100 diff --git a/lib/gitlab/security/scan_configuration.rb b/lib/gitlab/security/scan_configuration.rb index 381adda7991..14883a34950 100644 --- a/lib/gitlab/security/scan_configuration.rb +++ b/lib/gitlab/security/scan_configuration.rb @@ -31,6 +31,8 @@ module Gitlab def configuration_path; end + def meta_info_path; end + private attr_reader :project, :configured diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 51d2091a165..31f97b0a13a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4220,9 +4220,6 @@ msgstr "" msgid "Analytics" msgstr "" -msgid "Analyze a review version of your web application." -msgstr "" - msgid "Analyze your dependencies for known vulnerabilities." msgstr "" @@ -5455,6 +5452,9 @@ msgstr "" msgid "Available group runners: %{runners}" msgstr "" +msgid "Available on-demand" +msgstr "" + msgid "Available runners: %{runners}" msgstr "" @@ -21295,9 +21295,6 @@ msgstr "" msgid "Iteration|cannot be more than 500 years in the future" msgstr "" -msgid "I’m joining my team who’s already on GitLab" -msgstr "" - msgid "Jaeger URL" msgstr "" @@ -25992,6 +25989,9 @@ msgstr "" msgid "On-call schedules" msgstr "" +msgid "On-demand scans run outside of the DevOps cycle and find vulnerabilities in your projects" +msgstr "" + msgid "OnCallScheduless|Any escalation rules that are using this schedule will also be deleted." msgstr "" @@ -44123,6 +44123,9 @@ msgstr "" msgid "ciReport|All tools" msgstr "" +msgid "ciReport|Analyze a deployed version of your web application for known vulnerabilities by examining it from the outside in. DAST works by simulating external attacks on your application while it is running." +msgstr "" + msgid "ciReport|Automatically apply the patch in a new branch" msgstr "" diff --git a/spec/features/projects/wikis_spec.rb b/spec/features/projects/wikis_spec.rb index e253d03b411..879ffd2932b 100644 --- a/spec/features/projects/wikis_spec.rb +++ b/spec/features/projects/wikis_spec.rb @@ -8,26 +8,14 @@ RSpec.describe 'Project wikis', :js do let(:wiki) { create(:project_wiki, user: user, project: project) } let(:project) { create(:project, namespace: user.namespace, creator: user) } - shared_examples 'wiki feature tests' do - it_behaves_like 'User creates wiki page' - it_behaves_like 'User deletes wiki page' - it_behaves_like 'User previews wiki changes' - it_behaves_like 'User updates wiki page' - it_behaves_like 'User uses wiki shortcuts' - it_behaves_like 'User views AsciiDoc page with includes' - it_behaves_like 'User views a wiki page' - it_behaves_like 'User views wiki pages' - it_behaves_like 'User views wiki sidebar' - it_behaves_like 'User views Git access wiki page' - end - - it_behaves_like 'wiki feature tests' - - context 'when feature flag :wiki_async_load is disabled' do - before do - stub_feature_flags(wiki_async_load: false) - end - - it_behaves_like 'wiki feature tests' - end + it_behaves_like 'User creates wiki page' + it_behaves_like 'User deletes wiki page' + it_behaves_like 'User previews wiki changes' + it_behaves_like 'User updates wiki page' + it_behaves_like 'User uses wiki shortcuts' + it_behaves_like 'User views AsciiDoc page with includes' + it_behaves_like 'User views a wiki page' + it_behaves_like 'User views wiki pages' + it_behaves_like 'User views wiki sidebar' + it_behaves_like 'User views Git access wiki page' end diff --git a/spec/frontend/security_configuration/components/feature_card_badge_spec.js b/spec/frontend/security_configuration/components/feature_card_badge_spec.js new file mode 100644 index 00000000000..dcde0808fa4 --- /dev/null +++ b/spec/frontend/security_configuration/components/feature_card_badge_spec.js @@ -0,0 +1,40 @@ +import { mount } from '@vue/test-utils'; +import { GlBadge, GlTooltip } from '@gitlab/ui'; +import { extendedWrapper } from 'helpers/vue_test_utils_helper'; +import FeatureCardBadge from '~/security_configuration/components/feature_card_badge.vue'; + +describe('Feature card badge component', () => { + let wrapper; + + const createComponent = (propsData) => { + wrapper = extendedWrapper( + mount(FeatureCardBadge, { + propsData, + }), + ); + }; + + const findTooltip = () => wrapper.findComponent(GlTooltip); + const findBadge = () => wrapper.findComponent(GlBadge); + + describe('tooltip render', () => { + describe.each` + context | badge | badgeHref + ${'href on a badge object'} | ${{ tooltipText: 'test', badgeHref: 'href' }} | ${undefined} + ${'href as property '} | ${{ tooltipText: null, badgeHref: '' }} | ${'link'} + ${'default href no property on badge or component'} | ${{ tooltipText: null, badgeHref: '' }} | ${undefined} + `('given $context', ({ badge, badgeHref }) => { + beforeEach(() => { + createComponent({ badge, badgeHref }); + }); + + it('should show badge when badge given in configuration and available', () => { + expect(findTooltip().exists()).toBe(Boolean(badge && badge.tooltipText)); + }); + + it('should render correct link if link is provided', () => { + expect(findBadge().attributes().href).toEqual(badgeHref); + }); + }); + }); +}); diff --git a/spec/frontend/security_configuration/components/feature_card_spec.js b/spec/frontend/security_configuration/components/feature_card_spec.js index f0d902bf9fe..d10722be8ea 100644 --- a/spec/frontend/security_configuration/components/feature_card_spec.js +++ b/spec/frontend/security_configuration/components/feature_card_spec.js @@ -2,6 +2,7 @@ import { GlIcon } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import FeatureCard from '~/security_configuration/components/feature_card.vue'; +import FeatureCardBadge from '~/security_configuration/components/feature_card_badge.vue'; import ManageViaMr from '~/vue_shared/security_configuration/components/manage_via_mr.vue'; import { REPORT_TYPE_SAST } from '~/vue_shared/security_reports/constants'; import { makeFeature } from './utils'; @@ -16,6 +17,7 @@ describe('FeatureCard component', () => { propsData, stubs: { ManageViaMr: true, + FeatureCardBadge: true, }, }), ); @@ -24,6 +26,8 @@ describe('FeatureCard component', () => { const findLinks = ({ text, href }) => wrapper.findAll(`a[href="${href}"]`).filter((link) => link.text() === text); + const findBadge = () => wrapper.findComponent(FeatureCardBadge); + const findEnableLinks = () => findLinks({ text: `Enable ${feature.shortName ?? feature.name}`, @@ -262,5 +266,28 @@ describe('FeatureCard component', () => { }); }); }); + + describe('information badge', () => { + describe.each` + context | available | badge + ${'available feature with badge'} | ${true} | ${{ text: 'test' }} + ${'unavailable feature without badge'} | ${false} | ${null} + ${'available feature without badge'} | ${true} | ${null} + ${'unavailable feature with badge'} | ${false} | ${{ text: 'test' }} + ${'available feature with empty badge'} | ${false} | ${{}} + `('given $context', ({ available, badge }) => { + beforeEach(() => { + feature = makeFeature({ + available, + badge, + }); + createComponent({ feature }); + }); + + it('should show badge when badge given in configuration and available', () => { + expect(findBadge().exists()).toBe(Boolean(available && badge && badge.text)); + }); + }); + }); }); }); diff --git a/spec/lib/gitlab/database/background_migration/batched_job_spec.rb b/spec/lib/gitlab/database/background_migration/batched_job_spec.rb index c2093cf5724..c39f6a78e93 100644 --- a/spec/lib/gitlab/database/background_migration/batched_job_spec.rb +++ b/spec/lib/gitlab/database/background_migration/batched_job_spec.rb @@ -21,7 +21,19 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d context 'when a job is running' do it 'logs the transition' do - expect(Gitlab::AppLogger).to receive(:info).with( { batched_job_id: job.id, message: 'BatchedJob transition', new_state: :running, previous_state: :failed } ) + expect(Gitlab::AppLogger).to receive(:info).with( + { + batched_job_id: job.id, + batched_migration_id: job.batched_background_migration_id, + exception_class: nil, + exception_message: nil, + job_arguments: job.batched_migration.job_arguments, + job_class_name: job.batched_migration.job_class_name, + message: 'BatchedJob transition', + new_state: :running, + previous_state: :failed + } + ) expect { job.run! }.to change(job, :started_at) end @@ -31,7 +43,19 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d let(:job) { create(:batched_background_migration_job, :running) } it 'logs the transition' do - expect(Gitlab::AppLogger).to receive(:info).with( { batched_job_id: job.id, message: 'BatchedJob transition', new_state: :succeeded, previous_state: :running } ) + expect(Gitlab::AppLogger).to receive(:info).with( + { + batched_job_id: job.id, + batched_migration_id: job.batched_background_migration_id, + exception_class: nil, + exception_message: nil, + job_arguments: job.batched_migration.job_arguments, + job_class_name: job.batched_migration.job_class_name, + message: 'BatchedJob transition', + new_state: :succeeded, + previous_state: :running + } + ) job.succeed! end @@ -89,7 +113,15 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d end it 'logs the error' do - expect(Gitlab::AppLogger).to receive(:error).with( { message: error_message, batched_job_id: job.id } ) + expect(Gitlab::AppLogger).to receive(:error).with( + { + batched_job_id: job.id, + batched_migration_id: job.batched_background_migration_id, + job_arguments: job.batched_migration.job_arguments, + job_class_name: job.batched_migration.job_class_name, + message: error_message + } + ) job.failure!(error: exception) end @@ -100,13 +132,32 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d let(:job) { create(:batched_background_migration_job, :running) } it 'logs the transition' do - expect(Gitlab::AppLogger).to receive(:info).with( { batched_job_id: job.id, message: 'BatchedJob transition', new_state: :failed, previous_state: :running } ) + expect(Gitlab::AppLogger).to receive(:info).with( + { + batched_job_id: job.id, + batched_migration_id: job.batched_background_migration_id, + exception_class: RuntimeError, + exception_message: 'error', + job_arguments: job.batched_migration.job_arguments, + job_class_name: job.batched_migration.job_class_name, + message: 'BatchedJob transition', + new_state: :failed, + previous_state: :running + } + ) - job.failure! + job.failure!(error: RuntimeError.new('error')) end it 'tracks the exception' do - expect(Gitlab::ErrorTracking).to receive(:track_exception).with(RuntimeError, { batched_job_id: job.id } ) + expect(Gitlab::ErrorTracking).to receive(:track_exception).with( + RuntimeError, + { + batched_job_id: job.id, + job_arguments: job.batched_migration.job_arguments, + job_class_name: job.batched_migration.job_class_name + } + ) job.failure!(error: RuntimeError.new) end @@ -198,6 +249,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d expect(batched_job.migration_job_arguments).to eq(batched_migration.job_arguments) end end + + describe '#migration_job_class_name' do + it 'returns the migration job_class_name' do + expect(batched_job.migration_job_class_name).to eq(batched_migration.job_class_name) + end + end end describe '#can_split?' do diff --git a/spec/lib/gitlab/security/scan_configuration_spec.rb b/spec/lib/gitlab/security/scan_configuration_spec.rb index 2e8a11dfda3..1760796c5a0 100644 --- a/spec/lib/gitlab/security/scan_configuration_spec.rb +++ b/spec/lib/gitlab/security/scan_configuration_spec.rb @@ -47,6 +47,16 @@ RSpec.describe ::Gitlab::Security::ScanConfiguration do it { is_expected.to be_nil } end + describe '#meta_info_path' do + subject { scan.meta_info_path } + + let(:configured) { true } + let(:available) { true } + let(:type) { :dast } + + it { is_expected.to be_nil } + end + describe '#can_enable_by_merge_request?' do subject { scan.can_enable_by_merge_request? } diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index b64f12d39a7..541fa1ac77a 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -1322,4 +1322,19 @@ RSpec.describe ApplicationSetting do end end end + + context "inactive project deletion" do + it "validates that inactive_projects_send_warning_email_after_months is less than inactive_projects_delete_after_months" do + subject[:inactive_projects_delete_after_months] = 3 + subject[:inactive_projects_send_warning_email_after_months] = 6 + + expect(subject).to be_invalid + end + + it { is_expected.to validate_numericality_of(:inactive_projects_send_warning_email_after_months).is_greater_than(0) } + + it { is_expected.to validate_numericality_of(:inactive_projects_delete_after_months).is_greater_than(0) } + + it { is_expected.to validate_numericality_of(:inactive_projects_min_size_mb).is_greater_than_or_equal_to(0) } + end end diff --git a/spec/presenters/projects/security/configuration_presenter_spec.rb b/spec/presenters/projects/security/configuration_presenter_spec.rb index 47ef0cf1192..779d6b88fd5 100644 --- a/spec/presenters/projects/security/configuration_presenter_spec.rb +++ b/spec/presenters/projects/security/configuration_presenter_spec.rb @@ -87,6 +87,7 @@ RSpec.describe Projects::Security::ConfigurationPresenter do expect(feature['configuration_path']).to be_nil expect(feature['available']).to eq(true) expect(feature['can_enable_by_merge_request']).to eq(true) + expect(feature['meta_info_path']).to be_nil end context 'when checking features configured status' do diff --git a/workhorse/internal/git/info-refs.go b/workhorse/internal/git/info-refs.go index 8390143b99b..b7f825839f8 100644 --- a/workhorse/internal/git/info-refs.go +++ b/workhorse/internal/git/info-refs.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "sync" "github.com/golang/gddo/httputil" grpccodes "google.golang.org/grpc/codes" @@ -64,21 +65,43 @@ func handleGetInfoRefsWithGitaly(ctx context.Context, responseWriter *HttpRespon return err } - var w io.Writer - + var w io.WriteCloser = nopCloser{responseWriter} if encoding == "gzip" { - gzWriter := gzip.NewWriter(responseWriter) - w = gzWriter - defer gzWriter.Close() + gzWriter := getGzWriter(responseWriter) + defer putGzWriter(gzWriter) + w = gzWriter responseWriter.Header().Set("Content-Encoding", "gzip") - } else { - w = responseWriter } if _, err = io.Copy(w, infoRefsResponseReader); err != nil { return err } + if err := w.Close(); err != nil { + return err + } + return nil } + +var gzipPool = &sync.Pool{New: func() interface{} { + // Invariant: the inner writer is io.Discard. We do not want to retain + // response writers of past requests in the pool. + return gzip.NewWriter(io.Discard) +}} + +func getGzWriter(w io.Writer) *gzip.Writer { + gzWriter := gzipPool.Get().(*gzip.Writer) + gzWriter.Reset(w) + return gzWriter +} + +func putGzWriter(w *gzip.Writer) { + w.Reset(io.Discard) // Maintain pool invariant + gzipPool.Put(w) +} + +type nopCloser struct{ io.Writer } + +func (nc nopCloser) Close() error { return nil } |