Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
62 changes: 62 additions & 0 deletions x-pack/plugins/cloud_security_posture/common/schemas/benchmark.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* 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 { type TypeOf, schema } from '@kbn/config-schema';

export const DEFAULT_BENCHMARKS_PER_PAGE = 20;
export const BENCHMARK_PACKAGE_POLICY_PREFIX = 'package_policy.';
export const benchmarksInputSchema = schema.object({
/**
* The page of objects to return
*/
page: schema.number({ defaultValue: 1, min: 1 }),
/**
* The number of objects to include in each page
*/
per_page: schema.number({ defaultValue: DEFAULT_BENCHMARKS_PER_PAGE, min: 0 }),
/**
* Once of PackagePolicy fields for sorting the found objects.
* Sortable fields:
* - package_policy.id
* - package_policy.name
* - package_policy.policy_id
* - package_policy.namespace
* - package_policy.updated_at
* - package_policy.updated_by
* - package_policy.created_at
* - package_policy.created_by,
* - package_policy.package.name
* - package_policy.package.title
* - package_policy.package.version
*/
sort_field: schema.oneOf(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think we support every field in PackagePolicy (as we don't own it anyway)

and maybe we should just take what's currently applicable by the UI. as in - just the column headers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes we technically support every field in that object type, but better be explicit in the docs and schema. Regarding using only what's in the UI - IMO its better to leave the API more flexible for the future

[
schema.literal('package_policy.id'),
schema.literal('package_policy.name'),
schema.literal('package_policy.policy_id'),
schema.literal('package_policy.namespace'),
schema.literal('package_policy.updated_at'),
schema.literal('package_policy.updated_by'),
schema.literal('package_policy.created_at'),
schema.literal('package_policy.created_by'),
schema.literal('package_policy.package.name'),
schema.literal('package_policy.package.title'),
],
{ defaultValue: 'package_policy.name' }
),
/**
* The order to sort by
*/
sort_order: schema.oneOf([schema.literal('asc'), schema.literal('desc')], {
defaultValue: 'asc',
}),
/**
* Benchmark filter
*/
benchmark_name: schema.maybe(schema.string()),
});

export type BenchmarksQuerySchema = TypeOf<typeof benchmarksInputSchema>;
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
* 2.0.
*/
import React from 'react';
import { render, screen } from '@testing-library/react';
import { render, screen, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { UseQueryResult } from 'react-query/types/react/types';
import { createStubDataView } from '../../../../../../src/plugins/data_views/public/data_views/data_view.stub';
import { CSP_KUBEBEAT_INDEX_PATTERN } from '../../../common/constants';
Expand All @@ -14,7 +15,11 @@ import { createCspBenchmarkIntegrationFixture } from '../../test/fixtures/csp_be
import { createReactQueryResponse } from '../../test/fixtures/react_query';
import { TestProvider } from '../../test/test_provider';
import { Benchmarks, BENCHMARKS_TABLE_DATA_TEST_SUBJ } from './benchmarks';
import { ADD_A_CIS_INTEGRATION, BENCHMARK_INTEGRATIONS } from './translations';
import {
ADD_A_CIS_INTEGRATION,
BENCHMARK_INTEGRATIONS,
TABLE_COLUMN_HEADERS,
} from './translations';
import { useCspBenchmarkIntegrations } from './use_csp_benchmark_integrations';

jest.mock('./use_csp_benchmark_integrations');
Expand Down Expand Up @@ -77,4 +82,68 @@ describe('<Benchmarks />', () => {

expect(screen.getByTestId(BENCHMARKS_TABLE_DATA_TEST_SUBJ)).toBeInTheDocument();
});

it('supports sorting the table by integrations', () => {
renderBenchmarks(
createReactQueryResponse({
status: 'success',
data: [createCspBenchmarkIntegrationFixture()],
})
);

// The table is sorted by integrations ascending by default, asserting that
const sortedHeaderAscending = screen
.getAllByRole('columnheader')
.find((element) => element.getAttribute('aria-sort') === 'ascending');

expect(sortedHeaderAscending).toBeInTheDocument();
expect(
within(sortedHeaderAscending!).getByText(TABLE_COLUMN_HEADERS.INTEGRATION)
).toBeInTheDocument();

// A click should now sort it by descending
userEvent.click(screen.getByText(TABLE_COLUMN_HEADERS.INTEGRATION));

const sortedHeaderDescending = screen
.getAllByRole('columnheader')
.find((element) => element.getAttribute('aria-sort') === 'descending');
expect(sortedHeaderDescending).toBeInTheDocument();
expect(
within(sortedHeaderDescending!).getByText(TABLE_COLUMN_HEADERS.INTEGRATION)
).toBeInTheDocument();
});

it('supports sorting the table by integration type, created by, and created at columns', () => {
renderBenchmarks(
createReactQueryResponse({
status: 'success',
data: [createCspBenchmarkIntegrationFixture()],
})
);

[
TABLE_COLUMN_HEADERS.INTEGRATION_TYPE,
TABLE_COLUMN_HEADERS.CREATED_AT,
TABLE_COLUMN_HEADERS.CREATED_AT,
].forEach((columnHeader) => {
const headerTextElement = screen.getByText(columnHeader);
expect(headerTextElement).toBeInTheDocument();

// Click on the header element to sort the column in ascending order
userEvent.click(headerTextElement!);

const sortedHeaderAscending = screen
.getAllByRole('columnheader')
.find((element) => element.getAttribute('aria-sort') === 'ascending');
expect(within(sortedHeaderAscending!).getByText(columnHeader)).toBeInTheDocument();

// Click on the header element again to sort the column in descending order
userEvent.click(headerTextElement!);

const sortedHeaderDescending = screen
.getAllByRole('columnheader')
.find((element) => element.getAttribute('aria-sort') === 'descending');
expect(within(sortedHeaderDescending!).getByText(columnHeader)).toBeInTheDocument();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ import { useCISIntegrationLink } from '../../common/navigation/use_navigate_to_c
import { CspPageTemplate } from '../../components/page_template';
import { BenchmarksTable } from './benchmarks_table';
import { ADD_A_CIS_INTEGRATION, BENCHMARK_INTEGRATIONS } from './translations';
import { useCspBenchmarkIntegrations } from './use_csp_benchmark_integrations';
import {
useCspBenchmarkIntegrations,
UseCspBenchmarkIntegrationsProps,
} from './use_csp_benchmark_integrations';
import { extractErrorMessage } from '../../../common/utils/helpers';
import { SEARCH_PLACEHOLDER } from './translations';

Expand Down Expand Up @@ -118,7 +121,13 @@ const PAGE_HEADER: EuiPageHeaderProps = {
};

export const Benchmarks = () => {
const [query, setQuery] = useState({ name: '', page: 1, perPage: 5 });
const [query, setQuery] = useState<UseCspBenchmarkIntegrationsProps>({
name: '',
page: 1,
perPage: 5,
sortField: 'package_policy.name',
sortOrder: 'asc',
});

const queryResult = useCspBenchmarkIntegrations(query);

Expand All @@ -129,7 +138,7 @@ export const Benchmarks = () => {
return (
<CspPageTemplate pageHeader={PAGE_HEADER}>
<BenchmarkSearchField
isLoading={queryResult.isLoading}
isLoading={queryResult.isFetching}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is this preferred over isLoading ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When sorting the data, react-query dispatches an additional query, but isLoading remains false (it only applies to initial loading of data), and we do want the table and search field to present the loading state

onSearch={(name) => setQuery((current) => ({ ...current, name }))}
/>
<EuiSpacer />
Expand All @@ -142,13 +151,25 @@ export const Benchmarks = () => {
benchmarks={queryResult.data?.items || []}
data-test-subj={BENCHMARKS_TABLE_DATA_TEST_SUBJ}
error={queryResult.error ? extractErrorMessage(queryResult.error) : undefined}
loading={queryResult.isLoading}
loading={queryResult.isFetching}
pageIndex={query.page}
pageSize={query.perPage}
sorting={{
// @ts-expect-error - EUI types currently do not support sorting by nested fields
sort: { field: query.sortField, direction: query.sortOrder },
allowNeutralSort: false,
}}
totalItemCount={totalItemCount}
setQuery={({ page }) =>
setQuery((current) => ({ ...current, page: page.index, perPage: page.size }))
}
setQuery={({ page, sort }) => {
setQuery((current) => ({
...current,
page: page.index,
perPage: page.size,
sortField:
(sort?.field as UseCspBenchmarkIntegrationsProps['sortField']) || current.sortField,
sortOrder: sort?.direction || current.sortOrder,
}));
}}
noItemsMessage={
queryResult.isSuccess && !queryResult.data.total ? (
<BenchmarkEmptyState name={query.name} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { useKibana } from '../../common/hooks/use_kibana';
import { allNavigationItems } from '../../common/navigation/constants';

interface BenchmarksTableProps
extends Pick<EuiBasicTableProps<Benchmark>, 'loading' | 'error' | 'noItemsMessage'>,
extends Pick<EuiBasicTableProps<Benchmark>, 'loading' | 'error' | 'noItemsMessage' | 'sorting'>,
Pagination {
benchmarks: Benchmark[];
setQuery(pagination: CriteriaWithPagination<Benchmark>): void;
Expand Down Expand Up @@ -66,12 +66,14 @@ const BENCHMARKS_TABLE_COLUMNS: Array<EuiBasicTableColumn<Benchmark>> = [
</Link>
),
truncateText: true,
sortable: true,
},
{
field: 'package_policy.package.title',
name: TABLE_COLUMN_HEADERS.INTEGRATION_TYPE,
dataType: 'string',
truncateText: true,
sortable: true,
},
{
field: 'agent_policy.name',
Expand All @@ -91,13 +93,15 @@ const BENCHMARKS_TABLE_COLUMNS: Array<EuiBasicTableColumn<Benchmark>> = [
name: TABLE_COLUMN_HEADERS.CREATED_BY,
dataType: 'string',
truncateText: true,
sortable: true,
},
{
field: 'package_policy.created_at',
name: TABLE_COLUMN_HEADERS.CREATED_AT,
dataType: 'date',
truncateText: true,
render: (date: Benchmark['package_policy']['created_at']) => moment(date).fromNow(),
sortable: true,
},
{
field: 'package_policy.rules', // TODO: add fields
Expand All @@ -117,6 +121,7 @@ export const BenchmarksTable = ({
error,
setQuery,
noItemsMessage,
sorting,
...rest
}: BenchmarksTableProps) => {
const history = useHistory();
Expand All @@ -137,9 +142,8 @@ export const BenchmarksTable = ({
totalItemCount,
};

const onChange = ({ page }: CriteriaWithPagination<Benchmark>) => {
if (!page) return;
setQuery({ page: { ...page, index: page.index + 1 } });
const onChange = ({ page, sort }: CriteriaWithPagination<Benchmark>) => {
setQuery({ page: { ...page, index: page.index + 1 }, sort });
};

return (
Expand All @@ -155,6 +159,7 @@ export const BenchmarksTable = ({
loading={loading}
noItemsMessage={noItemsMessage}
error={error}
sorting={sorting}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,39 @@
import { useQuery } from 'react-query';
import type { ListResult } from '../../../../fleet/common';
import { BENCHMARKS_ROUTE_PATH } from '../../../common/constants';
import { BenchmarksQuerySchema } from '../../../common/schemas/benchmark';
import { useKibana } from '../../common/hooks/use_kibana';
import type { Benchmark } from '../../../common/types';

const QUERY_KEY = 'csp_benchmark_integrations';

interface Props {
export interface UseCspBenchmarkIntegrationsProps {
name: string;
page: number;
perPage: number;
sortField: BenchmarksQuerySchema['sort_field'];
sortOrder: BenchmarksQuerySchema['sort_order'];
}

export const useCspBenchmarkIntegrations = ({ name, perPage, page }: Props) => {
export const useCspBenchmarkIntegrations = ({
name,
perPage,
page,
sortField,
sortOrder,
}: UseCspBenchmarkIntegrationsProps) => {
const { http } = useKibana().services;
return useQuery([QUERY_KEY, { name, perPage, page }], () =>
http.get<ListResult<Benchmark>>(BENCHMARKS_ROUTE_PATH, {
query: {
benchmark_name: name,
per_page: perPage,
page,
sort_field: 'name',
sort_order: 'asc',
},
})
const query: BenchmarksQuerySchema = {
benchmark_name: name,
per_page: perPage,
page,
sort_field: sortField,
sort_order: sortOrder,
};

return useQuery(
[QUERY_KEY, query],
() => http.get<ListResult<Benchmark>>(BENCHMARKS_ROUTE_PATH, { query }),
{ keepPreviousData: true }
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ import {
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { KibanaRequest } from 'src/core/server/http/router/request';
import {
defineGetBenchmarksRoute,
benchmarksInputSchema,
DEFAULT_BENCHMARKS_PER_PAGE,
} from '../../../common/schemas/benchmark';
import {
defineGetBenchmarksRoute,
PACKAGE_POLICY_SAVED_OBJECT_TYPE,
getPackagePolicies,
getAgentPolicies,
Expand Down Expand Up @@ -84,7 +86,7 @@ describe('benchmarks API', () => {
};
defineGetBenchmarksRoute(router, cspContext);

const [config, _] = router.get.mock.calls[0];
const [config] = router.get.mock.calls[0];

expect(config.path).toEqual('/api/csp/benchmarks');
});
Expand Down Expand Up @@ -180,7 +182,7 @@ describe('benchmarks API', () => {

it('should not throw when sort_field is a string', async () => {
expect(() => {
benchmarksInputSchema.validate({ sort_field: 'name' });
benchmarksInputSchema.validate({ sort_field: 'package_policy.name' });
}).not.toThrow();
});

Expand All @@ -204,7 +206,7 @@ describe('benchmarks API', () => {

it('should not throw when fields is a known string literal', async () => {
expect(() => {
benchmarksInputSchema.validate({ sort_field: 'name' });
benchmarksInputSchema.validate({ sort_field: 'package_policy.name' });
}).not.toThrow();
});

Expand Down Expand Up @@ -240,7 +242,7 @@ describe('benchmarks API', () => {
await getPackagePolicies(mockSoClient, mockAgentPolicyService, 'myPackage', {
page: 1,
per_page: 100,
sort_field: 'name',
sort_field: 'package_policy.name',
sort_order: 'desc',
});

Expand All @@ -261,7 +263,7 @@ describe('benchmarks API', () => {
await getPackagePolicies(mockSoClient, mockAgentPolicyService, 'myPackage', {
page: 1,
per_page: 100,
sort_field: 'name',
sort_field: 'package_policy.name',
sort_order: 'asc',
});

Expand Down
Loading