Merge branch 'update_docs_for_vulnerability_check_api_and_validation' into 'master'

Update docs for Vulnerability-Check api

See merge request gitlab-org/gitlab!74558

doc/api/merge_request_approvals.md

@@ -294,7 +294,12 @@ POST /projects/:id/approval_rules
 | `rule_type`            | string  | no       | The type of rule. `any_approver` is a pre-configured default rule with `approvals_required` at `0`. Other rules are `regular`.
 | `user_ids`             | Array   | no       | The ids of users as approvers                                    |
 | `group_ids`            | Array   | no       | The ids of groups as approvers                                   |
-| `protected_branch_ids` | Array   | no       | **(PREMIUM)** The ids of protected branches to scope the rule by. To identify the ID, [use the API](protected_branches.md#list-protected-branches). |
+| `protected_branch_ids` | Array   | no       | The IDs of protected branches to scope the rule by. To identify the ID, [use the API](protected_branches.md#list-protected-branches). |
+| `report_type` | string   | no       | The report type required when the rule type is `report_approver`. The supported report types are: `vulnerability`, `license_scanning`, `code_coverage`. |
+| `scanners` | Array   | no       | The security scanners the `Vulnerability-Check` approval rule considers. The supported scanners are: `sast`, `secret_detection`, `dependency_scanning`, `container_scanning`, `dast`, `coverage_fuzzing`, `api_fuzzing`. Defaults to all supported scanners. |
+| `severity_levels` | Array   | no       | The severity levels the `Vulnerability-Check` approval rule considers. The supported severity levels are: `info`, `unknown`, `low`, `medium`, `high`, `critical`. Defaults to `unknown`, `high`, and `critical`. |
+| `vulnerabilities_allowed` | integer   | no       | The number of vulnerabilities allowed for the `Vulnerability-Check` approval rule. Defaults to `0`. |
+| `vulnerability_states` | Array   | no       | The vulnerability states the `Vulnerability-Check` approval rule considers. The supported vulnerability states are: `newly_detected` (default), `detected`, `confirmed`, `resolved`, `dismissed`. |
 
 ```json
 {
@@ -417,7 +422,11 @@ PUT /projects/:id/approval_rules/:approval_rule_id
 | `approvals_required`   | integer | yes      | The number of required approvals for this rule                   |
 | `user_ids`             | Array   | no       | The ids of users as approvers                                    |
 | `group_ids`            | Array   | no       | The ids of groups as approvers                                   |
-| `protected_branch_ids` | Array   | no       | **(PREMIUM)** The ids of protected branches to scope the rule by. To identify the ID, [use the API](protected_branches.md#list-protected-branches). |
+| `protected_branch_ids` | Array   | no       | The IDs of protected branches to scope the rule by. To identify the ID, [use the API](protected_branches.md#list-protected-branches). |
+| `scanners` | Array   | no       | The security scanners the `Vulnerability-Check` approval rule considers. The supported scanners are: `sast`, `secret_detection`, `dependency_scanning`, `container_scanning`, `dast`, `coverage_fuzzing`, `api_fuzzing`. Defaults to all supported scanners. |
+| `severity_levels` | Array   | no       | The severity levels the `Vulnerability-Check` approval rule considers. The supported severity levels are: `info`, `unknown`, `low`, `medium`, `high`, `critical`. Defaults to `unknown`, `high`, and `critical`. |
+| `vulnerabilities_allowed` | integer   | no       | The number of vulnerabilities allowed for the `Vulnerability-Check` approval rule. Defaults to `0`. |
+| `vulnerability_states` | Array   | no       | The vulnerability states the `Vulnerability-Check` approval rule considers. The supported vulnerability states are: `newly_detected` (default), `detected`, `confirmed`, `resolved`, `dismissed`. |
 
 ```json
 {

ee/app/models/approval_project_rule.rb

@@ -27,9 +27,9 @@ class ApprovalProjectRule < ApplicationRecord
   scope :distinct_scanners, -> { scan_finding.select(:scanners).distinct }
 
   alias_method :code_owner, :code_owner?
-  validate :validate_default_license_report_name, on: :update, if: :report_approver?
 
   validates :name, uniqueness: { scope: [:project_id, :rule_type] }
+  validate :validate_security_report_approver_name
   validates :rule_type, uniqueness: { scope: :project_id, message: proc { _('any-approver for the project already exists') } }, if: :any_approver?
 
   validates :scanners, if: :scanners_changed?, inclusion: { in: SUPPORTED_SCANNERS }
@@ -93,13 +93,6 @@ class ApprovalProjectRule < ApplicationRecord
       )
   end
 
-  def validate_default_license_report_name
-    return unless name_changed?
-    return unless name_was == ApprovalRuleLike::DEFAULT_NAME_FOR_LICENSE_REPORT
-
-    errors.add(:name, _("cannot be modified"))
-  end
-
   def merge_request_report_approver_rule(merge_request)
     if scan_finding?
       merge_request
@@ -115,4 +108,25 @@ class ApprovalProjectRule < ApplicationRecord
         .find_or_initialize_by(report_type: report_type)
     end
   end
+
+  def validate_security_report_approver_name
+    [
+      [DEFAULT_NAME_FOR_VULNERABILITY_REPORT, 'vulnerability'],
+      [DEFAULT_NAME_FOR_LICENSE_REPORT, 'license_scanning'],
+      [DEFAULT_NAME_FOR_COVERAGE, 'code_coverage']
+    ].each do |report|
+      name_type = { name: report[0], type: report[1] }
+
+      validate_name_type(name_type)
+    end
+  end
+
+  def validate_name_type(name_type)
+    if name != name_type[:name] && report_type == name_type[:type]
+      errors.add(:report_type, _("%{type} only supports %{name} name") % name_type)
+
+    elsif name == name_type[:name] && report_type != name_type[:type]
+      errors.add(:name, _("%{name} is reserved for %{type} report type") % name_type)
+    end
+  end
 end

ee/spec/models/approval_merge_request_rule_spec.rb

@@ -70,20 +70,6 @@ RSpec.describe ApprovalMergeRequestRule, factory_default: :keep do
       end
     end
 
-    context 'report_approver rules' do
-      it 'is valid' do
-        expect(build(:report_approver_rule)).to be_valid
-      end
-
-      it 'validates presence of report_type' do
-        rule = build(:report_approver_rule)
-        expect(rule).to be_valid
-
-        rule.report_type = nil
-        expect(rule).not_to be_valid
-      end
-    end
-
     context 'any_approver rules' do
       let(:rule) { build(:approval_merge_request_rule, merge_request: merge_request, rule_type: :any_approver) }
 

ee/spec/models/approval_project_rule_spec.rb

@@ -157,11 +157,13 @@ RSpec.describe ApprovalProjectRule do
     let(:project_approval_rule) { create(:approval_project_rule) }
     let(:license_compliance_rule) { create(:approval_project_rule, :license_scanning) }
     let(:vulnerability_check_rule) { create(:approval_project_rule, :vulnerability) }
+    let(:coverage_check_rule) { create(:approval_project_rule, :code_coverage) }
 
     context "when creating a new rule" do
       specify { expect(project_approval_rule).to be_valid }
       specify { expect(license_compliance_rule).to be_valid }
       specify { expect(vulnerability_check_rule).to be_valid }
+      specify { expect(coverage_check_rule).to be_valid }
     end
 
     context "when attempting to edit the name of the rule" do
@@ -177,29 +179,45 @@ RSpec.describe ApprovalProjectRule do
         subject { license_compliance_rule }
 
         specify { expect(subject).not_to be_valid }
-        specify { expect { subject.valid? }.to change { subject.errors[:name].present? } }
+        specify { expect { subject.valid? }.to change { subject.errors[:report_type].present? } }
+      end
+
+      context "with a `Coverage-Check` rule" do
+        subject { coverage_check_rule }
+
+        specify { expect(subject).not_to be_valid }
+        specify { expect { subject.valid? }.to change { subject.errors[:report_type].present? } }
       end
 
       context "with a `Vulnerability-Check` rule" do
-        using RSpec::Parameterized::TableSyntax
-
-        where(:is_valid, :scanners, :vulnerabilities_allowed, :severity_levels, :vulnerability_states) do
-          true  | []                                     | 0     | []                       | %w(newly_detected)
-          true  | %w(dast)                               | 1     | %w(critical high medium) | %w(newly_detected resolved)
-          true  | %w(dast sast)                          | 10    | %w(critical high)        | %w(resolved detected)
-          true  | %w(dast dast)                          | 100   | %w(critical)             | %w(detected dismissed)
-          false | %w(dast dast)                          | 100   | %w(critical)             | %w(dismissed unknown)
-          false | %w(dast dast)                          | 100   | %w(unknown_severity)     | %w(detected dismissed)
-          false | %w(dast unknown_scanner)               | 100   | %w(critical)             | %w(detected dismissed)
-          false | [described_class::UNSUPPORTED_SCANNER] | 100   | %w(critical)             | %w(detected dismissed)
-          false | %w(dast sast)                          | 1.1   | %w(critical)             | %w(detected dismissed)
-          false | %w(dast sast)                          | 'one' | %w(critical)             | %w(detected dismissed)
+        context 'different combinations of specific attributes' do
+          using RSpec::Parameterized::TableSyntax
+
+          where(:is_valid, :scanners, :vulnerabilities_allowed, :severity_levels, :vulnerability_states) do
+            true  | []                                     | 0     | []                       | %w(newly_detected)
+            true  | %w(dast)                               | 1     | %w(critical high medium) | %w(newly_detected resolved)
+            true  | %w(dast sast)                          | 10    | %w(critical high)        | %w(resolved detected)
+            true  | %w(dast dast)                          | 100   | %w(critical)             | %w(detected dismissed)
+            false | %w(dast dast)                          | 100   | %w(critical)             | %w(dismissed unknown)
+            false | %w(dast dast)                          | 100   | %w(unknown_severity)     | %w(detected dismissed)
+            false | %w(dast unknown_scanner)               | 100   | %w(critical)             | %w(detected dismissed)
+            false | [described_class::UNSUPPORTED_SCANNER] | 100   | %w(critical)             | %w(detected dismissed)
+            false | %w(dast sast)                          | 1.1   | %w(critical)             | %w(detected dismissed)
+            false | %w(dast sast)                          | 'one' | %w(critical)             | %w(detected dismissed)
+          end
+
+          with_them do
+            let(:vulnerability_check_rule) { build(:approval_project_rule, :vulnerability, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed, severity_levels: severity_levels, vulnerability_states: vulnerability_states) }
+
+            specify { expect(vulnerability_check_rule.valid?).to be(is_valid) }
+          end
         end
 
-        with_them do
-          let(:vulnerability_check_rule) { build(:approval_project_rule, :vulnerability, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed, severity_levels: severity_levels, vulnerability_states: vulnerability_states) }
+        context 'with invalid name' do
+          subject { vulnerability_check_rule }
 
-          specify { expect(vulnerability_check_rule.valid?).to be(is_valid) }
+          specify { expect(subject).not_to be_valid }
+          specify { expect { subject.valid? }.to change { subject.errors[:report_type].present? } }
         end
       end
     end

ee/spec/models/concerns/approval_rule_like_spec.rb

@@ -77,6 +77,23 @@ RSpec.describe ApprovalRuleLike do
           expect(subject).to be_valid
         end
       end
+
+      context 'with report_type set to report_approver' do
+        before do
+          subject.rule_type = :report_approver
+        end
+
+        it 'is valid' do
+          subject.report_type = :vulnerability
+          subject.name = described_class::DEFAULT_NAME_FOR_VULNERABILITY_REPORT
+          expect(subject).to be_valid
+        end
+
+        it 'is invalid' do
+          subject.report_type = nil
+          expect(subject).not_to be_valid
+        end
+      end
     end
   end
 

locale/gitlab.pot

@@ -776,6 +776,9 @@ msgstr ""
 msgid "%{name} is already being used for another emoji"
 msgstr ""
 
+msgid "%{name} is reserved for %{type} report type"
+msgstr ""
+
 msgid "%{name} is scheduled for %{action}"
 msgstr ""
 
@@ -1014,6 +1017,9 @@ msgstr ""
 msgid "%{total} warnings found: showing first %{warningsDisplayed}"
 msgstr ""
 
+msgid "%{type} only supports %{name} name"
+msgstr ""
+
 msgid "%{userName} (cannot merge)"
 msgstr ""
 
@@ -40603,9 +40609,6 @@ msgstr ""
 msgid "cannot be enabled until a valid credit card is on file"
 msgstr ""
 
-msgid "cannot be modified"
-msgstr ""
-
 msgid "cannot be used for user namespace"
 msgstr ""