Merge branch 'only-syntax' into 'master'

Extend yml syntax for only and except to support specifying repository path

This allows to limit execution of jobs to specific repository.

For example:
```yaml
job:
  only:
    - branches@gitlab-org/gitlab-ce
  except:
    - master@gitlab-org/gitlab-ce
```

The above will run `job` for all branches on `gitlab-org/gitlab-ce`, except master.

@dzaporozhets @JobV @vsizov Please review.


See merge request !1720

CHANGELOG

@@ -13,6 +13,7 @@ v 8.2.0 (unreleased)
   - Fix: Inability to reply to code comments in the MR view, if the MR comes from a fork
   - Use git follow flag for commits page when retrieve history for file or directory
   - Show merge request CI status on merge requests index page
+  - Extend yml syntax for only and except to support specifying repository path
   - Fix: 500 error returned if destroy request without HTTP referer (Kazuki Shimizu)
   - Remove deprecated CI events from project settings page
   - Use issue editor as cross reference comment author when issue is edited with a new mention.

app/models/ci/commit.rb

@@ -187,7 +187,7 @@ module Ci
     end
 
     def config_processor
-      @config_processor ||= Ci::GitlabCiYamlProcessor.new(ci_yaml_file)
+      @config_processor ||= Ci::GitlabCiYamlProcessor.new(ci_yaml_file, gl_project.path_with_namespace)
     rescue Ci::GitlabCiYamlProcessor::ValidationError => e
       save_yaml_error(e.message)
       nil

doc/ci/yaml/README.md

@@ -169,7 +169,7 @@ This are two parameters that allow for setting a refs policy to limit when jobs
 
 There are a few rules that apply to usage of refs policy:
 
-1. `only` and `except` are exclusive. If both `only` and `except` are defined in job specification only `only` is taken into account.
+1. `only` and `except` are inclusive. If both `only` and `except` are defined in job specification the ref is filtered by `only` and `except`.
 1. `only` and `except` allow for using the regexp expressions.
 1. `only` and `except` allow for using special keywords: `branches` and `tags`.
 These names can be used for example to exclude all tags and all branches.
@@ -182,6 +182,18 @@ job:
     - branches # use special keyword
 ```
 
+1. `only` and `except` allow for specify repository path to filter jobs for forks.
+The repository path can be used to have jobs executed only for parent repository.
+
+```yaml
+job:
+  only:
+    - branches@gitlab-org/gitlab-ce
+  except:
+    - master@gitlab-org/gitlab-ce
+```
+The above will run `job` for all branches on `gitlab-org/gitlab-ce`, except master .
+
 ### tags
 `tags` is used to select specific runners from the list of all runners that are allowed to run this project.
 

lib/ci/gitlab_ci_yaml_processor.rb

@@ -7,10 +7,11 @@ module Ci
     ALLOWED_YAML_KEYS = [:before_script, :image, :services, :types, :stages, :variables]
     ALLOWED_JOB_KEYS = [:tags, :script, :only, :except, :type, :image, :services, :allow_failure, :type, :stage, :when]
 
-    attr_reader :before_script, :image, :services, :variables
+    attr_reader :before_script, :image, :services, :variables, :path
 
-    def initialize(config)
+    def initialize(config, path = nil)
       @config = YAML.load(config)
+      @path = path
 
       unless @config.is_a? Hash
         raise ValidationError, "YAML should be a hash"
@@ -63,26 +64,6 @@ module Ci
       end
     end
 
-    def process?(only_params, except_params, ref, tag)
-      return true if only_params.nil? && except_params.nil?
-
-      if only_params
-        return true if tag && only_params.include?("tags")
-        return true if !tag && only_params.include?("branches")
-        
-        only_params.find do |pattern|
-          match_ref?(pattern, ref)
-        end
-      else
-        return false if tag && except_params.include?("tags")
-        return false if !tag && except_params.include?("branches")
-
-        except_params.each do |pattern|
-          return false if match_ref?(pattern, ref)
-        end
-      end
-    end
-
     def build_job(name, job)
       {
         stage_idx: stages.index(job[:stage]),
@@ -101,14 +82,6 @@ module Ci
       }
     end
 
-    def match_ref?(pattern, ref)
-      if pattern.first == "/" && pattern.last == "/"
-        Regexp.new(pattern[1...-1]) =~ ref
-      else
-        pattern == ref
-      end
-    end
-
     def normalize_script(script)
       if script.is_a? Array
         script.join("\n")
@@ -208,5 +181,36 @@ module Ci
     def validate_string(value)
       value.is_a?(String) || value.is_a?(Symbol)
     end
+
+    def process?(only_params, except_params, ref, tag)
+      if only_params.present?
+        return false unless matching?(only_params, ref, tag)
+      end
+
+      if except_params.present?
+        return false if matching?(except_params, ref, tag)
+      end
+
+      true
+    end
+
+    def matching?(patterns, ref, tag)
+      patterns.any? do |pattern|
+        match_ref?(pattern, ref, tag)
+      end
+    end
+
+    def match_ref?(pattern, ref, tag)
+      pattern, path = pattern.split('@', 2)
+      return false if path && path != self.path
+      return true if tag && pattern == 'tags'
+      return true if !tag && pattern == 'branches'
+
+      if pattern.first == "/" && pattern.last == "/"
+        Regexp.new(pattern[1...-1]) =~ ref
+      else
+        pattern == ref
+      end
+    end
   end
 end

spec/lib/ci/gitlab_ci_yaml_processor_spec.rb

@@ -2,6 +2,7 @@ require 'spec_helper'
 
 module Ci
   describe GitlabCiYamlProcessor do
+    let(:path) { 'path' }
     
     describe "#builds_for_ref" do
       let(:type) { 'test' }
@@ -12,7 +13,7 @@ module Ci
           rspec: { script: "rspec" }
         })
 
-        config_processor = GitlabCiYamlProcessor.new(config)
+        config_processor = GitlabCiYamlProcessor.new(config, path)
 
         expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(1)
         expect(config_processor.builds_for_stage_and_ref(type, "master").first).to eq({
@@ -29,13 +30,14 @@ module Ci
         })
       end
       
+      describe :only do
         it "does not return builds if only has another branch" do
           config = YAML.dump({
                                before_script: ["pwd"],
                                rspec: { script: "rspec", only: ["deploy"] }
                              })
 
-        config_processor = GitlabCiYamlProcessor.new(config)
+          config_processor = GitlabCiYamlProcessor.new(config, path)
 
           expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(0)
         end
@@ -46,7 +48,7 @@ module Ci
                                rspec: { script: "rspec", only: ["/^deploy$/"] }
                              })
 
-        config_processor = GitlabCiYamlProcessor.new(config)
+          config_processor = GitlabCiYamlProcessor.new(config, path)
 
           expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(0)
         end
@@ -57,49 +59,188 @@ module Ci
                                rspec: { script: "rspec", only: ["master"] }
                              })
 
-        config_processor = GitlabCiYamlProcessor.new(config)
+          config_processor = GitlabCiYamlProcessor.new(config, path)
 
           expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(1)
         end
 
-      it "does not build tags" do
+        it "returns builds if only has a list of branches including specified" do
           config = YAML.dump({
                                before_script: ["pwd"],
-          rspec: { script: "rspec", except: ["tags"] }
+                               rspec: { script: "rspec", type: type, only: ["master", "deploy"] }
                              })
 
-        config_processor = GitlabCiYamlProcessor.new(config)
+          config_processor = GitlabCiYamlProcessor.new(config, path)
 
-        expect(config_processor.builds_for_stage_and_ref(type, "0-1", true).size).to eq(0)
+          expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(1)
         end
 
-      it "returns builds if only has a list of branches including specified" do
+        it "returns builds if only has a branches keyword specified" do
           config = YAML.dump({
                                before_script: ["pwd"],
-                             rspec: { script: "rspec", type: type, only: ["master", "deploy"] }
+                               rspec: { script: "rspec", type: type, only: ["branches"] }
                              })
 
-        config_processor = GitlabCiYamlProcessor.new(config)
+          config_processor = GitlabCiYamlProcessor.new(config, path)
 
           expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(1)
         end
 
+        it "does not return builds if only has a tags keyword" do
+          config = YAML.dump({
+                               before_script: ["pwd"],
+                               rspec: { script: "rspec", type: type, only: ["tags"] }
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config, path)
+
+          expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(0)
+        end
+
+        it "returns builds if only has current repository path" do
+          config = YAML.dump({
+                               before_script: ["pwd"],
+                               rspec: { script: "rspec", type: type, only: ["branches@path"] }
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config, path)
+
+          expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(1)
+        end
+
+        it "does not return builds if only has different repository path" do
+          config = YAML.dump({
+                               before_script: ["pwd"],
+                               rspec: { script: "rspec", type: type, only: ["branches@fork"] }
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config, path)
+
+          expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(0)
+        end
+
         it "returns build only for specified type" do
 
           config = YAML.dump({
                                before_script: ["pwd"],
-                             build: { script: "build", type: "build", only: ["master", "deploy"] },
-                             rspec: { script: "rspec", type: type, only: ["master", "deploy"] },
+                               rspec: { script: "rspec", type: "test", only: ["master", "deploy"] },
                                staging: { script: "deploy", type: "deploy", only: ["master", "deploy"] },
-                             production: { script: "deploy", type: "deploy", only: ["master", "deploy"] },
+                               production: { script: "deploy", type: "deploy", only: ["master@path", "deploy"] },
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config, 'fork')
+
+          expect(config_processor.builds_for_stage_and_ref("deploy", "deploy").size).to eq(2)
+          expect(config_processor.builds_for_stage_and_ref("test", "deploy").size).to eq(1)
+          expect(config_processor.builds_for_stage_and_ref("deploy", "master").size).to eq(1)
+        end
+      end
+
+      describe :except do
+        it "returns builds if except has another branch" do
+          config = YAML.dump({
+                               before_script: ["pwd"],
+                               rspec: { script: "rspec", except: ["deploy"] }
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config, path)
+
+          expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(1)
+        end
+
+        it "returns builds if except has regexp with another branch" do
+          config = YAML.dump({
+                               before_script: ["pwd"],
+                               rspec: { script: "rspec", except: ["/^deploy$/"] }
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config, path)
+
+          expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(1)
+        end
+
+        it "does not return builds if except has specified this branch" do
+          config = YAML.dump({
+                               before_script: ["pwd"],
+                               rspec: { script: "rspec", except: ["master"] }
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config, path)
+
+          expect(config_processor.builds_for_stage_and_ref(type, "master").size).to eq(0)
+        end
+
+        it "does not return builds if except has a list of branches including specified" do
+          config = YAML.dump({
+                               before_script: ["pwd"],
+                               rspec: { script: "rspec", type: type, except: ["master", "deploy"] }
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config, path)
+
+          expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(0)
+        end
+
+        it "does not return builds if except has a branches keyword specified" do
+          config = YAML.dump({
+                               before_script: ["pwd"],
+                               rspec: { script: "rspec", type: type, except: ["branches"] }
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config, path)
+
+          expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(0)
+        end
+
+        it "returns builds if except has a tags keyword" do
+          config = YAML.dump({
+                               before_script: ["pwd"],
+                               rspec: { script: "rspec", type: type, except: ["tags"] }
                              })
 
-        config_processor = GitlabCiYamlProcessor.new(config)
+          config_processor = GitlabCiYamlProcessor.new(config, path)
 
-        expect(config_processor.builds_for_stage_and_ref("production", "deploy").size).to eq(0)
           expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(1)
+        end
+
+        it "does not return builds if except has current repository path" do
+          config = YAML.dump({
+                               before_script: ["pwd"],
+                               rspec: { script: "rspec", type: type, except: ["branches@path"] }
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config, path)
+
+          expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(0)
+        end
+
+        it "returns builds if except has different repository path" do
+          config = YAML.dump({
+                               before_script: ["pwd"],
+                               rspec: { script: "rspec", type: type, except: ["branches@fork"] }
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config, path)
+
+          expect(config_processor.builds_for_stage_and_ref(type, "deploy").size).to eq(1)
+        end
+
+        it "returns build except specified type" do
+          config = YAML.dump({
+                               before_script: ["pwd"],
+                               rspec: { script: "rspec", type: "test", except: ["master", "deploy", "test@fork"] },
+                               staging: { script: "deploy", type: "deploy", except: ["master"] },
+                               production: { script: "deploy", type: "deploy", except: ["master@fork"] },
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config, 'fork')
+
           expect(config_processor.builds_for_stage_and_ref("deploy", "deploy").size).to eq(2)
+          expect(config_processor.builds_for_stage_and_ref("test", "test").size).to eq(0)
+          expect(config_processor.builds_for_stage_and_ref("deploy", "master").size).to eq(0)
+        end
       end
+
     end
 
     describe "Image and service handling" do
@@ -111,7 +252,7 @@ module Ci
                              rspec: { script: "rspec" }
                            })
 
-        config_processor = GitlabCiYamlProcessor.new(config)
+        config_processor = GitlabCiYamlProcessor.new(config, path)
 
         expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1)
         expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
@@ -139,7 +280,7 @@ module Ci
                              rspec:         { image: "ruby:2.5", services: ["postgresql"], script: "rspec" }
                            })
 
-        config_processor = GitlabCiYamlProcessor.new(config)
+        config_processor = GitlabCiYamlProcessor.new(config, path)
 
         expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1)
         expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
@@ -172,7 +313,7 @@ module Ci
                              rspec: { script: "rspec" }
                            })
 
-        config_processor = GitlabCiYamlProcessor.new(config)
+        config_processor = GitlabCiYamlProcessor.new(config, path)
         expect(config_processor.variables).to eq(variables)
       end
     end
@@ -184,7 +325,7 @@ module Ci
                                rspec: { script: "rspec", when: when_state }
                              })
 
-          config_processor = GitlabCiYamlProcessor.new(config)
+          config_processor = GitlabCiYamlProcessor.new(config, path)
           builds = config_processor.builds_for_stage_and_ref("test", "master")
           expect(builds.size).to eq(1)
           expect(builds.first[:when]).to eq(when_state)
@@ -200,154 +341,154 @@ module Ci
       it "returns errors if tags parameter is invalid" do
         config = YAML.dump({ rspec: { script: "test", tags: "mysql" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: tags parameter should be an array of strings")
       end
 
       it "returns errors if before_script parameter is invalid" do
         config = YAML.dump({ before_script: "bundle update", rspec: { script: "test" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script should be an array of strings")
       end
 
       it "returns errors if image parameter is invalid" do
         config = YAML.dump({ image: ["test"], rspec: { script: "test" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "image should be a string")
       end
 
       it "returns errors if job name is blank" do
         config = YAML.dump({ '' => { script: "test" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "job name should be non-empty string")
       end
 
       it "returns errors if job name is non-string" do
         config = YAML.dump({ 10 => { script: "test" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "job name should be non-empty string")
       end
 
       it "returns errors if job image parameter is invalid" do
         config = YAML.dump({ rspec: { script: "test", image: ["test"] } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: image should be a string")
       end
 
       it "returns errors if services parameter is not an array" do
         config = YAML.dump({ services: "test", rspec: { script: "test" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services should be an array of strings")
       end
 
       it "returns errors if services parameter is not an array of strings" do
         config = YAML.dump({ services: [10, "test"], rspec: { script: "test" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services should be an array of strings")
       end
 
       it "returns errors if job services parameter is not an array" do
         config = YAML.dump({ rspec: { script: "test", services: "test" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: services should be an array of strings")
       end
 
       it "returns errors if job services parameter is not an array of strings" do
         config = YAML.dump({ rspec: { script: "test", services: [10, "test"] } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: services should be an array of strings")
       end
 
       it "returns errors if there are unknown parameters" do
         config = YAML.dump({ extra: "bundle update" })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Unknown parameter: extra")
       end
 
       it "returns errors if there are unknown parameters that are hashes, but doesn't have a script" do
         config = YAML.dump({ extra: { services: "test" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Unknown parameter: extra")
       end
 
       it "returns errors if there is no any jobs defined" do
         config = YAML.dump({ before_script: ["bundle update"] })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Please define at least one job")
       end
 
       it "returns errors if job allow_failure parameter is not an boolean" do
         config = YAML.dump({ rspec: { script: "test", allow_failure: "string" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: allow_failure parameter should be an boolean")
       end
 
       it "returns errors if job stage is not a string" do
         config = YAML.dump({ rspec: { script: "test", type: 1, allow_failure: "string" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test, deploy")
       end
 
       it "returns errors if job stage is not a pre-defined stage" do
         config = YAML.dump({ rspec: { script: "test", type: "acceptance", allow_failure: "string" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test, deploy")
       end
 
       it "returns errors if job stage is not a defined stage" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", type: "acceptance", allow_failure: "string" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test")
       end
 
       it "returns errors if stages is not an array" do
         config = YAML.dump({ types: "test", rspec: { script: "test" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages should be an array of strings")
       end
 
       it "returns errors if stages is not an array of strings" do
         config = YAML.dump({ types: [true, "test"], rspec: { script: "test" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages should be an array of strings")
       end
 
       it "returns errors if variables is not a map" do
         config = YAML.dump({ variables: "test", rspec: { script: "test" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables should be a map of key-valued strings")
       end
 
       it "returns errors if variables is not a map of key-valued strings" do
         config = YAML.dump({ variables: { test: false }, rspec: { script: "test" } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables should be a map of key-valued strings")
       end
 
       it "returns errors if job when is not on_success, on_failure or always" do
         config = YAML.dump({ rspec: { script: "test", when: 1 } })
         expect do
-          GitlabCiYamlProcessor.new(config)
+          GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: when parameter should be on_success, on_failure or always")
       end
     end