Add latest changes from gitlab-org/gitlab@master

Gemfile

@@ -239,7 +239,7 @@ gem 'ruby-progressbar', '~> 1.10'
 gem 'settingslogic', '~> 2.0.9'
 
 # Linear-time regex library for untrusted regular expressions
-gem 're2', '~> 1.5.0'
+gem 're2', '~> 1.6.0'
 
 # Misc
 

Gemfile.checksum

@@ -455,7 +455,7 @@
 {"name":"rbtree","version":"0.4.4","platform":"ruby","checksum":"c1277a502a96fe8fd8656cb619db1ac87145df809ea4db35f7242b50bb161d5c"},
 {"name":"rchardet","version":"1.8.0","platform":"ruby","checksum":"693acd5253d5ade81a51940697955f6dd4bb2f0d245bda76a8e23deec70a52c7"},
 {"name":"rdoc","version":"6.3.2","platform":"ruby","checksum":"def4a720235c27d56c176ae73555e647eb04ea58a8bbaa927f8f9f79de7805a6"},
-{"name":"re2","version":"1.5.0","platform":"ruby","checksum":"35fe8b408de9f1ef609b1e54e01ea1e55413ca3e9daf1e4b20756d9a02f630cc"},
+{"name":"re2","version":"1.6.0","platform":"ruby","checksum":"2e37f27971f6a76223eac688c04f3e48aea374f34b302ec22d75b4635cd64bc1"},
 {"name":"recaptcha","version":"4.13.1","platform":"ruby","checksum":"dc6c2cb78afa87034358b7ba1c6f7175972b5709fdf7500e2550623e119e3788"},
 {"name":"recursive-open-struct","version":"1.1.3","platform":"ruby","checksum":"a3538a72552fcebcd0ada657bdff313641a4a5fbc482c08cfb9a65acb1c9de5a"},
 {"name":"redcarpet","version":"3.5.1","platform":"ruby","checksum":"717f64cb6ec11c8d9ec9b521ed26ca2eeda68b4fe1fc3388a641176dbd47732f"},

Gemfile.lock

@@ -1139,7 +1139,7 @@ GEM
     rbtree (0.4.4)
     rchardet (1.8.0)
     rdoc (6.3.2)
-    re2 (1.5.0)
+    re2 (1.6.0)
     recaptcha (4.13.1)
       json
     recursive-open-struct (1.1.3)
@@ -1753,7 +1753,7 @@ DEPENDENCIES
   rainbow (~> 3.0)
   rbtrace (~> 0.4)
   rdoc (~> 6.3.2)
-  re2 (~> 1.5.0)
+  re2 (~> 1.6.0)
   recaptcha (~> 4.11)
   redis (~> 4.7.0)
   redis-actionpack (~> 5.3.0)

app/assets/javascripts/members/components/action_buttons/access_request_action_buttons.vue

@@ -7,15 +7,12 @@ import RemoveMemberButton from './remove_member_button.vue';
 export default {
   name: 'AccessRequestActionButtons',
   components: { ActionButtonGroup, RemoveMemberButton, ApproveAccessRequestButton },
+  inheritAttrs: false,
   props: {
     member: {
       type: Object,
       required: true,
     },
-    permissions: {
-      type: Object,
-      required: true,
-    },
     isCurrentUser: {
       type: Boolean,
       required: true,
@@ -43,10 +40,10 @@ export default {
 
 <template>
   <action-button-group>
-    <div v-if="permissions.canUpdate" class="gl-px-1">
+    <div class="gl-px-1">
       <approve-access-request-button :member-id="member.id" />
     </div>
-    <div v-if="permissions.canRemove" class="gl-px-1">
+    <div class="gl-px-1">
       <remove-member-button
         :member-id="member.id"
         :message="message"

app/assets/javascripts/members/components/members_tabs.vue

@@ -27,13 +27,13 @@ export const TABS = [
   {
     namespace: MEMBER_TYPES.invite,
     title: __('Invited'),
-    canManageMembersPermissionsRequired: true,
+    requiredPermissions: ['canManageMembers'],
     queryParamValue: TAB_QUERY_PARAM_VALUES.invite,
   },
   {
     namespace: MEMBER_TYPES.accessRequest,
     title: __('Access requests'),
-    canManageMembersPermissionsRequired: true,
+    requiredPermissions: ['canManageAccessRequests'],
     queryParamValue: TAB_QUERY_PARAM_VALUES.accessRequest,
   },
   ...EE_TABS,
@@ -44,7 +44,7 @@ export default {
   ACTIVE_TAB_QUERY_PARAM_NAME,
   TABS,
   components: { MembersApp, GlTabs, GlTab, GlBadge, GlButton },
-  inject: ['canManageMembers', 'canExportMembers', 'exportCsvPath'],
+  inject: ['canManageMembers', 'canManageAccessRequests', 'canExportMembers', 'exportCsvPath'],
   data() {
     return {
       selectedTabIndex: 0,
@@ -96,15 +96,13 @@ export default {
         return true;
       }
 
-      const { canManageMembersPermissionsRequired = false } = tab;
+      const { requiredPermissions = [] } = tab;
       const tabCanBeShown =
         this.getTabCount(tab) > 0 || this.activeTabIndexCalculatedFromUrlParams === index;
 
-      if (canManageMembersPermissionsRequired) {
+      return (
-        return this.canManageMembers && tabCanBeShown;
+        tabCanBeShown && requiredPermissions.every((requiredPermission) => this[requiredPermission])
-      }
+      );
-
-      return tabCanBeShown;
     },
   },
 };

app/assets/javascripts/members/index.js

@@ -17,6 +17,7 @@ export const initMembersApp = (el, options) => {
   const {
     sourceId,
     canManageMembers,
+    canManageAccessRequests,
     canExportMembers,
     canFilterByEnterprise,
     exportCsvPath,
@@ -61,6 +62,7 @@ export const initMembersApp = (el, options) => {
       currentUserId: gon.current_user_id || null,
       sourceId,
       canManageMembers,
+      canManageAccessRequests,
       canFilterByEnterprise,
       canExportMembers,
       exportCsvPath,

app/assets/javascripts/vue_shared/components/markdown_drawer/makrdown_drawer.stories.js

@@ -1,54 +0,0 @@
-import { GlButton } from '@gitlab/ui';
-import { MOCK_HTML } from '../../../../../../spec/frontend/vue_shared/components/markdown_drawer/mock_data';
-import MarkdownDrawer from './markdown_drawer.vue';
-
-export default {
-  component: MarkdownDrawer,
-  title: 'vue_shared/markdown_drawer',
-  parameters: {
-    mirage: {
-      timing: 1000,
-      handlers: {
-        get: {
-          '/help/user/search/global_search/advanced_search_syntax.json': [
-            200,
-            {},
-            { html: MOCK_HTML },
-          ],
-        },
-      },
-    },
-  },
-};
-
-const createStory = ({ ...options }) => (_, { argTypes }) => ({
-  components: { MarkdownDrawer, GlButton },
-  props: Object.keys(argTypes),
-  data() {
-    return {
-      render: false,
-    };
-  },
-  methods: {
-    toggleDrawer() {
-      this.$refs.drawer.toggleDrawer();
-    },
-  },
-  mounted() {
-    window.requestAnimationFrame(() => {
-      this.render = true;
-    });
-  },
-  template: `
-       <div v-if="render">
-        <gl-button @click="toggleDrawer">Open Drawer</gl-button>
-        <markdown-drawer
-          :documentPath="'user/search/global_search/advanced_search_syntax.json'"
-          ref="drawer"
-        />
-        </div>
-      `,
-  ...options,
-});
-
-export const Default = createStory({});

app/assets/javascripts/vue_shared/components/markdown_drawer/markdown_drawer.vue

@@ -1,117 +0,0 @@
-<script>
-import { GlSafeHtmlDirective as SafeHtml, GlDrawer, GlAlert, GlSkeletonLoader } from '@gitlab/ui';
-import $ from 'jquery';
-import '~/behaviors/markdown/render_gfm';
-import { s__ } from '~/locale';
-import { contentTop } from '~/lib/utils/common_utils';
-import { getRenderedMarkdown } from './utils/fetch';
-
-export const cache = {};
-
-export default {
-  name: 'MarkdownDrawer',
-  components: {
-    GlDrawer,
-    GlAlert,
-    GlSkeletonLoader,
-  },
-  directives: {
-    SafeHtml,
-  },
-  i18n: {
-    alert: s__('MardownDrawer|Could not fetch help contents.'),
-  },
-  props: {
-    documentPath: {
-      type: String,
-      required: true,
-    },
-  },
-  data() {
-    return {
-      loading: false,
-      hasFetchError: false,
-      title: '',
-      body: null,
-      open: false,
-    };
-  },
-  computed: {
-    drawerOffsetTop() {
-      return `${contentTop()}px`;
-    },
-  },
-  watch: {
-    documentPath: {
-      immediate: true,
-      handler: 'fetchMarkdown',
-    },
-    open(open) {
-      if (open && this.body) {
-        this.renderGLFM();
-      }
-    },
-  },
-  methods: {
-    async fetchMarkdown() {
-      const cached = cache[this.documentPath];
-      this.hasFetchError = false;
-      this.title = '';
-      if (cached) {
-        this.title = cached.title;
-        this.body = cached.body;
-        if (this.open) {
-          this.renderGLFM();
-        }
-      } else {
-        this.loading = true;
-        const { body, title, hasFetchError } = await getRenderedMarkdown(this.documentPath);
-        this.title = title;
-        this.body = body;
-        this.loading = false;
-        this.hasFetchError = hasFetchError;
-        if (this.open) {
-          this.renderGLFM();
-        }
-        cache[this.documentPath] = { title, body };
-      }
-    },
-    renderGLFM() {
-      this.$nextTick(() => {
-        $(this.$refs['content-element']).renderGFM();
-      });
-    },
-    closeDrawer() {
-      this.open = false;
-    },
-    toggleDrawer() {
-      this.open = !this.open;
-    },
-    openDrawer() {
-      this.open = true;
-    },
-  },
-  safeHtmlConfig: {
-    ADD_TAGS: ['copy-code'],
-  },
-};
-</script>
-<template>
-  <gl-drawer :header-height="drawerOffsetTop" :open="open" header-sticky @close="closeDrawer">
-    <template #title>
-      <h4 data-testid="title-element" class="gl-m-0">{{ title }}</h4>
-    </template>
-    <template #default>
-      <div v-if="hasFetchError">
-        <gl-alert :dismissible="false" variant="danger">{{ $options.i18n.alert }}</gl-alert>
-      </div>
-      <gl-skeleton-loader v-else-if="loading" />
-      <div
-        v-else
-        ref="content-element"
-        v-safe-html:[$options.safeHtmlConfig]="body"
-        class="md"
-      ></div>
-    </template>
-  </gl-drawer>
-</template>

app/assets/javascripts/vue_shared/components/markdown_drawer/utils/fetch.js

@@ -1,32 +0,0 @@
-import * as Sentry from '@sentry/browser';
-import { helpPagePath } from '~/helpers/help_page_helper';
-import axios from '~/lib/utils/axios_utils';
-
-export const splitDocument = (htmlString) => {
-  const htmlDocument = new DOMParser().parseFromString(htmlString, 'text/html');
-  const title = htmlDocument.querySelector('h1')?.innerText;
-  htmlDocument.querySelector('h1')?.remove();
-  return {
-    title,
-    body: htmlDocument.querySelector('body').innerHTML.toString(),
-  };
-};
-
-export const getRenderedMarkdown = (documentPath) => {
-  return axios
-    .get(helpPagePath(documentPath))
-    .then(({ data }) => {
-      const { body, title } = splitDocument(data.html);
-      return {
-        body,
-        title,
-        hasFetchError: false,
-      };
-    })
-    .catch((e) => {
-      Sentry.captureException(e);
-      return {
-        hasFetchError: true,
-      };
-    });
-};

app/controllers/concerns/web_hooks/hook_actions.rb

@@ -53,6 +53,8 @@ module WebHooks
 
       ps = params.require(:hook).permit(*permitted).to_h
 
+      ps.delete(:token) if action_name == 'update' && ps[:token] == WebHook::SECRET_MASK
+
       ps[:url_variables] = ps[:url_variables].to_h { [_1[:key], _1[:value].presence] } if ps.key?(:url_variables)
 
       if action_name == 'update' && ps.key?(:url_variables)

app/finders/access_requests_finder.rb

@@ -24,6 +24,6 @@ class AccessRequestsFinder
   private
 
   def can_see_access_requests?(current_user)
-    source && Ability.allowed?(current_user, :"admin_#{source.class.to_s.underscore}", source)
+    source && Ability.allowed?(current_user, :admin_member_access_request, source)
   end
 end

app/helpers/groups/group_members_helper.rb

@@ -12,7 +12,8 @@ module Groups::GroupMembersHelper
       invite: group_members_list_data(group, invited.nil? ? [] : invited, { param_name: :invited_members_page, params: { page: nil } }),
       access_request: group_members_list_data(group, access_requests.nil? ? [] : access_requests),
       source_id: group.id,
-      can_manage_members: can?(current_user, :admin_group_member, group)
+      can_manage_members: can?(current_user, :admin_group_member, group),
+      can_manage_access_requests: can?(current_user, :admin_member_access_request, group)
     }
   end
 

app/helpers/projects/project_members_helper.rb

@@ -8,7 +8,8 @@ module Projects::ProjectMembersHelper
       invite: project_members_list_data(project, invited.nil? ? [] : invited),
       access_request: project_members_list_data(project, access_requests.nil? ? [] : access_requests),
       source_id: project.id,
-      can_manage_members: Ability.allowed?(current_user, :admin_project_member, project)
+      can_manage_members: Ability.allowed?(current_user, :admin_project_member, project),
+      can_manage_access_requests: Ability.allowed?(current_user, :admin_member_access_request, project)
     })
   end
 

app/models/hooks/web_hook.rb

@@ -11,6 +11,7 @@ class WebHook < ApplicationRecord
   INITIAL_BACKOFF = 1.minute
   MAX_BACKOFF = 1.day
   BACKOFF_GROWTH_FACTOR = 2.0
+  SECRET_MASK = '************'
 
   attr_encrypted :token,
                  mode: :per_attribute_iv,
@@ -210,6 +211,10 @@ class WebHook < ApplicationRecord
     # Overridden in child classes.
   end
 
+  def masked_token
+    token.present? ? SECRET_MASK : nil
+  end
+
   private
 
   def next_failure_count

app/policies/concerns/member_policy_helpers.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module MemberPolicyHelpers
+  extend ActiveSupport::Concern
+
+  private
+
+  def record_is_access_request_of_self?
+    record_is_access_request? && record_belongs_to_self?
+  end
+
+  def record_is_access_request?
+    @subject.request? # rubocop:disable Gitlab/ModuleWithInstanceVariables
+  end
+
+  def record_belongs_to_self?
+    @user && @subject.user == @user # rubocop:disable Gitlab/ModuleWithInstanceVariables
+  end
+end

app/policies/group_member_policy.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class GroupMemberPolicy < BasePolicy
+  include MemberPolicyHelpers
+
   delegate :group
 
   with_scope :subject
@@ -9,7 +11,11 @@ class GroupMemberPolicy < BasePolicy
 
   desc "Membership is users' own"
   with_score 0
-  condition(:is_target_user) { @user && @subject.user_id == @user.id }
+  condition(:target_is_self) { record_belongs_to_self? }
+
+  desc "Membership is users' own access request"
+  with_score 0
+  condition(:access_request_of_self) { record_is_access_request_of_self? }
 
   rule { anonymous }.policy do
     prevent :update_group_member
@@ -28,9 +34,13 @@ class GroupMemberPolicy < BasePolicy
 
   rule { project_bot & can?(:admin_group_member) }.enable :destroy_project_bot_member
 
-  rule { is_target_user }.policy do
+  rule { target_is_self }.policy do
     enable :destroy_group_member
   end
+
+  rule { access_request_of_self }.policy do
+    enable :withdraw_member_access_request
+  end
 end
 
 GroupMemberPolicy.prepend_mod_with('GroupMemberPolicy')

app/policies/group_policy.rb

@@ -283,6 +283,11 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
     prevent :create_resource_access_tokens
   end
 
+  rule { can?(:admin_group_member) }.policy do
+    # ability to read, approve or reject member access requests of other users
+    enable :admin_member_access_request
+  end
+
   rule { support_bot & has_project_with_service_desk_enabled }.policy do
     enable :read_label
   end

app/policies/project_member_policy.rb

@@ -1,13 +1,18 @@
 # frozen_string_literal: true
 
 class ProjectMemberPolicy < BasePolicy
+  include MemberPolicyHelpers
   delegate { @subject.project }
 
   condition(:target_is_holder_of_the_personal_namespace, scope: :subject) do
     @subject.project.personal_namespace_holder?(@subject.user)
   end
 
-  condition(:target_is_self) { @user && @subject.user == @user }
+  desc "Membership is users' own access request"
+  with_score 0
+  condition(:access_request_of_self) { record_is_access_request_of_self? }
+
+  condition(:target_is_self) { record_belongs_to_self? }
   condition(:project_bot) { @subject.user&.project_bot? }
 
   rule { anonymous }.prevent_all
@@ -24,5 +29,11 @@ class ProjectMemberPolicy < BasePolicy
 
   rule { project_bot & can?(:admin_project_member) }.enable :destroy_project_bot_member
 
-  rule { target_is_self }.enable :destroy_project_member
+  rule { target_is_self }.policy do
+    enable :destroy_project_member
+  end
+
+  rule { access_request_of_self }.policy do
+    enable :withdraw_member_access_request
+  end
 end

app/policies/project_policy.rb

@@ -834,6 +834,8 @@ class ProjectPolicy < BasePolicy
 
   rule { can?(:admin_project_member) }.policy do
     enable :import_project_members_from_another_project
+    # ability to read, approve or reject member access requests of other users
+    enable :admin_member_access_request
   end
 
   rule { registry_enabled & can?(:admin_container_image) }.policy do

app/services/members/approve_access_request_service.rb

@@ -16,7 +16,7 @@ module Members
     private
 
     def validate_access!(access_requester)
-      raise Gitlab::Access::AccessDeniedError unless can_update_access_requester?(access_requester)
+      raise Gitlab::Access::AccessDeniedError unless can_approve_access_requester?(access_requester)
 
       if approving_member_with_owner_access_level?(access_requester) &&
         cannot_assign_owner_responsibilities_to_member_in_project?(access_requester)
@@ -24,8 +24,8 @@ module Members
       end
     end
 
-    def can_update_access_requester?(access_requester)
+    def can_approve_access_requester?(access_requester)
-      can?(current_user, update_member_permission(access_requester), access_requester)
+      can?(current_user, :admin_member_access_request, access_requester.source)
     end
 
     def approving_member_with_owner_access_level?(access_requester)

app/services/members/destroy_service.rb

@@ -48,6 +48,10 @@ module Members
     def authorized?(member, destroy_bot)
       return can_destroy_bot_member?(member) if destroy_bot
 
+      if member.request?
+        return can_destroy_member_access_request?(member) || can_withdraw_member_access_request?(member)
+      end
+
       can_destroy_member?(member)
     end
 
@@ -106,6 +110,14 @@ module Members
       can?(current_user, destroy_bot_member_permission(member), member)
     end
 
+    def can_destroy_member_access_request?(member)
+      can?(current_user, :admin_member_access_request, member.source)
+    end
+
+    def can_withdraw_member_access_request?(member)
+      can?(current_user, :withdraw_member_access_request, member)
+    end
+
     def destroying_member_with_owner_access_level?(member)
       member.owner?
     end

app/views/shared/members/_access_request_links.html.haml

@@ -8,9 +8,10 @@
             data: { confirm: leave_confirmation_message(source), confirm_btn_variant: 'danger', qa_selector: 'leave_group_link' },
             class: 'js-leave-link'
 - elsif requester = source.requesters.find_by(user_id: current_user.id) # rubocop: disable CodeReuse/ActiveRecord
-  = link_to _('Withdraw Access Request'), polymorphic_path([:leave, source, :members]),
+  - if can?(current_user, :withdraw_member_access_request, requester)
-            method: :delete,
+    = link_to _('Withdraw Access Request'), polymorphic_path([:leave, source, :members]),
-            data: { confirm: remove_member_message(requester) }
+              method: :delete,
+              data: { confirm: remove_member_message(requester) }
 - elsif source.request_access_enabled && can?(current_user, :request_access, source)
   = link_to _('Request Access'), polymorphic_path([:request_access, source, :members]),
             method: :post

app/views/shared/web_hooks/_form.html.haml

@@ -10,11 +10,11 @@
       = s_('Webhooks|URL must be percent-encoded if it contains one or more special characters.')
 .form-group
   = form.label :token, s_('Webhooks|Secret token'), class: 'label-bold'
-  = form.text_field :token, class: 'form-control gl-form-input', placeholder: ''
+  = form.password_field :token, value: hook.masked_token, autocomplete: 'new-password', class: 'form-control gl-form-input'
   %p.form-text.text-muted
     - code_start = '<code>'.html_safe
     - code_end = '</code>'.html_safe
-    = s_('Webhooks|Used to validate received payloads. Sent with the request in the %{code_start}X-Gitlab-Token HTTP%{code_end} header.').html_safe % { code_start: code_start, code_end: code_end }
+    = s_('Webhooks|Used to validate received payloads. Sent with the request in the %{code_start}X-Gitlab-Token%{code_end} HTTP header.').html_safe % { code_start: code_start, code_end: code_end }
 .form-group
   = form.label :url, s_('Webhooks|Trigger'), class: 'label-bold'
   %ul.list-unstyled

config/initializers/0_marginalia.rb

@@ -13,7 +13,8 @@ require 'marginalia'
 # matching against the raw SQL, and prepending the comment prevents color
 # coding from working in the development log.
 Marginalia::Comment.prepend_comment = true if Rails.env.production?
-Marginalia::Comment.components = [:application, :correlation_id, :jid, :endpoint_id, :db_config_name]
+Marginalia::Comment.components = [:application, :correlation_id, :jid, :endpoint_id, :db_config_name,
+                                  :console_hostname, :console_username]
 
 # As mentioned in https://github.com/basecamp/marginalia/pull/93/files,
 # adding :line has some overhead because a regexp on the backtrace has

config/open_api.yml

@@ -20,5 +20,9 @@ metadata:
     description: Operations related to access requests
   - name: merge_requests
     description: Operations related to merge requests
+  - name: deploy_keys
+    description: Operations related to deploy keys
   - name: deployments
     description: Operations related to deployments
+  - name: release_links
+    description: Operations related to release assets (links)

doc/development/testing_guide/flaky_tests.md

@@ -11,9 +11,141 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 It's a test that sometimes fails, but if you retry it enough times, it passes,
 eventually.
 
+## What are the potential cause for a test to be flaky?
+
+### Unclean environment
+
+**Label:** `flaky-test::unclean environment`
+
+**Description:** The environment got dirtied by a previous test. The actual cause is probably not the flaky test here.
+
+**Difficulty to reproduce:** Moderate. Usually, running the same spec files until the one that's failing reproduces the problem.
+
+**Resolution:** Fix the previous tests and/or places where the environment is modified, so that
+it's reset to a pristine test after each test.
+
+**Examples:**
+
+- [Example 1](https://gitlab.com/gitlab-org/gitlab/-/issues/378414#note_1142026988): A migration
+  test might roll-back the database, perform its testing, and then roll-up the database in an
+  inconsistent state, so that following tests might not know about certain columns.
+- [Example 2](https://gitlab.com/gitlab-org/gitlab/-/issues/368500): A test modifies data that is
+  used by a following test.
+
+### Ordering assertion
+
+**Label:** `flaky-test::ordering assertion`
+
+**Description:** The test is expecting a specific order in the data under test yet the data is in
+a non-deterministic order.
+
+**Difficulty to reproduce:** Easy. Usually, running the test locally several times would reproduce
+the problem.
+
+**Resolution:** Depending on the problem, you might want to:
+
+- loosen the assertion if the test shouldn't care about ordering but only on the elements
+- fix the test by specifying a deterministic ordering
+- fix the app code by specifying a deterministic ordering
+
+**Examples:**
+
+- [Example 1](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/10148/diffs): Without
+  specifying `ORDER BY`, database will not give deterministic ordering, or data race happening
+  in the tests.
+
+### Dataset-specific
+
+**Label:** `flaky-test::dataset-specific`
+
+**Description:** The test assumes the dataset is in a particular (usually limited) state, which
+might not be true depending on when the test run during the test suite.
+
+**Difficulty to reproduce:** Moderate, as the amount of data needed to reproduce the issue might be
+difficult to achieve locally.
+
+**Resolution:** Fix the test to not assume that the dataset is in a particular state, don't hardcode IDs.
+
+**Examples:**
+
+- [Example 1](https://gitlab.com/gitlab-org/gitlab/-/issues/378381): The database is recreated when
+  any table has more than 500 columns. It could pass in the merge request, but fail later in
+  `master` if the order of tests changes.
+- [Example 2](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91016/diffs): A test asserts
+  that trying to find a record with an unexisting ID retuns an error message. The test uses an
+  hardcoded ID that's supposed to not exist (e.g. `42`). If the test is run early in the test
+  suite, it might pass as not enough records were created before it, but as soon as it would run
+  later in the suite, there could be a record that actually has the ID `42`, hence the test would
+  start to fail.
+
+### Random input
+
+**Label:** `flaky-test::random input`
+
+**Description:** The test use random values, that sometimes match the expectations, and sometimes not.
+
+**Difficulty to reproduce:** Easy, as the test can be modified locally to use the "random value"
+used at the time the test failed
+
+**Resolution:** Once the problem is reproduced, it should be easy to debug and fix either the test
+or the app.
+
+**Examples:**
+
+- [Example 1](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/20121): The test isn't robust enough to handle a specific data, that only appears sporadically since the data input is random.
+
+### Unreliable DOM Selector
+
+**Label:** `flaky-test::unreliable dom selector`
+
+**Description:** The DOM selector used in the test is unreliable.
+
+**Difficulty to reproduce:** Moderate to difficult. Depending on whether the DOM selector is duplicated, or appears after a delay etc.
+
+**Resolution:** It really depends on the problem here. It could be to wait for requests to finish, to scroll down the page etc.
+
+**Examples:**
+
+- [Example 1](https://gitlab.com/gitlab-org/gitlab/-/issues/338341): A non-unique CSS selector
+  matching more than one element, or a non-waiting selector method that does not allow rendering
+  time before throwing an `element not found` error.
+- [Example 2](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/101728/diffs): A CSS selector
+  only appears after a GraphQL requests has finished, and the UI has updated.
+
+### Datetime-sensitive
+
+**Label:** `flaky-test::datetime-sensitive`
+
+**Description:** The test is assuming a specific date or time.
+
+**Difficulty to reproduce:** Easy to moderate, depending on whether the test consistently fails after a certain date, or only fails at a given time or date.
+
+**Resolution:** Freezing the time is usually a good solution.
+
+**Examples:**
+
+- [Example 1](https://gitlab.com/gitlab-org/gitlab/-/issues/118612): A test that breaks after some time passed.
+
+### Unstable infrastructure
+
+**Label:** `flaky-test::unstable infrastructure`
+
+**Description:** The test fails from time to time due to infrastructure issues.
+
+**Difficulty to reproduce:** Hard. It's really hard to reproduce CI infrastructure issues. It might
+be possible by using containers locally.
+
+**Resolution:** Starting a conversation with the Infrastructure department in a dedicated issue is
+usually a good idea.
+
+**Examples:**
+
+- [Example 1](https://gitlab.com/gitlab-org/gitlab/-/issues/363214): The runner is under heavy load at this time.
+- [Example 2](https://gitlab.com/gitlab-org/gitlab/-/issues/360559): The runner is having networking issues, making a job failing early
+
 ## Quarantined tests
 
-When a test frequently fails in `main`,
+When a test frequently fails in `master`,
 create [a ~"failure::flaky-test" issue](https://about.gitlab.com/handbook/engineering/workflow/#broken-master).
 
 If the test cannot be fixed in a timely fashion, there is an impact on the

lib/api/api.rb

@@ -171,11 +171,13 @@ module API
       namespace do
         mount ::API::AccessRequests
         mount ::API::Appearance
+        mount ::API::DeployKeys
         mount ::API::Deployments
         mount ::API::Metadata
         mount ::API::MergeRequestDiffs
         mount ::API::UserCounts
         mount ::API::ProjectRepositoryStorageMoves
+        mount ::API::Release::Links
         mount ::API::SnippetRepositoryStorageMoves
         mount ::API::Statistics
 
@@ -219,7 +221,6 @@ module API
       mount ::API::DebianGroupPackages
       mount ::API::DebianProjectPackages
       mount ::API::DependencyProxy
-      mount ::API::DeployKeys
       mount ::API::DeployTokens
       mount ::API::Discussions
       mount ::API::Environments
@@ -294,7 +295,6 @@ module API
       mount ::API::ProtectedBranches
       mount ::API::ProtectedTags
       mount ::API::PypiPackages
-      mount ::API::Release::Links
       mount ::API::Releases
       mount ::API::RemoteMirrors
       mount ::API::Repositories

lib/api/deploy_keys.rb

@@ -21,7 +21,12 @@ module API
       # rubocop: enable CodeReuse/ActiveRecord
     end
 
-    desc 'Return all deploy keys'
+    desc 'List all deploy keys' do
+      detail 'Get a list of all deploy keys across all projects of the GitLab instance. This endpoint requires administrator access and is not available on GitLab.com.'
+      success Entities::DeployKey
+      is_array true
+      tags %w[deploy_keys]
+    end
     params do
       use :pagination
       optional :public, type: Boolean, default: false, desc: "Only return deploy keys that are public"
@@ -35,13 +40,16 @@ module API
     end
 
     params do
-      requires :id, type: String, desc: 'The ID of the project'
+      requires :id, types: [String, Integer], desc: 'The ID or URL-encoded path of the project owned by the authenticated user'
     end
     resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
       before { authorize_admin_project }
 
-      desc "Get a specific project's deploy keys" do
+      desc 'List deploy keys for project' do
+        detail "Get a list of a project's deploy keys."
         success Entities::DeployKeysProject
+        is_array true
+        tags %w[deploy_keys]
       end
       params do
         use :pagination
@@ -54,8 +62,10 @@ module API
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
-      desc 'Get single deploy key' do
+      desc 'Get a single deploy key' do
+        detail 'Get a single key.'
         success Entities::DeployKeysProject
+        tags %w[deploy_keys]
       end
       params do
         requires :key_id, type: Integer, desc: 'The ID of the deploy key'
@@ -66,12 +76,14 @@ module API
         present key, with: Entities::DeployKeysProject
       end
 
-      desc 'Add new deploy key to a project' do
+      desc 'Add deploy key' do
+        detail "Creates a new deploy key for a project. If the deploy key already exists in another project, it's joined to the current project only if the original one is accessible by the same user."
         success Entities::DeployKeysProject
+        tags %w[deploy_keys]
       end
       params do
-        requires :key, type: String, desc: 'The new deploy key'
+        requires :key, type: String, desc: 'New deploy key'
-        requires :title, type: String, desc: 'The name of the deploy key'
+        requires :title, type: String, desc: "New deploy key's title"
         optional :can_push, type: Boolean, desc: "Can deploy key push to the project's repository"
       end
       # rubocop: disable CodeReuse/ActiveRecord
@@ -109,12 +121,14 @@ module API
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
-      desc 'Update an existing deploy key for a project' do
+      desc 'Update deploy key' do
+        detail 'Updates a deploy key for a project.'
         success Entities::DeployKey
+        tags %w[deploy_keys]
       end
       params do
         requires :key_id, type: Integer, desc: 'The ID of the deploy key'
-        optional :title, type: String, desc: 'The name of the deploy key'
+        optional :title, type: String, desc: "New deploy key's title"
         optional :can_push, type: Boolean, desc: "Can deploy key push to the project's repository"
         at_least_one_of :title, :can_push
       end
@@ -143,9 +157,10 @@ module API
         end
       end
 
-      desc 'Enable a deploy key for a project' do
+      desc 'Enable a deploy key' do
-        detail 'This feature was added in GitLab 8.11'
+        detail 'Enables a deploy key for a project so this can be used. Returns the enabled key, with a status code 201 when successful. This feature was added in GitLab 8.11.'
         success Entities::DeployKey
+        tags %w[deploy_keys]
       end
       params do
         requires :key_id, type: Integer, desc: 'The ID of the deploy key'
@@ -161,7 +176,10 @@ module API
         end
       end
 
-      desc 'Delete deploy key for a project'
+      desc 'Delete deploy key' do
+        detail "Removes a deploy key from the project. If the deploy key is used only for this project, it's deleted from the system."
+        tags %w[deploy_keys]
+      end
       params do
         requires :key_id, type: Integer, desc: 'The ID of the deploy key'
       end

lib/api/release/links.rb

@@ -14,17 +14,19 @@ module API
       urgency :low
 
       params do
-        requires :id, type: String, desc: 'The ID of a project'
+        requires :id, type: [String, Integer], desc: 'The ID or URL-encoded path of the project'
       end
       resource 'projects/:id', requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
         params do
-          requires :tag_name, type: String, desc: 'The name of the tag', as: :tag
+          requires :tag_name, type: String, desc: 'The tag associated with the release', as: :tag
         end
         resource 'releases/:tag_name', requirements: RELEASE_ENDPOINT_REQUIREMENTS do
           resource :assets do
-            desc 'Get a list of links of a release' do
+            desc 'List links of a release' do
-              detail 'This feature was introduced in GitLab 11.7.'
+              detail 'Get assets as links from a release. This feature was introduced in GitLab 11.7.'
               success Entities::Releases::Link
+              is_array true
+              tags %w[release_links]
             end
             params do
               use :pagination
@@ -36,16 +38,20 @@ module API
               present paginate(release.links.sorted), with: Entities::Releases::Link
             end
 
-            desc 'Create a link of a release' do
+            desc 'Create a release link' do
-              detail 'This feature was introduced in GitLab 11.7.'
+              detail 'Create an asset as a link from a release. This feature was introduced in GitLab 11.7.'
               success Entities::Releases::Link
+              tags %w[release_links]
             end
             params do
-              requires :name, type: String, desc: 'The name of the link'
+              requires :name, type: String, desc: 'The name of the link. Link names must be unique in the release'
-              requires :url, type: String, desc: 'The URL of the link'
+              requires :url, type: String, desc: 'The URL of the link. Link URLs must be unique in the release.'
-              optional :filepath, type: String, desc: 'The filepath of the link'
+              optional :filepath, type: String, desc: 'Optional path for a direct asset link'
-              optional :link_type, type: String, values: %w[other runbook image package], default: 'other',
+              optional :link_type,
-                                   desc: 'The link type, one of: "runbook", "image", "package" or "other"'
+                type: String,
+                values: %w[other runbook image package],
+                default: 'other',
+                desc: 'The type of the link: `other`, `runbook`, `image`, or `package`. Defaults to `other`'
             end
             route_setting :authentication, job_token_allowed: true
             post 'links' do
@@ -61,12 +67,13 @@ module API
             end
 
             params do
-              requires :link_id, type: String, desc: 'The ID of the link'
+              requires :link_id, type: Integer, desc: 'The ID of the link'
             end
             resource 'links/:link_id' do
-              desc 'Get a link detail of a release' do
+              desc 'Get a release link' do
-                detail 'This feature was introduced in GitLab 11.7.'
+                detail 'Get an asset as a link from a release. This feature was introduced in GitLab 11.7.'
                 success Entities::Releases::Link
+                tags %w[release_links]
               end
               route_setting :authentication, job_token_allowed: true
               get do
@@ -75,16 +82,21 @@ module API
                 present link, with: Entities::Releases::Link
               end
 
-              desc 'Update a link of a release' do
+              desc 'Update a release link' do
-                detail 'This feature was introduced in GitLab 11.7.'
+                detail 'Update an asset as a link from a release. This feature was introduced in GitLab 11.7.'
                 success Entities::Releases::Link
+                tags %w[release_links]
               end
               params do
                 optional :name, type: String, desc: 'The name of the link'
                 optional :url, type: String, desc: 'The URL of the link'
-                optional :filepath, type: String, desc: 'The filepath of the link'
+                optional :filepath, type: String, desc: 'Optional path for a direct asset link'
-                optional :link_type, type: String, values: %w[other runbook image package], default: 'other',
+                optional :link_type,
-                                     desc: 'The link type, one of: "runbook", "image", "package" or "other"'
+                  type: String,
+                  values: %w[other runbook image package],
+                  default: 'other',
+                  desc: 'The type of the link: `other`, `runbook`, `image`, or `package`. Defaults to `other`'
+
                 at_least_one_of :name, :url
               end
               route_setting :authentication, job_token_allowed: true
@@ -98,9 +110,10 @@ module API
                 end
               end
 
-              desc 'Delete a link of a release' do
+              desc 'Delete a release link' do
-                detail 'This feature was introduced in GitLab 11.7.'
+                detail 'Deletes an asset as a link from a release. This feature was introduced in GitLab 11.7.'
                 success Entities::Releases::Link
+                tags %w[release_links]
               end
               route_setting :authentication, job_token_allowed: true
               delete do

lib/gitlab/marginalia/comment.rb

@@ -45,6 +45,18 @@ module Gitlab
       def db_config_name
         ::Gitlab::Database.db_config_name(marginalia_adapter)
       end
+
+      def console_hostname
+        return unless ::Gitlab::Runtime.console?
+
+        @cached_console_hostname ||= Socket.gethostname # rubocop:disable Gitlab/ModuleWithInstanceVariables
+      end
+
+      def console_username
+        return unless ::Gitlab::Runtime.console?
+
+        ENV['SUDO_USER'] || ENV['USER']
+      end
     end
   end
 end

locale/gitlab.pot

@@ -24768,9 +24768,6 @@ msgstr ""
 msgid "March"
 msgstr ""
 
-msgid "MardownDrawer|Could not fetch help contents."
-msgstr ""
-
 msgid "Mark as done"
 msgstr ""
 
@@ -45517,7 +45514,7 @@ msgstr ""
 msgid "Webhooks|URL preview"
 msgstr ""
 
-msgid "Webhooks|Used to validate received payloads. Sent with the request in the %{code_start}X-Gitlab-Token HTTP%{code_end} header."
+msgid "Webhooks|Used to validate received payloads. Sent with the request in the %{code_start}X-Gitlab-Token%{code_end} HTTP header."
 msgstr ""
 
 msgid "Webhooks|Webhook disabled"

spec/controllers/admin/hooks_controller_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Admin::HooksController do
-  let(:admin) { create(:admin) }
+  let_it_be(:admin) { create(:admin) }
 
   before do
     sign_in(admin)
@@ -33,7 +33,23 @@ RSpec.describe Admin::HooksController do
   end
 
   describe 'POST #update' do
-    let!(:hook) { create(:system_hook) }
+    let_it_be_with_reload(:hook) { create(:system_hook) }
+
+    context 'with an existing token' do
+      hook_params = {
+        token: WebHook::SECRET_MASK,
+        url: "http://example.com"
+      }
+
+      it 'does not change a token' do
+        expect do
+          post :update, params: { id: hook.id, hook: hook_params }
+        end.not_to change { hook.reload.token }
+
+        expect(response).to have_gitlab_http_status(:found)
+        expect(flash[:alert]).to be_blank
+      end
+    end
 
     it 'sets all parameters' do
       hook.update!(url_variables: { 'foo' => 'bar', 'baz' => 'woo' })
@@ -61,8 +77,8 @@ RSpec.describe Admin::HooksController do
   end
 
   describe 'DELETE #destroy' do
-    let!(:hook) { create(:system_hook) }
+    let_it_be(:hook) { create(:system_hook) }
-    let!(:log) { create(:web_hook_log, web_hook: hook) }
+    let_it_be(:log) { create(:web_hook_log, web_hook: hook) }
     let(:params) { { id: hook } }
 
     it_behaves_like 'Web hook destroyer'

spec/controllers/projects/hooks_controller_spec.rb

@@ -29,6 +29,22 @@ RSpec.describe Projects::HooksController do
       { namespace_id: project.namespace, project_id: project, id: hook.id }
     end
 
+    context 'with an existing token' do
+      hook_params = {
+        token: WebHook::SECRET_MASK,
+        url: "http://example.com"
+      }
+
+      it 'does not change a token' do
+        expect do
+          post :update, params: params.merge({ hook: hook_params })
+        end.not_to change { hook.reload.token }
+
+        expect(response).to have_gitlab_http_status(:found)
+        expect(flash[:alert]).to be_blank
+      end
+    end
+
     it 'adds, updates and deletes URL variables' do
       hook.update!(url_variables: { 'a' => 'bar', 'b' => 'woo' })
 

spec/frontend/members/components/action_buttons/access_request_action_buttons_spec.js

@@ -24,86 +24,52 @@ describe('AccessRequestActionButtons', () => {
     wrapper.destroy();
   });
 
-  describe('when user has `canRemove` permissions', () => {
+  it('renders remove member button', () => {
-    beforeEach(() => {
+    createComponent();
+
+    expect(findRemoveMemberButton().exists()).toBe(true);
+  });
+
+  it('sets props correctly on remove member button', () => {
+    createComponent();
+
+    expect(findRemoveMemberButton().props()).toMatchObject({
+      memberId: member.id,
+      title: 'Deny access',
+      isAccessRequest: true,
+      isInvite: false,
+      icon: 'close',
+    });
+  });
+
+  describe('when member is the current user', () => {
+    it('sets `message` prop correctly', () => {
+      createComponent();
+
+      expect(findRemoveMemberButton().props('message')).toBe(
+        `Are you sure you want to withdraw your access request for "${member.source.fullName}"`,
+      );
+    });
+  });
+
+  describe('when member is not the current user', () => {
+    it('sets `message` prop correctly', () => {
       createComponent({
+        isCurrentUser: false,
         permissions: {
           canRemove: true,
         },
       });
-    });
 
-    it('renders remove member button', () => {
+      expect(findRemoveMemberButton().props('message')).toBe(
-      expect(findRemoveMemberButton().exists()).toBe(true);
+        `Are you sure you want to deny ${member.user.name}'s request to join "${member.source.fullName}"`,
-    });
+      );
-
-    it('sets props correctly', () => {
-      expect(findRemoveMemberButton().props()).toMatchObject({
-        memberId: member.id,
-        title: 'Deny access',
-        isAccessRequest: true,
-        isInvite: false,
-        icon: 'close',
-      });
-    });
-
-    describe('when member is the current user', () => {
-      it('sets `message` prop correctly', () => {
-        expect(findRemoveMemberButton().props('message')).toBe(
-          `Are you sure you want to withdraw your access request for "${member.source.fullName}"`,
-        );
-      });
-    });
-
-    describe('when member is not the current user', () => {
-      it('sets `message` prop correctly', () => {
-        createComponent({
-          isCurrentUser: false,
-          permissions: {
-            canRemove: true,
-          },
-        });
-
-        expect(findRemoveMemberButton().props('message')).toBe(
-          `Are you sure you want to deny ${member.user.name}'s request to join "${member.source.fullName}"`,
-        );
-      });
     });
   });
 
-  describe('when user does not have `canRemove` permissions', () => {
+  it('renders the approve button', () => {
-    it('does not render remove member button', () => {
+    createComponent();
-      createComponent({
-        permissions: {
-          canRemove: false,
-        },
-      });
 
-      expect(findRemoveMemberButton().exists()).toBe(false);
+    expect(findApproveButton().exists()).toBe(true);
-    });
-  });
-
-  describe('when user has `canUpdate` permissions', () => {
-    it('renders the approve button', () => {
-      createComponent({
-        permissions: {
-          canUpdate: true,
-        },
-      });
-
-      expect(findApproveButton().exists()).toBe(true);
-    });
-  });
-
-  describe('when user does not have `canUpdate` permissions', () => {
-    it('does not render the approve button', () => {
-      createComponent({
-        permissions: {
-          canUpdate: false,
-        },
-      });
-
-      expect(findApproveButton().exists()).toBe(false);
-    });
   });
 });

spec/frontend/members/components/members_tabs_spec.js

@@ -81,6 +81,7 @@ describe('MembersTabs', () => {
       stubs: ['members-app'],
       provide: {
         canManageMembers: true,
+        canManageAccessRequests: true,
         canExportMembers: true,
         exportCsvPath: '',
         ...provide,
@@ -181,7 +182,9 @@ describe('MembersTabs', () => {
 
   describe('when `canManageMembers` is `false`', () => {
     it('shows all tabs except `Invited` and `Access requests`', async () => {
-      await createComponent({ provide: { canManageMembers: false } });
+      await createComponent({
+        provide: { canManageMembers: false, canManageAccessRequests: false },
+      });
 
       expect(findTabByText('Members')).not.toBeUndefined();
       expect(findTabByText('Groups')).not.toBeUndefined();

spec/frontend/vue_shared/components/markdown_drawer/markdown_drawer_spec.js

@@ -1,205 +0,0 @@
-import { GlDrawer, GlAlert, GlSkeletonLoader } from '@gitlab/ui';
-import { nextTick } from 'vue';
-import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
-import MarkdownDrawer, { cache } from '~/vue_shared/components/markdown_drawer/markdown_drawer.vue';
-import { getRenderedMarkdown } from '~/vue_shared/components/markdown_drawer/utils/fetch';
-import { contentTop } from '~/lib/utils/common_utils';
-
-jest.mock('~/vue_shared/components/markdown_drawer/utils/fetch', () => ({
-  getRenderedMarkdown: jest.fn().mockReturnValue({
-    title: 'test title test',
-    body: `<div id="content-body">
-          <div class="documentation md gl-mt-3">
-            test body
-          </div>
-      </div>`,
-  }),
-}));
-
-jest.mock('~/lib/utils/common_utils', () => ({
-  contentTop: jest.fn(),
-}));
-
-describe('MarkdownDrawer', () => {
-  let wrapper;
-  const defaultProps = {
-    documentPath: 'user/search/global_search/advanced_search_syntax.json',
-  };
-
-  const createComponent = (props) => {
-    wrapper = shallowMountExtended(MarkdownDrawer, {
-      propsData: {
-        ...defaultProps,
-        ...props,
-      },
-    });
-  };
-
-  afterEach(() => {
-    wrapper.destroy();
-    wrapper = null;
-    Object.keys(cache).forEach((key) => delete cache[key]);
-  });
-
-  const findDrawer = () => wrapper.findComponent(GlDrawer);
-  const findAlert = () => wrapper.findComponent(GlAlert);
-  const findSkeleton = () => wrapper.findComponent(GlSkeletonLoader);
-  const findDrawerTitle = () => wrapper.findComponent('[data-testid="title-element"]');
-  const findDrawerBody = () => wrapper.findComponent({ ref: 'content-element' });
-
-  describe('component', () => {
-    beforeEach(() => {
-      createComponent();
-    });
-
-    it('renders correctly', () => {
-      expect(findDrawer().exists()).toBe(true);
-      expect(findDrawerTitle().text()).toBe('test title test');
-      expect(findDrawerBody().text()).toBe('test body');
-    });
-  });
-
-  describe.each`
-    hasNavbar | navbarHeight
-    ${false}  | ${0}
-    ${true}   | ${100}
-  `('computes offsetTop', ({ hasNavbar, navbarHeight }) => {
-    beforeEach(() => {
-      global.document.querySelector = jest.fn(() =>
-        hasNavbar
-          ? {
-              dataset: {
-                page: 'test',
-              },
-            }
-          : undefined,
-      );
-      contentTop.mockReturnValue(navbarHeight);
-      createComponent();
-    });
-
-    afterEach(() => {
-      contentTop.mockClear();
-    });
-
-    it(`computes offsetTop ${hasNavbar ? 'with' : 'without'} .navbar-gitlab`, () => {
-      expect(findDrawer().attributes('headerheight')).toBe(`${navbarHeight}px`);
-    });
-  });
-
-  describe('watcher', () => {
-    let renderGLFMSpy;
-    let fetchMarkdownSpy;
-
-    beforeEach(async () => {
-      renderGLFMSpy = jest.spyOn(MarkdownDrawer.methods, 'renderGLFM');
-      fetchMarkdownSpy = jest.spyOn(MarkdownDrawer.methods, 'fetchMarkdown');
-      global.document.querySelector = jest.fn(() => ({
-        getBoundingClientRect: jest.fn(() => ({ bottom: 100 })),
-        dataset: {
-          page: 'test',
-        },
-      }));
-      createComponent();
-      await nextTick();
-    });
-
-    afterEach(() => {
-      renderGLFMSpy.mockClear();
-      fetchMarkdownSpy.mockClear();
-    });
-
-    it('for documentPath triggers fetch', async () => {
-      expect(fetchMarkdownSpy).toHaveBeenCalledTimes(1);
-
-      await wrapper.setProps({ documentPath: '/test/me' });
-      await nextTick();
-
-      expect(fetchMarkdownSpy).toHaveBeenCalledTimes(2);
-    });
-
-    it('for open triggers renderGLFM', async () => {
-      wrapper.vm.fetchMarkdown();
-      wrapper.vm.openDrawer();
-      await nextTick();
-      expect(renderGLFMSpy).toHaveBeenCalled();
-    });
-  });
-
-  describe('Markdown fetching', () => {
-    let renderGLFMSpy;
-
-    beforeEach(async () => {
-      renderGLFMSpy = jest.spyOn(MarkdownDrawer.methods, 'renderGLFM');
-      createComponent();
-      await nextTick();
-    });
-
-    afterEach(() => {
-      renderGLFMSpy.mockClear();
-    });
-
-    it('fetches the Markdown and caches it', async () => {
-      expect(getRenderedMarkdown).toHaveBeenCalledTimes(1);
-      expect(Object.keys(cache)).toHaveLength(1);
-    });
-
-    it('when the document changes, fetches it and caches it as well', async () => {
-      expect(getRenderedMarkdown).toHaveBeenCalledTimes(1);
-      expect(Object.keys(cache)).toHaveLength(1);
-
-      await wrapper.setProps({ documentPath: '/test/me2' });
-      await nextTick();
-
-      expect(getRenderedMarkdown).toHaveBeenCalledTimes(2);
-      expect(Object.keys(cache)).toHaveLength(2);
-    });
-
-    it('when re-using an already fetched document, gets it from the cache', async () => {
-      await wrapper.setProps({ documentPath: '/test/me2' });
-      await nextTick();
-
-      expect(getRenderedMarkdown).toHaveBeenCalledTimes(2);
-      expect(Object.keys(cache)).toHaveLength(2);
-
-      await wrapper.setProps({ documentPath: defaultProps.documentPath });
-      await nextTick();
-
-      expect(getRenderedMarkdown).toHaveBeenCalledTimes(2);
-      expect(Object.keys(cache)).toHaveLength(2);
-    });
-  });
-
-  describe('Markdown fetching returns error', () => {
-    beforeEach(async () => {
-      getRenderedMarkdown.mockReturnValue({
-        hasFetchError: true,
-      });
-
-      createComponent();
-      await nextTick();
-    });
-    afterEach(() => {
-      getRenderedMarkdown.mockClear();
-    });
-    it('shows alert', () => {
-      expect(findAlert().exists()).toBe(true);
-    });
-  });
-
-  describe('While Markdown is fetching', () => {
-    beforeEach(async () => {
-      getRenderedMarkdown.mockReturnValue(new Promise(() => {}));
-
-      createComponent();
-    });
-
-    afterEach(() => {
-      getRenderedMarkdown.mockClear();
-    });
-
-    it('shows skeleton', async () => {
-      expect(findSkeleton().exists()).toBe(true);
-    });
-  });
-});

spec/frontend/vue_shared/components/markdown_drawer/mock_data.js

@@ -1,42 +0,0 @@
-export const MOCK_HTML = `<!DOCTYPE html>
-<html>
-<body>
-  <div id="content-body">
-    <h1>test title <strong>test</strong></h1>
-    <div class="documentation md gl-mt-3">
-      <a href="../advanced_search.md">Advanced Search</a>
-      <a href="../advanced_search2.md">Advanced Search2</a>
-      <h2>test header h2</h2>
-      <table class="testClass">
-        <tr>
-          <td>Emil</td>
-          <td>Tobias</td>
-          <td>Linus</td>
-        </tr>
-        <tr>
-          <td>16</td>
-          <td>14</td>
-          <td>10</td>
-        </tr>
-      </table>
-    </div>
-  </div>
-</body>
-</html>`.replace(/\n/g, '');
-
-export const MOCK_DRAWER_DATA = {
-  hasFetchError: false,
-  title: 'test title test',
-  body: `  <div id="content-body">        <div class="documentation md gl-mt-3">      <a href="../advanced_search.md">Advanced Search</a>      <a href="../advanced_search2.md">Advanced Search2</a>      <h2>test header h2</h2>      <table class="testClass">        <tbody><tr>          <td>Emil</td>          <td>Tobias</td>          <td>Linus</td>        </tr>        <tr>          <td>16</td>          <td>14</td>          <td>10</td>        </tr>      </tbody></table>    </div>  </div>`,
-};
-
-export const MOCK_DRAWER_DATA_ERROR = {
-  hasFetchError: true,
-};
-
-export const MOCK_TABLE_DATA_BEFORE = `<head></head><body><h1>test</h1></test><table><tbody><tr><td></td></tr></tbody></table></body>`;
-
-export const MOCK_HTML_DATA_AFTER = {
-  body: '<table><tbody><tr><td></td></tr></tbody></table>',
-  title: 'test',
-};

spec/frontend/vue_shared/components/markdown_drawer/utils/fetch_spec.js

@@ -1,43 +0,0 @@
-import MockAdapter from 'axios-mock-adapter';
-import {
-  getRenderedMarkdown,
-  splitDocument,
-} from '~/vue_shared/components/markdown_drawer/utils/fetch';
-import axios from '~/lib/utils/axios_utils';
-import {
-  MOCK_HTML,
-  MOCK_DRAWER_DATA,
-  MOCK_DRAWER_DATA_ERROR,
-  MOCK_TABLE_DATA_BEFORE,
-  MOCK_HTML_DATA_AFTER,
-} from '../mock_data';
-
-describe('utils/fetch', () => {
-  let mock;
-
-  afterEach(() => {
-    mock.restore();
-  });
-
-  describe.each`
-    axiosMock                                  | type         | toExpect
-    ${{ code: 200, res: { html: MOCK_HTML } }} | ${'success'} | ${MOCK_DRAWER_DATA}
-    ${{ code: 500, res: null }}                | ${'error'}   | ${MOCK_DRAWER_DATA_ERROR}
-  `('process markdown data', ({ axiosMock, type, toExpect }) => {
-    describe(`if api fetch responds with ${type}`, () => {
-      beforeEach(() => {
-        mock = new MockAdapter(axios);
-        mock.onGet().reply(axiosMock.code, axiosMock.res);
-      });
-      it(`should update drawer correctly`, async () => {
-        expect(await getRenderedMarkdown('/any/path')).toStrictEqual(toExpect);
-      });
-    });
-  });
-
-  describe('splitDocument', () => {
-    it(`should update tables correctly`, () => {
-      expect(splitDocument(MOCK_TABLE_DATA_BEFORE)).toStrictEqual(MOCK_HTML_DATA_AFTER);
-    });
-  });
-});

spec/helpers/groups/group_members_helper_spec.rb

@@ -26,6 +26,7 @@ RSpec.describe Groups::GroupMembersHelper do
       allow(helper).to receive(:group_group_member_path).with(shared_group, ':id').and_return('/groups/foo-bar/-/group_members/:id')
       allow(helper).to receive(:group_group_link_path).with(shared_group, ':id').and_return('/groups/foo-bar/-/group_links/:id')
       allow(helper).to receive(:can?).with(current_user, :admin_group_member, shared_group).and_return(true)
+      allow(helper).to receive(:can?).with(current_user, :admin_member_access_request, shared_group).and_return(true)
     end
 
     subject do
@@ -53,7 +54,8 @@ RSpec.describe Groups::GroupMembersHelper do
     it 'returns expected json' do
       expected = {
         source_id: shared_group.id,
-        can_manage_members: true
+        can_manage_members: true,
+        can_manage_access_requests: true
       }
 
       expect(subject).to include(expected)
@@ -99,6 +101,7 @@ RSpec.describe Groups::GroupMembersHelper do
           allow(helper).to receive(:group_group_member_path).with(sub_shared_group, ':id').and_return('/groups/foo-bar/-/group_members/:id')
           allow(helper).to receive(:group_group_link_path).with(sub_shared_group, ':id').and_return('/groups/foo-bar/-/group_links/:id')
           allow(helper).to receive(:can?).with(current_user, :admin_group_member, sub_shared_group).and_return(true)
+          allow(helper).to receive(:can?).with(current_user, :admin_member_access_request, sub_shared_group).and_return(true)
           allow(helper).to receive(:can?).with(current_user, :export_group_memberships, sub_shared_group).and_return(true)
         end
 

spec/helpers/projects/project_members_helper_spec.rb

@@ -41,7 +41,8 @@ RSpec.describe Projects::ProjectMembersHelper do
       it 'returns expected json' do
         expected = {
           source_id: project.id,
-          can_manage_members: true
+          can_manage_members: true,
+          can_manage_access_requests: true
         }.as_json
 
         expect(subject).to include(expected)

spec/models/hooks/web_hook_spec.rb

@@ -299,7 +299,7 @@ RSpec.describe WebHook do
   end
 
   describe '#executable?' do
-    let(:web_hook) { create(:project_hook, project: project) }
+    let_it_be_with_reload(:web_hook) { create(:project_hook, project: project) }
 
     where(:recent_failures, :not_until, :executable) do
       [
@@ -757,4 +757,14 @@ RSpec.describe WebHook do
       expect { described_class.new.update_last_failure }.not_to raise_error
     end
   end
+
+  describe '#masked_token' do
+    it { expect(hook.masked_token).to be_nil }
+
+    context 'with a token' do
+      let(:hook) { build(:project_hook, :token, project: project) }
+
+      it { expect(hook.masked_token).to eq described_class::SECRET_MASK }
+    end
+  end
 end

spec/policies/group_member_policy_spec.rb

@@ -83,6 +83,31 @@ RSpec.describe GroupMemberPolicy do
     specify { expect_allowed(:read_group) }
   end
 
+  context 'for access requests' do
+    let_it_be(:group) { create(:group, :public) }
+    let_it_be(:user) { create(:user) }
+
+    let(:current_user) { user }
+
+    context 'for own access request' do
+      let(:membership) { create(:group_member, :access_request, group: group, user: user) }
+
+      specify { expect_allowed(:withdraw_member_access_request) }
+    end
+
+    context "for another user's access request" do
+      let(:membership) { create(:group_member, :access_request, group: group, user: create(:user)) }
+
+      specify { expect_disallowed(:withdraw_member_access_request) }
+    end
+
+    context 'for own, valid membership' do
+      let(:membership) { create(:group_member, :developer, group: group, user: user) }
+
+      specify { expect_disallowed(:withdraw_member_access_request) }
+    end
+  end
+
   context 'with bot user' do
     let(:current_user) { create(:user, :project_bot) }
 

spec/policies/project_member_policy_spec.rb

@@ -4,13 +4,14 @@ require 'spec_helper'
 
 RSpec.describe ProjectMemberPolicy do
   let(:project) { create(:project) }
-  let(:maintainer_user) { create(:user) }
+  let(:maintainer) { create(:user) }
   let(:member) { create(:project_member, project: project, user: member_user) }
+  let(:current_user) { maintainer }
 
-  subject { described_class.new(maintainer_user, member) }
+  subject { described_class.new(current_user, member) }
 
   before do
-    create(:project_member, :maintainer, project: project, user: maintainer_user)
+    create(:project_member, :maintainer, project: project, user: maintainer)
   end
 
   context 'with regular member' do
@@ -40,4 +41,29 @@ RSpec.describe ProjectMemberPolicy do
     it { is_expected.not_to be_allowed(:update_project_member) }
     it { is_expected.not_to be_allowed(:destroy_project_member) }
   end
+
+  context 'for access requests' do
+    let_it_be(:project) { create(:project, :public) }
+    let_it_be(:user) { create(:user) }
+
+    let(:current_user) { user }
+
+    context 'for own access request' do
+      let(:member) { create(:project_member, :access_request, project: project, user: user) }
+
+      specify { expect_allowed(:withdraw_member_access_request) }
+    end
+
+    context "for another user's access request" do
+      let(:member) { create(:project_member, :access_request, project: project, user: create(:user)) }
+
+      specify { expect_disallowed(:withdraw_member_access_request) }
+    end
+
+    context 'for own, valid membership' do
+      let(:member) { create(:project_member, :developer, project: project, user: user) }
+
+      specify { expect_disallowed(:withdraw_member_access_request) }
+    end
+  end
 end

spec/services/members/destroy_service_spec.rb

@@ -77,7 +77,7 @@ RSpec.describe Members::DestroyService do
     end
   end
 
-  shared_examples 'a service destroying an access requester' do
+  shared_examples 'a service destroying an access request of another user' do
     it_behaves_like 'a service destroying a member'
 
     it 'calls Member#after_decline_request' do
@@ -85,12 +85,16 @@ RSpec.describe Members::DestroyService do
 
       described_class.new(current_user).execute(member, **opts)
     end
+  end
+
+  shared_examples 'a service destroying an access request of self' do
+    it_behaves_like 'a service destroying a member'
 
     context 'when current user is the member' do
       it 'does not call Member#after_decline_request' do
         expect_any_instance_of(NotificationService).not_to receive(:decline_access_request).with(member)
 
-        described_class.new(member_user).execute(member, **opts)
+        described_class.new(current_user).execute(member, **opts)
       end
     end
   end
@@ -277,11 +281,24 @@ RSpec.describe Members::DestroyService do
         group.add_owner(current_user)
       end
 
-      it_behaves_like 'a service destroying an access requester' do
+      it_behaves_like 'a service destroying an access request of another user' do
         let(:member) { group_project.requesters.find_by(user_id: member_user.id) }
       end
 
-      it_behaves_like 'a service destroying an access requester' do
+      it_behaves_like 'a service destroying an access request of another user' do
+        let(:member) { group.requesters.find_by(user_id: member_user.id) }
+      end
+    end
+
+    context 'on withdrawing their own access request' do
+      let(:opts) { { skip_subresources: true } }
+      let(:current_user) { member_user }
+
+      it_behaves_like 'a service destroying an access request of self' do
+        let(:member) { group_project.requesters.find_by(user_id: member_user.id) }
+      end
+
+      it_behaves_like 'a service destroying an access request of self' do
         let(:member) { group.requesters.find_by(user_id: member_user.id) }
       end
     end

spec/support/shared_contexts/policies/group_policy_shared_context.rb

@@ -76,6 +76,7 @@ RSpec.shared_context 'GroupPolicy context' do
       register_group_runners
       read_billing
       edit_billing
+      admin_member_access_request
     ]
   end
 

spec/support/shared_contexts/policies/project_policy_shared_context.rb

@@ -68,7 +68,7 @@ RSpec.shared_context 'ProjectPolicy context' do
       admin_project admin_project_member admin_snippet admin_terraform_state
       admin_wiki create_deploy_token destroy_deploy_token
       push_to_delete_protected_branch read_deploy_token update_snippet
-      destroy_upload
+      destroy_upload admin_member_access_request
     ]
   end
 

storybook/config/preview.js

@@ -3,10 +3,6 @@ import Vue from 'vue';
 import { createMockServer } from 'test_helpers/mock_server';
 import translateMixin from '~/vue_shared/translate';
 
-// fixing toJSON error
-// https://github.com/storybookjs/storybook/issues/14933
-Vue.prototype.toJSON = () => {};
-
 const stylesheetsRequireCtx = require.context(
   '../../app/assets/stylesheets',
   true,