Merge branch 'remove-ff-create_vulnerabilities_via_api' into 'master'

Remove feature flag `create_vulnerabilities_via_api`

See merge request gitlab-org/gitlab!75685

config/feature_flags/development/create_vulnerabilities_via_api.yml

----
-name: create_vulnerabilities_via_api
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68158
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/338694
-milestone: '14.3'
-type: development
-group: group::threat insights
-default_enabled: true

ee/app/graphql/mutations/vulnerabilities/create.rb

@@ -73,8 +73,6 @@ module Mutations
       def resolve(**attributes)
         project = authorized_find!(id: attributes.fetch(:project))
 
-        raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless Feature.enabled?(:create_vulnerabilities_via_api, project, default_enabled: :yaml)
-
         params = build_vulnerability_params(attributes)
 
         result = ::Vulnerabilities::ManuallyCreateService.new(

ee/app/services/vulnerabilities/manually_create_service.rb

@@ -17,10 +17,6 @@ module Vulnerabilities
     end
 
     def execute
-      unless Feature.enabled?(:create_vulnerabilities_via_api, @project, default_enabled: :yaml)
-        return ServiceResponse.error(message: "create_vulnerabilities_via_api feature flag is not enabled for this project")
-      end
-
       raise Gitlab::Access::AccessDeniedError unless authorized?
 
       timestamps_dont_match_state_message = match_state_fields_with_state

ee/spec/graphql/mutations/vulnerabilities/create_spec.rb

@@ -77,77 +77,61 @@ RSpec.describe Mutations::Vulnerabilities::Create do
 
       let(:project_gid) { GitlabSchema.id_from_object(project) }
 
-      context 'when feature flag is disabled' do
-        before do
-          stub_feature_flags(create_vulnerabilities_via_api: false)
-        end
-
-        it 'raises an error' do
-          expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
-        end
+      it 'returns the created vulnerability' do
+        expect(mutated_vulnerability).to be_detected
+        expect(mutated_vulnerability.description).to eq(attributes.dig(:description))
+        expect(mutated_vulnerability.finding_description).to eq(attributes.dig(:description))
+        expect(mutated_vulnerability.finding_message).to eq(attributes.dig(:message))
+        expect(mutated_vulnerability.solution).to eq(attributes.dig(:solution))
+        expect(subject[:errors]).to be_empty
       end
 
-      context 'when feature flag is enabled' do
-        before do
-          stub_feature_flags(create_vulnerabilities_via_api: project)
-        end
+      context 'with custom state' do
+        let(:custom_timestamp) { Time.new(2020, 6, 21, 14, 22, 20) }
 
-        it 'returns the created vulnerability' do
-          expect(mutated_vulnerability).to be_detected
-          expect(mutated_vulnerability.description).to eq(attributes.dig(:description))
-          expect(mutated_vulnerability.finding_description).to eq(attributes.dig(:description))
-          expect(mutated_vulnerability.finding_message).to eq(attributes.dig(:message))
-          expect(mutated_vulnerability.solution).to eq(attributes.dig(:solution))
-          expect(subject[:errors]).to be_empty
+        where(:state, :detected_at, :confirmed_at, :confirmed_by, :resolved_at, :resolved_by, :dismissed_at, :dismissed_by) do
+          [
+            ['confirmed', ref(:custom_timestamp), ref(:custom_timestamp), ref(:user), nil, nil, nil, nil],
+            ['resolved', ref(:custom_timestamp), nil, nil, ref(:custom_timestamp), ref(:user), nil, nil],
+            ['dismissed', ref(:custom_timestamp), nil, nil, nil, nil, ref(:custom_timestamp), ref(:user)]
+          ]
         end
 
-        context 'with custom state' do
-          let(:custom_timestamp) { Time.new(2020, 6, 21, 14, 22, 20) }
-
-          where(:state, :detected_at, :confirmed_at, :confirmed_by, :resolved_at, :resolved_by, :dismissed_at, :dismissed_by) do
-            [
-              ['confirmed', ref(:custom_timestamp), ref(:custom_timestamp), ref(:user), nil, nil, nil, nil],
-              ['resolved', ref(:custom_timestamp), nil, nil, ref(:custom_timestamp), ref(:user), nil, nil],
-              ['dismissed', ref(:custom_timestamp), nil, nil, nil, nil, ref(:custom_timestamp), ref(:user)]
-            ]
+        with_them do
+          let(:attributes) do
+            {
+              project: project_gid,
+              name: "Test vulnerability",
+              description: "Test vulnerability created via GraphQL",
+              scanner: scanner_attributes,
+              identifiers: [identifier_attributes],
+              state: state,
+              severity: "unknown",
+              confidence: "unknown",
+              detected_at: detected_at,
+              confirmed_at: confirmed_at,
+              resolved_at: resolved_at,
+              dismissed_at: dismissed_at,
+              solution: "rm -rf --no-preserve-root /",
+              message: "You can't fix this"
+            }
           end
 
-          with_them do
-            let(:attributes) do
-              {
-                project: project_gid,
-                name: "Test vulnerability",
-                description: "Test vulnerability created via GraphQL",
-                scanner: scanner_attributes,
-                identifiers: [identifier_attributes],
-                state: state,
-                severity: "unknown",
-                confidence: "unknown",
-                detected_at: detected_at,
-                confirmed_at: confirmed_at,
-                resolved_at: resolved_at,
-                dismissed_at: dismissed_at,
-                solution: "rm -rf --no-preserve-root /",
-                message: "You can't fix this"
-              }
-            end
-
-            it "returns a #{params[:state]} vulnerability", :aggregate_failures do
-              expect(mutated_vulnerability.state).to eq(state)
-
-              expect(mutated_vulnerability.detected_at).to eq(detected_at)
-
-              expect(mutated_vulnerability.confirmed_at).to eq(confirmed_at)
-              expect(mutated_vulnerability.confirmed_by).to eq(confirmed_by)
-
-              expect(mutated_vulnerability.resolved_at).to eq(resolved_at)
-              expect(mutated_vulnerability.resolved_by).to eq(resolved_by)
-
-              expect(mutated_vulnerability.dismissed_at).to eq(dismissed_at)
-              expect(mutated_vulnerability.dismissed_by).to eq(dismissed_by)
-
-              expect(subject[:errors]).to be_empty
-            end
+          it "returns a #{params[:state]} vulnerability", :aggregate_failures do
+            expect(mutated_vulnerability.state).to eq(state)
+
+            expect(mutated_vulnerability.detected_at).to eq(detected_at)
+
+            expect(mutated_vulnerability.confirmed_at).to eq(confirmed_at)
+            expect(mutated_vulnerability.confirmed_by).to eq(confirmed_by)
+
+            expect(mutated_vulnerability.resolved_at).to eq(resolved_at)
+            expect(mutated_vulnerability.resolved_by).to eq(resolved_by)
+
+            expect(mutated_vulnerability.dismissed_at).to eq(dismissed_at)
+            expect(mutated_vulnerability.dismissed_by).to eq(dismissed_by)
+
+            expect(subject[:errors]).to be_empty
           end
         end
       end

ee/spec/services/vulnerabilities/manually_create_service_spec.rb

@@ -18,11 +18,7 @@ RSpec.describe Vulnerabilities::ManuallyCreateService do
       project.add_developer(user)
     end
 
-    context 'when feature flag is disabled' do
-      before do
-        stub_feature_flags(create_vulnerabilities_via_api: false)
-      end
-
+    context 'with valid parameters' do
       let(:scanner_attributes) do
         {
           id: "my-custom-scanner",
@@ -46,6 +42,10 @@ RSpec.describe Vulnerabilities::ManuallyCreateService do
         }
       end
 
+      let(:identifier_fingerprint) do
+        Digest::SHA1.hexdigest("other:#{identifier_attributes[:name]}")
+      end
+
       let(:params) do
         {
           vulnerability: {
@@ -55,225 +55,170 @@ RSpec.describe Vulnerabilities::ManuallyCreateService do
             confidence: "unknown",
             identifiers: [identifier_attributes],
             scanner: scanner_attributes,
-            solution: "rm -rf --no-preserve-root /"
+            solution: "Explanation of how to fix the vulnerability.",
+            description: "A long text section describing the vulnerability more fully.",
+            message: "A short text section that describes the vulnerability. This may include the finding's specific information."
           }
         }
       end
 
-      it 'returns an error' do
-        result = subject
-        expect(result.success?).to be_falsey
-        expect(subject.message).to match(/create_vulnerabilities_via_api feature flag is not enabled for this project/)
-      end
-    end
-
-    context 'when feature flag is enabled' do
-      before do
-        stub_feature_flags(create_vulnerabilities_via_api: project)
-      end
-
-      context 'with valid parameters' do
-        let(:scanner_attributes) do
-          {
-            id: "my-custom-scanner",
-            name: "My Custom Scanner",
-            url: "https://superscanner.com",
-            vendor: vendor_attributes,
-            version: "21.37.00"
-          }
-        end
-
-        let(:vendor_attributes) do
-          {
-            name: "Custom Scanner Vendor"
-          }
-        end
+      let(:vulnerability) { subject.payload[:vulnerability] }
 
+      context 'with custom external_type and external_id' do
         let(:identifier_attributes) do
           {
             name: "Test identifier 1",
-            url: "https://test.com"
+            url: "https://test.com",
+            external_id: "my external id",
+            external_type: "my external type"
           }
         end
 
         let(:identifier_fingerprint) do
-          Digest::SHA1.hexdigest("other:#{identifier_attributes[:name]}")
-        end
-
-        let(:params) do
-          {
-            vulnerability: {
-              name: "Test vulnerability",
-              state: "detected",
-              severity: "unknown",
-              confidence: "unknown",
-              identifiers: [identifier_attributes],
-              scanner: scanner_attributes,
-              solution: "Explanation of how to fix the vulnerability.",
-              description: "A long text section describing the vulnerability more fully.",
-              message: "A short text section that describes the vulnerability. This may include the finding's specific information."
-            }
-          }
+          Digest::SHA1.hexdigest("#{identifier_attributes[:external_type]}:#{identifier_attributes[:external_id]}")
         end
 
-        let(:vulnerability) { subject.payload[:vulnerability] }
-
-        context 'with custom external_type and external_id' do
-          let(:identifier_attributes) do
-            {
-              name: "Test identifier 1",
-              url: "https://test.com",
-              external_id: "my external id",
-              external_type: "my external type"
-            }
-          end
-
-          let(:identifier_fingerprint) do
-            Digest::SHA1.hexdigest("#{identifier_attributes[:external_type]}:#{identifier_attributes[:external_id]}")
-          end
-
-          it 'uses them to create a Vulnerabilities::Identifier' do
-            primary_identifier = vulnerability.finding.primary_identifier
-            expect(primary_identifier.external_id).to eq(identifier_attributes.dig(:external_id))
-            expect(primary_identifier.external_type).to eq(identifier_attributes.dig(:external_type))
-            expect(primary_identifier.fingerprint).to eq(identifier_fingerprint)
-          end
+        it 'uses them to create a Vulnerabilities::Identifier' do
+          primary_identifier = vulnerability.finding.primary_identifier
+          expect(primary_identifier.external_id).to eq(identifier_attributes.dig(:external_id))
+          expect(primary_identifier.external_type).to eq(identifier_attributes.dig(:external_type))
+          expect(primary_identifier.fingerprint).to eq(identifier_fingerprint)
         end
+      end
 
-        it 'does not exceed query limit' do
-          expect { subject }.not_to exceed_query_limit(20)
-        end
+      it 'does not exceed query limit' do
+        expect { subject }.not_to exceed_query_limit(20)
+      end
 
-        it 'creates a new Vulnerability' do
-          expect { subject }.to change(Vulnerability, :count).by(1)
-        end
+      it 'creates a new Vulnerability' do
+        expect { subject }.to change(Vulnerability, :count).by(1)
+      end
 
-        it 'creates a Vulnerability with correct attributes' do
-          expect(vulnerability.report_type).to eq("generic")
-          expect(vulnerability.state).to eq(params.dig(:vulnerability, :state))
-          expect(vulnerability.severity).to eq(params.dig(:vulnerability, :severity))
-          expect(vulnerability.confidence).to eq(params.dig(:vulnerability, :confidence))
-        end
+      it 'creates a Vulnerability with correct attributes' do
+        expect(vulnerability.report_type).to eq("generic")
+        expect(vulnerability.state).to eq(params.dig(:vulnerability, :state))
+        expect(vulnerability.severity).to eq(params.dig(:vulnerability, :severity))
+        expect(vulnerability.confidence).to eq(params.dig(:vulnerability, :confidence))
+      end
 
-        it 'creates associated objects', :aggregate_failures do
-          expect { subject }.to change(Vulnerabilities::Finding, :count).by(1)
-            .and change(Vulnerabilities::Scanner, :count).by(1)
-            .and change(Vulnerabilities::Identifier, :count).by(1)
-        end
+      it 'creates associated objects', :aggregate_failures do
+        expect { subject }.to change(Vulnerabilities::Finding, :count).by(1)
+          .and change(Vulnerabilities::Scanner, :count).by(1)
+          .and change(Vulnerabilities::Identifier, :count).by(1)
+      end
 
-        context 'when Scanner already exists' do
-          let!(:scanner) { create(:vulnerabilities_scanner, external_id: scanner_attributes[:id]) }
+      context 'when Scanner already exists' do
+        let!(:scanner) { create(:vulnerabilities_scanner, external_id: scanner_attributes[:id]) }
 
-          it 'does not create a new Scanner' do
-            expect { subject }.to change(Vulnerabilities::Scanner, :count).by(0)
-          end
+        it 'does not create a new Scanner' do
+          expect { subject }.to change(Vulnerabilities::Scanner, :count).by(0)
         end
+      end
 
-        context 'when Identifier already exists' do
-          let!(:identifier) { create(:vulnerabilities_identifier, name: identifier_attributes[:name]) }
+      context 'when Identifier already exists' do
+        let!(:identifier) { create(:vulnerabilities_identifier, name: identifier_attributes[:name]) }
 
-          it 'does not create a new Identifier' do
-            expect { subject }.not_to change(Vulnerabilities::Identifier, :count)
-          end
+        it 'does not create a new Identifier' do
+          expect { subject }.not_to change(Vulnerabilities::Identifier, :count)
         end
+      end
 
-        it 'creates all objects with correct attributes' do
-          expect(vulnerability.title).to eq(params.dig(:vulnerability, :name))
-          expect(vulnerability.report_type).to eq("generic")
-          expect(vulnerability.state).to eq(params.dig(:vulnerability, :state))
-          expect(vulnerability.severity).to eq(params.dig(:vulnerability, :severity))
-          expect(vulnerability.confidence).to eq(params.dig(:vulnerability, :confidence))
-          expect(vulnerability.description).to eq(params.dig(:vulnerability, :description))
-          expect(vulnerability.finding_description).to eq(params.dig(:vulnerability, :description))
-          expect(vulnerability.finding_message).to eq(params.dig(:vulnerability, :message))
-          expect(vulnerability.solution).to eq(params.dig(:vulnerability, :solution))
-
-          finding = vulnerability.finding
-          expect(finding.report_type).to eq("generic")
-          expect(finding.severity).to eq(params.dig(:vulnerability, :severity))
-          expect(finding.confidence).to eq(params.dig(:vulnerability, :confidence))
-          expect(finding.message).to eq(params.dig(:vulnerability, :message))
-          expect(finding.description).to eq(params.dig(:vulnerability, :description))
-          expect(finding.solution).to eq(params.dig(:vulnerability, :solution))
-
-          scanner = finding.scanner
-          expect(scanner.name).to eq(params.dig(:vulnerability, :scanner, :name))
-
-          primary_identifier = finding.primary_identifier
-          expect(primary_identifier.name).to eq(params.dig(:vulnerability, :identifiers, 0, :name))
-          expect(primary_identifier.url).to eq(params.dig(:vulnerability, :identifiers, 0, :url))
-          expect(primary_identifier.external_id).to eq(params.dig(:vulnerability, :identifiers, 0, :name))
-          expect(primary_identifier.external_type).to eq("other")
-          expect(primary_identifier.fingerprint).to eq(identifier_fingerprint)
-        end
+      it 'creates all objects with correct attributes' do
+        expect(vulnerability.title).to eq(params.dig(:vulnerability, :name))
+        expect(vulnerability.report_type).to eq("generic")
+        expect(vulnerability.state).to eq(params.dig(:vulnerability, :state))
+        expect(vulnerability.severity).to eq(params.dig(:vulnerability, :severity))
+        expect(vulnerability.confidence).to eq(params.dig(:vulnerability, :confidence))
+        expect(vulnerability.description).to eq(params.dig(:vulnerability, :description))
+        expect(vulnerability.finding_description).to eq(params.dig(:vulnerability, :description))
+        expect(vulnerability.finding_message).to eq(params.dig(:vulnerability, :message))
+        expect(vulnerability.solution).to eq(params.dig(:vulnerability, :solution))
+
+        finding = vulnerability.finding
+        expect(finding.report_type).to eq("generic")
+        expect(finding.severity).to eq(params.dig(:vulnerability, :severity))
+        expect(finding.confidence).to eq(params.dig(:vulnerability, :confidence))
+        expect(finding.message).to eq(params.dig(:vulnerability, :message))
+        expect(finding.description).to eq(params.dig(:vulnerability, :description))
+        expect(finding.solution).to eq(params.dig(:vulnerability, :solution))
+
+        scanner = finding.scanner
+        expect(scanner.name).to eq(params.dig(:vulnerability, :scanner, :name))
+
+        primary_identifier = finding.primary_identifier
+        expect(primary_identifier.name).to eq(params.dig(:vulnerability, :identifiers, 0, :name))
+        expect(primary_identifier.url).to eq(params.dig(:vulnerability, :identifiers, 0, :url))
+        expect(primary_identifier.external_id).to eq(params.dig(:vulnerability, :identifiers, 0, :name))
+        expect(primary_identifier.external_type).to eq("other")
+        expect(primary_identifier.fingerprint).to eq(identifier_fingerprint)
+      end
 
-        context "when state fields match state" do
-          let(:params) do
-            {
-              vulnerability: {
-                name: "Test vulnerability",
-                state: "confirmed",
-                severity: "unknown",
-                confidence: "unknown",
-                confirmed_at: Time.now.iso8601,
-                identifiers: [identifier_attributes],
-                scanner: scanner_attributes
-              }
+      context "when state fields match state" do
+        let(:params) do
+          {
+            vulnerability: {
+              name: "Test vulnerability",
+              state: "confirmed",
+              severity: "unknown",
+              confidence: "unknown",
+              confirmed_at: Time.now.iso8601,
+              identifiers: [identifier_attributes],
+              scanner: scanner_attributes
             }
-          end
-
-          it 'creates Vulnerability in a different state with timestamps' do
-            freeze_time do
-              expect(vulnerability.state).to eq(params.dig(:vulnerability, :state))
-              expect(vulnerability.confirmed_at).to eq(params.dig(:vulnerability, :confirmed_at))
-              expect(vulnerability.confirmed_by).to eq(user)
-            end
-          end
+          }
         end
 
-        context "when state fields don't match state" do
-          let(:params) do
-            {
-              vulnerability: {
-                name: "Test vulnerability",
-                state: "detected",
-                severity: "unknown",
-                confidence: "unknown",
-                confirmed_at: Time.now.iso8601,
-                identifiers: [identifier_attributes],
-                scanner: scanner_attributes
-              }
-            }
-          end
-
-          it 'returns an error' do
-            result = subject
-            expect(result.success?).to be_falsey
-            expect(subject.message).to match(/confirmed_at can only be set/)
+        it 'creates Vulnerability in a different state with timestamps' do
+          freeze_time do
+            expect(vulnerability.state).to eq(params.dig(:vulnerability, :state))
+            expect(vulnerability.confirmed_at).to eq(params.dig(:vulnerability, :confirmed_at))
+            expect(vulnerability.confirmed_by).to eq(user)
           end
         end
       end
 
-      context 'with invalid parameters' do
+      context "when state fields don't match state" do
         let(:params) do
           {
             vulnerability: {
-              identifiers: [{
-                name: "Test identfier 1",
-                url: "https://test.com"
-              }],
-              scanner: {
-                name: "My manual scanner"
-              }
+              name: "Test vulnerability",
+              state: "detected",
+              severity: "unknown",
+              confidence: "unknown",
+              confirmed_at: Time.now.iso8601,
+              identifiers: [identifier_attributes],
+              scanner: scanner_attributes
             }
           }
         end
 
         it 'returns an error' do
-          expect(subject.error?).to be_truthy
+          result = subject
+          expect(result.success?).to be_falsey
+          expect(subject.message).to match(/confirmed_at can only be set/)
         end
       end
     end
+
+    context 'with invalid parameters' do
+      let(:params) do
+        {
+          vulnerability: {
+            identifiers: [{
+              name: "Test identfier 1",
+              url: "https://test.com"
+            }],
+            scanner: {
+              name: "My manual scanner"
+            }
+          }
+        }
+      end
+
+      it 'returns an error' do
+        expect(subject.error?).to be_truthy
+      end
+    end
   end
 
   context 'when user does not have rights to dismiss a vulnerability' do