Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cloud Security] exclude unknown findings from compliance score calculation #197829

Merged
merged 2 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ import { euiThemeVars } from '@kbn/ui-theme';
export const statusColors = {
passed: euiThemeVars.euiColorSuccess,
failed: euiThemeVars.euiColorVis9,
unknown: euiThemeVars.euiColorLightShade,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';
import { render, screen } from '@testing-library/react';
import { ComplianceScoreBar } from './compliance_score_bar';
import {
COMPLIANCE_SCORE_BAR_UNKNOWN,
COMPLIANCE_SCORE_BAR_PASSED,
COMPLIANCE_SCORE_BAR_FAILED,
} from './test_subjects';

describe('<ComplianceScoreBar />', () => {
it('should display 0% compliance score with status unknown when both passed and failed are 0', () => {
render(<ComplianceScoreBar totalPassed={0} totalFailed={0} />);
expect(screen.getByText('0%')).toBeInTheDocument();
expect(screen.queryByTestId(COMPLIANCE_SCORE_BAR_UNKNOWN)).not.toBeNull();
expect(screen.queryByTestId(COMPLIANCE_SCORE_BAR_FAILED)).toBeNull();
expect(screen.queryByTestId(COMPLIANCE_SCORE_BAR_PASSED)).toBeNull();
});

it('should display 100% compliance score when passed is greater than 0 and failed is 0', () => {
render(<ComplianceScoreBar totalPassed={10} totalFailed={0} />);
expect(screen.getByText('100%')).toBeInTheDocument();
expect(screen.queryByTestId(COMPLIANCE_SCORE_BAR_PASSED)).not.toBeNull();
expect(screen.queryByTestId(COMPLIANCE_SCORE_BAR_FAILED)).toBeNull();
expect(screen.queryByTestId(COMPLIANCE_SCORE_BAR_UNKNOWN)).toBeNull();
});

it('should display 0% compliance score when passed is 0 and failed is greater than 0', () => {
render(<ComplianceScoreBar totalPassed={0} totalFailed={10} />);
expect(screen.getByText('0%')).toBeInTheDocument();
expect(screen.queryByTestId(COMPLIANCE_SCORE_BAR_FAILED)).not.toBeNull();
expect(screen.queryByTestId(COMPLIANCE_SCORE_BAR_PASSED)).toBeNull();
expect(screen.queryByTestId(COMPLIANCE_SCORE_BAR_UNKNOWN)).toBeNull();
});

it('should display 50% compliance score when passed is equal to failed', () => {
render(<ComplianceScoreBar totalPassed={5} totalFailed={5} />);
expect(screen.getByText('50%')).toBeInTheDocument();
expect(screen.queryByTestId(COMPLIANCE_SCORE_BAR_FAILED)).not.toBeNull();
expect(screen.queryByTestId(COMPLIANCE_SCORE_BAR_PASSED)).not.toBeNull();
expect(screen.queryByTestId(COMPLIANCE_SCORE_BAR_UNKNOWN)).toBeNull();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import { i18n } from '@kbn/i18n';
import React from 'react';
import { statusColors } from '@kbn/cloud-security-posture';
import { calculatePostureScore } from '../../common/utils/helpers';
import { CSP_FINDINGS_COMPLIANCE_SCORE } from './test_subjects';
import {
CSP_FINDINGS_COMPLIANCE_SCORE,
COMPLIANCE_SCORE_BAR_UNKNOWN,
COMPLIANCE_SCORE_BAR_FAILED,
COMPLIANCE_SCORE_BAR_PASSED,
} from './test_subjects';

/**
* This component will take 100% of the width set by the parent
Expand Down Expand Up @@ -59,12 +64,22 @@ export const ComplianceScoreBar = ({
gap: 1px;
`}
>
{!totalPassed && !totalFailed && (
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can only happen when there are unknown findings, for now when there are only unknown findings we represent it as 0% score with gray color bar chart

<EuiFlexItem
css={css`
flex: 1;
background: ${statusColors.unknown};
`}
data-test-subj={COMPLIANCE_SCORE_BAR_UNKNOWN}
/>
)}
{!!totalPassed && (
<EuiFlexItem
css={css`
flex: ${totalPassed};
background: ${statusColors.passed};
`}
data-test-subj={COMPLIANCE_SCORE_BAR_PASSED}
/>
)}
{!!totalFailed && (
Expand All @@ -73,6 +88,7 @@ export const ComplianceScoreBar = ({
flex: ${totalFailed};
background: ${statusColors.failed};
`}
data-test-subj={COMPLIANCE_SCORE_BAR_FAILED}
/>
)}
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,7 @@ export const CIS_GCP_INPUT_FIELDS_TEST_SUBJECTS = {
};

export const SUBSCRIPTION_NOT_ALLOWED_TEST_SUBJECT = 'cloud_posture_page_subscription_not_allowed';

export const COMPLIANCE_SCORE_BAR_UNKNOWN = 'complianceScoreBarUnknown';
export const COMPLIANCE_SCORE_BAR_FAILED = 'complianceScoreBarFailed';
export const COMPLIANCE_SCORE_BAR_PASSED = 'complianceScoreBarPassed';
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';
import { render } from '@testing-library/react';
import { useEuiTheme } from '@elastic/eui';
import { ComplianceBarComponent } from './latest_findings_group_renderer';
import { RawBucket } from '@kbn/grouping/src';
import { FindingsGroupingAggregation } from './use_grouped_findings';
import { ComplianceScoreBar } from '../../../components/compliance_score_bar';

jest.mock('@elastic/eui', () => {
const actual = jest.requireActual('@elastic/eui');
return {
...actual,
useEuiTheme: jest.fn(),
};
});

jest.mock('../../../components/compliance_score_bar', () => ({
ComplianceScoreBar: jest.fn(() => null),
}));

jest.mock('../../../components/cloud_security_grouping');

describe('<ComplianceBarComponent />', () => {
beforeEach(() => {
(useEuiTheme as jest.Mock).mockReturnValue({ euiTheme: { size: { s: 's' } } });
(ComplianceScoreBar as jest.Mock).mockClear();
});

it('renders ComplianceScoreBar with correct totalFailed and totalPassed, when total = failed+passed', () => {
const bucket = {
doc_count: 10,
failedFindings: {
doc_count: 4,
},
passedFindings: {
doc_count: 6,
},
} as RawBucket<FindingsGroupingAggregation>;

render(<ComplianceBarComponent bucket={bucket} />);

expect(ComplianceScoreBar).toHaveBeenCalledWith(
expect.objectContaining({
totalFailed: 4,
totalPassed: 6,
}),
{}
);
});

it('renders ComplianceScoreBar with correct totalFailed and totalPassed, when there are unknown findings', () => {
const bucket = {
doc_count: 10,
failedFindings: {
doc_count: 3,
},
passedFindings: {
doc_count: 6,
},
} as RawBucket<FindingsGroupingAggregation>;

render(<ComplianceBarComponent bucket={bucket} />);

expect(ComplianceScoreBar).toHaveBeenCalledWith(
expect.objectContaining({
totalFailed: 3,
totalPassed: 6,
}),
{}
);
});

it('renders ComplianceScoreBar with correct totalFailed and totalPassed, when there are no findings', () => {
const bucket = {
doc_count: 10,
failedFindings: {
doc_count: 0,
},
passedFindings: {
doc_count: 0,
},
} as RawBucket<FindingsGroupingAggregation>;

render(<ComplianceBarComponent bucket={bucket} />);

expect(ComplianceScoreBar).toHaveBeenCalledWith(
expect.objectContaining({
totalFailed: 0,
totalPassed: 0,
}),
{}
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,15 @@ const FindingsCountComponent = ({ bucket }: { bucket: RawBucket<FindingsGrouping

const FindingsCount = React.memo(FindingsCountComponent);

const ComplianceBarComponent = ({ bucket }: { bucket: RawBucket<FindingsGroupingAggregation> }) => {
export const ComplianceBarComponent = ({
bucket,
}: {
bucket: RawBucket<FindingsGroupingAggregation>;
}) => {
const { euiTheme } = useEuiTheme();

const totalFailed = bucket.failedFindings?.doc_count || 0;
const totalPassed = bucket.doc_count - totalFailed;
const totalPassed = bucket.passedFindings?.doc_count || 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the main fix, with introducing unknown status we can't assume anymore that total = passed + failed

return (
<ComplianceScoreBar
size="l"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,12 @@ import { EuiFlexGroup, EuiFlexItem, EuiSpacer, EuiText, useEuiTheme, EuiTitle }
import { FormattedMessage } from '@kbn/i18n-react';
import { DistributionBar } from '@kbn/security-solution-distribution-bar';
import { useMisconfigurationPreview } from '@kbn/cloud-security-posture/src/hooks/use_misconfiguration_preview';
import { euiThemeVars } from '@kbn/ui-theme';
import { i18n } from '@kbn/i18n';
import { ExpandablePanel } from '@kbn/security-solution-common';
import { buildEntityFlyoutPreviewQuery } from '@kbn/cloud-security-posture-common';
import { useExpandableFlyoutApi } from '@kbn/expandable-flyout';
import { useVulnerabilitiesPreview } from '@kbn/cloud-security-posture/src/hooks/use_vulnerabilities_preview';
import { hasVulnerabilitiesData } from '@kbn/cloud-security-posture';
import { hasVulnerabilitiesData, statusColors } from '@kbn/cloud-security-posture';
import { METRIC_TYPE } from '@kbn/analytics';
import {
ENTITY_FLYOUT_WITH_MISCONFIGURATION_VISIT,
Expand Down Expand Up @@ -51,7 +50,7 @@ export const getFindingsStats = (passedFindingsStats: number, failedFindingsStat
}
),
count: passedFindingsStats,
color: euiThemeVars.euiColorSuccess,
color: statusColors.passed,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no color change, just reusing existing contants

},
{
key: i18n.translate(
Expand All @@ -61,7 +60,7 @@ export const getFindingsStats = (passedFindingsStats: number, failedFindingsStat
}
),
count: failedFindingsStats,
color: euiThemeVars.euiColorVis9,
color: statusColors.failed,
},
];
};
Expand All @@ -70,14 +69,10 @@ const MisconfigurationPreviewScore = ({
passedFindings,
failedFindings,
euiTheme,
numberOfPassedFindings,
numberOfFailedFindings,
}: {
passedFindings: number;
failedFindings: number;
euiTheme: EuiThemeComputed<{}>;
numberOfPassedFindings?: number;
numberOfFailedFindings?: number;
}) => {
return (
<EuiFlexItem>
Expand Down