diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-03-11 21:09:09 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-03-11 21:09:09 +0000 |
| commit | 19d63dbca44d8380b8bc0d51d890137691ddf2a7 (patch) | |
| tree | 534885124a1edfcf4c22f61b93f6f8d270e03ac3 | |
| parent | 9d195600e6266da44917f08c622a21a6d4c2317b (diff) | |
| download | gitlab-ce-19d63dbca44d8380b8bc0d51d890137691ddf2a7.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
79 files changed, 1478 insertions, 1308 deletions
diff --git a/.gitlab/issue_templates/Experiment Successful Cleanup.md b/.gitlab/issue_templates/Experiment Successful Cleanup.md index b84bf1066f6..afe4793cdfc 100644 --- a/.gitlab/issue_templates/Experiment Successful Cleanup.md +++ b/.gitlab/issue_templates/Experiment Successful Cleanup.md @@ -9,10 +9,10 @@ The changes need to become an official part of the product. - [ ] Determine whether the feature should apply to SaaS and/or self-managed - [ ] Determine whether the feature should apply to EE - and which tiers - and/or Core -- [ ] Determine if tracking should be kept as is, removed, or modified +- [ ] Determine if tracking should be kept as is, removed, or modified. - [ ] Ensure any relevant documentation has been updated. - [ ] Consider changes to any `feature_category:` introduced by the experiment if ownership is changing (PM for Growth and PM for the new category as DRIs) -- [ ] Optional: Migrate experiment to a default enabled [feature flag](https://docs.gitlab.com/ee/development/feature_flags/development.html) for one milestone and add a changelog. Converting to a feature flag can be skipped at the ICs discretion if risk is deemed low with consideration to both SaaS and (if applicable) self managed +- [ ] Optional: Migrate experiment to a default enabled [feature flag](https://docs.gitlab.com/ee/development/feature_flags) for one milestone and add a changelog. Converting to a feature flag can be skipped at the ICs discretion if risk is deemed low with consideration to both SaaS and (if applicable) self managed - [ ] In the next milestone, [remove the feature flag](https://docs.gitlab.com/ee/development/feature_flags/controls.html#cleaning-up) if applicable - [ ] After the flag removal is deployed, [clean up the feature/experiment feature flags](https://docs.gitlab.com/ee/development/feature_flags/controls.html#cleaning-up) by running chatops command in `#production` channel @@ -485,7 +485,7 @@ gem 'flipper', '~> 0.17.1' gem 'flipper-active_record', '~> 0.17.1' gem 'flipper-active_support_cache_store', '~> 0.17.1' gem 'unleash', '~> 0.1.5' -gem 'gitlab-experiment', '~> 0.4.12' +gem 'gitlab-experiment', '~> 0.5.0' # Structured logging gem 'lograge', '~> 0.5' diff --git a/Gemfile.lock b/Gemfile.lock index ace2442e026..e637ef37e8e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -430,7 +430,7 @@ GEM github-markup (1.7.0) gitlab-chronic (0.10.5) numerizer (~> 0.2) - gitlab-experiment (0.4.12) + gitlab-experiment (0.5.0) activesupport (>= 3.0) scientist (~> 1.5, >= 1.5.0) gitlab-fog-azure-rm (1.0.1) @@ -1140,7 +1140,7 @@ GEM sawyer (0.8.2) addressable (>= 2.3.5) faraday (> 0.8, < 2.0) - scientist (1.5.0) + scientist (1.6.0) securecompare (1.0.0) seed-fu (2.3.7) activerecord (>= 3.1) @@ -1413,7 +1413,7 @@ DEPENDENCIES gitaly (~> 13.9.0.pre.rc1) github-markup (~> 1.7.0) gitlab-chronic (~> 0.10.5) - gitlab-experiment (~> 0.4.12) + gitlab-experiment (~> 0.5.0) gitlab-fog-azure-rm (~> 1.0.1) gitlab-fog-google (~> 1.13) gitlab-labkit (~> 0.16.0) diff --git a/PROCESS.md b/PROCESS.md index 820f19a290b..67abe2f0a98 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -77,9 +77,9 @@ star, smile, etc.). Some good tips about code reviews can be found in our ## Feature flags -Overview and details of feature flag processes in development of GitLab itself is described in [feature flags process documentation](https://docs.gitlab.com/ee/development/feature_flags/process.html). +Overview and details of feature flag processes in development of GitLab itself is described in [feature flags process documentation](https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle/). -Guides on how to include feature flags in your backend/frontend code while developing GitLab are described in [developing with feature flags documentation](https://docs.gitlab.com/ee/development/feature_flags/development.html). +Guides on how to include feature flags in your backend/frontend code while developing GitLab are described in [developing with feature flags documentation](https://docs.gitlab.com/ee/development/feature_flags). Getting access and how to expose the feature to users is detailed in [controlling feature flags documentation](https://docs.gitlab.com/ee/development/feature_flags/controls.html). diff --git a/app/assets/javascripts/batch_comments/stores/modules/batch_comments/actions.js b/app/assets/javascripts/batch_comments/stores/modules/batch_comments/actions.js index 36fef06eeff..88be64d0a1a 100644 --- a/app/assets/javascripts/batch_comments/stores/modules/batch_comments/actions.js +++ b/app/assets/javascripts/batch_comments/stores/modules/batch_comments/actions.js @@ -1,3 +1,4 @@ +import { isEmpty } from 'lodash'; import { deprecatedCreateFlash as flash } from '~/flash'; import { scrollToElement } from '~/lib/utils/common_utils'; import { __ } from '~/locale'; @@ -88,18 +89,23 @@ export const updateDiscussionsAfterPublish = async ({ dispatch, getters, rootGet export const updateDraft = ( { commit, getters }, { note, noteText, resolveDiscussion, position, callback }, -) => - service - .update(getters.getNotesData.draftsPath, { - draftId: note.id, - note: noteText, - resolveDiscussion, - position: JSON.stringify(position), - }) +) => { + const params = { + draftId: note.id, + note: noteText, + resolveDiscussion, + }; + // Stringifying an empty object yields `{}` which breaks graphql queries + // https://gitlab.com/gitlab-org/gitlab/-/issues/298827 + if (!isEmpty(position)) params.position = JSON.stringify(position); + + return service + .update(getters.getNotesData.draftsPath, params) .then((res) => res.data) .then((data) => commit(types.RECEIVE_DRAFT_UPDATE_SUCCESS, data)) .then(callback) .catch(() => flash(__('An error occurred while updating the comment'))); +}; export const scrollToDraft = ({ dispatch, rootGetters }, draft) => { const discussion = draft.discussion_id && rootGetters.getDiscussion(draft.discussion_id); diff --git a/app/assets/javascripts/environments/components/environments_app.vue b/app/assets/javascripts/environments/components/environments_app.vue index 3eabc10cd22..602639f09a6 100644 --- a/app/assets/javascripts/environments/components/environments_app.vue +++ b/app/assets/javascripts/environments/components/environments_app.vue @@ -129,7 +129,7 @@ export default { :href="newEnvironmentPath" data-testid="new-environment" category="primary" - variant="success" + variant="confirm" >{{ $options.i18n.newEnvironmentButtonLabel }}</gl-button > </div> @@ -164,7 +164,7 @@ export default { :href="newEnvironmentPath" data-testid="new-environment" category="primary" - variant="success" + variant="confirm" >{{ $options.i18n.newEnvironmentButtonLabel }}</gl-button > </div> diff --git a/app/assets/javascripts/issuable/components/csv_import_export_buttons.vue b/app/assets/javascripts/issuable/components/csv_import_export_buttons.vue index 8f18902b590..d75fa1e8323 100644 --- a/app/assets/javascripts/issuable/components/csv_import_export_buttons.vue +++ b/app/assets/javascripts/issuable/components/csv_import_export_buttons.vue @@ -66,7 +66,6 @@ export default { <gl-dropdown v-if="showImportButton" v-gl-tooltip.hover="__('Import issues')" - data-qa-selector="import_issues_dropdown" data-testid="import-csv-dropdown" icon="import" > diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 9bf1496f479..185f4a70367 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -1,7 +1,7 @@ <script> import { GlSprintf, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import $ from 'jquery'; -import { escape } from 'lodash'; +import { escape, isEmpty } from 'lodash'; import { mapGetters, mapActions } from 'vuex'; import { INLINE_DIFF_LINES_KEY } from '~/diffs/constants'; import httpStatusCodes from '~/lib/utils/http_status'; @@ -282,9 +282,13 @@ export default { note: { target_type: this.getNoteableData.targetType, target_id: this.note.noteable_id, - note: { note: noteText, position: JSON.stringify(position) }, + note: { note: noteText }, }, }; + + // Stringifying an empty object yields `{}` which breaks graphql queries + // https://gitlab.com/gitlab-org/gitlab/-/issues/298827 + if (!isEmpty(position)) data.note.note.position = JSON.stringify(position); this.isRequesting = true; this.oldContent = this.note.note_html; // eslint-disable-next-line vue/no-mutating-props diff --git a/app/assets/javascripts/pages/dashboard/todos/index/index.js b/app/assets/javascripts/pages/dashboard/todos/index/index.js index 9d2c2f2994f..2fe90c24e77 100644 --- a/app/assets/javascripts/pages/dashboard/todos/index/index.js +++ b/app/assets/javascripts/pages/dashboard/todos/index/index.js @@ -1,3 +1,3 @@ import Todos from './todos'; -document.addEventListener('DOMContentLoaded', () => new Todos()); +new Todos(); // eslint-disable-line no-new diff --git a/app/experiments/application_experiment.rb b/app/experiments/application_experiment.rb index 6ba851fbc8b..019e74bcd48 100644 --- a/app/experiments/application_experiment.rb +++ b/app/experiments/application_experiment.rb @@ -9,11 +9,15 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp Feature.get(feature_flag_name).state != :off # rubocop:disable Gitlab/AvoidFeatureGet end - def publish(_result) + def publish(_result = nil) track(:assignment) # track that we've assigned a variant for this context # push the experiment data to the client - Gon.push({ experiment: { name => signature } }, true) if in_request_cycle? + begin + Gon.push({ experiment: { name => signature } }, true) # push the experiment data to the client + rescue NoMethodError + # means we're not in the request cycle, and can't add to Gon. Log a warning maybe? + end end def track(action, **event_args) @@ -49,12 +53,6 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp name.tr('/', '_') end - def in_request_cycle? - # Gon is only accessible when having a request. This will be fixed with - # https://gitlab.com/gitlab-org/gitlab/-/issues/323352 - context.instance_variable_defined?(:@request) - end - def resolve_variant_name case rollout_strategy when :round_robin diff --git a/app/graphql/mutations/boards/create.rb b/app/graphql/mutations/boards/create.rb index fe324f848e7..003c4f7761b 100644 --- a/app/graphql/mutations/boards/create.rb +++ b/app/graphql/mutations/boards/create.rb @@ -22,7 +22,7 @@ module Mutations response = ::Boards::CreateService.new(board_parent, current_user, args).execute { - board: response.payload, + board: response.success? ? response.payload : nil, errors: response.errors } end diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index 9059f61b5de..23a7144e2bb 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -30,6 +30,10 @@ class Packages::PackageFile < ApplicationRecord scope :preload_conan_file_metadata, -> { preload(:conan_file_metadatum) } scope :preload_debian_file_metadata, -> { preload(:debian_file_metadatum) } + scope :for_rubygem_with_file_name, ->(project, file_name) do + joins(:package).merge(project.packages.rubygems).with_file_name(file_name) + end + scope :with_conan_file_type, ->(file_type) do joins(:conan_file_metadatum) .where(packages_conan_file_metadata: { conan_file_type: ::Packages::Conan::FileMetadatum.conan_file_types[file_type] }) diff --git a/app/models/wiki.rb b/app/models/wiki.rb index 45747c0b03c..abaa4e05d74 100644 --- a/app/models/wiki.rb +++ b/app/models/wiki.rb @@ -159,7 +159,7 @@ class Wiki find_page(SIDEBAR, version) end - def find_file(name, version = nil) + def find_file(name, version = 'HEAD') wiki.file(name, version) end diff --git a/app/views/clusters/clusters/_advanced_settings.html.haml b/app/views/clusters/clusters/_advanced_settings.html.haml index bbdbda40297..7f508fd0a59 100644 --- a/app/views/clusters/clusters/_advanced_settings.html.haml +++ b/app/views/clusters/clusters/_advanced_settings.html.haml @@ -24,7 +24,7 @@ .text-muted = html_escape(s_('ClusterIntegration|A cluster management project can be used to run deployment jobs with Kubernetes %{code_open}cluster-admin%{code_close} privileges.')) % { code_open: '<code>'.html_safe, code_close: '</code>'.html_safe } = link_to _('More information'), help_page_path('user/clusters/management_project.md'), target: '_blank' - = field.submit _('Save changes'), class: 'btn gl-button btn-success' + = field.submit _('Save changes'), class: 'btn gl-button btn-confirm' .sub-section.form-group %h4 diff --git a/app/views/clusters/clusters/_cluster_list.html.haml b/app/views/clusters/clusters/_cluster_list.html.haml index 9627d940126..38ed7e334c9 100644 --- a/app/views/clusters/clusters/_cluster_list.html.haml +++ b/app/views/clusters/clusters/_cluster_list.html.haml @@ -4,9 +4,9 @@ .top-area.adjust .gl-display-block.gl-text-right.gl-my-4.gl-w-full - if clusterable.can_add_cluster? - = link_to s_('ClusterIntegration|Connect cluster with certificate'), clusterable.new_path, class: 'btn gl-button btn-success js-add-cluster gl-py-2', qa_selector: :integrate_kubernetes_cluster_button + = link_to s_('ClusterIntegration|Connect cluster with certificate'), clusterable.new_path, class: 'btn gl-button btn-confirm js-add-cluster gl-py-2', qa_selector: :integrate_kubernetes_cluster_button - else - %span.btn.gl-button.btn-success.js-add-cluster.disabled.gl-py-2 + %span.btn.gl-button.btn-confirm.js-add-cluster.disabled.gl-py-2 = s_("ClusterIntegration|Connect cluster with certificate") #js-clusters-list-app{ data: js_clusters_list_data(clusterable.index_path(format: :json)) } diff --git a/app/views/clusters/clusters/_empty_state.html.haml b/app/views/clusters/clusters/_empty_state.html.haml index 257944f5e65..feef3e0027f 100644 --- a/app/views/clusters/clusters/_empty_state.html.haml +++ b/app/views/clusters/clusters/_empty_state.html.haml @@ -11,4 +11,4 @@ - if clusterable.can_add_cluster? .gl-text-center - = link_to s_('ClusterIntegration|Integrate with a cluster certificate'), clusterable.new_path, class: 'gl-button btn btn-success', data: { qa_selector: 'add_kubernetes_cluster_link' } + = link_to s_('ClusterIntegration|Integrate with a cluster certificate'), clusterable.new_path, class: 'gl-button btn btn-confirm', data: { qa_selector: 'add_kubernetes_cluster_link' } diff --git a/app/views/clusters/clusters/_provider_details_form.html.haml b/app/views/clusters/clusters/_provider_details_form.html.haml index 7fc803e9579..a936cdc04dd 100644 --- a/app/views/clusters/clusters/_provider_details_form.html.haml +++ b/app/views/clusters/clusters/_provider_details_form.html.haml @@ -55,4 +55,4 @@ = render('clusters/clusters/namespace', platform_field: platform_field, field: field) .form-group - = field.submit s_('ClusterIntegration|Save changes'), class: 'gl-button btn btn-success' + = field.submit s_('ClusterIntegration|Save changes'), class: 'gl-button btn btn-confirm' diff --git a/app/views/clusters/clusters/gcp/_form.html.haml b/app/views/clusters/clusters/gcp/_form.html.haml index 50e3d29f974..ee2817879be 100644 --- a/app/views/clusters/clusters/gcp/_form.html.haml +++ b/app/views/clusters/clusters/gcp/_form.html.haml @@ -85,4 +85,4 @@ .form-group.js-gke-cluster-creation-submit-container = field.submit s_('ClusterIntegration|Create Kubernetes cluster'), - class: 'js-gke-cluster-creation-submit gl-button btn btn-success', disabled: true + class: 'js-gke-cluster-creation-submit gl-button btn btn-confirm', disabled: true diff --git a/app/views/clusters/clusters/user/_form.html.haml b/app/views/clusters/clusters/user/_form.html.haml index e2779d1b683..7d82fe06799 100644 --- a/app/views/clusters/clusters/user/_form.html.haml +++ b/app/views/clusters/clusters/user/_form.html.haml @@ -60,4 +60,4 @@ = render('clusters/clusters/namespace', platform_field: platform_kubernetes_field) .form-group - = field.submit s_('ClusterIntegration|Add Kubernetes cluster'), class: 'gl-button btn btn-success', data: { qa_selector: 'add_kubernetes_cluster_button' } + = field.submit s_('ClusterIntegration|Add Kubernetes cluster'), class: 'gl-button btn btn-confirm', data: { qa_selector: 'add_kubernetes_cluster_button' } diff --git a/changelogs/unreleased/298827-graphql-getting-mr-diff-discussions-often-returns-500.yml b/changelogs/unreleased/298827-graphql-getting-mr-diff-discussions-often-returns-500.yml new file mode 100644 index 00000000000..c994a7d11aa --- /dev/null +++ b/changelogs/unreleased/298827-graphql-getting-mr-diff-discussions-often-returns-500.yml @@ -0,0 +1,5 @@ +--- +title: fix stringify empty position object +merge_request: 56037 +author: +type: fixed diff --git a/changelogs/unreleased/board_args_consolidation.yml b/changelogs/unreleased/board_args_consolidation.yml new file mode 100644 index 00000000000..e62b49fe06a --- /dev/null +++ b/changelogs/unreleased/board_args_consolidation.yml @@ -0,0 +1,5 @@ +--- +title: Fixed error handling GraphQL API when issue board creation fails +merge_request: 55982 +author: +type: fixed diff --git a/changelogs/unreleased/btn-confirm-clusters.yml b/changelogs/unreleased/btn-confirm-clusters.yml new file mode 100644 index 00000000000..fe1b0d732dc --- /dev/null +++ b/changelogs/unreleased/btn-confirm-clusters.yml @@ -0,0 +1,5 @@ +--- +title: Move from btn-success to btn-confirm in clusters directory +merge_request: 56205 +author: Yogi (@yo) +type: changed diff --git a/changelogs/unreleased/btn-confirm-envs.yml b/changelogs/unreleased/btn-confirm-envs.yml new file mode 100644 index 00000000000..00331aa31f6 --- /dev/null +++ b/changelogs/unreleased/btn-confirm-envs.yml @@ -0,0 +1,5 @@ +--- +title: Move to confirm variant from success in environments directory +merge_request: 56203 +author: Yogi (@yo) +type: changed diff --git a/config/initializers/rack_multipart_patch.rb b/config/initializers/rack_multipart_patch.rb index e664827e15e..78cf6244814 100644 --- a/config/initializers/rack_multipart_patch.rb +++ b/config/initializers/rack_multipart_patch.rb @@ -13,7 +13,7 @@ module Rack def log_multipart_warning(req) content_length = req.content_length.to_i - return unless content_length > 500_000_000 + return unless content_length > log_threshold message = { message: "Large multipart body detected", @@ -32,6 +32,10 @@ module Rack def log_large_multipart? Gitlab::Utils.to_boolean(ENV['ENABLE_RACK_MULTIPART_LOGGING'], default: true) && Gitlab.com? end + + def log_threshold + ENV.fetch('RACK_MULTIPART_LOGGING_BYTES', 100_000_000).to_i + end end prepend MultipartPatch diff --git a/danger/feature_flag/Dangerfile b/danger/feature_flag/Dangerfile index c90f60640f2..e66ed35c9ab 100644 --- a/danger/feature_flag/Dangerfile +++ b/danger/feature_flag/Dangerfile @@ -1,7 +1,7 @@ # frozen_string_literal: true # rubocop:disable Style/SignalException -SEE_DOC = "See the [feature flag documentation](https://docs.gitlab.com/ee/development/feature_flags/development.html#feature-flag-definition-and-validation)." +SEE_DOC = "See the [feature flag documentation](https://docs.gitlab.com/ee/development/feature_flags#feature-flag-definition-and-validation)." SUGGEST_MR_COMMENT = <<~SUGGEST_COMMENT ```suggestion diff --git a/db/migrate/20210301200601_rename_asset_proxy_allowlist_on_application_settings.rb b/db/migrate/20210301200601_rename_asset_proxy_allowlist_on_application_settings.rb index 8a9acd8b86e..8ac334fc6a4 100644 --- a/db/migrate/20210301200601_rename_asset_proxy_allowlist_on_application_settings.rb +++ b/db/migrate/20210301200601_rename_asset_proxy_allowlist_on_application_settings.rb @@ -8,6 +8,10 @@ class RenameAssetProxyAllowlistOnApplicationSettings < ActiveRecord::Migration[6 disable_ddl_transaction! def up + cleanup_concurrent_column_rename :application_settings, + :asset_proxy_whitelist, + :asset_proxy_allowlist + rename_column_concurrently :application_settings, :asset_proxy_allowlist, :asset_proxy_whitelist @@ -17,5 +21,9 @@ class RenameAssetProxyAllowlistOnApplicationSettings < ActiveRecord::Migration[6 undo_rename_column_concurrently :application_settings, :asset_proxy_allowlist, :asset_proxy_whitelist + + undo_cleanup_concurrent_column_rename :application_settings, + :asset_proxy_whitelist, + :asset_proxy_allowlist end end diff --git a/db/post_migrate/20210105052229_clean_up_asset_proxy_whitelist_rename_on_application_settings.rb b/db/post_migrate/20210105052229_clean_up_asset_proxy_whitelist_rename_on_application_settings.rb index 8f6d312ae5e..87f391e240d 100644 --- a/db/post_migrate/20210105052229_clean_up_asset_proxy_whitelist_rename_on_application_settings.rb +++ b/db/post_migrate/20210105052229_clean_up_asset_proxy_whitelist_rename_on_application_settings.rb @@ -8,14 +8,12 @@ class CleanUpAssetProxyWhitelistRenameOnApplicationSettings < ActiveRecord::Migr disable_ddl_transaction! def up - cleanup_concurrent_column_rename :application_settings, - :asset_proxy_whitelist, - :asset_proxy_allowlist + # This migration has been made a no-op in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56352 + # because to revert the rename in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55419 we need + # to cleanup the triggers on the `asset_proxy_allowlist` column. As such, this migration would do nothing. end def down - undo_cleanup_concurrent_column_rename :application_settings, - :asset_proxy_whitelist, - :asset_proxy_allowlist + # no-op end end diff --git a/doc/administration/auth/ldap/index.md b/doc/administration/auth/ldap/index.md index 1ce9dd79caf..0e55efba8ae 100644 --- a/doc/administration/auth/ldap/index.md +++ b/doc/administration/auth/ldap/index.md @@ -476,8 +476,8 @@ be mandatory and clients cannot be authenticated with the TLS protocol. ## Multiple LDAP servers **(PREMIUM SELF)** -With GitLab Enterprise Edition Starter, you can configure multiple LDAP servers -that your GitLab instance connects to. +With GitLab, you can configure multiple LDAP servers that your GitLab instance +connects to. To add another LDAP server: diff --git a/doc/administration/feature_flags.md b/doc/administration/feature_flags.md index 55727822654..6882797064b 100644 --- a/doc/administration/feature_flags.md +++ b/doc/administration/feature_flags.md @@ -13,7 +13,7 @@ to deploy features in an early stage of development so that they can be incrementally rolled out. Before making them permanently available, features can be deployed behind -flags for a [number of reasons](../development/feature_flags/index.md#when-to-use-feature-flags), such as: +flags for a [number of reasons](https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle#when-to-use-feature-flags), such as: - To test the feature. - To get feedback from users and customers while in an early stage of the development of the feature. diff --git a/doc/administration/geo/replication/datatypes.md b/doc/administration/geo/replication/datatypes.md index 37633ee0f18..293417c9c64 100644 --- a/doc/administration/geo/replication/datatypes.md +++ b/doc/administration/geo/replication/datatypes.md @@ -178,8 +178,8 @@ successfully, you must replicate their data using some other means. | [Group wiki repository](../../../user/group/index.md#group-wikis) | [No](https://gitlab.com/gitlab-org/gitlab/-/issues/208147) | [No](https://gitlab.com/gitlab-org/gitlab/-/issues/208147) | No | | | [Uploads](../../uploads.md) | **Yes** (10.2) | [No](https://gitlab.com/groups/gitlab-org/-/epics/1817) | No | Verified only on transfer or manually using [Integrity Check Rake Task](../../raketasks/check.md) on both nodes and comparing the output between them. | | [LFS objects](../../lfs/index.md) | **Yes** (10.2) | [No](https://gitlab.com/gitlab-org/gitlab/-/issues/8922) | Via Object Storage provider if supported. Native Geo support (Beta). | Verified only on transfer or manually using [Integrity Check Rake Task](../../raketasks/check.md) on both nodes and comparing the output between them. GitLab versions 11.11.x and 12.0.x are affected by [a bug that prevents any new LFS objects from replicating](https://gitlab.com/gitlab-org/gitlab/-/issues/32696). | -| [Personal snippets](../../../user/snippets.md#personal-snippets) | **Yes** (10.2) | **Yes** (10.2) | No | | -| [Project snippets](../../../user/snippets.md#project-snippets) | **Yes** (10.2) | **Yes** (10.2) | No | | +| [Personal snippets](../../../user/snippets.md) | **Yes** (10.2) | **Yes** (10.2) | No | | +| [Project snippets](../../../user/snippets.md) | **Yes** (10.2) | **Yes** (10.2) | No | | | [CI job artifacts (other than Job Logs)](../../../ci/pipelines/job_artifacts.md) | **Yes** (10.4) | [No](https://gitlab.com/gitlab-org/gitlab/-/issues/8923) | Via Object Storage provider if supported. Native Geo support (Beta) . | Verified only manually using [Integrity Check Rake Task](../../raketasks/check.md) on both nodes and comparing the output between them | | [Job logs](../../job_logs.md) | **Yes** (10.4) | [No](https://gitlab.com/gitlab-org/gitlab/-/issues/8923) | Via Object Storage provider if supported. Native Geo support (Beta). | Verified only on transfer or manually using [Integrity Check Rake Task](../../raketasks/check.md) on both nodes and comparing the output between them | | [Object pools for forked project deduplication](../../../development/git_object_deduplication.md) | **Yes** | No | No | | diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index f7f69ac9748..8bac02c99af 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -470,7 +470,7 @@ fails. Consider this when toggling the visibility of the feature on or off on production. The `feature_flag` property does not allow the use of -[feature gates based on actors](../development/feature_flags/development.md). +[feature gates based on actors](../development/feature_flags/index.md). This means that the feature flag cannot be toggled only for particular projects, groups, or users, but instead can only be toggled globally for everyone. diff --git a/doc/development/changelog.md b/doc/development/changelog.md index fa771f96259..98a3e75bb3c 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -57,7 +57,7 @@ the `author` field. GitLab team members **should not**. - Any change behind an enabled feature flag **should** have a changelog entry. - Any change that adds new usage data metrics and changes that needs to be documented in Product Intelligence [Event Dictionary](https://about.gitlab.com/handbook/product/product-intelligence-guide/#event-dictionary) **should** have a changelog entry. - A change that adds snowplow events **should** have a changelog entry - -- A change that [removes a feature flag](feature_flags/development.md) **must** have a changelog entry. +- A change that [removes a feature flag](feature_flags/index.md) **must** have a changelog entry. - A fix for a regression introduced and then fixed in the same release (i.e., fixing a bug introduced during a monthly release candidate) **should not** have a changelog entry. diff --git a/doc/development/chatops_on_gitlabcom.md b/doc/development/chatops_on_gitlabcom.md index 0341abf5eeb..4ae49103d1b 100644 --- a/doc/development/chatops_on_gitlabcom.md +++ b/doc/development/chatops_on_gitlabcom.md @@ -61,4 +61,4 @@ To request access to ChatOps on GitLab.com: - [ChatOps Usage](../ci/chatops/index.md) - [Understanding EXPLAIN plans](understanding_explain_plans.md) -- [Feature Groups](feature_flags/development.md#feature-groups) +- [Feature Groups](feature_flags/index.md#feature-groups) diff --git a/doc/development/documentation/feature_flags.md b/doc/development/documentation/feature_flags.md index d318390b210..8fe3f0cbf8e 100644 --- a/doc/development/documentation/feature_flags.md +++ b/doc/development/documentation/feature_flags.md @@ -20,7 +20,7 @@ must be documented. For context, see the ## Criteria -According to the process of [deploying GitLab features behind feature flags](../feature_flags/process.md): +According to the process of [deploying GitLab features behind feature flags](https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle): > - _By default, feature flags should be off._ > - _Feature flags should remain in the codebase for a short period as possible to reduce the need for feature flag accounting._ diff --git a/doc/development/documentation/styleguide/index.md b/doc/development/documentation/styleguide/index.md index db13701d5db..4831e5bbce5 100644 --- a/doc/development/documentation/styleguide/index.md +++ b/doc/development/documentation/styleguide/index.md @@ -1611,11 +1611,12 @@ If all content in a section is related, add version text after the header for the section. The version information must: - Be surrounded by blank lines. -- Start with `>`. -- Version histories with more than one entry should have each entry on its own - line (long lines are okay). Start each line with `> -` to get unordered list - formatting. -- Whenever possible, have a link to the completed issue, merge request, or epic +- Start with `>`. If there are multiple bullets, each line must start with `> -`. +- The string must include these words in this order (capitalization doesn't matter): + - `introduced`, `deprecated`, `moved` + - `in` or `to` + - `GitLab` +- Whenever possible, include a link to the completed issue, merge request, or epic that introduced the feature. An issue is preferred over a merge request, and a merge request is preferred over an epic. diff --git a/doc/development/experiment_guide/index.md b/doc/development/experiment_guide/index.md index 652b4f8f80b..15430831f4a 100644 --- a/doc/development/experiment_guide/index.md +++ b/doc/development/experiment_guide/index.md @@ -45,7 +45,7 @@ One is built into GitLab directly and has been around for a while (this is calle [`gitlab-experiment`](https://gitlab.com/gitlab-org/gitlab-experiment) and is referred to as `Gitlab::Experiment` -- GLEX for short. -Both approaches use [experiment](../feature_flags/development.md#experiment-type) +Both approaches use [experiment](../feature_flags/index.md#experiment-type) feature flags, and there is currently no strong suggestion to use one over the other. | Feature | `Experimentation Module` | GLEX | diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index ec784eb3bd8..2e812d9fa0a 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -423,7 +423,7 @@ query getAuthorData($authorNameEnabled: Boolean = false) { ``` Then in the Vue (or JavaScript) call to the query we can pass in our feature flag. This feature -flag needs to be already set up correctly. See the [feature flag documentation](../feature_flags/development.md) +flag needs to be already set up correctly. See the [feature flag documentation](../feature_flags/index.md) for the correct way to do this. ```javascript diff --git a/doc/development/feature_flags/controls.md b/doc/development/feature_flags/controls.md index df98996158e..fc327a2defc 100644 --- a/doc/development/feature_flags/controls.md +++ b/doc/development/feature_flags/controls.md @@ -36,7 +36,7 @@ easier to measure the impact of both separately. The GitLab feature library (using [Flipper](https://github.com/jnunemaker/flipper), and covered in the [Feature -Flags process](process.md) guide) supports rolling out changes to a percentage of +Flags process](https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle) guide) supports rolling out changes to a percentage of time to users. This in turn can be controlled using [GitLab ChatOps](../../ci/chatops/index.md). For an up to date list of feature flag commands please see [the source @@ -240,7 +240,7 @@ To disable a feature flag that has been enabled for a specific project you can r /chatops run feature set --group=gitlab-org some_feature false ``` -You cannot selectively disable feature flags for a specific project/group/user without applying a [specific method of implementing](development.md#selectively-disable-by-actor) the feature flags. +You cannot selectively disable feature flags for a specific project/group/user without applying a [specific method of implementing](index.md#selectively-disable-by-actor) the feature flags. ### Feature flag change logging @@ -281,7 +281,7 @@ To remove a feature flag, open **one merge request** to make the changes. In the 1. Add the ~"feature flag" label so release managers are aware the changes are hidden behind a feature flag. 1. If the merge request has to be picked into a stable branch, add the appropriate `~"Pick into X.Y"` label, for example `~"Pick into 13.0"`. - See [the feature flag process](process.md#including-a-feature-behind-feature-flag-in-the-final-release) + See [the feature flag process](https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle#including-a-feature-behind-feature-flag-in-the-final-release) for further details. 1. Remove all references to the feature flag from the codebase, including tests. 1. Remove the YAML definition for the feature from the repository. diff --git a/doc/development/feature_flags/development.md b/doc/development/feature_flags/development.md index a76c76673bf..79efd6d5502 100644 --- a/doc/development/feature_flags/development.md +++ b/doc/development/feature_flags/development.md @@ -1,604 +1,7 @@ --- -type: reference, dev -stage: none -group: Development -info: "See the Technical Writers assigned to Development Guidelines: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments-to-development-guidelines" +redirect_to: 'index.md' --- -# Developing with feature flags - -This document provides guidelines on how to use feature flags -in the GitLab codebase to conditionally enable features -and test them. - -Features that are developed and merged behind a feature flag -should not include a changelog entry. The entry should be added either in the merge -request removing the feature flag or the merge request where the default value of -the feature flag is set to enabled. If the feature contains any database migrations, it -*should* include a changelog entry for the database changes. - -WARNING: -All newly-introduced feature flags should be [disabled by default](process.md#feature-flags-in-gitlab-development). - -NOTE: -This document is the subject of continued work as part of an epic to [improve internal usage of Feature Flags](https://gitlab.com/groups/gitlab-org/-/epics/3551). Raise any suggestions as new issues and attach them to the epic. - -## Risk of a broken master (main) branch - -Feature flags **must** be used in the MR that introduces them. Not doing so causes a -[broken master](https://about.gitlab.com/handbook/engineering/workflow/#broken-master) scenario due -to the `rspec:feature-flags` job that only runs on the `master` branch. - -## Types of feature flags - -Choose a feature flag type that matches the expected usage. - -### `development` type - -`development` feature flags are short-lived feature flags, -used so that unfinished code can be deployed in production. - -A `development` feature flag should have a rollout issue, -ideally created using the [Feature Flag Roll Out template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/issue_templates/Feature%20Flag%20Roll%20Out.md). - -This is the default type used when calling `Feature.enabled?`. - -### `ops` type - -`ops` feature flags are long-lived feature flags that control operational aspects -of GitLab product behavior. For example, feature flags that disable features that might -have a performance impact, like special Sidekiq worker behavior. - -`ops` feature flags likely do not have rollout issues, as it is hard to -predict when they are enabled or disabled. - -To use `ops` feature flags, you must append `type: :ops` to `Feature.enabled?` -invocations: - -```ruby -# Check if feature flag is enabled -Feature.enabled?(:my_ops_flag, project, type: :ops) - -# Check if feature flag is disabled -Feature.disabled?(:my_ops_flag, project, type: :ops) - -# Push feature flag to Frontend -push_frontend_feature_flag(:my_ops_flag, project, type: :ops) -``` - -### `experiment` type - -`experiment` feature flags are used for A/B testing on GitLab.com. - -An `experiment` feature flag should conform to the same standards as a `development` feature flag, -although the interface has some differences. An experiment feature flag should have a rollout issue, -ideally created using the [Experiment Tracking template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/issue_templates/experiment_tracking_template.md). More information can be found in the [experiment guide](../experiment_guide/index.md). - -## Feature flag definition and validation - -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/229161) in GitLab 13.3. - -During development (`RAILS_ENV=development`) or testing (`RAILS_ENV=test`) all feature flag usage is being strictly validated. - -This process is meant to ensure consistent feature flag usage in the codebase. All feature flags **must**: - -- Be known. Only use feature flags that are explicitly defined. -- Not be defined twice. They have to be defined either in FOSS or EE, but not both. -- Use a valid and consistent `type:` across all invocations. -- Use the same `default_enabled:` across all invocations. -- Have an owner. - -All feature flags known to GitLab are self-documented in YAML files stored in: - -- [`config/feature_flags`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/config/feature_flags) -- [`ee/config/feature_flags`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/ee/config/feature_flags) - -Each feature flag is defined in a separate YAML file consisting of a number of fields: - -| Field | Required | Description | -|---------------------|----------|----------------------------------------------------------------| -| `name` | yes | Name of the feature flag. | -| `type` | yes | Type of feature flag. | -| `default_enabled` | yes | The default state of the feature flag that is strictly validated, with `default_enabled:` passed as an argument. | -| `introduced_by_url` | no | The URL to the Merge Request that introduced the feature flag. | -| `rollout_issue_url` | no | The URL to the Issue covering the feature flag rollout. | -| `group` | no | The [group](https://about.gitlab.com/handbook/product/categories/#devops-stages) that owns the feature flag. | - -NOTE: -All validations are skipped when running in `RAILS_ENV=production`. - -## Create a new feature flag - -The GitLab codebase provides [`bin/feature-flag`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/bin/feature-flag), -a dedicated tool to create new feature flag definitions. -The tool asks various questions about the new feature flag, then creates -a YAML definition in `config/feature_flags` or `ee/config/feature_flags`. - -Only feature flags that have a YAML definition file can be used when running the development or testing environments. - -```shell -$ bin/feature-flag my_feature_flag ->> Specify the group introducing the feature flag, like `group::apm`: -?> group::memory - ->> URL of the MR introducing the feature flag (enter to skip): -?> https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38602 - ->> Open this URL and fill in the rest of the details: -https://gitlab.com/gitlab-org/gitlab/-/issues/new?issue%5Btitle%5D=%5BFeature+flag%5D+Rollout+of+%60test-flag%60&issuable_template=Feature+Flag+Roll+Out - ->> URL of the rollout issue (enter to skip): -?> https://gitlab.com/gitlab-org/gitlab/-/issues/232533 -create config/feature_flags/development/my_feature_flag.yml ---- -name: my_feature_flag -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38602 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/232533 -group: group::memory -type: development -default_enabled: false -``` - -NOTE: -To create a feature flag that is only used in EE, add the `--ee` flag: `bin/feature-flag --ee` - -## Delete a feature flag - -See [cleaning up feature flags](controls.md#cleaning-up) for more information about -deleting feature flags. - -## Develop with a feature flag - -There are two main ways of using Feature Flags in the GitLab codebase: - -- [Backend code (Rails)](#backend) -- [Frontend code (VueJS)](#frontend) - -### Backend - -The feature flag interface is defined in [`lib/feature.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/feature.rb). -This interface provides a set of methods to check if the feature flag is enabled or disabled: - -```ruby -if Feature.enabled?(:my_feature_flag, project) - # execute code if feature flag is enabled -else - # execute code if feature flag is disabled -end - -if Feature.disabled?(:my_feature_flag, project) - # execute code if feature flag is disabled -end -``` - -In rare cases you may want to make a feature enabled by default. If so, explain the reasoning -in the merge request. Use `default_enabled: true` when checking the feature flag state: - -```ruby -if Feature.enabled?(:feature_flag, project, default_enabled: true) - # execute code if feature flag is enabled -else - # execute code if feature flag is disabled -end - -if Feature.disabled?(:my_feature_flag, project, default_enabled: true) - # execute code if feature flag is disabled -end -``` - -If not specified, `default_enabled` is `false`. - -To force reading the `default_enabled` value from the relative YAML definition file, use -`default_enabled: :yaml`: - -```ruby -if Feature.enabled?(:feature_flag, project, default_enabled: :yaml) - # execute code if feature flag is enabled -end -``` - -```ruby -if Feature.disabled?(:feature_flag, project, default_enabled: :yaml) - # execute code if feature flag is disabled -end -``` - -This allows to use the same feature flag check across various parts of the codebase and -maintain the status of `default_enabled` in the YAML definition file which is the SSOT. - -If `default_enabled: :yaml` is used, a YAML definition is expected or an error is raised -in development or test environment, while returning `false` on production. - -If not specified, the default feature flag type for `Feature.enabled?` and `Feature.disabled?` -is `type: development`. For all other feature flag types, you must specify the `type:`: - -```ruby -if Feature.enabled?(:feature_flag, project, type: :ops) - # execute code if ops feature flag is enabled -else - # execute code if ops feature flag is disabled -end - -if Feature.disabled?(:my_feature_flag, project, type: :ops) - # execute code if feature flag is disabled -end -``` - -WARNING: -Don't use feature flags at application load time. For example, using the `Feature` class in -`config/initializers/*` or at the class level could cause an unexpected error. This error occurs -because a database that a feature flag adapter might depend on doesn't exist at load time -(especially for fresh installations). Checking for the database's existence at the caller isn't -recommended, as some adapters don't require a database at all (for example, the HTTP adapter). The -feature flag setup check must be abstracted in the `Feature` namespace. This approach also requires -application reload when the feature flag changes. You must therefore ask SREs to reload the -Web/API/Sidekiq fleet on production, which takes time to fully rollout/rollback the changes. For -these reasons, use environment variables (for example, `ENV['YOUR_FEATURE_NAME']`) or `gitlab.yml` -instead. - -Here's an example of a pattern that you should avoid: - -```ruby -class MyClass - if Feature.enabled?(:...) - new_process - else - legacy_process - end -end -``` - -### Frontend - -Use the `push_frontend_feature_flag` method for frontend code, which is -available to all controllers that inherit from `ApplicationController`. You can use -this method to expose the state of a feature flag, for example: - -```ruby -before_action do - # Prefer to scope it per project or user e.g. - push_frontend_feature_flag(:vim_bindings, project) -end - -def index - # ... -end - -def edit - # ... -end -``` - -You can then check the state of the feature flag in JavaScript as follows: - -```javascript -if ( gon.features.vimBindings ) { - // ... -} -``` - -The name of the feature flag in JavaScript is always camelCase, -so checking for `gon.features.vim_bindings` would not work. - -See the [Vue guide](../fe_guide/vue.md#accessing-feature-flags) for details about -how to access feature flags in a Vue component. - -In rare cases you may want to make a feature enabled by default. If so, explain the reasoning -in the merge request. Use `default_enabled: true` when checking the feature flag state: - -```ruby -before_action do - # Prefer to scope it per project or user e.g. - push_frontend_feature_flag(:vim_bindings, project, default_enabled: true) -end -``` - -If not specified, the default feature flag type for `push_frontend_feature_flag` -is `type: development`. For all other feature flag types, you must specify the `type:`: - -```ruby -before_action do - push_frontend_feature_flag(:vim_bindings, project, type: :ops) -end -``` - -### Feature actors - -**It is strongly advised to use actors with feature flags.** Actors provide a simple -way to enable a feature flag only for a given project, group or user. This makes debugging -easier, as you can filter logs and errors for example, based on actors. This also makes it possible -to enable the feature on the `gitlab-org` or `gitlab-com` groups first, while the rest of -the users aren't impacted. - -Actors also provide an easy way to do a percentage rollout of a feature in a sticky way. -If a 1% rollout enabled a feature for a specific actor, that actor will continue to have the feature enabled at -10%, 50%, and 100%. - -GitLab currently supports the following models as feature flag actors: - -- `User` -- `Project` -- `Group` - -The actor is a second parameter of the `Feature.enabled?` call. The -same actor type must be used consistently for all invocations of `Feature.enabled?`. - -```ruby -Feature.enabled?(:feature_flag, project) -Feature.enabled?(:feature_flag, group) -Feature.enabled?(:feature_flag, user) -``` - -#### Selectively disable by actor - -By default you cannot selectively disable a feature flag by actor. - -```shell -# This will not work how you would expect. -/chatops run feature set some_feature true -/chatops run feature set --project=gitlab-org/gitlab some_feature false -``` - -However, if you add two feature flags, you can write your conditional statement in such a way that the equivalent selective disable is possible. - -```ruby -Feature.enabled?(:a_feature, project) && Feature.disabled?(:a_feature_override, project) -``` - -```shell -# This will enable a feature flag globally, except for gitlab-org/gitlab -/chatops run feature set a_feature true -/chatops run feature set --project=gitlab-org/gitlab a_feature_override true -``` - -### Enable additional objects as actors - -To use feature gates based on actors, the model needs to respond to -`flipper_id`. For example, to enable for the Foo model: - -```ruby -class Foo < ActiveRecord::Base - include FeatureGate -end -``` - -Only models that `include FeatureGate` or expose `flipper_id` method can be -used as an actor for `Feature.enabled?`. - -### Feature flags for licensed features - -You can't use a feature flag with the same name as a licensed feature name, because -it would cause a naming collision. This was [widely discussed and removed](https://gitlab.com/gitlab-org/gitlab/-/issues/259611) -because it is confusing. - -To check for licensed features, add a dedicated feature flag under a different name -and check it explicitly, for example: - -```ruby -Feature.enabled?(:licensed_feature_feature_flag, project) && - project.feature_available?(:licensed_feature) -``` - -### Feature groups - -Feature groups must be defined statically in `lib/feature.rb` (in the -`.register_feature_groups` method), but their implementation can obviously be -dynamic (querying the DB, for example). - -Once defined in `lib/feature.rb`, you can to activate a -feature for a given feature group via the [`feature_group` parameter of the features API](../../api/features.md#set-or-create-a-feature) - -### Enabling a feature flag locally (in development) - -In the rails console (`rails c`), enter the following command to enable a feature flag: - -```ruby -Feature.enable(:feature_flag_name) -``` - -Similarly, the following command disables a feature flag: - -```ruby -Feature.disable(:feature_flag_name) -``` - -You can also enable a feature flag for a given gate: - -```ruby -Feature.enable(:feature_flag_name, Project.find_by_full_path("root/my-project")) -``` - -### Removing a feature flag locally (in development) - -Once you have manually enabled or disabled a feature flag to test in your local environment, -the flag's default value gets overwritten and it takes precedence over the `default_enabled` value. -This can cause confusion when changing the flag's `default_enabled` attribute. - -For example, flags are commonly enabled and disabled several times during the development process. -When we finally enable the flag by default, we set `default_enabled: true` in the YAML file. - -- If the flag was manually enabled before setting `default_enabled: true`, the feature will be enabled. -Not because of the `default_enabled: true` value of the flag but because it was manually enabled. -- If the flag was manually disabled before setting `default_enabled: true`, the feature will -remain disabled. The `default_enabled: true` value does not take precendence over the explicit `false` -value set when disabling it manually. - -To reset the feature flag to the default status set in its YAML file, remove it using the Rails console -(`rails c`) as follows: - -```ruby -Feature.remove(:feature_flag_name) -``` - -## Feature flags in tests - -Introducing a feature flag into the codebase creates an additional code path that should be tested. -It is strongly advised to test all code affected by a feature flag, both when **enabled** and **disabled** -to ensure the feature works properly. - -When using the testing environment, all feature flags are enabled by default. - -WARNING: -This does not apply to end-to-end (QA) tests, which [do not disable feature flags by default](#end-to-end-qa-tests). There is a different [process for using feature flags in end-to-end tests](../testing_guide/end_to_end/feature_flags.md). - -To disable a feature flag in a test, use the `stub_feature_flags` -helper. For example, to globally disable the `ci_live_trace` feature -flag in a test: - -```ruby -stub_feature_flags(ci_live_trace: false) - -Feature.enabled?(:ci_live_trace) # => false -``` - -If you wish to set up a test where a feature flag is enabled only -for some actors and not others, you can specify this in options -passed to the helper. For example, to enable the `ci_live_trace` -feature flag for a specific project: - -```ruby -project1, project2 = build_list(:project, 2) - -# Feature will only be enabled for project1 -stub_feature_flags(ci_live_trace: project1) - -Feature.enabled?(:ci_live_trace) # => false -Feature.enabled?(:ci_live_trace, project1) # => true -Feature.enabled?(:ci_live_trace, project2) # => false -``` - -The behavior of FlipperGate is as follows: - -1. You can enable an override for a specified actor to be enabled. -1. You can disable (remove) an override for a specified actor, - falling back to the default state. -1. There's no way to model that you explicitly disabled a specified actor. - -```ruby -Feature.enable(:my_feature) -Feature.disable(:my_feature, project1) -Feature.enabled?(:my_feature) # => true -Feature.enabled?(:my_feature, project1) # => true - -Feature.disable(:my_feature2) -Feature.enable(:my_feature2, project1) -Feature.enabled?(:my_feature2) # => false -Feature.enabled?(:my_feature2, project1) # => true -``` - -### `have_pushed_frontend_feature_flags` - -Use `have_pushed_frontend_feature_flags` to test if [`push_frontend_feature_flag`](#frontend) -has added the feature flag to the HTML. - -For example, - -```ruby -stub_feature_flags(value_stream_analytics_path_navigation: false) - -visit group_analytics_cycle_analytics_path(group) - -expect(page).to have_pushed_frontend_feature_flags(valueStreamAnalyticsPathNavigation: false) -``` - -### `stub_feature_flags` vs `Feature.enable*` - -It is preferred to use `stub_feature_flags` to enable feature flags -in the testing environment. This method provides a simple and well described -interface for simple use cases. - -However, in some cases more complex behavior needs to be tested, -like percentage rollouts of feature flags. This can be done using -`.enable_percentage_of_time` or `.enable_percentage_of_actors`: - -```ruby -# Good: feature needs to be explicitly disabled, as it is enabled by default if not defined -stub_feature_flags(my_feature: false) -stub_feature_flags(my_feature: true) -stub_feature_flags(my_feature: project) -stub_feature_flags(my_feature: [project, project2]) - -# Bad -Feature.enable(:my_feature_2) - -# Good: enable my_feature for 50% of time -Feature.enable_percentage_of_time(:my_feature_3, 50) - -# Good: enable my_feature for 50% of actors/gates/things -Feature.enable_percentage_of_actors(:my_feature_4, 50) -``` - -Each feature flag that has a defined state is persisted -during test execution time: - -```ruby -Feature.persisted_names.include?('my_feature') => true -Feature.persisted_names.include?('my_feature_2') => true -Feature.persisted_names.include?('my_feature_3') => true -Feature.persisted_names.include?('my_feature_4') => true -``` - -### Stubbing actor - -When you want to enable a feature flag for a specific actor only, -you can stub its representation. A gate that is passed -as an argument to `Feature.enabled?` and `Feature.disabled?` must be an object -that includes `FeatureGate`. - -In specs you can use the `stub_feature_flag_gate` method that allows you to -quickly create a custom actor: - -```ruby -gate = stub_feature_flag_gate('CustomActor') - -stub_feature_flags(ci_live_trace: gate) - -Feature.enabled?(:ci_live_trace) # => false -Feature.enabled?(:ci_live_trace, gate) # => true -``` - -You can also disable a feature flag for a specific actor: - -```ruby -gate = stub_feature_flag_gate('CustomActor') - -stub_feature_flags(ci_live_trace: false, thing: gate) -``` - -### Controlling feature flags engine in tests - -Our Flipper engine in the test environment works in a memory mode `Flipper::Adapters::Memory`. -`production` and `development` modes use `Flipper::Adapters::ActiveRecord`. - -You can control whether the `Flipper::Adapters::Memory` or `ActiveRecord` mode is being used. - -#### `stub_feature_flags: true` (default and preferred) - -In this mode Flipper is configured to use `Flipper::Adapters::Memory` and mark all feature -flags to be on-by-default and persisted on a first use. This overwrites the `default_enabled:` -of `Feature.enabled?` and `Feature.disabled?` returning always `true` unless feature flag -is persisted. - -Make sure behavior under feature flag doesn't go untested in some non-specific contexts. - -### `stub_feature_flags: false` - -This disables a memory-stubbed flipper, and uses `Flipper::Adapters::ActiveRecord` -a mode that is used by `production` and `development`. - -You should use this mode only when you really want to tests aspects of Flipper -with how it interacts with `ActiveRecord`. - -### End-to-end (QA) tests - -Toggling feature flags works differently in end-to-end (QA) tests. The end-to-end test framework does not have direct access to -Rails or the database, so it can't use Flipper. Instead, it uses [the public API](../../api/features.md#set-or-create-a-feature). Each end-to-end test can [enable or disable a feature flag during the test](../testing_guide/end_to_end/feature_flags.md). Alternatively, you can enable or disable a feature flag before one or more tests when you [run them from your GitLab repository's `qa` directory](https://gitlab.com/gitlab-org/gitlab/tree/master/qa#running-tests-with-a-feature-flag-enabled-or-disabled), or if you [run the tests via GitLab QA](https://gitlab.com/gitlab-org/gitlab-qa/-/blob/master/docs/what_tests_can_be_run.md#running-tests-with-a-feature-flag-enabled). - -[As noted above, feature flags are not enabled by default in end-to-end tests.](#feature-flags-in-tests) -This means that end-to-end tests will run with feature flags in the default state implemented in the source -code, or with the feature flag in its current state on the GitLab instance under test, unless the -test is written to enable/disable a feature flag explicitly. - -When a feature flag is changed on Staging or on GitLab.com, a Slack message will be posted to the `#qa-staging` or `#qa-production` channels to inform -the pipeline triage DRI so that they can more easily determine if any failures are related to a feature flag change. However, if you are working on a change you can -help to avoid unexpected failures by [confirming that the end-to-end tests pass with a feature flag enabled.](../testing_guide/end_to_end/feature_flags.md#confirming-that-end-to-end-tests-pass-with-a-feature-flag-enabled) +This document was moved to [another location](index.md). +<!-- This redirect file can be deleted after 2021-06-01. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page -->
\ No newline at end of file diff --git a/doc/development/feature_flags/index.md b/doc/development/feature_flags/index.md index 4890cc5da35..5c98dc2e473 100644 --- a/doc/development/feature_flags/index.md +++ b/doc/development/feature_flags/index.md @@ -1,75 +1,636 @@ --- +type: reference, dev stage: none group: Development info: "See the Technical Writers assigned to Development Guidelines: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments-to-development-guidelines" --- -# Feature flags in development of GitLab +# Developing with feature flags **NOTE**: The documentation below covers feature flags used by GitLab to deploy its own features, which **is not** the same as the [feature flags offered as part of the product](../../operations/feature_flags.md). -## When to use feature flags +This document provides guidelines on how to use feature flags +in the GitLab codebase to conditionally enable features +and test them. -Developers are required to use feature flags for changes that could affect availability of existing GitLab functionality (if it only affects the new feature you're making that is probably acceptable). -Such changes include: +Features that are developed and merged behind a feature flag +should not include a changelog entry. The entry should be added either in the merge +request removing the feature flag or the merge request where the default value of +the feature flag is set to enabled. If the feature contains any database migrations, it +*should* include a changelog entry for the database changes. -1. New features in high traffic areas (e.g. a new merge request widget, new option in issues/epics, new CI functionality). -1. Complex performance improvements that may require additional testing in production (e.g. rewriting complex queries, changes to frequently used API endpoints). -1. Invasive changes to the user interface (e.g. introducing a new navigation bar, removal of a sidebar, UI element change in issues or MR interface). -1. Introducing dependencies on third-party services (e.g. adding support for importing projects). -1. Changes to features that can cause data corruption or cause data loss (e.g. features processing repository data or user uploaded content). +WARNING: +All newly-introduced feature flags should be [disabled by default](https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle/#feature-flags-in-gitlab-development). -Situations where you might consider not using a feature flag: +NOTE: +This document is the subject of continued work as part of an epic to [improve internal usage of Feature Flags](https://gitlab.com/groups/gitlab-org/-/epics/3551). Raise any suggestions as new issues and attach them to the epic. -1. Adding a new API endpoint -1. Introducing new features in low traffic areas (e.g. adding a new export functionality in the admin area/group settings/project settings) -1. Non-invasive frontend changes (e.g. changing the color of a button, or moving a UI element in a low traffic area) +## Feature flags in GitLab development -In all cases, those working on the changes should ask themselves: +The following highlights should be considered when deciding if feature flags +should be leveraged: -> Why do I need to add a feature flag? If I don't add one, what options do I have to control the impact on application reliability, and user experience? +- By default, the feature flags should be **off**. +- Feature flags should remain in the codebase for as short period as possible + to reduce the need for feature flag accounting. +- The person operating with feature flags is responsible for clearly communicating + the status of a feature behind the feature flag with responsible stakeholders. The + issue description should be updated with the feature flag name and whether it is + defaulted on or off as soon it is evident that a feature flag is needed. +- Merge requests that make changes hidden behind a feature flag, or remove an + existing feature flag because a feature is deemed stable must have the + ~"feature flag" label assigned. +- When development of a feature will be spread across multiple merge + requests, you can use the following workflow: -For perspective on why we limit our use of feature flags please see -<i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Feature flags only when needed](https://www.youtube.com/watch?v=DQaGqyolOd8). + 1. [Create a new feature flag](#create-a-new-feature-flag) + which is **off** by default, in the first merge request which uses the flag. + Flags [should not be added separately](#risk-of-a-broken-master-main-branch). + 1. Submit incremental changes via one or more merge requests, ensuring that any + new code added can only be reached if the feature flag is **on**. + You can keep the feature flag enabled on your local GDK during development. + 1. When the feature is ready to be tested, enable the feature flag for + a specific project and ensure that there are no issues with the implementation. + 1. When the feature is ready to be announced, create a merge request that adds + documentation about the feature, including [documentation for the feature flag itself](../documentation/feature_flags.md), + and a changelog entry. In the same merge request either flip the feature flag to + be **on by default** or remove it entirely in order to enable the new behavior. -In case you are uncertain if a feature flag is necessary, simply ask about this in an early merge request, and those reviewing the changes will likely provide you with an answer. +One might be tempted to think that feature flags will delay the release of a +feature by at least one month (= one release). This is not the case. A feature +flag does not have to stick around for a specific amount of time +(e.g. at least one release), instead they should stick around until the feature +is deemed stable. Stable means it works on GitLab.com without causing any +problems, such as outages. -When using a feature flag for UI elements, make sure to _also_ use a feature -flag for the underlying backend code, if there is any. This ensures there is -absolutely no way to use the feature until it is enabled. +## Risk of a broken master (main) branch -## How to use Feature Flags +Feature flags **must** be used in the MR that introduces them. Not doing so causes a +[broken master](https://about.gitlab.com/handbook/engineering/workflow/#broken-master) scenario due +to the `rspec:feature-flags` job that only runs on the `master` branch. -Feature flags can be used to gradually deploy changes, regardless of whether -they are new features or performance improvements. By using feature flags, -you can determine the impact of GitLab-directed changes, while still being able -to disable those changes without having to revert an entire release. +## Types of feature flags -For an overview about starting with feature flags in GitLab development, -use this [training template](https://gitlab.com/gitlab-com/www-gitlab-com/-/blob/master/.gitlab/issue_templates/feature-flag-training.md). +Choose a feature flag type that matches the expected usage. -Before using feature flags for GitLab development, review the following development guides: +### `development` type -1. [Process for using features flags](process.md): When you should use - feature flags in the development of GitLab, what's the cost of using them, - and how to include them in a release. -1. [Developing with feature flags](development.md): Learn about the types of - feature flags, their definition and validation, how to create them, frontend and - backend details, and other information. -1. [Documenting features deployed behind feature flags](../documentation/feature_flags.md): - How to document features deployed behind feature flags, and how to update the - documentation for features' flags when their states change. -1. [Controlling feature flags](controls.md): Learn the process for deploying - a new feature, enabling it on GitLab.com, communicating the change, - logging, and cleaning up. +`development` feature flags are short-lived feature flags, +used so that unfinished code can be deployed in production. -User guides: +A `development` feature flag should have a rollout issue, +ideally created using the [Feature Flag Roll Out template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/issue_templates/Feature%20Flag%20Roll%20Out.md). -1. [How GitLab administrators can enable and disable features behind flags](../../administration/feature_flags.md): - An explanation for GitLab administrators about how they can - enable or disable GitLab features behind feature flags. -1. [What "features deployed behind flags" means to the GitLab user](../../user/feature_flags.md): - An explanation for GitLab users regarding how certain features - might not be available to them until they are enabled. +This is the default type used when calling `Feature.enabled?`. + +### `ops` type + +`ops` feature flags are long-lived feature flags that control operational aspects +of GitLab product behavior. For example, feature flags that disable features that might +have a performance impact, like special Sidekiq worker behavior. + +`ops` feature flags likely do not have rollout issues, as it is hard to +predict when they are enabled or disabled. + +To use `ops` feature flags, you must append `type: :ops` to `Feature.enabled?` +invocations: + +```ruby +# Check if feature flag is enabled +Feature.enabled?(:my_ops_flag, project, type: :ops) + +# Check if feature flag is disabled +Feature.disabled?(:my_ops_flag, project, type: :ops) + +# Push feature flag to Frontend +push_frontend_feature_flag(:my_ops_flag, project, type: :ops) +``` + +### `experiment` type + +`experiment` feature flags are used for A/B testing on GitLab.com. + +An `experiment` feature flag should conform to the same standards as a `development` feature flag, +although the interface has some differences. An experiment feature flag should have a rollout issue, +ideally created using the [Experiment Tracking template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/issue_templates/experiment_tracking_template.md). More information can be found in the [experiment guide](../experiment_guide/index.md). + +## Feature flag definition and validation + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/229161) in GitLab 13.3. + +During development (`RAILS_ENV=development`) or testing (`RAILS_ENV=test`) all feature flag usage is being strictly validated. + +This process is meant to ensure consistent feature flag usage in the codebase. All feature flags **must**: + +- Be known. Only use feature flags that are explicitly defined. +- Not be defined twice. They have to be defined either in FOSS or EE, but not both. +- Use a valid and consistent `type:` across all invocations. +- Use the same `default_enabled:` across all invocations. +- Have an owner. + +All feature flags known to GitLab are self-documented in YAML files stored in: + +- [`config/feature_flags`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/config/feature_flags) +- [`ee/config/feature_flags`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/ee/config/feature_flags) + +Each feature flag is defined in a separate YAML file consisting of a number of fields: + +| Field | Required | Description | +|---------------------|----------|----------------------------------------------------------------| +| `name` | yes | Name of the feature flag. | +| `type` | yes | Type of feature flag. | +| `default_enabled` | yes | The default state of the feature flag that is strictly validated, with `default_enabled:` passed as an argument. | +| `introduced_by_url` | no | The URL to the Merge Request that introduced the feature flag. | +| `rollout_issue_url` | no | The URL to the Issue covering the feature flag rollout. | +| `group` | no | The [group](https://about.gitlab.com/handbook/product/categories/#devops-stages) that owns the feature flag. | + +NOTE: +All validations are skipped when running in `RAILS_ENV=production`. + +## Create a new feature flag + +The GitLab codebase provides [`bin/feature-flag`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/bin/feature-flag), +a dedicated tool to create new feature flag definitions. +The tool asks various questions about the new feature flag, then creates +a YAML definition in `config/feature_flags` or `ee/config/feature_flags`. + +Only feature flags that have a YAML definition file can be used when running the development or testing environments. + +```shell +$ bin/feature-flag my_feature_flag +>> Specify the group introducing the feature flag, like `group::apm`: +?> group::memory + +>> URL of the MR introducing the feature flag (enter to skip): +?> https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38602 + +>> Open this URL and fill in the rest of the details: +https://gitlab.com/gitlab-org/gitlab/-/issues/new?issue%5Btitle%5D=%5BFeature+flag%5D+Rollout+of+%60test-flag%60&issuable_template=Feature+Flag+Roll+Out + +>> URL of the rollout issue (enter to skip): +?> https://gitlab.com/gitlab-org/gitlab/-/issues/232533 +create config/feature_flags/development/my_feature_flag.yml +--- +name: my_feature_flag +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38602 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/232533 +group: group::memory +type: development +default_enabled: false +``` + +NOTE: +To create a feature flag that is only used in EE, add the `--ee` flag: `bin/feature-flag --ee` + +## Delete a feature flag + +See [cleaning up feature flags](controls.md#cleaning-up) for more information about +deleting feature flags. + +## Develop with a feature flag + +There are two main ways of using Feature Flags in the GitLab codebase: + +- [Backend code (Rails)](#backend) +- [Frontend code (VueJS)](#frontend) + +### Backend + +The feature flag interface is defined in [`lib/feature.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/feature.rb). +This interface provides a set of methods to check if the feature flag is enabled or disabled: + +```ruby +if Feature.enabled?(:my_feature_flag, project) + # execute code if feature flag is enabled +else + # execute code if feature flag is disabled +end + +if Feature.disabled?(:my_feature_flag, project) + # execute code if feature flag is disabled +end +``` + +In rare cases you may want to make a feature enabled by default. If so, explain the reasoning +in the merge request. Use `default_enabled: true` when checking the feature flag state: + +```ruby +if Feature.enabled?(:feature_flag, project, default_enabled: true) + # execute code if feature flag is enabled +else + # execute code if feature flag is disabled +end + +if Feature.disabled?(:my_feature_flag, project, default_enabled: true) + # execute code if feature flag is disabled +end +``` + +If not specified, `default_enabled` is `false`. + +To force reading the `default_enabled` value from the relative YAML definition file, use +`default_enabled: :yaml`: + +```ruby +if Feature.enabled?(:feature_flag, project, default_enabled: :yaml) + # execute code if feature flag is enabled +end +``` + +```ruby +if Feature.disabled?(:feature_flag, project, default_enabled: :yaml) + # execute code if feature flag is disabled +end +``` + +This allows to use the same feature flag check across various parts of the codebase and +maintain the status of `default_enabled` in the YAML definition file which is the SSOT. + +If `default_enabled: :yaml` is used, a YAML definition is expected or an error is raised +in development or test environment, while returning `false` on production. + +If not specified, the default feature flag type for `Feature.enabled?` and `Feature.disabled?` +is `type: development`. For all other feature flag types, you must specify the `type:`: + +```ruby +if Feature.enabled?(:feature_flag, project, type: :ops) + # execute code if ops feature flag is enabled +else + # execute code if ops feature flag is disabled +end + +if Feature.disabled?(:my_feature_flag, project, type: :ops) + # execute code if feature flag is disabled +end +``` + +WARNING: +Don't use feature flags at application load time. For example, using the `Feature` class in +`config/initializers/*` or at the class level could cause an unexpected error. This error occurs +because a database that a feature flag adapter might depend on doesn't exist at load time +(especially for fresh installations). Checking for the database's existence at the caller isn't +recommended, as some adapters don't require a database at all (for example, the HTTP adapter). The +feature flag setup check must be abstracted in the `Feature` namespace. This approach also requires +application reload when the feature flag changes. You must therefore ask SREs to reload the +Web/API/Sidekiq fleet on production, which takes time to fully rollout/rollback the changes. For +these reasons, use environment variables (for example, `ENV['YOUR_FEATURE_NAME']`) or `gitlab.yml` +instead. + +Here's an example of a pattern that you should avoid: + +```ruby +class MyClass + if Feature.enabled?(:...) + new_process + else + legacy_process + end +end +``` + +### Frontend + +Use the `push_frontend_feature_flag` method for frontend code, which is +available to all controllers that inherit from `ApplicationController`. You can use +this method to expose the state of a feature flag, for example: + +```ruby +before_action do + # Prefer to scope it per project or user e.g. + push_frontend_feature_flag(:vim_bindings, project) +end + +def index + # ... +end + +def edit + # ... +end +``` + +You can then check the state of the feature flag in JavaScript as follows: + +```javascript +if ( gon.features.vimBindings ) { + // ... +} +``` + +The name of the feature flag in JavaScript is always camelCase, +so checking for `gon.features.vim_bindings` would not work. + +See the [Vue guide](../fe_guide/vue.md#accessing-feature-flags) for details about +how to access feature flags in a Vue component. + +In rare cases you may want to make a feature enabled by default. If so, explain the reasoning +in the merge request. Use `default_enabled: true` when checking the feature flag state: + +```ruby +before_action do + # Prefer to scope it per project or user e.g. + push_frontend_feature_flag(:vim_bindings, project, default_enabled: true) +end +``` + +If not specified, the default feature flag type for `push_frontend_feature_flag` +is `type: development`. For all other feature flag types, you must specify the `type:`: + +```ruby +before_action do + push_frontend_feature_flag(:vim_bindings, project, type: :ops) +end +``` + +### Feature actors + +**It is strongly advised to use actors with feature flags.** Actors provide a simple +way to enable a feature flag only for a given project, group or user. This makes debugging +easier, as you can filter logs and errors for example, based on actors. This also makes it possible +to enable the feature on the `gitlab-org` or `gitlab-com` groups first, while the rest of +the users aren't impacted. + +Actors also provide an easy way to do a percentage rollout of a feature in a sticky way. +If a 1% rollout enabled a feature for a specific actor, that actor will continue to have the feature enabled at +10%, 50%, and 100%. + +GitLab currently supports the following models as feature flag actors: + +- `User` +- `Project` +- `Group` + +The actor is a second parameter of the `Feature.enabled?` call. The +same actor type must be used consistently for all invocations of `Feature.enabled?`. + +```ruby +Feature.enabled?(:feature_flag, project) +Feature.enabled?(:feature_flag, group) +Feature.enabled?(:feature_flag, user) +``` + +#### Selectively disable by actor + +By default you cannot selectively disable a feature flag by actor. + +```shell +# This will not work how you would expect. +/chatops run feature set some_feature true +/chatops run feature set --project=gitlab-org/gitlab some_feature false +``` + +However, if you add two feature flags, you can write your conditional statement in such a way that the equivalent selective disable is possible. + +```ruby +Feature.enabled?(:a_feature, project) && Feature.disabled?(:a_feature_override, project) +``` + +```shell +# This will enable a feature flag globally, except for gitlab-org/gitlab +/chatops run feature set a_feature true +/chatops run feature set --project=gitlab-org/gitlab a_feature_override true +``` + +### Enable additional objects as actors + +To use feature gates based on actors, the model needs to respond to +`flipper_id`. For example, to enable for the Foo model: + +```ruby +class Foo < ActiveRecord::Base + include FeatureGate +end +``` + +Only models that `include FeatureGate` or expose `flipper_id` method can be +used as an actor for `Feature.enabled?`. + +### Feature flags for licensed features + +You can't use a feature flag with the same name as a licensed feature name, because +it would cause a naming collision. This was [widely discussed and removed](https://gitlab.com/gitlab-org/gitlab/-/issues/259611) +because it is confusing. + +To check for licensed features, add a dedicated feature flag under a different name +and check it explicitly, for example: + +```ruby +Feature.enabled?(:licensed_feature_feature_flag, project) && + project.feature_available?(:licensed_feature) +``` + +### Feature groups + +Feature groups must be defined statically in `lib/feature.rb` (in the +`.register_feature_groups` method), but their implementation can obviously be +dynamic (querying the DB, for example). + +Once defined in `lib/feature.rb`, you can to activate a +feature for a given feature group via the [`feature_group` parameter of the features API](../../api/features.md#set-or-create-a-feature) + +### Enabling a feature flag locally (in development) + +In the rails console (`rails c`), enter the following command to enable a feature flag: + +```ruby +Feature.enable(:feature_flag_name) +``` + +Similarly, the following command disables a feature flag: + +```ruby +Feature.disable(:feature_flag_name) +``` + +You can also enable a feature flag for a given gate: + +```ruby +Feature.enable(:feature_flag_name, Project.find_by_full_path("root/my-project")) +``` + +### Removing a feature flag locally (in development) + +When manually enabling or disabling a feature flag from the Rails console, its default value gets overwritten. +This can cause confusion when changing the flag's `default_enabled` attribute. + +To reset the feature flag to the default status, you can remove it in the rails console (`rails c`) +as follows: + +```ruby +Feature.remove(:feature_flag_name) +``` + +## Feature flags in tests + +Introducing a feature flag into the codebase creates an additional code path that should be tested. +It is strongly advised to test all code affected by a feature flag, both when **enabled** and **disabled** +to ensure the feature works properly. + +When using the testing environment, all feature flags are enabled by default. + +WARNING: +This does not apply to end-to-end (QA) tests, which [do not disable feature flags by default](#end-to-end-qa-tests). There is a different [process for using feature flags in end-to-end tests](../testing_guide/end_to_end/feature_flags.md). + +To disable a feature flag in a test, use the `stub_feature_flags` +helper. For example, to globally disable the `ci_live_trace` feature +flag in a test: + +```ruby +stub_feature_flags(ci_live_trace: false) + +Feature.enabled?(:ci_live_trace) # => false +``` + +If you wish to set up a test where a feature flag is enabled only +for some actors and not others, you can specify this in options +passed to the helper. For example, to enable the `ci_live_trace` +feature flag for a specific project: + +```ruby +project1, project2 = build_list(:project, 2) + +# Feature will only be enabled for project1 +stub_feature_flags(ci_live_trace: project1) + +Feature.enabled?(:ci_live_trace) # => false +Feature.enabled?(:ci_live_trace, project1) # => true +Feature.enabled?(:ci_live_trace, project2) # => false +``` + +The behavior of FlipperGate is as follows: + +1. You can enable an override for a specified actor to be enabled. +1. You can disable (remove) an override for a specified actor, + falling back to the default state. +1. There's no way to model that you explicitly disabled a specified actor. + +```ruby +Feature.enable(:my_feature) +Feature.disable(:my_feature, project1) +Feature.enabled?(:my_feature) # => true +Feature.enabled?(:my_feature, project1) # => true + +Feature.disable(:my_feature2) +Feature.enable(:my_feature2, project1) +Feature.enabled?(:my_feature2) # => false +Feature.enabled?(:my_feature2, project1) # => true +``` + +### `have_pushed_frontend_feature_flags` + +Use `have_pushed_frontend_feature_flags` to test if [`push_frontend_feature_flag`](#frontend) +has added the feature flag to the HTML. + +For example, + +```ruby +stub_feature_flags(value_stream_analytics_path_navigation: false) + +visit group_analytics_cycle_analytics_path(group) + +expect(page).to have_pushed_frontend_feature_flags(valueStreamAnalyticsPathNavigation: false) +``` + +### `stub_feature_flags` vs `Feature.enable*` + +It is preferred to use `stub_feature_flags` to enable feature flags +in the testing environment. This method provides a simple and well described +interface for simple use cases. + +However, in some cases more complex behavior needs to be tested, +like percentage rollouts of feature flags. This can be done using +`.enable_percentage_of_time` or `.enable_percentage_of_actors`: + +```ruby +# Good: feature needs to be explicitly disabled, as it is enabled by default if not defined +stub_feature_flags(my_feature: false) +stub_feature_flags(my_feature: true) +stub_feature_flags(my_feature: project) +stub_feature_flags(my_feature: [project, project2]) + +# Bad +Feature.enable(:my_feature_2) + +# Good: enable my_feature for 50% of time +Feature.enable_percentage_of_time(:my_feature_3, 50) + +# Good: enable my_feature for 50% of actors/gates/things +Feature.enable_percentage_of_actors(:my_feature_4, 50) +``` + +Each feature flag that has a defined state is persisted +during test execution time: + +```ruby +Feature.persisted_names.include?('my_feature') => true +Feature.persisted_names.include?('my_feature_2') => true +Feature.persisted_names.include?('my_feature_3') => true +Feature.persisted_names.include?('my_feature_4') => true +``` + +### Stubbing actor + +When you want to enable a feature flag for a specific actor only, +you can stub its representation. A gate that is passed +as an argument to `Feature.enabled?` and `Feature.disabled?` must be an object +that includes `FeatureGate`. + +In specs you can use the `stub_feature_flag_gate` method that allows you to +quickly create a custom actor: + +```ruby +gate = stub_feature_flag_gate('CustomActor') + +stub_feature_flags(ci_live_trace: gate) + +Feature.enabled?(:ci_live_trace) # => false +Feature.enabled?(:ci_live_trace, gate) # => true +``` + +You can also disable a feature flag for a specific actor: + +```ruby +gate = stub_feature_flag_gate('CustomActor') + +stub_feature_flags(ci_live_trace: false, thing: gate) +``` + +### Controlling feature flags engine in tests + +Our Flipper engine in the test environment works in a memory mode `Flipper::Adapters::Memory`. +`production` and `development` modes use `Flipper::Adapters::ActiveRecord`. + +You can control whether the `Flipper::Adapters::Memory` or `ActiveRecord` mode is being used. + +#### `stub_feature_flags: true` (default and preferred) + +In this mode Flipper is configured to use `Flipper::Adapters::Memory` and mark all feature +flags to be on-by-default and persisted on a first use. This overwrites the `default_enabled:` +of `Feature.enabled?` and `Feature.disabled?` returning always `true` unless feature flag +is persisted. + +Make sure behavior under feature flag doesn't go untested in some non-specific contexts. + +### `stub_feature_flags: false` + +This disables a memory-stubbed flipper, and uses `Flipper::Adapters::ActiveRecord` +a mode that is used by `production` and `development`. + +You should use this mode only when you really want to tests aspects of Flipper +with how it interacts with `ActiveRecord`. + +### End-to-end (QA) tests + +Toggling feature flags works differently in end-to-end (QA) tests. The end-to-end test framework does not have direct access to +Rails or the database, so it can't use Flipper. Instead, it uses [the public API](../../api/features.md#set-or-create-a-feature). Each end-to-end test can [enable or disable a feature flag during the test](../testing_guide/end_to_end/feature_flags.md). Alternatively, you can enable or disable a feature flag before one or more tests when you [run them from your GitLab repository's `qa` directory](https://gitlab.com/gitlab-org/gitlab/tree/master/qa#running-tests-with-a-feature-flag-enabled-or-disabled), or if you [run the tests via GitLab QA](https://gitlab.com/gitlab-org/gitlab-qa/-/blob/master/docs/what_tests_can_be_run.md#running-tests-with-a-feature-flag-enabled). + +[As noted above, feature flags are not enabled by default in end-to-end tests.](#feature-flags-in-tests) +This means that end-to-end tests will run with feature flags in the default state implemented in the source +code, or with the feature flag in its current state on the GitLab instance under test, unless the +test is written to enable/disable a feature flag explicitly. + +When a feature flag is changed on Staging or on GitLab.com, a Slack message will be posted to the `#qa-staging` or `#qa-production` channels to inform +the pipeline triage DRI so that they can more easily determine if any failures are related to a feature flag change. However, if you are working on a change you can +help to avoid unexpected failures by [confirming that the end-to-end tests pass with a feature flag enabled.](../testing_guide/end_to_end/feature_flags.md#confirming-that-end-to-end-tests-pass-with-a-feature-flag-enabled) diff --git a/doc/development/feature_flags/process.md b/doc/development/feature_flags/process.md index 65a6d01e0a2..247dafe9f0b 100644 --- a/doc/development/feature_flags/process.md +++ b/doc/development/feature_flags/process.md @@ -1,177 +1,8 @@ --- -type: reference, dev -stage: none -group: Development -info: "See the Technical Writers assigned to Development Guidelines: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments-to-development-guidelines" +redirect_to: 'https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle/' --- -# Feature flags process +This document was moved to [another location](https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle/). -## Feature flags for user applications - -This document only covers feature flags used in the development of GitLab -itself. Feature flags in deployed user applications can be found at -[Feature Flags feature documentation](../../operations/feature_flags.md). - -## Feature flags in GitLab development - -The following highlights should be considered when deciding if feature flags -should be leveraged: - -- By default, the feature flags should be **off**. -- Feature flags should remain in the codebase for as short period as possible - to reduce the need for feature flag accounting. -- The person operating with feature flags is responsible for clearly communicating - the status of a feature behind the feature flag with responsible stakeholders. The - issue description should be updated with the feature flag name and whether it is - defaulted on or off as soon it is evident that a feature flag is needed. -- Merge requests that make changes hidden behind a feature flag, or remove an - existing feature flag because a feature is deemed stable must have the - ~"feature flag" label assigned. -- When development of a feature will be spread across multiple merge - requests, you can use the following workflow: - - 1. [Create a new feature flag](development.md#create-a-new-feature-flag) - which is **off** by default, in the first merge request which uses the flag. - Flags [should not be added separately](development.md#risk-of-a-broken-master-main-branch). - 1. Submit incremental changes via one or more merge requests, ensuring that any - new code added can only be reached if the feature flag is **on**. - You can keep the feature flag enabled on your local GDK during development. - 1. When the feature is ready to be tested, enable the feature flag for - a specific project and ensure that there are no issues with the implementation. - 1. When the feature is ready to be announced, create a merge request that adds - documentation about the feature, including [documentation for the feature flag itself](../documentation/feature_flags.md), - and a changelog entry. In the same merge request either flip the feature flag to - be **on by default** or remove it entirely in order to enable the new behavior. - -One might be tempted to think that feature flags will delay the release of a -feature by at least one month (= one release). This is not the case. A feature -flag does not have to stick around for a specific amount of time -(e.g. at least one release), instead they should stick around until the feature -is deemed stable. Stable means it works on GitLab.com without causing any -problems, such as outages. - -Please also read the [development guide for feature flags](development.md). - -### Including a feature behind feature flag in the final release - -In order to build a final release and present the feature for self-managed -users, the feature flag should be at least defaulted to **on**. If the feature -is deemed stable and there is confidence that removing the feature flag is safe, -consider removing the feature flag altogether. It's _strongly_ recommended that -the feature flag is [enabled **globally** on **production**](controls.md#enabling-a-feature-for-gitlabcom) for **at least one day** -before making this decision. Unexpected bugs are sometimes discovered during this period. - -The process for enabling features that are disabled by default can take 5-6 days -from when the merge request is first reviewed to when the change is deployed to -GitLab.com. However, it is recommended to allow 10-14 days for this activity to -account for unforeseen problems. - -Feature flags must be [documented according to their state (enabled/disabled)](../documentation/feature_flags.md), -and when the state changes, docs **must** be updated accordingly. - -NOTE: -Take into consideration that such action can make the feature available on -GitLab.com shortly after the change to the feature flag is merged. - -Changing the default state or removing the feature flag has to be done before -the 22nd of the month, _at least_ 3-4 working days before, in order for the change -to be included in the final self-managed release. - -In addition to this, the feature behind feature flag should: - -- Run in all GitLab.com environments for a sufficient period of time. This time - period depends on the feature behind the feature flag, but as a general rule of - thumb 2-4 working days should be sufficient to gather enough feedback. -- The feature should be exposed to all users within the GitLab.com plan during - the above mentioned period of time. Exposing the feature to a smaller percentage - or only a group of users might not expose a sufficient amount of information to aid in - making a decision on feature stability. - -While rare, release managers may decide to reject picking or revert a change in -a stable branch, even when feature flags are used. This might be necessary if -the changes are deemed problematic, too invasive, or there simply isn't enough -time to properly measure how the changes behave on GitLab.com. - -### The cost of feature flags - -When reading the above, one might be tempted to think this procedure is going to -add a lot of work. Fortunately, this is not the case, and we'll show why. For -this example we'll specify the cost of the work to do as a number, ranging from -0 to infinity. The greater the number, the more expensive the work is. The cost -does _not_ translate to time, it's just a way of measuring complexity of one -change relative to another. - -Let's say we are building a new feature, and we have determined that the cost of -this is 10. We have also determined that the cost of adding a feature flag check -in a variety of places is 1. If we do not use feature flags, and our feature -works as intended, our total cost is 10. This however is the best case scenario. -Optimizing for the best case scenario is guaranteed to lead to trouble, whereas -optimizing for the worst case scenario is almost always better. - -To illustrate this, let's say our feature causes an outage, and there's no -immediate way to resolve it. This means we'd have to take the following steps to -resolve the outage: - -1. Revert the release. -1. Perform any cleanups that might be necessary, depending on the changes that - were made. -1. Revert the commit, ensuring the "master" branch remains stable. This is - especially necessary if solving the problem can take days or even weeks. -1. Pick the revert commit into the appropriate stable branches, ensuring we - don't block any future releases until the problem is resolved. - -As history has shown, these steps are time consuming, complex, often involve -many developers, and worst of all: our users will have a bad experience using -GitLab.com until the problem is resolved. - -Now let's say that all of this has an associated cost of 10. This means that in -the worst case scenario, which we should optimize for, our total cost is now 20. - -If we had used a feature flag, things would have been very different. We don't -need to revert a release, and because feature flags are disabled by default we -don't need to revert and pick any Git commits. In fact, all we have to do is -disable the feature, and in the worst case, perform cleanup. Let's say that -the cost of this is 2. In this case, our best case cost is 11: 10 to build the -feature, and 1 to add the feature flag. The worst case cost is now 13: - -- 10 to build the feature. -- 1 to add the feature flag. -- 2 to disable and clean up. - -Here we can see that in the best case scenario the work necessary is only a tiny -bit more compared to not using a feature flag. Meanwhile, the process of -reverting our changes has been made significantly and reliably cheaper. - -In other words, feature flags do not slow down the development process. Instead, -they speed up the process as managing incidents now becomes _much_ easier. Once -continuous deployments are easier to perform, the time to iterate on a feature -is reduced even further, as you no longer need to wait weeks before your changes -are available on GitLab.com. - -### The benefits of feature flags - -It may seem like feature flags are configuration, which goes against our [convention-over-configuration](https://about.gitlab.com/handbook/product/product-principles/#convention-over-configuration) -principle. However, configuration is by definition something that is user-manageable. -Feature flags are not intended to be user-editable. Instead, they are intended as a tool for Engineers -and Site Reliability Engineers to use to de-risk their changes. Feature flags are the shim that gets us -to Continuous Delivery with our monorepo and without having to deploy the entire codebase on every change. -Feature flags are created to ensure that we can safely rollout our work on our terms. -If we use Feature Flags as a configuration, we are doing it wrong and are indeed in violation of our -principles. If something needs to be configured, we should intentionally make it configuration from the -first moment. - -Some of the benefits of using development-type feature flags are: - -1. It enables Continuous Delivery for GitLab.com. -1. It significantly reduces Mean-Time-To-Recovery. -1. It helps engineers to monitor and reduce the impact of their changes gradually, at any scale, - allowing us to be more metrics-driven and execute good DevOps practices, [shifting some responsibility "left"](https://devops.com/why-its-time-for-site-reliability-engineering-to-shift-left/). -1. Controlled feature rollout timing: without feature flags, we would need to wait until a specific - deployment was complete (which at GitLab could be at any time). -1. Increased psychological safety: when a feature flag is used, an engineer has the confidence that if anything goes wrong they can quickly disable the code and minimize the impact of a change that might be risky. -1. Improved throughput: when a change is less risky because a flag exists, theoretical tests about - scalability can potentially become unnecessary or less important. This allows an engineer to - potentially test a feature on a small project, monitor the impact, and proceed. The alternative might - be to build complex benchmarks locally, or on staging, or on another GitLab deployment, which has a - large impact on the time it can take to build and release a feature. +<!-- This redirect file can be deleted after 2021-06-01. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page -->
\ No newline at end of file diff --git a/doc/development/merge_request_performance_guidelines.md b/doc/development/merge_request_performance_guidelines.md index 8d5b2db828e..50362269c1b 100644 --- a/doc/development/merge_request_performance_guidelines.md +++ b/doc/development/merge_request_performance_guidelines.md @@ -459,7 +459,7 @@ Performance deficiencies should be addressed right away after we merge initial changes. Read more about when and how feature flags should be used in -[Feature flags in GitLab development](feature_flags/process.md#feature-flags-in-gitlab-development). +[Feature flags in GitLab development](https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle#feature-flags-in-gitlab-development). ## Storage diff --git a/doc/development/new_fe_guide/tips.md b/doc/development/new_fe_guide/tips.md index d38e261b99f..c60d70b3b16 100644 --- a/doc/development/new_fe_guide/tips.md +++ b/doc/development/new_fe_guide/tips.md @@ -16,18 +16,18 @@ yarn clean ## Creating feature flags in development -The process for creating a feature flag is the same as [enabling a feature flag in development](../feature_flags/development.md#enabling-a-feature-flag-locally-in-development). +The process for creating a feature flag is the same as [enabling a feature flag in development](../feature_flags/index.md#enabling-a-feature-flag-locally-in-development). Your feature flag can now be: -- [Made available to the frontend](../feature_flags/development.md#frontend) via the `gon` -- Queried in [tests](../feature_flags/development.md#feature-flags-in-tests) +- [Made available to the frontend](../feature_flags/index.md#frontend) via the `gon` +- Queried in [tests](../feature_flags/index.md#feature-flags-in-tests) - Queried in HAML templates and Ruby files via the `Feature.enabled?(:my_shiny_new_feature_flag)` method ### More on feature flags - [Deleting a feature flag](../../api/features.md#delete-a-feature) -- [Manage feature flags](../feature_flags/process.md) +- [Manage feature flags](https://about.gitlab.com/handbook/product-development-flow/feature-flag-lifecycle) - [Feature flags API](../../api/features.md) ## Running tests locally diff --git a/doc/development/packages.md b/doc/development/packages.md index 5a3ca79f992..e8c326da974 100644 --- a/doc/development/packages.md +++ b/doc/development/packages.md @@ -122,7 +122,7 @@ There are usually 2 phases for the MVC: When implementing a new package manager, it is tempting to create one large merge request containing all of the necessary endpoints and services necessary to support basic usage. Instead, put the -API endpoints behind a [feature flag](feature_flags/development.md) and +API endpoints behind a [feature flag](feature_flags/index.md) and submit each endpoint or behavior (download, upload, etc) in a different merge request to shorten the review process. diff --git a/doc/development/service_measurement.md b/doc/development/service_measurement.md index 91fb8248db4..895ac540838 100644 --- a/doc/development/service_measurement.md +++ b/doc/development/service_measurement.md @@ -75,7 +75,7 @@ To actually use it, you need to enable measuring for the desired service by enab ### Enabling measurement using feature flags In the following example, the `:gitlab_service_measuring_projects_import_service` -[feature flag](feature_flags/development.md#enabling-a-feature-flag-locally-in-development) is used to enable the measuring feature +[feature flag](feature_flags/index.md#enabling-a-feature-flag-locally-in-development) is used to enable the measuring feature for `Projects::ImportService`. From ChatOps: diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index ad16f3201e2..ee8401e08d4 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -545,7 +545,7 @@ end ### Feature flags in tests -This section was moved to [developing with feature flags](../feature_flags/development.md). +This section was moved to [developing with feature flags](../feature_flags/index.md). ### Pristine test environments diff --git a/doc/development/testing_guide/end_to_end/feature_flags.md b/doc/development/testing_guide/end_to_end/feature_flags.md index 1bc33b79c7c..e3719393d41 100644 --- a/doc/development/testing_guide/end_to_end/feature_flags.md +++ b/doc/development/testing_guide/end_to_end/feature_flags.md @@ -18,8 +18,8 @@ Please be sure to include the tag `:requires_admin` so that the test can be skip where admin access is not available. WARNING: -You are strongly advised to [enable feature flags only for a group, project, user](../../feature_flags/development.md#feature-actors), -or [feature group](../../feature_flags/development.md#feature-groups). This makes it possible to +You are strongly advised to [enable feature flags only for a group, project, user](../../feature_flags/index.md#feature-actors), +or [feature group](../../feature_flags/index.md#feature-groups). This makes it possible to test a feature in a shared environment without affecting other users. For example, the code below would enable a feature flag named `:feature_flag_name` for the project diff --git a/doc/development/usage_ping.md b/doc/development/usage_ping.md index c340f1eb10d..b8f08caaebd 100644 --- a/doc/development/usage_ping.md +++ b/doc/development/usage_ping.md @@ -3,6 +3,5 @@ redirect_to: 'usage_ping/index.md' --- This document was moved to [another location](usage_ping/index.md). - <!-- This redirect file can be deleted after <2021-05-23>. --> -<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page -->
\ No newline at end of file diff --git a/doc/development/usage_ping/index.md b/doc/development/usage_ping/index.md index 40635e20495..604d59f42e1 100644 --- a/doc/development/usage_ping/index.md +++ b/doc/development/usage_ping/index.md @@ -947,7 +947,7 @@ Each aggregate definition includes following parts: relay on the same data source. Additional data source requirements are described in the [Database sourced aggregated metrics](#database-sourced-aggregated-metrics) and [Redis sourced aggregated metrics](#redis-sourced-aggregated-metrics) sections. -- `feature_flag`: Name of [development feature flag](../feature_flags/development.md#development-type) +- `feature_flag`: Name of [development feature flag](../feature_flags/index.md#development-type) that is checked before metrics aggregation is performed. Corresponding feature flag should have `default_enabled` attribute set to `false`. The `feature_flag` attribute is optional and can be omitted. When `feature_flag` is missing, no feature flag is checked. diff --git a/doc/user/analytics/index.md b/doc/user/analytics/index.md index f5da373ee6d..fe35d575373 100644 --- a/doc/user/analytics/index.md +++ b/doc/user/analytics/index.md @@ -61,6 +61,6 @@ The following analytics features are available at the project level: - [Insights](../project/insights/index.md). **(ULTIMATE)** - [Issue](../group/issues_analytics/index.md). **(PREMIUM)** - [Merge Request](merge_request_analytics.md), enabled with the `project_merge_request_analytics` - [feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-locally-in-development). **(PREMIUM)** + [feature flag](../../development/feature_flags/index.md#enabling-a-feature-flag-locally-in-development). **(PREMIUM)** - [Repository](repository_analytics.md). **(FREE)** - [Value Stream](value_stream_analytics.md). **(FREE)** diff --git a/doc/user/group/index.md b/doc/user/group/index.md index d632fc47249..655a0dcb00f 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -242,7 +242,7 @@ a [beta feature](https://about.gitlab.com/handbook/product/#beta). For a group, you can view how many merge requests, issues, and members were created in the last 90 days. -These Group Activity Analytics can be enabled with the `group_activity_analytics` [feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-locally-in-development). +These Group Activity Analytics can be enabled with the `group_activity_analytics` [feature flag](../../development/feature_flags/index.md#enabling-a-feature-flag-locally-in-development).  diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index 61da664f134..5079b927fbe 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -103,7 +103,6 @@ With this option enabled, users must go through your group's GitLab single sign- However, users are not prompted to sign in through SSO on each visit. GitLab checks whether a user has authenticated through SSO. If it's been more than 1 day since the last sign-in, GitLab prompts the user to sign in again through SSO. -You can see more information about how long a session is valid in our [user profile documentation](../../profile/#why-do-i-keep-getting-signed-out). We intend to add a similar SSO requirement for [Git and API activity](https://gitlab.com/gitlab-org/gitlab/-/issues/9152). diff --git a/doc/user/img/gitlab_snippet_v13_0.png b/doc/user/img/gitlab_snippet_v13_0.png Binary files differdeleted file mode 100644 index 33194c512df..00000000000 --- a/doc/user/img/gitlab_snippet_v13_0.png +++ /dev/null diff --git a/doc/user/img/gitlab_snippet_v13_5.png b/doc/user/img/gitlab_snippet_v13_5.png Binary files differdeleted file mode 100644 index 3fce1d25c3d..00000000000 --- a/doc/user/img/gitlab_snippet_v13_5.png +++ /dev/null diff --git a/doc/user/img/new_personal_snippet_from_project_v12_10.png b/doc/user/img/new_personal_snippet_from_project_v12_10.png Binary files differdeleted file mode 100644 index af8cce01208..00000000000 --- a/doc/user/img/new_personal_snippet_from_project_v12_10.png +++ /dev/null diff --git a/doc/user/img/new_personal_snippet_v12_10.png b/doc/user/img/new_personal_snippet_v12_10.png Binary files differdeleted file mode 100644 index a995e4d4b40..00000000000 --- a/doc/user/img/new_personal_snippet_v12_10.png +++ /dev/null diff --git a/doc/user/packages/generic_packages/index.md b/doc/user/packages/generic_packages/index.md index 8d5a41a6e7d..b9186e99357 100644 --- a/doc/user/packages/generic_packages/index.md +++ b/doc/user/packages/generic_packages/index.md @@ -53,7 +53,7 @@ PUT /projects/:id/packages/generic/:package_name/:package_version/:file_name?sta Provide the file context in the request body. -Example request: +Example request using a personal access token: ```shell curl --header "PRIVATE-TOKEN: <your_access_token>" \ @@ -69,6 +69,22 @@ Example response: } ``` +Example request using a deploy token: + +```shell +curl --header "DEPLOY-TOKEN: <deploy_token>" \ + --upload-file path/to/file.txt \ + "https://gitlab.example.com/api/v4/projects/24/packages/generic/my_package/0.0.1/file.txt?status=hidden" +``` + +Example response: + +```json +{ + "message":"201 Created" +} +``` + ## Download package file Download a package file. diff --git a/doc/user/profile/index.md b/doc/user/profile/index.md index 6a13ec4311c..65d83c2b413 100644 --- a/doc/user/profile/index.md +++ b/doc/user/profile/index.md @@ -40,7 +40,7 @@ On your profile page, you can see the following information: - Contributed projects: [projects](../project/index.md) you contributed to - Personal projects: your personal projects (respecting the project's visibility level) - Starred projects: projects you starred -- Snippets: your personal code [snippets](../snippets.md#personal-snippets) +- Snippets: your personal code [snippets](../snippets.md) - Followers: people [following](../index.md#user-activity) you - Following: people you are [following](../index.md#user-activity) diff --git a/doc/user/project/deploy_tokens/index.md b/doc/user/project/deploy_tokens/index.md index 3c846db7b47..8606ce36a82 100644 --- a/doc/user/project/deploy_tokens/index.md +++ b/doc/user/project/deploy_tokens/index.md @@ -130,6 +130,22 @@ To pull packages in the GitLab package registry, you must: 1. For the [package type of your choice](../../packages/index.md), follow the authentication instructions for deploy tokens. +Example request publishing a generic package using a deploy token: + +```shell +curl --header "DEPLOY-TOKEN: <deploy_token>" \ + --upload-file path/to/file.txt \ + "https://gitlab.example.com/api/v4/projects/24/packages/generic/my_package/0.0.1/file.txt?status=hidden" +``` + +Example response: + +```json +{ + "message":"201 Created" +} +``` + ### Push or upload packages > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/213566) in GitLab 13.0. diff --git a/doc/user/project/issues/index.md b/doc/user/project/issues/index.md index 7a7c3578d42..c97196ff861 100644 --- a/doc/user/project/issues/index.md +++ b/doc/user/project/issues/index.md @@ -137,7 +137,7 @@ You can mark two issues as related, so that when viewing one, the other is alway listed in its [Related Issues](related_issues.md) section. This can help display important context, such as past work, dependencies, or duplicates. -Users on [GitLab Starter, GitLab Bronze, and higher tiers](https://about.gitlab.com/pricing/), can +Users of [GitLab Premium](https://about.gitlab.com/pricing/) or higher can also mark issues as blocking or blocked by another issue. ### Crosslinking issues @@ -162,9 +162,9 @@ Up to five similar issues, sorted by most recently updated, are displayed below ### Health status **(ULTIMATE)** -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/36427) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 12.10. -> - Health status of closed issues [can't be edited](https://gitlab.com/gitlab-org/gitlab/-/issues/220867) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.4 and later. -> - Issue health status visible in issue lists [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45141) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.6. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/36427) in GitLab Ultimate 12.10. +> - Health status of closed issues [can't be edited](https://gitlab.com/gitlab-org/gitlab/-/issues/220867) in GitLab Ultimate 13.4 and later. +> - Issue health status visible in issue lists [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45141) in GitLab Ultimate 13.6. > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/213567) in GitLab 13.7. To help you track issue statuses, you can assign a status to each issue. diff --git a/doc/user/project/settings/index.md b/doc/user/project/settings/index.md index c3427830426..785618a862a 100644 --- a/doc/user/project/settings/index.md +++ b/doc/user/project/settings/index.md @@ -44,6 +44,7 @@ Compliance framework labels do not affect your project settings. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/276221) in GitLab 13.9. > - It's [deployed behind a feature flag](../../feature_flags.md), disabled by default. +> - It can be enabled or disabled for a single group > - It's disabled on GitLab.com. > - It's not recommended for production use. > - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-custom-compliance-frameworks). **(PREMIUM)** @@ -329,11 +330,11 @@ can enable it. To enable it: ```ruby -Feature.enable(:ff_custom_compliance_frameworks) +Feature.enable(:ff_custom_compliance_frameworks, Group.find(<group id>)) ``` To disable it: ```ruby -Feature.disable(:ff_custom_compliance_frameworks) +Feature.disable(:ff_custom_compliance_frameworks, Group.find(<group id>)) ``` diff --git a/doc/user/snippets.md b/doc/user/snippets.md index 30da6eceaf4..a2a18c0e8de 100644 --- a/doc/user/snippets.md +++ b/doc/user/snippets.md @@ -7,83 +7,98 @@ type: reference # Snippets **(FREE)** -With GitLab Snippets you can store and share bits of code and text with other users. +With GitLab snippets, you can store and share bits of code and text with other users. +You can [comment on](#comment-on-snippets), [clone](#clone-snippets), and +[use version control](#versioned-snippets) in snippets. They can +[contain multiple files](#add-or-remove-multiple-files). They also support +[syntax highlighting](#filenames), [embedding](#embed-snippets), [downloading](#download-snippets), +and you can maintain your snippets with the [snippets API](../api/snippets.md). + +GitLab provides two types of snippets: + +- **Personal snippets**: Created independent of any project. + You can set a [visibility level](../public_access/public_access.md) + for your snippet: public, internal, or private. +- **Project snippets**: Always related to a specific project. + Project snippets can be visible publicly or to only group members. + +## Create snippets + +You can create snippets in multiple ways, depending on whether you want to create a personal or project snippet: + +1. Select the kind of snippet you want to create: + - **To create a personal snippet**: + - *If you're on a project's page,* select the plus icon (**{plus-square-o}**) + in the top navigation bar, then select **New snippet** from the **GitLab** (for GitLab.com) + or **Your Instance** (self-managed) section of the same dropdown menu. + - *For all other pages,* select the plus icon (**{plus-square-o}**) + in the top navigation bar, then select **New snippet** from the dropdown menu. + - **To create a project snippet**: Go to your project's page. Select the plus icon + (**{plus-square-o}**), then select **New snippet** from the **This project** section + of the dropdown menu. +1. Add a **Title** and **Description**. +1. Name your **File** with an appropriate extension, such as `example.rb` or `index.html`. + Filenames with appropriate extensions display [syntax highlighting](#filenames). + Failure to add a filename can cause a known + [copy-pasting bug](https://gitlab.com/gitlab-org/gitlab/-/issues/22870). If you don't provide a filename, GitLab [creates a name for you](#filenames). +1. (Optional) Add [multiple files](#add-or-remove-multiple-files) to your snippet. +1. Select a visibility level, and select **Create snippet**. + +After you create a snippet, you can still [add more files to it](#add-or-remove-multiple-files). +In GitLab versions 13.0 and later, snippets are [versioned by default](#versioned-snippets). - - -Snippets can be maintained using [snippets API](../api/snippets.md). - -There are two types of snippets: - -- Personal snippets. -- Project snippets. - -## Personal snippets - -Personal snippets are not related to any project and can be created completely -independently. There are 3 visibility levels that can be set, public, internal -and private. See [Public access](../public_access/public_access.md) for more information. - -## Project snippets - -Project snippets are always related to a specific project. -See [Project features](project/index.md#project-features) for more information. - -## Create a snippet - -To create a personal snippet, click the plus icon (**{plus-square-o}**) -on the top navigation and select **New snippet** from the dropdown menu: - - - -If you're on a project's page but you want to create a new personal snippet, -click the plus icon (**{plus-square-o}**) and select **New snippet** from the -lower part of the dropdown (**GitLab** on GitLab.com; **Your Instance** on -self-managed instances): +## Discover snippets - +To discover all snippets visible to you in GitLab, you can: -To create a project snippet, navigate to your project's page and click the -plus icon (**{plus-square-o}**), then select **New snippet** from the upper -part of the dropdown (**This project**). +- **View all snippets visible to you**: In the top navigation bar of your GitLab + instance, go to **More > Snippets** to view your snippets dashboard. +- **Visit [GitLab snippets](http://snippets.gitlab.com/)** for your snippets on GitLab.com. +- **Explore all public snippets**: In the top navigation bar of your GitLab + instance, go to **More > Snippets** and select **Explore snippets** to view + [all public snippets](https://gitlab.com/explore/snippets). +- **View a project's snippets**: In your project, + go to **Snippets**. - +## Change default visibility of snippets -From there, add the **Title**, **Description**, and a **File** name with the -appropriate extension (for example, `example.rb`, `index.html`). +Project snippets are enabled and available by default. To change their +default visibility: -WARNING: -Make sure to add the filename to get code highlighting and to avoid this -[copy-pasting bug](https://gitlab.com/gitlab-org/gitlab/-/issues/22870). +1. In your project, + go to **Settings**. +1. Expand the **Visibility, project features, permissions** section, and scroll to **Snippets**. +1. Toggle the default visibility, and select whether snippets can be viewed by + everyone, or only project members. +1. Select **Save changes**. -## Versioned Snippets +## Versioned snippets > [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/239) in GitLab 13.0. -Starting in 13.0, snippets (both personal and project snippets) +In GitLab versions 13.0 and later, snippets (both personal and project snippets) have version control enabled by default. This means that all snippets get their own underlying repository initialized with -a `master` branch at the moment the snippet is created. Whenever a change to the snippet is saved, a -new commit to the `master` branch is recorded. Commit messages are automatically -generated. The snippet's repository has only one branch (`master`) by default, deleting -it or creating other branches is not supported. +a default branch at the moment the snippet is created. Whenever a change to the snippet is saved, a +new commit to the default branch is recorded. Commit messages are automatically +generated. The snippet's repository has only one branch. You can't delete this branch, +or create other branches. -Existing snippets are automatically migrated in 13.0. Their current -content is saved as the initial commit to the snippets' repository. +Existing snippets were automatically migrated in 13.0. Their current +content was saved as the initial commit to the snippets' repository. -### Filenames +## Filenames Snippets support syntax highlighting based on the filename and extension provided for them. While you can submit a snippet -without specifying a filename and extension, it needs a valid name so the +without a filename and extension, it needs a valid name so the content can be created as a file in the snippet's repository. -If you don't attribute a filename and extension to a snippet, +If you don't give a snippet a filename and extension, GitLab adds a filename in the format `snippetfile<x>.txt` where `<x>` represents a number added to the file, starting with 1. This -number increments when more snippets without an attributed -filename are added. +number increments if you add more unnamed snippets. When upgrading from an earlier version of GitLab to 13.0, existing snippets without a supported filename are renamed to a compatible format. For @@ -92,139 +107,110 @@ changed to `http-a-weird-filename-me` to be included in the snippet's repository. As snippets are stored by ID, changing their filenames breaks direct or embedded links to the snippet. -### Multiple files by Snippet +## Add or remove multiple files > [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2829) in GitLab 13.5. -GitLab Snippets support multiple files in one single snippet. This is helpful -when your code snippet is composed of multiple parts or when they relate -to a certain context. For example: +A single snippet can support up to 10 files, which helps keep related files together, such as: - A snippet that includes a script and its output. -- A snippet that includes HTML, CSS, and JS code. +- A snippet that includes HTML, CSS, and JavaScript code. - A snippet with a `docker-compose.yml` file and its associated `.env` file. -- A `gulpfile.js` file coupled with a `package.json` file, which together can be +- A `gulpfile.js` file and a `package.json` file, which together can be used to bootstrap a project and manage its dependencies. -Snippets support between 1 and 10 files. They can be managed via Git (because they're [versioned](#versioned-snippets) -by a Git repository), through the [Snippets API](../api/snippets.md), or in the GitLab UI. - - +You can manage these by using Git (because they're [versioned](#versioned-snippets) +by a Git repository), through the [Snippets API](../api/snippets.md), and in the GitLab UI. To add a new file to your snippet through the GitLab UI: 1. Go to your snippet in the GitLab UI. -1. Click **Edit** in the top right. +1. Select **Edit** in the top right corner. 1. Select **Add another file**. 1. Add your content to the file in the form fields provided. -1. Click **Save changes**. +1. Select **Save changes**. To delete a file from your snippet through the GitLab UI: 1. Go to your snippet in the GitLab UI. -1. Click **Edit** in the top right. +1. Select **Edit** in the top right corner. 1. Select **Delete file** alongside the filename of each file you wish to delete. -1. Click **Save changes**. - -### Cloning snippets - -Snippets can be cloned as a regular Git repository using SSH or HTTPS. Click the **Clone** -button above the snippet content to copy the URL of your choice. +1. Select **Save changes**. - +## Clone snippets -This allows you to have a local copy of the snippet's repository and make -changes as needed. You can commit those changes and push them to the remote -`master` branch. +Instead of copying a snippet to a local file, you may want to clone a snippet to +preserve its relationship with the repository, so you can receive or make updates +as needed. Select the **Clone** button on a snippet to display the URLs to clone with SSH or HTTPS: -### Reduce snippets repository size + -Because versioned Snippets are considered as part of the [namespace storage size](../user/admin_area/settings/account_and_limit_settings.md), -it's recommended to keep snippets' repositories as compact as possible. +You can commit changes to a cloned snippet, and push the changes to GitLab. -For more information about tools to compact repositories, -see the documentation on [reducing repository size](../user/project/repository/reducing_the_repo_size_using_git.md). +## Embed snippets -### Limitations +Public snippets can be shared and embedded on any website. You can reuse a GitLab snippet in multiple places, and any change to the source +is reflected in the embedded snippets. When embedded, users can download it, or view the snippet in raw format. -- Binary files are not supported. -- Creating or deleting branches is not supported. Only a default `master` branch is used. -- Git tags are not supported in snippet repositories. -- Snippets' repositories are limited to 10 files. Attempting to push more - than 10 files results in an error. -- Revisions are not visible to the user on the GitLab UI, but this feature is planned - in future iterations. See the [revisions tab issue](https://gitlab.com/gitlab-org/gitlab/-/issues/39271) - for updates. -- The [maximum size for a snippet](../administration/snippets/index.md#snippets-content-size-limit) - is 50 MB, by default. -- Git LFS is not supported. +To embed a snippet: -## Discover snippets +1. Confirm your snippet is publicly visible: + - *If it's a project snippet*, the project must be public. + - The snippet is publicly visible. + - In **Project > Settings > Permissions**, the snippets + permissions are set to **Everyone with access**. +1. In your snippet's **Embed** section, select **Copy** to copy a one-line script you can add to any website or blog post. For example: -There are two main ways of how you can discover snippets in GitLab. + ```html + <script src="https://gitlab.com/namespace/project/snippets/SNIPPET_ID.js"></script> + ``` -For exploring all snippets that are visible to you, you can go to the Snippets -dashboard of your GitLab instance via the top navigation. For GitLab.com you can -visit [GitLab Snippets](http://snippets.gitlab.com/) or navigate to an [overview]((https://gitlab.com/dashboard/snippets)) that shows snippets -you created and allows you to explore all snippets. +1. Add your script to your file. -To discover snippets that belong to a specific project, navigate -to the Snippets page via the left side navigation on the project page. -Project snippets are enabled and available by default. You can -disable them by navigating to your project's **Settings**, expanding -**Visibility, project features, permissions** and scrolling down to -**Snippets**. From there, you can toggle to disable them or select a -different visibility level from the dropdown menu. +Embedded snippets display a header that shows: -## Snippet comments - -> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/12910) in GitLab 9.2. +- The filename, if defined. +- The snippet size. +- A link to GitLab. +- The actual snippet content. -With GitLab Snippets you engage in a conversation about that piece of code, -encouraging user collaboration. +For example: -## Downloading snippets +<script src="https://gitlab.com/gitlab-org/gitlab-foss/snippets/1717978.js"></script> -You can download the raw content of a snippet. +## Download snippets -By default snippets are downloaded with Linux-style line endings (`LF`). If +You can download the raw content of a snippet. By default, they download with Linux-style line endings (`LF`). If you want to preserve the original line endings you need to add a parameter `line_ending=raw` (For example: `https://gitlab.com/snippets/SNIPPET_ID/raw?line_ending=raw`). In case a snippet was created using the GitLab web interface the original line ending is Windows-like (`CRLF`). -## Embedded snippets - -> Introduced in GitLab 10.8. - -Public snippets can not only be shared, but also embedded on any website. With -this, you can reuse a GitLab snippet in multiple places and any change to the source -is automatically reflected in the embedded snippet. +## Comment on snippets -To embed a snippet, first make sure that: +With snippets, you engage in a conversation about that piece of code, +which can encourage user collaboration. -- The project is public (if it's a project snippet) -- The snippet is public -- In **Project > Settings > Permissions**, the snippets permissions are - set to **Everyone with access** +## Troubleshooting -After the above conditions are met, the **Embed** section appears in your -snippet. Click the **Copy** button to copy a one-line -script that you can add to any website or blog post. For example: +### Snippet limitations -```html -<script src="https://gitlab.com/namespace/project/snippets/SNIPPET_ID.js"></script> -``` - -Here's what an embedded snippet looks like: - -<script src="https://gitlab.com/gitlab-org/gitlab-foss/snippets/1717978.js"></script> +- Binary files are not supported. +- Creating or deleting branches is not supported. Only the default branch is used. +- Git tags are not supported in snippet repositories. +- Snippets' repositories are limited to 10 files. Attempting to push more + than 10 files results in an error. +- Revisions are not visible to the user on the GitLab UI, but this feature is planned + in future iterations. See the [revisions tab issue](https://gitlab.com/gitlab-org/gitlab/-/issues/39271) + for updates. +- The [maximum size for a snippet](../administration/snippets/index.md#snippets-content-size-limit) + is 50 MB, by default. +- Git LFS is not supported. -Embedded snippets are displayed with a header that shows: +### Reduce snippets repository size -- The filename, if defined. -- The snippet size. -- A link to GitLab. -- The actual snippet content. +Because versioned snippets are considered as part of the [namespace storage size](../user/admin_area/settings/account_and_limit_settings.md), +it's recommended to keep snippets' repositories as compact as possible. -Actions in the header enable users to see the snippet in raw format, and download it. +For more information about tools to compact repositories, +see the documentation on [reducing repository size](../user/project/repository/reducing_the_repo_size_using_git.md). diff --git a/lib/api/rubygem_packages.rb b/lib/api/rubygem_packages.rb index 136e217b982..8d2d4586d8d 100644 --- a/lib/api/rubygem_packages.rb +++ b/lib/api/rubygem_packages.rb @@ -64,8 +64,15 @@ module API requires :file_name, type: String, desc: 'Package file name' end get "gems/:file_name", requirements: FILE_NAME_REQUIREMENTS do - # To be implemented in https://gitlab.com/gitlab-org/gitlab/-/issues/299283 - not_found! + authorize!(:read_package, user_project) + + package_file = ::Packages::PackageFile.for_rubygem_with_file_name( + user_project, params[:file_name] + ).last! + + track_package_event('pull_package', :rubygems) + + present_carrierwave_file!(package_file.file) end namespace 'api/v1' do diff --git a/lib/gitlab/ci/templates/Terraform/Base.latest.gitlab-ci.yml b/lib/gitlab/ci/templates/Terraform/Base.latest.gitlab-ci.yml index c2db0fc44f1..200388a274c 100644 --- a/lib/gitlab/ci/templates/Terraform/Base.latest.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Terraform/Base.latest.gitlab-ci.yml @@ -52,7 +52,8 @@ cache: - gitlab-terraform apply when: manual only: - - master + variables: + - $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH .destroy: &destroy stage: cleanup diff --git a/lib/gitlab/database/migrations/background_migration_helpers.rb b/lib/gitlab/database/migrations/background_migration_helpers.rb index 12dcf68da2f..e8cbea72887 100644 --- a/lib/gitlab/database/migrations/background_migration_helpers.rb +++ b/lib/gitlab/database/migrations/background_migration_helpers.rb @@ -4,8 +4,12 @@ module Gitlab module Database module Migrations module BackgroundMigrationHelpers - BACKGROUND_MIGRATION_BATCH_SIZE = 1_000 # Number of rows to process per job - BACKGROUND_MIGRATION_JOB_BUFFER_SIZE = 1_000 # Number of jobs to bulk queue at a time + BATCH_SIZE = 1_000 # Number of rows to process per job + SUB_BATCH_SIZE = 100 # Number of rows to process per sub-batch + JOB_BUFFER_SIZE = 1_000 # Number of jobs to bulk queue at a time + BATCH_CLASS_NAME = 'PrimaryKeyBatchingStrategy' # Default batch class for batched migrations + BATCH_MIN_VALUE = 1 # Default minimum value for batched migrations + BATCH_MIN_DELAY = 2.minutes.freeze # Minimum delay between batched migrations # Bulk queues background migration jobs for an entire table, batched by ID range. # "Bulk" meaning many jobs will be pushed at a time for efficiency. @@ -31,7 +35,7 @@ module Gitlab # # do something # end # end - def bulk_queue_background_migration_jobs_by_range(model_class, job_class_name, batch_size: BACKGROUND_MIGRATION_BATCH_SIZE) + def bulk_queue_background_migration_jobs_by_range(model_class, job_class_name, batch_size: BATCH_SIZE) raise "#{model_class} does not have an ID to use for batch ranges" unless model_class.column_names.include?('id') jobs = [] @@ -40,7 +44,7 @@ module Gitlab model_class.each_batch(of: batch_size) do |relation| start_id, end_id = relation.pluck("MIN(#{table_name}.id)", "MAX(#{table_name}.id)").first - if jobs.length >= BACKGROUND_MIGRATION_JOB_BUFFER_SIZE + if jobs.length >= JOB_BUFFER_SIZE # Note: This code path generally only helps with many millions of rows # We push multiple jobs at a time to reduce the time spent in # Sidekiq/Redis operations. We're using this buffer based approach so we @@ -89,7 +93,7 @@ module Gitlab # # do something # end # end - def queue_background_migration_jobs_by_range_at_intervals(model_class, job_class_name, delay_interval, batch_size: BACKGROUND_MIGRATION_BATCH_SIZE, other_job_arguments: [], initial_delay: 0, track_jobs: false, primary_column_name: :id) + def queue_background_migration_jobs_by_range_at_intervals(model_class, job_class_name, delay_interval, batch_size: BATCH_SIZE, other_job_arguments: [], initial_delay: 0, track_jobs: false, primary_column_name: :id) raise "#{model_class} does not have an ID column of #{primary_column_name} to use for batch ranges" unless model_class.column_names.include?(primary_column_name.to_s) raise "#{primary_column_name} is not an integer column" unless model_class.columns_hash[primary_column_name.to_s].type == :integer @@ -127,6 +131,79 @@ module Gitlab final_delay end + # Creates a batched background migration for the given table. A batched migration runs one job + # at a time, computing the bounds of the next batch based on the current migration settings and the previous + # batch bounds. Each job's execution status is tracked in the database as the migration runs. The given job + # class must be present in the Gitlab::BackgroundMigration module, and the batch class (if specified) must be + # present in the Gitlab::BackgroundMigration::BatchingStrategies module. + # + # job_class_name - The background migration job class as a string + # batch_table_name - The name of the table the migration will batch over + # batch_column_name - The name of the column the migration will batch over + # job_arguments - Extra arguments to pass to the job instance when the migration runs + # job_interval - The pause interval between each job's execution, minimum of 2 minutes + # batch_min_value - The value in the column the batching will begin at + # batch_max_value - The value in the column the batching will end at, defaults to `SELECT MAX(batch_column)` + # batch_class_name - The name of the class that will be called to find the range of each next batch + # batch_size - The maximum number of rows per job + # sub_batch_size - The maximum number of rows processed per "iteration" within the job + # + # + # *Returns the created BatchedMigration record* + # + # Example: + # + # queue_batched_background_migration( + # 'CopyColumnUsingBackgroundMigrationJob', + # :events, + # :id, + # job_interval: 2.minutes, + # other_job_arguments: ['column1', 'column2']) + # + # Where the the background migration exists: + # + # class Gitlab::BackgroundMigration::CopyColumnUsingBackgroundMigrationJob + # def perform(start_id, end_id, batch_table, batch_column, sub_batch_size, *other_args) + # # do something + # end + # end + def queue_batched_background_migration( # rubocop:disable Metrics/ParameterLists + job_class_name, + batch_table_name, + batch_column_name, + *job_arguments, + job_interval:, + batch_min_value: BATCH_MIN_VALUE, + batch_max_value: nil, + batch_class_name: BATCH_CLASS_NAME, + batch_size: BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + + job_interval = BATCH_MIN_DELAY if job_interval < BATCH_MIN_DELAY + + batch_max_value ||= connection.select_value(<<~SQL) + SELECT MAX(#{connection.quote_column_name(batch_column_name)}) + FROM #{connection.quote_table_name(batch_table_name)} + SQL + + migration_status = batch_max_value.nil? ? :finished : :active + batch_max_value ||= batch_min_value + + Gitlab::Database::BackgroundMigration::BatchedMigration.create!( + job_class_name: job_class_name, + table_name: batch_table_name, + column_name: batch_column_name, + interval: job_interval, + min_value: batch_min_value, + max_value: batch_max_value, + batch_class_name: batch_class_name, + batch_size: batch_size, + sub_batch_size: sub_batch_size, + job_arguments: job_arguments, + status: migration_status) + end + def perform_background_migration_inline? Rails.env.test? || Rails.env.development? end diff --git a/qa/qa/page/project/issue/index.rb b/qa/qa/page/project/issue/index.rb index bd1e44a9d82..032f02268bc 100644 --- a/qa/qa/page/project/issue/index.rb +++ b/qa/qa/page/project/issue/index.rb @@ -15,18 +15,21 @@ module QA element :avatar_counter_content end + view 'app/assets/javascripts/issuable/components/csv_export_modal.vue' do + element :export_issuable_modal + end + view 'app/assets/javascripts/issuable/components/csv_import_export_buttons.vue' do element :export_as_csv_button - element :import_issues_dropdown element :import_from_jira_link end - view 'app/views/shared/issuable/csv_export/_modal.html.haml' do - element :export_issues_button + view 'app/views/projects/issues/import_csv/_button.html.haml' do + element :import_issues_button end - view 'app/assets/javascripts/issuable/components/csv_export_modal.vue' do - element :export_issuable_modal + view 'app/views/shared/issuable/csv_export/_modal.html.haml' do + element :export_issues_button end view 'app/views/shared/issuable/_nav.html.haml' do @@ -60,7 +63,7 @@ module QA def click_import_issues_dropdown # When there are no issues, the image that loads causes the buttons to jump has_loaded_all_images? - click_element(:import_issues_dropdown) + click_element(:import_issues_button) end def export_issues_modal diff --git a/spec/experiments/application_experiment_spec.rb b/spec/experiments/application_experiment_spec.rb index a0fe9f0f310..1a0f35846fe 100644 --- a/spec/experiments/application_experiment_spec.rb +++ b/spec/experiments/application_experiment_spec.rb @@ -61,38 +61,19 @@ RSpec.describe ApplicationExperiment, :experiment do it "tracks the assignment" do expect(subject).to receive(:track).with(:assignment) - subject.publish(nil) + subject.publish end - context "when inside a request cycle" do - before do - subject.context.instance_variable_set(:@request, double('Request', headers: 'true')) - end + it "pushes the experiment knowledge into the client using Gon" do + expect(Gon).to receive(:push).with({ experiment: { 'namespaced/stub' => subject.signature } }, true) - it "pushes the experiment knowledge into the client using Gon" do - expect(Gon).to receive(:push).with( - { - experiment: { - 'namespaced/stub' => { # string key because it can be namespaced - experiment: 'namespaced/stub', - key: '86208ac54ca798e11f127e8b23ec396a', - variant: 'control' - } - } - }, - true - ) - - subject.publish(nil) - end + subject.publish end - context "when outside a request cycle" do - it "does not push to gon when outside request cycle" do - expect(Gon).not_to receive(:push) + it "handles when Gon raises exceptions (like when it can't be pushed into)" do + expect(Gon).to receive(:push).and_raise(NoMethodError) - subject.publish(nil) - end + expect { subject.publish }.not_to raise_error end end diff --git a/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js b/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js index 885e02ef60f..da19265ce82 100644 --- a/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js +++ b/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js @@ -1,6 +1,7 @@ import MockAdapter from 'axios-mock-adapter'; import { TEST_HOST } from 'helpers/test_constants'; import testAction from 'helpers/vuex_action_helper'; +import service from '~/batch_comments/services/drafts_service'; import * as actions from '~/batch_comments/stores/modules/batch_comments/actions'; import axios from '~/lib/utils/axios_utils'; @@ -201,6 +202,12 @@ describe('Batch comments store actions', () => { describe('updateDraft', () => { let getters; + service.update = jest.fn(); + service.update.mockResolvedValue({ data: { id: 1 } }); + + const commit = jest.fn(); + let context; + let params; beforeEach(() => { getters = { @@ -208,43 +215,43 @@ describe('Batch comments store actions', () => { draftsPath: TEST_HOST, }, }; - }); - it('commits RECEIVE_DRAFT_UPDATE_SUCCESS with returned data', (done) => { - const commit = jest.fn(); - const context = { + context = { getters, commit, }; res = { id: 1 }; mock.onAny().reply(200, res); + params = { note: { id: 1 }, noteText: 'test' }; + }); - actions - .updateDraft(context, { note: { id: 1 }, noteText: 'test', callback() {} }) - .then(() => { - expect(commit).toHaveBeenCalledWith('RECEIVE_DRAFT_UPDATE_SUCCESS', { id: 1 }); - }) - .then(done) - .catch(done.fail); + afterEach(() => jest.clearAllMocks()); + + it('commits RECEIVE_DRAFT_UPDATE_SUCCESS with returned data', () => { + return actions.updateDraft(context, { ...params, callback() {} }).then(() => { + expect(commit).toHaveBeenCalledWith('RECEIVE_DRAFT_UPDATE_SUCCESS', { id: 1 }); + }); }); - it('calls passed callback', (done) => { - const commit = jest.fn(); - const context = { - getters, - commit, - }; + it('calls passed callback', () => { const callback = jest.fn(); - res = { id: 1 }; - mock.onAny().reply(200, res); + return actions.updateDraft(context, { ...params, callback }).then(() => { + expect(callback).toHaveBeenCalled(); + }); + }); - actions - .updateDraft(context, { note: { id: 1 }, noteText: 'test', callback }) - .then(() => { - expect(callback).toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); + it('does not stringify empty position', () => { + return actions.updateDraft(context, { ...params, position: {}, callback() {} }).then(() => { + expect(service.update.mock.calls[0][1].position).toBeUndefined(); + }); + }); + + it('stringifies a non-empty position', () => { + const position = { test: true }; + const expectation = JSON.stringify(position); + return actions.updateDraft(context, { ...params, position, callback() {} }).then(() => { + expect(service.update.mock.calls[0][1].position).toBe(expectation); + }); }); }); diff --git a/spec/frontend/notes/components/noteable_note_spec.js b/spec/frontend/notes/components/noteable_note_spec.js index fe78e086403..112983f3ac2 100644 --- a/spec/frontend/notes/components/noteable_note_spec.js +++ b/spec/frontend/notes/components/noteable_note_spec.js @@ -1,5 +1,6 @@ import { mount, createLocalVue } from '@vue/test-utils'; import { escape } from 'lodash'; +import waitForPromises from 'helpers/wait_for_promises'; import NoteActions from '~/notes/components/note_actions.vue'; import NoteBody from '~/notes/components/note_body.vue'; import NoteHeader from '~/notes/components/note_header.vue'; @@ -13,7 +14,7 @@ describe('issue_note', () => { let wrapper; const findMultilineComment = () => wrapper.find('[data-testid="multiline-comment"]'); - beforeEach(() => { + const createWrapper = (props = {}) => { store = createStore(); store.dispatch('setNoteableData', noteableDataMock); store.dispatch('setNotesData', notesDataMock); @@ -23,6 +24,7 @@ describe('issue_note', () => { store, propsData: { note, + ...props, }, localVue, stubs: [ @@ -33,14 +35,18 @@ describe('issue_note', () => { 'multiline-comment-form', ], }); - }); + }; afterEach(() => { wrapper.destroy(); }); describe('mutiline comments', () => { - it('should render if has multiline comment', () => { + beforeEach(() => { + createWrapper(); + }); + + it('should render if has multiline comment', async () => { const position = { line_range: { start: { @@ -69,9 +75,8 @@ describe('issue_note', () => { line, }); - return wrapper.vm.$nextTick().then(() => { - expect(findMultilineComment().text()).toEqual('Comment on lines 1 to 2'); - }); + await wrapper.vm.$nextTick(); + expect(findMultilineComment().text()).toBe('Comment on lines 1 to 2'); }); it('should only render if it has everything it needs', () => { @@ -147,108 +152,151 @@ describe('issue_note', () => { }); }); - it('should render user information', () => { - const { author } = note; - const avatar = wrapper.find(UserAvatarLink); - const avatarProps = avatar.props(); + describe('rendering', () => { + beforeEach(() => { + createWrapper(); + }); - expect(avatarProps.linkHref).toBe(author.path); - expect(avatarProps.imgSrc).toBe(author.avatar_url); - expect(avatarProps.imgAlt).toBe(author.name); - expect(avatarProps.imgSize).toBe(40); - }); + it('should render user information', () => { + const { author } = note; + const avatar = wrapper.findComponent(UserAvatarLink); + const avatarProps = avatar.props(); - it('should render note header content', () => { - const noteHeader = wrapper.find(NoteHeader); - const noteHeaderProps = noteHeader.props(); + expect(avatarProps.linkHref).toBe(author.path); + expect(avatarProps.imgSrc).toBe(author.avatar_url); + expect(avatarProps.imgAlt).toBe(author.name); + expect(avatarProps.imgSize).toBe(40); + }); - expect(noteHeaderProps.author).toEqual(note.author); - expect(noteHeaderProps.createdAt).toEqual(note.created_at); - expect(noteHeaderProps.noteId).toEqual(note.id); - }); + it('should render note header content', () => { + const noteHeader = wrapper.findComponent(NoteHeader); + const noteHeaderProps = noteHeader.props(); - it('should render note actions', () => { - const { author } = note; - const noteActions = wrapper.find(NoteActions); - const noteActionsProps = noteActions.props(); - - expect(noteActionsProps.authorId).toBe(author.id); - expect(noteActionsProps.noteId).toBe(note.id); - expect(noteActionsProps.noteUrl).toBe(note.noteable_note_url); - expect(noteActionsProps.accessLevel).toBe(note.human_access); - expect(noteActionsProps.canEdit).toBe(note.current_user.can_edit); - expect(noteActionsProps.canAwardEmoji).toBe(note.current_user.can_award_emoji); - expect(noteActionsProps.canDelete).toBe(note.current_user.can_edit); - expect(noteActionsProps.canReportAsAbuse).toBe(true); - expect(noteActionsProps.canResolve).toBe(false); - expect(noteActionsProps.reportAbusePath).toBe(note.report_abuse_path); - expect(noteActionsProps.resolvable).toBe(false); - expect(noteActionsProps.isResolved).toBe(false); - expect(noteActionsProps.isResolving).toBe(false); - expect(noteActionsProps.resolvedBy).toEqual({}); - }); + expect(noteHeaderProps.author).toBe(note.author); + expect(noteHeaderProps.createdAt).toBe(note.created_at); + expect(noteHeaderProps.noteId).toBe(note.id); + }); - it('should render issue body', () => { - const noteBody = wrapper.find(NoteBody); - const noteBodyProps = noteBody.props(); + it('should render note actions', () => { + const { author } = note; + const noteActions = wrapper.findComponent(NoteActions); + const noteActionsProps = noteActions.props(); - expect(noteBodyProps.note).toEqual(note); - expect(noteBodyProps.line).toBe(null); - expect(noteBodyProps.canEdit).toBe(note.current_user.can_edit); - expect(noteBodyProps.isEditing).toBe(false); - expect(noteBodyProps.helpPagePath).toBe(''); - }); + expect(noteActionsProps.authorId).toBe(author.id); + expect(noteActionsProps.noteId).toBe(note.id); + expect(noteActionsProps.noteUrl).toBe(note.noteable_note_url); + expect(noteActionsProps.accessLevel).toBe(note.human_access); + expect(noteActionsProps.canEdit).toBe(note.current_user.can_edit); + expect(noteActionsProps.canAwardEmoji).toBe(note.current_user.can_award_emoji); + expect(noteActionsProps.canDelete).toBe(note.current_user.can_edit); + expect(noteActionsProps.canReportAsAbuse).toBe(true); + expect(noteActionsProps.canResolve).toBe(false); + expect(noteActionsProps.reportAbusePath).toBe(note.report_abuse_path); + expect(noteActionsProps.resolvable).toBe(false); + expect(noteActionsProps.isResolved).toBe(false); + expect(noteActionsProps.isResolving).toBe(false); + expect(noteActionsProps.resolvedBy).toEqual({}); + }); - it('prevents note preview xss', (done) => { - const imgSrc = 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7'; - const noteBody = `<img src="${imgSrc}" onload="alert(1)" />`; - const alertSpy = jest.spyOn(window, 'alert'); - store.hotUpdate({ - actions: { - updateNote() {}, - setSelectedCommentPositionHover() {}, - }, + it('should render issue body', () => { + const noteBody = wrapper.findComponent(NoteBody); + const noteBodyProps = noteBody.props(); + + expect(noteBodyProps.note).toBe(note); + expect(noteBodyProps.line).toBe(null); + expect(noteBodyProps.canEdit).toBe(note.current_user.can_edit); + expect(noteBodyProps.isEditing).toBe(false); + expect(noteBodyProps.helpPagePath).toBe(''); }); - const noteBodyComponent = wrapper.find(NoteBody); - noteBodyComponent.vm.$emit('handleFormUpdate', noteBody, null, () => {}); + it('prevents note preview xss', async () => { + const noteBody = + '<img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" onload="alert(1)" />'; + const alertSpy = jest.spyOn(window, 'alert').mockImplementation(() => {}); + const noteBodyComponent = wrapper.findComponent(NoteBody); + + store.hotUpdate({ + actions: { + updateNote() {}, + setSelectedCommentPositionHover() {}, + }, + }); + + noteBodyComponent.vm.$emit('handleFormUpdate', noteBody, null, () => {}); - setImmediate(() => { + await waitForPromises(); expect(alertSpy).not.toHaveBeenCalled(); - expect(wrapper.vm.note.note_html).toEqual(escape(noteBody)); - done(); + expect(wrapper.vm.note.note_html).toBe(escape(noteBody)); }); }); describe('cancel edit', () => { - it('restores content of updated note', (done) => { + beforeEach(() => { + createWrapper(); + }); + + it('restores content of updated note', async () => { const updatedText = 'updated note text'; store.hotUpdate({ actions: { updateNote() {}, }, }); - const noteBody = wrapper.find(NoteBody); + const noteBody = wrapper.findComponent(NoteBody); noteBody.vm.resetAutoSave = () => {}; noteBody.vm.$emit('handleFormUpdate', updatedText, null, () => {}); - wrapper.vm - .$nextTick() - .then(() => { - const noteBodyProps = noteBody.props(); - - expect(noteBodyProps.note.note_html).toBe(updatedText); - noteBody.vm.$emit('cancelForm'); - }) - .then(() => wrapper.vm.$nextTick()) - .then(() => { - const noteBodyProps = noteBody.props(); - - expect(noteBodyProps.note.note_html).toBe(note.note_html); - }) - .then(done) - .catch(done.fail); + await wrapper.vm.$nextTick(); + let noteBodyProps = noteBody.props(); + + expect(noteBodyProps.note.note_html).toBe(updatedText); + + noteBody.vm.$emit('cancelForm'); + await wrapper.vm.$nextTick(); + + noteBodyProps = noteBody.props(); + + expect(noteBodyProps.note.note_html).toBe(note.note_html); + }); + }); + + describe('formUpdateHandler', () => { + const updateNote = jest.fn(); + const params = ['', null, jest.fn(), '']; + + const updateActions = () => { + store.hotUpdate({ + actions: { + updateNote, + setSelectedCommentPositionHover() {}, + }, + }); + }; + + afterEach(() => updateNote.mockReset()); + + it('responds to handleFormUpdate', () => { + createWrapper(); + updateActions(); + wrapper.findComponent(NoteBody).vm.$emit('handleFormUpdate', ...params); + expect(wrapper.emitted('handleUpdateNote')).toBeTruthy(); + }); + + it('does not stringify empty position', () => { + createWrapper(); + updateActions(); + wrapper.findComponent(NoteBody).vm.$emit('handleFormUpdate', ...params); + expect(updateNote.mock.calls[0][1].note.note.position).toBeUndefined(); + }); + + it('stringifies populated position', () => { + const position = { test: true }; + const expectation = JSON.stringify(position); + createWrapper({ note: { ...note, position } }); + updateActions(); + wrapper.findComponent(NoteBody).vm.$emit('handleFormUpdate', ...params); + expect(updateNote.mock.calls[0][1].note.note.position).toBe(expectation); }); }); }); diff --git a/spec/initializers/rack_multipart_patch_spec.rb b/spec/initializers/rack_multipart_patch_spec.rb index e51d7bbeb2d..862fdc7901b 100644 --- a/spec/initializers/rack_multipart_patch_spec.rb +++ b/spec/initializers/rack_multipart_patch_spec.rb @@ -42,20 +42,38 @@ EOF end context 'with Content-Length over the limit' do - it 'extracts multipart message' do - env = Rack::MockRequest.env_for("/", multipart_fixture(:text, 500_000_001)) - - expect(described_class).to receive(:log_large_multipart?).and_return(true) - expect(described_class).to receive(:log_multipart_warning).and_call_original - expect(described_class).to receive(:log_warn).with({ - message: 'Large multipart body detected', - path: '/', - content_length: anything, - correlation_id: anything - }) - params = described_class.parse_multipart(env) + shared_examples 'logs multipart message' do + it 'extracts multipart message' do + env = Rack::MockRequest.env_for("/", multipart_fixture(:text, length)) - expect(params.keys).to include(*%w(reply fileupload)) + expect(described_class).to receive(:log_large_multipart?).and_return(true) + expect(described_class).to receive(:log_multipart_warning).and_call_original + expect(described_class).to receive(:log_warn).with({ + message: 'Large multipart body detected', + path: '/', + content_length: anything, + correlation_id: anything + }) + params = described_class.parse_multipart(env) + + expect(params.keys).to include(*%w(reply fileupload)) + end + end + + context 'from environment' do + let(:length) { 1001 } + + before do + stub_env('RACK_MULTIPART_LOGGING_BYTES', 1000) + end + + it_behaves_like 'logs multipart message' + end + + context 'default limit' do + let(:length) { 100_000_001 } + + it_behaves_like 'logs multipart message' end end end diff --git a/spec/lib/banzai/filter/custom_emoji_filter_spec.rb b/spec/lib/banzai/filter/custom_emoji_filter_spec.rb index 82e399fff9b..5e76e8164dd 100644 --- a/spec/lib/banzai/filter/custom_emoji_filter_spec.rb +++ b/spec/lib/banzai/filter/custom_emoji_filter_spec.rb @@ -10,6 +10,10 @@ RSpec.describe Banzai::Filter::CustomEmojiFilter do let_it_be(:custom_emoji) { create(:custom_emoji, name: 'tanuki', group: group) } let_it_be(:custom_emoji2) { create(:custom_emoji, name: 'happy_tanuki', group: group, file: 'https://foo.bar/happy.png') } + it_behaves_like 'emoji filter' do + let(:emoji_name) { ':tanuki:' } + end + it 'replaces supported name custom emoji' do doc = filter('<p>:tanuki:</p>', project: project) @@ -17,25 +21,12 @@ RSpec.describe Banzai::Filter::CustomEmojiFilter do expect(doc.css('gl-emoji img').size).to eq 1 end - it 'ignores non existent custom emoji' do - exp = '<p>:foo:</p>' - doc = filter(exp) - - expect(doc.to_html).to eq(exp) - end - it 'correctly uses the custom emoji URL' do doc = filter('<p>:tanuki:</p>') expect(doc.css('img').first.attributes['src'].value).to eq(custom_emoji.file) end - it 'matches with adjacent text' do - doc = filter('tanuki (:tanuki:)') - - expect(doc.css('img').size).to eq 1 - end - it 'matches multiple same custom emoji' do doc = filter(':tanuki: :tanuki:') @@ -54,18 +45,6 @@ RSpec.describe Banzai::Filter::CustomEmojiFilter do expect(doc.css('img').size).to be 0 end - it 'keeps whitespace intact' do - doc = filter('This deserves a :tanuki:, big time.') - - expect(doc.to_html).to match(/^This deserves a <gl-emoji.+>, big time\.\z/) - end - - it 'does not match emoji in a string' do - doc = filter("'2a00:tanuki:100::1'") - - expect(doc.css('gl-emoji').size).to eq 0 - end - it 'does not do N+1 query' do create(:custom_emoji, name: 'party-parrot', group: group) @@ -77,10 +56,4 @@ RSpec.describe Banzai::Filter::CustomEmojiFilter do filter('<p>:tanuki: :party-parrot:</p>') end.not_to exceed_all_query_limit(control_count.count) end - - context 'ancestor tags' do - let(:emoji_name) { ':tanuki:' } - - it_behaves_like 'ignored ancestor tags' - end end diff --git a/spec/lib/banzai/filter/emoji_filter_spec.rb b/spec/lib/banzai/filter/emoji_filter_spec.rb index d76a36b24e1..cb0b470eaa1 100644 --- a/spec/lib/banzai/filter/emoji_filter_spec.rb +++ b/spec/lib/banzai/filter/emoji_filter_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' RSpec.describe Banzai::Filter::EmojiFilter do include FilterSpecHelper + it_behaves_like 'emoji filter' do + let(:emoji_name) { ':+1:' } + end + it 'replaces supported name emoji' do doc = filter('<p>:heart:</p>') expect(doc.css('gl-emoji').first.text).to eq '❤' @@ -15,12 +19,6 @@ RSpec.describe Banzai::Filter::EmojiFilter do expect(doc.css('gl-emoji').first.text).to eq '❤' end - it 'ignores unsupported emoji' do - exp = act = '<p>:foo:</p>' - doc = filter(act) - expect(doc.to_html).to match Regexp.escape(exp) - end - it 'ignores unicode versions of trademark, copyright, and registered trademark' do exp = act = '<p>™ © ®</p>' doc = filter(act) @@ -65,11 +63,6 @@ RSpec.describe Banzai::Filter::EmojiFilter do expect(doc.css('gl-emoji').size).to eq 1 end - it 'matches with adjacent text' do - doc = filter('+1 (:+1:)') - expect(doc.css('gl-emoji').size).to eq 1 - end - it 'unicode matches with adjacent text' do doc = filter('+1 (👍)') expect(doc.css('gl-emoji').size).to eq 1 @@ -90,12 +83,6 @@ RSpec.describe Banzai::Filter::EmojiFilter do expect(doc.css('gl-emoji').size).to eq 6 end - it 'does not match emoji in a string' do - doc = filter("'2a00:a4c0:100::1'") - - expect(doc.css('gl-emoji').size).to eq 0 - end - it 'has a data-name attribute' do doc = filter(':-1:') expect(doc.css('gl-emoji').first.attr('data-name')).to eq 'thumbsdown' @@ -106,21 +93,9 @@ RSpec.describe Banzai::Filter::EmojiFilter do expect(doc.css('gl-emoji').first.attr('data-unicode-version')).to eq '6.0' end - it 'keeps whitespace intact' do - doc = filter('This deserves a :+1:, big time.') - - expect(doc.to_html).to match(/^This deserves a <gl-emoji.+>, big time\.\z/) - end - it 'unicode keeps whitespace intact' do doc = filter('This deserves a 🎱, big time.') expect(doc.to_html).to match(/^This deserves a <gl-emoji.+>, big time\.\z/) end - - context 'ancestor tags' do - let(:emoji_name) { ':see_no_evil:' } - - it_behaves_like 'ignored ancestor tags' - end end diff --git a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb index 3e8563376ce..e25e4af2e86 100644 --- a/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migrations/background_migration_helpers_spec.rb @@ -21,7 +21,7 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do context 'with enough rows to bulk queue jobs more than once' do before do - stub_const('Gitlab::Database::Migrations::BackgroundMigrationHelpers::BACKGROUND_MIGRATION_JOB_BUFFER_SIZE', 1) + stub_const('Gitlab::Database::Migrations::BackgroundMigrationHelpers::JOB_BUFFER_SIZE', 1) end it 'queues jobs correctly' do @@ -262,6 +262,120 @@ RSpec.describe Gitlab::Database::Migrations::BackgroundMigrationHelpers do end end + describe '#queue_batched_background_migration' do + it 'creates the database record for the migration' do + expect do + model.queue_batched_background_migration( + 'MyJobClass', + :projects, + :id, + job_interval: 5.minutes, + batch_min_value: 5, + batch_max_value: 1000, + batch_class_name: 'MyBatchClass', + batch_size: 100, + sub_batch_size: 10) + end.to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.by(1) + + expect(Gitlab::Database::BackgroundMigration::BatchedMigration.last).to have_attributes( + job_class_name: 'MyJobClass', + table_name: 'projects', + column_name: 'id', + interval: 300, + min_value: 5, + max_value: 1000, + batch_class_name: 'MyBatchClass', + batch_size: 100, + sub_batch_size: 10, + job_arguments: %w[], + status: 'active') + end + + context 'when the job interval is lower than the minimum' do + let(:minimum_delay) { described_class::BATCH_MIN_DELAY } + + it 'sets the job interval to the minimum value' do + expect do + model.queue_batched_background_migration('MyJobClass', :events, :id, job_interval: minimum_delay - 1.minute) + end.to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.by(1) + + created_migration = Gitlab::Database::BackgroundMigration::BatchedMigration.last + + expect(created_migration.interval).to eq(minimum_delay) + end + end + + context 'when additional arguments are passed to the method' do + it 'saves the arguments on the database record' do + expect do + model.queue_batched_background_migration( + 'MyJobClass', + :projects, + :id, + 'my', + 'arguments', + job_interval: 5.minutes, + batch_max_value: 1000) + end.to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.by(1) + + expect(Gitlab::Database::BackgroundMigration::BatchedMigration.last).to have_attributes( + job_class_name: 'MyJobClass', + table_name: 'projects', + column_name: 'id', + interval: 300, + min_value: 1, + max_value: 1000, + job_arguments: %w[my arguments]) + end + end + + context 'when the max_value is not given' do + context 'when records exist in the database' do + let!(:event1) { create(:event) } + let!(:event2) { create(:event) } + let!(:event3) { create(:event) } + + it 'creates the record with the current max value' do + expect do + model.queue_batched_background_migration('MyJobClass', :events, :id, job_interval: 5.minutes) + end.to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.by(1) + + created_migration = Gitlab::Database::BackgroundMigration::BatchedMigration.last + + expect(created_migration.max_value).to eq(event3.id) + end + + it 'creates the record with an active status' do + expect do + model.queue_batched_background_migration('MyJobClass', :events, :id, job_interval: 5.minutes) + end.to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.by(1) + + expect(Gitlab::Database::BackgroundMigration::BatchedMigration.last).to be_active + end + end + + context 'when the database is empty' do + it 'sets the max value to the min value' do + expect do + model.queue_batched_background_migration('MyJobClass', :events, :id, job_interval: 5.minutes) + end.to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.by(1) + + created_migration = Gitlab::Database::BackgroundMigration::BatchedMigration.last + + expect(created_migration.max_value).to eq(created_migration.min_value) + end + + it 'creates the record with a finished status' do + expect do + model.queue_batched_background_migration('MyJobClass', :projects, :id, job_interval: 5.minutes) + end.to change { Gitlab::Database::BackgroundMigration::BatchedMigration.count }.by(1) + + expect(Gitlab::Database::BackgroundMigration::BatchedMigration.last).to be_finished + end + end + end + end + describe '#migrate_async' do it 'calls BackgroundMigrationWorker.perform_async' do expect(BackgroundMigrationWorker).to receive(:perform_async).with("Class", "hello", "world") diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index ebb10e991ad..9cf998a0639 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -62,6 +62,21 @@ RSpec.describe Packages::PackageFile, type: :model do end end + describe '.for_rubygem_with_file_name' do + let_it_be(:project) { create(:project) } + let_it_be(:non_ruby_package) { create(:nuget_package, project: project, package_type: :nuget) } + let_it_be(:ruby_package) { create(:rubygems_package, project: project, package_type: :rubygems) } + let_it_be(:file_name) { 'other.gem' } + + let_it_be(:non_ruby_file) { create(:package_file, :nuget, package: non_ruby_package, file_name: file_name) } + let_it_be(:gem_file1) { create(:package_file, :gem, package: ruby_package) } + let_it_be(:gem_file2) { create(:package_file, :gem, package: ruby_package, file_name: file_name) } + + it 'returns the matching gem file only for ruby packages' do + expect(described_class.for_rubygem_with_file_name(project, file_name)).to contain_exactly(gem_file2) + end + end + describe '#update_file_store callback' do let_it_be(:package_file) { build(:package_file, :nuget, size: nil) } diff --git a/spec/requests/api/rubygem_packages_spec.rb b/spec/requests/api/rubygem_packages_spec.rb index 75aa7954ca3..d6ad8186063 100644 --- a/spec/requests/api/rubygem_packages_spec.rb +++ b/spec/requests/api/rubygem_packages_spec.rb @@ -108,11 +108,68 @@ RSpec.describe API::RubygemPackages do end describe 'GET /api/v4/projects/:project_id/packages/rubygems/gems/:file_name' do - let(:url) { api("/projects/#{project.id}/packages/rubygems/gems/my_gem-1.0.0.gem") } + let_it_be(:package_name) { 'package' } + let_it_be(:version) { '0.0.1' } + let_it_be(:package) { create(:rubygems_package, project: project, name: package_name, version: version) } + let_it_be(:file_name) { "#{package_name}-#{version}.gem" } + + let(:url) { api("/projects/#{project.id}/packages/rubygems/gems/#{file_name}") } subject { get(url, headers: headers) } - it_behaves_like 'an unimplemented route' + context 'with valid project' do + where(:visibility, :user_role, :member, :token_type, :valid_token, :shared_examples_name, :expected_status) do + :public | :developer | true | :personal_access_token | true | 'Rubygems gem download' | :success + :public | :guest | true | :personal_access_token | true | 'Rubygems gem download' | :success + :public | :developer | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | false | :personal_access_token | true | 'Rubygems gem download' | :success + :public | :guest | false | :personal_access_token | true | 'Rubygems gem download' | :success + :public | :developer | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :anonymous | false | :personal_access_token | true | 'Rubygems gem download' | :success + :private | :developer | true | :personal_access_token | true | 'Rubygems gem download' | :success + :private | :guest | true | :personal_access_token | true | 'rejects rubygems packages access' | :forbidden + :private | :developer | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :private | :guest | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :private | :developer | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :anonymous | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :public | :developer | true | :job_token | true | 'Rubygems gem download' | :success + :public | :guest | true | :job_token | true | 'Rubygems gem download' | :success + :public | :developer | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | false | :job_token | true | 'Rubygems gem download' | :success + :public | :guest | false | :job_token | true | 'Rubygems gem download' | :success + :public | :developer | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | true | :job_token | true | 'Rubygems gem download' | :success + :private | :guest | true | :job_token | true | 'rejects rubygems packages access' | :forbidden + :private | :developer | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | false | :job_token | true | 'rejects rubygems packages access' | :not_found + :private | :guest | false | :job_token | true | 'rejects rubygems packages access' | :not_found + :private | :developer | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | true | :deploy_token | true | 'Rubygems gem download' | :success + :public | :developer | true | :deploy_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | true | :deploy_token | true | 'Rubygems gem download' | :success + :private | :developer | true | :deploy_token | false | 'rejects rubygems packages access' | :unauthorized + end + + with_them do + let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } + let(:headers) { user_role == :anonymous ? {} : { 'HTTP_AUTHORIZATION' => token } } + + before do + project.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value(visibility.to_s)) + end + + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] + end + end end describe 'POST /api/v4/projects/:project_id/packages/rubygems/api/v1/gems/authorize' do @@ -171,7 +228,7 @@ RSpec.describe API::RubygemPackages do let(:headers) { user_headers.merge(workhorse_headers) } before do - project.update!(visibility: visibility.to_s) + project.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value(visibility.to_s)) end it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] @@ -249,7 +306,7 @@ RSpec.describe API::RubygemPackages do let(:headers) { user_headers.merge(workhorse_headers) } before do - project.update!(visibility: visibility.to_s) + project.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value(visibility.to_s)) end it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] diff --git a/spec/support/shared_examples/banzai/filters/emoji_shared_examples.rb b/spec/support/shared_examples/banzai/filters/emoji_shared_examples.rb new file mode 100644 index 00000000000..da305f5ccaa --- /dev/null +++ b/spec/support/shared_examples/banzai/filters/emoji_shared_examples.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'emoji filter' do + it 'keeps whitespace intact' do + doc = filter("This deserves a #{emoji_name}, big time.") + + expect(doc.to_html).to match(/^This deserves a <gl-emoji.+>, big time\.\z/) + end + + it 'does not match emoji in a string' do + doc = filter("'2a00:a4c0#{emoji_name}:1'") + + expect(doc.css('gl-emoji')).to be_empty + end + + it 'ignores non existent/unsupported emoji' do + exp = '<p>:foo:</p>' + doc = filter(exp) + + expect(doc.to_html).to eq(exp) + end + + it 'matches with adjacent text' do + doc = filter("#{emoji_name.delete(':')} (#{emoji_name})") + + expect(doc.css('gl-emoji').size).to eq 1 + end + + it 'does not match emoji in a pre tag' do + doc = filter("<p><pre>#{emoji_name}</pre></p>") + + expect(doc.css('img')).to be_empty + end + + it 'does not match emoji in code tag' do + doc = filter("<p><code>#{emoji_name} wow</code></p>") + + expect(doc.css('img')).to be_empty + end + + it 'does not match emoji in tt tag' do + doc = filter("<p><tt>#{emoji_name} yes!</tt></p>") + + expect(doc.css('img')).to be_empty + end +end diff --git a/spec/support/shared_examples/banzai/filters/ignored_tags_emoji_shared_examples.rb b/spec/support/shared_examples/banzai/filters/ignored_tags_emoji_shared_examples.rb deleted file mode 100644 index 89d7cdc4bba..00000000000 --- a/spec/support/shared_examples/banzai/filters/ignored_tags_emoji_shared_examples.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -RSpec.shared_examples 'ignored ancestor tags' do - it 'does not match emoji in a pre tag' do - doc = filter("<p><pre>#{emoji_name}</pre></p>") - - expect(doc.css('img').size).to eq 0 - end - - it 'does not match emoji in code tag' do - doc = filter("<p><code>#{emoji_name} wow</code></p>") - - expect(doc.css('img').size).to eq 0 - end - - it 'does not match emoji in tt tag' do - doc = filter("<p><tt>#{emoji_name} yes!</tt></p>") - - expect(doc.css('img').size).to eq 0 - end -end diff --git a/spec/support/shared_examples/graphql/mutations/boards_create_shared_examples.rb b/spec/support/shared_examples/graphql/mutations/boards_create_shared_examples.rb index 2b93d174653..2e3a3ce6b41 100644 --- a/spec/support/shared_examples/graphql/mutations/boards_create_shared_examples.rb +++ b/spec/support/shared_examples/graphql/mutations/boards_create_shared_examples.rb @@ -66,9 +66,7 @@ RSpec.shared_examples 'boards create mutation' do context 'when the Boards::CreateService returns an error response' do before do - allow_next_instance_of(Boards::CreateService) do |service| - allow(service).to receive(:execute).and_return(ServiceResponse.error(message: 'There was an error.')) - end + params[:name] = '' end it 'does not create a board' do @@ -80,7 +78,7 @@ RSpec.shared_examples 'boards create mutation' do expect(mutation_response).to have_key('board') expect(mutation_response['board']).to be_nil - expect(mutation_response['errors'].first).to eq('There was an error.') + expect(mutation_response['errors'].first).to eq('There was an error when creating a board.') end end end diff --git a/spec/support/shared_examples/requests/api/rubygems_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/rubygems_packages_shared_examples.rb index cd4cb99d8a5..15fb6611b90 100644 --- a/spec/support/shared_examples/requests/api/rubygems_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/rubygems_packages_shared_examples.rb @@ -175,3 +175,20 @@ RSpec.shared_examples 'dependency endpoint success' do |user_type, status, add_m end end end + +RSpec.shared_examples 'Rubygems gem download' do |user_type, status, add_member = true| + context "for user type #{user_type}" do + before do + project.send("add_#{user_type}", user) if add_member && user_type != :anonymous + end + + it 'returns the gem', :aggregate_failures do + subject + + expect(response.media_type).to eq('application/octet-stream') + expect(response).to have_gitlab_http_status(status) + end + + it_behaves_like 'a package tracking event', described_class.name, 'pull_package' + end +end |
