Skip to content

Fix to prevent unlimited number of CI includes

Leaminn Ma requested to merge 367150-ci-fix-max-includes into master

What does this MR do and why?

Currently there is a bug that allows the user to have unlimited includes (#367150 (closed)). This is because the file validation method in Config::External::Mapper::Verifier expands all its nested includes before adding itself to the structure that we use for counting the includes (expandset).

This MR rearranges the operations in Verifier so that all relevant files are included in expandset to obtain the true total file count. It also includes a check context.is_internal_include? that ensures that an internally injected include is excluded from the count.

The changes have been made behind a new FF: ci_fix_max_includes

Also did a minor refactor for the specs in spec/lib/gitlab/ci/config/external/context_spec.rb.

How to set up and validate locally

  1. Update the max includes value to 2 in lib/gitlab/ci/config/external/context.rb:12.
MAX_INCLUDES = 2
  1. Set up a simple config of 3 nested includes by creating the following files in your project root.

File1: template1.yml

job1:
  script: exit 0

File2: template2.yml

include:
  - local: template1.yml
job2:
  script: exit 0

File3: template3.yml

include:
  - local: template2.yml
job3:
  script: exit 0
  1. Go to the CI/CD Editor and update the contents with:
include:
  - local: template3.yml

And observe that the validation passes even though there are a total of 3 include files.

  1. Now enable the ci_fix_max_includes feature flag.
Feature.enable('ci_fix_max_includes')
  1. Refresh the CI/CD Editor and now observe that the config provided in Step 2 now produces an error. Screenshot_2023-02-24_at_3.54.28_PM

  2. Update the CI/CD Editor with:

include:
  - local: template2.yml

And observe you no longer get the error.

  1. Commit the change and see that the pipeline runs successfully.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #367150 (closed)

Edited by Leaminn Ma

Merge request reports

Loading