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.

Sign up for GitHub

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] Filters for Contextual Flyout Datagrid #201708

Merged
merged 12 commits into from
Dec 2, 2024
Merged

[Cloud Security] Filters for Contextual Flyout Datagrid #201708

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ddd0636
initial push - added filters functionality and state for Vulnerabilit…
animehart Nov 26, 2024
0f7efea
added filter for Alerts
animehart Nov 27, 2024
cb51053
added new jest, clean up
animehart Nov 27, 2024
95ba1bd
updated jest
animehart Nov 27, 2024
f61c6ba
Merge branch 'main' into contextual-flyout-datagrid-filters
animehart Nov 27, 2024
ab8dace
Merge branch 'main' into contextual-flyout-datagrid-filters
animehart Nov 27, 2024
50c058a
pr comments
animehart Nov 28, 2024
7d3efa9
Merge branch 'contextual-flyout-datagrid-filters' of github.com:anime…
animehart Nov 28, 2024
823c3b4
forgot to add these 2
animehart Nov 28, 2024
395d839
added status as constants
animehart Nov 29, 2024
ccf30c5
more pr comments
animehart Nov 29, 2024
def34d8
Merge branch 'main' into contextual-flyout-datagrid-filters
animehart Dec 2, 2024
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
99 changes: 88 additions & 11 deletions x-pack/packages/kbn-cloud-security-posture/common/utils/helpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,61 @@ describe('test helper methods', () => {
expect(buildEntityFlyoutPreviewQuery(field, query)).toEqual(expectedQuery);
});

it('should return the correct query when given field and empty query', () => {
it('should return the correct query when given field, query, status and queryType Misconfiguration', () => {
const field = 'host.name';
const query = 'exampleHost';
const status = 'pass';
const queryType = 'Misconfiguration';
const expectedQuery = {
bool: {
filter: [
{
bool: {
should: [{ term: { 'host.name': 'exampleHost' } }],
minimum_should_match: 1,
},
},
{
bool: {
should: [{ term: { 'result.evaluation': 'pass' } }],
minimum_should_match: 1,
},
},
],
},
};

expect(buildEntityFlyoutPreviewQuery(field, query, status, queryType)).toEqual(expectedQuery);
});

it('should return the correct query when given field, query, status and queryType Vulnerability', () => {
const field = 'host.name';
const query = 'exampleHost';
const status = 'low';
const queryType = 'Vulnerability';
const expectedQuery = {
bool: {
filter: [
{
bool: {
should: [{ term: { 'host.name': 'exampleHost' } }],
minimum_should_match: 1,
},
},
{
bool: {
should: [{ term: { 'vulnerability.severity': 'low' } }],
minimum_should_match: 1,
},
},
],
},
};

expect(buildEntityFlyoutPreviewQuery(field, query, status, queryType)).toEqual(expectedQuery);
});

it('should return the correct query when given field and empty query and empty status', () => {
const field = 'host.name';
const expectedQuery = {
bool: {
Expand All @@ -185,7 +239,7 @@ describe('test helper methods', () => {
});

describe('buildEntityAlertsQuery', () => {
const getExpectedAlertsQuery = (size?: number) => {
const getExpectedAlertsQuery = (size?: number, severity?: string) => {
return {
size: size || 0,
_source: false,
seanrathier marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -202,20 +256,30 @@ describe('test helper methods', () => {
filter: [
{
bool: {
must: [],
filter: [
should: [
{
match_phrase: {
'host.name': {
query: 'exampleHost',
},
term: {
'host.name': 'exampleHost',
},
},
],
should: [],
must_not: [],
minimum_should_match: 1,
},
},
severity
? {
bool: {
should: [
{
term: {
'kibana.alert.severity': 'low',
},
},
],
minimum_should_match: 1,
},
}
: undefined,
{
range: {
'@timestamp': {
Expand All @@ -229,7 +293,7 @@ describe('test helper methods', () => {
'kibana.alert.workflow_status': ['open', 'acknowledged'],
},
},
],
].filter(Boolean),
},
},
};
Expand All @@ -256,5 +320,18 @@ describe('test helper methods', () => {

expect(buildEntityAlertsQuery(field, to, from, query)).toEqual(getExpectedAlertsQuery(size));
});

it('should return the correct query when given severity query', () => {
const field = 'host.name';
const query = 'exampleHost';
const to = 'Tomorrow';
const from = 'Today';
const size = undefined;
const severity = 'low';

expect(buildEntityAlertsQuery(field, to, from, query, size, severity)).toEqual(
getExpectedAlertsQuery(size, 'low')
);
});
});
});
56 changes: 44 additions & 12 deletions x-pack/packages/kbn-cloud-security-posture/common/utils/helpers.ts
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

:opinion

We should consider renaming this file to something like queries.ts because all exported functions are related to building queries.

This is a little pet peeve of mine when directories are called utils and files helpers. I've seen this in many cases over time files like this become a kitchen sink of common functions and it becomes hard to reorganize the structure later. This will be especially hard to reorganize if other plugins start to use our shared package.

x-pack/packages/kbn-cloud-security-posture/common/queries.ts

Since this is a shared package everything is by definition common so another suggestion is to remove comment altogether

x-pack/packages/kbn-cloud-security-posture/queries.ts

wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmmm you are right that almost all of the helpers functions here are query related and I can see the issue that this might have later in the future

I'm down to do this change but maybe in separate ticket/PR (as this PR is for Filters for the Datagrid, and this suggested change might touch other features thats not related at all with Contextual Flyout)

wdyt ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes please and thank you! Can you open that issue as a tech debt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add the link to the tech debt issue here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#202242

Lemme know if i missed anything :)

Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,14 @@ export const buildMutedRulesFilter = (
return mutedRulesFilterQuery;
};

export const buildEntityFlyoutPreviewQuery = (field: string, queryValue?: string) => {
export const buildEntityFlyoutPreviewQuery = (
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we could create some High Order functions instead:

// Generic function
export const buildGenericEntityFlyoutPreviewQuery = (
  field: string,
  queryValue?: string,
  status?: string,
  queryField?: string
) => {
  return {
    bool: {
      filter: [
        {
          bool: {
            should: [
              {
                term: {
                  [field]: `${queryValue || ''}`,
                },
              },
            ],
            minimum_should_match: 1,
          },
        },
        status && queryField
          ? {
              bool: {
                should: [
                  {
                    term: {
                      [queryField]: status,
                    },
                  },
                ],
                minimum_should_match: 1,
              },
            }
          : undefined,
      ].filter(Boolean),
    },
  };
};

// Higher-order function for Misconfiguration
export const buildMisconfigurationEntityFlyoutPreviewQuery = (
  field: string,
  queryValue?: string,
  status?: string
) => {
  const queryField = 'result.evaluation';
  return buildGenericEntityFlyoutPreviewQuery(field, queryValue, status, queryField);
};

// Higher-order function for Vulnerability
export const buildVulnerabilityEntityFlyoutPreviewQuery = (
  field: string,
  queryValue?: string,
  status?: string
) => {
  const queryField = 'vulnerability.severity';
  return buildGenericEntityFlyoutPreviewQuery(field, queryValue, status, queryField);
};

field: string,
queryValue?: string,
status?: string,
queryType?: 'Misconfiguration' | 'Vulnerability'
) => {
const queryField =
queryType === 'Misconfiguration' ? 'result.evaluation' : 'vulnerability.severity';
return {
bool: {
filter: [
Expand All @@ -59,7 +66,21 @@ export const buildEntityFlyoutPreviewQuery = (field: string, queryValue?: string
minimum_should_match: 1,
},
},
],
status && queryType
? {
bool: {
should: [
{
term: {
[queryField]: status,
},
},
],
minimum_should_match: 1,
},
}
: undefined,
].filter(Boolean),
},
};
};
Expand All @@ -69,7 +90,8 @@ export const buildEntityAlertsQuery = (
to: string,
from: string,
queryValue?: string,
size?: number
size?: number,
severity?: string
) => {
return {
size: size || 0,
Expand All @@ -87,20 +109,30 @@ export const buildEntityAlertsQuery = (
filter: [
{
bool: {
must: [],
filter: [
should: [
{
match_phrase: {
[field]: {
query: queryValue,
},
term: {
[field]: `${queryValue || ''}`,
},
},
],
should: [],
must_not: [],
minimum_should_match: 1,
},
},
severity
? {
bool: {
should: [
{
term: {
'kibana.alert.severity': severity,
},
},
],
minimum_should_match: 1,
},
}
: undefined,
{
range: {
'@timestamp': {
Expand All @@ -114,7 +146,7 @@ export const buildEntityAlertsQuery = (
'kibana.alert.workflow_status': ['open', 'acknowledged'],
},
},
],
].filter(Boolean),
},
},
};
Expand Down
3 changes: 3 additions & 0 deletions x-pack/packages/kbn-cloud-security-posture/public/src/constants/component_constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,6 @@ export const statusColors = {
failed: euiThemeVars.euiColorVis9,
unknown: euiThemeVars.euiColorLightShade,
};

export const HOST_NAME = 'host.name';
export const USER_NAME = 'user.name';
2 changes: 1 addition & 1 deletion x-pack/packages/kbn-cloud-security-posture/public/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export interface CspClientPluginStartDeps {
export interface CspBaseEsQuery {
query?: {
bool: {
filter: estypes.QueryDslQueryContainer[];
filter: Array<estypes.QueryDslQueryContainer | undefined> | undefined;
};
};
}
Expand Down
47 changes: 46 additions & 1 deletion x-pack/packages/kbn-cloud-security-posture/public/src/utils/vulnerability_helpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,20 @@ import { getVulnerabilityStats } from './vulnerability_helpers';
import { i18n } from '@kbn/i18n';

describe('getVulnerabilitiesAggregationCount', () => {
const mockFilterFunction = jest.fn();
it('should return empty array when all severity count is 0', () => {
const result = getVulnerabilityStats({ critical: 0, high: 0, medium: 0, low: 0, none: 0 });
expect(result).toEqual([]);
});

it('should return stats for low, medium, high, and critical vulnerabilities', () => {
const result = getVulnerabilityStats({ critical: 1, high: 2, medium: 3, low: 4, none: 5 });
const resultWithoutFunctions = result.map((item) => {
const { onClick, onClickReset, ...rest } = item;
return rest;
});

expect(result).toEqual([
expect(resultWithoutFunctions).toEqual([
{
key: i18n.translate(
'xpack.securitySolution.flyout.right.insights.vulnerabilities.noneVulnerabilitiesText',
Expand All @@ -28,6 +33,7 @@ describe('getVulnerabilitiesAggregationCount', () => {
),
count: 5,
color: '#aaa',
isCurrentFilter: false,
},
{
key: i18n.translate(
Expand All @@ -38,6 +44,7 @@ describe('getVulnerabilitiesAggregationCount', () => {
),
count: 4,
color: euiThemeVars.euiColorVis0,
isCurrentFilter: false,
},
{
key: i18n.translate(
Expand All @@ -48,6 +55,7 @@ describe('getVulnerabilitiesAggregationCount', () => {
),
count: 3,
color: euiThemeVars.euiColorVis5_behindText,
isCurrentFilter: false,
},
{
key: i18n.translate(
Expand All @@ -58,6 +66,7 @@ describe('getVulnerabilitiesAggregationCount', () => {
),
count: 2,
color: euiThemeVars.euiColorVis9_behindText,
isCurrentFilter: false,
},
{
key: i18n.translate(
Expand All @@ -68,7 +77,43 @@ describe('getVulnerabilitiesAggregationCount', () => {
),
count: 1,
color: euiThemeVars.euiColorDanger,
isCurrentFilter: false,
},
]);
});

it('should return correct stats with correct onClick functions', () => {
const result = getVulnerabilityStats(
{ critical: 1, high: 2, medium: 3, low: 4, none: 5 },
mockFilterFunction
);
const event = { stopPropagation: jest.fn() } as unknown as React.MouseEvent<
SVGElement,
MouseEvent
>;
result[1].onClick?.();
expect(mockFilterFunction).toHaveBeenCalledWith('LOW');
result[1].onClickReset?.(event);
expect(mockFilterFunction).toHaveBeenCalledWith('');
});

it('should identify correct currentFilter', () => {
const currentFilter = 'LOW';
const result = getVulnerabilityStats(
{ critical: 1, high: 2, medium: 3, low: 4, none: 5 },
mockFilterFunction,
currentFilter
);

// Make sure that Low is set to Current Filter
expect(result[1].isCurrentFilter).toBe(true);
expect(result[1].key).toBe('Low');

// Make sure only Low is set as Current Filter and no other severity
result.forEach((item) => {
if (item.key !== 'Low') {
expect(item.isCurrentFilter).toBe(false);
}
});
});
});
Loading