From d834cdb39a9bb93bd43f8f5cae70899c35046215 Mon Sep 17 00:00:00 2001 From: Dave Pisek Date: Fri, 16 May 2025 20:29:11 +0200 Subject: [PATCH 1/5] Add new security dashboard behind feature flag This MR adds a new component that will provide the base for building out the new security dashboard. It also adds a feature flag to toggle the rendering of the new dashboard on a group level. --- .../charts/open_vulnerabilities_over_time.vue | 11 +- .../shared/security_dashboard_new.vue | 152 ++++++++++++++++++ .../security_dashboard_init.js | 12 +- .../groups/security/dashboard_controller.rb | 4 + .../wip/group_security_dashboard_new.yml | 10 ++ .../open_vulnerabilities_over_time_spec.js | 6 - .../shared/security_dashboard_new_spec.js | 59 +++++++ ...ty_dashboard_init_integration_spec.js.snap | 93 +++++++++++ ...ecurity_dashboard_init_integration_spec.js | 32 +++- locale/gitlab.pot | 3 + 10 files changed, 371 insertions(+), 11 deletions(-) create mode 100644 ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue create mode 100644 ee/config/feature_flags/wip/group_security_dashboard_new.yml create mode 100644 ee/spec/frontend/security_dashboard/components/shared/security_dashboard_new_spec.js diff --git a/ee/app/assets/javascripts/security_dashboard/components/shared/charts/open_vulnerabilities_over_time.vue b/ee/app/assets/javascripts/security_dashboard/components/shared/charts/open_vulnerabilities_over_time.vue index d39255c01200fd..d75074db553159 100644 --- a/ee/app/assets/javascripts/security_dashboard/components/shared/charts/open_vulnerabilities_over_time.vue +++ b/ee/app/assets/javascripts/security_dashboard/components/shared/charts/open_vulnerabilities_over_time.vue @@ -25,6 +25,14 @@ export default { }, chartOptions() { return { + grid: { + left: '10x', + right: '10px', + bottom: '10px', + top: '10px', + // Setting `containLabel` to `true` ensures the grid area is large enough to contain the labels + containLabel: true, + }, xAxis: { // Setting the `name` to `null` hides the axis name name: null, @@ -37,9 +45,6 @@ export default { type: 'value', minInterval: 1, }, - series: { - smooth: true, - }, ...(this.chartStartDate !== null && { dataZoom: [ { diff --git a/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue b/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue new file mode 100644 index 00000000000000..5e60eca22c5353 --- /dev/null +++ b/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue @@ -0,0 +1,152 @@ + + + diff --git a/ee/app/assets/javascripts/security_dashboard/security_dashboard_init.js b/ee/app/assets/javascripts/security_dashboard/security_dashboard_init.js index 34296b94627d27..ff9cd753472d20 100644 --- a/ee/app/assets/javascripts/security_dashboard/security_dashboard_init.js +++ b/ee/app/assets/javascripts/security_dashboard/security_dashboard_init.js @@ -13,6 +13,7 @@ import groupVulnerabilityHistoryQuery from 'ee/security_dashboard/graphql/querie import instanceVulnerabilityGradesQuery from 'ee/security_dashboard/graphql/queries/instance_vulnerability_grades.query.graphql'; import instanceVulnerabilityHistoryQuery from 'ee/security_dashboard/graphql/queries/instance_vulnerability_history.query.graphql'; import SecurityDashboard from './components/shared/security_dashboard.vue'; +import SecurityDashboardNew from './components/shared/security_dashboard_new.vue'; import ProjectSecurityCharts from './components/project/project_security_dashboard.vue'; import UnavailableState from './components/shared/empty_states/unavailable_state.vue'; import apolloProvider from './graphql/provider'; @@ -58,7 +59,16 @@ export default (el, dashboardType) => { let component; if (dashboardType === DASHBOARD_TYPE_GROUP) { - component = hasProjects ? SecurityDashboard : ReportNotConfiguredGroup; + const isGroupSecurityDashboardNewEnabled = gon.features.groupSecurityDashboardNew; + + if (!hasProjects) { + component = ReportNotConfiguredGroup; + } else if (isGroupSecurityDashboardNewEnabled) { + component = SecurityDashboardNew; + } else { + component = SecurityDashboard; + } + props = { historyQuery: groupVulnerabilityHistoryQuery, gradesQuery: groupVulnerabilityGradesQuery, diff --git a/ee/app/controllers/groups/security/dashboard_controller.rb b/ee/app/controllers/groups/security/dashboard_controller.rb index 53b111e07b8069..688d0794bb8561 100644 --- a/ee/app/controllers/groups/security/dashboard_controller.rb +++ b/ee/app/controllers/groups/security/dashboard_controller.rb @@ -10,6 +10,10 @@ class Groups::Security::DashboardController < Groups::ApplicationController track_internal_event :show, name: 'visit_security_dashboard', category: name, conditions: -> { dashboard_available? } + before_action only: :show do + push_frontend_feature_flag(:group_security_dashboard_new, group) + end + def show render :unavailable unless dashboard_available? end diff --git a/ee/config/feature_flags/wip/group_security_dashboard_new.yml b/ee/config/feature_flags/wip/group_security_dashboard_new.yml new file mode 100644 index 00000000000000..dcff7987da3355 --- /dev/null +++ b/ee/config/feature_flags/wip/group_security_dashboard_new.yml @@ -0,0 +1,10 @@ +--- +name: group_security_dashboard_new +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/542373 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/191974 +rollout_issue_url: +milestone: '18.1' +group: group::security insights +type: wip +default_enabled: false diff --git a/ee/spec/frontend/security_dashboard/components/shared/charts/open_vulnerabilities_over_time_spec.js b/ee/spec/frontend/security_dashboard/components/shared/charts/open_vulnerabilities_over_time_spec.js index c68217e4a38a7b..b3fbb4ea767c35 100644 --- a/ee/spec/frontend/security_dashboard/components/shared/charts/open_vulnerabilities_over_time_spec.js +++ b/ee/spec/frontend/security_dashboard/components/shared/charts/open_vulnerabilities_over_time_spec.js @@ -67,12 +67,6 @@ describe('OpenVulnerabilitiesOverTimeChart', () => { }); }); - it('configures series with smooth lines', () => { - expect(findLineChart().props('option').series).toMatchObject({ - smooth: true, - }); - }); - it('configures dataZoom with the correct start date when chartStartDate is available', () => { expect(findLineChart().props('option').dataZoom[0].startValue).toBe(firstDayOfChartSeries); }); diff --git a/ee/spec/frontend/security_dashboard/components/shared/security_dashboard_new_spec.js b/ee/spec/frontend/security_dashboard/components/shared/security_dashboard_new_spec.js new file mode 100644 index 00000000000000..92b8304e54a1cc --- /dev/null +++ b/ee/spec/frontend/security_dashboard/components/shared/security_dashboard_new_spec.js @@ -0,0 +1,59 @@ +import { shallowMount } from '@vue/test-utils'; +import DashboardLayout from '~/vue_shared/components/customizable_dashboard/dashboard_layout.vue'; +import PanelsBase from '~/vue_shared/components/customizable_dashboard/panels_base.vue'; +import OpenVulnerabilitiesOverTimeChart from 'ee/security_dashboard/components/shared/charts/open_vulnerabilities_over_time.vue'; +import GroupSecurityDashboardV2 from 'ee/security_dashboard/components/shared/security_dashboard_new.vue'; + +describe('Security Dashboard (new version) - Component', () => { + let wrapper; + + const createComponent = (props = {}) => { + wrapper = shallowMount(GroupSecurityDashboardV2, { + propsData: { + ...props, + }, + stubs: { + DashboardLayout, + PanelsBase, + OpenVulnerabilitiesOverTimeChart, + }, + }); + }; + + const findDashboardLayout = () => wrapper.findComponent(DashboardLayout); + + const getDashboardConfig = () => findDashboardLayout().props('config'); + const getFirstPanel = () => getDashboardConfig().panels[0]; + + beforeEach(() => { + createComponent(); + }); + + describe('component rendering', () => { + it('renders the dashboard layout component', () => { + expect(findDashboardLayout().exists()).toBe(true); + }); + + it('passes the correct dashboard configuration to the layout', () => { + const dashboardConfig = getDashboardConfig(); + + expect(dashboardConfig.title).toBe('Security dashboard'); + expect(dashboardConfig.description).toBe( + 'This dashboard provides an overview of your security vulnerabilities.', + ); + }); + + it('renders the panels with correct configuration', () => { + const firstPanel = getFirstPanel(); + + expect(firstPanel.title).toBe('Vulnerabilities over time'); + expect(firstPanel.component).toBe(OpenVulnerabilitiesOverTimeChart); + expect(firstPanel.gridAttributes).toEqual({ + width: 7, + height: 4, + yPos: 0, + xPos: 0, + }); + }); + }); +}); diff --git a/ee/spec/frontend_integration/security_dashboard/__snapshots__/security_dashboard_init_integration_spec.js.snap b/ee/spec/frontend_integration/security_dashboard/__snapshots__/security_dashboard_init_integration_spec.js.snap index 4dac6f402453d7..3f327fc3d8e3bc 100644 --- a/ee/spec/frontend_integration/security_dashboard/__snapshots__/security_dashboard_init_integration_spec.js.snap +++ b/ee/spec/frontend_integration/security_dashboard/__snapshots__/security_dashboard_init_integration_spec.js.snap @@ -112,6 +112,53 @@ exports[`Security Dashboard default states sets up group-level 1`] = ` `; +exports[`Security Dashboard default states sets up group-level with \`groupSecurityDashboardNew\` feature flag set to \`true\` 1`] = ` +
+
+
+
+
+

+ Security dashboard +

+
+
+

+ This dashboard provides an overview of your security vulnerabilities. +

+
+
+
+
+
+
+
+
+
+
+`; + exports[`Security Dashboard default states sets up instance-level 1`] = `
@@ -224,6 +271,52 @@ exports[`Security Dashboard default states sets up instance-level 1`] = `
`; +exports[`Security Dashboard default states sets up project-level 1`] = ` +
+
+
+
+

+ Security dashboard +

+
+
+ Historical view of open vulnerabilities in the default branch. Excludes vulnerabilities that were resolved or dismissed. + + Learn more. + +
+
+
+ +
+
+
+`; + exports[`Security Dashboard error states has unavailable pages 1`] = `
{ document.body.appendChild(root); setWindowLocation(`${TEST_HOST}/-/security/dashboard`); + + // We currently have feature flag logic that needs gon.features to be set + window.gon.features = { + groupSecurityDashboardNew: false, + }; }); afterEach(() => { @@ -48,6 +57,14 @@ describe('Security Dashboard', () => { expect(root).toMatchSnapshot(); }); + it('sets up group-level with `groupSecurityDashboardNew` feature flag set to `true`', () => { + window.gon.features.groupSecurityDashboardNew = true; + + createComponent({ data: { groupFullPath: '/test/' }, type: DASHBOARD_TYPE_GROUP }); + + expect(root).toMatchSnapshot(); + }); + it('sets up instance-level', () => { createComponent({ data: { instanceDashboardSettingsPath: '/instance/settings_page' }, @@ -56,6 +73,19 @@ describe('Security Dashboard', () => { expect(root).toMatchSnapshot(); }); + + it('sets up project-level', () => { + createComponent({ + data: { + projectFullPath: '/test/project', + hasVulnerabilities: 'true', + securityConfigurationPath: '/test/configuration', + }, + type: DASHBOARD_TYPE_PROJECT, + }); + + expect(root).toMatchSnapshot(); + }); }); describe('error states', () => { diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b8453fd93b9c98..2ff10cb03cb3b4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -56825,6 +56825,9 @@ msgstr "" msgid "SecurityReports|There was an error while generating the report." msgstr "" +msgid "SecurityReports|This dashboard provides an overview of your security vulnerabilities." +msgstr "" + msgid "SecurityReports|This project has reached the maximum number of vulnerabilities it can contain and some of the vulnerabilities couldn't be ingested." msgstr "" -- GitLab From 9a1755e2bb7f954a0c7be719b5f7c458cdffdecd Mon Sep 17 00:00:00 2001 From: Dave Pisek Date: Wed, 21 May 2025 17:03:13 +0200 Subject: [PATCH 2/5] Reviewer feedback: add code-comments --- .../components/shared/security_dashboard_new.vue | 2 ++ .../security_dashboard_init_integration_spec.js | 1 + 2 files changed, 3 insertions(+) diff --git a/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue b/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue index 5e60eca22c5353..fc32512a497ccb 100644 --- a/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue +++ b/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue @@ -108,6 +108,7 @@ export default { dashboard: { title: s__('SecurityReports|Security dashboard'), description: s__( + // Note: This is just a placeholder text and will be replaced with the final copy, once it is ready 'SecurityReports|This dashboard provides an overview of your security vulnerabilities.', ), panels: [ @@ -115,6 +116,7 @@ export default { id: '1', title: __('Vulnerabilities over time'), tooltip: { + // Note: This is just a placeholder text and will be replaced with the final copy, once it is ready description: __('Vulnerabilities over time'), }, component: VulnerabilitiesOverTimeChart, diff --git a/ee/spec/frontend_integration/security_dashboard/security_dashboard_init_integration_spec.js b/ee/spec/frontend_integration/security_dashboard/security_dashboard_init_integration_spec.js index 8138daefaf3018..49204c0e2eed12 100644 --- a/ee/spec/frontend_integration/security_dashboard/security_dashboard_init_integration_spec.js +++ b/ee/spec/frontend_integration/security_dashboard/security_dashboard_init_integration_spec.js @@ -27,6 +27,7 @@ describe('Security Dashboard', () => { setWindowLocation(`${TEST_HOST}/-/security/dashboard`); // We currently have feature flag logic that needs gon.features to be set + // It is set to false by default, so the original test's (where there was no feature flag) snapshot is preserved. window.gon.features = { groupSecurityDashboardNew: false, }; -- GitLab From d4a15a03d9f97d931128f9d9d50a5fcadef48bd8 Mon Sep 17 00:00:00 2001 From: Dave Pisek Date: Thu, 22 May 2025 12:59:42 +0200 Subject: [PATCH 3/5] Review feedback: CSS and code-comment --- .../components/shared/charts/open_vulnerabilities_over_time.vue | 2 ++ .../components/shared/security_dashboard_new.vue | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ee/app/assets/javascripts/security_dashboard/components/shared/charts/open_vulnerabilities_over_time.vue b/ee/app/assets/javascripts/security_dashboard/components/shared/charts/open_vulnerabilities_over_time.vue index d75074db553159..43d8bae345766b 100644 --- a/ee/app/assets/javascripts/security_dashboard/components/shared/charts/open_vulnerabilities_over_time.vue +++ b/ee/app/assets/javascripts/security_dashboard/components/shared/charts/open_vulnerabilities_over_time.vue @@ -25,6 +25,8 @@ export default { }, chartOptions() { return { + // Note: This is a workaround to remove the extra whitespace when the chart has no title + // Once https://gitlab.com/gitlab-org/gitlab-ui/-/issues/3191 has been fixed, this can be removed grid: { left: '10x', right: '10px', diff --git a/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue b/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue index fc32512a497ccb..9f37fd8eee3fed 100644 --- a/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue +++ b/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue @@ -144,7 +144,7 @@ export default { -- GitLab From 44a999aba6fb14af5ae39690d0c31916051418d5 Mon Sep 17 00:00:00 2001 From: Dave Pisek Date: Thu, 22 May 2025 15:01:08 +0200 Subject: [PATCH 4/5] Fix failing e2e spec --- .../security_reports_spec.rb | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/qa/qa/specs/features/ee/browser_ui/18_security_risk_management/security_reports_spec.rb b/qa/qa/specs/features/ee/browser_ui/18_security_risk_management/security_reports_spec.rb index 7703d1bf50e30c..59921df8c79ff9 100644 --- a/qa/qa/specs/features/ee/browser_ui/18_security_risk_management/security_reports_spec.rb +++ b/qa/qa/specs/features/ee/browser_ui/18_security_risk_management/security_reports_spec.rb @@ -2,7 +2,7 @@ module QA RSpec.describe 'Security Risk Management', :external_api_calls, product_group: :security_insights do - describe 'Security Reports' do + describe('Security Reports', feature_flag: { name: :group_security_dashboard_new }) do let(:number_of_dependencies_dependency_scanning) { 9 } let(:dependency_scan_example_vuln) { 'Prototype pollution attack in mixin-deep' } let(:container_scan_example_vuln) { 'CVE-2017-18269 in glibc' } @@ -153,13 +153,30 @@ module QA Page::Group::Menu.perform(&:go_to_security_dashboard) - EE::Page::Group::Secure::Show.perform do |dashboard| - Support::Retrier.retry_on_exception( - max_attempts: 2, - reload_page: page, - message: "Retry project security status in security dashboard" - ) do - expect(dashboard).to have_security_status_project_for_severity('F', project) + # Check if group_security_dashboard_new feature flag is enabled + is_new_dashboard = Runtime::Feature.enabled?(:group_security_dashboard_new) + + if is_new_dashboard + # Handle the new dashboard implementation + EE::Page::Group::Secure::Show.perform do |_| + Support::Retrier.retry_on_exception( + max_attempts: 2, + reload_page: page, + message: "Retry project visibility in new security dashboard" + ) do + expect(page).to have_content('Security dashboard') + end + end + else + # Original implementation for when the feature flag is disabled + EE::Page::Group::Secure::Show.perform do |dashboard| + Support::Retrier.retry_on_exception( + max_attempts: 2, + reload_page: page, + message: "Retry project security status in security dashboard" + ) do + expect(dashboard).to have_security_status_project_for_severity('F', project) + end end end -- GitLab From b21887384356733d4598997f2e7d4f09719a0406 Mon Sep 17 00:00:00 2001 From: Dave Pisek Date: Fri, 23 May 2025 09:52:45 +0200 Subject: [PATCH 5/5] Review feedback: check for test-id --- .../components/shared/security_dashboard_new.vue | 2 +- .../security_dashboard_init_integration_spec.js.snap | 4 +++- .../18_security_risk_management/security_reports_spec.rb | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue b/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue index 9f37fd8eee3fed..7668759ad28dca 100644 --- a/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue +++ b/ee/app/assets/javascripts/security_dashboard/components/shared/security_dashboard_new.vue @@ -138,7 +138,7 @@ export default {