diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-11-30 15:10:26 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-11-30 15:10:26 +0300 |
commit | 19c9422e1f3792680aa3f9e6190218b31a838fe3 (patch) | |
tree | 36deaed9777d38e0acea456d3aacaf5b5f800958 | |
parent | 915a5b6e89195aab5d7eb0deb16e4825cfce509e (diff) |
Add latest changes from gitlab-org/gitlab@master
28 files changed, 960 insertions, 676 deletions
diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index efc65431d50..aaa486fce68 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -19,6 +19,9 @@ .if-default-branch-refs: &if-default-branch-refs if: '$CI_COMMIT_REF_NAME == $CI_DEFAULT_BRANCH' +.if-stable-branch-refs: &if-stable-branch-refs + if: '$CI_COMMIT_REF_NAME =~ /^[\d-]+-stable(-ee)?$/' + .if-default-branch-push: &if-default-branch-push if: '$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH && $CI_PIPELINE_SOURCE == "push"' @@ -581,6 +584,8 @@ when: never - <<: *if-merge-request-targeting-stable-branch when: never + - <<: *if-stable-branch-refs + when: never - <<: *if-merge-request-labels-as-if-jh - <<: *if-merge-request-labels-run-all-rspec - changes: *code-backstage-qa-patterns @@ -616,6 +621,8 @@ when: never - <<: *if-merge-request-targeting-stable-branch when: never + - <<: *if-stable-branch-refs + when: never - <<: *if-merge-request-labels-as-if-jh - <<: *if-merge-request-labels-run-all-rspec - <<: *if-merge-request @@ -1259,6 +1266,8 @@ when: never - <<: *if-merge-request-targeting-stable-branch when: never + - <<: *if-stable-branch-refs + when: never - <<: *if-merge-request-labels-as-if-jh allow_failure: true - <<: *if-merge-request @@ -1714,6 +1723,8 @@ when: never - <<: *if-merge-request-targeting-stable-branch when: never + - <<: *if-stable-branch-refs + when: never - <<: *if-merge-request-labels-as-if-jh - <<: *if-merge-request-labels-run-all-rspec - changes: *code-backstage-qa-patterns diff --git a/app/assets/javascripts/behaviors/index.js b/app/assets/javascripts/behaviors/index.js index bfd025e8dab..30160248a77 100644 --- a/app/assets/javascripts/behaviors/index.js +++ b/app/assets/javascripts/behaviors/index.js @@ -1,6 +1,5 @@ import $ from 'jquery'; import './autosize'; -import './bind_in_out'; import './markdown/render_gfm'; import initCollapseSidebarOnWindowResize from './collapse_sidebar_on_window_resize'; import initCopyToClipboard from './copy_to_clipboard'; diff --git a/app/assets/javascripts/boards/boards_util.js b/app/assets/javascripts/boards/boards_util.js index e6c91c7ac1f..e1f5d26cbee 100644 --- a/app/assets/javascripts/boards/boards_util.js +++ b/app/assets/javascripts/boards/boards_util.js @@ -1,6 +1,6 @@ import { sortBy, cloneDeep } from 'lodash'; import { isGid } from '~/graphql_shared/utils'; -import { ListType, MilestoneIDs } from './constants'; +import { ListType, MilestoneIDs, AssigneeFilterType, MilestoneFilterType } from './constants'; export function getMilestone() { return null; @@ -186,6 +186,7 @@ export function isListDraggable(list) { export const FiltersInfo = { assigneeUsername: { negatedSupport: true, + remap: (k, v) => (v === AssigneeFilterType.any ? 'assigneeWildcardId' : k), }, assigneeId: { // assigneeId should be renamed to assigneeWildcardId. @@ -204,6 +205,11 @@ export const FiltersInfo = { }, milestoneTitle: { negatedSupport: true, + remap: (k, v) => (Object.values(MilestoneFilterType).includes(v) ? 'milestoneWildcardId' : k), + }, + milestoneWildcardId: { + negatedSupport: true, + transform: (val) => val.toUpperCase(), }, myReactionEmoji: { negatedSupport: true, diff --git a/app/assets/javascripts/boards/components/board_filtered_search.vue b/app/assets/javascripts/boards/components/board_filtered_search.vue index f66bc7887dc..3da50c89fc7 100644 --- a/app/assets/javascripts/boards/components/board_filtered_search.vue +++ b/app/assets/javascripts/boards/components/board_filtered_search.vue @@ -1,7 +1,7 @@ <script> import { pickBy, isEmpty } from 'lodash'; import { mapActions } from 'vuex'; -import { getIdFromGraphQLId } from '~/graphql_shared/utils'; +import { getIdFromGraphQLId, isGid } from '~/graphql_shared/utils'; import { updateHistory, setUrlParams } from '~/lib/utils/url_utility'; import { __ } from '~/locale'; import { FILTERED_SEARCH_TERM } from '~/vue_shared/components/filtered_search_bar/constants'; @@ -80,7 +80,7 @@ export default { if (milestoneTitle) { filteredSearchValue.push({ - type: 'milestone_title', + type: 'milestone', value: { data: milestoneTitle, operator: '=' }, }); } @@ -129,7 +129,7 @@ export default { if (this.filterParams['not[milestoneTitle]']) { filteredSearchValue.push({ - type: 'milestone_title', + type: 'milestone', value: { data: this.filterParams['not[milestoneTitle]'], operator: '!=' }, }); } @@ -242,7 +242,7 @@ export default { search, types, weight, - epic_id: getIdFromGraphQLId(epicId), + epic_id: isGid(epicId) ? getIdFromGraphQLId(epicId) : epicId, my_reaction_emoji: myReactionEmoji, release_tag: releaseTag, }; @@ -293,7 +293,7 @@ export default { case 'label_name': labels.push(filter.value.data); break; - case 'milestone_title': + case 'milestone': filterParams.milestoneTitle = filter.value.data; break; case 'iteration': @@ -326,6 +326,7 @@ export default { if (plainText.length) { filterParams.search = plainText.join(' '); } + return filterParams; }, }, diff --git a/app/assets/javascripts/boards/components/issue_board_filtered_search.vue b/app/assets/javascripts/boards/components/issue_board_filtered_search.vue index b5270c9d5fa..6e786932ca8 100644 --- a/app/assets/javascripts/boards/components/issue_board_filtered_search.vue +++ b/app/assets/javascripts/boards/components/issue_board_filtered_search.vue @@ -11,7 +11,6 @@ import { TYPE_USER } from '~/graphql_shared/constants'; import { convertToGraphQLId } from '~/graphql_shared/utils'; import { __ } from '~/locale'; import { - DEFAULT_MILESTONES_GRAPHQL, TOKEN_TITLE_MY_REACTION, OPERATOR_IS_AND_IS_NOT, } from '~/vue_shared/components/filtered_search_bar/constants'; @@ -136,13 +135,12 @@ export default { ] : []), { - type: 'milestone_title', + type: 'milestone', title: milestone, icon: 'clock', symbol: '%', token: MilestoneToken, unique: true, - defaultMilestones: DEFAULT_MILESTONES_GRAPHQL, fetchMilestones: this.fetchMilestones, }, { diff --git a/app/assets/javascripts/boards/constants.js b/app/assets/javascripts/boards/constants.js index 391e0d1fb0a..8443786fb29 100644 --- a/app/assets/javascripts/boards/constants.js +++ b/app/assets/javascripts/boards/constants.js @@ -106,6 +106,7 @@ export const FilterFields = { 'authorUsername', 'labelName', 'milestoneTitle', + 'milestoneWildcardId', 'myReactionEmoji', 'releaseTag', 'search', @@ -114,6 +115,18 @@ export const FilterFields = { ], }; +/* eslint-disable @gitlab/require-i18n-strings */ +export const AssigneeFilterType = { + any: 'Any', +}; + +export const MilestoneFilterType = { + any: 'Any', + none: 'None', + started: 'Started', + upcoming: 'Upcoming', +}; + export const DraggableItemTypes = { card: 'card', list: 'list', diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.vue index c314261d3f5..730d11b1208 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.vue @@ -1,9 +1,13 @@ <script> +import { GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import { s__, n__ } from '~/locale'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; export default { name: 'MRWidgetRelatedLinks', + directives: { + SafeHtml, + }, mixins: [glFeatureFlagMixin()], props: { relatedLinks: { @@ -43,14 +47,14 @@ export default { :class="{ 'gl-display-line gl-m-0': glFeatures.restructuredMrWidget }" > {{ closesText }} - <span v-html="relatedLinks.closing /* eslint-disable-line vue/no-v-html */"></span> + <span v-safe-html="relatedLinks.closing"></span> </p> <p v-if="relatedLinks.mentioned" :class="{ 'gl-display-line gl-m-0': glFeatures.restructuredMrWidget }" > {{ n__('mrWidget|Mentions issue', 'mrWidget|Mentions issues', relatedLinks.mentionedCount) }} - <span v-html="relatedLinks.mentioned /* eslint-disable-line vue/no-v-html */"></span> + <span v-safe-html="relatedLinks.mentioned"></span> </p> <p v-if="relatedLinks.assignToMe && showAssignToMe" diff --git a/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js b/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js index c881d3cf6a4..810d9f782b9 100644 --- a/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js +++ b/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js @@ -28,13 +28,6 @@ export const DEFAULT_MILESTONES = DEFAULT_NONE_ANY.concat([ { value: FILTER_STARTED, text: __('Started'), title: __('Started') }, ]); -export const DEFAULT_MILESTONES_GRAPHQL = [ - { value: 'any', text: __('Any'), title: __('Any') }, - { value: 'none', text: __('None'), title: __('None') }, - { value: '#upcoming', text: __('Upcoming'), title: __('Upcoming') }, - { value: '#started', text: __('Started'), title: __('Started') }, -]; - export const SortDirection = { descending: 'descending', ascending: 'ascending', diff --git a/app/services/ci/update_pending_build_service.rb b/app/services/ci/update_pending_build_service.rb index d546dbcfe3d..0fa43b1edb7 100644 --- a/app/services/ci/update_pending_build_service.rb +++ b/app/services/ci/update_pending_build_service.rb @@ -9,7 +9,7 @@ module Ci def initialize(model, update_params) @model = model - @update_params = update_params + @update_params = update_params.symbolize_keys validations! end diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index cd89eb799dc..f0582106df5 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -29,11 +29,11 @@ module Groups update_group_attributes ensure_ownership update_integrations - update_pending_builds! end post_update_hooks(@updated_project_ids) propagate_integrations + update_pending_builds true end @@ -228,13 +228,19 @@ module Groups end end - def update_pending_builds! - update_params = { + def update_pending_builds + if Feature.enabled?(:ci_pending_builds_async_update, default_enabled: :yaml) + ::Ci::PendingBuilds::UpdateGroupWorker.perform_async(group.id, pending_builds_params) + else + ::Ci::UpdatePendingBuildService.new(group, pending_builds_params).execute + end + end + + def pending_builds_params + { namespace_traversal_ids: group.traversal_ids, namespace_id: group.id } - - ::Ci::UpdatePendingBuildService.new(group, update_params).execute end end end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index a69e6488ebc..66b07d58bbd 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -104,10 +104,10 @@ module Projects update_repository_configuration(@new_path) execute_system_hooks - - update_pending_builds! end + update_pending_builds + post_update_hooks(project) rescue Exception # rubocop:disable Lint/RescueException rollback_side_effects @@ -244,13 +244,19 @@ module Projects Integration.create_from_active_default_integrations(project, :project_id) end - def update_pending_builds! - update_params = { + def update_pending_builds + if Feature.enabled?(:ci_pending_builds_async_update, default_enabled: :yaml) + ::Ci::PendingBuilds::UpdateProjectWorker.perform_async(project.id, pending_builds_params) + else + ::Ci::UpdatePendingBuildService.new(project, pending_builds_params).execute + end + end + + def pending_builds_params + { namespace_id: new_namespace.id, namespace_traversal_ids: new_namespace.traversal_ids } - - ::Ci::UpdatePendingBuildService.new(project, update_params).execute end end end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 733a878907c..fbec46dbc2f 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -1447,6 +1447,24 @@ :weight: 1 :idempotent: true :tags: [] +- :name: pipeline_background:ci_pending_builds_update_group + :worker_name: Ci::PendingBuilds::UpdateGroupWorker + :feature_category: :continuous_integration + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] +- :name: pipeline_background:ci_pending_builds_update_project + :worker_name: Ci::PendingBuilds::UpdateProjectWorker + :feature_category: :continuous_integration + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: pipeline_background:ci_pipeline_artifacts_coverage_report :worker_name: Ci::PipelineArtifacts::CoverageReportWorker :feature_category: :code_testing diff --git a/app/workers/ci/pending_builds/update_group_worker.rb b/app/workers/ci/pending_builds/update_group_worker.rb new file mode 100644 index 00000000000..3ee3a9116d8 --- /dev/null +++ b/app/workers/ci/pending_builds/update_group_worker.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Ci + module PendingBuilds + class UpdateGroupWorker + include ApplicationWorker + include PipelineBackgroundQueue + + data_consistency :always + idempotent! + + def perform(group_id, update_params) + ::Group.find_by_id(group_id).try do |group| + ::Ci::UpdatePendingBuildService.new(group, update_params).execute + end + end + end + end +end diff --git a/app/workers/ci/pending_builds/update_project_worker.rb b/app/workers/ci/pending_builds/update_project_worker.rb new file mode 100644 index 00000000000..bac0316c80b --- /dev/null +++ b/app/workers/ci/pending_builds/update_project_worker.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Ci + module PendingBuilds + class UpdateProjectWorker + include ApplicationWorker + include PipelineBackgroundQueue + + data_consistency :always + idempotent! + + def perform(project_id, update_params) + ::Project.find_by_id(project_id).try do |project| + ::Ci::UpdatePendingBuildService.new(project, update_params).execute + end + end + end + end +end diff --git a/config/feature_flags/development/ci_pending_builds_async_update.yml b/config/feature_flags/development/ci_pending_builds_async_update.yml new file mode 100644 index 00000000000..3871d2a8bca --- /dev/null +++ b/config/feature_flags/development/ci_pending_builds_async_update.yml @@ -0,0 +1,8 @@ +--- +name: ci_pending_builds_async_update +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75197 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/346641 +milestone: '14.6' +type: development +group: group::pipeline execution +default_enabled: false diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5e0b39c88b9..67fef35eaa7 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6745,6 +6745,9 @@ msgstr "" msgid "Checkout|%{name}'s storage subscription" msgstr "" +msgid "Checkout|%{quantity} CI minutes" +msgstr "" + msgid "Checkout|%{quantity} GB of storage" msgstr "" @@ -6759,9 +6762,6 @@ msgstr "" msgid "Checkout|%{startDate} - %{endDate}" msgstr "" -msgid "Checkout|%{totalCiMinutes} CI minutes" -msgstr "" - msgid "Checkout|(may be %{linkStart}charged upon purchase%{linkEnd})" msgstr "" diff --git a/spec/features/boards/board_filters_spec.rb b/spec/features/boards/board_filters_spec.rb index 5eb8f43df07..5215909c0da 100644 --- a/spec/features/boards/board_filters_spec.rb +++ b/spec/features/boards/board_filters_spec.rb @@ -30,9 +30,7 @@ RSpec.describe 'Issue board filters', :js do describe 'filters by releases' do before do - filter_input.click - filter_input.set('release:') - filter_first_suggestion.click # Select `=` operator + set_filter('release') end it 'loads all the releases when opened and submit one as filter', :aggregate_failures do @@ -47,6 +45,35 @@ RSpec.describe 'Issue board filters', :js do end end + describe 'filters by milestone' do + before do + set_filter('milestone') + end + + it 'loads all the milestones when opened and submit one as filter', :aggregate_failures do + expect(find('.board:nth-child(1)')).to have_selector('.board-card', count: 2) + + expect_filtered_search_dropdown_results(filter_dropdown, 6) + expect(filter_dropdown).to have_content('None') + expect(filter_dropdown).to have_content('Any') + expect(filter_dropdown).to have_content('Started') + expect(filter_dropdown).to have_content('Upcoming') + expect(filter_dropdown).to have_content(milestone_1.title) + expect(filter_dropdown).to have_content(milestone_2.title) + + click_on milestone_1.title + filter_submit.click + + expect(find('.board:nth-child(1)')).to have_selector('.board-card', count: 1) + end + end + + def set_filter(filter) + filter_input.click + filter_input.set("#{filter}:") + filter_first_suggestion.click # Select `=` operator + end + def expect_filtered_search_dropdown_results(filter_dropdown, count) expect(filter_dropdown).to have_selector('.gl-new-dropdown-item', count: count) end diff --git a/spec/frontend/boards/components/board_filtered_search_spec.js b/spec/frontend/boards/components/board_filtered_search_spec.js index 65a3ed2f788..50f3a7e96d9 100644 --- a/spec/frontend/boards/components/board_filtered_search_spec.js +++ b/spec/frontend/boards/components/board_filtered_search_spec.js @@ -120,7 +120,7 @@ describe('BoardFilteredSearch', () => { { type: 'author_username', value: { data: 'root', operator: '=' } }, { type: 'label_name', value: { data: 'label', operator: '=' } }, { type: 'label_name', value: { data: 'label2', operator: '=' } }, - { type: 'milestone_title', value: { data: 'New Milestone', operator: '=' } }, + { type: 'milestone', value: { data: 'New Milestone', operator: '=' } }, { type: 'types', value: { data: 'INCIDENT', operator: '=' } }, { type: 'weight', value: { data: '2', operator: '=' } }, { type: 'iteration', value: { data: '3341', operator: '=' } }, diff --git a/spec/frontend/boards/mock_data.js b/spec/frontend/boards/mock_data.js index 035f0ac2915..4001fe5cdf8 100644 --- a/spec/frontend/boards/mock_data.js +++ b/spec/frontend/boards/mock_data.js @@ -2,7 +2,6 @@ import { GlFilteredSearchToken } from '@gitlab/ui'; import { keyBy } from 'lodash'; import { ListType } from '~/boards/constants'; import { __ } from '~/locale'; -import { DEFAULT_MILESTONES_GRAPHQL } from '~/vue_shared/components/filtered_search_bar/constants'; import AuthorToken from '~/vue_shared/components/filtered_search_bar/tokens/author_token.vue'; import EmojiToken from '~/vue_shared/components/filtered_search_bar/tokens/emoji_token.vue'; import LabelToken from '~/vue_shared/components/filtered_search_bar/tokens/label_token.vue'; @@ -599,10 +598,9 @@ export const mockTokens = (fetchLabels, fetchAuthors, fetchMilestones, hasEmoji) icon: 'clock', title: __('Milestone'), symbol: '%', - type: 'milestone_title', + type: 'milestone', token: MilestoneToken, unique: true, - defaultMilestones: DEFAULT_MILESTONES_GRAPHQL, fetchMilestones, }, { diff --git a/spec/frontend/vue_shared/components/filtered_search_bar/tokens/milestone_token_spec.js b/spec/frontend/vue_shared/components/filtered_search_bar/tokens/milestone_token_spec.js index 936841651d1..81ec2622f4c 100644 --- a/spec/frontend/vue_shared/components/filtered_search_bar/tokens/milestone_token_spec.js +++ b/spec/frontend/vue_shared/components/filtered_search_bar/tokens/milestone_token_spec.js @@ -11,10 +11,7 @@ import createFlash from '~/flash'; import axios from '~/lib/utils/axios_utils'; import { sortMilestonesByDueDate } from '~/milestones/milestone_utils'; -import { - DEFAULT_MILESTONES, - DEFAULT_MILESTONES_GRAPHQL, -} from '~/vue_shared/components/filtered_search_bar/constants'; +import { DEFAULT_MILESTONES } from '~/vue_shared/components/filtered_search_bar/constants'; import MilestoneToken from '~/vue_shared/components/filtered_search_bar/tokens/milestone_token.vue'; import { mockMilestoneToken, mockMilestones, mockRegularMilestone } from '../mock_data'; @@ -199,12 +196,12 @@ describe('MilestoneToken', () => { beforeEach(() => { wrapper = createComponent({ active: true, - config: { ...mockMilestoneToken, defaultMilestones: DEFAULT_MILESTONES_GRAPHQL }, + config: { ...mockMilestoneToken, defaultMilestones: DEFAULT_MILESTONES }, }); }); it('finds the correct value from the activeToken', () => { - DEFAULT_MILESTONES_GRAPHQL.forEach(({ value, title }) => { + DEFAULT_MILESTONES.forEach(({ value, title }) => { const activeToken = wrapper.vm.getActiveMilestone([], value); expect(activeToken.title).toEqual(title); diff --git a/spec/requests/api/ci/job_artifacts_spec.rb b/spec/requests/api/ci/job_artifacts_spec.rb new file mode 100644 index 00000000000..8cc8521a23a --- /dev/null +++ b/spec/requests/api/ci/job_artifacts_spec.rb @@ -0,0 +1,629 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Ci::JobArtifacts do + include HttpBasicAuthHelpers + include DependencyProxyHelpers + + include HttpIOHelpers + + let_it_be(:project, reload: true) do + create(:project, :repository, public_builds: false) + end + + let_it_be(:pipeline, reload: true) do + create(:ci_pipeline, project: project, + sha: project.commit.id, + ref: project.default_branch) + end + + let(:user) { create(:user) } + let(:api_user) { user } + let(:reporter) { create(:project_member, :reporter, project: project).user } + let(:guest) { create(:project_member, :guest, project: project).user } + + let!(:job) do + create(:ci_build, :success, :tags, pipeline: pipeline, + artifacts_expire_at: 1.day.since) + end + + before do + project.add_developer(user) + end + + shared_examples 'returns unauthorized' do + it 'returns unauthorized' do + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + 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 user is anonymous' do + let(:api_user) { nil } + + 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) + end + end + + 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 + + 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 + + describe 'GET /projects/:id/jobs/:job_id/artifacts/:artifact_path' do + context 'when job has artifacts' do + let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) } + + let(:artifact) do + 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' + end + + context 'when user is anonymous' do + let(:api_user) { nil } + + context 'when project is public' do + it 'allows to access artifacts' do + project.update_column(:visibility_level, + Gitlab::VisibilityLevel::PUBLIC) + project.update_column(:public_builds, true) + + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when project is public with artifacts that are non public' do + let(:job) { create(:ci_build, :artifacts, :non_public_artifacts, pipeline: pipeline) } + + it 'rejects access to artifacts' do + project.update_column(:visibility_level, + Gitlab::VisibilityLevel::PUBLIC) + project.update_column(:public_builds, true) + + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + context 'with the non_public_artifacts feature flag disabled' do + before do + stub_feature_flags(non_public_artifacts: false) + end + + it 'allows access to artifacts' do + project.update_column(:visibility_level, + Gitlab::VisibilityLevel::PUBLIC) + project.update_column(:public_builds, true) + + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + context 'when project is public with builds access disabled' do + it 'rejects access to artifacts' do + project.update_column(:visibility_level, + Gitlab::VisibilityLevel::PUBLIC) + project.update_column(:public_builds, false) + + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when project is private' do + it 'rejects access and hides existence of artifacts' do + project.update_column(:visibility_level, + Gitlab::VisibilityLevel::PRIVATE) + project.update_column(:public_builds, true) + + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when user is authorized' do + it 'returns a specific artifact file for a valid path' do + expect(Gitlab::Workhorse) + .to receive(:send_artifacts_entry) + .and_call_original + + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers.to_h) + .to include('Content-Type' => 'application/json', + 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + expect(response.parsed_body).to be_empty + end + + context 'when artifacts are locked' do + it 'allows access to expired artifact' do + pipeline.artifacts_locked! + job.update!(artifacts_expire_at: Time.now - 7.days) + + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + end + + context 'when job does not have artifacts' do + it 'does not return job artifact file' do + get_artifact_file('some/artifact') + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + def get_artifact_file(artifact_path) + get api("/projects/#{project.id}/jobs/#{job.id}/" \ + "artifacts/#{artifact_path}", api_user) + end + end + + describe 'GET /projects/:id/jobs/:job_id/artifacts' do + shared_examples 'downloads artifact' do + let(:download_headers) do + { 'Content-Transfer-Encoding' => 'binary', + 'Content-Disposition' => %q(attachment; filename="ci_build_artifacts.zip"; filename*=UTF-8''ci_build_artifacts.zip) } + end + + it 'returns specific job artifacts' do + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers.to_h).to include(download_headers) + expect(response.body).to match_file(job.artifacts_file.file.file) + end + end + + context 'normal authentication' do + context 'job with artifacts' do + context 'when artifacts are stored locally' do + let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) } + + before do + get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) + end + + context 'authorized user' do + it_behaves_like 'downloads artifact' + end + + context 'unauthorized user' do + let(:api_user) { nil } + + it 'does not return specific job artifacts' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when artifacts are stored remotely' do + let(:proxy_download) { false } + let(:job) { create(:ci_build, pipeline: pipeline) } + let(:artifact) { create(:ci_job_artifact, :archive, :remote_store, job: job) } + + before do + stub_artifacts_object_storage(proxy_download: proxy_download) + + artifact + job.reload + + get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) + end + + context 'when proxy download is enabled' do + let(:proxy_download) { true } + + it 'responds with the workhorse send-url' do + expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("send-url:") + end + end + + context 'when proxy download is disabled' do + it 'returns location redirect' do + expect(response).to have_gitlab_http_status(:found) + end + end + + context 'authorized user' do + it 'returns the file remote URL' do + expect(response).to redirect_to(artifact.file.url) + end + end + + context 'unauthorized user' do + let(:api_user) { nil } + + it 'does not return specific job artifacts' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when public project guest and artifacts are non public' do + let(:api_user) { guest } + let(:job) { create(:ci_build, :artifacts, :non_public_artifacts, pipeline: pipeline) } + + before do + project.update_column(:visibility_level, + Gitlab::VisibilityLevel::PUBLIC) + project.update_column(:public_builds, true) + get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) + end + + it 'rejects access and hides existence of artifacts' do + expect(response).to have_gitlab_http_status(:forbidden) + end + + context 'with the non_public_artifacts feature flag disabled' do + before do + stub_feature_flags(non_public_artifacts: false) + get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) + end + + it 'allows access to artifacts' do + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + it 'does not return job artifacts if not uploaded' do + get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + + describe 'GET /projects/:id/artifacts/:ref_name/download?job=name' do + let(:api_user) { reporter } + let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } + + before do + stub_artifacts_object_storage + job.success + end + + def get_for_ref(ref = pipeline.ref, job_name = job.name) + get api("/projects/#{project.id}/jobs/artifacts/#{ref}/download", api_user), params: { job: job_name } + end + + context 'when not logged in' do + let(:api_user) { nil } + + before do + get_for_ref + end + + it 'does not find a resource in a private project' do + expect(project).to be_private + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when logging as guest' do + let(:api_user) { guest } + + before do + get_for_ref + end + + it 'gives 403' do + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'non-existing job' do + shared_examples 'not found' do + it { expect(response).to have_gitlab_http_status(:not_found) } + end + + context 'has no such ref' do + before do + get_for_ref('TAIL') + end + + it_behaves_like 'not found' + end + + context 'has no such job' do + before do + get_for_ref(pipeline.ref, 'NOBUILD') + end + + it_behaves_like 'not found' + end + end + + context 'find proper job' do + let(:job_with_artifacts) { job } + + shared_examples 'a valid file' do + context 'when artifacts are stored locally', :sidekiq_might_not_need_inline do + let(:download_headers) do + { 'Content-Transfer-Encoding' => 'binary', + 'Content-Disposition' => + %Q(attachment; filename="#{job_with_artifacts.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename}) } + end + + it { expect(response).to have_gitlab_http_status(:ok) } + it { expect(response.headers.to_h).to include(download_headers) } + end + + context 'when artifacts are stored remotely' do + let(:job) { create(:ci_build, pipeline: pipeline, user: api_user) } + let!(:artifact) { create(:ci_job_artifact, :archive, :remote_store, job: job) } + + before do + job.reload + + get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) + end + + it 'returns location redirect' do + expect(response).to have_gitlab_http_status(:found) + end + end + end + + context 'with regular branch' do + before do + pipeline.reload + pipeline.update!(ref: 'master', + sha: project.commit('master').sha) + + get_for_ref('master') + end + + it_behaves_like 'a valid file' + end + + context 'with branch name containing slash' do + before do + pipeline.reload + pipeline.update!(ref: 'improve/awesome', sha: project.commit('improve/awesome').sha) + get_for_ref('improve/awesome') + end + + it_behaves_like 'a valid file' + end + + context 'with job name in a child pipeline' do + let(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) } + let!(:child_job) { create(:ci_build, :artifacts, :success, name: 'rspec', pipeline: child_pipeline) } + let(:job_with_artifacts) { child_job } + + before do + get_for_ref('master', child_job.name) + end + + it_behaves_like 'a valid file' + end + end + end + + describe 'GET id/jobs/artifacts/:ref_name/raw/*artifact_path?job=name' do + context 'when job has artifacts' do + let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } + let(:artifact) { 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' } + let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } + let(:public_builds) { true } + + before do + stub_artifacts_object_storage + job.success + + project.update!(visibility_level: visibility_level, + public_builds: public_builds) + + get_artifact_file(artifact) + end + + context 'when user is anonymous' do + let(:api_user) { nil } + + context 'when project is public' do + let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } + let(:public_builds) { true } + + it 'allows to access artifacts', :sidekiq_might_not_need_inline do + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers.to_h) + .to include('Content-Type' => 'application/json', + 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + end + end + + context 'when project is public with builds access disabled' do + let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } + let(:public_builds) { false } + + it 'rejects access to artifacts' do + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response).to have_key('message') + expect(response.headers.to_h) + .not_to include('Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + end + end + + context 'when project is public with non public artifacts' do + let(:job) { create(:ci_build, :artifacts, :non_public_artifacts, pipeline: pipeline, user: api_user) } + let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } + let(:public_builds) { true } + + it 'rejects access and hides existence of artifacts', :sidekiq_might_not_need_inline do + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response).to have_key('message') + expect(response.headers.to_h) + .not_to include('Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + end + + context 'with the non_public_artifacts feature flag disabled' do + before do + stub_feature_flags(non_public_artifacts: false) + end + + it 'allows access to artifacts', :sidekiq_might_not_need_inline do + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + context 'when project is private' do + let(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE } + let(:public_builds) { true } + + it 'rejects access and hides existence of artifacts' do + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response).to have_key('message') + expect(response.headers.to_h) + .not_to include('Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + end + end + end + + context 'when user is authorized' do + let(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE } + let(:public_builds) { true } + + it 'returns a specific artifact file for a valid path', :sidekiq_might_not_need_inline do + expect(Gitlab::Workhorse) + .to receive(:send_artifacts_entry) + .and_call_original + + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers.to_h) + .to include('Content-Type' => 'application/json', + 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + expect(response.parsed_body).to be_empty + end + end + + context 'with branch name containing slash' do + before do + pipeline.reload + pipeline.update!(ref: 'improve/awesome', + sha: project.commit('improve/awesome').sha) + end + + it 'returns a specific artifact file for a valid path', :sidekiq_might_not_need_inline do + get_artifact_file(artifact, 'improve/awesome') + + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers.to_h) + .to include('Content-Type' => 'application/json', + 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) + end + end + + context 'non-existing job' do + shared_examples 'not found' do + it { expect(response).to have_gitlab_http_status(:not_found) } + end + + context 'has no such ref' do + before do + get_artifact_file('some/artifact', 'wrong-ref') + end + + it_behaves_like 'not found' + end + + context 'has no such job' do + before do + get_artifact_file('some/artifact', pipeline.ref, 'wrong-job-name') + end + + it_behaves_like 'not found' + end + end + end + + context 'when job does not have artifacts' do + let(:job) { create(:ci_build, pipeline: pipeline, user: api_user) } + + it 'does not return job artifact file' do + get_artifact_file('some/artifact') + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + def get_artifact_file(artifact_path, ref = pipeline.ref, job_name = job.name) + get api("/projects/#{project.id}/jobs/artifacts/#{ref}/raw/#{artifact_path}", api_user), params: { job: job_name } + end + end + + describe 'POST /projects/:id/jobs/:job_id/artifacts/keep' do + before do + post api("/projects/#{project.id}/jobs/#{job.id}/artifacts/keep", user) + end + + context 'artifacts did not expire' do + let(:job) do + create(:ci_build, :trace_artifact, :artifacts, :success, + project: project, pipeline: pipeline, artifacts_expire_at: Time.now + 7.days) + end + + it 'keeps artifacts' do + expect(response).to have_gitlab_http_status(:ok) + expect(job.reload.artifacts_expire_at).to be_nil + end + end + + context 'no artifacts' do + let(:job) { create(:ci_build, project: project, pipeline: pipeline) } + + it 'responds with not found' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 410020b68cd..4978630b5ef 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -445,569 +445,6 @@ RSpec.describe API::Ci::Jobs do end end - 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 user is anonymous' do - let(:api_user) { nil } - - 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) - end - end - - 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 - - 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 - - describe 'GET /projects/:id/jobs/:job_id/artifacts/:artifact_path' do - context 'when job has artifacts' do - let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) } - - let(:artifact) do - 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' - end - - context 'when user is anonymous' do - let(:api_user) { nil } - - context 'when project is public' do - it 'allows to access artifacts' do - project.update_column(:visibility_level, - Gitlab::VisibilityLevel::PUBLIC) - project.update_column(:public_builds, true) - - get_artifact_file(artifact) - - expect(response).to have_gitlab_http_status(:ok) - end - end - - context 'when project is public with artifacts that are non public' do - let(:job) { create(:ci_build, :artifacts, :non_public_artifacts, pipeline: pipeline) } - - it 'rejects access to artifacts' do - project.update_column(:visibility_level, - Gitlab::VisibilityLevel::PUBLIC) - project.update_column(:public_builds, true) - - get_artifact_file(artifact) - - expect(response).to have_gitlab_http_status(:forbidden) - end - - context 'with the non_public_artifacts feature flag disabled' do - before do - stub_feature_flags(non_public_artifacts: false) - end - - it 'allows access to artifacts' do - project.update_column(:visibility_level, - Gitlab::VisibilityLevel::PUBLIC) - project.update_column(:public_builds, true) - - get_artifact_file(artifact) - - expect(response).to have_gitlab_http_status(:ok) - end - end - end - - context 'when project is public with builds access disabled' do - it 'rejects access to artifacts' do - project.update_column(:visibility_level, - Gitlab::VisibilityLevel::PUBLIC) - project.update_column(:public_builds, false) - - get_artifact_file(artifact) - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'when project is private' do - it 'rejects access and hides existence of artifacts' do - project.update_column(:visibility_level, - Gitlab::VisibilityLevel::PRIVATE) - project.update_column(:public_builds, true) - - get_artifact_file(artifact) - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - context 'when user is authorized' do - it 'returns a specific artifact file for a valid path' do - expect(Gitlab::Workhorse) - .to receive(:send_artifacts_entry) - .and_call_original - - get_artifact_file(artifact) - - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers.to_h) - .to include('Content-Type' => 'application/json', - 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) - expect(response.parsed_body).to be_empty - end - - context 'when artifacts are locked' do - it 'allows access to expired artifact' do - pipeline.artifacts_locked! - job.update!(artifacts_expire_at: Time.now - 7.days) - - get_artifact_file(artifact) - - expect(response).to have_gitlab_http_status(:ok) - end - end - end - end - - context 'when job does not have artifacts' do - it 'does not return job artifact file' do - get_artifact_file('some/artifact') - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - def get_artifact_file(artifact_path) - get api("/projects/#{project.id}/jobs/#{job.id}/" \ - "artifacts/#{artifact_path}", api_user) - end - end - - describe 'GET /projects/:id/jobs/:job_id/artifacts' do - shared_examples 'downloads artifact' do - let(:download_headers) do - { 'Content-Transfer-Encoding' => 'binary', - 'Content-Disposition' => %q(attachment; filename="ci_build_artifacts.zip"; filename*=UTF-8''ci_build_artifacts.zip) } - end - - it 'returns specific job artifacts' do - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers.to_h).to include(download_headers) - expect(response.body).to match_file(job.artifacts_file.file.file) - end - end - - context 'normal authentication' do - context 'job with artifacts' do - context 'when artifacts are stored locally' do - let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) } - - before do - get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) - end - - context 'authorized user' do - it_behaves_like 'downloads artifact' - end - - context 'unauthorized user' do - let(:api_user) { nil } - - it 'does not return specific job artifacts' do - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - context 'when artifacts are stored remotely' do - let(:proxy_download) { false } - let(:job) { create(:ci_build, pipeline: pipeline) } - let(:artifact) { create(:ci_job_artifact, :archive, :remote_store, job: job) } - - before do - stub_artifacts_object_storage(proxy_download: proxy_download) - - artifact - job.reload - - get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) - end - - context 'when proxy download is enabled' do - let(:proxy_download) { true } - - it 'responds with the workhorse send-url' do - expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("send-url:") - end - end - - context 'when proxy download is disabled' do - it 'returns location redirect' do - expect(response).to have_gitlab_http_status(:found) - end - end - - context 'authorized user' do - it 'returns the file remote URL' do - expect(response).to redirect_to(artifact.file.url) - end - end - - context 'unauthorized user' do - let(:api_user) { nil } - - it 'does not return specific job artifacts' do - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - - context 'when public project guest and artifacts are non public' do - let(:api_user) { guest } - let(:job) { create(:ci_build, :artifacts, :non_public_artifacts, pipeline: pipeline) } - - before do - project.update_column(:visibility_level, - Gitlab::VisibilityLevel::PUBLIC) - project.update_column(:public_builds, true) - get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) - end - - it 'rejects access and hides existence of artifacts' do - expect(response).to have_gitlab_http_status(:forbidden) - end - - context 'with the non_public_artifacts feature flag disabled' do - before do - stub_feature_flags(non_public_artifacts: false) - get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) - end - - it 'allows access to artifacts' do - expect(response).to have_gitlab_http_status(:ok) - end - end - end - - it 'does not return job artifacts if not uploaded' do - get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) - - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - end - - describe 'GET /projects/:id/artifacts/:ref_name/download?job=name' do - let(:api_user) { reporter } - let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } - - before do - stub_artifacts_object_storage - job.success - end - - def get_for_ref(ref = pipeline.ref, job_name = job.name) - get api("/projects/#{project.id}/jobs/artifacts/#{ref}/download", api_user), params: { job: job_name } - end - - context 'when not logged in' do - let(:api_user) { nil } - - before do - get_for_ref - end - - it 'does not find a resource in a private project' do - expect(project).to be_private - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when logging as guest' do - let(:api_user) { guest } - - before do - get_for_ref - end - - it 'gives 403' do - expect(response).to have_gitlab_http_status(:forbidden) - end - end - - context 'non-existing job' do - shared_examples 'not found' do - it { expect(response).to have_gitlab_http_status(:not_found) } - end - - context 'has no such ref' do - before do - get_for_ref('TAIL') - end - - it_behaves_like 'not found' - end - - context 'has no such job' do - before do - get_for_ref(pipeline.ref, 'NOBUILD') - end - - it_behaves_like 'not found' - end - end - - context 'find proper job' do - let(:job_with_artifacts) { job } - - shared_examples 'a valid file' do - context 'when artifacts are stored locally', :sidekiq_might_not_need_inline do - let(:download_headers) do - { 'Content-Transfer-Encoding' => 'binary', - 'Content-Disposition' => - %Q(attachment; filename="#{job_with_artifacts.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename}) } - end - - it { expect(response).to have_gitlab_http_status(:ok) } - it { expect(response.headers.to_h).to include(download_headers) } - end - - context 'when artifacts are stored remotely' do - let(:job) { create(:ci_build, pipeline: pipeline, user: api_user) } - let!(:artifact) { create(:ci_job_artifact, :archive, :remote_store, job: job) } - - before do - job.reload - - get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) - end - - it 'returns location redirect' do - expect(response).to have_gitlab_http_status(:found) - end - end - end - - context 'with regular branch' do - before do - pipeline.reload - pipeline.update!(ref: 'master', - sha: project.commit('master').sha) - - get_for_ref('master') - end - - it_behaves_like 'a valid file' - end - - context 'with branch name containing slash' do - before do - pipeline.reload - pipeline.update!(ref: 'improve/awesome', sha: project.commit('improve/awesome').sha) - get_for_ref('improve/awesome') - end - - it_behaves_like 'a valid file' - end - - context 'with job name in a child pipeline' do - let(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) } - let!(:child_job) { create(:ci_build, :artifacts, :success, name: 'rspec', pipeline: child_pipeline) } - let(:job_with_artifacts) { child_job } - - before do - get_for_ref('master', child_job.name) - end - - it_behaves_like 'a valid file' - end - end - end - - describe 'GET id/jobs/artifacts/:ref_name/raw/*artifact_path?job=name' do - context 'when job has artifacts' do - let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } - let(:artifact) { 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' } - let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } - let(:public_builds) { true } - - before do - stub_artifacts_object_storage - job.success - - project.update!(visibility_level: visibility_level, - public_builds: public_builds) - - get_artifact_file(artifact) - end - - context 'when user is anonymous' do - let(:api_user) { nil } - - context 'when project is public' do - let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } - let(:public_builds) { true } - - it 'allows to access artifacts', :sidekiq_might_not_need_inline do - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers.to_h) - .to include('Content-Type' => 'application/json', - 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) - end - end - - context 'when project is public with builds access disabled' do - let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } - let(:public_builds) { false } - - it 'rejects access to artifacts' do - expect(response).to have_gitlab_http_status(:forbidden) - expect(json_response).to have_key('message') - expect(response.headers.to_h) - .not_to include('Gitlab-Workhorse-Send-Data' => /artifacts-entry/) - end - end - - context 'when project is public with non public artifacts' do - let(:job) { create(:ci_build, :artifacts, :non_public_artifacts, pipeline: pipeline, user: api_user) } - let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } - let(:public_builds) { true } - - it 'rejects access and hides existence of artifacts', :sidekiq_might_not_need_inline do - get_artifact_file(artifact) - - expect(response).to have_gitlab_http_status(:forbidden) - expect(json_response).to have_key('message') - expect(response.headers.to_h) - .not_to include('Gitlab-Workhorse-Send-Data' => /artifacts-entry/) - end - - context 'with the non_public_artifacts feature flag disabled' do - before do - stub_feature_flags(non_public_artifacts: false) - end - - it 'allows access to artifacts', :sidekiq_might_not_need_inline do - get_artifact_file(artifact) - - expect(response).to have_gitlab_http_status(:ok) - end - end - end - - context 'when project is private' do - let(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE } - let(:public_builds) { true } - - it 'rejects access and hides existence of artifacts' do - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response).to have_key('message') - expect(response.headers.to_h) - .not_to include('Gitlab-Workhorse-Send-Data' => /artifacts-entry/) - end - end - end - - context 'when user is authorized' do - let(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE } - let(:public_builds) { true } - - it 'returns a specific artifact file for a valid path', :sidekiq_might_not_need_inline do - expect(Gitlab::Workhorse) - .to receive(:send_artifacts_entry) - .and_call_original - - get_artifact_file(artifact) - - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers.to_h) - .to include('Content-Type' => 'application/json', - 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) - expect(response.parsed_body).to be_empty - end - end - - context 'with branch name containing slash' do - before do - pipeline.reload - pipeline.update!(ref: 'improve/awesome', - sha: project.commit('improve/awesome').sha) - end - - it 'returns a specific artifact file for a valid path', :sidekiq_might_not_need_inline do - get_artifact_file(artifact, 'improve/awesome') - - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers.to_h) - .to include('Content-Type' => 'application/json', - 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) - end - end - - context 'non-existing job' do - shared_examples 'not found' do - it { expect(response).to have_gitlab_http_status(:not_found) } - end - - context 'has no such ref' do - before do - get_artifact_file('some/artifact', 'wrong-ref') - end - - it_behaves_like 'not found' - end - - context 'has no such job' do - before do - get_artifact_file('some/artifact', pipeline.ref, 'wrong-job-name') - end - - it_behaves_like 'not found' - end - end - end - - context 'when job does not have artifacts' do - let(:job) { create(:ci_build, pipeline: pipeline, user: api_user) } - - it 'does not return job artifact file' do - get_artifact_file('some/artifact') - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - def get_artifact_file(artifact_path, ref = pipeline.ref, job_name = job.name) - get api("/projects/#{project.id}/jobs/artifacts/#{ref}/raw/#{artifact_path}", api_user), params: { job: job_name } - end - end - describe 'GET /projects/:id/jobs/:job_id/trace' do before do get api("/projects/#{project.id}/jobs/#{job.id}/trace", api_user) @@ -1249,32 +686,6 @@ RSpec.describe API::Ci::Jobs do end end - describe 'POST /projects/:id/jobs/:job_id/artifacts/keep' do - before do - post api("/projects/#{project.id}/jobs/#{job.id}/artifacts/keep", user) - end - - context 'artifacts did not expire' do - let(:job) do - create(:ci_build, :trace_artifact, :artifacts, :success, - project: project, pipeline: pipeline, artifacts_expire_at: Time.now + 7.days) - end - - it 'keeps artifacts' do - expect(response).to have_gitlab_http_status(:ok) - expect(job.reload.artifacts_expire_at).to be_nil - end - end - - context 'no artifacts' do - let(:job) { create(:ci_build, project: project, pipeline: pipeline) } - - it 'responds with not found' do - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - describe 'POST /projects/:id/jobs/:job_id/play' do before do post api("/projects/#{project.id}/jobs/#{job.id}/play", api_user) diff --git a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb index 1cc856734fc..0e7230c042e 100644 --- a/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb +++ b/spec/services/ci/job_artifacts/destroy_batch_service_spec.rb @@ -3,59 +3,74 @@ require 'spec_helper' RSpec.describe Ci::JobArtifacts::DestroyBatchService do - let(:artifacts) { Ci::JobArtifact.all } + let(:artifacts) { Ci::JobArtifact.where(id: [artifact_with_file.id, artifact_without_file.id]) } let(:service) { described_class.new(artifacts, pick_up_at: Time.current) } + let_it_be(:artifact_with_file, refind: true) do + create(:ci_job_artifact, :zip) + end + + let_it_be(:artifact_without_file, refind: true) do + create(:ci_job_artifact) + end + + let_it_be(:undeleted_artifact, refind: true) do + create(:ci_job_artifact) + end + describe '.execute' do subject(:execute) { service.execute } - let_it_be(:artifact, refind: true) do - create(:ci_job_artifact) + it 'creates a deleted object for artifact with attached file' do + expect { subject }.to change { Ci::DeletedObject.count }.by(1) end - context 'when the artifact has a file attached to it' do - before do - artifact.file = fixture_file_upload(Rails.root.join('spec/fixtures/ci_build_artifacts.zip'), 'application/zip') - artifact.save! - end + it 'does not remove the attached file' do + expect { execute }.not_to change { artifact_with_file.file.exists? } + end - it 'creates a deleted object' do - expect { subject }.to change { Ci::DeletedObject.count }.by(1) - end + it 'deletes the artifact records' do + expect { subject }.to change { Ci::JobArtifact.count }.by(-2) + end - it 'does not remove the files' do - expect { execute }.not_to change { artifact.file.exists? } + it 'reports metrics for destroyed artifacts' do + expect_next_instance_of(Gitlab::Ci::Artifacts::Metrics) do |metrics| + expect(metrics).to receive(:increment_destroyed_artifacts_count).with(2).and_call_original + expect(metrics).to receive(:increment_destroyed_artifacts_bytes).with(107464).and_call_original end - it 'reports metrics for destroyed artifacts' do - expect_next_instance_of(Gitlab::Ci::Artifacts::Metrics) do |metrics| - expect(metrics).to receive(:increment_destroyed_artifacts_count).with(1).and_call_original - expect(metrics).to receive(:increment_destroyed_artifacts_bytes).with(107464).and_call_original - end + execute + end + + context 'ProjectStatistics' do + it 'resets project statistics' do + expect(ProjectStatistics).to receive(:increment_statistic).once + .with(artifact_with_file.project, :build_artifacts_size, -artifact_with_file.file.size) + .and_call_original + expect(ProjectStatistics).to receive(:increment_statistic).once + .with(artifact_without_file.project, :build_artifacts_size, 0) + .and_call_original execute end - context 'ProjectStatistics' do - it 'resets project statistics' do - expect(ProjectStatistics).to receive(:increment_statistic).once - .with(artifact.project, :build_artifacts_size, -artifact.file.size) - .and_call_original + context 'with update_stats: false' do + it 'does not update project statistics' do + expect(ProjectStatistics).not_to receive(:increment_statistic) - execute + service.execute(update_stats: false) end - context 'with update_stats: false' do - it 'does not update project statistics' do - expect(ProjectStatistics).not_to receive(:increment_statistic) - - service.execute(update_stats: false) - end + it 'returns size statistics' do + expected_updates = { + statistics_updates: { + artifact_with_file.project => -artifact_with_file.file.size, + artifact_without_file.project => 0 + } + } - it 'returns size statistics' do - expect(service.execute(update_stats: false)).to match( - a_hash_including(statistics_updates: { artifact.project => -artifact.file.size })) - end + expect(service.execute(update_stats: false)).to match( + a_hash_including(expected_updates)) end end end @@ -71,7 +86,7 @@ RSpec.describe Ci::JobArtifacts::DestroyBatchService do it 'raises an exception and stop destroying' do expect { execute }.to raise_error(ActiveRecord::RecordNotDestroyed) - .and not_change { Ci::JobArtifact.count }.from(1) + .and not_change { Ci::JobArtifact.count } end end end diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb index 35d46884f4d..59ec78bf9ec 100644 --- a/spec/services/groups/transfer_service_spec.rb +++ b/spec/services/groups/transfer_service_spec.rb @@ -792,7 +792,7 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do end end - context 'when group has pending builds' do + context 'when group has pending builds', :sidekiq_inline do let_it_be(:project) { create(:project, :public, namespace: group.reload) } let_it_be(:other_project) { create(:project) } let_it_be(:pending_build) { create(:ci_pending_build, project: project) } @@ -814,6 +814,20 @@ RSpec.describe Groups::TransferService, :sidekiq_inline do expect(unrelated_pending_build.namespace_id).to eq(other_project.namespace_id) expect(unrelated_pending_build.namespace_traversal_ids).to eq(other_project.namespace.traversal_ids) end + + context 'when ci_pending_builds_async_update is disabled' do + let(:update_pending_build_service) { instance_double(::Ci::PendingBuilds::UpdateGroupWorker) } + + before do + stub_feature_flags(ci_pending_builds_async_update: false) + end + + it 'does not call the new worker' do + expect(::Ci::PendingBuilds::UpdateGroupWorker).not_to receive(:perform_async) + + transfer_service.execute(new_parent_group) + end + end end end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index c47d44002cc..93b7ce56247 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -169,7 +169,7 @@ RSpec.describe Projects::TransferService do end end - context 'when project has pending builds' do + context 'when project has pending builds', :sidekiq_inline do let!(:other_project) { create(:project) } let!(:pending_build) { create(:ci_pending_build, project: project.reload) } let!(:unrelated_pending_build) { create(:ci_pending_build, project: other_project) } @@ -189,6 +189,20 @@ RSpec.describe Projects::TransferService do expect(unrelated_pending_build.namespace_id).to eq(other_project.namespace_id) expect(unrelated_pending_build.namespace_traversal_ids).to eq(other_project.namespace.traversal_ids) end + + context 'when ci_pending_builds_async_update is disabled' do + let(:update_pending_build_service) { instance_double(::Ci::PendingBuilds::UpdateProjectWorker) } + + before do + stub_feature_flags(ci_pending_builds_async_update: false) + end + + it 'does not call the new worker' do + expect(::Ci::PendingBuilds::UpdateProjectWorker).not_to receive(:perform_async) + + execute_transfer + end + end end end @@ -251,7 +265,7 @@ RSpec.describe Projects::TransferService do ) end - context 'when project has pending builds' do + context 'when project has pending builds', :sidekiq_inline do let!(:other_project) { create(:project) } let!(:pending_build) { create(:ci_pending_build, project: project.reload) } let!(:unrelated_pending_build) { create(:ci_pending_build, project: other_project) } diff --git a/spec/support/database/cross-database-modification-allowlist.yml b/spec/support/database/cross-database-modification-allowlist.yml index 1a81ff79973..a28832a615e 100644 --- a/spec/support/database/cross-database-modification-allowlist.yml +++ b/spec/support/database/cross-database-modification-allowlist.yml @@ -78,10 +78,8 @@ - "./spec/services/ci/pipeline_bridge_status_service_spec.rb" - "./spec/services/ci/pipelines/add_job_service_spec.rb" - "./spec/services/ci/retry_build_service_spec.rb" -- "./spec/services/groups/transfer_service_spec.rb" - "./spec/services/projects/destroy_service_spec.rb" - "./spec/services/projects/overwrite_project_service_spec.rb" -- "./spec/services/projects/transfer_service_spec.rb" - "./spec/services/resource_access_tokens/revoke_service_spec.rb" - "./spec/services/users/destroy_service_spec.rb" - "./spec/services/users/reject_service_spec.rb" diff --git a/spec/workers/ci/pending_builds/update_group_worker_spec.rb b/spec/workers/ci/pending_builds/update_group_worker_spec.rb new file mode 100644 index 00000000000..8c6bf018158 --- /dev/null +++ b/spec/workers/ci/pending_builds/update_group_worker_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::PendingBuilds::UpdateGroupWorker do + describe '#perform' do + let(:worker) { described_class.new } + + context 'when a group is not provided' do + it 'does not call the service' do + expect(::Ci::UpdatePendingBuildService).not_to receive(:new) + end + end + + context 'when everything is ok' do + let(:group) { create(:group) } + let(:update_pending_build_service) { instance_double(::Ci::UpdatePendingBuildService) } + let(:update_params) { { "namespace_id" => group.id } } + + it 'calls the service' do + expect(::Ci::UpdatePendingBuildService).to receive(:new).with(group, update_params).and_return(update_pending_build_service) + expect(update_pending_build_service).to receive(:execute) + + worker.perform(group.id, update_params) + end + + include_examples 'an idempotent worker' do + let(:pending_build) { create(:ci_pending_build) } + let(:update_params) { { "namespace_id" => pending_build.namespace_id } } + let(:job_args) { [pending_build.namespace_id, update_params] } + + it 'updates the pending builds' do + subject + + expect(pending_build.reload.namespace_id).to eq(update_params["namespace_id"]) + end + end + end + end +end diff --git a/spec/workers/ci/pending_builds/update_project_worker_spec.rb b/spec/workers/ci/pending_builds/update_project_worker_spec.rb new file mode 100644 index 00000000000..4a67127564e --- /dev/null +++ b/spec/workers/ci/pending_builds/update_project_worker_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::PendingBuilds::UpdateProjectWorker do + describe '#perform' do + let(:worker) { described_class.new } + + context 'when a project is not provided' do + it 'does not call the service' do + expect(::Ci::UpdatePendingBuildService).not_to receive(:new) + end + end + + context 'when everything is ok' do + let(:project) { create(:project) } + let(:group) { create(:group) } + let(:update_pending_build_service) { instance_double(::Ci::UpdatePendingBuildService) } + let(:update_params) { { "namespace_id" => group.id } } + + it 'calls the service' do + expect(::Ci::UpdatePendingBuildService).to receive(:new).with(project, update_params).and_return(update_pending_build_service) + expect(update_pending_build_service).to receive(:execute) + + worker.perform(project.id, update_params) + end + + include_examples 'an idempotent worker' do + let(:pending_build) { create(:ci_pending_build) } + let(:job_args) { [pending_build.project_id, update_params] } + + it 'updates the pending builds' do + subject + + expect(pending_build.reload.namespace_id).to eq(update_params["namespace_id"]) + end + end + end + end +end |