diff options
38 files changed, 383 insertions, 39 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 8adaafe6439..ba3156154ac 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -3,6 +3,7 @@ module Ci include TokenAuthenticatable include AfterCommitQueue include Presentable + include Importable belongs_to :runner belongs_to :trigger_request @@ -26,6 +27,7 @@ module Ci validates :coverage, numericality: true, allow_blank: true validates :ref, presence: true + validates :protected, inclusion: { in: [true, false], unless: :importing? }, on: :create scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } @@ -34,6 +36,7 @@ module Ci scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) } scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) } scope :manual_actions, ->() { where(when: :manual, status: COMPLETED_STATUSES + [:manual]) } + scope :ref_protected, -> { where(protected: true) } mount_uploader :artifacts_file, ArtifactUploader mount_uploader :artifacts_metadata, ArtifactUploader diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 2d40f8012a3..ca9a350ea79 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/models/ci/runner.rb b/app/models/ci/runner.rb index 906a76ec560..b1798084787 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -5,7 +5,7 @@ module Ci RUNNER_QUEUE_EXPIRY_TIME = 60.minutes ONLINE_CONTACT_TIMEOUT = 1.hour AVAILABLE_SCOPES = %w[specific shared active paused online].freeze - FORM_EDITABLE = %i[description tag_list active run_untagged locked].freeze + FORM_EDITABLE = %i[description tag_list active run_untagged locked access_level].freeze has_many :builds has_many :runner_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent @@ -35,11 +35,17 @@ module Ci end validate :tag_constraints + validates :access_level, presence: true acts_as_taggable after_destroy :cleanup_runner_queue + enum access_level: { + not_protected: 0, + ref_protected: 1 + } + # Searches for runners matching the given query. # # This method uses ILIKE on PostgreSQL and LIKE on MySQL. @@ -106,6 +112,8 @@ module Ci end def can_pick?(build) + return false if self.ref_protected? && !build.protected? + assignable_for?(build.project) && accepting_tags?(build) end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index de2cd7e87be..414c01b2546 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -12,7 +12,8 @@ module Ci tag: tag?, trigger_requests: Array(trigger_request), user: current_user, - pipeline_schedule: schedule + pipeline_schedule: schedule, + protected: project.protected_for?(ref) ) result = validate(current_user, diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index 414f672cc6a..b8db709211a 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -77,7 +77,9 @@ module Ci end def new_builds - Ci::Build.pending.unstarted + builds = Ci::Build.pending.unstarted + builds = builds.ref_protected if runner.ref_protected? + builds end def shared_runner_build_limits_feature_enabled? 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/app/views/projects/runners/_form.html.haml b/app/views/projects/runners/_form.html.haml index 2ef1f98ba48..ac8e15a48b2 100644 --- a/app/views/projects/runners/_form.html.haml +++ b/app/views/projects/runners/_form.html.haml @@ -7,6 +7,12 @@ = f.check_box :active %span.light Paused Runners don't accept new jobs .form-group + = label :protected, "Protected", class: 'control-label' + .col-sm-10 + .checkbox + = f.check_box :access_level, {}, 'ref_protected', 'not_protected' + %span.light This runner will only run on pipelines trigged on protected branches + .form-group = label :run_untagged, 'Run untagged jobs', class: 'control-label' .col-sm-10 .checkbox diff --git a/app/views/projects/runners/show.html.haml b/app/views/projects/runners/show.html.haml index 49415ba557b..dfab04aa1fb 100644 --- a/app/views/projects/runners/show.html.haml +++ b/app/views/projects/runners/show.html.haml @@ -20,6 +20,9 @@ %td Active %td= @runner.active? ? 'Yes' : 'No' %tr + %td Protected + %td= @runner.ref_protected? ? 'Yes' : 'No' + %tr %td Can run untagged jobs %td= @runner.run_untagged? ? 'Yes' : 'No' %tr diff --git a/changelogs/unreleased/feature-sm-33281-protected-runner-executes-jobs-on-protected-branch.yml b/changelogs/unreleased/feature-sm-33281-protected-runner-executes-jobs-on-protected-branch.yml new file mode 100644 index 00000000000..b57b9a3dfbe --- /dev/null +++ b/changelogs/unreleased/feature-sm-33281-protected-runner-executes-jobs-on-protected-branch.yml @@ -0,0 +1,5 @@ +--- +title: Protected runners +merge_request: 13194 +author: +type: added diff --git a/db/migrate/20170816133938_add_access_level_to_ci_runners.rb b/db/migrate/20170816133938_add_access_level_to_ci_runners.rb new file mode 100644 index 00000000000..fc484730f42 --- /dev/null +++ b/db/migrate/20170816133938_add_access_level_to_ci_runners.rb @@ -0,0 +1,16 @@ +class AddAccessLevelToCiRunners < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default(:ci_runners, :access_level, :integer, + default: Ci::Runner.access_levels['not_protected']) + end + + def down + remove_column(:ci_runners, :access_level) + end +end diff --git a/db/migrate/20170816133940_add_protected_to_ci_builds.rb b/db/migrate/20170816133940_add_protected_to_ci_builds.rb new file mode 100644 index 00000000000..c73a4387d29 --- /dev/null +++ b/db/migrate/20170816133940_add_protected_to_ci_builds.rb @@ -0,0 +1,7 @@ +class AddProtectedToCiBuilds < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column :ci_builds, :protected, :boolean + end +end diff --git a/db/migrate/20170816143940_add_protected_to_ci_pipelines.rb b/db/migrate/20170816143940_add_protected_to_ci_pipelines.rb new file mode 100644 index 00000000000..ce8f1e03686 --- /dev/null +++ b/db/migrate/20170816143940_add_protected_to_ci_pipelines.rb @@ -0,0 +1,7 @@ +class AddProtectedToCiPipelines < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column :ci_pipelines, :protected, :boolean + end +end diff --git a/db/migrate/20170816153940_add_index_on_ci_builds_protected.rb b/db/migrate/20170816153940_add_index_on_ci_builds_protected.rb new file mode 100644 index 00000000000..caf7c705a6e --- /dev/null +++ b/db/migrate/20170816153940_add_index_on_ci_builds_protected.rb @@ -0,0 +1,15 @@ +class AddIndexOnCiBuildsProtected < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :ci_builds, :protected + end + + def down + remove_concurrent_index :ci_builds, :protected if index_exists?(:ci_builds, :protected) + end +end diff --git a/db/schema.rb b/db/schema.rb index 434d1326419..a5f867df9ae 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -246,6 +246,7 @@ ActiveRecord::Schema.define(version: 20170824162758) do t.integer "auto_canceled_by_id" t.boolean "retried" t.integer "stage_id" + t.boolean "protected" end add_index "ci_builds", ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree @@ -254,6 +255,7 @@ ActiveRecord::Schema.define(version: 20170824162758) do add_index "ci_builds", ["commit_id", "type", "name", "ref"], name: "index_ci_builds_on_commit_id_and_type_and_name_and_ref", using: :btree add_index "ci_builds", ["commit_id", "type", "ref"], name: "index_ci_builds_on_commit_id_and_type_and_ref", using: :btree add_index "ci_builds", ["project_id"], name: "index_ci_builds_on_project_id", using: :btree + add_index "ci_builds", ["protected"], name: "index_ci_builds_on_protected", using: :btree add_index "ci_builds", ["runner_id"], name: "index_ci_builds_on_runner_id", using: :btree add_index "ci_builds", ["stage_id"], name: "index_ci_builds_on_stage_id", using: :btree add_index "ci_builds", ["status", "type", "runner_id"], name: "index_ci_builds_on_status_and_type_and_runner_id", using: :btree @@ -336,6 +338,7 @@ ActiveRecord::Schema.define(version: 20170824162758) do t.integer "auto_canceled_by_id" t.integer "pipeline_schedule_id" t.integer "source" + t.boolean "protected" end add_index "ci_pipelines", ["auto_canceled_by_id"], name: "index_ci_pipelines_on_auto_canceled_by_id", using: :btree @@ -371,6 +374,7 @@ ActiveRecord::Schema.define(version: 20170824162758) do t.string "architecture" t.boolean "run_untagged", default: true, null: false t.boolean "locked", default: false, null: false + t.integer "access_level", default: 0, null: false end add_index "ci_runners", ["contacted_at"], name: "index_ci_runners_on_contacted_at", using: :btree diff --git a/doc/api/runners.md b/doc/api/runners.md index 16d362a3530..6304a496f94 100644 --- a/doc/api/runners.md +++ b/doc/api/runners.md @@ -138,7 +138,8 @@ Example response: "ruby", "mysql" ], - "version": null + "version": null, + "access_level": "ref_protected" } ``` @@ -156,6 +157,9 @@ PUT /runners/:id | `description` | string | no | The description of a runner | | `active` | boolean | no | The state of a runner; can be set to `true` or `false` | | `tag_list` | array | no | The list of tags for a runner; put array of tags, that should be finally assigned to a runner | +| `run_untagged` | boolean | no | Flag indicating the runner can execute untagged jobs | +| `locked` | boolean | no | Flag indicating the runner is locked | +| `access_level` | string | no | The access_level of the runner; `not_protected` or `ref_protected` | ``` curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/runners/6" --form "description=test-1-20150125-test" --form "tag_list=ruby,mysql,tag1,tag2" @@ -190,7 +194,8 @@ Example response: "tag1", "tag2" ], - "version": null + "version": null, + "access_level": "ref_protected" } ``` diff --git a/doc/ci/runners/README.md b/doc/ci/runners/README.md index 76d746155eb..4ccf1b56771 100644 --- a/doc/ci/runners/README.md +++ b/doc/ci/runners/README.md @@ -107,6 +107,26 @@ To lock/unlock a Runner: 1. Check the **Lock to current projects** option 1. Click **Save changes** for the changes to take effect +## Protected Runners + +>**Notes:** +[Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13194) +in GitLab 10.0. + +You can protect Runners from revealing sensitive information. +Whenever a Runner is protected, the Runner picks only jobs created on +[protected branches] or [protected tags], and ignores other jobs. + +To protect/unprotect Runners: + +1. Visit your project's **Settings ➔ Pipelines** +1. Find a Runner you want to protect/unprotect and make sure it's enabled +1. Click the pencil button besides the Runner name +1. Check the **Protected** option +1. Click **Save changes** for the changes to take effect + + + ## How shared Runners pick jobs Shared Runners abide to a process queue we call fair usage. The fair usage @@ -218,3 +238,5 @@ We're always looking for contributions that can mitigate these [install]: http://docs.gitlab.com/runner/install/ [fifo]: https://en.wikipedia.org/wiki/FIFO_(computing_and_electronics) [register]: http://docs.gitlab.com/runner/register/ +[protected branches]: ../../user/project/protected_branches.md +[protected tags]: ../../user/project/protected_tags.md diff --git a/doc/ci/runners/img/protected_runners_check_box.png b/doc/ci/runners/img/protected_runners_check_box.png Binary files differnew file mode 100644 index 00000000000..fb58498c7ce --- /dev/null +++ b/doc/ci/runners/img/protected_runners_check_box.png diff --git a/features/steps/project/pages.rb b/features/steps/project/pages.rb index 9705470738e..124a132d688 100644 --- a/features/steps/project/pages.rb +++ b/features/steps/project/pages.rb @@ -37,7 +37,8 @@ class Spinach::Features::ProjectPages < Spinach::FeatureSteps step 'pages are deployed' do pipeline = @project.pipelines.create(ref: 'HEAD', sha: @project.commit('HEAD').sha, - source: :push) + source: :push, + protected: false) build = build(:ci_build, project: @project, diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index 78e889a4c35..6314ea63197 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -74,7 +74,8 @@ module API source: :external, sha: commit.sha, ref: ref, - user: current_user) + user: current_user, + protected: @project.protected_for?(ref)) end status = GenericCommitStatus.running_or_pending.find_or_initialize_by( @@ -82,7 +83,8 @@ module API pipeline: pipeline, name: name, ref: ref, - user: current_user + user: current_user, + protected: @project.protected_for?(ref) ) optional_attributes = diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 9df9a515990..f13f2d723bb 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -775,6 +775,7 @@ module API expose :tag_list expose :run_untagged expose :locked + expose :access_level expose :version, :revision, :platform, :architecture expose :contacted_at expose :token, if: lambda { |runner, options| options[:current_user].admin? || !runner.is_shared? } diff --git a/lib/api/runners.rb b/lib/api/runners.rb index 1ea9a7918d7..d3559ef71be 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -55,7 +55,9 @@ module API optional :tag_list, type: Array[String], desc: 'The list of tags for a runner' optional :run_untagged, type: Boolean, desc: 'Flag indicating the runner can execute untagged jobs' optional :locked, type: Boolean, desc: 'Flag indicating the runner is locked' - at_least_one_of :description, :active, :tag_list, :run_untagged, :locked + optional :access_level, type: String, values: Ci::Runner.access_levels.keys, + desc: 'The access_level of the runner' + at_least_one_of :description, :active, :tag_list, :run_untagged, :locked, :access_level end put ':id' do runner = get_runner(params.delete(:id)) diff --git a/lib/gitlab/ci/stage/seed.rb b/lib/gitlab/ci/stage/seed.rb index f81f9347b4d..e19aae35a81 100644 --- a/lib/gitlab/ci/stage/seed.rb +++ b/lib/gitlab/ci/stage/seed.rb @@ -28,7 +28,8 @@ module Gitlab attributes.merge(project: project, ref: pipeline.ref, tag: pipeline.tag, - trigger_request: trigger) + trigger_request: trigger, + protected: protected_ref?) end end @@ -43,6 +44,12 @@ module Gitlab end end end + + private + + def protected_ref? + @protected_ref ||= project.protected_for?(pipeline.ref) + end end end end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 5bba1dec7db..25ec63de94a 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 { @@ -226,5 +227,9 @@ FactoryGirl.define do status 'created' self.when 'manual' end + + trait :protected do + protected true + end end end diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index e83a0e599a8..e5ea6b41ea3 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,10 @@ FactoryGirl.define do trait :failed do status :failed end + + trait :protected do + protected true + end end end end diff --git a/spec/factories/ci/runners.rb b/spec/factories/ci/runners.rb index 05abf60d5ce..88bb755d068 100644 --- a/spec/factories/ci/runners.rb +++ b/spec/factories/ci/runners.rb @@ -5,6 +5,7 @@ FactoryGirl.define do platform "darwin" is_shared false active true + access_level :not_protected trait :online do contacted_at Time.now @@ -21,5 +22,9 @@ FactoryGirl.define do trait :inactive do active false end + + trait :ref_protected do + access_level :ref_protected + end end end diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index 785cfeb34bd..c7f0e342809 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -43,6 +43,21 @@ feature 'Runners' do expect(page).not_to have_content(specific_runner.display_name) end + scenario 'user edits the runner to be protected' do + visit runners_path(project) + + within '.activated-specific-runners' do + first('.edit-runner > a').click + end + + expect(page.find_field('runner[access_level]')).not_to be_checked + + check 'runner_access_level' + click_button 'Save changes' + + expect(page).to have_content 'Protected Yes' + end + context 'when a runner has a tag' do background do specific_runner.update(tag_list: ['tag']) diff --git a/spec/lib/gitlab/ci/stage/seed_spec.rb b/spec/lib/gitlab/ci/stage/seed_spec.rb index d7e91a5a62c..9ecd128faca 100644 --- a/spec/lib/gitlab/ci/stage/seed_spec.rb +++ b/spec/lib/gitlab/ci/stage/seed_spec.rb @@ -27,6 +27,26 @@ describe Gitlab::Ci::Stage::Seed do expect(subject.builds) .to all(include(trigger_request: pipeline.trigger_requests.first)) end + + context 'when a ref is protected' do + before do + allow_any_instance_of(Project).to receive(:protected_for?).and_return(true) + end + + it 'returns protected builds' do + expect(subject.builds).to all(include(protected: true)) + end + end + + context 'when a ref is unprotected' do + before do + allow_any_instance_of(Project).to receive(:protected_for?).and_return(false) + end + + it 'returns unprotected builds' do + expect(subject.builds).to all(include(protected: false)) + end + end end describe '#user=' do diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index a5e03e149a7..27f2ce60084 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -224,6 +224,7 @@ Ci::Pipeline: - lock_version - auto_canceled_by_id - pipeline_schedule_id +- protected Ci::Stage: - id - name @@ -276,6 +277,7 @@ CommitStatus: - coverage_regex - auto_canceled_by_id - retried +- protected Ci::Variable: - id - project_id diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 0c35ad3c9d8..3fe3ec17d36 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -43,6 +43,32 @@ describe Ci::Build do it { is_expected.not_to include(manual_but_created) } end + describe '.ref_protected' do + subject { described_class.ref_protected } + + context 'when protected is true' do + let!(:job) { create(:ci_build, :protected) } + + it { is_expected.to include(job) } + end + + context 'when protected is false' do + let!(:job) { create(:ci_build) } + + it { is_expected.not_to include(job) } + end + + context 'when protected is nil' do + let!(:job) { create(:ci_build) } + + before do + job.update_attribute(:protected, nil) + end + + it { is_expected.not_to include(job) } + end + end + describe '#actionize' do context 'when build is a created' do before do diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 48f878bbee6..2e686e515c5 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' describe Ci::Runner do describe 'validation' do + it { is_expected.to validate_presence_of(:access_level) } + context 'when runner is not allowed to pick untagged jobs' do context 'when runner does not have tags' do it 'is not valid' do @@ -19,6 +21,34 @@ describe Ci::Runner do end end + describe '#access_level' do + context 'when creating new runner and access_level is nil' do + let(:runner) do + build(:ci_runner, access_level: nil) + end + + it "object is invalid" do + expect(runner).not_to be_valid + end + end + + context 'when creating new runner and access_level is defined in enum' do + let(:runner) do + build(:ci_runner, access_level: :not_protected) + end + + it "object is valid" do + expect(runner).to be_valid + end + end + + context 'when creating new runner and access_level is not defined in enum' do + it "raises an error" do + expect { build(:ci_runner, access_level: :this_is_not_defined) }.to raise_error(ArgumentError) + end + end + end + describe '#display_name' do it 'returns the description if it has a value' do runner = FactoryGirl.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448') @@ -95,6 +125,8 @@ describe Ci::Runner do let(:build) { create(:ci_build, pipeline: pipeline) } let(:runner) { create(:ci_runner) } + subject { runner.can_pick?(build) } + before do build.project.runners << runner end @@ -222,6 +254,50 @@ describe Ci::Runner do end end end + + context 'when access_level of runner is not_protected' do + before do + runner.not_protected! + end + + context 'when build is protected' do + before do + build.protected = true + end + + it { is_expected.to be_truthy } + end + + context 'when build is unprotected' do + before do + build.protected = false + end + + it { is_expected.to be_truthy } + end + end + + context 'when access_level of runner is ref_protected' do + before do + runner.ref_protected! + end + + context 'when build is protected' do + before do + build.protected = true + end + + it { is_expected.to be_truthy } + end + + context 'when build is unprotected' do + before do + build.protected = false + end + + it { is_expected.to be_falsey } + end + end end describe '#status' do diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index 3c02e6302b4..cc71865e1f3 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -16,8 +16,8 @@ describe API::CommitStatuses do let(:get_url) { "/projects/#{project.id}/repository/commits/#{sha}/statuses" } context 'ci commit exists' do - let!(:master) { project.pipelines.create(source: :push, sha: commit.id, ref: 'master') } - let!(:develop) { project.pipelines.create(source: :push, sha: commit.id, ref: 'develop') } + let!(:master) { project.pipelines.create(source: :push, sha: commit.id, ref: 'master', protected: false) } + let!(:develop) { project.pipelines.create(source: :push, sha: commit.id, ref: 'develop', protected: false) } context "reporter user" do let(:statuses_id) { json_response.map { |status| status['id'] } } diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index d3b48f948f6..edbfaf510c5 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -565,7 +565,7 @@ describe API::Commits do end context 'when the ref has a pipeline' do - let!(:pipeline) { project.pipelines.create(source: :push, ref: 'master', sha: commit.sha) } + let!(:pipeline) { project.pipelines.create(source: :push, ref: 'master', sha: commit.sha, protected: false) } it 'includes a "created" status' do get api(route, current_user) diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index 244895a417e..67907579225 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -191,7 +191,8 @@ describe API::Runners do active: !active, tag_list: ['ruby2.1', 'pgsql', 'mysql'], run_untagged: 'false', - locked: 'true') + locked: 'true', + access_level: 'ref_protected') shared_runner.reload expect(response).to have_http_status(200) @@ -200,6 +201,7 @@ describe API::Runners do expect(shared_runner.tag_list).to include('ruby2.1', 'pgsql', 'mysql') expect(shared_runner.run_untagged?).to be(false) expect(shared_runner.locked?).to be(true) + expect(shared_runner.ref_protected?).to be_truthy expect(shared_runner.ensure_runner_queue_value) .not_to eq(runner_queue_value) end diff --git a/spec/requests/api/v3/commits_spec.rb b/spec/requests/api/v3/commits_spec.rb index 8fb96b3c7c5..6d0ca33a6fa 100644 --- a/spec/requests/api/v3/commits_spec.rb +++ b/spec/requests/api/v3/commits_spec.rb @@ -386,7 +386,7 @@ describe API::V3::Commits do end it "returns status for CI" do - pipeline = project.pipelines.create(source: :push, ref: 'master', sha: project.repository.commit.sha) + pipeline = project.pipelines.create(source: :push, ref: 'master', sha: project.repository.commit.sha, protected: false) pipeline.update(status: 'success') get v3_api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) @@ -396,7 +396,7 @@ describe API::V3::Commits do end it "returns status for CI when pipeline is created" do - project.pipelines.create(source: :push, ref: 'master', sha: project.repository.commit.sha) + project.pipelines.create(source: :push, ref: 'master', sha: project.repository.commit.sha, protected: false) get v3_api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user) diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 4ba3dada37c..49d7c663128 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -391,12 +391,15 @@ describe Ci::CreatePipelineService do end context 'when user is master' do + let(:pipeline) { execute_service } + before do project.add_master(user) end - it 'creates a pipeline' do - expect(execute_service).to be_persisted + it 'creates a protected pipeline' do + expect(pipeline).to be_persisted + expect(pipeline).to be_protected expect(Ci::Pipeline.count).to eq(1) end end @@ -468,10 +471,11 @@ describe Ci::CreatePipelineService do let(:user) {} let(:trigger) { create(:ci_trigger, owner: nil) } let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) } + let(:pipeline) { execute_service(trigger_request: trigger_request) } - it 'creates a pipeline' do - expect(execute_service(trigger_request: trigger_request)) - .to be_persisted + it 'creates an unprotected pipeline' do + expect(pipeline).to be_persisted + expect(pipeline).not_to be_protected expect(Ci::Pipeline.count).to eq(1) end end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index 8eb0d2d10a4..5ac30111ec9 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -4,7 +4,7 @@ module Ci describe RegisterJobService do let!(:project) { FactoryGirl.create :project, shared_runners_enabled: false } let!(:pipeline) { FactoryGirl.create :ci_pipeline, project: project } - let!(:pending_build) { FactoryGirl.create :ci_build, pipeline: pipeline } + let!(:pending_job) { FactoryGirl.create :ci_build, pipeline: pipeline } let!(:shared_runner) { FactoryGirl.create(:ci_runner, is_shared: true) } let!(:specific_runner) { FactoryGirl.create(:ci_runner, is_shared: false) } @@ -15,32 +15,32 @@ module Ci describe '#execute' do context 'runner follow tag list' do it "picks build with the same tag" do - pending_build.tag_list = ["linux"] - pending_build.save + pending_job.tag_list = ["linux"] + pending_job.save specific_runner.tag_list = ["linux"] - expect(execute(specific_runner)).to eq(pending_build) + expect(execute(specific_runner)).to eq(pending_job) end it "does not pick build with different tag" do - pending_build.tag_list = ["linux"] - pending_build.save + pending_job.tag_list = ["linux"] + pending_job.save specific_runner.tag_list = ["win32"] expect(execute(specific_runner)).to be_falsey end it "picks build without tag" do - expect(execute(specific_runner)).to eq(pending_build) + expect(execute(specific_runner)).to eq(pending_job) end it "does not pick build with tag" do - pending_build.tag_list = ["linux"] - pending_build.save + pending_job.tag_list = ["linux"] + pending_job.save expect(execute(specific_runner)).to be_falsey end it "pick build without tag" do specific_runner.tag_list = ["win32"] - expect(execute(specific_runner)).to eq(pending_build) + expect(execute(specific_runner)).to eq(pending_job) end end @@ -76,7 +76,7 @@ module Ci let!(:pipeline2) { create :ci_pipeline, project: project2 } let!(:project3) { create :project, shared_runners_enabled: true } let!(:pipeline3) { create :ci_pipeline, project: project3 } - let!(:build1_project1) { pending_build } + let!(:build1_project1) { pending_job } let!(:build2_project1) { FactoryGirl.create :ci_build, pipeline: pipeline } let!(:build3_project1) { FactoryGirl.create :ci_build, pipeline: pipeline } let!(:build1_project2) { FactoryGirl.create :ci_build, pipeline: pipeline2 } @@ -172,7 +172,7 @@ module Ci context 'when first build is stalled' do before do - pending_build.lock_version = 10 + pending_job.lock_version = 10 end subject { described_class.new(specific_runner).execute } @@ -182,7 +182,7 @@ module Ci before do allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner) - .and_return([pending_build, other_build]) + .and_return([pending_job, other_build]) end it "receives second build from the queue" do @@ -194,7 +194,7 @@ module Ci context 'when single build is in queue' do before do allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner) - .and_return([pending_build]) + .and_return([pending_job]) end it "does not receive any valid result" do @@ -215,6 +215,70 @@ module Ci end end + context 'when access_level of runner is not_protected' do + let!(:specific_runner) { create(:ci_runner, :specific) } + + context 'when a job is protected' do + let!(:pending_job) { create(:ci_build, :protected, pipeline: pipeline) } + + it 'picks the job' do + expect(execute(specific_runner)).to eq(pending_job) + end + end + + context 'when a job is unprotected' do + let!(:pending_job) { create(:ci_build, pipeline: pipeline) } + + it 'picks the job' do + expect(execute(specific_runner)).to eq(pending_job) + end + end + + context 'when protected attribute of a job is nil' do + let!(:pending_job) { create(:ci_build, pipeline: pipeline) } + + before do + pending_job.update_attribute(:protected, nil) + end + + it 'picks the job' do + expect(execute(specific_runner)).to eq(pending_job) + end + end + end + + context 'when access_level of runner is ref_protected' do + let!(:specific_runner) { create(:ci_runner, :ref_protected, :specific) } + + context 'when a job is protected' do + let!(:pending_job) { create(:ci_build, :protected, pipeline: pipeline) } + + it 'picks the job' do + expect(execute(specific_runner)).to eq(pending_job) + end + end + + context 'when a job is unprotected' do + let!(:pending_job) { create(:ci_build, pipeline: pipeline) } + + it 'does not pick the job' do + expect(execute(specific_runner)).to be_nil + end + end + + context 'when protected attribute of a job is nil' do + let!(:pending_job) { create(:ci_build, pipeline: pipeline) } + + before do + pending_job.update_attribute(:protected, nil) + end + + it 'does not pick the job' do + expect(execute(specific_runner)).to be_nil + end + end + end + def execute(runner) described_class.new(runner).execute.build end diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index cec667071cc..7c9c117bf71 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -48,7 +48,7 @@ describe Ci::RetryBuildService do describe 'clone accessors' do CLONE_ACCESSORS.each do |attribute| it "clones #{attribute} build attribute" do - expect(new_build.send(attribute)).to be_present + expect(new_build.send(attribute)).not_to be_nil expect(new_build.send(attribute)).to eq build.send(attribute) end end diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb index 39586d37e93..934b4557ba2 100644 --- a/spec/support/cycle_analytics_helpers.rb +++ b/spec/support/cycle_analytics_helpers.rb @@ -80,7 +80,8 @@ module CycleAnalyticsHelpers sha: project.repository.commit('master').sha, ref: 'master', source: :push, - project: project) + project: project, + protected: false) end def new_dummy_job(environment) @@ -93,7 +94,8 @@ module CycleAnalyticsHelpers ref: 'master', tag: false, name: 'dummy', - pipeline: dummy_pipeline) + pipeline: dummy_pipeline, + protected: false) end end |
