From c4d5ac49db3d87792ef4eadd6c55b1129ccac3d4 Mon Sep 17 00:00:00 2001 From: Alishan Ladhani <aladhani@gitlab.com> Date: Fri, 19 Nov 2021 14:59:07 -0500 Subject: [PATCH] Create data model for Deployment Approvals This is the first MR working towards an MVC for the new Deployment Approvals feature. Changelog: added --- app/models/ci/build.rb | 2 +- app/models/commit_status.rb | 2 +- app/models/concerns/enums/ci/commit_status.rb | 1 + app/models/deployment.rb | 12 +- app/models/environment.rb | 2 +- app/presenters/commit_status_presenter.rb | 3 +- .../development/deployment_approvals.yml | 8 ++ ...pproval_count_to_protected_environments.rb | 7 ++ ...11119195201_create_deployment_approvals.rb | 13 ++ ...ser_foreign_key_to_deployment_approvals.rb | 15 +++ ...ent_foreign_key_to_deployment_approvals.rb | 15 +++ ...equired_approval_count_check_constraint.rb | 15 +++ db/schema_migrations/20211119194024 | 1 + db/schema_migrations/20211119195201 | 1 + db/schema_migrations/20211123181236 | 1 + db/schema_migrations/20211202041233 | 1 + db/schema_migrations/20211207165508 | 1 + db/structure.sql | 37 +++++- ee/app/models/deployments/approval.rb | 19 +++ ee/app/models/ee/deployment.rb | 16 +++ ee/app/models/ee/environment.rb | 12 ++ ee/app/models/ee/user.rb | 2 + ee/app/models/protected_environment.rb | 1 + ee/spec/factories/deployments/approval.rb | 13 ++ ee/spec/models/deployment_spec.rb | 114 ++++++++++++++++++ ee/spec/models/deployments/approval_spec.rb | 19 +++ ee/spec/models/ee/user_spec.rb | 1 + ee/spec/models/environment_spec.rb | 84 +++++++++++++ ee/spec/models/protected_environment_spec.rb | 6 + lib/gitlab/ci/status/build/failed.rb | 3 +- lib/gitlab/database/gitlab_schemas.yml | 1 + spec/models/ci/build_spec.rb | 8 ++ spec/models/commit_status_spec.rb | 8 ++ spec/models/deployment_spec.rb | 71 ++++++++++- spec/models/environment_spec.rb | 6 + .../older_deployments_drop_service_spec.rb | 9 +- 36 files changed, 514 insertions(+), 16 deletions(-) create mode 100644 config/feature_flags/development/deployment_approvals.yml create mode 100644 db/migrate/20211119194024_add_required_approval_count_to_protected_environments.rb create mode 100644 db/migrate/20211119195201_create_deployment_approvals.rb create mode 100644 db/migrate/20211123181236_add_user_foreign_key_to_deployment_approvals.rb create mode 100644 db/migrate/20211202041233_add_deployment_foreign_key_to_deployment_approvals.rb create mode 100644 db/migrate/20211207165508_add_protected_environments_required_approval_count_check_constraint.rb create mode 100644 db/schema_migrations/20211119194024 create mode 100644 db/schema_migrations/20211119195201 create mode 100644 db/schema_migrations/20211123181236 create mode 100644 db/schema_migrations/20211202041233 create mode 100644 db/schema_migrations/20211207165508 create mode 100644 ee/app/models/deployments/approval.rb create mode 100644 ee/spec/factories/deployments/approval.rb create mode 100644 ee/spec/models/deployments/approval_spec.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index af188e7a143dcdb4..92742e89496a6411 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -454,7 +454,7 @@ def cancelable? end def retryable? - return false if retried? || archived? + return false if retried? || archived? || deployment_rejected? success? || failed? || canceled? end diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index d80b2fe37dc0069a..099dea3d81e65f15 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -144,7 +144,7 @@ class CommitStatus < Ci::ApplicationRecord end event :drop do - transition [:created, :waiting_for_resource, :preparing, :pending, :running, :scheduled] => :failed + transition [:created, :waiting_for_resource, :preparing, :pending, :running, :manual, :scheduled] => :failed end event :success do diff --git a/app/models/concerns/enums/ci/commit_status.rb b/app/models/concerns/enums/ci/commit_status.rb index 1b4cc14f4a204f0b..312b88a4d6dabcef 100644 --- a/app/models/concerns/enums/ci/commit_status.rb +++ b/app/models/concerns/enums/ci/commit_status.rb @@ -28,6 +28,7 @@ def self.failure_reasons trace_size_exceeded: 19, builds_disabled: 20, environment_creation_failure: 21, + deployment_rejected: 22, insufficient_bridge_permissions: 1_001, downstream_bridge_project_not_found: 1_002, invalid_bridge_trigger: 1_003, diff --git a/app/models/deployment.rb b/app/models/deployment.rb index ade19ce02a8d1180..48641c71593068f5 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -46,9 +46,10 @@ class Deployment < ApplicationRecord scope :for_project, -> (project_id) { where(project_id: project_id) } scope :for_projects, -> (projects) { where(project: projects) } - scope :visible, -> { where(status: %i[running success failed canceled]) } + scope :visible, -> { where(status: %i[running success failed canceled blocked]) } scope :stoppable, -> { where.not(on_stop: nil).where.not(deployable_id: nil).success } scope :active, -> { where(status: %i[created running]) } + scope :upcoming, -> { where(status: %i[blocked running]) } scope :older_than, -> (deployment) { where('deployments.id < ?', deployment.id) } scope :with_api_entity_associations, -> { preload({ deployable: { runner: [], tags: [], user: [], job_artifacts_archive: [] } }) } @@ -64,6 +65,10 @@ class Deployment < ApplicationRecord transition created: :running end + event :block do + transition created: :blocked + end + event :succeed do transition any - [:success] => :success end @@ -136,7 +141,8 @@ class Deployment < ApplicationRecord success: 2, failed: 3, canceled: 4, - skipped: 5 + skipped: 5, + blocked: 6 } def self.archivables_in(project, limit:) @@ -387,6 +393,8 @@ def update_status!(status) cancel! when 'skipped' skip! + when 'blocked' + block! else raise ArgumentError, "The status #{status.inspect} is invalid" end diff --git a/app/models/environment.rb b/app/models/environment.rb index f51c9fab8dd2e632..ca1a842d1bd627ae 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -31,7 +31,7 @@ class Environment < ApplicationRecord has_one :last_visible_deployable, through: :last_visible_deployment, source: 'deployable', source_type: 'CommitStatus', disable_joins: true has_one :last_visible_pipeline, through: :last_visible_deployable, source: 'pipeline', disable_joins: true - has_one :upcoming_deployment, -> { running.distinct_on_environment }, class_name: 'Deployment', inverse_of: :environment + has_one :upcoming_deployment, -> { upcoming.distinct_on_environment }, class_name: 'Deployment', inverse_of: :environment has_one :latest_opened_most_severe_alert, -> { order_severity_with_open_prometheus_alert }, class_name: 'AlertManagement::Alert', inverse_of: :environment before_validation :generate_slug, if: ->(env) { env.slug.blank? } diff --git a/app/presenters/commit_status_presenter.rb b/app/presenters/commit_status_presenter.rb index 7919e501bf0e8708..250715d7c9c6b223 100644 --- a/app/presenters/commit_status_presenter.rb +++ b/app/presenters/commit_status_presenter.rb @@ -29,7 +29,8 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated no_matching_runner: 'No matching runner available', trace_size_exceeded: 'The job log size limit was reached', builds_disabled: 'The CI/CD is disabled for this project', - environment_creation_failure: 'This job could not be executed because it would create an environment with an invalid parameter.' + environment_creation_failure: 'This job could not be executed because it would create an environment with an invalid parameter.', + deployment_rejected: 'This deployment job was rejected.' }.freeze TROUBLESHOOTING_DOC = { diff --git a/config/feature_flags/development/deployment_approvals.yml b/config/feature_flags/development/deployment_approvals.yml new file mode 100644 index 0000000000000000..5083ccd28bfb9aea --- /dev/null +++ b/config/feature_flags/development/deployment_approvals.yml @@ -0,0 +1,8 @@ +--- +name: deployment_approvals +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74932 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/347342 +milestone: '14.6' +type: development +group: group::release +default_enabled: false diff --git a/db/migrate/20211119194024_add_required_approval_count_to_protected_environments.rb b/db/migrate/20211119194024_add_required_approval_count_to_protected_environments.rb new file mode 100644 index 0000000000000000..ca6b78efbc73e2e2 --- /dev/null +++ b/db/migrate/20211119194024_add_required_approval_count_to_protected_environments.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddRequiredApprovalCountToProtectedEnvironments < Gitlab::Database::Migration[1.0] + def change + add_column :protected_environments, :required_approval_count, :integer, default: 0, null: false + end +end diff --git a/db/migrate/20211119195201_create_deployment_approvals.rb b/db/migrate/20211119195201_create_deployment_approvals.rb new file mode 100644 index 0000000000000000..a238da302f944950 --- /dev/null +++ b/db/migrate/20211119195201_create_deployment_approvals.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class CreateDeploymentApprovals < Gitlab::Database::Migration[1.0] + def change + create_table :deployment_approvals do |t| + t.bigint :deployment_id, null: false + t.bigint :user_id, null: false, index: true + t.timestamps_with_timezone null: false + t.integer :status, limit: 2, null: false + t.index [:deployment_id, :user_id], unique: true + end + end +end diff --git a/db/migrate/20211123181236_add_user_foreign_key_to_deployment_approvals.rb b/db/migrate/20211123181236_add_user_foreign_key_to_deployment_approvals.rb new file mode 100644 index 0000000000000000..da20e9a8f8edc0ca --- /dev/null +++ b/db/migrate/20211123181236_add_user_foreign_key_to_deployment_approvals.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddUserForeignKeyToDeploymentApprovals < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :deployment_approvals, :users, column: :user_id + end + + def down + with_lock_retries do + remove_foreign_key :deployment_approvals, :users + end + end +end diff --git a/db/migrate/20211202041233_add_deployment_foreign_key_to_deployment_approvals.rb b/db/migrate/20211202041233_add_deployment_foreign_key_to_deployment_approvals.rb new file mode 100644 index 0000000000000000..60bc892d792d765e --- /dev/null +++ b/db/migrate/20211202041233_add_deployment_foreign_key_to_deployment_approvals.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddDeploymentForeignKeyToDeploymentApprovals < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :deployment_approvals, :deployments, column: :deployment_id + end + + def down + with_lock_retries do + remove_foreign_key :deployment_approvals, :deployments + end + end +end diff --git a/db/migrate/20211207165508_add_protected_environments_required_approval_count_check_constraint.rb b/db/migrate/20211207165508_add_protected_environments_required_approval_count_check_constraint.rb new file mode 100644 index 0000000000000000..fb1339cecfac73a2 --- /dev/null +++ b/db/migrate/20211207165508_add_protected_environments_required_approval_count_check_constraint.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddProtectedEnvironmentsRequiredApprovalCountCheckConstraint < Gitlab::Database::Migration[1.0] + CONSTRAINT_NAME = 'protected_environments_required_approval_count_positive' + + disable_ddl_transaction! + + def up + add_check_constraint :protected_environments, 'required_approval_count >= 0', CONSTRAINT_NAME + end + + def down + remove_check_constraint :protected_environments, CONSTRAINT_NAME + end +end diff --git a/db/schema_migrations/20211119194024 b/db/schema_migrations/20211119194024 new file mode 100644 index 0000000000000000..0d90b09e732ef5f6 --- /dev/null +++ b/db/schema_migrations/20211119194024 @@ -0,0 +1 @@ +ac2e376ad32f0e2fd45d8695f13a0b46c2d5964b881f79e3a30a51ac85d4359b \ No newline at end of file diff --git a/db/schema_migrations/20211119195201 b/db/schema_migrations/20211119195201 new file mode 100644 index 0000000000000000..dd7f7b83d8d746a9 --- /dev/null +++ b/db/schema_migrations/20211119195201 @@ -0,0 +1 @@ +caaf92f12bf0ed144d99f629c9e5d64fd45832a90bbd743e40febcdc4802cd59 \ No newline at end of file diff --git a/db/schema_migrations/20211123181236 b/db/schema_migrations/20211123181236 new file mode 100644 index 0000000000000000..25f00af5d726cc12 --- /dev/null +++ b/db/schema_migrations/20211123181236 @@ -0,0 +1 @@ +ac21109099642d5934c16b3f0130736a587c4f20143552545c2b524062ff71e0 \ No newline at end of file diff --git a/db/schema_migrations/20211202041233 b/db/schema_migrations/20211202041233 new file mode 100644 index 0000000000000000..fb19264fbd584e85 --- /dev/null +++ b/db/schema_migrations/20211202041233 @@ -0,0 +1 @@ +61c949b42338b248a0950cfafc82d58816c3fec44a2bf41c4ecb4cf09340a424 \ No newline at end of file diff --git a/db/schema_migrations/20211207165508 b/db/schema_migrations/20211207165508 new file mode 100644 index 0000000000000000..df0c91bad7da9110 --- /dev/null +++ b/db/schema_migrations/20211207165508 @@ -0,0 +1 @@ +d1ed3ddf51c0bcebbac2a8dee05aa168daa35129110a463ac296ff2e640b0dbd \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 3c3caac5cce6daf7..f089d3c33730fc46 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13409,6 +13409,24 @@ CREATE SEQUENCE deploy_tokens_id_seq ALTER SEQUENCE deploy_tokens_id_seq OWNED BY deploy_tokens.id; +CREATE TABLE deployment_approvals ( + id bigint NOT NULL, + deployment_id bigint NOT NULL, + user_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + status smallint NOT NULL +); + +CREATE SEQUENCE deployment_approvals_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE deployment_approvals_id_seq OWNED BY deployment_approvals.id; + CREATE TABLE deployment_clusters ( deployment_id integer NOT NULL, cluster_id integer NOT NULL, @@ -18695,7 +18713,9 @@ CREATE TABLE protected_environments ( updated_at timestamp with time zone NOT NULL, name character varying NOT NULL, group_id bigint, - CONSTRAINT protected_environments_project_or_group_existence CHECK (((project_id IS NULL) <> (group_id IS NULL))) + required_approval_count integer DEFAULT 0 NOT NULL, + CONSTRAINT protected_environments_project_or_group_existence CHECK (((project_id IS NULL) <> (group_id IS NULL))), + CONSTRAINT protected_environments_required_approval_count_positive CHECK ((required_approval_count >= 0)) ); CREATE SEQUENCE protected_environments_id_seq @@ -21495,6 +21515,8 @@ ALTER TABLE ONLY deploy_keys_projects ALTER COLUMN id SET DEFAULT nextval('deplo ALTER TABLE ONLY deploy_tokens ALTER COLUMN id SET DEFAULT nextval('deploy_tokens_id_seq'::regclass); +ALTER TABLE ONLY deployment_approvals ALTER COLUMN id SET DEFAULT nextval('deployment_approvals_id_seq'::regclass); + ALTER TABLE ONLY deployments ALTER COLUMN id SET DEFAULT nextval('deployments_id_seq'::regclass); ALTER TABLE ONLY description_versions ALTER COLUMN id SET DEFAULT nextval('description_versions_id_seq'::regclass); @@ -23062,6 +23084,9 @@ ALTER TABLE ONLY deploy_keys_projects ALTER TABLE ONLY deploy_tokens ADD CONSTRAINT deploy_tokens_pkey PRIMARY KEY (id); +ALTER TABLE ONLY deployment_approvals + ADD CONSTRAINT deployment_approvals_pkey PRIMARY KEY (id); + ALTER TABLE ONLY deployment_clusters ADD CONSTRAINT deployment_clusters_pkey PRIMARY KEY (deployment_id); @@ -25804,6 +25829,10 @@ CREATE INDEX index_deploy_tokens_on_token_and_expires_at_and_id ON deploy_tokens CREATE UNIQUE INDEX index_deploy_tokens_on_token_encrypted ON deploy_tokens USING btree (token_encrypted); +CREATE UNIQUE INDEX index_deployment_approvals_on_deployment_id_and_user_id ON deployment_approvals USING btree (deployment_id, user_id); + +CREATE INDEX index_deployment_approvals_on_user_id ON deployment_approvals USING btree (user_id); + CREATE UNIQUE INDEX index_deployment_clusters_on_cluster_id_and_deployment_id ON deployment_clusters USING btree (cluster_id, deployment_id); CREATE INDEX index_deployment_merge_requests_on_merge_request_id ON deployment_merge_requests USING btree (merge_request_id); @@ -28935,6 +28964,9 @@ ALTER TABLE ONLY lists ALTER TABLE ONLY ci_unit_test_failures ADD CONSTRAINT fk_0f09856e1f FOREIGN KEY (build_id) REFERENCES ci_builds(id) ON DELETE CASCADE; +ALTER TABLE ONLY deployment_approvals + ADD CONSTRAINT fk_0f58311058 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; + ALTER TABLE ONLY project_pages_metadata ADD CONSTRAINT fk_0fd5b22688 FOREIGN KEY (pages_deployment_id) REFERENCES pages_deployments(id) ON DELETE SET NULL; @@ -29043,6 +29075,9 @@ ALTER TABLE ONLY coverage_fuzzing_corpuses ALTER TABLE ONLY agent_group_authorizations ADD CONSTRAINT fk_2c9f941965 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; +ALTER TABLE ONLY deployment_approvals + ADD CONSTRAINT fk_2d060dfc73 FOREIGN KEY (deployment_id) REFERENCES deployments(id) ON DELETE CASCADE; + ALTER TABLE ONLY ci_freeze_periods ADD CONSTRAINT fk_2e02bbd1a6 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/ee/app/models/deployments/approval.rb b/ee/app/models/deployments/approval.rb new file mode 100644 index 0000000000000000..3fc414cc5a67c58b --- /dev/null +++ b/ee/app/models/deployments/approval.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Deployments + class Approval < ApplicationRecord + self.table_name = 'deployment_approvals' + + belongs_to :deployment + belongs_to :user + + validates :user, presence: true, uniqueness: { scope: :deployment_id } + validates :deployment, presence: true + validates :status, presence: true + + enum status: { + approved: 0, + rejected: 1 + } + end +end diff --git a/ee/app/models/ee/deployment.rb b/ee/app/models/ee/deployment.rb index a67da55fc5474265..ba55e02f573a13c7 100644 --- a/ee/app/models/ee/deployment.rb +++ b/ee/app/models/ee/deployment.rb @@ -7,10 +7,15 @@ module EE # and be prepended in the `Deployment` model module Deployment extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override prepended do include UsageStatistics + delegate :needs_approval?, to: :environment + + has_many :approvals, class_name: 'Deployments::Approval' + state_machine :status do after_transition any => :success do |deployment| deployment.run_after_commit do @@ -25,5 +30,16 @@ module Deployment end end end + + override :sync_status_with + def sync_status_with(build) + return update_status!('blocked') if build.status == 'manual' && needs_approval? + + super + end + + def pending_approval_count + environment.required_approval_count - approvals.approved.count + end end end diff --git a/ee/app/models/ee/environment.rb b/ee/app/models/ee/environment.rb index 7b21f65c722f860b..e51d2426a073c2b2 100644 --- a/ee/app/models/ee/environment.rb +++ b/ee/app/models/ee/environment.rb @@ -79,6 +79,18 @@ def protected_by?(user) protected_environment_accesses(user).all? { |access, _| access == true } end + def needs_approval? + return false unless ::Feature.enabled?(:deployment_approvals, project, default_enabled: :yaml) + + required_approval_count > 0 + end + + def required_approval_count + return 0 unless protected? + + associated_protected_environments.map(&:required_approval_count).max + end + private def protected_environment_accesses(user) diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index ca8c26be3ecaf102..8bff861ba689331a 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -65,6 +65,8 @@ module User has_many :protected_branch_push_access_levels, dependent: :destroy, class_name: "::ProtectedBranch::PushAccessLevel" # rubocop:disable Cop/ActiveRecordDependent has_many :protected_branch_unprotect_access_levels, dependent: :destroy, class_name: "::ProtectedBranch::UnprotectAccessLevel" # rubocop:disable Cop/ActiveRecordDependent + has_many :deployment_approvals, class_name: 'Deployments::Approval' + has_many :smartcard_identities has_many :scim_identities diff --git a/ee/app/models/protected_environment.rb b/ee/app/models/protected_environment.rb index 335f95210c6945ce..ee1686cffc6f2fb2 100644 --- a/ee/app/models/protected_environment.rb +++ b/ee/app/models/protected_environment.rb @@ -12,6 +12,7 @@ class ProtectedEnvironment < ApplicationRecord validates :deploy_access_levels, length: { minimum: 1 } validates :name, presence: true validate :valid_tier_name, if: :group_level? + validates :required_approval_count, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 5 } scope :sorted_by_name, -> { order(:name) } diff --git a/ee/spec/factories/deployments/approval.rb b/ee/spec/factories/deployments/approval.rb new file mode 100644 index 0000000000000000..64be3cafefcadcf0 --- /dev/null +++ b/ee/spec/factories/deployments/approval.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :deployment_approval, class: 'Deployments::Approval' do + user + deployment + status { :approved } + + trait :rejected do + status { :rejected } + end + end +end diff --git a/ee/spec/models/deployment_spec.rb b/ee/spec/models/deployment_spec.rb index 6172ae7adcc00e57..0be6b756d7355de1 100644 --- a/ee/spec/models/deployment_spec.rb +++ b/ee/spec/models/deployment_spec.rb @@ -3,6 +3,9 @@ require 'spec_helper' RSpec.describe Deployment do + it { is_expected.to have_many(:approvals) } + it { is_expected.to delegate_method(:needs_approval?).to(:environment) } + describe 'state machine' do context 'when deployment succeeded' do let(:deployment) { create(:deployment, :running) } @@ -20,4 +23,115 @@ end end end + + describe '#sync_status_with' do + subject { deployment.sync_status_with(ci_build) } + + let_it_be(:project) { create(:project, :repository) } + + let(:environment) { create(:environment, project: project) } + let(:deployment) { create(:deployment, project: project, environment: environment) } + + context 'when build is manual' do + let(:ci_build) { create(:ci_build, project: project, status: :manual) } + + context 'and Protected Environments feature is available' do + before do + stub_licensed_features(protected_environments: true) + create(:protected_environment, name: environment.name, project: project, required_approval_count: required_approval_count) + end + + context 'and deployment needs approval' do + let(:required_approval_count) { 1 } + + it 'blocks the deployment' do + expect(Gitlab::ErrorTracking).not_to receive(:track_exception) + + is_expected.to eq(true) + + expect(deployment.status).to eq('blocked') + expect(deployment.errors).to be_empty + end + end + + context 'and deployment does not need approval' do + let(:required_approval_count) { 0 } + + it 'does not change the deployment' do + expect(Gitlab::ErrorTracking).not_to receive(:track_exception) + + is_expected.to eq(false) + + expect(deployment.status).to eq('created') + expect(deployment.errors).to be_empty + end + end + end + + context 'and Protected Environments feature is not available' do + before do + stub_licensed_features(protected_environments: false) + end + + it 'does not change the deployment' do + expect(Gitlab::ErrorTracking).not_to receive(:track_exception) + + is_expected.to eq(false) + + expect(deployment.status).to eq('created') + expect(deployment.errors).to be_empty + end + end + end + end + + describe '#pending_approval_count' do + let_it_be(:project) { create(:project, :repository) } + + let(:environment) { create(:environment, project: project) } + let(:deployment) { create(:deployment, project: project, environment: environment) } + + context 'when Protected Environments feature is available' do + before do + stub_licensed_features(protected_environments: true) + create(:protected_environment, name: environment.name, project: project, required_approval_count: 3) + end + + context 'with no approvals' do + it 'returns the number of approvals required by the environment' do + expect(deployment.pending_approval_count).to eq(3) + end + end + + context 'with some approvals' do + before do + create(:deployment_approval, deployment: deployment) + end + + it 'returns the number of pending approvals' do + expect(deployment.pending_approval_count).to eq(2) + end + end + + context 'with all approvals satisfied' do + before do + create_list(:deployment_approval, 3, deployment: deployment) + end + + it 'returns zero' do + expect(deployment.pending_approval_count).to eq(0) + end + end + end + + context 'when Protected Environments feature is not available' do + before do + stub_licensed_features(protected_environments: false) + end + + it 'returns zero' do + expect(deployment.pending_approval_count).to eq(0) + end + end + end end diff --git a/ee/spec/models/deployments/approval_spec.rb b/ee/spec/models/deployments/approval_spec.rb new file mode 100644 index 0000000000000000..a6c97a071aa68c0d --- /dev/null +++ b/ee/spec/models/deployments/approval_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Deployments::Approval do + describe 'associations' do + it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:deployment) } + end + + describe 'validations' do + subject { create(:deployment_approval) } + + it { is_expected.to validate_presence_of(:user) } + it { is_expected.to validate_uniqueness_of(:user).scoped_to(:deployment_id) } + it { is_expected.to validate_presence_of(:deployment) } + it { is_expected.to validate_presence_of(:status) } + end +end diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index a4e6859fb798953f..a76166e50d09bad6 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -34,6 +34,7 @@ it { is_expected.to have_many(:escalation_rules).class_name('IncidentManagement::EscalationRule') } it { is_expected.to have_many(:escalation_policies).class_name('IncidentManagement::EscalationPolicy').through(:escalation_rules) } it { is_expected.to have_many(:epic_board_recent_visits).inverse_of(:user) } + it { is_expected.to have_many(:deployment_approvals) } end describe 'nested attributes' do diff --git a/ee/spec/models/environment_spec.rb b/ee/spec/models/environment_spec.rb index 80fcd3fe34372830..8d119c0fb0b8225e 100644 --- a/ee/spec/models/environment_spec.rb +++ b/ee/spec/models/environment_spec.rb @@ -245,4 +245,88 @@ end end end + + describe '#needs_approval?' do + subject { environment.needs_approval? } + + context 'when Protected Environments feature is available' do + before do + stub_licensed_features(protected_environments: true) + create(:protected_environment, name: environment.name, project: project, required_approval_count: required_approval_count) + end + + context 'with some approvals required' do + let(:required_approval_count) { 1 } + + it { is_expected.to be_truthy } + + context 'and deployment_approvals feature flag turned off' do + before do + stub_feature_flags(deployment_approvals: false) + end + + it { is_expected.to be_falsey } + end + end + + context 'with no approvals required' do + let(:required_approval_count) { 0 } + + it { is_expected.to be_falsey } + end + end + + context 'when Protected Environments feature is not available' do + before do + stub_licensed_features(protected_environments: false) + end + + it { is_expected.to be_falsey } + end + end + + describe '#required_approval_count' do + subject { environment.required_approval_count } + + let_it_be(:project) { create(:project, group: create(:group)) } + + context 'when Protected Environments feature is not available' do + before do + stub_licensed_features(protected_environments: false) + end + + it { is_expected.to eq(0) } + end + + context 'when Protected Environments feature is available' do + before do + stub_licensed_features(protected_environments: true) + end + + context 'and no associated protected environments exist' do + it { is_expected.to eq(0) } + end + + context 'with one associated protected environment' do + before do + create(:protected_environment, name: environment.name, project: project, required_approval_count: 3) + end + + it 'returns the required_approval_count of the protected environment' do + expect(subject).to eq(3) + end + end + + context 'with multiple associated protected environments' do + before do + create(:protected_environment, name: environment.name, project: project, required_approval_count: 3) + create(:protected_environment, name: environment.tier, project: nil, group: project.group, required_approval_count: 5) + end + + it 'returns the highest required_approval_count of the protected environments' do + expect(subject).to eq(5) + end + end + end + end end diff --git a/ee/spec/models/protected_environment_spec.rb b/ee/spec/models/protected_environment_spec.rb index 788f64fd287ca4b2..519a1cf194fc29a2 100644 --- a/ee/spec/models/protected_environment_spec.rb +++ b/ee/spec/models/protected_environment_spec.rb @@ -10,6 +10,12 @@ describe 'validation' do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_length_of(:deploy_access_levels) } + it do + is_expected.to validate_numericality_of(:required_approval_count) + .only_integer + .is_greater_than_or_equal_to(0) + .is_less_than_or_equal_to(5) + end it 'can not belong to both group and project' do group = build(:group) diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb index b0f12ff75175ffe3..5dd28157b1f73ba9 100644 --- a/lib/gitlab/ci/status/build/failed.rb +++ b/lib/gitlab/ci/status/build/failed.rb @@ -34,7 +34,8 @@ class Failed < Status::Extended no_matching_runner: 'no matching runner available', trace_size_exceeded: 'log size limit exceeded', builds_disabled: 'project builds are disabled', - environment_creation_failure: 'environment creation failure' + environment_creation_failure: 'environment creation failure', + deployment_rejected: 'deployment rejected' }.freeze private_constant :REASONS diff --git a/lib/gitlab/database/gitlab_schemas.yml b/lib/gitlab/database/gitlab_schemas.yml index ee4e5dc0b1c20c6d..057109a24b781905 100644 --- a/lib/gitlab/database/gitlab_schemas.yml +++ b/lib/gitlab/database/gitlab_schemas.yml @@ -164,6 +164,7 @@ dependency_proxy_group_settings: :gitlab_main dependency_proxy_image_ttl_group_policies: :gitlab_main dependency_proxy_manifests: :gitlab_main deploy_keys_projects: :gitlab_main +deployment_approvals: :gitlab_main deployment_clusters: :gitlab_main deployment_merge_requests: :gitlab_main deployments: :gitlab_main diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 77bc6804125173ef..6f5bb75314e190f1 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1994,6 +1994,14 @@ it { is_expected.not_to be_retryable } end + + context 'when deployment is rejected' do + before do + build.drop!(:deployment_rejected) + end + + it { is_expected.not_to be_retryable } + end end end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index d675b0e7221d96ba..665a2a936af26f10 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -765,6 +765,14 @@ def create_status(**opts) it_behaves_like 'incrementing failure reason counter' end + + context 'when status is manual' do + let(:commit_status) { create(:commit_status, :manual) } + + it 'is able to be dropped' do + expect { commit_status.drop! }.to change { commit_status.status }.from('manual').to('failed') + end + end end describe 'ensure stage assignment' do diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 51e1e63da8de2bc7..9185207b910de58e 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -268,6 +268,29 @@ end end + context 'when deployment is blocked' do + let(:deployment) { create(:deployment, :created) } + + it 'has correct status' do + deployment.block! + + expect(deployment).to be_blocked + expect(deployment.finished_at).to be_nil + end + + it 'does not execute Deployments::LinkMergeRequestWorker asynchronously' do + expect(Deployments::LinkMergeRequestWorker).not_to receive(:perform_async) + + deployment.block! + end + + it 'does not execute Deployments::HooksWorker' do + expect(Deployments::HooksWorker).not_to receive(:perform_async) + + deployment.block! + end + end + describe 'synching status to Jira' do let(:deployment) { create(:deployment) } @@ -448,11 +471,12 @@ subject { described_class.active } it 'retrieves the active deployments' do - deployment1 = create(:deployment, status: :created ) - deployment2 = create(:deployment, status: :running ) - create(:deployment, status: :failed ) - create(:deployment, status: :canceled ) + deployment1 = create(:deployment, status: :created) + deployment2 = create(:deployment, status: :running) + create(:deployment, status: :failed) + create(:deployment, status: :canceled) create(:deployment, status: :skipped) + create(:deployment, status: :blocked) is_expected.to contain_exactly(deployment1, deployment2) end @@ -512,9 +536,25 @@ deployment2 = create(:deployment, status: :success) deployment3 = create(:deployment, status: :failed) deployment4 = create(:deployment, status: :canceled) + deployment5 = create(:deployment, status: :blocked) create(:deployment, status: :skipped) - is_expected.to contain_exactly(deployment1, deployment2, deployment3, deployment4) + is_expected.to contain_exactly(deployment1, deployment2, deployment3, deployment4, deployment5) + end + end + + describe 'upcoming' do + subject { described_class.upcoming } + + it 'retrieves the upcoming deployments' do + deployment1 = create(:deployment, status: :running) + deployment2 = create(:deployment, status: :blocked) + create(:deployment, status: :success) + create(:deployment, status: :failed) + create(:deployment, status: :canceled) + create(:deployment, status: :skipped) + + is_expected.to contain_exactly(deployment1, deployment2) end end end @@ -840,6 +880,27 @@ expect(deploy.update_status('created')).to eq(false) end + + context 'mapping status to event' do + using RSpec::Parameterized::TableSyntax + + where(:status, :method) do + 'running' | :run! + 'success' | :succeed! + 'failed' | :drop! + 'canceled' | :cancel! + 'skipped' | :skip! + 'blocked' | :block! + end + + with_them do + it 'calls the correct method for the given status' do + expect(deploy).to receive(method) + + deploy.update_status(status) + end + end + end end describe '#sync_status_with' do diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 9d9862aa3d38f85d..3dd0e01d7b33d7d3 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -947,6 +947,12 @@ it { is_expected.to eq(deployment) } end + + context 'when environment has a blocked deployment' do + let!(:deployment) { create(:deployment, :blocked, environment: environment, project: project) } + + it { is_expected.to eq(deployment) } + end end describe '#has_terminals?' do diff --git a/spec/services/deployments/older_deployments_drop_service_spec.rb b/spec/services/deployments/older_deployments_drop_service_spec.rb index e6fd6725d7d23f94..15bd894e7bf1fe0f 100644 --- a/spec/services/deployments/older_deployments_drop_service_spec.rb +++ b/spec/services/deployments/older_deployments_drop_service_spec.rb @@ -70,10 +70,13 @@ let(:older_deployment) { create(:deployment, :created, environment: environment, deployable: build) } let(:build) { create(:ci_build, :manual) } - it 'does not drop any builds nor track the exception' do - expect(Gitlab::ErrorTracking).not_to receive(:track_exception) + it 'drops the older deployment' do + deployable = older_deployment.deployable + expect(deployable.failed?).to be_falsey - expect { subject }.not_to change { Ci::Build.failed.count } + subject + + expect(deployable.reload.failed?).to be_truthy end end -- GitLab