Merge branch 'lm-remove-duplicate-includes' into 'master'

Remove warning/error for duplicate includes

See merge request gitlab-org/gitlab!78297

doc/ci/yaml/includes.md

@@ -217,6 +217,69 @@ default:
     - echo "Job complete."
 ```
 
+### Use nested includes with duplicate `includes` entries
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/28987) in GitLab 14.8
+
+Nested includes can include the same configuration file. The duplicate configuration
+file is included multiple times, but the effect is the same as if it was only
+included once.
+
+For example, with the following nested includes, where `defaults.gitlab-ci.yml`
+is included multiple times:
+
+- Contents of the `.gitlab-ci.yml` file:
+
+  ```yaml
+  include:
+    - template: defaults.gitlab-ci.yml
+    - local: unit-tests.gitlab-ci.yml
+    - local: smoke-tests.gitlab-ci.yml
+  ```
+
+- Contents of the `defaults.gitlab-ci.yml` file:
+
+  ```yaml
+  default:
+    before_script: default-before-script.sh
+    retry: 2
+  ```
+
+- Contents of the `unit-tests.gitlab-ci.yml` file:
+
+  ```yaml
+  include:
+    - template: defaults.gitlab-ci.yml
+
+  unit-test-job:
+    script: unit-test.sh
+    retry: 0
+  ```
+
+- Contents of the `smoke-tests.gitlab-ci.yml` file:
+
+  ```yaml
+  include:
+    - template: defaults.gitlab-ci.yml
+
+  smoke-test-job:
+    script: smoke-test.sh
+  ```
+
+The final configuration would be:
+
+```yaml
+unit-test-job:
+  before_script: default-before-script.sh
+  script: unit-test.sh
+  retry: 0
+
+smoke-test-job:
+  before_script: default-before-script.sh
+  script: smoke-test.sh
+  retry: 2
+```
+
 ## Use variables with `include`
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/284883) in GitLab 13.8.

lib/gitlab/ci/config/external/mapper.rb

@@ -19,7 +19,6 @@ module Gitlab
 
           Error = Class.new(StandardError)
           AmbigiousSpecificationError = Class.new(Error)
-          DuplicateIncludesError = Class.new(Error)
           TooManyIncludesError = Class.new(Error)
 
           def initialize(values, context)
@@ -114,25 +113,22 @@ module Gitlab
 
           def verify_duplicates!(location)
             logger.instrument(:config_mapper_verify) do
-              verify_duplicates_without_instrumentation!(location)
+              verify_max_includes_and_add_location!(location)
             end
           end
 
-          def verify_duplicates_without_instrumentation!(location)
+          def verify_max_includes_and_add_location!(location)
             if expandset.count >= MAX_INCLUDES
               raise TooManyIncludesError, "Maximum of #{MAX_INCLUDES} nested includes are allowed!"
             end
 
-            # We scope location to context, as this allows us to properly support
-            # relative includes, and similarly looking relative in another project
-            # does not trigger duplicate error
+            # Scope location to context to allow support of
+            # relative includes
             scoped_location = location.merge(
               context_project: context.project,
               context_sha: context.sha)
 
-            unless expandset.add?(scoped_location)
-              raise DuplicateIncludesError, "Include `#{location.to_json}` was already included!"
-            end
+            expandset.add(scoped_location)
           end
 
           def select_first_matching(location)

spec/lib/gitlab/ci/config/external/mapper_spec.rb

@@ -175,7 +175,7 @@ RSpec.describe Gitlab::Ci::Config::External::Mapper do
       end
     end
 
-    context "when duplicate 'include' is defined" do
+    context "when duplicate 'include's are defined" do
       let(:values) do
         { include: [
           { 'local' => local_file },
@@ -184,18 +184,26 @@ RSpec.describe Gitlab::Ci::Config::External::Mapper do
         image: 'ruby:2.7' }
       end
 
-      it 'raises an exception' do
-        expect { subject }.to raise_error(described_class::DuplicateIncludesError)
+      it 'does not raise an exception' do
+        expect { subject }.not_to raise_error
+      end
     end
 
-      context 'when including multiple files from a project' do
+    context 'when passing max number of files' do
       let(:values) do
-          { include: { project: project.full_path, file: [local_file, local_file] } }
+        { include: [
+            { 'local' => local_file },
+            { 'remote' => remote_url }
+          ],
+          image: 'ruby:2.7' }
       end
 
-        it 'raises an exception' do
-          expect { subject }.to raise_error(described_class::DuplicateIncludesError)
+      before do
+        stub_const("#{described_class}::MAX_INCLUDES", 2)
       end
+
+      it 'does not raise an exception' do
+        expect { subject }.not_to raise_error
       end
     end