From d8ac2e35ace131bdf620400bc5ca4547bb1db0ef Mon Sep 17 00:00:00 2001
From: Charlie Kroon <ckroon@gitlab.com>
Date: Mon, 27 Jan 2025 20:46:05 +0000
Subject: [PATCH] Remove feature flag after globally enbaling
This commit removes the `vulnerability_representation_information` feature flag and deletes its configuration file.
- Updated related files and tests to reflect the removal.
- Ensured no unintended dependencies remain.
Changelog: changed
EE: true
---
.../vulnerabilities/components/footer.vue | 1 -
.../components/status_description.vue | 6 +-
.../security/vulnerabilities_controller.rb | 1 -
ee/app/graphql/types/vulnerability_type.rb | 6 --
...ulnerability_representation_information.rb | 6 --
...lnerability_representation_information.yml | 9 ---
.../frontend/vulnerabilities/footer_spec.js | 35 +++---------
.../status_description_spec.js | 19 +------
.../representation_information_spec.rb | 11 ----
...ability_representation_information_spec.rb | 32 ++++-------
.../mark_as_resolved_service_spec.rb | 55 +++++++------------
11 files changed, 41 insertions(+), 140 deletions(-)
delete mode 100644 ee/config/feature_flags/gitlab_com_derisk/vulnerability_representation_information.yml
diff --git a/ee/app/assets/javascripts/vulnerabilities/components/footer.vue b/ee/app/assets/javascripts/vulnerabilities/components/footer.vue
index 7a2cead84882..52f0e7ba0624 100644
--- a/ee/app/assets/javascripts/vulnerabilities/components/footer.vue
+++ b/ee/app/assets/javascripts/vulnerabilities/components/footer.vue
@@ -100,7 +100,6 @@ export default {
},
isRepresentationInfoAvailable() {
return (
- this.glFeatures.vulnerabilityRepresentationInformation &&
this.vulnerability.resolvedOnDefaultBranch &&
this.vulnerability.representationInformation?.resolvedInCommitShaLink
);
diff --git a/ee/app/assets/javascripts/vulnerabilities/components/status_description.vue b/ee/app/assets/javascripts/vulnerabilities/components/status_description.vue
index f99162350015..a2520c319af0 100644
--- a/ee/app/assets/javascripts/vulnerabilities/components/status_description.vue
+++ b/ee/app/assets/javascripts/vulnerabilities/components/status_description.vue
@@ -61,11 +61,7 @@ export default {
representationInformationAvailable() {
const { resolvedOnDefaultBranch } = this.vulnerability;
- return (
- this.glFeatures.vulnerabilityRepresentationInformation &&
- resolvedOnDefaultBranch &&
- this.representationInformation?.resolvedInCommitShaLink
- );
+ return resolvedOnDefaultBranch && this.representationInformation?.resolvedInCommitShaLink;
},
time() {
diff --git a/ee/app/controllers/projects/security/vulnerabilities_controller.rb b/ee/app/controllers/projects/security/vulnerabilities_controller.rb
index a33872e8f38c..921f8629ca4c 100644
--- a/ee/app/controllers/projects/security/vulnerabilities_controller.rb
+++ b/ee/app/controllers/projects/security/vulnerabilities_controller.rb
@@ -11,7 +11,6 @@ class VulnerabilitiesController < Projects::ApplicationController
before_action :authorize_read_vulnerability!, except: [:new]
before_action do
- push_frontend_feature_flag(:vulnerability_representation_information, project)
push_frontend_feature_flag(:vulnerability_details_state_modal, project)
end
diff --git a/ee/app/graphql/types/vulnerability_type.rb b/ee/app/graphql/types/vulnerability_type.rb
index d86bfc23a503..4a76ff7eace7 100644
--- a/ee/app/graphql/types/vulnerability_type.rb
+++ b/ee/app/graphql/types/vulnerability_type.rb
@@ -239,12 +239,6 @@ def ai_resolution_enabled
object.vulnerability_read.has_vulnerability_resolution?
end
- def representation_information
- return unless Feature.enabled?(:vulnerability_representation_information, object.project)
-
- object.representation_information
- end
-
private
def expose_false_positive?
diff --git a/ee/app/services/security/ingestion/create_vulnerability_representation_information.rb b/ee/app/services/security/ingestion/create_vulnerability_representation_information.rb
index b396f1690ecb..5cebac44a858 100644
--- a/ee/app/services/security/ingestion/create_vulnerability_representation_information.rb
+++ b/ee/app/services/security/ingestion/create_vulnerability_representation_information.rb
@@ -17,12 +17,6 @@ def initialize(pipeline, resolved_vulnerability_ids)
@resolved_vulnerability_ids = resolved_vulnerability_ids
end
- def execute
- return unless Feature.enabled?(:vulnerability_representation_information, pipeline.project)
-
- super
- end
-
private
attr_reader :pipeline, :resolved_vulnerability_ids
diff --git a/ee/config/feature_flags/gitlab_com_derisk/vulnerability_representation_information.yml b/ee/config/feature_flags/gitlab_com_derisk/vulnerability_representation_information.yml
deleted file mode 100644
index 8b63b89fca4d..000000000000
--- a/ee/config/feature_flags/gitlab_com_derisk/vulnerability_representation_information.yml
+++ /dev/null
@@ -1,9 +0,0 @@
----
-name: vulnerability_representation_information
-feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/372799
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/171805
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/502628
-milestone: '17.7'
-group: group::security insights
-type: gitlab_com_derisk
-default_enabled: false
diff --git a/ee/spec/frontend/vulnerabilities/footer_spec.js b/ee/spec/frontend/vulnerabilities/footer_spec.js
index a8301a6ad59f..f0b4f3e77eda 100644
--- a/ee/spec/frontend/vulnerabilities/footer_spec.js
+++ b/ee/spec/frontend/vulnerabilities/footer_spec.js
@@ -66,17 +66,9 @@ describe('Vulnerability Footer', () => {
},
});
- const createWrapper = ({
- properties,
- queryHandler,
- mountOptions,
- vulnerabilityRepresentationFlag = true,
- } = {}) => {
+ const createWrapper = ({ properties, queryHandler, mountOptions } = {}) => {
wrapper = shallowMountExtended(VulnerabilityFooter, {
propsData: { vulnerability: { ...vulnerability, ...properties } },
- provide: {
- glFeatures: { vulnerabilityRepresentationInformation: vulnerabilityRepresentationFlag },
- },
apolloProvider: createMockApollo([[vulnerabilityDiscussionsQuery, queryHandler]]),
...mountOptions,
});
@@ -378,7 +370,7 @@ describe('Vulnerability Footer', () => {
expect(statusDescription().exists()).toBe(false);
});
- it('should show the status description when the vulnerability is resolved on the default branch and there is respresentation information', () => {
+ it('should show the status description when the vulnerability is resolved on the default branch and there is representation information', () => {
createWrapper({
properties: {
pipeline: null,
@@ -407,28 +399,19 @@ describe('Vulnerability Footer', () => {
});
it.each`
- representationInformation | resolvedOnDefaultBranch | vulnerabilityRepresentationFlag | shouldIncludeRepresentationInfo
- ${vulnerability.representationInformation} | ${true} | ${true} | ${true}
- ${vulnerability.representationInformation} | ${false} | ${true} | ${false}
- ${null} | ${true} | ${true} | ${false}
- ${null} | ${false} | ${true} | ${false}
- ${vulnerability.representationInformation} | ${true} | ${false} | ${false}
- ${vulnerability.representationInformation} | ${false} | ${false} | ${false}
- ${null} | ${true} | ${false} | ${false}
+ representationInformation | resolvedOnDefaultBranch | shouldIncludeRepresentationInfo
+ ${vulnerability.representationInformation} | ${true} | ${true}
+ ${vulnerability.representationInformation} | ${false} | ${false}
+ ${null} | ${true} | ${false}
+ ${null} | ${false} | ${false}
`(
- 'shows representation information: "$shouldIncludeRepresentationInfo" when feature flag is "$vulnerabilityRepresentationFlag", resolvedOnDefaultBranch is "$resolvedOnDefaultBranch" and representationInformation is "$representationInformation"',
- ({
- resolvedOnDefaultBranch,
- vulnerabilityRepresentationFlag,
- representationInformation,
- shouldIncludeRepresentationInfo,
- }) => {
+ 'shows representation information: "$shouldIncludeRepresentationInfo" when resolvedOnDefaultBranch is "$resolvedOnDefaultBranch" and representationInformation is "$representationInformation"',
+ ({ resolvedOnDefaultBranch, representationInformation, shouldIncludeRepresentationInfo }) => {
createWrapper({
properties: {
resolvedOnDefaultBranch,
representationInformation,
},
- vulnerabilityRepresentationFlag,
});
expect(Boolean(statusDescription().props('vulnerability').representationInformation)).toBe(
diff --git a/ee/spec/frontend/vulnerabilities/status_description_spec.js b/ee/spec/frontend/vulnerabilities/status_description_spec.js
index 4f774bcfc910..590d104ee1a2 100644
--- a/ee/spec/frontend/vulnerabilities/status_description_spec.js
+++ b/ee/spec/frontend/vulnerabilities/status_description_spec.js
@@ -30,7 +30,7 @@ describe('Vulnerability status description component', () => {
// Create a date using the passed-in string, or just use the current time if nothing was passed in.
const createDate = (value) => (value ? new Date(value) : new Date()).toISOString();
- const createWrapper = (props = {}, vulnerabilityRepresentationInformation = true) => {
+ const createWrapper = (props = {}) => {
const vulnerability = props.vulnerability || { pipeline: {} };
// Automatically create the ${v.state}_at property if it doesn't exist. Otherwise, every test would need to create
// it manually for the component to mount properly.
@@ -43,7 +43,6 @@ describe('Vulnerability status description component', () => {
wrapper = mountExtended(StatusDescription, {
propsData: { ...props, vulnerability },
- provide: { glFeatures: { vulnerabilityRepresentationInformation } },
});
};
@@ -288,21 +287,5 @@ describe('Vulnerability status description component', () => {
expect(commitShaLink().exists()).toBe(false);
},
);
-
- it('does not show the commitShaLink when the "vulnerabilityRepresentationInformation" feature flag is disabled', () => {
- createWrapper(
- {
- vulnerability: {
- resolvedOnDefaultBranch: true,
- representationInformation: {
- resolvedInCommitShaLink: 'https://gitlab.com/gitlab',
- },
- },
- },
- false,
- );
-
- expect(commitShaLink().exists()).toBe(false);
- });
});
});
diff --git a/ee/spec/requests/api/graphql/vulnerabilities/representation_information_spec.rb b/ee/spec/requests/api/graphql/vulnerabilities/representation_information_spec.rb
index 4e43b866c271..e7c45749781d 100644
--- a/ee/spec/requests/api/graphql/vulnerabilities/representation_information_spec.rb
+++ b/ee/spec/requests/api/graphql/vulnerabilities/representation_information_spec.rb
@@ -74,15 +74,4 @@
post_graphql(vulnerabilities_query, current_user: user)
end.to issue_same_number_of_queries_as(control).or_fewer
end
-
- context 'when vulnerability_representation_information is disabled' do
- before do
- stub_feature_flags(vulnerability_representation_information: false)
- end
-
- it 'does not return representation information' do
- post_graphql(query, current_user: user)
- expect(representation_info).to be_nil
- end
- end
end
diff --git a/ee/spec/services/security/ingestion/create_vulnerability_representation_information_spec.rb b/ee/spec/services/security/ingestion/create_vulnerability_representation_information_spec.rb
index f4bc446f2a1f..cb2bcb403928 100644
--- a/ee/spec/services/security/ingestion/create_vulnerability_representation_information_spec.rb
+++ b/ee/spec/services/security/ingestion/create_vulnerability_representation_information_spec.rb
@@ -33,18 +33,16 @@
describe '#execute' do
subject(:execute_service) { service.execute }
- context 'when the representation_information feature flag is enabled' do
- it 'creates Vulnerabilities::RepresentationInformation for the resolved vulnerabilitiles' do
- expect { execute_service }.to change { Vulnerabilities::RepresentationInformation.count }.by(2)
-
- resolved_vulnerability_ids.each do |id|
- record = Vulnerabilities::RepresentationInformation.find_by(vulnerability_id: id)
- expect(record).to have_attributes(
- vulnerability_id: id,
- project_id: project.id,
- resolved_in_commit_sha: pipeline.sha
- )
- end
+ it 'creates Vulnerabilities::RepresentationInformation for the resolved vulnerabilities' do
+ expect { execute_service }.to change { Vulnerabilities::RepresentationInformation.count }.by(2)
+
+ resolved_vulnerability_ids.each do |id|
+ record = Vulnerabilities::RepresentationInformation.find_by(vulnerability_id: id)
+ expect(record).to have_attributes(
+ vulnerability_id: id,
+ project_id: project.id,
+ resolved_in_commit_sha: pipeline.sha
+ )
end
end
@@ -55,15 +53,5 @@
expect { service.execute }.not_to change { Vulnerabilities::RepresentationInformation.count }
end
end
-
- context 'when the feature flag is disabled' do
- before do
- stub_feature_flags(vulnerability_representation_information: false)
- end
-
- it 'does not create any records' do
- expect { service.execute }.not_to change { Vulnerabilities::RepresentationInformation.count }
- end
- end
end
end
diff --git a/ee/spec/services/security/ingestion/mark_as_resolved_service_spec.rb b/ee/spec/services/security/ingestion/mark_as_resolved_service_spec.rb
index 49300ca4d638..fae38c04d3f1 100644
--- a/ee/spec/services/security/ingestion/mark_as_resolved_service_spec.rb
+++ b/ee/spec/services/security/ingestion/mark_as_resolved_service_spec.rb
@@ -80,43 +80,28 @@
expect(second_vulnerability.reload).to be_resolved_on_default_branch
end
- context 'when the representation_information feature flag is enabled' do
- it 'creates a RepresentationInformation record for the resolved vulnerability' do
- vulnerability = create(
- :vulnerability,
- :sast,
- project: project,
- present_on_default_branch: true,
- resolved_on_default_branch: false,
- findings: [create(:vulnerabilities_finding, :with_pipeline, project: project,
- scanner: scanner)]
- )
-
- command.execute
-
- expect(vulnerability.reload).to be_resolved_on_default_branch
- representation_info = Vulnerabilities::RepresentationInformation
- .find_or_initialize_by(vulnerability_id: vulnerability.id)
- representation_info.update!(
- project_id: vulnerability.project_id,
- resolved_in_commit_sha: vulnerability.findings.first.sha
- )
- expect(representation_info.project_id).to eq(vulnerability.project_id)
- expect(representation_info.resolved_in_commit_sha).to eq(vulnerability.findings.first.sha)
- end
- end
-
- context 'when the representation_information feature flag is disabled' do
- before do
- stub_feature_flags(vulnerability_representation_information: false)
- end
+ it 'creates a RepresentationInformation record for the resolved vulnerability' do
+ vulnerability = create(
+ :vulnerability,
+ :sast,
+ project: project,
+ present_on_default_branch: true,
+ resolved_on_default_branch: false,
+ findings: [create(:vulnerabilities_finding, :with_pipeline, project: project,
+ scanner: scanner)]
+ )
- it 'does not create a RepresentationInformation record for the resolved vulnerability' do
- command.execute
+ command.execute
- expect(vulnerability.reload).to be_resolved_on_default_branch
- expect(vulnerability.representation_information).to be_nil
- end
+ expect(vulnerability.reload).to be_resolved_on_default_branch
+ representation_info = Vulnerabilities::RepresentationInformation
+ .find_or_initialize_by(vulnerability_id: vulnerability.id)
+ representation_info.update!(
+ project_id: vulnerability.project_id,
+ resolved_in_commit_sha: vulnerability.findings.first.sha
+ )
+ expect(representation_info.project_id).to eq(vulnerability.project_id)
+ expect(representation_info.resolved_in_commit_sha).to eq(vulnerability.findings.first.sha)
end
end
--
GitLab