Commit a15dc480 authored by Shinya Maeda's avatar Shinya Maeda

Docs for create pipelines in the parent project for fork mr

This commit documents for the feature

Apply 3 suggestion(s) to 1 file(s)
parent 8bd32ec0
<script> <script>
import { GlDeprecatedButton, GlLoadingIcon, GlModal, GlModalDirective } from '@gitlab/ui'; import { GlButton, GlLoadingIcon, GlModal, GlLink } from '@gitlab/ui';
import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
import PipelinesService from '~/pipelines/services/pipelines_service'; import PipelinesService from '~/pipelines/services/pipelines_service';
import PipelineStore from '~/pipelines/stores/pipelines_store'; import PipelineStore from '~/pipelines/stores/pipelines_store';
...@@ -12,12 +12,10 @@ import CIPaginationMixin from '~/vue_shared/mixins/ci_pagination_api_mixin'; ...@@ -12,12 +12,10 @@ import CIPaginationMixin from '~/vue_shared/mixins/ci_pagination_api_mixin';
export default { export default {
components: { components: {
TablePagination, TablePagination,
GlDeprecatedButton, GlButton,
GlLoadingIcon, GlLoadingIcon,
GlModal, GlModal,
}, GlLink,
directives: {
GlModalDirective,
}, },
mixins: [pipelinesMixin, CIPaginationMixin], mixins: [pipelinesMixin, CIPaginationMixin],
props: { props: {
...@@ -104,7 +102,7 @@ export default { ...@@ -104,7 +102,7 @@ export default {
isLatestPipelineCreatedInTargetProject() { isLatestPipelineCreatedInTargetProject() {
const latest = this.state.pipelines[0]; const latest = this.state.pipelines[0];
return latest && latest.project.full_path === `/${this.targetProjectFullPath}`; return latest?.project?.full_path === `/${this.targetProjectFullPath}`;
}, },
shouldShowSecurityWarning() { shouldShowSecurityWarning() {
return ( return (
...@@ -178,6 +176,13 @@ export default { ...@@ -178,6 +176,13 @@ export default {
mergeRequestId: this.mergeRequestId, mergeRequestId: this.mergeRequestId,
}); });
}, },
tryRunPipeline() {
if (!this.shouldShowSecurityWarning) {
this.onClickRunPipeline();
} else {
this.$refs.modal.show();
}
},
}, },
}; };
</script> </script>
...@@ -201,30 +206,19 @@ export default { ...@@ -201,30 +206,19 @@ export default {
<div v-else-if="shouldRenderTable" class="table-holder"> <div v-else-if="shouldRenderTable" class="table-holder">
<div v-if="canRenderPipelineButton" class="nav justify-content-end"> <div v-if="canRenderPipelineButton" class="nav justify-content-end">
<gl-deprecated-button <gl-button
v-if="!shouldShowSecurityWarning"
variant="success" variant="success"
class="js-run-mr-pipeline prepend-top-10 btn-wide-on-xs" class="js-run-mr-pipeline prepend-top-10 btn-wide-on-xs"
:disabled="state.isRunningMergeRequestPipeline" :disabled="state.isRunningMergeRequestPipeline"
@click="onClickRunPipeline" @click="tryRunPipeline"
> >
<gl-loading-icon v-if="state.isRunningMergeRequestPipeline" inline /> <gl-loading-icon v-if="state.isRunningMergeRequestPipeline" inline />
{{ s__('Pipelines|Run Pipeline') }} {{ s__('Pipelines|Run Pipeline') }}
</gl-deprecated-button> </gl-button>
<gl-deprecated-button
v-if="shouldShowSecurityWarning"
v-gl-modal-directive="modalId"
variant="success"
class="js-run-mr-pipeline prepend-top-10 btn-wide-on-xs"
:disabled="state.isRunningMergeRequestPipeline"
>
<gl-loading-icon v-if="state.isRunningMergeRequestPipeline" inline />
{{ s__('Pipelines|Run Pipeline') }}
</gl-deprecated-button>
<gl-modal <gl-modal
:id="$options.id" :id="modalId"
ref="modal"
:modal-id="modalId" :modal-id="modalId"
:title="s__('Pipelines|Are you sure you want to run this pipeline?')" :title="s__('Pipelines|Are you sure you want to run this pipeline?')"
:ok-title="s__('Pipelines|Run Pipeline')" :ok-title="s__('Pipelines|Run Pipeline')"
...@@ -252,6 +246,12 @@ export default { ...@@ -252,6 +246,12 @@ export default {
) )
}} }}
</p> </p>
<gl-link
href="/help/ci/merge_request_pipelines/index.html#create-pipelines-in-the-parent-project-for-merge-requests-from-a-forked-project"
target="_blank"
>
{{ s__('Pipelines|More Information') }}
</gl-link>
</gl-modal> </gl-modal>
</div> </div>
......
...@@ -358,11 +358,11 @@ export default class MergeRequestTabs { ...@@ -358,11 +358,11 @@ export default class MergeRequestTabs {
emptyStateSvgPath: pipelineTableViewEl.dataset.emptyStateSvgPath, emptyStateSvgPath: pipelineTableViewEl.dataset.emptyStateSvgPath,
errorStateSvgPath: pipelineTableViewEl.dataset.errorStateSvgPath, errorStateSvgPath: pipelineTableViewEl.dataset.errorStateSvgPath,
autoDevopsHelpPath: pipelineTableViewEl.dataset.helpAutoDevopsPath, autoDevopsHelpPath: pipelineTableViewEl.dataset.helpAutoDevopsPath,
canCreatePipelineInTargetProject: mrWidgetData canCreatePipelineInTargetProject: Boolean(
? mrWidgetData.can_create_pipeline_in_target_project mrWidgetData?.can_create_pipeline_in_target_project,
: null, ),
sourceProjectFullPath: mrWidgetData ? mrWidgetData.source_project_full_path : null, sourceProjectFullPath: mrWidgetData?.source_project_full_path || '',
targetProjectFullPath: mrWidgetData ? mrWidgetData.target_project_full_path : null, targetProjectFullPath: mrWidgetData?.target_project_full_path || '',
projectId: pipelineTableViewEl.dataset.projectId, projectId: pipelineTableViewEl.dataset.projectId,
mergeRequestId: mrWidgetData ? mrWidgetData.iid : null, mergeRequestId: mrWidgetData ? mrWidgetData.iid : null,
}, },
......
---
title: Show Security Warning Modal for fork pipelines
merge_request: 36951
author:
type: added
...@@ -166,31 +166,33 @@ Read the [documentation on Pipelines for Merged Results](pipelines_for_merged_re ...@@ -166,31 +166,33 @@ Read the [documentation on Pipelines for Merged Results](pipelines_for_merged_re
Read the [documentation on Merge Trains](pipelines_for_merged_results/merge_trains/index.md). Read the [documentation on Merge Trains](pipelines_for_merged_results/merge_trains/index.md).
## Important notes about merge requests from forked projects ## Create pipelines in the parent project for merge requests from a forked project
Note that the current behavior is subject to change. In the usual contribution > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/217451) in GitLab 13.3.
flow, external contributors follow the following steps:
By default, external contributors working from forks can't create pipelines in the
1. Fork a parent project. parent project. When a pipeline for merge requests is triggered by a merge request
1. Create a merge request from the forked project that targets the `master` branch coming from a fork:
in the parent project.
1. A pipeline runs on the merge request. - It's created and runs in the fork (source) project, not the parent (target) project.
1. A maintainer from the parent project checks the pipeline result, and merge - It uses the fork project's CI/CD configuration and resources.
into a target branch if the latest pipeline has passed.
Sometimes parent project members want the pipeline to run in the parent
Currently, those pipelines are created in a **forked** project, not in the project. This could be to ensure that the post-merge pipeline passes in the parent project.
parent project. This means you cannot completely trust the pipeline result, For example, a fork project could try to use a corrupted Runner that doesn't execute
because, technically, external contributors can disguise their pipeline results test scripts properly, but reports a passed pipeline. Reviewers in the parent project
by tweaking their GitLab Runner in the forked project. could mistakenly trust the merge request because it passed a faked pipeline.
There are multiple reasons why GitLab doesn't allow those pipelines to be Parent project members with at least [Developer permissions](../../user/permissions.md)
created in the parent project, but one of the biggest reasons is security concern. can create pipelines in the parent project for merge requests
External users could steal secret variables from the parent project by modifying from a forked project. In the merge request, go to the **Pipelines** and click
`.gitlab-ci.yml`, which could be some sort of credentials. This should not happen. **Run Pipeline** button.
We're discussing a secure solution of running pipelines for merge requests CAUTION: **Caution:**
that are submitted from forked projects, Fork merge requests could contain malicious code that tries to steal secrets in the
see [the issue about the permission extension](https://gitlab.com/gitlab-org/gitlab/-/issues/11934). parent project when the pipeline runs, even before merge. Reviewers must carefully
check the changes in the merge request before triggering the pipeline. GitLab shows
a warning that must be accepted before the pipeline can be triggered.
## Additional predefined variables ## Additional predefined variables
......
...@@ -45,8 +45,6 @@ To enable pipelines for merge results: ...@@ -45,8 +45,6 @@ To enable pipelines for merge results:
- You must have maintainer [permissions](../../../user/permissions.md). - You must have maintainer [permissions](../../../user/permissions.md).
- You must be using [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-runner) 11.9 or later. - You must be using [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-runner) 11.9 or later.
- You must not be forking or using cross-repo workflows. To follow progress,
see [#11934](https://gitlab.com/gitlab-org/gitlab/-/issues/11934).
- You must not be using - You must not be using
[fast forward merges](../../../user/project/merge_requests/fast_forward_merge.md) yet. [fast forward merges](../../../user/project/merge_requests/fast_forward_merge.md) yet.
To follow progress, see [#58226](https://gitlab.com/gitlab-org/gitlab/-/issues/26996). To follow progress, see [#58226](https://gitlab.com/gitlab-org/gitlab/-/issues/26996).
......
...@@ -17194,6 +17194,9 @@ msgstr "" ...@@ -17194,6 +17194,9 @@ msgstr ""
msgid "Pipelines|Loading Pipelines" msgid "Pipelines|Loading Pipelines"
msgstr "" msgstr ""
msgid "Pipelines|More Information"
msgstr ""
msgid "Pipelines|Project cache successfully reset." msgid "Pipelines|Project cache successfully reset."
msgstr "" msgstr ""
......
...@@ -153,23 +153,47 @@ describe('Pipelines table in Commits and Merge requests', () => { ...@@ -153,23 +153,47 @@ describe('Pipelines table in Commits and Merge requests', () => {
}); });
}); });
describe('when latest pipeline does not have detached flag and merge_request_pipeline is true', () => { describe('on click', () => {
it('does not render the run pipeline button', done => { const findModal = () =>
pipelineCopy.flags.detached_merge_request_pipeline = false; document.querySelector('#create-pipeline-for-fork-merge-request-modal');
pipelineCopy.flags.merge_request_pipeline = true;
beforeEach(() => {
pipelineCopy.flags.detached_merge_request_pipeline = true;
mock.onGet('endpoint.json').reply(200, [pipelineCopy]); mock.onGet('endpoint.json').reply(200, [pipelineCopy]);
vm = mountComponent(PipelinesTable, { ...props }); vm = mountComponent(PipelinesTable, {
...props,
canRunPipeline: true,
projectId: '5',
mergeRequestId: 3,
});
});
it('updates the loading state', done => {
jest.spyOn(Api, 'postMergeRequestPipeline').mockReturnValue(Promise.resolve());
setImmediate(() => { setImmediate(() => {
expect(vm.$el.querySelector('.js-run-mr-pipeline')).toBeNull(); vm.$el.querySelector('.js-run-mr-pipeline').click();
vm.$nextTick(() => {
expect(findModal()).toBeNull();
expect(vm.state.isRunningMergeRequestPipeline).toBe(true);
setImmediate(() => {
expect(vm.state.isRunningMergeRequestPipeline).toBe(false);
done(); done();
}); });
}); });
}); });
});
});
describe('on click for fork merge request', () => {
const findModal = () =>
document.querySelector('#create-pipeline-for-fork-merge-request-modal');
describe('on click', () => {
beforeEach(() => { beforeEach(() => {
pipelineCopy.flags.detached_merge_request_pipeline = true; pipelineCopy.flags.detached_merge_request_pipeline = true;
...@@ -177,31 +201,28 @@ describe('Pipelines table in Commits and Merge requests', () => { ...@@ -177,31 +201,28 @@ describe('Pipelines table in Commits and Merge requests', () => {
vm = mountComponent(PipelinesTable, { vm = mountComponent(PipelinesTable, {
...props, ...props,
canRunPipeline: true,
projectId: '5', projectId: '5',
mergeRequestId: 3, mergeRequestId: 3,
canCreatePipelineInTargetProject: true,
sourceProjectFullPath: 'test/parent-project',
targetProjectFullPath: 'test/fork-project',
}); });
}); });
it('updates the loading state', done => { it('shows a security warning modal', done => {
jest.spyOn(Api, 'postMergeRequestPipeline').mockReturnValue(Promise.resolve()); jest.spyOn(Api, 'postMergeRequestPipeline').mockReturnValue(Promise.resolve());
setImmediate(() => { setImmediate(() => {
vm.$el.querySelector('.js-run-mr-pipeline').click(); vm.$el.querySelector('.js-run-mr-pipeline').click();
vm.$nextTick(() => { vm.$nextTick(() => {
expect(vm.state.isRunningMergeRequestPipeline).toBe(true); expect(findModal()).not.toBeNull();
setImmediate(() => {
expect(vm.state.isRunningMergeRequestPipeline).toBe(false);
done(); done();
}); });
}); });
}); });
}); });
}); });
});
describe('unsuccessfull request', () => { describe('unsuccessfull request', () => {
beforeEach(() => { beforeEach(() => {
......
...@@ -31,6 +31,28 @@ RSpec.describe MergeRequestWidgetEntity do ...@@ -31,6 +31,28 @@ RSpec.describe MergeRequestWidgetEntity do
end end
end end
describe 'can_create_pipeline_in_target_project' do
context 'when user has permission' do
before do
project.add_developer(user)
end
it 'includes the correct permission info' do
expect(subject[:can_create_pipeline_in_target_project]).to eq(true)
end
end
context 'when user does not have permission' do
before do
project.add_guest(user)
end
it 'includes the correct permission info' do
expect(subject[:can_create_pipeline_in_target_project]).to eq(false)
end
end
end
describe 'issues links' do describe 'issues links' do
it 'includes issues links when requested' do it 'includes issues links when requested' do
data = described_class.new(resource, request: request, issues_links: true).as_json data = described_class.new(resource, request: request, issues_links: true).as_json
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment