Refresh license compliance checks

When a new software license policy is created,
this will refresh the license check approval rules associated
with each open merge request for the project that the
software license policy was created for.

config/sidekiq_queues.yml

@@ -116,3 +116,4 @@
   - [incident_management, 2]
   - [jira_connect, 1]
   - [update_external_pull_requests, 3]
+  - [refresh_license_compliance_checks, 2]

ee/app/models/approval_merge_request_rule.rb

@@ -55,6 +55,10 @@ class ApprovalMergeRequestRule < ApplicationRecord
   # To be removed with https://gitlab.com/gitlab-org/gitlab-ee/issues/11834
   scope :code_owner, -> { where(code_owner: true).or(where(rule_type: :code_owner)) }
   scope :security_report, -> { report_approver.where(report_type: :security) }
+  scope :license_compliance, -> { report_approver.where(report_type: :license_management) }
+  scope :with_head_pipeline, -> { includes(merge_request: [:head_pipeline]) }
+  scope :open_merge_requests, -> { merge(MergeRequest.opened) }
+  scope :for_checks_that_can_be_refreshed, -> { license_compliance.open_merge_requests.with_head_pipeline }
 
   def self.find_or_create_code_owner_rule(merge_request, pattern)
     merge_request.approval_rules.code_owner.where(name: pattern).first_or_create do |rule|
@@ -114,6 +118,12 @@ class ApprovalMergeRequestRule < ApplicationRecord
     self.approved_approver_ids = merge_request.approvals.map(&:user_id) & approvers.map(&:id)
   end
 
+  def refresh_required_approvals!(project_approval_rule)
+    return unless report_approver?
+
+    refresh_license_management_approvals(project_approval_rule) if license_management?
+  end
+
   private
 
   def validate_approval_project_rule
@@ -122,4 +132,15 @@ class ApprovalMergeRequestRule < ApplicationRecord
 
     errors.add(:approval_project_rule, 'must be for the same project')
   end
+
+  def refresh_license_management_approvals(project_approval_rule)
+    license_report = merge_request.head_pipeline&.license_management_report
+    return if license_report.blank?
+
+    if license_report.violates?(project.software_license_policies)
+      update!(approvals_required: project_approval_rule.approvals_required)
+    else
+      update!(approvals_required: 0)
+    end
+  end
 end

ee/app/models/ee/project.rb

@@ -53,6 +53,7 @@ module EE
       has_many :approver_users, through: :approvers, source: :user
       has_many :approver_groups, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
       has_many :approval_rules, class_name: 'ApprovalProjectRule'
+      has_many :approval_merge_request_rules, through: :merge_requests, source: :approval_rules
       has_many :audit_events, as: :entity
       has_many :designs, inverse_of: :project, class_name: 'DesignManagement::Design'
       has_many :path_locks

ee/app/models/software_license.rb

@@ -8,4 +8,11 @@ class SoftwareLicense < ApplicationRecord
   validates :name, presence: true
 
   scope :ordered, -> { order(:name) }
+
+  def self.create_policy_for!(project:, name:, approval_status:)
+    project.software_license_policies.create!(
+      approval_status: approval_status,
+      software_license: safe_find_or_create_by!(name: name)
+    )
+  end
 end

ee/app/services/software_license_policies/create_service.rb

@@ -7,37 +7,27 @@ module SoftwareLicensePolicies
       super(project, user, params.with_indifferent_access)
     end
 
-    # Returns the created managed license.
-    # rubocop: disable CodeReuse/ActiveRecord
     def execute
       return error("", 403) unless can?(@current_user, :admin_software_license_policy, @project)
 
-      # Load or create the software license
-      name = params.delete(:name)
-
-      software_license = SoftwareLicense.transaction do
-        SoftwareLicense.transaction(requires_new: true) do
-          SoftwareLicense.find_or_create_by(name: name)
-        end
-      rescue ActiveRecord::RecordNotUnique
-        retry
-      end
-
-      # Add the software license to params
-      params[:software_license] = software_license
-
-      begin
-        software_license_policy = @project.software_license_policies.create(params)
-      rescue ArgumentError => ex
-        return error(ex.message, 400)
-      end
+      success(software_license_policy: create_software_license_policy)
+    rescue ActiveRecord::RecordInvalid => exception
+      error(exception.record.errors.full_messages, 400)
+    rescue ArgumentError => exception
+      log_error(exception.message)
+      error(exception.message, 400)
+    end
 
-      if software_license_policy.errors.any?
-        return error(software_license_policy.errors.full_messages.join("\n"), 400)
-      end
+    private
 
-      success(software_license_policy: software_license_policy)
+    def create_software_license_policy
+      policy = SoftwareLicense.create_policy_for!(
+        project: project,
+        name: params[:name],
+        approval_status: params[:approval_status]
+      )
+      RefreshLicenseComplianceChecksWorker.perform_async(project.id)
+      policy
     end
-    # rubocop: enable CodeReuse/ActiveRecord
   end
 end

ee/app/services/software_license_policies/update_service.rb

@@ -15,6 +15,7 @@ module SoftwareLicensePolicies
 
       begin
         software_license_policy.update(params)
+        RefreshLicenseComplianceChecksWorker.perform_async(project.id)
       rescue ArgumentError => ex
         return error(ex.message, 400)
       end

ee/app/workers/all_queues.yml

@@ -67,5 +67,6 @@
 - project_import_schedule
 - project_update_repository_storage
 - rebase
+- refresh_license_compliance_checks
 - repository_update_mirror
 - repository_push_audit_event

ee/app/workers/refresh_license_compliance_checks_worker.rb

+# frozen_string_literal: true
+
+class RefreshLicenseComplianceChecksWorker
+  include ApplicationWorker
+
+  def perform(project_id)
+    project = Project.find(project_id)
+    project_approval_rule = project
+      .approval_rules
+      .report_approver
+      .find_by_name!(ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT)
+
+    approval_rules = project.approval_merge_request_rules.for_checks_that_can_be_refreshed
+    approval_rules.find_each do |approval_rule|
+      approval_rule.refresh_required_approvals!(project_approval_rule)
+    end
+    # If the project or project approval rule is deleted
+    # before this job runs, then it is possible that
+    # the project and project approval rule record
+    # will not be found.
+  rescue ActiveRecord::RecordNotFound => error
+    logger.error(error.message)
+  end
+end

ee/changelogs/unreleased/14708-refresh-license-approval-new-blacklist-item.yml

+---
+title: Refresh license approval check when a license is blacklisted
+merge_request: 16070
+author:
+type: added

ee/spec/factories/approval_rules.rb

@@ -19,6 +19,10 @@ FactoryBot.define do
     report_type :security
     sequence(:name) { |n| "*-#{n}.js" }
 
+    trait :requires_approval do
+      approvals_required { rand(1..ApprovalProjectRule::APPROVALS_REQUIRED_MAX) }
+    end
+
     trait :license_management do
       name ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT
       report_type :license_management

ee/spec/models/approval_merge_request_rule_spec.rb

@@ -280,4 +280,36 @@ describe ApprovalMergeRequestRule do
       end
     end
   end
+
+  describe "#refresh_required_approvals!" do
+    before do
+      stub_licensed_features(license_management: true)
+    end
+
+    context "when the rule is a `#{ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT}` rule" do
+      subject { create(:report_approver_rule, :requires_approval, :license_management, merge_request: open_merge_request) }
+      let(:open_merge_request) { create(:merge_request, :opened, target_project: project, source_project: project) }
+      let!(:project_approval_rule) { create(:approval_project_rule, :requires_approval, :license_management, project: project) }
+      let(:project) { create(:project) }
+      let!(:open_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) }
+      let!(:blacklist_policy) { create(:software_license_policy, project: project, software_license: license, approval_status: :blacklisted) }
+
+      before do
+        subject.refresh_required_approvals!(project_approval_rule)
+      end
+
+      context "when the latest license report violates the compliance policy" do
+        let(:license) { create(:software_license, name: license_report.license_names[0]) }
+        let(:license_report) { open_pipeline.license_management_report }
+
+        specify { expect(subject.approvals_required).to be(project_approval_rule.approvals_required) }
+      end
+
+      context "when the latest license report adheres to the compliance policy" do
+        let(:license) { create(:software_license, name: SecureRandom.uuid) }
+
+        specify { expect(subject.approvals_required).to be_zero }
+      end
+    end
+  end
 end

ee/spec/models/software_license_spec.rb

@@ -9,4 +9,28 @@ describe SoftwareLicense do
     it { is_expected.to include_module(Presentable) }
     it { is_expected.to validate_presence_of(:name) }
   end
+
+  describe ".create_policy_for!" do
+    subject { described_class }
+    let(:project) { create(:project) }
+
+    context "when a software license with a given name has already been created" do
+      let(:mit_license) { create(:software_license, :mit) }
+      let(:result) { subject.create_policy_for!(project: project, name: mit_license.name, approval_status: :approved) }
+
+      specify { expect(result).to be_persisted }
+      specify { expect(result).to be_approved }
+      specify { expect(result.software_license).to eql(mit_license) }
+    end
+
+    context "when a software license with a given name has NOT been created" do
+      let(:license_name) { SecureRandom.uuid }
+      let(:result) { subject.create_policy_for!(project: project, name: license_name, approval_status: :blacklisted) }
+
+      specify { expect(result).to be_persisted }
+      specify { expect(result).to be_blacklisted }
+      specify { expect(result.software_license).to be_persisted }
+      specify { expect(result.software_license.name).to eql(license_name) }
+    end
+  end
 end

ee/spec/services/software_license_policies/create_service_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe SoftwareLicensePolicies::CreateService do
-  let(:project) { create(:project)}
+  let(:project) { create(:project) }
   let(:params) { { name: 'ExamplePL/2.1', approval_status: 'blacklisted' } }
 
   let(:user) do
@@ -16,7 +16,7 @@ describe SoftwareLicensePolicies::CreateService do
     stub_licensed_features(license_management: true)
   end
 
-  subject { described_class.new(project, user, params).execute }
+  subject { described_class.new(project, user, params) }
 
   describe '#execute' do
     context 'with license management unavailable' do
@@ -25,26 +25,62 @@ describe SoftwareLicensePolicies::CreateService do
       end
 
       it 'does not creates a software license policy' do
-        expect { subject }.to change { project.software_license_policies.count }.by(0)
+        expect { subject.execute }.to change { project.software_license_policies.count }.by(0)
       end
     end
 
     context 'with a user who is allowed to admin' do
       it 'creates one software license policy correctly' do
-        expect { subject }.to change { project.software_license_policies.count }.from(0).to(1)
+        expect { subject.execute }.to change { project.software_license_policies.count }.from(0).to(1)
 
         software_license_policy = project.software_license_policies.last
         expect(software_license_policy).to be_persisted
-        expect(software_license_policy.name).to eq('ExamplePL/2.1')
+        expect(software_license_policy.name).to eq(params[:name])
         expect(software_license_policy.approval_status).to eq('blacklisted')
       end
+
+      context "when valid parameters are specified" do
+        let(:result) { subject.execute }
+
+        before do
+          allow(RefreshLicenseComplianceChecksWorker).to receive(:perform_async)
+          result
+        end
+
+        specify { expect(result[:status]).to be(:success) }
+        specify { expect(result[:software_license_policy]).to be_present }
+        specify { expect(result[:software_license_policy]).to be_persisted }
+        specify { expect(result[:software_license_policy].name).to eql(params[:name]) }
+        specify { expect(result[:software_license_policy].approval_status).to eql('blacklisted') }
+        specify { expect(RefreshLicenseComplianceChecksWorker).to have_received(:perform_async).with(project.id) }
+      end
+
+      context "when an argument error is raised" do
+        before do
+          allow_any_instance_of(Project).to receive(:software_license_policies).and_raise(ArgumentError)
+        end
+
+        specify { expect(subject.execute[:status]).to be(:error) }
+        specify { expect(subject.execute[:message]).to be_present }
+        specify { expect(subject.execute[:http_status]).to be(400) }
+      end
+
+      context "when invalid input is provided" do
+        before do
+          params[:approval_status] = nil
+        end
+
+        specify { expect(subject.execute[:status]).to be(:error) }
+        specify { expect(subject.execute[:message]).to be_present }
+        specify { expect(subject.execute[:http_status]).to be(400) }
+      end
     end
 
     context 'with a user not allowed to admin' do
       let(:user) { create(:user) }
 
       it 'does not create a software license policy' do
-        expect { subject }.to change { project.software_license_policies.count }.by(0)
+        expect { subject.execute }.to change { project.software_license_policies.count }.by(0)
       end
     end
   end

ee/spec/services/software_license_policies/update_service_spec.rb

@@ -48,10 +48,12 @@ describe SoftwareLicensePolicies::UpdateService do
 
       context 'with a user allowed to admin' do
         it 'updates the software license policy correctly' do
+          allow(RefreshLicenseComplianceChecksWorker).to receive(:perform_async)
           update_software_license_policy(opts)
 
           expect(software_license_policy).to be_valid
           expect(software_license_policy.approval_status).to eq(opts[:approval_status])
+          expect(RefreshLicenseComplianceChecksWorker).to have_received(:perform_async).with(project.id)
         end
       end
 

ee/spec/workers/refresh_license_compliance_checks_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe RefreshLicenseComplianceChecksWorker do
+  subject { described_class.new }
+
+  describe '#perform' do
+    let(:project) { create(:project) }
+
+    before do
+      stub_licensed_features(license_management: true)
+    end
+
+    context "when there are merge requests associated with the project" do
+      let!(:open_merge_request) { create(:merge_request, :opened, target_project: project, source_project: project) }
+      let!(:closed_merge_request) { create(:merge_request, :closed, target_project: project, source_project: project) }
+
+      context "when the `#{ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT}` approval rule is enabled" do
+        let!(:open_merge_request_approval_rule) { create(:report_approver_rule, :requires_approval, :license_management, merge_request: open_merge_request) }
+        let!(:closed_merge_request_approval_rule) { create(:report_approver_rule, :license_management, merge_request: closed_merge_request, approvals_required: 0) }
+        let!(:project_approval_rule) { create(:approval_project_rule, :requires_approval, :license_management, project: project) }
+
+        context "when a license is blacklisted, that appears in some of the license management reports" do
+          let!(:open_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) }
+          let!(:closed_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [closed_merge_request]) }
+          let!(:blacklist_policy) { create(:software_license_policy, project: project, software_license: license, approval_status: :blacklisted) }
+          let(:license) { create(:software_license, name: license_report.license_names[0]) }
+          let(:license_report) { open_pipeline.license_management_report }
+
+          before do
+            subject.perform(project.id)
+          end
+
+          specify { expect(open_merge_request_approval_rule.reload.approvals_required).to eql(project_approval_rule.approvals_required) }
+          specify { expect(closed_merge_request_approval_rule.reload.approvals_required).to be_zero }
+        end
+
+        context "when none of the blacklisted licenses appear in the most recent license management reports" do
+          let!(:open_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [open_merge_request]) }
+          let!(:closed_pipeline) { create(:ee_ci_pipeline, :success, :with_license_management_report, project: project, merge_requests_as_head_pipeline: [closed_merge_request]) }
+          let!(:blacklist_policy) { create(:software_license_policy, project: project, software_license: license, approval_status: :blacklisted) }
+          let(:license) { create(:software_license, name: SecureRandom.uuid) }
+
+          before do
+            subject.perform(project.id)
+          end
+
+          specify { expect(open_merge_request_approval_rule.reload.approvals_required).to be_zero }
+          specify { expect(closed_merge_request_approval_rule.reload.approvals_required).to be_zero }
+        end
+      end
+    end
+
+    context "when the project does not exist" do
+      specify do
+        expect { subject.perform(SecureRandom.uuid) }.not_to raise_error
+      end
+    end
+
+    context "when the project does not have a license check rule" do
+      specify do
+        expect { subject.perform(project.id) }.not_to raise_error
+      end
+    end
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -376,6 +376,7 @@ project:
 - index_status
 - feature_usage
 - approval_rules
+- approval_merge_request_rules
 - approvers
 - approver_users
 - pages_domains