Merge branch 'dep-approval-multi-access-levels-authorization' into 'master'

Implement the status calculation for Approval Rules

See merge request gitlab-org/gitlab!84248

ee/app/models/ee/deployment.rb

@@ -36,5 +36,15 @@ module EE
 
       environment.required_approval_count - approvals.length
     end
+
+    def approval_summary
+      strong_memoize(:approval_summary) do
+        ::ProtectedEnvironments::ApprovalSummary.new(deployment: self)
+      end
+    end
+
+    def approved?
+      approval_summary.all_rules_approved?
+    end
   end
 end

ee/app/models/ee/environment.rb

@@ -80,7 +80,7 @@ module EE
     end
 
     def needs_approval?
-      required_approval_count > 0
+      has_approval_rules? || required_approval_count > 0
     end
 
     def required_approval_count
@@ -103,6 +103,13 @@ module EE
       end
     end
 
+    def associated_approval_rules
+      strong_memoize(:associated_approval_rules) do
+        ::ProtectedEnvironments::ApprovalRule
+          .where(protected_environment: associated_protected_environments)
+      end
+    end
+
     private
 
     def protected_environment_accesses(user)
@@ -120,12 +127,5 @@ module EE
         ::ProtectedEnvironment.for_environment(self)
       end
     end
-
-    def associated_approval_rules
-      strong_memoize(:associated_approval_rules) do
-        ::ProtectedEnvironments::ApprovalRule
-          .where(protected_environment: associated_protected_environments)
-      end
-    end
   end
 end

ee/app/models/protected_environments/approval_summary.rb

+# frozen_string_literal: true
+
+module ProtectedEnvironments
+  class ApprovalSummary
+    include ActiveModel::Model
+    include ::Gitlab::Utils::StrongMemoize
+
+    attr_accessor :deployment
+
+    delegate :environment, :approvals, to: :deployment
+    delegate :associated_approval_rules, to: :environment
+
+    def all_rules_approved?
+      rules.all? do |rule|
+        rule.required_approvals <= rule.deployment_approvals.count(&:approved?)
+      end
+    end
+
+    def rules
+      strong_memoize(:rules) do
+        approvals_by_rule_id = approvals.group_by(&:approval_rule_id)
+
+        associated_approval_rules.each do |rule|
+          rule.association(:deployment_approvals).target =
+            approvals_by_rule_id[rule.id] || Deployments::Approval.none
+
+          rule.association(:deployment_approvals).loaded!
+        end
+      end
+    end
+  end
+end

ee/app/services/deployments/approval_service.rb

@@ -43,6 +43,11 @@ module Deployments
 
       if approval.rejected?
         deployment.deployable.drop!(:deployment_rejected)
+      elsif environment.has_approval_rules?
+        # Approvers might not have sufficient permission to execute the deployment job,
+        # so we just unblock the deployment, which stays as manual job.
+        # Executors can later run the manual job at their ideal timing.
+        deployment.unblock! if deployment.approved?
       elsif deployment.pending_approval_count <= 0
         deployment.unblock!
         deployment.deployable.enqueue!

ee/spec/models/environment_spec.rb

@@ -253,19 +253,40 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
     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 }
+      context 'with unified access level' do
+        before do
+          create(:protected_environment, name: environment.name, project: project, required_approval_count: required_approval_count)
+        end
 
-        it { is_expected.to be_truthy }
+        context 'with some approvals required' do
+          let(:required_approval_count) { 1 }
+
+          it { is_expected.to be_truthy }
+        end
+
+        context 'with no approvals required' do
+          let(:required_approval_count) { 0 }
+
+          it { is_expected.to be_falsey }
+        end
       end
 
-      context 'with no approvals required' do
-        let(:required_approval_count) { 0 }
+      context 'with multi access levels' do
+        let!(:protected_environment) { create(:protected_environment, name: environment.name, project: project) }
+
+        context 'with some approvals required' do
+          let!(:approval_rule) do
+            create(:protected_environment_approval_rule, :maintainer_access, protected_environment: protected_environment)
+          end
 
-        it { is_expected.to be_falsey }
+          it { is_expected.to be_truthy }
+        end
+
+        context 'with no approvals required' do
+          it { is_expected.to be_falsey }
+        end
       end
     end
 

ee/spec/models/protected_environments/approval_summary_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe ProtectedEnvironments::ApprovalSummary do
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:group_user_1) { create(:user) }
+  let_it_be(:group_user_2) { create(:user) }
+  let_it_be(:approver_user) { create(:user) }
+  let_it_be(:project_maintainer) { create(:user) }
+  let_it_be_with_refind(:environment) { create(:environment, project: project) }
+  let_it_be_with_refind(:deployment) { create(:deployment, project: project, environment: environment) }
+
+  let_it_be_with_refind(:protected_environment) do
+    create(:protected_environment, name: environment.name, project: project)
+  end
+
+  let_it_be_with_refind(:approval_rule_group) do
+    create(:protected_environment_approval_rule, group: group, protected_environment: protected_environment,
+           required_approvals: 2)
+  end
+
+  let_it_be_with_refind(:approval_rule_user) do
+    create(:protected_environment_approval_rule, user: approver_user, protected_environment: protected_environment)
+  end
+
+  let_it_be_with_refind(:approval_rule_role) do
+    create(:protected_environment_approval_rule, :maintainer_access, protected_environment: protected_environment)
+  end
+
+  let(:approval_summary) { described_class.new(deployment: deployment) }
+
+  before_all do
+    group.add_developer(group_user_1)
+    group.add_developer(group_user_2)
+    project.add_maintainer(project_maintainer)
+  end
+
+  shared_context 'all rules have been approved' do
+    before do
+      create(:deployment_approval, deployment: deployment, user: group_user_1, approval_rule: approval_rule_group)
+      create(:deployment_approval, deployment: deployment, user: group_user_2, approval_rule: approval_rule_group)
+      create(:deployment_approval, deployment: deployment, user: approver_user, approval_rule: approval_rule_user)
+      create(:deployment_approval, deployment: deployment, user: project_maintainer, approval_rule: approval_rule_role)
+    end
+  end
+
+  shared_context 'one rule has multiple approvals' do
+    before do
+      create(:deployment_approval, deployment: deployment, user: group_user_1, approval_rule: approval_rule_group)
+      create(:deployment_approval, deployment: deployment, user: group_user_2, approval_rule: approval_rule_group)
+    end
+  end
+
+  shared_context 'one rule has been approved' do
+    before do
+      create(:deployment_approval, deployment: deployment, user: approver_user, approval_rule: approval_rule_user)
+    end
+  end
+
+  shared_context 'unrelated deployment approvals exist' do
+    before do
+      create(:deployment_approval, user: group_user_1, approval_rule: approval_rule_group)
+      create(:deployment_approval, user: group_user_2, approval_rule: approval_rule_group)
+      create(:deployment_approval, user: approver_user, approval_rule: approval_rule_user)
+      create(:deployment_approval, user: project_maintainer, approval_rule: approval_rule_role)
+    end
+  end
+
+  describe '#all_rules_approved?' do
+    subject { approval_summary.all_rules_approved? }
+
+    context 'when all rules have been approved' do
+      include_context 'all rules have been approved'
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'when one rule has multiple approvals' do
+      include_context 'one rule has multiple approvals'
+
+      it { is_expected.to eq(false) }
+    end
+
+    context 'when one rule has been approved' do
+      include_context 'one rule has been approved'
+
+      it { is_expected.to eq(false) }
+    end
+
+    context 'when no rules have been approved' do
+      it { is_expected.to eq(false) }
+    end
+
+    context 'when unrelated deployment approvals exist' do
+      include_context 'unrelated deployment approvals exist'
+
+      it { is_expected.to eq(false) }
+    end
+  end
+
+  describe '#rules' do
+    subject { approval_summary.rules }
+
+    let(:group_type_rule) { subject.find(&:group_type?) }
+    let(:user_type_rule) { subject.find(&:user_type?) }
+    let(:role_type_rule) { subject.find(&:role?) }
+
+    shared_examples 'contains the required approval counts per type' do
+      it do
+        expect(group_type_rule.required_approvals).to eq(2)
+        expect(user_type_rule.required_approvals).to eq(1)
+        expect(role_type_rule.required_approvals).to eq(1)
+      end
+    end
+
+    context 'when all rules have been approved' do
+      include_context 'all rules have been approved'
+      it_behaves_like 'contains the required approval counts per type'
+
+      it 'correctly renders the approval summary' do
+        expect(group_type_rule.deployment_approvals.map(&:user)).to contain_exactly(group_user_1, group_user_2)
+        expect(user_type_rule.deployment_approvals.map(&:user)).to contain_exactly(approver_user)
+        expect(role_type_rule.deployment_approvals.map(&:user)).to contain_exactly(project_maintainer)
+      end
+    end
+
+    context 'when one rule has multiple approvals' do
+      include_context 'one rule has multiple approvals'
+      it_behaves_like 'contains the required approval counts per type'
+
+      it 'correctly renders the approval summary' do
+        expect(group_type_rule.deployment_approvals.map(&:user)).to contain_exactly(group_user_1, group_user_2)
+        expect(role_type_rule.deployment_approvals.map(&:user)).to be_empty
+        expect(role_type_rule.deployment_approvals.map(&:user)).to be_empty
+      end
+    end
+
+    context 'when one rule has been approved' do
+      include_context 'one rule has been approved'
+      it_behaves_like 'contains the required approval counts per type'
+
+      it 'correctly renders the approval summary' do
+        expect(group_type_rule.deployment_approvals.map(&:user)).to be_empty
+        expect(user_type_rule.deployment_approvals.map(&:user)).to contain_exactly(approver_user)
+        expect(role_type_rule.deployment_approvals.map(&:user)).to be_empty
+      end
+    end
+
+    context 'when no rules have been approved' do
+      it_behaves_like 'contains the required approval counts per type'
+
+      it 'correctly renders the approval summary' do
+        expect(group_type_rule.deployment_approvals.map(&:user)).to be_empty
+        expect(user_type_rule.deployment_approvals.map(&:user)).to be_empty
+        expect(role_type_rule.deployment_approvals.map(&:user)).to be_empty
+      end
+    end
+
+    context 'when unrelated deployment approvals exist' do
+      include_context 'unrelated deployment approvals exist'
+      it_behaves_like 'contains the required approval counts per type'
+
+      it 'correctly renders the approval summary' do
+        expect(group_type_rule.deployment_approvals.map(&:user)).to be_empty
+        expect(user_type_rule.deployment_approvals.map(&:user)).to be_empty
+        expect(role_type_rule.deployment_approvals.map(&:user)).to be_empty
+      end
+    end
+  end
+end

ee/spec/services/ci/process_build_service_spec.rb

@@ -61,11 +61,7 @@ RSpec.describe Ci::ProcessBuildService, '#execute' do
           expect(ci_build.pending?).to be_truthy
         end
 
-        context 'and environment needs approval' do
-          before do
-            protected_environment.update!(required_approval_count: 1)
-          end
-
+        shared_examples_for 'blocking deployment job' do
           it 'makes the build a manual action' do
             expect { subject }.to change { ci_build.status }.from('created').to('manual')
           end
@@ -98,6 +94,22 @@ RSpec.describe Ci::ProcessBuildService, '#execute' do
             end
           end
         end
+
+        context 'with unified access level' do
+          before do
+            protected_environment.update!(required_approval_count: 1)
+          end
+
+          it_behaves_like 'blocking deployment job'
+        end
+
+        context 'with multi access levels' do
+          let!(:approval_rule) do
+            create(:protected_environment_approval_rule, :maintainer_access, protected_environment: protected_environment)
+          end
+
+          it_behaves_like 'blocking deployment job'
+        end
       end
     end
   end

ee/spec/services/deployments/approval_service_spec.rb

@@ -11,10 +11,18 @@ RSpec.describe Deployments::ApprovalService do
   let(:environment) { create(:environment, project: project) }
   let(:status) { 'approved' }
   let(:comment) { nil }
+  let(:ci_build) { create(:ci_build, :manual, project: project) }
+  let(:deployment) { create(:deployment, :blocked, project: project, environment: environment, deployable: ci_build) }
+  let!(:protected_environment) { create(:protected_environment, :maintainers_can_deploy, name: environment.name, project: project, **access_level_setting) }
+  let(:access_level_setting) { unified_access_level }
+
+  # Unified Access Level setting (MVC version)
+  let(:unified_access_level) { { required_approval_count: required_approval_count } }
   let(:required_approval_count) { 2 }
-  let(:build) { create(:ci_build, :manual, project: project) }
-  let(:deployment) { create(:deployment, :blocked, project: project, environment: environment, deployable: build) }
-  let!(:protected_environment) { create(:protected_environment, :maintainers_can_deploy, name: environment.name, project: project, required_approval_count: required_approval_count) }
+
+  # Multi Access Level setting (extended MVC)
+  let(:multi_access_level) { { approval_rules: approval_rules } }
+  let(:approval_rules) { [build(:protected_environment_approval_rule, :maintainer_access)] }
 
   before do
     stub_licensed_features(protected_environments: true)
@@ -63,7 +71,8 @@ RSpec.describe Deployments::ApprovalService do
 
   shared_examples_for 'set approval rule' do
     context 'with approval rule' do
-      let!(:approval_rule) { create(:protected_environment_approval_rule, :maintainer_access, protected_environment: protected_environment) }
+      let(:access_level_setting) { multi_access_level }
+      let(:approval_rule) { approval_rules.first.reload }
 
       it 'sets an rule to the deployment approval' do
         expect(subject[:status]).to eq(:success)
@@ -136,7 +145,7 @@ RSpec.describe Deployments::ApprovalService do
       end
     end
 
-    context 'processing the build' do
+    context 'processing the build with unified access level' do
       context 'when build is nil' do
         before do
           deployment.deployable = nil
@@ -179,6 +188,53 @@ RSpec.describe Deployments::ApprovalService do
       end
     end
 
+    context 'processing the build with multi access levels' do
+      context 'when build is nil' do
+        before do
+          deployment.deployable = nil
+        end
+
+        it 'does not raise an error' do
+          expect { subject }.not_to raise_error
+        end
+      end
+
+      context 'when deployment was rejected' do
+        let(:status) { 'rejected' }
+
+        it 'drops the build' do
+          subject
+
+          expect(deployment.deployable.status).to eq('failed')
+          expect(deployment.deployable.failure_reason).to eq('deployment_rejected')
+        end
+      end
+
+      context 'when no additional approvals are required' do
+        let(:access_level_setting) { multi_access_level }
+        let(:approval_rules) { [build(:protected_environment_approval_rule, :maintainer_access, required_approvals: 1)] }
+
+        it 'keeps the build manual' do
+          expect { subject }.not_to change { deployment.deployable.status }
+
+          expect(deployment.deployable).to be_manual
+        end
+
+        it 'unblocks the deployment' do
+          expect { subject }.to change { deployment.status }.from('blocked').to('created')
+        end
+      end
+
+      context 'when additional approvals are required' do
+        let(:access_level_setting) { multi_access_level }
+        let(:approval_rules) { [build(:protected_environment_approval_rule, :maintainer_access, required_approvals: 2)] }
+
+        it 'does not change the build' do
+          expect { subject }.not_to change { deployment.deployable.reload.status }
+        end
+      end
+    end
+
     context 'validations' do
       context 'when status is not recognized' do
         let(:status) { 'foo' }
@@ -187,7 +243,7 @@ RSpec.describe Deployments::ApprovalService do
       end
 
       context 'when environment is not protected' do
-        let(:deployment) { create(:deployment, project: project, deployable: build) }
+        let(:deployment) { create(:deployment, project: project, deployable: ci_build) }
 
         include_examples 'error', message: 'This environment is not protected.'
       end
@@ -235,7 +291,7 @@ RSpec.describe Deployments::ApprovalService do
       end
 
       context 'when deployment is not blocked' do
-        let(:deployment) { create(:deployment, project: project, environment: environment, deployable: build) }
+        let(:deployment) { create(:deployment, project: project, environment: environment, deployable: ci_build) }
 
         include_examples 'error', message: 'This deployment is not waiting for approvals.'
       end