diff options
111 files changed, 1677 insertions, 1003 deletions
diff --git a/.gitlab/ci/global.gitlab-ci.yml b/.gitlab/ci/global.gitlab-ci.yml index 7e06a4a71bd..792e0ccc346 100644 --- a/.gitlab/ci/global.gitlab-ci.yml +++ b/.gitlab/ci/global.gitlab-ci.yml @@ -281,7 +281,7 @@ - name: postgres:12 command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"] - name: redis:6.0-alpine - - name: elasticsearch:8.1.1 + - name: elasticsearch:8.2.0 variables: POSTGRES_HOST_AUTH_METHOD: trust PG_VERSION: "12" diff --git a/.gitlab/ci/review-apps/dast.gitlab-ci.yml b/.gitlab/ci/review-apps/dast.gitlab-ci.yml index df8ad4c517a..6116aae3bea 100644 --- a/.gitlab/ci/review-apps/dast.gitlab-ci.yml +++ b/.gitlab/ci/review-apps/dast.gitlab-ci.yml @@ -10,7 +10,7 @@ variables: DAST_USERNAME_FIELD: "user[login]" DAST_PASSWORD_FIELD: "user[password]" - DAST_SUBMIT_FIELD: "commit" + DAST_SUBMIT_FIELD: "name:button" DAST_FULL_SCAN_ENABLED: "true" DAST_VERSION: 2 GIT_STRATEGY: none @@ -28,7 +28,7 @@ needs: ["review-deploy"] stage: dast # Default job timeout set to 90m and dast rules needs 2h to so that it won't timeout. - timeout: 2h + timeout: 3h # Add retry because of intermittent connection problems. See https://gitlab.com/gitlab-org/gitlab/-/issues/244313 retry: 1 artifacts: @@ -42,149 +42,65 @@ # DAST scan with a subset of Release scan rules. # ZAP rule details can be found at https://www.zaproxy.org/docs/alerts/ -# 10019, 10021 Missing security headers -# 10023, 10024, 10025, 10037 Information Disclosure -# 10040 Secure Pages Include Mixed Content -# 10056 X-Debug-Token Information Leak -# Duration: 14 minutes 20 seconds - -dast:secureHeaders-csp-infoLeak: +dast:anti-clickjacking-header: extends: - .dast_conf variables: DAST_USERNAME: "user1" - DAST_ONLY_INCLUDE_RULES: "10019,10021,10023,10024,10025,10037,10040,10056" + DAST_ONLY_INCLUDE_RULES: "10020" script: - /analyze -# 90023 XML External Entity Attack -# Duration: 41 minutes 20 seconds -# 90019 Server Side Code Injection -# Duration: 34 minutes 31 seconds -dast:XXE-SrvSideInj: +dast:xss-persistant: extends: - .dast_conf variables: DAST_USERNAME: "user2" - DAST_ONLY_INCLUDE_RULES: "90023,90019" + DAST_ONLY_INCLUDE_RULES: "40014" script: - /analyze -# 0 Directory Browsing -# 2 Private IP Disclosure -# 3 Session ID in URL Rewrite -# 7 Remote File Inclusion -# Duration: 63 minutes 43 seconds -# 90034 Cloud Metadata Potentially Exposed -# Duration: 13 minutes 48 seconds -# 90022 Application Error Disclosure -# Duration: 12 minutes 7 seconds -dast:infoLeak-fileInc-DirBrowsing: +dast:insecure-http-method: extends: - .dast_conf variables: DAST_USERNAME: "user3" - DAST_ONLY_INCLUDE_RULES: "0,2,3,7,90034,90022" + DAST_ONLY_INCLUDE_RULES: "90028" script: - /analyze -# 10010 Cookie No HttpOnly Flag -# 10011 Cookie Without Secure Flag -# 10017 Cross-Domain JavaScript Source File Inclusion -# 10029 Cookie Poisoning -# 90033 Loosely Scoped Cookie -# 10054 Cookie Without SameSite Attribute -# Duration: 13 minutes 23 seconds -dast:insecureCookie: +dast:server-side-template-inj: extends: - .dast_conf variables: DAST_USERNAME: "user4" - DAST_ONLY_INCLUDE_RULES: "10010,10011,10017,10029,90033,10054" + DAST_ONLY_INCLUDE_RULES: "90035" script: - /analyze - -# 20012 Anti-CSRF Tokens Check -# 10202 Absence of Anti-CSRF Tokens -# https://gitlab.com/gitlab-com/gl-security/appsec/appsec-team/-/issues/192 - -# Commented because of lot of FP's -# dast:csrfTokenCheck: -# extends: -# - .dast_conf -# variables: -# DAST_USERNAME: "user6" -# DAST_ONLY_INCLUDE_RULES: "20012,10202" -# script: -# - /analyze - -# 10098 Cross-Domain Misconfiguration -# 10105 Weak Authentication Method -# 40003 CRLF Injection -# 40008 Parameter Tampering -# Duration: 71 minutes 15 seconds -dast:corsMisconfig-weakauth-crlfInj: +dast:server-side-template-inj-blind: extends: - .dast_conf variables: DAST_USERNAME: "user5" - DAST_ONLY_INCLUDE_RULES: "10098,10105,40003,40008" + DAST_ONLY_INCLUDE_RULES: "90035" script: - /analyze -# 20019 External Redirect -# 20014 HTTP Parameter Pollution -# Duration: 46 minutes 12 seconds -dast:extRedirect-paramPollution: +dast:session-fixation: extends: - .dast_conf variables: DAST_USERNAME: "user6" - DAST_ONLY_INCLUDE_RULES: "20019,20014" - script: - - /analyze - -# 40022 SQL Injection - PostgreSQL -# Duration: 53 minutes 59 seconds -dast:sqlInjection: - extends: - - .dast_conf - variables: - DAST_USERNAME: "user7" - DAST_ONLY_INCLUDE_RULES: "40022" - script: - - /analyze - -# 40014 Cross Site Scripting (Persistent) -# Duration: 21 minutes 50 seconds -dast:xss-persistent: - extends: - - .dast_conf - variables: - DAST_USERNAME: "user8" - DAST_ONLY_INCLUDE_RULES: "40014" - script: - - /analyze - -# 40012 Cross Site Scripting (Reflected) -# Duration: 73 minutes 15 seconds -dast:xss-reflected: - extends: - - .dast_conf - variables: - DAST_USERNAME: "user9" - DAST_ONLY_INCLUDE_RULES: "40012" + DAST_ONLY_INCLUDE_RULES: "40013" script: - /analyze -# 40013 Session Fixation -# Duration: 44 minutes 25 seconds -dast:sessionFixation: +dast:xss-dombased: extends: - .dast_conf variables: DAST_USERNAME: "user10" - DAST_ONLY_INCLUDE_RULES: "40013" + DAST_ONLY_INCLUDE_RULES: "40026" script: - /analyze diff --git a/app/assets/javascripts/analytics/usage_trends/components/usage_counts.vue b/app/assets/javascripts/analytics/usage_trends/components/usage_counts.vue index 63ec40d4ec6..457a52d3807 100644 --- a/app/assets/javascripts/analytics/usage_trends/components/usage_counts.vue +++ b/app/assets/javascripts/analytics/usage_trends/components/usage_counts.vue @@ -1,5 +1,5 @@ <script> -import { GlDeprecatedSkeletonLoading as GlSkeletonLoading } from '@gitlab/ui'; +import { GlSkeletonLoader } from '@gitlab/ui'; import { GlSingleStat } from '@gitlab/ui/dist/charts'; import createFlash from '~/flash'; import { number } from '~/lib/utils/unit_format'; @@ -11,7 +11,7 @@ const defaultPrecision = 0; export default { name: 'UsageCounts', components: { - GlSkeletonLoading, + GlSkeletonLoader, GlSingleStat, }, data() { @@ -65,7 +65,7 @@ export default { <div class="gl-display-flex gl-flex-direction-column gl-md-flex-direction-row gl-my-6 gl-align-items-flex-start" > - <gl-skeleton-loading v-if="$apollo.queries.counts.loading" /> + <gl-skeleton-loader v-if="$apollo.queries.counts.loading" /> <template v-else> <gl-single-stat v-for="count in counts" diff --git a/app/assets/javascripts/editor/extensions/source_editor_markdown_livepreview_ext.js b/app/assets/javascripts/editor/extensions/source_editor_markdown_livepreview_ext.js index 11cc85c659d..e4ad0bf8e76 100644 --- a/app/assets/javascripts/editor/extensions/source_editor_markdown_livepreview_ext.js +++ b/app/assets/javascripts/editor/extensions/source_editor_markdown_livepreview_ext.js @@ -36,6 +36,8 @@ const setupDomElement = ({ injectToEl = null } = {}) => { return previewEl; }; +let dimResize = false; + export class EditorMarkdownPreviewExtension { static get extensionName() { return 'EditorMarkdownPreview'; @@ -50,6 +52,7 @@ export class EditorMarkdownPreviewExtension { }, shown: false, modelChangeListener: undefined, + layoutChangeListener: undefined, path: setupOptions.previewMarkdownPath, actionShowPreviewCondition: instance.createContextKey('toggleLivePreview', true), }; @@ -59,6 +62,14 @@ export class EditorMarkdownPreviewExtension { if (instance.toolbar) { this.setupToolbar(instance); } + + this.preview.layoutChangeListener = instance.onDidLayoutChange(() => { + if (instance.markdownPreview?.shown && !dimResize) { + const { width } = instance.getLayoutInfo(); + const newWidth = width * EXTENSION_MARKDOWN_PREVIEW_PANEL_WIDTH; + EditorMarkdownPreviewExtension.resizePreviewLayout(instance, newWidth); + } + }); } onBeforeUnuse(instance) { @@ -70,6 +81,9 @@ export class EditorMarkdownPreviewExtension { } cleanup(instance) { + if (this.preview.layoutChangeListener) { + this.preview.layoutChangeListener.dispose(); + } if (this.preview.modelChangeListener) { this.preview.modelChangeListener.dispose(); } @@ -82,6 +96,15 @@ export class EditorMarkdownPreviewExtension { this.preview.shown = false; } + static resizePreviewLayout(instance, width) { + const { height } = instance.getLayoutInfo(); + dimResize = true; + instance.layout({ width, height }); + window.requestAnimationFrame(() => { + dimResize = false; + }); + } + setupToolbar(instance) { this.toolbarButtons = [ { @@ -99,11 +122,11 @@ export class EditorMarkdownPreviewExtension { } togglePreviewLayout(instance) { - const { width, height } = instance.getLayoutInfo(); + const { width } = instance.getLayoutInfo(); const newWidth = this.preview.shown ? width / EXTENSION_MARKDOWN_PREVIEW_PANEL_WIDTH : width * EXTENSION_MARKDOWN_PREVIEW_PANEL_WIDTH; - instance.layout({ width: newWidth, height }); + EditorMarkdownPreviewExtension.resizePreviewLayout(instance, newWidth); } togglePreviewPanel(instance) { diff --git a/app/assets/javascripts/editor/extensions/source_editor_webide_ext.js b/app/assets/javascripts/editor/extensions/source_editor_webide_ext.js index 4e8c11bac54..6270517b3f3 100644 --- a/app/assets/javascripts/editor/extensions/source_editor_webide_ext.js +++ b/app/assets/javascripts/editor/extensions/source_editor_webide_ext.js @@ -7,7 +7,6 @@ * @property {Object} options The Monaco editor options */ -import { debounce } from 'lodash'; import { KeyCode, KeyMod, Range } from 'monaco-editor'; import { EDITOR_TYPE_DIFF } from '~/editor/constants'; import Disposable from '~/ide/lib/common/disposable'; @@ -59,13 +58,10 @@ const renderSideBySide = (domElement) => { return domElement.offsetWidth >= 700; }; -const updateInstanceDimensions = (instance) => { - instance.layout(); - if (isDiffEditorType(instance)) { - instance.updateOptions({ - renderSideBySide: renderSideBySide(instance.getDomNode()), - }); - } +const updateDiffInstanceRendering = (instance) => { + instance.updateOptions({ + renderSideBySide: renderSideBySide(instance.getDomNode()), + }); }; export class EditorWebIdeExtension { @@ -85,15 +81,14 @@ export class EditorWebIdeExtension { this.options = setupOptions.options; this.disposable = new Disposable(); - this.debouncedUpdate = debounce(() => { - updateInstanceDimensions(instance); - }, UPDATE_DIMENSIONS_DELAY); - addActions(instance, setupOptions.store); - } - onUse(instance) { - window.addEventListener('resize', this.debouncedUpdate, false); + if (isDiffEditorType(instance)) { + updateDiffInstanceRendering(instance); + instance.getModifiedEditor().onDidLayoutChange(() => { + updateDiffInstanceRendering(instance); + }); + } instance.onDidDispose(() => { this.onUnuse(); @@ -101,8 +96,6 @@ export class EditorWebIdeExtension { } onUnuse() { - window.removeEventListener('resize', this.debouncedUpdate); - // catch any potential errors with disposing the error // this is mainly for tests caused by elements not existing try { @@ -149,7 +142,6 @@ export class EditorWebIdeExtension { modified: model.getModel(), }); }, - updateDimensions: (instance) => updateInstanceDimensions(instance), setPos: (instance, { lineNumber, column }) => { instance.revealPositionInCenter({ lineNumber, diff --git a/app/assets/javascripts/editor/schema/ci.json b/app/assets/javascripts/editor/schema/ci.json index 2f8719ee6af..0a950cd1057 100644 --- a/app/assets/javascripts/editor/schema/ci.json +++ b/app/assets/javascripts/editor/schema/ci.json @@ -912,7 +912,7 @@ "cache": { "$ref": "#/definitions/cache" }, "secrets": { "$ref": "#/definitions/secrets" }, "script": { - "description": "Shell scripts executed by the Runner. The only required property of jobs. Be careful with special characters (e.g. `:`, `{`, `}`, `&`) and use single or double quotes to avoid issues.", + "markdownDescription": "Shell scripts executed by the Runner. The only required property of jobs. Be careful with special characters (e.g. `:`, `{`, `}`, `&`) and use single or double quotes to avoid issues. [Learn More](https://docs.gitlab.com/ee/ci/yaml/#script)", "oneOf": [ { "type": "string", diff --git a/app/assets/javascripts/ide/components/repo_editor.vue b/app/assets/javascripts/ide/components/repo_editor.vue index f14d86114b8..880272752b5 100644 --- a/app/assets/javascripts/ide/components/repo_editor.vue +++ b/app/assets/javascripts/ide/components/repo_editor.vue @@ -192,23 +192,6 @@ export default { this.createEditorInstance(); } }, - panelResizing() { - if (!this.panelResizing) { - this.refreshEditorDimensions(); - } - }, - showTabs() { - this.$nextTick(() => this.refreshEditorDimensions()); - }, - rightPaneIsOpen() { - this.refreshEditorDimensions(); - }, - showEditor(val) { - if (val) { - // We need to wait for the editor to actually be rendered. - this.$nextTick(() => this.refreshEditorDimensions()); - } - }, showContentViewer(val) { if (!val) return; @@ -396,10 +379,6 @@ export default { fileLanguage: this.model.language, }); - this.$nextTick(() => { - this.editor.updateDimensions(); - }); - this.$emit('editorSetup'); if (performance.getEntriesByName(WEBIDE_MARK_FILE_CLICKED).length) { eventHub.$emit(WEBIDE_MEASURE_FILE_AFTER_INTERACTION); @@ -415,11 +394,6 @@ export default { }); } }, - refreshEditorDimensions() { - if (this.showEditor && this.editor) { - this.editor.updateDimensions(); - } - }, fetchEditorconfigRules() { return getRulesWithTraversal(this.file.path, (path) => { const entry = this.entries[path]; diff --git a/app/assets/javascripts/ide/lib/editor_options.js b/app/assets/javascripts/ide/lib/editor_options.js index 52da9942efe..525afcb2083 100644 --- a/app/assets/javascripts/ide/lib/editor_options.js +++ b/app/assets/javascripts/ide/lib/editor_options.js @@ -8,6 +8,7 @@ export const defaultEditorOptions = { }, wordWrap: 'on', glyphMargin: true, + automaticLayout: true, }; export const defaultDiffOptions = { diff --git a/app/assets/javascripts/pages/projects/settings/repository/show/index.js b/app/assets/javascripts/pages/projects/settings/repository/show/index.js index d45052d76f4..655243eee30 100644 --- a/app/assets/javascripts/pages/projects/settings/repository/show/index.js +++ b/app/assets/javascripts/pages/projects/settings/repository/show/index.js @@ -1,7 +1,10 @@ import MirrorRepos from '~/mirrors/mirror_repos'; +import mountBranchRules from '~/projects/settings/repository/branch_rules/mount_branch_rules'; import initForm from '../form'; initForm(); const mirrorReposContainer = document.querySelector('.js-mirror-settings'); if (mirrorReposContainer) new MirrorRepos(mirrorReposContainer).init(); + +mountBranchRules(document.getElementById('js-branch-rules')); diff --git a/app/assets/javascripts/projects/commit/components/form_modal.vue b/app/assets/javascripts/projects/commit/components/form_modal.vue index f9dd72119d1..d9aaa574fec 100644 --- a/app/assets/javascripts/projects/commit/components/form_modal.vue +++ b/app/assets/javascripts/projects/commit/components/form_modal.vue @@ -53,7 +53,7 @@ export default { actionPrimary: { text: this.i18n.actionPrimaryText, attributes: [ - { variant: 'success' }, + { variant: 'confirm' }, { category: 'primary' }, { 'data-testid': 'submit-commit' }, { 'data-qa-selector': 'submit_commit_button' }, diff --git a/app/assets/javascripts/projects/settings/repository/branch_rules/app.vue b/app/assets/javascripts/projects/settings/repository/branch_rules/app.vue new file mode 100644 index 00000000000..ada951f6867 --- /dev/null +++ b/app/assets/javascripts/projects/settings/repository/branch_rules/app.vue @@ -0,0 +1,16 @@ +<script> +import { __ } from '~/locale'; + +export default { + name: 'BranchRules', + i18n: { heading: __('Branch') }, +}; +</script> + +<template> + <div> + <strong>{{ $options.i18n.heading }}</strong> + + <!-- TODO - List branch rules (https://gitlab.com/gitlab-org/gitlab/-/issues/362217) --> + </div> +</template> diff --git a/app/assets/javascripts/projects/settings/repository/branch_rules/mount_branch_rules.js b/app/assets/javascripts/projects/settings/repository/branch_rules/mount_branch_rules.js new file mode 100644 index 00000000000..abe0b93081e --- /dev/null +++ b/app/assets/javascripts/projects/settings/repository/branch_rules/mount_branch_rules.js @@ -0,0 +1,13 @@ +import Vue from 'vue'; +import BranchRulesApp from '~/projects/settings/repository/branch_rules/app.vue'; + +export default function mountBranchRules(el) { + if (!el) return null; + + return new Vue({ + el, + render(createElement) { + return createElement(BranchRulesApp); + }, + }); +} diff --git a/app/assets/javascripts/releases/graphql/fragments/release.fragment.graphql b/app/assets/javascripts/releases/graphql/fragments/release.fragment.graphql index 8a5613c75d2..e0de6d12b13 100644 --- a/app/assets/javascripts/releases/graphql/fragments/release.fragment.graphql +++ b/app/assets/javascripts/releases/graphql/fragments/release.fragment.graphql @@ -1,5 +1,6 @@ fragment Release on Release { __typename + id name tagName tagPath diff --git a/app/assets/javascripts/releases/graphql/fragments/release_for_editing.fragment.graphql b/app/assets/javascripts/releases/graphql/fragments/release_for_editing.fragment.graphql index 1823a327350..236d266a40a 100644 --- a/app/assets/javascripts/releases/graphql/fragments/release_for_editing.fragment.graphql +++ b/app/assets/javascripts/releases/graphql/fragments/release_for_editing.fragment.graphql @@ -1,4 +1,5 @@ fragment ReleaseForEditing on Release { + id name tagName description diff --git a/app/assets/javascripts/releases/graphql/mutations/create_release.mutation.graphql b/app/assets/javascripts/releases/graphql/mutations/create_release.mutation.graphql index 56bfe7c23d6..7344772adb9 100644 --- a/app/assets/javascripts/releases/graphql/mutations/create_release.mutation.graphql +++ b/app/assets/javascripts/releases/graphql/mutations/create_release.mutation.graphql @@ -1,6 +1,7 @@ mutation createRelease($input: ReleaseCreateInput!) { releaseCreate(input: $input) { release { + id links { selfUrl } diff --git a/app/assets/javascripts/releases/graphql/queries/all_releases.query.graphql b/app/assets/javascripts/releases/graphql/queries/all_releases.query.graphql index bda7ac52a47..61a06f268bd 100644 --- a/app/assets/javascripts/releases/graphql/queries/all_releases.query.graphql +++ b/app/assets/javascripts/releases/graphql/queries/all_releases.query.graphql @@ -13,6 +13,7 @@ query allReleases( __typename nodes { __typename + id name tagName tagPath diff --git a/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue b/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue index e7d5e4086bc..5b9845df5c7 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue @@ -110,8 +110,7 @@ export default { } else if (this.showUnapprove) { return { text: s__('mrWidget|Revoke approval'), - variant: 'warning', - category: 'secondary', + variant: 'default', action: () => this.unapprove(), }; } diff --git a/app/assets/javascripts/vue_shared/components/clone_dropdown.vue b/app/assets/javascripts/vue_shared/components/clone_dropdown.vue index f14e1992901..dd6923d9fcd 100644 --- a/app/assets/javascripts/vue_shared/components/clone_dropdown.vue +++ b/app/assets/javascripts/vue_shared/components/clone_dropdown.vue @@ -45,7 +45,7 @@ export default { }; </script> <template> - <gl-dropdown right :text="$options.labels.defaultLabel" category="primary" variant="info"> + <gl-dropdown right :text="$options.labels.defaultLabel" category="primary" variant="confirm"> <div class="pb-2 mx-1"> <template v-if="sshLink"> <gl-dropdown-section-header>{{ $options.labels.ssh }}</gl-dropdown-section-header> diff --git a/app/controllers/concerns/project_stats_refresh_conflicts_guard.rb b/app/controllers/concerns/project_stats_refresh_conflicts_guard.rb new file mode 100644 index 00000000000..a3349997dbd --- /dev/null +++ b/app/controllers/concerns/project_stats_refresh_conflicts_guard.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module ProjectStatsRefreshConflictsGuard + extend ActiveSupport::Concern + + def reject_if_build_artifacts_size_refreshing! + return unless project.refreshing_build_artifacts_size? + + Gitlab::ProjectStatsRefreshConflictsLogger.warn_request_rejected_during_stats_refresh(project.id) + + render_409('Action temporarily disabled. The project this pipeline belongs to is undergoing stats refresh.') + end +end diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb index 5be2d7527ff..1508531828d 100644 --- a/app/controllers/help_controller.rb +++ b/app/controllers/help_controller.rb @@ -13,7 +13,7 @@ class HelpController < ApplicationController def index # Remove YAML frontmatter so that it doesn't look weird - @help_index = File.read(Rails.root.join('doc', 'index.md')).sub(YAML_FRONT_MATTER_REGEXP, '') + @help_index = File.read(path_to_doc('index.md')).sub(YAML_FRONT_MATTER_REGEXP, '') # Prefix Markdown links with `help/` unless they are external links. # '//' not necessarily part of URL, e.g., mailto:mail@example.com @@ -24,7 +24,7 @@ class HelpController < ApplicationController end def show - @path = Rack::Utils.clean_path_info(path_params[:path]) + @path = Rack::Utils.clean_path_info(params[:path]) respond_to do |format| format.any(:markdown, :md, :html) do @@ -38,7 +38,7 @@ class HelpController < ApplicationController # Allow access to specific media files in the doc folder format.any(:png, :gif, :jpeg, :mp4, :mp3) do # Note: We are purposefully NOT using `Rails.root.join` because of https://gitlab.com/gitlab-org/gitlab/-/issues/216028. - path = File.join(Rails.root, 'doc', "#{@path}.#{params[:format]}") + path = path_to_doc("#{@path}.#{params[:format]}") if File.exist?(path) send_file(path, disposition: 'inline') @@ -61,16 +61,8 @@ class HelpController < ApplicationController private - def path_params - params.require(:path) - - params - end - def redirect_to_documentation_website? - return false unless Gitlab::UrlSanitizer.valid_web?(documentation_url) - - true + Gitlab::UrlSanitizer.valid_web?(documentation_url) end def documentation_url @@ -105,18 +97,22 @@ class HelpController < ApplicationController def render_documentation # Note: We are purposefully NOT using `Rails.root.join` because of https://gitlab.com/gitlab-org/gitlab/-/issues/216028. - path = File.join(Rails.root, 'doc', "#{@path}.md") + path = path_to_doc("#{@path}.md") if File.exist?(path) # Remove YAML frontmatter so that it doesn't look weird @markdown = File.read(path).gsub(YAML_FRONT_MATTER_REGEXP, '') - render 'show.html.haml' + render :show, formats: :html else # Force template to Haml - render 'errors/not_found.html.haml', layout: 'errors', status: :not_found + render 'errors/not_found', layout: 'errors', status: :not_found, formats: :html end end + + def path_to_doc(file_name) + File.join(Rails.root, 'doc', file_name) + end end ::HelpController.prepend_mod diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 8c9f82b9dc1..c7e983d3960 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -3,6 +3,7 @@ class Projects::JobsController < Projects::ApplicationController include SendFileUpload include ContinueParams + include ProjectStatsRefreshConflictsGuard urgency :low, [:index, :show, :trace, :retry, :play, :cancel, :unschedule, :status, :erase, :raw] @@ -19,6 +20,7 @@ class Projects::JobsController < Projects::ApplicationController before_action :verify_proxy_request!, only: :proxy_websocket_authorize before_action :push_jobs_table_vue, only: [:index] before_action :push_jobs_table_vue_search, only: [:index] + before_action :reject_if_build_artifacts_size_refreshing!, only: [:erase] before_action do push_frontend_feature_flag(:infinitely_collapsible_sections, @project) diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 94865024688..81e099d1aaf 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -3,6 +3,7 @@ class Projects::PipelinesController < Projects::ApplicationController include ::Gitlab::Utils::StrongMemoize include RedisTracking + include ProjectStatsRefreshConflictsGuard urgency :low, [ :index, :new, :builds, :show, :failures, :create, @@ -19,6 +20,7 @@ class Projects::PipelinesController < Projects::ApplicationController before_action :authorize_create_pipeline!, only: [:new, :create, :config_variables] before_action :authorize_update_pipeline!, only: [:retry, :cancel] before_action :ensure_pipeline, only: [:show, :downloadable_artifacts] + before_action :reject_if_build_artifacts_size_refreshing!, only: [:destroy] before_action do push_frontend_feature_flag(:pipeline_tabs_vue, @project) diff --git a/app/controllers/projects/settings/repository_controller.rb b/app/controllers/projects/settings/repository_controller.rb index 0fd2d56229a..a178b8f7aa3 100644 --- a/app/controllers/projects/settings/repository_controller.rb +++ b/app/controllers/projects/settings/repository_controller.rb @@ -15,6 +15,7 @@ module Projects urgency :low, [:show, :create_deploy_token] def show + push_frontend_feature_flag(:branch_rules, @project) render_show end diff --git a/app/graphql/mutations/ci/pipeline/destroy.rb b/app/graphql/mutations/ci/pipeline/destroy.rb index 3f933818ce1..935cf45c4ab 100644 --- a/app/graphql/mutations/ci/pipeline/destroy.rb +++ b/app/graphql/mutations/ci/pipeline/destroy.rb @@ -12,12 +12,25 @@ module Mutations pipeline = authorized_find!(id: id) project = pipeline.project + return undergoing_refresh_error(project) if project.refreshing_build_artifacts_size? + result = ::Ci::DestroyPipelineService.new(project, current_user).execute(pipeline) { success: result.success?, errors: result.errors } end + + private + + def undergoing_refresh_error(project) + Gitlab::ProjectStatsRefreshConflictsLogger.warn_request_rejected_during_stats_refresh(project.id) + + { + success: false, + errors: ['Action temporarily disabled. The project this pipeline belongs to is undergoing stats refresh.'] + } + end end end end diff --git a/app/graphql/resolvers/milestones_resolver.rb b/app/graphql/resolvers/milestones_resolver.rb index dc6d781f584..25ff783b408 100644 --- a/app/graphql/resolvers/milestones_resolver.rb +++ b/app/graphql/resolvers/milestones_resolver.rb @@ -4,6 +4,11 @@ module Resolvers class MilestonesResolver < BaseResolver include Gitlab::Graphql::Authorize::AuthorizeResource include TimeFrameArguments + include LooksAhead + + # authorize before resolution + authorize :read_milestone + authorizes_object! argument :ids, [GraphQL::Types::ID], required: false, @@ -34,12 +39,10 @@ module Resolvers NON_STABLE_CURSOR_SORTS = %i[expired_last_due_date_asc expired_last_due_date_desc].freeze - def resolve(**args) + def resolve_with_lookahead(**args) validate_timeframe_params!(args) - authorize! - - milestones = MilestonesFinder.new(milestones_finder_params(args)).execute + milestones = apply_lookahead(MilestonesFinder.new(milestones_finder_params(args)).execute) if non_stable_cursor_sort?(args[:sort]) offset_pagination(milestones) @@ -50,6 +53,12 @@ module Resolvers private + def preloads + { + releases: :releases + } + end + def milestones_finder_params(args) { ids: parse_gids(args[:ids]), @@ -69,12 +78,6 @@ module Resolvers raise NotImplementedError end - # MilestonesFinder does not check for current_user permissions, - # so for now we need to keep it here. - def authorize! - Ability.allowed?(context[:current_user], :read_milestone, parent) || raise_resource_not_available_error! - end - def parse_gids(gids) gids&.map { |gid| GitlabSchema.parse_gid(gid, expected_type: Milestone).model_id } end diff --git a/app/graphql/types/milestone_type.rb b/app/graphql/types/milestone_type.rb index 18e4a5d33e3..7741fd723f0 100644 --- a/app/graphql/types/milestone_type.rb +++ b/app/graphql/types/milestone_type.rb @@ -59,6 +59,10 @@ module Types field :stats, Types::MilestoneStatsType, null: true, description: 'Milestone statistics.' + field :releases, ::Types::ReleaseType.connection_type, + null: true, + description: 'Releases associated with this milestone.' + def stats milestone end diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index 01b1a71896a..bc3774bcc1d 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -52,6 +52,7 @@ module Types field :milestone, ::Types::MilestoneType, null: true, + extras: [:lookahead], description: 'Find a milestone.' do argument :id, ::Types::GlobalIDType[Milestone], required: true, description: 'Find a milestone by its ID.' end @@ -156,8 +157,9 @@ module Types GitlabSchema.find_by_gid(id) end - def milestone(id:) - GitlabSchema.find_by_gid(id) + def milestone(id:, lookahead:) + preloads = [:releases] if lookahead.selects?(:releases) + Gitlab::Graphql::Loaders::BatchModelLoader.new(id.model_class, id.model_id, preloads).find end def container_repository(id:) diff --git a/app/graphql/types/release_type.rb b/app/graphql/types/release_type.rb index 95b6b43bb46..43dc0c4ce85 100644 --- a/app/graphql/types/release_type.rb +++ b/app/graphql/types/release_type.rb @@ -13,6 +13,9 @@ module Types present_using ReleasePresenter + field :id, ::Types::GlobalIDType[Release], + null: false, + description: 'Global ID of the release.' field :assets, Types::ReleaseAssetsType, null: true, method: :itself, description: 'Assets of the release.' field :created_at, Types::TimeType, null: true, diff --git a/app/models/member.rb b/app/models/member.rb index 45ad47f56a4..bb5d2b10f8e 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -199,7 +199,6 @@ class Member < ApplicationRecord before_validation :generate_invite_token, on: :create, if: -> (member) { member.invite_email.present? && !member.invite_accepted_at? } after_create :send_invite, if: :invite?, unless: :importing? - after_create :send_request, if: :request?, unless: :importing? after_create :create_notification_setting, unless: [:pending?, :importing?] after_create :post_create_hook, unless: [:pending?, :importing?], if: :hook_prerequisites_met? after_update :post_update_hook, unless: [:pending?, :importing?], if: :hook_prerequisites_met? @@ -207,6 +206,7 @@ class Member < ApplicationRecord after_destroy :post_destroy_hook, unless: :pending?, if: :hook_prerequisites_met? after_save :log_invitation_token_cleanup + after_commit :send_request, if: :request?, unless: :importing?, on: [:create] after_commit on: [:create, :update], unless: :importing? do refresh_member_authorized_projects(blocking: blocking_refresh) end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 2700531235a..17235034aa5 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -245,6 +245,7 @@ class ProjectPolicy < BasePolicy enable :set_warn_about_potentially_unwanted_characters enable :register_project_runners + enable :manage_owners end rule { can?(:guest_access) }.policy do diff --git a/app/services/concerns/members/bulk_create_users.rb b/app/services/concerns/members/bulk_create_users.rb index e60c84af89e..5b2cd0a2e43 100644 --- a/app/services/concerns/members/bulk_create_users.rb +++ b/app/services/concerns/members/bulk_create_users.rb @@ -9,6 +9,9 @@ module Members def add_users(source, users, access_level, current_user: nil, expires_at: nil, tasks_to_be_done: [], tasks_project_id: nil) return [] unless users.present? + # If this user is attempting to manage Owner members and doesn't have permission, do not allow + return [] if managing_owners?(current_user, access_level) && cannot_manage_owners?(source, current_user) + emails, users, existing_members = parse_users_list(source, users) Member.transaction do @@ -28,6 +31,10 @@ module Members private + def managing_owners?(current_user, access_level) + current_user && Gitlab::Access.sym_options_with_owner[access_level] == Gitlab::Access::OWNER + end + def parse_users_list(source, list) emails = [] user_ids = [] diff --git a/app/services/members/base_service.rb b/app/services/members/base_service.rb index 3f55f661d9b..62b8fc5d6f7 100644 --- a/app/services/members/base_service.rb +++ b/app/services/members/base_service.rb @@ -60,5 +60,18 @@ module Members TodosDestroyer::EntityLeaveWorker.perform_in(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, type) end end + + def cannot_assign_owner_responsibilities_to_member_in_project?(member) + # The purpose of this check is - + # We can have direct members who are "Owners" in a project going forward and + # we do not want Maintainers of the project updating/adding/removing other "Owners" + # within the project. + # Only OWNERs in a project should be able to manage any action around OWNERship in that project. + member.is_a?(ProjectMember) && + !can?(current_user, :manage_owners, member.source) + end + + alias_method :cannot_revoke_owner_responsibilities_from_member_in_project?, + :cannot_assign_owner_responsibilities_to_member_in_project? end end diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index 8485e7cbafa..57d9da4cefd 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -22,6 +22,11 @@ module Members def execute raise Gitlab::Access::AccessDeniedError unless can?(current_user, create_member_permission(source), source) + # rubocop:disable Layout/EmptyLineAfterGuardClause + raise Gitlab::Access::AccessDeniedError if adding_at_least_one_owner && + cannot_assign_owner_responsibilities_to_member_in_project? + # rubocop:enable Layout/EmptyLineAfterGuardClause + validate_invite_source! validate_invitable! @@ -45,6 +50,14 @@ module Members attr_reader :source, :errors, :invites, :member_created_namespace_id, :members, :tasks_to_be_done_members, :member_created_member_task_id + def adding_at_least_one_owner + params[:access_level] == Gitlab::Access::OWNER + end + + def cannot_assign_owner_responsibilities_to_member_in_project? + source.is_a?(Project) && !current_user.can?(:manage_owners, source) + end + def invites_from_params # String, Nil, Array, Integer return params[:user_id] if params[:user_id].is_a?(Array) diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index bb2d419c046..0a8344c58db 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -3,7 +3,12 @@ module Members class DestroyService < Members::BaseService def execute(member, skip_authorization: false, skip_subresources: false, unassign_issuables: false, destroy_bot: false) - raise Gitlab::Access::AccessDeniedError unless skip_authorization || authorized?(member, destroy_bot) + unless skip_authorization + raise Gitlab::Access::AccessDeniedError unless authorized?(member, destroy_bot) + + raise Gitlab::Access::AccessDeniedError if destroying_member_with_owner_access_level?(member) && + cannot_revoke_owner_responsibilities_from_member_in_project?(member) + end @skip_auth = skip_authorization @@ -90,6 +95,10 @@ module Members can?(current_user, destroy_bot_member_permission(member), member) end + def destroying_member_with_owner_access_level?(member) + member.owner? + end + def destroy_member_permission(member) case member when GroupMember diff --git a/app/services/members/groups/bulk_creator_service.rb b/app/services/members/groups/bulk_creator_service.rb index 57cec241584..e93ef0e021c 100644 --- a/app/services/members/groups/bulk_creator_service.rb +++ b/app/services/members/groups/bulk_creator_service.rb @@ -4,6 +4,12 @@ module Members module Groups class BulkCreatorService < Members::Groups::CreatorService include Members::BulkCreateUsers + + class << self + def cannot_manage_owners?(source, current_user) + source.max_member_access_for_user(current_user) < Gitlab::Access::OWNER + end + end end end end diff --git a/app/services/members/projects/bulk_creator_service.rb b/app/services/members/projects/bulk_creator_service.rb index 68e71e35d12..17488aa3b38 100644 --- a/app/services/members/projects/bulk_creator_service.rb +++ b/app/services/members/projects/bulk_creator_service.rb @@ -4,6 +4,12 @@ module Members module Projects class BulkCreatorService < Members::Projects::CreatorService include Members::BulkCreateUsers + + class << self + def cannot_manage_owners?(source, current_user) + !Ability.allowed?(current_user, :manage_owners, source) + end + end end end end diff --git a/app/services/members/projects/creator_service.rb b/app/services/members/projects/creator_service.rb index 9e9389d3c18..56d005e24ab 100644 --- a/app/services/members/projects/creator_service.rb +++ b/app/services/members/projects/creator_service.rb @@ -6,6 +6,9 @@ module Members private def can_create_new_member? + return false if assigning_project_member_with_owner_access_level? && + cannot_assign_owner_responsibilities_to_member_in_project? + # This access check(`admin_project_member`) will write to safe request store cache for the user being added. # This means any operations inside the same request will need to purge that safe request # store cache if operations are needed to be done inside the same request that checks max member access again on @@ -14,6 +17,11 @@ module Members end def can_update_existing_member? + # rubocop:disable Layout/EmptyLineAfterGuardClause + raise ::Gitlab::Access::AccessDeniedError if assigning_project_member_with_owner_access_level? && + cannot_assign_owner_responsibilities_to_member_in_project? + # rubocop:enable Layout/EmptyLineAfterGuardClause + current_user.can?(:update_project_member, member) end @@ -21,6 +29,16 @@ module Members # this condition is reached during testing setup a lot due to use of `.add_user` member.project.personal_namespace_holder?(member.user) end + + def assigning_project_member_with_owner_access_level? + return true if member && member.owner? + + access_level == Gitlab::Access::OWNER + end + + def cannot_assign_owner_responsibilities_to_member_in_project? + member.is_a?(ProjectMember) && !current_user.can?(:manage_owners, member.source) + end end end end diff --git a/app/services/members/update_service.rb b/app/services/members/update_service.rb index 257698f65ae..b4d1b80e5a3 100644 --- a/app/services/members/update_service.rb +++ b/app/services/members/update_service.rb @@ -5,6 +5,7 @@ module Members # returns the updated member def execute(member, permission: :update) raise Gitlab::Access::AccessDeniedError unless can?(current_user, action_member_permission(permission, member), member) + raise Gitlab::Access::AccessDeniedError if prevent_upgrade_to_owner?(member) || prevent_downgrade_from_owner?(member) old_access_level = member.human_access old_expiry = member.expires_at @@ -28,6 +29,22 @@ module Members def downgrading_to_guest? params[:access_level] == Gitlab::Access::GUEST end + + def upgrading_to_owner? + params[:access_level] == Gitlab::Access::OWNER + end + + def downgrading_from_owner?(member) + member.owner? + end + + def prevent_upgrade_to_owner?(member) + upgrading_to_owner? && cannot_assign_owner_responsibilities_to_member_in_project?(member) + end + + def prevent_downgrade_from_owner?(member) + downgrading_from_owner?(member) && cannot_revoke_owner_responsibilities_from_member_in_project?(member) + end end end diff --git a/app/views/admin/application_settings/_whats_new.html.haml b/app/views/admin/application_settings/_whats_new.html.haml index 70ba994d21e..b84e3f12e63 100644 --- a/app/views/admin/application_settings/_whats_new.html.haml +++ b/app/views/admin/application_settings/_whats_new.html.haml @@ -1,13 +1,8 @@ -= form_for @application_setting, url: preferences_admin_application_settings_path(anchor: 'js-whats-new-settings'), html: { class: 'fieldset-form whats-new-settings' } do |f| += gitlab_ui_form_for @application_setting, url: preferences_admin_application_settings_path(anchor: 'js-whats-new-settings'), html: { class: 'fieldset-form whats-new-settings' } do |f| = form_errors(@application_setting) - whats_new_variants.keys.each do |variant| - .form-check.gl-mb-4 - = f.radio_button :whats_new_variant, variant, class: 'form-check-input' - = f.label :whats_new_variant, value: variant, class: 'form-check-label' do - .font-weight-bold - = whats_new_variants_label(variant) - .option-description - = whats_new_variants_description(variant) + .gl-mb-4 + = f.gitlab_ui_radio_component :whats_new_variant, variant, whats_new_variants_label(variant), help_text: whats_new_variants_description(variant) = f.submit _('Save changes'), class: "gl-button btn btn-confirm" diff --git a/app/views/projects/branch_rules/_show.html.haml b/app/views/projects/branch_rules/_show.html.haml new file mode 100644 index 00000000000..5f1e7d74b35 --- /dev/null +++ b/app/views/projects/branch_rules/_show.html.haml @@ -0,0 +1,12 @@ +- expanded = expanded_by_default? + +%section.settings.no-animate#branch-rules{ class: ('expanded' if expanded) } + .settings-header + %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only= _('Branch rules') + %button.btn.gl-button.btn-default.js-settings-toggle + = expanded ? _('Collapse') : _('Expand') + %p + = _('Define rules for who can push, merge, and the required approvals for each branch.') + + .settings-content.gl-pr-0 + #js-branch-rules diff --git a/app/views/projects/merge_requests/_mr_box.html.haml b/app/views/projects/merge_requests/_mr_box.html.haml index 04fcafd1277..4fc405c63ff 100644 --- a/app/views/projects/merge_requests/_mr_box.html.haml +++ b/app/views/projects/merge_requests/_mr_box.html.haml @@ -1,3 +1,3 @@ -.detail-page-description.py-2{ class: "#{'is-merge-request' if !fluid_layout}" } +.detail-page-description.py-2{ class: "#{'is-merge-request' if moved_mr_sidebar_enabled? && !fluid_layout}" } = render 'shared/issuable/status_box', issuable: @merge_request = merge_request_header(@project, @merge_request) diff --git a/app/views/projects/merge_requests/creations/_new_compare.html.haml b/app/views/projects/merge_requests/creations/_new_compare.html.haml index 811b45ef8af..12faba68315 100644 --- a/app/views/projects/merge_requests/creations/_new_compare.html.haml +++ b/app/views/projects/merge_requests/creations/_new_compare.html.haml @@ -1,4 +1,4 @@ -%h3.page-title +%h1.page-title = _('New merge request') = form_for [@project, @merge_request], url: project_new_merge_request_path(@project), method: :get, html: { class: "merge-request-form js-requires-input" } do |f| @@ -6,10 +6,10 @@ = hidden_field_tag(:nav_source, params[:nav_source]) .js-merge-request-new-compare.row{ 'data-source-branch-url': project_new_merge_request_branch_from_path(@source_project), 'data-target-branch-url': project_new_merge_request_branch_to_path(@source_project) } .col-lg-6 - .card.card-new-merge-request - .card-header + .card-new-merge-request + %h2.gl-font-size-h2 Source branch - .card-body.clearfix + .clearfix .merge-request-select.dropdown = f.hidden_field :source_project_id = dropdown_toggle @merge_request.source_project_path, { toggle: "dropdown", 'field-name': "#{f.object_name}[source_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-source-project" } @@ -28,15 +28,15 @@ = dropdown_filter(_("Search branches")) = dropdown_content = dropdown_loading - .card-footer + .gl-bg-gray-50.gl-rounded-base.gl-mx-2.gl-my-4 = gl_loading_icon(css_class: 'js-source-loading gl-my-3') %ul.list-unstyled.mr_source_commit .col-lg-6 - .card.card-new-merge-request - .card-header + .card-new-merge-request + %h2.gl-font-size-h2 Target branch - .card-body.clearfix + .clearfix - projects = target_projects(@project) .merge-request-select.dropdown = f.hidden_field :target_project_id @@ -56,10 +56,10 @@ = dropdown_filter(_("Search branches")) = dropdown_content = dropdown_loading - .card-footer + .gl-bg-gray-50.gl-rounded-base.gl-mx-2.gl-my-4 = gl_loading_icon(css_class: 'js-target-loading gl-my-3') %ul.list-unstyled.mr_target_commit - if @merge_request.errors.any? = form_errors(@merge_request) - = f.submit 'Compare branches and continue', class: "gl-button btn btn-confirm mr-compare-btn", data: { qa_selector: "compare_branches_button" } + = f.submit 'Compare branches and continue', class: "gl-button btn btn-confirm mr-compare-btn gl-mt-4", data: { qa_selector: "compare_branches_button" } diff --git a/app/views/projects/pages_domains/show.html.haml b/app/views/projects/pages_domains/show.html.haml index d16821c3940..215af766e5a 100644 --- a/app/views/projects/pages_domains/show.html.haml +++ b/app/views/projects/pages_domains/show.html.haml @@ -6,9 +6,10 @@ - if verification_enabled && domain_presenter.unverified? = content_for :flash_message do - .gl-alert.gl-alert-warning - .container-fluid.container-limited - = _("This domain is not verified. You will need to verify ownership before access is enabled.") + = render Pajamas::AlertComponent.new(variant: :warning, dismissible: false) do |c| + = c.body do + .container-fluid.container-limited + = _("This domain is not verified. You will need to verify ownership before access is enabled.") %h3.page-title = _('Pages Domain') diff --git a/app/views/projects/settings/repository/show.html.haml b/app/views/projects/settings/repository/show.html.haml index 24fc137fd29..500cfdcb62b 100644 --- a/app/views/projects/settings/repository/show.html.haml +++ b/app/views/projects/settings/repository/show.html.haml @@ -4,6 +4,8 @@ - deploy_token_description = s_('DeployTokens|Deploy tokens allow access to packages, your repository, and registry images.') = render "projects/default_branch/show" +- if Feature.enabled?(:branch_rules, @project) + = render "projects/branch_rules/show" = render_if_exists "projects/push_rules/index" = render "projects/mirrors/mirror_repos" diff --git a/config/feature_flags/development/branch_rules.yml b/config/feature_flags/development/branch_rules.yml new file mode 100644 index 00000000000..822496b48b0 --- /dev/null +++ b/config/feature_flags/development/branch_rules.yml @@ -0,0 +1,8 @@ +--- +name: branch_rules +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/88279 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/363170 +milestone: '15.1' +type: development +group: group::source code +default_enabled: false diff --git a/config/metrics/counts_28d/20210520111133_total.yml b/config/metrics/counts_28d/20210520111133_total.yml index 3da6de21632..cef2766c95a 100644 --- a/config/metrics/counts_28d/20210520111133_total.yml +++ b/config/metrics/counts_28d/20210520111133_total.yml @@ -12,6 +12,7 @@ milestone: "14.0" introduced_by_url: "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61775" time_frame: 28d data_source: database +instrumentation_class: CountImportedProjectsTotalMetric distribution: - ce - ee diff --git a/config/metrics/counts_all/20210514141520_project_imports_total.yml b/config/metrics/counts_all/20210514141520_project_imports_total.yml index 2608714799a..14eac482774 100644 --- a/config/metrics/counts_all/20210514141520_project_imports_total.yml +++ b/config/metrics/counts_all/20210514141520_project_imports_total.yml @@ -12,6 +12,7 @@ milestone: "14.0" introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61775 time_frame: all data_source: database +instrumentation_class: CountImportedProjectsTotalMetric distribution: - ce - ee diff --git a/doc/.vale/gitlab/Uppercase.yml b/doc/.vale/gitlab/Uppercase.yml index bd63d066941..cddbe405c02 100644 --- a/doc/.vale/gitlab/Uppercase.yml +++ b/doc/.vale/gitlab/Uppercase.yml @@ -14,7 +14,6 @@ first: '\b([A-Z]{3,5})\b' second: '(?:\b[A-Z][a-z]+ )+\(([A-Z]{3,5})\)' # ... with the exception of these: exceptions: - - AAAA - AJAX - ANSI - API @@ -30,7 +29,6 @@ exceptions: - CIDR - CLI - CNA - - CNAME - CNCF - CORE - CORS @@ -52,6 +50,7 @@ exceptions: - DSA - DSL - DVCS + - DVD - ECDSA - ECS - EFS @@ -80,6 +79,7 @@ exceptions: - GNU - GPG - GPL + - GPS - GPU - GUI - HAML @@ -107,6 +107,7 @@ exceptions: - JSON - JVM - JWT + - KICS - LAN - LDAP - LDAPS @@ -118,6 +119,7 @@ exceptions: - LTS - MIME - MIT + - MITRE - MVC - NAT - NDA @@ -165,6 +167,7 @@ exceptions: - SAN - SAST - SATA + - SBOM - SCIM - SCP - SCSS @@ -186,7 +189,6 @@ exceptions: - SPF - SQL - SRE - - SRV - SSD - SSG - SSH @@ -205,6 +207,7 @@ exceptions: - TOML - TOTP - TTL + - UBI - UDP - UID - UID diff --git a/doc/administration/pages/index.md b/doc/administration/pages/index.md index 8734775dffc..425a1db27da 100644 --- a/doc/administration/pages/index.md +++ b/doc/administration/pages/index.md @@ -83,7 +83,7 @@ added `gitlab.io` [in 2016](https://gitlab.com/gitlab-com/infrastructure/-/issue ### DNS configuration GitLab Pages expect to run on their own virtual host. In your DNS server/provider -add a [wildcard DNS A record](https://en.wikipedia.org/wiki/Wildcard_DNS_record) pointing to the +add a [wildcard DNS `A` record](https://en.wikipedia.org/wiki/Wildcard_DNS_record) pointing to the host that GitLab runs. For example, an entry would look like this: ```plaintext diff --git a/doc/administration/pages/source.md b/doc/administration/pages/source.md index 6c148387d7d..4bb9fc56b6f 100644 --- a/doc/administration/pages/source.md +++ b/doc/administration/pages/source.md @@ -68,7 +68,7 @@ Before proceeding with the Pages configuration, make sure that: ### DNS configuration GitLab Pages expect to run on their own virtual host. In your DNS server/provider -you need to add a [wildcard DNS A record](https://en.wikipedia.org/wiki/Wildcard_DNS_record) pointing to the +you need to add a [wildcard DNS `A` record](https://en.wikipedia.org/wiki/Wildcard_DNS_record) pointing to the host that GitLab runs. For example, an entry would look like this: ```plaintext diff --git a/doc/administration/postgresql/database_load_balancing.md b/doc/administration/postgresql/database_load_balancing.md index 1324666c32b..3b37ffd2d9d 100644 --- a/doc/administration/postgresql/database_load_balancing.md +++ b/doc/administration/postgresql/database_load_balancing.md @@ -97,9 +97,9 @@ For example, on an environment that has PostgreSQL running on the hosts `host1.e Service discovery allows GitLab to automatically retrieve a list of PostgreSQL hosts to use. It periodically -checks a DNS A record, using the IPs returned by this record as the addresses +checks a DNS `A` record, using the IPs returned by this record as the addresses for the secondaries. For service discovery to work, all you need is a DNS server -and an A record containing the IP addresses of your secondaries. +and an `A` record containing the IP addresses of your secondaries. When using Omnibus GitLab the provided [Consul](../consul.md) service works as a DNS server and returns PostgreSQL addresses via the `postgresql-ha.service.consul` @@ -125,23 +125,23 @@ record. For example: |----------------------|---------------------------------------------------------------------------------------------------|-----------| | `nameserver` | The nameserver to use for looking up the DNS record. | localhost | | `record` | The record to look up. This option is required for service discovery to work. | | -| `record_type` | Optional record type to look up, this can be either A or SRV (GitLab 12.3 and later) | A | +| `record_type` | Optional record type to look up, this can be either `A` or `SRV` (GitLab 12.3 and later) | `A` | | `port` | The port of the nameserver. | 8600 | | `interval` | The minimum time in seconds between checking the DNS record. | 60 | | `disconnect_timeout` | The time in seconds after which an old connection is closed, after the list of hosts was updated. | 120 | | `use_tcp` | Lookup DNS resources using TCP instead of UDP | false | If `record_type` is set to `SRV`, then GitLab continues to use round-robin algorithm -and ignores the `weight` and `priority` in the record. Since SRV records usually +and ignores the `weight` and `priority` in the record. Since `SRV` records usually return hostnames instead of IPs, GitLab needs to look for the IPs of returned hostnames -in the additional section of the SRV response. If no IP is found for a hostname, GitLab -needs to query the configured `nameserver` for ANY record for each such hostname looking for A or AAAA +in the additional section of the `SRV` response. If no IP is found for a hostname, GitLab +needs to query the configured `nameserver` for ANY record for each such hostname looking for `A` or `AAAA` records, eventually dropping this hostname from rotation if it can't resolve its IP. -The `interval` value specifies the _minimum_ time between checks. If the A +The `interval` value specifies the _minimum_ time between checks. If the `A` record has a TTL greater than this value, then service discovery honors said -TTL. For example, if the TTL of the A record is 90 seconds, then service -discovery waits at least 90 seconds before checking the A record again. +TTL. For example, if the TTL of the `A` record is 90 seconds, then service +discovery waits at least 90 seconds before checking the `A` record again. When the list of hosts is updated, it might take a while for the old connections to be terminated. The `disconnect_timeout` setting can be used to enforce an diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 742383e411a..9860abcb8e6 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -13854,6 +13854,7 @@ Represents a milestone. | <a id="milestoneid"></a>`id` | [`ID!`](#id) | ID of the milestone. | | <a id="milestoneiid"></a>`iid` | [`ID!`](#id) | Internal ID of the milestone. | | <a id="milestoneprojectmilestone"></a>`projectMilestone` | [`Boolean!`](#boolean) | Indicates if milestone is at project level. | +| <a id="milestonereleases"></a>`releases` | [`ReleaseConnection`](#releaseconnection) | Releases associated with this milestone. (see [Connections](#connections)) | | <a id="milestonestartdate"></a>`startDate` | [`Time`](#time) | Timestamp of the milestone start date. | | <a id="milestonestate"></a>`state` | [`MilestoneStateEnum!`](#milestonestateenum) | State of the milestone. | | <a id="milestonestats"></a>`stats` | [`MilestoneStats`](#milestonestats) | Milestone statistics. | @@ -15851,6 +15852,7 @@ Represents a release. | <a id="releasedescription"></a>`description` | [`String`](#string) | Description (also known as "release notes") of the release. | | <a id="releasedescriptionhtml"></a>`descriptionHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `description`. | | <a id="releaseevidences"></a>`evidences` | [`ReleaseEvidenceConnection`](#releaseevidenceconnection) | Evidence for the release. (see [Connections](#connections)) | +| <a id="releaseid"></a>`id` | [`ReleaseID!`](#releaseid) | Global ID of the release. | | <a id="releaselinks"></a>`links` | [`ReleaseLinks`](#releaselinks) | Links of the release. | | <a id="releasemilestones"></a>`milestones` | [`MilestoneConnection`](#milestoneconnection) | Milestones associated to the release. (see [Connections](#connections)) | | <a id="releasename"></a>`name` | [`String`](#string) | Name of the release. | @@ -20133,6 +20135,12 @@ A `ProjectID` is a global ID. It is encoded as a string. An example `ProjectID` is: `"gid://gitlab/Project/1"`. +### `ReleaseID` + +A `ReleaseID` is a global ID. It is encoded as a string. + +An example `ReleaseID` is: `"gid://gitlab/Release/1"`. + ### `ReleasesLinkID` A `ReleasesLinkID` is a global ID. It is encoded as a string. diff --git a/doc/topics/autodevops/prepare_deployment.md b/doc/topics/autodevops/prepare_deployment.md index 7c9bf5a770e..22a50df7f54 100644 --- a/doc/topics/autodevops/prepare_deployment.md +++ b/doc/topics/autodevops/prepare_deployment.md @@ -51,7 +51,7 @@ as other environment [variables](../../ci/variables/index.md#cicd-variable-prece If you don't specify the base domain in your projects and groups, Auto DevOps uses the instance-wide **Auto DevOps domain**. -Auto DevOps requires a wildcard DNS A record matching the base domains. For +Auto DevOps requires a wildcard DNS `A` record matching the base domains. For a base domain of `example.com`, you'd need a DNS entry like: ```plaintext diff --git a/doc/user/permissions.md b/doc/user/permissions.md index a141806c813..d429312161e 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -150,6 +150,7 @@ The following table lists project permissions available for each role: | [Projects](project/index.md):<br>View project [Audit Events](../administration/audit_events.md) | | | ✓ (*10*) | ✓ | ✓ | | [Projects](project/index.md):<br>Add [deploy keys](project/deploy_keys/index.md) | | | | ✓ | ✓ | | [Projects](project/index.md):<br>Add new [team members](project/members/index.md) | | | | ✓ | ✓ | +| [Projects](project/index.md):<br>Manage [team members](project/members/index.md) | | | | ✓ (*21*) | ✓ | | [Projects](project/index.md):<br>Change [project features visibility](public_access.md) level | | | | ✓ (*13*) | ✓ | | [Projects](project/index.md):<br>Configure [webhooks](project/integrations/webhooks.md) | | | | ✓ | ✓ | | [Projects](project/index.md):<br>Delete [wiki](project/wiki/index.md) pages | | | ✓ | ✓ | ✓ | @@ -237,6 +238,7 @@ The following table lists project permissions available for each role: 18. Authors and assignees of issues can modify the title and description even if they don't have the Reporter role. 19. Authors and assignees can close and reopen issues even if they don't have the Reporter role. 20. The ability to view the Container Registry and pull images is controlled by the [Container Registry's visibility permissions](packages/container_registry/index.md#container-registry-visibility-permissions). +21. Maintainers cannot create, demote, or remove Owners, and they cannot promote users to the Owner role. <!-- markdownlint-enable MD029 --> diff --git a/doc/user/project/pages/custom_domains_ssl_tls_certification/dns_concepts.md b/doc/user/project/pages/custom_domains_ssl_tls_certification/dns_concepts.md index 5433e02b210..6daf671a751 100644 --- a/doc/user/project/pages/custom_domains_ssl_tls_certification/dns_concepts.md +++ b/doc/user/project/pages/custom_domains_ssl_tls_certification/dns_concepts.md @@ -53,7 +53,7 @@ search the web for `how to add dns record on <my hosting service>`. ## `A` record -A DNS A record maps a host to an IPv4 IP address. +A DNS `A` record maps a host to an IPv4 IP address. It points a root domain as `example.com` to the host's IP address as `192.192.192.192`. @@ -61,10 +61,10 @@ Example: - `example.com` => `A` => `192.192.192.192` -## CNAME record +## `CNAME` record -CNAME records define an alias for canonical name for your server (one defined -by an A record). It points a subdomain to another domain. +`CNAME` records define an alias for canonical name for your server (one defined +by an `A` record). It points a subdomain to another domain. Example: @@ -84,14 +84,14 @@ Example: Then you can register emails for `users@mail.example.com`. -## TXT record +## `TXT` record A `TXT` record can associate arbitrary text with a host or other name. A common use is for site verification. Example: -- `example.com`=> TXT => `"google-site-verification=6P08Ow5E-8Q0m6vQ7FMAqAYIDprkVV8fUf_7hZ4Qvc8"` +- `example.com`=> `TXT` => `"google-site-verification=6P08Ow5E-8Q0m6vQ7FMAqAYIDprkVV8fUf_7hZ4Qvc8"` This way, you can verify the ownership for that domain name. @@ -102,4 +102,4 @@ You can have one DNS record or more than one combined: - `example.com` => `A` => `192.192.192.192` - `www` => `CNAME` => `example.com` - `MX` => `mail.example.com` -- `example.com`=> TXT => `"google-site-verification=6P08Ow5E-8Q0m6vQ7FMAqAYIDprkVV8fUf_7hZ4Qvc8"` +- `example.com`=> `TXT` => `"google-site-verification=6P08Ow5E-8Q0m6vQ7FMAqAYIDprkVV8fUf_7hZ4Qvc8"` diff --git a/doc/user/project/pages/custom_domains_ssl_tls_certification/index.md b/doc/user/project/pages/custom_domains_ssl_tls_certification/index.md index ce35f8c3ebe..2c668e2c409 100644 --- a/doc/user/project/pages/custom_domains_ssl_tls_certification/index.md +++ b/doc/user/project/pages/custom_domains_ssl_tls_certification/index.md @@ -82,20 +82,20 @@ Follow [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/214718) for de Root domains (`example.com`) require: -- A [DNS A record](dns_concepts.md#a-record) pointing your domain to the Pages server. -- A [TXT record](dns_concepts.md#txt-record) to verify your domain's ownership. +- A [DNS `A` record](dns_concepts.md#a-record) pointing your domain to the Pages server. +- A [`TXT` record](dns_concepts.md#txt-record) to verify your domain's ownership. | From | DNS Record | To | | --------------------------------------------- | ---------- | --------------- | -| `example.com` | A | `35.185.44.232` | -| `_gitlab-pages-verification-code.example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | +| `example.com` | `A` | `35.185.44.232` | +| `_gitlab-pages-verification-code.example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | For projects on GitLab.com, this IP is `35.185.44.232`. For projects living in other GitLab instances (CE or EE), please contact your sysadmin asking for this information (which IP address is Pages server running on your instance). - + WARNING: Note that if you use your root domain for your GitLab Pages website @@ -111,7 +111,7 @@ as it most likely doesn't work if you set an [`MX` record](dns_concepts.md#mx-re Subdomains (`subdomain.example.com`) require: - A DNS [`ALIAS` or `CNAME` record](dns_concepts.md#cname-record) pointing your subdomain to the Pages server. -- A DNS [TXT record](dns_concepts.md#txt-record) to verify your domain's ownership. +- A DNS [`TXT` record](dns_concepts.md#txt-record) to verify your domain's ownership. | From | DNS Record | To | |:--------------------------------------------------------|:----------------|:----------------------| @@ -122,7 +122,7 @@ Note that, whether it's a user or a project website, the DNS record should point to your Pages domain (`namespace.gitlab.io`), without any `/project-name`. - + ##### For both root and subdomains @@ -137,11 +137,11 @@ They require: | From | DNS Record | To | | ------------------------------------------------- | ---------- | ---------------------- | -| `example.com` | A | `35.185.44.232` | -| `_gitlab-pages-verification-code.example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | +| `example.com` | `A` | `35.185.44.232` | +| `_gitlab-pages-verification-code.example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | |---------------------------------------------------+------------+------------------------| -| `www.example.com` | CNAME | `namespace.gitlab.io` | -| `_gitlab-pages-verification-code.www.example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | +| `www.example.com` | `CNAME` | `namespace.gitlab.io` | +| `_gitlab-pages-verification-code.www.example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | If you're using Cloudflare, check [Redirecting `www.domain.com` to `domain.com` with Cloudflare](#redirecting-wwwdomaincom-to-domaincom-with-cloudflare). @@ -208,15 +208,15 @@ For a root domain: | From | DNS Record | To | | ------------------------------------------------- | ---------- | ---------------------- | -| `example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | -| `_gitlab-pages-verification-code.example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | +| `example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | +| `_gitlab-pages-verification-code.example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | For a subdomain: | From | DNS Record | To | | ------------------------------------------------- | ---------- | ---------------------- | -| `www.example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | -| `_gitlab-pages-verification-code.www.example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | +| `www.example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | +| `_gitlab-pages-verification-code.www.example.com` | `TXT` | `gitlab-pages-verification-code=00112233445566778899aabbccddeeff` | ### Adding more domain aliases diff --git a/lib/api/ci/job_artifacts.rb b/lib/api/ci/job_artifacts.rb index 0800993602b..657ceb44596 100644 --- a/lib/api/ci/job_artifacts.rb +++ b/lib/api/ci/job_artifacts.rb @@ -3,6 +3,8 @@ module API module Ci class JobArtifacts < ::API::Base + helpers ::API::Helpers::ProjectStatsRefreshConflictsHelpers + before { authenticate_non_get! } feature_category :build_artifacts @@ -137,6 +139,8 @@ module API build = find_build!(params[:job_id]) authorize!(:destroy_artifacts, build) + reject_if_build_artifacts_size_refreshing!(build.project) + build.erase_erasable_artifacts! status :no_content @@ -146,6 +150,8 @@ module API delete ':id/artifacts' do authorize_destroy_artifacts! + reject_if_build_artifacts_size_refreshing!(user_project) + ::Ci::JobArtifacts::DeleteProjectArtifactsService.new(project: user_project).execute accepted! diff --git a/lib/api/ci/jobs.rb b/lib/api/ci/jobs.rb index 04999b5fb44..97e476f2c00 100644 --- a/lib/api/ci/jobs.rb +++ b/lib/api/ci/jobs.rb @@ -4,6 +4,9 @@ module API module Ci class Jobs < ::API::Base include PaginationParams + + helpers ::API::Helpers::ProjectStatsRefreshConflictsHelpers + before { authenticate! } resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do @@ -137,6 +140,8 @@ module API authorize!(:erase_build, build) break forbidden!('Job is not erasable!') unless build.erasable? + reject_if_build_artifacts_size_refreshing!(build.project) + build.erase(erased_by: current_user) present build, with: Entities::Ci::Job end diff --git a/lib/api/ci/pipelines.rb b/lib/api/ci/pipelines.rb index 4253a9eb4d7..cd686a28dd2 100644 --- a/lib/api/ci/pipelines.rb +++ b/lib/api/ci/pipelines.rb @@ -5,6 +5,8 @@ module API class Pipelines < ::API::Base include PaginationParams + helpers ::API::Helpers::ProjectStatsRefreshConflictsHelpers + before { authenticate_non_get! } params do @@ -208,6 +210,8 @@ module API delete ':id/pipelines/:pipeline_id', urgency: :low, feature_category: :continuous_integration do authorize! :destroy_pipeline, pipeline + reject_if_build_artifacts_size_refreshing!(pipeline.project) + destroy_conditionally!(pipeline) do ::Ci::DestroyPipelineService.new(user_project, current_user).execute(pipeline) end diff --git a/lib/api/helpers/project_stats_refresh_conflicts_helpers.rb b/lib/api/helpers/project_stats_refresh_conflicts_helpers.rb new file mode 100644 index 00000000000..db464521033 --- /dev/null +++ b/lib/api/helpers/project_stats_refresh_conflicts_helpers.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module API + module Helpers + module ProjectStatsRefreshConflictsHelpers + def reject_if_build_artifacts_size_refreshing!(project) + return unless project.refreshing_build_artifacts_size? + + Gitlab::ProjectStatsRefreshConflictsLogger.warn_request_rejected_during_stats_refresh(project.id) + + conflict!('Action temporarily disabled. The project this pipeline belongs to is undergoing stats refresh.') + end + end + end +end diff --git a/lib/gitlab/background_migration/fix_merge_request_diff_commit_users.rb b/lib/gitlab/background_migration/fix_merge_request_diff_commit_users.rb index ea3e56cb14a..4df55a7b02a 100644 --- a/lib/gitlab/background_migration/fix_merge_request_diff_commit_users.rb +++ b/lib/gitlab/background_migration/fix_merge_request_diff_commit_users.rb @@ -5,12 +5,6 @@ module Gitlab # Background migration for fixing merge_request_diff_commit rows that don't # have committer/author details due to # https://gitlab.com/gitlab-org/gitlab/-/issues/344080. - # - # This migration acts on a single project and corrects its data. Because - # this process needs Git/Gitaly access, and duplicating all that code is far - # too much, this migration relies on global models such as Project, - # MergeRequest, etc. - # rubocop: disable Metrics/ClassLength class FixMergeRequestDiffCommitUsers BATCH_SIZE = 100 @@ -20,137 +14,8 @@ module Gitlab end def perform(project_id) - if (project = ::Project.find_by_id(project_id)) - process(project) - end - - ::Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded( - 'FixMergeRequestDiffCommitUsers', - [project_id] - ) - - schedule_next_job - end - - def process(project) - # Loading everything using one big query may result in timeouts (e.g. - # for projects the size of gitlab-org/gitlab). So instead we query - # data on a per merge request basis. - project.merge_requests.each_batch(column: :iid) do |mrs| - mrs.ids.each do |mr_id| - each_row_to_check(mr_id) do |commit| - update_commit(project, commit) - end - end - end - end - - def each_row_to_check(merge_request_id, &block) - columns = %w[merge_request_diff_id relative_order].map do |col| - Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: col, - order_expression: MergeRequestDiffCommit.arel_table[col.to_sym].asc, - nullable: :not_nullable, - distinct: false - ) - end - - order = Pagination::Keyset::Order.build(columns) - scope = MergeRequestDiffCommit - .joins(:merge_request_diff) - .where(merge_request_diffs: { merge_request_id: merge_request_id }) - .where('commit_author_id IS NULL OR committer_id IS NULL') - .order(order) - - Pagination::Keyset::Iterator - .new(scope: scope, use_union_optimization: true) - .each_batch(of: BATCH_SIZE) do |rows| - rows - .select([ - :merge_request_diff_id, - :relative_order, - :sha, - :committer_id, - :commit_author_id - ]) - .each(&block) - end - end - - # rubocop: disable Metrics/AbcSize - def update_commit(project, row) - commit = find_commit(project, row.sha) - updates = [] - - unless row.commit_author_id - author_id = find_or_create_user(commit, :author_name, :author_email) - - updates << [arel_table[:commit_author_id], author_id] if author_id - end - - unless row.committer_id - committer_id = - find_or_create_user(commit, :committer_name, :committer_email) - - updates << [arel_table[:committer_id], committer_id] if committer_id - end - - return if updates.empty? - - update = Arel::UpdateManager - .new - .table(MergeRequestDiffCommit.arel_table) - .where(matches_row(row)) - .set(updates) - .to_sql - - MergeRequestDiffCommit.connection.execute(update) - end - # rubocop: enable Metrics/AbcSize - - def schedule_next_job - job = Database::BackgroundMigrationJob - .for_migration_class('FixMergeRequestDiffCommitUsers') - .pending - .first - - return unless job - - BackgroundMigrationWorker.perform_in( - 2.minutes, - 'FixMergeRequestDiffCommitUsers', - job.arguments - ) - end - - def find_commit(project, sha) - @commits[sha] ||= (project.commit(sha)&.to_hash || {}) - end - - def find_or_create_user(commit, name_field, email_field) - name = commit[name_field] - email = commit[email_field] - - return unless name && email - - @users[[name, email]] ||= - MergeRequest::DiffCommitUser.find_or_create(name, email).id - end - - def matches_row(row) - primary_key = Arel::Nodes::Grouping - .new([arel_table[:merge_request_diff_id], arel_table[:relative_order]]) - - primary_val = Arel::Nodes::Grouping - .new([row.merge_request_diff_id, row.relative_order]) - - primary_key.eq(primary_val) - end - - def arel_table - MergeRequestDiffCommit.arel_table + # No-op, see https://gitlab.com/gitlab-org/gitlab/-/issues/344540 end end - # rubocop: enable Metrics/ClassLength end end diff --git a/lib/gitlab/graphql/authorize/authorize_resource.rb b/lib/gitlab/graphql/authorize/authorize_resource.rb index dc49c806398..884fc85c4ec 100644 --- a/lib/gitlab/graphql/authorize/authorize_resource.rb +++ b/lib/gitlab/graphql/authorize/authorize_resource.rb @@ -15,11 +15,7 @@ module Gitlab # If the `#authorize` call is used on multiple classes, we add the # permissions specified on a subclass, to the ones that were specified # on its superclass. - @required_permissions ||= if respond_to?(:superclass) && superclass.respond_to?(:required_permissions) - superclass.required_permissions.dup - else - [] - end + @required_permissions ||= call_superclass_method(:required_permissions, []).dup end def authorize(*permissions) @@ -27,6 +23,8 @@ module Gitlab end def authorizes_object? + return true if call_superclass_method(:authorizes_object?, false) + defined?(@authorizes_object) ? @authorizes_object : false end @@ -37,6 +35,14 @@ module Gitlab def raise_resource_not_available_error!(msg = RESOURCE_ACCESS_ERROR) raise ::Gitlab::Graphql::Errors::ResourceNotAvailable, msg end + + private + + def call_superclass_method(method_name, or_else) + return or_else unless respond_to?(:superclass) && superclass.respond_to?(method_name) + + superclass.send(method_name) # rubocop: disable GitlabSecurity/PublicSend + end end def find_object(*args) diff --git a/lib/gitlab/graphql/loaders/batch_model_loader.rb b/lib/gitlab/graphql/loaders/batch_model_loader.rb index 805864cdd4c..41c3af33909 100644 --- a/lib/gitlab/graphql/loaders/batch_model_loader.rb +++ b/lib/gitlab/graphql/loaders/batch_model_loader.rb @@ -4,20 +4,27 @@ module Gitlab module Graphql module Loaders class BatchModelLoader - attr_reader :model_class, :model_id + attr_reader :model_class, :model_id, :preloads - def initialize(model_class, model_id) + def initialize(model_class, model_id, preloads = nil) @model_class = model_class @model_id = model_id + @preloads = preloads || [] end # rubocop: disable CodeReuse/ActiveRecord def find - BatchLoader::GraphQL.for(model_id.to_i).batch(key: model_class) do |ids, loader, args| + BatchLoader::GraphQL.for([model_id.to_i, preloads]).batch(key: model_class) do |for_params, loader, args| model = args[:key] + keys_by_id = for_params.group_by(&:first) + ids = for_params.map(&:first) + preloads = for_params.flat_map(&:second).uniq results = model.where(id: ids) + results = results.preload(*preloads) unless preloads.empty? - results.each { |record| loader.call(record.id, record) } + results.each do |record| + keys_by_id.fetch(record.id, []).each { |k| loader.call(k, record) } + end end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/gitlab/project_stats_refresh_conflicts_logger.rb b/lib/gitlab/project_stats_refresh_conflicts_logger.rb index e0e71507577..49f5a544a87 100644 --- a/lib/gitlab/project_stats_refresh_conflicts_logger.rb +++ b/lib/gitlab/project_stats_refresh_conflicts_logger.rb @@ -11,5 +11,14 @@ module Gitlab Gitlab::AppLogger.warn(payload) end + + def self.warn_request_rejected_during_stats_refresh(project_id) + payload = Gitlab::ApplicationContext.current.merge( + message: 'Rejected request due to project undergoing stats refresh', + project_id: project_id + ) + + Gitlab::AppLogger.warn(payload) + end end end diff --git a/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_total_metric.rb b/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_total_metric.rb new file mode 100644 index 00000000000..109d2245635 --- /dev/null +++ b/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_total_metric.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Gitlab + module Usage + module Metrics + module Instrumentations + class CountImportedProjectsTotalMetric < DatabaseMetric + # Relation and operation are not used, but are included to satisfy expectations + # of other metric generation logic. + relation { Project } + operation :count + + IMPORT_TYPES = %w(gitlab_project gitlab github bitbucket bitbucket_server gitea git manifest + gitlab_migration).freeze + + def value + count(project_relation) + count(entity_relation) + end + + def to_sql + project_relation_sql = Gitlab::Usage::Metrics::Query.for(:count, project_relation) + entity_relation_sql = Gitlab::Usage::Metrics::Query.for(:count, entity_relation) + + "SELECT (#{project_relation_sql}) + (#{entity_relation_sql})" + end + + private + + def project_relation + Project.imported_from(IMPORT_TYPES).where(time_constraints) + end + + def entity_relation + BulkImports::Entity.where(source_type: :project_entity).where(time_constraints) + end + end + end + end + end +end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 208d7c327c3..9d312b3b2fe 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -877,7 +877,7 @@ module Gitlab gitlab_migration: add_metric('CountBulkImportsEntitiesMetric', time_frame: time_frame, options: { source_type: :project_entity }) } - counters[:total] = add(*counters.values) + counters[:total] = add_metric('CountImportedProjectsTotalMetric', time_frame: time_frame) counters end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 460e54d17b5..e79d28bc1c2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6490,6 +6490,9 @@ msgstr "" msgid "Branch not loaded - %{branchId}" msgstr "" +msgid "Branch rules" +msgstr "" + msgid "Branches" msgstr "" @@ -11944,6 +11947,9 @@ msgstr "" msgid "Define how approval rules are applied to merge requests." msgstr "" +msgid "Define rules for who can push, merge, and the required approvals for each branch." +msgstr "" + msgid "Definition" msgstr "" @@ -14560,9 +14566,6 @@ msgstr "" msgid "Epics|Assign Epic" msgstr "" -msgid "Epics|Enter a title for your epic" -msgstr "" - msgid "Epics|Leave empty to inherit from milestone dates" msgstr "" @@ -39388,6 +39391,9 @@ msgstr "" msgid "Title" msgstr "" +msgid "Title (required)" +msgstr "" + msgid "Title:" msgstr "" diff --git a/qa/qa/runtime/namespace.rb b/qa/qa/runtime/namespace.rb index 6b4cbe6af6e..36726080294 100644 --- a/qa/qa/runtime/namespace.rb +++ b/qa/qa/runtime/namespace.rb @@ -25,7 +25,7 @@ module QA end def sandbox_name - Runtime::Env.sandbox_name || 'gitlab-qa-sandbox-group' + @sandbox_name ||= Runtime::Env.sandbox_name || "gitlab-qa-sandbox-group-#{Time.now.wday}" end end end diff --git a/spec/controllers/help_controller_spec.rb b/spec/controllers/help_controller_spec.rb index 70dc710f604..26e65711e9f 100644 --- a/spec/controllers/help_controller_spec.rb +++ b/spec/controllers/help_controller_spec.rb @@ -4,34 +4,35 @@ require 'spec_helper' RSpec.describe HelpController do include StubVersion + include DocUrlHelper let(:user) { create(:user) } shared_examples 'documentation pages local render' do it 'renders HTML' do aggregate_failures do - is_expected.to render_template('show.html.haml') + is_expected.to render_template('help/show') expect(response.media_type).to eq 'text/html' end end end shared_examples 'documentation pages redirect' do |documentation_base_url| - let(:gitlab_version) { '13.4.0-ee' } + let(:gitlab_version) { version } before do stub_version(gitlab_version, 'ignored_revision_value') end it 'redirects user to custom documentation url with a specified version' do - is_expected.to redirect_to("#{documentation_base_url}/13.4/ee/#{path}.html") + is_expected.to redirect_to(doc_url(documentation_base_url)) end context 'when it is a pre-release' do let(:gitlab_version) { '13.4.0-pre' } it 'redirects user to custom documentation url without a version' do - is_expected.to redirect_to("#{documentation_base_url}/ee/#{path}.html") + is_expected.to redirect_to(doc_url_without_version(documentation_base_url)) end end end @@ -43,7 +44,7 @@ RSpec.describe HelpController do describe 'GET #index' do context 'with absolute url' do it 'keeps the URL absolute' do - stub_readme("[API](/api/README.md)") + stub_doc_file_read(content: "[API](/api/README.md)") get :index @@ -53,7 +54,7 @@ RSpec.describe HelpController do context 'with relative url' do it 'prefixes it with /help/' do - stub_readme("[API](api/README.md)") + stub_doc_file_read(content: "[API](api/README.md)") get :index @@ -63,7 +64,7 @@ RSpec.describe HelpController do context 'when url is an external link' do it 'does not change it' do - stub_readme("[external](https://some.external.link)") + stub_doc_file_read(content: "[external](https://some.external.link)") get :index @@ -73,7 +74,7 @@ RSpec.describe HelpController do context 'when relative url with external on same line' do it 'prefix it with /help/' do - stub_readme("[API](api/README.md) [external](https://some.external.link)") + stub_doc_file_read(content: "[API](api/README.md) [external](https://some.external.link)") get :index @@ -83,7 +84,7 @@ RSpec.describe HelpController do context 'when relative url with http:// in query' do it 'prefix it with /help/' do - stub_readme("[API](api/README.md?go=https://example.com/)") + stub_doc_file_read(content: "[API](api/README.md?go=https://example.com/)") get :index @@ -93,7 +94,7 @@ RSpec.describe HelpController do context 'when mailto URL' do it 'do not change it' do - stub_readme("[report bug](mailto:bugs@example.com)") + stub_doc_file_read(content: "[report bug](mailto:bugs@example.com)") get :index @@ -103,7 +104,7 @@ RSpec.describe HelpController do context 'when protocol-relative link' do it 'do not change it' do - stub_readme("[protocol-relative](//example.com)") + stub_doc_file_read(content: "[protocol-relative](//example.com)") get :index @@ -146,7 +147,7 @@ RSpec.describe HelpController do context 'when requested file exists' do before do - expect_file_read(File.join(Rails.root, 'doc/user/ssh.md'), content: fixture_file('blockquote_fence_after.md')) + stub_doc_file_read(file_name: 'user/ssh.md', content: fixture_file('blockquote_fence_after.md')) subject end @@ -265,10 +266,6 @@ RSpec.describe HelpController do end end - def stub_readme(content) - expect_file_read(Rails.root.join('doc', 'index.md'), content: content) - end - def stub_two_factor_required allow(controller).to receive(:two_factor_authentication_required?).and_return(true) allow(controller).to receive(:current_user_requires_two_factor?).and_return(true) diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 162c36f5069..5aafddd94da 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -1075,63 +1075,81 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do before do project.add_role(user, role) sign_in(user) - - post_erase end - shared_examples_for 'erases' do - it 'redirects to the erased job page' do - expect(response).to have_gitlab_http_status(:found) - expect(response).to redirect_to(namespace_project_job_path(id: job.id)) + context 'when project is not undergoing stats refresh' do + before do + post_erase end - it 'erases artifacts' do - expect(job.artifacts_file.present?).to be_falsey - expect(job.artifacts_metadata.present?).to be_falsey - end + shared_examples_for 'erases' do + it 'redirects to the erased job page' do + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(namespace_project_job_path(id: job.id)) + end - it 'erases trace' do - expect(job.trace.exist?).to be_falsey + it 'erases artifacts' do + expect(job.artifacts_file.present?).to be_falsey + expect(job.artifacts_metadata.present?).to be_falsey + end + + it 'erases trace' do + expect(job.trace.exist?).to be_falsey + end end - end - context 'when job is successful and has artifacts' do - let(:job) { create(:ci_build, :erasable, :trace_artifact, pipeline: pipeline) } + context 'when job is successful and has artifacts' do + let(:job) { create(:ci_build, :erasable, :trace_artifact, pipeline: pipeline) } - it_behaves_like 'erases' - end + it_behaves_like 'erases' + end - context 'when job has live trace and unarchived artifact' do - let(:job) { create(:ci_build, :success, :trace_live, :unarchived_trace_artifact, pipeline: pipeline) } + context 'when job has live trace and unarchived artifact' do + let(:job) { create(:ci_build, :success, :trace_live, :unarchived_trace_artifact, pipeline: pipeline) } - it_behaves_like 'erases' - end + it_behaves_like 'erases' + end - context 'when job is erased' do - let(:job) { create(:ci_build, :erased, pipeline: pipeline) } + context 'when job is erased' do + let(:job) { create(:ci_build, :erased, pipeline: pipeline) } - it 'returns unprocessable_entity' do - expect(response).to have_gitlab_http_status(:unprocessable_entity) + it 'returns unprocessable_entity' do + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end end - end - context 'when user is developer' do - let(:role) { :developer } - let(:job) { create(:ci_build, :erasable, :trace_artifact, pipeline: pipeline, user: triggered_by) } + context 'when user is developer' do + let(:role) { :developer } + let(:job) { create(:ci_build, :erasable, :trace_artifact, pipeline: pipeline, user: triggered_by) } - context 'when triggered by same user' do - let(:triggered_by) { user } + context 'when triggered by same user' do + let(:triggered_by) { user } - it 'has successful status' do - expect(response).to have_gitlab_http_status(:found) + it 'has successful status' do + expect(response).to have_gitlab_http_status(:found) + end + end + + context 'when triggered by different user' do + let(:triggered_by) { create(:user) } + + it 'does not have successful status' do + expect(response).not_to have_gitlab_http_status(:found) + end end end + end + + context 'when project is undergoing stats refresh' do + it_behaves_like 'preventing request because of ongoing project stats refresh' do + let(:job) { create(:ci_build, :erasable, :trace_artifact, pipeline: pipeline) } + let(:make_request) { post_erase } - context 'when triggered by different user' do - let(:triggered_by) { create(:user) } + it 'does not erase artifacts' do + make_request - it 'does not have successful status' do - expect(response).not_to have_gitlab_http_status(:found) + expect(job.artifacts_file).to be_present + expect(job.artifacts_metadata).to be_present end end end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 1be4177acd1..b3b803649d1 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -1289,6 +1289,18 @@ RSpec.describe Projects::PipelinesController do expect(response).to have_gitlab_http_status(:not_found) end end + + context 'and project is undergoing stats refresh' do + it_behaves_like 'preventing request because of ongoing project stats refresh' do + let(:make_request) { delete_pipeline } + + it 'does not delete the pipeline' do + make_request + + expect(Ci::Pipeline.exists?(pipeline.id)).to be_truthy + end + end + end end context 'when user has no privileges' do diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 20a114bbe8c..9bb34a38005 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -170,6 +170,46 @@ RSpec.describe Projects::ProjectMembersController do expect(requester.reload.human_access).to eq(label) end end + + describe 'managing project direct owners' do + context 'when a Maintainer tries to elevate another user to OWNER' do + it 'does not allow the operation' do + params = { + project_member: { access_level: Gitlab::Access::OWNER }, + namespace_id: project.namespace, + project_id: project, + id: requester + } + + put :update, params: params, xhr: true + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when a user with OWNER access tries to elevate another user to OWNER' do + # inherited owner role via personal project association + let(:user) { project.first_owner } + + before do + sign_in(user) + end + + it 'returns success' do + params = { + project_member: { access_level: Gitlab::Access::OWNER }, + namespace_id: project.namespace, + project_id: project, + id: requester + } + + put :update, params: params, xhr: true + + expect(response).to have_gitlab_http_status(:ok) + expect(requester.reload.access_level).to eq(Gitlab::Access::OWNER) + end + end + end end context 'access expiry date' do @@ -275,19 +315,40 @@ RSpec.describe Projects::ProjectMembersController do context 'when member is found' do context 'when user does not have enough rights' do - before do - project.add_developer(user) + context 'when user does not have rights to manage other members' do + before do + project.add_developer(user) + end + + it 'returns 404', :aggregate_failures do + delete :destroy, params: { + namespace_id: project.namespace, + project_id: project, + id: member + } + + expect(response).to have_gitlab_http_status(:not_found) + expect(project.members).to include member + end end - it 'returns 404', :aggregate_failures do - delete :destroy, params: { - namespace_id: project.namespace, - project_id: project, - id: member - } + context 'when user does not have rights to manage Owner members' do + let_it_be(:member) { create(:project_member, project: project, access_level: Gitlab::Access::OWNER) } - expect(response).to have_gitlab_http_status(:not_found) - expect(project.members).to include member + before do + project.add_maintainer(user) + end + + it 'returns 403', :aggregate_failures do + delete :destroy, params: { + namespace_id: project.namespace, + project_id: project, + id: member + } + + expect(response).to have_gitlab_http_status(:forbidden) + expect(project.members).to include member + end end end @@ -434,7 +495,7 @@ RSpec.describe Projects::ProjectMembersController do end context 'when member is found' do - context 'when user does not have enough rights' do + context 'when user does not have rights to manage other members' do before do project.add_developer(user) end diff --git a/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb index 4e1b55d3d70..cfdd3d9224d 100644 --- a/spec/features/projects/settings/repository_settings_spec.rb +++ b/spec/features/projects/settings/repository_settings_spec.rb @@ -39,6 +39,22 @@ RSpec.describe 'Projects > Settings > Repository settings' do end end + context 'Branch rules', :js do + it 'renders branch rules settings' do + visit project_settings_repository_path(project) + expect(page).to have_content('Branch rules') + end + + context 'branch_rules feature flag disabled', :js do + it 'does not render branch rules settings' do + stub_feature_flags(branch_rules: false) + visit project_settings_repository_path(project) + + expect(page).not_to have_content('Branch rules') + end + end + end + context 'Deploy Keys', :js do let_it_be(:private_deploy_key) { create(:deploy_key, title: 'private_deploy_key', public: false) } let_it_be(:public_deploy_key) { create(:another_deploy_key, title: 'public_deploy_key', public: true) } diff --git a/spec/frontend/analytics/usage_trends/components/usage_counts_spec.js b/spec/frontend/analytics/usage_trends/components/usage_counts_spec.js index 703767dab47..f4cbc56be5c 100644 --- a/spec/frontend/analytics/usage_trends/components/usage_counts_spec.js +++ b/spec/frontend/analytics/usage_trends/components/usage_counts_spec.js @@ -1,4 +1,4 @@ -import { GlDeprecatedSkeletonLoading as GlSkeletonLoading } from '@gitlab/ui'; +import { GlSkeletonLoader } from '@gitlab/ui'; import { GlSingleStat } from '@gitlab/ui/dist/charts'; import { shallowMount } from '@vue/test-utils'; import UsageCounts from '~/analytics/usage_trends/components/usage_counts.vue'; @@ -30,7 +30,7 @@ describe('UsageCounts', () => { wrapper.destroy(); }); - const findSkeletonLoader = () => wrapper.findComponent(GlSkeletonLoading); + const findSkeletonLoader = () => wrapper.findComponent(GlSkeletonLoader); const findAllSingleStats = () => wrapper.findAllComponents(GlSingleStat); describe('while loading', () => { diff --git a/spec/frontend/editor/source_editor_markdown_livepreview_ext_spec.js b/spec/frontend/editor/source_editor_markdown_livepreview_ext_spec.js index 1926f3e268e..fe20c23e4d7 100644 --- a/spec/frontend/editor/source_editor_markdown_livepreview_ext_spec.js +++ b/spec/frontend/editor/source_editor_markdown_livepreview_ext_spec.js @@ -1,4 +1,6 @@ import MockAdapter from 'axios-mock-adapter'; +import { Emitter } from 'monaco-editor'; +import { useFakeRequestAnimationFrame } from 'helpers/fake_request_animation_frame'; import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; import waitForPromises from 'helpers/wait_for_promises'; import { @@ -64,7 +66,6 @@ describe('Markdown Live Preview Extension for Source Editor', () => { afterEach(() => { instance.dispose(); - editorEl.remove(); mockAxios.restore(); resetHTMLFixture(); }); @@ -75,11 +76,47 @@ describe('Markdown Live Preview Extension for Source Editor', () => { actions: expect.any(Object), shown: false, modelChangeListener: undefined, + layoutChangeListener: { + dispose: expect.anything(), + }, path: previewMarkdownPath, actionShowPreviewCondition: expect.any(Object), }); }); + describe('onDidLayoutChange', () => { + const emitter = new Emitter(); + let layoutSpy; + + useFakeRequestAnimationFrame(); + + beforeEach(() => { + instance.unuse(extension); + instance.onDidLayoutChange = emitter.event; + extension = instance.use({ + definition: EditorMarkdownPreviewExtension, + setupOptions: { previewMarkdownPath }, + }); + layoutSpy = jest.spyOn(instance, 'layout'); + }); + + it('does not trigger the layout when the preview is not active [default]', async () => { + expect(instance.markdownPreview.shown).toBe(false); + expect(layoutSpy).not.toHaveBeenCalled(); + await emitter.fire(); + expect(layoutSpy).not.toHaveBeenCalled(); + }); + + it('triggers the layout if the preview panel is opened', async () => { + expect(layoutSpy).not.toHaveBeenCalled(); + instance.togglePreview(); + layoutSpy.mockReset(); + + await emitter.fire(); + expect(layoutSpy).toHaveBeenCalledTimes(1); + }); + }); + describe('model change listener', () => { let cleanupSpy; let actionSpy; @@ -111,6 +148,9 @@ describe('Markdown Live Preview Extension for Source Editor', () => { mockAxios.onPost().reply(200, { body: responseData }); await togglePreview(); }); + afterEach(() => { + jest.clearAllMocks(); + }); it('removes the registered buttons from the toolbar', () => { expect(instance.toolbar.removeItems).not.toHaveBeenCalled(); @@ -175,6 +215,31 @@ describe('Markdown Live Preview Extension for Source Editor', () => { instance.unuse(extension); expect(newWidth === width / EXTENSION_MARKDOWN_PREVIEW_PANEL_WIDTH).toBe(true); }); + + it('disposes the layoutChange listener and does not re-layout on layout changes', () => { + expect(instance.markdownPreview.layoutChangeListener).toBeDefined(); + instance.unuse(extension); + + expect(instance.markdownPreview?.layoutChangeListener).toBeUndefined(); + }); + + it('does not trigger the re-layout after instance is unused', async () => { + const emitter = new Emitter(); + + instance.unuse(extension); + instance.onDidLayoutChange = emitter.event; + + // we have to re-use the extension to pick up the emitter + extension = instance.use({ + definition: EditorMarkdownPreviewExtension, + setupOptions: { previewMarkdownPath }, + }); + instance.unuse(extension); + const layoutSpy = jest.spyOn(instance, 'layout'); + + await emitter.fire(); + expect(layoutSpy).not.toHaveBeenCalled(); + }); }); describe('fetchPreview', () => { diff --git a/spec/frontend/editor/source_editor_webide_ext_spec.js b/spec/frontend/editor/source_editor_webide_ext_spec.js new file mode 100644 index 00000000000..096b6b1646f --- /dev/null +++ b/spec/frontend/editor/source_editor_webide_ext_spec.js @@ -0,0 +1,55 @@ +import { Emitter } from 'monaco-editor'; +import { setHTMLFixture } from 'helpers/fixtures'; +import { EditorWebIdeExtension } from '~/editor/extensions/source_editor_webide_ext'; +import SourceEditor from '~/editor/source_editor'; + +describe('Source Editor Web IDE Extension', () => { + let editorEl; + let editor; + let instance; + + beforeEach(() => { + setHTMLFixture('<div id="editor" data-editor-loading></div>'); + editorEl = document.getElementById('editor'); + editor = new SourceEditor(); + }); + afterEach(() => {}); + + describe('onSetup', () => { + it.each` + width | renderSideBySide + ${'0'} | ${false} + ${'699px'} | ${false} + ${'700px'} | ${true} + `( + "correctly renders the Diff Editor when the parent element's width is $width", + ({ width, renderSideBySide }) => { + editorEl.style.width = width; + instance = editor.createDiffInstance({ el: editorEl }); + + const sideBySideSpy = jest.spyOn(instance, 'updateOptions'); + instance.use({ definition: EditorWebIdeExtension }); + + expect(sideBySideSpy).toBeCalledWith({ renderSideBySide }); + }, + ); + + it('re-renders the Diff Editor when layout of the modified editor is changed', async () => { + const emitter = new Emitter(); + editorEl.style.width = '700px'; + + instance = editor.createDiffInstance({ el: editorEl }); + instance.getModifiedEditor().onDidLayoutChange = emitter.event; + instance.use({ definition: EditorWebIdeExtension }); + + const sideBySideSpy = jest.spyOn(instance, 'updateOptions'); + await emitter.fire(); + + expect(sideBySideSpy).toBeCalledWith({ renderSideBySide: true }); + + editorEl.style.width = '0px'; + await emitter.fire(); + expect(sideBySideSpy).toBeCalledWith({ renderSideBySide: false }); + }); + }); +}); diff --git a/spec/frontend/ide/components/repo_editor_spec.js b/spec/frontend/ide/components/repo_editor_spec.js index 9a30fd5f5c3..b48372afdea 100644 --- a/spec/frontend/ide/components/repo_editor_spec.js +++ b/spec/frontend/ide/components/repo_editor_spec.js @@ -11,19 +11,13 @@ import { EditorMarkdownExtension } from '~/editor/extensions/source_editor_markd import { EditorMarkdownPreviewExtension } from '~/editor/extensions/source_editor_markdown_livepreview_ext'; import SourceEditor from '~/editor/source_editor'; import RepoEditor from '~/ide/components/repo_editor.vue'; -import { - leftSidebarViews, - FILE_VIEW_MODE_EDITOR, - FILE_VIEW_MODE_PREVIEW, - viewerTypes, -} from '~/ide/constants'; +import { leftSidebarViews, FILE_VIEW_MODE_PREVIEW, viewerTypes } from '~/ide/constants'; import ModelManager from '~/ide/lib/common/model_manager'; import service from '~/ide/services'; import { createStoreOptions } from '~/ide/stores'; import axios from '~/lib/utils/axios_utils'; import ContentViewer from '~/vue_shared/components/content_viewer/content_viewer.vue'; import SourceEditorInstance from '~/editor/source_editor_instance'; -import { spyOnApi } from 'jest/editor/helpers'; import { file } from '../helpers'; const PREVIEW_MARKDOWN_PATH = '/foo/bar/preview_markdown'; @@ -196,11 +190,8 @@ describe('RepoEditor', () => { }); describe('when files is markdown', () => { - let layoutSpy; - beforeEach(async () => { await createComponent({ activeFile }); - layoutSpy = jest.spyOn(wrapper.vm.editor, 'layout'); }); it('renders an Edit and a Preview Tab', () => { @@ -217,10 +208,6 @@ describe('RepoEditor', () => { expect(wrapper.find(ContentViewer).html()).toContain(defaultFileProps.content); }); - it('should not trigger layout', async () => { - expect(layoutSpy).not.toHaveBeenCalled(); - }); - describe('when file changes to non-markdown file', () => { beforeEach(async () => { wrapper.setProps({ file: dummyFile.empty }); @@ -229,10 +216,6 @@ describe('RepoEditor', () => { it('should hide tabs', () => { expect(findTabs()).toHaveLength(0); }); - - it('should trigger refresh dimensions', async () => { - expect(layoutSpy).toHaveBeenCalledTimes(1); - }); }); }); @@ -373,53 +356,6 @@ describe('RepoEditor', () => { }); }); - describe('editor updateDimensions', () => { - let updateDimensionsSpy; - beforeEach(async () => { - await createComponent(); - const ext = extensionsStore.get('EditorWebIde'); - updateDimensionsSpy = jest.fn(); - spyOnApi(ext, { - updateDimensions: updateDimensionsSpy, - }); - }); - - it('calls updateDimensions only when panelResizing is false', async () => { - expect(updateDimensionsSpy).not.toHaveBeenCalled(); - expect(vm.$store.state.panelResizing).toBe(false); // default value - - vm.$store.state.panelResizing = true; - await nextTick(); - - expect(updateDimensionsSpy).not.toHaveBeenCalled(); - - vm.$store.state.panelResizing = false; - await nextTick(); - - expect(updateDimensionsSpy).toHaveBeenCalledTimes(1); - - vm.$store.state.panelResizing = true; - await nextTick(); - - expect(updateDimensionsSpy).toHaveBeenCalledTimes(1); - }); - - it('calls updateDimensions when rightPane is toggled', async () => { - expect(updateDimensionsSpy).not.toHaveBeenCalled(); - expect(vm.$store.state.rightPane.isOpen).toBe(false); // default value - - vm.$store.state.rightPane.isOpen = true; - await nextTick(); - - expect(updateDimensionsSpy).toHaveBeenCalledTimes(1); - - vm.$store.state.rightPane.isOpen = false; - await nextTick(); - - expect(updateDimensionsSpy).toHaveBeenCalledTimes(2); - }); - }); - describe('editor tabs', () => { beforeEach(async () => { await createComponent(); @@ -439,7 +375,6 @@ describe('RepoEditor', () => { }); describe('files in preview mode', () => { - let updateDimensionsSpy; const changeViewMode = (viewMode) => vm.$store.dispatch('editor/updateFileEditor', { path: vm.file.path, @@ -451,12 +386,6 @@ describe('RepoEditor', () => { activeFile: dummyFile.markdown, }); - const ext = extensionsStore.get('EditorWebIde'); - updateDimensionsSpy = jest.fn(); - spyOnApi(ext, { - updateDimensions: updateDimensionsSpy, - }); - changeViewMode(FILE_VIEW_MODE_PREVIEW); await nextTick(); }); @@ -465,15 +394,6 @@ describe('RepoEditor', () => { expect(vm.showEditor).toBe(false); expect(findEditor().isVisible()).toBe(false); }); - - it('updates dimensions when switching view back to edit', async () => { - expect(updateDimensionsSpy).not.toHaveBeenCalled(); - - changeViewMode(FILE_VIEW_MODE_EDITOR); - await nextTick(); - - expect(updateDimensionsSpy).toHaveBeenCalled(); - }); }); describe('initEditor', () => { diff --git a/spec/frontend/projects/settings/repository/branch_rules/app_spec.js b/spec/frontend/projects/settings/repository/branch_rules/app_spec.js new file mode 100644 index 00000000000..e12c3aeedd6 --- /dev/null +++ b/spec/frontend/projects/settings/repository/branch_rules/app_spec.js @@ -0,0 +1,18 @@ +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import BranchRules from '~/projects/settings/repository/branch_rules/app.vue'; + +describe('Branch rules app', () => { + let wrapper; + + const createComponent = () => { + wrapper = mountExtended(BranchRules); + }; + + const findTitle = () => wrapper.find('strong'); + + beforeEach(() => createComponent()); + + it('renders a title', () => { + expect(findTitle().text()).toBe('Branch'); + }); +}); diff --git a/spec/frontend/vue_mr_widget/components/approvals/approvals_spec.js b/spec/frontend/vue_mr_widget/components/approvals/approvals_spec.js index 4985417ad99..e7a98d2ebee 100644 --- a/spec/frontend/vue_mr_widget/components/approvals/approvals_spec.js +++ b/spec/frontend/vue_mr_widget/components/approvals/approvals_spec.js @@ -279,9 +279,9 @@ describe('MRWidget approvals', () => { it('revoke action is rendered', () => { expect(findActionData()).toEqual({ - variant: 'warning', + category: 'primary', + variant: 'default', text: 'Revoke approval', - category: 'secondary', }); }); diff --git a/spec/frontend/vue_shared/components/__snapshots__/clone_dropdown_spec.js.snap b/spec/frontend/vue_shared/components/__snapshots__/clone_dropdown_spec.js.snap index cf2ed3331b7..30e15595193 100644 --- a/spec/frontend/vue_shared/components/__snapshots__/clone_dropdown_spec.js.snap +++ b/spec/frontend/vue_shared/components/__snapshots__/clone_dropdown_spec.js.snap @@ -12,7 +12,7 @@ exports[`Clone Dropdown Button rendering matches the snapshot 1`] = ` right="true" size="medium" text="Clone" - variant="info" + variant="confirm" > <div class="pb-2 mx-1" diff --git a/spec/graphql/features/authorization_spec.rb b/spec/graphql/features/authorization_spec.rb index 1d518e20da7..5ae497f9d37 100644 --- a/spec/graphql/features/authorization_spec.rb +++ b/spec/graphql/features/authorization_spec.rb @@ -179,7 +179,7 @@ RSpec.describe 'DeclarativePolicy authorization in GraphQL ' do describe 'type and field authorizations together' do let(:authorizing_object) { anything } let(:permission_1) { permission_collection.first } - let(:permission_2) { permission_collection.last } + let(:permission_2) { permission_collection.second } let(:type) do type_factory do |type| @@ -224,6 +224,55 @@ RSpec.describe 'DeclarativePolicy authorization in GraphQL ' do include_examples 'authorization with a collection of permissions' end + context 'when the resolver is a subclass of one that authorizes the object' do + let(:permission_object_one) { be_nil } + let(:permission_object_two) { be_nil } + let(:parent) do + parent = Class.new(Resolvers::BaseResolver) + parent.include(::Gitlab::Graphql::Authorize::AuthorizeResource) + parent.authorizes_object! + parent.authorize permission_1 + parent + end + + let(:resolver) do + simple_resolver(test_object, base_class: parent) + end + + include_examples 'authorization with a collection of permissions' + end + + context 'when the resolver is a subclass of one that authorizes the object, extra permission' do + let(:permission_object_one) { be_nil } + let(:permission_object_two) { be_nil } + let(:parent) do + parent = Class.new(Resolvers::BaseResolver) + parent.include(::Gitlab::Graphql::Authorize::AuthorizeResource) + parent.authorizes_object! + parent.authorize permission_1 + parent + end + + let(:resolver) do + resolver = simple_resolver(test_object, base_class: parent) + resolver.include(::Gitlab::Graphql::Authorize::AuthorizeResource) + resolver.authorize permission_2 + resolver + end + + context 'when the field does not define any permissions' do + let(:query_type) do + query_factory do |query| + query.field :item, type, + null: true, + resolver: resolver + end + end + + include_examples 'authorization with a collection of permissions' + end + end + context 'when the resolver does not authorize the object, but instead calls authorized_find!' do let(:permission_object_one) { test_object } let(:permission_object_two) { be_nil } diff --git a/spec/graphql/resolvers/group_milestones_resolver_spec.rb b/spec/graphql/resolvers/group_milestones_resolver_spec.rb index 7abc779a63c..3d0c4a9d7cb 100644 --- a/spec/graphql/resolvers/group_milestones_resolver_spec.rb +++ b/spec/graphql/resolvers/group_milestones_resolver_spec.rb @@ -126,16 +126,6 @@ RSpec.describe Resolvers::GroupMilestonesResolver do end end - context 'when user cannot read milestones' do - it 'generates an error' do - unauthorized_user = create(:user) - - expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do - resolve_group_milestones({}, { current_user: unauthorized_user }) - end - end - end - context 'when including descendant milestones in a public group' do let_it_be(:group) { create(:group, :public) } diff --git a/spec/graphql/resolvers/project_milestones_resolver_spec.rb b/spec/graphql/resolvers/project_milestones_resolver_spec.rb index 2cf490c2b6a..d99a3a40a6c 100644 --- a/spec/graphql/resolvers/project_milestones_resolver_spec.rb +++ b/spec/graphql/resolvers/project_milestones_resolver_spec.rb @@ -172,15 +172,5 @@ RSpec.describe Resolvers::ProjectMilestonesResolver do resolve_project_milestones(containing_date: t) end end - - context 'when user cannot read milestones' do - it 'generates an error' do - unauthorized_user = create(:user) - - expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do - resolve_project_milestones({}, { current_user: unauthorized_user }) - end - end - end end end diff --git a/spec/graphql/types/base_field_spec.rb b/spec/graphql/types/base_field_spec.rb index 9d02f061435..e701ec483a3 100644 --- a/spec/graphql/types/base_field_spec.rb +++ b/spec/graphql/types/base_field_spec.rb @@ -3,6 +3,91 @@ require 'spec_helper' RSpec.describe Types::BaseField do + describe 'authorized?' do + let(:object) { double } + let(:current_user) { nil } + let(:ctx) { { current_user: current_user } } + + it 'defaults to true' do + field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true) + + expect(field).to be_authorized(object, nil, ctx) + end + + it 'tests the field authorization, if provided' do + field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true, authorize: :foo) + + expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(false) + + expect(field).not_to be_authorized(object, nil, ctx) + end + + it 'tests the field authorization, if provided, when it succeeds' do + field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true, authorize: :foo) + + expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(true) + + expect(field).to be_authorized(object, nil, ctx) + end + + it 'only tests the resolver authorization if it authorizes_object?' do + resolver = Class.new + + field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true, + resolver_class: resolver) + + expect(field).to be_authorized(object, nil, ctx) + end + + it 'tests the resolver authorization, if provided' do + resolver = Class.new do + include Gitlab::Graphql::Authorize::AuthorizeResource + + authorizes_object! + end + + field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true, + resolver_class: resolver) + + expect(resolver).to receive(:authorized?).with(object, ctx).and_return(false) + + expect(field).not_to be_authorized(object, nil, ctx) + end + + it 'tests field authorization before resolver authorization, when field auth fails' do + resolver = Class.new do + include Gitlab::Graphql::Authorize::AuthorizeResource + + authorizes_object! + end + + field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true, + authorize: :foo, + resolver_class: resolver) + + expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(false) + + expect(field).not_to be_authorized(object, nil, ctx) + end + + it 'tests field authorization before resolver authorization, when field auth succeeds' do + resolver = Class.new do + include Gitlab::Graphql::Authorize::AuthorizeResource + + authorizes_object! + end + + field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true, + authorize: :foo, + resolver_class: resolver) + + expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(true) + expect(resolver).to receive(:authorized?).with(object, ctx).and_return(false) + + expect(field).not_to be_authorized(object, nil, ctx) + end + end + context 'when considering complexity' do let(:resolver) do Class.new(described_class) do diff --git a/spec/lib/api/helpers/project_stats_refresh_conflicts_helpers_spec.rb b/spec/lib/api/helpers/project_stats_refresh_conflicts_helpers_spec.rb new file mode 100644 index 00000000000..ae5c21e01c3 --- /dev/null +++ b/spec/lib/api/helpers/project_stats_refresh_conflicts_helpers_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Helpers::ProjectStatsRefreshConflictsHelpers do + let_it_be(:project) { create(:project) } + + let(:api_class) do + Class.new do + include API::Helpers::ProjectStatsRefreshConflictsHelpers + end + end + + let(:api_controller) { api_class.new } + + describe '#reject_if_build_artifacts_size_refreshing!' do + let(:entrypoint) { '/some/thing' } + + before do + allow(project).to receive(:refreshing_build_artifacts_size?).and_return(refreshing) + allow(api_controller).to receive_message_chain(:request, :path).and_return(entrypoint) + end + + context 'when project is undergoing stats refresh' do + let(:refreshing) { true } + + it 'logs and returns a 409 conflict error' do + expect(Gitlab::ProjectStatsRefreshConflictsLogger) + .to receive(:warn_request_rejected_during_stats_refresh) + .with(project.id) + + expect(api_controller).to receive(:conflict!) + + api_controller.reject_if_build_artifacts_size_refreshing!(project) + end + end + + context 'when project is not undergoing stats refresh' do + let(:refreshing) { false } + + it 'does nothing' do + expect(Gitlab::ProjectStatsRefreshConflictsLogger).not_to receive(:warn_request_rejected_during_stats_refresh) + expect(api_controller).not_to receive(:conflict) + + api_controller.reject_if_build_artifacts_size_refreshing!(project) + end + end + end +end diff --git a/spec/lib/gitlab/background_migration/fix_merge_request_diff_commit_users_spec.rb b/spec/lib/gitlab/background_migration/fix_merge_request_diff_commit_users_spec.rb index c343ee438b8..99df21562b0 100644 --- a/spec/lib/gitlab/background_migration/fix_merge_request_diff_commit_users_spec.rb +++ b/spec/lib/gitlab/background_migration/fix_merge_request_diff_commit_users_spec.rb @@ -2,314 +2,23 @@ require 'spec_helper' -# The underlying migration relies on the global models (e.g. Project). This -# means we also need to use FactoryBot factories to ensure everything is -# operating using the same types. If we use `table()` and similar methods we -# would have to duplicate a lot of logic just for these tests. -# # rubocop: disable RSpec/FactoriesInMigrationSpecs RSpec.describe Gitlab::BackgroundMigration::FixMergeRequestDiffCommitUsers do let(:migration) { described_class.new } describe '#perform' do context 'when the project exists' do - it 'processes the project' do + it 'does nothing' do project = create(:project) - expect(migration).to receive(:process).with(project) - expect(migration).to receive(:schedule_next_job) - - migration.perform(project.id) - end - - it 'marks the background job as finished' do - project = create(:project) - - Gitlab::Database::BackgroundMigrationJob.create!( - class_name: 'FixMergeRequestDiffCommitUsers', - arguments: [project.id] - ) - - migration.perform(project.id) - - job = Gitlab::Database::BackgroundMigrationJob - .find_by(class_name: 'FixMergeRequestDiffCommitUsers') - - expect(job.status).to eq('succeeded') + expect { migration.perform(project.id) }.not_to raise_error end end context 'when the project does not exist' do it 'does nothing' do - expect(migration).not_to receive(:process) - expect(migration).to receive(:schedule_next_job) - - migration.perform(-1) - end - end - end - - describe '#process' do - it 'processes the merge requests of the project' do - project = create(:project, :repository) - commit = project.commit - mr = create( - :merge_request_with_diffs, - source_project: project, - target_project: project - ) - - diff = mr.merge_request_diffs.first - - create( - :merge_request_diff_commit, - merge_request_diff: diff, - sha: commit.sha, - relative_order: 9000 - ) - - migration.process(project) - - updated = diff - .merge_request_diff_commits - .find_by(sha: commit.sha, relative_order: 9000) - - expect(updated.commit_author_id).not_to be_nil - expect(updated.committer_id).not_to be_nil - end - end - - describe '#update_commit' do - let(:project) { create(:project, :repository) } - let(:mr) do - create( - :merge_request_with_diffs, - source_project: project, - target_project: project - ) - end - - let(:diff) { mr.merge_request_diffs.first } - let(:commit) { project.commit } - - def update_row(migration, project, diff, row) - migration.update_commit(project, row) - - diff - .merge_request_diff_commits - .find_by(sha: row.sha, relative_order: row.relative_order) - end - - it 'populates missing commit authors' do - commit_row = create( - :merge_request_diff_commit, - merge_request_diff: diff, - sha: commit.sha, - relative_order: 9000 - ) - - updated = update_row(migration, project, diff, commit_row) - - expect(updated.commit_author.name).to eq(commit.to_hash[:author_name]) - expect(updated.commit_author.email).to eq(commit.to_hash[:author_email]) - end - - it 'populates missing committers' do - commit_row = create( - :merge_request_diff_commit, - merge_request_diff: diff, - sha: commit.sha, - relative_order: 9000 - ) - - updated = update_row(migration, project, diff, commit_row) - - expect(updated.committer.name).to eq(commit.to_hash[:committer_name]) - expect(updated.committer.email).to eq(commit.to_hash[:committer_email]) - end - - it 'leaves existing commit authors as-is' do - user = create(:merge_request_diff_commit_user) - commit_row = create( - :merge_request_diff_commit, - merge_request_diff: diff, - sha: commit.sha, - relative_order: 9000, - commit_author: user - ) - - updated = update_row(migration, project, diff, commit_row) - - expect(updated.commit_author).to eq(user) - end - - it 'leaves existing committers as-is' do - user = create(:merge_request_diff_commit_user) - commit_row = create( - :merge_request_diff_commit, - merge_request_diff: diff, - sha: commit.sha, - relative_order: 9000, - committer: user - ) - - updated = update_row(migration, project, diff, commit_row) - - expect(updated.committer).to eq(user) - end - - it 'does nothing when both the author and committer are present' do - user = create(:merge_request_diff_commit_user) - commit_row = create( - :merge_request_diff_commit, - merge_request_diff: diff, - sha: commit.sha, - relative_order: 9000, - committer: user, - commit_author: user - ) - - recorder = ActiveRecord::QueryRecorder.new do - migration.update_commit(project, commit_row) - end - - expect(recorder.count).to be_zero - end - - it 'does nothing if the commit does not exist in Git' do - user = create(:merge_request_diff_commit_user) - commit_row = create( - :merge_request_diff_commit, - merge_request_diff: diff, - sha: 'kittens', - relative_order: 9000, - committer: user, - commit_author: user - ) - - recorder = ActiveRecord::QueryRecorder.new do - migration.update_commit(project, commit_row) + expect { migration.perform(-1) }.not_to raise_error end - - expect(recorder.count).to be_zero - end - - it 'does nothing when the committer/author are missing in the Git commit' do - user = create(:merge_request_diff_commit_user) - commit_row = create( - :merge_request_diff_commit, - merge_request_diff: diff, - sha: commit.sha, - relative_order: 9000, - committer: user, - commit_author: user - ) - - allow(migration).to receive(:find_or_create_user).and_return(nil) - - recorder = ActiveRecord::QueryRecorder.new do - migration.update_commit(project, commit_row) - end - - expect(recorder.count).to be_zero - end - end - - describe '#schedule_next_job' do - it 'schedules the next background migration' do - Gitlab::Database::BackgroundMigrationJob - .create!(class_name: 'FixMergeRequestDiffCommitUsers', arguments: [42]) - - expect(BackgroundMigrationWorker) - .to receive(:perform_in) - .with(2.minutes, 'FixMergeRequestDiffCommitUsers', [42]) - - migration.schedule_next_job - end - - it 'does nothing when there are no jobs' do - expect(BackgroundMigrationWorker) - .not_to receive(:perform_in) - - migration.schedule_next_job - end - end - - describe '#find_commit' do - let(:project) { create(:project, :repository) } - - it 'finds a commit using Git' do - commit = project.commit - found = migration.find_commit(project, commit.sha) - - expect(found).to eq(commit.to_hash) - end - - it 'caches the results' do - commit = project.commit - - migration.find_commit(project, commit.sha) - - expect { migration.find_commit(project, commit.sha) } - .not_to change { Gitlab::GitalyClient.get_request_count } - end - - it 'returns an empty hash if the commit does not exist' do - expect(migration.find_commit(project, 'kittens')).to eq({}) - end - end - - describe '#find_or_create_user' do - let(:project) { create(:project, :repository) } - - it 'creates missing users' do - commit = project.commit.to_hash - id = migration.find_or_create_user(commit, :author_name, :author_email) - - expect(MergeRequest::DiffCommitUser.count).to eq(1) - - created = MergeRequest::DiffCommitUser.first - - expect(created.name).to eq(commit[:author_name]) - expect(created.email).to eq(commit[:author_email]) - expect(created.id).to eq(id) - end - - it 'returns users that already exist' do - commit = project.commit.to_hash - user1 = migration.find_or_create_user(commit, :author_name, :author_email) - user2 = migration.find_or_create_user(commit, :author_name, :author_email) - - expect(user1).to eq(user2) - end - - it 'caches the results' do - commit = project.commit.to_hash - - migration.find_or_create_user(commit, :author_name, :author_email) - - recorder = ActiveRecord::QueryRecorder.new do - migration.find_or_create_user(commit, :author_name, :author_email) - end - - expect(recorder.count).to be_zero - end - - it 'returns nil if the commit details are missing' do - id = migration.find_or_create_user({}, :author_name, :author_email) - - expect(id).to be_nil - end - end - - describe '#matches_row' do - it 'returns the query matches for the composite primary key' do - row = double(:commit, merge_request_diff_id: 4, relative_order: 5) - arel = migration.matches_row(row) - - expect(arel.to_sql).to eq( - '("merge_request_diff_commits"."merge_request_diff_id", "merge_request_diff_commits"."relative_order") = (4, 5)' - ) end end end diff --git a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb index 0c548e1ce32..ac512e28e7b 100644 --- a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb +++ b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb @@ -103,4 +103,36 @@ RSpec.describe Gitlab::Graphql::Authorize::AuthorizeResource do .to contain_exactly(:base_authorization, :sub_authorization) end end + + describe 'authorizes_object?' do + it 'is false by default' do + a_class = Class.new do + include Gitlab::Graphql::Authorize::AuthorizeResource + end + + expect(a_class).not_to be_authorizes_object + end + + it 'is true after calling authorizes_object!' do + a_class = Class.new do + include Gitlab::Graphql::Authorize::AuthorizeResource + + authorizes_object! + end + + expect(a_class).to be_authorizes_object + end + + it 'is true if a parent authorizes_object' do + parent = Class.new do + include Gitlab::Graphql::Authorize::AuthorizeResource + + authorizes_object! + end + + child = Class.new(parent) + + expect(child).to be_authorizes_object + end + end end diff --git a/spec/lib/gitlab/project_stats_refresh_conflicts_logger_spec.rb b/spec/lib/gitlab/project_stats_refresh_conflicts_logger_spec.rb index dadff90bf27..6dbfd5804d7 100644 --- a/spec/lib/gitlab/project_stats_refresh_conflicts_logger_spec.rb +++ b/spec/lib/gitlab/project_stats_refresh_conflicts_logger_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe Gitlab::ProjectStatsRefreshConflictsLogger do before do - Gitlab::ApplicationContext.push(feature_category: 'test') + Gitlab::ApplicationContext.push(feature_category: 'test', caller_id: 'caller') end describe '.warn_artifact_deletion_during_stats_refresh' do @@ -18,11 +18,30 @@ RSpec.describe Gitlab::ProjectStatsRefreshConflictsLogger do method: method, project_id: project_id, 'correlation_id' => an_instance_of(String), - 'meta.feature_category' => 'test' + 'meta.feature_category' => 'test', + 'meta.caller_id' => 'caller' ) ) described_class.warn_artifact_deletion_during_stats_refresh(project_id: project_id, method: method) end end + + describe '.warn_request_rejected_during_stats_refresh' do + it 'logs a warning about artifacts being deleted while the project is undergoing stats refresh' do + project_id = 123 + + expect(Gitlab::AppLogger).to receive(:warn).with( + hash_including( + message: 'Rejected request due to project undergoing stats refresh', + project_id: project_id, + 'correlation_id' => an_instance_of(String), + 'meta.feature_category' => 'test', + 'meta.caller_id' => 'caller' + ) + ) + + described_class.warn_request_rejected_during_stats_refresh(project_id) + end + end end diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_total_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_total_metric_spec.rb new file mode 100644 index 00000000000..bfc4240def6 --- /dev/null +++ b/spec/lib/gitlab/usage/metrics/instrumentations/count_imported_projects_total_metric_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Usage::Metrics::Instrumentations::CountImportedProjectsTotalMetric do + let_it_be(:user) { create(:user) } + let_it_be(:gitea_imports) do + create_list(:project, 3, import_type: 'gitea', creator_id: user.id, created_at: 3.weeks.ago) + end + + let_it_be(:bitbucket_imports) do + create_list(:project, 2, import_type: 'bitbucket', creator_id: user.id, created_at: 3.weeks.ago) + end + + let_it_be(:old_import) { create(:project, import_type: 'gitea', creator_id: user.id, created_at: 2.months.ago) } + + let_it_be(:bulk_import_projects) do + create_list(:bulk_import_entity, 3, source_type: 'project_entity', created_at: 3.weeks.ago) + end + + let_it_be(:bulk_import_groups) do + create_list(:bulk_import_entity, 3, source_type: 'group_entity', created_at: 3.weeks.ago) + end + + let_it_be(:old_bulk_import_project) do + create(:bulk_import_entity, source_type: 'project_entity', created_at: 2.months.ago) + end + + before do + allow(ApplicationRecord.connection).to receive(:transaction_open?).and_return(false) + end + + context 'with all time frame' do + let(:expected_value) { 10 } + let(:expected_query) do + "SELECT (SELECT COUNT(\"projects\".\"id\") FROM \"projects\" WHERE \"projects\".\"import_type\""\ + " IN ('gitlab_project', 'gitlab', 'github', 'bitbucket', 'bitbucket_server', 'gitea', 'git', 'manifest',"\ + " 'gitlab_migration'))"\ + " + (SELECT COUNT(\"bulk_import_entities\".\"id\") FROM \"bulk_import_entities\""\ + " WHERE \"bulk_import_entities\".\"source_type\" = 1)" + end + + it_behaves_like 'a correct instrumented metric value and query', time_frame: 'all' + end + + context 'for 28d time frame' do + let(:expected_value) { 8 } + let(:start) { 30.days.ago.to_s(:db) } + let(:finish) { 2.days.ago.to_s(:db) } + let(:expected_query) do + "SELECT (SELECT COUNT(\"projects\".\"id\") FROM \"projects\" WHERE \"projects\".\"import_type\""\ + " IN ('gitlab_project', 'gitlab', 'github', 'bitbucket', 'bitbucket_server', 'gitea', 'git', 'manifest',"\ + " 'gitlab_migration')"\ + " AND \"projects\".\"created_at\" BETWEEN '#{start}' AND '#{finish}')"\ + " + (SELECT COUNT(\"bulk_import_entities\".\"id\") FROM \"bulk_import_entities\""\ + " WHERE \"bulk_import_entities\".\"source_type\" = 1 AND \"bulk_import_entities\".\"created_at\""\ + " BETWEEN '#{start}' AND '#{finish}')" + end + + it_behaves_like 'a correct instrumented metric value and query', time_frame: '28d' + end +end diff --git a/spec/migrations/20220502015011_clean_up_fix_merge_request_diff_commit_users_spec.rb b/spec/migrations/20220502015011_clean_up_fix_merge_request_diff_commit_users_spec.rb index 769c0993b67..2bc3e89a748 100644 --- a/spec/migrations/20220502015011_clean_up_fix_merge_request_diff_commit_users_spec.rb +++ b/spec/migrations/20220502015011_clean_up_fix_merge_request_diff_commit_users_spec.rb @@ -15,21 +15,5 @@ RSpec.describe CleanUpFixMergeRequestDiffCommitUsers, :migration do migrate! end - - it 'processes pending background jobs' do - project = projects.create!(name: 'p1', namespace_id: namespace.id, project_namespace_id: project_namespace.id) - - Gitlab::Database::BackgroundMigrationJob.create!( - class_name: 'FixMergeRequestDiffCommitUsers', - arguments: [project.id] - ) - - migrate! - - background_migrations = Gitlab::Database::BackgroundMigrationJob - .where(class_name: 'FixMergeRequestDiffCommitUsers') - - expect(background_migrations.count).to eq(0) - end end end diff --git a/spec/models/project_group_link_spec.rb b/spec/models/project_group_link_spec.rb index c925d87170c..8b95b86b14b 100644 --- a/spec/models/project_group_link_spec.rb +++ b/spec/models/project_group_link_spec.rb @@ -30,6 +30,12 @@ RSpec.describe ProjectGroupLink do expect(project_group_link).not_to be_valid end + + it 'does not allow a project to be shared with `OWNER` access level' do + project_group_link.group_access = Gitlab::Access::OWNER + + expect(project_group_link).not_to be_valid + end end describe 'scopes' do diff --git a/spec/requests/api/ci/job_artifacts_spec.rb b/spec/requests/api/ci/job_artifacts_spec.rb index 1dd1ca4e115..2fa1ffb4974 100644 --- a/spec/requests/api/ci/job_artifacts_spec.rb +++ b/spec/requests/api/ci/job_artifacts_spec.rb @@ -41,42 +41,58 @@ RSpec.describe API::Ci::JobArtifacts do describe 'DELETE /projects/:id/jobs/:job_id/artifacts' do let!(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } - before do - delete api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) - end + context 'when project is not undergoing stats refresh' do + before do + delete api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) + end - context 'when user is anonymous' do - let(:api_user) { nil } + context 'when user is anonymous' do + let(:api_user) { nil } - it 'does not delete artifacts' do - expect(job.job_artifacts.size).to eq 2 - end + it 'does not delete artifacts' do + expect(job.job_artifacts.size).to eq 2 + end - it 'returns status 401 (unauthorized)' do - expect(response).to have_gitlab_http_status(:unauthorized) + it 'returns status 401 (unauthorized)' do + expect(response).to have_gitlab_http_status(:unauthorized) + end end - end - context 'with developer' do - it 'does not delete artifacts' do - expect(job.job_artifacts.size).to eq 2 + context 'with developer' do + it 'does not delete artifacts' do + expect(job.job_artifacts.size).to eq 2 + end + + it 'returns status 403 (forbidden)' do + expect(response).to have_gitlab_http_status(:forbidden) + end end - it 'returns status 403 (forbidden)' do - expect(response).to have_gitlab_http_status(:forbidden) + context 'with authorized user' do + let(:maintainer) { create(:project_member, :maintainer, project: project).user } + let!(:api_user) { maintainer } + + it 'deletes artifacts' do + expect(job.job_artifacts.size).to eq 0 + end + + it 'returns status 204 (no content)' do + expect(response).to have_gitlab_http_status(:no_content) + end end end - context 'with authorized user' do - let(:maintainer) { create(:project_member, :maintainer, project: project).user } - let!(:api_user) { maintainer } + context 'when project is undergoing stats refresh' do + it_behaves_like 'preventing request because of ongoing project stats refresh' do + let(:maintainer) { create(:project_member, :maintainer, project: project).user } + let(:api_user) { maintainer } + let(:make_request) { delete api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) } - it 'deletes artifacts' do - expect(job.job_artifacts.size).to eq 0 - end + it 'does not delete artifacts' do + make_request - it 'returns status 204 (no content)' do - expect(response).to have_gitlab_http_status(:no_content) + expect(job.job_artifacts.size).to eq 2 + end end end end @@ -131,6 +147,22 @@ RSpec.describe API::Ci::JobArtifacts do expect(response).to have_gitlab_http_status(:accepted) end + + context 'when project is undergoing stats refresh' do + let!(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } + + it_behaves_like 'preventing request because of ongoing project stats refresh' do + let(:maintainer) { create(:project_member, :maintainer, project: project).user } + let(:api_user) { maintainer } + let(:make_request) { delete api("/projects/#{project.id}/artifacts", api_user) } + + it 'does not delete artifacts' do + make_request + + expect(job.job_artifacts.size).to eq 2 + end + end + end end end diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 4bd9f81fd1d..a6cf2dc6d9f 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -655,62 +655,80 @@ RSpec.describe API::Ci::Jobs do before do project.add_role(user, role) - - post api("/projects/#{project.id}/jobs/#{job.id}/erase", user) end - shared_examples_for 'erases job' do - it 'erases job content' do - expect(response).to have_gitlab_http_status(:created) - expect(job.job_artifacts.count).to eq(0) - expect(job.trace.exist?).to be_falsy - expect(job.artifacts_file.present?).to be_falsy - expect(job.artifacts_metadata.present?).to be_falsy - expect(job.has_job_artifacts?).to be_falsy + context 'when project is not undergoing stats refresh' do + before do + post api("/projects/#{project.id}/jobs/#{job.id}/erase", user) end - end - context 'job is erasable' do - let(:job) { create(:ci_build, :trace_artifact, :artifacts, :test_reports, :success, project: project, pipeline: pipeline) } + shared_examples_for 'erases job' do + it 'erases job content' do + expect(response).to have_gitlab_http_status(:created) + expect(job.job_artifacts.count).to eq(0) + expect(job.trace.exist?).to be_falsy + expect(job.artifacts_file.present?).to be_falsy + expect(job.artifacts_metadata.present?).to be_falsy + expect(job.has_job_artifacts?).to be_falsy + end + end + + context 'job is erasable' do + let(:job) { create(:ci_build, :trace_artifact, :artifacts, :test_reports, :success, project: project, pipeline: pipeline) } - it_behaves_like 'erases job' + it_behaves_like 'erases job' - it 'updates job' do - job.reload + it 'updates job' do + job.reload - expect(job.erased_at).to be_truthy - expect(job.erased_by).to eq(user) + expect(job.erased_at).to be_truthy + expect(job.erased_by).to eq(user) + end end - end - context 'when job has an unarchived trace artifact' do - let(:job) { create(:ci_build, :success, :trace_live, :unarchived_trace_artifact, project: project, pipeline: pipeline) } + context 'when job has an unarchived trace artifact' do + let(:job) { create(:ci_build, :success, :trace_live, :unarchived_trace_artifact, project: project, pipeline: pipeline) } - it_behaves_like 'erases job' - end + it_behaves_like 'erases job' + end - context 'job is not erasable' do - let(:job) { create(:ci_build, :trace_live, project: project, pipeline: pipeline) } + context 'job is not erasable' do + let(:job) { create(:ci_build, :trace_live, project: project, pipeline: pipeline) } - it 'responds with forbidden' do - expect(response).to have_gitlab_http_status(:forbidden) + it 'responds with forbidden' do + expect(response).to have_gitlab_http_status(:forbidden) + end end - end - context 'when a developer erases a build' do - let(:role) { :developer } - let(:job) { create(:ci_build, :trace_artifact, :artifacts, :success, project: project, pipeline: pipeline, user: owner) } + context 'when a developer erases a build' do + let(:role) { :developer } + let(:job) { create(:ci_build, :trace_artifact, :artifacts, :success, project: project, pipeline: pipeline, user: owner) } + + context 'when the build was created by the developer' do + let(:owner) { user } + + it { expect(response).to have_gitlab_http_status(:created) } + end - context 'when the build was created by the developer' do - let(:owner) { user } + context 'when the build was created by another user' do + let(:owner) { create(:user) } - it { expect(response).to have_gitlab_http_status(:created) } + it { expect(response).to have_gitlab_http_status(:forbidden) } + end end + end + + context 'when project is undergoing stats refresh' do + let(:job) { create(:ci_build, :trace_artifact, :artifacts, :test_reports, :success, project: project, pipeline: pipeline) } + + it_behaves_like 'preventing request because of ongoing project stats refresh' do + let(:make_request) { post api("/projects/#{project.id}/jobs/#{job.id}/erase", user) } - context 'when the build was created by the other' do - let(:owner) { create(:user) } + it 'does not delete artifacts' do + make_request - it { expect(response).to have_gitlab_http_status(:forbidden) } + expect(job.reload.job_artifacts).not_to be_empty + end end end end diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index 12faeec94da..697fe16e222 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -1018,6 +1018,18 @@ RSpec.describe API::Ci::Pipelines do expect { build.reload }.to raise_error(ActiveRecord::RecordNotFound) end end + + context 'when project is undergoing stats refresh' do + it_behaves_like 'preventing request because of ongoing project stats refresh' do + let(:make_request) { delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) } + + it 'does not delete the pipeline' do + make_request + + expect(pipeline.reload).to be_persisted + end + end + end end context 'unauthorized user' do diff --git a/spec/requests/api/graphql/milestone_spec.rb b/spec/requests/api/graphql/milestone_spec.rb index 59de116fa2b..f6835936418 100644 --- a/spec/requests/api/graphql/milestone_spec.rb +++ b/spec/requests/api/graphql/milestone_spec.rb @@ -5,43 +5,125 @@ require 'spec_helper' RSpec.describe 'Querying a Milestone' do include GraphqlHelpers - let_it_be(:current_user) { create(:user) } + let_it_be(:guest) { create(:user) } let_it_be(:project) { create(:project) } let_it_be(:milestone) { create(:milestone, project: project) } + let_it_be(:release_a) { create(:release, project: project) } + let_it_be(:release_b) { create(:release, project: project) } - let(:query) do - graphql_query_for('milestone', { id: milestone.to_global_id.to_s }, 'title') + before_all do + milestone.releases << [release_a, release_b] + project.add_guest(guest) end - subject { graphql_data['milestone'] } - - before do - post_graphql(query, current_user: current_user) + let(:expected_release_nodes) do + contain_exactly(a_graphql_entity_for(release_a), a_graphql_entity_for(release_b)) end - context 'when the user has access to the milestone' do - before_all do - project.add_guest(current_user) + context 'when we post the query' do + let(:current_user) { nil } + let(:query) do + graphql_query_for('milestone', { id: milestone.to_global_id.to_s }, all_graphql_fields_for('Milestone')) end - it_behaves_like 'a working graphql query' + subject { graphql_data['milestone'] } - it { is_expected.to include('title' => milestone.name) } - end + before do + post_graphql(query, current_user: current_user) + end - context 'when the user does not have access to the milestone' do - it_behaves_like 'a working graphql query' + context 'when the user has access to the milestone' do + let(:current_user) { guest } - it { is_expected.to be_nil } + it_behaves_like 'a working graphql query' + + it { is_expected.to include('title' => milestone.name) } + + it 'contains release information' do + is_expected.to include('releases' => include('nodes' => expected_release_nodes)) + end + end + + context 'when the user does not have access to the milestone' do + it_behaves_like 'a working graphql query' + + it { is_expected.to be_nil } + end + + context 'when ID argument is missing' do + let(:query) do + graphql_query_for('milestone', {}, 'title') + end + + it 'raises an exception' do + expect(graphql_errors).to include(a_hash_including('message' => "Field 'milestone' is missing required arguments: id")) + end + end end - context 'when ID argument is missing' do - let(:query) do - graphql_query_for('milestone', {}, 'title') + context 'when there are two milestones' do + let_it_be(:milestone_b) { create(:milestone, project: project) } + + let(:current_user) { guest } + let(:milestone_fields) do + <<~GQL + fragment milestoneFields on Milestone { + #{all_graphql_fields_for('Milestone', max_depth: 1)} + releases { nodes { #{all_graphql_fields_for('Release', max_depth: 1)} } } + } + GQL + end + + let(:single_query) do + <<~GQL + query ($id_a: MilestoneID!) { + a: milestone(id: $id_a) { ...milestoneFields } + } + + #{milestone_fields} + GQL + end + + let(:multi_query) do + <<~GQL + query ($id_a: MilestoneID!, $id_b: MilestoneID!) { + a: milestone(id: $id_a) { ...milestoneFields } + b: milestone(id: $id_b) { ...milestoneFields } + } + #{milestone_fields} + GQL + end + + it 'produces correct results' do + r = run_with_clean_state(multi_query, + context: { current_user: current_user }, + variables: { + id_a: global_id_of(milestone).to_s, + id_b: milestone_b.to_global_id.to_s + }) + + expect(r.to_h['errors']).to be_blank + expect(graphql_dig_at(r.to_h, :data, :a, :releases, :nodes)).to match expected_release_nodes + expect(graphql_dig_at(r.to_h, :data, :b, :releases, :nodes)).to be_empty end - it 'raises an exception' do - expect(graphql_errors).to include(a_hash_including('message' => "Field 'milestone' is missing required arguments: id")) + it 'does not suffer from N+1 performance issues' do + baseline = ActiveRecord::QueryRecorder.new do + run_with_clean_state(single_query, + context: { current_user: current_user }, + variables: { id_a: milestone.to_global_id.to_s }) + end + + multi = ActiveRecord::QueryRecorder.new do + run_with_clean_state(multi_query, + context: { current_user: current_user }, + variables: { + id_a: milestone.to_global_id.to_s, + id_b: milestone_b.to_global_id.to_s + }) + end + + expect(multi).not_to exceed_query_limit(baseline) end end end diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb index 37656ab4eea..7abd5ca8772 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_destroy_spec.rb @@ -28,4 +28,21 @@ RSpec.describe 'PipelineDestroy' do expect(response).to have_gitlab_http_status(:success) expect { pipeline.reload }.to raise_error(ActiveRecord::RecordNotFound) end + + context 'when project is undergoing stats refresh' do + before do + create(:project_build_artifacts_size_refresh, :pending, project: pipeline.project) + end + + it 'returns an error and does not destroy the pipeline' do + expect(Gitlab::ProjectStatsRefreshConflictsLogger) + .to receive(:warn_request_rejected_during_stats_refresh) + .with(pipeline.project.id) + + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_mutation_response(:pipeline_destroy)['errors']).not_to be_empty + expect(pipeline.reload).to be_persisted + end + end end diff --git a/spec/requests/api/graphql/project/milestones_spec.rb b/spec/requests/api/graphql/project/milestones_spec.rb index 3e8948d83b1..d1ee157fc74 100644 --- a/spec/requests/api/graphql/project/milestones_spec.rb +++ b/spec/requests/api/graphql/project/milestones_spec.rb @@ -59,6 +59,27 @@ RSpec.describe 'getting milestone listings nested in a project' do end end + context 'the user does not have access' do + let_it_be(:project) { create(:project) } + let_it_be(:milestones) { create_list(:milestone, 2, project: project) } + + it 'is nil' do + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:project)).to be_nil + end + + context 'the user has access' do + let(:expected) { milestones } + + before do + project.add_guest(current_user) + end + + it_behaves_like 'searching with parameters' + end + end + context 'there are no search params' do let(:search_params) { nil } let(:expected) { all_milestones } diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 0db42e7439c..94f1bf13830 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -4,13 +4,14 @@ require 'spec_helper' RSpec.describe API::Members do let(:maintainer) { create(:user, username: 'maintainer_user') } + let(:maintainer2) { create(:user, username: 'user-with-maintainer-role') } let(:developer) { create(:user) } let(:access_requester) { create(:user) } let(:stranger) { create(:user) } let(:user_with_minimal_access) { create(:user) } let(:project) do - create(:project, :public, creator_id: maintainer.id, namespace: maintainer.namespace) do |project| + create(:project, :public, creator_id: maintainer.id, group: create(:group, :public)) do |project| project.add_maintainer(maintainer) project.add_developer(developer, current_user: maintainer) project.request_access(access_requester) @@ -238,21 +239,48 @@ RSpec.describe API::Members do expect(response).to have_gitlab_http_status(:forbidden) end end + + context 'adding a member of higher access level' do + before do + # the other 'maintainer' is in fact an owner of the group! + source.add_maintainer(maintainer2) + end + + context 'when an access requester' do + it 'is not successful' do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer2), + params: { user_id: access_requester.id, access_level: Member::OWNER } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when a totally new user' do + it 'is not successful' do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer2), + params: { user_id: stranger.id, access_level: Member::OWNER } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end end end - context 'when authenticated as a maintainer/owner' do + context 'when authenticated as a member with membership management rights' do context 'and new member is already a requester' do - it 'transforms the requester into a proper member' do - expect do - post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), - params: { user_id: access_requester.id, access_level: Member::MAINTAINER } - - expect(response).to have_gitlab_http_status(:created) - end.to change { source.members.count }.by(1) - expect(source.requesters.count).to eq(0) - expect(json_response['id']).to eq(access_requester.id) - expect(json_response['access_level']).to eq(Member::MAINTAINER) + context 'when the requester is of equal or lower access level' do + it 'transforms the requester into a proper member' do + expect do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), + params: { user_id: access_requester.id, access_level: Member::MAINTAINER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.count }.by(1) + expect(source.requesters.count).to eq(0) + expect(json_response['id']).to eq(access_requester.id) + expect(json_response['access_level']).to eq(Member::MAINTAINER) + end end end @@ -430,7 +458,7 @@ RSpec.describe API::Members do it 'returns 404 when the user_id is not valid' do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), - params: { user_id: 0, access_level: Member::MAINTAINER } + params: { user_id: non_existing_record_id, access_level: Member::MAINTAINER } expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') @@ -500,16 +528,49 @@ RSpec.describe API::Members do end end end + + context 'as a maintainer updating a member to one with higher access level than themselves' do + before do + # the other 'maintainer' is in fact an owner of the group! + source.add_maintainer(maintainer2) + end + + it 'returns 403' do + put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer2), + params: { access_level: Member::OWNER } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end end context 'when authenticated as a maintainer/owner' do - it 'updates the member' do - put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer), - params: { access_level: Member::MAINTAINER } + context 'when updating a member with the same or lower access level' do + it 'updates the member' do + put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer), + params: { access_level: Member::MAINTAINER } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['id']).to eq(developer.id) - expect(json_response['access_level']).to eq(Member::MAINTAINER) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['id']).to eq(developer.id) + expect(json_response['access_level']).to eq(Member::MAINTAINER) + end + end + + context 'when updating a member with higher access level' do + let(:owner) { create(:user) } + + before do + source.add_owner(owner) + # the other 'maintainer' is in fact an owner of the group! + source.add_maintainer(maintainer2) + end + + it 'returns 403' do + put api("/#{source_type.pluralize}/#{source.id}/members/#{owner.id}", maintainer2), + params: { access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:forbidden) + end end end @@ -604,6 +665,23 @@ RSpec.describe API::Members do end end + context 'when attempting to delete a member with higher access level' do + let(:owner) { create(:user) } + + before do + source.add_owner(owner) + # the other 'maintainer' is in fact an owner of the group! + source.add_maintainer(maintainer2) + end + + it 'returns 403' do + delete api("/#{source_type.pluralize}/#{source.id}/members/#{owner.id}", maintainer2), + params: { access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + it 'deletes the member' do expect do delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer) @@ -679,13 +757,11 @@ RSpec.describe API::Members do end context 'adding owner to project' do - it 'returns created status' do - expect do - post api("/projects/#{project.id}/members", maintainer), - params: { user_id: stranger.id, access_level: Member::OWNER } + it 'returns 403' do + post api("/projects/#{project.id}/members", maintainer), + params: { user_id: stranger.id, access_level: Member::OWNER } - expect(response).to have_gitlab_http_status(:created) - end.to change { project.members.count }.by(1) + expect(response).to have_gitlab_http_status(:forbidden) end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index d2189ab02ea..431d2e56cb5 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -3106,6 +3106,13 @@ RSpec.describe API::Projects do expect(json_response['error']).to eq 'group_access does not have a valid value' end + it "returns a 400 error when the project-group share is created with an OWNER access level" do + post api("/projects/#{project.id}/share", user), params: { group_id: group.id, group_access: Gitlab::Access::OWNER } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq 'group_access does not have a valid value' + end + it "returns a 409 error when link is not saved" do allow(::Projects::GroupLinks::CreateService).to receive_message_chain(:new, :execute) .and_return({ status: :error, http_status: 409, message: 'error' }) diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb index 730175af0bb..e79e13af769 100644 --- a/spec/services/members/create_service_spec.rb +++ b/spec/services/members/create_service_spec.rb @@ -33,6 +33,18 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_ it 'raises a Gitlab::Access::AccessDeniedError' do expect { execute_service }.to raise_error(Gitlab::Access::AccessDeniedError) end + + context 'when a project maintainer attempts to add owners' do + let(:access_level) { Gitlab::Access::OWNER } + + before do + source.add_maintainer(current_user) + end + + it 'raises a Gitlab::Access::AccessDeniedError' do + expect { execute_service }.to raise_error(Gitlab::Access::AccessDeniedError) + end + end end context 'when passing an invalid source' do diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index 1a1283b1078..9f0daba3327 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -105,26 +105,46 @@ RSpec.describe Members::DestroyService do context 'with a project member' do let(:member) { group_project.members.find_by(user_id: member_user.id) } - before do - group_project.add_developer(member_user) + context 'when current user does not have any membership management permissions' do + before do + group_project.add_developer(member_user) + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' + + context 'when skipping authorisation' do + it_behaves_like 'a service destroying a member with access' do + let(:opts) { { skip_authorization: true, unassign_issuables: true } } + end + end end - it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' + context 'when a project maintainer tries to destroy a project owner' do + before do + group_project.add_owner(member_user) + end - it_behaves_like 'a service destroying a member with access' do - let(:opts) { { skip_authorization: true, unassign_issuables: true } } + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' + + context 'when skipping authorisation' do + it_behaves_like 'a service destroying a member with access' do + let(:opts) { { skip_authorization: true, unassign_issuables: true } } + end + end end end + end - context 'with a group member' do - let(:member) { group.members.find_by(user_id: member_user.id) } + context 'with a group member' do + let(:member) { group.members.find_by(user_id: member_user.id) } - before do - group.add_developer(member_user) - end + before do + group.add_developer(member_user) + end - it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' + context 'when skipping authorisation' do it_behaves_like 'a service destroying a member with access' do let(:opts) { { skip_authorization: true, unassign_issuables: true } } end diff --git a/spec/services/members/groups/bulk_creator_service_spec.rb b/spec/services/members/groups/bulk_creator_service_spec.rb index 0623ae00080..3922c37487c 100644 --- a/spec/services/members/groups/bulk_creator_service_spec.rb +++ b/spec/services/members/groups/bulk_creator_service_spec.rb @@ -3,8 +3,12 @@ require 'spec_helper' RSpec.describe Members::Groups::BulkCreatorService do + let_it_be(:source, reload: true) { create(:group, :public) } + let_it_be(:current_user) { create(:user) } + it_behaves_like 'bulk member creation' do - let_it_be(:source, reload: true) { create(:group, :public) } let_it_be(:member_type) { GroupMember } end + + it_behaves_like 'owner management' end diff --git a/spec/services/members/projects/bulk_creator_service_spec.rb b/spec/services/members/projects/bulk_creator_service_spec.rb index 7acb7d79fe7..dd998b47eb3 100644 --- a/spec/services/members/projects/bulk_creator_service_spec.rb +++ b/spec/services/members/projects/bulk_creator_service_spec.rb @@ -3,8 +3,12 @@ require 'spec_helper' RSpec.describe Members::Projects::BulkCreatorService do + let_it_be(:source, reload: true) { create(:project, :public) } + let_it_be(:current_user) { create(:user) } + it_behaves_like 'bulk member creation' do - let_it_be(:source, reload: true) { create(:project, :public) } let_it_be(:member_type) { ProjectMember } end + + it_behaves_like 'owner management' end diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb index a1b1397d444..f919d6d1516 100644 --- a/spec/services/members/update_service_spec.rb +++ b/spec/services/members/update_service_spec.rb @@ -9,8 +9,9 @@ RSpec.describe Members::UpdateService do let(:member_user) { create(:user) } let(:permission) { :update } let(:member) { source.members_and_requesters.find_by!(user_id: member_user.id) } + let(:access_level) { Gitlab::Access::MAINTAINER } let(:params) do - { access_level: Gitlab::Access::MAINTAINER } + { access_level: access_level } end subject { described_class.new(current_user, params).execute(member, permission: permission) } @@ -29,7 +30,7 @@ RSpec.describe Members::UpdateService do updated_member = subject.fetch(:member) expect(updated_member).to be_valid - expect(updated_member.access_level).to eq(Gitlab::Access::MAINTAINER) + expect(updated_member.access_level).to eq(access_level) end it 'returns success status' do @@ -111,4 +112,75 @@ RSpec.describe Members::UpdateService do let(:source) { group } end end + + context 'in a project' do + let_it_be(:group_project) { create(:project, group: create(:group)) } + + let(:source) { group_project } + + context 'a project maintainer' do + before do + group_project.add_maintainer(current_user) + end + + context 'cannot update a member to OWNER' do + before do + group_project.add_developer(member_user) + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:access_level) { Gitlab::Access::OWNER } + end + end + + context 'cannot update themselves to OWNER' do + let(:member) { source.members_and_requesters.find_by!(user_id: current_user.id) } + + before do + group_project.add_developer(member_user) + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:access_level) { Gitlab::Access::OWNER } + end + end + + context 'cannot downgrade a member from OWNER' do + before do + group_project.add_owner(member_user) + end + + it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do + let(:access_level) { Gitlab::Access::MAINTAINER } + end + end + end + + context 'owners' do + before do + # so that `current_user` is considered an `OWNER` in the project via inheritance. + group_project.group.add_owner(current_user) + end + + context 'can update a member to OWNER' do + before do + group_project.add_developer(member_user) + end + + it_behaves_like 'a service updating a member' do + let(:access_level) { Gitlab::Access::OWNER } + end + end + + context 'can downgrade a member from OWNER' do + before do + group_project.add_owner(member_user) + end + + it_behaves_like 'a service updating a member' do + let(:access_level) { Gitlab::Access::MAINTAINER } + end + end + end + end end diff --git a/spec/support/graphql/resolver_factories.rb b/spec/support/graphql/resolver_factories.rb index 8188f17cc43..3c5aad34e8b 100644 --- a/spec/support/graphql/resolver_factories.rb +++ b/spec/support/graphql/resolver_factories.rb @@ -15,8 +15,8 @@ module Graphql private - def simple_resolver(resolved_value = 'Resolved value') - Class.new(Resolvers::BaseResolver) do + def simple_resolver(resolved_value = 'Resolved value', base_class: Resolvers::BaseResolver) + Class.new(base_class) do define_method :resolve do |**_args| resolved_value end diff --git a/spec/support/helpers/doc_url_helper.rb b/spec/support/helpers/doc_url_helper.rb new file mode 100644 index 00000000000..bbff4827c56 --- /dev/null +++ b/spec/support/helpers/doc_url_helper.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module DocUrlHelper + def version + "13.4.0-ee" + end + + def doc_url(documentation_base_url) + "#{documentation_base_url}/13.4/ee/#{path}.html" + end + + def doc_url_without_version(documentation_base_url) + "#{documentation_base_url}/ee/#{path}.html" + end + + def stub_doc_file_read(file_name: 'index.md', content: ) + expect_file_read(File.join(Rails.root, 'doc', file_name), content: content) + end +end + +DocUrlHelper.prepend_mod diff --git a/spec/support/matchers/exceed_query_limit.rb b/spec/support/matchers/exceed_query_limit.rb index b48c7f905b2..23f43e05a10 100644 --- a/spec/support/matchers/exceed_query_limit.rb +++ b/spec/support/matchers/exceed_query_limit.rb @@ -323,7 +323,12 @@ RSpec::Matchers.define :exceed_query_limit do |expected| include ExceedQueryLimitHelpers match do |block| - verify_count(&block) + if block.is_a?(ActiveRecord::QueryRecorder) + @recorder = block + verify_count + else + verify_count(&block) + end end failure_message_when_negated do |actual| diff --git a/spec/support/shared_contexts/policies/project_policy_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_shared_context.rb index e50083a10e7..7396643823c 100644 --- a/spec/support/shared_contexts/policies/project_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/project_policy_shared_context.rb @@ -75,7 +75,7 @@ RSpec.shared_context 'ProjectPolicy context' do let(:base_owner_permissions) do %i[ archive_project change_namespace change_visibility_level destroy_issue - destroy_merge_request remove_fork_project remove_project rename_project + destroy_merge_request manage_owners remove_fork_project remove_project rename_project set_issue_created_at set_issue_iid set_issue_updated_at set_note_created_at ] diff --git a/spec/support/shared_examples/models/member_shared_examples.rb b/spec/support/shared_examples/models/member_shared_examples.rb index e293d10964b..0537c3b7327 100644 --- a/spec/support/shared_examples/models/member_shared_examples.rb +++ b/spec/support/shared_examples/models/member_shared_examples.rb @@ -401,6 +401,15 @@ RSpec.shared_examples_for "bulk member creation" do let_it_be(:user1) { create(:user) } let_it_be(:user2) { create(:user) } + context 'when current user does not have permission' do + it 'does not succeed' do + # maintainers cannot add owners + source.add_maintainer(user) + + expect(described_class.add_users(source, [user1, user2], :owner, current_user: user)).to be_empty + end + end + it 'returns a Member objects' do members = described_class.add_users(source, [user1, user2], :maintainer) @@ -546,3 +555,29 @@ RSpec.shared_examples_for "bulk member creation" do end end end + +RSpec.shared_examples 'owner management' do + describe '.cannot_manage_owners?' do + subject { described_class.cannot_manage_owners?(source, current_user) } + + context 'when maintainer' do + before do + source.add_maintainer(current_user) + end + + it 'cannot manage owners' do + expect(subject).to be_truthy + end + end + + context 'when owner' do + before do + source.add_owner(current_user) + end + + it 'can manage owners' do + expect(subject).to be_falsey + end + end + end +end diff --git a/spec/support/shared_examples/models/members_notifications_shared_example.rb b/spec/support/shared_examples/models/members_notifications_shared_example.rb index 04af3935d15..75eed0203a7 100644 --- a/spec/support/shared_examples/models/members_notifications_shared_example.rb +++ b/spec/support/shared_examples/models/members_notifications_shared_example.rb @@ -33,6 +33,18 @@ RSpec.shared_examples 'members notifications' do |entity_type| end end + describe '#after_commit' do + context 'on creation of a member requesting access' do + let(:member) { build(:"#{entity_type}_member", :access_request) } + + it "calls NotificationService.new_access_request" do + expect(notification_service).to receive(:new_access_request).with(member) + + member.save! + end + end + end + describe '#accept_request' do let(:member) { create(:"#{entity_type}_member", :access_request) } diff --git a/spec/support/shared_examples/requests/api/project_statistics_refresh_conflicts_shared_examples.rb b/spec/support/shared_examples/requests/api/project_statistics_refresh_conflicts_shared_examples.rb new file mode 100644 index 00000000000..7c3f4781472 --- /dev/null +++ b/spec/support/shared_examples/requests/api/project_statistics_refresh_conflicts_shared_examples.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'preventing request because of ongoing project stats refresh' do |entrypoint| + before do + create(:project_build_artifacts_size_refresh, :pending, project: project) + end + + it 'logs about the rejected request' do + expect(Gitlab::ProjectStatsRefreshConflictsLogger) + .to receive(:warn_request_rejected_during_stats_refresh) + .with(project.id) + + make_request + end + + it 'returns 409 error' do + make_request + + expect(response).to have_gitlab_http_status(:conflict) + end +end |