Merge branch '5078-ci-include-deep-merge' into 'master'

Resolve "`include` not working"

Closes #5078

See merge request gitlab-org/gitlab-ee!5527

doc/ci/yaml/README.md

@@ -1201,6 +1201,7 @@ test:
 
 > Introduced in [GitLab Edition Premium][ee] 10.5.
 > Available for Starter, Premium and Ultimate [versions][gitlab-versions] since 10.6.
+> Behaviour expanded in GitLab 10.8 to allow more flexible overriding
 
 Using the `include` keyword, you can allow the inclusion of external YAML files.
 
@@ -1276,10 +1277,16 @@ include:
 
 ---
 
-Since external files defined by `include` are evaluated first, the content of
-`.gitlab-ci.yml` will always take precedence over the content of the external
-files, no matter of the position of the `include` keyword. This allows you to
-override values and functions with local definitions. For example:
+
+Since GitLab 10.8 we are now recursively merging the files defined in `include`
+with those in `.gitlab-ci.yml`. Files defined by `include` are always
+evaluated first and recursively merged with the content of `.gitlab-ci.yml`, no
+matter the position of the `include` keyword. You can take advantage of
+recursive merging to customize and override details in included CI
+configurations with local definitions.
+
+The following example shows specific YAML-defined variables and details of the
+`production` job from an include file being customized in `.gitlab-ci.yml`.
 
 ```yaml
 # Content of https://company.com/autodevops-template.yml
@@ -1311,7 +1318,6 @@ image: alpine:latest
 variables:
   POSTGRES_USER: root
   POSTGRES_PASSWORD: secure_password
-  POSTGRES_DB: company_database
 
 stages:
   - build
@@ -1319,26 +1325,57 @@ stages:
   - production
 
 production:
-  stage: production
-  script:
-    - install_dependencies
-    - deploy
   environment:
-    name: production
     url: https://domain.com
-  only:
-    - master
 ```
 
-In this case, the variables `POSTGRES_USER`, `POSTGRES_PASSWORD` and
-`POSTGRES_DB` along with the `production` job defined in
-`autodevops-template.yml` will be overridden by the ones defined in
+In this case, the variables `POSTGRES_USER` and `POSTGRES_PASSWORD` along
+with the environment url of the `production` job defined in
+`autodevops-template.yml` have been overridden by new values defined in
 `.gitlab-ci.yml`.
 
 NOTE: **Note:**
 Recursive includes are not supported meaning your external files
 should not use the `include` keyword, as it will be ignored.
 
+Recursive merging lets you extend and override dictionary mappings, but
+you cannot add or modify items to an included array. For example, to add
+an additional item to the production job script, you must repeat the
+existing script items.
+
+```yaml
+# Content of https://company.com/autodevops-template.yml
+
+production:
+  stage: production
+  script:
+    - install_dependencies
+    - deploy
+```
+
+```yaml
+# Content of .gitlab-ci.yml
+
+include: 'https://company.com/autodevops-template.yml'
+
+stages:
+  - production
+
+production:
+  script:
+    - install_depedencies
+    - deploy
+    - notify_owner
+```
+
+In this case, if `install_dependencies` and `deploy` were not repeated in
+`.gitlab-ci.yml`, they would not be part of the script for the `production`
+job in the combined CI configuration.
+
+NOTE: **Note:**
+We currently do not support using YAML aliases across different YAML files
+sourced by `include`. You must only refer to aliases in the same file.
+
 ## `variables`
 
 > Introduced in GitLab Runner v0.5.0.

ee/changelogs/unreleased/5078-ci-include-deep-merge.yml

+---
+title: Allow easier customization of included CI configurations
+merge_request: 5288
+author: King Chung Huang
+type: changed

ee/lib/gitlab/ci/external/processor.rb

@@ -15,7 +15,7 @@ module Gitlab
 
           external_files.each do |external_file|
             validate_external_file(external_file)
-            @content.merge!(content_of(external_file))
+            @content.deep_merge!(content_of(external_file))
           end
 
           append_inline_content
@@ -37,7 +37,7 @@ module Gitlab
         end
 
         def append_inline_content
-          @content.merge!(@values)
+          @content.deep_merge!(@values)
         end
 
         def remove_include_keyword

ee/spec/lib/ee/gitlab/ci/config_spec.rb

@@ -111,5 +111,118 @@ describe EE::Gitlab::Ci::Config do
         expect(config.to_hash).to eq({ image: 'ruby:2.2' })
       end
     end
+
+    context "when both external files and gitlab_ci.yml define a dictionary of distinct variables" do
+      let(:remote_file_content) do
+        <<~HEREDOC
+        variables:
+          A: 'alpha'
+          B: 'beta'
+        HEREDOC
+      end
+
+      let(:gitlab_ci_yml) do
+        <<~HEREDOC
+        include:
+          - #{remote_location}
+
+        variables:
+          C: 'gamma'
+          D: 'delta'
+        HEREDOC
+      end
+
+      it 'should merge the variables dictionaries' do
+        WebMock.stub_request(:get, remote_location).to_return(body: remote_file_content)
+        expect(config.to_hash).to eq({ variables: { A: 'alpha', B: 'beta', C: 'gamma', D: 'delta' } })
+      end
+    end
+
+    context "when both external files and gitlab_ci.yml define a dictionary of overlapping variables" do
+      let(:remote_file_content) do
+        <<~HEREDOC
+        variables:
+          A: 'alpha'
+          B: 'beta'
+          C: 'omnicron'
+        HEREDOC
+      end
+
+      let(:gitlab_ci_yml) do
+        <<~HEREDOC
+        include:
+          - #{remote_location}
+
+        variables:
+          C: 'gamma'
+          D: 'delta'
+        HEREDOC
+      end
+
+      it 'later declarations should take precedence' do
+        WebMock.stub_request(:get, remote_location).to_return(body: remote_file_content)
+        expect(config.to_hash).to eq({ variables: { A: 'alpha', B: 'beta', C: 'gamma', D: 'delta' } })
+      end
+    end
+
+    context 'when both external files and gitlab_ci.yml define a job' do
+      let(:remote_file_content) do
+        <<~HEREDOC
+        job1:
+          script:
+          - echo 'hello from remote file'
+        HEREDOC
+      end
+
+      let(:gitlab_ci_yml) do
+        <<~HEREDOC
+        include:
+          - #{remote_location}
+
+        job1:
+          variables:
+            VARIABLE_DEFINED_IN_MAIN_FILE: 'some value'
+        HEREDOC
+      end
+
+      it 'merges the jobs' do
+        WebMock.stub_request(:get, remote_location).to_return(body: remote_file_content)
+        expect(config.to_hash).to eq({
+          job1: {
+            script: ["echo 'hello from remote file'"],
+            variables: {
+              VARIABLE_DEFINED_IN_MAIN_FILE: 'some value'
+            }
+          }
+        })
+      end
+
+      context 'when the script key is in both' do
+        let(:gitlab_ci_yml) do
+          <<~HEREDOC
+          include:
+            - #{remote_location}
+
+          job1:
+            script:
+            - echo 'hello from main file'
+            variables:
+              VARIABLE_DEFINED_IN_MAIN_FILE: 'some value'
+          HEREDOC
+        end
+
+        it 'uses the script from the gitlab_ci.yml' do
+          WebMock.stub_request(:get, remote_location).to_return(body: remote_file_content)
+          expect(config.to_hash).to eq({
+            job1: {
+              script: ["echo 'hello from main file'"],
+              variables: {
+                VARIABLE_DEFINED_IN_MAIN_FILE: 'some value'
+              }
+            }
+          })
+        end
+      end
+    end
   end
 end