Skip to content

Draft: Fix duplicate pipelines for invalid syntax

Furkan Ayhan requested to merge fix-duplicate-pipeline-problem into master

What does this MR do?

This is draft MR, no test, no feature flag...

This MR fixes the problem of duplicate pipelines for invalid syntax.

Copying from the comments (1, 2)

In a scenario where I don't use Merge Request Pipelines. When I push a second commit to an MR, Ci::CreatePipelineService is called twice;

  1. For push source.
  2. For merge_request_event source.

The first call successfully creates the pipeline, but the second one does not. Why? =>

  • In the Seed step, we try to build jobs/stages, etc.
  • When building a job, we check @only.all? { |spec| spec.satisfied_by?(@pipeline, evaluate_context) } before adding it to the pipeline.
  • Since the default only is [branches, tags], we do not return true for merge request pipelines.

This default condition helps us not to have duplicate pipelines. However, if the pipeline is invalid before the Seed step, the duplicate one for merge_request_event is created. For example; when the syntax is invalid, it returns from the Config::Process step and creates an invalid pipeline.

Also, it does not matter if I use Merge Request Pipelines.

After adding

include:
  - template: 'Workflows/MergeRequest-Pipelines.gitlab-ci.yml'

duplicate pipelines are still created because we drop the pipeline in the Config::Process step when the syntax is invalid. So the EvaluateWorkflowRules step is not run at all.

Screenshots (strongly suggested)

Before this MR

We have our initial commit in the MR Screen_Shot_2021-05-06_at_14.41.06

Here are the pipelines Screen_Shot_2021-05-06_at_14.40.55

We have the second commit in the MR, which makes the syntax invalid Screen_Shot_2021-05-06_at_14.41.39

And we have duplicate pipelines Screen_Shot_2021-05-06_at_14.41.52

After this MR

We have our initial commit in the MR Screen_Shot_2021-05-06_at_14.45.40

Here are the pipelines Screen_Shot_2021-05-06_at_14.45.25

We have the second commit in the MR, which makes the syntax invalid Screen_Shot_2021-05-06_at_14.46.20

And we don't have duplicate pipelines Screen_Shot_2021-05-06_at_14.46.12

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Furkan Ayhan

Merge request reports

Loading