Improved validation of .gitlab-ci.yml

6 files changed, 30 insertions, 7 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 2edeb8c..577ed36 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,7 +1,6 @@
 v7.13.0
   - Fix: No runner notification can see managers only
   - Ability to cancel all builds in commit at once
-  - Improved Lint stability
   - Disable colors in rake tasks automatically (if IO is not a TTY)
   - Implemented "rake env:info". Rake task to receive system information
   - Fix coverage calculation on commit page
@@ -13,7 +12,7 @@ v7.12.1
   - Update webhook example
   - Improved Lint stability
   - Add warning when .gitlab-ci.yml not found
-  - Add one more validation rule for .gitlab-ci.yml
+  - Improved validation for .gitlab-ci.yml
   - Fix list of branches in only section
 
 v7.12.0
diff --git a/lib/gitlab_ci_yaml_processor.rb b/lib/gitlab_ci_yaml_processor.rb
index 18ce1ad..46093d6 100644
--- a/lib/gitlab_ci_yaml_processor.rb
+++ b/lib/gitlab_ci_yaml_processor.rb
@@ -54,6 +54,11 @@ class GitlabCiYamlProcessor
   def initial_parsing
     @before_script = @config[:before_script] || []
     @config.delete(:before_script)
+
+    unless @config.is_a?(Hash) && !@config.values.any?{|job| !job.is_a?(Hash)}
+      raise ValidationError, "Please define at least one job"
+    end
+
     @jobs = @config.select{|key, value| value[:type] != "deploy"}
     @deploy_jobs = @config.select{|key, value| value[:type] == "deploy"}
   end
@@ -100,6 +105,12 @@ class GitlabCiYamlProcessor
     end
 
     @jobs.each do |name, job|
+      job.keys.each do |key|
+        unless [:tags, :script, :only, :except, :type].include? key
+          raise ValidationError, "#{name} job: unknow parameter #{key}"
+        end
+      end
+
       if job[:tags] && !job[:tags].is_a?(Array)
         raise ValidationError, "#{name} job: tags parameter should be an array"
       end
@@ -114,6 +125,12 @@ class GitlabCiYamlProcessor
     end
 
     @deploy_jobs.each do |name, job|
+      job.keys.each do |key|
+        unless [:tags, :script, :only, :except, :type].include? key
+          raise ValidationError, "#{name} job: unknow parameter #{key}"
+        end
+      end
+      
       if job[:tags] && !job[:tags].is_a?(Array)
         raise ValidationError, "#{name} deploy job: tags parameter should be an array"
       end
diff --git a/spec/lib/gitlab_ci_yaml_processor_spec.rb b/spec/lib/gitlab_ci_yaml_processor_spec.rb
index 155fcf1..34a71f9 100644
--- a/spec/lib/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/gitlab_ci_yaml_processor_spec.rb
@@ -57,12 +57,12 @@ describe GitlabCiYamlProcessor do
     it "does not build tags" do
       config = YAML.dump({
         before_script: ["pwd"],
-        rspec: {script: "rspec", exclude: ["tags"]}
+        rspec: {script: "rspec", except: ["tags"]}
       })
 
       config_processor = GitlabCiYamlProcessor.new(config)
 
-      config_processor.builds_for_ref("0-1", true).size.should == 1
+      config_processor.builds_for_ref("0-1", true).size.should == 0
     end
   end
 
@@ -148,5 +148,12 @@ describe GitlabCiYamlProcessor do
         GitlabCiYamlProcessor.new(config)
       end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script should be an array")
     end
+
+    it "returns errors if there is no any jobs defined" do
+      config = YAML.dump({test: "bundle update"})
+      expect do
+        GitlabCiYamlProcessor.new(config)
+      end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Please define at least one job")
+    end
   end
 end
\ No newline at end of file
diff --git a/spec/models/project_services/hip_chat_message_spec.rb b/spec/models/project_services/hip_chat_message_spec.rb
index 5fa5549..1afe3ed 100644
--- a/spec/models/project_services/hip_chat_message_spec.rb
+++ b/spec/models/project_services/hip_chat_message_spec.rb
@@ -8,7 +8,7 @@ describe HipChatMessage do
   context "One build" do
     let(:commit) do
       commit = FactoryGirl.create(:commit, project: project)
-      commit.push_data[:ci_yaml_file] = YAML.dump({rspec: { test: 'pwd' }})
+      commit.push_data[:ci_yaml_file] = YAML.dump({rspec: { script: 'pwd' }})
       commit.save
       commit
     end
diff --git a/spec/models/project_services/slack_message_spec.rb b/spec/models/project_services/slack_message_spec.rb
index 857935d..f60f89c 100644
--- a/spec/models/project_services/slack_message_spec.rb
+++ b/spec/models/project_services/slack_message_spec.rb
@@ -8,7 +8,7 @@ describe SlackMessage do
   context "One build" do
     let(:commit) do
       commit = FactoryGirl.create(:commit, project: project)
-      commit.push_data[:ci_yaml_file] = YAML.dump({rspec: { test: "ls" }})
+      commit.push_data[:ci_yaml_file] = YAML.dump({rspec: { script: "ls" }})
       commit.save
       commit
     end
diff --git a/spec/services/create_commit_service_spec.rb b/spec/services/create_commit_service_spec.rb
index ad6ef8e..fe26ddb 100644
--- a/spec/services/create_commit_service_spec.rb
+++ b/spec/services/create_commit_service_spec.rb
@@ -37,7 +37,7 @@ describe CreateCommitService do
       end
 
       it "does not call create_deploy_builds if there is build" do
-        config = YAML.dump({rspec: {test: "ls"},production: {deploy: "ls"}})
+        config = YAML.dump({rspec: {script: "ls"},production: { script: "ls", type: "deploy"}})
         Commit.any_instance.should_not_receive(:create_deploy_builds)
         service.execute(project,
           ref: 'refs/heads/master',