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

Set an approval rule to deployment approval

See merge request gitlab-org/gitlab!83495

config/feature_flags/development/deployment_approval_rules.yml

+---
+name: deployment_approval_rules
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/83495
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/354726
+milestone: '14.10'
+type: development
+group: group::release
+default_enabled: false

ee/app/models/ee/environment.rb

@@ -89,6 +89,20 @@ module EE
       associated_protected_environments.map(&:required_approval_count).max
     end
 
+    def has_approval_rules?
+      return false unless ::Feature.enabled?(:deployment_approval_rules, project, default_enabled: :yaml)
+
+      associated_approval_rules.any?
+    end
+
+    def find_approval_rule_for(user, represented_as: nil)
+      associated_approval_rules.find do |rule|
+        next if represented_as && rule.humanize.exclude?(represented_as)
+
+        rule.check_access(user)
+      end
+    end
+
     private
 
     def protected_environment_accesses(user)
@@ -106,5 +120,12 @@ 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/ee/project_group_link.rb

@@ -19,7 +19,7 @@ module EE
       project.protected_tags.create_access_by_group(group).delete_all
 
       # For protected environments
-      project.protected_environments.deploy_access_levels_by_group(group).delete_all
+      project.protected_environments.revoke_group(group)
     end
   end
 end

ee/app/models/ee/project_member.rb

@@ -33,7 +33,7 @@ module EE
     def delete_protected_environment_acceses
       return unless user.present? && project.present?
 
-      project.protected_environments.deploy_access_levels_by_user(user).delete_all
+      project.protected_environments.revoke_user(user)
     end
 
     def gma_enforcement

ee/app/models/protected_environment.rb

@@ -25,16 +25,32 @@ class ProtectedEnvironment < ApplicationRecord
   end
 
   class << self
-    def deploy_access_levels_by_user(user)
-      ProtectedEnvironment::DeployAccessLevel
-        .where(protected_environment_id: select(:id))
-        .where(user: user)
+    def revoke_user(user)
+      transaction do
+        ProtectedEnvironment::DeployAccessLevel
+          .where(protected_environment_id: select(:id))
+          .where(user: user)
+          .delete_all
+
+        ProtectedEnvironments::ApprovalRule
+          .where(protected_environment_id: select(:id))
+          .where(user: user)
+          .delete_all
+      end
     end
 
-    def deploy_access_levels_by_group(group)
-      ProtectedEnvironment::DeployAccessLevel
-        .where(protected_environment_id: select(:id))
-        .where(group: group)
+    def revoke_group(group)
+      transaction do
+        ProtectedEnvironment::DeployAccessLevel
+          .where(protected_environment_id: select(:id))
+          .where(group: group)
+          .delete_all
+
+        ProtectedEnvironments::ApprovalRule
+          .where(protected_environment_id: select(:id))
+          .where(group: group)
+          .delete_all
+      end
     end
 
     def for_environment(environment)

ee/app/services/deployments/approval_service.rb

@@ -2,7 +2,15 @@
 
 module Deployments
   class ApprovalService < ::BaseService
+    include Gitlab::Utils::StrongMemoize
+
+    attr_reader :deployment
+
+    delegate :environment, to: :deployment
+
     def execute(deployment, status)
+      @deployment = deployment
+
       error_message = validate(deployment, status)
       return error(error_message) if error_message
 
@@ -22,7 +30,11 @@ module Deployments
 
         approval.tap { |a| a.update(status: status, comment: comment) }
       else
-        deployment.approvals.create(user: current_user, status: status, comment: comment)
+        if environment.has_approval_rules?
+          deployment.approvals.create(user: current_user, status: status, comment: comment, approval_rule: approval_rule)
+        else
+          deployment.approvals.create(user: current_user, status: status, comment: comment)
+        end
       end
     end
 
@@ -42,11 +54,25 @@ module Deployments
 
       return _('This environment is not protected.') unless deployment.environment.protected?
 
-      return _("You don't have permission to review this deployment. Contact the project or group owner for help.") unless current_user&.can?(:update_deployment, deployment)
+      if environment.has_approval_rules?
+        unless current_user&.can?(:read_deployment, deployment) && approval_rule
+          return _("You don't have permission to review this deployment. Contact the project or group owner for help.")
+        end
+      else
+        unless current_user&.can?(:update_deployment, deployment)
+          return _("You don't have permission to review this deployment. Contact the project or group owner for help.")
+        end
+      end
 
       return _('This deployment is not waiting for approvals.') unless deployment.blocked?
 
       _('You cannot approve your own deployment.') if deployment.user == current_user && status == 'approved'
     end
+
+    def approval_rule
+      strong_memoize(:approval_rule) do
+        environment.find_approval_rule_for(current_user, represented_as: params[:represented_as])
+      end
+    end
   end
 end

ee/lib/ee/api/deployments.rb

@@ -18,6 +18,7 @@ module EE
             requires :deployment_id, type: String, desc: 'The Deployment ID'
             requires :status, type: String, values: ::Deployments::Approval.statuses.keys
             optional :comment, type: String, desc: 'A comment to go with the approval'
+            optional :represented_as, type: String, desc: 'The name of the User/Group/Role to use for the approval, when the user belongs to multiple approval rules.'
           end
           post ':id/deployments/:deployment_id/approval' do
             deployment = user_project.deployments.find(params[:deployment_id])

ee/spec/models/ee/project_group_link_spec.rb

@@ -53,6 +53,17 @@ RSpec.describe ProjectGroupLink do
       let(:access_levels) { protected_environment.deploy_access_levels }
 
       it_behaves_like 'deleted related access levels', ProtectedEnvironment::DeployAccessLevel
+
+      context 'with approval rules' do
+        let(:access_levels) { protected_environment.approval_rules }
+
+        before do
+          create(:protected_environment_approval_rule, protected_environment: protected_environment, group: group)
+          create(:protected_environment_approval_rule, protected_environment: protected_environment, user: user)
+        end
+
+        it_behaves_like 'deleted related access levels', ::ProtectedEnvironments::ApprovalRule
+      end
     end
   end
 end

ee/spec/models/environment_spec.rb

@@ -5,8 +5,9 @@ require 'spec_helper'
 RSpec.describe Environment, :use_clean_rails_memory_store_caching do
   include ReactiveCachingHelpers
 
-  let(:project) { create(:project, :repository) }
-  let(:environment) { create(:environment, project: project) }
+  let_it_be_with_refind(:group) { create(:group) }
+  let_it_be_with_refind(:project) { create(:project, :repository, group: group) }
+  let_it_be_with_refind(:environment) { create(:environment, project: project) }
 
   it { is_expected.to have_many(:dora_daily_metrics) }
 
@@ -280,8 +281,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
   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)
@@ -321,4 +320,86 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
       end
     end
   end
+
+  describe '#has_approval_rules?' do
+    subject { environment.has_approval_rules? }
+
+    let_it_be(:protected_environment) { create(:protected_environment, name: environment.name, project: project) }
+
+    it { is_expected.to eq(false) }
+
+    context 'with approval rules' do
+      let!(:approval_rule) { create(:protected_environment_approval_rule, :maintainer_access, protected_environment: protected_environment) }
+
+      it { is_expected.to eq(true) }
+
+      context 'when deployment_approval_rules feature flag is disabled' do
+        before do
+          stub_feature_flags(deployment_approval_rules: false)
+        end
+
+        it { is_expected.to eq(false) }
+      end
+    end
+  end
+
+  describe '#find_approval_rule_for' do
+    subject { environment.find_approval_rule_for(user, represented_as: represented_as) }
+
+    let_it_be(:qa_group) { create(:group, name: 'QA') }
+    let_it_be(:security_group) { create(:group, name: 'Security') }
+    let_it_be(:qa_user) { create(:user) }
+    let_it_be(:security_user) { create(:user) }
+    let_it_be(:super_user) { create(:user) }
+    let_it_be(:protected_environment) { create(:protected_environment, name: environment.name, project: project) }
+
+    let(:user) { qa_user }
+    let(:represented_as) { }
+
+    before_all do
+      qa_group.add_developer(qa_user)
+      qa_group.add_developer(super_user)
+      security_group.add_developer(security_user)
+      security_group.add_developer(super_user)
+    end
+
+    it { is_expected.to be_nil }
+
+    context 'with approval rules' do
+      let!(:approval_rule_for_qa) { create(:protected_environment_approval_rule, group: qa_group, protected_environment: protected_environment) }
+      let!(:approval_rule_for_security) { create(:protected_environment_approval_rule, group: security_group, protected_environment: protected_environment) }
+
+      context 'when user belongs to QA group' do
+        let(:user) { qa_user }
+
+        it { is_expected.to eq(approval_rule_for_qa) }
+      end
+
+      context 'when user belongs to Security group' do
+        let(:user) { security_user }
+
+        it { is_expected.to eq(approval_rule_for_security) }
+      end
+
+      context 'when user belongs to both groups' do
+        let(:user) { super_user }
+
+        it 'returns one of the rules' do
+          expect([approval_rule_for_qa, approval_rule_for_security]).to include(subject)
+        end
+
+        context 'when represented as QA group' do
+          let(:represented_as) { 'QA' }
+
+          it { is_expected.to eq(approval_rule_for_qa) }
+        end
+
+        context 'when represented as Security group' do
+          let(:represented_as) { 'Security' }
+
+          it { is_expected.to eq(approval_rule_for_security) }
+        end
+      end
+    end
+  end
 end

ee/spec/models/protected_environment_spec.rb

@@ -205,7 +205,7 @@ RSpec.describe ProtectedEnvironment do
     end
   end
 
-  describe '.deploy_access_levels_by_user' do
+  describe '.revoke_user' do
     let(:user) { create(:user) }
     let(:project) { create(:project) }
     let(:environment) { create(:environment, project: project, name: 'production') }
@@ -217,9 +217,13 @@ RSpec.describe ProtectedEnvironment do
       create_deploy_access_level(protected_environment, group: create(:group))
     end
 
-    it 'returns matching deploy access levels for the given user' do
-      expect(described_class.deploy_access_levels_by_user(user))
-        .to contain_exactly(deploy_access_level_for_user)
+    it 'deletes matching deploy access levels for the given user' do
+      expect(protected_environment.deploy_access_levels).to include(deploy_access_level_for_user)
+
+      described_class.revoke_user(user)
+
+      protected_environment.reload
+      expect(protected_environment.deploy_access_levels).not_to include(deploy_access_level_for_user)
     end
 
     context 'when user is assigned to protected environment in the other project' do
@@ -227,28 +231,38 @@ RSpec.describe ProtectedEnvironment do
       let(:other_protected_environment) { create(:protected_environment, project: other_project, name: 'production') }
       let(:other_deploy_access_level_for_user) { create_deploy_access_level(other_protected_environment, user: user) }
 
-      it 'returns matching deploy access levels for the given user in the specific project' do
-        expect(project.protected_environments.deploy_access_levels_by_user(user))
-          .to contain_exactly(deploy_access_level_for_user)
-        expect(other_project.protected_environments.deploy_access_levels_by_user(user))
-          .to contain_exactly(other_deploy_access_level_for_user)
+      it 'deletes matching deploy access levels for the given user in the specific project' do
+        expect(protected_environment.deploy_access_levels).to include(deploy_access_level_for_user)
+        expect(other_protected_environment.deploy_access_levels).to include(other_deploy_access_level_for_user)
+
+        project.protected_environments.revoke_user(user)
+        other_project.protected_environments.revoke_user(user)
+
+        protected_environment.reload
+        other_protected_environment.reload
+        expect(protected_environment.deploy_access_levels).not_to include(deploy_access_level_for_user)
+        expect(other_protected_environment.deploy_access_levels).not_to include(other_deploy_access_level_for_user)
       end
     end
   end
 
-  describe '.deploy_access_levels_by_group' do
+  describe '.revoke_group' do
     let(:group) { create(:group) }
     let(:project) { create(:project) }
     let(:environment) { create(:environment, project: project, name: 'production') }
     let(:protected_environment) { create(:protected_environment, project: project, name: 'production') }
     let(:deploy_access_level_for_group) { create_deploy_access_level(protected_environment, group: group) }
 
-    it 'returns matching deploy access levels for the given group' do
+    it 'deletes matching deploy access levels for the given group' do
       _deploy_access_level_for_different_group = create_deploy_access_level(protected_environment, group: create(:group))
       _deploy_access_level_for_user = create_deploy_access_level(protected_environment, user: create(:user))
 
-      expect(described_class.deploy_access_levels_by_group(group))
-        .to contain_exactly(deploy_access_level_for_group)
+      expect(protected_environment.deploy_access_levels).to include(deploy_access_level_for_group)
+
+      described_class.revoke_group(group)
+
+      protected_environment.reload
+      expect(protected_environment.deploy_access_levels).not_to include(deploy_access_level_for_group)
     end
 
     context 'when user is assigned to protected environment in the other project' do
@@ -257,10 +271,16 @@ RSpec.describe ProtectedEnvironment do
       let(:other_deploy_access_level_for_group) { create_deploy_access_level(other_protected_environment, group: group) }
 
       it 'returns matching deploy access levels for the given group in the specific project' do
-        expect(project.protected_environments.deploy_access_levels_by_group(group))
-          .to contain_exactly(deploy_access_level_for_group)
-        expect(other_project.protected_environments.deploy_access_levels_by_group(group))
-          .to contain_exactly(other_deploy_access_level_for_group)
+        expect(protected_environment.deploy_access_levels).to include(deploy_access_level_for_group)
+        expect(other_protected_environment.deploy_access_levels).to include(other_deploy_access_level_for_group)
+
+        project.protected_environments.revoke_group(group)
+        other_project.protected_environments.revoke_group(group)
+
+        protected_environment.reload
+        other_protected_environment.reload
+        expect(protected_environment.deploy_access_levels).not_to include(deploy_access_level_for_group)
+        expect(other_protected_environment.deploy_access_levels).not_to include(other_deploy_access_level_for_group)
       end
     end
   end

ee/spec/services/deployments/approval_service_spec.rb

@@ -14,10 +14,10 @@ RSpec.describe Deployments::ApprovalService do
   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) }
 
   before do
     stub_licensed_features(protected_environments: true)
-    create(:protected_environment, :maintainers_can_deploy, name: environment.name, project: project, required_approval_count: required_approval_count)
     project.add_maintainer(user) if user
   end
 
@@ -61,12 +61,37 @@ RSpec.describe Deployments::ApprovalService do
     end
   end
 
+  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) }
+
+      it 'sets an rule to the deployment approval' do
+        expect(subject[:status]).to eq(:success)
+        expect(subject[:approval].approval_rule).to eq(approval_rule)
+        expect(::Deployments::Approval.last.approval_rule).to eq(approval_rule)
+      end
+
+      context 'when deployment_approval_rules feature flag is disabled' do
+        before do
+          stub_feature_flags(deployment_approval_rules: false)
+        end
+
+        it 'does not set an rule to the deployment approval' do
+          expect(subject[:status]).to eq(:success)
+          expect(subject[:approval].approval_rule).to be_nil
+          expect(::Deployments::Approval.last.approval_rule).to be_nil
+        end
+      end
+    end
+  end
+
   describe '#execute' do
     subject { service.execute(deployment, status) }
 
     context 'when status is approved' do
       include_examples 'approve'
       include_examples 'comment'
+      include_examples 'set approval rule'
     end
 
     context 'when status is rejected' do
@@ -74,6 +99,7 @@ RSpec.describe Deployments::ApprovalService do
 
       include_examples 'reject'
       include_examples 'comment'
+      include_examples 'set approval rule'
     end
 
     context 'when user already approved' do
@@ -182,6 +208,26 @@ RSpec.describe Deployments::ApprovalService do
         include_examples 'error', message: "You don't have permission to review this deployment. Contact the project or group owner for help."
       end
 
+      context 'with approval rule' do
+        let!(:approval_rule) { create(:protected_environment_approval_rule, :maintainer_access, protected_environment: protected_environment) }
+
+        context 'when the user does not have permission to read deployment' do
+          before do
+            project.add_guest(user)
+          end
+
+          include_examples 'error', message: "You don't have permission to review this deployment. Contact the project or group owner for help."
+        end
+
+        context 'when there are no rules for the user' do
+          before do
+            project.add_developer(user)
+          end
+
+          include_examples 'error', message: "You don't have permission to review this deployment. Contact the project or group owner for help."
+        end
+      end
+
       context 'when user is nil' do
         let(:user) { nil }