From ce7c0ac3dbdc3f2f2f34b39bf5ef3755e79e9d42 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Thu, 31 Aug 2017 21:13:40 +0900
Subject: [PATCH] Add validation for protected attributes

---
 app/models/ci/build.rb                 | 1 +
 app/models/ci/pipeline.rb              | 1 +
 app/services/ci/retry_build_service.rb | 2 +-
 spec/factories/ci/builds.rb            | 5 +++--
 spec/factories/ci/pipelines.rb         | 9 +++++++++
 spec/models/ci/build_spec.rb           | 6 +++++-
 6 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index e58484878ba..11717a1bb12 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -26,6 +26,7 @@ module Ci
 
     validates :coverage, numericality: true, allow_blank: true
     validates :ref, presence: true
+    validates :protected, inclusion: { in: [ true, false ] }, on: :create
 
     scope :unstarted, ->() { where(runner_id: nil) }
     scope :ignore_failures, ->() { where(allow_failure: false) }
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 2d40f8012a3..d877e51c2a7 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -36,6 +36,7 @@ module Ci
     validates :sha, presence: { unless: :importing? }
     validates :ref, presence: { unless: :importing? }
     validates :status, presence: { unless: :importing? }
+    validates :protected, inclusion: { in: [ true, false ], unless: :importing? }, on: :create
     validate :valid_commit_sha, unless: :importing?
 
     after_create :keep_around_commits, unless: :importing?
diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb
index ea3b8d66ed9..d67b9f5cc56 100644
--- a/app/services/ci/retry_build_service.rb
+++ b/app/services/ci/retry_build_service.rb
@@ -3,7 +3,7 @@ module Ci
     CLONE_ACCESSORS = %i[pipeline project ref tag options commands name
                          allow_failure stage_id stage stage_idx trigger_request
                          yaml_variables when environment coverage_regex
-                         description tag_list].freeze
+                         description tag_list protected].freeze
 
     def execute(build)
       reprocess!(build).tap do |new_build|
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index f8922275860..bdc3e8acc07 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -12,6 +12,7 @@ FactoryGirl.define do
     started_at 'Di 29. Okt 09:51:28 CET 2013'
     finished_at 'Di 29. Okt 09:53:28 CET 2013'
     commands 'ls -a'
+    protected false
 
     options do
       {
@@ -227,11 +228,11 @@ FactoryGirl.define do
       self.when 'manual'
     end
 
-    trait(:protected) do
+    trait :protected do
       protected true
     end
 
-    trait(:unprotected) do
+    trait :unprotected do
       protected false
     end
   end
diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb
index e83a0e599a8..5b51f5898a3 100644
--- a/spec/factories/ci/pipelines.rb
+++ b/spec/factories/ci/pipelines.rb
@@ -4,6 +4,7 @@ FactoryGirl.define do
     ref 'master'
     sha '97de212e80737a608d939f648d959671fb0a0142'
     status 'pending'
+    protected false
 
     project
 
@@ -59,6 +60,14 @@ FactoryGirl.define do
       trait :failed do
         status :failed
       end
+
+      trait :protected do
+        protected true
+      end
+
+      trait :unprotected do
+        protected false
+      end
     end
   end
 end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index a94fdbf8434..cab59db3580 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -59,7 +59,11 @@ describe Ci::Build do
     end
 
     context 'when protected is false' do
-      let!(:job) { create(:ci_build, protected: nil) }
+      let!(:job) { create(:ci_build) }
+
+      before do
+        job.update_attribute(:protected, nil)
+      end
 
       it { is_expected.not_to include(job) }
     end